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
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 tra=
nsports it would be different files)
> bt, ti-radio, st-e-radio,...: cg2900_chip together with btcg2900 and othe=
r 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 t=
his:
>
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 bt =C2=A0 st-e-radio =C2=A0st-e-gps
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0| =C2=A0 =C2=A0 =C2=A0 =C2=A0 | =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0|
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0+---------+----------+
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0|
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ti-xx =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0st-e cg2900...
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 | =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0|
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 +--=
-------+----------+
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 |
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 common-hci-module
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 |
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 +---------=
--+-----------+
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 | =C2=A0 =
=C2=A0 =C2=A0 =C2=A0| =C2=A0 =C2=A0 =C2=A0 | =C2=A0 =C2=A0 =C2=A0|
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 serial =C2=A0 =C2=
=A0spi =C2=A0 =C2=A0 i2c =C2=A0 =C2=A0...
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 ea=
ch chip (like startup and shutdown and in the case of CG2900 flow control o=
ver 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 mo=
dified 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 in=
dividual since they don't have a standardized =C2=A0interface.
>
> /P-G
>
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
>=20
> Hi Linus,
>=20
> 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.
>=20
> let me first first repost the part from Arnd's email on the
> architecture of the "shared transport" type of thing:
>=20
> > 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 ...
>=20
> I think an explanation on how the patchset from Par maps to this
> architecture could be a good starting point for confusion minimization
> :)
>=20
> 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 trans=
ports 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 thi=
s:
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 ar=
e 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 ove=
r FM and BT channels for audio commands). If you then look at the users I g=
uess it would be possible to have one BT user, but it would have to be modi=
fied to handle vendor specific commands (as btcg2900 does with BT_ENABLE co=
mmand). As Arnd has drawn for FM and GPS the users would be completely indi=
vidual since they don't have a standardized interface.
/P-G
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
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 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
>=20
> Hi Par,
>=20
> 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:
> > =A0* UART handling is moved from mfd to bluetooth folder. It now reuses
> the
> > =A0 existing N_HCI line discipline.
> > =A0* mfd creation has been moved from cg2900_core into chip specific
> files.
> > =A0* All information for each channel, including API functions, exist
> in each
> > =A0 MFD devices, making them independent of each other.
> > =A0* All chip specific information has been moved from cg2900_core into
> the
> > =A0 chip specific files. cg2900_core now only handles registration and
> > =A0 connection between transport and chip driver.
> > =A0* Fixes for several review comments including use of existing debug
> system.
>=20
> this patchset has only multiplied my confusion.
>=20
> 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.
>=20
> 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.
>=20
> So...
>=20
> 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)?
>=20
You are absolutely correct that there is not much that is generic and the r=
eason for this is that it is not much that is generic. If you for example l=
ook at packet routing the method can be considered pretty generic. Check fi=
rst 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 second=
ly 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 majorit=
y 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_aud=
io and fm_audio, which is really chip specific.
Also, if you want to keep the structure modular and supporting e.g. multipl=
e transports with the same chip driver, you must focus on what is chip spec=
ific, 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?
>=20
The changes in hci_ldisc.c is not what I would call dramatic. It is quite s=
imple 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.
>=20
> 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 f=
ocus. I'm in no way trying to replace ti-st. I'm not trying to force anyone=
into using our structure.
/P-G
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 functionaliti=
es
> on a common transport, where first byte in the data indicates the current
> channel. Channels 1-4 are standardized in the Bluetooth Core specificatio=
n
> while the other channels are vendor specific.
>
> Compared to 2nd patch set this patch set has the following changes:
> =A0* UART handling is moved from mfd to bluetooth folder. It now reuses t=
he
> =A0 existing N_HCI line discipline.
> =A0* mfd creation has been moved from cg2900_core into chip specific file=
s.
> =A0* All information for each channel, including API functions, exist in =
each
> =A0 MFD devices, making them independent of each other.
> =A0* All chip specific information has been moved from cg2900_core into t=
he
> =A0 chip specific files. cg2900_core now only handles registration and
> =A0 connection between transport and chip driver.
> =A0* Fixes for several review comments including use of existing debug sy=
stem.
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
SGksDQoNClNvcnJ5IGZvciBub3QgYW5zd2VyaW5nIGVhcmxpZXIuIEkndmUgYmVlbiBvdmVybG9h
ZGVkIHdpdGggdGhpbmdzIHRvIGRvIG5vdyBhZnRlciBOZXcgWWVhci4gU2VlIGJlbG93Lg0KDQo+
IC0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+IEZyb206IEFybmQgQmVyZ21hbm4gW21haWx0
bzphcm5kQGFybmRiLmRlXQ0KPiBTZW50OiBkZW4gOSBqYW51YXJpIDIwMTEgMTk6NTUNCj4gVG86
IFBhdmFuIFNhdm95DQo+IENjOiBQYXItR3VubmFyIEhKQUxNREFITDsgVml0YWx5IFdvb2w7IExp
bnVzIFdhbGxlaWo7IEFsYW4gQ294OyBTYW11ZWwNCj4gT3J0aXo7IE1hcmNlbCBIb2x0bWFubjsg
bGludXgta2VybmVsQHZnZXIua2VybmVsLm9yZzsgbGludXgtDQo+IGJsdWV0b290aEB2Z2VyLmtl
cm5lbC5vcmc7IEx1a2FzeiBSeW1hbm93c2tpOyBQYXItR3VubmFyIEhqYWxtZGFobA0KPiBTdWJq
ZWN0OiBSZTogW1BBVENIIDAwLzExXSBtZmQgYW5kIGJsdWV0b290aDogQWRkIENHMjkwMCBzdXBw
b3INCj4gDQo+IE9uIFN1bmRheSAwOSBKYW51YXJ5IDIwMTEsIFBhdmFuIFNhdm95IHdyb3RlOg0K
PiA+IE9uIEZyaSwgSmFuIDcsIDIwMTEgYXQgMTI6MTYgQU0sIEFybmQgQmVyZ21hbm4gPGFybmRA
YXJuZGIuZGU+IHdyb3RlOg0KPiA+ID4gT24gV2VkbmVzZGF5IDA1IEphbnVhcnkgMjAxMSwgUGFy
LUd1bm5hciBISkFMTURBSEwgd3JvdGU6DQo+ID4gPg0KPiA+ID4+IFNvcnJ5IGZvciBub3QgYW5z
d2VyaW5nIHNvb25lci4gSSd2ZSBiZWVuIG9uIENocmlzdG1hcyBhbmQgTmV3DQo+IFllYXIgdmFj
YXRpb24uDQo+ID4gPg0KPiA+ID4gSSdtIGFsc28gc3RpbGwgY2F0Y2hpbmcgdXAgd2l0aCBlbWFp
bCB0aGF0IGhhcyBhY2N1bXVsYXRlZCBvdmVyIG15DQo+ID4gPiB2YWNhdGlvbiwgaW5jbHVkaW5n
IHlvdXIgcHJldmlvdXMgcmVzcG9uc2UuDQo+ID4gPg0KPiA+ID4gVGhpcyBzb3VuZHMgd3Jvbmcg
Zm9yIGJvdGggVEkgYW5kIFNULUU6IEFGQUlDVCB0aGV5IGFyZSBhY3R1YWxseQ0KPiBidWlsdA0K
PiA+ID4gYXJvdW5kIGFuIEhDSSBpbnRlcmZhY2UuIEl0J3MgdW5mb3J0dW5hdGUgdGhhdCB0aGUg
VEkgY29kZSBhY3R1YWxseQ0KPiBnb3QNCj4gPiA+IG1lcmdlZCBpbnRvIHRoZSBrZXJuZWwgbGlr
ZSB0aGlzLg0KPiA+DQo+ID4gSSBhbSBub3Qgc3VyZSB3aGF0IGRvZXMgYnVpbHQgYXJvdW5kIEhD
SSBJbnRlcmZhY2UgbWVhbj8gQWxzbyB5ZXMsIGluDQo+IFRJLSBjb2RlDQo+ID4gd2UgZG8gcmVm
ZXIgdG8gQmx1ZXRvb3RoIGhlYWRlcnMuDQo+ID4gSG93ZXZlciB0aGUgZmFjdCB0aGF0IEkgd2Fu
dGVkIHRvIHBvaW50IG91dCB0byBQYXItR3VubmFyIHdhcywgdGhhdA0KPiB3ZQ0KPiA+IGRvbid0
IHdhbnQgdG8gdXNlDQo+ID4gaGNpYXR0YWNoIGFuZCBlbmFibGUgSENJLVVBUlQgKyBIQ0ktSDQg
Zm9yIGVuYWJsaW5nIG91ciBkcml2ZXIgb3Igb3VyDQo+ID4gZHJpdmVyIHNob3VsZCBub3QNCj4g
PiBkZXBlbmQgb24gdGhvc2UgbW9kdWxlcyBhcyBzdWNoLi4uDQo+IA0KPiBHb29kIHBvaW50IGFi
b3V0IGhjaWF0dGFjaCwgeW91IGNlcnRhaW5seSBzaG91bGRuJ3QgbmVlZCB0byB1c2UgdGhhdCBp
Zg0KPiB0aGUga2VybmVsIGFscmVhZHkga25vd3MgdGhhdCBhIHR0eSBpcyBjb25uZWN0ZWQgdG8g
YW4gSENJIGFuZCB3aGF0IHRoZQ0KPiBwYXJhbWV0ZXJzIGFyZS4gRXZlbiBtb3JlIHNvIGlmIHRo
ZSBIQ0kgaXMgbm90IGFjdHVhbGx5IG9uIGEgcnMyMzIgbGluZQ0KPiBidXQgc29tZXRoaW5nIGxp
a2UgaTJjIG9yIHNwaS4gQXV0b21hdGljYWxseSBiaW5kaW5nIHRvIHRoZSByaWdodCBsaW5lDQo+
IGRpc2NpcGxpbmUgc2hvdWxkIGJlIGVhc2lseSBkb2FibGUgaW4gdGhlIGRyaXZlcnMgdGhvdWdo
Lg0KPiANCg0KV2hlbiBoYXZpbmcgVUFSVCBhcyB0cmFuc3BvcnQgeW91IG5lZWQgc29tZXRoaW5n
IHRvIG9wZW4gdGhlIFVBUlQgYW5kIHNldCB0aGUgbGluZSBkaXNjaXBsaW5lLiBJZiB0aGlzIGlz
IGhjaWF0dGFjaCBvciBzb21ldGhpbmcgZWxzZSBpcyB1cCB0byBlYWNoIGRldmVsb3BlciB0byBz
dWl0IHdoYXQgdGhleSBhcmUgZG9pbmcuIEkgZG9uJ3Qgc2VlIGEgcHJvYmxlbSB3aXRoIHVzaW5n
IGhjaWF0dGFjaCBldmVuIGlmIHlvdSBkb24ndCB1c2UgdGhlIEJsdWV0b290aCBwYXJ0IChpZiB0
aGUgZXhlIGlzIHBhcnQgb2YgdGhlIHN5c3RlbSBhbnl3YXkpLCBidXQgaWYgYSBjb21wYW55L2Rl
dmVsb3BlciB3YW50IHRvIHVzZSBzb21ldGhpbmcgZWxzZSB0aGV5IGNhbiBkbyB0aGF0LiBJdCdz
IGEgdXNhZ2Ugb2Ygc3RhbmRhcmQgaW50ZXJmYWNlcyB1c2luZyBvcGVuKCkgYW5kIGlvY3RsKCku
DQpJIHN0aWxsIGRvbid0IHRoaW5rIHRoYXQgeW91IHNob3VsZCBoYXZlIGEgbGluZSBkaXNjaXBs
aW5lIGZvciBvdGhlciB0cmFuc3BvcnRzIHRoYW4gVUFSVC4gSWYgSSB3b3VsZCBsb29rIGF0IGhv
dyBJIHdvdWxkIGltcGxlbWVudCBhbiBTUEkgZHJpdmVyIGZvciBDRzI5MDAsIHRoZXJlIHdvdWxk
IGFsbW9zdCBiZSBubyBjb2RlIHRoYXQgY291bGQgYmUgdXNlZCBpbiBjb21tb24gYmV0d2VlbiBj
ZzI5MDBfdWFydCBhbmQgY2cyOTAwX3NwaS4gU1BJIGRvZXNuJ3QgY2hhbmdlIGJhdWQgcmF0ZSwg
U1BJIHVzZXMgY29tbWFuZHMgZm9yIHNsZWVwL3dha2UgaW5zdGVhZCBvZiBCcmVhaywgU1BJIHBh
Y2tldHMgZG9lc24ndCBuZWVkIGV4dHJhIHBhY2tldGl6aW5nLCBldGMuDQoNCj4gSSBkb24ndCB1
bmRlcnN0YW5kIHRoZSBwcm9ibGVtIHdpdGggcmVseWluZyBvbiB0aGUgaGNpLXVhcnQgb3IgaGNp
LWg0DQo+IG1vZHVsZXMuIElmIHRoZSBoYXJkd2FyZSB1c2VzIHRoZSBINCBwcm90b2NvbCwgd2Ug
Y2VydGFpbmx5IHNob3VsZCB1c2UNCj4gdGhlIGtlcm5lbCBtb2R1bGUgdGhhdCBrbm93cyBob3cg
dG8gZGVhbCB3aXRoIEg0LCBhbmQgd2UgZG9uJ3Qgd2FudA0KPiB0byBoYXZlIHR3byBtb2R1bGVz
IGltcGxlbWVudGluZyB0aGUgc2FtZSBwcm90b2NvbC4NCj4gDQoNCkkgbXVzdCBzYXkgSSBkb24n
dCB1bmRlcnN0YW5kIHRoaXMgcHJvYmxlbSBlaXRoZXIuIFVubGVzcyB0aGUgcHJvdG9jb2wgZHJp
dmVyIGlzIGFjdGl2YXRlZCB0aHJvdWdoIGlvY3RsIFNFVF9QUk9UT0NPTCwgdGhlIGNvZGUgd2ls
bCBub3QgYmUgZXhlY3V0ZWQsIGFuZCB0aGUgYW1vdW50IG9mIFJPTSBuZWVkZWQgaXMgbmVnbGln
aWJsZS4NCklmIHlvdSBsb29rIGF0IG91ciBzdWJtaXNzaW9uLCB0aGUgaGNpLWg0IGNvdWxkIHBv
c3NpYmx5IGJlIHJldXNlZCB0byBzb21lIGV4dGVudC4gQmFzaWNhbGx5IHRoZSBwYWNrZXRpemlu
ZyB0byB0aGUgQmx1ZXRvb3RoIEg0IGNoYW5uZWxzIDEtNCBjb3VsZCBiZSByZS11c2VkLiBJbiBv
cmRlciB0byBhbGxvdyBmb3IgdmVuZG9yIHNwZWNpZmljIGNoYW5uZWxzIHRvIGJlIGhhbmRsZWQg
c29tZSBuZXcgcmVnaXN0cmF0aW9uIHN5c3RlbSBvbiB0b3Agb2YgaGNpLWg0IHBsdXMgYSBjYWxs
YmFjayBzeXN0ZW0gZm9yIGRhdGEgcmVjZXB0aW9uIHdvdWxkIGhhdmUgdG8gYmUgYWRkZWQgKGlu
IG9yZGVyIHRvIGZhY2lsaXRhdGUgc3lzdGVtcyB0aGF0IGRvIG5vdCB3YW50IGFsbCBkYXRhIHRv
IGJlIHNlbnQgZGlyZWN0bHkgdG8gdGhlIEJsdWV0b290aCBzdGFjayBzdWNoIGFzIHRoZSBDRzI5
MDAgZHJpdmVyKS4gSSdtIGFmcmFpZCB0aGF0IHdlIHdvdWxkIGhhdmUgYSBzaWduaWZpY2FudGx5
IG1vcmUgY29tcGxleCBzeXN0ZW0gYW5kIGxhcmdlciBhbW91bnQgb2YgY29kZSBpZiB3ZSB3b3Vs
ZCB0cnkgdG8gZ2VuZXJhbGl6ZSB0aGUgaGNpLWg0IG1vZHVsZS4gSSBkZWZpbml0ZWx5IHByZWZl
ciB0aGUgY3VycmVudCBtb2RlbCB3aGVyZSBhIHZlbmRvciBzcGVjaWZpYyBkcml2ZXIgcmVwbGFj
ZXMgdGhlIGhjaS1oNCBwcm90b2NvbCBkcml2ZXIuDQoNCj4gPiA+PiA+IGluc3RlYWQgb2YgY29t
bW9uLWhjaS1tb2R1bGUsIHdoeSBub3QgY3JlYXRlIGEgYWxnby1kcml2ZXIgbGF5ZXINCj4gJ2Fs
YQ0KPiA+ID4+ID4gaTJjID8gd2hlcmUgaW5kaXZpZHVhbCBkcml2ZXJzIGNhbiByZWdpc3RlciB0
aGVpciByZWNlaXZlDQo+IGhhbmRsZXJzIGZvcg0KPiA+ID4+ID4gZGF0YSBpbnRlcnByZXRhdGlv
biA/DQo+ID4gPg0KPiA+ID4gVGhhdCB3b3VsZCBiZSB3aGF0IEkgc3VnZ2VzdGVkIDstKQ0KPiA+
DQo+ID4gQnV0IGV2ZW4gaGVyZSB0b28sIHRoZSBhbGdvcyBsYXllciBpZiB5b3UgaW1hZ2luZSB3
aGljaCBjYW4gYmUgdGhlDQo+ID4gc29ydCBvZiB0aGUgZmlyc3QNCj4gPiByZWNlaXZlciBvZiBk
YXRhIGZyb20gdGhlIHRyYW5zcG9ydCB3b3VsZCByZWZlciB0byBCVCBoZWFkZXJzIHRvDQo+ID4g
aW50ZXJwcmV0IHRoZSBkYXRhIChub3QganVzdCBCVCwgYnV0IEZNL0dQUykNCj4gPiBhbmQgcGFz
cyBpdCBvbnRvIG90aGVyIHByb3RvY29sL2NsaWVudCBkcml2ZXJzLA0KPiANCj4gUmlnaHQsIHRo
YXQgaXMgdGhlIGVudGlyZSBpZGVhLCBhbmQgSSBkb24ndCBzZWUgYSBwcm9ibGVtIGhlcmUuDQo+
IElmIHlvdSBkbyB0aGlzLCB5b3UgdXNlIHRoZSBoZWFkZXJzIG9mIHRoZSB0d28gc3Vic3lzdGVt
cyB5b3UNCj4gaW50ZXJmYWNlIHdpdGguIFdoYXQgeW91IHNob3VsZCAvbm90LyBpbnN0ZWFkIGlz
IHVzZSBoZWFkZXINCj4gZmlsZXMgb2YgYSBzdWJzeXN0ZW0geW91IGRvbid0IGludGVyZmFjZSB3
aXRoIGFuZCByZWludGVycHJldGUNCj4gdGhlIGRlZmluaXRpb25zIGluIGNyZWF0aXZlIHdheXMs
IHdoaWNoIGlzIHdoYXQgSSB1bmRlcnN0b29kDQo+IHdhcyBiZWluZyBkaXNjdXNzZWQgZWFybGll
ci4NCj4gDQo+ID4gPj4gSW4gc29tZSB3YXkgeW91IHRoZW4gcnVuIGludG8gdGhlIHNhbWUgcHJv
YmxlbSBoYXMgSSBoYWQgaW4NCj4gcHJldmlvdXMgcGF0Y2gNCj4gPiA+PiBzZXRzLiBUaGUgZnVu
Y3Rpb25hbGl0aWVzIHN1cHBvcnRlZCBpcyByZWFsbHkgZGV0ZXJtaW5lZCBieSBlYWNoDQo+IGNo
aXAuDQo+ID4gPj4gWW91IG1pZ2h0IG9yIG1pZ2h0IG5vdCBoYXZlIGZvciBleGFtcGxlIEdQUyBp
biBhIGNlcnRhaW4gY2hpcC4gU28NCj4geW91IGRvIG5vdA0KPiA+ID4+IHdhbnQgYSBjZW50cmFs
IG1vZHVsZSB0byBleHBvc2UgYWxsIHBvc3NpYmxlIGNoYW5uZWxzIHRvIHRoZQ0KPiBzdGFja3Mg
b24gdG9wLg0KPiA+ID4+DQo+ID4gPj4gWW91IG9ubHkgd2FudCB0aGUgYWN0dWFsbHkgc3VwcG9y
dGVkIGZlYXR1cmVzIHRvIGJlIGV4cG9zZWQgdG8NCj4gdXBwZXIgbGF5ZXJzLg0KPiA+ID4+IFRo
ZW4gdGhlIE1GRCBzeXN0ZW0gaXMgcHJldHR5IG5pY2UuIEl0J3MgZWFzeSBhbmQgbW9kdWxhcml6
ZWQgdG8NCj4gZXhwb3NlIHRoZQ0KPiA+ID4+IGRpZmZlcmVudCBjaGFubmVscyBhcyBNRkQgY2Vs
bHMuDQo+ID4gPg0KPiA+ID4gQnV0IGFzIHNvb24gYXMgeW91IGhhdmUgdGhlIGNvbmNlcHQgb2Yg
Y2hhbm5lbHMgd2l0aCBhIGNsZWFybHkNCj4gZGVmaW5lZA0KPiA+ID4gaW50ZXJmYWNlLCB5b3Ug
aGF2ZSBhbG1vc3QgYWJzdHJhY3RlZCBpdCBlbm91Z2ggdG8gZ28gYWxsIHRoZSB3YXkuDQo+ID4N
Cj4gPiBTb21ldGhpbmcgbGlrZSB0aGlzIGlzIHdoYXQgdGhlIHJlY2VudCBSRkMgcG9zdGVkIHRv
DQo+ID4gbGttbC9saW51eC1ibHVldG9vdGgNCj4gPiBodHRwOi8vd3d3LnNwaW5pY3MubmV0L2xp
c3RzL2xpbnV4LWJsdWV0b290aC9tc2cwOTk5MC5odG1sLA0KPiA+IGl0IGtpbmRhIGxvb2tzIGNs
dW1zeQ0KPiA+IGJ1dCB3aGF0IEkgZmVlbCBpcyB3ZSBzaG91bGRuJ3Qgc2h5IGF3YXkgZnJvbSBu
b3QgcmVmZXJlbmNpbmcNCj4gPiBCbHVldG9vdGgsIChvciBtYXkgYmUgdG9tb3Jyb3cgR1BTDQo+
ID4gd2l0aCBOTUVBIGhlYWRlcnMpLi4uLg0KPiANCj4gVGhlIG9uZSBpbXBvcnRhbnQgZ29hbCBo
ZXJlIHNob3VsZCBiZSB0byBhdm9pZCBjb2RlIGR1cGxpY2F0aW9uLg0KPiBVc2luZyB0aGUgaGVh
ZGVyIHRvIGdldCB0aGUgZGF0YSBzdHJ1Y3R1cmVzIGZyb20gc2VwYXJhdGUgY29kZQ0KPiBtZWFu
cyB5b3UgbmVlZCB0byBoYXZlIHNpbWlsYXIgcGFyc2luZyBmdW5jdGlvbnMgaW4gZWFjaCBvZiB0
aGUgbW9kdWxlcw0KPiB1c2luZyBpdC4gTm90IHNoYXJpbmcgdGhlIGhlYWRlciB3b3VsZG4ndCBo
ZWxwLCBiZWNhdXNlIHRoZW4geW91IGVuZA0KPiB1cCBkdXBsaWNhdGluZyBldmVuIG1vcmUuIFRo
ZSByZWFsIHNvbHV0aW9uLCBzcGVha2luZyB2ZXJ5IGJyb2FkbHksDQo+IG11c3QgYmUgdG8gaGF2
ZSBhIGdlbmVyYWwgbW9kdWxlIHRoYXQgZGVhbHMgd2l0aCB3aGF0ZXZlciB0aGUgbW9yZQ0KPiBz
cGVjaWZpYyBtb2R1bGVzIGhhdmUgaW4gY29tbW9uLCBhbmQgaGF2ZSBhIGhlYWRlciBmaWxlIHRo
YXQgZGVmaW5lcw0KPiB0aGUgaW50ZXJmYWNlIHRvIGl0Lg0KPiANCg0KSW4gZ2VuZXJhbCBJIGFn
cmVlIHdpdGggeW91LCBidXQgdGhlcmUgYXJlIHNvbWUgcHJvYmxlbXMgaGVyZS4NClRoZSBtb3N0
IHVzZWQgQlQgSENJIGV2ZW50cyBhcmUgQ29tbWFuZCBTdGF0dXMgYW5kIENvbW1hbmQgQ29tcGxl
dGUuDQpDb21tYW5kIFN0YXR1cyBjb3VsZCBiZSBwYXJzZWQgY29tcGxldGVseSBpbiBhIGdvb2Qg
d2F5IChyZXRyaWV2aW5nIG9wIGNvZGUsIG5iciBvZiBjb21tYW5kcyBhbGxvd2VkLCBhbmQgc3Rh
dHVzIG9mIGNvbW1hbmQgc2VudCkuDQpDb21tYW5kIENvbXBsZXRlIGlzIGhvd2V2ZXIgcXVpdGUg
Y29tcGxleCBzaW5jZSB0aGUgcmV0dXJuZWQgZGF0YSB3aWxsIGRpZmZlciBkZXBlbmRpbmcgb24g
Y29tbWFuZCBzZW50LiBPcCBjb2RlIGFuZCBuYnIgb2YgY29tbWFuZHMgYWxsb3dlZCBjYW4gYmUg
cmV0cmlldmVkIGJ1dCBldmVyeXRoaW5nIGVsc2UgaGF2ZSB0byBiZSBleHRyYWN0ZWQgZGlmZmVy
ZW50bHkgZGVwZW5kaW5nIG9uIGNvbW1hbmQuIFRoaXMgbWVhbnMgdGhhdCB0aGVyZSBpcyBub3Qg
bXVjaCB0aGF0IHdpbGwgYmUgc2F2ZWQgaGVyZS4gQnV0IG1heWJlIHdlIGNvdWxkIGV4dHJhY3Qg
c29tZSBwYXJzaW5nIGludG8gY29tbW9uIGZ1bmN0aW9ucywgYnV0IEkgZG9uJ3QgdGhpbmsgeW91
IHdvdWxkIGdhaW4gdGhhdCBtdWNoLg0KTW9yZW92ZXIsIHRoaXMgd291bGQgbGVhZCB0byBhIG1h
am9yIHJlaW1wbGVtZW50YXRpb24gb2YgdGhlIGhjaV9jb3JlLmMgYW5kIHJlbGF0ZWQgZmlsZXMs
IHNpbmNlIHRoZXkgZG8gbm90IHVzZSBhbnkgZXhwb3J0ZWQgY29tbW9uIGZ1bmN0aW9ucyBhcyBp
dCBpcyB0b2RheS4gSSBkbyBub3Qga25vdyBpZiB0aGV5IChCbHVlWiBjb21tdW5pdHkpIHdvdWxk
IHdhbnQgdGhpcyBhbmQgSSBkbyBub3QgdGhpbmsgdGhhdCB0aGF0IHNob3VsZCBiZSBwYXJ0IG9m
IHRoZSBDRzI5MDAgZHJpdmVyIHRvIGRvLiBJdCBzaG91bGQgaW4gdGhhdCBjYXNlIGJlIGRvbmUg
c2VwYXJhdGVseS4NCg0KPiA+ID4+IEFsc28gbm90ZSB0aGF0IHRoZSBjb21tb24taGNpLW1vZHVs
ZSBpcyBvbmx5IHJlYWxseSB1c2VkIHVudGlsIHRoZQ0KPiBjb25uZWN0ZWQNCj4gPiA+PiBjaGlw
IGhhcyBiZWVuIGRldGVjdGVkLiBUaGUgY2hpcCBoYW5kbGVyIHdpbGwgdGhlbiBzZXQgdGhlDQo+
IGNhbGxiYWNrIGZ1bmN0aW9ucw0KPiA+ID4+IHNvIGFjdHVhbCBkYXRhIHRyYW5zbWlzc2lvbnMg
bmV2ZXIgcGFzcyB0aGUgY29tbW9uLWhjaS1tb2R1bGUuDQo+IFRoZXkgZ28gZGlyZWN0bHkNCj4g
PiA+PiBmcm9tIHRyYW5zcG9ydCB0byBjaGlwIGhhbmRsZXIuIFRoYXQgaXMgbm90IHJlYWxseSBz
aG93biBpbiB0aGUNCj4gcGljdHVyZSBhYm92ZS4NCj4gPiA+PiBKdXN0IGltYWdpbmUgdGhhdCBj
b21tb24taGNpLW1vZHVsZSBpcyByZW1vdmVkIGFmdGVyIGEgY2hpcCBoYXMNCj4gYmVlbiBjb25u
ZWN0ZWQNCj4gPiA+PiBhbmQgYSBjaGlwIGhhbmRsZXIgaGFzIGJlZW4gZGV0ZXJtaW5lZC4NCj4g
PiA+Pg0KPiA+ID4+IEkgaG9wZSBJIGhhdmVuJ3QgbWlzdW5kZXJzdG9vZCB5b3VyIHF1ZXN0aW9u
LiBJIGRvIG5vdCBrbm93IG11Y2gNCj4gYWJvdXQgdGhlIEkyQw0KPiA+ID4+IHN5c3RlbSwgYnV0
IEkgdHJpZWQgdG8gdW5kZXJzdGFuZCB5b3VyIHF1ZXN0aW9uIGFzIGJlc3QgYXMgSQ0KPiBjb3Vs
ZC4NCj4gPiA+DQo+ID4gPiBJIHRoaW5rIHRoZXJlIGlzIGEgZGlzY29ubmVjdCB3aGVuIHRhbGtp
bmcgYWJvdXQgaGllcmFyY2hpZXMsIGFzIGl0DQo+IGNhbiBiZSBhcHBsaWVkDQo+ID4gPiB0byBk
aWZmZXJlbnQgYXJlYXM6DQo+ID4gPg0KPiA+ID4gKiBtb2R1bGUgZGVwZW5kZW5jaWVzDQo+ID4g
PiAqIGRldmljZSBkZXRlY3Rpb24NCj4gPiA+ICogc3lzZnMgb2JqZWN0IGhpZXJhcmNoeQ0KPiA+
ID4gKiBkYXRhIGZsb3cNCj4gPg0KPiA+IG1vZHVsZSBkZXBlbmRlY3ktd2lzZSBJIGFncmVlLA0K
PiA+IEkgd291bGQgd2FudCBGTS1WNEwyIHdpdGhvdXQgQlQgb3IgSSBtaWdodCB3YW50IEdQUyBz
aW1wbGlzdGljIGNoYXINCj4gPiBkcml2ZXIgd2l0aG91dCBCVCBvciBGTSBWNEwyLg0KPiANCj4g
QnV0IHlvdSdkIHN0aWxsIG5lZWQgdG8gaGF2ZSBhbiBIQ0kgbW9kdWxlLiBJIGJlbGlldmUgcmln
aHQgbm93LCB0aGUNCj4gaGNpIG1vZHVsZSBkZXBlbmRzIG9uIHRoZSBibHVldG9vdGggbW9kdWxl
LCBhbmQgeW91IGFyZSByaWdodCB0aGF0IHRoaXMNCj4gaXMgYW4gdW5kZXNpcmFibGUgZGVwZW5k
ZW5jeSB0byBoYXZlIGZvciBhIEdQUyBtb2R1bGUuIEhvd2V2ZXIsIHRoZQ0KPiBzb2x1dGlvbg0K
PiB0byB0aGlzIHNob3VsZCBub3QgYmUgdG8gbWFrZSBHUFMgaW5kZXBlbmRlbnQgb2YgSENJLCBi
dXQgdG8gbWFrZSBwYXJ0cw0KPiBvZiBIQ0kgaW5kZXBlbmRlbnQgb2YgYmx1ZXRvb3RoLg0KPiAN
Cg0KRm9yIHRoZSBDRzI5MDAgdGhhdCBpcyBub3QgcG9zc2libGUuIEV2ZW4gaWYgeW91IGFyZSBy
dW5uaW5nIGp1c3QgR1BTIHlvdSBzdGlsbCBtdXN0IGRvd25sb2FkIHBhdGNoZXMgYW5kIHNldHRp
bmdzIGFuZCB0aGF0IGlzIGRvbmUgdGhyb3VnaCBIQ0kgY29tbWFuZHMgb3ZlciB0aGUgQmx1ZXRv
b3RoIGNvbW1hbmQgaW50ZXJmYWNlLiBXZSBhbHNvIHVzZSBCbHVldG9vdGggY29tbWFuZHMgdG8g
aWRlbnRpZnkgdGhlIGNvbnRyb2xsZXIuDQoNCj4gPiBkZXZpY2UgZGV0ZWN0aW9uIHdpc2UsIEl0
IGlzIGEgcHJvYmxlbSwgdGhlcmUgaXMgbm90ICJfcHJvYmUiDQo+ID4gbWVjaGFuaXNtIGZvciBV
QVJUIGJhc2VkIHRyYW5zcG9ydCBhcyBpdCBpcw0KPiA+IHVuZGVyc3RhbmRhYmxlLCBhbmQgcHJl
dHR5IG11Y2ggdGhlIGRyaXZlciB3b3VsZCBlbmQgdXAgYmVpbmcNCj4gcGxhdGZvcm0NCj4gPiBk
ZXZpY2UgZHJpdmVyIC4uLg0KPiANCj4gVGhlIHBsYXRmb3JtIGRldmljZSBpcyBqdXN0IHRoZSBs
b3dlc3QgbGV2ZWwgaW4gdGhlIGRldmljZSBoaWVyYXJjaHkuDQo+IA0KPiBJZiBlYWNoIEhDSSBj
aGFubmVsIGlzIGEgZGV2aWNlLCB5b3UgY2FuIHN0YWNrIGJ0LCBncHMsIGF1ZGlvLCAuLi4NCj4g
ZGV2aWNlcyBhbGwgb24gdG9wIG9mIHRoZSBIQ0kgZGV2aWNlLCB3aGljaCBpcyBlaXRoZXIgc3Rh
Y2tlZCBvbiB0b3ANCj4gb2YgYSBzZXJpYWwgcG9ydCBvciBpbiBzb21lIG90aGVyIHdheSBjb25u
ZWN0ZWQgdG8gdGhlIHVuZGVyeWluZw0KPiB0cmFuc3BvcnQuDQo+IA0KPiBJbiB0aGlzIGNhc2Us
IHRoZSBjaGFubmVscyB0aGVtc2VsdmVzIGFyZSBub3QgcGxhdGZvcm0gZGV2aWNlcywgYnV0DQo+
IHdvdWxkIGdldCBhIG5ldyBidXMgZm9yIHRoZW0uIFRoYXQgYnVzIGlzIHBvcHVsYXRlZCBieSBh
IGRyaXZlciB0aGF0DQo+IG93bnMgdGhlIEhDSSBhbmQgdGhhdCBrbm93cyAoZS5nLiBmcm9tIGl0
cyBwbGF0Zm9ybSBkYXRhLCBoYXJkd2FyZQ0KPiByZWdpc3RlcnMsIHVzZXIgY29uZmlnIG9yIGRl
dmljZSB0cmVlKSB3aGF0IGNoYW5uZWxzIGFyZSBwcmVzZW50Lg0KPiANCj4gPiBkYXRhIGZsb3cg
aXMgd2hlcmUgSSBndWVzcyB0aGUgYWJzdHJhY3Rpb24gaGFzIHRvIGxpZSBpbiwgZm9yDQo+ID4g
ZGlmZmVyZW50IHZlbmRvcnMuLi4NCj4gDQo+IEkgZG9uJ3Qga25vdyB3aGF0IHlvdSBtZWFuIHdp
dGggdGhhdC4gTXkgcG9pbnQgd2FzIHRoYXQgd2UgbmVlZCB0bw0KPiBjb25zaWRlciBhbGwgdGhl
IGhpZXJhcmNoaWVzLCBhbmQgdGhhdCB0aGV5IG1pZ2h0IGJlIGRpZmZlcmVudC4NCj4gDQo+IAlB
cm5kDQoNCi9QLUcNCg0K
On Sunday 09 January 2011, Pavan Savoy wrote:
> On Fri, Jan 7, 2011 at 12:16 AM, Arnd Bergmann <[email protected]> wrote:
> > On Wednesday 05 January 2011, Par-Gunnar HJALMDAHL wrote:
> >
> >> Sorry for not answering sooner. I've been on Christmas and New Year vacation.
> >
> > I'm also still catching up with email that has accumulated over my
> > vacation, including your previous response.
> >
> > This sounds wrong for both TI and ST-E: AFAICT they are actually built
> > around an HCI interface. It's unfortunate that the TI code actually got
> > merged into the kernel like this.
>
> I am not sure what does built around HCI Interface mean? Also yes, in TI- code
> we do refer to Bluetooth headers.
> However the fact that I wanted to point out to Par-Gunnar was, that we
> don't want to use
> hciattach and enable HCI-UART + HCI-H4 for enabling our driver or our
> driver should not
> depend on those modules as such...
Good point about hciattach, you certainly shouldn't need to use that if
the kernel already knows that a tty is connected to an HCI and what the
parameters are. Even more so if the HCI is not actually on a rs232 line
but something like i2c or spi. Automatically binding to the right line
discipline should be easily doable in the drivers though.
I don't understand the problem with relying on the hci-uart or hci-h4
modules. If the hardware uses the H4 protocol, we certainly should use
the kernel module that knows how to deal with H4, and we don't want
to have two modules implementing the same protocol.
> >> > 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 ?
> >
> > That would be what I suggested ;-)
>
> But even here too, the algos layer if you imagine which can be the
> sort of the first
> receiver of data from the transport would refer to BT headers to
> interpret the data (not just BT, but FM/GPS)
> and pass it onto other protocol/client drivers,
Right, that is the entire idea, and I don't see a problem here.
If you do this, you use the headers of the two subsystems you
interface with. What you should /not/ instead is use header
files of a subsystem you don't interface with and reinterprete
the definitions in creative ways, which is what I understood
was being discussed earlier.
> >> In some way you then run into the same problem has I had in previous patch
> >> sets. The functionalities supported is really determined by each chip.
> >> You might or might not have for example GPS in a certain chip. So you do not
> >> want a central module to expose all possible channels to the stacks on top.
> >>
> >> You only want the actually supported features to be exposed to upper layers.
> >> Then the MFD system is pretty nice. It's easy and modularized to expose the
> >> different channels as MFD cells.
> >
> > But as soon as you have the concept of channels with a clearly defined
> > interface, you have almost abstracted it enough to go all the way.
>
> Something like this is what the recent RFC posted to
> lkml/linux-bluetooth
> http://www.spinics.net/lists/linux-bluetooth/msg09990.html,
> it kinda looks clumsy
> but what I feel is we shouldn't shy away from not referencing
> Bluetooth, (or may be tomorrow GPS
> with NMEA headers)....
The one important goal here should be to avoid code duplication.
Using the header to get the data structures from separate code
means you need to have similar parsing functions in each of the modules
using it. Not sharing the header wouldn't help, because then you end
up duplicating even more. The real solution, speaking very broadly,
must be to have a general module that deals with whatever the more
specific modules have in common, and have a header file that defines
the interface to it.
> >> Also note that the common-hci-module is only really used until the connected
> >> chip has been detected. The chip handler will then set the callback functions
> >> so actual data transmissions never pass the common-hci-module. They go directly
> >> from transport to chip handler. That is not really shown in the picture above.
> >> Just imagine that common-hci-module is removed after a chip has been connected
> >> and a chip handler has been determined.
> >>
> >> I hope I haven't misunderstood your question. I do not know much about the I2C
> >> system, but I tried to understand your question as best as I could.
> >
> > I think there is a disconnect when talking about hierarchies, as it can be applied
> > to different areas:
> >
> > * module dependencies
> > * device detection
> > * sysfs object hierarchy
> > * data flow
>
> module dependecy-wise I agree,
> I would want FM-V4L2 without BT or I might want GPS simplistic char
> driver without BT or FM V4L2.
But you'd still need to have an HCI module. I believe right now, the
hci module depends on the bluetooth module, and you are right that this
is an undesirable dependency to have for a GPS module. However, the solution
to this should not be to make GPS independent of HCI, but to make parts
of HCI independent of bluetooth.
> device detection wise, It is a problem, there is not "_probe"
> mechanism for UART based transport as it is
> understandable, and pretty much the driver would end up being platform
> device driver ...
The platform device is just the lowest level in the device hierarchy.
If each HCI channel is a device, you can stack bt, gps, audio, ...
devices all on top of the HCI device, which is either stacked on top
of a serial port or in some other way connected to the underying transport.
In this case, the channels themselves are not platform devices, but
would get a new bus for them. That bus is populated by a driver that
owns the HCI and that knows (e.g. from its platform data, hardware
registers, user config or device tree) what channels are present.
> data flow is where I guess the abstraction has to lie in, for
> different vendors...
I don't know what you mean with that. My point was that we need to
consider all the hierarchies, and that they might be different.
Arnd
On Sun, Jan 9, 2011 at 7:12 PM, Pavan Savoy <[email protected]> wrote:
>> This sounds wrong for both TI and ST-E: AFAICT they are actually built
>> around an HCI interface. It's unfortunate that the TI code actually got
>> merged into the kernel like this.
>
> I am not sure what does built around HCI Interface mean? Also yes, in TI- code
> we do refer to Bluetooth headers.
I think that the first and the major problem with TI code is that it
is _very_ dependent on the HCI-LL protocol.
I urge you again to sort that out first and then the whole thing will
get quite a bit clearer to all the parties.
> However the fact that I wanted to point out to Par-Gunnar was, that we
> don't want to use
> hciattach and enable HCI-UART + HCI-H4 for enabling our driver or our
> driver should not
> depend on those modules as such...
I really don't care _that_ much if we have to enable HCI interface to
bring say the FM radio up. What I do care about is the startup time
for the same FM radio, or GPS, which turns to be far longer if we go
the awkward way of launching hciattach and friends which are not in
fact needed at all. So I tend to agree with you here.
>
> The references to bluetooth headers in a certain way is inevitable
> because as he pointed
> out, firmware is downloaded as HCI-VS commands, too bad the firmware
> doesn't have any other
> means :(, But it sorts of allows violations, as in we can afford to
> have HCI-VS commands sent after
> disabling events, which would mean they need not be interpreted at all..
Well if we've got this common-hci-module Arnd was talking about, I
believe that firmware download can be sort of abstracted.
>>> > 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 ?
>>
>> That would be what I suggested ;-)
>
> But even here too, the algos layer if you imagine which can be the
> sort of the first
> receiver of data from the transport would refer to BT headers to
> interpret the data (not just BT, but FM/GPS)
> and pass it onto other protocol/client drivers,
> The only abstraction being that different adapter drivers can register
> their own receive function
> which the algo-driver can sort of call, (again all imagination....)
>
Let's keep things simple. Could you please come up with a step-by-step
on how you would transform the existing TI solution for shared
transport into something really generic?
Or, the other option would IMHO be to start with sorting out some
obvious shared transport flaws.
Thanks,
Vitaly
On Fri, Jan 7, 2011 at 12:16 AM, Arnd Bergmann <[email protected]> wrote:
> On Wednesday 05 January 2011, Par-Gunnar HJALMDAHL wrote:
>
>> Sorry for not answering sooner. I've been on Christmas and New Year vaca=
tion.
>
> I'm also still catching up with email that has accumulated over my
> vacation, including your previous response.
>
> This sounds wrong for both TI and ST-E: AFAICT they are actually built
> around an HCI interface. It's unfortunate that the TI code actually got
> merged into the kernel like this.
I am not sure what does built around HCI Interface mean? Also yes, in TI- c=
ode
we do refer to Bluetooth headers.
However the fact that I wanted to point out to Par-Gunnar was, that we
don't want to use
hciattach and enable HCI-UART + HCI-H4 for enabling our driver or our
driver should not
depend on those modules as such...
The references to bluetooth headers in a certain way is inevitable
because as he pointed
out, firmware is downloaded as HCI-VS commands, too bad the firmware
doesn't have any other
means :(, But it sorts of allows violations, as in we can afford to
have HCI-VS commands sent after
disabling events, which would mean they need not be interpreted at all..
>> > 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 ?
>
> That would be what I suggested ;-)
But even here too, the algos layer if you imagine which can be the
sort of the first
receiver of data from the transport would refer to BT headers to
interpret the data (not just BT, but FM/GPS)
and pass it onto other protocol/client drivers,
The only abstraction being that different adapter drivers can register
their own receive function
which the algo-driver can sort of call, (again all imagination....)
>> In some way you then run into the same problem has I had in previous pat=
ch
>> sets. The functionalities supported is really determined by each chip.
>> You might or might not have for example GPS in a certain chip. So you do=
not
>> want a central module to expose all possible channels to the stacks on t=
op.
>>
>> You only want the actually supported features to be exposed to upper lay=
ers.
>> Then the MFD system is pretty nice. It's easy and modularized to expose =
the
>> different channels as MFD cells.
>
> But as soon as you have the concept of channels with a clearly defined
> interface, you have almost abstracted it enough to go all the way.
Something like this is what the recent RFC posted to
lkml/linux-bluetooth
http://www.spinics.net/lists/linux-bluetooth/msg09990.html,
it kinda looks clumsy
but what I feel is we shouldn't shy away from not referencing
Bluetooth, (or may be tomorrow GPS
with NMEA headers)....
>> Also note that the common-hci-module is only really used until the conne=
cted
>> chip has been detected. The chip handler will then set the callback func=
tions
>> so actual data transmissions never pass the common-hci-module. They go d=
irectly
>> from transport to chip handler. That is not really shown in the picture =
above.
>> Just imagine that common-hci-module is removed after a chip has been con=
nected
>> and a chip handler has been determined.
>>
>> I hope I haven't misunderstood your question. I do not know much about t=
he I2C
>> system, but I tried to understand your question as best as I could.
>
> I think there is a disconnect when talking about hierarchies, as it can b=
e applied
> to different areas:
>
> * module dependencies
> * device detection
> * sysfs object hierarchy
> * data flow
module dependecy-wise I agree,
I would want FM-V4L2 without BT or I might want GPS simplistic char
driver without BT or FM V4L2.
device detection wise, It is a problem, there is not "_probe"
mechanism for UART based transport as it is
understandable, and pretty much the driver would end up being platform
device driver ...
data flow is where I guess the abstraction has to lie in, for
different vendors...
> These are often the same or at least related, but we may be talking about=
different
> aspects here. One issue that is often confusing is that from a module lay=
ering,
> you typically have a common module at the bottom and have both providers =
(e.g. hosts
> controller) and consumers of data (e.g. protocol drivers) on the top, whe=
re in a
> data flow chart, you typically have the provider below the common code an=
d the
> consumer above if (or the other way round, if you prefer).
>
> Since the HCI code in this case is the common component, it really needs =
to be the
> module that everything else registers with in some way. You can have mult=
iple
> modules providing HCIs to that, and I suppose a module that provides an H=
CI should
> also be the one that identifies the channels that are present.
>
> The consumers of those channels however should not interface with the mod=
ule that
> provides the channels, but use the HCI code or something on top of it as =
an
> abstraction.
>
> =C2=A0 =C2=A0 =C2=A0 =C2=A0Arnd
Thanks for the comments...
>
On Wednesday 05 January 2011, Par-Gunnar HJALMDAHL wrote:
> Sorry for not answering sooner. I've been on Christmas and New Year vacation.
I'm also still catching up with email that has accumulated over my
vacation, including your previous response.
> > -----Original Message-----
> > >
> > > 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 ...
Even if the cg2900 based drivers (bt, st-e-radio, ...) have things in common
that are not true in general, I'd still advocate a model where they all
register directly to the hardware-independent layer. Your base cg2900
driver can then become a library module that is used by some drivers that
use the cg2900 specific functionality, just like there can be other similar
libraries.
> > regarding the architecture above suggested...
> > Is having the common-hci-module, only way ?
> > Why is this dependency on bluetooth at all?
> >
>
> The reason I use Bluetooth is that it is the only standardized Host-Controller
> interface in these controllers (at least in the CG2900 which I'm primarily addressing).
That doesn't seem to make any sense if you don't present the standard interface
to Linux but basically still need to come up with your own abstraction layer
for it. It may have been the intention to use a standard HC interface, but either
the HW guys screwed up completely by making it impossible to use it that way,
or you haven't found the right way to integrate with the standard code.
> > 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 ?
>
> Yes, I use the header files from the Bluetooth files inside the driver (for
> packet structures and protocol IDs). I do not use the BT binaries at all in
> the common-hci-module or in the st-e cg2900 module. So it should be OK to
> configure this out. The header files will be there any way.
This proves the point I just made: If it's a standard hardware interface, you
should be able to use the same driver module for a regular bluetooth HCI and
for a cf2900 without bluetooth. There is nothing wrong with requiring kernel
support for HCI if what you have is actually an HCI. Contrary to what Pavan
said, I would not require these to be independent implementations. Of course
in this case you would only require the base hci module to do this, not any of
the upper-level BT modules.
> > Even in TI case, although the firmware download is HCI-VS way, we
> > don't use hci_core
> > to interpret the responses...
> >
>
> We don't use that either. We just use the structs and defines in the BT
> headers. The dependency is only in btcg2900.c which is only included if
> BT is configuration enabled.
This sounds wrong for both TI and ST-E: AFAICT they are actually built
around an HCI interface. It's unfortunate that the TI code actually got
merged into the kernel like this.
> > 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 ?
That would be what I suggested ;-)
> In some way you then run into the same problem has I had in previous patch
> sets. The functionalities supported is really determined by each chip.
> You might or might not have for example GPS in a certain chip. So you do not
> want a central module to expose all possible channels to the stacks on top.
>
> You only want the actually supported features to be exposed to upper layers.
> Then the MFD system is pretty nice. It's easy and modularized to expose the
> different channels as MFD cells.
But as soon as you have the concept of channels with a clearly defined
interface, you have almost abstracted it enough to go all the way.
> Also note that the common-hci-module is only really used until the connected
> chip has been detected. The chip handler will then set the callback functions
> so actual data transmissions never pass the common-hci-module. They go directly
> from transport to chip handler. That is not really shown in the picture above.
> Just imagine that common-hci-module is removed after a chip has been connected
> and a chip handler has been determined.
>
> I hope I haven't misunderstood your question. I do not know much about the I2C
> system, but I tried to understand your question as best as I could.
I think there is a disconnect when talking about hierarchies, as it can be applied
to different areas:
* module dependencies
* device detection
* sysfs object hierarchy
* data flow
These are often the same or at least related, but we may be talking about different
aspects here. One issue that is often confusing is that from a module layering,
you typically have a common module at the bottom and have both providers (e.g. hosts
controller) and consumers of data (e.g. protocol drivers) on the top, where in a
data flow chart, you typically have the provider below the common code and the
consumer above if (or the other way round, if you prefer).
Since the HCI code in this case is the common component, it really needs to be the
module that everything else registers with in some way. You can have multiple
modules providing HCIs to that, and I suppose a module that provides an HCI should
also be the one that identifies the channels that are present.
The consumers of those channels however should not interface with the module that
provides the channels, but use the HCI code or something on top of it as an
abstraction.
Arnd
SGkgUGF2YW4sDQoNClNvcnJ5IGZvciBub3QgYW5zd2VyaW5nIHNvb25lci4gSSd2ZSBiZWVuIG9u
IENocmlzdG1hcyBhbmQgTmV3IFllYXIgdmFjYXRpb24uDQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNz
YWdlLS0tLS0NCj4gRnJvbTogUGF2YW4gU2F2b3kgW21haWx0bzpwYXZhbl9zYXZveUBzaWZ5LmNv
bV0NCj4gU2VudDogZGVuIDIzIGRlY2VtYmVyIDIwMTAgMTE6NDkNCj4gVG86IFBhci1HdW5uYXIg
SEpBTE1EQUhMDQo+IENjOiBWaXRhbHkgV29vbDsgTGludXMgV2FsbGVpajsgQWxhbiBDb3g7IEFy
bmQgQmVyZ21hbm47IFNhbXVlbCBPcnRpejsNCj4gTWFyY2VsIEhvbHRtYW5uOyBsaW51eC1rZXJu
ZWxAdmdlci5rZXJuZWwub3JnOyBsaW51eC0NCj4gYmx1ZXRvb3RoQHZnZXIua2VybmVsLm9yZzsg
THVrYXN6IFJ5bWFub3dza2k7IFBhci1HdW5uYXIgSGphbG1kYWhsDQo+IFN1YmplY3Q6IFJlOiBb
UEFUQ0ggMDAvMTFdIG1mZCBhbmQgYmx1ZXRvb3RoOiBBZGQgQ0cyOTAwIHN1cHBvcnQNCj4gDQo+
IFAtRywgVml0YWx5LA0KPiANCj4gPg0KPiA+IEkgd291bGQgc2F5IG91ciBkZXNpZ24gd291bGQg
bWFwIGxpa2UgdGhpczoNCj4gPiBjb21tb24taGNpLW1vZHVsZTogY2cyOTAwX2NvcmUNCj4gPiBz
ZXJpYWwsIHNwaSwgaTJjLC4uLiA6IGNnMjkwMF91YXJ0IHRvZ2V0aGVyIHdpdGggaGNpX2xkaXNj
IChmb3Igb3RoZXINCj4gdHJhbnNwb3J0cyBpdCB3b3VsZCBiZSBkaWZmZXJlbnQgZmlsZXMpDQo+
ID4gYnQsIHRpLXJhZGlvLCBzdC1lLXJhZGlvLC4uLjogY2cyOTAwX2NoaXAgdG9nZXRoZXIgd2l0
aCBidGNnMjkwMCBhbmQNCj4gb3RoZXIgdXNlcnMgcGVyIGNoYW5uZWwgKGNnMjkwMF9jaGFyX2Rl
dmljZXMgZm9yIHVzZXJzIGluIFVzZXIgc3BhY2UpDQo+ID4gU28gaXQgaXMgbm90IGEgMS10by0x
IG1hcHBpbmcgZm9yIHRoZSB1cHBlciBwYXJ0cy4gSSB3b3VsZCBkcmF3IGl0DQo+IGxpa2UgdGhp
czoNCj4gPg0KPiA+IMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKg
IGJ0IMKgIHN0LWUtcmFkaW8gwqBzdC1lLWdwcw0KPiA+IMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKg
IMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgfCDCoCDCoCDCoCDCoCB8IMKgIMKgIMKgIMKgIMKgfA0K
PiA+IMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgKy0tLS0t
LS0tLSstLS0tLS0tLS0tKw0KPiA+IMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKg
IMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgfA0KPiA+IMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKg
IMKgIHRpLXh4IMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgc3QtZSBjZzI5MDAuLi4NCj4gPiDCoCDC
oCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCB8IMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKg
fA0KPiA+IMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgICstLS0tLS0tLS0rLS0tLS0tLS0t
LSsNCj4gPiDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCB8DQo+
ID4gwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgY29tbW9uLWhjaS1tb2R1bGUNCj4g
PiDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCB8DQo+ID4gwqAg
wqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgKy0tLS0tLS0tLS0tKy0tLS0tLS0tLS0tKw0KPiA+IMKg
IMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIHwgwqAgwqAgwqAgwqB8IMKgIMKgIMKgIHwgwqAgwqAg
wqB8DQo+ID4gwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgc2VyaWFsIMKgIMKgc3BpIMKgIMKgIGky
YyDCoCDCoC4uLg0KPiANCj4gcmVnYXJkaW5nIHRoZSBhcmNoaXRlY3R1cmUgYWJvdmUgc3VnZ2Vz
dGVkLi4uDQo+IElzIGhhdmluZyB0aGUgY29tbW9uLWhjaS1tb2R1bGUsIG9ubHkgd2F5ID8NCj4g
V2h5IGlzIHRoaXMgZGVwZW5kZW5jeSBvbiBibHVldG9vdGggYXQgYWxsPw0KPiANCg0KVGhlIHJl
YXNvbiBJIHVzZSBCbHVldG9vdGggaXMgdGhhdCBpdCBpcyB0aGUgb25seSBzdGFuZGFyZGl6ZWQg
SG9zdC1Db250cm9sbGVyIGludGVyZmFjZSBpbiB0aGVzZSBjb250cm9sbGVycyAoYXQgbGVhc3Qg
aW4gdGhlIENHMjkwMCB3aGljaCBJJ20gcHJpbWFyaWx5IGFkZHJlc3NpbmcpLg0KDQo+IGZvciBl
eGFtcGxlOiB0b2RheSBJIGRvbid0IGNvbXBpbGUgbXkga2VybmVsIHdpdGggQlQgc3VwcG9ydCwg
YnV0DQo+IHN0aWxsIHdhbnQgdG8gdXNlDQo+IHRoZSBjaGlwIGZvciBGTSAmIEdQUywgSXQgc2hv
dWxkIGJlIHBvc3NpYmxlIGNvcnJlY3QgPw0KDQpZZXMsIEkgdXNlIHRoZSBoZWFkZXIgZmlsZXMg
ZnJvbSB0aGUgQmx1ZXRvb3RoIGZpbGVzIGluc2lkZSB0aGUgZHJpdmVyIChmb3IgcGFja2V0IHN0
cnVjdHVyZXMgYW5kIHByb3RvY29sIElEcykuIEkgZG8gbm90IHVzZSB0aGUgQlQgYmluYXJpZXMg
YXQgYWxsIGluIHRoZSBjb21tb24taGNpLW1vZHVsZSBvciBpbiB0aGUgc3QtZSBjZzI5MDAgbW9k
dWxlLiBTbyBpdCBzaG91bGQgYmUgT0sgdG8gY29uZmlndXJlIHRoaXMgb3V0LiBUaGUgaGVhZGVy
IGZpbGVzIHdpbGwgYmUgdGhlcmUgYW55IHdheS4NCg0KPiBFdmVuIGluIFRJIGNhc2UsIGFsdGhv
dWdoIHRoZSBmaXJtd2FyZSBkb3dubG9hZCBpcyBIQ0ktVlMgd2F5LCB3ZQ0KPiBkb24ndCB1c2Ug
aGNpX2NvcmUNCj4gdG8gaW50ZXJwcmV0IHRoZSByZXNwb25zZXMuLi4NCj4gDQoNCldlIGRvbid0
IHVzZSB0aGF0IGVpdGhlci4gV2UganVzdCB1c2UgdGhlIHN0cnVjdHMgYW5kIGRlZmluZXMgaW4g
dGhlIEJUIGhlYWRlcnMuIFRoZSBkZXBlbmRlbmN5IGlzIG9ubHkgaW4gYnRjZzI5MDAuYyB3aGlj
aCBpcyBvbmx5IGluY2x1ZGVkIGlmIEJUIGlzIGNvbmZpZ3VyYXRpb24gZW5hYmxlZC4NCg0KPiBp
bnN0ZWFkIG9mIGNvbW1vbi1oY2ktbW9kdWxlLCB3aHkgbm90IGNyZWF0ZSBhIGFsZ28tZHJpdmVy
IGxheWVyICdhbGENCj4gaTJjID8gd2hlcmUgaW5kaXZpZHVhbCBkcml2ZXJzIGNhbiByZWdpc3Rl
ciB0aGVpciByZWNlaXZlIGhhbmRsZXJzIGZvcg0KPiBkYXRhIGludGVycHJldGF0aW9uID8NCj4g
DQoNCkluIHNvbWUgd2F5IHlvdSB0aGVuIHJ1biBpbnRvIHRoZSBzYW1lIHByb2JsZW0gaGFzIEkg
aGFkIGluIHByZXZpb3VzIHBhdGNoIHNldHMuIFRoZSBmdW5jdGlvbmFsaXRpZXMgc3VwcG9ydGVk
IGlzIHJlYWxseSBkZXRlcm1pbmVkIGJ5IGVhY2ggY2hpcC4gWW91IG1pZ2h0IG9yIG1pZ2h0IG5v
dCBoYXZlIGZvciBleGFtcGxlIEdQUyBpbiBhIGNlcnRhaW4gY2hpcC4gU28geW91IGRvIG5vdCB3
YW50IGEgY2VudHJhbCBtb2R1bGUgdG8gZXhwb3NlIGFsbCBwb3NzaWJsZSBjaGFubmVscyB0byB0
aGUgc3RhY2tzIG9uIHRvcC4gWW91IG9ubHkgd2FudCB0aGUgYWN0dWFsbHkgc3VwcG9ydGVkIGZl
YXR1cmVzIHRvIGJlIGV4cG9zZWQgdG8gdXBwZXIgbGF5ZXJzLiBUaGVuIHRoZSBNRkQgc3lzdGVt
IGlzIHByZXR0eSBuaWNlLiBJdCdzIGVhc3kgYW5kIG1vZHVsYXJpemVkIHRvIGV4cG9zZSB0aGUg
ZGlmZmVyZW50IGNoYW5uZWxzIGFzIE1GRCBjZWxscy4NCg0KQWxzbyBub3RlIHRoYXQgdGhlIGNv
bW1vbi1oY2ktbW9kdWxlIGlzIG9ubHkgcmVhbGx5IHVzZWQgdW50aWwgdGhlIGNvbm5lY3RlZCBj
aGlwIGhhcyBiZWVuIGRldGVjdGVkLiBUaGUgY2hpcCBoYW5kbGVyIHdpbGwgdGhlbiBzZXQgdGhl
IGNhbGxiYWNrIGZ1bmN0aW9ucyBzbyBhY3R1YWwgZGF0YSB0cmFuc21pc3Npb25zIG5ldmVyIHBh
c3MgdGhlIGNvbW1vbi1oY2ktbW9kdWxlLiBUaGV5IGdvIGRpcmVjdGx5IGZyb20gdHJhbnNwb3J0
IHRvIGNoaXAgaGFuZGxlci4gVGhhdCBpcyBub3QgcmVhbGx5IHNob3duIGluIHRoZSBwaWN0dXJl
IGFib3ZlLiBKdXN0IGltYWdpbmUgdGhhdCBjb21tb24taGNpLW1vZHVsZSBpcyByZW1vdmVkIGFm
dGVyIGEgY2hpcCBoYXMgYmVlbiBjb25uZWN0ZWQgYW5kIGEgY2hpcCBoYW5kbGVyIGhhcyBiZWVu
IGRldGVybWluZWQuDQoNCkkgaG9wZSBJIGhhdmVuJ3QgbWlzdW5kZXJzdG9vZCB5b3VyIHF1ZXN0
aW9uLiBJIGRvIG5vdCBrbm93IG11Y2ggYWJvdXQgdGhlIEkyQyBzeXN0ZW0sIGJ1dCBJIHRyaWVk
IHRvIHVuZGVyc3RhbmQgeW91ciBxdWVzdGlvbiBhcyBiZXN0IGFzIEkgY291bGQuDQoNCi9QLUcN
Cg0K
Hi Pär-Gunnar,
Sorry for the long delay, this time on my side.
On Thursday 24 February 2011, Par-Gunnar HJALMDAHL wrote:
> I sent the mail below one month ago, but have still not received any answers or further comments.
> Has anyone of you who made comments on the CG2900 patches earlier, any more comments or questions?
I tend to procrastinate replying to emails when it's a lot of work, and
yours dropped off the radar. I have not even read it until now, because
it's too long, has broken line-wraps and does not crop the citations
to a minimum. If you want people to listen to what you have to say, try
following email netiquette and put a little more effort into clear
communication like the rest of us do.
Please excuse my ignorance about topics we have already discussed here,
I don't know much about bluetooth and had to reread all the discussions
and patches.
> It is unclear to me where we stand with the CG2900 patches. I am myself happy with the
> current architecture, but I understand there are differing opinions.
> I would like to know what we need to do in order to get this driver into the Linux Kernel tree.
I have no idea what the current architecture is. In doubt, keep re-posting
the patches until all differences are resolved. If a thread does not
get a new message within a week, you can normally assume that poeple
no longer bother reading, and you should try harder to get feedback,
e.g. by sending a reminder that you are waiting or by resending
the modified patch set.
I still haven't looked at the patches from December, but I assume that
the code has changed in the meantime. Please go ahead and post the
current version again, then we can discuss the new code.
On Monday 17 January 2011, Par-Gunnar HJALMDAHL wrote:
> Arnd Bergmann wrote:
> >
> > Good point about hciattach, you certainly shouldn't need to use that if
> > the kernel already knows that a tty is connected to an HCI and what the
> > parameters are. Even more so if the HCI is not actually on a rs232 line
> > but something like i2c or spi. Automatically binding to the right line
> > discipline should be easily doable in the drivers though.
> >
>
> When having UART as transport you need something to open the UART and set the
> line discipline. If this is hciattach or something else is up to each developer
> to suit what they are doing. I don't see a problem with using hciattach even
> if you don't use the Bluetooth part (if the exe is part of the system anyway),
> but if a company/developer want to use something else they can do that. It's a
> usage of standard interfaces using open() and ioctl().
That's not what I meant. You don't really need to use the ioctl if the
kernel already knows what the device does. Just call tty_set_ldisc from
an appropriate location in the code.
> I still don't think that you should have a line discipline for other transports
> than UART. If I would look at how I would implement an SPI driver for CG2900,
> there would almost be no code that could be used in common between cg2900_uart
> and cg2900_spi. SPI doesn't change baud rate, SPI uses commands for sleep/wake
> instead of Break, SPI packets doesn't need extra packetizing, etc.
If you don't want the others to be handled in a line discipline, you should
start out with a patch that splits the hci uart protocol registration from the
line discipline code in drivers/bluetooth/hci_ldisc.c. Today, you can have
multiple HCI drivers in the system, but only the serial one can have
subprotocols.
> > I don't understand the problem with relying on the hci-uart or hci-h4
> > modules. If the hardware uses the H4 protocol, we certainly should use
> > the kernel module that knows how to deal with H4, and we don't want
> > to have two modules implementing the same protocol.
> >
>
> I must say I don't understand this problem either. Unless the protocol driver is
> activated through ioctl SET_PROTOCOL, the code will not be executed, and the amount
> of ROM needed is negligible.
> If you look at our submission, the hci-h4 could possibly be reused to some extent.
> Basically the packetizing to the Bluetooth H4 channels 1-4 could be re-used. In order
> to allow for vendor specific channels to be handled some new registration system on
> top of hci-h4 plus a callback system for data reception would have to be added (in
> order to facilitate systems that do not want all data to be sent directly to the
> Bluetooth stack such as the CG2900 driver). I'm afraid that we would have a
> significantly more complex system and larger amount of code if we would try to generalize
> the hci-h4 module. I definitely prefer the current model where a vendor specific driver
> replaces the hci-h4 protocol driver.
Then remove the hci-h4 driver from the kernel and make sure that your driver can
handle all the hardware that h4 can today, using identical user interfaces.
Two infrastructure drivers doing almost the same thing are not acceptable.
We sometimes have device drivers that are meant for the same hardware, but
then one of them is going away over time, when it is shown that the new one
does everything we need. For infrastructure, this is not as easy, because
then you get other people writing new backend for both of them and they
won't be interoperable.
> > The one important goal here should be to avoid code duplication.
> > Using the header to get the data structures from separate code
> > means you need to have similar parsing functions in each of the modules
> > using it. Not sharing the header wouldn't help, because then you end
> > up duplicating even more. The real solution, speaking very broadly,
> > must be to have a general module that deals with whatever the more
> > specific modules have in common, and have a header file that defines
> > the interface to it.
> >
>
> In general I agree with you, but there are some problems here.
> The most used BT HCI events are Command Status and Command Complete.
> Command Status could be parsed completely in a good way (retrieving op
> code, nbr of commands allowed, and status of command sent).
> Command Complete is however quite complex since the returned data will
> differ depending on command sent. Op code and nbr of commands allowed can
> be retrieved but everything else have to be extracted differently depending
> on command. This means that there is not much that will be saved here. But
> maybe we could extract some parsing into common functions, but I don't
> think you would gain that much.
>
> Moreover, this would lead to a major reimplementation of the hci_core.c and
> related files, since they do not use any exported common functions as it is
> today. I do not know if they (BlueZ community) would want this and I do not
> think that that should be part of the CG2900 driver to do. It should in that
> case be done separately.
No. If hci_core is not sufficient to work with cg2900, the answer is certainly
not to ignore hci_core and roll your own copy. Can you give an example
of how the command complete logic needs to react to different commands?
Can this be done using callbacks into the code that initially sent the
commands?
> For the CG2900 that is not possible. Even if you are running just GPS you
> still must download patches and settings and that is done through HCI
> commands over the Bluetooth command interface. We also use Bluetooth
> commands to identify the controller.
I don't understand. Do you mean the settings are sent over the wireless
interface in order to control devices that are connected directly to
the HCI? Why would you do that instead of just attaching the
GPS to the HCI on the non-bluetooth side?
Arnd