2021-08-21 01:14:52

by Christian Lamparter

[permalink] [raw]
Subject: [RFC PATCH v1 1/3] dt-bindings:net:wireless:qca,ath9k: add nvmem-cells for calibration data

On most embedded ath9k devices (like range extenders,
routers, accesspoints, ...) the calibration data for
the RF/PHY is simply stored in a MTD partition named
"ART", "caldata"/"calibration", etc.

Any mtd partition is automatically registered in the
nvmem subsystem. This makes is possible to fetch the
necessary calibration directly from there at the low
cost of adding nvmem cell information via the
device-tree or via similar means.

This speeds up the driver's initialization a lot,
because the driver doesn't have to wait for userspace
to provide the data via helpers.

Signed-off-by: Christian Lamparter <[email protected]>
---
.../devicetree/bindings/net/wireless/qca,ath9k.txt | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt
index aaaeeb5f935b..88b322b55dd8 100644
--- a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt
+++ b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt
@@ -34,6 +34,13 @@ Optional properties:
ath9k wireless chip (in this case the calibration /
EEPROM data will be loaded from userspace using the
kernel firmware loader).
+- nvmem-cells: A phandle pointing to a nvmem-cells node representing registers
+ that has information about the MAC-Address or
+ calibration data.
+ Please refer the for nvmem-cells bindings
+ Documentation/devicetree/bindings/nvmem/nvmem.yaml.
+- nvmem-cell-names: Should be "address" for the MAC address. "calibration"
+ for ART/caldata.

The MAC address will be determined using the optional properties defined in
net/ethernet.txt.
@@ -44,5 +51,8 @@ In this example, the node is defined as child node of the PCI controller:
compatible = "pci168c,002d";
reg = <0x7000 0 0 0 0x1000>;
qca,no-eeprom;
+
+ nvmem-cells = <&macaddr_art_c>, <&cal_art_1000>;
+ nvmem-cell-names = "mac-address", "calibration";
};
};
--
2.33.0


2021-08-21 01:16:54

by Christian Lamparter

[permalink] [raw]
Subject: [RFC PATCH v1 3/3] ath9k: owl-loader: fetch pci init values through nvmem

extends the owl loader to fetch important pci initialization
values - which are stored together with the calibration data -
through the nvmem subsystem.

This allows for much faster WIFI/ath9k initializations on devices
that do not require to perform any post-processing (like XOR'ing/
reversal or unpacking) since no userspace helper is required.

Signed-off-by: Christian Lamparter <[email protected]>
---
.../wireless/ath/ath9k/ath9k_pci_owl_loader.c | 105 +++++++++++++-----
1 file changed, 76 insertions(+), 29 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/ath9k_pci_owl_loader.c b/drivers/net/wireless/ath/ath9k/ath9k_pci_owl_loader.c
index 56d1a7764b9f..708c8969b503 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k_pci_owl_loader.c
+++ b/drivers/net/wireless/ath/ath9k/ath9k_pci_owl_loader.c
@@ -19,9 +19,14 @@
#include <linux/delay.h>
#include <linux/platform_device.h>
#include <linux/ath9k_platform.h>
+#include <linux/nvmem-consumer.h>
+#include <linux/workqueue.h>

struct owl_ctx {
+ struct pci_dev *pdev;
struct completion eeprom_load;
+ struct work_struct work;
+ struct nvmem_cell *cell;
};

