| Message ID | 20221006232219.37505-3-mailingradian@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | SDM670 Pin Control Driver | expand |
On Thu, Oct 06, 2022 at 07:22:18PM -0400, Richard Acayan wrote: > It may be necessary for some devices to specify reserved gpios in the > device-specific DTS, in addition to the reserved gpios common to all > devices with a given SoC. Remove this bitmap_fill() call so that the > settings applied to the gpio valid mask by DTS are not overridden by > the driver's reserved gpios. > > Signed-off-by: Richard Acayan <mailingradian@gmail.com> > --- > drivers/pinctrl/qcom/pinctrl-msm.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c > index a2abfe987ab1..f697e9f64360 100644 > --- a/drivers/pinctrl/qcom/pinctrl-msm.c > +++ b/drivers/pinctrl/qcom/pinctrl-msm.c > @@ -687,9 +687,8 @@ static int msm_gpio_init_valid_mask(struct gpio_chip *gc, > const int *reserved = pctrl->soc->reserved_gpios; > u16 *tmp; > > - /* Driver provided reserved list overrides DT and ACPI */ > + /* Driver provided reserved list overrides other settings here */ IMHO this no longer overrides things, how about changing it to: /* Remove driver-provided reserved GPIOs from valid_mask */ The rest of the change looks good and the commit message looks good. So please consider updating the comment and feel free to add my Reviewed-by: Bjorn Andersson <andersson@kernel.org> Regards, Bjorn > if (reserved) { > - bitmap_fill(valid_mask, ngpios); > for (i = 0; reserved[i] >= 0; i++) { > if (i >= ngpios || reserved[i] >= ngpios) { > dev_err(pctrl->dev, "invalid list of reserved GPIOs\n"); > -- > 2.38.0 >
> On Thu, Oct 06, 2022 at 07:22:18PM -0400, Richard Acayan wrote: > > It may be necessary for some devices to specify reserved gpios in the > > device-specific DTS, in addition to the reserved gpios common to all > > devices with a given SoC. Remove this bitmap_fill() call so that the > > settings applied to the gpio valid mask by DTS are not overridden by > > the driver's reserved gpios. > > > > Signed-off-by: Richard Acayan <mailingradian@gmail.com> > > --- > > drivers/pinctrl/qcom/pinctrl-msm.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c > > index a2abfe987ab1..f697e9f64360 100644 > > --- a/drivers/pinctrl/qcom/pinctrl-msm.c > > +++ b/drivers/pinctrl/qcom/pinctrl-msm.c > > @@ -687,9 +687,8 @@ static int msm_gpio_init_valid_mask(struct gpio_chip *gc, > > const int *reserved = pctrl->soc->reserved_gpios; > > u16 *tmp; > > > > - /* Driver provided reserved list overrides DT and ACPI */ > > + /* Driver provided reserved list overrides other settings here */ > > IMHO this no longer overrides things, how about changing it to: Technically, it doesn't. It skips the ACPI settings entirely: if (reserved) { for (i = 0; reserved[i] >= 0; i++) { if (i >= ngpios || reserved[i] >= ngpios) { dev_err(pctrl->dev, "invalid list of reserved GPIOs\n"); return -EINVAL; } clear_bit(reserved[i], valid_mask); } return 0; } /* The number of GPIOs in the ACPI tables */ len = ret = device_property_count_u16(pctrl->dev, "gpios"); if (ret < 0) return 0; > > /* Remove driver-provided reserved GPIOs from valid_mask */ Will do. Let me know if you think the above information changes what the comment should be. > > > The rest of the change looks good and the commit message looks good. So > please consider updating the comment and feel free to add my > > Reviewed-by: Bjorn Andersson <andersson@kernel.org> > > Regards, > Bjorn > > > if (reserved) { > > - bitmap_fill(valid_mask, ngpios); > > for (i = 0; reserved[i] >= 0; i++) { > > if (i >= ngpios || reserved[i] >= ngpios) { > > dev_err(pctrl->dev, "invalid list of reserved GPIOs\n"); > > -- > > 2.38.0 > >
diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c index a2abfe987ab1..f697e9f64360 100644 --- a/drivers/pinctrl/qcom/pinctrl-msm.c +++ b/drivers/pinctrl/qcom/pinctrl-msm.c @@ -687,9 +687,8 @@ static int msm_gpio_init_valid_mask(struct gpio_chip *gc, const int *reserved = pctrl->soc->reserved_gpios; u16 *tmp; - /* Driver provided reserved list overrides DT and ACPI */ + /* Driver provided reserved list overrides other settings here */ if (reserved) { - bitmap_fill(valid_mask, ngpios); for (i = 0; reserved[i] >= 0; i++) { if (i >= ngpios || reserved[i] >= ngpios) { dev_err(pctrl->dev, "invalid list of reserved GPIOs\n");
It may be necessary for some devices to specify reserved gpios in the device-specific DTS, in addition to the reserved gpios common to all devices with a given SoC. Remove this bitmap_fill() call so that the settings applied to the gpio valid mask by DTS are not overridden by the driver's reserved gpios. Signed-off-by: Richard Acayan <mailingradian@gmail.com> --- drivers/pinctrl/qcom/pinctrl-msm.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)