| Message ID | 1550765459-14519-2-git-send-email-gareth.williams.jx@renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | i2c: designware: Add support for a bus clock | expand |
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>
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?
> > + - 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.
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? >
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
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 >
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.