Message ID | 20230315211040.2455855-1-dario.binacchi@amarulasolutions.com |
---|---|
Headers | show |
Series |
|
Related | show |
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