On Wed, Aug 28, 2024 at 05:17:40PM +0300, Andy Shevchenko wrote: > On Wed, Aug 28, 2024 at 04:01:19PM +0200, Vasileios Amoiridis wrote: > > On Mon, Aug 26, 2024 at 01:23:56PM +0300, Andy Shevchenko wrote: > > > On Sat, Aug 24, 2024 at 02:02:22PM +0200, Vasileios Amoiridis wrote: > > > > On Fri, Aug 23, 2024 at 11:06:28PM +0300, Andy Shevchenko wrote: > > > > > On Fri, Aug 23, 2024 at 08:17:13PM +0200, Vasileios Amoiridis wrote: > > ... > > > > > > > + irq = fwnode_irq_get(fwnode, 0); > > > > > > + if (!irq) > > > > > > > > > > Are you sure this is correct check? > > > > > > > > > Well, I think yes, because the function return either the Linux IRQ number > > > > on success or a negative errno on failure. > > > > > > Where is 0 mentioned in this? > > > > > > > https://elixir.bootlin.com/linux/v6.10.6/source/drivers/base/property.c#L987 > > > > > > > > > > + return dev_err_probe(data->dev, -ENODEV, > > > > > > > > > > Shadowed error code. > > > > > > > > I am not sure I understand what you mean here. You mean that there is no > > > > chance that the first one will pass and this one will fail? > > > > > > -ENODEV is not what fwnode_irq_get() returns on error. > > > > > > > > > + "No interrupt found.\n"); > > ... > > > > > > > + desc = irq_get_irq_data(irq); > > > > > > + if (!desc) > > > > > > + return -EINVAL; > > > > > > > > > > When may this fail? > > > > > > > > I think that this will fail when Linux were not able to actually > > > > register that interrupt. > > > > > > Wouldn't fwnode_irq_get() fail already? > > > > By looking at it again, I didn't reply correct here. This function > > internally calls the irq_to_desc() which basically returns the > > irq desctiptor for this irq. This function can return NULL in > > case the interrupt is not found in the maple tree (CONFIG_SPARSE_IRQ) > > or in case the interrupt number is bigger than the NR_IRQs which > > the irq controller can handle (!CONFIG_SPARSE_IRQ). > > > > So in my opinion, it makes sense to keep this check. > > So, you mean that if fwnode_irq_get() succeeded there is a chance that returned > Linux IRQ number is invalid?! If it's so, it's something new to me. I would like > to see the details, please! > > -- > With Best Regards, > Andy Shevchenko > > Hi Andy, I did some more digging, and from my understanding, fwnode_irq_get() directly returns the Linux IRQ which means that there should already exist the irq_desc structure that is returned by the irq_to_desc(). So as you already said, I cannot see how this function can fail, if the fwnode_irq_get() has succeeded. Thanks for asking the right questions, I am getting to know things better. Cheers, Vasilis
