diff mbox series

[v5,3/7] hw/mdio: Generalize phy initialization routine

Message ID 20170922171323.10348-4-f4bug@amsat.org
State New
Headers show
Series [v5,1/7] hw/mdio: Generalize etraxfs MDIO bitbanging emulation | expand

Commit Message

Philippe Mathieu-Daudé Sept. 22, 2017, 5:13 p.m. UTC
From: Grant Likely <grant.likely@arm.com>


There really isn't anything tdk-specific about tdk_init() other than the
phy id registers. The function should instead be generalized for any
phy, at least as far as the ID registers are concerned. For the most
part the read/write behaviour should be very similar across PHYs.

This patch renames tdk_{read,write,init}() to mdio_phy_*() so it can be
used for any PHY.

More work definitely needs to be done here to make it easy to override
the default behaviour for specific PHYs, but this at least is a
reasonable start.

Signed-off-by: Grant Likely <grant.likely@arm.com>

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

[PMD: just rebased]
---
 include/hw/net/mdio.h   |  2 +-
 hw/net/etraxfs_eth.c    |  2 +-
 hw/net/mdio.c           | 14 +++++++-------
 hw/net/xilinx_axienet.c |  2 +-
 4 files changed, 10 insertions(+), 10 deletions(-)

-- 
2.14.1

Comments

Alistair Francis Feb. 27, 2018, 10:33 p.m. UTC | #1
On Fri, Sep 22, 2017 at 10:13 AM, Philippe Mathieu-Daudé
<f4bug@amsat.org> wrote:
> From: Grant Likely <grant.likely@arm.com>

>

> There really isn't anything tdk-specific about tdk_init() other than the

> phy id registers. The function should instead be generalized for any

> phy, at least as far as the ID registers are concerned. For the most

> part the read/write behaviour should be very similar across PHYs.

>

> This patch renames tdk_{read,write,init}() to mdio_phy_*() so it can be

> used for any PHY.

>

> More work definitely needs to be done here to make it easy to override

> the default behaviour for specific PHYs, but this at least is a

> reasonable start.

>

> Signed-off-by: Grant Likely <grant.likely@arm.com>

> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> [PMD: just rebased]

> ---

>  include/hw/net/mdio.h   |  2 +-

>  hw/net/etraxfs_eth.c    |  2 +-

>  hw/net/mdio.c           | 14 +++++++-------

>  hw/net/xilinx_axienet.c |  2 +-

>  4 files changed, 10 insertions(+), 10 deletions(-)

>

> diff --git a/include/hw/net/mdio.h b/include/hw/net/mdio.h

> index 7ffa4389b9..b3b4f497c0 100644

> --- a/include/hw/net/mdio.h

> +++ b/include/hw/net/mdio.h

> @@ -86,7 +86,7 @@ struct qemu_mdio {

>      struct qemu_phy *devs[32];

>  };

>

> -void tdk_init(struct qemu_phy *phy);

> +void mdio_phy_init(struct qemu_phy *phy, uint16_t id1, uint16_t id2);

>  void mdio_attach(struct qemu_mdio *bus, struct qemu_phy *phy,

>                   unsigned int addr);

>  uint16_t mdio_read_req(struct qemu_mdio *bus, uint8_t addr, uint8_t req);

> diff --git a/hw/net/etraxfs_eth.c b/hw/net/etraxfs_eth.c

> index f8d8f8441d..4c5415771f 100644

> --- a/hw/net/etraxfs_eth.c

> +++ b/hw/net/etraxfs_eth.c

> @@ -333,7 +333,7 @@ static int fs_eth_init(SysBusDevice *sbd)

>      qemu_format_nic_info_str(qemu_get_queue(s->nic), s->conf.macaddr.a);

>

>

> -    tdk_init(&s->phy);

> +    mdio_phy_init(&s->phy, 0x0300, 0xe400);

>      mdio_attach(&s->mdio_bus, &s->phy, s->phyaddr);

>      return 0;

>  }

> diff --git a/hw/net/mdio.c b/hw/net/mdio.c

> index 3d70d99077..33bfbb4623 100644

> --- a/hw/net/mdio.c

> +++ b/hw/net/mdio.c

> @@ -43,7 +43,7 @@

>   * linux driver (PHYID and Diagnostics reg).

>   * TODO: Add friendly names for the register nums.

>   */

> -static unsigned int tdk_read(struct qemu_phy *phy, unsigned int req)

