Re: [RFC PATCH] iwlwifi: yoyo: don't print failure if debug firmware is missing

[Luciano Coelho]

On Sun, 2020-07-26 at 21:11 +0300, Kalle Valo wrote:
> Wolfram Sang  writes:
> 
> > On Thu, Jun 25, 2020 at 06:52:10PM +0200, Wolfram Sang wrote:
> > > Missing this firmware is not fatal, my wifi card still works. Even more,
> > > I couldn't find any documentation what it is or where to get it. So, I
> > > don't think the users should be notified if it is missing. If you browse
> > > the net, you see the message is present is in quite some logs. Better
> > > remove it.
> > > 
> > > Signed-off-by: Wolfram Sang 
> > > ---
> > 
> > Any input on this? Or people I should add to CC?
> 
> This was discussed on another thread:
> 
> https://lkml.kernel.org/r/87mu3magfp@tynnyri.adurom.net
> 
> Unless Intel folks object I'm planning to take this to
> wireless-drivers-next.

Yes, please, just go ahead and take it.  I have the same change in our
internal tree, but I didn't have the time to send it out due to my
vacations (from which I'm now back).

--
Cheers,
Luca.

Re: [PATCH] iwl: fix crash in iwl_dbg_tlv_alloc_trigger

[Luciano Coelho]

On Fri, 2020-06-12 at 10:55 +0300, Kalle Valo wrote:
> Jiri Slaby  writes:
> 
> > The tlv passed to iwl_dbg_tlv_alloc_trigger comes from a loaded firmware
> > file. The memory can be marked as read-only as firmware could be
> > shared. In anyway, writing to this memory is not expected. So,
> > iwl_dbg_tlv_alloc_trigger can crash now:
> > 
> >   BUG: unable to handle page fault for address: ae2c01bfa794
> >   PF: supervisor write access in kernel mode
> >   PF: error_code(0x0003) - permissions violation
> >   PGD 107d51067 P4D 107d51067 PUD 107d52067 PMD 659ad2067 PTE 
> > 800662298161
> >   CPU: 2 PID: 161 Comm: kworker/2:1 Not tainted 5.7.0-3.gad96a07-default #1 
> > openSUSE Tumbleweed (unreleased)
> >   RIP: 0010:iwl_dbg_tlv_alloc_trigger+0x25/0x60 [iwlwifi]
> >   Code: eb f2 0f 1f 00 66 66 66 66 90 83 7e 04 33 48 89 f8 44 8b 46 10 48 
> > 89 f7 76 40 41 8d 50 ff 83 fa 19 77 23 8b 56 20 85 d2 75 07  46 20 ff 
> > ff ff ff 4b 8d 14 40 48 c1 e2 04 48 8d b4 10 00 05 00
> >   RSP: 0018:ae2c00417ce8 EFLAGS: 00010246
> >   RAX: 8f0522334018 RBX: 8f0522334018 RCX: c0fc26c0
> >   RDX:  RSI: ae2c01bfa774 RDI: ae2c01bfa774
> >   RBP:  R08: 0004 R09: 0001
> >   R10: 0034 R11: ae2c01bfa77c R12: 8f0522334230
> >   R13: 0109 R14: 8f0523fdbc00 R15: 8f051f395800
> >   FS:  () GS:8f0527c8() 
> > knlGS:
> >   CS:  0010 DS:  ES:  CR0: 80050033
> >   CR2: ae2c01bfa794 CR3: 000389eba000 CR4: 06e0
> >   Call Trace:
> >iwl_dbg_tlv_alloc+0x79/0x120 [iwlwifi]
> >iwl_parse_tlv_firmware.isra.0+0x57d/0x1550 [iwlwifi]
> >iwl_req_fw_callback+0x3f8/0x6a0 [iwlwifi]
> >request_firmware_work_func+0x47/0x90
> >process_one_work+0x1e3/0x3b0
> >worker_thread+0x46/0x340
> >kthread+0x115/0x140
> >ret_from_fork+0x1f/0x40
> > 
> > As can be seen, write bit is not set in the PTE. Read of
> > trig->occurrences succeeds in iwl_dbg_tlv_alloc_trigger, but
> > trig->occurrences = cpu_to_le32(-1); fails there, obviously.
> > 
> > This is likely because we (at SUSE) use compressed firmware and that is
> > marked as RO after decompression (see fw_map_paged_buf).
> > 
> > Fix it by creating a temporary buffer in case we need to change the
> > memory.
> > 
> > Signed-off-by: Jiri Slaby 
> > Reported-by: Dieter Nützel 
> > Tested-by: Dieter Nützel 
> > Cc: Johannes Berg 
> > Cc: Emmanuel Grumbach 
> > Cc: Luca Coelho 
> > Cc: Intel Linux Wireless 
> > Cc: Kalle Valo 
> > Cc: "David S. Miller" 
> > Cc: Jakub Kicinski 
> > Cc: linux-wirel...@vger.kernel.org
> > Cc: net...@vger.kernel.org
> > ---
> >  drivers/net/wireless/intel/iwlwifi/iwl-dbg-tlv.c | 16 ++--
> 
> The prefix should be "iwlwifi: ", I can fix that.
> 
> Luca, should I take this to wireless-drivers?

Yeah, this looks good.  Thanks, Jiri!

And yes, Kalle, please apply it directly to w-d.  Thank you!

--
Cheers,
Luca.

Re: [PATCH 02/15] iwlwifi: mvm: fix gcc-10 zero-length-bounds warning

[Luciano Coelho]

On Thu, 2020-04-30 at 23:30 +0200, Arnd Bergmann wrote:
> gcc-10 complains when a zero-length array is accessed:
> 
> drivers/net/wireless/intel/iwlwifi/mvm/tx.c: In function 
> 'iwl_mvm_rx_ba_notif':
> drivers/net/wireless/intel/iwlwifi/mvm/tx.c:1929:17: warning: array subscript 
> 9 is outside the bounds of an interior zero-length array 'struct 
> iwl_mvm_compressed_ba_tfd[0]' [-Wzero-length-bounds]
>  1929 | _res->tfd[i];
>   |  ~~~^~~
> In file included from 
> drivers/net/wireless/intel/iwlwifi/mvm/../fw/api/tdls.h:68,
>  from drivers/net/wireless/intel/iwlwifi/mvm/fw-api.h:68,
>  from drivers/net/wireless/intel/iwlwifi/mvm/sta.h:73,
>  from drivers/net/wireless/intel/iwlwifi/mvm/mvm.h:83,
>  from drivers/net/wireless/intel/iwlwifi/mvm/tx.c:72:
> drivers/net/wireless/intel/iwlwifi/mvm/../fw/api/tx.h:769:35: note: while 
> referencing 'tfd'
>   769 |  struct iwl_mvm_compressed_ba_tfd tfd[0];
>   |   ^~~
> 
> Change this structure to use a flexible-array member for 'tfd' instead,
> along with the various structures using an zero-length ieee80211_hdr
> array that do not show warnings today but might be affected by similar
> issues in the future.
> 
> Fixes: 6f68cc367ab6 ("iwlwifi: api: annotate compressed BA notif array sizes")
> Fixes: c46e7724bfe9 ("iwlwifi: mvm: support new BA notification response")
> Signed-off-by: Arnd Bergmann 

Patch applied to iwlwifi-next.git, thanks.

9cec1d547cb7 iwlwifi: mvm: fix gcc-10 zero-length-bounds warning

Re: [PATCH -next] iwlwifi: pcie: Use bitwise instead of arithmetic operator for flags

[Luciano Coelho]

On Tue, 2020-05-05 at 20:19 -0700, Joe Perches wrote:
> On Wed, 2020-05-06 at 11:07 +0800, Samuel Zou wrote:
> > This silences the following coccinelle warning:
> > 
> > "WARNING: sum of probable bitmasks, consider |"
> 
> I suggest instead ignoring bad and irrelevant warnings.
> 
> PREFIX_LEN is 32 not 0x20 or BIT(5)
> PCI_DUMP_SIZE is 352
> 
> > diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c 
> > b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
> []
> > @@ -109,9 +109,9 @@ void iwl_trans_pcie_dump_regs(struct iwl_trans *trans)
> >  
> > /* Alloc a max size buffer */
> > alloc_size = PCI_ERR_ROOT_ERR_SRC +  4 + PREFIX_LEN;
> > -   alloc_size = max_t(u32, alloc_size, PCI_DUMP_SIZE + PREFIX_LEN);
> > -   alloc_size = max_t(u32, alloc_size, PCI_MEM_DUMP_SIZE + PREFIX_LEN);
> > -   alloc_size = max_t(u32, alloc_size, PCI_PARENT_DUMP_SIZE + PREFIX_LEN);
> > +   alloc_size = max_t(u32, alloc_size, PCI_DUMP_SIZE | PREFIX_LEN);
> > +   alloc_size = max_t(u32, alloc_size, PCI_MEM_DUMP_SIZE | PREFIX_LEN);
> > +   alloc_size = max_t(u32, alloc_size, PCI_PARENT_DUMP_SIZE | PREFIX_LEN);
> >  
> > buf = kmalloc(alloc_size, GFP_ATOMIC);
> > if (!buf)

Yeah, those macros are clearly not bitmasks.  I'm dropping this patch.

--
Cheers,
Luca.

Re: [PATCH] iwlwifi: fw: don't send GEO_TX_POWER_LIMIT command to FW version 29

[Luciano Coelho]

On Tue, 2019-10-08 at 14:05 +0800, You-Sheng Yang wrote:
> Follow-up for commit fddbfeece9c7 ("iwlwifi: fw: don't send
> GEO_TX_POWER_LIMIT command to FW version 36"). There is no
> GEO_TX_POWER_LIMIT command support for all revisions of FW version
> 29, either.
> 
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=204151
> Signed-off-by: You-Sheng Yang 
> ---
>  drivers/net/wireless/intel/iwlwifi/mvm/fw.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/fw.c 
> b/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
> index 32a5e4e5461f..dbba616c19de 100644
> --- a/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
> +++ b/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
> @@ -889,14 +889,14 @@ static bool iwl_mvm_sar_geo_support(struct iwl_mvm *mvm)
>* firmware versions.  Unfortunately, we don't have a TLV API
>* flag to rely on, so rely on the major version which is in
>* the first byte of ucode_ver.  This was implemented
> -  * initially on version 38 and then backported to29 and 17.
> +  * initially on version 38 and then backported to 29 and 17.
>* The intention was to have it in 36 as well, but not all
>* 8000 family got this feature enabled.  The 8000 family is
>* the only one using version 36, so skip this version
> -  * entirely.
> +  * entirely. All revisions of -29 fw still don't have
> +  * GEO_TX_POWER_LIMIT supported yet.
>*/
>   return IWL_UCODE_SERIAL(mvm->fw->ucode_ver) >= 38 ||
> -IWL_UCODE_SERIAL(mvm->fw->ucode_ver) == 29 ||
>  IWL_UCODE_SERIAL(mvm->fw->ucode_ver) == 17;
>  }

Thanks for the patch!

But I have investigated this (even) further and now I see that 3168
doesn't have this command, but 7265D does.  The latter also uses -29,
so we can't blindly disable all -29 versions.

Can you try this instead?

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
b/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
index 0d2229319261..38d89ee9bd28 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
@@ -906,8 +906,10 @@ static bool iwl_mvm_sar_geo_support(struct iwl_mvm
*mvm)
 * entirely.
 */
return IWL_UCODE_SERIAL(mvm->fw->ucode_ver) >= 38 ||
-  IWL_UCODE_SERIAL(mvm->fw->ucode_ver) == 29 ||
-  IWL_UCODE_SERIAL(mvm->fw->ucode_ver) == 17;
+  IWL_UCODE_SERIAL(mvm->fw->ucode_ver) == 17 ||
+  (IWL_UCODE_SERIAL(mvm->fw->ucode_ver) == 29 &&
+   (mvm->trans->hw_rev &
+CSR_HW_REV_TYPE_MSK) == CSR_HW_REV_TYPE_7265D);
 }
 
 int iwl_mvm_get_sar_geo_profile(struct iwl_mvm *mvm)


--
Cheers,
Luca.

Re: [PATCH] iwlwifi: dvm: excessive if in rs_bt_update_lq()

[Luciano Coelho]

On Wed, 2019-09-25 at 23:49 +0300, Denis Efremov wrote:
> There is no need to check 'priv->bt_ant_couple_ok' twice in
> rs_bt_update_lq(). The second check is always true. Thus, the
> expression can be simplified.
> 
> Signed-off-by: Denis Efremov 
> ---
>  drivers/net/wireless/intel/iwlwifi/dvm/rs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/intel/iwlwifi/dvm/rs.c 
> b/drivers/net/wireless/intel/iwlwifi/dvm/rs.c
> index 74229fcb63a9..226165db7dfd 100644
> --- a/drivers/net/wireless/intel/iwlwifi/dvm/rs.c
> +++ b/drivers/net/wireless/intel/iwlwifi/dvm/rs.c
> @@ -851,7 +851,7 @@ static void rs_bt_update_lq(struct iwl_priv *priv, struct 
> iwl_rxon_context *ctx,
>* Is there a need to switch between
>* full concurrency and 3-wire?
>*/
> - if (priv->bt_ci_compliance && priv->bt_ant_couple_ok)
> + if (priv->bt_ci_compliance)
>   full_concurrent = true;
>   else
>   full_concurrent = false;

Thanks, Denis! I have applied this to our internal tree and it will
reach the mainline following our usual upstreaming process.

--
Cheers,
Luca.

Re: [PATCH net] iwlwifi: add dependency of THERMAL with IWLMVM

[Luciano Coelho]

On Wed, 2019-09-18 at 16:08 +0300, Kalle Valo wrote:
> Mao Wenan  writes:
> 
> > If CONFIG_IWLMVM=y, CONFIG_THERMAL=n, below error can be found:
> > drivers/net/wireless/intel/iwlwifi/mvm/fw.o: In function `iwl_mvm_up':
> > fw.c:(.text+0x2c26): undefined reference to 
> > `iwl_mvm_send_temp_report_ths_cmd'
> > make: *** [vmlinux] Error 1
> > 
> > After commit 242d9c8b9a93 ("iwlwifi: mvm: use FW thermal
> > monitoring regardless of CONFIG_THERMAL"), iwl_mvm_up()
> > calls iwl_mvm_send_temp_report_ths_cmd(), but this function
> > is under CONFIG_THERMAL, which is depended on CONFIG_THERMAL.
> > 
> > Fixes: 242d9c8b9a93 ("iwlwifi: mvm: use FW thermal monitoring regardless of 
> > CONFIG_THERMAL")
> > Signed-off-by: Mao Wenan 
> 
> Luca, should I apply this directly to wireless-drivers?

No, this patch defeats the point of the patch it fixes.

We have a proper fix already internally, which I haven't sent out yet,
that moves a couple of #ifdef's around to solve the issue.  I'll send
the patch in a sec.

--
Cheers,
Luca.

Re: linux-next: Signed-off-by missing for commit in the net-next tree

[Luciano Coelho]

On Wed, 2019-09-11 at 00:42 +1000, Stephen Rothwell wrote:
> Hi all,
> 
> Commit
> 
>   aa43ae121675 ("iwlwifi: LTR updates")
> 
> is missing a Signed-off-by from its committer.

Oops, that was my fault.  What can we do about it? Is it enough if I
give my s-o-b publicly here?

I hereby sign off this change:

Signed-off-by: Luca Coelho 


--
Cheers,
Luca.

Re: [PATCH] iwlwifi: mvm: fix old-style declaration

[Luciano Coelho]

On Fri, 2019-07-26 at 22:18 +0800, YueHaibing wrote:
> There expect the 'static' keyword to come first in a
> declaration, and we get a warning for this with "make 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]
> 
> Reported-by: Hulk Robot 
> Signed-off-by: YueHaibing 
> ---

Thanks! Johannes applied this in our internal tree a few weeks ago and
it will reach the mainline following our usual upstreaming process.

--
Cheers,
Luca.

Re: [PATCH net-next 07/10] iwlwifi: Use dev_get_drvdata where possible

[Luciano Coelho]

On Wed, 2019-07-24 at 19:27 +0800, Chuhong Yuan wrote:
> Instead of using to_pci_dev + pci_get_drvdata,
> use dev_get_drvdata to make code simpler.
> 
> Signed-off-by: Chuhong Yuan 
> ---

This patch is not relevant anymore because we have removed all
D0i3/runtime PM code.

Thanks anyway!

--
Cheers,
Luca.

Re: PROBLEM: 5.3.0-rc* causes iwlwifi failure

[Luciano Coelho]

On Tue, 2019-08-20 at 19:37 -0400, Stuart Little wrote:
> On Tue, Aug 20, 2019 at 01:45:37PM +0300, Luciano Coelho wrote:
> > I'll have to look into all NIC/FW-version combinations that we have
> > and
> > update the iwl_mvm_sar_geo_support() function accordingly, which
> > is,
> > BTW, the easier place for you to change if you want to workaround
> > the
> > issue.
> 
> Thanks!
> 
> I didn't quite know how to interpret this suggestion (i.e. what the
> change should be), so I was poking around in there out of curiosity.
> One simple-minded thing that worked was to just pretend that that
> function always returns false:
> 
> --- cut here ---
> 
> diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
> b/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
> index 5de54d1559dd..8c0160e5588f 100644
> --- a/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
> +++ b/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
> @@ -925,7 +925,7 @@ int iwl_mvm_get_sar_geo_profile(struct iwl_mvm
> *mvm)
> .data = { data },
> };
>  
> -   if (!iwl_mvm_sar_geo_support(mvm))
> +   /*if (!iwl_mvm_sar_geo_support(mvm))*/
> return -EOPNOTSUPP;
>  
> ret = iwl_mvm_send_cmd(mvm, );
> @@ -953,7 +953,7 @@ static int iwl_mvm_sar_geo_init(struct iwl_mvm
> *mvm)
> int ret, i, j;
> u16 cmd_wide_id =  WIDE_ID(PHY_OPS_GROUP,
> GEO_TX_POWER_LIMIT);
>  
> -   if (!iwl_mvm_sar_geo_support(mvm))
> +   /*if (!iwl_mvm_sar_geo_support(mvm))*/
> return 0;
>  
> ret = iwl_mvm_sar_get_wgds_table(mvm);
> 
> --- cut here ---

Yeah, I meant more or less to return false for your NIC.  You could
have just forced that function return false.

--
Cheers,
Luca.

Re: PROBLEM: 5.3.0-rc* causes iwlwifi failure

[Luciano Coelho]

On Sat, 2019-08-17 at 17:44 -0400, 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.).

Sorry for the delay in responding, I had to go and dig in our FW
sources to see what was going on.

Unfortunately when this feature was implemented in the FW, we forgot to
add the usual flag (capabilities TLV) that we add to let the driver
know whether the command is supported or not.  So we need to match on
the FW version instead, but apparently that doesn't work for all
different NICs.

I'll have to look into all NIC/FW-version combinations that we have and
update the iwl_mvm_sar_geo_support() function accordingly, which is,
BTW, the easier place for you to change if you want to workaround the
issue.

--
Cheers,
Luca.

Re: [PATCH -next] iwlwifi: dbg: work around clang bug by marking debug strings static

[Luciano Coelho]

On Tue, 2019-08-06 at 22:15 -0700, Nathan Chancellor wrote:
> On Tue, Aug 06, 2019 at 03:37:42PM -0700, Nick Desaulniers wrote:
> > On Thu, Aug 1, 2019 at 12:11 AM Johannes Berg  
> > wrote:
> > > 
> > > > Luca, you said this was already fixed in your internal tree, and the fix
> > > > would appear soon in next, but I don't see anything in linux-next?
> > > 
> > > Luca is still on vacation, but I just sent out a version of the patch we
> > > had applied internally.
> > > 
> > > Also turns out it wasn't actually _fixed_, just _moved_, so those
> > > internal patches wouldn't have helped anyway.
> > 
> > Thanks for the report. Do you have a link?
> > I'll rebase my patch then.
> > -- 
> > Thanks,
> > ~Nick Desaulniers
> 
> Just for everyone else (since I commented on our issue tracker), this is
> now fixed in Linus's tree as of commit  1f6607250331 ("iwlwifi: dbg_ini:
> fix compile time assert build errors").

Yes, thanks Nathan! I was just digging for this patch to reply to you,
I'm still catching up with what happened during my vacations.

--
Cheers,
Luca.

Re: Regression with the latest iwlwifi-9260-*-46.ucode

[Luciano Coelho]

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.

In any case, today or tomorrow, I'll release a new FW version for 9260,
hopefully it will solve the problem.  I'll send you a link so you can
test it when I push it to our tree (which feeds linux-firmware.git).

Thanks for reporting!

--
Cheers,
Luca.


On Fri, 2019-07-19 at 17:35 +0200, Takashi Iwai wrote:
> Hi,
> 
> we've got a report about the regression with the latest iwlwifi 9260
> *-46.ucode that landed recently in linux-firmware tree.  WiFi doesn't
> work any longer on some Dell machines with that firmware.  See details
> in:
>   https://bugzilla.opensuse.org/show_bug.cgi?id=1142128
> 
> Currently we're reverting to the previously released firmware files.
> 
> Is it a known problem that has been already addressed?
> If not, shall we revert the changes as a tentative workaround?
> 
> 
> Thanks!
> 
> Takashi

Re: iwl_mvm_add_new_dqa_stream_wk BUG in lib/list_debug.c:56

[Luciano Coelho]

On Fri, 2019-06-07 at 22:44 +0200, Marc Haber wrote:
> On Fri, Jun 07, 2019 at 10:20:56PM +0200, Yussuf Khalil wrote:
> > CC'ing iwlwifi maintainers to get some attention for this issue.
> > 
> > I am experiencing the very same bug on a ThinkPad T480s running 5.1.6 with
> > Fedora 30. A friend is seeing it on his X1 Carbon 6th Gen, too. Both have an
> > "Intel Corporation Wireless 8265 / 8275" card according to lspci.
> 
> I have an older 04:00.0 Network controller [0280]: Intel Corporation
> Wireless 8260 [8086:24f3] (rev 3a) on a Thinkpad X260.
> 
> > Notably, in all cases I've observed it occurred right after roaming from one
> > AP to another (though I can't guarantee this isn't a coincidence).
> 
> I also have multiple Access Points broadcasting the same SSID in my
> house, and yes, I experience those issues often when I move from one
> part of the hose to another. I have, however, also experienced it in a
> hotel when I was using the mobile hotspot offered by my mobile, so that
> was clearly not a roaming situation.

Hi,

Sorry this got under the radar for a while.  Yesterday someone created
a bugzilla entry with the same error:

https://bugzilla.kernel.org/show_bug.cgi?id=204141

I'm going to file an internal bug report and then have someone look
further into it.

Any additional comments/reproductions/etc. please use that bugzilla
entry.

Thanks for reporting!

--
Cheers,
Luca.

Re: [PATCH] iwlwifi: fix 64-bit division

[Luciano Coelho]

On Tue, 2019-03-05 at 09:37 +0100, Arnd Bergmann wrote:
> On Tue, Mar 5, 2019 at 7:44 AM Luciano Coelho <
> luciano.coe...@intel.com> wrote:
> > On Mon, 2019-03-04 at 21:38 +0100, Arnd Bergmann wrote:
> > This was already fixed with this patch:
> > 
> > https://patchwork.kernel.org/patch/10823267/
> > 
> > ...but it hasn't reached the mainline yet.
> > 
> > I'm planning to send it to the v5.1-rc series as soon as the merge
> > window closes.  Is that quick enough for you?
> 
> That's fine, I just want it to reach 5.1 so it does not regress
> against 5.0.

Okay, I'll push it to 5.1-rc series for sure.


> Sorry for the slightly bad timing of my submission. I had just
> restarted
> my randconfig tests the other day and wanted to get my 60 new patches
> out as quickly as possible.

No problem! I really appreciate your sending patches and reviews.

--
Cheers,
Luca.

Re: [PATCH] iwlwifi: fix 64-bit division

[Luciano Coelho]

Hi Arnd,

On Mon, 2019-03-04 at 21:38 +0100, Arnd Bergmann wrote:
> 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 
> ---

This was already fixed with this patch:

https://patchwork.kernel.org/patch/10823267/

...but it hasn't reached the mainline yet.

I'm planning to send it to the v5.1-rc series as soon as the merge
window closes.  Is that quick enough for you?

--
Cheers,
Luca.

Re: [PATCH v2] iwlwifi: mvm: Use div_s64 instead of do_div in iwl_mvm_debug_range_resp

[Luciano Coelho]

On Thu, 2019-02-21 at 16:13 -0800, Nick Desaulniers wrote:
> On Thu, Feb 21, 2019 at 12:08 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 div_s64, which expects a
> > signed
> > dividend.
> > 
> > Fixes: 937b10c0de68 ("iwlwifi: mvm: add debug prints for FTM")
> > Link: https://github.com/ClangBuiltLinux/linux/issues/372
> > Suggested-by: Arnd Bergmann 
> > Signed-off-by: Nathan Chancellor 
> > ---
> > 
> > v1 -> v2:
> > 
> > * Fix logic (as the return value of div{,64}_s64 must be used),
> > thanks
> >   to Arnd for the review.
> 
> oh boy, sorry I missed that in the initial code review, thanks Arnd
> for the sharp eye!
> Reviewed-by: Nick Desaulniers 

Thanks, guys, I really didn't pay much attention when I applied the
previous versions either.

I have applied this in our internal tree and will send it out instead
of the previous one as part of our upstreaming process.

--
Cheers,
Luca.

Re: [PATCH] iwlwifi: mvm: Use div64_s64 instead of do_div in iwl_mvm_debug_range_resp

[Luciano Coelho]

