Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <160179b7-eec0-3db6-e7af-bc62333f9457@windriver.com>
Date:   Thu, 17 Nov 2022 12:40:21 +0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.2
Subject: Re: [PATCH 2/2] net: fec: Create device link between phy dev and mac
 dev
Content-Language: en-US
To:     Florian Fainelli <f.fainelli@gmail.com>,
        Andrew Lunn <andrew@lunn.ch>
Cc:     hkallweit1@gmail.com, linux@armlinux.org.uk, davem@davemloft.net,
        edumazet@google.com, kuba@kernel.org, pabeni@redhat.com,
        netdev@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20221116144305.2317573-1-xiaolei.wang@windriver.com>
 <20221116144305.2317573-3-xiaolei.wang@windriver.com>
 <Y3T8wliAKdl/paS6@lunn.ch> <355a8611-b60e-1166-0f7b-87a194debd07@gmail.com>
 <Y3V5AgBMBOx/ptwx@lunn.ch> <9c9643e3-db53-bd35-6028-1c8b718e1cc2@gmail.com>
From:   "Wang, Xiaolei" <xiaolei.wang@windriver.com>
In-Reply-To: <9c9643e3-db53-bd35-6028-1c8b718e1cc2@gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?WUZJaDhtMlovV2tqWXB6YXkxQTNjZ1RqYWd4WldxSEVzdzF1N0FIQldXazBM?=
 =?utf-8?B?bUN6NWIzYnZqUVNhVFFGU2daZnk4ejRNQjhxV2V6RFo3eFRRNEI4VWIvbWpP?=
 =?utf-8?B?aFBPMUVjMVZoaXFCMkJONkkzREFERENIUVJxWm1pVXZIYjBzanNiUGZ3dFE5?=
 =?utf-8?B?UWxPdWNIaHFxa3N3aG9PTXVlbUNDbG1Tdi8vTzRMUENwUWxSRllmMHNFb05P?=
 =?utf-8?B?YS9pR295OTZFeDdqRlJTd3NycWg2eVRWbldLMkd6Q3Q5SUN5UlFRTmxxRG5J?=
 =?utf-8?B?Ylc3TTJMbUFub2lqWUsyNC9BamVYdXJETDRRZGdjeE1nbDhIREdwUGVRKzUv?=
 =?utf-8?B?VHpuZHFsdkZPTUE4Qlo2TEJlellwMzNrWnZmSWxIYWtBNFFUTFA0WFpzSzA3?=
 =?utf-8?B?dzRsZ0NCMVcycjUxMk9sOEU4dFc3NUpsSCtTeFNac3h1UUx5Z1dpeksyNGNE?=
 =?utf-8?B?a1Q3REZ3dHpsb2hIVnlkWGdKdDZDQ05TK2MxWFRyamgzM0Jpa0ZLcVg1RXJK?=
 =?utf-8?B?R3k0V3Vsd0NjN1JzWGMwY05KUjlpQjdtWHNITzB5M0wyQnVEOVJ4MGZxbkdz?=
 =?utf-8?B?dTdsU29NZktmUi9FT1FrSjB2U0h6MC9JNGlNZEdISnFHUlQ5aFpYNTB2Qm1B?=
 =?utf-8?B?MldQL2Q1bElneTFLS29PaTFaL3V1aGFBSlIxb0hJWG14NTF1ZWdtNThZZy9s?=
 =?utf-8?B?bnl5U2NwVkZ1VFVDQ3hMcVBCTkpIdkZiYXNjTlRTWCtoaDlVQW9sUXZBclEy?=
 =?utf-8?B?aTdYekdaSnRObEFEMU5QTTBRcDhrZnFsdDlZOGoyVVpCbHdLQjJSUUpYSVNz?=
 =?utf-8?B?TjJnWElEOC9PWGFGUUxBQ0hvSDY5b2wrbVkzMmswUWkwcHJXOFBvR1VXK01h?=
 =?utf-8?B?M3lsdi9SYUVySGlIMDJUdWNvT3phRFBZUVdBTitnbURWang3bkE4MXZGOUw3?=
 =?utf-8?B?RWUxajJsRTgyVEdhMUdOUlVJY2xpQkZ2d1N4VDcwUmUxSHlhbURGVHRQVWRv?=
 =?utf-8?B?WVdqbFk3ZFZWTnMwajVCRm1MbnhFZUFyOFRHbUNpSG96NTMzMlB1MFVOMmdL?=
 =?utf-8?B?ZGRubnZVeGJBdDBjZDY5MFdlNTBTbzVNNDRUMEN1Y2NtaVo0TG9kc3UxamVO?=
 =?utf-8?B?Sy9kbndZOFptRFVrMXNBYklWQ2gwWjBST0FRdnh0cGo4em9xbUh0WEZENkJo?=
 =?utf-8?B?K0t2dDJSWWE0cU5iaGptYnlRM2dIUDhjY0NlM25ET2o2Y25ZM2UyWkp2TnBn?=
 =?utf-8?B?N2g4c3RvN2x1bFcvcVU4UUpOVUxITE0rbXM0RzBkdnp1MUwxOW1mZXV2TDF3?=
 =?utf-8?B?L2NCVTRMci9IeTNlVWlXS2RTWW1XVzZwcFhPcm14TVVtVmIwMzBsZk1RdGVI?=
 =?utf-8?B?VmpBRW1YZC9TL2RYYUlSSWVBbk5VWEwxUFlROXVjYlZwY3hqRDNlNlU0RmtS?=
 =?utf-8?B?ckhmSHEvdDVYVEFQc2ZYRXRIVzBwck1aVTJYRHA3MUtVSm1vZzVQMmQ4VXJZ?=
 =?utf-8?B?dTZ6M2RqdnBVOU5BSm9SSFcwK2xlazl0ZkVRclJDVmhsU0YvMWdGQXlNVFhF?=
 =?utf-8?B?MHVHeklRelBSRTdBTFBuLzdaYkNib2ZOankxcDgrbUszQlZoWitFN0cyY3BV?=
 =?utf-8?B?SGphUzFiOS9ySkI3UTk0aGt1SHdGYlZPVy9PUW1ENnVOWjFLZHZpMjJZUWhj?=
 =?utf-8?B?RG0vRGhvQ1RvenBLZGdaaXhobkZwTjlmY1BRUDIwUWtZTzNHVzFCRlFXdHNk?=
 =?utf-8?B?UEJGbnNLR2JQenRtZSsyR0t6bkxUNVNoSkJNSStEWHk5My9WUDBlL01iQ3Zy?=
 =?utf-8?B?d0Y2SThiM0pGYjcrVUFCZm9wZmY2Q09jd3N3RHowbVJTck10empxVUNpL0Nm?=
 =?utf-8?B?V1ZGNGxTcjBPQ1pldGxhWkdSWjZVSHcybkdEdUMwRGRsTGNaWThUbW4vTVM2?=
 =?utf-8?B?a2RCSDkrNWZDQkNSeGdpcGRKTG4reWVDdTdKbWN5d3ZObFRTTllKMUZJY1hs?=
 =?utf-8?B?ODU5UlBvUUdzT2t1Q1lLMGNScmdaaVd2TFpPT0p5bDNFY3NJUmM0Nmk1MGJl?=
 =?utf-8?B?NTNsR0srOWE2MS9HeERScDVUM3ZmYTJ3VW5ycTBDUE8zbURsMHJYMUw5aTFD?=
 =?utf-8?B?WURmU3Y2Qmg2ZkVpZmRBNG0xUzF2NVFLdTh2YjJxTW5nRjVRWHZ1UG83a0dB?=
 =?utf-8?B?cWc9PQ==?=
