Re: [PATCH v41 3/3] can: mcp25xxfd: initial commit

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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.




[Index of Archives]     [Automotive Discussions]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]     [CAN Bus]

  Powered by Linux