Message-ID: <AANLkTikMAWikj9HQ76YVeHr7pMoFK6ME2=HHpuQ1kUxq@mail.gmail.com>
Date:	Mon, 6 Dec 2010 13:01:46 +0100
From:	Par-Gunnar Hjalmdahl <pghatwork@...il.com>
To:	Vitaly Wool <vitalywool@...il.com>
Cc:	Alan Cox <alan@...rguk.ukuu.org.uk>, linus.walleij@...ricsson.com,
	linux-bluetooth@...r.kernel.org, linux-kernel@...r.kernel.org,
	Arnd Bergmann <arnd@...db.de>,
	Marcel Holtmann <marcel@...tmann.org>
Subject: Re: [PATCH 5/9] mfd: Add UART support for the ST-Ericsson CG2900.

Hi Vitaly,

2010/12/6 Vitaly Wool <vitalywool@...il.com>:
> Hi Par,
>
> On Mon, Dec 6, 2010 at 10:06 AM, Par-Gunnar Hjalmdahl
> <pghatwork@...il.com> wrote:
>
>>> So is it true that if there's the other chip with similar
>>> functionality I have to implement support for, I'll have to pretty
>>> much duplicate your line discipline driver?
>>>
>>
>> What will be in the protocol file, i.e. the same level as hci_h4.c,
>> hci_bcsp.c, etc, will be to ~80% vendor specific. The only H4 channels
>> that are in a standardized specification are the Bluetooth channels
>> 1-4. Other channels such as FM and GPS are vendor specific. I've seen
>> that for example TI has the same channels for FM and GPS, but these
>> channels, 8 and 9, are in no way standardized. The CG2900 also carries
>> a number of other channels, such as debug and device channels, which
>> I'm pretty sure is individual for this chip.
>> This identification of the channels is also only one function, even
>> though it is an important function. There are also a lot of other code
>> regarding baud rate settings, low power handling, etc that I'm
>> positive will not apply to chips of other vendors. It is of course
>> possible that other chips from ST-Ericsson, current and future, will
>> be able to reuse this file, but as I said I doubt that other vendors
>> might be able to use it.
>
> Wait. What level of abstraction is this file at if even some future
> STE chips are not guaranteed to work with it? Is it at a level of
> hardcoding the GPIO numbers?
>

There are no GPIOs or similar in the driver code; those are defined
and used in the board specific files under the /arch/ files. What
could be a problem with a future chip would be if for example the HCI
command to change baud rate would change. We have no indication that
this will change, but I cannot give a lifetime guarantee that nothing
will ever change with how the chip startup is handled.

>>> Isn't it better to have a new line discipline that the standard
>>> drivers (H4, LL, ...) will be applicable to?
>
>> I'm not certain what you mean. We are modifying the line discipline a
>> bit, but the only thing we have needed to do with existing protocol
>> drivers has been to add a boolean parameter for the protocol settings
>> (so they will register to the Bluetooth stack). I don't see why we
>> should create a new line discipline driver and then move the existing
>> protocol drivers to this.
>
> Okay, let me try to be more specific. There's for instance
> drivers/bluetooth/hci_ll.c which is the line discipline driver that is
> meant to work with any BT chip supporting LL protocol. Your solution
> seems to imply that one will have to create a variation of this
> implementation for each BT chip supporting LL that will use your
> shared transport implementation. Is it really the case?
>
> Thanks,
>   Vitaly
>

Well, from what I can see LL is an extension of the H4, basically it
adds sleep mode handling in a vendor specific way to the normal H4
protocol. So it is also based on the hci_h4 just as our file is. But
our file has basically nothing in common with what has been for the LL
file. We don't support any of the LL sleep commands for example.
So if I would make a driver for a combo chip supporting LL, I would
either modify the existing hci_ll.c or I would make a new file based
on hci_ll.c. There is not much you could really reuse from our new
file. Basically it would be the handling of any common channels, so if
you would for example have the same specification of FM and GPS you
could maybe save around 20 lines of common code, but you would
probably have to add a lot of more code just to keep the solution
generic.
One major difference is also that hci_ll never changes baud rate or
other settings. I assume that is done from hciattach during startup
instead. But we cannot run with that since we have to shut down the
chip when no user is connected in order to save power. This means that
we have to add vendor specific commands in order to for example set
baud rate. And then you run into these vendor specific problems. If
there would be a standardized specification on how to set baud rate
and how to put chip in sleep I assume things could be solved
differently, but that is not the case.

As a quick answer to your question: that would depend on the
difference between the different controllers, I guess. But CG2900
doesn't support the LL protocol so it is not an issue for that.

/P-G
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists