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

[Mingyen Hsieh (謝明諺)]

On Wed, 2025-03-05 at 01:21 +0000, Ping-Ke Shih wrote:
> 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> Mingyen Hsieh (謝明諺) <Mingyen.Hsieh at mediatek.com>  wrote:
> > On Tue, 2025-03-04 at 09:10 +0000, Ping-Ke Shih wrote:
> > > Mingyen Hsieh <mailto:mingyen.hsieh at 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.
> 

Ok. I got it. Thanks for the suggestions.
Perhaps i will modify it in the next version.

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