All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kalle Valo <kvalo@qca.qualcomm.com>
To: Jouni Malinen <jouni@qca.qualcomm.com>
Cc: <linux-wireless@vger.kernel.org>
Subject: Re: [PATCH 4/4] ath6kl: Allow enabling of P2P support
Date: Wed, 7 Sep 2011 10:16:33 +0300	[thread overview]
Message-ID: <4E671A51.2050109@qca.qualcomm.com> (raw)
In-Reply-To: <20110906093509.GA14112@jouni.qca.qualcomm.com>

On 09/06/2011 12:35 PM, Jouni Malinen wrote:
> On Tue, Sep 06, 2011 at 10:51:55AM +0300, Kalle Valo wrote:
>> I was more thinking that ath6kl_p2p would not be exposed outside init.c,
>> instead you would set the appropriate conf_flag in ath6kl_core_init() or
>> similar function. But leave the module_param as it is for now, I will
>> cleanup the module parameters anyway soon.
> 
> This and the change to using ar->conf_flags instead of ar->p2p turned
> out to be more complex changes. The main problem behind this is in the
> order that the driver is allocating and initializing the data
> structures. In theory, this sounds great and should be the longer term
> direction, but with the current initialization code path, changing the
> struct ath6kl data within ath6kl_core_alloc() (including cfg80211 alloc
> and registration) and ath6kl_core_init() can conflict pretty easily. For
> example, conf_flags are initialized in ath6kl_init() which is called
> from ath6kl_core_init(), but this is done only after ath6kl_core_alloc()
> has already returned and the P2P flags are needed there before cfg80211
> registration..
> 
> I think that the initialization steps need to be reordered in a way
> that cfg80211 registration happens somewhere near the end of init()
> rather than in alloc(). Once this is done, the conf_flags can be set
> based on firmware and target information and those can then be used to
> set up the cfg80211 information before calling wiphy_register(). Until
> that gets done, it seems safest to apply this P2P enabling patch as-is
> and do the proposed clean up separately after the wiphy_register() call
> is moved.

I agree. I didn't look at code closely enough when I suggested the
changes, but it's obvious that the way you implemented is the best for
now. I have applied your original patch 4 now. Sorry for causing you
extra work.

I need to work on firmware boot changes anyway and I will also try to
cleanup cfg80211 registration as well. After that I can change p2p
implementation as we planned.

Kalle

      reply	other threads:[~2011-09-07 16:49 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-09-05 14:38 [PATCH 0/4] ath6kl: Fix AP mode PS buffering and enable AP/P2P Jouni Malinen
2011-09-05 14:38 ` [PATCH 1/4] ath6kl: Fix WMI message structure for AP_SET_PVB Jouni Malinen
2011-09-05 14:38 ` [PATCH 2/4] ath6kl: Fix AP mode connect event parsing and TIM updates Jouni Malinen
2011-09-05 14:38 ` [PATCH 3/4] ath6kl: Allow AP mode to be configured Jouni Malinen
2011-09-05 14:38 ` [PATCH 4/4] ath6kl: Allow enabling of P2P support Jouni Malinen
2011-09-06  6:58   ` Kalle Valo
2011-09-06  7:16     ` Kalle Valo
2011-09-06  7:21     ` Jouni Malinen
2011-09-06  7:51       ` Kalle Valo
2011-09-06  9:35         ` Jouni Malinen
2011-09-07  7:16           ` Kalle Valo [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4E671A51.2050109@qca.qualcomm.com \
    --to=kvalo@qca.qualcomm.com \
    --cc=jouni@qca.qualcomm.com \
    --cc=linux-wireless@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.