mbox series

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

Message ID 20200908112520.3439-1-m.felsch@pengutronix.de
Headers show
Series SMSC: Cleanups and clock setup | expand

Message

Marco Felsch Sept. 8, 2020, 11:25 a.m. UTC
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(-)

Comments

Florian Fainelli Sept. 9, 2020, 1:58 a.m. UTC | #1
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 Fainelli Sept. 9, 2020, 1:59 a.m. UTC | #2
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 Fainelli Sept. 9, 2020, 2:02 a.m. UTC | #3
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>
Marco Felsch Sept. 9, 2020, 8:43 a.m. UTC | #4
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
> 
>