[PATCH] Staging: btmtk_usb: Add hdev parameter to hdev->send driver callback

[PATCH] Staging: btmtk_usb: Add hdev parameter to hdev->send driver callback

[Geert Uytterhoeven]

drivers/staging/btmtk_usb/btmtk_usb.c: In function ‘btmtk_usb_probe’:
drivers/staging/btmtk_usb/btmtk_usb.c:1610: warning: assignment from incompatible pointer type

Add the new hdev parameter, cfr. commit
7bd8f09f69f8a190f9b8334a07bb0a9237612314 ("Bluetooth: Add hdev parameter to
hdev->send driver callback").

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 drivers/staging/btmtk_usb/btmtk_usb.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/btmtk_usb/btmtk_usb.c b/drivers/staging/btmtk_usb/btmtk_usb.c
index 7a9bf3b57810..9a5ebd6cc512 100644
--- a/drivers/staging/btmtk_usb/btmtk_usb.c
+++ b/drivers/staging/btmtk_usb/btmtk_usb.c
@@ -1284,9 +1284,8 @@ done:
 	kfree_skb(skb);
 }
 
-static int btmtk_usb_send_frame(struct sk_buff *skb)
+static int btmtk_usb_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
 {
-	struct hci_dev *hdev = (struct hci_dev *)skb->dev;
 	struct btmtk_usb_data *data = hci_get_drvdata(hdev);
 	struct usb_ctrlrequest *dr;
 	struct urb *urb;
-- 
1.7.9.5

Re: [PATCH] Staging: btmtk_usb: Add hdev parameter to hdev->send driver callback

[Marcel Holtmann]

Hi Geert,

> drivers/staging/btmtk_usb/btmtk_usb.c: In function ‘btmtk_usb_probe’:
> drivers/staging/btmtk_usb/btmtk_usb.c:1610: warning: assignment from incompatible pointer type
> 
> Add the new hdev parameter, cfr. commit
> 7bd8f09f69f8a190f9b8334a07bb0a9237612314 ("Bluetooth: Add hdev parameter to
> hdev->send driver callback").
> 
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> ---
> drivers/staging/btmtk_usb/btmtk_usb.c |    3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/btmtk_usb/btmtk_usb.c b/drivers/staging/btmtk_usb/btmtk_usb.c
> index 7a9bf3b57810..9a5ebd6cc512 100644
> --- a/drivers/staging/btmtk_usb/btmtk_usb.c
> +++ b/drivers/staging/btmtk_usb/btmtk_usb.c
> @@ -1284,9 +1284,8 @@ done:
> 	kfree_skb(skb);
> }
> 
> -static int btmtk_usb_send_frame(struct sk_buff *skb)
> +static int btmtk_usb_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
> {
> -	struct hci_dev *hdev = (struct hci_dev *)skb->dev;
> 	struct btmtk_usb_data *data = hci_get_drvdata(hdev);
> 	struct usb_ctrlrequest *dr;
> 	struct urb *urb;

while this is patch is correct, I do not really care about staging drivers that actually bluntly violate my copyright.

Regards

Marcel

Re: [PATCH] Staging: btmtk_usb: Add hdev parameter to hdev->send driver callback

[Dan Carpenter]

On Fri, Nov 15, 2013 at 09:30:04PM +0900, Marcel Holtmann wrote:
> 
> while this is patch is correct, I do not really care about staging drivers that actually bluntly violate my copyright.
> 

That's very cryptic.

What is going on here?  I googled it and I wasn't able to find what you
are talking about.  Care to give us a hint and what you want us to do
here?

I have also added Johan Hedberg to the CC list because he also helped
break the build.  Don't do that.

regards,
dan carpenter

Re: [PATCH] Staging: btmtk_usb: Add hdev parameter to hdev->send driver callback

[Marcel Holtmann]

Hi Dan,

>> while this is patch is correct, I do not really care about staging drivers that actually bluntly violate my copyright.
>> 
> 
> That's very cryptic.
> 
> What is going on here?  I googled it and I wasn't able to find what you
> are talking about.  Care to give us a hint and what you want us to do
> here?

the last time I checked, the majority of drivers/bluetooth/btusb.c has been written by myself. Now go and compare btusb.c to btmtk_usb.[ch].

> I have also added Johan Hedberg to the CC list because he also helped
> break the build.  Don't do that.

Yes, we are doing exactly that. It is a staging driver. I could not care less if a staging drivers breaks the build or not.

If anybody cares about this driver, then take the time to merge it upstream. It has never been submitted to linux-bluetooth mailing list.

There are drivers that should have never been merged into staging. This is one of them. Look for yourself and explain to me why this driver is part of staging in the first place.

Regards

Marcel

Re: [PATCH] Staging: btmtk_usb: Add hdev parameter to hdev->send driver callback

[Greg Kroah-Hartman]

On Fri, Nov 15, 2013 at 10:26:41PM +0900, Marcel Holtmann wrote:
> Hi Dan,
> 
> >> while this is patch is correct, I do not really care about staging drivers that actually bluntly violate my copyright.
> >> 
> > 
> > That's very cryptic.
> > 
> > What is going on here?  I googled it and I wasn't able to find what you
> > are talking about.  Care to give us a hint and what you want us to do
> > here?
> 
> the last time I checked, the majority of drivers/bluetooth/btusb.c has been written by myself. Now go and compare btusb.c to btmtk_usb.[ch].
> 
> > I have also added Johan Hedberg to the CC list because he also helped
> > break the build.  Don't do that.
> 
> Yes, we are doing exactly that. It is a staging driver. I could not care less if a staging drivers breaks the build or not.
> 
> If anybody cares about this driver, then take the time to merge it upstream. It has never been submitted to linux-bluetooth mailing list.
> 
> There are drivers that should have never been merged into staging.
> This is one of them. Look for yourself and explain to me why this
> driver is part of staging in the first place.

Because it was sent to me by a developer?

Seriously, what's the issue here, I didn't notice it was a fork of your
code, sorry, I didn't check.  I figured it would be eventually cleaned
up properly and then it will be sent to linux-bluetooth for proper
merging.

Yu-Chen, what's the satus of getting this cleaned up "properly"?  You
haven't really done anything on this since I took the driver in May.

thanks,

greg k-h

Re: [PATCH] Staging: btmtk_usb: Add hdev parameter to hdev->send driver callback

[Marcel Holtmann]

Hi Greg,

>>>> while this is patch is correct, I do not really care about staging drivers that actually bluntly violate my copyright.
>>>> 
>>> 
>>> That's very cryptic.
>>> 
>>> What is going on here?  I googled it and I wasn't able to find what you
>>> are talking about.  Care to give us a hint and what you want us to do
>>> here?
>> 
>> the last time I checked, the majority of drivers/bluetooth/btusb.c has been written by myself. Now go and compare btusb.c to btmtk_usb.[ch].
>> 
>>> I have also added Johan Hedberg to the CC list because he also helped
>>> break the build.  Don't do that.
>> 
>> Yes, we are doing exactly that. It is a staging driver. I could not care less if a staging drivers breaks the build or not.
>> 
>> If anybody cares about this driver, then take the time to merge it upstream. It has never been submitted to linux-bluetooth mailing list.
>> 
>> There are drivers that should have never been merged into staging.
>> This is one of them. Look for yourself and explain to me why this
>> driver is part of staging in the first place.
> 
> Because it was sent to me by a developer?

it is a problem when staging just becomes a dumping ground for drivers that the distributions find somewhere on the Internet or CD-ROMs. And then nobody has any intentions to clean up and integrate properly. This one did not even go through linux-bluetooth mailing list once. It was submitted right to staging. And then the submitter walked away.

> Seriously, what's the issue here, I didn't notice it was a fork of your
> code, sorry, I didn't check.  I figured it would be eventually cleaned
> up properly and then it will be sent to linux-bluetooth for proper
> merging.

Kernel subsystem maintainers should not be responsible for fixing staging drivers. I did not even know that this driver existed in staging. I remember you saying that we can just ignore staging. The group of people looking after staging will take care of drivers that break.

Actually I am taken personal offense when someone takes my code, removes my copyright and slaps their copyright notice on top of it. Yes, I am looking at you Mediatek. I feel completely in my right to say that I am not touching this driver or care if it breaks.

And of course there is the technical problem that this driver as it is should not exist in the first place. We do not need duplicated code. The only difference between btusb.c and btmtk_usb.c is the driver init for loading firmware and poking at vendor specific registers. The Bluetooth subsystem has a vendor specific driver setup stage for exactly this. That should be used instead of forking the driver.

> Yu-Chen, what's the satus of getting this cleaned up "properly"?  You
> haven't really done anything on this since I took the driver in May.

My comment above stands, distributions do not seem to care :(

The Realtek Bluetooth driver is the same mess. I rejected it for the same reasons, but so far nobody made the effort to clean it up and build it as mini-driver of btusb.c.

Only our Intel guys managed to get that one done properly for their hardware. So yes, it can be done. I am not talking about unicorns here ;)

Regards

Marcel

Re: [PATCH] Staging: btmtk_usb: Add hdev parameter to hdev->send driver callback

[Marcel Holtmann]

Hi Greg,

>>>>> while this is patch is correct, I do not really care about staging drivers that actually bluntly violate my copyright.
>>>>> 
>>>> 
>>>> That's very cryptic.
>>>> 
>>>> What is going on here?  I googled it and I wasn't able to find what you
>>>> are talking about.  Care to give us a hint and what you want us to do
>>>> here?
>>> 
>>> the last time I checked, the majority of drivers/bluetooth/btusb.c has been written by myself. Now go and compare btusb.c to btmtk_usb.[ch].
>>> 
>>>> I have also added Johan Hedberg to the CC list because he also helped
>>>> break the build.  Don't do that.
>>> 
>>> Yes, we are doing exactly that. It is a staging driver. I could not care less if a staging drivers breaks the build or not.
>>> 
>>> If anybody cares about this driver, then take the time to merge it upstream. It has never been submitted to linux-bluetooth mailing list.
>>> 
>>> There are drivers that should have never been merged into staging.
>>> This is one of them. Look for yourself and explain to me why this
>>> driver is part of staging in the first place.
>> 
>> Because it was sent to me by a developer?
> 
> it is a problem when staging just becomes a dumping ground for drivers that the distributions find somewhere on the Internet or CD-ROMs. And then nobody has any intentions to clean up and integrate properly. This one did not even go through linux-bluetooth mailing list once. It was submitted right to staging. And then the submitter walked away.

and if I quote the TODO file:

  TODO:
          - checkpatch.pl clean
          - determine if the driver should not be using a duplicate
            version of the usb-bluetooth interface code, but should
            be merged into the drivers/bluetooth/ directory and
            infrastructure instead.
          - review by the bluetooth developer community

  Please send any patches for this driver to Yu-Chen, Cho <acho@suse.com> and
  jay.hung@mediatek.com

So from the submission we can assume that the submitter knew that this was duplicated code. The code also never got submitted for review to linux-bluetooth. And now 6 month later, none of the TODO items have been actually worked on.

I do not know what your timeline is for removing drivers from staging, but this one seems to be a good candidate to get removed next.

Regards

Marcel

Re: [PATCH] Staging: btmtk_usb: Add hdev parameter to hdev->send driver callback

[Greg Kroah-Hartman]

On Sat, Nov 16, 2013 at 07:36:31AM +0900, Marcel Holtmann wrote:
> Hi Greg,
> 
> >>>>> while this is patch is correct, I do not really care about staging drivers that actually bluntly violate my copyright.
> >>>>> 
> >>>> 
> >>>> That's very cryptic.
> >>>> 
> >>>> What is going on here?  I googled it and I wasn't able to find what you
> >>>> are talking about.  Care to give us a hint and what you want us to do
> >>>> here?
> >>> 
> >>> the last time I checked, the majority of drivers/bluetooth/btusb.c has been written by myself. Now go and compare btusb.c to btmtk_usb.[ch].
> >>> 
> >>>> I have also added Johan Hedberg to the CC list because he also helped
> >>>> break the build.  Don't do that.
> >>> 
> >>> Yes, we are doing exactly that. It is a staging driver. I could not care less if a staging drivers breaks the build or not.
> >>> 
> >>> If anybody cares about this driver, then take the time to merge it upstream. It has never been submitted to linux-bluetooth mailing list.
> >>> 
> >>> There are drivers that should have never been merged into staging.
> >>> This is one of them. Look for yourself and explain to me why this
> >>> driver is part of staging in the first place.
> >> 
> >> Because it was sent to me by a developer?
> > 
> > it is a problem when staging just becomes a dumping ground for drivers that the distributions find somewhere on the Internet or CD-ROMs. And then nobody has any intentions to clean up and integrate properly. This one did not even go through linux-bluetooth mailing list once. It was submitted right to staging. And then the submitter walked away.
> 
> and if I quote the TODO file:
> 
>   TODO:
>           - checkpatch.pl clean
>           - determine if the driver should not be using a duplicate
>             version of the usb-bluetooth interface code, but should
>             be merged into the drivers/bluetooth/ directory and
>             infrastructure instead.
>           - review by the bluetooth developer community
> 
>   Please send any patches for this driver to Yu-Chen, Cho <acho@suse.com> and
>   jay.hung@mediatek.com
> 
> So from the submission we can assume that the submitter knew that this
> was duplicated code. The code also never got submitted for review to
> linux-bluetooth. And now 6 month later, none of the TODO items have
> been actually worked on.
> 
> I do not know what your timeline is for removing drivers from staging,
> but this one seems to be a good candidate to get removed next.

6 months without any active contribution to getting it cleaned up and
merged is the timeline.  Which this one fits, so yes, I will remove it
for 3.14, unless Jay or Cho is going to start doing work on this.

thanks,

greg k-h