Re: [PATCH v3 4/6] platform/x86: dell-smo8800: Allow lis3lv02d i2c_client instantiation without IRQ

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

 



On Fri, Jun 21, 2024 at 2:26 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
>
> The Dell XPS 15 9550 can have a 60Wh battery, leaving space for a 2.5"
> sata disk; or a 90Wh battery in which case the battery occupies the space
> for the optional 2.5" sata disk.
>
> On models with the 90Wh battery and thus without a 2.5" sata disk, the BIOS
> does not add an IRQ resource to the SMO8810 ACPI device.
>
> Make the misc-device registration and the requesting of the IRQ optional
> and instantiate a lis3lv02d i2c_client independent of the IRQ being there,
> so that the non freefall lis3lv02d functionality can still be used.

non-freefall

> Note that IRQ 0 is not a valid IRQ number for platform IRQs
> and this patch relies on that.

...

> +       err = platform_get_irq_optional(device, 0);
> +       if (err > 0)
> +               smo8800->irq = err;
> +
> +       if (smo8800->irq) {

You can still do it in one branch less. But I think I understand the
motivation of the split.

> +               err = misc_register(&smo8800->miscdev);
> +               if (err) {
> +                       dev_err(&device->dev, "failed to register misc dev: %d\n", err);
> +                       return err;
> +               }
> +
> +               err = request_threaded_irq(smo8800->irq, smo8800_interrupt_quick,
> +                                          smo8800_interrupt_thread,
> +                                          IRQF_TRIGGER_RISING | IRQF_ONESHOT,
> +                                          DRIVER_NAME, smo8800);
> +               if (err) {
> +                       dev_err(&device->dev,
> +                               "failed to request thread for IRQ %d: %d\n",
> +                               smo8800->irq, err);
> +                       goto error;
> +               }
> +
> +               dev_dbg(&device->dev, "device /dev/freefall registered with IRQ %d\n",
> +                        smo8800->irq);
>         }

...

>  error_free_irq:
> -       free_irq(smo8800->irq, smo8800);
> +       if (smo8800->irq) {
> +               free_irq(smo8800->irq, smo8800);
>  error:
> -       misc_deregister(&smo8800->miscdev);
> +               misc_deregister(&smo8800->miscdev);
> +       }

This is quite unusual.
I would rather expect

  label_foo:
    if (...)
      foo()
  label_bar:
    if (...)
      bar()


--
With Best Regards,
Andy Shevchenko





[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux