Hi Luigi,
Luigi 'Comio' Mantellini wrote:
> From: Luigi 'Comio' Mantellini <luigi.mantell...@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.mantell...@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.mantell...@idf-hit.com>
> + *
> * (C) Copyright 2001
> * Gerald Van Baren, Custom IDEAS, vanba...@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
