diff mbox

accessing ath10k calibration data

Message ID CAOiWkA8NSv1VskZi4AnTgjt6p-NsRoTMzHqyAPwCsMKU-c9ujg@mail.gmail.com (mailing list archive)
State Not Applicable
Delegated to: Kalle Valo
Headers show

Commit Message

Marty Faltesek Sept. 9, 2016, 5:42 p.m. UTC
It's blocked by the code below which I tried to ifdef out, but then it
returns all 0's.

> Hi,
>
> It's just in OTP? You should be able to read the OTP data without
> needing a STA/AP up?
>
>
> -a
>
>
> On 9 September 2016 at 10:35, Marty Faltesek <mfaltesek@google.com> wrote:
>> Is there a way to access calibration data without having to start an
>> AP (or client)?
>>
>> Thanks,
>>
>> Marty
>>
>> _______________________________________________
>> ath10k mailing list
>> ath10k@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/ath10k

Comments

Michal Kazior Sept. 12, 2016, 8:50 a.m. UTC | #1
On 9 September 2016 at 19:42, Marty Faltesek <mfaltesek@google.com> wrote:
> It's blocked by the code below which I tried to ifdef out, but then it
> returns all 0's.
>
> diff --git a/drivers/net/wireless/ath/ath10k/debug.c
> b/drivers/net/wireless/ath/ath10k/debug.c
> index 8b01e3e..bb8b7ec 100644
> --- a/drivers/net/wireless/ath/ath10k/debug.c
> +++ b/drivers/net/wireless/ath/ath10k/debug.c
> @@ -1433,12 +1433,13 @@ static int ath10k_debug_cal_data_open(struct
> inode *inode, struct file *file)
>         int ret;
>
>         mutex_lock(&ar->conf_mutex);
> -
> +#if 0
>         if (ar->state != ATH10K_STATE_ON &&
>             ar->state != ATH10K_STATE_UTF) {
>                 ret = -ENETDOWN;
>                 goto err;
>         }
> +#endif

This won't work. The driver needs to go through ath10k_start(), i.e.
firmware must be loaded. Cal data is cooked as part of that.

You could get away with just `ifconfig wlan0 up`. You don't need to
connect or anything.

I guess the driver *could* cache the end resulting cal data when the
device is probed and re-use it in subsequent firmware boot-up attempts
(instead of re-computing it) making it available when the device is
stopped as well. Any volunteers to try *and* test if it doesn't break
anything? :)


> On Fri, Sep 9, 2016 at 1:39 PM, Adrian Chadd <adrian@freebsd.org> wrote:
>> Hi,
>>
>> It's just in OTP? You should be able to read the OTP data without
>> needing a STA/AP up?

I would argue with the "just OTP" at least from the driver-device
protocol point of view.


Michał
Marty Faltesek Sept. 12, 2016, 7:47 p.m. UTC | #2
On Mon, Sep 12, 2016 at 4:50 AM, Michal Kazior <michal.kazior@tieto.com> wrote:
> On 9 September 2016 at 19:42, Marty Faltesek <mfaltesek@google.com> wrote:
>> It's blocked by the code below which I tried to ifdef out, but then it
>> returns all 0's.
>>
>> diff --git a/drivers/net/wireless/ath/ath10k/debug.c
>> b/drivers/net/wireless/ath/ath10k/debug.c
>> index 8b01e3e..bb8b7ec 100644
>> --- a/drivers/net/wireless/ath/ath10k/debug.c
>> +++ b/drivers/net/wireless/ath/ath10k/debug.c
>> @@ -1433,12 +1433,13 @@ static int ath10k_debug_cal_data_open(struct
>> inode *inode, struct file *file)
>>         int ret;
>>
>>         mutex_lock(&ar->conf_mutex);
>> -
>> +#if 0
>>         if (ar->state != ATH10K_STATE_ON &&
>>             ar->state != ATH10K_STATE_UTF) {
>>                 ret = -ENETDOWN;
>>                 goto err;
>>         }
>> +#endif
>
> This won't work. The driver needs to go through ath10k_start(), i.e.
> firmware must be loaded. Cal data is cooked as part of that.
>
> You could get away with just `ifconfig wlan0 up`. You don't need to
> connect or anything.

This does not work:

hexdump: ./kernel/debug/ieee80211/phy1/ath10k/cal_data: Network is down

But it works after starting the AP. Are you sure about what you said?


>
> I guess the driver *could* cache the end resulting cal data when the
> device is probed and re-use it in subsequent firmware boot-up attempts
> (instead of re-computing it) making it available when the device is
> stopped as well. Any volunteers to try *and* test if it doesn't break
> anything? :)

This might break what we are trying to accomplish, which is read the
calibration data
and search for mis-calibrated hardware, and if found, apply a fix.

