devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Bjorn Andersson <bjorn.andersson@linaro.org>
To: Stephen Boyd <sboyd@codeaurora.org>
Cc: Linus Walleij <linus.walleij@linaro.org>,
	linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	Timur Tabi <timur@codeaurora.org>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	linux-gpio@vger.kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH 3/3] pinctrl: qcom: Don't allow protected pins to be requested
Date: Tue, 9 Jan 2018 22:11:33 -0800	[thread overview]
Message-ID: <20180110061133.GP12655@minitux> (raw)
In-Reply-To: <20180110015848.11480-4-sboyd@codeaurora.org>

On Tue 09 Jan 17:58 PST 2018, Stephen Boyd wrote:

I like it, a few comment below though.

> +static int msm_gpio_init_irq_valid_mask(struct gpio_chip *chip,
> +					struct msm_pinctrl *pctrl)
> +{
> +	int ret;
> +	unsigned int len, i;
> +	unsigned int max_gpios = pctrl->soc->ngpios;
> +	struct device_node *np = pctrl->dev->of_node;
> +
> +	/* The number of GPIOs in the ACPI tables */
> +	ret = device_property_read_u16_array(pctrl->dev, "gpios", NULL, 0);
> +	if (ret > 0 && ret < max_gpios) {
> +		u16 *tmp;
> +
> +		len = ret;
> +		tmp = kmalloc_array(len, sizeof(tmp[0]), GFP_KERNEL);
> +		if (!tmp)
> +			return -ENOMEM;
> +
> +		ret = device_property_read_u16_array(pctrl->dev, "gpios", tmp,
> +						     len);
> +		if (ret < 0) {
> +			dev_err(pctrl->dev, "could not read list of GPIOs\n");
> +			kfree(tmp);
> +			return ret;
> +		}
> +
> +		bitmap_zero(chip->irq_valid_mask, max_gpios);

The valid_mask is moving into the gpio_irq_chip, so this should be
chip->irq.valid_mask.

See dc7b0387ee89 ("gpio: Move irq_valid_mask into struct gpio_irq_chip")

> +		for (i = 0; i < len; i++)
> +			set_bit(tmp[i], chip->irq_valid_mask);
> +

You're leaking tmp here.

> +		return 0;
> +	}
> +
> +	/* If there's a DT ngpios-ranges property then add those ranges */
> +	ret = of_property_count_u32_elems(np,  "ngpios-ranges");
> +	if (ret > 0 && ret % 2 == 0 && ret / 2 < max_gpios) {
> +		u32 start;
> +		u32 count;
> +
> +		len = ret / 2;
> +		bitmap_zero(chip->irq_valid_mask, max_gpios);
> +
> +		for (i = 0; i < len; i++) {

If you take steps of 2, when looping from 0 to ret, your loop index will
have the value that you're passing as index into the read - without
duplicating it.

> +			of_property_read_u32_index(np, "ngpios-ranges",
> +						   i * 2, &start);
> +			of_property_read_u32_index(np, "ngpios-ranges",
> +						   i * 2 + 1, &count);
> +			bitmap_set(chip->irq_valid_mask, start, count);
> +		}
> +	}
> +
> +	return 0;
> +}
[..]
> @@ -824,6 +907,7 @@ static int msm_gpio_init(struct msm_pinctrl *pctrl)
>  	chip->parent = pctrl->dev;
>  	chip->owner = THIS_MODULE;
>  	chip->of_node = pctrl->dev->of_node;
> +	chip->irq_need_valid_mask = msm_gpio_needs_irq_valid_mask(pctrl);

chip->irq.need_valid_mask

>  
>  	ret = gpiochip_add_data(&pctrl->chip, pctrl);
>  	if (ret) {
> @@ -831,6 +915,12 @@ static int msm_gpio_init(struct msm_pinctrl *pctrl)
>  		return ret;
>  	}
>  
> +	ret = msm_gpio_init_irq_valid_mask(chip, pctrl);
> +	if (ret) {
> +		dev_err(pctrl->dev, "Failed to setup irq valid bits\n");

gpiochip_remove() here, to release resources allocated by
gpiochip_add_data.

> +		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");

Regards,
Bjorn

  reply	other threads:[~2018-01-10  6:11 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-10  1:58 [PATCH 0/3] Support qcom pinctrl protected pins Stephen Boyd
     [not found] ` <20180110015848.11480-1-sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2018-01-10  1:58   ` [PATCH 1/3] gpiolib: Export gpiochip_irqchip_irq_valid() to drivers Stephen Boyd
2018-01-10  6:16     ` Bjorn Andersson
2018-01-10 13:22     ` Linus Walleij
2018-01-10  1:58 ` [PATCH 2/3] dt-bindings: pinctrl: Add a ngpios-ranges property Stephen Boyd
2018-01-10 12:54   ` Andy Shevchenko
     [not found]   ` <20180110015848.11480-3-sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2018-01-10 13:37     ` Linus Walleij
2018-01-10 16:37       ` Stephen Boyd
2018-01-10 17:59         ` Andy Shevchenko
2018-01-11 16:33       ` Grant Likely
2018-01-11 16:36         ` Timur Tabi
2018-01-11 19:56           ` Grant Likely
2018-01-10  1:58 ` [PATCH 3/3] pinctrl: qcom: Don't allow protected pins to be requested Stephen Boyd
2018-01-10  6:11   ` Bjorn Andersson [this message]
2018-01-22 13:55   ` Timur Tabi
2018-01-22 20:03     ` Timur Tabi
2018-01-25 21:51     ` Stephen Boyd
2018-01-25 21:53       ` Timur Tabi
2018-01-25 20:48   ` Timur Tabi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180110061133.GP12655@minitux \
    --to=bjorn.andersson@linaro.org \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=devicetree@vger.kernel.org \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sboyd@codeaurora.org \
    --cc=timur@codeaurora.org \
    --subject='Re: [PATCH 3/3] pinctrl: qcom: Don'\''t allow protected pins to be requested' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).