Marvell phy errata origins?

Marvell phy errata origins?

[Daniel Walker]

Hi,

Cisco is using a Marvell 88E1112 phy. It seems to be fairly similar to 
the 88E1111 which Harini added a fix for. In Harini's commit message for ,

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/net/phy/marvell.c?id=3ec0a0f10ceb

"This function has a sequence accessing Page 5 and Register 31, both of 
which are not defined or reserved for this PHY"

For the 88E1112 we see that these are "Factory Test Modes" which the 
contents of are not documented. They aren't really "not defied", and 
aren't really "reserved" .. Marvell support claims they don't support 
these drivers, and Freescale seems to be adding these drivers, and the 
line we are looking at.

We had some issues with our PHY which were corrected with the same patch 
Harini used but modified for the M88E1112. We're trying to get to the 
bottom of where this code came from and what it was suppose to be doing.

Here are the problem lines where were removed,

drivers/net/phy/marvell.c:marvell_config_aneg()

  266         err = phy_write(phydev, 0x1d, 0x1f);
  267         if (err < 0)
  268                 return err;
  269
  270         err = phy_write(phydev, 0x1e, 0x200c);
  271         if (err < 0)
  272                 return err;
  273
  274         err = phy_write(phydev, 0x1d, 0x5);
  275         if (err < 0)
  276                 return err;
  277
  278         err = phy_write(phydev, 0x1e, 0);
  279         if (err < 0)
  280                 return err;
  281
  282         err = phy_write(phydev, 0x1e, 0x100);
  283         if (err < 0)
  284                 return err;

Does anyone have any clues as to why this was added?

Thanks,

Daniel

Re: Marvell phy errata origins?

[Andrew Lunn]

On Tue, Apr 18, 2017 at 06:16:33AM -0700, Daniel Walker wrote:
> 
> Hi,
> 
> Cisco is using a Marvell 88E1112 phy. It seems to be fairly similar
> to the 88E1111 which Harini added a fix for.

Hi Daniel

If you look at Marvell reference drive, DSDT, they are actually quite
different. Different virtual cable tester, different downshift
configuration, different packet generator, different loopback. I would
say they are different generations of PHY.

> In Harini's commit
> message for ,
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/net/phy/marvell.c?id=3ec0a0f10ceb
> 
> "This function has a sequence accessing Page 5 and Register 31, both
> of which are not defined or reserved for this PHY"
> 
> For the 88E1112 we see that these are "Factory Test Modes" which the
> contents of are not documented. They aren't really "not defied", and
> aren't really "reserved" .. Marvell support claims they don't
> support these drivers, and Freescale seems to be adding these
> drivers, and the line we are looking at.
> 
> We had some issues with our PHY which were corrected with the same
> patch Harini used but modified for the M88E1112. We're trying to get
> to the bottom of where this code came from and what it was suppose
> to be doing.

I tried to find this errata fix in the Marvell reference code. And
failed to find it. But it is "Vendor Crap" code, hard to find anything
in it.

My guess is, this errata just applies to one model of PHY, maybe even
one revision of one model of a PHY. The hard bit is figuring out what
actually needs it. Do you have access to Marvell datasheets?

Otherwise, go back to what appears to be the first commit for this
driver:

commit 00db8189d984d6c51226dafbbe4a667ce9b7d5da
Author: Andy Fleming <afleming@freescale.com>
Date:   Sat Jul 30 19:31:23 2005 -0400

    This patch adds a PHY Abstraction Layer to the Linux Kernel, enabling
    ethernet drivers to remain as ignorant as is reasonable of the connected
    PHY's design and operation details.
    
    Signed-off-by: Andy Fleming <afleming@freescale.com>
    Signed-off-by: Jeff Garzik <jgarzik@pobox.com>

It seems a reasonable guess that any PHY added after this commit does
not need the errata.

    Andrew

Re: Marvell phy errata origins?

[Daniel Walker]

On 04/18/2017 07:04 AM, Andrew Lunn wrote:
> On Tue, Apr 18, 2017 at 06:16:33AM -0700, Daniel Walker wrote:
>> Hi,
>>
>> Cisco is using a Marvell 88E1112 phy. It seems to be fairly similar
>> to the 88E1111 which Harini added a fix for.
> Hi Daniel
>
> If you look at Marvell reference drive, DSDT, they are actually quite
> different. Different virtual cable tester, different downshift
> configuration, different packet generator, different loopback. I would
> say they are different generations of PHY.

Ok .

