Re: patch "staging: remove nokia_hp4p driver

Re: patch "staging: remove nokia_hp4p driver

[Pavel Machek]

Hi!

> This is a note to let you know that I've just added the patch titled
> 
>     staging: remove nokia_hp4p driver
> -Re: [PATCH v6] Bluetooth: Add hci_h4p driver
> 
> to my staging git tree which can be found at
>     git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git
> in the staging-next branch.
> 
> The patch will show up in the next release of the linux-next tree
> (usually sometime within the next 24 hours during the week.)
> 
> The patch will also be merged in the next major kernel release
> during the merge window.
> 
> If you have any questions about this process, please let me know.

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...?

Please don't, I'd still like to clean the driver up and get included,
as n900's are still under active use.
									Pavel

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

Re: patch "staging: remove nokia_hp4p driver

[Greg KH]

On Sat, Aug 30, 2014 at 11:30:22PM +0200, Pavel Machek wrote:
> Hi!
> 
> > This is a note to let you know that I've just added the patch titled
> > 
> >     staging: remove nokia_hp4p driver
> > -Re: [PATCH v6] Bluetooth: Add hci_h4p driver
> > 
> > to my staging git tree which can be found at
> >     git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git
> > in the staging-next branch.
> > 
> > The patch will show up in the next release of the linux-next tree
> > (usually sometime within the next 24 hours during the week.)
> > 
> > The patch will also be merged in the next major kernel release
> > during the merge window.
> > 
> > If you have any questions about this process, please let me know.
> 
> 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.

> 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.

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
standing in your way on applying patches, and you can work much quicker
to resolve the issues that Marcel has pointed out.

thanks,

greg k-h

Re: patch "staging: remove nokia_hp4p driver

[Pavel Machek]

Hi!

> > 
> > 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.

> > 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 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.
									Pavel

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

Re: patch "staging: remove nokia_hp4p driver

[Greg KH]

On Sun, Aug 31, 2014 at 12:44:30AM +0200, Pavel Machek wrote:
> Hi!
> 
> > > 
> > > 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.

I didn't see those, were they mixed in with the previous ones?

> > > 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.

Those 3 patches do nothing to address the issues that the bluetooth
maintainers have raised, right?

> > 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.

Look, the driver has been in my tree for a very long time with nothing
done on it.  The bluetooth maintainer has asked me to remove it as it
does not follow the proper api calls.  So I'll remove it.  It's a
staging driver, who cares about the "history" of it, just clean it up on
your own, and submit it to the bluetooth maintainers for inclusion in
the "real" part of the tree.

greg k-h

Re: patch "staging: remove nokia_hp4p driver

[Pavel Machek]

Hi!

> > > > 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.
> 
> I didn't see those, were they mixed in with the previous ones?

Well, I did sent them in reply to discussion... Do you want me to
resend them?

> > > > 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.
> 
> Those 3 patches do nothing to address the issues that the bluetooth
> maintainers have raised, right?

Bluetooth maintainers have raised a ton of issues, this addresses
"custom random address" part of them.

> > > 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.
> 
> Look, the driver has been in my tree for a very long time with nothing
> done on it.  The bluetooth maintainer has asked me to remove it as it

Very long time == 1 kernel release. When single patch takes 20 days to
apply, I'd not characterise it as very long time.
									Pavel

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

Re: patch "staging: remove nokia_hp4p driver

[Marcel Holtmann]

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

Re: patch "staging: remove nokia_hp4p driver

[Pali Rohár]

[-- 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 --]

Re: patch "staging: remove nokia_hp4p driver

[Pavel Machek]

Hi!

> >>> 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.

[Could you please wrap at 80 characters?]

Would be done in january if someone pointed the problems out.

For the record, 3/3 addresses your comment

"These are a lot of public functions. Are they all really needed or can
the code be done smart."

in 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.

Actually it did trigger me to work on the patch series above. It is
just that the series was ignored, and now I'm told that driver is
going to be removed because I did not do the work.

> 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

I thought staging is for merging code to be cleaned up. Not "please
rewrite, and possibly break the driver before merging it into
staging". Merge it into staging, then clean it up there.

(Also hopefully get some testing from n900 community; code is more
visible in staging).

