| Message ID | 20220412122750.279058-3-aidanmacdonald.0x0@gmail.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | Fix missing TCU clock for X1000/X1830 SoCs | expand |
Hi, Le mar., avril 12 2022 at 13:27:50 +0100, Aidan MacDonald <aidanmacdonald.0x0@gmail.com> a écrit : > The TCU clock gate on X1000 wasn't requested by the driver and could > be gated automatically later on in boot, which prevents timers from > running and breaks PWM. > > Add a workaround to support old device trees that don't specify the > "tcu" clock gate. In this case the kernel will print a warning and > attempt to continue without the clock, which is wrong, but it could > work if "clk_ignore_unused" is in the kernel arguments. > > Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com> Reviewed-by: Paul Cercueil <paul@crapouillou.net> Cheers, -Paul > --- > drivers/clk/ingenic/tcu.c | 35 +++++++++++++++++++++++++---------- > 1 file changed, 25 insertions(+), 10 deletions(-) > > diff --git a/drivers/clk/ingenic/tcu.c b/drivers/clk/ingenic/tcu.c > index 77acfbeb4830..201bf6e6b6e0 100644 > --- a/drivers/clk/ingenic/tcu.c > +++ b/drivers/clk/ingenic/tcu.c > @@ -31,6 +31,7 @@ struct ingenic_soc_info { > unsigned int num_channels; > bool has_ost; > bool has_tcu_clk; > + bool allow_missing_tcu_clk; > }; > > struct ingenic_tcu_clk_info { > @@ -320,7 +321,8 @@ static const struct ingenic_soc_info > jz4770_soc_info = { > static const struct ingenic_soc_info x1000_soc_info = { > .num_channels = 8, > .has_ost = false, /* X1000 has OST, but it not belong TCU */ > - .has_tcu_clk = false, > + .has_tcu_clk = true, > + .allow_missing_tcu_clk = true, > }; > > static const struct of_device_id __maybe_unused > ingenic_tcu_of_match[] __initconst = { > @@ -355,14 +357,27 @@ static int __init ingenic_tcu_probe(struct > device_node *np) > tcu->clk = of_clk_get_by_name(np, "tcu"); > if (IS_ERR(tcu->clk)) { > ret = PTR_ERR(tcu->clk); > - pr_crit("Cannot get TCU clock\n"); > - goto err_free_tcu; > - } > > - ret = clk_prepare_enable(tcu->clk); > - if (ret) { > - pr_crit("Unable to enable TCU clock\n"); > - goto err_put_clk; > + /* > + * Old device trees for some SoCs did not include the > + * TCU clock because this driver (incorrectly) didn't > + * use it. In this case we complain loudly and attempt > + * to continue without the clock, which might work if > + * booting with workarounds like "clk_ignore_unused". > + */ > + if (tcu->soc_info->allow_missing_tcu_clk && ret == -EINVAL) { > + pr_warn("TCU clock missing from device tree, please update your > device tree\n"); > + tcu->clk = NULL; > + } else { > + pr_crit("Cannot get TCU clock from device tree\n"); > + goto err_free_tcu; > + } > + } else { > + ret = clk_prepare_enable(tcu->clk); > + if (ret) { > + pr_crit("Unable to enable TCU clock\n"); > + goto err_put_clk; > + } > } > } > > @@ -432,10 +447,10 @@ static int __init ingenic_tcu_probe(struct > device_node *np) > clk_hw_unregister(tcu->clocks->hws[i]); > kfree(tcu->clocks); > err_clk_disable: > - if (tcu->soc_info->has_tcu_clk) > + if (tcu->clk) > clk_disable_unprepare(tcu->clk); > err_put_clk: > - if (tcu->soc_info->has_tcu_clk) > + if (tcu->clk) > clk_put(tcu->clk); > err_free_tcu: > kfree(tcu); > -- > 2.35.1 >
Quoting Aidan MacDonald (2022-04-12 05:27:50) > The TCU clock gate on X1000 wasn't requested by the driver and could > be gated automatically later on in boot, which prevents timers from > running and breaks PWM. > > Add a workaround to support old device trees that don't specify the > "tcu" clock gate. In this case the kernel will print a warning and > attempt to continue without the clock, which is wrong, but it could > work if "clk_ignore_unused" is in the kernel arguments. > > Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com> > --- Can I take just this one patch through clk tree without the dts part? dts snippets go through soc trees.
On Wed, May 18, 2022 at 01:47:32PM -0700, Stephen Boyd wrote: > Quoting Aidan MacDonald (2022-04-12 05:27:50) > > The TCU clock gate on X1000 wasn't requested by the driver and could > > be gated automatically later on in boot, which prevents timers from > > running and breaks PWM. > > > > Add a workaround to support old device trees that don't specify the > > "tcu" clock gate. In this case the kernel will print a warning and > > attempt to continue without the clock, which is wrong, but it could > > work if "clk_ignore_unused" is in the kernel arguments. > > > > Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com> > > --- > > Can I take just this one patch through clk tree without the dts part? > dts snippets go through soc trees. Yeah. The patches can go through in any order but they both need to be merged to fix the bug -- each on its own should not change any behavior. Regards, Aidan
Quoting Aidan MacDonald (2022-04-12 05:27:50) > The TCU clock gate on X1000 wasn't requested by the driver and could > be gated automatically later on in boot, which prevents timers from > running and breaks PWM. > > Add a workaround to support old device trees that don't specify the > "tcu" clock gate. In this case the kernel will print a warning and > attempt to continue without the clock, which is wrong, but it could > work if "clk_ignore_unused" is in the kernel arguments. > > Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com> > --- Applied to clk-next
diff --git a/drivers/clk/ingenic/tcu.c b/drivers/clk/ingenic/tcu.c index 77acfbeb4830..201bf6e6b6e0 100644 --- a/drivers/clk/ingenic/tcu.c +++ b/drivers/clk/ingenic/tcu.c @@ -31,6 +31,7 @@ struct ingenic_soc_info { unsigned int num_channels; bool has_ost; bool has_tcu_clk; + bool allow_missing_tcu_clk; }; struct ingenic_tcu_clk_info { @@ -320,7 +321,8 @@ static const struct ingenic_soc_info jz4770_soc_info = { static const struct ingenic_soc_info x1000_soc_info = { .num_channels = 8, .has_ost = false, /* X1000 has OST, but it not belong TCU */ - .has_tcu_clk = false, + .has_tcu_clk = true, + .allow_missing_tcu_clk = true, }; static const struct of_device_id __maybe_unused ingenic_tcu_of_match[] __initconst = { @@ -355,14 +357,27 @@ static int __init ingenic_tcu_probe(struct device_node *np) tcu->clk = of_clk_get_by_name(np, "tcu"); if (IS_ERR(tcu->clk)) { ret = PTR_ERR(tcu->clk); - pr_crit("Cannot get TCU clock\n"); - goto err_free_tcu; - } - ret = clk_prepare_enable(tcu->clk); - if (ret) { - pr_crit("Unable to enable TCU clock\n"); - goto err_put_clk; + /* + * Old device trees for some SoCs did not include the + * TCU clock because this driver (incorrectly) didn't + * use it. In this case we complain loudly and attempt + * to continue without the clock, which might work if + * booting with workarounds like "clk_ignore_unused". + */ + if (tcu->soc_info->allow_missing_tcu_clk && ret == -EINVAL) { + pr_warn("TCU clock missing from device tree, please update your device tree\n"); + tcu->clk = NULL; + } else { + pr_crit("Cannot get TCU clock from device tree\n"); + goto err_free_tcu; + } + } else { + ret = clk_prepare_enable(tcu->clk); + if (ret) { + pr_crit("Unable to enable TCU clock\n"); + goto err_put_clk; + } } } @@ -432,10 +447,10 @@ static int __init ingenic_tcu_probe(struct device_node *np) clk_hw_unregister(tcu->clocks->hws[i]); kfree(tcu->clocks); err_clk_disable: - if (tcu->soc_info->has_tcu_clk) + if (tcu->clk) clk_disable_unprepare(tcu->clk); err_put_clk: - if (tcu->soc_info->has_tcu_clk) + if (tcu->clk) clk_put(tcu->clk); err_free_tcu: kfree(tcu);
The TCU clock gate on X1000 wasn't requested by the driver and could be gated automatically later on in boot, which prevents timers from running and breaks PWM. Add a workaround to support old device trees that don't specify the "tcu" clock gate. In this case the kernel will print a warning and attempt to continue without the clock, which is wrong, but it could work if "clk_ignore_unused" is in the kernel arguments. Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com> --- drivers/clk/ingenic/tcu.c | 35 +++++++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 10 deletions(-)