[PATCH 0/2] spi: dw: Add support for an optional interface clock

[PATCH 0/2] spi: dw: Add support for an optional interface clock

[Gareth Williams]

The Synopsys SSI Controller has an interface clock that must be 
explicitly enabled in order to access the registers. This patch series
adds support for the interface clock and adds the associated bindings
documentation.

Phil Edworthy (2):
  dt: snps,dw-apb-ssi: Add clock bindings documentation
  spi: dw: Add support for an optional interface clock

 Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt | 10 +++++++++-
 drivers/spi/spi-dw-mmio.c                                 | 12 ++++++++++++
 2 files changed, 21 insertions(+), 1 deletion(-)

-- 
2.7.4

[PATCH 1/2] dt: snps,dw-apb-ssi: Add clock bindings documentation

[Gareth Williams]

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

The driver requires a clock property, so detail it in the docs.
Fix a typo, 'pis' to 'pins'.
Add documentation for a separate, optional, interface clock.

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
Signed-off-by: Gareth Williams <gareth.williams.jx@renesas.com>
---
 Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt | 10 +++++++++-
 1 file changed, 9 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 2864bc6..7971193 100644
--- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
+++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
@@ -8,9 +8,16 @@ Required properties:
 - interrupts : One interrupt, used by the controller.
 - #address-cells : <1>, as required by generic SPI binding.
 - #size-cells : <0>, also as required by generic SPI binding.
+- clocks : phandles for the clocks, see the description of clock-names below.
+   The phandle for the "ssi_clk" clock is required. The phandle for the "pclk"
+   clock is optional. If a single clock is specified but no clock-name, it is
+   the "ssi_clk" clock. If both clocks are listed, the "ssi_clk" must be first.
 
 Optional properties:
-- cs-gpios : Specifies the gpio pis to be used for chipselects.
+- 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.
+- 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
   device.  Supported values are 2 or 4 (the default).
@@ -25,6 +32,7 @@ Example:
 		interrupts = <0 154 4>;
 		#address-cells = <1>;
 		#size-cells = <0>;
+		clocks = <&spi_m_clk>;
 		num-cs = <2>;
 		cs-gpios = <&gpio0 13 0>,
 			   <&gpio0 14 0>;
-- 
2.7.4

[PATCH 2/2] spi: dw: Add support for an optional interface clock

[Gareth Williams]

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

The Synopsys SSI Controller has an interface clock, but most SoCs hide 
this away. However, on some SoCs you need to explicity enable the 
interface clock in order to access the registers. Therefore, add 
support for an optional interface clock.

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
Signed-off-by: Gareth Williams <gareth.williams.jx@renesas.com>
---
 drivers/spi/spi-dw-mmio.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c
index 4bd59a9..7cbc173 100644
--- a/drivers/spi/spi-dw-mmio.c
+++ b/drivers/spi/spi-dw-mmio.c
@@ -30,6 +30,7 @@
 struct dw_spi_mmio {
 	struct dw_spi  dws;
 	struct clk     *clk;
+	struct clk     *pclk;
 	void           *priv;
 };
 
@@ -172,6 +173,14 @@ static int dw_spi_mmio_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	/* Optional interface clock */
+	dwsmmio->pclk = devm_clk_get_optional(&pdev->dev, "pclk");
+	if (IS_ERR(dwsmmio->pclk))
+		return PTR_ERR(dwsmmio->pclk);
+	ret = clk_prepare_enable(dwsmmio->pclk);
+	if (ret)
+		goto out_clk;
+
 	dws->bus_num = pdev->id;
 
 	dws->max_freq = clk_get_rate(dwsmmio->clk);
@@ -199,6 +208,8 @@ static int dw_spi_mmio_probe(struct platform_device *pdev)
 	return 0;
 
 out:
+	clk_disable_unprepare(dwsmmio->pclk);
+out_clk:
 	clk_disable_unprepare(dwsmmio->clk);
 	return ret;
 }
@@ -208,6 +219,7 @@ static int dw_spi_mmio_remove(struct platform_device *pdev)
 	struct dw_spi_mmio *dwsmmio = platform_get_drvdata(pdev);
 
 	dw_spi_remove_host(&dwsmmio->dws);
+	clk_disable_unprepare(dwsmmio->pclk);
 	clk_disable_unprepare(dwsmmio->clk);
 
 	return 0;
-- 
2.7.4

Re: [PATCH 1/2] dt: snps,dw-apb-ssi: Add clock bindings documentation

[Mark Brown]

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

On Thu, Feb 28, 2019 at 01:25:41PM +0000, Gareth Williams wrote:
> From: Phil Edworthy <phil.edworthy@renesas.com>
> 
> The driver requires a clock property, so detail it in the docs.
> Fix a typo, 'pis' to 'pins'.
> Add documentation for a separate, optional, interface clock.

Please use subject lines matching the style for the subsystem.  This
makes it easier for people to identify relevant patches.

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

Re: [PATCH 2/2] spi: dw: Add support for an optional interface clock

[Mark Brown]

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

On Thu, Feb 28, 2019 at 01:25:42PM +0000, Gareth Williams wrote:
> From: Phil Edworthy <phil.edworthy@renesas.com>
> 
> The Synopsys SSI Controller has an interface clock, but most SoCs hide 
> this away. However, on some SoCs you need to explicity enable the 
> interface clock in order to access the registers. Therefore, add 
> support for an optional interface clock.

This doesn't build for me:

  CC      drivers/spi/spi-dw-mmio.o
drivers/spi/spi-dw-mmio.c: In function ‘dw_spi_mmio_probe’:
drivers/spi/spi-dw-mmio.c:177:18: error: implicit declaration of function ‘devm_clk_get_optional’; did you mean ‘devm_gpiod_get_optional’? [-Werror=implicit-function-declaration]
  dwsmmio->pclk = devm_clk_get_optional(&pdev->dev, "pclk");
                  ^~~~~~~~~~~~~~~~~~~~~
                  devm_gpiod_get_optional
drivers/spi/spi-dw-mmio.c:177:16: warning: assignment to ‘struct clk *’ from ‘int’ makes pointer from integer without a cast [-Wint-conversion]
  dwsmmio->pclk = devm_clk_get_optional(&pdev->dev, "pclk");
                ^

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

RE: [PATCH 2/2] spi: dw: Add support for an optional interface clock

[Gareth Williams]

Hi Mark,

> On Mon, Mar 04, 2019 00:05:00, Mark Brown wrote:
>
> On Thu, Feb 28, 2019 at 01:25:42PM +0000, Gareth Williams wrote:
> > From: Phil Edworthy <phil.edworthy@renesas.com>
> >
> > The Synopsys SSI Controller has an interface clock, but most SoCs hide
> > this away. However, on some SoCs you need to explicity enable the
> > interface clock in order to access the registers. Therefore, add
> > support for an optional interface clock.
>
> This doesn't build for me:
>
>   CC      drivers/spi/spi-dw-mmio.o
> drivers/spi/spi-dw-mmio.c: In function ‘dw_spi_mmio_probe’:
> drivers/spi/spi-dw-mmio.c:177:18: error: implicit declaration of function
> ‘devm_clk_get_optional’; did you mean ‘devm_gpiod_get_optional’? [-
> Werror=implicit-function-declaration]
>   dwsmmio->pclk = devm_clk_get_optional(&pdev->dev, "pclk");
>                   ^~~~~~~~~~~~~~~~~~~~~
>                   devm_gpiod_get_optional
> drivers/spi/spi-dw-mmio.c:177:16: warning: assignment to ‘struct clk *’ from
> ‘int’ makes pointer from integer without a cast [-Wint-conversion]
>   dwsmmio->pclk = devm_clk_get_optional(&pdev->dev, "pclk");
>                 ^
Sorry, I should have noted the dependency for the patch below. I will add this into the next version.
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/clk/clk-devres.c?h=next-20190304&id=60b8f0ddf1a927ef02141a6610fd52575134f821

Kind Regards,

Gareth


Renesas Electronics Europe GmbH,Geschaeftsfuehrer/President : Michael Hannawald, Sitz der Gesellschaft/Registered office: Duesseldorf, Arcadiastrasse 10, 40472 Duesseldorf, Germany,Handelsregister/Commercial Register: Duesseldorf, HRB 3708 USt-IDNr./Tax identification no.: DE 119353406 WEEE-Reg.-Nr./WEEE reg. no.: DE 14978647

RE: [PATCH 1/2] dt: snps,dw-apb-ssi: Add clock bindings documentation

[Gareth Williams]

> On Sat, Mar 03, 2019 at 23:32 +0100, Mark Brown wrote:
> > On Thu, Feb 28, 2019 at 01:25:41PM +0000, Gareth Williams wrote:
> > From: Phil Edworthy <phil.edworthy@renesas.com>
> >
> > The driver requires a clock property, so detail it in the docs.
> > Fix a typo, 'pis' to 'pins'.
> > Add documentation for a separate, optional, interface clock.
> 
> Please use subject lines matching the style for the subsystem.  This makes it
> easier for people to identify relevant patches.

Thanks for the feedback Mark.
I will wait another week to see if there is any other feedback before sending an updated version of this patch series.