RE: [PATCH 1/3] net: stmmac: change GMAC control register for SGMII

[Byungho An <bh74.an@samsung.com>]

On 11/26/2012 07:31 PM, Giuseppe CABALLARO wrote:
> On 11/23/2012 10:04 AM, Byungho An wrote:
> >
> > This patch changes GMAC control register (TC(Transmit
> > Configuration) and PS(Port Selection) bit for SGMII.
> > In case of SGMII, TC bit is '1' and PS bit is 0.
> 
> IMO this new support that should be released for net-next and further 
> effort is actually needed.
>
OK, I see but if possible, I want to support the new features which is
included in this patch from v3.8
 
> The availability of the PCS registers is given by looking at the HW 
> feature register. In fact, these are optional registers.
> I don't want to break the compatibility with old chips.
> 
It means that old chip doesn't have this bit or this register? If that, how
about using compatible in DT blob like snps,dwmac-3.70a and then in just
this case trying to read this bit and this register.

> I do not see why we have to use Kconfig macro to select ANE etc (as 
> you do in your patches).
OK. I agree with you.

> The driver could directly manage the phy device by itself if possible 
> and the stmmac_init_phy should be reworked.
> 
Could you explain more detail? As I understood, after set ANE bit in MAC
side then PHY auto-negotiation can be enabled. If I'm wrong let me know.
According to your mention, MAC and PHY auto-negotiation can be managed in
stmmac_init_phy?

> There are several things that need to be implemented. For example:
> 
> The ISR (e.g. priv->hw->mac->host_irq_status) should be able to manage 
> these new interrupts.
I think that there would be two additional interrupts."PCS Auto-Negotiation
Complete" and "PCS Link Status Changed". These two interrupts are added to
"stmmac_interrupt". In my opinion, there are no specific processing for
these two irqs. What do you think about it?

> The code has to be able to maintain the user interface.
> For example if you want to enable ANE or manage Advertisement caps.
> 
Does it mean that command line or other network command(e.g. ifconfig...) or
ioctol? Actually I don't understand exact user interface way. Could you
recommend the method for user interface?

> > Signed-off-by: Byungho An <bh74.an@samsung.com>
> > ---
> 
> [snip]
> 
> > +	if (priv->phydev->interface == PHY_INTERFACE_MODE_SGMII) {
> > +		value = readl(priv->ioaddr);
> > +		/* GMAC_CONTROL_TC : transmit config in RGMII/SGMII */
> > +		value |= 0x1000000;
> > +		/* GMAC_CONTROL_PS : Port Selection for GMII */
> > +		value &= ~(0x8000);
> > +		writel(value, priv->ioaddr);
> > +	}
> > +
> 
> 
> This parts of code have to be moved in 
> drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
> 
OK.

> Pls, do not use value |= 0x1000000 but provide the appropriate defines.
> 
OK.

> >   	/* Request the IRQ lines */
> >   	ret = request_irq(dev->irq, stmmac_interrupt,
> >   			 IRQF_SHARED, dev->name, dev);
> >
Thank you.
Byungho An.