All of lore.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.