* RE: [PATCH v5 4/4] Sim Access Profile test scripts
From: Waldemar.Rymarkiewicz @ 2011-03-24 9:40 UTC (permalink / raw)
To: johan.hedberg; +Cc: linux-bluetooth
In-Reply-To: <20110323130239.GB20755@jh-x301>
Hi Johan,
>We don't use the .py extension for any other python scripts in
>the tree so please don't use it for your scripts either.
>You'll probably want to name the scripts a little bit more
>descriptively than "sap" and "test-sap". Also remember to
>mention them to Makefile.tools so they are part of "make dist"
>(i.e. add them to EXTRA_DIST).
>
In fact, I missed EXTRA_DIST. Will fix this then.
I will remove .py extensions as well. I understand that the dbusdef.py is an exception?
I could propose test/sap-client and test/test-sap or test/test-sap-server . It seems more saner I think. Doesn't it?
Waldek
^ permalink raw reply
* RE: [PATCH v5 3/4] Sim Access Profile dummy driver
From: Waldemar.Rymarkiewicz @ 2011-03-24 9:32 UTC (permalink / raw)
To: johan.hedberg; +Cc: linux-bluetooth
In-Reply-To: <20110323130000.GA20755@jh-x301>
Johan,
>The first three patches have been pushed upstream. Thanks.
>However, I hope you do pay attention the refactoring
>suggestions that were made and keep a look-out for
>possibilities for simplifying the code.
>
Thanks.
Yes, I will see what I can refactor there.
Waldek
^ permalink raw reply
* Re: [PATCH 1/5] Inline ATT dump functions
From: Johan Hedberg @ 2011-03-24 9:24 UTC (permalink / raw)
To: Andre Dieb Martins; +Cc: linux-bluetooth
In-Reply-To: <1300891601-11412-1-git-send-email-andre.dieb@signove.com>
Hi André,
On Wed, Mar 23, 2011, Andre Dieb Martins wrote:
> ---
> parser/att.c | 32 ++++++++++++++++----------------
> 1 files changed, 16 insertions(+), 16 deletions(-)
Do you have some measurements that show that inlining actually has a
noticable effect? You're also missing the justification for this change
in the commit message.
Johan
^ permalink raw reply
* Re: [PATCH] TODO: Add item related to authorization and authentication
From: Johan Hedberg @ 2011-03-24 9:21 UTC (permalink / raw)
To: Claudio Takahasi; +Cc: linux-bluetooth
In-Reply-To: <1300909212-15709-2-git-send-email-claudio.takahasi@openbossa.org>
Hi Claudio,
On Wed, Mar 23, 2011, Claudio Takahasi wrote:
> ---
> TODO | 9 +++++++++
> 1 files changed, 9 insertions(+), 0 deletions(-)
Applied. Thanks.
Johan
^ permalink raw reply
* Re: [PATCH] Return an error if the attribute requires authorization
From: Johan Hedberg @ 2011-03-24 9:20 UTC (permalink / raw)
To: Claudio Takahasi; +Cc: linux-bluetooth
In-Reply-To: <1300909212-15709-1-git-send-email-claudio.takahasi@openbossa.org>
Hi Claudio,
On Wed, Mar 23, 2011, Claudio Takahasi wrote:
> If an attribute requires authorization, Insuficient Authorization
> will be returned by the attribute server until the Agent supports
> a method to authorize attribute access.
> ---
> src/attrib-server.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/src/attrib-server.c b/src/attrib-server.c
> index 9543da6..dc05d7e 100644
> --- a/src/attrib-server.c
> +++ b/src/attrib-server.c
> @@ -173,6 +173,8 @@ static uint8_t att_check_reqs(struct gatt_channel *channel, uint8_t opcode,
> channel->encrypted = g_attrib_is_encrypted(channel->attrib);
> if (reqs == ATT_AUTHENTICATION && !channel->encrypted)
> return ATT_ECODE_INSUFF_AUTHEN;
> + else if (reqs == ATT_AUTHORIZATION)
> + return ATT_ECODE_INSUFF_AUTHO;
>
> switch (opcode) {
> case ATT_OP_READ_BY_GROUP_REQ:
Pushed upstream. Thanks.
Btw, I'm not really a fan of those "authen" and "autho" short versions.
Since you're already spelling the words out in other defines (e.g.
ATT_AUTHENTICATION) I suppose it'd make sense to do the same for the
error codes as well.
Johan
^ permalink raw reply
* Re: [PATCH] Fix crash while exiting when endpoint has a a2dp stream
From: Johan Hedberg @ 2011-03-24 9:17 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <1300895013-32010-1-git-send-email-luiz.dentz@gmail.com>
Hi Luiz,
On Wed, Mar 23, 2011, Luiz Augusto von Dentz wrote:
> After releasing the endpoint should not be used anymore.
> ---
> audio/a2dp.c | 4 +++-
> 1 files changed, 3 insertions(+), 1 deletions(-)
This one has also been pushed upstream. Thanks.
Johan
^ permalink raw reply
* Re: [PATCH] telephony-ofono: fix handling of telephony_key_press_req
From: Johan Hedberg @ 2011-03-24 9:16 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <1300894927-31702-1-git-send-email-luiz.dentz@gmail.com>
Hi Luiz,
On Wed, Mar 23, 2011, Luiz Augusto von Dentz wrote:
> ---
> audio/telephony-ofono.c | 10 ++++------
> 1 files changed, 4 insertions(+), 6 deletions(-)
Pushed upstream. Thanks. Btw, don't be afraid to use a bit more verbose
commit messages. I can guess what was wrong with the original code, but
it would be nice to have it explicitly stated in the commit message.
Johan
^ permalink raw reply
* [PATCH] TODO: Add item related to authorization and authentication
From: Claudio Takahasi @ 2011-03-23 19:40 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Claudio Takahasi
In-Reply-To: <1300909212-15709-1-git-send-email-claudio.takahasi@openbossa.org>
---
TODO | 9 +++++++++
1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/TODO b/TODO
index b05cfc0..0e06f99 100644
--- a/TODO
+++ b/TODO
@@ -124,6 +124,15 @@ ATT/GATT
Priority: Medium
Complexity: C1
+- At the moment authentication and authorization is not supported at the
+ same time, read/write requirements in the attribute server needs to
+ be extended. According to Bluetooth Specification a server shall check
+ authentication and authorization requirements before any other check is
+ performed.
+
+ Priority: Medium
+ Complexity: C1
+
- ATT/GATT parsing to hcidump. Partially implemented, missing to fix
multiple advertises in the same event and RSSI.
--
1.7.4.1
^ permalink raw reply related
* [PATCH] Return an error if the attribute requires authorization
From: Claudio Takahasi @ 2011-03-23 19:40 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Claudio Takahasi
If an attribute requires authorization, Insuficient Authorization
will be returned by the attribute server until the Agent supports
a method to authorize attribute access.
---
src/attrib-server.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/src/attrib-server.c b/src/attrib-server.c
index 9543da6..dc05d7e 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -173,6 +173,8 @@ static uint8_t att_check_reqs(struct gatt_channel *channel, uint8_t opcode,
channel->encrypted = g_attrib_is_encrypted(channel->attrib);
if (reqs == ATT_AUTHENTICATION && !channel->encrypted)
return ATT_ECODE_INSUFF_AUTHEN;
+ else if (reqs == ATT_AUTHORIZATION)
+ return ATT_ECODE_INSUFF_AUTHO;
switch (opcode) {
case ATT_OP_READ_BY_GROUP_REQ:
--
1.7.4.1
^ permalink raw reply related
* Re: [RFC] Auto Connections
From: Claudio Takahasi @ 2011-03-23 19:11 UTC (permalink / raw)
To: Arun Kumar SINGH
Cc: Arun K. Singh, BlueZ development, Ville Tervo, Gustavo F. Padovan,
Andre Guedes
In-Reply-To: <AFCDDB4A3EA003429EEF1E7B211FDBBA334C4DE29C@EXDCVYMBSTM005.EQ1STM.local>
Hi Arun,
On Wed, Mar 23, 2011 at 7:37 AM, Arun Kumar SINGH
<arunkr.singh@stericsson.com> wrote:
> Hi Claudio,
>
>
>>There are four Connection
>>Establishment Procedures:
>>Auto, General,
>>Selective and Direct.
>>CreateDevice/CreatePairedDev
>>ice will use Direct. But
>>after pairing or
>>create device("known"
>>devices) we need a more
>>transparent procedure. I
>>am not saying that we will
>>implement straitly Auto
>>procedure, we will
>>also need to handle the
>>resolvable address, a hybrid
>>approach seems to
>>be more appropriated.
>>
>> We could use Direct
>>procedure,
>>managing(serializing) the
>>connection
>>establishment implementing
>>logic in the kernel or
>>userspace for the
>>"known" devices. However, I
>>prefer to transfer the
>>responsibility to
>>the controller. If I choose
>>connections using whitelist,
>>how the STE
>>controller prioritizes the
>>advertises? Does directed
>>advertises have a
>>higher priority?
>>
>
> Agreed but not sure whether we have a choice to use both a)auto connection via white-lists and b)direct connection ignoring white-lists - at the same time?
> In my opinion for doing either a) or b) you need to set the intitiator filter plicy to either use white-list or ignore it for direct connection. So, if you eventually implement auto-connection, you may not be able to do direct connect at all unless of course if you change the filter policy everytime. STE controller behaves the same way as per controller spec requirements letting the host decide on the filter policy and act suitably.
Not at the same time, controllers don't allow a second LE Create
Connection command if there is a pending command.
My question about your controller was how it decides in the case that
it receives connectable advertises from more than one remote. Does it
use RSSI(included in the advertise) and/or direct advertises has
higher priority?
>
> Coming back to the connection policy debate, we can as well have this as configurable option in bluetoothd to give a choice whether to use auto connection or direct connect policy by default?
>
> Also all the seven steps you need[9.3.5] for auto connection could either be done from user-space itself. And doing same from user-space would make more sense once the connection policy is configurable in bluetoothd?
>
> Please excuse if these sound too wild ...
Your comments are being very useful! No worries.
We have been discussing internally how to implement the connection
management and new facts came up:
1. If we wanna support GATT over BR/EDR, manage automatically
connections in the kernel will be hard to keep a similar approach for
LE and basic rate.
2. Change the server socket to "notify" local host initiated
connections is also breaking the socket API and requires some nasty
code changes in the kernel.
3. There are a lot of LE kernel patches pending(SM, cache and MTU)
After that we decided to start our first shot managing the connection
is the usespace(as you suggested). Hide the resolvable address is
still possible to be implemented in the kernel if we manage the
advertise kernel cache and passive scanning properly in the kernel.
>
>
>>At API level, what is the
>>benefit of exposing this
>>option to the API users?
>>
>>Some profiles define the
>>recommended connection
>>parameters, there is
>>also the Slave Connection
>>Interval Range AD type that
>>may be used to
>>in the connection
>>establishment.
>>
>
>
> I don’t have a strong reason to give this option as an API to profiles but considering that a profile may have more flexibiltiy to set the connection parameters etc while initiating a new connection etc. As if this is done by Auto- Connection policy, you may have to set same connection parameters for all devices --something which be undesirable from profile point of view ...
>
At the moment the values are hardcoded in the kernel. Ville sent last
year a proposal to allow changing the connection parameters. We didn't
decide yet we we will use management interface or extend the socket to
pass the connection parameters.
BR,
Claudio.
>
> Thanks,
> Arun
>
^ permalink raw reply
* Re: android: bluetoothd hung after bluetooth had been turned off
From: Vitaly Wool @ 2011-03-23 18:47 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <AANLkTi=R7kt2tFTpj_RBBbnoPBGNT90K0grshUSJNVnD@mail.gmail.com>
Hi,
On Tue, Mar 22, 2011 at 8:11 PM, Vitaly Wool <vitalywool@gmail.com> wrote:
> Greetings,
>
> I'd got a nasty hang in my Android phone when I turned bluetooth off
> from the UI while the music was playing over bluetooth connection.
as a follow-up on this, it looks like the problem is fairly generic on
one hand but may be specific to Android, on the other.
bluetoothd defines its own signal handler for SIGTERM (sig_term())
which calls g_main_loop_quit(event_loop).
The latter will call g_main_context_wakeup_unlocked() which is exactly
the following:
g_main_context_wakeup_unlocked (GMainContext *context)
{
#ifdef G_THREADS_ENABLED
if (g_thread_supported() && context->poll_waiting)
{
context->poll_waiting = FALSE;
#ifndef G_OS_WIN32
write (context->wake_up_pipe[1], "A", 1);
#else
ReleaseSemaphore (context->wake_up_semaphore, 1, NULL);
#endif
}
#endif
This is meant to make bluetoothd quit waiting on a poll() call in
g_main_context_poll() which is polling on the read descriptor of this
pipe along with all the other descriptors supplied by bluetoothd.
Unfortunately gthreads are not initialized in Android, moreover, this
part of glib code is not even compiled, so g_thread_supported()
returns false and this function is basically no-op in the Android
case.
So if there are no other events for the poll() to quit, we'll get
stuck in poll() forever because the write to wake_up_pipe will never
happen.
Still don't have a good enough idea on how to fix it in the right way though...
~Vitaly
^ permalink raw reply
* Re: [PATCH 2/2] mach-ux500: Add CG2900 devices
From: Linus Walleij @ 2011-03-23 17:55 UTC (permalink / raw)
To: Greg KH, Par-Gunnar Hjalmdahl
Cc: devel, linux-kernel, linux-bluetooth, Pavan Savoy, Vitaly Wool,
Alan Cox, Arnd Bergmann, Marcel Holtmann, Lukasz Rymanowski,
Par-Gunnar Hjalmdahl, Lee Jones
In-Reply-To: <20110323142719.GA17369@suse.de>
2011/3/23 Greg KH <gregkh@suse.de>:
>> =A0arch/arm/mach-ux500/Makefile =A0 =A0 =A0 =A0 | =A0 =A04 +
>> =A0arch/arm/mach-ux500/board-mop500.c =A0 | =A0160 ++++++++++++++++
>> =A0arch/arm/mach-ux500/devices-cg2900.c | =A0349 +++++++++++++++++++++++=
+++++++++++
>> =A0arch/arm/mach-ux500/devices-cg2900.h | =A0 19 ++
>> =A04 files changed, 532 insertions(+), 0 deletions(-)
>> =A0create mode 100644 arch/arm/mach-ux500/devices-cg2900.c
>> =A0create mode 100644 arch/arm/mach-ux500/devices-cg2900.h
>
> As this touches ARM specific code, I need an ack from the ARM maintainer
> before I can take this through the staging tree. =A0Is there any way to
> make it part of your drivers/staging/ submission? =A0There are other
> examples in the staging tree already that do this, why not do that here
> as well?
Hm yeah maybe it's possible if you move devices-cg2900.*
to staging/cg2900, then split off the stuff in board-mop500.c into
some drivers/staging/cg2900/board-mop500-cg2900.c which
adds the device in some proper initcall.
Do you think it can be done P-G? I might have overestimated
the work involved in moving that stuff to /staging.
Yours,
Linus Walleij
^ permalink raw reply
* Re: how to set adapter to master with bluez 4.69?
From: Brad Midgley @ 2011-03-23 16:24 UTC (permalink / raw)
To: Brian J. Murrell; +Cc: Brad Midgley, linux-bluetooth
In-Reply-To: <4D89E589.5010909@interlinx.bc.ca>
Brian,
My suggestion earlier about influencing connection order won't apply I
think because the esco is opened indirectly after some negotiation on
the acl connection.
You could try a different headset. I think it is unusual for it to
behave this way. I haven't played with multiple bt adapters, that
would fix it but there's the possibility for more complications.
--
Brad Midgley
^ permalink raw reply
* Re: [PATCH 2/2] mach-ux500: Add CG2900 devices
From: Arnd Bergmann @ 2011-03-23 16:13 UTC (permalink / raw)
To: Par-Gunnar HJALMDAHL
Cc: Greg Kroah-Hartman, devel@driverdev.osuosl.org, Linus Walleij,
linux-kernel@vger.kernel.org, linux-bluetooth@vger.kernel.org,
Pavan Savoy, Vitaly Wool, Alan Cox, Marcel Holtmann,
Lukasz Rymanowski, Linus WALLEIJ, Par-Gunnar Hjalmdahl, Lee Jones
In-Reply-To: <AFCDDB4A3EA003429EEF1E7B211FDBBA334C4DE937@EXDCVYMBSTM005.EQ1STM.local>
On Wednesday 23 March 2011, Par-Gunnar HJALMDAHL wrote:
> I will see how I can fix this. I'm not 100% how I will solve the
> "asking the hardware" part, but as you say we might be able to do
> this in a better way by doing it from the main staging driver instead.
The easiest way to do this would be to have separate board files for
devices with and without cg2900 and identify the hardware based on the
board number. If all ux500 boards have a cg2900, that would be trivial ;-)
Arnd
^ permalink raw reply
* RE: [PATCH 2/2] mach-ux500: Add CG2900 devices
From: Par-Gunnar HJALMDAHL @ 2011-03-23 16:08 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Greg Kroah-Hartman, devel@driverdev.osuosl.org, Linus Walleij,
linux-kernel@vger.kernel.org, linux-bluetooth@vger.kernel.org,
Pavan Savoy, Vitaly Wool, Alan Cox, Marcel Holtmann,
Lukasz Rymanowski, Linus WALLEIJ, Par-Gunnar Hjalmdahl, Lee Jones
In-Reply-To: <201103231542.28311.arnd@arndb.de>
Hi Arnd,
=20
> > diff --git a/arch/arm/mach-ux500/Makefile b/arch/arm/mach-
> ux500/Makefile
> > index b549a8f..47c92fa 100644
> > --- a/arch/arm/mach-ux500/Makefile
> > +++ b/arch/arm/mach-ux500/Makefile
> > @@ -2,6 +2,9 @@
> > # Makefile for the linux kernel, U8500 machine.
> > #
> >
> > +ccflags-y :=3D \
> > + -Idrivers/staging/cg2900/include
> > +
> > obj-y :=3D clock.o cpu.o devices.o
> devices-common.o \
> > id.o usb.o
>=20
> Could we keep this more self-contained? Just register a
> single device with the necessary resources and let the
> staging driver figure out how to initialize it, rather
> than splitting it between mach-ux500 and drivers/staging.
>=20
I will see what I can do.
> > +#ifdef CONFIG_CG2900
> > +#define CG2900_BT_ENABLE_GPIO 170
> > +#define CG2900_GBF_ENA_RESET_GPIO 171
> > +#define CG2900_BT_CTS_GPIO 0
>=20
> Don't make hardware definitions depending on Kconfig symbols.
> Just describe what the hardware looks like if present, and
> let the board code figure out if it's actually there.
>=20
Will fix.
> > +static struct platform_device ux500_cg2900_device =3D {
> > + .name =3D "cg2900",
> > +};
> > +
> > +#ifdef CONFIG_CG2900_CHIP
> > +static struct platform_device ux500_cg2900_chip_device =3D {
> > + .name =3D "cg2900-chip",
> > + .dev =3D {
> > + .parent =3D &ux500_cg2900_device.dev,
> > + },
> > +};
> > +#endif /* CONFIG_CG2900_CHIP */
> > +
> > +#ifdef CONFIG_STLC2690_CHIP
> > +static struct platform_device ux500_stlc2690_chip_device =3D {
> > + .name =3D "stlc2690-chip",
> > + .dev =3D {
> > + .parent =3D &ux500_cg2900_device.dev,
> > + },
> > +};
> > +#endif /* CONFIG_STLC2690_CHIP */
> > +
> > +#ifdef CONFIG_CG2900_TEST
> > +static struct cg2900_platform_data cg2900_test_platform_data =3D {
> > + .bus =3D HCI_VIRTUAL,
> > + .gpio_sleep =3D cg2900_sleep_gpio,
> > +};
>=20
> Also, don't make the device registration dependent on the Kconfig.
> Make sure that the hardware is there by asking the hardware, then
> register it, even if we don't compile the driver using it.
>=20
> I assume that this would get much simpler if you register everything
> from the .probe function of the main "cg2900" device.
>=20
I will see how I can fix this. I'm not 100% how I will solve the
"asking the hardware" part, but as you say we might be able to do
this in a better way by doing it from the main staging driver instead.
> > diff --git a/arch/arm/mach-ux500/devices-cg2900.c b/arch/arm/mach-
> ux500/devices-cg2900.c
> > new file mode 100644
> > index 0000000..525c871
> > --- /dev/null
> > +++ b/arch/arm/mach-ux500/devices-cg2900.c
>=20
> As far as I can tell, everything in this file can simply become part of
> the
> staging driver. I'm fine with basically anything that compiles going
> into
> drivers/staging, but we should keep the platform code outside of
> staging
> clean of stuff that might have to change as part of the staging
> process.
>=20
> Arnd
I agree that we can probably move at least most of the code, maybe all.
I will check and update.
Thanks,
/P-G
^ permalink raw reply
* [PATCH] Fix crash while exiting when endpoint has a a2dp stream
From: Luiz Augusto von Dentz @ 2011-03-23 15:43 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
After releasing the endpoint should not be used anymore.
---
audio/a2dp.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/audio/a2dp.c b/audio/a2dp.c
index 8595350..88c280a 100644
--- a/audio/a2dp.c
+++ b/audio/a2dp.c
@@ -1517,8 +1517,10 @@ proceed:
static void a2dp_unregister_sep(struct a2dp_sep *sep)
{
- if (sep->endpoint)
+ if (sep->endpoint) {
media_endpoint_release(sep->endpoint);
+ sep->endpoint = NULL;
+ }
avdtp_unregister_sep(sep->lsep);
g_free(sep);
--
1.7.1
^ permalink raw reply related
* [PATCH] telephony-ofono: fix handling of telephony_key_press_req
From: Luiz Augusto von Dentz @ 2011-03-23 15:42 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
---
audio/telephony-ofono.c | 10 ++++------
1 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/audio/telephony-ofono.c b/audio/telephony-ofono.c
index ef4ede7..6f5685b 100644
--- a/audio/telephony-ofono.c
+++ b/audio/telephony-ofono.c
@@ -579,19 +579,17 @@ void telephony_nr_and_ec_req(void *telephony_device, gboolean enable)
void telephony_key_press_req(void *telephony_device, const char *keys)
{
- struct voice_call *active, *waiting;
+ struct voice_call *active, *incoming;
int err;
DBG("telephony-ofono: got key press request for %s", keys);
- waiting = find_vc_with_status(CALL_STATUS_INCOMING);
- if (!waiting)
- waiting = find_vc_with_status(CALL_STATUS_DIALING);
+ incoming = find_vc_with_status(CALL_STATUS_INCOMING);
active = find_vc_with_status(CALL_STATUS_ACTIVE);
- if (waiting)
- err = answer_call(waiting);
+ if (incoming)
+ err = answer_call(incoming);
else if (active)
err = release_call(active);
else
--
1.7.1
^ permalink raw reply related
* RE: [PATCH 1/2] staging: Add ST-Ericsson CG2900 driver
From: Par-Gunnar HJALMDAHL @ 2011-03-23 15:26 UTC (permalink / raw)
To: Randy Dunlap
Cc: Greg Kroah-Hartman, devel@driverdev.osuosl.org, Linus Walleij,
Linus WALLEIJ, Arnd Bergmann, Vitaly Wool, Marcel Holtmann,
linux-kernel@vger.kernel.org, Lukasz Rymanowski,
linux-bluetooth@vger.kernel.org, Pavan Savoy, Lee Jones, Alan Cox,
Par-Gunnar Hjalmdahl
In-Reply-To: <20110323082002.bd60cea1.rdunlap@xenotime.net>
Hi Randy,
> > +
> > +config CG2900_AUDIO
> > + tristate "Support CG2900 audio interface"
> > + depends on CG2900
> > + help
> > + Support for ST-Ericsson CG2900 Connectivity audio
> interface. Gives a
> > + module the ability to setup audio paths within the CG2900
> controller.
> > +
>=20
> Just to be clear: the cg2900_audio driver does not use any current
> kernel
> sound/audio interfaces, right?
>=20
No, it provides an interface to the audio features within the chip.
So it is rather used by other sound/audio drivers, but it never
use any sound/audio interfaces itself.
Thanks,
/P-G
^ permalink raw reply
* Re: [PATCH 1/2] staging: Add ST-Ericsson CG2900 driver
From: Randy Dunlap @ 2011-03-23 15:20 UTC (permalink / raw)
To: Par-Gunnar Hjalmdahl
Cc: Greg Kroah-Hartman, devel, Linus Walleij, Linus Walleij,
Arnd Bergmann, Vitaly Wool, Marcel Holtmann, linux-kernel,
Lukasz Rymanowski, linux-bluetooth, Pavan Savoy, Lee Jones,
Alan Cox, Par-Gunnar Hjalmdahl
In-Reply-To: <1300888771-26437-1-git-send-email-par-gunnar.p.hjalmdahl@stericsson.com>
On Wed, 23 Mar 2011 14:59:31 +0100 Par-Gunnar Hjalmdahl wrote:
> This patch adds support for the ST-Ericsson CG2900
> Connectivity Combo controller (Bluetooth, FM, GPS).
>
> diff --git a/drivers/staging/cg2900/Kconfig b/drivers/staging/cg2900/Kconfig
> new file mode 100644
> index 0000000..d0ba5c7
> --- /dev/null
> +++ b/drivers/staging/cg2900/Kconfig
> @@ -0,0 +1,52 @@
> +#
> +# CG2900
> +#
> +
> +config CG2900
> + tristate "Support ST-Ericsson CG2900 main structure"
> + depends on NET && MFD_SUPPORT
> + help
> + Support for ST-Ericsson CG2900 Connectivity Combo controller main structure.
> + Supports multiple functionalities muxed over a Bluetooth HCI H:4 interface.
> + CG2900 support Bluetooth, FM radio, and GPS.
> +
> +config CG2900_CHIP
> + tristate "Support CG2900 Connectivity controller"
> + depends on CG2900
> + help
> + Support for ST-Ericsson CG2900 Connectivity Controller
> +
> +config STLC2690_CHIP
> + tristate "Support STLC2690 Connectivity controller"
> + depends on CG2900
> + help
> + Support for ST-Ericsson STLC2690 Connectivity Controller
> +
> +config CG2900_UART
> + tristate "Support CG2900 UART transport"
> + depends on CG2900
> + select BT
> + select BT_HCIUART
> + help
> + Support for UART as transport for ST-Ericsson CG2900 Connectivity
> + Controller
> +
> +config CG2900_AUDIO
> + tristate "Support CG2900 audio interface"
> + depends on CG2900
> + help
> + Support for ST-Ericsson CG2900 Connectivity audio interface. Gives a
> + module the ability to setup audio paths within the CG2900 controller.
> +
Just to be clear: the cg2900_audio driver does not use any current kernel
sound/audio interfaces, right?
> +config CG2900_TEST
> + tristate "Support CG2900 Test Char Device"
> + depends on CG2900
> + help
> + Support for ST-Ericsson CG2900 Test Character Device.
> +
> +config BT_CG2900
> + tristate "ST-Ericsson CG2900 Bluetooth driver"
> + depends on CG2900 && BT
> + help
> + Select if ST-Ericsson CG2900 Connectivity controller shall be used as
> + Bluetooth controller for BlueZ.
---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
^ permalink raw reply
* RE: [PATCH 1/2] staging: Add ST-Ericsson CG2900 driver
From: Par-Gunnar HJALMDAHL @ 2011-03-23 15:05 UTC (permalink / raw)
To: Greg KH
Cc: devel@driverdev.osuosl.org, Linus Walleij,
linux-kernel@vger.kernel.org, linux-bluetooth@vger.kernel.org,
Pavan Savoy, Vitaly Wool, Alan Cox, Arnd Bergmann,
Marcel Holtmann, Lukasz Rymanowski, Linus WALLEIJ,
Par-Gunnar Hjalmdahl, Lee Jones
In-Reply-To: <20110323143144.GB17369@suse.de>
Hi Greg,
Thanks for your comments.
> > +
> > + - Decide upon architecture. Some people consider architecture in
> the cg2900
> > + driver to be too complex. We consider it to be not more complex
> than needed.
>=20
> What do you mean by this? It sounds as if you do not consider this a
> valid thing. If so, why list it?
>=20
I've been trying to get this driver into the "normal" driver tree
(Bluetooth and mfd) for the past half year. We have at this point
come to such a standstill that we wanted to get the driver into staging
where we could then continue the architecture discussion.
> > + - Currently the cg2900_uart registers as protocol driver against
> hci_ldisc.c.
> > + There is however some common functionality with hci_h4.c and the
> cg2900 could
> > + therefore register it's vendor specific channels to hci_h4.c, but
> this would
> > + require adding a registration functionality in the hci_h4 file.
>=20
> Putting a "but" makes it sound like this is something you will not do.
> If so, why list it?
>=20
Again, this is due to discussions and putting the driver in staging
will make it easier to get a view of the driver. Making such a change
would also quite heavily affect the Bluetooth driver.
> > + - Some people demand that the cg2900 driver re-use the Bluetooth
> driver to send
> > + and receive BT commands and events. That is however not possible
> with current
> > + BT API and might not be feasible, for example when using FM only
> in
> > + the cg2900 chip.
>=20
> Again, a review comment that you are saying is not valid. Why list it?
>=20
And, again, it was blocking us from getting the driver into the Kernel.
So we have to solve the issue in some way, but it is not clear at this
point exactly how it will be solved (and therefore also not where it
will be solved).
> > + - TI has already delivered a driver for a multi-function chip
> called ti-st.
> > + This driver is currently located in drivers/misc/ti-st/. There
> has however
> > + been criticism raised against design/architecture of the driver.
> There
> > + currently also doesn't seem to be a way to add support for cg2900
> in that
> > + driver even though some people has raised this as an alternative.
>=20
> And again, the same thing.
>=20
> What criticism of that driver? It's now accepted and is working and in
> the tree.
>=20
I will remove this text. There was criticism against the driver in the
mail discussions, but I agree that it should not be stated in this TODO
file.
> My main point here is that this looks like a rant against people who
> have reviewed your code in the past and why you feel you can not
> address
> those complaints. That's not a valid thing for a TODO file at all.
> Please list things that need to be fixed in the driver to get it merged
> into the main tree. As it is, you have a list of things that you say
> you will not do, which is not encouraging at all.
>=20
I'm sorry if it sounds like a rant against people who've reviewed the code.
That was never my intention.
I will see if I can rephrase, but the problem is that nothing has still bee=
n
decided so at this point it is hard to say exactly what shall be fixed.
But as I said, I will try to rewrite it in a better way.
> > diff --git a/drivers/staging/cg2900/bluetooth/Makefile
> b/drivers/staging/cg2900/bluetooth/Makefile
> > new file mode 100644
> > index 0000000..6f4255b
> > --- /dev/null
> > +++ b/drivers/staging/cg2900/bluetooth/Makefile
> > @@ -0,0 +1,9 @@
> > +#
> > +# Makefile for ST-Ericsson CG2900 connectivity combo controller
> > +#
> > +
> > +ccflags-y :=3D \
> > + -Idrivers/staging/cg2900/include
> > +
>=20
> Trailing whitespace, did you run this through checkpatch.pl before
> sending it to me?
>=20
> thanks,
>=20
> greg k-h
I will fix and make certain that I haven't missed anything more.
I have run checkpatch but in the hurry I must have made a last
minute change and forgot to run checkpatch on the last patch version.
/P-G
^ permalink raw reply
* Re: [PATCH 1/2] staging: Add ST-Ericsson CG2900 driver
From: Vitaly Wool @ 2011-03-23 14:53 UTC (permalink / raw)
To: Par-Gunnar Hjalmdahl
Cc: Greg Kroah-Hartman, devel, Linus Walleij, linux-kernel,
linux-bluetooth, Pavan Savoy, Alan Cox, Arnd Bergmann,
Marcel Holtmann, Lukasz Rymanowski, Linus Walleij,
Par-Gunnar Hjalmdahl, Lee Jones
In-Reply-To: <1300888771-26437-1-git-send-email-par-gunnar.p.hjalmdahl@stericsson.com>
On Wed, Mar 23, 2011 at 2:59 PM, Par-Gunnar Hjalmdahl
<par-gunnar.p.hjalmdahl@stericsson.com> wrote:
> diff --git a/drivers/staging/cg2900/TODO b/drivers/staging/cg2900/TODO
> new file mode 100644
> index 0000000..f6fb76a
> --- /dev/null
> +++ b/drivers/staging/cg2900/TODO
> @@ -0,0 +1,21 @@
> +TODO
> +----
> +
> + - Decide upon architecture. Some people consider architecture in the cg=
2900
> + =A0 driver to be too complex. We consider it to be not more complex tha=
n needed.
> +
> + - Currently the cg2900_uart registers as protocol driver against hci_ld=
isc.c.
> + =A0 There is however some common functionality with hci_h4.c and the cg=
2900 could
> + =A0 therefore register it's vendor specific channels to hci_h4.c, but t=
his would
> + =A0 require adding a registration functionality in the hci_h4 file.
> +
> + - Some people demand that the cg2900 driver re-use the Bluetooth driver=
to send
> + =A0 and receive BT commands and events. That is however not possible wi=
th current
> + =A0 BT API and might not be feasible, for example when using FM only in
> + =A0 the cg2900 chip.
> +
> + - TI has already delivered a driver for a multi-function chip called ti=
-st.
> + =A0 This driver is currently located in drivers/misc/ti-st/. There has =
however
> + =A0 been criticism raised against design/architecture of the driver. Th=
ere
> + =A0 currently also doesn't seem to be a way to add support for cg2900 i=
n that
> + =A0 driver even though some people has raised this as an alternative.
As someone who is definitely the part of these demotivating and
unreasonable "some people" I have to say that even though I'm okay
with this driver going into staging, I see a huge gap between the
desire of its creators to have something generic and the ability to do
so. I am absolutely sure that given the widely advertised
peculiarities of this chip, the authors should stop trying to come up
with a generic solution and just present a simplistic MFD driver for
their particular device.
Thanks,
Vitaly
^ permalink raw reply
* [PATCH 5/5] Fix handle formatting for ATT Handle Notify
From: Andre Dieb Martins @ 2011-03-23 14:46 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Andre Dieb Martins
In-Reply-To: <1300891601-11412-1-git-send-email-andre.dieb@signove.com>
---
parser/att.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/parser/att.c b/parser/att.c
index 4141f99..5e589c4 100644
--- a/parser/att.c
+++ b/parser/att.c
@@ -512,7 +512,7 @@ static inline void att_handle_notify_dump(int level, struct frame *frm)
uint16_t handle = btohs(htons(get_u16(frm)));
p_indent(level, frm);
- printf("handle 0x%2.2x\n", handle);
+ printf("handle 0x%4.4x\n", handle);
p_indent(level, frm);
printf("value ");
--
1.7.1
^ permalink raw reply related
* [PATCH 4/5] Add parsing for ATT Signed Write
From: Andre Dieb Martins @ 2011-03-23 14:46 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Andre Dieb Martins
In-Reply-To: <1300891601-11412-1-git-send-email-andre.dieb@signove.com>
---
parser/att.c | 22 ++++++++++++++++++++++
1 files changed, 22 insertions(+), 0 deletions(-)
diff --git a/parser/att.c b/parser/att.c
index b85ff0c..4141f99 100644
--- a/parser/att.c
+++ b/parser/att.c
@@ -488,6 +488,25 @@ static inline void att_write_req_dump(int level, struct frame *frm)
printf("\n");
}
+static inline void att_signed_write_dump(int level, struct frame *frm)
+{
+ uint16_t handle = btohs(htons(get_u16(frm)));
+ int value_len = frm->len - 12; /* handle:2 already accounted, sig: 12 */
+
+ p_indent(level, frm);
+ printf("handle 0x%4.4x value ", handle);
+
+ while (value_len--)
+ printf(" 0x%2.2x", get_u8(frm));
+ printf("\n");
+
+ p_indent(level, frm);
+ printf("auth signature ");
+ while (frm->len > 0)
+ printf(" 0x%2.2x", get_u8(frm));
+ printf("\n");
+}
+
static inline void att_handle_notify_dump(int level, struct frame *frm)
{
uint16_t handle = btohs(htons(get_u16(frm)));
@@ -565,6 +584,9 @@ void att_dump(int level, struct frame *frm)
case ATT_OP_WRITE_CMD:
att_write_req_dump(level + 1, frm);
break;
+ case ATT_OP_SIGNED_WRITE_CMD:
+ att_signed_write_dump(level + 1, frm);
+ break;
case ATT_OP_HANDLE_NOTIFY:
att_handle_notify_dump(level + 1, frm);
break;
--
1.7.1
^ permalink raw reply related
* [PATCH 3/5] Add parsing for ATT Write Command
From: Andre Dieb Martins @ 2011-03-23 14:46 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Andre Dieb Martins
In-Reply-To: <1300891601-11412-1-git-send-email-andre.dieb@signove.com>
---
parser/att.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/parser/att.c b/parser/att.c
index 6a1b1b5..b85ff0c 100644
--- a/parser/att.c
+++ b/parser/att.c
@@ -562,6 +562,7 @@ void att_dump(int level, struct frame *frm)
att_read_by_group_resp_dump(level + 1, frm);
break;
case ATT_OP_WRITE_REQ:
+ case ATT_OP_WRITE_CMD:
att_write_req_dump(level + 1, frm);
break;
case ATT_OP_HANDLE_NOTIFY:
--
1.7.1
^ permalink raw reply related
* [PATCH 2/5] Add parsing for ATT Write Request
From: Andre Dieb Martins @ 2011-03-23 14:46 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Andre Dieb Martins
In-Reply-To: <1300891601-11412-1-git-send-email-andre.dieb@signove.com>
Note we do not need extra parsing for ATT Write Response as it only has one
field (opcode).
---
parser/att.c | 15 +++++++++++++++
1 files changed, 15 insertions(+), 0 deletions(-)
diff --git a/parser/att.c b/parser/att.c
index 6c1d0d8..6a1b1b5 100644
--- a/parser/att.c
+++ b/parser/att.c
@@ -476,6 +476,18 @@ static inline void att_read_by_group_resp_dump(int level, struct frame *frm)
}
}
+static inline void att_write_req_dump(int level, struct frame *frm)
+{
+ uint16_t handle = btohs(htons(get_u16(frm)));
+
+ p_indent(level, frm);
+ printf("handle 0x%4.4x value ", handle);
+
+ while (frm->len > 0)
+ printf(" 0x%2.2x", get_u8(frm));
+ printf("\n");
+}
+
static inline void att_handle_notify_dump(int level, struct frame *frm)
{
uint16_t handle = btohs(htons(get_u16(frm)));
@@ -549,6 +561,9 @@ void att_dump(int level, struct frame *frm)
case ATT_OP_READ_BY_GROUP_RESP:
att_read_by_group_resp_dump(level + 1, frm);
break;
+ case ATT_OP_WRITE_REQ:
+ att_write_req_dump(level + 1, frm);
+ break;
case ATT_OP_HANDLE_NOTIFY:
att_handle_notify_dump(level + 1, frm);
break;
--
1.7.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox