RE: [PATCH v2 2/6] wifi: mt76: mt7925: add EHT control support based on the CLC data

[Ping-Ke Shih <pkshih@realtek.com>]

Mingyen Hsieh (謝明諺) <Mingyen.Hsieh@mediatek.com>  wrote:
> On Tue, 2025-03-04 at 09:10 +0000, Ping-Ke Shih wrote:
> > Mingyen Hsieh <mailto:mingyen.hsieh@mediatek.com> wrote:
> > 
> > [...]
> > 
> > > 
> > > +void mt7925_regd_be_ctrl(struct mt792x_dev *dev, u8 *alpha2)
> > > +{
> > > +       struct mt792x_phy *phy = &dev->phy;
> > > +       struct mt7925_clc_rule_v2 *rule;
> > > +       struct mt7925_clc *clc;
> > > +       bool old = dev->has_eht, new = true;
> > > +       u8 *pos;
> > > +
> > > +       if (!phy->clc[MT792x_CLC_BE_CTRL])
> > > +               goto out;
> > > +
> > > +       clc = (struct mt7925_clc *)phy->clc[MT792x_CLC_BE_CTRL];
> > > +       pos = clc->data;
> > > +
> > > +       while (1) {
> > 
> > while (1) could lead infinite loop unexpectedly.
> > Adding a checking of clc->len would be safer.
> > 
> > > +               rule = (struct mt7925_clc_rule_v2 *)pos;
> > > +
> > > +               if (rule->alpha2[0] == alpha2[0] &&
> > > +                   rule->alpha2[1] == alpha2[1]) {
> > > +                       new = false;
> > > +                       break;
> > > +               }
> > > +
> > > +               /* Check the last one */
> > > +               if (rule->flag && BIT(0))
> > > +                       break;
> > > +
> 
> Hi Ping-Ke,
> 
> I designed a flag to prevent the infinite loop.

I knew. 

People believe their code is safe, but somehow `while (1)` get unexpected
result. An alternative way is to use for-loop with limit range, like

clc_end = &clc->data[clc->len];

for (pos = clc->data; pos < clc_end; pos += sizeof(*rule))

Just FYR.

> 
> > > +               pos += sizeof(*rule);
> > > +       }
> > > +
> > > +out:
> > > +       if (old == new)
> > > +               return;
> > > +
> > > +       dev->has_eht = new;
> > > +       mt7925_set_stream_he_eht_caps(phy);
> > > +}
> > > +
> > 
> > [...]
> > 
>