diff mbox series

[v4,2/5] soc: mediatek: pwrap: add arbiter capability

Message ID 1605700894-32699-3-git-send-email-hsin-hsiung.wang@mediatek.com
State New
Headers show
Series Add PMIC wrapper support for Mediatek MT6873/8192 SoC IC | expand

Commit Message

Hsin-Hsiung Wang Nov. 18, 2020, 12:01 p.m. UTC 
Add arbiter capability for pwrap driver.
The arbiter capability uses new design to judge the priority and latency
for multi-channel.
This patch is preparing for adding mt6873/8192 pwrap support.

Signed-off-by: Hsin-Hsiung Wang <hsin-hsiung.wang@mediatek.com>
---
 drivers/soc/mediatek/mtk-pmic-wrap.c | 57 ++++++++++++++++++++++++++++++------
 1 file changed, 48 insertions(+), 9 deletions(-)

Comments

Nicolas Boichat Dec. 21, 2020, 2:33 a.m. UTC | #1 
On Wed, Nov 18, 2020 at 8:08 PM Hsin-Hsiung Wang
<hsin-hsiung.wang@mediatek.com> wrote:
>

> Add arbiter capability for pwrap driver.

> The arbiter capability uses new design to judge the priority and latency

> for multi-channel.

> This patch is preparing for adding mt6873/8192 pwrap support.

>

> Signed-off-by: Hsin-Hsiung Wang <hsin-hsiung.wang@mediatek.com>

> ---

>  drivers/soc/mediatek/mtk-pmic-wrap.c | 57 ++++++++++++++++++++++++++++++------

>  1 file changed, 48 insertions(+), 9 deletions(-)

>

> diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c

> index c897205..5678f46 100644

> --- a/drivers/soc/mediatek/mtk-pmic-wrap.c

> +++ b/drivers/soc/mediatek/mtk-pmic-wrap.c

> @@ -25,10 +25,12 @@

>

>  /* macro for wrapper status */

>  #define PWRAP_GET_WACS_RDATA(x)                (((x) >> 0) & 0x0000ffff)

> +#define PWRAP_GET_WACS_ARB_FSM(x)      (((x) >> 1) & 0x00000007)

>  #define PWRAP_GET_WACS_FSM(x)          (((x) >> 16) & 0x00000007)

>  #define PWRAP_GET_WACS_REQ(x)          (((x) >> 19) & 0x00000001)

>  #define PWRAP_STATE_SYNC_IDLE0         BIT(20)

>  #define PWRAP_STATE_INIT_DONE0         BIT(21)

> +#define PWRAP_STATE_INIT_DONE1         BIT(15)

>

>  /* macro for WACS FSM */

>  #define PWRAP_WACS_FSM_IDLE            0x00

> @@ -74,6 +76,7 @@

>  #define PWRAP_CAP_DCM          BIT(2)

>  #define PWRAP_CAP_INT1_EN      BIT(3)

>  #define PWRAP_CAP_WDT_SRC1     BIT(4)

> +#define PWRAP_CAP_ARB          BIT(5)

>

>  /* defines for slave device wrapper registers */

