[{"id":1865294,"web_url":"http://patchwork.ozlabs.org/comment/1865294/","msgid":"","list_archive_url":null,"date":"2018-02-27T22:40:56","subject":"Re: [Qemu-devel] [PATCH v5 5/7] hw/mdio: Refactor bitbanging state\n\tmachine","submitter":{"id":64571,"url":"http://patchwork.ozlabs.org/api/people/64571/","name":"Alistair Francis","email":"alistair23@gmail.com"},"content":"On Fri, Sep 22, 2017 at 10:13 AM, Philippe Mathieu-Daudé\n wrote:\n> From: Grant Likely \n>\n> The MDIO state machine has a moderate amount of duplicate code in the\n> state processing that can be consolidated. This patch does so and\n> reorganizes it a bit so that far less code is required. Most of the\n> states simply stream a fixed number of bits in as a single integer and\n> can be handled by a common processing function that checks for\n> completion of the state and returns the streamed in value.\n>\n> Changes include:\n> - Move clock state change tracking into core code\n> - Use a common shift register for clocking data in and out\n> - Create separate mdc & mdio accessor functions\n> - will be replaced with GPIO connection in a follow-on patch\n>\n> Signed-off-by: Grant Likely \n> Signed-off-by: Philippe Mathieu-Daudé \n> [PMD: just rebased]\n\nAcked-by: Alistair Francis \n\nAlistair\n\n> ---\n> include/hw/net/mdio.h | 41 ++++++++-------\n> hw/net/etraxfs_eth.c | 11 ++--\n> hw/net/mdio.c | 140 ++++++++++++++++++++++----------------------------\n> 3 files changed, 87 insertions(+), 105 deletions(-)\n>\n> diff --git a/include/hw/net/mdio.h b/include/hw/net/mdio.h\n> index ed1879a728..7fca19784e 100644\n> --- a/include/hw/net/mdio.h\n> +++ b/include/hw/net/mdio.h\n> @@ -52,37 +52,33 @@\n> #define PHY_ADVERTISE_100FULL 0x0100 /* Try for 100mbps full-duplex */\n>\n> struct qemu_phy {\n> - uint32_t regs[NUM_PHY_REGS];\n> + uint16_t regs[NUM_PHY_REGS];\n> const uint16_t *regs_readonly_mask; /* 0=writable, 1=read-only */\n>\n> - int link;\n> + bool link;\n>\n> - unsigned int (*read)(struct qemu_phy *phy, unsigned int req);\n> - void (*write)(struct qemu_phy *phy, unsigned int req, unsigned int data);\n> + uint16_t (*read)(struct qemu_phy *phy, unsigned int req);\n> + void (*write)(struct qemu_phy *phy, unsigned int req, uint16_t data);\n> };\n>\n> struct qemu_mdio {\n> - /* bus. */\n> - int mdc;\n> - int mdio;\n> -\n> - /* decoder. */\n> + /* bitbanging state machine */\n> + bool mdc;\n> + bool mdio;\n> enum {\n> PREAMBLE,\n> - SOF,\n> OPC,\n> ADDR,\n> REQ,\n> TURNAROUND,\n> DATA\n> } state;\n> - unsigned int drive;\n>\n> - unsigned int cnt;\n> - unsigned int addr;\n> - unsigned int opc;\n> - unsigned int req;\n> - unsigned int data;\n> + uint16_t cnt; /* Bit count for current state */\n> + uint16_t addr; /* PHY Address; retrieved during ADDR state */\n> + uint16_t opc; /* Operation; 2:read */\n> + uint16_t req; /* Register address */\n> + uint32_t shiftreg; /* shift register; bits in to or out from PHY */\n>\n> struct qemu_phy *devs[32];\n> };\n> @@ -91,7 +87,16 @@ void mdio_phy_init(struct qemu_phy *phy, uint16_t id1, uint16_t id2);\n> void mdio_attach(struct qemu_mdio *bus, struct qemu_phy *phy,\n> unsigned int addr);\n> uint16_t mdio_read_req(struct qemu_mdio *bus, uint8_t addr, uint8_t req);\n> -void mdio_write_req(struct qemu_mdio *bus, uint8_t addr, uint8_t req, uint16_t data);\n> -void mdio_cycle(struct qemu_mdio *bus);\n> +void mdio_write_req(struct qemu_mdio *bus, uint8_t addr, uint8_t req,\n> + uint16_t data);\n> +void mdio_bitbang_set_clk(struct qemu_mdio *bus, bool mdc);\n> +static inline void mdio_bitbang_set_data(struct qemu_mdio *bus, bool mdio)\n> +{\n> + bus->mdio = mdio;\n> +}\n> +static inline bool mdio_bitbang_get_data(struct qemu_mdio *bus)\n> +{\n> + return bus->mdio;\n> +}\n>\n> #endif\n> diff --git a/hw/net/etraxfs_eth.c b/hw/net/etraxfs_eth.c\n> index 4c5415771f..1b518ea16e 100644\n> --- a/hw/net/etraxfs_eth.c\n> +++ b/hw/net/etraxfs_eth.c\n> @@ -119,7 +119,7 @@ eth_read(void *opaque, hwaddr addr, unsigned int size)\n>\n> switch (addr) {\n> case R_STAT:\n> - r = eth->mdio_bus.mdio & 1;\n> + r = mdio_bitbang_get_data(ð->mdio_bus);\n> break;\n> default:\n> r = eth->regs[addr];\n> @@ -177,13 +177,10 @@ eth_write(void *opaque, hwaddr addr,\n> case RW_MGM_CTRL:\n> /* Attach an MDIO/PHY abstraction. */\n> if (value & 2) {\n> - eth->mdio_bus.mdio = value & 1;\n> + mdio_bitbang_set_data(ð->mdio_bus, value & 1);\n> }\n> - if (eth->mdio_bus.mdc != (value & 4)) {\n> - mdio_cycle(ð->mdio_bus);\n> - eth_validate_duplex(eth);\n> - }\n> - eth->mdio_bus.mdc = !!(value & 4);\n> + mdio_bitbang_set_clk(ð->mdio_bus, value & 4);\n> + eth_validate_duplex(eth);\n> eth->regs[addr] = value;\n> break;\n>\n> diff --git a/hw/net/mdio.c b/hw/net/mdio.c\n> index 89a6a3a590..96e10fada0 100644\n> --- a/hw/net/mdio.c\n> +++ b/hw/net/mdio.c\n> @@ -43,7 +43,7 @@\n> * linux driver (PHYID and Diagnostics reg).\n> * TODO: Add friendly names for the register nums.\n> */\n> -static unsigned int mdio_phy_read(struct qemu_phy *phy, unsigned int req)\n> +static uint16_t mdio_phy_read(struct qemu_phy *phy, unsigned int req)\n> {\n> int regnum;\n> unsigned r = 0;\n> @@ -107,7 +107,8 @@ static unsigned int mdio_phy_read(struct qemu_phy *phy, unsigned int req)\n> return r;\n> }\n>\n> -static void mdio_phy_write(struct qemu_phy *phy, unsigned int req, unsigned int data)\n> +static void mdio_phy_write(struct qemu_phy *phy, unsigned int req,\n> + uint16_t data)\n> {\n> int regnum = req & 0x1f;\n> uint16_t mask = phy->regs_readonly_mask[regnum];\n> @@ -136,13 +137,14 @@ void mdio_phy_init(struct qemu_phy *phy, uint16_t id1, uint16_t id2)\n> /* Autonegotiation advertisement reg. */\n> phy->regs[PHY_AUTONEG_ADV] = 0x01e1;\n> phy->regs_readonly_mask = default_readonly_mask;\n> - phy->link = 1;\n> + phy->link = true;\n>\n> phy->read = mdio_phy_read;\n> phy->write = mdio_phy_write;\n> }\n>\n> -void mdio_attach(struct qemu_mdio *bus, struct qemu_phy *phy, unsigned int addr)\n> +void mdio_attach(struct qemu_mdio *bus, struct qemu_phy *phy,\n> + unsigned int addr)\n> {\n> bus->devs[addr & 0x1f] = phy;\n> }\n> @@ -169,99 +171,77 @@ void mdio_write_req(struct qemu_mdio *bus, uint8_t addr, uint8_t req,\n> }\n> }\n>\n> -void mdio_cycle(struct qemu_mdio *bus)\n> +/**\n> + * mdio_bitbang_update() - internal function to check how many clocks have\n> + * passed and move to the next state if necessary. Returns TRUE on state change.\n> + */\n> +static bool mdio_bitbang_update(struct qemu_mdio *bus, int num_bits, int next,\n> + uint16_t *reg)\n> {\n> + if (bus->cnt < num_bits) {\n> + return false;\n> + }\n> + if (reg) {\n> + *reg = bus->shiftreg;\n> + }\n> + bus->state = next;\n> + bus->cnt = 0;\n> + bus->shiftreg = 0;\n> + return true;\n> +}\n> +\n> +/**\n> + * mdio_bitbang_set_clk() - set value of mdc signal and update state\n> + */\n> +void mdio_bitbang_set_clk(struct qemu_mdio *bus, bool mdc)\n> +{\n> + uint16_t tmp;\n> +\n> + if (mdc == bus->mdc) {\n> + return; /* Clock state hasn't changed; do nothing */\n> + }\n> +\n> + bus->mdc = mdc;\n> + if (bus->mdc) {\n> + /* Falling (inactive) clock edge */\n> + if ((bus->state == DATA) && (bus->opc == 2)) {\n> + bus->mdio = !!(bus->shiftreg & 0x8000);\n> + }\n> + return;\n> + }\n> +\n> + /* Rising clock Edge */\n> + bus->shiftreg = (bus->shiftreg << 1) | bus->mdio;\n> bus->cnt++;\n> -\n> D(printf(\"mdc=%d mdio=%d state=%d cnt=%d drv=%d\\n\",\n> - bus->mdc, bus->mdio, bus->state, bus->cnt, bus->drive));\n> + bus->mdc, bus->mdio, bus->state, bus->cnt));\n> switch (bus->state) {\n> case PREAMBLE:\n> - if (bus->mdc) {\n> - if (bus->cnt >= (32 * 2) && !bus->mdio) {\n> - bus->cnt = 0;\n> - bus->state = SOF;\n> - bus->data = 0;\n> - }\n> - }\n> - break;\n> - case SOF:\n> - if (bus->mdc) {\n> - if (bus->mdio != 1) {\n> - printf(\"WARNING: no SOF\\n\");\n> - }\n> - if (bus->cnt == 1 * 2) {\n> - bus->cnt = 0;\n> - bus->opc = 0;\n> - bus->state = OPC;\n> - }\n> + /* MDIO must be 30 clocks high, 1 low, and 1 high to get out of\n> + preamble */\n> + if (bus->shiftreg == 0xfffffffd) {\n> + mdio_bitbang_update(bus, 0, OPC, NULL);\n> }\n> break;\n> case OPC:\n> - if (bus->mdc) {\n> - bus->opc <<= 1;\n> - bus->opc |= bus->mdio & 1;\n> - if (bus->cnt == 2 * 2) {\n> - bus->cnt = 0;\n> - bus->addr = 0;\n> - bus->state = ADDR;\n> - }\n> - }\n> + mdio_bitbang_update(bus, 2, ADDR, &bus->opc);\n> break;\n> case ADDR:\n> - if (bus->mdc) {\n> - bus->addr <<= 1;\n> - bus->addr |= bus->mdio & 1;\n> -\n> - if (bus->cnt == 5 * 2) {\n> - bus->cnt = 0;\n> - bus->req = 0;\n> - bus->state = REQ;\n> - }\n> - }\n> + mdio_bitbang_update(bus, 5, REQ, &bus->addr);\n> break;\n> case REQ:\n> - if (bus->mdc) {\n> - bus->req <<= 1;\n> - bus->req |= bus->mdio & 1;\n> - if (bus->cnt == 5 * 2) {\n> - bus->cnt = 0;\n> - bus->state = TURNAROUND;\n> - }\n> - }\n> + mdio_bitbang_update(bus, 5, TURNAROUND, &bus->req);\n> break;\n> case TURNAROUND:\n> - if (bus->mdc && bus->cnt == 2 * 2) {\n> - bus->mdio = 0;\n> - bus->cnt = 0;\n> -\n> - if (bus->opc == 2) {\n> - bus->drive = 1;\n> - bus->data = mdio_read_req(bus, bus->addr, bus->req);\n> - bus->mdio = bus->data & 1;\n> - }\n> - bus->state = DATA;\n> + /* If beginning of DATA READ cycle, then read PHY into shift register */\n> + if (mdio_bitbang_update(bus, 2, DATA, NULL) && (bus->opc == 2)) {\n> + bus->shiftreg = mdio_read_req(bus, bus->addr, bus->req);\n> }\n> break;\n> case DATA:\n> - if (!bus->mdc) {\n> - if (bus->drive) {\n> - bus->mdio = !!(bus->data & (1 << 15));\n> - bus->data <<= 1;\n> - }\n> - } else {\n> - if (!bus->drive) {\n> - bus->data <<= 1;\n> - bus->data |= bus->mdio;\n> - }\n> - if (bus->cnt == 16 * 2) {\n> - bus->cnt = 0;\n> - bus->state = PREAMBLE;\n> - if (!bus->drive) {\n> - mdio_write_req(bus, bus->addr, bus->req, bus->data);\n> - }\n> - bus->drive = 0;\n> - }\n> + /* If end of DATA WRITE cycle, then write shift register to PHY */\n> + if (mdio_bitbang_update(bus, 16, PREAMBLE, &tmp) && (bus->opc == 1)) {\n> + mdio_write_req(bus, bus->addr, bus->req, tmp);\n> }\n> break;\n> default:\n> --\n> 2.14.1\n>\n>","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","ozlabs.org;\n\tdmarc=fail (p=none dis=none) header.from=gmail.com","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"H1ygmnCT\"; dkim-atps=neutral"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3zrYbV1N8Mz9s1n\n\tfor ;\n\tWed, 28 Feb 2018 09:41:56 +1100 (AEDT)","from localhost ([::1]:40505 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1eqnwY-0004Ol-Hz\n\tfor incoming@patchwork.ozlabs.org; Tue, 27 Feb 2018 17:41:54 -0500","from eggs.gnu.org ([2001:4830:134:3::10]:49872)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1eqnwA-0004Nf-HR\n\tfor qemu-devel@nongnu.org; Tue, 27 Feb 2018 17:41:33 -0500","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1eqnw8-0007Wi-Su\n\tfor qemu-devel@nongnu.org; Tue, 27 Feb 2018 17:41:30 -0500","from mail-lf0-x244.google.com ([2a00:1450:4010:c07::244]:35741)\n\tby eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16)\n\t(Exim 4.71) (envelope-from )\n\tid 1eqnw8-0007VP-GO\n\tfor qemu-devel@nongnu.org; Tue, 27 Feb 2018 17:41:28 -0500","by mail-lf0-x244.google.com with SMTP id 70so661706lfw.2\n\tfor ; Tue, 27 Feb 2018 14:41:28 -0800 (PST)","by 10.46.32.215 with HTTP; Tue, 27 Feb 2018 14:40:56 -0800 (PST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;\n\th=mime-version:in-reply-to:references:from:date:message-id:subject:to\n\t:cc:content-transfer-encoding;\n\tbh=d49AvziC7TFZRnsgluOzwzyKwtj/KYF6LJDGugDu0ec=;\n\tb=H1ygmnCT/aqKccxWPMpPzXhGpZY2RNZ+lFvzRmPOVCCsRCCH91+y2Gz3Sx0ueQCweh\n\t3x6sgAdcvb6jpGsNrDZuMhfNve6Aaco/XIsj/M3mbvDA7dJgR+zSNPJkYmcd72dlYGUt\n\tOZpycQKWO4slKo/42HWySg3qfDe8hqOAg7oiiUKnplTyGO4RfdI5sbNaBxIeEN2hEoIJ\n\tQDdfwIHIt6aBGw/hWa1a+AYsVYlt2IUmTUjRreJxlDG0QCMqQJn6Sf9rE8WlfY3yVQA7\n\tw0iqFlxN44gmCUYcneo//Ya4dJfkiM4oEOYc1Sf0Q0dLjG6cVspQrb++CVjTXopD/Sde\n\tGoxQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:in-reply-to:references:from:date\n\t:message-id:subject:to:cc:content-transfer-encoding;\n\tbh=d49AvziC7TFZRnsgluOzwzyKwtj/KYF6LJDGugDu0ec=;\n\tb=R+LK9YIdB/xUbiBFd6S3VX+SlBqiuSdWXA+BIvau8c9N27v+RtrjzJKR2mtq8cQzwj\n\tRAlM5+sHCB4sWapWQaTATkXtz1UHuFz1nAyazY06tMlEGDlPwRIXbDQrE7MCzrUQmLa5\n\t5X8sXvS6mE0jH7d/I/6qzwLfF7OM8ZDLrHDJozN1IbOnaX4Adb4gcwGu6mD10aqGb2Fz\n\tFfyJqtQlYf6fEuSdsmps0WNP9QDcGgO2nTOjJRjgs6szTcRxMtCV0ILQgTFp8g8Ovb1c\n\t5t1GUYxnBeAet84nQaGXuXrjDqW+sQi2llwZxySKoDvFgIGBsTJKlaIiFTqI5wsFUwZv\n\t2YKQ==","X-Gm-Message-State":"APf1xPBFzmfr0xMtdgPmql3sbRDpNfOZtSQansOFWfMvQXgNy9TgSdX0\n\tp3OjjPU5BuNoCVC31mj0M9zjTBRO1wIfs9+kBlM=","X-Google-Smtp-Source":"AG47ELvFw3TxX976s8iw1Rp5RK7IszGAGkCpg4+SC77ItuEA+EsSRNr2H4XsvQ2vJF5GQCmkfY5Z8BdisaP4AgAnbro=","X-Received":"by 10.25.145.72 with SMTP id y8mr10703417lfj.1.1519771287107;\n\tTue, 27 Feb 2018 14:41:27 -0800 (PST)","MIME-Version":"1.0","In-Reply-To":"<20170922171323.10348-6-f4bug@amsat.org>","References":"<20170922171323.10348-1-f4bug@amsat.org>\n\t<20170922171323.10348-6-f4bug@amsat.org>","From":"Alistair Francis ","Date":"Tue, 27 Feb 2018 14:40:56 -0800","Message-ID":"","To":"=?utf-8?q?Philippe_Mathieu-Daud=C3=A9?= ","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","X-detected-operating-system":"by eggs.gnu.org: Genre and OS details not\n\trecognized.","X-Received-From":"2a00:1450:4010:c07::244","Subject":"Re: [Qemu-devel] [PATCH v5 5/7] hw/mdio: Refactor bitbanging state\n\tmachine","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"Peter Maydell ,\n\tJason Wang , Grant Likely ,\n\t\"qemu-devel@nongnu.org Developers\" ,\n\t\"Edgar E. Iglesias\" ","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}}]