>
>> In Harini's commit
>> message for ,
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/net/phy/marvell.c?id=3ec0a0f10ceb
>>
>> "This function has a sequence accessing Page 5 and Register 31, both
>> of which are not defined or reserved for this PHY"
>>
>> For the 88E1112 we see that these are "Factory Test Modes" which the
>> contents of are not documented. They aren't really "not defied", and
>> aren't really "reserved" .. Marvell support claims they don't
>> support these drivers, and Freescale seems to be adding these
>> drivers, and the line we are looking at.
>>
>> We had some issues with our PHY which were corrected with the same
>> patch Harini used but modified for the M88E1112. We're trying to get
>> to the bottom of where this code came from and what it was suppose
>> to be doing.
> I tried to find this errata fix in the Marvell reference code. And
> failed to find it. But it is "Vendor Crap" code, hard to find anything
> in it.
>
> My guess is, this errata just applies to one model of PHY, maybe even
> one revision of one model of a PHY. The hard bit is figuring out what
> actually needs it. Do you have access to Marvell datasheets?

Prior to Andy's commit this code was duplicated in at least a couple of 
the ethernet drivers from Freescale. I think all Andy did was combine 
them all.

I think the data sheet is here for the 1112,

http://www.dexsilicium.com/Marvell_88E1112.pdf

This is what I've been using. My contact with Marvell for support is 
minimal. Andy's email at Freescale bounced , so maybe someone else form 
Freescale could comment. There must have been something very strange 
getting fixed due to using unnamed hex values in the code.

Daniel

RE: Marvell phy errata origins?

[Harini Katakam]

Hi Daniel,

> -----Original Message-----
> From: Daniel Walker [mailto:danielwa@cisco.com]
> Sent: Tuesday, April 18, 2017 7:48 PM
> To: Andrew Lunn <andrew@lunn.ch>
> Cc: Florian Fainelli <f.fainelli@gmail.com>; Andy Fleming
> <afleming@freescale.com>; Harini Katakam <harinik@xilinx.com>;
> netdev@vger.kernel.org; HEMANT RAMDASI <hramdasi@cisco.com>; Julius
> Hemanth Pitti -X (jpitti - MONTA VISTA SOFTWARE INC at Cisco)
> <jpitti@cisco.com>
> Subject: Re: Marvell phy errata origins?
> 
> On 04/18/2017 07:04 AM, Andrew Lunn wrote:
> > On Tue, Apr 18, 2017 at 06:16:33AM -0700, Daniel Walker wrote:

<snip>
> >> In Harini's commit
> >> message for ,
> >>
> >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/co
> >> mmit/drivers/net/phy/marvell.c?id=3ec0a0f10ceb
> >>
> >> "This function has a sequence accessing Page 5 and Register 31, both
> >> of which are not defined or reserved for this PHY"
> >>
> >> For the 88E1112 we see that these are "Factory Test Modes" which the
> >> contents of are not documented. They aren't really "not defied", and
> >> aren't really "reserved" .. Marvell support claims they don't support
> >> these drivers, and Freescale seems to be adding these drivers, and
> >> the line we are looking at.
> >>
> >> We had some issues with our PHY which were corrected with the same
> >> patch Harini used but modified for the M88E1112. We're trying to get
> >> to the bottom of where this code came from and what it was suppose to
> >> be doing.
> > I tried to find this errata fix in the Marvell reference code. And
> > failed to find it. But it is "Vendor Crap" code, hard to find anything
> > in it.
> >
> > My guess is, this errata just applies to one model of PHY, maybe even
> > one revision of one model of a PHY. The hard bit is figuring out what
> > actually needs it. Do you have access to Marvell datasheets?
> 

I have access to the datasheets for 88E1111, 88E1112 and 88E1116 (and another family 151x) -
None of them have these register details or the errata documented.

Regards,
Harini

Re: Marvell phy errata origins?

[Daniel Walker]