>
>
>> On Fri, Sep 9, 2016 at 1:39 PM, Adrian Chadd <adrian@freebsd.org> wrote:
>>> Hi,
>>>
>>> It's just in OTP? You should be able to read the OTP data without
>>> needing a STA/AP up?
>
> I would argue with the "just OTP" at least from the driver-device
> protocol point of view.
>
>
> Michał
Marty Faltesek Sept. 12, 2016, 10:20 p.m. UTC | #3
On Mon, Sep 12, 2016 at 4:50 AM, Michal Kazior <michal.kazior@tieto.com> wrote:
> On 9 September 2016 at 19:42, Marty Faltesek <mfaltesek@google.com> wrote:
>> It's blocked by the code below which I tried to ifdef out, but then it
>> returns all 0's.
>>
>> diff --git a/drivers/net/wireless/ath/ath10k/debug.c
>> b/drivers/net/wireless/ath/ath10k/debug.c
>> index 8b01e3e..bb8b7ec 100644
>> --- a/drivers/net/wireless/ath/ath10k/debug.c
>> +++ b/drivers/net/wireless/ath/ath10k/debug.c
>> @@ -1433,12 +1433,13 @@ static int ath10k_debug_cal_data_open(struct
>> inode *inode, struct file *file)
>>         int ret;
>>
>>         mutex_lock(&ar->conf_mutex);
>> -
>> +#if 0
>>         if (ar->state != ATH10K_STATE_ON &&
>>             ar->state != ATH10K_STATE_UTF) {
>>                 ret = -ENETDOWN;
>>                 goto err;
>>         }
>> +#endif
>
> This won't work. The driver needs to go through ath10k_start(), i.e.
> firmware must be loaded. Cal data is cooked as part of that.
>
> You could get away with just `ifconfig wlan0 up`. You don't need to
> connect or anything.
>
> I guess the driver *could* cache the end resulting cal data when the
> device is probed and re-use it in subsequent firmware boot-up attempts
> (instead of re-computing it) making it available when the device is
> stopped as well. Any volunteers to try *and* test if it doesn't break
> anything? :)
>

I see what you mean now. This might actually work for us. I'll give it a shot
and see.

thanks,

Marty
Kalle Valo Sept. 13, 2016, 10:46 a.m. UTC | #4
Marty Faltesek <mfaltesek@google.com> writes:

> On Mon, Sep 12, 2016 at 4:50 AM, Michal Kazior <michal.kazior@tieto.com> wrote:
>> On 9 September 2016 at 19:42, Marty Faltesek <mfaltesek@google.com> wrote:
>>> It's blocked by the code below which I tried to ifdef out, but then it
>>> returns all 0's.
>>>
>>> diff --git a/drivers/net/wireless/ath/ath10k/debug.c
>>> b/drivers/net/wireless/ath/ath10k/debug.c
>>> index 8b01e3e..bb8b7ec 100644
>>> --- a/drivers/net/wireless/ath/ath10k/debug.c
>>> +++ b/drivers/net/wireless/ath/ath10k/debug.c
>>> @@ -1433,12 +1433,13 @@ static int ath10k_debug_cal_data_open(struct
>>> inode *inode, struct file *file)
>>>         int ret;
>>>
>>>         mutex_lock(&ar->conf_mutex);
>>> -
>>> +#if 0
>>>         if (ar->state != ATH10K_STATE_ON &&
>>>             ar->state != ATH10K_STATE_UTF) {
>>>                 ret = -ENETDOWN;
>>>                 goto err;
>>>         }
>>> +#endif
>>
>> This won't work. The driver needs to go through ath10k_start(), i.e.
>> firmware must be loaded. Cal data is cooked as part of that.
>>
>> You could get away with just `ifconfig wlan0 up`. You don't need to
>> connect or anything.
>
> This does not work:
>
> hexdump: ./kernel/debug/ieee80211/phy1/ath10k/cal_data: Network is down
>
> But it works after starting the AP. Are you sure about what you said?

It should work:

# ip link show wlan0
5: wlan0: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000
    link/ether 00:03:7f:48:d6:05 brd ff:ff:ff:ff:ff:ff
# ip link set wlan0 up
# hexdump /sys/kernel/debug/ieee80211/phy0/ath10k/cal_data
# ip link set wlan0 down
# hexdump /sys/kernel/debug/ieee80211/phy0/ath10k/cal_data
hexdump: /sys/kernel/debug/ieee80211/phy0/ath10k/cal_data: Network is down
# 

What's odd is that I got a file with zero bytes, but no time to
investigate it now.
Marty Faltesek Sept. 13, 2016, 8:49 p.m. UTC | #5
Hey Kalle

OK this does work for me after all. Thanks.

I wrote a patch to cache cal_data only while the core is off. I don't
need it now, but wondering if there is any benefit
to submitting it?

thanks,

Marty





On Tue, Sep 13, 2016 at 6:46 AM, Valo, Kalle <kvalo@qca.qualcomm.com> wrote:
> Marty Faltesek <mfaltesek@google.com> writes:
>
>> On Mon, Sep 12, 2016 at 4:50 AM, Michal Kazior <michal.kazior@tieto.com> wrote:
>>> On 9 September 2016 at 19:42, Marty Faltesek <mfaltesek@google.com> wrote:
>>>> It's blocked by the code below which I tried to ifdef out, but then it
>>>> returns all 0's.
>>>>
>>>> diff --git a/drivers/net/wireless/ath/ath10k/debug.c
>>>> b/drivers/net/wireless/ath/ath10k/debug.c
>>>> index 8b01e3e..bb8b7ec 100644
>>>> --- a/drivers/net/wireless/ath/ath10k/debug.c
>>>> +++ b/drivers/net/wireless/ath/ath10k/debug.c
>>>> @@ -1433,12 +1433,13 @@ static int ath10k_debug_cal_data_open(struct
>>>> inode *inode, struct file *file)
>>>>         int ret;
>>>>
>>>>         mutex_lock(&ar->conf_mutex);
>>>> -
>>>> +#if 0
>>>>         if (ar->state != ATH10K_STATE_ON &&
>>>>             ar->state != ATH10K_STATE_UTF) {
>>>>                 ret = -ENETDOWN;
>>>>                 goto err;
>>>>         }
>>>> +#endif
>>>
>>> This won't work. The driver needs to go through ath10k_start(), i.e.
>>> firmware must be loaded. Cal data is cooked as part of that.
>>>
>>> You could get away with just `ifconfig wlan0 up`. You don't need to
>>> connect or anything.
>>
>> This does not work:
>>
>> hexdump: ./kernel/debug/ieee80211/phy1/ath10k/cal_data: Network is down
>>
>> But it works after starting the AP. Are you sure about what you said?
>
> It should work:
>
> # ip link show wlan0
> 5: wlan0: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000
>     link/ether 00:03:7f:48:d6:05 brd ff:ff:ff:ff:ff:ff
> # ip link set wlan0 up
> # hexdump /sys/kernel/debug/ieee80211/phy0/ath10k/cal_data
> # ip link set wlan0 down
> # hexdump /sys/kernel/debug/ieee80211/phy0/ath10k/cal_data
> hexdump: /sys/kernel/debug/ieee80211/phy0/ath10k/cal_data: Network is down
> #
>
> What's odd is that I got a file with zero bytes, but no time to
> investigate it now.
>
> --
> Kalle Valo
Adrian Chadd Sept. 13, 2016, 8:50 p.m. UTC | #6
Yes! :)


-a


On 13 September 2016 at 13:49, Marty Faltesek <mfaltesek@google.com> wrote:
> Hey Kalle
>
> OK this does work for me after all. Thanks.
>
> I wrote a patch to cache cal_data only while the core is off. I don't
> need it now, but wondering if there is any benefit
> to submitting it?
>
> thanks,
>
> Marty
>
>
>
>
>
> On Tue, Sep 13, 2016 at 6:46 AM, Valo, Kalle <kvalo@qca.qualcomm.com> wrote:
>> Marty Faltesek <mfaltesek@google.com> writes:
>>
>>> On Mon, Sep 12, 2016 at 4:50 AM, Michal Kazior <michal.kazior@tieto.com> wrote:
>>>> On 9 September 2016 at 19:42, Marty Faltesek <mfaltesek@google.com> wrote:
>>>>> It's blocked by the code below which I tried to ifdef out, but then it
>>>>> returns all 0's.
>>>>>
>>>>> diff --git a/drivers/net/wireless/ath/ath10k/debug.c
>>>>> b/drivers/net/wireless/ath/ath10k/debug.c
>>>>> index 8b01e3e..bb8b7ec 100644
>>>>> --- a/drivers/net/wireless/ath/ath10k/debug.c
>>>>> +++ b/drivers/net/wireless/ath/ath10k/debug.c
>>>>> @@ -1433,12 +1433,13 @@ static int ath10k_debug_cal_data_open(struct
>>>>> inode *inode, struct file *file)
>>>>>         int ret;
>>>>>
>>>>>         mutex_lock(&ar->conf_mutex);
>>>>> -
>>>>> +#if 0
>>>>>         if (ar->state != ATH10K_STATE_ON &&
>>>>>             ar->state != ATH10K_STATE_UTF) {
>>>>>                 ret = -ENETDOWN;
>>>>>                 goto err;
>>>>>         }
>>>>> +#endif
>>>>
>>>> This won't work. The driver needs to go through ath10k_start(), i.e.
>>>> firmware must be loaded. Cal data is cooked as part of that.
>>>>
>>>> You could get away with just `ifconfig wlan0 up`. You don't need to
>>>> connect or anything.
>>>
>>> This does not work:
>>>
>>> hexdump: ./kernel/debug/ieee80211/phy1/ath10k/cal_data: Network is down
>>>
>>> But it works after starting the AP. Are you sure about what you said?
>>
>> It should work:
>>
>> # ip link show wlan0
>> 5: wlan0: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000
>>     link/ether 00:03:7f:48:d6:05 brd ff:ff:ff:ff:ff:ff
>> # ip link set wlan0 up
>> # hexdump /sys/kernel/debug/ieee80211/phy0/ath10k/cal_data
>> # ip link set wlan0 down
>> # hexdump /sys/kernel/debug/ieee80211/phy0/ath10k/cal_data
>> hexdump: /sys/kernel/debug/ieee80211/phy0/ath10k/cal_data: Network is down
>> #
>>
>> What's odd is that I got a file with zero bytes, but no time to
>> investigate it now.
>>
>> --
>> Kalle Valo
Kalle Valo Sept. 14, 2016, 5:01 a.m. UTC | #7
Marty Faltesek <mfaltesek@google.com> writes:

