Re: [RFC v3 1/3] NET: PHY: adds driver for Intel XWAY PHY
Am 04.06.2016 um 16:43 schrieb Langer, Thomas:
> Hello Hauke,
>
> [...]
>> +
>> +static int xway_gphy_ack_interrupt(struct phy_device *phydev)
>> +{
>> +int reg;
>> +
>> +/**
>> + * Possible IRQ numbers:
>> + * - IM3_IRL18 for GPHY0
>> + * - IM3_IRL17 for GPHY1
>
> These are references to the SoC interrupts.
>
>> + *
>> + * Due to a silicon bug IRQ lines are not really independent from
>> + * each other. Sometimes the two lines are driven at the same time
>> + * if only one GPHY core raises the interrupt.
>
> So this errata sounds like a SoC errata, not an errata for the PHY.
>
this comment is a remnant from the first driver version which was mainly
used on a VRX200 SoC. But interrupts are generally working fine so this
comment should be removed.
--
- Daniel
Re: [RFC v3 1/3] NET: PHY: adds driver for Intel XWAY PHY
Hi Thomas Hi Hauke On 04/06/2016 16:43, Langer, Thomas wrote: >> + /* there is an errata regarding irqs in this rev */ > And then this is comment is also now valid. > What about a system with a single external phy connected, > on a non-Lantiq/Intel SoC? > > I think the feasibility of using interrupts is not related to the phy version, > but indirectly by the version of the SoC it is integrated. > > So maybe he use of interrupts (on these SoCs) should be controlled by > devicetree or > network driver, where the SoC type and version can be handled? > IIRC the 2 irq lines are broken on xrx200 v1.1 SoC silicon. irqs were unreliable and sometimes fired on the wrong phy or not at all. maybe this was fixed on v1.2 silicon ? this is not related to the phy per-se but the SoC silicon it is integrated into. the PHY driver should be agnostic of the SoC having a functional IRQ block i think. devictrees for v1.1 SoC silicon should simply not define an IRQ inside the devicetree and rely on the phy polling done by the mdio/phy layer. John
RE: [RFC v3 1/3] NET: PHY: adds driver for Intel XWAY PHY
Hello Hauke,
[...]
> +
> +static int xway_gphy_ack_interrupt(struct phy_device *phydev)
> +{
> + int reg;
> +
> + /**
> + * Possible IRQ numbers:
> + * - IM3_IRL18 for GPHY0
> + * - IM3_IRL17 for GPHY1
These are references to the SoC interrupts.
> + *
> + * Due to a silicon bug IRQ lines are not really independent from
> + * each other. Sometimes the two lines are driven at the same time
> + * if only one GPHY core raises the interrupt.
So this errata sounds like a SoC errata, not an errata for the PHY.
> + */
> + reg = phy_read(phydev, XWAY_MDIO_ISTAT);
> +
> + return (reg < 0) ? reg : 0;
> +}
> +
[...]
> +
> +static struct phy_driver xway_gphy[] = {
> + {
> + .phy_id = PHY_ID_PHY11G_1_3,
> + .phy_id_mask= 0x,
> + .name = "Intel XWAY PHY11G (PEF 7071/PEF 7072)
> v1.3",
> + .features = (PHY_GBIT_FEATURES |
> SUPPORTED_Pause |
> +SUPPORTED_Asym_Pause),
> + /* there is an errata regarding irqs in this rev */
And then this is comment is also now valid.
What about a system with a single external phy connected,
on a non-Lantiq/Intel SoC?
I think the feasibility of using interrupts is not related to the phy version,
but indirectly by the version of the SoC it is integrated.
So maybe he use of interrupts (on these SoCs) should be controlled by
devicetree or
network driver, where the SoC type and version can be handled?
> + .flags = 0,
> + .config_init= xway_gphy_config_init,
> + .config_aneg= xway_gphy14_config_aneg,
> + .read_status= genphy_read_status,
> + .ack_interrupt = xway_gphy_ack_interrupt,
> + .did_interrupt = xway_gphy_did_interrupt,
> + .config_intr= xway_gphy_config_intr,
> + .suspend= genphy_suspend,
> + .resume = genphy_resume,
> + }, {
[...]
Regards,
Thomas
