[PATCH] clk: ralink: mtmips: fix clock plan for Ralink SoC RT3883

[PATCH] clk: ralink: mtmips: fix clock plan for Ralink SoC RT3883

[Sergio Paracuellos]

Clock plan for Ralink SoC RT3883 needs an extra 'periph' clock to properly
set some peripherals that has this clock as their parent. When this driver
was mainlined we could not find any active users of this SoC so we cannot
perform any real tests for it. Now, one user of a Belkin f9k1109 version 1
device which uses this SoC appear and reported some issues in openWRT:
- https://github.com/openwrt/openwrt/issues/16054
The peripherals that are wrong are 'uart', 'i2c', 'i2s' and 'uartlite' which
has a not defined 'periph' clock as parent. Hence, introduce it to have a
properly working clock plan for this SoC.

Fixes: 6f3b15586eef ("clk: ralink: add clock and reset driver for MTMIPS SoCs")
Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/clk/ralink/clk-mtmips.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/ralink/clk-mtmips.c b/drivers/clk/ralink/clk-mtmips.c
index 50a443bf79ec..787ff3e66b34 100644
--- a/drivers/clk/ralink/clk-mtmips.c
+++ b/drivers/clk/ralink/clk-mtmips.c
@@ -267,6 +267,11 @@ static struct mtmips_clk_fixed rt305x_fixed_clocks[] = {
 	CLK_FIXED("xtal", NULL, 40000000)
 };
 
+static struct mtmips_clk_fixed rt3383_fixed_clocks[] = {
+	CLK_FIXED("xtal", NULL, 40000000),
+	CLK_FIXED("periph", "xtal", 40000000)
+};
+
 static struct mtmips_clk_fixed rt3352_fixed_clocks[] = {
 	CLK_FIXED("periph", "xtal", 40000000)
 };