On 04/18/2017 07:31 AM, Harini Katakam wrote:
> Hi Daniel,
>
>> -----Original Message-----
>> From: Daniel Walker [mailto:danielwa@cisco.com]
>> Sent: Tuesday, April 18, 2017 7:48 PM
>> To: Andrew Lunn <andrew@lunn.ch>
>> Cc: Florian Fainelli <f.fainelli@gmail.com>; Andy Fleming
>> <afleming@freescale.com>; Harini Katakam <harinik@xilinx.com>;
>> netdev@vger.kernel.org; HEMANT RAMDASI <hramdasi@cisco.com>; Julius
>> Hemanth Pitti -X (jpitti - MONTA VISTA SOFTWARE INC at Cisco)
>> <jpitti@cisco.com>
>> Subject: Re: Marvell phy errata origins?
>>
>> On 04/18/2017 07:04 AM, Andrew Lunn wrote:
>>> On Tue, Apr 18, 2017 at 06:16:33AM -0700, Daniel Walker wrote:
> <snip>
>>>> In Harini's commit
>>>> message for ,
>>>>
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/co
>>>> mmit/drivers/net/phy/marvell.c?id=3ec0a0f10ceb
>>>>
>>>> "This function has a sequence accessing Page 5 and Register 31, both
>>>> of which are not defined or reserved for this PHY"
>>>>
>>>> For the 88E1112 we see that these are "Factory Test Modes" which the
>>>> contents of are not documented. They aren't really "not defied", and
>>>> aren't really "reserved" .. Marvell support claims they don't support
>>>> these drivers, and Freescale seems to be adding these drivers, and
>>>> the line we are looking at.
>>>>
>>>> We had some issues with our PHY which were corrected with the same
>>>> patch Harini used but modified for the M88E1112. We're trying to get
>>>> to the bottom of where this code came from and what it was suppose to
>>>> be doing.
>>> I tried to find this errata fix in the Marvell reference code. And
>>> failed to find it. But it is "Vendor Crap" code, hard to find anything
>>> in it.
>>>
>>> My guess is, this errata just applies to one model of PHY, maybe even
>>> one revision of one model of a PHY. The hard bit is figuring out what
>>> actually needs it. Do you have access to Marvell datasheets?
> I have access to the datasheets for 88E1111, 88E1112 and 88E1116 (and another family 151x) -
> None of them have these register details or the errata documented.

http://www.dexsilicium.com/Marvell_88E1112.pd


Is this the same datasheet ? In this one on page 81 there is page 5 
register 31 is "Factory Test Modes" , do you not have that?

Daniel

RE: Marvell phy errata origins?

[Harini Katakam]

Hi Daniel,

> -----Original Message-----
> From: Daniel Walker [mailto:danielwa@cisco.com]
> Sent: Tuesday, April 18, 2017 8:05 PM
> To: Harini Katakam <harinik@xilinx.com>; Andrew Lunn <andrew@lunn.ch>
> Cc: Florian Fainelli <f.fainelli@gmail.com>; Andy Fleming
> <afleming@freescale.com>; netdev@vger.kernel.org; HEMANT RAMDASI
> <hramdasi@cisco.com>; Julius Hemanth Pitti -X (jpitti - MONTA VISTA
> SOFTWARE INC at Cisco) <jpitti@cisco.com>
> Subject: Re: Marvell phy errata origins?
> 
> On 04/18/2017 07:31 AM, Harini Katakam wrote:
> > Hi Daniel,
> >
> >> -----Original Message-----
> >> From: Daniel Walker [mailto:danielwa@cisco.com]
> >> Sent: Tuesday, April 18, 2017 7:48 PM
> >> To: Andrew Lunn <andrew@lunn.ch>
> >> Cc: Florian Fainelli <f.fainelli@gmail.com>; Andy Fleming
> >> <afleming@freescale.com>; Harini Katakam <harinik@xilinx.com>;
> >> netdev@vger.kernel.org; HEMANT RAMDASI <hramdasi@cisco.com>; Julius
> >> Hemanth Pitti -X (jpitti - MONTA VISTA SOFTWARE INC at Cisco)
> >> <jpitti@cisco.com>
> >> Subject: Re: Marvell phy errata origins?
> >>
> >> On 04/18/2017 07:04 AM, Andrew Lunn wrote:
> >>> On Tue, Apr 18, 2017 at 06:16:33AM -0700, Daniel Walker wrote:
> > <snip>
> >>>> In Harini's commit
> >>>> message for ,
> >>>>
> >>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/
> >>>> co mmit/drivers/net/phy/marvell.c?id=3ec0a0f10ceb
> >>>>
> >>>> "This function has a sequence accessing Page 5 and Register 31,
> >>>> both of which are not defined or reserved for this PHY"
> >>>>
> >>>> For the 88E1112 we see that these are "Factory Test Modes" which
> >>>> the contents of are not documented. They aren't really "not
> >>>> defied", and aren't really "reserved" .. Marvell support claims
> >>>> they don't support these drivers, and Freescale seems to be adding
> >>>> these drivers, and the line we are looking at.
> >>>>
> >>>> We had some issues with our PHY which were corrected with the same
> >>>> patch Harini used but modified for the M88E1112. We're trying to
> >>>> get to the bottom of where this code came from and what it was
> >>>> suppose to be doing.
> >>> I tried to find this errata fix in the Marvell reference code. And
> >>> failed to find it. But it is "Vendor Crap" code, hard to find
> >>> anything in it.
> >>>
> >>> My guess is, this errata just applies to one model of PHY, maybe
> >>> even one revision of one model of a PHY. The hard bit is figuring
> >>> out what actually needs it. Do you have access to Marvell datasheets?
> > I have access to the datasheets for 88E1111, 88E1112 and 88E1116 (and
> > another family 151x) - None of them have these register details or the errata
> documented.
> 
> http://www.dexsilicium.com/Marvell_88E1112.pd
> 
> 
> Is this the same datasheet ? In this one on page 81 there is page 5 register 31 is
> "Factory Test Modes" , do you not have that?

Yes, it is the same datasheet. That is what I meant - it just says factory test modes
But there is no register description.

Regards,
Harini

Re: Marvell phy errata origins?

[Daniel Walker]

On 04/18/2017 07:41 AM, Harini Katakam wrote:
> Hi Daniel,
>
>> -----Original Message-----
>> From: Daniel Walker [mailto:danielwa@cisco.com]
>> Sent: Tuesday, April 18, 2017 8:05 PM
>> To: Harini Katakam <harinik@xilinx.com>; Andrew Lunn <andrew@lunn.ch>
>> Cc: Florian Fainelli <f.fainelli@gmail.com>; Andy Fleming
>> <afleming@freescale.com>; netdev@vger.kernel.org; HEMANT RAMDASI
>> <hramdasi@cisco.com>; Julius Hemanth Pitti -X (jpitti - MONTA VISTA
>> SOFTWARE INC at Cisco) <jpitti@cisco.com>
>> Subject: Re: Marvell phy errata origins?
>>
>> On 04/18/2017 07:31 AM, Harini Katakam wrote:
>>> Hi Daniel,
>>>
>>>> -----Original Message-----
>>>> From: Daniel Walker [mailto:danielwa@cisco.com]
>>>> Sent: Tuesday, April 18, 2017 7:48 PM
>>>> To: Andrew Lunn <andrew@lunn.ch>
>>>> Cc: Florian Fainelli <f.fainelli@gmail.com>; Andy Fleming
>>>> <afleming@freescale.com>; Harini Katakam <harinik@xilinx.com>;
>>>> netdev@vger.kernel.org; HEMANT RAMDASI <hramdasi@cisco.com>; Julius
>>>> Hemanth Pitti -X (jpitti - MONTA VISTA SOFTWARE INC at Cisco)
>>>> <jpitti@cisco.com>
>>>> Subject: Re: Marvell phy errata origins?
>>>>
>>>> On 04/18/2017 07:04 AM, Andrew Lunn wrote:
>>>>> On Tue, Apr 18, 2017 at 06:16:33AM -0700, Daniel Walker wrote:
>>> <snip>
>>>>>> In Harini's commit
>>>>>> message for ,
>>>>>>
>>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/
>>>>>> co mmit/drivers/net/phy/marvell.c?id=3ec0a0f10ceb
>>>>>>
>>>>>> "This function has a sequence accessing Page 5 and Register 31,
>>>>>> both of which are not defined or reserved for this PHY"
>>>>>>
>>>>>> For the 88E1112 we see that these are "Factory Test Modes" which
>>>>>> the contents of are not documented. They aren't really "not
>>>>>> defied", and aren't really "reserved" .. Marvell support claims
>>>>>> they don't support these drivers, and Freescale seems to be adding
>>>>>> these drivers, and the line we are looking at.
>>>>>>
>>>>>> We had some issues with our PHY which were corrected with the same
>>>>>> patch Harini used but modified for the M88E1112. We're trying to
>>>>>> get to the bottom of where this code came from and what it was
>>>>>> suppose to be doing.
>>>>> I tried to find this errata fix in the Marvell reference code. And
>>>>> failed to find it. But it is "Vendor Crap" code, hard to find
>>>>> anything in it.
>>>>>
>>>>> My guess is, this errata just applies to one model of PHY, maybe
>>>>> even one revision of one model of a PHY. The hard bit is figuring
>>>>> out what actually needs it. Do you have access to Marvell datasheets?
>>> I have access to the datasheets for 88E1111, 88E1112 and 88E1116 (and
>>> another family 151x) - None of them have these register details or the errata
>> documented.
>>
>> http://www.dexsilicium.com/Marvell_88E1112.pd
>>
>>
>> Is this the same datasheet ? In this one on page 81 there is page 5 register 31 is
>> "Factory Test Modes" , do you not have that?
> Yes, it is the same datasheet. That is what I meant - it just says factory test modes
> But there is no register description.

