Re: [PATCH] iwlwifi: Fix softirq/hardirq disabling in iwl_pcie_enqueue_hcmd()
On Sat, 2021-03-13 at 19:06 +0200, Kalle Valo wrote: > Luca Coelho writes: > > > On Sat, 2021-03-13 at 16:43 +0100, Jiri Kosina wrote: > > > On Sat, 13 Mar 2021, Kalle Valo wrote: > > > > > > > > > > From: Jiri Kosina > > > > > > > > > > > > > > It's possible for iwl_pcie_enqueue_hcmd() to be called with hard > > > > > > > IRQs > > > > > > > disabled (e.g. from LED core). We can't enable BHs in such a > > > > > > > situation. > > > > > > > > > > > > > > Turn the unconditional BH-enable/BH-disable code into > > > > > > > hardirq-disable/conditional-enable. > > > > > > > > > > > > > > This fixes the warning below. > > > > > > > > > > > > Hi, > > > > > > > > > > > > friendly ping on this one ... > > > > > > > > > > Luca, > > > > > > > > > > Johannes is telling me that he merged this patch internally, but I > > > > > have no > > > > > idea what is happening to it ... ? > > > > > > > > > > The reported splat is a clear bug, so it should be fixed one way or > > > > > the > > > > > other. > > > > > > > > Should I take this to wireless-drivers? > > > > > > I can't speak for the maintainers, but as far as I am concerned, it > > > definitely is a 5.12 material, as it fixes real scheduling bug. > > > > Yes, please take this to w-d. We have a similar patch internally, but > > there's a backlog and it will take me some time to get to it. I'll > > resolve eventual conflicts when time comes. > > Ok, can I have your ack for patchwork? Sorry, forgot that. Acked-by: Luca Coelho -- Cheers, Luca.
Re: [PATCH] iwlwifi: Fix softirq/hardirq disabling in iwl_pcie_enqueue_hcmd()
On Sat, 2021-03-13 at 16:43 +0100, Jiri Kosina wrote: > On Sat, 13 Mar 2021, Kalle Valo wrote: > > > > > > From: Jiri Kosina > > > > > > > > > > It's possible for iwl_pcie_enqueue_hcmd() to be called with hard IRQs > > > > > disabled (e.g. from LED core). We can't enable BHs in such a > > > > > situation. > > > > > > > > > > Turn the unconditional BH-enable/BH-disable code into > > > > > hardirq-disable/conditional-enable. > > > > > > > > > > This fixes the warning below. > > > > > > > > Hi, > > > > > > > > friendly ping on this one ... > > > > > > Luca, > > > > > > Johannes is telling me that he merged this patch internally, but I have > > > no > > > idea what is happening to it ... ? > > > > > > The reported splat is a clear bug, so it should be fixed one way or the > > > other. > > > > Should I take this to wireless-drivers? > > I can't speak for the maintainers, but as far as I am concerned, it > definitely is a 5.12 material, as it fixes real scheduling bug. Yes, please take this to w-d. We have a similar patch internally, but there's a backlog and it will take me some time to get to it. I'll resolve eventual conflicts when time comes. Thanks! -- Cheers, Luca.
Re: [PATCH v2 00/29] [Set 1,2,3] Rid W=1 warnings in Wireless
On Tue, 2020-10-06 at 10:10 +0300, Kalle Valo wrote:
> Lee Jones writes:
>
> > On Tue, 06 Oct 2020, Kalle Valo wrote:
> >
> > > Lee Jones writes:
> > >
> > > > On Thu, 10 Sep 2020, Lee Jones wrote:
> > > >
> > > > > This is a rebased/re-worked set of patches which have been
> > > > > previously posted to the mailing list(s).
> > > > >
> > > > > This set is part of a larger effort attempting to clean-up W=1
> > > > > kernel builds, which are currently overwhelmingly riddled with
> > > > > niggly little warnings.
> > > > >
> > > > > There are quite a few W=1 warnings in the Wireless. My plan
> > > > > is to work through all of them over the next few weeks.
> > > > > Hopefully it won't be too long before drivers/net/wireless
> > > > > builds clean with W=1 enabled.
> > > > >
> > > > > Lee Jones (29):
> > > > > iwlwifi: dvm: Demote non-compliant kernel-doc headers
> > > > > iwlwifi: rs: Demote non-compliant kernel-doc headers
> > > > > iwlwifi: dvm: tx: Demote non-compliant kernel-doc headers
> > > > > iwlwifi: dvm: lib: Demote non-compliant kernel-doc headers
> > > > > iwlwifi: calib: Demote seemingly unintentional kerneldoc header
> > > > > wil6210: Fix a couple of formatting issues in 'wil6210_debugfs_init'
> > > > > iwlwifi: dvm: sta: Demote a bunch of nonconformant kernel-doc
> > > > > headers
> > > > > iwlwifi: mvm: ops: Remove unused static struct 'iwl_mvm_debug_names'
> > > > > iwlwifi: dvm: Demote a couple of nonconformant kernel-doc headers
> > > > > iwlwifi: mvm: utils: Fix some doc-rot
> > > > > iwlwifi: dvm: scan: Demote a few nonconformant kernel-doc headers
> > > > > iwlwifi: dvm: rxon: Demote non-conformant kernel-doc headers
> > > > > iwlwifi: mvm: tx: Demote misuse of kernel-doc headers
> > > > > iwlwifi: dvm: devices: Fix function documentation formatting issues
> > > > > iwlwifi: iwl-drv: Provide descriptions debugfs dentries
> > > > > wil6210: wmi: Fix formatting and demote non-conforming function
> > > > > headers
> > > > > wil6210: interrupt: Demote comment header which is clearly not
> > > > > kernel-doc
> > > > > wil6210: txrx: Demote obvious abuse of kernel-doc
> > > > > wil6210: txrx_edma: Demote comments which are clearly not kernel-doc
> > > > > wil6210: pmc: Demote a few nonconformant kernel-doc function headers
> > > > > wil6210: wil_platform: Demote kernel-doc header to standard comment
> > > > > block
> > > > > wil6210: wmi: Correct misnamed function parameter 'ptr_'
> > > > > ath6kl: wmi: Remove unused variable 'rate'
> > > > > ath9k: ar9002_initvals: Remove unused array
> > > > > 'ar9280PciePhy_clkreq_off_L1_9280'
> > > > > ath9k: ar9001_initvals: Remove unused array 'ar5416Bank6_9100'
> > > > > ath9k: ar5008_initvals: Remove unused table entirely
> > > > > ath9k: ar5008_initvals: Move ar5416Bank{0,1,2,3,7} to where they are
> > > > > used
> > > > > brcmsmac: phytbl_lcn: Remove unused array 'dot11lcn_gain_tbl_rev1'
> > > > > brcmsmac: phy_lcn: Remove unused variable
> > > > > 'lcnphy_rx_iqcomp_table_rev0'
> > > >
> > > > What's happening with all of these iwlwifi patches?
> > > >
> > > > Looks like they are still not applied.
> > >
> > > Luca (CCed) takes iwlwifi patches to his iwlwifi tree.
> >
> > Thanks Kalle.
> >
> > Luca,
> >
> > Do you know why these patches have not been applied yet? Do you
> > plan on applying them this week? -rc1 is not due for release for
> > nearly 3 weeks now that Linus tagged an -rc8.
>
> I can also take Lee's patches directly to wireless-drivers-next, if
> that's easier for Luca.
Hi,
Yes, please take these patches directly. It simplifies things.
Thanks!
--
Cheers,
Luca.
Re: [PATCH] iwlwifi: mvm: Remove unused inline function iwl_mvm_tid_to_ac_queue
YueHaibing wrote:
> commit cfbc6c4c5b91 ("iwlwifi: mvm: support mac80211 TXQs model")
> left behind this, remove it.
>
> Signed-off-by: YueHaibing
Patch applied to iwlwifi-next.git, thanks.
f12694634153 iwlwifi: mvm: Remove unused inline function iwl_mvm_tid_to_ac_queue
Re: [PATCH 6/9] net: wireless: intel: fix wiki website url
Flavio Suligoi wrote: > In some Intel files, the wiki url is still the old > "wireless.kernel.org" instead of the new > "wireless.wiki.kernel.org" > > Signed-off-by: Flavio Suligoi Patch applied to iwlwifi-next.git, thanks. e00c6d8d491b net: wireless: intel: fix wiki website url
Re: [PATCH] iwlwifi: Replace zero-length array with flexible-array
"Gustavo A. R. Silva" wrote:
> The current codebase makes use of the zero-length array language
> extension to the C90 standard, but the preferred mechanism to declare
> variable-length types such as these ones is a flexible array member[1][2],
> introduced in C99:
>
> struct foo {
> int stuff;
> struct boo array[];
> };
>
> By making use of the mechanism above, we will get a compiler warning
> in case the flexible array does not occur last in the structure, which
> will help us prevent some kind of undefined behavior bugs from being
> inadvertently introduced[3] to the codebase from now on.
>
> Also, notice that, dynamic memory allocations won't be affected by
> this change:
>
> "Flexible array members have incomplete type, and so the sizeof operator
> may not be applied. As a quirk of the original implementation of
> zero-length arrays, sizeof evaluates to zero."[1]
>
> sizeof(flexible-array-member) triggers a warning because flexible array
> members have incomplete type[1]. There are some instances of code in
> which the sizeof operator is being incorrectly/erroneously applied to
> zero-length arrays and the result is zero. Such instances may be hiding
> some bugs. So, this work (flexible-array member conversions) will also
> help to get completely rid of those sorts of issues.
>
> This issue was found with the help of Coccinelle.
>
> [1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
> [2] https://github.com/KSPP/linux/issues/21
> [3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour")
>
> Signed-off-by: Gustavo A. R. Silva
Patch applied to iwlwifi-next.git, thanks.
45c21a0e5ba4 iwlwifi: Replace zero-length array with flexible-array
Re: [PATCH] iwlwifi: fix memory leaks in iwl_pcie_ctxt_info_gen3_init
On Fri, 2019-09-27 at 15:56 -0500, Navid Emamdoost wrote:
> In iwl_pcie_ctxt_info_gen3_init there are cases that the allocated dma
> memory is leaked in case of error.
> DMA memories prph_scratch, prph_info, and ctxt_info_gen3 are allocated
> and initialized to be later assigned to trans_pcie. But in any error case
> before such assignment the allocated memories should be released.
> First of such error cases happens when iwl_pcie_init_fw_sec fails.
> Current implementation correctly releases prph_scratch. But in two
> sunsequent error cases where dma_alloc_coherent may fail, such releases
> are missing. This commit adds release for prph_scratch when allocation
> for prph_info fails, and adds releases for prph_scratch and prph_info
> when allocation for ctxt_info_gen3 fails.
>
> Fixes: 2ee824026288 ("iwlwifi: pcie: support context information for 22560
> devices")
>
> Signed-off-by: Navid Emamdoost
> ---
Thanks, Navid! I have applied this to our internal tree and it will
reach the mainline following our usual upstreaming process.
--
Cheers,
Luca.
Re: [PATCH] iwlwifi: dbg_ini: fix memory leak in alloc_sgtable
On Thu, 2019-09-12 at 23:23 -0500, Navid Emamdoost wrote: > In alloc_sgtable if alloc_page fails, the alocated table should be > released. > > Signed-off-by: Navid Emamdoost > --- > drivers/net/wireless/intel/iwlwifi/fw/dbg.c | 1 + > 1 file changed, 1 insertion(+) Thanks, Navid! I have applied this to our internal tree and it will reach the mainline following our usual upstreaming process. -- Cheers, Luca.
Re: [PATCH] iwlwifi: fix building without CONFIG_THERMAL
On Thu, 2019-09-19 at 13:55 +0200, Arnd Bergmann wrote:
> The iwl_mvm_send_temp_report_ths_cmd() function is now called without
> CONFIG_THERMAL, but not defined:
>
> ERROR: "iwl_mvm_send_temp_report_ths_cmd"
> [drivers/net/wireless/intel/iwlwifi/mvm/iwlmvm.ko] undefined!
>
> Move that function out of the #ifdef as well and change it so
> that empty data gets sent even if no thermal device was
> registered.
>
> Fixes: 242d9c8b9a93 ("iwlwifi: mvm: use FW thermal monitoring regardless of
> CONFIG_THERMAL")
> Signed-off-by: Arnd Bergmann
> ---
> No idea if this does what was intended in the commit that introduced
> the link failure, please see for youself.
Thanks for the fix, Arnd! We already sent a fix for this though[1] and
Kalle has already queued it for v5.3.
[1] https://patchwork.kernel.org/patch/11150431/
--
Cheers,
Luca.
Re: [PATCH] iwlwifi: mvm: Move static keyword to the front of declarations
On Sun, 2019-09-01 at 00:01 +0200, Krzysztof Wilczynski wrote:
> Move the static keyword to the front of declarations of
> he_if_types_ext_capa_sta and he_iftypes_ext_capa, and
> resolve the following compiler warnings that can be seen
> when building with warnings enabled (W=1):
>
> drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c:427:1: warning:
> ‘static’ is not at beginning of declaration [-Wold-style-declaration]
>
> drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c:434:1: warning:
> ‘static’ is not at beginning of declaration [-Wold-style-declaration]
>
> Signed-off-by: Krzysztof Wilczynski
> ---
> Related: https://lore.kernel.org/r/20190827233017.gk9...@google.com
>
> drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
> b/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
> index d6499763f0dd..937a843fed56 100644
> --- a/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
> +++ b/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
> @@ -424,14 +424,14 @@ int iwl_mvm_init_fw_regd(struct iwl_mvm *mvm)
> return ret;
> }
>
> -const static u8 he_if_types_ext_capa_sta[] = {
> +static const u8 he_if_types_ext_capa_sta[] = {
>[0] = WLAN_EXT_CAPA1_EXT_CHANNEL_SWITCHING,
>[2] = WLAN_EXT_CAPA3_MULTI_BSSID_SUPPORT,
>[7] = WLAN_EXT_CAPA8_OPMODE_NOTIF,
>[9] = WLAN_EXT_CAPA10_TWT_REQUESTER_SUPPORT,
> };
>
> -const static struct wiphy_iftype_ext_capab he_iftypes_ext_capa[] = {
> +static const struct wiphy_iftype_ext_capab he_iftypes_ext_capa[] = {
> {
> .iftype = NL80211_IFTYPE_STATION,
> .extended_capabilities = he_if_types_ext_capa_sta,
Thanks for your patch! Though we already have this change in our
internal tree (submitted by YueHaibing) and it will reach the mainline
soon.
--
Cheers,
Luca.
Re: [PATCH] iwlwifi: remove redundant assignment to variable bufsz
On Thu, 2019-08-01 at 17:44 +0100, Colin King wrote:
> From: Colin Ian King
>
> The variable bufsz is being initialized with a value that is never
> read and it is being updated later with a new value. The
> initialization is redundant and can be removed.
>
> Addresses-Coverity: ("Unused value")
> Signed-off-by: Colin Ian King
> ---
Thanks! I applied this to our internal tree and it will reach the
mainline following our usual upstreaming process.
--
Cheers,
Luca.
Re: PROBLEM: 5.3.0-rc* causes iwlwifi failure
On Thu, 2019-08-22 at 09:59 +0100, Chris Clayton wrote: > Thanks, Stuart. > > On 18/08/2019 11:55, Stuart Little wrote: > > On Sun, Aug 18, 2019 at 09:17:59AM +0100, Chris Clayton wrote: > > > > > > On 17/08/2019 22:44, Stuart Little wrote: > > > > After some private coaching from Serge Belyshev on git-revert I can > > > > confirm that reverting that commit atop the current tree resolves the > > > > issue (the wifi card scans for and finds networks just fine, no dmesg > > > > errors reported, etc.). > > > > > > > > > > I've reported the "Microcode SW error detected" issue too, but, wrongly, > > > only to LKML. I'll point that thread to this > > > one. I've also been experiencing my network stopping working after > > > suspend resume, but haven't got round to reporting > > > that yet. > > > > > > What was the git magic that you acquired to revert the patch, please? > > > > > By following the advice below, I reverted > 4fd445a2c855bbcab81fbe06d110e78dbd974a5b and using the resultant kernel I > haven't seen the "Microcode SW error detected" again. I am, however, still > experiencing the problem of my network not > working after resume from suspend. I've reported it to LKML, so it can be > followed there should anyone need/want to. FWIW, we're tracking the iwlwifi bug here: https://bugzilla.kernel.org/show_bug.cgi?id=204151 I'm thinking about how to solve this and will probably have a proper patch by the end of the week. -- Cheers, Luca.
Re: Regression with the latest iwlwifi-9260-*-46.ucode
On Fri, 2019-08-09 at 22:00 +0300, Thomas Backlund wrote: > Den 06-08-2019 kl. 16:04, skrev Takashi Iwai: > > On Mon, 05 Aug 2019 14:03:55 +0200, > > Now we got a feedback from the latest linux-firmware (20190726) and > > surprising the result was negative. The dmesg after the cold boot is > > found at: > >https://bugzilla.suse.com/show_bug.cgi?id=1142128#c26 > > > > The kernel is 5.2.3, so it should be new enough. > > > > If anything else needed (or something missing), let us know. > > > > I read on some forum that some commented that the "Add support for SAR > South Korea limitation" fix is needed, but it seemed weird... > > Anyway, with theese on top of 5.2.7 > > 39bd984c203e86f3 iwlwifi: mvm: don't send GEO_TX_POWER_LIMIT on version > < 41 > f5a47fae6aa3eb06 iwlwifi: mvm: fix version check for GEO_TX_POWER_LIMIT > support > 0c3d7282233c7b02 iwlwifi: Add support for SAR South Korea limitation > > > We have confirmation from an affected user that its now stable with both > older and newer firmwares... > > And we earlier tried with only the: > 39bd984c203e86f3 iwlwifi: mvm: don't send GEO_TX_POWER_LIMIT on version > < 41 > > But that did not help (not that I really expected it since its loading > version 46 firmwares anyway) > > So my guess is that the newer firmware actually subtly expects to get > the behaviour of the: > 0c3d7282233c7b02 iwlwifi: Add support for SAR South Korea limitation > > Of course that's still guessing and I assume only Intel fw guys can > verify that... Yes, you need the 3 patches. The first two should solve the "BAD_COMMAND" issue and the last one fixes the "NMI_INTERRUPT_WDG" issue. The first two are already in v5.3-rc4 and in v5.2.9 stable. I'm going to send the third one to stable now. -- Cheers, Luca.
Re: Regression with the latest iwlwifi-9260-*-46.ucode
On Mon, 2019-08-05 at 13:10 +0300, Luca Coelho wrote: > On Mon, 2019-08-05 at 12:05 +0200, Takashi Iwai wrote: > > On Mon, 05 Aug 2019 11:53:33 +0200, > > Luca Coelho wrote: > > > On Mon, 2019-08-05 at 12:48 +0300, Luca Coelho wrote: > > > > On Sun, 2019-07-21 at 18:43 +0200, Takashi Iwai wrote: > > > > > On Sat, 20 Jul 2019 22:49:33 +0200, > > > > > Luca Coelho wrote: > > > > > > On Sat, 2019-07-20 at 22:42 +0200, Takashi Iwai wrote: > > > > > > > On Fri, 19 Jul 2019 20:07:46 +0200, > > > > > > > Takashi Iwai wrote: > > > > > > > > On Fri, 19 Jul 2019 18:36:53 +0200, > > > > > > > > Luciano Coelho wrote: > > > > > > > > > Adding Dor. > > > > > > > > > > > > > > > > > > Hi Takashi, > > > > > > > > > > > > > > > > > > Do you have full logs of the crash? We can't see much from > > > > > > > > > the log > > > > > > > > > snippet pasted in the bug report. > > > > > > > > > > > > > > > > OK, I'll ask reporters. If you have a SUSE/openSUSE bugzilla > > > > > > > > account, > > > > > > > > feel free to join there. > > > > > > > > > > > > > > FYI, the dmesg's have been uploaded to the same bugzilla entry: > > > > > > > https://bugzilla.opensuse.org/show_bug.cgi?id=1142128 > > > > > > > > > > > > > > > > > > > Thanks! > > > > > > > > > > > > BTW, I pushed new firmwares to our firmware tree in git.kernel.org > > > > > > today. This is the patch: > > > > > > > > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/iwlwifi/linux-firmware.git/commit/?id=b5f09bb4f816abace0227d0f4e749859364cef6b > > > > > > > > > > > > It would be great if you can try it out and let us know whether the > > > > > > problem is gone or not. > > > > > > > > > > I created a test package and asked for testing. > > > > > The test result seems negative, showing the same error, > > > > > unfortunately. > > > > > > > > > > The dmesg was uploaded on the bugzilla entry. > > > > > > > > Thanks Takashi! We will look into them as soon as possible (sorry for > > > > the late reply, I just came back from vacations). > > > > > > Actually, I just noticed that your bugzilla is closed as "RESOLVED > > > FIXED". Is this still an issue? > > > > It's "closed" because our package contains the revert. > > > > > This seems like a mismatch between the WiFi and BT firmwares... And > > > most likely the same issue as this: > > > > > > https://bugzilla.kernel.org/show_bug.cgi?id=202163 > > > > OK, so we need the update of the whole linux-firmware. > > I'll refresh the package and ask for testing. > > I'm not sure the new BT firmware is in linux-firmware yet, since I > don't handle BT stuff. I hope it is. I just double-checked this and got confirmation that the latest BT firmware in linux-firmware.git is compatible with the latest WiFi firmware there as well. The only caveat is that the machine needs to be cold-booted for it to work But we are taking steps to make sure this will not happen next time (i.e. for future hardware), by syncing our BT and WiFi firmware releases. -- Cheers, Luca.
Re: Regression with the latest iwlwifi-9260-*-46.ucode
On Mon, 2019-08-05 at 12:05 +0200, Takashi Iwai wrote: > On Mon, 05 Aug 2019 11:53:33 +0200, > Luca Coelho wrote: > > On Mon, 2019-08-05 at 12:48 +0300, Luca Coelho wrote: > > > On Sun, 2019-07-21 at 18:43 +0200, Takashi Iwai wrote: > > > > On Sat, 20 Jul 2019 22:49:33 +0200, > > > > Luca Coelho wrote: > > > > > On Sat, 2019-07-20 at 22:42 +0200, Takashi Iwai wrote: > > > > > > On Fri, 19 Jul 2019 20:07:46 +0200, > > > > > > Takashi Iwai wrote: > > > > > > > On Fri, 19 Jul 2019 18:36:53 +0200, > > > > > > > Luciano Coelho wrote: > > > > > > > > Adding Dor. > > > > > > > > > > > > > > > > Hi Takashi, > > > > > > > > > > > > > > > > Do you have full logs of the crash? We can't see much from the > > > > > > > > log > > > > > > > > snippet pasted in the bug report. > > > > > > > > > > > > > > OK, I'll ask reporters. If you have a SUSE/openSUSE bugzilla > > > > > > > account, > > > > > > > feel free to join there. > > > > > > > > > > > > FYI, the dmesg's have been uploaded to the same bugzilla entry: > > > > > > https://bugzilla.opensuse.org/show_bug.cgi?id=1142128 > > > > > > > > > > > > > > > > Thanks! > > > > > > > > > > BTW, I pushed new firmwares to our firmware tree in git.kernel.org > > > > > today. This is the patch: > > > > > > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/iwlwifi/linux-firmware.git/commit/?id=b5f09bb4f816abace0227d0f4e749859364cef6b > > > > > > > > > > It would be great if you can try it out and let us know whether the > > > > > problem is gone or not. > > > > > > > > I created a test package and asked for testing. > > > > The test result seems negative, showing the same error, > > > > unfortunately. > > > > > > > > The dmesg was uploaded on the bugzilla entry. > > > > > > Thanks Takashi! We will look into them as soon as possible (sorry for > > > the late reply, I just came back from vacations). > > > > Actually, I just noticed that your bugzilla is closed as "RESOLVED > > FIXED". Is this still an issue? > > It's "closed" because our package contains the revert. > > > This seems like a mismatch between the WiFi and BT firmwares... And > > most likely the same issue as this: > > > > https://bugzilla.kernel.org/show_bug.cgi?id=202163 > > OK, so we need the update of the whole linux-firmware. > I'll refresh the package and ask for testing. I'm not sure the new BT firmware is in linux-firmware yet, since I don't handle BT stuff. I hope it is. -- Cheers, Luca.
Re: Regression with the latest iwlwifi-9260-*-46.ucode
On Mon, 2019-08-05 at 12:48 +0300, Luca Coelho wrote: > On Sun, 2019-07-21 at 18:43 +0200, Takashi Iwai wrote: > > On Sat, 20 Jul 2019 22:49:33 +0200, > > Luca Coelho wrote: > > > On Sat, 2019-07-20 at 22:42 +0200, Takashi Iwai wrote: > > > > On Fri, 19 Jul 2019 20:07:46 +0200, > > > > Takashi Iwai wrote: > > > > > On Fri, 19 Jul 2019 18:36:53 +0200, > > > > > Luciano Coelho wrote: > > > > > > Adding Dor. > > > > > > > > > > > > Hi Takashi, > > > > > > > > > > > > Do you have full logs of the crash? We can't see much from the log > > > > > > snippet pasted in the bug report. > > > > > > > > > > OK, I'll ask reporters. If you have a SUSE/openSUSE bugzilla account, > > > > > feel free to join there. > > > > > > > > FYI, the dmesg's have been uploaded to the same bugzilla entry: > > > > https://bugzilla.opensuse.org/show_bug.cgi?id=1142128 > > > > > > > > > > Thanks! > > > > > > BTW, I pushed new firmwares to our firmware tree in git.kernel.org > > > today. This is the patch: > > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/iwlwifi/linux-firmware.git/commit/?id=b5f09bb4f816abace0227d0f4e749859364cef6b > > > > > > It would be great if you can try it out and let us know whether the > > > problem is gone or not. > > > > I created a test package and asked for testing. > > The test result seems negative, showing the same error, > > unfortunately. > > > > The dmesg was uploaded on the bugzilla entry. > > Thanks Takashi! We will look into them as soon as possible (sorry for > the late reply, I just came back from vacations). Actually, I just noticed that your bugzilla is closed as "RESOLVED FIXED". Is this still an issue? This seems like a mismatch between the WiFi and BT firmwares... And most likely the same issue as this: https://bugzilla.kernel.org/show_bug.cgi?id=202163 -- Cheers, Luca.
Re: Regression with the latest iwlwifi-9260-*-46.ucode
On Sun, 2019-07-21 at 18:43 +0200, Takashi Iwai wrote: > On Sat, 20 Jul 2019 22:49:33 +0200, > Luca Coelho wrote: > > On Sat, 2019-07-20 at 22:42 +0200, Takashi Iwai wrote: > > > On Fri, 19 Jul 2019 20:07:46 +0200, > > > Takashi Iwai wrote: > > > > On Fri, 19 Jul 2019 18:36:53 +0200, > > > > Luciano Coelho wrote: > > > > > Adding Dor. > > > > > > > > > > Hi Takashi, > > > > > > > > > > Do you have full logs of the crash? We can't see much from the log > > > > > snippet pasted in the bug report. > > > > > > > > OK, I'll ask reporters. If you have a SUSE/openSUSE bugzilla account, > > > > feel free to join there. > > > > > > FYI, the dmesg's have been uploaded to the same bugzilla entry: > > > https://bugzilla.opensuse.org/show_bug.cgi?id=1142128 > > > > > > > Thanks! > > > > BTW, I pushed new firmwares to our firmware tree in git.kernel.org > > today. This is the patch: > > > > https://git.kernel.org/pub/scm/linux/kernel/git/iwlwifi/linux-firmware.git/commit/?id=b5f09bb4f816abace0227d0f4e749859364cef6b > > > > It would be great if you can try it out and let us know whether the problem > > is gone or not. > > I created a test package and asked for testing. > The test result seems negative, showing the same error, > unfortunately. > > The dmesg was uploaded on the bugzilla entry. Thanks Takashi! We will look into them as soon as possible (sorry for the late reply, I just came back from vacations). -- Cheers, Luca.
Re: Regression with the latest iwlwifi-9260-*-46.ucode
On Sat, 2019-07-20 at 22:42 +0200, Takashi Iwai wrote: > On Fri, 19 Jul 2019 20:07:46 +0200, > Takashi Iwai wrote: > > On Fri, 19 Jul 2019 18:36:53 +0200, > > Luciano Coelho wrote: > > > Adding Dor. > > > > > > Hi Takashi, > > > > > > Do you have full logs of the crash? We can't see much from the log > > > snippet pasted in the bug report. > > > > OK, I'll ask reporters. If you have a SUSE/openSUSE bugzilla account, > > feel free to join there. > > FYI, the dmesg's have been uploaded to the same bugzilla entry: > https://bugzilla.opensuse.org/show_bug.cgi?id=1142128 > Thanks! BTW, I pushed new firmwares to our firmware tree in git.kernel.org today. This is the patch: https://git.kernel.org/pub/scm/linux/kernel/git/iwlwifi/linux-firmware.git/commit/?id=b5f09bb4f816abace0227d0f4e749859364cef6b It would be great if you can try it out and let us know whether the problem is gone or not. -- Cheers, Luca.
Re: [PATCH -next] iwlwifi: dbg: work around clang bug by marking debug strings static
On Tue, 2019-07-16 at 10:28 -0700, Nick Desaulniers wrote:
> On Thu, Jul 11, 2019 at 7:15 PM Joe Perches wrote:
> > On Thu, 2019-07-11 at 17:17 -0700, Nick Desaulniers wrote:
> > > Commit r353569 in prerelease Clang-9 is producing a linkage failure:
> > >
> > > ld: drivers/net/wireless/intel/iwlwifi/fw/dbg.o:
> > > in function `_iwl_fw_dbg_apply_point':
> > > dbg.c:(.text+0x827a): undefined reference to `__compiletime_assert_2387'
> > >
> > > when the following configs are enabled:
> > > - CONFIG_IWLWIFI
> > > - CONFIG_IWLMVM
> > > - CONFIG_KASAN
> > >
> > > Work around the issue for now by marking the debug strings as `static`,
> > > which they probably should be any ways.
> > >
> > > Link: https://bugs.llvm.org/show_bug.cgi?id=42580
> > > Link: https://github.com/ClangBuiltLinux/linux/issues/580
> > > Reported-by: Arnd Bergmann
> > > Reported-by: Nathan Chancellor
> > > Signed-off-by: Nick Desaulniers
> > > ---
> > > drivers/net/wireless/intel/iwlwifi/fw/dbg.c | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/net/wireless/intel/iwlwifi/fw/dbg.c
> > > b/drivers/net/wireless/intel/iwlwifi/fw/dbg.c
> > > index e411ac98290d..f8c90ea4e9b4 100644
> > > --- a/drivers/net/wireless/intel/iwlwifi/fw/dbg.c
> > > +++ b/drivers/net/wireless/intel/iwlwifi/fw/dbg.c
> > > @@ -2438,7 +2438,7 @@ static void iwl_fw_dbg_info_apply(struct
> > > iwl_fw_runtime *fwrt,
> > > {
> > > u32 img_name_len = le32_to_cpu(dbg_info->img_name_len);
> > > u32 dbg_cfg_name_len = le32_to_cpu(dbg_info->dbg_cfg_name_len);
> > > - const char err_str[] =
> > > + static const char err_str[] =
> > > "WRT: ext=%d. Invalid %s name length %d, expected %d\n";
> >
> > Better still would be to use the format string directly
> > in both locations instead of trying to deduplicate it
> > via storing it into a separate pointer.
> >
> > Let the compiler/linker consolidate the format.
> > It's smaller object code, allows format/argument verification,
> > and is simpler for humans to understand.
>
> Whichever Kalle prefers, I just want my CI green again.
We already changed this in a later internal patch, which will reach the
mainline (-next) soon. So let's skip this for now.
--
Cheers,
Luca.
Re: [PATCH] iwlwifi: fix warning iwl-trans.h is included more than once
On Sun, 2019-05-26 at 17:08 +0530, Hariprasad Kelam wrote: > remove duplication include of iwl-trans.h > > issue identified by includecheck > > Signed-off-by: Hariprasad Kelam > --- Thanks! I have applied this (with small modifications to the commit message) in our internal tree and it will reach the mainline following our normal upstreaming process. -- Cheers, Luca.
Re: [PATCH][next] iwlwifi: mvm: fix comparison of u32 variable with less than zero
On Mon, 2019-07-01 at 17:26 +0100, Colin King wrote:
> From: Colin Ian King
>
> The comparison of the u32 variable wgds_tbl_idx with less than zero is
> always going to be false because it is unsigned. Fix this by making
> wgds_tbl_idx a plain signed int.
>
> Addresses-Coverity: ("Unsigned compared against 0")
> Fixes: 4fd445a2c855 ("iwlwifi: mvm: Add log information about SAR status")
> Signed-off-by: Colin Ian King
> ---
> drivers/net/wireless/intel/iwlwifi/mvm/nvm.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/nvm.c
> b/drivers/net/wireless/intel/iwlwifi/mvm/nvm.c
> index 719f793b3487..a9bb43a2f27b 100644
> --- a/drivers/net/wireless/intel/iwlwifi/mvm/nvm.c
> +++ b/drivers/net/wireless/intel/iwlwifi/mvm/nvm.c
> @@ -620,7 +620,7 @@ void iwl_mvm_rx_chub_update_mcc(struct iwl_mvm *mvm,
> enum iwl_mcc_source src;
> char mcc[3];
> struct ieee80211_regdomain *regd;
> - u32 wgds_tbl_idx;
> + int wgds_tbl_idx;
>
> lockdep_assert_held(&mvm->mutex);
Thanks, Colin!
I applied this to our internal tree and it will reach the mainline
following our normal upstreaming process.
--
Cheers,
Luca.
[PATCH] iwlwifi: don't panic in error path on non-msix systems
From: Shahar S Matityahu
The driver uses msix causes-register to handle both msix and non msix
interrupts when performing sync nmi. On devices that do not support
msix this register is unmapped and accessing it causes a kernel panic.
Solve this by differentiating the two cases and accessing the proper
causes-register in each case.
Reported-by: Michal Hocko
Signed-off-by: Shahar S Matityahu
Signed-off-by: Luca Coelho
---
.../net/wireless/intel/iwlwifi/pcie/trans.c | 19 +--
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
index 79c1dc05f948..c4375b868901 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
@@ -3644,20 +3644,27 @@ struct iwl_trans *iwl_trans_pcie_alloc(struct pci_dev
*pdev,
void iwl_trans_pcie_sync_nmi(struct iwl_trans *trans)
{
+ struct iwl_trans_pcie *trans_pcie = IWL_TRANS_GET_PCIE_TRANS(trans);
unsigned long timeout = jiffies + IWL_TRANS_NMI_TIMEOUT;
+ u32 inta_addr, sw_err_bit;
+
+ if (trans_pcie->msix_enabled) {
+ inta_addr = CSR_MSIX_HW_INT_CAUSES_AD;
+ sw_err_bit = MSIX_HW_INT_CAUSES_REG_SW_ERR;
+ } else {
+ inta_addr = CSR_INT;
+ sw_err_bit = CSR_INT_BIT_SW_ERR;
+ }
iwl_disable_interrupts(trans);
iwl_force_nmi(trans);
while (time_after(timeout, jiffies)) {
- u32 inta_hw = iwl_read32(trans,
-CSR_MSIX_HW_INT_CAUSES_AD);
+ u32 inta_hw = iwl_read32(trans, inta_addr);
/* Error detected by uCode */
- if (inta_hw & MSIX_HW_INT_CAUSES_REG_SW_ERR) {
+ if (inta_hw & sw_err_bit) {
/* Clear causes register */
- iwl_write32(trans, CSR_MSIX_HW_INT_CAUSES_AD,
- inta_hw &
- MSIX_HW_INT_CAUSES_REG_SW_ERR);
+ iwl_write32(trans, inta_addr, inta_hw & sw_err_bit);
break;
}
--
2.20.1
Re: [PATCH] iwlwifi: fix 64-bit division
On Tue, 2019-03-05 at 13:11 +0200, Kalle Valo wrote:
> Arnd Bergmann writes:
>
> > do_div() expects unsigned operands and otherwise triggers a warning
> > like:
> >
> > drivers/net/wireless/intel/iwlwifi/mvm/ftm-initiator.c:465:2:
> > error: comparison of distinct pointer types ('typeof ((rtt_avg)) *'
> > (aka 'long long *') and 'uint64_t *' (aka 'unsigned long long *'))
> > [-Werror,-Wcompare-distinct-pointer-types]
> > do_div(rtt_avg, );
> > ^
> > include/asm-generic/div64.h:222:28: note: expanded from macro
> > 'do_div'
> > (void)(((typeof((n)) *)0) == ((uint64_t *)0)); \
> >~~ ^ ~~~
> > 1 error generated.
> >
> > Change the do_div() to the simpler div_s64() that can handle
> > negative inputs correctly.
> >
> > Fixes: 937b10c0de68 ("iwlwifi: mvm: add debug prints for FTM")
> > Signed-off-by: Arnd Bergmann
>
> Luca, can I take this directly?
Yeah, I guess to make things simpler, and since you're planning to send
fixes to 5.1 already anyway, you can just take this one. I'll assign
it to you in patchwork.
Arnd, this way you'll get it earlier. ;)
--
Cheers,
Luca.
Re: [PATCH] iwlwifi: mvm: Use div64_s64 instead of do_div in iwl_mvm_debug_range_resp
On Tue, 2019-02-19 at 11:05 -0800, Nick Desaulniers wrote:
> On Tue, Feb 19, 2019 at 10:21 AM Nathan Chancellor
> wrote:
> > Clang warns:
> >
> > drivers/net/wireless/intel/iwlwifi/mvm/ftm-initiator.c:465:2:
> > warning:
> > comparison of distinct pointer types ('typeof ((rtt_avg)) *' (aka
> > 'long
> > long *') and 'uint64_t *' (aka 'unsigned long long *'))
> > [-Wcompare-distinct-pointer-types]
> > do_div(rtt_avg, );
> > ^
> > include/asm-generic/div64.h:222:28: note: expanded from macro
> > 'do_div'
> > (void)(((typeof((n)) *)0) == ((uint64_t *)0)); \
> >~~ ^ ~~~
> > 1 warning generated.
> >
> > do_div expects an unsigned dividend. Use div64_s64, which expects a
> > signed dividend.
>
> Eh, IIRC, signed vs unsigned division has implications for rounding
> towards zero or not, but I doubt that the round trip time average
> (RTT
> avg) should ever be negative. General rule of thumb for C is to keep
> arithmetic signed (even when working with non zero values), so rather
> than make the literal () a unsigned long, I agree with your
> change
> to keep the division signed as well. Thanks for the fix.
> Reviewed-by: Nick Desaulniers
Thanks for the patch and for the review. I've applied this to our
internal tree and it will be sent upstreaming following our normal
upstreaming process.
--
Cheers,
Luca.
Re: linux-next: build warning after merge of the wireless-drivers-next tree
On Thu, 2019-01-31 at 10:46 +1100, Stephen Rothwell wrote:
> Hi all,
>
> After merging the wireless-drivers-next tree, today's linux-next
> build
> (x86_64 allmodconfig) produced this warning:
>
> drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c:195:13: warning:
> 'iwl_mvm_add_rtap_sniffer_config' defined but not used [-Wunused-
> function]
> static void iwl_mvm_add_rtap_sniffer_config(struct iwl_mvm *mvm,
> ^~~
>
> Introduced by commit
>
> 9bf13bee2d74 ("iwlwifi: mvm: include configured sniffer AID in
> radiotap")
This was a conflict resolution damage in one of the patches I applied.
We already have a fix for it[1] and Kalle will apply it to wireless-
drivers-next soon.
Sorry for the trouble, but somehow I didn't see this warning and
kbuildbot also reported successful compilation with it. :(
[1] https://patchwork.kernel.org/patch/10788503/
--
Cheers,
Luca.
Re: [PATCH] net: wireless: prefix header search paths with $(srctree)/
On Mon, 2019-01-28 at 10:21 +0200, Kalle Valo wrote:
> Masahiro Yamada writes:
>
> > Currently, the Kbuild core manipulates header search paths in a
> > crazy
> > way [1].
> >
> > To fix this mess, I want all Makefiles to add explicit $(srctree)/
> > to
> > the search paths in the srctree. Some Makefiles are already written
> > in
> > that way, but not all. The goal of this work is to make the
> > notation
> > consistent, and finally get rid of the gross hacks.
> >
> > Having whitespaces after -I does not matter since commit
> > 48f6e3cf5bc6
> > ("kbuild: do not drop -I without parameter").
> >
> > I also removed one header search path in:
> >
> > drivers/net/wireless/broadcom/brcm80211/brcmutil/Makefile
> >
> > I was able to compile without it.
> >
> > [1]: https://patchwork.kernel.org/patch/9632347/
> >
> > Signed-off-by: Masahiro Yamada
> > ---
> >
> > drivers/net/wireless/broadcom/brcm80211/brcmfmac/Makefile | 4 ++--
> > drivers/net/wireless/broadcom/brcm80211/brcmsmac/Makefile | 6 +++-
> > --
> > drivers/net/wireless/broadcom/brcm80211/brcmutil/Makefile | 4 +---
> > drivers/net/wireless/intel/iwlwifi/dvm/Makefile | 2 +-
> > drivers/net/wireless/intel/iwlwifi/mvm/Makefile | 2 +-
> > drivers/net/wireless/realtek/rtl818x/rtl8180/Makefile | 2 +-
> > drivers/net/wireless/realtek/rtl818x/rtl8187/Makefile | 2 +-
> > 7 files changed, 10 insertions(+), 12 deletions(-)
>
> Luca, is it ok if I take this to wireless-drivers-next even though it
> touches iwlwifi makefiles?
Sure, it's much easier like that.
Acked-by: Luca Coelho
--
Cheers,
Luca.
Re: [PATCH] iwlwifi: fix false-positive maybe-uninitialized warning
On Mon, 2018-12-10 at 21:39 +0100, Arnd Bergmann wrote: > With CONFIG_NO_AUTO_INLINE, we run into a silly warning when > gcc fails to remember that n_profiles is constant across > the function call to iwl_mvm_sar_set_profile: > > drivers/net/wireless/intel/iwlwifi/mvm/fw.c: In function > 'iwl_mvm_sar_get_ewrd_table': > drivers/net/wireless/intel/iwlwifi/mvm/fw.c:746:9: error: 'ret' may > be used uninitialized in this function [-Werror=maybe-uninitialized] > > Marking that function 'inline' avoids the warning. > > Signed-off-by: Arnd Bergmann > --- Thanks! This has been applied in our internal tree and will reach the mainline following our normal upstreaming process. -- Cheers, Luca.
Re: linux-next: build warnings after merge of the wireless-drivers-next tree
On Wed, 2018-12-19 at 08:31 +, Grumbach, Emmanuel wrote:
> > Stephen Rothwell writes:
> >
> > > On Fri, 30 Nov 2018 12:05:55 +1100 Stephen Rothwell
> > wrote:
> > > > After merging the wireless-drivers-next tree, today's linux-next
> > > > build
> > > > (x86_64 allmodconfig) produced these warnings:
> > > >
> > > > drivers/net/wireless/intel/iwlwifi/iwl-drv.c: In function
> > 'iwl_parse_tlv_firmware':
> > > > drivers/net/wireless/intel/iwlwifi/iwl-drv.c:1098:7: warning: this
> > statement may fall through [-Wimplicit-fallthrough=]
> > > > if (iwlwifi_mod_params.enable_ini)
> > > >^
> > > > drivers/net/wireless/intel/iwlwifi/iwl-drv.c:1100:3: note: here
> > > >default:
> > > >^~~
> > > > drivers/net/wireless/intel/iwlwifi/iwl-dbg-tlv.c: In function
> > 'iwl_parse_fw_dbg_tlv':
> > > > drivers/net/wireless/intel/iwlwifi/iwl-dbg-tlv.c:203:4: warning: this
> > statement may fall through [-Wimplicit-fallthrough=]
> > > > iwl_fw_dbg_copy_tlv(trans, tlv, true);
> > > > ^
> > > > drivers/net/wireless/intel/iwlwifi/iwl-dbg-tlv.c:204:3: note: here
> > > >default:
> > > >^~~
> > > >
> > > > Introduced by commit
> > > >
> > > > f14cda6f3b31 ("iwlwifi: trans: parse and store debug ini TLVs")
> > > >
> > > > These are noted because I use -Wimplict-fallthrough
> > > >
> > > > The warnings can be suppressed by adding a comment like
> > > > /* fall through */
> > > > at the appropriate place to indicate that the fallthough is intended.
> > >
> > > I am still seeing these warnings (but in the net-next tree now) and I
> > > do not see a fix patch in the wireless-drivers-next tree.
> >
> > Luca did submit a patch[1] for cfg80211 to fix those but I don't see any
> > patches for iwlwifi (even in the one pending pull request he sent), not sure
> > what happened. I know that Luca is already on holidays but adding
> > Emmanuel, maybe he can help here?
> >
>
> Just sent a patch.
Those patches were still queued for upstreaming and I didn't think they
were that urgent, so I didn't pick them.
--
Luca.
Re: [PATCH 07/33] iwlwifi: mvm: use match_string() helper
On Mon, 2018-05-21 at 19:57 +0800, Yisheng Xie wrote:
> match_string() returns the index of an array for a matching string,
> which can be used intead of open coded variant.
>
> Cc: Kalle Valo
> Cc: Intel Linux Wireless
> Cc: Johannes Berg
> Cc: Emmanuel Grumbach
> Cc: linux-wirel...@vger.kernel.org
> Cc: net...@vger.kernel.org
> Signed-off-by: Yisheng Xie
> ---
> drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c | 11 +++
> 1 file changed, 3 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
> b/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
> index 0e6401c..e8249a6 100644
> --- a/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
> +++ b/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
> @@ -671,14 +671,9 @@ static ssize_t iwl_dbgfs_bt_cmd_read(struct file
> *file, char __user *user_buf,
> };
> int ret, bt_force_ant_mode;
>
> - for (bt_force_ant_mode = 0;
> - bt_force_ant_mode < ARRAY_SIZE(modes_str);
> - bt_force_ant_mode++) {
> - if (!strcmp(buf, modes_str[bt_force_ant_mode]))
> - break;
> - }
> -
> - if (bt_force_ant_mode >= ARRAY_SIZE(modes_str))
> + bt_force_ant_mode = match_string(modes_str,
> + ARRAY_SIZE(modes_str),
> buf);
> + if (bt_force_ant_mode < 0)
> return -EINVAL;
>
> ret = 0;
Looks fine, I'll push this to our internal tree for review and take a
closer look at what the match_string() function does exactly.
Thanks for the patch.
--
Cheers,
Luca.
Re: [PATCH][next] iwlwifi: mvm: remove division by size of sizeof(struct ieee80211_wmm_rule)
On Wed, 2018-04-11 at 14:05 +0100, Colin King wrote:
> From: Colin Ian King
>
> The subtraction of two struct ieee80211_wmm_rule pointers leaves a
> result
> that is automatically scaled down by the size of the size of pointed-
> to
> type, hence the division by sizeof(struct ieee80211_wmm_rule) is
> bogus and should be removed.
>
> Detected by CoverityScan, CID#146 ("Extra sizeof expression")
>
> Fixes: 77e30e10ee28 ("iwlwifi: mvm: query regdb for wmm rule if
> needed")
> Signed-off-by: Colin Ian King
> ---
Thanks, Colin! I've pushed this to our internal tree for review and if
everything goes fine it will land upstream following our normal
upstreaming process.
--
Cheers,
Luca.
Re: UBSAN: Undefined behaviour in drivers/net/wireless/intel/iwlwifi/mvm/utils.c:838:5
On Thu, 2017-12-21 at 11:49 +0100, Paul Menzel wrote: > Dear Kalle, > > > On 12/21/17 11:38, Kalle Valo wrote: > > Paul Menzel writes: > > > > > > http://pastebin.coelho.fi/7b624f474846da52.txt > > > > > > Thank you. The warning is gone now. Thank you. For the next time, > > > it’d > > > be great to provide the output of `git format-patch -1`, which > > > can be > > > applied with `git am`. The output of `git show` is with my > > > knowledge > > > hard to apply. > > > > 'patch -p1 < foo.patch' should work. Of course it omits the commit > > log > > but is usually enough for testing purposes. > > That’s what I did in the end. And then I have to do `git commit -a` > to > get the tree in a clean state. > > So `git format-patch -1` and uploading of that, for example with > some > CLI tool, for the win. ;-) The choice here is: more work for me vs. more work for you. :) But I guess I can easily do "git format-patch -1 --stdout" and pipe it to my pastebin script... -- Luca.
Re: UBSAN: Undefined behaviour in drivers/net/wireless/intel/iwlwifi/mvm/utils.c:838:5
On Wed, 2017-12-20 at 12:00 +0100, Paul Menzel wrote: > Dear Luca, > > > Am 18.12.2017 um 19:30 schrieb Luca Coelho: > > On Wed, 2017-12-13 at 16:32 +0200, Luciano Coelho wrote: > > > On Wed, 2017-12-13 at 14:25 +0100, Paul Menzel wrote: > > > > I enabled the undefined behavior sanitizer, and built Linus’ > > > > master branch under Ubuntu 17.10 with gcc (Ubuntu 7.2.0- > > > > 8ubuntu3) > > > > 7.2.0. > > > > > > > > ``` $ grep UBSAN /boot/config-4.15.0-rc3+ > > > > CONFIG_ARCH_HAS_UBSAN_SANITIZE_ALL=y # > > > > CONFIG_ARCH_WANTS_UBSAN_NO_NULL is not set CONFIG_UBSAN=y > > > > CONFIG_UBSAN_SANITIZE_ALL=y # CONFIG_UBSAN_ALIGNMENT is not > > > > set > > > > CONFIG_UBSAN_NULL=y ``` > > > > > > > > Starting the system the messages below are printed. > > > > > > > > Starting the system and using the wireless device shows the > > > > messages below. > > > > > > Thanks for reporting! This shouldn't cause any problems, but I'll > > > fix it by checking that the mac80211_queue is not INVALID_QUEUE > > > (255) which seems to be the trigger for this warning. > > > > Can you try the following patch to see if the problem goes away? > > > > http://pastebin.coelho.fi/7b624f474846da52.txt > > Thank you. The warning is gone now. Thank you. Thanks for testing! We are actually improving the fix a bit, because it seems that we should not have gotten INVALID_QUEUE there. We will have a two-fold fix (Sari, whom I CCed, is working on the other patch). > For the next time, it’d > be great to provide the output of `git format-patch -1`, which can > be > applied with `git am`. The output of `git show` is with my knowledge > hard to apply. Ah, sorry about that. I didn't want to send a full patch here so that it wouldn't get picked up by patchworks, so I just pastebinned it. -- Luca.
Re: UBSAN: Undefined behaviour in drivers/net/wireless/intel/iwlwifi/mvm/utils.c:838:5
On Wed, 2017-12-13 at 16:32 +0200, Luciano Coelho wrote: > On Wed, 2017-12-13 at 14:25 +0100, Paul Menzel wrote: > > Dear Linux folks, > > > > > > I enabled the undefined behavior sanitizer, and built Linus’ > > master > > branch under Ubuntu 17.10 with gcc (Ubuntu 7.2.0-8ubuntu3) 7.2.0. > > > > ``` > > $ grep UBSAN /boot/config-4.15.0-rc3+ > > CONFIG_ARCH_HAS_UBSAN_SANITIZE_ALL=y > > # CONFIG_ARCH_WANTS_UBSAN_NO_NULL is not set > > CONFIG_UBSAN=y > > CONFIG_UBSAN_SANITIZE_ALL=y > > # CONFIG_UBSAN_ALIGNMENT is not set > > CONFIG_UBSAN_NULL=y > > ``` > > > > Starting the system the messages below are printed. > > > > Starting the system and using the wireless device shows the > > messages > > below. > > Thanks for reporting! This shouldn't cause any problems, but I'll fix > it by checking that the mac80211_queue is not INVALID_QUEUE (255) > which seems to be the trigger for this warning. Can you try the following patch to see if the problem goes away? http://pastebin.coelho.fi/7b624f474846da52.txt Thanks! -- Cheers, Luca.
Re: [PATCH] drivers/wireless: iwlwifi/mvm: Convert timers to use timer_setup()
On Tue, 2017-10-24 at 02:29 -0700, Kees Cook wrote: > In preparation for unconditionally passing the struct timer_list > pointer to > all timer callbacks, switch to using the new timer_setup() and > from_timer() > to pass the timer pointer explicitly. > > The RCU lifetime on baid_data is unclear, so this adds a direct copy > of the > rcu_ptr passed to the original callback. It may be possible to > improve this > to just use baid_data->mvm->baid_map[baid_data->baid] instead. > > Cc: Johannes Berg > Cc: Emmanuel Grumbach > Cc: Luca Coelho > Cc: Intel Linux Wireless > Cc: Kalle Valo > Cc: Sara Sharon > Cc: linux-wirel...@vger.kernel.org > Cc: net...@vger.kernel.org > Signed-off-by: Kees Cook > --- Thanks, Kees. I'm taking this for review on our internal tree. If all our checks pass, I'll apply it and it will reach the mainline following our usual upstreaming process. -- Cheers, Luca.
Re: linux-next: manual merge of the wireless-drivers-next tree with the wireless-drivers tree
On Thu, 2017-10-12 at 19:59 +0100, Mark Brown wrote: > On Thu, Oct 12, 2017 at 09:50:51PM +0300, Luca Coelho wrote: > > On Thu, 2017-10-12 at 19:35 +0100, Mark Brown wrote: > > > With trees like this that don't coordinate with their fixes > > > branch > > > there > > > are frequently multiple conflicts introduced so I generally > > > report > > > things file by file without even looking at the new ones. > > Sorry for the trouble. But how do you suggest that we "coordinate > > our > > fixes branch"? Merge fixes into the main tree? > > That'd be easiest for me! It's not of necessity a problem if the > conflicts are easy enough to resolve if you just let things get > merged > in -next, it's just more an observation that that's a thing that > happens > and that this is how I cope with it. Stephen may do things a bit > differently. Cool, I'll discuss this with Kalle and make sure I note these potential conflicts early enough so everyone is aware when they're coming. Thanks for taking over while Stephen is away! I really appreciate it, since linux-next is a very important part of our process. :) -- Luca.
Re: linux-next: manual merge of the wireless-drivers-next tree with the wireless-drivers tree
On Thu, 2017-10-12 at 19:35 +0100, Mark Brown wrote: > On Thu, Oct 12, 2017 at 09:27:46PM +0300, Luciano Coelho wrote: > > On Thu, 2017-10-12 at 19:21 +0100, Mark Brown wrote: > > > I may have confused the trees when I was pasting things in, the > > > commits > > > are filled in by hand. > > > > Ah, okay. But still, if the same patches conflicted twice, why > > wasn't > > there only one occurrence with both conflicts at once? > > With trees like this that don't coordinate with their fixes branch > there > are frequently multiple conflicts introduced so I generally report > things file by file without even looking at the new ones. Sorry for the trouble. But how do you suggest that we "coordinate our fixes branch"? Merge fixes into the main tree? -- Luca.
Re: linux-next: manual merge of the wireless-drivers-next tree with the wireless-drivers tree
On Thu, 2017-10-12 at 18:25 +0100, Mark Brown wrote:
> Hi all,
>
> Today's linux-next merge of the wireless-drivers-next tree got a
> conflict in:
>
> drivers/net/wireless/intel/iwlwifi/iwl-config.h
>
> between commit:
>
>dd05f9aab4426f ("iwlwifi: pcie: dynamic Tx command queue size")
>
> from the wireless-drivers tree and commit:
>
>44fd09dad5d2b7 ("iwlwifi: nvm: set the correct offsets to 3168
> series")
>
> from the wireless-drivers-next tree.
>
> I fixed it up (see below) and can carry the fix as necessary. This
> is now fixed as far as linux-next is concerned, but any non trivial
> conflicts should be mentioned to your upstream maintainer when your
> tree
> is submitted for merging. You may also want to consider cooperating
> with the maintainer of the conflicting tree to minimise any
> particularly
> complex conflicts.
>
> diff --cc drivers/net/wireless/intel/iwlwifi/iwl-config.h
> index 71cb1ecde0f7,b9f3b350fe34..
> --- a/drivers/net/wireless/intel/iwlwifi/iwl-config.h
> +++ b/drivers/net/wireless/intel/iwlwifi/iwl-config.h
> @@@ -332,7 -320,9 +332,9 @@@ struct iwl_pwr_tx_backoff
>* @integrated: discrete or integrated
>* @gen2: a000 and on transport operation
>* @cdb: CDB support
> - * @ext_nvm: extended NVM format
> + * @nvm_type: see &enum iwl_nvm_type
> + * @tx_cmd_queue_size: size of the cmd queue. If zero, use the same
> value as
> + * the regular queues
>*
>* We enable the driver to be backward compatible wrt. hardware
> features.
>* API differences in uCode shouldn't be handled here but through
> TLVs
> @@@ -382,7 -371,9 +384,8 @@@ struct iwl_cfg
> use_tfh:1,
> gen2:1,
> cdb:1,
> -ext_nvm:1,
nvm_type seems to be missing from here?
> dbgc_supported:1;
> + u16 tx_cmd_queue_size;
> u8 valid_tx_ant;
> u8 valid_rx_ant;
> u8 non_shared_ant;
--
Luca.
Re: 4.14.0-rc3 iwlwifi: Hardware became unavailable during restart
On Tue, 2017-10-10 at 09:44 +0200, Seraphime Kirkovski wrote: > Hello, Hi Seraphime, > I've got this splat after a couple of suspend-resume cycles on my > HP-laptop. I haven't had the time to bisect or test other rcs for now. > Pasting some logs before the actual WARN_ON, as they may be relevant. > Just after the splat the connection is successfully reestablished. > > [14293.758404] iwlwifi :24:00.0: Error sending REPLY_SCAN_ABORT_CMD: > time out after 2000ms. > [14293.758429] iwlwifi :24:00.0: Current CMD queue read_ptr 67 write_ptr > 68 > [14293.758518] iwlwifi :24:00.0: Loaded firmware version: 18.168.6.1 This seems to be a DVM device (iwldvm). What is the exact device model? And you have never noticed this problem before? Unfortunately this is a very old device and we don't support it actively anymore. Hopefully this will turn out to be a trivial fix in the driver side, so we can get it solved for you. The best way to track this is to report it in https://bugzilla.kernel.org. -- Cheers, Luca.
[PATCH] iwlwifi: mvm: only send LEDS_CMD when the FW supports it
From: Luca Coelho
The LEDS_CMD command is only supported in some newer FW versions
(e.g. iwlwifi-8000C-31.ucode), so we can't send it to older versions
(such as iwlwifi-8000C-27.ucode).
To fix this, check for a new bit in the FW capabilities TLV that tells
when the command is supported.
Note that the current version of -31.ucode in linux-firmware.git
(31.532993.0) does not have this capability bit set, so the LED won't
work, even though this version should support it. But we will update
this firmware soon, so it won't be a problem anymore.
Fixes: 7089ae634c50 ("iwlwifi: mvm: use firmware LED command where applicable")
Reported-by: Linus Torvalds
Signed-off-by: Luca Coelho
---
drivers/net/wireless/intel/iwlwifi/fw/file.h | 1 +
drivers/net/wireless/intel/iwlwifi/mvm/led.c | 3 ++-
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/intel/iwlwifi/fw/file.h
b/drivers/net/wireless/intel/iwlwifi/fw/file.h
index 887f6d8fc8a7..279248cd9cfb 100644
--- a/drivers/net/wireless/intel/iwlwifi/fw/file.h
+++ b/drivers/net/wireless/intel/iwlwifi/fw/file.h
@@ -378,6 +378,7 @@ enum iwl_ucode_tlv_capa {
IWL_UCODE_TLV_CAPA_EXTEND_SHARED_MEM_CFG= (__force
iwl_ucode_tlv_capa_t)80,
IWL_UCODE_TLV_CAPA_LQM_SUPPORT = (__force
iwl_ucode_tlv_capa_t)81,
IWL_UCODE_TLV_CAPA_TX_POWER_ACK = (__force
iwl_ucode_tlv_capa_t)84,
+ IWL_UCODE_TLV_CAPA_LED_CMD_SUPPORT = (__force
iwl_ucode_tlv_capa_t)86,
IWL_UCODE_TLV_CAPA_MLME_OFFLOAD = (__force
iwl_ucode_tlv_capa_t)96,
NUM_IWL_UCODE_TLV_CAPA
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/led.c
b/drivers/net/wireless/intel/iwlwifi/mvm/led.c
index 005e2e7278a5..b27269504a62 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/led.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/led.c
@@ -92,7 +92,8 @@ static void iwl_mvm_send_led_fw_cmd(struct iwl_mvm *mvm, bool
on)
static void iwl_mvm_led_set(struct iwl_mvm *mvm, bool on)
{
- if (mvm->cfg->device_family >= IWL_DEVICE_FAMILY_8000) {
+ if (fw_has_capa(&mvm->fw->ucode_capa,
+ IWL_UCODE_TLV_CAPA_LED_CMD_SUPPORT)) {
iwl_mvm_send_led_fw_cmd(mvm, on);
return;
}
--
2.14.1
Re: [GIT] Networking
On Thu, 2017-09-07 at 05:04 +, Coelho, Luciano wrote:
> On Wed, 2017-09-06 at 21:57 -0700, Linus Torvalds wrote:
> > On Wed, Sep 6, 2017 at 9:11 PM, Coelho, Luciano
> > wrote:
> > >
> > > This seems to be a problem with backwards-compatibility with FW version
> > > 27. We are now in version 31[1] and upgrading will probably fix that.
> >
> > I can confirm that fw version 31 works.
>
> Great, so I know for sure that this is a backwards-compatibility issue
> with the FW API.
>
>
> > > But obviously the driver should not fail miserably like this with
> > > version 27, because it claims to support it still.
> >
> > Not just "claims to support it", but if it's what is shipped with a
> > fairly recent distro like an up-to-date version of F26, I would really
> > hope that the driver can still work with it.
>
> I totally agree, we support a bunch of older versions for that exact
> reason. We just don't really test all the supported versions very
> often. We should probably change that.
>
> I'll make sure it still works with version 27.
Okay, I found the offending patch:
commit 7089ae634c50544b29b31faf1a751e8765c8de3b
Author: Johannes Berg
AuthorDate: Wed Jun 28 16:19:49 2017 +0200
Commit: Luca Coelho
CommitDate: Wed Aug 9 09:15:32 2017 +0300
iwlwifi: mvm: use firmware LED command where applicable
On devices starting from 8000 series, the host can no longer toggle
the LED through the CSR_LED_REG register, but must do it via the
firmware instead. Add support for this. Note that this means that
the LED cannot be turned on while the firmware is off, so using an
arbitrary LED trigger may not work as expected.
Fixes: 503ab8c56ca0 ("iwlwifi: Add 8000 HW family support")
Signed-off-by: Johannes Berg
Signed-off-by: Luca Coelho
Reverting it solves the problem. We introduced a new command to control
the LED lights and assumed it was available in older FW versions as
well, which turned out not to be the case.
This patch is not very important (unless you really like blinking lights
-- maybe I'll change my mind when the holidays approach :P). so it is
fine if you just want to revert it for now.
In any case, I'll send a patch fixing this problem soon.
--
Cheers,
Luca.
Re: [PATCH v2] iwlwifi: pcie: move rx workqueue initialization to iwl_trans_pcie_alloc()
On Mon, 2017-09-04 at 13:43 +0200, Jiri Kosina wrote: > On Thu, 24 Aug 2017, Luca Coelho wrote: > > > > looks like a correct fix to me, so feel free to add > > > > > > Acked-by: Jiri Kosina > > > > > > I'll be able to provide my Tested-by: eventually only in ~10 days. > > > > > > Kalle already picked it up in wireless-drivers and this should make it's > > way to 4.13-rc7 (we hope). > > > > In any case, thanks for reporting and the help debugging it. > > I know it's pretty late for this to be added to the commit, but I don't > want this to be left in the void, so for the sake of completness: > > Tested-by: Jiri Kosina Thanks, Jiri, for reporting, debugging and testing! -- Cheers, Luca.
Re: [PATCH v2] iwlwifi: pcie: move rx workqueue initialization to iwl_trans_pcie_alloc()
On Wed, 2017-08-30 at 17:57 +0300, David Weinehall wrote:
> On Tue, Aug 22, 2017 at 10:37:29AM +0300, Luca Coelho wrote:
> > From: Luca Coelho
> >
> > Work queues cannot be allocated when a mutex is held because the mutex
> > may be in use and that would make it sleep. Doing so generates the
> > following splat with 4.13+:
> >
> > [ 19.513298] ==
> > [ 19.513429] WARNING: possible circular locking dependency detected
> > [ 19.513557] 4.13.0-rc5+ #6 Not tainted
> > [ 19.513638] --
> > [ 19.513767] cpuhp/0/12 is trying to acquire lock:
> > [ 19.513867] (&tz->lock){+.+.+.}, at: []
> > thermal_zone_get_temp+0x5b/0xb0
> > [ 19.514047]
> > [ 19.514047] but task is already holding lock:
> > [ 19.514166] (cpuhp_state){+.+.+.}, at: []
> > cpuhp_thread_fun+0x3a/0x210
> > [ 19.514338]
> > [ 19.514338] which lock already depends on the new lock.
> >
> > This lock dependency already existed with previous kernel versions,
> > but it was not detected until commit 49dfe2a67797 ("cpuhotplug: Link
> > lock stacks for hotplug callbacks") was introduced.
> >
> > Reported-by: David Weinehall
> > Reported-by: Jiri Kosina
> > Signed-off-by: Luca Coelho
>
> With this patch I no longer get the lockdep warning,
> and the driver seems to work just as well as before.
Great! Thanks for reporting and testing, David!
> Thanks!
>
> Tested-by: David Weinehall
Thanks for this too, but unfortunately it's too late to add it, since
the patch is already in net tree.
--
Cheers,
Luca.
Re: [PATCH v2] iwlwifi: pcie: move rx workqueue initialization to iwl_trans_pcie_alloc()
On Thu, 2017-08-24 at 21:56 +0200, Jiri Kosina wrote:
> On Thu, 24 Aug 2017, Luca Coelho wrote:
>
> > > Work queues cannot be allocated when a mutex is held because the mutex
> > > may be in use and that would make it sleep. Doing so generates the
> > > following splat with 4.13+:
> > >
> > > [ 19.513298] ==
> > > [ 19.513429] WARNING: possible circular locking dependency detected
> > > [ 19.513557] 4.13.0-rc5+ #6 Not tainted
> > > [ 19.513638] --
> > > [ 19.513767] cpuhp/0/12 is trying to acquire lock:
> > > [ 19.513867] (&tz->lock){+.+.+.}, at: []
> > > thermal_zone_get_temp+0x5b/0xb0
> > > [ 19.514047]
> > > [ 19.514047] but task is already holding lock:
> > > [ 19.514166] (cpuhp_state){+.+.+.}, at: []
> > > cpuhp_thread_fun+0x3a/0x210
> > > [ 19.514338]
> > > [ 19.514338] which lock already depends on the new lock.
> > >
> > > This lock dependency already existed with previous kernel versions,
> > > but it was not detected until commit 49dfe2a67797 ("cpuhotplug: Link
> > > lock stacks for hotplug callbacks") was introduced.
> > >
> > > Reported-by: David Weinehall
> > > Reported-by: Jiri Kosina
> > > Signed-off-by: Luca Coelho
> >
> > Jiri, did you have a chance to try this out? I'm about to ask Kalle to
> > merge this so it gets in in time for 4.13-rc7.
>
> Sorry, I am almost completely offline for one more week (vacation), and
> will not have access to the affected system before that.
Sounds good! Enjoy! ;)
> But this indeed
> looks like a correct fix to me, so feel free to add
>
> Acked-by: Jiri Kosina
>
> I'll be able to provide my Tested-by: eventually only in ~10 days.
Kalle already picked it up in wireless-drivers and this should make it's
way to 4.13-rc7 (we hope).
In any case, thanks for reporting and the help debugging it.
--
Cheers,
Luca.
Re: [PATCH v2] iwlwifi: pcie: move rx workqueue initialization to iwl_trans_pcie_alloc()
On Tue, 2017-08-22 at 10:37 +0300, Luca Coelho wrote:
> From: Luca Coelho
>
> Work queues cannot be allocated when a mutex is held because the mutex
> may be in use and that would make it sleep. Doing so generates the
> following splat with 4.13+:
>
> [ 19.513298] ==
> [ 19.513429] WARNING: possible circular locking dependency detected
> [ 19.513557] 4.13.0-rc5+ #6 Not tainted
> [ 19.513638] --
> [ 19.513767] cpuhp/0/12 is trying to acquire lock:
> [ 19.513867] (&tz->lock){+.+.+.}, at: []
> thermal_zone_get_temp+0x5b/0xb0
> [ 19.514047]
> [ 19.514047] but task is already holding lock:
> [ 19.514166] (cpuhp_state){+.+.+.}, at: []
> cpuhp_thread_fun+0x3a/0x210
> [ 19.514338]
> [ 19.514338] which lock already depends on the new lock.
>
> This lock dependency already existed with previous kernel versions,
> but it was not detected until commit 49dfe2a67797 ("cpuhotplug: Link
> lock stacks for hotplug callbacks") was introduced.
>
> Reported-by: David Weinehall
> Reported-by: Jiri Kosina
> Signed-off-by: Luca Coelho
Jiri, did you have a chance to try this out? I'm about to ask Kalle to
merge this so it gets in in time for 4.13-rc7.
--
Cheers,
Luca.
[PATCH v2] iwlwifi: pcie: move rx workqueue initialization to iwl_trans_pcie_alloc()
From: Luca Coelho
Work queues cannot be allocated when a mutex is held because the mutex
may be in use and that would make it sleep. Doing so generates the
following splat with 4.13+:
[ 19.513298] ==
[ 19.513429] WARNING: possible circular locking dependency detected
[ 19.513557] 4.13.0-rc5+ #6 Not tainted
[ 19.513638] --
[ 19.513767] cpuhp/0/12 is trying to acquire lock:
[ 19.513867] (&tz->lock){+.+.+.}, at: []
thermal_zone_get_temp+0x5b/0xb0
[ 19.514047]
[ 19.514047] but task is already holding lock:
[ 19.514166] (cpuhp_state){+.+.+.}, at: []
cpuhp_thread_fun+0x3a/0x210
[ 19.514338]
[ 19.514338] which lock already depends on the new lock.
This lock dependency already existed with previous kernel versions,
but it was not detected until commit 49dfe2a67797 ("cpuhotplug: Link
lock stacks for hotplug callbacks") was introduced.
Reported-by: David Weinehall
Reported-by: Jiri Kosina
Signed-off-by: Luca Coelho
---
In v2:
- updated the commit message to a new version, with a grammar fix
and the actual commit that exposed the problem;
drivers/net/wireless/intel/iwlwifi/pcie/internal.h | 2 ++
drivers/net/wireless/intel/iwlwifi/pcie/rx.c | 10 +-
drivers/net/wireless/intel/iwlwifi/pcie/trans.c| 9 +
3 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/internal.h
b/drivers/net/wireless/intel/iwlwifi/pcie/internal.h
index fa315d84e98e..a1ea9ef97ed9 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/internal.h
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/internal.h
@@ -787,6 +787,8 @@ int iwl_pci_fw_enter_d0i3(struct iwl_trans *trans);
void iwl_pcie_enable_rx_wake(struct iwl_trans *trans, bool enable);
+void iwl_pcie_rx_allocator_work(struct work_struct *data);
+
/* common functions that are used by gen2 transport */
void iwl_pcie_apm_config(struct iwl_trans *trans);
int iwl_pcie_prepare_card_hw(struct iwl_trans *trans);
diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/rx.c
b/drivers/net/wireless/intel/iwlwifi/pcie/rx.c
index 351c4423125a..942736d3fa75 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/rx.c
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/rx.c
@@ -597,7 +597,7 @@ static void iwl_pcie_rx_allocator_get(struct iwl_trans
*trans,
rxq->free_count += RX_CLAIM_REQ_ALLOC;
}
-static void iwl_pcie_rx_allocator_work(struct work_struct *data)
+void iwl_pcie_rx_allocator_work(struct work_struct *data)
{
struct iwl_rb_allocator *rba_p =
container_of(data, struct iwl_rb_allocator, rx_alloc);
@@ -900,10 +900,6 @@ static int _iwl_pcie_rx_init(struct iwl_trans *trans)
return err;
}
def_rxq = trans_pcie->rxq;
- if (!rba->alloc_wq)
- rba->alloc_wq = alloc_workqueue("rb_allocator",
- WQ_HIGHPRI | WQ_UNBOUND, 1);
- INIT_WORK(&rba->rx_alloc, iwl_pcie_rx_allocator_work);
spin_lock(&rba->lock);
atomic_set(&rba->req_pending, 0);
@@ -1017,10 +1013,6 @@ void iwl_pcie_rx_free(struct iwl_trans *trans)
}
cancel_work_sync(&rba->rx_alloc);
- if (rba->alloc_wq) {
- destroy_workqueue(rba->alloc_wq);
- rba->alloc_wq = NULL;
- }
iwl_pcie_free_rbs_pool(trans);
diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
index f95eec52508e..3927bbf04f72 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
@@ -1786,6 +1786,11 @@ void iwl_trans_pcie_free(struct iwl_trans *trans)
iwl_pcie_tx_free(trans);
iwl_pcie_rx_free(trans);
+ if (trans_pcie->rba.alloc_wq) {
+ destroy_workqueue(trans_pcie->rba.alloc_wq);
+ trans_pcie->rba.alloc_wq = NULL;
+ }
+
if (trans_pcie->msix_enabled) {
for (i = 0; i < trans_pcie->alloc_vecs; i++) {
irq_set_affinity_hint(
@@ -3169,6 +3174,10 @@ struct iwl_trans *iwl_trans_pcie_alloc(struct pci_dev
*pdev,
trans_pcie->inta_mask = CSR_INI_SET_MASK;
}
+ trans_pcie->rba.alloc_wq = alloc_workqueue("rb_allocator",
+ WQ_HIGHPRI | WQ_UNBOUND, 1);
+ INIT_WORK(&trans_pcie->rba.rx_alloc, iwl_pcie_rx_allocator_work);
+
#ifdef CONFIG_IWLWIFI_PCIE_RTPM
trans->runtime_pm_mode = IWL_PLAT_PM_MODE_D0I3;
#else
--
2.14.1
[PATCH] iwlwifi: pcie: move rx workqueue initialization to iwl_trans_pcie_alloc()
From: Luca Coelho
Work queues cannot be allocated in when a mutex is held because the
mutex may be in use and that would make it sleep. Doing so generates
the following splat with 4.13+:
[ 19.513298] ==
[ 19.513429] WARNING: possible circular locking dependency detected
[ 19.513557] 4.13.0-rc5+ #6 Not tainted
[ 19.513638] --
[ 19.513767] cpuhp/0/12 is trying to acquire lock:
[ 19.513867] (&tz->lock){+.+.+.}, at: []
thermal_zone_get_temp+0x5b/0xb0
[ 19.514047]
[ 19.514047] but task is already holding lock:
[ 19.514166] (cpuhp_state){+.+.+.}, at: []
cpuhp_thread_fun+0x3a/0x210
[ 19.514338]
[ 19.514338] which lock already depends on the new lock.
This lock dependency already existed with previous kernel versions,
but it was not detected until commit ... was introduced.
Reported-by: David Weinehall
Reported-by: Jiri Kosina
Signed-off-by: Luca Coelho
---
drivers/net/wireless/intel/iwlwifi/pcie/internal.h | 2 ++
drivers/net/wireless/intel/iwlwifi/pcie/rx.c | 10 +-
drivers/net/wireless/intel/iwlwifi/pcie/trans.c| 9 +
3 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/internal.h
b/drivers/net/wireless/intel/iwlwifi/pcie/internal.h
index fa315d84e98e..a1ea9ef97ed9 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/internal.h
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/internal.h
@@ -787,6 +787,8 @@ int iwl_pci_fw_enter_d0i3(struct iwl_trans *trans);
void iwl_pcie_enable_rx_wake(struct iwl_trans *trans, bool enable);
+void iwl_pcie_rx_allocator_work(struct work_struct *data);
+
/* common functions that are used by gen2 transport */
void iwl_pcie_apm_config(struct iwl_trans *trans);
int iwl_pcie_prepare_card_hw(struct iwl_trans *trans);
diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/rx.c
b/drivers/net/wireless/intel/iwlwifi/pcie/rx.c
index 351c4423125a..942736d3fa75 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/rx.c
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/rx.c
@@ -597,7 +597,7 @@ static void iwl_pcie_rx_allocator_get(struct iwl_trans
*trans,
rxq->free_count += RX_CLAIM_REQ_ALLOC;
}
-static void iwl_pcie_rx_allocator_work(struct work_struct *data)
+void iwl_pcie_rx_allocator_work(struct work_struct *data)
{
struct iwl_rb_allocator *rba_p =
container_of(data, struct iwl_rb_allocator, rx_alloc);
@@ -900,10 +900,6 @@ static int _iwl_pcie_rx_init(struct iwl_trans *trans)
return err;
}
def_rxq = trans_pcie->rxq;
- if (!rba->alloc_wq)
- rba->alloc_wq = alloc_workqueue("rb_allocator",
- WQ_HIGHPRI | WQ_UNBOUND, 1);
- INIT_WORK(&rba->rx_alloc, iwl_pcie_rx_allocator_work);
spin_lock(&rba->lock);
atomic_set(&rba->req_pending, 0);
@@ -1017,10 +1013,6 @@ void iwl_pcie_rx_free(struct iwl_trans *trans)
}
cancel_work_sync(&rba->rx_alloc);
- if (rba->alloc_wq) {
- destroy_workqueue(rba->alloc_wq);
- rba->alloc_wq = NULL;
- }
iwl_pcie_free_rbs_pool(trans);
diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
index f95eec52508e..3927bbf04f72 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
@@ -1786,6 +1786,11 @@ void iwl_trans_pcie_free(struct iwl_trans *trans)
iwl_pcie_tx_free(trans);
iwl_pcie_rx_free(trans);
+ if (trans_pcie->rba.alloc_wq) {
+ destroy_workqueue(trans_pcie->rba.alloc_wq);
+ trans_pcie->rba.alloc_wq = NULL;
+ }
+
if (trans_pcie->msix_enabled) {
for (i = 0; i < trans_pcie->alloc_vecs; i++) {
irq_set_affinity_hint(
@@ -3169,6 +3174,10 @@ struct iwl_trans *iwl_trans_pcie_alloc(struct pci_dev
*pdev,
trans_pcie->inta_mask = CSR_INI_SET_MASK;
}
+ trans_pcie->rba.alloc_wq = alloc_workqueue("rb_allocator",
+ WQ_HIGHPRI | WQ_UNBOUND, 1);
+ INIT_WORK(&trans_pcie->rba.rx_alloc, iwl_pcie_rx_allocator_work);
+
#ifdef CONFIG_IWLWIFI_PCIE_RTPM
trans->runtime_pm_mode = IWL_PLAT_PM_MODE_D0I3;
#else
--
2.14.1
Re: [PATCH] iwlwifi: Demote messages about fw flags size to info
Hi João Paulo, On Tue, 2017-08-01 at 15:58 -0700, João Paulo Rechi Vita wrote: > Hello Luca, > > On Mon, Jul 24, 2017 at 4:01 AM, Coelho, Luciano > wrote: > > On Fri, 2017-07-21 at 07:51 -0700, João Paulo Rechi Vita wrote: > > (...) > > > > Currently these messages are presented to the user during boot if there > > > is no bootsplash covering the console, sometimes even if the boot splash > > > is enabled but has not started yet by the time this message is shown. > > > > > I should have provided another piece of information here: all of this > happens even when booting with the 'quiet' kernel parameter. Oh, okay, that's annoying. > > This specific case is harmless, but I'd rather keep this message as an > > error, because in other situations it could lead to unexpected > > behavioir, so I prefer to keep it very visible. > > > > > > I see your point, and I understand the purpose of these messages. I'm > wondering if perhaps having them at the warning level would give them > enough visibility, while still keeping a clean boot process to the end > user. If so, I can send an updated patch. > > Thanks for your reply and for pointing to the fix for the root cause > of that message. Sure, I agree it's better to make it use IWL_WARN(), which will generate a dev_warn() instead of a dev_err(). -- Cheers, Luca.
Re: [PATCH v2] iwlwifi: mvm: Fix a memory leak in an error handling path in 'iwl_mvm_sar_get_wgds_table()'
On Fri, 2017-07-14 at 12:06 +0200, Christophe JAILLET wrote:
> We should free 'wgds.pointer' here as done a few lines above in another
> error handling path.
> It was allocated within 'acpi_evaluate_object()'.
>
> Signed-off-by: Christophe JAILLET
> ---
> v2: rebase after 7fe90e0e3d60 ("iwlwifi: mvm: refactor geo init")
Thanks, Christophe!
I've pushed this to our internal tree and it will eventually reach the
mainline via our normal process.
> Moreovern a comment in '/drivers/acpi/acpica/utalloc.c' states that:
> /* [...] Note: The caller should use acpi_os_free to free this
> * buffer created via ACPI_ALLOCATE_BUFFER.
> */
>
> So, at the end of this function:
> out_free:
> kfree(wgds.pointer);
> should maybe be:
> out_free:
> acpi_os_free(wgds.pointer);
>
> If correct, several places should be fixed accordingly.
Thanks for pointing out! I'm about to do some refactoring in this code
and I'll make sure I check the proper way to free the acpi buffer when
doing so.
--
Cheers,
Luca.
Re: [PATCH] iwlwifi: mvm: add const to thermal_cooling_device_ops structure
On Wed, 2017-06-28 at 14:49 +0300, Luca Coelho wrote: > On Wed, 2017-06-21 at 14:10 +0530, Bhumika Goyal wrote: > > Declare thermal_cooling_device_ops structure as const as it is only passed > > as an argument to the function thermal_cooling_device_register and this > > argument is of type const. So, declare the structure as const. > > > > Signed-off-by: Bhumika Goyal > > --- > > Thanks, we're reviewing this internally. It looks fine, but I need to > assess whether this will have any impacts in our backports project > before we can apply it. This has been applied in our internal tree and will eventually land in the mainline. Thanks! -- Cheers, Luca.
Re: [PATCH] iwlwifi: mvm: fix iwl_mvm_sar_find_wifi_pkg corner case
On Tue, 2017-06-27 at 17:24 +0200, Arnd Bergmann wrote:
> gcc warns about what it thinks is an uninitialized variable
> access:
>
> drivers/net/wireless/intel/iwlwifi/mvm/fw.c: In function
> 'iwl_mvm_sar_find_wifi_pkg.isra.14':
> drivers/net/wireless/intel/iwlwifi/mvm/fw.c:1102:5: error: 'wifi_pkg' may be
> used uninitialized in this function [-Werror=maybe-uninitialized]
>
> That problem cannot really happen, as we check data->package.count
> to ensure that the loop is entered at least once.
> However, something that can indeed happen is returning an incorrect
> wifi_pkg pointer in case none of the elements are what we are looking
> for.
>
> This modifies the loop again, to only return a correct object, and
> to shut up that warning.
>
> Fixes: c386dacb4ed6 ("iwlwifi: mvm: refactor SAR init to prepare for dynamic
> SAR")
> Signed-off-by: Arnd Bergmann
> ---
Thanks, Arnd!
I've pushed this to our internal tree and it will eventually reach the
mainline, via our normal upstreaming process.
--
Cheers,
Luca.
Re: [PATCH] iwlwifi: mvm: add const to thermal_cooling_device_ops structure
On Wed, 2017-06-21 at 14:10 +0530, Bhumika Goyal wrote: > Declare thermal_cooling_device_ops structure as const as it is only passed > as an argument to the function thermal_cooling_device_register and this > argument is of type const. So, declare the structure as const. > > Signed-off-by: Bhumika Goyal > --- Thanks, we're reviewing this internally. It looks fine, but I need to assess whether this will have any impacts in our backports project before we can apply it. -- Cheers, Luca.
Re: [PATCH] net: wireless: intel: iwlwifi: dvm: remove unused defines
On Wed, 2017-06-07 at 01:20 +0200, Seraphime Kirkovski wrote: > Those constants have been unused for quite some time now. > > Signed-off-by: Seraphime Kirkovski > --- > I've compile-tested it. Thanks. I've applied it to our internal tree and it will reach the mainline at some point. -- Cheers, Luca.
Re: [PATCH v6 4/5] iwlwifi: convert to use driver data API
On Fri, 2017-04-28 at 02:56 +0200, Luis R. Rodriguez wrote:
> On Mon, Apr 10, 2017 at 04:19:12PM +0300, Luca Coelho wrote:
> > On Wed, 2017-03-29 at 20:25 -0700, Luis R. Rodriguez wrote:
> > > The driver data API provides support for looking for firmware
> > > from a specific set of API ranges, so just use that. Since we
> > > free the firmware on the callback immediately after consuming it,
> > > this also takes avantage of that feature.
> > >
> > > Signed-off-by: Luis R. Rodriguez
> > > ---
> >
> > Looks fine, with one nitpick.
> >
> >
> > > drivers/net/wireless/intel/iwlwifi/iwl-drv.c | 67
> > > ++--
> > > 1 file changed, 23 insertions(+), 44 deletions(-)
> > >
> > > diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
> > > b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
> > > index be466a074c1d..b6643aa5b344 100644
> > > --- a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
> > > +++ b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
> >
> > [...]
> >
> > > @@ -1541,11 +1522,9 @@ struct iwl_drv *iwl_drv_start(struct iwl_trans
> > > *trans)
> > > }
> > > #endif
> > >
> > > - ret = iwl_request_firmware(drv, true);
> > > - if (ret) {
> > > - IWL_ERR(trans, "Couldn't request the fw\n");
> > > + ret = iwl_request_firmware(drv);
> > > + if (ret)
> > > goto err_fw;
> > > - }
> >
> > Why remove the error message here?
>
> The driver data API now has enough semantics even for async requests so that
> an error is either always issued or supressed (optional is true), driver
> errors
> then are superfluous on error now.
>
> Let me know if this is OK.
Yeah, that's okay. We can always add something back if we think it's
needed.
--
Cheers,
Luca.
Re: [PATCH v6 2/5] firmware: add extensible driver data API
On Thu, 2017-04-27 at 05:16 +0200, Luis R. Rodriguez wrote:
> > > +int driver_data_request_sync(const char *name,
> > > + const struct driver_data_req_params *req_params,
> > > + struct device *device)
> > > +{
> > > + const struct firmware *driver_data;
> > > + const struct driver_data_reqs *sync_reqs;
> > > + struct driver_data_params params = {
> > > + .req_params = *req_params,
> > > + };
> > > + int ret;
> > > +
> > > + if (!device || !req_params || !name || name[0] == '\0')
> > > + return -EINVAL;
> > > +
> > > + if (req_params->sync_reqs.mode != DRIVER_DATA_SYNC)
> > > + return -EINVAL;
> >
> > Why do you need to check this here? If the caller is calling _sync(),
> > it's because that's what it needs. This mode value here seems
> > redundant.
>
> Because we allow one data structure to be used for the specified requirements
> and technically someone can make a mistake and use the wrong macros to set up
> the data structure. This ensures users don't async macros to set up the
> parameters and then use the sync call. Eventually the underlying
> firmware_class.c code does its own conditional checks on this as well.
If this could only happen in a programming error, maybe it's worth a
WARN() then, to make it easier to spot?
> > OTOH, if you do have a reason for this value, then you could use
> > driver_data_request_sync() in this if.
>
> You mean to allow *one* API call for all. Sure, that's possible, but I think
> its nicer to at least expose async/sync mechanisms on the caller side. The
> sync/async differences seem like a reasonable enough place to split the API.
I don't remember the details of this anymore, but doesn't the
driver_data_sync() function does exactly the same check? I meant that
you could do this:
if(WARN_ON_ONCE(!driver_data_request_sync()))
return -EINVAL;
And yes, I think either a single API call for all or not having the mode
in the struct would be cleaner. I think there are better ways to
prevent coding errors (since this should only happen if the user code is
implemented incorrectly).
--
Cheers,
Luca.
Re: [PATCH v6 2/5] firmware: add extensible driver data API
On Thu, 2017-04-27 at 05:16 +0200, Luis R. Rodriguez wrote:
> > > @@ -1460,6 +1471,128 @@ void release_firmware(const struct firmware *fw)
> > > }
> > > EXPORT_SYMBOL(release_firmware);
> > >
> > > +static int _driver_data_request_api(struct driver_data_params *params,
> > > + struct device *device,
> > > + const char *name)
> > > +{
> > > + struct driver_data_priv_params *priv_params = ¶ms->priv_params;
> > > + const struct driver_data_req_params *req_params = ¶ms->req_params;
> > > + int ret;
> > > + char *try_name;
> > > + u8 api_max;
> > > +
> > > + if (priv_params->retry_api) {
> > > + if (!priv_params->api)
> > > + return -ENOENT;
> > > + api_max = priv_params->api - 1;
> > > + } else
> > > + api_max = req_params->api_max;
> >
> > Braces.
>
> Not sure what you mean here, the else is a 1 liner so I skip the brace.
It's really a nitpick, but the CodingStyle[1] says:
"Do not unnecessarily use braces where a single statement will do.
[...]
This does not apply if only one branch of a conditional statement is a
single statement; in the latter case use braces in both branches:
if (condition) {
do_this();
do_that();
} else {
otherwise();
}"
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst#n175
--
Cheers,
Luca.
Re: [PATCH v6 4/5] iwlwifi: convert to use driver data API
On Wed, 2017-03-29 at 20:25 -0700, Luis R. Rodriguez wrote:
> The driver data API provides support for looking for firmware
> from a specific set of API ranges, so just use that. Since we
> free the firmware on the callback immediately after consuming it,
> this also takes avantage of that feature.
>
> Signed-off-by: Luis R. Rodriguez
> ---
Looks find, with one nitpick.
> drivers/net/wireless/intel/iwlwifi/iwl-drv.c | 67
> ++--
> 1 file changed, 23 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
> b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
> index be466a074c1d..b6643aa5b344 100644
> --- a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
> +++ b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
[...]
> @@ -1541,11 +1522,9 @@ struct iwl_drv *iwl_drv_start(struct iwl_trans *trans)
> }
> #endif
>
> - ret = iwl_request_firmware(drv, true);
> - if (ret) {
> - IWL_ERR(trans, "Couldn't request the fw\n");
> + ret = iwl_request_firmware(drv);
> + if (ret)
> goto err_fw;
> - }
Why remove the error message here?
--
Cheers,
Luca.
Re: [PATCH v6 1/5] firmware: add extensible driver data params
On Wed, 2017-03-29 at 20:25 -0700, Luis R. Rodriguez wrote:
> As the firmware API evolves we keep extending functions with more arguments.
> Stop this nonsense by proving an extensible data structure which can be used
> to represent both user parameters and private internal parameters.
>
> We introduce 3 data structures:
>
> o struct driver_data_req_params - used for user specified parameters
> o struct driver_data_priv_params - used for internal use
> o struct driver_data_params - stiches both of the the above together,
> also only for internal use
>
> This starts off by just making the existing APIs use the new data
> structures, it will make subsequent changes easier to review which will
> be adding new flexible APIs.
>
> This commit should introduces no functional changes (TM).
>
> Signed-off-by: Luis R. Rodriguez
> ---
Looks fine with a few nitpicks.
> diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
> index 54fc4c42f126..f702566554e1 100644
> --- a/drivers/base/firmware_class.c
> +++ b/drivers/base/firmware_class.c
[...]
> @@ -40,6 +41,77 @@ MODULE_AUTHOR("Manuel Estrada Sainz");
> MODULE_DESCRIPTION("Multi purpose firmware loading support");
> MODULE_LICENSE("GPL");
>
> +struct driver_data_priv_params {
> + bool use_fallback;
> + bool use_fallback_uevent;
> + bool no_cache;
> + void *alloc_buf;
> + size_t alloc_buf_size;
> +};
> +
> +struct driver_data_params {
> + const struct firmware *driver_data;
> + const struct driver_data_req_params req_params;
> + struct driver_data_priv_params priv_params;
> +};
> +
> +/*
> + * These are kept to remain backward compatible with old behaviour. Do not
> + * modify them unless you know what you are doing. These are to be used only
> + * by the old API, so:
> + *
> + * Old sync APIs:
> + * o request_firmware(): __DATA_REQ_FIRMWARE()
> + * o request_firmware_direct():__DATA_REQ_FIRMWARE_DIRECT()
> + * o request_firmware_into_buf(): __DATA_REQ_FIRMWARE_BUF()
> + *
> + * Old async API:
> + * o request_firmware_nowait():__DATA_REQ_FIRMWARE_NOWAIT()
> + */
> +#define __DATA_REQ_FIRMWARE()
> \
> + .priv_params = {\
> + .use_fallback = !!FW_OPT_FALLBACK, \
use_fallback is a boolean, so you don't need the double negation here.
[...]
> @@ -1332,12 +1433,15 @@ request_firmware_into_buf(const struct firmware
> **firmware_p, const char *name,
> struct device *device, void *buf, size_t size)
> {
> int ret;
> + struct driver_data_params data_params = {
> + __DATA_REQ_FIRMWARE_BUF(buf, size),
> + };
>
> __module_get(THIS_MODULE);
> - ret = _request_firmware(firmware_p, name, device, buf, size,
> - FW_OPT_UEVENT | FW_OPT_FALLBACK |
> - FW_OPT_NOCACHE);
> + ret = _request_firmware(firmware_p, name, &data_params, device);
> module_put(THIS_MODULE);
> +
> +
Double empty-lines here.
> return ret;
> }
> EXPORT_SYMBOL(request_firmware_into_buf);
>
[...]
> diff --git a/include/linux/driver_data.h b/include/linux/driver_data.h
> new file mode 100644
> index ..c3d3a4129838
> --- /dev/null
> +++ b/include/linux/driver_data.h
> @@ -0,0 +1,88 @@
> +#ifndef _LINUX_DRIVER_DATA_H
> +#define _LINUX_DRIVER_DATA_H
> +
> +#include
> +#include
> +#include
> +#include
> +
> +/*
> + * Driver Data internals
> + *
> + * Copyright (C) 2017 Luis R. Rodriguez
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of copyleft-next (version 0.3.1 or later) as published
> + * at http://copyleft-next.org/.
> + */
> +
> +/**
> + * enum driver_data_mode - driver data mode of operation
> + *
> + * DRIVER_DATA_SYNC: your call to request driver data is synchronous. We will
> + * look for the driver data file you have requested immediatley.
> + * DRIVER_DATA_ASYNC: your call to request driver data is asynchronous. We
> will
It should be @DRIVER_DATA_SYNC and @DRIVER_DATA_ASYNC here.
--
Cheers,
Luca.
Re: Issues with FW with Intel 720 wifi card
Hi Bharat, On Fri, 2017-02-03 at 16:27 +, Bharat Kumar Gogada wrote: > Hi, > > Im using linux 4.6 kernel, I get following error when I do interface up. > > Here is the boot log. > [4.407681] iwlwifi :01:00.0: loaded firmware version 16.242414.0 > op_mode iwlmvm > [4.407742] iwlwifi :01:00.0: Detected Intel(R) Wireless N 7260, > REV=0x144 > [4.407831] iwlwifi :01:00.0: L1 Disabled - LTR Disabled > [4.408116] iwlwifi :01:00.0: L1 Disabled - LTR Disabled > > When I do interface up I get following : > [9.41] iwlwifi :01:00.0: Failed to load firmware chunk! > [9.425947] iwlwifi :01:00.0: Could not load the [0] uCode section > [9.432472] iwlwifi :01:00.0: Failed to start INIT ucode: -110 > [9.438899] iwlwifi :01:00.0: Failed to run INIT ucode: -110 > [9.444858] iwlwifi :01:00.0: L1 Disabled - LTR Disabled It's hard to say why this is happening. > I downloaded firmware from here > https://wireless.wiki.kernel.org/en/users/Drivers/iwlwifi Why did you have to download the firmware? Doesn't your distro include the binaries from linux-firmware.git? The firmware you are using is version 16, but that kernel already supports version -17. I recommend you try to take the latest firmware for the 7260 NIC from our copy of linux-firmware.git[2]. If this doesn't help, the best thing to do would be to report this as a bug in bugzilla.kernel.org following the instructions in our wiki[1]. This way we can help you more easily. [1] https://wireless.wiki.kernel.org/en/users/drivers/iwlwifi/debugging [2] http://git.kernel.org/cgit/linux/kernel/git/iwlwifi/linux-firmware.git/plain/iwlwifi-7260-17.ucode -- Cheers, Luca.
Re: kernel 4.9 iwlwifi startup error
On Tue, 2017-01-10 at 09:21 +0100, Fabio Coatti wrote: > In data martedì 10 gennaio 2017 00:21:51 CET, Luca Coelho ha scritto: > > On Tue, 2017-01-03 at 13:42 +1100, Andrew Donnellan wrote: > > > On 02/01/17 21:12, Fabio Coatti wrote: > > > > Hi all, > > > > I'm using kernel 4.9 and maybe half of the times I boot my laptop I get > > > > the > > > > error reported below, and the wifi does not work. I have to remove > > > > iwlwifi (like modprobe -r iwldvm iwlwifi) and insert it again to get > > > > things workig again. This seems a bit random, it does not happens all > > > > the times so it could be a timing issue or even a flaky hardware > > > > (unlikely, as I see only this issue with wifi, once it starts it works > > > > just fine) > > > > I'm pretty sure to have seen the same behaviour at some point in 4.8.X > > > > release, but right now I lost the related notes. > > > > Environment: > > > > Distro: gentoo > > > > gcc 5.4.0 > > > > HW: Hewlett-Packard HP EliteBook Folio 9470m/18DF, BIOS 68IBD Ver. F.63 > > > > 04/26/2016 > > > > Intel(R) Core(TM) i5-3427U CPU @ 1.80GHz > > > > Network controller: Intel Corporation Centrino Advanced-N 6235 (rev 24) > > > > > > > > Of course if more info is needed, just drop me a note. > > > > > > > > I'm not subscribed to mailing lists, so please keep me in CC: for any > > > > information request. > > > > > > > > Many thanks. > > > > > > I've so far seen this once on my laptop, a Samsung NP540U3C (don't have > > > it with me right now, so I'm not sure what the wifi chipset is), running > > > with a Debian 4.9 kernel. > > > > This bug has already been reported in bugzilla: > > > > https://bugzilla.kernel.org/show_bug.cgi?id=190281 > > > > Did you have any problems with older kernel versions? If so, would it be > > possible for you to bisect? > > > Well, it happens on a laptop that I use quite intensively, so it will take > some time to bisect. However, in the meantime I checked the old logs and the > first recorded occurrence happened with 4.8.10 release and firmware version > 18.168.6.1 Thanks! Let's continue tracking this in bugzilla. -- Cheers, Luca.
Re: kernel 4.9 iwlwifi startup error
On Tue, 2017-01-03 at 07:41 -0800, Alexander Morozov wrote: > I have a similar problem on Gentoo. But in my case, it just can't load > firmware: "no suitable firmware found". I've tried to reinstall > firmware with no luck. Everything is ok with 4.8.6. This is a completely different issue. This means that you don't have the correct firmware installed for this kernel version. We do support several versions back from the latest, but we can't support older firmwares forever, so if you're using a recent kernel you must upgrade the firmwares too. Let me know which device you have, so I can tell you which firmware you need to use. But as a general rule, if you clone the linux-firmware repository[1] and install all the iwlwifi*.ucode files in /lib/firmware (or wherever gentoo looks for the firmware in your filesystem), you will surely get the latest version of the firmware that works with your kernel. [1] http://git.kernel.org/cgit/linux/kernel/git/firmware/linux-firmware.git -- Cheers, Luca.
Re: kernel 4.9 iwlwifi startup error
On Tue, 2017-01-03 at 13:42 +1100, Andrew Donnellan wrote: > On 02/01/17 21:12, Fabio Coatti wrote: > > Hi all, > > I'm using kernel 4.9 and maybe half of the times I boot my laptop I get the > > error reported below, and the wifi does not work. I have to remove iwlwifi > > (like > > modprobe -r iwldvm iwlwifi) and insert it again to get things workig again. > > This seems a bit random, it does not happens all the times so it could be a > > timing issue or even a flaky hardware (unlikely, as I see only this issue > > with > > wifi, once it starts it works just fine) > > I'm pretty sure to have seen the same behaviour at some point in 4.8.X > > release, but right now I lost the related notes. > > Environment: > > Distro: gentoo > > gcc 5.4.0 > > HW: Hewlett-Packard HP EliteBook Folio 9470m/18DF, BIOS 68IBD Ver. F.63 > > 04/26/2016 > > Intel(R) Core(TM) i5-3427U CPU @ 1.80GHz > > Network controller: Intel Corporation Centrino Advanced-N 6235 (rev 24) > > > > Of course if more info is needed, just drop me a note. > > > > I'm not subscribed to mailing lists, so please keep me in CC: for any > > information request. > > > > Many thanks. > > I've so far seen this once on my laptop, a Samsung NP540U3C (don't have > it with me right now, so I'm not sure what the wifi chipset is), running > with a Debian 4.9 kernel. This bug has already been reported in bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=190281 Did you have any problems with older kernel versions? If so, would it be possible for you to bisect? -- Cheers, Luca.
Re: drivers/net/wireless/intel/iwlwifi/pcie/trans.c: 2 * suspicious code ?
On Fri, 2017-01-06 at 17:47 +, David Binderman wrote: > Hello there, Hi David, > 1. > > drivers/net/wireless/intel/iwlwifi/pcie/trans.c:2039:14: warning: decrement > of a boolean expression [-Wbool-operation] > > Source code is > >txq->block--; > > Maybe someone got a bool and a int mixed up ? > > 2. > > drivers/net/wireless/intel/iwlwifi/pcie/trans.c:2045:14: warning: increment > of a boolean expression [-Wbool-operation] > > Duplicate a few lines further down. Emmanuel has fixed this in our internal tree and I'll be sending it out together with our normal upstreaming process. Thanks for reporting! -- Cheers, Luca.
Re: Intel Wireless 7260 failed to work
On Wed, 2016-12-28 at 10:17 +0200, Emmanuel Grumbach wrote: > On Wed, Dec 28, 2016 at 10:10 AM, Peter Xu wrote: > > On Wed, Dec 28, 2016 at 09:27:15AM +0200, Luca Coelho wrote: > > > > [...] > > > > > > > > Is this a known issue? Please let me know if anyone wants more info > > > > > > or > > > > > > logs, since this error triggers easily (everytime I boot). > > > > > > > > > > The error message isn't really telling much to the user (hint hint) > > > > > but > > > > > I suspect this is by design: > > > > > > > > > > "iwlwifi: remove support for fw older than -17 and -22 > > > > > > Right, we only maintain support for a certain number of firmware > > > versions. The FW APIs change with time and we don't want to keep all > > > legacy code supporting old firmwares in the driver forever. > > > > > > I agree that "no suitable firmware found!" is a bit scarce. I'll see > > > if we can improve that with something: "no suitable firmware found! You > > > need iwlwifi-7260-17.ucode ()". > > > > That'll be great if we can have this info in the log. Maybe no need > > for a full URL, the firmware name would suffice at least for me. > > In this case, I think we should also print the range that is supported > when we fail to find any suitable firmware. Sure, I'll do that. I just wanted to keep it simple in this thread. ;) -- Luca.
Re: Intel Wireless 7260 failed to work
On Wed, 2016-12-28 at 11:59 +0800, Peter Xu wrote: > On Tue, Dec 27, 2016 at 09:46:55PM +0200, Kalle Valo wrote: > > Peter Xu writes: > > > > > Looks like latest Linux master (4.10-rc1, 7ce7d89f) cannot work well > > > with my wireless card, which is: > > > > > > Intel Corporation Wireless 7260 (rev bb) > > > > > > Boot message shows that no suitable firmware found: > > > > > > # journalctl -kp3 > > > Dec 27 16:38:00 kernel: Error parsing PCC subspaces from PCCT > > > Dec 27 16:38:00 kernel: mmc0: Unknown controller version (3). You may > > > experience problems. > > > Dec 27 16:38:02 kernel: DMAR: DRHD: handling fault status reg 2 > > > Dec 27 16:38:02 kernel: DMAR: [DMA Write] Request device [00:02.0] > > > fault addr 72 [fault reason 05] PTE Write a > > > Dec 27 16:38:03 kernel: tpm tpm0: A TPM error (6) occurred attempting > > > to read a pcr value > > > Dec 27 16:38:04 kernel: iwlwifi :03:00.0: no suitable firmware > > > found! > > > > > > Linux stable/master (Linux 4.8-rc8, 08895a8b6b) works for me. And no > > > further tests have been done yet. > > > > > > Is this a known issue? Please let me know if anyone wants more info or > > > logs, since this error triggers easily (everytime I boot). > > > > The error message isn't really telling much to the user (hint hint) but > > I suspect this is by design: > > > > "iwlwifi: remove support for fw older than -17 and -22 Right, we only maintain support for a certain number of firmware versions. The FW APIs change with time and we don't want to keep all legacy code supporting old firmwares in the driver forever. I agree that "no suitable firmware found!" is a bit scarce. I'll see if we can improve that with something: "no suitable firmware found! You need iwlwifi-7260-17.ucode ()". > > FW versions older than -17 for 3160 and 7260 and older than -22 for > > newer NICs are not supported anymore. Don't load these versions > > and remove code that handles them." > > > > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=4b87e5af638b6056bd6c20b0954d09a5a58633be > > > > Adding luca. > > Thanks for the triage. > > Larry's link for -17 firmware solved my issue. Though I still don't > know whether it is too aggresive if we just remove the support for -16 > (which is the one I was using with the old 4.6 kernel, btw I am using > Fedora 24, which is relatively new as well). I don't think we are very aggressive, we have been supporting -17 since v4.3: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=5865f3658ba37c54e346b0fdee08a1c7a152681b And we have published the firmware about half a year ago: http://git.kernel.org/cgit/linux/kernel/git/firmware/linux-firmware.git/commit/iwlwifi-7260-17.ucode?id=f2cf4d67e8eced29c8a473d3a27057aa2df57c42 I understand your concern, but if you want to be on the bleeding-edge kernel, you should be on the bleeding-edge linux-firmware as well. ;) But as I said, I'll try to improve the error message, as that should make it easier to figure out. -- Cheers, Luca.
Re: [PATCH 5/8] linux: drop __bitwise__ everywhere
On Thu, 2016-12-15 at 07:15 +0200, Michael S. Tsirkin wrote: > __bitwise__ used to mean "yes, please enable sparse checks > unconditionally", but now that we dropped __CHECK_ENDIAN__ > __bitwise is exactly the same. > There aren't many users, replace it by __bitwise everywhere. > > Signed-off-by: Michael S. Tsirkin > --- > arch/arm/plat-samsung/include/plat/gpio-cfg.h| 2 +- > drivers/md/dm-cache-block-types.h| 6 +++--- > drivers/net/ethernet/sun/sunhme.h| 2 +- > drivers/net/wireless/intel/iwlwifi/iwl-fw-file.h | 4 ++-- For drivers/net/wireless/intel/iwlwifi/iwl-fw-file.h: Acked-by: Luca Coelho -- Luca.
Re: [PATCH 8/8] Makefile: drop -D__CHECK_ENDIAN__ from cflags
On Thu, 2016-12-15 at 07:15 +0200, Michael S. Tsirkin wrote: > That's the default now, no need for makefiles to set it. > > Signed-off-by: Michael S. Tsirkin > --- > drivers/bluetooth/Makefile| 2 -- > drivers/net/can/Makefile | 1 - > drivers/net/ethernet/altera/Makefile | 1 - > drivers/net/ethernet/atheros/alx/Makefile | 1 - > drivers/net/ethernet/freescale/Makefile | 2 -- > drivers/net/wireless/ath/Makefile | 2 -- > drivers/net/wireless/ath/wil6210/Makefile | 2 -- > drivers/net/wireless/broadcom/brcm80211/brcmfmac/Makefile | 2 -- > drivers/net/wireless/broadcom/brcm80211/brcmsmac/Makefile | 1 - > drivers/net/wireless/intel/iwlegacy/Makefile | 2 -- > drivers/net/wireless/intel/iwlwifi/Makefile | 2 +- > drivers/net/wireless/intel/iwlwifi/dvm/Makefile | 2 +- > drivers/net/wireless/intel/iwlwifi/mvm/Makefile | 2 +- For the drivers/net/wireless/intel/iwlwifi/ part: Acked-by: Luca Coelho -- Luca.
Re: [linuxwifi] [PATCH 1/2] iwlwifi: fix MODULE_FIRMWARE for 6030
Hi Jürg,
On Mon, 2016-10-10 at 18:30 +0200, Jürg Billeter wrote:
> IWL6000G2B_UCODE_API_MAX is not defined. ucode_api_max of
> IWL_DEVICE_6030 uses IWL6000G2_UCODE_API_MAX. Use this also for
> MODULE_FIRMWARE.
>
> Fixes: 9d9b21d1b616 ("iwlwifi: remove IWL_*_UCODE_API_OK")
> Signed-off-by: Jürg Billeter
> ---
> drivers/net/wireless/intel/iwlwifi/iwl-6000.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-6000.c
> b/drivers/net/wireless/intel/iwlwifi/iwl-6000.c
> index 0b9f6a7..39335b7 100644
> --- a/drivers/net/wireless/intel/iwlwifi/iwl-6000.c
> +++ b/drivers/net/wireless/intel/iwlwifi/iwl-6000.c
> @@ -371,4 +371,4 @@ const struct iwl_cfg iwl6000_3agn_cfg = {
> MODULE_FIRMWARE(IWL6000_MODULE_FIRMWARE(IWL6000_UCODE_API_MAX));
> MODULE_FIRMWARE(IWL6050_MODULE_FIRMWARE(IWL6050_UCODE_API_MAX));
> MODULE_FIRMWARE(IWL6005_MODULE_FIRMWARE(IWL6000G2_UCODE_API_MAX));
> -MODULE_FIRMWARE(IWL6030_MODULE_FIRMWARE(IWL6000G2B_UCODE_API_MAX));
> +MODULE_FIRMWARE(IWL6030_MODULE_FIRMWARE(IWL6000G2_UCODE_API_MAX));
Sorry I had missed these patches because linux-wireless@vger was not
CCed, so it didn't get to patchwork. I have applied both in our
internal tree and they'll reach the mainline in my next pull-request.
Thanks!
--
Cheers,
Luca.
Re: linux-4.9-rc1/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c:1561: poor error checking ?
Hi David,
On Mon, 2016-10-17 at 07:40 +, David Binderman wrote:
> Hello there,
>
> linux-4.9-rc1/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c:1561]: (style)
> Checking if unsigned variable 'len' is less than zero.
>
> Source code is
>
> len = min((size_t)le32_to_cpu(rsp->len) << 2,
> iwl_rx_packet_payload_len(hcmd.resp_pkt) - sizeof(*rsp));
> len = min(len - delta, count);
> if (len < 0) {
> ret = -EFAULT;
> goto out;
> }
>
> Suggest improve error checking.
Thanks for reporting! A fix for this is already queued in our internal
tree and will be sent upstream soon.
--
Cheers,
Luca.
Re: [PATCH] iwlwifi: pcie: fix SPLC structure parsing
On Thu, 2016-10-13 at 14:55 +0200, Paul Bolle wrote: > On Thu, 2016-10-13 at 15:44 +0300, Luca Coelho wrote: > > Even though there is apparently something wrong with this part of the > > ACPI table on you laptop, since it doesn't match our specifications. > > In any case, it's mostly harmless. > > > Would a correct implementation by Dell have any benefits for the users > of these laptops? In other words: should I bother somehow contacting > Dell and point them to this discussion in order to have them fix this? This value provides a way for the OEM to fine tune the power budget of the device. This is (usually) used to prevent parts of the platform from getting too hot. We have a certain default that is good enough for most cases. If Dell didn't want to set proper limits for this device, it probably means that it is not a concern. Dell does set these values correctly for other platforms. So, I guess it's up to you if you want to clarify this with them. This could be some default "blank" values when they don't want to change them. -- Luca.
Re: [PATCH] iwlwifi: pcie: fix SPLC structure parsing
On Thu, 2016-10-13 at 08:56 -0500, Chris Rorvick wrote: > Hi Luca, > > > On Thu, 2016-10-13 at 13:21 +0300, Luca Coelho wrote: > > Could you please give this a spin? I have tested it with some handmade > > ACPI tables in QEMU and it seems to work fine now. > > > Tested-by: Chris Rorvick > > I think the debug output looks as expected, see below for the first 20 > lines or so. And, more importantly, everything seems to be working! > :-) Yes, you got exactly what was expected. Thanks for testing! -- Luca.
Re: [PATCH] iwlwifi: pcie: fix SPLC structure parsing
On Thu, 2016-10-13 at 14:36 +0200, Paul Bolle wrote: > On Thu, 2016-10-13 at 14:30 +0300, Luca Coelho wrote: > > I forgot to say... could you load the iwlwifi module with debug=0x01 > > (module parameter), so we can see the messages the driver is printing > > when it doesn't find a proper structure? > > > That makes a lot of noise! Here's the first 100 lines or so, that > apparently are generated directly after modprobing iwlwifi. > > After these 100 lines there's a ten second gap (I guess it took me ten > seconds to actually use the wifi). I assume you don't care about that > part of the debug messages. > > Have fun! Thanks, Paul! Yeah, this is the "INFO" debugging level and it is a sort of fallback bucket when there is nothing more specific. Sorry for that. > <7>[ 767.691342] iwlwifi :3a:00.0: U iwl_pcie_prepare_card_hw > iwl_trans_prepare_card_hw enter > <7>[ 767.691362] iwlwifi :3a:00.0: U iwl_pcie_set_hw_ready hardware ready > <7>[ 767.692127] iwlwifi :3a:00.0: U iwl_request_firmware attempting to > load firmware 'iwlwifi-8000C-24.ucode' > <7>[ 767.692322] iwlwifi :3a:00.0: U splc_get_pwr_limit No element for > the WiFi domain returned by the SPLC method. This is the line I was looking for. :) So everything looks fine now. Even though there is apparently something wrong with this part of the ACPI table on you laptop, since it doesn't match our specifications. In any case, it's mostly harmless. Thanks for the help! -- Cheers, Luca.
Re: [PATCH] iwlwifi: pcie: fix SPLC structure parsing
On Thu, 2016-10-13 at 13:27 +0200, Paul Bolle wrote: > Luca, > > On Thu, 2016-10-13 at 13:21 +0300, Luca Coelho wrote: > > Could you please give this a spin? I have tested it with some handmade > > ACPI tables in QEMU and it seems to work fine now. > > > Tested-by: Paul Bolle > > Not that this test was worth a lot: it builds cleanly (on top of > 4.8.1), the error at boot is gone, obviously, and wifi still works (as > you're reading a message that was sent out over wifi). And I haven't > even tested this on another machine than my XPS 13 (9350). Thanks for testing! I forgot to say... could you load the iwlwifi module with debug=0x01 (module parameter), so we can see the messages the driver is printing when it doesn't find a proper structure? -- Luca.
[PATCH] iwlwifi: pcie: fix SPLC structure parsing
From: Luca Coelho
The SPLC data parsing is too restrictive and was not trying find the
correct element for WiFi. This causes problems with some BIOSes where
the SPLC method exists, but doesn't have a WiFi entry on the first
element of the list. The domain type values are also incorrect
according to the specification.
Fix this by complying with the actual specification.
Additionally, replace all occurrences of SPLX to SPLC, since SPLX is
only a structure internal to the ACPI tables, and may not even exist.
Fixes: bcb079a14d75 ("iwlwifi: pcie: retrieve and parse ACPI power limitations")
Reported-by: Chris Rorvick
Signed-off-by: Luca Coelho
---
Paul, Chris,
Could you please give this a spin? I have tested it with some handmade
ACPI tables in QEMU and it seems to work fine now.
Thanks!
drivers/net/wireless/intel/iwlwifi/pcie/drv.c | 79 ---
1 file changed, 48 insertions(+), 31 deletions(-)
diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/drv.c
b/drivers/net/wireless/intel/iwlwifi/pcie/drv.c
index 001be40..da4810f 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/drv.c
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/drv.c
@@ -541,48 +541,64 @@ static const struct pci_device_id iwl_hw_card_ids[] = {
MODULE_DEVICE_TABLE(pci, iwl_hw_card_ids);
#ifdef CONFIG_ACPI
-#define SPL_METHOD "SPLC"
-#define SPL_DOMAINTYPE_MODULE BIT(0)
-#define SPL_DOMAINTYPE_WIFIBIT(1)
-#define SPL_DOMAINTYPE_WIGIG BIT(2)
-#define SPL_DOMAINTYPE_RFEMBIT(3)
+#define ACPI_SPLC_METHOD "SPLC"
+#define ACPI_SPLC_DOMAIN_WIFI (0x07)
-static u64 splx_get_pwr_limit(struct iwl_trans *trans, union acpi_object *splx)
+static u64 splc_get_pwr_limit(struct iwl_trans *trans, union acpi_object *splc)
{
- union acpi_object *limits, *domain_type, *power_limit;
-
- if (splx->type != ACPI_TYPE_PACKAGE ||
- splx->package.count != 2 ||
- splx->package.elements[0].type != ACPI_TYPE_INTEGER ||
- splx->package.elements[0].integer.value != 0) {
- IWL_ERR(trans, "Unsupported splx structure\n");
+ union acpi_object *data_pkg, *dflt_pwr_limit;
+ int i;
+
+ /* We need at least two elemets, one for the revision and one
+* for the data itself. Also check that the revision is
+* supported (currently only revision 0).
+ */
+ if (splc->type != ACPI_TYPE_PACKAGE ||
+ splc->package.count < 2 ||
+ splc->package.elements[0].type != ACPI_TYPE_INTEGER ||
+ splc->package.elements[0].integer.value != 0) {
+ IWL_DEBUG_INFO(trans,
+ "Unsupported structure returned by the SPLC
method. Ignoring.\n");
return 0;
}
- limits = &splx->package.elements[1];
- if (limits->type != ACPI_TYPE_PACKAGE ||
- limits->package.count < 2 ||
- limits->package.elements[0].type != ACPI_TYPE_INTEGER ||
- limits->package.elements[1].type != ACPI_TYPE_INTEGER) {
- IWL_ERR(trans, "Invalid limits element\n");
- return 0;
+ /* loop through all the packages to find the one for WiFi */
+ for (i = 1; i < splc->package.count; i++) {
+ union acpi_object *domain;
+
+ data_pkg = &splc->package.elements[i];
+
+ /* Skip anything that is not a package with the right
+* amount of elements (i.e. at least 2 integers).
+*/
+ if (data_pkg->type != ACPI_TYPE_PACKAGE ||
+ data_pkg->package.count < 2 ||
+ data_pkg->package.elements[0].type != ACPI_TYPE_INTEGER ||
+ data_pkg->package.elements[1].type != ACPI_TYPE_INTEGER)
+ continue;
+
+ domain = &data_pkg->package.elements[0];
+ if (domain->integer.value == ACPI_SPLC_DOMAIN_WIFI)
+ break;
+
+ data_pkg = NULL;
}
- domain_type = &limits->package.elements[0];
- power_limit = &limits->package.elements[1];
- if (!(domain_type->integer.value & SPL_DOMAINTYPE_WIFI)) {
- IWL_DEBUG_INFO(trans, "WiFi power is not limited\n");
+ if (!data_pkg) {
+ IWL_DEBUG_INFO(trans,
+ "No element for the WiFi domain returned by the
SPLC method.\n");
return 0;
}
- return power_limit->integer.value;
+ dflt_pwr_limit = &data_pkg->package.elements[1];
+ return dflt_pwr_limit->integer.value;
}
static void set_dflt_pwr_limit(struct iwl_trans *trans, struct pci_dev *pdev)
{
acpi_handle pxsx_handle;
acpi_handle handle;
- struct acpi_buffer splx = {ACPI_ALLOCATE_BU
Re: [PATCH] iwlwifi: pcie: reduce "unsupported splx" to a warning
On Wed, 2016-10-12 at 12:50 -0500, Chris Rorvick wrote: > Hi Luca, > > FYI, It seems that Google does not like your email as I'm not > receiving any of your messages in gmail. Some responses below: That's odd. It works for me. Replied privately to try to sort this out. > On Wed, 2016-10-12 at 15:24 +0300, Luca Coelho wrote: > > Hi Chris, > > On Tue, 2016-10-11 at 09:09 -0500, Chris Rorvick wrote: > > > On Tue, Oct 11, 2016 at 5:11 AM, Paul Bolle wrote: > > > > > This is not coming from the NIC itself, but from the platform's ACPI > > > > > tables. Can you tell us which platform you are using? > > > > > > > > > Interesting. I'm running a Dell XPS 13 9350. I replaced the > > > factory-provided Broadcom card with an AC 8260. I can update the > > > commit log to reflect this. > > > > > > Okay, so this makes sense. Those entries are probably formatted for > > the Broadcom card, which the iwlwifi driver obviously doesn't > > understand. The best we can do, as I already said, is to ignore values > > we don't understand. > > > This may already be apparent, but Dell sells two versions of the 9350: > one with the Broadcom adapter and one with the AC 8260. I just > happened to find the former version at a deep discount at Costco so > decided to chance it. Turns out the Broadcom card is not so good even > with new kernels so I upgraded. Anyway, since Paul is seeing the same > issue I don't think the values are intended to be Broadcom-specific. Right, this is not for Broadcom. I found out that this is something Intel specifies as part of the entire platform's ACPI recommendations. > On Wed, 2016-10-12 at 17:21 +0300, Luca Coelho wrote: > > And, the values in the SPLX structs are being changed here, to DOM1, > > LIM1, TIM1 etc., before being returned. Â This also matches your > > description that, at runtime, you got something different than the pure > > dump. Â If you follow these DOM*, LIM*, TIM* symbols, you'll probably > > end up getting the values you observed at runtime. > > > Probably not important, but it seems that there is some additional > indirection. The only values I'm seeing associated with those symbols > are 8 and 16: > > $ grep -e 'DOM[0-9]' -e 'LIM[0-9]' -e 'TIM[0-9]' dsdt.dsl | grep -v Store > DOM1, 8, > LIM1, 16, > TIM1, 16, > DOM2, 8, > LIM2, 16, > TIM2, 16, > DOM3, 8, > LIM3, 16, > TIM3, 16, Yeah, there are often many levels of indirection. These settings can be also tied to the configuration that is reachable by the user in the BIOS setup, so you never know. The easiest way is probably to run the ASL with acpiexec and execute the method... > > I'll send you a patch for testing soon. > > > I will keep an eye on the list archive, thanks! I'll ping you from my Intel address and provide you with a link to the patch, to make it easier for you. ;) -- Cheers, Luca.
Re: [PATCH] iwlwifi: pcie: reduce "unsupported splx" to a warning
On Wed, 2016-10-12 at 14:36 +0200, Paul Bolle wrote:
> On Wed, 2016-10-12 at 15:24 +0300, Luca Coelho wrote:
> > Okay... Actually this is a structure in the BIOS and the actual method
> > we call is SPLC. The SPLC method may return one item from this table,
> > or something entirely different, possible one of the three values
> > depending on a configuration option or so.
> >
> > Can you to find and send me the actual SPLC method that we call, from
> > your BIOS?
>
>
> It seems Chris and I basically have identical setups, so I'll answer.
Thanks! Yeah, I implied any of you two. ;)
> There are 20 SPLC methods in the BIOS. The first reads
> Method (SPLC, 0, Serialized)
> {
> DerefOf (SPLX [One]) [Zero] = DOM1 /* \DOM1 */
> DerefOf (SPLX [One]) [One] = LIM1 /* \LIM1 */
> DerefOf (SPLX [One]) [0x02] = TIM1 /* \TIM1 */
> DerefOf (SPLX [0x02]) [Zero] = DOM2 /* \DOM2 */
> DerefOf (SPLX [0x02]) [One] = LIM2 /* \LIM2 */
> DerefOf (SPLX [0x02]) [0x02] = TIM2 /* \TIM2 */
> DerefOf (SPLX [0x03]) [Zero] = DOM3 /* \DOM3 */
> DerefOf (SPLX [0x03]) [One] = LIM3 /* \LIM3 */
> DerefOf (SPLX [0x03]) [0x02] = TIM3 /* \TIM3 */
> Return (SPLX) /* \_SB_.PCI0.RP01.PXSX.SPLX */
> }
>
> The only difference is in the last comment. Ie, RP01 is increased until
> it reaches RP20. (The machine has 20 PCI devices according to lspci. I
> have no clue how to match that RPxx number to the 20 devices showing up
> in lspci, sorry.)
No problem, these BIOSes are usually quite cryptic. :) But what you're
saying makes sense. They have added the SPLC method to all PCI root-
ports (which is what RP stands for here).
And, the values in the SPLX structs are being changed here, to DOM1,
LIM1, TIM1 etc., before being returned. This also matches your
description that, at runtime, you got something different than the pure
dump. If you follow these DOM*, LIM*, TIM* symbols, you'll probably
end up getting the values you observed at runtime.
Basically this tells me that indeed 3 "structs" are being returned (as
your dumps already showed). And, according to the specs that I found
(which unfortunately are confidential, so I can't share) this is
correct and the driver code is broken.
I'll send you a patch for testing soon.
Thanks for all the help!
--
Cheers,
Luca.
Re: [PATCH] iwlwifi: pcie: reduce "unsupported splx" to a warning
On Tue, 2016-10-11 at 23:32 -0500, Chris Rorvick wrote:
> On Tue, Oct 11, 2016 at 5:11 AM, Paul Bolle wrote:
> > For what it's worth, on my machine I have twenty (!) SPLX entries, all
> > reading:
> > Name (SPLX, Package (0x04)
> > {
> > Zero,
> > Package (0x03)
> > {
> > 0x8000,
> > 0x8000,
> > 0x8000
> > },
> >
> > Package (0x03)
> > {
> >0x8000,
> >0x8000,
> >0x8000
> > },
> >
> > Package (0x03)
> > {
> > 0x8000,
> > 0x8000,
> > 0x8000
> > }
> > })
>
>
> I actually see exactly the same on my Dell XPS 13 (9350) when I use
> acpidump, etc. I typed the entry I included in the commit log by hand
> based on what the driver gets back from the SPLC method (I added a
> function to dump the returned object.)
Okay... Actually this is a structure in the BIOS and the actual method
we call is SPLC. The SPLC method may return one item from this table,
or something entirely different, possible one of the three values
depending on a configuration option or so.
Can you to find and send me the actual SPLC method that we call, from
your BIOS?
--
Cheers,
Luca.
Re: [PATCH] iwlwifi: pcie: reduce "unsupported splx" to a warning
Hi Chris, On Tue, 2016-10-11 at 09:09 -0500, Chris Rorvick wrote: > On Tue, Oct 11, 2016 at 5:11 AM, Paul Bolle wrote: > > > This is not coming from the NIC itself, but from the platform's ACPI > > > tables. Can you tell us which platform you are using? > > > Interesting. I'm running a Dell XPS 13 9350. I replaced the > factory-provided Broadcom card with an AC 8260. I can update the > commit log to reflect this. Okay, so this makes sense. Those entries are probably formatted for the Broadcom card, which the iwlwifi driver obviously doesn't understand. The best we can do, as I already said, is to ignore values we don't understand. I will also check what is the correct procedure in such cases, because it is possible, in theory, that the format *matches* but applies only to another device. > > > If this is really bothering you, I guess I could apply this patch for > > > now. But as I said, this is not solving the actual problem. > > > > > > Bikeshedding: I think IWL_INFO() is more appropriate, as info doesn't > > imply one needs to act on this message, while warn does imply that > > action is needed. > > > Agreed. I still think making this a warning is appropriate, but it > seems pretty clear this is not an error. This has nothing to do with > how much it bothers me. An error tells the user something needs to be > fixed, but in this case the interface is working fine. Making it a > warning with an improved message will result in fewer people wasting > their time. Yes, so I'll try to stop wasting people's timing by trying to do the correct thing without bothering the user at all. :) Thanks for pointing this all out! -- Cheers, Luca.
Re: [PATCH] iwlwifi: pcie: reduce "unsupported splx" to a warning
Hi Paul,
On Tue, 2016-10-11 at 12:11 +0200, Paul Bolle wrote:
> On Mon, 2016-10-10 at 17:02 +0300, Luca Coelho wrote:
> > On Mon, 2016-10-10 at 02:19 -0500, Chris Rorvick wrote:
> > This is not coming from the NIC itself, but from the platform's ACPI
> > tables. Can you tell us which platform you are using?
>
>
> On my machine I'm seeing the same error as Chris. So what exactly do
> you mean with "platform" here?
By "platform" I meant the PC you are using. The ACPI table is created
by the OEM, so different PCs have different tables.
>
> > > Name (SPLX, Package (0x04)
> > > {
> > > Zero,
> > > Package (0x03)
> > > {
> > > 0,
> > > 1200,
> > > 1000
> > > },
> > > Package (0x03)
> > > {
> > > 0,
> > > 1200,
> > > 1000
> > > },
> > > Package (0x03)
> > > {
> > > 0,
> > > 1200,
> > > 1000
> > > }
> > > })
> >
> >
> > This is not the structure that we are expecting. We expect this:
> >
> > Name (SPLX, Package (0x02)
> > {
> > Zero,
> > Package (0x03)
> > {
> > 0x07,
> > ,
> >
> > }
> > })
> >
> > ...as you correctly pointed out. The data in the structure you have is
> > not for WiFi (actually I don't think 0 is a valid value, but I'll
> > double-check).
>
>
> For what it's worth, on my machine I have twenty (!) SPLX entries, all
> reading:
> Name (SPLX, Package (0x04)
> {
> Zero,
> Package (0x03)
> {
> 0x8000,
> 0x8000,
> 0x8000
> },
>
> Package (0x03)
> {
>0x8000,
>0x8000,
>0x8000
> },
>
> Package (0x03)
> {
> 0x8000,
> 0x8000,
> 0x8000
> }
> })
Thanks. So this is another case where the first value doesn't match
what we are expecting and we should just ignore that.
> > There are other things that look a bit inconsistent in this code...
> > I'll try to find the official ACPI table definitions for this entries
> > to make sure it's correct.
>
>
> When I looked into this error, some time ago, I searched around a bit
> for documentation on this splx stuff. Sadly, commit bcb079a14d75
> ("iwlwifi: pcie: retrieve and parse ACPI power limitations") provides
> very few clues and my searches turned up nothing useful. So a pointer
> or two would be really appreciated.
Yeah, I looked into that commit too and there's not much there. I'll
try to find the documentation and, if I can, I'll share it with you.
> > > --- a/drivers/net/wireless/intel/iwlwifi/pcie/drv.c
> > > +++ b/drivers/net/wireless/intel/iwlwifi/pcie/drv.c
> > > @@ -540,7 +540,7 @@ static u64 splx_get_pwr_limit(struct iwl_trans
> > > *trans, union acpi_object *splx)
> > > splx->package.count != 2 ||
> > > splx->package.elements[0].type != ACPI_TYPE_INTEGER ||
> > > splx->package.elements[0].integer.value != 0) {
> > > - IWL_ERR(trans, "Unsupported splx structure\n");
> > > + IWL_WARN(trans, "Unsupported splx structure, not limiting WiFi
> > > power\n");
> > > return 0;
> > > }
> >
> >
> > If this is really bothering you, I guess I could apply this patch for
> > now. But as I said, this is not solving the actual problem.
>
>
> Bikeshedding: I think IWL_INFO() is more appropriate, as info doesn't
> imply one needs to act on this message, while warn does imply that
> action is needed.
Right, but in fact, the code considers that if the SPLX method exists,
it must return a value iwlwifi can understand, thus the error. That
assumption is wrong, so we should just ignore entries that don't match
and continue without printing anything out (as would happen if the splx
method were not even there).
In any case, I will rework this code, so I'd prefer if we skip this
patch entirely.
--
Cheers,
Luca.
Re: [PATCH] iwlwifi: pcie: reduce "unsupported splx" to a warning
Hi,
On Mon, 2016-10-10 at 02:19 -0500, Chris Rorvick wrote:
> Commit bcb079a14d75 ("iwlwifi: pcie: retrieve and parse ACPI power
> limitations") looks for a specific structure in the ACPI tables for
> setting the default power limit. The data returned for at least some
> dual band chipsets is not recognized, though. For example, the AC 8260
> reports the following:
This is not coming from the NIC itself, but from the platform's ACPI
tables. Can you tell us which platform you are using?
> Name (SPLX, Package (0x04)
> {
> Zero,
> Package (0x03)
> {
> 0,
> 1200,
> 1000
> },
> Package (0x03)
> {
> 0,
> 1200,
> 1000
> },
> Package (0x03)
> {
> 0,
> 1200,
> 1000
> }
> })
This is not the structure that we are expecting. We expect this:
Name (SPLX, Package (0x02)
{
Zero,
Package (0x03)
{
0x07,
,
}
})
...as you correctly pointed out. The data in the structure you have is
not for WiFi (actually I don't think 0 is a valid value, but I'll
double-check).
> The current logic expects exactly two elements in the outer package,
> causing the above to be ignored and the power limit unset.
>
> Despite the interface being fully functional after initialization, the
> above condition is reported as an error. Knock the message down to a
> warning and provide better context for understanding its consequence.
Reducing this to a warning is an easy way to reduce the verbosity of
the problem, but I think the correct thing to do would be to accept
multiple entries and ignore the ones that don't have the WIFI marker.
And only type-check the WIFI ones.
There are other things that look a bit inconsistent in this code...
I'll try to find the official ACPI table definitions for this entries
to make sure it's correct.
> Signed-off-by: Chris Rorvick
> ---
> drivers/net/wireless/intel/iwlwifi/pcie/drv.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/drv.c
> b/drivers/net/wireless/intel/iwlwifi/pcie/drv.c
> index 78cf9a7..19b531f 100644
> --- a/drivers/net/wireless/intel/iwlwifi/pcie/drv.c
> +++ b/drivers/net/wireless/intel/iwlwifi/pcie/drv.c
> @@ -540,7 +540,7 @@ static u64 splx_get_pwr_limit(struct iwl_trans *trans,
> union acpi_object *splx)
> splx->package.count != 2 ||
> splx->package.elements[0].type != ACPI_TYPE_INTEGER ||
> splx->package.elements[0].integer.value != 0) {
> - IWL_ERR(trans, "Unsupported splx structure\n");
> + IWL_WARN(trans, "Unsupported splx structure, not limiting WiFi
> power\n");
> return 0;
> }
If this is really bothering you, I guess I could apply this patch for
now. But as I said, this is not solving the actual problem.
--
Cheers,
Luca.
Re: [PATCH RESEND] iwlwifi, Do not implement thermal zone unless ucode is loaded
On Wed, 2016-07-13 at 09:50 +0300, Kalle Valo wrote: > Prarit Bhargava writes: > > > > We implement thermal zone because we do support it, but the > > > problem is > > > that we need the firmware to be loaded for that. So you can argue > > > that > > > we should register *later* when the firmware is loaded. But this > > > is > > > really not helping all that much because the firmware can also be > > > stopped at any time. So you'd want us to register / unregister > > > the > > > thermal zone anytime the firmware is loaded / unloaded? > > > > You might have to do that. I think that if the firmware enables a > > feature then > > the act of loading the firmware should run the code that enables > > the feature. > > IMO of course. > > But I suspect that the iwlwifi firmware is loaded during interface up > (and unloaded during interface down) and in that case > register/unregister would be happening all the time. That doesn't > sound > like a good idea. I would rather try to fix the thermal interface to > handle the cases when the measurement is not available. I totally agree with Emmanuel and Kalle. We should not change this. It is a design decision to return an error when the interface is down, this is very common with other subsystems as well. The userspace should be able to handle errors and report something like "unavailable" when this kind of error is returned. I'm not sure EIO is the best we can have, but for me that's exactly what it is. The thermal zone *is* there, but cannot be accessed because the firmware is not available. I'm okay to change it to EBUSY, if that would help userspace, but I think that's a bit misleading. The device is not busy, on the contrary, it's not even running at all. Furthermore, I don't think this is "breaking userspace" in the sense of being a regression. The userspace API has always been implemented with the possibility of returning errors. It's not a good design if a single device returning an error causes all the other devices to also fail. -- Cheers, Luca.
Re: [PATCH] iwlwifi: add missing type declaration
On Mon, 2016-07-11 at 22:49 +0200, Arnd Bergmann wrote:
> The iwl-debug.h header relies in implicit inclusion of linux/device.h
> and
> we get a lot of warnings without that:
>
> drivers/net/wireless/intel/iwlwifi/iwl-debug.h:44:23: error: 'struct
> device' declared inside parameter list will not be visible outside of
> this definition or declaration [-Werror]
> void __iwl_err(struct device *dev, bool rfkill_prefix, bool
> only_trace,
> ^~
> In file included from drivers/net/wireless/intel/iwlwifi/iwl-eeprom-
> read.h:66:0,
> from drivers/net/wireless/intel/iwlwifi/iwl-eeprom-
> read.c:68:
> drivers/net/wireless/intel/iwlwifi/iwl-trans.h: In function
> 'iwl_trans_tx':
> drivers/net/wireless/intel/iwlwifi/iwl-trans.h:1030:348: error:
> passing argument 1 of '__iwl_err' from incompatible pointer type [-
> Werror=incompatible-pointer-types]
> IWL_ERR(trans, "%s bad state = %d\n", __func__, trans->state);
>
>
>
>
>
> ^
> In file included from drivers/net/wireless/intel/iwlwifi/iwl-eeprom-
> read.c:67:0:
> drivers/net/wireless/intel/iwlwifi/iwl-debug.h:44:6: note: expected
> 'struct device *' but argument is of type 'struct device *'
> void __iwl_err(struct device *dev, bool rfkill_prefix, bool
> only_trace,
> ^
>
> The easiest workaround is to just declare 'struct device' before its
> first use,
> rather than including the entire header file.
>
> Signed-off-by: Arnd Bergmann
> Fixes: 21cb3222fe56 ("iwlwifi: decouple PCIe transport from
> mac80211")
> ---
Acked-by: Luca Coelho
Agree with Kalle that he will take this directly to wireless-drivers-
next.
--
Cheers,
Luca.
Re: [PATCH] iwlwifi: Remove unused array 'iwlagn_loose_lookup'
On Fri, 2016-06-03 at 14:39 -0700, Guenter Roeck wrote: > gcc-6 reports the following error if -Werror=unused-const-variable > is enabled. > > drivers/net/wireless/intel/iwlwifi/dvm/lib.c:210:21: error: > 'iwlagn_loose_lookup' defined but not used > > Signed-off-by: Guenter Roeck > --- Thanks! I'm queuing this via our internal trees (adding "dvm: " to the subject, to indicate this is related to the dvm module). -- Cheers, Luca.
[PATCH] iwlwifi: mvm: fix merge damage in tx.c
From: Luca Coelho
During the merge in commit 909b27f70643 ("Merge
git://git.kernel.org/pub/scm/linux/kernel/git/davem/net"), there was a
small merge damage where one instance of info was not converted into
skb_info. Fix this.
Signed-off-by: Luca Coelho
---
drivers/net/wireless/intel/iwlwifi/mvm/tx.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/tx.c
b/drivers/net/wireless/intel/iwlwifi/mvm/tx.c
index 8802109..c53aa0f 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/tx.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/tx.c
@@ -211,6 +211,7 @@ void iwl_mvm_set_tx_cmd(struct iwl_mvm *mvm, struct sk_buff
*skb,
struct iwl_tx_cmd *tx_cmd,
struct ieee80211_tx_info *info, u8 sta_id)
{
+ struct ieee80211_tx_info *skb_info = IEEE80211_SKB_CB(skb);
struct ieee80211_hdr *hdr = (void *)skb->data;
__le16 fc = hdr->frame_control;
u32 tx_flags = le32_to_cpu(tx_cmd->tx_flags);
@@ -294,7 +295,7 @@ void iwl_mvm_set_tx_cmd(struct iwl_mvm *mvm, struct sk_buff
*skb,
tx_cmd->tx_flags = cpu_to_le32(tx_flags);
/* Total # bytes to be transmitted */
tx_cmd->len = cpu_to_le16((u16)skb->len +
- (uintptr_t)info->driver_data[0]);
+ (uintptr_t)skb_info->driver_data[0]);
tx_cmd->life_time = cpu_to_le32(TX_CMD_LIFE_TIME_INFINITE);
tx_cmd->sta_id = sta_id;
--
2.8.1
Re: [PATCH] iwlwifi: Deinline iwl_write8/write32/read32()
Hi Denys, On Thu, 2015-09-24 at 16:58 +0200, Denys Vlasenko wrote: > With this .config: http://busybox.net/~vda/kernel_config_ALLYES_Os, > after deinlining these functions have sizes and callsite counts > as follows: > > iwl_write8: 315 bytes, 3 calls > iwl_write32: 296 bytes, 90 calls > iwl_read32: 278 bytes, 51 calls > > Total size reduction is about 40 kbytes. > > Signed-off-by: Denys Vlasenko > CC: Johannes Berg > CC: Emmanuel Grumbach > CC: Intel Linux Wireless > CC: Gregory Greenman > CC: John Linville > CC: linux-wirel...@vger.kernel.org > CC: linux-kernel@vger.kernel.org Emmanuel already wrote and applied a patch like this. Check here: http://mid.gmane.org/1442869141-31637-4-git-send-email-l...@coelho.fi Thanks for your patch anyway and for reporting this. :) -- Cheers, Luca. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Input: ALPS - fix max coordinates for v5 and v7 protocols
On Sat, 2015-03-21 at 20:36 -0700, Dmitry Torokhov wrote:
> Commit 3296f71cd2fde7a2ad52e66a27eae419f6328066 ("Input: ALPS - consolidate
> setting protocol parameters") inadvertently moved call to
> alps_dolphin_get_device_area() from v5 to v7 protocol, causing both
> protocols report incorrect maximum values for X and Y axes which resulted
> in crash in Synaptics X driver.
>
> Reported-by: Santiago Gala
> Reported-by: Pali Rohár
> Signed-off-by: Dmitry Torokhov
> ---
Tested-by: Luciano Coelho
Tested on a Dell E5250.
Are you planning to send this to 4.0-rc*? That would be nice, since this
is a regression from 3.19.
--
Cheers,
Luca.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] compiler.h: don't use temporary variable in __compiletime_assert()
On Thu, 2014-05-08 at 08:31 +0200, Johannes Berg wrote: > From: Johannes Berg > > Usually, BUG_ON and friends aren't even evaluated in sparse, but > recently compiletime_assert_atomic_type() was added, and that now > results in a sparse warning every time it is used. > > The reason turns out to be the temporary variable, after it sparse > no longer considers the value to be a constant, and results in a > warning and an error. The error is the more annoying part of this > as it suppresses any further warnings in the same file, hiding > other problems. > > Since this is all about compile time and the condition should be > side-effect free to start with, there's no downside (apart maybe > from a slight compilation time penalty?) to just duplicating it, > leaving sparse able to evaluate it at check time, getting rid of > the warning and error. > > Signed-off-by: Johannes Berg > --- Tested-by: Luciano Coelho -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Drivers:net:wireless:ti:wl1251: Fixed Sparse invalid assignment warning
On Tue, 2014-03-04 at 00:18 -0800, Surendra Patil wrote: > Sparse warns about invalid assignment in > drivers/net/wireless/ti/wl1251/cmd.c:451:42: warning: invalid assignment: |= > drivers/net/wireless/ti/wl1251/cmd.c:451:42:left side has type restricted > __le16 > drivers/net/wireless/ti/wl1251/cmd.c:451:42:right side has type int > Hence type converted right side to __le16. > > Signed-off-by: Surendra Patil > --- > drivers/net/wireless/ti/wl1251/cmd.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/wireless/ti/wl1251/cmd.c > b/drivers/net/wireless/ti/wl1251/cmd.c > index 223649b..bf1fa18 100644 > --- a/drivers/net/wireless/ti/wl1251/cmd.c > +++ b/drivers/net/wireless/ti/wl1251/cmd.c > @@ -448,7 +448,7 @@ int wl1251_cmd_scan(struct wl1251 *wl, u8 *ssid, size_t > ssid_len, >* Note: This bug may be caused by the fw's DTIM handling. >*/ > if (is_zero_ether_addr(wl->bssid)) > - cmd->params.scan_options |= WL1251_SCAN_OPT_PRIORITY_HIGH; > + cmd->params.scan_options |= > cpu_to_le16(WL1251_SCAN_OPT_PRIORITY_HIGH); > cmd->params.num_channels = n_channels; > cmd->params.num_probe_requests = n_probes; > cmd->params.tx_rate = cpu_to_le16(1 << 1); /* 2 Mbps */ AFAIR wl1251 is not really endianess-safe. I think there are lots of other places where endianess macros should be added. Nothing wrong with this patch, I'm just saying that while at it, maybe someone could take care of all the endianess issues with wl1251? ;) -- Luca. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 0/8] wilink: add device tree support
On Fri, 2014-02-28 at 14:29 +0200, Luca Coelho wrote: > On Fri, 2014-02-28 at 08:26 +0100, Yegor Yefremov wrote: > > On Tue, Jul 30, 2013 at 3:04 PM, Luciano Coelho wrote: > > > Hi, > > > > > > This patch series adds device tree support to the wlcore_sdio driver, > > > which is used by WiLink6, WiLink7 and WiLink8. > > > > > > The first patches do some clean-up to make the data needed in the > > > wilink device tree node smaller. The remaining patches implement the > > > actual device tree node parsing in wlcore_sdio. > > > > > > Regarding the XTAL clock issues, for now we don't support XTAL mode > > > with DT, but I have sent a proposal for a small change in the clock > > > framework to support this, but it's still under discussions [1]. > > > > > > The DTS file changes will be sent separately, since they need to go > > > via different trees. > > > > > > A new version of the bindings documentation has been sent [2] and, if > > > no more comments are given to it, I'll apply it via my tree. > > > > > > [1] > > > http://news.gmane.org/find-root.php?message_id=1372971912-10877-1-git-send-email-coe...@ti.com > > > [2] > > > http://news.gmane.org/find-root.php?message_id=1375109728-5931-1-git-send-email-coe...@ti.com > > > > > > Changes in v4: > > > > > > * Rebased on top of 3.11-rc3 (eg. no more changes on the board files > > > that were removed); > > > > > > * Use the new irq_get_trigger_type() instead of > > > irqd_get_trigger_type() (thanks Javier); > > > > > > * Added some missing const's (thanks Felipe); > > > > > > * Reverted Tony's workaround to get WiLink to work on Panda while DT > > > was not supported yet. > > > > > > > > > Please review. > > > > What is the state of the series? Who is now responsible for the > > patches? What issues were still not handled? > > There were some comments about this series (or more precisely, the one > that added the bindings documentation[1]). Unfortunately, I have left > TI since and have no time to handle them. Feel free to pick it up and > continue from there. > > [1] https://lkml.org/lkml/2013/7/30/712 There's also this thread, which I'm not sure has been resolved: http://thread.gmane.org/gmane.linux.kernel/1520752 -- Luca. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 0/8] wilink: add device tree support
On Fri, 2014-02-28 at 08:26 +0100, Yegor Yefremov wrote: > On Tue, Jul 30, 2013 at 3:04 PM, Luciano Coelho wrote: > > Hi, > > > > This patch series adds device tree support to the wlcore_sdio driver, > > which is used by WiLink6, WiLink7 and WiLink8. > > > > The first patches do some clean-up to make the data needed in the > > wilink device tree node smaller. The remaining patches implement the > > actual device tree node parsing in wlcore_sdio. > > > > Regarding the XTAL clock issues, for now we don't support XTAL mode > > with DT, but I have sent a proposal for a small change in the clock > > framework to support this, but it's still under discussions [1]. > > > > The DTS file changes will be sent separately, since they need to go > > via different trees. > > > > A new version of the bindings documentation has been sent [2] and, if > > no more comments are given to it, I'll apply it via my tree. > > > > [1] > > http://news.gmane.org/find-root.php?message_id=1372971912-10877-1-git-send-email-coe...@ti.com > > [2] > > http://news.gmane.org/find-root.php?message_id=1375109728-5931-1-git-send-email-coe...@ti.com > > > > Changes in v4: > > > > * Rebased on top of 3.11-rc3 (eg. no more changes on the board files > > that were removed); > > > > * Use the new irq_get_trigger_type() instead of > > irqd_get_trigger_type() (thanks Javier); > > > > * Added some missing const's (thanks Felipe); > > > > * Reverted Tony's workaround to get WiLink to work on Panda while DT > > was not supported yet. > > > > > > Please review. > > What is the state of the series? Who is now responsible for the > patches? What issues were still not handled? There were some comments about this series (or more precisely, the one that added the bindings documentation[1]). Unfortunately, I have left TI since and have no time to handle them. Feel free to pick it up and continue from there. [1] https://lkml.org/lkml/2013/7/30/712 -- Cheers, Luca. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ieee80211: Print human-readable disassoc/deauth reason codes
On Fri, 2014-02-07 at 09:46 -0600, Larry Finger wrote: > On 02/07/2014 06:53 AM, Kalle Valo wrote: > > Johannes Berg writes: > > > >> On Wed, 2014-02-05 at 19:44 -0600, Calvin Owens wrote: > >>> Create a function to return a descriptive string for each reason code, > >>> and print that instead of the numeric value in the kernel log. These > >>> codes are easily found on popular search engines, but one is generally > >>> not able to access the internet when dealing with wireless connectivity > >>> issues. > >> > >> I believe both iw and wpa_supplicant already have the reason code > >> printout, and if you're diagnosing connectivity issues then you're > >> probably using those anyway (e.g. iw event -t), so I don't really see > >> much point in adding this to the kernel? > > > > FWIW I find this useful. When I have connection problems I rarely look > > at wpasupplicant, mostly I'm so lazy that I just check from the kernel > > log what's happening. Just my 0.02 EUR. > > I would also find it useful. I assist with wireless problems on the openSUSE > Forum, and there we are lucky if we get dmesg output. When we do and it > contains > a deauthentication reason, I always need to bring up a web page to interpret > the > output. With this change, one step could be skipped. But is it worth putting this parsing in the *kernel*? I mean, if anyone is interested enough in the problem, a simple google query is not that hard, right? If this happens to you "on-the-fly" and all you want is your connection to work, you won't start debugging right away. You'll (maybe) save the kernel logs, try to reconnect and debug later, at home. If you think you *can* fix the problem on-the-fly, but you can't google because the connection doesn't work, you certainly have either the IEEE 802.11 specs or the kernel sources somewhere in your device. :) -- Luca. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] clk: add flags to distinguish xtal clocks
On Mon, 2013-11-11 at 21:59 +0100, Maxime Ripard wrote: > Hi Luca, > > On Mon, Nov 11, 2013 at 09:50:56PM +0200, Luca Coelho wrote: > > On Mon, 2013-11-11 at 13:42 -0600, Felipe Balbi wrote: > > > > > + if (of_property_read_bool(node, "clock-xtal")) > > > > > + flags |= CLK_IS_TYPE_XTAL; > > > > > + > > > > > > > > Introducing a new compatible instead of a property would make more > > > > sense here I think. > > > > > > > > Do you have a reason not to do so? > > > > > > As you can see, this is original work from Luca but I disagree that > > > adding a new compatible makes more sense. This still related to a fixed > > > rate clock, we're just giving it one extra metadata which willAnd t > > > differentiate between crystal and oscilator fixed rate clocks. > > > > I agree with Felipe. This was discussed before [1]. While still at TI, > > I tried to figure out the exact need for the firmware to know whether it > > was an oscillator or not. It was mostly because the stabilization time > > and such things differ with oscillators, but I wasn't able to find out > > how exactly this affected things. > > > > In any case, as I concluded earlier (but it's not really my call), being > > a crystal or an oscillator *is* a characteristic of the hardware, > > regardless of whether that information is useful or not. In the WiLink > > case it is, at least it can differentiate the clocks that are used in > > the HW modules it uses. > > > > So IMHO it doesn't really hurt and it's not really against the DT > > principles. > > Just to be clear, I'm not against your patch. If you need this to make > your driver work, then it's fine for me. Mike will probably know > better if we actually need some extra metadata. :) I understand, we should really try to make this as clean as possible, DT should really be a good description of the hardware. > What I'm not really convinced about is *how* you carry that metadata > in the DT, that's all, nothing more. Okay, I get you. My point is that being a crystal or not *is* a characteristic of the clock, so I think it could be part of the flags that describe it. In any case, it's not really my call. This is about the clock and it's not even my home turf (wireless). ;) Thanks for your comments. And I'm sorry if the tone of my previous email sounded harsh, it was not supposed to. :) -- Cheers, Luca. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] clk: add flags to distinguish xtal clocks
On Mon, 2013-11-11 at 13:42 -0600, Felipe Balbi wrote:
> Hi,
>
> On Sun, Nov 10, 2013 at 12:37:16PM +0100, Maxime Ripard wrote:
> > Hi Felipe,
> >
> > On Fri, Nov 08, 2013 at 12:00:48PM -0600, Felipe Balbi wrote:
> > > From: Luciano Coelho
> > >
> > > Add a flag that indicate whether the clock is a crystal or not.
> > >
> > > Additionally, parse a new device tree binding in clk-fixed-rate to set
> > > this flag.
> > >
> > > If clock-xtal isn't set, the clock framework will assume clock to be
> > > generated by an oscillator. There's only one user for this binding
> > > right now which is Texas Instruments' WiLink devices which need to know
> > > details about the clock in order to initialize the underlying WiFi HW
> > > correctly.
> > >
> > > Signed-off-by: Luciano Coelho
> > > Signed-off-by: Felipe Balbi
> > > ---
> > >
> > > Dropped CLK_IS_TYPE_DEFINED flag and just assume that if the flag
> > > isn't there, default behavior will be taken.
> > >
> > > Documentation/devicetree/bindings/clock/fixed-clock.txt | 1 +
> > > drivers/clk/clk-fixed-rate.c| 6 +-
> > > include/linux/clk-provider.h| 1 +
> > > 3 files changed, 7 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/clock/fixed-clock.txt
> > > b/Documentation/devicetree/bindings/clock/fixed-clock.txt
> > > index 0b1fe78..3036dfe 100644
> > > --- a/Documentation/devicetree/bindings/clock/fixed-clock.txt
> > > +++ b/Documentation/devicetree/bindings/clock/fixed-clock.txt
> > > @@ -12,6 +12,7 @@ Required properties:
> > > Optional properties:
> > > - gpios : From common gpio binding; gpio connection to clock enable pin.
> > > - clock-output-names : From common clock binding.
> > > +- clock-xtal: true when a clock is provided by a crystal
> > >
> > > Example:
> > > clock {
> > > diff --git a/drivers/clk/clk-fixed-rate.c b/drivers/clk/clk-fixed-rate.c
> > > index 1ed591a..5db9bf0 100644
> > > --- a/drivers/clk/clk-fixed-rate.c
> > > +++ b/drivers/clk/clk-fixed-rate.c
> > > @@ -91,13 +91,17 @@ void of_fixed_clk_setup(struct device_node *node)
> > > struct clk *clk;
> > > const char *clk_name = node->name;
> > > u32 rate;
> > > + unsigned long flags = CLK_IS_ROOT;
> > >
> > > if (of_property_read_u32(node, "clock-frequency", &rate))
> > > return;
> > >
> > > + if (of_property_read_bool(node, "clock-xtal"))
> > > + flags |= CLK_IS_TYPE_XTAL;
> > > +
> >
> > Introducing a new compatible instead of a property would make more
> > sense here I think.
> >
> > Do you have a reason not to do so?
>
> As you can see, this is original work from Luca but I disagree that
> adding a new compatible makes more sense. This still related to a fixed
> rate clock, we're just giving it one extra metadata which willAnd t
> differentiate between crystal and oscilator fixed rate clocks.
I agree with Felipe. This was discussed before [1]. While still at TI,
I tried to figure out the exact need for the firmware to know whether it
was an oscillator or not. It was mostly because the stabilization time
and such things differ with oscillators, but I wasn't able to find out
how exactly this affected things.
In any case, as I concluded earlier (but it's not really my call), being
a crystal or an oscillator *is* a characteristic of the hardware,
regardless of whether that information is useful or not. In the WiLink
case it is, at least it can differentiate the clocks that are used in
the HW modules it uses.
So IMHO it doesn't really hurt and it's not really against the DT
principles.
[1] https://lkml.org/lkml/2013/7/29/321
--
Cheers,
Luca.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] clk: add flags to distinguish xtal clocks
Hi Felipe, On Fri, 2013-11-08 at 12:00 -0600, Felipe Balbi wrote: > From: Luciano Coelho > > Add a flag that indicate whether the clock is a crystal or not. > > Additionally, parse a new device tree binding in clk-fixed-rate to set > this flag. > > If clock-xtal isn't set, the clock framework will assume clock to be > generated by an oscillator. There's only one user for this binding > right now which is Texas Instruments' WiLink devices which need to know > details about the clock in order to initialize the underlying WiFi HW > correctly. > > Signed-off-by: Luciano Coelho > Signed-off-by: Felipe Balbi > --- > > Dropped CLK_IS_TYPE_DEFINED flag and just assume that if the flag > isn't there, default behavior will be taken. I don't think this is a good idea, because how would code that relies on the CLK_IS_TYPE_XTAL flag know whether 0 means that the clock is *not* crystal or if the flag is simply not defined? There are many clocks which *are* crystal but don't define this flag (obviously, because it didn't exist). That's the reason why I added the CLK_IS_TYPE_DEFINED flag... If you want to get rid of this flag, you should traverse all clocks and set the CLK_IS_TYPE_XTAL on all those that are crystals. -- Cheers, Luca. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 03/16] wl1251: add sysfs interface for bluetooth coexistence mode configuration
On Mon, 2013-10-28 at 23:39 +, Ben Hutchings wrote: > On Sat, 2013-10-26 at 22:34 +0200, Pali Rohár wrote: > > From: David Gnedt > > > > Port the bt_coex_mode sysfs interface from wl1251 driver version included > > in the Maemo Fremantle kernel to allow bt-coexistence mode configuration. > > This enables userspace applications to set one of the modes > > WL1251_BT_COEX_OFF, WL1251_BT_COEX_ENABLE and WL1251_BT_COEX_MONOAUDIO. > > The default mode is WL1251_BT_COEX_OFF. > > It should be noted that this driver always enabled bt-coexistence before > > and enabled bt-coexistence directly affects the receiving performance, > > rendering it unusable in some low-signal situations. Especially monitor > > mode is affected very badly with bt-coexistence enabled. > [...] > > This should be implemented consistently with other drivers: > > drivers/net/wireless/ath/ath9k/htc_drv_init.c:module_param_named(btcoex_enable, > ath9k_htc_btcoex_enable, int, 0444); > drivers/net/wireless/ath/ath9k/init.c:module_param_named(btcoex_enable, > ath9k_btcoex_enable, int, 0444); > drivers/net/wireless/b43/main.c:module_param_named(btcoex, modparam_btcoex, > int, 0444); > drivers/net/wireless/ipw2x00/ipw2200.c:module_param(bt_coexist, int, 0444); > drivers/net/wireless/iwlegacy/common.c:module_param(bt_coex_active, bool, > S_IRUGO); > drivers/net/wireless/iwlwifi/iwl-drv.c:module_param_named(bt_coex_active, > iwlwifi_mod_params.bt_coex_active, > drivers/net/wireless/ti/wlcore/sysfs.c:static DEVICE_ATTR(bt_coex_state, > S_IRUGO | S_IWUSR, > > Oh, hmm, I see a problem here. With so many drivers doing the same thing, isn't it about time to add this to nl80211? -- Luca. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] clk: add flags to distinguish xtal clocks
On Wed, 2013-10-23 at 02:24 -0700, Mike Turquette wrote: > Quoting Luca Coelho (2013-10-16 03:24:27) > > Hi, > > > > Sorry for the delayed response. > > > > On Tue, 2013-10-08 at 10:27 -0500, Felipe Balbi wrote: > > > Fixing Luca's address since he left TI > > > > Thanks, Felipe! I wouldn't have seen this otherwise. > > > > > > > On Mon, Oct 07, 2013 at 12:44:24AM -0700, Mike Turquette wrote: > > > > Quoting Luciano Coelho (2013-07-29 06:50:42) > > > > > Hi Mike, > > > > > > > > > > On Fri, 2013-07-05 at 10:54 +0300, Luciano Coelho wrote: > > > > > > On Thu, 2013-07-04 at 16:19 -0700, Mike Turquette wrote: > > > > > > > Quoting Luciano Coelho (2013-07-04 15:37:45) > > > > > > > > On Thu, 2013-07-04 at 15:25 -0700, Mike Turquette wrote: > > > > > > > > > Or is it the same clock input and basically the problem is > > > > > > > > > that you need > > > > > > > > > to know what kind of waveform to expect (e.g. square versus > > > > > > > > > sine)? > > > > > > > > > > > > > > > > It's the same clock input in the chip's perspective. One clock > > > > > > > > input > > > > > > > > that can be any of the combinations I mentioned above. Again, > > > > > > > > I'm not > > > > > > > > familiar with clocks, so I guess the square vs. sine > > > > > > > > explanation is > > > > > > > > plausible. What I could see in the firmware is that it handles > > > > > > > > the > > > > > > > > clocks differently if they're xtal or not. > > > > > > > > > > > > > > OMAP has a similar thing where sys_clkin (the fast reference > > > > > > > clock for > > > > > > > the chip) can be 19.2, 26, 38.4, etc. This is easy to handle > > > > > > > since only > > > > > > > the rates matter. > > > > > > > > > > > > Right, this part is easy and I already have the code for that. > > > > > > What I'm > > > > > > missing is a way to pass this XTAL flag to the chip. > > > > > > > > > > > > > > > > > > > In your case you need some extra metadata to know what to do. I'm > > > > > > > really > > > > > > > not sure if CLK_IS_TYPE_XTAL is the most useful form this > > > > > > > metadata can > > > > > > > take. It would be best to know if the waveform is what you really > > > > > > > need > > > > > > > to know, or perhaps something else. For instance you might be > > > > > > > affected > > > > > > > by some clock signal stabilization time. Can you talk to your > > > > > > > hardware > > > > > > > guys and figure it out? I'd rather model the actual needs instead > > > > > > > of > > > > > > > just tossing a flag in there. > > > > > > > > > > > > I get your point. I have tried to investigate how this flag is > > > > > > used by > > > > > > the firmware and I could see that it is used to set different > > > > > > "buffer > > > > > > gains" and "delays" when waking up (I guess this means when the > > > > > > clock is > > > > > > starting, so probably related to stabilization time). They specify > > > > > > two > > > > > > "modes", "boost" and "normal" and use different delay values for > > > > > > each. > > > > > > > > > > I tried but I couldn't find any more information on how exactly this > > > > > works. But since this change is really simple and there seems to be > > > > > other people who need the same information, couldn't we add it as is > > > > > and > > > > > try to figure out more specific information about the clocks later on? > > > > > > > > > > Even if XTAL is not that useful if we know the other details, at least > > > > > it wouldn't hur
Re: [RFC] clk: add flags to distinguish xtal clocks
Hi, Sorry for the delayed response. On Tue, 2013-10-08 at 10:27 -0500, Felipe Balbi wrote: > Fixing Luca's address since he left TI Thanks, Felipe! I wouldn't have seen this otherwise. > On Mon, Oct 07, 2013 at 12:44:24AM -0700, Mike Turquette wrote: > > Quoting Luciano Coelho (2013-07-29 06:50:42) > > > Hi Mike, > > > > > > On Fri, 2013-07-05 at 10:54 +0300, Luciano Coelho wrote: > > > > On Thu, 2013-07-04 at 16:19 -0700, Mike Turquette wrote: > > > > > Quoting Luciano Coelho (2013-07-04 15:37:45) > > > > > > On Thu, 2013-07-04 at 15:25 -0700, Mike Turquette wrote: > > > > > > > Or is it the same clock input and basically the problem is that > > > > > > > you need > > > > > > > to know what kind of waveform to expect (e.g. square versus sine)? > > > > > > > > > > > > It's the same clock input in the chip's perspective. One clock > > > > > > input > > > > > > that can be any of the combinations I mentioned above. Again, I'm > > > > > > not > > > > > > familiar with clocks, so I guess the square vs. sine explanation is > > > > > > plausible. What I could see in the firmware is that it handles the > > > > > > clocks differently if they're xtal or not. > > > > > > > > > > OMAP has a similar thing where sys_clkin (the fast reference clock for > > > > > the chip) can be 19.2, 26, 38.4, etc. This is easy to handle since > > > > > only > > > > > the rates matter. > > > > > > > > Right, this part is easy and I already have the code for that. What I'm > > > > missing is a way to pass this XTAL flag to the chip. > > > > > > > > > > > > > In your case you need some extra metadata to know what to do. I'm > > > > > really > > > > > not sure if CLK_IS_TYPE_XTAL is the most useful form this metadata can > > > > > take. It would be best to know if the waveform is what you really need > > > > > to know, or perhaps something else. For instance you might be affected > > > > > by some clock signal stabilization time. Can you talk to your hardware > > > > > guys and figure it out? I'd rather model the actual needs instead of > > > > > just tossing a flag in there. > > > > > > > > I get your point. I have tried to investigate how this flag is used by > > > > the firmware and I could see that it is used to set different "buffer > > > > gains" and "delays" when waking up (I guess this means when the clock is > > > > starting, so probably related to stabilization time). They specify two > > > > "modes", "boost" and "normal" and use different delay values for each. > > > > > > I tried but I couldn't find any more information on how exactly this > > > works. But since this change is really simple and there seems to be > > > other people who need the same information, couldn't we add it as is and > > > try to figure out more specific information about the clocks later on? > > > > > > Even if XTAL is not that useful if we know the other details, at least > > > it wouldn't hurt to have the flag there anyway. > > > > Luca, > > > > By any chance did you come to a different solution for this problem? I > > can take the patch, but I do not feel like we're solving the right > > problem the right way. Unfortunately I didn't find any better solution for this. From the WiLink firmware's point of view, there was not much information about the details of stabilization time requirements and such. > > If not I will take it for 3.13. That would be great! As I mentioned above, the XTAL/non-XTAL flag wouldn't hurt, even if in most cases it may not provide the necessary details for the clock code. But at least for the WiLink hardware it is very useful. ;) -- Cheers, Luca. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL] firmware: wl1251 firmware binary
On Fri, 2013-10-04 at 10:32 -0500, Felipe Balbi wrote:
> diff --git a/LICENCE.wl1251 b/LICENCE.wl1251
> new file mode 100644
> index 000..bd0f5f1
> --- /dev/null
> +++ b/LICENCE.wl1251
> @@ -0,0 +1,59 @@
> +Copyright (c) 2000 – 2013 Texas Instruments Incorporated
> +
> +All rights reserved not granted herein.
> +
> +Limited License.
> +
> +Texas Instruments Incorporated grants a world-wide, royalty-free,
> non-exclusive
> +license under copyrights and patents it now or hereafter owns or controls to
> +make, have made, use, import, offer to sell and sell ("Utilize") this
> software
> +subject to the terms herein. With respect to the foregoing patent license,
> +such license is granted solely to the extent that any such patent is
> necessary
> +to Utilize the software alone. The patent license shall not apply to any
> +combinations which include this software, other than combinations with
> devices
> +manufactured by or for TI (“TI Devices”). No hardware patent is licensed
> +hereunder.
> +
> +Redistributions must preserve existing copyright notices and reproduce this
> +license (including the above copyright notice and the disclaimer and (if
> +applicable) source code license limitations below) in the documentation
> and/or
> +other materials provided with the distribution
> +
> +Redistribution and use in binary form, without modification, are permitted
> +provided that the following conditions are met:
> +
> +*No reverse engineering, decompilation, or disassembly of this software
> + is permitted with respect to any software provided in binary form.
> +
> +*any redistribution and use are licensed by TI for use only with TI
> + Devices.
> +
> +*Nothing shall obligate TI to provide you with source code for the
> + software licensed and provided to you in object code.
> +
> +If software source code is provided to you, modification and redistribution
> of
> +the source code are permitted provided that the following conditions are met:
> +
> +*any redistribution and use of the source code, including any resulting
> + derivative works, are licensed by TI for use only with TI Devices.
> +
> +*any redistribution and use of any object code compiled from the source
> + code and any resulting derivative works, are licensed by TI for use
> + only with TI Devices.
> +
> +Neither the name of Texas Instruments Incorporated nor the names of its
> +suppliers may be used to endorse or promote products derived from this
> software
> +without specific prior written permission.
> +
> +DISCLAIMER.
> +
> +THIS SOFTWARE IS PROVIDED BY TI AND TI’S LICENSORS "AS IS" AND ANY EXPRESS OR
> +IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
> +MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO
> +EVENT SHALL TI AND TI’S LICENSORS BE LIABLE FOR ANY DIRECT, INDIRECT,
> +INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> +LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA,
> OR
> +PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
> +LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
> NEGLIGENCE
> +OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
> +ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> diff --git a/WHENCE b/WHENCE
> index aac3ba9..3a35670 100644
> --- a/WHENCE
> +++ b/WHENCE
> @@ -1712,6 +1712,24 @@ Licence: Redistributable. See
> LICENCE.tda7706-firmware.txt for details.
>
> --
>
> +Driver: wl1251 - Texas Instruments 802.11 WLAN driver for WiLink4 chips
> +
> +File: ti-connectivity/wl1251-fw.bin
> +Version: 4.0.4.3.7
> +
> +File: ti-connectivity/wl1251-nvs.bin
> +
> +Licence: See LICENCE.wl1251 for details.
> +
> +The published NVS files are for testing only. Every device needs to
> +have a unique NVS which is properly calibrated for best results.
> +
> +The driver expects to find the firmwares under a ti-connectivity
> subdirectory.
> +So if your system looks for firmwares in /lib/firmware, the firmwares for
> +wl12xx chips must be located in /lib/firmware/ti-connectivity/.
> +
> +--
> +
> Driver: wl12xx - Texas Instruments 802.11 WLAN driver for WiLink6/7 chips
>
> File: ti-connectivity/wl1271-fw.bin
> diff --git a/ti-connectivity/wl1251-fw.bin b/ti-connectivity/wl1251-fw.bin
> new file mode 100644
> index 000..f89c983
> Binary files /dev/null and b/ti-connectivity/wl1251-fw.bin differ
> diff --git a/ti-connectivity/wl1251-nvs.bin b/ti-connectivity/wl1251-nvs.bin
> new file mode 100644
> index 000..2bf9c50
> Binary files /dev/null and b/ti-connectivity/wl1251-nvs.bin differ
>
Looks good to me:
Acked-by: Luciano Coelho
--
Luca.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...
Re: [PATCH] net: wireless: wl1251: update firmware path
On Wed, 2013-10-02 at 08:00 -0500, Felipe Balbi wrote: > TI firmwares are located under ti-connectivity > directory. Update path to make sure driver can > find and load firmware blob. > > Signed-off-by: Felipe Balbi > --- Applied, thanks Felipe! -- Luca. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL] firmware: wl1251 firmware binary
Hi Felipe, On Wed, 2013-10-02 at 07:55 -0500, Felipe Balbi wrote: > Hi, > > here's a pull request for wl4 firmware. I'll send a patch for wl1251 > driver updating firmware load path. > > The following changes since commit b8ac7c7e27dcd13fa3c843aaf62457e9c57ea4db: > > linux-firmware: Add Brocade FC/FCOE Adapter firmware files (2013-09-30 > 04:53:32 +0100) > > are available in the git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/balbi/linux-firmware.git > wilink4 > > for you to fetch changes up to d726804dbc8dad88587b6be17716714cd91ed86c: > > ti-connectivity: add wl1251 firmware and license (2013-10-02 06:55:39 -0500) > > > Felipe Balbi (1): > ti-connectivity: add wl1251 firmware and license > > LICENCE.wl1251 | 59 > + > WHENCE | 18 + > ti-connectivity/wl1251-fw.bin | Bin 0 -> 194180 bytes > ti-connectivity/wl1251-nvs.bin | Bin 0 -> 752 bytes > 4 files changed, 77 insertions(+) > create mode 100644 LICENCE.wl1251 > create mode 100644 ti-connectivity/wl1251-fw.bin > create mode 100644 ti-connectivity/wl1251-nvs.bin Since you didn't send v2 of the patch, you could have used the -p option with git request-pull so we could see the changes you made here... -- Cheers, Luca. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: using mmc2 on panda [was: Regression 3.11-rc1: omap4panda: no usb and consequently no ethernet]
On Wed, 2013-10-02 at 12:20 +0200, Arend van Spriel wrote: > On 10/01/2013 01:29 PM, Luca Coelho wrote: > > Hi, > > > > On Tue, 2013-10-01 at 12:53 +0200, Arend van Spriel wrote: > >> On 10/01/2013 11:53 AM, Roger Quadros wrote: > >>> On 10/01/2013 12:49 PM, Roger Quadros wrote: > >>>> Hi Arend, > >>>> > >>>> On 10/01/2013 11:05 AM, Arend van Spriel wrote: > >>>>> On 07/19/2013 12:57 PM, Arend van Spriel wrote: > >>>>>> On 07/19/2013 12:49 PM, Roger Quadros wrote: > >>>>>>> On 07/19/2013 01:36 PM, Arend van Spriel wrote: > >>>>>>>> On 07/18/2013 10:59 AM, Tony Lindgren wrote: > >>>>>>>>> Then for the SDIO with device tree, take a look at the following > >>>>>>>>> patches: > >>>>>>>>> > >>>>>>>>> [PATCH 0/3] WLAN support for omap4 when booted with devicetree > >>>>>>>>> http://comments.gmane.org/gmane.linux.ports.arm.omap/97522# > >>>>>>>> > >>>>>>>> I have been looking at the pandaboard patch in the series above and I > >>>>>>>> do have a question. Among other things the patch adds these dt > >>>>>>>> entries. > >>>>>>>> > >>>>>>>> +0x108 0x118/* sdmmc5_clk.sdmmc5_clk INPUT_PULLUP | > >>>>>>>> MODE0 */ > >>>>>>>> +0x10a 0x118/* sdmmc5_cmd.sdmmc5_cmd INPUT_PULLUP | > >>>>>>>> MODE0 */ > >>>>>>>> > >>>>>>>> If I look at the similar names in the deceased board-omap4panda.c: > >>>>>>>> > >>>>>>>> board-omap4panda.c:OMAP4_MUX(SDMMC5_CMD, OMAP_MUX_MODE0 | > >>>>>>>> OMAP_PIN_INPUT_PULLUP), > >>>>>>>> board-omap4panda.c:OMAP4_MUX(SDMMC5_CLK, OMAP_MUX_MODE0 | > >>>>>>>> OMAP_PIN_INPUT_PULLUP), > >>>>>>>> > >>>>>>>> and in mux44xx.h: > >>>>>>>> > >>>>>>>> mux44xx.h:#define OMAP4_CTRL_MODULE_PAD_SDMMC5_CLK_OFFSET0x0148 > >>>>>>>> mux44xx.h:#define OMAP4_CTRL_MODULE_PAD_SDMMC5_CMD_OFFSET0x014a > >>>>>>>> > >>>>>>>> So how did 0x0148 get 0x0108 in DT and 0x014a get 0x010a. There is > >>>>>>>> probably an explanation to it and it would help my understanding to > >>>>>>>> know where this difference comes from. Hope you can help me out here. > >>>>>>>> > >>>>>>> > >>>>>>> If you see omap4.dtsi, omap4_pmx_core starts at register address > >>>>>>> 0x4a100040. > >>>>>>> > >>>>>>> So, you need to subtract 0x40 from the offsets defined in mux44xx.h > >>>>>>> for pmx_core registers. > >>>>>> > >>>>>> That was what I was looking for. Thanks! > >>>>> > >>>>> Hi Roger, > >>>>> > >>>>> It has been a while, but I would like to pickup this thread. We have a > >>>>> couple of pandaboards used as test setup. These have an SDIO adapter > >>>>> hooked up to expansion connector A using MMC2. I have attached the > >>>>> patch file (just ignore platform_data stuff). Now on one board it > >>>>> works, but not for the other. I suspect a board issue so listing the > >>>>> two types that we use: > >>>>> > >>>>> PandaBoard rev A2 (dmesg: OMAP4430 ES2.1): works > >>>>> PandaBoardES rev B1 (dmesg: OMAP4460 ES1.1): nope > >>>>> > >>>>> Any hints for me. > >>>> > >>>> Does your PandaboardES have the WLAN chip (U4) mounted? If yes, how do > >>>> you isolate > >>>> it from your external SDIO adapter? > >> > >> On my 4460 board in front of me U4 is not populated, but U3 is (the TiWi > >> thing). > >> > >>> > >>> OK, just realized that the expansion connector uses different pads for > >>> MMC2. However, you still > >>> need to make sure that the other pins (connected to on board WLAN chip) > >>> are not muxed as MMC2. > >> > >> I think Luciano added DT patches for on-board WLAN and it uses MMC5 if I > >> am not mistaken(?). Attached are the dmesg logs of the two boards. > > > > I sent 2 patch series to add DT support for the on-board WLAN, but they > > were not applied, since there were some comments that required changes. > > I really don't have the time to revisit them now that I'm not with TI > > anymore, so I'm hoping someone else will pick them up at some point. > > I found this one in my email archive: > > [PATCH v2 0/4] ARM: dts: add WiLink support to panda and omap4-sdp > > Guess that is what you are referring to, right? Yes, that one and also this (which implements DT in the driver itself): [PATCH v4 0/8] wilink: add device tree support http://mid.gmane.org/1375189476-21557-1-git-send-email-coe...@ti.com -- Luca. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
