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 <kis...@ti.com>
>> Reviewed-by: Suman Anna <s-a...@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@22_17;"               \
>> +             "gpio clear gpio@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

Reply via email to