I would call that undocumented. I suspect there are things inside the 
"Factory Test Modes" which could be helpful to fix errata. Things which 
Marvell might not want to document universally.

Can you describe what sort of issue you found which was the motivation 
for your patch ? Or did you remove the code because you did not know 
what it was used for?

Daniel

RE: Marvell phy errata origins?

[Harini Katakam]

Hi Daniel,

> -----Original Message-----
> From: Daniel Walker [mailto:danielwa@cisco.com]
> Sent: Tuesday, April 18, 2017 8:26 PM
> To: Harini Katakam <harinik@xilinx.com>; Andrew Lunn <andrew@lunn.ch>
> Cc: Florian Fainelli <f.fainelli@gmail.com>; Andy Fleming
> <afleming@freescale.com>; netdev@vger.kernel.org; HEMANT RAMDASI
> <hramdasi@cisco.com>; Julius Hemanth Pitti -X (jpitti - MONTA VISTA
> SOFTWARE INC at Cisco) <jpitti@cisco.com>; harinikatakamlinux@gmail.com
> Subject: Re: Marvell phy errata origins?
> 
> On 04/18/2017 07:41 AM, Harini Katakam wrote:
> > Hi Daniel,
> >
> >> -----Original Message-----
> >> From: Daniel Walker [mailto:danielwa@cisco.com]
> >> Sent: Tuesday, April 18, 2017 8:05 PM
> >> To: Harini Katakam <harinik@xilinx.com>; Andrew Lunn <andrew@lunn.ch>
> >> Cc: Florian Fainelli <f.fainelli@gmail.com>; Andy Fleming
> >> <afleming@freescale.com>; netdev@vger.kernel.org; HEMANT RAMDASI
> >> <hramdasi@cisco.com>; Julius Hemanth Pitti -X (jpitti - MONTA VISTA
> >> SOFTWARE INC at Cisco) <jpitti@cisco.com>
> >> Subject: Re: Marvell phy errata origins?
> >>
> >> On 04/18/2017 07:31 AM, Harini Katakam wrote:
> >>> Hi Daniel,
> >>>
> >>>> -----Original Message-----
> >>>> From: Daniel Walker [mailto:danielwa@cisco.com]
> >>>> Sent: Tuesday, April 18, 2017 7:48 PM
> >>>> To: Andrew Lunn <andrew@lunn.ch>
> >>>> Cc: Florian Fainelli <f.fainelli@gmail.com>; Andy Fleming
> >>>> <afleming@freescale.com>; Harini Katakam <harinik@xilinx.com>;
> >>>> netdev@vger.kernel.org; HEMANT RAMDASI <hramdasi@cisco.com>;
> Julius
> >>>> Hemanth Pitti -X (jpitti - MONTA VISTA SOFTWARE INC at Cisco)
> >>>> <jpitti@cisco.com>
> >>>> Subject: Re: Marvell phy errata origins?
> >>>>
> >>>> On 04/18/2017 07:04 AM, Andrew Lunn wrote:
> >>>>> On Tue, Apr 18, 2017 at 06:16:33AM -0700, Daniel Walker wrote:
> >>> <snip>
> >>>>>> In Harini's commit
> >>>>>> message for ,
> >>>>>>
> >>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.gi
> >>>>>> t/ co mmit/drivers/net/phy/marvell.c?id=3ec0a0f10ceb
> >>>>>>
> >>>>>> "This function has a sequence accessing Page 5 and Register 31,
> >>>>>> both of which are not defined or reserved for this PHY"
> >>>>>>
> >>>>>> For the 88E1112 we see that these are "Factory Test Modes" which
> >>>>>> the contents of are not documented. They aren't really "not
> >>>>>> defied", and aren't really "reserved" .. Marvell support claims
> >>>>>> they don't support these drivers, and Freescale seems to be
> >>>>>> adding these drivers, and the line we are looking at.
> >>>>>>
> >>>>>> We had some issues with our PHY which were corrected with the
> >>>>>> same patch Harini used but modified for the M88E1112. We're
> >>>>>> trying to get to the bottom of where this code came from and what
> >>>>>> it was suppose to be doing.
> >>>>> I tried to find this errata fix in the Marvell reference code. And
> >>>>> failed to find it. But it is "Vendor Crap" code, hard to find
> >>>>> anything in it.
> >>>>>
> >>>>> My guess is, this errata just applies to one model of PHY, maybe
> >>>>> even one revision of one model of a PHY. The hard bit is figuring
> >>>>> out what actually needs it. Do you have access to Marvell datasheets?
> >>> I have access to the datasheets for 88E1111, 88E1112 and 88E1116
> >>> (and another family 151x) - None of them have these register details
> >>> or the errata
> >> documented.
> >>
> >> http://www.dexsilicium.com/Marvell_88E1112.pd
> >>
> >>
> >> Is this the same datasheet ? In this one on page 81 there is page 5
> >> register 31 is "Factory Test Modes" , do you not have that?
> > Yes, it is the same datasheet. That is what I meant - it just says
> > factory test modes But there is no register description.
> 
> I would call that undocumented. I suspect there are things inside the "Factory
> Test Modes" which could be helpful to fix errata. Things which Marvell might not
> want to document universally.

