[PATCH v2 0/5] SMSC: Cleanups and clock setup

[PATCH v2 0/5] SMSC: Cleanups and clock setup

[Marco Felsch]

Hi,

this small series cleans the smsc-phy code a bit and adds the support to
specify the phy clock source. Adding the phy clock source support is
also the main purpose of this series.

Each file has its own changelog.

Regards,
  Marco

Marco Felsch (5):
  net: phy: smsc: skip ENERGYON interrupt if disabled
  net: phy: smsc: simplify config_init callback
  dt-bindings: net: phy: smsc: document reference clock
  net: phy: smsc: LAN8710/20: add phy refclk in support
  net: phy: smsc: LAN8710/20: remove PHY_RST_AFTER_CLK_EN flag

 .../devicetree/bindings/net/smsc-lan87xx.txt  |  4 ++
 drivers/net/phy/smsc.c                        | 59 +++++++++++++++----
 2 files changed, 50 insertions(+), 13 deletions(-)

-- 
2.20.1

[PATCH v2 1/5] net: phy: smsc: skip ENERGYON interrupt if disabled

[Marco Felsch]

Don't enable the interrupt if the platform disable the energy detection
by "smsc,disable-energy-detect".

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
v2:
- Add Andrew's tag

 drivers/net/phy/smsc.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
index 74568ae16125..fa539a867de6 100644
--- a/drivers/net/phy/smsc.c
+++ b/drivers/net/phy/smsc.c
@@ -37,10 +37,17 @@ struct smsc_phy_priv {
 
 static int smsc_phy_config_intr(struct phy_device *phydev)
 {
-	int rc = phy_write (phydev, MII_LAN83C185_IM,
-			((PHY_INTERRUPT_ENABLED == phydev->interrupts)
-			? MII_LAN83C185_ISF_INT_PHYLIB_EVENTS
-			: 0));
+	struct smsc_phy_priv *priv = phydev->priv;
+	u16 intmask = 0;
+	int rc;
+
+	if (phydev->interrupts) {
+		intmask = MII_LAN83C185_ISF_INT4 | MII_LAN83C185_ISF_INT6;
+		if (priv->energy_enable)
+			intmask |= MII_LAN83C185_ISF_INT7;
+	}
+
+	rc = phy_write(phydev, MII_LAN83C185_IM, intmask);
 
 	return rc < 0 ? rc : 0;
 }
-- 
2.20.1

[PATCH v2 2/5] net: phy: smsc: simplify config_init callback

[Marco Felsch]

Exit the driver specific config_init hook early if energy detection is
disabled. We can do this because we don't need to clear the interrupt
status here. Clearing the status should be removed anyway since this is
handled by the phy_enable_interrupts().

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
v2:
- no change

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

diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
index fa539a867de6..79574fcbd880 100644
--- a/drivers/net/phy/smsc.c
+++ b/drivers/net/phy/smsc.c
@@ -62,19 +62,21 @@ static int smsc_phy_ack_interrupt(struct phy_device *phydev)
 static int smsc_phy_config_init(struct phy_device *phydev)
 {
 	struct smsc_phy_priv *priv = phydev->priv;
+	int rc;
+
+	if (!priv->energy_enable)
+		return 0;
 
-	int rc = phy_read(phydev, MII_LAN83C185_CTRL_STATUS);
+	rc = phy_read(phydev, MII_LAN83C185_CTRL_STATUS);
 
 	if (rc < 0)
 		return rc;
 
-	if (priv->energy_enable) {
-		/* 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;
-	}
+	/* 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.20.1

[PATCH v2 3/5] dt-bindings: net: phy: smsc: document reference clock

[Marco Felsch]

Add support to specify the reference clock for the phy.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
v2:
- no change

 Documentation/devicetree/bindings/net/smsc-lan87xx.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/smsc-lan87xx.txt b/Documentation/devicetree/bindings/net/smsc-lan87xx.txt
index 8b7c719b0bb9..a8d0dc9a8c0e 100644
--- a/Documentation/devicetree/bindings/net/smsc-lan87xx.txt
+++ b/Documentation/devicetree/bindings/net/smsc-lan87xx.txt
@@ -5,6 +5,10 @@ through an Ethernet OF device node.
 
 Optional properties:
 
+- clocks:
+  The clock used as phy reference clock and is connected to phy
+  pin XTAL1/CLKIN.
+
 - smsc,disable-energy-detect:
   If set, do not enable energy detect mode for the SMSC phy.
   default: enable energy detect mode
-- 
2.20.1

[PATCH v2 4/5] net: phy: smsc: LAN8710/20: add phy refclk in support

[Marco Felsch]

Add support to specify the clock provider for the phy refclk and don't
rely on 'magic' host clock setup. Commit [1] tried to address this by
introducing a flag and fixing the corresponding host. But this commit
breaks the IRQ support since the irq setup during .config_intr() is
thrown away because the reset comes from the side without respecting the
current phy-state within the phy-state-machine. Furthermore the commit
fixed the problem only for FEC based hosts other hosts acting like the
FEC are not covered.

This commit goes the other way around to address the bug fixed by [1].
Instead of resetting the device from the side every time the refclk gets
(re-)enabled it requests and enables the clock till the device gets
removed. The phy is still rest but now within the phylib and  with
respect to the phy-state-machine.

[1] commit 7f64e5b18ebb ("net: phy: smsc: LAN8710/20: add
    PHY_RST_AFTER_CLK_EN flag")

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
v2:
- use non-devres clk_* functions and instead use the remove() function
- propagate errors upstream if the optional clk can't be retrieved.
- make use if dev_err_probe()
- adapt commit subject to cover that only the LAN8710/20 devices are
  changed

 drivers/net/phy/smsc.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
index 79574fcbd880..7a1a7e13db85 100644
--- a/drivers/net/phy/smsc.c
+++ b/drivers/net/phy/smsc.c
@@ -12,6 +12,7 @@
  *
  */
 
+#include <linux/clk.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/mii.h>
@@ -33,6 +34,7 @@ static struct smsc_hw_stat smsc_hw_stats[] = {
 
 struct smsc_phy_priv {
 	bool energy_enable;
+	struct clk *refclk;
 };
 
 static int smsc_phy_config_intr(struct phy_device *phydev)
@@ -194,11 +196,20 @@ static void smsc_get_stats(struct phy_device *phydev,
 		data[i] = smsc_get_stat(phydev, i);
 }
 
+static void smsc_phy_remove(struct phy_device *phydev)
+{
+	struct smsc_phy_priv *priv = phydev->priv;
+
+	clk_disable_unprepare(priv->refclk);
+	clk_put(priv->refclk);
+}
+
 static int smsc_phy_probe(struct phy_device *phydev)
 {
 	struct device *dev = &phydev->mdio.dev;
 	struct device_node *of_node = dev->of_node;
 	struct smsc_phy_priv *priv;
+	int ret;
 
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
@@ -211,6 +222,19 @@ static int smsc_phy_probe(struct phy_device *phydev)
 
 	phydev->priv = priv;
 
+	/* Make clk optional to keep DTB backward compatibility. */
+	priv->refclk = clk_get_optional(dev, NULL);
+	if (IS_ERR(priv->refclk))
+		dev_err_probe(dev, PTR_ERR(priv->refclk), "Failed to request clock\n");
+
+	ret = clk_prepare_enable(priv->refclk);
+	if (ret)
+		return ret;
+
+	ret = clk_set_rate(priv->refclk, 50 * 1000 * 1000);
+	if (ret)
+		return ret;
+
 	return 0;
 }
 
@@ -310,6 +334,7 @@ static struct phy_driver smsc_phy_driver[] = {
 	.flags		= PHY_RST_AFTER_CLK_EN,
 
 	.probe		= smsc_phy_probe,
+	.remove		= smsc_phy_remove,
 
 	/* basic functions */
 	.read_status	= lan87xx_read_status,
-- 
2.20.1

[PATCH v2 5/5] net: phy: smsc: LAN8710/20: remove PHY_RST_AFTER_CLK_EN flag

[Marco Felsch]

Don't reset the phy without respect to the phy-state-machine because
this breaks the phy IRQ mode. We can archive the same behaviour if the
refclk in is specified.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
v2:
- adapt the subject to align with previous patch

 drivers/net/phy/smsc.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
index 7a1a7e13db85..0e618d2b6aba 100644
--- a/drivers/net/phy/smsc.c
+++ b/drivers/net/phy/smsc.c
@@ -331,7 +331,6 @@ static struct phy_driver smsc_phy_driver[] = {
 	.name		= "SMSC LAN8710/LAN8720",
 
 	/* PHY_BASIC_FEATURES */
-	.flags		= PHY_RST_AFTER_CLK_EN,
 
 	.probe		= smsc_phy_probe,
 	.remove		= smsc_phy_remove,
-- 
2.20.1

Re: [PATCH v2 1/5] net: phy: smsc: skip ENERGYON interrupt if disabled

[Florian Fainelli]

On 9/8/2020 4:25 AM, Marco Felsch wrote:
> Don't enable the interrupt if the platform disable the energy detection
> by "smsc,disable-energy-detect".
> 
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> ---
> v2:
> - Add Andrew's tag
> 
>   drivers/net/phy/smsc.c | 15 +++++++++++----
>   1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
> index 74568ae16125..fa539a867de6 100644
> --- a/drivers/net/phy/smsc.c
> +++ b/drivers/net/phy/smsc.c
> @@ -37,10 +37,17 @@ struct smsc_phy_priv {
>   
>   static int smsc_phy_config_intr(struct phy_device *phydev)
>   {
> -	int rc = phy_write (phydev, MII_LAN83C185_IM,
> -			((PHY_INTERRUPT_ENABLED == phydev->interrupts)
> -			? MII_LAN83C185_ISF_INT_PHYLIB_EVENTS
> -			: 0));
> +	struct smsc_phy_priv *priv = phydev->priv;
> +	u16 intmask = 0;
> +	int rc;
> +
> +	if (phydev->interrupts) {

Not that it changes the code functionally, but it would be nice to 
preserve the phydev->interrupts == PHY_INTERRUPT_ENABLED.

> +		intmask = MII_LAN83C185_ISF_INT4 | MII_LAN83C185_ISF_INT6;
> +		if (priv->energy_enable)
> +			intmask |= MII_LAN83C185_ISF_INT7;
> +	}

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [PATCH v2 2/5] net: phy: smsc: simplify config_init callback

[Florian Fainelli]

On 9/8/2020 4:25 AM, Marco Felsch wrote:
> Exit the driver specific config_init hook early if energy detection is
> disabled. We can do this because we don't need to clear the interrupt
> status here. Clearing the status should be removed anyway since this is
> handled by the phy_enable_interrupts().
> 
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [PATCH v2 3/5] dt-bindings: net: phy: smsc: document reference clock

[Florian Fainelli]

On 9/8/2020 4:25 AM, Marco Felsch wrote:
> Add support to specify the reference clock for the phy.
> 
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [PATCH v2 4/5] net: phy: smsc: LAN8710/20: add phy refclk in support

[Florian Fainelli]

On 9/8/2020 4:25 AM, Marco Felsch wrote:
> Add support to specify the clock provider for the phy refclk and don't
> rely on 'magic' host clock setup. Commit [1] tried to address this by
> introducing a flag and fixing the corresponding host. But this commit
> breaks the IRQ support since the irq setup during .config_intr() is
> thrown away because the reset comes from the side without respecting the
> current phy-state within the phy-state-machine. Furthermore the commit
> fixed the problem only for FEC based hosts other hosts acting like the
> FEC are not covered.
> 
> This commit goes the other way around to address the bug fixed by [1].
> Instead of resetting the device from the side every time the refclk gets
> (re-)enabled it requests and enables the clock till the device gets
> removed. The phy is still rest but now within the phylib and  with
> respect to the phy-state-machine.

s/rest/reset/
s/phy/PHY/
s/phy-state-machine/PHY library state machine/

(this applies to patch #5 as well)

With those things corrected:

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [PATCH v2 1/5] net: phy: smsc: skip ENERGYON interrupt if disabled

[Marco Felsch]

On 20-09-08 18:58, Florian Fainelli wrote:
> 
> 
> On 9/8/2020 4:25 AM, Marco Felsch wrote:
> > Don't enable the interrupt if the platform disable the energy detection
> > by "smsc,disable-energy-detect".
> > 
> > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> > ---
> > v2:
> > - Add Andrew's tag
> > 
> >   drivers/net/phy/smsc.c | 15 +++++++++++----
> >   1 file changed, 11 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
> > index 74568ae16125..fa539a867de6 100644
> > --- a/drivers/net/phy/smsc.c
> > +++ b/drivers/net/phy/smsc.c
> > @@ -37,10 +37,17 @@ struct smsc_phy_priv {
> >   static int smsc_phy_config_intr(struct phy_device *phydev)
> >   {
> > -	int rc = phy_write (phydev, MII_LAN83C185_IM,
> > -			((PHY_INTERRUPT_ENABLED == phydev->interrupts)
> > -			? MII_LAN83C185_ISF_INT_PHYLIB_EVENTS
> > -			: 0));
> > +	struct smsc_phy_priv *priv = phydev->priv;
> > +	u16 intmask = 0;
> > +	int rc;
> > +
> > +	if (phydev->interrupts) {
> 
> Not that it changes the code functionally, but it would be nice to preserve
> the phydev->interrupts == PHY_INTERRUPT_ENABLED.

Okay, I will apply this and add your reviewed-by tag.

Thanks,
  Marco

> 
> > +		intmask = MII_LAN83C185_ISF_INT4 | MII_LAN83C185_ISF_INT6;
> > +		if (priv->energy_enable)
> > +			intmask |= MII_LAN83C185_ISF_INT7;
> > +	}
> 
> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
> -- 
> Florian
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |