Re: [PATCH 3/3] Fixed coding style issues.

From: Oliver Neukum
Date: Wed May 17 2017 - 03:33:38 EST


Am Dienstag, den 16.05.2017, 21:17 -0500 schrieb Alberto Ladron:
> Signed-off-by: Alberto Ladron <alberto.ladron@xxxxxxxxx>
> ---
> drivers/usb/storage/shuttle_usbat.c | 83 ++++++++++++++++++-------------------
> 1 file changed, 41 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/usb/storage/shuttle_usbat.c b/drivers/usb/storage/shuttle_usbat.c
> index 3b0294e..9eddc40 100644
> --- a/drivers/usb/storage/shuttle_usbat.c
> +++ b/drivers/usb/storage/shuttle_usbat.c
> @@ -152,13 +152,13 @@ struct usbat_info {
> unsigned long sense_ascq; /* additional sense code qualifier */
> };
>
> -#define short_pack(LSB,MSB) ( ((u16)(LSB)) | ( ((u16)(MSB))<<8 ) )
> +#define short_pack(LSB, MSB) (((u16)(LSB)) | (((u16)(MSB))<<8))
> #define LSB_of(s) ((s)&0xFF)
> #define MSB_of(s) ((s)>>8)
>
> -static int transferred = 0;
> +static int transferred;
>
> -static int usbat_flash_transport(struct scsi_cmnd * srb, struct us_data *us);
> +static int usbat_flash_transport(struct scsi_cmnd *srb, struct us_data *us);
> static int usbat_hp8200e_transport(struct scsi_cmnd *srb, struct us_data *us);
>
> static int init_usbat_cd(struct us_data *us);
> @@ -172,7 +172,7 @@ static int init_usbat_flash(struct us_data *us);
> vendorName, productName, useProtocol, useTransport, \
> initFunction, flags) \
> { USB_DEVICE_VER(id_vendor, id_product, bcdDeviceMin, bcdDeviceMax), \
> - .driver_info = (flags) }
> +.driver_info = (flags) }
>
> static struct usb_device_id usbat_usb_ids[] = {
> # include "unusual_usbat.h"
> @@ -225,7 +225,7 @@ static void usbat_pack_ata_sector_cmd(unsigned char *buf,
> */
> static int usbat_get_device_type(struct us_data *us)
> {
> - return ((struct usbat_info*)us->extra)->devicetype;
> + return ((struct usbat_info *)us->extra)->devicetype;
> }
>
> /*
> @@ -268,7 +268,7 @@ static int usbat_write(struct us_data *us,
> * Convenience function to perform a bulk read
> */
> static int usbat_bulk_read(struct us_data *us,
> - void* buf,
> + void *buf,
> unsigned int len,
> int use_sg)
> {
> @@ -283,7 +283,7 @@ static int usbat_bulk_read(struct us_data *us,
> * Convenience function to perform a bulk write
> */
> static int usbat_bulk_write(struct us_data *us,
> - void* buf,
> + void *buf,
> unsigned int len,
> int use_sg)
> {
> @@ -416,11 +416,11 @@ static int usbat_wait_not_busy(struct us_data *us, int minutes)
> * minutes!
> */
>
> - for (i=0; i<1200+minutes*60; i++) {
> + for (i = 0; i < 1200+minutes*60; i++) {
>
> - result = usbat_get_status(us, status);
> + result = usbat_get_status(us, status);
>
> - if (result!=USB_STOR_XFER_GOOD)
> + if (result != USB_STOR_XFER_GOOD)
> return USB_STOR_TRANSPORT_ERROR;
> if (*status & 0x01) { /* check condition */
> result = usbat_read(us, USBAT_ATA, 0x10, status);
> @@ -429,16 +429,16 @@ static int usbat_wait_not_busy(struct us_data *us, int minutes)
> if (*status & 0x20) /* device fault */
> return USB_STOR_TRANSPORT_FAILED;
>
> - if ((*status & 0x80)==0x00) { /* not busy */
> + if ((*status & 0x80) == 0x00) { /* not busy */

Would you care to make a second pass and submit patches using the BIT()
macro where appropriate?

> [..]
> @@ -1753,13 +1752,13 @@ static int usbat_flash_transport(struct scsi_cmnd * srb, struct us_data *us)
> * I don't think we'll ever see a READ_12 but support it anyway
> */
> block = ((u32)(srb->cmnd[2]) << 24) | ((u32)(srb->cmnd[3]) << 16) |
> - ((u32)(srb->cmnd[4]) << 8) | ((u32)(srb->cmnd[5]));
> + ((u32)(srb->cmnd[4]) << 8) | ((u32)(srb->cmnd[5]));
>
> blocks = ((u32)(srb->cmnd[6]) << 24) | ((u32)(srb->cmnd[7]) << 16) |
> - ((u32)(srb->cmnd[8]) << 8) | ((u32)(srb->cmnd[9]));
> + ((u32)(srb->cmnd[8]) << 8) | ((u32)(srb->cmnd[9]));

There is no point in beautifying such things. Just make another pass
and use the macros for endianness conversion with unaligned access.

Regards
Oliver