mbox series

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

Message ID 20230328073328.3949796-1-dario.binacchi@amarulasolutions.com (mailing list archive)
Headers show
Series can: bxcan: add support for ST bxCAN controller | expand

Message

Dario Binacchi March 28, 2023, 7:33 a.m. UTC
The series adds support for the basic extended CAN controller (bxCAN)
found in many low- to middle-end STM32 SoCs.

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 v10:
- Fix errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'.
  Fix the "st,can-primary" description removing the "Note:" word that
  caused the failure.
- Slightly change the note text at the top of the driver module. No
  functional changes.

Changes in v9:
- Fix commit description formatting. No semantic changes have been made.
- Replace master/slave terms with primary/secondary.
- Replace master/slave terms with primary/secondary.
- Replace master/slave terms with primary/secondary.

Changes in v8:
- Do not enable the clock in probe and enable/disable it in open/close.
- Return IRQ_NONE if no IRQ is active.

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      |   85 ++
 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                       | 1098 +++++++++++++++++
 8 files changed, 1264 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/can/st,stm32-bxcan.yaml
 create mode 100644 drivers/net/can/bxcan.c

Comments

Marc Kleine-Budde March 28, 2023, 8:47 a.m. UTC | #1
On 28.03.2023 09:33:23, Dario Binacchi wrote:
> The series adds support for the basic extended CAN controller (bxCAN)
> found in many low- to middle-end STM32 SoCs.
> 
> 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.

Applied to linux-can-next.

Thanks,
Marc
Dario Binacchi March 28, 2023, 9:28 a.m. UTC | #2
Hi Marc,

On Tue, Mar 28, 2023 at 10:47 AM Marc Kleine-Budde <mkl@pengutronix.de> wrote:
>
> On 28.03.2023 09:33:23, Dario Binacchi wrote:
> > The series adds support for the basic extended CAN controller (bxCAN)
> > found in many low- to middle-end STM32 SoCs.
> >
> > 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.
>
> Applied to linux-can-next.

Just one last question:
To test this series, as described in the cover letter, I could not use
the iproute2
package since the microcontroller is without MMU. I then extended busybox for
the ip link command. I actually also added the rtnl-link-can.c
application to the
libmnl library. So now I find myself with two applications that have
been useful
to me for this type of use case.
Did I do useless work because I could use other tools? If instead the tools for
this use case are missing, what do you think is better to do?
Submit to their respective repos or add this functionality to another
project that
I haven't considered ?

Thanks and regards,
Dario

>
> Thanks,
> 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 |
Marc Kleine-Budde April 4, 2023, 11:28 a.m. UTC | #3
On 28.03.2023 11:28:59, Dario Binacchi wrote:
> > Applied to linux-can-next.
> 
> Just one last question: To test this series, as described in the cover
> letter, I could not use the iproute2 package since the microcontroller
> is without MMU. I then extended busybox for the ip link command. I
> actually also added the rtnl-link-can.c application to the libmnl
> library. So now I find myself with two applications that have been
> useful to me for this type of use case.
> 
> Did I do useless work because I could use other tools?

systemd-networkd also supports CAN configuration, but I this will
probably not work on no-MMU systemd, too.

Then there is:

| https://git.pengutronix.de/cgit/tools/canutils
| https://git.pengutronix.de/cgit/tools/libsocketcan

that contains canconfig, but it lacks CAN-FD support.

> If instead the tools for this use case are missing, what do you think
> is better to do? Submit to their respective repos or add this
> functionality to another project that I haven't considered ?

Yes, go ahead and upstream your changes!

Marc