linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4] bluetooth: hci_bcm: enable IRQ capability from node
@ 2019-12-13 10:55 Guillaume La Roque
  2019-12-13 11:17 ` Johan Hovold
  0 siblings, 1 reply; 8+ messages in thread
From: Guillaume La Roque @ 2019-12-13 10:55 UTC (permalink / raw)
  To: marcel, johan.hedberg, linux-bluetooth
  Cc: nsaenzjulienne, linux-kernel, khilman

Actually IRQ can be found from GPIO but all platforms don't support
gpiod_to_irq, it's the case on amlogic chip.
so to have possibility to use interrupt mode we need to add interrupts
field in node and support it in driver.

Signed-off-by: Guillaume La Roque <glaroque@baylibre.com>
---
 drivers/bluetooth/hci_bcm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
index f8f5c593a05c..aa194f8d703e 100644
--- a/drivers/bluetooth/hci_bcm.c
+++ b/drivers/bluetooth/hci_bcm.c
@@ -13,6 +13,7 @@
 #include <linux/module.h>
 #include <linux/acpi.h>
 #include <linux/of.h>
+#include <linux/of_irq.h>
 #include <linux/property.h>
 #include <linux/platform_data/x86/apple.h>
 #include <linux/platform_device.h>
@@ -1421,6 +1422,7 @@ static int bcm_serdev_probe(struct serdev_device *serdev)
 #endif
 	bcmdev->serdev_hu.serdev = serdev;
 	serdev_device_set_drvdata(serdev, bcmdev);
+	bcmdev->irq = of_irq_get(bcmdev->dev->of_node, 0);
 
 	/* Initialize routing field to an unused value */
 	bcmdev->pcm_int_params[0] = 0xff;
-- 
2.17.1


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

* Re: [PATCH v4] bluetooth: hci_bcm: enable IRQ capability from node
  2019-12-13 10:55 [PATCH v4] bluetooth: hci_bcm: enable IRQ capability from node Guillaume La Roque
@ 2019-12-13 11:17 ` Johan Hovold
  2019-12-13 12:31   ` guillaume La Roque
  0 siblings, 1 reply; 8+ messages in thread
From: Johan Hovold @ 2019-12-13 11:17 UTC (permalink / raw)
  To: Guillaume La Roque
  Cc: marcel, johan.hedberg, linux-bluetooth, nsaenzjulienne,
	linux-kernel, khilman

On Fri, Dec 13, 2019 at 11:55:21AM +0100, Guillaume La Roque wrote:
> Actually IRQ can be found from GPIO but all platforms don't support
> gpiod_to_irq, it's the case on amlogic chip.
> so to have possibility to use interrupt mode we need to add interrupts
> field in node and support it in driver.

"node" is a bit vague, please refer to devicetree here and in the patch
summary.

> Signed-off-by: Guillaume La Roque <glaroque@baylibre.com>
> ---
>  drivers/bluetooth/hci_bcm.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
> index f8f5c593a05c..aa194f8d703e 100644
> --- a/drivers/bluetooth/hci_bcm.c
> +++ b/drivers/bluetooth/hci_bcm.c
> @@ -13,6 +13,7 @@
>  #include <linux/module.h>
>  #include <linux/acpi.h>
>  #include <linux/of.h>
> +#include <linux/of_irq.h>
>  #include <linux/property.h>
>  #include <linux/platform_data/x86/apple.h>
>  #include <linux/platform_device.h>
> @@ -1421,6 +1422,7 @@ static int bcm_serdev_probe(struct serdev_device *serdev)
>  #endif
>  	bcmdev->serdev_hu.serdev = serdev;
>  	serdev_device_set_drvdata(serdev, bcmdev);
> +	bcmdev->irq = of_irq_get(bcmdev->dev->of_node, 0);

And this clearly makes no sense for acpi, so you'd need to add it to
bcm_of_probe().

Shouldn't you be used using of_irq_get_byname()?

And since you're extending the binding, I think you need a preparatory
patch documenting this in:

	Documentation/devicetree/bindings/net/broadcom-bluetooth.txt

>  	/* Initialize routing field to an unused value */
>  	bcmdev->pcm_int_params[0] = 0xff;

Johan

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

* Re: [PATCH v4] bluetooth: hci_bcm: enable IRQ capability from node
  2019-12-13 11:17 ` Johan Hovold
