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

[Neha Malcom Francis]

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.
> 
>> 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