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: <CAGLn_=tieSCGWix-0mGC7n8MnD46WPxuWh9xhtB6r+YZry463g@mail.gmail.com>
Date: Wed, 3 Jul 2024 18:42:46 +0530
From: Kanak Shilledar <kanakshilledar@...il.com>
To: Conor Dooley <conor.dooley@...rochip.com>
Cc: Serge Semin <fancer.lancer@...il.com>, Samuel Holland <samuel.holland@...ive.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

Hi,
So, I will drop this patch.
In the next version (i.e. v2) of this patchset, do I need to include
the dt-binding patch as it is already in for-next.
I am waiting for comments on the devicetree files before sending the
v2 (if required).

Thanks and Regards,
Kanak Shilledar

On Wed, Jul 3, 2024 at 12:59 PM Conor Dooley <conor.dooley@...rochip.com> wrote:
>
> 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.
>