@ 2019-12-13 12:31   ` guillaume La Roque
  2019-12-13 13:44     ` Johan Hovold
  0 siblings, 1 reply; 8+ messages in thread
From: guillaume La Roque @ 2019-12-13 12:31 UTC (permalink / raw)
  To: Johan Hovold
  Cc: marcel, johan.hedberg, linux-bluetooth, nsaenzjulienne,
	linux-kernel, khilman

Hi Johan,

On 12/13/19 12:17 PM, Johan Hovold wrote:
> On Fri, Dec 13, 2019 at 11:55:21AM +0100, Guillaume La Roque wrote:
>> Actually IRQ can be found from GPIO but all platforms don't support
>> gpiod_to_irq, it's the case on amlogic chip.
>> so to have possibility to use interrupt mode we need to add interrupts
>> field in node and support it in driver.
> "node" is a bit vague, please refer to devicetree here and in the patch
> summary.
>
 i will
>> Signed-off-by: Guillaume La Roque <glaroque@baylibre.com>
>> ---
>>  drivers/bluetooth/hci_bcm.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
>> index f8f5c593a05c..aa194f8d703e 100644
>> --- a/drivers/bluetooth/hci_bcm.c
>> +++ b/drivers/bluetooth/hci_bcm.c
>> @@ -13,6 +13,7 @@
>>  #include <linux/module.h>
>>  #include <linux/acpi.h>
>>  #include <linux/of.h>
>> +#include <linux/of_irq.h>
>>  #include <linux/property.h>
>>  #include <linux/platform_data/x86/apple.h>
>>  #include <linux/platform_device.h>
>> @@ -1421,6 +1422,7 @@ static int bcm_serdev_probe(struct serdev_device *serdev)
>>  #endif
>>  	bcmdev->serdev_hu.serdev = serdev;
>>  	serdev_device_set_drvdata(serdev, bcmdev);
>> +	bcmdev->irq = of_irq_get(bcmdev->dev->of_node, 0);
> And this clearly makes no sense for acpi, so you'd need to add it to
> bcm_of_probe().
>
> Shouldn't you be used using of_irq_get_byname()?
i can use it if you prefer but no other interrupt need to be defined
>
> And since you're extending the binding, I think you need a preparatory
> patch documenting this in:
>
> 	Documentation/devicetree/bindings/net/broadcom-bluetooth.txt
sorry i forgot this, i will do
>>  	/* Initialize routing field to an unused value */
>>  	bcmdev->pcm_int_params[0] = 0xff;
> Johan


thanks

Guillaume


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

* Re: [PATCH v4] bluetooth: hci_bcm: enable IRQ capability from node
  2019-12-13 12:31   ` guillaume La Roque
@ 2019-12-13 13:44     ` Johan Hovold
  2019-12-13 14:07       ` guillaume La Roque
  0 siblings, 1 reply; 8+ messages in thread
From: Johan Hovold @ 2019-12-13 13:44 UTC (permalink / raw)
  To: guillaume La Roque
  Cc: Johan Hovold, marcel, johan.hedberg, linux-bluetooth,
	nsaenzjulienne, linux-kernel, khilman

On Fri, Dec 13, 2019 at 01:31:18PM +0100, guillaume La Roque wrote:
> Hi Johan,
> 
> On 12/13/19 12:17 PM, Johan Hovold wrote:
> > On Fri, Dec 13, 2019 at 11:55:21AM +0100, Guillaume La Roque wrote:

> >> @@ -1421,6 +1422,7 @@ static int bcm_serdev_probe(struct serdev_device *serdev)
> >>  #endif
> >>  	bcmdev->serdev_hu.serdev = serdev;
> >>  	serdev_device_set_drvdata(serdev, bcmdev);
> >> +	bcmdev->irq = of_irq_get(bcmdev->dev->of_node, 0);

> > Shouldn't you be used using of_irq_get_byname()?

> i can use it if you prefer but no other interrupt need to be defined

Maybe not needed then. Was just thinking it may make it more clear that
you now have two ways to specify the "host-wakeup" interrupt (and in
your proposed implementation the interrupts-property happens to take
priority). Perhaps that can be sorted out when you submit the binding
update for review.

Johan

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

* Re: [PATCH v4] bluetooth: hci_bcm: enable IRQ capability from node
  2019-12-13 13:44     ` Johan Hovold
