2015-05-27 17:31:51

by Arend van Spriel

[permalink] [raw]
Subject: [PATCH] brcmfmac: fix invalid access to struct acpi_device fields

The fields of struct acpi_device are only known when CONFIG_ACPI is
defined. Fix this by using a helper function. This will resolve the
issue found in linux-next:

../brcmfmac/bcmsdh.c: In function 'brcmf_ops_sdio_probe':
../brcmfmac/bcmsdh.c:1139:7: error: dereferencing pointer to incomplete type
adev->flags.power_manageable = 0;
^

Fixes: commit f0992ace680c ("brcmfmac: prohibit ACPI power management ...")
Cc: Fu, Zhonghui <[email protected]>
Reported-by: Stephen Rothwell <[email protected]>
Signed-off-by: Arend van Spriel <[email protected]>
---
drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
index b0d0ff5..71779b9 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
@@ -1117,6 +1117,18 @@ MODULE_DEVICE_TABLE(sdio, brcmf_sdmmc_ids);
static struct brcmfmac_sdio_platform_data *brcmfmac_sdio_pdata;


+static void brcmf_sdiod_acpi_set_power_manageable(struct device *dev,
+ int val)
+{
+#if IS_ENABLED(CONFIG_ACPI)
+ struct acpi_device *adev;
+
+ adev = ACPI_COMPANION(dev);
+ if (adev)
+ adev->flags.power_manageable = 0;
+#endif
+}
+
static int brcmf_ops_sdio_probe(struct sdio_func *func,
const struct sdio_device_id *id)
{
@@ -1124,7 +1136,6 @@ static int brcmf_ops_sdio_probe(struct sdio_func *func,
struct brcmf_sdio_dev *sdiodev;
struct brcmf_bus *bus_if;
struct device *dev;
- struct acpi_device *adev;

brcmf_dbg(SDIO, "Enter\n");
brcmf_dbg(SDIO, "Class=%x\n", func->class);
@@ -1132,11 +1143,9 @@ static int brcmf_ops_sdio_probe(struct sdio_func *func,
brcmf_dbg(SDIO, "sdio device ID: 0x%04x\n", func->device);
brcmf_dbg(SDIO, "Function#: %d\n", func->num);

- /* prohibit ACPI power management for this device */
dev = &func->dev;
- adev = ACPI_COMPANION(dev);
- if (adev)
- adev->flags.power_manageable = 0;
+ /* prohibit ACPI power management for this device */
+ brcmf_sdiod_acpi_set_power_manageable(dev, 0);

/* Consume func num 1 but dont do anything with it. */
if (func->num == 1)
--
1.9.1



2015-05-28 16:48:35

by Jason Andryuk

[permalink] [raw]
Subject: Re: [PATCH] brcmfmac: fix invalid access to struct acpi_device fields

On Wed, May 27, 2015 at 1:31 PM, Arend van Spriel <[email protected]> wrote:
> diff --git a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
> index b0d0ff5..71779b9 100644
> --- a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
> +++ b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
> @@ -1117,6 +1117,18 @@ MODULE_DEVICE_TABLE(sdio, brcmf_sdmmc_ids);
> static struct brcmfmac_sdio_platform_data *brcmfmac_sdio_pdata;
>
>
> +static void brcmf_sdiod_acpi_set_power_manageable(struct device *dev,
> + int val)
> +{
> +#if IS_ENABLED(CONFIG_ACPI)
> + struct acpi_device *adev;
> +
> + adev = ACPI_COMPANION(dev);
> + if (adev)
> + adev->flags.power_manageable = 0;

Shouldn't this be " = val"?

-Jason

> +#endif
> +}
> +
> static int brcmf_ops_sdio_probe(struct sdio_func *func,
> const struct sdio_device_id *id)
> {

2015-05-28 08:29:30

by Kalle Valo

[permalink] [raw]
Subject: Re: brcmfmac: fix invalid access to struct acpi_device fields


> The fields of struct acpi_device are only known when CONFIG_ACPI is
> defined. Fix this by using a helper function. This will resolve the
> issue found in linux-next:
>
> ../brcmfmac/bcmsdh.c: In function 'brcmf_ops_sdio_probe':
> ../brcmfmac/bcmsdh.c:1139:7: error: dereferencing pointer to incomplete type
> adev->flags.power_manageable = 0;
> ^
>
> Fixes: f0992ace680c ("brcmfmac: prohibit ACPI power management ...")
> Cc: Fu, Zhonghui <[email protected]>
> Reported-by: Stephen Rothwell <[email protected]>
> Signed-off-by: Arend van Spriel <[email protected]>

Thanks, applied to wireless-drivers-next.git.

Kalle Valo

2015-05-28 18:31:08

by Arend van Spriel

[permalink] [raw]
Subject: Re: [PATCH] brcmfmac: fix invalid access to struct acpi_device fields

On 05/28/15 18:48, Jason Andryuk wrote:
> On Wed, May 27, 2015 at 1:31 PM, Arend van Spriel<[email protected]> wrote:
>> diff --git a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
>> index b0d0ff5..71779b9 100644
>> --- a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
>> +++ b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
>> @@ -1117,6 +1117,18 @@ MODULE_DEVICE_TABLE(sdio, brcmf_sdmmc_ids);
>> static struct brcmfmac_sdio_platform_data *brcmfmac_sdio_pdata;
>>
>>
>> +static void brcmf_sdiod_acpi_set_power_manageable(struct device *dev,
>> + int val)
>> +{
>> +#if IS_ENABLED(CONFIG_ACPI)
>> + struct acpi_device *adev;
>> +
>> + adev = ACPI_COMPANION(dev);
>> + if (adev)
>> + adev->flags.power_manageable = 0;
>
> Shouldn't this be " = val"?

Definitely. The only place where it is called uses 0 for val parameter
so there is no big issue, but I will fix it.

Thanks for spotting that bit.

Regards,
Arend

> -Jason
>
>> +#endif
>> +}
>> +
>> static int brcmf_ops_sdio_probe(struct sdio_func *func,
>> const struct sdio_device_id *id)
>> {