> +static unsigned int mdio_phy_read(struct qemu_phy *phy, unsigned int req)

>  {

>      int regnum;

>      unsigned r = 0;

> @@ -107,7 +107,7 @@ static unsigned int tdk_read(struct qemu_phy *phy, unsigned int req)

>      return r;

>  }

>

> -static void tdk_write(struct qemu_phy *phy, unsigned int req, unsigned int data)

> +static void mdio_phy_write(struct qemu_phy *phy, unsigned int req, unsigned int data)

>  {

>      int regnum;

>

> @@ -120,18 +120,18 @@ static void tdk_write(struct qemu_phy *phy, unsigned int req, unsigned int data)

>      }

>  }

>

> -void tdk_init(struct qemu_phy *phy)

> +void mdio_phy_init(struct qemu_phy *phy, uint16_t id1, uint16_t id2)

>  {

>      phy->regs[PHY_CTRL] = 0x3100;

>      /* PHY Id. */

> -    phy->regs[PHY_ID1] = 0x0300;

> -    phy->regs[PHY_ID2] = 0xe400;

> +    phy->regs[PHY_ID1] = id1;

> +    phy->regs[PHY_ID2] = id2;


These should be set by QEMU properties instead of values to the init() function.

Alistair

>      /* Autonegotiation advertisement reg. */

>      phy->regs[PHY_AUTONEG_ADV] = 0x01e1;

>      phy->link = 1;

>

> -    phy->read = tdk_read;

> -    phy->write = tdk_write;

> +    phy->read = mdio_phy_read;

> +    phy->write = mdio_phy_write;

>  }

>

>  void mdio_attach(struct qemu_mdio *bus, struct qemu_phy *phy, unsigned int addr)

> diff --git a/hw/net/xilinx_axienet.c b/hw/net/xilinx_axienet.c

> index 1e859fdaae..408cd6e675 100644

> --- a/hw/net/xilinx_axienet.c

> +++ b/hw/net/xilinx_axienet.c

> @@ -791,7 +791,7 @@ static void xilinx_enet_realize(DeviceState *dev, Error **errp)

>                            object_get_typename(OBJECT(dev)), dev->id, s);

>      qemu_format_nic_info_str(qemu_get_queue(s->nic), s->conf.macaddr.a);

>

> -    tdk_init(&s->TEMAC.phy);

> +    mdio_phy_init(&s->TEMAC.phy, 0x0300, 0xe400);

>      mdio_attach(&s->TEMAC.mdio_bus, &s->TEMAC.phy, s->c_phyaddr);

>

>      s->TEMAC.parent = s;

> --

> 2.14.1

>

>
Philippe Mathieu-Daudé May 28, 2018, 3:09 a.m. UTC | #2
Hi Alistair,

On 02/27/2018 07:33 PM, Alistair Francis wrote:
> On Fri, Sep 22, 2017 at 10:13 AM, Philippe Mathieu-Daudé

> <f4bug@amsat.org> wrote:

>> From: Grant Likely <grant.likely@arm.com>

>>

>> There really isn't anything tdk-specific about tdk_init() other than the

>> phy id registers. The function should instead be generalized for any

>> phy, at least as far as the ID registers are concerned. For the most

>> part the read/write behaviour should be very similar across PHYs.

>>

>> This patch renames tdk_{read,write,init}() to mdio_phy_*() so it can be

>> used for any PHY.

>>

>> More work definitely needs to be done here to make it easy to override

>> the default behaviour for specific PHYs, but this at least is a

>> reasonable start.

>>

>> Signed-off-by: Grant Likely <grant.likely@arm.com>

>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

>> [PMD: just rebased]

>> ---

>>  include/hw/net/mdio.h   |  2 +-

>>  hw/net/etraxfs_eth.c    |  2 +-

>>  hw/net/mdio.c           | 14 +++++++-------

>>  hw/net/xilinx_axienet.c |  2 +-

>>  4 files changed, 10 insertions(+), 10 deletions(-)

>>

>> diff --git a/include/hw/net/mdio.h b/include/hw/net/mdio.h

>> index 7ffa4389b9..b3b4f497c0 100644

>> --- a/include/hw/net/mdio.h

>> +++ b/include/hw/net/mdio.h

>> @@ -86,7 +86,7 @@ struct qemu_mdio {

>>      struct qemu_phy *devs[32];

>>  };

>>

>> -void tdk_init(struct qemu_phy *phy);

