* Re: [Intel-wired-lan] [PATCH iwl-next 1/2] ixgbe: e610: add missing endianness conversion
2026-01-16 12:23 [PATCH iwl-next 1/2] ixgbe: e610: add missing endianness conversion Piotr Kwapulinski
@ 2026-01-16 13:17 ` Paul Menzel
2026-01-19 16:03 ` Kwapulinski, Piotr
2026-01-17 13:34 ` kernel test robot
2026-01-18 12:56 ` Dan Carpenter
2 siblings, 1 reply; 6+ messages in thread
From: Paul Menzel @ 2026-01-16 13:17 UTC (permalink / raw)
To: Piotr Kwapulinski
Cc: intel-wired-lan, netdev, dan.carpenter, horms,
Aleksandr Loktionov
Dear Piotr,
Thank you for your patch.
Am 16.01.26 um 13:23 schrieb Piotr Kwapulinski:
> Fix a possible ACI issue on big-endian platforms.
Please elaborate, why this is needed, and `raw_desc[i]` needs to be
converted.
For the summary/title, you could also be more specific. Something like:
> ixgbe: e610: Convert ACI descriptor buffer(?) to little endian
> Fixes: 46761fd52a88 ("ixgbe: Add support for E610 FW Admin Command Interface")
> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> Signed-off-by: Piotr Kwapulinski <piotr.kwapulinski@intel.com>
> ---
> drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
> index c2f8189..f494e90 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
> @@ -113,7 +113,8 @@ static int ixgbe_aci_send_cmd_execute(struct ixgbe_hw *hw,
>
> /* Descriptor is written to specific registers */
> for (i = 0; i < IXGBE_ACI_DESC_SIZE_IN_DWORDS; i++)
> - IXGBE_WRITE_REG(hw, IXGBE_PF_HIDA(i), raw_desc[i]);
> + IXGBE_WRITE_REG(hw, IXGBE_PF_HIDA(i),
> + le32_to_cpu(raw_desc[i]));
>
> /* SW has to set PF_HICR.C bit and clear PF_HICR.SV and
> * PF_HICR_EV
> @@ -145,7 +146,7 @@ static int ixgbe_aci_send_cmd_execute(struct ixgbe_hw *hw,
> if ((hicr & IXGBE_PF_HICR_SV)) {
> for (i = 0; i < IXGBE_ACI_DESC_SIZE_IN_DWORDS; i++) {
> raw_desc[i] = IXGBE_READ_REG(hw, IXGBE_PF_HIDA(i));
> - raw_desc[i] = raw_desc[i];
> + raw_desc[i] = cpu_to_le32(raw_desc[i]);
> }
> }
>
> @@ -153,7 +154,7 @@ static int ixgbe_aci_send_cmd_execute(struct ixgbe_hw *hw,
> if ((hicr & IXGBE_PF_HICR_EV) && !(hicr & IXGBE_PF_HICR_C)) {
> for (i = 0; i < IXGBE_ACI_DESC_SIZE_IN_DWORDS; i++) {
> raw_desc[i] = IXGBE_READ_REG(hw, IXGBE_PF_HIDA_2(i));
> - raw_desc[i] = raw_desc[i];
> + raw_desc[i] = cpu_to_le32(raw_desc[i]);
> }
> }
Kind regards,
Paul
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH iwl-next 1/2] ixgbe: e610: add missing endianness conversion
2026-01-16 12:23 [PATCH iwl-next 1/2] ixgbe: e610: add missing endianness conversion Piotr Kwapulinski
2026-01-16 13:17 ` [Intel-wired-lan] " Paul Menzel
@ 2026-01-17 13:34 ` kernel test robot
2026-01-18 12:56 ` Dan Carpenter
2 siblings, 0 replies; 6+ messages in thread
From: kernel test robot @ 2026-01-17 13:34 UTC (permalink / raw)
To: Piotr Kwapulinski, intel-wired-lan
Cc: oe-kbuild-all, netdev, dan.carpenter, horms, Piotr Kwapulinski,
Aleksandr Loktionov
Hi Piotr,
kernel test robot noticed the following build warnings:
[auto build test WARNING on tnguy-next-queue/dev-queue]
url: https://github.com/intel-lab-lkp/linux/commits/Piotr-Kwapulinski/ixgbe-e610-add-missing-endianness-conversion/20260116-200705
base: https://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue.git dev-queue
patch link: https://lore.kernel.org/r/20260116122353.78235-1-piotr.kwapulinski%40intel.com
patch subject: [PATCH iwl-next 1/2] ixgbe: e610: add missing endianness conversion
config: x86_64-randconfig-r111-20260117 (https://download.01.org/0day-ci/archive/20260117/202601172130.UkaUgjIE-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260117/202601172130.UkaUgjIE-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202601172130.UkaUgjIE-lkp@intel.com/
sparse warnings: (new ones prefixed by >>)
>> drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c:116:17: sparse: sparse: cast to restricted __le32
>> drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c:149:37: sparse: sparse: incorrect type in assignment (different base types) @@ expected unsigned int [usertype] @@ got restricted __le32 [usertype] @@
drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c:149:37: sparse: expected unsigned int [usertype]
drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c:149:37: sparse: got restricted __le32 [usertype]
drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c:157:37: sparse: sparse: incorrect type in assignment (different base types) @@ expected unsigned int [usertype] @@ got restricted __le32 [usertype] @@
drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c:157:37: sparse: expected unsigned int [usertype]
drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c:157:37: sparse: got restricted __le32 [usertype]
vim +116 drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
35
36 /**
37 * ixgbe_aci_send_cmd_execute - execute sending FW Admin Command to FW Admin
38 * Command Interface
39 * @hw: pointer to the HW struct
40 * @desc: descriptor describing the command
41 * @buf: buffer to use for indirect commands (NULL for direct commands)
42 * @buf_size: size of buffer for indirect commands (0 for direct commands)
43 *
44 * Admin Command is sent using CSR by setting descriptor and buffer in specific
45 * registers.
46 *
47 * Return: the exit code of the operation.
48 * * - 0 - success.
49 * * - -EIO - CSR mechanism is not enabled.
50 * * - -EBUSY - CSR mechanism is busy.
51 * * - -EINVAL - buf_size is too big or
52 * invalid argument buf or buf_size.
53 * * - -ETIME - Admin Command X command timeout.
54 * * - -EIO - Admin Command X invalid state of HICR register or
55 * Admin Command failed because of bad opcode was returned or
56 * Admin Command failed with error Y.
57 */
58 static int ixgbe_aci_send_cmd_execute(struct ixgbe_hw *hw,
59 struct libie_aq_desc *desc,
60 void *buf, u16 buf_size)
61 {
62 u16 opcode, buf_tail_size = buf_size % 4;
63 u32 *raw_desc = (u32 *)desc;
64 u32 hicr, i, buf_tail = 0;
65 bool valid_buf = false;
66
67 hw->aci.last_status = LIBIE_AQ_RC_OK;
68
69 /* It's necessary to check if mechanism is enabled */
70 hicr = IXGBE_READ_REG(hw, IXGBE_PF_HICR);
71
72 if (!(hicr & IXGBE_PF_HICR_EN))
73 return -EIO;
74
75 if (hicr & IXGBE_PF_HICR_C) {
76 hw->aci.last_status = LIBIE_AQ_RC_EBUSY;
77 return -EBUSY;
78 }
79
80 opcode = le16_to_cpu(desc->opcode);
81
82 if (buf_size > IXGBE_ACI_MAX_BUFFER_SIZE)
83 return -EINVAL;
84
85 if (buf)
86 desc->flags |= cpu_to_le16(LIBIE_AQ_FLAG_BUF);
87
88 if (desc->flags & cpu_to_le16(LIBIE_AQ_FLAG_BUF)) {
89 if ((buf && !buf_size) ||
90 (!buf && buf_size))
91 return -EINVAL;
92 if (buf && buf_size)
93 valid_buf = true;
94 }
95
96 if (valid_buf) {
97 if (buf_tail_size)
98 memcpy(&buf_tail, buf + buf_size - buf_tail_size,
99 buf_tail_size);
100
101 if (((buf_size + 3) & ~0x3) > LIBIE_AQ_LG_BUF)
102 desc->flags |= cpu_to_le16(LIBIE_AQ_FLAG_LB);
103
104 desc->datalen = cpu_to_le16(buf_size);
105
106 if (desc->flags & cpu_to_le16(LIBIE_AQ_FLAG_RD)) {
107 for (i = 0; i < buf_size / 4; i++)
108 IXGBE_WRITE_REG(hw, IXGBE_PF_HIBA(i), ((u32 *)buf)[i]);
109 if (buf_tail_size)
110 IXGBE_WRITE_REG(hw, IXGBE_PF_HIBA(i), buf_tail);
111 }
112 }
113
114 /* Descriptor is written to specific registers */
115 for (i = 0; i < IXGBE_ACI_DESC_SIZE_IN_DWORDS; i++)
> 116 IXGBE_WRITE_REG(hw, IXGBE_PF_HIDA(i),
117 le32_to_cpu(raw_desc[i]));
118
119 /* SW has to set PF_HICR.C bit and clear PF_HICR.SV and
120 * PF_HICR_EV
121 */
122 hicr = (IXGBE_READ_REG(hw, IXGBE_PF_HICR) | IXGBE_PF_HICR_C) &
123 ~(IXGBE_PF_HICR_SV | IXGBE_PF_HICR_EV);
124 IXGBE_WRITE_REG(hw, IXGBE_PF_HICR, hicr);
125
126 #define MAX_SLEEP_RESP_US 1000
127 #define MAX_TMOUT_RESP_SYNC_US 100000000
128
129 /* Wait for sync Admin Command response */
130 read_poll_timeout(IXGBE_READ_REG, hicr,
131 (hicr & IXGBE_PF_HICR_SV) ||
132 !(hicr & IXGBE_PF_HICR_C),
133 MAX_SLEEP_RESP_US, MAX_TMOUT_RESP_SYNC_US, true, hw,
134 IXGBE_PF_HICR);
135
136 #define MAX_TMOUT_RESP_ASYNC_US 150000000
137
138 /* Wait for async Admin Command response */
139 read_poll_timeout(IXGBE_READ_REG, hicr,
140 (hicr & IXGBE_PF_HICR_EV) ||
141 !(hicr & IXGBE_PF_HICR_C),
142 MAX_SLEEP_RESP_US, MAX_TMOUT_RESP_ASYNC_US, true, hw,
143 IXGBE_PF_HICR);
144
145 /* Read sync Admin Command response */
146 if ((hicr & IXGBE_PF_HICR_SV)) {
147 for (i = 0; i < IXGBE_ACI_DESC_SIZE_IN_DWORDS; i++) {
148 raw_desc[i] = IXGBE_READ_REG(hw, IXGBE_PF_HIDA(i));
> 149 raw_desc[i] = cpu_to_le32(raw_desc[i]);
150 }
151 }
152
153 /* Read async Admin Command response */
154 if ((hicr & IXGBE_PF_HICR_EV) && !(hicr & IXGBE_PF_HICR_C)) {
155 for (i = 0; i < IXGBE_ACI_DESC_SIZE_IN_DWORDS; i++) {
156 raw_desc[i] = IXGBE_READ_REG(hw, IXGBE_PF_HIDA_2(i));
157 raw_desc[i] = cpu_to_le32(raw_desc[i]);
158 }
159 }
160
161 /* Handle timeout and invalid state of HICR register */
162 if (hicr & IXGBE_PF_HICR_C)
163 return -ETIME;
164
165 if (!(hicr & IXGBE_PF_HICR_SV) && !(hicr & IXGBE_PF_HICR_EV))
166 return -EIO;
167
168 /* For every command other than 0x0014 treat opcode mismatch
169 * as an error. Response to 0x0014 command read from HIDA_2
170 * is a descriptor of an event which is expected to contain
171 * different opcode than the command.
172 */
173 if (desc->opcode != cpu_to_le16(opcode) &&
174 opcode != ixgbe_aci_opc_get_fw_event)
175 return -EIO;
176
177 if (desc->retval) {
178 hw->aci.last_status = (enum libie_aq_err)
179 le16_to_cpu(desc->retval);
180 return -EIO;
181 }
182
183 /* Write a response values to a buf */
184 if (valid_buf) {
185 for (i = 0; i < buf_size / 4; i++)
186 ((u32 *)buf)[i] = IXGBE_READ_REG(hw, IXGBE_PF_HIBA(i));
187 if (buf_tail_size) {
188 buf_tail = IXGBE_READ_REG(hw, IXGBE_PF_HIBA(i));
189 memcpy(buf + buf_size - buf_tail_size, &buf_tail,
190 buf_tail_size);
191 }
192 }
193
194 return 0;
195 }
196
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH iwl-next 1/2] ixgbe: e610: add missing endianness conversion
2026-01-16 12:23 [PATCH iwl-next 1/2] ixgbe: e610: add missing endianness conversion Piotr Kwapulinski
2026-01-16 13:17 ` [Intel-wired-lan] " Paul Menzel
2026-01-17 13:34 ` kernel test robot
@ 2026-01-18 12:56 ` Dan Carpenter
2026-01-19 16:07 ` Kwapulinski, Piotr
2 siblings, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2026-01-18 12:56 UTC (permalink / raw)
To: Piotr Kwapulinski; +Cc: intel-wired-lan, netdev, horms, Aleksandr Loktionov
On Fri, Jan 16, 2026 at 01:23:53PM +0100, Piotr Kwapulinski wrote:
> Fix a possible ACI issue on big-endian platforms.
>
> Fixes: 46761fd52a88 ("ixgbe: Add support for E610 FW Admin Command Interface")
> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> Signed-off-by: Piotr Kwapulinski <piotr.kwapulinski@intel.com>
> ---
Intel has a lot of code which assume that it will only run on little
endian systems... Which is probably a fair assumption, honestly.
For example:
drivers/platform/x86/intel/uncore-frequency/uncore-frequency-common.c:90 store_attr() warn: passing casted pointer '&input' to 'kstrtobool()' 32 vs 1.
drivers/net/ethernet/intel/i40e/i40e_common.c:4345 i40e_led_get_reg() warn: passing casted pointer 'reg_val' to 'i40e_read_phy_register_clause45()' 32 vs 16.
I seem them on occasion when I'm reviewing static checker warnings but
I ignore them because Intel chips are little endian.
I don't have a problem with fixing Sparse endianness warnings, but the
commit message should really say that it doesn't affect real life.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread