[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 12 Dec 2018 14:25:09 +0000
From: Joakim Tjernlund <Joakim.Tjernlund@...inera.com>
To: "claudiu.manoil@....com" <claudiu.manoil@....com>,
"andrew@...n.ch" <andrew@...n.ch>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"f.fainelli@...il.com" <f.fainelli@...il.com>
Subject: Re: [PATCHv2] gianfar: Add gfar_change_carrier() for Fixed PHYs
On Wed, 2018-12-12 at 13:10 +0000, Claudiu Manoil wrote:
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.
>
>
> > -----Original Message-----
> > From: Andrew Lunn <andrew@...n.ch>
> > Sent: Wednesday, December 12, 2018 2:43 PM
> > To: jocke@...inera.com <joakim.tjernlund@...inera.com>
> > Cc: netdev @ vger . kernel . org <netdev@...r.kernel.org>; Claudiu Manoil
> > <claudiu.manoil@....com>; Florian Fainelli <f.fainelli@...il.com>
> > Subject: Re: [PATCHv2] gianfar: Add gfar_change_carrier() for Fixed PHYs
> >
> > On Wed, Dec 12, 2018 at 01:33:08PM +0100, Joakim Tjernlund wrote:
> > > This allows to control carrier from /sys/class/net/ethX/carrier for
> > > Fixed PHYs.
> > >
> > > Signed-off-by: Joakim Tjernlund <joakim.tjernlund@...inera.com>
> > > ---
> > > v2 - Only allow carrier changes for Fixed PHYs
> > >
> > > Florian: I have reimpl. this as I think you meant by registering
> > > a Fixed PHY callback.
> > > Andrew: Are happy with this as well?
> >
> > Hi Joakim
> >
> > The basic idea is O.K.
> >
> > > If this is OK I will sent the other 2 drivers.
> >
> > Rather than replicating the code three times, how about putting all
> > the code in the fixed phy driver, exporting a fixed_phy_change_carrier
> > function which can be assigned to the .ndo.
> >
>
> I agree with Andrew on this. This code is not device (eTSEC) specific, it is generic
> and only uses the gianfar driver to get a reference to its net_device.
> Thanks,
> Claudiu
fast check, would you be happy with this in fixed PHY:
--- a/drivers/net/phy/fixed_phy.c
+++ b/drivers/net/phy/fixed_phy.c
@@ -25,6 +25,7 @@
#include <linux/gpio.h>
#include <linux/seqlock.h>
#include <linux/idr.h>
+#include <linux/netdevice.h>
#include "swphy.h"
@@ -38,6 +39,7 @@ struct fixed_phy {
struct phy_device *phydev;
seqcount_t seqcount;
struct fixed_phy_status status;
+ bool no_carrier;
int (*link_update)(struct net_device *, struct fixed_phy_status *);
struct list_head node;
int link_gpio;
@@ -48,6 +50,24 @@ static struct fixed_mdio_bus platform_fmb = {
.phys = LIST_HEAD_INIT(platform_fmb.phys),
};
+int
+fixed_phy_change_carrier(struct net_device *dev, bool new_carrier)
+{
+ struct fixed_mdio_bus *fmb = &platform_fmb;
+ struct phy_device *phydev = dev->phydev;
+ struct fixed_phy *fp;
+
+ if (!phydev || !phydev->mdio.bus)
+ return -EINVAL;
+
+ list_for_each_entry(fp, &fmb->phys, node) {
+ if (fp->addr == phydev->mdio.addr) {
+ fp->no_carrier = !new_carrier;
+ return 0;
+ }
+ }
+ return -EINVAL;
+}
static void fixed_phy_update(struct fixed_phy *fp)
{
if (gpio_is_valid(fp->link_gpio))
@@ -72,6 +92,12 @@ static int fixed_mdio_read(struct mii_bus *bus, int phy_addr, int reg_num)
&fp->status);
fixed_phy_update(fp);
}
+ if (fp->no_carrier) {
+ fp->link_update(fp->phydev->attached_dev,
+ &fp->status);
+ fp->status.link = 0;
+ }
+
state = fp->status;
} while (read_seqcount_retry(&fp->seqcount, s));
Then one just set:
+ .ndo_change_carrier = fixed_phy_change_carrier,
in each ethernet driver.
OK?
Jocke
Powered by blists - more mailing lists