OK, agreed.

> 
> Can you describe what sort of issue you found which was the motivation for
> your patch ? Or did you remove the code because you did not know what it was
> used for?
> 

I added this patch for 88E1111 because that datasheet does not even have
Page 5 Reg 31 as "Factory Test mode" - it is reserved space.
Any access to that register failed. Since this register was clearly not part of
88E1111 PHY and irrelevant, I removed that code.

Regards,
Harini

Re: Marvell phy errata origins?

[Daniel Walker]

On 04/18/2017 07:04 AM, Andrew Lunn wrote:
> On Tue, Apr 18, 2017 at 06:16:33AM -0700, Daniel Walker wrote:
>> Hi,
>>
>> Cisco is using a Marvell 88E1112 phy. It seems to be fairly similar
>> to the 88E1111 which Harini added a fix for.
> Hi Daniel
>
> If you look at Marvell reference drive, DSDT, they are actually quite
> different. Different virtual cable tester, different downshift
> configuration, different packet generator, different loopback. I would
> say they are different generations of PHY.
>
>> In Harini's commit
>> message for ,
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/net/phy/marvell.c?id=3ec0a0f10ceb
>>
>> "This function has a sequence accessing Page 5 and Register 31, both
>> of which are not defined or reserved for this PHY"
>>
>> For the 88E1112 we see that these are "Factory Test Modes" which the
>> contents of are not documented. They aren't really "not defied", and
>> aren't really "reserved" .. Marvell support claims they don't
>> support these drivers, and Freescale seems to be adding these
>> drivers, and the line we are looking at.
>>
>> We had some issues with our PHY which were corrected with the same
>> patch Harini used but modified for the M88E1112. We're trying to get
>> to the bottom of where this code came from and what it was suppose
>> to be doing.
> I tried to find this errata fix in the Marvell reference code. And
> failed to find it. But it is "Vendor Crap" code, hard to find anything
> in it.
>
> My guess is, this errata just applies to one model of PHY, maybe even
> one revision of one model of a PHY. The hard bit is figuring out what
> actually needs it. Do you have access to Marvell datasheets?

According to Marvell this was errata for 88M1101 , and should not be 
applied to any other phy .. So we should be removing these lines and 
make a special aneg for 88M1101 then restore everything that doesn't 
need this back to the generic aneg,

Daniel

Re: Marvell phy errata origins?

[Andrew Lunn]

> According to Marvell this was errata for 88M1101 , and should not be
> applied to any other phy .. So we should be removing these lines and
> make a special aneg for 88M1101 then restore everything that doesn't
> need this back to the generic aneg,

Hi Daniel

Thanks for finding this out. Can you role a patch?

       Andrew

Re: Marvell phy errata origins?

[Daniel Walker]

On 04/18/2017 07:04 AM, Andrew Lunn wrote:
> On Tue, Apr 18, 2017 at 06:16:33AM -0700, Daniel Walker wrote:
>>
>> Hi,
>>
>> Cisco is using a Marvell 88E1112 phy. It seems to be fairly similar
>> to the 88E1111 which Harini added a fix for.
> 
> Hi Daniel
> 
> If you look at Marvell reference drive, DSDT, they are actually quite
> different. Different virtual cable tester, different downshift
> configuration, different packet generator, different loopback. I would
> say they are different generations of PHY.
> 
>> In Harini's commit
>> message for ,
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/net/phy/marvell.c?id=3ec0a0f10ceb
>>
>> "This function has a sequence accessing Page 5 and Register 31, both
>> of which are not defined or reserved for this PHY"


I hope this this thread isn't too old to bring back to life. So it seems 
that Harini found that m88e1111 did not need this errata, and Cisco 
previously found that Harini's patch fixed m88e1112, we included it 
internally for that reason