@ 2019-12-13 14:07       ` guillaume La Roque
  2020-01-04  9:58         ` Marcel Holtmann
  0 siblings, 1 reply; 8+ messages in thread
From: guillaume La Roque @ 2019-12-13 14:07 UTC (permalink / raw)
  To: Johan Hovold
  Cc: marcel, johan.hedberg, linux-bluetooth, nsaenzjulienne,
	linux-kernel, khilman


On 12/13/19 2:44 PM, Johan Hovold wrote:
> On Fri, Dec 13, 2019 at 01:31:18PM +0100, guillaume La Roque wrote:
>> Hi Johan,
>>
>> On 12/13/19 12:17 PM, Johan Hovold wrote:
>>> On Fri, Dec 13, 2019 at 11:55:21AM +0100, Guillaume La Roque wrote:
>>>> @@ -1421,6 +1422,7 @@ static int bcm_serdev_probe(struct serdev_device *serdev)
>>>>  #endif
>>>>  	bcmdev->serdev_hu.serdev = serdev;
>>>>  	serdev_device_set_drvdata(serdev, bcmdev);
>>>> +	bcmdev->irq = of_irq_get(bcmdev->dev->of_node, 0);
>>> Shouldn't you be used using of_irq_get_byname()?
>> i can use it if you prefer but no other interrupt need to be defined
> Maybe not needed then. Was just thinking it may make it more clear that
> you now have two ways to specify the "host-wakeup" interrupt (and in
> your proposed implementation the interrupts-property happens to take
> priority). Perhaps that can be sorted out when you submit the binding
> update for review.

no problem i add a "host-wakeup" interrupt-name.
you are right it will be more clear with name and we know why this interrupt is needed.

> Johan

thanks

Guillaume


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

* Re: [PATCH v4] bluetooth: hci_bcm: enable IRQ capability from node
  2019-12-13 14:07       ` guillaume La Roque
@ 2020-01-04  9:58         ` Marcel Holtmann
  2020-01-07 12:33           ` guillaume La Roque
  0 siblings, 1 reply; 8+ messages in thread
From: Marcel Holtmann @ 2020-01-04  9:58 UTC (permalink / raw)
  To: Guillaume La Roque
  Cc: Johan Hovold, Johan Hedberg, BlueZ devel list, nsaenzjulienne,
	linux-kernel, khilman

Hi Guillaume,

>>>>> @@ -1421,6 +1422,7 @@ static int bcm_serdev_probe(struct serdev_device *serdev)
>>>>> #endif
>>>>> 	bcmdev->serdev_hu.serdev = serdev;
>>>>> 	serdev_device_set_drvdata(serdev, bcmdev);
>>>>> +	bcmdev->irq = of_irq_get(bcmdev->dev->of_node, 0);
>>>> Shouldn't you be used using of_irq_get_byname()?
>>> i can use it if you prefer but no other interrupt need to be defined
>> Maybe not needed then. Was just thinking it may make it more clear that
>> you now have two ways to specify the "host-wakeup" interrupt (and in
>> your proposed implementation the interrupts-property happens to take
>> priority). Perhaps that can be sorted out when you submit the binding
>> update for review.
> 
> no problem i add a "host-wakeup" interrupt-name.
> you are right it will be more clear with name and we know why this interrupt is needed.

have I missed the v5 or are still sending it?

Regards

Marcel


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

* Re: [PATCH v4] bluetooth: hci_bcm: enable IRQ capability from node
  2020-01-04  9:58         ` Marcel Holtmann
