Message ID | 20250304062854.829194-1-mingyen.hsieh@mediatek.com |
---|---|
State | Superseded |
Headers | show |
Series | [v2,1/6] wifi: mt76: mt7925: load the appropriate CLC data based on hardware type | expand |
Mingyen Hsieh <mingyen.hsieh@mediatek.com> wrote: [...] > + struct evt { > + u8 rsv[4]; > + > + __le16 tag; > + __le16 len; > + > + __le32 ver; > + __le32 addr; > + __le32 valid; > + __le32 size; > + __le32 magic_num; > + __le32 type; > + __le32 rsv1[4]; > + u8 data[32]; > + } __packed * res; nit: no need space between * and res, i.e. "__packed *res". > + struct sk_buff *skb; > + int ret; > + > + ret = mt76_mcu_send_and_get_msg(&dev->mt76, MCU_WM_UNI_CMD_QUERY(EFUSE_CTRL), > + &req, sizeof(req), true, &skb); > + if (ret) > + return ret; > + > + res = (struct evt *)skb->data; > + *val = res->data[offset % MT7925_EEPROM_BLOCK_SIZE]; > + > + dev_kfree_skb(skb); > + > + return 0; > +} > + > static int mt7925_load_clc(struct mt792x_dev *dev, const char *fw_name) > { > const struct mt76_connac2_fw_trailer *hdr; > @@ -809,12 +857,19 @@ static int mt7925_load_clc(struct mt792x_dev *dev, const char *fw_name) > struct mt792x_phy *phy = &dev->phy; > const struct firmware *fw; > int ret, i, len, offset = 0; > - u8 *clc_base = NULL; > + u8 *clc_base = NULL, hw_encap = 0; not sure if mt76 declare local variables in reverse X'mas tree order?
Mingyen Hsieh <mingyen.hsieh@mediatek.com> wrote: [...] > +static void > +mt7925_regd_channel_update(struct wiphy *wiphy, struct mt792x_dev *dev) > +{ > +#define IS_UNII_INVALID(idx, sfreq, efreq) \ > + (!(dev->phy.clc_chan_conf & BIT(idx)) && (cfreq) >= (sfreq) && (cfreq) <= (efreq)) Implicitly using 'cfreq' would be missing something by reviewers. How about adding it as an argument of macro? > + struct ieee80211_supported_band *sband; > + struct mt76_dev *mdev = &dev->mt76; > + struct ieee80211_channel *ch; > + int i, cfreq; > + > + sband = wiphy->bands[NL80211_BAND_5GHZ]; > + if (!sband) > + return; > + > + for (i = 0; i < sband->n_channels; i++) { > + ch = &sband->channels[i]; > + cfreq = ch->center_freq; > + > + /* UNII-4 */ > + if (IS_UNII_INVALID(0, 5845, 5925)) > + ch->flags |= IEEE80211_CHAN_DISABLED; > + } > + > + sband = wiphy->bands[NL80211_BAND_6GHZ]; > + if (!sband) > + return; > + > + for (i = 0; i < sband->n_channels; i++) { > + ch = &sband->channels[i]; > + cfreq = ch->center_freq; > + > + /* UNII-5/6/7/8 */ > + if (IS_UNII_INVALID(1, 5925, 6425) || > + IS_UNII_INVALID(2, 6425, 6525) || > + IS_UNII_INVALID(3, 6525, 6875) || > + IS_UNII_INVALID(4, 6875, 7125)) > + ch->flags |= IEEE80211_CHAN_DISABLED; > + } > +} > + [...]
On Tue, 2025-03-04 at 09:06 +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@mediatek.com> wrote: > > [...] > > > + struct evt { > > + u8 rsv[4]; > > + > > + __le16 tag; > > + __le16 len; > > + > > + __le32 ver; > > + __le32 addr; > > + __le32 valid; > > + __le32 size; > > + __le32 magic_num; > > + __le32 type; > > + __le32 rsv1[4]; > > + u8 data[32]; > > + } __packed * res; > > nit: no need space between * and res, i.e. "__packed *res". > Hi Ping-Ke, I also think this is better, but this was suggested to me by script/checkpatch.pl. I will send v4 for this. > > + struct sk_buff *skb; > > + int ret; > > + > > + ret = mt76_mcu_send_and_get_msg(&dev->mt76, > > MCU_WM_UNI_CMD_QUERY(EFUSE_CTRL), > > + &req, sizeof(req), true, > > &skb); > > + if (ret) > > + return ret; > > + > > + res = (struct evt *)skb->data; > > + *val = res->data[offset % MT7925_EEPROM_BLOCK_SIZE]; > > + > > + dev_kfree_skb(skb); > > + > > + return 0; > > +} > > + > > static int mt7925_load_clc(struct mt792x_dev *dev, const char > > *fw_name) > > { > > const struct mt76_connac2_fw_trailer *hdr; > > @@ -809,12 +857,19 @@ static int mt7925_load_clc(struct mt792x_dev > > *dev, const char *fw_name) > > struct mt792x_phy *phy = &dev->phy; > > const struct firmware *fw; > > int ret, i, len, offset = 0; > > - u8 *clc_base = NULL; > > + u8 *clc_base = NULL, hw_encap = 0; > > not sure if mt76 declare local variables in reverse X'mas tree order? > >
> > > + struct evt { > > > + u8 rsv[4]; > > > + > > > + __le16 tag; > > > + __le16 len; > > > + > > > + __le32 ver; > > > + __le32 addr; > > > + __le32 valid; > > > + __le32 size; > > > + __le32 magic_num; > > > + __le32 type; > > > + __le32 rsv1[4]; > > > + u8 data[32]; > > > + } __packed * res; > > > > nit: no need space between * and res, i.e. "__packed *res". > > > Hi Ping-Ke, > > I also think this is better, but this was suggested to me by > script/checkpatch.pl. > checkpatch says "CHECK:SPACING: spaces preferred around that '*'" for this case, because it treats as a binary operator, so this is a false alarm. By the way, "git grep "__packed \* " drivers/net/wireless/" can see many similar instances I guess they are also suggested by checkpatch.
diff --git a/drivers/net/wireless/mediatek/mt76/mt7925/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7925/mcu.c index d4fac7c2d0e6..505a6467f147 100644 --- a/drivers/net/wireless/mediatek/mt76/mt7925/mcu.c +++ b/drivers/net/wireless/mediatek/mt76/mt7925/mcu.c @@ -800,6 +800,54 @@ int mt7925_mcu_uni_rx_ba(struct mt792x_dev *dev, enable, false); } +static int mt7925_mcu_read_eeprom(struct mt792x_dev *dev, u32 offset, u8 *val) +{ + struct { + u8 rsv[4]; + + __le16 tag; + __le16 len; + + __le32 addr; + __le32 valid; + u8 data[MT7925_EEPROM_BLOCK_SIZE]; + } __packed req = { + .tag = cpu_to_le16(1), + .len = cpu_to_le16(sizeof(req) - 4), + .addr = cpu_to_le32(round_down(offset, + MT7925_EEPROM_BLOCK_SIZE)), + }; + struct evt { + u8 rsv[4]; + + __le16 tag; + __le16 len; + + __le32 ver; + __le32 addr; + __le32 valid; + __le32 size; + __le32 magic_num; + __le32 type; + __le32 rsv1[4]; + u8 data[32]; + } __packed * res; + struct sk_buff *skb; + int ret; + + ret = mt76_mcu_send_and_get_msg(&dev->mt76, MCU_WM_UNI_CMD_QUERY(EFUSE_CTRL), + &req, sizeof(req), true, &skb); + if (ret) + return ret; + + res = (struct evt *)skb->data; + *val = res->data[offset % MT7925_EEPROM_BLOCK_SIZE]; + + dev_kfree_skb(skb); + + return 0; +} + static int mt7925_load_clc(struct mt792x_dev *dev, const char *fw_name) { const struct mt76_connac2_fw_trailer *hdr; @@ -809,12 +857,19 @@ static int mt7925_load_clc(struct mt792x_dev *dev, const char *fw_name) struct mt792x_phy *phy = &dev->phy; const struct firmware *fw; int ret, i, len, offset = 0; - u8 *clc_base = NULL; + u8 *clc_base = NULL, hw_encap = 0; if (mt7925_disable_clc || mt76_is_usb(&dev->mt76)) return 0; + if (mt76_is_mmio(&dev->mt76)) { + ret = mt7925_mcu_read_eeprom(dev, MT_EE_HW_TYPE, &hw_encap); + if (ret) + return ret; + hw_encap = u8_get_bits(hw_encap, MT_EE_HW_TYPE_ENCAP); + } + ret = request_firmware(&fw, fw_name, mdev->dev); if (ret) return ret; @@ -859,6 +914,10 @@ static int mt7925_load_clc(struct mt792x_dev *dev, const char *fw_name) if (phy->clc[clc->idx]) continue; + /* header content sanity */ + if (u8_get_bits(clc->type, MT_EE_HW_TYPE_ENCAP) != hw_encap) + continue; + phy->clc[clc->idx] = devm_kmemdup(mdev->dev, clc, le32_to_cpu(clc->len), GFP_KERNEL); diff --git a/drivers/net/wireless/mediatek/mt76/mt7925/mt7925.h b/drivers/net/wireless/mediatek/mt76/mt7925/mt7925.h index 3f7187309513..abecaf897159 100644 --- a/drivers/net/wireless/mediatek/mt76/mt7925/mt7925.h +++ b/drivers/net/wireless/mediatek/mt76/mt7925/mt7925.h @@ -167,9 +167,12 @@ enum mt7925_eeprom_field { MT_EE_CHIP_ID = 0x000, MT_EE_VERSION = 0x002, MT_EE_MAC_ADDR = 0x004, + MT_EE_HW_TYPE = 0xa71, __MT_EE_MAX = 0x9ff }; +#define MT_EE_HW_TYPE_ENCAP GENMASK(1, 0) + enum { TXPWR_USER, TXPWR_EEPROM,