Now I'm getting reports that this errata fixes issues we're seeing on 
m88e1111. We see an interrupt storm without the errata, despite the 
errata not being defined in the datasheet.

I would just send a patch adding the errata, but because Harini removed 
it I guess we really need to suss out what's going on.

I've added Gokul who reported the issue to me. Is it possible that 
Harini and Cisco have different m88e1111 phys? Maybe there's an issue 
with how they are hooked up ?

Daniel

Re: Marvell phy errata origins?

[Andrew Lunn]

> I've added Gokul who reported the issue to me. Is it possible that Harini
> and Cisco have different m88e1111 phys? Maybe there's an issue with how they
> are hooked up ?

Hi Daniel

The lower 4 bits of the ID registers normally indicate the revision of
the PHY. It might be worth checking if everybody has the same
revision, or the problems are limited to just one revision.

	  Andrew

Re: Marvell phy errata origins?

[Andrew Lunn]

> I hope this this thread isn't too old to bring back to life. So it seems
> that Harini found that m88e1111 did not need this errata, and Cisco
> previously found that Harini's patch fixed m88e1112, we included it
> internally for that reason
> 
> Now I'm getting reports that this errata fixes issues we're seeing on
> m88e1111. We see an interrupt storm without the errata, despite the errata
> not being defined in the datasheet.

Is everybody actually using interrupts? It could be in one system
phylib is polling.

When you get an interrupt storm, which interrupt bit is not cleared by
reading the ievent register?

	Andrew

Re: Marvell phy errata origins?

[Harini Katakam]

Hi Daniel,

On Tue, Sep 25, 2018 at 9:10 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > I hope this this thread isn't too old to bring back to life. So it seems
> > that Harini found that m88e1111 did not need this errata, and Cisco
> > previously found that Harini's patch fixed m88e1112, we included it
> > internally for that reason
> >
> > Now I'm getting reports that this errata fixes issues we're seeing on
> > m88e1111. We see an interrupt storm without the errata, despite the errata
> > not being defined in the datasheet.
>
> Is everybody actually using interrupts? It could be in one system
> phylib is polling.
>

Yes, we weren't using interrupts; we used phy poll.

As I recall, the register and page combination was reserved and
the access seemed to fail.
It will be useful if we can the errata description or version details.
I'll check if I can get any more information.

Regards,
Harini

Re: Marvell phy errata origins?

[Harini Katakam]

Hi,
On Tue, Sep 25, 2018 at 11:00 PM Harini Katakam <harinik@xilinx.com> wrote:
>
> Hi Daniel,
>
> On Tue, Sep 25, 2018 at 9:10 PM Andrew Lunn <andrew@lunn.ch> wrote:
> >
> > > I hope this this thread isn't too old to bring back to life. So it seems
> > > that Harini found that m88e1111 did not need this errata, and Cisco
> > > previously found that Harini's patch fixed m88e1112, we included it
> > > internally for that reason
> > >
> > > Now I'm getting reports that this errata fixes issues we're seeing on
> > > m88e1111. We see an interrupt storm without the errata, despite the errata
> > > not being defined in the datasheet.
> >
> > Is everybody actually using interrupts? It could be in one system
> > phylib is polling.
> >
>
> Yes, we weren't using interrupts; we used phy poll.
>
> As I recall, the register and page combination was reserved and
> the access seemed to fail.
> It will be useful if we can the errata description or version details.
> I'll check if I can get any more information.

One of the PHY parts used was "88E1111-B2-bab1i000"

Regards,
Harini

Re: Marvell phy errata origins?

[Daniel Walker]

On 09/25/2018 10:42 PM, Harini Katakam wrote:
> Hi,
> On Tue, Sep 25, 2018 at 11:00 PM Harini Katakam <harinik@xilinx.com> wrote:
>>
>> Hi Daniel,
>>
>> On Tue, Sep 25, 2018 at 9:10 PM Andrew Lunn <andrew@lunn.ch> wrote:
>>>
>>>> I hope this this thread isn't too old to bring back to life. So it seems
>>>> that Harini found that m88e1111 did not need this errata, and Cisco
>>>> previously found that Harini's patch fixed m88e1112, we included it
>>>> internally for that reason
>>>>
>>>> Now I'm getting reports that this errata fixes issues we're seeing on
>>>> m88e1111. We see an interrupt storm without the errata, despite the errata
>>>> not being defined in the datasheet.
>>>
>>> Is everybody actually using interrupts? It could be in one system
>>> phylib is polling.
>>>
>>
>> Yes, we weren't using interrupts; we used phy poll.
>>
>> As I recall, the register and page combination was reserved and
>> the access seemed to fail.
>> It will be useful if we can the errata description or version details.
>> I'll check if I can get any more information.
> 
> One of the PHY parts used was "88E1111-B2-bab1i000"

