All of lore.kernel.org
 help / color / mirror / Atom feed
From: Maxime Chevallier <maxime.chevallier@bootlin.com>
To: "Russell King (Oracle)" <linux@armlinux.org.uk>
Cc: davem@davemloft.net, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, thomas.petazzoni@bootlin.com,
	Andrew Lunn <andrew@lunn.ch>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	linux-arm-kernel@lists.infradead.org,
	Horatiu.Vultur@microchip.com, Allan.Nielsen@microchip.com,
	UNGLinuxDriver@microchip.com,
	Vladimir Oltean <vladimir.oltean@nxp.com>
Subject: Re: [PATCH net 2/2] net: phylink: use USXGMII control-word format to parse Q-USGMII word
Date: Thu, 8 Jun 2023 19:53:30 +0200	[thread overview]
Message-ID: <20230608195330.476fca86@pc-7.home> (raw)
In-Reply-To: <ZIICtN5zrxvTuEwz@shell.armlinux.org.uk>

Hi Russell,

On Thu, 8 Jun 2023 17:32:52 +0100
"Russell King (Oracle)" <linux@armlinux.org.uk> wrote:

> On Thu, Jun 08, 2023 at 06:34:15PM +0200, Maxime Chevallier wrote:
> > diff --git a/include/uapi/linux/mdio.h b/include/uapi/linux/mdio.h
> > index 256b463e47a6..1d20a9082507 100644
> > --- a/include/uapi/linux/mdio.h
> > +++ b/include/uapi/linux/mdio.h
> > @@ -444,4 +444,7 @@ static inline __u16 mdio_phy_id_c45(int prtad, int devad)
> >  #define MDIO_USXGMII_5000FULL		0x1a00	/* 5000Mbps full-duplex */
> >  #define MDIO_USXGMII_LINK		0x8000	/* PHY link with copper-side partner */
> >  
> > +/* Usgmii control word is based on Usxgmii, masking away 2.5, 5 and 10Gbps */
> > +#define MDIO_USGMII_SPD_MASK		0x0600  
> 
> This isn't correct:
> 
> 11:9	Speed: Bit 11, 10, 9:
> 	1 1 1 to 011 = Reserved
> 	0 1 0 = 1000 Mbps: 1000BASE-TX, 1000BASE-X
> 	0 0 1 = 100 Mbps: 100BASE-TX, 100BASE-FX
> 	0 0 0 = 10 Mbps: 10BASET, 10BASE2, 10BASE5
> 
> If we only look at bits 10 and 9, then we're interpreting the reserved
> combinations as valid as well.

That's why I rewrote the decoding helper instead of simply masking away
the extra bit, so that we exclude the 0 1 1 combination ( 10G speed ).

The comment is wrong though.

This patch set needs more coffee,

Best regards,

Maxime

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: Maxime Chevallier <maxime.chevallier@bootlin.com>
To: "Russell King (Oracle)" <linux@armlinux.org.uk>
Cc: davem@davemloft.net, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, thomas.petazzoni@bootlin.com,
	Andrew Lunn <andrew@lunn.ch>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	linux-arm-kernel@lists.infradead.org,
	Horatiu.Vultur@microchip.com, Allan.Nielsen@microchip.com,
	UNGLinuxDriver@microchip.com,
	Vladimir Oltean <vladimir.oltean@nxp.com>
Subject: Re: [PATCH net 2/2] net: phylink: use USXGMII control-word format to parse Q-USGMII word
Date: Thu, 8 Jun 2023 19:53:30 +0200	[thread overview]
Message-ID: <20230608195330.476fca86@pc-7.home> (raw)
In-Reply-To: <ZIICtN5zrxvTuEwz@shell.armlinux.org.uk>

Hi Russell,

On Thu, 8 Jun 2023 17:32:52 +0100
"Russell King (Oracle)" <linux@armlinux.org.uk> wrote:

> On Thu, Jun 08, 2023 at 06:34:15PM +0200, Maxime Chevallier wrote:
> > diff --git a/include/uapi/linux/mdio.h b/include/uapi/linux/mdio.h
> > index 256b463e47a6..1d20a9082507 100644
> > --- a/include/uapi/linux/mdio.h
> > +++ b/include/uapi/linux/mdio.h
> > @@ -444,4 +444,7 @@ static inline __u16 mdio_phy_id_c45(int prtad, int devad)
> >  #define MDIO_USXGMII_5000FULL		0x1a00	/* 5000Mbps full-duplex */
> >  #define MDIO_USXGMII_LINK		0x8000	/* PHY link with copper-side partner */
> >  
> > +/* Usgmii control word is based on Usxgmii, masking away 2.5, 5 and 10Gbps */
> > +#define MDIO_USGMII_SPD_MASK		0x0600  
> 
> This isn't correct:
> 
> 11:9	Speed: Bit 11, 10, 9:
> 	1 1 1 to 011 = Reserved
> 	0 1 0 = 1000 Mbps: 1000BASE-TX, 1000BASE-X
> 	0 0 1 = 100 Mbps: 100BASE-TX, 100BASE-FX
> 	0 0 0 = 10 Mbps: 10BASET, 10BASE2, 10BASE5
> 
> If we only look at bits 10 and 9, then we're interpreting the reserved
> combinations as valid as well.

That's why I rewrote the decoding helper instead of simply masking away
the extra bit, so that we exclude the 0 1 1 combination ( 10G speed ).

The comment is wrong though.

This patch set needs more coffee,

Best regards,

Maxime

  reply	other threads:[~2023-06-08 17:18 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-08 16:34 [PATCH net 0/2] fixes for Q-USGMII speeds and autoneg Maxime Chevallier
2023-06-08 16:34 ` Maxime Chevallier
2023-06-08 16:34 ` [PATCH net 1/2] net: phylink: report correct max speed for QUSGMII Maxime Chevallier
2023-06-08 16:34   ` Maxime Chevallier
2023-06-08 16:34 ` [PATCH net 2/2] net: phylink: use a dedicated helper to parse usgmii control word Maxime Chevallier
2023-06-08 16:34   ` Maxime Chevallier
2023-06-08 16:33   ` Russell King (Oracle)
2023-06-08 16:33     ` Russell King (Oracle)
2023-06-08 17:54     ` Maxime Chevallier
2023-06-08 17:54       ` Maxime Chevallier
2023-06-08 16:34 ` [PATCH net 2/2] net: phylink: use USXGMII control-word format to parse Q-USGMII word Maxime Chevallier
2023-06-08 16:34   ` Maxime Chevallier
2023-06-08 16:32   ` Russell King (Oracle)
2023-06-08 16:32     ` Russell King (Oracle)
2023-06-08 17:53     ` Maxime Chevallier [this message]
2023-06-08 17:53       ` Maxime Chevallier
2023-06-08 17:30       ` Russell King (Oracle)
2023-06-08 17:30         ` Russell King (Oracle)
2023-06-09  8:09         ` Maxime Chevallier
2023-06-09  8:09           ` Maxime Chevallier
2023-06-08 17:45   ` Simon Horman
2023-06-08 17:45     ` Simon Horman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230608195330.476fca86@pc-7.home \
    --to=maxime.chevallier@bootlin.com \
    --cc=Allan.Nielsen@microchip.com \
    --cc=Horatiu.Vultur@microchip.com \
    --cc=UNGLinuxDriver@microchip.com \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=f.fainelli@gmail.com \
    --cc=hkallweit1@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=netdev@vger.kernel.org \
    --cc=thomas.petazzoni@bootlin.com \
    --cc=vladimir.oltean@nxp.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.