| Message ID | 20230315211040.2455855-1-dario.binacchi@amarulasolutions.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | can: bxcan: add support for ST bxCAN controller | expand |
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
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