Re: [PATCH net-next v3 3/5] net: pcs: add new PCS driver for altera TSE PCS

[Russell King (Oracle)]

On Wed, Oct 26, 2022 at 11:37:11AM +0200, Maxime Chevallier wrote:
> Hello Sean,
> 
> On Sun, 9 Oct 2022 01:38:15 -0400
> Sean Anderson <seanga2@xxxxxxxxx> wrote:
> 
> 
> > > +#define   SGMII_PCS_LINK_TIMER_REG(x)		(0x12 + (x))  
> > 
> > Not used.
> 
> Right, I'll remove that in a followup patch
> 
> > > +#define SGMII_PCS_LINK_TIMER_1	0x13
> > > +#define SGMII_PCS_IF_MODE	0x14
> > > +#define   PCS_IF_MODE_SGMII_ENA		BIT(0)
> > > +#define   PCS_IF_MODE_USE_SGMII_AN	BIT(1)
> > > +#define   PCS_IF_MODE_SGMI_SPEED_MASK	GENMASK(3, 2)
> > > +#define   PCS_IF_MODE_SGMI_SPEED_10	(0 << 2)
> > > +#define   PCS_IF_MODE_SGMI_SPEED_100	(1 << 2)
> > > +#define   PCS_IF_MODE_SGMI_SPEED_1000	(2 << 2)  
> > 
> > You can use FIELD_PREP if you're so inclined. I assume SGMI is from
> > the datasheet.
> 
> Will do ! thanks :)
> 
> > > +#define   PCS_IF_MODE_SGMI_HALF_DUPLEX	BIT(4)
> > > +#define   PCS_IF_MODE_SGMI_PHY_ANi	BIT(5)

The definitions up to here look very similar to pcs-lynx.c when it comes
to 1000base-X and SGMII. I wonder whether regmap can help here to
abstract the register access differences and then maybe code can be
shared.

What value is in registers 2 and 3 (the ID registers) for this PCS?

On the link timer value setting, I have a patch to add a phylink helper
which returns the link timer in nanoseconds. May be a good idea if we
get that queued up so drivers can make use of it rather than hard-coding
a register value everywhere.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!