Re: [PATCH v13 4/6] pinctrl: add NXP MC33978/MC34978 pinctrl driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The GPIO chip is exposed to consumers before its pin ranges are established, creating a race condition.
--

commit 3bc65f1451eade57ad8e2b23e1cd7f59d63eb878
Author: David Jander <david@protonic.nl>

pinctrl: add NXP MC33978/MC34978 pinctrl driver

Add pin control and GPIO driver for the NXP MC33978/MC34978 Multiple Switch
Detection Interface (MSDI) devices. This driver exposes the 22 mechanical
switch detection inputs as standard GPIOs and supports emulated outputs
and hierarchical interrupt routing.

> diff --git a/drivers/pinctrl/pinctrl-mc33978.c b/drivers/pinctrl/pinctrl-mc33978.c
> new file mode 100644
> index 0000000000000..5ac4ebb115b66
> --- /dev/null
> +++ b/drivers/pinctrl/pinctrl-mc33978.c

[ ... ]

> +	mpc->pctldev = devm_pinctrl_register(dev, &mpc->pinctrl_desc, mpc);
> +	if (IS_ERR(mpc->pctldev))
> +		return dev_err_probe(dev, PTR_ERR(mpc->pctldev),
> +				     "can't register pinctrl\n");
> +
> +	ret = devm_gpiochip_add_data(dev, &mpc->chip, mpc);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "can't add GPIO chip\n");
> +
> +	/*
> +	 * Distinguish GPIO IRQ domain from parent MFD domain sharing the same
> +	 * fwnode. Matches the pattern used by other GPIO drivers (e.g.,
> +	 * crystalcove). DOMAIN_BUS_WIRED indicates this domain represents
> +	 * actual GPIO pin interrupts (wired lines).
> +	 */
> +	irq_domain_update_bus_token(mpc->chip.irq.domain, DOMAIN_BUS_WIRED);
> +
> +	ret = gpiochip_add_pin_range(&mpc->chip, dev_name(dev), 0, 0,
> +				     MC33978_NGPIO);

Could this create a race condition where the GPIO chip is exposed to
consumers before its pin ranges are established?

If a consumer requests a GPIO in the narrow window after
devm_gpiochip_add_data() completes but before gpiochip_add_pin_range()
is called, gpiolib's pinctrl_gpio_request() might not find the pinctrl
mapping and could bypass the pinctrl back-end synchronization.

Additionally, if gpiochip_add_pin_range() fails, returning an error here
would unregister the already-exposed gpiochip, potentially leaving
consumers with dangling handles.

Would it be better to implement the .add_pin_ranges callback in
struct gpio_chip so that gpiolib can safely and atomically establish the
pin ranges under its own internal locks during registration?

> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to add pin range\n");
> +
> +	platform_set_drvdata(pdev, mpc);
> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260518100054.3552143-1-o.rempel@pengutronix.de?part=4