Received-SPF: pass (google.com: domain of linux-kernel+bounces-72060-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID:
 <SEZPR06MB695926ACA91530EB8642A30F96512@SEZPR06MB6959.apcprd06.prod.outlook.com>
Date: Tue, 20 Feb 2024 06:31:51 +0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH net-next v3 3/6] net: hisilicon: add support for
 hisi_femac core on Hi3798MV200
Content-Language: en-US
To: Andrew Lunn <andrew@lunn.ch>
Cc: Yisen Zhuang <yisen.zhuang@huawei.com>,
 Salil Mehta <salil.mehta@huawei.com>, "David S. Miller"
 <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>,
 Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
 Rob Herring <robh+dt@kernel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
 Conor Dooley <conor+dt@kernel.org>, Heiner Kallweit <hkallweit1@gmail.com>,
 Russell King <linux@armlinux.org.uk>, netdev@vger.kernel.org,
 linux-kernel@vger.kernel.org, devicetree@vger.kernel.org
References: <20240220-net-v3-0-b68e5b75e765@outlook.com>
 <20240220-net-v3-3-b68e5b75e765@outlook.com>
 <29fc21f0-0e46-4d0f-8d4b-c4dbd1689c55@lunn.ch>
 <SEZPR06MB695901E7D4BEABE1B6F319D096512@SEZPR06MB6959.apcprd06.prod.outlook.com>
 <5572f4dd-dcf2-42ec-99c8-51bf4d1f28ba@lunn.ch>
 <SEZPR06MB6959E5BDA57AF61BFAB19FC096512@SEZPR06MB6959.apcprd06.prod.outlook.com>
 <0beab72d-2919-447f-88ff-fb7c92b28b61@lunn.ch>
From: Yang Xiwen <forbidden405@outlook.com>
In-Reply-To: <0beab72d-2919-447f-88ff-fb7c92b28b61@lunn.ch>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-MessageSentRepresentingType: 1
X-MS-PublicTrafficType: Email
X-MS-TrafficTypeDiagnostic: SEZPR06MB6959:EE_|SEZPR06MB5958:EE_
X-MS-Office365-Filtering-Correlation-Id: e403f00d-d0cb-4d96-679d-08dc319a955c
X-Microsoft-Antispam: BCL:0;
X-Microsoft-Antispam-Message-Info:
	aIXfUUPJMq6DDtBRfTz0bAkXMAbm4UG3YMCDlTJLBPDUDuQVomG/WFJde320tyt5MAY4dRK5Uhuh+cvUgM3vhgZj5d9tUxB4Z1uvEkSt4Ry6UGc+fgbj2MoUa23eZj5tQBJIkEyjZq3GLdnJmITWVQpCpEWLzfzq0qn75lTgKIGWdK1Gqs85E+it5RLdd/Dhz4MH/02U1ENrkhjKTD4ZnqWtZgTOBbfdvZnqC/pmMplXu061drV3Skwu5MAhDP5C1RYS0QfFzcKKHVZrDmcdslVysdq3uj4X7dsOkpPuER3iUwMkVW1xgCUaVy3Pm/SygNHaQJ+tUuWxzm0uT5I+KufEAiBf2sFzWzab0hcz64MnUgVES/Fof8WplpuyOKqneA7PerQo2M4RinNCWsL0COPSaBwJbpR3DSSG6q7NCzC313lPqlAerN+odtxv4t5FhfSm35RTT6Mzh+dUld0Qwrmu6r0gf0bOuv4XWSYo2MGMSwy4reUIWkv3LzguvoSPjsWeAdkimuqFGsdkMRFAF23B1sykkexzSE6ARrEGeOpfurx92l52cTTNdwGeMiJs
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?UGNNK0lPaFpid0tYUzhvbk40ZkZhb1Nhb2xJblIraWRFd3ZYb25ZU09ncXZN?=
 =?utf-8?B?SXNZK3M2bVk2d3U1eVVvVnFTSEFjSG8xQ0xKcEYyamxvR3M2cXVVUDNYcy9Q?=
 =?utf-8?B?aXY5R0lnaWprbDNVZkZwdVpLTVJvR0NuUjlnekdtemVBd2NSbEs3Rkt1VFE2?=
 =?utf-8?B?RTh5VmFmTzBldGhDTUR5RkxPMnRPY3IxWkZnSnYwTXYyaWh6Tlg5eTBNaHRM?=
 =?utf-8?B?b3lOUldwVTBLaHJsK2N3bStXUDl6b0Y1OVJHODBHVTVIVno1bldCaUFjZ2pO?=
 =?utf-8?B?WWVGUlpLN1RKVDJ2WFE4MEZ0NW1FaHVlRlIybERWSDZKOFdmZEdUT25NYmRG?=
 =?utf-8?B?VlZ4Y1hhZEIzSXgyc1R5RmcxeXpDRWswbHJzd3FhbXpybDhWU3hMd1BTcXZG?=
 =?utf-8?B?ZEtvdFh5cGd4TUI1cVgvZ0lpNFhUOTRxbmxCV0d3aWVoUnZMV2toRVoxVUpW?=
 =?utf-8?B?UHNjeUowblFFbTh2YVAyZDdHVHh1eGg1RTJQdDZCYUwzU2dGYjU5MkVjY3Nx?=
 =?utf-8?B?NUkzb054d1BPS003cnEraFhPU0pIeTNHbVVTSUpjVll1ZHM5ZStpQnhEVnIw?=
 =?utf-8?B?SlE1Mlp5SW5taU1yVkdFbUJOdEtrTmdyV253ZEZhbzlZYWxaQ1daNmVmQkxt?=
 =?utf-8?B?MnE3R1lkQi9NeGZwaDlObEtjRk1hR2lGcUhPeDA1U1QvdS9aeUVqRC9TREhu?=
 =?utf-8?B?TStGM1cwR00vM0hNTnd5UE01RnkzLzhpaGlubElUR3JxTUNtYWhtUkRuSjgv?=
 =?utf-8?B?NE9XNEdkUUN1TDlPUWUvVUEvODVMU01qSkQrbUZLSlNtVCs4NHdVbnlwZnNL?=
 =?utf-8?B?VjlaUzRwckNLZ2Vsa2VMcDV0SFMvcEY3TjZieHpqUXdPWXRURkFraUoxSW96?=
 =?utf-8?B?VERvRThBRTRtYUhxczllKzR1S3oySFNmTFB2ditCdmpxNkhhOTY2U002U1ZB?=
 =?utf-8?B?TThxcmJ0MlEyWElqWHFXR2FYUDA1aEppdWZoRmdDLzNQR3lsQndxRU5VV2dT?=
 =?utf-8?B?RnV2cm05UDNyUm51NEhRNUFHM05nNXBiUFdFdExSSmpFcjBHUUF6TFkweVhS?=
 =?utf-8?B?T2EvUUV4WDMwWTNFMnpSZDFIcHdLMStaVm16Y2Zqblo4Zmg5dGZZZXdia2R6?=
 =?utf-8?B?SEE0Q2RKdTFxWk4rSWZiYTJNeEtycldmRnBEMm5xdTRUaFRxTG1JMXk1djQr?=
 =?utf-8?B?emU2ckljV1g1aGhqdzVqMVc5VzZheUcvWk9xMzA0bXJXZnpueUhvellLV3BJ?=
 =?utf-8?B?RllnWUxaY0EzSXFRNDRKZXhJR3B0OWxCZEt2YTJxYjdzSVlMQ01pdFIyN3lu?=
 =?utf-8?B?T1dybno1MVBhN1dmcGl0dWxjb0t1OGZ1QWkvNjRmQ0Nwbk9ONXdLYm4rbTNa?=
 =?utf-8?B?RXBaaTRuYjIrK2JTY2lxLzhQaE1KSlBnNkxMZ2xrc0dSNHh4OFBNZUNrS29B?=
 =?utf-8?B?cVVKcElqcXJWeFkzMGRGQmpyaWhkTHlBdzg5VGdPMmxObEhkN1c1QmhpbVNP?=
 =?utf-8?B?L1ZNYW5VQktxNm5BczdRa2xDbXl0alNwOG9YbUZyd1RVcVJYYnlFU093ZE1u?=
 =?utf-8?B?NVZhVUVkd09UZEExUGt0eW5tYjVoWjdFL3Y5cktqZXpiQlVBK0szWkRzTi9L?=
 =?utf-8?B?L2V3cnl4ZXl2UWRvaGt1TXVuVC9oNGhyVnpUdmRheSs1KzJCdjZ3RVhRL096?=
 =?utf-8?B?d2lZNGdyUmR2NHluL3ZGUWhIUHVoL3pzMEZDRmI4SFE0ZWd4T2Z0emdmUzlJ?=
 =?utf-8?Q?raDfM/gxWKia4ev1w8nv8b4sXZJEjuOB0HLPfJ6?=
