Re: [PATCH v2] gpio: mxc: Shift generic request/free after gpiochip registration

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Linus, Deepak,

On 11/02/2016 12:55 AM, Linus Walleij wrote:
On Tue, Oct 25, 2016 at 8:02 PM, Deepak Das <deepak_das@xxxxxxxxxx> wrote:

generic gpio request/free should be added after gpiocip registration
to validate mapping of gpiochip with pinctrl subsystem.

gpiochip->pin_ranges list contains the information used by pinctrl
subsystem to configure corresponding pins for gpio usage.
This list will be empty if gpiochip fails to map with
pinctrl subsystem for any reason.
For Ex.:-
generic gpio request/free should not be used if IOMUX pin controller
device node is disabled in device tree.

This commit checks above list and skips adding generic gpio request/free
if list is found empty.

Signed-off-by: Deepak Das <deepak_das@xxxxxxxxxx>
Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@xxxxxxxxxx>

So we should not dereference gpiolib internals.

-       if (of_property_read_bool(np, "gpio-ranges")) {
-               port->gc.request = gpiochip_generic_request;
-               port->gc.free = gpiochip_generic_free;
-       }
-
        port->gc.to_irq = mxc_gpio_to_irq;
        port->gc.base = (pdev->id < 0) ? of_alias_get_id(np, "gpio") * 32 :
                                             pdev->id * 32;
@@ -471,6 +466,11 @@ static int mxc_gpio_probe(struct platform_device *pdev)
        if (err)
                goto out_bgio;

+       if (!list_empty(&port->gc.pin_ranges)) {
+               port->gc.request = gpiochip_generic_request;
+               port->gc.free = gpiochip_generic_free;
+       }

Can't you just check the "gpio-ranges" property later instead?

The change was done for a kernel when of_gpiochip_add_pin_range() did not
return an error code and the change is not applicable to the vanilla kernel.

Potentially of_gpiochip_add_pin_range() may fail due to some other reason
than a missing pinctrl device from the supplied phandle (e.g. -ENOMEM),
but in that case gpiochip_add_data() will also fail and the late check
for empty pin_ranges becomes redundant.

Originally the change is needed exclusively to cover the case, when IOMUX
device node is removed or disabled in a board DTB, while GPIOs still can
be requested by other controller drivers. The disabled pinctrl device case
(pinmux/pinctrl settings are done from a bootloader only) is inapplicable
to any boards found in upstream, so I would propose *not* to include this
change or any its derivative fix to the upstream.

If somebody wants to support the scenario described above on a board not
found in vanilla, the best option is to remove gpio-ranges proporty from
the board DTS file along with disabling IOMUX, then the GPIO driver in
its currently existing state can be successfully reused without any
runtime issues.

--
With best wishes,
Vladimir
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux SPI]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux