Re: [PATCH net-next v2 2/2] net: phy: micrel: lan8814 cable improvement errata

[Horatiu Vultur - M31836]

The 03/01/2024 03:27, Arun Ramadoss - I17769 wrote:
> Hi Horatiu,

Hi Arun,

> 
> On Thu, 2024-02-29 at 20:52 +0100, Horatiu Vultur wrote:
> > When the length of the cable is more than 100m and the lan8814 is
> > configured to run in 1000Base-T Slave then the register of the device
> > needs to be optimized.
> > 
> > Workaround this by setting the measure time to a value of 0xb. This
> > value can be set regardless of the configuration.
> > 
> > This issue is described in 'LAN8814 Silicon Errata and Data Sheet
> > Clarification' and according to that, this will not be corrected in a
> > future silicon revision.
> > 
> > Signed-off-by: Horatiu Vultur <horatiu.vultur@xxxxxxxxxxxxx>
> > ---
> >  drivers/net/phy/micrel.c | 19 +++++++++++++++++++
> >  1 file changed, 19 insertions(+)
> > 
> > diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> > index 88cc03982bb78..788fdd54fd22d 100644
> > --- a/drivers/net/phy/micrel.c
> > +++ b/drivers/net/phy/micrel.c
> > @@ -117,6 +117,10 @@
> >  #define LAN8814_EEE_STATE			0x38
> >  #define LAN8814_EEE_STATE_MASK2P5P		BIT(10)
> >  
> > +#define LAN8814_PD_CONTROLS			0x9d
> > +#define LAN8814_PD_CONTROLS_PD_MEAS_TIME_MASK_	GENMASK(3, 0)
> > +#define LAN8814_PD_CONTROLS_PD_MEAS_TIME_VAL_	0xb
> 
> nitpick: TIME_VAL macro is very generic if it can end with specific
> like TIME_VAL_100M or something similar will gives more readability.

Actually I prefer to keep it like this the name if it is possible..
Because the VAL_ represents the value and MASK_ represents the mask
value. Therefore the actual bits name of the register is
LAN8814_PD_CONTROLS_PD_MEAS_TIME.

I am trying to have a naming convetion about how to define names in this
file:
<TARGET>_<REG_NAME>_<REG_BITS_NAME>

In this way it way it is easier to find in the datasheet to what it
refers to.

> 
> > +
> > 

-- 
/Horatiu