staging: nokia_h4: nokia_core.c: use usleep_range() instead of msleep()

PATCH In review Miguel Oliveira 5 participants 27 messages May 30, 2014 – Aug 10, 2014

Subsystems (2)

Staging Bluetooth

lore

Share & tools

lore

Pavel Machek

2014-05-30 11:34 UTC #1 lore raw

Quoted 3 lines · Miguel Oliveira

Creating this patch for the Eudyptula Challenge.
Replaced msleep() for a delay < 20ms with a usleep_range() between 10000us and 15000us. Also inserted a blank line after adeclaration.

So you are changing timings without testing. Plus, burning CPU power instead of sleeping.

Seems you'll need another patch for the challenge :-).
Pavel

Signature · 2 lines

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Marcel Holtmann

maintainer

2014-05-30 11:52 UTC #2 lore raw

Hi Pavel,

Quoted 8 lines

Quoted 3 lines
Creating this patch for the Eudyptula Challenge.
Replaced msleep() for a delay < 20ms with a usleep_range() between 10000us and 15000us. Also inserted a blank line after adeclaration.
So you are changing timings without testing. Plus, burning CPU power instead of sleeping.
Seems you'll need another patch for the challenge :-).

I actually wonder if anybody is seriously working on this driver. My concern with the staging drivers has always been that we are quick with merging them when the work on getting them into upstream shape is actually hard. However reality is once they are in staging nobody is doing the work to clean them up and fix the issues.

This one seems no exception. Merged in January and 5 month later none of the TODO items have been addressed. All the promises that we fix the issues in the staging tree went out the window.

Greg, instead of wasting our time with this, can we just remove this driver from staging.

Regards

Marcel

Pavel Machek

2014-05-30 12:30 UTC #3 lore raw

Hi!

Quoted 11 lines

Quoted 8 lines
Quoted 3 lines
Creating this patch for the Eudyptula Challenge.
Replaced msleep() for a delay < 20ms with a usleep_range() between 10000us and 15000us. Also inserted a blank line after adeclaration.
So you are changing timings without testing. Plus, burning CPU power instead of sleeping.
Seems you'll need another patch for the challenge :-).
I actually wonder if anybody is seriously working on this driver. My concern with the staging drivers has always been that we are quick with merging them when the work on getting them into upstream shape is actually hard. However reality is once they are in staging nobody is doing the work to clean them up and fix the issues.

There is active work on merging n900 changes. And no, it does not progress as quickly as I'd like, but we'll get there. It is also requirement for n900 FM radio receiving...

Quoted 1 line

Greg, instead of wasting our time with this, can we just remove this driver from staging.

Please don't.
Pavel

Signature · 2 lines

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Greg KH

maintainer

2014-05-30 15:56 UTC #4 lore raw

Quoted 9 lines · Pavel Machek

Hi!
Quoted 7 lines
Quoted 7 lines
Quoted 3 lines
Creating this patch for the Eudyptula Challenge.
Replaced msleep() for a delay < 20ms with a usleep_range() between 10000us and 15000us. Also inserted a blank line after adeclaration.
So you are changing timings without testing. Plus, burning CPU power instead of sleeping.

Quoted 6 lines

Quoted 4 lines
Quoted 1 line
Seems you'll need another patch for the challenge :-).
I actually wonder if anybody is seriously working on this driver. My concern with the staging drivers has always been that we are quick with merging them when the work on getting them into upstream shape is actually hard. However reality is once they are in staging nobody is doing the work to clean them up and fix the issues.
There is active work on merging n900 changes.

Really? Where?

Quoted 6 lines

And no, it does not progress as quickly as I'd like, but we'll get there. It is also requirement for n900 FM radio receiving...
Quoted 1 line
Greg, instead of wasting our time with this, can we just remove this driver from staging.
Please don't.

As there has not been any real work on it since it has been merged, I don't see why I shouldn't remove it. If you do get some work done on it, you can always revert the removal and continue on. But the existance of code in staging that is not progressing forward at all is something that I don't like at all, and will be doing a large sweep of soon to remove.

thanks,

greg k-h

Pali Rohár

2014-05-30 16:15 UTC #5 lore raw

Hi Greg,

Quoted 20 lines · On Friday 30 May 2014 17:59:59 Greg KH wrote:

Quoted 19 lines · Pavel Machek
Hi!
Quoted 16 lines
Quoted 9 lines
Quoted 4 lines
Creating this patch for the Eudyptula Challenge. Replaced msleep() for a delay < 20ms with a usleep_range() between 10000us and 15000us. Also inserted a blank line after adeclaration.
So you are changing timings without testing. Plus, burning CPU power instead of sleeping.
Seems you'll need another patch for the challenge :-).
I actually wonder if anybody is seriously working on this driver. My concern with the staging drivers has always been that we are quick with merging them when the work on getting them into upstream shape is actually hard. However reality is once they are in staging nobody is doing the work to clean them up and fix the issues.

Quoted 4 lines

Quoted 1 line
There is active work on merging n900 changes.
Really? Where?

You can look at elinux wiki where is table how process is going:
http://elinux.org/N900

Also look at planed future list and its progress:
http://elinux.org/N900/Changelog

You can see that drivers are including step by step.

Quoted 16 lines

Quoted 8 lines
And no, it does not progress as quickly as I'd like, but we'll get there. It is also requirement for n900 FM radio receiving...
Quoted 2 lines
Greg, instead of wasting our time with this, can we just remove this driver from staging.
Please don't.
As there has not been any real work on it since it has been merged, I don't see why I shouldn't remove it. If you do get some work done on it, you can always revert the removal and continue on. But the existance of code in staging that is not progressing forward at all is something that I don't like at all, and will be doing a large sweep of soon to remove.

Quoted 3 lines

thanks,
greg k-h

Just look how much time took to include my patch for radio- bcm2048 (fm radio part of this chip) which fixing wrong overflow check. I sent it at the end of December and... yes it is still not included in linus tree. Now it is somethere in media tree and probably will be pulled in next merge window.

This means that it takes about half of year to include patches for these drivers.

So why now you want to very quickly remove this driver from staging but you did not wanted to take my above patch?

Signature · 2 lines

Pali Rohár
[email protected]

Greg KH

maintainer

2014-05-30 16:32 UTC #6 lore raw

Quoted 28 lines · Pali Rohár

Hi Greg,
Quoted 20 lines · On Friday 30 May 2014 17:59:59 Greg KH wrote:
Quoted 19 lines · Pavel Machek
Hi!
Quoted 16 lines
Quoted 9 lines
Quoted 4 lines
Creating this patch for the Eudyptula Challenge. Replaced msleep() for a delay < 20ms with a usleep_range() between 10000us and 15000us. Also inserted a blank line after adeclaration.
So you are changing timings without testing. Plus, burning CPU power instead of sleeping.
Seems you'll need another patch for the challenge :-).
I actually wonder if anybody is seriously working on this driver. My concern with the staging drivers has always been that we are quick with merging them when the work on getting them into upstream shape is actually hard. However reality is once they are in staging nobody is doing the work to clean them up and fix the issues.
Quoted 4 lines
Quoted 1 line
There is active work on merging n900 changes.
Really? Where?

Quoted 7 lines

You can look at elinux wiki where is table how process is going:
http://elinux.org/N900
Also look at planed future list and its progress:
http://elinux.org/N900/Changelog
You can see that drivers are including step by step.

I'm not going to dig through random web pages, sorry. If patches aren't sent to me for a driver, I consider it dead, that's only fair for me given my workload, don't you think?

Quoted 20 lines

Quoted 16 lines
Quoted 8 lines
And no, it does not progress as quickly as I'd like, but we'll get there. It is also requirement for n900 FM radio receiving...
Quoted 2 lines
Greg, instead of wasting our time with this, can we just remove this driver from staging.
Please don't.
As there has not been any real work on it since it has been merged, I don't see why I shouldn't remove it. If you do get some work done on it, you can always revert the removal and continue on. But the existance of code in staging that is not progressing forward at all is something that I don't like at all, and will be doing a large sweep of soon to remove.
Quoted 3 lines
thanks,
greg k-h

Quoted 8 lines

Just look how much time took to include my patch for radio- bcm2048 (fm radio part of this chip) which fixing wrong overflow check. I sent it at the end of December and... yes it is still not included in linus tree. Now it is somethere in media tree and probably will be pulled in next merge window.
This means that it takes about half of year to include patches for these drivers.

Just because the media drivers take a long time to get fixes merged, don't use that as an excuse to not fix up the staging drivers.

In fact, it sounds like you have lots of time while that patch is getting merged, why not work on fixing up the staging driver? :)

Quoted 2 lines

So why now you want to very quickly remove this driver from staging but you did not wanted to take my above patch?

I'm not the maintainer of drivers/staging/media/ and have no insight into what goes on there at all, nor do I care. I'm not the one responsible for taking your patch, so I did not "want" to take your patch at all.

thanks,

greg k-h

Marcel Holtmann

maintainer

2014-08-06 20:56 UTC #7 lore raw

Hi Greg,

Quoted 60 lines

Quoted 22 lines
Quoted 17 lines
Quoted 17 lines
Quoted 16 lines
Quoted 9 lines
Quoted 4 lines
Creating this patch for the Eudyptula Challenge. Replaced msleep() for a delay < 20ms with a usleep_range() between 10000us and 15000us. Also inserted a blank line after adeclaration.
So you are changing timings without testing. Plus, burning CPU power instead of sleeping.
Seems you'll need another patch for the challenge :-).
I actually wonder if anybody is seriously working on this driver. My concern with the staging drivers has always been that we are quick with merging them when the work on getting them into upstream shape is actually hard. However reality is once they are in staging nobody is doing the work to clean them up and fix the issues.
Quoted 4 lines
Quoted 1 line
There is active work on merging n900 changes.
Really? Where?
Quoted 7 lines
You can look at elinux wiki where is table how process is going:
http://elinux.org/N900
Also look at planed future list and its progress:
http://elinux.org/N900/Changelog
You can see that drivers are including step by step.
I'm not going to dig through random web pages, sorry. If patches aren't sent to me for a driver, I consider it dead, that's only fair for me given my workload, don't you think?
Quoted 26 lines
Quoted 16 lines
Quoted 8 lines
And no, it does not progress as quickly as I'd like, but we'll get there. It is also requirement for n900 FM radio receiving...
Quoted 2 lines
Greg, instead of wasting our time with this, can we just remove this driver from staging.
Please don't.
As there has not been any real work on it since it has been merged, I don't see why I shouldn't remove it. If you do get some work done on it, you can always revert the removal and continue on. But the existance of code in staging that is not progressing forward at all is something that I don't like at all, and will be doing a large sweep of soon to remove.
Quoted 3 lines
thanks,
greg k-h
Just look how much time took to include my patch for radio- bcm2048 (fm radio part of this chip) which fixing wrong overflow check. I sent it at the end of December and... yes it is still not included in linus tree. Now it is somethere in media tree and probably will be pulled in next merge window.

Quoted 8 lines

Quoted 2 lines
This means that it takes about half of year to include patches for these drivers.
Just because the media drivers take a long time to get fixes merged, don't use that as an excuse to not fix up the staging drivers.
In fact, it sounds like you have lots of time while that patch is getting merged, why not work on fixing up the staging driver? :)

and 2 month later, we still have nothing accomplished with this driver.

So this driver is 6 month in staging and not a single commit to address anything in the TODO file. I only see pointless coding style change.

There is a reason why I dislike staging drivers. And this is exactly it. It gets treated as dumping ground. So can we please remove this driver now. Maybe someone finally wakes up and does their promised job.

Regards

Marcel

Pavel Machek

2014-08-06 21:30 UTC #8 lore raw

Quoted 68 lines · Marcel Holtmann

Hi Greg,
Quoted 66 lines
Quoted 22 lines
Quoted 17 lines
Quoted 17 lines
Quoted 16 lines
Quoted 9 lines
Quoted 4 lines
Creating this patch for the Eudyptula Challenge. Replaced msleep() for a delay < 20ms with a usleep_range() between 10000us and 15000us. Also inserted a blank line after adeclaration.
So you are changing timings without testing. Plus, burning CPU power instead of sleeping.
Seems you'll need another patch for the challenge :-).
I actually wonder if anybody is seriously working on this driver. My concern with the staging drivers has always been that we are quick with merging them when the work on getting them into upstream shape is actually hard. However reality is once they are in staging nobody is doing the work to clean them up and fix the issues.
Quoted 4 lines
Quoted 1 line
There is active work on merging n900 changes.
Really? Where?
Quoted 7 lines
You can look at elinux wiki where is table how process is going:
http://elinux.org/N900
Also look at planed future list and its progress:
http://elinux.org/N900/Changelog
You can see that drivers are including step by step.
I'm not going to dig through random web pages, sorry. If patches aren't sent to me for a driver, I consider it dead, that's only fair for me given my workload, don't you think?
Quoted 20 lines
Quoted 16 lines
Quoted 8 lines
And no, it does not progress as quickly as I'd like, but we'll get there. It is also requirement for n900 FM radio receiving...
Quoted 2 lines
Greg, instead of wasting our time with this, can we just remove this driver from staging.
Please don't.
As there has not been any real work on it since it has been merged, I don't see why I shouldn't remove it. If you do get some work done on it, you can always revert the removal and continue on. But the existance of code in staging that is not progressing forward at all is something that I don't like at all, and will be doing a large sweep of soon to remove.
Quoted 3 lines
thanks,
greg k-h
Quoted 8 lines
Just look how much time took to include my patch for radio- bcm2048 (fm radio part of this chip) which fixing wrong overflow check. I sent it at the end of December and... yes it is still not included in linus tree. Now it is somethere in media tree and probably will be pulled in next merge window.
This means that it takes about half of year to include patches for these drivers.
Just because the media drivers take a long time to get fixes merged, don't use that as an excuse to not fix up the staging drivers.

Quoted 9 lines

Quoted 2 lines
In fact, it sounds like you have lots of time while that patch is getting merged, why not work on fixing up the staging driver? :)
and 2 month later, we still have nothing accomplished with this driver.
So this driver is 6 month in staging and not a single commit to address anything in the TODO file. I only see pointless coding style change.
There is a reason why I dislike staging drivers. And this is exactly it. It gets treated as dumping ground. So can we please remove this driver now. Maybe someone finally wakes up and does their promised job.

Yeah, so I clearly make a mistake of cleaning the driver up _before_ merging it into staging.

And yes, I'm currently trying to get 3.16 working in the qemu, which is prerequisite for any useful n900 work.

Pavel

Signature · 2 lines

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Greg KH

maintainer

2014-08-07 00:25 UTC #9 lore raw

Quoted 74 lines · Pavel Machek

Quoted 73 lines · Marcel Holtmann
Hi Greg,
Quoted 60 lines
Quoted 22 lines
Quoted 17 lines
Quoted 17 lines
Quoted 16 lines
Quoted 9 lines
Quoted 4 lines
Creating this patch for the Eudyptula Challenge. Replaced msleep() for a delay < 20ms with a usleep_range() between 10000us and 15000us. Also inserted a blank line after adeclaration.
So you are changing timings without testing. Plus, burning CPU power instead of sleeping.
Seems you'll need another patch for the challenge :-).
I actually wonder if anybody is seriously working on this driver. My concern with the staging drivers has always been that we are quick with merging them when the work on getting them into upstream shape is actually hard. However reality is once they are in staging nobody is doing the work to clean them up and fix the issues.
Quoted 4 lines
Quoted 1 line
There is active work on merging n900 changes.
Really? Where?
Quoted 7 lines
You can look at elinux wiki where is table how process is going:
http://elinux.org/N900
Also look at planed future list and its progress:
http://elinux.org/N900/Changelog
You can see that drivers are including step by step.
I'm not going to dig through random web pages, sorry. If patches aren't sent to me for a driver, I consider it dead, that's only fair for me given my workload, don't you think?
Quoted 26 lines
Quoted 16 lines
Quoted 8 lines
And no, it does not progress as quickly as I'd like, but we'll get there. It is also requirement for n900 FM radio receiving...
Quoted 2 lines
Greg, instead of wasting our time with this, can we just remove this driver from staging.
Please don't.
As there has not been any real work on it since it has been merged, I don't see why I shouldn't remove it. If you do get some work done on it, you can always revert the removal and continue on. But the existance of code in staging that is not progressing forward at all is something that I don't like at all, and will be doing a large sweep of soon to remove.
Quoted 3 lines
thanks,
greg k-h
Just look how much time took to include my patch for radio- bcm2048 (fm radio part of this chip) which fixing wrong overflow check. I sent it at the end of December and... yes it is still not included in linus tree. Now it is somethere in media tree and probably will be pulled in next merge window.
Quoted 8 lines
Quoted 2 lines
This means that it takes about half of year to include patches for these drivers.
Just because the media drivers take a long time to get fixes merged, don't use that as an excuse to not fix up the staging drivers.
In fact, it sounds like you have lots of time while that patch is getting merged, why not work on fixing up the staging driver? :)
and 2 month later, we still have nothing accomplished with this driver.

Quoted 7 lines

Quoted 4 lines
So this driver is 6 month in staging and not a single commit to address anything in the TODO file. I only see pointless coding style change.
There is a reason why I dislike staging drivers. And this is exactly it. It gets treated as dumping ground. So can we please remove this driver now. Maybe someone finally wakes up and does their promised job.
Yeah, so I clearly make a mistake of cleaning the driver up _before_ merging it into staging.

No, the mistake was that you didn't ever follow it up with any real work, not that it was "cleaned up" at all.

Quoted 2 lines

And yes, I'm currently trying to get 3.16 working in the qemu, which is prerequisite for any useful n900 work.

So I guess I'll queue up a patch to delete this for 3.17-rc2. Pavel, if you get the device working properly, we can easily revert the deletion and you can send follow-on patches to fix things up properly.

thanks,

greg k-h

Pavel Machek

2014-08-07 08:13 UTC #10 lore raw

PATCH 1/1 staging: nokia_h4: use more consistent name for intermediate object

Signed-off-by Pavel Machek <[email protected]>

drivers/staging/nokia_h4p/Makefile +2 -2

index 9625db4..daffe3c 100644
@@ -1,6 +1,6 @@
 
