[PATCH v2] clk: rockchip: clk-rk3588: Fix 32k clock name for pmu_24m_32k_100m_src_p

[PATCH v2] clk: rockchip: clk-rk3588: Fix 32k clock name for pmu_24m_32k_100m_src_p

[Alexander Shiyan]

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>
Reviewed-by: Dragan Simic <dsimic@manjaro.org>
Fixes: f1c506d152ff ("clk: rockchip: add clock controller for the RK3588")
---
 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" };
-- 
2.39.1

Re: [PATCH v2] clk: rockchip: clk-rk3588: Fix 32k clock name for pmu_24m_32k_100m_src_p

[Heiko Stuebner]

On Thu, 29 Aug 2024 08:28:20 +0300, 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.
> 
> 

Applied, thanks!

[1/1] clk: rockchip: clk-rk3588: Fix 32k clock name for pmu_24m_32k_100m_src_p
      commit: 0d02e8d284a45bfa8997ebe8764437b8eb6b108b

Best regards,
-- 
Heiko Stuebner <heiko@sntech.de>

[PATCH v2] clk: rockchip: clk-rk3588: Fix 32k clock name for pmu_24m_32k_100m_src_p

[Alexander Shiyan]

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(-)

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" };
-- 
2.38.2

Re: [PATCH v2] clk: rockchip: clk-rk3588: Fix 32k clock name for pmu_24m_32k_100m_src_p

[Dragan Simic]

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" };

Re: [PATCH v2] clk: rockchip: clk-rk3588: Fix 32k clock name for pmu_24m_32k_100m_src_p

[Dragan Simic]

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" };