On Wed, 2021-09-01 at 10:55 -0700, Ben Greear wrote:
> On 9/1/21 10:49 AM, Ryder Lee wrote:
> > From: Ben Greear <greearb@xxxxxxxxxxxxxxx>
> >
> > Without this change, garbage is seen in the hwmon name and sensors
> > output
> > for mt7915 is garbled. It appears that the hwmon logic does not
> > make a
> > copy of the incoming string, but instead just copies a char* and
> > expects
> > it to never go away.
> >
> > Fixes: d6938251bb5b ("mt76: mt7915: add thermal sensor device
> > support")
> > Signed-off-by: Ben Greear <greearb@xxxxxxxxxxxxxxx>
> > Signed-off-by: Ryder Lee <ryder.lee@xxxxxxxxxxxx>
> > ---
> > v5: Use devm_kstrdup on the wiphy name as suggested.
>
> I don't care a great deal either way, but phyname can change (which
> was original
> way to reproduce this corruption bug), so with this v5 change, then
> the hwmon 'id' could confusingly
> be 'phy0' while user has renamed the phy0 to wiphy0 or whatever.
>
> It won't break my usage either way, so if you are happy with this,
> then good
> enough for me.
I thought of this ... Considering the hwmon can't never know the
phyname once it got changed. We can only specifiy a static "mt7915-xxx"
to name it, or just use an initial phyname. Anyhow it's not the real
phyname you change actually.
> But, see below, there is spurious change...
>
>
> > v4: Simplify flow.
> > v3: Add 'fixes' tag to aid backports.
> > ---
> > drivers/net/wireless/mediatek/mt76/mt7915/init.c | 8 ++++----
> > drivers/net/wireless/mediatek/mt76/mt7915/mcu.c | 2 +-
> > 2 files changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/init.c
> > b/drivers/net/wireless/mediatek/mt76/mt7915/init.c
> > index acc83e9f409b..78b9abbe63f3 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt7915/init.c
> > +++ b/drivers/net/wireless/mediatek/mt76/mt7915/init.c
> > @@ -160,9 +160,10 @@ static int mt7915_thermal_init(struct
> > mt7915_phy *phy)
> > struct wiphy *wiphy = phy->mt76->hw->wiphy;
> > struct thermal_cooling_device *cdev;
> > struct device *hwmon;
> > + const char *name;
> >
> > - cdev = thermal_cooling_device_register(wiphy_name(wiphy), phy,
> > - &mt7915_thermal_ops);
> > + name = devm_kstrdup(&wiphy->dev, wiphy_name(wiphy),
> > GFP_KERNEL);
> > + cdev = thermal_cooling_device_register(name, phy,
> > &mt7915_thermal_ops);
> > if (!IS_ERR(cdev)) {
> > if (sysfs_create_link(&wiphy->dev.kobj, &cdev-
> > >device.kobj,
> > "cooling_device") < 0)
> > @@ -174,8 +175,7 @@ static int mt7915_thermal_init(struct
> > mt7915_phy *phy)
> > if (!IS_REACHABLE(CONFIG_HWMON))
> > return 0;
> >
> > - hwmon = devm_hwmon_device_register_with_groups(&wiphy->dev,
> > - wiphy_name(wiphy
> > ), phy,
> > + hwmon = devm_hwmon_device_register_with_groups(&wiphy->dev,
> > name, phy,
> > mt7915_hwmon_gro
> > ups);
> > if (IS_ERR(hwmon))
> > return PTR_ERR(hwmon);
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c
> > b/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c
> > index 932cf5a629db..219bb353b56d 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c
> > +++ b/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c
> > @@ -1962,7 +1962,7 @@ mt7915_mcu_sta_bfer_tlv(struct mt7915_dev
> > *dev, struct sk_buff *skb,
> > else
> > return;
> >
> > - bf->bf_cap = BIT(!ebf && dev->ibf);
> > + bf->bf_cap = ebf ? ebf : dev->ibf << 1;
>
> And this should not be in this patch.
>
Oops. I messed patch up while merging to other series. Will fix.
Ryder
>
