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

[Ludwig Zenz]

On 7/8/19 14:50 Marek Vasut wrote:
> 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 at 2100000/i2c at 21a8000/eeprom at 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.
> 

I will now send the solution suggested above with uclass_get_device_by_ofnode() as V3.
With this I want to avoid another device tree change for now and be closer to the Linux device tree.

Best regards,
Ludwig Zenz