>> +void mdio_phy_init(struct qemu_phy *phy, uint16_t id1, uint16_t id2);

>>  void mdio_attach(struct qemu_mdio *bus, struct qemu_phy *phy,

>>                   unsigned int addr);

>>  uint16_t mdio_read_req(struct qemu_mdio *bus, uint8_t addr, uint8_t req);

>> diff --git a/hw/net/etraxfs_eth.c b/hw/net/etraxfs_eth.c

>> index f8d8f8441d..4c5415771f 100644

>> --- a/hw/net/etraxfs_eth.c

>> +++ b/hw/net/etraxfs_eth.c

>> @@ -333,7 +333,7 @@ static int fs_eth_init(SysBusDevice *sbd)

>>      qemu_format_nic_info_str(qemu_get_queue(s->nic), s->conf.macaddr.a);

>>

>>

>> -    tdk_init(&s->phy);

>> +    mdio_phy_init(&s->phy, 0x0300, 0xe400);

>>      mdio_attach(&s->mdio_bus, &s->phy, s->phyaddr);

>>      return 0;

>>  }

>> diff --git a/hw/net/mdio.c b/hw/net/mdio.c

>> index 3d70d99077..33bfbb4623 100644

>> --- a/hw/net/mdio.c

>> +++ b/hw/net/mdio.c

>> @@ -43,7 +43,7 @@

>>   * linux driver (PHYID and Diagnostics reg).

>>   * TODO: Add friendly names for the register nums.

>>   */

>> -static unsigned int tdk_read(struct qemu_phy *phy, unsigned int req)

>> +static unsigned int mdio_phy_read(struct qemu_phy *phy, unsigned int req)

>>  {

>>      int regnum;

>>      unsigned r = 0;

>> @@ -107,7 +107,7 @@ static unsigned int tdk_read(struct qemu_phy *phy, unsigned int req)

>>      return r;

>>  }

>>

>> -static void tdk_write(struct qemu_phy *phy, unsigned int req, unsigned int data)

>> +static void mdio_phy_write(struct qemu_phy *phy, unsigned int req, unsigned int data)

>>  {

>>      int regnum;

>>

>> @@ -120,18 +120,18 @@ static void tdk_write(struct qemu_phy *phy, unsigned int req, unsigned int data)

>>      }

>>  }

>>

>> -void tdk_init(struct qemu_phy *phy)

>> +void mdio_phy_init(struct qemu_phy *phy, uint16_t id1, uint16_t id2)

>>  {

>>      phy->regs[PHY_CTRL] = 0x3100;

>>      /* PHY Id. */

>> -    phy->regs[PHY_ID1] = 0x0300;

>> -    phy->regs[PHY_ID2] = 0xe400;

>> +    phy->regs[PHY_ID1] = id1;

>> +    phy->regs[PHY_ID2] = id2;

> 

> These should be set by QEMU properties instead of values to the init() function.


I agree, but this is not (yet) a QOM'ified device.
I believe if I start to QOM'ify it this series will finish in 1 more
year. This is however a nice cleanup so far, and the QOM device can come
in a follow up series.

> 

> Alistair

> 

>>      /* Autonegotiation advertisement reg. */

>>      phy->regs[PHY_AUTONEG_ADV] = 0x01e1;

>>      phy->link = 1;

>>

>> -    phy->read = tdk_read;

>> -    phy->write = tdk_write;

>> +    phy->read = mdio_phy_read;

>> +    phy->write = mdio_phy_write;

>>  }

>>

>>  void mdio_attach(struct qemu_mdio *bus, struct qemu_phy *phy, unsigned int addr)

>> diff --git a/hw/net/xilinx_axienet.c b/hw/net/xilinx_axienet.c

>> index 1e859fdaae..408cd6e675 100644

>> --- a/hw/net/xilinx_axienet.c

>> +++ b/hw/net/xilinx_axienet.c

>> @@ -791,7 +791,7 @@ static void xilinx_enet_realize(DeviceState *dev, Error **errp)

>>                            object_get_typename(OBJECT(dev)), dev->id, s);

>>      qemu_format_nic_info_str(qemu_get_queue(s->nic), s->conf.macaddr.a);

>>

>> -    tdk_init(&s->TEMAC.phy);

>> +    mdio_phy_init(&s->TEMAC.phy, 0x0300, 0xe400);

>>      mdio_attach(&s->TEMAC.mdio_bus, &s->TEMAC.phy, s->c_phyaddr);

