Re: [PATCH v4 4/5] soc: mediatek: pwrap: add pwrap driver for MT6873/8192 SoCs

[Hsin-hsiung Wang]

Hi,

On Mon, 2020-12-21 at 10:42 +0800, Nicolas Boichat wrote:
> On Wed, Nov 18, 2020 at 8:08 PM Hsin-Hsiung Wang
> <hsin-hsiung.wang@xxxxxxxxxxxx> wrote:
> >
> > MT6873/8192 are highly integrated SoCs and use PMIC_MT6359 for
> > power management. This patch adds pwrap master driver to
> > access PMIC_MT6359.
> >
> > Signed-off-by: Hsin-Hsiung Wang <hsin-hsiung.wang@xxxxxxxxxxxx>
> > ---
> >  drivers/soc/mediatek/mtk-pmic-wrap.c | 29 +++++++++++++++++++++++++++++
> >  1 file changed, 29 insertions(+)
> >
> > diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
> > index 5678f46..d1cd050 100644
> > --- a/drivers/soc/mediatek/mtk-pmic-wrap.c
> > +++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
> > @@ -632,6 +632,17 @@ static int mt6797_regs[] = {
> >         [PWRAP_DCM_DBC_PRD] =           0x1D4,
> >  };
> >
> > +static int mt6873_regs[] = {
> > +       [PWRAP_INIT_DONE2] =            0x0,
> 
> I don't have the datasheet for the PWRAP interface, is this register
> at offset 0x0 real?
> 
> If so, I'm concerned that mt6873_regs only defines very few registers
> offsets, so the init code will do a _lot_ writes at register address
> 0x0 (e.g. for all the undefined values in this array: PWRAP_WACS0_EN,
> PWRAP_WACS1_EN, PWRAP_WACS2_EN, etc, etc.)
> 

The register for the init_done is 0x0.
However, the pwrap driver of mt6873 only checks this register in the
beginning and wouldn't check it again.
In other words, the  register is the first register we check, so I think
there is no problem for it.

> > +       [PWRAP_TIMER_EN] =              0x3E0,
> > +       [PWRAP_INT_EN] =                0x448,
> > +       [PWRAP_WACS2_CMD] =             0xC80,
> > +       [PWRAP_SWINF_2_WDATA_31_0] =    0xC84,
> > +       [PWRAP_SWINF_2_RDATA_31_0] =    0xC94,
> > +       [PWRAP_WACS2_VLDCLR] =          0xCA4,
> > +       [PWRAP_WACS2_RDATA] =           0xCA8,
> > +};
> > +
> >  static int mt7622_regs[] = {
> >         [PWRAP_MUX_SEL] =               0x0,
> >         [PWRAP_WRAP_EN] =               0x4,
> > @@ -1050,6 +1061,7 @@ enum pwrap_type {
> >         PWRAP_MT6765,
> >         PWRAP_MT6779,
> >         PWRAP_MT6797,
> > +       PWRAP_MT6873,
> >         PWRAP_MT7622,
> >         PWRAP_MT8135,
> >         PWRAP_MT8173,
> > @@ -1512,6 +1524,7 @@ static int pwrap_init_cipher(struct pmic_wrapper *wrp)
> >         case PWRAP_MT7622:
> >                 pwrap_writel(wrp, 0, PWRAP_CIPHER_EN);
> >                 break;
> > +       case PWRAP_MT6873:
> >         case PWRAP_MT8183:
> >                 break;
> >         }
> > @@ -1948,6 +1961,19 @@ static const struct pmic_wrapper_type pwrap_mt6797 = {
> >         .init_soc_specific = NULL,
> >  };
> >
> > +static const struct pmic_wrapper_type pwrap_mt6873 = {
> > +       .regs = mt6873_regs,
> > +       .type = PWRAP_MT6873,
> > +       .arb_en_all = 0x777f,
> > +       .int_en_all = BIT(4) | BIT(5),
> > +       .int1_en_all = 0,
> > +       .spi_w = PWRAP_MAN_CMD_SPI_WRITE,
> > +       .wdt_src = PWRAP_WDT_SRC_MASK_ALL,
> > +       .caps = PWRAP_CAP_ARB,
> > +       .init_reg_clock = pwrap_common_init_reg_clock,
> > +       .init_soc_specific = NULL,
> > +};
> > +
> >  static const struct pmic_wrapper_type pwrap_mt7622 = {
> >         .regs = mt7622_regs,
> >         .type = PWRAP_MT7622,
> > @@ -2026,6 +2052,9 @@ static const struct of_device_id of_pwrap_match_tbl[] = {
> >                 .compatible = "mediatek,mt6797-pwrap",
> >                 .data = &pwrap_mt6797,
> >         }, {
> > +               .compatible = "mediatek,mt6873-pwrap",
> > +               .data = &pwrap_mt6873,
> > +       }, {
> >                 .compatible = "mediatek,mt7622-pwrap",
> >                 .data = &pwrap_mt7622,
> >         }, {
> > --
> > 2.6.4
> > _______________________________________________
> > Linux-mediatek mailing list
> > Linux-mediatek@xxxxxxxxxxxxxxxxxxx
> > http://lists.infradead.org/mailman/listinfo/linux-mediatek