This is the 3rd patch set for the ST-Ericsson CG2900 connectivity
controller. The CG2900 is a combo controller supporting GPS, Bluetooth,
and FM radio. It uses HCI H:4 protocol to combine different functionalities
on a common transport, where first byte in the data indicates the current
channel. Channels 1-4 are standardized in the Bluetooth Core specification
while the other channels are vendor specific.
Compared to 2nd patch set this patch set has the following changes:
* UART handling is moved from mfd to bluetooth folder. It now reuses the
existing N_HCI line discipline.
* mfd creation has been moved from cg2900_core into chip specific files.
* All information for each channel, including API functions, exist in each
MFD devices, making them independent of each other.
* All chip specific information has been moved from cg2900_core into the
chip specific files. cg2900_core now only handles registration and
connection between transport and chip driver.
* Fixes for several review comments including use of existing debug system.
Par-Gunnar Hjalmdahl (11):
mfd: Add support for CG2900 controller framework
mfd: Add CG2900 character devices
mfd: Add support for CG2900 controller
mfd: Add support for STLC2690 controller
mfd: Add CG2900 audio
mfd: Add CG2900 test character device
Bluetooth: Add UART API functions to ldisc
Bluetooth: Add support for CG2900 UART
Bluetooth: Add support for CG2900 controller
arch_mach-ux500: Add U8500 board support for CG2900
Bluetooth and mach-ux500: Fix of minor issues
arch/arm/mach-ux500/Makefile | 1 +
arch/arm/mach-ux500/board-mop500.c | 152 ++
arch/arm/mach-ux500/devices-cg2900.c | 315 +++
arch/arm/mach-ux500/devices-cg2900.h | 19 +
drivers/bluetooth/Kconfig | 7 +
drivers/bluetooth/Makefile | 2 +
drivers/bluetooth/btcg2900.c | 1134 ++++++++++
drivers/bluetooth/cg2900_uart.c | 1849 ++++++++++++++++
drivers/bluetooth/hci_ath.c | 1 +
drivers/bluetooth/hci_bcsp.c | 3 +-
drivers/bluetooth/hci_h4.c | 1 +
drivers/bluetooth/hci_ldisc.c | 101 +-
drivers/bluetooth/hci_ll.c | 1 +
drivers/bluetooth/hci_uart.h | 18 +-
drivers/mfd/Kconfig | 53 +
drivers/mfd/Makefile | 2 +
drivers/mfd/cg2900/Makefile | 16 +
drivers/mfd/cg2900/cg2900_audio.c | 3415 ++++++++++++++++++++++++++++++
drivers/mfd/cg2900/cg2900_char_devices.c | 701 ++++++
drivers/mfd/cg2900/cg2900_chip.c | 3250 ++++++++++++++++++++++++++++
drivers/mfd/cg2900/cg2900_chip.h | 602 ++++++
drivers/mfd/cg2900/cg2900_core.c | 711 +++++++
drivers/mfd/cg2900/cg2900_core.h | 51 +
drivers/mfd/cg2900/cg2900_lib.c | 391 ++++
drivers/mfd/cg2900/cg2900_lib.h | 61 +
drivers/mfd/cg2900/cg2900_test.c | 402 ++++
drivers/mfd/cg2900/stlc2690_chip.c | 1673 +++++++++++++++
drivers/mfd/cg2900/stlc2690_chip.h | 47 +
include/linux/mfd/cg2900.h | 287 +++
include/linux/mfd/cg2900_audio.h | 473 +++++
include/net/bluetooth/hci.h | 5 +
31 files changed, 15734 insertions(+), 10 deletions(-)
create mode 100644 arch/arm/mach-ux500/devices-cg2900.c
create mode 100644 arch/arm/mach-ux500/devices-cg2900.h
create mode 100644 drivers/bluetooth/btcg2900.c
create mode 100644 drivers/bluetooth/cg2900_uart.c
create mode 100644 drivers/mfd/cg2900/Makefile
create mode 100644 drivers/mfd/cg2900/cg2900_audio.c
create mode 100644 drivers/mfd/cg2900/cg2900_char_devices.c
create mode 100644 drivers/mfd/cg2900/cg2900_chip.c
create mode 100644 drivers/mfd/cg2900/cg2900_chip.h
create mode 100644 drivers/mfd/cg2900/cg2900_core.c
create mode 100644 drivers/mfd/cg2900/cg2900_core.h
create mode 100644 drivers/mfd/cg2900/cg2900_lib.c
create mode 100644 drivers/mfd/cg2900/cg2900_lib.h
create mode 100644 drivers/mfd/cg2900/cg2900_test.c
create mode 100644 drivers/mfd/cg2900/stlc2690_chip.c
create mode 100644 drivers/mfd/cg2900/stlc2690_chip.h
create mode 100644 include/linux/mfd/cg2900.h
create mode 100644 include/linux/mfd/cg2900_audio.h
--
1.7.3.2
Hi Par,
On Fri, Dec 17, 2010 at 12:20 PM, Par-Gunnar Hjalmdahl
<[email protected]> wrote:
> This is the 3rd patch set for the ST-Ericsson CG2900 connectivity
> controller. The CG2900 is a combo controller supporting GPS, Bluetooth,
> and FM radio. It uses HCI H:4 protocol to combine different functionalities
> on a common transport, where first byte in the data indicates the current
> channel. Channels 1-4 are standardized in the Bluetooth Core specification
> while the other channels are vendor specific.
>
> Compared to 2nd patch set this patch set has the following changes:
> ?* UART handling is moved from mfd to bluetooth folder. It now reuses the
> ? existing N_HCI line discipline.
> ?* mfd creation has been moved from cg2900_core into chip specific files.
> ?* All information for each channel, including API functions, exist in each
> ? MFD devices, making them independent of each other.
> ?* All chip specific information has been moved from cg2900_core into the
> ? chip specific files. cg2900_core now only handles registration and
> ? connection between transport and chip driver.
> ?* Fixes for several review comments including use of existing debug system.
this patchset has only multiplied my confusion.
Not really diving into the details, I'm just trying to understand how
I'd rewrite the TI shared transport using your approach if I had to.
It looks like I'd have to, at least:
- implement a heavyweight line discipline driver (like cg2900_uart.c)
half duplicating the functionality in yours;
- implement specific MFD driver.
Actually the reuse of this implementation will be at a level of
statistics error. The only thing to reuse is actually the approach
which I'm not really happy about.
So...
If you are aiming for a generic solution, then why you get more and
more things handled in cg2900-specific code (e. g. packet routing
which is fairly generic)?
If you are solving your particular problems, isn't the change too
dramatic e. g. when it comes to line discipline drivers and baudrate
manipulation?
As of now, I really see no reason why anyone would promote this
solution instead of making something more generic out of ti-st and
reusing it then.
Thanks,
Vitaly
Hi Vitaly,
> -----Original Message-----
> From: Vitaly Wool [mailto:[email protected]]
> Sent: den 17 december 2010 13:11
> To: Par-Gunnar HJALMDAHL
> Cc: Pavan Savoy; Alan Cox; Arnd Bergmann; Samuel Ortiz; Marcel
> Holtmann; [email protected]; linux-
> [email protected]; Lukasz Rymanowski; Linus WALLEIJ; Par-Gunnar
> Hjalmdahl
> Subject: Re: [PATCH 00/11] mfd and bluetooth: Add CG2900 support
>
> Hi Par,
>
> On Fri, Dec 17, 2010 at 12:20 PM, Par-Gunnar Hjalmdahl
> <[email protected]> wrote:
> > This is the 3rd patch set for the ST-Ericsson CG2900 connectivity
> > controller. The CG2900 is a combo controller supporting GPS,
> Bluetooth,
> > and FM radio. It uses HCI H:4 protocol to combine different
> functionalities
> > on a common transport, where first byte in the data indicates the
> current
> > channel. Channels 1-4 are standardized in the Bluetooth Core
> specification
> > while the other channels are vendor specific.
> >
> > Compared to 2nd patch set this patch set has the following changes:
> > ?* UART handling is moved from mfd to bluetooth folder. It now reuses
> the
> > ? existing N_HCI line discipline.
> > ?* mfd creation has been moved from cg2900_core into chip specific
> files.
> > ?* All information for each channel, including API functions, exist
> in each
> > ? MFD devices, making them independent of each other.
> > ?* All chip specific information has been moved from cg2900_core into
> the
> > ? chip specific files. cg2900_core now only handles registration and
> > ? connection between transport and chip driver.
> > ?* Fixes for several review comments including use of existing debug
> system.
>
> this patchset has only multiplied my confusion.
>
> Not really diving into the details, I'm just trying to understand how
> I'd rewrite the TI shared transport using your approach if I had to.
> It looks like I'd have to, at least:
> - implement a heavyweight line discipline driver (like cg2900_uart.c)
> half duplicating the functionality in yours;
> - implement specific MFD driver.
>
> Actually the reuse of this implementation will be at a level of
> statistics error. The only thing to reuse is actually the approach
> which I'm not really happy about.
>
> So...
>
> If you are aiming for a generic solution, then why you get more and
> more things handled in cg2900-specific code (e. g. packet routing
> which is fairly generic)?
>
You are absolutely correct that there is not much that is generic and the reason for this is that it is not much that is generic. If you for example look at packet routing the method can be considered pretty generic. Check first byte and choose a structure type... But first of all this must be done with minimum overhead since it is done on every packet received. And secondly if you look at number of lines of code, it is not really much you save.
If you look closer at the code for the CG2900 you will see that the majority of the code is things that you could probably never apply to the chip of another vendor. A lot of the code in e.g. cg2900_chip.c is regarding bt_audio and fm_audio, which is really chip specific.
Also, if you want to keep the structure modular and supporting e.g. multiple transports with the same chip driver, you must focus on what is chip specific, what is transport specific, and what is chip and transport specific. When you start to think about it like this it becomes very hard to make too much code generic. Usually what you get with that kind of generic code is a solution way more complex than you started out with.
> If you are solving your particular problems, isn't the change too
> dramatic e. g. when it comes to line discipline drivers and baudrate
> manipulation?
>
The changes in hci_ldisc.c is not what I would call dramatic. It is quite simple functions. As I've stated earlier, if people object to this, there is no big issue to solve this within cg2900_uart.c instead. But we think that these API functions are a proper extension to the current hci_ldisc API.
> As of now, I really see no reason why anyone would promote this
> solution instead of making something more generic out of ti-st and
> reusing it then.
>
> Thanks,
> Vitaly
Just to be clear, I have no problem with having both solutions (our and ti-st) side by side. They are structured quite differently and has different focus. I'm in no way trying to replace ti-st. I'm not trying to force anyone into using our structure.
/P-G
2010/12/17 Par-Gunnar Hjalmdahl <[email protected]>:
> This is the 3rd patch set for the ST-Ericsson CG2900 connectivity
> controller. The CG2900 is a combo controller supporting GPS, Bluetooth,
> and FM radio. It uses HCI H:4 protocol to combine different functionalities
> on a common transport, where first byte in the data indicates the current
> channel. Channels 1-4 are standardized in the Bluetooth Core specification
> while the other channels are vendor specific.
I'm slightly confused by different comment threads on this patch set.
I would certainly appreciate if the subsystem maintainers and reviewers
who shaped this patch set could add their Acked-by to the parts
they're happy with.
Alan, Marcel, Sam & Arnd especially. (Your work is MUCH
appreciated.)
I'm trying to get an idea of what patches are OK and what patches
are being disputed here, so as to whether there is some consensus
or not.
Yours,
Linus Walleij
Hi Linus,
On Sun, Dec 19, 2010 at 10:23 PM, Linus Walleij
<[email protected]> wrote:
> I'm slightly confused by different comment threads on this patch set.
let me first first repost the part from Arnd's email on the
architecture of the "shared transport" type of thing:
> I believe we already agree it should be something like
>
> bt ti-radio st-e-radio ti-gps st-e-gps broadcom-radio ...
> | | | | | | |
> +---------+----------+---------+--+----------+----------+---------+
> |
> common-hci-module
> |
> +-----------+-----------+
> | | | |
> serial spi i2c ...
I think an explanation on how the patchset from Par maps to this
architecture could be a good starting point for confusion minimization
:)
Thanks,
Vitaly
Hi Vitaly,
> -----Original Message-----
> From: Vitaly Wool [mailto:[email protected]]
> Sent: den 19 december 2010 23:58
> To: Linus Walleij
> Cc: Par-Gunnar HJALMDAHL; Pavan Savoy; Alan Cox; Arnd Bergmann; Samuel
> Ortiz; Marcel Holtmann; [email protected]; linux-
> [email protected]; Lukasz Rymanowski; Par-Gunnar Hjalmdahl
> Subject: Re: [PATCH 00/11] mfd and bluetooth: Add CG2900 support
>
> Hi Linus,
>
> On Sun, Dec 19, 2010 at 10:23 PM, Linus Walleij
> <[email protected]> wrote:
> > I'm slightly confused by different comment threads on this patch set.
>
> let me first first repost the part from Arnd's email on the
> architecture of the "shared transport" type of thing:
>
> > I believe we already agree it should be something like
> >
> > bt ti-radio st-e-radio ti-gps st-e-gps broadcom-radio
> ...
> > | | | | | | |
> > +---------+----------+---------+--+----------+----------+---------+
> > |
> > common-hci-module
> > |
> > +-----------+-----------+
> > | | | |
> > serial spi i2c ...
>
> I think an explanation on how the patchset from Par maps to this
> architecture could be a good starting point for confusion minimization
> :)
>
> Thanks,
> Vitaly
I would say our design would map like this:
common-hci-module: cg2900_core
serial, spi, i2c,... : cg2900_uart together with hci_ldisc (for other transports it would be different files)
bt, ti-radio, st-e-radio,...: cg2900_chip together with btcg2900 and other users per channel (cg2900_char_devices for users in User space)
So it is not a 1-to-1 mapping for the upper parts. I would draw it like this:
bt st-e-radio st-e-gps
| | |
+---------+----------+
|
ti-xx st-e cg2900...
| |
+---------+----------+
|
common-hci-module
|
+-----------+-----------+
| | | |
serial spi i2c ...
The reason for this difference I've gone through before. Basically there are so many special behaviors and needed handling that is individual for each chip (like startup and shutdown and in the case of CG2900 flow control over FM and BT channels for audio commands). If you then look at the users I guess it would be possible to have one BT user, but it would have to be modified to handle vendor specific commands (as btcg2900 does with BT_ENABLE command). As Arnd has drawn for FM and GPS the users would be completely individual since they don't have a standardized interface.
/P-G
P-G, Vitaly,
>
> I would say our design would map like this:
> common-hci-module: cg2900_core
> serial, spi, i2c,... : cg2900_uart together with hci_ldisc (for other transports it would be different files)
> bt, ti-radio, st-e-radio,...: cg2900_chip together with btcg2900 and other users per channel (cg2900_char_devices for users in User space)
> So it is not a 1-to-1 mapping for the upper parts. I would draw it like this:
>
> bt st-e-radio st-e-gps
> | | |
> +---------+----------+
> |
> ti-xx st-e cg2900...
> | |
> +---------+----------+
> |
> common-hci-module
> |
> +-----------+-----------+
> | | | |
> serial spi i2c ...
regarding the architecture above suggested...
Is having the common-hci-module, only way ?
Why is this dependency on bluetooth at all?
for example: today I don't compile my kernel with BT support, but
still want to use
the chip for FM & GPS, It should be possible correct ?
Even in TI case, although the firmware download is HCI-VS way, we
don't use hci_core
to interpret the responses...
instead of common-hci-module, why not create a algo-driver layer 'ala
i2c ? where individual drivers can register their receive handlers for
data interpretation ?
>
> The reason for this difference I've gone through before. Basically there are so many special behaviors and needed handling that is individual for each chip (like startup and shutdown and in the case of CG2900 flow control over FM and BT channels for audio commands). If you then look at the users I guess it would be possible to have one BT user, but it would have to be modified to handle vendor specific commands (as btcg2900 does with BT_ENABLE command). As Arnd has drawn for FM and GPS the users would be completely individual since they don't have a standardized interface.
>
> /P-G
>