On 11:14-20231214, Siddharth Vadapalli wrote: > Hello Nishanth, > > On 13/12/23 18:08, Nishanth Menon wrote: > > On 13:32-20231213, Siddharth Vadapalli wrote: > >> Enable interrupt mode of operation of the DP83867 Ethernet PHY which is > >> used by ICSSG2. The DP83867 PHY driver already supports interrupt handling > >> for interrupts generated by the PHY. Thus, add the necessary device-tree > >> support to enable it. > >> > >> Since the GPIO1_87 line is muxed with EXT_REFCLK1 and SYNC1_OUT, update > >> the pinmux to select GPIO1_87 for routing the interrupt. > >> > >> As the same interrupt line and therefore the same pinmux configuration is > >> applicable to both Ethernet PHYs used by ICSSG2, allocate the pinmux > >> resource to the first Ethernet PHY alone. > > ... > > > > > https://www.ti.com/lit/ds/symlink/dp83867ir.pdf -> it looks like the > > interrupt pin is level event. but drivers/gpio/gpio-davinci.c:: > > gpio_irq_type() -> The SoC cannot handle level, only edge. > > > > A bit confused here.. GPIO 87 is shared between two phys. isn't it a > > case of race? > > > > PHY1 assets low > > phy1 handler starts, but before the driver it clears the condition: > > PHY2 asserts low - but since the signal is already low, there is no > > pulse > > phy1 handler clears phy1 condition, but signal is still low due to phy2? > > now phy2 OR phy1 never gets handled since there is never a pulse event > > ever again. > > Yes, you are right! Edge-Triggered interrupts shouldn't be shared. I missed > noticing this. Thank you for pointing it out. Since the SoC only supports > Edge-Triggered interrupts, I believe that the correct decision would be to use > the interrupt for only one of the two PHYs, while leaving the other PHY in > polled mode of operation which is the default. > > Kindly let me know if this is acceptable and I shall update this patch accordingly. Sounds like a bug in board design there (due to an choice of IP limitation) - I suggest getting it noted in board documentation and refer to the errata in the second phy (else folks will wonder why we aren't using interrupts on the second phy. > > > > > > >> ti,rx-internal-delay = <DP83867_RGMIIDCTL_2_00_NS>; > >> ti,fifo-depth = <DP83867_PHYCR_FIFO_DEPTH_4_B_NIB>; > >> }; > >> -- > >> 2.34.1 > >> > > > > -- > Regards, > Siddharth. -- Regards, Nishanth Menon Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D
