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, can you send a
formal review please ?
>
> Thanks,
> John
--
Damien Le Moal
Western Digital Research
