* Re: [PATCH 1/8] Bluetooth: Make sure the L2CAP FCS is only enabled for ERTM or streaming.
From: Marcel Holtmann @ 2010-08-03 16:50 UTC (permalink / raw)
To: Mat Martineau; +Cc: Gustavo F. Padovan, linux-bluetooth, rshaffer
In-Reply-To: <alpine.DEB.2.00.1008030918360.19861@linux-sea-02>
Hi Mat,
> >>>>>>> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> >>>>>>> ---
> >>>>>>> net/bluetooth/l2cap.c | 12 ++++++++----
> >>>>>>> 1 files changed, 8 insertions(+), 4 deletions(-)
> >>>>>>>
> >>>>>>> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> >>>>>>> index 9ba1e8e..aed72f2 100644
> >>>>>>> --- a/net/bluetooth/l2cap.c
> >>>>>>> +++ b/net/bluetooth/l2cap.c
> >>>>>>> @@ -3127,8 +3127,10 @@ static inline int l2cap_config_req(struct l2cap_conn *conn, struct l2cap_cmd_hdr
> >>>>>>> goto unlock;
> >>>>>>>
> >>>>>>> if (l2cap_pi(sk)->conf_state & L2CAP_CONF_INPUT_DONE) {
> >>>>>>> - if (!(l2cap_pi(sk)->conf_state & L2CAP_CONF_NO_FCS_RECV) ||
> >>>>>>> - l2cap_pi(sk)->fcs != L2CAP_FCS_NONE)
> >>>>>>> + if ((l2cap_pi(sk)->mode == L2CAP_MODE_ERTM ||
> >>>>>>> + l2cap_pi(sk)->mode == L2CAP_MODE_STREAMING) &&
> >>>>>>> + (!(l2cap_pi(sk)->conf_state & L2CAP_CONF_NO_FCS_RECV) ||
> >>>>>>> + l2cap_pi(sk)->fcs != L2CAP_FCS_NONE))
> >>>>>>> l2cap_pi(sk)->fcs = L2CAP_FCS_CRC16;
> >>>>>>
> >>>>>> this becomes unreadable and my brain starts to throw a core dump. So it
> >>>>>> clearly needs to be put into a helper inline function.
> >>>>>
> >>>>> Actually we don't need that, since the code that deals with Basic Mode
> >>>>> never check and use the l2cap_pi(sk)->fcs. So we don't care about FCS
> >>>>> value in the Basic Mode.
> >>>>
> >>>> There isn't currently any Basic Mode code that triggers this latent
> >>>> bug, but I have a patch coming up that does require this fix.
> >>>>
> >>>> As it stands, getsockopt() on a connected basic mode socket shows FCS
> >>>> enabled, so this bug is visible from userspace.
> >>>
> >>> can we just fail the setsockopt() when trying to set basic mode and FCS
> >>> off.
> >>
> >> It definitely makes sense to have more validation of L2CAP_OPTIONS
> >> passed to setsockopt().
> >>
> >>> And also in case fallback to basic mode happens, then FCS should be set
> >>> to be enabled. Since for FCS and basic mode we always have to use FCS.
> >>> So that seems just fine to me.
> >>
> >> The spec says "The FCS option shall only be used when the mode is
> >> being, or is already configured to Enhanced Retransmission mode or
> >> Streaming mode." FCS is never used in basic mode (fallback or not).
> >>
> >> (Maybe I've misunderstood your point)
> >
> > So basic mode uses CRC16 as FCS. And with basic that is not changeable.
> > If you try basic mode + no FCS then we should clearly fail the call to
> > setsockopt() for that.
>
> That's the opposite of what I'm trying to say! :)
>
> Basic mode B-frames have *no* FCS (L2CAP spec section 3.1). The FCS
> field is only found in I-frames and S-frames used in ERTM and
> streaming modes. If you try basic mode + FCS, that should fail.
I confused myself right now. You are fully correct. The default for
basic mode is no fcs.
And as soon as an application wants to use ERTM we just make it to
specific the FCS option. That being CRC16 or no FCS. And of course the
getsockopt() call will return whatever we ended up configuring.
And yes, basic mode + crc16 should fail.
> >>> Maybe you need to explain a bit more in detail what you are trying to
> >>> achieve in conjunction with userspace API.
> >>
> >> My goal is to only have l2cap_pi(sk)->fcs == L2CAP_FCS_CRC16 when the
> >> FCS option is actually in use. Otherwise, any logic checking for FCS
> >> also has to check the L2CAP mode. Might as well check the mode once
> >> and set fcs accordingly -- which is what my patch does.
> >>
> >> Gustavo is correct that l2cap_pi(sk)->fcs is currently only checked on
> >> code paths used with ERTM and streaming mode. However, future code
> >> (including a patch I'll be posting soon) will depend on the fcs value
> >> being accurate in all modes.
> >
> > As I said, I have no problem with returning basic mode + crc16. Since
> > that is actually what you are doing in that case.
> >
> > The only thing that we have to keep in mind to be backward compatible is
> > to allow setting of basic mode and fcs = 0x00 and then change that into
> > crc16.
>
> The fcs field in L2CAP_OPTIONS is new for ERTM/Streaming modes, so
> there should be no backward compatibility issues with basic mode.
There is none. I got confused. Too many emails at the same time.
Regards
Marcel
^ permalink raw reply
* Multiple SPP connections, possible ? And, where are the docs ?
From: Banibrata Dutta @ 2010-08-03 16:38 UTC (permalink / raw)
To: linux-bluetooth
Hello,
Quite new to BlueZ, but familiar with Bluetooth on Windows, and Linux
in general. Did try to go through some documentation around BlueZ I
found on the net including a HOWTO, but suprisingly a lot seems to
have changed in terms of how to configure BlueZ. Is there a man-page,
a newer HOWTO, or user's guide or README that may help me figure out,
how to configure BlueZ ?
What I intend to do is to connect my PC to 2 devices over SPP,
simultaneously. My PC is dual-boot (boots into Win-XP SP3, and also
Ubuntu 9.10 on x86 (32-bit plaf). My Win-XP SP3 runs WIDCOMM stack,
and I can communicate with both of the devices but one-at-a-time, so I
know that my BT transceiver and end-devices (mobile phones) are
working fine, and their configuration (atleast phone side) is okay.
What I would like to do on Ubuntu, is to use BlueZ to connect to the 2
phones, simultaneously (something that I am unable to achieve in
Windows-XP). I have managed to pair with both devices from within
Ubuntu, using the Gnome BT-Manager tool, and even succeeded in
browsing file-systems on both phones over OBEX, simultaneously.
However, I find no information on how to setup/configure SP Profile,
and how to achieve the simultaneous usage.
Any help / pointers to right set of docs would really help.
regards,
BD
^ permalink raw reply
* Re: [PATCH 2/6] Clean up code that generates extended inquiry response.
From: Johan Hedberg @ 2010-08-03 16:26 UTC (permalink / raw)
To: ingas; +Cc: linux-bluetooth
In-Reply-To: <c830f11e501a70291cd4b5b3bde2f4a9.squirrel@www.codeaurora.org>
Hi Inga,
On Tue, Aug 03, 2010, ingas@codeaurora.org wrote:
> Just a clarification. I will still have to have the "clean up" patch as a
> part of this patch set since it relies on the header file with defines
> (getting rid of magic numbers).>
That's fine. I.e. the first patch of the set can remain as is.
Johan
^ permalink raw reply
* Re: [PATCH 1/8] Bluetooth: Make sure the L2CAP FCS is only enabled for ERTM or streaming.
From: Mat Martineau @ 2010-08-03 16:23 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: Gustavo F. Padovan, linux-bluetooth, rshaffer
In-Reply-To: <1280851870.12579.84.camel@localhost.localdomain>
Hi Marcel -
On Tue, 3 Aug 2010, Marcel Holtmann wrote:
>>>>>>> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
>>>>>>> ---
>>>>>>> net/bluetooth/l2cap.c | 12 ++++++++----
>>>>>>> 1 files changed, 8 insertions(+), 4 deletions(-)
>>>>>>>
>>>>>>> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
>>>>>>> index 9ba1e8e..aed72f2 100644
>>>>>>> --- a/net/bluetooth/l2cap.c
>>>>>>> +++ b/net/bluetooth/l2cap.c
>>>>>>> @@ -3127,8 +3127,10 @@ static inline int l2cap_config_req(struct l2cap_conn *conn, struct l2cap_cmd_hdr
>>>>>>> goto unlock;
>>>>>>>
>>>>>>> if (l2cap_pi(sk)->conf_state & L2CAP_CONF_INPUT_DONE) {
>>>>>>> - if (!(l2cap_pi(sk)->conf_state & L2CAP_CONF_NO_FCS_RECV) ||
>>>>>>> - l2cap_pi(sk)->fcs != L2CAP_FCS_NONE)
>>>>>>> + if ((l2cap_pi(sk)->mode == L2CAP_MODE_ERTM ||
>>>>>>> + l2cap_pi(sk)->mode == L2CAP_MODE_STREAMING) &&
>>>>>>> + (!(l2cap_pi(sk)->conf_state & L2CAP_CONF_NO_FCS_RECV) ||
>>>>>>> + l2cap_pi(sk)->fcs != L2CAP_FCS_NONE))
>>>>>>> l2cap_pi(sk)->fcs = L2CAP_FCS_CRC16;
>>>>>>
>>>>>> this becomes unreadable and my brain starts to throw a core dump. So it
>>>>>> clearly needs to be put into a helper inline function.
>>>>>
>>>>> Actually we don't need that, since the code that deals with Basic Mode
>>>>> never check and use the l2cap_pi(sk)->fcs. So we don't care about FCS
>>>>> value in the Basic Mode.
>>>>
>>>> There isn't currently any Basic Mode code that triggers this latent
>>>> bug, but I have a patch coming up that does require this fix.
>>>>
>>>> As it stands, getsockopt() on a connected basic mode socket shows FCS
>>>> enabled, so this bug is visible from userspace.
>>>
>>> can we just fail the setsockopt() when trying to set basic mode and FCS
>>> off.
>>
>> It definitely makes sense to have more validation of L2CAP_OPTIONS
>> passed to setsockopt().
>>
>>> And also in case fallback to basic mode happens, then FCS should be set
>>> to be enabled. Since for FCS and basic mode we always have to use FCS.
>>> So that seems just fine to me.
>>
>> The spec says "The FCS option shall only be used when the mode is
>> being, or is already configured to Enhanced Retransmission mode or
>> Streaming mode." FCS is never used in basic mode (fallback or not).
>>
>> (Maybe I've misunderstood your point)
>
> So basic mode uses CRC16 as FCS. And with basic that is not changeable.
> If you try basic mode + no FCS then we should clearly fail the call to
> setsockopt() for that.
That's the opposite of what I'm trying to say! :)
Basic mode B-frames have *no* FCS (L2CAP spec section 3.1). The FCS
field is only found in I-frames and S-frames used in ERTM and
streaming modes. If you try basic mode + FCS, that should fail.
>>> Maybe you need to explain a bit more in detail what you are trying to
>>> achieve in conjunction with userspace API.
>>
>> My goal is to only have l2cap_pi(sk)->fcs == L2CAP_FCS_CRC16 when the
>> FCS option is actually in use. Otherwise, any logic checking for FCS
>> also has to check the L2CAP mode. Might as well check the mode once
>> and set fcs accordingly -- which is what my patch does.
>>
>> Gustavo is correct that l2cap_pi(sk)->fcs is currently only checked on
>> code paths used with ERTM and streaming mode. However, future code
>> (including a patch I'll be posting soon) will depend on the fcs value
>> being accurate in all modes.
>
> As I said, I have no problem with returning basic mode + crc16. Since
> that is actually what you are doing in that case.
>
> The only thing that we have to keep in mind to be backward compatible is
> to allow setting of basic mode and fcs = 0x00 and then change that into
> crc16.
The fcs field in L2CAP_OPTIONS is new for ERTM/Streaming modes, so
there should be no backward compatibility issues with basic mode.
Regards,
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply
* Re: What is the motivation for conn->power_save
From: Nick Pelly @ 2010-08-03 16:20 UTC (permalink / raw)
To: Liang Bao; +Cc: Andrei Emeltchenko, Marcel Holtmann, linux-bluetooth
In-Reply-To: <AANLkTinxeqy3KMu0WLo7q-6yD2NbJU104BB4krSOo8gX@mail.gmail.com>
On Tue, Aug 3, 2010 at 2:31 AM, Liang Bao <tim.bao@gmail.com> wrote:
>
>
> 2010/7/9 Nick Pelly <npelly@google.com>
>>
>> On Thu, Jul 8, 2010 at 8:07 AM, Andrei Emeltchenko
>> <andrei.emeltchenko.news@gmail.com> wrote:
>> > Hi,
>> >
>> > On Wed, Jan 13, 2010 at 10:04 AM, Marcel Holtmann <marcel@holtmann.org=
> wrote:
>> >> Hi Nick,
>> >>
>> >>> >>> >> If I understand correctly, conn->power_save prevents the host=
stack
>> >>> >>> >> from requesting active mode if it was not the host stack that
>> >>> >>> >> requested sniff mode.
>> >>> >>> >>
>> >>> >>> >> I don't understand the motivation for this. If we have ACL da=
ta to
>> >>> >>> >> send, then it seems like a good idea for the host stack to ex=
plicitly
>> >>> >>> >> request active mode, regardless of the reason that we entered=
sniff
>> >>> >>> >> mode.
>> >>> >>> >>
>> >>> >>> >> We want to enter active mode more aggressively when setting u=
p SCO
>> >>> >>> >> connections, to avoid a 5 second delay with certain sniff mod=
es. But
>> >>> >>> >> the conn->power_save code is getting in the way and doesn't a=
ppear to
>> >>> >>> >> be useful in the first place.
>> >>> >>> >
>> >>> >>> > we have discussed this a few times. And if you lock through th=
e code
>> >>> >>> > history then you see that initially we just took devices out o=
f sniff
>> >>> >>> > mode if we had to send data. However with HID devices this fal=
ls flat on
>> >>> >>> > its face. They need to stay in control of sniff mode if they i=
nitiated
>> >>> >>> > it. Some of them crash and others just drain the battery. With=
sniff
>> >>> >>> > mode you can send small amounts of data even while in sniff an=
d for HID
>> >>> >>> > that is sometimes used. So the remote side better not interfer=
e.
>> >>> >>> >
>> >>> >>> > What we really need is a socket option where we can control th=
is on a
>> >>> >>> > per socket basis if we take devices out of sniff mode. And one=
extra
>> >>> >>> > case might be when we try to establish a SCO channel, because =
then it is
>> >>> >>> > clearly not an HID device. However even A2DP has this sort of =
problems
>> >>> >>> > sometimes where the stream setup takes time.
>> >>> >>>
>> >>> >>> Makes sense. Thanks for the explanation.
>> >>> >>
>> >>> >> this means you will be working on a patch for this :)
>> >>>
>> >>> Actually, I want to put a patch together for a socket option to not
>> >>> use power_save (so that we *always* exit sniff mode when sending ACL
>> >>> data). We're seeing this problem with the Plantronics Voyager 855
>> >>> which enters sniff mode during A2DP.
>> >>>
>> >>> Given that power_save is just for HID devices, my preferred design i=
s
>> >>> to disable power_save by default, and have an L2CAP socket option to
>> >>> turn on power_save that would be used by HID L2CAP sockets.
>> >>> Unfortunately this would require userspace HID code to use the new
>> >>> socket option to keep current behavior. But it seems preferable to t=
he
>> >>> alternative of having every single other L2CAP socket use a new sock=
et
>> >>> option just to disable power_save for the sake of HID.
>> >>
>> >> actually you still mix up the meaning of the power_save option. From =
the
>> >> stack side, the automatic sniff mode is off by default. You have to
>> >> enable via sysfs first. The power_save option is just to control when
>> >> sniff mode is activated and the remote enters sniff mode, that we are
>> >> not getting out of it if we have ACL data.
>> >>
>> >> In conclusion, I am fine with a socket option that allows to control
>> >> sniff mode (preferable with interval details) and sniff subrate detai=
ls
>> >> so that we can use them for that ACL link.
>> >>
>> >> So go ahead and work on this. We can fix userspace easily since a new
>> >> socket can be detected easily with newer kernels.
>> >
>> > We have found that some Nokia BT headsets stop working after idle
>> > period when they enter
>> > "power save" mode.
>> >
>> > Do we have any solution for this problem? So far I see good enough
>> > patch :-)) below:
>> >
>> > http://android.git.kernel.org/?p=3Dkernel/common.git;a=3Dblobdiff;f=3D=
net/bluetooth/hci_conn.c;h=3Dfa8b412205cd89546b4a325c558c68040eeaf491;hp=3D=
0cf25114a3f576fc2788a549eb96d0087dd39b44;hb=3Dd8488237646920cd71ef43e5f3ae1=
001c6f4cf7b;hpb=3D3f68e5050c5ae559f56d5da9202cb88928d42b36
>> >
>> > - =A0 =A0 =A0 if (conn->mode !=3D HCI_CM_SNIFF || !conn->power_save)
>> > + =A0 =A0 =A0 if (conn->mode !=3D HCI_CM_SNIFF /* || !conn->power_save=
*/)
>> >
>> > Nick have you done socket option for "power_save"?
>>
>> No. We'll do it once we need it for HID support.
>
> Can we do this: try to do check against some existing fields in structure=
hci_conn, for example device_class and then determine if conn->power_save =
shall be used here? By doing this, applications in user space can be saved =
from having knowledge how sniff mode works for different devices.
The class bits cannot be trusted - many devices have the wrong class
bits. And even with the right class bits, what about a device that
supports both HID and A2DP? A socket option still seems like the best
approach.
Nick
^ permalink raw reply
* Re: What is the motivation for conn->power_save
From: Nick Pelly @ 2010-08-03 16:16 UTC (permalink / raw)
To: Liang Bao; +Cc: Andrei Emeltchenko, Marcel Holtmann, linux-bluetooth
In-Reply-To: <AANLkTinxeqy3KMu0WLo7q-6yD2NbJU104BB4krSOo8gX@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 5217 bytes --]
On Tue, Aug 3, 2010 at 2:31 AM, Liang Bao <tim.bao@gmail.com> wrote:
>
>
> 2010/7/9 Nick Pelly <npelly@google.com>
>
> On Thu, Jul 8, 2010 at 8:07 AM, Andrei Emeltchenko
>> <andrei.emeltchenko.news@gmail.com> wrote:
>> > Hi,
>> >
>> > On Wed, Jan 13, 2010 at 10:04 AM, Marcel Holtmann <marcel@holtmann.org>
>> wrote:
>> >> Hi Nick,
>> >>
>> >>> >>> >> If I understand correctly, conn->power_save prevents the host
>> stack
>> >>> >>> >> from requesting active mode if it was not the host stack that
>> >>> >>> >> requested sniff mode.
>> >>> >>> >>
>> >>> >>> >> I don't understand the motivation for this. If we have ACL data
>> to
>> >>> >>> >> send, then it seems like a good idea for the host stack to
>> explicitly
>> >>> >>> >> request active mode, regardless of the reason that we entered
>> sniff
>> >>> >>> >> mode.
>> >>> >>> >>
>> >>> >>> >> We want to enter active mode more aggressively when setting up
>> SCO
>> >>> >>> >> connections, to avoid a 5 second delay with certain sniff
>> modes. But
>> >>> >>> >> the conn->power_save code is getting in the way and doesn't
>> appear to
>> >>> >>> >> be useful in the first place.
>> >>> >>> >
>> >>> >>> > we have discussed this a few times. And if you lock through the
>> code
>> >>> >>> > history then you see that initially we just took devices out of
>> sniff
>> >>> >>> > mode if we had to send data. However with HID devices this falls
>> flat on
>> >>> >>> > its face. They need to stay in control of sniff mode if they
>> initiated
>> >>> >>> > it. Some of them crash and others just drain the battery. With
>> sniff
>> >>> >>> > mode you can send small amounts of data even while in sniff and
>> for HID
>> >>> >>> > that is sometimes used. So the remote side better not interfere.
>> >>> >>> >
>> >>> >>> > What we really need is a socket option where we can control this
>> on a
>> >>> >>> > per socket basis if we take devices out of sniff mode. And one
>> extra
>> >>> >>> > case might be when we try to establish a SCO channel, because
>> then it is
>> >>> >>> > clearly not an HID device. However even A2DP has this sort of
>> problems
>> >>> >>> > sometimes where the stream setup takes time.
>> >>> >>>
>> >>> >>> Makes sense. Thanks for the explanation.
>> >>> >>
>> >>> >> this means you will be working on a patch for this :)
>> >>>
>> >>> Actually, I want to put a patch together for a socket option to not
>> >>> use power_save (so that we *always* exit sniff mode when sending ACL
>> >>> data). We're seeing this problem with the Plantronics Voyager 855
>> >>> which enters sniff mode during A2DP.
>> >>>
>> >>> Given that power_save is just for HID devices, my preferred design is
>> >>> to disable power_save by default, and have an L2CAP socket option to
>> >>> turn on power_save that would be used by HID L2CAP sockets.
>> >>> Unfortunately this would require userspace HID code to use the new
>> >>> socket option to keep current behavior. But it seems preferable to the
>> >>> alternative of having every single other L2CAP socket use a new socket
>> >>> option just to disable power_save for the sake of HID.
>> >>
>> >> actually you still mix up the meaning of the power_save option. From
>> the
>> >> stack side, the automatic sniff mode is off by default. You have to
>> >> enable via sysfs first. The power_save option is just to control when
>> >> sniff mode is activated and the remote enters sniff mode, that we are
>> >> not getting out of it if we have ACL data.
>> >>
>> >> In conclusion, I am fine with a socket option that allows to control
>> >> sniff mode (preferable with interval details) and sniff subrate details
>> >> so that we can use them for that ACL link.
>> >>
>> >> So go ahead and work on this. We can fix userspace easily since a new
>> >> socket can be detected easily with newer kernels.
>> >
>> > We have found that some Nokia BT headsets stop working after idle
>> > period when they enter
>> > "power save" mode.
>> >
>> > Do we have any solution for this problem? So far I see good enough
>> > patch :-)) below:
>> >
>> >
>> http://android.git.kernel.org/?p=kernel/common.git;a=blobdiff;f=net/bluetooth/hci_conn.c;h=fa8b412205cd89546b4a325c558c68040eeaf491;hp=0cf25114a3f576fc2788a549eb96d0087dd39b44;hb=d8488237646920cd71ef43e5f3ae1001c6f4cf7b;hpb=3f68e5050c5ae559f56d5da9202cb88928d42b36
>> >
>> > - if (conn->mode != HCI_CM_SNIFF || !conn->power_save)
>> > + if (conn->mode != HCI_CM_SNIFF /* || !conn->power_save */)
>> >
>> > Nick have you done socket option for "power_save"?
>>
>> No. We'll do it once we need it for HID support.
>>
> Can we do this: try to do check against some existing fields in structure
> hci_conn, for example device_class and then determine if conn->power_save
> shall be used here? By doing this, applications in user space can be saved
> from having knowledge how sniff mode works for different devices.
>
>
The class bits cannot be trusted - many devices have the wrong class bits.
And even with the right class bits, what about a device that supports both
HID and A2DP? A socket option still seems like the best approach.
Nick
[-- Attachment #2: Type: text/html, Size: 7242 bytes --]
^ permalink raw reply
* Re: Enhancements to allow l2cap channel to use either AMP or BR/EDR
From: Marcel Holtmann @ 2010-08-03 16:14 UTC (permalink / raw)
To: David Vrabel
Cc: tmonahan, Inga Stotland, linux-bluetooth, rshaffer, johan.hedberg
In-Reply-To: <4C58129E.3080306@csr.com>
Hi David,
> > That said, nothing stops us from allowing to make our AMP polices
> > dynamic and allow the application to change it threshold during
> > lifetime. For example it starts out as BR/EDR only and then during the
> > usage of the link it decides that now AMP preferred would make sense. In
> > that sense you would have manual switching to some level.
>
> If the policy is dynamic throughout the lifetime of the connection then
> that's okay for best effort links.
>
> I think applications (especially those that stream real-time data at
> high rates) will need to know the currently available bandwidth. This
> will be useful for even simple file transfer profiles -- if that file
> transfer is going to take 100x as long the user probably want to know
> about this so they can (for example) turn HS mode on the other device.
>
> This information could be conveyed in some sort of "channel move
> complete" event or a more generic "available bandwidth changed" event.
so here you have the following problem, the only interface you get from
an application point of view is a L2CAP socket. So you have to work with
these constraints. Potentially we can provide something additional via
the OOB msg structs (like we do for timestamps), but I am really careful
in adding to much into these ones.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 8/8] Bluetooth: Use 3-DH5 payload size for default ERTM max PDU size.
From: Mat Martineau @ 2010-08-03 16:13 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth, gustavo, rshaffer, linux-arm-msm
In-Reply-To: <1280778508.12579.48.camel@localhost.localdomain>
Marcel,
On Mon, 2 Aug 2010, Marcel Holtmann wrote:
> Hi Mat,
>
>> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
>> ---
>> include/net/bluetooth/l2cap.h | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> this should have a commit message with technical details to explain why
> it is a good idea.
>
>> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
>> index 16e412f..93aba17 100644
>> --- a/include/net/bluetooth/l2cap.h
>> +++ b/include/net/bluetooth/l2cap.h
>> @@ -35,7 +35,7 @@
>> #define L2CAP_DEFAULT_MAX_TX 3
>> #define L2CAP_DEFAULT_RETRANS_TO 2000 /* 2 seconds */
>> #define L2CAP_DEFAULT_MONITOR_TO 12000 /* 12 seconds */
>> -#define L2CAP_DEFAULT_MAX_PDU_SIZE 672
>> +#define L2CAP_DEFAULT_MAX_PDU_SIZE 1011 /* Sized for 3-DH5 packet */
>> #define L2CAP_DEFAULT_ACK_TO 200
>> #define L2CAP_LOCAL_BUSY_TRIES 12
>
> Also the default PDU size for basic mode should stay with 672. And only
> for ERTM move it to 1010. Is this ensured?
>
> And then again here. Is that explained in the commit message?
It is ensured that the max PDU size is only used for ERTM and
streaming mode. The default MTU for basic mode and ERTM/streaming
remains unchanged.
I will update the commit message to fully describe this.
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply
* Re: [PATCH 1/8] Bluetooth: Make sure the L2CAP FCS is only enabled for ERTM or streaming.
From: Marcel Holtmann @ 2010-08-03 16:11 UTC (permalink / raw)
To: Mat Martineau; +Cc: Gustavo F. Padovan, linux-bluetooth, rshaffer
In-Reply-To: <alpine.DEB.2.00.1008030814570.19861@linux-sea-02>
Hi mat,
> >>>>> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> >>>>> ---
> >>>>> net/bluetooth/l2cap.c | 12 ++++++++----
> >>>>> 1 files changed, 8 insertions(+), 4 deletions(-)
> >>>>>
> >>>>> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> >>>>> index 9ba1e8e..aed72f2 100644
> >>>>> --- a/net/bluetooth/l2cap.c
> >>>>> +++ b/net/bluetooth/l2cap.c
> >>>>> @@ -3127,8 +3127,10 @@ static inline int l2cap_config_req(struct l2cap_conn *conn, struct l2cap_cmd_hdr
> >>>>> goto unlock;
> >>>>>
> >>>>> if (l2cap_pi(sk)->conf_state & L2CAP_CONF_INPUT_DONE) {
> >>>>> - if (!(l2cap_pi(sk)->conf_state & L2CAP_CONF_NO_FCS_RECV) ||
> >>>>> - l2cap_pi(sk)->fcs != L2CAP_FCS_NONE)
> >>>>> + if ((l2cap_pi(sk)->mode == L2CAP_MODE_ERTM ||
> >>>>> + l2cap_pi(sk)->mode == L2CAP_MODE_STREAMING) &&
> >>>>> + (!(l2cap_pi(sk)->conf_state & L2CAP_CONF_NO_FCS_RECV) ||
> >>>>> + l2cap_pi(sk)->fcs != L2CAP_FCS_NONE))
> >>>>> l2cap_pi(sk)->fcs = L2CAP_FCS_CRC16;
> >>>>
> >>>> this becomes unreadable and my brain starts to throw a core dump. So it
> >>>> clearly needs to be put into a helper inline function.
> >>>
> >>> Actually we don't need that, since the code that deals with Basic Mode
> >>> never check and use the l2cap_pi(sk)->fcs. So we don't care about FCS
> >>> value in the Basic Mode.
> >>
> >> There isn't currently any Basic Mode code that triggers this latent
> >> bug, but I have a patch coming up that does require this fix.
> >>
> >> As it stands, getsockopt() on a connected basic mode socket shows FCS
> >> enabled, so this bug is visible from userspace.
> >
> > can we just fail the setsockopt() when trying to set basic mode and FCS
> > off.
>
> It definitely makes sense to have more validation of L2CAP_OPTIONS
> passed to setsockopt().
>
> > And also in case fallback to basic mode happens, then FCS should be set
> > to be enabled. Since for FCS and basic mode we always have to use FCS.
> > So that seems just fine to me.
>
> The spec says "The FCS option shall only be used when the mode is
> being, or is already configured to Enhanced Retransmission mode or
> Streaming mode." FCS is never used in basic mode (fallback or not).
>
> (Maybe I've misunderstood your point)
So basic mode uses CRC16 as FCS. And with basic that is not changeable.
If you try basic mode + no FCS then we should clearly fail the call to
setsockopt() for that.
> > Maybe you need to explain a bit more in detail what you are trying to
> > achieve in conjunction with userspace API.
>
> My goal is to only have l2cap_pi(sk)->fcs == L2CAP_FCS_CRC16 when the
> FCS option is actually in use. Otherwise, any logic checking for FCS
> also has to check the L2CAP mode. Might as well check the mode once
> and set fcs accordingly -- which is what my patch does.
>
> Gustavo is correct that l2cap_pi(sk)->fcs is currently only checked on
> code paths used with ERTM and streaming mode. However, future code
> (including a patch I'll be posting soon) will depend on the fcs value
> being accurate in all modes.
As I said, I have no problem with returning basic mode + crc16. Since
that is actually what you are doing in that case.
The only thing that we have to keep in mind to be backward compatible is
to allow setting of basic mode and fcs = 0x00 and then change that into
crc16.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 1/8] Bluetooth: Make sure the L2CAP FCS is only enabled for ERTM or streaming.
From: Mat Martineau @ 2010-08-03 15:58 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: Gustavo F. Padovan, linux-bluetooth, rshaffer
In-Reply-To: <1280789488.12579.60.camel@localhost.localdomain>
On Mon, 2 Aug 2010, Marcel Holtmann wrote:
> Hi Mat,
>
>>> * Marcel Holtmann <marcel@holtmann.org> [2010-08-02 12:38:32 -0700]:
>>>
>>>> Hi Mat,
>>>>
>>>>> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
>>>>> ---
>>>>> net/bluetooth/l2cap.c | 12 ++++++++----
>>>>> 1 files changed, 8 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
>>>>> index 9ba1e8e..aed72f2 100644
>>>>> --- a/net/bluetooth/l2cap.c
>>>>> +++ b/net/bluetooth/l2cap.c
>>>>> @@ -3127,8 +3127,10 @@ static inline int l2cap_config_req(struct l2cap_conn *conn, struct l2cap_cmd_hdr
>>>>> goto unlock;
>>>>>
>>>>> if (l2cap_pi(sk)->conf_state & L2CAP_CONF_INPUT_DONE) {
>>>>> - if (!(l2cap_pi(sk)->conf_state & L2CAP_CONF_NO_FCS_RECV) ||
>>>>> - l2cap_pi(sk)->fcs != L2CAP_FCS_NONE)
>>>>> + if ((l2cap_pi(sk)->mode == L2CAP_MODE_ERTM ||
>>>>> + l2cap_pi(sk)->mode == L2CAP_MODE_STREAMING) &&
>>>>> + (!(l2cap_pi(sk)->conf_state & L2CAP_CONF_NO_FCS_RECV) ||
>>>>> + l2cap_pi(sk)->fcs != L2CAP_FCS_NONE))
>>>>> l2cap_pi(sk)->fcs = L2CAP_FCS_CRC16;
>>>>
>>>> this becomes unreadable and my brain starts to throw a core dump. So it
>>>> clearly needs to be put into a helper inline function.
>>>
>>> Actually we don't need that, since the code that deals with Basic Mode
>>> never check and use the l2cap_pi(sk)->fcs. So we don't care about FCS
>>> value in the Basic Mode.
>>
>> There isn't currently any Basic Mode code that triggers this latent
>> bug, but I have a patch coming up that does require this fix.
>>
>> As it stands, getsockopt() on a connected basic mode socket shows FCS
>> enabled, so this bug is visible from userspace.
>
> can we just fail the setsockopt() when trying to set basic mode and FCS
> off.
It definitely makes sense to have more validation of L2CAP_OPTIONS
passed to setsockopt().
> And also in case fallback to basic mode happens, then FCS should be set
> to be enabled. Since for FCS and basic mode we always have to use FCS.
> So that seems just fine to me.
The spec says "The FCS option shall only be used when the mode is
being, or is already configured to Enhanced Retransmission mode or
Streaming mode." FCS is never used in basic mode (fallback or not).
(Maybe I've misunderstood your point)
> Maybe you need to explain a bit more in detail what you are trying to
> achieve in conjunction with userspace API.
My goal is to only have l2cap_pi(sk)->fcs == L2CAP_FCS_CRC16 when the
FCS option is actually in use. Otherwise, any logic checking for FCS
also has to check the L2CAP mode. Might as well check the mode once
and set fcs accordingly -- which is what my patch does.
Gustavo is correct that l2cap_pi(sk)->fcs is currently only checked on
code paths used with ERTM and streaming mode. However, future code
(including a patch I'll be posting soon) will depend on the fcs value
being accurate in all modes.
I only mentioned getsockopt() to show that this issue is not
completely invisible, and is worth patching.
Regards,
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply
* Re: [PATCH 2/6] Clean up code that generates extended inquiry response.
From: ingas @ 2010-08-03 15:37 UTC (permalink / raw)
To: johan.hedberg; +Cc: linux-bluetooth
In-Reply-To: <20100803081258.GA5831@jh-x301>
Hi Johan,
>> ---
>> src/sdpd-service.c | 46
>> ++++++++++++++++++++++++++++++----------------
>> 1 files changed, 30 insertions(+), 16 deletions(-)
>
> So your commit message says two things: "Clean up code..." and "Minor
> fix..". Cleanups and fixes should be in separate patches. Don't be
> afraid of "too small" or trivial patches. As long as a patch contains a
> single logically independent change it's fine.
>
Just a clarification. I will still have to have the "clean up" patch as a
part of this patch set since it relies on the header file with defines
(getting rid of magic numbers). Not sure, if you want me to cut the "fix"
one to be separate patch submission altogether...
Thanks,
Inga
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply
* btd_adapter_driver
From: Daniel Örstadius @ 2010-08-03 14:38 UTC (permalink / raw)
To: linux-bluetooth
Hi Marcel,
I have a question about the btd_adapter_driver struct.
The Maemo6 MCE plugin (which uses that struct) needs to be notified
when the adapter is powered on or off. One idea is to add a callback
to btd_adapter_driver to be called in adapter.c, but from discussion
with Johan the struct was intended to stay very simple, so that this
might not be an acceptable solution.
What would be a good way to handle it? Could listening to a raw hci
socket inside the driver be an option?
Thanks in advance,
Daniel
^ permalink raw reply
* Re: Enhancements to allow l2cap channel to use either AMP or BR/EDR
From: Tim Monahan-Mitchell @ 2010-08-03 13:40 UTC (permalink / raw)
To: Gustavo F. Padovan
Cc: Peter Krystad, Kevin Hayes, Marcel Holtmann,
tmonahan@codeaurora.org, David Vrabel, Inga Stotland,
linux-bluetooth@vger.kernel.org, rshaffer@codeaurora.org,
johan.hedberg@gmail.com
In-Reply-To: <20100803080719.GD20149@vigoh>
Hi, Gustavo and Marcel,
> Hi Peter,
>
> * Peter Krystad <pkrystad@codeaurora.org> [2010-08-02 18:30:34 -0700]:
>
>>
>> Hi Marcel and Kevin,
>>
>> >> >
>> >> > Agree that it should be done "in background" and that a size
>> >> threshold would be useful. But who evaluates that threshold? The
>> >> bluez kernel components, which essentially implement a transport
>> >> driver, should not be examining objects (files, phonebooks, etc) size
>> >> to see if this threshold is met. Therefore, it would seem Tim's
>> >> suggestion of having the profile send a 'prefer_amp' bit would be
>> >> useful, right?
>> >>
>> >> I would do something like "prefer_amp when over 100kb" or something.
>> >> And
>> >> then the kernel needs to count. Meaning the L2CAP layer could easily
>> >> count this by itself.
>> >
>> > What? Let's say the effect we want is that if the object is greater
>> > than, say, 10 megabytes, then we want to use AMP for the transfer.
>> > With your scheme, you want the kernel to count up to 10 megabytes,
>> > THEN switch over to AMP?
>> > No, you don't, he says rhetorically. :)
>>
>> If the policy was defined as "prefer_amp after n seconds" we would get
>> the desired effect without having to do any counting in the kernel.
>
> In both cases we have to take in account the total size of the file, or
> the estimated time of transfer, so we can avoid to move to AMP when the
> transfer is about to end.
Right! Don't want to switch to AMP just to push those last 100 bytes :)
Since the kernel does not know the total object size to be transferred,
(right?), I've yet to come up with any 'threshold' ideas the kernel can
make. Note that the transfer could be one huge file or many small ones.
--
Tim Monahan-Mitchell
Employee of Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply
* Re: [PATCH][RFC] Fix SDP resolving segfault
From: Manuel Naranjo @ 2010-08-03 13:21 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: Johan Hedberg, BlueZ
In-Reply-To: <4C518373.6000706@aircable.net>
> I will give this a try and let you know. Maybe I can results before the
> Tuesday, but I doubt it.
As I said on IRC it keeps crashing. Here's the log, as you can see the
crash happened in a different place now.
bluetoothd[1842]: src/device.c:btd_device_ref() 0x9411de0: ref=1
bluetoothd[1842]: src/device.c:btd_device_ref() 0x9411de0: ref=2
bluetoothd[1842]: src/device.c:btd_device_ref() 0x9409570: ref=2
bluetoothd[1842]: src/device.c:btd_device_ref() 0x9408b60: ref=2
bluetoothd[1842]: src/adapter.c:create_device() C8:7E:75:D6:B3:B0
bluetoothd[1842]: src/adapter.c:adapter_create_device() C8:7E:75:D6:B3:B0
bluetoothd[1842]: src/device.c:device_create() Creating device
/org/bluez/1842/hci0/dev_C8_7E_75_D6_B3_B0
bluetoothd[1842]: src/device.c:btd_device_ref() 0x93fd258: ref=1
bluetoothd[1842]: src/device.c:btd_device_ref() 0x93fd258: ref=2
bluetoothd[1842]: src/device.c:btd_device_ref() 0x9404dc0: ref=3
bluetoothd[1842]: Discovery session 0x93fb658 with :1.58 activated
bluetoothd[1842]: src/adapter.c:session_ref() 0x93fb658: ref=1
bluetoothd[1842]: src/adapter.c:adapter_get_device() 00:25:47:2D:20:98
bluetoothd[1842]: src/adapter.c:session_unref() 0x93fb658: ref=0
bluetoothd[1842]: src/adapter.c:session_remove() Discovery session
0x93fb658 with :1.58 deactivated
bluetoothd[1842]: src/adapter.c:session_remove() Stopping discovery
bluetoothd[1842]: Stopping discovery
bluetoothd[1842]: /org/bluez/1842/hci0/dev_C8_7E_75_D6_B3_B0: error
updating services: Device or resource busy (16)
bluetoothd[1842]: src/device.c:btd_device_unref() 0x93fd258: ref=1
bluetoothd[1842]: Discovery session 0x93fd4b0 with :1.58 activated
bluetoothd[1842]: src/adapter.c:session_ref() 0x93fd4b0: ref=1
bluetoothd[1842]: /org/bluez/1842/hci0/dev_00_26_E2_3C_5E_05: error
updating services: Host is down (112)
bluetoothd[1842]: src/device.c:btd_device_unref() 0x9404dc0: ref=2
bluetoothd[1842]: src/adapter.c:adapter_get_device() 00:26:E2:3C:5E:05
bluetoothd[1842]: src/adapter.c:session_unref() 0x93fd4b0: ref=0
bluetoothd[1842]: src/adapter.c:session_remove() Discovery session
0x93fd4b0 with :1.58 deactivated
bluetoothd[1842]: src/adapter.c:session_remove() Stopping discovery
bluetoothd[1842]: Stopping discovery
bluetoothd[1842]: /org/bluez/1842/hci0/dev_00_1C_D6_B1_58_1B: error
updating services: Device or resource busy (16)
bluetoothd[1842]: src/device.c:btd_device_unref() 0x9409570: ref=1
bluetoothd[1842]: Discovery session 0x93f9f80 with :1.58 activated
bluetoothd[1842]: src/adapter.c:session_ref() 0x93f9f80: ref=1
bluetoothd[1842]: src/adapter.c:adapter_remove_connection() Removing
temporary device /org/bluez/1842/hci0/dev_00_25_47_2D_20_98
bluetoothd[1842]: src/device.c:device_remove() Removing device
/org/bluez/1842/hci0/dev_00_25_47_2D_20_98
bluetoothd[1842]: src/device.c:btd_device_unref() 0x9411120: ref=1
bluetoothd[1842]: src/device.c:btd_device_unref() 0x9411120: ref=0
bluetoothd[1842]: src/device.c:device_free() 0x9411120
bluetoothd[1842]: src/adapter.c:adapter_get_device() 00:17:D5:65:D0:3D
bluetoothd[1842]: src/adapter.c:session_unref() 0x93f9f80: ref=0
bluetoothd[1842]: src/adapter.c:session_remove() Discovery session
0x93f9f80 with :1.58 deactivated
bluetoothd[1842]: src/adapter.c:session_remove() Stopping discovery
bluetoothd[1842]: Stopping discovery
bluetoothd[1842]: Discovery session 0x9410560 with :1.58 activated
bluetoothd[1842]: src/adapter.c:session_ref() 0x9410560: ref=1
bluetoothd[1842]: src/device.c:device_probe_drivers() Probe drivers
for /org/bluez/1842/hci0/dev_00_17_D5_65_D0_3D
bluetoothd[1842]: serial/manager.c:serial_probe() path
/org/bluez/1842/hci0/dev_00_17_D5_65_D0_3D:
00001105-0000-1000-8000-00805f9b34fb
bluetoothd[1842]: serial/port.c:create_serial_device() Registered
interface org.bluez.Serial on path
/org/bluez/1842/hci0/dev_00_17_D5_65_D0_3D
bluetoothd[1842]: src/device.c:btd_device_unref() 0x9408b60: ref=1
Segmentation fault
--
Manuel Francisco Naranjo
Software Department Argentina
Wireless Cables Inc
www.aircable.net
cel: +5493412010019
skype: naranjomanuelfrancisco
^ permalink raw reply
* Re: Enhancements to allow l2cap channel to use either AMP or BR/EDR
From: David Vrabel @ 2010-08-03 12:59 UTC (permalink / raw)
To: Marcel Holtmann
Cc: tmonahan, Inga Stotland, linux-bluetooth, rshaffer, johan.hedberg
In-Reply-To: <1280775200.12579.30.camel@localhost.localdomain>
Marcel Holtmann wrote:
>
> That said, nothing stops us from allowing to make our AMP polices
> dynamic and allow the application to change it threshold during
> lifetime. For example it starts out as BR/EDR only and then during the
> usage of the link it decides that now AMP preferred would make sense. In
> that sense you would have manual switching to some level.
If the policy is dynamic throughout the lifetime of the connection then
that's okay for best effort links.
I think applications (especially those that stream real-time data at
high rates) will need to know the currently available bandwidth. This
will be useful for even simple file transfer profiles -- if that file
transfer is going to take 100x as long the user probably want to know
about this so they can (for example) turn HS mode on the other device.
This information could be conveyed in some sort of "channel move
complete" event or a more generic "available bandwidth changed" event.
Applications requiring guaranteed links will need some way of supplying
extended flow specifications.
David
--
David Vrabel, Senior Software Engineer, Drivers
CSR, Churchill House, Cambridge Business Park, Tel: +44 (0)1223 692562
Cowley Road, Cambridge, CB4 0WZ http://www.csr.com/
Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
^ permalink raw reply
* Re: [PATCH] Notify to device drivers when the SDP records change
From: José Antonio Santos Cadenas @ 2010-08-03 12:43 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <AANLkTin9WnU0v2NqS5cAjRxyhipJCBYX0=KWponiCk14@mail.gmail.com>
El Tuesday 03 August 2010 13:42:15 Luiz Augusto von Dentz escribió:
> Hi,
>
> On Tue, Aug 3, 2010 at 12:12 PM, José Antonio Santos Cadenas
>
> <santoscadenas@gmail.com> wrote:
> > The different with a2dp is that the application will decide the end point
> > to connect to, because there can be more than one matching the source.
> > This end point is represented by a path that is passed to the
> > HealthAgent when is discovered. If the remote record changes it is
> > possible that new end points are available so the application needs
> > something to discover the available end points. That is why I think that
> > we need something like this.
>
> I guess we already suggest you guys to take a look in the
> MediaEndpoint interface, which can be found here:
>
> http://gitorious.org/~vudentz/bluez/vudentzs-clone/commit/2af3d2821f0aaafae
> 12be10522c82dea6a701688.patch
>
> It doesn't expose the remote endpoints it basically do the best
> matching, if the application requesting the connection has local
> endpoint they have priority, and then the 'agent' or in case of audio
> the endpoint will be notified with the remote endpoint capabilities.
>
> Also while looking the health API it seems you guys missed a very
> important point, HealthAgent only knows about the remote services
> since there is no signal for announcing a new HealthApplication nor
> the application knows the about HealthService, so the only way make
> this to work is that the HealthApplication and HealthAgent leaves in
> the same process but this defeats the point of having them separated.
Yes, this only works in the same process. It is a way for receiving callbacks
from the daemon.
> So either we have the applications and services being announced to
> everyone and then anyone can do the matching or we don't announce them
> at all and make bluetoothd do the matching itself, I guess the latter
> is better so we don't have to split the logic in many place (with
> potential code duplication + bugs).
At this point, I have the same opinion, Services can be notified using
signals. But Marcel thought that it will be better and will let the
programmers make less mistakes if only the services related to the application
are notified, that is why this information is send through the agent instead
of sending signals. Of course this can be changed if we think that is better.
>
> Another important thing is that dbus introspection does not have any
> way to notify changes, so it really need to be signal to notify
> applications about new services.
^ permalink raw reply
* Re: [PATCH] Notify to device drivers when the SDP records change
From: José Antonio Santos Cadenas @ 2010-08-03 12:33 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <AANLkTin9WnU0v2NqS5cAjRxyhipJCBYX0=KWponiCk14@mail.gmail.com>
Hi,
El Tuesday 03 August 2010 13:42:15 Luiz Augusto von Dentz escribió:
> Hi,
>
> On Tue, Aug 3, 2010 at 12:12 PM, José Antonio Santos Cadenas
>
> <santoscadenas@gmail.com> wrote:
> > The different with a2dp is that the application will decide the end point
> > to connect to, because there can be more than one matching the source.
> > This end point is represented by a path that is passed to the
> > HealthAgent when is discovered. If the remote record changes it is
> > possible that new end points are available so the application needs
> > something to discover the available end points. That is why I think that
> > we need something like this.
>
> I guess we already suggest you guys to take a look in the
> MediaEndpoint interface, which can be found here:
>
> http://gitorious.org/~vudentz/bluez/vudentzs-clone/commit/2af3d2821f0aaafae
> 12be10522c82dea6a701688.patch
>
> It doesn't expose the remote endpoints it basically do the best
> matching, if the application requesting the connection has local
> endpoint they have priority, and then the 'agent' or in case of audio
> the endpoint will be notified with the remote endpoint capabilities.
>
> Also while looking the health API it seems you guys missed a very
> important point, HealthAgent only knows about the remote services
> since there is no signal for announcing a new HealthApplication nor
> the application knows the about HealthService, so the only way make
> this to work is that the HealthApplication and HealthAgent leaves in
> the same process but this defeats the point of having them separated.
> So either we have the applications and services being announced to
> everyone and then anyone can do the matching or we don't announce them
> at all and make bluetoothd do the matching itself, I guess the latter
> is better so we don't have to split the logic in many place (with
> potential code duplication + bugs).
>
> Another important thing is that dbus introspection does not have any
> way to notify changes, so it really need to be signal to notify
> applications about new services.
Can we have an IRC session to talk about the API and continuing this
discussion? I think that this thread is changing too much from its original
topic.
Regards.
^ permalink raw reply
* Re: [PATCH] Notify to device drivers when the SDP records change
From: Luiz Augusto von Dentz @ 2010-08-03 11:42 UTC (permalink / raw)
To: José Antonio Santos Cadenas; +Cc: linux-bluetooth
In-Reply-To: <201008031112.11556.santoscadenas@gmail.com>
Hi,
On Tue, Aug 3, 2010 at 12:12 PM, José Antonio Santos Cadenas
<santoscadenas@gmail.com> wrote:
> The different with a2dp is that the application will decide the end point to
> connect to, because there can be more than one matching the source. This end
> point is represented by a path that is passed to the HealthAgent when is
> discovered. If the remote record changes it is possible that new end points
> are available so the application needs something to discover the available end
> points. That is why I think that we need something like this.
I guess we already suggest you guys to take a look in the
MediaEndpoint interface, which can be found here:
http://gitorious.org/~vudentz/bluez/vudentzs-clone/commit/2af3d2821f0aaafae12be10522c82dea6a701688.patch
It doesn't expose the remote endpoints it basically do the best
matching, if the application requesting the connection has local
endpoint they have priority, and then the 'agent' or in case of audio
the endpoint will be notified with the remote endpoint capabilities.
Also while looking the health API it seems you guys missed a very
important point, HealthAgent only knows about the remote services
since there is no signal for announcing a new HealthApplication nor
the application knows the about HealthService, so the only way make
this to work is that the HealthApplication and HealthAgent leaves in
the same process but this defeats the point of having them separated.
So either we have the applications and services being announced to
everyone and then anyone can do the matching or we don't announce them
at all and make bluetoothd do the matching itself, I guess the latter
is better so we don't have to split the logic in many place (with
potential code duplication + bugs).
Another important thing is that dbus introspection does not have any
way to notify changes, so it really need to be signal to notify
applications about new services.
--
Luiz Augusto von Dentz
Computer Engineer
^ permalink raw reply
* Re: [PATCH] Fix NULL checking for pbap_get
From: Johan Hedberg @ 2010-08-03 9:54 UTC (permalink / raw)
To: Radoslaw Jablonski; +Cc: linux-bluetooth
In-Reply-To: <1280828987-15562-1-git-send-email-ext-jablonski.radoslaw@nokia.com>
Hi Radek,
On Tue, Aug 03, 2010, Radoslaw Jablonski wrote:
> Fixed NULL checking for name parameter - it shouldn't be there because
> handling for NULL name for some cases is already below. Also passing path
> which is NULL further makes no sense for PBAP so leaving with -EBADR if
> that kind of situation occurs after generating path.
> ---
> plugins/pbap.c | 5 ++++-
> 1 files changed, 4 insertions(+), 1 deletions(-)
Thanks. The patch is now upstream.
Johan
^ permalink raw reply
* [PATCH] Fix NULL checking for pbap_get
From: Radoslaw Jablonski @ 2010-08-03 9:49 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Radoslaw Jablonski
Fixed NULL checking for name parameter - it shouldn't be there because
handling for NULL name for some cases is already below. Also passing path
which is NULL further makes no sense for PBAP so leaving with -EBADR if
that kind of situation occurs after generating path.
---
plugins/pbap.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/plugins/pbap.c b/plugins/pbap.c
index 7422a1e..c008e95 100644
--- a/plugins/pbap.c
+++ b/plugins/pbap.c
@@ -555,7 +555,7 @@ static int pbap_get(struct obex_session *os, obex_object_t *obj,
DBG("name %s type %s pbap %p", name, type, pbap);
- if (type == NULL || name == NULL)
+ if (type == NULL)
return -EBADR;
rsize = obex_aparam_read(os, obj, &buffer);
@@ -594,6 +594,9 @@ static int pbap_get(struct obex_session *os, obex_object_t *obj,
} else
return -EBADR;
+ if (path == NULL)
+ return -EBADR;
+
pbap->params = params;
ret = obex_get_stream_start(os, path);
--
1.7.0.4
^ permalink raw reply related
* Re: What is the motivation for conn->power_save
From: Liang Bao @ 2010-08-03 9:32 UTC (permalink / raw)
To: Nick Pelly; +Cc: Andrei Emeltchenko, Marcel Holtmann, linux-bluetooth
In-Reply-To: <AANLkTinIhVG_4CpxLR1p1OqVeNqOzXojY3stnkJJdQAp@mail.gmail.com>
Hi,
2010/7/9 Nick Pelly <npelly@google.com>
>
> On Thu, Jul 8, 2010 at 8:07 AM, Andrei Emeltchenko
> <andrei.emeltchenko.news@gmail.com> wrote:
> > Hi,
> >
> > On Wed, Jan 13, 2010 at 10:04 AM, Marcel Holtmann <marcel@holtmann.org>=
wrote:
> >> Hi Nick,
> >>
> >>> >>> >> If I understand correctly, conn->power_save prevents the host =
stack
> >>> >>> >> from requesting active mode if it was not the host stack that
> >>> >>> >> requested sniff mode.
> >>> >>> >>
> >>> >>> >> I don't understand the motivation for this. If we have ACL dat=
a to
> >>> >>> >> send, then it seems like a good idea for the host stack to exp=
licitly
> >>> >>> >> request active mode, regardless of the reason that we entered =
sniff
> >>> >>> >> mode.
> >>> >>> >>
> >>> >>> >> We want to enter active mode more aggressively when setting up=
SCO
> >>> >>> >> connections, to avoid a 5 second delay with certain sniff mode=
s. But
> >>> >>> >> the conn->power_save code is getting in the way and doesn't ap=
pear to
> >>> >>> >> be useful in the first place.
> >>> >>> >
> >>> >>> > we have discussed this a few times. And if you lock through the=
code
> >>> >>> > history then you see that initially we just took devices out of=
sniff
> >>> >>> > mode if we had to send data. However with HID devices this fall=
s flat on
> >>> >>> > its face. They need to stay in control of sniff mode if they in=
itiated
> >>> >>> > it. Some of them crash and others just drain the battery. With =
sniff
> >>> >>> > mode you can send small amounts of data even while in sniff and=
for HID
> >>> >>> > that is sometimes used. So the remote side better not interfere=
.
> >>> >>> >
> >>> >>> > What we really need is a socket option where we can control thi=
s on a
> >>> >>> > per socket basis if we take devices out of sniff mode. And one =
extra
> >>> >>> > case might be when we try to establish a SCO channel, because t=
hen it is
> >>> >>> > clearly not an HID device. However even A2DP has this sort of p=
roblems
> >>> >>> > sometimes where the stream setup takes time.
> >>> >>>
> >>> >>> Makes sense. Thanks for the explanation.
> >>> >>
> >>> >> this means you will be working on a patch for this :)
> >>>
> >>> Actually, I want to put a patch together for a socket option to not
> >>> use power_save (so that we *always* exit sniff mode when sending ACL
> >>> data). We're seeing this problem with the Plantronics Voyager 855
> >>> which enters sniff mode during A2DP.
> >>>
> >>> Given that power_save is just for HID devices, my preferred design is
> >>> to disable power_save by default, and have an L2CAP socket option to
> >>> turn on power_save that would be used by HID L2CAP sockets.
> >>> Unfortunately this would require userspace HID code to use the new
> >>> socket option to keep current behavior. But it seems preferable to th=
e
> >>> alternative of having every single other L2CAP socket use a new socke=
t
> >>> option just to disable power_save for the sake of HID.
> >>
> >> actually you still mix up the meaning of the power_save option. From t=
he
> >> stack side, the automatic sniff mode is off by default. You have to
> >> enable via sysfs first. The power_save option is just to control when
> >> sniff mode is activated and the remote enters sniff mode, that we are
> >> not getting out of it if we have ACL data.
> >>
> >> In conclusion, I am fine with a socket option that allows to control
> >> sniff mode (preferable with interval details) and sniff subrate detail=
s
> >> so that we can use them for that ACL link.
> >>
> >> So go ahead and work on this. We can fix userspace easily since a new
> >> socket can be detected easily with newer kernels.
> >
> > We have found that some Nokia BT headsets stop working after idle
> > period when they enter
> > "power save" mode.
> >
> > Do we have any solution for this problem? So far I see good enough
> > patch :-)) below:
> >
> > http://android.git.kernel.org/?p=3Dkernel/common.git;a=3Dblobdiff;f=3Dn=
et/bluetooth/hci_conn.c;h=3Dfa8b412205cd89546b4a325c558c68040eeaf491;hp=3D0=
cf25114a3f576fc2788a549eb96d0087dd39b44;hb=3Dd8488237646920cd71ef43e5f3ae10=
01c6f4cf7b;hpb=3D3f68e5050c5ae559f56d5da9202cb88928d42b36
> >
> > - =A0 =A0 =A0 if (conn->mode !=3D HCI_CM_SNIFF || !conn->power_save)
> > + =A0 =A0 =A0 if (conn->mode !=3D HCI_CM_SNIFF /* || !conn->power_save =
*/)
> >
> > Nick have you done socket option for "power_save"?
>
> No. We'll do it once we need it for HID support.
Can we do this: try to do check against some existing fields in
structure hci_conn, for example device_class and then determine if
conn->power_save shall be used here? By doing this, applications in
user space can be saved from having knowledge how sniff mode works for
different devices.
Alternatively, is it possible to add another flag - I know the
structure is getting big so I agree adding something might not be a
good idea. :)
>
> Nick
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth=
" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at =A0http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: What is the motivation for conn->power_save
From: Liang Bao @ 2010-08-03 9:31 UTC (permalink / raw)
To: Nick Pelly; +Cc: Andrei Emeltchenko, Marcel Holtmann, linux-bluetooth
In-Reply-To: <AANLkTinIhVG_4CpxLR1p1OqVeNqOzXojY3stnkJJdQAp@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 5173 bytes --]
2010/7/9 Nick Pelly <npelly@google.com>
> On Thu, Jul 8, 2010 at 8:07 AM, Andrei Emeltchenko
> <andrei.emeltchenko.news@gmail.com> wrote:
> > Hi,
> >
> > On Wed, Jan 13, 2010 at 10:04 AM, Marcel Holtmann <marcel@holtmann.org>
> wrote:
> >> Hi Nick,
> >>
> >>> >>> >> If I understand correctly, conn->power_save prevents the host
> stack
> >>> >>> >> from requesting active mode if it was not the host stack that
> >>> >>> >> requested sniff mode.
> >>> >>> >>
> >>> >>> >> I don't understand the motivation for this. If we have ACL data
> to
> >>> >>> >> send, then it seems like a good idea for the host stack to
> explicitly
> >>> >>> >> request active mode, regardless of the reason that we entered
> sniff
> >>> >>> >> mode.
> >>> >>> >>
> >>> >>> >> We want to enter active mode more aggressively when setting up
> SCO
> >>> >>> >> connections, to avoid a 5 second delay with certain sniff modes.
> But
> >>> >>> >> the conn->power_save code is getting in the way and doesn't
> appear to
> >>> >>> >> be useful in the first place.
> >>> >>> >
> >>> >>> > we have discussed this a few times. And if you lock through the
> code
> >>> >>> > history then you see that initially we just took devices out of
> sniff
> >>> >>> > mode if we had to send data. However with HID devices this falls
> flat on
> >>> >>> > its face. They need to stay in control of sniff mode if they
> initiated
> >>> >>> > it. Some of them crash and others just drain the battery. With
> sniff
> >>> >>> > mode you can send small amounts of data even while in sniff and
> for HID
> >>> >>> > that is sometimes used. So the remote side better not interfere.
> >>> >>> >
> >>> >>> > What we really need is a socket option where we can control this
> on a
> >>> >>> > per socket basis if we take devices out of sniff mode. And one
> extra
> >>> >>> > case might be when we try to establish a SCO channel, because
> then it is
> >>> >>> > clearly not an HID device. However even A2DP has this sort of
> problems
> >>> >>> > sometimes where the stream setup takes time.
> >>> >>>
> >>> >>> Makes sense. Thanks for the explanation.
> >>> >>
> >>> >> this means you will be working on a patch for this :)
> >>>
> >>> Actually, I want to put a patch together for a socket option to not
> >>> use power_save (so that we *always* exit sniff mode when sending ACL
> >>> data). We're seeing this problem with the Plantronics Voyager 855
> >>> which enters sniff mode during A2DP.
> >>>
> >>> Given that power_save is just for HID devices, my preferred design is
> >>> to disable power_save by default, and have an L2CAP socket option to
> >>> turn on power_save that would be used by HID L2CAP sockets.
> >>> Unfortunately this would require userspace HID code to use the new
> >>> socket option to keep current behavior. But it seems preferable to the
> >>> alternative of having every single other L2CAP socket use a new socket
> >>> option just to disable power_save for the sake of HID.
> >>
> >> actually you still mix up the meaning of the power_save option. From the
> >> stack side, the automatic sniff mode is off by default. You have to
> >> enable via sysfs first. The power_save option is just to control when
> >> sniff mode is activated and the remote enters sniff mode, that we are
> >> not getting out of it if we have ACL data.
> >>
> >> In conclusion, I am fine with a socket option that allows to control
> >> sniff mode (preferable with interval details) and sniff subrate details
> >> so that we can use them for that ACL link.
> >>
> >> So go ahead and work on this. We can fix userspace easily since a new
> >> socket can be detected easily with newer kernels.
> >
> > We have found that some Nokia BT headsets stop working after idle
> > period when they enter
> > "power save" mode.
> >
> > Do we have any solution for this problem? So far I see good enough
> > patch :-)) below:
> >
> >
> http://android.git.kernel.org/?p=kernel/common.git;a=blobdiff;f=net/bluetooth/hci_conn.c;h=fa8b412205cd89546b4a325c558c68040eeaf491;hp=0cf25114a3f576fc2788a549eb96d0087dd39b44;hb=d8488237646920cd71ef43e5f3ae1001c6f4cf7b;hpb=3f68e5050c5ae559f56d5da9202cb88928d42b36
> >
> > - if (conn->mode != HCI_CM_SNIFF || !conn->power_save)
> > + if (conn->mode != HCI_CM_SNIFF /* || !conn->power_save */)
> >
> > Nick have you done socket option for "power_save"?
>
> No. We'll do it once we need it for HID support.
>
Can we do this: try to do check against some existing fields in structure
hci_conn, for example device_class and then determine if conn->power_save
shall be used here? By doing this, applications in user space can be saved
from having knowledge how sniff mode works for different devices.
Alternatively, is it possible to add another flag - I know the structure is
getting big so I agree adding something might not be a good idea. :)
>
> Nick
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth"
> in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
[-- Attachment #2: Type: text/html, Size: 7334 bytes --]
^ permalink raw reply
* Re: [PATCH] Notify to device drivers when the SDP records change
From: José Antonio Santos Cadenas @ 2010-08-03 9:12 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <AANLkTi=iNPgNJaUpUK2v=whbWs_nzCpwq5TUbbDKk-7H@mail.gmail.com>
Hi,
El Tuesday 03 August 2010 11:00:07 Luiz Augusto von Dentz escribió:
> Hi,
>
> On Tue, Aug 3, 2010 at 10:30 AM, José Antonio Santos Cadenas
>
> <santoscadenas@gmail.com> wrote:
> > Hi Luiz,
> >
> > El Monday 02 August 2010 16:39:18 Luiz Augusto von Dentz escribió:
> >> Hi,
> >>
> >> On Mon, Jul 26, 2010 at 10:55 AM, Jose Antonio Santos Cadenas
> >>
> >> <santoscadenas@gmail.com> wrote:
> >> > When the remote SDP records that make a driver to be loaded change
> >> > (added, remove, or updated) And the driver is not going to be
> >> > removed, a notification is sent to the device driver. This
> >> > notification is optional annnd does not fail if the driver does
> >> > not implemment it.
> >>
> >> How this is supposed to work?
> >
> > This tries to be a generic way to update the remote services. When we
> > planned the HDP API some weeks ago in Recife, we decided that we need a
> > way to update the services that a device offers (this implies to make
> > some calls to the HealtAgent). Marcel and Johan tell me that it will be
> > better to implement it in a generic way using a generic update services.
> > May be I misunderstood something or this is not the best way to do this,
> > but that's the objective. The objective is to provide an API to the user
> > that notifies them about new health services.
> >
> > Can you see better ways to do this? We are open to any new ideas.
> >
> >> If does seems to me that you guys are
> >> planning to keep polling the services records to update the drivers
> >> which IMO is not a good idea, DiscoverServices is not meant to update
> >> the drivers either it is basically a replacement to sdptool to so
> >> application can do it over dbus.
> >
> > Of course I am not trying to poll remote SDP nor having it cached. Just
> > offering a way to update services for the user.
> >
> >> Also if you take a closer look in the
> >> plugins that use rfcomm channel, which can be changed at any point,
> >> they basically fetch the record when attempting to connect, avdtp also
> >> work in a similar way, we have to do the discover + get capabilities
> >> every time we want to connect to a sink/source since there is no way
> >> to keep this information updated locally.
> >
> > Of course, every time that you are going to connect is when you check the
> > remote record. In the HDP implementation is doing the same way.
> >
> >> In the other hand, this can be useful for LE which we can subscribe to
> >> changes, but for sdp I don't see the point.
> >
> > For HDP is also useful in SDP, because the device can update services and
> > the device driver will not receive any notification. This way we are
> > offering a way to update it when the user wants. This is almost the same
> > than making a service discovery and notify a new UUID discovered, but
> > also notifying the changes in the records.
>
> But you must do the discover every time you want to connect, as you
> state there is no notification for SDP, so updating the drivers
> doesn't guarantee anything. Application should not know about this,
> they basically want to connect and bluetoothd has to figure out which
> sink/source do match for them.
>
> So IMO this has to be done similar to a2dp/avdtp, prior to connect
> discover the endpoints and them proceed with the connection attempt,
> the stored records are just a hint of what the remote device supports.
The different with a2dp is that the application will decide the end point to
connect to, because there can be more than one matching the source. This end
point is represented by a path that is passed to the HealthAgent when is
discovered. If the remote record changes it is possible that new end points
are available so the application needs something to discover the available end
points. That is why I think that we need something like this.
Regards.
^ permalink raw reply
* Re: [PATCH] Notify to device drivers when the SDP records change
From: Luiz Augusto von Dentz @ 2010-08-03 9:00 UTC (permalink / raw)
To: José Antonio Santos Cadenas; +Cc: linux-bluetooth
In-Reply-To: <201008030930.45330.santoscadenas@gmail.com>
Hi,
On Tue, Aug 3, 2010 at 10:30 AM, José Antonio Santos Cadenas
<santoscadenas@gmail.com> wrote:
> Hi Luiz,
>
> El Monday 02 August 2010 16:39:18 Luiz Augusto von Dentz escribió:
>> Hi,
>>
>> On Mon, Jul 26, 2010 at 10:55 AM, Jose Antonio Santos Cadenas
>>
>> <santoscadenas@gmail.com> wrote:
>> > When the remote SDP records that make a driver to be loaded change
>> > (added, remove, or updated) And the driver is not going to be
>> > removed, a notification is sent to the device driver. This
>> > notification is optional annnd does not fail if the driver does
>> > not implemment it.
>>
>> How this is supposed to work?
>
> This tries to be a generic way to update the remote services. When we planned
> the HDP API some weeks ago in Recife, we decided that we need a way to update
> the services that a device offers (this implies to make some calls to the
> HealtAgent). Marcel and Johan tell me that it will be better to implement it
> in a generic way using a generic update services. May be I misunderstood
> something or this is not the best way to do this, but that's the objective.
> The objective is to provide an API to the user that notifies them about new
> health services.
>
> Can you see better ways to do this? We are open to any new ideas.
>
>
>> If does seems to me that you guys are
>> planning to keep polling the services records to update the drivers
>> which IMO is not a good idea, DiscoverServices is not meant to update
>> the drivers either it is basically a replacement to sdptool to so
>> application can do it over dbus.
>
> Of course I am not trying to poll remote SDP nor having it cached. Just
> offering a way to update services for the user.
>
>
>> Also if you take a closer look in the
>> plugins that use rfcomm channel, which can be changed at any point,
>> they basically fetch the record when attempting to connect, avdtp also
>> work in a similar way, we have to do the discover + get capabilities
>> every time we want to connect to a sink/source since there is no way
>> to keep this information updated locally.
>
> Of course, every time that you are going to connect is when you check the
> remote record. In the HDP implementation is doing the same way.
>
>>
>> In the other hand, this can be useful for LE which we can subscribe to
>> changes, but for sdp I don't see the point.
>
> For HDP is also useful in SDP, because the device can update services and the
> device driver will not receive any notification. This way we are offering a
> way to update it when the user wants. This is almost the same than making a
> service discovery and notify a new UUID discovered, but also notifying the
> changes in the records.
But you must do the discover every time you want to connect, as you
state there is no notification for SDP, so updating the drivers
doesn't guarantee anything. Application should not know about this,
they basically want to connect and bluetoothd has to figure out which
sink/source do match for them.
So IMO this has to be done similar to a2dp/avdtp, prior to connect
discover the endpoints and them proceed with the connection attempt,
the stored records are just a hint of what the remote device supports.
--
Luiz Augusto von Dentz
Computer Engineer
^ permalink raw reply
* Re: [PATCH] Add NULL checking for name and path params
From: Johan Hedberg @ 2010-08-03 8:42 UTC (permalink / raw)
To: Radoslaw Jablonski; +Cc: linux-bluetooth
In-Reply-To: <1280823784-14551-1-git-send-email-ext-jablonski.radoslaw@nokia.com>
Hi Radek,
On Tue, Aug 03, 2010, Radoslaw Jablonski wrote:
> Added NULL checking for "name" and "path" parameters in pbap_open_*
> functions. If this variables will be NULL, then setting -EBADR error
> code and ending function. This fixes many obexd crashes.
> ---
> plugins/pbap.c | 15 ++++++++++++++-
> 1 files changed, 14 insertions(+), 1 deletions(-)
Please update your tree against latest upstream. Your patch doesn't
apply anymore (it seems like a new version of the previous one, but that
one got already pushed upstream). Also fix the commit message to explain
what issue in the previous patch this one fixes.
Johan
^ permalink raw reply
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