All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/11] mfd and bluetooth: Add CG2900 support
@ 2010-12-17 11:20 Par-Gunnar Hjalmdahl
  2010-12-17 12:11   ` Vitaly Wool
  2010-12-19 21:23 ` Linus Walleij
  0 siblings, 2 replies; 30+ messages in thread
From: Par-Gunnar Hjalmdahl @ 2010-12-17 11:20 UTC (permalink / raw
  To: Pavan Savoy, Vitaly Wool, Alan Cox, Arnd Bergmann, Samuel Ortiz,
	Marcel Holtmann
  Cc: linux-kernel, linux-bluetooth, Lukasz Rymanowski, Linus Walleij,
	Par-Gunnar Hjalmdahl, Par-Gunnar Hjalmdahl

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


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 00/11] mfd and bluetooth: Add CG2900 support
  2010-12-17 11:20 [PATCH 00/11] mfd and bluetooth: Add CG2900 support Par-Gunnar Hjalmdahl
@ 2010-12-17 12:11   ` Vitaly Wool
  2010-12-19 21:23 ` Linus Walleij
  1 sibling, 0 replies; 30+ messages in thread
From: Vitaly Wool @ 2010-12-17 12:11 UTC (permalink / raw
  To: Par-Gunnar Hjalmdahl
  Cc: Pavan Savoy, Alan Cox, Arnd Bergmann, Samuel Ortiz,
	Marcel Holtmann, linux-kernel, linux-bluetooth, Lukasz Rymanowski,
	Linus Walleij, Par-Gunnar Hjalmdahl

Hi Par,

On Fri, Dec 17, 2010 at 12:20 PM, Par-Gunnar Hjalmdahl
<par-gunnar.p.hjalmdahl@stericsson.com> 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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 00/11] mfd and bluetooth: Add CG2900 support
@ 2010-12-17 12:11   ` Vitaly Wool
  0 siblings, 0 replies; 30+ messages in thread
From: Vitaly Wool @ 2010-12-17 12:11 UTC (permalink / raw
  To: Par-Gunnar Hjalmdahl
  Cc: Pavan Savoy, Alan Cox, Arnd Bergmann, Samuel Ortiz,
	Marcel Holtmann, linux-kernel, linux-bluetooth, Lukasz Rymanowski,
	Linus Walleij, Par-Gunnar Hjalmdahl

Hi Par,

On Fri, Dec 17, 2010 at 12:20 PM, Par-Gunnar Hjalmdahl
<par-gunnar.p.hjalmdahl@stericsson.com> 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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* RE: [PATCH 00/11] mfd and bluetooth: Add CG2900 support
  2010-12-17 12:11   ` Vitaly Wool
@ 2010-12-17 12:43     ` Par-Gunnar HJALMDAHL
  -1 siblings, 0 replies; 30+ messages in thread
From: Par-Gunnar HJALMDAHL @ 2010-12-17 12:43 UTC (permalink / raw
  To: Vitaly Wool
  Cc: Pavan Savoy, Alan Cox, Arnd Bergmann, Samuel Ortiz,
	Marcel Holtmann, linux-kernel@vger.kernel.org,
	linux-bluetooth@vger.kernel.org, Lukasz Rymanowski, Linus WALLEIJ,
	Par-Gunnar Hjalmdahl

Hi Vitaly,

> -----Original Message-----
> From: Vitaly Wool [mailto:vitalywool@gmail.com]
> Sent: den 17 december 2010 13:11
> To: Par-Gunnar HJALMDAHL
> Cc: Pavan Savoy; Alan Cox; Arnd Bergmann; Samuel Ortiz; Marcel
> Holtmann; linux-kernel@vger.kernel.org; linux-
> bluetooth@vger.kernel.org; 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
> <par-gunnar.p.hjalmdahl@stericsson.com> 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


^ permalink raw reply	[flat|nested] 30+ messages in thread

* RE: [PATCH 00/11] mfd and bluetooth: Add CG2900 support
@ 2010-12-17 12:43     ` Par-Gunnar HJALMDAHL
  0 siblings, 0 replies; 30+ messages in thread
From: Par-Gunnar HJALMDAHL @ 2010-12-17 12:43 UTC (permalink / raw
  To: Vitaly Wool
  Cc: Pavan Savoy, Alan Cox, Arnd Bergmann, Samuel Ortiz,
	Marcel Holtmann, linux-kernel@vger.kernel.org,
	linux-bluetooth@vger.kernel.org, Lukasz Rymanowski, Linus WALLEIJ,
	Par-Gunnar Hjalmdahl

Hi Vitaly,

> -----Original Message-----
> From: Vitaly Wool [mailto:vitalywool@gmail.com]
> Sent: den 17 december 2010 13:11
> To: Par-Gunnar HJALMDAHL
> Cc: Pavan Savoy; Alan Cox; Arnd Bergmann; Samuel Ortiz; Marcel
> Holtmann; linux-kernel@vger.kernel.org; linux-
> bluetooth@vger.kernel.org; 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
> <par-gunnar.p.hjalmdahl@stericsson.com> 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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 00/11] mfd and bluetooth: Add CG2900 support
  2010-12-17 11:20 [PATCH 00/11] mfd and bluetooth: Add CG2900 support Par-Gunnar Hjalmdahl
  2010-12-17 12:11   ` Vitaly Wool
@ 2010-12-19 21:23 ` Linus Walleij
  2010-12-19 22:57   ` Vitaly Wool
  1 sibling, 1 reply; 30+ messages in thread
From: Linus Walleij @ 2010-12-19 21:23 UTC (permalink / raw
  To: Par-Gunnar Hjalmdahl
  Cc: Pavan Savoy, Vitaly Wool, Alan Cox, Arnd Bergmann, Samuel Ortiz,
	Marcel Holtmann, linux-kernel, linux-bluetooth, Lukasz Rymanowski,
	Par-Gunnar Hjalmdahl

2010/12/17 Par-Gunnar Hjalmdahl <par-gunnar.p.hjalmdahl@stericsson.com>:

> 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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 00/11] mfd and bluetooth: Add CG2900 support
  2010-12-19 21:23 ` Linus Walleij
@ 2010-12-19 22:57   ` Vitaly Wool
  2010-12-20  9:15       ` Par-Gunnar HJALMDAHL
  0 siblings, 1 reply; 30+ messages in thread
From: Vitaly Wool @ 2010-12-19 22:57 UTC (permalink / raw
  To: Linus Walleij
  Cc: Par-Gunnar Hjalmdahl, Pavan Savoy, Alan Cox, Arnd Bergmann,
	Samuel Ortiz, Marcel Holtmann, linux-kernel, linux-bluetooth,
	Lukasz Rymanowski, Par-Gunnar Hjalmdahl

Hi Linus,

On Sun, Dec 19, 2010 at 10:23 PM, Linus Walleij
<linus.walleij@linaro.org> 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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* RE: [PATCH 00/11] mfd and bluetooth: Add CG2900 support
  2010-12-19 22:57   ` Vitaly Wool
@ 2010-12-20  9:15       ` Par-Gunnar HJALMDAHL
  0 siblings, 0 replies; 30+ messages in thread
From: Par-Gunnar HJALMDAHL @ 2010-12-20  9:15 UTC (permalink / raw
  To: Vitaly Wool, Linus Walleij
  Cc: Pavan Savoy, Alan Cox, Arnd Bergmann, Samuel Ortiz,
	Marcel Holtmann, linux-kernel@vger.kernel.org,
	linux-bluetooth@vger.kernel.org, Lukasz Rymanowski,
	Par-Gunnar Hjalmdahl

Hi Vitaly,

> -----Original Message-----
> From: Vitaly Wool [mailto:vitalywool@gmail.com]
> Sent: den 19 december 2010 23:58
> To: Linus Walleij
> Cc: Par-Gunnar HJALMDAHL; Pavan Savoy; Alan Cox; Arnd Bergmann; Samuel
> Ortiz; Marcel Holtmann; linux-kernel@vger.kernel.org; linux-
> bluetooth@vger.kernel.org; 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
> <linus.walleij@linaro.org> 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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* RE: [PATCH 00/11] mfd and bluetooth: Add CG2900 support
@ 2010-12-20  9:15       ` Par-Gunnar HJALMDAHL
  0 siblings, 0 replies; 30+ messages in thread
From: Par-Gunnar HJALMDAHL @ 2010-12-20  9:15 UTC (permalink / raw
  To: Vitaly Wool, Linus Walleij
  Cc: Pavan Savoy, Alan Cox, Arnd Bergmann, Samuel Ortiz,
	Marcel Holtmann, linux-kernel@vger.kernel.org,
	linux-bluetooth@vger.kernel.org, Lukasz Rymanowski,
	Par-Gunnar Hjalmdahl

Hi Vitaly,

> -----Original Message-----
> From: Vitaly Wool [mailto:vitalywool@gmail.com]
> Sent: den 19 december 2010 23:58
> To: Linus Walleij
> Cc: Par-Gunnar HJALMDAHL; Pavan Savoy; Alan Cox; Arnd Bergmann; Samuel
> Ortiz; Marcel Holtmann; linux-kernel@vger.kernel.org; linux-
> bluetooth@vger.kernel.org; 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
> <linus.walleij@linaro.org> 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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 00/11] mfd and bluetooth: Add CG2900 support
  2010-12-20  9:15       ` Par-Gunnar HJALMDAHL
@ 2010-12-23 10:48         ` Pavan Savoy
  -1 siblings, 0 replies; 30+ messages in thread
From: Pavan Savoy @ 2010-12-23 10:48 UTC (permalink / raw
  To: Par-Gunnar HJALMDAHL
  Cc: Vitaly Wool, Linus Walleij, Alan Cox, Arnd Bergmann, Samuel Ortiz,
	Marcel Holtmann, linux-kernel@vger.kernel.org,
	linux-bluetooth@vger.kernel.org, Lukasz Rymanowski,
	Par-Gunnar Hjalmdahl

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
>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 00/11] mfd and bluetooth: Add CG2900 support
@ 2010-12-23 10:48         ` Pavan Savoy
  0 siblings, 0 replies; 30+ messages in thread
From: Pavan Savoy @ 2010-12-23 10:48 UTC (permalink / raw
  To: Par-Gunnar HJALMDAHL
  Cc: Vitaly Wool, Linus Walleij, Alan Cox, Arnd Bergmann, Samuel Ortiz,
	Marcel Holtmann, linux-kernel@vger.kernel.org,
	linux-bluetooth@vger.kernel.org, Lukasz Rymanowski,
	Par-Gunnar Hjalmdahl

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
>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* RE: [PATCH 00/11] mfd and bluetooth: Add CG2900 support
  2010-12-23 10:48         ` Pavan Savoy
@ 2011-01-05 12:56           ` Par-Gunnar HJALMDAHL
  -1 siblings, 0 replies; 30+ messages in thread
From: Par-Gunnar HJALMDAHL @ 2011-01-05 12:56 UTC (permalink / raw
  To: Pavan Savoy
  Cc: Vitaly Wool, Linus Walleij, Alan Cox, Arnd Bergmann, Samuel Ortiz,
	Marcel Holtmann, linux-kernel@vger.kernel.org,
	linux-bluetooth@vger.kernel.org, Lukasz Rymanowski,
	Par-Gunnar Hjalmdahl

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 4252 bytes --]

Hi Pavan,

Sorry for not answering sooner. I've been on Christmas and New Year vacation.

> -----Original Message-----
> From: Pavan Savoy [mailto:pavan_savoy@sify.com]
> Sent: den 23 december 2010 11:49
> To: Par-Gunnar HJALMDAHL
> Cc: Vitaly Wool; Linus Walleij; Alan Cox; Arnd Bergmann; Samuel Ortiz;
> Marcel Holtmann; linux-kernel@vger.kernel.org; linux-
> bluetooth@vger.kernel.org; Lukasz Rymanowski; Par-Gunnar Hjalmdahl
> Subject: Re: [PATCH 00/11] mfd and bluetooth: Add CG2900 support
> 
> 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?
> 

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

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

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

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

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.

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.

/P-G

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

^ permalink raw reply	[flat|nested] 30+ messages in thread

* RE: [PATCH 00/11] mfd and bluetooth: Add CG2900 support
@ 2011-01-05 12:56           ` Par-Gunnar HJALMDAHL
  0 siblings, 0 replies; 30+ messages in thread
From: Par-Gunnar HJALMDAHL @ 2011-01-05 12:56 UTC (permalink / raw
  To: Pavan Savoy
  Cc: Vitaly Wool, Linus Walleij, Alan Cox, Arnd Bergmann, Samuel Ortiz,
	Marcel Holtmann, linux-kernel@vger.kernel.org,
	linux-bluetooth@vger.kernel.org, Lukasz Rymanowski,
	Par-Gunnar Hjalmdahl

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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 00/11] mfd and bluetooth: Add CG2900 support
  2011-01-05 12:56           ` Par-Gunnar HJALMDAHL
  (?)
@ 2011-01-06 18:46           ` Arnd Bergmann
  2011-01-09 18:12               ` Pavan Savoy
  -1 siblings, 1 reply; 30+ messages in thread
From: Arnd Bergmann @ 2011-01-06 18:46 UTC (permalink / raw
  To: Par-Gunnar HJALMDAHL
  Cc: Pavan Savoy, Vitaly Wool, Linus Walleij, Alan Cox, Samuel Ortiz,
	Marcel Holtmann, linux-kernel@vger.kernel.org,
	linux-bluetooth@vger.kernel.org, Lukasz Rymanowski,
	Par-Gunnar Hjalmdahl

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


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 00/11] mfd and bluetooth: Add CG2900 support
  2011-01-06 18:46           ` Arnd Bergmann
@ 2011-01-09 18:12               ` Pavan Savoy
  0 siblings, 0 replies; 30+ messages in thread
From: Pavan Savoy @ 2011-01-09 18:12 UTC (permalink / raw
  To: Arnd Bergmann
  Cc: Par-Gunnar HJALMDAHL, Vitaly Wool, Linus Walleij, Alan Cox,
	Samuel Ortiz, Marcel Holtmann, linux-kernel@vger.kernel.org,
	linux-bluetooth@vger.kernel.org, Lukasz Rymanowski,
	Par-Gunnar Hjalmdahl

On Fri, Jan 7, 2011 at 12:16 AM, Arnd Bergmann <arnd@arndb.de> 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...

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


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

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

Thanks for the comments...
>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 00/11] mfd and bluetooth: Add CG2900 support
@ 2011-01-09 18:12               ` Pavan Savoy
  0 siblings, 0 replies; 30+ messages in thread
From: Pavan Savoy @ 2011-01-09 18:12 UTC (permalink / raw
  To: Arnd Bergmann
  Cc: Par-Gunnar HJALMDAHL, Vitaly Wool, Linus Walleij, Alan Cox,
	Samuel Ortiz, Marcel Holtmann, linux-kernel@vger.kernel.org,
	linux-bluetooth@vger.kernel.org, Lukasz Rymanowski,
	Par-Gunnar Hjalmdahl

On Fri, Jan 7, 2011 at 12:16 AM, Arnd Bergmann <arnd@arndb.de> 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...
>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 00/11] mfd and bluetooth: Add CG2900 support
  2011-01-09 18:12               ` Pavan Savoy
  (?)
@ 2011-01-09 18:48               ` Vitaly Wool
  -1 siblings, 0 replies; 30+ messages in thread
From: Vitaly Wool @ 2011-01-09 18:48 UTC (permalink / raw
  To: Pavan Savoy
  Cc: Arnd Bergmann, Par-Gunnar HJALMDAHL, Linus Walleij, Alan Cox,
	Samuel Ortiz, Marcel Holtmann, linux-kernel@vger.kernel.org,
	linux-bluetooth@vger.kernel.org, Lukasz Rymanowski,
	Par-Gunnar Hjalmdahl

On Sun, Jan 9, 2011 at 7:12 PM, Pavan Savoy <pavan_savoy@sify.com> 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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 00/11] mfd and bluetooth: Add CG2900 suppor
  2011-01-09 18:12               ` Pavan Savoy
  (?)
  (?)
@ 2011-01-09 18:55               ` Arnd Bergmann
  2011-01-17 15:32                   ` Par-Gunnar HJALMDAHL
  2011-02-24 14:16                   ` Par-Gunnar HJALMDAHL
  -1 siblings, 2 replies; 30+ messages in thread
From: Arnd Bergmann @ 2011-01-09 18:55 UTC (permalink / raw
  To: Pavan Savoy
  Cc: Par-Gunnar HJALMDAHL, Vitaly Wool, Linus Walleij, Alan Cox,
	Samuel Ortiz, Marcel Holtmann, linux-kernel@vger.kernel.org,
	linux-bluetooth@vger.kernel.org, Lukasz Rymanowski,
	Par-Gunnar Hjalmdahl

On Sunday 09 January 2011, Pavan Savoy wrote:
> On Fri, Jan 7, 2011 at 12:16 AM, Arnd Bergmann <arnd@arndb.de> 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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* RE: [PATCH 00/11] mfd and bluetooth: Add CG2900 suppor
  2011-01-09 18:55               ` [PATCH 00/11] mfd and bluetooth: Add CG2900 suppor Arnd Bergmann
@ 2011-01-17 15:32                   ` Par-Gunnar HJALMDAHL
  2011-02-24 14:16                   ` Par-Gunnar HJALMDAHL
  1 sibling, 0 replies; 30+ messages in thread
From: Par-Gunnar HJALMDAHL @ 2011-01-17 15:32 UTC (permalink / raw
  To: Arnd Bergmann, Pavan Savoy
  Cc: Vitaly Wool, Linus Walleij, Alan Cox, Samuel Ortiz,
	Marcel Holtmann, linux-kernel@vger.kernel.org,
	linux-bluetooth@vger.kernel.org, Lukasz Rymanowski,
	Par-Gunnar Hjalmdahl

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 10074 bytes --]

Hi,

Sorry for not answering earlier. I've been overloaded with things to do now after New Year. See below.

> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: den 9 januari 2011 19:55
> To: Pavan Savoy
> Cc: Par-Gunnar HJALMDAHL; Vitaly Wool; Linus Walleij; Alan Cox; Samuel
> Ortiz; Marcel Holtmann; linux-kernel@vger.kernel.org; linux-
> bluetooth@vger.kernel.org; Lukasz Rymanowski; Par-Gunnar Hjalmdahl
> Subject: Re: [PATCH 00/11] mfd and bluetooth: Add CG2900 suppor
> 
> On Sunday 09 January 2011, Pavan Savoy wrote:
> > On Fri, Jan 7, 2011 at 12:16 AM, Arnd Bergmann <arnd@arndb.de> 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.
> 

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

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

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

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.

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

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.

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

/P-G

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

^ permalink raw reply	[flat|nested] 30+ messages in thread

* RE: [PATCH 00/11] mfd and bluetooth: Add CG2900 suppor
@ 2011-01-17 15:32                   ` Par-Gunnar HJALMDAHL
  0 siblings, 0 replies; 30+ messages in thread
From: Par-Gunnar HJALMDAHL @ 2011-01-17 15:32 UTC (permalink / raw
  To: Arnd Bergmann, Pavan Savoy
  Cc: Vitaly Wool, Linus Walleij, Alan Cox, Samuel Ortiz,
	Marcel Holtmann, linux-kernel@vger.kernel.org,
	linux-bluetooth@vger.kernel.org, Lukasz Rymanowski,
	Par-Gunnar Hjalmdahl

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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* RE: [PATCH 00/11] mfd and bluetooth: Add CG2900 suppor
  2011-01-09 18:55               ` [PATCH 00/11] mfd and bluetooth: Add CG2900 suppor Arnd Bergmann
@ 2011-02-24 14:16                   ` Par-Gunnar HJALMDAHL
  2011-02-24 14:16                   ` Par-Gunnar HJALMDAHL
  1 sibling, 0 replies; 30+ messages in thread
From: Par-Gunnar HJALMDAHL @ 2011-02-24 14:16 UTC (permalink / raw
  To: Par-Gunnar HJALMDAHL, Arnd Bergmann, Pavan Savoy
  Cc: Vitaly Wool, Linus Walleij, Alan Cox, Samuel Ortiz,
	Marcel Holtmann, linux-kernel@vger.kernel.org,
	linux-bluetooth@vger.kernel.org, Lukasz Rymanowski,
	Par-Gunnar Hjalmdahl, Lee Jones

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 11389 bytes --]

Hi,

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?

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.

/P-G

> -----Original Message-----
> From: Par-Gunnar HJALMDAHL
> Sent: den 17 januari 2011 16:33
> To: 'Arnd Bergmann'; Pavan Savoy
> Cc: Vitaly Wool; Linus Walleij; Alan Cox; Samuel Ortiz; Marcel
> Holtmann; linux-kernel@vger.kernel.org; linux-
> bluetooth@vger.kernel.org; Lukasz Rymanowski; Par-Gunnar Hjalmdahl
> Subject: RE: [PATCH 00/11] mfd and bluetooth: Add CG2900 suppor
> 
> Hi,
> 
> Sorry for not answering earlier. I've been overloaded with things to do
> now after New Year. See below.
> 
> > -----Original Message-----
> > From: Arnd Bergmann [mailto:arnd@arndb.de]
> > Sent: den 9 januari 2011 19:55
> > To: Pavan Savoy
> > Cc: Par-Gunnar HJALMDAHL; Vitaly Wool; Linus Walleij; Alan Cox;
> Samuel
> > Ortiz; Marcel Holtmann; linux-kernel@vger.kernel.org; linux-
> > bluetooth@vger.kernel.org; Lukasz Rymanowski; Par-Gunnar Hjalmdahl
> > Subject: Re: [PATCH 00/11] mfd and bluetooth: Add CG2900 suppor
> >
> > On Sunday 09 January 2011, Pavan Savoy wrote:
> > > On Fri, Jan 7, 2011 at 12:16 AM, Arnd Bergmann <arnd@arndb.de>
> 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.
> >
> 
> 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().
> 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.
> 
> > 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.
> 
> > > >> > 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.
> >
> 
> 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.
> 
> > > >> 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.
> >
> 
> 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.
> 
> > > 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
> 
> /P-G

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

^ permalink raw reply	[flat|nested] 30+ messages in thread

* RE: [PATCH 00/11] mfd and bluetooth: Add CG2900 suppor
@ 2011-02-24 14:16                   ` Par-Gunnar HJALMDAHL
  0 siblings, 0 replies; 30+ messages in thread
From: Par-Gunnar HJALMDAHL @ 2011-02-24 14:16 UTC (permalink / raw
  To: Par-Gunnar HJALMDAHL, Arnd Bergmann, Pavan Savoy
  Cc: Vitaly Wool, Linus Walleij, Alan Cox, Samuel Ortiz,
	Marcel Holtmann, linux-kernel@vger.kernel.org,
	linux-bluetooth@vger.kernel.org, Lukasz Rymanowski,
	Par-Gunnar Hjalmdahl, Lee Jones

SGksDQoNCkkgc2VudCB0aGUgbWFpbCBiZWxvdyBvbmUgbW9udGggYWdvLCBidXQgaGF2ZSBzdGls
bCBub3QgcmVjZWl2ZWQgYW55IGFuc3dlcnMgb3IgZnVydGhlciBjb21tZW50cy4NCkhhcyBhbnlv
bmUgb2YgeW91IHdobyBtYWRlIGNvbW1lbnRzIG9uIHRoZSBDRzI5MDAgcGF0Y2hlcyBlYXJsaWVy
LCBhbnkgbW9yZSBjb21tZW50cyBvciBxdWVzdGlvbnM/DQoNCkl0IGlzIHVuY2xlYXIgdG8gbWUg
d2hlcmUgd2Ugc3RhbmQgd2l0aCB0aGUgQ0cyOTAwIHBhdGNoZXMuIEkgYW0gbXlzZWxmIGhhcHB5
IHdpdGggdGhlIGN1cnJlbnQgYXJjaGl0ZWN0dXJlLCBidXQgSSB1bmRlcnN0YW5kIHRoZXJlIGFy
ZSBkaWZmZXJpbmcgb3BpbmlvbnMuDQpJIHdvdWxkIGxpa2UgdG8ga25vdyB3aGF0IHdlIG5lZWQg
dG8gZG8gaW4gb3JkZXIgdG8gZ2V0IHRoaXMgZHJpdmVyIGludG8gdGhlIExpbnV4IEtlcm5lbCB0
cmVlLg0KDQovUC1HDQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogUGFy
LUd1bm5hciBISkFMTURBSEwNCj4gU2VudDogZGVuIDE3IGphbnVhcmkgMjAxMSAxNjozMw0KPiBU
bzogJ0FybmQgQmVyZ21hbm4nOyBQYXZhbiBTYXZveQ0KPiBDYzogVml0YWx5IFdvb2w7IExpbnVz
IFdhbGxlaWo7IEFsYW4gQ294OyBTYW11ZWwgT3J0aXo7IE1hcmNlbA0KPiBIb2x0bWFubjsgbGlu
dXgta2VybmVsQHZnZXIua2VybmVsLm9yZzsgbGludXgtDQo+IGJsdWV0b290aEB2Z2VyLmtlcm5l
bC5vcmc7IEx1a2FzeiBSeW1hbm93c2tpOyBQYXItR3VubmFyIEhqYWxtZGFobA0KPiBTdWJqZWN0
OiBSRTogW1BBVENIIDAwLzExXSBtZmQgYW5kIGJsdWV0b290aDogQWRkIENHMjkwMCBzdXBwb3IN
Cj4gDQo+IEhpLA0KPiANCj4gU29ycnkgZm9yIG5vdCBhbnN3ZXJpbmcgZWFybGllci4gSSd2ZSBi
ZWVuIG92ZXJsb2FkZWQgd2l0aCB0aGluZ3MgdG8gZG8NCj4gbm93IGFmdGVyIE5ldyBZZWFyLiBT
ZWUgYmVsb3cuDQo+IA0KPiA+IC0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+ID4gRnJvbTog
QXJuZCBCZXJnbWFubiBbbWFpbHRvOmFybmRAYXJuZGIuZGVdDQo+ID4gU2VudDogZGVuIDkgamFu
dWFyaSAyMDExIDE5OjU1DQo+ID4gVG86IFBhdmFuIFNhdm95DQo+ID4gQ2M6IFBhci1HdW5uYXIg
SEpBTE1EQUhMOyBWaXRhbHkgV29vbDsgTGludXMgV2FsbGVpajsgQWxhbiBDb3g7DQo+IFNhbXVl
bA0KPiA+IE9ydGl6OyBNYXJjZWwgSG9sdG1hbm47IGxpbnV4LWtlcm5lbEB2Z2VyLmtlcm5lbC5v
cmc7IGxpbnV4LQ0KPiA+IGJsdWV0b290aEB2Z2VyLmtlcm5lbC5vcmc7IEx1a2FzeiBSeW1hbm93
c2tpOyBQYXItR3VubmFyIEhqYWxtZGFobA0KPiA+IFN1YmplY3Q6IFJlOiBbUEFUQ0ggMDAvMTFd
IG1mZCBhbmQgYmx1ZXRvb3RoOiBBZGQgQ0cyOTAwIHN1cHBvcg0KPiA+DQo+ID4gT24gU3VuZGF5
IDA5IEphbnVhcnkgMjAxMSwgUGF2YW4gU2F2b3kgd3JvdGU6DQo+ID4gPiBPbiBGcmksIEphbiA3
LCAyMDExIGF0IDEyOjE2IEFNLCBBcm5kIEJlcmdtYW5uIDxhcm5kQGFybmRiLmRlPg0KPiB3cm90
ZToNCj4gPiA+ID4gT24gV2VkbmVzZGF5IDA1IEphbnVhcnkgMjAxMSwgUGFyLUd1bm5hciBISkFM
TURBSEwgd3JvdGU6DQo+ID4gPiA+DQo+ID4gPiA+PiBTb3JyeSBmb3Igbm90IGFuc3dlcmluZyBz
b29uZXIuIEkndmUgYmVlbiBvbiBDaHJpc3RtYXMgYW5kIE5ldw0KPiA+IFllYXIgdmFjYXRpb24u
DQo+ID4gPiA+DQo+ID4gPiA+IEknbSBhbHNvIHN0aWxsIGNhdGNoaW5nIHVwIHdpdGggZW1haWwg
dGhhdCBoYXMgYWNjdW11bGF0ZWQgb3Zlcg0KPiBteQ0KPiA+ID4gPiB2YWNhdGlvbiwgaW5jbHVk
aW5nIHlvdXIgcHJldmlvdXMgcmVzcG9uc2UuDQo+ID4gPiA+DQo+ID4gPiA+IFRoaXMgc291bmRz
IHdyb25nIGZvciBib3RoIFRJIGFuZCBTVC1FOiBBRkFJQ1QgdGhleSBhcmUgYWN0dWFsbHkNCj4g
PiBidWlsdA0KPiA+ID4gPiBhcm91bmQgYW4gSENJIGludGVyZmFjZS4gSXQncyB1bmZvcnR1bmF0
ZSB0aGF0IHRoZSBUSSBjb2RlDQo+IGFjdHVhbGx5DQo+ID4gZ290DQo+ID4gPiA+IG1lcmdlZCBp
bnRvIHRoZSBrZXJuZWwgbGlrZSB0aGlzLg0KPiA+ID4NCj4gPiA+IEkgYW0gbm90IHN1cmUgd2hh
dCBkb2VzIGJ1aWx0IGFyb3VuZCBIQ0kgSW50ZXJmYWNlIG1lYW4/IEFsc28geWVzLA0KPiBpbg0K
PiA+IFRJLSBjb2RlDQo+ID4gPiB3ZSBkbyByZWZlciB0byBCbHVldG9vdGggaGVhZGVycy4NCj4g
PiA+IEhvd2V2ZXIgdGhlIGZhY3QgdGhhdCBJIHdhbnRlZCB0byBwb2ludCBvdXQgdG8gUGFyLUd1
bm5hciB3YXMsIHRoYXQNCj4gPiB3ZQ0KPiA+ID4gZG9uJ3Qgd2FudCB0byB1c2UNCj4gPiA+IGhj
aWF0dGFjaCBhbmQgZW5hYmxlIEhDSS1VQVJUICsgSENJLUg0IGZvciBlbmFibGluZyBvdXIgZHJp
dmVyIG9yDQo+IG91cg0KPiA+ID4gZHJpdmVyIHNob3VsZCBub3QNCj4gPiA+IGRlcGVuZCBvbiB0
aG9zZSBtb2R1bGVzIGFzIHN1Y2guLi4NCj4gPg0KPiA+IEdvb2QgcG9pbnQgYWJvdXQgaGNpYXR0
YWNoLCB5b3UgY2VydGFpbmx5IHNob3VsZG4ndCBuZWVkIHRvIHVzZSB0aGF0DQo+IGlmDQo+ID4g
dGhlIGtlcm5lbCBhbHJlYWR5IGtub3dzIHRoYXQgYSB0dHkgaXMgY29ubmVjdGVkIHRvIGFuIEhD
SSBhbmQgd2hhdA0KPiB0aGUNCj4gPiBwYXJhbWV0ZXJzIGFyZS4gRXZlbiBtb3JlIHNvIGlmIHRo
ZSBIQ0kgaXMgbm90IGFjdHVhbGx5IG9uIGEgcnMyMzINCj4gbGluZQ0KPiA+IGJ1dCBzb21ldGhp
bmcgbGlrZSBpMmMgb3Igc3BpLiBBdXRvbWF0aWNhbGx5IGJpbmRpbmcgdG8gdGhlIHJpZ2h0DQo+
IGxpbmUNCj4gPiBkaXNjaXBsaW5lIHNob3VsZCBiZSBlYXNpbHkgZG9hYmxlIGluIHRoZSBkcml2
ZXJzIHRob3VnaC4NCj4gPg0KPiANCj4gV2hlbiBoYXZpbmcgVUFSVCBhcyB0cmFuc3BvcnQgeW91
IG5lZWQgc29tZXRoaW5nIHRvIG9wZW4gdGhlIFVBUlQgYW5kDQo+IHNldCB0aGUgbGluZSBkaXNj
aXBsaW5lLiBJZiB0aGlzIGlzIGhjaWF0dGFjaCBvciBzb21ldGhpbmcgZWxzZSBpcyB1cA0KPiB0
byBlYWNoIGRldmVsb3BlciB0byBzdWl0IHdoYXQgdGhleSBhcmUgZG9pbmcuIEkgZG9uJ3Qgc2Vl
IGEgcHJvYmxlbQ0KPiB3aXRoIHVzaW5nIGhjaWF0dGFjaCBldmVuIGlmIHlvdSBkb24ndCB1c2Ug
dGhlIEJsdWV0b290aCBwYXJ0IChpZiB0aGUNCj4gZXhlIGlzIHBhcnQgb2YgdGhlIHN5c3RlbSBh
bnl3YXkpLCBidXQgaWYgYSBjb21wYW55L2RldmVsb3BlciB3YW50IHRvDQo+IHVzZSBzb21ldGhp
bmcgZWxzZSB0aGV5IGNhbiBkbyB0aGF0LiBJdCdzIGEgdXNhZ2Ugb2Ygc3RhbmRhcmQNCj4gaW50
ZXJmYWNlcyB1c2luZyBvcGVuKCkgYW5kIGlvY3RsKCkuDQo+IEkgc3RpbGwgZG9uJ3QgdGhpbmsg
dGhhdCB5b3Ugc2hvdWxkIGhhdmUgYSBsaW5lIGRpc2NpcGxpbmUgZm9yIG90aGVyDQo+IHRyYW5z
cG9ydHMgdGhhbiBVQVJULiBJZiBJIHdvdWxkIGxvb2sgYXQgaG93IEkgd291bGQgaW1wbGVtZW50
IGFuIFNQSQ0KPiBkcml2ZXIgZm9yIENHMjkwMCwgdGhlcmUgd291bGQgYWxtb3N0IGJlIG5vIGNv
ZGUgdGhhdCBjb3VsZCBiZSB1c2VkIGluDQo+IGNvbW1vbiBiZXR3ZWVuIGNnMjkwMF91YXJ0IGFu
ZCBjZzI5MDBfc3BpLiBTUEkgZG9lc24ndCBjaGFuZ2UgYmF1ZA0KPiByYXRlLCBTUEkgdXNlcyBj
b21tYW5kcyBmb3Igc2xlZXAvd2FrZSBpbnN0ZWFkIG9mIEJyZWFrLCBTUEkgcGFja2V0cw0KPiBk
b2Vzbid0IG5lZWQgZXh0cmEgcGFja2V0aXppbmcsIGV0Yy4NCj4gDQo+ID4gSSBkb24ndCB1bmRl
cnN0YW5kIHRoZSBwcm9ibGVtIHdpdGggcmVseWluZyBvbiB0aGUgaGNpLXVhcnQgb3IgaGNpLWg0
DQo+ID4gbW9kdWxlcy4gSWYgdGhlIGhhcmR3YXJlIHVzZXMgdGhlIEg0IHByb3RvY29sLCB3ZSBj
ZXJ0YWlubHkgc2hvdWxkDQo+IHVzZQ0KPiA+IHRoZSBrZXJuZWwgbW9kdWxlIHRoYXQga25vd3Mg
aG93IHRvIGRlYWwgd2l0aCBINCwgYW5kIHdlIGRvbid0IHdhbnQNCj4gPiB0byBoYXZlIHR3byBt
b2R1bGVzIGltcGxlbWVudGluZyB0aGUgc2FtZSBwcm90b2NvbC4NCj4gPg0KPiANCj4gSSBtdXN0
IHNheSBJIGRvbid0IHVuZGVyc3RhbmQgdGhpcyBwcm9ibGVtIGVpdGhlci4gVW5sZXNzIHRoZSBw
cm90b2NvbA0KPiBkcml2ZXIgaXMgYWN0aXZhdGVkIHRocm91Z2ggaW9jdGwgU0VUX1BST1RPQ09M
LCB0aGUgY29kZSB3aWxsIG5vdCBiZQ0KPiBleGVjdXRlZCwgYW5kIHRoZSBhbW91bnQgb2YgUk9N
IG5lZWRlZCBpcyBuZWdsaWdpYmxlLg0KPiBJZiB5b3UgbG9vayBhdCBvdXIgc3VibWlzc2lvbiwg
dGhlIGhjaS1oNCBjb3VsZCBwb3NzaWJseSBiZSByZXVzZWQgdG8NCj4gc29tZSBleHRlbnQuIEJh
c2ljYWxseSB0aGUgcGFja2V0aXppbmcgdG8gdGhlIEJsdWV0b290aCBINCBjaGFubmVscyAxLTQN
Cj4gY291bGQgYmUgcmUtdXNlZC4gSW4gb3JkZXIgdG8gYWxsb3cgZm9yIHZlbmRvciBzcGVjaWZp
YyBjaGFubmVscyB0byBiZQ0KPiBoYW5kbGVkIHNvbWUgbmV3IHJlZ2lzdHJhdGlvbiBzeXN0ZW0g
b24gdG9wIG9mIGhjaS1oNCBwbHVzIGEgY2FsbGJhY2sNCj4gc3lzdGVtIGZvciBkYXRhIHJlY2Vw
dGlvbiB3b3VsZCBoYXZlIHRvIGJlIGFkZGVkIChpbiBvcmRlciB0bw0KPiBmYWNpbGl0YXRlIHN5
c3RlbXMgdGhhdCBkbyBub3Qgd2FudCBhbGwgZGF0YSB0byBiZSBzZW50IGRpcmVjdGx5IHRvIHRo
ZQ0KPiBCbHVldG9vdGggc3RhY2sgc3VjaCBhcyB0aGUgQ0cyOTAwIGRyaXZlcikuIEknbSBhZnJh
aWQgdGhhdCB3ZSB3b3VsZA0KPiBoYXZlIGEgc2lnbmlmaWNhbnRseSBtb3JlIGNvbXBsZXggc3lz
dGVtIGFuZCBsYXJnZXIgYW1vdW50IG9mIGNvZGUgaWYNCj4gd2Ugd291bGQgdHJ5IHRvIGdlbmVy
YWxpemUgdGhlIGhjaS1oNCBtb2R1bGUuIEkgZGVmaW5pdGVseSBwcmVmZXIgdGhlDQo+IGN1cnJl
bnQgbW9kZWwgd2hlcmUgYSB2ZW5kb3Igc3BlY2lmaWMgZHJpdmVyIHJlcGxhY2VzIHRoZSBoY2kt
aDQNCj4gcHJvdG9jb2wgZHJpdmVyLg0KPiANCj4gPiA+ID4+ID4gaW5zdGVhZCBvZiBjb21tb24t
aGNpLW1vZHVsZSwgd2h5IG5vdCBjcmVhdGUgYSBhbGdvLWRyaXZlcg0KPiBsYXllcg0KPiA+ICdh
bGENCj4gPiA+ID4+ID4gaTJjID8gd2hlcmUgaW5kaXZpZHVhbCBkcml2ZXJzIGNhbiByZWdpc3Rl
ciB0aGVpciByZWNlaXZlDQo+ID4gaGFuZGxlcnMgZm9yDQo+ID4gPiA+PiA+IGRhdGEgaW50ZXJw
cmV0YXRpb24gPw0KPiA+ID4gPg0KPiA+ID4gPiBUaGF0IHdvdWxkIGJlIHdoYXQgSSBzdWdnZXN0
ZWQgOy0pDQo+ID4gPg0KPiA+ID4gQnV0IGV2ZW4gaGVyZSB0b28sIHRoZSBhbGdvcyBsYXllciBp
ZiB5b3UgaW1hZ2luZSB3aGljaCBjYW4gYmUgdGhlDQo+ID4gPiBzb3J0IG9mIHRoZSBmaXJzdA0K
PiA+ID4gcmVjZWl2ZXIgb2YgZGF0YSBmcm9tIHRoZSB0cmFuc3BvcnQgd291bGQgcmVmZXIgdG8g
QlQgaGVhZGVycyB0bw0KPiA+ID4gaW50ZXJwcmV0IHRoZSBkYXRhIChub3QganVzdCBCVCwgYnV0
IEZNL0dQUykNCj4gPiA+IGFuZCBwYXNzIGl0IG9udG8gb3RoZXIgcHJvdG9jb2wvY2xpZW50IGRy
aXZlcnMsDQo+ID4NCj4gPiBSaWdodCwgdGhhdCBpcyB0aGUgZW50aXJlIGlkZWEsIGFuZCBJIGRv
bid0IHNlZSBhIHByb2JsZW0gaGVyZS4NCj4gPiBJZiB5b3UgZG8gdGhpcywgeW91IHVzZSB0aGUg
aGVhZGVycyBvZiB0aGUgdHdvIHN1YnN5c3RlbXMgeW91DQo+ID4gaW50ZXJmYWNlIHdpdGguIFdo
YXQgeW91IHNob3VsZCAvbm90LyBpbnN0ZWFkIGlzIHVzZSBoZWFkZXINCj4gPiBmaWxlcyBvZiBh
IHN1YnN5c3RlbSB5b3UgZG9uJ3QgaW50ZXJmYWNlIHdpdGggYW5kIHJlaW50ZXJwcmV0ZQ0KPiA+
IHRoZSBkZWZpbml0aW9ucyBpbiBjcmVhdGl2ZSB3YXlzLCB3aGljaCBpcyB3aGF0IEkgdW5kZXJz
dG9vZA0KPiA+IHdhcyBiZWluZyBkaXNjdXNzZWQgZWFybGllci4NCj4gPg0KPiA+ID4gPj4gSW4g
c29tZSB3YXkgeW91IHRoZW4gcnVuIGludG8gdGhlIHNhbWUgcHJvYmxlbSBoYXMgSSBoYWQgaW4N
Cj4gPiBwcmV2aW91cyBwYXRjaA0KPiA+ID4gPj4gc2V0cy4gVGhlIGZ1bmN0aW9uYWxpdGllcyBz
dXBwb3J0ZWQgaXMgcmVhbGx5IGRldGVybWluZWQgYnkgZWFjaA0KPiA+IGNoaXAuDQo+ID4gPiA+
PiBZb3UgbWlnaHQgb3IgbWlnaHQgbm90IGhhdmUgZm9yIGV4YW1wbGUgR1BTIGluIGEgY2VydGFp
biBjaGlwLg0KPiBTbw0KPiA+IHlvdSBkbyBub3QNCj4gPiA+ID4+IHdhbnQgYSBjZW50cmFsIG1v
ZHVsZSB0byBleHBvc2UgYWxsIHBvc3NpYmxlIGNoYW5uZWxzIHRvIHRoZQ0KPiA+IHN0YWNrcyBv
biB0b3AuDQo+ID4gPiA+Pg0KPiA+ID4gPj4gWW91IG9ubHkgd2FudCB0aGUgYWN0dWFsbHkgc3Vw
cG9ydGVkIGZlYXR1cmVzIHRvIGJlIGV4cG9zZWQgdG8NCj4gPiB1cHBlciBsYXllcnMuDQo+ID4g
PiA+PiBUaGVuIHRoZSBNRkQgc3lzdGVtIGlzIHByZXR0eSBuaWNlLiBJdCdzIGVhc3kgYW5kIG1v
ZHVsYXJpemVkIHRvDQo+ID4gZXhwb3NlIHRoZQ0KPiA+ID4gPj4gZGlmZmVyZW50IGNoYW5uZWxz
IGFzIE1GRCBjZWxscy4NCj4gPiA+ID4NCj4gPiA+ID4gQnV0IGFzIHNvb24gYXMgeW91IGhhdmUg
dGhlIGNvbmNlcHQgb2YgY2hhbm5lbHMgd2l0aCBhIGNsZWFybHkNCj4gPiBkZWZpbmVkDQo+ID4g
PiA+IGludGVyZmFjZSwgeW91IGhhdmUgYWxtb3N0IGFic3RyYWN0ZWQgaXQgZW5vdWdoIHRvIGdv
IGFsbCB0aGUNCj4gd2F5Lg0KPiA+ID4NCj4gPiA+IFNvbWV0aGluZyBsaWtlIHRoaXMgaXMgd2hh
dCB0aGUgcmVjZW50IFJGQyBwb3N0ZWQgdG8NCj4gPiA+IGxrbWwvbGludXgtYmx1ZXRvb3RoDQo+
ID4gPiBodHRwOi8vd3d3LnNwaW5pY3MubmV0L2xpc3RzL2xpbnV4LWJsdWV0b290aC9tc2cwOTk5
MC5odG1sLA0KPiA+ID4gaXQga2luZGEgbG9va3MgY2x1bXN5DQo+ID4gPiBidXQgd2hhdCBJIGZl
ZWwgaXMgd2Ugc2hvdWxkbid0IHNoeSBhd2F5IGZyb20gbm90IHJlZmVyZW5jaW5nDQo+ID4gPiBC
bHVldG9vdGgsIChvciBtYXkgYmUgdG9tb3Jyb3cgR1BTDQo+ID4gPiB3aXRoIE5NRUEgaGVhZGVy
cykuLi4uDQo+ID4NCj4gPiBUaGUgb25lIGltcG9ydGFudCBnb2FsIGhlcmUgc2hvdWxkIGJlIHRv
IGF2b2lkIGNvZGUgZHVwbGljYXRpb24uDQo+ID4gVXNpbmcgdGhlIGhlYWRlciB0byBnZXQgdGhl
IGRhdGEgc3RydWN0dXJlcyBmcm9tIHNlcGFyYXRlIGNvZGUNCj4gPiBtZWFucyB5b3UgbmVlZCB0
byBoYXZlIHNpbWlsYXIgcGFyc2luZyBmdW5jdGlvbnMgaW4gZWFjaCBvZiB0aGUNCj4gbW9kdWxl
cw0KPiA+IHVzaW5nIGl0LiBOb3Qgc2hhcmluZyB0aGUgaGVhZGVyIHdvdWxkbid0IGhlbHAsIGJl
Y2F1c2UgdGhlbiB5b3UgZW5kDQo+ID4gdXAgZHVwbGljYXRpbmcgZXZlbiBtb3JlLiBUaGUgcmVh
bCBzb2x1dGlvbiwgc3BlYWtpbmcgdmVyeSBicm9hZGx5LA0KPiA+IG11c3QgYmUgdG8gaGF2ZSBh
IGdlbmVyYWwgbW9kdWxlIHRoYXQgZGVhbHMgd2l0aCB3aGF0ZXZlciB0aGUgbW9yZQ0KPiA+IHNw
ZWNpZmljIG1vZHVsZXMgaGF2ZSBpbiBjb21tb24sIGFuZCBoYXZlIGEgaGVhZGVyIGZpbGUgdGhh
dCBkZWZpbmVzDQo+ID4gdGhlIGludGVyZmFjZSB0byBpdC4NCj4gPg0KPiANCj4gSW4gZ2VuZXJh
bCBJIGFncmVlIHdpdGggeW91LCBidXQgdGhlcmUgYXJlIHNvbWUgcHJvYmxlbXMgaGVyZS4NCj4g
VGhlIG1vc3QgdXNlZCBCVCBIQ0kgZXZlbnRzIGFyZSBDb21tYW5kIFN0YXR1cyBhbmQgQ29tbWFu
ZCBDb21wbGV0ZS4NCj4gQ29tbWFuZCBTdGF0dXMgY291bGQgYmUgcGFyc2VkIGNvbXBsZXRlbHkg
aW4gYSBnb29kIHdheSAocmV0cmlldmluZyBvcA0KPiBjb2RlLCBuYnIgb2YgY29tbWFuZHMgYWxs
b3dlZCwgYW5kIHN0YXR1cyBvZiBjb21tYW5kIHNlbnQpLg0KPiBDb21tYW5kIENvbXBsZXRlIGlz
IGhvd2V2ZXIgcXVpdGUgY29tcGxleCBzaW5jZSB0aGUgcmV0dXJuZWQgZGF0YSB3aWxsDQo+IGRp
ZmZlciBkZXBlbmRpbmcgb24gY29tbWFuZCBzZW50LiBPcCBjb2RlIGFuZCBuYnIgb2YgY29tbWFu
ZHMgYWxsb3dlZA0KPiBjYW4gYmUgcmV0cmlldmVkIGJ1dCBldmVyeXRoaW5nIGVsc2UgaGF2ZSB0
byBiZSBleHRyYWN0ZWQgZGlmZmVyZW50bHkNCj4gZGVwZW5kaW5nIG9uIGNvbW1hbmQuIFRoaXMg
bWVhbnMgdGhhdCB0aGVyZSBpcyBub3QgbXVjaCB0aGF0IHdpbGwgYmUNCj4gc2F2ZWQgaGVyZS4g
QnV0IG1heWJlIHdlIGNvdWxkIGV4dHJhY3Qgc29tZSBwYXJzaW5nIGludG8gY29tbW9uDQo+IGZ1
bmN0aW9ucywgYnV0IEkgZG9uJ3QgdGhpbmsgeW91IHdvdWxkIGdhaW4gdGhhdCBtdWNoLg0KPiBN
b3Jlb3ZlciwgdGhpcyB3b3VsZCBsZWFkIHRvIGEgbWFqb3IgcmVpbXBsZW1lbnRhdGlvbiBvZiB0
aGUgaGNpX2NvcmUuYw0KPiBhbmQgcmVsYXRlZCBmaWxlcywgc2luY2UgdGhleSBkbyBub3QgdXNl
IGFueSBleHBvcnRlZCBjb21tb24gZnVuY3Rpb25zDQo+IGFzIGl0IGlzIHRvZGF5LiBJIGRvIG5v
dCBrbm93IGlmIHRoZXkgKEJsdWVaIGNvbW11bml0eSkgd291bGQgd2FudCB0aGlzDQo+IGFuZCBJ
IGRvIG5vdCB0aGluayB0aGF0IHRoYXQgc2hvdWxkIGJlIHBhcnQgb2YgdGhlIENHMjkwMCBkcml2
ZXIgdG8gZG8uDQo+IEl0IHNob3VsZCBpbiB0aGF0IGNhc2UgYmUgZG9uZSBzZXBhcmF0ZWx5Lg0K
PiANCj4gPiA+ID4+IEFsc28gbm90ZSB0aGF0IHRoZSBjb21tb24taGNpLW1vZHVsZSBpcyBvbmx5
IHJlYWxseSB1c2VkIHVudGlsDQo+IHRoZQ0KPiA+IGNvbm5lY3RlZA0KPiA+ID4gPj4gY2hpcCBo
YXMgYmVlbiBkZXRlY3RlZC4gVGhlIGNoaXAgaGFuZGxlciB3aWxsIHRoZW4gc2V0IHRoZQ0KPiA+
IGNhbGxiYWNrIGZ1bmN0aW9ucw0KPiA+ID4gPj4gc28gYWN0dWFsIGRhdGEgdHJhbnNtaXNzaW9u
cyBuZXZlciBwYXNzIHRoZSBjb21tb24taGNpLW1vZHVsZS4NCj4gPiBUaGV5IGdvIGRpcmVjdGx5
DQo+ID4gPiA+PiBmcm9tIHRyYW5zcG9ydCB0byBjaGlwIGhhbmRsZXIuIFRoYXQgaXMgbm90IHJl
YWxseSBzaG93biBpbiB0aGUNCj4gPiBwaWN0dXJlIGFib3ZlLg0KPiA+ID4gPj4gSnVzdCBpbWFn
aW5lIHRoYXQgY29tbW9uLWhjaS1tb2R1bGUgaXMgcmVtb3ZlZCBhZnRlciBhIGNoaXAgaGFzDQo+
ID4gYmVlbiBjb25uZWN0ZWQNCj4gPiA+ID4+IGFuZCBhIGNoaXAgaGFuZGxlciBoYXMgYmVlbiBk
ZXRlcm1pbmVkLg0KPiA+ID4gPj4NCj4gPiA+ID4+IEkgaG9wZSBJIGhhdmVuJ3QgbWlzdW5kZXJz
dG9vZCB5b3VyIHF1ZXN0aW9uLiBJIGRvIG5vdCBrbm93IG11Y2gNCj4gPiBhYm91dCB0aGUgSTJD
DQo+ID4gPiA+PiBzeXN0ZW0sIGJ1dCBJIHRyaWVkIHRvIHVuZGVyc3RhbmQgeW91ciBxdWVzdGlv
biBhcyBiZXN0IGFzIEkNCj4gPiBjb3VsZC4NCj4gPiA+ID4NCj4gPiA+ID4gSSB0aGluayB0aGVy
ZSBpcyBhIGRpc2Nvbm5lY3Qgd2hlbiB0YWxraW5nIGFib3V0IGhpZXJhcmNoaWVzLCBhcw0KPiBp
dA0KPiA+IGNhbiBiZSBhcHBsaWVkDQo+ID4gPiA+IHRvIGRpZmZlcmVudCBhcmVhczoNCj4gPiA+
ID4NCj4gPiA+ID4gKiBtb2R1bGUgZGVwZW5kZW5jaWVzDQo+ID4gPiA+ICogZGV2aWNlIGRldGVj
dGlvbg0KPiA+ID4gPiAqIHN5c2ZzIG9iamVjdCBoaWVyYXJjaHkNCj4gPiA+ID4gKiBkYXRhIGZs
b3cNCj4gPiA+DQo+ID4gPiBtb2R1bGUgZGVwZW5kZWN5LXdpc2UgSSBhZ3JlZSwNCj4gPiA+IEkg
d291bGQgd2FudCBGTS1WNEwyIHdpdGhvdXQgQlQgb3IgSSBtaWdodCB3YW50IEdQUyBzaW1wbGlz
dGljIGNoYXINCj4gPiA+IGRyaXZlciB3aXRob3V0IEJUIG9yIEZNIFY0TDIuDQo+ID4NCj4gPiBC
dXQgeW91J2Qgc3RpbGwgbmVlZCB0byBoYXZlIGFuIEhDSSBtb2R1bGUuIEkgYmVsaWV2ZSByaWdo
dCBub3csIHRoZQ0KPiA+IGhjaSBtb2R1bGUgZGVwZW5kcyBvbiB0aGUgYmx1ZXRvb3RoIG1vZHVs
ZSwgYW5kIHlvdSBhcmUgcmlnaHQgdGhhdA0KPiB0aGlzDQo+ID4gaXMgYW4gdW5kZXNpcmFibGUg
ZGVwZW5kZW5jeSB0byBoYXZlIGZvciBhIEdQUyBtb2R1bGUuIEhvd2V2ZXIsIHRoZQ0KPiA+IHNv
bHV0aW9uDQo+ID4gdG8gdGhpcyBzaG91bGQgbm90IGJlIHRvIG1ha2UgR1BTIGluZGVwZW5kZW50
IG9mIEhDSSwgYnV0IHRvIG1ha2UNCj4gcGFydHMNCj4gPiBvZiBIQ0kgaW5kZXBlbmRlbnQgb2Yg
Ymx1ZXRvb3RoLg0KPiA+DQo+IA0KPiBGb3IgdGhlIENHMjkwMCB0aGF0IGlzIG5vdCBwb3NzaWJs
ZS4gRXZlbiBpZiB5b3UgYXJlIHJ1bm5pbmcganVzdCBHUFMNCj4geW91IHN0aWxsIG11c3QgZG93
bmxvYWQgcGF0Y2hlcyBhbmQgc2V0dGluZ3MgYW5kIHRoYXQgaXMgZG9uZSB0aHJvdWdoDQo+IEhD
SSBjb21tYW5kcyBvdmVyIHRoZSBCbHVldG9vdGggY29tbWFuZCBpbnRlcmZhY2UuIFdlIGFsc28g
dXNlDQo+IEJsdWV0b290aCBjb21tYW5kcyB0byBpZGVudGlmeSB0aGUgY29udHJvbGxlci4NCj4g
DQo+ID4gPiBkZXZpY2UgZGV0ZWN0aW9uIHdpc2UsIEl0IGlzIGEgcHJvYmxlbSwgdGhlcmUgaXMg
bm90ICJfcHJvYmUiDQo+ID4gPiBtZWNoYW5pc20gZm9yIFVBUlQgYmFzZWQgdHJhbnNwb3J0IGFz
IGl0IGlzDQo+ID4gPiB1bmRlcnN0YW5kYWJsZSwgYW5kIHByZXR0eSBtdWNoIHRoZSBkcml2ZXIg
d291bGQgZW5kIHVwIGJlaW5nDQo+ID4gcGxhdGZvcm0NCj4gPiA+IGRldmljZSBkcml2ZXIgLi4u
DQo+ID4NCj4gPiBUaGUgcGxhdGZvcm0gZGV2aWNlIGlzIGp1c3QgdGhlIGxvd2VzdCBsZXZlbCBp
biB0aGUgZGV2aWNlIGhpZXJhcmNoeS4NCj4gPg0KPiA+IElmIGVhY2ggSENJIGNoYW5uZWwgaXMg
YSBkZXZpY2UsIHlvdSBjYW4gc3RhY2sgYnQsIGdwcywgYXVkaW8sIC4uLg0KPiA+IGRldmljZXMg
YWxsIG9uIHRvcCBvZiB0aGUgSENJIGRldmljZSwgd2hpY2ggaXMgZWl0aGVyIHN0YWNrZWQgb24g
dG9wDQo+ID4gb2YgYSBzZXJpYWwgcG9ydCBvciBpbiBzb21lIG90aGVyIHdheSBjb25uZWN0ZWQg
dG8gdGhlIHVuZGVyeWluZw0KPiA+IHRyYW5zcG9ydC4NCj4gPg0KPiA+IEluIHRoaXMgY2FzZSwg
dGhlIGNoYW5uZWxzIHRoZW1zZWx2ZXMgYXJlIG5vdCBwbGF0Zm9ybSBkZXZpY2VzLCBidXQNCj4g
PiB3b3VsZCBnZXQgYSBuZXcgYnVzIGZvciB0aGVtLiBUaGF0IGJ1cyBpcyBwb3B1bGF0ZWQgYnkg
YSBkcml2ZXIgdGhhdA0KPiA+IG93bnMgdGhlIEhDSSBhbmQgdGhhdCBrbm93cyAoZS5nLiBmcm9t
IGl0cyBwbGF0Zm9ybSBkYXRhLCBoYXJkd2FyZQ0KPiA+IHJlZ2lzdGVycywgdXNlciBjb25maWcg
b3IgZGV2aWNlIHRyZWUpIHdoYXQgY2hhbm5lbHMgYXJlIHByZXNlbnQuDQo+ID4NCj4gPiA+IGRh
dGEgZmxvdyBpcyB3aGVyZSBJIGd1ZXNzIHRoZSBhYnN0cmFjdGlvbiBoYXMgdG8gbGllIGluLCBm
b3INCj4gPiA+IGRpZmZlcmVudCB2ZW5kb3JzLi4uDQo+ID4NCj4gPiBJIGRvbid0IGtub3cgd2hh
dCB5b3UgbWVhbiB3aXRoIHRoYXQuIE15IHBvaW50IHdhcyB0aGF0IHdlIG5lZWQgdG8NCj4gPiBj
b25zaWRlciBhbGwgdGhlIGhpZXJhcmNoaWVzLCBhbmQgdGhhdCB0aGV5IG1pZ2h0IGJlIGRpZmZl
cmVudC4NCj4gPg0KPiA+IAlBcm5kDQo+IA0KPiAvUC1HDQoNCg==

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 00/11] mfd and bluetooth: Add CG2900 suppor
  2011-02-24 14:16                   ` Par-Gunnar HJALMDAHL
@ 2011-02-25 17:08                     ` Linus Walleij
  -1 siblings, 0 replies; 30+ messages in thread
From: Linus Walleij @ 2011-02-25 17:08 UTC (permalink / raw
  To: Par-Gunnar HJALMDAHL, Samuel Ortiz
  Cc: Arnd Bergmann, Pavan Savoy, Vitaly Wool, Alan Cox,
	Marcel Holtmann, linux-kernel@vger.kernel.org,
	linux-bluetooth@vger.kernel.org, Lukasz Rymanowski,
	Par-Gunnar Hjalmdahl, Lee Jones

2011/2/24 Par-Gunnar HJALMDAHL <par-gunnar.p.hjalmdahl@stericsson.com>:

> 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 think I have provided
Acked-by: Linus Walleij <linus.walleij@linaro.org>
(or the earlier ST-Ericsson address, which is equal) for these patches.
Else I do so now.

FWIW: I think those who want another architectural solution
can propose a refactoring patch any day they like, and if it's
recieved like "ah, that's better" ACK from Pär-Gunnar et al, then
it's no big deal.

This makes the hardware work with 2.6.39 which is really most
important IMO, Sam can you merge this as it stands?

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 00/11] mfd and bluetooth: Add CG2900 suppor
@ 2011-02-25 17:08                     ` Linus Walleij
  0 siblings, 0 replies; 30+ messages in thread
From: Linus Walleij @ 2011-02-25 17:08 UTC (permalink / raw
  To: Par-Gunnar HJALMDAHL, Samuel Ortiz
  Cc: Arnd Bergmann, Pavan Savoy, Vitaly Wool, Alan Cox,
	Marcel Holtmann, linux-kernel@vger.kernel.org,
	linux-bluetooth@vger.kernel.org, Lukasz Rymanowski,
	Par-Gunnar Hjalmdahl, Lee Jones

2011/2/24 Par-Gunnar HJALMDAHL <par-gunnar.p.hjalmdahl@stericsson.com>:

> I sent the mail below one month ago, but have still not received any answ=
ers or further comments.
> Has anyone of you who made comments on the CG2900 patches earlier, any mo=
re comments or questions?

I think I have provided
Acked-by: Linus Walleij <linus.walleij@linaro.org>
(or the earlier ST-Ericsson address, which is equal) for these patches.
Else I do so now.

FWIW: I think those who want another architectural solution
can propose a refactoring patch any day they like, and if it's
recieved like "ah, that's better" ACK from P=E4r-Gunnar et al, then
it's no big deal.

This makes the hardware work with 2.6.39 which is really most
important IMO, Sam can you merge this as it stands?

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 00/11] mfd and bluetooth: Add CG2900 suppor
  2011-02-25 17:08                     ` Linus Walleij
  (?)
@ 2011-02-25 17:55                     ` Vitaly Wool
  2011-02-28 22:43                       ` Linus Walleij
  -1 siblings, 1 reply; 30+ messages in thread
From: Vitaly Wool @ 2011-02-25 17:55 UTC (permalink / raw
  To: Linus Walleij
  Cc: Par-Gunnar HJALMDAHL, Samuel Ortiz, Marcel Holtmann, Lee Jones,
	Par-Gunnar Hjalmdahl, Alan Cox, Arnd Bergmann,
	linux-bluetooth@vger.kernel.org, linux-kernel@vger.kernel.org,
	Lukasz Rymanowski, Pavan Savoy

[-- Attachment #1: Type: text/plain, Size: 1312 bytes --]

Hi,

Sorry to say, but I have to disagree. This patchset represents
overcomplicated and unclear model, which may be misleading for developers
implementing similar solution for another platform.

Making hardware work is, if course, a valid point, but porting a windows
driver would do that as well, and it's not the reason to pull windows
drivers' ports into mainline.

Thanks,
   Vitaly
Den 25 feb 2011 18:08 skrev "Linus Walleij" <linus.walleij@linaro.org>:
> 2011/2/24 Par-Gunnar HJALMDAHL <par-gunnar.p.hjalmdahl@stericsson.com>:
>
>> 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 think I have provided
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> (or the earlier ST-Ericsson address, which is equal) for these patches.
> Else I do so now.
>
> FWIW: I think those who want another architectural solution
> can propose a refactoring patch any day they like, and if it's
> recieved like "ah, that's better" ACK from Pär-Gunnar et al, then
> it's no big deal.
>
> This makes the hardware work with 2.6.39 which is really most
> important IMO, Sam can you merge this as it stands?
>
> Yours,
> Linus Walleij

[-- Attachment #2: Type: text/html, Size: 1715 bytes --]

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 00/11] mfd and bluetooth: Add CG2900 suppor
  2011-01-17 15:32                   ` Par-Gunnar HJALMDAHL
  (?)
@ 2011-02-28 15:01                   ` Arnd Bergmann
  -1 siblings, 0 replies; 30+ messages in thread
From: Arnd Bergmann @ 2011-02-28 15:01 UTC (permalink / raw
  To: Par-Gunnar HJALMDAHL
  Cc: Pavan Savoy, Vitaly Wool, Linus Walleij, Alan Cox, Samuel Ortiz,
	Marcel Holtmann, linux-kernel@vger.kernel.org,
	linux-bluetooth@vger.kernel.org, Lukasz Rymanowski,
	Par-Gunnar Hjalmdahl

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


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 00/11] mfd and bluetooth: Add CG2900 suppor
  2011-02-25 17:55                     ` Vitaly Wool
@ 2011-02-28 22:43                       ` Linus Walleij
  2011-03-01 14:02                           ` Par-Gunnar HJALMDAHL
  0 siblings, 1 reply; 30+ messages in thread
From: Linus Walleij @ 2011-02-28 22:43 UTC (permalink / raw
  To: Vitaly Wool, Par-Gunnar Hjalmdahl
  Cc: Par-Gunnar HJALMDAHL, Samuel Ortiz, Marcel Holtmann, Lee Jones,
	Alan Cox, Arnd Bergmann, linux-bluetooth@vger.kernel.org,
	linux-kernel@vger.kernel.org, Lukasz Rymanowski, Pavan Savoy

On Fri, Feb 25, 2011 at 6:55 PM, Vitaly Wool <vitalywool@gmail.com> wrote:

> Making hardware work is, if course, a valid point, but porting a windows
> driver would do that as well, and it's not the reason to pull windows
> drivers' ports into mainline.

Yeah hm, that sounds like it has "drivers/staging" printed all over it then,
so either we solve these issues or we need to put it into staging so we
can get help from others in fixing it up (we have a similar situation with
the U8500 graphics stuff).

P-G do you think it would be proper to take this into staging for the
moment, or do you prefer to drive it outside the mainline kernel tree until
it lands in the proper place(s)?

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 30+ messages in thread

* RE: [PATCH 00/11] mfd and bluetooth: Add CG2900 suppor
  2011-02-28 22:43                       ` Linus Walleij
@ 2011-03-01 14:02                           ` Par-Gunnar HJALMDAHL
  0 siblings, 0 replies; 30+ messages in thread
From: Par-Gunnar HJALMDAHL @ 2011-03-01 14:02 UTC (permalink / raw
  To: Linus Walleij, Vitaly Wool, Par-Gunnar Hjalmdahl
  Cc: Samuel Ortiz, Marcel Holtmann, Lee Jones, Alan Cox, Arnd Bergmann,
	linux-bluetooth@vger.kernel.org, linux-kernel@vger.kernel.org,
	Lukasz Rymanowski, Pavan Savoy

As you say, I think it could be good to put it into staging so we have a driver in place and then we can continue discussing the architecture from there.

/P-G

> -----Original Message-----
> From: Linus Walleij [mailto:linus.walleij@linaro.org]
> Sent: den 28 februari 2011 23:44
> To: Vitaly Wool; Par-Gunnar Hjalmdahl
> Cc: Par-Gunnar HJALMDAHL; Samuel Ortiz; Marcel Holtmann; Lee Jones;
> Alan Cox; Arnd Bergmann; linux-bluetooth@vger.kernel.org; linux-
> kernel@vger.kernel.org; Lukasz Rymanowski; Pavan Savoy
> Subject: Re: [PATCH 00/11] mfd and bluetooth: Add CG2900 suppor
> 
> On Fri, Feb 25, 2011 at 6:55 PM, Vitaly Wool <vitalywool@gmail.com>
> wrote:
> 
> > Making hardware work is, if course, a valid point, but porting a
> windows
> > driver would do that as well, and it's not the reason to pull windows
> > drivers' ports into mainline.
> 
> Yeah hm, that sounds like it has "drivers/staging" printed all over it
> then,
> so either we solve these issues or we need to put it into staging so we
> can get help from others in fixing it up (we have a similar situation
> with
> the U8500 graphics stuff).
> 
> P-G do you think it would be proper to take this into staging for the
> moment, or do you prefer to drive it outside the mainline kernel tree
> until
> it lands in the proper place(s)?
> 
> Yours,
> Linus Walleij

^ permalink raw reply	[flat|nested] 30+ messages in thread

* RE: [PATCH 00/11] mfd and bluetooth: Add CG2900 suppor
@ 2011-03-01 14:02                           ` Par-Gunnar HJALMDAHL
  0 siblings, 0 replies; 30+ messages in thread
From: Par-Gunnar HJALMDAHL @ 2011-03-01 14:02 UTC (permalink / raw
  To: Linus Walleij, Vitaly Wool, Par-Gunnar Hjalmdahl
  Cc: Samuel Ortiz, Marcel Holtmann, Lee Jones, Alan Cox, Arnd Bergmann,
	linux-bluetooth@vger.kernel.org, linux-kernel@vger.kernel.org,
	Lukasz Rymanowski, Pavan Savoy

As you say, I think it could be good to put it into staging so we have a dr=
iver in place and then we can continue discussing the architecture from the=
re.

/P-G

> -----Original Message-----
> From: Linus Walleij [mailto:linus.walleij@linaro.org]
> Sent: den 28 februari 2011 23:44
> To: Vitaly Wool; Par-Gunnar Hjalmdahl
> Cc: Par-Gunnar HJALMDAHL; Samuel Ortiz; Marcel Holtmann; Lee Jones;
> Alan Cox; Arnd Bergmann; linux-bluetooth@vger.kernel.org; linux-
> kernel@vger.kernel.org; Lukasz Rymanowski; Pavan Savoy
> Subject: Re: [PATCH 00/11] mfd and bluetooth: Add CG2900 suppor
>=20
> On Fri, Feb 25, 2011 at 6:55 PM, Vitaly Wool <vitalywool@gmail.com>
> wrote:
>=20
> > Making hardware work is, if course, a valid point, but porting a
> windows
> > driver would do that as well, and it's not the reason to pull windows
> > drivers' ports into mainline.
>=20
> Yeah hm, that sounds like it has "drivers/staging" printed all over it
> then,
> so either we solve these issues or we need to put it into staging so we
> can get help from others in fixing it up (we have a similar situation
> with
> the U8500 graphics stuff).
>=20
> P-G do you think it would be proper to take this into staging for the
> moment, or do you prefer to drive it outside the mainline kernel tree
> until
> it lands in the proper place(s)?
>=20
> Yours,
> Linus Walleij

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 00/11] mfd and bluetooth: Add CG2900 suppor
  2011-03-01 14:02                           ` Par-Gunnar HJALMDAHL
  (?)
@ 2011-03-01 14:23                           ` Arnd Bergmann
  -1 siblings, 0 replies; 30+ messages in thread
From: Arnd Bergmann @ 2011-03-01 14:23 UTC (permalink / raw
  To: Par-Gunnar HJALMDAHL
  Cc: Linus Walleij, Vitaly Wool, Par-Gunnar Hjalmdahl, Samuel Ortiz,
	Marcel Holtmann, Lee Jones, Alan Cox,
	linux-bluetooth@vger.kernel.org, linux-kernel@vger.kernel.org,
	Lukasz Rymanowski, Pavan Savoy

On Tuesday 01 March 2011, Par-Gunnar HJALMDAHL wrote:
> As you say, I think it could be good to put it into staging so we have
> a driver in place and then we can continue discussing the architecture from there.

Yes, makes sense.

	Arnd

^ permalink raw reply	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2011-03-01 14:24 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-12-17 11:20 [PATCH 00/11] mfd and bluetooth: Add CG2900 support Par-Gunnar Hjalmdahl
2010-12-17 12:11 ` Vitaly Wool
2010-12-17 12:11   ` Vitaly Wool
2010-12-17 12:43   ` Par-Gunnar HJALMDAHL
2010-12-17 12:43     ` Par-Gunnar HJALMDAHL
2010-12-19 21:23 ` Linus Walleij
2010-12-19 22:57   ` Vitaly Wool
2010-12-20  9:15     ` Par-Gunnar HJALMDAHL
2010-12-20  9:15       ` Par-Gunnar HJALMDAHL
2010-12-23 10:48       ` Pavan Savoy
2010-12-23 10:48         ` Pavan Savoy
2011-01-05 12:56         ` Par-Gunnar HJALMDAHL
2011-01-05 12:56           ` Par-Gunnar HJALMDAHL
2011-01-06 18:46           ` Arnd Bergmann
2011-01-09 18:12             ` Pavan Savoy
2011-01-09 18:12               ` Pavan Savoy
2011-01-09 18:48               ` Vitaly Wool
2011-01-09 18:55               ` [PATCH 00/11] mfd and bluetooth: Add CG2900 suppor Arnd Bergmann
2011-01-17 15:32                 ` Par-Gunnar HJALMDAHL
2011-01-17 15:32                   ` Par-Gunnar HJALMDAHL
2011-02-28 15:01                   ` Arnd Bergmann
2011-02-24 14:16                 ` Par-Gunnar HJALMDAHL
2011-02-24 14:16                   ` Par-Gunnar HJALMDAHL
2011-02-25 17:08                   ` Linus Walleij
2011-02-25 17:08                     ` Linus Walleij
2011-02-25 17:55                     ` Vitaly Wool
2011-02-28 22:43                       ` Linus Walleij
2011-03-01 14:02                         ` Par-Gunnar HJALMDAHL
2011-03-01 14:02                           ` Par-Gunnar HJALMDAHL
2011-03-01 14:23                           ` Arnd Bergmann

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.