[PATCH 1/2] arm64: imx8mp: Read MAC address from M24C32-D write-lockable page on DH i.MX8MP DHCOM if available

[Marek Vasut]

On 10/17/24 1:09 PM, Christoph Niedermaier wrote:
> From: Marek Vasut <marex at denx.de>
> Sent: Wednesday, October 16, 2024 2:16 PM
>> On 10/16/24 1:57 PM, Christoph Niedermaier wrote:
>>> From: Marek Vasut <marex at denx.de>
>>> Sent: Saturday, October 12, 2024 10:43 PM
>>>> On 10/10/24 3:23 PM, Christoph Niedermaier wrote:
>>>>> The i.MX8M Plus DHCOM currently supports parsing ethernet MAC address
>>>>> from multiple sources in the following priority order:
>>>>>
>>>>> 1) U-Boot environment 'ethaddr'/'eth1addr' environment variable
>>>>> 2) SoC OTP fuses
>>>>> 3) On-SoM EEPROM
>>>>>
>>>>> The new i.MX8M Plus DHCOM rev.200 is populated with M24C32-D EEPROM
>>>>> which contains additional write-lockable page, which can also be
>>>>> populated with a structure containing ethernet MAC address.
>>>>>
>>>>> Add support for parsing the content of this new write-lockable page
>>>>> and place it between 2) and 3) on the priority list. The new entry is
>>>>> 2.5) On-SoM EEPROM write-lockable page
>>>>>
>>>>> Because the write-lockable page is not present on rev.100 i.MX8MP DHCOM
>>>>> SoM, test whether EEPROM ID page exists in DT and whether it is enabled
>>>>> first. If so, read the entire ID page out, validate it, and determine
>>>>> whether EEPROM MAC address is populated in it in DH specific format. If
>>>>> so, use the MAC address. There may be multiple EEPROMs with an ID page
>>>>> on this platform, always use the first one.
>>>>>
>>>>> Signed-off-by: Christoph Niedermaier <cniedermaier at dh-electronics.com>
>>>>> ---
>>>>> Cc: "NXP i.MX U-Boot Team" <uboot-imx at nxp.com>
>>>>> Cc: Marek Vasut <marex at denx.de>
>>>>> Cc: Fabio Estevam <festevam at gmail.com>
>>>>> Cc: Stefano Babic <sbabic at denx.de>
>>>>> Cc: Tom Rini <trini at konsulko.com>
>>>>> Cc: u-boot at dh-electronics.com
>>>>> ---
>>>>>     board/dhelectronics/common/dh_common.c        | 113 ++++++++++++++++++
>>>>>     board/dhelectronics/common/dh_common.h        |  23 ++++
>>>>>     .../dh_imx8mp/imx8mp_dhcom_pdk2.c             |   6 +
>>>>>     3 files changed, 142 insertions(+)
>>>>>
>>>>> diff --git a/board/dhelectronics/common/dh_common.c b/board/dhelectronics/common/dh_common.c
>>>>> index 32c50b4f0f..8ea70fc984 100644
>>>>> --- a/board/dhelectronics/common/dh_common.c
>>>>> +++ b/board/dhelectronics/common/dh_common.c
>>>>> @@ -7,9 +7,22 @@
>>>>>     #include <dm.h>
>>>>>     #include <i2c_eeprom.h>
>>>>>     #include <net.h>
>>>>> +#include <u-boot/crc.h>
>>>>>
>>>>>     #include "dh_common.h"
>>>>>
>>>>> +struct eeprom_id_page {
>>>>> +     u8      id[3];          /* Identifier 'D', 'H', 'E' - 'D' is at index 0 */
>>>>> +     u8      version;        /* 0x10 -- Version 1.0 */
>>>>> +     u8      data_crc16[2];  /* [1] is MSbyte */
>>>>> +     u8      header_crc8;
>>>>> +     u8      mac0[6];
>>>>> +     u8      mac1[6];
>>>>> +     u8      item_prefix;    /* H/F is coded in MSbits, Vendor coding starts at LSbits */
>>>>> +     u8      item_num[3];    /* [2] is MSbyte */
>>>>> +     u8      serial[9];      /* [8] is MSbyte */
>>>>> +} __packed;
>>>>
>>>> Is __packed needed ?
>>>
>>> I want to avoid padding in any case.
>> Every single field is u8, do you observe any padding ?
> 
> In this case, I don't expect padding, but if the struct is extended,
> padding could be possible.

Please add the padding only once it is really required, ideally the 
design of the structure or its extension(s) would be able to avoid such 
necessity altogether.

> This struct is for an EEPROM and therefore
> it should be defined with no padding. I don't want to give the compile
> the change to add padding if something changed in the struct.
> What are the disadvantages of keeping __packed here?
If it is unnecessary, then please remove it, there is no good 
justification for packing a structure which is naturally aligned already.