Skip to content

Commit 7edce37

Browse files
Ansuelkuba-moo
authored andcommitted
net: phy: aquantia: drop wrong endianness conversion for addr and CRC
On further testing on BE target with kernel test robot, it was notice that the endianness conversion for addr and CRC in fw_load_memory was wrong. Drop the cpu_to_le32 conversion for addr load as it's not needed. Use get_unaligned_le32 instead of get_unaligned for FW data word load to correctly convert data in the correct order to follow system endian. Also drop the cpu_to_be32 for CRC calculation as it's wrong and would cause different CRC on BE system. The loaded word is swapped internally and MAILBOX calculates the CRC on the swapped word. To correctly calculate the CRC to be later matched with the one from MAILBOX, use an u8 struct and swap the word there to keep the same order on both LE and BE for crc_ccitt_false function. Also add additional comments on how the CRC verification for the loaded section works. CRC is calculated as we load the section and verified with the MAILBOX only after the entire section is loaded to skip additional slowdown by loop the section data again. Reported-by: kernel test robot <[email protected]> Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/ Fixes: e93984e ("net: phy: aquantia: add firmware load support") Tested-by: Robert Marko <[email protected]> # ipq8072 LE device Signed-off-by: Christian Marangi <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Jakub Kicinski <[email protected]>
1 parent cb2f01b commit 7edce37

File tree

1 file changed

+14
-10
lines changed

1 file changed

+14
-10
lines changed

drivers/net/phy/aquantia/aquantia_firmware.c

+14-10
Original file line numberDiff line numberDiff line change
@@ -93,9 +93,6 @@ static int aqr_fw_load_memory(struct phy_device *phydev, u32 addr,
9393
u16 crc = 0, up_crc;
9494
size_t pos;
9595

96-
/* PHY expect addr in LE */
97-
addr = (__force u32)cpu_to_le32(addr);
98-
9996
phy_write_mmd(phydev, MDIO_MMD_VEND1,
10097
VEND1_GLOBAL_MAILBOX_INTERFACE1,
10198
VEND1_GLOBAL_MAILBOX_INTERFACE1_CRC_RESET);
@@ -110,10 +107,11 @@ static int aqr_fw_load_memory(struct phy_device *phydev, u32 addr,
110107
* If a firmware that is not word aligned is found, please report upstream.
111108
*/
112109
for (pos = 0; pos < len; pos += sizeof(u32)) {
110+
u8 crc_data[4];
113111
u32 word;
114112

115113
/* FW data is always stored in little-endian */
116-
word = get_unaligned((const u32 *)(data + pos));
114+
word = get_unaligned_le32((const u32 *)(data + pos));
117115

118116
phy_write_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLOBAL_MAILBOX_INTERFACE5,
119117
VEND1_GLOBAL_MAILBOX_INTERFACE5_MSW_DATA(word));
@@ -124,15 +122,21 @@ static int aqr_fw_load_memory(struct phy_device *phydev, u32 addr,
124122
VEND1_GLOBAL_MAILBOX_INTERFACE1_EXECUTE |
125123
VEND1_GLOBAL_MAILBOX_INTERFACE1_WRITE);
126124

127-
/* calculate CRC as we load data to the mailbox.
128-
* We convert word to big-endian as PHY is BE and mailbox will
129-
* return a BE CRC.
125+
/* Word is swapped internally and MAILBOX CRC is calculated
126+
* using big-endian order. Mimic what the PHY does to have a
127+
* matching CRC...
130128
*/
131-
word = (__force u32)cpu_to_be32(word);
132-
crc = crc_ccitt_false(crc, (u8 *)&word, sizeof(word));
133-
}
129+
crc_data[0] = word >> 24;
130+
crc_data[1] = word >> 16;
131+
crc_data[2] = word >> 8;
132+
crc_data[3] = word;
134133

134+
/* ...calculate CRC as we load data... */
135+
crc = crc_ccitt_false(crc, crc_data, sizeof(crc_data));
136+
}
137+
/* ...gets CRC from MAILBOX after we have loaded the entire section... */
135138
up_crc = phy_read_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLOBAL_MAILBOX_INTERFACE2);
139+
/* ...and make sure it does match our calculated CRC */
136140
if (crc != up_crc) {
137141
phydev_err(phydev, "CRC mismatch: calculated 0x%04x PHY 0x%04x\n",
138142
crc, up_crc);

0 commit comments

Comments
 (0)