Hi Joe,
On 2 February 2015 at 17:38, Joe Hershberger joe.hershberger@ni.com wrote:
First just add support for MAC drivers.
Signed-off-by: Joe Hershberger joe.hershberger@ni.com
Changes in v2: -Updated comments -Removed extra parentheses -Changed eth_uclass_priv local var names to be uc_priv -Update error codes -Cause an invalid name to fail binding -Rebase on top of dm/master -Stop maintaining our own index and use DM seq now that it works for our needs -Move the hwaddr to platdata so that its memory is allocated at bind when we need it -Prevent device from being probed before used by a command (i.e. before eth_init()).
common/board_r.c | 4 +- common/cmd_bdinfo.c | 2 + include/dm/uclass-id.h | 1 + include/net.h | 24 ++++ net/eth.c | 319 ++++++++++++++++++++++++++++++++++++++++++++++++- 5 files changed, 346 insertions(+), 4 deletions(-)
diff --git a/common/board_r.c b/common/board_r.c index 68a9448..75147b7 100644 --- a/common/board_r.c +++ b/common/board_r.c @@ -556,7 +556,7 @@ static int initr_bbmii(void) } #endif
-#ifdef CONFIG_CMD_NET +#if defined(CONFIG_CMD_NET) && !defined(CONFIG_DM_ETH) static int initr_net(void) { puts("Net: "); @@ -825,7 +825,7 @@ init_fnc_t init_sequence_r[] = { #ifdef CONFIG_BITBANGMII initr_bbmii, #endif -#ifdef CONFIG_CMD_NET +#if defined(CONFIG_CMD_NET) && !defined(CONFIG_DM_ETH) INIT_FUNC_WATCHDOG_RESET initr_net, #endif diff --git a/common/cmd_bdinfo.c b/common/cmd_bdinfo.c index e6d8a7a..8688cf9 100644 --- a/common/cmd_bdinfo.c +++ b/common/cmd_bdinfo.c @@ -34,6 +34,7 @@ static void print_eth(int idx) printf("%-12s= %s\n", name, val); }
+#ifndef CONFIG_DM_ETH __maybe_unused static void print_eths(void) { @@ -52,6 +53,7 @@ static void print_eths(void) printf("current eth = %s\n", eth_get_name()); printf("ip_addr = %s\n", getenv("ipaddr")); } +#endif
__maybe_unused static void print_lnum(const char *name, unsigned long long value) diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h index 91bb90d..ad96682 100644 --- a/include/dm/uclass-id.h +++ b/include/dm/uclass-id.h @@ -34,6 +34,7 @@ enum uclass_id { UCLASS_I2C_GENERIC, /* Generic I2C device */ UCLASS_I2C_EEPROM, /* I2C EEPROM device */ UCLASS_MOD_EXP, /* RSA Mod Exp device */
UCLASS_ETH, /* Ethernet device */ UCLASS_COUNT, UCLASS_INVALID = -1,diff --git a/include/net.h b/include/net.h index 7eef9cc..4d21d91 100644 --- a/include/net.h +++ b/include/net.h @@ -78,6 +78,30 @@ enum eth_state_t { ETH_STATE_ACTIVE };
+#ifdef CONFIG_DM_ETH
You may not need this?
+struct eth_pdata {
phys_addr_t iobase;unsigned char enetaddr[6];+};
+struct eth_ops {
int (*init)(struct udevice *dev, bd_t *bis);int (*send)(struct udevice *dev, void *packet, int length);int (*recv)(struct udevice *dev);void (*halt)(struct udevice *dev);+#ifdef CONFIG_MCAST_TFTP
int (*mcast)(struct udevice *dev, const u8 *enetaddr, u8 set);+#endif
int (*write_hwaddr)(struct udevice *dev);+};
+struct udevice *eth_get_dev(void); /* get the current device */ +unsigned char *eth_get_ethaddr(void); /* get the current device MAC */ +int eth_init_state_only(bd_t *bis); /* Set active state */ +void eth_halt_state_only(void); /* Set passive state */ +#endif
+#ifndef CONFIG_DM_ETH struct eth_device { char name[16]; unsigned char enetaddr[6]; diff --git a/net/eth.c b/net/eth.c index c02548c..1b5a169 100644 --- a/net/eth.c +++ b/net/eth.c @@ -72,6 +72,320 @@ static int eth_mac_skip(int index) return ((skip_state = getenv(enetvar)) != NULL); }
+static void eth_current_changed(void);
+#ifdef CONFIG_DM_ETH +#include <dm.h> +#include <dm/device-internal.h>
These should go at the top of the file - should be OK to always include them (i.e. no #ifdef)
+struct eth_device_priv {
int state;void *priv;+};
+struct eth_uclass_priv {
struct udevice *current;+};
+static void eth_set_current_to_next(void) +{
struct uclass *uc;struct eth_uclass_priv *uc_priv;uclass_get(UCLASS_ETH, &uc);
This can actually return an error, although I agree there is little point in handling it. So I suppose this is OK.
uc_priv = uc->priv;uclass_next_device(&uc_priv->current);if (!uc_priv->current)uclass_first_device(UCLASS_ETH, &uc_priv->current);+}
+struct udevice *eth_get_dev(void) +{
struct uclass *uc;
blank line here
uclass_get(UCLASS_ETH, &uc);struct eth_uclass_priv *uc_priv = uc->priv;return uc_priv->current;+}
+static void eth_set_dev(struct udevice *dev) +{
struct uclass *uc;
blank line here
uclass_get(UCLASS_ETH, &uc);struct eth_uclass_priv *uc_priv = uc->priv;uc_priv->current = dev;+}
+unsigned char *eth_get_ethaddr(void) +{
struct eth_pdata *pdata;
blank line here
if (eth_get_dev()) {pdata = eth_get_dev()->platdata;if (pdata)return pdata->enetaddr;}return NULL;+}
+/* Set active state */ +int eth_init_state_only(bd_t *bis) +{
struct eth_device_priv *priv;
blank line here
if (eth_get_dev()) {priv = eth_get_dev()->uclass_priv;if (priv)priv->state = ETH_STATE_ACTIVE;}return 0;+} +/* Set passive state */ +void eth_halt_state_only(void) +{
struct eth_device_priv *priv;
blank line here
if (eth_get_dev()) {priv = eth_get_dev()->uclass_priv;if (priv)priv->state = ETH_STATE_PASSIVE;}+}
+int eth_get_dev_index(void) +{
if (eth_get_dev())return eth_get_dev()->seq;return -1;+}
+int eth_init(bd_t *bis) +{
struct udevice *current, *old_current, *dev;struct uclass *uc;current = eth_get_dev();if (!current) {puts("No ethernet found.\n");return -1;}device_probe(current);
Check error
/* Sync environment with network devices */uclass_get(UCLASS_ETH, &uc);uclass_foreach_dev(dev, uc) {uchar env_enetaddr[6];if (eth_getenv_enetaddr_by_index("eth", dev->seq,env_enetaddr)) {struct eth_pdata *pdata = dev->platdata;
blank line
Do all devices have the same platdata by design? What if a particular device wants its own?
if (pdata)
What do you need this check?
memcpy(pdata->enetaddr, env_enetaddr, 6);}};old_current = current;do {debug("Trying %s\n", current->name);if (current->driver) {const struct eth_ops *ops = current->driver->ops;
blank line
if (ops->init(current, bis) >= 0) {struct eth_device_priv *priv =current->uclass_priv;if (priv)priv->state = ETH_STATE_ACTIVE;return 0;}}debug("FAIL\n");eth_try_another(0);current = eth_get_dev();} while (old_current != current);return -ENODEV;+}
+void eth_halt(void) +{
struct udevice *current;current = eth_get_dev();if (!current)return;if (!current->driver)return;const struct eth_ops *ops = current->driver->ops;ops->halt(current);struct eth_device_priv *priv = current->uclass_priv;if (priv)priv->state = ETH_STATE_PASSIVE;+}
+int eth_send(void *packet, int length) +{
struct udevice *current;current = eth_get_dev();if (!current)return -1;if (!current->driver)return -1;
Seems like eth_get_dev() should return an error code if current or current->driver is NULL.
const struct eth_ops *ops = current->driver->ops;return ops->send(current, packet, length);+}
+int eth_rx(void) +{
struct udevice *current;current = eth_get_dev();if (!current)return -1;if (!current->driver)return -1;const struct eth_ops *ops = current->driver->ops;return ops->recv(current);+}
+static int eth_write_hwaddr(struct udevice *dev, const char *base_name,
int eth_number)+{
unsigned char env_enetaddr[6];int ret = 0;eth_getenv_enetaddr_by_index(base_name, eth_number, env_enetaddr);struct eth_pdata *pdata = dev->platdata;if (!is_zero_ether_addr(env_enetaddr)) {if (!is_zero_ether_addr(pdata->enetaddr) &&memcmp(pdata->enetaddr, env_enetaddr, 6)) {printf("\nWarning: %s MAC addresses don't match:\n",dev->name);printf("Address in SROM is %pM\n",pdata->enetaddr);printf("Address in environment is %pM\n",env_enetaddr);}memcpy(pdata->enetaddr, env_enetaddr, 6);} else if (is_valid_ether_addr(pdata->enetaddr)) {eth_setenv_enetaddr_by_index(base_name, eth_number,pdata->enetaddr);printf("\nWarning: %s using MAC address from net device\n",dev->name);} else if (is_zero_ether_addr(pdata->enetaddr)) {printf("\nError: %s address not set.\n",dev->name);return -EINVAL;}const struct eth_ops *ops = dev->driver->ops;if (ops->write_hwaddr && !eth_mac_skip(eth_number)) {if (!is_valid_ether_addr(pdata->enetaddr)) {printf("\nError: %s address %pM illegal value\n",dev->name, pdata->enetaddr);return -EINVAL;}ret = ops->write_hwaddr(dev);if (ret)printf("\nWarning: %s failed to set MAC address\n",dev->name);}
The above code seems mostly duplicated but I suppose it is hard to make it common?
return ret;+}
+static int eth_uclass_init(struct uclass *class) +{
bootstage_mark(BOOTSTAGE_ID_NET_ETH_START);eth_env_init();return 0;+}
+static int eth_post_bind(struct udevice *dev) +{
if (strchr(dev->name, ' ')) {printf("\nError: eth device name \"%s\" has a space!\n",dev->name);return -EINVAL;}if (!eth_get_dev()) {eth_set_dev(dev);eth_current_changed();}
I still don't really get what you are doing here. Perhaps you could add some comments? There should be no probed devices at this stage since you are binding only.
char *ethprime = getenv("ethprime");if (ethprime && strcmp(dev->name, ethprime) == 0) {eth_set_dev(dev);eth_current_changed();}/** Devices need to write the hwaddr even if not probed so that Linux* will have access to the hwaddr that u-boot stored for the device.*/eth_write_hwaddr(dev, "eth", uclass_resolve_seq(dev));return 0;+}
+static int eth_pre_unbind(struct udevice *dev) +{
struct udevice *first = NULL;struct udevice *current;struct uclass *uc;uclass_get(UCLASS_ETH, &uc);uclass_foreach_dev(current, uc) {if (!first)first = current;if (current == dev) {if (dev == first) {current = list_entry(current->uclass_node.next,struct udevice, uclass_node);} else {current = first;}eth_set_dev(current);eth_current_changed();}}
If this is turning down a device it really should happen in a remove() method. Maybe in post_remove()?
return 0;+}
+static int eth_post_probe(struct udevice *dev) +{
struct eth_device_priv *priv = dev->uclass_priv;if (priv)priv->state = ETH_STATE_INIT;return 0;+}
+UCLASS_DRIVER(eth) = {
.name = "eth",.id = UCLASS_ETH,.post_bind = eth_post_bind,.pre_unbind = eth_pre_unbind,.post_probe = eth_post_probe,.init = eth_uclass_init,.priv_auto_alloc_size = sizeof(struct eth_uclass_priv),.per_device_auto_alloc_size = sizeof(struct eth_device_priv),+}; +#endif
+#ifndef CONFIG_DM_ETH /*
- CPU and board-specific Ethernet initializations. Aliased function
- signals caller to move on
@@ -423,6 +737,7 @@ int eth_rx(void)
return eth_current->recv(eth_current);} +#endif /* ifndef CONFIG_DM_ETH */
#ifdef CONFIG_API static void eth_save_packet(void *packet, int length) @@ -486,7 +801,7 @@ static void eth_current_changed(void)
void eth_try_another(int first_restart) {
static struct eth_device *first_failed;
static void *first_failed; char *ethrotate; /*@@ -515,7 +830,7 @@ void eth_set_current(void) { static char *act; static int env_changed_id;
struct eth_device *old_current;
void *old_current; int env_id; if (!eth_get_dev()) /* XXX no current */-- 1.7.11.5
Regards, Simon