Re: [PATCH v2 0/3] Add support for dual port USXGMII+SGMII

[Siddharth Vadapalli <s-vadapalli@ti.com>]

On 29/06/26 4:40 PM, Gokul Praveen wrote:
> Hi Siddharth,
> 
> Thank your for the prompt feedback.
> 
> On 29/06/26 16:16, Siddharth Vadapalli wrote:
>> On 29/06/26 15:53, Gokul Praveen wrote:
>>> This patch series enables dual port USXGMII and SGMII Ethernet mode
>>> support on the TI J784S4 EVM platform using the CPSW9G Ethernet switch.
>>
>> Patch 1 => For Bindings
>> Patch 2 => For Driver
>> Patch 3 => For Device-tree overlay
>>
>> Please split patch 1 and patch 2 into a different patch series and 
>> post patch 3 only after patches 1 and 2 are merged. Also, the patch 
>> series containing patches 1 and 2 should have the 'net-next' suffix in 
>> the patch subject:
>>     [PATCH net-next ...]
> 
> Sure, I will add the 'net-next' suffix ,siddharth.
> 
>>
>> Additionally, the subject for Patch 1 looks incorrect as it doesn't 
>> follow the format expected for Bindings patches.
>>
> Sure, I will follow the format.
>>>
>>> The CPSW9G instance supports both SGMII and USXGMII modes simultaneously
>>> on MAC Ports 1 and 2, which connect to ENET Expansion 1 and ENET
>>> Expansion 2 slots through the SERDES2 instance. The series includes:
>>>
>>> 1. A fix to the SERDES2 node to add the missing PLL1 refclk, which is
>>>     required for multilink SERDES configuration to function properly.
>>
>> Is it a 'fix' or a 'feature'? Please don't use them interchangeably. 
>> If it is a 'fix', which commit is it fixing? Specifically, which 
>> commit 'claimed' to enable the configuration that you seem to be fixing?
> Yes, it is a feature, siddharth. I will change the commit description.
>>
>>>
>>> 2. A driver fix for the am65-cpsw Ethernet driver to properly configure
>>>     the MAC in SGMII mode according to the CPSGMII Module specification,
>>>     including correct advertisement ability register settings and master
>>>     mode configuration.
>>
>> Although you have called it a 'fix' again, the CPSW is a MAC and not 
>> an Ethernet PHY. Therefore, the existing configuration is intended to 
>> advertise CPSW as an SGMII capable MAC (SGMII Slave Mode Configuration).
>>
>> For more details, please refer:
>> https://lore.kernel.org/all/20220914095053.189851-6-s-vadapalli@ti.com/
>>
>> [TRIMMED]
> 
> 
> Actually, this patch which you have mentioned "https://lore.kernel.org/ 
> all/20220914095053.189851-6-s-vadapalli@ti.com/" is no longer present in 
> the latest kernel driver , siddharth.
That was exactly my point. Please go through the conversation on my 
patch to understand why the patch isn't correct.

Regards,
Siddharth.