Re: [PATCH v2 2/2] net: ethernet: Add driver for Sunplus SP7021

quoted

quoted

+static const char def_mac_addr[ETHERNET_MAC_ADDR_LEN] = {
+	0xfc, 0x4b, 0xbc, 0x00, 0x00, 0x00

This does not have the locally administered bit set. Should it? Or is this and address
from your OUI?

This is default MAC address when MAC address in NVMEM is not found.
Fc:4b:bc:00:00:00 is OUI of "Sunplus Technology Co., Ltd.".
Can I keep this? or it should be removed?

Please add a comment about whos OUI it is.

It is however more normal to use a random MAC address if no other MAC
address is available. That way, you avoid multiple devices on one LAN
using the same default MAC address.

quoted

quoted

+	if (mac->next_ndev) {
+		struct net_device *ndev2 = mac->next_ndev;
+
+		if (!netif_carrier_ok(ndev2) && (reg & PORT_ABILITY_LINK_ST_P1)) {
+			netif_carrier_on(ndev2);
+			netif_start_queue(ndev2);
+		} else if (netif_carrier_ok(ndev2) && !(reg & PORT_ABILITY_LINK_ST_P1)) {
+			netif_carrier_off(ndev2);
+			netif_stop_queue(ndev2);
+		}

Looks very odd. The two netdev should be independent.

I don't understand your comment.
ndev checks PORT_ABILITY_LINK_ST_P0
ndev2 checks PORT_ABILITY_LINK_ST_P1
They are independent already.

I would try to remove the mac->next_ndev. I think without that, you
will get a cleaner abstraction. You might want to keep an array of mac
pointers in your top level shared structure.

	 Andrew