[PATCH] gpio: menz127: add support for 16Z034 and 16Z037 GPIO controllers

Jose Javier Rodriguez Barbarin posted 1 patch 3 months, 1 week ago
There is a newer version of this series
drivers/gpio/gpio-menz127.c | 36 ++++++++++++++++++++++++++++++++++--
1 file changed, 34 insertions(+), 2 deletions(-)
[PATCH] gpio: menz127: add support for 16Z034 and 16Z037 GPIO controllers
Posted by Jose Javier Rodriguez Barbarin 3 months, 1 week ago
From 7655a73f3888a5d164d1f287ba1f2989bb2aadd2 Mon Sep 17 00:00:00 2001
From: Javier Rodriguez <josejavier.rodriguez@duagon.com>
Date: Tue, 28 Oct 2025 17:40:14 +0100
Subject: [PATCH] gpio: menz127: add support for 16Z034 and 16Z037 GPIO
 controllers

The 16Z034 and 16Z037 are 8 bits GPIO controllers that share the
same registers and features of the 16Z127 GPIO controller.

Reviewed-by: Felipe Fensen Casado <felipe.jensen@duagon.com>
Signed-off-by: Javier Rodriguez <josejavier.rodriguez@duagon.com>
---
 drivers/gpio/gpio-menz127.c | 36 ++++++++++++++++++++++++++++++++++--
 1 file changed, 34 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpio-menz127.c b/drivers/gpio/gpio-menz127.c
index da2bf9381cc4..ec9228f1e631 100644
--- a/drivers/gpio/gpio-menz127.c
+++ b/drivers/gpio/gpio-menz127.c
@@ -24,6 +24,12 @@
 #define MEN_Z127_ODER	0x1C
 #define GPIO_TO_DBCNT_REG(gpio)	((gpio * 4) + 0x80)
 
+
+/* MEN Z127 supported model ids*/
+#define MEN_Z127_ID	0x7f
+#define MEN_Z034_ID	0x22
+#define MEN_Z037_ID	0x25
+
 #define MEN_Z127_DB_MIN_US	50
 /* 16 bit compare register. Each bit represents 50us */
 #define MEN_Z127_DB_MAX_US	(0xffff * MEN_Z127_DB_MIN_US)
@@ -36,6 +42,25 @@ struct men_z127_gpio {
 	struct resource *mem;
 };
 
