RE: [PATCH net-next 6/9] net: hns3: Add MDIO support to HNS3 Ethernet driver for hip08 SoC

2017-06-17 Thread Salil Mehta 
Hi Andrew

> -Original Message-
> From: Andrew Lunn [mailto:and...@lunn.ch]
> Sent: Wednesday, June 14, 2017 2:10 AM
> To: Salil Mehta
> Cc: Florian Fainelli; da...@davemloft.net; Zhuangyuzeng (Yisen);
> huangdaode; lipeng (Y); mehta.salil@gmail.com;
> net...@vger.kernel.org; linux-kernel@vger.kernel.org; Linuxarm
> Subject: Re: [PATCH net-next 6/9] net: hns3: Add MDIO support to HNS3
> Ethernet driver for hip08 SoC
> 
> > Since I would be touching the core, lots of drivers will get impacted
> and will
> > have to wait till everyone gives clean signal. This will impact our
> internal
> > deadlines. But as I said I am eager to cooperate & contribute :)
> 
> Just as an FYI.
> 
> Your internal deadlines are irrelevant. We will reject your patches
> until we are happy with it. We probably won't accept a hack around a
> core feature limitation. We will want you to fix the core limitation
> and then do it right in the driver.
> 
> Andrew

We are dropping the support of C45 in our driver till we fix the issue
just commented upon. I will try to fix this issue and would soon float
a  separate patch for that. Later, we shall re-introduce the support of
C45 in our HNS3 driver once the PHY core fix has been reviewed and accepted.

Thanks
Salil

RE: [PATCH net-next 6/9] net: hns3: Add MDIO support to HNS3 Ethernet driver for hip08 SoC

2017-06-17 Thread Salil Mehta 
Hi Andrew

> -Original Message-
> From: Andrew Lunn [mailto:and...@lunn.ch]
> Sent: Wednesday, June 14, 2017 2:10 AM
> To: Salil Mehta
> Cc: Florian Fainelli; da...@davemloft.net; Zhuangyuzeng (Yisen);
> huangdaode; lipeng (Y); mehta.salil@gmail.com;
> net...@vger.kernel.org; linux-kernel@vger.kernel.org; Linuxarm
> Subject: Re: [PATCH net-next 6/9] net: hns3: Add MDIO support to HNS3
> Ethernet driver for hip08 SoC
> 
> > Since I would be touching the core, lots of drivers will get impacted
> and will
> > have to wait till everyone gives clean signal. This will impact our
> internal
> > deadlines. But as I said I am eager to cooperate & contribute :)
> 
> Just as an FYI.
> 
> Your internal deadlines are irrelevant. We will reject your patches
> until we are happy with it. We probably won't accept a hack around a
> core feature limitation. We will want you to fix the core limitation
> and then do it right in the driver.
> 
> Andrew

We are dropping the support of C45 in our driver till we fix the issue
just commented upon. I will try to fix this issue and would soon float
a  separate patch for that. Later, we shall re-introduce the support of
C45 in our HNS3 driver once the PHY core fix has been reviewed and accepted.

Thanks
Salil

Re: [PATCH net-next 6/9] net: hns3: Add MDIO support to HNS3 Ethernet driver for hip08 SoC

2017-06-13 Thread Andrew Lunn 
> Since I would be touching the core, lots of drivers will get impacted and will
> have to wait till everyone gives clean signal. This will impact our internal
> deadlines. But as I said I am eager to cooperate & contribute :)

Just as an FYI.

Your internal deadlines are irrelevant. We will reject your patches
until we are happy with it. We probably won't accept a hack around a
core feature limitation. We will want you to fix the core limitation
and then do it right in the driver.

Andrew

Re: [PATCH net-next 6/9] net: hns3: Add MDIO support to HNS3 Ethernet driver for hip08 SoC

2017-06-13 Thread Andrew Lunn 
> Since I would be touching the core, lots of drivers will get impacted and will
> have to wait till everyone gives clean signal. This will impact our internal
> deadlines. But as I said I am eager to cooperate & contribute :)

Just as an FYI.

Your internal deadlines are irrelevant. We will reject your patches
until we are happy with it. We probably won't accept a hack around a
core feature limitation. We will want you to fix the core limitation
and then do it right in the driver.

Andrew

RE: [PATCH net-next 6/9] net: hns3: Add MDIO support to HNS3 Ethernet driver for hip08 SoC

2017-06-13 Thread Salil Mehta 
Hi Andrew,

> -Original Message-
> From: Andrew Lunn [mailto:and...@lunn.ch]
> Sent: Tuesday, June 13, 2017 11:41 PM
> To: Salil Mehta
> Cc: Florian Fainelli; da...@davemloft.net; Zhuangyuzeng (Yisen);
> huangdaode; lipeng (Y); mehta.salil@gmail.com;
> net...@vger.kernel.org; linux-kernel@vger.kernel.org; Linuxarm
> Subject: Re: [PATCH net-next 6/9] net: hns3: Add MDIO support to HNS3
> Ethernet driver for hip08 SoC
> 
> > > Hum why do you do this? mdiobus_register() will scan through your
> bus
> > > provided that you set an appropriate phy_mask value (here you tell
> it
> > > not to) and you already provide the PHY address to scan for
> > >
> > I know this looks weird but the reason why it appears as it is in
> code is:
> >
> > mdiobus_register() calls mdiobus_scan(). If you see below code leg
> function
> > get_phy_device() assumes to be having supporting Clause 22 so its
> input
> > parameter 'is_c45' is always 'false'.
> >
> > struct phy_device *mdiobus_scan(struct mii_bus *bus, int addr)
> > {
> > struct phy_device *phydev;
> > int err;
> >
> > phydev = get_phy_device(bus, addr, false);
> > if (IS_ERR(phydev))^
> > return phydev;
> > [...]
> > }
> >
> > Therefore, to support C45 device we did below:
> >
> > * disabled the autoscan/mdiobus_scan() Of the PHY devices using the
> >   phy_mask(= ~0)
> > * Now, did almost the same thing what mdiobus_scan does i.e.
> > * get_phy_device but with is_c45 (=true/false)
> > * register the above phy device with phy_device_register()
> >
> > There could be some gap in my understanding, please help to correct
> this?
> 
> So this is the question i was asking Florian
> 
> Rather than hack around limitations of the core, you should fix the
> core. I think we should make the core first try probing using c45. If
> that comes back with an error, or does not find a device, try the
> probe using c22.
I can take this activity but please allow me to do this as a separate activity
and not part of this driver Up-streaming activity.

Since I would be touching the core, lots of drivers will get impacted and will
have to wait till everyone gives clean signal. This will impact our internal
deadlines. But as I said I am eager to cooperate & contribute :)

Thanks
Salil
> 
>   Andrew

RE: [PATCH net-next 6/9] net: hns3: Add MDIO support to HNS3 Ethernet driver for hip08 SoC

2017-06-13 Thread Salil Mehta 
Hi Andrew,

