On Wed, 2022-04-27 at 15:38 +0200, AngeloGioacchino Del Regno wrote:
> Il 27/04/22 05:09, Rex-BC Chen ha scritto:
> > The bank offsets are not serial for all reset registers.
> > For example, there are five infra reset banks for MT8192: 0x120,
> > 0x130,
> > 0x140, 0x150 and 0x730.
> >
> > To support this,
> > - Change reg_ofs to rst_bank_ofs which is a pointer to base offsets
> > of
> > the reset register.
> > - Add a new define RST_NR_PER_BANK to define reset number for each
> > reset bank.
> >
> > Signed-off-by: Rex-BC Chen <rex-bc.chen@xxxxxxxxxxxx>
> > ---
> > drivers/clk/mediatek/clk-mt2701-eth.c | 6 ++++--
> > drivers/clk/mediatek/clk-mt2701-g3d.c | 6 ++++--
> > drivers/clk/mediatek/clk-mt2701-hif.c | 6 ++++--
> > drivers/clk/mediatek/clk-mt2701.c | 11 +++++++----
> > drivers/clk/mediatek/clk-mt2712.c | 15 +++++++++------
> > drivers/clk/mediatek/clk-mt7622-eth.c | 6 ++++--
> > drivers/clk/mediatek/clk-mt7622-hif.c | 6 ++++--
> > drivers/clk/mediatek/clk-mt7622.c | 11 +++++++----
> > drivers/clk/mediatek/clk-mt7629-eth.c | 6 ++++--
> > drivers/clk/mediatek/clk-mt7629-hif.c | 6 ++++--
> > drivers/clk/mediatek/clk-mt8135.c | 11 +++++++----
> > drivers/clk/mediatek/clk-mt8173.c | 11 +++++++----
> > drivers/clk/mediatek/clk-mt8183.c | 14 ++++++++++++--
> > drivers/clk/mediatek/reset.c | 11 ++++++-----
> > drivers/clk/mediatek/reset.h | 6 ++++--
> > 15 files changed, 87 insertions(+), 45 deletions(-)
> >
>
> ..snip..
>
> > diff --git a/drivers/clk/mediatek/clk-mt2701.c
> > b/drivers/clk/mediatek/clk-mt2701.c
> > index 70a934faa529..ebb1b9975ab0 100644
> > --- a/drivers/clk/mediatek/clk-mt2701.c
> > +++ b/drivers/clk/mediatek/clk-mt2701.c
> > @@ -735,18 +735,21 @@ static const struct mtk_fixed_factor
> > infra_fixed_divs[] = {
> > FACTOR(CLK_INFRA_CLK_13M, "clk13m", "clk26m", 1, 2),
> > };
> >
> > +static u16 infrasys_rst_ofs[] = { 0x30, 0x34, };
> > +static u16 perfcfg_rst_ofs[] = { 0x0, 0x4, };
>
> Typo: perfcfg -> pericfg ... here and in some more files :))
>
Hello Angelo,
Thanks for your review!
I will fix them.
> > +
> > static const struct mtk_clk_rst_desc clk_rst_desc[] = {
> > /* infrasys */
> > {
> > .version = MTK_RST_SIMPLE,
> > - .rst_bank_nr = 2,
> > - .reg_ofs = 0x30,
> > + .rst_bank_ofs = infrasys_rst_ofs,
> > + .rst_bank_nr = ARRAY_SIZE(infrasys_rst_ofs),
> > },
> > /* pericfg */
> > {
> > .version = MTK_RST_SIMPLE,
> > - .rst_bank_nr = 2,
> > - .reg_ofs = 0x0,
> > + .rst_bank_ofs = perfcfg_rst_ofs,
> > + .rst_bank_nr = ARRAY_SIZE(perfcfg_rst_ofs),
> > },
> > };
> >
> > diff --git a/drivers/clk/mediatek/clk-mt2712.c
> > b/drivers/clk/mediatek/clk-mt2712.c
> > index cef7c79788ec..2a9d70dd97d6 100644
> > --- a/drivers/clk/mediatek/clk-mt2712.c
> > +++ b/drivers/clk/mediatek/clk-mt2712.c
> > @@ -1258,18 +1258,21 @@ static const struct mtk_pll_data plls[] = {
> > 0, 31, 0x0300, 4, 0, 0, 0, 0x0304, 0),
> > };
> >
> > +static u16 infrasys_rst_ofs[] = { 0x30, 0x34, };
> > +static u16 perfcfg_rst_ofs[] = { 0x0, 0x4, };
> > +
> > static const struct mtk_clk_rst_desc clk_rst_desc[] = {
> > - /* infra */
> > + /* infrasys */
>
> Instead of renaming these here, if you really want this renamed, can
> you please
> do that in patch [06/15]?
>
I will remove them.
> > {
> > .version = MTK_RST_SIMPLE,
> > - .rst_bank_nr = 2,
> > - .reg_ofs = 0x30,
> > + .rst_bank_ofs = infrasys_rst_ofs,
> > + .rst_bank_nr = ARRAY_SIZE(infrasys_rst_ofs),
> > },
>
> ..snip..
>
> > diff --git a/drivers/clk/mediatek/reset.h
> > b/drivers/clk/mediatek/reset.h
> > index 91358e8cb851..83840ecf8b27 100644
> > --- a/drivers/clk/mediatek/reset.h
> > +++ b/drivers/clk/mediatek/reset.h
> > @@ -9,6 +9,8 @@
> > #include <linux/reset-controller.h>
> > #include <linux/types.h>
> >
> > +#define RST_NR_PER_BANK 32
> > +
> > /**
> > * enum mtk_reset_version - Version of MediaTek clock reset
> > controller.
> > * @MTK_RST_SIMPLE: Use the same registers for bit set and clear.
> > @@ -24,12 +26,12 @@ enum mtk_reset_version {
> > /**
> > * struct mtk_clk_rst_desc - Description of MediaTek clock reset.
> > * @version: Reset version which is defined in enum
> > mtk_reset_version.
> > - * @reg_ofs: Base offset of the reset register.
> > + * @rst_bank_ofs: Pointer to base offsets of the reset register.
>
> Instead of generically saying that this is a pointer, it would be
> more
> appropriate to say that this is a pointer to an array containing base
> offsets (etc).
>
ok, I will modify it.
BRs,
Rex
> Thanks,
> Angelo
>
> > * @rst_bank_nr: Quantity of reset bank.
> > */
> > struct mtk_clk_rst_desc {
> > u8 version;
> > - u16 reg_ofs;
> > + u16 *rst_bank_ofs;
> > u32 rst_bank_nr;
> > };
> >
