Messages in this thread | ![/](/images/icornerl.gif) | | From | Mark Hasemeyer <> | Date | Wed, 27 Dec 2023 14:29:16 -0700 | Subject | Re: [PATCH v3 24/24] platform/chrome: cros_ec: Use PM subsystem to manage wakeirq |
| |
> > - irq = platform_get_irq_optional(pdev, 0); > > - if (irq > 0) > > + irq = platform_get_irq_resource_optional(pdev, 0, &irqres); > > + if (irq > 0) { > > ec_dev->irq = irq; > > - else if (irq != -ENXIO) { > > + if (should_force_irq_wake_capable()) > > + ec_dev->irq_wake = true; > > + else > > + ec_dev->irq_wake = irqres.flags & IORESOURCE_IRQ_WAKECAPABLE; > > + } else if (irq != -ENXIO) { > > dev_err(dev, "couldn't retrieve IRQ number (%d)\n", irq); > > return irq; > > } > > Still I do not like ambiguity behind irq > 0 vs. irqres.start. > > For this, and if needed others, return plain error. > Seems I gave the tag for the previous patch, consider > that tag conditional (it seems I missed this).
What "others" do you mean? Modify platform_get_irq_resource_optional() to return success/err? Or do you mean to modify all irq resource based functions? of_irq_to_resource() already existed and returns the irq number on success. Modifying it would mean updating all references to it, in addition to modifying the fwnode abstraction layer (and its references). I'm open to modifying platform_get_irq_resource_optional(), but would like to avoid blowing up this patch train any further.
> ... > > > u16 proto_version; > > void *priv; > > int irq; > > + bool irq_wake; > > u8 *din; > > u8 *dout; > > int din_size; > > int dout_size; > > - bool wake_enabled; > > bool suspended; > > int (*cmd_xfer)(struct cros_ec_device *ec, > > struct cros_ec_command *msg); > > Have you run pahole on this (before and after)?
Yes I did. The structure is not fully optimized, but this change keps the overall size unchanged (328 bytes).
| ![\](/images/icornerr.gif) |