diff mbox

[V3,08/11] soc: mediatek: PMIC wrap: remove pwrap_is_mt8135() and pwrap_is_mt8173()

Message ID 1453715604-36856-9-git-send-email-blogic@openwrt.org (mailing list archive)
State New, archived
Headers show

Commit Message

John Crispin Jan. 25, 2016, 9:53 a.m. UTC
With ore SoCs being added the list of helper functions like these would
grow. While at it also add a new flag "bridge" and use that insted of
pwrap_is_mt8173() where appropriate.

Signed-off-by: John Crispin <blogic@openwrt.org>
---
 drivers/soc/mediatek/mtk-pmic-wrap.c |   27 +++++++++++----------------
 1 file changed, 11 insertions(+), 16 deletions(-)

Comments

Yingjoe Chen Jan. 26, 2016, 12:53 p.m. UTC | #1
On Mon, 2016-01-25 at 10:53 +0100, John Crispin wrote:
> With ore SoCs being added the list of helper functions like these would
> grow. While at it also add a new flag "bridge" and use that insted of
> pwrap_is_mt8173() where appropriate.


typos?
s/ore/more/
s/insted/instead/

I think you mean "has_bridge" flag?


> 
> Signed-off-by: John Crispin <blogic@openwrt.org>
> ---
>  drivers/soc/mediatek/mtk-pmic-wrap.c |   27 +++++++++++----------------
>  1 file changed, 11 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
> index 8bb091b..54553b4 100644
> --- a/drivers/soc/mediatek/mtk-pmic-wrap.c
> +++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
> @@ -374,20 +374,11 @@ struct pmic_wrapper_type {
>  	u32 int_en_all;
>  	u32 spi_w;
>  	u32 wdt_src;
> +	int has_bridge;

How about using :1 for flag?

Joe.C

>  	int (*init_reg_clock)(struct pmic_wrapper *wrp);
>  	int (*init_special)(struct pmic_wrapper *wrp);
>  };
>  
> -static inline int pwrap_is_mt8135(struct pmic_wrapper *wrp)
> -{
> -	return wrp->master->type == PWRAP_MT8135;
> -}
> -
> -static inline int pwrap_is_mt8173(struct pmic_wrapper *wrp)
> -{
> -	return wrp->master->type == PWRAP_MT8173;
> -}
> -
>  static u32 pwrap_readl(struct pmic_wrapper *wrp, enum pwrap_regs reg)
>  {
>  	return readl(wrp->base + wrp->master->regs[reg]);
> @@ -619,11 +610,14 @@ static int pwrap_init_cipher(struct pmic_wrapper *wrp)
>  	pwrap_writel(wrp, 0x1, PWRAP_CIPHER_KEY_SEL);
>  	pwrap_writel(wrp, 0x2, PWRAP_CIPHER_IV_SEL);
>  
> -	if (pwrap_is_mt8135(wrp)) {
> +	switch (wrp->master->type) {
> +	case PWRAP_MT8135:
>  		pwrap_writel(wrp, 1, PWRAP_CIPHER_LOAD);
>  		pwrap_writel(wrp, 1, PWRAP_CIPHER_START);
> -	} else {
> +		break;
> +	case PWRAP_MT8173:
>  		pwrap_writel(wrp, 1, PWRAP_CIPHER_EN);
> +		break;
>  	}
>  
>  	/* Config cipher mode @PMIC */
> @@ -713,7 +707,7 @@ static int pwrap_init(struct pmic_wrapper *wrp)
>  	if (wrp->rstc_bridge)
>  		reset_control_reset(wrp->rstc_bridge);
>  
> -	if (pwrap_is_mt8173(wrp)) {
> +	if (wrp->master->type == PWRAP_MT8173) {
>  		/* Enable DCM */
>  		pwrap_writel(wrp, 3, PWRAP_DCM_EN);
>  		pwrap_writel(wrp, 0, PWRAP_DCM_DBC_PRD);
> @@ -773,7 +767,7 @@ static int pwrap_init(struct pmic_wrapper *wrp)
>  	pwrap_writel(wrp, PWRAP_DEW_CRC_VAL, PWRAP_SIG_ADR);
>  	pwrap_writel(wrp, wrp->master->arb_en_all, PWRAP_HIPRIO_ARB_EN);
>  
> -	if (pwrap_is_mt8135(wrp))
> +	if (wrp->master->type == PWRAP_MT8135)
>  		pwrap_writel(wrp, 0x7, PWRAP_RRARB_EN);
>  
>  	pwrap_writel(wrp, 0x1, PWRAP_WACS0_EN);
> @@ -792,7 +786,7 @@ static int pwrap_init(struct pmic_wrapper *wrp)
>  	pwrap_writel(wrp, 1, PWRAP_INIT_DONE0);
>  	pwrap_writel(wrp, 1, PWRAP_INIT_DONE1);
>  
> -	if (pwrap_is_mt8135(wrp)) {
> +	if (wrp->master->has_bridge) {
>  		writel(1, wrp->bridge_base + PWRAP_MT8135_BRIDGE_INIT_DONE3);
>  		writel(1, wrp->bridge_base + PWRAP_MT8135_BRIDGE_INIT_DONE4);
>  	}
> @@ -830,6 +824,7 @@ static struct pmic_wrapper_type pwrap_mt8135 = {
>  	.int_en_all = BIT(31) | BIT(1),
>  	.spi_w = PWRAP_MAN_CMD_SPI_WRITE,
>  	.wdt_src = PWRAP_WDT_SRC_MASK_ALL,
> +	.has_bridge = 1,
>  	.init_reg_clock = pwrap_mt8135_init_reg_clock,
>  	.init_special = pwrap_mt8135_init_special,
>  };
> @@ -888,7 +883,7 @@ static int pwrap_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> -	if (pwrap_is_mt8135(wrp)) {
> +	if (wrp->master->has_bridge) {
>  		res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
>  				"pwrap-bridge");
>  		wrp->bridge_base = devm_ioremap_resource(wrp->dev, res);
Matthias Brugger Feb. 1, 2016, 10:55 a.m. UTC | #2
On 25/01/16 10:53, John Crispin wrote:
> With ore SoCs being added the list of helper functions like these would

The commit message is something strange:
"With every new SoC being added..." maybe?

> grow. While at it also add a new flag "bridge" and use that insted of

s/insted/instead

> pwrap_is_mt8173() where appropriate.
>
> Signed-off-by: John Crispin <blogic@openwrt.org>
> ---
>   drivers/soc/mediatek/mtk-pmic-wrap.c |   27 +++++++++++----------------
>   1 file changed, 11 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
> index 8bb091b..54553b4 100644
> --- a/drivers/soc/mediatek/mtk-pmic-wrap.c
> +++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
> @@ -374,20 +374,11 @@ struct pmic_wrapper_type {
>   	u32 int_en_all;
>   	u32 spi_w;
>   	u32 wdt_src;
> +	int has_bridge;
>   	int (*init_reg_clock)(struct pmic_wrapper *wrp);
>   	int (*init_special)(struct pmic_wrapper *wrp);
>   };
>
> -static inline int pwrap_is_mt8135(struct pmic_wrapper *wrp)
> -{
> -	return wrp->master->type == PWRAP_MT8135;
> -}
> -
> -static inline int pwrap_is_mt8173(struct pmic_wrapper *wrp)
> -{
> -	return wrp->master->type == PWRAP_MT8173;
> -}
> -
>   static u32 pwrap_readl(struct pmic_wrapper *wrp, enum pwrap_regs reg)
>   {
>   	return readl(wrp->base + wrp->master->regs[reg]);
> @@ -619,11 +610,14 @@ static int pwrap_init_cipher(struct pmic_wrapper *wrp)
>   	pwrap_writel(wrp, 0x1, PWRAP_CIPHER_KEY_SEL);
>   	pwrap_writel(wrp, 0x2, PWRAP_CIPHER_IV_SEL);
>
> -	if (pwrap_is_mt8135(wrp)) {
> +	switch (wrp->master->type) {
> +	case PWRAP_MT8135:
>   		pwrap_writel(wrp, 1, PWRAP_CIPHER_LOAD);
>   		pwrap_writel(wrp, 1, PWRAP_CIPHER_START);
> -	} else {
> +		break;
> +	case PWRAP_MT8173:
>   		pwrap_writel(wrp, 1, PWRAP_CIPHER_EN);
> +		break;
>   	}
>
>   	/* Config cipher mode @PMIC */
> @@ -713,7 +707,7 @@ static int pwrap_init(struct pmic_wrapper *wrp)
>   	if (wrp->rstc_bridge)
>   		reset_control_reset(wrp->rstc_bridge);
>
> -	if (pwrap_is_mt8173(wrp)) {
> +	if (wrp->master->type == PWRAP_MT8173) {
>   		/* Enable DCM */
>   		pwrap_writel(wrp, 3, PWRAP_DCM_EN);
>   		pwrap_writel(wrp, 0, PWRAP_DCM_DBC_PRD);
> @@ -773,7 +767,7 @@ static int pwrap_init(struct pmic_wrapper *wrp)
>   	pwrap_writel(wrp, PWRAP_DEW_CRC_VAL, PWRAP_SIG_ADR);
>   	pwrap_writel(wrp, wrp->master->arb_en_all, PWRAP_HIPRIO_ARB_EN);
>
> -	if (pwrap_is_mt8135(wrp))
> +	if (wrp->master->type == PWRAP_MT8135)
>   		pwrap_writel(wrp, 0x7, PWRAP_RRARB_EN);
>
>   	pwrap_writel(wrp, 0x1, PWRAP_WACS0_EN);
> @@ -792,7 +786,7 @@ static int pwrap_init(struct pmic_wrapper *wrp)
>   	pwrap_writel(wrp, 1, PWRAP_INIT_DONE0);
>   	pwrap_writel(wrp, 1, PWRAP_INIT_DONE1);
>
> -	if (pwrap_is_mt8135(wrp)) {
> +	if (wrp->master->has_bridge) {
>   		writel(1, wrp->bridge_base + PWRAP_MT8135_BRIDGE_INIT_DONE3);
>   		writel(1, wrp->bridge_base + PWRAP_MT8135_BRIDGE_INIT_DONE4);
>   	}
> @@ -830,6 +824,7 @@ static struct pmic_wrapper_type pwrap_mt8135 = {
>   	.int_en_all = BIT(31) | BIT(1),
>   	.spi_w = PWRAP_MAN_CMD_SPI_WRITE,
>   	.wdt_src = PWRAP_WDT_SRC_MASK_ALL,
> +	.has_bridge = 1,
>   	.init_reg_clock = pwrap_mt8135_init_reg_clock,
>   	.init_special = pwrap_mt8135_init_special,
>   };

Please set has_bridge explicitly for mt8173.
John Crispin Feb. 1, 2016, 11 a.m. UTC | #3
On 01/02/2016 11:55, Matthias Brugger wrote:
> 
> 
> On 25/01/16 10:53, John Crispin wrote:
>> With ore SoCs being added the list of helper functions like these would
> 
> The commit message is something strange:
> "With every new SoC being added..." maybe?
> 
>> grow. While at it also add a new flag "bridge" and use that insted of
> 
> s/insted/instead
> 
>> pwrap_is_mt8173() where appropriate.

you are lookign at V3 of the series, V4 has this fix done already

[...]


>>       }
>> @@ -830,6 +824,7 @@ static struct pmic_wrapper_type pwrap_mt8135 = {
>>       .int_en_all = BIT(31) | BIT(1),
>>       .spi_w = PWRAP_MAN_CMD_SPI_WRITE,
>>       .wdt_src = PWRAP_WDT_SRC_MASK_ALL,
>> +    .has_bridge = 1,
>>       .init_reg_clock = pwrap_mt8135_init_reg_clock,
>>       .init_special = pwrap_mt8135_init_special,
>>   };
> 
> Please set has_bridge explicitly for mt8173.

I dont get it. the original code never did that.

	John

> 
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
Matthias Brugger Feb. 1, 2016, 11:11 a.m. UTC | #4
On 01/02/16 12:00, John Crispin wrote:
>
>
> On 01/02/2016 11:55, Matthias Brugger wrote:
>>
>>
>> On 25/01/16 10:53, John Crispin wrote:
>>> With ore SoCs being added the list of helper functions like these would
>>
>> The commit message is something strange:
>> "With every new SoC being added..." maybe?
>>
>>> grow. While at it also add a new flag "bridge" and use that insted of
>>
>> s/insted/instead
>>
>>> pwrap_is_mt8173() where appropriate.
>
> you are lookign at V3 of the series, V4 has this fix done already
>
> [...]
>
>
>>>        }
>>> @@ -830,6 +824,7 @@ static struct pmic_wrapper_type pwrap_mt8135 = {
>>>        .int_en_all = BIT(31) | BIT(1),
>>>        .spi_w = PWRAP_MAN_CMD_SPI_WRITE,
>>>        .wdt_src = PWRAP_WDT_SRC_MASK_ALL,
>>> +    .has_bridge = 1,
>>>        .init_reg_clock = pwrap_mt8135_init_reg_clock,
>>>        .init_special = pwrap_mt8135_init_special,
>>>    };
>>
>> Please set has_bridge explicitly for mt8173.
>
> I dont get it. the original code never did that.
>

has_bridge was introduced by this patch, but you don't set it explicitly 
to 0 in pwrap_mt8173.

Just as I see it, please try to write a summary to every new version of 
a patch set which explains what you changed between one version and 
another. This will help a lot making the review easier.

Thanks,
Matthias
John Crispin Feb. 1, 2016, 11:15 a.m. UTC | #5
On 01/02/2016 12:11, Matthias Brugger wrote:
> 
> 
> On 01/02/16 12:00, John Crispin wrote:
>>
>>
>> On 01/02/2016 11:55, Matthias Brugger wrote:
>>>
>>>
>>> On 25/01/16 10:53, John Crispin wrote:
>>>> With ore SoCs being added the list of helper functions like these would
>>>
>>> The commit message is something strange:
>>> "With every new SoC being added..." maybe?
>>>
>>>> grow. While at it also add a new flag "bridge" and use that insted of
>>>
>>> s/insted/instead
>>>
>>>> pwrap_is_mt8173() where appropriate.
>>
>> you are lookign at V3 of the series, V4 has this fix done already
>>
>> [...]
>>
>>
>>>>        }
>>>> @@ -830,6 +824,7 @@ static struct pmic_wrapper_type pwrap_mt8135 = {
>>>>        .int_en_all = BIT(31) | BIT(1),
>>>>        .spi_w = PWRAP_MAN_CMD_SPI_WRITE,
>>>>        .wdt_src = PWRAP_WDT_SRC_MASK_ALL,
>>>> +    .has_bridge = 1,
>>>>        .init_reg_clock = pwrap_mt8135_init_reg_clock,
>>>>        .init_special = pwrap_mt8135_init_special,
>>>>    };
>>>
>>> Please set has_bridge explicitly for mt8173.
>>
>> I dont get it. the original code never did that.
>>
> 
> has_bridge was introduced by this patch, but you don't set it explicitly
> to 0 in pwrap_mt8173.
> 
> Just as I see it, please try to write a summary to every new version of
> a patch set which explains what you changed between one version and
> another. This will help a lot making the review easier.
> 
> Thanks,
> Matthias
> 


You missed the "to zero" part before. now the comment makes sense. I can
set it to 0 if it is more obvious for you in that case.

general consent is to not declare statics to 0. check_patch.pl will
actually complain about those declarations. that is why i was confused.

	John
Matthias Brugger Feb. 1, 2016, 11:27 a.m. UTC | #6
On 01/02/16 12:15, John Crispin wrote:
>
>
> On 01/02/2016 12:11, Matthias Brugger wrote:
>>
>>
>> On 01/02/16 12:00, John Crispin wrote:
>>>
>>>
>>> On 01/02/2016 11:55, Matthias Brugger wrote:
>>>>
>>>>
>>>> On 25/01/16 10:53, John Crispin wrote:
>>>>> With ore SoCs being added the list of helper functions like these would
>>>>
>>>> The commit message is something strange:
>>>> "With every new SoC being added..." maybe?
>>>>
>>>>> grow. While at it also add a new flag "bridge" and use that insted of
>>>>
>>>> s/insted/instead
>>>>
>>>>> pwrap_is_mt8173() where appropriate.
>>>
>>> you are lookign at V3 of the series, V4 has this fix done already
>>>
>>> [...]
>>>
>>>
>>>>>         }
>>>>> @@ -830,6 +824,7 @@ static struct pmic_wrapper_type pwrap_mt8135 = {
>>>>>         .int_en_all = BIT(31) | BIT(1),
>>>>>         .spi_w = PWRAP_MAN_CMD_SPI_WRITE,
>>>>>         .wdt_src = PWRAP_WDT_SRC_MASK_ALL,
>>>>> +    .has_bridge = 1,
>>>>>         .init_reg_clock = pwrap_mt8135_init_reg_clock,
>>>>>         .init_special = pwrap_mt8135_init_special,
>>>>>     };
>>>>
>>>> Please set has_bridge explicitly for mt8173.
>>>
>>> I dont get it. the original code never did that.
>>>
>>
>> has_bridge was introduced by this patch, but you don't set it explicitly
>> to 0 in pwrap_mt8173.
>>
>> Just as I see it, please try to write a summary to every new version of
>> a patch set which explains what you changed between one version and
>> another. This will help a lot making the review easier.
>>
>> Thanks,
>> Matthias
>>
>
>
> You missed the "to zero" part before. now the comment makes sense. I can
> set it to 0 if it is more obvious for you in that case.
>
> general consent is to not declare statics to 0. check_patch.pl will
> actually complain about those declarations. that is why i was confused.

If that's the case, then we accept the authority of check_patch.pl ;)
I didn't know that, so just leave has_bridge as it was.

Regards,
Matthias
Yingjoe Chen Feb. 4, 2016, 9:36 a.m. UTC | #7
On Mon, 2016-02-01 at 12:27 +0100, Matthias Brugger wrote:
> 
> On 01/02/16 12:15, John Crispin wrote:
> >
> >
> > On 01/02/2016 12:11, Matthias Brugger wrote:
> >>
> >>
> >> On 01/02/16 12:00, John Crispin wrote:
> >>>
> >>>
> >>> On 01/02/2016 11:55, Matthias Brugger wrote:
> >>>>
> >>>>
> >>>> On 25/01/16 10:53, John Crispin wrote:
> >>>>> With ore SoCs being added the list of helper functions like these would
> >>>>
> >>>> The commit message is something strange:
> >>>> "With every new SoC being added..." maybe?
> >>>>
> >>>>> grow. While at it also add a new flag "bridge" and use that insted of
> >>>>
> >>>> s/insted/instead
> >>>>
> >>>>> pwrap_is_mt8173() where appropriate.
> >>>
> >>> you are lookign at V3 of the series, V4 has this fix done already
> >>>
> >>> [...]
> >>>
> >>>
> >>>>>         }
> >>>>> @@ -830,6 +824,7 @@ static struct pmic_wrapper_type pwrap_mt8135 = {
> >>>>>         .int_en_all = BIT(31) | BIT(1),
> >>>>>         .spi_w = PWRAP_MAN_CMD_SPI_WRITE,
> >>>>>         .wdt_src = PWRAP_WDT_SRC_MASK_ALL,
> >>>>> +    .has_bridge = 1,
> >>>>>         .init_reg_clock = pwrap_mt8135_init_reg_clock,
> >>>>>         .init_special = pwrap_mt8135_init_special,
> >>>>>     };
> >>>>
> >>>> Please set has_bridge explicitly for mt8173.
> >>>
> >>> I dont get it. the original code never did that.
> >>>
> >>
> >> has_bridge was introduced by this patch, but you don't set it explicitly
> >> to 0 in pwrap_mt8173.
> >>
> >> Just as I see it, please try to write a summary to every new version of
> >> a patch set which explains what you changed between one version and
> >> another. This will help a lot making the review easier.
> >>
> >> Thanks,
> >> Matthias
> >>
> >
> >
> > You missed the "to zero" part before. now the comment makes sense. I can
> > set it to 0 if it is more obvious for you in that case.
> >
> > general consent is to not declare statics to 0. check_patch.pl will
> > actually complain about those declarations. that is why i was confused.
> 
> If that's the case, then we accept the authority of check_patch.pl ;)
> I didn't know that, so just leave has_bridge as it was.

I believe checkpatch only complain for global/static variables
initializers, not inside a struct initializers even when they are
global.

In MTK i2c drivers drivers/i2c/busses/i2c-mt65xx.c, we always explicitly
init flags in compatible struct, IMHO it make support feature for each
IC more clearly. This does not trigger any warning from checkpatch.

Joe.C
Matthias Brugger Feb. 4, 2016, 6:37 p.m. UTC | #8
On 04/02/16 10:36, Yingjoe Chen wrote:
> On Mon, 2016-02-01 at 12:27 +0100, Matthias Brugger wrote:
>>
>> On 01/02/16 12:15, John Crispin wrote:
>>>
>>>
>>> On 01/02/2016 12:11, Matthias Brugger wrote:
>>>>
>>>>
>>>> On 01/02/16 12:00, John Crispin wrote:
>>>>>
>>>>>
>>>>> On 01/02/2016 11:55, Matthias Brugger wrote:
>>>>>>
>>>>>>
>>>>>> On 25/01/16 10:53, John Crispin wrote:
>>>>>>> With ore SoCs being added the list of helper functions like these would
>>>>>>
>>>>>> The commit message is something strange:
>>>>>> "With every new SoC being added..." maybe?
>>>>>>
>>>>>>> grow. While at it also add a new flag "bridge" and use that insted of
>>>>>>
>>>>>> s/insted/instead
>>>>>>
>>>>>>> pwrap_is_mt8173() where appropriate.
>>>>>
>>>>> you are lookign at V3 of the series, V4 has this fix done already
>>>>>
>>>>> [...]
>>>>>
>>>>>
>>>>>>>          }
>>>>>>> @@ -830,6 +824,7 @@ static struct pmic_wrapper_type pwrap_mt8135 = {
>>>>>>>          .int_en_all = BIT(31) | BIT(1),
>>>>>>>          .spi_w = PWRAP_MAN_CMD_SPI_WRITE,
>>>>>>>          .wdt_src = PWRAP_WDT_SRC_MASK_ALL,
>>>>>>> +    .has_bridge = 1,
>>>>>>>          .init_reg_clock = pwrap_mt8135_init_reg_clock,
>>>>>>>          .init_special = pwrap_mt8135_init_special,
>>>>>>>      };
>>>>>>
>>>>>> Please set has_bridge explicitly for mt8173.
>>>>>
>>>>> I dont get it. the original code never did that.
>>>>>
>>>>
>>>> has_bridge was introduced by this patch, but you don't set it explicitly
>>>> to 0 in pwrap_mt8173.
>>>>
>>>> Just as I see it, please try to write a summary to every new version of
>>>> a patch set which explains what you changed between one version and
>>>> another. This will help a lot making the review easier.
>>>>
>>>> Thanks,
>>>> Matthias
>>>>
>>>
>>>
>>> You missed the "to zero" part before. now the comment makes sense. I can
>>> set it to 0 if it is more obvious for you in that case.
>>>
>>> general consent is to not declare statics to 0. check_patch.pl will
>>> actually complain about those declarations. that is why i was confused.
>>
>> If that's the case, then we accept the authority of check_patch.pl ;)
>> I didn't know that, so just leave has_bridge as it was.
>
> I believe checkpatch only complain for global/static variables
> initializers, not inside a struct initializers even when they are
> global.
>
> In MTK i2c drivers drivers/i2c/busses/i2c-mt65xx.c, we always explicitly
> init flags in compatible struct, IMHO it make support feature for each
> IC more clearly. This does not trigger any warning from checkpatch.
>

Ok, thanks for clarification.
diff mbox

Patch

diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
index 8bb091b..54553b4 100644
--- a/drivers/soc/mediatek/mtk-pmic-wrap.c
+++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
@@ -374,20 +374,11 @@  struct pmic_wrapper_type {
 	u32 int_en_all;
 	u32 spi_w;
 	u32 wdt_src;
+	int has_bridge;
 	int (*init_reg_clock)(struct pmic_wrapper *wrp);
 	int (*init_special)(struct pmic_wrapper *wrp);
 };
 
-static inline int pwrap_is_mt8135(struct pmic_wrapper *wrp)
-{
-	return wrp->master->type == PWRAP_MT8135;
-}
-
-static inline int pwrap_is_mt8173(struct pmic_wrapper *wrp)
-{
-	return wrp->master->type == PWRAP_MT8173;
-}
-
 static u32 pwrap_readl(struct pmic_wrapper *wrp, enum pwrap_regs reg)
 {
 	return readl(wrp->base + wrp->master->regs[reg]);
@@ -619,11 +610,14 @@  static int pwrap_init_cipher(struct pmic_wrapper *wrp)
 	pwrap_writel(wrp, 0x1, PWRAP_CIPHER_KEY_SEL);
 	pwrap_writel(wrp, 0x2, PWRAP_CIPHER_IV_SEL);
 
-	if (pwrap_is_mt8135(wrp)) {
+	switch (wrp->master->type) {
+	case PWRAP_MT8135:
 		pwrap_writel(wrp, 1, PWRAP_CIPHER_LOAD);
 		pwrap_writel(wrp, 1, PWRAP_CIPHER_START);
-	} else {
+		break;
+	case PWRAP_MT8173:
 		pwrap_writel(wrp, 1, PWRAP_CIPHER_EN);
+		break;
 	}
 
 	/* Config cipher mode @PMIC */
@@ -713,7 +707,7 @@  static int pwrap_init(struct pmic_wrapper *wrp)
 	if (wrp->rstc_bridge)
 		reset_control_reset(wrp->rstc_bridge);
 
-	if (pwrap_is_mt8173(wrp)) {
+	if (wrp->master->type == PWRAP_MT8173) {
 		/* Enable DCM */
 		pwrap_writel(wrp, 3, PWRAP_DCM_EN);
 		pwrap_writel(wrp, 0, PWRAP_DCM_DBC_PRD);
@@ -773,7 +767,7 @@  static int pwrap_init(struct pmic_wrapper *wrp)
 	pwrap_writel(wrp, PWRAP_DEW_CRC_VAL, PWRAP_SIG_ADR);
 	pwrap_writel(wrp, wrp->master->arb_en_all, PWRAP_HIPRIO_ARB_EN);
 
-	if (pwrap_is_mt8135(wrp))
+	if (wrp->master->type == PWRAP_MT8135)
 		pwrap_writel(wrp, 0x7, PWRAP_RRARB_EN);
 
 	pwrap_writel(wrp, 0x1, PWRAP_WACS0_EN);
@@ -792,7 +786,7 @@  static int pwrap_init(struct pmic_wrapper *wrp)
 	pwrap_writel(wrp, 1, PWRAP_INIT_DONE0);
 	pwrap_writel(wrp, 1, PWRAP_INIT_DONE1);
 
-	if (pwrap_is_mt8135(wrp)) {
+	if (wrp->master->has_bridge) {
 		writel(1, wrp->bridge_base + PWRAP_MT8135_BRIDGE_INIT_DONE3);
 		writel(1, wrp->bridge_base + PWRAP_MT8135_BRIDGE_INIT_DONE4);
 	}
@@ -830,6 +824,7 @@  static struct pmic_wrapper_type pwrap_mt8135 = {
 	.int_en_all = BIT(31) | BIT(1),
 	.spi_w = PWRAP_MAN_CMD_SPI_WRITE,
 	.wdt_src = PWRAP_WDT_SRC_MASK_ALL,
+	.has_bridge = 1,
 	.init_reg_clock = pwrap_mt8135_init_reg_clock,
 	.init_special = pwrap_mt8135_init_special,
 };
@@ -888,7 +883,7 @@  static int pwrap_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	if (pwrap_is_mt8135(wrp)) {
+	if (wrp->master->has_bridge) {
 		res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
 				"pwrap-bridge");
 		wrp->bridge_base = devm_ioremap_resource(wrp->dev, res);