From: Sean Wang <sean.wang@kernel.org> To: Light Hsieh <light.hsieh@mediatek.com> Cc: Linus Walleij <linus.walleij@linaro.org>, "moderated list:ARM/Mediatek SoC support" <linux-mediatek@lists.infradead.org>, "open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>, lkml <linux-kernel@vger.kernel.org>, kuohong.wang@mediatek.com Subject: Re: [PATCH v7 1/6] Check gpio pin number and use binary search in mtk_hw_pin_field_lookup() Date: Fri, 17 Jan 2020 00:30:15 -0800 [thread overview] Message-ID: <CAGp9LzqMU8u1gi5mLMFj4=eUuRKZMJSZ16tbxo1J_07nP72Q1Q@mail.gmail.com> (raw) In-Reply-To: <1577799707-11855-1-git-send-email-light.hsieh@mediatek.com> On Tue, Dec 31, 2019 at 5:42 AM Light Hsieh <light.hsieh@mediatek.com> wrote: > > 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. Improve mtk_hw_pin_field_lookup() > 2.1 Modify mtk_hw_pin_field_lookup() to use binary search for accelerating > search. > 2.2 Correct message after the following check fail: > if (hw->soc->reg_cal && hw->soc->reg_cal[field].range) { > rc = &hw->soc->reg_cal[field]; > The original message is: > "Not support field %d for pin %d (%s)\n" > However, the check is on soc chip level, not on pin level yet. > So the message is corrected as: > "Not support field %d for this soc\n" > > Change-Id: I498a18df73e6a693e0e35635d5b92b8dc9c063ac If removing the change-ids, add a prefix "pinctrl: mediatek:" to the subject, and proper signed-off-by tags, and then the overall looks good to me. > --- > drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c | 27 ++++++++++++++++++------ > drivers/pinctrl/mediatek/pinctrl-paris.c | 25 ++++++++++++++++++++++ > 2 files changed, 46 insertions(+), 6 deletions(-) > > diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > index 20e1c89..d63e05e 100644 > --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > @@ -68,32 +68,44 @@ 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; > + int start = 0, end, check; > + bool found = false; > u32 bits; > > if (hw->soc->reg_cal && hw->soc->reg_cal[field].range) { > rc = &hw->soc->reg_cal[field]; > } else { > dev_dbg(hw->dev, > - "Not support field %d for pin %d (%s)\n", > - field, desc->number, desc->name); > + "Not support field %d for this soc\n", field); > 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 = true; > + break; > + } else if (start == end) > break; > - c++; > + 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 +194,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..3e13ae7 100644 > --- a/drivers/pinctrl/mediatek/pinctrl-paris.c > +++ b/drivers/pinctrl/mediatek/pinctrl-paris.c > @@ -81,6 +81,8 @@ static int mtk_pinconf_get(struct pinctrl_dev *pctldev, > int val, val2, err, reg, ret = 1; > const struct mtk_pin_desc *desc; > > + if (pin >= hw->soc->npins) > + return -EINVAL; > desc = (const struct mtk_pin_desc *)&hw->soc->pins[pin]; > > switch (param) { > @@ -206,6 +208,10 @@ static int mtk_pinconf_set(struct pinctrl_dev *pctldev, unsigned int pin, > int err = 0; > u32 reg; > > + if (pin >= hw->soc->npins) { > + err = -EINVAL; > + goto err; > + } > desc = (const struct mtk_pin_desc *)&hw->soc->pins[pin]; > > switch ((u32)param) { > @@ -693,6 +699,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 +717,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 +734,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 +744,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); > -- > 1.8.1.1.dirty
WARNING: multiple messages have this Message-ID (diff)
From: Sean Wang <sean.wang@kernel.org> To: Light Hsieh <light.hsieh@mediatek.com> Cc: "open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>, Linus Walleij <linus.walleij@linaro.org>, "moderated list:ARM/Mediatek SoC support" <linux-mediatek@lists.infradead.org>, lkml <linux-kernel@vger.kernel.org>, kuohong.wang@mediatek.com Subject: Re: [PATCH v7 1/6] Check gpio pin number and use binary search in mtk_hw_pin_field_lookup() Date: Fri, 17 Jan 2020 00:30:15 -0800 [thread overview] Message-ID: <CAGp9LzqMU8u1gi5mLMFj4=eUuRKZMJSZ16tbxo1J_07nP72Q1Q@mail.gmail.com> (raw) In-Reply-To: <1577799707-11855-1-git-send-email-light.hsieh@mediatek.com> On Tue, Dec 31, 2019 at 5:42 AM Light Hsieh <light.hsieh@mediatek.com> wrote: > > 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. Improve mtk_hw_pin_field_lookup() > 2.1 Modify mtk_hw_pin_field_lookup() to use binary search for accelerating > search. > 2.2 Correct message after the following check fail: > if (hw->soc->reg_cal && hw->soc->reg_cal[field].range) { > rc = &hw->soc->reg_cal[field]; > The original message is: > "Not support field %d for pin %d (%s)\n" > However, the check is on soc chip level, not on pin level yet. > So the message is corrected as: > "Not support field %d for this soc\n" > > Change-Id: I498a18df73e6a693e0e35635d5b92b8dc9c063ac If removing the change-ids, add a prefix "pinctrl: mediatek:" to the subject, and proper signed-off-by tags, and then the overall looks good to me. > --- > drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c | 27 ++++++++++++++++++------ > drivers/pinctrl/mediatek/pinctrl-paris.c | 25 ++++++++++++++++++++++ > 2 files changed, 46 insertions(+), 6 deletions(-) > > diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > index 20e1c89..d63e05e 100644 > --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > @@ -68,32 +68,44 @@ 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; > + int start = 0, end, check; > + bool found = false; > u32 bits; > > if (hw->soc->reg_cal && hw->soc->reg_cal[field].range) { > rc = &hw->soc->reg_cal[field]; > } else { > dev_dbg(hw->dev, > - "Not support field %d for pin %d (%s)\n", > - field, desc->number, desc->name); > + "Not support field %d for this soc\n", field); > 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 = true; > + break; > + } else if (start == end) > break; > - c++; > + 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 +194,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..3e13ae7 100644 > --- a/drivers/pinctrl/mediatek/pinctrl-paris.c > +++ b/drivers/pinctrl/mediatek/pinctrl-paris.c > @@ -81,6 +81,8 @@ static int mtk_pinconf_get(struct pinctrl_dev *pctldev, > int val, val2, err, reg, ret = 1; > const struct mtk_pin_desc *desc; > > + if (pin >= hw->soc->npins) > + return -EINVAL; > desc = (const struct mtk_pin_desc *)&hw->soc->pins[pin]; > > switch (param) { > @@ -206,6 +208,10 @@ static int mtk_pinconf_set(struct pinctrl_dev *pctldev, unsigned int pin, > int err = 0; > u32 reg; > > + if (pin >= hw->soc->npins) { > + err = -EINVAL; > + goto err; > + } > desc = (const struct mtk_pin_desc *)&hw->soc->pins[pin]; > > switch ((u32)param) { > @@ -693,6 +699,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 +717,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 +734,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 +744,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); > -- > 1.8.1.1.dirty _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek
next prev parent reply other threads:[~2020-01-17 8:30 UTC|newest] Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-12-31 13:41 [PATCH v7 1/6] Check gpio pin number and use binary search in mtk_hw_pin_field_lookup() Light Hsieh 2019-12-31 13:41 ` Light Hsieh 2019-12-31 13:41 ` [PATCH v7 2/6] Supporting driving setting without mapping current to register value Light Hsieh 2019-12-31 13:41 ` Light Hsieh 2020-01-04 4:48 ` Dejin Zheng 2020-01-04 4:48 ` Dejin Zheng 2020-01-17 8:35 ` Sean Wang 2020-01-17 8:35 ` Sean Wang 2019-12-31 13:41 ` [PATCH v7 3/6] Refine mtk_pinconf_get() and mtk_pinconf_set() Light Hsieh 2019-12-31 13:41 ` Light Hsieh 2019-12-31 13:41 ` [PATCH v7 4/6] Refine mtk_pinconf_get() Light Hsieh 2019-12-31 13:41 ` Light Hsieh 2019-12-31 13:41 ` [PATCH v7 5/6] Backward compatible to previous Mediatek's bias-pull usage Light Hsieh 2019-12-31 13:41 ` Light Hsieh 2020-01-07 10:40 ` Linus Walleij 2020-01-07 10:40 ` Linus Walleij 2020-01-07 12:16 ` Light Hsieh 2020-01-07 12:16 ` Light Hsieh 2020-01-07 13:26 ` Linus Walleij 2020-01-07 13:26 ` Linus Walleij 2020-01-17 9:18 ` Sean Wang 2020-01-17 9:18 ` Sean Wang 2019-12-31 13:41 ` [PATCH v7 6/6] Add support for pin configuration dump via debugfs Light Hsieh 2019-12-31 13:41 ` Light Hsieh 2020-01-17 9:05 ` Sean Wang 2020-01-17 9:05 ` Sean Wang 2020-01-02 9:02 ` [PATCH v7 1/6] Check gpio pin number and use binary search in mtk_hw_pin_field_lookup() Yingjoe Chen 2020-01-02 9:02 ` Yingjoe Chen 2020-01-17 8:30 ` Sean Wang [this message] 2020-01-17 8:30 ` Sean Wang
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='CAGp9LzqMU8u1gi5mLMFj4=eUuRKZMJSZ16tbxo1J_07nP72Q1Q@mail.gmail.com' \ --to=sean.wang@kernel.org \ --cc=kuohong.wang@mediatek.com \ --cc=light.hsieh@mediatek.com \ --cc=linus.walleij@linaro.org \ --cc=linux-gpio@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mediatek@lists.infradead.org \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.