Re: [PATCH 3/4] gpiolib: Add support for GPIO "hog" nodes

From: Nikita Yushchenko <nikita.yoush@cogentembedded.com>
To: Andrey Smirnov <andrew.smirnov@gmail.com>, barebox@lists.infradead.org
Cc: cphealy@gmail.com
Subject: Re: [PATCH 3/4] gpiolib: Add support for GPIO "hog" nodes
Date: Tue, 23 May 2017 09:52:13 +0300	[thread overview]
Message-ID: <46762e56-b2a0-4afd-6967-99b36af1e6fb@cogentembedded.com> (raw)
In-Reply-To: <20170522152420.14443-3-andrew.smirnov@gmail.com>

> +	ret = of_property_read_u32(chip_np, "#gpio-cells", &gpio_cells);
> +	if (ret)
> +		return ret;
> +
> +	if (WARN_ON(gpio_cells != 2))
> +		return -ENOTSUPP;
> +
> +	ret = of_property_read_u32_index(np, "gpios", idx * gpio_cells,
> +					 &gpio_num);
> +	if (ret)
> +		return ret;
> +
> +	ret = of_property_read_u32_index(np, "gpios", idx * gpio_cells + 1,
> +					 &gpio_flags);
> +	if (ret)
> +		return ret;

Doesn't this hardcode interpretation of device tree words in gpio
specification - while this is intended to be gpio-provider specific and
that's why #gpio-cells exist?

> +static int of_gpiochip_scan_gpios(struct gpio_chip *chip)

Not best choice of name for routine that scans hogs?

(although I understand that it comes from linux counterpart)

> -	return 0;
> +	return of_gpiochip_scan_gpios(chip);

Should we fail gpiochip registration on failure to claim hogs?
I don't know.

Nikita

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox