linux-renesas-soc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/2] i2c: designware: Add support for a bus clock
@ 2019-02-20 15:50 Gareth Williams
  2019-02-20 15:50 ` [PATCH v3 1/2] dt: snps,designware-i2c: Add clock bindings documentation Gareth Williams
  2019-02-20 15:50 ` [PATCH v3 2/2] i2c: designware: Add support for a bus clock Gareth Williams
  0 siblings, 2 replies; 6+ messages in thread
From: Gareth Williams @ 2019-02-20 15:50 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Alexandre Belloni, Wolfram Sang,
	Jarkko Nikula, Andy Shevchenko, Mika Westerberg
  Cc: Gareth Williams, devicetree, linux-i2c, linux-renesas-soc, linux-kernel

The Synopsys I2C Controller has a bus clock that some SoCs require to access
the registers. This series also details the new clock property in the bindings
documentation.

v3:
 - busclk renamed to pclk.
 - Added comment with dw_i2c_dev struct definition describing pclk.
 - Added enable rollback of first clock if second fails to enable.
 - Changed clocks and clock-names sections to use term "peripheral clock"
   (pclk) instead of "bus clock" (busclk) in dt-bindings documentation.
v2:
 - Use new devm_clk_get_optional() function as it simplifies handling when
   the optional clock is not present.
   
Phil Edworthy (2):
  dt: snps,designware-i2c: Add clock bindings documentation
  i2c: designware: Add support for a bus clock

 .../devicetree/bindings/i2c/i2c-designware.txt         |  9 +++++++++
 drivers/i2c/busses/i2c-designware-common.c             | 18 ++++++++++++++++--
 drivers/i2c/busses/i2c-designware-core.h               |  2 ++
 drivers/i2c/busses/i2c-designware-platdrv.c            |  5 +++++
 4 files changed, 32 insertions(+), 2 deletions(-)

-- 
2.7.4


^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH v3 1/2] dt: snps,designware-i2c: Add clock bindings documentation
  2019-02-20 15:50 [PATCH v3 0/2] i2c: designware: Add support for a bus clock Gareth Williams
@ 2019-02-20 15:50 ` Gareth Williams
  2019-02-20 19:52   ` Wolfram Sang
  2019-02-20 15:50 ` [PATCH v3 2/2] i2c: designware: Add support for a bus clock Gareth Williams
  1 sibling, 1 reply; 6+ messages in thread
From: Gareth Williams @ 2019-02-20 15:50 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Alexandre Belloni, Wolfram Sang
  Cc: Phil Edworthy, devicetree, linux-i2c, linux-renesas-soc

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

The driver requires an undocumented clock property, so detail it.
Add documentation for a separate, optional, bus clock.

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>

v3:
 - Changed clocks and clock-names sections to use term "peripheral clock"
   (pclk) instead of "bus clock" (busclk).
v2:
 - No changes.
v1:
 - Reviewed-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/i2c/i2c-designware.txt | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-designware.txt b/Documentation/devicetree/bindings/i2c/i2c-designware.txt
index 3e4bcc2..f94aa59 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-designware.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-designware.txt
@@ -6,12 +6,21 @@ Required properties :
                 or "mscc,ocelot-i2c" with "snps,designware-i2c" for fallback
  - reg : Offset and length of the register set for the device
  - interrupts : <IRQ> where IRQ is the interrupt number.
+ - clocks : phandles for the clocks, see the description of clock-names below.
+   The phandle for the "ic_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 "ic_clk" clock. If both clocks are listed, the "ic_clk" must be first.
 
 Recommended properties :
 
  - clock-frequency : desired I2C bus clock frequency in Hz.
 
 Optional properties :
+
+ - clock-names : Contains the names of the clocks:
+    "ic_clk", for the core clock used to generate the external I2C clock.
+    "pclk", the peripheral clock, required for register accesses.
+
  - reg : for "mscc,ocelot-i2c", a second register set to configure the SDA hold
    time, named ICPU_CFG:TWI_DELAY in the datasheet.
 
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH v3 2/2] i2c: designware: Add support for a bus clock
  2019-02-20 15:50 [PATCH v3 0/2] i2c: designware: Add support for a bus clock Gareth Williams
  2019-02-20 15:50 ` [PATCH v3 1/2] dt: snps,designware-i2c: Add clock bindings documentation Gareth Williams
