Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Sean Anderson <sean.anderson@seco.com>
Subject: Re: [RFC net-next PATCH 06/16] net: phylink: Add function for
 optionally adding a PCS
To:     Andrew Lunn <andrew@lunn.ch>,
        "Russell King (Oracle)" <linux@armlinux.org.uk>
Cc:     netdev@vger.kernel.org, "David S . Miller" <davem@davemloft.net>,
        Jakub Kicinski <kuba@kernel.org>, linux-kernel@vger.kernel.org,
        Heiner Kallweit <hkallweit1@gmail.com>
References: <20211004191527.1610759-1-sean.anderson@seco.com>
 <20211004191527.1610759-7-sean.anderson@seco.com>
 <YVwgKnxuOeZC6IxW@shell.armlinux.org.uk> <YVxWaif9jE/fCE0O@lunn.ch>
Message-ID: <9545e6d4-7d0b-736f-cb0c-a95d62aba6c3@seco.com>
Date:   Tue, 5 Oct 2021 12:17:38 -0400
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.10.0
In-Reply-To: <YVxWaif9jE/fCE0O@lunn.ch>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?M3dSbjBpN25aSEdpS0p5NXNlcUJ6dml0MHc4UjBaOVpuRlovTDEyZFJCczdu?=
 =?utf-8?B?ZGhCM3ZMZW5ta0cyN3ZnSzdjZmM3TjlmZkRXMHdzWUg4ZGtWelcvYW1BMjlL?=
 =?utf-8?B?NmNjbS9Ec0xRVi96bjdMLzJGY0RmakhpeVdSMllxblpBS2Y0VnE5eUN3MTkw?=
 =?utf-8?B?NHBKaWJYeHhYVldDMHFnbHpxNWMzbHo1L2d3WFJYeWhQRWFmZ1hJY0l6VWlr?=
 =?utf-8?B?RUtsL0JtK1hCYTZYYXhQeVdycTdVSFI2TVZNbEVGT2xyNHBReEY0OU1YVUIx?=
 =?utf-8?B?emNiQU9Pd0VmLzVpRzM1UmhCMmdBcE5mTWFBVnJyTUNzaWVCYng2YkFwZ01a?=
 =?utf-8?B?V2VTSDFISHJjK1J0alpGVUM3Q3pIR3k2Y0R4ZW56SnBYTDBXNFI0SlE2M3pL?=
 =?utf-8?B?L1ZGektQT0JkYW5zb0phSXhwaWxCbnE2MXpROElRaUU4ei9rVWl0U3hCTmc5?=
 =?utf-8?B?UWFPNEhCTEE1VjdNaHYzcnJWbnpvUERXY2UybWd6YkF5dXJaeU45cGxlbThJ?=
 =?utf-8?B?a0t6cU9ySDhYaFU1cHl1dFJqeXZxWTB2eUI0UEtqTzA0N3lnOVFxb2ZPMjBY?=
 =?utf-8?B?bitjRVpZUmxvS1VKeThucUkyMlg4TE56K2M1b1NPekVPMHpyWkFleEdVNEdz?=
 =?utf-8?B?UWU0Q1hHVXlNNlBmbUpDWUNYejdNYlRYRERac3Y3U2ZkeFMxZzRnWVV2RFNI?=
 =?utf-8?B?UDIxZUNIbEdFSFZtYmhNU1UyT3c4RFFyNUxwZHN0SUlLcTd2cGpNdVY5N1lm?=
 =?utf-8?B?c01mN2swb3g3Y1VnMnY2bGxQMHdCcFhnZCtRSllCR2FESS96RGZ5M2dKWExo?=
 =?utf-8?B?bWUxSnNhdytxNWV1QlZYUGQ1Z1Y3V25LNU8rdEFxdll1VERxaDdBSllyWHFq?=
 =?utf-8?B?anNTZm1jbE5kVlBDVkN4UklVSE40ZUYydEFCZ1JQQ0U1YndVcmFqRFdDNUlp?=
 =?utf-8?B?b3EwVFd2ZXcxN21rMVRQblh4dlEvMGs3U0szUVBGSVVNZGpSTWpyeDZEeC84?=
 =?utf-8?B?VEhIYjVHd0lnRGR6NlVCOTh2NDdWallrM2VYRys5V2hUdEU1dmh0bWNUSDRF?=
 =?utf-8?B?NXhhL1NkMlRESE1KVVZrQmxGbWFqVkRGcUlTRUxUY1lJcFNQV0dUR3BIVXJl?=
 =?utf-8?B?eWRma1NiY0RYQ0g0ODJ1di9QOS9iczdsU255WjZLVWxqcGd4U3A5NEJnWE9k?=
 =?utf-8?B?NzRocmc1VExHL0FlbFg5ZHVwWlhPajJJNm1NUm5nWjIyOTNkbkhoYUM0M092?=
 =?utf-8?B?QU5VWUt0YUdKN2F6TFE0bWNtQng1eGNuVmk4K2NjVWpIWXEreklsa25GdUlB?=
 =?utf-8?B?V3Y5dWFRbEdMOE1IcnhHYm1xd2I3bUx6QXhNbFpiZEppNyswWENQSitrTDgv?=
 =?utf-8?B?OW5odWpFbVVaaGtOTTVsNEJRTlh2b2tNRnRwbVgvelFDNlExcVE5ZkR0UDY0?=
 =?utf-8?B?SW1tbVV6SlRYd0RLQW9PVVlwV2ZNVTFndTVvRDF2RkRSeGdrbW5VcUZZeXB2?=
 =?utf-8?B?aXUxUGQ2b2YxVWEyNEQwM2pEQ3BGRzFBV0xZVUJXZGNNcXhSNHV3Z0ZTVlFZ?=
 =?utf-8?B?RnVuc2pvYWZDN25CbG5aSzVtbW04M0VISDhReTB4bzNzU0gvUVRqN0t5d3Bt?=
 =?utf-8?B?ZG8rZlhCYks0M0JwNm56N2hJQ0tLMkhON2xqOHFrVForRmxyYm5EZmdRc1JJ?=
 =?utf-8?B?TWtYdlFUZUtpVWRYczRqSXdjZStwYmJjYmdOMVZjUHFMTExpNUxyV08zMTRV?=
 =?utf-8?Q?DypCy3iSPF3wuhNbPe4Ppgy9y59kR8H8JprdSWO?=
X-OriginatorOrg: seco.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 5345833b-51ad-49a9-c32e-08d9881ba8a5
X-MS-Exchange-CrossTenant-AuthSource: DB7PR03MB4523.eurprd03.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 05 Oct 2021 16:17:42.5429
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: bebe97c3-6438-442e-ade3-ff17aa50e733
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: WH3moYvC9rdlqVjtsuBE6nEzjbnRF7SNLRh5SlmhH2tvVIG4uycCCGC08ZSi7BLf6x8y3edZwidWFdiS8XfeJg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB7PR03MB3659
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 10/5/21 9:43 AM, Andrew Lunn wrote:
> On Tue, Oct 05, 2021 at 10:51:38AM +0100, Russell King (Oracle) wrote:
>> On Mon, Oct 04, 2021 at 03:15:17PM -0400, Sean Anderson wrote:
>> > This adds a function to set the PCS only if there is not one currently
>> > set. The intention here is to allow MAC drivers to have a "default" PCS
>> > (such as an internal one) which may be used when one has not been set
>> > via the device tree. This allows for backwards compatibility for cases
>> > where a PCS was automatically attached if necessary.
>>
>> I'm not sure I entirely like this approach. Why can't the network
>> driver check for the pcs-handle property and avoid using its
>> "default" if present?
>
> And that would also be in line with
>
> ethernet/freescale/dpaa2/dpaa2-mac.c:	node = fwnode_find_reference(dpmac_node, "pcs-handle", 0);
>
> We need a uniform meaning of pcs-handle. And dpaa2-mac.c has set the
> precedent that the MAC uses it, not phylink. That can however be
> changed, if it make sense, but both users should do the same.

The tricky bit here happens when drivers set the PCS in mac_prepare()
depending on the interface. So you have to remember during probe()
whether you are supposed to set the PCS for later. I would like to leave
more of this to phylink to ensure that the process is done in a uniform
way.

--Sean