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 <[email protected]>
---
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
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 <[email protected]>
> ---
> 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
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 <[email protected]>
>> ---
>> 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
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
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
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
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
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