| Message ID | 1567662796-25508-1-git-send-email-light.hsieh@mediatek.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/5] pinctrl: mediatek: Check gpio pin number and use binary search in mtk_hw_pin_field_lookup() | expand |
Dear reviewers, v2 is the same as v1 except that commit message is corrected according to Linus' comment for v1: 1. remove Change-Id lines 2. correct sysfs as debugfs On Thu, 2019-09-05 at 13:53 +0800, Light Hsieh wrote: > From: Light Hsieh <light.hsieh@mediatek.com> > > 1. Check if gpio pin number is in valid range to prevent from get invalid > pointer 'desc' in the following code: > desc = (const struct mtk_pin_desc *)&hw->soc->pins[gpio]; > > 2. Use binary search in mtk_hw_pin_field_lookup() > Modify mtk_hw_pin_field_lookup() to use binary search for accelerating > search. > > --- > drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c | 24 +++++++++++++++++++----- > drivers/pinctrl/mediatek/pinctrl-paris.c | 19 +++++++++++++++++++ > 2 files changed, 38 insertions(+), 5 deletions(-) > > diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > index 20e1c89..4687f63 100644 > --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > @@ -68,7 +68,7 @@ static int mtk_hw_pin_field_lookup(struct mtk_pinctrl *hw, > { > const struct mtk_pin_field_calc *c, *e; > const struct mtk_pin_reg_calc *rc; > - u32 bits; > + u32 bits, start = 0, end, found = 0, check; > > if (hw->soc->reg_cal && hw->soc->reg_cal[field].range) { > rc = &hw->soc->reg_cal[field]; > @@ -79,21 +79,32 @@ static int mtk_hw_pin_field_lookup(struct mtk_pinctrl *hw, > return -ENOTSUPP; > } > > + end = rc->nranges - 1; > c = rc->range; > e = c + rc->nranges; > > - while (c < e) { > - if (desc->number >= c->s_pin && desc->number <= c->e_pin) > + while (start <= end) { > + check = (start + end) >> 1; > + if (desc->number >= rc->range[check].s_pin > + && desc->number <= rc->range[check].e_pin) { > + found = 1; > break; > - c++; > + } else if (start == end) > + break; > + else if (desc->number < rc->range[check].s_pin) > + end = check - 1; > + else > + start = check + 1; > } > > - if (c >= e) { > + if (!found) { > dev_dbg(hw->dev, "Not support field %d for pin = %d (%s)\n", > field, desc->number, desc->name); > return -ENOTSUPP; > } > > + c = rc->range + check; > + > if (c->i_base > hw->nbase - 1) { > dev_err(hw->dev, > "Invalid base for field %d for pin = %d (%s)\n", > @@ -182,6 +193,9 @@ int mtk_hw_set_value(struct mtk_pinctrl *hw, const struct mtk_pin_desc *desc, > if (err) > return err; > > + if (value < 0 || value > pf.mask) > + return -EINVAL; > + > if (!pf.next) > mtk_rmw(hw, pf.index, pf.offset, pf.mask << pf.bitpos, > (value & pf.mask) << pf.bitpos); > diff --git a/drivers/pinctrl/mediatek/pinctrl-paris.c b/drivers/pinctrl/mediatek/pinctrl-paris.c > index 923264d..28b4951 100644 > --- a/drivers/pinctrl/mediatek/pinctrl-paris.c > +++ b/drivers/pinctrl/mediatek/pinctrl-paris.c > @@ -693,6 +693,9 @@ static int mtk_gpio_get_direction(struct gpio_chip *chip, unsigned int gpio) > const struct mtk_pin_desc *desc; > int value, err; > > + if (gpio > hw->soc->npins) > + return -EINVAL; > + > desc = (const struct mtk_pin_desc *)&hw->soc->pins[gpio]; > > err = mtk_hw_get_value(hw, desc, PINCTRL_PIN_REG_DIR, &value); > @@ -708,6 +711,9 @@ static int mtk_gpio_get(struct gpio_chip *chip, unsigned int gpio) > const struct mtk_pin_desc *desc; > int value, err; > > + if (gpio > hw->soc->npins) > + return -EINVAL; > + > desc = (const struct mtk_pin_desc *)&hw->soc->pins[gpio]; > > err = mtk_hw_get_value(hw, desc, PINCTRL_PIN_REG_DI, &value); > @@ -722,6 +728,9 @@ static void mtk_gpio_set(struct gpio_chip *chip, unsigned int gpio, int value) > struct mtk_pinctrl *hw = gpiochip_get_data(chip); > const struct mtk_pin_desc *desc; > > + if (gpio > hw->soc->npins) > + return; > + > desc = (const struct mtk_pin_desc *)&hw->soc->pins[gpio]; > > mtk_hw_set_value(hw, desc, PINCTRL_PIN_REG_DO, !!value); > @@ -729,12 +738,22 @@ static void mtk_gpio_set(struct gpio_chip *chip, unsigned int gpio, int value) > > static int mtk_gpio_direction_input(struct gpio_chip *chip, unsigned int gpio) > { > + struct mtk_pinctrl *hw = gpiochip_get_data(chip); > + > + if (gpio > hw->soc->npins) > + return -EINVAL; > + > return pinctrl_gpio_direction_input(chip->base + gpio); > } > > static int mtk_gpio_direction_output(struct gpio_chip *chip, unsigned int gpio, > int value) > { > + struct mtk_pinctrl *hw = gpiochip_get_data(chip); > + > + if (gpio > hw->soc->npins) > + return -EINVAL; > + > mtk_gpio_set(chip, gpio, value); > > return pinctrl_gpio_direction_output(chip->base + gpio);
On Thu, Sep 5, 2019 at 7:00 AM Light Hsieh <light.hsieh@mediatek.com> wrote: > v2 is the same as v1 except that commit message is corrected according > to Linus' comment for v1: > > 1. remove Change-Id lines > 2. correct sysfs as debugfs Looks OK to me, but i need Sean's review on this, Sean? Yours, Linus Walleij
Hi Linus, That looks OK to me too Acked-by: Sean Wang <sean.wang@kernel.org> On Wed, Sep 11, 2019 at 2:29 AM Linus Walleij <linus.walleij@linaro.org> wrote: > > On Thu, Sep 5, 2019 at 7:00 AM Light Hsieh <light.hsieh@mediatek.com> wrote: > > > v2 is the same as v1 except that commit message is corrected according > > to Linus' comment for v1: > > > > 1. remove Change-Id lines > > 2. correct sysfs as debugfs > > Looks OK to me, but i need Sean's review on this, Sean? > > Yours, > Linus Walleij
On Thu, Sep 5, 2019 at 7:00 AM Light Hsieh <light.hsieh@mediatek.com> wrote: > v2 is the same as v1 except that commit message is corrected according > to Linus' comment for v1: > > 1. remove Change-Id lines > 2. correct sysfs as debugfs Patches applied with Sean's ACK. Yours, Linus Walleij
On Thu, Sep 12, 2019 at 3:43 PM Linus Walleij <linus.walleij@linaro.org> wrote: > On Thu, Sep 5, 2019 at 7:00 AM Light Hsieh <light.hsieh@mediatek.com> wrote: > > > v2 is the same as v1 except that commit message is corrected according > > to Linus' comment for v1: > > > > 1. remove Change-Id lines > > 2. correct sysfs as debugfs > > Patches applied with Sean's ACK. Pulled all patches out again because they don't compile. No big deal, let's just rebase and fix it up, Yours, Linus Walleij
diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c index 20e1c89..4687f63 100644 --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c @@ -68,7 +68,7 @@ static int mtk_hw_pin_field_lookup(struct mtk_pinctrl *hw, { const struct mtk_pin_field_calc *c, *e; const struct mtk_pin_reg_calc *rc; - u32 bits; + u32 bits, start = 0, end, found = 0, check; if (hw->soc->reg_cal && hw->soc->reg_cal[field].range) { rc = &hw->soc->reg_cal[field]; @@ -79,21 +79,32 @@ static int mtk_hw_pin_field_lookup(struct mtk_pinctrl *hw, return -ENOTSUPP; } + end = rc->nranges - 1; c = rc->range; e = c + rc->nranges; - while (c < e) { - if (desc->number >= c->s_pin && desc->number <= c->e_pin) + while (start <= end) { + check = (start + end) >> 1; + if (desc->number >= rc->range[check].s_pin + && desc->number <= rc->range[check].e_pin) { + found = 1; break; - c++; + } else if (start == end) + break; + else if (desc->number < rc->range[check].s_pin) + end = check - 1; + else + start = check + 1; } - if (c >= e) { + if (!found) { dev_dbg(hw->dev, "Not support field %d for pin = %d (%s)\n", field, desc->number, desc->name); return -ENOTSUPP; } + c = rc->range + check; + if (c->i_base > hw->nbase - 1) { dev_err(hw->dev, "Invalid base for field %d for pin = %d (%s)\n", @@ -182,6 +193,9 @@ int mtk_hw_set_value(struct mtk_pinctrl *hw, const struct mtk_pin_desc *desc, if (err) return err; + if (value < 0 || value > pf.mask) + return -EINVAL; + if (!pf.next) mtk_rmw(hw, pf.index, pf.offset, pf.mask << pf.bitpos, (value & pf.mask) << pf.bitpos); diff --git a/drivers/pinctrl/mediatek/pinctrl-paris.c b/drivers/pinctrl/mediatek/pinctrl-paris.c index 923264d..28b4951 100644 --- a/drivers/pinctrl/mediatek/pinctrl-paris.c +++ b/drivers/pinctrl/mediatek/pinctrl-paris.c @@ -693,6 +693,9 @@ static int mtk_gpio_get_direction(struct gpio_chip *chip, unsigned int gpio) const struct mtk_pin_desc *desc; int value, err; + if (gpio > hw->soc->npins) + return -EINVAL; + desc = (const struct mtk_pin_desc *)&hw->soc->pins[gpio]; err = mtk_hw_get_value(hw, desc, PINCTRL_PIN_REG_DIR, &value); @@ -708,6 +711,9 @@ static int mtk_gpio_get(struct gpio_chip *chip, unsigned int gpio) const struct mtk_pin_desc *desc; int value, err; + if (gpio > hw->soc->npins) + return -EINVAL; + desc = (const struct mtk_pin_desc *)&hw->soc->pins[gpio]; err = mtk_hw_get_value(hw, desc, PINCTRL_PIN_REG_DI, &value); @@ -722,6 +728,9 @@ static void mtk_gpio_set(struct gpio_chip *chip, unsigned int gpio, int value) struct mtk_pinctrl *hw = gpiochip_get_data(chip); const struct mtk_pin_desc *desc; + if (gpio > hw->soc->npins) + return; + desc = (const struct mtk_pin_desc *)&hw->soc->pins[gpio]; mtk_hw_set_value(hw, desc, PINCTRL_PIN_REG_DO, !!value); @@ -729,12 +738,22 @@ static void mtk_gpio_set(struct gpio_chip *chip, unsigned int gpio, int value) static int mtk_gpio_direction_input(struct gpio_chip *chip, unsigned int gpio) { + struct mtk_pinctrl *hw = gpiochip_get_data(chip); + + if (gpio > hw->soc->npins) + return -EINVAL; + return pinctrl_gpio_direction_input(chip->base + gpio); } static int mtk_gpio_direction_output(struct gpio_chip *chip, unsigned int gpio, int value) { + struct mtk_pinctrl *hw = gpiochip_get_data(chip); + + if (gpio > hw->soc->npins) + return -EINVAL; + mtk_gpio_set(chip, gpio, value); return pinctrl_gpio_direction_output(chip->base + gpio);
From: Light Hsieh <light.hsieh@mediatek.com> 1. Check if gpio pin number is in valid range to prevent from get invalid pointer 'desc' in the following code: desc = (const struct mtk_pin_desc *)&hw->soc->pins[gpio]; 2. Use binary search in mtk_hw_pin_field_lookup() Modify mtk_hw_pin_field_lookup() to use binary search for accelerating search. --- drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c | 24 +++++++++++++++++++----- drivers/pinctrl/mediatek/pinctrl-paris.c | 19 +++++++++++++++++++ 2 files changed, 38 insertions(+), 5 deletions(-)