| Message ID | 20180112103956.4875-3-gregory.clement@free-electrons.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On Fri, Jan 12, 2018 at 11:39:56AM +0100, Gregory CLEMENT wrote: > On Armada 7K/8K we need to explicitly enable the bus clock. The bus clock > is optional because not all the SoCs need them but at least for Armada > 7K/8K it is actually mandatory. > > The binding documentation is updating accordingly. > > Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> > --- > .../devicetree/bindings/i2c/i2c-mv64xxx.txt | 20 ++++++++++++++++++++ > drivers/i2c/busses/i2c-mv64xxx.c | 12 +++++++++++- > 2 files changed, 31 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt b/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt > index 5c30026921ae..3d76bb19492f 100644 > --- a/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt > +++ b/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt > @@ -25,6 +25,15 @@ default frequency is 100kHz > whenever you're using the "allwinner,sun6i-a31-i2c" > compatible. > > + - clocks: : pointers to the reference clocks for this device, the > + first one is the one used for the clock on the i2c bus, > + the second one is the clock used for the functional part > + of the controller This documentation is confusing, as usually the functional clock is the clock driving the bus, as opposed to the interface clock clocking the bus interface. > + - clock-names : names of used clocks, mandatory if the second clock is > + used, the name must be "core", and "axi" (the latter is > + only for Armada 7K/8K). > + Are you sure the i2c controller is on the AXI bus? It seems more likely to be on an APB bus. Maxime
Hi Maxime, On ven., janv. 12 2018, Maxime Ripard <maxime.ripard@free-electrons.com> wrote: > Hi, > > On Fri, Jan 12, 2018 at 11:39:56AM +0100, Gregory CLEMENT wrote: >> On Armada 7K/8K we need to explicitly enable the bus clock. The bus clock >> is optional because not all the SoCs need them but at least for Armada >> 7K/8K it is actually mandatory. >> >> The binding documentation is updating accordingly. >> >> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> >> --- >> .../devicetree/bindings/i2c/i2c-mv64xxx.txt | 20 ++++++++++++++++++++ >> drivers/i2c/busses/i2c-mv64xxx.c | 12 +++++++++++- >> 2 files changed, 31 insertions(+), 1 deletion(-) >> >> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt b/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt >> index 5c30026921ae..3d76bb19492f 100644 >> --- a/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt >> +++ b/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt >> @@ -25,6 +25,15 @@ default frequency is 100kHz >> whenever you're using the "allwinner,sun6i-a31-i2c" >> compatible. >> >> + - clocks: : pointers to the reference clocks for this device, the >> + first one is the one used for the clock on the i2c bus, >> + the second one is the clock used for the functional part >> + of the controller > > This documentation is confusing, as usually the functional clock is > the clock driving the bus, as opposed to the interface clock clocking > the bus interface. I don't find it less confusing, for my point of view the interface of a bus is the lines interfacing the bus with the devices on this bus. But I don't mind modifying it, as I clearly state that the first clock is the one used to generate the clock of the i2c bus, so we can change the name. > >> + - clock-names : names of used clocks, mandatory if the second clock is >> + used, the name must be "core", and "axi" (the latter is >> + only for Armada 7K/8K). >> + > > Are you sure the i2c controller is on the AXI bus? It seems more > likely to be on an APB bus. No I am not sure :) Actually I don't have this information, so lets call it "reg" for register clock. Gregory > > Maxime > > -- > Maxime Ripard, Free Electrons > Embedded Linux and Kernel engineering > http://free-electrons.com >
> > Are you sure the i2c controller is on the AXI bus? It seems more > > likely to be on an APB bus. > > No I am not sure :) > Actually I don't have this information, so lets call it "reg" for > register clock. So, I assume a V4 is needed for this series. BTW would be someone from FreeElectrons be up to maintaining this driver?
Hi Wolfram, On lun., janv. 15 2018, Wolfram Sang <wsa@the-dreams.de> wrote: >> > Are you sure the i2c controller is on the AXI bus? It seems more >> > likely to be on an APB bus. >> >> No I am not sure :) >> Actually I don't have this information, so lets call it "reg" for >> register clock. > > So, I assume a V4 is needed for this series. > > BTW would be someone from FreeElectrons be up to maintaining this > driver? So I am going to add a patch adding myself as maintainer to the series in the v4. Gregory >
diff --git a/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt b/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt index 5c30026921ae..3d76bb19492f 100644 --- a/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt +++ b/Documentation/devicetree/bindings/i2c/i2c-mv64xxx.txt @@ -25,6 +25,15 @@ default frequency is 100kHz whenever you're using the "allwinner,sun6i-a31-i2c" compatible. + - clocks: : pointers to the reference clocks for this device, the + first one is the one used for the clock on the i2c bus, + the second one is the clock used for the functional part + of the controller + + - clock-names : names of used clocks, mandatory if the second clock is + used, the name must be "core", and "axi" (the latter is + only for Armada 7K/8K). + Examples: i2c@11000 { @@ -42,3 +51,14 @@ For the Armada XP: interrupts = <29>; clock-frequency = <100000>; }; + +For the Armada 7040: + + i2c@701000 { + compatible = "marvell,mv78230-i2c"; + reg = <0x701000 0x20>; + interrupts = <29>; + clock-frequency = <100000>; + clock-names = "core", "axi"; + clocks = <&core_clock>, <&axi_clock>; + }; diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c index f69066266faa..cce37d8ecf41 100644 --- a/drivers/i2c/busses/i2c-mv64xxx.c +++ b/drivers/i2c/busses/i2c-mv64xxx.c @@ -135,6 +135,7 @@ struct mv64xxx_i2c_data { u32 freq_m; u32 freq_n; struct clk *clk; + struct clk *axi_clk; wait_queue_head_t waitq; spinlock_t lock; struct i2c_msg *msg; @@ -894,13 +895,20 @@ mv64xxx_i2c_probe(struct platform_device *pd) init_waitqueue_head(&drv_data->waitq); spin_lock_init(&drv_data->lock); - /* Not all platforms have a clk */ + /* Not all platforms have clocks */ drv_data->clk = devm_clk_get(&pd->dev, NULL); if (IS_ERR(drv_data->clk) && PTR_ERR(drv_data->clk) == -EPROBE_DEFER) return -EPROBE_DEFER; if (!IS_ERR(drv_data->clk)) clk_prepare_enable(drv_data->clk); + drv_data->axi_clk = devm_clk_get(&pd->dev, "axi"); + if (IS_ERR(drv_data->axi_clk) && + PTR_ERR(drv_data->axi_clk) == -EPROBE_DEFER) + return -EPROBE_DEFER; + if (!IS_ERR(drv_data->axi_clk)) + clk_prepare_enable(drv_data->axi_clk); + drv_data->irq = platform_get_irq(pd, 0); if (pdata) { @@ -950,6 +958,7 @@ mv64xxx_i2c_probe(struct platform_device *pd) exit_reset: reset_control_assert(drv_data->rstc); exit_clk: + clk_disable_unprepare(drv_data->axi_clk); clk_disable_unprepare(drv_data->clk); return rc; @@ -963,6 +972,7 @@ mv64xxx_i2c_remove(struct platform_device *dev) i2c_del_adapter(&drv_data->adapter); free_irq(drv_data->irq, drv_data); reset_control_assert(drv_data->rstc); + clk_disable_unprepare(drv_data->axi_clk); clk_disable_unprepare(drv_data->clk); return 0;
On Armada 7K/8K we need to explicitly enable the bus clock. The bus clock is optional because not all the SoCs need them but at least for Armada 7K/8K it is actually mandatory. The binding documentation is updating accordingly. Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> --- .../devicetree/bindings/i2c/i2c-mv64xxx.txt | 20 ++++++++++++++++++++ drivers/i2c/busses/i2c-mv64xxx.c | 12 +++++++++++- 2 files changed, 31 insertions(+), 1 deletion(-)