Re: [U-Boot] [PATCH v2] ARM: imx6: DHCOM i.MX6 PDK: Switch to DM for I2C
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, );
> >>
> >> 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@210/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, );
> > 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
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2] ARM: imx6: DHCOM i.MX6 PDK: Switch to DM for I2C
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, );
>>
>> 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@210/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, );
> 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.
--
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
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, );
>
> 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@210/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, );
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
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2] ARM: imx6: DHCOM i.MX6 PDK: Switch to DM for I2C
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, );
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.
[...]
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
