diff mbox

brcmfmac: fix invalid access to struct acpi_device fields

Message ID 1432747901-8472-1-git-send-email-arend@broadcom.com (mailing list archive)
State Accepted
Delegated to: Kalle Valo
Headers show

Commit Message

Arend van Spriel May 27, 2015, 5:31 p.m. UTC
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 <zhonghui.fu@linux.intel.com>
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Arend van Spriel <arend@broadcom.com>
---
 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

Comments

Kalle Valo May 28, 2015, 8:29 a.m. UTC | #1
> 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 <zhonghui.fu@linux.intel.com>
> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> Signed-off-by: Arend van Spriel <arend@broadcom.com>

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

Kalle Valo
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jason Andryuk May 28, 2015, 4:48 p.m. UTC | #2
On Wed, May 27, 2015 at 1:31 PM, Arend van Spriel <arend@broadcom.com> 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)
>  {
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arend van Spriel May 28, 2015, 6:31 p.m. UTC | #3
On 05/28/15 18:48, Jason Andryuk wrote:
> On Wed, May 27, 2015 at 1:31 PM, Arend van Spriel<arend@broadcom.com>  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)
>>   {

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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)