#define EEPROM_FILENAME_LEN 100
@@ -42,6 +47,12 @@ static int ath9k_pci_fixup(struct pci_dev *pdev, const u16 *cal_data,
u32 bar0;
bool swap_needed = false;

+ /* also note that we are doing *u16 operations on the file */
+ if (cal_len > 4096 || cal_len < 0x200 || (cal_len & 1) == 1) {
+ dev_err(&pdev->dev, "eeprom has an invalid size.\n");
+ return -EINVAL;
+ }
+
if (*cal_data != AR5416_EEPROM_MAGIC) {
if (*cal_data != swab16(AR5416_EEPROM_MAGIC)) {
dev_err(&pdev->dev, "invalid calibration data\n");
@@ -99,38 +110,31 @@ static int ath9k_pci_fixup(struct pci_dev *pdev, const u16 *cal_data,
return 0;
}

-static void owl_fw_cb(const struct firmware *fw, void *context)
+static void owl_rescan(struct pci_dev *pdev)
{
- struct pci_dev *pdev = (struct pci_dev *)context;
- struct owl_ctx *ctx = (struct owl_ctx *)pci_get_drvdata(pdev);
- struct pci_bus *bus;
-
- complete(&ctx->eeprom_load);
-
- if (!fw) {
- dev_err(&pdev->dev, "no eeprom data received.\n");
- goto release;
- }
-
- /* also note that we are doing *u16 operations on the file */
- if (fw->size > 4096 || fw->size < 0x200 || (fw->size & 1) == 1) {
- dev_err(&pdev->dev, "eeprom file has an invalid size.\n");
- goto release;
- }
-
- if (ath9k_pci_fixup(pdev, (const u16 *)fw->data, fw->size))
- goto release;
+ struct pci_bus *bus = pdev->bus;

pci_lock_rescan_remove();
- bus = pdev->bus;
pci_stop_and_remove_bus_device(pdev);
/* the device should come back with the proper
* ProductId. But we have to initiate a rescan.
*/
pci_rescan_bus(bus);
pci_unlock_rescan_remove();
+}
+
+static void owl_fw_cb(const struct firmware *fw, void *context)
+{
+ struct owl_ctx *ctx = (struct owl_ctx *)context;
+
+ complete(&ctx->eeprom_load);

-release:
+ if (fw) {
+ ath9k_pci_fixup(ctx->pdev, (const u16 *)fw->data, fw->size);
+ owl_rescan(ctx->pdev);
+ } else {
+ dev_err(&ctx->pdev->dev, "no eeprom data received.\n");
+ }
release_firmware(fw);
}

@@ -152,6 +156,43 @@ static const char *owl_get_eeprom_name(struct pci_dev *pdev)
return eeprom_name;
}

+static void owl_nvmem_work(struct work_struct *work)
+{
+ struct owl_ctx *ctx = container_of(work, struct owl_ctx, work);
+ void *buf;
+ size_t len;
+
+ complete(&ctx->eeprom_load);
+
+ buf = nvmem_cell_read(ctx->cell, &len);
+ if (!IS_ERR(buf)) {
+ ath9k_pci_fixup(ctx->pdev, buf, len);
+ kfree(buf);
+ owl_rescan(ctx->pdev);
+ } else {
+ dev_err(&ctx->pdev->dev, "no nvmem data received.\n");
+ }
+}
+
+static int owl_nvmem_probe(struct owl_ctx *ctx)
+{
+ int err;
+
+ ctx->cell = devm_nvmem_cell_get(&ctx->pdev->dev, "calibration");
+ if (IS_ERR(ctx->cell)) {
+ err = PTR_ERR(ctx->cell);
+ if (err == -ENOENT || err == -EOPNOTSUPP)
+ return 1; /* not present, try firmware_request */
+
+ return err;
+ }
+
+ INIT_WORK(&ctx->work, owl_nvmem_work);
+ schedule_work(&ctx->work);
+
+ return 0;
+}
+
static int owl_probe(struct pci_dev *pdev,
const struct pci_device_id *id)
{
@@ -164,21 +205,27 @@ static int owl_probe(struct pci_dev *pdev,

pcim_pin_device(pdev);

- eeprom_name = owl_get_eeprom_name(pdev);
- if (!eeprom_name) {
- dev_err(&pdev->dev, "no eeprom filename found.\n");
- return -ENODEV;
- }
-
ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_KERNEL);
if (!ctx)
return -ENOMEM;

init_completion(&ctx->eeprom_load);
+ ctx->pdev = pdev;

pci_set_drvdata(pdev, ctx);
+
+ err = owl_nvmem_probe(ctx);
+ if (err <= 0)
+ return err;
+
+ eeprom_name = owl_get_eeprom_name(pdev);
+ if (!eeprom_name) {
+ dev_err(&pdev->dev, "no eeprom filename found.\n");
+ return -ENODEV;
+ }
+
err = request_firmware_nowait(THIS_MODULE, true, eeprom_name,
- &pdev->dev, GFP_KERNEL, pdev, owl_fw_cb);
+ &pdev->dev, GFP_KERNEL, ctx, owl_fw_cb);
if (err)
dev_err(&pdev->dev, "failed to request caldata (%d).\n", err);

--
2.33.0

2021-08-21 05:41:05

by Kalle Valo

[permalink] [raw]
Subject: Re: [RFC PATCH v1 1/3] dt-bindings:net:wireless:qca,ath9k: add nvmem-cells for calibration data

Christian Lamparter <[email protected]> writes:

> On most embedded ath9k devices (like range extenders,
> routers, accesspoints, ...) the calibration data for
> the RF/PHY is simply stored in a MTD partition named
> "ART", "caldata"/"calibration", etc.
>
> Any mtd partition is automatically registered in the
> nvmem subsystem. This makes is possible to fetch the
> necessary calibration directly from there at the low
> cost of adding nvmem cell information via the
> device-tree or via similar means.
>
> This speeds up the driver's initialization a lot,
> because the driver doesn't have to wait for userspace
> to provide the data via helpers.
>
> Signed-off-by: Christian Lamparter <[email protected]>

The series looks good to me. But I'm curious, why you marked this as
RFC? Is there something controversial I missed?

--
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2021-08-21 20:09:35

by Christian Lamparter

[permalink] [raw]
Subject: Re: [RFC PATCH v1 1/3] dt-bindings:net:wireless:qca,ath9k: add nvmem-cells for calibration data

On 21/08/2021 07:40, Kalle Valo wrote:
> Christian Lamparter <[email protected]> writes:
>
>> On most embedded ath9k devices (like range extenders,
>> routers, accesspoints, ...) the calibration data for
>> the RF/PHY is simply stored in a MTD partition named
>> "ART", "caldata"/"calibration", etc.
>>
>> Any mtd partition is automatically registered in the
>> nvmem subsystem. This makes is possible to fetch the
>> necessary calibration directly from there at the low
>> cost of adding nvmem cell information via the
>> device-tree or via similar means.
>>
>> This speeds up the driver's initialization a lot,
>> because the driver doesn't have to wait for userspace
>> to provide the data via helpers.
>>
>> Signed-off-by: Christian Lamparter <[email protected]>
>
> The series looks good to me. But I'm curious, why you marked this as
> RFC? Is there something controversial I missed?

yeah. Last night (it was already really late) I was tunnel-visioning
at the thought that device-tree binding update was a must there.
... And ath9k's qca,ath9k.txt is still in that .txt and not .yaml
format. So, I'm not sure if that file has to be converted first.
(I couldn't get Rob's tools to work. And without them, I've no idea
what error messages a converted .yaml of it will pop up)

However... since then, I had the change to re-read:
<https://www.kernel.org/doc/Documentation/nvmem/nvmem.txt>

And found that nvmem cells and lookups can be specified in
the old platform data way as well... So that binding patch
1/3 is optional.

So, yeah. If nobody has an comment until next week, I'll post this
series with only patches 2/3 and 3/3.

Cheers,
Christian

2021-08-22 05:12:38

by Kalle Valo

[permalink] [raw]
Subject: Re: [RFC PATCH v1 1/3] dt-bindings:net:wireless:qca,ath9k: add nvmem-cells for calibration data

Christian Lamparter <[email protected]> writes:

> On 21/08/2021 07:40, Kalle Valo wrote:
>> Christian Lamparter <[email protected]> writes:
>>
>>> On most embedded ath9k devices (like range extenders,
>>> routers, accesspoints, ...) the calibration data for
>>> the RF/PHY is simply stored in a MTD partition named
>>> "ART", "caldata"/"calibration", etc.
>>>
>>> Any mtd partition is automatically registered in the
>>> nvmem subsystem. This makes is possible to fetch the
>>> necessary calibration directly from there at the low
>>> cost of adding nvmem cell information via the
>>> device-tree or via similar means.
>>>
>>> This speeds up the driver's initialization a lot,
>>> because the driver doesn't have to wait for userspace
>>> to provide the data via helpers.
>>>
>>> Signed-off-by: Christian Lamparter <[email protected]>
>>
>> The series looks good to me. But I'm curious, why you marked this as
>> RFC? Is there something controversial I missed?
>
> yeah. Last night (it was already really late) I was tunnel-visioning
> at the thought that device-tree binding update was a must there.
> ... And ath9k's qca,ath9k.txt is still in that .txt and not .yaml
> format. So, I'm not sure if that file has to be converted first.
> (I couldn't get Rob's tools to work. And without them, I've no idea
> what error messages a converted .yaml of it will pop up)
>
> However... since then, I had the change to re-read:
> <https://www.kernel.org/doc/Documentation/nvmem/nvmem.txt>
>
> And found that nvmem cells and lookups can be specified in
> the old platform data way as well... So that binding patch
> 1/3 is optional.
>
> So, yeah. If nobody has an comment until next week, I'll post this
> series with only patches 2/3 and 3/3.

Sounds good, thanks for the background.

--
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2021-08-24 16:51:34

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [RFC PATCH v1 1/3] dt-bindings:net:wireless:qca,ath9k: add nvmem-cells for calibration data

On Sat, 21 Aug 2021 03:09:13 +0200, Christian Lamparter wrote:
> On most embedded ath9k devices (like range extenders,
> routers, accesspoints, ...) the calibration data for
> the RF/PHY is simply stored in a MTD partition named
> "ART", "caldata"/"calibration", etc.
>
> Any mtd partition is automatically registered in the
> nvmem subsystem. This makes is possible to fetch the
> necessary calibration directly from there at the low
> cost of adding nvmem cell information via the
> device-tree or via similar means.
>
> This speeds up the driver's initialization a lot,
> because the driver doesn't have to wait for userspace
> to provide the data via helpers.
>
> Signed-off-by: Christian Lamparter <[email protected]>
> ---
> .../devicetree/bindings/net/wireless/qca,ath9k.txt | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>

Acked-by: Rob Herring <[email protected]>

2021-08-24 16:51:34

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [RFC PATCH v1 1/3] dt-bindings:net:wireless:qca,ath9k: add nvmem-cells for calibration data

On Sat, Aug 21, 2021 at 10:02:57PM +0200, Christian Lamparter wrote:
> On 21/08/2021 07:40, Kalle Valo wrote:
> > Christian Lamparter <[email protected]> writes:
> >
> > > On most embedded ath9k devices (like range extenders,
> > > routers, accesspoints, ...) the calibration data for
> > > the RF/PHY is simply stored in a MTD partition named
> > > "ART", "caldata"/"calibration", etc.
> > >
> > > Any mtd partition is automatically registered in the
> > > nvmem subsystem. This makes is possible to fetch the
> > > necessary calibration directly from there at the low
> > > cost of adding nvmem cell information via the
> > > device-tree or via similar means.
> > >
> > > This speeds up the driver's initialization a lot,
> > > because the driver doesn't have to wait for userspace
> > > to provide the data via helpers.
> > >
> > > Signed-off-by: Christian Lamparter <[email protected]>
> >
> > The series looks good to me. But I'm curious, why you marked this as
> > RFC? Is there something controversial I missed?
>
> yeah. Last night (it was already really late) I was tunnel-visioning
> at the thought that device-tree binding update was a must there.
> ... And ath9k's qca,ath9k.txt is still in that .txt and not .yaml
> format. So, I'm not sure if that file has to be converted first.

That would be nice, but it still falls under my bar for must convert
first. :)

> (I couldn't get Rob's tools to work. And without them, I've no idea
> what error messages a converted .yaml of it will pop up)

What issue were you having?

Rob