On 2013-12-05 22:41, Tijs Van Buggenhout wrote:
> Hi Felix,
>
> I have a question about https://dev.openwrt.org/changeset/38486. While
> backporting the aforementioned patch from trunk to attitude adjustment, I
> noticed that the patch does not include a gettor function for the adc entropy
> of the ar9002 chip, only those of ar5008 and ar9003.
>
> static void ar9003_hw_get_adc_entropy(struct ath_hw *ah, u8 *buf, size_t len)
> static void ar5008_hw_get_adc_entropy(struct ath_hw *ah, u8 *buf, size_t len)
>
> but no
>
> static void ar9002_hw_get_adc_entropy(struct ath_hw *ah, u8 *buf, size_t len)
>
> Furthermore, when the phy_ops are constructed in ar9002_hw_attach_phy_ops,
> the
> get_adc_entropy function pointer is never assigned.
>
> I can be wrong ofcourse, but am under the impression this will create a
> problem when the driver is probed for an ar9002 chip and possibly result in a
> kernel oops, because "ath9k_hw_get_adc_entropy" expects the ath_hw struct to
> have a valid (non-NULL) function pointer for get_adc_entropy. See below:
>
> static inline void ath9k_hw_get_adc_entropy(struct ath_hw *ah,
> u8 *buf, size_t len)
> {
> ath9k_hw_ops(ah)->get_adc_entropy(ah, buf, len);
> }
>
> Did changes in ar9002_phy.c somehow miss inclusion in the patch?
ar5008_hw_attach_phy_ops is called for all chips older than AR9003, and
it is generic enough to work across that range of chips.
> When testing the backported patch for my ar9003 chip, I was confronted with a
> kernel oops at initialisation, which I narrowed down to ath9k_hw_reset
> function, called from ath_get_initial_entropy, where the channels in
> ieee80211_conf data field in common hardware config are not yet initialised
> it
> seems. I changed the condition to verify for a valid pointer before
> dereferencing it, which seems to solve the problem.
> This is different in the mac82011 version from trunk (compat-
> wireless-2013-11-05), where channelFlags from channel are used in the
> condition, which is an u32, and thus can not result in a null pointer
> dereference. My question now is, although the fix seems sufficient, is it
> also
> correct/expected behaviour?
The patch assumes that ah->curchan is left initializes after the tx
power limit test.
In ath9k_init_txpower_limits, the new version has this code:
if (curchan)
ah->curchan = curchan;
I think the old version probably overwrites ah->curchan without checking
for NULL.
- Felix
_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
