[PATCH 0/9] Bluetooth: hci_bcm: Add (runtime)pm support to the serdev drv

[PATCH 0/9] Bluetooth: hci_bcm: Add (runtime)pm support to the serdev drv

[Hans de Goede]

Hi All,

The ACPI subsys is going to move over to instantiating ACPI enumerated
HCIs as serdevs, rather then as platform devices.

This means that the serdev driver paths of hci_bcm.c also need to start
supporting (runtime)pm through GPIOs and a host-wake IRQ.

This series makes most of the code in hci_bcm.c indepdenent from how
the hci was instantiated and then more or less unifies the platform
and serdev drivers, adding (runtime)pm / GPIO / IRQ support to the
serdev driver.

Besides adding (runtime)pm support, GPIO control in general is necessary
to e.g. keep the ACPI BCM2E7E id used with the BCM4356A2 wifi/bt combo
on the GPD pocket working after moving over to serdev enumeration.

I've chosen to keep the platform enumeration based code paths in place
for now. This way we can first merge this series and then, once this
series is in place, the first 2 patches of Frédéric Danis RFC series
for ACPI serdev support can be merged without causing any regressions.

Once the ACPI serdev support is merged a follow up patch removing the
platform-device related code can be submitted.

Note the 2nd patch in this series is a resend of a bug-fix I submitted
a couple of days ago, I've included that with the series to avoid
conflicts when applying the series without that fix.

Regards,

Hans

[PATCH 1/9] Bluetooth: hci_uart_set_flow_control: Fix NULL deref when using serdev

[Hans de Goede]

Fix a NULL pointer deref (hu->tty) when calling hci_uart_set_flow_control
on hci_uart-s using serdev.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/bluetooth/hci_ldisc.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index a746627e784e..d02f6d029093 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -41,6 +41,7 @@
 #include <linux/ioctl.h>
 #include <linux/skbuff.h>
 #include <linux/firmware.h>
+#include <linux/serdev.h>
 
 #include <net/bluetooth/bluetooth.h>
 #include <net/bluetooth/hci_core.h>
@@ -298,6 +299,11 @@ void hci_uart_set_flow_control(struct hci_uart *hu, bool enable)
 	unsigned int set = 0;
 	unsigned int clear = 0;
 
+	if (hu->serdev) {
+		serdev_device_set_flow_control(hu->serdev, !enable);
+		return;
+	}
+
 	if (enable) {
 		/* Disable hardware flow control */
 		ktermios = tty->termios;
-- 
2.14.2

Re: [PATCH 1/9] Bluetooth: hci_uart_set_flow_control: Fix NULL deref when using serdev

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1279 bytes --]

Hi,

On Mon, Oct 02, 2017 at 05:23:48PM +0200, Hans de Goede wrote:
> Fix a NULL pointer deref (hu->tty) when calling hci_uart_set_flow_control
> on hci_uart-s using serdev.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/bluetooth/hci_ldisc.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
> index a746627e784e..d02f6d029093 100644
> --- a/drivers/bluetooth/hci_ldisc.c
> +++ b/drivers/bluetooth/hci_ldisc.c
> @@ -41,6 +41,7 @@
>  #include <linux/ioctl.h>
>  #include <linux/skbuff.h>
>  #include <linux/firmware.h>
> +#include <linux/serdev.h>
>  
>  #include <net/bluetooth/bluetooth.h>
>  #include <net/bluetooth/hci_core.h>
> @@ -298,6 +299,11 @@ void hci_uart_set_flow_control(struct hci_uart *hu, bool enable)
>  	unsigned int set = 0;
>  	unsigned int clear = 0;
>  
> +	if (hu->serdev) {
> +		serdev_device_set_flow_control(hu->serdev, !enable);
> +		return;
> +	}

I think this should also control rts, so that the behaviour is the
same for serdev and ldisc implementation:

serdev_device_set_rts(serdev, enable);

> +
>  	if (enable) {
>  		/* Disable hardware flow control */
>  		ktermios = tty->termios;

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/9] Bluetooth: hci_uart_set_flow_control: Fix NULL deref when using serdev

[Hans de Goede]

Hi,

On 03-10-17 00:34, Sebastian Reichel wrote:
> Hi,
> 
> On Mon, Oct 02, 2017 at 05:23:48PM +0200, Hans de Goede wrote:
>> Fix a NULL pointer deref (hu->tty) when calling hci_uart_set_flow_control
>> on hci_uart-s using serdev.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   drivers/bluetooth/hci_ldisc.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
>> index a746627e784e..d02f6d029093 100644
>> --- a/drivers/bluetooth/hci_ldisc.c
>> +++ b/drivers/bluetooth/hci_ldisc.c
>> @@ -41,6 +41,7 @@
>>   #include <linux/ioctl.h>
>>   #include <linux/skbuff.h>
>>   #include <linux/firmware.h>
>> +#include <linux/serdev.h>
>>   
>>   #include <net/bluetooth/bluetooth.h>
>>   #include <net/bluetooth/hci_core.h>
>> @@ -298,6 +299,11 @@ void hci_uart_set_flow_control(struct hci_uart *hu, bool enable)
>>   	unsigned int set = 0;
>>   	unsigned int clear = 0;
>>   
>> +	if (hu->serdev) {
>> +		serdev_device_set_flow_control(hu->serdev, !enable);
>> +		return;
>> +	}
> 
> I think this should also control rts, so that the behaviour is the
> same for serdev and ldisc implementation:
> 
> serdev_device_set_rts(serdev, enable);

Good point, fixed for v2 of this set (which I will send out right
after this mail).

Regards,

Hans

[PATCH 2/9] Bluetooth: hci_bcm: Fix setting of irq trigger type

[Hans de Goede]

This commit fixes 2 issues with host-wake irq trigger type handling
in hci_bcm:

