Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <a7895e73-70a3-450d-64f9-8256c9470d25@gmail.com>
Date:   Sat, 15 Apr 2023 18:02:32 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.9.1
Subject: Re: [PATCH] ath9k: fix calibration data endianness
To:     =?UTF-8?Q?Toke_H=c3=b8iland-J=c3=b8rgensen?= <toke@toke.dk>,
        =?UTF-8?Q?=c3=81lvaro_Fern=c3=a1ndez_Rojas?= <noltari@gmail.com>,
        f.fainelli@gmail.com, jonas.gorski@gmail.com, nbd@nbd.name,
        kvalo@kernel.org, davem@davemloft.net, edumazet@google.com,
        kuba@kernel.org, pabeni@redhat.com, linux-wireless@vger.kernel.org,
        netdev@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20230415150542.2368179-1-noltari@gmail.com>
 <87leitxj4k.fsf@toke.dk>
Content-Language: de-DE
From:   Christian Lamparter <chunkeey@gmail.com>
In-Reply-To: <87leitxj4k.fsf@toke.dk>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

Hi,

On 4/15/23 17:25, Toke Høiland-Jørgensen wrote:
> Álvaro Fernández Rojas <noltari@gmail.com> writes:
> 
>> BCM63xx (Big Endian MIPS) devices store the calibration data in MTD
>> partitions but it needs to be swapped in order to work, otherwise it fails:
>> ath9k 0000:00:01.0: enabling device (0000 -> 0002)
>> ath: phy0: Ignoring endianness difference in EEPROM magic bytes.
>> ath: phy0: Bad EEPROM VER 0x0001 or REV 0x00e0
>> ath: phy0: Unable to initialize hardware; initialization status: -22
>> ath9k 0000:00:01.0: Failed to initialize device
>> ath9k: probe of 0000:00:01.0 failed with error -22
> 
> How does this affect other platforms? Why was the NO_EEP_SWAP flag set
> in the first place? Christian, care to comment on this?

I knew this would come up. I've written what I know and remember in the
pull-request/buglink.

Maybe this can be added to the commit?
Link: https://github.com/openwrt/openwrt/pull/12365

| From what I remember, the ah->ah_flags |= AH_NO_EEP_SWAP; was copied verbatim from ath9k_of_init's request_eeprom.

Since the existing request_firmware eeprom fetcher code set the flag,
the nvmem code had to do it too.

In theory, I don't think that not setting the AH_NO_EEP_SWAP flag will cause havoc.
I don't know if there are devices out there, which have a swapped magic (which is
used to detect the endianess), but the caldata is in the correct endiannes (or
vice versa - Magic is correct, but data needs swapping).

I can run tests with it on a Netzgear WNDR3700v2 (AR7161+2xAR9220)
and FritzBox 7360v2 (Lantiq XWAY+AR9220). (But these worked fine.
So I don't expect there to be a new issue there).

The older platform_data code has a "endian_check" flag in
the ath9k_platform_data struct:
<https://github.com/torvalds/linux/blob/master/include/linux/ath9k_platform.h#L38>
Which enables the check and the endian-swapping (though it's disabled by default).

Cheers,
Christian