Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 26 Oct 2022 13:47:52 +0100
From:   "Russell King (Oracle)" <linux@armlinux.org.uk>
To:     Maxime Chevallier <maxime.chevallier@bootlin.com>
Cc:     Sean Anderson <seanga2@gmail.com>, davem@davemloft.net,
        Rob Herring <robh+dt@kernel.org>, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org, thomas.petazzoni@bootlin.com,
        Andrew Lunn <andrew@lunn.ch>, Jakub Kicinski <kuba@kernel.org>,
        Eric Dumazet <edumazet@google.com>,
        Paolo Abeni <pabeni@redhat.com>,
        Florian Fainelli <f.fainelli@gmail.com>,
        Heiner Kallweit <hkallweit1@gmail.com>,
        linux-arm-kernel@lists.infradead.org,
        Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
        devicetree@vger.kernel.org
Subject: Re: [PATCH net-next v3 3/5] net: pcs: add new PCS driver for altera
 TSE PCS
Message-ID: <Y1kseONzzimgWQP3@shell.armlinux.org.uk>
References: <20220901143543.416977-1-maxime.chevallier@bootlin.com>
 <20220901143543.416977-4-maxime.chevallier@bootlin.com>
 <68b3dfbf-9bab-2554-254e-bffd280ba97e@gmail.com>
 <20221026113711.2b740c7a@pc-8.home>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20221026113711.2b740c7a@pc-8.home>
Sender: Russell King (Oracle) <linux@armlinux.org.uk>
Precedence: bulk

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@gmail.com> 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!