Even though the DW I2C controller reference clock source is requested by
the method devm_clk_get() with non-optional clock requirement the way the
clock handler is used afterwards has a pure optional clock semantic
(though in some circumstances we can get a warning about the clock missing
printed in the system console). There is no point in reimplementing that
functionality seeing the kernel clock framework already supports the
optional interface from scratch. Thus let's convert the platform driver to
using it.
Note by providing this commit we get to fix two problems. The first one
was introduced in commit c62ebb3d5f0d ("i2c: designware: Add support for
an interface clock"). It causes not having the interface clock (pclk)
enabled/disabled in case if the reference clock isn't provided. The second
problem was first introduced in commit b33af11de236 ("i2c: designware: Do
not require clock when SSCN and FFCN are provided"). Since that
modification the deferred probe procedure has been unsupported in case if
the interface clock isn't ready.
Fixes: c62ebb3d5f0d ("i2c: designware: Add support for an interface clock")
Fixes: b33af11de236 ("i2c: designware: Do not require clock when SSCN and FFCN are provided")
Signed-off-by: Serge Semin <[email protected]>
---
drivers/i2c/busses/i2c-designware-common.c | 3 ---
drivers/i2c/busses/i2c-designware-platdrv.c | 13 +++++++++++--
2 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
index 9f8574320eb2..b08e5bc2b64c 100644
--- a/drivers/i2c/busses/i2c-designware-common.c
+++ b/drivers/i2c/busses/i2c-designware-common.c
@@ -477,9 +477,6 @@ 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) {
/* Optional interface clock */
ret = clk_prepare_enable(dev->pclk);
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 70ade5306e45..ba043b547393 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -320,8 +320,17 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
goto exit_reset;
}
- dev->clk = devm_clk_get(&pdev->dev, NULL);
- if (!i2c_dw_prepare_clk(dev, true)) {
+ dev->clk = devm_clk_get_optional(&pdev->dev, NULL);
+ if (IS_ERR(dev->clk)) {
+ ret = PTR_ERR(dev->clk);
+ goto exit_reset;
+ }
+
+ ret = i2c_dw_prepare_clk(dev, true);
+ if (ret)
+ goto exit_reset;
+
+ if (dev->clk) {
u64 clk_khz;
dev->get_clk_rate_khz = i2c_dw_get_clk_rate_khz;
--
2.35.1
On Fri, Jun 10, 2022 at 10:42:33AM +0300, Serge Semin wrote:
> Even though the DW I2C controller reference clock source is requested by
> the method devm_clk_get() with non-optional clock requirement the way the
> clock handler is used afterwards has a pure optional clock semantic
> (though in some circumstances we can get a warning about the clock missing
> printed in the system console). There is no point in reimplementing that
> functionality seeing the kernel clock framework already supports the
> optional interface from scratch. Thus let's convert the platform driver to
> using it.
>
> Note by providing this commit we get to fix two problems. The first one
> was introduced in commit c62ebb3d5f0d ("i2c: designware: Add support for
> an interface clock"). It causes not having the interface clock (pclk)
> enabled/disabled in case if the reference clock isn't provided. The second
> problem was first introduced in commit b33af11de236 ("i2c: designware: Do
> not require clock when SSCN and FFCN are provided"). Since that
> modification the deferred probe procedure has been unsupported in case if
> the interface clock isn't ready.
Makes sense,
Reviewed-by: Andy Shevchenko <[email protected]>
> Fixes: c62ebb3d5f0d ("i2c: designware: Add support for an interface clock")
> Fixes: b33af11de236 ("i2c: designware: Do not require clock when SSCN and FFCN are provided")
> Signed-off-by: Serge Semin <[email protected]>
> ---
> drivers/i2c/busses/i2c-designware-common.c | 3 ---
> drivers/i2c/busses/i2c-designware-platdrv.c | 13 +++++++++++--
> 2 files changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
> index 9f8574320eb2..b08e5bc2b64c 100644
> --- a/drivers/i2c/busses/i2c-designware-common.c
> +++ b/drivers/i2c/busses/i2c-designware-common.c
> @@ -477,9 +477,6 @@ 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) {
> /* Optional interface clock */
> ret = clk_prepare_enable(dev->pclk);
> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
> index 70ade5306e45..ba043b547393 100644
> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> @@ -320,8 +320,17 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
> goto exit_reset;
> }
>
> - dev->clk = devm_clk_get(&pdev->dev, NULL);
> - if (!i2c_dw_prepare_clk(dev, true)) {
> + dev->clk = devm_clk_get_optional(&pdev->dev, NULL);
> + if (IS_ERR(dev->clk)) {
> + ret = PTR_ERR(dev->clk);
> + goto exit_reset;
> + }
> +
> + ret = i2c_dw_prepare_clk(dev, true);
> + if (ret)
> + goto exit_reset;
> +
> + if (dev->clk) {
> u64 clk_khz;
>
> dev->get_clk_rate_khz = i2c_dw_get_clk_rate_khz;
> --
> 2.35.1
>
--
With Best Regards,
Andy Shevchenko
On 6/10/22 14:19, Andy Shevchenko wrote:
> On Fri, Jun 10, 2022 at 10:42:33AM +0300, Serge Semin wrote:
>> Even though the DW I2C controller reference clock source is requested by
>> the method devm_clk_get() with non-optional clock requirement the way the
>> clock handler is used afterwards has a pure optional clock semantic
>> (though in some circumstances we can get a warning about the clock missing
>> printed in the system console). There is no point in reimplementing that
>> functionality seeing the kernel clock framework already supports the
>> optional interface from scratch. Thus let's convert the platform driver to
>> using it.
>>
>> Note by providing this commit we get to fix two problems. The first one
>> was introduced in commit c62ebb3d5f0d ("i2c: designware: Add support for
>> an interface clock"). It causes not having the interface clock (pclk)
>> enabled/disabled in case if the reference clock isn't provided. The second
>> problem was first introduced in commit b33af11de236 ("i2c: designware: Do
>> not require clock when SSCN and FFCN are provided"). Since that
>> modification the deferred probe procedure has been unsupported in case if
>> the interface clock isn't ready.
>
> Makes sense,
> Reviewed-by: Andy Shevchenko <[email protected]>
>
Acked-by: Jarkko Nikula <[email protected]>
On Fri, Jun 10, 2022 at 10:42:33AM +0300, Serge Semin wrote:
> Even though the DW I2C controller reference clock source is requested by
> the method devm_clk_get() with non-optional clock requirement the way the
> clock handler is used afterwards has a pure optional clock semantic
> (though in some circumstances we can get a warning about the clock missing
> printed in the system console). There is no point in reimplementing that
> functionality seeing the kernel clock framework already supports the
> optional interface from scratch. Thus let's convert the platform driver to
> using it.
>
> Note by providing this commit we get to fix two problems. The first one
> was introduced in commit c62ebb3d5f0d ("i2c: designware: Add support for
> an interface clock"). It causes not having the interface clock (pclk)
> enabled/disabled in case if the reference clock isn't provided. The second
> problem was first introduced in commit b33af11de236 ("i2c: designware: Do
> not require clock when SSCN and FFCN are provided"). Since that
> modification the deferred probe procedure has been unsupported in case if
> the interface clock isn't ready.
>
> Fixes: c62ebb3d5f0d ("i2c: designware: Add support for an interface clock")
> Fixes: b33af11de236 ("i2c: designware: Do not require clock when SSCN and FFCN are provided")
> Signed-off-by: Serge Semin <[email protected]>
Applied to for-current, thanks!