X-OriginatorOrg: windriver.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 9d87598d-1ecc-4b9a-e6d2-08dac855db9c
X-MS-Exchange-CrossTenant-AuthSource: MW5PR11MB5764.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 Nov 2022 04:40:31.0801
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 8ddb2873-a1ad-4a18-ae4e-4644631433be
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: yoKNKyWnDPi385uv4Cr1CnFSEJ2Av/e0EKOIHO4cL/eV5tpRU0cb4bDz2qc4fUIA+x9wLgdFmwPR9m106TYQOMDIBvoi5iAcGFYaFQ7oKc0=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR11MB7028
X-Proofpoint-GUID: -mDzdHSutXUfHoGQz8DeLkKRF1Ejsz_v
X-Proofpoint-ORIG-GUID: -mDzdHSutXUfHoGQz8DeLkKRF1Ejsz_v
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.219,Aquarius:18.0.895,Hydra:6.0.545,FMLib:17.11.122.1
 definitions=2022-11-17_02,2022-11-16_01,2022-06-22_01
X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 adultscore=0
 malwarescore=0 mlxscore=0 bulkscore=0 phishscore=0 clxscore=1015
 priorityscore=1501 lowpriorityscore=0 suspectscore=0 impostorscore=0
 mlxlogscore=999 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2210170000 definitions=main-2211170032
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,
        SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 11/17/2022 8:21 AM, Florian Fainelli wrote:
