linux-kernel - Re: [PATCH 4/5] pinctrl: actions: Add gpio support for Actions S900 SoC

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] [day] [month] [year] [list]

Message-ID: <20180519101149.7bq6vjiibyrsedv3@linaro.org>
Date:   Sat, 19 May 2018 15:41:49 +0530
From:   Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
To:     Christian Lamparter <chunkeey@...il.com>
Cc:     linus.walleij@...aro.org, robh+dt@...nel.org, afaerber@...e.de,
        liuwei@...ions-semi.com, mp-cs@...ions-semi.com,
        96boards@...obotics.com, devicetree@...r.kernel.org,
        andy.shevchenko@...il.com, daniel.thompson@...aro.org,
        amit.kucheria@...aro.org, linux-arm-kernel@...ts.infradead.org,
        linux-gpio@...r.kernel.org, linux-kernel@...r.kernel.org,
        hzhang@...obotics.com, bdong@...obotics.com,
        manivannanece23@...il.com
Subject: Re: [PATCH 4/5] pinctrl: actions: Add gpio support for Actions S900
 SoC

Hi Christian,

On Sat, May 19, 2018 at 11:18:53AM +0200, Christian Lamparter wrote:
> On Friday, May 18, 2018 4:30:55 AM CEST Manivannan Sadhasivam wrote:
> > Add gpio support to pinctrl driver for Actions Semi S900 SoC.
> > 
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
> > ---
> > [...]
> > +static int owl_gpio_init(struct owl_pinctrl *pctrl)
> > +{
> > +	struct gpio_chip *chip;
> > +	int ret;
> > +
> > +	chip = &pctrl->chip;
> > +	chip->base = -1;
> > +	chip->ngpio = pctrl->soc->ngpios;
> > +	chip->label = dev_name(pctrl->dev);
> > +	chip->parent = pctrl->dev;
> > +	chip->owner = THIS_MODULE;
> > +	chip->of_node = pctrl->dev->of_node;
> > +
> > +	ret = gpiochip_add_data(&pctrl->chip, pctrl);
> > +	if (ret) {
> > +		dev_err(pctrl->dev, "failed to register gpiochip\n");
> > +		return ret;
> > +	}
> > +
> > +	ret = gpiochip_add_pin_range(&pctrl->chip, dev_name(pctrl->dev),
> > +							0, 0, chip->ngpio);
> > +	if (ret) {
> > +		dev_err(pctrl->dev, "failed to add pin range\n");
> > +		gpiochip_remove(&pctrl->chip);
> > +		return ret;
> > +	}
> > +
> gpiochip_add_pin_range()? That's not going to work with gpio-hogs. 
> 

Hmmm. Just looked into the gpio-hog mechanism and the patch you have
implemented for MSM driver. I agree with you on replacing
gpiochip_add_pin_range() with gpio-ranges property. But I'm curious
whether we should document it somewhere or not (probably in [1]).

Anyway I will send the v2 incorporating your suggestion.

Thanks,
Mani

[1] Documentation/devicetree/bindings/gpio/gpio.txt

> But, you can easily test this. Just add a gpio-hog [0] 
> ( Section 2. gpio-controller nodes) into the Devicetree's
> pinctrl node.
> 
> something like: (No idea if GPIO1 is already used, but any free
> gpio will do)
> | 		[...]
> |		pinctrl@...b0000 {
> |			compatible = "actions,s900-pinctrl";
> |			reg = <0x0 0xe01b0000 0x0 0x1000>;
> |			clocks = <&cmu CLK_GPIO>;
> |			gpio-controller;
> |			#gpio-cells = <2>;
> |
> |			line_b {
> |				gpio-hog;
> |				gpios = <1 GPIO_ACTIVE_HIGH>;
> |				output-low;
> |				line-name = "foo-bar-gpio";
> |			};
> |		};
> 
> The pinctrl probe will fail. You can fix this by
> replacing the gpiochip_add_pin_range() and use
> the gpio-ranges [0] property to define the range.
> 
> [0] <https://www.kernel.org/doc/Documentation/devicetree/bindings/gpio/gpio.txt>
> 
> 
>