| Message ID | 1494829706-5661-3-git-send-email-aisheng.dong@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 2017-05-14 23:28, Dong Aisheng wrote: > The Rapid General-Purpose Input and Output with 2 Ports (RGPIO2P) > on MX7ULP is similar to GPIO on Vibrid. But unlike Vibrid, the > RGPIO2P has an extra Port Data Direction Register (PDDR) used > to configure the individual port pins for input or output. > > We introduce a FSL_GPIO_HAVE_PDDR with fsl_gpio_soc_data data > to distinguish this differences. And we support getting the output > status by checking the GPIO direction in PDDR. > > Cc: Linus Walleij <linus.walleij@linaro.org> > Cc: Alexandre Courbot <gnurou@gmail.com> > Cc: Shawn Guo <shawnguo@kernel.org> > Cc: Stefan Agner <stefan@agner.ch> > Cc: Fugang Duan <fugang.duan@nxp.com> > Cc: Peter Chen <peter.chen@nxp.com> > Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com> > --- > drivers/gpio/gpio-vf610.c | 49 ++++++++++++++++++++++++++++++++++++++++++++--- > 1 file changed, 46 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpio/gpio-vf610.c b/drivers/gpio/gpio-vf610.c > index 521fbe3..a439d27 100644 > --- a/drivers/gpio/gpio-vf610.c > +++ b/drivers/gpio/gpio-vf610.c > @@ -30,10 +30,15 @@ > > #define VF610_GPIO_PER_PORT 32 > > +struct fsl_gpio_soc_data { > + u32 flags; > +}; > + > struct vf610_gpio_port { > struct gpio_chip gc; > void __iomem *base; > void __iomem *gpio_base; > + const struct fsl_gpio_soc_data *sdata; > u8 irqc[VF610_GPIO_PER_PORT]; > int irq; > }; > @@ -43,6 +48,7 @@ struct vf610_gpio_port { > #define GPIO_PCOR 0x08 > #define GPIO_PTOR 0x0c > #define GPIO_PDIR 0x10 > +#define GPIO_PDDR 0x14 > > #define PORT_PCR(n) ((n) * 0x4) > #define PORT_PCR_IRQC_OFFSET 16 > @@ -59,10 +65,18 @@ struct vf610_gpio_port { > #define PORT_INT_EITHER_EDGE 0xb > #define PORT_INT_LOGIC_ONE 0xc > > +/* SoC has a Port Data Direction Register (PDDR) */ > +#define FSL_GPIO_HAVE_PDDR BIT(0) > + > static struct irq_chip vf610_gpio_irq_chip; > > +static const struct fsl_gpio_soc_data imx_data = { > + .flags = FSL_GPIO_HAVE_PDDR, > +}; > + > static const struct of_device_id vf610_gpio_dt_ids[] = { > - { .compatible = "fsl,vf610-gpio" }, > + { .compatible = "fsl,vf610-gpio", .data = NULL, }, > + { .compatible = "fsl,imx7ulp-gpio", .data = &imx_data, }, > { /* sentinel */ } > }; > > @@ -79,8 +93,18 @@ static inline u32 vf610_gpio_readl(void __iomem *reg) > static int vf610_gpio_get(struct gpio_chip *gc, unsigned int gpio) > { > struct vf610_gpio_port *port = gpiochip_get_data(gc); > - > - return !!(vf610_gpio_readl(port->gpio_base + GPIO_PDIR) & BIT(gpio)); > + unsigned long mask = BIT(gpio); > + void __iomem *addr; > + > + if (port->sdata && port->sdata->flags & FSL_GPIO_HAVE_PDDR) { > + mask &= vf610_gpio_readl(port->gpio_base + GPIO_PDDR); > + addr = mask ? port->gpio_base + GPIO_PDOR : > + port->gpio_base + GPIO_PDIR; > + return !!(vf610_gpio_readl(addr) & BIT(gpio)); > + } else { > + return !!(vf610_gpio_readl(port->gpio_base + GPIO_PDIR) > + & BIT(gpio)); > + } I would rather prefer to read the actual value on the wire... > } > > static void vf610_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val) > @@ -96,12 +120,28 @@ static void vf610_gpio_set(struct gpio_chip *gc, > unsigned int gpio, int val) > > static int vf610_gpio_direction_input(struct gpio_chip *chip, unsigned gpio) > { > + struct vf610_gpio_port *port = gpiochip_get_data(chip); > + unsigned long mask = BIT(gpio); > + u32 val; > + > + if (port->sdata && port->sdata->flags & FSL_GPIO_HAVE_PDDR) { > + val = vf610_gpio_readl(port->gpio_base + GPIO_PDDR); > + val &= ~mask; > + vf610_gpio_writel(val, port->gpio_base + GPIO_PDDR); As mentioned in the pinctrl patchset review, if we can do this orthogonal, I would rather prefer to not involve pinctrl here in the i.MX 7ULP case. So I would make sure that pinctrl_gpio_direction_input/output does not get called at all (e.g. returning in this if case)... -- Stefan > + } > + > return pinctrl_gpio_direction_input(chip->base + gpio); > } > > static int vf610_gpio_direction_output(struct gpio_chip *chip, unsigned gpio, > int value) > { > + struct vf610_gpio_port *port = gpiochip_get_data(chip); > + unsigned long mask = BIT(gpio); > + > + if (port->sdata && port->sdata->flags & FSL_GPIO_HAVE_PDDR) > + vf610_gpio_writel(mask, port->gpio_base + GPIO_PDDR); > + > vf610_gpio_set(chip, gpio, value); > > return pinctrl_gpio_direction_output(chip->base + gpio); > @@ -216,6 +256,8 @@ static struct irq_chip vf610_gpio_irq_chip = { > > static int vf610_gpio_probe(struct platform_device *pdev) > { > + const struct of_device_id *of_id = of_match_device(vf610_gpio_dt_ids, > + &pdev->dev); > struct device *dev = &pdev->dev; > struct device_node *np = dev->of_node; > struct vf610_gpio_port *port; > @@ -227,6 +269,7 @@ static int vf610_gpio_probe(struct platform_device *pdev) > if (!port) > return -ENOMEM; > > + port->sdata = of_id->data; > iores = platform_get_resource(pdev, IORESOURCE_MEM, 0); > port->base = devm_ioremap_resource(dev, iores); > if (IS_ERR(port->base))
Hi Stefan, > -----Original Message----- > From: Stefan Agner [mailto:stefan@agner.ch] > Sent: Tuesday, May 16, 2017 1:59 AM > To: A.S. Dong > Cc: linux-gpio@vger.kernel.org; linux-arm-kernel@lists.infradead.org; > linus.walleij@linaro.org; shawnguo@kernel.org; Jacky Bai; Andy Duan; > kernel@pengutronix.de; Alexandre Courbot; Peter Chen > Subject: Re: [PATCH 2/2] gpio: gpio-vf610: add imx7ulp support > > On 2017-05-14 23:28, Dong Aisheng wrote: > > The Rapid General-Purpose Input and Output with 2 Ports (RGPIO2P) on > > MX7ULP is similar to GPIO on Vibrid. But unlike Vibrid, the RGPIO2P > > has an extra Port Data Direction Register (PDDR) used to configure the > > individual port pins for input or output. > > > > We introduce a FSL_GPIO_HAVE_PDDR with fsl_gpio_soc_data data to > > distinguish this differences. And we support getting the output status > > by checking the GPIO direction in PDDR. > > > > Cc: Linus Walleij <linus.walleij@linaro.org> > > Cc: Alexandre Courbot <gnurou@gmail.com> > > Cc: Shawn Guo <shawnguo@kernel.org> > > Cc: Stefan Agner <stefan@agner.ch> > > Cc: Fugang Duan <fugang.duan@nxp.com> > > Cc: Peter Chen <peter.chen@nxp.com> > > Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com> > > --- > > drivers/gpio/gpio-vf610.c | 49 > > ++++++++++++++++++++++++++++++++++++++++++++--- > > 1 file changed, 46 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpio/gpio-vf610.c b/drivers/gpio/gpio-vf610.c > > index 521fbe3..a439d27 100644 > > --- a/drivers/gpio/gpio-vf610.c > > +++ b/drivers/gpio/gpio-vf610.c > > @@ -30,10 +30,15 @@ > > > > #define VF610_GPIO_PER_PORT 32 > > > > +struct fsl_gpio_soc_data { > > + u32 flags; > > +}; > > + > > struct vf610_gpio_port { > > struct gpio_chip gc; > > void __iomem *base; > > void __iomem *gpio_base; > > + const struct fsl_gpio_soc_data *sdata; > > u8 irqc[VF610_GPIO_PER_PORT]; > > int irq; > > }; > > @@ -43,6 +48,7 @@ struct vf610_gpio_port { > > #define GPIO_PCOR 0x08 > > #define GPIO_PTOR 0x0c > > #define GPIO_PDIR 0x10 > > +#define GPIO_PDDR 0x14 > > > > #define PORT_PCR(n) ((n) * 0x4) > > #define PORT_PCR_IRQC_OFFSET 16 > > @@ -59,10 +65,18 @@ struct vf610_gpio_port { > > #define PORT_INT_EITHER_EDGE 0xb > > #define PORT_INT_LOGIC_ONE 0xc > > > > +/* SoC has a Port Data Direction Register (PDDR) */ > > +#define FSL_GPIO_HAVE_PDDR BIT(0) > > + > > static struct irq_chip vf610_gpio_irq_chip; > > > > +static const struct fsl_gpio_soc_data imx_data = { > > + .flags = FSL_GPIO_HAVE_PDDR, > > +}; > > + > > static const struct of_device_id vf610_gpio_dt_ids[] = { > > - { .compatible = "fsl,vf610-gpio" }, > > + { .compatible = "fsl,vf610-gpio", .data = NULL, }, > > + { .compatible = "fsl,imx7ulp-gpio", .data = &imx_data, }, > > { /* sentinel */ } > > }; > > > > @@ -79,8 +93,18 @@ static inline u32 vf610_gpio_readl(void __iomem > > *reg) static int vf610_gpio_get(struct gpio_chip *gc, unsigned int > > gpio) { > > struct vf610_gpio_port *port = gpiochip_get_data(gc); > > - > > - return !!(vf610_gpio_readl(port->gpio_base + GPIO_PDIR) & BIT(gpio)); > > + unsigned long mask = BIT(gpio); > > + void __iomem *addr; > > + > > + if (port->sdata && port->sdata->flags & FSL_GPIO_HAVE_PDDR) { > > + mask &= vf610_gpio_readl(port->gpio_base + GPIO_PDDR); > > + addr = mask ? port->gpio_base + GPIO_PDOR : > > + port->gpio_base + GPIO_PDIR; > > + return !!(vf610_gpio_readl(addr) & BIT(gpio)); > > + } else { > > + return !!(vf610_gpio_readl(port->gpio_base + GPIO_PDIR) > > + & BIT(gpio)); > > + } > > I would rather prefer to read the actual value on the wire... > > > } > > > > static void vf610_gpio_set(struct gpio_chip *gc, unsigned int gpio, > > int val) @@ -96,12 +120,28 @@ static void vf610_gpio_set(struct > > gpio_chip *gc, unsigned int gpio, int val) > > > > static int vf610_gpio_direction_input(struct gpio_chip *chip, > > unsigned gpio) { > > + struct vf610_gpio_port *port = gpiochip_get_data(chip); > > + unsigned long mask = BIT(gpio); > > + u32 val; > > + > > + if (port->sdata && port->sdata->flags & FSL_GPIO_HAVE_PDDR) { > > + val = vf610_gpio_readl(port->gpio_base + GPIO_PDDR); > > + val &= ~mask; > > + vf610_gpio_writel(val, port->gpio_base + GPIO_PDDR); > > As mentioned in the pinctrl patchset review, if we can do this orthogonal, > I would rather prefer to not involve pinctrl here in the i.MX 7ULP case. > So I would make sure that pinctrl_gpio_direction_input/output does not get > called at all (e.g. > returning in this if case)... > As we already discussed and decided to remove pinctrl_gpio_enable/disable and only keep pinctrl_gpio_direction_input/output for both VF610 and ULP [1], and ULP will read PDOR as it has FSL_GPIO_HAVE_PDDR flag, so there will be no changes for this series. Would you help double review and give a tag if it's ok for you? [1] [PATCH V4 0/7] pinctrl: imx: add imx7ulp pinctrl support https://www.spinics.net/lists/arm-kernel/msg589700.html Regards Dong Aisheng > -- > Stefan > > > + } > > + > > return pinctrl_gpio_direction_input(chip->base + gpio); } > > > > static int vf610_gpio_direction_output(struct gpio_chip *chip, unsigned > gpio, > > int value) > > { > > + struct vf610_gpio_port *port = gpiochip_get_data(chip); > > + unsigned long mask = BIT(gpio); > > + > > + if (port->sdata && port->sdata->flags & FSL_GPIO_HAVE_PDDR) > > + vf610_gpio_writel(mask, port->gpio_base + GPIO_PDDR); > > + > > vf610_gpio_set(chip, gpio, value); > > > > return pinctrl_gpio_direction_output(chip->base + gpio); @@ -216,6 > > +256,8 @@ static struct irq_chip vf610_gpio_irq_chip = { > > > > static int vf610_gpio_probe(struct platform_device *pdev) { > > + const struct of_device_id *of_id = of_match_device(vf610_gpio_dt_ids, > > + &pdev->dev); > > struct device *dev = &pdev->dev; > > struct device_node *np = dev->of_node; > > struct vf610_gpio_port *port; > > @@ -227,6 +269,7 @@ static int vf610_gpio_probe(struct platform_device > *pdev) > > if (!port) > > return -ENOMEM; > > > > + port->sdata = of_id->data; > > iores = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > port->base = devm_ioremap_resource(dev, iores); > > if (IS_ERR(port->base))
On 2017-06-21 06:30, A.s. Dong wrote: > Hi Stefan, > >> -----Original Message----- >> From: Stefan Agner [mailto:stefan@agner.ch] >> Sent: Tuesday, May 16, 2017 1:59 AM >> To: A.S. Dong >> Cc: linux-gpio@vger.kernel.org; linux-arm-kernel@lists.infradead.org; >> linus.walleij@linaro.org; shawnguo@kernel.org; Jacky Bai; Andy Duan; >> kernel@pengutronix.de; Alexandre Courbot; Peter Chen >> Subject: Re: [PATCH 2/2] gpio: gpio-vf610: add imx7ulp support >> >> On 2017-05-14 23:28, Dong Aisheng wrote: >> > The Rapid General-Purpose Input and Output with 2 Ports (RGPIO2P) on >> > MX7ULP is similar to GPIO on Vibrid. But unlike Vibrid, the RGPIO2P >> > has an extra Port Data Direction Register (PDDR) used to configure the >> > individual port pins for input or output. >> > >> > We introduce a FSL_GPIO_HAVE_PDDR with fsl_gpio_soc_data data to >> > distinguish this differences. And we support getting the output status >> > by checking the GPIO direction in PDDR. >> > >> > Cc: Linus Walleij <linus.walleij@linaro.org> >> > Cc: Alexandre Courbot <gnurou@gmail.com> >> > Cc: Shawn Guo <shawnguo@kernel.org> >> > Cc: Stefan Agner <stefan@agner.ch> >> > Cc: Fugang Duan <fugang.duan@nxp.com> >> > Cc: Peter Chen <peter.chen@nxp.com> >> > Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com> >> > --- >> > drivers/gpio/gpio-vf610.c | 49 >> > ++++++++++++++++++++++++++++++++++++++++++++--- >> > 1 file changed, 46 insertions(+), 3 deletions(-) >> > >> > diff --git a/drivers/gpio/gpio-vf610.c b/drivers/gpio/gpio-vf610.c >> > index 521fbe3..a439d27 100644 >> > --- a/drivers/gpio/gpio-vf610.c >> > +++ b/drivers/gpio/gpio-vf610.c >> > @@ -30,10 +30,15 @@ >> > >> > #define VF610_GPIO_PER_PORT 32 >> > >> > +struct fsl_gpio_soc_data { >> > + u32 flags; >> > +}; >> > + >> > struct vf610_gpio_port { >> > struct gpio_chip gc; >> > void __iomem *base; >> > void __iomem *gpio_base; >> > + const struct fsl_gpio_soc_data *sdata; >> > u8 irqc[VF610_GPIO_PER_PORT]; >> > int irq; >> > }; >> > @@ -43,6 +48,7 @@ struct vf610_gpio_port { >> > #define GPIO_PCOR 0x08 >> > #define GPIO_PTOR 0x0c >> > #define GPIO_PDIR 0x10 >> > +#define GPIO_PDDR 0x14 >> > >> > #define PORT_PCR(n) ((n) * 0x4) >> > #define PORT_PCR_IRQC_OFFSET 16 >> > @@ -59,10 +65,18 @@ struct vf610_gpio_port { >> > #define PORT_INT_EITHER_EDGE 0xb >> > #define PORT_INT_LOGIC_ONE 0xc >> > >> > +/* SoC has a Port Data Direction Register (PDDR) */ >> > +#define FSL_GPIO_HAVE_PDDR BIT(0) >> > + >> > static struct irq_chip vf610_gpio_irq_chip; >> > >> > +static const struct fsl_gpio_soc_data imx_data = { >> > + .flags = FSL_GPIO_HAVE_PDDR, >> > +}; >> > + >> > static const struct of_device_id vf610_gpio_dt_ids[] = { >> > - { .compatible = "fsl,vf610-gpio" }, >> > + { .compatible = "fsl,vf610-gpio", .data = NULL, }, >> > + { .compatible = "fsl,imx7ulp-gpio", .data = &imx_data, }, >> > { /* sentinel */ } >> > }; >> > >> > @@ -79,8 +93,18 @@ static inline u32 vf610_gpio_readl(void __iomem >> > *reg) static int vf610_gpio_get(struct gpio_chip *gc, unsigned int >> > gpio) { >> > struct vf610_gpio_port *port = gpiochip_get_data(gc); >> > - >> > - return !!(vf610_gpio_readl(port->gpio_base + GPIO_PDIR) & BIT(gpio)); >> > + unsigned long mask = BIT(gpio); >> > + void __iomem *addr; >> > + >> > + if (port->sdata && port->sdata->flags & FSL_GPIO_HAVE_PDDR) { >> > + mask &= vf610_gpio_readl(port->gpio_base + GPIO_PDDR); >> > + addr = mask ? port->gpio_base + GPIO_PDOR : >> > + port->gpio_base + GPIO_PDIR; >> > + return !!(vf610_gpio_readl(addr) & BIT(gpio)); I still feel it would be better to read back the actual value on the wire... But this would require to enable the input buffer in imx7ulp_pmx_gpio_set_direction. Lothar was with me in this discussion: https://patchwork.kernel.org/patch/9726163/ Or is there really a technical limitation on i.MX7ULP? We could also change it later, especially if the pinctrl changes already got merged anyway. From a Vybrid perspective: Acked-by: Stefan Agner <stefan@agner.ch> -- Stefan >> > + } else { >> > + return !!(vf610_gpio_readl(port->gpio_base + GPIO_PDIR) >> > + & BIT(gpio)); >> > + } >> >> I would rather prefer to read the actual value on the wire... >> >> > } >> > >> > static void vf610_gpio_set(struct gpio_chip *gc, unsigned int gpio, >> > int val) @@ -96,12 +120,28 @@ static void vf610_gpio_set(struct >> > gpio_chip *gc, unsigned int gpio, int val) >> > >> > static int vf610_gpio_direction_input(struct gpio_chip *chip, >> > unsigned gpio) { >> > + struct vf610_gpio_port *port = gpiochip_get_data(chip); >> > + unsigned long mask = BIT(gpio); >> > + u32 val; >> > + >> > + if (port->sdata && port->sdata->flags & FSL_GPIO_HAVE_PDDR) { >> > + val = vf610_gpio_readl(port->gpio_base + GPIO_PDDR); >> > + val &= ~mask; >> > + vf610_gpio_writel(val, port->gpio_base + GPIO_PDDR); >> >> As mentioned in the pinctrl patchset review, if we can do this orthogonal, >> I would rather prefer to not involve pinctrl here in the i.MX 7ULP case. >> So I would make sure that pinctrl_gpio_direction_input/output does not get >> called at all (e.g. >> returning in this if case)... >> > > As we already discussed and decided to remove pinctrl_gpio_enable/disable > and only keep pinctrl_gpio_direction_input/output for both VF610 and ULP [1], > and ULP will read PDOR as it has FSL_GPIO_HAVE_PDDR flag, so there will > be no changes for this series. > > Would you help double review and give a tag if it's ok for you? > > [1] [PATCH V4 0/7] pinctrl: imx: add imx7ulp pinctrl support > https://www.spinics.net/lists/arm-kernel/msg589700.html > > Regards > Dong Aisheng > >> -- >> Stefan >> >> > + } >> > + >> > return pinctrl_gpio_direction_input(chip->base + gpio); } >> > >> > static int vf610_gpio_direction_output(struct gpio_chip *chip, unsigned >> gpio, >> > int value) >> > { >> > + struct vf610_gpio_port *port = gpiochip_get_data(chip); >> > + unsigned long mask = BIT(gpio); >> > + >> > + if (port->sdata && port->sdata->flags & FSL_GPIO_HAVE_PDDR) >> > + vf610_gpio_writel(mask, port->gpio_base + GPIO_PDDR); >> > + >> > vf610_gpio_set(chip, gpio, value); >> > >> > return pinctrl_gpio_direction_output(chip->base + gpio); @@ -216,6 >> > +256,8 @@ static struct irq_chip vf610_gpio_irq_chip = { >> > >> > static int vf610_gpio_probe(struct platform_device *pdev) { >> > + const struct of_device_id *of_id = of_match_device(vf610_gpio_dt_ids, >> > + &pdev->dev); >> > struct device *dev = &pdev->dev; >> > struct device_node *np = dev->of_node; >> > struct vf610_gpio_port *port; >> > @@ -227,6 +269,7 @@ static int vf610_gpio_probe(struct platform_device >> *pdev) >> > if (!port) >> > return -ENOMEM; >> > >> > + port->sdata = of_id->data; >> > iores = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> > port->base = devm_ioremap_resource(dev, iores); >> > if (IS_ERR(port->base))
Hi Stefan, > -----Original Message----- > From: Stefan Agner [mailto:stefan@agner.ch] > Sent: Saturday, July 01, 2017 5:54 AM > To: A.s. Dong > Cc: linux-gpio@vger.kernel.org; linux-arm-kernel@lists.infradead.org; > linus.walleij@linaro.org; shawnguo@kernel.org; Jacky Bai; Andy Duan; > kernel@pengutronix.de; Alexandre Courbot; Peter Chen; LW@KARO- > electronics.de > Subject: Re: [PATCH 2/2] gpio: gpio-vf610: add imx7ulp support > > On 2017-06-21 06:30, A.s. Dong wrote: > > Hi Stefan, > > > >> -----Original Message----- > >> From: Stefan Agner [mailto:stefan@agner.ch] > >> Sent: Tuesday, May 16, 2017 1:59 AM > >> To: A.S. Dong > >> Cc: linux-gpio@vger.kernel.org; linux-arm-kernel@lists.infradead.org; > >> linus.walleij@linaro.org; shawnguo@kernel.org; Jacky Bai; Andy Duan; > >> kernel@pengutronix.de; Alexandre Courbot; Peter Chen > >> Subject: Re: [PATCH 2/2] gpio: gpio-vf610: add imx7ulp support > >> > >> On 2017-05-14 23:28, Dong Aisheng wrote: > >> > The Rapid General-Purpose Input and Output with 2 Ports (RGPIO2P) > >> > on MX7ULP is similar to GPIO on Vibrid. But unlike Vibrid, the > >> > RGPIO2P has an extra Port Data Direction Register (PDDR) used to > >> > configure the individual port pins for input or output. > >> > > >> > We introduce a FSL_GPIO_HAVE_PDDR with fsl_gpio_soc_data data to > >> > distinguish this differences. And we support getting the output > >> > status by checking the GPIO direction in PDDR. > >> > > >> > Cc: Linus Walleij <linus.walleij@linaro.org> > >> > Cc: Alexandre Courbot <gnurou@gmail.com> > >> > Cc: Shawn Guo <shawnguo@kernel.org> > >> > Cc: Stefan Agner <stefan@agner.ch> > >> > Cc: Fugang Duan <fugang.duan@nxp.com> > >> > Cc: Peter Chen <peter.chen@nxp.com> > >> > Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com> > >> > --- > >> > drivers/gpio/gpio-vf610.c | 49 > >> > ++++++++++++++++++++++++++++++++++++++++++++--- > >> > 1 file changed, 46 insertions(+), 3 deletions(-) > >> > > >> > diff --git a/drivers/gpio/gpio-vf610.c b/drivers/gpio/gpio-vf610.c > >> > index 521fbe3..a439d27 100644 > >> > --- a/drivers/gpio/gpio-vf610.c > >> > +++ b/drivers/gpio/gpio-vf610.c > >> > @@ -30,10 +30,15 @@ > >> > > >> > #define VF610_GPIO_PER_PORT 32 > >> > > >> > +struct fsl_gpio_soc_data { > >> > + u32 flags; > >> > +}; > >> > + > >> > struct vf610_gpio_port { > >> > struct gpio_chip gc; > >> > void __iomem *base; > >> > void __iomem *gpio_base; > >> > + const struct fsl_gpio_soc_data *sdata; > >> > u8 irqc[VF610_GPIO_PER_PORT]; > >> > int irq; > >> > }; > >> > @@ -43,6 +48,7 @@ struct vf610_gpio_port { > >> > #define GPIO_PCOR 0x08 > >> > #define GPIO_PTOR 0x0c > >> > #define GPIO_PDIR 0x10 > >> > +#define GPIO_PDDR 0x14 > >> > > >> > #define PORT_PCR(n) ((n) * 0x4) > >> > #define PORT_PCR_IRQC_OFFSET 16 > >> > @@ -59,10 +65,18 @@ struct vf610_gpio_port { > >> > #define PORT_INT_EITHER_EDGE 0xb > >> > #define PORT_INT_LOGIC_ONE 0xc > >> > > >> > +/* SoC has a Port Data Direction Register (PDDR) */ > >> > +#define FSL_GPIO_HAVE_PDDR BIT(0) > >> > + > >> > static struct irq_chip vf610_gpio_irq_chip; > >> > > >> > +static const struct fsl_gpio_soc_data imx_data = { > >> > + .flags = FSL_GPIO_HAVE_PDDR, > >> > +}; > >> > + > >> > static const struct of_device_id vf610_gpio_dt_ids[] = { > >> > - { .compatible = "fsl,vf610-gpio" }, > >> > + { .compatible = "fsl,vf610-gpio", .data = NULL, }, > >> > + { .compatible = "fsl,imx7ulp-gpio", .data = &imx_data, }, > >> > { /* sentinel */ } > >> > }; > >> > > >> > @@ -79,8 +93,18 @@ static inline u32 vf610_gpio_readl(void __iomem > >> > *reg) static int vf610_gpio_get(struct gpio_chip *gc, unsigned int > >> > gpio) { > >> > struct vf610_gpio_port *port = gpiochip_get_data(gc); > >> > - > >> > - return !!(vf610_gpio_readl(port->gpio_base + GPIO_PDIR) & > BIT(gpio)); > >> > + unsigned long mask = BIT(gpio); > >> > + void __iomem *addr; > >> > + > >> > + if (port->sdata && port->sdata->flags & FSL_GPIO_HAVE_PDDR) { > >> > + mask &= vf610_gpio_readl(port->gpio_base + GPIO_PDDR); > >> > + addr = mask ? port->gpio_base + GPIO_PDOR : > >> > + port->gpio_base + GPIO_PDIR; > >> > + return !!(vf610_gpio_readl(addr) & BIT(gpio)); > > I still feel it would be better to read back the actual value on the > wire... > > But this would require to enable the input buffer in > imx7ulp_pmx_gpio_set_direction. > > Lothar was with me in this discussion: > https://patchwork.kernel.org/patch/9726163/ > > Or is there really a technical limitation on i.MX7ULP? > No actually technical limitation, just different from Vybrid, MX7ULP has a GPIO_PDDR register to distinguish the input and output, We want to disable the input logic for output case to save power. > We could also change it later, especially if the pinctrl changes already > got merged anyway. > > From a Vybrid perspective: > Acked-by: Stefan Agner <stefan@agner.ch> > Thanks. Regards Dong Aisheng
diff --git a/drivers/gpio/gpio-vf610.c b/drivers/gpio/gpio-vf610.c index 521fbe3..a439d27 100644 --- a/drivers/gpio/gpio-vf610.c +++ b/drivers/gpio/gpio-vf610.c @@ -30,10 +30,15 @@ #define VF610_GPIO_PER_PORT 32 +struct fsl_gpio_soc_data { + u32 flags; +}; + struct vf610_gpio_port { struct gpio_chip gc; void __iomem *base; void __iomem *gpio_base; + const struct fsl_gpio_soc_data *sdata; u8 irqc[VF610_GPIO_PER_PORT]; int irq; }; @@ -43,6 +48,7 @@ struct vf610_gpio_port { #define GPIO_PCOR 0x08 #define GPIO_PTOR 0x0c #define GPIO_PDIR 0x10 +#define GPIO_PDDR 0x14 #define PORT_PCR(n) ((n) * 0x4) #define PORT_PCR_IRQC_OFFSET 16 @@ -59,10 +65,18 @@ struct vf610_gpio_port { #define PORT_INT_EITHER_EDGE 0xb #define PORT_INT_LOGIC_ONE 0xc +/* SoC has a Port Data Direction Register (PDDR) */ +#define FSL_GPIO_HAVE_PDDR BIT(0) + static struct irq_chip vf610_gpio_irq_chip; +static const struct fsl_gpio_soc_data imx_data = { + .flags = FSL_GPIO_HAVE_PDDR, +}; + static const struct of_device_id vf610_gpio_dt_ids[] = { - { .compatible = "fsl,vf610-gpio" }, + { .compatible = "fsl,vf610-gpio", .data = NULL, }, + { .compatible = "fsl,imx7ulp-gpio", .data = &imx_data, }, { /* sentinel */ } }; @@ -79,8 +93,18 @@ static inline u32 vf610_gpio_readl(void __iomem *reg) static int vf610_gpio_get(struct gpio_chip *gc, unsigned int gpio) { struct vf610_gpio_port *port = gpiochip_get_data(gc); - - return !!(vf610_gpio_readl(port->gpio_base + GPIO_PDIR) & BIT(gpio)); + unsigned long mask = BIT(gpio); + void __iomem *addr; + + if (port->sdata && port->sdata->flags & FSL_GPIO_HAVE_PDDR) { + mask &= vf610_gpio_readl(port->gpio_base + GPIO_PDDR); + addr = mask ? port->gpio_base + GPIO_PDOR : + port->gpio_base + GPIO_PDIR; + return !!(vf610_gpio_readl(addr) & BIT(gpio)); + } else { + return !!(vf610_gpio_readl(port->gpio_base + GPIO_PDIR) + & BIT(gpio)); + } } static void vf610_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val) @@ -96,12 +120,28 @@ static void vf610_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val) static int vf610_gpio_direction_input(struct gpio_chip *chip, unsigned gpio) { + struct vf610_gpio_port *port = gpiochip_get_data(chip); + unsigned long mask = BIT(gpio); + u32 val; + + if (port->sdata && port->sdata->flags & FSL_GPIO_HAVE_PDDR) { + val = vf610_gpio_readl(port->gpio_base + GPIO_PDDR); + val &= ~mask; + vf610_gpio_writel(val, port->gpio_base + GPIO_PDDR); + } + return pinctrl_gpio_direction_input(chip->base + gpio); } static int vf610_gpio_direction_output(struct gpio_chip *chip, unsigned gpio, int value) { + struct vf610_gpio_port *port = gpiochip_get_data(chip); + unsigned long mask = BIT(gpio); + + if (port->sdata && port->sdata->flags & FSL_GPIO_HAVE_PDDR) + vf610_gpio_writel(mask, port->gpio_base + GPIO_PDDR); + vf610_gpio_set(chip, gpio, value); return pinctrl_gpio_direction_output(chip->base + gpio); @@ -216,6 +256,8 @@ static struct irq_chip vf610_gpio_irq_chip = { static int vf610_gpio_probe(struct platform_device *pdev) { + const struct of_device_id *of_id = of_match_device(vf610_gpio_dt_ids, + &pdev->dev); struct device *dev = &pdev->dev; struct device_node *np = dev->of_node; struct vf610_gpio_port *port; @@ -227,6 +269,7 @@ static int vf610_gpio_probe(struct platform_device *pdev) if (!port) return -ENOMEM; + port->sdata = of_id->data; iores = platform_get_resource(pdev, IORESOURCE_MEM, 0); port->base = devm_ioremap_resource(dev, iores); if (IS_ERR(port->base))
The Rapid General-Purpose Input and Output with 2 Ports (RGPIO2P) on MX7ULP is similar to GPIO on Vibrid. But unlike Vibrid, the RGPIO2P has an extra Port Data Direction Register (PDDR) used to configure the individual port pins for input or output. We introduce a FSL_GPIO_HAVE_PDDR with fsl_gpio_soc_data data to distinguish this differences. And we support getting the output status by checking the GPIO direction in PDDR. Cc: Linus Walleij <linus.walleij@linaro.org> Cc: Alexandre Courbot <gnurou@gmail.com> Cc: Shawn Guo <shawnguo@kernel.org> Cc: Stefan Agner <stefan@agner.ch> Cc: Fugang Duan <fugang.duan@nxp.com> Cc: Peter Chen <peter.chen@nxp.com> Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com> --- drivers/gpio/gpio-vf610.c | 49 ++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 46 insertions(+), 3 deletions(-)