| Message ID | 20241220-amlogic-clk-gxbb-32k-fixes-v1-2-baca56ecf2db@baylibre.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream, archived |
| Headers | show |
| Series | clk: amlogic: gxbb: 32k clock fixes | expand |
On 20/12/2024 11:25, Jerome Brunet wrote: > The 32k clock reference a parent 'cts_slow_oscin' with a fixme note saying > that this clock should be provided by AO controller. > > The HW probably has this clock but it does not exist at the moment in > any controller implementation. Furthermore, referencing clock by the global > name should be avoided whenever possible. > > There is no reason to keep this hack around, at least for now. > > Fixes: 14c735c8e308 ("clk: meson-gxbb: Add EE 32K Clock for CEC") > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> > --- > drivers/clk/meson/gxbb.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/drivers/clk/meson/gxbb.c b/drivers/clk/meson/gxbb.c > index 62494cf06e7d775bdb18b2242c3d45bf246bdd0e..66f299f6fef1442ead5e978c1d3750b55b800c2e 100644 > --- a/drivers/clk/meson/gxbb.c > +++ b/drivers/clk/meson/gxbb.c > @@ -1266,14 +1266,13 @@ static struct clk_regmap gxbb_cts_i958 = { > }, > }; > > +/* > + * This table skips a clock named 'cts_slow_oscin' in the documentation > + * This clock does not exist yet in this controller or the AO one > + */ > +static u32 gxbb_32k_clk_parents_val_table[] = { 0, 2, 3 }; > static const struct clk_parent_data gxbb_32k_clk_parent_data[] = { > { .fw_name = "xtal", }, > - /* > - * FIXME: This clock is provided by the ao clock controller but the > - * clock is not yet part of the binding of this controller, so string > - * name must be use to set this parent. > - */ > - { .name = "cts_slow_oscin", .index = -1 }, > { .hw = &gxbb_fclk_div3.hw }, > { .hw = &gxbb_fclk_div5.hw }, > }; > @@ -1283,6 +1282,7 @@ static struct clk_regmap gxbb_32k_clk_sel = { > .offset = HHI_32K_CLK_CNTL, > .mask = 0x3, > .shift = 16, > + .table = gxbb_32k_clk_parents_val_table, > }, > .hw.init = &(struct clk_init_data){ > .name = "32k_clk_sel", > I guess it's for using alternate 32k clk source from the GPIOAO_6/CLK_32K_IN, but AFAIK no boards never used that so we can't even test it... Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
diff --git a/drivers/clk/meson/gxbb.c b/drivers/clk/meson/gxbb.c index 62494cf06e7d775bdb18b2242c3d45bf246bdd0e..66f299f6fef1442ead5e978c1d3750b55b800c2e 100644 --- a/drivers/clk/meson/gxbb.c +++ b/drivers/clk/meson/gxbb.c @@ -1266,14 +1266,13 @@ static struct clk_regmap gxbb_cts_i958 = { }, }; +/* + * This table skips a clock named 'cts_slow_oscin' in the documentation + * This clock does not exist yet in this controller or the AO one + */ +static u32 gxbb_32k_clk_parents_val_table[] = { 0, 2, 3 }; static const struct clk_parent_data gxbb_32k_clk_parent_data[] = { { .fw_name = "xtal", }, - /* - * FIXME: This clock is provided by the ao clock controller but the - * clock is not yet part of the binding of this controller, so string - * name must be use to set this parent. - */ - { .name = "cts_slow_oscin", .index = -1 }, { .hw = &gxbb_fclk_div3.hw }, { .hw = &gxbb_fclk_div5.hw }, }; @@ -1283,6 +1282,7 @@ static struct clk_regmap gxbb_32k_clk_sel = { .offset = HHI_32K_CLK_CNTL, .mask = 0x3, .shift = 16, + .table = gxbb_32k_clk_parents_val_table, }, .hw.init = &(struct clk_init_data){ .name = "32k_clk_sel",
The 32k clock reference a parent 'cts_slow_oscin' with a fixme note saying that this clock should be provided by AO controller. The HW probably has this clock but it does not exist at the moment in any controller implementation. Furthermore, referencing clock by the global name should be avoided whenever possible. There is no reason to keep this hack around, at least for now. Fixes: 14c735c8e308 ("clk: meson-gxbb: Add EE 32K Clock for CEC") Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> --- drivers/clk/meson/gxbb.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)