1) bcm_setup_sleep sets sleep_params.host_wake_active based on
bcm_device.irq_polarity, but bcm_request_irq was always requesting
IRQF_TRIGGER_RISING as trigger type independent of irq_polarity.

This was a problem when the irq is described as a GpioInt rather then
an Interrupt in the DSDT as for GpioInt-s the value passed to request_irq
is honored. This commit fixes this by requesting the correct trigger
type depending on bcm_device.irq_polarity.

2) bcm_device.irq_polarity was used to directly store an ACPI polarity
value (ACPI_ACTIVE_*). This is undesirable because hci_bcm is also
used with device-tree and checking for something like ACPI_ACTIVE_LOW
in a non ACPI specific function like bcm_request_irq feels wrong.

This commit fixes this by renaming irq_polarity to irq_active_low
and changing its type to a bool.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/bluetooth/hci_bcm.c | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
index c821234b9668..2285ca673ae3 100644
--- a/drivers/bluetooth/hci_bcm.c
+++ b/drivers/bluetooth/hci_bcm.c
@@ -68,7 +68,7 @@ struct bcm_device {
 	u32			init_speed;
 	u32			oper_speed;
 	int			irq;
-	u8			irq_polarity;
+	bool			irq_active_low;
 
 #ifdef CONFIG_PM
 	struct hci_uart		*hu;
@@ -213,7 +213,9 @@ static int bcm_request_irq(struct bcm_data *bcm)
 	}
 
 	err = devm_request_irq(&bdev->pdev->dev, bdev->irq, bcm_host_wake,
-			       IRQF_TRIGGER_RISING, "host_wake", bdev);
+			       bdev->irq_active_low ? IRQF_TRIGGER_FALLING :
+						      IRQF_TRIGGER_RISING,
+			       "host_wake", bdev);
 	if (err)
 		goto unlock;
 
@@ -253,7 +255,7 @@ static int bcm_setup_sleep(struct hci_uart *hu)
 	struct sk_buff *skb;
 	struct bcm_set_sleep_mode sleep_params = default_sleep_params;
 
-	sleep_params.host_wake_active = !bcm->dev->irq_polarity;
+	sleep_params.host_wake_active = !bcm->dev->irq_active_low;
 
 	skb = __hci_cmd_sync(hu->hdev, 0xfc27, sizeof(sleep_params),
 			     &sleep_params, HCI_INIT_TIMEOUT);
@@ -690,10 +692,8 @@ static const struct acpi_gpio_mapping acpi_bcm_int_first_gpios[] = {
 };
 
 #ifdef CONFIG_ACPI
-static u8 acpi_active_low = ACPI_ACTIVE_LOW;
-
 /* IRQ polarity of some chipsets are not defined correctly in ACPI table. */
-static const struct dmi_system_id bcm_wrong_irq_dmi_table[] = {
+static const struct dmi_system_id bcm_active_low_irq_dmi_table[] = {
 	{
 		.ident = "Asus T100TA",
 		.matches = {
@@ -701,7 +701,6 @@ static const struct dmi_system_id bcm_wrong_irq_dmi_table[] = {
 					"ASUSTeK COMPUTER INC."),
 			DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "T100TA"),
 		},
-		.driver_data = &acpi_active_low,
 	},
 	{
 		.ident = "Asus T100CHI",
@@ -710,7 +709,6 @@ static const struct dmi_system_id bcm_wrong_irq_dmi_table[] = {
 					"ASUSTeK COMPUTER INC."),
 			DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "T100CHI"),
 		},
-		.driver_data = &acpi_active_low,
 	},
 	{	/* Handle ThinkPad 8 tablets with BCM2E55 chipset ACPI ID */
 		.ident = "Lenovo ThinkPad 8",
@@ -718,7 +716,6 @@ static const struct dmi_system_id bcm_wrong_irq_dmi_table[] = {
 			DMI_EXACT_MATCH(DMI_SYS_VENDOR, "LENOVO"),
 			DMI_EXACT_MATCH(DMI_PRODUCT_VERSION, "ThinkPad 8"),
 		},
-		.driver_data = &acpi_active_low,
 	},
 	{ }
 };
