On Fri, May 21, 2021 at 12:54 PM Matti Vaittinen <matti.vaittinen@xxxxxxxxxxxxxxxxx> 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@xxxxxxxxx> > Signed-off-by: Matti Vaittinen <matti.vaittinen@xxxxxxxxxxxxxxxxx> > --- > 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