On 28 June 2020 1:26:05 AM IST, Kurt Van Dijck <dev.kurt@xxxxxxxxxxxxxxxxxxxxxx> wrote: >On vr, 26 jun 2020 19:02:43 +0530, Manivannan Sadhasivam wrote: >> Hi, >> >> On Mon, Jun 22, 2020 at 01:46:03PM +0200, Marc Kleine-Budde wrote: >> > This patch add support for the Microchip MCP25xxFD SPI CAN >controller family. >> > >> > Pending-Tested-by: Kurt Van Dijck <dev.kurt@xxxxxxxxxxxxxxxxxxxxxx> >> > Pending-Tested-by: Manivannan Sadhasivam ><manivannan.sadhasivam@xxxxxxxxxx> >> > Signed-off-by: Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx> >> >> Could you please split this patch into multiple ones? Having ~4k >lines for a >> patch makes it difficult to review. I know that some parts are >difficult to >> split (happened with my series as well) but anything below 1k should >be fine. > >IMHO, there is a huge difference between a complete new driver of 4k >lines and a change of 100 lines. >It's useless to add a non-functional driver, only to add functionality >later on. > I just asked to split commits in this series logically. Because, its hard for the reviewers to do the review of a bulk patch. This is an unwritten rule of the kernel community... >You're right concerning real changes to a driver. > >Just my opinion ... > >From what I see, Marc did a good job, providing minimal functionality >in >the first series. now, the driver can evolve (like adding listen-only >:-) ) Again, I have no concern over the patch series. I'm also planning to submit incremental patches on top of this. Thanks, Mani > >Kurt > >> >> Thanks, >> Mani >> >> > --- >> > 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 | 2890 >+++++++++++++++++ >> > .../net/can/spi/mcp25xxfd/mcp25xxfd-crc16.c | 89 + >> > .../net/can/spi/mcp25xxfd/mcp25xxfd-regmap.c | 554 ++++ >> > drivers/net/can/spi/mcp25xxfd/mcp25xxfd.h | 828 +++++ >> > 8 files changed, 4389 insertions(+) >> > 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 >> > -- Sent from my Android device with K-9 Mail. Please excuse my brevity.