[PATCH v3 19/20] env: ti: j721e-evm: Add env variable to power on & reset QSGMII PHY in J7200 EVM

[Kishon Vijay Abraham I]

Hi Tom,

On 07/05/21 10:44 pm, Tom Rini wrote:
> On Tue, May 04, 2021 at 04:11:54PM +0530, Kishon Vijay Abraham I wrote:
> 
>> MAIN CPSW0 requires the PHY to be powered on and reset for QSGMII
>> operation. Add a env variable to configure driving "0" on ENET_EXP_PWRDN
>> controlled by GPIO EXPANDER2 (I2C Addr: 0x22), PIN: 17 and driving "1"
>> on ENET_EXP_RESETZ controlled by GPIO EXPANDER2 (I2C Addr: 0x22),
>> PIN: 18.
>>
>> Signed-off-by: Kishon Vijay Abraham I <kishon at ti.com>
>> Reviewed-by: Suman Anna <s-anna at ti.com>
>> ---
>>  include/configs/j721e_evm.h | 16 +++++++++++++++-
>>  1 file changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/configs/j721e_evm.h b/include/configs/j721e_evm.h
>> index b707fc4e89..00d0a18a68 100644
>> --- a/include/configs/j721e_evm.h
>> +++ b/include/configs/j721e_evm.h
>> @@ -139,11 +139,24 @@
>>  #endif /* CONFIG_TARGET_J721E_A72_EVM */
>>  
>>  #ifdef CONFIG_TARGET_J7200_A72_EVM
>> +#define EXTRA_ENV_CONFIG_MAIN_CPSW0_QSGMII_PHY				\
>> +	"do_main_cpsw0_qsgmii_phyinit=1\0"				\
> 
> When would this be not true?

If the user don't want to use QSGMII, this could be set to false. For
instance the SERDES in J7200 can be used such that it can be used with
two protocols at a time. So it can be either PCIe + QSGMII or PCIe +
USB. So for use cases which require PCIe + USB, this could be set to false.
> 
>> +	"init_main_cpsw0_qsgmii_phy=gpio set gpio at 22_17;"		\
>> +		 "gpio clear gpio at 22_16\0"				\
>> +	"main_cpsw0_qsgmii_phyinit="					\
>> +	"if test ${do_main_cpsw0_qsgmii_phyinit} -eq 1 && test ${dorprocboot} -eq 1 && " \
>> +			"test ${boot} = mmc; then "			\
> 
> And why only on mmc?

The current J7200 u-boot code loads firmwares for remote cores only from
MMC. So if it's not mmc, it's not going to load ethernet firmware and
hence not required to configure the PHY.

> 
>> +		"run init_main_cpsw0_qsgmii_phy;"			\
>> +	"fi;\0"
> 
> Finally, this feels like something we should be doing in CONFIG_PREBOOT,
> so it's always done, rather than part of seemingly the bootcmd.
> 
I'll check on this.

Thanks for reviewing!

Regards
Kishon