| Message ID | 1450889081-21717-1-git-send-email-abrodkin@synopsys.com |
|---|---|
| State | Accepted, archived |
| Commit | 44bc317487386117a9a08d50be4cb105d512224c |
| Delegated to: | Joe Hershberger |
| Headers | show |
Hi Joe, On Wed, 2015-12-23 at 19:44 +0300, Alexey Brodkin wrote: > From: Sascha Hauer <s.hauer@pengutronix.de> > > 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 <s.hauer@pengutronix.de> > Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com> > Cc: Joe Hershberger <joe.hershberger@ni.com> > --- > drivers/net/phy/phy.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c > index 084276f..ec9be6b 100644 > --- a/drivers/net/phy/phy.c > +++ b/drivers/net/phy/phy.c > @@ -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, Any chance for that one to be applied? This patch is required to implement phy max speed limitation by subsequent patches. -Alexey
Hi Alexey, On Mon, Jan 11, 2016 at 3:45 AM, Alexey Brodkin <Alexey.Brodkin@synopsys.com> wrote: > Hi Joe, > > On Wed, 2015-12-23 at 19:44 +0300, Alexey Brodkin wrote: >> From: Sascha Hauer <s.hauer@pengutronix.de> >> >> 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 <s.hauer@pengutronix.de> >> Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com> >> Cc: Joe Hershberger <joe.hershberger@ni.com> >> --- > > Any chance for that one to be applied? I'll review when the merge window opens. > 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
Hi Joe, On Mon, 2016-01-11 at 10:54 -0600, Joe Hershberger wrote: > Hi Alexey, > > On Mon, Jan 11, 2016 at 3:45 AM, Alexey Brodkin > <Alexey.Brodkin@synopsys.com> wrote: > > Hi Joe, > > > > On Wed, 2015-12-23 at 19:44 +0300, Alexey Brodkin wrote: > > > From: Sascha Hauer <s.hauer@pengutronix.de> > > > > > > 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 <s.hauer@pengutronix.de> > > > Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com> > > > Cc: Joe Hershberger <joe.hershberger@ni.com> > > > --- > > > > Any chance for that one to be applied? > > I'll review when the merge window opens. > > > 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
Hi Alexey, On Mon, Jan 11, 2016 at 11:50 AM, Alexey Brodkin <Alexey.Brodkin@synopsys.com> wrote: > Hi Joe, > > On Mon, 2016-01-11 at 10:54 -0600, Joe Hershberger wrote: >> Hi Alexey, >> >> On Mon, Jan 11, 2016 at 3:45 AM, Alexey Brodkin >> <Alexey.Brodkin@synopsys.com> wrote: >> > Hi Joe, >> > >> > On Wed, 2015-12-23 at 19:44 +0300, Alexey Brodkin wrote: >> > > From: Sascha Hauer <s.hauer@pengutronix.de> >> > > >> > > 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 <s.hauer@pengutronix.de> >> > > Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com> >> > > Cc: Joe Hershberger <joe.hershberger@ni.com> >> > > --- >> > >> > Any chance for that one to be applied? >> >> I'll review when the merge window opens. >> >> > 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. 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
Hi Joe, On Mon, 2016-01-11 at 11:55 -0600, Joe Hershberger wrote: > Hi Alexey, > > On Mon, Jan 11, 2016 at 11:50 AM, Alexey Brodkin > <Alexey.Brodkin@synopsys.com> wrote: > > Hi Joe, > > > > On Mon, 2016-01-11 at 10:54 -0600, Joe Hershberger wrote: > > > Hi Alexey, > > > > > > On Mon, Jan 11, 2016 at 3:45 AM, Alexey Brodkin > > > <Alexey.Brodkin@synopsys.com> wrote: > > > > Hi Joe, > > > > > > > > On Wed, 2015-12-23 at 19:44 +0300, Alexey Brodkin wrote: > > > > > From: Sascha Hauer <s.hauer@pengutronix.de> > > > > > > > > > > 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 <s.hauer@pengutronix.de> > > > > > Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com> > > > > > Cc: Joe Hershberger <joe.hershberger@ni.com> > > > > > --- > > > > > > > > Any chance for that one to be applied? > > > > > > I'll review when the merge window opens. > > > > > > > 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. > > 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
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c index 084276f..ec9be6b 100644 --- a/drivers/net/phy/phy.c +++ b/drivers/net/phy/phy.c @@ -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,