> OK this does work for me after all. Thanks.

Great.

> I wrote a patch to cache cal_data only while the core is off. I don't
> need it now, but wondering if there is any benefit
> to submitting it?

I guess Adrian already answered that :) So please do submit and let's
take a look at it.
Marty Faltesek Sept. 14, 2016, 11:54 p.m. UTC | #8
I sent out the patch.

I ran into another related issue. Let me explain what we are trying to do:

We want to examine cal_data for a possibly mis-calibrated OTP, and if
so, patch it and store
the result in /tmp (creating a symlink from
/ilb/firmware/ath10k/cal-pci-0001:01:00.0.bin to /tmp).

But ath10k_fetch_cal_file() is only read once at load time, so I've
modified the code to call this function from
ath10k_download_cal_file()
so that the driver catches the modified cal_data. This seems to work
based on the read back of the patched cal_data, but is it the right
approach, or is there a better or cleaner way?

On Wed, Sep 14, 2016 at 1:01 AM, Valo, Kalle <kvalo@qca.qualcomm.com> wrote:
> Marty Faltesek <mfaltesek@google.com> writes:
>
>> OK this does work for me after all. Thanks.
>
> Great.
>
>> I wrote a patch to cache cal_data only while the core is off. I don't
>> need it now, but wondering if there is any benefit
>> to submitting it?
>
> I guess Adrian already answered that :) So please do submit and let's
> take a look at it.
>
> --
> Kalle Valo
Michal Kazior Sept. 15, 2016, 11:24 a.m. UTC | #9
On 15 September 2016 at 01:54, Marty Faltesek <mfaltesek@google.com> wrote:
> I sent out the patch.
>
> I ran into another related issue. Let me explain what we are trying to do:
>
> We want to examine cal_data for a possibly mis-calibrated OTP, and if
> so, patch it and store
> the result in /tmp (creating a symlink from
> /ilb/firmware/ath10k/cal-pci-0001:01:00.0.bin to /tmp).
>
> But ath10k_fetch_cal_file() is only read once at load time, so I've
> modified the code to call this function from
> ath10k_download_cal_file()
> so that the driver catches the modified cal_data. This seems to work
> based on the read back of the patched cal_data, but is it the right
> approach, or is there a better or cleaner way?

The reason why firmware files (including cal, et al) in memory is to
guarantee firmware reloads result in the same set of hw capabilities
which can be advertised during mac80211 register time. Changing them
in runtime is not really clean.

I don't really have a clean answer for you.

Do you really need to keep given driver instance running without
interruption? At what point do you verify caldata? If it's before
upping interfaces you could either unload/load the driver or
unbind/bind the device itself to force it to read firmware files anew.


Michał
diff mbox

Patch

diff --git a/drivers/net/wireless/ath/ath10k/debug.c
b/drivers/net/wireless/ath/ath10k/debug.c
index 8b01e3e..bb8b7ec 100644
--- a/drivers/net/wireless/ath/ath10k/debug.c
+++ b/drivers/net/wireless/ath/ath10k/debug.c
@@ -1433,12 +1433,13 @@  static int ath10k_debug_cal_data_open(struct
inode *inode, struct file *file)
        int ret;

        mutex_lock(&ar->conf_mutex);
-
+#if 0
        if (ar->state != ATH10K_STATE_ON &&
            ar->state != ATH10K_STATE_UTF) {
                ret = -ENETDOWN;
                goto err;
        }
+#endif

On Fri, Sep 9, 2016 at 1:39 PM, Adrian Chadd <adrian@freebsd.org> wrote: