[RESEND,v7,0/5] can: bxcan: add support for ST bxCAN controller

The series adds support for the basic extended CAN controller (bxCAN)
found in many low- to middle-end STM32 SoCs.

The driver design (one core module and one driver module) was inspired
by other ST drivers (e. g. drivers/iio/adc/stm32-adc.c,
drivers/iio/adc/stm32-adc-core.c) where device instances share resources.
The shared resources functions are implemented in the core module, the
device driver in a separate module.

The driver has been tested on the stm32f469i-discovery board with a
kernel version 5.19.0-rc2 in loopback + silent mode:

ip link set can0 type can bitrate 125000 loopback on listen-only on
ip link set up can0
candump can0 -L &
cansend can0 300#AC.AB.AD.AE.75.49.AD.D1

For uboot and kernel compilation, as well as for rootfs creation I used
buildroot:

make stm32f469_disco_sd_defconfig
make

but I had to patch can-utils and busybox as can-utils and iproute are
not compiled for MMU-less microcotrollers. In the case of can-utils,
replacing the calls to fork() with vfork(), I was able to compile the
package with working candump and cansend applications, while in the
case of iproute, I ran into more than one problem and finally I decided
to extend busybox's ip link command for CAN-type devices. I'm still
wondering if it was really necessary, but this way I was able to test
the driver.

Changes in v7:
- Add Vincent Mailhol's Reviewed-by tag.
- Remove all unused macros for reading/writing the controller registers.
- Add CAN_ERR_CNT flag to notify availability of error counter.
- Move the "break" before the newline in the switch/case statements.
- Print the mnemotechnic instead of the error value in each netdev_err().
- Remove the debug print for timings parameter.
- Do not copy the data if CAN_RTR_FLAG is set in bxcan_start_xmit().
- Populate ndev->ethtool_ops with the default timestamp info.

Changes in v6:
- move can1 node before gcan to keep ordering by address.

Changes in v5:
- Add Rob Herring's Acked-by tag.
- Add Rob Herring's Reviewed-by tag.
- Put static in front of bxcan_enable_filters() definition.

Changes in v4:
- Remove "st,stm32f4-bxcan-core" compatible. In this way the can nodes
 (compatible "st,stm32f4-bxcan") are no longer children of a parent
  node with compatible "st,stm32f4-bxcan-core".
- Add the "st,gcan" property (global can memory) to can nodes which
  references a "syscon" node containing the shared clock and memory
  addresses.
- Replace the node can@40006400 (compatible "st,stm32f4-bxcan-core")
  with the gcan@40006600 node ("sysnode" compatible). The gcan node
  contains clocks and memory addresses shared by the two can nodes
  of which it's no longer the parent.
