Re: [U-Boot] [PATCH v2] ARM: imx6: DHCOM i.MX6 PDK: Switch to DM for I2C

On 7/5/19 14:24 PM, Marek Vasut wrote:
On 7/5/19 12:46 PM, Ludwig Zenz wrote: [...]
static int setup_dhcom_mac_from_fuse(void) {
- struct udevice *dev; unsigned char enetaddr[6]; int ret;
@@ -228,13 +142,13 @@ static int setup_dhcom_mac_from_fuse(void) return 0; }
- ret = i2c_set_bus_num(2);
- ret = uclass_first_device_err(UCLASS_I2C_EEPROM, &dev);
What about uclass_find_device_by_of_node() ? Then you can specify a stable fixed hardware path to the EEPROM, and no matter how many EEPROMs someone connects to the board, this code won't be affected.
[...]
The function uclass_find_device_by_ofnode() is used only once in U-Boot and the declaration is in 'include/dh/uclass-interal.h'. I found a similar function called uclass_get_device_by_ofnode(), which is used in several places.
Unfortunately I didn't manage to work out the difference. What I don't like is that the former is from uclass-internal.h. --------
A possible implementation would be as follows:
[...] struct udevice *dev; ofnode eeprom; unsigned char enetaddr[6]; int ret;
[...]
eeprom = ofnode_path("/soc/aips-bus@2100000/i2c@21a8000/eeprom@50"); if (!ofnode_valid(eeprom)) { printf("Invalid hardware path to EEPROM!\n"); return -ENODEV; }
ret = uclass_get_device_by_ofnode(UCLASS_I2C_EEPROM, eeprom, &dev); if (ret) { printf("Cannot find EEPROM!\n"); return ret; }
ret = i2c_eeprom_read(dev, 0xfa, enetaddr, 0x6); if (ret) { printf("Error reading configuration EEPROM!\n"); return ret; }
[...]
We're lucky here that the Duallite and Quad mapped the I2C controllers on the same addresses. But I'm not sure if this is really a good implementation.
Would it be advantageous to make a device tree entry in /chosen for the MAC EEPROM instead? Then we could do something like:
eeprom = ofnode_get_chosen_node("mac,eeprom");
I'd be happy to get feedback on that.
regards, Ludwig Zenz

On 7/8/19 10:21 AM, Ludwig Zenz wrote:
On 7/5/19 14:24 PM, Marek Vasut wrote:
On 7/5/19 12:46 PM, Ludwig Zenz wrote: [...]
static int setup_dhcom_mac_from_fuse(void) {
- struct udevice *dev; unsigned char enetaddr[6]; int ret;
@@ -228,13 +142,13 @@ static int setup_dhcom_mac_from_fuse(void) return 0; }
- ret = i2c_set_bus_num(2);
- ret = uclass_first_device_err(UCLASS_I2C_EEPROM, &dev);
What about uclass_find_device_by_of_node() ? Then you can specify a stable fixed hardware path to the EEPROM, and no matter how many EEPROMs someone connects to the board, this code won't be affected.
[...]
The function uclass_find_device_by_ofnode() is used only once in U-Boot and the declaration is in 'include/dh/uclass-interal.h'. I found a similar function called uclass_get_device_by_ofnode(), which is used in several places.
Unfortunately I didn't manage to work out the difference. What I don't like is that the former is from uclass-internal.h. --------
A possible implementation would be as follows:
[...] struct udevice *dev; ofnode eeprom; unsigned char enetaddr[6]; int ret;
[...]
eeprom = ofnode_path("/soc/aips-bus@2100000/i2c@21a8000/eeprom@50"); if (!ofnode_valid(eeprom)) { printf("Invalid hardware path to EEPROM!\n"); return -ENODEV; }
ret = uclass_get_device_by_ofnode(UCLASS_I2C_EEPROM, eeprom, &dev); if (ret) { printf("Cannot find EEPROM!\n"); return ret; }
ret = i2c_eeprom_read(dev, 0xfa, enetaddr, 0x6); if (ret) { printf("Error reading configuration EEPROM!\n"); return ret; }
[...]
We're lucky here that the Duallite and Quad mapped the I2C controllers on the same addresses. But I'm not sure if this is really a good implementation.
Would it be advantageous to make a device tree entry in /chosen for the MAC EEPROM instead? Then we could do something like:
eeprom = ofnode_get_chosen_node("mac,eeprom");
I'd be happy to get feedback on that.
The correct solution would be to add some sort of link from the ethernet MAC to the EEPROM, maybe via nvmem or somesuch. But I don't think U-Boot implements that.
I don't have any better suggestion, maybe someone else does.
participants (2)
-
Ludwig Zenz
-
Marek Vasut