| Message ID | 20200910133806.25077-1-manivannan.sadhasivam@linaro.org |
|---|---|
| Headers | show |
| Series | Add support for MCP25XXFD SPI-CAN Network driver | expand |
Hi, On Thu, Sep 10, 2020 at 07:08:00PM +0530, Manivannan Sadhasivam wrote: > Hello, > > This series is the continuation of the work done by Marc Kleine-Budde on > adding support for Microchip MCP25XXFD SPI-CAN driver [1]. I've taken the > patches from Marc's tree [2] and posted with an additional MAINTAINERS > patch on top since he seems to be very busy. This series has been tested > on RB5 board featuring MCP2518FD transceiver. > > Marc: I'd like to co-maintain this driver in upstream hence added myself > as the co-maintainer. Shout at me if you do not want it! Also I've removed > the v4x tag since I don't think all the versions are posted to mailing > list. > > I'll add my review on top of this posting. > Just a quick question: I don't see any activity on this specific driver for sometime (back in Martin days itself). Is it due to lack of reviewers or it is due to the patch size (lines of code) so that nobody is interested in reviewing? How are we going to move forward? Thanks, Mani > Thanks, > Mani > > [1] https://www.spinics.net/lists/linux-can/msg03712.html > [2] https://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git/log/?h=mcp25xxfd-47 > > Kurt Van Dijck (1): > can: mcp25xxfd: add listen-only mode > > Manivannan Sadhasivam (1): > MAINTAINERS: Add entry for Microchip MCP25XXFD SPI-CAN network driver > > Marc Kleine-Budde (3): > can: rx-offload: can_rx_offload_add_manual(): add new initialization > function > can: mcp25xxfd: add regmap infrastructure > can: mcp25xxfd: add driver for Microchip MCP25xxFD SPI CAN > > Oleksij Rempel (1): > dt-bindings: can: mcp25xxfd: document device tree bindings > > .../bindings/net/can/microchip,mcp25xxfd.yaml | 79 + > MAINTAINERS | 8 + > drivers/net/can/rx-offload.c | 11 + > drivers/net/can/spi/Kconfig | 2 + > drivers/net/can/spi/Makefile | 1 + > drivers/net/can/spi/mcp25xxfd/Kconfig | 17 + > drivers/net/can/spi/mcp25xxfd/Makefile | 8 + > .../net/can/spi/mcp25xxfd/mcp25xxfd-core.c | 2884 +++++++++++++++++ > .../net/can/spi/mcp25xxfd/mcp25xxfd-crc16.c | 89 + > .../net/can/spi/mcp25xxfd/mcp25xxfd-regmap.c | 556 ++++ > drivers/net/can/spi/mcp25xxfd/mcp25xxfd.h | 828 +++++ > include/linux/can/rx-offload.h | 3 + > 12 files changed, 4486 insertions(+) > create mode 100644 Documentation/devicetree/bindings/net/can/microchip,mcp25xxfd.yaml > create mode 100644 drivers/net/can/spi/mcp25xxfd/Kconfig > create mode 100644 drivers/net/can/spi/mcp25xxfd/Makefile > create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd-core.c > create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd-crc16.c > create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd-regmap.c > create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd.h > > -- > 2.17.1 >
On di, 15 sep 2020 21:49:25 +0530, Manivannan Sadhasivam wrote: > Hi, > > On Thu, Sep 10, 2020 at 07:08:00PM +0530, Manivannan Sadhasivam wrote: > > Hello, > > Just a quick question: I don't see any activity on this specific driver for > sometime (back in Martin days itself). Is it due to lack of reviewers or > it is due to the patch size (lines of code) so that nobody is interested > in reviewing? If you look around, there are currently several versions of mcp251x driver around, shipped by hardware vendors who glue the chip on there SOM etc. Until something more-or-less clean becomes mainline, the effort remains spread. A problem to import a complete driver is that ... its complete. There was an suggestion to split into several patches, but that does not really affect the review work. The original driver failed to initialize under a loaded CAN bus, on my desk. The current driver is more cleanly written than the original and it seems to survive more than 1 use case (although I have a MAB overflow report pending to investigate). So, this is a good candidate for mainline. Kind regards, Kurt
Hi, On Tue, Sep 15, 2020 at 07:58:38PM +0200, Kurt Van Dijck wrote: > On di, 15 sep 2020 21:49:25 +0530, Manivannan Sadhasivam wrote: > > Hi, > > > > On Thu, Sep 10, 2020 at 07:08:00PM +0530, Manivannan Sadhasivam wrote: > > > Hello, > > > > Just a quick question: I don't see any activity on this specific driver for > > sometime (back in Martin days itself). Is it due to lack of reviewers or > > it is due to the patch size (lines of code) so that nobody is interested > > in reviewing? > > If you look around, there are currently several versions of mcp251x > driver around, shipped by hardware vendors who glue the chip on there > SOM etc. > Until something more-or-less clean becomes mainline, the effort remains > spread. > > A problem to import a complete driver is that ... its complete. > There was an suggestion to split into several patches, but that does not > really affect the review work. > > The original driver failed to initialize under a loaded CAN bus, on my > desk. The current driver is more cleanly written than the original > and it seems to survive more than 1 use case (although I have a MAB overflow > report pending to investigate). > So, this is a good candidate for mainline. > I just saw that you've pushed these patches to your testing branch. Does this mean that you're going to include it in v5.10 PR? Thanks, Mani > Kind regards, > Kurt
On 9/16/20 6:07 AM, Manivannan Sadhasivam wrote: >>> Just a quick question: I don't see any activity on this specific driver for >>> sometime (back in Martin days itself). Is it due to lack of reviewers or >>> it is due to the patch size (lines of code) so that nobody is interested >>> in reviewing? >> >> If you look around, there are currently several versions of mcp251x >> driver around, shipped by hardware vendors who glue the chip on there >> SOM etc. >> Until something more-or-less clean becomes mainline, the effort remains >> spread. >> >> A problem to import a complete driver is that ... its complete. >> There was an suggestion to split into several patches, but that does not >> really affect the review work. >> >> The original driver failed to initialize under a loaded CAN bus, on my >> desk. The current driver is more cleanly written than the original >> and it seems to survive more than 1 use case (although I have a MAB overflow >> report pending to investigate). >> So, this is a good candidate for mainline. > > I just saw that you've pushed these patches to your testing branch. Does this > mean that you're going to include it in v5.10 PR? yes, that's the plan. Marc -- Pengutronix e.K. | Marc Kleine-Budde | Embedded Linux | https://www.pengutronix.de | Vertretung West/Dortmund | Phone: +49-231-2826-924 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Hello, This series is the continuation of the work done by Marc Kleine-Budde on adding support for Microchip MCP25XXFD SPI-CAN driver [1]. I've taken the patches from Marc's tree [2] and posted with an additional MAINTAINERS patch on top since he seems to be very busy. This series has been tested on RB5 board featuring MCP2518FD transceiver. Marc: I'd like to co-maintain this driver in upstream hence added myself as the co-maintainer. Shout at me if you do not want it! Also I've removed the v4x tag since I don't think all the versions are posted to mailing list. I'll add my review on top of this posting. Thanks, Mani [1] https://www.spinics.net/lists/linux-can/msg03712.html [2] https://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git/log/?h=mcp25xxfd-47 Kurt Van Dijck (1): can: mcp25xxfd: add listen-only mode Manivannan Sadhasivam (1): MAINTAINERS: Add entry for Microchip MCP25XXFD SPI-CAN network driver Marc Kleine-Budde (3): can: rx-offload: can_rx_offload_add_manual(): add new initialization function can: mcp25xxfd: add regmap infrastructure can: mcp25xxfd: add driver for Microchip MCP25xxFD SPI CAN Oleksij Rempel (1): dt-bindings: can: mcp25xxfd: document device tree bindings .../bindings/net/can/microchip,mcp25xxfd.yaml | 79 + MAINTAINERS | 8 + drivers/net/can/rx-offload.c | 11 + drivers/net/can/spi/Kconfig | 2 + drivers/net/can/spi/Makefile | 1 + drivers/net/can/spi/mcp25xxfd/Kconfig | 17 + drivers/net/can/spi/mcp25xxfd/Makefile | 8 + .../net/can/spi/mcp25xxfd/mcp25xxfd-core.c | 2884 +++++++++++++++++ .../net/can/spi/mcp25xxfd/mcp25xxfd-crc16.c | 89 + .../net/can/spi/mcp25xxfd/mcp25xxfd-regmap.c | 556 ++++ drivers/net/can/spi/mcp25xxfd/mcp25xxfd.h | 828 +++++ include/linux/can/rx-offload.h | 3 + 12 files changed, 4486 insertions(+) create mode 100644 Documentation/devicetree/bindings/net/can/microchip,mcp25xxfd.yaml create mode 100644 drivers/net/can/spi/mcp25xxfd/Kconfig create mode 100644 drivers/net/can/spi/mcp25xxfd/Makefile create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd-core.c create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd-crc16.c create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd-regmap.c create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd.h -- 2.17.1