@ 2020-01-07 12:33           ` guillaume La Roque
  2020-01-08 20:53             ` Marcel Holtmann
  0 siblings, 1 reply; 8+ messages in thread
From: guillaume La Roque @ 2020-01-07 12:33 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Johan Hovold, Johan Hedberg, BlueZ devel list, nsaenzjulienne,
	linux-kernel, khilman

Hi Marcel,

On 1/4/20 10:58 AM, Marcel Holtmann wrote:
> Hi Guillaume,
>
>>>>>> @@ -1421,6 +1422,7 @@ static int bcm_serdev_probe(struct serdev_device *serdev)
>>>>>> #endif
>>>>>> 	bcmdev->serdev_hu.serdev = serdev;
>>>>>> 	serdev_device_set_drvdata(serdev, bcmdev);
>>>>>> +	bcmdev->irq = of_irq_get(bcmdev->dev->of_node, 0);
>>>>> Shouldn't you be used using of_irq_get_byname()?
>>>> i can use it if you prefer but no other interrupt need to be defined
>>> Maybe not needed then. Was just thinking it may make it more clear that
>>> you now have two ways to specify the "host-wakeup" interrupt (and in
>>> your proposed implementation the interrupts-property happens to take
>>> priority). Perhaps that can be sorted out when you submit the binding
>>> update for review.
>> no problem i add a "host-wakeup" interrupt-name.
>> you are right it will be more clear with name and we know why this interrupt is needed.
> have I missed the v5 or are still sending it?

sorry i was in chrismas holidays .

v5 was sent before holiday and you comment it [1] ;) , on v5 you ask me to send v6 with tag.


Regards

Guillaume

>
> Regards
>
> Marcel
>
[1] : https://www.spinics.net/lists/linux-bluetooth/msg82424.html

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

* Re: [PATCH v4] bluetooth: hci_bcm: enable IRQ capability from node
  2020-01-07 12:33           ` guillaume La Roque
@ 2020-01-08 20:53             ` Marcel Holtmann
  0 siblings, 0 replies; 8+ messages in thread
From: Marcel Holtmann @ 2020-01-08 20:53 UTC (permalink / raw)
  To: guillaume La Roque
  Cc: Johan Hovold, Johan Hedberg, BlueZ devel list, nsaenzjulienne,
	linux-kernel, khilman

Hi Guillaume,

>>>>>>> @@ -1421,6 +1422,7 @@ static int bcm_serdev_probe(struct serdev_device *serdev)
>>>>>>> #endif
>>>>>>> 	bcmdev->serdev_hu.serdev = serdev;
>>>>>>> 	serdev_device_set_drvdata(serdev, bcmdev);
>>>>>>> +	bcmdev->irq = of_irq_get(bcmdev->dev->of_node, 0);
>>>>>> Shouldn't you be used using of_irq_get_byname()?
>>>>> i can use it if you prefer but no other interrupt need to be defined
>>>> Maybe not needed then. Was just thinking it may make it more clear that
>>>> you now have two ways to specify the "host-wakeup" interrupt (and in
>>>> your proposed implementation the interrupts-property happens to take
>>>> priority). Perhaps that can be sorted out when you submit the binding
>>>> update for review.
>>> no problem i add a "host-wakeup" interrupt-name.
>>> you are right it will be more clear with name and we know why this interrupt is needed.
>> have I missed the v5 or are still sending it?
> 
> sorry i was in chrismas holidays .
> 
> v5 was sent before holiday and you comment it [1] ;) , on v5 you ask me to send v6 with tag.

ok, then I am waiting for v6.

Regards

Marcel


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

end of thread, other threads:[~2020-01-08 20:53 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-13 10:55 [PATCH v4] bluetooth: hci_bcm: enable IRQ capability from node Guillaume La Roque
2019-12-13 11:17 ` Johan Hovold
2019-12-13 12:31   ` guillaume La Roque
2019-12-13 13:44     ` Johan Hovold
2019-12-13 14:07       ` guillaume La Roque
2020-01-04  9:58         ` Marcel Holtmann
2020-01-07 12:33           ` guillaume La Roque
2020-01-08 20:53             ` Marcel Holtmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).