Re: [PATCH 4/5] ARM: novena: Read Ethernet MAC address from EEPROM

[Sascha Hauer <sha@pengutronix.de>]

On Wed, Jan 25, 2023 at 05:35:53AM +1100, John Watts wrote:
> On Mon, Jan 23, 2023 at 10:33:05AM +0100, Sascha Hauer wrote:
> > And here is the point where you have to request that the EEPROM is
> > actually availabe to support the deep probe mechanism. Before reading
> > the EEPROM add a call to:
> > 
> > 	of_device_ensure_probed_by_alias("eeprom0");
> > 
> > Nowadays the read-MAC-address-from-EEPROM thingy would likely be done
> > using nvmem cells which would boil this code down to device tree changes
> > only. If you're feeling brave you could change this, I won't insist on
> > it though.
> > 
> > Sascha
> 
> Hey Sascha,
> 
> I did some refactoring today based on your suggestions and now have this code:
> 
>         rc = of_device_ensure_probed_by_alias("eeprom0");
> 
>         if (rc < 0) {
>                 pr_err("Unable to probe Novena EEPROM: %s\n", strerror(-rc));
>                 return NULL;
>         }
> 
>         rc = read_file_2("/dev/eeprom0", &read, &eeprom, max);
> 
> This gets me this bootup sequence:
> 
>   of_platform_device_create: register device 21a8000.i2c@21a8000.of, io=0x021a8000
>   register_device: 21a8000.i2c@21a8000.of
>           probe-> 21a8000.i2c@21a8000.of
>   imx-iomuxv3 20e0000.pinctrl@20e0000.of: set state: /soc/bus@2000000/pinctrl@20e0000/i2c3grp-novena
>   __request_region ok: 0x021a8000:0x021abfff flags=0x0
>   of_get_named_gpio_flags: cannot parse sda-gpios property: -2
>   of_get_named_gpio_flags: cannot parse scl-gpios property: -2

That's expected because you haven't specified sda-gpios or scl-gpios.
The message is only printed at debug level after all.

>   <NULL>: <i2c_fsl_set_clk> I2C_CLK=66000000, REQ DIV=660
>   <NULL>: <i2c_fsl_set_clk> IFDR[IC]=0x39, REAL DIV=768

Shouldn't be <NULL>. I have sent a patch for this a moment ago.
This is not your problem though.

>   register_device: i2c0
>   register_device: es83280
>   es83280: registered on bus 0, chip->addr 0x11
>   register_device: 24c5120
>               probe-> 24c5120
>   of_get_named_gpio_flags: cannot parse wp-gpios property: -2
>   at24 24c5120: Registering nvmem device eeprom0
>   register_device: eeprom00
>   at24 24c5120: registered on bus 0, chip->addr 0x56
>   eeprom00: read ofs: 0x00000000 count: 0x00000014
>   i2c0: master_xfer[0] W, addr=0x56, len=2
>   i2c0: master_xfer[1] R, addr=0x56, len=20
>   i2c0: timeout waiting for status set 0x20, cur status: 0x93
>   at24 24c5120: read 20@0 --> -95 (1046280961)

-95 (Operation not supported) is the return value of i2c_recover_bus()
when no bus recovery is implemented. The return value is confusing, I
sent a patch changing this to -EBUSY instead as the kernel does.
This also doesn't point us to the problem.

>   Unable to read Novena EEPROM: Connection timed out
> 
> After the shell boots I'm able to read /dev/eeprom0 just fine.

It looks like something is not initialized correctly yet. Normally
the usual suspects are pinctrl, clocks or regulators. Each of them
should be tied in by the deep probe mechanism as needed already, at
least when it's properly registered in the device tree. Clock looks
correct (66MHz), for the (pfuze) regulator I can't find anything in the
board code that configures something there later. You could verify
that the pinctrl has been configured already by activating the debugging
in drivers/pinctrl/imx-iomux-v3.c.
It could be something that pulls SDA low, but I don't have an idea why
it should work later.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |