Re: [PATCH 7/8] asus-wireless: Export ids list

[Andy Shevchenko]

On Thu, Jan 26, 2017 at 5:30 PM, JoÃo Paulo Rechi Vita
<jprvita@xxxxxxxxx> wrote:
> Signed-off-by: JoÃo Paulo Rechi Vita <jprvita@xxxxxxxxxxxx>
> ---
>  drivers/platform/x86/asus-wireless.c | 15 ++++++---------
>  drivers/platform/x86/asus-wireless.h | 10 ++++++++++
>  2 files changed, 16 insertions(+), 9 deletions(-)
>  create mode 100644 drivers/platform/x86/asus-wireless.h
>
> diff --git a/drivers/platform/x86/asus-wireless.c b/drivers/platform/x86/asus-wireless.c
> index 5a238fad35fb..fa28613688b4 100644
> --- a/drivers/platform/x86/asus-wireless.c
> +++ b/drivers/platform/x86/asus-wireless.c
> @@ -17,6 +17,8 @@
>  #include <linux/pci_ids.h>
>  #include <linux/leds.h>
>
> +#include "asus-wireless.h"
> +
>  #define ASUS_WIRELESS_LED_STATUS 0x2
>
>  struct hswc_params {
> @@ -40,12 +42,7 @@ static const struct hswc_params id_params[] = {
>         { 0x5, 0x4 },
>  };
>
> -static const struct acpi_device_id device_ids[] = {
> -       {"ATK4001", 0},
> -       {"ATK4002", 0},
> -       {"", 0},
> -};
> -MODULE_DEVICE_TABLE(acpi, device_ids);
> +MODULE_DEVICE_TABLE(acpi, asus_wireless_ids);
>

No, Don't do this.

>  static u64 asus_wireless_method(acpi_handle handle, const char *method,
>                                 int param)
> @@ -130,8 +127,8 @@ static int asus_wireless_add(struct acpi_device *adev)
>         adev->driver_data = data;
>
>         hid = acpi_device_hid(adev);
> -       for (i = 0; strcmp(device_ids[i].id, ""); i++) {

This is wrong.

> -               if (!strcmp(device_ids[i].id, hid)) {
> +       for (i = 0; strcmp(asus_wireless_ids[i].id, ""); i++) {

This is too.

Potential infinite loop.

On top of that seems you just introduced this by previous patches and
changing here.
Often it means you need to reconsider how you actually split the
series on logical pieces.

> +               if (!strcmp(asus_wireless_ids[i].id, hid)) {
>                         data->hswc_params = &id_params[i];
>                         break;
>                 }
> @@ -179,7 +176,7 @@ static int asus_wireless_remove(struct acpi_device *adev)
>  static struct acpi_driver asus_wireless_driver = {
>         .name = "Asus Wireless Radio Control Driver",
>         .class = "hotkey",
> -       .ids = device_ids,
> +       .ids = asus_wireless_ids,
>         .ops = {
>                 .add = asus_wireless_add,
>                 .remove = asus_wireless_remove,
> diff --git a/drivers/platform/x86/asus-wireless.h b/drivers/platform/x86/asus-wireless.h
> new file mode 100644
> index 000000000000..10a345863da6
> --- /dev/null
> +++ b/drivers/platform/x86/asus-wireless.h
> @@ -0,0 +1,10 @@
> +#ifndef _ASUS_WIRELESS_H_
> +#define _ASUS_WIRELESS_H_
> +
> +const struct acpi_device_id asus_wireless_ids[] = {
> +       {"ATK4001", 0},
> +       {"ATK4002", 0},
> +       {"", 0},
> +};
> +
> +#endif /* !_ASUS_WIRELESS_H_ */
> --
> 2.11.0
>



-- 
With Best Regards,
Andy Shevchenko