Message-Id: <BF71A04E-7FFB-42D1-8C8D-6FD13415EED5@nexthop.ai>
Date: Mon, 19 Jan 2026 16:20:06 -0800
From: Abdurrahman Hussain <abdurrahman@...thop.ai>
To: Mark Brown <broonie@...nel.org>
Cc: Michal Simek <michal.simek@....com>,
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>,
 Andrew Lunn <andrew@...n.ch>,
 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 Jan 19, 2026, at 3:20 PM, Mark Brown <broonie@...nel.org> wrote:
> 
> On Mon, Jan 19, 2026 at 12:30:43PM -0800, Abdurrahman Hussain wrote:
> 
> To repeat what I previously said:
> 
> | Please fix your mail client to word wrap within paragraphs at something
> | substantially less than 80 columns.  Doing this makes your messages much
> | easier to read and reply to.
> 
>>> On Jan 19, 2026, at 11:56 AM, Mark Brown <broonie@...nel.org> 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:
> 
>>> 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.
> 
>> These "random" cleanups make the spi-xilinx.c driver work on non-DT platforms.
>> Which is what the cover letter says.
> 
> The cover letter just says:
> 
> | Transition the driver to use the generic device property API.
> 
> | Additionally, make interrupts optional to allow the driver to fall back
> | to its existing polling mode on systems where interrupts are either missing
> | or broken.
> 
> which doesn't mention any motivation for this, it's just a statement of
> what the patches do.  It's very unclear why this is a series TBH, the
> interrupts changes have no visible relationship to the device properties
> conversion and should have been submitted separately.

V1 was a single patch. And the commit body there was:

"This makes the driver work on non-OF platforms.”

See https://lkml.org/lkml/2026/1/15/66

It was suggested to reword the body in imperative mood. Maybe that’s where it got lost.
I can re-add the mentioning of non-OF platforms to the cover letter to make it clear.

Andrew Lunn also suggested to split it into 3.

> 
>> We are trying NOT to introduce new ACPI IDS, or fork the existing drivers.
>> But rather, to re-use the existing drivers as much as possible by relying on the
>> special DT namespace link "PRP0001":
> 
> What is the goal in avoiding using native ACPI bindings?  PRP0001 is a
> workaround for cases where you have things that ACPI has never dreamed
> of and can't abstract well but which have already been handled by DT,
> it is not something we're aiming.  The hardware you have described
> seems like fairly normal server style hardware and like it should fit
> well with normal ACPI.  Adding ACPI IDs to existing drivers is pretty
> common and standard, I'd class that as reuse.

I did not intentionally avoid introducing new ACPI _HIDs, but at the same time
don’t understand the need to, especially when PRP0001 workaround works and
it works well. We don’t own the spi-xilinx driver and neither do we own the i2c-xiic.
These are standard Xilinx IP platform drivers that could be made usable on ACPI platforms
by just switching to device_property APIs. Which is what these series is trying to do.


> 
> As far as I can see from the example you posted the devices are all
> fairly standard and just need IDs assigning to work naturally with ACPI.
> There's possibly an argument for using PRP0001 for the flash given that
> it's not especially idiomatic to have OS visible flash on ACPI systems,
> usually flash would only be visible to UEFI, but equally the description
> would trivial and systems wouldn't have to use it.

You mention “idiomatic” a lot. Are you implying the SPI NOR-flash devices should
not be exposed to user-space? Then why does the spi-nor driver even exist?

We have a SPI device that needs to be accessible from users-space in order to
“flash" various FPGAs. How do you suggest we do that?

Also, there are hundreds of drivers in the DT namespace that are missing the ACPI IDs today.
Do you suggest adding ACPI IDs to every driver just so it’s more “idiomatic” to use?
I am just trying to get this 2-line small change merged so we can start using the standard spi-xilinx driver today. I am not trying to boil the ocean.

> 
>>> 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.
> 
>> Which is exactly what we are doing - using ACPI on x86 to describe the hardware.
> 
> There is the bit where there's a PCI bus in the way and you don't use
> ACPI particularly idiomatically...  I can see the dodging out on the PCI
> bus description, but the way the devices behind the PCI bus are
> described seems confusing.

What exactly about this usage is not idiomatic? Our PCI device description in ACPI looks like
this (GPP5 is the PCIe bridge under which the FPGA is located):

    Scope (_SB.PCI0.GPP5)
    {
        Device (FPGA)
        {
            Name (_ADR, 0x00000000)
…
            Device (I2C0) {
            ...
            }

            Device (I2C1) {
            ...
            }

            Device (SPI0) {
            ...
            }

            Device (SPI0) {
            ...
            }

        }
     }

I have provided ASL fragments for I2C and SPI earlier.

Which part of it is not idiomatic?

Powered by blists - more mailing lists