>  enum dew_regs {

> @@ -340,6 +343,8 @@ enum pwrap_regs {

>         PWRAP_DCM_DBC_PRD,

>         PWRAP_EINT_STA0_ADR,

>         PWRAP_EINT_STA1_ADR,

> +       PWRAP_SWINF_2_WDATA_31_0,

> +       PWRAP_SWINF_2_RDATA_31_0,

>

>         /* MT2701 only regs */

>         PWRAP_ADC_CMD_ADDR,

> @@ -1108,14 +1113,22 @@ static void pwrap_writel(struct pmic_wrapper *wrp, u32 val, enum pwrap_regs reg)

>

>  static bool pwrap_is_fsm_idle(struct pmic_wrapper *wrp)

>  {

> -       u32 val = pwrap_readl(wrp, PWRAP_WACS2_RDATA);

> +       u32 val;

> +

> +       val = pwrap_readl(wrp, PWRAP_WACS2_RDATA);

> +       if (HAS_CAP(wrp->master->caps, PWRAP_CAP_ARB))

> +               return PWRAP_GET_WACS_ARB_FSM(val) == PWRAP_WACS_FSM_IDLE;

>

>         return PWRAP_GET_WACS_FSM(val) == PWRAP_WACS_FSM_IDLE;

>  }

>

>  static bool pwrap_is_fsm_vldclr(struct pmic_wrapper *wrp)

>  {

> -       u32 val = pwrap_readl(wrp, PWRAP_WACS2_RDATA);

> +       u32 val;

> +

> +       val = pwrap_readl(wrp, PWRAP_WACS2_RDATA);

> +       if (HAS_CAP(wrp->master->caps, PWRAP_CAP_ARB))

> +               return PWRAP_GET_WACS_ARB_FSM(val) == PWRAP_WACS_FSM_WFVLDCLR;


This code is now copied twice. Do you think it'd be better to create a
new function?

static u32 pwrap_get_fsm_state(struct pmic_wrapper *wrp) {
   if (HAS_CAP(wrp->master->caps, PWRAP_CAP_ARB))
      return PWRAP_GET_WACS_ARB_FSM(val);
   else
      return PWRAP_GET_WACS_FSM(val);
}

>

>         return PWRAP_GET_WACS_FSM(val) == PWRAP_WACS_FSM_WFVLDCLR;

>  }

> @@ -1165,6 +1178,7 @@ static int pwrap_wait_for_state(struct pmic_wrapper *wrp,

>  static int pwrap_read16(struct pmic_wrapper *wrp, u32 adr, u32 *rdata)

>  {

>         int ret;

> +       u32 val;

>

>         ret = pwrap_wait_for_state(wrp, pwrap_is_fsm_idle);

>         if (ret) {

> @@ -1172,13 +1186,21 @@ static int pwrap_read16(struct pmic_wrapper *wrp, u32 adr, u32 *rdata)

>                 return ret;

>         }

>

> -       pwrap_writel(wrp, (adr >> 1) << 16, PWRAP_WACS2_CMD);

> +       if (HAS_CAP(wrp->master->caps, PWRAP_CAP_ARB))

> +               val = adr;

> +       else

> +               val = (adr >> 1) << 16;

> +       pwrap_writel(wrp, val, PWRAP_WACS2_CMD);

>

>         ret = pwrap_wait_for_state(wrp, pwrap_is_fsm_vldclr);

>         if (ret)

>                 return ret;

>

> -       *rdata = PWRAP_GET_WACS_RDATA(pwrap_readl(wrp, PWRAP_WACS2_RDATA));

> +       if (HAS_CAP(wrp->master->caps, PWRAP_CAP_ARB))

> +               val = pwrap_readl(wrp, PWRAP_SWINF_2_RDATA_31_0);

> +       else

> +               val = pwrap_readl(wrp, PWRAP_WACS2_RDATA);

> +       *rdata = PWRAP_GET_WACS_RDATA(val);

>

>         pwrap_writel(wrp, 1, PWRAP_WACS2_VLDCLR);

>

> @@ -1228,8 +1250,13 @@ static int pwrap_write16(struct pmic_wrapper *wrp, u32 adr, u32 wdata)

>                 return ret;

>         }

>

> -       pwrap_writel(wrp, (1 << 31) | ((adr >> 1) << 16) | wdata,

> -                    PWRAP_WACS2_CMD);

> +       if (HAS_CAP(wrp->master->caps, PWRAP_CAP_ARB)) {

> +               pwrap_writel(wrp, wdata, PWRAP_SWINF_2_WDATA_31_0);

> +               pwrap_writel(wrp, BIT(29) | adr, PWRAP_WACS2_CMD);

> +       } else {

> +               pwrap_writel(wrp, BIT(31) | ((adr >> 1) << 16) | wdata,

> +                            PWRAP_WACS2_CMD);

> +       }

>

>         return 0;

>  }

> @@ -2022,6 +2049,7 @@ MODULE_DEVICE_TABLE(of, of_pwrap_match_tbl);

>  static int pwrap_probe(struct platform_device *pdev)

>  {

>         int ret, irq;

> +       u32 mask_done;

>         struct pmic_wrapper *wrp;

>         struct device_node *np = pdev->dev.of_node;

>         const struct of_device_id *of_slave_id = NULL;

> @@ -2116,14 +2144,21 @@ static int pwrap_probe(struct platform_device *pdev)

>                 }

>         }

>

> -       if (!(pwrap_readl(wrp, PWRAP_WACS2_RDATA) & PWRAP_STATE_INIT_DONE0)) {

> +       if (HAS_CAP(wrp->master->caps, PWRAP_CAP_ARB))

> +               mask_done = PWRAP_STATE_INIT_DONE1;

> +       else

> +               mask_done = PWRAP_STATE_INIT_DONE0;

> +

> +       if (!(pwrap_readl(wrp, PWRAP_WACS2_RDATA) & mask_done)) {

>                 dev_dbg(wrp->dev, "initialization isn't finished\n");

>                 ret = -ENODEV;

>                 goto err_out2;

>         }

>

>         /* Initialize watchdog, may not be done by the bootloader */

> -       pwrap_writel(wrp, 0xf, PWRAP_WDT_UNIT);

> +       if (!HAS_CAP(wrp->master->caps, PWRAP_CAP_ARB))

> +               pwrap_writel(wrp, 0xf, PWRAP_WDT_UNIT);

> +


To expand on Matthias' question on v3
(https://patchwork.kernel.org/project/linux-mediatek/patch/1600686235-27979-3-git-send-email-hsin-hsiung.wang@mediatek.com/):
is there any PWRAP implementation where a design with an arbiter is
still able to control the watchdog?

If not, at the very least, it'd be good to expand the comment above
(e.g. "designs with arbiter support cannot change the watchdog
timer").

>         /*

>          * Since STAUPD was not used on mt8173 platform,

>          * so STAUPD of WDT_SRC which should be turned off

> @@ -2132,7 +2167,11 @@ static int pwrap_probe(struct platform_device *pdev)

>         if (HAS_CAP(wrp->master->caps, PWRAP_CAP_WDT_SRC1))

>                 pwrap_writel(wrp, wrp->master->wdt_src, PWRAP_WDT_SRC_EN_1);

>

> -       pwrap_writel(wrp, 0x1, PWRAP_TIMER_EN);

> +       if (!HAS_CAP(wrp->master->caps, PWRAP_CAP_ARB))


Please invert this if test:

if (HAS_CAP(wrp->master->caps, PWRAP_CAP_ARB))
   ... 0x3 ...
else
   ... 0x1 ...

> +               pwrap_writel(wrp, 0x1, PWRAP_TIMER_EN);

> +       else

> +               pwrap_writel(wrp, 0x3, PWRAP_TIMER_EN);

> +

>         pwrap_writel(wrp, wrp->master->int_en_all, PWRAP_INT_EN);

>         /*

>          * We add INT1 interrupt to handle starvation and request exception

> --

> 2.6.4

> _______________________________________________

> Linux-mediatek mailing list

> Linux-mediatek@lists.infradead.org

> http://lists.infradead.org/mailman/listinfo/linux-mediatek
Matthias Brugger Jan. 31, 2021, 1:15 p.m. UTC | #2 
On 21/12/2020 03:33, Nicolas Boichat wrote:
> On Wed, Nov 18, 2020 at 8:08 PM Hsin-Hsiung Wang

> <hsin-hsiung.wang@mediatek.com> wrote:

>>

>> Add arbiter capability for pwrap driver.

>> The arbiter capability uses new design to judge the priority and latency

>> for multi-channel.

>> This patch is preparing for adding mt6873/8192 pwrap support.

>>

>> Signed-off-by: Hsin-Hsiung Wang <hsin-hsiung.wang@mediatek.com>

>> ---

>>  drivers/soc/mediatek/mtk-pmic-wrap.c | 57 ++++++++++++++++++++++++++++++------

>>  1 file changed, 48 insertions(+), 9 deletions(-)

>>

>> diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c

>> index c897205..5678f46 100644

>> --- a/drivers/soc/mediatek/mtk-pmic-wrap.c

>> +++ b/drivers/soc/mediatek/mtk-pmic-wrap.c

>> @@ -25,10 +25,12 @@

>>

>>  /* macro for wrapper status */

>>  #define PWRAP_GET_WACS_RDATA(x)                (((x) >> 0) & 0x0000ffff)

>> +#define PWRAP_GET_WACS_ARB_FSM(x)      (((x) >> 1) & 0x00000007)

>>  #define PWRAP_GET_WACS_FSM(x)          (((x) >> 16) & 0x00000007)

>>  #define PWRAP_GET_WACS_REQ(x)          (((x) >> 19) & 0x00000001)

>>  #define PWRAP_STATE_SYNC_IDLE0         BIT(20)

>>  #define PWRAP_STATE_INIT_DONE0         BIT(21)

>> +#define PWRAP_STATE_INIT_DONE1         BIT(15)

>>

>>  /* macro for WACS FSM */

>>  #define PWRAP_WACS_FSM_IDLE            0x00

>> @@ -74,6 +76,7 @@

>>  #define PWRAP_CAP_DCM          BIT(2)

>>  #define PWRAP_CAP_INT1_EN      BIT(3)

>>  #define PWRAP_CAP_WDT_SRC1     BIT(4)

>> +#define PWRAP_CAP_ARB          BIT(5)

>>

>>  /* defines for slave device wrapper registers */

>>  enum dew_regs {

>> @@ -340,6 +343,8 @@ enum pwrap_regs {

>>         PWRAP_DCM_DBC_PRD,

>>         PWRAP_EINT_STA0_ADR,

>>         PWRAP_EINT_STA1_ADR,

>> +       PWRAP_SWINF_2_WDATA_31_0,

>> +       PWRAP_SWINF_2_RDATA_31_0,

>>

>>         /* MT2701 only regs */

>>         PWRAP_ADC_CMD_ADDR,

>> @@ -1108,14 +1113,22 @@ static void pwrap_writel(struct pmic_wrapper *wrp, u32 val, enum pwrap_regs reg)

>>

>>  static bool pwrap_is_fsm_idle(struct pmic_wrapper *wrp)

>>  {

>> -       u32 val = pwrap_readl(wrp, PWRAP_WACS2_RDATA);

>> +       u32 val;

>> +

>> +       val = pwrap_readl(wrp, PWRAP_WACS2_RDATA);

>> +       if (HAS_CAP(wrp->master->caps, PWRAP_CAP_ARB))

>> +               return PWRAP_GET_WACS_ARB_FSM(val) == PWRAP_WACS_FSM_IDLE;

>>

>>         return PWRAP_GET_WACS_FSM(val) == PWRAP_WACS_FSM_IDLE;

>>  }

>>

>>  static bool pwrap_is_fsm_vldclr(struct pmic_wrapper *wrp)

>>  {

>> -       u32 val = pwrap_readl(wrp, PWRAP_WACS2_RDATA);

>> +       u32 val;

>> +

>> +       val = pwrap_readl(wrp, PWRAP_WACS2_RDATA);

>> +       if (HAS_CAP(wrp->master->caps, PWRAP_CAP_ARB))

>> +               return PWRAP_GET_WACS_ARB_FSM(val) == PWRAP_WACS_FSM_WFVLDCLR;

> 

> This code is now copied twice. Do you think it'd be better to create a

> new function?

> 

> static u32 pwrap_get_fsm_state(struct pmic_wrapper *wrp) {

>    if (HAS_CAP(wrp->master->caps, PWRAP_CAP_ARB))

>       return PWRAP_GET_WACS_ARB_FSM(val);

>    else

>       return PWRAP_GET_WACS_FSM(val);

> }

> 

>>

>>         return PWRAP_GET_WACS_FSM(val) == PWRAP_WACS_FSM_WFVLDCLR;

>>  }

>> @@ -1165,6 +1178,7 @@ static int pwrap_wait_for_state(struct pmic_wrapper *wrp,

>>  static int pwrap_read16(struct pmic_wrapper *wrp, u32 adr, u32 *rdata)

>>  {

>>         int ret;

>> +       u32 val;

>>

>>         ret = pwrap_wait_for_state(wrp, pwrap_is_fsm_idle);

>>         if (ret) {

>> @@ -1172,13 +1186,21 @@ static int pwrap_read16(struct pmic_wrapper *wrp, u32 adr, u32 *rdata)

>>                 return ret;

>>         }

>>

>> -       pwrap_writel(wrp, (adr >> 1) << 16, PWRAP_WACS2_CMD);

>> +       if (HAS_CAP(wrp->master->caps, PWRAP_CAP_ARB))

>> +               val = adr;

>> +       else

>> +               val = (adr >> 1) << 16;

>> +       pwrap_writel(wrp, val, PWRAP_WACS2_CMD);

>>

>>         ret = pwrap_wait_for_state(wrp, pwrap_is_fsm_vldclr);

>>         if (ret)

>>                 return ret;

>>

>> -       *rdata = PWRAP_GET_WACS_RDATA(pwrap_readl(wrp, PWRAP_WACS2_RDATA));

>> +       if (HAS_CAP(wrp->master->caps, PWRAP_CAP_ARB))

>> +               val = pwrap_readl(wrp, PWRAP_SWINF_2_RDATA_31_0);

>> +       else

>> +               val = pwrap_readl(wrp, PWRAP_WACS2_RDATA);

>> +       *rdata = PWRAP_GET_WACS_RDATA(val);

>>

>>         pwrap_writel(wrp, 1, PWRAP_WACS2_VLDCLR);

>>

>> @@ -1228,8 +1250,13 @@ static int pwrap_write16(struct pmic_wrapper *wrp, u32 adr, u32 wdata)

>>                 return ret;

>>         }

>>

>> -       pwrap_writel(wrp, (1 << 31) | ((adr >> 1) << 16) | wdata,

>> -                    PWRAP_WACS2_CMD);

>> +       if (HAS_CAP(wrp->master->caps, PWRAP_CAP_ARB)) {

>> +               pwrap_writel(wrp, wdata, PWRAP_SWINF_2_WDATA_31_0);

>> +               pwrap_writel(wrp, BIT(29) | adr, PWRAP_WACS2_CMD);

>> +       } else {

>> +               pwrap_writel(wrp, BIT(31) | ((adr >> 1) << 16) | wdata,

>> +                            PWRAP_WACS2_CMD);

>> +       }

>>

>>         return 0;

>>  }

>> @@ -2022,6 +2049,7 @@ MODULE_DEVICE_TABLE(of, of_pwrap_match_tbl);

>>  static int pwrap_probe(struct platform_device *pdev)

>>  {

>>         int ret, irq;

>> +       u32 mask_done;

>>         struct pmic_wrapper *wrp;

>>         struct device_node *np = pdev->dev.of_node;

>>         const struct of_device_id *of_slave_id = NULL;

>> @@ -2116,14 +2144,21 @@ static int pwrap_probe(struct platform_device *pdev)

>>                 }

>>         }

>>

>> -       if (!(pwrap_readl(wrp, PWRAP_WACS2_RDATA) & PWRAP_STATE_INIT_DONE0)) {

>> +       if (HAS_CAP(wrp->master->caps, PWRAP_CAP_ARB))

>> +               mask_done = PWRAP_STATE_INIT_DONE1;

>> +       else

>> +               mask_done = PWRAP_STATE_INIT_DONE0;

>> +

>> +       if (!(pwrap_readl(wrp, PWRAP_WACS2_RDATA) & mask_done)) {

>>                 dev_dbg(wrp->dev, "initialization isn't finished\n");

>>                 ret = -ENODEV;

>>                 goto err_out2;

>>         }

>>

>>         /* Initialize watchdog, may not be done by the bootloader */

>> -       pwrap_writel(wrp, 0xf, PWRAP_WDT_UNIT);

>> +       if (!HAS_CAP(wrp->master->caps, PWRAP_CAP_ARB))

>> +               pwrap_writel(wrp, 0xf, PWRAP_WDT_UNIT);

>> +

> 

> To expand on Matthias' question on v3

> (https://patchwork.kernel.org/project/linux-mediatek/patch/1600686235-27979-3-git-send-email-hsin-hsiung.wang@mediatek.com/):

> is there any PWRAP implementation where a design with an arbiter is

> still able to control the watchdog?

> 

> If not, at the very least, it'd be good to expand the comment above

> (e.g. "designs with arbiter support cannot change the watchdog

> timer").

> 

>>         /*

>>          * Since STAUPD was not used on mt8173 platform,

>>          * so STAUPD of WDT_SRC which should be turned off

>> @@ -2132,7 +2167,11 @@ static int pwrap_probe(struct platform_device *pdev)

>>         if (HAS_CAP(wrp->master->caps, PWRAP_CAP_WDT_SRC1))

>>                 pwrap_writel(wrp, wrp->master->wdt_src, PWRAP_WDT_SRC_EN_1);

>>

>> -       pwrap_writel(wrp, 0x1, PWRAP_TIMER_EN);

>> +       if (!HAS_CAP(wrp->master->caps, PWRAP_CAP_ARB))

> 

> Please invert this if test:

> 

> if (HAS_CAP(wrp->master->caps, PWRAP_CAP_ARB))

>    ... 0x3 ...

> else

>    ... 0x1 ...

> 

>> +               pwrap_writel(wrp, 0x1, PWRAP_TIMER_EN);

>> +       else

>> +               pwrap_writel(wrp, 0x3, PWRAP_TIMER_EN);

>> +


Thanks for the review Nicolas, I think with these two comments addressed the
patch is ready for mainline. Given of course that 4/5 isn't a hack with all the
registers being not defined.

Regards,
Matthias

>>         pwrap_writel(wrp, wrp->master->int_en_all, PWRAP_INT_EN);

>>         /*

>>          * We add INT1 interrupt to handle starvation and request exception

>> --

>> 2.6.4

>> _______________________________________________

>> Linux-mediatek mailing list

>> Linux-mediatek@lists.infradead.org

>> http://lists.infradead.org/mailman/listinfo/linux-mediatek
Hsin-Hsiung Wang Feb. 4, 2021, 1:04 p.m. UTC | #3 
Hi,

On Mon, 2020-12-21 at 10:33 +0800, Nicolas Boichat wrote:
> On Wed, Nov 18, 2020 at 8:08 PM Hsin-Hsiung Wang

> <hsin-hsiung.wang@mediatek.com> wrote:

> >

> > Add arbiter capability for pwrap driver.

> > The arbiter capability uses new design to judge the priority and latency

> > for multi-channel.

> > This patch is preparing for adding mt6873/8192 pwrap support.

> >

> > Signed-off-by: Hsin-Hsiung Wang <hsin-hsiung.wang@mediatek.com>

> > ---

> >  drivers/soc/mediatek/mtk-pmic-wrap.c | 57 ++++++++++++++++++++++++++++++------

> >  1 file changed, 48 insertions(+), 9 deletions(-)

> >

> > diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c

> > index c897205..5678f46 100644

> > --- a/drivers/soc/mediatek/mtk-pmic-wrap.c

> > +++ b/drivers/soc/mediatek/mtk-pmic-wrap.c

> > @@ -25,10 +25,12 @@

> >

> >  /* macro for wrapper status */

> >  #define PWRAP_GET_WACS_RDATA(x)                (((x) >> 0) & 0x0000ffff)

> > +#define PWRAP_GET_WACS_ARB_FSM(x)      (((x) >> 1) & 0x00000007)

> >  #define PWRAP_GET_WACS_FSM(x)          (((x) >> 16) & 0x00000007)

> >  #define PWRAP_GET_WACS_REQ(x)          (((x) >> 19) & 0x00000001)

> >  #define PWRAP_STATE_SYNC_IDLE0         BIT(20)

> >  #define PWRAP_STATE_INIT_DONE0         BIT(21)

> > +#define PWRAP_STATE_INIT_DONE1         BIT(15)

> >

> >  /* macro for WACS FSM */

> >  #define PWRAP_WACS_FSM_IDLE            0x00

> > @@ -74,6 +76,7 @@

> >  #define PWRAP_CAP_DCM          BIT(2)

> >  #define PWRAP_CAP_INT1_EN      BIT(3)

> >  #define PWRAP_CAP_WDT_SRC1     BIT(4)

> > +#define PWRAP_CAP_ARB          BIT(5)

> >

> >  /* defines for slave device wrapper registers */

> >  enum dew_regs {

> > @@ -340,6 +343,8 @@ enum pwrap_regs {

> >         PWRAP_DCM_DBC_PRD,

> >         PWRAP_EINT_STA0_ADR,

> >         PWRAP_EINT_STA1_ADR,

> > +       PWRAP_SWINF_2_WDATA_31_0,

> > +       PWRAP_SWINF_2_RDATA_31_0,

> >

> >         /* MT2701 only regs */

> >         PWRAP_ADC_CMD_ADDR,

> > @@ -1108,14 +1113,22 @@ static void pwrap_writel(struct pmic_wrapper *wrp, u32 val, enum pwrap_regs reg)

> >

> >  static bool pwrap_is_fsm_idle(struct pmic_wrapper *wrp)

> >  {

> > -       u32 val = pwrap_readl(wrp, PWRAP_WACS2_RDATA);

> > +       u32 val;

> > +

> > +       val = pwrap_readl(wrp, PWRAP_WACS2_RDATA);

> > +       if (HAS_CAP(wrp->master->caps, PWRAP_CAP_ARB))

> > +               return PWRAP_GET_WACS_ARB_FSM(val) == PWRAP_WACS_FSM_IDLE;

> >

> >         return PWRAP_GET_WACS_FSM(val) == PWRAP_WACS_FSM_IDLE;

> >  }

> >

> >  static bool pwrap_is_fsm_vldclr(struct pmic_wrapper *wrp)

> >  {

> > -       u32 val = pwrap_readl(wrp, PWRAP_WACS2_RDATA);

> > +       u32 val;

> > +

> > +       val = pwrap_readl(wrp, PWRAP_WACS2_RDATA);

> > +       if (HAS_CAP(wrp->master->caps, PWRAP_CAP_ARB))

> > +               return PWRAP_GET_WACS_ARB_FSM(val) == PWRAP_WACS_FSM_WFVLDCLR;

> 

> This code is now copied twice. Do you think it'd be better to create a

> new function?

> 

> static u32 pwrap_get_fsm_state(struct pmic_wrapper *wrp) {

>    if (HAS_CAP(wrp->master->caps, PWRAP_CAP_ARB))

>       return PWRAP_GET_WACS_ARB_FSM(val);

>    else

>       return PWRAP_GET_WACS_FSM(val);

> }

> 


Thanks for the review. I will update it in the next patch.

> >

> >         return PWRAP_GET_WACS_FSM(val) == PWRAP_WACS_FSM_WFVLDCLR;

> >  }

> > @@ -1165,6 +1178,7 @@ static int pwrap_wait_for_state(struct pmic_wrapper *wrp,

> >  static int pwrap_read16(struct pmic_wrapper *wrp, u32 adr, u32 *rdata)

> >  {

> >         int ret;

> > +       u32 val;

> >

> >         ret = pwrap_wait_for_state(wrp, pwrap_is_fsm_idle);

> >         if (ret) {

> > @@ -1172,13 +1186,21 @@ static int pwrap_read16(struct pmic_wrapper *wrp, u32 adr, u32 *rdata)

> >                 return ret;

> >         }

> >

> > -       pwrap_writel(wrp, (adr >> 1) << 16, PWRAP_WACS2_CMD);

> > +       if (HAS_CAP(wrp->master->caps, PWRAP_CAP_ARB))

> > +               val = adr;

> > +       else

> > +               val = (adr >> 1) << 16;

> > +       pwrap_writel(wrp, val, PWRAP_WACS2_CMD);

> >

> >         ret = pwrap_wait_for_state(wrp, pwrap_is_fsm_vldclr);

> >         if (ret)

> >                 return ret;

> >

> > -       *rdata = PWRAP_GET_WACS_RDATA(pwrap_readl(wrp, PWRAP_WACS2_RDATA));

> > +       if (HAS_CAP(wrp->master->caps, PWRAP_CAP_ARB))

> > +               val = pwrap_readl(wrp, PWRAP_SWINF_2_RDATA_31_0);

> > +       else

> > +               val = pwrap_readl(wrp, PWRAP_WACS2_RDATA);

> > +       *rdata = PWRAP_GET_WACS_RDATA(val);

> >

> >         pwrap_writel(wrp, 1, PWRAP_WACS2_VLDCLR);

> >

> > @@ -1228,8 +1250,13 @@ static int pwrap_write16(struct pmic_wrapper *wrp, u32 adr, u32 wdata)

> >                 return ret;

> >         }

> >

> > -       pwrap_writel(wrp, (1 << 31) | ((adr >> 1) << 16) | wdata,

> > -                    PWRAP_WACS2_CMD);

> > +       if (HAS_CAP(wrp->master->caps, PWRAP_CAP_ARB)) {

> > +               pwrap_writel(wrp, wdata, PWRAP_SWINF_2_WDATA_31_0);

> > +               pwrap_writel(wrp, BIT(29) | adr, PWRAP_WACS2_CMD);

> > +       } else {

> > +               pwrap_writel(wrp, BIT(31) | ((adr >> 1) << 16) | wdata,

> > +                            PWRAP_WACS2_CMD);

> > +       }

> >

> >         return 0;

> >  }

> > @@ -2022,6 +2049,7 @@ MODULE_DEVICE_TABLE(of, of_pwrap_match_tbl);

> >  static int pwrap_probe(struct platform_device *pdev)

> >  {

> >         int ret, irq;

> > +       u32 mask_done;

> >         struct pmic_wrapper *wrp;

> >         struct device_node *np = pdev->dev.of_node;

> >         const struct of_device_id *of_slave_id = NULL;

> > @@ -2116,14 +2144,21 @@ static int pwrap_probe(struct platform_device *pdev)

> >                 }

> >         }

> >

> > -       if (!(pwrap_readl(wrp, PWRAP_WACS2_RDATA) & PWRAP_STATE_INIT_DONE0)) {

> > +       if (HAS_CAP(wrp->master->caps, PWRAP_CAP_ARB))

> > +               mask_done = PWRAP_STATE_INIT_DONE1;

> > +       else

> > +               mask_done = PWRAP_STATE_INIT_DONE0;

> > +

> > +       if (!(pwrap_readl(wrp, PWRAP_WACS2_RDATA) & mask_done)) {

> >                 dev_dbg(wrp->dev, "initialization isn't finished\n");

> >                 ret = -ENODEV;

> >                 goto err_out2;

> >         }

> >

> >         /* Initialize watchdog, may not be done by the bootloader */

> > -       pwrap_writel(wrp, 0xf, PWRAP_WDT_UNIT);

> > +       if (!HAS_CAP(wrp->master->caps, PWRAP_CAP_ARB))

> > +               pwrap_writel(wrp, 0xf, PWRAP_WDT_UNIT);

> > +

> 

> To expand on Matthias' question on v3

> (https://patchwork.kernel.org/project/linux-mediatek/patch/1600686235-27979-3-git-send-email-hsin-hsiung.wang@mediatek.com/):

> is there any PWRAP implementation where a design with an arbiter is

> still able to control the watchdog?

> 

> If not, at the very least, it'd be good to expand the comment above

> (e.g. "designs with arbiter support cannot change the watchdog

> timer").

> 


No, there is no other wrap design with an arbiter to control the
watchdog.
Thanks for the comment, I will update the comment in next patch.

> >         /*

> >          * Since STAUPD was not used on mt8173 platform,

> >          * so STAUPD of WDT_SRC which should be turned off

> > @@ -2132,7 +2167,11 @@ static int pwrap_probe(struct platform_device *pdev)

> >         if (HAS_CAP(wrp->master->caps, PWRAP_CAP_WDT_SRC1))

> >                 pwrap_writel(wrp, wrp->master->wdt_src, PWRAP_WDT_SRC_EN_1);

> >

> > -       pwrap_writel(wrp, 0x1, PWRAP_TIMER_EN);

> > +       if (!HAS_CAP(wrp->master->caps, PWRAP_CAP_ARB))

> 

> Please invert this if test:

> 

> if (HAS_CAP(wrp->master->caps, PWRAP_CAP_ARB))

>    ... 0x3 ...

> else

>    ... 0x1 ...

> 


Thanks for the review. I will update it in the next patch.

> > +               pwrap_writel(wrp, 0x1, PWRAP_TIMER_EN);

> > +       else

> > +               pwrap_writel(wrp, 0x3, PWRAP_TIMER_EN);

> > +

> >         pwrap_writel(wrp, wrp->master->int_en_all, PWRAP_INT_EN);

> >         /*

> >          * We add INT1 interrupt to handle starvation and request exception

> > --

> > 2.6.4

> > _______________________________________________

> > Linux-mediatek mailing list

> > Linux-mediatek@lists.infradead.org

> > http://lists.infradead.org/mailman/listinfo/linux-mediatek
diff mbox series

Patch

diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
index c897205..5678f46 100644
--- a/drivers/soc/mediatek/mtk-pmic-wrap.c
+++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
@@ -25,10 +25,12 @@ 
 
 /* macro for wrapper status */
 #define PWRAP_GET_WACS_RDATA(x)		(((x) >> 0) & 0x0000ffff)
+#define PWRAP_GET_WACS_ARB_FSM(x)	(((x) >> 1) & 0x00000007)
 #define PWRAP_GET_WACS_FSM(x)		(((x) >> 16) & 0x00000007)
 #define PWRAP_GET_WACS_REQ(x)		(((x) >> 19) & 0x00000001)
 #define PWRAP_STATE_SYNC_IDLE0		BIT(20)
 #define PWRAP_STATE_INIT_DONE0		BIT(21)
+#define PWRAP_STATE_INIT_DONE1		BIT(15)
 
 /* macro for WACS FSM */
 #define PWRAP_WACS_FSM_IDLE		0x00
@@ -74,6 +76,7 @@ 
 #define PWRAP_CAP_DCM		BIT(2)
 #define PWRAP_CAP_INT1_EN	BIT(3)
 #define PWRAP_CAP_WDT_SRC1	BIT(4)
+#define PWRAP_CAP_ARB		BIT(5)
 
 /* defines for slave device wrapper registers */
 enum dew_regs {
@@ -340,6 +343,8 @@  enum pwrap_regs {
 	PWRAP_DCM_DBC_PRD,
 	PWRAP_EINT_STA0_ADR,
 	PWRAP_EINT_STA1_ADR,
+	PWRAP_SWINF_2_WDATA_31_0,
+	PWRAP_SWINF_2_RDATA_31_0,
 
 	/* MT2701 only regs */
 	PWRAP_ADC_CMD_ADDR,
@@ -1108,14 +1113,22 @@  static void pwrap_writel(struct pmic_wrapper *wrp, u32 val, enum pwrap_regs reg)
 
 static bool pwrap_is_fsm_idle(struct pmic_wrapper *wrp)
 {
-	u32 val = pwrap_readl(wrp, PWRAP_WACS2_RDATA);
+	u32 val;
+
+	val = pwrap_readl(wrp, PWRAP_WACS2_RDATA);
+	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_ARB))
+		return PWRAP_GET_WACS_ARB_FSM(val) == PWRAP_WACS_FSM_IDLE;
 
 	return PWRAP_GET_WACS_FSM(val) == PWRAP_WACS_FSM_IDLE;
 }
 
 static bool pwrap_is_fsm_vldclr(struct pmic_wrapper *wrp)
 {
-	u32 val = pwrap_readl(wrp, PWRAP_WACS2_RDATA);
+	u32 val;
+
+	val = pwrap_readl(wrp, PWRAP_WACS2_RDATA);
+	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_ARB))
+		return PWRAP_GET_WACS_ARB_FSM(val) == PWRAP_WACS_FSM_WFVLDCLR;
 
 	return PWRAP_GET_WACS_FSM(val) == PWRAP_WACS_FSM_WFVLDCLR;
 }
@@ -1165,6 +1178,7 @@  static int pwrap_wait_for_state(struct pmic_wrapper *wrp,
 static int pwrap_read16(struct pmic_wrapper *wrp, u32 adr, u32 *rdata)
 {
 	int ret;
+	u32 val;
 
 	ret = pwrap_wait_for_state(wrp, pwrap_is_fsm_idle);
 	if (ret) {
@@ -1172,13 +1186,21 @@  static int pwrap_read16(struct pmic_wrapper *wrp, u32 adr, u32 *rdata)
 		return ret;
 	}
 
-	pwrap_writel(wrp, (adr >> 1) << 16, PWRAP_WACS2_CMD);
+	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_ARB))
+		val = adr;
+	else
+		val = (adr >> 1) << 16;
+	pwrap_writel(wrp, val, PWRAP_WACS2_CMD);
 
 	ret = pwrap_wait_for_state(wrp, pwrap_is_fsm_vldclr);
 	if (ret)
 		return ret;
 
-	*rdata = PWRAP_GET_WACS_RDATA(pwrap_readl(wrp, PWRAP_WACS2_RDATA));
+	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_ARB))
+		val = pwrap_readl(wrp, PWRAP_SWINF_2_RDATA_31_0);
+	else
+		val = pwrap_readl(wrp, PWRAP_WACS2_RDATA);
+	*rdata = PWRAP_GET_WACS_RDATA(val);
 
 	pwrap_writel(wrp, 1, PWRAP_WACS2_VLDCLR);
 
@@ -1228,8 +1250,13 @@  static int pwrap_write16(struct pmic_wrapper *wrp, u32 adr, u32 wdata)
 		return ret;
 	}
 
-	pwrap_writel(wrp, (1 << 31) | ((adr >> 1) << 16) | wdata,
-		     PWRAP_WACS2_CMD);
+	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_ARB)) {
+		pwrap_writel(wrp, wdata, PWRAP_SWINF_2_WDATA_31_0);
+		pwrap_writel(wrp, BIT(29) | adr, PWRAP_WACS2_CMD);
+	} else {
+		pwrap_writel(wrp, BIT(31) | ((adr >> 1) << 16) | wdata,
+			     PWRAP_WACS2_CMD);
+	}
 
 	return 0;
 }
@@ -2022,6 +2049,7 @@  MODULE_DEVICE_TABLE(of, of_pwrap_match_tbl);
 static int pwrap_probe(struct platform_device *pdev)
 {
 	int ret, irq;
+	u32 mask_done;
 	struct pmic_wrapper *wrp;
 	struct device_node *np = pdev->dev.of_node;
 	const struct of_device_id *of_slave_id = NULL;
@@ -2116,14 +2144,21 @@  static int pwrap_probe(struct platform_device *pdev)
 		}
 	}
 
-	if (!(pwrap_readl(wrp, PWRAP_WACS2_RDATA) & PWRAP_STATE_INIT_DONE0)) {
+	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_ARB))
+		mask_done = PWRAP_STATE_INIT_DONE1;
+	else
+		mask_done = PWRAP_STATE_INIT_DONE0;
+
+	if (!(pwrap_readl(wrp, PWRAP_WACS2_RDATA) & mask_done)) {
 		dev_dbg(wrp->dev, "initialization isn't finished\n");
 		ret = -ENODEV;
 		goto err_out2;
 	}
 
 	/* Initialize watchdog, may not be done by the bootloader */
-	pwrap_writel(wrp, 0xf, PWRAP_WDT_UNIT);
+	if (!HAS_CAP(wrp->master->caps, PWRAP_CAP_ARB))
+		pwrap_writel(wrp, 0xf, PWRAP_WDT_UNIT);
+
 	/*
 	 * Since STAUPD was not used on mt8173 platform,
 	 * so STAUPD of WDT_SRC which should be turned off
@@ -2132,7 +2167,11 @@  static int pwrap_probe(struct platform_device *pdev)
 	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_WDT_SRC1))
 		pwrap_writel(wrp, wrp->master->wdt_src, PWRAP_WDT_SRC_EN_1);
 
-	pwrap_writel(wrp, 0x1, PWRAP_TIMER_EN);
+	if (!HAS_CAP(wrp->master->caps, PWRAP_CAP_ARB))
+		pwrap_writel(wrp, 0x1, PWRAP_TIMER_EN);
+	else
+		pwrap_writel(wrp, 0x3, PWRAP_TIMER_EN);
+
 	pwrap_writel(wrp, wrp->master->int_en_all, PWRAP_INT_EN);
 	/*
 	 * We add INT1 interrupt to handle starvation and request exception