| Message ID | 20220212150927.39513-2-aidanmacdonald.0x0@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,1/2] dts: x1000: Fix missing TCU clock in tcu device node | expand |
Hi Aidan, Le sam., févr. 12 2022 at 15:09:28 +0000, Aidan MacDonald <aidanmacdonald.0x0@gmail.com> a écrit : > The X1000 does have a TCU clock gate, so pass it to the driver. > Without this the TCU can be gated automatically, which prevents > timers from running. > > Fixes: dc6a81c3382f74fe ("clk: Ingenic: Add support for TCU of > X1000.") > Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com> > --- > I've just realized, maybe this is an ABI break. Now that the TCU > clock is > required, the driver probe will fail if given an old device tree > which is > missing that clock. Is it necessary to add a hack of some sort to > support > the old device tree? Yes, that's a valid concern. The driver should then support the TCU clock being missing (but only for the x1000), with a comment that explain why the workaround exists. You can use of_clk_get_by_name(), and if you get -EINVAL and the workaround flag is set, allow the driver to continue. Also change the checks for (tcu->soc_info->has_tcu_clk) in the function's cleanup to checks for (tcu->clk) so that the clk_disable_unprepare/clk_put are only done on a valid pointer. Note that the x1830 also has a TCU clock that's not specified in the device tree; so you could add a patch similar to your current [1/2] that adds it to x1830.dtsi as well. It uses the "ingenic,x1000-tcu" string as fallback, so the driver wouldn't have to be modified further. Cheers, -Paul > > drivers/clk/ingenic/tcu.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/clk/ingenic/tcu.c b/drivers/clk/ingenic/tcu.c > index 77acfbeb4830..9c86043f673a 100644 > --- a/drivers/clk/ingenic/tcu.c > +++ b/drivers/clk/ingenic/tcu.c > @@ -320,7 +320,7 @@ 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, > }; > > static const struct of_device_id __maybe_unused > ingenic_tcu_of_match[] __initconst = { > -- > 2.34.1 >
diff --git a/drivers/clk/ingenic/tcu.c b/drivers/clk/ingenic/tcu.c index 77acfbeb4830..9c86043f673a 100644 --- a/drivers/clk/ingenic/tcu.c +++ b/drivers/clk/ingenic/tcu.c @@ -320,7 +320,7 @@ 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, }; static const struct of_device_id __maybe_unused ingenic_tcu_of_match[] __initconst = {
The X1000 does have a TCU clock gate, so pass it to the driver. Without this the TCU can be gated automatically, which prevents timers from running. Fixes: dc6a81c3382f74fe ("clk: Ingenic: Add support for TCU of X1000.") Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com> --- I've just realized, maybe this is an ABI break. Now that the TCU clock is required, the driver probe will fail if given an old device tree which is missing that clock. Is it necessary to add a hack of some sort to support the old device tree? drivers/clk/ingenic/tcu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)