[PATCH 1/3] include: environment: ti: Use .env for environment variables

[Neha Malcom Francis]

Hi Tom,

On 15/03/23 09:35, Neha Malcom Francis wrote:
> Hi Tom,
> 
> On 14/03/23 22:17, Tom Rini wrote:
>> On Tue, Mar 14, 2023 at 04:07:50PM +0530, Neha Malcom Francis wrote:
>>
>>> Add K3 common environment variables to .env. We retain the old-style C
>>> environment .h files to maintain compatibility with other K3 boards that
>>> have not moved to using .env yet.
>>>
>>> Signed-off-by: Neha Malcom Francis <n-francis at ti.com>
>>
>> Thanks for starting this off, a few comments:
>>
>>> +#if CONFIG_MTD_RAW_NAND
>>> +dfu_alt_info_nand=
>>> +    SPL part 0 1;
>>> +    SPL.backup1 part 0 2;
>>> +    SPL.backup2 part 0 3;
>>> +    SPL.backup3 part 0 4;
>>> +    u-boot part 0 5;
>>> +    u-boot-spl-os part 0 6;
>>> +    kernel part 0 8;
>>> +    rootfs part 0 9
>>> +#endif
>>
>> We don't guard any of the other dfu_alt_innfo_xxx options, so I don't
>> think we need to here either.  If it's a concern about having the
>> variables when we don't have the support, perhaps a slightly different
>> structure of the files makes sense? All of the NAND variables in a
>> nand.env, emmc in emmc.env, etc.
>>

Left out this reply in the earlier email, but I will drop the addition 
of this file altogether in this patch series as none of the K3 boards 
use it. If extending to the older boards later, we can do like you 
suggest with separate files for each.

>>> diff --git a/include/environment/ti/k3_rproc.env 
>>> b/include/environment/ti/k3_rproc.env
>>> new file mode 100644
>>> index 0000000000..ed19ff4a52
>>> --- /dev/null
>>> +++ b/include/environment/ti/k3_rproc.env
>>> @@ -0,0 +1,28 @@
>>> +#if CONFIG_CMD_REMOTEPROC
>>
>> We should guard including this file or not based on REMOTEPROC, not the
>> contents.
>>
>>> +#define CFG_SYS_SDRAM_BASE        0x80000000
>>
>> I don't see this used anywhere else, please drop it.  Everything else
>> seems fine, thanks!
>>
> 
> Thanks for the comments, I'll send v2 soon!
> 

-- 
Thanking You
Neha Malcom Francis