> -Original Message-
> From: Andrew Lunn [mailto:and...@lunn.ch]
> Sent: Tuesday, June 13, 2017 11:41 PM
> To: Salil Mehta
> Cc: Florian Fainelli; da...@davemloft.net; Zhuangyuzeng (Yisen);
> huangdaode; lipeng (Y); mehta.salil@gmail.com;
> net...@vger.kernel.org; linux-kernel@vger.kernel.org; Linuxarm
> Subject: Re: [PATCH net-next 6/9] net: hns3: Add MDIO support to HNS3
> Ethernet driver for hip08 SoC
> 
> > > Hum why do you do this? mdiobus_register() will scan through your
> bus
> > > provided that you set an appropriate phy_mask value (here you tell
> it
> > > not to) and you already provide the PHY address to scan for
> > >
> > I know this looks weird but the reason why it appears as it is in
> code is:
> >
> > mdiobus_register() calls mdiobus_scan(). If you see below code leg
> function
> > get_phy_device() assumes to be having supporting Clause 22 so its
> input
> > parameter 'is_c45' is always 'false'.
> >
> > struct phy_device *mdiobus_scan(struct mii_bus *bus, int addr)
> > {
> > struct phy_device *phydev;
> > int err;
> >
> > phydev = get_phy_device(bus, addr, false);
> > if (IS_ERR(phydev))^
> > return phydev;
> > [...]
> > }
> >
> > Therefore, to support C45 device we did below:
> >
> > * disabled the autoscan/mdiobus_scan() Of the PHY devices using the
> >   phy_mask(= ~0)
> > * Now, did almost the same thing what mdiobus_scan does i.e.
> > * get_phy_device but with is_c45 (=true/false)
> > * register the above phy device with phy_device_register()
> >
> > There could be some gap in my understanding, please help to correct
> this?
> 
> So this is the question i was asking Florian
> 
> Rather than hack around limitations of the core, you should fix the
> core. I think we should make the core first try probing using c45. If
> that comes back with an error, or does not find a device, try the
> probe using c22.
I can take this activity but please allow me to do this as a separate activity
and not part of this driver Up-streaming activity.

Since I would be touching the core, lots of drivers will get impacted and will
have to wait till everyone gives clean signal. This will impact our internal
deadlines. But as I said I am eager to cooperate & contribute :)

Thanks
Salil
> 
>   Andrew

Re: [PATCH net-next 6/9] net: hns3: Add MDIO support to HNS3 Ethernet driver for hip08 SoC

2017-06-13 Thread Andrew Lunn 
> > Hum why do you do this? mdiobus_register() will scan through your bus
> > provided that you set an appropriate phy_mask value (here you tell it
> > not to) and you already provide the PHY address to scan for
> >
> I know this looks weird but the reason why it appears as it is in code is:
>  
> mdiobus_register() calls mdiobus_scan(). If you see below code leg function
> get_phy_device() assumes to be having supporting Clause 22 so its input
> parameter 'is_c45' is always 'false'.
> 
> struct phy_device *mdiobus_scan(struct mii_bus *bus, int addr)
> {
>   struct phy_device *phydev;
>   int err;
> 
>   phydev = get_phy_device(bus, addr, false);
>   if (IS_ERR(phydev))^
>   return phydev;
>   [...]
> }
> 
> Therefore, to support C45 device we did below:
> 
> * disabled the autoscan/mdiobus_scan() Of the PHY devices using the
>   phy_mask(= ~0) 
> * Now, did almost the same thing what mdiobus_scan does i.e.
> * get_phy_device but with is_c45 (=true/false)
> * register the above phy device with phy_device_register()
> 
> There could be some gap in my understanding, please help to correct this?

So this is the question i was asking Florian

Rather than hack around limitations of the core, you should fix the
core. I think we should make the core first try probing using c45. If
that comes back with an error, or does not find a device, try the
probe using c22.

  Andrew

Re: [PATCH net-next 6/9] net: hns3: Add MDIO support to HNS3 Ethernet driver for hip08 SoC

2017-06-13 Thread Andrew Lunn 
> > Hum why do you do this? mdiobus_register() will scan through your bus
> > provided that you set an appropriate phy_mask value (here you tell it
> > not to) and you already provide the PHY address to scan for
> >
> I know this looks weird but the reason why it appears as it is in code is:
>  
> mdiobus_register() calls mdiobus_scan(). If you see below code leg function
> get_phy_device() assumes to be having supporting Clause 22 so its input
> parameter 'is_c45' is always 'false'.
> 
> struct phy_device *mdiobus_scan(struct mii_bus *bus, int addr)
> {
>   struct phy_device *phydev;
>   int err;
> 
>   phydev = get_phy_device(bus, addr, false);
>   if (IS_ERR(phydev))^
>   return phydev;
>   [...]
> }
> 
> Therefore, to support C45 device we did below:
> 
> * disabled the autoscan/mdiobus_scan() Of the PHY devices using the
>   phy_mask(= ~0) 
> * Now, did almost the same thing what mdiobus_scan does i.e.
> * get_phy_device but with is_c45 (=true/false)
> * register the above phy device with phy_device_register()
> 
> There could be some gap in my understanding, please help to correct this?

So this is the question i was asking Florian

Rather than hack around limitations of the core, you should fix the
core. I think we should make the core first try probing using c45. If
that comes back with an error, or does not find a device, try the
probe using c22.

  Andrew

RE: [PATCH net-next 6/9] net: hns3: Add MDIO support to HNS3 Ethernet driver for hip08 SoC

2017-06-13 Thread Salil Mehta 
Hi Florian,

