Search Linux Wireless

Re: [PATCH 06/10] mt76x2: move mt76x2_dev in mt76x02_util.h

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> On 3. Oct 2018, at 13:12, Stanislaw Gruszka <sgruszka@xxxxxxxxxx> wrote:
> 
> On Wed, Oct 03, 2018 at 01:01:40PM +0200, Lorenzo Bianconi wrote:
>>> 
>>> On Tue, Oct 02, 2018 at 12:19:04AM +0200, Lorenzo Bianconi wrote:
>>>> Move mt76x2_dev in mt76x02_util.h and rename it in mt76x02_dev
>>>> in order to be shared between mt76x2 and mt76x0 driver
>>> <snip>
>>>> +struct mt76x02_dev {
>>>> +     struct mt76_dev mt76; /* must be first */
>>>> +
>>>> +     struct mac_address macaddr_list[8];
>>>> +
>>>> +     struct mutex mutex;
>>>> +
>>>> +     u8 txdone_seq;
>>>> +     DECLARE_KFIFO_PTR(txstatus_fifo, struct mt76x02_tx_status);
>>>> +
>>>> +     struct sk_buff *rx_head;
>>>> +
>>>> +     struct tasklet_struct tx_tasklet;
>>>> +     struct tasklet_struct pre_tbtt_tasklet;
>>>> +     struct delayed_work cal_work;
>>>> +     struct delayed_work mac_work;
>>>> +
>>>> +     u32 aggr_stats[32];
>>>> +
>>>> +     struct sk_buff *beacons[8];
>>>> +     u8 beacon_mask;
>>>> +     u8 beacon_data_mask;
>>>> +
>>>> +     u8 tbtt_count;
>>>> +     u16 beacon_int;
>>>> +
>>>> +     struct mt76x02_calibration cal;
>>>> +
>>>> +     s8 target_power;
>>>> +     s8 target_power_delta[2];
>>>> +     bool enable_tpc;
>>>> +
>>>> +     u8 coverage_class;
>>>> +     u8 slottime;
>>>> +
>>>> +     struct mt76x02_dfs_pattern_detector dfs_pd;
>>>> +};
>>>> +
>>> <snip>
>>>> static bool
>>>> -mt76x2_has_cal_free_data(struct mt76x2_dev *dev, u8 *efuse)
>>>> +mt76x2_has_cal_free_data(struct mt76x02_dev *dev, u8 *efuse)
>>> 
>>> I don't think this is right approach. I would rather prefer to have
>>> common data structures embeded in mt76x2_dev and mt76x0_dev
>>> structures to have chip sepcific fields/data separated.
>>> 
>> 
>> The reason of this patch is that mt76x0_dev fields are already in mt76x2_dev
>> so I guess there is no need to have different structures. Moreover in
>> this way we can
>> remove a lot of duplicated code between mt76x0 and mt76x2 drivers.
> 
> But you can still create additional structures i.e.
> 
> mt76x02_power {
>    s8 target_power;
>    s8 target_power_delta[2];
>    bool enable_tpc;
> }
> 
> mt76x02_conf {
>    u8 coverage_class;
>         u8 slottime;
> }
> 
> put them into mt76xN_dev and still remove dupicated code ?
Quite often, mt76_dev would be needed as well for register access, which means extra parameters for a lot of functions.
I think Lorenzo’s approach makes the code a lot more concise, and makes it easier to share more code between mt76x0 and mt76x2.

- Felix




[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Wireless Personal Area Network]     [Linux Bluetooth]     [Wireless Regulations]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Hiking]     [MIPS Linux]     [ARM Linux]     [Linux RAID]

  Powered by Linux