* [PATCH] brcmfmac: add debugfs entry for reading firmware capabilities
@ 2018-05-11 10:15 Rafał Miłecki
2018-05-13 18:59 ` Arend van Spriel
2018-05-14 6:48 ` [PATCH V2] " Rafał Miłecki
0 siblings, 2 replies; 6+ messages in thread
From: Rafał Miłecki @ 2018-05-11 10:15 UTC (permalink / raw)
To: Kalle Valo
Cc: Arend van Spriel, Franky Lin, Hante Meuleman, Chi-Hsien Lin,
Wright Feng, Pieter-Paul Giesberts, linux-wireless,
brcm80211-dev-list.pdl, brcm80211-dev-list,
Rafał Miłecki
From: Rafał Miłecki <rafal@milecki.pl>
This allows reading all capabilities as reported by a firmware. They are
printed using native (raw) names, just like developers like it the most.
It's how firmware reports support for various features, e.g. supported
modes, supported standards, power saving details, max BSS-es.
Access to all that info is useful for trying new firmwares, comparing
them and debugging features AKA bugs.
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
.../wireless/broadcom/brcm80211/brcmfmac/feature.c | 36 ++++++++++++++++++++++
1 file changed, 36 insertions(+)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
index 876731c57bf5..782121cb9399 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
@@ -165,6 +165,41 @@ static void brcmf_feat_firmware_capabilities(struct brcmf_if *ifp)
}
}
+/**
+ * brcmf_cap_read() - expose firmware capabilities to debugfs.
+ *
+ * @seq: sequence for debugfs entry.
+ * @data: raw data pointer.
+ */
+static int brcmf_cap_read(struct seq_file *seq, void *data)
+{
+ struct brcmf_bus *bus_if = dev_get_drvdata(seq->private);
+ struct brcmf_if *ifp = brcmf_get_ifp(bus_if->drvr, 0);
+ char caps[MAX_CAPS_BUFFER_SIZE + 1] = { };
+ char *tmp;
+ int err;
+
+ err = brcmf_fil_iovar_data_get(ifp, "cap", caps, sizeof(caps));
+ if (err) {
+ brcmf_err("could not get firmware cap (%d)\n", err);
+ return err;
+ }
+
+ /* Put every capability in a new line */
+ for (tmp = caps; *tmp; tmp++) {
+ if (*tmp == ' ')
+ *tmp = '\n';
+ }
+
+ /* Usually there is a space at the end of capabilities string */
+ seq_printf(seq, "%s", caps);
+ /* So make sure we don't print two line breaks */
+ if (tmp > caps && *(tmp - 1) != '\n')
+ seq_printf(seq, "\n");
+
+ return 0;
+}
+
void brcmf_feat_attach(struct brcmf_pub *drvr)
{
struct brcmf_if *ifp = brcmf_get_ifp(drvr, 0);
@@ -233,6 +268,7 @@ void brcmf_feat_attach(struct brcmf_pub *drvr)
void brcmf_feat_debugfs_create(struct brcmf_pub *drvr)
{
brcmf_debugfs_add_entry(drvr, "features", brcmf_feat_debugfs_read);
+ brcmf_debugfs_add_entry(drvr, "cap", brcmf_cap_read);
}
bool brcmf_feat_is_enabled(struct brcmf_if *ifp, enum brcmf_feat_id id)
--
2.13.6
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] brcmfmac: add debugfs entry for reading firmware capabilities
2018-05-11 10:15 [PATCH] brcmfmac: add debugfs entry for reading firmware capabilities Rafał Miłecki
@ 2018-05-13 18:59 ` Arend van Spriel
2018-05-14 5:11 ` Rafał Miłecki
2018-05-14 6:48 ` [PATCH V2] " Rafał Miłecki
1 sibling, 1 reply; 6+ messages in thread
From: Arend van Spriel @ 2018-05-13 18:59 UTC (permalink / raw)
To: Rafał Miłecki, Kalle Valo
Cc: Franky Lin, Hante Meuleman, Chi-Hsien Lin, Wright Feng,
Pieter-Paul Giesberts, linux-wireless, brcm80211-dev-list.pdl,
brcm80211-dev-list, Rafał Miłecki
On 5/11/2018 12:15 PM, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
>
> This allows reading all capabilities as reported by a firmware. They are
> printed using native (raw) names, just like developers like it the most.
> It's how firmware reports support for various features, e.g. supported
> modes, supported standards, power saving details, max BSS-es.
>
> Access to all that info is useful for trying new firmwares, comparing
> them and debugging features AKA bugs.
What are you implying :-p ? Some comments below.
Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
> .../wireless/broadcom/brcm80211/brcmfmac/feature.c | 36 ++++++++++++++++++++++
> 1 file changed, 36 insertions(+)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
> index 876731c57bf5..782121cb9399 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
> @@ -165,6 +165,41 @@ static void brcmf_feat_firmware_capabilities(struct brcmf_if *ifp)
> }
> }
>
> +/**
> + * brcmf_cap_read() - expose firmware capabilities to debugfs.
Please stick to naming convention brcmf_<module>_foo(), ie.
brcmf_feat_cap_read().
> + *
> + * @seq: sequence for debugfs entry.
> + * @data: raw data pointer.
> + */
> +static int brcmf_cap_read(struct seq_file *seq, void *data)
> +{
> + struct brcmf_bus *bus_if = dev_get_drvdata(seq->private);
> + struct brcmf_if *ifp = brcmf_get_ifp(bus_if->drvr, 0);
> + char caps[MAX_CAPS_BUFFER_SIZE + 1] = { };
> + char *tmp;
> + int err;
> +
> + err = brcmf_fil_iovar_data_get(ifp, "cap", caps, sizeof(caps));
> + if (err) {
> + brcmf_err("could not get firmware cap (%d)\n", err);
> + return err;
> + }
> +
> + /* Put every capability in a new line */
> + for (tmp = caps; *tmp; tmp++) {
> + if (*tmp == ' ')
> + *tmp = '\n';
> + }
> +
> + /* Usually there is a space at the end of capabilities string */
> + seq_printf(seq, "%s", caps);
> + /* So make sure we don't print two line breaks */
> + if (tmp > caps && *(tmp - 1) != '\n')
> + seq_printf(seq, "\n");
> +
> + return 0;
> +}
> +
> void brcmf_feat_attach(struct brcmf_pub *drvr)
> {
> struct brcmf_if *ifp = brcmf_get_ifp(drvr, 0);
> @@ -233,6 +268,7 @@ void brcmf_feat_attach(struct brcmf_pub *drvr)
> void brcmf_feat_debugfs_create(struct brcmf_pub *drvr)
> {
> brcmf_debugfs_add_entry(drvr, "features", brcmf_feat_debugfs_read);
> + brcmf_debugfs_add_entry(drvr, "cap", brcmf_cap_read);
Prefer to name the debugfs entry "fwcap".
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] brcmfmac: add debugfs entry for reading firmware capabilities
2018-05-13 18:59 ` Arend van Spriel
@ 2018-05-14 5:11 ` Rafał Miłecki
2018-05-14 7:24 ` Arend van Spriel
0 siblings, 1 reply; 6+ messages in thread
From: Rafał Miłecki @ 2018-05-14 5:11 UTC (permalink / raw)
To: Arend van Spriel
Cc: Rafał Miłecki, Kalle Valo, Franky Lin, Hante Meuleman,
Chi-Hsien Lin, Wright Feng, Pieter-Paul Giesberts,
linux-wireless, brcm80211-dev-list.pdl, brcm80211-dev-list
On 2018-05-13 20:59, Arend van Spriel wrote:
> On 5/11/2018 12:15 PM, Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal@milecki.pl>
>>
>> This allows reading all capabilities as reported by a firmware. They
>> are
>> printed using native (raw) names, just like developers like it the
>> most.
>> It's how firmware reports support for various features, e.g. supported
>> modes, supported standards, power saving details, max BSS-es.
>>
>> Access to all that info is useful for trying new firmwares, comparing
>> them and debugging features AKA bugs.
>
> What are you implying :-p ? Some comments below.
;) For real I was trying to figure out supported modes. It appears e.g.
many/most firmwares support up to 8 MBSS ("mbss8") while brcmfmac allows
up to 4 only.
> Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>> ---
>> .../wireless/broadcom/brcm80211/brcmfmac/feature.c | 36
>> ++++++++++++++++++++++
>> 1 file changed, 36 insertions(+)
>>
>> diff --git
>> a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
>> b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
>> index 876731c57bf5..782121cb9399 100644
>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
>> @@ -165,6 +165,41 @@ static void
>> brcmf_feat_firmware_capabilities(struct brcmf_if *ifp)
>> }
>> }
>>
>> +/**
>> + * brcmf_cap_read() - expose firmware capabilities to debugfs.
>
> Please stick to naming convention brcmf_<module>_foo(), ie.
> brcmf_feat_cap_read().
Will do, thanks! I missed that naming convention because of
brcmf_debugfs_fws_stats_read() / brcmf_debugfs_sdio_count_read() which
slightly confused me. I see your point now!
>> + *
>> + * @seq: sequence for debugfs entry.
>> + * @data: raw data pointer.
>> + */
>> +static int brcmf_cap_read(struct seq_file *seq, void *data)
>> +{
>> + struct brcmf_bus *bus_if = dev_get_drvdata(seq->private);
>> + struct brcmf_if *ifp = brcmf_get_ifp(bus_if->drvr, 0);
>> + char caps[MAX_CAPS_BUFFER_SIZE + 1] = { };
>> + char *tmp;
>> + int err;
>> +
>> + err = brcmf_fil_iovar_data_get(ifp, "cap", caps, sizeof(caps));
>> + if (err) {
>> + brcmf_err("could not get firmware cap (%d)\n", err);
>> + return err;
>> + }
>> +
>> + /* Put every capability in a new line */
>> + for (tmp = caps; *tmp; tmp++) {
>> + if (*tmp == ' ')
>> + *tmp = '\n';
>> + }
>> +
>> + /* Usually there is a space at the end of capabilities string */
>> + seq_printf(seq, "%s", caps);
>> + /* So make sure we don't print two line breaks */
>> + if (tmp > caps && *(tmp - 1) != '\n')
>> + seq_printf(seq, "\n");
>> +
>> + return 0;
>> +}
>> +
>> void brcmf_feat_attach(struct brcmf_pub *drvr)
>> {
>> struct brcmf_if *ifp = brcmf_get_ifp(drvr, 0);
>> @@ -233,6 +268,7 @@ void brcmf_feat_attach(struct brcmf_pub *drvr)
>> void brcmf_feat_debugfs_create(struct brcmf_pub *drvr)
>> {
>> brcmf_debugfs_add_entry(drvr, "features", brcmf_feat_debugfs_read);
>> + brcmf_debugfs_add_entry(drvr, "cap", brcmf_cap_read);
>
> Prefer to name the debugfs entry "fwcap".
Sure.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH V2] brcmfmac: add debugfs entry for reading firmware capabilities
2018-05-11 10:15 [PATCH] brcmfmac: add debugfs entry for reading firmware capabilities Rafał Miłecki
2018-05-13 18:59 ` Arend van Spriel
@ 2018-05-14 6:48 ` Rafał Miłecki
2018-05-23 8:01 ` [V2] " Kalle Valo
1 sibling, 1 reply; 6+ messages in thread
From: Rafał Miłecki @ 2018-05-14 6:48 UTC (permalink / raw)
To: Kalle Valo
Cc: Arend van Spriel, Franky Lin, Hante Meuleman, Chi-Hsien Lin,
Wright Feng, Pieter-Paul Giesberts, linux-wireless,
brcm80211-dev-list.pdl, brcm80211-dev-list,
Rafał Miłecki
From: Rafał Miłecki <rafal@milecki.pl>
This allows reading all capabilities as reported by a firmware. They are
printed using native (raw) names, just like developers like it the most.
It's how firmware reports support for various features, e.g. supported
modes, supported standards, power saving details, max BSS-es.
Access to all that info is useful for trying new firmwares, comparing
them and debugging features AKA bugs.
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
V2: Use "fwcap" as debugfs entry
Rename function to the brcmf_feat_fwcap_debugfs_read()
---
.../wireless/broadcom/brcm80211/brcmfmac/feature.c | 36 ++++++++++++++++++++++
1 file changed, 36 insertions(+)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
index 876731c57bf5..800a423c7bc2 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
@@ -165,6 +165,41 @@ static void brcmf_feat_firmware_capabilities(struct brcmf_if *ifp)
}
}
+/**
+ * brcmf_feat_fwcap_debugfs_read() - expose firmware capabilities to debugfs.
+ *
+ * @seq: sequence for debugfs entry.
+ * @data: raw data pointer.
+ */
+static int brcmf_feat_fwcap_debugfs_read(struct seq_file *seq, void *data)
+{
+ struct brcmf_bus *bus_if = dev_get_drvdata(seq->private);
+ struct brcmf_if *ifp = brcmf_get_ifp(bus_if->drvr, 0);
+ char caps[MAX_CAPS_BUFFER_SIZE + 1] = { };
+ char *tmp;
+ int err;
+
+ err = brcmf_fil_iovar_data_get(ifp, "cap", caps, sizeof(caps));
+ if (err) {
+ brcmf_err("could not get firmware cap (%d)\n", err);
+ return err;
+ }
+
+ /* Put every capability in a new line */
+ for (tmp = caps; *tmp; tmp++) {
+ if (*tmp == ' ')
+ *tmp = '\n';
+ }
+
+ /* Usually there is a space at the end of capabilities string */
+ seq_printf(seq, "%s", caps);
+ /* So make sure we don't print two line breaks */
+ if (tmp > caps && *(tmp - 1) != '\n')
+ seq_printf(seq, "\n");
+
+ return 0;
+}
+
void brcmf_feat_attach(struct brcmf_pub *drvr)
{
struct brcmf_if *ifp = brcmf_get_ifp(drvr, 0);
@@ -233,6 +268,7 @@ void brcmf_feat_attach(struct brcmf_pub *drvr)
void brcmf_feat_debugfs_create(struct brcmf_pub *drvr)
{
brcmf_debugfs_add_entry(drvr, "features", brcmf_feat_debugfs_read);
+ brcmf_debugfs_add_entry(drvr, "fwcap", brcmf_feat_fwcap_debugfs_read);
}
bool brcmf_feat_is_enabled(struct brcmf_if *ifp, enum brcmf_feat_id id)
--
2.13.6
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] brcmfmac: add debugfs entry for reading firmware capabilities
2018-05-14 5:11 ` Rafał Miłecki
@ 2018-05-14 7:24 ` Arend van Spriel
0 siblings, 0 replies; 6+ messages in thread
From: Arend van Spriel @ 2018-05-14 7:24 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Rafał Miłecki, Kalle Valo, Franky Lin, Hante Meuleman,
Chi-Hsien Lin, Wright Feng, Pieter-Paul Giesberts,
linux-wireless, brcm80211-dev-list.pdl, brcm80211-dev-list
On 5/14/2018 7:11 AM, Rafał Miłecki wrote:
>>> +/**
>>> + * brcmf_cap_read() - expose firmware capabilities to debugfs.
>>
>> Please stick to naming convention brcmf_<module>_foo(), ie.
>> brcmf_feat_cap_read().
>
> Will do, thanks! I missed that naming convention because of
> brcmf_debugfs_fws_stats_read() / brcmf_debugfs_sdio_count_read() which
> slightly confused me. I see your point now!
Yeah. Sorry for the confusion. There is always room to improve.
Gr. AvS
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [V2] brcmfmac: add debugfs entry for reading firmware capabilities
2018-05-14 6:48 ` [PATCH V2] " Rafał Miłecki
@ 2018-05-23 8:01 ` Kalle Valo
0 siblings, 0 replies; 6+ messages in thread
From: Kalle Valo @ 2018-05-23 8:01 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Arend van Spriel, Franky Lin, Hante Meuleman, Chi-Hsien Lin,
Wright Feng, Pieter-Paul Giesberts, linux-wireless,
brcm80211-dev-list.pdl, brcm80211-dev-list,
Rafał Miłecki
Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
>
> This allows reading all capabilities as reported by a firmware. They are
> printed using native (raw) names, just like developers like it the most.
> It's how firmware reports support for various features, e.g. supported
> modes, supported standards, power saving details, max BSS-es.
>
> Access to all that info is useful for trying new firmwares, comparing
> them and debugging features AKA bugs.
>
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
Patch applied to wireless-drivers-next.git, thanks.
88001968245c brcmfmac: add debugfs entry for reading firmware capabilities
--
https://patchwork.kernel.org/patch/10396811/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-05-23 8:01 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-11 10:15 [PATCH] brcmfmac: add debugfs entry for reading firmware capabilities Rafał Miłecki
2018-05-13 18:59 ` Arend van Spriel
2018-05-14 5:11 ` Rafał Miłecki
2018-05-14 7:24 ` Arend van Spriel
2018-05-14 6:48 ` [PATCH V2] " Rafał Miłecki
2018-05-23 8:01 ` [V2] " Kalle Valo
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.