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]
Message-ID: <916654ca-e70f-5663-f3a3-9b370c24aea9@i2se.com>
Date:   Fri, 13 Jan 2023 21:13:23 +0100
From:   Stefan Wahren <stefan.wahren@...e.com>
To:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Bartosz Golaszewski <bartosz.golaszewski@...aro.org>,
        Dmitry Torokhov <dmitry.torokhov@...il.com>,
        Linus Walleij <linus.walleij@...aro.org>,
        linux-gpio@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-rpi-kernel@...ts.infradead.org,
        linux-arm-kernel@...ts.infradead.org
Cc:     Bartosz Golaszewski <brgl@...ev.pl>,
        Florian Fainelli <f.fainelli@...il.com>,
        Broadcom internal kernel review list 
        <bcm-kernel-feedback-list@...adcom.com>,
        Ray Jui <rjui@...adcom.com>,
        Scott Branden <sbranden@...adcom.com>
Subject: Re: [PATCH v2 1/2] pinctrl: bcm: bcm2835: Switch to use
 ->add_pin_ranges()

Hi Andy,

Am 13.01.23 um 18:10 schrieb Andy Shevchenko:
> Yeah, while the ->add_pin_ranges() shouldn't be used by DT drivers,
> this one requires it to support quite old firmware descriptions that
> do not have gpio-ranges property.
>
> The change allows to clean up GPIO library from OF specifics.
> There is no functional change intended.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
> ---
> v2: fixed compilation issues (LKP), Cc'ed to the author of original code
>
> Btw, the commit d2b67744fd99 ("pinctrl: bcm2835: implement hook for
> missing gpio-ranges") seems problematic in the fist place due to
> odd of_node_put() call. I dunno how that part had been tested, or
> how it's supposed to work, i.e. where is the counterpart of_node_get().
> Anyway this change drops it for good.

The countpart is in of_pinctrl_get(). I was just following the pattern 
like in other drivers like gpio-rockchip. The original commit has been 
tested by Florian Fainelli and me. I'm not sure if it's safe to drop it 
completely.

Btw this is not the only platform affected by the gpio-ranges 
compatibility issue [1].

> Perhaps we can check gpio-ranges property presence inside the GPIO
> library, so this ->add_pin_ranges() won't be called at all.

I thought this could be very platform specific, so i implemented a hook. 
But yes my initial hack modified gpiolib-of [2].

[1] 
- https://patchwork.kernel.org/project/linux-arm-msm/patch/20180412190138.12372-1-chunkeey@gmail.com/

[2] - 
https://lore.kernel.org/linux-arm-kernel/75266ed1-666a-138b-80f1-ae9a06b7bdf3@i2se.com/

> Also I would like to understand the dance around checking for pin
> control device. The original commit lacks of comments in the non-trivial
> code.
>
>   drivers/pinctrl/bcm/pinctrl-bcm2835.c | 13 +++++++------
>   1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/pinctrl/bcm/pinctrl-bcm2835.c b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
> index 7857e612a100..29f278c49103 100644
> --- a/drivers/pinctrl/bcm/pinctrl-bcm2835.c
> +++ b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
> @@ -358,16 +358,17 @@ static int bcm2835_gpio_direction_output(struct gpio_chip *chip,
>   	return 0;
>   }
>   
> -static int bcm2835_of_gpio_ranges_fallback(struct gpio_chip *gc,
> -					   struct device_node *np)
> +static int bcm2835_add_pin_ranges_fallback(struct gpio_chip *gc)
>   {
> +	struct device_node *np = dev_of_node(gc->parent);
>   	struct pinctrl_dev *pctldev = of_pinctrl_get(np);
>   
> -	of_node_put(np);
> -
>   	if (!pctldev)
>   		return 0;
>   
> +	if (of_property_read_bool(np, "gpio-ranges"))
> +		return 0;
> +
>   	gpiochip_add_pin_range(gc, pinctrl_dev_get_devname(pctldev), 0, 0,
>   			       gc->ngpio);
>   
> @@ -388,7 +389,7 @@ static const struct gpio_chip bcm2835_gpio_chip = {
>   	.base = -1,
>   	.ngpio = BCM2835_NUM_GPIOS,
>   	.can_sleep = false,
> -	.of_gpio_ranges_fallback = bcm2835_of_gpio_ranges_fallback,
> +	.add_pin_ranges = bcm2835_add_pin_ranges_fallback,
>   };
>   
>   static const struct gpio_chip bcm2711_gpio_chip = {
> @@ -405,7 +406,7 @@ static const struct gpio_chip bcm2711_gpio_chip = {
>   	.base = -1,
>   	.ngpio = BCM2711_NUM_GPIOS,
>   	.can_sleep = false,
> -	.of_gpio_ranges_fallback = bcm2835_of_gpio_ranges_fallback,
> +	.add_pin_ranges = bcm2835_add_pin_ranges_fallback,
>   };
>   
>   static void bcm2835_gpio_irq_handle_bank(struct bcm2835_pinctrl *pc,

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