Jakub Sitnicki <jsitnicki@xxxxxxxxx> writes:
> On Wed, Oct 21, 2015 at 03:58 AM CEST, Jes Sorensen
> <Jes.Sorensen@xxxxxxxxxx> wrote:
>> Jakub Sitnicki <jsitnicki@xxxxxxxxx> writes:
>>> Signed-off-by: Jakub Sitnicki <jsitnicki@xxxxxxxxx>
>>> ---
>>> drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.c | 95
>>> ++++++++++++------------
>>> drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 30 ++++----
>>> 2 files changed, 66 insertions(+), 59 deletions(-)
>>
>> I don't really see this patch adding any value - it just adds an
>> additional layer of convolution. Unless there is a strong reason for
>> applying it, I am going to drop this one.
>
> The reasoning behind this change, which I should have had explained in
> the commit description, is to later on have a union of two struct
> *_tx_power's:
>
> union {
> struct rtl8723au_tx_power tx_power8723;
> struct rtl8188eu_tx_power tx_power8188;
> } tx_power;
>
> We arrive at this in the last patch from this series ("rtl8xxxu:
> rtl8188eu: Implement parse_efuse()").
>
> The existing *_tx_power_* fields in struct rtl8xxxu_priv don't fit the
> rtl8188eu needs because the vendor driver divides the 2.4 GHz channels
> into 6 as opposed to 3 groups (see the MAX_CHNL_GROUP_24G constant and
> struct txpowerinfo24g in staging/rtl8188eu).
>
> I agree that it does complicate the core structure (rtl8xxxu_priv) for
> no reason until there is actually support for RTL8188EU. However, I
> gave the rtl8192eu driver a glance, and it looks like you might be
> facing the same challenge there.
Jakub,
I see, looks like 8723bu has the same number of groups as well. An
alternative solution would be to do the conversion in parse_efuse() and
explode the values into per-channel arrays instead of keeping them in
groups. That should simplify the runtime portion of the code.
Cheers,
Jes
--
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
