Re: [PATCH] HID: intel-ish-hid: Prevent loading of driver on Mehlow

[Benjamin Tissoires]

On Tue, Jul 17, 2018 at 10:06 PM Srinivas Pandruvada
<srinivas.pandruvada@xxxxxxxxxxxxxxx> wrote:
>
> On Mehlow Xeon-E workstation, ISH PCI device is enabled but without ISH
> firmware. Here the ISH device PCI device id was reused for some non Linux
> storage drivers. So this was not done for enabling ISH. But this has a
> undesirable side effect for Linux.
>
> Here the ISH driver will be loaded via PCI enumeration and will try to do
> reset sequence. But reset sequence will wait till timeout as there is no
> real ISH firmware is present to take action. This delay will add to boot
> time of Linux (This platform will still continue to boot after this
> timeout).
>
> To avoid this boot delay we need to prevent loading of ISH drivers on
> this platform. So we need to have hack to avoid treating this device as
> ISH on this platform. To identify this workstation, we need some runtime
> method. Luckily there are special PCI id on this workstation to
> distinguish from the client version of this platform. On client version,
> the ISH is supported using same PCI device id. So this change look for
> the presence of PCI device IDs A309 and A30A and exit.
>
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@xxxxxxxxxxxxxxx>
> ---
> This is not a change for 4.18-rc.
>
>  drivers/hid/intel-ish-hid/ipc/pci-ish.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>
> diff --git a/drivers/hid/intel-ish-hid/ipc/pci-ish.c b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
> index a2c53ea3b5ed..d6e7156c36d9 100644
> --- a/drivers/hid/intel-ish-hid/ipc/pci-ish.c
> +++ b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
> @@ -95,6 +95,26 @@ static int ish_init(struct ishtp_device *dev)
>         return 0;
>  }
>
> +
> +static bool ish_invalid_firmware(void)
> +{
> +       struct pci_dev *pdev;
> +
> +       pdev = pci_get_device(PCI_VENDOR_ID_INTEL, 0xA309, NULL);
> +       if (pdev) {
> +               pci_dev_put(pdev);
> +               return true;
> +       }
> +
> +       pdev = pci_get_device(PCI_VENDOR_ID_INTEL, 0xA30A, NULL);
> +       if (pdev) {
> +               pci_dev_put(pdev);
> +               return true;
> +       }

I think this duplicated code should be merged in a for loop on an
array of blacklisted items :)

Cheers,
Benjamin

> +
> +       return false;
> +}
> +
>  /**
>   * ish_probe() - PCI driver probe callback
>   * @pdev:      pci device
> @@ -110,6 +130,9 @@ static int ish_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>         struct ish_hw *hw;
>         int     ret;
>
> +       if (ish_invalid_firmware())
> +               return -ENODEV;
> +
>         /* enable pci dev */
>         ret = pci_enable_device(pdev);
>         if (ret) {
> --
> 2.17.1
>