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: <9e94c275-4752-061c-3090-144d83769eb6@amd.com>
Date:   Wed, 10 May 2023 09:11:37 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.10.1
Content-Language: en-US
To:     Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc:     linux-kernel@vger.kernel.org, monstr@monstr.eu,
        michal.simek@xilinx.com, git@xilinx.com,
        Amit Kumar Mahapatra <amit.kumar-mahapatra@xilinx.com>,
        Ashok Reddy Soma <ashok.reddy.soma@xilinx.com>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Parth Gajjar <parth.gajjar@amd.com>,
        Rob Herring <robh+dt@kernel.org>,
        Vishal Sagar <vishal.sagar@amd.com>,
        devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org
References: <cover.1683034376.git.michal.simek@amd.com>
 <9eefc40c727928e0c7b794a3a2e061ecf6ea230c.1683034376.git.michal.simek@amd.com>
 <20230510065253.GA11711@pendragon.ideasonboard.com>
From:   Michal Simek <michal.simek@amd.com>
Subject: Re: [PATCH 01/23] arm64: zynqmp: Describe TI phy as ethernet-phy-id
In-Reply-To: <20230510065253.GA11711@pendragon.ideasonboard.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?RW04L21tZGtmazJCM21CU0x0QTBxOU55WHBLbFJzNktyelR1UXJEa0x3cnNa?=
 =?utf-8?B?ZFpLMDlVRFZ2a0RmZHFRT2YwZzdya2NNaWtSN0FaQTJjYmQxKzRDU2t5dTR6?=
 =?utf-8?B?dVRiTUlnckNrbDJjeVByeU40U3BRa1pjcFVDZDROSHplZXppRDltQnFVekFW?=
 =?utf-8?B?RmV4SUFaYmhzNUdNT3FYZEVKR1BsUjZ1RDAzaVMxcXpjek5jZ0UzdTlucVZS?=
 =?utf-8?B?SzF0cnkvdUpNL2JPNDlpMTJwTkhHMUJEVG5xYmtWamVRSk1QUDF6Q29Dd05H?=
 =?utf-8?B?OUJtZGlISWp5VzBtZnhLMVdvTDl5VVBiRTFKR3cxZ2o1N2JyNVNTZURHMnVI?=
 =?utf-8?B?ZmtuNWVvTFFEOWJaK2dENWo4UUtUSmVreTQ2RkVKSEhqckM5WjNLNHB5WnVV?=
 =?utf-8?B?WElXSnFDV3d6dVArQ3RmT3E3QkY5OGcwRzJ3ZDRPaWlKNlEvemF2NE1BY0U4?=
 =?utf-8?B?NER3OEdQS0duSGg5bzYrUUFZSXNvVlpoaXZrbWVYT1hNd0wyckV6RmhmUE05?=
 =?utf-8?B?REFvbStaKy9NRnAvSUlPT1k5M09vbUNRalVMQURGS0t1Y2hYVTNha3RCblNm?=
 =?utf-8?B?ZnE2NDNUT0VmanZseWZGQlBpL2VBMVFYdXJCWHl6Yy9ISThHcDNDeERpQ1Jp?=
 =?utf-8?B?WExtZjVOVzNCUnlPNUdvUVZKYWJtRG5KQ2d1R3RUdWM2TU1NTXVscm0rUVVM?=
 =?utf-8?B?eFRlT1JsVHZxYlBaTmo0VFhsTnZUSkFyQ1J6eG5tNkFHMXNqNXZSbFFtNUpx?=
 =?utf-8?B?WitiT3E2UUJSY0grdnFkOEl1NEJ1bjJNa0g2L0Faak51WjVBa1NrNHdLUFdp?=
 =?utf-8?B?enFIdVlabWwrWUwvcVFOWlZIR04vZlExa1FRY2taYW9JNkpqQlVLOHNNeFJG?=
 =?utf-8?B?RTNWaTZzMjlLb3ZOd3pVclJCNVFYNGd1Vy9VTkM2dnJUcjlFTVc4NGRzaTU0?=
 =?utf-8?B?QVB3L3BVL2xTSzRyWDdzQXkxVWFNUmV0ZVpPZjVRS2x0YW9BdFVPQ2NESVRp?=
 =?utf-8?B?ZkJqWW9vYU9ra3g1eCs3cHVEMWQxNE0zMXl3UEhXYXBoN2ZGd1N2SW5YKzlj?=
 =?utf-8?B?LzVSY3hXQ0FuQXF2a1B0eVhvK3lEUndhRkVEK1NpS1ljTTNOOHgvYmpwaU9q?=
 =?utf-8?B?Qm0xSzFGWlE4OEJta2NpeXhsbm5CbUpDYWZlUWk0amhJcGFNOWI1ZzVtRkVO?=
 =?utf-8?B?Q2txVnQ3U3UyREJQcExmSlBDM3lVcDhiTUlxM2NOMjcvWUNOb3B1NjFRUDQ0?=
 =?utf-8?B?S09uVituc2ZrNGp6QlVSOWVYS0w2QUZVRWJyU25CRFJBalZ1UFNnQzRiNEl5?=
 =?utf-8?B?di9YYXcvbm9oZmwzdWFDbGYyQ3ViSGJ3NDBWbERaUkxrVnNMU205YTRBQXh2?=
 =?utf-8?B?WW1Rd0dPMU03cDFDN0xldDVQQkFjTnNKWjZBY3VwV1BoeUpkK1pDUHlNc3dK?=
 =?utf-8?B?a0Izc2prbGF4eS9UQnhGREI5ZFRYQkJ1SFVEUUVkaElOak9rWDM3Nnhrdy94?=
 =?utf-8?B?RHZUNDRJeHlFaFIrR0JFUnRmZ29LcFNsQmxFWG9QTzh4SWx2ODJHZDk5azFh?=
 =?utf-8?B?THFnSmwxaG9iUmwzQzdNVFE5ZkxhTHhzL29lNmpvQ1ZYZ1ViVVJLKzQvV2ZF?=
 =?utf-8?B?ait5Y0hBK0RRYWVpT202bUJOV3JOTEhiOTA0L09od205ZCtXSitNSlBEZ2p1?=
 =?utf-8?B?M1ZVa3ZtVFBpMzVodEZTci9EZHVXVjdxQjZoU0NrWlFrZnpQdFZ2OEhNTVM3?=
 =?utf-8?B?UUU2Rk9KajdOMkw0ZWswTG1BWFpFSnZndnczSUowSXpPb0NTL0NuUlpXUkNT?=
 =?utf-8?B?NVJMSWVXcllIUCttNnNrYkxjWmtXVGFWSGZOMnhaV0JlZVd6aFkvK29BbU1y?=
 =?utf-8?B?TWxKTjczU3QydExXdnA0MWQ2UjFoamhBTWhtZUwvSGlVa2V2eDBxUThiSUlk?=
 =?utf-8?B?bzd3YUpDREpMbTVBK0dJL2w0OEVvSm01ZkpaanlWdXkvbXZNNk9zZ280Sjg5?=
 =?utf-8?B?bHh3MEdIRVZSTjVhVmd6dEhBbDkrd0hGLzJFWGtIYlFxemJNN0lzdnNPTTZv?=
 =?utf-8?B?Q2htNXRRK2htUzhlKzVqdFMwWHpSV0ExcnJYbVdKWng1Z29CZmF1RjFMTkJB?=
 =?utf-8?Q?XU0lGx33CgT5qGejDm+zQmRN5?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 559d4ac3-c4df-4397-6609-08db5125f7a6
