Hi Andrew,
On Thu, May 18, 2017 at 6:33 PM, Andrew Lunn <andrew@xxxxxxx> wrote:
>> >> phy = get_phy_device(mdio, addr, is_c45);
>> >> if (IS_ERR(phy))
>> >> - return;
>> >> + return PTR_ERR(phy);
>> >>
>> >> - rc = irq_of_parse_and_map(child, 0);
>> >> + rc = of_irq_get(child, 0);
>> >> + if (rc == -EPROBE_DEFER) {
>> >> + phy_device_free(phy);
>> >> + return rc;
>> >> + }
>> >
>> > Maybe this should be consistent. All other places there is an error,
>> > you return it. Here however, you only return the error if it is
>> > EPROBE_DEFER.
>>
>> That's because of the "else" branch in the code below:
>>
>> if (rc > 0) {
>> phy->irq = rc;
>> mdio->irq[addr] = rc;
>> } else {
>> phy->irq = mdio->irq[addr];
>> }
>>
>> cfr. the marked part of the patch description.
>> I didn't want to change that behavior, as it's not clear to me why it's handled
>> that way.
>
> So there seems to be 3 conditions that need handling:
>
> 1) of_irq_get() gives us an interrupt number.
> 2) of_irq_get() indicates there is no irq in the device tree.
> 3) of_irq_get() indicates a real error
>
> 1) We have.
>
> 2) We should fall back to using the mdio busses irq for the
> device. There are a couple of mdio drivers which do this, e.g.
> stmicro/stmmac/stmmac_mdio.c. mdiobus_alloc() ensures it is set to
> PHY_POLL, so if the driver does not set it, we poll.
>
> 3) This is new. We have two choices. Ignore the error and poll. Or
> return the error. Historically we have ignored the error. But should
> we? I would probably return the error, now that we can. But...
The issue itself isn't new, though.
I reported it in "of_mdiobus_register_phy() and deferred probe"
(https://lkml.org/lkml/2015/10/22/377), and posted a workaround in "[PATCH v2]
irqchip/renesas-irqc: Postpone driver initialization"
(https://lkml.org/lkml/2016/11/8/794).
Due to the fallback to polling, so far it was easier to complain when
someone broke polling, than to fix the real problem ;-)
But when I saw Thomas' patch[*] for of_irq_to_resource(), the time was ripe
to tackle the root cause.
[*] https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git/commit/?h=dt/next&id=7a4228bbff769ebf449981a4248616db9f0cffec
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
