linux-kernel - Re: [PATCH v4 5/6] gpiolib: consolidate GPIO lookups

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 13 Nov 2022 15:07:50 +0200
From:   Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To:     Dmitry Torokhov <dmitry.torokhov@...il.com>
Cc:     Bartosz Golaszewski <brgl@...ev.pl>,
        Linus Walleij <linus.walleij@...aro.org>,
        linux-gpio@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-acpi@...r.kernel.org
Subject: Re: [PATCH v4 5/6] gpiolib: consolidate GPIO lookups

On Fri, Nov 11, 2022 at 02:19:07PM -0800, Dmitry Torokhov wrote:
> Ensure that all paths to obtain/look up GPIOD from generic
> consumer-visible APIs go through the new gpiod_find_and_request()
> helper, so that we can easily extend it with support for new firmware
> mechanisms.
> 
> The only exception is OF-specific [devm_]gpiod_get_from_of_node() API
> that is still being used by a couple of drivers and will be removed as
> soon as patches converting them to use generic fwnode/device APIs are
> accepted.

...

> +static struct gpio_desc *gpiod_find_and_request(struct device *consumer,
> +						struct fwnode_handle *fwnode,
> +						const char *con_id,
> +						unsigned int idx,
> +						enum gpiod_flags flags,
> +						const char *label,
> +						bool platform_lookup_allowed)
> +{
> +	struct gpio_desc *desc = ERR_PTR(-ENOENT);
> +	unsigned long lookupflags;
> +	int ret;

> +	if (!IS_ERR_OR_NULL(fwnode))

Just for the record. I haven't given my tag to this patch, because I think that
the above check (and respective assignment above) are redundant. Even comment
inside the below condition clarifies the point of the meaning of descriptor
being not found. Besides that many of device property APIs designed the way
that input fwnode can be invalid.

Nevertheless, we agreed with Dmitry that this disagreement should be solved on
maintainer's level, while it doesn't affect code functionality.

The rest of the patch is fine.

> +		desc = gpiod_find_by_fwnode(fwnode, consumer, con_id, idx,
> +					    &flags, &lookupflags);
> +
> +	if (gpiod_not_found(desc) && platform_lookup_allowed) {
> +		/*
> +		 * Either we are not using DT or ACPI, or their lookup did not
> +		 * return a result. In that case, use platform lookup as a
> +		 * fallback.
> +		 */
> +		dev_dbg(consumer, "using lookup tables for GPIO lookup\n");
> +		desc = gpiod_find(consumer, con_id, idx, &lookupflags);
> +	}
> +
> +	if (IS_ERR(desc)) {
> +		dev_dbg(consumer, "No GPIO consumer %s found\n", con_id);
> +		return desc;
> +	}

-- 
With Best Regards,
Andy Shevchenko