Re: [PATCH 2/4] i3c: master: add Microchip SAMA7D65 I3C HCI master driver

From: Wolfram Sang <wsa+renesas@sang-engineering.com>
To: Durai.ManickamKR@microchip.com
Cc: Frank.li@nxp.com, linux-i3c@lists.infradead.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	alexandre.belloni@bootlin.com, robh@kernel.org,
	krzk+dt@kernel.org, conor+dt@kernel.org,
	Balamanikandan.Gunasundar@microchip.com,
	Nicolas.Ferre@microchip.com
Subject: Re: [PATCH 2/4] i3c: master: add Microchip SAMA7D65 I3C HCI master driver
Date: Wed, 10 Sep 2025 08:53:44 +0200	[thread overview]
Message-ID: <aMEgeHLnrDyGjYM3@shikoro> (raw)
In-Reply-To: <3229da67-9d67-47ff-9f01-0d71bfabb6a6@microchip.com>

Hi Durai,

> 1. To introduce a Microchip SoC specific macro to add Microchip specific 
> I3C changes to the existing driver. But in this approach, if suppose we 
> have a changes in the i3c-hci driver, we have to adapt the changes to 
> our IP also during every kernel updation and I donot know whether this 
> is accepted by the i3c-hci driver maintainer.
> 
> 2. Otherwise, creating a separate platform driver by reusing the 
> existing i3c-hci driver.

Usually, 2) is the way to go for the reason you gave above. If there are
fixes to the core part, you don't need to sync with your driver.
Maintenance burden is lower for most of the times.

If the hardware is so different that the modifications to the core
driver turn out to be more complex (and harder to maintain) than a
seperate driver, then 1) can be an option.

Without knowing your hardware, from the description above I'd think you
can reuse the existing HCI core driver. It is mainly about not using
stuff like DMA. Maybe that can be handled with newly introduced quirk
flags.

> I raised a query few weeks back to decide which approach to proceed 
> before sending this patch to upstream. But i have received comments like 
> "its upto us to decide which is best way". I dont have much idea on 
> which is best way to proceed and maintain.So, I have decided to go with 
> approach 2.

Your patch looks like approach 1), though? You don't hook into the HCI
driver or am I overlooking this?

> >> Features tested and supported :
> >>     Standard CCC commands.
> >>     I3C SDR mode private transfers in PIO mode.
> >>     I2C transfers in PIO mode.
> >>     Pure bus mode and mixed bus mode.
> >>
> >> Signed-off-by: Durai Manickam KR <durai.manickamkr@microchip.com>

Please delete lines you don't quote anymore (here, the whole driver).

Happy hacking,

   Wolfram

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c