* [PATCH v4 1/2] dt: snps,designware-i2c: Add clock bindings documentation
2019-02-21 16:10 [PATCH v4 0/2] i2c: designware: Add support for a bus clock Gareth Williams
@ 2019-02-21 16:10 ` Gareth Williams
2019-02-22 15:06 ` Rob Herring
2019-02-26 14:54 ` Jarkko Nikula
2019-02-21 16:10 ` [PATCH v4 2/2] i2c: designware: Add support for a peripheral clock Gareth Williams
2019-02-23 9:32 ` [PATCH v4 0/2] i2c: designware: Add support for a bus clock Wolfram Sang
2 siblings, 2 replies; 11+ messages in thread
From: Gareth Williams @ 2019-02-21 16:10 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, peripheral clock.
Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
Acked-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
v4:
- Updated commit message to reference "peripheral clock" instead of
"bus clock"
- Added Wolfram's Acked-by
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] 11+ messages in thread* Re: [PATCH v4 1/2] dt: snps,designware-i2c: Add clock bindings documentation
2019-02-21 16:10 ` [PATCH v4 1/2] dt: snps,designware-i2c: Add clock bindings documentation Gareth Williams
@ 2019-02-22 15:06 ` Rob Herring
2019-02-26 14:54 ` Jarkko Nikula
1 sibling, 0 replies; 11+ messages in thread
From: Rob Herring @ 2019-02-22 15:06 UTC (permalink / raw)
To: Gareth Williams
Cc: Mark Rutland, Alexandre Belloni, Wolfram Sang, Phil Edworthy,
devicetree, linux-i2c, linux-renesas-soc
On Thu, Feb 21, 2019 at 04:10:58PM +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, peripheral clock.
>
> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> Acked-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
> v4:
> - Updated commit message to reference "peripheral clock" instead of
> "bus clock"
> - Added Wolfram's Acked-by
> 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(+)
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 1/2] dt: snps,designware-i2c: Add clock bindings documentation
2019-02-21 16:10 ` [PATCH v4 1/2] dt: snps,designware-i2c: Add clock bindings documentation Gareth Williams
2019-02-22 15:06 ` Rob Herring
@ 2019-02-26 14:54 ` Jarkko Nikula
2019-02-26 15:39 ` Wolfram Sang
2019-02-26 18:39 ` Luis de Oliveira
1 sibling, 2 replies; 11+ messages in thread
From: Jarkko Nikula @ 2019-02-26 14:54 UTC (permalink / raw)
To: Gareth Williams, Rob Herring, Mark Rutland, Alexandre Belloni,
Wolfram Sang
Cc: Phil Edworthy, devicetree, linux-i2c, linux-renesas-soc, Luis Oliveira
Hi
+ Luis from Synopsys.
Sorry the delay, I was out of office last week. Comment below.
On 2/21/19 6:10 PM, 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, peripheral clock.
>
> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> Acked-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
> v4:
> - Updated commit message to reference "peripheral clock" instead of
> "bus clock"
> - Added Wolfram's Acked-by
> v3:
> - Changed clocks and clock-names sections to use term "peripheral clock"
> (pclk) instead of "bus clock" (busclk).
...
> 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.
> +
Actually it looks there is need to revert back to bus clock (or better)
in comments but keep the "pclk" property.
The specification I have tells the ic_clk is the peripheral clock which
runs the logic and the pclk (exactly pclk) is for bus interface and
where registers are.
Luis: did I interpret it right?
--
Jarkko
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 1/2] dt: snps,designware-i2c: Add clock bindings documentation
2019-02-26 14:54 ` Jarkko Nikula
@ 2019-02-26 15:39 ` Wolfram Sang
2019-02-27 7:10 ` Jarkko Nikula
2019-02-26 18:39 ` Luis de Oliveira
1 sibling, 1 reply; 11+ messages in thread
From: Wolfram Sang @ 2019-02-26 15:39 UTC (permalink / raw)
To: Jarkko Nikula
Cc: Gareth Williams, Rob Herring, Mark Rutland, Alexandre Belloni,
Phil Edworthy, devicetree, linux-i2c, linux-renesas-soc,
Luis Oliveira
[-- Attachment #1: Type: text/plain, Size: 597 bytes --]
> > + - 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.
> > +
>
> Actually it looks there is need to revert back to bus clock (or better) in
> comments but keep the "pclk" property.
>
> The specification I have tells the ic_clk is the peripheral clock which runs
> the logic and the pclk (exactly pclk) is for bus interface and where
> registers are.
Can we make it "bus interface clock" then? I'd think this is a tad
better.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 1/2] dt: snps,designware-i2c: Add clock bindings documentation
2019-02-26 15:39 ` Wolfram Sang
@ 2019-02-27 7:10 ` Jarkko Nikula
2019-02-27 9:43 ` Luis de Oliveira
0 siblings, 1 reply; 11+ messages in thread
From: Jarkko Nikula @ 2019-02-27 7:10 UTC (permalink / raw)
To: Wolfram Sang
Cc: Gareth Williams, Rob Herring, Mark Rutland, Alexandre Belloni,
Phil Edworthy, devicetree, linux-i2c, linux-renesas-soc,
Luis Oliveira
On 2/26/19 5:39 PM, Wolfram Sang wrote:
>
>>> + - 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.
>>> +
>>
>> Actually it looks there is need to revert back to bus clock (or better) in
>> comments but keep the "pclk" property.
>>
>> The specification I have tells the ic_clk is the peripheral clock which runs
>> the logic and the pclk (exactly pclk) is for bus interface and where
>> registers are.
>
> Can we make it "bus interface clock" then? I'd think this is a tad
> better.
>
Yes, that makes it clear. Plain "interface clock" might work too. TI
OMAPs are using that term for register access clock domains.
Luis: Does that make sense for HW point of view? You mention PCLK is
called also as application clock but for me personally it is not as
clear as interface clock when I see it. I'll let Luis have the final
word here.
ic_clk - peripheral clock
pclk - (bus) interface/application clock
--
Jarkko
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 1/2] dt: snps,designware-i2c: Add clock bindings documentation
2019-02-27 7:10 ` Jarkko Nikula
@ 2019-02-27 9:43 ` Luis de Oliveira
0 siblings, 0 replies; 11+ messages in thread
From: Luis de Oliveira @ 2019-02-27 9:43 UTC (permalink / raw)
To: Jarkko Nikula, Wolfram Sang
Cc: Gareth Williams, Rob Herring, Mark Rutland, Alexandre Belloni,
Phil Edworthy, devicetree, linux-i2c, linux-renesas-soc,
Luis Oliveira
Hi,
Thanks Jarkko.
Yes, "interface clock" for pclk seems good.
Thanks,
Luis
On 27-Feb-19 7:10, Jarkko Nikula wrote:
> On 2/26/19 5:39 PM, Wolfram Sang wrote:
>>
>>>> + - 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.
>>>> +
>>>
>>> Actually it looks there is need to revert back to bus clock (or better) in
>>> comments but keep the "pclk" property.
>>>
>>> The specification I have tells the ic_clk is the peripheral clock which runs
>>> the logic and the pclk (exactly pclk) is for bus interface and where
>>> registers are.
>>
>> Can we make it "bus interface clock" then? I'd think this is a tad
>> better.
>>
> Yes, that makes it clear. Plain "interface clock" might work too. TI OMAPs are
> using that term for register access clock domains.
>
> Luis: Does that make sense for HW point of view? You mention PCLK is called also
> as application clock but for me personally it is not as clear as interface clock
> when I see it. I'll let Luis have the final word here.
>
> ic_clk - peripheral clock
> pclk - (bus) interface/application clock
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 1/2] dt: snps,designware-i2c: Add clock bindings documentation
2019-02-26 14:54 ` Jarkko Nikula
2019-02-26 15:39 ` Wolfram Sang
@ 2019-02-26 18:39 ` Luis de Oliveira
1 sibling, 0 replies; 11+ messages in thread
From: Luis de Oliveira @ 2019-02-26 18:39 UTC (permalink / raw)
To: Jarkko Nikula, Gareth Williams, Rob Herring, Mark Rutland,
Alexandre Belloni, Wolfram Sang
Cc: Phil Edworthy, devicetree, linux-i2c, linux-renesas-soc, Luis Oliveira
Hi,
Yes Jarkko.
Registers are on the PCLK domain (applications clock) and the IC_CLK is the
clock that controls peripheral logic (like SCL definition).
Thanks!
On 26-Feb-19 14:54, Jarkko Nikula wrote:
> Hi
>
> + Luis from Synopsys.
>
> Sorry the delay, I was out of office last week. Comment below.
>
> On 2/21/19 6:10 PM, 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, peripheral clock.
>>
>> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
>> Acked-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
>> ---
>> v4:
>> - Updated commit message to reference "peripheral clock" instead of
>> "bus clock"
>> - Added Wolfram's Acked-by
>> v3:
>> - Changed clocks and clock-names sections to use term "peripheral clock"
>> (pclk) instead of "bus clock" (busclk).
> ...
>> 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.
>> +
>
> Actually it looks there is need to revert back to bus clock (or better) in
> comments but keep the "pclk" property.
>
> The specification I have tells the ic_clk is the peripheral clock which runs the
> logic and the pclk (exactly pclk) is for bus interface and where registers are.
>
> Luis: did I interpret it right?
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v4 2/2] i2c: designware: Add support for a peripheral clock
2019-02-21 16:10 [PATCH v4 0/2] i2c: designware: Add support for a bus clock Gareth Williams
2019-02-21 16:10 ` [PATCH v4 1/2] dt: snps,designware-i2c: Add clock bindings documentation Gareth Williams
@ 2019-02-21 16:10 ` Gareth Williams
2019-02-23 9:32 ` [PATCH v4 0/2] i2c: designware: Add support for a bus clock Wolfram Sang
2 siblings, 0 replies; 11+ messages in thread
From: Gareth Williams @ 2019-02-21 16:10 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 peripheral clock, but most SoCs hide
this away. However, some SoCs need explicit enabling of the peripheral
clock in order to access the registers. Therefore, add support for an
optional peripheral clock.
Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
Signed-off-by: Gareth Williams <gareth.williams.jx@renesas.com>
Acked-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
v4:
- Updated comments to reference "peripheral clock"
instead of "bus clock".
- Added Wolfram's Acked-by
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..e807aa2 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 peripheral 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..03fff6a 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 peripheral 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] 11+ messages in thread* Re: [PATCH v4 0/2] i2c: designware: Add support for a bus clock
2019-02-21 16:10 [PATCH v4 0/2] i2c: designware: Add support for a bus clock Gareth Williams
2019-02-21 16:10 ` [PATCH v4 1/2] dt: snps,designware-i2c: Add clock bindings documentation Gareth Williams
2019-02-21 16:10 ` [PATCH v4 2/2] i2c: designware: Add support for a peripheral clock Gareth Williams
@ 2019-02-23 9:32 ` Wolfram Sang
2019-02-25 9:28 ` Gareth Williams
2 siblings, 1 reply; 11+ messages in thread
From: Wolfram Sang @ 2019-02-23 9:32 UTC (permalink / raw)
To: Gareth Williams
Cc: Rob Herring, Mark Rutland, Alexandre Belloni, Jarkko Nikula,
Andy Shevchenko, Mika Westerberg, devicetree, linux-i2c,
linux-renesas-soc, linux-kernel, Phil Edworthy
[-- Attachment #1: Type: text/plain, Size: 378 bytes --]
On Thu, Feb 21, 2019 at 04:10:57PM +0000, Gareth Williams wrote:
> The Synopsys I2C Controller has a peripheral clock that some SoCs
> require to access the registers. This series also details the new clock
> property in the bindings documentation.
Your SoB is missing for patch 1. And besides my ack, I'd still like some
feedback from the actual driver maintainers.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH v4 0/2] i2c: designware: Add support for a bus clock
2019-02-23 9:32 ` [PATCH v4 0/2] i2c: designware: Add support for a bus clock Wolfram Sang
@ 2019-02-25 9:28 ` Gareth Williams
0 siblings, 0 replies; 11+ messages in thread
From: Gareth Williams @ 2019-02-25 9:28 UTC (permalink / raw)
To: Wolfram Sang
Cc: Rob Herring, Mark Rutland, Alexandre Belloni, Jarkko Nikula,
Andy Shevchenko, Mika Westerberg, devicetree, linux-i2c,
linux-renesas-soc, linux-kernel, Phil Edworthy
> On Sat, Feb 23, 2019 at 09:32 +0100, wsa@the-dreams.de wrote:
>> On Thu, Feb 21, 2019 at 04:10:57PM +0000, Gareth Williams wrote:
>> The Synopsys I2C Controller has a peripheral clock that some SoCs
>> require to access the registers. This series also details the new
>> clock property in the bindings documentation.
>
> On Sat, Feb 23, 2019 at 09:32 +0100, wsa@the-dreams.de wrote:
> Your SoB is missing for patch 1. And besides my ack, I'd still like some feedback from the actual driver maintainers.
Thanks for the feedback Wolfram. I will wait for input from the driver maintainers.
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
^ permalink raw reply [flat|nested] 11+ messages in thread