> -Original Message-
> From: Florian Fainelli [mailto:f.faine...@gmail.com]
> Sent: Saturday, June 10, 2017 8:04 PM
> To: Salil Mehta; da...@davemloft.net
> Cc: Zhuangyuzeng (Yisen); huangdaode; lipeng (Y);
> mehta.salil@gmail.com; net...@vger.kernel.org; linux-
> ker...@vger.kernel.org; Linuxarm; Andrew Lunn
> Subject: Re: [PATCH net-next 6/9] net: hns3: Add MDIO support to HNS3
> Ethernet driver for hip08 SoC
> 
> Le 06/09/17 à 20:46, Salil Mehta a écrit :
> > This patch adds the support of MDIO bus interface for HNS3 driver.
> > Code provides various interfaces to start and stop the PHY layer
> > and to read and write the MDIO bus or PHY.
> >
> > Signed-off-by: Daode Huang <huangda...@hisilicon.com>
> > Signed-off-by: lipeng <lipeng...@huawei.com>
> > Signed-off-by: Salil Mehta <salil.me...@huawei.com>
> > Signed-off-by: Yisen Zhuang <yisen.zhu...@huawei.com>
> > ---
> >  .../ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c| 310
> +
> >  1 file changed, 310 insertions(+)
> >  create mode 100644
> drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c
> >
> > diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c
> b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c
> > new file mode 100644
> > index 000..c6812d2
> > --- /dev/null
> > +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c
> > @@ -0,0 +1,310 @@
> > +/*
> > + * Copyright (c) 2016~2017 Hisilicon Limited.
> > + *
> > + * This program is free software; you can redistribute it and/or
> modify
> > + * it under the terms of the GNU General Public License as published
> by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + */
> > +
> > +#include 
> > +#include 
> > +
> > +#include "hclge_cmd.h"
> > +#include "hclge_main.h"
> > +
> > +enum hclge_mdio_c22_op_seq {
> > +   HCLGE_MDIO_C22_WRITE = 1,
> > +   HCLGE_MDIO_C22_READ = 2
> > +};
> > +
> > +enum hclge_mdio_c45_op_seq {
> > +   HCLGE_MDIO_C45_WRITE_ADDR = 0,
> > +   HCLGE_MDIO_C45_WRITE_DATA,
> > +   HCLGE_MDIO_C45_READ_INCREMENT,
> > +   HCLGE_MDIO_C45_READ
> > +};
> > +
> > +#define HCLGE_MDIO_CTRL_START_BIT   BIT(0)
> > +#define HCLGE_MDIO_CTRL_ST_MSK  GENMASK(2, 1)
> > +#define HCLGE_MDIO_CTRL_ST_LSH  1
> > +#define HCLGE_MDIO_IS_C22(c22)  (((c22) << HCLGE_MDIO_CTRL_ST_LSH) &
> \
> > +   HCLGE_MDIO_CTRL_ST_MSK)
> > +
> > +#define HCLGE_MDIO_CTRL_OP_MSK  GENMASK(4, 3)
> > +#define HCLGE_MDIO_CTRL_OP_LSH  3
> > +#define HCLGE_MDIO_CTRL_OP(access) \
> > +   (((access) << HCLGE_MDIO_CTRL_OP_LSH) & HCLGE_MDIO_CTRL_OP_MSK)
> > +#define HCLGE_MDIO_CTRL_PRTAD_MSK   GENMASK(4, 0)
> > +#define HCLGE_MDIO_CTRL_DEVAD_MSK   GENMASK(4, 0)
> > +
> > +#define HCLGE_MDIO_STA_VAL(val)((val) & BIT(0))
> > +
> > +struct hclge_mdio_cfg_cmd {
> > +   u8 ctrl_bit;
> > +   u8 prtad;   /* The external port address */
> > +   u8 devad;   /* The external device address */
> > +   u8 rsvd;
> > +   __le16 addr_c45;/* Only valid for c45 */
> > +   __le16 data_wr;
> > +   __le16 data_rd;
> > +   __le16 sta;> +};
> > +
> > +static int hclge_mdio_write(struct mii_bus *bus, int phy_id, int
> regnum,
> > +   u16 data)
> > +{
> > +   struct hclge_dev *hdev = (struct hclge_dev *)bus->priv;
> > +   struct hclge_mdio_cfg_cmd *mdio_cmd;
> > +   enum hclge_cmd_status status;
> > +   struct hclge_desc desc;
> > +   u8 is_c45, devad;
> > +   u16 reg;
> > +
> > +   if (!bus)
> > +   return -EINVAL;
> > +
> > +   is_c45 = !!(regnum & MII_ADDR_C45);
> > +   devad = ((regnum >> 16) & 0x1f);
> > +   reg = (u16)(regnum & 0x);
> > +
> > +   hclge_cmd_setup_basic_desc(, HCLGE_OPC_MDIO_CONFIG, false);
> > +
> > +   mdio_cmd = (struct hclge_mdio_cfg_cmd *)desc.data;
> > +
> > +   if (!is_c45) {
> 
> It would be more readable with positive logic: if (is_c45) { } else { }
Fine, will change.

Thanks
Salil
> 
> > +   /* C22 write reg and data */
> > +   mdio_cmd->ctrl_bit = HCLGE_MDIO_IS_C22(!is_c45);
> > +   mdio_cmd->ctrl_bit |=
> HCLGE_MDIO_CTRL_OP(HCLGE_MDIO_C22_WRITE);
> > +   mdio_cmd->ctrl_bit |= HCLGE_MDIO_CTRL_START_BIT;
> > +   mdio_cmd->dat

RE: [PATCH net-next 6/9] net: hns3: Add MDIO support to HNS3 Ethernet driver for hip08 SoC

2017-06-13 Thread Salil Mehta 
Hi Florian,

> -Original Message-
> From: Florian Fainelli [mailto:f.faine...@gmail.com]
> Sent: Saturday, June 10, 2017 8:04 PM
> To: Salil Mehta; da...@davemloft.net
> Cc: Zhuangyuzeng (Yisen); huangdaode; lipeng (Y);
> mehta.salil@gmail.com; net...@vger.kernel.org; linux-
> ker...@vger.kernel.org; Linuxarm; Andrew Lunn
> Subject: Re: [PATCH net-next 6/9] net: hns3: Add MDIO support to HNS3
> Ethernet driver for hip08 SoC
> 
> Le 06/09/17 à 20:46, Salil Mehta a écrit :
> > This patch adds the support of MDIO bus interface for HNS3 driver.
> > Code provides various interfaces to start and stop the PHY layer
> > and to read and write the MDIO bus or PHY.
> >
> > Signed-off-by: Daode Huang 
> > Signed-off-by: lipeng 
> > Signed-off-by: Salil Mehta 
> > Signed-off-by: Yisen Zhuang 
> > ---
> >  .../ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c| 310
> +
> >  1 file changed, 310 insertions(+)
> >  create mode 100644
> drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c
> >
> > diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c
> b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c
> > new file mode 100644
> > index 000..c6812d2
> > --- /dev/null
> > +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c
> > @@ -0,0 +1,310 @@
> > +/*
> > + * Copyright (c) 2016~2017 Hisilicon Limited.
> > + *
> > + * This program is free software; you can redistribute it and/or
> modify
> > + * it under the terms of the GNU General Public License as published
> by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + */
> > +
> > +#include 
> > +#include 
> > +
> > +#include "hclge_cmd.h"
> > +#include "hclge_main.h"
> > +
> > +enum hclge_mdio_c22_op_seq {
> > +   HCLGE_MDIO_C22_WRITE = 1,
> > +   HCLGE_MDIO_C22_READ = 2
> > +};
> > +
> > +enum hclge_mdio_c45_op_seq {
> > +   HCLGE_MDIO_C45_WRITE_ADDR = 0,
> > +   HCLGE_MDIO_C45_WRITE_DATA,
> > +   HCLGE_MDIO_C45_READ_INCREMENT,
> > +   HCLGE_MDIO_C45_READ
> > +};
> > +
> > +#define HCLGE_MDIO_CTRL_START_BIT   BIT(0)
> > +#define HCLGE_MDIO_CTRL_ST_MSK  GENMASK(2, 1)
> > +#define HCLGE_MDIO_CTRL_ST_LSH  1
> > +#define HCLGE_MDIO_IS_C22(c22)  (((c22) << HCLGE_MDIO_CTRL_ST_LSH) &
> \
> > +   HCLGE_MDIO_CTRL_ST_MSK)
> > +
> > +#define HCLGE_MDIO_CTRL_OP_MSK  GENMASK(4, 3)
> > +#define HCLGE_MDIO_CTRL_OP_LSH  3
> > +#define HCLGE_MDIO_CTRL_OP(access) \
> > +   (((access) << HCLGE_MDIO_CTRL_OP_LSH) & HCLGE_MDIO_CTRL_OP_MSK)
> > +#define HCLGE_MDIO_CTRL_PRTAD_MSK   GENMASK(4, 0)
> > +#define HCLGE_MDIO_CTRL_DEVAD_MSK   GENMASK(4, 0)
> > +
> > +#define HCLGE_MDIO_STA_VAL(val)((val) & BIT(0))
> > +
> > +struct hclge_mdio_cfg_cmd {
> > +   u8 ctrl_bit;
> > +   u8 prtad;   /* The external port address */
> > +   u8 devad;   /* The external device address */
> > +   u8 rsvd;
> > +   __le16 addr_c45;/* Only valid for c45 */
> > +   __le16 data_wr;
> > +   __le16 data_rd;
> > +   __le16 sta;> +};
> > +
> > +static int hclge_mdio_write(struct mii_bus *bus, int phy_id, int
> regnum,
> > +   u16 data)
> > +{
> > +   struct hclge_dev *hdev = (struct hclge_dev *)bus->priv;
> > +   struct hclge_mdio_cfg_cmd *mdio_cmd;
> > +   enum hclge_cmd_status status;
> > +   struct hclge_desc desc;
> > +   u8 is_c45, devad;
> > +   u16 reg;
> > +
> > +   if (!bus)
> > +   return -EINVAL;
> > +
> > +   is_c45 = !!(regnum & MII_ADDR_C45);
> > +   devad = ((regnum >> 16) & 0x1f);
> > +   reg = (u16)(regnum & 0x);
> > +
> > +   hclge_cmd_setup_basic_desc(, HCLGE_OPC_MDIO_CONFIG, false);
> > +
> > +   mdio_cmd = (struct hclge_mdio_cfg_cmd *)desc.data;
> > +
> > +   if (!is_c45) {
> 
> It would be more readable with positive logic: if (is_c45) { } else { }
Fine, will change.

Thanks
Salil
> 
> > +   /* C22 write reg and data */
> > +   mdio_cmd->ctrl_bit = HCLGE_MDIO_IS_C22(!is_c45);
> > +   mdio_cmd->ctrl_bit |=
> HCLGE_MDIO_CTRL_OP(HCLGE_MDIO_C22_WRITE);
> > +   mdio_cmd->ctrl_bit |= HCLGE_MDIO_CTRL_START_BIT;
> > +   mdio_cmd->data_wr = cpu_to_le16(data);
> > +   mdio_cmd->devad = devad & HCLGE_MDIO_CTRL_DEVAD_MSK;
&

Re: [PATCH net-next 6/9] net: hns3: Add MDIO support to HNS3 Ethernet driver for hip08 SoC

2017-06-13 Thread Andrew Lunn 
> You are correct is_c45 should not be derived from the type of PHY like
> it is being done here. I have changed this code and now we are getting this
> information from the IMP(Integrated Management Processor) which handles
> the NIC commands and also maintains Ethernet specific configuration.

Florian

With c22, registering the MDIO bus is enough to cause all c22
addresses to be probed and the PHYs found. If you are not using device
tree, you can then use phy_find_first() to get the first phy on the
bus.

As far as i remember, this does not work for c45. mdiobus_scan() is
only looking for c22 PHYs. Maybe we should be making mdiobus_scan()
look first for a c45 and then try c22?

 Andrew

Re: [PATCH net-next 6/9] net: hns3: Add MDIO support to HNS3 Ethernet driver for hip08 SoC

2017-06-13 Thread Andrew Lunn 
> You are correct is_c45 should not be derived from the type of PHY like
> it is being done here. I have changed this code and now we are getting this
> information from the IMP(Integrated Management Processor) which handles
> the NIC commands and also maintains Ethernet specific configuration.

Florian

With c22, registering the MDIO bus is enough to cause all c22
addresses to be probed and the PHYs found. If you are not using device
tree, you can then use phy_find_first() to get the first phy on the
bus.

As far as i remember, this does not work for c45. mdiobus_scan() is
only looking for c22 PHYs. Maybe we should be making mdiobus_scan()
look first for a c45 and then try c22?

 Andrew

RE: [PATCH net-next 6/9] net: hns3: Add MDIO support to HNS3 Ethernet driver for hip08 SoC

2017-06-13 Thread Salil Mehta 
Hi Andrew,

> -Original Message-
> From: Andrew Lunn [mailto:and...@lunn.ch]
> Sent: Sunday, June 11, 2017 3:43 AM
> To: Florian Fainelli
> Cc: Salil Mehta; da...@davemloft.net; Zhuangyuzeng (Yisen); huangdaode;
> lipeng (Y); mehta.salil@gmail.com; net...@vger.kernel.org; linux-
> ker...@vger.kernel.org; Linuxarm
> Subject: Re: [PATCH net-next 6/9] net: hns3: Add MDIO support to HNS3
> Ethernet driver for hip08 SoC
> 
> > > +int hclge_mac_mdio_config(struct hclge_dev *hdev)
> > > +{
> > > + struct hclge_mac *mac = >hw.mac;
> > > + struct mii_bus *mdio_bus;
> > > + struct net_device *ndev = >ndev;
> > > + struct phy_device *phy;
> > > + bool is_c45;
> > > + int ret;
> > > +
> > > + if (hdev->hw.mac.phy_addr >= PHY_MAX_ADDR)
> > > + return 0;
> > > +
> > > + if (hdev->hw.mac.phy_if == PHY_INTERFACE_MODE_NA)
> > > + return 0;
> > > + else if (mac->phy_if == PHY_INTERFACE_MODE_SGMII)
> > > + is_c45 = 0;
> > > + else if (mac->phy_if == PHY_INTERFACE_MODE_XGMII)
> > > + is_c45 = 1;
> > > + else
> > > + return -ENODATA;
> >
> > Can you consider using a switch () case statement here?
> 
> Does this concept even make sense? The Marvell 10G phy will use SGMII
> for 10/100/1000Mbs and swap to XGMII for 10Gbps. It however stays a
> c45 device all the time.
> 
> In general, i don't think PHY mode is related to C22/C45.
You are correct is_c45 should not be derived from the type of PHY like
it is being done here. I have changed this code and now we are getting this
information from the IMP(Integrated Management Processor) which handles
the NIC commands and also maintains Ethernet specific configuration.

Thanks
Salil
> 
>Andrew

RE: [PATCH net-next 6/9] net: hns3: Add MDIO support to HNS3 Ethernet driver for hip08 SoC

2017-06-13 Thread Salil Mehta 
Hi Andrew,

> -Original Message-
> From: Andrew Lunn [mailto:and...@lunn.ch]
> Sent: Sunday, June 11, 2017 3:43 AM
> To: Florian Fainelli
> Cc: Salil Mehta; da...@davemloft.net; Zhuangyuzeng (Yisen); huangdaode;
> lipeng (Y); mehta.salil@gmail.com; net...@vger.kernel.org; linux-
> ker...@vger.kernel.org; Linuxarm
> Subject: Re: [PATCH net-next 6/9] net: hns3: Add MDIO support to HNS3
> Ethernet driver for hip08 SoC
> 
> > > +int hclge_mac_mdio_config(struct hclge_dev *hdev)
> > > +{
> > > + struct hclge_mac *mac = >hw.mac;
> > > + struct mii_bus *mdio_bus;
> > > + struct net_device *ndev = >ndev;
> > > + struct phy_device *phy;
> > > + bool is_c45;
> > > + int ret;
> > > +
> > > + if (hdev->hw.mac.phy_addr >= PHY_MAX_ADDR)
> > > + return 0;
> > > +
> > > + if (hdev->hw.mac.phy_if == PHY_INTERFACE_MODE_NA)
> > > + return 0;
> > > + else if (mac->phy_if == PHY_INTERFACE_MODE_SGMII)
> > > + is_c45 = 0;
> > > + else if (mac->phy_if == PHY_INTERFACE_MODE_XGMII)
> > > + is_c45 = 1;
> > > + else
> > > + return -ENODATA;
> >
> > Can you consider using a switch () case statement here?
> 
> Does this concept even make sense? The Marvell 10G phy will use SGMII
> for 10/100/1000Mbs and swap to XGMII for 10Gbps. It however stays a
> c45 device all the time.
> 
> In general, i don't think PHY mode is related to C22/C45.
You are correct is_c45 should not be derived from the type of PHY like
it is being done here. I have changed this code and now we are getting this
information from the IMP(Integrated Management Processor) which handles
the NIC commands and also maintains Ethernet specific configuration.

Thanks
Salil
> 
>Andrew

Re: [PATCH net-next 6/9] net: hns3: Add MDIO support to HNS3 Ethernet driver for hip08 SoC

2017-06-10 Thread Andrew Lunn 
> > +int hclge_mac_mdio_config(struct hclge_dev *hdev)
> > +{
> > +   struct hclge_mac *mac = >hw.mac;
> > +   struct mii_bus *mdio_bus;
> > +   struct net_device *ndev = >ndev;
> > +   struct phy_device *phy;
> > +   bool is_c45;
> > +   int ret;
> > +
> > +   if (hdev->hw.mac.phy_addr >= PHY_MAX_ADDR)
> > +   return 0;
> > +
> > +   if (hdev->hw.mac.phy_if == PHY_INTERFACE_MODE_NA)
> > +   return 0;
> > +   else if (mac->phy_if == PHY_INTERFACE_MODE_SGMII)
> > +   is_c45 = 0;
> > +   else if (mac->phy_if == PHY_INTERFACE_MODE_XGMII)
> > +   is_c45 = 1;
> > +   else
> > +   return -ENODATA;
> 
> Can you consider using a switch () case statement here?

Does this concept even make sense? The Marvell 10G phy will use SGMII
for 10/100/1000Mbs and swap to XGMII for 10Gbps. It however stays a
c45 device all the time.

In general, i don't think PHY mode is related to C22/C45.

   Andrew

Re: [PATCH net-next 6/9] net: hns3: Add MDIO support to HNS3 Ethernet driver for hip08 SoC

2017-06-10 Thread Andrew Lunn 
> > +int hclge_mac_mdio_config(struct hclge_dev *hdev)
> > +{
> > +   struct hclge_mac *mac = >hw.mac;
> > +   struct mii_bus *mdio_bus;
> > +   struct net_device *ndev = >ndev;
> > +   struct phy_device *phy;
> > +   bool is_c45;
> > +   int ret;
> > +
> > +   if (hdev->hw.mac.phy_addr >= PHY_MAX_ADDR)
> > +   return 0;
> > +
> > +   if (hdev->hw.mac.phy_if == PHY_INTERFACE_MODE_NA)
> > +   return 0;
> > +   else if (mac->phy_if == PHY_INTERFACE_MODE_SGMII)
> > +   is_c45 = 0;
> > +   else if (mac->phy_if == PHY_INTERFACE_MODE_XGMII)
> > +   is_c45 = 1;
> > +   else
> > +   return -ENODATA;
> 
> Can you consider using a switch () case statement here?

Does this concept even make sense? The Marvell 10G phy will use SGMII
for 10/100/1000Mbs and swap to XGMII for 10Gbps. It however stays a
c45 device all the time.

In general, i don't think PHY mode is related to C22/C45.

   Andrew

Re: [PATCH net-next 6/9] net: hns3: Add MDIO support to HNS3 Ethernet driver for hip08 SoC

2017-06-10 Thread Florian Fainelli 
Le 06/09/17 à 20:46, Salil Mehta a écrit :
> This patch adds the support of MDIO bus interface for HNS3 driver.
> Code provides various interfaces to start and stop the PHY layer
> and to read and write the MDIO bus or PHY.
> 
> Signed-off-by: Daode Huang 
> Signed-off-by: lipeng 
> Signed-off-by: Salil Mehta 
> Signed-off-by: Yisen Zhuang 
> ---
>  .../ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c| 310 
> +
>  1 file changed, 310 insertions(+)
>  create mode 100644 drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c
> 
> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c 
> b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c
> new file mode 100644
> index 000..c6812d2
> --- /dev/null
> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c
> @@ -0,0 +1,310 @@
> +/*
> + * Copyright (c) 2016~2017 Hisilicon Limited.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#include 
> +#include 
> +
> +#include "hclge_cmd.h"
> +#include "hclge_main.h"
> +
> +enum hclge_mdio_c22_op_seq {
> + HCLGE_MDIO_C22_WRITE = 1,
> + HCLGE_MDIO_C22_READ = 2
> +};
> +
> +enum hclge_mdio_c45_op_seq {
> + HCLGE_MDIO_C45_WRITE_ADDR = 0,
> + HCLGE_MDIO_C45_WRITE_DATA,
> + HCLGE_MDIO_C45_READ_INCREMENT,
> + HCLGE_MDIO_C45_READ
> +};
> +
> +#define HCLGE_MDIO_CTRL_START_BIT   BIT(0)
> +#define HCLGE_MDIO_CTRL_ST_MSK  GENMASK(2, 1)
> +#define HCLGE_MDIO_CTRL_ST_LSH  1
> +#define HCLGE_MDIO_IS_C22(c22)  (((c22) << HCLGE_MDIO_CTRL_ST_LSH) & \
> + HCLGE_MDIO_CTRL_ST_MSK)
> +
> +#define HCLGE_MDIO_CTRL_OP_MSK  GENMASK(4, 3)
> +#define HCLGE_MDIO_CTRL_OP_LSH  3
> +#define HCLGE_MDIO_CTRL_OP(access) \
> + (((access) << HCLGE_MDIO_CTRL_OP_LSH) & HCLGE_MDIO_CTRL_OP_MSK)
> +#define HCLGE_MDIO_CTRL_PRTAD_MSK   GENMASK(4, 0)
> +#define HCLGE_MDIO_CTRL_DEVAD_MSK   GENMASK(4, 0)
> +
> +#define HCLGE_MDIO_STA_VAL(val)  ((val) & BIT(0))
> +
> +struct hclge_mdio_cfg_cmd {
> + u8 ctrl_bit;
> + u8 prtad;   /* The external port address */
> + u8 devad;   /* The external device address */
> + u8 rsvd;
> + __le16 addr_c45;/* Only valid for c45 */
> + __le16 data_wr;
> + __le16 data_rd;
> + __le16 sta;> +};
> +
> +static int hclge_mdio_write(struct mii_bus *bus, int phy_id, int regnum,
> + u16 data)
> +{
> + struct hclge_dev *hdev = (struct hclge_dev *)bus->priv;
> + struct hclge_mdio_cfg_cmd *mdio_cmd;
> + enum hclge_cmd_status status;
> + struct hclge_desc desc;
> + u8 is_c45, devad;
> + u16 reg;
> +
> + if (!bus)
> + return -EINVAL;
> +
> + is_c45 = !!(regnum & MII_ADDR_C45);
> + devad = ((regnum >> 16) & 0x1f);
> + reg = (u16)(regnum & 0x);
> +
> + hclge_cmd_setup_basic_desc(, HCLGE_OPC_MDIO_CONFIG, false);
> +
> + mdio_cmd = (struct hclge_mdio_cfg_cmd *)desc.data;
> +
> + if (!is_c45) {

It would be more readable with positive logic: if (is_c45) { } else { }

> + /* C22 write reg and data */
> + mdio_cmd->ctrl_bit = HCLGE_MDIO_IS_C22(!is_c45);
> + mdio_cmd->ctrl_bit |= HCLGE_MDIO_CTRL_OP(HCLGE_MDIO_C22_WRITE);
> + mdio_cmd->ctrl_bit |= HCLGE_MDIO_CTRL_START_BIT;
> + mdio_cmd->data_wr = cpu_to_le16(data);
> + mdio_cmd->devad = devad & HCLGE_MDIO_CTRL_DEVAD_MSK;
> + mdio_cmd->prtad = phy_id & HCLGE_MDIO_CTRL_PRTAD_MSK;
> + } else {
> + /* Set phy addr */
> + mdio_cmd->ctrl_bit |= HCLGE_MDIO_CTRL_START_BIT;
> + mdio_cmd->addr_c45 = cpu_to_le16(reg);
> + mdio_cmd->data_wr = cpu_to_le16(data);
> + mdio_cmd->devad = devad & HCLGE_MDIO_CTRL_DEVAD_MSK;
> + mdio_cmd->prtad = phy_id & HCLGE_MDIO_CTRL_PRTAD_MSK;
> + }

There is some common initialization that you could probably extracted
out of the C22/C45 clause here.

> +
> + status = hclge_cmd_send(>hw, , 1);
> + if (status) {
> + dev_err(>pdev->dev,
> + "mdio write fail when sending cmd, status is %d.\n",
> + status);
> + return -EIO;
> + }
> +
> + return 0;
> +}
> +
> +static int hclge_mdio_read(struct mii_bus *bus, int phy_id, int regnum)
> +{
> + struct hclge_dev *hdev = (struct hclge_dev *)bus->priv;
> + struct hclge_mdio_cfg_cmd *mdio_cmd;
> + enum hclge_cmd_status status;
> + struct hclge_desc desc;
> + u8 is_c45, devad;
> + u16 reg;
> +
> + if (!bus)
> + return -EINVAL;
> +
> + is_c45 = !!(regnum & MII_ADDR_C45);
> + devad = ((regnum >> 16) & GENMASK(4,

Re: [PATCH net-next 6/9] net: hns3: Add MDIO support to HNS3 Ethernet driver for hip08 SoC

2017-06-10 Thread Florian Fainelli 
Le 06/09/17 à 20:46, Salil Mehta a écrit :
> This patch adds the support of MDIO bus interface for HNS3 driver.
> Code provides various interfaces to start and stop the PHY layer
> and to read and write the MDIO bus or PHY.
> 
> Signed-off-by: Daode Huang 
> Signed-off-by: lipeng 
> Signed-off-by: Salil Mehta 
> Signed-off-by: Yisen Zhuang 
> ---
>  .../ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c| 310 
> +
>  1 file changed, 310 insertions(+)
>  create mode 100644 drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c
> 
> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c 
> b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c
> new file mode 100644
> index 000..c6812d2
> --- /dev/null
> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c
> @@ -0,0 +1,310 @@
> +/*
> + * Copyright (c) 2016~2017 Hisilicon Limited.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#include 
> +#include 
> +
> +#include "hclge_cmd.h"
> +#include "hclge_main.h"
> +
> +enum hclge_mdio_c22_op_seq {
> + HCLGE_MDIO_C22_WRITE = 1,
> + HCLGE_MDIO_C22_READ = 2
> +};
> +
> +enum hclge_mdio_c45_op_seq {
> + HCLGE_MDIO_C45_WRITE_ADDR = 0,
> + HCLGE_MDIO_C45_WRITE_DATA,
> + HCLGE_MDIO_C45_READ_INCREMENT,
> + HCLGE_MDIO_C45_READ
> +};
> +
> +#define HCLGE_MDIO_CTRL_START_BIT   BIT(0)
> +#define HCLGE_MDIO_CTRL_ST_MSK  GENMASK(2, 1)
> +#define HCLGE_MDIO_CTRL_ST_LSH  1
> +#define HCLGE_MDIO_IS_C22(c22)  (((c22) << HCLGE_MDIO_CTRL_ST_LSH) & \
> + HCLGE_MDIO_CTRL_ST_MSK)
> +
> +#define HCLGE_MDIO_CTRL_OP_MSK  GENMASK(4, 3)
> +#define HCLGE_MDIO_CTRL_OP_LSH  3
> +#define HCLGE_MDIO_CTRL_OP(access) \
> + (((access) << HCLGE_MDIO_CTRL_OP_LSH) & HCLGE_MDIO_CTRL_OP_MSK)
> +#define HCLGE_MDIO_CTRL_PRTAD_MSK   GENMASK(4, 0)
> +#define HCLGE_MDIO_CTRL_DEVAD_MSK   GENMASK(4, 0)
> +
> +#define HCLGE_MDIO_STA_VAL(val)  ((val) & BIT(0))
> +
> +struct hclge_mdio_cfg_cmd {
> + u8 ctrl_bit;
> + u8 prtad;   /* The external port address */
> + u8 devad;   /* The external device address */
> + u8 rsvd;
> + __le16 addr_c45;/* Only valid for c45 */
> + __le16 data_wr;
> + __le16 data_rd;
> + __le16 sta;> +};
> +
> +static int hclge_mdio_write(struct mii_bus *bus, int phy_id, int regnum,
> + u16 data)
> +{
> + struct hclge_dev *hdev = (struct hclge_dev *)bus->priv;
> + struct hclge_mdio_cfg_cmd *mdio_cmd;
> + enum hclge_cmd_status status;
> + struct hclge_desc desc;
> + u8 is_c45, devad;
> + u16 reg;
> +
> + if (!bus)
> + return -EINVAL;
> +
> + is_c45 = !!(regnum & MII_ADDR_C45);
> + devad = ((regnum >> 16) & 0x1f);
> + reg = (u16)(regnum & 0x);
> +
> + hclge_cmd_setup_basic_desc(, HCLGE_OPC_MDIO_CONFIG, false);
> +
> + mdio_cmd = (struct hclge_mdio_cfg_cmd *)desc.data;
> +
> + if (!is_c45) {

It would be more readable with positive logic: if (is_c45) { } else { }

> + /* C22 write reg and data */
> + mdio_cmd->ctrl_bit = HCLGE_MDIO_IS_C22(!is_c45);
> + mdio_cmd->ctrl_bit |= HCLGE_MDIO_CTRL_OP(HCLGE_MDIO_C22_WRITE);
> + mdio_cmd->ctrl_bit |= HCLGE_MDIO_CTRL_START_BIT;
> + mdio_cmd->data_wr = cpu_to_le16(data);
> + mdio_cmd->devad = devad & HCLGE_MDIO_CTRL_DEVAD_MSK;
> + mdio_cmd->prtad = phy_id & HCLGE_MDIO_CTRL_PRTAD_MSK;
> + } else {
> + /* Set phy addr */
> + mdio_cmd->ctrl_bit |= HCLGE_MDIO_CTRL_START_BIT;
> + mdio_cmd->addr_c45 = cpu_to_le16(reg);
> + mdio_cmd->data_wr = cpu_to_le16(data);
> + mdio_cmd->devad = devad & HCLGE_MDIO_CTRL_DEVAD_MSK;
> + mdio_cmd->prtad = phy_id & HCLGE_MDIO_CTRL_PRTAD_MSK;
> + }

There is some common initialization that you could probably extracted
out of the C22/C45 clause here.

> +
> + status = hclge_cmd_send(>hw, , 1);
> + if (status) {
> + dev_err(>pdev->dev,
> + "mdio write fail when sending cmd, status is %d.\n",
> + status);
> + return -EIO;
> + }
> +
> + return 0;
> +}
> +
> +static int hclge_mdio_read(struct mii_bus *bus, int phy_id, int regnum)
> +{
> + struct hclge_dev *hdev = (struct hclge_dev *)bus->priv;
> + struct hclge_mdio_cfg_cmd *mdio_cmd;
> + enum hclge_cmd_status status;
> + struct hclge_desc desc;
> + u8 is_c45, devad;
> + u16 reg;
> +
> + if (!bus)
> + return -EINVAL;
> +
> + is_c45 = !!(regnum & MII_ADDR_C45);
> + devad = ((regnum >> 16) & GENMASK(4, 0));
> + reg = (u16)(regnum & GENMASK(15, 0));
> +
> + hclge_cmd_setup_basic_desc(,

[PATCH net-next 6/9] net: hns3: Add MDIO support to HNS3 Ethernet driver for hip08 SoC

2017-06-09 Thread Salil Mehta 
This patch adds the support of MDIO bus interface for HNS3 driver.
Code provides various interfaces to start and stop the PHY layer
and to read and write the MDIO bus or PHY.

Signed-off-by: Daode Huang 
Signed-off-by: lipeng 
Signed-off-by: Salil Mehta 
Signed-off-by: Yisen Zhuang 
---
 .../ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c| 310 +
 1 file changed, 310 insertions(+)
 create mode 100644 drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c 
b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c
new file mode 100644
index 000..c6812d2
--- /dev/null
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c
@@ -0,0 +1,310 @@
+/*
+ * Copyright (c) 2016~2017 Hisilicon Limited.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include 
+#include 
+
+#include "hclge_cmd.h"
+#include "hclge_main.h"
+
+enum hclge_mdio_c22_op_seq {
+   HCLGE_MDIO_C22_WRITE = 1,
+   HCLGE_MDIO_C22_READ = 2
+};
+
+enum hclge_mdio_c45_op_seq {
+   HCLGE_MDIO_C45_WRITE_ADDR = 0,
+   HCLGE_MDIO_C45_WRITE_DATA,
+   HCLGE_MDIO_C45_READ_INCREMENT,
+   HCLGE_MDIO_C45_READ
+};
+
+#define HCLGE_MDIO_CTRL_START_BIT   BIT(0)
+#define HCLGE_MDIO_CTRL_ST_MSK  GENMASK(2, 1)
+#define HCLGE_MDIO_CTRL_ST_LSH  1
+#define HCLGE_MDIO_IS_C22(c22)  (((c22) << HCLGE_MDIO_CTRL_ST_LSH) & \
+   HCLGE_MDIO_CTRL_ST_MSK)
+
+#define HCLGE_MDIO_CTRL_OP_MSK  GENMASK(4, 3)
+#define HCLGE_MDIO_CTRL_OP_LSH  3
+#define HCLGE_MDIO_CTRL_OP(access) \
+   (((access) << HCLGE_MDIO_CTRL_OP_LSH) & HCLGE_MDIO_CTRL_OP_MSK)
+#define HCLGE_MDIO_CTRL_PRTAD_MSK   GENMASK(4, 0)
+#define HCLGE_MDIO_CTRL_DEVAD_MSK   GENMASK(4, 0)
+
+#define HCLGE_MDIO_STA_VAL(val)((val) & BIT(0))
+
+struct hclge_mdio_cfg_cmd {
+   u8 ctrl_bit;
+   u8 prtad;   /* The external port address */
+   u8 devad;   /* The external device address */
+   u8 rsvd;
+   __le16 addr_c45;/* Only valid for c45 */
+   __le16 data_wr;
+   __le16 data_rd;
+   __le16 sta;
+};
+
+static int hclge_mdio_write(struct mii_bus *bus, int phy_id, int regnum,
+   u16 data)
+{
+   struct hclge_dev *hdev = (struct hclge_dev *)bus->priv;
+   struct hclge_mdio_cfg_cmd *mdio_cmd;
+   enum hclge_cmd_status status;
+   struct hclge_desc desc;
+   u8 is_c45, devad;
+   u16 reg;
+
+   if (!bus)
+   return -EINVAL;
+
+   is_c45 = !!(regnum & MII_ADDR_C45);
+   devad = ((regnum >> 16) & 0x1f);
+   reg = (u16)(regnum & 0x);
+
+   hclge_cmd_setup_basic_desc(, HCLGE_OPC_MDIO_CONFIG, false);
+
+   mdio_cmd = (struct hclge_mdio_cfg_cmd *)desc.data;
+
+   if (!is_c45) {
+   /* C22 write reg and data */
+   mdio_cmd->ctrl_bit = HCLGE_MDIO_IS_C22(!is_c45);
+   mdio_cmd->ctrl_bit |= HCLGE_MDIO_CTRL_OP(HCLGE_MDIO_C22_WRITE);
+   mdio_cmd->ctrl_bit |= HCLGE_MDIO_CTRL_START_BIT;
+   mdio_cmd->data_wr = cpu_to_le16(data);
+   mdio_cmd->devad = devad & HCLGE_MDIO_CTRL_DEVAD_MSK;
+   mdio_cmd->prtad = phy_id & HCLGE_MDIO_CTRL_PRTAD_MSK;
+   } else {
+   /* Set phy addr */
+   mdio_cmd->ctrl_bit |= HCLGE_MDIO_CTRL_START_BIT;
+   mdio_cmd->addr_c45 = cpu_to_le16(reg);
+   mdio_cmd->data_wr = cpu_to_le16(data);
+   mdio_cmd->devad = devad & HCLGE_MDIO_CTRL_DEVAD_MSK;
+   mdio_cmd->prtad = phy_id & HCLGE_MDIO_CTRL_PRTAD_MSK;
+   }
+
+   status = hclge_cmd_send(>hw, , 1);
+   if (status) {
+   dev_err(>pdev->dev,
+   "mdio write fail when sending cmd, status is %d.\n",
+   status);
+   return -EIO;
+   }
+
+   return 0;
+}
+
+static int hclge_mdio_read(struct mii_bus *bus, int phy_id, int regnum)
+{
+   struct hclge_dev *hdev = (struct hclge_dev *)bus->priv;
+   struct hclge_mdio_cfg_cmd *mdio_cmd;
+   enum hclge_cmd_status status;
+   struct hclge_desc desc;
+   u8 is_c45, devad;
+   u16 reg;
+
+   if (!bus)
+   return -EINVAL;
+
+   is_c45 = !!(regnum & MII_ADDR_C45);
+   devad = ((regnum >> 16) & GENMASK(4, 0));
+   reg = (u16)(regnum & GENMASK(15, 0));
+
+   hclge_cmd_setup_basic_desc(, HCLGE_OPC_MDIO_CONFIG, true);
+
+   mdio_cmd = (struct hclge_mdio_cfg_cmd *)desc.data;
+
+   dev_dbg(>dev, "phy id=%d, is_c45=%d, devad=%d, reg=%#x!\n",
+   phy_id, is_c45, devad, reg);
+
+   if (!is_c45) {
+   /* C22 read reg */
+

[PATCH net-next 6/9] net: hns3: Add MDIO support to HNS3 Ethernet driver for hip08 SoC

2017-06-09 Thread Salil Mehta 
This patch adds the support of MDIO bus interface for HNS3 driver.
Code provides various interfaces to start and stop the PHY layer
and to read and write the MDIO bus or PHY.

Signed-off-by: Daode Huang 
Signed-off-by: lipeng 
Signed-off-by: Salil Mehta 
Signed-off-by: Yisen Zhuang 
---
 .../ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c| 310 +
 1 file changed, 310 insertions(+)
 create mode 100644 drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c 
b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c
new file mode 100644
index 000..c6812d2
--- /dev/null
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c
@@ -0,0 +1,310 @@
+/*
+ * Copyright (c) 2016~2017 Hisilicon Limited.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include 
+#include 
+
+#include "hclge_cmd.h"
+#include "hclge_main.h"
+
+enum hclge_mdio_c22_op_seq {
+   HCLGE_MDIO_C22_WRITE = 1,
+   HCLGE_MDIO_C22_READ = 2
+};
+
+enum hclge_mdio_c45_op_seq {
+   HCLGE_MDIO_C45_WRITE_ADDR = 0,
+   HCLGE_MDIO_C45_WRITE_DATA,
+   HCLGE_MDIO_C45_READ_INCREMENT,
+   HCLGE_MDIO_C45_READ
+};
+
+#define HCLGE_MDIO_CTRL_START_BIT   BIT(0)
+#define HCLGE_MDIO_CTRL_ST_MSK  GENMASK(2, 1)
+#define HCLGE_MDIO_CTRL_ST_LSH  1
+#define HCLGE_MDIO_IS_C22(c22)  (((c22) << HCLGE_MDIO_CTRL_ST_LSH) & \
+   HCLGE_MDIO_CTRL_ST_MSK)
+
+#define HCLGE_MDIO_CTRL_OP_MSK  GENMASK(4, 3)
+#define HCLGE_MDIO_CTRL_OP_LSH  3
+#define HCLGE_MDIO_CTRL_OP(access) \
+   (((access) << HCLGE_MDIO_CTRL_OP_LSH) & HCLGE_MDIO_CTRL_OP_MSK)
+#define HCLGE_MDIO_CTRL_PRTAD_MSK   GENMASK(4, 0)
+#define HCLGE_MDIO_CTRL_DEVAD_MSK   GENMASK(4, 0)
+
+#define HCLGE_MDIO_STA_VAL(val)((val) & BIT(0))
+
+struct hclge_mdio_cfg_cmd {
+   u8 ctrl_bit;
+   u8 prtad;   /* The external port address */
+   u8 devad;   /* The external device address */
+   u8 rsvd;
+   __le16 addr_c45;/* Only valid for c45 */
+   __le16 data_wr;
+   __le16 data_rd;
+   __le16 sta;
+};
+
+static int hclge_mdio_write(struct mii_bus *bus, int phy_id, int regnum,
+   u16 data)
+{
+   struct hclge_dev *hdev = (struct hclge_dev *)bus->priv;
+   struct hclge_mdio_cfg_cmd *mdio_cmd;
+   enum hclge_cmd_status status;
+   struct hclge_desc desc;
+   u8 is_c45, devad;
+   u16 reg;
+
+   if (!bus)
+   return -EINVAL;
+
+   is_c45 = !!(regnum & MII_ADDR_C45);
+   devad = ((regnum >> 16) & 0x1f);
+   reg = (u16)(regnum & 0x);
+
+   hclge_cmd_setup_basic_desc(, HCLGE_OPC_MDIO_CONFIG, false);
+
+   mdio_cmd = (struct hclge_mdio_cfg_cmd *)desc.data;
+
+   if (!is_c45) {
+   /* C22 write reg and data */
+   mdio_cmd->ctrl_bit = HCLGE_MDIO_IS_C22(!is_c45);
+   mdio_cmd->ctrl_bit |= HCLGE_MDIO_CTRL_OP(HCLGE_MDIO_C22_WRITE);
+   mdio_cmd->ctrl_bit |= HCLGE_MDIO_CTRL_START_BIT;
+   mdio_cmd->data_wr = cpu_to_le16(data);
+   mdio_cmd->devad = devad & HCLGE_MDIO_CTRL_DEVAD_MSK;
+   mdio_cmd->prtad = phy_id & HCLGE_MDIO_CTRL_PRTAD_MSK;
+   } else {
+   /* Set phy addr */
+   mdio_cmd->ctrl_bit |= HCLGE_MDIO_CTRL_START_BIT;
+   mdio_cmd->addr_c45 = cpu_to_le16(reg);
+   mdio_cmd->data_wr = cpu_to_le16(data);
+   mdio_cmd->devad = devad & HCLGE_MDIO_CTRL_DEVAD_MSK;
+   mdio_cmd->prtad = phy_id & HCLGE_MDIO_CTRL_PRTAD_MSK;
+   }
+
+   status = hclge_cmd_send(>hw, , 1);
+   if (status) {
+   dev_err(>pdev->dev,
+   "mdio write fail when sending cmd, status is %d.\n",
+   status);
+   return -EIO;
+   }
+
+   return 0;
+}
+
+static int hclge_mdio_read(struct mii_bus *bus, int phy_id, int regnum)
+{
+   struct hclge_dev *hdev = (struct hclge_dev *)bus->priv;
+   struct hclge_mdio_cfg_cmd *mdio_cmd;
+   enum hclge_cmd_status status;
+   struct hclge_desc desc;
+   u8 is_c45, devad;
+   u16 reg;
+
+   if (!bus)
+   return -EINVAL;
+
+   is_c45 = !!(regnum & MII_ADDR_C45);
+   devad = ((regnum >> 16) & GENMASK(4, 0));
+   reg = (u16)(regnum & GENMASK(15, 0));
+
+   hclge_cmd_setup_basic_desc(, HCLGE_OPC_MDIO_CONFIG, true);
+
+   mdio_cmd = (struct hclge_mdio_cfg_cmd *)desc.data;
+
+   dev_dbg(>dev, "phy id=%d, is_c45=%d, devad=%d, reg=%#x!\n",
+   phy_id, is_c45, devad, reg);
+
+   if (!is_c45) {
+   /* C22 read reg */
+   mdio_cmd->ctrl_bit = HCLGE_MDIO_IS_C22(!is_c45);
+   mdio_cmd->ctrl_bit |=