mbox series

[0/2] rewrite mtk-quadspi spi-nor driver with spi-mem

Message ID 20200215065826.739102-1-gch981213@gmail.com (mailing list archive)
Headers show
Series rewrite mtk-quadspi spi-nor driver with spi-mem | expand

Message

Chuanhong Guo Feb. 15, 2020, 6:58 a.m. UTC
This patchset adds a spi-mem driver for Mediatek SPI-NOR controller,
which already has limited support by mtk-quadspi. For benefits of
this replacement, please check commit message of the 1st patch.

To keep patchset small for easier reviewing, there will be 3 patchsets
including this one.
1. add the new driver, which is this patchset.
2. update existing dts for the new driver:
   spi-max-frequency is missing in current mtk-quadspi binding. Old
   driver parses child node manually so it doesn't need this, but
   new spi-mem driver is probed via spi subsystem which requires the
   presence of spi-max-frequency. Since this doesn't break old driver
   support, I'll send this separately as a standalone patch.
3. removing the old driver. I'll create this commit after 1 and 2 are
   applied to avoid possible rebasing due to any changes in the old
   driver.

Chuanhong Guo (2):
  spi: add support for mediatek spi-nor controller
  dt-bindings: convert mtk-quadspi binding doc for spi-mtk-nor

 .../mtk-quadspi.txt => spi/spi-mtk-nor.txt}   |  34 +-
 drivers/spi/Kconfig                           |  10 +
 drivers/spi/Makefile                          |   1 +
 drivers/spi/spi-mtk-nor.c                     | 689 ++++++++++++++++++
 4 files changed, 715 insertions(+), 19 deletions(-)
 rename Documentation/devicetree/bindings/{mtd/mtk-quadspi.txt => spi/spi-mtk-nor.txt} (62%)
 create mode 100644 drivers/spi/spi-mtk-nor.c

Comments

Mark Brown Feb. 18, 2020, 12:55 p.m. UTC | #1
On Sat, Feb 15, 2020 at 02:58:24PM +0800, Chuanhong Guo wrote:

> To keep patchset small for easier reviewing, there will be 3 patchsets
> including this one.
> 1. add the new driver, which is this patchset.
> 2. update existing dts for the new driver:
>    spi-max-frequency is missing in current mtk-quadspi binding. Old
>    driver parses child node manually so it doesn't need this, but
>    new spi-mem driver is probed via spi subsystem which requires the
>    presence of spi-max-frequency. Since this doesn't break old driver
>    support, I'll send this separately as a standalone patch.

This is an ABI break so you shouldn't be doing this, if the existing
binding works it should continue to work.

> 3. removing the old driver. I'll create this commit after 1 and 2 are
>    applied to avoid possible rebasing due to any changes in the old
>    driver.

This isn't great as it means we have a period with two drivers for the
same thing in tree which is at best going to be confusing.  There's no
advantage to splitting this out.
Chuanhong Guo Feb. 19, 2020, 11:58 p.m. UTC | #2
Hi!

On Tue, Feb 18, 2020 at 8:55 PM Mark Brown <broonie@kernel.org> wrote:
> This is an ABI break so you shouldn't be doing this, if the existing
> binding works it should continue to work.

The missing spi-max-frequency is the only part preventing old
device tree to work with this driver.
If the goal is to make existing dt binding work, I could patch dt using
of_add_property in v2. I saw similar device tree patching for legacy
bindings in pinctrl-single driver.

>
> > 3. removing the old driver. I'll create this commit after 1 and 2 are
> >    applied to avoid possible rebasing due to any changes in the old
> >    driver.
>
> This isn't great as it means we have a period with two drivers for the
> same thing in tree which is at best going to be confusing.  There's no
> advantage to splitting this out.

Got it. I'll add this patch in v2.

--
Regards,
Chuanhong Guo
Mark Brown Feb. 20, 2020, 7:51 p.m. UTC | #3
On Thu, Feb 20, 2020 at 07:58:06AM +0800, Chuanhong Guo wrote:
> On Tue, Feb 18, 2020 at 8:55 PM Mark Brown <broonie@kernel.org> wrote:

> > This is an ABI break so you shouldn't be doing this, if the existing
> > binding works it should continue to work.

> The missing spi-max-frequency is the only part preventing old
> device tree to work with this driver.
> If the goal is to make existing dt binding work, I could patch dt using
> of_add_property in v2. I saw similar device tree patching for legacy
> bindings in pinctrl-single driver.

That's fine I think, so long as old DTs continue to work.
Rob Herring Feb. 25, 2020, 5:31 p.m. UTC | #4
On Thu, Feb 20, 2020 at 07:58:06AM +0800, Chuanhong Guo wrote:
> Hi!
> 
> On Tue, Feb 18, 2020 at 8:55 PM Mark Brown <broonie@kernel.org> wrote:
> > This is an ABI break so you shouldn't be doing this, if the existing
> > binding works it should continue to work.
> 
> The missing spi-max-frequency is the only part preventing old
> device tree to work with this driver.
> If the goal is to make existing dt binding work, I could patch dt using
> of_add_property in v2. I saw similar device tree patching for legacy
> bindings in pinctrl-single driver.

You should should really only need 'spi-max-frequency' if the max freq 
is less than the minimum of the host and device max freq.

Rob
Chuanhong Guo Feb. 26, 2020, 1:31 a.m. UTC | #5
Hi!

On Wed, Feb 26, 2020 at 1:31 AM Rob Herring <robh@kernel.org> wrote:
>
> On Thu, Feb 20, 2020 at 07:58:06AM +0800, Chuanhong Guo wrote:
> > Hi!
> >
> > On Tue, Feb 18, 2020 at 8:55 PM Mark Brown <broonie@kernel.org> wrote:
> > > This is an ABI break so you shouldn't be doing this, if the existing
> > > binding works it should continue to work.
> >
> > The missing spi-max-frequency is the only part preventing old
> > device tree to work with this driver.
> > If the goal is to make existing dt binding work, I could patch dt using
> > of_add_property in v2. I saw similar device tree patching for legacy
> > bindings in pinctrl-single driver.

I just noticed that of_add_property isn't a exported symbol, which means that
device tree patching isn't possible unless driver is builtin.

>
> You should should really only need 'spi-max-frequency' if the max freq
> is less than the minimum of the host and device max freq.

But current spi framework forces that a "spi-max-frequency" property
is present. [0]
Should we patch spi framework then?

[0] https://elixir.bootlin.com/linux/latest/source/drivers/spi/spi.c#L1951
Mark Brown Feb. 26, 2020, 11:36 a.m. UTC | #6
On Wed, Feb 26, 2020 at 09:31:33AM +0800, Chuanhong Guo wrote:
> On Wed, Feb 26, 2020 at 1:31 AM Rob Herring <robh@kernel.org> wrote:

> > You should should really only need 'spi-max-frequency' if the max freq
> > is less than the minimum of the host and device max freq.

> But current spi framework forces that a "spi-max-frequency" property
> is present. [0]
> Should we patch spi framework then?

That's one option, yes.  As far as I can tell the bindings have always
required an explicit frequency specified in the bindings but I've no
idea why.