On 2/19/2025 1:37 AM, Michal Swiatkowski wrote:
> On Tue, Feb 18, 2025 at 04:46:34PM -0800, Jacob Keller wrote:
>> From: Paul Greenwalt <paul.greenw...@intel.com>
>>
>> With E830 Get Tx Topology AQ command (opcode 0x0418) returns an error when
>> setting the AQ command read flag, and since the get command is a direct
>> command there is no need to set the read flag.
>>
>> Fix this by only setting read flag on set command.
>
> Why it isn't true for other hw? I mean, why not:
> if (set)
> RD_FLAG
> else
> NOT_RD_FLAG
> Other hw needs RD flag in case of get too?
>
>From what I understand, we didn't anticipate this flow changing. E810
and E822 hardware require FLAG_RD for both get and set, while E825-C and
E830 expect FLAG_RD only for set, but not for get.
>>
>
> Don't you need fixes tag?
You're correct. I'll add it in v2
>> Signed-off-by: Paul Greenwalt <paul.greenw...@intel.com>
>> Signed-off-by: Jacob Keller <jacob.e.kel...@intel.com>
>> ---
>> drivers/net/ethernet/intel/ice/ice_ddp.c | 10 +++++-----
>> 1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/ice/ice_ddp.c
>> b/drivers/net/ethernet/intel/ice/ice_ddp.c
>> index
>> 03988be03729b76e96188864896527060c8c4d5b..49bd49ab3ccf36c990144894e887341459377a2d
>> 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_ddp.c
>> +++ b/drivers/net/ethernet/intel/ice/ice_ddp.c
>> @@ -2345,15 +2345,15 @@ ice_get_set_tx_topo(struct ice_hw *hw, u8 *buf, u16
>> buf_size,
>> cmd->set_flags |= ICE_AQC_TX_TOPO_FLAGS_SRC_RAM |
>> ICE_AQC_TX_TOPO_FLAGS_LOAD_NEW;
>>
>> - if (ice_is_e825c(hw))
>> - desc.flags |= cpu_to_le16(ICE_AQ_FLAG_RD);
>> + desc.flags |= cpu_to_le16(ICE_AQ_FLAG_RD);
>> } else {
>> ice_fill_dflt_direct_cmd_desc(&desc, ice_aqc_opc_get_tx_topo);
>> cmd->get_flags = ICE_AQC_TX_TOPO_GET_RAM;
>> - }
>>
>> - if (!ice_is_e825c(hw))
>> - desc.flags |= cpu_to_le16(ICE_AQ_FLAG_RD);
>> + if (hw->mac_type != ICE_MAC_GENERIC_3K_E825 &&
>> + hw->mac_type != ICE_MAC_E830)
>> + desc.flags |= cpu_to_le16(ICE_AQ_FLAG_RD);
>> + }
>>
>> status = ice_aq_send_cmd(hw, &desc, buf, buf_size, cd);
>> if (status)
>>
>
> In general looks fine, only one question.
>
> Thanks,
> Michal
Thanks for the review, I'll send a v2 with this cleaned up and include a
fixes tag.
>
>> ---
>> base-commit: f5da7c45188eea71394bf445655cae2df88a7788
>> change-id: 20250218-jk-e830-ddp-loading-fix-9efdbdfc270a
>>
>> Best regards,
>> --
>> Jacob Keller <jacob.e.kel...@intel.com>
