[PATCH v3 11/16] board: ti: j784s4: Add support for board detection by EEPROM read

[Andrew Davis]

On 9/8/23 12:03 PM, Nishanth Menon wrote:
> On 10:59-20230908, Andrew Davis wrote:
>> On 9/8/23 9:42 AM, Nishanth Menon wrote:
>>> On 16:35-20230908, Apurva Nandan wrote:
>>>> From: Dasnavis Sabiya <sabiya.d at ti.com>
>>>>
>>>> The board name is programmed in the EEPROM. Add support for board
>>>> detection and choose the right dtb based on the board name read
>>>> from EEPROM.
>>>>
>>>> The J784S4/AM69 has two platforms naming J784S4-EVM and AM69-SK. The
>>>> J784S4 has EEPROM populated at 0x50. AM69 SK has EEPROM
>>>> populated at next address 0x51. So start looking for TI specific EEPROM
>>>> at 0x50, if not found look for EEPROM at 0x51.
>>>>
>>>> Also, add support for setup_serial() to read the serial number from EEPROM
>>>> and update the serial environment variable with the same.
>>>>
>>>> Signed-off-by: Dasnavis Sabiya <sabiya.d at ti.com>
>>>> Signed-off-by: Apurva Nandan <a-nandan at ti.com>
>>>
>>>
>>> NAK. go with the approach similar to AM62x. AM69-SK has it's own
>>> fragment.
>>>
>>
>> NAK to your NAK, there is no reason to introduce more fragments here
>> when both boards can be supported with one defconfig.
> 
> Sorry, but no. the evm.c has become a mess. Time to stop it is in the
> introduction itself
> 
> Need the eeprom stuff clearly pulled out of evm.c -> evm.c should still
> work with CONFIG_EEPROM_DETECT disabled and should be board independent.
> 

Sure evm.c is a mess, but that is the price for one config/build working
on multiple boards. That is why we have EEPROM on *every* EVM, you throw
that away if you got to "just have a different config/build for each EVM".

Honestly I'm fine either way (I never did think we got much out of
multi-board detection support), but that decision should come with more
reasoning than "NAK"..

Andrew