@ 2019-02-20 15:50 ` Gareth Williams
  2019-02-20 19:55   ` Wolfram Sang
  1 sibling, 1 reply; 6+ messages in thread
From: Gareth Williams @ 2019-02-20 15:50 UTC (permalink / raw)
  To: Jarkko Nikula, Andy Shevchenko, Mika Westerberg, linux-i2c
  Cc: Phil Edworthy, linux-kernel, linux-renesas-soc, Gareth Williams

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

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

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
Signed-off-by: Gareth Williams <gareth.williams.jx@renesas.com>
---
v3:
 - busclk renamed to pclk.
 - Added comment with dw_i2c_dev struct definition describing pclk.
 - Added enable rollback of first clock if second fails to enable.
v2:
 - Use new devm_clk_get_optional() function as it simplifies handling when
   the optional clock is not present.
---
 drivers/i2c/busses/i2c-designware-common.c  | 18 ++++++++++++++++--
 drivers/i2c/busses/i2c-designware-core.h    |  2 ++
 drivers/i2c/busses/i2c-designware-platdrv.c |  5 +++++
 3 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
index a473011..5f70078 100644
--- a/drivers/i2c/busses/i2c-designware-common.c
+++ b/drivers/i2c/busses/i2c-designware-common.c
@@ -251,13 +251,27 @@ unsigned long i2c_dw_clk_rate(struct dw_i2c_dev *dev)
 
 int i2c_dw_prepare_clk(struct dw_i2c_dev *dev, bool prepare)
 {
+	int ret;
+
 	if (IS_ERR(dev->clk))
 		return PTR_ERR(dev->clk);
 
-	if (prepare)
-		return clk_prepare_enable(dev->clk);
+	if (prepare) {
+		/* Optional bus clock */
+		ret = clk_prepare_enable(dev->pclk);
+		if (ret)
+			return ret;
+
+		ret = clk_prepare_enable(dev->clk);
+		if (ret)
+			clk_disable_unprepare(dev->pclk);
+
+		return ret;
+	}
 
 	clk_disable_unprepare(dev->clk);
+	clk_disable_unprepare(dev->pclk);
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(i2c_dw_prepare_clk);
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index b4a0b2b..e88c711 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -177,6 +177,7 @@
  * @base: IO registers pointer
  * @cmd_complete: tx completion indicator
  * @clk: input reference clock
+ * @pclk: clock required to access the registers
  * @slave: represent an I2C slave device
  * @cmd_err: run time hadware error code
  * @msgs: points to an array of messages currently being transferred
@@ -226,6 +227,7 @@ struct dw_i2c_dev {
 	void __iomem		*ext;
 	struct completion	cmd_complete;
 	struct clk		*clk;
+	struct clk		*pclk;
 	struct reset_control	*rst;
 	struct i2c_client		*slave;
 	u32			(*get_clk_rate_khz) (struct dw_i2c_dev *dev);
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 9eaac3b..c550fb2 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -346,6 +346,11 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
 	else
 		i2c_dw_configure_master(dev);
 
+	/* Optional bus clock */
+	dev->pclk = devm_clk_get_optional(&pdev->dev, "pclk");
+	if (IS_ERR(dev->pclk))
+		return PTR_ERR(dev->pclk);
+
 	dev->clk = devm_clk_get(&pdev->dev, NULL);
 	if (!i2c_dw_prepare_clk(dev, true)) {
 		u64 clk_khz;
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/2] dt: snps,designware-i2c: Add clock bindings documentation
  2019-02-20 15:50 ` [PATCH v3 1/2] dt: snps,designware-i2c: Add clock bindings documentation Gareth Williams
@ 2019-02-20 19:52   ` Wolfram Sang
  0 siblings, 0 replies; 6+ messages in thread
From: Wolfram Sang @ 2019-02-20 19:52 UTC (permalink / raw)
  To: Gareth Williams
  Cc: Rob Herring, Mark Rutland, Alexandre Belloni, Phil Edworthy,
	devicetree, linux-i2c, linux-renesas-soc

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

On Wed, Feb 20, 2019 at 03:50:02PM +0000, Gareth Williams wrote:
> From: Phil Edworthy <phil.edworthy@renesas.com>
> 
> The driver requires an undocumented clock property, so detail it.
> Add documentation for a separate, optional, bus clock.
> 
> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>

Looks good to me:

Acked-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Let's see what the designware maintainers have to say.

> 
> v3:
>  - Changed clocks and clock-names sections to use term "peripheral clock"
>    (pclk) instead of "bus clock" (busclk).
> v2:
>  - No changes.
> v1:
>  - Reviewed-by: Rob Herring <robh@kernel.org>
> ---
>  Documentation/devicetree/bindings/i2c/i2c-designware.txt | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-designware.txt b/Documentation/devicetree/bindings/i2c/i2c-designware.txt
> index 3e4bcc2..f94aa59 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-designware.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c-designware.txt
> @@ -6,12 +6,21 @@ Required properties :
>                  or "mscc,ocelot-i2c" with "snps,designware-i2c" for fallback
>   - reg : Offset and length of the register set for the device
>   - interrupts : <IRQ> where IRQ is the interrupt number.
> + - clocks : phandles for the clocks, see the description of clock-names below.
> +   The phandle for the "ic_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 "ic_clk" clock. If both clocks are listed, the "ic_clk" must be first.
>  
>  Recommended properties :
>  
>   - clock-frequency : desired I2C bus clock frequency in Hz.
>  
>  Optional properties :
> +
> + - clock-names : Contains the names of the clocks:
> +    "ic_clk", for the core clock used to generate the external I2C clock.
> +    "pclk", the peripheral clock, required for register accesses.
> +
>   - reg : for "mscc,ocelot-i2c", a second register set to configure the SDA hold
>     time, named ICPU_CFG:TWI_DELAY in the datasheet.
>  
> -- 
> 2.7.4
> 

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

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH v3 2/2] i2c: designware: Add support for a bus clock
  2019-02-20 15:50 ` [PATCH v3 2/2] i2c: designware: Add support for a bus clock Gareth Williams
@ 2019-02-20 19:55   ` Wolfram Sang
  0 siblings, 0 replies; 6+ messages in thread
From: Wolfram Sang @ 2019-02-20 19:55 UTC (permalink / raw)
  To: Gareth Williams
  Cc: Jarkko Nikula, Andy Shevchenko, Mika Westerberg, linux-i2c,
	Phil Edworthy, linux-kernel, linux-renesas-soc

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

On Wed, Feb 20, 2019 at 03:50:03PM +0000, Gareth Williams wrote:
> From: Phil Edworthy <phil.edworthy@renesas.com>
> 
> The Synopsys I2C Controller has a bus clock, but most SoCs hide this away.
> However, on some SoCs you need to explicity enable the bus clock in order
> to access the registers. Therefore, add support for an optional bus clock.
> 
> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> Signed-off-by: Gareth Williams <gareth.williams.jx@renesas.com>

Code looks good to me. For clarity, though, s/bus clock/peripheral
clock/ in the commit message and code comments.

After that:

Acked-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

> ---
> v3:
>  - busclk renamed to pclk.
>  - Added comment with dw_i2c_dev struct definition describing pclk.
>  - Added enable rollback of first clock if second fails to enable.
> v2:
>  - Use new devm_clk_get_optional() function as it simplifies handling when
>    the optional clock is not present.
> ---
>  drivers/i2c/busses/i2c-designware-common.c  | 18 ++++++++++++++++--
>  drivers/i2c/busses/i2c-designware-core.h    |  2 ++
>  drivers/i2c/busses/i2c-designware-platdrv.c |  5 +++++
>  3 files changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
> index a473011..5f70078 100644
> --- a/drivers/i2c/busses/i2c-designware-common.c
> +++ b/drivers/i2c/busses/i2c-designware-common.c
> @@ -251,13 +251,27 @@ unsigned long i2c_dw_clk_rate(struct dw_i2c_dev *dev)
>  
>  int i2c_dw_prepare_clk(struct dw_i2c_dev *dev, bool prepare)
>  {
> +	int ret;
> +
>  	if (IS_ERR(dev->clk))
>  		return PTR_ERR(dev->clk);
>  
> -	if (prepare)
> -		return clk_prepare_enable(dev->clk);
> +	if (prepare) {
> +		/* Optional bus clock */
> +		ret = clk_prepare_enable(dev->pclk);
> +		if (ret)
> +			return ret;
> +
> +		ret = clk_prepare_enable(dev->clk);
> +		if (ret)
> +			clk_disable_unprepare(dev->pclk);
> +
> +		return ret;
> +	}
>  
>  	clk_disable_unprepare(dev->clk);
> +	clk_disable_unprepare(dev->pclk);
> +
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(i2c_dw_prepare_clk);
> diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
> index b4a0b2b..e88c711 100644
> --- a/drivers/i2c/busses/i2c-designware-core.h
> +++ b/drivers/i2c/busses/i2c-designware-core.h
> @@ -177,6 +177,7 @@
>   * @base: IO registers pointer
>   * @cmd_complete: tx completion indicator
>   * @clk: input reference clock
> + * @pclk: clock required to access the registers
>   * @slave: represent an I2C slave device
>   * @cmd_err: run time hadware error code
>   * @msgs: points to an array of messages currently being transferred
> @@ -226,6 +227,7 @@ struct dw_i2c_dev {
>  	void __iomem		*ext;
>  	struct completion	cmd_complete;
>  	struct clk		*clk;
> +	struct clk		*pclk;
>  	struct reset_control	*rst;
>  	struct i2c_client		*slave;
>  	u32			(*get_clk_rate_khz) (struct dw_i2c_dev *dev);
> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
> index 9eaac3b..c550fb2 100644
> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> @@ -346,6 +346,11 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
>  	else
>  		i2c_dw_configure_master(dev);
>  
> +	/* Optional bus clock */
> +	dev->pclk = devm_clk_get_optional(&pdev->dev, "pclk");
> +	if (IS_ERR(dev->pclk))
> +		return PTR_ERR(dev->pclk);
> +
>  	dev->clk = devm_clk_get(&pdev->dev, NULL);
>  	if (!i2c_dw_prepare_clk(dev, true)) {
>  		u64 clk_khz;
> -- 
> 2.7.4
> 

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

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH v3 1/2] dt: snps,designware-i2c: Add clock bindings documentation
  2019-02-20 13:25 [PATCH v3 0/2] " Gareth Williams
@ 2019-02-20 13:25 ` Gareth Williams
  0 siblings, 0 replies; 6+ messages in thread
From: Gareth Williams @ 2019-02-20 13:25 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Alexandre Belloni, Wolfram Sang
  Cc: Phil Edworthy, devicetree, linux-i2c, linux-renesas-soc

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

The driver requires an undocumented clock property, so detail it.
Add documentation for a separate, optional, bus clock.

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>

v3:
 - Changed clocks and clock-names sections to use term "peripheral clock"
   (pclk) instead of "bus clock" (busclk).
v2:
 - No changes.
v1:
 - Reviewed-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/i2c/i2c-designware.txt | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-designware.txt b/Documentation/devicetree/bindings/i2c/i2c-designware.txt
index 3e4bcc2..f94aa59 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-designware.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-designware.txt
@@ -6,12 +6,21 @@ Required properties :
                 or "mscc,ocelot-i2c" with "snps,designware-i2c" for fallback
  - reg : Offset and length of the register set for the device
  - interrupts : <IRQ> where IRQ is the interrupt number.
+ - clocks : phandles for the clocks, see the description of clock-names below.
+   The phandle for the "ic_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 "ic_clk" clock. If both clocks are listed, the "ic_clk" must be first.
 
 Recommended properties :
 
  - clock-frequency : desired I2C bus clock frequency in Hz.
 
 Optional properties :
+
+ - clock-names : Contains the names of the clocks:
+    "ic_clk", for the core clock used to generate the external I2C clock.
+    "pclk", the peripheral clock, required for register accesses.
+
  - reg : for "mscc,ocelot-i2c", a second register set to configure the SDA hold
    time, named ICPU_CFG:TWI_DELAY in the datasheet.
 
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-02-20 19:56 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-20 15:50 [PATCH v3 0/2] i2c: designware: Add support for a bus clock Gareth Williams
2019-02-20 15:50 ` [PATCH v3 1/2] dt: snps,designware-i2c: Add clock bindings documentation Gareth Williams
2019-02-20 19:52   ` Wolfram Sang
2019-02-20 15:50 ` [PATCH v3 2/2] i2c: designware: Add support for a bus clock Gareth Williams
2019-02-20 19:55   ` Wolfram Sang
  -- strict thread matches above, loose matches on Subject: below --
2019-02-20 13:25 [PATCH v3 0/2] " Gareth Williams
2019-02-20 13:25 ` [PATCH v3 1/2] dt: snps,designware-i2c: Add clock bindings documentation Gareth Williams

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).