Message-ID: <aW_Y0h7RNsN12H7H@black.igk.intel.com>
Date: Tue, 20 Jan 2026 20:34:42 +0100
From: Andy Shevchenko <andriy.shevchenko@...el.com>
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.

+Cc: Rafael

On Tue, Jan 20, 2026 at 10:23:22AM +0100, Andy Shevchenko wrote:
> 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