| Message ID | 20230109172812.28488-1-ansuelsmth@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Ansuel Smith |
| Headers | show |
| Series | [iwinfo] devices: add support for declaring compatible matched devices | expand |
On Mon, 9 Jan 2023 at 18:29, Christian Marangi <ansuelsmth@gmail.com> wrote: > > From: Jo-Philipp Wich <jo@mein.io> > > Some device have embedded wifi card that are not connected with usb or > internall with pci. Such device have fake device_id and only the > vendor_id actually reflect something real but internally they don't have > any id and are just matched by the node compatible binding in DT. > > We currently match this with a big if-else to match the single devices > but this can be improved and be matched directly in devices.txt > > Rework this so that we can drop the big if-else and move the matching > from devices.txt > > When a device is matched using compatible in iwinfo the hardware will be > flagged as embedded and won't print empty ids. > > Tested-by: Christian Marangi <ansuelsmth@gmail.com> > Co-developed-by: Christian Marangi <ansuelsmth@gmail.com> > Signed-off-by: Jo-Philipp Wich <jo@mein.io> > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> Tested-by: Robert Marko <robimarko@gmail.com> Regards, Robert > --- > devices.txt | 13 +++++++++ > include/iwinfo.h | 2 ++ > iwinfo_cli.c | 9 ++++-- > iwinfo_nl80211.c | 71 ++++++------------------------------------------ > iwinfo_utils.c | 8 +++++- > 5 files changed, 36 insertions(+), 67 deletions(-) > > diff --git a/devices.txt b/devices.txt > index d76bbca..93938b5 100644 > --- a/devices.txt > +++ b/devices.txt > @@ -206,3 +206,16 @@ > # USB devices > # 0x0000 | 0x0000 | vendor id | product id | ... > 0x0000 0x0000 0x0e8d 0x7961 0 0 "MediaTek" "MT7921AU" > + > +# FDT compatible strings > +# "compatible" | txpower offset | frequency offset | ... > +"qca,ar9130-wmac" 0 0 "Atheros" "AR9130" > +"qca,ar9330-wmac" 0 0 "Atheros" "AR9330" > +"qca,ar9340-wmac" 0 0 "Atheros" "AR9340" > +"qca,qca9530-wmac" 0 0 "Qualcomm Atheros" "QCA9530" > +"qca,qca9550-wmac" 0 0 "Qualcomm Atheros" "QCA9550" > +"qca,qca9560-wmac" 0 0 "Qualcomm Atheros" "QCA9560" > +"qcom,ipq4019-wifi" 0 0 "Qualcomm Atheros" "IPQ4019" > +"qcom,ipq8074-wifi" 0 0 "Qualcomm Atheros" "IPQ8074" > +"mediatek,mt7622-wmac" 0 0 "MediaTek" "MT7622" > +"mediatek,mt7986-wmac" 0 0 "MediaTek" "MT7986" > diff --git a/include/iwinfo.h b/include/iwinfo.h > index e87ad18..4b63f1e 100644 > --- a/include/iwinfo.h > +++ b/include/iwinfo.h > @@ -243,6 +243,7 @@ struct iwinfo_hardware_id { > uint16_t device_id; > uint16_t subsystem_vendor_id; > uint16_t subsystem_device_id; > + char compatible[128]; > }; > > struct iwinfo_hardware_entry { > @@ -254,6 +255,7 @@ struct iwinfo_hardware_entry { > uint16_t subsystem_device_id; > int16_t txpower_offset; > int16_t frequency_offset; > + char compatible[128]; > }; > > extern const struct iwinfo_iso3166_label IWINFO_ISO3166_NAMES[]; > diff --git a/iwinfo_cli.c b/iwinfo_cli.c > index d70f7fb..9b3e8e3 100644 > --- a/iwinfo_cli.c > +++ b/iwinfo_cli.c > @@ -335,9 +335,12 @@ static char * print_hardware_id(const struct iwinfo_ops *iw, const char *ifname) > > if (!iw->hardware_id(ifname, (char *)&ids)) > { > - snprintf(buf, sizeof(buf), "%04X:%04X %04X:%04X", > - ids.vendor_id, ids.device_id, > - ids.subsystem_vendor_id, ids.subsystem_device_id); > + if (strlen(ids.compatible) > 0) > + snprintf(buf, sizeof(buf), "embedded"); > + else > + snprintf(buf, sizeof(buf), "%04X:%04X %04X:%04X", > + ids.vendor_id, ids.device_id, > + ids.subsystem_vendor_id, ids.subsystem_device_id); > } > else > { > diff --git a/iwinfo_nl80211.c b/iwinfo_nl80211.c > index 916192f..a9e2adf 100644 > --- a/iwinfo_nl80211.c > +++ b/iwinfo_nl80211.c > @@ -3445,7 +3445,7 @@ static int nl80211_get_mbssid_support(const char *ifname, int *buf) > > static int nl80211_hardware_id_from_fdt(struct iwinfo_hardware_id *id, const char *ifname) > { > - char *phy, compat[64], path[PATH_MAX]; > + char *phy, path[PATH_MAX]; > > /* Try to determine the phy name from the given interface */ > phy = nl80211_ifname2phy(ifname); > @@ -3453,62 +3453,10 @@ static int nl80211_hardware_id_from_fdt(struct iwinfo_hardware_id *id, const cha > snprintf(path, sizeof(path), "/sys/class/%s/%s/device/of_node/compatible", > phy ? "ieee80211" : "net", phy ? phy : ifname); > > - if (nl80211_readstr(path, compat, sizeof(compat)) <= 0) > + if (nl80211_readstr(path, id->compatible, sizeof(id->compatible)) <= 0) > return -1; > > - if (!strcmp(compat, "qca,ar9130-wmac")) { > - id->vendor_id = 0x168c; > - id->device_id = 0x0029; > - id->subsystem_vendor_id = 0x168c; > - id->subsystem_device_id = 0x9130; > - } else if (!strcmp(compat, "qca,ar9330-wmac")) { > - id->vendor_id = 0x168c; > - id->device_id = 0x0030; > - id->subsystem_vendor_id = 0x168c; > - id->subsystem_device_id = 0x9330; > - } else if (!strcmp(compat, "qca,ar9340-wmac")) { > - id->vendor_id = 0x168c; > - id->device_id = 0x0030; > - id->subsystem_vendor_id = 0x168c; > - id->subsystem_device_id = 0x9340; > - } else if (!strcmp(compat, "qca,qca9530-wmac")) { > - id->vendor_id = 0x168c; > - id->device_id = 0x0033; > - id->subsystem_vendor_id = 0x168c; > - id->subsystem_device_id = 0x9530; > - } else if (!strcmp(compat, "qca,qca9550-wmac")) { > - id->vendor_id = 0x168c; > - id->device_id = 0x0033; > - id->subsystem_vendor_id = 0x168c; > - id->subsystem_device_id = 0x9550; > - } else if (!strcmp(compat, "qca,qca9560-wmac")) { > - id->vendor_id = 0x168c; > - id->device_id = 0x0033; > - id->subsystem_vendor_id = 0x168c; > - id->subsystem_device_id = 0x9560; > - } else if (!strcmp(compat, "qcom,ipq4019-wifi")) { > - id->vendor_id = 0x168c; > - id->device_id = 0x003c; > - id->subsystem_vendor_id = 0x168c; > - id->subsystem_device_id = 0x4019; > - } else if (!strcmp(compat, "qcom,ipq8074-wifi")) { > - id->vendor_id = 0x168c; > - id->device_id = 0x8074; > - id->subsystem_vendor_id = 0x168c; > - id->subsystem_device_id = 0x8074; > - } else if (!strcmp(compat, "mediatek,mt7622-wmac")) { > - id->vendor_id = 0x14c3; > - id->device_id = 0x7622; > - id->subsystem_vendor_id = 0x14c3; > - id->subsystem_device_id = 0x7622; > - } else if (!strcmp(compat, "mediatek,mt7986-wmac")) { > - id->vendor_id = 0x14c3; > - id->device_id = 0x7986; > - id->subsystem_vendor_id = 0x14c3; > - id->subsystem_device_id = 0x7986; > - } > - > - return (id->vendor_id && id->device_id) ? 0 : -1; > + return 0; > } > > > @@ -3542,16 +3490,13 @@ static int nl80211_get_hardware_id(const char *ifname, char *buf) > *lookup[i].dest = strtoul(num, NULL, 16); > } > > - /* Failed to obtain hardware IDs, try FDT */ > - if (id->vendor_id == 0 && id->device_id == 0 && > - id->subsystem_vendor_id == 0 && id->subsystem_device_id == 0) > - if (!nl80211_hardware_id_from_fdt(id, ifname)) > - return 0; > - > - /* Failed to obtain hardware IDs, search board config */ > + /* Failed to obtain hardware PCI/USB IDs... */ > if (id->vendor_id == 0 && id->device_id == 0 && > id->subsystem_vendor_id == 0 && id->subsystem_device_id == 0) > - return iwinfo_hardware_id_from_mtd(id); > + /* ... first fallback to FDT ... */ > + if (nl80211_hardware_id_from_fdt(id, ifname) == -1) > + /* ... then board config */ > + return iwinfo_hardware_id_from_mtd(id); > > return 0; > } > diff --git a/iwinfo_utils.c b/iwinfo_utils.c > index a342b6a..ecd1dff 100644 > --- a/iwinfo_utils.c > +++ b/iwinfo_utils.c > @@ -286,7 +286,10 @@ struct iwinfo_hardware_entry * iwinfo_hardware(struct iwinfo_hardware_id *id) > &e.vendor_id, &e.device_id, > &e.subsystem_vendor_id, &e.subsystem_device_id, > &e.txpower_offset, &e.frequency_offset, > - e.vendor_name, e.device_name) < 8) > + e.vendor_name, e.device_name) != 8 && > + sscanf(buf, "\"%127[^\"]\" %hd %hd \"%63[^\"]\" \"%63[^\"]\"", > + e.compatible, &e.txpower_offset, &e.frequency_offset, > + e.vendor_name, e.device_name) != 5) > continue; > > if ((e.vendor_id != 0xffff) && (e.vendor_id != id->vendor_id)) > @@ -303,6 +306,9 @@ struct iwinfo_hardware_entry * iwinfo_hardware(struct iwinfo_hardware_id *id) > (e.subsystem_device_id != id->subsystem_device_id)) > continue; > > + if (strcmp(e.compatible, id->compatible)) > + continue; > + > rv = &e; > break; > } > -- > 2.37.2 > > > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/mailman/listinfo/openwrt-devel
On 09/01/2023 18:28, Christian Marangi wrote: > From: Jo-Philipp Wich <jo@mein.io> > > Some device have embedded wifi card that are not connected with usb or > internall with pci. Such device have fake device_id and only the > vendor_id actually reflect something real but internally they don't have > any id and are just matched by the node compatible binding in DT. Nice cleanup! But those fake entries in devices.txt can then be removed, right? (Assuming all of those _are_ fake and not mapped to actual pci ids) Cheers, Andre
On Mon, Jan 09, 2023 at 07:44:34PM +0100, Andre Heider wrote: > On 09/01/2023 18:28, Christian Marangi wrote: > > From: Jo-Philipp Wich <jo@mein.io> > > > > Some device have embedded wifi card that are not connected with usb or > > internall with pci. Such device have fake device_id and only the > > vendor_id actually reflect something real but internally they don't have > > any id and are just matched by the node compatible binding in DT. > > Nice cleanup! But those fake entries in devices.txt can then be removed, > right? (Assuming all of those _are_ fake and not mapped to actual pci ids) > But they are dropped. Am I missing something? Everything with compatible doesn't have the id declared and internally they are all set to 0.
On 09/01/2023 19:46, Christian Marangi wrote: > On Mon, Jan 09, 2023 at 07:44:34PM +0100, Andre Heider wrote: >> On 09/01/2023 18:28, Christian Marangi wrote: >>> From: Jo-Philipp Wich <jo@mein.io> >>> >>> Some device have embedded wifi card that are not connected with usb or >>> internall with pci. Such device have fake device_id and only the >>> vendor_id actually reflect something real but internally they don't have >>> any id and are just matched by the node compatible binding in DT. >> >> Nice cleanup! But those fake entries in devices.txt can then be removed, >> right? (Assuming all of those _are_ fake and not mapped to actual pci ids) >> > > But they are dropped. Am I missing something? Everything with compatible > doesn't have the id declared and internally they are all set to 0. The code that maps them to ids is dropped, the id entries mapping those to the result values are not (which is why this is so ugly ;) For the first if() that's: diff --git a/devices.txt b/devices.txt index e0663b8..040766c 100644 --- a/devices.txt +++ b/devices.txt @@ -141,7 +141,6 @@ 0x168c 0x002a 0x0777 0xe202 12 0 "Ubiquiti" "Bullet M2" 0x168c 0x002a 0x0777 0xe805 5 0 "Ubiquiti" "Bullet M5" 0x168c 0x002a 0x0777 0xe345 0 0 "Ubiquiti" "WispStation M5" /* ToDo: confirm offset - Wrong! */ -0x168c 0x0029 0x168c 0x9130 0 0 "Atheros" "AR9130" 0x168c 0x0029 0x168c 0xa094 0 0 "Atheros" "AR9220" 0x168c 0x0029 0x168c 0xa095 0 0 "Atheros" "AR9223" 0x168c 0x002a 0x168c 0xa093 0 0 "Atheros" "AR9280"
On Mon, Jan 09, 2023 at 07:44:34PM +0100, Andre Heider wrote: > On 09/01/2023 18:28, Christian Marangi wrote: > > From: Jo-Philipp Wich <jo@mein.io> > > > > Some device have embedded wifi card that are not connected with usb or > > internall with pci. Such device have fake device_id and only the > > vendor_id actually reflect something real but internally they don't have > > any id and are just matched by the node compatible binding in DT. > > Nice cleanup! But those fake entries in devices.txt can then be removed, > right? (Assuming all of those _are_ fake and not mapped to actual pci ids) Yes, they are all fake and actual PCI hardware with these IDs doesn't exist. Hence they should be removed.
On Mon, Jan 09, 2023 at 07:53:02PM +0100, Andre Heider wrote: > On 09/01/2023 19:46, Christian Marangi wrote: > > On Mon, Jan 09, 2023 at 07:44:34PM +0100, Andre Heider wrote: > > > On 09/01/2023 18:28, Christian Marangi wrote: > > > > From: Jo-Philipp Wich <jo@mein.io> > > > > > > > > Some device have embedded wifi card that are not connected with usb or > > > > internall with pci. Such device have fake device_id and only the > > > > vendor_id actually reflect something real but internally they don't have > > > > any id and are just matched by the node compatible binding in DT. > > > > > > Nice cleanup! But those fake entries in devices.txt can then be removed, > > > right? (Assuming all of those _are_ fake and not mapped to actual pci ids) > > > > > > > But they are dropped. Am I missing something? Everything with compatible > > doesn't have the id declared and internally they are all set to 0. > > The code that maps them to ids is dropped, the id entries mapping those to > the result values are not (which is why this is so ugly ;) > > For the first if() that's: > > diff --git a/devices.txt b/devices.txt > index e0663b8..040766c 100644 > --- a/devices.txt > +++ b/devices.txt > @@ -141,7 +141,6 @@ > 0x168c 0x002a 0x0777 0xe202 12 0 "Ubiquiti" "Bullet M2" > 0x168c 0x002a 0x0777 0xe805 5 0 "Ubiquiti" "Bullet M5" > 0x168c 0x002a 0x0777 0xe345 0 0 "Ubiquiti" "WispStation M5" /* > ToDo: confirm offset - Wrong! */ > -0x168c 0x0029 0x168c 0x9130 0 0 "Atheros" "AR9130" > 0x168c 0x0029 0x168c 0xa094 0 0 "Atheros" "AR9220" > 0x168c 0x0029 0x168c 0xa095 0 0 "Atheros" "AR9223" > 0x168c 0x002a 0x168c 0xa093 0 0 "Atheros" "AR9280" Still I'm not following... the only place we did compatible matching was in nl80211_hardware_id_from_fdt. Stuff that was present in devices.txt is all matched from pci id. Unless they are faked by some driver iwinfo in theory is using provided pci id to do the match. If that is not the case then I have no idea how iwinfo is taking the id if the connected pci card is not providing them. Happy to fix if you can give me better example but I wasn't aware we had pci device with fake id. Am I missing a patch on a different place?
On 09/01/2023 19:58, Christian Marangi wrote: > On Mon, Jan 09, 2023 at 07:53:02PM +0100, Andre Heider wrote: >> On 09/01/2023 19:46, Christian Marangi wrote: >>> On Mon, Jan 09, 2023 at 07:44:34PM +0100, Andre Heider wrote: >>>> On 09/01/2023 18:28, Christian Marangi wrote: >>>>> From: Jo-Philipp Wich <jo@mein.io> >>>>> >>>>> Some device have embedded wifi card that are not connected with usb or >>>>> internall with pci. Such device have fake device_id and only the >>>>> vendor_id actually reflect something real but internally they don't have >>>>> any id and are just matched by the node compatible binding in DT. >>>> >>>> Nice cleanup! But those fake entries in devices.txt can then be removed, >>>> right? (Assuming all of those _are_ fake and not mapped to actual pci ids) >>>> >>> >>> But they are dropped. Am I missing something? Everything with compatible >>> doesn't have the id declared and internally they are all set to 0. >> >> The code that maps them to ids is dropped, the id entries mapping those to >> the result values are not (which is why this is so ugly ;) >> >> For the first if() that's: >> >> diff --git a/devices.txt b/devices.txt >> index e0663b8..040766c 100644 >> --- a/devices.txt >> +++ b/devices.txt >> @@ -141,7 +141,6 @@ >> 0x168c 0x002a 0x0777 0xe202 12 0 "Ubiquiti" "Bullet M2" >> 0x168c 0x002a 0x0777 0xe805 5 0 "Ubiquiti" "Bullet M5" >> 0x168c 0x002a 0x0777 0xe345 0 0 "Ubiquiti" "WispStation M5" /* >> ToDo: confirm offset - Wrong! */ >> -0x168c 0x0029 0x168c 0x9130 0 0 "Atheros" "AR9130" >> 0x168c 0x0029 0x168c 0xa094 0 0 "Atheros" "AR9220" >> 0x168c 0x0029 0x168c 0xa095 0 0 "Atheros" "AR9223" >> 0x168c 0x002a 0x168c 0xa093 0 0 "Atheros" "AR9280" > > Still I'm not following... the only place we did compatible matching was > in nl80211_hardware_id_from_fdt. Stuff that was present in devices.txt > is all matched from pci id. Unless they are faked by some driver iwinfo > in theory is using provided pci id to do the match. > > If that is not the case then I have no idea how iwinfo is taking the id if > the connected pci card is not providing them. > > Happy to fix if you can give me better example but I wasn't aware we had > pci device with fake id. Am I missing a patch on a different place? Drivers are not faking anything, iwinfo is. Once upon a time it only supported pci based cards, using the four id values. Later support for non-pci dt-compatible was hacked in. For that, it maps those compatible strings to made up pci ids in code, and at the same time added those fake ids to devices.txt. Like the recent commit here: https://git.openwrt.org/?p=project/iwinfo.git;a=commitdiff;h=5914d7113ecf77de63eb21fc233684d1a1a52ca5 This patch cleans that up, but it left out the device entries, which are dead now. Hope that clears it up?
On Mon, Jan 09, 2023 at 08:14:36PM +0100, Andre Heider wrote: > On 09/01/2023 19:58, Christian Marangi wrote: > > On Mon, Jan 09, 2023 at 07:53:02PM +0100, Andre Heider wrote: > > > On 09/01/2023 19:46, Christian Marangi wrote: > > > > On Mon, Jan 09, 2023 at 07:44:34PM +0100, Andre Heider wrote: > > > > > On 09/01/2023 18:28, Christian Marangi wrote: > > > > > > From: Jo-Philipp Wich <jo@mein.io> > > > > > > > > > > > > Some device have embedded wifi card that are not connected with usb or > > > > > > internall with pci. Such device have fake device_id and only the > > > > > > vendor_id actually reflect something real but internally they don't have > > > > > > any id and are just matched by the node compatible binding in DT. > > > > > > > > > > Nice cleanup! But those fake entries in devices.txt can then be removed, > > > > > right? (Assuming all of those _are_ fake and not mapped to actual pci ids) > > > > > > > > > > > > > But they are dropped. Am I missing something? Everything with compatible > > > > doesn't have the id declared and internally they are all set to 0. > > > > > > The code that maps them to ids is dropped, the id entries mapping those to > > > the result values are not (which is why this is so ugly ;) > > > > > > For the first if() that's: > > > > > > diff --git a/devices.txt b/devices.txt > > > index e0663b8..040766c 100644 > > > --- a/devices.txt > > > +++ b/devices.txt > > > @@ -141,7 +141,6 @@ > > > 0x168c 0x002a 0x0777 0xe202 12 0 "Ubiquiti" "Bullet M2" > > > 0x168c 0x002a 0x0777 0xe805 5 0 "Ubiquiti" "Bullet M5" > > > 0x168c 0x002a 0x0777 0xe345 0 0 "Ubiquiti" "WispStation M5" /* > > > ToDo: confirm offset - Wrong! */ > > > -0x168c 0x0029 0x168c 0x9130 0 0 "Atheros" "AR9130" > > > 0x168c 0x0029 0x168c 0xa094 0 0 "Atheros" "AR9220" > > > 0x168c 0x0029 0x168c 0xa095 0 0 "Atheros" "AR9223" > > > 0x168c 0x002a 0x168c 0xa093 0 0 "Atheros" "AR9280" > > > > Still I'm not following... the only place we did compatible matching was > > in nl80211_hardware_id_from_fdt. Stuff that was present in devices.txt > > is all matched from pci id. Unless they are faked by some driver iwinfo > > in theory is using provided pci id to do the match. > > > > If that is not the case then I have no idea how iwinfo is taking the id if > > the connected pci card is not providing them. > > > > Happy to fix if you can give me better example but I wasn't aware we had > > pci device with fake id. Am I missing a patch on a different place? > > Drivers are not faking anything, iwinfo is. > > Once upon a time it only supported pci based cards, using the four id > values. Later support for non-pci dt-compatible was hacked in. For that, it > maps those compatible strings to made up pci ids in code, and at the same > time added those fake ids to devices.txt. > > Like the recent commit here: > https://git.openwrt.org/?p=project/iwinfo.git;a=commitdiff;h=5914d7113ecf77de63eb21fc233684d1a1a52ca5 > > This patch cleans that up, but it left out the device entries, which are > dead now. > > Hope that clears it up? OHHHHHHH ok yes! I was aware that iwinfo was faking ids but totally forgot the entry were still there in devices.txt Let me send v2 with the entry dropped... Wonder if you can take an extra check in case I miss some.
On 09/01/2023 20:17, Christian Marangi wrote: > On Mon, Jan 09, 2023 at 08:14:36PM +0100, Andre Heider wrote: >> On 09/01/2023 19:58, Christian Marangi wrote: >>> On Mon, Jan 09, 2023 at 07:53:02PM +0100, Andre Heider wrote: >>>> On 09/01/2023 19:46, Christian Marangi wrote: >>>>> On Mon, Jan 09, 2023 at 07:44:34PM +0100, Andre Heider wrote: >>>>>> On 09/01/2023 18:28, Christian Marangi wrote: >>>>>>> From: Jo-Philipp Wich <jo@mein.io> >>>>>>> >>>>>>> Some device have embedded wifi card that are not connected with usb or >>>>>>> internall with pci. Such device have fake device_id and only the >>>>>>> vendor_id actually reflect something real but internally they don't have >>>>>>> any id and are just matched by the node compatible binding in DT. >>>>>> >>>>>> Nice cleanup! But those fake entries in devices.txt can then be removed, >>>>>> right? (Assuming all of those _are_ fake and not mapped to actual pci ids) >>>>>> >>>>> >>>>> But they are dropped. Am I missing something? Everything with compatible >>>>> doesn't have the id declared and internally they are all set to 0. >>>> >>>> The code that maps them to ids is dropped, the id entries mapping those to >>>> the result values are not (which is why this is so ugly ;) >>>> >>>> For the first if() that's: >>>> >>>> diff --git a/devices.txt b/devices.txt >>>> index e0663b8..040766c 100644 >>>> --- a/devices.txt >>>> +++ b/devices.txt >>>> @@ -141,7 +141,6 @@ >>>> 0x168c 0x002a 0x0777 0xe202 12 0 "Ubiquiti" "Bullet M2" >>>> 0x168c 0x002a 0x0777 0xe805 5 0 "Ubiquiti" "Bullet M5" >>>> 0x168c 0x002a 0x0777 0xe345 0 0 "Ubiquiti" "WispStation M5" /* >>>> ToDo: confirm offset - Wrong! */ >>>> -0x168c 0x0029 0x168c 0x9130 0 0 "Atheros" "AR9130" >>>> 0x168c 0x0029 0x168c 0xa094 0 0 "Atheros" "AR9220" >>>> 0x168c 0x0029 0x168c 0xa095 0 0 "Atheros" "AR9223" >>>> 0x168c 0x002a 0x168c 0xa093 0 0 "Atheros" "AR9280" >>> >>> Still I'm not following... the only place we did compatible matching was >>> in nl80211_hardware_id_from_fdt. Stuff that was present in devices.txt >>> is all matched from pci id. Unless they are faked by some driver iwinfo >>> in theory is using provided pci id to do the match. >>> >>> If that is not the case then I have no idea how iwinfo is taking the id if >>> the connected pci card is not providing them. >>> >>> Happy to fix if you can give me better example but I wasn't aware we had >>> pci device with fake id. Am I missing a patch on a different place? >> >> Drivers are not faking anything, iwinfo is. >> >> Once upon a time it only supported pci based cards, using the four id >> values. Later support for non-pci dt-compatible was hacked in. For that, it >> maps those compatible strings to made up pci ids in code, and at the same >> time added those fake ids to devices.txt. >> >> Like the recent commit here: >> https://git.openwrt.org/?p=project/iwinfo.git;a=commitdiff;h=5914d7113ecf77de63eb21fc233684d1a1a52ca5 >> >> This patch cleans that up, but it left out the device entries, which are >> dead now. >> >> Hope that clears it up? > > OHHHHHHH ok yes! > I was aware that iwinfo was faking ids but totally forgot the entry were > still there in devices.txt > > Let me send v2 with the entry dropped... Wonder if you can take an extra > check in case I miss some. Sure. BTW, not directly related, but mainline linux also supports something related to map dt wifi devices to pci ids so the desired driver binds: $ git grep 'compatible = "pci' ... target/linux/ath79/dts/ar7161_aruba_ap-105.dts- ath9k0: wifi@0,11 { /* 2.4 GHz */ target/linux/ath79/dts/ar7161_aruba_ap-105.dts: compatible = "pci168c,0029"; ... iwinfo could evaluate those embedded pci ids to then properly match based on pci instead of dt-compatibles. I don't have a board using that feature though. Cheers, Andre
diff --git a/devices.txt b/devices.txt index d76bbca..93938b5 100644 --- a/devices.txt +++ b/devices.txt @@ -206,3 +206,16 @@ # USB devices # 0x0000 | 0x0000 | vendor id | product id | ... 0x0000 0x0000 0x0e8d 0x7961 0 0 "MediaTek" "MT7921AU" + +# FDT compatible strings +# "compatible" | txpower offset | frequency offset | ... +"qca,ar9130-wmac" 0 0 "Atheros" "AR9130" +"qca,ar9330-wmac" 0 0 "Atheros" "AR9330" +"qca,ar9340-wmac" 0 0 "Atheros" "AR9340" +"qca,qca9530-wmac" 0 0 "Qualcomm Atheros" "QCA9530" +"qca,qca9550-wmac" 0 0 "Qualcomm Atheros" "QCA9550" +"qca,qca9560-wmac" 0 0 "Qualcomm Atheros" "QCA9560" +"qcom,ipq4019-wifi" 0 0 "Qualcomm Atheros" "IPQ4019" +"qcom,ipq8074-wifi" 0 0 "Qualcomm Atheros" "IPQ8074" +"mediatek,mt7622-wmac" 0 0 "MediaTek" "MT7622" +"mediatek,mt7986-wmac" 0 0 "MediaTek" "MT7986" diff --git a/include/iwinfo.h b/include/iwinfo.h index e87ad18..4b63f1e 100644 --- a/include/iwinfo.h +++ b/include/iwinfo.h @@ -243,6 +243,7 @@ struct iwinfo_hardware_id { uint16_t device_id; uint16_t subsystem_vendor_id; uint16_t subsystem_device_id; + char compatible[128]; }; struct iwinfo_hardware_entry { @@ -254,6 +255,7 @@ struct iwinfo_hardware_entry { uint16_t subsystem_device_id; int16_t txpower_offset; int16_t frequency_offset; + char compatible[128]; }; extern const struct iwinfo_iso3166_label IWINFO_ISO3166_NAMES[]; diff --git a/iwinfo_cli.c b/iwinfo_cli.c index d70f7fb..9b3e8e3 100644 --- a/iwinfo_cli.c +++ b/iwinfo_cli.c @@ -335,9 +335,12 @@ static char * print_hardware_id(const struct iwinfo_ops *iw, const char *ifname) if (!iw->hardware_id(ifname, (char *)&ids)) { - snprintf(buf, sizeof(buf), "%04X:%04X %04X:%04X", - ids.vendor_id, ids.device_id, - ids.subsystem_vendor_id, ids.subsystem_device_id); + if (strlen(ids.compatible) > 0) + snprintf(buf, sizeof(buf), "embedded"); + else + snprintf(buf, sizeof(buf), "%04X:%04X %04X:%04X", + ids.vendor_id, ids.device_id, + ids.subsystem_vendor_id, ids.subsystem_device_id); } else { diff --git a/iwinfo_nl80211.c b/iwinfo_nl80211.c index 916192f..a9e2adf 100644 --- a/iwinfo_nl80211.c +++ b/iwinfo_nl80211.c @@ -3445,7 +3445,7 @@ static int nl80211_get_mbssid_support(const char *ifname, int *buf) static int nl80211_hardware_id_from_fdt(struct iwinfo_hardware_id *id, const char *ifname) { - char *phy, compat[64], path[PATH_MAX]; + char *phy, path[PATH_MAX]; /* Try to determine the phy name from the given interface */ phy = nl80211_ifname2phy(ifname); @@ -3453,62 +3453,10 @@ static int nl80211_hardware_id_from_fdt(struct iwinfo_hardware_id *id, const cha snprintf(path, sizeof(path), "/sys/class/%s/%s/device/of_node/compatible", phy ? "ieee80211" : "net", phy ? phy : ifname); - if (nl80211_readstr(path, compat, sizeof(compat)) <= 0) + if (nl80211_readstr(path, id->compatible, sizeof(id->compatible)) <= 0) return -1; - if (!strcmp(compat, "qca,ar9130-wmac")) { - id->vendor_id = 0x168c; - id->device_id = 0x0029; - id->subsystem_vendor_id = 0x168c; - id->subsystem_device_id = 0x9130; - } else if (!strcmp(compat, "qca,ar9330-wmac")) { - id->vendor_id = 0x168c; - id->device_id = 0x0030; - id->subsystem_vendor_id = 0x168c; - id->subsystem_device_id = 0x9330; - } else if (!strcmp(compat, "qca,ar9340-wmac")) { - id->vendor_id = 0x168c; - id->device_id = 0x0030; - id->subsystem_vendor_id = 0x168c; - id->subsystem_device_id = 0x9340; - } else if (!strcmp(compat, "qca,qca9530-wmac")) { - id->vendor_id = 0x168c; - id->device_id = 0x0033; - id->subsystem_vendor_id = 0x168c; - id->subsystem_device_id = 0x9530; - } else if (!strcmp(compat, "qca,qca9550-wmac")) { - id->vendor_id = 0x168c; - id->device_id = 0x0033; - id->subsystem_vendor_id = 0x168c; - id->subsystem_device_id = 0x9550; - } else if (!strcmp(compat, "qca,qca9560-wmac")) { - id->vendor_id = 0x168c; - id->device_id = 0x0033; - id->subsystem_vendor_id = 0x168c; - id->subsystem_device_id = 0x9560; - } else if (!strcmp(compat, "qcom,ipq4019-wifi")) { - id->vendor_id = 0x168c; - id->device_id = 0x003c; - id->subsystem_vendor_id = 0x168c; - id->subsystem_device_id = 0x4019; - } else if (!strcmp(compat, "qcom,ipq8074-wifi")) { - id->vendor_id = 0x168c; - id->device_id = 0x8074; - id->subsystem_vendor_id = 0x168c; - id->subsystem_device_id = 0x8074; - } else if (!strcmp(compat, "mediatek,mt7622-wmac")) { - id->vendor_id = 0x14c3; - id->device_id = 0x7622; - id->subsystem_vendor_id = 0x14c3; - id->subsystem_device_id = 0x7622; - } else if (!strcmp(compat, "mediatek,mt7986-wmac")) { - id->vendor_id = 0x14c3; - id->device_id = 0x7986; - id->subsystem_vendor_id = 0x14c3; - id->subsystem_device_id = 0x7986; - } - - return (id->vendor_id && id->device_id) ? 0 : -1; + return 0; } @@ -3542,16 +3490,13 @@ static int nl80211_get_hardware_id(const char *ifname, char *buf) *lookup[i].dest = strtoul(num, NULL, 16); } - /* Failed to obtain hardware IDs, try FDT */ - if (id->vendor_id == 0 && id->device_id == 0 && - id->subsystem_vendor_id == 0 && id->subsystem_device_id == 0) - if (!nl80211_hardware_id_from_fdt(id, ifname)) - return 0; - - /* Failed to obtain hardware IDs, search board config */ + /* Failed to obtain hardware PCI/USB IDs... */ if (id->vendor_id == 0 && id->device_id == 0 && id->subsystem_vendor_id == 0 && id->subsystem_device_id == 0) - return iwinfo_hardware_id_from_mtd(id); + /* ... first fallback to FDT ... */ + if (nl80211_hardware_id_from_fdt(id, ifname) == -1) + /* ... then board config */ + return iwinfo_hardware_id_from_mtd(id); return 0; } diff --git a/iwinfo_utils.c b/iwinfo_utils.c index a342b6a..ecd1dff 100644 --- a/iwinfo_utils.c +++ b/iwinfo_utils.c @@ -286,7 +286,10 @@ struct iwinfo_hardware_entry * iwinfo_hardware(struct iwinfo_hardware_id *id) &e.vendor_id, &e.device_id, &e.subsystem_vendor_id, &e.subsystem_device_id, &e.txpower_offset, &e.frequency_offset, - e.vendor_name, e.device_name) < 8) + e.vendor_name, e.device_name) != 8 && + sscanf(buf, "\"%127[^\"]\" %hd %hd \"%63[^\"]\" \"%63[^\"]\"", + e.compatible, &e.txpower_offset, &e.frequency_offset, + e.vendor_name, e.device_name) != 5) continue; if ((e.vendor_id != 0xffff) && (e.vendor_id != id->vendor_id)) @@ -303,6 +306,9 @@ struct iwinfo_hardware_entry * iwinfo_hardware(struct iwinfo_hardware_id *id) (e.subsystem_device_id != id->subsystem_device_id)) continue; + if (strcmp(e.compatible, id->compatible)) + continue; + rv = &e; break; }