NET: QE: UEC: Make uec_miiphy_read() and uec_miiphy_write() use the devname arg.
PATCH 1/2 NET: QE: UEC: Make uec_miiphy_read() and uec_miiphy_write() use the devname arg.
Added a new function uec_miiphy_find_dev_by_name to allow uec_miiphy_read and uec_miiphy_write to use the passed devname and not hardcoded to devlist[0]
drivers/qe/uec.c +42 -3
@@ -639,6 +639,32 @@ static void phy_change(struct eth_device *dev) && !defined (BITBANGMII )/* +* Find a device index from the devlist by name +* +* Returns: +* The index where the device is located, else 0 + */ +static int uec_miiphy_find_dev_by_name (char *devname) +{ +int i =0 ; + + +for (i =0 ; i <MAXCONTROLLERS ; i++) { +if (strncmp (devname, devlist[i]->name,strlen (devname)) ==0 ) { +break ; + } + } + +// If device cannot be found, default to 0 +if (i ==MAXCONTROLLERS ) { +debug ("%s: device %s not found in devlist\n" , __FUNCTION__, devname); + i =0 ; + } + +return (i); +} + +/* * Read a MII PHY register. * * Returns:
@@ -647,8 +673,15 @@ static void phy_change(struct eth_device *dev)static int uec_miiphy_read (char *devname,unsigned char addr,unsigned char reg,unsigned short *value) { - *value =uec_read_phy_reg (devlist[0 ], addr, reg); +int i =0 ; + +if (devname ==NULL || value ==NULL ) { +debug ("%s: NULL pointer given\n" , __FUNCTION__); + }else { + i =uec_miiphy_find_dev_by_name (devname); + *value =uec_read_phy_reg (devlist[i], addr, reg); + }return 0 ; }
@@ -661,11 +694,17 @@ static int uec_miiphy_read(char *devname, unsigned char addr,static int uec_miiphy_write (char *devname,unsigned char addr,unsigned char reg,unsigned short value) { -uec_write_phy_reg (devlist[0 ], addr, reg, value); +int i =0 ; + +if (devname ==NULL ) { +debug ("%s: NULL pointer given\n" , __FUNCTION__); + }else { + i =uec_miiphy_find_dev_by_name (devname); +uec_write_phy_reg (devlist[i], addr, reg, value); + }return 0 ; } -#endif static int uec_set_mac_address (uec_private_t *uec,u8 *mac_addr)
PATCH 1/2 NET: QE: UEC: Make uec_miiphy_read() and uec_miiphy_write() use the devname arg.
Quoted 10 lines · richardretanubun
Added a new function
uec_miiphy_find_dev_by_nameto allowuec_miiphy_readanduec_miiphy_writeto use the passed devname and not hardcoded to devlist[0]Signed-off-by Richard Retanubun <RichardRetanubun_at_ruggedcom.com>drivers/qe/uec.c +0
-0
drivers/qe/uec.c static void phy_change(struct eth_device *dev) · line 646
Quoted 8 lines
drivers/qe/uec.c +4
-0@@ -639,6 +639,32 @@ static void phy_change(struct eth_device *dev) && !defined (BITBANGMII )/* +* Find a device index from the devlist by name +* +* Returns: +* The index where the device is located, else 0
But index 0 is a valid device. Return -1 on failure and check for it.
drivers/qe/uec.c static void phy_change(struct eth_device *dev)
Quoted 5 lines
drivers/qe/uec.c +4
-0+*/ +static int uec_miiphy_find_dev_by_name (char *devname) +{ +int i =0 ;
No need to initialize this variable.
drivers/qe/uec.c static void phy_change(struct eth_device *dev)
Quoted 10 lines
drivers/qe/uec.c +9
-0+ + +for (i =0 ; i <MAXCONTROLLERS ; i++) { +if (strncmp (devname, devlist[i]->name,strlen (devname)) ==0 ) { +break ; + } + } + +// If device cannot be found, default to 0
No C++ - style comments please.
drivers/qe/uec.c static void phy_change(struct eth_device *dev) · line 661
Quoted 4 lines
drivers/qe/uec.c +3
-0+if (i ==MAXCONTROLLERS ) { +debug ("%s: device %s not found in devlist\n" , __FUNCTION__, devname); + i =0 ;
As mentioned above, don't set to a valid index on failure. -1 is standard for this type of thing.
drivers/qe/uec.c static void phy_change(struct eth_device *dev) · line 664
Quoted 4 lines
drivers/qe/uec.c +3
-0+ } + +return (i);
No brackets needed around return value.
drivers/qe/uec.c static void phy_change(struct eth_device *dev) · line 677
Quoted 13 lines
drivers/qe/uec.c +3
-0+} + +/* * Read a MII PHY register.
*
* Returns:drivers/qe/uec.c +1
-1@@ -647,8 +673,15 @@ static void phy_change(struct eth_device *dev)static int uec_miiphy_read (char *devname,unsigned char addr,unsigned char reg,unsigned short *value) { - *value =uec_read_phy_reg (devlist[0 ], addr, reg); +int i =0 ;
This isn't a good variable name except for iterators. Please use something more meaningful.
drivers/qe/uec.c static void phy_change(struct eth_device *dev) · line 687
Quoted 7 lines
drivers/qe/uec.c +5
-0+ +if (devname ==NULL || value ==NULL ) { +debug ("%s: NULL pointer given\n" , __FUNCTION__); + }else { + i =uec_miiphy_find_dev_by_name (devname);
Bail if i<0
Quoted 10 lines
drivers/qe/uec.c +2
-0+ *value =uec_read_phy_reg (devlist[i], addr, reg); + }return 0 ;}
drivers/qe/uec.c +0
-1@@ -661,11 +694,17 @@ static int uec_miiphy_read(char *devname, unsigned char addr,static int uec_miiphy_write (char *devname,unsigned char addr,unsigned char reg,unsigned short value) { -uec_write_phy_reg (devlist[0 ], addr, reg, value);
drivers/qe/uec.c static int uec_miiphy_read(char *devname, unsigned char addr, · line 706
Quoted 10 lines
drivers/qe/uec.c +8
-0+int i =0 ; + +if (devname ==NULL ) { +debug ("%s: NULL pointer given\n" , __FUNCTION__); + }else { + i =uec_miiphy_find_dev_by_name (devname); +uec_write_phy_reg (devlist[i], addr, reg, value); + }
Same comments as with other function.
Quoted 8 lines
drivers/qe/uec.c +0
-1return 0; } -#endif static int
uec_set_mac_address(uec_private_t *uec, u8 *mac_addr)
regards,
Ben
PATCH v2 1/2 NET: QE: UEC: Make uec_miiphy_read() and uec_miiphy_write() use the devname arg.
Hi Ben,
Thanks for the feedback. I've made the changes you suggested. Here is the patch re-pasted (please let me know if this is not the proper way to submit patch v2).
--------------------------------------------
Added a new function uec_miiphy_find_dev_by_name to allow uec_miiphy_read and uec_miiphy_write to use the passed devname and not hardcoded to devlist[0]
drivers/qe/uec.c +19 -0
@@ -639,16 +639,51 @@ static void phy_change(struct eth_device *dev) && !defined (BITBANGMII )/* +* Find a device index from the devlist by name +* +* Returns: +* The index where the device is located, -1 on error + */ +static int uec_miiphy_find_dev_by_name (char *devname) +{ +int i; + + +for (i =0 ; i <MAXCONTROLLERS ; i++) { +if (strncmp (devname, devlist[i]->name,strlen (devname)) ==0 ) { +break ; + } + } + +/* If device cannot be found, returns -1 */ +if (i ==MAXCONTROLLERS ) { + debug ("%s: device %s not found in devlist\n", __FUNCTION__,
devname);
+ i = -1;
+ }
+
+ return i;
+}
+
+/*
* Read a MII PHY register.
*
* Returns:
* 0 on success
*/
static int uec_miiphy_read(char *devname, unsigned char addr,
- unsigned char reg, unsigned short *value)
+ unsigned char reg, unsigned short *value)
{
- *value = uec_read_phy_reg(devlist[0], addr, reg);
+ int devindex = 0;
+
+ if (devname == NULL || value == NULL) {
+ debug("%s: NULL pointer given\n", __FUNCTION__);
+ } else {
+ devindex = uec_miiphy_find_dev_by_name(devname);
+ if (devindex >= 0) {
+ *value = uec_read_phy_reg(devlist[devindex], addr, reg);
+ }
+ }
return 0;
}
@@ -659,13 +694,21 @@ static int uec_miiphy_read(char *devname, unsigned char addr,
* 0 on success
*/
static int uec_miiphy_write(char *devname, unsigned char addr,
- unsigned char reg, unsigned short value)
+ unsigned char reg, unsigned short value)
{
- uec_write_phy_reg(devlist[0], addr, reg, value);
+ int devindex = 0;
+
+ if (devname == NULL) {
+ debug("%s: NULL pointer given\n", __FUNCTION__);
+ } else {
+ devindex = uec_miiphy_find_dev_by_name(devname);
+ if (devindex >= 0) {
+ uec_write_phy_reg(devlist[devindex], addr, reg, value);
+ }
+ }
return 0;
}
-
#endif
static int uec_set_mac_address(uec_private_t *uec, u8 *mac_addr)
Quoted 105 lines · Ben Warren
Quoted 11 lines · richardretanubun
Added a new function
uec_miiphy_find_dev_by_nameto allowuec_miiphy_readanduec_miiphy_writeto use the passed devname and not hardcoded to devlist[0]Signed-off-by Richard Retanubun <RichardRetanubun_at_ruggedcom.com>drivers/qe/uec.c +0
-0drivers/qe/uec.c static void phy_change(struct eth_device *dev) · line 646
Quoted 8 lines
drivers/qe/uec.c +4
-0@@ -639,6 +639,32 @@ static void phy_change(struct eth_device *dev) && !defined (BITBANGMII )/* +* Find a device index from the devlist by name +* +* Returns: +* The index where the device is located, else 0 But index 0 is a valid device. Return -1 on failure and check for it.
drivers/qe/uec.c static void phy_change(struct eth_device *dev) · line 651
Quoted 5 lines
drivers/qe/uec.c +4
-0+*/ +static int uec_miiphy_find_dev_by_name (char *devname) +{ +int i =0 ;No need to initialize this variable.
drivers/qe/uec.c static void phy_change(struct eth_device *dev) · line 661
Quoted 10 lines
drivers/qe/uec.c +9
-0+ + +for (i =0 ; i <MAXCONTROLLERS ; i++) { +if (strncmp (devname, devlist[i]->name,strlen (devname)) ==0 ) { +break ; + } + } + +// If device cannot be found, default to 0 No C++ - style comments please.
Quoted 5 lines
drivers/qe/uec.c +2
-0+if (i ==MAXCONTROLLERS ) { + debug ("%s: device %s not found in devlist\n", __FUNCTION__,devname);
+ i = 0;As mentioned above, don't set to a valid index on failure. -1 is standard for this type of thing.
drivers/qe/uec.c
Quoted 4 lines
drivers/qe/uec.c +3
-0+ } + +return (i);No brackets needed around return value.
drivers/qe/uec.c static void phy_change(struct eth_device *dev) · line 677
Quoted 13 lines
drivers/qe/uec.c +4
-1+} + +/* * Read a MII PHY register. * * Returns: @@ -647,8 +673,15 @@ static void phy_change(struct eth_device *dev)static int uec_miiphy_read (char *devname,unsigned char addr,unsigned char reg,unsigned short *value) { - *value =uec_read_phy_reg (devlist[0 ], addr, reg); +int i =0 ;This isn't a good variable name except for iterators. Please use something more meaningful.
drivers/qe/uec.c static void phy_change(struct eth_device *dev) · line 684
Quoted 7 lines
drivers/qe/uec.c +5
-0+ +if (devname ==NULL || value ==NULL ) { +debug ("%s: NULL pointer given\n" , __FUNCTION__); + }else { + i =uec_miiphy_find_dev_by_name (devname);Bail if i<0
Quoted 11 lines
drivers/qe/uec.c +2
-0+ *value =uec_read_phy_reg (devlist[i], addr, reg); + }return 0 ;}
drivers/qe/uec.c +0
-1@@ -661,11 +694,17 @@ static int uec_miiphy_read(char *devname, unsigned char addr, static int uec_miiphy_write(char *devname, unsigned char addr, unsigned char reg, unsigned short value) { -uec_write_phy_reg (devlist [0 ],addr ,reg ,value );drivers/qe/uec.c static int uec_miiphy_read(char *devname, · line 707
Quoted 10 lines
drivers/qe/uec.c +8
-0+int i =0 ; + +if (devname ==NULL ) { +debug ("%s: NULL pointer given\n" , __FUNCTION__); + }else { + i =uec_miiphy_find_dev_by_name (devname); +uec_write_phy_reg (devlist[i], addr, reg, value); + }Same comments as with other function.
Quoted 8 lines
drivers/qe/uec.c +0
-1return 0; } -#endif static int
uec_set_mac_address(uec_private_t *uec, u8 *mac_addr)regards,
Ben
PATCH v2 1/2 NET: QE: UEC: Make uec_miiphy_read() and uec_miiphy_write() use the devname arg.
Hi Richard,
Quoted 7 lines · richardretanubun
Hi Ben,
Thanks for the feedback. I've made the changes you suggested. Here is the patch re-pasted (please let me know if this is not the proper way to submit patch v2).
--------------------------------------------
I starte to fix this up so that it would apply, but it's been badly mangled by your mailer. Would you mind please re-sending? If you're using Thunderbird or Evolution, select the "preformat" text type when you paste the patch. If using something else, please just attach the patch file.
regards,
Ben
PATCH v2 1/2 NET: QE: UEC: Make uec_miiphy_read() and uec_miiphy_write() use the devname arg.
Hi Ben,
Quoted 10 lines · Ben Warren
Hi Richard,
Quoted 7 lines · richardretanubun
Hi Ben,
Thanks for the feedback. I've made the changes you suggested. Here is the patch re-pasted (please let me know if this is not the proper way to submit patch v2).
--------------------------------------------
Quoted 6 lines
I started to fix this up so that it would apply, but it's been badly mangled by your mailer. Would you mind please re-sending? If you're using Thunderbird or Evolution, select the "preformat" text type when you paste the patch. If using something else, please just attach the patch file.
Sorry for replying so late (and twice, I forgot to cc the mailing list). Here is the patch in preformat form and also attached, just in case.
Quoted 4 lines
regards,
Ben
Thanks for all your help
Richard
-------------------------------------
Added a new function uec_miiphy_find_dev_by_name to allow
uec_miiphy_read and uec_miiphy_write to use the passed devname and not
hardcoded to devlist[0]
drivers/qe/uec.c +0 -0
index 344c649..9b0b3bc 100644
--- a/drivers/qe/uec.c
+++ b/drivers/qe/uec.c
@@ -639,16 +639,51 @@ static void phy_change(struct eth_device *dev)
&& !defined(BITBANGMII)
/*
+ * Find a device index from the devlist by name
+ *
+ * Returns:
+ * The index where the device is located, -1 on error
+ */
+static int uec_miiphy_find_dev_by_name(char *devname)
+{
+ int i;
+
+
+ for (i = 0; i < MAXCONTROLLERS; i++) {
+ if (strncmp(devname, devlist[i]->name, strlen(devname)) == 0) {
+ break;
+ }
+ }
+
+ /* If device cannot be found, returns -1 */
+ if (i == MAXCONTROLLERS) {
+ debug ("%s: device %s not found in devlist\n", __FUNCTION__,
devname);
+ i = -1;
+ }
+
+ return i;
+}
+
+/*
* Read a MII PHY register.
*
* Returns:
* 0 on success
*/
static int uec_miiphy_read(char *devname, unsigned char addr,
- unsigned char reg, unsigned short *value)
+ unsigned char reg, unsigned short *value)
{
- *value = uec_read_phy_reg(devlist[0], addr, reg);
+ int devindex = 0;
+
+ if (devname == NULL || value == NULL) {
+ debug("%s: NULL pointer given\n", __FUNCTION__);
+ } else {
+ devindex = uec_miiphy_find_dev_by_name(devname);
+ if (devindex >= 0) {
+ *value = uec_read_phy_reg(devlist[devindex], addr, reg);
+ }
+ }
return 0;
}
@@ -659,13 +694,21 @@ static int uec_miiphy_read(char *devname, unsigned
char addr,
* 0 on success
*/
static int uec_miiphy_write(char *devname, unsigned char addr,
- unsigned char reg, unsigned short value)
+ unsigned char reg, unsigned short value)
{
- uec_write_phy_reg(devlist[0], addr, reg, value);
+ int devindex = 0;
+
+ if (devname == NULL) {
+ debug("%s: NULL pointer given\n", __FUNCTION__);
+ } else {
+ devindex = uec_miiphy_find_dev_by_name(devname);
+ if (devindex >= 0) {
+ uec_write_phy_reg(devlist[devindex], addr, reg, value);
+ }
+ }
return 0;
}
-
#endif
static int uec_set_mac_address(uec_private_t *uec, u8 *mac_addr)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: qe_uec_miiphy_not_hardcode.patch
Type: text/x-patch
Size: 2441 bytes
Desc: not available
Url : http://lists.denx.de/pipermail/u-boot/attachments/20080916/6fb10861/attachment.bin
PATCH v2 1/2 NET: QE: UEC: Make uec_miiphy_read() and uec_miiphy_write() use the devname arg.
Hi Ben,
Sorry I keep mangling the patch.
I made an updated version based on the latest files from the git-tree.
If you don't mind, can you try applying it?
Thanks for all the help
-Richard
PATCH v2 1/2 NET: QE: UEC: Make uec_miiphy_read() and uec_miiphy_write() use the devname arg.
Quoted 7 lines · richardretanubun
Hi Ben,
Sorry I keep mangling the patch. I made an updated version based on the latest files from the git-tree.
If you don't mind, can you try applying it?
Good news and bad news:
good news: it applies clean
bad news: you'll need to resubmit with the patch description above your signed-off-by, and stuff like 'Hi Ben ... ' below the --- line.
When we use 'git am' to apply patches, it includes everything above '---' in the changelog and discards everything below that's not patch code.
Quoted 2 lines
Thanks for all the help
thanks for your patience.
Quoted 1 line
-Richard
regards,
Ben
PATCH v2 1/2 NET: QE: UEC: Make uec_miiphy_read() and uec_miiphy_write() use the devname arg.
--- drivers/qe/uec.c | 46 +++++++++++++++++++++++++++++++++++++++++++--- 1 files changed, 43 insertions(+), 3 deletions(-)
drivers/qe/uec.c +43 -3
@@ -639,6 +639,31 @@ static void phy_change(struct eth_device *dev) && !defined (BITBANGMII )/* +* Find a device index from the devlist by name +* +* Returns: +* The index where the device is located, -1 on error + */ +static int uec_miiphy_find_dev_by_name (char *devname) +{ +int i; + +for (i =0 ; i <MAXCONTROLLERS ; i++) { +if (strncmp (devname, devlist[i]->name,strlen (devname)) ==0 ) { +break ; + } + } + +/* If device cannot be found, returns -1 */ +if (i ==MAXCONTROLLERS ) { +debug ("%s: device %s not found in devlist\n" , __FUNCTION__, devname); + i =-1 ; + } + +return i; +} + +/* * Read a MII PHY register. * * Returns:
@@ -647,8 +672,16 @@ static void phy_change(struct eth_device *dev)static int uec_miiphy_read (char *devname,unsigned char addr,unsigned char reg,unsigned short *value) { - *value =uec_read_phy_reg (devlist[0 ], addr, reg); +int devindex =0 ; +if (devname ==NULL || value ==NULL ) { +debug ("%s: NULL pointer given\n" , __FUNCTION__); + }else { + devindex =uec_miiphy_find_dev_by_name (devname); +if (devindex >=0 ) { + *value =uec_read_phy_reg (devlist[devindex], addr, reg); + } + }return 0 ; }
@@ -661,11 +694,18 @@ static int uec_miiphy_read(char *devname, unsigned char addr,static int uec_miiphy_write (char *devname,unsigned char addr,unsigned char reg,unsigned short value) { -uec_write_phy_reg (devlist[0 ], addr, reg, value); +int devindex =0 ; +if (devname ==NULL ) { +debug ("%s: NULL pointer given\n" , __FUNCTION__); + }else { + devindex =uec_miiphy_find_dev_by_name (devname); +if (devindex >=0 ) { +uec_write_phy_reg (devlist[devindex], addr, reg, value); + } + }return 0 ; } -#endif static int uec_set_mac_address (uec_private_t *uec,u8 *mac_addr)
Signature · 1 line
1.5.5.GIT
PATCH v2 1/2 NET: QE: UEC: Make uec_miiphy_read() and uec_miiphy_write() use the devname arg.
The current uec_miiphy_read and uec_miiphy_write hardcode access devlist[0] This patch makes these function use the devname argument that is passed in to allow access to the phy registers of other devices in devlist[].
---
Hi Ben,
I'm hoping the 7th try will do it. I forgot to add the commit message before.
drivers/qe/uec.c | 46 +++++++++++++++++++++++++++++++++++++++++++--- 1 files changed, 43 insertions(+), 3 deletions(-)
drivers/qe/uec.c +43 -3
@@ -639,6 +639,31 @@ static void phy_change(struct eth_device *dev) && !defined (BITBANGMII )/* +* Find a device index from the devlist by name +* +* Returns: +* The index where the device is located, -1 on error + */ +static int uec_miiphy_find_dev_by_name (char *devname) +{ +int i; + +for (i =0 ; i <MAXCONTROLLERS ; i++) { +if (strncmp (devname, devlist[i]->name,strlen (devname)) ==0 ) { +break ; + } + } + +/* If device cannot be found, returns -1 */ +if (i ==MAXCONTROLLERS ) { +debug ("%s: device %s not found in devlist\n" , __FUNCTION__, devname); + i =-1 ; + } + +return i; +} + +/* * Read a MII PHY register. * * Returns:
@@ -647,8 +672,16 @@ static void phy_change(struct eth_device *dev)static int uec_miiphy_read (char *devname,unsigned char addr,unsigned char reg,unsigned short *value) { - *value =uec_read_phy_reg (devlist[0 ], addr, reg); +int devindex =0 ; +if (devname ==NULL || value ==NULL ) { +debug ("%s: NULL pointer given\n" , __FUNCTION__); + }else { + devindex =uec_miiphy_find_dev_by_name (devname); +if (devindex >=0 ) { + *value =uec_read_phy_reg (devlist[devindex], addr, reg); + } + }return 0 ; }
@@ -661,11 +694,18 @@ static int uec_miiphy_read(char *devname, unsigned char addr,static int uec_miiphy_write (char *devname,unsigned char addr,unsigned char reg,unsigned short value) { -uec_write_phy_reg (devlist[0 ], addr, reg, value); +int devindex =0 ; +if (devname ==NULL ) { +debug ("%s: NULL pointer given\n" , __FUNCTION__); + }else { + devindex =uec_miiphy_find_dev_by_name (devname); +if (devindex >=0 ) { +uec_write_phy_reg (devlist[devindex], addr, reg, value); + } + }return 0 ; } -#endif static int uec_set_mac_address (uec_private_t *uec,u8 *mac_addr)
Signature · 1 line
1.5.5.GIT
PATCH v2 1/2 NET: QE: UEC: Make uec_miiphy_read() and uec_miiphy_write() use the devname arg.
Quoted 9 lines · richardretanubun
The current
uec_miiphy_readanduec_miiphy_writehardcode access devlist[0] This patch makes these function use the devname argument that is passed in to allow access to the phy registers of other devices in devlist[].Signed-of-by Richard Retanubun <[email protected]>---
Hi Ben,I'm hoping the 7th try will do it. I forgot to add the commit message before.
[snip]
Hi Ben,
Apologies for nagging, just checking if you get a chance to re-re-re-apply my 7th attempt? :)
Gratefully-Embarrassed,
- Richard
Ben Warren
AppliedPATCH v2 1/2 NET: QE: UEC: Make uec_miiphy_read() and uec_miiphy_write() use the devname arg.
Quoted 17 lines · richardretanubun
Quoted 12 lines · richardretanubun
The current
uec_miiphy_readanduec_miiphy_writehardcode access devlist[0]
This patch makes these function use the devname argument that is passed in to
allow access to the phy registers of other devices in devlist[].Signed-of-by Richard Retanubun <[email protected]>---
Hi Ben,I'm hoping the 7th try will do it. I forgot to add the commit message before.
[snip]
Quoted 9 lines
Hi Ben,
Apologies for nagging, just checking if you get a chance to re-re-re-apply my 7th attempt? :)Gratefully-Embarrassed,
- Richard
Applied to net/next. Lucky #7
cheers,
Ben