top_early_divs are registered in the CLK_OF_DECLARE_DRIVER() half of the
topckgen clk driver. Don't try to register it again in the actual probe
function. This gets rid of the "Trying to register duplicate clock ..."
warning.
Signed-off-by: Chen-Yu Tsai <[email protected]>
---
drivers/clk/mediatek/clk-mt8192.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/clk/mediatek/clk-mt8192.c b/drivers/clk/mediatek/clk-mt8192.c
index ebbd2798d9a3..e39012583675 100644
--- a/drivers/clk/mediatek/clk-mt8192.c
+++ b/drivers/clk/mediatek/clk-mt8192.c
@@ -1235,7 +1235,6 @@ static int clk_mt8192_top_probe(struct platform_device *pdev)
return PTR_ERR(base);
mtk_clk_register_fixed_clks(top_fixed_clks, ARRAY_SIZE(top_fixed_clks), top_clk_data);
- mtk_clk_register_factors(top_early_divs, ARRAY_SIZE(top_early_divs), top_clk_data);
mtk_clk_register_factors(top_divs, ARRAY_SIZE(top_divs), top_clk_data);
mtk_clk_register_muxes(top_mtk_muxes, ARRAY_SIZE(top_mtk_muxes), node, &mt8192_clk_lock,
top_clk_data);
--
2.37.3.998.g577e59143f-goog
Il 26/09/22 12:25, Chen-Yu Tsai ha scritto:
> top_early_divs are registered in the CLK_OF_DECLARE_DRIVER() half of the
> topckgen clk driver. Don't try to register it again in the actual probe
> function. This gets rid of the "Trying to register duplicate clock ..."
> warning.
>
> Signed-off-by: Chen-Yu Tsai <[email protected]>
Can't we simply remove the CLK_OF_DECLARE_DRIVER() and top_init_early entirely,
and transfer TOP_CSW_F26M_D2 to top_divs[] instead?
I get that systimer concern and we have something similar in MT8195, where the
TOP_CLK26M_D2 is registered "late".
Getting back to MT8192, TOP_CSW_F26M_D2 seems to be used only for:
1. systimer
2. SPMI MST (registered "late").
Being it a fixed factor clock, parented to another fixed clock, it doesn't
even have any ON/OFF switch, so I think it would be actually possible to go
for the proposed removal... which would further improve this cleanup.
Regards,
Angelo
>> top_early_divs are registered in the CLK_OF_DECLARE_DRIVER() half of the
>> topckgen clk driver. Don't try to register it again in the actual probe
>> function. This gets rid of the "Trying to register duplicate clock ..."
>> warning.
>>
>> Signed-off-by: Chen-Yu Tsai <[email protected]>
>
>Can't we simply remove the CLK_OF_DECLARE_DRIVER() and top_init_early entirely,
>and transfer TOP_CSW_F26M_D2 to top_divs[] instead?
>I get that systimer concern and we have something similar in MT8195, where the
>TOP_CLK26M_D2 is registered "late".
Another reason for this:
Removing the CLK_OF_DECLARE_DRIVER() is good when we want to build our driver as
kernel modules because it does not work with kernel modules.
thanks,
Miles
>
>Getting back to MT8192, TOP_CSW_F26M_D2 seems to be used only for:
>1. systimer
>2. SPMI MST (registered "late").
>
>Being it a fixed factor clock, parented to another fixed clock, it doesn't
>even have any ON/OFF switch, so I think it would be actually possible to go
>for the proposed removal... which would further improve this cleanup.
>
>Regards,
>Angelo
>
On Tue, Sep 27, 2022 at 6:39 PM AngeloGioacchino Del Regno
<[email protected]> wrote:
>
> Il 26/09/22 12:25, Chen-Yu Tsai ha scritto:
> > top_early_divs are registered in the CLK_OF_DECLARE_DRIVER() half of the
> > topckgen clk driver. Don't try to register it again in the actual probe
> > function. This gets rid of the "Trying to register duplicate clock ..."
> > warning.
> >
> > Signed-off-by: Chen-Yu Tsai <[email protected]>
>
> Can't we simply remove the CLK_OF_DECLARE_DRIVER() and top_init_early entirely,
> and transfer TOP_CSW_F26M_D2 to top_divs[] instead?
> I get that systimer concern and we have something similar in MT8195, where the
> TOP_CLK26M_D2 is registered "late".
That was what I initially wanted to do. However I asked MTK whether the
system would work fully without systimer, and apparently it is used during
suspend (presumably it is supposed to be running?), so making it not
functional was a bit concerning.
That said, I do plan to rework the systimer stuff. The /2 divider is
actually internal to the systimer block, and should not have been modeled
the way it is now. Notably, the divider is actually variable. It is
only configured and locked by the bootloader.
For this I think we have two options:
a. Move the /2 fixed factor clock into a standalone device node, like
what was done for the MT8195
b. Rework the systimer to internalize the divider, and thus moving the
systimer input clock to osc26M.
Either one is outside the scope of this series. Option a. works especially
well for MT8192, as the configurable divider was removed from the systimer
block (only for this chip).
> Getting back to MT8192, TOP_CSW_F26M_D2 seems to be used only for:
> 1. systimer
> 2. SPMI MST (registered "late").
>
> Being it a fixed factor clock, parented to another fixed clock, it doesn't
> even have any ON/OFF switch, so I think it would be actually possible to go
> for the proposed removal... which would further improve this cleanup.
As mentioned above, I do have some plans to rework the stuff, but it is
kind of beyond the scope of this series, as it changes the device tree
binding and ABI.
Regards
ChenYu
On Wed, Sep 28, 2022 at 9:55 AM Miles Chen <[email protected]> wrote:
>
> >> top_early_divs are registered in the CLK_OF_DECLARE_DRIVER() half of the
> >> topckgen clk driver. Don't try to register it again in the actual probe
> >> function. This gets rid of the "Trying to register duplicate clock ..."
> >> warning.
> >>
> >> Signed-off-by: Chen-Yu Tsai <[email protected]>
> >
> >Can't we simply remove the CLK_OF_DECLARE_DRIVER() and top_init_early entirely,
> >and transfer TOP_CSW_F26M_D2 to top_divs[] instead?
> >I get that systimer concern and we have something similar in MT8195, where the
> >TOP_CLK26M_D2 is registered "late".
>
> Another reason for this:
> Removing the CLK_OF_DECLARE_DRIVER() is good when we want to build our driver as
> kernel modules because it does not work with kernel modules.
I agree. But as I mentioned in my other reply, we need to fix the clock
user first before dropping that clock. And there's also the matter of
DT backward compatibility. So we need to do it incrementally.
ChenYu
>On Wed, Sep 28, 2022 at 9:55 AM Miles Chen <[email protected]> wrote:
>>
>> >> top_early_divs are registered in the CLK_OF_DECLARE_DRIVER() half of the
>> >> topckgen clk driver. Don't try to register it again in the actual probe
>> >> function. This gets rid of the "Trying to register duplicate clock ..."
>> >> warning.
>> >>
>> >> Signed-off-by: Chen-Yu Tsai <[email protected]>
>> >
>> >Can't we simply remove the CLK_OF_DECLARE_DRIVER() and top_init_early entirely,
>> >and transfer TOP_CSW_F26M_D2 to top_divs[] instead?
>> >I get that systimer concern and we have something similar in MT8195, where the
>> >TOP_CLK26M_D2 is registered "late".
>>
>> Another reason for this:
>> Removing the CLK_OF_DECLARE_DRIVER() is good when we want to build our driver as
>> kernel modules because it does not work with kernel modules.
>
>I agree. But as I mentioned in my other reply, we need to fix the clock
>user first before dropping that clock. And there's also the matter of
>DT backward compatibility. So we need to do it incrementally.
>
>
>ChenYu
Got it. thanks for doing this.
thanks,
Miles