On Sat, 2023-06-10 at 13:49 +0000, Shayne Chen (陳軒丞) wrote:
> On Sat, 2023-06-10 at 12:06 +0200, lorenzo@xxxxxxxxxx wrote:
> > > On Sat, 2023-06-10 at 02:47 +0800, Shayne Chen wrote:
> > > > > > On Fri, 2023-06-09 at 18:34 +0200,
> > > > > > lorenzo.bianconi@xxxxxxxxxx
> > > > >
> > > > > wrote:
> > > > > > > On Jun 09, Ryder Lee wrote:
> > > > > > > > On Fri, 2023-06-09 at 10:15 +0200, Lorenzo Bianconi
> > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > External email : Please do not click links or open
> > > > >
> > > > > attachments
> > > > > > > > > until you have verified the sender or the content.
> > > > > > > > > Introduce connac3_mac in mt76_connac library to
> > > > > > > > > reuse
> > > > > > > > > mac
> > > > >
> > > > > code
> > > > > > > > > shared between WiFi7 chipsets.
> > > > > > > > >
> > > > > > > > > Signed-off-by: Lorenzo Bianconi <lorenzo@xxxxxxxxxx>
> > > >
> > > > Hi Lorenzo,
> >
> > Hi Shayne,
> >
>
> Hi Lorenzo,
>
> > > >
> > > > I don't think it's a good idea to start sharing mac or mcu
> > > > functions
> > > > for all WiFi7 chipsets at such early stage.
> > > >
> > > > The driver is still under many processes of bug fixing,
> > > > performance
> > > > tuning, and new features development.
> > > > Chipsets that mainly used for AP or STA segment have different
> > > > designs
> > > > and should face different problems.
> > > >
> > > > Start sharing the code at early stage will break the
> > > > independence,
> > > > make
> > > > it more hard to develop and do verifications, since many
> > > > changes
> > > > will
> > > > affect chipsets of both segments.
> >
> > my goal is to share the code that hardly will be changed (or that
> > will have
> > very few changes in the future, i.e. I have not changed mcu
> > codebase).
> > If you consider for example the routines below:
> >
> > - mt76_connac*_mac_write_txwi_8023()
> > - mt76_connac*_mac_write_txwi_80211()
> > - mt76_connac*_mac_add_txs_skb()
> > - mt76_connac*_txwi_free()
> > - ...
> >
> > they are the same for mt7615/mt7663, mt7921/mt7915, mt7996/.. ,
> > what
> > is
> > changing is just register map between versions (e.g. mt7615/mt7663
> > and
> > mt7915/mt7921). I think it is not a good idea to copy-paste the
> > code
> > because it will just make the code much harder to maintain, and
> > this
> > will be
> > more difficult to address in the future.
> > If you consider the commit below:
> >
> > commit ead449023d3acb1424886d7b8cc672ed69bdd27e
> > Author: Lorenzo Bianconi <lorenzo@xxxxxxxxxx>
> > Date: Sun May 28 12:28:49 2023 +0200
> >
> > wifi: mt76: mt7996: fix possible NULL pointer dereference in
> > mt7996_mac_write_txwi()
> >
> > Fix possible NULL pointer dereference on mvif pointer in
> > mt7996_mac_write_txwi routine.
> >
> > Deren will needs to monitor the ML and apply the same fix for the
> > other WiFi7
> > chipset too.
> >
This depends on viewpoint. For instance, I planed to add many changes
for mt7986 but 7921 doesn't use (or even don't care), but I need to
check other don't care devices without any help (because they don't
care), so we maintain more an more patches in local branch rather than
upstreming.
Other that that, I think it's ok in wifi6 but wifi7 is REALLY
different. it's better to wait the major MLO development finished to
have a whole picture to know what the best way to unify.
> > Moreover I kept mt7996/mac.{c,h} to put the per-chipset codebase.
> > If you think MT76_CONNAC3_BASIC_RATES_TBL11 and
> > MT76_CONNAC3_BEACON_RATES_TBL25
> > are sensible, we can put them in mt7996/mac.h
>
and this is just one anther example I can tell right now.
> I understand your concern, but if the code starts to be shared at
> early
> stage, we may need to add several is_mt7996() to split different
> parts
> temporarily or permanently, since we have different development
> procedure and focusing, on devices of different segements.
>
> Below are some examples I concern:
> - We're doing some tests about switching all TXS to PPDU TXS, and the
> code in mac_add_txs_skb() needs to be changed. If devices of STA
> segement don't want to change or won't switch in short time, we may
> need to add is_mt7996() to split.
>
> - When adding MLO support, there will be many changes in
> mac_write_txwi() and txwi_free(), compared to previous chipsets. To
> prevent from adding side effects to other devices, we may also need
> to
> add is_mt7996() to split code at least for temporary.
>
> I would like to know is it fine with you to add some is_mt7996() to
> the
> shared code for the above examples?
> I think this could help to balance the trade off between sharing code
> and assuring the driver stability on different segements.
>
> Thanks,
> Shayne
> > > > > > > > >
