All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marcel Holtmann <marcel@holtmann.org>
To: Pavel Machek <pavel@ucw.cz>
Cc: "Pali Rohár" <pali.rohar@gmail.com>,
	"Sebastian Reichel" <sre@debian.org>,
	"Sebastian Reichel" <sre@ring0.de>,
	"kernel list" <linux-kernel@vger.kernel.org>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	"Linux OMAP Mailing List" <linux-omap@vger.kernel.org>,
	"Tony Lindgren" <tony@atomide.com>,
	khilman@kernel.org, "Aaro Koskinen" <aaro.koskinen@iki.fi>,
	"Ивайло Димитров" <ivo.g.dimitrov.75@gmail.com>,
	"Gustavo F. Padovan" <gustavo@padovan.org>,
	"Johan Hedberg" <johan.hedberg@gmail.com>,
	linux-bluetooth@vger.kernel.org
Subject: Re: __hci_cmd_sync() not suitable for nokia h4p
Date: Fri, 12 Dec 2014 13:28:00 +0100	[thread overview]
Message-ID: <45FED76E-F0E2-4810-8D79-64C5D1693EFA@holtmann.org> (raw)
In-Reply-To: <20141212095153.GB2905@amd>

Hi Pavel,

>>>>>> What needs to be done is the bring up of the device including the proper UART settings and speed and then just run the firmware downloads. All firmware files on the Nokia devices where just HCI commands with vendor specific details. Some from CSR, some from Broadcom and some from TI. You can actually decode them if you really want to. Shouldn't be that hard.
>>>>>> 
>>>>> 
>>>>> Speed changes at the end of firmware load, but I guess that's detail?
>>>>> Anyway, patch would look like this.
>>>> 
>>>> You should really look into providing hdev->setup() callback. That is normally the callback where you want to load the firmware.
>>>> 
>>> 
>>> I can provide setup() callback and load firmware from there.
>>> 
>>> I have created provisional device tree binding, and the driver still
>>> works.
>>> 
>>> Some time ago you mentioned that with the "big" issues fixed, you'd be
>>> willing to take it into the tree. What way forward do you see? Would
>>> it make sense to re-enable the driver in staging, so that "big"
>>> changes could be applied, followed by renames?
>> 
>> If the driver is clean, there is no point in taking it through staging. It can be cleaned up and then merged all together.
>> 
> 
> Ok, so you can revert a4102f90e87cfaa3fdbed6fdf469b23f0eeb4bfd in your
> tree, and I can post patches against that?

just treat this a submission of a new driver.

>> I think the important part is to focus on the N900 derivative with
>> the Broadcom chip inside. And just ignore all the rest. So start
>> small and do not try to carry the N770, N800, N810 hacks over.
> 
> Well, I did remove non-relevant firmware loaders, and I can remove the
> switches for different firmware loaders, too, but spending time
> removing support for different hardware does not sound quite right.

Lets face it, you are not getting it upstream that way. If nobody has the hardware to test it or cares about the hardware anymore, then this should not be upstream in the first place.

Make your life easier and get your hardware supported. Then someone can evolve this for older Nokia devices. But I am not taking code that nobody has tested.

Keep it simple is really the only way to get this driver merged. There is so much old cruft in there that you are better off only caring about the N900 version of the hardware.

>> However you might want to check Neil Brown's patches for TTY-slave
>> devices he just posted. Maybe the N900 devices should be exposed as
>> TTY and we just attach N_HCI line discipline to it. If all the GPIO
>> wiggling can be done automatically at TTY open, then that should be
>> the way to go. I also send an email about using the new unconfigured
>> stage to get this done cleanly.
> 
> I seen them, but they don't help, I'm afraid. The GPIOs are used for
> power saving, and they are used in various places including the serial
> irq handler.

That is something that might need some extra work on it, but it seems that the general direction of TTY slaves is the right one.

Anyway, the main order of business is really that it supports DT and that it can be compiled on every single platform and you could even load the driver on x86 without doing any harm.

Regards

Marcel


WARNING: multiple messages have this Message-ID (diff)
From: marcel@holtmann.org (Marcel Holtmann)
To: linux-arm-kernel@lists.infradead.org
Subject: __hci_cmd_sync() not suitable for nokia h4p
Date: Fri, 12 Dec 2014 13:28:00 +0100	[thread overview]
Message-ID: <45FED76E-F0E2-4810-8D79-64C5D1693EFA@holtmann.org> (raw)
In-Reply-To: <20141212095153.GB2905@amd>

Hi Pavel,

>>>>>> What needs to be done is the bring up of the device including the proper UART settings and speed and then just run the firmware downloads. All firmware files on the Nokia devices where just HCI commands with vendor specific details. Some from CSR, some from Broadcom and some from TI. You can actually decode them if you really want to. Shouldn't be that hard.
>>>>>> 
>>>>> 
>>>>> Speed changes at the end of firmware load, but I guess that's detail?
>>>>> Anyway, patch would look like this.
>>>> 
>>>> You should really look into providing hdev->setup() callback. That is normally the callback where you want to load the firmware.
>>>> 
>>> 
>>> I can provide setup() callback and load firmware from there.
>>> 
>>> I have created provisional device tree binding, and the driver still
>>> works.
>>> 
>>> Some time ago you mentioned that with the "big" issues fixed, you'd be
>>> willing to take it into the tree. What way forward do you see? Would
>>> it make sense to re-enable the driver in staging, so that "big"
>>> changes could be applied, followed by renames?
>> 
>> If the driver is clean, there is no point in taking it through staging. It can be cleaned up and then merged all together.
>> 
> 
> Ok, so you can revert a4102f90e87cfaa3fdbed6fdf469b23f0eeb4bfd in your
> tree, and I can post patches against that?

just treat this a submission of a new driver.

>> I think the important part is to focus on the N900 derivative with
>> the Broadcom chip inside. And just ignore all the rest. So start
>> small and do not try to carry the N770, N800, N810 hacks over.
> 
> Well, I did remove non-relevant firmware loaders, and I can remove the
> switches for different firmware loaders, too, but spending time
> removing support for different hardware does not sound quite right.

Lets face it, you are not getting it upstream that way. If nobody has the hardware to test it or cares about the hardware anymore, then this should not be upstream in the first place.

Make your life easier and get your hardware supported. Then someone can evolve this for older Nokia devices. But I am not taking code that nobody has tested.

Keep it simple is really the only way to get this driver merged. There is so much old cruft in there that you are better off only caring about the N900 version of the hardware.

>> However you might want to check Neil Brown's patches for TTY-slave
>> devices he just posted. Maybe the N900 devices should be exposed as
>> TTY and we just attach N_HCI line discipline to it. If all the GPIO
>> wiggling can be done automatically at TTY open, then that should be
>> the way to go. I also send an email about using the new unconfigured
>> stage to get this done cleanly.
> 
> I seen them, but they don't help, I'm afraid. The GPIOs are used for
> power saving, and they are used in various places including the serial
> irq handler.

That is something that might need some extra work on it, but it seems that the general direction of TTY slaves is the right one.

Anyway, the main order of business is really that it supports DT and that it can be compiled on every single platform and you could even load the driver on x86 without doing any harm.

Regards

Marcel

  reply	other threads:[~2014-12-12 12:28 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-09 19:02 __hci_cmd_sync() not suitable for nokia h4p Pavel Machek
2014-12-09 19:02 ` Pavel Machek
2014-12-09 19:07 ` Marcel Holtmann
2014-12-09 19:07   ` Marcel Holtmann
2014-12-09 20:13   ` Pavel Machek
2014-12-09 20:13     ` Pavel Machek
2014-12-09 21:19     ` Marcel Holtmann
2014-12-09 21:19       ` Marcel Holtmann
2014-12-10 13:15       ` Pavel Machek
2014-12-10 13:15         ` Pavel Machek
2014-12-11 12:58         ` Marcel Holtmann
2014-12-11 12:58           ` Marcel Holtmann
2014-12-11 22:13           ` Pavel Machek
2014-12-11 22:13             ` Pavel Machek
2014-12-11 22:25             ` Marcel Holtmann
2014-12-11 22:25               ` Marcel Holtmann
2014-12-12  9:51               ` Pavel Machek
2014-12-12  9:51                 ` Pavel Machek
2014-12-12 12:28                 ` Marcel Holtmann [this message]
2014-12-12 12:28                   ` Marcel Holtmann
2014-12-12  1:15             ` Sebastian Reichel
2014-12-12  1:15               ` Sebastian Reichel
2014-12-12 12:14               ` Pavel Machek
2014-12-12 12:14                 ` Pavel Machek
2014-12-13 17:35                 ` Sebastian Reichel
2014-12-13 17:35                   ` Sebastian Reichel

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=45FED76E-F0E2-4810-8D79-64C5D1693EFA@holtmann.org \
    --to=marcel@holtmann.org \
    --cc=aaro.koskinen@iki.fi \
    --cc=gustavo@padovan.org \
    --cc=ivo.g.dimitrov.75@gmail.com \
    --cc=johan.hedberg@gmail.com \
    --cc=khilman@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-bluetooth@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=pali.rohar@gmail.com \
    --cc=pavel@ucw.cz \
    --cc=sre@debian.org \
    --cc=sre@ring0.de \
    --cc=tony@atomide.com \
    /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.