* [PATCH v3 0/2] i2c: mv64xxx: Fix clock resource for Armada 7K/8K @ 2018-01-12 10:39 Gregory CLEMENT 2018-01-12 10:39 ` [PATCH v3 1/2] i2c: mv64xxx: Remove useless test before clk_disable_unprepare Gregory CLEMENT 2018-01-12 10:39 ` [PATCH v3 2/2] i2c: mv64xxx: Fix clock resource by adding an optional bus clock Gregory CLEMENT 0 siblings, 2 replies; 6+ messages in thread From: Gregory CLEMENT @ 2018-01-12 10:39 UTC (permalink / raw) To: linux-arm-kernel Hi, This short series fixes the way the clocks are used for the mv64xxx controller embedded in the Marvell Armada 7K/8K SoCs. On these SoCs a second one is needed in order to clock the registers. It was not noticed until now because we relied on the bootloader and also because the clock driver was wrong. Thanks to this fix, it would be possible to fix the clock driver without introducing a regression. The first patch is just a small cleanup found when I wrote the main patch. Thanks, Gregory Changelog: v1 -> v2: - Really add the binding documentation in the second patch, noticed by Thomas Petazzoni. v2 -> v3 - Fix typo in binding documentation reported by Thomas Petazzoni - Use correct name for the axi clock, reported by Thomas Petazzoni Gregory CLEMENT (2): i2c: mv64xxx: Remove useless test before clk_disable_unprepare i2c: mv64xxx: Fix clock resource by adding an optional bus clock .../devicetree/bindings/i2c/i2c-mv64xxx.txt | 20 ++++++++++++++++++++ drivers/i2c/busses/i2c-mv64xxx.c | 20 +++++++++++++------- 2 files changed, 33 insertions(+), 7 deletions(-) -- 2.15.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3 1/2] i2c: mv64xxx: Remove useless test before clk_disable_unprepare 2018-01-12 10:39 [PATCH v3 0/2] i2c: mv64xxx: Fix clock resource for Armada 7K/8K Gregory CLEMENT @ 2018-01-12 10:39 ` Gregory CLEMENT 2018-01-12 10:39 ` [PATCH v3 2/2] i2c: mv64xxx: Fix clock resource by adding an optional bus clock Gregory CLEMENT 1 sibling, 0 replies; 6+ messages in thread From: Gregory CLEMENT @ 2018-01-12 10:39 UTC (permalink / raw) To: linux-arm-kernel The 2 functions called from clk_disable_unprepare() already check that the clock pointer is valid: no need to test it before calling it. Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> --- drivers/i2c/busses/i2c-mv64xxx.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c index a832c45276a4..f69066266faa 100644 --- a/drivers/i2c/busses/i2c-mv64xxx.c +++ b/drivers/i2c/busses/i2c-mv64xxx.c @@ -950,9 +950,7 @@ mv64xxx_i2c_probe(struct platform_device *pd) exit_reset: reset_control_assert(drv_data->rstc); exit_clk: - /* Not all platforms have a clk */ - if (!IS_ERR(drv_data->clk)) - clk_disable_unprepare(drv_data->clk); + clk_disable_unprepare(drv_data->clk); return rc; } @@ -965,9 +963,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); - /* Not all platforms have a clk */ - if (!IS_ERR(drv_data->clk)) - clk_disable_unprepare(drv_data->clk); + clk_disable_unprepare(drv_data->clk); return 0; } -- 2.15.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v3 2/2] i2c: mv64xxx: Fix clock resource by adding an optional bus clock 2018-01-12 10:39 [PATCH v3 0/2] i2c: mv64xxx: Fix clock resource for Armada 7K/8K Gregory CLEMENT 2018-01-12 10:39 ` [PATCH v3 1/2] i2c: mv64xxx: Remove useless test before clk_disable_unprepare Gregory CLEMENT @ 2018-01-12 10:39 ` Gregory CLEMENT 2018-01-12 10:54 ` Maxime Ripard 1 sibling, 1 reply; 6+ messages in thread From: Gregory CLEMENT @ 2018-01-12 10:39 UTC (permalink / raw) To: linux-arm-kernel 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 + + - 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 at 11000 { @@ -42,3 +51,14 @@ For the Armada XP: interrupts = <29>; clock-frequency = <100000>; }; + +For the Armada 7040: + + i2c at 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; -- 2.15.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v3 2/2] i2c: mv64xxx: Fix clock resource by adding an optional bus clock 2018-01-12 10:39 ` [PATCH v3 2/2] i2c: mv64xxx: Fix clock resource by adding an optional bus clock Gregory CLEMENT @ 2018-01-12 10:54 ` Maxime Ripard 2018-01-12 11:15 ` Gregory CLEMENT 0 siblings, 1 reply; 6+ messages in thread From: Maxime Ripard @ 2018-01-12 10:54 UTC (permalink / raw) To: linux-arm-kernel 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 -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 833 bytes Desc: not available URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180112/4f2142f6/attachment.sig> ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3 2/2] i2c: mv64xxx: Fix clock resource by adding an optional bus clock 2018-01-12 10:54 ` Maxime Ripard @ 2018-01-12 11:15 ` Gregory CLEMENT 2018-01-15 17:19 ` Wolfram Sang 0 siblings, 1 reply; 6+ messages in thread From: Gregory CLEMENT @ 2018-01-12 11:15 UTC (permalink / raw) To: linux-arm-kernel 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 > -- Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3 2/2] i2c: mv64xxx: Fix clock resource by adding an optional bus clock 2018-01-12 11:15 ` Gregory CLEMENT @ 2018-01-15 17:19 ` Wolfram Sang 0 siblings, 0 replies; 6+ messages in thread From: Wolfram Sang @ 2018-01-15 17:19 UTC (permalink / raw) To: linux-arm-kernel > > 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? -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 833 bytes Desc: not available URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180115/50eaf8f1/attachment.sig> ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-01-15 17:19 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-01-12 10:39 [PATCH v3 0/2] i2c: mv64xxx: Fix clock resource for Armada 7K/8K Gregory CLEMENT 2018-01-12 10:39 ` [PATCH v3 1/2] i2c: mv64xxx: Remove useless test before clk_disable_unprepare Gregory CLEMENT 2018-01-12 10:39 ` [PATCH v3 2/2] i2c: mv64xxx: Fix clock resource by adding an optional bus clock Gregory CLEMENT 2018-01-12 10:54 ` Maxime Ripard 2018-01-12 11:15 ` Gregory CLEMENT 2018-01-15 17:19 ` Wolfram Sang
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).