@@ -733,13 +730,13 @@ static int bcm_resource(struct acpi_resource *ares, void *data)
 	switch (ares->type) {
 	case ACPI_RESOURCE_TYPE_EXTENDED_IRQ:
 		irq = &ares->data.extended_irq;
-		dev->irq_polarity = irq->polarity;
+		dev->irq_active_low = irq->polarity == ACPI_ACTIVE_LOW;
 		break;
 
 	case ACPI_RESOURCE_TYPE_GPIO:
 		gpio = &ares->data.gpio;
 		if (gpio->connection_type == ACPI_RESOURCE_GPIO_TYPE_INT)
-			dev->irq_polarity = gpio->polarity;
+			dev->irq_active_low = gpio->polarity == ACPI_ACTIVE_LOW;
 		break;
 
 	case ACPI_RESOURCE_TYPE_SERIAL_BUS:
@@ -834,11 +831,11 @@ static int bcm_acpi_probe(struct bcm_device *dev)
 		return ret;
 	acpi_dev_free_resource_list(&resources);
 
-	dmi_id = dmi_first_match(bcm_wrong_irq_dmi_table);
+	dmi_id = dmi_first_match(bcm_active_low_irq_dmi_table);
 	if (dmi_id) {
 		bt_dev_warn(dev, "%s: Overwriting IRQ polarity to active low",
 			    dmi_id->ident);
-		dev->irq_polarity = *(u8 *)dmi_id->driver_data;
+		dev->irq_active_low = true;
 	}
 
 	return 0;
-- 
2.14.2

[PATCH 3/9] Bluetooth: hci_bcm: Move bcm_platform_probe call out of bcm_acpi_probe

[Hans de Goede]

Since bcm_acpi_probe calls bcm_platform_probe, bcm_probe always ends up
calling bcm_platform_probe.

This commit simplifies things by making bcm_probe always call
bcm_platform_probe itself.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/bluetooth/hci_bcm.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
index 2285ca673ae3..00103307a776 100644
--- a/drivers/bluetooth/hci_bcm.c
+++ b/drivers/bluetooth/hci_bcm.c
@@ -820,10 +820,6 @@ static int bcm_acpi_probe(struct bcm_device *dev)
 	if (ret)
 		return ret;
 
-	ret = bcm_platform_probe(dev);
-	if (ret)
-		return ret;
-
 	/* Retrieve UART ACPI info */
 	ret = acpi_dev_get_resources(ACPI_COMPANION(&dev->pdev->dev),
 				     &resources, bcm_resource, dev);
@@ -858,10 +854,13 @@ static int bcm_probe(struct platform_device *pdev)
 
 	dev->pdev = pdev;
 
-	if (has_acpi_companion(&pdev->dev))
+	if (has_acpi_companion(&pdev->dev)) {
 		ret = bcm_acpi_probe(dev);
-	else
-		ret = bcm_platform_probe(dev);
+		if (ret)
+			return ret;
+	}
+
+	ret = bcm_platform_probe(dev);
 	if (ret)
 		return ret;
 
-- 
2.14.2

[PATCH 4/9] Bluetooth: hci_bcm: Move platform_get_irq call to bcm_probe

[Hans de Goede]

The ACPI subsys is going to move over to instantiating ACPI enumerated
HCIs as serdevs, rather then as platform devices.

Most of the code in bcm_platform_probe is actually not platform
specific and will work with any struct device passed to it, the one
platform specific call in bcm_platform_probe is platform_get_irq.

This commit moves platform_get_irq call to the platform-driver's bcm_probe
function, this is a preparation patch for adding (runtime)pm support to
the serdev path.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/bluetooth/hci_bcm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
index 00103307a776..ea530a56778d 100644
--- a/drivers/bluetooth/hci_bcm.c
+++ b/drivers/bluetooth/hci_bcm.c
@@ -776,7 +776,6 @@ static int bcm_platform_probe(struct bcm_device *dev)
 		return PTR_ERR(dev->shutdown);
 
 	/* IRQ can be declared in ACPI table as Interrupt or GpioInt */
-	dev->irq = platform_get_irq(pdev, 0);
 	if (dev->irq <= 0) {
 		struct gpio_desc *gpio;
 
@@ -853,6 +852,7 @@ static int bcm_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	dev->pdev = pdev;
+	dev->irq = platform_get_irq(pdev, 0);
 
 	if (has_acpi_companion(&pdev->dev)) {
 		ret = bcm_acpi_probe(dev);
-- 
2.14.2

[PATCH 6/9] Bluetooth: hci_bcm: Rename bcm_platform_probe to bcm_get_resources

[Hans de Goede]

After our previous changes, there is nothing platform specific about
bcm_platform_probe anymore, rename it to bcm_get_resources.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/bluetooth/hci_bcm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
index cfc87fb5051c..5c8371d8aace 100644
--- a/drivers/bluetooth/hci_bcm.c
+++ b/drivers/bluetooth/hci_bcm.c
@@ -756,7 +756,7 @@ static int bcm_resource(struct acpi_resource *ares, void *data)
 }
 #endif /* CONFIG_ACPI */
 
-static int bcm_platform_probe(struct bcm_device *dev)
+static int bcm_get_resources(struct bcm_device *dev)
 {
 	dev->name = dev_name(dev->dev);
 
@@ -857,7 +857,7 @@ static int bcm_probe(struct platform_device *pdev)
 			return ret;
 	}
 
-	ret = bcm_platform_probe(dev);
+	ret = bcm_get_resources(dev);
 	if (ret)
 		return ret;
 
-- 
2.14.2

[PATCH 7/9] Bluetooth: hci_bcm: Make acpi_probe get irq from ACPI resources

[Hans de Goede]

The ACPI subsys is going to move over to instantiating ACPI enumerated
HCIs as serdevs, rather then as platform devices.

So we need to make bcm_acpi_probe() suitable for use on non platform-
devices too, which means that we cannot rely on platform_get_irq()
getting called.

This commit modifies bcm_acpi_probe() to directly get the irq from
the ACPI resources, this is a preparation patch for adding (runtime)pm
support to the serdev path.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/bluetooth/hci_bcm.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
index 5c8371d8aace..48a428909958 100644
--- a/drivers/bluetooth/hci_bcm.c
+++ b/drivers/bluetooth/hci_bcm.c
@@ -805,6 +805,7 @@ static int bcm_acpi_probe(struct bcm_device *dev)
 	const struct dmi_system_id *dmi_id;
 	const struct acpi_gpio_mapping *gpio_mapping = acpi_bcm_int_last_gpios;
 	const struct acpi_device_id *id;
+	struct resource_entry *entry;
 	int ret;
 
 	/* Retrieve GPIO data */
@@ -821,6 +822,13 @@ static int bcm_acpi_probe(struct bcm_device *dev)
 				     &resources, bcm_resource, dev);
 	if (ret < 0)
 		return ret;
+
+	resource_list_for_each_entry(entry, &resources) {
+		if (resource_type(entry->res) == IORESOURCE_IRQ) {
+			dev->irq = entry->res->start;
+			break;
+		}
+	}
 	acpi_dev_free_resource_list(&resources);
 
 	dmi_id = dmi_first_match(bcm_active_low_irq_dmi_table);
-- 
2.14.2

Re: [PATCH 7/9] Bluetooth: hci_bcm: Make acpi_probe get irq from ACPI resources

[Frédéric Danis]

Hi Hans,

First of all, many thanks for your work on this, it helped me a lot to 
get full Bluetooth on my Asus T100.

Le 02/10/2017 à 17:23, Hans de Goede a écrit :
> The ACPI subsys is going to move over to instantiating ACPI enumerated
> HCIs as serdevs, rather then as platform devices.
>
> So we need to make bcm_acpi_probe() suitable for use on non platform-
> devices too, which means that we cannot rely on platform_get_irq()
> getting called.
>
> This commit modifies bcm_acpi_probe() to directly get the irq from
> the ACPI resources, this is a preparation patch for adding (runtime)pm
> support to the serdev path.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>   drivers/bluetooth/hci_bcm.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
>
> diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
> index 5c8371d8aace..48a428909958 100644
> --- a/drivers/bluetooth/hci_bcm.c
> +++ b/drivers/bluetooth/hci_bcm.c
> @@ -805,6 +805,7 @@ static int bcm_acpi_probe(struct bcm_device *dev)
>   	const struct dmi_system_id *dmi_id;
>   	const struct acpi_gpio_mapping *gpio_mapping = acpi_bcm_int_last_gpios;
>   	const struct acpi_device_id *id;
> +	struct resource_entry *entry;
>   	int ret;
>   
>   	/* Retrieve GPIO data */
> @@ -821,6 +822,13 @@ static int bcm_acpi_probe(struct bcm_device *dev)
>   				     &resources, bcm_resource, dev);
>   	if (ret < 0)
>   		return ret;
> +
> +	resource_list_for_each_entry(entry, &resources) {
> +		if (resource_type(entry->res) == IORESOURCE_IRQ) {
> +			dev->irq = entry->res->start;
> +			break;
> +		}
> +	}
>   	acpi_dev_free_resource_list(&resources);
>   
>   	dmi_id = dmi_first_match(bcm_active_low_irq_dmi_table);

