[OPW,kernel] Staging: Fix line length exceeding 80 characters
diff mbox

Message ID 20131005115529.GA3918@gmail.com
State Changes Requested
Headers show

Commit Message

Rashika Oct. 5, 2013, 11:55 a.m. UTC
Signed-off-by: Rashika <rashika.kheria@gmail.com>
---
 drivers/staging/btmtk_usb/btmtk_usb.c |  137 +++++++++++++++++++++------------
 1 file changed, 86 insertions(+), 51 deletions(-)

Comments

Xenia Ragiadakou Oct. 5, 2013, 1:34 p.m. UTC | #1
Hi Rashika,

I noticed that you corrected the subject line by yourself. Good job!
It would be nice to add the name of the driver on the subject line too, 
so that somebody will can infer quickly to which driver this patch 
applies. For example, you can write in the future:
Staging: btmtk_usb: Fix line length exceeding 80 characters

Also you need to add a changelog in your patch, which will state why you 
did that change and which tool you used to find the bad coding style (if 
you used one) for example:
This patch fixes the following checkpatch.pl warning/error: <warning/error>

I have added some other comments below.

On 10/05/2013 02:55 PM, Rashika wrote:
> Signed-off-by: Rashika <rashika.kheria@gmail.com>
> ---
>   drivers/staging/btmtk_usb/btmtk_usb.c |  137 +++++++++++++++++++++------------
>   1 file changed, 86 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/staging/btmtk_usb/btmtk_usb.c b/drivers/staging/btmtk_usb/btmtk_usb.c
> index 0e783e8..0b3e593 100644
> --- a/drivers/staging/btmtk_usb/btmtk_usb.c
> +++ b/drivers/staging/btmtk_usb/btmtk_usb.c
> @@ -16,7 +16,8 @@
>    *  You should have received a copy of the GNU General Public License
>    *  along with this program; if not, write to the Free Software
>    *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
> - *  or on the worldwide web at http://www.gnu.org/licenses/old-licenses/gpl-2.0.txt.
> + *  or on the worldwide web at http://www.gnu.org/licenses/old-licenses/
> + *  gpl-2.0.txt.
>    *
>    */
>   

Maybe here is not a good idea to break the link. You could move all of 
it in the next line or leave it as is.

> @@ -72,8 +73,9 @@ static int btmtk_usb_reset(struct usb_device *udev)
>   
>   	BT_DBG("%s\n", __func__);
>   
> -	ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), 0x01, DEVICE_VENDOR_REQUEST_OUT,
> -						  0x01, 0x00, NULL, 0x00, CONTROL_TIMEOUT_JIFFIES);
> +	ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), 0x01,
> +			DEVICE_VENDOR_REQUEST_OUT, 0x01, 0x00, NULL, 0x00,
> +			CONTROL_TIMEOUT_JIFFIES);
>   
>   	if (ret < 0) {
>   		BT_ERR("%s error(%d)\n", __func__, ret);
> @@ -92,13 +94,14 @@ static int btmtk_usb_io_read32(struct btmtk_usb_data *data, u32 reg, u32 *val)
>   	struct usb_device *udev = data->udev;
>   	int ret;
>   
> -	ret = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0), request, DEVICE_VENDOR_REQUEST_IN,
> -						  0x0, reg, data->io_buf, 4,
> +	ret = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0), request,
> +			DEVICE_VENDOR_REQUEST_IN, 0x0, reg, data->io_buf, 4,
>   						  CONTROL_TIMEOUT_JIFFIES);
>   

Try that your changes lead to uniform identation, that means the above 
could be:

> +	ret = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0), request,
> +			DEVICE_VENDOR_REQUEST_IN, 0x0, reg, data->io_buf, 4,
> + 			CONTROL_TIMEOUT_JIFFIES);


>   	if (ret < 0) {
>   		*val = 0xffffffff;
> -		BT_ERR("%s error(%d), reg=%x, value=%x\n", __func__, ret, reg, *val);
> +		BT_ERR("%s error(%d), reg=%x, value=%x\n", __func__, ret, reg,
> +				*val);
>   		return ret;
>   	}
>   
> @@ -122,12 +125,13 @@ static int btmtk_usb_io_write32(struct btmtk_usb_data *data, u32 reg, u32 val)
>   	index = (u16)reg;
>   	value = val & 0x0000ffff;
>   
> -	ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), request, DEVICE_VENDOR_REQUEST_OUT,
> -						  value, index, NULL, 0,
> +	ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), request,
> +			DEVICE_VENDOR_REQUEST_OUT, value, index, NULL, 0,
>   						  CONTROL_TIMEOUT_JIFFIES);
>   

The same here as far as concerns the identation.

>   	if (ret < 0) {
> -		BT_ERR("%s error(%d), reg=%x, value=%x\n", __func__, ret, reg, val);
> +		BT_ERR("%s error(%d), reg=%x, value=%x\n", __func__, ret, reg,
> +				val);
>   		return ret;
>   	}
>   
> @@ -139,7 +143,8 @@ static int btmtk_usb_io_write32(struct btmtk_usb_data *data, u32 reg, u32 val)
>   				value, index, NULL, 0, CONTROL_TIMEOUT_JIFFIES);
>   
>   	if (ret < 0) {
> -		BT_ERR("%s error(%d), reg=%x, value=%x\n", __func__, ret, reg, val);
> +		BT_ERR("%s error(%d), reg=%x, value=%x\n", __func__, ret, reg,
> +				val);
>   		return ret;
>   	}
>   
> @@ -186,13 +191,16 @@ static void btmtk_usb_cap_init(struct btmtk_usb_data *data)
>   		ret = request_firmware(&firmware, MT7650_FIRMWARE, &udev->dev);
>   		if (ret < 0) {
>   			if (ret == -ENOENT) {
> -				BT_ERR("Firmware file \"%s\" not found \n", MT7650_FIRMWARE);
> +				BT_ERR("Firmware file \"%s\" not found \n",
> +						MT7650_FIRMWARE);
>   			} else {
> -				BT_ERR("Firmware file \"%s\" request failed (err=%d) \n",
> -					MT7650_FIRMWARE, ret);
> +				BT_ERR("Firmware file \"%s\" request failed
> +						(err=%d) \n", MT7650_FIRMWARE,
> +	

Here, by breaking the string you introduced another checkpatch warning. 
Better leave it as it is.
Also, if ever you need to break a line do:

+				BT_ERR("Firmware file \"%s\" request failed"
+						"(err=%d) \n", MT7650_FIRMWARE,


> 					ret);
>   			}
>   		} else {
> -			BT_DBG("Firmware file \"%s\" Found \n", MT7650_FIRMWARE);
> +			BT_DBG("Firmware file \"%s\" Found \n",
> +					MT7650_FIRMWARE);
>   			/* load firmware here */
>   			data->firmware = firmware;
>   			btmtk_usb_load_fw(data);
> @@ -205,10 +213,12 @@ static void btmtk_usb_cap_init(struct btmtk_usb_data *data)
>   		ret = request_firmware(&firmware, MT7662_FIRMWARE, &udev->dev);
>   		if (ret < 0) {
>   			if (ret == -ENOENT) {
> -				BT_ERR("Firmware file \"%s\" not found\n", MT7662_FIRMWARE);
> +				BT_ERR("Firmware file \"%s\" not found\n",
> +						MT7662_FIRMWARE);
>   			} else {
> -				BT_ERR("Firmware file \"%s\" request failed (err=%d)\n",
> -					MT7662_FIRMWARE, ret);
> +				BT_ERR("Firmware file \"%s\" request failed
> +						(err=%d)\n", MT7662_FIRMWARE,
> +	

Here also you broke the string.

> 					ret);
>   			}
>   		} else {
>   		    BT_DBG("Firmware file \"%s\" Found\n", MT7662_FIRMWARE);
> @@ -258,8 +268,8 @@ static int btmtk_usb_chk_crc(struct btmtk_usb_data *data, u32 checksum_len)
>   	memmove(data->io_buf, &data->rom_patch_offset, 4);
>   	memmove(&data->io_buf[4], &checksum_len, 4);
>   
> -	ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), 0x1, DEVICE_VENDOR_REQUEST_IN,
> -						  0x20, 0x00, data->io_buf, 8,
> +	ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), 0x1,
> +			DEVICE_VENDOR_REQUEST_IN, 0x20, 0x00, data->io_buf, 8,
>   						  CONTROL_TIMEOUT_JIFFIES);

Here you need to align it better, so that it can be more readable.

