Reduce the scope of the iterator variables by defining them inside
the respective for-loops. This makes code more robust against reuse
of the same variable in the future, which might lead to some mistakes.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/pinctrl/intel/pinctrl-intel.c | 44 ++++++++++++---------------
1 file changed, 19 insertions(+), 25 deletions(-)
diff --git a/drivers/pinctrl/intel/pinctrl-intel.c b/drivers/pinctrl/intel/pinctrl-intel.c
index 7311b787dfc6..c506f9f343c3 100644
--- a/drivers/pinctrl/intel/pinctrl-intel.c
+++ b/drivers/pinctrl/intel/pinctrl-intel.c
@@ -431,7 +431,6 @@ static int intel_pinmux_set_mux(struct pinctrl_dev *pctldev,
{
struct intel_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
const struct intel_pingroup *grp = &pctrl->soc->groups[group];
- int i;
guard(raw_spinlock_irqsave)(&pctrl->lock);
@@ -439,13 +438,13 @@ static int intel_pinmux_set_mux(struct pinctrl_dev *pctldev,
* All pins in the groups needs to be accessible and writable
* before we can enable the mux for this group.
*/
- for (i = 0; i < grp->grp.npins; i++) {
+ for (unsigned int i = 0; i < grp->grp.npins; i++) {
if (!intel_pad_usable(pctrl, grp->grp.pins[i]))
return -EBUSY;
}
/* Now enable the mux setting for each pin in the group */
- for (i = 0; i < grp->grp.npins; i++) {
+ for (unsigned int i = 0; i < grp->grp.npins; i++) {
void __iomem *padcfg0;
u32 value, pmode;
@@ -909,12 +908,12 @@ static int intel_config_set(struct pinctrl_dev *pctldev, unsigned int pin,
unsigned long *configs, unsigned int nconfigs)
{
struct intel_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
- int i, ret;
+ int ret;
if (!intel_pad_usable(pctrl, pin))
return -ENOTSUPP;
- for (i = 0; i < nconfigs; i++) {
+ for (unsigned int i = 0; i < nconfigs; i++) {
switch (pinconf_to_config_param(configs[i])) {
case PIN_CONFIG_BIAS_DISABLE:
case PIN_CONFIG_BIAS_PULL_UP:
@@ -1323,9 +1322,8 @@ static void intel_gpio_irq_init(struct intel_pinctrl *pctrl)
for_each_intel_pin_community(pctrl, community) {
void __iomem *reg, *is;
- unsigned int gpp;
- for (gpp = 0; gpp < community->ngpps; gpp++) {
+ for (unsigned int gpp = 0; gpp < community->ngpps; gpp++) {
reg = community->regs + community->ie_offset + gpp * 4;
is = community->regs + community->is_offset + gpp * 4;
@@ -1436,14 +1434,14 @@ static int intel_pinctrl_add_padgroups_by_gpps(struct intel_pinctrl *pctrl,
struct intel_community *community)
{
struct intel_padgroup *gpps;
+ unsigned int ngpps = community->ngpps;
unsigned int padown_num = 0;
- size_t i, ngpps = community->ngpps;
gpps = devm_kcalloc(pctrl->dev, ngpps, sizeof(*gpps), GFP_KERNEL);
if (!gpps)
return -ENOMEM;
- for (i = 0; i < ngpps; i++) {
+ for (unsigned int i = 0; i < ngpps; i++) {
gpps[i] = community->gpps[i];
if (gpps[i].size > INTEL_PINCTRL_MAX_GPP_SIZE)
@@ -1476,18 +1474,18 @@ static int intel_pinctrl_add_padgroups_by_size(struct intel_pinctrl *pctrl,
struct intel_community *community)
{
struct intel_padgroup *gpps;
- unsigned int npins = community->npins;
+ unsigned int npins = community->npins, ngpps;
unsigned int padown_num = 0;
- size_t i, ngpps = DIV_ROUND_UP(npins, community->gpp_size);
if (community->gpp_size > INTEL_PINCTRL_MAX_GPP_SIZE)
return -EINVAL;
+ ngpps = DIV_ROUND_UP(npins, community->gpp_size);
gpps = devm_kcalloc(pctrl->dev, ngpps, sizeof(*gpps), GFP_KERNEL);
if (!gpps)
return -ENOMEM;
- for (i = 0; i < ngpps; i++) {
+ for (unsigned int i = 0; i < ngpps; i++) {
unsigned int gpp_size = community->gpp_size;
gpps[i].reg_num = i;
@@ -1513,7 +1511,6 @@ static int intel_pinctrl_pm_init(struct intel_pinctrl *pctrl)
const struct intel_pinctrl_soc_data *soc = pctrl->soc;
struct intel_community_context *communities;
struct intel_pad_context *pads;
- int i;
pads = devm_kcalloc(pctrl->dev, soc->npins, sizeof(*pads), GFP_KERNEL);
if (!pads)
@@ -1525,7 +1522,7 @@ static int intel_pinctrl_pm_init(struct intel_pinctrl *pctrl)
return -ENOMEM;
- for (i = 0; i < pctrl->ncommunities; i++) {
+ for (unsigned int i = 0; i < pctrl->ncommunities; i++) {
struct intel_community *community = &pctrl->communities[i];
u32 *intmask, *hostown;
@@ -1578,7 +1575,7 @@ int intel_pinctrl_probe(struct platform_device *pdev,
{
struct device *dev = &pdev->dev;
struct intel_pinctrl *pctrl;
- int i, ret, irq;
+ int ret, irq;
pctrl = devm_kzalloc(dev, sizeof(*pctrl), GFP_KERNEL);
if (!pctrl)
@@ -1598,7 +1595,7 @@ int intel_pinctrl_probe(struct platform_device *pdev,
if (!pctrl->communities)
return -ENOMEM;
- for (i = 0; i < pctrl->ncommunities; i++) {
+ for (unsigned int i = 0; i < pctrl->ncommunities; i++) {
struct intel_community *community = &pctrl->communities[i];
unsigned short capability_offset[6];
void __iomem *regs;
@@ -1806,10 +1803,9 @@ static int intel_pinctrl_suspend_noirq(struct device *dev)
struct intel_pinctrl *pctrl = dev_get_drvdata(dev);
struct intel_community_context *communities;
struct intel_pad_context *pads;
- int i;
pads = pctrl->context.pads;
- for (i = 0; i < pctrl->soc->npins; i++) {
+ for (unsigned int i = 0; i < pctrl->soc->npins; i++) {
const struct pinctrl_pin_desc *desc = &pctrl->soc->pins[i];
void __iomem *padcfg;
u32 val;
@@ -1828,7 +1824,7 @@ static int intel_pinctrl_suspend_noirq(struct device *dev)
}
communities = pctrl->context.communities;
- for (i = 0; i < pctrl->ncommunities; i++) {
+ for (unsigned int i = 0; i < pctrl->ncommunities; i++) {
struct intel_community *community = &pctrl->communities[i];
void __iomem *base;
unsigned int gpp;
@@ -1915,13 +1911,12 @@ static int intel_pinctrl_resume_noirq(struct device *dev)
struct intel_pinctrl *pctrl = dev_get_drvdata(dev);
const struct intel_community_context *communities;
const struct intel_pad_context *pads;
- int i;
/* Mask all interrupts */
intel_gpio_irq_init(pctrl);
pads = pctrl->context.pads;
- for (i = 0; i < pctrl->soc->npins; i++) {
+ for (unsigned int i = 0; i < pctrl->soc->npins; i++) {
const struct pinctrl_pin_desc *desc = &pctrl->soc->pins[i];
if (!(intel_pinctrl_should_save(pctrl, desc->number) ||
@@ -1938,17 +1933,16 @@ static int intel_pinctrl_resume_noirq(struct device *dev)
}
communities = pctrl->context.communities;
- for (i = 0; i < pctrl->ncommunities; i++) {
+ for (unsigned int i = 0; i < pctrl->ncommunities; i++) {
struct intel_community *community = &pctrl->communities[i];
void __iomem *base;
- unsigned int gpp;
base = community->regs + community->ie_offset;
- for (gpp = 0; gpp < community->ngpps; gpp++)
+ for (unsigned int gpp = 0; gpp < community->ngpps; gpp++)
intel_restore_intmask(pctrl, i, base, gpp, communities[i].intmask[gpp]);
base = community->regs + community->hostown_offset;
- for (gpp = 0; gpp < community->ngpps; gpp++)
+ for (unsigned int gpp = 0; gpp < community->ngpps; gpp++)
intel_restore_hostown(pctrl, i, base, gpp, communities[i].hostown[gpp]);
}
--
2.50.1On Wed, Mar 18, 2026 at 04:10:19PM +0100, Andy Shevchenko wrote:
> Reduce the scope of the iterator variables by defining them inside
> the respective for-loops. This makes code more robust against reuse
> of the same variable in the future, which might lead to some mistakes.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> drivers/pinctrl/intel/pinctrl-intel.c | 44 ++++++++++++---------------
> 1 file changed, 19 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/pinctrl/intel/pinctrl-intel.c b/drivers/pinctrl/intel/pinctrl-intel.c
> index 7311b787dfc6..c506f9f343c3 100644
> --- a/drivers/pinctrl/intel/pinctrl-intel.c
> +++ b/drivers/pinctrl/intel/pinctrl-intel.c
> @@ -431,7 +431,6 @@ static int intel_pinmux_set_mux(struct pinctrl_dev *pctldev,
> {
> struct intel_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
> const struct intel_pingroup *grp = &pctrl->soc->groups[group];
> - int i;
If there are multiple loops, I prefer to declare the variable outside of
them.
If it is just a single loop then for (int i = 0, ..) is fine.
>
> guard(raw_spinlock_irqsave)(&pctrl->lock);
>
> @@ -439,13 +438,13 @@ static int intel_pinmux_set_mux(struct pinctrl_dev *pctldev,
> * All pins in the groups needs to be accessible and writable
> * before we can enable the mux for this group.
> */
> - for (i = 0; i < grp->grp.npins; i++) {
> + for (unsigned int i = 0; i < grp->grp.npins; i++) {
also why you use "unsigned int". int i is fine here.
> if (!intel_pad_usable(pctrl, grp->grp.pins[i]))
> return -EBUSY;
> }
On Thu, Mar 19, 2026 at 07:02:21AM +0100, Mika Westerberg wrote:
> On Wed, Mar 18, 2026 at 04:10:19PM +0100, Andy Shevchenko wrote:
> > Reduce the scope of the iterator variables by defining them inside
> > the respective for-loops. This makes code more robust against reuse
> > of the same variable in the future, which might lead to some mistakes.
...
> > - int i;
>
> If there are multiple loops, I prefer to declare the variable outside of
> them.
Why?! It's exactly where it make even more sense to hide.
> If it is just a single loop then for (int i = 0, ..) is fine.
...
> > - for (i = 0; i < grp->grp.npins; i++) {
> > + for (unsigned int i = 0; i < grp->grp.npins; i++) {
>
> also why you use "unsigned int". int i is fine here.
Because grp.npins is unsigned. This is the common sense to use the same
variable type that's used for the (upper) limit.
> > }
--
With Best Regards,
Andy Shevchenko
On Thu, Mar 19, 2026 at 08:57:58AM +0200, Andy Shevchenko wrote:
> On Thu, Mar 19, 2026 at 07:02:21AM +0100, Mika Westerberg wrote:
> > On Wed, Mar 18, 2026 at 04:10:19PM +0100, Andy Shevchenko wrote:
> > > Reduce the scope of the iterator variables by defining them inside
> > > the respective for-loops. This makes code more robust against reuse
> > > of the same variable in the future, which might lead to some mistakes.
>
> ...
>
> > > - int i;
> >
> > If there are multiple loops, I prefer to declare the variable outside of
> > them.
>
> Why?! It's exactly where it make even more sense to hide.
I disagree.
>
> > If it is just a single loop then for (int i = 0, ..) is fine.
>
> ...
>
> > > - for (i = 0; i < grp->grp.npins; i++) {
> > > + for (unsigned int i = 0; i < grp->grp.npins; i++) {
> >
> > also why you use "unsigned int". int i is fine here.
>
> Because grp.npins is unsigned. This is the common sense to use the same
> variable type that's used for the (upper) limit.
No, just use "int i" there. Compiler is fine and this is more idiomatic C
anyways.
On Thu, Mar 19, 2026 at 08:09:25AM +0100, Mika Westerberg wrote:
> On Thu, Mar 19, 2026 at 08:57:58AM +0200, Andy Shevchenko wrote:
> > On Thu, Mar 19, 2026 at 07:02:21AM +0100, Mika Westerberg wrote:
> > > On Wed, Mar 18, 2026 at 04:10:19PM +0100, Andy Shevchenko wrote:
...
> > > > - int i;
> > >
> > > If there are multiple loops, I prefer to declare the variable outside of
> > > them.
> >
> > Why?! It's exactly where it make even more sense to hide.
>
> I disagree.
Why? Can you give a constructive feedback, please?
> > > If it is just a single loop then for (int i = 0, ..) is fine.
...
> > > > - for (i = 0; i < grp->grp.npins; i++) {
> > > > + for (unsigned int i = 0; i < grp->grp.npins; i++) {
> > >
> > > also why you use "unsigned int". int i is fine here.
> >
> > Because grp.npins is unsigned. This is the common sense to use the same
> > variable type that's used for the (upper) limit.
>
> No, just use "int i" there. Compiler is fine and this is more idiomatic C
> anyways.
I can agree on this, but it (in this case theoretically) might lead to subtle
signdness issues if compiler is not capable to see the upper limit and predict
no overflow or wrap-around.
--
With Best Regards,
Andy Shevchenko
On Thu, Mar 19, 2026 at 09:20:51AM +0200, Andy Shevchenko wrote: > On Thu, Mar 19, 2026 at 08:09:25AM +0100, Mika Westerberg wrote: > > On Thu, Mar 19, 2026 at 08:57:58AM +0200, Andy Shevchenko wrote: > > > On Thu, Mar 19, 2026 at 07:02:21AM +0100, Mika Westerberg wrote: > > > > On Wed, Mar 18, 2026 at 04:10:19PM +0100, Andy Shevchenko wrote: > > ... > > > > > > - int i; > > > > > > > > If there are multiple loops, I prefer to declare the variable outside of > > > > them. > > > > > > Why?! It's exactly where it make even more sense to hide. > > > > I disagree. > > Why? Can you give a constructive feedback, please? I think I did already. If you have just a single loop it's okay. If multiple then it makes more sense to declare the variable in the outer scope. This is at least what I'm used to do and prefer here as well.
© 2016 - 2026 Red Hat, Inc.