On 6/9/22 21:02, John Garry wrote:
> On 09/06/2022 12:59, Damien Le Moal wrote:
>> On 6/9/22 17:43, John Garry wrote:
>>>> #define DISCOVER_REQ_SIZE 16
>>>> -#define DISCOVER_RESP_SIZE 56
>>>> +#define DISCOVER_RESP_SIZE sizeof(struct smp_disc_resp)
>>> I feel that it would be nice to get rid of these.
>>>
>>> Maybe something like:
>>>
>>> #define smp_execute_task_wrap(dev, req, resp)\
>>> smp_execute_task(dev, req, sizeof(*req), resp, DISCOVER_REQ_SIZE)
>>>
>>>
>>> static int sas_ex_phy_discover_helper(struct domain_device *dev, u8
>>> *disc_req, struct smp_disc_resp *disc_resp, int single)
>>> {
>>> res = smp_execute_task_wrap(dev, disc_req, disc_resp);
>>>
>>> We could even introduce a smp req struct to get rid of DISCOVER_REQ_SIZE
>>> - the (current) code would be a bit less cryptic then.
>> Yes, I think the req size needs a similar treatment too, and we can remove
>> all these REQ/RESP_SIZE macros. But for now, the req side does not bother
>> gcc and I do not see any warning, so I left it. This series is really
>> about getting rid of the warnings so that we can use CONFIG_WERROR.
>> There are some xfs warnings that needs to be taken care of too to be able
>> to use that one again though.
>>
>>> But no big deal. Looks ok apart from that.
>> If you agree to do the req cleanup as a followup series,
>
> ok, I'll assume that you can do it when you get a chance..
Yep, one more pile of patches added to the to-do list :)
>
> can you send a
>> formal review please ?
>>
> Reviewed-by: John Garry <john.garry@xxxxxxxxxx>
Thanks !
--
Damien Le Moal
Western Digital Research
