Re: [PATCH v1 4/4] pinctrl: intel: Convert capability list to features

[Andy Shevchenko <andy.shevchenko@xxxxxxxxx>]

On Fri, Jan 8, 2021 at 9:09 AM Mika Westerberg
<mika.westerberg@xxxxxxxxxxxxxxx> wrote:
> On Thu, Jan 07, 2021 at 09:02:00PM +0200, Andy Shevchenko wrote:
> > Communities can have features provided in the capability list.
> > Traverse the list and convert to respective features.

...

> > +             /* Determine community features based on the capabilities */
> > +             offset = CAPLIST;
> > +             do {
> > +                     value = readl(regs + offset);
> > +                     switch ((value & CAPLIST_ID_MASK) >> CAPLIST_ID_SHIFT) {
> > +                     case CAPLIST_ID_GPIO_HW_INFO:
> > +                             community->features |= PINCTRL_FEATURE_GPIO_HW_INFO;
> > +                             break;
> > +                     case CAPLIST_ID_PWM:
> > +                             community->features |= PINCTRL_FEATURE_PWM;
> > +                             break;
> > +                     case CAPLIST_ID_BLINK:
> > +                             community->features |= PINCTRL_FEATURE_BLINK;
> > +                             break;
> > +                     case CAPLIST_ID_EXP:
> > +                             community->features |= PINCTRL_FEATURE_EXP;
> > +                             break;
> > +                     default:
> > +                             break;
> > +                     }
> > +                     offset = (value & CAPLIST_NEXT_MASK) >> CAPLIST_NEXT_SHIFT;
>
> I suggest adding some check, like that we visited the previous offset
> already, so that we do not loop here forever if we find wrongly
> formatted capability list.

I don't see how it could be achieved (offsets can be unordered). If
there is such an issue it will mean a silicon bug.
I never heard that we have similar checks in the PCI or xHCI code.
Maybe it's something new, do you know if it has similar code to see?

> Otherwise looks good.
>
> > +             } while (offset);



-- 
With Best Regards,
Andy Shevchenko