> CAUTION: This email comes from a non Wind River email account!
> Do not click links or open attachments unless you recognize the sender 
> and know the content is safe.
>
> On 11/16/22 15:57, Andrew Lunn wrote:
>> On Wed, Nov 16, 2022 at 03:27:39PM -0800, Florian Fainelli wrote:
>>> On 11/16/22 07:07, Andrew Lunn wrote:
>>>> On Wed, Nov 16, 2022 at 10:43:05PM +0800, Xiaolei Wang wrote:
>>>>> On imx6sx, there are two fec interfaces, but the external
>>>>> phys can only be configured by fec0 mii_bus. That means
>>>>> the fec1 can't work independently, it only work when the
>>>>> fec0 is active. It is alright in the normal boot since the
>>>>> fec0 will be probed first. But then the fec0 maybe moved
>>>>> behind of fec1 in the dpm_list due to various device link.
>>>
>>> Humm, but if FEC1 depends upon its PHY to be available by the FEC0 
>>> MDIO bus
>>> provider, then surely we will need to make sure that FEC0's MDIO bus is
>>> always functional, and that includes surviving re-ordering as well 
>>> as any
>>> sort of run-time power management that can occur.
>>
>> Runtime PM is solved for the FECs MDIO bus, because there are switches
>> hanging off it, which have their own life cycle independent of the
>> MAC. This is something i had to fix many moons ago, when the FEC would
>> power off the MDIO bus when the interface is admin down, stopping
>> access to the switch. The mdio read and write functions now do run
>> time pm get and put as needed.
>>
>> I've never done suspend/resume with a switch, it is not something
>> needed in the use cases i've covered.
>
> All of the systems with integrated I worked on had to support
> suspend/resume both with HW maintaining the state, and with HW losing
> the state because of being powered off. The whole thing is IMHO still
> not quite well supported upstream if you have applied some configuration
> more complicated than a bridge or standalone ports. Anyway, this is a
> topic for another 10 years to come :)
>
>>
>>>>> So in system suspend and resume, we would get the following
>>>>> warning when configuring the external phy of fec1 via the
>>>>> fec0 mii_bus due to the inactive of fec0. In order to fix
>>>>> this issue, we create a device link between phy dev and fec0.
>>>>> This will make sure that fec0 is always active when fec1
>>>>> is in active mode.
>>>
>>> Still not clear to me how the proposed fix works, let alone how it 
>>> does not
>>> leak device links since there is no device_link_del(), also you are 
>>> going to
>>> be creating guaranteed regressions by putting that change in the PHY
>>> library.
>>
>> The reference leak is bad, but i think phylib is the correct place to
>> fix this general issue. It is not specific to the FEC. There are other
>> boards with dual MAC SoCs and they save a couple of pins by putting
>> both PHYs on one MDIO bus. Having the link should help better
>> represent the device tree so that suspend/resume can do stuff in the
>> right order.
>
> My concern is that we already have had a hard time solving the proper
> suspend/resume sequence whether the MAC suspends the PHY or the MDIO bus
> suspends the PHY and throwing device links will either change the
> ordering in subtle ways, or hopefully just provide the same piece of
> information we already have via mac_managed_pm.
>
> It seems like in Xiaolei's case, the MDIO bus should suspend the PHY and
> that ought to take care of all dependencies, one would think.

Hi

mac_managed_pm solves the soft reset triggered during aeg. If you modify 
it back to MDIO bus to suspend phy, you still need to solve the problem 
of auto-negotiation,

thanks

xiaolei

> -- 
> Florian
>