All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andy Shevchenko <andy.shevchenko@gmail.com>
To: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
Cc: Matti Vaittinen <mazziesaccount@gmail.com>,
	Linus Walleij <linus.walleij@linaro.org>,
	Bartosz Golaszewski <bgolaszewski@baylibre.com>,
	Michael Walle <michael@walle.cc>,
	"open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-power <linux-power@fi.rohmeurope.com>
Subject: Re: [PATCH v2 3/3] gpio: bd71815: Use gpio-regmap
Date: Fri, 21 May 2021 13:12:13 +0300	[thread overview]
Message-ID: <CAHp75VctYvEOXep2-gtfsuTRp+yh+s_0EYubTO2cmh7YQ5OWYg@mail.gmail.com> (raw)
In-Reply-To: <9b040b3610a50e8c3c9579f5d28713af5a59942c.1621577204.git.matti.vaittinen@fi.rohmeurope.com>

On Fri, May 21, 2021 at 12:54 PM Matti Vaittinen
<matti.vaittinen@fi.rohmeurope.com> wrote:
>
> Utilize the gpio-regmap helper and drop the custom functions

I like the statistics of this change!
FWIW,
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> ---
> Changelog:
>  - No changes
>
>  drivers/gpio/Kconfig        |   1 +
>  drivers/gpio/gpio-bd71815.c | 106 ++++++++++--------------------------
>  2 files changed, 29 insertions(+), 78 deletions(-)
>
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 1dd0ec6727fd..97e1348cd410 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -1120,6 +1120,7 @@ config GPIO_BD70528
>  config GPIO_BD71815
>         tristate "ROHM BD71815 PMIC GPIO support"
>         depends on MFD_ROHM_BD71828
> +       select GPIO_REGMAP
>         help
>           Support for GPO(s) on ROHM BD71815 PMIC. There are two GPOs
>           available on the ROHM PMIC.
> diff --git a/drivers/gpio/gpio-bd71815.c b/drivers/gpio/gpio-bd71815.c
> index 08ff2857256f..a241c01e08d1 100644
> --- a/drivers/gpio/gpio-bd71815.c
> +++ b/drivers/gpio/gpio-bd71815.c
> @@ -9,6 +9,7 @@
>   */
>
>  #include <linux/gpio/driver.h>
> +#include <linux/gpio/regmap.h>
>  #include <linux/init.h>
>  #include <linux/irq.h>
>  #include <linux/module.h>
> @@ -18,81 +19,33 @@
>  #include <linux/mfd/rohm-bd71815.h>
>
>  struct bd71815_gpio {
> -       /* chip.parent points the MFD which provides DT node and regmap */
> -       struct gpio_chip chip;
> -       /* dev points to the platform device for devm and prints */
>         struct device *dev;
> -       struct regmap *regmap;
>  };
>
> -static int bd71815gpo_get(struct gpio_chip *chip, unsigned int offset)
> -{
> -       struct bd71815_gpio *bd71815 = gpiochip_get_data(chip);
> -       int ret, val;
> -
> -       ret = regmap_read(bd71815->regmap, BD71815_REG_GPO, &val);
> -       if (ret)
> -               return ret;
> -
> -       return (val >> offset) & 1;
> -}
> -
> -static void bd71815gpo_set(struct gpio_chip *chip, unsigned int offset,
> -                          int value)
> -{
> -       struct bd71815_gpio *bd71815 = gpiochip_get_data(chip);
> -       int ret, bit;
> -
> -       bit = BIT(offset);
> -
> -       if (value)
> -               ret = regmap_set_bits(bd71815->regmap, BD71815_REG_GPO, bit);
> -       else
> -               ret = regmap_clear_bits(bd71815->regmap, BD71815_REG_GPO, bit);
> -
> -       if (ret)
> -               dev_warn(bd71815->dev, "failed to toggle GPO\n");
> -}
> -
> -static int bd71815_gpio_set_config(struct gpio_chip *chip, unsigned int offset,
> +static int bd71815_gpio_set_config(struct regmap *regmap, void *drvdata,
> +                                  unsigned int offset,
>                                    unsigned long config)
>  {
> -       struct bd71815_gpio *bdgpio = gpiochip_get_data(chip);
> +       struct bd71815_gpio *bdgpio = (struct bd71815_gpio *)drvdata;
>
>         switch (pinconf_to_config_param(config)) {
>         case PIN_CONFIG_DRIVE_OPEN_DRAIN:
> -               return regmap_update_bits(bdgpio->regmap,
> +               return regmap_update_bits(regmap,
>                                           BD71815_REG_GPO,
>                                           BD71815_GPIO_DRIVE_MASK << offset,
>                                           BD71815_GPIO_OPEN_DRAIN << offset);
>         case PIN_CONFIG_DRIVE_PUSH_PULL:
> -               return regmap_update_bits(bdgpio->regmap,
> +               return regmap_update_bits(regmap,
>                                           BD71815_REG_GPO,
>                                           BD71815_GPIO_DRIVE_MASK << offset,
>                                           BD71815_GPIO_CMOS << offset);
>         default:
> +               dev_err(bdgpio->dev, "Unsupported config (0x%lx)\n", config);
>                 break;
>         }
>         return -ENOTSUPP;
>  }
>
> -/* BD71815 GPIO is actually GPO */
> -static int bd71815gpo_direction_get(struct gpio_chip *gc, unsigned int offset)
> -{
> -       return GPIO_LINE_DIRECTION_OUT;
> -}
> -
> -/* Template for GPIO chip */
> -static const struct gpio_chip bd71815gpo_chip = {
> -       .label                  = "bd71815",
> -       .owner                  = THIS_MODULE,
> -       .get                    = bd71815gpo_get,
> -       .get_direction          = bd71815gpo_direction_get,
> -       .set                    = bd71815gpo_set,
> -       .set_config             = bd71815_gpio_set_config,
> -       .can_sleep              = true,
> -};
> -
>  #define BD71815_TWO_GPIOS      GENMASK(1, 0)
>  #define BD71815_ONE_GPIO       BIT(0)
>
> @@ -111,14 +64,16 @@ static const struct gpio_chip bd71815gpo_chip = {
>   * but allows using it by providing the DT property
>   * "rohm,enable-hidden-gpo".
>   */
> -static int bd71815_init_valid_mask(struct gpio_chip *gc,
> +static int bd71815_init_valid_mask(struct regmap *regmap, void *drvdata,
>                                    unsigned long *valid_mask,
>                                    unsigned int ngpios)
>  {
> +       struct bd71815_gpio *bdgpio = (struct bd71815_gpio *)drvdata;
> +
>         if (ngpios != 2)
>                 return 0;
>
> -       if (gc->parent && device_property_present(gc->parent,
> +       if (bdgpio->dev && device_property_present(bdgpio->dev->parent,
>                                                   "rohm,enable-hidden-gpo"))
>                 *valid_mask = BD71815_TWO_GPIOS;
>         else
> @@ -127,9 +82,19 @@ static int bd71815_init_valid_mask(struct gpio_chip *gc,
>         return 0;
>  }
>
> +/* Template for regmap gpio config */
> +static const struct gpio_regmap_config gpio_cfg_template = {
> +       .label                  = "bd71815",
> +       .reg_set_base           = BD71815_REG_GPO,
> +       .ngpio                  = 2,
> +       .set_config             = bd71815_gpio_set_config,
> +       .init_valid_mask        = bd71815_init_valid_mask,
> +};
> +
>  static int gpo_bd71815_probe(struct platform_device *pdev)
>  {
>         struct bd71815_gpio *g;
> +       struct gpio_regmap_config cfg;
>         struct device *parent, *dev;
>
>         /*
> @@ -144,30 +109,15 @@ static int gpo_bd71815_probe(struct platform_device *pdev)
>         if (!g)
>                 return -ENOMEM;
>
> -       g->chip = bd71815gpo_chip;
> -
> -       /*
> -        * FIXME: As writing of this the sysfs interface for GPIO control does
> -        * not respect the valid_mask. Do not trust it but rather set the ngpios
> -        * to 1 if "rohm,enable-hidden-gpo" is not given.
> -        *
> -        * This check can be removed later if the sysfs export is fixed and
> -        * if the fix is backported.
> -        *
> -        * For now it is safest to just set the ngpios though.
> -        */
> -       if (device_property_present(parent, "rohm,enable-hidden-gpo"))
> -               g->chip.ngpio = 2;
> -       else
> -               g->chip.ngpio = 1;
> -
> -       g->chip.init_valid_mask = bd71815_init_valid_mask;
> -       g->chip.base = -1;
> -       g->chip.parent = parent;
> -       g->regmap = dev_get_regmap(parent, NULL);
>         g->dev = dev;
>
> -       return devm_gpiochip_add_data(dev, &g->chip, g);
> +       cfg = gpio_cfg_template;
> +       cfg.parent = parent;
> +       cfg.regmap = dev_get_regmap(parent, NULL);
> +       cfg.fwnode = dev_fwnode(dev);
> +       cfg.drvdata = g;
> +
> +       return PTR_ERR_OR_ZERO(devm_gpio_regmap_register(dev, &cfg));
>  }
>
>  static struct platform_driver gpo_bd71815_driver = {
> --
> 2.25.4
>
>
> --
> Matti Vaittinen, Linux device drivers
> ROHM Semiconductors, Finland SWDC
> Kiviharjunlenkki 1E
> 90220 OULU
> FINLAND
>
> ~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
> Simon says - in Latin please.
> ~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
> Thanks to Simon Glass for the translation =]



-- 
With Best Regards,
Andy Shevchenko

      reply	other threads:[~2021-05-21 10:12 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-21  6:27 [PATCH v2 0/3] gpio: gpio-regmap: Support few custom operations Matti Vaittinen
2021-05-21  6:27 ` [PATCH v2 1/3] gpio: regmap: Support few IC specific operations Matti Vaittinen
2021-05-21  8:04   ` Michael Walle
2021-05-21 10:09   ` Andy Shevchenko
2021-05-21 10:19     ` Michael Walle
2021-05-21 10:25       ` Vaittinen, Matti
2021-05-21 10:46         ` Michael Walle
2021-05-21 11:36           ` Vaittinen, Matti
2021-05-21 11:19       ` Andy Shevchenko
2021-05-21 11:41         ` Vaittinen, Matti
2021-05-21 10:20     ` Vaittinen, Matti
2021-05-21  6:28 ` [PATCH v2 2/3] gpio: gpio-regmap: Use devm_add_action() Matti Vaittinen
2021-05-21  8:10   ` Michael Walle
2021-05-21  8:38     ` Bjørn Mork
2021-05-21 10:30       ` Vaittinen, Matti
2021-05-21 16:35         ` Bartosz Golaszewski
2021-05-24  5:01           ` Vaittinen, Matti
2021-05-21 10:10   ` Andy Shevchenko
2021-05-21  6:29 ` [PATCH v2 3/3] gpio: bd71815: Use gpio-regmap Matti Vaittinen
2021-05-21 10:12   ` Andy Shevchenko [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAHp75VctYvEOXep2-gtfsuTRp+yh+s_0EYubTO2cmh7YQ5OWYg@mail.gmail.com \
    --to=andy.shevchenko@gmail.com \
    --cc=bgolaszewski@baylibre.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-power@fi.rohmeurope.com \
    --cc=matti.vaittinen@fi.rohmeurope.com \
    --cc=mazziesaccount@gmail.com \
    --cc=michael@walle.cc \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.