Re: [PATCH 6/6] keucr: fix some alignment- and whitespace-problems
On Thu, Jun 06, 2013 at 06:10:50PM +0200, Johannes Schilling wrote: > resolves checkpatch errors and warnings regarding whitespace around > operators, line lengths and indentation. > I feel like this should be broken up into several patches. A lot of these changes will make checkpatch.pl happy but they are not beautiful. > --- a/drivers/staging/keucr/init.c > +++ b/drivers/staging/keucr/init.c > @@ -98,11 +98,16 @@ int ENE_SMInit(struct us_data *us) > us->SM_CardID = buf[2]; > > if (us->SM_Status.Insert && us->SM_Status.Ready) { The better way to solve this is: - dev_info(>pusb_dev->dev, "Insert = %x\n", us->SM_Status.Insert); + dev_info(us_dev, "Insert = %x\n", us->SM_Status.Insert); > - dev_info(>pusb_dev->dev, "Ready = %x\n", > us->SM_Status.Ready); > - dev_info(>pusb_dev->dev, "WtP= %x\n", > us->SM_Status.WtP); > - dev_info(>pusb_dev->dev, "DeviceID = %x\n", > us->SM_DeviceID); > - dev_info(>pusb_dev->dev, "CardID = %x\n", > us->SM_CardID); > + dev_info(>pusb_dev->dev, "Insert = %x\n", > + us->SM_Status.Insert); > + dev_info(>pusb_dev->dev, "Ready = %x\n", > + us->SM_Status.Ready); > + dev_info(>pusb_dev->dev, "WtP= %x\n", > + us->SM_Status.WtP); > + dev_info(>pusb_dev->dev, "DeviceID = %x\n", > + us->SM_DeviceID); > + dev_info(>pusb_dev->dev, "CardID = %x\n", > + us->SM_CardID); > MediaChange = 1; > Check_D_MediaFmt(us); > } else { > @@ -174,7 +179,8 @@ int ENE_SendScsiCmd(struct us_data *us, BYTE fDir, void > *buf, int use_sg) > result = usb_stor_bulk_transfer_buf(us, us->send_bulk_pipe, > bcb, US_BULK_CB_WRAP_LEN, NULL); > if (result != USB_STOR_XFER_GOOD) { > - dev_err(>pusb_dev->dev, "send cmd to out endpoint fail > ---\n"); > + dev_err(>pusb_dev->dev, > + "send cmd to out endpoint fail ---\n"); Line the parameters up: + dev_info(>pusb_dev->dev, +"my extra long meagesfasdfadf ---\n"); I used 3 tabs and a space to make everything line up correctly. But if you use a temporary variable this will fit on one line. > @@ -604,9 +604,9 @@ static int eucr_probe(struct usb_interface *intf, > if (!(MiscReg03 & 0x02)) { > result = -ENODEV; > quiesce_and_remove_host(us); > - pr_info("keucr: The driver only supports SM/MS card.\ > - To use SD card, \ > - please build driver/usb/storage/ums-eneub6250.ko\n"); > + pr_info("keucr: The driver only supports SM/MS card. " > + "To use SD card, " > + "please build driver/usb/storage/ums-eneub6250.ko\n"); > goto BadDevice; > } > This is a bug fix. It should go in as a separate patch. regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 6/6] keucr: fix some alignment- and whitespace-problems
On Thu, 2013-06-06 at 18:10 +0200, Johannes Schilling wrote: > resolves checkpatch errors and warnings regarding whitespace around > operators, line lengths and indentation. I suggest adding --strict to your checkpatch runs to report a few more style usage elements. > diff --git a/drivers/staging/keucr/init.c b/drivers/staging/keucr/init.c [] > @@ -98,11 +98,16 @@ int ENE_SMInit(struct us_data *us) [] > if (us->SM_Status.Insert && us->SM_Status.Ready) { > - dev_info(>pusb_dev->dev, "Insert = %x\n", > us->SM_Status.Insert); [] > + dev_info(>pusb_dev->dev, "Insert = %x\n", > + us->SM_Status.Insert); I think this would be nicer aligning the arguments to the open parenthesis like: dev_info(>pusb_dev->dev, "Insert = %x\n", us->SM_Status.Insert); but using us_info(us, "Insert = %x\n", us->SM_Status.Insert); would be nicer still and fit 80 cols, etc... Another option would be to use a macro like: #define us_show_status(us, field) \ us_info(us, "%-11s= %x\n", #field, us->SM_Status.field) And these become us_show_status(us, "Insert"); us_show_status(us, "Ready"); us_show_status(us, "WtP"); etc... It depends on how many of these actually exist whether or not a macro is appropriate. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 6/6] keucr: fix some alignment- and whitespace-problems
resolves checkpatch errors and warnings regarding whitespace around operators, line lengths and indentation. Signed-off-by: Laura Lawniczak Signed-off-by: Johannes Schilling --- drivers/staging/keucr/init.c | 24 +--- drivers/staging/keucr/scsiglue.c |5 +- drivers/staging/keucr/smilmain.c | 122 ++--- drivers/staging/keucr/transport.c | 31 ++ drivers/staging/keucr/usb.c | 10 +-- drivers/staging/keucr/usb.h | 12 ++-- 6 files changed, 106 insertions(+), 98 deletions(-) diff --git a/drivers/staging/keucr/init.c b/drivers/staging/keucr/init.c index 53478e1..f5d41e0 100644 --- a/drivers/staging/keucr/init.c +++ b/drivers/staging/keucr/init.c @@ -98,11 +98,16 @@ int ENE_SMInit(struct us_data *us) us->SM_CardID = buf[2]; if (us->SM_Status.Insert && us->SM_Status.Ready) { - dev_info(>pusb_dev->dev, "Insert = %x\n", us->SM_Status.Insert); - dev_info(>pusb_dev->dev, "Ready = %x\n", us->SM_Status.Ready); - dev_info(>pusb_dev->dev, "WtP= %x\n", us->SM_Status.WtP); - dev_info(>pusb_dev->dev, "DeviceID = %x\n", us->SM_DeviceID); - dev_info(>pusb_dev->dev, "CardID = %x\n", us->SM_CardID); + dev_info(>pusb_dev->dev, "Insert = %x\n", +us->SM_Status.Insert); + dev_info(>pusb_dev->dev, "Ready = %x\n", +us->SM_Status.Ready); + dev_info(>pusb_dev->dev, "WtP= %x\n", +us->SM_Status.WtP); + dev_info(>pusb_dev->dev, "DeviceID = %x\n", +us->SM_DeviceID); + dev_info(>pusb_dev->dev, "CardID = %x\n", +us->SM_CardID); MediaChange = 1; Check_D_MediaFmt(us); } else { @@ -174,7 +179,8 @@ int ENE_SendScsiCmd(struct us_data *us, BYTE fDir, void *buf, int use_sg) result = usb_stor_bulk_transfer_buf(us, us->send_bulk_pipe, bcb, US_BULK_CB_WRAP_LEN, NULL); if (result != USB_STOR_XFER_GOOD) { - dev_err(>pusb_dev->dev, "send cmd to out endpoint fail ---\n"); + dev_err(>pusb_dev->dev, + "send cmd to out endpoint fail ---\n"); return USB_STOR_TRANSPORT_ERROR; } @@ -203,14 +209,16 @@ int ENE_SendScsiCmd(struct us_data *us, BYTE fDir, void *buf, int use_sg) US_BULK_CS_WRAP_LEN, ); if (result == USB_STOR_XFER_SHORT && cswlen == 0) { - dev_warn(>pusb_dev->dev, "Received 0-length CSW; retrying...\n"); + dev_warn(>pusb_dev->dev, + "Received 0-length CSW; retrying...\n"); result = usb_stor_bulk_transfer_buf(us, us->recv_bulk_pipe, bcs, US_BULK_CS_WRAP_LEN, ); } if (result == USB_STOR_XFER_STALLED) { /* get the status again */ - dev_warn(>pusb_dev->dev, "Attempting to get CSW (2nd try)...\n"); + dev_warn(>pusb_dev->dev, + "Attempting to get CSW (2nd try)...\n"); result = usb_stor_bulk_transfer_buf(us, us->recv_bulk_pipe, bcs, US_BULK_CS_WRAP_LEN, NULL); } diff --git a/drivers/staging/keucr/scsiglue.c b/drivers/staging/keucr/scsiglue.c index 54a89ff..afb00d8 100644 --- a/drivers/staging/keucr/scsiglue.c +++ b/drivers/staging/keucr/scsiglue.c @@ -322,8 +322,9 @@ static ssize_t store_max_sectors(struct device *dev, static DEVICE_ATTR(max_sectors, S_IRUGO | S_IWUSR, show_max_sectors, store_max_sectors); -static struct device_attribute *sysfs_device_attr_list[] = - {_attr_max_sectors, NULL, }; +static struct device_attribute *sysfs_device_attr_list[] = { + _attr_max_sectors, NULL, +}; /* this defines our host template, with which we'll allocate hosts */ diff --git a/drivers/staging/keucr/smilmain.c b/drivers/staging/keucr/smilmain.c index dd73f88..2786808 100644 --- a/drivers/staging/keucr/smilmain.c +++ b/drivers/staging/keucr/smilmain.c @@ -302,7 +302,7 @@ int Media_D_ReadOneSect(struct us_data *us, WORD count, BYTE *buf) return ERROR; #ifdef RDERR_REASSIGN - if (Ssfdc.Attribute ) { + if (Ssfdc.Attribute & MWP) { if (ErrCode == ERR_CorReadErr) return SMSUCCESS; return ERROR; @@ -675,90 +675,84 @@ int Make_D_LogTable(struct us_data *us) Media.Sector = 0; - { - /* pr_info("Make_D_LogTable --- MediaZone = 0x%x\n", -
[PATCH 6/6] keucr: fix some alignment- and whitespace-problems
resolves checkpatch errors and warnings regarding whitespace around operators, line lengths and indentation. Signed-off-by: Laura Lawniczak laura.lawnic...@googlemail.com Signed-off-by: Johannes Schilling of82e...@cip.cs.fau.de --- drivers/staging/keucr/init.c | 24 +--- drivers/staging/keucr/scsiglue.c |5 +- drivers/staging/keucr/smilmain.c | 122 ++--- drivers/staging/keucr/transport.c | 31 ++ drivers/staging/keucr/usb.c | 10 +-- drivers/staging/keucr/usb.h | 12 ++-- 6 files changed, 106 insertions(+), 98 deletions(-) diff --git a/drivers/staging/keucr/init.c b/drivers/staging/keucr/init.c index 53478e1..f5d41e0 100644 --- a/drivers/staging/keucr/init.c +++ b/drivers/staging/keucr/init.c @@ -98,11 +98,16 @@ int ENE_SMInit(struct us_data *us) us-SM_CardID = buf[2]; if (us-SM_Status.Insert us-SM_Status.Ready) { - dev_info(us-pusb_dev-dev, Insert = %x\n, us-SM_Status.Insert); - dev_info(us-pusb_dev-dev, Ready = %x\n, us-SM_Status.Ready); - dev_info(us-pusb_dev-dev, WtP= %x\n, us-SM_Status.WtP); - dev_info(us-pusb_dev-dev, DeviceID = %x\n, us-SM_DeviceID); - dev_info(us-pusb_dev-dev, CardID = %x\n, us-SM_CardID); + dev_info(us-pusb_dev-dev, Insert = %x\n, +us-SM_Status.Insert); + dev_info(us-pusb_dev-dev, Ready = %x\n, +us-SM_Status.Ready); + dev_info(us-pusb_dev-dev, WtP= %x\n, +us-SM_Status.WtP); + dev_info(us-pusb_dev-dev, DeviceID = %x\n, +us-SM_DeviceID); + dev_info(us-pusb_dev-dev, CardID = %x\n, +us-SM_CardID); MediaChange = 1; Check_D_MediaFmt(us); } else { @@ -174,7 +179,8 @@ int ENE_SendScsiCmd(struct us_data *us, BYTE fDir, void *buf, int use_sg) result = usb_stor_bulk_transfer_buf(us, us-send_bulk_pipe, bcb, US_BULK_CB_WRAP_LEN, NULL); if (result != USB_STOR_XFER_GOOD) { - dev_err(us-pusb_dev-dev, send cmd to out endpoint fail ---\n); + dev_err(us-pusb_dev-dev, + send cmd to out endpoint fail ---\n); return USB_STOR_TRANSPORT_ERROR; } @@ -203,14 +209,16 @@ int ENE_SendScsiCmd(struct us_data *us, BYTE fDir, void *buf, int use_sg) US_BULK_CS_WRAP_LEN, cswlen); if (result == USB_STOR_XFER_SHORT cswlen == 0) { - dev_warn(us-pusb_dev-dev, Received 0-length CSW; retrying...\n); + dev_warn(us-pusb_dev-dev, + Received 0-length CSW; retrying...\n); result = usb_stor_bulk_transfer_buf(us, us-recv_bulk_pipe, bcs, US_BULK_CS_WRAP_LEN, cswlen); } if (result == USB_STOR_XFER_STALLED) { /* get the status again */ - dev_warn(us-pusb_dev-dev, Attempting to get CSW (2nd try)...\n); + dev_warn(us-pusb_dev-dev, + Attempting to get CSW (2nd try)...\n); result = usb_stor_bulk_transfer_buf(us, us-recv_bulk_pipe, bcs, US_BULK_CS_WRAP_LEN, NULL); } diff --git a/drivers/staging/keucr/scsiglue.c b/drivers/staging/keucr/scsiglue.c index 54a89ff..afb00d8 100644 --- a/drivers/staging/keucr/scsiglue.c +++ b/drivers/staging/keucr/scsiglue.c @@ -322,8 +322,9 @@ static ssize_t store_max_sectors(struct device *dev, static DEVICE_ATTR(max_sectors, S_IRUGO | S_IWUSR, show_max_sectors, store_max_sectors); -static struct device_attribute *sysfs_device_attr_list[] = - {dev_attr_max_sectors, NULL, }; +static struct device_attribute *sysfs_device_attr_list[] = { + dev_attr_max_sectors, NULL, +}; /* this defines our host template, with which we'll allocate hosts */ diff --git a/drivers/staging/keucr/smilmain.c b/drivers/staging/keucr/smilmain.c index dd73f88..2786808 100644 --- a/drivers/staging/keucr/smilmain.c +++ b/drivers/staging/keucr/smilmain.c @@ -302,7 +302,7 @@ int Media_D_ReadOneSect(struct us_data *us, WORD count, BYTE *buf) return ERROR; #ifdef RDERR_REASSIGN - if (Ssfdc.Attribute MWP) { + if (Ssfdc.Attribute MWP) { if (ErrCode == ERR_CorReadErr) return SMSUCCESS; return ERROR; @@ -675,90 +675,84 @@ int Make_D_LogTable(struct us_data *us) Media.Sector = 0; - { - /*
Re: [PATCH 6/6] keucr: fix some alignment- and whitespace-problems
On Thu, 2013-06-06 at 18:10 +0200, Johannes Schilling wrote: resolves checkpatch errors and warnings regarding whitespace around operators, line lengths and indentation. I suggest adding --strict to your checkpatch runs to report a few more style usage elements. diff --git a/drivers/staging/keucr/init.c b/drivers/staging/keucr/init.c [] @@ -98,11 +98,16 @@ int ENE_SMInit(struct us_data *us) [] if (us-SM_Status.Insert us-SM_Status.Ready) { - dev_info(us-pusb_dev-dev, Insert = %x\n, us-SM_Status.Insert); [] + dev_info(us-pusb_dev-dev, Insert = %x\n, + us-SM_Status.Insert); I think this would be nicer aligning the arguments to the open parenthesis like: dev_info(us-pusb_dev-dev, Insert = %x\n, us-SM_Status.Insert); but using us_info(us, Insert = %x\n, us-SM_Status.Insert); would be nicer still and fit 80 cols, etc... Another option would be to use a macro like: #define us_show_status(us, field) \ us_info(us, %-11s= %x\n, #field, us-SM_Status.field) And these become us_show_status(us, Insert); us_show_status(us, Ready); us_show_status(us, WtP); etc... It depends on how many of these actually exist whether or not a macro is appropriate. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 6/6] keucr: fix some alignment- and whitespace-problems
On Thu, Jun 06, 2013 at 06:10:50PM +0200, Johannes Schilling wrote: resolves checkpatch errors and warnings regarding whitespace around operators, line lengths and indentation. I feel like this should be broken up into several patches. A lot of these changes will make checkpatch.pl happy but they are not beautiful. --- a/drivers/staging/keucr/init.c +++ b/drivers/staging/keucr/init.c @@ -98,11 +98,16 @@ int ENE_SMInit(struct us_data *us) us-SM_CardID = buf[2]; if (us-SM_Status.Insert us-SM_Status.Ready) { The better way to solve this is: - dev_info(us-pusb_dev-dev, Insert = %x\n, us-SM_Status.Insert); + dev_info(us_dev, Insert = %x\n, us-SM_Status.Insert); - dev_info(us-pusb_dev-dev, Ready = %x\n, us-SM_Status.Ready); - dev_info(us-pusb_dev-dev, WtP= %x\n, us-SM_Status.WtP); - dev_info(us-pusb_dev-dev, DeviceID = %x\n, us-SM_DeviceID); - dev_info(us-pusb_dev-dev, CardID = %x\n, us-SM_CardID); + dev_info(us-pusb_dev-dev, Insert = %x\n, + us-SM_Status.Insert); + dev_info(us-pusb_dev-dev, Ready = %x\n, + us-SM_Status.Ready); + dev_info(us-pusb_dev-dev, WtP= %x\n, + us-SM_Status.WtP); + dev_info(us-pusb_dev-dev, DeviceID = %x\n, + us-SM_DeviceID); + dev_info(us-pusb_dev-dev, CardID = %x\n, + us-SM_CardID); MediaChange = 1; Check_D_MediaFmt(us); } else { @@ -174,7 +179,8 @@ int ENE_SendScsiCmd(struct us_data *us, BYTE fDir, void *buf, int use_sg) result = usb_stor_bulk_transfer_buf(us, us-send_bulk_pipe, bcb, US_BULK_CB_WRAP_LEN, NULL); if (result != USB_STOR_XFER_GOOD) { - dev_err(us-pusb_dev-dev, send cmd to out endpoint fail ---\n); + dev_err(us-pusb_dev-dev, + send cmd to out endpoint fail ---\n); Line the parameters up: + dev_info(us-pusb_dev-dev, +my extra long meagesfasdfadf ---\n); I used 3 tabs and a space to make everything line up correctly. But if you use a temporary variable this will fit on one line. @@ -604,9 +604,9 @@ static int eucr_probe(struct usb_interface *intf, if (!(MiscReg03 0x02)) { result = -ENODEV; quiesce_and_remove_host(us); - pr_info(keucr: The driver only supports SM/MS card.\ - To use SD card, \ - please build driver/usb/storage/ums-eneub6250.ko\n); + pr_info(keucr: The driver only supports SM/MS card. + To use SD card, + please build driver/usb/storage/ums-eneub6250.ko\n); goto BadDevice; } This is a bug fix. It should go in as a separate patch. regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/