2019-02-28 13:34:10

by Gareth Williams

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

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



2019-02-28 13:34:12

by Gareth Williams

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

From: Phil Edworthy <[email protected]>

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 <[email protected]>
Signed-off-by: Gareth Williams <[email protected]>
---
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


2019-02-28 13:35:17

by Gareth Williams

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

From: Phil Edworthy <[email protected]>

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 <[email protected]>
Signed-off-by: Gareth Williams <[email protected]>
---
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


2019-03-03 23:32:50

by Mark Brown

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

On Thu, Feb 28, 2019 at 01:25:41PM +0000, Gareth Williams wrote:
> From: Phil Edworthy <[email protected]>
>
> 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.


Attachments:
(No filename) (414.00 B)
signature.asc (499.00 B)
Download all attachments

2019-03-04 00:06:12

by Mark Brown

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

On Thu, Feb 28, 2019 at 01:25:42PM +0000, Gareth Williams wrote:
> From: Phil Edworthy <[email protected]>
>
> 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");
^


Attachments:
(No filename) (1.06 kB)
signature.asc (499.00 B)
Download all attachments

2019-03-04 15:23:05

by Gareth Williams

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

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 <[email protected]>
> >
> > 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

2019-03-11 11:52:24

by Gareth Williams

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

> 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 <[email protected]>
> >
> > 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.