From: Ben Warren <biggerbadderben@gmail.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 1/3 v3] Bit-banged MII driver with multi-bus support.
Date: Sun, 04 Oct 2009 23:27:44 -0700 [thread overview]
Message-ID: <4AC991E0.8080809@gmail.com> (raw)
In-Reply-To: <1253711456-6309-2-git-send-email-luigi.mantellini.ml@gmail.com>
Hi Luigi,
Luigi 'Comio' Mantellini wrote:
> From: Luigi 'Comio' Mantellini <luigi.mantellini@idf-hit.com>
>
>
Please add some descriptive information here. This is a big patch and
deserves a changelong
> Signed-off-by: Luigi 'Comio' Mantellini <luigi.mantellini@idf-hit.com>
> ---
> drivers/net/phy/miiphybb.c | 324 +++++++++++++++++++++++++++++++-------------
> include/miiphy.h | 22 +++
> 2 files changed, 250 insertions(+), 96 deletions(-)
>
> diff --git a/drivers/net/phy/miiphybb.c b/drivers/net/phy/miiphybb.c
> index b77c917..1ed27f1 100644
> --- a/drivers/net/phy/miiphybb.c
> +++ b/drivers/net/phy/miiphybb.c
> @@ -1,4 +1,7 @@
> /*
> + * (C) Copyright 2009 Industrie Dial Face S.p.A.
> + * Luigi 'Comio' Mantellini <luigi.mantellini@idf-hit.com>
> + *
> * (C) Copyright 2001
> * Gerald Van Baren, Custom IDEAS, vanbaren at cideas.com.
> *
> @@ -29,18 +32,137 @@
> #include <common.h>
> #include <ioports.h>
> #include <ppc_asm.tmpl>
> +#include <miiphy.h>
> +
> +#define BBMII_RELOATE(v,off) (v += (v?off:0))
>
s/RELOATE/RELOCATE/
Please apply globally
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +#ifndef CONFIG_BITBANGMII_MULTI
> +/*
> + * If CONFIG_BITBANGMII_MULTI is not defined we use a
> + * compatibility layer with the previous miiphybb implementation
> + * based on macros usage.
> + *
> + */
> +static int bb_mii_init_wrap(struct bbmiibus *bus)
> +{
> +#ifdef MII_INIT
> + MII_INIT;
> +#endif
> + return 0;
> +}
> +
> +static int bb_mdio_active_wrap(struct bbmiibus *bus)
> +{
> +#ifdef MDIO_DECLARE
> + MDIO_DECLARE;
> +#endif
> + MDIO_ACTIVE;
> + return 0;
> +}
> +
> +static int bb_mdio_tristate_wrap(struct bbmiibus *bus)
> +{
> +#ifdef MDIO_DECLARE
> + MDIO_DECLARE;
> +#endif
> + MDIO_TRISTATE;
> + return 0;
> +}
> +
> +static int bb_set_mdio_wrap(struct bbmiibus *bus, int v)
> +{
> +#ifdef MDIO_DECLARE
> + MDIO_DECLARE;
> +#endif
> + MDIO (v);
> + return 0;
> +}
> +
> +static int bb_get_mdio_wrap(struct bbmiibus *bus, int *v)
> +{
> +#ifdef MDIO_DECLARE
> + MDIO_DECLARE;
> +#endif
> + *v = MDIO_READ;
> + return 0;
> +}
> +
> +static int bb_set_mdc_wrap(struct bbmiibus *bus, int v)
> +{
> +#ifdef MDC_DECLARE
> + MDC_DECLARE;
> +#endif
> + MDC (v);
> + return 0;
> +}
> +
> +static int bb_delay_wrap(struct bbmiibus *bus)
> +{
> + MIIDELAY;
> + return 0;
> +}
> +
> +struct bbmiibus bbmiibusses[] = {
>
Elsewhere, you name things 'bb_mii', but here it's 'bbmii'. I
personally prefer adding the underscores, but please be consistent.
> + {
> + .name = BB_MII_DEVNAME,
> + .init = bb_mii_init_wrap,
> + .mdio_active = bb_mdio_active_wrap,
> + .mdio_tristate = bb_mdio_tristate_wrap,
> + .set_mdio = bb_set_mdio_wrap,
> + .get_mdio = bb_get_mdio_wrap,
> + .set_mdc = bb_set_mdc_wrap,
> + .delay = bb_delay_wrap,
> + }
> +};
> +#endif
> +
> +void bb_miiphy_init(void)
> +{
> + int i;
> + for (i = 0; i < sizeof(bbmiibusses)/sizeof(bbmiibusses[0]); i++) {
> +#if !defined(CONFIG_RELOC_FIXUP_WORKS)
> + /* Reloate the hooks pointers*/
>
s/Reloate/Relocate/
s/hooks/hook/
> + BBMII_RELOATE(bbmiibusses[i].init, gd->reloc_off);
> + BBMII_RELOATE(bbmiibusses[i].mdio_active, gd->reloc_off);
> + BBMII_RELOATE(bbmiibusses[i].mdio_tristate, gd->reloc_off);
> + BBMII_RELOATE(bbmiibusses[i].set_mdio, gd->reloc_off);
> + BBMII_RELOATE(bbmiibusses[i].get_mdio, gd->reloc_off);
> + BBMII_RELOATE(bbmiibusses[i].set_mdc, gd->reloc_off);
> + BBMII_RELOATE(bbmiibusses[i].delay, gd->reloc_off);
> +#endif
> +
> + if (bbmiibusses[i].init != NULL) {
> + bbmiibusses[i].init(&bbmiibusses[i]);
> + }
> + }
> +}
> +
> +static inline struct bbmiibus *bb_miiphy_getbus(char *devname)
> +{
> +#ifdef CONFIG_BITBANGMII_MULTI
> + /* Search the correct bus */
> + for (j = 0; j < sizeof(bbmiibusses)/sizeof(bbmmis[0]); j++) {
> + if (!strcmp(bbmiibusses[i].name, devname)) {
> + return &bbmiibusses[i];
> + }
> + }
> + return NULL;
> +#else
> + /* We have just one bitbanging bus */
> + return &bbmiibusses[0];
> +#endif
> +}
>
> /*****************************************************************************
> *
> * Utility to send the preamble, address, and register (common to read
> * and write).
> */
> -static void miiphy_pre (char read, unsigned char addr, unsigned char reg)
> +static void miiphy_pre (struct bbmiibus *bus, char read, unsigned char addr, unsigned char reg)
>
This line's too long (please keep < 78 characters). Apply globally
> {
> int j; /* counter */
> -#if !(defined(CONFIG_EP8248) || defined(CONFIG_EP82XXM))
> - volatile ioport_t *iop = ioport_addr ((immap_t *) CONFIG_SYS_IMMR, MDIO_PORT);
> -#endif
>
> /*
> * Send a 32 bit preamble ('1's) with an extra '1' bit for good measure.
> @@ -50,67 +172,66 @@ static void miiphy_pre (char read, unsigned char addr, unsigned char reg)
> * but it is safer and will be much more robust.
> */
>
> - MDIO_ACTIVE;
> - MDIO (1);
> + bus->mdio_active(bus);
> + bus->set_mdio (bus, 1);
> for (j = 0; j < 32; j++) {
> - MDC (0);
> - MIIDELAY;
> - MDC (1);
> - MIIDELAY;
> + bus->set_mdc (bus, 0);
> + bus->delay(bus);
> + bus->set_mdc (bus, 1);
> + bus->delay(bus);
> }
>
> /* send the start bit (01) and the read opcode (10) or write (10) */
> - MDC (0);
> - MDIO (0);
> - MIIDELAY;
> - MDC (1);
> - MIIDELAY;
> - MDC (0);
> - MDIO (1);
> - MIIDELAY;
> - MDC (1);
> - MIIDELAY;
> - MDC (0);
> - MDIO (read);
> - MIIDELAY;
> - MDC (1);
> - MIIDELAY;
> - MDC (0);
> - MDIO (!read);
> - MIIDELAY;
> - MDC (1);
> - MIIDELAY;
> + bus->set_mdc (bus, 0);
> + bus->set_mdio (bus, 0);
> + bus->delay(bus);
>
Please be consistent with whitespace. I prefer no space before the
opening paren, although I think Wolfgang likes that. He's funny that
way. Note that this only relates to function calls. Control logic (if,
for, while etc.) should always have a space before the opening paren.
> + bus->set_mdc (bus, 1);
> + bus->delay(bus);
> + bus->set_mdc (bus, 0);
> + bus->set_mdio (bus, 1);
> + bus->delay(bus);
> + bus->set_mdc (bus, 1);
> + bus->delay(bus);
> + bus->set_mdc (bus, 0);
> + bus->set_mdio (bus, read);
> + bus->delay(bus);
> + bus->set_mdc (bus, 1);
> + bus->delay(bus);
> + bus->set_mdc (bus, 0);
> + bus->set_mdio (bus, !read);
> + bus->delay(bus);
> + bus->set_mdc (bus, 1);
> + bus->delay(bus);
>
> /* send the PHY address */
> for (j = 0; j < 5; j++) {
> - MDC (0);
> + bus->set_mdc (bus, 0);
> if ((addr & 0x10) == 0) {
> - MDIO (0);
> + bus->set_mdio (bus, 0);
> } else {
> - MDIO (1);
> + bus->set_mdio (bus, 1);
> }
> - MIIDELAY;
> - MDC (1);
> - MIIDELAY;
> + bus->delay(bus);
> + bus->set_mdc (bus, 1);
> + bus->delay(bus);
> addr <<= 1;
> }
>
> /* send the register address */
> for (j = 0; j < 5; j++) {
> - MDC (0);
> + bus->set_mdc (bus, 0);
> if ((reg & 0x10) == 0) {
> - MDIO (0);
> + bus->set_mdio (bus, 0);
> } else {
> - MDIO (1);
> + bus->set_mdio (bus, 1);
> }
> - MIIDELAY;
> - MDC (1);
> - MIIDELAY;
> + bus->delay(bus);
> + bus->set_mdc (bus, 1);
> + bus->delay(bus);
> reg <<= 1;
> }
> }
>
> -
> /*****************************************************************************
> *
> * Read a MII PHY register.
> @@ -122,59 +243,66 @@ int bb_miiphy_read (char *devname, unsigned char addr,
> unsigned char reg, unsigned short *value)
> {
> short rdreg; /* register working value */
> + int v;
> int j; /* counter */
> -#if !(defined(CONFIG_EP8248) || defined(CONFIG_EP82XXM))
> - volatile ioport_t *iop = ioport_addr ((immap_t *) CONFIG_SYS_IMMR, MDIO_PORT);
> -#endif
> + struct bbmiibus *bus;
> +
> + bus = bb_miiphy_getbus(devname);
> + if (bus == NULL) {
> + /* Bus not found! */
>
Comment's kinda superfluous...
> + return -1;
> + }
>
> if (value == NULL) {
> puts("NULL value pointer\n");
> return (-1);
> }
>
> - miiphy_pre (1, addr, reg);
> + miiphy_pre (bus, 1, addr, reg);
>
> /* tri-state our MDIO I/O pin so we can read */
> - MDC (0);
> - MDIO_TRISTATE;
> - MIIDELAY;
> - MDC (1);
> - MIIDELAY;
> + bus->set_mdc (bus, 0);
> + bus->mdio_tristate(bus);
> + bus->delay(bus);
> + bus->set_mdc (bus, 1);
> + bus->delay(bus);
>
> /* check the turnaround bit: the PHY should be driving it to zero */
> - if (MDIO_READ != 0) {
> + bus->get_mdio(bus, &v);
> + if (v != 0) {
> /* puts ("PHY didn't drive TA low\n"); */
> for (j = 0; j < 32; j++) {
> - MDC (0);
> - MIIDELAY;
> - MDC (1);
> - MIIDELAY;
> + bus->set_mdc (bus, 0);
> + bus->delay(bus);
> + bus->set_mdc (bus, 1);
> + bus->delay(bus);
> }
> /* There is no PHY, set value to 0xFFFF and return */
> *value = 0xFFFF;
> return (-1);
> }
>
> - MDC (0);
> - MIIDELAY;
> + bus->set_mdc (bus, 0);
> + bus->delay(bus);
>
> /* read 16 bits of register data, MSB first */
> rdreg = 0;
> for (j = 0; j < 16; j++) {
> - MDC (1);
> - MIIDELAY;
> + bus->set_mdc (bus, 1);
> + bus->delay(bus);
> rdreg <<= 1;
> - rdreg |= MDIO_READ;
> - MDC (0);
> - MIIDELAY;
> + bus->get_mdio(bus, &v);
> + rdreg |= (v & 0x1);
> + bus->set_mdc (bus, 0);
> + bus->delay(bus);
> }
>
> - MDC (1);
> - MIIDELAY;
> - MDC (0);
> - MIIDELAY;
> - MDC (1);
> - MIIDELAY;
> + bus->set_mdc (bus, 1);
> + bus->delay(bus);
> + bus->set_mdc (bus, 0);
> + bus->delay(bus);
> + bus->set_mdc (bus, 1);
> + bus->delay(bus);
>
> *value = rdreg;
>
> @@ -196,47 +324,51 @@ int bb_miiphy_read (char *devname, unsigned char addr,
> int bb_miiphy_write (char *devname, unsigned char addr,
> unsigned char reg, unsigned short value)
> {
> + struct bbmiibus *bus;
> int j; /* counter */
> -#if !(defined(CONFIG_EP8248) || defined(CONFIG_EP82XXM))
> - volatile ioport_t *iop = ioport_addr ((immap_t *) CONFIG_SYS_IMMR, MDIO_PORT);
> -#endif
>
> - miiphy_pre (0, addr, reg);
> + bus = bb_miiphy_getbus(devname);
> + if (bus == NULL) {
> + /* Bus not found! */
> + return -1;
> + }
> +
> + miiphy_pre (bus, 0, addr, reg);
>
> /* send the turnaround (10) */
> - MDC (0);
> - MDIO (1);
> - MIIDELAY;
> - MDC (1);
> - MIIDELAY;
> - MDC (0);
> - MDIO (0);
> - MIIDELAY;
> - MDC (1);
> - MIIDELAY;
> + bus->set_mdc (bus, 0);
> + bus->set_mdio (bus, 1);
> + bus->delay(bus);
> + bus->set_mdc (bus, 1);
> + bus->delay(bus);
> + bus->set_mdc (bus, 0);
> + bus->set_mdio (bus, 0);
> + bus->delay(bus);
> + bus->set_mdc (bus, 1);
> + bus->delay(bus);
>
> /* write 16 bits of register data, MSB first */
> for (j = 0; j < 16; j++) {
> - MDC (0);
> + bus->set_mdc (bus, 0);
> if ((value & 0x00008000) == 0) {
> - MDIO (0);
> + bus->set_mdio (bus, 0);
> } else {
> - MDIO (1);
> + bus->set_mdio (bus, 1);
> }
> - MIIDELAY;
> - MDC (1);
> - MIIDELAY;
> + bus->delay(bus);
> + bus->set_mdc (bus, 1);
> + bus->delay(bus);
> value <<= 1;
> }
>
> /*
> * Tri-state the MDIO line.
> */
> - MDIO_TRISTATE;
> - MDC (0);
> - MIIDELAY;
> - MDC (1);
> - MIIDELAY;
> + bus->mdio_tristate(bus);
> + bus->set_mdc (bus, 0);
> + bus->delay(bus);
> + bus->set_mdc (bus, 1);
> + bus->delay(bus);
>
> return 0;
> -}
> +}
> \ No newline at end of file
> diff --git a/include/miiphy.h b/include/miiphy.h
> index fa33ec7..478c050 100644
> --- a/include/miiphy.h
> +++ b/include/miiphy.h
> @@ -19,6 +19,8 @@
> |
> | COPYRIGHT I B M CORPORATION 1999
> | LICENSED MATERIAL - PROGRAM PROPERTY OF I B M
> +|
> +| Additions (C) Copyright 2009 Industrie Dial Face S.p.A.
> +----------------------------------------------------------------------------*/
> /*----------------------------------------------------------------------------+
> |
> @@ -61,12 +63,32 @@ char *miiphy_get_current_dev (void);
>
> void miiphy_listdev (void);
>
> +#ifdef CONFIG_BITBANGMII
> +
> #define BB_MII_DEVNAME "bbmii"
>
> +struct bbmiibus {
> + char name[NAMESIZE];
> + int (*init)(struct bbmiibus *bus);
> + int (*mdio_active)(struct bbmiibus *bus);
> + int (*mdio_tristate)(struct bbmiibus *bus);
> + int (*set_mdio)(struct bbmiibus *bus, int v);
> + int (*get_mdio)(struct bbmiibus *bus, int *v);
> + int (*set_mdc)(struct bbmiibus *bus, int v);
> + int (*delay)(struct bbmiibus *bus);
> +#ifdef CONFIG_BITBANGMII_MULTI
> + void *priv;
> +#endif
> +};
> +
> +extern struct bbmiibus bbmiibusses[];
> +
> +void bb_miiphy_init (void);
> int bb_miiphy_read (char *devname, unsigned char addr,
> unsigned char reg, unsigned short *value);
> int bb_miiphy_write (char *devname, unsigned char addr,
> unsigned char reg, unsigned short value);
> +#endif
>
> /* phy seed setup */
> #define AUTO 99
>
Thanks for this submission. It's a really big improvement.
regards,
Ben
next prev parent reply other threads:[~2009-10-05 6:27 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-09-23 13:10 [U-Boot] [PATCH 0/3 v3] New MIIPHYBB implementation with multi-bus support Luigi 'Comio' Mantellini
2009-09-23 13:10 ` [U-Boot] [PATCH 1/3 v3] Bit-banged MII driver " Luigi 'Comio' Mantellini
2009-09-23 13:10 ` [U-Boot] [PATCH 2/3 v3] Add bb_miiphy_init call before any ethernet bring-up code Luigi 'Comio' Mantellini
2009-09-23 13:10 ` [U-Boot] [PATCH 3/3 v3] Update all board to support new bbmiiphy driver (with multibus support) Luigi 'Comio' Mantellini
2009-10-05 6:34 ` Ben Warren
2009-10-05 6:30 ` [U-Boot] [PATCH 2/3 v3] Add bb_miiphy_init call before any ethernet bring-up code Ben Warren
2009-10-05 20:04 ` Luigi Mantellini
2009-10-05 20:18 ` Ben Warren
2009-10-05 20:35 ` Luigi Mantellini
2009-09-24 12:52 ` [U-Boot] [PATCH 1/3 v3] Bit-banged MII driver with multi-bus support Luigi 'Comio' Mantellini
2009-09-24 13:35 ` Jerry Van Baren
2009-10-05 6:27 ` Ben Warren [this message]
2009-10-05 19:55 ` Luigi Mantellini
2009-10-05 20:04 ` Ben Warren
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4AC991E0.8080809@gmail.com \
--to=biggerbadderben@gmail.com \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.