2011-10-11 18:58:53

by Gustavo Padovan

[permalink] [raw]
Subject: [PATCH] Bluetooth: Fix permission of enable_le param

From: "Gustavo F. Padovan" <[email protected]>

With 0444 it is impossible to change the param, changing it to 0644.

Signed-off-by: Gustavo F. Padovan <[email protected]>
---
net/bluetooth/hci_event.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index d7d96b6..0e57634 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3104,5 +3104,5 @@ void hci_si_event(struct hci_dev *hdev, int type, int dlen, void *data)
kfree_skb(skb);
}

-module_param(enable_le, bool, 0444);
+module_param(enable_le, bool, 0644);
MODULE_PARM_DESC(enable_le, "Enable LE support");
--
1.7.6.4



2011-10-13 17:25:44

by Vinicius Costa Gomes

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: Fix permission of enable_le param

Hi Lizardo,

On 16:28 Tue 11 Oct, Anderson Lizardo wrote:
> Hi,
>
> On Tue, Oct 11, 2011 at 3:33 PM, Claudio Takahasi
> <[email protected]> wrote:
> > On Tue, Oct 11, 2011 at 3:58 PM, Gustavo F. Padovan
> > <[email protected]> wrote:
> >> With 0444 it is impossible to change the param, changing it to 0644.
> >
> > If you allow changing the "enable_le" parameter dynamically it is also
> > necessary check the controller features and update the LE host
> > supported bit(if necessary).
> > I am afraid that allowing to change it dynamically will break the
> > consistency: what is gonna happen with the active operations(discovery
> > & connections)?
> >
> > In our backlog we were planning to check LE host supported bit to
> > allow/block LE connections or any other feature related to LE.
>
> For me, it makes more sense to have bluez set LE host supported bit on
> start, and have no "enable_le" module parameter at all.

While I agree with this, IMHO we are not ready for it yet, I would wait
for all the major functionalities to be integrated before removing it.

And there's the minor point that sometimes for testing and qualification
you have the hardware but don't want to have LE enabled.

>
> I think just allowing to change enable_le after the module has been
> loaded will not work as expected.
>
> Regards,
> --
> Anderson Lizardo
> Instituto Nokia de Tecnologia - INdT
> Manaus - Brazil
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html


Cheers,
--
Vinicius

2011-10-11 20:28:51

by Anderson Lizardo

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: Fix permission of enable_le param

Hi,

On Tue, Oct 11, 2011 at 3:33 PM, Claudio Takahasi
<[email protected]> wrote:
> On Tue, Oct 11, 2011 at 3:58 PM, Gustavo F. Padovan
> <[email protected]> wrote:
>> With 0444 it is impossible to change the param, changing it to 0644.
>
> If you allow changing the "enable_le" parameter dynamically it is also
> necessary check the controller features and update the LE host
> supported bit(if necessary).
> I am afraid that allowing to change it dynamically will break the
> consistency: what is gonna happen with the active operations(discovery
> & connections)?
>
> In our backlog we were planning to check LE host supported bit to
> allow/block LE connections or any other feature related to LE.

For me, it makes more sense to have bluez set LE host supported bit on
start, and have no "enable_le" module parameter at all.

I think just allowing to change enable_le after the module has been
loaded will not work as expected.

Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

2011-10-11 19:33:44

by Claudio Takahasi

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: Fix permission of enable_le param

Hi Gustavo,

On Tue, Oct 11, 2011 at 3:58 PM, Gustavo F. Padovan
<[email protected]> wrote:
> From: "Gustavo F. Padovan" <[email protected]>
>
> With 0444 it is impossible to change the param, changing it to 0644.
>
> Signed-off-by: Gustavo F. Padovan <[email protected]>
> ---
>  net/bluetooth/hci_event.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index d7d96b6..0e57634 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -3104,5 +3104,5 @@ void hci_si_event(struct hci_dev *hdev, int type, int dlen, void *data)
>        kfree_skb(skb);
>  }
>
> -module_param(enable_le, bool, 0444);
> +module_param(enable_le, bool, 0644);
>  MODULE_PARM_DESC(enable_le, "Enable LE support");
> --
> 1.7.6.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

If you allow changing the "enable_le" parameter dynamically it is also
necessary check the controller features and update the LE host
supported bit(if necessary).
I am afraid that allowing to change it dynamically will break the
consistency: what is gonna happen with the active operations(discovery
& connections)?

In our backlog we were planning to check LE host supported bit to
allow/block LE connections or any other feature related to LE.

BR,
Claudio.