Re: [PATCH v5 2/2] clk: mediatek: Add drivers for MediaTek MT6735 main clock and reset drivers

[Yassine Oudjana]

On 10/10/2024 3:59 pm, AngeloGioacchino Del Regno wrote:
> Il 10/10/24 14:45, Yassine Oudjana ha scritto:
> > From: Yassine Oudjana <y.oudjana@xxxxxxxxxxxxxx>
> >
> > Add drivers for MT6735 apmixedsys, topckgen, infracfg and pericfg
> > clock and reset controllers. These provide the base clocks and resets
> > on the platform, enough to bring up all essential blocks including
> > PWRAP, MSDC and peripherals (UART, I2C, SPI).
> >
> > Signed-off-by: Yassine Oudjana <y.oudjana@xxxxxxxxxxxxxx>
> > ---
> >   MAINTAINERS                                  |   4 +
> >   drivers/clk/mediatek/Kconfig                 |   9 +
> >   drivers/clk/mediatek/Makefile                |   1 +
> >   drivers/clk/mediatek/clk-mt6735-apmixedsys.c | 138 +++++++
> >   drivers/clk/mediatek/clk-mt6735-infracfg.c   | 107 +++++
> >   drivers/clk/mediatek/clk-mt6735-pericfg.c    | 124 ++++++
> >   drivers/clk/mediatek/clk-mt6735-topckgen.c   | 394 +++++++++++++++++++
> >   7 files changed, 777 insertions(+)
> >   create mode 100644 drivers/clk/mediatek/clk-mt6735-apmixedsys.c
> >   create mode 100644 drivers/clk/mediatek/clk-mt6735-infracfg.c
> >   create mode 100644 drivers/clk/mediatek/clk-mt6735-pericfg.c
> >   create mode 100644 drivers/clk/mediatek/clk-mt6735-topckgen.c
>
> You're so, so, so, so..... *so close* to get this one upstream.
>
> Read the comments below and *please*, fix it and send a v6 *as soon as 
> you can*,
> so that we get a good chance to see this in v6.13 and so that you can 
> move on
> with upstreaming the other stuff that you got for this SoC.
>
> ..snip...
>
> > diff --git a/drivers/clk/mediatek/clk-mt6735-infracfg.c b/drivers/clk/ 
> > mediatek/clk-mt6735-infracfg.c
> > new file mode 100644
> > index 0000000000000..eceb13af3222a
> > --- /dev/null
> > +++ b/drivers/clk/mediatek/clk-mt6735-infracfg.c
> > @@ -0,0 +1,107 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) 2022 Yassine Oudjana <y.oudjana@xxxxxxxxxxxxxx>
> > + */
> > +
> > +#include <linux/clk-provider.h>
> > +#include <linux/platform_device.h>
> > +
> > +#include "clk-gate.h"
> > +#include "clk-mtk.h"
> > +
> > +#include <dt-bindings/clock/mediatek,mt6735-infracfg.h>
> > +#include <dt-bindings/reset/mediatek,mt6735-infracfg.h>
> > +
> > +#define INFRA_RST0            0x30
> > +#define INFRA_GLOBALCON_PDN0        0x40
> > +#define INFRA_PDN1            0x44
> > +#define    INFRA_PDN_STA            0x48
>
> #define<tab>INFRA_PDN_STA .... I know this is a typo, but you'll have to 
> fix it.
>
> Please replace the tabulation with a space, so that this is consistent 
> with all
> of the other definitions that you have in here.
>
> > +
> > +#define RST_NR_PER_BANK            32
> > +
>
> ..snip..
>
> > diff --git a/drivers/clk/mediatek/clk-mt6735-pericfg.c b/drivers/clk/ 
> > mediatek/clk-mt6735-pericfg.c
> > new file mode 100644
> > index 0000000000000..6f298d5538782
> > --- /dev/null
> > +++ b/drivers/clk/mediatek/clk-mt6735-pericfg.c
> > @@ -0,0 +1,124 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) 2022 Yassine Oudjana <y.oudjana@xxxxxxxxxxxxxx>
> > + */
> > +
> > +#include <linux/clk-provider.h>
> > +#include <linux/platform_device.h>
> > +
> > +#include "clk-gate.h"
> > +#include "clk-mtk.h"
> > +
> > +#include <dt-bindings/clock/mediatek,mt6735-pericfg.h>
> > +#include <dt-bindings/reset/mediatek,mt6735-pericfg.h>
> > +
> > +#define PERI_GLOBALCON_RST0        0x00
> > +#define PERI_GLOBALCON_RST1        0x04
> > +#define PERI_GLOBALCON_PDN0_SET        0x08
> > +#define PERI_GLOBALCON_PDN0_CLR        0x10
> > +#define    PERI_GLOBALCON_PDN0_STA        0x18
>
> same here
>
> > +
> > +#define RST_NR_PER_BANK            32
> After which:
>
> Reviewed-by: AngeloGioacchino Del Regno 
> <angelogioacchino.delregno@xxxxxxxxxxxxx>

Tried so hard to catch all typos but I guess some still had to make it 
through. One quick fix coming up!