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.bouhara@xxxxxxxxxxx>
> Signed-off-by: Kamel Bouhara <kamel.bouhara@xxxxxxxxxxx>
> Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@xxxxxxxxxxx>

...

> +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