Message-ID: <aW9JihlsjnJ-uBul@black.igk.intel.com>
Date: Tue, 20 Jan 2026 10:23:22 +0100
From: Andy Shevchenko <andriy.shevchenko@...el.com>
To: Mark Brown <broonie@...nel.org>
Cc: Michal Simek <michal.simek@....com>,
	Abdurrahman Hussain <abdurrahman@...thop.ai>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>, linux-spi@...r.kernel.org,
	devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 3/3] spi: xilinx: use device property accessors.

On Mon, Jan 19, 2026 at 07:56:57PM +0000, Mark Brown wrote:
> On Mon, Jan 19, 2026 at 08:17:46PM +0100, Michal Simek wrote:
> > On 1/19/26 20:01, Mark Brown wrote:
> > > On Mon, Jan 19, 2026 at 07:52:35PM +0100, Michal Simek wrote:

> > > > Is it a better way to use auxiliary bus as was recommended by Greg in past
> > > > on drivers/misc/keba/cp500.c review?
> > > > https://lore.kernel.org/linux-i2c/2024060203-impeding-curing-e6cd@gregkh/
> 
> > > The driver there appears to be doing runtime enumeration based on some
> > > EEPROMs on the system and creating platform devices based on what it
> > > finds there so it's a bit of a different thing, the aux bus suggestion
> > > is about what the code that does with the data it got from the EEPROM.
> > > This patch is for something described directly by firmware so there's no
> > > way we'd create an aux device, that's purely in kernel.
> 
> > I don't thing it is actually eeprom because in fpga you can place at certain
> > location just memory (or RO memory) to describe what it is inside.
> 
> The table of per module I2C EEPROM devices (there's a whole pile flagged
> as optional) sure does look like it, and it's a very standard way to
> implement hardware enumeration of non-enumerable systems.
> 
> > If you know it I think you have multiple options how to wire existing drivers.
> 
> > 1. ACPI - which is what this series is trying to do
> 
> Is it?  It just looks like random cleanups.  We've got a change to make
> interrupts optional and this change to device properties - the cover
> letter just says it's a transition to device property but there's no
> indiciation why it's being done.  The cover letter for the series just
> says it's switching to device properties with no further explanation.
> It looks like a "that's the newer API" thing than something that's been
> thought through.
> 
> None of this looks like something intended to add ACPI bindings,
> it's not clear to me how we'd even get the device instantiated on a
> normal ACPI system.  There's no ACPI IDs defined (and there aren't any
> existing ones), just a conversion of the property parsing code.

For the matter of this change, the only thing that can be preferred in ACPI is
to have the real ID, id est ACPI _HID (allocated by the vendor, namely Xilix).
So, ideally at the end it should be enumerated by ACPI ID table with properly
allocated HID and not with the compatible line.

The compatible line is for prototyping and DIY (discrete components, like
small temperature sensors, when allocating proper ACPI ID is a big deal
for a vendor).

The set of _DSD properties is okay to be shared even in that case.

TL;DR: We prefer ACPI HID to be properly allocated and used, or explained
why it can't be feasible (which I don't believe the case for Xilinx).

> > 2. DT - on x86 not sure if feasible
> 
> No, x86 decided not to use DT and shoehorn everything into APCI (and the
> x86 SoCs put their platform devices behind fake PCI that looks like PCI
> to the OS).

There are three platforms in the past that used DT on x86.
Latest one was dated ca. 2017 (SpreadTrum SoC with x86 core in it).

DT on x86 is a real thing, but very niche.

> > 3. platform drivers - as described above by Greg not an option on PCIe
> > 4. aux bus - for example keba drivers
> > 5. dfl - drivers/fpga/dfl* - used for accelerators.
> 
> These are orthogonal to the above, they're Linux internal things not a
> concept the firmware has.  You have firmware descriptions of things that
> can be mapped onto platform devices but that's a separate thing.
> 
> > Pretty much all current Xilinx drivers for soft IPs (spi, i2c, uarts,
> > watchdogs, etc) are platform drivers (more OF drivers because platform data
> > are mostly not used).
> 
> > It means I think would be good to get any recommendation which way to go.
> 
> You should use whatever firmware interface is sensible for the platform,
> if that's x86 that's always ACPI.  For other architectures there's a
> split with servers using ACPI and more embedded platforms using DT.
> 
> > > I have no idea what the hardware this series targets is (other than that
> > > it's using a FPGA) or if there's even a motivation for the change other
> > > than code inspection.
> 
> > I think all these cases are very similar. You have x86 with pcie root port
> > which is connected directly (or via pcie slot) to fpga. In fpga you have
> > pcie endpoint HW which connects other IPs sitting on AXI.
> 
> What are "all these cases"?  For something connected via PCI I would
> expect a PCI driver that knows via some mechanism what's connected to it
> and then instantiates the IPs, probably as aux devices.  I would not
> expect to see the contents of the PCI device described in firmware at
> all, that's a big goal with using PCI.  If something is not connected
> via PCI that's obviously not going to fly but it sounds like you're only
> interested in PCI cases here.



-- 
With Best Regards,
Andy Shevchenko

Powered by blists - more mailing lists