- Add to can nodes the "st,gcan" property (global can memory) which
  references the gcan@40006600 node ("sysnode compatibble).
- Add "dt-bindings: arm: stm32: add compatible for syscon gcan node" patch.
- Drop the core driver. Thus bxcan-drv.c has been renamed to bxcan.c and
  moved to the drivers/net/can folder. The drivers/net/can/bxcan directory
  has therefore been removed.
- Use the regmap_*() functions to access the shared memory registers.
- Use spinlock to protect bxcan_rmw().
- Use 1 space, instead of tabs, in the macros definition.
- Drop clock ref-counting.
- Drop unused code.
- Drop the _SHIFT macros and use FIELD_GET()/FIELD_PREP() directly.
- Add BXCAN_ prefix to lec error codes.
- Add the macro BXCAN_RX_MB_NUM.
- Enable time triggered mode and use can_rx_offload().
- Use readx_poll_timeout() in function with timeouts.
- Loop from tail to head in bxcan_tx_isr().
- Check bits of tsr register instead of pkts variable in bxcan_tx_isr().
- Don't return from bxcan_handle_state_change() if skb/cf are NULL.
- Enable/disable the generation of the bus error interrupt depending
  on can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING.
- Don't return from bxcan_handle_bus_err() if skb is NULL.
- Drop statistics updating from bxcan_handle_bus_err().
- Add an empty line in front of 'return IRQ_HANDLED;'
- Rename bxcan_start() to bxcan_chip_start().
- Rename bxcan_stop() to bxcan_chip_stop().
- Disable all IRQs in bxcan_chip_stop().
- Rename bxcan_close() to bxcan_ndo_stop().
- Use writel instead of bxcan_rmw() to update the dlc register.

Changes in v3:
- Remove 'Dario Binacchi <dariobin@libero.it>' SOB.
- Add description to the parent of the two child nodes.
- Move "patterProperties:" after "properties: in top level before "required".
- Add "clocks" to the "required:" list of the child nodes.
- Remove 'Dario Binacchi <dariobin@libero.it>' SOB.
- Add "clocks" to can@0 node.
- Remove 'Dario Binacchi <dariobin@libero.it>' SOB.
- Remove a blank line.
- Remove 'Dario Binacchi <dariobin@libero.it>' SOB.
- Fix the documentation file path in the MAINTAINERS entry.
- Do not increment the "stats->rx_bytes" if the frame is remote.
- Remove pr_debug() call from bxcan_rmw().

Changes in v2:
- Change the file name into 'st,stm32-bxcan-core.yaml'.
- Rename compatibles:
  - st,stm32-bxcan-core -> st,stm32f4-bxcan-core
  - st,stm32-bxcan -> st,stm32f4-bxcan
- Rename master property to st,can-master.
- Remove the status property from the example.
- Put the node child properties as required.
- Remove a blank line.
- Fix sparse errors.
- Create a MAINTAINERS entry.
- Remove the print of the registers address.
- Remove the volatile keyword from bxcan_rmw().
- Use tx ring algorithm to manage tx mailboxes.
- Use can_{get|put}_echo_skb().
- Update DT properties.

Dario Binacchi (5):
  dt-bindings: arm: stm32: add compatible for syscon gcan node
  dt-bindings: net: can: add STM32 bxcan DT bindings
  ARM: dts: stm32: add CAN support on stm32f429
  ARM: dts: stm32: add pin map for CAN controller on stm32f4
  can: bxcan: add support for ST bxCAN controller

 .../bindings/arm/stm32/st,stm32-syscon.yaml   |    2 +
 .../bindings/net/can/st,stm32-bxcan.yaml      |   83 ++
 MAINTAINERS                                   |    7 +
 arch/arm/boot/dts/stm32f4-pinctrl.dtsi        |   30 +
 arch/arm/boot/dts/stm32f429.dtsi              |   29 +
 drivers/net/can/Kconfig                       |   12 +
 drivers/net/can/Makefile                      |    1 +
 drivers/net/can/bxcan.c                       | 1088 +++++++++++++++++
 8 files changed, 1252 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/can/st,stm32-bxcan.yaml
 create mode 100644 drivers/net/can/bxcan.c
  

A gentle ping to remind you of this series.
I have no idea why it hasn't deserved any response for quite some
time.
Is there anything I am still missing?

Please let me know.

Thanks and regards,

Dario


On Wed, Mar 15, 2023 at 10:10 PM Dario Binacchi
<dario.binacchi@amarulasolutions.com> wrote:
>
> The series adds support for the basic extended CAN controller (bxCAN)
> found in many low- to middle-end STM32 SoCs.
>
> The driver design (one core module and one driver module) was inspired
> by other ST drivers (e. g. drivers/iio/adc/stm32-adc.c,
> drivers/iio/adc/stm32-adc-core.c) where device instances share resources.
> The shared resources functions are implemented in the core module, the
> device driver in a separate module.
>
> The driver has been tested on the stm32f469i-discovery board with a
> kernel version 5.19.0-rc2 in loopback + silent mode:
>
> ip link set can0 type can bitrate 125000 loopback on listen-only on
> ip link set up can0
> candump can0 -L &
> cansend can0 300#AC.AB.AD.AE.75.49.AD.D1
>
> For uboot and kernel compilation, as well as for rootfs creation I used
> buildroot:
>
> make stm32f469_disco_sd_defconfig
> make
>
> but I had to patch can-utils and busybox as can-utils and iproute are
> not compiled for MMU-less microcotrollers. In the case of can-utils,
> replacing the calls to fork() with vfork(), I was able to compile the
> package with working candump and cansend applications, while in the
> case of iproute, I ran into more than one problem and finally I decided
> to extend busybox's ip link command for CAN-type devices. I'm still
> wondering if it was really necessary, but this way I was able to test
> the driver.
>
> Changes in v7:
> - Add Vincent Mailhol's Reviewed-by tag.
> - Remove all unused macros for reading/writing the controller registers.
> - Add CAN_ERR_CNT flag to notify availability of error counter.
> - Move the "break" before the newline in the switch/case statements.
> - Print the mnemotechnic instead of the error value in each netdev_err().
> - Remove the debug print for timings parameter.
> - Do not copy the data if CAN_RTR_FLAG is set in bxcan_start_xmit().
> - Populate ndev->ethtool_ops with the default timestamp info.
>
> Changes in v6:
> - move can1 node before gcan to keep ordering by address.
>
> Changes in v5:
> - Add Rob Herring's Acked-by tag.
> - Add Rob Herring's Reviewed-by tag.
> - Put static in front of bxcan_enable_filters() definition.
>
> Changes in v4:
> - Remove "st,stm32f4-bxcan-core" compatible. In this way the can nodes
>  (compatible "st,stm32f4-bxcan") are no longer children of a parent
>   node with compatible "st,stm32f4-bxcan-core".
> - Add the "st,gcan" property (global can memory) to can nodes which
>   references a "syscon" node containing the shared clock and memory
>   addresses.
> - Replace the node can@40006400 (compatible "st,stm32f4-bxcan-core")
>   with the gcan@40006600 node ("sysnode" compatible). The gcan node
>   contains clocks and memory addresses shared by the two can nodes
>   of which it's no longer the parent.
> - Add to can nodes the "st,gcan" property (global can memory) which
>   references the gcan@40006600 node ("sysnode compatibble).
> - Add "dt-bindings: arm: stm32: add compatible for syscon gcan node" patch.
> - Drop the core driver. Thus bxcan-drv.c has been renamed to bxcan.c and
>   moved to the drivers/net/can folder. The drivers/net/can/bxcan directory
>   has therefore been removed.
> - Use the regmap_*() functions to access the shared memory registers.
> - Use spinlock to protect bxcan_rmw().
> - Use 1 space, instead of tabs, in the macros definition.
> - Drop clock ref-counting.
> - Drop unused code.
> - Drop the _SHIFT macros and use FIELD_GET()/FIELD_PREP() directly.
> - Add BXCAN_ prefix to lec error codes.
> - Add the macro BXCAN_RX_MB_NUM.
> - Enable time triggered mode and use can_rx_offload().
> - Use readx_poll_timeout() in function with timeouts.
> - Loop from tail to head in bxcan_tx_isr().
> - Check bits of tsr register instead of pkts variable in bxcan_tx_isr().
> - Don't return from bxcan_handle_state_change() if skb/cf are NULL.
> - Enable/disable the generation of the bus error interrupt depending
>   on can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING.
> - Don't return from bxcan_handle_bus_err() if skb is NULL.
> - Drop statistics updating from bxcan_handle_bus_err().
> - Add an empty line in front of 'return IRQ_HANDLED;'
> - Rename bxcan_start() to bxcan_chip_start().
> - Rename bxcan_stop() to bxcan_chip_stop().
> - Disable all IRQs in bxcan_chip_stop().
> - Rename bxcan_close() to bxcan_ndo_stop().
> - Use writel instead of bxcan_rmw() to update the dlc register.
>
> Changes in v3:
> - Remove 'Dario Binacchi <dariobin@libero.it>' SOB.
> - Add description to the parent of the two child nodes.
> - Move "patterProperties:" after "properties: in top level before "required".
> - Add "clocks" to the "required:" list of the child nodes.
> - Remove 'Dario Binacchi <dariobin@libero.it>' SOB.
> - Add "clocks" to can@0 node.
> - Remove 'Dario Binacchi <dariobin@libero.it>' SOB.
> - Remove a blank line.
> - Remove 'Dario Binacchi <dariobin@libero.it>' SOB.
> - Fix the documentation file path in the MAINTAINERS entry.
> - Do not increment the "stats->rx_bytes" if the frame is remote.
> - Remove pr_debug() call from bxcan_rmw().
>
> Changes in v2:
> - Change the file name into 'st,stm32-bxcan-core.yaml'.
> - Rename compatibles:
>   - st,stm32-bxcan-core -> st,stm32f4-bxcan-core
>   - st,stm32-bxcan -> st,stm32f4-bxcan
> - Rename master property to st,can-master.
> - Remove the status property from the example.
> - Put the node child properties as required.
> - Remove a blank line.
> - Fix sparse errors.
> - Create a MAINTAINERS entry.
> - Remove the print of the registers address.
> - Remove the volatile keyword from bxcan_rmw().
> - Use tx ring algorithm to manage tx mailboxes.
> - Use can_{get|put}_echo_skb().
> - Update DT properties.
>
> Dario Binacchi (5):
>   dt-bindings: arm: stm32: add compatible for syscon gcan node
>   dt-bindings: net: can: add STM32 bxcan DT bindings
>   ARM: dts: stm32: add CAN support on stm32f429
>   ARM: dts: stm32: add pin map for CAN controller on stm32f4
>   can: bxcan: add support for ST bxCAN controller
>
>  .../bindings/arm/stm32/st,stm32-syscon.yaml   |    2 +
>  .../bindings/net/can/st,stm32-bxcan.yaml      |   83 ++
>  MAINTAINERS                                   |    7 +
>  arch/arm/boot/dts/stm32f4-pinctrl.dtsi        |   30 +
>  arch/arm/boot/dts/stm32f429.dtsi              |   29 +
>  drivers/net/can/Kconfig                       |   12 +
>  drivers/net/can/Makefile                      |    1 +
>  drivers/net/can/bxcan.c                       | 1088 +++++++++++++++++
>  8 files changed, 1252 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/can/st,stm32-bxcan.yaml
>  create mode 100644 drivers/net/can/bxcan.c
>
> --
> 2.32.0
>
  

Hi Dario,


On 3/21/23 12:25, Dario Binacchi wrote:
> A gentle ping to remind you of this series.
> I have no idea why it hasn't deserved any response for quite some
> time.
> Is there anything I am still missing?
> 
> Please let me know.

I'm just waiting driver (+dt-binding) merge. I prefer that dt-bindings 
and driver patches are merged first (to avoid yaml issue with DT 
pacthes). To be honest, I have not checked the status about those patches.


Cheers
Alex



> Thanks and regards,
> 
> Dario
> 
> 
> On Wed, Mar 15, 2023 at 10:10 PM Dario Binacchi
> <dario.binacchi@amarulasolutions.com> wrote:
>>
>> The series adds support for the basic extended CAN controller (bxCAN)
>> found in many low- to middle-end STM32 SoCs.
>>
>> The driver design (one core module and one driver module) was inspired
>> by other ST drivers (e. g. drivers/iio/adc/stm32-adc.c,
>> drivers/iio/adc/stm32-adc-core.c) where device instances share resources.
>> The shared resources functions are implemented in the core module, the
>> device driver in a separate module.
>>
>> The driver has been tested on the stm32f469i-discovery board with a
>> kernel version 5.19.0-rc2 in loopback + silent mode:
>>
>> ip link set can0 type can bitrate 125000 loopback on listen-only on
>> ip link set up can0
>> candump can0 -L &
>> cansend can0 300#AC.AB.AD.AE.75.49.AD.D1
>>
>> For uboot and kernel compilation, as well as for rootfs creation I used
>> buildroot:
>>
>> make stm32f469_disco_sd_defconfig
>> make
>>
>> but I had to patch can-utils and busybox as can-utils and iproute are
>> not compiled for MMU-less microcotrollers. In the case of can-utils,
>> replacing the calls to fork() with vfork(), I was able to compile the
>> package with working candump and cansend applications, while in the
>> case of iproute, I ran into more than one problem and finally I decided
>> to extend busybox's ip link command for CAN-type devices. I'm still
>> wondering if it was really necessary, but this way I was able to test
>> the driver.
>>
>> Changes in v7:
>> - Add Vincent Mailhol's Reviewed-by tag.
>> - Remove all unused macros for reading/writing the controller registers.
>> - Add CAN_ERR_CNT flag to notify availability of error counter.
>> - Move the "break" before the newline in the switch/case statements.
>> - Print the mnemotechnic instead of the error value in each netdev_err().
>> - Remove the debug print for timings parameter.
>> - Do not copy the data if CAN_RTR_FLAG is set in bxcan_start_xmit().
>> - Populate ndev->ethtool_ops with the default timestamp info.
>>
>> Changes in v6:
>> - move can1 node before gcan to keep ordering by address.
>>
>> Changes in v5:
>> - Add Rob Herring's Acked-by tag.
>> - Add Rob Herring's Reviewed-by tag.
>> - Put static in front of bxcan_enable_filters() definition.
>>
>> Changes in v4:
>> - Remove "st,stm32f4-bxcan-core" compatible. In this way the can nodes
>>   (compatible "st,stm32f4-bxcan") are no longer children of a parent
>>    node with compatible "st,stm32f4-bxcan-core".
>> - Add the "st,gcan" property (global can memory) to can nodes which
>>    references a "syscon" node containing the shared clock and memory
>>    addresses.
>> - Replace the node can@40006400 (compatible "st,stm32f4-bxcan-core")
>>    with the gcan@40006600 node ("sysnode" compatible). The gcan node
>>    contains clocks and memory addresses shared by the two can nodes
>>    of which it's no longer the parent.
>> - Add to can nodes the "st,gcan" property (global can memory) which
>>    references the gcan@40006600 node ("sysnode compatibble).
>> - Add "dt-bindings: arm: stm32: add compatible for syscon gcan node" patch.
>> - Drop the core driver. Thus bxcan-drv.c has been renamed to bxcan.c and
>>    moved to the drivers/net/can folder. The drivers/net/can/bxcan directory
>>    has therefore been removed.
>> - Use the regmap_*() functions to access the shared memory registers.
>> - Use spinlock to protect bxcan_rmw().
>> - Use 1 space, instead of tabs, in the macros definition.
>> - Drop clock ref-counting.
>> - Drop unused code.
>> - Drop the _SHIFT macros and use FIELD_GET()/FIELD_PREP() directly.
>> - Add BXCAN_ prefix to lec error codes.
>> - Add the macro BXCAN_RX_MB_NUM.
>> - Enable time triggered mode and use can_rx_offload().
>> - Use readx_poll_timeout() in function with timeouts.
>> - Loop from tail to head in bxcan_tx_isr().
>> - Check bits of tsr register instead of pkts variable in bxcan_tx_isr().
>> - Don't return from bxcan_handle_state_change() if skb/cf are NULL.
>> - Enable/disable the generation of the bus error interrupt depending
>>    on can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING.
>> - Don't return from bxcan_handle_bus_err() if skb is NULL.
>> - Drop statistics updating from bxcan_handle_bus_err().
>> - Add an empty line in front of 'return IRQ_HANDLED;'
>> - Rename bxcan_start() to bxcan_chip_start().
>> - Rename bxcan_stop() to bxcan_chip_stop().
>> - Disable all IRQs in bxcan_chip_stop().
>> - Rename bxcan_close() to bxcan_ndo_stop().
>> - Use writel instead of bxcan_rmw() to update the dlc register.
>>
>> Changes in v3:
>> - Remove 'Dario Binacchi <dariobin@libero.it>' SOB.
>> - Add description to the parent of the two child nodes.
>> - Move "patterProperties:" after "properties: in top level before "required".
>> - Add "clocks" to the "required:" list of the child nodes.
>> - Remove 'Dario Binacchi <dariobin@libero.it>' SOB.
>> - Add "clocks" to can@0 node.
>> - Remove 'Dario Binacchi <dariobin@libero.it>' SOB.
>> - Remove a blank line.
>> - Remove 'Dario Binacchi <dariobin@libero.it>' SOB.
>> - Fix the documentation file path in the MAINTAINERS entry.
>> - Do not increment the "stats->rx_bytes" if the frame is remote.
>> - Remove pr_debug() call from bxcan_rmw().
>>
>> Changes in v2:
>> - Change the file name into 'st,stm32-bxcan-core.yaml'.
>> - Rename compatibles:
>>    - st,stm32-bxcan-core -> st,stm32f4-bxcan-core
>>    - st,stm32-bxcan -> st,stm32f4-bxcan
>> - Rename master property to st,can-master.
>> - Remove the status property from the example.
>> - Put the node child properties as required.
>> - Remove a blank line.
>> - Fix sparse errors.
>> - Create a MAINTAINERS entry.
>> - Remove the print of the registers address.
>> - Remove the volatile keyword from bxcan_rmw().
>> - Use tx ring algorithm to manage tx mailboxes.
>> - Use can_{get|put}_echo_skb().
>> - Update DT properties.
>>
>> Dario Binacchi (5):
>>    dt-bindings: arm: stm32: add compatible for syscon gcan node
>>    dt-bindings: net: can: add STM32 bxcan DT bindings
>>    ARM: dts: stm32: add CAN support on stm32f429
>>    ARM: dts: stm32: add pin map for CAN controller on stm32f4
>>    can: bxcan: add support for ST bxCAN controller
>>
>>   .../bindings/arm/stm32/st,stm32-syscon.yaml   |    2 +
>>   .../bindings/net/can/st,stm32-bxcan.yaml      |   83 ++
>>   MAINTAINERS                                   |    7 +
>>   arch/arm/boot/dts/stm32f4-pinctrl.dtsi        |   30 +
>>   arch/arm/boot/dts/stm32f429.dtsi              |   29 +
>>   drivers/net/can/Kconfig                       |   12 +
>>   drivers/net/can/Makefile                      |    1 +
>>   drivers/net/can/bxcan.c                       | 1088 +++++++++++++++++
>>   8 files changed, 1252 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/net/can/st,stm32-bxcan.yaml
>>   create mode 100644 drivers/net/can/bxcan.c
>>
>> --
>> 2.32.0
>>
> 
>
  

On 21.03.2023 12:25:15, Dario Binacchi wrote:
> A gentle ping to remind you of this series.
> I have no idea why it hasn't deserved any response for quite some
> time.
> Is there anything I am still missing?

I wonder if we want to do a s/master/primary/ in the DT bindings and
driver?

regards,
Marc
  

Hi Marc,

On Fri, Mar 24, 2023 at 4:56 PM Marc Kleine-Budde <mkl@pengutronix.de> wrote:
>
> On 21.03.2023 12:25:15, Dario Binacchi wrote:
> > A gentle ping to remind you of this series.
> > I have no idea why it hasn't deserved any response for quite some
> > time.
> > Is there anything I am still missing?
>
> I wonder if we want to do a s/master/primary/ in the DT bindings and
> driver?

The ST reference manual (RM0386) explicitly uses the master and slave words
in the bxcan chapter. I would stay consistent with it. But I have no problem
changing it to primary. I just sent v8 with the changes you suggested
for shared irq
and clock enable/disable, but if you prefer to use primary I will send
the v9 version
with that change.
Please let me know your opinion.

Thanks and regards,
Dario

>
> regards,
> Marc
>
> --
> Pengutronix e.K.                 | Marc Kleine-Budde           |
> Embedded Linux                   | https://www.pengutronix.de  |
> Vertretung Nürnberg              | Phone: +49-5121-206917-129  |
> Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |
  

On 26.03.2023 18:07:14, Dario Binacchi wrote:
> > On 21.03.2023 12:25:15, Dario Binacchi wrote:
> > > A gentle ping to remind you of this series.
> > > I have no idea why it hasn't deserved any response for quite some
> > > time.
> > > Is there anything I am still missing?
> >
> > I wonder if we want to do a s/master/primary/ in the DT bindings and
> > driver?
> 
> The ST reference manual (RM0386) explicitly uses the master and slave words
> in the bxcan chapter.

ACK

> I would stay consistent with it.

Yes, this is a known problem, on the one hand I'd like the drivers to
match the datasheet, but here I am in favor of a deviation.

> But I have no problem changing it to primary. I just sent v8 with the
> changes you suggested for shared irq and clock enable/disable,

These changes look good!

> but if you prefer to use primary I will send the v9 version with that
> change. Please let me know your opinion.

Please convert the driver and bindings to use "primary". Feel free to
mention that the datasheet calls the primary peripheral "master".

Marc