>>

>>      s->TEMAC.parent = s;

>> --

>> 2.14.1

>>

>>
diff mbox series

Patch

diff --git a/include/hw/net/mdio.h b/include/hw/net/mdio.h
index 7ffa4389b9..b3b4f497c0 100644
--- a/include/hw/net/mdio.h
+++ b/include/hw/net/mdio.h
@@ -86,7 +86,7 @@  struct qemu_mdio {
     struct qemu_phy *devs[32];
 };
 
-void tdk_init(struct qemu_phy *phy);
+void mdio_phy_init(struct qemu_phy *phy, uint16_t id1, uint16_t id2);
 void mdio_attach(struct qemu_mdio *bus, struct qemu_phy *phy,
                  unsigned int addr);
 uint16_t mdio_read_req(struct qemu_mdio *bus, uint8_t addr, uint8_t req);
diff --git a/hw/net/etraxfs_eth.c b/hw/net/etraxfs_eth.c
index f8d8f8441d..4c5415771f 100644
--- a/hw/net/etraxfs_eth.c
+++ b/hw/net/etraxfs_eth.c
@@ -333,7 +333,7 @@  static int fs_eth_init(SysBusDevice *sbd)
     qemu_format_nic_info_str(qemu_get_queue(s->nic), s->conf.macaddr.a);
 
 
-    tdk_init(&s->phy);
+    mdio_phy_init(&s->phy, 0x0300, 0xe400);
     mdio_attach(&s->mdio_bus, &s->phy, s->phyaddr);
     return 0;
 }
diff --git a/hw/net/mdio.c b/hw/net/mdio.c
index 3d70d99077..33bfbb4623 100644
--- a/hw/net/mdio.c
+++ b/hw/net/mdio.c
@@ -43,7 +43,7 @@ 
  * linux driver (PHYID and Diagnostics reg).
  * TODO: Add friendly names for the register nums.
  */
-static unsigned int tdk_read(struct qemu_phy *phy, unsigned int req)
+static unsigned int mdio_phy_read(struct qemu_phy *phy, unsigned int req)
 {
     int regnum;
     unsigned r = 0;
@@ -107,7 +107,7 @@  static unsigned int tdk_read(struct qemu_phy *phy, unsigned int req)
     return r;
 }
 
-static void tdk_write(struct qemu_phy *phy, unsigned int req, unsigned int data)
+static void mdio_phy_write(struct qemu_phy *phy, unsigned int req, unsigned int data)
 {
     int regnum;
 
@@ -120,18 +120,18 @@  static void tdk_write(struct qemu_phy *phy, unsigned int req, unsigned int data)
     }
 }
 
-void tdk_init(struct qemu_phy *phy)
+void mdio_phy_init(struct qemu_phy *phy, uint16_t id1, uint16_t id2)
 {
     phy->regs[PHY_CTRL] = 0x3100;
     /* PHY Id. */
-    phy->regs[PHY_ID1] = 0x0300;
-    phy->regs[PHY_ID2] = 0xe400;
+    phy->regs[PHY_ID1] = id1;
+    phy->regs[PHY_ID2] = id2;
     /* Autonegotiation advertisement reg. */
     phy->regs[PHY_AUTONEG_ADV] = 0x01e1;
     phy->link = 1;
 
-    phy->read = tdk_read;
-    phy->write = tdk_write;
+    phy->read = mdio_phy_read;
+    phy->write = mdio_phy_write;
 }
 
 void mdio_attach(struct qemu_mdio *bus, struct qemu_phy *phy, unsigned int addr)
diff --git a/hw/net/xilinx_axienet.c b/hw/net/xilinx_axienet.c
index 1e859fdaae..408cd6e675 100644
--- a/hw/net/xilinx_axienet.c
+++ b/hw/net/xilinx_axienet.c
@@ -791,7 +791,7 @@  static void xilinx_enet_realize(DeviceState *dev, Error **errp)
                           object_get_typename(OBJECT(dev)), dev->id, s);
     qemu_format_nic_info_str(qemu_get_queue(s->nic), s->conf.macaddr.a);
 
-    tdk_init(&s->TEMAC.phy);
+    mdio_phy_init(&s->TEMAC.phy, 0x0300, 0xe400);
     mdio_attach(&s->TEMAC.mdio_bus, &s->TEMAC.phy, s->c_phyaddr);
 
     s->TEMAC.parent = s;