net: phy: genphy: Allow overwriting features

PATCH New Alexey Brodkin 2 participants 6 messages Dec 23, 2015 – Jan 11, 2016

Subsystems (2)

Network drivers Ethernet PHY

lore

Share & tools

lore

Alexey Brodkin

author

2015-12-23 16:44 UTC #0 lore raw

PATCH 1/1 net: phy: genphy: Allow overwriting features

From: Sascha Hauer <[email protected]>

of_set_phy_supported allows overwiting hardware capabilities of a phy with values from the devicetree. This does not work with the genphy driver though because the genphys config_init function will overwrite all values adjusted by of_set_phy_supported. Fix this by initialising the genphy features in the phy_driver struct and in config_init just limit the features to the ones the hardware can actually support. The resulting features are a subset of the devicetree specified features and the hardware features.

This is a copy of the patch from Linux kernel, see
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=c242a47238fa2a6a54af8a16e62b54e6e031d4bc

Signed-off-by Sascha Hauer <[email protected]>

Signed-off-by Alexey Brodkin <[email protected]>

Cc Joe Hershberger <[email protected]>

---
 drivers/net/phy/phy.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

drivers/net/phy/phy.c +5 -5

index 084276f..ec9be6b 100644
@@ -380,8 +380,6 @@ int genphy_config(struct phy_device *phydev)
 	int val;
 	u32 features;
 
-	/* For now, I'll claim that the generic driver supports
-	 * all possible port types */
 	features = (SUPPORTED_TP | SUPPORTED_MII
 			| SUPPORTED_AUI | SUPPORTED_FIBRE |
 			SUPPORTED_BNC);

@@ -420,8 +418,8 @@ int genphy_config(struct phy_device *phydev)
 			features |= SUPPORTED_1000baseX_Half;
 	}
 
-	phydev->supported = features;
-	phydev->advertising = features;
+	phydev->supported &= features;
+	phydev->advertising &= features;
 
 	genphy_config_aneg(phydev);

@@ -445,7 +443,9 @@ static struct phy_driver genphy_driver = {
 	.uid		= 0xffffffff,
 	.mask		= 0xffffffff,
 	.name		= "Generic PHY",
-	.features	= 0,
+	.features	= PHY_GBIT_FEATURES | SUPPORTED_MII |
+			  SUPPORTED_AUI | SUPPORTED_FIBRE |
+			  SUPPORTED_BNC,
 	.config		= genphy_config,
 	.startup	= genphy_startup,
 	.shutdown	= genphy_shutdown,

Signature · 1 line

2.4.3

Alexey Brodkin

author

2016-01-11 09:45 UTC #1 lore raw

PATCH 1/1 net: phy: genphy: Allow overwriting features

Hi Joe,

Quoted 46 lines · Alexey Brodkin

From: Sascha Hauer <[email protected]>
of_set_phy_supported allows overwiting hardware capabilities of a phy with values from the devicetree. This does not work with the genphy driver though because the genphys config_init function will overwrite all values adjusted by of_set_phy_supported. Fix this by initialising the genphy features in the phy_driver struct and in config_init just limit the features to the ones the hardware can actually support. The resulting features are a subset of the devicetree specified features and the hardware features.
This is a copy of the patch from Linux kernel, see
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=c242a47238fa2a6a54af8a16e62b54e6e031d4bc
Signed-off-by Sascha Hauer <[email protected]>
Signed-off-by Alexey Brodkin <[email protected]>
Cc Joe Hershberger <[email protected]>
---
 drivers/net/phy/phy.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
drivers/net/phy/phy.c +2 -4
index 084276f..ec9be6b 100644
@@ -380,8 +380,6 @@ int genphy_config(struct phy_device *phydev)
 	int val;
 	u32 features;
 
-	/* For now, I'll claim that the generic driver supports
-	 * all possible port types */
 	features = (SUPPORTED_TP | SUPPORTED_MII
 			| SUPPORTED_AUI | SUPPORTED_FIBRE |
 			SUPPORTED_BNC);
@@ -420,8 +418,8 @@ int genphy_config(struct phy_device *phydev)
 			features |= SUPPORTED_1000baseX_Half;
 	}
 
-	phydev->supported = features;
-	phydev->advertising = features;
+	phydev->supported &= features;
+	phydev->advertising &= features;
 
 	genphy_config_aneg(phydev);
 

