[U-Boot] [PATCH V2 3/5] dm9000: Add struct eth_device * to SROM functions

[Joe Hershberger]

Hi Andrew,

On Fri, Aug 21, 2015 at 6:25 AM, Andrew Ruder <andy at aeruder.net> wrote:
> On Wed, Aug 12, 2015 at 02:07:20PM -0500, Joe Hershberger wrote:
>> On Wed, Aug 12, 2015 at 12:24 PM, Andrew Ruder
>> <andrew.ruder at elecsyscorp.com> wrote:
>> >  /******************  function prototypes **********************/
>> >  #if !defined(CONFIG_DM9000_NO_SROM)
>> > -void dm9000_write_srom_word(int offset, u16 val);
>> > -void dm9000_read_srom_word(int offset, u8 *to);
>> > +struct eth_device;
>> > +
>> > +void dm9000_write_srom_word(struct eth_device *dev, int offset, u16 val);
>> > +void dm9000_read_srom_word(struct eth_device *dev, int offset, u8 *to);
>>
>> It will be better to pass the dm9000_priv* instead. See patch 5 for details.
>
> Even on the "public"-facing functions?

Your question got me looking closer at why are these public. These are
public just so that a command (presumably only available on one board
- trizepsiv) can access the eeprom of a dm9000. It also currently
assumes that there is only one dm9000 on that board.

I think it should not expose any internal dm9000 struct. Here's what I
mocked up:


diff --git a/board/trizepsiv/eeprom.c b/board/trizepsiv/eeprom.c
index 1318edc..93c5733 100644
--- a/board/trizepsiv/eeprom.c
+++ b/board/trizepsiv/eeprom.c
@@ -16,7 +16,7 @@ static int do_read_dm9000_eeprom ( cmd_tbl_t *cmdtp,
int flag, int argc, char *
        for (i=0; i < 0x40; i++) {
                if (!(i % 0x10))
                        printf("\n%08x:", i);
-               dm9000_read_srom_word(i, data);
+               dm9000_read_srom_word(0, i, data);
                printf(" %02x%02x", data[1], data[0]);
        }
        printf ("\n");
@@ -35,7 +35,7 @@ static int do_write_dm9000_eeprom ( cmd_tbl_t
*cmdtp, int flag, int argc, char *
                printf("Wrong offset : 0x%x\n",offset);
                return cmd_usage(cmdtp);
        }
-       dm9000_write_srom_word(offset, value);
+       dm9000_write_srom_word(0, offset, value);
        return (0);
 }

diff --git a/drivers/net/dm9000x.c b/drivers/net/dm9000x.c
index 3c41cec..8299f55 100644
--- a/drivers/net/dm9000x.c
+++ b/drivers/net/dm9000x.c
@@ -530,8 +530,10 @@ static int dm9000_rx(struct eth_device *netdev)
   Read a word data from SROM
 */
 #if !defined(CONFIG_DM9000_NO_SROM)
-void dm9000_read_srom_word(int offset, u8 *to)
+void dm9000_read_srom_word(int adapter_no, int offset, u8 *to)
 {
+       struct eth_device *dev = eth_get_dev_by_index(adapter_no);
+
        DM9000_iow(DM9000_EPAR, offset);
        DM9000_iow(DM9000_EPCR, 0x4);
        udelay(8000);
@@ -540,8 +542,10 @@ void dm9000_read_srom_word(int offset, u8 *to)
        to[1] = DM9000_ior(DM9000_EPDRH);
 }

-void dm9000_write_srom_word(int offset, u16 val)
+void dm9000_write_srom_word(int adapter_no, int offset, u16 val)
 {
+       struct eth_device *dev = eth_get_dev_by_index(adapter_no);
+
        DM9000_iow(DM9000_EPAR, offset);
        DM9000_iow(DM9000_EPDRH, ((val >> 8) & 0xff));
        DM9000_iow(DM9000_EPDRL, (val & 0xff));
diff --git a/include/dm9000.h b/include/dm9000.h
index 42b04fa..27d69cf 100644
--- a/include/dm9000.h
+++ b/include/dm9000.h
@@ -10,8 +10,8 @@

 /******************  function prototypes **********************/
 #if !defined(CONFIG_DM9000_NO_SROM)
-void dm9000_write_srom_word(int offset, u16 val);
-void dm9000_read_srom_word(int offset, u8 *to);
+void dm9000_write_srom_word(int adapter_no, int offset, u16 val);
+void dm9000_read_srom_word(int adapter_no, int offset, u8 *to);
 #endif

 #endif /* __DM9000_H__ */

Naturally the dev variable will be unused so it would need to not
actually be added until the patch that adds the used to the macro.

-Joe