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
