* [PATCH] net: phy: smsc: disable energy detect mode @ 2015-10-13 5:13 Heiko Schocher 2015-10-13 19:26 ` Florian Fainelli 0 siblings, 1 reply; 6+ messages in thread From: Heiko Schocher @ 2015-10-13 5:13 UTC (permalink / raw) To: linux-kernel Cc: Heiko Schocher, devicetree, netdev, Georg.Soffel, Florian Fainelli On some boards the energy enable detect mode leads in trouble with some switches, so make the enabling of this mode configurable through DT. Signed-off-by: Heiko Schocher <hs@denx.de> --- .../devicetree/bindings/net/smsc-lan87xx.txt | 19 +++++++++++++++++ drivers/net/phy/smsc.c | 24 +++++++++++++++++----- 2 files changed, 38 insertions(+), 5 deletions(-) create mode 100644 Documentation/devicetree/bindings/net/smsc-lan87xx.txt diff --git a/Documentation/devicetree/bindings/net/smsc-lan87xx.txt b/Documentation/devicetree/bindings/net/smsc-lan87xx.txt new file mode 100644 index 0000000..39aa1dc --- /dev/null +++ b/Documentation/devicetree/bindings/net/smsc-lan87xx.txt @@ -0,0 +1,19 @@ +SMSC LAN87xx Ethernet PHY + +Some boards require special tuning values. Configure them +through an Ethernet OF device node. + +Optional properties: + +- disable-energy-detect: + If set, do not enable energy detect mode for the SMSC phy. + default: enable energy detect mode + +Examples: + + /* Attach to an Ethernet device with autodetected PHY */ + &cpsw_emac0 { + phy_id = <&davinci_mdio>, <0>; + phy-mode = "mii"; + disable-energy-detect; + }; diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c index 70b0895..f90fbf3 100644 --- a/drivers/net/phy/smsc.c +++ b/drivers/net/phy/smsc.c @@ -43,16 +43,30 @@ static int smsc_phy_ack_interrupt(struct phy_device *phydev) static int smsc_phy_config_init(struct phy_device *phydev) { +#ifdef CONFIG_OF + int len; + struct device *dev = &phydev->dev; + struct device_node *of_node = dev->of_node; +#endif int rc = phy_read(phydev, MII_LAN83C185_CTRL_STATUS); + int enable_energy = 1; if (rc < 0) return rc; - /* Enable energy detect mode for this SMSC Transceivers */ - rc = phy_write(phydev, MII_LAN83C185_CTRL_STATUS, - rc | MII_LAN83C185_EDPWRDOWN); - if (rc < 0) - return rc; +#ifdef CONFIG_OF + if (!of_node && dev->parent->of_node) + of_node = dev->parent->of_node; + if (of_find_property(of_node, "disable-energy-detect", &len)) + enable_energy = 0; +#endif + if (enable_energy) { + /* Enable energy detect mode for this SMSC Transceivers */ + rc = phy_write(phydev, MII_LAN83C185_CTRL_STATUS, + rc | MII_LAN83C185_EDPWRDOWN); + if (rc < 0) + return rc; + } return smsc_phy_ack_interrupt(phydev); } -- 2.1.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] net: phy: smsc: disable energy detect mode 2015-10-13 5:13 [PATCH] net: phy: smsc: disable energy detect mode Heiko Schocher @ 2015-10-13 19:26 ` Florian Fainelli 2015-10-14 4:17 ` Heiko Schocher 0 siblings, 1 reply; 6+ messages in thread From: Florian Fainelli @ 2015-10-13 19:26 UTC (permalink / raw) To: Heiko Schocher, linux-kernel; +Cc: devicetree, netdev, Georg.Soffel On 12/10/15 22:13, Heiko Schocher wrote: > On some boards the energy enable detect mode leads in > trouble with some switches, so make the enabling of > this mode configurable through DT. > > Signed-off-by: Heiko Schocher <hs@denx.de> > --- > > .../devicetree/bindings/net/smsc-lan87xx.txt | 19 +++++++++++++++++ > drivers/net/phy/smsc.c | 24 +++++++++++++++++----- > 2 files changed, 38 insertions(+), 5 deletions(-) > create mode 100644 Documentation/devicetree/bindings/net/smsc-lan87xx.txt > > diff --git a/Documentation/devicetree/bindings/net/smsc-lan87xx.txt b/Documentation/devicetree/bindings/net/smsc-lan87xx.txt > new file mode 100644 > index 0000000..39aa1dc > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/smsc-lan87xx.txt > @@ -0,0 +1,19 @@ > +SMSC LAN87xx Ethernet PHY > + > +Some boards require special tuning values. Configure them > +through an Ethernet OF device node. > + > +Optional properties: > + > +- disable-energy-detect: > + If set, do not enable energy detect mode for the SMSC phy. > + default: enable energy detect mode Although energy detection is something that is implemented by many PHYs, I am not sure a generic property is suitable here, I would prefix that with the SMSC vendor prefix here to make it clear this only applies to this PHY. Would not you want to make it a reverse property here though, something like this: smsc,energy-detect: boolean, when present indicates the PHY reliably supports energy detection > + > +Examples: > + > + /* Attach to an Ethernet device with autodetected PHY */ > + &cpsw_emac0 { > + phy_id = <&davinci_mdio>, <0>; > + phy-mode = "mii"; > + disable-energy-detect; > + }; > diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c > index 70b0895..f90fbf3 100644 > --- a/drivers/net/phy/smsc.c > +++ b/drivers/net/phy/smsc.c > @@ -43,16 +43,30 @@ static int smsc_phy_ack_interrupt(struct phy_device *phydev) > > static int smsc_phy_config_init(struct phy_device *phydev) > { > +#ifdef CONFIG_OF > + int len; > + struct device *dev = &phydev->dev; > + struct device_node *of_node = dev->of_node; That does not need to be ifdefd out, at best annontate with __maybe_unused? > +#endif > int rc = phy_read(phydev, MII_LAN83C185_CTRL_STATUS); > + int enable_energy = 1; > > if (rc < 0) > return rc; > > - /* Enable energy detect mode for this SMSC Transceivers */ > - rc = phy_write(phydev, MII_LAN83C185_CTRL_STATUS, > - rc | MII_LAN83C185_EDPWRDOWN); > - if (rc < 0) > - return rc; > +#ifdef CONFIG_OF > + if (!of_node && dev->parent->of_node) > + of_node = dev->parent->of_node; That looks strange, why would the property be placed at the parent level when this is a PHY device tree node property? > + if (of_find_property(of_node, "disable-energy-detect", &len)) > + enable_energy = 0; > +#endif > + if (enable_energy) { > + /* Enable energy detect mode for this SMSC Transceivers */ > + rc = phy_write(phydev, MII_LAN83C185_CTRL_STATUS, > + rc | MII_LAN83C185_EDPWRDOWN); > + if (rc < 0) > + return rc; > + } > > return smsc_phy_ack_interrupt(phydev); > } > -- Florian ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net: phy: smsc: disable energy detect mode 2015-10-13 19:26 ` Florian Fainelli @ 2015-10-14 4:17 ` Heiko Schocher 2015-10-16 5:37 ` Heiko Schocher 2015-10-16 16:27 ` Florian Fainelli 0 siblings, 2 replies; 6+ messages in thread From: Heiko Schocher @ 2015-10-14 4:17 UTC (permalink / raw) To: Florian Fainelli; +Cc: linux-kernel, devicetree, netdev, Georg.Soffel Hello Florian, Am 13.10.2015 um 21:26 schrieb Florian Fainelli: > On 12/10/15 22:13, Heiko Schocher wrote: >> On some boards the energy enable detect mode leads in >> trouble with some switches, so make the enabling of >> this mode configurable through DT. >> >> Signed-off-by: Heiko Schocher <hs@denx.de> >> --- >> >> .../devicetree/bindings/net/smsc-lan87xx.txt | 19 +++++++++++++++++ >> drivers/net/phy/smsc.c | 24 +++++++++++++++++----- >> 2 files changed, 38 insertions(+), 5 deletions(-) >> create mode 100644 Documentation/devicetree/bindings/net/smsc-lan87xx.txt >> >> diff --git a/Documentation/devicetree/bindings/net/smsc-lan87xx.txt b/Documentation/devicetree/bindings/net/smsc-lan87xx.txt >> new file mode 100644 >> index 0000000..39aa1dc >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/net/smsc-lan87xx.txt >> @@ -0,0 +1,19 @@ >> +SMSC LAN87xx Ethernet PHY >> + >> +Some boards require special tuning values. Configure them >> +through an Ethernet OF device node. >> + >> +Optional properties: >> + >> +- disable-energy-detect: >> + If set, do not enable energy detect mode for the SMSC phy. >> + default: enable energy detect mode > > Although energy detection is something that is implemented by many PHYs, > I am not sure a generic property is suitable here, I would prefix that > with the SMSC vendor prefix here to make it clear this only applies to > this PHY. Hmm... but all PHYs should be able to enable, disable it in some way, or? > Would not you want to make it a reverse property here though, something > like this: > > smsc,energy-detect: boolean, when present indicates the PHY reliably > supports energy detection Yes, that was also my first thought, but currently, on this PHYs energy detect mode is on ... and if I introduce such a property, it will disable it for all existing boards, because property is missing ... so, maybe I break boards ... >> + >> +Examples: >> + >> + /* Attach to an Ethernet device with autodetected PHY */ >> + &cpsw_emac0 { >> + phy_id = <&davinci_mdio>, <0>; >> + phy-mode = "mii"; >> + disable-energy-detect; >> + }; >> diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c >> index 70b0895..f90fbf3 100644 >> --- a/drivers/net/phy/smsc.c >> +++ b/drivers/net/phy/smsc.c >> @@ -43,16 +43,30 @@ static int smsc_phy_ack_interrupt(struct phy_device *phydev) >> >> static int smsc_phy_config_init(struct phy_device *phydev) >> { >> +#ifdef CONFIG_OF >> + int len; >> + struct device *dev = &phydev->dev; >> + struct device_node *of_node = dev->of_node; > > That does not need to be ifdefd out, at best annontate with __maybe_unused? Yes, I try it. >> +#endif >> int rc = phy_read(phydev, MII_LAN83C185_CTRL_STATUS); >> + int enable_energy = 1; >> >> if (rc < 0) >> return rc; >> >> - /* Enable energy detect mode for this SMSC Transceivers */ >> - rc = phy_write(phydev, MII_LAN83C185_CTRL_STATUS, >> - rc | MII_LAN83C185_EDPWRDOWN); >> - if (rc < 0) >> - return rc; >> +#ifdef CONFIG_OF >> + if (!of_node && dev->parent->of_node) >> + of_node = dev->parent->of_node; > > That looks strange, why would the property be placed at the parent level > when this is a PHY device tree node property? Hmm.. I recheck this. >> + if (of_find_property(of_node, "disable-energy-detect", &len)) >> + enable_energy = 0; >> +#endif >> + if (enable_energy) { >> + /* Enable energy detect mode for this SMSC Transceivers */ >> + rc = phy_write(phydev, MII_LAN83C185_CTRL_STATUS, >> + rc | MII_LAN83C185_EDPWRDOWN); >> + if (rc < 0) >> + return rc; >> + } >> >> return smsc_phy_ack_interrupt(phydev); >> } >> Thanks for your review. bye, Heiko -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net: phy: smsc: disable energy detect mode 2015-10-14 4:17 ` Heiko Schocher @ 2015-10-16 5:37 ` Heiko Schocher 2015-10-16 16:27 ` Florian Fainelli 1 sibling, 0 replies; 6+ messages in thread From: Heiko Schocher @ 2015-10-16 5:37 UTC (permalink / raw) To: Florian Fainelli; +Cc: linux-kernel, devicetree, netdev, Georg.Soffel Hello Florian, Am 14.10.2015 um 06:17 schrieb Heiko Schocher: > Hello Florian, > > Am 13.10.2015 um 21:26 schrieb Florian Fainelli: >> On 12/10/15 22:13, Heiko Schocher wrote: >>> On some boards the energy enable detect mode leads in >>> trouble with some switches, so make the enabling of >>> this mode configurable through DT. >>> >>> Signed-off-by: Heiko Schocher <hs@denx.de> >>> --- >>> >>> .../devicetree/bindings/net/smsc-lan87xx.txt | 19 +++++++++++++++++ >>> drivers/net/phy/smsc.c | 24 +++++++++++++++++----- >>> 2 files changed, 38 insertions(+), 5 deletions(-) >>> create mode 100644 Documentation/devicetree/bindings/net/smsc-lan87xx.txt >>> >>> diff --git a/Documentation/devicetree/bindings/net/smsc-lan87xx.txt >>> b/Documentation/devicetree/bindings/net/smsc-lan87xx.txt >>> new file mode 100644 >>> index 0000000..39aa1dc >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/net/smsc-lan87xx.txt >>> @@ -0,0 +1,19 @@ >>> +SMSC LAN87xx Ethernet PHY >>> + >>> +Some boards require special tuning values. Configure them >>> +through an Ethernet OF device node. >>> + >>> +Optional properties: >>> + >>> +- disable-energy-detect: >>> + If set, do not enable energy detect mode for the SMSC phy. >>> + default: enable energy detect mode >> >> Although energy detection is something that is implemented by many PHYs, >> I am not sure a generic property is suitable here, I would prefix that >> with the SMSC vendor prefix here to make it clear this only applies to >> this PHY. > > Hmm... but all PHYs should be able to enable, disable it in some way, or? ping? >> Would not you want to make it a reverse property here though, something >> like this: >> >> smsc,energy-detect: boolean, when present indicates the PHY reliably >> supports energy detection > > Yes, that was also my first thought, but currently, on this PHYs > energy detect mode is on ... and if I introduce such a property, > it will disable it for all existing boards, because property is > missing ... so, maybe I break boards ... I have a v2 prepared, but what to do here? >>> + >>> +Examples: >>> + >>> + /* Attach to an Ethernet device with autodetected PHY */ >>> + &cpsw_emac0 { >>> + phy_id = <&davinci_mdio>, <0>; >>> + phy-mode = "mii"; >>> + disable-energy-detect; >>> + }; >>> diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c >>> index 70b0895..f90fbf3 100644 >>> --- a/drivers/net/phy/smsc.c >>> +++ b/drivers/net/phy/smsc.c >>> @@ -43,16 +43,30 @@ static int smsc_phy_ack_interrupt(struct phy_device *phydev) >>> >>> static int smsc_phy_config_init(struct phy_device *phydev) >>> { >>> +#ifdef CONFIG_OF >>> + int len; >>> + struct device *dev = &phydev->dev; >>> + struct device_node *of_node = dev->of_node; >> >> That does not need to be ifdefd out, at best annontate with __maybe_unused? > > Yes, I try it. removed. >>> +#endif >>> int rc = phy_read(phydev, MII_LAN83C185_CTRL_STATUS); >>> + int enable_energy = 1; >>> >>> if (rc < 0) >>> return rc; >>> >>> - /* Enable energy detect mode for this SMSC Transceivers */ >>> - rc = phy_write(phydev, MII_LAN83C185_CTRL_STATUS, >>> - rc | MII_LAN83C185_EDPWRDOWN); >>> - if (rc < 0) >>> - return rc; >>> +#ifdef CONFIG_OF >>> + if (!of_node && dev->parent->of_node) >>> + of_node = dev->parent->of_node; >> >> That looks strange, why would the property be placed at the parent level >> when this is a PHY device tree node property? > > Hmm.. I recheck this. Hmm.. the drivers/net/ethernet/ti/cpsw.c has no phy-handle support ... I added this, so my v2 is now a patchset of 2 patches. bye, Heiko -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net: phy: smsc: disable energy detect mode 2015-10-14 4:17 ` Heiko Schocher 2015-10-16 5:37 ` Heiko Schocher @ 2015-10-16 16:27 ` Florian Fainelli 2015-10-17 3:42 ` Heiko Schocher 1 sibling, 1 reply; 6+ messages in thread From: Florian Fainelli @ 2015-10-16 16:27 UTC (permalink / raw) To: Heiko Schocher Cc: linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, netdev, Georg.Soffel 2015-10-13 21:17 GMT-07:00 Heiko Schocher <hs@denx.de>: > Hello Florian, > > > Am 13.10.2015 um 21:26 schrieb Florian Fainelli: >> >> On 12/10/15 22:13, Heiko Schocher wrote: >>> >>> On some boards the energy enable detect mode leads in >>> trouble with some switches, so make the enabling of >>> this mode configurable through DT. >>> >>> Signed-off-by: Heiko Schocher <hs@denx.de> >>> --- >>> >>> .../devicetree/bindings/net/smsc-lan87xx.txt | 19 >>> +++++++++++++++++ >>> drivers/net/phy/smsc.c | 24 >>> +++++++++++++++++----- >>> 2 files changed, 38 insertions(+), 5 deletions(-) >>> create mode 100644 >>> Documentation/devicetree/bindings/net/smsc-lan87xx.txt >>> >>> diff --git a/Documentation/devicetree/bindings/net/smsc-lan87xx.txt >>> b/Documentation/devicetree/bindings/net/smsc-lan87xx.txt >>> new file mode 100644 >>> index 0000000..39aa1dc >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/net/smsc-lan87xx.txt >>> @@ -0,0 +1,19 @@ >>> +SMSC LAN87xx Ethernet PHY >>> + >>> +Some boards require special tuning values. Configure them >>> +through an Ethernet OF device node. >>> + >>> +Optional properties: >>> + >>> +- disable-energy-detect: >>> + If set, do not enable energy detect mode for the SMSC phy. >>> + default: enable energy detect mode >> >> >> Although energy detection is something that is implemented by many PHYs, >> I am not sure a generic property is suitable here, I would prefix that >> with the SMSC vendor prefix here to make it clear this only applies to >> this PHY. > > > Hmm... but all PHYs should be able to enable, disable it in some way, or? It may not always be controlled directly at the PHY level, sometimes this is something that needs cooperation with the Ethernet MAC as well in case of integrated designs. > >> Would not you want to make it a reverse property here though, something >> like this: >> >> smsc,energy-detect: boolean, when present indicates the PHY reliably >> supports energy detection > > > Yes, that was also my first thought, but currently, on this PHYs > energy detect mode is on ... and if I introduce such a property, > it will disable it for all existing boards, because property is > missing ... so, maybe I break boards ... Fair enough, how about smsc,disabled-energy-detect or something like that then? -- Florian ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net: phy: smsc: disable energy detect mode 2015-10-16 16:27 ` Florian Fainelli @ 2015-10-17 3:42 ` Heiko Schocher 0 siblings, 0 replies; 6+ messages in thread From: Heiko Schocher @ 2015-10-17 3:42 UTC (permalink / raw) To: Florian Fainelli Cc: linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, netdev, Georg.Soffel Hello Florian, Am 16.10.2015 um 18:27 schrieb Florian Fainelli: > 2015-10-13 21:17 GMT-07:00 Heiko Schocher <hs@denx.de>: >> Hello Florian, >> >> >> Am 13.10.2015 um 21:26 schrieb Florian Fainelli: >>> >>> On 12/10/15 22:13, Heiko Schocher wrote: >>>> >>>> On some boards the energy enable detect mode leads in >>>> trouble with some switches, so make the enabling of >>>> this mode configurable through DT. >>>> >>>> Signed-off-by: Heiko Schocher <hs@denx.de> >>>> --- >>>> >>>> .../devicetree/bindings/net/smsc-lan87xx.txt | 19 >>>> +++++++++++++++++ >>>> drivers/net/phy/smsc.c | 24 >>>> +++++++++++++++++----- >>>> 2 files changed, 38 insertions(+), 5 deletions(-) >>>> create mode 100644 >>>> Documentation/devicetree/bindings/net/smsc-lan87xx.txt >>>> >>>> diff --git a/Documentation/devicetree/bindings/net/smsc-lan87xx.txt >>>> b/Documentation/devicetree/bindings/net/smsc-lan87xx.txt >>>> new file mode 100644 >>>> index 0000000..39aa1dc >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/net/smsc-lan87xx.txt >>>> @@ -0,0 +1,19 @@ >>>> +SMSC LAN87xx Ethernet PHY >>>> + >>>> +Some boards require special tuning values. Configure them >>>> +through an Ethernet OF device node. >>>> + >>>> +Optional properties: >>>> + >>>> +- disable-energy-detect: >>>> + If set, do not enable energy detect mode for the SMSC phy. >>>> + default: enable energy detect mode >>> >>> >>> Although energy detection is something that is implemented by many PHYs, >>> I am not sure a generic property is suitable here, I would prefix that >>> with the SMSC vendor prefix here to make it clear this only applies to >>> this PHY. >> >> >> Hmm... but all PHYs should be able to enable, disable it in some way, or? > > It may not always be controlled directly at the PHY level, sometimes > this is something that needs cooperation with the Ethernet MAC as well > in case of integrated designs. Ah, ok! >>> Would not you want to make it a reverse property here though, something >>> like this: >>> >>> smsc,energy-detect: boolean, when present indicates the PHY reliably >>> supports energy detection >> >> >> Yes, that was also my first thought, but currently, on this PHYs >> energy detect mode is on ... and if I introduce such a property, >> it will disable it for all existing boards, because property is >> missing ... so, maybe I break boards ... > > Fair enough, how about smsc,disabled-energy-detect or something like that then? Yes, changed it to "smsc,disable-energy-detect" bye, Heiko -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-10-17 3:42 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-10-13 5:13 [PATCH] net: phy: smsc: disable energy detect mode Heiko Schocher 2015-10-13 19:26 ` Florian Fainelli 2015-10-14 4:17 ` Heiko Schocher 2015-10-16 5:37 ` Heiko Schocher 2015-10-16 16:27 ` Florian Fainelli 2015-10-17 3:42 ` Heiko Schocher
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox