Re: [PATCH v5 3/4] gpio: adp5585: Add Analog Devices ADP5585 support

From: Frank Li
Date: Fri Jul 19 2024 - 17:39:14 EST

Next message: pr-tracker-bot: "Re: [GIT PULL] power sequencing fixes for v6.11-rc1"
Previous message: Frank Li: "Re: [PATCH v5 2/4] mfd: adp5585: Add Analog Devices ADP5585 core support"
In reply to: Laurent Pinchart: "[PATCH v5 3/4] gpio: adp5585: Add Analog Devices ADP5585 support"
Next in thread: Laurent Pinchart: "[PATCH v5 2/4] mfd: adp5585: Add Analog Devices ADP5585 core support"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Fri, Jul 19, 2024 at 11:39:45PM +0300, Laurent Pinchart wrote:
> From: Haibo Chen <haibo.chen@xxxxxxx>
>
> The ADP5585 is a 10/11 input/output port expander with a built in keypad
> matrix decoder, programmable logic, reset generator, and PWM generator.
> This driver supports the GPIO function using the platform device
> registered by the core MFD driver.
>
> The driver is derived from an initial implementation from NXP, available
> in commit 451f61b46b76 ("MLK-25917-2 gpio: adp5585-gpio: add
> adp5585-gpio support") in their BSP kernel tree. It has been extensively
> rewritten.
>
> Signed-off-by: Haibo Chen <haibo.chen@xxxxxxx>
> Reviewed-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
> Acked-by: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxx>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> ---
> Changes compared to v2:
>
> - Add missing headers
> - Drop platform_set_drvdata()
> - Fix bit shift in bias configuration
>
> Changes compared to v1:
>
> - Drop OF match table
> - Fix .get() for GPOs
> - Add platform ID table
> - Set struct device of_node manually
> - Merge child DT node into parent node
> - Implement .get_direction()
> - Drop mutex
>
> Changes compared to the NXP original version
>
> - Add MAINTAINERS entry
> - Add prefix to compatible string
> - Switch to regmap
> - White space fixes
> - Use sizeof(*variable)
> - Initialize variables at declaration time
> - Use mutex scope guards
> - Cleanup header includes
> - Support R5 GPIO pin
> - Reorder functions
> - Add bias support
> - Return real pin value from .get()
> - Add debounce support
> - Add drive mode support
> - Destroy mutex on remove
> - Update copyright
> - Update license to GPL-2.0-only
> ---
> MAINTAINERS | 1 +
> drivers/gpio/Kconfig | 7 ++
> drivers/gpio/Makefile | 1 +
> drivers/gpio/gpio-adp5585.c | 231 ++++++++++++++++++++++++++++++++++++
> 4 files changed, 240 insertions(+)
> create mode 100644 drivers/gpio/gpio-adp5585.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index ebb1a1833bbc..b748af2acf9f 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -532,6 +532,7 @@ L: linux-gpio@xxxxxxxxxxxxxxx
> L: linux-pwm@xxxxxxxxxxxxxxx
> S: Maintained
> F: Documentation/devicetree/bindings/*/adi,adp5585*.yaml
> +F: drivers/gpio/gpio-adp5585.c
> F: drivers/mfd/adp5585.c
> F: include/linux/mfd/adp5585.h
>
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 1c28a48915bb..fc66e8264eef 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -1233,6 +1233,13 @@ config GPIO_ADP5520
> This option enables support for on-chip GPIO found
> on Analog Devices ADP5520 PMICs.
>
> +config GPIO_ADP5585
> + tristate "GPIO Support for ADP5585"
> + depends on MFD_ADP5585
> + help
> + This option enables support for the GPIO function found in the Analog
> + Devices ADP5585.
> +
> config GPIO_ALTERA_A10SR
> tristate "Altera Arria10 System Resource GPIO"
> depends on MFD_ALTERA_A10SR
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index e2a53013780e..04bfa2bc7e11 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -26,6 +26,7 @@ obj-$(CONFIG_GPIO_74X164) += gpio-74x164.o
> obj-$(CONFIG_GPIO_74XX_MMIO) += gpio-74xx-mmio.o
> obj-$(CONFIG_GPIO_ADNP) += gpio-adnp.o
> obj-$(CONFIG_GPIO_ADP5520) += gpio-adp5520.o
> +obj-$(CONFIG_GPIO_ADP5585) += gpio-adp5585.o
> obj-$(CONFIG_GPIO_AGGREGATOR) += gpio-aggregator.o
> obj-$(CONFIG_GPIO_ALTERA_A10SR) += gpio-altera-a10sr.o
> obj-$(CONFIG_GPIO_ALTERA) += gpio-altera.o
> diff --git a/drivers/gpio/gpio-adp5585.c b/drivers/gpio/gpio-adp5585.c
> new file mode 100644
> index 000000000000..49f8d91623ba
> --- /dev/null
> +++ b/drivers/gpio/gpio-adp5585.c
> @@ -0,0 +1,231 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Analog Devices ADP5585 GPIO driver
> + *
> + * Copyright 2022 NXP
> + * Copyright 2024 Ideas on Board Oy
> + */
> +
> +#include <linux/device.h>
> +#include <linux/gpio/driver.h>
> +#include <linux/mfd/adp5585.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/types.h>
> +
> +#define ADP5585_GPIO_MAX 11
> +
> +struct adp5585_gpio_dev {
> + struct gpio_chip gpio_chip;
> + struct regmap *regmap;
> +};
> +
> +static int adp5585_gpio_get_direction(struct gpio_chip *chip, unsigned int off)
> +{
> + struct adp5585_gpio_dev *adp5585_gpio = gpiochip_get_data(chip);
> + unsigned int bank = ADP5585_BANK(off);
> + unsigned int bit = ADP5585_BIT(off);
> + unsigned int val;
> +
> + regmap_read(adp5585_gpio->regmap, ADP5585_GPIO_DIRECTION_A + bank, &val);
> +
> + return val & bit ? GPIO_LINE_DIRECTION_OUT : GPIO_LINE_DIRECTION_IN;
> +}
> +
> +static int adp5585_gpio_direction_input(struct gpio_chip *chip, unsigned int off)
> +{
> + struct adp5585_gpio_dev *adp5585_gpio = gpiochip_get_data(chip);
> + unsigned int bank = ADP5585_BANK(off);
> + unsigned int bit = ADP5585_BIT(off);
> +
> + return regmap_update_bits(adp5585_gpio->regmap,
> + ADP5585_GPIO_DIRECTION_A + bank,
> + bit, 0);

regmap_clear_bits()?

> +}
> +
> +static int adp5585_gpio_direction_output(struct gpio_chip *chip, unsigned int off, int val)
> +{
> + struct adp5585_gpio_dev *adp5585_gpio = gpiochip_get_data(chip);
> + unsigned int bank = ADP5585_BANK(off);
> + unsigned int bit = ADP5585_BIT(off);
> + int ret;
> +
> + ret = regmap_update_bits(adp5585_gpio->regmap,
> + ADP5585_GPO_DATA_OUT_A + bank, bit,
> + val ? bit : 0);
> + if (ret)
> + return ret;
> +
> + return regmap_update_bits(adp5585_gpio->regmap,
> + ADP5585_GPIO_DIRECTION_A + bank,
> + bit, bit);

regmap_set_bits()?

> +}
> +
> +static int adp5585_gpio_get_value(struct gpio_chip *chip, unsigned int off)
> +{
> + struct adp5585_gpio_dev *adp5585_gpio = gpiochip_get_data(chip);
> + unsigned int bank = ADP5585_BANK(off);
> + unsigned int bit = ADP5585_BIT(off);
> + unsigned int reg;
> + unsigned int val;
> +
> + /*
> + * The input status register doesn't reflect the pin state when the
> + * GPIO is configured as an output. Check the direction, and read the
> + * input status from GPI_STATUS or output value from GPO_DATA_OUT
> + * accordingly.
> + *
> + * We don't need any locking, as concurrent access to the same GPIO
> + * isn't allowed by the GPIO API, so there's no risk of the
> + * .direction_input(), .direction_output() or .set() operations racing
> + * with this.
> + */
> + regmap_read(adp5585_gpio->regmap, ADP5585_GPIO_DIRECTION_A + bank, &val);
> + reg = val & bit ? ADP5585_GPO_DATA_OUT_A : ADP5585_GPI_STATUS_A;
> + regmap_read(adp5585_gpio->regmap, reg + bank, &val);
> +
> + return !!(val & bit);
> +}
> +
> +static void adp5585_gpio_set_value(struct gpio_chip *chip, unsigned int off, int val)
> +{
> + struct adp5585_gpio_dev *adp5585_gpio = gpiochip_get_data(chip);
> + unsigned int bank = ADP5585_BANK(off);
> + unsigned int bit = ADP5585_BIT(off);
> +
> + regmap_update_bits(adp5585_gpio->regmap, ADP5585_GPO_DATA_OUT_A + bank,
> + bit, val ? bit : 0);
> +}
> +
> +static int adp5585_gpio_set_bias(struct adp5585_gpio_dev *adp5585_gpio,
> + unsigned int off, unsigned int bias)
> +{
> + unsigned int bit, reg, mask, val;
> +
> + /*
> + * The bias configuration fields are 2 bits wide and laid down in
> + * consecutive registers ADP5585_RPULL_CONFIG_*, with a hole of 4 bits
> + * after R5.
> + */
> + bit = off * 2 + (off > 5 ? 4 : 0);
> + reg = ADP5585_RPULL_CONFIG_A + bit / 8;
> + mask = ADP5585_Rx_PULL_CFG_MASK << (bit % 8);
> + val = bias << (bit % 8);
> +
> + return regmap_update_bits(adp5585_gpio->regmap, reg, mask, val);
> +}
> +
> +static int adp5585_gpio_set_drive(struct adp5585_gpio_dev *adp5585_gpio,
> + unsigned int off, enum pin_config_param drive)
> +{
> + unsigned int bank = ADP5585_BANK(off);
> + unsigned int bit = ADP5585_BIT(off);
> +
> + return regmap_update_bits(adp5585_gpio->regmap,
> + ADP5585_GPO_OUT_MODE_A + bank, bit,
> + drive == PIN_CONFIG_DRIVE_OPEN_DRAIN ? 1 : 0);

look like should be

drive == PIN_CONFIG_DRIVE_OPEN_DRAIN ? bit : 0

same as the other place.

> +}
> +
> +static int adp5585_gpio_set_debounce(struct adp5585_gpio_dev *adp5585_gpio,
> + unsigned int off, unsigned int debounce)
> +{
> + unsigned int bank = ADP5585_BANK(off);
> + unsigned int bit = ADP5585_BIT(off);
> +
> + return regmap_update_bits(adp5585_gpio->regmap,
> + ADP5585_DEBOUNCE_DIS_A + bank, bit,
> + debounce ? 0 : 1);
> +}
> +
> +static int adp5585_gpio_set_config(struct gpio_chip *chip, unsigned int off,
> + unsigned long config)
> +{
> + struct adp5585_gpio_dev *adp5585_gpio = gpiochip_get_data(chip);
> + enum pin_config_param param = pinconf_to_config_param(config);
> + u32 arg = pinconf_to_config_argument(config);
> +
> + switch (param) {
> + case PIN_CONFIG_BIAS_DISABLE:
> + return adp5585_gpio_set_bias(adp5585_gpio, off,
> + ADP5585_Rx_PULL_CFG_DISABLE);
> +
> + case PIN_CONFIG_BIAS_PULL_DOWN:
> + return adp5585_gpio_set_bias(adp5585_gpio, off, arg ?
> + ADP5585_Rx_PULL_CFG_PD_300K :
> + ADP5585_Rx_PULL_CFG_DISABLE);
> +
> + case PIN_CONFIG_BIAS_PULL_UP:
> + return adp5585_gpio_set_bias(adp5585_gpio, off, arg ?
> + ADP5585_Rx_PULL_CFG_PU_300K :
> + ADP5585_Rx_PULL_CFG_DISABLE);
> +
> + case PIN_CONFIG_DRIVE_OPEN_DRAIN:
> + case PIN_CONFIG_DRIVE_PUSH_PULL:
> + return adp5585_gpio_set_drive(adp5585_gpio, off, param);
> +
> + case PIN_CONFIG_INPUT_DEBOUNCE:
> + return adp5585_gpio_set_debounce(adp5585_gpio, off, arg);
> +
> + default:
> + return -ENOTSUPP;
> + };
> +}
> +
> +static int adp5585_gpio_probe(struct platform_device *pdev)
> +{
> + struct adp5585_dev *adp5585 = dev_get_drvdata(pdev->dev.parent);
> + struct adp5585_gpio_dev *adp5585_gpio;
> + struct device *dev = &pdev->dev;
> + struct gpio_chip *gc;
> + int ret;
> +
> + adp5585_gpio = devm_kzalloc(dev, sizeof(*adp5585_gpio), GFP_KERNEL);
> + if (!adp5585_gpio)
> + return -ENOMEM;
> +
> + adp5585_gpio->regmap = adp5585->regmap;
> +
> + device_set_of_node_from_dev(dev, dev->parent);
> +
> + gc = &adp5585_gpio->gpio_chip;
> + gc->parent = dev;
> + gc->get_direction = adp5585_gpio_get_direction;
> + gc->direction_input = adp5585_gpio_direction_input;
> + gc->direction_output = adp5585_gpio_direction_output;
> + gc->get = adp5585_gpio_get_value;
> + gc->set = adp5585_gpio_set_value;
> + gc->set_config = adp5585_gpio_set_config;
> + gc->can_sleep = true;
> +
> + gc->base = -1;
> + gc->ngpio = ADP5585_GPIO_MAX;
> + gc->label = pdev->name;
> + gc->owner = THIS_MODULE;
> +
> + ret = devm_gpiochip_add_data(dev, &adp5585_gpio->gpio_chip,
> + adp5585_gpio);
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to add GPIO chip\n");
> +
> + return 0;
> +}
> +
> +static const struct platform_device_id adp5585_gpio_id_table[] = {
> + { "adp5585-gpio" },
> + { /* Sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(platform, adp5585_gpio_id_table);
> +
> +static struct platform_driver adp5585_gpio_driver = {
> + .driver = {
> + .name = "adp5585-gpio",
> + },
> + .probe = adp5585_gpio_probe,
> + .id_table = adp5585_gpio_id_table,
> +};
> +module_platform_driver(adp5585_gpio_driver);
> +
> +MODULE_AUTHOR("Haibo Chen <haibo.chen@xxxxxxx>");
> +MODULE_DESCRIPTION("GPIO ADP5585 Driver");
> +MODULE_LICENSE("GPL");
> --
> Regards,
>
> Laurent Pinchart
>

Next message: pr-tracker-bot: "Re: [GIT PULL] power sequencing fixes for v6.11-rc1"
Previous message: Frank Li: "Re: [PATCH v5 2/4] mfd: adp5585: Add Analog Devices ADP5585 core support"
In reply to: Laurent Pinchart: "[PATCH v5 3/4] gpio: adp5585: Add Analog Devices ADP5585 support"
Next in thread: Laurent Pinchart: "[PATCH v5 2/4] mfd: adp5585: Add Analog Devices ADP5585 core support"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]