diff mbox

[v2,5/8] pinctrl: freescale: imx: add ZERO_OFFSET_VALID flag

Message ID 1441147753-13239-5-git-send-email-aalonso@freescale.com
State New
Headers show

Commit Message

Adrian Alonso Sept. 1, 2015, 10:49 p.m. UTC 
- Add ZERO_OFFSET_VALID flag, on imx7d mux_conf reg
  offset is zero for iomuxc-lspr controller
- Do default initialization on parse group function.

Signed-off-by: Adrian Alonso <aalonso@freescale.com>
---
Changes for V2: Resend

 drivers/pinctrl/freescale/pinctrl-imx.c | 23 +++++++++++++----------
 drivers/pinctrl/freescale/pinctrl-imx.h |  1 +
 2 files changed, 14 insertions(+), 10 deletions(-)

Comments

Shawn Guo Sept. 7, 2015, 1:28 a.m. UTC | #1 
On Tue, Sep 01, 2015 at 05:49:10PM -0500, Adrian Alonso wrote:
> - Add ZERO_OFFSET_VALID flag, on imx7d mux_conf reg
>   offset is zero for iomuxc-lspr controller
> - Do default initialization on parse group function.
> 
> Signed-off-by: Adrian Alonso <aalonso@freescale.com>

I do not follow why this patch is needed at all.  All the register
validity check are done against -1, and zero offset is already supported
by the driver in the current form.  Or am I missing anything?

Shawn

