Messages in this thread | | | Date | Wed, 22 Nov 2023 00:32:56 +0100 | From | Christian Marangi <> | Subject | Re: [net-next PATCH] net: phy: aquantia: make mailbox interface4 lsw addr mask more specific |
| |
On Tue, Nov 21, 2023 at 03:08:59PM -0800, Jakub Kicinski wrote: > On Mon, 20 Nov 2023 20:35:04 +0100 Christian Marangi wrote: > > It seems some arch (s390) require a more specific mask for FIELD_PREP > > and doesn't like using GENMASK(15, 2) for u16 values. > > > > Fix the compilation error by adding the additional mask for the BITS > > that the PHY ignore and AND the passed addr with the real mask that the > > PHY will parse for the mailbox interface 4 addr to make sure extra > > values are correctly removed. > > Ah. Um. Pff. Erm. I'm not sure. > > Endianness is not my strong suit but this code: > > /* PHY expect addr in LE */ > addr = (__force u32)cpu_to_le32(addr); > > /* ... use (u16)(addr) */ > /* ... use (u16)(addr >> 16) */ > > does not make sense to me. > > You're operating on register values here, there is no endian. > Endian only exists when you store or load from memory. IOW, this: > > addr = 0x12345678; > print((u16)addr); > print(addr >> 16); > > will print the same exact thing regardless of the CPU endian. > > Why did you put the byte swap in there?
the 2 addr comes from a define
#define DRAM_BASE_ADDR 0x3FFE0000 #define IRAM_BASE_ADDR 0x40000000
it wasn't clear to me if on BE these addrs gets saved differently or not. PHY wants the addr in LE.
On testing by removing the cpu_to_le32 the error is correctly removed!
I guess on BE the addr was actually swapped and FIELD_GET was correctly warning (and failing) as data was missing in applying the mask.
If all of this makes sense, will send a followup patch that drop the cpu_to_le32 and also the other in the bottom that does cpu_to_be32 (to a __swab32 as FW is LE and mailbox calculate CRC in BE)
-- Ansuel
| |