drivers/net/phy/phy.c static struct phy_driver genphy_driver = { · line 451

Quoted 10 lines

drivers/net/phy/phy.c +3 -1

@@ -445,7 +443,9 @@ static struct phy_driver genphy_driver = {
 	.uid		= 0xffffffff,
 	.mask		= 0xffffffff,
 	.name		= "Generic PHY",
-	.features	= 0,
+	.features	= PHY_GBIT_FEATURES | SUPPORTED_MII |
+			  SUPPORTED_AUI | SUPPORTED_FIBRE |
+			  SUPPORTED_BNC,
 	.config		= genphy_config,
 	.startup	= genphy_startup,
 	.shutdown	= genphy_shutdown,

Any chance for that one to be applied?
This patch is required to implement phy max
speed limitation by subsequent patches.

-Alexey

Joe Hershberger

2016-01-11 16:54 UTC #2 lore raw

PATCH 1/1 net: phy: genphy: Allow overwriting features

Hi Alexey,

Quoted 17 lines · Alexey Brodkin

Hi Joe,
Quoted 14 lines · Alexey Brodkin
From: Sascha Hauer <[email protected]>
of_set_phy_supported allows overwiting hardware capabilities of a phy with values from the devicetree. This does not work with the genphy driver though because the genphys config_init function will overwrite all values adjusted by of_set_phy_supported. Fix this by initialising the genphy features in the phy_driver struct and in config_init just limit the features to the ones the hardware can actually support. The resulting features are a subset of the devicetree specified features and the hardware features.
This is a copy of the patch from Linux kernel, see
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=c242a47238fa2a6a54af8a16e62b54e6e031d4bc

Quoted 6 lines

Quoted 4 lines
Signed-off-by Sascha Hauer <[email protected]>
Signed-off-by Alexey Brodkin <[email protected]>
Cc Joe Hershberger <[email protected]>
---
Any chance for that one to be applied?

I'll review when the merge window opens.

Quoted 2 lines

This patch is required to implement phy max
speed limitation by subsequent patches.

Any reason you did not send as a series if there are dependencies?

Thanks,
-Joe

Alexey Brodkin

author

2016-01-11 17:50 UTC #3 lore raw

PATCH 1/1 net: phy: genphy: Allow overwriting features

Hi Joe,

Quoted 26 lines · Joe Hershberger

Hi Alexey,
Quoted 22 lines · Alexey Brodkin
Hi Joe,
Quoted 18 lines · Alexey Brodkin
From: Sascha Hauer <[email protected]>
of_set_phy_supported allows overwiting hardware capabilities of a phy with values from the devicetree. This does not work with the genphy driver though because the genphys config_init function will overwrite all values adjusted by of_set_phy_supported. Fix this by initialising the genphy features in the phy_driver struct and in config_init just limit the features to the ones the hardware can actually support. The resulting features are a subset of the devicetree specified features and the hardware features.
This is a copy of the patch from Linux kernel, see
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=c242a47238fa2a6a54af8a16e62b54e6e031d4bc
Signed-off-by Sascha Hauer <[email protected]>
Signed-off-by Alexey Brodkin <[email protected]>
Cc Joe Hershberger <[email protected]>
---

Quoted 8 lines

Quoted 1 line
Any chance for that one to be applied?
I'll review when the merge window opens.
Quoted 2 lines
This patch is required to implement phy max
speed limitation by subsequent patches.
Any reason you did not send as a series if there are dependencies?

I thought about putting some of those patches in one series initially but then decided to send them separately.

Even though together they solve one particular problem (ability to set phy speed limit) they are a bit different by their nature.

http://patchwork.ozlabs.org/patch/560608/,
http://patchwork.ozlabs.org/patch/560634/ and
http://patchwork.ozlabs.org/patch/560635/ are back-ports from Linux kernel and could be actually applied separately because they are not related to each other.

Following two are really preparatory for implementing capping:
http://patchwork.ozlabs.org/patch/560636/
http://patchwork.ozlabs.org/patch/560637/

...in patch I actually forgot to send out... (will do it shortly).

And finally http://patchwork.ozlabs.org/patch/560638/ really a plain fix for DW GMAC driver which may happen in case of phy force set lower than possible. So it will easily manifest if all above is applied.

That said it was conscious decision but probably incorrect one.

If you do think it all fits well in a series I'll re-send it that way.

-Alexey

Joe Hershberger

2016-01-11 17:55 UTC #4 lore raw

PATCH 1/1 net: phy: genphy: Allow overwriting features

Hi Alexey,

Quoted 56 lines · Alexey Brodkin

Hi Joe,
Quoted 26 lines · Joe Hershberger
Hi Alexey,
Quoted 22 lines · Alexey Brodkin
Hi Joe,
Quoted 18 lines · Alexey Brodkin
From: Sascha Hauer <[email protected]>
of_set_phy_supported allows overwiting hardware capabilities of a phy with values from the devicetree. This does not work with the genphy driver though because the genphys config_init function will overwrite all values adjusted by of_set_phy_supported. Fix this by initialising the genphy features in the phy_driver struct and in config_init just limit the features to the ones the hardware can actually support. The resulting features are a subset of the devicetree specified features and the hardware features.
This is a copy of the patch from Linux kernel, see
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=c242a47238fa2a6a54af8a16e62b54e6e031d4bc
Signed-off-by Sascha Hauer <[email protected]>
Signed-off-by Alexey Brodkin <[email protected]>
Cc Joe Hershberger <[email protected]>
---
Quoted 8 lines
Quoted 1 line
Any chance for that one to be applied?
I'll review when the merge window opens.
Quoted 2 lines
This patch is required to implement phy max
speed limitation by subsequent patches.
Any reason you did not send as a series if there are dependencies?
I thought about putting some of those patches in one series initially but then decided to send them separately.
Even though together they solve one particular problem (ability to set phy speed limit) they are a bit different by their nature.
http://patchwork.ozlabs.org/patch/560608/,
http://patchwork.ozlabs.org/patch/560634/ and
http://patchwork.ozlabs.org/patch/560635/ are back-ports from Linux kernel and could be actually applied separately because they are not related to each other.
Following two are really preparatory for implementing capping:
http://patchwork.ozlabs.org/patch/560636/
http://patchwork.ozlabs.org/patch/560637/
...in patch I actually forgot to send out... (will do it shortly).

Quoted 7 lines

And finally http://patchwork.ozlabs.org/patch/560638/ really a plain fix for DW GMAC driver which may happen in case of phy force set lower than possible. So it will easily manifest if all above is applied.
That said it was conscious decision but probably incorrect one.
If you do think it all fits well in a series I'll re-send it that way.

If there is no build or functionality breaking order dependency then it's ok that they are not in a series. If there is any dependency like that, then I would appreciate a series so that I know what order to apply them without having to figure it out.

Thanks,
-Joe

Alexey Brodkin

author

2016-01-11 17:57 UTC #5 lore raw

PATCH 1/1 net: phy: genphy: Allow overwriting features

Hi Joe,

Quoted 65 lines · Joe Hershberger

Hi Alexey,
Quoted 61 lines · Alexey Brodkin
Hi Joe,
Quoted 27 lines · Joe Hershberger
Hi Alexey,
Quoted 23 lines · Alexey Brodkin
Hi Joe,
Quoted 19 lines · Alexey Brodkin
From: Sascha Hauer <[email protected]>
of_set_phy_supported allows overwiting hardware capabilities of a phy with values from the devicetree. This does not work with the genphy driver though because the genphys config_init function will overwrite all values adjusted by of_set_phy_supported. Fix this by initialising the genphy features in the phy_driver struct and in config_init just limit the features to the ones the hardware can actually support. The resulting features are a subset of the devicetree specified features and the hardware features.
This is a copy of the patch from Linux kernel, see
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=c242a47238fa2a6a54af8a16e62b54e6e031
d4bc
Signed-off-by Sascha Hauer <[email protected]>
Signed-off-by Alexey Brodkin <[email protected]>
Cc Joe Hershberger <[email protected]>
---
Quoted 8 lines
Quoted 1 line
Any chance for that one to be applied?
I'll review when the merge window opens.
Quoted 2 lines
This patch is required to implement phy max
speed limitation by subsequent patches.
Any reason you did not send as a series if there are dependencies?
I thought about putting some of those patches in one series initially but then decided to send them separately.
Even though together they solve one particular problem (ability to set phy speed limit) they are a bit different by their nature.
http://patchwork.ozlabs.org/patch/560608/,
http://patchwork.ozlabs.org/patch/560634/ and
http://patchwork.ozlabs.org/patch/560635/ are back-ports from Linux kernel and could be actually applied separately because they are not related to each other.
Following two are really preparatory for implementing capping:
http://patchwork.ozlabs.org/patch/560636/
http://patchwork.ozlabs.org/patch/560637/
...in patch I actually forgot to send out... (will do it shortly).
And finally http://patchwork.ozlabs.org/patch/560638/ really a plain fix for DW GMAC driver which may happen in case of phy force set lower than possible. So it will easily manifest if all above is applied.

Quoted 8 lines

Quoted 3 lines
That said it was conscious decision but probably incorrect one.
If you do think it all fits well in a series I'll re-send it that way.
If there is no build or functionality breaking order dependency then it's ok that they are not in a series. If there is any dependency like that, then I would appreciate a series so that I know what order to apply them without having to figure it out.

Ok then I'll re-send them all as a series now.

-Alexey