You should also return 0 in bcm_resource(), otherwise the resources list 
is empty, ending up with "BCM irq: -22" trace in dmesg.

Regards,

Frédéric Danis

Re: [PATCH 7/9] Bluetooth: hci_bcm: Make acpi_probe get irq from ACPI resources

[Hans de Goede]

Hi,

On 03-10-17 19:21, Frédéric Danis wrote:
> Hi Hans,
> 
> First of all, many thanks for your work on this, it helped me a lot to get full Bluetooth on my Asus T100.

You're welcome thank you for your patches tying everything
together, it will be good to have this all finally working
OOTB.

> Le 02/10/2017 à 17:23, Hans de Goede a écrit :
>> The ACPI subsys is going to move over to instantiating ACPI enumerated
>> HCIs as serdevs, rather then as platform devices.
>>
>> So we need to make bcm_acpi_probe() suitable for use on non platform-
>> devices too, which means that we cannot rely on platform_get_irq()
>> getting called.
>>
>> This commit modifies bcm_acpi_probe() to directly get the irq from
>> the ACPI resources, this is a preparation patch for adding (runtime)pm
>> support to the serdev path.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   drivers/bluetooth/hci_bcm.c | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
>> index 5c8371d8aace..48a428909958 100644
>> --- a/drivers/bluetooth/hci_bcm.c
>> +++ b/drivers/bluetooth/hci_bcm.c
>> @@ -805,6 +805,7 @@ static int bcm_acpi_probe(struct bcm_device *dev)
>>       const struct dmi_system_id *dmi_id;
>>       const struct acpi_gpio_mapping *gpio_mapping = acpi_bcm_int_last_gpios;
>>       const struct acpi_device_id *id;
>> +    struct resource_entry *entry;
>>       int ret;
>>       /* Retrieve GPIO data */
>> @@ -821,6 +822,13 @@ static int bcm_acpi_probe(struct bcm_device *dev)
>>                        &resources, bcm_resource, dev);
>>       if (ret < 0)
>>           return ret;
>> +
>> +    resource_list_for_each_entry(entry, &resources) {
>> +        if (resource_type(entry->res) == IORESOURCE_IRQ) {
>> +            dev->irq = entry->res->start;
>> +            break;
>> +        }
>> +    }
>>       acpi_dev_free_resource_list(&resources);
>>       dmi_id = dmi_first_match(bcm_active_low_irq_dmi_table);
> 
> You should also return 0 in bcm_resource(), otherwise the resources list is empty, ending up with "BCM irq: -22" trace in dmesg.

Right, good one. I did not notice that as on the device I was testing with
the IRQ is specified in the DSDT as a GpioInt rather as an Interrupt.

Fixed for v2 of this series.

Regards,

Hans

[PATCH 5/9] Bluetooth: hci_bcm: Store device pointer instead of platform_device pointer

[Hans de Goede]

The ACPI subsys is going to move over to instantiating ACPI enumerated
HCIs as serdevs, rather then as platform devices.

This means that the serdev driver paths of hci_bcm.c also need to start
supporting (runtime)pm through GPIOs and a host-wake IRQ.

The hci_bcm code is already mostly independent of how the HCI gets
instantiated, but even though the code only cares about pdev->dev, it
was storing pdev itself in struct bcm_device.

This commit stores pdev->dev rather then pdev in struct bcm_device, this
is a preparation patch for adding (runtime)pm support to the serdev path.

Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 drivers/bluetooth/hci_bcm.c | 73 ++++++++++++++++++++++-----------------------
 1 file changed, 35 insertions(+), 38 deletions(-)

diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
index ea530a56778d..cfc87fb5051c 100644
--- a/drivers/bluetooth/hci_bcm.c
+++ b/drivers/bluetooth/hci_bcm.c
@@ -56,7 +56,7 @@
 struct bcm_device {
 	struct list_head	list;
 
-	struct platform_device	*pdev;
+	struct device		*dev;
 
 	const char		*name;
 	struct gpio_desc	*device_wakeup;
@@ -188,9 +188,9 @@ static irqreturn_t bcm_host_wake(int irq, void *data)
 
 	bt_dev_dbg(bdev, "Host wake IRQ");
 
-	pm_runtime_get(&bdev->pdev->dev);
-	pm_runtime_mark_last_busy(&bdev->pdev->dev);
-	pm_runtime_put_autosuspend(&bdev->pdev->dev);
+	pm_runtime_get(bdev->dev);
+	pm_runtime_mark_last_busy(bdev->dev);
+	pm_runtime_put_autosuspend(bdev->dev);
 
 	return IRQ_HANDLED;
 }
@@ -212,20 +212,20 @@ static int bcm_request_irq(struct bcm_data *bcm)
 		goto unlock;
 	}
 
-	err = devm_request_irq(&bdev->pdev->dev, bdev->irq, bcm_host_wake,
+	err = devm_request_irq(bdev->dev, bdev->irq, bcm_host_wake,
 			       bdev->irq_active_low ? IRQF_TRIGGER_FALLING :
 						      IRQF_TRIGGER_RISING,
 			       "host_wake", bdev);
 	if (err)
 		goto unlock;
 
-	device_init_wakeup(&bdev->pdev->dev, true);
+	device_init_wakeup(bdev->dev, true);
 
-	pm_runtime_set_autosuspend_delay(&bdev->pdev->dev,
+	pm_runtime_set_autosuspend_delay(bdev->dev,
 					 BCM_AUTOSUSPEND_DELAY);
-	pm_runtime_use_autosuspend(&bdev->pdev->dev);
-	pm_runtime_set_active(&bdev->pdev->dev);
-	pm_runtime_enable(&bdev->pdev->dev);
+	pm_runtime_use_autosuspend(bdev->dev);
+	pm_runtime_set_active(bdev->dev);
+	pm_runtime_enable(bdev->dev);
 
 unlock:
 	mutex_unlock(&bcm_device_lock);
@@ -332,7 +332,7 @@ static int bcm_open(struct hci_uart *hu)
 		 * platform device (saved during device probe) and
 		 * parent of tty device used by hci_uart
 		 */
-		if (hu->tty->dev->parent == dev->pdev->dev.parent) {
+		if (hu->tty->dev->parent == dev->dev->parent) {
 			bcm->dev = dev;
 			hu->init_speed = dev->init_speed;
 			hu->oper_speed = dev->oper_speed;
@@ -367,12 +367,12 @@ static int bcm_close(struct hci_uart *hu)
 	if (bcm_device_exists(bdev)) {
 		bcm_gpio_set_power(bdev, false);
 #ifdef CONFIG_PM
-		pm_runtime_disable(&bdev->pdev->dev);
-		pm_runtime_set_suspended(&bdev->pdev->dev);
+		pm_runtime_disable(bdev->dev);
+		pm_runtime_set_suspended(bdev->dev);
 
-		if (device_can_wakeup(&bdev->pdev->dev)) {
-			devm_free_irq(&bdev->pdev->dev, bdev->irq, bdev);
-			device_init_wakeup(&bdev->pdev->dev, false);
+		if (device_can_wakeup(bdev->dev)) {
+			devm_free_irq(bdev->dev, bdev->irq, bdev);
+			device_init_wakeup(bdev->dev, false);
 		}
 
 		bdev->hu = NULL;
@@ -506,9 +506,9 @@ static int bcm_recv(struct hci_uart *hu, const void *data, int count)
 		/* Delay auto-suspend when receiving completed packet */
 		mutex_lock(&bcm_device_lock);
 		if (bcm->dev && bcm_device_exists(bcm->dev)) {
-			pm_runtime_get(&bcm->dev->pdev->dev);
-			pm_runtime_mark_last_busy(&bcm->dev->pdev->dev);
-			pm_runtime_put_autosuspend(&bcm->dev->pdev->dev);
+			pm_runtime_get(bcm->dev->dev);
+			pm_runtime_mark_last_busy(bcm->dev->dev);
+			pm_runtime_put_autosuspend(bcm->dev->dev);
 		}
 		mutex_unlock(&bcm_device_lock);
 	}
@@ -539,15 +539,15 @@ static struct sk_buff *bcm_dequeue(struct hci_uart *hu)
 
 	if (bcm_device_exists(bcm->dev)) {
 		bdev = bcm->dev;
-		pm_runtime_get_sync(&bdev->pdev->dev);
+		pm_runtime_get_sync(bdev->dev);
 		/* Shall be resumed here */
 	}
 
 	skb = skb_dequeue(&bcm->txq);
 
 	if (bdev) {
-		pm_runtime_mark_last_busy(&bdev->pdev->dev);
-		pm_runtime_put_autosuspend(&bdev->pdev->dev);
+		pm_runtime_mark_last_busy(bdev->dev);
+		pm_runtime_put_autosuspend(bdev->dev);
 	}
 
 	mutex_unlock(&bcm_device_lock);
@@ -623,7 +623,7 @@ static int bcm_suspend(struct device *dev)
 	if (pm_runtime_active(dev))
 		bcm_suspend_device(dev);
 
-	if (device_may_wakeup(&bdev->pdev->dev)) {
+	if (device_may_wakeup(dev)) {
 		error = enable_irq_wake(bdev->irq);
 		if (!error)
 			bt_dev_dbg(bdev, "BCM irq: enabled");
@@ -651,7 +651,7 @@ static int bcm_resume(struct device *dev)
 	if (!bdev->hu)
 		goto unlock;
 
-	if (device_may_wakeup(&bdev->pdev->dev)) {
+	if (device_may_wakeup(dev)) {
 		disable_irq_wake(bdev->irq);
 		bt_dev_dbg(bdev, "BCM irq: disabled");
 	}
@@ -758,19 +758,17 @@ static int bcm_resource(struct acpi_resource *ares, void *data)
 
 static int bcm_platform_probe(struct bcm_device *dev)
 {
-	struct platform_device *pdev = dev->pdev;
+	dev->name = dev_name(dev->dev);
 
-	dev->name = dev_name(&pdev->dev);
+	dev->clk = devm_clk_get(dev->dev, NULL);
 
-	dev->clk = devm_clk_get(&pdev->dev, NULL);
-
-	dev->device_wakeup = devm_gpiod_get_optional(&pdev->dev,
+	dev->device_wakeup = devm_gpiod_get_optional(dev->dev,
 						     "device-wakeup",
 						     GPIOD_OUT_LOW);
 	if (IS_ERR(dev->device_wakeup))
 		return PTR_ERR(dev->device_wakeup);
 
-	dev->shutdown = devm_gpiod_get_optional(&pdev->dev, "shutdown",
+	dev->shutdown = devm_gpiod_get_optional(dev->dev, "shutdown",
 						GPIOD_OUT_LOW);
 	if (IS_ERR(dev->shutdown))
 		return PTR_ERR(dev->shutdown);
@@ -779,7 +777,7 @@ static int bcm_platform_probe(struct bcm_device *dev)
 	if (dev->irq <= 0) {
 		struct gpio_desc *gpio;
 
-		gpio = devm_gpiod_get_optional(&pdev->dev, "host-wakeup",
+		gpio = devm_gpiod_get_optional(dev->dev, "host-wakeup",
 					       GPIOD_IN);
 		if (IS_ERR(gpio))
 			return PTR_ERR(gpio);
@@ -787,13 +785,13 @@ static int bcm_platform_probe(struct bcm_device *dev)
 		dev->irq = gpiod_to_irq(gpio);
 	}
 
-	dev_info(&pdev->dev, "BCM irq: %d\n", dev->irq);
+	dev_info(dev->dev, "BCM irq: %d\n", dev->irq);
 
 	/* Make sure at-least one of the GPIO is defined and that
 	 * a name is specified for this instance
 	 */
 	if ((!dev->device_wakeup && !dev->shutdown) || !dev->name) {
-		dev_err(&pdev->dev, "invalid platform data\n");
+		dev_err(dev->dev, "invalid platform data\n");
 		return -EINVAL;
 	}
 
@@ -803,7 +801,6 @@ static int bcm_platform_probe(struct bcm_device *dev)
 #ifdef CONFIG_ACPI
 static int bcm_acpi_probe(struct bcm_device *dev)
 {
-	struct platform_device *pdev = dev->pdev;
 	LIST_HEAD(resources);
 	const struct dmi_system_id *dmi_id;
 	const struct acpi_gpio_mapping *gpio_mapping = acpi_bcm_int_last_gpios;
@@ -811,16 +808,16 @@ static int bcm_acpi_probe(struct bcm_device *dev)
 	int ret;
 
 	/* Retrieve GPIO data */
-	id = acpi_match_device(pdev->dev.driver->acpi_match_table, &pdev->dev);
+	id = acpi_match_device(dev->dev->driver->acpi_match_table, dev->dev);
 	if (id)
 		gpio_mapping = (const struct acpi_gpio_mapping *) id->driver_data;
 
-	ret = devm_acpi_dev_add_driver_gpios(&pdev->dev, gpio_mapping);
+	ret = devm_acpi_dev_add_driver_gpios(dev->dev, gpio_mapping);
 	if (ret)
 		return ret;
 
 	/* Retrieve UART ACPI info */
-	ret = acpi_dev_get_resources(ACPI_COMPANION(&dev->pdev->dev),
+	ret = acpi_dev_get_resources(ACPI_COMPANION(dev->dev),
 				     &resources, bcm_resource, dev);
 	if (ret < 0)
 		return ret;
@@ -851,7 +848,7 @@ static int bcm_probe(struct platform_device *pdev)
 	if (!dev)
 		return -ENOMEM;
 
-	dev->pdev = pdev;
+	dev->dev = &pdev->dev;
 	dev->irq = platform_get_irq(pdev, 0);
 
 	if (has_acpi_companion(&pdev->dev)) {
-- 
2.14.2

[PATCH 8/9] Bluetooth: hci_bcm: Make suspend/resume functions platform_dev independent

[Hans de Goede]

Use dev_get_drvdata instead of platform_get_drvdata in the suspend /
resume functions. This is a preparation patch for adding (runtime)pm
support to the serdev path.

Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 drivers/bluetooth/hci_bcm.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
index 48a428909958..7191cee5ced0 100644
--- a/drivers/bluetooth/hci_bcm.c
+++ b/drivers/bluetooth/hci_bcm.c
@@ -558,7 +558,7 @@ static struct sk_buff *bcm_dequeue(struct hci_uart *hu)
 #ifdef CONFIG_PM
 static int bcm_suspend_device(struct device *dev)
 {
-	struct bcm_device *bdev = platform_get_drvdata(to_platform_device(dev));
+	struct bcm_device *bdev = dev_get_drvdata(dev);
 
 	bt_dev_dbg(bdev, "");
 
@@ -581,7 +581,7 @@ static int bcm_suspend_device(struct device *dev)
 
 static int bcm_resume_device(struct device *dev)
 {
-	struct bcm_device *bdev = platform_get_drvdata(to_platform_device(dev));
+	struct bcm_device *bdev = dev_get_drvdata(dev);
 
 	bt_dev_dbg(bdev, "");
 
@@ -606,7 +606,7 @@ static int bcm_resume_device(struct device *dev)
 /* Platform suspend callback */
 static int bcm_suspend(struct device *dev)
 {
-	struct bcm_device *bdev = platform_get_drvdata(to_platform_device(dev));
+	struct bcm_device *bdev = dev_get_drvdata(dev);
 	int error;
 
 	bt_dev_dbg(bdev, "suspend: is_suspended %d", bdev->is_suspended);
@@ -638,7 +638,7 @@ static int bcm_suspend(struct device *dev)
 /* Platform resume callback */
 static int bcm_resume(struct device *dev)
 {
-	struct bcm_device *bdev = platform_get_drvdata(to_platform_device(dev));
+	struct bcm_device *bdev = dev_get_drvdata(dev);
 
 	bt_dev_dbg(bdev, "resume: is_suspended %d", bdev->is_suspended);
 
-- 
2.14.2

[PATCH 9/9] Bluetooth: hci_bcm: Add (runtime)pm support to the serdev driver

[Hans de Goede]

Make the serdev driver use struct bcm_device as its driver data and share
all the pm / GPIO / IRQ related code paths with the platform driver.

After this commit the 2 drivers are in essence the same and the serdev
driver interface can be used for all ACPI enumerated HCI UARTs.

Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 drivers/bluetooth/hci_bcm.c | 118 +++++++++++++++++++++++++-------------------
 1 file changed, 68 insertions(+), 50 deletions(-)

diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
index 7191cee5ced0..5fff99bb6444 100644
--- a/drivers/bluetooth/hci_bcm.c
+++ b/drivers/bluetooth/hci_bcm.c
@@ -52,8 +52,10 @@
 
 #define BCM_AUTOSUSPEND_DELAY	5000 /* default autosleep delay */
 
-/* platform device driver resources */
+/* device driver resources */
 struct bcm_device {
+	/* Must be the first member, hci_serdev.c expects this. */
+	struct hci_uart		serdev_hu;
 	struct list_head	list;
 
 	struct device		*dev;
@@ -76,11 +78,6 @@ struct bcm_device {
 #endif
 };
 
-/* serdev driver resources */
-struct bcm_serdev {
-	struct hci_uart hu;
-};
-
 /* generic bcm uart resources */
 struct bcm_data {
 	struct sk_buff		*rx_skb;
@@ -155,6 +152,10 @@ static bool bcm_device_exists(struct bcm_device *device)
 {
 	struct list_head *p;
 
+	/* Devices using serdev always exist */
+	if (device && device->hu && device->hu->serdev)
+		return true;
+
 	list_for_each(p, &bcm_device_list) {
 		struct bcm_device *dev = list_entry(p, struct bcm_device, list);
 
@@ -200,7 +201,6 @@ static int bcm_request_irq(struct bcm_data *bcm)
 	struct bcm_device *bdev = bcm->dev;
 	int err;
 
-	/* If this is not a platform device, do not enable PM functionalities */
 	mutex_lock(&bcm_device_lock);
 	if (!bcm_device_exists(bdev)) {
 		err = -ENODEV;
@@ -313,18 +313,17 @@ static int bcm_open(struct hci_uart *hu)
 
 	hu->priv = bcm;
 
-	/* If this is a serdev defined device, then only use
-	 * serdev open primitive and skip the rest.
-	 */
+	mutex_lock(&bcm_device_lock);
+
 	if (hu->serdev) {
 		serdev_device_open(hu->serdev);
+		bcm->dev = serdev_device_get_drvdata(hu->serdev);
 		goto out;
 	}
 
 	if (!hu->tty->dev)
 		goto out;
 
-	mutex_lock(&bcm_device_lock);
 	list_for_each(p, &bcm_device_list) {
 		struct bcm_device *dev = list_entry(p, struct bcm_device, list);
 
@@ -334,37 +333,45 @@ static int bcm_open(struct hci_uart *hu)
 		 */
 		if (hu->tty->dev->parent == dev->dev->parent) {
 			bcm->dev = dev;
-			hu->init_speed = dev->init_speed;
-			hu->oper_speed = dev->oper_speed;
 #ifdef CONFIG_PM
 			dev->hu = hu;
 #endif
-			bcm_gpio_set_power(bcm->dev, true);
 			break;
 		}
 	}
 
-	mutex_unlock(&bcm_device_lock);
 out:
+	if (bcm->dev) {
+		hu->init_speed = bcm->dev->init_speed;
+		hu->oper_speed = bcm->dev->oper_speed;
+		bcm_gpio_set_power(bcm->dev, true);
+	}
+
+	mutex_unlock(&bcm_device_lock);
 	return 0;
 }
 
 static int bcm_close(struct hci_uart *hu)
 {
 	struct bcm_data *bcm = hu->priv;
-	struct bcm_device *bdev = bcm->dev;
+	struct bcm_device *bdev = NULL;
 
 	bt_dev_dbg(hu->hdev, "hu %p", hu);
 
-	/* If this is a serdev defined device, only use serdev
-	 * close primitive and then continue as usual.
-	 */
-	if (hu->serdev)
-		serdev_device_close(hu->serdev);
-
 	/* Protect bcm->dev against removal of the device or driver */
 	mutex_lock(&bcm_device_lock);
-	if (bcm_device_exists(bdev)) {
+
+	if (hu->serdev) {
+		serdev_device_close(hu->serdev);
+		bdev = serdev_device_get_drvdata(hu->serdev);
+	} else if (bcm_device_exists(bcm->dev)) {
+		bdev = bcm->dev;
+#ifdef CONFIG_PM
+		bdev->hu = NULL;
+#endif
+	}
+
+	if (bdev) {
 		bcm_gpio_set_power(bdev, false);
 #ifdef CONFIG_PM
 		pm_runtime_disable(bdev->dev);
@@ -374,8 +381,6 @@ static int bcm_close(struct hci_uart *hu)
 			devm_free_irq(bdev->dev, bdev->irq, bdev);
 			device_init_wakeup(bdev->dev, false);
 		}
-
-		bdev->hu = NULL;
 #endif
 	}
 	mutex_unlock(&bcm_device_lock);
@@ -603,7 +608,7 @@ static int bcm_resume_device(struct device *dev)
 #endif
 
 #ifdef CONFIG_PM_SLEEP
-/* Platform suspend callback */
+/* suspend callback */
 static int bcm_suspend(struct device *dev)
 {
 	struct bcm_device *bdev = dev_get_drvdata(dev);
@@ -611,8 +616,10 @@ static int bcm_suspend(struct device *dev)
 
 	bt_dev_dbg(bdev, "suspend: is_suspended %d", bdev->is_suspended);
 
-	/* bcm_suspend can be called at any time as long as platform device is
-	 * bound, so it should use bcm_device_lock to protect access to hci_uart
+	/*
+	 * When used with a device instantiated as platform_device, bcm_suspend
+	 * can be called at any time as long as the platform device is bound,
+	 * so it should use bcm_device_lock to protect access to hci_uart
 	 * and device_wake-up GPIO.
 	 */
 	mutex_lock(&bcm_device_lock);
@@ -635,15 +642,17 @@ static int bcm_suspend(struct device *dev)
 	return 0;
 }
 
-/* Platform resume callback */
+/* resume callback */
 static int bcm_resume(struct device *dev)
 {
 	struct bcm_device *bdev = dev_get_drvdata(dev);
 
 	bt_dev_dbg(bdev, "resume: is_suspended %d", bdev->is_suspended);
 
-	/* bcm_resume can be called at any time as long as platform device is
-	 * bound, so it should use bcm_device_lock to protect access to hci_uart
+	/*
+	 * When used with a device instantiated as platform_device, bcm_resume
+	 * can be called at any time as long as platform device is bound,
+	 * so it should use bcm_device_lock to protect access to hci_uart
 	 * and device_wake-up GPIO.
 	 */
 	mutex_lock(&bcm_device_lock);
@@ -786,15 +795,6 @@ static int bcm_get_resources(struct bcm_device *dev)
 	}
 
 	dev_info(dev->dev, "BCM irq: %d\n", dev->irq);
-
-	/* Make sure at-least one of the GPIO is defined and that
-	 * a name is specified for this instance
-	 */
-	if ((!dev->device_wakeup && !dev->shutdown) || !dev->name) {
-		dev_err(dev->dev, "invalid platform data\n");
-		return -EINVAL;
-	}
-
 	return 0;
 }
 
@@ -847,6 +847,12 @@ static int bcm_acpi_probe(struct bcm_device *dev)
 }
 #endif /* CONFIG_ACPI */
 
+static int bcm_of_probe(struct bcm_device *bdev)
+{
+	device_property_read_u32(bdev->dev, "max-speed", &bdev->oper_speed);
+	return 0;
+}
+
 static int bcm_probe(struct platform_device *pdev)
 {
 	struct bcm_device *dev;
@@ -935,7 +941,7 @@ static const struct acpi_device_id bcm_acpi_match[] = {
 MODULE_DEVICE_TABLE(acpi, bcm_acpi_match);
 #endif
 
-/* Platform suspend and resume callbacks */
+/* suspend and resume callbacks */
 static const struct dev_pm_ops bcm_pm_ops = {
 	SET_SYSTEM_SLEEP_PM_OPS(bcm_suspend, bcm_resume)
 	SET_RUNTIME_PM_OPS(bcm_suspend_device, bcm_resume_device, NULL)
@@ -953,29 +959,39 @@ static struct platform_driver bcm_driver = {
 
 static int bcm_serdev_probe(struct serdev_device *serdev)
 {
-	struct bcm_serdev *bcmdev;
-	u32 speed;
+	struct bcm_device *bcmdev;
 	int err;
 
 	bcmdev = devm_kzalloc(&serdev->dev, sizeof(*bcmdev), GFP_KERNEL);
 	if (!bcmdev)
 		return -ENOMEM;
 
-	bcmdev->hu.serdev = serdev;
+	bcmdev->dev = &serdev->dev;
+	bcmdev->hu = &bcmdev->serdev_hu;
+	bcmdev->serdev_hu.serdev = serdev;
 	serdev_device_set_drvdata(serdev, bcmdev);
 
-	err = device_property_read_u32(&serdev->dev, "max-speed", &speed);
-	if (!err)
-		bcmdev->hu.oper_speed = speed;
+	if (has_acpi_companion(&serdev->dev))
+		err = bcm_acpi_probe(bcmdev);
+	else
+		err = bcm_of_probe(bcmdev);
+	if (err)
+		return err;
 
-	return hci_uart_register_device(&bcmdev->hu, &bcm_proto);
+	err = bcm_get_resources(bcmdev);
+	if (err)
+		return err;
+
+	bcm_gpio_set_power(bcmdev, false);
+
+	return hci_uart_register_device(&bcmdev->serdev_hu, &bcm_proto);
 }
 
 static void bcm_serdev_remove(struct serdev_device *serdev)
 {
-	struct bcm_serdev *bcmdev = serdev_device_get_drvdata(serdev);
+	struct bcm_device *bcmdev = serdev_device_get_drvdata(serdev);
 
-	hci_uart_unregister_device(&bcmdev->hu);
+	hci_uart_unregister_device(&bcmdev->serdev_hu);
 }
 
 #ifdef CONFIG_OF
@@ -992,6 +1008,8 @@ static struct serdev_device_driver bcm_serdev_driver = {
 	.driver = {
 		.name = "hci_uart_bcm",
 		.of_match_table = of_match_ptr(bcm_bluetooth_of_match),
+		.acpi_match_table = ACPI_PTR(bcm_acpi_match),
+		.pm = &bcm_pm_ops,
 	},
 };
 
-- 
2.14.2