| Message ID | 20241023133004.2253830-5-kvalo@kernel.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Jeff Johnson |
| Headers | show |
| Series | wifi: ath12k: MLO support part 2 | expand |
On 10/23/2024 6:30 AM, Kalle Valo wrote: > From: Kalle Valo <quic_kvalo@quicinc.com> > > In the following patch we need to use ath12k_warn() but don't easily have > access to struct ath12k_base (ab) but do have access to struct ath12k_hw (ah). > So add a new warning helper ath12_hw_warn() which takes the latter but the log > output is still identical but uses the struct device pointer stored to struct > ath12k_hw. > > Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3 > > Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com> > --- > drivers/net/wireless/ath/ath12k/core.h | 2 ++ > drivers/net/wireless/ath/ath12k/debug.c | 4 ++-- Qualcomm Innovation Center copyright missing 2024 > drivers/net/wireless/ath/ath12k/debug.h | 5 ++++- > drivers/net/wireless/ath/ath12k/mac.c | 2 ++ > 4 files changed, 10 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/wireless/ath/ath12k/core.h b/drivers/net/wireless/ath/ath12k/core.h > index 6faa46b9adc9..9c4e5fae8930 100644 > --- a/drivers/net/wireless/ath/ath12k/core.h > +++ b/drivers/net/wireless/ath/ath12k/core.h > @@ -684,6 +684,8 @@ struct ath12k { > > struct ath12k_hw { > struct ieee80211_hw *hw; > + struct device *dev; > + > /* Protect the write operation of the hardware state ath12k_hw::state > * between hardware start<=>reconfigure<=>stop transitions. > */ > diff --git a/drivers/net/wireless/ath/ath12k/debug.c b/drivers/net/wireless/ath/ath12k/debug.c > index fe5a732ba9ec..c5c8c7624cdb 100644 > --- a/drivers/net/wireless/ath/ath12k/debug.c > +++ b/drivers/net/wireless/ath/ath12k/debug.c > @@ -36,7 +36,7 @@ void ath12k_err(struct ath12k_base *ab, const char *fmt, ...) > va_end(args); > } > > -void ath12k_warn(struct ath12k_base *ab, const char *fmt, ...) > +void __ath12k_warn(struct device *dev, const char *fmt, ...) > { > struct va_format vaf = { > .fmt = fmt, > @@ -45,7 +45,7 @@ void ath12k_warn(struct ath12k_base *ab, const char *fmt, ...) > > va_start(args, fmt); > vaf.va = &args; > - dev_warn_ratelimited(ab->dev, "%pV", &vaf); > + dev_warn_ratelimited(dev, "%pV", &vaf); > /* TODO: Trace the log */ > va_end(args); > } > diff --git a/drivers/net/wireless/ath/ath12k/debug.h b/drivers/net/wireless/ath/ath12k/debug.h > index f7005917362c..90e801136bc6 100644 > --- a/drivers/net/wireless/ath/ath12k/debug.h > +++ b/drivers/net/wireless/ath/ath12k/debug.h > @@ -31,7 +31,10 @@ enum ath12k_debug_mask { > > __printf(2, 3) void ath12k_info(struct ath12k_base *ab, const char *fmt, ...); > __printf(2, 3) void ath12k_err(struct ath12k_base *ab, const char *fmt, ...); > -__printf(2, 3) void ath12k_warn(struct ath12k_base *ab, const char *fmt, ...); > +__printf(2, 3) void __ath12k_warn(struct device *dev, const char *fmt, ...); > + > +#define ath12k_warn(ab, fmt, ...) __ath12k_warn((ab)->dev, fmt, ##__VA_ARGS__) > +#define ath12k_hw_warn(ah, fmt, ...) __ath12k_warn((ah)->dev, fmt, ##__VA_ARGS__) for consistency should we do this for the other log levels as well? are there places where we currently retrieve ab just for logging where we already have ah, and hence could avoid the extra dereference? > > extern unsigned int ath12k_debug_mask; > > diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c > index 3de6d605cd74..19c445cf52f1 100644 > --- a/drivers/net/wireless/ath/ath12k/mac.c > +++ b/drivers/net/wireless/ath/ath12k/mac.c > @@ -10193,6 +10193,8 @@ int ath12k_mac_allocate(struct ath12k_base *ab) > goto err; > } > > + ah->dev = ab->dev; > + > ab->ah[i] = ah; > } >
Jeff Johnson <quic_jjohnson@quicinc.com> writes: > On 10/23/2024 6:30 AM, Kalle Valo wrote: >> From: Kalle Valo <quic_kvalo@quicinc.com> >> >> In the following patch we need to use ath12k_warn() but don't easily have >> access to struct ath12k_base (ab) but do have access to struct ath12k_hw (ah). >> So add a new warning helper ath12_hw_warn() which takes the latter but the log >> output is still identical but uses the struct device pointer stored to struct >> ath12k_hw. >> >> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3 >> >> Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com> >> --- >> drivers/net/wireless/ath/ath12k/core.h | 2 ++ >> drivers/net/wireless/ath/ath12k/debug.c | 4 ++-- > > Qualcomm Innovation Center copyright missing 2024 Will fix. >> --- a/drivers/net/wireless/ath/ath12k/debug.h >> +++ b/drivers/net/wireless/ath/ath12k/debug.h >> @@ -31,7 +31,10 @@ enum ath12k_debug_mask { >> >> __printf(2, 3) void ath12k_info(struct ath12k_base *ab, const char *fmt, ...); >> __printf(2, 3) void ath12k_err(struct ath12k_base *ab, const char *fmt, ...); >> -__printf(2, 3) void ath12k_warn(struct ath12k_base *ab, const char *fmt, ...); >> +__printf(2, 3) void __ath12k_warn(struct device *dev, const char *fmt, ...); >> + >> +#define ath12k_warn(ab, fmt, ...) __ath12k_warn((ab)->dev, fmt, ##__VA_ARGS__) >> +#define ath12k_hw_warn(ah, fmt, ...) __ath12k_warn((ah)->dev, fmt, ##__VA_ARGS__) > > for consistency should we do this for the other log levels as well? > > are there places where we currently retrieve ab just for logging where we > already have ah, and hence could avoid the extra dereference? That's a good idea but IMHO the cleanup can wait after MLO has landed. Right now there should not be many places where we have to use the ath12k_hw_warn() variant.
diff --git a/drivers/net/wireless/ath/ath12k/core.h b/drivers/net/wireless/ath/ath12k/core.h index 6faa46b9adc9..9c4e5fae8930 100644 --- a/drivers/net/wireless/ath/ath12k/core.h +++ b/drivers/net/wireless/ath/ath12k/core.h @@ -684,6 +684,8 @@ struct ath12k { struct ath12k_hw { struct ieee80211_hw *hw; + struct device *dev; + /* Protect the write operation of the hardware state ath12k_hw::state * between hardware start<=>reconfigure<=>stop transitions. */ diff --git a/drivers/net/wireless/ath/ath12k/debug.c b/drivers/net/wireless/ath/ath12k/debug.c index fe5a732ba9ec..c5c8c7624cdb 100644 --- a/drivers/net/wireless/ath/ath12k/debug.c +++ b/drivers/net/wireless/ath/ath12k/debug.c @@ -36,7 +36,7 @@ void ath12k_err(struct ath12k_base *ab, const char *fmt, ...) va_end(args); } -void ath12k_warn(struct ath12k_base *ab, const char *fmt, ...) +void __ath12k_warn(struct device *dev, const char *fmt, ...) { struct va_format vaf = { .fmt = fmt, @@ -45,7 +45,7 @@ void ath12k_warn(struct ath12k_base *ab, const char *fmt, ...) va_start(args, fmt); vaf.va = &args; - dev_warn_ratelimited(ab->dev, "%pV", &vaf); + dev_warn_ratelimited(dev, "%pV", &vaf); /* TODO: Trace the log */ va_end(args); } diff --git a/drivers/net/wireless/ath/ath12k/debug.h b/drivers/net/wireless/ath/ath12k/debug.h index f7005917362c..90e801136bc6 100644 --- a/drivers/net/wireless/ath/ath12k/debug.h +++ b/drivers/net/wireless/ath/ath12k/debug.h @@ -31,7 +31,10 @@ enum ath12k_debug_mask { __printf(2, 3) void ath12k_info(struct ath12k_base *ab, const char *fmt, ...); __printf(2, 3) void ath12k_err(struct ath12k_base *ab, const char *fmt, ...); -__printf(2, 3) void ath12k_warn(struct ath12k_base *ab, const char *fmt, ...); +__printf(2, 3) void __ath12k_warn(struct device *dev, const char *fmt, ...); + +#define ath12k_warn(ab, fmt, ...) __ath12k_warn((ab)->dev, fmt, ##__VA_ARGS__) +#define ath12k_hw_warn(ah, fmt, ...) __ath12k_warn((ah)->dev, fmt, ##__VA_ARGS__) extern unsigned int ath12k_debug_mask; diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c index 3de6d605cd74..19c445cf52f1 100644 --- a/drivers/net/wireless/ath/ath12k/mac.c +++ b/drivers/net/wireless/ath/ath12k/mac.c @@ -10193,6 +10193,8 @@ int ath12k_mac_allocate(struct ath12k_base *ab) goto err; } + ah->dev = ab->dev; + ab->ah[i] = ah; }