Re: [PATCH v2 4/7] gpio: max7360: Add MAX7360 gpio support

[Christophe JAILLET]

Le 23/12/2024 à 17:42, Mathieu Dubois-Briand a écrit :
> Add driver for Maxim Integrated MAX7360 GPIO/GPO controller.
>
> Two sets of GPIOs are provided by the device:
> - Up to 8 GPIOs, shared with the PWM and rotary encoder functionalities.
> These GPIOs also provide interrupts on input changes.
> - Up to 6 GPOs, on unused keypad columns pins.
>
> Co-developed-by: Kamel Bouhara <kamel....@bootlin.com>
> Signed-off-by: Kamel Bouhara <kamel....@bootlin.com>
> Signed-off-by: Mathieu Dubois-Briand <mathieu.du...@bootlin.com>

...

> +static int max7360_gpio_probe(struct platform_device *pdev)
> +{
> + struct max7360_gpio *max7360_gpio;
> + struct platform_device *parent;
> + unsigned int ngpios;
> + unsigned int outconf;
> + struct gpio_irq_chip *girq;
> + unsigned long flags;
> + int irq;
> + int ret;
> +
> + if (!pdev->dev.parent) {
> + dev_err(&pdev->dev, "no parent device\n");
> + return -ENODEV;
> + }
> + parent = to_platform_device(pdev->dev.parent);
> +
> + max7360_gpio = devm_kzalloc(&pdev->dev, sizeof(struct max7360_gpio),

sizeof(*max7360_gpio)?

> + GFP_KERNEL);
> + if (!max7360_gpio)
> + return -ENOMEM;
> +
> + if (of_property_read_u32(pdev->dev.of_node, "ngpios", &ngpios)) {
> + dev_err(&pdev->dev, "Missing ngpios OF property\n");
> + return -ENODEV;
> + }
> +
> + max7360_gpio->regmap = dev_get_regmap(pdev->dev.parent, NULL);
> + if (!max7360_gpio->regmap) {
> + dev_err(&pdev->dev, "could not get parent regmap\n");
> + return -ENODEV;
> + }
> +
> + max7360_gpio->dev = &pdev->dev;
> + max7360_gpio->chip = max7360_gpio_chip;
> + max7360_gpio->chip.ngpio = ngpios;
> + max7360_gpio->chip.parent = &pdev->dev;
> + max7360_gpio->chip.base = -1;
> + max7360_gpio->gpio_function = (uintptr_t)device_get_match_data(&pdev->dev);
> +
> + dev_dbg(&pdev->dev, "gpio count: %d\n", max7360_gpio->chip.ngpio);
> +
> + if (max7360_gpio->gpio_function == MAX7360_GPIO_PORT) {
> + /* Port GPIOs: set output mode configuration (constant-current
> + * or not).
> + * This property is optional.
> + */
> + outconf = 0;
> + ret = of_property_read_u32(pdev->dev.of_node,
> + "maxim,constant-current-disable",
> + &outconf);
> + if (ret && (ret != -EINVAL)) {
> + dev_err(&pdev->dev,
> + "Failed to read maxim,constant-current-disable OF property\n");
> + return -ENODEV;
> + }
> +
> + regmap_write(max7360_gpio->regmap, MAX7360_REG_GPIOOUTM, outconf);
> + }
> +
> + if (max7360_gpio->gpio_function == MAX7360_GPIO_PORT &&
> + of_property_read_bool(pdev->dev.of_node, "interrupt-controller")) {
> + /* Port GPIOs: declare IRQ chip, if configuration was provided.
> + */
> + irq = platform_get_irq_byname(parent, "inti");
> + if (irq < 0)
> + return dev_err_probe(&pdev->dev, irq,
> + "Failed to get IRQ");
> +
> + flags = IRQF_TRIGGER_LOW | IRQF_ONESHOT | IRQF_SHARED;
> + ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
> + max7360_gpio_irq, flags,
> + "max7360-gpio", max7360_gpio);
> + if (ret)
> + return dev_err_probe(&pdev->dev, ret,
> + "Failed to register interrupt: %d\n",
> + ret);

No need to duplicate ret in the message.

> +
> + girq = &max7360_gpio->chip.irq;
> + gpio_irq_chip_set_chip(girq, &max7360_gpio_irqchip);
> + girq->parent_handler = NULL;
> + girq->num_parents = 0;
> + girq->parents = NULL;
> + girq->threaded = true;
> + girq->default_type = IRQ_TYPE_NONE;
> + girq->handler = handle_simple_irq;
> + }
> +
> + ret = devm_gpiochip_add_data(&pdev->dev, &max7360_gpio->chip, max7360_gpio);
> + if (ret) {
> + dev_err(&pdev->dev, "unable to add gpiochip: %d\n", ret);

return dev_err_probe()?

> + return ret;
> + }
> +
> + return 0;
> +}

CJ