Re: [PATCH v2] i3c: dw-i3c-master: Fix IBI count register selection for versalnet

From: Jeremy Kerr <jk@codeconstruct.com.au>
To: "Datta, Shubhrajyoti" <shubhrajyoti.datta@amd.com>,
	 "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Cc: "git (AMD-Xilinx)" <git@amd.com>,
	Alexandre Belloni <alexandre.belloni@bootlin.com>,
	Frank Li <Frank.Li@nxp.com>, Joel Stanley <joel@jms.id.au>,
	"linux-i3c@lists.infradead.org" <linux-i3c@lists.infradead.org>
Subject: Re: [PATCH v2] i3c: dw-i3c-master: Fix IBI count register selection for versalnet
Date: Thu, 02 Apr 2026 16:32:30 +0800	[thread overview]
Message-ID: <5cb83afcd5340a108fd5a7346494ccf73752cc27.camel@codeconstruct.com.au> (raw)
In-Reply-To: <LV5PR12MB9828D700D168803CBD022A708151A@LV5PR12MB9828.namprd12.prod.outlook.com>

Hi Shubhrajyoti,

> > How are you binding the driver to this device? Are you using a unique OF
> > compatible string, or something ACPI-based?
> > 
> > ... and if that can be specific to this hardware instance, would that be an
> > effective mechanism to select the IBI read method instead?
> Hi Jeremy,
> 
> VersalNet currently uses the generic "snps,dw-i3c-master-1.00a"
> compatible string — there is no unique compatible string for this
> hardware instance. The DTS    entry looks like:
> 
>   compatible = "snps,dw-i3c-master-1.00a";

You should *always* have a unique compatible string for each device
instance, if there can be any variation in behaviours from that generic
one (which you certainly do have here).

You can still fall-back on a generic one, but using that as your only
compatible value means you can't do device-specific behaviours in your
driver, as you have just found.

>   We could introduce a VersalNet-specific compatible with a generic fallback:
> 
>   compatible = "xlnx,versalnet-dw-i3c-master", "snps,dw-i3c-master-1.00a";

Yes, you want that anyway.

>   and pass a quirk flag via of_device_id.data to select the IBI read method.

Exactly, and there is already the struct dw_i3c_drvdata to help with
this.

> However, the probe-time detection avoids having to enumerate all affected
> variants — the IC_HAS_IBI_DATA=0 configuration is a synthesizable
> option in the IP and may appear in other SoCs using the same core.
> 
> Do you have a preference? If the DTS change is acceptable, I can go
> with the compatible + match-data approach

That would certainly be my recommendation.

If it's a synthesisable option, it may even be worth adding as a flag to
the binding definition (along with the same for other options that might
be present). This would mean you don't need a-priori knowledge of the
mapping of compatible strings to their synthesis options.

I don't have any access to documentation on those options though, so
can't be of much help with doing that.

> However I thought that detecting it will be helpful for backward compatible.

Given it doesn't work at the moment, there shouldn't be any backwards
compat concerns, I think?

Cheers,

Jeremy

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