[U-Boot] [U-Boot PATCH MX31:] smc911x MII made available
From: Helmut Raiger helmut.raiger@hale.at
The driver already had the MII functions, but they have not been registered using miiphy_register().
Signed-off-by: Helmut Raiger helmut.raiger@hale.at --- drivers/net/smc911x.c | 24 ++++++++++++++++++++++++ 1 files changed, 24 insertions(+), 0 deletions(-)
diff --git a/drivers/net/smc911x.c b/drivers/net/smc911x.c index aeafeba..9378a63 100644 --- a/drivers/net/smc911x.c +++ b/drivers/net/smc911x.c @@ -235,6 +235,25 @@ static int smc911x_rx(struct eth_device *dev) return 0; }
+#if defined(CONFIG_MII) || defined(CONFIG_CMD_MII) +/* wrapper for smc911x_miiphy_read */ +static int _phy_read(char *devname, u8 phy, u8 reg, u16 *val) +{ + struct eth_device *dev = eth_get_dev_by_name(devname); + if (dev) + return smc911x_miiphy_read(dev, phy, reg, val); + return -1; +} +/* wrapper for smc911x_miiphy_write */ +static int _phy_write(char *devname, u8 phy, u8 reg, u16 val) +{ + struct eth_device *dev = eth_get_dev_by_name(devname); + if (dev) + return smc911x_miiphy_write(dev, phy, reg, val); + return -1; +} +#endif + int smc911x_initialize(u8 dev_num, int base_addr) { unsigned long addrl, addrh; @@ -273,5 +292,10 @@ int smc911x_initialize(u8 dev_num, int base_addr) sprintf(dev->name, "%s-%hu", DRIVERNAME, dev_num);
eth_register(dev); + +#if defined(CONFIG_MII) || defined(CONFIG_CMD_MII) + miiphy_register(dev->name, _phy_read, _phy_write); +#endif + return 1; }
On 06/20/2011 08:10 AM, Helmut.Raiger@hale.at wrote:
From: Helmut Raiger helmut.raiger@hale.at
Hi Helmut,
The driver already had the MII functions, but they have not been registered using miiphy_register().
Signed-off-by: Helmut Raiger helmut.raiger@hale.at
Not noted before, thanks for fixing it. Only to remark the issue, on boards with SMC911x (at least the one I tested your patch) and CONFIG_CMD_MII set, a simple "mii info" returns "Read MDIO failed.."
You should always put in CC the maintainer for your patches. Because this patch is related to network, you should send your changes to Wolfgang Denk (Network Maintaner), too. I have already set his name in CC.
+#if defined(CONFIG_MII) || defined(CONFIG_CMD_MII) +/* wrapper for smc911x_miiphy_read */ +static int _phy_read(char *devname, u8 phy, u8 reg, u16 *val)
Is there some reason to use name starting with _ ? They have special meaning, and there is no need here.
I have tested your patch on the mx35pdk board.
Tested-by: Stefano Babic sbabic@denx.de
Best regards, Stefano Babic
On 06/20/2011 02:30 PM, Stefano Babic wrote:
Not noted before, thanks for fixing it. Only to remark the issue, on boards with SMC911x (at least the one I tested your patch) and CONFIG_CMD_MII set, a simple "mii info" returns "Read MDIO failed.."
Our board holds a SMSC LAN9211-ABZJ , 'mii info' returns:
PHY 0x00: OUI = 0x0000, Model = 0x00, Rev = 0x00, 10baseT, HDX PHY 0x01: OUI = 0x01F0, Model = 0x0C, Rev = 0x03, 100baseT, FDX PHY 0x02: OUI = 0x0000, Model = 0x00, Rev = 0x00, 10baseT, HDX PHY 0x03: OUI = 0x0000, Model = 0x00, Rev = 0x00, 10baseT, HDX PHY 0x04: OUI = 0x0000, Model = 0x00, Rev = 0x00, 10baseT, HDX PHY 0x05: OUI = 0x0000, Model = 0x00, Rev = 0x00, 10baseT, HDX PHY 0x06: OUI = 0x0000, Model = 0x00, Rev = 0x00, 10baseT, HDX PHY 0x07: OUI = 0x0000, Model = 0x00, Rev = 0x00, 10baseT, HDX PHY 0x08: OUI = 0x0000, Model = 0x00, Rev = 0x00, 10baseT, HDX PHY 0x09: OUI = 0x0000, Model = 0x00, Rev = 0x00, 10baseT, HDX PHY 0x0A: OUI = 0x0000, Model = 0x00, Rev = 0x00, 10baseT, HDX PHY 0x0B: OUI = 0x0000, Model = 0x00, Rev = 0x00, 10baseT, HDX PHY 0x0C: OUI = 0x0000, Model = 0x00, Rev = 0x00, 10baseT, HDX PHY 0x0D: OUI = 0x0000, Model = 0x00, Rev = 0x00, 10baseT, HDX PHY 0x0E: OUI = 0x0000, Model = 0x00, Rev = 0x00, 10baseT, HDX PHY 0x0F: OUI = 0x0000, Model = 0x00, Rev = 0x00, 10baseT, HDX PHY 0x10: OUI = 0x0000, Model = 0x00, Rev = 0x00, 10baseT, HDX PHY 0x11: OUI = 0x0000, Model = 0x00, Rev = 0x00, 10baseT, HDX PHY 0x12: OUI = 0x0000, Model = 0x00, Rev = 0x00, 10baseT, HDX PHY 0x13: OUI = 0x0000, Model = 0x00, Rev = 0x00, 10baseT, HDX PHY 0x14: OUI = 0x0000, Model = 0x00, Rev = 0x00, 10baseT, HDX PHY 0x15: OUI = 0x0000, Model = 0x00, Rev = 0x00, 10baseT, HDX PHY 0x16: OUI = 0x0000, Model = 0x00, Rev = 0x00, 10baseT, HDX PHY 0x17: OUI = 0x0000, Model = 0x00, Rev = 0x00, 10baseT, HDX PHY 0x18: OUI = 0x0000, Model = 0x00, Rev = 0x00, 10baseT, HDX PHY 0x19: OUI = 0x0000, Model = 0x00, Rev = 0x00, 10baseT, HDX PHY 0x1A: OUI = 0x0000, Model = 0x00, Rev = 0x00, 10baseT, HDX PHY 0x1B: OUI = 0x0000, Model = 0x00, Rev = 0x00, 10baseT, HDX PHY 0x1C: OUI = 0x0000, Model = 0x00, Rev = 0x00, 10baseT, HDX PHY 0x1D: OUI = 0x0000, Model = 0x00, Rev = 0x00, 10baseT, HDX PHY 0x1E: OUI = 0x0000, Model = 0x00, Rev = 0x00, 10baseT, HDX PHY 0x1F: OUI = 0x0000, Model = 0x00, Rev = 0x00, 10baseT, HDX
'mii device' says:
MII devices: 'smc911x-0' Current device: 'smc911x-0'
You should always put in CC the maintainer for your patches. Because this patch is related to network, you should send your changes to Wolfgang Denk (Network Maintaner), too. I have already set his name in CC.
Ok, I did search the MAINTAINER file, but could not attach a name to my fix, neither smc... nor network, nor something else.
+#if defined(CONFIG_MII) || defined(CONFIG_CMD_MII) +/* wrapper for smc911x_miiphy_read */ +static int _phy_read(char *devname, u8 phy, u8 reg, u16 *val)
Is there some reason to use name starting with _ ? They have special meaning, and there is no need here.
No, I wasn't aware of the _ meaning. I usually name wrappers that way, but only personal preference. I'll fix the naming along with other things that might come up.
I have tested your patch on the mx35pdk board.
Tested-by: Stefano Babicsbabic@denx.de
Best regards, Stefano Babic
Thanks for testing, Helmut
-- Scanned by MailScanner.
From: Helmut Raiger helmut.raiger@hale.at
The driver already had the MII functions, but they have not been registered using miiphy_register(). --- drivers/net/smc911x.c | 24 ++++++++++++++++++++++++ 1 files changed, 24 insertions(+), 0 deletions(-)
diff --git a/drivers/net/smc911x.c b/drivers/net/smc911x.c index aeafeba..8753588 100644 --- a/drivers/net/smc911x.c +++ b/drivers/net/smc911x.c @@ -235,6 +235,25 @@ static int smc911x_rx(struct eth_device *dev) return 0; }
+#if defined(CONFIG_MII) || defined(CONFIG_CMD_MII) +/* wrapper for smc911x_miiphy_read */ +static int mii_phy_read(char *devname, u8 phy, u8 reg, u16 *val) +{ + struct eth_device *dev = eth_get_dev_by_name(devname); + if (dev) + return smc911x_miiphy_read(dev, phy, reg, val); + return -1; +} +/* wrapper for smc911x_miiphy_write */ +static int mii_phy_write(char *devname, u8 phy, u8 reg, u16 val) +{ + struct eth_device *dev = eth_get_dev_by_name(devname); + if (dev) + return smc911x_miiphy_write(dev, phy, reg, val); + return -1; +} +#endif + int smc911x_initialize(u8 dev_num, int base_addr) { unsigned long addrl, addrh; @@ -273,5 +292,10 @@ int smc911x_initialize(u8 dev_num, int base_addr) sprintf(dev->name, "%s-%hu", DRIVERNAME, dev_num);
eth_register(dev); + +#if defined(CONFIG_MII) || defined(CONFIG_CMD_MII) + miiphy_register(dev->name, mii_phy_read, mii_phy_write); +#endif + return 1; }
On Monday, June 27, 2011 03:22:03 helmut.raiger@hale.at wrote:
From: Helmut Raiger helmut.raiger@hale.at
The driver already had the MII functions, but they have not been registered using miiphy_register().
missing signed-off-by tag
+static int mii_phy_read(char *devname, u8 phy, u8 reg, u16 *val)
this name is already in common name space in mii_phy.h. all driver funcs really should be prefixed anyways. so perhaps: smc911x_mii_phy_read -mike
From: Helmut Raiger helmut.raiger@hale.at
The driver already had the MII functions, but they have not been registered using miiphy_register().
Signed-off-by: Helmut Raiger helmut.raiger@hale.at --- drivers/net/smc911x.c | 36 ++++++++++++++++++++++++++++++------ 1 files changed, 30 insertions(+), 6 deletions(-)
diff --git a/drivers/net/smc911x.c b/drivers/net/smc911x.c index aeafeba..6cc236c 100644 --- a/drivers/net/smc911x.c +++ b/drivers/net/smc911x.c @@ -50,7 +50,7 @@ static void smc911x_handle_mac_address(struct eth_device *dev) printf(DRIVERNAME ": MAC %pM\n", m); }
-static int smc911x_miiphy_read(struct eth_device *dev, +static int smc911x_eth_phy_read(struct eth_device *dev, u8 phy, u8 reg, u16 *val) { while (smc911x_get_mac_csr(dev, MII_ACC) & MII_ACC_MII_BUSY) @@ -67,7 +67,7 @@ static int smc911x_miiphy_read(struct eth_device *dev, return 0; }
-static int smc911x_miiphy_write(struct eth_device *dev, +static int smc911x_eth_phy_write(struct eth_device *dev, u8 phy, u8 reg, u16 val) { while (smc911x_get_mac_csr(dev, MII_ACC) & MII_ACC_MII_BUSY) @@ -103,10 +103,10 @@ static void smc911x_phy_configure(struct eth_device *dev)
smc911x_phy_reset(dev);
- smc911x_miiphy_write(dev, 1, MII_BMCR, BMCR_RESET); + smc911x_eth_phy_write(dev, 1, MII_BMCR, BMCR_RESET); mdelay(1); - smc911x_miiphy_write(dev, 1, MII_ADVERTISE, 0x01e1); - smc911x_miiphy_write(dev, 1, MII_BMCR, BMCR_ANENABLE | + smc911x_eth_phy_write(dev, 1, MII_ADVERTISE, 0x01e1); + smc911x_eth_phy_write(dev, 1, MII_BMCR, BMCR_ANENABLE | BMCR_ANRESTART);
timeout = 5000; @@ -115,7 +115,7 @@ static void smc911x_phy_configure(struct eth_device *dev) if ((timeout--) == 0) goto err_out;
- if (smc911x_miiphy_read(dev, 1, MII_BMSR, &status) != 0) + if (smc911x_eth_phy_read(dev, 1, MII_BMSR, &status) != 0) goto err_out; } while (!(status & BMSR_LSTATUS));
@@ -235,6 +235,25 @@ static int smc911x_rx(struct eth_device *dev) return 0; }
+#if defined(CONFIG_MII) || defined(CONFIG_CMD_MII) +/* wrapper for smc911x_eth_phy_read */ +static int smc911x_miiphy_read(char *devname, u8 phy, u8 reg, u16 *val) +{ + struct eth_device *dev = eth_get_dev_by_name(devname); + if (dev) + return smc911x_eth_phy_read(dev, phy, reg, val); + return -1; +} +/* wrapper for smc911x_eth_phy_write */ +static int smc911x_miiphy_write(char *devname, u8 phy, u8 reg, u16 val) +{ + struct eth_device *dev = eth_get_dev_by_name(devname); + if (dev) + return smc911x_eth_phy_write(dev, phy, reg, val); + return -1; +} +#endif + int smc911x_initialize(u8 dev_num, int base_addr) { unsigned long addrl, addrh; @@ -273,5 +292,10 @@ int smc911x_initialize(u8 dev_num, int base_addr) sprintf(dev->name, "%s-%hu", DRIVERNAME, dev_num);
eth_register(dev); + +#if defined(CONFIG_MII) || defined(CONFIG_CMD_MII) + miiphy_register(dev->name, smc911x_miiphy_read, smc911x_miiphy_write); +#endif + return 1; }
Hi Helmut,
helmut.raiger@hale.at wrote:
From: Helmut Raigerhelmut.raiger@hale.at
The driver already had the MII functions, but they have not been registered using miiphy_register().
Signed-off-by: Helmut Raigerhelmut.raiger@hale.at
[...]
I implemented in the past, but never found the time to polish it for submission. It looks very similar to yours, and the actual code is the same.
Your version has more complete #ifdefs, mine has a more complete error checking.
I'm sending my patch in reply to this e-mail. Feel free to have a look and integrate my changes with yours.
Luca
-------------------------------------------------------------------------- Luca Ceresoli Comelit R&D - Progettazione Software
Phone Fax Mail Web YouTube
luca.ceresoli@comelit.it http://www.comelitgroup.com/ http://www.youtube.com/comelitgroup -------------------------------------------------------------------------- Prima di stampare pensa all'ambiente / Think about environment before printing AVVISO DI CONFIDENZIALITÀ Questo messaggio ed i suoi eventuali allegati può contenere informazioni confidenziali, di proprietà, legalmente protette.È destinato all'utilizzo esclusivo del destinatario sopra indicato; privatezza e confidenzialità non sono modificati da eventuali errori di trasmissione. Se il messaggio non è a lei destinato, non deve utilizzarlo, diffonderlo, copiarlo con qualunque mezzo, o intraprendere azioni basandosi su di esso. Se ha ricevuto questo messaggio per errore, la preghiamo di volerlo distruggere (unitamente ad eventuali copie dello stesso) e di volerci cortesemente informare del fatto scrivendo al mittente. CONFIDENTIALITY NOTICE This message and its attachments (if any) may contain confidential, proprietary or legally privileged information and it is intended only for the use of the addressee named above. No confidentiality or privilege is waived or lost by any erroneous transmission. If you are not the intended recipient of this message you are hereby notified that you must not use, disseminate, copy it in any form or take any action in reliance on it. If you have received this message in error, please, delete it (and any copies of it) and kindly inform the sender.
Signed-off-by: Luca Ceresoli luca.ceresoli@comelit.it --- This patch is my implementation of the same functionality that Helmut's patch implements. I'm sending it as a reference for him to look at the error checking code and possibly merge it into his own patch.
drivers/net/smc911x.c | 41 +++++++++++++++++++++++++++++++++++++++++ 1 files changed, 41 insertions(+), 0 deletions(-)
diff --git a/drivers/net/smc911x.c b/drivers/net/smc911x.c index aeafeba..d946fd4 100644 --- a/drivers/net/smc911x.c +++ b/drivers/net/smc911x.c @@ -82,6 +82,42 @@ static int smc911x_miiphy_write(struct eth_device *dev, return 0; }
+static int smc911x_miiphy_read_byname(char *devname, unsigned char addr, + unsigned char reg, unsigned short *value) +{ + struct eth_device *dev; + + if (devname == NULL) + return -1; + + dev = eth_get_dev_by_name(devname); + + if (dev == NULL) { + printf(DRIVERNAME ": device %s not found\n", devname); + return -1; + } + + return smc911x_miiphy_read(dev, addr, reg, value); +} + +static int smc911x_miiphy_write_byname(char *devname, unsigned char addr, + unsigned char reg, unsigned short value) +{ + struct eth_device *dev; + + if (devname == NULL) + return -1; + + dev = eth_get_dev_by_name(devname); + + if (dev == NULL) { + printf(DRIVERNAME ": device %s not found\n", devname); + return -1; + } + + return smc911x_miiphy_write(dev, addr, reg, value); +} + static int smc911x_phy_reset(struct eth_device *dev) { u32 reg; @@ -273,5 +309,10 @@ int smc911x_initialize(u8 dev_num, int base_addr) sprintf(dev->name, "%s-%hu", DRIVERNAME, dev_num);
eth_register(dev); + +#if defined(CONFIG_MII) || defined(CONFIG_CMD_MII) + miiphy_register(dev->name, smc911x_miiphy_read_byname, smc911x_miiphy_write_byname); +#endif + return 1; }
On 06/30/2011 04:02 PM, Luca Ceresoli wrote:
+static int smc911x_miiphy_read_byname(char *devname, unsigned char addr,
unsigned char reg, unsigned short *value)+{
- struct eth_device *dev;
- if (devname == NULL)
return -1;- dev = eth_get_dev_by_name(devname);
You're right. eth_get_dev_by_name() is not safe for devname == NULL as it uses strcmp(). Best would be to fix this there, I'll adjust my patch accordingly.
- if (dev == NULL) {
printf(DRIVERNAME ": device %s not found\n", devname);return -1;- }
None of the other drivers in drivers/net add this kind of verbosity, so I tend to leave it at that.
Helmut
-- Scanned by MailScanner.
From: Helmut Raiger helmut.raiger@hale.at
eth_get_dev_by_name() is not safe to use for devname being NULL as it uses strcmp. This patch makes it return NULL if devname NULL is passed.
Signed-off-by: Helmut Raiger helmut.raiger@hale.at --- net/eth.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/net/eth.c b/net/eth.c index 6523834..c75b7c9 100644 --- a/net/eth.c +++ b/net/eth.c @@ -107,7 +107,7 @@ struct eth_device *eth_get_dev_by_name(const char *devname) { struct eth_device *dev, *target_dev;
- if (!eth_devices) + if (!eth_devices || !devname) return NULL;
dev = eth_devices;
From: Helmut Raiger helmut.raiger@hale.at
The driver already had the MII functions, but they have not been registered using miiphy_register().
Signed-off-by: Helmut Raiger helmut.raiger@hale.at --- drivers/net/smc911x.c | 36 ++++++++++++++++++++++++++++++------ 1 files changed, 30 insertions(+), 6 deletions(-)
diff --git a/drivers/net/smc911x.c b/drivers/net/smc911x.c index aeafeba..6cc236c 100644 --- a/drivers/net/smc911x.c +++ b/drivers/net/smc911x.c @@ -50,7 +50,7 @@ static void smc911x_handle_mac_address(struct eth_device *dev) printf(DRIVERNAME ": MAC %pM\n", m); }
-static int smc911x_miiphy_read(struct eth_device *dev, +static int smc911x_eth_phy_read(struct eth_device *dev, u8 phy, u8 reg, u16 *val) { while (smc911x_get_mac_csr(dev, MII_ACC) & MII_ACC_MII_BUSY) @@ -67,7 +67,7 @@ static int smc911x_miiphy_read(struct eth_device *dev, return 0; }
-static int smc911x_miiphy_write(struct eth_device *dev, +static int smc911x_eth_phy_write(struct eth_device *dev, u8 phy, u8 reg, u16 val) { while (smc911x_get_mac_csr(dev, MII_ACC) & MII_ACC_MII_BUSY) @@ -103,10 +103,10 @@ static void smc911x_phy_configure(struct eth_device *dev)
smc911x_phy_reset(dev);
- smc911x_miiphy_write(dev, 1, MII_BMCR, BMCR_RESET); + smc911x_eth_phy_write(dev, 1, MII_BMCR, BMCR_RESET); mdelay(1); - smc911x_miiphy_write(dev, 1, MII_ADVERTISE, 0x01e1); - smc911x_miiphy_write(dev, 1, MII_BMCR, BMCR_ANENABLE | + smc911x_eth_phy_write(dev, 1, MII_ADVERTISE, 0x01e1); + smc911x_eth_phy_write(dev, 1, MII_BMCR, BMCR_ANENABLE | BMCR_ANRESTART);
timeout = 5000; @@ -115,7 +115,7 @@ static void smc911x_phy_configure(struct eth_device *dev) if ((timeout--) == 0) goto err_out;
- if (smc911x_miiphy_read(dev, 1, MII_BMSR, &status) != 0) + if (smc911x_eth_phy_read(dev, 1, MII_BMSR, &status) != 0) goto err_out; } while (!(status & BMSR_LSTATUS));
@@ -235,6 +235,25 @@ static int smc911x_rx(struct eth_device *dev) return 0; }
+#if defined(CONFIG_MII) || defined(CONFIG_CMD_MII) +/* wrapper for smc911x_eth_phy_read */ +static int smc911x_miiphy_read(char *devname, u8 phy, u8 reg, u16 *val) +{ + struct eth_device *dev = eth_get_dev_by_name(devname); + if (dev) + return smc911x_eth_phy_read(dev, phy, reg, val); + return -1; +} +/* wrapper for smc911x_eth_phy_write */ +static int smc911x_miiphy_write(char *devname, u8 phy, u8 reg, u16 val) +{ + struct eth_device *dev = eth_get_dev_by_name(devname); + if (dev) + return smc911x_eth_phy_write(dev, phy, reg, val); + return -1; +} +#endif + int smc911x_initialize(u8 dev_num, int base_addr) { unsigned long addrl, addrh; @@ -273,5 +292,10 @@ int smc911x_initialize(u8 dev_num, int base_addr) sprintf(dev->name, "%s-%hu", DRIVERNAME, dev_num);
eth_register(dev); + +#if defined(CONFIG_MII) || defined(CONFIG_CMD_MII) + miiphy_register(dev->name, smc911x_miiphy_read, smc911x_miiphy_write); +#endif + return 1; }
On 01/-10/-28163 08:59 PM, Helmut Raiger wrote:
From: Helmut Raiger helmut.raiger@hale.at
The driver already had the MII functions, but they have not been registered using miiphy_register().
Signed-off-by: Helmut Raiger helmut.raiger@hale.at
As for V1, tested on a MX35.
Tested-by: Stefano Babic sbabic@denx.de
Best regards, Stefano Babic
On 09/07/2011 02:33 PM, Stefano Babic wrote:
On 01/-10/-28163 08:59 PM, Helmut Raiger wrote:
From: Helmut Raigerhelmut.raiger@hale.at
The driver already had the MII functions, but they have not been registered using miiphy_register().
Signed-off-by: Helmut Raigerhelmut.raiger@hale.at
As for V1, tested on a MX35.
Tested-by: Stefano Babicsbabic@denx.de
Best regards, Stefano Babic
Thx Stefano, I do not know how to handle Tested-by: tags, however. Helmut
-- Scanned by MailScanner.
On 09/07/2011 03:06 PM, Helmut Raiger wrote:
Thx Stefano, I do not know how to handle Tested-by: tags, however.
Do not worry: you have not to handle. These tags are automatically inserted by patchworks in a patch by downloading and this helps the maintainer who does not need to insert them manually before applying the patch to the tree.
As submitter, nothing is requested to you to handle them ;-).
Best regards, Stefano Babic
Dear helmut.raiger@hale.at,
In message 1309775392-8282-2-git-send-email-helmut.raiger@hale.at you wrote:
From: Helmut Raiger helmut.raiger@hale.at
The driver already had the MII functions, but they have not been registered using miiphy_register().
Signed-off-by: Helmut Raiger helmut.raiger@hale.at
drivers/net/smc911x.c | 36 ++++++++++++++++++++++++++++++------ 1 files changed, 30 insertions(+), 6 deletions(-)
Applied, thanks.
Best regards,
Wolfgang Denk
Commit 6af1d41 "smc911x MII made available" was missing a few "const" qualifiers. Fix the resulting in build warnings:
smc911x.c: In function 'smc911x_initialize': smc911x.c:297: warning: passing argument 2 of 'miiphy_register' from incompatible pointer type smc911x.c:297: warning: passing argument 3 of 'miiphy_register' from incompatible pointer type
Signed-off-by: Wolfgang Denk wd@denx.de Cc: Helmut Raiger helmut.raiger@hale.at --- drivers/net/smc911x.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/smc911x.c b/drivers/net/smc911x.c index 6cc236c..a677fd4 100644 --- a/drivers/net/smc911x.c +++ b/drivers/net/smc911x.c @@ -237,7 +237,7 @@ static int smc911x_rx(struct eth_device *dev)
#if defined(CONFIG_MII) || defined(CONFIG_CMD_MII) /* wrapper for smc911x_eth_phy_read */ -static int smc911x_miiphy_read(char *devname, u8 phy, u8 reg, u16 *val) +static int smc911x_miiphy_read(const char *devname, u8 phy, u8 reg, u16 *val) { struct eth_device *dev = eth_get_dev_by_name(devname); if (dev) @@ -245,7 +245,7 @@ static int smc911x_miiphy_read(char *devname, u8 phy, u8 reg, u16 *val) return -1; } /* wrapper for smc911x_eth_phy_write */ -static int smc911x_miiphy_write(char *devname, u8 phy, u8 reg, u16 val) +static int smc911x_miiphy_write(const char *devname, u8 phy, u8 reg, u16 val) { struct eth_device *dev = eth_get_dev_by_name(devname); if (dev)
Dear Wolfgang Denk,
In message 1315479867-23506-1-git-send-email-wd@denx.de you wrote:
Commit 6af1d41 "smc911x MII made available" was missing a few "const" qualifiers. Fix the resulting in build warnings:
smc911x.c: In function 'smc911x_initialize': smc911x.c:297: warning: passing argument 2 of 'miiphy_register' from incompatible pointer type smc911x.c:297: warning: passing argument 3 of 'miiphy_register' from incompatible pointer type
Signed-off-by: Wolfgang Denk wd@denx.de Cc: Helmut Raiger helmut.raiger@hale.at
drivers/net/smc911x.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
Applied, thanks.
Best regards,
Wolfgang Denk
On Monday, July 04, 2011 06:29:51 helmut.raiger@hale.at wrote:
eth_get_dev_by_name() is not safe to use for devname being NULL as it uses strcmp. This patch makes it return NULL if devname NULL is passed.
i'm not sure about this. passing NULL is wrong, and the caller should catch that shouldnt it ? -mike
On 07/05/2011 05:44 AM, Mike Frysinger wrote:
On Monday, July 04, 2011 06:29:51 helmut.raiger@hale.at wrote:
eth_get_dev_by_name() is not safe to use for devname being NULL as it uses strcmp. This patch makes it return NULL if devname NULL is passed.
i'm not sure about this. passing NULL is wrong, and the caller should catch that shouldnt it ? -mike
So what is your suggestion how to deal with it?
It returns: "There is no ethernet device with name NULL"
This is pretty much the only thing it can return. The user of the function may handle this situation individually like:
printf("ethernet device '%s' not found\n, devname); --> "ethernet device '(NULL)' not found".
A panic on a NULL pointer de-reference is probably not helpful either.
Helmut
-- Scanned by MailScanner.
On Wednesday, July 06, 2011 03:15:08 Helmut Raiger wrote:
On 07/05/2011 05:44 AM, Mike Frysinger wrote:
On Monday, July 04, 2011 06:29:51 helmut.raiger@hale.at wrote:
eth_get_dev_by_name() is not safe to use for devname being NULL as it uses strcmp. This patch makes it return NULL if devname NULL is passed.
i'm not sure about this. passing NULL is wrong, and the caller should catch that shouldnt it ?
So what is your suggestion how to deal with it?
in what situation is eth_get_dev_by_name(NULL) being called ? my suggestion would be to fix that call point since it's doing something wrong. -mike
On 07/06/2011 09:38 PM, Mike Frysinger wrote:
On Wednesday, July 06, 2011 03:15:08 Helmut Raiger wrote:
On 07/05/2011 05:44 AM, Mike Frysinger wrote:
On Monday, July 04, 2011 06:29:51 helmut.raiger@hale.at wrote:
eth_get_dev_by_name() is not safe to use for devname being NULL as it uses strcmp. This patch makes it return NULL if devname NULL is passed.
i'm not sure about this. passing NULL is wrong, and the caller should catch that shouldnt it ?
So what is your suggestion how to deal with it?
in what situation is eth_get_dev_by_name(NULL) being called ? my suggestion would be to fix that call point since it's doing something wrong. -mike
I couldn't find a situation where this might be the case. But as Luca Ceresoli pointed out in his e-mail, somewhere up the thread, that he tested for devname being NULL in his miiphy_read and write routines, I checked eth_get_dev_by_name() and found that it is vulnerable to passing a NULL pointer, hence the fix.
Is there something missing for the patch to be acknowledged? It's hanging there quite a while now?
Helmut
-- Scanned by MailScanner.
Hi Helmut,
On 07/06/2011 09:38 PM, Mike Frysinger wrote:
On Wednesday, July 06, 2011 03:15:08 Helmut Raiger wrote:
On 07/05/2011 05:44 AM, Mike Frysinger wrote:
On Monday, July 04, 2011 06:29:51 helmut.raiger@hale.at wrote:
eth_get_dev_by_name() is not safe to use for devname being NULL as it uses strcmp. This patch makes it return NULL if devname NULL is passed.
i'm not sure about this. passing NULL is wrong, and the caller should catch that shouldnt it ?
So what is your suggestion how to deal with it?
in what situation is eth_get_dev_by_name(NULL) being called ? my suggestion would be to fix that call point since it's doing something wrong. -mike
I couldn't find a situation where this might be the case. But as Luca Ceresoli pointed out in his e-mail, somewhere up the thread, that he tested for devname being NULL in his miiphy_read and write routines, I checked eth_get_dev_by_name() and found that it is vulnerable to passing a NULL pointer, hence the fix.
Is there something missing for the patch to be acknowledged? It's hanging there quite a while now?
Actually I want to ack your patch. eth_get_dev_by_name is a project wide utility function and as such should be tolerant to being called with wrong parameters.
Mike's argument of course also has merit to it, but because of the "library function" argument, I'd give it less priority. So
Acked-by: Detlev Zundel dzu@denx.de
On Thu, Jul 7, 2011 at 02:12, Helmut Raiger wrote:
On 07/06/2011 09:38 PM, Mike Frysinger wrote:
On Wednesday, July 06, 2011 03:15:08 Helmut Raiger wrote:
On 07/05/2011 05:44 AM, Mike Frysinger wrote:
On Monday, July 04, 2011 06:29:51 helmut.raiger@hale.at wrote:
eth_get_dev_by_name() is not safe to use for devname being NULL as it uses strcmp. This patch makes it return NULL if devname NULL is passed.
i'm not sure about this. passing NULL is wrong, and the caller should catch that shouldnt it ?
So what is your suggestion how to deal with it?
in what situation is eth_get_dev_by_name(NULL) being called ? my suggestion would be to fix that call point since it's doing something wrong.
I couldn't find a situation where this might be the case. But as Luca Ceresoli pointed out in his e-mail, somewhere up the thread, that he tested for devname being NULL in his miiphy_read and write routines, I checked eth_get_dev_by_name() and found that it is vulnerable to passing a NULL pointer, hence the fix.
those NULL checks should not be necessary either. a correctly written networking driver should only register itself with the miiphy layer when it has successfully registered itself with the eth layer. thus any of the miiphy callbacks should always come in with a name that is found via eth_get_dev_by_name().
checking for NULLs here and gracefully returning is unnecessary overhead imo as you're only catering to broken code. fix the broken drivers instead.
by your logic, why put the NULL check in eth_get_dev_by_name() ? why not handle strcmp(NULL, NULL) too ? then eth_get_dev_by_name() would automatically get "fixed" as would all other call points. -mike
On 07/07/2011 07:46 PM, Mike Frysinger wrote:
those NULL checks should not be necessary either. a correctly written networking driver should only register itself with the miiphy layer when it has successfully registered itself with the eth layer. thus any of the miiphy callbacks should always come in with a name that is found via eth_get_dev_by_name().
You are right, in a perfect world nobody ever falters.
checking for NULLs here and gracefully returning is unnecessary overhead imo as you're only catering to broken code. fix the broken drivers instead.
I could not find drivers that have the potential of delivering NULLs to the miiphy_read and write functions, I simply refused to test at this level. Either there is a potential of having NULL passed then the test should be in eth_get_dev_by_name() or there is none then we should simply leave it away. I'm rather indifferent to either solution. And about 'catering to broken code': It must be broken in 2 ways, first it must pass a NULL to the function and second it must ignore the return code.
by your logic, why put the NULL check in eth_get_dev_by_name() ? why not handle strcmp(NULL, NULL) too ? then eth_get_dev_by_name() would automatically get "fixed" as would all other call points.
For strcmp() you have several issues at hand: Compatibility, performance and even a logical problem. What should be the result in case one of the arguments is NULL (or both). The logic for eth_get_dev_by_name() is completely sane "There is no ethernet device named (NULL)", performance and compatibility don't matter. Your comparison is flawed.
And finally we are discussing a few _chararacters_ of code and a probably a single assembly instruction.
Helmut
-- Scanned by MailScanner.
On Monday, July 11, 2011 05:53:49 Helmut Raiger wrote:
On 07/07/2011 07:46 PM, Mike Frysinger wrote:
those NULL checks should not be necessary either. a correctly written networking driver should only register itself with the miiphy layer when it has successfully registered itself with the eth layer. thus any of the miiphy callbacks should always come in with a name that is found via eth_get_dev_by_name().
You are right, in a perfect world nobody ever falters.
you missed the point. either the driver works, or it doesnt. this func is used in such a way that the behavior is fairly deterministic (as i clearly laid out). it isnt relying on allocated memory that could fall to be allocated for example.
checking for NULLs here and gracefully returning is unnecessary overhead imo as you're only catering to broken code. fix the broken drivers instead.
I could not find drivers that have the potential of delivering NULLs to the miiphy_read and write functions, I simply refused to test at this level. Either there is a potential of having NULL passed then the test should be in eth_get_dev_by_name() or there is none then we should simply leave it away.
i did go through the level of detail and showed the call graphs ... none of which should allow a driver tested as working to even once hit the NULL path.
I'm rather indifferent to either solution. And about 'catering to broken code': It must be broken in 2 ways, first it must pass a NULL to the function and second it must ignore the return code.
not necessarily. many platforms will abort on NULL pointer accesses.
by your logic, why put the NULL check in eth_get_dev_by_name() ? why not handle strcmp(NULL, NULL) too ? then eth_get_dev_by_name() would automatically get "fixed" as would all other call points.
For strcmp() you have several issues at hand: Compatibility, performance and even a logical problem. What should be the result in case one of the arguments is NULL (or both).
fair enough on the API, but your performance aspect is kind of hard to swallow when you turn around and say that NULL pointer checking elsewhere is minuscule.
And finally we are discussing a few _chararacters_ of code and a probably a single assembly instruction.
it's not a single assembly insn. try generating it. it adds like 8 to my platform, mostly because of the increased register pressure.
but the point isnt the impact of this single check. it sets the precedence that every function in u-boot that takes a pointer should start over protecting itself against poorly written code originating elsewhere. now your "few characters" is quite a bit more.
what i wouldnt mind is annotating the prototype with gcc attributes saying that the argument is nonnull. ... #define __nonnull(x) __attribute__((__nonnull__ x)) ... extern struct eth_device *eth_get_dev_by_name(const char *devname) __nonnull(1); ... -mike
Hi Mike,
On Monday, July 11, 2011 05:53:49 Helmut Raiger wrote:
On 07/07/2011 07:46 PM, Mike Frysinger wrote:
those NULL checks should not be necessary either. a correctly written networking driver should only register itself with the miiphy layer when it has successfully registered itself with the eth layer. thus any of the miiphy callbacks should always come in with a name that is found via eth_get_dev_by_name().
You are right, in a perfect world nobody ever falters.
you missed the point. either the driver works, or it doesnt. this func is used in such a way that the behavior is fairly deterministic (as i clearly laid out). it isnt relying on allocated memory that could fall to be allocated for example.
I for myself am also thinking of code that will be written in the future, i.e. probably new uses of eth_get_dev_by_name. Saving time in this development because errors are caught early is a good thing IMHO.
checking for NULLs here and gracefully returning is unnecessary overhead imo as you're only catering to broken code. fix the broken drivers instead.
I could not find drivers that have the potential of delivering NULLs to the miiphy_read and write functions, I simply refused to test at this level. Either there is a potential of having NULL passed then the test should be in eth_get_dev_by_name() or there is none then we should simply leave it away.
i did go through the level of detail and showed the call graphs ... none of which should allow a driver tested as working to even once hit the NULL path.
As I said, these are tha call graphs currently existing...
I'm rather indifferent to either solution. And about 'catering to broken code': It must be broken in 2 ways, first it must pass a NULL to the function and second it must ignore the return code.
not necessarily. many platforms will abort on NULL pointer accesses.
by your logic, why put the NULL check in eth_get_dev_by_name() ? why not handle strcmp(NULL, NULL) too ? then eth_get_dev_by_name() would automatically get "fixed" as would all other call points.
For strcmp() you have several issues at hand: Compatibility, performance and even a logical problem. What should be the result in case one of the arguments is NULL (or both).
fair enough on the API, but your performance aspect is kind of hard to swallow when you turn around and say that NULL pointer checking elsewhere is minuscule.
And finally we are discussing a few _chararacters_ of code and a probably a single assembly instruction.
it's not a single assembly insn. try generating it. it adds like 8 to my platform, mostly because of the increased register pressure.
On PowerPC with ELDK 4.2 this is the difference:
before:
00000004 g F .text.eth_get_dev_by_name 00000080 eth_get_dev_by_name
after:
00000004 g F .text.eth_get_dev_by_name 00000084 eth_get_dev_by_name
So at least for this arch it is indeed one word difference.
but the point isnt the impact of this single check. it sets the precedence that every function in u-boot that takes a pointer should start over protecting itself against poorly written code originating elsewhere. now your "few characters" is quite a bit more.
I still stand by what I said that if we have functions that can be called from many places (i.e. "library"-like), then the functions should be conservative in what they expect. Tightly coupled code can be looser in this respect. Maybe our disagreement stems from the fact that you consider this function to be "tightly coupled" and not really library like?
what i wouldnt mind is annotating the prototype with gcc attributes saying that the argument is nonnull. ... #define __nonnull(x) __attribute__((__nonnull__ x)) ... extern struct eth_device *eth_get_dev_by_name(const char *devname) __nonnull(1); ...
This can only catch calls the compiler can statically derive, but still I think it is a good thing.
Cheers Detlev
On Tue, Jul 12, 2011 at 05:22, Detlev Zundel wrote:
Mike Frysinger wrote:
but the point isnt the impact of this single check. it sets the precedence that every function in u-boot that takes a pointer should start over protecting itself against poorly written code originating elsewhere. now your "few characters" is quite a bit more.
I still stand by what I said that if we have functions that can be called from many places (i.e. "library"-like), then the functions should be conservative in what they expect. Tightly coupled code can be looser in this respect. Maybe our disagreement stems from the fact that you consider this function to be "tightly coupled" and not really library like?
not really. i consider this to be "garbage-in garbage-out". imo, u-boot isnt a C library that should be padded with garbage checking all over. the result only helps broken systems (edge cases) while hindering the rest.
i wouldnt have a problem with adopting an NDEBUG system, or perhaps adding assert()'s to this code. then people can easily opt-out of it all and for the people doing development, can easily turn things on. assert(name != NULL);
the current miiphy system needs to be replaced (this runtime string based approach is crazy), but that's a completely different topic :). -mike
Hi Mike,
[...]
not really. i consider this to be "garbage-in garbage-out". imo, u-boot isnt a C library that should be padded with garbage checking all over. the result only helps broken systems (edge cases) while hindering the rest.
i wouldnt have a problem with adopting an NDEBUG system, or perhaps adding assert()'s to this code. then people can easily opt-out of it all and for the people doing development, can easily turn things on. assert(name != NULL);
While developing, I certainly appreciate 'garbage-in error-out' and as your approach allows this, I believe we have reached a consensus.
the current miiphy system needs to be replaced (this runtime string based approach is crazy), but that's a completely different topic :).
I'm looking forward to your changes :)
Thanks! Detlev
On 07/12/2011 11:22 AM, Detlev Zundel wrote:
i did go through the level of detail and showed the call graphs ... none of which should allow a driver tested as working to even once hit the NULL path.
As I said, these are the call graphs currently existing...
This was also my trail.
what i wouldnt mind is annotating the prototype with gcc attributes saying that the argument is nonnull. ... #define __nonnull(x) __attribute__((__nonnull__ x)) ... extern struct eth_device *eth_get_dev_by_name(const char *devname) __nonnull(1); ...
This can only catch calls the compiler can statically derive, but still I think it is a good thing.
__nonnull__ is actually a optimization attribute, gcc removes tests for NULL in the function body, warnings are only generated if one literally writes: eth_get_dev_by_name(NULL), so 'statically derive' is already exageration. This really is no help at all. It would indeed establish a precendence to using an IMHO quite flawed attribute in gcc. If I had a vote, I'd be against it.
The NDEBUG approach however, as Mike suggested, was what I was looking for in the first place.
Helmut
-- Scanned by MailScanner.
Hi Helmut,
On 07/12/2011 11:22 AM, Detlev Zundel wrote:
i did go through the level of detail and showed the call graphs ... none of which should allow a driver tested as working to even once hit the NULL path.
As I said, these are the call graphs currently existing...
This was also my trail.
what i wouldnt mind is annotating the prototype with gcc attributes saying that the argument is nonnull. ... #define __nonnull(x) __attribute__((__nonnull__ x)) ... extern struct eth_device *eth_get_dev_by_name(const char *devname) __nonnull(1); ...
This can only catch calls the compiler can statically derive, but still I think it is a good thing.
__nonnull__ is actually a optimization attribute, gcc removestests for NULL in the function body, warnings are only generated if one literally writes: eth_get_dev_by_name(NULL), so 'statically derive' is already exageration.
I just checked and can confirm that currently gcc does not do any static analysis of char* arguments - however in theory it could.
This really is no help at all. It would indeed establish a precendence to using an IMHO quite flawed attribute in gcc. If I had a vote, I'd be against it.
I agree that how this is implemented in gcc is no big help. Rather than believing documentation I should have checked how this works before lobbying for it.
The NDEBUG approach however, as Mike suggested, was what I was looking for in the first place.
Great! Detlev
On 07/13/2011 01:46 PM, Detlev Zundel wrote:
The NDEBUG approach however, as Mike suggested, was what I was looking for in the first place.
Great! Detlev
Again, not so great. U-boot uses all kinds of assert(), BUG_ON(), ASSERT() all over the place. This probably needs a project wide effort, which is why I simply threw in my NULL pointer check.
Helmut
-- Scanned by MailScanner.
On Thursday, July 14, 2011 05:14:07 Helmut Raiger wrote:
On 07/13/2011 01:46 PM, Detlev Zundel wrote:
The NDEBUG approach however, as Mike suggested, was what I was looking for in the first place.
Again, not so great. U-boot uses all kinds of assert(), BUG_ON(), ASSERT() all over the place. This probably needs a project wide effort, which is why I simply threw in my NULL pointer check.
we tend to look at the long term picture with the best/correct solution rather than one-offs which get thrown away in the end -mike
eth_get_dev_by_name() is not safe to use for devname being NULL as it uses strcmp. This patch makes it fail with a BUG().
Signed-off-by: Helmut Raiger helmut.raiger@hale.at --- V2: use BUG_ON() instead of gracefully returning 0
net/eth.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/net/eth.c b/net/eth.c index 6523834..25c147c 100644 --- a/net/eth.c +++ b/net/eth.c @@ -107,6 +107,8 @@ struct eth_device *eth_get_dev_by_name(const char *devname) { struct eth_device *dev, *target_dev;
+ BUG_ON(devname == 0); + if (!eth_devices) return NULL;
Hello.
On 22-08-2011 12:45, Helmut Raiger wrote:
eth_get_dev_by_name() is not safe to use for devname being NULL as it uses strcmp. This patch makes it fail with a BUG().
Signed-off-by: Helmut Raigerhelmut.raiger@hale.at
V2: use BUG_ON() instead of gracefully returning 0
net/eth.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/net/eth.c b/net/eth.c index 6523834..25c147c 100644 --- a/net/eth.c +++ b/net/eth.c @@ -107,6 +107,8 @@ struct eth_device *eth_get_dev_by_name(const char *devname) { struct eth_device *dev, *target_dev;
- BUG_ON(devname == 0);
Do not use 0 instead of NULL.
WBR, Sergei
eth_get_dev_by_name() is not safe to use for devname being NULL as it uses strcmp. This patch makes it fail with a BUG().
Signed-off-by: Helmut Raiger helmut.raiger@hale.at --- V2: use BUG_ON() instead of gracefully returning 0
net/eth.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/net/eth.c b/net/eth.c index 6523834..deb41ba 100644 --- a/net/eth.c +++ b/net/eth.c @@ -107,6 +107,8 @@ struct eth_device *eth_get_dev_by_name(const char *devname) { struct eth_device *dev, *target_dev;
+ BUG_ON(devname == NULL); + if (!eth_devices) return NULL;
On Monday, August 22, 2011 06:17:17 Helmut Raiger wrote:
eth_get_dev_by_name() is not safe to use for devname being NULL as it uses strcmp. This patch makes it fail with a BUG().
Acked-by: Mike Frysinger vapier@gentoo.org -mike
Dear Helmut Raiger,
In message 1314008237-24180-1-git-send-email-helmut.raiger@hale.at you wrote:
eth_get_dev_by_name() is not safe to use for devname being NULL as it uses strcmp. This patch makes it fail with a BUG().
Signed-off-by: Helmut Raiger helmut.raiger@hale.at
V2: use BUG_ON() instead of gracefully returning 0
net/eth.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
Applied, thanks.
Best regards,
Wolfgang Denk
On Wednesday, July 13, 2011 02:32:37 Helmut Raiger wrote:
for future reference, please dont send html e-mails -mike
Hi Helmut,
Le 04/07/2011 12:29, helmut.raiger@hale.at a écrit :
From: Helmut Raigerhelmut.raiger@hale.at
Seems like your git send-email config does not have your name along with your e-mail address, causing this From: to appear in the patch body (and the mail itself to lack your name) -- git can handle this, I think, but I'd be grateful if you fixed your config.
Amicalement,
On 07/07/2011 06:46 PM, Albert ARIBAUD wrote:
Hi Helmut,
Le 04/07/2011 12:29, helmut.raiger@hale.at a écrit :
From: Helmut Raigerhelmut.raiger@hale.at
Seems like your git send-email config does not have your name along with your e-mail address, causing this From: to appear in the patch body (and the mail itself to lack your name) -- git can handle this, I think, but I'd be grateful if you fixed your config.
Amicalement,
Hi Albert,
I just checked my config, user and e-mail were fine, but I had the from configured in the sendemail section which obviously generates the 'From:' line. Do you like me to resend the patches?
Helmut
-- Scanned by MailScanner.
Hi Helmut,
Le 11/07/2011 12:10, Helmut Raiger a écrit :
On 07/07/2011 06:46 PM, Albert ARIBAUD wrote:
Hi Helmut,
Le 04/07/2011 12:29, helmut.raiger@hale.at a écrit :
From: Helmut Raigerhelmut.raiger@hale.at
Seems like your git send-email config does not have your name along with your e-mail address, causing this From: to appear in the patch body (and the mail itself to lack your name) -- git can handle this, I think, but I'd be grateful if you fixed your config.
Amicalement,
Hi Albert,
I just checked my config, user and e-mail were fine, but I had the from configured in the sendemail section which obviously generates the 'From:' line. Do you like me to resend the patches?
Not needed for this patch (if that From: is ok with patchwork and Git) but if you send a V2 patch or for future patches, please do avoid the added From.
Amicalement,
Is there anything missing for this patch to be accepted?
Helmut
-- Scanned by MailScanner.
Dear Helmut Raiger,
In message 4E5DE5A2.70603@hale.at you wrote:
Is there anything missing for this patch to be accepted?
Yes, definitely.
for example, you fail to mention which patch you are talking about.
Best regards,
Wolfgang Denk
This is sitting here for more than 2 months, could someone please ACK and/or apply.
Helmut
-- Scanned by MailScanner.
On 09/07/2011 07:40 AM, Helmut Raiger wrote:
Hi Helmut,
This is sitting here for more than 2 months, could someone please ACK and/or apply.
I think that one reason for the delay is that you do not add the network maintainer (Wolfgang) as CC in V2 of your patch - I missed also this patch, and I send now after testing my tested-by as I did for V1.
Best regards, Stefano Babic
Dear Stefano Babic,
In message 4E676571.5090003@denx.de you wrote:
On 09/07/2011 07:40 AM, Helmut Raiger wrote:
Hi Helmut,
This is sitting here for more than 2 months, could someone please ACK and/or apply.
I think that one reason for the delay is that you do not add the network maintainer (Wolfgang) as CC in V2 of your patch - I missed also this patch, and I send now after testing my tested-by as I did for V1.
Even more so: the patch is flagged as for MX31, so I don;t even look at it.
Best regards,
Wolfgang Denk
On 09/07/2011 11:47 PM, Wolfgang Denk wrote:
Dear Stefano Babic,
In message4E676571.5090003@denx.de you wrote:
On 09/07/2011 07:40 AM, Helmut Raiger wrote:
Hi Helmut,
This is sitting here for more than 2 months, could someone please ACK and/or apply.
I think that one reason for the delay is that you do not add the network maintainer (Wolfgang) as CC in V2 of your patch - I missed also this patch, and I send now after testing my tested-by as I did for V1.
Even more so: the patch is flagged as for MX31, so I don;t even look at it.
Best regards,
Wolfgang Denk
Damn, yes, my fault. In the heat of the action ... Thx, Helmut
-- Scanned by MailScanner.
participants (10)
-
Albert ARIBAUD -
Detlev Zundel -
Helmut Raiger -
Helmut.Raiger＠hale.at
-
helmut.raiger＠hale.at
-
Luca Ceresoli -
Mike Frysinger -
Sergei Shtylyov -
Stefano Babic -
Wolfgang Denk