Re: [PATCH v3 2/9] clk: ralink: add clock and reset driver for MTMIPS SoCs

[Sergio Paracuellos]

On Sat, Jun 17, 2023 at 7:51 AM Shiji Yang <yangshiji66@xxxxxxxxxxx> wrote:
>
> Thank you for your quick reply!
>
> >> >+      {
> >> >+              .compatible = "ralink,mt7620a-sysc",
> >> >+              .data = &mt7620_clk_data,
> >> >+      },
> >> >+      {
> >> >+              .compatible = "ralink,mt7620-sysc",
> >> >+              .data = &mt7620_clk_data,
> >> >+      },
> >> >+      {
> >>
> >> I am confused about the difference between `ralink,mt7620-sysc` and
> >> `ralink,mt7620a-sysc`. Do you mean mt7620n?
> >> https://www.mediatek.com/products/home-networking/mt7620n-a
> >
> >There is no real difference. As you can see both of them use the same
> >'mt7620_clk_data' but since both compatible strings already exist
> >somewhere I introduced both of them.
> >arch/mips/boot/dts/ralink
>
> If they are the same, perhaps `mt7620a` also need to be checked in mtmips_clk_regs_init().
> Or just remove compatible string of `mt7620a` and update dtsi files?

We are planning to properly port dts files from openWRT into the
mainline kernel and also update the current out of date files which
already exist on tree, so we will take care of this kind of details
then.

>
> >+static void __init mtmips_clk_regs_init(struct device_node *node,
> >+                                      struct mtmips_clk_priv *priv)
> >+{
> >+      u32 t;
> >+
> >+      if (!of_device_is_compatible(node, "ralink,mt7620-sysc"))
> >+              return;
> >+
> >+      /*
> >+       * When the CPU goes into sleep mode, the BUS
> >+       * clock will be too low for USB to function properly.
> >+       * Adjust the busses fractional divider to fix this
> >+       */
> >+      regmap_read(priv->sysc, SYSC_REG_CPU_SYS_CLKCFG, &t);
> >+      t &= ~(CLKCFG_FDIV_MASK | CLKCFG_FFRAC_MASK);
> >+      t |= CLKCFG_FDIV_USB_VAL | CLKCFG_FFRAC_USB_VAL;
> >+      regmap_write(priv->sysc, SYSC_REG_CPU_SYS_CLKCFG, t);
> >+}
> >+
>
> If we choose to update dts file. We can also remove the legacy
> "ralink,rt2880-reset" compatible string by the way.

Ditto.

>
> >+static const struct of_device_id mtmips_clk_of_match[] = {
> >+      { .compatible = "ralink,rt2880-reset" },
> >+      { .compatible = "ralink,rt2880-sysc" },
> >+      { .compatible = "ralink,rt3050-sysc" },
> >+      { .compatible = "ralink,rt3050-sysc" },
> >+      { .compatible = "ralink,rt3352-sysc" },
> >+      { .compatible = "ralink,rt3883-sysc" },
> >+      { .compatible = "ralink,rt5350-sysc" },
> >+      { .compatible = "ralink,mt7620a-sysc" },
> >+      { .compatible = "ralink,mt7620-sysc" },
> >+      { .compatible = "ralink,mt7628-sysc" },
> >+      { .compatible = "ralink,mt7688-sysc" },
> >+      {}
> >+};
>
> Thanks,
>     Shiji Yang

Thanks,
    Sergio Paracuellos