| Message ID | 20240710165354.1338287-1-eagle.alexander923@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] clk: rockchip: clk-rk3588: Fix 32k clock name for pmu_24m_32k_100m_src_p | expand |
Hello Alexander, On 2024-07-10 18:53, Alexander Shiyan wrote: > The 32kHz input clock is named "xin32k" in the driver, > so the name "32k" appears to be a typo in this case. Lets fix this. > > Signed-off-by: Alexander Shiyan <eagle.alexander923@gmail.com> Makes sense to me, and it seems to be a typo inherited from the downstream code, [1] which the base RK3588 dtsi confirms, [2] as well as the RK3588 Hardware Design Guide, version 1.0. Thus, please include: Reviewed-by: Dragan Simic <dsimic@manjaro.org> I'd also suggest that this patch receives a Fixes tag, and gets submitted for inclusion into stable kernels. Thus: Fixes: f1c506d152ff ("clk: rockchip: add clock controller for the RK3588") Cc: stable@vger.kernel.org ... but you should actually submit the v2 with these tags, if you choose to agree with this suggestion. Furthermore, it seems that the board dts for Radxa ROCK 5B needs some related fixes, because it deviates from the RK3588 EVB design, but I still need to dig deeper into that and actually do some testing to confirm it. Perhaps that will apply to other similar RK3588-based boards as well. [1] https://raw.githubusercontent.com/rockchip-linux/kernel/develop-5.10/drivers/clk/rockchip/clk-rk3588.c [2] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi#n423 > --- > drivers/clk/rockchip/clk-rk3588.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/clk/rockchip/clk-rk3588.c > b/drivers/clk/rockchip/clk-rk3588.c > index b30279a96dc8..3027379f2fdd 100644 > --- a/drivers/clk/rockchip/clk-rk3588.c > +++ b/drivers/clk/rockchip/clk-rk3588.c > @@ -526,7 +526,7 @@ PNAME(pmu_200m_100m_p) = { "clk_pmu1_200m_src", > "clk_pmu1_100m_src" }; > PNAME(pmu_300m_24m_p) = { "clk_300m_src", "xin24m" }; > PNAME(pmu_400m_24m_p) = { "clk_400m_src", "xin24m" }; > PNAME(pmu_100m_50m_24m_src_p) = { "clk_pmu1_100m_src", > "clk_pmu1_50m_src", "xin24m" }; > -PNAME(pmu_24m_32k_100m_src_p) = { "xin24m", "32k", > "clk_pmu1_100m_src" }; > +PNAME(pmu_24m_32k_100m_src_p) = { "xin24m", "xin32k", > "clk_pmu1_100m_src" }; > PNAME(hclk_pmu1_root_p) = { "clk_pmu1_200m_src", > "clk_pmu1_100m_src", "clk_pmu1_50m_src", "xin24m" }; > PNAME(hclk_pmu_cm0_root_p) = { "clk_pmu1_400m_src", > "clk_pmu1_200m_src", "clk_pmu1_100m_src", "xin24m" }; > PNAME(mclk_pdm0_p) = { "clk_pmu1_300m_src", "clk_pmu1_200m_src" };
Just a brief reminder about this patch... On 2024-07-13 05:51, Dragan Simic wrote: > Hello Alexander, > > On 2024-07-10 18:53, Alexander Shiyan wrote: >> The 32kHz input clock is named "xin32k" in the driver, >> so the name "32k" appears to be a typo in this case. Lets fix this. >> >> Signed-off-by: Alexander Shiyan <eagle.alexander923@gmail.com> > > Makes sense to me, and it seems to be a typo inherited from the > downstream code, [1] which the base RK3588 dtsi confirms, [2] as > well as the RK3588 Hardware Design Guide, version 1.0. > > Thus, please include: > > Reviewed-by: Dragan Simic <dsimic@manjaro.org> > > I'd also suggest that this patch receives a Fixes tag, and gets > submitted for inclusion into stable kernels. Thus: > > Fixes: f1c506d152ff ("clk: rockchip: add clock controller for the > RK3588") > Cc: stable@vger.kernel.org > > ... but you should actually submit the v2 with these tags, if you > choose to agree with this suggestion. > > Furthermore, it seems that the board dts for Radxa ROCK 5B needs > some related fixes, because it deviates from the RK3588 EVB design, > but I still need to dig deeper into that and actually do some > testing to confirm it. Perhaps that will apply to other similar > RK3588-based boards as well. > > [1] > https://raw.githubusercontent.com/rockchip-linux/kernel/develop-5.10/drivers/clk/rockchip/clk-rk3588.c > [2] > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi#n423 > >> --- >> drivers/clk/rockchip/clk-rk3588.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/clk/rockchip/clk-rk3588.c >> b/drivers/clk/rockchip/clk-rk3588.c >> index b30279a96dc8..3027379f2fdd 100644 >> --- a/drivers/clk/rockchip/clk-rk3588.c >> +++ b/drivers/clk/rockchip/clk-rk3588.c >> @@ -526,7 +526,7 @@ PNAME(pmu_200m_100m_p) = { "clk_pmu1_200m_src", >> "clk_pmu1_100m_src" }; >> PNAME(pmu_300m_24m_p) = { "clk_300m_src", "xin24m" }; >> PNAME(pmu_400m_24m_p) = { "clk_400m_src", "xin24m" }; >> PNAME(pmu_100m_50m_24m_src_p) = { "clk_pmu1_100m_src", >> "clk_pmu1_50m_src", "xin24m" }; >> -PNAME(pmu_24m_32k_100m_src_p) = { "xin24m", "32k", >> "clk_pmu1_100m_src" }; >> +PNAME(pmu_24m_32k_100m_src_p) = { "xin24m", "xin32k", >> "clk_pmu1_100m_src" }; >> PNAME(hclk_pmu1_root_p) = { "clk_pmu1_200m_src", >> "clk_pmu1_100m_src", "clk_pmu1_50m_src", "xin24m" }; >> PNAME(hclk_pmu_cm0_root_p) = { "clk_pmu1_400m_src", >> "clk_pmu1_200m_src", "clk_pmu1_100m_src", "xin24m" }; >> PNAME(mclk_pdm0_p) = { "clk_pmu1_300m_src", "clk_pmu1_200m_src" };
diff --git a/drivers/clk/rockchip/clk-rk3588.c b/drivers/clk/rockchip/clk-rk3588.c index b30279a96dc8..3027379f2fdd 100644 --- a/drivers/clk/rockchip/clk-rk3588.c +++ b/drivers/clk/rockchip/clk-rk3588.c @@ -526,7 +526,7 @@ PNAME(pmu_200m_100m_p) = { "clk_pmu1_200m_src", "clk_pmu1_100m_src" }; PNAME(pmu_300m_24m_p) = { "clk_300m_src", "xin24m" }; PNAME(pmu_400m_24m_p) = { "clk_400m_src", "xin24m" }; PNAME(pmu_100m_50m_24m_src_p) = { "clk_pmu1_100m_src", "clk_pmu1_50m_src", "xin24m" }; -PNAME(pmu_24m_32k_100m_src_p) = { "xin24m", "32k", "clk_pmu1_100m_src" }; +PNAME(pmu_24m_32k_100m_src_p) = { "xin24m", "xin32k", "clk_pmu1_100m_src" }; PNAME(hclk_pmu1_root_p) = { "clk_pmu1_200m_src", "clk_pmu1_100m_src", "clk_pmu1_50m_src", "xin24m" }; PNAME(hclk_pmu_cm0_root_p) = { "clk_pmu1_400m_src", "clk_pmu1_200m_src", "clk_pmu1_100m_src", "xin24m" }; PNAME(mclk_pdm0_p) = { "clk_pmu1_300m_src", "clk_pmu1_200m_src" };
The 32kHz input clock is named "xin32k" in the driver, so the name "32k" appears to be a typo in this case. Lets fix this. Signed-off-by: Alexander Shiyan <eagle.alexander923@gmail.com> --- drivers/clk/rockchip/clk-rk3588.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)