X-MS-Exchange-CrossTenant-AuthSource: SN6PR12MB4765.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 May 2023 07:12:51.4865
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 6WA/4HakD//2CLKyKv13FlruVds8dYiDT1d2o3ch4PIwlqtqCIGcR8z6NhxW+0mk
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CYYPR12MB8856
X-Spam-Status: No, score=-1.5 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,NICE_REPLY_A,
        RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_NONE,
        T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=no 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

Hi Laurent,

On 5/10/23 08:52, Laurent Pinchart wrote:
> Hi Michal,
> 
> Thank you for the patch.
> 
> On Tue, May 02, 2023 at 03:35:29PM +0200, Michal Simek wrote:
>> TI DP83867 is using strapping based on MIO pins. Tristate setup can influce

And typo here.

>> PHY address. That's why switch description with ethernet-phy-id compatible
>> string which enable calling reset. PHY itself setups phy address after
>> power up or reset.
> 
> I'm sorry but I don't understand this :-(

What exactly is not clear? Phy has some pins which is using for strapping for 
phy address after phy reset or power on. Pretty much it is resistor array which 
based on datasheet is decoded to certain phy address.
And because some phy pins are also used as data pin for RGMII they are connected 
via MIO pins on a silicon. That's why IO block output setting really matter here 
because it changes resistor array and it moves phy address.
That's why there is a need to do proper IO pin setup and after it call phy reset 
to get it to address which was decided by PCB designer.

> 
>> Phy reset is done via gpio.
>>
>> Signed-off-by: Michal Simek <michal.simek@amd.com>
>> ---
>>
>> Checkpatch is reporting issue
>> warning: DT compatible string "ethernet-phy-id2000.a231" appears un-documented
>> but it should be fully aligned with
>> Documentation/devicetree/bindings/net/ethernet-phy.yaml
>> ---
>>   .../boot/dts/xilinx/zynqmp-zcu102-revA.dts    | 23 +++++++++++------
>>   .../boot/dts/xilinx/zynqmp-zcu102-revB.dts    | 25 +++++++++++--------
>>   .../boot/dts/xilinx/zynqmp-zcu104-revA.dts    | 22 ++++++++++------
>>   .../boot/dts/xilinx/zynqmp-zcu104-revC.dts    | 22 ++++++++++------
>>   .../boot/dts/xilinx/zynqmp-zcu106-revA.dts    | 22 ++++++++++------
>>   .../boot/dts/xilinx/zynqmp-zcu111-revA.dts    | 22 ++++++++++------
>>   6 files changed, 90 insertions(+), 46 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/xilinx/zynqmp-zcu102-revA.dts b/arch/arm64/boot/dts/xilinx/zynqmp-zcu102-revA.dts
>> index 13c43324f1d2..c193579400cf 100644
>> --- a/arch/arm64/boot/dts/xilinx/zynqmp-zcu102-revA.dts
>> +++ b/arch/arm64/boot/dts/xilinx/zynqmp-zcu102-revA.dts
>> @@ -2,7 +2,8 @@
>>   /*
>>    * dts file for Xilinx ZynqMP ZCU102 RevA
>>    *
>> - * (C) Copyright 2015 - 2021, Xilinx, Inc.
>> + * (C) Copyright 2015 - 2022, Xilinx, Inc.
>> + * (C) Copyright 2022 - 2023, Advanced Micro Devices, Inc.
>>    *
>>    * Michal Simek <michal.simek@xilinx.com>
>>    */
>> @@ -200,13 +201,19 @@ &gem3 {
>>   	phy-mode = "rgmii-id";
>>   	pinctrl-names = "default";
>>   	pinctrl-0 = <&pinctrl_gem3_default>;
>> -	phy0: ethernet-phy@21 {
>> -		reg = <21>;
>> -		ti,rx-internal-delay = <0x8>;
>> -		ti,tx-internal-delay = <0xa>;
>> -		ti,fifo-depth = <0x1>;
>> -		ti,dp83867-rxctrl-strap-quirk;
>> -		/* reset-gpios = <&tca6416_u97 6 GPIO_ACTIVE_LOW>; */
>> +	mdio: mdio {
> 
> The "mdio" label isn't needed. Same below.

I am doing it by purpose to be able to reference all nodes and add/remove 
property in an easy way. There shouldn't be any conflict with dt spec to have 
labels even they are not used in current DT.

Thanks,
Michal