Re: [PATCH v3 0/9] mips: ralink: add complete clock and reset driver for mtmips SoCs

[Thomas Bogendoerfer]

On Fri, Jun 16, 2023 at 08:12:00AM +0200, Sergio Paracuellos wrote:
> On Tue, May 23, 2023 at 11:47 AM Sergio Paracuellos
> <sergio.paracuellos@xxxxxxxxx> wrote:
> >
> > On Thu, May 4, 2023 at 4:48 PM Sergio Paracuellos
> > <sergio.paracuellos@xxxxxxxxx> wrote:
> > >
> > > On Tue, Apr 18, 2023 at 11:03 AM Sergio Paracuellos
> > > <sergio.paracuellos@xxxxxxxxx> wrote:
> > > >
> > > > Hi all!
> > > >
> > > > This patchset is a big effort to properly implement a clock and reset
> > > > driver for old ralink SoCs. This allow to properly define clocks in
> > > > device tree and avoid to use fixed-clocks directly from 'arch/mips/ralink'
> > > > architecture directory code.
> > > >
> > > > Device tree 'sysc' node will be both clock and reset provider using
> > > > 'clock-cells' and 'reset-cells' properties.
> > > >
> > > > The ralink SoCs we are taking about are RT2880, RT3050, RT3052, RT3350,
> > > > RT3352, RT3883, RT5350, MT7620, MT7628 and MT7688. Mostly the code in
> > > > this new driver has been extracted from 'arch/mips/ralink' and cleanly
> > > > put using kernel clock and reset driver APIs. The clock plans for this
> > > > SoCs only talks about relation between CPU frequency and BUS frequency.
> > > > This relation is different depending on the particular SoC. CPU clock is
> > > > derived from XTAL frequencies.
> > > >
> > > >  Depending on the SoC we have the following frequencies:
> > > >  * RT2880 SoC:
> > > >      - XTAL: 40 MHz.
> > > >      - CPU: 250, 266, 280 or 300 MHz.
> > > >      - BUS: CPU / 2 MHz.
> > > >   * RT3050, RT3052, RT3350:
> > > >      - XTAL: 40 MHz.
> > > >      - CPU: 320 or 384 MHz.
> > > >      - BUS: CPU / 3 MHz.
> > > >   * RT3352:
> > > >      - XTAL: 40 MHz.
> > > >      - CPU: 384 or 400 MHz.
> > > >      - BUS: CPU / 3 MHz.
> > > >      - PERIPH: 40 MHz.
> > > >   * RT3383:
> > > >      - XTAL: 40 MHz.
> > > >      - CPU: 250, 384, 480 or 500 MHz.
> > > >      - BUS: Depends on RAM Type and CPU:
> > > >        + RAM DDR2: 125. ELSE 83 MHz.
> > > >        + RAM DDR2: 128. ELSE 96 MHz.
> > > >        + RAM DDR2: 160. ELSE 120 MHz.
> > > >        + RAM DDR2: 166. ELSE 125 MHz.
> > > >   * RT5350:
> > > >       - XTAL: 40 MHz.
> > > >       - CPU: 300, 320 or 360 MHz.
> > > >       - BUS: CPU / 3, CPU / 4, CPU / 3 MHz.
> > > >       - PERIPH: 40 MHz.
> > > >   * MT7628 and MT7688:
> > > >      - XTAL: 20 MHz or 40 MHz.
> > > >      - CPU: 575 or 580 MHz.
> > > >      - BUS: CPU / 3.
> > > >      - PCMI2S: 480 MHz.
> > > >      - PERIPH: 40 MHz.
> > > >   * MT7620:
> > > >      - XTAL: 20 MHz or 40 MHz.
> > > >      - PLL: XTAL, 480, 600 MHz.
> > > >      - CPU: depends on PLL and some mult and dividers.
> > > >      - BUS: depends on PLL and some mult and dividers.
> > > >      - PERIPH: 40 or XTAL MHz.
> > > >
> > > > MT7620 is a bit more complex deriving CPU clock from a PLL and an bunch of
> > > > register reads and predividers. To derive CPU and BUS frequencies in the
> > > > MT7620 SoC 'mt7620_calc_rate()' helper is used.
> > > > In the case XTAL can have different frequencies and we need a different
> > > > clock frequency for peripherals 'periph' clock in introduced.
> > > > The rest of the peripherals present in the SoC just follow their parent
> > > > frequencies.
> > > >
> > > > I am using 'mtmips' inside for ralink clock driver. This is aligned with
> > > > pinctrl series recently merged through pinctrl git tree [0].
> > > >
> > > > I am maintaining ralink as prefix for compatible strings after discussions
> > > > between Rob and Arinc in v2 of this series [1].
> > > >
> > > > Changes have been compile tested for:
> > > > - RT2880
> > > > - RT3883
> > > > - MT7620
> > > >
> > > > Changes have been properly tested in RT5350 SoC based board (ALL5003 board)
> > > > resulting in a working platform.
> > > >
> > > > Dts files for these SoCs in-tree except MT7621 are incomplete. We are
> > > > planning to align with openWRT files at some point and add extra needed
> > > > changes. Hence I am not touching them at all in these series. If this is
> > > > a problem, please let me know and I will update them.
> > > >
> > > > Talking about merging this series I'd like all of the patches going through
> > > > the MIPS tree if possible.
> > > >
> > > > Thanks in advance for your time.
> > > >
> > > > Best regards,
> > > >     Sergio Paracuellos
> > > >
> > > > Changes in v3:
> > > > - Address Stephen comments in v2:
> > > >     + Drop unsused include '<linux/clk.h>'.
> > > >     + Add fixed and factor clocks when it makes sense.
> > > >     + Make 'mtmips_periph_clk_ops' named variable.
> > > >     + WARN_ON -> WARN_ON_ONCE.
> > > >     + Avoid CONFIG_USB dependent code. Introduce new 'mtmips_clk_regs_init'.
> > > >     + Don't validate the bindings in the driver.
> > > >     + Make const 'struct clk_init_data' used inside macros.
> > > >     + do_div -> div_u64.
> > > >     + Make use of dev_err_probe.
> > >
> > > Hi Stephen,
> > >
> > > Does anything else need to be addressed to have all of these added?
> > >
> > > Philipp, can you please review the reset related code in PATCH 2/9 of
> > > these series?
> > >
> > > Thanks in advance for your time!
> > >
> > > Best regards,
> > >     Sergio Paracuellos
> >
> > Gentle ping for this series :-)
> 
> Another gentle ping for this series :-)

I'm reday to take it through the MIPS tree, if Stephen acks the
clk driver changes.

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]