On Wed, 2019-02-20 at 10:56 -0700, Nathan Chancellor wrote:
> On Wed, Feb 20, 2019 at 11:51:34AM +0100, Arnd Bergmann wrote:
> > On Tue, Feb 19, 2019 at 7:22 PM Nathan Chancellor
> >  wrote:
> > > diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/ftm-
> > > initiator.c b/drivers/net/wireless/intel/iwlwifi/mvm/ftm-
> > > initiator.c
> > > index e9822a3ec373..92b22250eb7d 100644
> > > --- a/drivers/net/wireless/intel/iwlwifi/mvm/ftm-initiator.c
> > > +++ b/drivers/net/wireless/intel/iwlwifi/mvm/ftm-initiator.c
> > > @@ -462,7 +462,7 @@ static void iwl_mvm_debug_range_resp(struct
> > > iwl_mvm *mvm, u8 index,
> > >  {
> > > s64 rtt_avg = res->ftm.rtt_avg * 100;
> > > 
> > > -   do_div(rtt_avg, );
> > > +   div64_s64(rtt_avg, );
> > 
> > This is wrong: div64_s64 does not modify its argument like
> > do_div(), but
> > it returns the result instead. You also don't want to divide by a
> > 64-bit
> > value when the second argument is a small constant.
> > 
> > I think the correct way should be
> > 
> >s64 rtt_avg = div_s64(res->ftm.rtt_avg * 100, );
> > 
> > If you know that the value is positive, using unsigned types
> > and div_u64() would be slightly faster.
> > 
> >   Arnd
> 
> Thanks for the review and explanation, Arnd.
> 
> Luca, could you drop this version so I can resend it?

Sure, please do! I already applied this internally, but I can just fix
it with your new patch and that will be squashed before being sent
upstream, so it will look like your second patch.

--
Cheers,
Luca.

Re: [PATCH v2 2/2] iwlwifi: Use struct_size() in kzalloc

[Luciano Coelho]

On Tue, 2019-01-29 at 11:21 +0800, YueHaibing wrote:
> Use struct_size() in kzalloc instead of the 'regd_to_copy'
> 
> Signed-off-by: YueHaibing 
> ---

Applied to our internal tree and it will reach the mainline following
our normal upstreaming process.

Thanks!

--
Luca.

Re: [PATCH v2 1/2] iwlwifi: Use kmemdup instead of duplicating its function

[Luciano Coelho]

On Tue, 2019-01-29 at 11:21 +0800, YueHaibing wrote:
> Use kmemdup rather than duplicating its implementation
> 
> Signed-off-by: YueHaibing 
> ---
>  drivers/net/wireless/intel/iwlwifi/iwl-nvm-parse.c | 8 ++--
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-nvm-parse.c
> b/drivers/net/wireless/intel/iwlwifi/iwl-nvm-parse.c
> index d9afedc..569cc50 100644
> --- a/drivers/net/wireless/intel/iwlwifi/iwl-nvm-parse.c
> +++ b/drivers/net/wireless/intel/iwlwifi/iwl-nvm-parse.c
> @@ -1196,13 +1196,9 @@ iwl_parse_nvm_mcc_info(struct device *dev,
> const struct iwl_cfg *cfg,
>   regd_to_copy = sizeof(struct ieee80211_regdomain) +
>   valid_rules * sizeof(struct ieee80211_reg_rule);
>  
> - copy_rd = kzalloc(regd_to_copy, GFP_KERNEL);
> - if (!copy_rd) {
> + copy_rd = kmemdup(regd, regd_to_copy, GFP_KERNEL);
> + if (!copy_rd)
>   copy_rd = ERR_PTR(-ENOMEM);
> - goto out;
> - }
> -
> - memcpy(copy_rd, regd, regd_to_copy);
>  
>  out:
>   kfree(regdb_ptrs);

This was already applied.  Dropping.

--
Luca.

Re: [PATCH] iwlwifi: iwl-drv: no need to check return value of debugfs_create functions

[Luciano Coelho]

On Tue, 2019-01-22 at 16:21 +0100, Greg Kroah-Hartman wrote:
> When calling debugfs functions, there is no need to ever check the
> return value.  The function can work or not, but the code logic
> should
> never do something different based on this.
> 
> Cc: Johannes Berg 
> Cc: Emmanuel Grumbach 
> Cc: Luca Coelho 
> Cc: Intel Linux Wireless 
> Cc: Kalle Valo 
> Cc: linux-wirel...@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman 
> ---

Thanks, Greg! I applied this in our internal tree and it will reach
upstream following our normal upstreaming process.

--
Cheers,
Luca.

Re: [PATCH] iwlwifi: dvm: no need to check return value of debugfs_create functions

[Luciano Coelho]

On Tue, 2019-01-22 at 16:21 +0100, Greg Kroah-Hartman wrote:
> When calling debugfs functions, there is no need to ever check the
> return value.  The function can work or not, but the code logic
> should
> never do something different based on this.
> 
> Cc: Johannes Berg 
> Cc: Emmanuel Grumbach 
> Cc: Luca Coelho 
> Cc: Intel Linux Wireless 
> Cc: Kalle Valo 
> Cc: linux-wirel...@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman 
> ---

Thanks, Greg! I applied this in our internal tree and it will reach
upstream following our normal upstreaming process.

--
Cheers,
Luca.

Re: [PATCH] iwlwifi: pcie: no need to check return value of debugfs_create functions

[Luciano Coelho]

On Tue, 2019-01-22 at 16:21 +0100, Greg Kroah-Hartman wrote:
> When calling debugfs functions, there is no need to ever check the
> return value.  The function can work or not, but the code logic
> should
> never do something different based on this.
> 
> Cc: Johannes Berg 
> Cc: Emmanuel Grumbach 
> Cc: Luca Coelho 
> Cc: Intel Linux Wireless 
> Cc: Kalle Valo 
> Cc: linux-wirel...@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman 
> ---


Thanks, Greg! I applied this in our internal tree and it will reach
upstream following our normal upstreaming process.

--
Cheers,
Luca.

Re: [PATCH] iwlwifi: mvm: no need to check return value of debugfs_create functions

[Luciano Coelho]

On Tue, 2019-01-22 at 16:21 +0100, Greg Kroah-Hartman wrote:
> When calling debugfs functions, there is no need to ever check the
> return value.  The function can work or not, but the code logic
> should
> never do something different based on this.
> 
> Cc: Johannes Berg 
> Cc: Emmanuel Grumbach 
> Cc: Luca Coelho 
> Cc: Intel Linux Wireless 
> Cc: Kalle Valo 
> Cc: linux-wirel...@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman 
> ---


Thanks, Greg! I applied this in our internal tree and it will reach
upstream following our normal upstreaming process.

--
Cheers,
Luca.

Re: [PATCH] iwlwifi: fw: no need to check return value of debugfs_create functions

[Luciano Coelho]

On Tue, 2019-01-22 at 16:21 +0100, Greg Kroah-Hartman wrote:
> When calling debugfs functions, there is no need to ever check the
> return value.  The function can work or not, but the code logic
> should
> never do something different based on this.
> 
> Cc: Johannes Berg 
> Cc: Emmanuel Grumbach 
> Cc: Luca Coelho 
> Cc: Intel Linux Wireless 
> Cc: Kalle Valo 
> Cc: linux-wirel...@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman 
> ---


Thanks, Greg! I applied this in our internal tree and it will reach
upstream following our normal upstreaming process.

--
Cheers,
Luca.

Re: [PATCH] iwlwifi: mvm: remove duplicated include from ops.c

[Luciano Coelho]

On Thu, 2019-01-17 at 15:00 +0800, YueHaibing wrote:
> Remove duplicated include.
> 
> Signed-off-by: YueHaibing 
> ---

Dropped, this is a duplicate.

--
Cheers,
Luca.

Re: [PATCH] iwlwifi: eeprom-parse: use struct_size() in kzalloc()

[Luciano Coelho]

On Tue, 2019-01-08 at 11:17 -0600, Gustavo A. R. Silva wrote:
> One of the more common cases of allocation size calculations is
> finding the
> size of a structure that has a zero-sized array at the end, along
> with memory
> for some number of elements for that array. For example:
> 
> struct foo {
> int stuff;
> void *entry[];
> };
> 
> instance = kzalloc(sizeof(struct foo) + sizeof(void *) * count,
> GFP_KERNEL);
> 
> Instead of leaving these open-coded and prone to type mistakes, we
> can now
> use the new struct_size() helper:
> 
> instance = kzalloc(struct_size(instance, entry, count), GFP_KERNEL);
> 
> This code was detected with the help of Coccinelle.
> 
> Signed-off-by: Gustavo A. R. Silva 
> ---

Applied to our internal tree, thanks! It will reach the mainline
following our normal upstreaming process.

--
Luca.

Re: [PATCH][next] iwlwifi: mvm: use struct_size() in kzalloc()

[Luciano Coelho]

On Tue, 2019-01-15 at 16:02 -0600, Gustavo A. R. Silva wrote:
> One of the more common cases of allocation size calculations is
> finding the
> size of a structure that has a zero-sized array at the end, along
> with memory
> for some number of elements for that array. For example:
> 
> struct foo {
> int stuff;
> struct boo entry[];
> };
> 
> instance = kzalloc(sizeof(struct foo) + count * sizeof(struct boo),
> GFP_KERNEL);
> 
> Instead of leaving these open-coded and prone to type mistakes, we
> can now
> use the new struct_size() helper:
> 
> instance = kzalloc(struct_size(instance, entry, count), GFP_KERNEL);
> 
> This code was detected with the help of Coccinelle.
> 
> Signed-off-by: Gustavo A. R. Silva 
> ---

Dropped.  This is a duplicate.

--
Luca.

Re: [PATCH] iwlwifi: nvm-parse: use struct_size() in kzalloc()

[Luciano Coelho]

On Tue, 2019-01-08 at 11:55 -0600, Gustavo A. R. Silva wrote:
> One of the more common cases of allocation size calculations is
> finding the
> size of a structure that has a zero-sized array at the end, along
> with memory
> for some number of elements for that array. For example:
> 
> struct foo {
> int stuff;
> void *entry[];
> };
> 
> instance = kzalloc(sizeof(struct foo) + sizeof(void *) * count,
> GFP_KERNEL);
> 
> Instead of leaving these open-coded and prone to type mistakes, we
> can now
> use the new struct_size() helper:
> 
> instance = kzalloc(struct_size(instance, entry, count), GFP_KERNEL);
> 
> This code was detected with the help of Coccinelle.
> 
> Signed-off-by: Gustavo A. R. Silva 
> ---

Thanks! I applied this to our internal tree and it will reach the
mainline following our normal upstreaming process.

--
Cheers,
Luca.

Re: [for next][PATCH] iwlwifi: Fix unmet dependency error for IWLWIFI_LEDS

[Luciano Coelho]

On Mon, 2019-01-21 at 23:31 +, Sinan Kaya wrote:
> There is an unresolved dependency as follows:
> 
> IWLWIFI_LEDS selects MAC80211_LEDS.
> MAC80211_LEDS depends on MAC80211.
> 
> It is possible to choose MAC80211_LEDS (y) but not choose MAC80211
> (n)
> 
> WARNING: unmet direct dependencies detected for MAC80211_LEDS
>   Depends on [n]: NET [=y] && WIRELESS [=y] && MAC80211 [=n] &&
> LEDS_CLASS [=y]
>   Selected by [y]:
>   - IWLWIFI_LEDS [=y] && NETDEVICES [=y] && WLAN [=y] &&
> WLAN_VENDOR_INTEL [=y] && IWLWIFI [=y] && (LEDS_CLASS [=y]=y ||
> LEDS_CLASS [=y]=IWLWIFI [=y])
> 
> Move the MAC80211 dependency into IWLWIFI_LEDS so that we avoid this
> configuration.
> 
> Signed-off-by: Sinan Kaya 
> ---

Thanks for your patch! But we already have another patch to fix this
issued queued for 5.0-rc4 (it's currently in wireless-drivers.git):

https://patchwork.kernel.org/patch/10762079/

--
Cheers,
Luca.

Re: [PATCH v1 3/4] iwlwifi: Remove unnecessary include of

[Luciano Coelho]

On Wed, 2018-07-25 at 14:52 -0500, Bjorn Helgaas wrote:
> From: Bjorn Helgaas 
> 
> This part of the iwlwifi driver doesn't need anything provided by
> pci-aspm.h, so remove the unnecessary include of it.
> 
> Signed-off-by: Bjorn Helgaas 
> ---

Acked-by: Luca Coelho 

Thanks!

--
Cheers,
Luca.

Re: [PATCH v1 3/4] iwlwifi: Remove unnecessary include of

[Luciano Coelho]

On Wed, 2018-07-25 at 14:52 -0500, Bjorn Helgaas wrote:
> From: Bjorn Helgaas 
> 
> This part of the iwlwifi driver doesn't need anything provided by
> pci-aspm.h, so remove the unnecessary include of it.
> 
> Signed-off-by: Bjorn Helgaas 
> ---

Acked-by: Luca Coelho 

Thanks!

--
Cheers,
Luca.

Re: [linux-firmware] Version in WHENCE file

[Luciano Coelho]

On Mon, 2018-05-07 at 09:47 +0200, Sedat Dilek wrote:
> On Sun, May 6, 2018 at 3:07 PM, Luciano Coelho <luciano.coelho@intel.
> com> wrote:
> > On Sun, 2018-05-06 at 14:46 +0200, Sedat Dilek wrote:
> > > Hi Luca,
> > > 
> > > I hope I catched the correct MLs (not sure if linux-firmware has
> > > a
> > > ML,
> > > I did not found any in the MAINTAINERS file).
> > > 
> > > I have seen that in the WHENCE file there is "Version" with and
> > > without ":", mostly iwlwifi ucodes.
> > > 
> > > As an example:
> > > 
> > >  File: iwlwifi-8265-36.ucode
> > > -Version 36.e91976c0.0
> > > +Version: 36.e91976c0.0
> > > 
> > > I don't know the workflow: Do you want to fix it in your tree or
> > > directly in linux-firmware.git upstream?
> > > My attached patch is against upstream.
> > 
> > Thanks, Sedat!
> > 
> > I'm going to send a new pull-request this week, so I can include
> > your
> > patch in my tree and as part of the pull-request.
> > 
> > --
> > Cheers,
> > Luca.
> 
> OK, Thanks.
> Attached Patch v2 is against your tree [1].
> It differs from v1 against upstream.

You need to write a proper commit message and sign it off so I can
apply it.

--
Luca.

Re: [linux-firmware] Version in WHENCE file

[Luciano Coelho]

On Mon, 2018-05-07 at 09:47 +0200, Sedat Dilek wrote:
> On Sun, May 6, 2018 at 3:07 PM, Luciano Coelho  com> wrote:
> > On Sun, 2018-05-06 at 14:46 +0200, Sedat Dilek wrote:
> > > Hi Luca,
> > > 
> > > I hope I catched the correct MLs (not sure if linux-firmware has
> > > a
> > > ML,
> > > I did not found any in the MAINTAINERS file).
> > > 
> > > I have seen that in the WHENCE file there is "Version" with and
> > > without ":", mostly iwlwifi ucodes.
> > > 
> > > As an example:
> > > 
> > >  File: iwlwifi-8265-36.ucode
> > > -Version 36.e91976c0.0
> > > +Version: 36.e91976c0.0
> > > 
> > > I don't know the workflow: Do you want to fix it in your tree or
> > > directly in linux-firmware.git upstream?
> > > My attached patch is against upstream.
> > 
> > Thanks, Sedat!
> > 
> > I'm going to send a new pull-request this week, so I can include
> > your
> > patch in my tree and as part of the pull-request.
> > 
> > --
> > Cheers,
> > Luca.
> 
> OK, Thanks.
> Attached Patch v2 is against your tree [1].
> It differs from v1 against upstream.

You need to write a proper commit message and sign it off so I can
apply it.

--
Luca.

Re: [linux-firmware] Version in WHENCE file

[Luciano Coelho]

On Sun, 2018-05-06 at 14:46 +0200, Sedat Dilek wrote:
> Hi Luca,
> 
> I hope I catched the correct MLs (not sure if linux-firmware has a
> ML,
> I did not found any in the MAINTAINERS file).
> 
> I have seen that in the WHENCE file there is "Version" with and
> without ":", mostly iwlwifi ucodes.
> 
> As an example:
> 
>  File: iwlwifi-8265-36.ucode
> -Version 36.e91976c0.0
> +Version: 36.e91976c0.0
> 
> I don't know the workflow: Do you want to fix it in your tree or
> directly in linux-firmware.git upstream?
> My attached patch is against upstream.

Thanks, Sedat!

I'm going to send a new pull-request this week, so I can include your
patch in my tree and as part of the pull-request.

--
Cheers,
Luca.

Re: [linux-firmware] Version in WHENCE file

[Luciano Coelho]

On Sun, 2018-05-06 at 14:46 +0200, Sedat Dilek wrote:
> Hi Luca,
> 
> I hope I catched the correct MLs (not sure if linux-firmware has a
> ML,
> I did not found any in the MAINTAINERS file).
> 
> I have seen that in the WHENCE file there is "Version" with and
> without ":", mostly iwlwifi ucodes.
> 
> As an example:
> 
>  File: iwlwifi-8265-36.ucode
> -Version 36.e91976c0.0
> +Version: 36.e91976c0.0
> 
> I don't know the workflow: Do you want to fix it in your tree or
> directly in linux-firmware.git upstream?
> My attached patch is against upstream.

Thanks, Sedat!

I'm going to send a new pull-request this week, so I can include your
patch in my tree and as part of the pull-request.

--
Cheers,
Luca.

Re: linux-next: manual merge of the wireless-drivers-next tree with the wireless-drivers tree

[Luciano Coelho]

On Thu, 2018-04-26 at 13:33 +0300, Kalle Valo wrote:
> Luciano Coelho <luciano.coe...@intel.com> writes:
> 
> > On Thu, 2018-04-26 at 11:09 +1000, Stephen Rothwell wrote:
> > > Hi all,
> > > 
> > > Today's linux-next merge of the wireless-drivers-next tree got a
> > > conflict in:
> > > 
> > >   drivers/net/wireless/intel/iwlwifi/iwl-nvm-parse.c
> > > 
> > > between commit:
> > > 
> > >   77e30e10ee28 ("iwlwifi: mvm: query regdb for wmm rule if
> > > needed")
> > > 
> > > from the wireless-drivers tree and commits:
> > > 
> > >   9c4f7d512740 ("iwlwifi: move all NVM parsing code to the common
> > > files")
> > >   4c625c564ba2 ("iwlwifi: get rid of fw/nvm.c")
> > > 
> > > 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.
> > 
> > Thanks for resolving this, Stephen!
> > 
> > I checked your resolution and it's spot on.
> 
> I now merged w-d to w-d-next to fix this, please check:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-driver
> s-next.git/commit/?id=0ddcf3e76ae4d02918e609342a1020b50258fadd
> 
> I was not sure what to do with these includes so I left them in:
> 
> #include "fw/api/commands.h"
> #include "fw/api/cmdhdr.h"
> #include "fw/img.h"

That's fine, these are all in our internal tree too.

Thanks!

--
Cheers,
Luca.

Re: linux-next: manual merge of the wireless-drivers-next tree with the wireless-drivers tree

[Luciano Coelho]

On Thu, 2018-04-26 at 13:33 +0300, Kalle Valo wrote:
> Luciano Coelho  writes:
> 
> > On Thu, 2018-04-26 at 11:09 +1000, Stephen Rothwell wrote:
> > > Hi all,
> > > 
> > > Today's linux-next merge of the wireless-drivers-next tree got a
> > > conflict in:
> > > 
> > >   drivers/net/wireless/intel/iwlwifi/iwl-nvm-parse.c
> > > 
> > > between commit:
> > > 
> > >   77e30e10ee28 ("iwlwifi: mvm: query regdb for wmm rule if
> > > needed")
> > > 
> > > from the wireless-drivers tree and commits:
> > > 
> > >   9c4f7d512740 ("iwlwifi: move all NVM parsing code to the common
> > > files")
> > >   4c625c564ba2 ("iwlwifi: get rid of fw/nvm.c")
> > > 
> > > 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.
> > 
> > Thanks for resolving this, Stephen!
> > 
> > I checked your resolution and it's spot on.
> 
> I now merged w-d to w-d-next to fix this, please check:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-driver
> s-next.git/commit/?id=0ddcf3e76ae4d02918e609342a1020b50258fadd
> 
> I was not sure what to do with these includes so I left them in:
> 
> #include "fw/api/commands.h"
> #include "fw/api/cmdhdr.h"
> #include "fw/img.h"

That's fine, these are all in our internal tree too.

Thanks!

--
Cheers,
Luca.

Re: linux-next: manual merge of the wireless-drivers-next tree with the wireless-drivers tree

[Luciano Coelho]

On Thu, 2018-04-26 at 11:09 +1000, Stephen Rothwell wrote:
> Hi all,
> 
> Today's linux-next merge of the wireless-drivers-next tree got a
> conflict in:
> 
>   drivers/net/wireless/intel/iwlwifi/iwl-nvm-parse.c
> 
> between commit:
> 
>   77e30e10ee28 ("iwlwifi: mvm: query regdb for wmm rule if needed")
> 
> from the wireless-drivers tree and commits:
> 
>   9c4f7d512740 ("iwlwifi: move all NVM parsing code to the common
> files")
>   4c625c564ba2 ("iwlwifi: get rid of fw/nvm.c")
> 
> 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.

Thanks for resolving this, Stephen!

I checked your resolution and it's spot on.

--
Cheers,
Luca.

Re: linux-next: manual merge of the wireless-drivers-next tree with the wireless-drivers tree

[Luciano Coelho]

On Thu, 2018-04-26 at 11:09 +1000, Stephen Rothwell wrote:
> Hi all,
> 
> Today's linux-next merge of the wireless-drivers-next tree got a
> conflict in:
> 
>   drivers/net/wireless/intel/iwlwifi/iwl-nvm-parse.c
> 
> between commit:
> 
>   77e30e10ee28 ("iwlwifi: mvm: query regdb for wmm rule if needed")
> 
> from the wireless-drivers tree and commits:
> 
>   9c4f7d512740 ("iwlwifi: move all NVM parsing code to the common
> files")
>   4c625c564ba2 ("iwlwifi: get rid of fw/nvm.c")
> 
> 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.

Thanks for resolving this, Stephen!

I checked your resolution and it's spot on.

--
Cheers,
Luca.

Re: linux-next: Signed-off-by missing for commit in the wireless-drivers-next tree

[Luciano Coelho]

Hi,

On Wed, 2018-04-25 at 10:56 +1000, Stephen Rothwell wrote:
> Hi all,
> 
> Commit
> 
>   84226ca1c5d3 ("iwlwifi: mvm: support offload of AMSDU rate
> control")
> 
> is missing a Signed-off-by from its author.

I checked this and it should be fine.  The author is actually wrong in
the commit.  Sara is the actual author and we have her S-o-B in the
commit.

I have no idea how Gregory became the author in that commit, though.  I
checked exactly how I committed it and checked the author in our
internal tree.  It's all fine, but for some really weird reason that I
can't figure out, Gregory became the author of the commit when I
applied that patch.  The AuthorDate is also wrong.  I checked my bash
history to see how I applied the patch and everything looks normal...
*confused*


--
Cheers,
Luca.

Re: linux-next: Signed-off-by missing for commit in the wireless-drivers-next tree

[Luciano Coelho]

Hi,

On Wed, 2018-04-25 at 10:56 +1000, Stephen Rothwell wrote:
> Hi all,
> 
> Commit
> 
>   84226ca1c5d3 ("iwlwifi: mvm: support offload of AMSDU rate
> control")
> 
> is missing a Signed-off-by from its author.

I checked this and it should be fine.  The author is actually wrong in
the commit.  Sara is the actual author and we have her S-o-B in the
commit.

I have no idea how Gregory became the author in that commit, though.  I
checked exactly how I committed it and checked the author in our
internal tree.  It's all fine, but for some really weird reason that I
can't figure out, Gregory became the author of the commit when I
applied that patch.  The AuthorDate is also wrong.  I checked my bash
history to see how I applied the patch and everything looks normal...
*confused*


--
Cheers,
Luca.

Re: New sparse warning from min_t(): expression using sizeof(void)

[Luciano Coelho]

On Sat, 2018-04-21 at 11:54 +0300, Kalle Valo wrote:
> Joey Pabalinas  writes:
> 
> > On Sat, Apr 21, 2018 at 10:50:51AM +0300, Kalle Valo wrote:
> > > Is there any way to fix it? With ath10k I use sparse a lot and
> > > because
> > > of these warnings sparse is now very annoying to use.
> > 
> > I submitted a sparse patch [1] for this not too long ago, but in
> > the
> > meantime you can still curl the patch [2] and apply it directly to
> > the v0.5.2 release to suppress the deluge of warnings (-Wpointer-
> > arith
> > is off by defaultl so no need to change any of you configurations).
> > 
> > [1] https://lkml.org/lkml/2018/4/10/923
> > [2] https://patchwork.kernel.org/patch/10334353/raw/
> 
> Perfect, thanks. Unfortunatelly I need to catch a boat and I can't
> test
> it right now, but I'll do that on Monday.

Thanks, this solves the problem for me, but I'm still getting a lot of
this:

./include/linux/mm.h:533:24: warning: constant 0xc900 is so big it 
is unsigned long

Is there a patch in sparse to solve this one as well? Or is this an
actual error that must be fixed in mm.h?

--
Cheers,
Luca.

Re: New sparse warning from min_t(): expression using sizeof(void)

[Luciano Coelho]

On Sat, 2018-04-21 at 11:54 +0300, Kalle Valo wrote:
> Joey Pabalinas  writes:
> 
> > On Sat, Apr 21, 2018 at 10:50:51AM +0300, Kalle Valo wrote:
> > > Is there any way to fix it? With ath10k I use sparse a lot and
> > > because
> > > of these warnings sparse is now very annoying to use.
> > 
> > I submitted a sparse patch [1] for this not too long ago, but in
> > the
> > meantime you can still curl the patch [2] and apply it directly to
> > the v0.5.2 release to suppress the deluge of warnings (-Wpointer-
> > arith
> > is off by defaultl so no need to change any of you configurations).
> > 
> > [1] https://lkml.org/lkml/2018/4/10/923
> > [2] https://patchwork.kernel.org/patch/10334353/raw/
> 
> Perfect, thanks. Unfortunatelly I need to catch a boat and I can't
> test
> it right now, but I'll do that on Monday.

Thanks, this solves the problem for me, but I'm still getting a lot of
this:

./include/linux/mm.h:533:24: warning: constant 0xc900 is so big it 
is unsigned long

Is there a patch in sparse to solve this one as well? Or is this an
actual error that must be fixed in mm.h?

--
Cheers,
Luca.

Re: [PATCH 18/20] iwlwifi: fix malformed CONFIG_IWLWIFI_PCIE_RTPM default

[Luciano Coelho]

On Mon, 2018-02-05 at 02:21 +0100, Ulf Magnusson wrote:
> 'default false' should be 'default n', though they happen to have the
> same effect here, due to undefined symbols ('false' in this case)
> evaluating to n in a tristate sense.
> 
> Remove the default instead of changing it. bool and tristate symbols
> implicitly default to n.
> 
> Discovered with the
> https://github.com/ulfalizer/Kconfiglib/blob/master/examples/list_und
> efined.py
> script.
> 
> Signed-off-by: Ulf Magnusson 
> ---

Thanks, Ulf!

This is applied in our internal tree and will eventually reach the
mainline, following our usual upstreaming process.

--
Cheers,
Luca.

Re: [PATCH 18/20] iwlwifi: fix malformed CONFIG_IWLWIFI_PCIE_RTPM default

[Luciano Coelho]

On Mon, 2018-02-05 at 02:21 +0100, Ulf Magnusson wrote:
> 'default false' should be 'default n', though they happen to have the
> same effect here, due to undefined symbols ('false' in this case)
> evaluating to n in a tristate sense.
> 
> Remove the default instead of changing it. bool and tristate symbols
> implicitly default to n.
> 
> Discovered with the
> https://github.com/ulfalizer/Kconfiglib/blob/master/examples/list_und
> efined.py
> script.
> 
> Signed-off-by: Ulf Magnusson 
> ---

Thanks, Ulf!

This is applied in our internal tree and will eventually reach the
mainline, following our usual upstreaming process.

--
Cheers,
Luca.

Re: UBSAN: Undefined behaviour in drivers/net/wireless/intel/iwlwifi/mvm/utils.c:838:5

[Luciano Coelho]

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.

--
Cheers,
Luca.

Re: UBSAN: Undefined behaviour in drivers/net/wireless/intel/iwlwifi/mvm/utils.c:838:5

[Luciano Coelho]

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.

--
Cheers,
Luca.

Re: Fwd: linux v4.14 causes firmware iwlwifi errors on Lenovo Thinkpad T440s

[Luciano Coelho]

On Mon, 2017-11-13 at 16:23 -0600, Larry Finger wrote:
> On 11/13/2017 03:30 PM, Bartosz Golaszewski wrote:
> > 2017-11-13 21:45 GMT+01:00 Larry Finger 
> > :
> > > On 11/13/2017 02:22 PM, Bartosz Golaszewski wrote:
> > > > 
> > > > Forwarding here too as I messed up the address the last time.
> > > > --
> > > > 
> > > > Hi,
> > > > 
> > > > I noticed my wireless interface can't get up with linux v4.14
> > > > and the
> > > > kernel log is flooded with firmware errors:
> > > > 
> > > > iwlwifi :03:00.0: Firmware error during reconfiguration -
> > > > reprobe!
> > > > iwlwifi :03:00.0: FW error in SYNC CMD DQA_ENABLE_CMD
> > > > 
> > > > and
> > > > 
> > > > ieee80211 phy63: Hardware restart was requested.
> > > > 
> > > > The wireless controller is: Intel Corporation Wireless 7260
> > > > (rev 83)
> > > > Firmware used is: iwlwifi-7260-17
> > > > 
> > > > Everything works fine with v4.13.12.
> > > > 
> > > > I didn't have time today to bisect for the offending commit.
> > > > Full log
> > > > uploaded[1].
> > > > 
> > > > Best regards,
> > > > Bartosz Golaszewski
> > > > 
> > > > [1] https://pastebin.com/jksqxvS6
> > > 
> > > 
> > > Your log shows "iwlwifi :03:00.0: loaded firmware version
> > > 17.228510.0
> > > op_mode iwlmvm"
> > > 
> > > Mine, where the 7260 works, shows "iwlwifi :04:00.0: loaded
> > > firmware
> > > version 17.459231.0 op_mode iwlmvm".
> > > 
> > > It seems as if you need newer firmware. A detailed file listing
> > > shows
> > > "-rw-r--r-- 1 root root 1049340 Oct  9 12:03
> > > /lib/firmware/iwlwifi-7260-17.ucode". That date is likely when I
> > > installed
> > > the updated kernel firmware package from my distro. The md5sum
> > > for the file
> > > is 73a217f55c47d3a70bb5dbbe1d676423.
> > > 
> > > Larry
> > > 
> > 
> > Ok so it seems the version in linux-firmware is outdated. The file
> > you're using is available on github[1] and fixed the issue for me.
> > 
> > Thanks!
> > Bartosz Golaszewski
> > 
> > [1] https://github.com/OpenELEC/iwlwifi-firmware
> 
> Interesting. Using md5sum of the git repo for linux-firmware gets
> 73a217f55c47d3a70bb5dbbe1d676423  iwlwifi-7260-17.ucode.
> 
> That is the file I'm using.

You shouldn't use firmwares from github or anywhere else except from
the two official places where we release it:

Mainline (but slow to get updated):

https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/plain/iwlwifi-7260-17.ucode


Our official public tree:

https://git.kernel.org/pub/scm/linux/kernel/git/iwlwifi/linux-firmware.git/plain/iwlwifi-7260-17.ucode


And, of course, your distro may distribute these in an official package
as well, but it's good to check the versions to be sure you're running
the latest one we released.

--
Cheers,
Luca.

Re: Fwd: linux v4.14 causes firmware iwlwifi errors on Lenovo Thinkpad T440s

[Luciano Coelho]

On Mon, 2017-11-13 at 16:23 -0600, Larry Finger wrote:
> On 11/13/2017 03:30 PM, Bartosz Golaszewski wrote:
> > 2017-11-13 21:45 GMT+01:00 Larry Finger 
> > :
> > > On 11/13/2017 02:22 PM, Bartosz Golaszewski wrote:
> > > > 
> > > > Forwarding here too as I messed up the address the last time.
> > > > --
> > > > 
> > > > Hi,
> > > > 
> > > > I noticed my wireless interface can't get up with linux v4.14
> > > > and the
> > > > kernel log is flooded with firmware errors:
> > > > 
> > > > iwlwifi :03:00.0: Firmware error during reconfiguration -
> > > > reprobe!
> > > > iwlwifi :03:00.0: FW error in SYNC CMD DQA_ENABLE_CMD
> > > > 
> > > > and
> > > > 
> > > > ieee80211 phy63: Hardware restart was requested.
> > > > 
> > > > The wireless controller is: Intel Corporation Wireless 7260
> > > > (rev 83)
> > > > Firmware used is: iwlwifi-7260-17
> > > > 
> > > > Everything works fine with v4.13.12.
> > > > 
> > > > I didn't have time today to bisect for the offending commit.
> > > > Full log
> > > > uploaded[1].
> > > > 
> > > > Best regards,
> > > > Bartosz Golaszewski
> > > > 
> > > > [1] https://pastebin.com/jksqxvS6
> > > 
> > > 
> > > Your log shows "iwlwifi :03:00.0: loaded firmware version
> > > 17.228510.0
> > > op_mode iwlmvm"
> > > 
> > > Mine, where the 7260 works, shows "iwlwifi :04:00.0: loaded
> > > firmware
> > > version 17.459231.0 op_mode iwlmvm".
> > > 
> > > It seems as if you need newer firmware. A detailed file listing
> > > shows
> > > "-rw-r--r-- 1 root root 1049340 Oct  9 12:03
> > > /lib/firmware/iwlwifi-7260-17.ucode". That date is likely when I
> > > installed
> > > the updated kernel firmware package from my distro. The md5sum
> > > for the file
> > > is 73a217f55c47d3a70bb5dbbe1d676423.
> > > 
> > > Larry
> > > 
> > 
> > Ok so it seems the version in linux-firmware is outdated. The file
> > you're using is available on github[1] and fixed the issue for me.
> > 
> > Thanks!
> > Bartosz Golaszewski
> > 
> > [1] https://github.com/OpenELEC/iwlwifi-firmware
> 
> Interesting. Using md5sum of the git repo for linux-firmware gets
> 73a217f55c47d3a70bb5dbbe1d676423  iwlwifi-7260-17.ucode.
> 
> That is the file I'm using.

You shouldn't use firmwares from github or anywhere else except from
the two official places where we release it:

Mainline (but slow to get updated):

https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/plain/iwlwifi-7260-17.ucode


Our official public tree:

https://git.kernel.org/pub/scm/linux/kernel/git/iwlwifi/linux-firmware.git/plain/iwlwifi-7260-17.ucode


And, of course, your distro may distribute these in an official package
as well, but it's good to check the versions to be sure you're running
the latest one we released.

--
Cheers,
Luca.

Re: linux-next: manual merge of the wireless-drivers-next tree with the wireless-drivers tree

[Luciano Coelho]

On Thu, 2017-10-12 at 19:21 +0100, Mark Brown wrote:
> On Thu, Oct 12, 2017 at 09:16:36PM +0300, Luciano Coelho wrote:
> 
> > This is weird... The previous conflict was the exact opposite of
> > this. 
> > 44fd09 came in from wireless-drivers and dd05f9 came from wireless-
> > drivers-next.  I don't understand why it is saying the opposite
> > here...
> 
> 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?

--
Luca.

Re: linux-next: manual merge of the wireless-drivers-next tree with the wireless-drivers tree

[Luciano Coelho]

On Thu, 2017-10-12 at 19:21 +0100, Mark Brown wrote:
> On Thu, Oct 12, 2017 at 09:16:36PM +0300, Luciano Coelho wrote:
> 
> > This is weird... The previous conflict was the exact opposite of
> > this. 
> > 44fd09 came in from wireless-drivers and dd05f9 came from wireless-
> > drivers-next.  I don't understand why it is saying the opposite
> > here...
> 
> 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?

--
Luca.

Re: linux-next: manual merge of the wireless-drivers-next tree with the wireless-drivers tree

[Luciano Coelho]

On Thu, 2017-10-12 at 18:25 +0100, Mark Brown wrote:
> Hi all,

Hi Mark,


> 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.

This is weird... The previous conflict was the exact opposite of this. 
44fd09 came in from wireless-drivers and dd05f9 came from wireless-
drivers-next.  I don't understand why it is saying the opposite here...

--
Cheers,
Luca.

Re: linux-next: manual merge of the wireless-drivers-next tree with the wireless-drivers tree

[Luciano Coelho]

On Thu, 2017-10-12 at 18:25 +0100, Mark Brown wrote:
> Hi all,

Hi Mark,


> 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.

This is weird... The previous conflict was the exact opposite of this. 
44fd09 came in from wireless-drivers and dd05f9 came from wireless-
drivers-next.  I don't understand why it is saying the opposite here...

--
Cheers,
Luca.

Re: [PATCH 1/3] wireless: iwlwifi: use bool instead of int

[Luciano Coelho]

On Wed, 2017-10-04 at 10:55 -0700, Joe Perches wrote:
> On Wed, 2017-10-04 at 19:39 +0300, Luciano Coelho wrote:
> > On Wed, 2017-10-04 at 09:26 -0700, Joe Perches wrote:
> 
> []
> > > This might be more intelligble as separate tests
> > > 
> > > static bool is_valid_channel(u16 ch_id)
> > > {
> > >   if (ch_id <= 14)
> > >   return true;
> > > 
> > >   if ((ch_id % 4 == 0) &&
> > >   ((ch_id >= 36 && ch_id <= 64) ||
> > >(ch_id >= 100 && ch_id <= 140)))
> > >   return true;
> > > 
> > >   if ((ch_id % 4 == 1) &&
> > >   (chid >= 145 && ch_id <= 165))
> > >   return true;
> > > 
> > >   return false;
> > > }
> > > 
> > > The compiler should produce the same object code.
> > 
> > Yeah, it may be a bit easier to read, but I don't want to start
> > getting
> > "fixes" to working and reasonable code.  There's nothing wrong with
> > the
> > existing function (except maybe for the int vs. boolean) so let's
> > not
> > change it.
> > 
> > A good time to change this would be the next time someone adds yet
> > another range of valid channels here. ;)
> 
>   Your choice.
> 
> I like code I can read and understand at a glance.

I do too, but I don't think the original is that hard to read, really. 
Each "if" you add is already corresponding to one separate line in the
original code...


> At case somebody needs to add channels, likely nobody
> would do the change suggested but would just add
> another test to the already odd looking block.

Yeah, that would most likely be the case, but if I saw that and thought
there was a better way to write it, believe me, I would definitely
nitpick the patch and ask the author to reorg the code so it would look
nicer.


> And constants should be on the right side of the tests.

Sure, in a new patch, we would definitely pay attention to that.  But
now, is it worth having one more patch go through the entire machinery
to change a relatively clear, extremely simple function just because
you could write it in a different way? My answer is a resounding no,
sorry.

--
Luca.

Re: [PATCH 1/3] wireless: iwlwifi: use bool instead of int

[Luciano Coelho]

On Wed, 2017-10-04 at 10:55 -0700, Joe Perches wrote:
> On Wed, 2017-10-04 at 19:39 +0300, Luciano Coelho wrote:
> > On Wed, 2017-10-04 at 09:26 -0700, Joe Perches wrote:
> 
> []
> > > This might be more intelligble as separate tests
> > > 
> > > static bool is_valid_channel(u16 ch_id)
> > > {
> > >   if (ch_id <= 14)
> > >   return true;
> > > 
> > >   if ((ch_id % 4 == 0) &&
> > >   ((ch_id >= 36 && ch_id <= 64) ||
> > >(ch_id >= 100 && ch_id <= 140)))
> > >   return true;
> > > 
> > >   if ((ch_id % 4 == 1) &&
> > >   (chid >= 145 && ch_id <= 165))
> > >   return true;
> > > 
> > >   return false;
> > > }
> > > 
> > > The compiler should produce the same object code.
> > 
> > Yeah, it may be a bit easier to read, but I don't want to start
> > getting
> > "fixes" to working and reasonable code.  There's nothing wrong with
> > the
> > existing function (except maybe for the int vs. boolean) so let's
> > not
> > change it.
> > 
> > A good time to change this would be the next time someone adds yet
> > another range of valid channels here. ;)
> 
>   Your choice.
> 
> I like code I can read and understand at a glance.

I do too, but I don't think the original is that hard to read, really. 
Each "if" you add is already corresponding to one separate line in the
original code...


> At case somebody needs to add channels, likely nobody
> would do the change suggested but would just add
> another test to the already odd looking block.

Yeah, that would most likely be the case, but if I saw that and thought
there was a better way to write it, believe me, I would definitely
nitpick the patch and ask the author to reorg the code so it would look
nicer.


> And constants should be on the right side of the tests.

Sure, in a new patch, we would definitely pay attention to that.  But
now, is it worth having one more patch go through the entire machinery
to change a relatively clear, extremely simple function just because
you could write it in a different way? My answer is a resounding no,
sorry.

--
Luca.

Re: [PATCH 1/3] wireless: iwlwifi: use bool instead of int

[Luciano Coelho]

On Wed, 2017-10-04 at 09:26 -0700, Joe Perches wrote:
> On Wed, 2017-10-04 at 17:56 +0200, Christoph Böhmwalder wrote:
> > Change a usage of int in a boolean context to use the bool type
> > instead, as it
> > makes the intent of the function clearer and helps clarify its
> > semantics.
> > 
> > Also eliminate the if/else and just return the boolean result
> > directly,
> > making the code more readable.
> > 
> > Signed-off-by: Christoph Böhmwalder 
> > ---
> >  drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c | 12 +---
> >  1 file changed, 5 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
> > b/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
> > index b7cd813ba70f..0eb815ae97e8 100644
> > --- a/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
> > +++ b/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
> > @@ -267,14 +267,12 @@ int iwl_phy_db_set_section(struct iwl_phy_db
> > *phy_db,
> >  }
> >  IWL_EXPORT_SYMBOL(iwl_phy_db_set_section);
> >  
> > -static int is_valid_channel(u16 ch_id)
> > +static bool is_valid_channel(u16 ch_id)
> >  {
> > -   if (ch_id <= 14 ||
> > -   (36 <= ch_id && ch_id <= 64 && ch_id % 4 == 0) ||
> > -   (100 <= ch_id && ch_id <= 140 && ch_id % 4 == 0) ||
> > -   (145 <= ch_id && ch_id <= 165 && ch_id % 4 == 1))
> > -   return 1;
> > -   return 0;
> > +   return (ch_id <= 14 ||
> > +  (36 <= ch_id && ch_id <= 64 && ch_id % 4 == 0) ||
> > +  (100 <= ch_id && ch_id <= 140 && ch_id % 4 == 0) ||
> > +  (145 <= ch_id && ch_id <= 165 && ch_id % 4 == 1));
> >  }
> 
> This might be more intelligble as separate tests
> 
> static bool is_valid_channel(u16 ch_id)
> {
>   if (ch_id <= 14)
>   return true;
> 
>   if ((ch_id % 4 == 0) &&
>   ((ch_id >= 36 && ch_id <= 64) ||
>(ch_id >= 100 && ch_id <= 140)))
>   return true;
> 
>   if ((ch_id % 4 == 1) &&
>   (chid >= 145 && ch_id <= 165))
>   return true;
> 
>   return false;
> }
> 
> The compiler should produce the same object code.

Yeah, it may be a bit easier to read, but I don't want to start getting
"fixes" to working and reasonable code.  There's nothing wrong with the
existing function (except maybe for the int vs. boolean) so let's not
change it.

A good time to change this would be the next time someone adds yet
another range of valid channels here. ;)

--
Luca.

Re: [PATCH 1/3] wireless: iwlwifi: use bool instead of int

[Luciano Coelho]

On Wed, 2017-10-04 at 09:26 -0700, Joe Perches wrote:
> On Wed, 2017-10-04 at 17:56 +0200, Christoph Böhmwalder wrote:
> > Change a usage of int in a boolean context to use the bool type
> > instead, as it
> > makes the intent of the function clearer and helps clarify its
> > semantics.
> > 
> > Also eliminate the if/else and just return the boolean result
> > directly,
> > making the code more readable.
> > 
> > Signed-off-by: Christoph Böhmwalder 
> > ---
> >  drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c | 12 +---
> >  1 file changed, 5 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
> > b/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
> > index b7cd813ba70f..0eb815ae97e8 100644
> > --- a/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
> > +++ b/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
> > @@ -267,14 +267,12 @@ int iwl_phy_db_set_section(struct iwl_phy_db
> > *phy_db,
> >  }
> >  IWL_EXPORT_SYMBOL(iwl_phy_db_set_section);
> >  
> > -static int is_valid_channel(u16 ch_id)
> > +static bool is_valid_channel(u16 ch_id)
> >  {
> > -   if (ch_id <= 14 ||
> > -   (36 <= ch_id && ch_id <= 64 && ch_id % 4 == 0) ||
> > -   (100 <= ch_id && ch_id <= 140 && ch_id % 4 == 0) ||
> > -   (145 <= ch_id && ch_id <= 165 && ch_id % 4 == 1))
> > -   return 1;
> > -   return 0;
> > +   return (ch_id <= 14 ||
> > +  (36 <= ch_id && ch_id <= 64 && ch_id % 4 == 0) ||
> > +  (100 <= ch_id && ch_id <= 140 && ch_id % 4 == 0) ||
> > +  (145 <= ch_id && ch_id <= 165 && ch_id % 4 == 1));
> >  }
> 
> This might be more intelligble as separate tests
> 
> static bool is_valid_channel(u16 ch_id)
> {
>   if (ch_id <= 14)
>   return true;
> 
>   if ((ch_id % 4 == 0) &&
>   ((ch_id >= 36 && ch_id <= 64) ||
>(ch_id >= 100 && ch_id <= 140)))
>   return true;
> 
>   if ((ch_id % 4 == 1) &&
>   (chid >= 145 && ch_id <= 165))
>   return true;
> 
>   return false;
> }
> 
> The compiler should produce the same object code.

Yeah, it may be a bit easier to read, but I don't want to start getting
"fixes" to working and reasonable code.  There's nothing wrong with the
existing function (except maybe for the int vs. boolean) so let's not
change it.

A good time to change this would be the next time someone adds yet
another range of valid channels here. ;)

--
Luca.

Re: [PATCH 0/3] iwlwifi: cosmetic fixes

[Luciano Coelho]

On Wed, 2017-10-04 at 17:56 +0200, Christoph Böhmwalder wrote:
> Fix several code style issues, some of which were reported by
> checkpatch.pl.
> 
> The changes are:
> * One instance of an `int` variable being used in a boolean context,
> chaned to
>   use the more appropriate `bool` type.
> * One very minor fix, removing a newline between a function
> definition and its
>   associated `static` keyword
> * One fix wrapping a macro in curly braces
> 
> 
> Christoph Böhmwalder (3):
>   wireless: iwlwifi: use bool instead of int
>   wireless: iwlwifi: function definition cosmetic fix
>   wireless: iwlwifi: wrap macro into braces
> 
>  drivers/net/wireless/intel/iwlwifi/iwl-io.c |  2 +-
>  drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c | 16 +++--
> ---
>  2 files changed, 8 insertions(+), 10 deletions(-)

Sorry, but this kind of series just generates churn.  Especially when 2
out of 3 patches are broken.  I applied your previous patch because it
was really trivial, but I really don't want to encourage this kind of
drive-by "fixes" that only cause additional work.

I generally only accept this kind of changes when people are changing
code close or related to it.

--
Luca.

Re: [PATCH 0/3] iwlwifi: cosmetic fixes

[Luciano Coelho]

On Wed, 2017-10-04 at 17:56 +0200, Christoph Böhmwalder wrote:
> Fix several code style issues, some of which were reported by
> checkpatch.pl.
> 
> The changes are:
> * One instance of an `int` variable being used in a boolean context,
> chaned to
>   use the more appropriate `bool` type.
> * One very minor fix, removing a newline between a function
> definition and its
>   associated `static` keyword
> * One fix wrapping a macro in curly braces
> 
> 
> Christoph Böhmwalder (3):
>   wireless: iwlwifi: use bool instead of int
>   wireless: iwlwifi: function definition cosmetic fix
>   wireless: iwlwifi: wrap macro into braces
> 
>  drivers/net/wireless/intel/iwlwifi/iwl-io.c |  2 +-
>  drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c | 16 +++--
> ---
>  2 files changed, 8 insertions(+), 10 deletions(-)

Sorry, but this kind of series just generates churn.  Especially when 2
out of 3 patches are broken.  I applied your previous patch because it
was really trivial, but I really don't want to encourage this kind of
drive-by "fixes" that only cause additional work.

I generally only accept this kind of changes when people are changing
code close or related to it.

--
Luca.

Re: [PATCH 1/3] wireless: iwlwifi: use bool instead of int

[Luciano Coelho]

On Wed, 2017-10-04 at 17:56 +0200, Christoph Böhmwalder wrote:
> Change a usage of int in a boolean context to use the bool type
> instead, as it
> makes the intent of the function clearer and helps clarify its
> semantics.
> 
> Also eliminate the if/else and just return the boolean result
> directly,
> making the code more readable.
> 
> Signed-off-by: Christoph Böhmwalder 
> ---
>  drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c | 12 +---
>  1 file changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
> b/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
> index b7cd813ba70f..0eb815ae97e8 100644
> --- a/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
> +++ b/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
> @@ -267,14 +267,12 @@ int iwl_phy_db_set_section(struct iwl_phy_db
> *phy_db,
>  }
>  IWL_EXPORT_SYMBOL(iwl_phy_db_set_section);
>  
> -static int is_valid_channel(u16 ch_id)
> +static bool is_valid_channel(u16 ch_id)
>  {
> - if (ch_id <= 14 ||
> - (36 <= ch_id && ch_id <= 64 && ch_id % 4 == 0) ||
> - (100 <= ch_id && ch_id <= 140 && ch_id % 4 == 0) ||
> - (145 <= ch_id && ch_id <= 165 && ch_id % 4 == 1))
> - return 1;
> - return 0;
> + return (ch_id <= 14 ||
> +(36 <= ch_id && ch_id <= 64 && ch_id % 4 == 0) ||
> +(100 <= ch_id && ch_id <= 140 && ch_id % 4 == 0) ||
> +(145 <= ch_id && ch_id <= 165 && ch_id % 4 == 1));
>  }
>  
>  static u8 ch_id_to_ch_index(u16 ch_id)

This actually makes some sense, and I would probably apply it if it
were part of a patchset that actually does something useful.

--
Luca.

Re: [PATCH 1/3] wireless: iwlwifi: use bool instead of int

[Luciano Coelho]

On Wed, 2017-10-04 at 17:56 +0200, Christoph Böhmwalder wrote:
> Change a usage of int in a boolean context to use the bool type
> instead, as it
> makes the intent of the function clearer and helps clarify its
> semantics.
> 
> Also eliminate the if/else and just return the boolean result
> directly,
> making the code more readable.
> 
> Signed-off-by: Christoph Böhmwalder 
> ---
>  drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c | 12 +---
>  1 file changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
> b/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
> index b7cd813ba70f..0eb815ae97e8 100644
> --- a/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
> +++ b/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
> @@ -267,14 +267,12 @@ int iwl_phy_db_set_section(struct iwl_phy_db
> *phy_db,
>  }
>  IWL_EXPORT_SYMBOL(iwl_phy_db_set_section);
>  
> -static int is_valid_channel(u16 ch_id)
> +static bool is_valid_channel(u16 ch_id)
>  {
> - if (ch_id <= 14 ||
> - (36 <= ch_id && ch_id <= 64 && ch_id % 4 == 0) ||
> - (100 <= ch_id && ch_id <= 140 && ch_id % 4 == 0) ||
> - (145 <= ch_id && ch_id <= 165 && ch_id % 4 == 1))
> - return 1;
> - return 0;
> + return (ch_id <= 14 ||
> +(36 <= ch_id && ch_id <= 64 && ch_id % 4 == 0) ||
> +(100 <= ch_id && ch_id <= 140 && ch_id % 4 == 0) ||
> +(145 <= ch_id && ch_id <= 165 && ch_id % 4 == 1));
>  }
>  
>  static u8 ch_id_to_ch_index(u16 ch_id)

This actually makes some sense, and I would probably apply it if it
were part of a patchset that actually does something useful.

--
Luca.

Re: [PATCH 2/3] wireless: iwlwifi: function definition cosmetic fix

[Luciano Coelho]

On Wed, 2017-10-04 at 17:56 +0200, Christoph Böhmwalder wrote:
> Separate the function from the previous definition with a newline and
> put the `static` keyword on the same line, as it just looks nicer.
> 
> Signed-off-by: Christoph Böhmwalder 
> ---
>  drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c 
> b/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
> index 0eb815ae97e8..249ee1c7b02f 100644
> --- a/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
> +++ b/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
> @@ -325,8 +325,8 @@ static u16 channel_id_to_txp(struct iwl_phy_db *phy_db, 
> u16 ch_id)
>   }
>   return 0xff;
>  }
> -static
> -int iwl_phy_db_get_section_data(struct iwl_phy_db *phy_db,
> +
> +static int iwl_phy_db_get_section_data(struct iwl_phy_db *phy_db,
>   u32 type, u8 **data, u16 *size, u16 ch_id)
>  {
>   struct iwl_phy_db_entry *entry;

Sorry, but this now looks much uglier because the second line is not
even aligned to the parenthesis.

NACK.

--
Luca.

Re: [PATCH 2/3] wireless: iwlwifi: function definition cosmetic fix

[Luciano Coelho]

On Wed, 2017-10-04 at 17:56 +0200, Christoph Böhmwalder wrote:
> Separate the function from the previous definition with a newline and
> put the `static` keyword on the same line, as it just looks nicer.
> 
> Signed-off-by: Christoph Böhmwalder 
> ---
>  drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c 
> b/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
> index 0eb815ae97e8..249ee1c7b02f 100644
> --- a/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
> +++ b/drivers/net/wireless/intel/iwlwifi/iwl-phy-db.c
> @@ -325,8 +325,8 @@ static u16 channel_id_to_txp(struct iwl_phy_db *phy_db, 
> u16 ch_id)
>   }
>   return 0xff;
>  }
> -static
> -int iwl_phy_db_get_section_data(struct iwl_phy_db *phy_db,
> +
> +static int iwl_phy_db_get_section_data(struct iwl_phy_db *phy_db,
>   u32 type, u8 **data, u16 *size, u16 ch_id)
>  {
>   struct iwl_phy_db_entry *entry;

Sorry, but this now looks much uglier because the second line is not
even aligned to the parenthesis.

NACK.

--
Luca.

Re: [PATCH v4 6/8] wlcore: sdio: add wilink clock providers

[Luciano Coelho]

On Tue, 2013-07-30 at 15:35 -0700, Mike Turquette wrote:
> Quoting Luciano Coelho (2013-07-30 06:04:34)
> > +static const struct of_device_id wlcore_sdio_of_clk_match_table[] = {
> > +   { .compatible = "ti,wilink-clock" },
> > +};
> > +
> >  static struct wl12xx_platform_data *wlcore_get_pdata_from_of(struct device 
> > *dev)
> >  {
> > struct wl12xx_platform_data *pdata;
> > struct device_node *np = dev->of_node;
> > +   struct device_node *clock_node;
> >  
> > if (!np) {
> > np = of_find_matching_node(NULL, 
> > dev->driver->of_match_table);
> > @@ -241,6 +247,9 @@ static struct wl12xx_platform_data 
> > *wlcore_get_pdata_from_of(struct device *dev)
> > goto out_free;
> > }
> >  
> > +   for_each_matching_node(clock_node, wlcore_sdio_of_clk_match_table)
> > +   of_fixed_clk_setup(clock_node);
> 
> Hi Luciano,

Hi Mike,


> Any reason for establishing your own compatible string if you just plan
> to use the fixed rate clock? You could just use "fixed-clock" compatible
> in your DTS.

The reason is that I can't call of_clk_init(), because this function is
not exported and my module can't use it.  I would have to link with the
clk code to be able to call it.

Also, I reckoned that, since these clock cannot be used by anyone else
than the WiLink module itself, it would make sense to have a different
compatible string.


> I will be posting patches this week which makes the fixed-rate clock a
> proper driver and matches that compatible string to instantiate those
> clocks. That means that your driver could probably remove the clock
> setup code completely.

Okay, if this is done, then I could probably use "fixed-clock" directly,
since the driver itself will take care of going through the DT and
initializing all the fixed-clocks.

--
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/

[PATCH v3] Documentation: dt: bindings: TI WiLink modules

[Luciano Coelho]

Add device tree bindings documentation for the TI WiLink modules.
Currently only the WLAN part of the WiLink6, WiLink7 and WiLink8
modules is supported.

Signed-off-by: Luciano Coelho 
---

In v3, use IRQ_TYPE_LEVEL_HIGH in the example, as suggested by Laurent.

 .../devicetree/bindings/net/wireless/ti-wilink.txt | 68 ++
 1 file changed, 68 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/wireless/ti-wilink.txt

diff --git a/Documentation/devicetree/bindings/net/wireless/ti-wilink.txt 
b/Documentation/devicetree/bindings/net/wireless/ti-wilink.txt
new file mode 100644
index 000..aafebb1
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/wireless/ti-wilink.txt
@@ -0,0 +1,68 @@
+TI WiLink Wireless Modules Device Tree Bindings
+===
+
+The WiLink modules provide wireless connectivity, such as WLAN,
+Bluetooth, FM and NFC.
+
+There are several different modules available, which can be grouped by
+their generation: WiLink6, WiLink7 and WiLink8.  WiLink4 is not
+currently supported with device tree.
+
+Currently, only the WLAN portion of the modules is supported with
+device tree.
+
+Required properties:
+
+
+- compatible: should be "ti,wilink6", "ti,wilink7" or "ti,wilink8"
+- interrupt-parent: the interrupt controller
+- interrupts: out-of-band WLAN interrupt
+   See the interrupt controller's bindings documentation for
+   detailed definition.
+
+Optional properties:
+
+
+- clocks: list of clocks needed by the chip as follows:
+
+  refclock: the internal WLAN reference clock frequency (required for
+   WiLink6 and WiLink7; not used for WiLink8).
+
+  tcxoclock: the internal WLAN TCXO clock frequency (required for
+   WiLink7 not used for WiLink6 and WiLink8).
+
+  The clocks must be defined and named accordingly.  For example:
+
+  clocks = <>
+  clock-names = "refclock";
+
+  refclock: refclock {
+compatible = "ti,wilink-clock";
+#clock-cells = <0>;
+clock-frequency = <3840>;
+   };
+
+  Some modules that contain the WiLink chip provide clocks in the
+  module itself.  In this case, we define a "ti,wilink-clock" as shown
+  above.  But any other clock could in theory be used, so the proper
+  clock definition should be used.
+
+
+Example:
+
+
+Example definition that can be used in OMAP4 Panda:
+
+wlan {
+   compatible = "ti,wilink6";
+   interrupt-parent = <>;
+   interrupts = <21 IRQ_TYPE_LEVEL_HIGH>;  /* gpio line 53 */
+   clocks = <>;
+   clock-names = "refclock";
+
+   refclock: refclock {
+   compatible = "ti,wilink-clock";
+   #clock-cells = <0>;
+   clock-frequency = <3840>;
+   };
+};
-- 
1.8.3.2

--
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 v2] Documentation: dt: bindings: TI WiLink modules

[Luciano Coelho]

(using the new devicetree mailing list address)

On Tue, 2013-07-30 at 20:24 +0200, Laurent Pinchart wrote:
> Hi Luciano,
> 
> Thank you for the patch.
> 
> On Monday 29 July 2013 17:55:28 Luciano Coelho wrote:
> > Add device tree bindings documentation for the TI WiLink modules.
> > Currently only the WLAN part of the WiLink6, WiLink7 and WiLink8
> > modules is supported.
> > 
> > Signed-off-by: Luciano Coelho 
> > ---
> > 
> > Changes in v2:
> > 
> > Use generic clock definitions to get the clock data instead of passing
> > the frequencies directly.  Also added definition for "internal"
> > ti,wilink-clock.
> > 
> > Please review.
> 
> The proposal looks good to me, I just have one small comment.

Cool! Thanks for the review.

[...]
> > +Example:
> > +
> > +
> > +Example definition that can be used in OMAP4 Panda:
> > +
> > +wlan {
> > +   compatible = "ti,wilink6";
> > +   interrupt-parent = <>;
> > +   interrupts = <21 0x4>;  /* gpio line 53, high level triggered */
> 
> Could you please use the IRQ_TYPE_LEVEL_HIGH macro (defined in  bindings/interrupt-controller/irq.h>) instead of the hardcode 0x4 constant ?

Ah, right, I saw this new header file recently but forgot to update my
example.  I'll update the OMAP4 Panda and OMAP4 SDP dts files I just
sent out as well.

--
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/

[PATCH v4 5/8] wlcore: add initial device tree support to the sdio module

[Luciano Coelho]

If platform data is not available, try to get the required information
from the device tree.  Register an OF match table and parse the
appropriate device tree nodes.

Parse interrupt property only, for now.

Signed-off-by: Luciano Coelho 
Reviewed-by: Felipe Balbi 
---
 drivers/net/wireless/ti/wlcore/sdio.c | 69 ---
 1 file changed, 63 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/ti/wlcore/sdio.c 
b/drivers/net/wireless/ti/wlcore/sdio.c
index 4c7e8ac..9370d7e 100644
--- a/drivers/net/wireless/ti/wlcore/sdio.c
+++ b/drivers/net/wireless/ti/wlcore/sdio.c
@@ -30,7 +30,7 @@
 #include 
 #include 
 #include 
-#include 
+#include 
 #include 
 #include 
 #include 
@@ -214,6 +214,43 @@ static struct wl1271_if_operations sdio_ops = {
.set_block_size = wl1271_sdio_set_block_size,
 };
 
+static struct wl12xx_platform_data *wlcore_get_pdata_from_of(struct device 
*dev)
+{
+   struct wl12xx_platform_data *pdata;
+   struct device_node *np = dev->of_node;
+
+   if (!np) {
+   np = of_find_matching_node(NULL, dev->driver->of_match_table);
+   if (!np) {
+   dev_notice(dev, "device tree node not available\n");
+   pdata = ERR_PTR(-ENODEV);
+   goto out;
+   }
+   }
+
+   pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);
+   if (!pdata) {
+   dev_err(dev, "can't allocate platform data\n");
+   pdata = ERR_PTR(-ENODEV);
+   goto out;
+   }
+
+   pdata->irq = irq_of_parse_and_map(np, 0);
+   if (pdata->irq < 0) {
+   dev_err(dev, "can't get interrupt gpio from the device tree\n");
+   goto out_free;
+   }
+
+   goto out;
+
+out_free:
+   kfree(pdata);
+   pdata = ERR_PTR(-ENODEV);
+
+out:
+   return pdata;
+}
+
 static int wl1271_probe(struct sdio_func *func,
  const struct sdio_device_id *id)
 {
@@ -248,11 +285,22 @@ static int wl1271_probe(struct sdio_func *func,
/* Use block mode for transferring over one block size of data */
func->card->quirks |= MMC_QUIRK_BLKSZ_FOR_BYTE_MODE;
 
+   /* The pdata allocated here is freed when the device is freed,
+* so we don't need an additional out label to free it in case
+* of error further on.
+*/
+
+   /* Try to get legacy platform data from the board file */
pdev_data->pdata = wl12xx_get_platform_data();
if (IS_ERR(pdev_data->pdata)) {
-   ret = PTR_ERR(pdev_data->pdata);
-   dev_err(glue->dev, "missing wlan platform data: %d\n", ret);
-   goto out_free_glue;
+   dev_info(>dev,
+"legacy platform data not found, trying device 
tree\n");
+
+   pdev_data->pdata = wlcore_get_pdata_from_of(>dev);
+   if (IS_ERR(pdev_data->pdata)) {
+   dev_err(>dev, "can't get platform data\n");
+   goto out_free_glue;
+   }
}
 
/* if sdio can keep power while host is suspended, enable wow */
@@ -386,16 +434,25 @@ static const struct dev_pm_ops wl1271_sdio_pm_ops = {
 };
 #endif
 
+static const struct of_device_id wlcore_sdio_of_match_table[] = {
+   { .compatible = "ti,wilink6" },
+   { .compatible = "ti,wilink7" },
+   { .compatible = "ti,wilink8" },
+   { }
+};
+MODULE_DEVICE_TABLE(of, wlcore_sdio_of_match_table);
+
 static struct sdio_driver wl1271_sdio_driver = {
.name   = "wl1271_sdio",
.id_table   = wl1271_devices,
.probe  = wl1271_probe,
.remove = wl1271_remove,
-#ifdef CONFIG_PM
.drv = {
+#ifdef CONFIG_PM
.pm = _sdio_pm_ops,
-   },
 #endif
+   .of_match_table = of_match_ptr(wlcore_sdio_of_match_table),
+   },
 };
 
 static int __init wl1271_init(void)
-- 
1.8.3.2

--
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/

[PATCH v4 1/8] wl1251: split wl251 platform data to a separate structure

[Luciano Coelho]

Move the wl1251 part of the wl12xx platform data structure into a new
structure specifically for wl1251.  Change the platform data built-in
block and board files accordingly.

Cc: Tony Lindgren 
Signed-off-by: Luciano Coelho 
Acked-by: Tony Lindgren 
Reviewed-by: Felipe Balbi 
---
 arch/arm/mach-omap2/board-omap3pandora.c   |  4 +--
 arch/arm/mach-omap2/board-rx51-peripherals.c   |  2 +-
 drivers/net/wireless/ti/wilink_platform_data.c | 37 +-
 drivers/net/wireless/ti/wl1251/sdio.c  | 12 -
 drivers/net/wireless/ti/wl1251/spi.c   |  2 +-
 include/linux/wl12xx.h | 22 ++-
 6 files changed, 62 insertions(+), 17 deletions(-)

diff --git a/arch/arm/mach-omap2/board-omap3pandora.c 
b/arch/arm/mach-omap2/board-omap3pandora.c
index b1547a0..84d56e8 100644
--- a/arch/arm/mach-omap2/board-omap3pandora.c
+++ b/arch/arm/mach-omap2/board-omap3pandora.c
@@ -541,7 +541,7 @@ static struct spi_board_info omap3pandora_spi_board_info[] 
__initdata = {
 
 static void __init pandora_wl1251_init(void)
 {
-   struct wl12xx_platform_data pandora_wl1251_pdata;
+   struct wl1251_platform_data pandora_wl1251_pdata;
int ret;
 
memset(_wl1251_pdata, 0, sizeof(pandora_wl1251_pdata));
@@ -555,7 +555,7 @@ static void __init pandora_wl1251_init(void)
goto fail_irq;
 
pandora_wl1251_pdata.use_eeprom = true;
-   ret = wl12xx_set_platform_data(_wl1251_pdata);
+   ret = wl1251_set_platform_data(_wl1251_pdata);
if (ret < 0)
goto fail_irq;
 
diff --git a/arch/arm/mach-omap2/board-rx51-peripherals.c 
b/arch/arm/mach-omap2/board-rx51-peripherals.c
index 9c2dd10..01e5711 100644
--- a/arch/arm/mach-omap2/board-rx51-peripherals.c
+++ b/arch/arm/mach-omap2/board-rx51-peripherals.c
@@ -80,7 +80,7 @@ enum {
RX51_SPI_MIPID, /* LCD panel */
 };
 
-static struct wl12xx_platform_data wl1251_pdata;
+static struct wl1251_platform_data wl1251_pdata;
 static struct tsc2005_platform_data tsc2005_pdata;
 
 #if defined(CONFIG_SENSORS_LIS3_I2C) || defined(CONFIG_SENSORS_LIS3_I2C_MODULE)
diff --git a/drivers/net/wireless/ti/wilink_platform_data.c 
b/drivers/net/wireless/ti/wilink_platform_data.c
index 998e958..a92bd3e 100644
--- a/drivers/net/wireless/ti/wilink_platform_data.c
+++ b/drivers/net/wireless/ti/wilink_platform_data.c
@@ -23,17 +23,17 @@
 #include 
 #include 
 
-static struct wl12xx_platform_data *platform_data;
+static struct wl12xx_platform_data *wl12xx_platform_data;
 
 int __init wl12xx_set_platform_data(const struct wl12xx_platform_data *data)
 {
-   if (platform_data)
+   if (wl12xx_platform_data)
return -EBUSY;
if (!data)
return -EINVAL;
 
-   platform_data = kmemdup(data, sizeof(*data), GFP_KERNEL);
-   if (!platform_data)
+   wl12xx_platform_data = kmemdup(data, sizeof(*data), GFP_KERNEL);
+   if (!wl12xx_platform_data)
return -ENOMEM;
 
return 0;
@@ -41,9 +41,34 @@ int __init wl12xx_set_platform_data(const struct 
wl12xx_platform_data *data)
 
 struct wl12xx_platform_data *wl12xx_get_platform_data(void)
 {
-   if (!platform_data)
+   if (!wl12xx_platform_data)
return ERR_PTR(-ENODEV);
 
-   return platform_data;
+   return wl12xx_platform_data;
 }
 EXPORT_SYMBOL(wl12xx_get_platform_data);
+
+static struct wl1251_platform_data *wl1251_platform_data;
+
+int __init wl1251_set_platform_data(const struct wl1251_platform_data *data)
+{
+   if (wl1251_platform_data)
+   return -EBUSY;
+   if (!data)
+   return -EINVAL;
+
+   wl1251_platform_data = kmemdup(data, sizeof(*data), GFP_KERNEL);
+   if (!wl1251_platform_data)
+   return -ENOMEM;
+
+   return 0;
+}
+
+struct wl1251_platform_data *wl1251_get_platform_data(void)
+{
+   if (!wl1251_platform_data)
+   return ERR_PTR(-ENODEV);
+
+   return wl1251_platform_data;
+}
+EXPORT_SYMBOL(wl1251_get_platform_data);
diff --git a/drivers/net/wireless/ti/wl1251/sdio.c 
b/drivers/net/wireless/ti/wl1251/sdio.c
index e2b3d9c..b75a37a 100644
--- a/drivers/net/wireless/ti/wl1251/sdio.c
+++ b/drivers/net/wireless/ti/wl1251/sdio.c
@@ -227,7 +227,7 @@ static int wl1251_sdio_probe(struct sdio_func *func,
struct wl1251 *wl;
struct ieee80211_hw *hw;
struct wl1251_sdio *wl_sdio;
-   const struct wl12xx_platform_data *wl12xx_board_data;
+   const struct wl1251_platform_data *wl1251_board_data;
 
hw = wl1251_alloc_hw();
if (IS_ERR(hw))
@@ -254,11 +254,11 @@ static int wl1251_sdio_probe(struct sdio_func *func,
wl->if_priv = wl_sdio;
wl->if_ops = _sdio_ops;
 
-   wl12xx_board_data = wl12xx_get_platform_data();
-   if (!IS_ERR(wl12xx_board_data)) {
-   wl->set_power = wl12xx_board_data->set_power;
-   wl->irq = wl12xx_board_data->ir

[PATCH v4 3/8] wlcore: remove pwr_in_suspend from platform data

[Luciano Coelho]

The pwr_in_suspend flag depends on the MMC settings which can be
retrieved from the SDIO subsystem, so it doesn't need to be part of
the platform data structure.  Move it to the platform device data that
is passed from SDIO to wlcore.

Signed-off-by: Luciano Coelho 
Reviewed-by: Felipe Balbi 
---
 drivers/net/wireless/ti/wlcore/main.c | 3 +--
 drivers/net/wireless/ti/wlcore/sdio.c | 2 +-
 drivers/net/wireless/ti/wlcore/wlcore_i.h | 1 +
 include/linux/wl12xx.h| 1 -
 4 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/ti/wlcore/main.c 
b/drivers/net/wireless/ti/wlcore/main.c
index 11dab9a..e771de0 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -5900,7 +5900,6 @@ static void wlcore_nvs_cb(const struct firmware *fw, void 
*context)
struct wl1271 *wl = context;
struct platform_device *pdev = wl->pdev;
struct wlcore_platdev_data *pdev_data = pdev->dev.platform_data;
-   struct wl12xx_platform_data *pdata = pdev_data->pdata;
 
int ret;
 
@@ -5947,7 +5946,7 @@ static void wlcore_nvs_cb(const struct firmware *fw, void 
*context)
if (!ret) {
wl->irq_wake_enabled = true;
device_init_wakeup(wl->dev, 1);
-   if (pdata->pwr_in_suspend)
+   if (pdev_data->pwr_in_suspend)
wl->hw->wiphy->wowlan = _wowlan_support;
}
 #endif
diff --git a/drivers/net/wireless/ti/wlcore/sdio.c 
b/drivers/net/wireless/ti/wlcore/sdio.c
index 29ef249..4c7e8ac 100644
--- a/drivers/net/wireless/ti/wlcore/sdio.c
+++ b/drivers/net/wireless/ti/wlcore/sdio.c
@@ -260,7 +260,7 @@ static int wl1271_probe(struct sdio_func *func,
dev_dbg(glue->dev, "sdio PM caps = 0x%x\n", mmcflags);
 
if (mmcflags & MMC_PM_KEEP_POWER)
-   pdev_data->pdata->pwr_in_suspend = true;
+   pdev_data->pwr_in_suspend = true;
 
sdio_set_drvdata(func, glue);
 
diff --git a/drivers/net/wireless/ti/wlcore/wlcore_i.h 
b/drivers/net/wireless/ti/wlcore/wlcore_i.h
index e5e1464..f2c4227 100644
--- a/drivers/net/wireless/ti/wlcore/wlcore_i.h
+++ b/drivers/net/wireless/ti/wlcore/wlcore_i.h
@@ -209,6 +209,7 @@ struct wl1271_if_operations {
 struct wlcore_platdev_data {
struct wl12xx_platform_data *pdata;
struct wl1271_if_operations *if_ops;
+   bool pwr_in_suspend;
 };
 
 #define MAX_NUM_KEYS 14
diff --git a/include/linux/wl12xx.h b/include/linux/wl12xx.h
index 1bfcd19..ab90b1c 100644
--- a/include/linux/wl12xx.h
+++ b/include/linux/wl12xx.h
@@ -59,7 +59,6 @@ struct wl12xx_platform_data {
int irq;
int board_ref_clock;
int board_tcxo_clock;
-   bool pwr_in_suspend;
 };
 
 #ifdef CONFIG_WILINK_PLATFORM_DATA
-- 
1.8.3.2

--
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/

[PATCH v4 4/8] wl12xx: use frequency instead of enumerations for pdata clocks

[Luciano Coelho]

Instead of defining an enumeration with the FW specific values for the
different clock rates, use the actual frequency instead.  Also add a
boolean to specify whether the clock is XTAL or not.

Change all board files to reflect this.

Additionally, this reverts commit 26f45c (ARM: OMAP2+: Legacy support
for wl12xx when booted with devicetree), since this is not be needed
anymore, now that DT support for WiLink is implemented.

Cc: Tony Lindgren 
Cc: Sekhar Nori 
Signed-off-by: Luciano Coelho 
Reviewed-by: Felipe Balbi 
---
 arch/arm/mach-davinci/board-da850-evm.c  |  3 +-
 arch/arm/mach-omap2/board-omap3evm.c |  3 +-
 arch/arm/mach-omap2/board-zoom-peripherals.c |  3 +-
 arch/arm/mach-omap2/devices.c| 39 ---
 drivers/net/wireless/ti/wl12xx/main.c| 58 +++-
 drivers/net/wireless/ti/wl12xx/wl12xx.h  | 28 ++
 include/linux/wl12xx.h   | 27 ++---
 7 files changed, 93 insertions(+), 68 deletions(-)

diff --git a/arch/arm/mach-davinci/board-da850-evm.c 
b/arch/arm/mach-davinci/board-da850-evm.c
index 03de2e9..6b2768f 100644
--- a/arch/arm/mach-davinci/board-da850-evm.c
+++ b/arch/arm/mach-davinci/board-da850-evm.c
@@ -1376,7 +1376,8 @@ static const short da850_wl12xx_pins[] __initconst = {
 
 static struct wl12xx_platform_data da850_wl12xx_wlan_data __initdata = {
.irq= -1,
-   .board_ref_clock= WL12XX_REFCLOCK_38,
+   .ref_clock_freq = 3840,
+   .ref_clock_xtal = false,
 };
 
 static __init int da850_wl12xx_init(void)
diff --git a/arch/arm/mach-omap2/board-omap3evm.c 
b/arch/arm/mach-omap2/board-omap3evm.c
index 9c7dfc5..4ccfcc0 100644
--- a/arch/arm/mach-omap2/board-omap3evm.c
+++ b/arch/arm/mach-omap2/board-omap3evm.c
@@ -460,7 +460,8 @@ static struct platform_device omap3evm_wlan_regulator = {
 };
 
 struct wl12xx_platform_data omap3evm_wlan_data __initdata = {
-   .board_ref_clock = WL12XX_REFCLOCK_38, /* 38.4 MHz */
+   .ref_clock_freq = 3840,
+   .ref_clock_xtal = false,
 };
 #endif
 
diff --git a/arch/arm/mach-omap2/board-zoom-peripherals.c 
b/arch/arm/mach-omap2/board-zoom-peripherals.c
index 4f84cf9..83a9a36 100644
--- a/arch/arm/mach-omap2/board-zoom-peripherals.c
+++ b/arch/arm/mach-omap2/board-zoom-peripherals.c
@@ -244,7 +244,8 @@ static struct platform_device *zoom_devices[] __initdata = {
 };
 
 static struct wl12xx_platform_data omap_zoom_wlan_data __initdata = {
-   .board_ref_clock = WL12XX_REFCLOCK_26, /* 26 MHz */
+   .ref_clock_freq = 2600,
+   .ref_clock_xtal = false,
 };
 
 static struct omap2_hsmmc_info mmc[] = {
diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
index 3c1279f..10e6126 100644
--- a/arch/arm/mach-omap2/devices.c
+++ b/arch/arm/mach-omap2/devices.c
@@ -8,7 +8,6 @@
  * the Free Software Foundation; either version 2 of the License, or
  * (at your option) any later version.
  */
-#include 
 #include 
 #include 
 #include 
@@ -19,7 +18,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 
 #include 
@@ -513,40 +511,6 @@ static void omap_init_vout(void)
 static inline void omap_init_vout(void) {}
 #endif
 
-#if IS_ENABLED(CONFIG_WL12XX)
-
-static struct wl12xx_platform_data wl12xx __initdata;
-
-void __init omap_init_wl12xx_of(void)
-{
-   int ret;
-
-   if (!of_have_populated_dt())
-   return;
-
-   if (of_machine_is_compatible("ti,omap4-sdp")) {
-   wl12xx.board_ref_clock = WL12XX_REFCLOCK_26;
-   wl12xx.board_tcxo_clock = WL12XX_TCXOCLOCK_26;
-   wl12xx.irq = gpio_to_irq(53);
-   } else if (of_machine_is_compatible("ti,omap4-panda")) {
-   wl12xx.board_ref_clock = WL12XX_REFCLOCK_38;
-   wl12xx.irq = gpio_to_irq(53);
-   } else {
-   return;
-   }
-
-   ret = wl12xx_set_platform_data();
-   if (ret) {
-   pr_err("error setting wl12xx data: %d\n", ret);
-   return;
-   }
-}
-#else
-static inline void omap_init_wl12xx_of(void)
-{
-}
-#endif
-
 /*-*/
 
 static int __init omap2_init_devices(void)
@@ -570,9 +534,6 @@ static int __init omap2_init_devices(void)
omap_init_mcspi();
omap_init_sham();
omap_init_aes();
-   } else {
-   /* These can be removed when bindings are done */
-   omap_init_wl12xx_of();
}
omap_init_sti();
omap_init_rng();
diff --git a/drivers/net/wireless/ti/wl12xx/main.c 
b/drivers/net/wireless/ti/wl12xx/main.c
index 1c627da..a6c2a14 100644
--- a/drivers/net/wireless/ti/wl12xx/main.c
+++ b/drivers/net/wireless/ti/wl12xx/main.c
@@ -1701,6 +1701,43 @@ static struct ieee80211_sta_ht_cap wl12xx_ht_cap = {
},
 };
 
+static const struct wl12

[PATCH v4 0/8] wilink: add device tree support

[Luciano Coelho]

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.


Luciano Coelho (8):
  wl1251: split wl251 platform data to a separate structure
  wlcore: set irq_flags in the board files instead of hiding behind a
quirk
  wlcore: remove pwr_in_suspend from platform data
  wl12xx: use frequency instead of enumerations for pdata clocks
  wlcore: add initial device tree support to the sdio module
  wlcore: sdio: add wilink clock providers
  wlcore: sdio: get clocks from device tree
  wlcore/wl12xx: check if we got correct clock data from DT

 arch/arm/mach-davinci/board-da850-evm.c|  11 ++-
 arch/arm/mach-omap2/board-omap3evm.c   |  22 -
 arch/arm/mach-omap2/board-omap3pandora.c   |   4 +-
 arch/arm/mach-omap2/board-rx51-peripherals.c   |   2 +-
 arch/arm/mach-omap2/board-zoom-peripherals.c   |  33 +++-
 arch/arm/mach-omap2/devices.c  |  39 -
 drivers/net/wireless/ti/wilink_platform_data.c |  37 ++--
 drivers/net/wireless/ti/wl1251/sdio.c  |  12 +--
 drivers/net/wireless/ti/wl1251/spi.c   |   2 +-
 drivers/net/wireless/ti/wl12xx/main.c  |  78 +++--
 drivers/net/wireless/ti/wl12xx/wl12xx.h|  28 +++
 drivers/net/wireless/ti/wlcore/debugfs.c   |   2 +-
 drivers/net/wireless/ti/wlcore/main.c  |  20 ++---
 drivers/net/wireless/ti/wlcore/sdio.c  | 112 +++--
 drivers/net/wireless/ti/wlcore/wlcore.h|   5 +-
 drivers/net/wireless/ti/wlcore/wlcore_i.h  |   1 +
 include/linux/wl12xx.h |  52 +---
 17 files changed, 340 insertions(+), 120 deletions(-)

-- 
1.8.3.2

--
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/

[PATCH v4 6/8] wlcore: sdio: add wilink clock providers

[Luciano Coelho]

Add refclock and tcxoclock as clock providers in WiLink.  These clocks
are not accesible outside the WiLink module, but they are registered
in the clock framework anyway.  Only the WiLink chip consumes these
clocks.

In theory, the WiLink chip could be connected to external clocks
instead of using these internal clocks, so make the clock consumer
code generic enough.  If external clocks are used, then the internal
clock device tree nodes are not necessary, but the external ones must
be specified.

Signed-off-by: Luciano Coelho 
Reviewed-by: Felipe Balbi 
---
 drivers/net/wireless/ti/wlcore/sdio.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/drivers/net/wireless/ti/wlcore/sdio.c 
b/drivers/net/wireless/ti/wlcore/sdio.c
index 9370d7e..980bf3d 100644
--- a/drivers/net/wireless/ti/wlcore/sdio.c
+++ b/drivers/net/wireless/ti/wlcore/sdio.c
@@ -34,6 +34,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "wlcore.h"
 #include "wl12xx_80211.h"
@@ -214,10 +215,15 @@ static struct wl1271_if_operations sdio_ops = {
.set_block_size = wl1271_sdio_set_block_size,
 };
 
+static const struct of_device_id wlcore_sdio_of_clk_match_table[] = {
+   { .compatible = "ti,wilink-clock" },
+};
+
 static struct wl12xx_platform_data *wlcore_get_pdata_from_of(struct device 
*dev)
 {
struct wl12xx_platform_data *pdata;
struct device_node *np = dev->of_node;
+   struct device_node *clock_node;
 
if (!np) {
np = of_find_matching_node(NULL, dev->driver->of_match_table);
@@ -241,6 +247,9 @@ static struct wl12xx_platform_data 
*wlcore_get_pdata_from_of(struct device *dev)
goto out_free;
}
 
+   for_each_matching_node(clock_node, wlcore_sdio_of_clk_match_table)
+   of_fixed_clk_setup(clock_node);
+
goto out;
 
 out_free:
-- 
1.8.3.2

--
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/

[PATCH v4 8/8] wlcore/wl12xx: check if we got correct clock data from DT

[Luciano Coelho]

The fref and the tcxo clocks settings are optional in some platforms.
WiLink8 doesn't need either, so we don't check the values.  WiLink 6
only needs the fref clock, so we check that it is valid or return with
an error.  WiLink7 needs both clocks, if either is not available we
return with an error.

Signed-off-by: Luciano Coelho 
Reviewed-by: Felipe Balbi 
---
 drivers/net/wireless/ti/wl12xx/main.c | 20 +---
 drivers/net/wireless/ti/wlcore/sdio.c |  4 
 2 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/ti/wl12xx/main.c 
b/drivers/net/wireless/ti/wl12xx/main.c
index a6c2a14..60d2ff4 100644
--- a/drivers/net/wireless/ti/wl12xx/main.c
+++ b/drivers/net/wireless/ti/wl12xx/main.c
@@ -927,6 +927,11 @@ static int wl128x_boot_clk(struct wl1271 *wl, int 
*selected_clock)
u16 sys_clk_cfg;
int ret;
 
+   if ((priv->ref_clock < 0) || (priv->tcxo_clock < 0)) {
+   wl1271_error("Missing fref and/or tcxo clock settings\n");
+   return -EINVAL;
+   }
+
/* For XTAL-only modes, FREF will be used after switching from TCXO */
if (priv->ref_clock == WL12XX_REFCLOCK_26_XTAL ||
priv->ref_clock == WL12XX_REFCLOCK_38_XTAL) {
@@ -976,6 +981,11 @@ static int wl127x_boot_clk(struct wl1271 *wl)
u32 clk;
int ret;
 
+   if (priv->ref_clock < 0) {
+   wl1271_error("Missing fref clock settings\n");
+   return -EINVAL;
+   }
+
if (WL127X_PG_GET_MAJOR(wl->hw_pg_ver) < 3)
wl->quirks |= WLCORE_QUIRK_END_OF_TRANSACTION;
 
@@ -1758,7 +1768,7 @@ static int wl12xx_setup(struct wl1271 *wl)
wlcore_set_ht_cap(wl, IEEE80211_BAND_5GHZ, _ht_cap);
wl12xx_conf_init(wl);
 
-   if (!fref_param) {
+   if (!fref_param && (pdata->ref_clock_freq > 0)) {
priv->ref_clock = wl12xx_get_clock_idx(wl12xx_refclock_table,
   pdata->ref_clock_freq,
   pdata->ref_clock_xtal);
@@ -1769,6 +1779,8 @@ static int wl12xx_setup(struct wl1271 *wl)
 
return priv->ref_clock;
}
+   } else if (!fref_param) {
+   priv->ref_clock = -EINVAL;
} else {
if (!strcmp(fref_param, "19.2"))
priv->ref_clock = WL12XX_REFCLOCK_19;
@@ -1786,7 +1798,7 @@ static int wl12xx_setup(struct wl1271 *wl)
wl1271_error("Invalid fref parameter %s", fref_param);
}
 
-   if (!tcxo_param) {
+   if (!fref_param && (pdata->tcxo_clock_freq > 0)) {
priv->tcxo_clock = wl12xx_get_clock_idx(wl12xx_tcxoclock_table,
pdata->tcxo_clock_freq,
true);
@@ -1796,7 +1808,9 @@ static int wl12xx_setup(struct wl1271 *wl)
 
return priv->tcxo_clock;
}
-   } else {
+   } else if (!fref_param) {
+   priv->tcxo_clock = -EINVAL;
+   }else {
if (!strcmp(tcxo_param, "19.2"))
priv->tcxo_clock = WL12XX_TCXOCLOCK_19_2;
else if (!strcmp(tcxo_param, "26"))
diff --git a/drivers/net/wireless/ti/wlcore/sdio.c 
b/drivers/net/wireless/ti/wlcore/sdio.c
index 60fce49..c76eb66 100644
--- a/drivers/net/wireless/ti/wlcore/sdio.c
+++ b/drivers/net/wireless/ti/wlcore/sdio.c
@@ -252,20 +252,16 @@ static struct wl12xx_platform_data 
*wlcore_get_pdata_from_of(struct device *dev)
for_each_matching_node(clock_node, wlcore_sdio_of_clk_match_table)
of_fixed_clk_setup(clock_node);
 
-   /* TODO: make sure we have this when needed (ie. for WL6 and WL7) */
glue->refclock = of_clk_get_by_name(np, "refclock");
if (IS_ERR(glue->refclock)) {
-   dev_err(dev, "couldn't find refclock on the device tree\n");
glue->refclock = NULL;
} else {
clk_prepare_enable(glue->refclock);
pdata->ref_clock_freq = clk_get_rate(glue->refclock);
}
 
-   /* TODO: make sure we have this when needed (ie. for WL7) */
glue->tcxoclock = of_clk_get_by_name(np, "tcxoclock");
if (IS_ERR(glue->tcxoclock)) {
-   dev_err(dev, "couldn't find tcxoclock on the device tree\n");
glue->tcxoclock = NULL;
} else {
clk_prepare_enable(glue->tcxoclock);
-- 
1.8.3.2

--
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/

[PATCH v4 2/8] wlcore: set irq_flags in the board files instead of hiding behind a quirk

[Luciano Coelho]

The platform_quirk element in the platform data was used to change the
way the IRQ is triggered.  When set, the EDGE_IRQ quirk would change
the irqflags used and treat edge trigger differently from the rest.

Instead of hiding this irq flag setting behind the quirk, have the
board files set the flags during initialization.  This will be more
meaningful than driver-specific quirks when we switch to DT.

Additionally, fix missing gpio_request() calls in the boarding files
(so that setting the flags actually works).

Cc: Tony Lindgren 
Cc: Sekhar Nori 
Signed-off-by: Luciano Coelho 
Reviewed-by: Felipe Balbi 
Acked-by: Sekhar Nori 
---
 arch/arm/mach-davinci/board-da850-evm.c  |  8 +++-
 arch/arm/mach-omap2/board-omap3evm.c | 19 ++
 arch/arm/mach-omap2/board-zoom-peripherals.c | 30 +---
 drivers/net/wireless/ti/wlcore/debugfs.c |  2 +-
 drivers/net/wireless/ti/wlcore/main.c| 17 
 drivers/net/wireless/ti/wlcore/wlcore.h  |  5 ++---
 include/linux/wl12xx.h   |  4 
 7 files changed, 64 insertions(+), 21 deletions(-)

diff --git a/arch/arm/mach-davinci/board-da850-evm.c 
b/arch/arm/mach-davinci/board-da850-evm.c
index bea6793..03de2e9 100644
--- a/arch/arm/mach-davinci/board-da850-evm.c
+++ b/arch/arm/mach-davinci/board-da850-evm.c
@@ -1377,7 +1377,6 @@ static const short da850_wl12xx_pins[] __initconst = {
 static struct wl12xx_platform_data da850_wl12xx_wlan_data __initdata = {
.irq= -1,
.board_ref_clock= WL12XX_REFCLOCK_38,
-   .platform_quirks= WL12XX_PLATFORM_QUIRK_EDGE_IRQ,
 };
 
 static __init int da850_wl12xx_init(void)
@@ -1408,6 +1407,13 @@ static __init int da850_wl12xx_init(void)
goto free_wlan_en;
}
 
+   ret = irq_set_irq_type(gpio_to_irq(DA850_WLAN_IRQ),
+  IRQ_TYPE_EDGE_RISING);
+   if (ret) {
+   pr_err("Could not set wl12xx irq type: %d\n", ret);
+   goto free;
+   }
+
da850_wl12xx_wlan_data.irq = gpio_to_irq(DA850_WLAN_IRQ);
 
ret = wl12xx_set_platform_data(_wl12xx_wlan_data);
diff --git a/arch/arm/mach-omap2/board-omap3evm.c 
b/arch/arm/mach-omap2/board-omap3evm.c
index 8c02626..9c7dfc5 100644
--- a/arch/arm/mach-omap2/board-omap3evm.c
+++ b/arch/arm/mach-omap2/board-omap3evm.c
@@ -614,12 +614,31 @@ static void __init omap3_evm_wl12xx_init(void)
 
/* WL12xx WLAN Init */
omap3evm_wlan_data.irq = gpio_to_irq(OMAP3EVM_WLAN_IRQ_GPIO);
+
+   ret = gpio_request_one(OMAP3EVM_WLAN_IRQ_GPIO, GPIOF_IN,
+  "OMAP3EVM_WLAN_IRQ_GPIO");
+   if (ret) {
+   pr_err("error requesting wl12xx gpio: %d\n", ret);
+   goto out;
+   }
+
+   ret = irq_set_irq_type(gpio_to_irq(OMAP3EVM_WLAN_IRQ_GPIO),
+  IRQ_TYPE_LEVEL_HIGH);
+   if (ret) {
+   pr_err("error setting wl12xx irq type: %d\n", ret);
+   goto free;
+   }
+
ret = wl12xx_set_platform_data(_wlan_data);
if (ret)
pr_err("error setting wl12xx data: %d\n", ret);
ret = platform_device_register(_wlan_regulator);
if (ret)
pr_err("error registering wl12xx device: %d\n", ret);
+out:
+   return;
+free:
+   gpio_free(OMAP3EVM_WLAN_IRQ_GPIO);
 #endif
 }
 
diff --git a/arch/arm/mach-omap2/board-zoom-peripherals.c 
b/arch/arm/mach-omap2/board-zoom-peripherals.c
index a90375d..4f84cf9 100644
--- a/arch/arm/mach-omap2/board-zoom-peripherals.c
+++ b/arch/arm/mach-omap2/board-zoom-peripherals.c
@@ -339,16 +339,40 @@ static void enable_board_wakeup_source(void)
OMAP_WAKEUP_EN | OMAP_PIN_INPUT_PULLUP);
 }
 
-void __init zoom_peripherals_init(void)
+static void __init zoom_wilink_init(void)
 {
int ret;
 
omap_zoom_wlan_data.irq = gpio_to_irq(OMAP_ZOOM_WLAN_IRQ_GPIO);
-   ret = wl12xx_set_platform_data(_zoom_wlan_data);
 
-   if (ret)
+   ret = gpio_request_one(OMAP_ZOOM_WLAN_IRQ_GPIO, GPIOF_IN,
+  "OMAP_ZOOM_WLAN_IRQ_GPIO");
+   if (ret) {
+   pr_err("error requesting wl12xx gpio: %d\n", ret);
+   goto out;
+   }
+
+   ret = irq_set_irq_type(gpio_to_irq(OMAP_ZOOM_WLAN_IRQ_GPIO),
+  IRQ_TYPE_LEVEL_HIGH);
+   if (ret) {
+   pr_err("error setting wl12xx irq type: %d\n", ret);
+   goto free;
+   }
+
+   ret = wl12xx_set_platform_data(_zoom_wlan_data);
+   if (ret) {
pr_err("error setting wl12xx data: %d\n", ret);
+   goto free;
+   }
+out:
+   return;
+free:
+   gpio_free(OMAP_ZOOM_WLAN_IRQ_GPIO);
+}
 
+void __init zoom_peripherals_init(void)
+{
+   zoom_wilink_init();
omap_hsmmc_i

[PATCH v4 7/8] wlcore: sdio: get clocks from device tree

[Luciano Coelho]

Read the clock nodes from the device tree and use them to set the
frequency for the refclock and the tcxo clock.

Also, call sdio_set_drvdata() earlier, so the glue is already set in
the driver data when we call wlcore_get_pdata_from_of() and we don't
need to pass it as a parameter.

Signed-off-by: Luciano Coelho 
Reviewed-by: Felipe Balbi 
---
 drivers/net/wireless/ti/wlcore/sdio.c | 36 +--
 1 file changed, 34 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ti/wlcore/sdio.c 
b/drivers/net/wireless/ti/wlcore/sdio.c
index 980bf3d..60fce49 100644
--- a/drivers/net/wireless/ti/wlcore/sdio.c
+++ b/drivers/net/wireless/ti/wlcore/sdio.c
@@ -53,6 +53,7 @@ static bool dump = false;
 struct wl12xx_sdio_glue {
struct device *dev;
struct platform_device *core;
+   struct clk *refclock, *tcxoclock;
 };
 
 static const struct sdio_device_id wl1271_devices[] = {
@@ -224,6 +225,7 @@ static struct wl12xx_platform_data 
*wlcore_get_pdata_from_of(struct device *dev)
struct wl12xx_platform_data *pdata;
struct device_node *np = dev->of_node;
struct device_node *clock_node;
+   struct wl12xx_sdio_glue *glue = sdio_get_drvdata(dev_to_sdio_func(dev));
 
if (!np) {
np = of_find_matching_node(NULL, dev->driver->of_match_table);
@@ -250,6 +252,26 @@ static struct wl12xx_platform_data 
*wlcore_get_pdata_from_of(struct device *dev)
for_each_matching_node(clock_node, wlcore_sdio_of_clk_match_table)
of_fixed_clk_setup(clock_node);
 
+   /* TODO: make sure we have this when needed (ie. for WL6 and WL7) */
+   glue->refclock = of_clk_get_by_name(np, "refclock");
+   if (IS_ERR(glue->refclock)) {
+   dev_err(dev, "couldn't find refclock on the device tree\n");
+   glue->refclock = NULL;
+   } else {
+   clk_prepare_enable(glue->refclock);
+   pdata->ref_clock_freq = clk_get_rate(glue->refclock);
+   }
+
+   /* TODO: make sure we have this when needed (ie. for WL7) */
+   glue->tcxoclock = of_clk_get_by_name(np, "tcxoclock");
+   if (IS_ERR(glue->tcxoclock)) {
+   dev_err(dev, "couldn't find tcxoclock on the device tree\n");
+   glue->tcxoclock = NULL;
+   } else {
+   clk_prepare_enable(glue->tcxoclock);
+   pdata->ref_clock_freq = clk_get_rate(glue->tcxoclock);
+   }
+
goto out;
 
 out_free:
@@ -294,6 +316,8 @@ static int wl1271_probe(struct sdio_func *func,
/* Use block mode for transferring over one block size of data */
func->card->quirks |= MMC_QUIRK_BLKSZ_FOR_BYTE_MODE;
 
+   sdio_set_drvdata(func, glue);
+
/* The pdata allocated here is freed when the device is freed,
 * so we don't need an additional out label to free it in case
 * of error further on.
@@ -319,8 +343,6 @@ static int wl1271_probe(struct sdio_func *func,
if (mmcflags & MMC_PM_KEEP_POWER)
pdev_data->pwr_in_suspend = true;
 
-   sdio_set_drvdata(func, glue);
-
/* Tell PM core that we don't need the card to be powered now */
pm_runtime_put_noidle(>dev);
 
@@ -387,6 +409,16 @@ static void wl1271_remove(struct sdio_func *func)
 {
struct wl12xx_sdio_glue *glue = sdio_get_drvdata(func);
 
+   if (glue->refclock) {
+   clk_disable_unprepare(glue->refclock);
+   clk_put(glue->refclock);
+   }
+
+   if (glue->tcxoclock) {
+   clk_disable_unprepare(glue->tcxoclock);
+   clk_put(glue->tcxoclock);
+   }
+
/* Undo decrement done above in wl1271_probe */
pm_runtime_get_noresume(>dev);
 
-- 
1.8.3.2

--
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/

[PATCH v4 0/8] wilink: add device tree support

[Luciano Coelho]

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.


Luciano Coelho (8):
  wl1251: split wl251 platform data to a separate structure
  wlcore: set irq_flags in the board files instead of hiding behind a
quirk
  wlcore: remove pwr_in_suspend from platform data
  wl12xx: use frequency instead of enumerations for pdata clocks
  wlcore: add initial device tree support to the sdio module
  wlcore: sdio: add wilink clock providers
  wlcore: sdio: get clocks from device tree
  wlcore/wl12xx: check if we got correct clock data from DT

 arch/arm/mach-davinci/board-da850-evm.c|  11 ++-
 arch/arm/mach-omap2/board-omap3evm.c   |  22 -
 arch/arm/mach-omap2/board-omap3pandora.c   |   4 +-
 arch/arm/mach-omap2/board-rx51-peripherals.c   |   2 +-
 arch/arm/mach-omap2/board-zoom-peripherals.c   |  33 +++-
 arch/arm/mach-omap2/devices.c  |  39 -
 drivers/net/wireless/ti/wilink_platform_data.c |  37 ++--
 drivers/net/wireless/ti/wl1251/sdio.c  |  12 +--
 drivers/net/wireless/ti/wl1251/spi.c   |   2 +-
 drivers/net/wireless/ti/wl12xx/main.c  |  78 +++--
 drivers/net/wireless/ti/wl12xx/wl12xx.h|  28 +++
 drivers/net/wireless/ti/wlcore/debugfs.c   |   2 +-
 drivers/net/wireless/ti/wlcore/main.c  |  20 ++---
 drivers/net/wireless/ti/wlcore/sdio.c  | 112 +++--
 drivers/net/wireless/ti/wlcore/wlcore.h|   5 +-
 drivers/net/wireless/ti/wlcore/wlcore_i.h  |   1 +
 include/linux/wl12xx.h |  52 +---
 17 files changed, 340 insertions(+), 120 deletions(-)

-- 
1.8.3.2

--
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/

[PATCH v4 6/8] wlcore: sdio: add wilink clock providers

[Luciano Coelho]

Add refclock and tcxoclock as clock providers in WiLink.  These clocks
are not accesible outside the WiLink module, but they are registered
in the clock framework anyway.  Only the WiLink chip consumes these
clocks.

In theory, the WiLink chip could be connected to external clocks
instead of using these internal clocks, so make the clock consumer
code generic enough.  If external clocks are used, then the internal
clock device tree nodes are not necessary, but the external ones must
be specified.

Signed-off-by: Luciano Coelho coe...@ti.com
Reviewed-by: Felipe Balbi ba...@ti.com
---
 drivers/net/wireless/ti/wlcore/sdio.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/drivers/net/wireless/ti/wlcore/sdio.c 
b/drivers/net/wireless/ti/wlcore/sdio.c
index 9370d7e..980bf3d 100644
--- a/drivers/net/wireless/ti/wlcore/sdio.c
+++ b/drivers/net/wireless/ti/wlcore/sdio.c
@@ -34,6 +34,7 @@
 #include linux/wl12xx.h
 #include linux/pm_runtime.h
 #include linux/printk.h
+#include linux/clk-provider.h
 
 #include wlcore.h
 #include wl12xx_80211.h
@@ -214,10 +215,15 @@ static struct wl1271_if_operations sdio_ops = {
.set_block_size = wl1271_sdio_set_block_size,
 };
 
+static const struct of_device_id wlcore_sdio_of_clk_match_table[] = {
+   { .compatible = ti,wilink-clock },
+};
+
 static struct wl12xx_platform_data *wlcore_get_pdata_from_of(struct device 
*dev)
 {
struct wl12xx_platform_data *pdata;
struct device_node *np = dev-of_node;
+   struct device_node *clock_node;
 
if (!np) {
np = of_find_matching_node(NULL, dev-driver-of_match_table);
@@ -241,6 +247,9 @@ static struct wl12xx_platform_data 
*wlcore_get_pdata_from_of(struct device *dev)
goto out_free;
}
 
+   for_each_matching_node(clock_node, wlcore_sdio_of_clk_match_table)
+   of_fixed_clk_setup(clock_node);
+
goto out;
 
 out_free:
-- 
1.8.3.2

--
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/

[PATCH v4 7/8] wlcore: sdio: get clocks from device tree

[Luciano Coelho]

Read the clock nodes from the device tree and use them to set the
frequency for the refclock and the tcxo clock.

Also, call sdio_set_drvdata() earlier, so the glue is already set in
the driver data when we call wlcore_get_pdata_from_of() and we don't
need to pass it as a parameter.

Signed-off-by: Luciano Coelho coe...@ti.com
Reviewed-by: Felipe Balbi ba...@ti.com
---
 drivers/net/wireless/ti/wlcore/sdio.c | 36 +--
 1 file changed, 34 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ti/wlcore/sdio.c 
b/drivers/net/wireless/ti/wlcore/sdio.c
index 980bf3d..60fce49 100644
--- a/drivers/net/wireless/ti/wlcore/sdio.c
+++ b/drivers/net/wireless/ti/wlcore/sdio.c
@@ -53,6 +53,7 @@ static bool dump = false;
 struct wl12xx_sdio_glue {
struct device *dev;
struct platform_device *core;
+   struct clk *refclock, *tcxoclock;
 };
 
 static const struct sdio_device_id wl1271_devices[] = {
@@ -224,6 +225,7 @@ static struct wl12xx_platform_data 
*wlcore_get_pdata_from_of(struct device *dev)
struct wl12xx_platform_data *pdata;
struct device_node *np = dev-of_node;
struct device_node *clock_node;
+   struct wl12xx_sdio_glue *glue = sdio_get_drvdata(dev_to_sdio_func(dev));
 
if (!np) {
np = of_find_matching_node(NULL, dev-driver-of_match_table);
@@ -250,6 +252,26 @@ static struct wl12xx_platform_data 
*wlcore_get_pdata_from_of(struct device *dev)
for_each_matching_node(clock_node, wlcore_sdio_of_clk_match_table)
of_fixed_clk_setup(clock_node);
 
+   /* TODO: make sure we have this when needed (ie. for WL6 and WL7) */
+   glue-refclock = of_clk_get_by_name(np, refclock);
+   if (IS_ERR(glue-refclock)) {
+   dev_err(dev, couldn't find refclock on the device tree\n);
+   glue-refclock = NULL;
+   } else {
+   clk_prepare_enable(glue-refclock);
+   pdata-ref_clock_freq = clk_get_rate(glue-refclock);
+   }
+
+   /* TODO: make sure we have this when needed (ie. for WL7) */
+   glue-tcxoclock = of_clk_get_by_name(np, tcxoclock);
+   if (IS_ERR(glue-tcxoclock)) {
+   dev_err(dev, couldn't find tcxoclock on the device tree\n);
+   glue-tcxoclock = NULL;
+   } else {
+   clk_prepare_enable(glue-tcxoclock);
+   pdata-ref_clock_freq = clk_get_rate(glue-tcxoclock);
+   }
+
goto out;
 
 out_free:
@@ -294,6 +316,8 @@ static int wl1271_probe(struct sdio_func *func,
/* Use block mode for transferring over one block size of data */
func-card-quirks |= MMC_QUIRK_BLKSZ_FOR_BYTE_MODE;
 
+   sdio_set_drvdata(func, glue);
+
/* The pdata allocated here is freed when the device is freed,
 * so we don't need an additional out label to free it in case
 * of error further on.
@@ -319,8 +343,6 @@ static int wl1271_probe(struct sdio_func *func,
if (mmcflags  MMC_PM_KEEP_POWER)
pdev_data-pwr_in_suspend = true;
 
-   sdio_set_drvdata(func, glue);
-
/* Tell PM core that we don't need the card to be powered now */
pm_runtime_put_noidle(func-dev);
 
@@ -387,6 +409,16 @@ static void wl1271_remove(struct sdio_func *func)
 {
struct wl12xx_sdio_glue *glue = sdio_get_drvdata(func);
 
+   if (glue-refclock) {
+   clk_disable_unprepare(glue-refclock);
+   clk_put(glue-refclock);
+   }
+
+   if (glue-tcxoclock) {
+   clk_disable_unprepare(glue-tcxoclock);
+   clk_put(glue-tcxoclock);
+   }
+
/* Undo decrement done above in wl1271_probe */
pm_runtime_get_noresume(func-dev);
 
-- 
1.8.3.2

--
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/

[PATCH v4 8/8] wlcore/wl12xx: check if we got correct clock data from DT

[Luciano Coelho]

The fref and the tcxo clocks settings are optional in some platforms.
WiLink8 doesn't need either, so we don't check the values.  WiLink 6
only needs the fref clock, so we check that it is valid or return with
an error.  WiLink7 needs both clocks, if either is not available we
return with an error.

Signed-off-by: Luciano Coelho coe...@ti.com
Reviewed-by: Felipe Balbi ba...@ti.com
---
 drivers/net/wireless/ti/wl12xx/main.c | 20 +---
 drivers/net/wireless/ti/wlcore/sdio.c |  4 
 2 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/ti/wl12xx/main.c 
b/drivers/net/wireless/ti/wl12xx/main.c
index a6c2a14..60d2ff4 100644
--- a/drivers/net/wireless/ti/wl12xx/main.c
+++ b/drivers/net/wireless/ti/wl12xx/main.c
@@ -927,6 +927,11 @@ static int wl128x_boot_clk(struct wl1271 *wl, int 
*selected_clock)
u16 sys_clk_cfg;
int ret;
 
+   if ((priv-ref_clock  0) || (priv-tcxo_clock  0)) {
+   wl1271_error(Missing fref and/or tcxo clock settings\n);
+   return -EINVAL;
+   }
+
/* For XTAL-only modes, FREF will be used after switching from TCXO */
if (priv-ref_clock == WL12XX_REFCLOCK_26_XTAL ||
priv-ref_clock == WL12XX_REFCLOCK_38_XTAL) {
@@ -976,6 +981,11 @@ static int wl127x_boot_clk(struct wl1271 *wl)
u32 clk;
int ret;
 
+   if (priv-ref_clock  0) {
+   wl1271_error(Missing fref clock settings\n);
+   return -EINVAL;
+   }
+
if (WL127X_PG_GET_MAJOR(wl-hw_pg_ver)  3)
wl-quirks |= WLCORE_QUIRK_END_OF_TRANSACTION;
 
@@ -1758,7 +1768,7 @@ static int wl12xx_setup(struct wl1271 *wl)
wlcore_set_ht_cap(wl, IEEE80211_BAND_5GHZ, wl12xx_ht_cap);
wl12xx_conf_init(wl);
 
-   if (!fref_param) {
+   if (!fref_param  (pdata-ref_clock_freq  0)) {
priv-ref_clock = wl12xx_get_clock_idx(wl12xx_refclock_table,
   pdata-ref_clock_freq,
   pdata-ref_clock_xtal);
@@ -1769,6 +1779,8 @@ static int wl12xx_setup(struct wl1271 *wl)
 
return priv-ref_clock;
}
+   } else if (!fref_param) {
+   priv-ref_clock = -EINVAL;
} else {
if (!strcmp(fref_param, 19.2))
priv-ref_clock = WL12XX_REFCLOCK_19;
@@ -1786,7 +1798,7 @@ static int wl12xx_setup(struct wl1271 *wl)
wl1271_error(Invalid fref parameter %s, fref_param);
}
 
-   if (!tcxo_param) {
+   if (!fref_param  (pdata-tcxo_clock_freq  0)) {
priv-tcxo_clock = wl12xx_get_clock_idx(wl12xx_tcxoclock_table,
pdata-tcxo_clock_freq,
true);
@@ -1796,7 +1808,9 @@ static int wl12xx_setup(struct wl1271 *wl)
 
return priv-tcxo_clock;
}
-   } else {
+   } else if (!fref_param) {
+   priv-tcxo_clock = -EINVAL;
+   }else {
if (!strcmp(tcxo_param, 19.2))
priv-tcxo_clock = WL12XX_TCXOCLOCK_19_2;
else if (!strcmp(tcxo_param, 26))
diff --git a/drivers/net/wireless/ti/wlcore/sdio.c 
b/drivers/net/wireless/ti/wlcore/sdio.c
index 60fce49..c76eb66 100644
--- a/drivers/net/wireless/ti/wlcore/sdio.c
+++ b/drivers/net/wireless/ti/wlcore/sdio.c
@@ -252,20 +252,16 @@ static struct wl12xx_platform_data 
*wlcore_get_pdata_from_of(struct device *dev)
for_each_matching_node(clock_node, wlcore_sdio_of_clk_match_table)
of_fixed_clk_setup(clock_node);
 
-   /* TODO: make sure we have this when needed (ie. for WL6 and WL7) */
glue-refclock = of_clk_get_by_name(np, refclock);
if (IS_ERR(glue-refclock)) {
-   dev_err(dev, couldn't find refclock on the device tree\n);
glue-refclock = NULL;
} else {
clk_prepare_enable(glue-refclock);
pdata-ref_clock_freq = clk_get_rate(glue-refclock);
}
 
-   /* TODO: make sure we have this when needed (ie. for WL7) */
glue-tcxoclock = of_clk_get_by_name(np, tcxoclock);
if (IS_ERR(glue-tcxoclock)) {
-   dev_err(dev, couldn't find tcxoclock on the device tree\n);
glue-tcxoclock = NULL;
} else {
clk_prepare_enable(glue-tcxoclock);
-- 
1.8.3.2

--
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/

[PATCH v4 2/8] wlcore: set irq_flags in the board files instead of hiding behind a quirk

[Luciano Coelho]

The platform_quirk element in the platform data was used to change the
way the IRQ is triggered.  When set, the EDGE_IRQ quirk would change
the irqflags used and treat edge trigger differently from the rest.

Instead of hiding this irq flag setting behind the quirk, have the
board files set the flags during initialization.  This will be more
meaningful than driver-specific quirks when we switch to DT.

Additionally, fix missing gpio_request() calls in the boarding files
(so that setting the flags actually works).

Cc: Tony Lindgren t...@atomide.com
Cc: Sekhar Nori nsek...@ti.com
Signed-off-by: Luciano Coelho coe...@ti.com
Reviewed-by: Felipe Balbi ba...@ti.com
Acked-by: Sekhar Nori nsek...@ti.com
---
 arch/arm/mach-davinci/board-da850-evm.c  |  8 +++-
 arch/arm/mach-omap2/board-omap3evm.c | 19 ++
 arch/arm/mach-omap2/board-zoom-peripherals.c | 30 +---
 drivers/net/wireless/ti/wlcore/debugfs.c |  2 +-
 drivers/net/wireless/ti/wlcore/main.c| 17 
 drivers/net/wireless/ti/wlcore/wlcore.h  |  5 ++---
 include/linux/wl12xx.h   |  4 
 7 files changed, 64 insertions(+), 21 deletions(-)

diff --git a/arch/arm/mach-davinci/board-da850-evm.c 
b/arch/arm/mach-davinci/board-da850-evm.c
index bea6793..03de2e9 100644
--- a/arch/arm/mach-davinci/board-da850-evm.c
+++ b/arch/arm/mach-davinci/board-da850-evm.c
@@ -1377,7 +1377,6 @@ static const short da850_wl12xx_pins[] __initconst = {
 static struct wl12xx_platform_data da850_wl12xx_wlan_data __initdata = {
.irq= -1,
.board_ref_clock= WL12XX_REFCLOCK_38,
-   .platform_quirks= WL12XX_PLATFORM_QUIRK_EDGE_IRQ,
 };
 
 static __init int da850_wl12xx_init(void)
@@ -1408,6 +1407,13 @@ static __init int da850_wl12xx_init(void)
goto free_wlan_en;
}
 
+   ret = irq_set_irq_type(gpio_to_irq(DA850_WLAN_IRQ),
+  IRQ_TYPE_EDGE_RISING);
+   if (ret) {
+   pr_err(Could not set wl12xx irq type: %d\n, ret);
+   goto free;
+   }
+
da850_wl12xx_wlan_data.irq = gpio_to_irq(DA850_WLAN_IRQ);
 
ret = wl12xx_set_platform_data(da850_wl12xx_wlan_data);
diff --git a/arch/arm/mach-omap2/board-omap3evm.c 
b/arch/arm/mach-omap2/board-omap3evm.c
index 8c02626..9c7dfc5 100644
--- a/arch/arm/mach-omap2/board-omap3evm.c
+++ b/arch/arm/mach-omap2/board-omap3evm.c
@@ -614,12 +614,31 @@ static void __init omap3_evm_wl12xx_init(void)
 
/* WL12xx WLAN Init */
omap3evm_wlan_data.irq = gpio_to_irq(OMAP3EVM_WLAN_IRQ_GPIO);
+
+   ret = gpio_request_one(OMAP3EVM_WLAN_IRQ_GPIO, GPIOF_IN,
+  OMAP3EVM_WLAN_IRQ_GPIO);
+   if (ret) {
+   pr_err(error requesting wl12xx gpio: %d\n, ret);
+   goto out;
+   }
+
+   ret = irq_set_irq_type(gpio_to_irq(OMAP3EVM_WLAN_IRQ_GPIO),
+  IRQ_TYPE_LEVEL_HIGH);
+   if (ret) {
+   pr_err(error setting wl12xx irq type: %d\n, ret);
+   goto free;
+   }
+
ret = wl12xx_set_platform_data(omap3evm_wlan_data);
if (ret)
pr_err(error setting wl12xx data: %d\n, ret);
ret = platform_device_register(omap3evm_wlan_regulator);
if (ret)
pr_err(error registering wl12xx device: %d\n, ret);
+out:
+   return;
+free:
+   gpio_free(OMAP3EVM_WLAN_IRQ_GPIO);
 #endif
 }
 
diff --git a/arch/arm/mach-omap2/board-zoom-peripherals.c 
b/arch/arm/mach-omap2/board-zoom-peripherals.c
index a90375d..4f84cf9 100644
--- a/arch/arm/mach-omap2/board-zoom-peripherals.c
+++ b/arch/arm/mach-omap2/board-zoom-peripherals.c
@@ -339,16 +339,40 @@ static void enable_board_wakeup_source(void)
OMAP_WAKEUP_EN | OMAP_PIN_INPUT_PULLUP);
 }
 
-void __init zoom_peripherals_init(void)
+static void __init zoom_wilink_init(void)
 {
int ret;
 
omap_zoom_wlan_data.irq = gpio_to_irq(OMAP_ZOOM_WLAN_IRQ_GPIO);
-   ret = wl12xx_set_platform_data(omap_zoom_wlan_data);
 
-   if (ret)
+   ret = gpio_request_one(OMAP_ZOOM_WLAN_IRQ_GPIO, GPIOF_IN,
+  OMAP_ZOOM_WLAN_IRQ_GPIO);
+   if (ret) {
+   pr_err(error requesting wl12xx gpio: %d\n, ret);
+   goto out;
+   }
+
+   ret = irq_set_irq_type(gpio_to_irq(OMAP_ZOOM_WLAN_IRQ_GPIO),
+  IRQ_TYPE_LEVEL_HIGH);
+   if (ret) {
+   pr_err(error setting wl12xx irq type: %d\n, ret);
+   goto free;
+   }
+
+   ret = wl12xx_set_platform_data(omap_zoom_wlan_data);
+   if (ret) {
pr_err(error setting wl12xx data: %d\n, ret);
+   goto free;
+   }
+out:
+   return;
+free:
+   gpio_free(OMAP_ZOOM_WLAN_IRQ_GPIO);
+}
 
+void __init zoom_peripherals_init(void)
+{
+   zoom_wilink_init();
omap_hsmmc_init(mmc

[PATCH v4 4/8] wl12xx: use frequency instead of enumerations for pdata clocks

[Luciano Coelho]

Instead of defining an enumeration with the FW specific values for the
different clock rates, use the actual frequency instead.  Also add a
boolean to specify whether the clock is XTAL or not.

Change all board files to reflect this.

Additionally, this reverts commit 26f45c (ARM: OMAP2+: Legacy support
for wl12xx when booted with devicetree), since this is not be needed
anymore, now that DT support for WiLink is implemented.

Cc: Tony Lindgren t...@atomide.com
Cc: Sekhar Nori nsek...@ti.com
Signed-off-by: Luciano Coelho coe...@ti.com
Reviewed-by: Felipe Balbi ba...@ti.com
---
 arch/arm/mach-davinci/board-da850-evm.c  |  3 +-
 arch/arm/mach-omap2/board-omap3evm.c |  3 +-
 arch/arm/mach-omap2/board-zoom-peripherals.c |  3 +-
 arch/arm/mach-omap2/devices.c| 39 ---
 drivers/net/wireless/ti/wl12xx/main.c| 58 +++-
 drivers/net/wireless/ti/wl12xx/wl12xx.h  | 28 ++
 include/linux/wl12xx.h   | 27 ++---
 7 files changed, 93 insertions(+), 68 deletions(-)

diff --git a/arch/arm/mach-davinci/board-da850-evm.c 
b/arch/arm/mach-davinci/board-da850-evm.c
index 03de2e9..6b2768f 100644
--- a/arch/arm/mach-davinci/board-da850-evm.c
+++ b/arch/arm/mach-davinci/board-da850-evm.c
@@ -1376,7 +1376,8 @@ static const short da850_wl12xx_pins[] __initconst = {
 
 static struct wl12xx_platform_data da850_wl12xx_wlan_data __initdata = {
.irq= -1,
-   .board_ref_clock= WL12XX_REFCLOCK_38,
+   .ref_clock_freq = 3840,
+   .ref_clock_xtal = false,
 };
 
 static __init int da850_wl12xx_init(void)
diff --git a/arch/arm/mach-omap2/board-omap3evm.c 
b/arch/arm/mach-omap2/board-omap3evm.c
index 9c7dfc5..4ccfcc0 100644
--- a/arch/arm/mach-omap2/board-omap3evm.c
+++ b/arch/arm/mach-omap2/board-omap3evm.c
@@ -460,7 +460,8 @@ static struct platform_device omap3evm_wlan_regulator = {
 };
 
 struct wl12xx_platform_data omap3evm_wlan_data __initdata = {
-   .board_ref_clock = WL12XX_REFCLOCK_38, /* 38.4 MHz */
+   .ref_clock_freq = 3840,
+   .ref_clock_xtal = false,
 };
 #endif
 
diff --git a/arch/arm/mach-omap2/board-zoom-peripherals.c 
b/arch/arm/mach-omap2/board-zoom-peripherals.c
index 4f84cf9..83a9a36 100644
--- a/arch/arm/mach-omap2/board-zoom-peripherals.c
+++ b/arch/arm/mach-omap2/board-zoom-peripherals.c
@@ -244,7 +244,8 @@ static struct platform_device *zoom_devices[] __initdata = {
 };
 
 static struct wl12xx_platform_data omap_zoom_wlan_data __initdata = {
-   .board_ref_clock = WL12XX_REFCLOCK_26, /* 26 MHz */
+   .ref_clock_freq = 2600,
+   .ref_clock_xtal = false,
 };
 
 static struct omap2_hsmmc_info mmc[] = {
diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
index 3c1279f..10e6126 100644
--- a/arch/arm/mach-omap2/devices.c
+++ b/arch/arm/mach-omap2/devices.c
@@ -8,7 +8,6 @@
  * the Free Software Foundation; either version 2 of the License, or
  * (at your option) any later version.
  */
-#include linux/gpio.h
 #include linux/kernel.h
 #include linux/init.h
 #include linux/platform_device.h
@@ -19,7 +18,6 @@
 #include linux/of.h
 #include linux/pinctrl/machine.h
 #include linux/platform_data/omap4-keypad.h
-#include linux/wl12xx.h
 #include linux/platform_data/mailbox-omap.h
 
 #include asm/mach-types.h
@@ -513,40 +511,6 @@ static void omap_init_vout(void)
 static inline void omap_init_vout(void) {}
 #endif
 
-#if IS_ENABLED(CONFIG_WL12XX)
-
-static struct wl12xx_platform_data wl12xx __initdata;
-
-void __init omap_init_wl12xx_of(void)
-{
-   int ret;
-
-   if (!of_have_populated_dt())
-   return;
-
-   if (of_machine_is_compatible(ti,omap4-sdp)) {
-   wl12xx.board_ref_clock = WL12XX_REFCLOCK_26;
-   wl12xx.board_tcxo_clock = WL12XX_TCXOCLOCK_26;
-   wl12xx.irq = gpio_to_irq(53);
-   } else if (of_machine_is_compatible(ti,omap4-panda)) {
-   wl12xx.board_ref_clock = WL12XX_REFCLOCK_38;
-   wl12xx.irq = gpio_to_irq(53);
-   } else {
-   return;
-   }
-
-   ret = wl12xx_set_platform_data(wl12xx);
-   if (ret) {
-   pr_err(error setting wl12xx data: %d\n, ret);
-   return;
-   }
-}
-#else
-static inline void omap_init_wl12xx_of(void)
-{
-}
-#endif
-
 /*-*/
 
 static int __init omap2_init_devices(void)
@@ -570,9 +534,6 @@ static int __init omap2_init_devices(void)
omap_init_mcspi();
omap_init_sham();
omap_init_aes();
-   } else {
-   /* These can be removed when bindings are done */
-   omap_init_wl12xx_of();
}
omap_init_sti();
omap_init_rng();
diff --git a/drivers/net/wireless/ti/wl12xx/main.c 
b/drivers/net/wireless/ti/wl12xx/main.c
index 1c627da..a6c2a14 100644
--- a/drivers/net

[PATCH v4 1/8] wl1251: split wl251 platform data to a separate structure

[Luciano Coelho]

Move the wl1251 part of the wl12xx platform data structure into a new
structure specifically for wl1251.  Change the platform data built-in
block and board files accordingly.

Cc: Tony Lindgren t...@atomide.com
Signed-off-by: Luciano Coelho coe...@ti.com
Acked-by: Tony Lindgren t...@atomide.com
Reviewed-by: Felipe Balbi ba...@ti.com
---
 arch/arm/mach-omap2/board-omap3pandora.c   |  4 +--
 arch/arm/mach-omap2/board-rx51-peripherals.c   |  2 +-
 drivers/net/wireless/ti/wilink_platform_data.c | 37 +-
 drivers/net/wireless/ti/wl1251/sdio.c  | 12 -
 drivers/net/wireless/ti/wl1251/spi.c   |  2 +-
 include/linux/wl12xx.h | 22 ++-
 6 files changed, 62 insertions(+), 17 deletions(-)

diff --git a/arch/arm/mach-omap2/board-omap3pandora.c 
b/arch/arm/mach-omap2/board-omap3pandora.c
index b1547a0..84d56e8 100644
--- a/arch/arm/mach-omap2/board-omap3pandora.c
+++ b/arch/arm/mach-omap2/board-omap3pandora.c
@@ -541,7 +541,7 @@ static struct spi_board_info omap3pandora_spi_board_info[] 
__initdata = {
 
 static void __init pandora_wl1251_init(void)
 {
-   struct wl12xx_platform_data pandora_wl1251_pdata;
+   struct wl1251_platform_data pandora_wl1251_pdata;
int ret;
 
memset(pandora_wl1251_pdata, 0, sizeof(pandora_wl1251_pdata));
@@ -555,7 +555,7 @@ static void __init pandora_wl1251_init(void)
goto fail_irq;
 
pandora_wl1251_pdata.use_eeprom = true;
-   ret = wl12xx_set_platform_data(pandora_wl1251_pdata);
+   ret = wl1251_set_platform_data(pandora_wl1251_pdata);
if (ret  0)
goto fail_irq;
 
diff --git a/arch/arm/mach-omap2/board-rx51-peripherals.c 
b/arch/arm/mach-omap2/board-rx51-peripherals.c
index 9c2dd10..01e5711 100644
--- a/arch/arm/mach-omap2/board-rx51-peripherals.c
+++ b/arch/arm/mach-omap2/board-rx51-peripherals.c
@@ -80,7 +80,7 @@ enum {
RX51_SPI_MIPID, /* LCD panel */
 };
 
-static struct wl12xx_platform_data wl1251_pdata;
+static struct wl1251_platform_data wl1251_pdata;
 static struct tsc2005_platform_data tsc2005_pdata;
 
 #if defined(CONFIG_SENSORS_LIS3_I2C) || defined(CONFIG_SENSORS_LIS3_I2C_MODULE)
diff --git a/drivers/net/wireless/ti/wilink_platform_data.c 
b/drivers/net/wireless/ti/wilink_platform_data.c
index 998e958..a92bd3e 100644
--- a/drivers/net/wireless/ti/wilink_platform_data.c
+++ b/drivers/net/wireless/ti/wilink_platform_data.c
@@ -23,17 +23,17 @@
 #include linux/err.h
 #include linux/wl12xx.h
 
-static struct wl12xx_platform_data *platform_data;
+static struct wl12xx_platform_data *wl12xx_platform_data;
 
 int __init wl12xx_set_platform_data(const struct wl12xx_platform_data *data)
 {
-   if (platform_data)
+   if (wl12xx_platform_data)
return -EBUSY;
if (!data)
return -EINVAL;
 
-   platform_data = kmemdup(data, sizeof(*data), GFP_KERNEL);
-   if (!platform_data)
+   wl12xx_platform_data = kmemdup(data, sizeof(*data), GFP_KERNEL);
+   if (!wl12xx_platform_data)
return -ENOMEM;
 
return 0;
@@ -41,9 +41,34 @@ int __init wl12xx_set_platform_data(const struct 
wl12xx_platform_data *data)
 
 struct wl12xx_platform_data *wl12xx_get_platform_data(void)
 {
-   if (!platform_data)
+   if (!wl12xx_platform_data)
return ERR_PTR(-ENODEV);
 
-   return platform_data;
+   return wl12xx_platform_data;
 }
 EXPORT_SYMBOL(wl12xx_get_platform_data);
+
+static struct wl1251_platform_data *wl1251_platform_data;
+
+int __init wl1251_set_platform_data(const struct wl1251_platform_data *data)
+{
+   if (wl1251_platform_data)
+   return -EBUSY;
+   if (!data)
+   return -EINVAL;
+
+   wl1251_platform_data = kmemdup(data, sizeof(*data), GFP_KERNEL);
+   if (!wl1251_platform_data)
+   return -ENOMEM;
+
+   return 0;
+}
+
+struct wl1251_platform_data *wl1251_get_platform_data(void)
+{
+   if (!wl1251_platform_data)
+   return ERR_PTR(-ENODEV);
+
+   return wl1251_platform_data;
+}
+EXPORT_SYMBOL(wl1251_get_platform_data);
diff --git a/drivers/net/wireless/ti/wl1251/sdio.c 
b/drivers/net/wireless/ti/wl1251/sdio.c
index e2b3d9c..b75a37a 100644
--- a/drivers/net/wireless/ti/wl1251/sdio.c
+++ b/drivers/net/wireless/ti/wl1251/sdio.c
@@ -227,7 +227,7 @@ static int wl1251_sdio_probe(struct sdio_func *func,
struct wl1251 *wl;
struct ieee80211_hw *hw;
struct wl1251_sdio *wl_sdio;
-   const struct wl12xx_platform_data *wl12xx_board_data;
+   const struct wl1251_platform_data *wl1251_board_data;
 
hw = wl1251_alloc_hw();
if (IS_ERR(hw))
@@ -254,11 +254,11 @@ static int wl1251_sdio_probe(struct sdio_func *func,
wl-if_priv = wl_sdio;
wl-if_ops = wl1251_sdio_ops;
 
-   wl12xx_board_data = wl12xx_get_platform_data();
-   if (!IS_ERR(wl12xx_board_data)) {
-   wl-set_power

[PATCH v4 3/8] wlcore: remove pwr_in_suspend from platform data

[Luciano Coelho]

The pwr_in_suspend flag depends on the MMC settings which can be
retrieved from the SDIO subsystem, so it doesn't need to be part of
the platform data structure.  Move it to the platform device data that
is passed from SDIO to wlcore.

Signed-off-by: Luciano Coelho coe...@ti.com
Reviewed-by: Felipe Balbi ba...@ti.com
---
 drivers/net/wireless/ti/wlcore/main.c | 3 +--
 drivers/net/wireless/ti/wlcore/sdio.c | 2 +-
 drivers/net/wireless/ti/wlcore/wlcore_i.h | 1 +
 include/linux/wl12xx.h| 1 -
 4 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/ti/wlcore/main.c 
b/drivers/net/wireless/ti/wlcore/main.c
index 11dab9a..e771de0 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -5900,7 +5900,6 @@ static void wlcore_nvs_cb(const struct firmware *fw, void 
*context)
struct wl1271 *wl = context;
struct platform_device *pdev = wl-pdev;
struct wlcore_platdev_data *pdev_data = pdev-dev.platform_data;
-   struct wl12xx_platform_data *pdata = pdev_data-pdata;
 
int ret;
 
@@ -5947,7 +5946,7 @@ static void wlcore_nvs_cb(const struct firmware *fw, void 
*context)
if (!ret) {
wl-irq_wake_enabled = true;
device_init_wakeup(wl-dev, 1);
-   if (pdata-pwr_in_suspend)
+   if (pdev_data-pwr_in_suspend)
wl-hw-wiphy-wowlan = wlcore_wowlan_support;
}
 #endif
diff --git a/drivers/net/wireless/ti/wlcore/sdio.c 
b/drivers/net/wireless/ti/wlcore/sdio.c
index 29ef249..4c7e8ac 100644
--- a/drivers/net/wireless/ti/wlcore/sdio.c
+++ b/drivers/net/wireless/ti/wlcore/sdio.c
@@ -260,7 +260,7 @@ static int wl1271_probe(struct sdio_func *func,
dev_dbg(glue-dev, sdio PM caps = 0x%x\n, mmcflags);
 
if (mmcflags  MMC_PM_KEEP_POWER)
-   pdev_data-pdata-pwr_in_suspend = true;
+   pdev_data-pwr_in_suspend = true;
 
sdio_set_drvdata(func, glue);
 
diff --git a/drivers/net/wireless/ti/wlcore/wlcore_i.h 
b/drivers/net/wireless/ti/wlcore/wlcore_i.h
index e5e1464..f2c4227 100644
--- a/drivers/net/wireless/ti/wlcore/wlcore_i.h
+++ b/drivers/net/wireless/ti/wlcore/wlcore_i.h
@@ -209,6 +209,7 @@ struct wl1271_if_operations {
 struct wlcore_platdev_data {
struct wl12xx_platform_data *pdata;
struct wl1271_if_operations *if_ops;
+   bool pwr_in_suspend;
 };
 
 #define MAX_NUM_KEYS 14
diff --git a/include/linux/wl12xx.h b/include/linux/wl12xx.h
index 1bfcd19..ab90b1c 100644
--- a/include/linux/wl12xx.h
+++ b/include/linux/wl12xx.h
@@ -59,7 +59,6 @@ struct wl12xx_platform_data {
int irq;
int board_ref_clock;
int board_tcxo_clock;
-   bool pwr_in_suspend;
 };
 
 #ifdef CONFIG_WILINK_PLATFORM_DATA
-- 
1.8.3.2

--
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/

[PATCH v4 5/8] wlcore: add initial device tree support to the sdio module

[Luciano Coelho]

If platform data is not available, try to get the required information
from the device tree.  Register an OF match table and parse the
appropriate device tree nodes.

Parse interrupt property only, for now.

Signed-off-by: Luciano Coelho coe...@ti.com
Reviewed-by: Felipe Balbi ba...@ti.com
---
 drivers/net/wireless/ti/wlcore/sdio.c | 69 ---
 1 file changed, 63 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/ti/wlcore/sdio.c 
b/drivers/net/wireless/ti/wlcore/sdio.c
index 4c7e8ac..9370d7e 100644
--- a/drivers/net/wireless/ti/wlcore/sdio.c
+++ b/drivers/net/wireless/ti/wlcore/sdio.c
@@ -30,7 +30,7 @@
 #include linux/mmc/sdio_ids.h
 #include linux/mmc/card.h
 #include linux/mmc/host.h
-#include linux/gpio.h
+#include linux/of_irq.h
 #include linux/wl12xx.h
 #include linux/pm_runtime.h
 #include linux/printk.h
@@ -214,6 +214,43 @@ static struct wl1271_if_operations sdio_ops = {
.set_block_size = wl1271_sdio_set_block_size,
 };
 
+static struct wl12xx_platform_data *wlcore_get_pdata_from_of(struct device 
*dev)
+{
+   struct wl12xx_platform_data *pdata;
+   struct device_node *np = dev-of_node;
+
+   if (!np) {
+   np = of_find_matching_node(NULL, dev-driver-of_match_table);
+   if (!np) {
+   dev_notice(dev, device tree node not available\n);
+   pdata = ERR_PTR(-ENODEV);
+   goto out;
+   }
+   }
+
+   pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);
+   if (!pdata) {
+   dev_err(dev, can't allocate platform data\n);
+   pdata = ERR_PTR(-ENODEV);
+   goto out;
+   }
+
+   pdata-irq = irq_of_parse_and_map(np, 0);
+   if (pdata-irq  0) {
+   dev_err(dev, can't get interrupt gpio from the device tree\n);
+   goto out_free;
+   }
+
+   goto out;
+
+out_free:
+   kfree(pdata);
+   pdata = ERR_PTR(-ENODEV);
+
+out:
+   return pdata;
+}
+
 static int wl1271_probe(struct sdio_func *func,
  const struct sdio_device_id *id)
 {
@@ -248,11 +285,22 @@ static int wl1271_probe(struct sdio_func *func,
/* Use block mode for transferring over one block size of data */
func-card-quirks |= MMC_QUIRK_BLKSZ_FOR_BYTE_MODE;
 
+   /* The pdata allocated here is freed when the device is freed,
+* so we don't need an additional out label to free it in case
+* of error further on.
+*/
+
+   /* Try to get legacy platform data from the board file */
pdev_data-pdata = wl12xx_get_platform_data();
if (IS_ERR(pdev_data-pdata)) {
-   ret = PTR_ERR(pdev_data-pdata);
-   dev_err(glue-dev, missing wlan platform data: %d\n, ret);
-   goto out_free_glue;
+   dev_info(func-dev,
+legacy platform data not found, trying device 
tree\n);
+
+   pdev_data-pdata = wlcore_get_pdata_from_of(func-dev);
+   if (IS_ERR(pdev_data-pdata)) {
+   dev_err(func-dev, can't get platform data\n);
+   goto out_free_glue;
+   }
}
 
/* if sdio can keep power while host is suspended, enable wow */
@@ -386,16 +434,25 @@ static const struct dev_pm_ops wl1271_sdio_pm_ops = {
 };
 #endif
 
+static const struct of_device_id wlcore_sdio_of_match_table[] = {
+   { .compatible = ti,wilink6 },
+   { .compatible = ti,wilink7 },
+   { .compatible = ti,wilink8 },
+   { }
+};
+MODULE_DEVICE_TABLE(of, wlcore_sdio_of_match_table);
+
 static struct sdio_driver wl1271_sdio_driver = {
.name   = wl1271_sdio,
.id_table   = wl1271_devices,
.probe  = wl1271_probe,
.remove = wl1271_remove,
-#ifdef CONFIG_PM
.drv = {
+#ifdef CONFIG_PM
.pm = wl1271_sdio_pm_ops,
-   },
 #endif
+   .of_match_table = of_match_ptr(wlcore_sdio_of_match_table),
+   },
 };
 
 static int __init wl1271_init(void)
-- 
1.8.3.2

--
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 v2] Documentation: dt: bindings: TI WiLink modules

[Luciano Coelho]

(using the new devicetree mailing list address)

On Tue, 2013-07-30 at 20:24 +0200, Laurent Pinchart wrote:
 Hi Luciano,
 
 Thank you for the patch.
 
 On Monday 29 July 2013 17:55:28 Luciano Coelho wrote:
  Add device tree bindings documentation for the TI WiLink modules.
  Currently only the WLAN part of the WiLink6, WiLink7 and WiLink8
  modules is supported.
  
  Signed-off-by: Luciano Coelho coe...@ti.com
  ---
  
  Changes in v2:
  
  Use generic clock definitions to get the clock data instead of passing
  the frequencies directly.  Also added definition for internal
  ti,wilink-clock.
  
  Please review.
 
 The proposal looks good to me, I just have one small comment.

Cool! Thanks for the review.

[...]
  +Example:
  +
  +
  +Example definition that can be used in OMAP4 Panda:
  +
  +wlan {
  +   compatible = ti,wilink6;
  +   interrupt-parent = gpio2;
  +   interrupts = 21 0x4;  /* gpio line 53, high level triggered */
 
 Could you please use the IRQ_TYPE_LEVEL_HIGH macro (defined in dt-
 bindings/interrupt-controller/irq.h) instead of the hardcode 0x4 constant ?

Ah, right, I saw this new header file recently but forgot to update my
example.  I'll update the OMAP4 Panda and OMAP4 SDP dts files I just
sent out as well.

--
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/

[PATCH v3] Documentation: dt: bindings: TI WiLink modules

[Luciano Coelho]

Add device tree bindings documentation for the TI WiLink modules.
Currently only the WLAN part of the WiLink6, WiLink7 and WiLink8
modules is supported.

Signed-off-by: Luciano Coelho coe...@ti.com
---

In v3, use IRQ_TYPE_LEVEL_HIGH in the example, as suggested by Laurent.

 .../devicetree/bindings/net/wireless/ti-wilink.txt | 68 ++
 1 file changed, 68 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/wireless/ti-wilink.txt

diff --git a/Documentation/devicetree/bindings/net/wireless/ti-wilink.txt 
b/Documentation/devicetree/bindings/net/wireless/ti-wilink.txt
new file mode 100644
index 000..aafebb1
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/wireless/ti-wilink.txt
@@ -0,0 +1,68 @@
+TI WiLink Wireless Modules Device Tree Bindings
+===
+
+The WiLink modules provide wireless connectivity, such as WLAN,
+Bluetooth, FM and NFC.
+
+There are several different modules available, which can be grouped by
+their generation: WiLink6, WiLink7 and WiLink8.  WiLink4 is not
+currently supported with device tree.
+
+Currently, only the WLAN portion of the modules is supported with
+device tree.
+
+Required properties:
+
+
+- compatible: should be ti,wilink6, ti,wilink7 or ti,wilink8
+- interrupt-parent: the interrupt controller
+- interrupts: out-of-band WLAN interrupt
+   See the interrupt controller's bindings documentation for
+   detailed definition.
+
+Optional properties:
+
+
+- clocks: list of clocks needed by the chip as follows:
+
+  refclock: the internal WLAN reference clock frequency (required for
+   WiLink6 and WiLink7; not used for WiLink8).
+
+  tcxoclock: the internal WLAN TCXO clock frequency (required for
+   WiLink7 not used for WiLink6 and WiLink8).
+
+  The clocks must be defined and named accordingly.  For example:
+
+  clocks = refclock
+  clock-names = refclock;
+
+  refclock: refclock {
+compatible = ti,wilink-clock;
+#clock-cells = 0;
+clock-frequency = 3840;
+   };
+
+  Some modules that contain the WiLink chip provide clocks in the
+  module itself.  In this case, we define a ti,wilink-clock as shown
+  above.  But any other clock could in theory be used, so the proper
+  clock definition should be used.
+
+
+Example:
+
+
+Example definition that can be used in OMAP4 Panda:
+
+wlan {
+   compatible = ti,wilink6;
+   interrupt-parent = gpio2;
+   interrupts = 21 IRQ_TYPE_LEVEL_HIGH;  /* gpio line 53 */
+   clocks = refclock;
+   clock-names = refclock;
+
+   refclock: refclock {
+   compatible = ti,wilink-clock;
+   #clock-cells = 0;
+   clock-frequency = 3840;
+   };
+};
-- 
1.8.3.2

--
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 6/8] wlcore: sdio: add wilink clock providers

[Luciano Coelho]

On Tue, 2013-07-30 at 15:35 -0700, Mike Turquette wrote:
 Quoting Luciano Coelho (2013-07-30 06:04:34)
  +static const struct of_device_id wlcore_sdio_of_clk_match_table[] = {
  +   { .compatible = ti,wilink-clock },
  +};
  +
   static struct wl12xx_platform_data *wlcore_get_pdata_from_of(struct device 
  *dev)
   {
  struct wl12xx_platform_data *pdata;
  struct device_node *np = dev-of_node;
  +   struct device_node *clock_node;
   
  if (!np) {
  np = of_find_matching_node(NULL, 
  dev-driver-of_match_table);
  @@ -241,6 +247,9 @@ static struct wl12xx_platform_data 
  *wlcore_get_pdata_from_of(struct device *dev)
  goto out_free;
  }
   
  +   for_each_matching_node(clock_node, wlcore_sdio_of_clk_match_table)
  +   of_fixed_clk_setup(clock_node);
 
 Hi Luciano,

Hi Mike,


 Any reason for establishing your own compatible string if you just plan
 to use the fixed rate clock? You could just use fixed-clock compatible
 in your DTS.

The reason is that I can't call of_clk_init(), because this function is
not exported and my module can't use it.  I would have to link with the
clk code to be able to call it.

Also, I reckoned that, since these clock cannot be used by anyone else
than the WiLink module itself, it would make sense to have a different
compatible string.


 I will be posting patches this week which makes the fixed-rate clock a
 proper driver and matches that compatible string to instantiate those
 clocks. That means that your driver could probably remove the clock
 setup code completely.

Okay, if this is done, then I could probably use fixed-clock directly,
since the driver itself will take care of going through the DT and
initializing all the fixed-clocks.

--
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/

[PATCH v2] Documentation: dt: bindings: TI WiLink modules

[Luciano Coelho]

Add device tree bindings documentation for the TI WiLink modules.
Currently only the WLAN part of the WiLink6, WiLink7 and WiLink8
modules is supported.

Signed-off-by: Luciano Coelho 
---

Changes in v2:

Use generic clock definitions to get the clock data instead of passing
the frequencies directly.  Also added definition for "internal"
ti,wilink-clock.

Please review.

Luca.

 .../devicetree/bindings/net/wireless/ti-wilink.txt | 68 ++
 1 file changed, 68 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/wireless/ti-wilink.txt

diff --git a/Documentation/devicetree/bindings/net/wireless/ti-wilink.txt 
b/Documentation/devicetree/bindings/net/wireless/ti-wilink.txt
new file mode 100644
index 000..5fd27dc
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/wireless/ti-wilink.txt
@@ -0,0 +1,68 @@
+TI WiLink Wireless Modules Device Tree Bindings
+===
+
+The WiLink modules provide wireless connectivity, such as WLAN,
+Bluetooth, FM and NFC.
+
+There are several different modules available, which can be grouped by
+their generation: WiLink6, WiLink7 and WiLink8.  WiLink4 is not
+currently supported with device tree.
+
+Currently, only the WLAN portion of the modules is supported with
+device tree.
+
+Required properties:
+
+
+- compatible: should be "ti,wilink6", "ti,wilink7" or "ti,wilink8"
+- interrupt-parent: the interrupt controller
+- interrupts: out-of-band WLAN interrupt
+   See the interrupt controller's bindings documentation for
+   detailed definition.
+
+Optional properties:
+
+
+- clocks: list of clocks needed by the chip as follows:
+
+  refclock: the internal WLAN reference clock frequency (required for
+   WiLink6 and WiLink7; not used for WiLink8).
+
+  tcxoclock: the internal WLAN TCXO clock frequency (required for
+   WiLink7 not used for WiLink6 and WiLink8).
+
+  The clocks must be defined and named accordingly.  For example:
+
+  clocks = <>
+  clock-names = "refclock";
+
+  refclock: refclock {
+compatible = "ti,wilink-clock";
+#clock-cells = <0>;
+clock-frequency = <3840>;
+   };
+
+  Some modules that contain the WiLink chip provide clocks in the
+  module itself.  In this case, we define a "ti,wilink-clock" as shown
+  above.  But any other clock could in theory be used, so the proper
+  clock definition should be used.
+
+
+Example:
+
+
+Example definition that can be used in OMAP4 Panda:
+
+wlan {
+   compatible = "ti,wilink6";
+   interrupt-parent = <>;
+   interrupts = <21 0x4>;  /* gpio line 53, high level triggered */
+   clocks = <>;
+   clock-names = "refclock";
+
+   refclock: refclock {
+   compatible = "ti,wilink-clock";
+   #clock-cells = <0>;
+   clock-frequency = <3840>;
+   };
+};
-- 
1.8.3.2

--
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

[Luciano Coelho]

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.

--
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: [RFC] clk: add flags to distinguish xtal clocks

[Luciano Coelho]

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.

--
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/

[PATCH v2] Documentation: dt: bindings: TI WiLink modules

[Luciano Coelho]

Add device tree bindings documentation for the TI WiLink modules.
Currently only the WLAN part of the WiLink6, WiLink7 and WiLink8
modules is supported.

Signed-off-by: Luciano Coelho coe...@ti.com
---

Changes in v2:

Use generic clock definitions to get the clock data instead of passing
the frequencies directly.  Also added definition for internal
ti,wilink-clock.

Please review.

Luca.

 .../devicetree/bindings/net/wireless/ti-wilink.txt | 68 ++
 1 file changed, 68 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/wireless/ti-wilink.txt

diff --git a/Documentation/devicetree/bindings/net/wireless/ti-wilink.txt 
b/Documentation/devicetree/bindings/net/wireless/ti-wilink.txt
new file mode 100644
index 000..5fd27dc
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/wireless/ti-wilink.txt
@@ -0,0 +1,68 @@
+TI WiLink Wireless Modules Device Tree Bindings
+===
+
+The WiLink modules provide wireless connectivity, such as WLAN,
+Bluetooth, FM and NFC.
+
+There are several different modules available, which can be grouped by
+their generation: WiLink6, WiLink7 and WiLink8.  WiLink4 is not
+currently supported with device tree.
+
+Currently, only the WLAN portion of the modules is supported with
+device tree.
+
+Required properties:
+
+
+- compatible: should be ti,wilink6, ti,wilink7 or ti,wilink8
+- interrupt-parent: the interrupt controller
+- interrupts: out-of-band WLAN interrupt
+   See the interrupt controller's bindings documentation for
+   detailed definition.
+
+Optional properties:
+
+
+- clocks: list of clocks needed by the chip as follows:
+
+  refclock: the internal WLAN reference clock frequency (required for
+   WiLink6 and WiLink7; not used for WiLink8).
+
+  tcxoclock: the internal WLAN TCXO clock frequency (required for
+   WiLink7 not used for WiLink6 and WiLink8).
+
+  The clocks must be defined and named accordingly.  For example:
+
+  clocks = refclock
+  clock-names = refclock;
+
+  refclock: refclock {
+compatible = ti,wilink-clock;
+#clock-cells = 0;
+clock-frequency = 3840;
+   };
+
+  Some modules that contain the WiLink chip provide clocks in the
+  module itself.  In this case, we define a ti,wilink-clock as shown
+  above.  But any other clock could in theory be used, so the proper
+  clock definition should be used.
+
+
+Example:
+
+
+Example definition that can be used in OMAP4 Panda:
+
+wlan {
+   compatible = ti,wilink6;
+   interrupt-parent = gpio2;
+   interrupts = 21 0x4;  /* gpio line 53, high level triggered */
+   clocks = refclock;
+   clock-names = refclock;
+
+   refclock: refclock {
+   compatible = ti,wilink-clock;
+   #clock-cells = 0;
+   clock-frequency = 3840;
+   };
+};
-- 
1.8.3.2

--
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] Documentation: dt: bindings: TI WiLink modules

[Luciano Coelho]

On Thu, 2013-07-18 at 01:58 +0200, Laurent Pinchart wrote:
> Hi Luciano,

Hi Laurent,

> On Monday 01 July 2013 15:39:30 Luciano Coelho wrote:
> > The only thing I can come up with is to make a small clock driver (maybe
> > even inside the WiLink module itself) that registers a new type of
> > clock, "ti,wilink-clock" or something.  But this would really be
> > overkill, wouldn't it?
> > 
> > Any other ideas?
> 
> One possibility would be to just call clk_get_rate() on the clock from the 
> WiLink driver, which would return the fixed frequency specified in DT, and 
> configure the WiLink hardware accordingly. This might be a bit hackish though.

The problem is not get the rate itself, the problem is knowing whether
the clock is XTAL or not.  The WiLink chip uses the clock in a slightly
different way if it is XTAL, due to some stabilization time constraints.

--
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: Regression 3.11-rc1: omap4panda: no usb and consequently no ethernet

[Luciano Coelho]

On Thu, 2013-07-18 at 01:59 -0700, Tony Lindgren wrote:
> * Arend van Spriel  [130718 01:47]:
> 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#
> 
> The wl12xx .dts changes for pandaboard are still missing too.

Just for the record, I already have the DTS patches for wl12xx on Panda
(and a few other boards).

I held them back because I wanted the bindings to be properly defined
first.  I'll continue this work pretty soon, when I come back from my
summer vacations (in a week).

--
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: Regression 3.11-rc1: omap4panda: no usb and consequently no ethernet

[Luciano Coelho]

On Thu, 2013-07-18 at 01:59 -0700, Tony Lindgren wrote:
 * Arend van Spriel ar...@broadcom.com [130718 01:47]:
 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#
 
 The wl12xx .dts changes for pandaboard are still missing too.

Just for the record, I already have the DTS patches for wl12xx on Panda
(and a few other boards).

I held them back because I wanted the bindings to be properly defined
first.  I'll continue this work pretty soon, when I come back from my
summer vacations (in a week).

--
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] Documentation: dt: bindings: TI WiLink modules

[Luciano Coelho]

On Thu, 2013-07-18 at 01:58 +0200, Laurent Pinchart wrote:
 Hi Luciano,

Hi Laurent,

 On Monday 01 July 2013 15:39:30 Luciano Coelho wrote:
  The only thing I can come up with is to make a small clock driver (maybe
  even inside the WiLink module itself) that registers a new type of
  clock, ti,wilink-clock or something.  But this would really be
  overkill, wouldn't it?
  
  Any other ideas?
 
 One possibility would be to just call clk_get_rate() on the clock from the 
 WiLink driver, which would return the fixed frequency specified in DT, and 
 configure the WiLink hardware accordingly. This might be a bit hackish though.

The problem is not get the rate itself, the problem is knowing whether
the clock is XTAL or not.  The WiLink chip uses the clock in a slightly
different way if it is XTAL, due to some stabilization time constraints.

--
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

[Luciano Coelho]

On Fri, 2013-07-05 at 14:12 +0100, James Hogan wrote:
> On 4 July 2013 22:05, Luciano Coelho  wrote:
> > Add a flag that indicate whether the clock is a crystal or not.  Since
> > no clocks set this flag right now, include an additional flag that
> > indicates whether the type is set or not.  If the CLK_IS_TYPE_DEFINED
> > flag is not set, the value of the CLK_IS_TYPE_XTAL flag is undefined.
> > This ensures backwards compatibility.
> >
> > Additionally, parse a new device tree binding in clk-fixed-rate to set
> > this flag.
> >
> > Signed-off-by: Luciano Coelho 
> > ---
> >
> > I'm not  familiar with the common clock framework and I'm not
> > entirely sure the flags can be used in such a way, but to me it looks
> > reasonable, since some clock consumers may need to know what type of
> > clock is being provided.
> >
> > Specifically, the wl12xx firmware needs to know if the clock is XTAL
> > or not to handle the stabilization and boosts properly.
> >
> > My main idea is that I need to pass this information in the device
> > tree definition of the clocks, so that the driver can pass this
> > information on to the firmware.
> >
> > Please let me know if this looks ok or not.  If not, please let me
> > know if you have any other ideas on how to solve my problem (of
> > knowing whether the clock attached to the WiLink chip is XTAL or not).
> 
> The TZ1090 SoC has something that sounds possibly similar, where some
> of the XTAL pads have a bypass bit, which according to the hardware
> engineer I asked should be enabled when you want to use the
> corresponding XTAL pads as a clock input pad rather than an
> oscillator.

Cool, good to know that I'm not alone here. ;)


>  I was considering extending clk-fixed-rate (via a wrapper
> driver) to parse a custom DT property and a register address / bit
> number and set the bypass bit as appropriate itself.

I thought about this too.  I actually have a "driver" for my clocks,
because normally they're not part of the main board, but part of the
module that contains the WiLink chip.  In those cases, I don't want my
clocks to be used as a generic clk-fixed-rate by the clock framework.
But the only difference in my "wrapper" is that it matches
"ti,wilink-clock" instead of "fixed-clock".


> So I was wondering, is there a particular reason you don't have a DT
> property on the node for the device that needs to know what type of
> clock it is, rather than the clock node itself? That way you're not
> depending directly on the generic common clock framework to be able to
> tell you such electrical details.

I think this is a detail of the clock itself, not of the user of the
clock.  Of course, the user of the clock needs to know what to do if the
clock is XTAL.  But the information of whether it is a XTAL or not
should be in the clock data.

I tried to make a generic solution that everyone could use.  For
instance, you. :) You could use the same bit that I implemented and, in
your driver, convert that bit into the action you need to take.


--
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

[Luciano Coelho]

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.

As I said, I don't know almost anything about clocks, so all this
doesn't make much sense to me.  But maybe you can get an idea?

--
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: [RFC] clk: add flags to distinguish xtal clocks

[Luciano Coelho]

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.

As I said, I don't know almost anything about clocks, so all this
doesn't make much sense to me.  But maybe you can get an idea?

--
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/