[PATCH v4] bluetooth: hci_bcm: enable IRQ capability from node

[PATCH v4] bluetooth: hci_bcm: enable IRQ capability from node

[Guillaume La Roque]

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

Re: [PATCH v4] bluetooth: hci_bcm: enable IRQ capability from node

[Johan Hovold]

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

Re: [PATCH v4] bluetooth: hci_bcm: enable IRQ capability from node

[guillaume La Roque]

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

Re: [PATCH v4] bluetooth: hci_bcm: enable IRQ capability from node

[Johan Hovold]

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

Re: [PATCH v4] bluetooth: hci_bcm: enable IRQ capability from node

[guillaume La Roque]

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

Re: [PATCH v4] bluetooth: hci_bcm: enable IRQ capability from node

[Marcel Holtmann]

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

Re: [PATCH v4] bluetooth: hci_bcm: enable IRQ capability from node

[guillaume La Roque]

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

Re: [PATCH v4] bluetooth: hci_bcm: enable IRQ capability from node

[Marcel Holtmann]

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