Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

[PATCH] net: phy: smsc: disable energy detect mode

99 views
Skip to first unread message

Heiko Schocher

unread,
Oct 13, 2015, 1:20:06 AM10/13/15
to
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 <h...@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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Florian Fainelli

unread,
Oct 13, 2015, 3:30:07 PM10/13/15
to
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

Heiko Schocher

unread,
Oct 14, 2015, 12:20:05 AM10/14/15
to
Hello Florian,
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

Heiko Schocher

unread,
Oct 16, 2015, 1:40:05 AM10/16/15
to
Hello Florian,
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.

Florian Fainelli

unread,
Oct 16, 2015, 12:30:08 PM10/16/15
to
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

Heiko Schocher

unread,
Oct 16, 2015, 11:50:08 PM10/16/15
to
Hello Florian,
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

Heiko Schocher

unread,
Oct 17, 2015, 12:10:06 AM10/17/15
to
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 <h...@denx.de>
---

Changes in v2:
- add comments from Florian Fainelli
- I did not change disable property name into enable
because I fear to break existing behaviour
- add smsc vendor prefix
- remove CONFIG_OF and use __maybe_unused
- introduce "phy-handle" ability into ti,cpsw
driver, so I can remove bogus:
if (!of_node && dev->parent->of_node)
of_node = dev->parent->of_node;
construct. Therefore new patch for the ti,cpsw
driver is necessary.

.../devicetree/bindings/net/smsc-lan87xx.txt | 24 ++++++++++++++++++++++
drivers/net/phy/smsc.c | 19 ++++++++++++-----
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..974edd5
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/smsc-lan87xx.txt
@@ -0,0 +1,24 @@
+SMSC LAN87xx Ethernet PHY
+
+Some boards require special tuning values. Configure them
+through an Ethernet OF device node.
+
+Optional properties:
+
+- smsc,disable-energy-detect:
+ If set, do not enable energy detect mode for the SMSC phy.
+ default: enable energy detect mode
+
+Examples:
+smsc phy with disabled energy detect mode on an am335x based board.
+&davinci_mdio {
+ pinctrl-names = "default", "sleep";
+ pinctrl-0 = <&davinci_mdio_default>;
+ pinctrl-1 = <&davinci_mdio_sleep>;
+ status = "okay";
+
+ ethernetphy0: ethernet-phy@0 {
+ reg = <0>;
+ smsc,disable-energy-detect;
+ };
+};
diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
index 70b0895..dc2da87 100644
--- a/drivers/net/phy/smsc.c
+++ b/drivers/net/phy/smsc.c
@@ -43,16 +43,25 @@ static int smsc_phy_ack_interrupt(struct phy_device *phydev)

static int smsc_phy_config_init(struct phy_device *phydev)
{
+ int __maybe_unused len;
+ struct device *dev __maybe_unused = &phydev->dev;
+ struct device_node *of_node __maybe_unused = dev->of_node;
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;
+ if (of_find_property(of_node, "smsc,disable-energy-detect", &len))
+ enable_energy = 0;
+
+ 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
0 new messages