And BTW it is first time I hear about a).

> 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.

So you are saying that most of the comments you had when I attempted
to merge the driver did not really need to be addressed? That's news
to me, normally maintainers want their comments addressed.

And yes, this driver bitroted a lot while being out of tree. It was
probably pretty good initially, mergeable with minimum effort. Now you
want it to bitrot a bit more.

> 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.
> 

I attempted hdev->setup conversion, but could not figure it out till
now. Clearly it needs to be done.

For doing that, it would be good to have userland to work with, and
yes it takes time. (Debugging on 4" screen sucks.)

> > 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.

Clearly. But at the same time not taking even the trivial patches is
great way of stalling the devopment.

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

Re: patch "staging: remove nokia_hp4p driver

[Marcel Holtmann]

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.
> 
> [Could you please wrap at 80 characters?]
> 
> Would be done in january if someone pointed the problems out.
> 
> For the record, 3/3 addresses your comment
> 
> "These are a lot of public functions. Are they all really needed or can
> the code be done smart."
> 
> in TODO file.

so what about all the other really major details that needed fixing. I mean anybody who was compiling for OMAP platform could have fixed the tons and tons of inlines quickly.

>> 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.
> 
> Actually it did trigger me to work on the patch series above. It is
> just that the series was ignored, and now I'm told that driver is
> going to be removed because I did not do the work.

The driver got removed, because nobody started the real work to make it an upstream 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
> 
> I thought staging is for merging code to be cleaned up. Not "please
> rewrite, and possibly break the driver before merging it into
> staging". Merge it into staging, then clean it up there.

The staging tree is for drivers to eventually go upstream. Staging is not a long term solution for a driver. It never was and never will be. So cleanup means whatever is required to get it upstream. And that means that you need to rewrite pieces.

For example a WiFi driver that ships its own 802.11 stack will never get upstream unless you re-write it to use mac80211.

> (Also hopefully get some testing from n900 community; code is more
> visible in staging).

The problem is that they are testing the wrong thing. And lets face it, we already knew this driver works since Nokia shipped it in products.

> And BTW it is first time I hear about a).

So the whole ARM community is moving over to DT and you are ignoring it. I complained about the platform data before. Someone told me that the N900 will be converted to DT. So I assume that this driver will also use DT then. If that was not clear, then now I made it clear.

>> 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.
> 
> So you are saying that most of the comments you had when I attempted
> to merge the driver did not really need to be addressed? That's news
> to me, normally maintainers want their comments addressed.

All the coding style and cleanups like moving code around could have been done with a few days. So if the 3 major items would have been fixed, then getting the driver into shape is dead simple.

> And yes, this driver bitroted a lot while being out of tree. It was
> probably pretty good initially, mergeable with minimum effort. Now you
> want it to bitrot a bit more.

Back in the days it was hard to merge since most of the OMAP stuff it depends on was not upstream. That why it never made it there. In addition that you need to build for specific OMAP boards makes it really hard to just take the driver. If it would have compiled on x86, I bet it would already been upstream.

>> 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.
>> 
> 
> I attempted hdev->setup conversion, but could not figure it out till
> now. Clearly it needs to be done.
> 
> For doing that, it would be good to have userland to work with, and
> yes it takes time. (Debugging on 4" screen sucks.)

Actually hdev->setup does not need any userland. As long as request_firmware() works, you are just fine. And since the driver already uses that in the first place, I assume it works.

The Nokia firmware files have been sequences of HCI commands since forever. And I pointed you towards __hci_cmd_sync early on. And we have good examples of that in use already.

>>> 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.
> 
> Clearly. But at the same time not taking even the trivial patches is
> great way of stalling the devopment.

I do not maintain staging. However in this specific case there were pretty clear instructions on what needed changing. You can check for yourself on how much got addressed.

Regards

Marcel

Re: patch "staging: remove nokia_hp4p driver

[Pavel Machek]

Hi!

> >> 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
> > 
> > I thought staging is for merging code to be cleaned up. Not "please
> > rewrite, and possibly break the driver before merging it into
> > staging". Merge it into staging, then clean it up there.
> 
> The staging tree is for drivers to eventually go upstream. Staging is not a long term solution for a driver. It never was and never will be. So cleanup means whatever is required to get it upstream. And that means that you need to rewrite pieces.
> 
> For example a WiFi driver that ships its own 802.11 stack will never get upstream unless you re-write it to use mac80211.
> 
> > (Also hopefully get some testing from n900 community; code is more
> > visible in staging).
> 
> The problem is that they are testing the wrong thing. And lets face it, we already knew this driver works since Nokia shipped it in products.
>

Testing is still useful after major changes, like changes you are
asking me to do.

> > And BTW it is first time I hear about a).
> 
> So the whole ARM community is moving over to DT and you are ignoring it. I complained about the platform data before. Someone told me that the N900 will be converted to DT. So I assume that this driver will also use DT then. If that was not clear, then now I made it clear.
>

Point noted.

> > And yes, this driver bitroted a lot while being out of tree. It was
> > probably pretty good initially, mergeable with minimum effort. Now you
> > want it to bitrot a bit more.
> 
> Back in the days it was hard to merge since most of the OMAP stuff it depends on was not upstream. That why it never made it there. In addition that you need to build for specific OMAP boards makes it really hard to just take the driver. If it would have compiled on x86, I bet it would already been upstream.
> 

Compiling for omap is really simple, testing is not.

> >> 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.
> >> 
> > 
> > I attempted hdev->setup conversion, but could not figure it out till
> > now. Clearly it needs to be done.
> > 
> > For doing that, it would be good to have userland to work with, and
> > yes it takes time. (Debugging on 4" screen sucks.)
> 
> Actually hdev->setup does not need any userland. As long as request_firmware() works, you are just fine. And since the driver already uses that in the first place, I assume it works.
>

Yes, I do need userland for debugging. The screen is 640x480, and I
don't have shift-pageup, so not enough screen space to debug without
userland. And no serial port.

If the hardware was available for x86 or suitable development board,
yes, development would be easier.

> The Nokia firmware files have been sequences of HCI commands since forever. And I pointed you towards __hci_cmd_sync early on. And we have good examples of that in use already.
>

Do you have an example of conversion to __hci_cmd_sync()? I that is
what would help here.

> >>> 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.
> > 
> > Clearly. But at the same time not taking even the trivial patches is
> > great way of stalling the devopment.
> 
> I do not maintain staging. However in this specific case there were pretty clear instructions on what needed changing. You can check for yourself on how much got addressed.
> 

You are not maintaining staging, so the driver in staging is no
additional work to you. Yet you insist driver must be removed,
creating additional work for me. When I pointed out that your reasons
for removal are actually untrue (patches were submitted), you point
out that you are not staging maintainer.

So, do you want to take responsibility for drivers/staging/nokia_h4p
or not?
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: patch "staging: remove nokia_hp4p driver

[Pavel Machek]

Hi!

> > "These are a lot of public functions. Are they all really needed or can
> > the code be done smart."
> > 
> > in TODO file.
> 
> so what about all the other really major details that needed fixing. I mean anybody who was compiling for OMAP platform could have fixed the tons and tons of inlines quickly.
> 

Do you have n900 nearby? You are welcome to try...

Unfortunately, it is not exactly easy. v3.18 with USB patch works with
nfsroot, which makes development possible (good).

Some core (or omap) change broke h4p between 3.15 and 3.16. Bisect
will not be fun, I suspect.

> > (Also hopefully get some testing from
> > n900 community; code is more
> > visible in staging).
> 
> The problem is that they are testing the wrong thing. And lets face it, we already knew this driver works since Nokia shipped it in products.
>

Except that it does not :-(. Apparently not even place in staging was
enough to get regular testing... but at least it will help bisect.

> > I attempted hdev->setup conversion, but could not figure it out till
> > now. Clearly it needs to be done.
> > 
> > For doing that, it would be good to have userland to work with, and
> > yes it takes time. (Debugging on 4" screen sucks.)
> 
> Actually hdev->setup does not need any userland. As long as request_firmware() works, you are just fine. And since the driver already uses that in the first place, I assume it works.
> 

Without working serial port, you can't really debug without
userland. Not on machine that lacks shift-pgup.

									Pavel

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