| Message ID | 50f72f1f7f28e969a1e0353712fcc530bce9dd06.1611037866.git.matti.vaittinen@fi.rohmeurope.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | Support ROHM BD71815 PMIC | expand |
On Tue, Jan 19, 2021 at 8:23 AM Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> wrote: > > Support GPO(s) found from ROHM BD71815 power management IC. The IC has two > GPO pins but only one is properly documented in data-sheet. The driver > exposes by default only the documented GPO. The second GPO is connected to > E5 pin and is marked as GND in data-sheet. Control for this undocumented > pin can be enabled using a special DT property. > > This driver is derived from work by Peter Yang <yanglsh@embest-tech.com> > although not so much of original is left. > > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> Hi Matti, looks great, just a couple nits. > --- > Changes since v1: > - removed unneeded headers > - clarified dev/parent->dev usage > - removed forgotten #define DEBUG > > drivers/gpio/Kconfig | 10 +++ > drivers/gpio/Makefile | 1 + > drivers/gpio/gpio-bd71815.c | 171 ++++++++++++++++++++++++++++++++++++ > 3 files changed, 182 insertions(+) > create mode 100644 drivers/gpio/gpio-bd71815.c > > diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig > index c70f46e80a3b..fd7283af858d 100644 > --- a/drivers/gpio/Kconfig > +++ b/drivers/gpio/Kconfig > @@ -1096,6 +1096,16 @@ config GPIO_BD70528 > This driver can also be built as a module. If so, the module > will be called gpio-bd70528. > > +config GPIO_BD71815 > + tristate "ROHM BD71815 PMIC GPIO support" > + depends on MFD_ROHM_BD71828 > + help > + Support for GPO(s) on ROHM BD71815 PMIC. There are two GPOs > + available on the ROHM PMIC. > + > + This driver can also be built as a module. If so, the module > + will be called gpio-bd71815. > + > config GPIO_BD71828 > tristate "ROHM BD71828 GPIO support" > depends on MFD_ROHM_BD71828 > diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile > index 35e3b6026665..86bb680522a6 100644 > --- a/drivers/gpio/Makefile > +++ b/drivers/gpio/Makefile > @@ -39,6 +39,7 @@ obj-$(CONFIG_GPIO_ATH79) += gpio-ath79.o > obj-$(CONFIG_GPIO_BCM_KONA) += gpio-bcm-kona.o > obj-$(CONFIG_GPIO_BCM_XGS_IPROC) += gpio-xgs-iproc.o > obj-$(CONFIG_GPIO_BD70528) += gpio-bd70528.o > +obj-$(CONFIG_GPIO_BD71815) += gpio-bd71815.o > obj-$(CONFIG_GPIO_BD71828) += gpio-bd71828.o > obj-$(CONFIG_GPIO_BD9571MWV) += gpio-bd9571mwv.o > obj-$(CONFIG_GPIO_BRCMSTB) += gpio-brcmstb.o > diff --git a/drivers/gpio/gpio-bd71815.c b/drivers/gpio/gpio-bd71815.c > new file mode 100644 > index 000000000000..664de5f69bf1 > --- /dev/null > +++ b/drivers/gpio/gpio-bd71815.c > @@ -0,0 +1,171 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Support to GPOs on ROHM BD71815 > + */ Newline here. > +#include <linux/module.h> > +#include <linux/init.h> > +#include <linux/irq.h> > +#include <linux/gpio/driver.h> > +#include <linux/platform_device.h> > +#include <linux/of.h> > +/* For the BD71815 register definitions */ > +#include <linux/mfd/rohm-bd71815.h> > + Please arrange headers alphabetically. > +struct bd71815_gpio { > + struct gpio_chip chip; > + struct device *dev; > + struct regmap *regmap; > + /* > + * Sigh. The BD71815 and BD71817 were originally designed to support two > + * GPO pins. At some point it was noticed the second GPO pin which is > + * the E5 pin located at the center of IC is hard to use on PCB (due to > + * the location). It was decided to not promote this second GPO and pin > + * is marked as GND on the data-sheet. The functionality is still there > + * though! I guess driving GPO connected to ground is a bad idea. Thus > + * we do not support it by default. OTOH - the original driver written > + * by colleagues at Embest did support controlling this second GPO. It > + * is thus possible this is used in some of the products. > + * > + * This driver does not by default support configuring this second GPO > + * but allows using it by providing the DT property > + * "rohm,enable-hidden-gpo". > + */ > + bool e5_pin_is_gpo; > +}; > + > +static int bd71815gpo_get(struct gpio_chip *chip, unsigned int offset) > +{ > + struct bd71815_gpio *bd71815 = gpiochip_get_data(chip); > + int ret = 0; > + int 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, val, mask; > + > + if (!bd71815->e5_pin_is_gpo && offset) > + return; > + > + mask = BIT(offset); > + val = value ? mask : 0; Maybe use regmap_set/clear_bits() here? > + ret = regmap_update_bits(bd71815->regmap, BD71815_REG_GPO, mask, val); > + if (ret) > + dev_warn(bd71815->dev, "failed to toggle GPO\n"); > +} > + > +static int bd71815_gpio_set_config(struct gpio_chip *chip, unsigned int offset, > + unsigned long config) > +{ > + struct bd71815_gpio *bdgpio = gpiochip_get_data(chip); > + > + if (!bdgpio->e5_pin_is_gpo && offset) > + return -EOPNOTSUPP; > + > + switch (pinconf_to_config_param(config)) { > + case PIN_CONFIG_DRIVE_OPEN_DRAIN: > + return regmap_update_bits(bdgpio->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, > + BD71815_REG_GPO, > + BD71815_GPIO_DRIVE_MASK << offset, > + BD71815_GPIO_CMOS << offset); > + default: > + break; > + } > + return -EOPNOTSUPP; > +} > + > +/* 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 */ So let's make it const? > +static 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 = 1, > +}; > + > +static int gpo_bd71815_probe(struct platform_device *pdev) > +{ > + int ret; > + struct bd71815_gpio *g; > + struct device *dev; > + struct device *parent; > + > + /* > + * Bind devm lifetime to this platform device => use dev for devm. > + * also the prints should originate from this device. > + */ > + dev = &pdev->dev; > + /* The device-tree and regmap come from MFD => use parent for that */ > + parent = dev->parent; > + > + g = devm_kzalloc(dev, sizeof(*g), GFP_KERNEL); > + if (!g) > + return -ENOMEM; > + > + g->e5_pin_is_gpo = of_property_read_bool(parent->of_node, > + "rohm,enable-hidden-gpo"); > + g->chip = bd71815gpo_chip; > + g->chip.base = -1; > + > + if (g->e5_pin_is_gpo) > + g->chip.ngpio = 2; > + else > + g->chip.ngpio = 1; > + > + g->chip.parent = parent; > + g->chip.of_node = parent->of_node; > + g->regmap = dev_get_regmap(parent, NULL); > + g->dev = dev; > + > + ret = devm_gpiochip_add_data(dev, &g->chip, g); > + if (ret < 0) { > + dev_err(dev, "could not register gpiochip, %d\n", ret); > + return ret; > + } > + > + return ret; > +} > +static const struct platform_device_id bd7181x_gpo_id[] = { > + { "bd71815-gpo" }, > + { }, > +}; > +MODULE_DEVICE_TABLE(platform, bd7181x_gpo_id); > + > +static struct platform_driver gpo_bd71815_driver = { > + .driver = { > + .name = "bd71815-gpo", > + .owner = THIS_MODULE, > + }, > + .probe = gpo_bd71815_probe, > + .id_table = bd7181x_gpo_id, > +}; > + > +module_platform_driver(gpo_bd71815_driver); > + > +/* Note: this hardware lives inside an I2C-based multi-function device. */ > +MODULE_ALIAS("platform:bd71815-gpo"); > + > +MODULE_AUTHOR("Peter Yang <yanglsh@embest-tech.com>"); > +MODULE_DESCRIPTION("GPO interface for BD71815"); > +MODULE_LICENSE("GPL"); > -- > 2.25.4 > Bartosz > > -- > 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 =]
Hi Bartosz, On Tue, 2021-01-19 at 12:07 +0100, Bartosz Golaszewski wrote: > On Tue, Jan 19, 2021 at 8:23 AM Matti Vaittinen > <matti.vaittinen@fi.rohmeurope.com> wrote: > > Support GPO(s) found from ROHM BD71815 power management IC. The IC > > has two > > GPO pins but only one is properly documented in data-sheet. The > > driver > > exposes by default only the documented GPO. The second GPO is > > connected to > > E5 pin and is marked as GND in data-sheet. Control for this > > undocumented > > pin can be enabled using a special DT property. > > > > This driver is derived from work by Peter Yang < > > yanglsh@embest-tech.com> > > although not so much of original is left. > > > > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> > > Hi Matti, > > looks great, just a couple nits. > Thanks for the review! I'll store your finsings and fix them when I respin this. I think all of your points were valid. As I know this is largish series (and as I know I accidentally sent first 10 v2 patches to all recipients no matter what subsystem was impacted) I'll wait for a while before resending (at least a week). Besides I don't expect the dependencies to be merged before next kernel release so this is not urgent :) - but thanks! Br, Matti
On Tue, 2021-01-19 at 12:07 +0100, Bartosz Golaszewski wrote: > On Tue, Jan 19, 2021 at 8:23 AM Matti Vaittinen > <matti.vaittinen@fi.rohmeurope.com> wrote: > > Support GPO(s) found from ROHM BD71815 power management IC. The IC > > has two > > GPO pins but only one is properly documented in data-sheet. The > > driver > > exposes by default only the documented GPO. The second GPO is > > connected to > > E5 pin and is marked as GND in data-sheet. Control for this > > undocumented > > pin can be enabled using a special DT property. > > > > This driver is derived from work by Peter Yang < > > yanglsh@embest-tech.com> > > although not so much of original is left. > > > > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> > > Hi Matti, > > looks great, just a couple nits. Hello Bartosz, I think fixed all the nits to v3. Can I translate this to an ack? (I will respin the series as I guess the regulator part may have fallen through the cracks so I'd like to add the relevant acks :] ) Best Regards Matti Vaittinen
On Tue, Mar 23, 2021 at 10:57 AM Vaittinen, Matti <Matti.Vaittinen@fi.rohmeurope.com> wrote: > > > On Tue, 2021-01-19 at 12:07 +0100, Bartosz Golaszewski wrote: > > On Tue, Jan 19, 2021 at 8:23 AM Matti Vaittinen > > <matti.vaittinen@fi.rohmeurope.com> wrote: > > > Support GPO(s) found from ROHM BD71815 power management IC. The IC > > > has two > > > GPO pins but only one is properly documented in data-sheet. The > > > driver > > > exposes by default only the documented GPO. The second GPO is > > > connected to > > > E5 pin and is marked as GND in data-sheet. Control for this > > > undocumented > > > pin can be enabled using a special DT property. > > > > > > This driver is derived from work by Peter Yang < > > > yanglsh@embest-tech.com> > > > although not so much of original is left. > > > > > > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> > > > > Hi Matti, > > > > looks great, just a couple nits. > > Hello Bartosz, > > I think fixed all the nits to v3. Can I translate this to an ack? (I > will respin the series as I guess the regulator part may have fallen > through the cracks so I'd like to add the relevant acks :] ) > > Best Regards > Matti Vaittinen Yes: Acked-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index c70f46e80a3b..fd7283af858d 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -1096,6 +1096,16 @@ config GPIO_BD70528 This driver can also be built as a module. If so, the module will be called gpio-bd70528. +config GPIO_BD71815 + tristate "ROHM BD71815 PMIC GPIO support" + depends on MFD_ROHM_BD71828 + help + Support for GPO(s) on ROHM BD71815 PMIC. There are two GPOs + available on the ROHM PMIC. + + This driver can also be built as a module. If so, the module + will be called gpio-bd71815. + config GPIO_BD71828 tristate "ROHM BD71828 GPIO support" depends on MFD_ROHM_BD71828 diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile index 35e3b6026665..86bb680522a6 100644 --- a/drivers/gpio/Makefile +++ b/drivers/gpio/Makefile @@ -39,6 +39,7 @@ obj-$(CONFIG_GPIO_ATH79) += gpio-ath79.o obj-$(CONFIG_GPIO_BCM_KONA) += gpio-bcm-kona.o obj-$(CONFIG_GPIO_BCM_XGS_IPROC) += gpio-xgs-iproc.o obj-$(CONFIG_GPIO_BD70528) += gpio-bd70528.o +obj-$(CONFIG_GPIO_BD71815) += gpio-bd71815.o obj-$(CONFIG_GPIO_BD71828) += gpio-bd71828.o obj-$(CONFIG_GPIO_BD9571MWV) += gpio-bd9571mwv.o obj-$(CONFIG_GPIO_BRCMSTB) += gpio-brcmstb.o diff --git a/drivers/gpio/gpio-bd71815.c b/drivers/gpio/gpio-bd71815.c new file mode 100644 index 000000000000..664de5f69bf1 --- /dev/null +++ b/drivers/gpio/gpio-bd71815.c @@ -0,0 +1,171 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Support to GPOs on ROHM BD71815 + */ +#include <linux/module.h> +#include <linux/init.h> +#include <linux/irq.h> +#include <linux/gpio/driver.h> +#include <linux/platform_device.h> +#include <linux/of.h> +/* For the BD71815 register definitions */ +#include <linux/mfd/rohm-bd71815.h> + +struct bd71815_gpio { + struct gpio_chip chip; + struct device *dev; + struct regmap *regmap; + /* + * Sigh. The BD71815 and BD71817 were originally designed to support two + * GPO pins. At some point it was noticed the second GPO pin which is + * the E5 pin located at the center of IC is hard to use on PCB (due to + * the location). It was decided to not promote this second GPO and pin + * is marked as GND on the data-sheet. The functionality is still there + * though! I guess driving GPO connected to ground is a bad idea. Thus + * we do not support it by default. OTOH - the original driver written + * by colleagues at Embest did support controlling this second GPO. It + * is thus possible this is used in some of the products. + * + * This driver does not by default support configuring this second GPO + * but allows using it by providing the DT property + * "rohm,enable-hidden-gpo". + */ + bool e5_pin_is_gpo; +}; + +static int bd71815gpo_get(struct gpio_chip *chip, unsigned int offset) +{ + struct bd71815_gpio *bd71815 = gpiochip_get_data(chip); + int ret = 0; + int 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, val, mask; + + if (!bd71815->e5_pin_is_gpo && offset) + return; + + mask = BIT(offset); + val = value ? mask : 0; + ret = regmap_update_bits(bd71815->regmap, BD71815_REG_GPO, mask, val); + if (ret) + dev_warn(bd71815->dev, "failed to toggle GPO\n"); +} + +static int bd71815_gpio_set_config(struct gpio_chip *chip, unsigned int offset, + unsigned long config) +{ + struct bd71815_gpio *bdgpio = gpiochip_get_data(chip); + + if (!bdgpio->e5_pin_is_gpo && offset) + return -EOPNOTSUPP; + + switch (pinconf_to_config_param(config)) { + case PIN_CONFIG_DRIVE_OPEN_DRAIN: + return regmap_update_bits(bdgpio->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, + BD71815_REG_GPO, + BD71815_GPIO_DRIVE_MASK << offset, + BD71815_GPIO_CMOS << offset); + default: + break; + } + return -EOPNOTSUPP; +} + +/* 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 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 = 1, +}; + +static int gpo_bd71815_probe(struct platform_device *pdev) +{ + int ret; + struct bd71815_gpio *g; + struct device *dev; + struct device *parent; + + /* + * Bind devm lifetime to this platform device => use dev for devm. + * also the prints should originate from this device. + */ + dev = &pdev->dev; + /* The device-tree and regmap come from MFD => use parent for that */ + parent = dev->parent; + + g = devm_kzalloc(dev, sizeof(*g), GFP_KERNEL); + if (!g) + return -ENOMEM; + + g->e5_pin_is_gpo = of_property_read_bool(parent->of_node, + "rohm,enable-hidden-gpo"); + g->chip = bd71815gpo_chip; + g->chip.base = -1; + + if (g->e5_pin_is_gpo) + g->chip.ngpio = 2; + else + g->chip.ngpio = 1; + + g->chip.parent = parent; + g->chip.of_node = parent->of_node; + g->regmap = dev_get_regmap(parent, NULL); + g->dev = dev; + + ret = devm_gpiochip_add_data(dev, &g->chip, g); + if (ret < 0) { + dev_err(dev, "could not register gpiochip, %d\n", ret); + return ret; + } + + return ret; +} +static const struct platform_device_id bd7181x_gpo_id[] = { + { "bd71815-gpo" }, + { }, +}; +MODULE_DEVICE_TABLE(platform, bd7181x_gpo_id); + +static struct platform_driver gpo_bd71815_driver = { + .driver = { + .name = "bd71815-gpo", + .owner = THIS_MODULE, + }, + .probe = gpo_bd71815_probe, + .id_table = bd7181x_gpo_id, +}; + +module_platform_driver(gpo_bd71815_driver); + +/* Note: this hardware lives inside an I2C-based multi-function device. */ +MODULE_ALIAS("platform:bd71815-gpo"); + +MODULE_AUTHOR("Peter Yang <yanglsh@embest-tech.com>"); +MODULE_DESCRIPTION("GPO interface for BD71815"); +MODULE_LICENSE("GPL");
Support GPO(s) found from ROHM BD71815 power management IC. The IC has two GPO pins but only one is properly documented in data-sheet. The driver exposes by default only the documented GPO. The second GPO is connected to E5 pin and is marked as GND in data-sheet. Control for this undocumented pin can be enabled using a special DT property. This driver is derived from work by Peter Yang <yanglsh@embest-tech.com> although not so much of original is left. Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> --- Changes since v1: - removed unneeded headers - clarified dev/parent->dev usage - removed forgotten #define DEBUG drivers/gpio/Kconfig | 10 +++ drivers/gpio/Makefile | 1 + drivers/gpio/gpio-bd71815.c | 171 ++++++++++++++++++++++++++++++++++++ 3 files changed, 182 insertions(+) create mode 100644 drivers/gpio/gpio-bd71815.c