On Mon, 2018-02-19 at 21:11 +0100, wilfried.weissm...@gmx.at wrote:
> From: Wilfried Weissmann <wilfried.weissm...@gmx.at>

Every patch should have a description that not only explains what has been
changed but also why these changes have been made. Please add such a
description. Additionally, please also fix spelling of the patch subject
("endianness").

> ---
>  drivers/scsi/mvsas/mv_94xx.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/mvsas/mv_94xx.c b/drivers/scsi/mvsas/mv_94xx.c
> index 7de5d8d75..926086f39 100644
> --- a/drivers/scsi/mvsas/mv_94xx.c
> +++ b/drivers/scsi/mvsas/mv_94xx.c
> @@ -1088,7 +1088,7 @@ static int mvs_94xx_gpio_write(struct mvs_prv_info 
> *mvs_prv,
>                       * if bit is set then create a mask with the first
>                       * bit of the drive set in the mask ...
>                       */
> -                     u32 bit = (write_data[i/8] & (1 << (i&(8-1)))) ?
> +                     u32 bit = (write_data[3-(i/8)] & (1 << (i&(8-1)))) ?
>                               1<<(24-drive*8) : 0;

The next person who reads this code but who has not followed the discussion
of this patch will have a hard time figuring out the purpose of the "3 - ".
Please consider to use get_unaligned_?e32(), swab32() or similar to make this
code easier to read.
 
>                       /*
> @@ -1132,7 +1132,7 @@ static int mvs_94xx_gpio_write(struct mvs_prv_info 
> *mvs_prv,
> -                             be32_to_cpu(((u32 *) write_data)[i]));
> +                             le32_to_cpu(((u32 *) write_data)[i]));

Have the changes in this patch been verified with sparse? I expect that sparse
will complain about this code. Please use get_unaligned_le32() instead of open-
coding it.

Thanks,

Bart.


Reply via email to