[PATCH v2 03/12] wifi: ath12k: modify ath12k mac start/stop ops for single wiphy

[Rameshkumar Sundaram (QUIC)]

> -----Original Message-----
> From: Kalle Valo <kvalo at kernel.org>
> Sent: Friday, February 16, 2024 10:06 PM
> To: Rameshkumar Sundaram (QUIC) <quic_ramess at quicinc.com>
> Cc: ath12k at lists.infradead.org; linux-wireless at vger.kernel.org; Sriram
> R <quic_srirrama at quicinc.com>
> Subject: Re: [PATCH v2 03/12] wifi: ath12k: modify ath12k mac
> start/stop ops for single wiphy
> 
> Rameshkumar Sundaram <quic_ramess at quicinc.com> writes:
> 
> > From: Sriram R <quic_srirrama at quicinc.com>
> >
> > When mac80211 does drv start/stop, apply the state change for all
> the
> > radios within the wiphy in ath12k.
> >
> > Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-
> QCAHKSWPL_SILICONZ-1
> > Tested-on: WCN7850 hw2.0 PCI
> > WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3
> >
> > Signed-off-by: Sriram R <quic_srirrama at quicinc.com>
> > Signed-off-by: Rameshkumar Sundaram <quic_ramess at quicinc.com>
> 
> I haven't reviewed the patchset fully yet, first quick impressions:
> 
> > +static void ath12k_drain_tx(struct ath12k_hw *ah) {
> > +	int i;
> > +	struct ath12k *ar;
> 
> We usually try to have reverse xmas style, of course not always possible
> but here it is.
> 
Sure, Will fix it v3.

> > +	ar = ah->radio;
> > +
> > +	for (i = 0; i < ah->num_radio; i++) {
> > +		ath12k_mac_drain_tx(ar);
> > +		ar++;
> > +	}
> > +}
> 
> Please avoid pointer arithmetic (in this case 'ar++') as much as possible,
> it's just so easy to shoot yourself in the foot. In patch 1 you add
> ath12k_ah_to_ar(), why not use it?
Sure, will avoid this and take reference from ah->radio[]
> 
> And I see this pattern quite a lot, should we have
> ath12k_for_each_radio() or something like that? Or did I see that macro
> in some patch already?
Sure, makes sense, guess it was in v1 of patch [01/12] in this series, but was removed and used inline in subsequent patch sets.
will introduced it back.
> 
> --
> https://patchwork.kernel.org/project/linux-wireless/list/
> 
> https://wireless.wiki.kernel.org/en/developers/documentation/submitti
> ngpatches