Re: [PATCH 2/3] platform/x86: apple-gmux: Add apple_gmux_detect() helper

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Jan 23, 2023 at 1:38 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
>
> Add a new (static inline) apple_gmux_detect() helper to apple-gmux.h
> which can be used for gmux detection instead of apple_gmux_present().
>
> The latter is not really reliable since an ACPI device with a HID
> of APP000B is present on some devices without a gmux at all, as well
> as on devices with a newer (unsupported) MMIO based gmux model.
>
> This causes apple_gmux_present() to return false-postives on

positives

> a number of different Apple laptop models.
>
> This new helper uses the same probing as the actual apple-gmux
> driver, so that it does not return false positives.
>
> To avoid code duplication the gmux_probe() function of the actual
> driver is also moved over to using the new apple_gmux_detect() helper.

...

> +       if (!apple_gmux_detect(pnp, &indexed)) {
> +               pr_info("gmux device not present\n");

You may start using dev_info(&pnp->dev, ...) if I'm not mistaken.

> +               return -ENODEV;
> +       }

...

> +static inline bool apple_gmux_detect(struct pnp_dev *pnp_dev, bool *indexed_ret)
> +{
> +       u8 ver_major, ver_minor, ver_release;
> +       struct resource *res;
> +       bool indexed = false;
> +
> +       if (!pnp_dev) {
> +               struct acpi_device *adev;
> +               struct device *dev;
> +
> +               adev = acpi_dev_get_first_match_dev(GMUX_ACPI_HID, NULL, -1);
> +               if (!adev)
> +                       return false;
> +
> +               dev = acpi_get_first_physical_node(adev);
> +               if (!dev)

I remember I saw something like this in your tree(?). I hope it's not
pending upstream (yet) because of a leak here. Don't forget to call
acpi_dev_put() after you finish with adev. Recently I have fixed a
bunch of similar issues in ASoC Intel.

> +                       return false;
> +
> +               pnp_dev = to_pnp_dev(dev);
> +       }
> +
> +       res = pnp_get_resource(pnp_dev, IORESOURCE_IO, 0);
> +       if (!res)
> +               return false;
> +
> +       if (resource_size(res) < GMUX_MIN_IO_LEN)
> +               return false;
> +
> +       /*
> +        * Invalid version information may indicate either that the gmux
> +        * device isn't present or that it's a new one that uses indexed io.
> +        */
> +       ver_major = inb(res->start + GMUX_PORT_VERSION_MAJOR);
> +       ver_minor = inb(res->start + GMUX_PORT_VERSION_MINOR);
> +       ver_release = inb(res->start + GMUX_PORT_VERSION_RELEASE);
> +       if (ver_major == 0xff && ver_minor == 0xff && ver_release == 0xff) {
> +               indexed = apple_gmux_is_indexed(res->start);
> +               if (!indexed)
> +                       return false;
> +       }
> +
> +       if (indexed_ret)
> +               *indexed_ret = indexed;
> +
> +       return true;
> +}

-- 
With Best Regards,
Andy Shevchenko



[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]
  Powered by Linux