On Wed, Jul 23, 2014 at 3:41 AM, Luis R. Rodriguez
<mcgrof@xxxxxxxxxxxxxxxx> wrote:
> Dropping Greg as we're no longer focusing on questions on the driver
> core. My reply below.
>
> On Wed, Jul 09, 2014 at 05:46:05PM +0300, Arik Nemtsov wrote:
>> On Tue, Jul 8, 2014 at 5:57 AM, Luis R. Rodriguez
>> <mcgrof@xxxxxxxxxxxxxxxx> wrote:
>> > Let's separate cell base station hints from userspace from firmware
>> > being loaded and assuming that magical firmware has now regulatory
>> > data (which obviously I consider silly since we already provide the
>> > same functionality with CRDA and let you override the db.txt). The
>> > cellular base station hint infrastructure can certainly be expanded
>> > to support overriding the *_orig parameters. The FW API looks less
>> > attractive now as it seems more than anything the hint can actually
>> > come from userspace and the base station hint mechanism for your
>> > use case.
>> >
>> > A FW API to let FW load regulatory data dynamically is still OK but
>> > lets be clear that if you are using it for a userspace work around
>> > for cell base astation hints it seems rather a work around for
>> > existing APIs and you can likely implement what you want with less
>> > code. If we don't need this FW API I rather not have it added.
>>
>> I don't see a way around having the get_regd() API to get regulatory
>> data from FW. How else would we get regdomain settings?
>
> Well it depends on the solution of course so let's review and be
> crystal clear on what your *requirements* are and determine exactly
> what you need.
>
> So far it seems clear you have requirements for:
>
> (0) asynchronous "cellular hint" from the *driver*, with the data
> source of the regulatory domain data structure being the driver
>
> Right now we only provide asynchronous cell base station hints but from
> userespace, and only for the alpha2.
>
>> A hint can come from userspace (as a cell-base hint) or from the
>> driver.
>
> Right. Today we only allow cell base station hints from userspace
> though.
Right.
>
>> In some platforms the Wifi HW is directly connected to the SIM
>> and the FW sends up events about country change. This eventually
>> causes a driver hint.
>
> OK, so userspace can't send the cell base station hint (or whatever
> we want to call it to make this fit). In that case the firmware should
> be able to trigger an event and send a regulatory hint but with a
> regulatory domain data structure. As I see it instead of of having the
> regulatory core *ask* the driver for the regdomain the driver should be
> able to initiate the request preemptively, a new call seems best,
> something like:
>
> regulatory_hint_regd(struct wiphy,
> const struct ieee80211_regdomain *regd,
> enum nl80211_driver_reg_hint_type);
>
> The nl80211_driver_reg_hint_type can be similar to nl80211_user_reg_hint_type
> to qualify the source. The first source would be:
>
> enum nl80211_driver_reg_hint_type {
> NL80211_DRIVER_REG_HINT_CELL_BASE = 0,
> };
That could work nicely actually.
>
> The struct regulatory_request can then be expanded with the
> nl80211_driver_reg_hint_type and the handler code can do what is needed.
> You can expand reg_request_cell_base() to include this type of request
> but be sure that you review its uses, the only case I see that requires
> treament of the cell base station hint differently is in
> reg_ignore_cell_hint() where we check if the alpha2 is already set. In
> the driver cell base station hint case where the regd structure comes
> from the driver we'e need to ensure the source was the same, that is the
> same wiphy.
Well currently this code is only invoked for user hints, so not sure
we should touch it. For instance reg_request_cell_base() checks the
initiator is usermode, etc.
Also in the driver case, we probably don't want to ignore regular
updates after cell-base updates. So the logic is simpler there.
I also suggest adding another argument to regulatory_hint_regd() to
allow the driver to specify the intersection policy. It seems that
currently the code (__reg_process_hint_driver()) only allows to
intersect, which is not appropriate to us.
So the final form can look like:
regulatory_hint_regd(struct wiphy,
const struct ieee80211_regdomain *regd,
enum nl80211_driver_reg_hint_type,
enum nl80211_reg_intersect_policy policy);
enum nl80211_reg_intersect_policy {
REG_INTERSECT_POLICY_DEFAULT,
REG_INTERSECT_POLICY_OVERRIDE,
REG_INTERSECT_POLICY_INTERSECT,
};
>
> When drivers are the sources of the regd we only want to trust that
> regd struct for that wiphy, ideally however it is very useful information
> for the entire kernel what alpha2 was passed, however using this alpha2
> to for example call CRDA and use the wireless-regdb *only* for drivers
> other than the caller can get rather sloppy and complicated really
> quickly. For instance if CRDA is not installed we'd timeout on the request
> and we don't want to reset the regulatory core as its not a fatal issue if
> CRDA was not present in the case of an internal driver call.
> Driver specific helpers added here should simply be documented and
> implemented as driver specific then, they really can't *easily* contribute
> much to the regulatory core unless we complicate things quite a bit. I don't
> think its worth it. We should therefore treat these type of driver hints
> that have the driver / firmware as source for the regulatory data as no
> different than the custom reg hint calls, which are only internal to
> the driver. The driver can still take advantage of other regulatory core
> helpers like beacon hints, and other regulatory helpers if it so
> wishes. Please make these new helpers EXPORT_SYMBOL_GPL() though.
Agree we should keep this simple. Why should these be exported as GPL?
Everything else is just EXPORT_SYMBOL() in reg.c.
>
> This should simplify your implementation then. You can even piggy back on
> top of the current cell base station hint code, it can be extended to include
> support for driver cell base station hints and I think all you'd
> need is to expand support to enable drivers to specify whether or not
> they want driver cell base station hints to be able to be more authoritative
> than what was specified on the _orig* parameters. To be clear you would
> not be overriding the _orig* parameters but these new settings would be
> a new super set for the driver. Right now we don't let stepping outside
> what is on the _orig* parameters and it seems you do want to do that for
> these hints, but you also *don't* want these saved and cached. Using a
> flag for this preference seems reasonable. You'd still have cached any
> previous settings on the _orig* parameters so a reset would restore
> the device to previous state.
>
>> Sometimes the SIM is removed or there's no reception etc. In this case
>> we must get back to the "default" country, which is burned in
>> NVRAM/FW.
>
> You explaining *two* other requirements here:
>
> (1) The driver provides an alpha2 regulatory domain which is burned onto
> the NVRAM or writtten on the firmware file. You could expand on the
> regulatory_hint_regd(), this would should probably go in first, so
> you'd have:
>
> enum nl80211_driver_reg_hint_type {
> NL80211_DRIVER_REG_HINT_INTERNAL = 0,
> NL80211_DRIVER_REG_HINT_CELL_BASE = 1,
> };
>
> As with (0) the source is internal and only applicable to the
> driver. This should simplify the implementation. As with
> REGULATORY_STRICT_REG, you'd want something similar to help you
> override the *_orig parameters.
I don't think we ever want to override _orig as they restrict changes.
But the overall solution is ok.
>
> (2) regulatory_hint_cell_disconnect() - this would allow the regulatory core
> to reset the device to the "default" alpha2 country. If you
> implement the driver cell base station as explained on (0)
> and the driver reg hint as in (1) then the reset of the regulatory
> settings should already take care of this.
>
>> As explained in my previous email, currently reg.c doesn't
>> have any suitable facility to save this "default" country-code.
>
> Yes it does, we have it for the REGULATORY_STRICT_REG and
> regulatory_hint() case, you have a similar use case but the
> source is simply internal to the driver. That's all.
So the only use for nl80211_driver_reg_hint_type for now would be to
cache the default alpha2 value?
We'll need a new global variable, something like "driver_alpha2",
similar to the existing "user_alpha2". We can't just use the last
request, since it will change..
>
>> And anyway a "restore to default" API doesn't exist.
>
> regulatory_hint_disconnect() exists but that's only used by the SME.
> So indeed we a regulatory_hint_cell_disconnect() seems in order.
Ok that sounds good.
>
> This can be much simpler, I'll reorder your requirements:
>
> (0) regulatory_hint_regd() for type NL80211_DRIVER_REG_HINT_INTERNAL
> with a flag similar to REGULATORY_STRICT_REG
> (1) regulatory_hint_regd() for type NL80211_DRIVER_REG_HINT_CELL_BASE
> (2) regulatory_hint_cell_disconnect()
All this is in addition to the new get_regd() callback for setting the
regdomain data when usermode requests a cell-base hint right?
If so, this should work well.
Arik
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
