public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Pali Rohár" <pali.rohar@gmail.com>
To: Marcel Holtmann <marcel@holtmann.org>
Cc: Pavel Machek <pavel@ucw.cz>, Greg KH <gregkh@linuxfoundation.org>,
	Miguel Oliveira <cmroliv@gmail.com>,
	kernel list <linux-kernel@vger.kernel.org>,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: patch "staging: remove nokia_hp4p driver
Date: Sun, 31 Aug 2014 10:23:57 +0200	[thread overview]
Message-ID: <201408311023.57669@pali> (raw)
In-Reply-To: <B39194F7-2057-4D14-A3F7-EB6E73BF84F8@holtmann.org>

[-- Attachment #1: Type: Text/Plain, Size: 3852 bytes --]

On Sunday 31 August 2014 01:09:01 Marcel Holtmann wrote:
> Hi Pavel,
> 
> >>> What is going on here? I get flamed for not cleaning up
> >>> the driver, because I cleaned it up before merging to
> >>> -staging. Ok, so I did more cleanups, sent 3 cleanup
> >>> patches, no reaction on those, and now I got a note that
> >>> you are going to remove the driver...?
> >> 
> >> For the 3 "cleanup" patches, the first one was rejected and
> >> you said to not include it, so I couldn't apply the
> >> others.
> > 
> > That was different series. I'm talking about:
> > 
> > [PATCH 1/3] staging: nokia_h4: switch to right types and use
> > bdaddr_t [PATCH 2/3] staging: nokia_h4: avoid __uX types
> > [PATCH 3/3] staging: use inlines where it makes sense
> > 
> > That is still valid and received no comments at all.
> 
> and these are all trivial cleanups and could have been done
> back in January when this driver was merged into staging. It
> is end of August now and nothing happened to address anything
> in the TODO file.
> 
> The warning from end of May that this driver will be removed
> seemed to not have triggered anybody to work on the core
> issues of the driver.
> 
> There are 3 major topics that needs addressing before this
> driver should come anywhere near upstream kernel again,
> staging or otherwise.
> 
> a) Convert to using device tree for device detection
> 
> b) Convert to using hdev->setup for firmware loading
> 
> c) Convert to using hdev->set_bdaddr and
> HCI_QUIRK_INVALID_BDADDR
> 
> Please keep in mind that this was not an ugly Windows driver
> with a lot of Windows specific typedefs or bad coding style
> or OS abstractions. From that point of view it was always
> good since it was written for Linux in the first place. It
> was just a bit dated. Any fixes to bring that in sync with
> all other drivers could have been done easily after it was
> merged into the Bluetooth subsystem.
> 
> The reason why it ended up in staging is that the 3 core
> problems needed fixing. And 8 month later they still have not
> been fixed.
> 
> >>> Please don't, I'd still like to clean the driver up and
> >>> get included, as n900's are still under active use.
> >> 
> >> As the Bluetooth maintainer has said a number of times, he
> >> doesn't want the driver in the tree as it is not doing the
> >> correct things.  It's been a long time in the tree with no
> >> work on it at all, and I follow the suggestions of the
> >> maintainers of the subsystems that staging drivers follow.
> > 
> > You asked for more work and explained how easy it is to
> > revert the removal.
> > 
> > I did more work, you ignored it, and are removing the
> > driver, anyway.
> 
> I have seen only fluff patches. Someone needs to address the
> core problems of the driver.
> 
> >> I suggest cleaning this up  in your own tree, and then just
> >> submitting it for inclusion in the "normal" part of the
> >> kernel.  That way I'm not
> > 
> > ...creating a mess in the history, and fun merge problems
> > for people actually using the driver :-(. And yes, n900
> > people actually are using it and have their own changes on
> > top of it.
> 
> That is even worse. We have a staging driver with external
> patches on top of it. Getting a driver into staging has an
> almost zero barrier and then people still not get their
> patches merged into staging. That is just plain said.
> 
> Regards
> 
> Marcel

Hello, external patch is only renaming driver name from 
btnokia_h4p to hci_h4p - nothing more. And it is only because of 
Maemo 5 userspace compatibility (which I fix when driver will be 
in regular bluetooth tree and will be stable). Driver itself is 
working (with any userspace). So it is not as bad as you think.

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

  reply	other threads:[~2014-08-31  8:24 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <14094295422869@kroah.com>
2014-08-30 21:30 ` patch "staging: remove nokia_hp4p driver Pavel Machek
2014-08-30 22:04   ` Greg KH
2014-08-30 22:44     ` Pavel Machek
2014-08-30 22:49       ` Greg KH
2014-08-31  9:28         ` Pavel Machek
2014-08-30 23:09       ` Marcel Holtmann
2014-08-31  8:23         ` Pali Rohár [this message]
2014-08-31  9:51         ` Pavel Machek
2014-09-01 22:16           ` Marcel Holtmann
2014-09-04 11:36             ` Pavel Machek
2014-12-03 12:33             ` Pavel Machek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=201408311023.57669@pali \
    --to=pali.rohar@gmail.com \
    --cc=cmroliv@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marcel@holtmann.org \
    --cc=pavel@ucw.cz \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox