| Message ID | 20211016234609.1568317-1-chunkeey@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 27deb0f1570b0dbf465443857ce10ac6443d141d |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | [v2] ath10k: fetch (pre-)calibration data via nvmem subsystem | expand |
Christian Lamparter <chunkeey@gmail.com> writes: > ATH10K chips are used it wide range of routers, > accesspoints, range extenders, network appliances. > On these embedded devices, calibration data is often > stored on the main system's flash and was out of reach > for the driver. > > To bridge this gap, ath10k is getting extended to pull > the (pre-)calibration data through nvmem subsystem. > To do this, a nvmem-cell containing the information can > either be specified in the platform data or via device-tree. > > Tested with: > Netgear EX6150v2 (IPQ4018 - pre-calibration method) > TP-Link Archer C7 v2 (QCA9880v2 - old calibration method) > > Cc: Robert Marko <robimarko@gmail.com> > Cc: Thibaut VARÈNE <hacks@slashdirt.org> > Signed-off-by: Christian Lamparter <chunkeey@gmail.com> > --- > > v1 -> v2: > - use %zu and %u in the format string for size_t > and u32 types (catched by the "kernel test robot"). > - reworded commit message + successfully tested on QCA9880v2 > > I placed the nvmem code in front of the current "file" method > (firmware_request). Reason is that this makes it easier for me > to test it. If needed it can be moved to a different place. Looks good to me. Before I apply this, I want to mention to that I have had a long in my deferred queue related two patchsets: https://patchwork.kernel.org/project/linux-wireless/patch/20200927192515.86-1-ansuelsmth@gmail.com/ https://patchwork.kernel.org/project/linux-wireless/patch/20200927192515.86-2-ansuelsmth@gmail.com/ https://patchwork.kernel.org/project/linux-wireless/patch/20200918181104.98-1-ansuelsmth@gmail.com/ https://patchwork.kernel.org/project/linux-wireless/patch/20200918181104.98-2-ansuelsmth@gmail.com/ Christian, we don't need those anymore, right? Expect the first patch maybe.
On 28/10/2021 10:58, Kalle Valo wrote: > Christian Lamparter <chunkeey@gmail.com> writes: > >> ATH10K chips are used it wide range of routers, >> accesspoints, range extenders, network appliances. >> On these embedded devices, calibration data is often >> stored on the main system's flash and was out of reach >> for the driver. >> >> To bridge this gap, ath10k is getting extended to pull >> the (pre-)calibration data through nvmem subsystem. >> To do this, a nvmem-cell containing the information can >> either be specified in the platform data or via device-tree. >> >> Tested with: >> Netgear EX6150v2 (IPQ4018 - pre-calibration method) >> TP-Link Archer C7 v2 (QCA9880v2 - old calibration method) >> >> Cc: Robert Marko <robimarko@gmail.com> >> Cc: Thibaut VARÈNE <hacks@slashdirt.org> >> Signed-off-by: Christian Lamparter <chunkeey@gmail.com> >> --- >> >> v1 -> v2: >> - use %zu and %u in the format string for size_t >> and u32 types (catched by the "kernel test robot"). >> - reworded commit message + successfully tested on QCA9880v2 >> >> I placed the nvmem code in front of the current "file" method >> (firmware_request). Reason is that this makes it easier for me >> to test it. If needed it can be moved to a different place. > > Looks good to me. Before I apply this, I want to mention to that I have > had a long in my deferred queue related two patchsets: > https://patchwork.kernel.org/project/linux-wireless/patch/20200927192515.86-1-ansuelsmth@gmail.com/ > https://patchwork.kernel.org/project/linux-wireless/patch/20200927192515.86-2-ansuelsmth@gmail.com/ Oh ok, serves me right for not looking thoroughly googling this first. Alban Bedel and Ansuel's work made this nvmem all possible. And indeed, the second patch here looks eerie similar. Do you want to go with his two patches instead? I'll change mine, so it just consists of the cal_mode for the older QCA9880v2,QCA9887 and add the -EPROBE_DEFER handling. This -EPROBE_DEFER only ever comes up with the Meraki gear. This is because Meraki likes putting the MACs-Values into SoC-connected AT24 eeproms-chips. Everyone else just have them in a proper FLASH partition. Though, this's usually nothing more than adding the following line: if (ret == -EPROBER_DEFER) return ret; > https://patchwork.kernel.org/project/linux-wireless/patch/20200918181104.98-1-ansuelsmth@gmail.com/ > https://patchwork.kernel.org/project/linux-wireless/patch/20200918181104.98-2-ansuelsmth@gmail.com/ Ansuel's post: https://patchwork.kernel.org/project/linux-wireless/patch/20200918181104.98-2-ansuelsmth@gmail.com/#23639361 > You are right about nvmem... Problem is that nvmem for mtd is still not > supported. I already sent a patch to fix this in the mtd mailing list but > I'm waiting for review... > If that will be accepted, I can convert this patch to use nvmem api. The nvmem api is there (which makes these two patches obsolete I think). Granted: The nvmem can't do all the same cases (From what I know, mtd partitions splitters and mtdparts through commandline is being worked on. But we always have userspace + firmware_request as a fallback). Cheers, Christian
> > On 28/10/2021 10:58, Kalle Valo wrote: > > Christian Lamparter <chunkeey@gmail.com> writes: > > > >> ATH10K chips are used it wide range of routers, > >> accesspoints, range extenders, network appliances. > >> On these embedded devices, calibration data is often > >> stored on the main system's flash and was out of reach > >> for the driver. > >> > >> To bridge this gap, ath10k is getting extended to pull > >> the (pre-)calibration data through nvmem subsystem. > >> To do this, a nvmem-cell containing the information can > >> either be specified in the platform data or via device-tree. > >> > >> Tested with: > >> Netgear EX6150v2 (IPQ4018 - pre-calibration method) > >> TP-Link Archer C7 v2 (QCA9880v2 - old calibration method) > >> > >> Cc: Robert Marko <robimarko@gmail.com> > >> Cc: Thibaut VARÈNE <hacks@slashdirt.org> > >> Signed-off-by: Christian Lamparter <chunkeey@gmail.com> > >> --- > >> > >> v1 -> v2: > >> - use %zu and %u in the format string for size_t > >> and u32 types (catched by the "kernel test robot"). > >> - reworded commit message + successfully tested on QCA9880v2 > >> > >> I placed the nvmem code in front of the current "file" method > >> (firmware_request). Reason is that this makes it easier for me > >> to test it. If needed it can be moved to a different place. > > > > Looks good to me. Before I apply this, I want to mention to that I have > > had a long in my deferred queue related two patchsets: > > > > https://patchwork.kernel.org/project/linux-wireless/patch/20200927192515.86-1-ansuelsmth@gmail.com/ > > https://patchwork.kernel.org/project/linux-wireless/patch/20200927192515.86-2-ansuelsmth@gmail.com/ > Oh ok, serves me right for not looking thoroughly googling this first. > Alban Bedel and Ansuel's work made this nvmem all possible. And indeed, > the second patch here looks eerie similar. > > Do you want to go with his two patches instead? I'll change mine, so it > just consists of the cal_mode for the older QCA9880v2,QCA9887 and > add the -EPROBE_DEFER handling. This -EPROBE_DEFER only ever comes up > with the Meraki gear. This is because Meraki likes putting the MACs-Values > into SoC-connected AT24 eeproms-chips. Everyone else just have them in a > proper FLASH partition. Though, this's usually nothing more than adding > the following line: > > if (ret == -EPROBER_DEFER) > return ret; > The 2 patch has to change to use the nvmem api. The mtd one is present in ath10k-ct and looks to work well on my devices. At times I also tested one implementation that used nvmem api to fetch cal data from nvmem using a specific nvmem cell name passed in the dts and it did work just right. Also with recent changes to nvmem api the of_platform_device_create and the pdev check are not needed anymore are the nvmem can find cell also if the platform is not registered > > https://patchwork.kernel.org/project/linux-wireless/patch/20200918181104.98-1-ansuelsmth@gmail.com/ > > https://patchwork.kernel.org/project/linux-wireless/patch/20200918181104.98-2-ansuelsmth@gmail.com/ > > Ansuel's post: https://patchwork.kernel.org/project/linux-wireless/patch/20200918181104.98-2-ansuelsmth@gmail.com/#23639361 > > You are right about nvmem... Problem is that nvmem for mtd is still not > > supported. I already sent a patch to fix this in the mtd mailing list but > > I'm waiting for review... > > If that will be accepted, I can convert this patch to use nvmem api. > > The nvmem api is there (which makes these two patches obsolete I think). > Granted: The nvmem can't do all the same cases (From what I know, mtd > partitions splitters and mtdparts through commandline is being worked on. > But we always have userspace + firmware_request as a fallback). > > Cheers, > Christian
Christian Lamparter <chunkeey@gmail.com> writes: > On 28/10/2021 10:58, Kalle Valo wrote: >> Christian Lamparter <chunkeey@gmail.com> writes: >> >>> ATH10K chips are used it wide range of routers, >>> accesspoints, range extenders, network appliances. >>> On these embedded devices, calibration data is often >>> stored on the main system's flash and was out of reach >>> for the driver. >>> >>> To bridge this gap, ath10k is getting extended to pull >>> the (pre-)calibration data through nvmem subsystem. >>> To do this, a nvmem-cell containing the information can >>> either be specified in the platform data or via device-tree. >>> >>> Tested with: >>> Netgear EX6150v2 (IPQ4018 - pre-calibration method) >>> TP-Link Archer C7 v2 (QCA9880v2 - old calibration method) >>> >>> Cc: Robert Marko <robimarko@gmail.com> >>> Cc: Thibaut VARÈNE <hacks@slashdirt.org> >>> Signed-off-by: Christian Lamparter <chunkeey@gmail.com> >>> --- >>> >>> v1 -> v2: >>> - use %zu and %u in the format string for size_t >>> and u32 types (catched by the "kernel test robot"). >>> - reworded commit message + successfully tested on QCA9880v2 >>> >>> I placed the nvmem code in front of the current "file" method >>> (firmware_request). Reason is that this makes it easier for me >>> to test it. If needed it can be moved to a different place. >> >> Looks good to me. Before I apply this, I want to mention to that I have >> had a long in my deferred queue related two patchsets: > > >> https://patchwork.kernel.org/project/linux-wireless/patch/20200927192515.86-1-ansuelsmth@gmail.com/ >> https://patchwork.kernel.org/project/linux-wireless/patch/20200927192515.86-2-ansuelsmth@gmail.com/ > > Oh ok, serves me right for not looking thoroughly googling this first. > Alban Bedel and Ansuel's work made this nvmem all possible. And indeed, > the second patch here looks eerie similar. > > Do you want to go with his two patches instead? I would prefer to take your patch. > I'll change mine, so it just consists of the cal_mode for the older > QCA9880v2,QCA9887 and add the -EPROBE_DEFER handling. This > -EPROBE_DEFER only ever comes up with the Meraki gear. This is because > Meraki likes putting the MACs-Values into SoC-connected AT24 > eeproms-chips. Everyone else just have them in a proper FLASH > partition. Though, this's usually nothing more than adding the > following line: > > if (ret == -EPROBER_DEFER) > return ret; So I'll drop this version and wait for v3?
On 28/10/2021 13:52, Kalle Valo wrote: >>>> >>>> v1 -> v2: >>>> - use %zu and %u in the format string for size_t >>>> and u32 types (catched by the "kernel test robot"). >>>> - reworded commit message + successfully tested on QCA9880v2 >>>> >>>> I placed the nvmem code in front of the current "file" method >>>> (firmware_request). Reason is that this makes it easier for me >>>> to test it. If needed it can be moved to a different place. >>> >>> Looks good to me. Before I apply this, I want to mention to that I have >>> had a long in my deferred queue related two patchsets: >> >> >>> https://patchwork.kernel.org/project/linux-wireless/patch/20200927192515.86-1-ansuelsmth@gmail.com/ >>> https://patchwork.kernel.org/project/linux-wireless/patch/20200927192515.86-2-ansuelsmth@gmail.com/ >> >> Oh ok, serves me right for not looking thoroughly googling this first. >> Alban Bedel and Ansuel's work made this nvmem all possible. And indeed, >> the second patch here looks eerie similar. >> >> Do you want to go with his two patches instead? > > I would prefer to take your patch. Ok. >> I'll change mine, so it just consists of the cal_mode for the older >> QCA9880v2,QCA9887 and add the -EPROBE_DEFER handling. This >> -EPROBE_DEFER only ever comes up with the Meraki gear. This is because >> Meraki likes putting the MACs-Values into SoC-connected AT24 >> eeproms-chips. Everyone else just have them in a proper FLASH >> partition. Though, this's usually nothing more than adding the >> following line: >> >> if (ret == -EPROBER_DEFER) >> return ret; > > So I'll drop this version and wait for v3? I guess that "waiting for v3" won't be necessary in this case. If @Ansuel doesn't voice any concerns, you might as well just apply v2. The "[1/2] ath10k: Try to get mac-address from dts" patch will need a respin, so it can apply cleanly. Is Anyone interested? If not, I can take a shot at it on Saturday. (There's the tiny question of that device_get_mac_address() which ath10k currently uses. It looks a lot like of_get_mac_address() too! but with extra ACPI (through FWNODE-which also includes OF), but without NVMEM.) Cheers, Christian
> > On 28/10/2021 13:52, Kalle Valo wrote: > > >>>> > >>>> v1 -> v2: > >>>> - use %zu and %u in the format string for size_t > >>>> and u32 types (catched by the "kernel test robot"). > >>>> - reworded commit message + successfully tested on QCA9880v2 > >>>> > >>>> I placed the nvmem code in front of the current "file" method > >>>> (firmware_request). Reason is that this makes it easier for me > >>>> to test it. If needed it can be moved to a different place. > >>> > >>> Looks good to me. Before I apply this, I want to mention to that I have > >>> had a long in my deferred queue related two patchsets: > >> > >> > >>> https://patchwork.kernel.org/project/linux-wireless/patch/20200927192515.86-1-ansuelsmth@gmail.com/ > >>> https://patchwork.kernel.org/project/linux-wireless/patch/20200927192515.86-2-ansuelsmth@gmail.com/ > >> > >> Oh ok, serves me right for not looking thoroughly googling this first. > >> Alban Bedel and Ansuel's work made this nvmem all possible. And indeed, > >> the second patch here looks eerie similar. > >> > >> Do you want to go with his two patches instead? > > > > I would prefer to take your patch. > > Ok. > > >> I'll change mine, so it just consists of the cal_mode for the older > >> QCA9880v2,QCA9887 and add the -EPROBE_DEFER handling. This > >> -EPROBE_DEFER only ever comes up with the Meraki gear. This is because > >> Meraki likes putting the MACs-Values into SoC-connected AT24 > >> eeproms-chips. Everyone else just have them in a proper FLASH > >> partition. Though, this's usually nothing more than adding the > >> following line: > >> > >> if (ret == -EPROBER_DEFER) > >> return ret; > > > > So I'll drop this version and wait for v3? > > I guess that "waiting for v3" won't be necessary in this case. > If @Ansuel doesn't voice any concerns, you might as well just > apply v2. > > The "[1/2] ath10k: Try to get mac-address from dts" patch > will need a respin, so it can apply cleanly. > > Is Anyone interested? If not, I can take a shot at it on Saturday. > A refreshed patch is applied to atk10k-ct repo so it would be good to have the same patch on normal ath10k. Many router would benefit from that. > (There's the tiny question of that device_get_mac_address() which > ath10k currently uses. It looks a lot like of_get_mac_address() too! > but with extra ACPI (through FWNODE-which also includes OF), but > without NVMEM.) > > Cheers, > Christian About this I never manage to understand the priority... Should ACPI variant have priority and fallback to the OF api or the OF api should overwrite any mac if a nvmem cell is found?
On 28/10/2021 20:57, Ansuel Smith wrote: >> >> The "[1/2] ath10k: Try to get mac-address from dts" patch >> will need a respin, so it can apply cleanly. >> >> Is Anyone interested? If not, I can take a shot at it on Saturday. >> > > A refreshed patch is applied to atk10k-ct repo so it would be good to > have the same patch on normal ath10k. Many router would benefit > from that. Found it! https://github.com/greearb/ath10k-ct/commit/e6a7d5b5b834737cd12e357b5efdc2e42d923bf6.patch Hmm, Author is now "Ben" and the whole commit message is gone. Now, adding the commit message back from your original patch is not a problem, but the missing "Signed-off-by" from him and you might be. ... But then, do we need it? Because there might be the option to extend device_get_mac_address() instead?! ... >> (There's the tiny question of that device_get_mac_address() which >> ath10k currently uses. It looks a lot like of_get_mac_address() too! >> but with extra ACPI (through FWNODE-which also includes OF), but >> without NVMEM.) >> >> Cheers, >> Christian > > About this I never manage to understand the priority... Should ACPI > variant have priority and fallback to the OF api or the OF api should > overwrite any mac if a nvmem cell is found? Hmm, from what I know the device_/fwnode_*() functions are just wrappers for either ACPI (on systems with ACPI - x86 and ARM) or OF (on systems with Device-tree) functions... (There is also "software nodes", I think these are the lookupd stuff that came up recently with APU2 + MX100) This confused me too. But I might be able to show this in the context of ath10k-ct's current ath10k_core_probe_fw() and this threads new subject: <https://github.com/greearb/ath10k-ct/blob/e6a7d5b5b834737cd12e357b5efdc2e42d923bf6/ath10k-5.15/core.c#L4044> copied in here for a better reading experience: | |device_get_mac_address(ar->dev, ar->mac_addr, sizeof(ar->mac_addr)); | |/* Try to get mac address from device node (from nvmem cell) */ |of_get_mac_address(ar->dev->of_node, ar->mac_addr); | The device_get_mac_address() will on OF platforms essentially check and process the same "mac-address", "local-mac-address" and "address" OF properties of the same device-tree node as the following of_get_mac_address() will do. There's no priority. I think now, that instead of adding of_get_mac_address() into ath10k, the time might be better spent asking what's the goal for device_get_mac_address() is. Is device_get_mac_address() poised to usurp of_get_mac_address()? And if it is: Should it be extended to get that "mac-address in nvmem-cell" as well? Because then we don't really need to touch ath10k at all (well, maybe except for -EPROBE_DEFER handling). Cheers, Christian
On 10/28/21 1:29 PM, Christian Lamparter wrote: > On 28/10/2021 20:57, Ansuel Smith wrote: >>> >>> The "[1/2] ath10k: Try to get mac-address from dts" patch >>> will need a respin, so it can apply cleanly. >>> >>> Is Anyone interested? If not, I can take a shot at it on Saturday. >>> >> >> A refreshed patch is applied to atk10k-ct repo so it would be good to >> have the same patch on normal ath10k. Many router would benefit >> from that. > > Found it! > > https://github.com/greearb/ath10k-ct/commit/e6a7d5b5b834737cd12e357b5efdc2e42d923bf6.patch > > Hmm, Author is now "Ben" and the whole commit message is gone. > Now, adding the commit message back from your original patch > is not a problem, but the missing "Signed-off-by" from him and > you might be. > ... > > But then, do we need it? Because there might be the option to extend > device_get_mac_address() instead?! ... I applied this since owrt was using it and it decreased out-of-tree patches, and it does not break my (non-owrt) use case. Other than that, I don't know much about this patch... If it is decided this patch should go away, please let me know so I can adjust ath10k-ct accordingly. Thanks Ben
> > On 28/10/2021 20:57, Ansuel Smith wrote: > >> > >> The "[1/2] ath10k: Try to get mac-address from dts" patch > >> will need a respin, so it can apply cleanly. > >> > >> Is Anyone interested? If not, I can take a shot at it on Saturday. > >> > > > > A refreshed patch is applied to atk10k-ct repo so it would be good to > > have the same patch on normal ath10k. Many router would benefit > > from that. > > Found it! > > https://github.com/greearb/ath10k-ct/commit/e6a7d5b5b834737cd12e357b5efdc2e42d923bf6.patch > > Hmm, Author is now "Ben" and the whole commit message is gone. > Now, adding the commit message back from your original patch > is not a problem, but the missing "Signed-off-by" from him and > you might be. > ... > Let's use the patch in ath10k. Seems cleaner and ready to merge. (probe_defer to handle) https://github.com/openwrt/openwrt/blob/master/package/kernel/mac80211/patches/ath10k/984-ath10k-Try-to-get-mac-address-from-dts.patch > But then, do we need it? Because there might be the option to extend > device_get_mac_address() instead?! ... > That is also my concern... It would be good to add support for nvmem in the device_get_mac_address(). > >> (There's the tiny question of that device_get_mac_address() which > >> ath10k currently uses. It looks a lot like of_get_mac_address() too! > >> but with extra ACPI (through FWNODE-which also includes OF), but > >> without NVMEM.) > >> > >> Cheers, > >> Christian > > > > About this I never manage to understand the priority... Should ACPI > > variant have priority and fallback to the OF api or the OF api should > > overwrite any mac if a nvmem cell is found? > > Hmm, from what I know the device_/fwnode_*() functions are just wrappers > for either ACPI (on systems with ACPI - x86 and ARM) or OF (on systems with > Device-tree) functions... (There is also "software nodes", I think these are > the lookupd stuff that came up recently with APU2 + MX100) > > This confused me too. But I might be able to show this in the context of > ath10k-ct's current ath10k_core_probe_fw() and this threads new subject: > > <https://github.com/greearb/ath10k-ct/blob/e6a7d5b5b834737cd12e357b5efdc2e42d923bf6/ath10k-5.15/core.c#L4044> > copied in here for a better reading experience: > | > |device_get_mac_address(ar->dev, ar->mac_addr, sizeof(ar->mac_addr)); > | > |/* Try to get mac address from device node (from nvmem cell) */ > |of_get_mac_address(ar->dev->of_node, ar->mac_addr); > | > > The device_get_mac_address() will on OF platforms essentially check and > process the same "mac-address", "local-mac-address" and "address" OF > properties of the same device-tree node as the following of_get_mac_address() > will do. There's no priority. > The question was with the strange case of both device_get_mac_address and and of_get_mac_addressproviding a mac what should be taken? In the current implementation of_get_mac_address sets the mac and has priority. > I think now, that instead of adding of_get_mac_address() into ath10k, > the time might be better spent asking what's the goal for device_get_mac_address() is. > Is device_get_mac_address() poised to usurp of_get_mac_address()? And if it > is: Should it be extended to get that "mac-address in nvmem-cell" as well? > Because then we don't really need to touch ath10k at all (well, maybe except > for -EPROBE_DEFER handling). > To me it seems device_get_mac_address is used as it has ACPI support and fwnode seems a better implementation of the OF api. Anyway yes i think the current patch should handle the error as nvmem can return probe_defer error. > Cheers, > Christian
Christian Lamparter <chunkeey@gmail.com> wrote: > ATH10K chips are used it wide range of routers, > accesspoints, range extenders, network appliances. > On these embedded devices, calibration data is often > stored on the main system's flash and was out of reach > for the driver. > > To bridge this gap, ath10k is getting extended to pull > the (pre-)calibration data through nvmem subsystem. > To do this, a nvmem-cell containing the information can > either be specified in the platform data or via device-tree. > > Tested with: > Netgear EX6150v2 (IPQ4018 - pre-calibration method) > TP-Link Archer C7 v2 (QCA9880v2 - old calibration method) > > Cc: Robert Marko <robimarko@gmail.com> > Cc: Thibaut VARÈNE <hacks@slashdirt.org> > Signed-off-by: Christian Lamparter <chunkeey@gmail.com> > Signed-off-by: Kalle Valo <kvalo@codeaurora.org> Patch applied to ath-next branch of ath.git, thanks. 27deb0f1570b ath10k: fetch (pre-)calibration data via nvmem subsystem
Christian Lamparter <chunkeey@gmail.com> writes: > On 28/10/2021 13:52, Kalle Valo wrote: > >>>>> >>>>> v1 -> v2: >>>>> - use %zu and %u in the format string for size_t >>>>> and u32 types (catched by the "kernel test robot"). >>>>> - reworded commit message + successfully tested on QCA9880v2 >>>>> >>>>> I placed the nvmem code in front of the current "file" method >>>>> (firmware_request). Reason is that this makes it easier for me >>>>> to test it. If needed it can be moved to a different place. >>>> >>>> Looks good to me. Before I apply this, I want to mention to that I have >>>> had a long in my deferred queue related two patchsets: >>> >>> >>>> https://patchwork.kernel.org/project/linux-wireless/patch/20200927192515.86-1-ansuelsmth@gmail.com/ >>>> https://patchwork.kernel.org/project/linux-wireless/patch/20200927192515.86-2-ansuelsmth@gmail.com/ >>> >>> Oh ok, serves me right for not looking thoroughly googling this first. >>> Alban Bedel and Ansuel's work made this nvmem all possible. And indeed, >>> the second patch here looks eerie similar. >>> >>> Do you want to go with his two patches instead? >> >> I would prefer to take your patch. > > Ok. > >>> I'll change mine, so it just consists of the cal_mode for the older >>> QCA9880v2,QCA9887 and add the -EPROBE_DEFER handling. This >>> -EPROBE_DEFER only ever comes up with the Meraki gear. This is because >>> Meraki likes putting the MACs-Values into SoC-connected AT24 >>> eeproms-chips. Everyone else just have them in a proper FLASH >>> partition. Though, this's usually nothing more than adding the >>> following line: >>> >>> if (ret == -EPROBER_DEFER) >>> return ret; >> >> So I'll drop this version and wait for v3? > > I guess that "waiting for v3" won't be necessary in this case. > If @Ansuel doesn't voice any concerns, you might as well just > apply v2. Ok, I took the v2 now. Thanks for helping out, I appreciate it!
diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c index c21e05549f61..9f0e3f010620 100644 --- a/drivers/net/wireless/ath/ath10k/core.c +++ b/drivers/net/wireless/ath/ath10k/core.c @@ -12,6 +12,7 @@ #include <linux/dmi.h> #include <linux/ctype.h> #include <linux/pm_qos.h> +#include <linux/nvmem-consumer.h> #include <asm/byteorder.h> #include "core.h" @@ -935,7 +936,8 @@ static int ath10k_core_get_board_id_from_otp(struct ath10k *ar) } if (ar->cal_mode == ATH10K_PRE_CAL_MODE_DT || - ar->cal_mode == ATH10K_PRE_CAL_MODE_FILE) + ar->cal_mode == ATH10K_PRE_CAL_MODE_FILE || + ar->cal_mode == ATH10K_PRE_CAL_MODE_NVMEM) bmi_board_id_param = BMI_PARAM_GET_FLASH_BOARD_ID; else bmi_board_id_param = BMI_PARAM_GET_EEPROM_BOARD_ID; @@ -1726,7 +1728,8 @@ static int ath10k_download_and_run_otp(struct ath10k *ar) /* As of now pre-cal is valid for 10_4 variants */ if (ar->cal_mode == ATH10K_PRE_CAL_MODE_DT || - ar->cal_mode == ATH10K_PRE_CAL_MODE_FILE) + ar->cal_mode == ATH10K_PRE_CAL_MODE_FILE || + ar->cal_mode == ATH10K_PRE_CAL_MODE_NVMEM) bmi_otp_exe_param = BMI_PARAM_FLASH_SECTION_ALL; ret = ath10k_bmi_execute(ar, address, bmi_otp_exe_param, &result); @@ -1853,6 +1856,39 @@ static int ath10k_download_cal_eeprom(struct ath10k *ar) return ret; } +static int ath10k_download_cal_nvmem(struct ath10k *ar, const char *cell_name) +{ + struct nvmem_cell *cell; + void *buf; + size_t len; + int ret; + + cell = devm_nvmem_cell_get(ar->dev, cell_name); + if (IS_ERR(cell)) { + ret = PTR_ERR(cell); + return ret; + } + + buf = nvmem_cell_read(cell, &len); + if (IS_ERR(buf)) + return PTR_ERR(buf); + + if (ar->hw_params.cal_data_len != len) { + kfree(buf); + ath10k_warn(ar, "invalid calibration data length in nvmem-cell '%s': %zu != %u\n", + cell_name, len, ar->hw_params.cal_data_len); + return -EMSGSIZE; + } + + ret = ath10k_download_board_data(ar, buf, len); + kfree(buf); + if (ret) + ath10k_warn(ar, "failed to download calibration data from nvmem-cell '%s': %d\n", + cell_name, ret); + + return ret; +} + int ath10k_core_fetch_firmware_api_n(struct ath10k *ar, const char *name, struct ath10k_fw_file *fw_file) { @@ -2087,6 +2123,18 @@ static int ath10k_core_pre_cal_download(struct ath10k *ar) { int ret; + ret = ath10k_download_cal_nvmem(ar, "pre-calibration"); + if (ret == 0) { + ar->cal_mode = ATH10K_PRE_CAL_MODE_NVMEM; + goto success; + } else if (ret == -EPROBE_DEFER) { + return ret; + } + + ath10k_dbg(ar, ATH10K_DBG_BOOT, + "boot did not find a pre-calibration nvmem-cell, try file next: %d\n", + ret); + ret = ath10k_download_cal_file(ar, ar->pre_cal_file); if (ret == 0) { ar->cal_mode = ATH10K_PRE_CAL_MODE_FILE; @@ -2153,6 +2201,18 @@ static int ath10k_download_cal_data(struct ath10k *ar) "pre cal download procedure failed, try cal file: %d\n", ret); + ret = ath10k_download_cal_nvmem(ar, "calibration"); + if (ret == 0) { + ar->cal_mode = ATH10K_CAL_MODE_NVMEM; + goto done; + } else if (ret == -EPROBE_DEFER) { + return ret; + } + + ath10k_dbg(ar, ATH10K_DBG_BOOT, + "boot did not find a calibration nvmem-cell, try file next: %d\n", + ret); + ret = ath10k_download_cal_file(ar, ar->cal_file); if (ret == 0) { ar->cal_mode = ATH10K_CAL_MODE_FILE; diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h index 5aeff2d9f6cf..9f6680b3be0a 100644 --- a/drivers/net/wireless/ath/ath10k/core.h +++ b/drivers/net/wireless/ath/ath10k/core.h @@ -877,8 +877,10 @@ enum ath10k_cal_mode { ATH10K_CAL_MODE_FILE, ATH10K_CAL_MODE_OTP, ATH10K_CAL_MODE_DT, + ATH10K_CAL_MODE_NVMEM, ATH10K_PRE_CAL_MODE_FILE, ATH10K_PRE_CAL_MODE_DT, + ATH10K_PRE_CAL_MODE_NVMEM, ATH10K_CAL_MODE_EEPROM, }; @@ -898,10 +900,14 @@ static inline const char *ath10k_cal_mode_str(enum ath10k_cal_mode mode) return "otp"; case ATH10K_CAL_MODE_DT: return "dt"; + case ATH10K_CAL_MODE_NVMEM: + return "nvmem"; case ATH10K_PRE_CAL_MODE_FILE: return "pre-cal-file"; case ATH10K_PRE_CAL_MODE_DT: return "pre-cal-dt"; + case ATH10K_PRE_CAL_MODE_NVMEM: + return "pre-cal-nvmem"; case ATH10K_CAL_MODE_EEPROM: return "eeprom"; }
ATH10K chips are used it wide range of routers, accesspoints, range extenders, network appliances. On these embedded devices, calibration data is often stored on the main system's flash and was out of reach for the driver. To bridge this gap, ath10k is getting extended to pull the (pre-)calibration data through nvmem subsystem. To do this, a nvmem-cell containing the information can either be specified in the platform data or via device-tree. Tested with: Netgear EX6150v2 (IPQ4018 - pre-calibration method) TP-Link Archer C7 v2 (QCA9880v2 - old calibration method) Cc: Robert Marko <robimarko@gmail.com> Cc: Thibaut VARÈNE <hacks@slashdirt.org> Signed-off-by: Christian Lamparter <chunkeey@gmail.com> --- v1 -> v2: - use %zu and %u in the format string for size_t and u32 types (catched by the "kernel test robot"). - reworded commit message + successfully tested on QCA9880v2 I placed the nvmem code in front of the current "file" method (firmware_request). Reason is that this makes it easier for me to test it. If needed it can be moved to a different place. --- drivers/net/wireless/ath/ath10k/core.c | 64 +++++++++++++++++++++++++- drivers/net/wireless/ath/ath10k/core.h | 6 +++ 2 files changed, 68 insertions(+), 2 deletions(-)