[PATCH v2 0/4] spi: dw: Add basic runtime PM support

[PATCH v2 0/4] spi: dw: Add basic runtime PM support

[Gareth Williams]

The Renesas RZ/N1 SPI Controller is based on the Synopsys DW SSI. This
series enables power mode in the driver so the clock domain will enable
the bus clock, adds the compatible string and updates the associated bindings
documentation.

v2:
 - Note that pclk should be renamed when using a clock domain in the
   bindings documentation.
 - Set spi_controller.auto_runtime_pm instead of using
   pm_runtime_get_sync.
 - Added pm_runtime_disable calls to dw_spi_remove_host and the error
   condition of dw_spi_add_host.

Gareth Williams (1):
  dt-bindings: snps,dw-apb-ssi: Add optional clock domain information

Phil Edworthy (3):
  dt: spi: Add Renesas RZ/N1 binding documentation
  spi: dw: Add basic runtime PM support
  spi: dw: Add compatible string for Renesas RZ/N1 SPI Controller

 Documentation/devicetree/bindings/spi/renesas,rzn1-spi.txt | 11 +++++++++++
 Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt  |  3 ++-
 drivers/spi/spi-dw-mmio.c                                  |  1 +
 drivers/spi/spi-dw.c                                       |  8 ++++++++
 4 files changed, 22 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/spi/renesas,rzn1-spi.txt

-- 
2.7.4

[PATCH v2 1/4] dt: spi: Add Renesas RZ/N1 binding documentation

[Gareth Williams]

From: Phil Edworthy <phil.edworthy@renesas.com>

The Renesas RZ/N1 SPI Controller is based on the Synopsys DW SSI, but has
additional registers for software CS control and DMA. This patch does not
address the changes required for DMA support, it simply adds the compatible
string. The CS functionality is not very useful and also not needed as
Linux can use gpios for the CS signals.

Add a compatible string to handle any unforeseen issues that may arise, and
pave the way for DMA support.

Signed-off-by: Gareth Williams <gareth.williams.jx@renesas.com>
Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
---
Note: All the other manufacturers detail their compatible strings in
snps,dw-apb-ssi.txt. I think it makes sense for rzn1 to be in it's own file
due to the changes made to the peripheral for DMA support.

v2:
 - No changes.
---
 Documentation/devicetree/bindings/spi/renesas,rzn1-spi.txt | 11 +++++++++++
 1 file changed, 11 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spi/renesas,rzn1-spi.txt

diff --git a/Documentation/devicetree/bindings/spi/renesas,rzn1-spi.txt b/Documentation/devicetree/bindings/spi/renesas,rzn1-spi.txt
new file mode 100644
index 0000000..fb1a672
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/renesas,rzn1-spi.txt
@@ -0,0 +1,11 @@
+Renesas RZ/N1 SPI Controller
+
+This controller is based on the Synopsys DW Synchronous Serial Interface and
+inherits all properties defined in snps,dw-apb-ssi.txt except for the
+compatible property.
+
+Required properties:
+- compatible : The device specific string followed by the generic RZ/N1 string.
+   Therefore it must be one of:
+   "renesas,r9a06g032-spi", "renesas,rzn1-spi"
+   "renesas,r9a06g033-spi", "renesas,rzn1-spi"
-- 
2.7.4

Applied "dt: spi: Add Renesas RZ/N1 binding documentation" to the spi tree

[Mark Brown]

The patch

   dt: spi: Add Renesas RZ/N1 binding documentation

has been applied to the spi tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-5.5

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From da182a61fce01dbd7c4a78c68a34af110f00e36f Mon Sep 17 00:00:00 2001
From: Phil Edworthy <phil.edworthy@renesas.com>
Date: Wed, 18 Sep 2019 09:04:33 +0100
Subject: [PATCH] dt: spi: Add Renesas RZ/N1 binding documentation

The Renesas RZ/N1 SPI Controller is based on the Synopsys DW SSI, but has
additional registers for software CS control and DMA. This patch does not
address the changes required for DMA support, it simply adds the compatible
string. The CS functionality is not very useful and also not needed as
Linux can use gpios for the CS signals.

Add a compatible string to handle any unforeseen issues that may arise, and
pave the way for DMA support.

Signed-off-by: Gareth Williams <gareth.williams.jx@renesas.com>
Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
Link: https://lore.kernel.org/r/1568793876-9009-2-git-send-email-gareth.williams.jx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 .../devicetree/bindings/spi/renesas,rzn1-spi.txt      | 11 +++++++++++
 1 file changed, 11 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spi/renesas,rzn1-spi.txt

diff --git a/Documentation/devicetree/bindings/spi/renesas,rzn1-spi.txt b/Documentation/devicetree/bindings/spi/renesas,rzn1-spi.txt
new file mode 100644
index 000000000000..fb1a6728638d
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/renesas,rzn1-spi.txt
@@ -0,0 +1,11 @@
+Renesas RZ/N1 SPI Controller
+
+This controller is based on the Synopsys DW Synchronous Serial Interface and
+inherits all properties defined in snps,dw-apb-ssi.txt except for the
+compatible property.
+
+Required properties:
+- compatible : The device specific string followed by the generic RZ/N1 string.
+   Therefore it must be one of:
+   "renesas,r9a06g032-spi", "renesas,rzn1-spi"
+   "renesas,r9a06g033-spi", "renesas,rzn1-spi"
-- 
2.20.1

Re: [PATCH v2 1/4] dt: spi: Add Renesas RZ/N1 binding documentation

[Rob Herring]

On Wed, 18 Sep 2019 09:04:33 +0100, Gareth Williams wrote:
> From: Phil Edworthy <phil.edworthy@renesas.com>
> 
> The Renesas RZ/N1 SPI Controller is based on the Synopsys DW SSI, but has
> additional registers for software CS control and DMA. This patch does not
> address the changes required for DMA support, it simply adds the compatible
> string. The CS functionality is not very useful and also not needed as
> Linux can use gpios for the CS signals.
> 
> Add a compatible string to handle any unforeseen issues that may arise, and
> pave the way for DMA support.
> 
> Signed-off-by: Gareth Williams <gareth.williams.jx@renesas.com>
> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> ---
> Note: All the other manufacturers detail their compatible strings in
> snps,dw-apb-ssi.txt. I think it makes sense for rzn1 to be in it's own file
> due to the changes made to the peripheral for DMA support.
> 
> v2:
>  - No changes.
> ---
>  Documentation/devicetree/bindings/spi/renesas,rzn1-spi.txt | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/spi/renesas,rzn1-spi.txt
> 

Reviewed-by: Rob Herring <robh@kernel.org>

[PATCH v2 2/4] dt-bindings: snps,dw-apb-ssi: Add optional clock domain information

[Gareth Williams]

Note in the bindings documentation that pclk should be renamed if a clock
domain is used to enable the optional bus clock.

Signed-off-by: Gareth Williams <gareth.williams.jx@renesas.com>
---
v2: Introduced this patch.
---
 Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
index f54c8c3..3ed08ee 100644
--- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
+++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
@@ -16,7 +16,8 @@ Required properties:
 Optional properties:
 - clock-names : Contains the names of the clocks:
     "ssi_clk", for the core clock used to generate the external SPI clock.
-    "pclk", the interface clock, required for register access.
+    "pclk", the interface clock, required for register access. If a clock domain
+     used to enable this clock then it should be named "pclk_clkdomain".
 - cs-gpios : Specifies the gpio pins to be used for chipselects.
 - num-cs : The number of chipselects. If omitted, this will default to 4.
 - reg-io-width : The I/O register width (in bytes) implemented by this
-- 
2.7.4

Applied "dt-bindings: snps,dw-apb-ssi: Add optional clock domain information" to the spi tree

[Mark Brown]

The patch

   dt-bindings: snps,dw-apb-ssi: Add optional clock domain information

has been applied to the spi tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-5.5

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 47cf13bc763c891c6192184c5e5aa8c1b331b2ff Mon Sep 17 00:00:00 2001
From: Gareth Williams <gareth.williams.jx@renesas.com>
Date: Wed, 18 Sep 2019 09:04:34 +0100
Subject: [PATCH] dt-bindings: snps,dw-apb-ssi: Add optional clock domain
 information

Note in the bindings documentation that pclk should be renamed if a clock
domain is used to enable the optional bus clock.

Signed-off-by: Gareth Williams <gareth.williams.jx@renesas.com>
Link: https://lore.kernel.org/r/1568793876-9009-3-git-send-email-gareth.williams.jx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
index f54c8c36395e..3ed08ee9feba 100644
--- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
+++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
@@ -16,7 +16,8 @@ Required properties:
 Optional properties:
 - clock-names : Contains the names of the clocks:
     "ssi_clk", for the core clock used to generate the external SPI clock.
-    "pclk", the interface clock, required for register access.
+    "pclk", the interface clock, required for register access. If a clock domain
+     used to enable this clock then it should be named "pclk_clkdomain".
 - cs-gpios : Specifies the gpio pins to be used for chipselects.
 - num-cs : The number of chipselects. If omitted, this will default to 4.
 - reg-io-width : The I/O register width (in bytes) implemented by this
-- 
2.20.1

Re: [PATCH v2 2/4] dt-bindings: snps,dw-apb-ssi: Add optional clock domain information

[Rob Herring]

On Wed, Sep 18, 2019 at 09:04:34AM +0100, Gareth Williams wrote:
> Note in the bindings documentation that pclk should be renamed if a clock
> domain is used to enable the optional bus clock.
> 
> Signed-off-by: Gareth Williams <gareth.williams.jx@renesas.com>
> ---
> v2: Introduced this patch.
> ---
>  Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
> index f54c8c3..3ed08ee 100644
> --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
> +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
> @@ -16,7 +16,8 @@ Required properties:
>  Optional properties:
>  - clock-names : Contains the names of the clocks:
>      "ssi_clk", for the core clock used to generate the external SPI clock.
> -    "pclk", the interface clock, required for register access.
> +    "pclk", the interface clock, required for register access. If a clock domain
> +     used to enable this clock then it should be named "pclk_clkdomain".

What's a clock domain?

Unless this is a h/w difference in the IP block, then this change 
doesn't make sense.

>  - cs-gpios : Specifies the gpio pins to be used for chipselects.
>  - num-cs : The number of chipselects. If omitted, this will default to 4.
>  - reg-io-width : The I/O register width (in bytes) implemented by this
> -- 
> 2.7.4
> 

RE: [PATCH v2 2/4] dt-bindings: snps,dw-apb-ssi: Add optional clock domain information

[Gareth Williams]

Hi Rob,

On Tue, Oct 01, 2019 at 13:02:34AM +0100, Rob Herring wrote:
> On Wed, Sep 18, 2019 at 09:04:34AM +0100, Gareth Williams wrote:
> > Note in the bindings documentation that pclk should be renamed if a
> > clock domain is used to enable the optional bus clock.
> >
> > Signed-off-by: Gareth Williams <gareth.williams.jx@renesas.com>
> > ---
> > v2: Introduced this patch.
> > ---
> >  Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
> > b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
> > index f54c8c3..3ed08ee 100644
> > --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
> > +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
> > @@ -16,7 +16,8 @@ Required properties:
> >  Optional properties:
> >  - clock-names : Contains the names of the clocks:
> >      "ssi_clk", for the core clock used to generate the external SPI clock.
> > -    "pclk", the interface clock, required for register access.
> > +    "pclk", the interface clock, required for register access. If a clock domain
> > +     used to enable this clock then it should be named "pclk_clkdomain".
> 
> What's a clock domain?
> 
> Unless this is a h/w difference in the IP block, then this change doesn't make
> sense.
This is a reference to the use of clock domains that are implemented through
generic power domains. The domain is implemented in 
drivers/clk/renesas/r9a06g032-clocks.c and general details of clock domains
can be found at 
https://elinux.org/images/1/14/Last_One_Out%2C_Turn_Off_The_Lights.pdf

> 
> >  - cs-gpios : Specifies the gpio pins to be used for chipselects.
> >  - num-cs : The number of chipselects. If omitted, this will default to 4.
> >  - reg-io-width : The I/O register width (in bytes) implemented by
> > this
> > --
> > 2.7.4
> >

Re: [PATCH v2 2/4] dt-bindings: snps,dw-apb-ssi: Add optional clock domain information

[Geert Uytterhoeven]

Hi Gareth,

On Tue, Oct 1, 2019 at 3:50 PM Gareth Williams
<gareth.williams.jx@renesas.com> wrote:
> On Tue, Oct 01, 2019 at 13:02:34AM +0100, Rob Herring wrote:
> > On Wed, Sep 18, 2019 at 09:04:34AM +0100, Gareth Williams wrote:
> > > Note in the bindings documentation that pclk should be renamed if a
> > > clock domain is used to enable the optional bus clock.
> > >
> > > Signed-off-by: Gareth Williams <gareth.williams.jx@renesas.com>
> > > ---
> > > v2: Introduced this patch.
> > > ---
> > >  Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
> > > b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
> > > index f54c8c3..3ed08ee 100644
> > > --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
> > > +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
> > > @@ -16,7 +16,8 @@ Required properties:
> > >  Optional properties:
> > >  - clock-names : Contains the names of the clocks:
> > >      "ssi_clk", for the core clock used to generate the external SPI clock.
> > > -    "pclk", the interface clock, required for register access.
> > > +    "pclk", the interface clock, required for register access. If a clock domain
> > > +     used to enable this clock then it should be named "pclk_clkdomain".
> >
> > What's a clock domain?
> >
> > Unless this is a h/w difference in the IP block, then this change doesn't make
> > sense.
> This is a reference to the use of clock domains that are implemented through
> generic power domains. The domain is implemented in
> drivers/clk/renesas/r9a06g032-clocks.c and general details of clock domains
> can be found at
> https://elinux.org/images/1/14/Last_One_Out%2C_Turn_Off_The_Lights.pdf

Rob is right: the clock domain is an SoC integration detail, not specific to
the snps,dw-apb-ssi block.
Remember, DT describes hardware, not implementation details.

So the Linux snps,dw-apb-ssi driver should take care of it.

Which brings us back to an old discussion topic: power-domains properties
describe integration, and thus should be documented at a higher level than
in individual binding documents, just like e.g. interrupt-parent.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v2 0/4] spi: dw: Add basic runtime PM support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 425 bytes --]

On Wed, Sep 18, 2019 at 09:04:32AM +0100, Gareth Williams wrote:

> Gareth Williams (1):
>   dt-bindings: snps,dw-apb-ssi: Add optional clock domain information
> 
> Phil Edworthy (3):
>   dt: spi: Add Renesas RZ/N1 binding documentation

Please use subject lines matching the style for the subsystem.  This
makes it easier for people to identify relevant patches.  This isn't
even consistent within the series :(

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

RE: [PATCH v2 0/4] spi: dw: Add basic runtime PM support

[Gareth Williams]

Hi Mark,

On Wed, Sep 19, 2019 at 14:31:32AM +0100, Mark Brown wrote:

> On Wed, Sep 18, 2019 at 09:04:32AM +0100, Gareth Williams wrote:
> 
> > Gareth Williams (1):
> >   dt-bindings: snps,dw-apb-ssi: Add optional clock domain information
> >
> > Phil Edworthy (3):
> >   dt: spi: Add Renesas RZ/N1 binding documentation
> 
> Please use subject lines matching the style for the subsystem.  This makes it
> easier for people to identify relevant patches.  This isn't even consistent
> within the series :(
Sorry about that, I will correct the subject lines for V3.
Is there a set convention for the subsystem I should follow in future?
Or should I follow the style of the individual files I work on?

Kind Regards,

Gareth

Re: [PATCH v2 0/4] spi: dw: Add basic runtime PM support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 803 bytes --]

On Thu, Sep 19, 2019 at 03:14:54PM +0000, Gareth Williams wrote:
> On Wed, Sep 19, 2019 at 14:31:32AM +0100, Mark Brown wrote:

> > Please use subject lines matching the style for the subsystem.  This makes it
> > easier for people to identify relevant patches.  This isn't even consistent
> > within the series :(

> Sorry about that, I will correct the subject lines for V3.

Don't worry about it unless you need to send a v3 for some other reason.

> Is there a set convention for the subsystem I should follow in future?
> Or should I follow the style of the individual files I work on?

Following the style for the file/directory is generally a good guide,
for SPI I tend to prefer spi: but I just moan about it rather than block
anything for it (unless I do end up missing the patch in my inbox).

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]