@@ -779,8 +784,8 @@ static const struct mtmips_clk_data rt3352_clk_data = {
 static const struct mtmips_clk_data rt3883_clk_data = {
 	.clk_base = rt3883_clks_base,
 	.num_clk_base = ARRAY_SIZE(rt3883_clks_base),
-	.clk_fixed = rt305x_fixed_clocks,
-	.num_clk_fixed = ARRAY_SIZE(rt305x_fixed_clocks),
+	.clk_fixed = rt3383_fixed_clocks,
+	.num_clk_fixed = ARRAY_SIZE(rt3383_fixed_clocks),
 	.clk_factor = NULL,
 	.num_clk_factor = 0,
 	.clk_periph = rt5350_pherip_clks,
-- 
2.25.1

Re: [PATCH] clk: ralink: mtmips: fix clock plan for Ralink SoC RT3883

[Sergio Paracuellos]

Hi Stephen,

On Tue, Aug 6, 2024 at 4:29 PM Sergio Paracuellos
<sergio.paracuellos@gmail.com> wrote:
>
> Clock plan for Ralink SoC RT3883 needs an extra 'periph' clock to properly
> set some peripherals that has this clock as their parent. When this driver
> was mainlined we could not find any active users of this SoC so we cannot
> perform any real tests for it. Now, one user of a Belkin f9k1109 version 1
> device which uses this SoC appear and reported some issues in openWRT:
> - https://github.com/openwrt/openwrt/issues/16054
> The peripherals that are wrong are 'uart', 'i2c', 'i2s' and 'uartlite' which
> has a not defined 'periph' clock as parent. Hence, introduce it to have a
> properly working clock plan for this SoC.
>
> Fixes: 6f3b15586eef ("clk: ralink: add clock and reset driver for MTMIPS SoCs")
> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> ---
>  drivers/clk/ralink/clk-mtmips.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clk/ralink/clk-mtmips.c b/drivers/clk/ralink/clk-mtmips.c
> index 50a443bf79ec..787ff3e66b34 100644
> --- a/drivers/clk/ralink/clk-mtmips.c
> +++ b/drivers/clk/ralink/clk-mtmips.c
> @@ -267,6 +267,11 @@ static struct mtmips_clk_fixed rt305x_fixed_clocks[] = {
>         CLK_FIXED("xtal", NULL, 40000000)
>  };
>
> +static struct mtmips_clk_fixed rt3383_fixed_clocks[] = {
> +       CLK_FIXED("xtal", NULL, 40000000),
> +       CLK_FIXED("periph", "xtal", 40000000)
> +};
> +
>  static struct mtmips_clk_fixed rt3352_fixed_clocks[] = {
>         CLK_FIXED("periph", "xtal", 40000000)
>  };
> @@ -779,8 +784,8 @@ static const struct mtmips_clk_data rt3352_clk_data = {
>  static const struct mtmips_clk_data rt3883_clk_data = {
>         .clk_base = rt3883_clks_base,
>         .num_clk_base = ARRAY_SIZE(rt3883_clks_base),
> -       .clk_fixed = rt305x_fixed_clocks,
> -       .num_clk_fixed = ARRAY_SIZE(rt305x_fixed_clocks),
> +       .clk_fixed = rt3383_fixed_clocks,
> +       .num_clk_fixed = ARRAY_SIZE(rt3383_fixed_clocks),
>         .clk_factor = NULL,
>         .num_clk_factor = 0,
>         .clk_periph = rt5350_pherip_clks,
> --
> 2.25.1
>

Gentle ping on this patch :)

Thanks,
    Sergio Paracuellos

Re: [PATCH] clk: ralink: mtmips: fix clock plan for Ralink SoC RT3883

[Shiji Yang]

On Tue,  6 Aug 2024 16:29:02 +0200, Sergio Paracuellos wrote:

>diff --git a/drivers/clk/ralink/clk-mtmips.c b/drivers/clk/ralink/clk-mtmips.c
>index 50a443bf79ec..787ff3e66b34 100644
>--- a/drivers/clk/ralink/clk-mtmips.c
>+++ b/drivers/clk/ralink/clk-mtmips.c
>@@ -267,6 +267,11 @@ static struct mtmips_clk_fixed rt305x_fixed_clocks[] = {
> 	CLK_FIXED("xtal", NULL, 40000000)
> };
> 
>+static struct mtmips_clk_fixed rt3383_fixed_clocks[] = {

Hi! I found a small issue. The SoC name is rt3883 instead of rt3383.

>+	CLK_FIXED("xtal", NULL, 40000000),
>+	CLK_FIXED("periph", "xtal", 40000000)
>+};
>+
> static struct mtmips_clk_fixed rt3352_fixed_clocks[] = {
> 	CLK_FIXED("periph", "xtal", 40000000)
> };
>@@ -779,8 +784,8 @@ static const struct mtmips_clk_data rt3352_clk_data = {
> static const struct mtmips_clk_data rt3883_clk_data = {
> 	.clk_base = rt3883_clks_base,
> 	.num_clk_base = ARRAY_SIZE(rt3883_clks_base),
>-	.clk_fixed = rt305x_fixed_clocks,
>-	.num_clk_fixed = ARRAY_SIZE(rt305x_fixed_clocks),
>+	.clk_fixed = rt3383_fixed_clocks,
>+	.num_clk_fixed = ARRAY_SIZE(rt3383_fixed_clocks),
> 	.clk_factor = NULL,
> 	.num_clk_factor = 0,
> 	.clk_periph = rt5350_pherip_clks,

Regards,
Shiji Yang

Re: [PATCH] clk: ralink: mtmips: fix clock plan for Ralink SoC RT3883

[Sergio Paracuellos]

Hi Shiji,

On Mon, Sep 9, 2024 at 1:20 PM Shiji Yang <yangshiji66@outlook.com> wrote:
>
> On Tue,  6 Aug 2024 16:29:02 +0200, Sergio Paracuellos wrote:
>
> >diff --git a/drivers/clk/ralink/clk-mtmips.c b/drivers/clk/ralink/clk-mtmips.c
> >index 50a443bf79ec..787ff3e66b34 100644
> >--- a/drivers/clk/ralink/clk-mtmips.c
> >+++ b/drivers/clk/ralink/clk-mtmips.c
> >@@ -267,6 +267,11 @@ static struct mtmips_clk_fixed rt305x_fixed_clocks[] = {
> >       CLK_FIXED("xtal", NULL, 40000000)
> > };
> >
> >+static struct mtmips_clk_fixed rt3383_fixed_clocks[] = {
>
> Hi! I found a small issue. The SoC name is rt3883 instead of rt3383.

Ouch :). True. Since I have some other pending patches of this driver
I will fix this and
send a new patchset including this fixed.

Thanks,
    Sergio Paracuellos
>
> >+      CLK_FIXED("xtal", NULL, 40000000),
> >+      CLK_FIXED("periph", "xtal", 40000000)
> >+};
> >+
> > static struct mtmips_clk_fixed rt3352_fixed_clocks[] = {
> >       CLK_FIXED("periph", "xtal", 40000000)
> > };
> >@@ -779,8 +784,8 @@ static const struct mtmips_clk_data rt3352_clk_data = {
> > static const struct mtmips_clk_data rt3883_clk_data = {
> >       .clk_base = rt3883_clks_base,
> >       .num_clk_base = ARRAY_SIZE(rt3883_clks_base),
> >-      .clk_fixed = rt305x_fixed_clocks,
> >-      .num_clk_fixed = ARRAY_SIZE(rt305x_fixed_clocks),
> >+      .clk_fixed = rt3383_fixed_clocks,
> >+      .num_clk_fixed = ARRAY_SIZE(rt3383_fixed_clocks),
> >       .clk_factor = NULL,
> >       .num_clk_factor = 0,
> >       .clk_periph = rt5350_pherip_clks,
>
> Regards,
> Shiji Yang