> 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