Re: [PATCH 5/5] spi: cadence-xspi: Add workaround for old Marvell firmwares

[Jisheng Zhang]

On Tue, Nov 10, 2026 at 10:58:06AM +0100, Paul Cercueil wrote:
> Hi Jisheng,
> 
> Le mardi 10 novembre 2026 à 13:07 +0800, Jisheng Zhang a écrit :
> > On Mon, Nov 09, 2026 at 05:34:55PM +0100, Paul Cercueil wrote:
> > > Hi Mark,
> > > 
> > > Le samedi 07 novembre 2026 à 21:21 +0800, Jisheng Zhang a écrit :
> > > > On Wed, Nov 04, 2026 at 11:06:43AM +0100, Paul Cercueil wrote:
> > > > > From: Tanmay Jagdale <tanmay@xxxxxxxxxxx>
> > > > > 
> > > > > Older firmwares from Marvell used the cdns,xspi-nor compatible string
> > > > > for the Marvell-specific xSPI IP, which is incompatible.
> > > > > 
> > > > > Add a workaround to support this by assuming that if the "xfer" resource
> > > > > is found, it means that a Marvell IP is used.
> > > > > S> 
> > > > > Signed-off-by: Tanmay Jagdale <tanmay@xxxxxxxxxxx>
> > > > > Co-developed-by: Paul Cercueil <paul.cercueil@xxxxxxxxxxx>
> > > > > Signed-off-by: Paul Cercueil <paul.cercueil@xxxxxxxxxxx>
> > > > > ---
> > > > >  drivers/spi/spi-cadence-xspi.c | 18 ++++++++++++++----
> > > > >  1 file changed, 14 insertions(+), 4 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/spi/spi-cadence-xspi.c b/drivers/spi/spi-cadence-
> > > > > xspi.c
> > > > > index 48a109f3fcdb..d326589a6ca3 100644
> > > > > --- a/drivers/spi/spi-cadence-xspi.c
> > > > > +++ b/drivers/spi/spi-cadence-xspi.c
> > > > > @@ -1301,11 +1301,21 @@ static int cdns_xspi_probe(struct
> > > > > platform_device
> > > > > *pdev)
> > > > >  	}
> > > > >  
> > > > >  #ifdef CONFIG_64BIT
> > > > > -	if (cdns_xspi->driver_data->mrvl_hw_overlay) {
> > > > > -		res = platform_get_resource_byname(pdev,
> > > > > IORESOURCE_MEM,
> > > > > "xfer");
> > > > > -		if (!res)
> > > > > -			res = platform_get_resource(pdev,
> > > > > IORESOURCE_MEM,
> > > > > 3);
> > > > > +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> > > > > "xfer");
> > > > > +	if (!res)
> > > > > +		res = platform_get_resource(pdev, IORESOURCE_MEM, 3);
> > > > 
> > > > This moves the mrvl specific code to all users, so I believe this
> > > > needs dt-binding update.
> > > 
> > > I actually disagree here. The binding says that "xfer" is only provided with
> > 
> > It's not only "xfer". The code falls back to 4th MEM resoure when "xfer"
> > failed.
> > 
> > > Marvell-specific compatible strings, and that's correct. The fact that some
> > > firmwares don't respect this spec is not a problem of the spec itself.
> > > 
> > > > From another side, this assumes other users doesn't have 4th resource
> > > > but what about if they have? they will get "Marvell-specific xfer
> > > > resource passed, but ..." warning below, this confuse the users.
> > > > I don't think this workaround can be added to all users.
> > > 
> > > Yes, I thought about that and I can understand that you don't like it...
> > > 
> > > I originally thought that it was fine because the dt-binding states that the
> > > 4th
> > > resource is only for Marvell compatibles; but the binding can change, so
> > > it's a
> > > bit wobbly.
> > > 
> > > I will come up with a better mechanism in the v2.
> > > 
> > > > Lastly, does the "firmware" mean dtb? But per my grep, I didn't
> > > > find any mrvl dts(i) uses "cdns,xspi-nor" compatible string, so
> > > > this means the dtb is outside upstream, then can the firmware
> > > > be fixed instead?
> > > 
> > > It is dtb and also ACPI. The firmware can and will be fixed, the idea here
> > > is to
> > > make sure that the driver still works on older firmwares that haven't been
> > > updated yet, because they are out there.
> > 
> > If firmware is dtb, the dtb is out of tree, why mainline driver needs to
> > workaroud its bug?
> > If firmwre is acpi, the commit msg says "used the cdns,xspi-nor
> > compatible string", I'm not sure how does ACPI use dt compatbile string? 
> 
> It's both dtb and ACPI. The firmware's ACPI table does use the PRP0001 Device
> Tree namespace HID with the "cdns,xspi-nor" string.
> 
> I don't understand your point about the dtb being out-of-tree. It does not
> change anything.
> 

My point is: since the dtb is from out-of-tree, mainline drivers
should not workaround any bug in it. If this is allowed, then
the mainline driver will full of such workarounds here and there.