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

[Christoph Niedermaier <cniedermaier@dh-electronics.com>]

From: Marek Vasut <marex@denx.de>
Sent: Wednesday, October 16, 2024 2:16 PM
> On 10/16/24 1:57 PM, Christoph Niedermaier wrote:
>> From: Marek Vasut <marex@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@dh-electronics.com>
>>>> ---
>>>> Cc: "NXP i.MX U-Boot Team" <uboot-imx@nxp.com>
>>>> Cc: Marek Vasut <marex@denx.de>
>>>> Cc: Fabio Estevam <festevam@gmail.com>
>>>> Cc: Stefano Babic <sbabic@denx.de>
>>>> Cc: Tom Rini <trini@konsulko.com>
>>>> Cc: u-boot@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. 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?

Regards
Christoph