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.
Signed-off-by: Byungho An <[email protected]>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index c6cdbc4..a719c87 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1037,6 +1037,7 @@ static int stmmac_open(struct net_device *dev)
{
struct stmmac_priv *priv = netdev_priv(dev);
int ret;
+ u32 value;
#ifdef CONFIG_STMMAC_TIMER
priv->tm = kzalloc(sizeof(struct stmmac_timer *), GFP_KERNEL);
@@ -1088,6 +1089,15 @@ static int stmmac_open(struct net_device *dev)
/* Initialize the MAC Core */
priv->hw->mac->core_init(priv->ioaddr);
+ 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);
+ }
+
/* Request the IRQ lines */
ret = request_irq(dev->irq, stmmac_interrupt,
IRQF_SHARED, dev->name, dev);
--
1.7.9.5
Hello An
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.
I was looking at this too. In particular, I was working on the rgmii
interrupt so I guess we could improve this part together.
First my note is that I would like to have this kind of code never
placed in the stmmac_main. It should stay in the core part.
Also I 'd like to avoid the Kconfig option where possible.
At any rate, I'll come back with further details soon.
BR,
Peppe
>
> Signed-off-by: Byungho An <[email protected]>
> ---
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index c6cdbc4..a719c87 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -1037,6 +1037,7 @@ static int stmmac_open(struct net_device *dev)
> {
> struct stmmac_priv *priv = netdev_priv(dev);
> int ret;
> + u32 value;
>
> #ifdef CONFIG_STMMAC_TIMER
> priv->tm = kzalloc(sizeof(struct stmmac_timer *), GFP_KERNEL);
> @@ -1088,6 +1089,15 @@ static int stmmac_open(struct net_device *dev)
> /* Initialize the MAC Core */
> priv->hw->mac->core_init(priv->ioaddr);
>
> + 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);
> + }
> +
> /* Request the IRQ lines */
> ret = request_irq(dev->irq, stmmac_interrupt,
> IRQF_SHARED, dev->name, dev);
>
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.
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.
I do not see why we have to use Kconfig macro to select ANE etc
(as you do in your patches).
The driver could directly manage the phy device by itself if possible
and the stmmac_init_phy should be reworked.
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.
The code has to be able to maintain the user interface.
For example if you want to enable ANE or manage Advertisement caps.
> Signed-off-by: Byungho An <[email protected]>
> ---
[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
Pls, do not use value |= 0x1000000 but provide the appropriate defines.
> /* Request the IRQ lines */
> ret = request_irq(dev->irq, stmmac_interrupt,
> IRQF_SHARED, dev->name, dev);
>
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 <[email protected]>
> > ---
>
> [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.