| Message ID | 20220320233010.123106-5-wlooi@ucalgary.ca |
|---|---|
| State | New |
| Headers | show |
| Series | ath9k: suggested improvements and bugfixes | expand |
Wenli Looi <wlooi@ucalgary.ca> writes: > The current implementation is reading the wrong eeprom type. > > Fixes: d8ec2e ("ath9k: Add an eeprom_ops callback for retrieving the eepmisc value") > Signed-off-by: Wenli Looi <wlooi@ucalgary.ca> > --- > drivers/net/wireless/ath/ath9k/ar9003_eeprom.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c b/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c > index 669b49b56..a109a44a1 100644 > --- a/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c > +++ b/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c > @@ -5615,7 +5615,7 @@ unsigned int ar9003_get_paprd_scale_factor(struct ath_hw *ah, > > static u8 ar9003_get_eepmisc(struct ath_hw *ah) > { > - return ah->eeprom.map4k.baseEepHeader.eepMisc; > + return ah->eeprom.ar9300_eep.baseEepHeader.opCapFlags.eepMisc; > } Looks like this is only ever used to check for the AR5416_EEPMISC_BIG_ENDIAN flag - so is that never used by AR9300, or how was this ever working given that it's a completely different offset? -Toke
On Mon, Mar 21, 2022 at 8:51 AM Toke Høiland-Jørgensen <toke@toke.dk> wrote: > > Wenli Looi <wlooi@ucalgary.ca> writes: > > > The current implementation is reading the wrong eeprom type. > > > > Fixes: d8ec2e ("ath9k: Add an eeprom_ops callback for retrieving the eepmisc value") > > Signed-off-by: Wenli Looi <wlooi@ucalgary.ca> > > --- > > drivers/net/wireless/ath/ath9k/ar9003_eeprom.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c b/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c > > index 669b49b56..a109a44a1 100644 > > --- a/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c > > +++ b/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c > > @@ -5615,7 +5615,7 @@ unsigned int ar9003_get_paprd_scale_factor(struct ath_hw *ah, > > > > static u8 ar9003_get_eepmisc(struct ath_hw *ah) > > { > > - return ah->eeprom.map4k.baseEepHeader.eepMisc; > > + return ah->eeprom.ar9300_eep.baseEepHeader.opCapFlags.eepMisc; > > } > > Looks like this is only ever used to check for the > AR5416_EEPMISC_BIG_ENDIAN flag - so is that never used by AR9300, or how > was this ever working given that it's a completely different offset? > > -Toke I think it's never used by AR9300, because get_eepmisc is only used in ath9k_hw_nvram_swap_data, which is only used in the eeprom types other than AR9300.
Wenli Looi <wlooi@ucalgary.ca> writes: > On Mon, Mar 21, 2022 at 8:51 AM Toke Høiland-Jørgensen <toke@toke.dk> wrote: >> >> Wenli Looi <wlooi@ucalgary.ca> writes: >> >> > The current implementation is reading the wrong eeprom type. >> > >> > Fixes: d8ec2e ("ath9k: Add an eeprom_ops callback for retrieving the eepmisc value") >> > Signed-off-by: Wenli Looi <wlooi@ucalgary.ca> >> > --- >> > drivers/net/wireless/ath/ath9k/ar9003_eeprom.c | 2 +- >> > 1 file changed, 1 insertion(+), 1 deletion(-) >> > >> > diff --git a/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c b/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c >> > index 669b49b56..a109a44a1 100644 >> > --- a/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c >> > +++ b/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c >> > @@ -5615,7 +5615,7 @@ unsigned int ar9003_get_paprd_scale_factor(struct ath_hw *ah, >> > >> > static u8 ar9003_get_eepmisc(struct ath_hw *ah) >> > { >> > - return ah->eeprom.map4k.baseEepHeader.eepMisc; >> > + return ah->eeprom.ar9300_eep.baseEepHeader.opCapFlags.eepMisc; >> > } >> >> Looks like this is only ever used to check for the >> AR5416_EEPMISC_BIG_ENDIAN flag - so is that never used by AR9300, or how >> was this ever working given that it's a completely different offset? >> >> -Toke > > I think it's never used by AR9300, because get_eepmisc is only used in > ath9k_hw_nvram_swap_data, which is only used in the eeprom types other > than AR9300. Alright, fair enough; let's merge this as a fix anyway... -Toke
Wenli Looi <wlooi@ucalgary.ca> writes: > The current implementation is reading the wrong eeprom type. > > Fixes: d8ec2e ("ath9k: Add an eeprom_ops callback for retrieving the eepmisc value") > Signed-off-by: Wenli Looi <wlooi@ucalgary.ca> The Fixes tag is wrong, I fixed it in the pending branch. In commit 265e303cc469 ("ath9k: fix ar9003_get_eepmisc") Fixes tag Fixes: d8ec2e ("ath9k: Add an eeprom_ops callback for retrieving the eepmisc value") has these problem(s): - SHA1 should be at least 12 digits long Can be fixed by setting core.abbrev to 12 (or more) or (for git v2.11 or later) just making sure it is not set (or set to "auto").
Kalle Valo <kvalo@kernel.org> writes: > Wenli Looi <wlooi@ucalgary.ca> writes: > >> The current implementation is reading the wrong eeprom type. >> >> Fixes: d8ec2e ("ath9k: Add an eeprom_ops callback for retrieving the eepmisc value") >> Signed-off-by: Wenli Looi <wlooi@ucalgary.ca> > > The Fixes tag is wrong, I fixed it in the pending branch. Ah, oops, my bad for not catching that; thanks for the fixup! :) -Toke
Toke Høiland-Jørgensen <toke@toke.dk> writes: > Kalle Valo <kvalo@kernel.org> writes: > >> Wenli Looi <wlooi@ucalgary.ca> writes: >> >>> The current implementation is reading the wrong eeprom type. >>> >>> Fixes: d8ec2e ("ath9k: Add an eeprom_ops callback for retrieving >>> the eepmisc value") >>> Signed-off-by: Wenli Looi <wlooi@ucalgary.ca> >> >> The Fixes tag is wrong, I fixed it in the pending branch. > > Ah, oops, my bad for not catching that; thanks for the fixup! :) No worries, I'm using Stephen's script which check that :)
Kalle Valo <kvalo@kernel.org> writes: > Toke Høiland-Jørgensen <toke@toke.dk> writes: > >> Kalle Valo <kvalo@kernel.org> writes: >> >>> Wenli Looi <wlooi@ucalgary.ca> writes: >>> >>>> The current implementation is reading the wrong eeprom type. >>>> >>>> Fixes: d8ec2e ("ath9k: Add an eeprom_ops callback for retrieving >>>> the eepmisc value") >>>> Signed-off-by: Wenli Looi <wlooi@ucalgary.ca> >>> >>> The Fixes tag is wrong, I fixed it in the pending branch. >> >> Ah, oops, my bad for not catching that; thanks for the fixup! :) > > No worries, I'm using Stephen's script which check that :) Ah yes, I thought I recognised the format of the notice ;) -Toke
On Wed, Mar 23, 2022 at 3:30 AM Kalle Valo <kvalo@kernel.org> wrote: > > Wenli Looi <wlooi@ucalgary.ca> writes: > > > The current implementation is reading the wrong eeprom type. > > > > Fixes: d8ec2e ("ath9k: Add an eeprom_ops callback for retrieving the eepmisc value") > > Signed-off-by: Wenli Looi <wlooi@ucalgary.ca> > > The Fixes tag is wrong, I fixed it in the pending branch. > Thanks for the fix!
diff --git a/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c b/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c index 669b49b56..a109a44a1 100644 --- a/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c +++ b/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c @@ -5615,7 +5615,7 @@ unsigned int ar9003_get_paprd_scale_factor(struct ath_hw *ah, static u8 ar9003_get_eepmisc(struct ath_hw *ah) { - return ah->eeprom.map4k.baseEepHeader.eepMisc; + return ah->eeprom.ar9300_eep.baseEepHeader.opCapFlags.eepMisc; } const struct eeprom_ops eep_ar9300_ops = {
The current implementation is reading the wrong eeprom type. Fixes: d8ec2e ("ath9k: Add an eeprom_ops callback for retrieving the eepmisc value") Signed-off-by: Wenli Looi <wlooi@ucalgary.ca> --- drivers/net/wireless/ath/ath9k/ar9003_eeprom.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)