| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <BY1PR0301MB090160B978C729C4FB022D918E6E0@BY1PR0301MB0901.namprd03.prod.outlook.com>
Date: Tue, 5 Feb 2019 16:00:56 +0000
From: "Hennerich, Michael" <Michael.Hennerich@...log.com>
To: Nikolaus Voss <nv@...n.de>,
Linus Walleij <linus.walleij@...aro.org>
CC: "linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Nikolaus Voss <nikolaus.voss@...wensteinmedical.de>
Subject: RE: [PATCH 1/2] drivers/gpio/gpio-adp5588.c: add device tree support
> -----Original Message-----
> From: Nikolaus Voss [mailto:nv@...n.de]
> Sent: Mittwoch, 25. Juli 2018 10:44
> To: Hennerich, Michael <Michael.Hennerich@...log.com>; Linus Walleij <linus.walleij@...aro.org>
> Cc: linux-gpio@...r.kernel.org; linux-kernel@...r.kernel.org; Nikolaus Voss <nikolaus.voss@...wensteinmedical.de>
> Subject: [PATCH 1/2] drivers/gpio/gpio-adp5588.c: add device tree support
>
> Make platform data optional and add DT id table.
> Switch to dynamically mapped GPIOs and IRQs if not provided
> via platform data.
Long overdue maintenance patch...
Looks good to me.
Thanks for this series!
>
> Signed-off-by: Nikolaus Voss <nikolaus.voss@...wensteinmedical.de>
Acked-by: Michael Hennerich <michael.hennerich@...log.com>
> ---
> drivers/gpio/gpio-adp5588.c | 151 ++++++++++++++++--------------------
> 1 file changed, 68 insertions(+), 83 deletions(-)
>
> diff --git a/drivers/gpio/gpio-adp5588.c b/drivers/gpio/gpio-adp5588.c
> index da9781a2ef4a..0a8cfccba818 100644
> --- a/drivers/gpio/gpio-adp5588.c
> +++ b/drivers/gpio/gpio-adp5588.c
> @@ -15,6 +15,7 @@
> #include <linux/gpio/driver.h>
> #include <linux/interrupt.h>
> #include <linux/irq.h>
> +#include <linux/of_device.h>
>
> #include <linux/platform_data/adp5588.h>
>
> @@ -33,8 +34,6 @@ struct adp5588_gpio {
> struct mutex lock; /* protect cached dir, dat_out */
> /* protect serialized access to the interrupt controller bus */
> struct mutex irq_lock;
> - unsigned gpio_start;
> - unsigned irq_base;
> uint8_t dat_out[3];
> uint8_t dir[3];
> uint8_t int_lvl[3];
> @@ -148,16 +147,11 @@ static int adp5588_gpio_direction_output(struct gpio_chip *chip,
> }
>
> #ifdef CONFIG_GPIO_ADP5588_IRQ
> -static int adp5588_gpio_to_irq(struct gpio_chip *chip, unsigned off)
> -{
> - struct adp5588_gpio *dev = gpiochip_get_data(chip);
> -
> - return dev->irq_base + off;
> -}
>
> static void adp5588_irq_bus_lock(struct irq_data *d)
> {
> - struct adp5588_gpio *dev = irq_data_get_irq_chip_data(d);
> + struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> + struct adp5588_gpio *dev = gpiochip_get_data(gc);
>
> mutex_lock(&dev->irq_lock);
> }
> @@ -172,7 +166,8 @@ static void adp5588_irq_bus_lock(struct irq_data *d)
>
> static void adp5588_irq_bus_sync_unlock(struct irq_data *d)
> {
> - struct adp5588_gpio *dev = irq_data_get_irq_chip_data(d);
> + struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> + struct adp5588_gpio *dev = gpiochip_get_data(gc);
> int i;
>
> for (i = 0; i <= ADP5588_BANK(ADP5588_MAXGPIO); i++) {
> @@ -203,24 +198,25 @@ static void adp5588_irq_bus_sync_unlock(struct irq_data *d)
>
> static void adp5588_irq_mask(struct irq_data *d)
> {
> - struct adp5588_gpio *dev = irq_data_get_irq_chip_data(d);
> - unsigned gpio = d->irq - dev->irq_base;
> + struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> + struct adp5588_gpio *dev = gpiochip_get_data(gc);
>
> - dev->irq_mask[ADP5588_BANK(gpio)] &= ~ADP5588_BIT(gpio);
> + dev->irq_mask[ADP5588_BANK(d->hwirq)] &= ~ADP5588_BIT(d->hwirq);
> }
>
> static void adp5588_irq_unmask(struct irq_data *d)
> {
> - struct adp5588_gpio *dev = irq_data_get_irq_chip_data(d);
> - unsigned gpio = d->irq - dev->irq_base;
> + struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> + struct adp5588_gpio *dev = gpiochip_get_data(gc);
>
> - dev->irq_mask[ADP5588_BANK(gpio)] |= ADP5588_BIT(gpio);
> + dev->irq_mask[ADP5588_BANK(d->hwirq)] |= ADP5588_BIT(d->hwirq);
> }
>
> static int adp5588_irq_set_type(struct irq_data *d, unsigned int type)
> {
> - struct adp5588_gpio *dev = irq_data_get_irq_chip_data(d);
> - uint16_t gpio = d->irq - dev->irq_base;
> + struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> + struct adp5588_gpio *dev = gpiochip_get_data(gc);
> + uint16_t gpio = d->hwirq;
> unsigned bank, bit;
>
> if ((type & IRQ_TYPE_EDGE_BOTH)) {
> @@ -281,10 +277,11 @@ static irqreturn_t adp5588_irq_handler(int irq, void *devid)
>
> while (pending) {
> if (pending & (1 << bit)) {
> - handle_nested_irq(dev->irq_base +
> - (bank << 3) + bit);
> + handle_nested_irq(
> + irq_find_mapping(
> + dev->gpio_chip.irq.domain,
> + (bank << 3) + bit));
> pending &= ~(1 << bit);
> -
> }
> bit++;
> }
> @@ -299,53 +296,43 @@ static irqreturn_t adp5588_irq_handler(int irq, void *devid)
> static int adp5588_irq_setup(struct adp5588_gpio *dev)
> {
> struct i2c_client *client = dev->client;
> + int ret;
> struct adp5588_gpio_platform_data *pdata =
> dev_get_platdata(&client->dev);
> - unsigned gpio;
> - int ret;
> + int irq_base = pdata ? pdata->irq_base : 0;
>
> adp5588_gpio_write(client, CFG, ADP5588_AUTO_INC);
> adp5588_gpio_write(client, INT_STAT, -1); /* status is W1C */
> adp5588_gpio_read_intstat(client, dev->irq_stat); /* read to clear */
>
> - dev->irq_base = pdata->irq_base;
> mutex_init(&dev->irq_lock);
>
> - for (gpio = 0; gpio < dev->gpio_chip.ngpio; gpio++) {
> - int irq = gpio + dev->irq_base;
> - irq_set_chip_data(irq, dev);
> - irq_set_chip_and_handler(irq, &adp5588_irq_chip,
> - handle_level_irq);
> - irq_set_nested_thread(irq, 1);
> - irq_modify_status(irq, IRQ_NOREQUEST, IRQ_NOPROBE);
> - }
> -
> - ret = request_threaded_irq(client->irq,
> - NULL,
> - adp5588_irq_handler,
> - IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> - dev_name(&client->dev), dev);
> + ret = devm_request_threaded_irq(&client->dev, client->irq,
> + NULL, adp5588_irq_handler, IRQF_ONESHOT
> + | IRQF_TRIGGER_FALLING | IRQF_SHARED,
> + dev_name(&client->dev), dev);
> if (ret) {
> dev_err(&client->dev, "failed to request irq %d\n",
> client->irq);
> - goto out;
> + return ret;
> }
> + ret = gpiochip_irqchip_add_nested(&dev->gpio_chip,
> + &adp5588_irq_chip, irq_base,
> + handle_simple_irq,
> + IRQ_TYPE_NONE);
> + if (ret) {
> + dev_err(&client->dev,
> + "could not connect irqchip to gpiochip\n");
> + return ret;
> + }
> + gpiochip_set_nested_irqchip(&dev->gpio_chip,
> + &adp5588_irq_chip,
> + client->irq);
>
> - dev->gpio_chip.to_irq = adp5588_gpio_to_irq;
> adp5588_gpio_write(client, CFG,
> ADP5588_AUTO_INC | ADP5588_INT_CFG | ADP5588_GPI_INT);
>
> return 0;
> -
> -out:
> - dev->irq_base = 0;
> - return ret;
> -}
> -
> -static void adp5588_irq_teardown(struct adp5588_gpio *dev)
> -{
> - if (dev->irq_base)
> - free_irq(dev->client->irq, dev);
> }
>
> #else
> @@ -357,24 +344,16 @@ static int adp5588_irq_setup(struct adp5588_gpio *dev)
> return 0;
> }
>
> -static void adp5588_irq_teardown(struct adp5588_gpio *dev)
> -{
> -}
> #endif /* CONFIG_GPIO_ADP5588_IRQ */
>
> -static int adp5588_gpio_probe(struct i2c_client *client,
> - const struct i2c_device_id *id)
> +static int adp5588_gpio_probe(struct i2c_client *client)
> {
> struct adp5588_gpio_platform_data *pdata =
> dev_get_platdata(&client->dev);
> struct adp5588_gpio *dev;
> struct gpio_chip *gc;
> int ret, i, revid;
> -
> - if (!pdata) {
> - dev_err(&client->dev, "missing platform data\n");
> - return -ENODEV;
> - }
> + unsigned int pullup_dis_mask = 0;
>
> if (!i2c_check_functionality(client->adapter,
> I2C_FUNC_SMBUS_BYTE_DATA)) {
> @@ -394,18 +373,24 @@ static int adp5588_gpio_probe(struct i2c_client *client,
> gc->get = adp5588_gpio_get_value;
> gc->set = adp5588_gpio_set_value;
> gc->can_sleep = true;
> + gc->base = -1;
> + gc->parent = &client->dev;
> +
> + if (pdata) {
> + gc->base = pdata->gpio_start;
> + gc->names = pdata->names;
> + pullup_dis_mask = pdata->pullup_dis_mask;
> + }
>
> - gc->base = pdata->gpio_start;
> gc->ngpio = ADP5588_MAXGPIO;
> gc->label = client->name;
> gc->owner = THIS_MODULE;
> - gc->names = pdata->names;
>
> mutex_init(&dev->lock);
>
> ret = adp5588_gpio_read(dev->client, DEV_ID);
> if (ret < 0)
> - goto err;
> + return ret;
>
> revid = ret & ADP5588_DEVICE_ID_MASK;
>
> @@ -414,30 +399,27 @@ static int adp5588_gpio_probe(struct i2c_client *client,
> dev->dir[i] = adp5588_gpio_read(client, GPIO_DIR1 + i);
> ret |= adp5588_gpio_write(client, KP_GPIO1 + i, 0);
> ret |= adp5588_gpio_write(client, GPIO_PULL1 + i,
> - (pdata->pullup_dis_mask >> (8 * i)) & 0xFF);
> + (pullup_dis_mask >> (8 * i)) & 0xFF);
> ret |= adp5588_gpio_write(client, GPIO_INT_EN1 + i, 0);
> if (ret)
> - goto err;
> + return ret;
> }
>
> - if (pdata->irq_base) {
> + if (client->irq) {
> if (WA_DELAYED_READOUT_REVID(revid)) {
> dev_warn(&client->dev, "GPIO int not supported\n");
> } else {
> ret = adp5588_irq_setup(dev);
> if (ret)
> - goto err;
> + return ret;
> }
> }
>
> ret = devm_gpiochip_add_data(&client->dev, &dev->gpio_chip, dev);
> if (ret)
> - goto err_irq;
> + return ret;
>
> - dev_info(&client->dev, "IRQ Base: %d Rev.: %d\n",
> - pdata->irq_base, revid);
> -
> - if (pdata->setup) {
> + if (pdata && pdata->setup) {
> ret = pdata->setup(client, gc->base, gc->ngpio, pdata->context);
> if (ret < 0)
> dev_warn(&client->dev, "setup failed, %d\n", ret);
> @@ -446,11 +428,6 @@ static int adp5588_gpio_probe(struct i2c_client *client,
> i2c_set_clientdata(client, dev);
>
> return 0;
> -
> -err_irq:
> - adp5588_irq_teardown(dev);
> -err:
> - return ret;
> }
>
> static int adp5588_gpio_remove(struct i2c_client *client)
> @@ -460,7 +437,7 @@ static int adp5588_gpio_remove(struct i2c_client *client)
> struct adp5588_gpio *dev = i2c_get_clientdata(client);
> int ret;
>
> - if (pdata->teardown) {
> + if (pdata && pdata->teardown) {
> ret = pdata->teardown(client,
> dev->gpio_chip.base, dev->gpio_chip.ngpio,
> pdata->context);
> @@ -470,7 +447,7 @@ static int adp5588_gpio_remove(struct i2c_client *client)
> }
> }
>
> - if (dev->irq_base)
> + if (dev->client->irq)
> free_irq(dev->client->irq, dev);
>
> return 0;
> @@ -480,14 +457,22 @@ static const struct i2c_device_id adp5588_gpio_id[] = {
> {DRV_NAME, 0},
> {}
> };
> -
> MODULE_DEVICE_TABLE(i2c, adp5588_gpio_id);
>
> +#ifdef CONFIG_OF
> +static const struct of_device_id adp5588_gpio_of_id[] = {
> + { .compatible = "adi," DRV_NAME, },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, adp5588_gpio_of_id);
> +#endif
> +
> static struct i2c_driver adp5588_gpio_driver = {
> .driver = {
> - .name = DRV_NAME,
> - },
> - .probe = adp5588_gpio_probe,
> + .name = DRV_NAME,
> + .of_match_table = of_match_ptr(adp5588_gpio_of_id),
> + },
> + .probe_new = adp5588_gpio_probe,
> .remove = adp5588_gpio_remove,
> .id_table = adp5588_gpio_id,
> };
> --
> 2.17.1
Powered by blists - more mailing lists