-obj-$(CONFIG_BT_NOKIA_H4P)		+= btnokia_h4p.o
-btnokia_h4p-objs := nokia_core.o nokia_fw.o nokia_uart.o nokia_fw-csr.o \
+obj-$(CONFIG_BT_NOKIA_H4P)		+= hci_h4p.o
+hci_h4p-objs := nokia_core.o nokia_fw.o nokia_uart.o nokia_fw-csr.o \
 		nokia_fw-bcm.o nokia_fw-ti1273.o
 
 ccflags-y += -D__CHECK_ENDIAN__

Signature · 2 lines

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Greg KH

maintainer

2014-08-07 20:24 UTC #11 lore raw

Re: [PATCH] staging: nokia_h4: use more consistent name for intermediate object

Quoted 8 lines · Pavel Machek

Signed-off-by Pavel Machek <[email protected]>
drivers/staging/nokia_h4p/Makefile +0 -0
index 9625db4..daffe3c 100644

drivers/staging/nokia_h4p/Makefile line 6

Quoted 8 lines

drivers/staging/nokia_h4p/Makefile +2 -2

@@ -1,6 +1,6 @@
 
-obj-$(CONFIG_BT_NOKIA_H4P)		+= btnokia_h4p.o
-btnokia_h4p-objs := nokia_core.o nokia_fw.o nokia_uart.o nokia_fw-csr.o \
+obj-$(CONFIG_BT_NOKIA_H4P)		+= hci_h4p.o
+hci_h4p-objs := nokia_core.o nokia_fw.o nokia_uart.o nokia_fw-csr.o \
 		nokia_fw-bcm.o nokia_fw-ti1273.o
 
 ccflags-y += -D__CHECK_ENDIAN__

You just changed the kernel module name, did you really mean to do that?

Why?

It's not an "intermediate" object here, it's a .ko file, right?

greg k-h

Pavel Machek

2014-08-09 15:48 UTC #12 lore raw

Re: [PATCH] staging: nokia_h4: use more consistent name for intermediate object

Quoted 16 lines · Greg KH

Quoted 15 lines · Pavel Machek

Signed-off-by Pavel Machek <[email protected]>

drivers/staging/nokia_h4p/Makefile +2 -2

index 9625db4..daffe3c 100644
@@ -1,6 +1,6 @@
 
-obj-$(CONFIG_BT_NOKIA_H4P)		+= btnokia_h4p.o
-btnokia_h4p-objs := nokia_core.o nokia_fw.o nokia_uart.o nokia_fw-csr.o \
+obj-$(CONFIG_BT_NOKIA_H4P)		+= hci_h4p.o
+hci_h4p-objs := nokia_core.o nokia_fw.o nokia_uart.o nokia_fw-csr.o \
 		nokia_fw-bcm.o nokia_fw-ti1273.o

Quoted 7 lines

drivers/staging/nokia_h4p/Makefile line 6
Quoted 1 line
drivers/staging/nokia_h4p/Makefile +0 -0
 ccflags-y += -D__CHECK_ENDIAN__
You just changed the kernel module name, did you really mean to do that?
Why?
It's not an "intermediate" object here, it's a .ko file, right?

Ok, lets not do that now. btnokia_h4p is really strange name, and it will probably need to be fixed in future, but now may not be a good time.

Pavel

Signature · 2 lines

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Pavel Machek

2014-08-10 07:30 UTC #13 lore raw

PATCH 1/3 staging: nokia_h4: switch to right types and use bdaddr_t

In reply to Greg KH

Switch bluetooth address to right type and use bacmp to replace explicit checks for zero addresses.

Signed-off-by Pavel Machek <[email protected]>

drivers/staging/nokia_h4p/hci_h4p.h +1 -1

