[2/5] pinctrl: imx: add soc specific mux_mode mask and shift property
diff mbox

Message ID 1494592686-30967-3-git-send-email-aisheng.dong@nxp.com
State New
Headers show

Commit Message

Aisheng Dong May 12, 2017, 12:38 p.m. UTC
MX7ULP MUX mode mask and shift bit is different from VF610.
Let's make it a platform specific property for the later easy of
adding MX7ULP support.

Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Stefan Agner <stefan@agner.ch>
Cc: Bai Ping <ping.bai@nxp.com>
Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
 drivers/pinctrl/freescale/pinctrl-imx.c   | 10 +++++-----
 drivers/pinctrl/freescale/pinctrl-imx.h   |  4 ++++
 drivers/pinctrl/freescale/pinctrl-vf610.c |  2 ++
 3 files changed, 11 insertions(+), 5 deletions(-)

Comments

Shawn Guo May 15, 2017, 8:52 a.m. UTC | #1
On Fri, May 12, 2017 at 08:38:02PM +0800, Dong Aisheng wrote:
> MX7ULP MUX mode mask and shift bit is different from VF610.
> Let's make it a platform specific property for the later easy of
> adding MX7ULP support.
> 
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Stefan Agner <stefan@agner.ch>
> Cc: Bai Ping <ping.bai@nxp.com>
> Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
> Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> ---
>  drivers/pinctrl/freescale/pinctrl-imx.c   | 10 +++++-----
>  drivers/pinctrl/freescale/pinctrl-imx.h   |  4 ++++
>  drivers/pinctrl/freescale/pinctrl-vf610.c |  2 ++
>  3 files changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/pinctrl/freescale/pinctrl-imx.c b/drivers/pinctrl/freescale/pinctrl-imx.c
> index db76e9d..8347197 100644
> --- a/drivers/pinctrl/freescale/pinctrl-imx.c
> +++ b/drivers/pinctrl/freescale/pinctrl-imx.c
> @@ -197,8 +197,8 @@ static int imx_pmx_set(struct pinctrl_dev *pctldev, unsigned selector,
>  		if (info->flags & SHARE_MUX_CONF_REG) {
>  			u32 reg;
>  			reg = readl(ipctl->base + pin_reg->mux_reg);
> -			reg &= ~(0x7 << 20);
> -			reg |= (pin->mux_mode << 20);
> +			reg &= ~info->mux_mask;
> +			reg |= (pin->mux_mode << info->mux_shift);
>  			writel(reg, ipctl->base + pin_reg->mux_reg);
>  		} else {
>  			writel(pin->mux_mode, ipctl->base + pin_reg->mux_reg);
> @@ -288,7 +288,7 @@ static int imx_pmx_gpio_request_enable(struct pinctrl_dev *pctldev,
>  
>  mux_pin:
>  	reg = readl(ipctl->base + pin_reg->mux_reg);
> -	reg &= ~(0x7 << 20);
> +	reg &= ~info->mux_mask;
>  	reg |= imx_pin->config;
>  	writel(reg, ipctl->base + pin_reg->mux_reg);
>  
> @@ -432,7 +432,7 @@ static int imx_pinconf_get(struct pinctrl_dev *pctldev,
>  	*config = readl(ipctl->base + pin_reg->conf_reg);
>  
>  	if (info->flags & SHARE_MUX_CONF_REG)
> -		*config &= 0xffff;
> +		*config &= ~info->mux_mask;

It changes the way how code works.  We need a note in the commit log
explaining why it's safe.

Shawn

>  
>  	return 0;
>  }
> @@ -459,7 +459,7 @@ static int imx_pinconf_set(struct pinctrl_dev *pctldev,
>  		if (info->flags & SHARE_MUX_CONF_REG) {
>  			u32 reg;
>  			reg = readl(ipctl->base + pin_reg->conf_reg);
> -			reg &= ~0xffff;
> +			reg &= info->mux_mask;
>  			reg |= configs[i];
>  			writel(reg, ipctl->base + pin_reg->conf_reg);
>  		} else {
> diff --git a/drivers/pinctrl/freescale/pinctrl-imx.h b/drivers/pinctrl/freescale/pinctrl-imx.h
> index b5c8fe1..eb0ce95 100644
> --- a/drivers/pinctrl/freescale/pinctrl-imx.h
> +++ b/drivers/pinctrl/freescale/pinctrl-imx.h
> @@ -64,6 +64,10 @@ struct imx_pinctrl_soc_info {
>  	const char *gpr_compatible;
>  	struct mutex mutex;
>  
> +	/* MUX_MODE shift and mask in case SHARE_MUX_CONF_REG */
> +	unsigned int mux_mask;
> +	u8 mux_shift;
> +
>  	/* generic pinconf */
>  	bool generic_pinconf;
>  	const struct pinconf_generic_params *custom_params;
> diff --git a/drivers/pinctrl/freescale/pinctrl-vf610.c b/drivers/pinctrl/freescale/pinctrl-vf610.c
> index 2b1e198..3bd8556 100644
> --- a/drivers/pinctrl/freescale/pinctrl-vf610.c
> +++ b/drivers/pinctrl/freescale/pinctrl-vf610.c
> @@ -299,6 +299,8 @@ static struct imx_pinctrl_soc_info vf610_pinctrl_info = {
>  	.pins = vf610_pinctrl_pads,
>  	.npins = ARRAY_SIZE(vf610_pinctrl_pads),
>  	.flags = SHARE_MUX_CONF_REG | ZERO_OFFSET_VALID,
> +	.mux_mask = 0x700000,
> +	.mux_shift = 20,
>  };
>  
>  static const struct of_device_id vf610_pinctrl_of_match[] = {
> -- 
> 2.7.4
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Aisheng Dong May 15, 2017, 9 a.m. UTC | #2
> -----Original Message-----
> From: Shawn Guo [mailto:shawnguo@kernel.org]
> Sent: Monday, May 15, 2017 4:53 PM
> To: A.S. Dong
> Cc: linux-gpio@vger.kernel.org; Andy Duan; Jacky Bai;
> linus.walleij@linaro.org; stefan@agner.ch; kernel@pengutronix.de; linux-
> arm-kernel@lists.infradead.org
> Subject: Re: [PATCH 2/5] pinctrl: imx: add soc specific mux_mode mask and
> shift property
> 
> On Fri, May 12, 2017 at 08:38:02PM +0800, Dong Aisheng wrote:
> > MX7ULP MUX mode mask and shift bit is different from VF610.
> > Let's make it a platform specific property for the later easy of
> > adding MX7ULP support.
> >
> > Cc: Linus Walleij <linus.walleij@linaro.org>
> > Cc: Shawn Guo <shawnguo@kernel.org>
> > Cc: Stefan Agner <stefan@agner.ch>
> > Cc: Bai Ping <ping.bai@nxp.com>
> > Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
> > Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> > ---
> >  drivers/pinctrl/freescale/pinctrl-imx.c   | 10 +++++-----
> >  drivers/pinctrl/freescale/pinctrl-imx.h   |  4 ++++
> >  drivers/pinctrl/freescale/pinctrl-vf610.c |  2 ++
> >  3 files changed, 11 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/pinctrl/freescale/pinctrl-imx.c
> > b/drivers/pinctrl/freescale/pinctrl-imx.c
> > index db76e9d..8347197 100644
> > --- a/drivers/pinctrl/freescale/pinctrl-imx.c
> > +++ b/drivers/pinctrl/freescale/pinctrl-imx.c
> > @@ -197,8 +197,8 @@ static int imx_pmx_set(struct pinctrl_dev *pctldev,
> unsigned selector,
> >  		if (info->flags & SHARE_MUX_CONF_REG) {
> >  			u32 reg;
> >  			reg = readl(ipctl->base + pin_reg->mux_reg);
> > -			reg &= ~(0x7 << 20);
> > -			reg |= (pin->mux_mode << 20);
> > +			reg &= ~info->mux_mask;
> > +			reg |= (pin->mux_mode << info->mux_shift);
> >  			writel(reg, ipctl->base + pin_reg->mux_reg);
> >  		} else {
> >  			writel(pin->mux_mode, ipctl->base + pin_reg->mux_reg);
> @@ -288,7
> > +288,7 @@ static int imx_pmx_gpio_request_enable(struct pinctrl_dev
> > *pctldev,
> >
> >  mux_pin:
> >  	reg = readl(ipctl->base + pin_reg->mux_reg);
> > -	reg &= ~(0x7 << 20);
> > +	reg &= ~info->mux_mask;
> >  	reg |= imx_pin->config;
> >  	writel(reg, ipctl->base + pin_reg->mux_reg);
> >
> > @@ -432,7 +432,7 @@ static int imx_pinconf_get(struct pinctrl_dev
> *pctldev,
> >  	*config = readl(ipctl->base + pin_reg->conf_reg);
> >
> >  	if (info->flags & SHARE_MUX_CONF_REG)
> > -		*config &= 0xffff;
> > +		*config &= ~info->mux_mask;
> 
> It changes the way how code works.  We need a note in the commit log
> explaining why it's safe.
> 

Well, that's Vybrid tricks that BIT[15-0] are all configs part.
So it hardcoded 0xffff there.

But it's not true in ULP, so use mux_mask instead to address
the difference.

Regards
Dong Aisheng

> Shawn
> 
> >
> >  	return 0;
> >  }
> > @@ -459,7 +459,7 @@ static int imx_pinconf_set(struct pinctrl_dev
> *pctldev,
> >  		if (info->flags & SHARE_MUX_CONF_REG) {
> >  			u32 reg;
> >  			reg = readl(ipctl->base + pin_reg->conf_reg);
> > -			reg &= ~0xffff;
> > +			reg &= info->mux_mask;
> >  			reg |= configs[i];
> >  			writel(reg, ipctl->base + pin_reg->conf_reg);
> >  		} else {
> > diff --git a/drivers/pinctrl/freescale/pinctrl-imx.h
> > b/drivers/pinctrl/freescale/pinctrl-imx.h
> > index b5c8fe1..eb0ce95 100644
> > --- a/drivers/pinctrl/freescale/pinctrl-imx.h
> > +++ b/drivers/pinctrl/freescale/pinctrl-imx.h
> > @@ -64,6 +64,10 @@ struct imx_pinctrl_soc_info {
> >  	const char *gpr_compatible;
> >  	struct mutex mutex;
> >
> > +	/* MUX_MODE shift and mask in case SHARE_MUX_CONF_REG */
> > +	unsigned int mux_mask;
> > +	u8 mux_shift;
> > +
> >  	/* generic pinconf */
> >  	bool generic_pinconf;
> >  	const struct pinconf_generic_params *custom_params; diff --git
> > a/drivers/pinctrl/freescale/pinctrl-vf610.c
> > b/drivers/pinctrl/freescale/pinctrl-vf610.c
> > index 2b1e198..3bd8556 100644
> > --- a/drivers/pinctrl/freescale/pinctrl-vf610.c
> > +++ b/drivers/pinctrl/freescale/pinctrl-vf610.c
> > @@ -299,6 +299,8 @@ static struct imx_pinctrl_soc_info
> vf610_pinctrl_info = {
> >  	.pins = vf610_pinctrl_pads,
> >  	.npins = ARRAY_SIZE(vf610_pinctrl_pads),
> >  	.flags = SHARE_MUX_CONF_REG | ZERO_OFFSET_VALID,
> > +	.mux_mask = 0x700000,
> > +	.mux_shift = 20,
> >  };
> >
> >  static const struct of_device_id vf610_pinctrl_of_match[] = {
> > --
> > 2.7.4
> >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Shawn Guo May 15, 2017, 10:59 a.m. UTC | #3
On Mon, May 15, 2017 at 09:00:32AM +0000, A.S. Dong wrote:
> > > @@ -432,7 +432,7 @@ static int imx_pinconf_get(struct pinctrl_dev
> > *pctldev,
> > >  	*config = readl(ipctl->base + pin_reg->conf_reg);
> > >
> > >  	if (info->flags & SHARE_MUX_CONF_REG)
> > > -		*config &= 0xffff;
> > > +		*config &= ~info->mux_mask;
> > 
> > It changes the way how code works.  We need a note in the commit log
> > explaining why it's safe.
> > 
> 
> Well, that's Vybrid tricks that BIT[15-0] are all configs part.
> So it hardcoded 0xffff there.
> 
> But it's not true in ULP, so use mux_mask instead to address
> the difference.

So you make the assumption that for all SHARE_MUX_CONF_REG SoCs, all
bits in the register except mux ones are config bits.  You at least need
to mention that in the commit log, IMO.

Shawn
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Aisheng Dong May 15, 2017, 11:04 a.m. UTC | #4
> -----Original Message-----
> From: Shawn Guo [mailto:shawnguo@kernel.org]
> Sent: Monday, May 15, 2017 6:59 PM
> To: A.S. Dong
> Cc: Andy Duan; Jacky Bai; linus.walleij@linaro.org; stefan@agner.ch;
> linux-gpio@vger.kernel.org; kernel@pengutronix.de; linux-arm-
> kernel@lists.infradead.org
> Subject: Re: [PATCH 2/5] pinctrl: imx: add soc specific mux_mode mask and
> shift property
> 
> On Mon, May 15, 2017 at 09:00:32AM +0000, A.S. Dong wrote:
> > > > @@ -432,7 +432,7 @@ static int imx_pinconf_get(struct pinctrl_dev
> > > *pctldev,
> > > >  	*config = readl(ipctl->base + pin_reg->conf_reg);
> > > >
> > > >  	if (info->flags & SHARE_MUX_CONF_REG)
> > > > -		*config &= 0xffff;
> > > > +		*config &= ~info->mux_mask;
> > >
> > > It changes the way how code works.  We need a note in the commit log
> > > explaining why it's safe.
> > >
> >
> > Well, that's Vybrid tricks that BIT[15-0] are all configs part.
> > So it hardcoded 0xffff there.
> >
> > But it's not true in ULP, so use mux_mask instead to address the
> > difference.
> 
> So you make the assumption that for all SHARE_MUX_CONF_REG SoCs, all bits
> in the register except mux ones are config bits.  You at least need to
> mention that in the commit log, IMO.

Yes, it's true, at least for all current known SoCs.
But your suggestion is good, I can add it in commit log.

Thanks

Regards
Dong Aisheng

> 
> Shawn
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/drivers/pinctrl/freescale/pinctrl-imx.c b/drivers/pinctrl/freescale/pinctrl-imx.c
index db76e9d..8347197 100644
--- a/drivers/pinctrl/freescale/pinctrl-imx.c
+++ b/drivers/pinctrl/freescale/pinctrl-imx.c
@@ -197,8 +197,8 @@  static int imx_pmx_set(struct pinctrl_dev *pctldev, unsigned selector,
 		if (info->flags & SHARE_MUX_CONF_REG) {
 			u32 reg;
 			reg = readl(ipctl->base + pin_reg->mux_reg);
-			reg &= ~(0x7 << 20);
-			reg |= (pin->mux_mode << 20);
+			reg &= ~info->mux_mask;
+			reg |= (pin->mux_mode << info->mux_shift);
 			writel(reg, ipctl->base + pin_reg->mux_reg);
 		} else {
 			writel(pin->mux_mode, ipctl->base + pin_reg->mux_reg);
@@ -288,7 +288,7 @@  static int imx_pmx_gpio_request_enable(struct pinctrl_dev *pctldev,
 
 mux_pin:
 	reg = readl(ipctl->base + pin_reg->mux_reg);
-	reg &= ~(0x7 << 20);
+	reg &= ~info->mux_mask;
 	reg |= imx_pin->config;
 	writel(reg, ipctl->base + pin_reg->mux_reg);
 
@@ -432,7 +432,7 @@  static int imx_pinconf_get(struct pinctrl_dev *pctldev,
 	*config = readl(ipctl->base + pin_reg->conf_reg);
 
 	if (info->flags & SHARE_MUX_CONF_REG)
-		*config &= 0xffff;
+		*config &= ~info->mux_mask;
 
 	return 0;
 }
@@ -459,7 +459,7 @@  static int imx_pinconf_set(struct pinctrl_dev *pctldev,
 		if (info->flags & SHARE_MUX_CONF_REG) {
 			u32 reg;
 			reg = readl(ipctl->base + pin_reg->conf_reg);
-			reg &= ~0xffff;
+			reg &= info->mux_mask;
 			reg |= configs[i];
 			writel(reg, ipctl->base + pin_reg->conf_reg);
 		} else {
diff --git a/drivers/pinctrl/freescale/pinctrl-imx.h b/drivers/pinctrl/freescale/pinctrl-imx.h
index b5c8fe1..eb0ce95 100644
--- a/drivers/pinctrl/freescale/pinctrl-imx.h
+++ b/drivers/pinctrl/freescale/pinctrl-imx.h
@@ -64,6 +64,10 @@  struct imx_pinctrl_soc_info {
 	const char *gpr_compatible;
 	struct mutex mutex;
 
+	/* MUX_MODE shift and mask in case SHARE_MUX_CONF_REG */
+	unsigned int mux_mask;
+	u8 mux_shift;
+
 	/* generic pinconf */
 	bool generic_pinconf;
 	const struct pinconf_generic_params *custom_params;
diff --git a/drivers/pinctrl/freescale/pinctrl-vf610.c b/drivers/pinctrl/freescale/pinctrl-vf610.c
index 2b1e198..3bd8556 100644
--- a/drivers/pinctrl/freescale/pinctrl-vf610.c
+++ b/drivers/pinctrl/freescale/pinctrl-vf610.c
@@ -299,6 +299,8 @@  static struct imx_pinctrl_soc_info vf610_pinctrl_info = {
 	.pins = vf610_pinctrl_pads,
 	.npins = ARRAY_SIZE(vf610_pinctrl_pads),
 	.flags = SHARE_MUX_CONF_REG | ZERO_OFFSET_VALID,
+	.mux_mask = 0x700000,
+	.mux_shift = 20,
 };
 
 static const struct of_device_id vf610_pinctrl_of_match[] = {