X-OriginatorOrg: outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: e403f00d-d0cb-4d96-679d-08dc319a955c
X-MS-Exchange-CrossTenant-AuthSource: SEZPR06MB6959.apcprd06.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 19 Feb 2024 22:31:58.1727
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa
X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg:
	00000000-0000-0000-0000-000000000000
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SEZPR06MB5958

On 2/20/2024 6:05 AM, Andrew Lunn wrote:
>> It's not MAC which behaves wrongly, it's the MDIO bus. If we don't follow
>> the reset procedure properly. The MDIO bus fails to respond to any
>> write/read commands. But i believe MAC controller and PHY are still working.
>> I recalled that it can still transfer network packets, though it fails to
>> read PHY registers from MDIO bus so only 10Mbps is available (And the phy id
>> read out is always 0x0, normally it's 0x20669853).
>>
>> Maybe during initialization, PHY sent some garbage to MDIO bus and killed
>> it.
> MDIO bus masters are really simple things, not much more than a shift
> register. I find it hard to believe the MDIO bus master breaks because
> of reset order. If the MDIO pins went to SoC pins, it would be simple
> to prove, a bus-pirate or similar can capture the signals and sigrok
> can decode MDIO.
>
> To me, its more likely the PHY side of the MDIO bus is broken somehow.

The MDIO pins are not accessible from outside, only PHY pins are exported.

Maybe. I've tried many different approaches before i sent this patch. 
This is almost the only simple way i can implement to make it work. The 
downstream is also not telling us why it is needed to disable MAC 
controller before PHY reset. But it is mandatory. All i can do is to 
guess, unless HiSilicon people can join this conversion and tell us why. 
So the conclusion i got from trial and error is that MAC controller MUST 
be disabled during PHY reset, no matter what the reason is. I hope we 
can have the framework providing utilities for me to resolve this (e.g. 
provide a custom callback for PHY reset, in mdio_device_reset() function).

Also it's very strange that, if the PHY is reset in a wrong order, (i.e. 
Now we can not operate MDIO bus), we have to do the PHY reset procedure 
once again, simply resetting MAC does not work. So it might also be the 
PHY which is broken (partially).

>
>     Andrew


-- 
Regards,
Yang Xiwen