linux-kernel - Re: [PATCH v2 2/3] spi: dw-mmio: update dw_spi_mmio_of

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240703-garbage-explicit-bd95f8deb716@wendy>
Date: Wed, 3 Jul 2024 08:28:36 +0100
From: Conor Dooley <conor.dooley@...rochip.com>
To: Serge Semin <fancer.lancer@...il.com>
CC: Samuel Holland <samuel.holland@...ive.com>, Kanak Shilledar
	<kanakshilledar@...il.com>, Conor Dooley <conor+dt@...nel.org>, Rob Herring
	<robh+dt@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Mark Brown
	<broonie@...nel.org>, Rob Herring <robh@...nel.org>, Jisheng Zhang
	<jszhang@...nel.org>, Guo Ren <guoren@...nel.org>, Fu Wei <wefu@...hat.com>,
	Paul Walmsley <paul.walmsley@...ive.com>, Palmer Dabbelt
	<palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>,
	<linux-spi@...r.kernel.org>, <devicetree@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <linux-riscv@...ts.infradead.org>
Subject: Re: [PATCH v2 2/3] spi: dw-mmio: update dw_spi_mmio_of_match struct
 with thead

On Mon, Jul 01, 2024 at 09:57:20PM +0300, Serge Semin wrote:
> Hi folks
> 
> On Mon, Jul 01, 2024 at 08:17:29AM -0500, Samuel Holland wrote:
> > Hi Kanak,
> > 
> > On 2024-07-01 7:13 AM, Kanak Shilledar wrote:
> > > updated the struct of_device_id dw_spi_mmio_of_match to include
> > > the updated compatible value for TH1520 SoC ("thead,th1520-spi")
> > > to initialize with dw_spi_pssi_init().
> > > 
> > > Signed-off-by: Kanak Shilledar <kanakshilledar@...il.com>
> > > ---
> > > Changes in v2:
> > > - Separated from a single patch file.
> > > ---
> > >  drivers/spi/spi-dw-mmio.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c
> > > index 819907e332c4..39e3d46ebf5d 100644
> > > --- a/drivers/spi/spi-dw-mmio.c
> > > +++ b/drivers/spi/spi-dw-mmio.c
> > > @@ -419,6 +419,7 @@ static const struct of_device_id dw_spi_mmio_of_match[] = {
> > >  	{ .compatible = "microchip,sparx5-spi", dw_spi_mscc_sparx5_init},
> > >  	{ .compatible = "canaan,k210-spi", dw_spi_canaan_k210_init},
> > >  	{ .compatible = "amd,pensando-elba-spi", .data = dw_spi_elba_init},
> > > +	{ .compatible = "thead,th1520-spi", .data = dw_spi_pssi_init},
> > 
> > Your binding requires snps,dw-apb-ssi as a fallback compatible string, which is
> > already supported by this driver and uses the same match data. So you don't need
> > this patch; its only effect is to make the kernel larger.
> 
> Agree with Samuel comment. Indeed there is no point in adding the
> vendor-specific device-name supported in the driver if the fallback
> compatible works as-is.

FWIW, Mark picked up the binding alone so I think there's nothing for
Kanak to do here & the driver patch should just be forgotten about :)

> >From that perspective we shouldn't have merged in the patch adding the
> Renesas RZN1 SPI device name support, since the generic fallback
> compatible works for it. On the contrary the Microsemi Ocelot/Jaguar2
> SoC SPI DT-bindings shouldn't have been defined with the generic
> fallback compatible since should the device be bound via the generic
> name it won't work as expected.
> 
> Although, it's better to hear out what Rob, Conor or Krzysztof think
> about this.

I agree with what you've written. If the fallback works identically, then
the specific compatible shouldn't be added here. And if the fallback
will cause the device to misbehave (or not behave at all), then it
should not have been added.
I'm not sure if the Microsemi stuff is in the "won't work {,properly}"
camp or in the "will work in a limited fashion" camp. The latter would
be suitable for a fallback, the former not.

Cheers,
Conor.


Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)