+static int men_z127_lookup_gpio_size(struct mcb_device *mdev,
+				     unsigned long *sz)
+{
+	switch (mdev->id) {
+	case MEN_Z127_ID:
+		*sz = 4;
+		break;
+	case MEN_Z034_ID:
+	case MEN_Z037_ID:
+		*sz = 1;
+		break;
+	default:
+		dev_err(&mdev->dev, "no size found for id %d", mdev->id);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int men_z127_debounce(struct gpio_chip *gc, unsigned gpio,
 			     unsigned debounce)
 {
@@ -140,6 +165,7 @@ static int men_z127_probe(struct mcb_device *mdev,
 	struct men_z127_gpio *men_z127_gpio;
 	struct device *dev = &mdev->dev;
 	int ret;
+	unsigned long sz;
 
 	men_z127_gpio = devm_kzalloc(dev, sizeof(struct men_z127_gpio),
 				     GFP_KERNEL);
@@ -163,9 +189,13 @@ static int men_z127_probe(struct mcb_device *mdev,
 
 	mcb_set_drvdata(mdev, men_z127_gpio);
 
+	ret = men_z127_lookup_gpio_size(mdev, &sz);
+	if (ret)
+		return ret;
+
 	config = (struct gpio_generic_chip_config) {
 		.dev = &mdev->dev,
-		.sz = 4,
+		.sz = sz,
 		.dat = men_z127_gpio->reg_base + MEN_Z127_PSR,
 		.set = men_z127_gpio->reg_base + MEN_Z127_CTRL,
 		.dirout = men_z127_gpio->reg_base + MEN_Z127_GPIODR,
@@ -186,7 +216,9 @@ static int men_z127_probe(struct mcb_device *mdev,
 }
 
 static const struct mcb_device_id men_z127_ids[] = {
-	{ .device = 0x7f },
+	{ .device = MEN_Z127_ID },
+	{ .device = MEN_Z034_ID },
+	{ .device = MEN_Z037_ID },
 	{ }
 };
 MODULE_DEVICE_TABLE(mcb, men_z127_ids);
-- 
2.51.0
Re: [PATCH] gpio: menz127: add support for 16Z034 and 16Z037 GPIO controllers
Posted by Bartosz Golaszewski 3 months ago
On Fri, Oct 31, 2025 at 11:08 AM Jose Javier Rodriguez Barbarin
<dev-josejavier.rodriguez@duagon.com> wrote:
>
> From 7655a73f3888a5d164d1f287ba1f2989bb2aadd2 Mon Sep 17 00:00:00 2001
> From: Javier Rodriguez <josejavier.rodriguez@duagon.com>
> Date: Tue, 28 Oct 2025 17:40:14 +0100
> Subject: [PATCH] gpio: menz127: add support for 16Z034 and 16Z037 GPIO
>  controllers
>

I don't think you used `git send-email` to send this, did you just
copy the contents of the generated .patch into the email client?

> The 16Z034 and 16Z037 are 8 bits GPIO controllers that share the
> same registers and features of the 16Z127 GPIO controller.
>
> Reviewed-by: Felipe Fensen Casado <felipe.jensen@duagon.com>
> Signed-off-by: Javier Rodriguez <josejavier.rodriguez@duagon.com>
> ---
>  drivers/gpio/gpio-menz127.c | 36 ++++++++++++++++++++++++++++++++++--
>  1 file changed, 34 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpio/gpio-menz127.c b/drivers/gpio/gpio-menz127.c
> index da2bf9381cc4..ec9228f1e631 100644
> --- a/drivers/gpio/gpio-menz127.c
> +++ b/drivers/gpio/gpio-menz127.c
> @@ -24,6 +24,12 @@
>  #define MEN_Z127_ODER  0x1C
>  #define GPIO_TO_DBCNT_REG(gpio)        ((gpio * 4) + 0x80)
>
> +

Stray newline.

> +/* MEN Z127 supported model ids*/
> +#define MEN_Z127_ID    0x7f
> +#define MEN_Z034_ID    0x22
> +#define MEN_Z037_ID    0x25
> +
>  #define MEN_Z127_DB_MIN_US     50
>  /* 16 bit compare register. Each bit represents 50us */
>  #define MEN_Z127_DB_MAX_US     (0xffff * MEN_Z127_DB_MIN_US)
> @@ -36,6 +42,25 @@ struct men_z127_gpio {
>         struct resource *mem;
>  };
>
> +static int men_z127_lookup_gpio_size(struct mcb_device *mdev,
> +                                    unsigned long *sz)
> +{
> +       switch (mdev->id) {
> +       case MEN_Z127_ID:
> +               *sz = 4;
> +               break;
> +       case MEN_Z034_ID:
> +       case MEN_Z037_ID:
> +               *sz = 1;
> +               break;
> +       default:
> +               dev_err(&mdev->dev, "no size found for id %d", mdev->id);
> +               return -EINVAL;

You can return dev_err_probe() here, it's only used in probe(). But
TBH probe() is so small I'd just inline this into it.

> +       }
> +
> +       return 0;
> +}
> +
>  static int men_z127_debounce(struct gpio_chip *gc, unsigned gpio,
>                              unsigned debounce)
>  {
> @@ -140,6 +165,7 @@ static int men_z127_probe(struct mcb_device *mdev,
>         struct men_z127_gpio *men_z127_gpio;
>         struct device *dev = &mdev->dev;
>         int ret;
> +       unsigned long sz;
>
>         men_z127_gpio = devm_kzalloc(dev, sizeof(struct men_z127_gpio),
>                                      GFP_KERNEL);
> @@ -163,9 +189,13 @@ static int men_z127_probe(struct mcb_device *mdev,
>
>         mcb_set_drvdata(mdev, men_z127_gpio);
>
> +       ret = men_z127_lookup_gpio_size(mdev, &sz);
> +       if (ret)
> +               return ret;
> +
>         config = (struct gpio_generic_chip_config) {
>                 .dev = &mdev->dev,
> -               .sz = 4,
> +               .sz = sz,
>                 .dat = men_z127_gpio->reg_base + MEN_Z127_PSR,
>                 .set = men_z127_gpio->reg_base + MEN_Z127_CTRL,
>                 .dirout = men_z127_gpio->reg_base + MEN_Z127_GPIODR,
> @@ -186,7 +216,9 @@ static int men_z127_probe(struct mcb_device *mdev,
>  }
>
>  static const struct mcb_device_id men_z127_ids[] = {
> -       { .device = 0x7f },
> +       { .device = MEN_Z127_ID },
> +       { .device = MEN_Z034_ID },
> +       { .device = MEN_Z037_ID },
>         { }
>  };
>  MODULE_DEVICE_TABLE(mcb, men_z127_ids);
> --
> 2.51.0

Bartosz
Re: [PATCH] gpio: menz127: add support for 16Z034 and 16Z037 GPIO controllers
Posted by Jose Javier Rodriguez Barbarin 3 months ago
On Wed, Nov 05, 2025 at 09:36:52AM +0100, Bartosz Golaszewski wrote:
> On Fri, Oct 31, 2025 at 11:08 AM Jose Javier Rodriguez Barbarin
> <dev-josejavier.rodriguez@duagon.com> wrote:
> >
> > From 7655a73f3888a5d164d1f287ba1f2989bb2aadd2 Mon Sep 17 00:00:00 2001
> > From: Javier Rodriguez <josejavier.rodriguez@duagon.com>
> > Date: Tue, 28 Oct 2025 17:40:14 +0100
> > Subject: [PATCH] gpio: menz127: add support for 16Z034 and 16Z037 GPIO
> >  controllers
> >
> 
> I don't think you used `git send-email` to send this, did you just
> copy the contents of the generated .patch into the email client?
> 

My bad. This was my first time using alpine mail client instead of using
`git send-email`. Since your answer, I did some local tests and now I know
how to use alpine for submitting patches.

> > The 16Z034 and 16Z037 are 8 bits GPIO controllers that share the
> > same registers and features of the 16Z127 GPIO controller.
> >
> > Reviewed-by: Felipe Fensen Casado <felipe.jensen@duagon.com>
> > Signed-off-by: Javier Rodriguez <josejavier.rodriguez@duagon.com>
> > ---
> >  drivers/gpio/gpio-menz127.c | 36 ++++++++++++++++++++++++++++++++++--
> >  1 file changed, 34 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpio/gpio-menz127.c b/drivers/gpio/gpio-menz127.c
> > index da2bf9381cc4..ec9228f1e631 100644
> > --- a/drivers/gpio/gpio-menz127.c
> > +++ b/drivers/gpio/gpio-menz127.c
> > @@ -24,6 +24,12 @@
> >  #define MEN_Z127_ODER  0x1C
> >  #define GPIO_TO_DBCNT_REG(gpio)        ((gpio * 4) + 0x80)
> >
> > +
> 
> Stray newline.
> 
> > +/* MEN Z127 supported model ids*/
> > +#define MEN_Z127_ID    0x7f
> > +#define MEN_Z034_ID    0x22
> > +#define MEN_Z037_ID    0x25
> > +
> >  #define MEN_Z127_DB_MIN_US     50
> >  /* 16 bit compare register. Each bit represents 50us */
> >  #define MEN_Z127_DB_MAX_US     (0xffff * MEN_Z127_DB_MIN_US)
> > @@ -36,6 +42,25 @@ struct men_z127_gpio {
> >         struct resource *mem;
> >  };
> >
> > +static int men_z127_lookup_gpio_size(struct mcb_device *mdev,
> > +                                    unsigned long *sz)
> > +{
> > +       switch (mdev->id) {
> > +       case MEN_Z127_ID:
> > +               *sz = 4;
> > +               break;
> > +       case MEN_Z034_ID:
> > +       case MEN_Z037_ID:
> > +               *sz = 1;
> > +               break;
> > +       default:
> > +               dev_err(&mdev->dev, "no size found for id %d", mdev->id);
> > +               return -EINVAL;
> 
> You can return dev_err_probe() here, it's only used in probe(). But
> TBH probe() is so small I'd just inline this into it.
>

Thanks for your suggestions. I usually try to implement small functions for
improving the reading, but you are right. I've moved these lines to probe(), 
using dev_err_probe() as well.

> > +       }
> > +
> > +       return 0;
> > +}
> > +
> >  static int men_z127_debounce(struct gpio_chip *gc, unsigned gpio,
> >                              unsigned debounce)
> >  {
> > @@ -140,6 +165,7 @@ static int men_z127_probe(struct mcb_device *mdev,
> >         struct men_z127_gpio *men_z127_gpio;
> >         struct device *dev = &mdev->dev;
> >         int ret;
> > +       unsigned long sz;
> >
> >         men_z127_gpio = devm_kzalloc(dev, sizeof(struct men_z127_gpio),
> >                                      GFP_KERNEL);
> > @@ -163,9 +189,13 @@ static int men_z127_probe(struct mcb_device *mdev,
> >
> >         mcb_set_drvdata(mdev, men_z127_gpio);
> >
> > +       ret = men_z127_lookup_gpio_size(mdev, &sz);
> > +       if (ret)
> > +               return ret;
> > +
> >         config = (struct gpio_generic_chip_config) {
> >                 .dev = &mdev->dev,
> > -               .sz = 4,
> > +               .sz = sz,
> >                 .dat = men_z127_gpio->reg_base + MEN_Z127_PSR,
> >                 .set = men_z127_gpio->reg_base + MEN_Z127_CTRL,
> >                 .dirout = men_z127_gpio->reg_base + MEN_Z127_GPIODR,
> > @@ -186,7 +216,9 @@ static int men_z127_probe(struct mcb_device *mdev,
> >  }
> >
> >  static const struct mcb_device_id men_z127_ids[] = {
> > -       { .device = 0x7f },
> > +       { .device = MEN_Z127_ID },
> > +       { .device = MEN_Z034_ID },
> > +       { .device = MEN_Z037_ID },
> >         { }
> >  };
> >  MODULE_DEVICE_TABLE(mcb, men_z127_ids);
> > --
> > 2.51.0
> 
> Bartosz

Thank you, I will submit v2 with these changes.