Re: [PATCH v9 0/2] spi: cadence-quadpsi: Add support for the Cadence QSPI controller

From: Mark Brown
Date: Fri Feb 14 2020 - 09:16:50 EST


On Fri, Feb 14, 2020 at 02:49:48PM +0100, Simon Goldschmidt wrote:
> On Fri, Feb 14, 2020 at 2:15 PM Mark Brown <broonie@xxxxxxxxxx> wrote:

> > I'm not seeing anything in the driver that removes whatever the current
> > support is? Unless it's just adding a duplicate driver for the same
> > compatible strings which is obviously a bad idea but at least means that
> > unless people enable the driver there's no risk of it colliding with the
> > existing one.

> It does add a duplicate driver for the same compatible strings. The current
> working driver is in 'drivers/mtd/spi-nor/cadence-quadspi.c'.

> In fact, the compatible string "cdns,qspi-nor" copied from the old driver to
> this new driver is *only* used for altera. TI has its own compatible string,
> the new Intel platform adds its own as well.

Oh, that's not good - it's adding a completely new binding for the same
compatibles which isn't OK. We can transition to a new driver using the
same binding but we should be keeping the old binding. If we're moving
the binding document around and/or transitioning to YAML that needs to
be done explicitly rather than adding a new document for the same
compatible.

> As long as that one doesn't get removed, I have nothing against this driver
> here. I'm only concerned that this will get forgotten. And given that I added
> altera guys to the loop in one of the previous versions, I just was surprised
> they aren't on CC in this version.

Yes, like I say it'd be much better to get their review.

Attachment: signature.asc
Description: PGP signature