index 99c4da6..76eaaaa 100644
@@ -77,7 +77,7 @@ struct hci_h4p_info {
 	unsigned long rx_state;
 	unsigned long garbage_bytes;
 
-	u8 bd_addr[6];
+	bdaddr_t bd_addr;
 	struct sk_buff_head *fw_q;
 
 	int pm_enabled;

drivers/staging/nokia_h4p/nokia_core.c +2 -2

index 775e1d0..79d8b01 100644
@@ -993,7 +993,7 @@ static ssize_t hci_h4p_store_bdaddr(struct device *dev,
 	for (i = 0; i < 6; i++) {
 		if (bdaddr[i] > 0xff)
 			return -EINVAL;
-		info->bd_addr[i] = bdaddr[i] & 0xff;
+		info->bd_addr.b[i] = bdaddr[i] & 0xff;
 	}
 
 	return count;

@@ -1004,7 +1004,7 @@ static ssize_t hci_h4p_show_bdaddr(struct device *dev,
 {
 	struct hci_h4p_info *info = dev_get_drvdata(dev);
 
-	return sprintf(buf, "%pMR\n", info->bd_addr);
+	return sprintf(buf, "%pMR\n", info->bd_addr.b);
 }
 
 static DEVICE_ATTR(bdaddr, S_IRUGO | S_IWUSR, hci_h4p_show_bdaddr,

drivers/staging/nokia_h4p/nokia_fw-bcm.c +4 -4

index 14fe3ae..054a78b 100644
@@ -30,17 +30,17 @@ static int hci_h4p_bcm_set_bdaddr(struct hci_h4p_info *info,
 {
 	int i;
 	static const u8 nokia_oui[3] = {0x00, 0x1f, 0xdf};
-	int not_valid = !bacmp(info->bd_addr, BDADDR_ANY);
+	int not_valid = !bacmp(&info->bd_addr, BDADDR_ANY);
 
 	if (not_valid) {
 		dev_info(info->dev, "Valid bluetooth address not found, setting some random\n");
 		/* When address is not valid, use some random but Nokia MAC */
-		memcpy(info->bd_addr, nokia_oui, 3);
-		get_random_bytes(info->bd_addr + 3, 3);
+		memcpy(info->bd_addr.b, nokia_oui, 3);
+		get_random_bytes(info->bd_addr.b + 3, 3);
 	}
 
 	for (i = 0; i < 6; i++)
-		skb->data[9 - i] = info->bd_addr[i];
+		skb->data[9 - i] = info->bd_addr.b[i];
 
 	return 0;
 }

drivers/staging/nokia_h4p/nokia_fw-csr.c +10 ~~-10~~

index 925ed86d0..a2c58c4 100644
@@ -48,7 +48,7 @@ int hci_h4p_bc4_send_fw(struct hci_h4p_info *info,
 	static const u8 nokia_oui[3] = {0x00, 0x19, 0x4F};
 	struct sk_buff *skb;
 	unsigned int offset;
-	int retries, count, i, not_valid;
+	int retries, count, not_valid;
 	unsigned long flags;
 
 	info->fw_error = 0;

@@ -65,21 +65,21 @@ int hci_h4p_bc4_send_fw(struct hci_h4p_info *info,
 		skb->data[offset + 1] = 0x00;
 		skb->data[offset + 5] = 0x00;
 
-		not_valid = !bacmp(info->bd_addr, BDADDR_ANY);
+		not_valid = !bacmp(&info->bd_addr, BDADDR_ANY);
 
 		if (not_valid) {
 			dev_info(info->dev, "Valid bluetooth address not found, setting some random\n");
 			/* When address is not valid, use some random */
-			memcpy(info->bd_addr, nokia_oui, 3);
-			get_random_bytes(info->bd_addr + 3, 3);
+			memcpy(info->bd_addr.b, nokia_oui, 3);
+			get_random_bytes(info->bd_addr.b + 3, 3);
 		}
 
-		skb->data[offset + 7] = info->bd_addr[0];
-		skb->data[offset + 6] = info->bd_addr[1];
-		skb->data[offset + 4] = info->bd_addr[2];
-		skb->data[offset + 0] = info->bd_addr[3];
-		skb->data[offset + 3] = info->bd_addr[4];
-		skb->data[offset + 2] = info->bd_addr[5];
+		skb->data[offset + 7] = info->bd_addr.b[0];
+		skb->data[offset + 6] = info->bd_addr.b[1];
+		skb->data[offset + 4] = info->bd_addr.b[2];
+		skb->data[offset + 0] = info->bd_addr.b[3];
+		skb->data[offset + 3] = info->bd_addr.b[4];
+		skb->data[offset + 2] = info->bd_addr.b[5];
 	}
 
 	for (count = 1; ; count++) {

Signature · 2 lines

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Pavel Machek

2014-08-10 07:31 UTC #14 lore raw

PATCH 2/3 staging: nokia_h4: avoid __uX types

In reply to Greg KH

__u8/16 types should not be neccessary inside kernel, u8/16 should be enough.

Signed-off-by Pavel Machek <[email protected]>

drivers/staging/nokia_h4p/hci_h4p.h +20 ~~-20~~

index 76eaaaa..b91e8b3 100644
@@ -100,12 +100,12 @@ struct hci_h4p_info {
 };
 
 struct hci_h4p_radio_hdr {
-	__u8 evt;
-	__u8 dlen;
+	u8 evt;
+	u8 dlen;
 } __packed;
 
 struct hci_h4p_neg_hdr {
-	__u8 dlen;
+	u8 dlen;
 } __packed;
 #define H4P_NEG_HDR_SIZE 1

@@ -121,36 +121,36 @@ struct hci_h4p_neg_hdr {
 #define H4P_ID_TI1271	0x31
 
 struct hci_h4p_neg_cmd {
-	__u8	ack;
-	__u16	baud;
-	__u16	unused1;
-	__u8	proto;
-	__u16	sys_clk;
-	__u16	unused2;
+	u8	ack;
+	u16	baud;
+	u16	unused1;
+	u8	proto;
+	u16	sys_clk;
+	u16	unused2;
 } __packed;
 
 struct hci_h4p_neg_evt {
-	__u8	ack;
-	__u16	baud;
-	__u16	unused1;
-	__u8	proto;
-	__u16	sys_clk;
-	__u16	unused2;
-	__u8	man_id;
-	__u8	ver_id;
+	u8	ack;
+	u16	baud;
+	u16	unused1;
+	u8	proto;
+	u16	sys_clk;
+	u16	unused2;
+	u8	man_id;
+	u8	ver_id;
 } __packed;
 
 #define H4P_ALIVE_REQ	0x55
 #define H4P_ALIVE_RESP	0xcc
 
 struct hci_h4p_alive_hdr {
-	__u8	dlen;
+	u8	dlen;
 } __packed;
 #define H4P_ALIVE_HDR_SIZE 1
 
 struct hci_h4p_alive_pkt {
-	__u8	mid;
-	__u8	unused;
+	u8	mid;
+	u8	unused;
 } __packed;
 
 #define MAX_BAUD_RATE		921600

Signature · 2 lines

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Pavel Machek

2014-08-10 07:33 UTC #15 lore raw

PATCH 3/3 staging: use inlines where it makes sense

In reply to Greg KH

Make some uart function static inline to reduce ammount of exported code.

Signed-off-by Pavel Machek <[email protected]>

drivers/staging/nokia_h4p/hci_h4p.h +24 -3

index b91e8b3..6c72ca5 100644
@@ -26,6 +26,8 @@
 #include <net/bluetooth/hci_core.h>
 #include <net/bluetooth/hci.h>
 
+#include <linux/serial_reg.h>
+
 #define UART_SYSC_OMAP_RESET	0x03
 #define UART_SYSS_RESETDONE	0x01
 #define UART_OMAP_SCR_EMPTY_THR	0x08

@@ -205,9 +207,28 @@ int hci_h4p_read_fw(struct hci_h4p_info *info, struct sk_buff_head *fw_queue);
 int hci_h4p_send_fw(struct hci_h4p_info *info, struct sk_buff_head *fw_queue);
 void hci_h4p_parse_fw_event(struct hci_h4p_info *info, struct sk_buff *skb);
 
-void hci_h4p_outb(struct hci_h4p_info *info, unsigned int offset, u8 val);
-u8 hci_h4p_inb(struct hci_h4p_info *info, unsigned int offset);
-void hci_h4p_set_rts(struct hci_h4p_info *info, int active);
+static inline void hci_h4p_outb(struct hci_h4p_info *info, unsigned int offset, u8 val)
+{
+	__raw_writeb(val, info->uart_base + (offset << 2));
+}
+
+static inline u8 hci_h4p_inb(struct hci_h4p_info *info, unsigned int offset)
+{
+	return __raw_readb(info->uart_base + (offset << 2));
+}
+
+static inline void hci_h4p_set_rts(struct hci_h4p_info *info, int active)
+{
+	u8 b;
+
+	b = hci_h4p_inb(info, UART_MCR);
+	if (active)
+		b |= UART_MCR_RTS;
+	else
+		b &= ~UART_MCR_RTS;
+	hci_h4p_outb(info, UART_MCR, b);
+}
+
 int hci_h4p_wait_for_cts(struct hci_h4p_info *info, int active, int timeout_ms);
 void __hci_h4p_set_auto_ctsrts(struct hci_h4p_info *info, int on, u8 which);
 void hci_h4p_set_auto_ctsrts(struct hci_h4p_info *info, int on, u8 which);

drivers/staging/nokia_h4p/nokia_uart.c +0 ~~-21~~

index 0fb57de..9aab7bb 100644
@@ -19,7 +19,6 @@
  *
  */
 
-#include <linux/serial_reg.h>
 #include <linux/delay.h>
 #include <linux/clk.h>

@@ -27,27 +26,7 @@
 
 #include "hci_h4p.h"
 
-inline void hci_h4p_outb(struct hci_h4p_info *info, unsigned int offset, u8 val)
-{
-	__raw_writeb(val, info->uart_base + (offset << 2));
-}
-
-inline u8 hci_h4p_inb(struct hci_h4p_info *info, unsigned int offset)
-{
-	return __raw_readb(info->uart_base + (offset << 2));
-}
 
-void hci_h4p_set_rts(struct hci_h4p_info *info, int active)
-{
-	u8 b;
-
-	b = hci_h4p_inb(info, UART_MCR);
-	if (active)
-		b |= UART_MCR_RTS;
-	else
-		b &= ~UART_MCR_RTS;
-	hci_h4p_outb(info, UART_MCR, b);
-}
 
 int hci_h4p_wait_for_cts(struct hci_h4p_info *info, int active,
 			 int timeout_ms)

Signature · 2 lines

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Pavel Machek

2014-08-10 07:34 UTC #16 lore raw

PATCH 1/1 bluetooth: avoid using __uX types

In reply to Greg KH

__u8/16 types should not be neccessary inside kernel, u8/16 should be enough.

Signed-off-by Pavel Machek <[email protected]>

include/net/bluetooth/bluetooth.h +16 ~~-16~~

index 373000d..1176e67 100644
@@ -58,8 +58,8 @@
 
 #define BT_SECURITY	4
 struct bt_security {
-	__u8 level;
-	__u8 key_size;
+	u8 level;
+	u8 key_size;
 };
 #define BT_SECURITY_SDP		0
 #define BT_SECURITY_LOW		1

@@ -76,7 +76,7 @@ struct bt_security {
 
 #define BT_POWER	9
 struct bt_power {
-	__u8 force_active;
+	u8 force_active;
 };
 #define BT_POWER_FORCE_ACTIVE_OFF 0
 #define BT_POWER_FORCE_ACTIVE_ON  1

@@ -110,7 +110,7 @@ struct bt_power {
 
 #define BT_VOICE		11
 struct bt_voice {
-	__u16 setting;
+	u16 setting;
 };
 
 #define BT_VOICE_TRANSPARENT			0x0003

@@ -170,7 +170,7 @@ static inline const char *state_to_string(int state)
 
 /* BD Address */
 typedef struct {
-	__u8 b[6];
+	u8 b[6];
 } __packed bdaddr_t;
 
 /* BD Address type */

@@ -178,7 +178,7 @@ typedef struct {
 #define BDADDR_LE_PUBLIC	0x01
 #define BDADDR_LE_RANDOM	0x02
 
-static inline bool bdaddr_type_is_valid(__u8 type)
+static inline bool bdaddr_type_is_valid(u8 type)
 {
 	switch (type) {
 	case BDADDR_BREDR:

@@ -190,7 +190,7 @@ static inline bool bdaddr_type_is_valid(__u8 type)
 	return false;
 }
 
-static inline bool bdaddr_type_is_le(__u8 type)
+static inline bool bdaddr_type_is_le(u8 type)
 {
 	switch (type) {
 	case BDADDR_LE_PUBLIC:

@@ -260,15 +260,15 @@ struct sock *bt_accept_dequeue(struct sock *parent, struct socket *newsock);
 
 /* Skb helpers */
 struct l2cap_ctrl {
-	__u8	sframe:1,
+	u8	sframe:1,
 		poll:1,
 		final:1,
 		fcs:1,
 		sar:2,
 		super:2;
-	__u16	reqseq;
-	__u16	txseq;
-	__u8	retries;
+	u16	reqseq;
+	u16	txseq;
+	u8	retries;
 };
 
 struct hci_dev;

@@ -282,10 +282,10 @@ struct hci_req_ctrl {
 };
 
 struct bt_skb_cb {
-	__u8 pkt_type;
-	__u8 incoming;
-	__u16 expect;
-	__u8 force_active;
+	u8 pkt_type;
+	u8 incoming;
+	u16 expect;
+	u8 force_active;
 	struct l2cap_chan *chan;
 	struct l2cap_ctrl control;
 	struct hci_req_ctrl req;

@@ -336,7 +336,7 @@ out:
 	return NULL;
 }
 
-int bt_to_errno(__u16 code);
+int bt_to_errno(u16 code);
 
 int hci_sock_init(void);
 void hci_sock_cleanup(void);

Signature · 2 lines

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Pavel Machek

2014-08-07 09:07 UTC #17 lore raw

PATCH 1/1 staging: nokia_h4: merge firmware together so that we can reduce ammount of exports

In reply to Greg KH

Merge firmware files to nokia_core, so that we can reduce ammount of exported functions. Also replace hand-coded check for invalid bluetooth address with bacmp.

Signed-off-by Pavel Machek <[email protected]>

drivers/staging/nokia_h4p/Makefile +1 -2

index 310b0f2..3398a1c 100644
@@ -1,6 +1,5 @@
 
 obj-$(CONFIG_BT_NOKIA_H4P)		+= nokia_h4p.o
-nokia_h4p-objs := nokia_core.o nokia_fw.o nokia_uart.o nokia_fw-csr.o \
-		nokia_fw-bcm.o nokia_fw-ti1273.o
+nokia_h4p-objs := nokia_core.o nokia_fw.o nokia_uart.o
 
 ccflags-y += -D__CHECK_ENDIAN__

drivers/staging/nokia_h4p/nokia_core.c +324 -0

330 lines not on this page · raw message

drivers/staging/nokia_h4p/nokia_fw-bcm.c deleted +0 ~~-149~~

149 lines not on this page · raw message

drivers/staging/nokia_h4p/nokia_fw-csr.c deleted +0 ~~-149~~

149 lines not on this page · raw message

drivers/staging/nokia_h4p/nokia_fw-ti1273.c deleted +0 ~~-110~~

110 lines not on this page · raw message

Pavel Machek

2014-08-07 09:36 UTC #18 lore raw

Re: [PATCH] staging: nokia_h4: merge firmware together so that we can reduce ammount of exports

Quoted 6 lines · Pavel Machek

Merge firmware files to nokia_core, so that we can reduce ammount of exported functions. Also replace hand-coded check for invalid bluetooth address with bacmp.
Signed-off-by Pavel Machek <[email protected]>

Actually, it might be better if you forgot about this patch. The code is more tangled up than I expected.

Pavel

Marcel Holtmann

maintainer

2014-08-07 14:48 UTC #19 lore raw

Re: [PATCH] staging: nokia_h4: merge firmware together so that we can reduce ammount of exports

In reply to Pavel Machek

Hi Pavel,

Quoted 76 lines

Merge firmware files to nokia_core, so that we can reduce ammount of exported functions. Also replace hand-coded check for invalid bluetooth address with bacmp.

Signed-off-by Pavel Machek <[email protected]>

drivers/staging/nokia_h4p/Makefile +0 -0

index 310b0f2..3398a1c 100644
@@ -1,6 +1,5 @@

obj-$(CONFIG_BT_NOKIA_H4P) += nokia_h4p.o

drivers/staging/nokia_h4p/Makefile +1 -2

-nokia_h4p-objs := nokia_core.o nokia_fw.o nokia_uart.o nokia_fw-csr.o \
-		nokia_fw-bcm.o nokia_fw-ti1273.o
+nokia_h4p-objs := nokia_core.o nokia_fw.o nokia_uart.o

ccflags-y += -D__CHECK_ENDIAN__

drivers/staging/nokia_h4p/nokia_core.c +50 -0

index 775e1d0..501be06 100644
@@ -1191,6 +1191,330 @@ static int hci_h4p_remove(struct platform_device *pdev)
 	return 0;
 }
 
+/* Code specific for BCM firmware -------------------------------- */
+
+static int hci_h4p_bcm_set_bdaddr(struct hci_h4p_info *info,
+				struct sk_buff *skb)
+{
+	int i;
+	static const u8 nokia_oui[3] = {0x00, 0x1f, 0xdf};
+	int not_valid = !bacmp(info->bd_addr, BDADDR_ANY);
+
+	if (not_valid) {
+		dev_info(info->dev, "Valid bluetooth address not found, setting some random\n");
+		/* When address is not valid, use some random but Nokia MAC */
+		memcpy(info->bd_addr, nokia_oui, 3);
+		get_random_bytes(info->bd_addr + 3, 3);
+	}
+
+	for (i = 0; i < 6; i++)
+		skb->data[9 - i] = info->bd_addr[i];
+
+	return 0;
+}
+
+void hci_h4p_bcm_parse_fw_event(struct hci_h4p_info *info, struct sk_buff *skb)
+{
+	struct sk_buff *fw_skb;
+	int err;
+	unsigned long flags;
+
+	if (skb->data[5] != 0x00) {
+		dev_err(info->dev, "Firmware sending command failed 0x%.2x\n",
+			skb->data[5]);
+		info->fw_error = -EPROTO;
+	}
+
+	kfree_skb(skb);
+
+	fw_skb = skb_dequeue(info->fw_q);
+	if (fw_skb == NULL || info->fw_error) {
+		complete(&info->fw_completion);
+		return;
+	}
+
+	if (fw_skb->data[1] == 0x01 && fw_skb->data[2] == 0xfc &&
+			fw_skb->len >= 10) {
+		BT_DBG("Setting bluetooth address");
+		err = hci_h4p_bcm_set_bdaddr(info, fw_skb);
+		if (err < 0) {
+			kfree_skb(fw_skb);
+			info->fw_error = err;
+			complete(&info->fw_completion);

drivers/staging/nokia_h4p/nokia_core.c static int hci_h4p_remove(struct platform_device *pdev) · line 1253

Quoted 10 lines

drivers/staging/nokia_h4p/nokia_core.c +10 -0

+			return;
+		}
+	}
+
+	skb_queue_tail(&info->txq, fw_skb);
+	spin_lock_irqsave(&info->lock, flags);
+	hci_h4p_outb(info, UART_IER, hci_h4p_inb(info, UART_IER) |
+			UART_IER_THRI);
+	spin_unlock_irqrestore(&info->lock, flags);
+}

and as I explained before, this crazy can not continue. Bluetooth drivers can provide a hdev->setup callback for loading firmware and doing other setup details. You can just bring up the HCI transport. We are providing __hci_cmd_sync for easy loading of the firmware. Especially if the firmware just consists of HCI commands. Which is clearly the case with the Nokia firmware files.

In addition with 3.17 you can also provide hdev->set_bdaddr to allow changing the public BD_ADDR. You can also set HCI_QUIRK_INVALID_BDADDR and bring up the controller in an unconfigured state. That way you can have userspace provide the address and avoid the hacking around the missing address issue.

Maybe at some point someone realizes that focusing on the N900 Bluetooth hardware would help in making this code simpler. Adding support for older Maemo devices can be done later.

Regards

Marcel

Pavel Machek

2014-08-09 15:47 UTC #20 lore raw

Re: [PATCH] staging: nokia_h4: merge firmware together so that we can reduce ammount of exports

Hi!

Quoted 10 lines

drivers/staging/nokia_h4p/nokia_core.c static int hci_h4p_remove(struct platform_device *pdev) · line 1253
Quoted 7 lines
drivers/staging/nokia_h4p/nokia_core.c +7 -0
+
+	skb_queue_tail(&info->txq, fw_skb);
+	spin_lock_irqsave(&info->lock, flags);
+	hci_h4p_outb(info, UART_IER, hci_h4p_inb(info, UART_IER) |
+			UART_IER_THRI);
+	spin_unlock_irqrestore(&info->lock, flags);
+}
and as I explained before, this crazy can not continue. Bluetooth drivers can provide a hdev->setup callback for loading firmware and doing other setup details. You can just bring up the HCI transport. We are providing __hci_cmd_sync for easy loading of the firmware. Especially if the firmware just consists of HCI commands. Which is clearly the case with the Nokia firmware files.

I know that this crazyness can not continue; but I need to be able to move "bad" code around to get the cleanups, ok? Because you also asked me to reduce ammount of exports.

Best regards,
Pavel

Signature · 2 lines

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Miguel Oliveira

author

2014-05-30 12:15 UTC #21 lore raw

In reply to Pavel Machek

So the /Documentation/timers/timers-howto.txt stats:

SLEEPING FOR ~USECS OR SMALL MSECS ( 10us - 20ms):
* Use usleep_range

- Why not msleep for (1ms - 20ms)?
Explained originally here:
http://lkml.org/lkml/2007/8/3/250
msleep(1~20) may not do what the caller intends, and will often sleep longer (~20 ms actual sleep for any value given in the 1~20ms range). In many cases this is not the desired behavior.

- Why is there no "usleep" / What is a good range? Since usleep_range is built on top of hrtimers, the wakeup will be very precise (ish), thus a simple usleep function would likely introduce a large number of undesired interrupts.

With the introduction of a range, the scheduler is free to coalesce your wakeup with any other wakeup that may have happened for other reasons, or at the worst case, fire an interrupt for your upper bound.

The larger a range you supply, the greater a chance that you will not trigger an interrupt; this should be balanced with what is an acceptable upper bound on delay / performance for your specific code path. Exact tolerances here are very situation specific, thus it is left to the caller to determine a reasonable range.

Should the documentation should be ignored?

Regards

2014-05-30 12:34 GMT+01:00 Pavel Machek <[email protected]>:

Quoted 14 lines · Miguel Oliveira

Quoted 3 lines
Creating this patch for the Eudyptula Challenge.
Replaced msleep() for a delay < 20ms with a usleep_range() between 10000us and 15000us. Also inserted a blank line after adeclaration.
So you are changing timings without testing. Plus, burning CPU power instead of sleeping.
Seems you'll need another patch for the challenge :-).
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Pavel Machek

2014-05-30 12:35 UTC #22 lore raw

Hi!

Quoted 12 lines

So the /Documentation/timers/timers-howto.txt stats:
SLEEPING FOR ~USECS OR SMALL MSECS ( 10us - 20ms):
* Use usleep_range
- Why not msleep for (1ms - 20ms)?
Explained originally here:
http://lkml.org/lkml/2007/8/3/250
msleep(1~20) may not do what the caller intends, and will often sleep longer (~20 ms actual sleep for any value given in the 1~20ms range). In many cases this is not the desired behavior.

...

Quoted 1 line

Should the documentation should be ignored?

Caller wants to sleep for 10ms; docs said it may sleep for 10-20msec; that sounds reasonable. You replaced it with busy-loop for 10-15msec.

That does not sound like improvement.

Thanks,
Pavel

Signature · 2 lines

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Miguel Oliveira

author

2014-05-30 12:58 UTC #23 lore raw

Not on my prespective. Its says "~20 ms actual sleep for any value given in the 1~20ms range", so from my point of view it will sleep for 20ms, so I will save between 5 to 10ms. But maybe I'm seeing it the wrong way.

Regards

2014-05-30 13:35 GMT+01:00 Pavel Machek <[email protected]>:

Quoted 28 lines

Hi!
Quoted 12 lines
So the /Documentation/timers/timers-howto.txt stats:
SLEEPING FOR ~USECS OR SMALL MSECS ( 10us - 20ms):
* Use usleep_range
- Why not msleep for (1ms - 20ms)?
Explained originally here:
http://lkml.org/lkml/2007/8/3/250
msleep(1~20) may not do what the caller intends, and will often sleep longer (~20 ms actual sleep for any value given in the 1~20ms range). In many cases this is not the desired behavior.
...
Quoted 1 line
Should the documentation should be ignored?
Caller wants to sleep for 10ms; docs said it may sleep for 10-20msec; that sounds reasonable. You replaced it with busy-loop for 10-15msec.
That does not sound like improvement.
Thanks,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Greg KH

maintainer

2014-05-30 15:57 UTC #24 lore raw

In reply to Miguel Oliveira

Quoted 3 lines · Miguel Oliveira

Creating this patch for the Eudyptula Challenge.
Replaced msleep() for a delay < 20ms with a usleep_range() between 10000us and 15000us. Also inserted a blank line after adeclaration.

As you are doing 2 different things in one patch, please break this up into two different patches. Remember, each patch can only do 1 thing.

That also lets us argue about the msleep() change, while I take your whitespace fix :)

thanks,

greg k-h

Miguel Oliveira

author

2014-05-30 16:05 UTC #25 lore raw

Greg should I make a patch for the whitespace while you are considering in remove the driver?

Regards

2014-05-30 17:01 GMT+01:00 Greg KH <[email protected]>:

Quoted 14 lines · Miguel Oliveira

Quoted 3 lines
Creating this patch for the Eudyptula Challenge.
Replaced msleep() for a delay < 20ms with a usleep_range() between 10000us and 15000us. Also inserted a blank line after adeclaration.
As you are doing 2 different things in one patch, please break this up into two different patches. Remember, each patch can only do 1 thing.
That also lets us argue about the msleep() change, while I take your whitespace fix :)
thanks,
greg k-h

Greg KH

maintainer

2014-05-30 16:13 UTC #26 lore raw

A: No.
Q: Should I include quotations after my reply?

http://daringfireball.net/2007/07/on_top

Quoted 2 lines · Miguel Oliveira

Greg should I make a patch for the whitespace while you are considering in remove the driver?

Yes, that would be fine, it will be another kernel release cycle before I remove any more drivers.

thanks,

greg k-h