>   
>   	if (ret < 0) {
> @@ -318,8 +328,8 @@ static int btmtk_usb_reset_wmt(struct btmtk_usb_data *data)
>   	BT_DBG("%s\n", __func__);
>   
>   	ret = usb_control_msg(data->udev, usb_sndctrlpipe(data->udev, 0), 0x01,
> -				DEVICE_CLASS_REQUEST_OUT, 0x12, 0x00, data->io_buf,
> -				8, CONTROL_TIMEOUT_JIFFIES);
> +				DEVICE_CLASS_REQUEST_OUT, 0x12, 0x00,
> +				data->io_buf, 8, CONTROL_TIMEOUT_JIFFIES);
>   
>   	if (ret)
>   		BT_ERR("%s:(%d)\n", __func__, ret);
> @@ -350,7 +360,8 @@ static int btmtk_usb_load_rom_patch(struct btmtk_usb_data *data)
>   	unsigned char phase;
>   	void *buf;
>   	char *pos;
> -	unsigned int pipe = usb_sndbulkpipe(data->udev, data->bulk_tx_ep->bEndpointAddress);
> +	unsigned int pipe = usb_sndbulkpipe(data->udev,
> +			data->bulk_tx_ep->bEndpointAddress);
>   
>   	if (!data->firmware) {
>   		BT_ERR("%s:please assign a rom patch\n", __func__);
> @@ -391,7 +402,8 @@ load_patch_protect:
>   		goto error0;
>   	}
>   
> -	buf = usb_alloc_coherent(data->udev, UPLOAD_PATCH_UNIT, GFP_ATOMIC, &data_dma);
> +	buf = usb_alloc_coherent(data->udev, UPLOAD_PATCH_UNIT, GFP_ATOMIC,
> +			&data_dma);
>   
>   	if (!buf) {
>   		ret = -ENOMEM;
> @@ -409,7 +421,8 @@ load_patch_protect:
>   	/* loading rom patch */
>   	while (1) {
>   		s32 sent_len_max = UPLOAD_PATCH_UNIT - PATCH_HEADER_SIZE;
> -		sent_len = (patch_len - cur_len) >= sent_len_max ? sent_len_max : (patch_len - cur_len);
> +		sent_len = (patch_len - cur_len) >= sent_len_max ? sent_len_max
> +			: (patch_len - cur_len);

Maybe here it would be more readable if you leave ':' in the previous 
line. As far as i have noticed there is a tendency to leave the 
operators in the previous lines.

>   
>   		BT_DBG("patch_len = %d\n", patch_len);
>   		BT_DBG("cur_len = %d\n", cur_len);
> @@ -442,10 +455,12 @@ load_patch_protect:
>   
>   			pos[8] = phase;
>   
> -			memcpy(&pos[9], data->firmware->data + PATCH_INFO_SIZE + cur_len, sent_len);
> +			memcpy(&pos[9], data->firmware->data + PATCH_INFO_SIZE
> +					+ cur_len, sent_len);
>   
> -			BT_DBG("sent_len + PATCH_HEADER_SIZE = %d, phase = %d\n",
> -					sent_len + PATCH_HEADER_SIZE, phase);
> +			BT_DBG("sent_len + PATCH_HEADER_SIZE = %d,
> +					phase = %d\n", sent_len +
> +					PATCH_HEADER_SIZE, phase);
>   

Here, you also broke the string.

>   			usb_fill_bulk_urb(urb,
>   					data->udev,
> @@ -463,7 +478,8 @@ load_patch_protect:
>   			if (ret)
>   				goto error2;
>   
> -			if (!wait_for_completion_timeout(&sent_to_mcu_done, msecs_to_jiffies(1000))) {
> +			if (!wait_for_completion_timeout(&sent_to_mcu_done,
> +						msecs_to_jiffies(1000))) {
>   				usb_kill_urb(urb);
>   				BT_ERR("upload rom_patch timeout\n");
>   				goto error2;
> @@ -480,7 +496,8 @@ load_patch_protect:
>   		}
>   	}
>   
> -	total_checksum = checksume16((u8 *)data->firmware->data + PATCH_INFO_SIZE, patch_len);
> +	total_checksum = checksume16((u8 *)data->firmware->data +
> +			PATCH_INFO_SIZE, patch_len);
>   
>   	BT_DBG("Send checksum req..\n");
>   
> @@ -520,7 +537,8 @@ static int load_fw_iv(struct btmtk_usb_data *data)
>   	memmove(buf, data->firmware->data + 32, 64);
>   
>   	ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), 0x01,
> -						  DEVICE_VENDOR_REQUEST_OUT, 0x12, 0x0, buf, 64,
> +						  DEVICE_VENDOR_REQUEST_OUT,
> +						  0x12, 0x0, buf, 64,
>   						  CONTROL_TIMEOUT_JIFFIES);
>   
>   	if (ret < 0) {
> @@ -559,7 +577,8 @@ static int btmtk_usb_load_fw(struct btmtk_usb_data *data)
>   	dma_addr_t data_dma;
>   	int ret = 0, sent_len;
>   	struct completion sent_to_mcu_done;
> -	unsigned int pipe = usb_sndbulkpipe(data->udev, data->bulk_tx_ep->bEndpointAddress);
> +	unsigned int pipe = usb_sndbulkpipe(data->udev,
> +			data->bulk_tx_ep->bEndpointAddress);
>   
>   	if (!data->firmware) {
>   		BT_ERR("%s:please assign a fw\n", __func__);
> @@ -598,9 +617,11 @@ loadfw_protect:
>   			| (*(data->firmware->data + 5) << 8)
>   			| (*(data->firmware->data + 4));
>   
> -	fw_ver = (*(data->firmware->data + 11) << 8) | (*(data->firmware->data + 10));
> +	fw_ver = (*(data->firmware->data + 11) << 8) |
> +		(*(data->firmware->data + 10));
>   
> -	build_ver = (*(data->firmware->data + 9) << 8) | (*(data->firmware->data + 8));
> +	build_ver = (*(data->firmware->data + 9) << 8) |
> +		(*(data->firmware->data + 8));

The above would more readable if it was:
> +	build_ver = (*(data->firmware->data + 9) << 8) |
> +		    (*(data->firmware->data + 8));
That would be the result if you press tab to align (actually i don't 
know how your editor is setup).

>   
>   	BT_DBG("fw version:%d.%d.%02d ",
>   			(fw_ver & 0xf000) >> 8,
> @@ -657,7 +678,8 @@ loadfw_protect:
>   
>   	/* Loading ILM */
>   	while (1) {
> -		sent_len = (ilm_len - cur_len) >= 14336 ? 14336 : (ilm_len - cur_len);
> +		sent_len = (ilm_len - cur_len) >= 14336 ? 14336
> +			: (ilm_len - cur_len);
>   

Here, better place ':' in the line above.

>   		if (sent_len > 0) {
>   			packet_header &= ~(0xffffffff);
> @@ -665,7 +687,9 @@ loadfw_protect:
>   			packet_header = cpu_to_le32(packet_header);
>   
>   			memmove(buf, &packet_header, 4);
> -			memmove(buf + 4, data->firmware->data + 32 + cur_len, sent_len);
> +			memmove(buf + 4, data->firmware->data + 32 + cur_len,
> +					sent_len);
> +
>   
>   			/* U2M_PDMA descriptor */
>   			btmtk_usb_io_write32(data, 0x230, cur_len);
> @@ -693,7 +717,8 @@ loadfw_protect:
>   			if (ret)
>   				goto error3;
>   
> -			if (!wait_for_completion_timeout(&sent_to_mcu_done, msecs_to_jiffies(1000))) {
> +			if (!wait_for_completion_timeout(&sent_to_mcu_done,
> +						msecs_to_jiffies(1000))) {
>   				usb_kill_urb(urb);
>   				BT_ERR("upload ilm fw timeout\n");
>   				goto error3;
> @@ -714,7 +739,8 @@ loadfw_protect:
>   
>   	/* Loading DLM */
>   	while (1) {
> -		sent_len = (dlm_len - cur_len) >= 14336 ? 14336 : (dlm_len - cur_len);
> +		sent_len = (dlm_len - cur_len) >= 14336 ? 14336
> +			: (dlm_len - cur_len);
>   
>   		if (sent_len > 0) {
>   			packet_header &= ~(0xffffffff);
> @@ -722,7 +748,8 @@ loadfw_protect:
>   			packet_header = cpu_to_le32(packet_header);
>   
>   			memmove(buf, &packet_header, 4);
> -			memmove(buf + 4, data->firmware->data + 32 + ilm_len + cur_len, sent_len);
> +			memmove(buf + 4, data->firmware->data + 32 + ilm_len
> +					+ cur_len, sent_len);
>   

Here, also leave the operator '+' in the previous line.

>   			/* U2M_PDMA descriptor */
>   			btmtk_usb_io_write32(data, 0x230, 0x80000 + cur_len);
> @@ -751,7 +778,8 @@ loadfw_protect:
>   			if (ret)
>   				goto error3;
>   
> -			if (!wait_for_completion_timeout(&sent_to_mcu_done, msecs_to_jiffies(1000))) {
> +			if (!wait_for_completion_timeout(&sent_to_mcu_done,
> +						msecs_to_jiffies(1000))) {
>   				usb_kill_urb(urb);
>   				BT_ERR("upload dlm fw timeout\n");
>   				goto error3;
> @@ -978,8 +1006,8 @@ static int btmtk_usb_submit_bulk_in_urb(struct hci_dev *hdev, gfp_t mem_flags)
>   
>   	pipe = usb_rcvbulkpipe(data->udev, data->bulk_rx_ep->bEndpointAddress);
>   
> -	usb_fill_bulk_urb(urb, data->udev, pipe,
> -					buf, size, btmtk_usb_bulk_in_complete, hdev);
> +	usb_fill_bulk_urb(urb, data->udev, pipe, buf, size,
> +			btmtk_usb_bulk_in_complete, hdev);
>   
>   	urb->transfer_flags |= URB_FREE_BUFFER;
>   
> @@ -1015,7 +1043,8 @@ static void btmtk_usb_isoc_in_complete(struct urb *urb)
>   	if (urb->status == 0) {
>   		for (i = 0; i < urb->number_of_packets; i++) {
>   			unsigned int offset = urb->iso_frame_desc[i].offset;
> -			unsigned int length = urb->iso_frame_desc[i].actual_length;
> +			unsigned int length = urb->iso_frame_desc[i]
> +				.actual_length;

You cannot do that, you cannot separate the structure from its field. 
You can either leave it as it is or do:
> +			unsigned int length =
> +				 urb->iso_frame_desc[i].actual_length;


>   
>   			if (urb->iso_frame_desc[i].status)
>   				continue;
> @@ -1096,8 +1125,9 @@ static int btmtk_usb_submit_isoc_in_urb(struct hci_dev *hdev, gfp_t mem_flags)
>   
>   	pipe = usb_rcvisocpipe(data->udev, data->isoc_rx_ep->bEndpointAddress);
>   
> -	usb_fill_int_urb(urb, data->udev, pipe, buf, size, btmtk_usb_isoc_in_complete,
> -				hdev, data->isoc_rx_ep->bInterval);
> +	usb_fill_int_urb(urb, data->udev, pipe, buf, size,
> +			btmtk_usb_isoc_in_complete, hdev,
> +			data->isoc_rx_ep->bInterval);
>   
>   	urb->transfer_flags  = URB_FREE_BUFFER | URB_ISO_ASAP;
>   
> @@ -1306,7 +1336,8 @@ static int btmtk_usb_send_frame(struct sk_buff *skb)
>   		}
>   
>   		usb_fill_control_urb(urb, data->udev, pipe, (void *) dr,
> -				skb->data, skb->len, btmtk_usb_tx_complete, skb);
> +				skb->data, skb->len, btmtk_usb_tx_complete,
> +				skb);
>   
>   		hdev->stat.cmd_tx++;
>   		break;
> @@ -1322,8 +1353,8 @@ static int btmtk_usb_send_frame(struct sk_buff *skb)
>   		pipe = usb_sndbulkpipe(data->udev,
>   					data->bulk_tx_ep->bEndpointAddress);
>   
> -		usb_fill_bulk_urb(urb, data->udev, pipe,
> -				skb->data, skb->len, btmtk_usb_tx_complete, skb);
> +		usb_fill_bulk_urb(urb, data->udev, pipe, skb->data, skb->len,
> +				btmtk_usb_tx_complete, skb);
>   
>   		hdev->stat.acl_tx++;
>   		BT_DBG("HCI_ACLDATA_PKT:\n");
> @@ -1442,7 +1473,8 @@ static inline int __set_isoc_interface(struct hci_dev *hdev, int altsetting)
>   
>   static void btmtk_usb_work(struct work_struct *work)
>   {
> -	struct btmtk_usb_data *data = container_of(work, struct btmtk_usb_data, work);
> +	struct btmtk_usb_data *data = container_of(work, struct btmtk_usb_data,
> +			work);
>   	struct hci_dev *hdev = data->hdev;
>   	int new_alts;
>   	int err;
> @@ -1451,7 +1483,8 @@ static void btmtk_usb_work(struct work_struct *work)
>   
>   	if (hdev->conn_hash.sco_num > 0) {
>   		if (!test_bit(BTUSB_DID_ISO_RESUME, &data->flags)) {
> -			err = usb_autopm_get_interface(data->isoc ? data->isoc : data->intf);
> +			err = usb_autopm_get_interface(data->isoc ? data->isoc
> +					: data->intf);
>   			if (err < 0) {
>   				clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
>   				usb_kill_anchored_urbs(&data->isoc_anchor);
> @@ -1489,13 +1522,15 @@ static void btmtk_usb_work(struct work_struct *work)
>   		__set_isoc_interface(hdev, 0);
>   
>   		if (test_and_clear_bit(BTUSB_DID_ISO_RESUME, &data->flags))
> -			 usb_autopm_put_interface(data->isoc ? data->isoc : data->intf);
> +			 usb_autopm_put_interface(data->isoc ? data->isoc
> +					 : data->intf);
>   	}
>   }
>   
>   static void btmtk_usb_waker(struct work_struct *work)
>   {
> -	struct btmtk_usb_data *data = container_of(work, struct btmtk_usb_data, waker);
> +	struct btmtk_usb_data *data = container_of(work, struct btmtk_usb_data,
> +			waker);
>   	int err;
>   
>   	err = usb_autopm_get_interface(data->intf);

I have not commended them all but the idea is the same. Good work by the 
way :)
Keep in mind to change the code in order to be more readable without 
violating the kernel coding style (significantly).

ksenia
Rashika Oct. 5, 2013, 1:43 p.m. UTC | #2
Hi Xenia,

Thank you for your feedback.

I tried to incorporate the name of the driver in the subject line but since
the subject line could contain only 50 characters, therefore, I was unable
to add the same. Kindly guide me to rectify this issue.

I will upload another version to correcting the other suggested changes.

Thanks in advance.

Rashika Kheria


On Sat, Oct 5, 2013 at 7:04 PM, Xenia Ragiadakou <burzalodowa@gmail.com>wrote:

> Hi Rashika,
>
> I noticed that you corrected the subject line by yourself. Good job!
> It would be nice to add the name of the driver on the subject line too, so
> that somebody will can infer quickly to which driver this patch applies.
> For example, you can write in the future:
> Staging: btmtk_usb: Fix line length exceeding 80 characters
>
> Also you need to add a changelog in your patch, which will state why you
> did that change and which tool you used to find the bad coding style (if
> you used one) for example:
> This patch fixes the following checkpatch.pl warning/error:
> <warning/error>
>
> I have added some other comments below.
>
>
> On 10/05/2013 02:55 PM, Rashika wrote:
>
>> Signed-off-by: Rashika <rashika.kheria@gmail.com>
>> ---
>>   drivers/staging/btmtk_usb/**btmtk_usb.c |  137
>> +++++++++++++++++++++---------**---
>>   1 file changed, 86 insertions(+), 51 deletions(-)
>>
>> diff --git a/drivers/staging/btmtk_usb/**btmtk_usb.c
>> b/drivers/staging/btmtk_usb/**btmtk_usb.c
>> index 0e783e8..0b3e593 100644
>> --- a/drivers/staging/btmtk_usb/**btmtk_usb.c
>> +++ b/drivers/staging/btmtk_usb/**btmtk_usb.c
>> @@ -16,7 +16,8 @@
>>    *  You should have received a copy of the GNU General Public License
>>    *  along with this program; if not, write to the Free Software
>>    *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
>>  02111-1307  USA
>> - *  or on the worldwide web at http://www.gnu.org/licenses/**
>> old-licenses/gpl-2.0.txt<http://www.gnu.org/licenses/old-licenses/gpl-2.0.txt>
>> .
>> + *  or on the worldwide web at http://www.gnu.org/licenses/**
>> old-licenses/ <http://www.gnu.org/licenses/old-licenses/>
>> + *  gpl-2.0.txt.
>>    *
>>    */
>>
>>
>
> Maybe here is not a good idea to break the link. You could move all of it
> in the next line or leave it as is.
>
>
>  @@ -72,8 +73,9 @@ static int btmtk_usb_reset(struct usb_device *udev)
>>         BT_DBG("%s\n", __func__);
>>   -     ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), 0x01,
>> DEVICE_VENDOR_REQUEST_OUT,
>> -                                                 0x01, 0x00, NULL, 0x00,
>> CONTROL_TIMEOUT_JIFFIES);
>> +       ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), 0x01,
>> +                       DEVICE_VENDOR_REQUEST_OUT, 0x01, 0x00, NULL, 0x00,
>> +                       CONTROL_TIMEOUT_JIFFIES);
>>         if (ret < 0) {
>>                 BT_ERR("%s error(%d)\n", __func__, ret);
>> @@ -92,13 +94,14 @@ static int btmtk_usb_io_read32(struct btmtk_usb_data
>> *data, u32 reg, u32 *val)
>>         struct usb_device *udev = data->udev;
>>         int ret;
>>   -     ret = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0), request,
>> DEVICE_VENDOR_REQUEST_IN,
>> -                                                 0x0, reg, data->io_buf,
>> 4,
>> +       ret = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0), request,
>> +                       DEVICE_VENDOR_REQUEST_IN, 0x0, reg, data->io_buf,
>> 4,
>>
>> CONTROL_TIMEOUT_JIFFIES);
>>
>>
>
> Try that your changes lead to uniform identation, that means the above
> could be:
>
>  +       ret = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0), request,
>> +                       DEVICE_VENDOR_REQUEST_IN, 0x0, reg, data->io_buf,
>> 4,
>> +                       CONTROL_TIMEOUT_JIFFIES);
>>
>
>
>          if (ret < 0) {
>>
>>                 *val = 0xffffffff;
>> -               BT_ERR("%s error(%d), reg=%x, value=%x\n", __func__, ret,
>> reg, *val);
>> +               BT_ERR("%s error(%d), reg=%x, value=%x\n", __func__, ret,
>> reg,
>> +                               *val);
>>                 return ret;
>>         }
>>   @@ -122,12 +125,13 @@ static int btmtk_usb_io_write32(struct
>> btmtk_usb_data *data, u32 reg, u32 val)
>>         index = (u16)reg;
>>         value = val & 0x0000ffff;
>>   -     ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), request,
>> DEVICE_VENDOR_REQUEST_OUT,
>> -                                                 value, index, NULL, 0,
>> +       ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), request,
>> +                       DEVICE_VENDOR_REQUEST_OUT, value, index, NULL, 0,
>>
>> CONTROL_TIMEOUT_JIFFIES);
>>
>>
>
> The same here as far as concerns the identation.
>
>
>          if (ret < 0) {
>> -               BT_ERR("%s error(%d), reg=%x, value=%x\n", __func__, ret,
>> reg, val);
>> +               BT_ERR("%s error(%d), reg=%x, value=%x\n", __func__, ret,
>> reg,
>> +                               val);
>>                 return ret;
>>         }
>>   @@ -139,7 +143,8 @@ static int btmtk_usb_io_write32(struct
>> btmtk_usb_data *data, u32 reg, u32 val)
>>                                 value, index, NULL, 0,
>> CONTROL_TIMEOUT_JIFFIES);
>>         if (ret < 0) {
>> -               BT_ERR("%s error(%d), reg=%x, value=%x\n", __func__, ret,
>> reg, val);
>> +               BT_ERR("%s error(%d), reg=%x, value=%x\n", __func__, ret,
>> reg,
>> +                               val);
>>                 return ret;
>>         }
>>   @@ -186,13 +191,16 @@ static void btmtk_usb_cap_init(struct
>> btmtk_usb_data *data)
>>                 ret = request_firmware(&firmware, MT7650_FIRMWARE,
>> &udev->dev);
>>                 if (ret < 0) {
>>                         if (ret == -ENOENT) {
>> -                               BT_ERR("Firmware file \"%s\" not found
>> \n", MT7650_FIRMWARE);
>> +                               BT_ERR("Firmware file \"%s\" not found
>> \n",
>> +                                               MT7650_FIRMWARE);
>>                         } else {
>> -                               BT_ERR("Firmware file \"%s\" request
>> failed (err=%d) \n",
>> -                                       MT7650_FIRMWARE, ret);
>> +                               BT_ERR("Firmware file \"%s\" request
>> failed
>> +                                               (err=%d) \n",
>> MT7650_FIRMWARE,
>> +
>>
>
> Here, by breaking the string you introduced another checkpatch warning.
> Better leave it as it is.
> Also, if ever you need to break a line do:
>
>
> +                               BT_ERR("Firmware file \"%s\" request
> failed"
> +                                               "(err=%d) \n",
> MT7650_FIRMWARE,
>
>
>                                          ret);
>>                         }
>>                 } else {
>> -                       BT_DBG("Firmware file \"%s\" Found \n",
>> MT7650_FIRMWARE);
>> +                       BT_DBG("Firmware file \"%s\" Found \n",
>> +                                       MT7650_FIRMWARE);
>>                         /* load firmware here */
>>                         data->firmware = firmware;
>>                         btmtk_usb_load_fw(data);
>> @@ -205,10 +213,12 @@ static void btmtk_usb_cap_init(struct
>> btmtk_usb_data *data)
>>                 ret = request_firmware(&firmware, MT7662_FIRMWARE,
>> &udev->dev);
>>                 if (ret < 0) {
>>                         if (ret == -ENOENT) {
>> -                               BT_ERR("Firmware file \"%s\" not
>> found\n", MT7662_FIRMWARE);
>> +                               BT_ERR("Firmware file \"%s\" not found\n",
>> +                                               MT7662_FIRMWARE);
>>                         } else {
>> -                               BT_ERR("Firmware file \"%s\" request
>> failed (err=%d)\n",
>> -                                       MT7662_FIRMWARE, ret);
>> +                               BT_ERR("Firmware file \"%s\" request
>> failed
>> +                                               (err=%d)\n",
>> MT7662_FIRMWARE,
>> +
>>
>
> Here also you broke the string.
>
>
>                                          ret);
>>                         }
>>                 } else {
>>                     BT_DBG("Firmware file \"%s\" Found\n",
>> MT7662_FIRMWARE);
>> @@ -258,8 +268,8 @@ static int btmtk_usb_chk_crc(struct btmtk_usb_data
>> *data, u32 checksum_len)
>>         memmove(data->io_buf, &data->rom_patch_offset, 4);
>>         memmove(&data->io_buf[4], &checksum_len, 4);
>>   -     ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), 0x1,
>> DEVICE_VENDOR_REQUEST_IN,
>> -                                                 0x20, 0x00,
>> data->io_buf, 8,
>> +       ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), 0x1,
>> +                       DEVICE_VENDOR_REQUEST_IN, 0x20, 0x00,
>> data->io_buf, 8,
>>
>> CONTROL_TIMEOUT_JIFFIES);
>>
>
> Here you need to align it better, so that it can be more readable.
>
>
>          if (ret < 0) {
>> @@ -318,8 +328,8 @@ static int btmtk_usb_reset_wmt(struct btmtk_usb_data
>> *data)
>>         BT_DBG("%s\n", __func__);
>>         ret = usb_control_msg(data->udev, usb_sndctrlpipe(data->udev, 0),
>> 0x01,
>> -                               DEVICE_CLASS_REQUEST_OUT, 0x12, 0x00,
>> data->io_buf,
>> -                               8, CONTROL_TIMEOUT_JIFFIES);
>> +                               DEVICE_CLASS_REQUEST_OUT, 0x12, 0x00,
>> +                               data->io_buf, 8, CONTROL_TIMEOUT_JIFFIES);
>>         if (ret)
>>                 BT_ERR("%s:(%d)\n", __func__, ret);
>> @@ -350,7 +360,8 @@ static int btmtk_usb_load_rom_patch(**struct
>> btmtk_usb_data *data)
>>         unsigned char phase;
>>         void *buf;
>>         char *pos;
>> -       unsigned int pipe = usb_sndbulkpipe(data->udev, data->bulk_tx_ep->
>> **bEndpointAddress);
>> +       unsigned int pipe = usb_sndbulkpipe(data->udev,
>> +                       data->bulk_tx_ep->**bEndpointAddress);
>>         if (!data->firmware) {
>>                 BT_ERR("%s:please assign a rom patch\n", __func__);
>> @@ -391,7 +402,8 @@ load_patch_protect:
>>                 goto error0;
>>         }
>>   -     buf = usb_alloc_coherent(data->udev, UPLOAD_PATCH_UNIT,
>> GFP_ATOMIC, &data_dma);
>> +       buf = usb_alloc_coherent(data->udev, UPLOAD_PATCH_UNIT,
>> GFP_ATOMIC,
>> +                       &data_dma);
>>         if (!buf) {
>>                 ret = -ENOMEM;
>> @@ -409,7 +421,8 @@ load_patch_protect:
>>         /* loading rom patch */
>>         while (1) {
>>                 s32 sent_len_max = UPLOAD_PATCH_UNIT - PATCH_HEADER_SIZE;
>> -               sent_len = (patch_len - cur_len) >= sent_len_max ?
>> sent_len_max : (patch_len - cur_len);
>> +               sent_len = (patch_len - cur_len) >= sent_len_max ?
>> sent_len_max
>> +                       : (patch_len - cur_len);
>>
>
> Maybe here it would be more readable if you leave ':' in the previous
> line. As far as i have noticed there is a tendency to leave the operators
> in the previous lines.
>
>
>                  BT_DBG("patch_len = %d\n", patch_len);
>>                 BT_DBG("cur_len = %d\n", cur_len);
>> @@ -442,10 +455,12 @@ load_patch_protect:
>>                         pos[8] = phase;
>>   -                     memcpy(&pos[9], data->firmware->data +
>> PATCH_INFO_SIZE + cur_len, sent_len);
>> +                       memcpy(&pos[9], data->firmware->data +
>> PATCH_INFO_SIZE
>> +                                       + cur_len, sent_len);
>>   -                     BT_DBG("sent_len + PATCH_HEADER_SIZE = %d, phase
>> = %d\n",
>> -                                       sent_len + PATCH_HEADER_SIZE,
>> phase);
>> +                       BT_DBG("sent_len + PATCH_HEADER_SIZE = %d,
>> +                                       phase = %d\n", sent_len +
>> +                                       PATCH_HEADER_SIZE, phase);
>>
>>
>
> Here, you also broke the string.
>
>
>                          usb_fill_bulk_urb(urb,
>>                                         data->udev,
>> @@ -463,7 +478,8 @@ load_patch_protect:
>>                         if (ret)
>>                                 goto error2;
>>   -                     if (!wait_for_completion_timeout(**&sent_to_mcu_done,
>> msecs_to_jiffies(1000))) {
>> +                       if (!wait_for_completion_timeout(**
>> &sent_to_mcu_done,
>> +                                               msecs_to_jiffies(1000))) {
>>                                 usb_kill_urb(urb);
>>                                 BT_ERR("upload rom_patch timeout\n");
>>                                 goto error2;
>> @@ -480,7 +496,8 @@ load_patch_protect:
>>                 }
>>         }
>>   -     total_checksum = checksume16((u8 *)data->firmware->data +
>> PATCH_INFO_SIZE, patch_len);
>> +       total_checksum = checksume16((u8 *)data->firmware->data +
>> +                       PATCH_INFO_SIZE, patch_len);
>>         BT_DBG("Send checksum req..\n");
>>   @@ -520,7 +537,8 @@ static int load_fw_iv(struct btmtk_usb_data *data)
>>         memmove(buf, data->firmware->data + 32, 64);
>>         ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), 0x01,
>> -
>> DEVICE_VENDOR_REQUEST_OUT, 0x12, 0x0, buf, 64,
>> +
>> DEVICE_VENDOR_REQUEST_OUT,
>> +                                                 0x12, 0x0, buf, 64,
>>
>> CONTROL_TIMEOUT_JIFFIES);
>>         if (ret < 0) {
>> @@ -559,7 +577,8 @@ static int btmtk_usb_load_fw(struct btmtk_usb_data
>> *data)
>>         dma_addr_t data_dma;
>>         int ret = 0, sent_len;
>>         struct completion sent_to_mcu_done;
>> -       unsigned int pipe = usb_sndbulkpipe(data->udev, data->bulk_tx_ep->
>> **bEndpointAddress);
>> +       unsigned int pipe = usb_sndbulkpipe(data->udev,
>> +                       data->bulk_tx_ep->**bEndpointAddress);
>>         if (!data->firmware) {
>>                 BT_ERR("%s:please assign a fw\n", __func__);
>> @@ -598,9 +617,11 @@ loadfw_protect:
>>                         | (*(data->firmware->data + 5) << 8)
>>                         | (*(data->firmware->data + 4));
>>   -     fw_ver = (*(data->firmware->data + 11) << 8) |
>> (*(data->firmware->data + 10));
>> +       fw_ver = (*(data->firmware->data + 11) << 8) |
>> +               (*(data->firmware->data + 10));
>>   -     build_ver = (*(data->firmware->data + 9) << 8) |
>> (*(data->firmware->data + 8));
>> +       build_ver = (*(data->firmware->data + 9) << 8) |
>> +               (*(data->firmware->data + 8));
>>
>
> The above would more readable if it was:
>
>  +       build_ver = (*(data->firmware->data + 9) << 8) |
>> +                   (*(data->firmware->data + 8));
>>
> That would be the result if you press tab to align (actually i don't know
> how your editor is setup).
>
>
>          BT_DBG("fw version:%d.%d.%02d ",
>>                         (fw_ver & 0xf000) >> 8,
>> @@ -657,7 +678,8 @@ loadfw_protect:
>>         /* Loading ILM */
>>         while (1) {
>> -               sent_len = (ilm_len - cur_len) >= 14336 ? 14336 :
>> (ilm_len - cur_len);
>> +               sent_len = (ilm_len - cur_len) >= 14336 ? 14336
>> +                       : (ilm_len - cur_len);
>>
>>
>
> Here, better place ':' in the line above.
>
>
>                  if (sent_len > 0) {
>>                         packet_header &= ~(0xffffffff);
>> @@ -665,7 +687,9 @@ loadfw_protect:
>>                         packet_header = cpu_to_le32(packet_header);
>>                         memmove(buf, &packet_header, 4);
>> -                       memmove(buf + 4, data->firmware->data + 32 +
>> cur_len, sent_len);
>> +                       memmove(buf + 4, data->firmware->data + 32 +
>> cur_len,
>> +                                       sent_len);
>> +
>>                         /* U2M_PDMA descriptor */
>>                         btmtk_usb_io_write32(data, 0x230, cur_len);
>> @@ -693,7 +717,8 @@ loadfw_protect:
>>                         if (ret)
>>                                 goto error3;
>>   -                     if (!wait_for_completion_timeout(**&sent_to_mcu_done,
>> msecs_to_jiffies(1000))) {
>> +                       if (!wait_for_completion_timeout(**
>> &sent_to_mcu_done,
>> +                                               msecs_to_jiffies(1000))) {
>>                                 usb_kill_urb(urb);
>>                                 BT_ERR("upload ilm fw timeout\n");
>>                                 goto error3;
>> @@ -714,7 +739,8 @@ loadfw_protect:
>>         /* Loading DLM */
>>         while (1) {
>> -               sent_len = (dlm_len - cur_len) >= 14336 ? 14336 :
>> (dlm_len - cur_len);
>> +               sent_len = (dlm_len - cur_len) >= 14336 ? 14336
>> +                       : (dlm_len - cur_len);
>>                 if (sent_len > 0) {
>>                         packet_header &= ~(0xffffffff);
>> @@ -722,7 +748,8 @@ loadfw_protect:
>>                         packet_header = cpu_to_le32(packet_header);
>>                         memmove(buf, &packet_header, 4);
>> -                       memmove(buf + 4, data->firmware->data + 32 +
>> ilm_len + cur_len, sent_len);
>> +                       memmove(buf + 4, data->firmware->data + 32 +
>> ilm_len
>> +                                       + cur_len, sent_len);
>>
>>
>
> Here, also leave the operator '+' in the previous line.
>
>
>                          /* U2M_PDMA descriptor */
>>                         btmtk_usb_io_write32(data, 0x230, 0x80000 +
>> cur_len);
>> @@ -751,7 +778,8 @@ loadfw_protect:
>>                         if (ret)
>>                                 goto error3;
>>   -                     if (!wait_for_completion_timeout(**&sent_to_mcu_done,
>> msecs_to_jiffies(1000))) {
>> +                       if (!wait_for_completion_timeout(**
>> &sent_to_mcu_done,
>> +                                               msecs_to_jiffies(1000))) {
>>                                 usb_kill_urb(urb);
>>                                 BT_ERR("upload dlm fw timeout\n");
>>                                 goto error3;
>> @@ -978,8 +1006,8 @@ static int btmtk_usb_submit_bulk_in_urb(**struct
>> hci_dev *hdev, gfp_t mem_flags)
>>         pipe = usb_rcvbulkpipe(data->udev, data->bulk_rx_ep->**
>> bEndpointAddress);
>>   -     usb_fill_bulk_urb(urb, data->udev, pipe,
>> -                                       buf, size,
>> btmtk_usb_bulk_in_complete, hdev);
>> +       usb_fill_bulk_urb(urb, data->udev, pipe, buf, size,
>> +                       btmtk_usb_bulk_in_complete, hdev);
>>         urb->transfer_flags |= URB_FREE_BUFFER;
>>   @@ -1015,7 +1043,8 @@ static void btmtk_usb_isoc_in_complete(**struct
>> urb *urb)
>>         if (urb->status == 0) {
>>                 for (i = 0; i < urb->number_of_packets; i++) {
>>                         unsigned int offset =
>> urb->iso_frame_desc[i].offset;
>> -                       unsigned int length =
>> urb->iso_frame_desc[i].actual_**length;
>> +                       unsigned int length = urb->iso_frame_desc[i]
>> +                               .actual_length;
>>
>
> You cannot do that, you cannot separate the structure from its field. You
> can either leave it as it is or do:
>
>> +                       unsigned int length =
>> +                                urb->iso_frame_desc[i].actual_**length;
>>
>
>
>                          if (urb->iso_frame_desc[i].**status)
>>                                 continue;
>> @@ -1096,8 +1125,9 @@ static int btmtk_usb_submit_isoc_in_urb(**struct
>> hci_dev *hdev, gfp_t mem_flags)
>>         pipe = usb_rcvisocpipe(data->udev, data->isoc_rx_ep->**
>> bEndpointAddress);
>>   -     usb_fill_int_urb(urb, data->udev, pipe, buf, size,
>> btmtk_usb_isoc_in_complete,
>> -                               hdev, data->isoc_rx_ep->bInterval);
>> +       usb_fill_int_urb(urb, data->udev, pipe, buf, size,
>> +                       btmtk_usb_isoc_in_complete, hdev,
>> +                       data->isoc_rx_ep->bInterval);
>>         urb->transfer_flags  = URB_FREE_BUFFER | URB_ISO_ASAP;
>>   @@ -1306,7 +1336,8 @@ static int btmtk_usb_send_frame(struct sk_buff
>> *skb)
>>                 }
>>                 usb_fill_control_urb(urb, data->udev, pipe, (void *) dr,
>> -                               skb->data, skb->len,
>> btmtk_usb_tx_complete, skb);
>> +                               skb->data, skb->len,
>> btmtk_usb_tx_complete,
>> +                               skb);
>>                 hdev->stat.cmd_tx++;
>>                 break;
>> @@ -1322,8 +1353,8 @@ static int btmtk_usb_send_frame(struct sk_buff *skb)
>>                 pipe = usb_sndbulkpipe(data->udev,
>>                                         data->bulk_tx_ep->**
>> bEndpointAddress);
>>   -             usb_fill_bulk_urb(urb, data->udev, pipe,
>> -                               skb->data, skb->len,
>> btmtk_usb_tx_complete, skb);
>> +               usb_fill_bulk_urb(urb, data->udev, pipe, skb->data,
>> skb->len,
>> +                               btmtk_usb_tx_complete, skb);
>>                 hdev->stat.acl_tx++;
>>                 BT_DBG("HCI_ACLDATA_PKT:\n");
>> @@ -1442,7 +1473,8 @@ static inline int __set_isoc_interface(struct
>> hci_dev *hdev, int altsetting)
>>     static void btmtk_usb_work(struct work_struct *work)
>>   {
>> -       struct btmtk_usb_data *data = container_of(work, struct
>> btmtk_usb_data, work);
>> +       struct btmtk_usb_data *data = container_of(work, struct
>> btmtk_usb_data,
>> +                       work);
>>         struct hci_dev *hdev = data->hdev;
>>         int new_alts;
>>         int err;
>> @@ -1451,7 +1483,8 @@ static void btmtk_usb_work(struct work_struct *work)
>>         if (hdev->conn_hash.sco_num > 0) {
>>                 if (!test_bit(BTUSB_DID_ISO_**RESUME, &data->flags)) {
>> -                       err = usb_autopm_get_interface(data-**>isoc ?
>> data->isoc : data->intf);
>> +                       err = usb_autopm_get_interface(data-**>isoc ?
>> data->isoc
>> +                                       : data->intf);
>>                         if (err < 0) {
>>                                 clear_bit(BTUSB_ISOC_RUNNING,
>> &data->flags);
>>                                 usb_kill_anchored_urbs(&data->**
>> isoc_anchor);
>> @@ -1489,13 +1522,15 @@ static void btmtk_usb_work(struct work_struct
>> *work)
>>                 __set_isoc_interface(hdev, 0);
>>                 if (test_and_clear_bit(BTUSB_DID_**ISO_RESUME,
>> &data->flags))
>> -                        usb_autopm_put_interface(data-**>isoc ?
>> data->isoc : data->intf);
>> +                        usb_autopm_put_interface(data-**>isoc ?
>> data->isoc
>> +                                        : data->intf);
>>         }
>>   }
>>     static void btmtk_usb_waker(struct work_struct *work)
>>   {
>> -       struct btmtk_usb_data *data = container_of(work, struct
>> btmtk_usb_data, waker);
>> +       struct btmtk_usb_data *data = container_of(work, struct
>> btmtk_usb_data,
>> +                       waker);
>>         int err;
>>         err = usb_autopm_get_interface(data-**>intf);
>>
>
> I have not commended them all but the idea is the same. Good work by the
> way :)
> Keep in mind to change the code in order to be more readable without
> violating the kernel coding style (significantly).
>
> ksenia
>
Xenia Ragiadakou Oct. 5, 2013, 1:50 p.m. UTC | #3
On 10/05/2013 04:43 PM, Rashika Kheria wrote:
> Hi Xenia,
>
> Thank you for your feedback.
>
> I tried to incorporate the name of the driver in the subject line but 
> since the subject line could contain only 50 characters, therefore, I 
> was unable to add the same. Kindly guide me to rectify this issue.
>
> I will upload another version to correcting the other suggested changes.
>
> Thanks in advance.
>
> Rashika Kheria
>
>

Yeah, i understand but i think it is better to include the driver name 
even if the subject line exceeds (a little) 50 chars. Maybe, in this 
case, you can arrange your subject to be shorter, i don't know "limit 
line below 80 chars" :) well.. i don't know and my english is bad. You 
will find a way I m sure :)

ksenia
> On Sat, Oct 5, 2013 at 7:04 PM, Xenia Ragiadakou 
> <burzalodowa@gmail.com <mailto:burzalodowa@gmail.com>> wrote:
>
>     Hi Rashika,
>
>     I noticed that you corrected the subject line by yourself. Good job!
>     It would be nice to add the name of the driver on the subject line
>     too, so that somebody will can infer quickly to which driver this
>     patch applies. For example, you can write in the future:
>     Staging: btmtk_usb: Fix line length exceeding 80 characters
>
>     Also you need to add a changelog in your patch, which will state
>     why you did that change and which tool you used to find the bad
>     coding style (if you used one) for example:
>     This patch fixes the following checkpatch.pl
>     <http://checkpatch.pl> warning/error: <warning/error>
>
>     I have added some other comments below.
>
>
>     On 10/05/2013 02:55 PM, Rashika wrote:
>
>         Signed-off-by: Rashika <rashika.kheria@gmail.com
>         <mailto:rashika.kheria@gmail.com>>
>         ---
>           drivers/staging/btmtk_usb/btmtk_usb.c |  137
>         +++++++++++++++++++++------------
>           1 file changed, 86 insertions(+), 51 deletions(-)
>
>         diff --git a/drivers/staging/btmtk_usb/btmtk_usb.c
>         b/drivers/staging/btmtk_usb/btmtk_usb.c
>         index 0e783e8..0b3e593 100644
>         --- a/drivers/staging/btmtk_usb/btmtk_usb.c
>         +++ b/drivers/staging/btmtk_usb/btmtk_usb.c
>         @@ -16,7 +16,8 @@
>            *  You should have received a copy of the GNU General
>         Public License
>            *  along with this program; if not, write to the Free Software
>            *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
>          02111-1307  USA
>         - *  or on the worldwide web at
>         http://www.gnu.org/licenses/old-licenses/gpl-2.0.txt.
>         + *  or on the worldwide web at
>         http://www.gnu.org/licenses/old-licenses/
>         + *  gpl-2.0.txt.
>            *
>            */
>
>
>     Maybe here is not a good idea to break the link. You could move
>     all of it in the next line or leave it as is.
>
>
>         @@ -72,8 +73,9 @@ static int btmtk_usb_reset(struct usb_device
>         *udev)
>                 BT_DBG("%s\n", __func__);
>           -     ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
>         0x01, DEVICE_VENDOR_REQUEST_OUT,
>         -                                                 0x01, 0x00,
>         NULL, 0x00, CONTROL_TIMEOUT_JIFFIES);
>         +       ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
>         0x01,
>         +                       DEVICE_VENDOR_REQUEST_OUT, 0x01, 0x00,
>         NULL, 0x00,
>         +                       CONTROL_TIMEOUT_JIFFIES);
>                 if (ret < 0) {
>                         BT_ERR("%s error(%d)\n", __func__, ret);
>         @@ -92,13 +94,14 @@ static int btmtk_usb_io_read32(struct
>         btmtk_usb_data *data, u32 reg, u32 *val)
>                 struct usb_device *udev = data->udev;
>                 int ret;
>           -     ret = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
>         request, DEVICE_VENDOR_REQUEST_IN,
>         -                                                 0x0, reg,
>         data->io_buf, 4,
>         +       ret = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
>         request,
>         +                       DEVICE_VENDOR_REQUEST_IN, 0x0, reg,
>         data->io_buf, 4,
>         CONTROL_TIMEOUT_JIFFIES);
>
>
>     Try that your changes lead to uniform identation, that means the
>     above could be:
>
>         +       ret = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
>         request,
>         +                       DEVICE_VENDOR_REQUEST_IN, 0x0, reg,
>         data->io_buf, 4,
>         +                       CONTROL_TIMEOUT_JIFFIES);
>
>
>
>                 if (ret < 0) {
>
>                         *val = 0xffffffff;
>         -               BT_ERR("%s error(%d), reg=%x, value=%x\n",
>         __func__, ret, reg, *val);
>         +               BT_ERR("%s error(%d), reg=%x, value=%x\n",
>         __func__, ret, reg,
>         +                               *val);
>                         return ret;
>                 }
>           @@ -122,12 +125,13 @@ static int btmtk_usb_io_write32(struct
>         btmtk_usb_data *data, u32 reg, u32 val)
>                 index = (u16)reg;
>                 value = val & 0x0000ffff;
>           -     ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
>         request, DEVICE_VENDOR_REQUEST_OUT,
>         -                                                 value,
>         index, NULL, 0,
>         +       ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
>         request,
>         +                       DEVICE_VENDOR_REQUEST_OUT, value,
>         index, NULL, 0,
>         CONTROL_TIMEOUT_JIFFIES);
>
>
>     The same here as far as concerns the identation.
>
>
>                 if (ret < 0) {
>         -               BT_ERR("%s error(%d), reg=%x, value=%x\n",
>         __func__, ret, reg, val);
>         +               BT_ERR("%s error(%d), reg=%x, value=%x\n",
>         __func__, ret, reg,
>         +                               val);
>                         return ret;
>                 }
>           @@ -139,7 +143,8 @@ static int btmtk_usb_io_write32(struct
>         btmtk_usb_data *data, u32 reg, u32 val)
>                                         value, index, NULL, 0,
>         CONTROL_TIMEOUT_JIFFIES);
>                 if (ret < 0) {
>         -               BT_ERR("%s error(%d), reg=%x, value=%x\n",
>         __func__, ret, reg, val);
>         +               BT_ERR("%s error(%d), reg=%x, value=%x\n",
>         __func__, ret, reg,
>         +                               val);
>                         return ret;
>                 }
>           @@ -186,13 +191,16 @@ static void btmtk_usb_cap_init(struct
>         btmtk_usb_data *data)
>                         ret = request_firmware(&firmware,
>         MT7650_FIRMWARE, &udev->dev);
>                         if (ret < 0) {
>                                 if (ret == -ENOENT) {
>         -                               BT_ERR("Firmware file \"%s\"
>         not found \n", MT7650_FIRMWARE);
>         +                               BT_ERR("Firmware file \"%s\"
>         not found \n",
>         + MT7650_FIRMWARE);
>                                 } else {
>         -                               BT_ERR("Firmware file \"%s\"
>         request failed (err=%d) \n",
>         - MT7650_FIRMWARE, ret);
>         +                               BT_ERR("Firmware file \"%s\"
>         request failed
>         + (err=%d) \n", MT7650_FIRMWARE,
>         +
>
>
>     Here, by breaking the string you introduced another checkpatch
>     warning. Better leave it as it is.
>     Also, if ever you need to break a line do:
>
>
>     +                               BT_ERR("Firmware file \"%s\"
>     request failed"
>     +                                               "(err=%d) \n",
>     MT7650_FIRMWARE,
>
>
>                                                 ret);
>                                 }
>                         } else {
>         -                       BT_DBG("Firmware file \"%s\" Found
>         \n", MT7650_FIRMWARE);
>         +                       BT_DBG("Firmware file \"%s\" Found \n",
>         + MT7650_FIRMWARE);
>                                 /* load firmware here */
>                                 data->firmware = firmware;
>                                 btmtk_usb_load_fw(data);
>         @@ -205,10 +213,12 @@ static void btmtk_usb_cap_init(struct
>         btmtk_usb_data *data)
>                         ret = request_firmware(&firmware,
>         MT7662_FIRMWARE, &udev->dev);
>                         if (ret < 0) {
>                                 if (ret == -ENOENT) {
>         -                               BT_ERR("Firmware file \"%s\"
>         not found\n", MT7662_FIRMWARE);
>         +                               BT_ERR("Firmware file \"%s\"
>         not found\n",
>         + MT7662_FIRMWARE);
>                                 } else {
>         -                               BT_ERR("Firmware file \"%s\"
>         request failed (err=%d)\n",
>         -                                       MT7662_FIRMWARE, ret);
>         +                               BT_ERR("Firmware file \"%s\"
>         request failed
>         + (err=%d)\n", MT7662_FIRMWARE,
>         +
>
>
>     Here also you broke the string.
>
>
>                                                 ret);
>                                 }
>                         } else {
>                             BT_DBG("Firmware file \"%s\" Found\n",
>         MT7662_FIRMWARE);
>         @@ -258,8 +268,8 @@ static int btmtk_usb_chk_crc(struct
>         btmtk_usb_data *data, u32 checksum_len)
>                 memmove(data->io_buf, &data->rom_patch_offset, 4);
>                 memmove(&data->io_buf[4], &checksum_len, 4);
>           -     ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
>         0x1, DEVICE_VENDOR_REQUEST_IN,
>         -                                                 0x20, 0x00,
>         data->io_buf, 8,
>         +       ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), 0x1,
>         +                       DEVICE_VENDOR_REQUEST_IN, 0x20, 0x00,
>         data->io_buf, 8,
>         CONTROL_TIMEOUT_JIFFIES);
>
>
>     Here you need to align it better, so that it can be more readable.
>
>
>                 if (ret < 0) {
>         @@ -318,8 +328,8 @@ static int btmtk_usb_reset_wmt(struct
>         btmtk_usb_data *data)
>                 BT_DBG("%s\n", __func__);
>                 ret = usb_control_msg(data->udev,
>         usb_sndctrlpipe(data->udev, 0), 0x01,
>         - DEVICE_CLASS_REQUEST_OUT, 0x12, 0x00, data->io_buf,
>         -                               8, CONTROL_TIMEOUT_JIFFIES);
>         + DEVICE_CLASS_REQUEST_OUT, 0x12, 0x00,
>         +                               data->io_buf, 8,
>         CONTROL_TIMEOUT_JIFFIES);
>                 if (ret)
>                         BT_ERR("%s:(%d)\n", __func__, ret);
>         @@ -350,7 +360,8 @@ static int btmtk_usb_load_rom_patch(struct
>         btmtk_usb_data *data)
>                 unsigned char phase;
>                 void *buf;
>                 char *pos;
>         -       unsigned int pipe = usb_sndbulkpipe(data->udev,
>         data->bulk_tx_ep->bEndpointAddress);
>         +       unsigned int pipe = usb_sndbulkpipe(data->udev,
>         +                       data->bulk_tx_ep->bEndpointAddress);
>                 if (!data->firmware) {
>                         BT_ERR("%s:please assign a rom patch\n",
>         __func__);
>         @@ -391,7 +402,8 @@ load_patch_protect:
>                         goto error0;
>                 }
>           -     buf = usb_alloc_coherent(data->udev,
>         UPLOAD_PATCH_UNIT, GFP_ATOMIC, &data_dma);
>         +       buf = usb_alloc_coherent(data->udev,
>         UPLOAD_PATCH_UNIT, GFP_ATOMIC,
>         +                       &data_dma);
>                 if (!buf) {
>                         ret = -ENOMEM;
>         @@ -409,7 +421,8 @@ load_patch_protect:
>                 /* loading rom patch */
>                 while (1) {
>                         s32 sent_len_max = UPLOAD_PATCH_UNIT -
>         PATCH_HEADER_SIZE;
>         -               sent_len = (patch_len - cur_len) >=
>         sent_len_max ? sent_len_max : (patch_len - cur_len);
>         +               sent_len = (patch_len - cur_len) >=
>         sent_len_max ? sent_len_max
>         +                       : (patch_len - cur_len);
>
>
>     Maybe here it would be more readable if you leave ':' in the
>     previous line. As far as i have noticed there is a tendency to
>     leave the operators in the previous lines.
>
>
>                         BT_DBG("patch_len = %d\n", patch_len);
>                         BT_DBG("cur_len = %d\n", cur_len);
>         @@ -442,10 +455,12 @@ load_patch_protect:
>                                 pos[8] = phase;
>           -                     memcpy(&pos[9], data->firmware->data +
>         PATCH_INFO_SIZE + cur_len, sent_len);
>         +                       memcpy(&pos[9], data->firmware->data +
>         PATCH_INFO_SIZE
>         +                                       + cur_len, sent_len);
>           -                     BT_DBG("sent_len + PATCH_HEADER_SIZE =
>         %d, phase = %d\n",
>         -                                       sent_len +
>         PATCH_HEADER_SIZE, phase);
>         +                       BT_DBG("sent_len + PATCH_HEADER_SIZE = %d,
>         +                                       phase = %d\n", sent_len +
>         + PATCH_HEADER_SIZE, phase);
>
>
>     Here, you also broke the string.
>
>
>                                 usb_fill_bulk_urb(urb,
>                                                 data->udev,
>         @@ -463,7 +478,8 @@ load_patch_protect:
>                                 if (ret)
>                                         goto error2;
>           -                     if
>         (!wait_for_completion_timeout(&sent_to_mcu_done,
>         msecs_to_jiffies(1000))) {
>         +                       if
>         (!wait_for_completion_timeout(&sent_to_mcu_done,
>         + msecs_to_jiffies(1000))) {
>                                         usb_kill_urb(urb);
>                                         BT_ERR("upload rom_patch
>         timeout\n");
>                                         goto error2;
>         @@ -480,7 +496,8 @@ load_patch_protect:
>                         }
>                 }
>           -     total_checksum = checksume16((u8
>         *)data->firmware->data + PATCH_INFO_SIZE, patch_len);
>         +       total_checksum = checksume16((u8 *)data->firmware->data +
>         +                       PATCH_INFO_SIZE, patch_len);
>                 BT_DBG("Send checksum req..\n");
>           @@ -520,7 +537,8 @@ static int load_fw_iv(struct
>         btmtk_usb_data *data)
>                 memmove(buf, data->firmware->data + 32, 64);
>                 ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
>         0x01,
>         - DEVICE_VENDOR_REQUEST_OUT, 0x12, 0x0, buf, 64,
>         + DEVICE_VENDOR_REQUEST_OUT,
>         + 0x12, 0x0, buf, 64,
>         CONTROL_TIMEOUT_JIFFIES);
>                 if (ret < 0) {
>         @@ -559,7 +577,8 @@ static int btmtk_usb_load_fw(struct
>         btmtk_usb_data *data)
>                 dma_addr_t data_dma;
>                 int ret = 0, sent_len;
>                 struct completion sent_to_mcu_done;
>         -       unsigned int pipe = usb_sndbulkpipe(data->udev,
>         data->bulk_tx_ep->bEndpointAddress);
>         +       unsigned int pipe = usb_sndbulkpipe(data->udev,
>         +                       data->bulk_tx_ep->bEndpointAddress);
>                 if (!data->firmware) {
>                         BT_ERR("%s:please assign a fw\n", __func__);
>         @@ -598,9 +617,11 @@ loadfw_protect:
>                                 | (*(data->firmware->data + 5) << 8)
>                                 | (*(data->firmware->data + 4));
>           -     fw_ver = (*(data->firmware->data + 11) << 8) |
>         (*(data->firmware->data + 10));
>         +       fw_ver = (*(data->firmware->data + 11) << 8) |
>         +               (*(data->firmware->data + 10));
>           -     build_ver = (*(data->firmware->data + 9) << 8) |
>         (*(data->firmware->data + 8));
>         +       build_ver = (*(data->firmware->data + 9) << 8) |
>         +               (*(data->firmware->data + 8));
>
>
>     The above would more readable if it was:
>
>         +       build_ver = (*(data->firmware->data + 9) << 8) |
>         +                   (*(data->firmware->data + 8));
>
>     That would be the result if you press tab to align (actually i
>     don't know how your editor is setup).
>
>
>                 BT_DBG("fw version:%d.%d.%02d ",
>                                 (fw_ver & 0xf000) >> 8,
>         @@ -657,7 +678,8 @@ loadfw_protect:
>                 /* Loading ILM */
>                 while (1) {
>         -               sent_len = (ilm_len - cur_len) >= 14336 ?
>         14336 : (ilm_len - cur_len);
>         +               sent_len = (ilm_len - cur_len) >= 14336 ? 14336
>         +                       : (ilm_len - cur_len);
>
>
>     Here, better place ':' in the line above.
>
>
>                         if (sent_len > 0) {
>                                 packet_header &= ~(0xffffffff);
>         @@ -665,7 +687,9 @@ loadfw_protect:
>                                 packet_header =
>         cpu_to_le32(packet_header);
>                                 memmove(buf, &packet_header, 4);
>         -                       memmove(buf + 4, data->firmware->data
>         + 32 + cur_len, sent_len);
>         +                       memmove(buf + 4, data->firmware->data
>         + 32 + cur_len,
>         +                                       sent_len);
>         +
>                                 /* U2M_PDMA descriptor */
>                                 btmtk_usb_io_write32(data, 0x230,
>         cur_len);
>         @@ -693,7 +717,8 @@ loadfw_protect:
>                                 if (ret)
>                                         goto error3;
>           -                     if
>         (!wait_for_completion_timeout(&sent_to_mcu_done,
>         msecs_to_jiffies(1000))) {
>         +                       if
>         (!wait_for_completion_timeout(&sent_to_mcu_done,
>         + msecs_to_jiffies(1000))) {
>                                         usb_kill_urb(urb);
>                                         BT_ERR("upload ilm fw timeout\n");
>                                         goto error3;
>         @@ -714,7 +739,8 @@ loadfw_protect:
>                 /* Loading DLM */
>                 while (1) {
>         -               sent_len = (dlm_len - cur_len) >= 14336 ?
>         14336 : (dlm_len - cur_len);
>         +               sent_len = (dlm_len - cur_len) >= 14336 ? 14336
>         +                       : (dlm_len - cur_len);
>                         if (sent_len > 0) {
>                                 packet_header &= ~(0xffffffff);
>         @@ -722,7 +748,8 @@ loadfw_protect:
>                                 packet_header =
>         cpu_to_le32(packet_header);
>                                 memmove(buf, &packet_header, 4);
>         -                       memmove(buf + 4, data->firmware->data
>         + 32 + ilm_len + cur_len, sent_len);
>         +                       memmove(buf + 4, data->firmware->data
>         + 32 + ilm_len
>         +                                       + cur_len, sent_len);
>
>
>     Here, also leave the operator '+' in the previous line.
>
>
>                                 /* U2M_PDMA descriptor */
>                                 btmtk_usb_io_write32(data, 0x230,
>         0x80000 + cur_len);
>         @@ -751,7 +778,8 @@ loadfw_protect:
>                                 if (ret)
>                                         goto error3;
>           -                     if
>         (!wait_for_completion_timeout(&sent_to_mcu_done,
>         msecs_to_jiffies(1000))) {
>         +                       if
>         (!wait_for_completion_timeout(&sent_to_mcu_done,
>         + msecs_to_jiffies(1000))) {
>                                         usb_kill_urb(urb);
>                                         BT_ERR("upload dlm fw timeout\n");
>                                         goto error3;
>         @@ -978,8 +1006,8 @@ static int
>         btmtk_usb_submit_bulk_in_urb(struct hci_dev *hdev, gfp_t
>         mem_flags)
>                 pipe = usb_rcvbulkpipe(data->udev,
>         data->bulk_rx_ep->bEndpointAddress);
>           -     usb_fill_bulk_urb(urb, data->udev, pipe,
>         -                                       buf, size,
>         btmtk_usb_bulk_in_complete, hdev);
>         +       usb_fill_bulk_urb(urb, data->udev, pipe, buf, size,
>         +                       btmtk_usb_bulk_in_complete, hdev);
>                 urb->transfer_flags |= URB_FREE_BUFFER;
>           @@ -1015,7 +1043,8 @@ static void
>         btmtk_usb_isoc_in_complete(struct urb *urb)
>                 if (urb->status == 0) {
>                         for (i = 0; i < urb->number_of_packets; i++) {
>                                 unsigned int offset =
>         urb->iso_frame_desc[i].offset;
>         -                       unsigned int length =
>         urb->iso_frame_desc[i].actual_length;
>         +                       unsigned int length =
>         urb->iso_frame_desc[i]
>         +                               .actual_length;
>
>
>     You cannot do that, you cannot separate the structure from its
>     field. You can either leave it as it is or do:
>
>         +                       unsigned int length =
>         +  urb->iso_frame_desc[i].actual_length;
>
>
>
>                                 if (urb->iso_frame_desc[i].status)
>                                         continue;
>         @@ -1096,8 +1125,9 @@ static int
>         btmtk_usb_submit_isoc_in_urb(struct hci_dev *hdev, gfp_t
>         mem_flags)
>                 pipe = usb_rcvisocpipe(data->udev,
>         data->isoc_rx_ep->bEndpointAddress);
>           -     usb_fill_int_urb(urb, data->udev, pipe, buf, size,
>         btmtk_usb_isoc_in_complete,
>         -                               hdev,
>         data->isoc_rx_ep->bInterval);
>         +       usb_fill_int_urb(urb, data->udev, pipe, buf, size,
>         +                       btmtk_usb_isoc_in_complete, hdev,
>         + data->isoc_rx_ep->bInterval);
>                 urb->transfer_flags  = URB_FREE_BUFFER | URB_ISO_ASAP;
>           @@ -1306,7 +1336,8 @@ static int btmtk_usb_send_frame(struct
>         sk_buff *skb)
>                         }
>                         usb_fill_control_urb(urb, data->udev, pipe,
>         (void *) dr,
>         -                               skb->data, skb->len,
>         btmtk_usb_tx_complete, skb);
>         +                               skb->data, skb->len,
>         btmtk_usb_tx_complete,
>         +                               skb);
>                         hdev->stat.cmd_tx++;
>                         break;
>         @@ -1322,8 +1353,8 @@ static int btmtk_usb_send_frame(struct
>         sk_buff *skb)
>                         pipe = usb_sndbulkpipe(data->udev,
>         data->bulk_tx_ep->bEndpointAddress);
>           -             usb_fill_bulk_urb(urb, data->udev, pipe,
>         -                               skb->data, skb->len,
>         btmtk_usb_tx_complete, skb);
>         +               usb_fill_bulk_urb(urb, data->udev, pipe,
>         skb->data, skb->len,
>         +                               btmtk_usb_tx_complete, skb);
>                         hdev->stat.acl_tx++;
>                         BT_DBG("HCI_ACLDATA_PKT:\n");
>         @@ -1442,7 +1473,8 @@ static inline int
>         __set_isoc_interface(struct hci_dev *hdev, int altsetting)
>             static void btmtk_usb_work(struct work_struct *work)
>           {
>         -       struct btmtk_usb_data *data = container_of(work,
>         struct btmtk_usb_data, work);
>         +       struct btmtk_usb_data *data = container_of(work,
>         struct btmtk_usb_data,
>         +                       work);
>                 struct hci_dev *hdev = data->hdev;
>                 int new_alts;
>                 int err;
>         @@ -1451,7 +1483,8 @@ static void btmtk_usb_work(struct
>         work_struct *work)
>                 if (hdev->conn_hash.sco_num > 0) {
>                         if (!test_bit(BTUSB_DID_ISO_RESUME,
>         &data->flags)) {
>         -                       err =
>         usb_autopm_get_interface(data->isoc ? data->isoc : data->intf);
>         +                       err =
>         usb_autopm_get_interface(data->isoc ? data->isoc
>         +                                       : data->intf);
>                                 if (err < 0) {
>         clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
>         usb_kill_anchored_urbs(&data->isoc_anchor);
>         @@ -1489,13 +1522,15 @@ static void btmtk_usb_work(struct
>         work_struct *work)
>                         __set_isoc_interface(hdev, 0);
>                         if (test_and_clear_bit(BTUSB_DID_ISO_RESUME,
>         &data->flags))
>         -  usb_autopm_put_interface(data->isoc ? data->isoc : data->intf);
>         +  usb_autopm_put_interface(data->isoc ? data->isoc
>         +                                        : data->intf);
>                 }
>           }
>             static void btmtk_usb_waker(struct work_struct *work)
>           {
>         -       struct btmtk_usb_data *data = container_of(work,
>         struct btmtk_usb_data, waker);
>         +       struct btmtk_usb_data *data = container_of(work,
>         struct btmtk_usb_data,
>         +                       waker);
>                 int err;
>                 err = usb_autopm_get_interface(data->intf);
>
>
>     I have not commended them all but the idea is the same. Good work
>     by the way :)
>     Keep in mind to change the code in order to be more readable
>     without violating the kernel coding style (significantly).
>
>     ksenia
>
>
>
>
> -- 
> Rashika Kheria
> B.Tech CSE
> IIIT Hyderabad
Rashika Oct. 5, 2013, 1:54 p.m. UTC | #4
Hi Xenia,

Thanks for your early response. I will rectify the mistakes.

Thanks,
Rashika Kheria


On Sat, Oct 5, 2013 at 7:20 PM, Xenia Ragiadakou <burzalodowa@gmail.com>wrote:

>  On 10/05/2013 04:43 PM, Rashika Kheria wrote:
>
>  Hi Xenia,
>
>  Thank you for your feedback.
>
>  I tried to incorporate the name of the driver in the subject line but
> since the subject line could contain only 50 characters, therefore, I was
> unable to add the same. Kindly guide me to rectify this issue.
>
>  I will upload another version to correcting the other suggested changes.
>
>  Thanks in advance.
>
>  Rashika Kheria
>
>
>
> Yeah, i understand but i think it is better to include the driver name
> even if the subject line exceeds (a little) 50 chars. Maybe, in this case,
> you can arrange your subject to be shorter, i don't know "limit line below
> 80 chars" :) well.. i don't know and my english is bad. You will find a way
> I m sure :)
>
> ksenia
>
>  On Sat, Oct 5, 2013 at 7:04 PM, Xenia Ragiadakou <burzalodowa@gmail.com>wrote:
>
>> Hi Rashika,
>>
>> I noticed that you corrected the subject line by yourself. Good job!
>> It would be nice to add the name of the driver on the subject line too,
>> so that somebody will can infer quickly to which driver this patch applies.
>> For example, you can write in the future:
>> Staging: btmtk_usb: Fix line length exceeding 80 characters
>>
>> Also you need to add a changelog in your patch, which will state why you
>> did that change and which tool you used to find the bad coding style (if
>> you used one) for example:
>> This patch fixes the following checkpatch.pl warning/error:
>> <warning/error>
>>
>> I have added some other comments below.
>>
>>
>> On 10/05/2013 02:55 PM, Rashika wrote:
>>
>>> Signed-off-by: Rashika <rashika.kheria@gmail.com>
>>> ---
>>>   drivers/staging/btmtk_usb/btmtk_usb.c |  137
>>> +++++++++++++++++++++------------
>>>   1 file changed, 86 insertions(+), 51 deletions(-)
>>>
>>> diff --git a/drivers/staging/btmtk_usb/btmtk_usb.c
>>> b/drivers/staging/btmtk_usb/btmtk_usb.c
>>> index 0e783e8..0b3e593 100644
>>> --- a/drivers/staging/btmtk_usb/btmtk_usb.c
>>> +++ b/drivers/staging/btmtk_usb/btmtk_usb.c
>>> @@ -16,7 +16,8 @@
>>>    *  You should have received a copy of the GNU General Public License
>>>    *  along with this program; if not, write to the Free Software
>>>    *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
>>>  02111-1307  USA
>>> - *  or on the worldwide web at
>>> http://www.gnu.org/licenses/old-licenses/gpl-2.0.txt.
>>> + *  or on the worldwide web at
>>> http://www.gnu.org/licenses/old-licenses/
>>> + *  gpl-2.0.txt.
>>>    *
>>>    */
>>>
>>>
>>
>>  Maybe here is not a good idea to break the link. You could move all of
>> it in the next line or leave it as is.
>>
>>
>>  @@ -72,8 +73,9 @@ static int btmtk_usb_reset(struct usb_device *udev)
>>>         BT_DBG("%s\n", __func__);
>>>   -     ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), 0x01,
>>> DEVICE_VENDOR_REQUEST_OUT,
>>> -                                                 0x01, 0x00, NULL,
>>> 0x00, CONTROL_TIMEOUT_JIFFIES);
>>> +       ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), 0x01,
>>> +                       DEVICE_VENDOR_REQUEST_OUT, 0x01, 0x00, NULL,
>>> 0x00,
>>> +                       CONTROL_TIMEOUT_JIFFIES);
>>>         if (ret < 0) {
>>>                 BT_ERR("%s error(%d)\n", __func__, ret);
>>> @@ -92,13 +94,14 @@ static int btmtk_usb_io_read32(struct btmtk_usb_data
>>> *data, u32 reg, u32 *val)
>>>         struct usb_device *udev = data->udev;
>>>         int ret;
>>>   -     ret = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0), request,
>>> DEVICE_VENDOR_REQUEST_IN,
>>> -                                                 0x0, reg,
>>> data->io_buf, 4,
>>> +       ret = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0), request,
>>> +                       DEVICE_VENDOR_REQUEST_IN, 0x0, reg,
>>> data->io_buf, 4,
>>>
>>> CONTROL_TIMEOUT_JIFFIES);
>>>
>>>
>>
>>  Try that your changes lead to uniform identation, that means the above
>> could be:
>>
>>  +       ret = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0), request,
>>> +                       DEVICE_VENDOR_REQUEST_IN, 0x0, reg,
>>> data->io_buf, 4,
>>>  +                       CONTROL_TIMEOUT_JIFFIES);
>>>
>>
>>
>>          if (ret < 0) {
>>>
>>>                 *val = 0xffffffff;
>>> -               BT_ERR("%s error(%d), reg=%x, value=%x\n", __func__,
>>> ret, reg, *val);
>>> +               BT_ERR("%s error(%d), reg=%x, value=%x\n", __func__,
>>> ret, reg,
>>> +                               *val);
>>>                 return ret;
>>>         }
>>>   @@ -122,12 +125,13 @@ static int btmtk_usb_io_write32(struct
>>> btmtk_usb_data *data, u32 reg, u32 val)
>>>         index = (u16)reg;
>>>         value = val & 0x0000ffff;
>>>   -     ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), request,
>>> DEVICE_VENDOR_REQUEST_OUT,
>>> -                                                 value, index, NULL, 0,
>>> +       ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), request,
>>> +                       DEVICE_VENDOR_REQUEST_OUT, value, index, NULL, 0,
>>>
>>> CONTROL_TIMEOUT_JIFFIES);
>>>
>>>
>>
>> The same here as far as concerns the identation.
>>
>>
>>          if (ret < 0) {
>>> -               BT_ERR("%s error(%d), reg=%x, value=%x\n", __func__,
>>> ret, reg, val);
>>> +               BT_ERR("%s error(%d), reg=%x, value=%x\n", __func__,
>>> ret, reg,
>>> +                               val);
>>>                 return ret;
>>>         }
>>>   @@ -139,7 +143,8 @@ static int btmtk_usb_io_write32(struct
>>> btmtk_usb_data *data, u32 reg, u32 val)
>>>                                 value, index, NULL, 0,
>>> CONTROL_TIMEOUT_JIFFIES);
>>>         if (ret < 0) {
>>> -               BT_ERR("%s error(%d), reg=%x, value=%x\n", __func__,
>>> ret, reg, val);
>>> +               BT_ERR("%s error(%d), reg=%x, value=%x\n", __func__,
>>> ret, reg,
>>> +                               val);
>>>                 return ret;
>>>         }
>>>   @@ -186,13 +191,16 @@ static void btmtk_usb_cap_init(struct
>>> btmtk_usb_data *data)
>>>                 ret = request_firmware(&firmware, MT7650_FIRMWARE,
>>> &udev->dev);
>>>                 if (ret < 0) {
>>>                         if (ret == -ENOENT) {
>>> -                               BT_ERR("Firmware file \"%s\" not found
>>> \n", MT7650_FIRMWARE);
>>> +                               BT_ERR("Firmware file \"%s\" not found
>>> \n",
>>> +                                               MT7650_FIRMWARE);
>>>                         } else {
>>> -                               BT_ERR("Firmware file \"%s\" request
>>> failed (err=%d) \n",
>>> -                                       MT7650_FIRMWARE, ret);
>>> +                               BT_ERR("Firmware file \"%s\" request
>>> failed
>>> +                                               (err=%d) \n",
>>> MT7650_FIRMWARE,
>>> +
>>>
>>
>>  Here, by breaking the string you introduced another checkpatch warning.
>> Better leave it as it is.
>> Also, if ever you need to break a line do:
>>
>>
>> +                               BT_ERR("Firmware file \"%s\" request
>> failed"
>> +                                               "(err=%d) \n",
>> MT7650_FIRMWARE,
>>
>>
>>                                           ret);
>>>                         }
>>>                 } else {
>>> -                       BT_DBG("Firmware file \"%s\" Found \n",
>>> MT7650_FIRMWARE);
>>> +                       BT_DBG("Firmware file \"%s\" Found \n",
>>> +                                       MT7650_FIRMWARE);
>>>                         /* load firmware here */
>>>                         data->firmware = firmware;
>>>                         btmtk_usb_load_fw(data);
>>> @@ -205,10 +213,12 @@ static void btmtk_usb_cap_init(struct
>>> btmtk_usb_data *data)
>>>                 ret = request_firmware(&firmware, MT7662_FIRMWARE,
>>> &udev->dev);
>>>                 if (ret < 0) {
>>>                         if (ret == -ENOENT) {
>>> -                               BT_ERR("Firmware file \"%s\" not
>>> found\n", MT7662_FIRMWARE);
>>> +                               BT_ERR("Firmware file \"%s\" not
>>> found\n",
>>> +                                               MT7662_FIRMWARE);
>>>                         } else {
>>> -                               BT_ERR("Firmware file \"%s\" request
>>> failed (err=%d)\n",
>>> -                                       MT7662_FIRMWARE, ret);
>>> +                               BT_ERR("Firmware file \"%s\" request
>>> failed
>>> +                                               (err=%d)\n",
>>> MT7662_FIRMWARE,
>>> +
>>>
>>
>>  Here also you broke the string.
>>
>>
>>                                          ret);
>>>                         }
>>>                 } else {
>>>                     BT_DBG("Firmware file \"%s\" Found\n",
>>> MT7662_FIRMWARE);
>>> @@ -258,8 +268,8 @@ static int btmtk_usb_chk_crc(struct btmtk_usb_data
>>> *data, u32 checksum_len)
>>>         memmove(data->io_buf, &data->rom_patch_offset, 4);
>>>         memmove(&data->io_buf[4], &checksum_len, 4);
>>>   -     ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), 0x1,
>>> DEVICE_VENDOR_REQUEST_IN,
>>> -                                                 0x20, 0x00,
>>> data->io_buf, 8,
>>> +       ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), 0x1,
>>> +                       DEVICE_VENDOR_REQUEST_IN, 0x20, 0x00,
>>> data->io_buf, 8,
>>>
>>> CONTROL_TIMEOUT_JIFFIES);
>>>
>>
>>  Here you need to align it better, so that it can be more readable.
>>
>>
>>          if (ret < 0) {
>>> @@ -318,8 +328,8 @@ static int btmtk_usb_reset_wmt(struct btmtk_usb_data
>>> *data)
>>>         BT_DBG("%s\n", __func__);
>>>         ret = usb_control_msg(data->udev, usb_sndctrlpipe(data->udev,
>>> 0), 0x01,
>>> -                               DEVICE_CLASS_REQUEST_OUT, 0x12, 0x00,
>>> data->io_buf,
>>> -                               8, CONTROL_TIMEOUT_JIFFIES);
>>> +                               DEVICE_CLASS_REQUEST_OUT, 0x12, 0x00,
>>> +                               data->io_buf, 8,
>>> CONTROL_TIMEOUT_JIFFIES);
>>>         if (ret)
>>>                 BT_ERR("%s:(%d)\n", __func__, ret);
>>> @@ -350,7 +360,8 @@ static int btmtk_usb_load_rom_patch(struct
>>> btmtk_usb_data *data)
>>>         unsigned char phase;
>>>         void *buf;
>>>         char *pos;
>>> -       unsigned int pipe = usb_sndbulkpipe(data->udev,
>>> data->bulk_tx_ep->bEndpointAddress);
>>> +       unsigned int pipe = usb_sndbulkpipe(data->udev,
>>> +                       data->bulk_tx_ep->bEndpointAddress);
>>>         if (!data->firmware) {
>>>                 BT_ERR("%s:please assign a rom patch\n", __func__);
>>> @@ -391,7 +402,8 @@ load_patch_protect:
>>>                 goto error0;
>>>         }
>>>   -     buf = usb_alloc_coherent(data->udev, UPLOAD_PATCH_UNIT,
>>> GFP_ATOMIC, &data_dma);
>>> +       buf = usb_alloc_coherent(data->udev, UPLOAD_PATCH_UNIT,
>>> GFP_ATOMIC,
>>> +                       &data_dma);
>>>         if (!buf) {
>>>                 ret = -ENOMEM;
>>> @@ -409,7 +421,8 @@ load_patch_protect:
>>>         /* loading rom patch */
>>>         while (1) {
>>>                 s32 sent_len_max = UPLOAD_PATCH_UNIT - PATCH_HEADER_SIZE;
>>> -               sent_len = (patch_len - cur_len) >= sent_len_max ?
>>> sent_len_max : (patch_len - cur_len);
>>> +               sent_len = (patch_len - cur_len) >= sent_len_max ?
>>> sent_len_max
>>> +                       : (patch_len - cur_len);
>>>
>>
>>  Maybe here it would be more readable if you leave ':' in the previous
>> line. As far as i have noticed there is a tendency to leave the operators
>> in the previous lines.
>>
>>
>>                  BT_DBG("patch_len = %d\n", patch_len);
>>>                 BT_DBG("cur_len = %d\n", cur_len);
>>> @@ -442,10 +455,12 @@ load_patch_protect:
>>>                         pos[8] = phase;
>>>   -                     memcpy(&pos[9], data->firmware->data +
>>> PATCH_INFO_SIZE + cur_len, sent_len);
>>> +                       memcpy(&pos[9], data->firmware->data +
>>> PATCH_INFO_SIZE
>>> +                                       + cur_len, sent_len);
>>>   -                     BT_DBG("sent_len + PATCH_HEADER_SIZE = %d, phase
>>> = %d\n",
>>> -                                       sent_len + PATCH_HEADER_SIZE,
>>> phase);
>>> +                       BT_DBG("sent_len + PATCH_HEADER_SIZE = %d,
>>> +                                       phase = %d\n", sent_len +
>>> +                                       PATCH_HEADER_SIZE, phase);
>>>
>>>
>>
>>  Here, you also broke the string.
>>
>>
>>                          usb_fill_bulk_urb(urb,
>>>                                         data->udev,
>>> @@ -463,7 +478,8 @@ load_patch_protect:
>>>                         if (ret)
>>>                                 goto error2;
>>>   -                     if
>>> (!wait_for_completion_timeout(&sent_to_mcu_done, msecs_to_jiffies(1000))) {
>>> +                       if
>>> (!wait_for_completion_timeout(&sent_to_mcu_done,
>>> +                                               msecs_to_jiffies(1000)))
>>> {
>>>                                 usb_kill_urb(urb);
>>>                                 BT_ERR("upload rom_patch timeout\n");
>>>                                 goto error2;
>>> @@ -480,7 +496,8 @@ load_patch_protect:
>>>                 }
>>>         }
>>>   -     total_checksum = checksume16((u8 *)data->firmware->data +
>>> PATCH_INFO_SIZE, patch_len);
>>> +       total_checksum = checksume16((u8 *)data->firmware->data +
>>> +                       PATCH_INFO_SIZE, patch_len);
>>>         BT_DBG("Send checksum req..\n");
>>>   @@ -520,7 +537,8 @@ static int load_fw_iv(struct btmtk_usb_data *data)
>>>         memmove(buf, data->firmware->data + 32, 64);
>>>         ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), 0x01,
>>> -
>>> DEVICE_VENDOR_REQUEST_OUT, 0x12, 0x0, buf, 64,
>>> +
>>> DEVICE_VENDOR_REQUEST_OUT,
>>> +                                                 0x12, 0x0, buf, 64,
>>>
>>> CONTROL_TIMEOUT_JIFFIES);
>>>         if (ret < 0) {
>>> @@ -559,7 +577,8 @@ static int btmtk_usb_load_fw(struct btmtk_usb_data
>>> *data)
>>>         dma_addr_t data_dma;
>>>         int ret = 0, sent_len;
>>>         struct completion sent_to_mcu_done;
>>> -       unsigned int pipe = usb_sndbulkpipe(data->udev,
>>> data->bulk_tx_ep->bEndpointAddress);
>>> +       unsigned int pipe = usb_sndbulkpipe(data->udev,
>>> +                       data->bulk_tx_ep->bEndpointAddress);
>>>         if (!data->firmware) {
>>>                 BT_ERR("%s:please assign a fw\n", __func__);
>>> @@ -598,9 +617,11 @@ loadfw_protect:
>>>                         | (*(data->firmware->data + 5) << 8)
>>>                         | (*(data->firmware->data + 4));
>>>   -     fw_ver = (*(data->firmware->data + 11) << 8) |
>>> (*(data->firmware->data + 10));
>>> +       fw_ver = (*(data->firmware->data + 11) << 8) |
>>> +               (*(data->firmware->data + 10));
>>>   -     build_ver = (*(data->firmware->data + 9) << 8) |
>>> (*(data->firmware->data + 8));
>>> +       build_ver = (*(data->firmware->data + 9) << 8) |
>>> +               (*(data->firmware->data + 8));
>>>
>>
>>  The above would more readable if it was:
>>
>>  +       build_ver = (*(data->firmware->data + 9) << 8) |
>>> +                   (*(data->firmware->data + 8));
>>>
>>  That would be the result if you press tab to align (actually i don't
>> know how your editor is setup).
>>
>>
>>          BT_DBG("fw version:%d.%d.%02d ",
>>>                         (fw_ver & 0xf000) >> 8,
>>> @@ -657,7 +678,8 @@ loadfw_protect:
>>>         /* Loading ILM */
>>>         while (1) {
>>> -               sent_len = (ilm_len - cur_len) >= 14336 ? 14336 :
>>> (ilm_len - cur_len);
>>> +               sent_len = (ilm_len - cur_len) >= 14336 ? 14336
>>> +                       : (ilm_len - cur_len);
>>>
>>>
>>
>>  Here, better place ':' in the line above.
>>
>>
>>                  if (sent_len > 0) {
>>>                         packet_header &= ~(0xffffffff);
>>> @@ -665,7 +687,9 @@ loadfw_protect:
>>>                         packet_header = cpu_to_le32(packet_header);
>>>                         memmove(buf, &packet_header, 4);
>>> -                       memmove(buf + 4, data->firmware->data + 32 +
>>> cur_len, sent_len);
>>> +                       memmove(buf + 4, data->firmware->data + 32 +
>>> cur_len,
>>> +                                       sent_len);
>>> +
>>>                         /* U2M_PDMA descriptor */
>>>                         btmtk_usb_io_write32(data, 0x230, cur_len);
>>> @@ -693,7 +717,8 @@ loadfw_protect:
>>>                         if (ret)
>>>                                 goto error3;
>>>   -                     if
>>> (!wait_for_completion_timeout(&sent_to_mcu_done, msecs_to_jiffies(1000))) {
>>> +                       if
>>> (!wait_for_completion_timeout(&sent_to_mcu_done,
>>> +                                               msecs_to_jiffies(1000)))
>>> {
>>>                                 usb_kill_urb(urb);
>>>                                 BT_ERR("upload ilm fw timeout\n");
>>>                                 goto error3;
>>> @@ -714,7 +739,8 @@ loadfw_protect:
>>>         /* Loading DLM */
>>>         while (1) {
>>> -               sent_len = (dlm_len - cur_len) >= 14336 ? 14336 :
>>> (dlm_len - cur_len);
>>> +               sent_len = (dlm_len - cur_len) >= 14336 ? 14336
>>> +                       : (dlm_len - cur_len);
>>>                 if (sent_len > 0) {
>>>                         packet_header &= ~(0xffffffff);
>>> @@ -722,7 +748,8 @@ loadfw_protect:
>>>                         packet_header = cpu_to_le32(packet_header);
>>>                         memmove(buf, &packet_header, 4);
>>> -                       memmove(buf + 4, data->firmware->data + 32 +
>>> ilm_len + cur_len, sent_len);
>>> +                       memmove(buf + 4, data->firmware->data + 32 +
>>> ilm_len
>>> +                                       + cur_len, sent_len);
>>>
>>>
>>
>>  Here, also leave the operator '+' in the previous line.
>>
>>
>>                          /* U2M_PDMA descriptor */
>>>                         btmtk_usb_io_write32(data, 0x230, 0x80000 +
>>> cur_len);
>>> @@ -751,7 +778,8 @@ loadfw_protect:
>>>                         if (ret)
>>>                                 goto error3;
>>>   -                     if
>>> (!wait_for_completion_timeout(&sent_to_mcu_done, msecs_to_jiffies(1000))) {
>>> +                       if
>>> (!wait_for_completion_timeout(&sent_to_mcu_done,
>>> +                                               msecs_to_jiffies(1000)))
>>> {
>>>                                 usb_kill_urb(urb);
>>>                                 BT_ERR("upload dlm fw timeout\n");
>>>                                 goto error3;
>>> @@ -978,8 +1006,8 @@ static int btmtk_usb_submit_bulk_in_urb(struct
>>> hci_dev *hdev, gfp_t mem_flags)
>>>         pipe = usb_rcvbulkpipe(data->udev,
>>> data->bulk_rx_ep->bEndpointAddress);
>>>   -     usb_fill_bulk_urb(urb, data->udev, pipe,
>>> -                                       buf, size,
>>> btmtk_usb_bulk_in_complete, hdev);
>>> +       usb_fill_bulk_urb(urb, data->udev, pipe, buf, size,
>>> +                       btmtk_usb_bulk_in_complete, hdev);
>>>         urb->transfer_flags |= URB_FREE_BUFFER;
>>>   @@ -1015,7 +1043,8 @@ static void btmtk_usb_isoc_in_complete(struct
>>> urb *urb)
>>>         if (urb->status == 0) {
>>>                 for (i = 0; i < urb->number_of_packets; i++) {
>>>                         unsigned int offset =
>>> urb->iso_frame_desc[i].offset;
>>> -                       unsigned int length =
>>> urb->iso_frame_desc[i].actual_length;
>>> +                       unsigned int length = urb->iso_frame_desc[i]
>>> +                               .actual_length;
>>>
>>
>>  You cannot do that, you cannot separate the structure from its field.
>> You can either leave it as it is or do:
>>
>>>  +                       unsigned int length =
>>>  +                                urb->iso_frame_desc[i].actual_length;
>>>
>>
>>
>>                          if (urb->iso_frame_desc[i].status)
>>>                                 continue;
>>> @@ -1096,8 +1125,9 @@ static int btmtk_usb_submit_isoc_in_urb(struct
>>> hci_dev *hdev, gfp_t mem_flags)
>>>         pipe = usb_rcvisocpipe(data->udev,
>>> data->isoc_rx_ep->bEndpointAddress);
>>>   -     usb_fill_int_urb(urb, data->udev, pipe, buf, size,
>>> btmtk_usb_isoc_in_complete,
>>> -                               hdev, data->isoc_rx_ep->bInterval);
>>> +       usb_fill_int_urb(urb, data->udev, pipe, buf, size,
>>> +                       btmtk_usb_isoc_in_complete, hdev,
>>> +                       data->isoc_rx_ep->bInterval);
>>>         urb->transfer_flags  = URB_FREE_BUFFER | URB_ISO_ASAP;
>>>   @@ -1306,7 +1336,8 @@ static int btmtk_usb_send_frame(struct sk_buff
>>> *skb)
>>>                 }
>>>                 usb_fill_control_urb(urb, data->udev, pipe, (void *) dr,
>>> -                               skb->data, skb->len,
>>> btmtk_usb_tx_complete, skb);
>>> +                               skb->data, skb->len,
>>> btmtk_usb_tx_complete,
>>> +                               skb);
>>>                 hdev->stat.cmd_tx++;
>>>                 break;
>>> @@ -1322,8 +1353,8 @@ static int btmtk_usb_send_frame(struct sk_buff
>>> *skb)
>>>                 pipe = usb_sndbulkpipe(data->udev,
>>>
>>> data->bulk_tx_ep->bEndpointAddress);
>>>   -             usb_fill_bulk_urb(urb, data->udev, pipe,
>>> -                               skb->data, skb->len,
>>> btmtk_usb_tx_complete, skb);
>>> +               usb_fill_bulk_urb(urb, data->udev, pipe, skb->data,
>>> skb->len,
>>> +                               btmtk_usb_tx_complete, skb);
>>>                 hdev->stat.acl_tx++;
>>>                 BT_DBG("HCI_ACLDATA_PKT:\n");
>>> @@ -1442,7 +1473,8 @@ static inline int __set_isoc_interface(struct
>>> hci_dev *hdev, int altsetting)
>>>     static void btmtk_usb_work(struct work_struct *work)
>>>   {
>>> -       struct btmtk_usb_data *data = container_of(work, struct
>>> btmtk_usb_data, work);
>>> +       struct btmtk_usb_data *data = container_of(work, struct
>>> btmtk_usb_data,
>>> +                       work);
>>>         struct hci_dev *hdev = data->hdev;
>>>         int new_alts;
>>>         int err;
>>> @@ -1451,7 +1483,8 @@ static void btmtk_usb_work(struct work_struct
>>> *work)
>>>         if (hdev->conn_hash.sco_num > 0) {
>>>                 if (!test_bit(BTUSB_DID_ISO_RESUME, &data->flags)) {
>>> -                       err = usb_autopm_get_interface(data->isoc ?
>>> data->isoc : data->intf);
>>> +                       err = usb_autopm_get_interface(data->isoc ?
>>> data->isoc
>>> +                                       : data->intf);
>>>                         if (err < 0) {
>>>                                 clear_bit(BTUSB_ISOC_RUNNING,
>>> &data->flags);
>>>
>>> usb_kill_anchored_urbs(&data->isoc_anchor);
>>> @@ -1489,13 +1522,15 @@ static void btmtk_usb_work(struct work_struct
>>> *work)
>>>                 __set_isoc_interface(hdev, 0);
>>>                 if (test_and_clear_bit(BTUSB_DID_ISO_RESUME,
>>> &data->flags))
>>> -                        usb_autopm_put_interface(data->isoc ?
>>> data->isoc : data->intf);
>>> +                        usb_autopm_put_interface(data->isoc ? data->isoc
>>> +                                        : data->intf);
>>>         }
>>>   }
>>>     static void btmtk_usb_waker(struct work_struct *work)
>>>   {
>>> -       struct btmtk_usb_data *data = container_of(work, struct
>>> btmtk_usb_data, waker);
>>> +       struct btmtk_usb_data *data = container_of(work, struct
>>> btmtk_usb_data,
>>> +                       waker);
>>>         int err;
>>>         err = usb_autopm_get_interface(data->intf);
>>>
>>
>>  I have not commended them all but the idea is the same. Good work by
>> the way :)
>> Keep in mind to change the code in order to be more readable without
>> violating the kernel coding style (significantly).
>>
>> ksenia
>>
>
>
>
> --
>  Rashika Kheria
>  B.Tech CSE
>  IIIT Hyderabad
>
>
>
Xenia Ragiadakou Oct. 5, 2013, 2:02 p.m. UTC | #5
On 10/05/2013 04:54 PM, Rashika Kheria wrote:
> Hi Xenia,
>
> Thanks for your early response. I will rectify the mistakes.
>
> Thanks,
> Rashika Kheria
>

One last thing, now that you will resend your patches put a revision 
number (with -v option in format-patch) so that the subject line will be 
[PATCH v2]. Another thing is that when you send a revision of a patch 
you need to state under --- (below your Signed-off-by line) the 
differences from your previous version. At this step maybe it is not 
necessary but it is a good practice to do in general.

ksenia

>
> On Sat, Oct 5, 2013 at 7:20 PM, Xenia Ragiadakou 
> <burzalodowa@gmail.com <mailto:burzalodowa@gmail.com>> wrote:
>
>     On 10/05/2013 04:43 PM, Rashika Kheria wrote:
>>     Hi Xenia,
>>
>>     Thank you for your feedback.
>>
>>     I tried to incorporate the name of the driver in the subject line
>>     but since the subject line could contain only 50 characters,
>>     therefore, I was unable to add the same. Kindly guide me to
>>     rectify this issue.
>>
>>     I will upload another version to correcting the other suggested
>>     changes.
>>
>>     Thanks in advance.
>>
>>     Rashika Kheria
>>
>>
>
>     Yeah, i understand but i think it is better to include the driver
>     name even if the subject line exceeds (a little) 50 chars. Maybe,
>     in this case, you can arrange your subject to be shorter, i don't
>     know "limit line below 80 chars" :) well.. i don't know and my
>     english is bad. You will find a way I m sure :)
>
>     ksenia
>>     On Sat, Oct 5, 2013 at 7:04 PM, Xenia Ragiadakou
>>     <burzalodowa@gmail.com <mailto:burzalodowa@gmail.com>> wrote:
>>
>>         Hi Rashika,
>>
>>         I noticed that you corrected the subject line by yourself.
>>         Good job!
>>         It would be nice to add the name of the driver on the subject
>>         line too, so that somebody will can infer quickly to which
>>         driver this patch applies. For example, you can write in the
>>         future:
>>         Staging: btmtk_usb: Fix line length exceeding 80 characters
>>
>>         Also you need to add a changelog in your patch, which will
>>         state why you did that change and which tool you used to find
>>         the bad coding style (if you used one) for example:
>>         This patch fixes the following checkpatch.pl
>>         <http://checkpatch.pl> warning/error: <warning/error>
>>
>>         I have added some other comments below.
>>
>>
>>         On 10/05/2013 02:55 PM, Rashika wrote:
>>
>>             Signed-off-by: Rashika <rashika.kheria@gmail.com
>>             <mailto:rashika.kheria@gmail.com>>
>>             ---
>>               drivers/staging/btmtk_usb/btmtk_usb.c |  137
>>             +++++++++++++++++++++------------
>>               1 file changed, 86 insertions(+), 51 deletions(-)
>>
>>             diff --git a/drivers/staging/btmtk_usb/btmtk_usb.c
>>             b/drivers/staging/btmtk_usb/btmtk_usb.c
>>             index 0e783e8..0b3e593 100644
>>             --- a/drivers/staging/btmtk_usb/btmtk_usb.c
>>             +++ b/drivers/staging/btmtk_usb/btmtk_usb.c
>>             @@ -16,7 +16,8 @@
>>                *  You should have received a copy of the GNU General
>>             Public License
>>                *  along with this program; if not, write to the Free
>>             Software
>>                *  Foundation, Inc., 59 Temple Place, Suite 330,
>>             Boston, MA  02111-1307  USA
>>             - *  or on the worldwide web at
>>             http://www.gnu.org/licenses/old-licenses/gpl-2.0.txt.
>>             + *  or on the worldwide web at
>>             http://www.gnu.org/licenses/old-licenses/
>>             + *  gpl-2.0.txt.
>>                *
>>                */
>>
>>
>>         Maybe here is not a good idea to break the link. You could
>>         move all of it in the next line or leave it as is.
>>
>>
>>             @@ -72,8 +73,9 @@ static int btmtk_usb_reset(struct
>>             usb_device *udev)
>>                     BT_DBG("%s\n", __func__);
>>               -     ret = usb_control_msg(udev, usb_sndctrlpipe(udev,
>>             0), 0x01, DEVICE_VENDOR_REQUEST_OUT,
>>             -         0x01, 0x00, NULL, 0x00, CONTROL_TIMEOUT_JIFFIES);
>>             +       ret = usb_control_msg(udev, usb_sndctrlpipe(udev,
>>             0), 0x01,
>>             + DEVICE_VENDOR_REQUEST_OUT, 0x01, 0x00, NULL, 0x00,
>>             + CONTROL_TIMEOUT_JIFFIES);
>>                     if (ret < 0) {
>>                             BT_ERR("%s error(%d)\n", __func__, ret);
>>             @@ -92,13 +94,14 @@ static int btmtk_usb_io_read32(struct
>>             btmtk_usb_data *data, u32 reg, u32 *val)
>>                     struct usb_device *udev = data->udev;
>>                     int ret;
>>               -     ret = usb_control_msg(udev, usb_rcvctrlpipe(udev,
>>             0), request, DEVICE_VENDOR_REQUEST_IN,
>>             -         0x0, reg, data->io_buf, 4,
>>             +       ret = usb_control_msg(udev, usb_rcvctrlpipe(udev,
>>             0), request,
>>             + DEVICE_VENDOR_REQUEST_IN, 0x0, reg, data->io_buf, 4,
>>                     CONTROL_TIMEOUT_JIFFIES);
>>
>>
>>         Try that your changes lead to uniform identation, that means
>>         the above could be:
>>
>>             +       ret = usb_control_msg(udev, usb_rcvctrlpipe(udev,
>>             0), request,
>>             + DEVICE_VENDOR_REQUEST_IN, 0x0, reg, data->io_buf, 4,
>>             + CONTROL_TIMEOUT_JIFFIES);
>>
>>
>>
>>                     if (ret < 0) {
>>
>>                             *val = 0xffffffff;
>>             -               BT_ERR("%s error(%d), reg=%x,
>>             value=%x\n", __func__, ret, reg, *val);
>>             +               BT_ERR("%s error(%d), reg=%x,
>>             value=%x\n", __func__, ret, reg,
>>             +                               *val);
>>                             return ret;
>>                     }
>>               @@ -122,12 +125,13 @@ static int
>>             btmtk_usb_io_write32(struct btmtk_usb_data *data, u32
>>             reg, u32 val)
>>                     index = (u16)reg;
>>                     value = val & 0x0000ffff;
>>               -     ret = usb_control_msg(udev, usb_sndctrlpipe(udev,
>>             0), request, DEVICE_VENDOR_REQUEST_OUT,
>>             -         value, index, NULL, 0,
>>             +       ret = usb_control_msg(udev, usb_sndctrlpipe(udev,
>>             0), request,
>>             + DEVICE_VENDOR_REQUEST_OUT, value, index, NULL, 0,
>>                     CONTROL_TIMEOUT_JIFFIES);
>>
>>
>>         The same here as far as concerns the identation.
>>
>>
>>                     if (ret < 0) {
>>             -               BT_ERR("%s error(%d), reg=%x,
>>             value=%x\n", __func__, ret, reg, val);
>>             +               BT_ERR("%s error(%d), reg=%x,
>>             value=%x\n", __func__, ret, reg,
>>             +                               val);
>>                             return ret;
>>                     }
>>               @@ -139,7 +143,8 @@ static int
>>             btmtk_usb_io_write32(struct btmtk_usb_data *data, u32
>>             reg, u32 val)
>>                                             value, index, NULL, 0,
>>             CONTROL_TIMEOUT_JIFFIES);
>>                     if (ret < 0) {
>>             -               BT_ERR("%s error(%d), reg=%x,
>>             value=%x\n", __func__, ret, reg, val);
>>             +               BT_ERR("%s error(%d), reg=%x,
>>             value=%x\n", __func__, ret, reg,
>>             +                               val);
>>                             return ret;
>>                     }
>>               @@ -186,13 +191,16 @@ static void
>>             btmtk_usb_cap_init(struct btmtk_usb_data *data)
>>                             ret = request_firmware(&firmware,
>>             MT7650_FIRMWARE, &udev->dev);
>>                             if (ret < 0) {
>>                                     if (ret == -ENOENT) {
>>             - BT_ERR("Firmware file \"%s\" not found \n",
>>             MT7650_FIRMWARE);
>>             + BT_ERR("Firmware file \"%s\" not found \n",
>>             +         MT7650_FIRMWARE);
>>                                     } else {
>>             - BT_ERR("Firmware file \"%s\" request failed (err=%d) \n",
>>             - MT7650_FIRMWARE, ret);
>>             + BT_ERR("Firmware file \"%s\" request failed
>>             +         (err=%d) \n", MT7650_FIRMWARE,
>>             +
>>
>>
>>         Here, by breaking the string you introduced another
>>         checkpatch warning. Better leave it as it is.
>>         Also, if ever you need to break a line do:
>>
>>
>>         + BT_ERR("Firmware file \"%s\" request failed"
>>         +     "(err=%d) \n", MT7650_FIRMWARE,
>>
>>
>>                                         ret);
>>                                     }
>>                             } else {
>>             -                       BT_DBG("Firmware file \"%s\"
>>             Found \n", MT7650_FIRMWARE);
>>             +                       BT_DBG("Firmware file \"%s\"
>>             Found \n",
>>             + MT7650_FIRMWARE);
>>                                     /* load firmware here */
>>                                     data->firmware = firmware;
>>             btmtk_usb_load_fw(data);
>>             @@ -205,10 +213,12 @@ static void
>>             btmtk_usb_cap_init(struct btmtk_usb_data *data)
>>                             ret = request_firmware(&firmware,
>>             MT7662_FIRMWARE, &udev->dev);
>>                             if (ret < 0) {
>>                                     if (ret == -ENOENT) {
>>             - BT_ERR("Firmware file \"%s\" not found\n",
>>             MT7662_FIRMWARE);
>>             + BT_ERR("Firmware file \"%s\" not found\n",
>>             +       MT7662_FIRMWARE);
>>                                     } else {
>>             - BT_ERR("Firmware file \"%s\" request failed (err=%d)\n",
>>             - MT7662_FIRMWARE, ret);
>>             + BT_ERR("Firmware file \"%s\" request failed
>>             +       (err=%d)\n", MT7662_FIRMWARE,
>>             +
>>
>>
>>         Here also you broke the string.
>>
>>
>>                                         ret);
>>                                     }
>>                             } else {
>>                                 BT_DBG("Firmware file \"%s\"
>>             Found\n", MT7662_FIRMWARE);
>>             @@ -258,8 +268,8 @@ static int btmtk_usb_chk_crc(struct
>>             btmtk_usb_data *data, u32 checksum_len)
>>                     memmove(data->io_buf, &data->rom_patch_offset, 4);
>>                     memmove(&data->io_buf[4], &checksum_len, 4);
>>               -     ret = usb_control_msg(udev, usb_sndctrlpipe(udev,
>>             0), 0x1, DEVICE_VENDOR_REQUEST_IN,
>>             -         0x20, 0x00, data->io_buf, 8,
>>             +       ret = usb_control_msg(udev, usb_sndctrlpipe(udev,
>>             0), 0x1,
>>             + DEVICE_VENDOR_REQUEST_IN, 0x20, 0x00, data->io_buf, 8,
>>                     CONTROL_TIMEOUT_JIFFIES);
>>
>>
>>         Here you need to align it better, so that it can be more
>>         readable.
>>
>>
>>                     if (ret < 0) {
>>             @@ -318,8 +328,8 @@ static int btmtk_usb_reset_wmt(struct
>>             btmtk_usb_data *data)
>>                     BT_DBG("%s\n", __func__);
>>                     ret = usb_control_msg(data->udev,
>>             usb_sndctrlpipe(data->udev, 0), 0x01,
>>             - DEVICE_CLASS_REQUEST_OUT, 0x12, 0x00, data->io_buf,
>>             -                               8, CONTROL_TIMEOUT_JIFFIES);
>>             + DEVICE_CLASS_REQUEST_OUT, 0x12, 0x00,
>>             + data->io_buf, 8, CONTROL_TIMEOUT_JIFFIES);
>>                     if (ret)
>>                             BT_ERR("%s:(%d)\n", __func__, ret);
>>             @@ -350,7 +360,8 @@ static int
>>             btmtk_usb_load_rom_patch(struct btmtk_usb_data *data)
>>                     unsigned char phase;
>>                     void *buf;
>>                     char *pos;
>>             -       unsigned int pipe = usb_sndbulkpipe(data->udev,
>>             data->bulk_tx_ep->bEndpointAddress);
>>             +       unsigned int pipe = usb_sndbulkpipe(data->udev,
>>             + data->bulk_tx_ep->bEndpointAddress);
>>                     if (!data->firmware) {
>>                             BT_ERR("%s:please assign a rom patch\n",
>>             __func__);
>>             @@ -391,7 +402,8 @@ load_patch_protect:
>>                             goto error0;
>>                     }
>>               -     buf = usb_alloc_coherent(data->udev,
>>             UPLOAD_PATCH_UNIT, GFP_ATOMIC, &data_dma);
>>             +       buf = usb_alloc_coherent(data->udev,
>>             UPLOAD_PATCH_UNIT, GFP_ATOMIC,
>>             +                       &data_dma);
>>                     if (!buf) {
>>                             ret = -ENOMEM;
>>             @@ -409,7 +421,8 @@ load_patch_protect:
>>                     /* loading rom patch */
>>                     while (1) {
>>                             s32 sent_len_max = UPLOAD_PATCH_UNIT -
>>             PATCH_HEADER_SIZE;
>>             -               sent_len = (patch_len - cur_len) >=
>>             sent_len_max ? sent_len_max : (patch_len - cur_len);
>>             +               sent_len = (patch_len - cur_len) >=
>>             sent_len_max ? sent_len_max
>>             +                       : (patch_len - cur_len);
>>
>>
>>         Maybe here it would be more readable if you leave ':' in the
>>         previous line. As far as i have noticed there is a tendency
>>         to leave the operators in the previous lines.
>>
>>
>>                 BT_DBG("patch_len = %d\n", patch_len);
>>                             BT_DBG("cur_len = %d\n", cur_len);
>>             @@ -442,10 +455,12 @@ load_patch_protect:
>>                                     pos[8] = phase;
>>               - memcpy(&pos[9], data->firmware->data +
>>             PATCH_INFO_SIZE + cur_len, sent_len);
>>             + memcpy(&pos[9], data->firmware->data + PATCH_INFO_SIZE
>>             +                                       + cur_len, sent_len);
>>               -                     BT_DBG("sent_len +
>>             PATCH_HEADER_SIZE = %d, phase = %d\n",
>>             - sent_len + PATCH_HEADER_SIZE, phase);
>>             +                       BT_DBG("sent_len +
>>             PATCH_HEADER_SIZE = %d,
>>             + phase = %d\n", sent_len +
>>             + PATCH_HEADER_SIZE, phase);
>>
>>
>>         Here, you also broke the string.
>>
>>
>>                           usb_fill_bulk_urb(urb,
>>             data->udev,
>>             @@ -463,7 +478,8 @@ load_patch_protect:
>>                                     if (ret)
>>                                             goto error2;
>>               -                     if
>>             (!wait_for_completion_timeout(&sent_to_mcu_done,
>>             msecs_to_jiffies(1000))) {
>>             +                       if
>>             (!wait_for_completion_timeout(&sent_to_mcu_done,
>>             +         msecs_to_jiffies(1000))) {
>>             usb_kill_urb(urb);
>>             BT_ERR("upload rom_patch timeout\n");
>>                                             goto error2;
>>             @@ -480,7 +496,8 @@ load_patch_protect:
>>                             }
>>                     }
>>               -     total_checksum = checksume16((u8
>>             *)data->firmware->data + PATCH_INFO_SIZE, patch_len);
>>             +       total_checksum = checksume16((u8
>>             *)data->firmware->data +
>>             +                       PATCH_INFO_SIZE, patch_len);
>>                     BT_DBG("Send checksum req..\n");
>>               @@ -520,7 +537,8 @@ static int load_fw_iv(struct
>>             btmtk_usb_data *data)
>>                     memmove(buf, data->firmware->data + 32, 64);
>>                     ret = usb_control_msg(udev, usb_sndctrlpipe(udev,
>>             0), 0x01,
>>             -           DEVICE_VENDOR_REQUEST_OUT, 0x12, 0x0, buf, 64,
>>             +           DEVICE_VENDOR_REQUEST_OUT,
>>             +           0x12, 0x0, buf, 64,
>>                       CONTROL_TIMEOUT_JIFFIES);
>>                     if (ret < 0) {
>>             @@ -559,7 +577,8 @@ static int btmtk_usb_load_fw(struct
>>             btmtk_usb_data *data)
>>                     dma_addr_t data_dma;
>>                     int ret = 0, sent_len;
>>                     struct completion sent_to_mcu_done;
>>             -       unsigned int pipe = usb_sndbulkpipe(data->udev,
>>             data->bulk_tx_ep->bEndpointAddress);
>>             +       unsigned int pipe = usb_sndbulkpipe(data->udev,
>>             + data->bulk_tx_ep->bEndpointAddress);
>>                     if (!data->firmware) {
>>                             BT_ERR("%s:please assign a fw\n", __func__);
>>             @@ -598,9 +617,11 @@ loadfw_protect:
>>                                     | (*(data->firmware->data + 5) << 8)
>>                                     | (*(data->firmware->data + 4));
>>               -     fw_ver = (*(data->firmware->data + 11) << 8) |
>>             (*(data->firmware->data + 10));
>>             +       fw_ver = (*(data->firmware->data + 11) << 8) |
>>             + (*(data->firmware->data + 10));
>>               -     build_ver = (*(data->firmware->data + 9) << 8) |
>>             (*(data->firmware->data + 8));
>>             +       build_ver = (*(data->firmware->data + 9) << 8) |
>>             + (*(data->firmware->data + 8));
>>
>>
>>         The above would more readable if it was:
>>
>>             + build_ver = (*(data->firmware->data + 9) << 8) |
>>             + (*(data->firmware->data + 8));
>>
>>         That would be the result if you press tab to align (actually
>>         i don't know how your editor is setup).
>>
>>
>>             BT_DBG("fw version:%d.%d.%02d ",
>>                                     (fw_ver & 0xf000) >> 8,
>>             @@ -657,7 +678,8 @@ loadfw_protect:
>>                     /* Loading ILM */
>>                     while (1) {
>>             -               sent_len = (ilm_len - cur_len) >= 14336 ?
>>             14336 : (ilm_len - cur_len);
>>             +               sent_len = (ilm_len - cur_len) >= 14336 ?
>>             14336
>>             +                       : (ilm_len - cur_len);
>>
>>
>>         Here, better place ':' in the line above.
>>
>>
>>                   if (sent_len > 0) {
>>                                     packet_header &= ~(0xffffffff);
>>             @@ -665,7 +687,9 @@ loadfw_protect:
>>                                     packet_header =
>>             cpu_to_le32(packet_header);
>>                                     memmove(buf, &packet_header, 4);
>>             -                       memmove(buf + 4,
>>             data->firmware->data + 32 + cur_len, sent_len);
>>             +                       memmove(buf + 4,
>>             data->firmware->data + 32 + cur_len,
>>             + sent_len);
>>             +
>>                                     /* U2M_PDMA descriptor */
>>             btmtk_usb_io_write32(data, 0x230, cur_len);
>>             @@ -693,7 +717,8 @@ loadfw_protect:
>>                                     if (ret)
>>                                             goto error3;
>>               -                     if
>>             (!wait_for_completion_timeout(&sent_to_mcu_done,
>>             msecs_to_jiffies(1000))) {
>>             +                       if
>>             (!wait_for_completion_timeout(&sent_to_mcu_done,
>>             +         msecs_to_jiffies(1000))) {
>>             usb_kill_urb(urb);
>>             BT_ERR("upload ilm fw timeout\n");
>>                                             goto error3;
>>             @@ -714,7 +739,8 @@ loadfw_protect:
>>                     /* Loading DLM */
>>                     while (1) {
>>             -               sent_len = (dlm_len - cur_len) >= 14336 ?
>>             14336 : (dlm_len - cur_len);
>>             +               sent_len = (dlm_len - cur_len) >= 14336 ?
>>             14336
>>             +                       : (dlm_len - cur_len);
>>                             if (sent_len > 0) {
>>                                     packet_header &= ~(0xffffffff);
>>             @@ -722,7 +748,8 @@ loadfw_protect:
>>                                     packet_header =
>>             cpu_to_le32(packet_header);
>>                                     memmove(buf, &packet_header, 4);
>>             -                       memmove(buf + 4,
>>             data->firmware->data + 32 + ilm_len + cur_len, sent_len);
>>             +                       memmove(buf + 4,
>>             data->firmware->data + 32 + ilm_len
>>             + + cur_len, sent_len);
>>
>>
>>         Here, also leave the operator '+' in the previous line.
>>
>>
>>                           /* U2M_PDMA descriptor */
>>             btmtk_usb_io_write32(data, 0x230, 0x80000 + cur_len);
>>             @@ -751,7 +778,8 @@ loadfw_protect:
>>                                     if (ret)
>>                                             goto error3;
>>               -                     if
>>             (!wait_for_completion_timeout(&sent_to_mcu_done,
>>             msecs_to_jiffies(1000))) {
>>             +                       if
>>             (!wait_for_completion_timeout(&sent_to_mcu_done,
>>             +         msecs_to_jiffies(1000))) {
>>             usb_kill_urb(urb);
>>             BT_ERR("upload dlm fw timeout\n");
>>                                             goto error3;
>>             @@ -978,8 +1006,8 @@ static int
>>             btmtk_usb_submit_bulk_in_urb(struct hci_dev *hdev, gfp_t
>>             mem_flags)
>>                     pipe = usb_rcvbulkpipe(data->udev,
>>             data->bulk_rx_ep->bEndpointAddress);
>>               -     usb_fill_bulk_urb(urb, data->udev, pipe,
>>             - buf, size, btmtk_usb_bulk_in_complete, hdev);
>>             +       usb_fill_bulk_urb(urb, data->udev, pipe, buf, size,
>>             + btmtk_usb_bulk_in_complete, hdev);
>>                     urb->transfer_flags |= URB_FREE_BUFFER;
>>               @@ -1015,7 +1043,8 @@ static void
>>             btmtk_usb_isoc_in_complete(struct urb *urb)
>>                     if (urb->status == 0) {
>>                             for (i = 0; i < urb->number_of_packets;
>>             i++) {
>>                                     unsigned int offset =
>>             urb->iso_frame_desc[i].offset;
>>             -                       unsigned int length =
>>             urb->iso_frame_desc[i].actual_length;
>>             +                       unsigned int length =
>>             urb->iso_frame_desc[i]
>>             + .actual_length;
>>
>>
>>         You cannot do that, you cannot separate the structure from
>>         its field. You can either leave it as it is or do:
>>
>>             +                       unsigned int length =
>>             +  urb->iso_frame_desc[i].actual_length;
>>
>>
>>
>>                           if (urb->iso_frame_desc[i].status)
>>             continue;
>>             @@ -1096,8 +1125,9 @@ static int
>>             btmtk_usb_submit_isoc_in_urb(struct hci_dev *hdev, gfp_t
>>             mem_flags)
>>                     pipe = usb_rcvisocpipe(data->udev,
>>             data->isoc_rx_ep->bEndpointAddress);
>>               -     usb_fill_int_urb(urb, data->udev, pipe, buf,
>>             size, btmtk_usb_isoc_in_complete,
>>             -                               hdev,
>>             data->isoc_rx_ep->bInterval);
>>             +       usb_fill_int_urb(urb, data->udev, pipe, buf, size,
>>             + btmtk_usb_isoc_in_complete, hdev,
>>             + data->isoc_rx_ep->bInterval);
>>                     urb->transfer_flags  = URB_FREE_BUFFER |
>>             URB_ISO_ASAP;
>>               @@ -1306,7 +1336,8 @@ static int
>>             btmtk_usb_send_frame(struct sk_buff *skb)
>>                             }
>>             usb_fill_control_urb(urb, data->udev, pipe, (void *) dr,
>>             - skb->data, skb->len, btmtk_usb_tx_complete, skb);
>>             + skb->data, skb->len, btmtk_usb_tx_complete,
>>             +                               skb);
>>                             hdev->stat.cmd_tx++;
>>                             break;
>>             @@ -1322,8 +1353,8 @@ static int
>>             btmtk_usb_send_frame(struct sk_buff *skb)
>>                             pipe = usb_sndbulkpipe(data->udev,
>>             data->bulk_tx_ep->bEndpointAddress);
>>               -             usb_fill_bulk_urb(urb, data->udev, pipe,
>>             - skb->data, skb->len, btmtk_usb_tx_complete, skb);
>>             +               usb_fill_bulk_urb(urb, data->udev, pipe,
>>             skb->data, skb->len,
>>             + btmtk_usb_tx_complete, skb);
>>                             hdev->stat.acl_tx++;
>>             BT_DBG("HCI_ACLDATA_PKT:\n");
>>             @@ -1442,7 +1473,8 @@ static inline int
>>             __set_isoc_interface(struct hci_dev *hdev, int altsetting)
>>                 static void btmtk_usb_work(struct work_struct *work)
>>               {
>>             -       struct btmtk_usb_data *data = container_of(work,
>>             struct btmtk_usb_data, work);
>>             +       struct btmtk_usb_data *data = container_of(work,
>>             struct btmtk_usb_data,
>>             +                       work);
>>                     struct hci_dev *hdev = data->hdev;
>>                     int new_alts;
>>                     int err;
>>             @@ -1451,7 +1483,8 @@ static void btmtk_usb_work(struct
>>             work_struct *work)
>>                     if (hdev->conn_hash.sco_num > 0) {
>>                             if (!test_bit(BTUSB_DID_ISO_RESUME,
>>             &data->flags)) {
>>             -                       err =
>>             usb_autopm_get_interface(data->isoc ? data->isoc :
>>             data->intf);
>>             +                       err =
>>             usb_autopm_get_interface(data->isoc ? data->isoc
>>             + : data->intf);
>>                                     if (err < 0) {
>>             clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
>>             usb_kill_anchored_urbs(&data->isoc_anchor);
>>             @@ -1489,13 +1522,15 @@ static void btmtk_usb_work(struct
>>             work_struct *work)
>>             __set_isoc_interface(hdev, 0);
>>                             if
>>             (test_and_clear_bit(BTUSB_DID_ISO_RESUME, &data->flags))
>>             -  usb_autopm_put_interface(data->isoc ? data->isoc :
>>             data->intf);
>>             +  usb_autopm_put_interface(data->isoc ? data->isoc
>>             +  : data->intf);
>>                     }
>>               }
>>                 static void btmtk_usb_waker(struct work_struct *work)
>>               {
>>             -       struct btmtk_usb_data *data = container_of(work,
>>             struct btmtk_usb_data, waker);
>>             +       struct btmtk_usb_data *data = container_of(work,
>>             struct btmtk_usb_data,
>>             +                       waker);
>>                     int err;
>>                     err = usb_autopm_get_interface(data->intf);
>>
>>
>>         I have not commended them all but the idea is the same. Good
>>         work by the way :)
>>         Keep in mind to change the code in order to be more readable
>>         without violating the kernel coding style (significantly).
>>
>>         ksenia
>>
>>
>>
>>
>>     -- 
>>     Rashika Kheria
>>     B.Tech CSE
>>     IIIT Hyderabad
>
>
>
>
> -- 
> Rashika Kheria
> B.Tech CSE
> IIIT Hyderabad

Patch
diff mbox

diff --git a/drivers/staging/btmtk_usb/btmtk_usb.c b/drivers/staging/btmtk_usb/btmtk_usb.c
index 0e783e8..0b3e593 100644
--- a/drivers/staging/btmtk_usb/btmtk_usb.c
+++ b/drivers/staging/btmtk_usb/btmtk_usb.c
@@ -16,7 +16,8 @@ 
  *  You should have received a copy of the GNU General Public License
  *  along with this program; if not, write to the Free Software
  *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
- *  or on the worldwide web at http://www.gnu.org/licenses/old-licenses/gpl-2.0.txt.
+ *  or on the worldwide web at http://www.gnu.org/licenses/old-licenses/
+ *  gpl-2.0.txt.
  *
  */
 
@@ -72,8 +73,9 @@  static int btmtk_usb_reset(struct usb_device *udev)
 
 	BT_DBG("%s\n", __func__);
 
-	ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), 0x01, DEVICE_VENDOR_REQUEST_OUT,
-						  0x01, 0x00, NULL, 0x00, CONTROL_TIMEOUT_JIFFIES);
+	ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), 0x01,
+			DEVICE_VENDOR_REQUEST_OUT, 0x01, 0x00, NULL, 0x00,
+			CONTROL_TIMEOUT_JIFFIES);
 
 	if (ret < 0) {
 		BT_ERR("%s error(%d)\n", __func__, ret);
@@ -92,13 +94,14 @@  static int btmtk_usb_io_read32(struct btmtk_usb_data *data, u32 reg, u32 *val)
 	struct usb_device *udev = data->udev;
 	int ret;
 
-	ret = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0), request, DEVICE_VENDOR_REQUEST_IN,
-						  0x0, reg, data->io_buf, 4,
+	ret = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0), request,
+			DEVICE_VENDOR_REQUEST_IN, 0x0, reg, data->io_buf, 4,
 						  CONTROL_TIMEOUT_JIFFIES);
 
 	if (ret < 0) {
 		*val = 0xffffffff;
-		BT_ERR("%s error(%d), reg=%x, value=%x\n", __func__, ret, reg, *val);
+		BT_ERR("%s error(%d), reg=%x, value=%x\n", __func__, ret, reg,
+				*val);
 		return ret;
 	}
 
@@ -122,12 +125,13 @@  static int btmtk_usb_io_write32(struct btmtk_usb_data *data, u32 reg, u32 val)
 	index = (u16)reg;
 	value = val & 0x0000ffff;
 
-	ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), request, DEVICE_VENDOR_REQUEST_OUT,
-						  value, index, NULL, 0,
+	ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), request,
+			DEVICE_VENDOR_REQUEST_OUT, value, index, NULL, 0,
 						  CONTROL_TIMEOUT_JIFFIES);
 
 	if (ret < 0) {
-		BT_ERR("%s error(%d), reg=%x, value=%x\n", __func__, ret, reg, val);
+		BT_ERR("%s error(%d), reg=%x, value=%x\n", __func__, ret, reg,
+				val);
 		return ret;
 	}
 
@@ -139,7 +143,8 @@  static int btmtk_usb_io_write32(struct btmtk_usb_data *data, u32 reg, u32 val)
 				value, index, NULL, 0, CONTROL_TIMEOUT_JIFFIES);
 
 	if (ret < 0) {
-		BT_ERR("%s error(%d), reg=%x, value=%x\n", __func__, ret, reg, val);
+		BT_ERR("%s error(%d), reg=%x, value=%x\n", __func__, ret, reg,
+				val);
 		return ret;
 	}
 
@@ -186,13 +191,16 @@  static void btmtk_usb_cap_init(struct btmtk_usb_data *data)
 		ret = request_firmware(&firmware, MT7650_FIRMWARE, &udev->dev);
 		if (ret < 0) {
 			if (ret == -ENOENT) {
-				BT_ERR("Firmware file \"%s\" not found \n", MT7650_FIRMWARE);
+				BT_ERR("Firmware file \"%s\" not found \n",
+						MT7650_FIRMWARE);
 			} else {
-				BT_ERR("Firmware file \"%s\" request failed (err=%d) \n",
-					MT7650_FIRMWARE, ret);
+				BT_ERR("Firmware file \"%s\" request failed
+						(err=%d) \n", MT7650_FIRMWARE,
+						ret);
 			}
 		} else {
-			BT_DBG("Firmware file \"%s\" Found \n", MT7650_FIRMWARE);
+			BT_DBG("Firmware file \"%s\" Found \n",
+					MT7650_FIRMWARE);
 			/* load firmware here */
 			data->firmware = firmware;
 			btmtk_usb_load_fw(data);
@@ -205,10 +213,12 @@  static void btmtk_usb_cap_init(struct btmtk_usb_data *data)
 		ret = request_firmware(&firmware, MT7662_FIRMWARE, &udev->dev);
 		if (ret < 0) {
 			if (ret == -ENOENT) {
-				BT_ERR("Firmware file \"%s\" not found\n", MT7662_FIRMWARE);
+				BT_ERR("Firmware file \"%s\" not found\n",
+						MT7662_FIRMWARE);
 			} else {
-				BT_ERR("Firmware file \"%s\" request failed (err=%d)\n",
-					MT7662_FIRMWARE, ret);
+				BT_ERR("Firmware file \"%s\" request failed
+						(err=%d)\n", MT7662_FIRMWARE,
+						ret);
 			}
 		} else {
 		    BT_DBG("Firmware file \"%s\" Found\n", MT7662_FIRMWARE);
@@ -258,8 +268,8 @@  static int btmtk_usb_chk_crc(struct btmtk_usb_data *data, u32 checksum_len)
 	memmove(data->io_buf, &data->rom_patch_offset, 4);
 	memmove(&data->io_buf[4], &checksum_len, 4);
 
-	ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), 0x1, DEVICE_VENDOR_REQUEST_IN,
-						  0x20, 0x00, data->io_buf, 8,
+	ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), 0x1,
+			DEVICE_VENDOR_REQUEST_IN, 0x20, 0x00, data->io_buf, 8,
 						  CONTROL_TIMEOUT_JIFFIES);
 
 	if (ret < 0) {
@@ -318,8 +328,8 @@  static int btmtk_usb_reset_wmt(struct btmtk_usb_data *data)
 	BT_DBG("%s\n", __func__);
 
 	ret = usb_control_msg(data->udev, usb_sndctrlpipe(data->udev, 0), 0x01,
-				DEVICE_CLASS_REQUEST_OUT, 0x12, 0x00, data->io_buf,
-				8, CONTROL_TIMEOUT_JIFFIES);
+				DEVICE_CLASS_REQUEST_OUT, 0x12, 0x00,
+				data->io_buf, 8, CONTROL_TIMEOUT_JIFFIES);
 
 	if (ret)
 		BT_ERR("%s:(%d)\n", __func__, ret);
@@ -350,7 +360,8 @@  static int btmtk_usb_load_rom_patch(struct btmtk_usb_data *data)
 	unsigned char phase;
 	void *buf;
 	char *pos;
-	unsigned int pipe = usb_sndbulkpipe(data->udev, data->bulk_tx_ep->bEndpointAddress);
+	unsigned int pipe = usb_sndbulkpipe(data->udev,
+			data->bulk_tx_ep->bEndpointAddress);
 
 	if (!data->firmware) {
 		BT_ERR("%s:please assign a rom patch\n", __func__);
@@ -391,7 +402,8 @@  load_patch_protect:
 		goto error0;
 	}
 
-	buf = usb_alloc_coherent(data->udev, UPLOAD_PATCH_UNIT, GFP_ATOMIC, &data_dma);
+	buf = usb_alloc_coherent(data->udev, UPLOAD_PATCH_UNIT, GFP_ATOMIC,
+			&data_dma);
 
 	if (!buf) {
 		ret = -ENOMEM;
@@ -409,7 +421,8 @@  load_patch_protect:
 	/* loading rom patch */
 	while (1) {
 		s32 sent_len_max = UPLOAD_PATCH_UNIT - PATCH_HEADER_SIZE;
-		sent_len = (patch_len - cur_len) >= sent_len_max ? sent_len_max : (patch_len - cur_len);
+		sent_len = (patch_len - cur_len) >= sent_len_max ? sent_len_max
+			: (patch_len - cur_len);
 
 		BT_DBG("patch_len = %d\n", patch_len);
 		BT_DBG("cur_len = %d\n", cur_len);
@@ -442,10 +455,12 @@  load_patch_protect:
 
 			pos[8] = phase;
 
-			memcpy(&pos[9], data->firmware->data + PATCH_INFO_SIZE + cur_len, sent_len);
+			memcpy(&pos[9], data->firmware->data + PATCH_INFO_SIZE
+					+ cur_len, sent_len);
 
-			BT_DBG("sent_len + PATCH_HEADER_SIZE = %d, phase = %d\n",
-					sent_len + PATCH_HEADER_SIZE, phase);
+			BT_DBG("sent_len + PATCH_HEADER_SIZE = %d,
+					phase = %d\n", sent_len +
+					PATCH_HEADER_SIZE, phase);
 
 			usb_fill_bulk_urb(urb,
 					data->udev,
@@ -463,7 +478,8 @@  load_patch_protect:
 			if (ret)
 				goto error2;
 
-			if (!wait_for_completion_timeout(&sent_to_mcu_done, msecs_to_jiffies(1000))) {
+			if (!wait_for_completion_timeout(&sent_to_mcu_done,
+						msecs_to_jiffies(1000))) {
 				usb_kill_urb(urb);
 				BT_ERR("upload rom_patch timeout\n");
 				goto error2;
@@ -480,7 +496,8 @@  load_patch_protect:
 		}
 	}
 
-	total_checksum = checksume16((u8 *)data->firmware->data + PATCH_INFO_SIZE, patch_len);
+	total_checksum = checksume16((u8 *)data->firmware->data +
+			PATCH_INFO_SIZE, patch_len);
 
 	BT_DBG("Send checksum req..\n");
 
@@ -520,7 +537,8 @@  static int load_fw_iv(struct btmtk_usb_data *data)
 	memmove(buf, data->firmware->data + 32, 64);
 
 	ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), 0x01,
-						  DEVICE_VENDOR_REQUEST_OUT, 0x12, 0x0, buf, 64,
+						  DEVICE_VENDOR_REQUEST_OUT,
+						  0x12, 0x0, buf, 64,
 						  CONTROL_TIMEOUT_JIFFIES);
 
 	if (ret < 0) {
@@ -559,7 +577,8 @@  static int btmtk_usb_load_fw(struct btmtk_usb_data *data)
 	dma_addr_t data_dma;
 	int ret = 0, sent_len;
 	struct completion sent_to_mcu_done;
-	unsigned int pipe = usb_sndbulkpipe(data->udev, data->bulk_tx_ep->bEndpointAddress);
+	unsigned int pipe = usb_sndbulkpipe(data->udev,
+			data->bulk_tx_ep->bEndpointAddress);
 
 	if (!data->firmware) {
 		BT_ERR("%s:please assign a fw\n", __func__);
@@ -598,9 +617,11 @@  loadfw_protect:
 			| (*(data->firmware->data + 5) << 8)
 			| (*(data->firmware->data + 4));
 
-	fw_ver = (*(data->firmware->data + 11) << 8) | (*(data->firmware->data + 10));
+	fw_ver = (*(data->firmware->data + 11) << 8) |
+		(*(data->firmware->data + 10));
 
-	build_ver = (*(data->firmware->data + 9) << 8) | (*(data->firmware->data + 8));
+	build_ver = (*(data->firmware->data + 9) << 8) |
+		(*(data->firmware->data + 8));
 
 	BT_DBG("fw version:%d.%d.%02d ",
 			(fw_ver & 0xf000) >> 8,
@@ -657,7 +678,8 @@  loadfw_protect:
 
 	/* Loading ILM */
 	while (1) {
-		sent_len = (ilm_len - cur_len) >= 14336 ? 14336 : (ilm_len - cur_len);
+		sent_len = (ilm_len - cur_len) >= 14336 ? 14336
+			: (ilm_len - cur_len);
 
 		if (sent_len > 0) {
 			packet_header &= ~(0xffffffff);
@@ -665,7 +687,9 @@  loadfw_protect:
 			packet_header = cpu_to_le32(packet_header);
 
 			memmove(buf, &packet_header, 4);
-			memmove(buf + 4, data->firmware->data + 32 + cur_len, sent_len);
+			memmove(buf + 4, data->firmware->data + 32 + cur_len,
+					sent_len);
+
 
 			/* U2M_PDMA descriptor */
 			btmtk_usb_io_write32(data, 0x230, cur_len);
@@ -693,7 +717,8 @@  loadfw_protect:
 			if (ret)
 				goto error3;
 
-			if (!wait_for_completion_timeout(&sent_to_mcu_done, msecs_to_jiffies(1000))) {
+			if (!wait_for_completion_timeout(&sent_to_mcu_done,
+						msecs_to_jiffies(1000))) {
 				usb_kill_urb(urb);
 				BT_ERR("upload ilm fw timeout\n");
 				goto error3;
@@ -714,7 +739,8 @@  loadfw_protect:
 
 	/* Loading DLM */
 	while (1) {
-		sent_len = (dlm_len - cur_len) >= 14336 ? 14336 : (dlm_len - cur_len);
+		sent_len = (dlm_len - cur_len) >= 14336 ? 14336
+			: (dlm_len - cur_len);
 
 		if (sent_len > 0) {
 			packet_header &= ~(0xffffffff);
@@ -722,7 +748,8 @@  loadfw_protect:
 			packet_header = cpu_to_le32(packet_header);
 
 			memmove(buf, &packet_header, 4);
-			memmove(buf + 4, data->firmware->data + 32 + ilm_len + cur_len, sent_len);
+			memmove(buf + 4, data->firmware->data + 32 + ilm_len
+					+ cur_len, sent_len);
 
 			/* U2M_PDMA descriptor */
 			btmtk_usb_io_write32(data, 0x230, 0x80000 + cur_len);
@@ -751,7 +778,8 @@  loadfw_protect:
 			if (ret)
 				goto error3;
 
-			if (!wait_for_completion_timeout(&sent_to_mcu_done, msecs_to_jiffies(1000))) {
+			if (!wait_for_completion_timeout(&sent_to_mcu_done,
+						msecs_to_jiffies(1000))) {
 				usb_kill_urb(urb);
 				BT_ERR("upload dlm fw timeout\n");
 				goto error3;
@@ -978,8 +1006,8 @@  static int btmtk_usb_submit_bulk_in_urb(struct hci_dev *hdev, gfp_t mem_flags)
 
 	pipe = usb_rcvbulkpipe(data->udev, data->bulk_rx_ep->bEndpointAddress);
 
-	usb_fill_bulk_urb(urb, data->udev, pipe,
-					buf, size, btmtk_usb_bulk_in_complete, hdev);
+	usb_fill_bulk_urb(urb, data->udev, pipe, buf, size,
+			btmtk_usb_bulk_in_complete, hdev);
 
 	urb->transfer_flags |= URB_FREE_BUFFER;
 
@@ -1015,7 +1043,8 @@  static void btmtk_usb_isoc_in_complete(struct urb *urb)
 	if (urb->status == 0) {
 		for (i = 0; i < urb->number_of_packets; i++) {
 			unsigned int offset = urb->iso_frame_desc[i].offset;
-			unsigned int length = urb->iso_frame_desc[i].actual_length;
+			unsigned int length = urb->iso_frame_desc[i]
+				.actual_length;
 
 			if (urb->iso_frame_desc[i].status)
 				continue;
@@ -1096,8 +1125,9 @@  static int btmtk_usb_submit_isoc_in_urb(struct hci_dev *hdev, gfp_t mem_flags)
 
 	pipe = usb_rcvisocpipe(data->udev, data->isoc_rx_ep->bEndpointAddress);
 
-	usb_fill_int_urb(urb, data->udev, pipe, buf, size, btmtk_usb_isoc_in_complete,
-				hdev, data->isoc_rx_ep->bInterval);
+	usb_fill_int_urb(urb, data->udev, pipe, buf, size,
+			btmtk_usb_isoc_in_complete, hdev,
+			data->isoc_rx_ep->bInterval);
 
 	urb->transfer_flags  = URB_FREE_BUFFER | URB_ISO_ASAP;
 
@@ -1306,7 +1336,8 @@  static int btmtk_usb_send_frame(struct sk_buff *skb)
 		}
 
 		usb_fill_control_urb(urb, data->udev, pipe, (void *) dr,
-				skb->data, skb->len, btmtk_usb_tx_complete, skb);
+				skb->data, skb->len, btmtk_usb_tx_complete,
+				skb);
 
 		hdev->stat.cmd_tx++;
 		break;
@@ -1322,8 +1353,8 @@  static int btmtk_usb_send_frame(struct sk_buff *skb)
 		pipe = usb_sndbulkpipe(data->udev,
 					data->bulk_tx_ep->bEndpointAddress);
 
-		usb_fill_bulk_urb(urb, data->udev, pipe,
-				skb->data, skb->len, btmtk_usb_tx_complete, skb);
+		usb_fill_bulk_urb(urb, data->udev, pipe, skb->data, skb->len,
+				btmtk_usb_tx_complete, skb);
 
 		hdev->stat.acl_tx++;
 		BT_DBG("HCI_ACLDATA_PKT:\n");
@@ -1442,7 +1473,8 @@  static inline int __set_isoc_interface(struct hci_dev *hdev, int altsetting)
 
 static void btmtk_usb_work(struct work_struct *work)
 {
-	struct btmtk_usb_data *data = container_of(work, struct btmtk_usb_data, work);
+	struct btmtk_usb_data *data = container_of(work, struct btmtk_usb_data,
+			work);
 	struct hci_dev *hdev = data->hdev;
 	int new_alts;
 	int err;
@@ -1451,7 +1483,8 @@  static void btmtk_usb_work(struct work_struct *work)
 
 	if (hdev->conn_hash.sco_num > 0) {
 		if (!test_bit(BTUSB_DID_ISO_RESUME, &data->flags)) {
-			err = usb_autopm_get_interface(data->isoc ? data->isoc : data->intf);
+			err = usb_autopm_get_interface(data->isoc ? data->isoc
+					: data->intf);
 			if (err < 0) {
 				clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
 				usb_kill_anchored_urbs(&data->isoc_anchor);
@@ -1489,13 +1522,15 @@  static void btmtk_usb_work(struct work_struct *work)
 		__set_isoc_interface(hdev, 0);
 
 		if (test_and_clear_bit(BTUSB_DID_ISO_RESUME, &data->flags))
-			 usb_autopm_put_interface(data->isoc ? data->isoc : data->intf);
+			 usb_autopm_put_interface(data->isoc ? data->isoc
+					 : data->intf);
 	}
 }
 
 static void btmtk_usb_waker(struct work_struct *work)
 {
-	struct btmtk_usb_data *data = container_of(work, struct btmtk_usb_data, waker);
+	struct btmtk_usb_data *data = container_of(work, struct btmtk_usb_data,
+			waker);
 	int err;
 
 	err = usb_autopm_get_interface(data->intf);