> ---
> Changes for V2: Resend
> 
>  drivers/pinctrl/freescale/pinctrl-imx.c | 23 +++++++++++++----------
>  drivers/pinctrl/freescale/pinctrl-imx.h |  1 +
>  2 files changed, 14 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/pinctrl/freescale/pinctrl-imx.c b/drivers/pinctrl/freescale/pinctrl-imx.c
> index 95db9e8..9f019be 100644
> --- a/drivers/pinctrl/freescale/pinctrl-imx.c
> +++ b/drivers/pinctrl/freescale/pinctrl-imx.c
> @@ -437,7 +437,7 @@ static void imx_pinconf_dbg_show(struct pinctrl_dev *pctldev,
>  	const struct imx_pin_reg *pin_reg = &info->pin_regs[pin_id];
>  	unsigned long config;
>  
> -	if (!pin_reg || pin_reg->conf_reg == -1) {
> +	if (pin_reg->conf_reg == -1) {
>  		seq_printf(s, "N/A");
>  		return;
>  	}
> @@ -536,21 +536,29 @@ static int imx_pinctrl_parse_groups(struct device_node *np,
>  		return -ENOMEM;
>  
>  	for (i = 0; i < grp->npins; i++) {
> -		u32 mux_reg = be32_to_cpu(*list++);
> +		u32 mux_reg;
>  		u32 conf_reg;
>  		unsigned int pin_id;
>  		struct imx_pin_reg *pin_reg;
>  		struct imx_pin *pin = &grp->pins[i];
>  
> +		mux_reg = be32_to_cpu(*list++);
> +		if (!(info->flags & ZERO_OFFSET_VALID) && !mux_reg)
> +			mux_reg = -1;
> +
>  		if (info->flags & SHARE_MUX_CONF_REG) {
>  			conf_reg = mux_reg;
>  		} else {
>  			conf_reg = be32_to_cpu(*list++);
> -			if (!conf_reg)
> +			if (!(info->flags & ZERO_OFFSET_VALID) && !conf_reg)
>  				conf_reg = -1;
>  		}
>  
> -		pin_id = mux_reg ? mux_reg / 4 : conf_reg / 4;
> +		if (info->flags & ZERO_OFFSET_VALID)
> +			pin_id = mux_reg / 4;
> +		else
> +			pin_id = mux_reg ? mux_reg / 4 : conf_reg / 4;
> +
>  		pin_reg = &info->pin_regs[pin_id];
>  		pin->pin = pin_id;
>  		grp->pin_ids[i] = pin_id;
> @@ -684,7 +692,7 @@ int imx_pinctrl_probe(struct platform_device *pdev,
>  {
>  	struct imx_pinctrl *ipctl;
>  	struct resource *res;
> -	int ret, i;
> +	int ret;
>  
>  	if (!info || !info->pins || !info->npins) {
>  		dev_err(&pdev->dev, "wrong pinctrl info\n");
> @@ -702,11 +710,6 @@ int imx_pinctrl_probe(struct platform_device *pdev,
>  	if (!info->pin_regs)
>  		return -ENOMEM;
>  
> -	for (i = 0; i < info->npins; i++) {
> -		info->pin_regs[i].mux_reg = -1;
> -		info->pin_regs[i].conf_reg = -1;
> -	}
> -
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	ipctl->base = devm_ioremap_resource(&pdev->dev, res);
>  	if (IS_ERR(ipctl->base))
> diff --git a/drivers/pinctrl/freescale/pinctrl-imx.h b/drivers/pinctrl/freescale/pinctrl-imx.h
> index 26f8f1c..67c07c2 100644
> --- a/drivers/pinctrl/freescale/pinctrl-imx.h
> +++ b/drivers/pinctrl/freescale/pinctrl-imx.h
> @@ -85,6 +85,7 @@ struct imx_pinctrl_soc_info {
>  };
>  
>  #define SHARE_MUX_CONF_REG	0x1
> +#define ZERO_OFFSET_VALID	0x2
>  
>  #define NO_MUX		0x0
>  #define NO_PAD		0x0
> -- 
> 2.1.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
Adrian Alonso Sept. 8, 2015, 4:05 p.m. UTC | #2 
Hi Shawn,

See comments below

Regards
Shawn Guo Sept. 18, 2015, 1:52 p.m. UTC | #3 
On Tue, Sep 08, 2015 at 04:05:32PM +0000, Alonso Adrian wrote:
> Hi Shawn,
> 
> See comments below

Please use bottom posting, not top posting.

...

> > @@ -536,21 +536,29 @@ static int imx_pinctrl_parse_groups(struct device_node *np,
> >               return -ENOMEM;
> >
> >       for (i = 0; i < grp->npins; i++) {
> > -             u32 mux_reg = be32_to_cpu(*list++);
> > +             u32 mux_reg;
> >               u32 conf_reg;
> >               unsigned int pin_id;
> >               struct imx_pin_reg *pin_reg;
> >               struct imx_pin *pin = &grp->pins[i];
> >
> > +             mux_reg = be32_to_cpu(*list++);
> > +             if (!(info->flags & ZERO_OFFSET_VALID) && !mux_reg)
> > +                     mux_reg = -1;
> > +
> >               if (info->flags & SHARE_MUX_CONF_REG) {
> >                       conf_reg = mux_reg;
> >               } else {
> >                       conf_reg = be32_to_cpu(*list++);
> > -                     if (!conf_reg)
> > +                     if (!(info->flags & ZERO_OFFSET_VALID) && !conf_reg)
> >                               conf_reg = -1;
> >               }
> >
> [Adrian] The below assignment is the important thing that this patch does,
> it allows the pad_id to be zero and match the pad id's according to the mux_reg offset.
> > -             pin_id = mux_reg ? mux_reg / 4 : conf_reg / 4;
> > +             if (info->flags & ZERO_OFFSET_VALID)
> > +                     pin_id = mux_reg / 4;
> > +             else
> > +                     pin_id = mux_reg ? mux_reg / 4 : conf_reg / 4;
> > +

Ah, yes.  So the only code change we need is just the line below, right?

	pin_id = (mux_reg != -1) ? mux_reg / 4 : conf_reg / 4;

Shawn

> >               pin_reg = &info->pin_regs[pin_id];
> >               pin->pin = pin_id;
> >               grp->pin_ids[i] = pin_id;
> > @@ -684,7 +692,7 @@ int imx_pinctrl_probe(struct platform_device *pdev,
> >  {
> >       struct imx_pinctrl *ipctl;
> >       struct resource *res;
> > -     int ret, i;
> > +     int ret;
> >
> >       if (!info || !info->pins || !info->npins) {
> >               dev_err(&pdev->dev, "wrong pinctrl info\n");
> > @@ -702,11 +710,6 @@ int imx_pinctrl_probe(struct platform_device *pdev,
> >       if (!info->pin_regs)
> >               return -ENOMEM;
> >
> > -     for (i = 0; i < info->npins; i++) {
> > -             info->pin_regs[i].mux_reg = -1;
> > -             info->pin_regs[i].conf_reg = -1;
> > -     }
> > -
> >       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >       ipctl->base = devm_ioremap_resource(&pdev->dev, res);
> >       if (IS_ERR(ipctl->base))
> > diff --git a/drivers/pinctrl/freescale/pinctrl-imx.h b/drivers/pinctrl/freescale/pinctrl-imx.h
> > index 26f8f1c..67c07c2 100644
> > --- a/drivers/pinctrl/freescale/pinctrl-imx.h
> > +++ b/drivers/pinctrl/freescale/pinctrl-imx.h
> > @@ -85,6 +85,7 @@ struct imx_pinctrl_soc_info {
> >  };
> >
> >  #define SHARE_MUX_CONF_REG   0x1
> > +#define ZERO_OFFSET_VALID    0x2
> >
> >  #define NO_MUX               0x0
> >  #define NO_PAD               0x0
> > --
> > 2.1.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
Adrian Alonso Sept. 18, 2015, 4:27 p.m. UTC | #4 
> -----Original Message-----
> From: Shawn Guo [mailto:shawnguo@kernel.org]
> Sent: Friday, September 18, 2015 8:53 AM
> To: Alonso Lazcano Adrian-B38018 <aalonso@freescale.com>
> Cc: linux-arm-kernel@lists.infradead.org; shawn.guo@linaro.org;
> linus.walleij@linaro.org; lznuaa@gmail.com; devicetree@vger.kernel.org; Li
> Frank-B20596 <Frank.Li@freescale.com>; Garg Nitin-B37173
> <nitin.garg@freescale.com>; Huang Yongcai-B20788
> <Anson.Huang@freescale.com>; linux-gpio@vger.kernel.org;
> robh+dt@kernel.org; Gong Yibin-B38343 <yibin.gong@freescale.com>
> Subject: Re: [PATCH v2 5/8] pinctrl: freescale: imx: add ZERO_OFFSET_VALID
> flag
> 
> On Tue, Sep 08, 2015 at 04:05:32PM +0000, Alonso Adrian wrote:
> > Hi Shawn,
> >
> > See comments below
> 
> Please use bottom posting, not top posting.
> 
> ...
> 
> > > @@ -536,21 +536,29 @@ static int imx_pinctrl_parse_groups(struct
> device_node *np,
> > >               return -ENOMEM;
> > >
> > >       for (i = 0; i < grp->npins; i++) {
> > > -             u32 mux_reg = be32_to_cpu(*list++);
> > > +             u32 mux_reg;
> > >               u32 conf_reg;
> > >               unsigned int pin_id;
> > >               struct imx_pin_reg *pin_reg;
> > >               struct imx_pin *pin = &grp->pins[i];
> > >
> > > +             mux_reg = be32_to_cpu(*list++);
> > > +             if (!(info->flags & ZERO_OFFSET_VALID) && !mux_reg)
> > > +                     mux_reg = -1;
> > > +
> > >               if (info->flags & SHARE_MUX_CONF_REG) {
> > >                       conf_reg = mux_reg;
> > >               } else {
> > >                       conf_reg = be32_to_cpu(*list++);
> > > -                     if (!conf_reg)
> > > +                     if (!(info->flags & ZERO_OFFSET_VALID) &&
> > > + !conf_reg)
> > >                               conf_reg = -1;
> > >               }
> > >
> > [Adrian] The below assignment is the important thing that this patch
> > does, it allows the pad_id to be zero and match the pad id's according to the
> mux_reg offset.
> > > -             pin_id = mux_reg ? mux_reg / 4 : conf_reg / 4;
> > > +             if (info->flags & ZERO_OFFSET_VALID)
> > > +                     pin_id = mux_reg / 4;
> > > +             else
> > > +                     pin_id = mux_reg ? mux_reg / 4 : conf_reg / 4;
> > > +
> 
> Ah, yes.  So the only code change we need is just the line below, right?
> 
> 	pin_id = (mux_reg != -1) ? mux_reg / 4 : conf_reg / 4;
> 
> Shawn
[Adrian] Yes that will work, let me resend a new patch series with this change, Thanks.
> 
> > >               pin_reg = &info->pin_regs[pin_id];
> > >               pin->pin = pin_id;
> > >               grp->pin_ids[i] = pin_id; @@ -684,7 +692,7 @@ int
> > > imx_pinctrl_probe(struct platform_device *pdev,  {
> > >       struct imx_pinctrl *ipctl;
> > >       struct resource *res;
> > > -     int ret, i;
> > > +     int ret;
> > >
> > >       if (!info || !info->pins || !info->npins) {
> > >               dev_err(&pdev->dev, "wrong pinctrl info\n"); @@
> > > -702,11 +710,6 @@ int imx_pinctrl_probe(struct platform_device *pdev,
> > >       if (!info->pin_regs)
> > >               return -ENOMEM;
> > >
> > > -     for (i = 0; i < info->npins; i++) {
> > > -             info->pin_regs[i].mux_reg = -1;
> > > -             info->pin_regs[i].conf_reg = -1;
> > > -     }
> > > -
> > >       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > >       ipctl->base = devm_ioremap_resource(&pdev->dev, res);
> > >       if (IS_ERR(ipctl->base))
> > > diff --git a/drivers/pinctrl/freescale/pinctrl-imx.h
> > > b/drivers/pinctrl/freescale/pinctrl-imx.h
> > > index 26f8f1c..67c07c2 100644
> > > --- a/drivers/pinctrl/freescale/pinctrl-imx.h
> > > +++ b/drivers/pinctrl/freescale/pinctrl-imx.h
> > > @@ -85,6 +85,7 @@ struct imx_pinctrl_soc_info {  };
> > >
> > >  #define SHARE_MUX_CONF_REG   0x1
> > > +#define ZERO_OFFSET_VALID    0x2
> > >
> > >  #define NO_MUX               0x0
> > >  #define NO_PAD               0x0
> > > --
> > > 2.1.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
diff mbox

Patch

diff --git a/drivers/pinctrl/freescale/pinctrl-imx.c b/drivers/pinctrl/freescale/pinctrl-imx.c
index 95db9e8..9f019be 100644
--- a/drivers/pinctrl/freescale/pinctrl-imx.c
+++ b/drivers/pinctrl/freescale/pinctrl-imx.c
@@ -437,7 +437,7 @@  static void imx_pinconf_dbg_show(struct pinctrl_dev *pctldev,
 	const struct imx_pin_reg *pin_reg = &info->pin_regs[pin_id];
 	unsigned long config;
 
-	if (!pin_reg || pin_reg->conf_reg == -1) {
+	if (pin_reg->conf_reg == -1) {
 		seq_printf(s, "N/A");
 		return;
 	}
@@ -536,21 +536,29 @@  static int imx_pinctrl_parse_groups(struct device_node *np,
 		return -ENOMEM;
 
 	for (i = 0; i < grp->npins; i++) {
-		u32 mux_reg = be32_to_cpu(*list++);
+		u32 mux_reg;
 		u32 conf_reg;
 		unsigned int pin_id;
 		struct imx_pin_reg *pin_reg;
 		struct imx_pin *pin = &grp->pins[i];
 
+		mux_reg = be32_to_cpu(*list++);
+		if (!(info->flags & ZERO_OFFSET_VALID) && !mux_reg)
+			mux_reg = -1;
+
 		if (info->flags & SHARE_MUX_CONF_REG) {
 			conf_reg = mux_reg;
 		} else {
 			conf_reg = be32_to_cpu(*list++);
-			if (!conf_reg)
+			if (!(info->flags & ZERO_OFFSET_VALID) && !conf_reg)
 				conf_reg = -1;
 		}
 
-		pin_id = mux_reg ? mux_reg / 4 : conf_reg / 4;
+		if (info->flags & ZERO_OFFSET_VALID)
+			pin_id = mux_reg / 4;
+		else
+			pin_id = mux_reg ? mux_reg / 4 : conf_reg / 4;
+
 		pin_reg = &info->pin_regs[pin_id];
 		pin->pin = pin_id;
 		grp->pin_ids[i] = pin_id;
@@ -684,7 +692,7 @@  int imx_pinctrl_probe(struct platform_device *pdev,
 {
 	struct imx_pinctrl *ipctl;
 	struct resource *res;
-	int ret, i;
+	int ret;
 
 	if (!info || !info->pins || !info->npins) {
 		dev_err(&pdev->dev, "wrong pinctrl info\n");
@@ -702,11 +710,6 @@  int imx_pinctrl_probe(struct platform_device *pdev,
 	if (!info->pin_regs)
 		return -ENOMEM;
 
-	for (i = 0; i < info->npins; i++) {
-		info->pin_regs[i].mux_reg = -1;
-		info->pin_regs[i].conf_reg = -1;
-	}
-
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	ipctl->base = devm_ioremap_resource(&pdev->dev, res);
 	if (IS_ERR(ipctl->base))
diff --git a/drivers/pinctrl/freescale/pinctrl-imx.h b/drivers/pinctrl/freescale/pinctrl-imx.h
index 26f8f1c..67c07c2 100644
--- a/drivers/pinctrl/freescale/pinctrl-imx.h
+++ b/drivers/pinctrl/freescale/pinctrl-imx.h
@@ -85,6 +85,7 @@  struct imx_pinctrl_soc_info {
 };
 
 #define SHARE_MUX_CONF_REG	0x1
+#define ZERO_OFFSET_VALID	0x2
 
 #define NO_MUX		0x0
 #define NO_PAD		0x0