I doubt I can find this level of detail .. We have many of these 
machines in the field so they may have different part numbers.

I may have been given some incorrect details on the issue. I'm not 
currently sure this errata code is related. I'll let you know when I 
have more information.

Daniel

Re: Marvell phy errata origins?

[GokulChand Casheekar (gcasheek)]

Sorry - I was sick and out of office.
The phy_id reads 0x01410cc1. 

Regards,

Gokul.
On 26/09/18, 9:05 PM, "Daniel Walker (danielwa)" <danielwa@cisco.com> wrote:

    On 09/25/2018 10:42 PM, Harini Katakam wrote:
    > Hi,
    > On Tue, Sep 25, 2018 at 11:00 PM Harini Katakam <harinik@xilinx.com> wrote:
    >>
    >> Hi Daniel,
    >>
    >> On Tue, Sep 25, 2018 at 9:10 PM Andrew Lunn <andrew@lunn.ch> wrote:
    >>>
    >>>> I hope this this thread isn't too old to bring back to life. So it seems
    >>>> that Harini found that m88e1111 did not need this errata, and Cisco
    >>>> previously found that Harini's patch fixed m88e1112, we included it
    >>>> internally for that reason
    >>>>
    >>>> Now I'm getting reports that this errata fixes issues we're seeing on
    >>>> m88e1111. We see an interrupt storm without the errata, despite the errata
    >>>> not being defined in the datasheet.
    >>>
    >>> Is everybody actually using interrupts? It could be in one system
    >>> phylib is polling.
    >>>
    >>
    >> Yes, we weren't using interrupts; we used phy poll.
    >>
    >> As I recall, the register and page combination was reserved and
    >> the access seemed to fail.
    >> It will be useful if we can the errata description or version details.
    >> I'll check if I can get any more information.
    > 
    > One of the PHY parts used was "88E1111-B2-bab1i000"
    
    I doubt I can find this level of detail .. We have many of these 
    machines in the field so they may have different part numbers.
    
    I may have been given some incorrect details on the issue. I'm not 
    currently sure this errata code is related. I'll let you know when I 
    have more information.
    
    Daniel
    

Re: Marvell phy errata origins?

[GokulChand Casheekar (gcasheek)]

The phy_id where I see the interrupt storm is :
0x01410cc2

Regards,

Gokul.

On 01/10/18, 3:56 PM, "GokulChand Casheekar (gcasheek)" <gcasheek@cisco.com> wrote:

    Sorry - I was sick and out of office.
    The phy_id reads 0x01410cc1. 
    
    Regards,
    
    Gokul.
    On 26/09/18, 9:05 PM, "Daniel Walker (danielwa)" <danielwa@cisco.com> wrote:
    
        On 09/25/2018 10:42 PM, Harini Katakam wrote:
        > Hi,
        > On Tue, Sep 25, 2018 at 11:00 PM Harini Katakam <harinik@xilinx.com> wrote:
        >>
        >> Hi Daniel,
        >>
        >> On Tue, Sep 25, 2018 at 9:10 PM Andrew Lunn <andrew@lunn.ch> wrote:
        >>>
        >>>> I hope this this thread isn't too old to bring back to life. So it seems
        >>>> that Harini found that m88e1111 did not need this errata, and Cisco
        >>>> previously found that Harini's patch fixed m88e1112, we included it
        >>>> internally for that reason
        >>>>
        >>>> Now I'm getting reports that this errata fixes issues we're seeing on
        >>>> m88e1111. We see an interrupt storm without the errata, despite the errata
        >>>> not being defined in the datasheet.
        >>>
        >>> Is everybody actually using interrupts? It could be in one system
        >>> phylib is polling.
        >>>
        >>
        >> Yes, we weren't using interrupts; we used phy poll.
        >>
        >> As I recall, the register and page combination was reserved and
        >> the access seemed to fail.
        >> It will be useful if we can the errata description or version details.
        >> I'll check if I can get any more information.
        > 
        > One of the PHY parts used was "88E1111-B2-bab1i000"
        
        I doubt I can find this level of detail .. We have many of these 
        machines in the field so they may have different part numbers.
        
        I may have been given some incorrect details on the issue. I'm not 
        currently sure this errata code is related. I'll let you know when I 
        have more information.
        
        Daniel