Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) client-ip=23.128.96.38;
Date:   Tue, 12 Dec 2023 18:18:55 -0500
From:   Frank Li <Frank.li@nxp.com>
To:     Rob Herring <robh@kernel.org>
Cc:     Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>,
        bhelgaas@google.com, conor+dt@kernel.org,
        devicetree@vger.kernel.org, festevam@gmail.com, helgaas@kernel.org,
        hongxing.zhu@nxp.com, imx@lists.linux.dev, kernel@pengutronix.de,
        krzysztof.kozlowski+dt@linaro.org, kw@linux.com,
        l.stach@pengutronix.de, linux-arm-kernel@lists.infradead.org,
        linux-imx@nxp.com, linux-kernel@vger.kernel.org,
        linux-pci@vger.kernel.org, lpieralisi@kernel.org,
        s.hauer@pengutronix.de, shawnguo@kernel.org
Subject: Re: [PATCH v3 01/13] PCI: imx6: Simplify clock handling by using
 HAS_CLK_* bitmask
Message-ID: <ZXjqX7uYm3TGFdzE@lizhi-Precision-Tower-5810>
References: <20231211215842.134823-1-Frank.Li@nxp.com>
 <20231211215842.134823-2-Frank.Li@nxp.com>
 <20231212164913.GA21240@thinkpad>
 <20231212225426.GB2948988-robh@kernel.org>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20231212225426.GB2948988-robh@kernel.org>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?mTkA3Z1JRD7r7XRjQpt0jI3IbmLPY/q4do/NoCfHoZREmYCNc8x/+WQL9LhG?=
 =?us-ascii?Q?FagMTIu/gdXz3sCxTR6b2a4HhocpG+//8RT87YsLcbvGccZD8ItOX8GxBgcF?=
 =?us-ascii?Q?a1Xfdrz75ic/8u4W1oFLpRYj4D2Pd0qwrknxawpnlqX4RAK04DWAsHqQIUJm?=
 =?us-ascii?Q?PaGH24rLJ5WzSLu0m2wIxT0r3eVNJLFVorhaRmkrAEPmzBcEcBVJblbSzGrr?=
 =?us-ascii?Q?UX3rUR16bQjZOhX4g3avKCWyl0V2XCiEL67EjRFEpAVSTjnLxydiZVRqv14n?=
 =?us-ascii?Q?kaOZLakrNj2dzG5mOAsN/rhom2vCMvhb3CqNLMV5K+BxEqtz3cOm0VtB+Ft2?=
 =?us-ascii?Q?ubZoR7j78n8g4IRAIWo9s0zMnUuGmPG1jJhmGWzdCGpxwOqvSkbRjY61nkEa?=
 =?us-ascii?Q?RMTvQznx4Z6n/Rff0DXvd6+i1fj3Sw7Ek4vQUsJlenzMMs0oC/9Ik4o6Ofqp?=
 =?us-ascii?Q?sySXVquAoy3s+VPShklh8RHUuJh9l77iLNw+0dtul/JdWSiLOmfC0gkAQog6?=
 =?us-ascii?Q?DupVUN+iyMp68Sx0jem0VF3oYXR7zbi3Yk5amTtGJA3Rcn1Ghx2AyxVE81MO?=
 =?us-ascii?Q?JU4wYR5zmqlJR2cb8ceLcjiaoYT5yTsagGa7xura0dKhri1yF12p3MAZ/mAs?=
 =?us-ascii?Q?RdQgnbVeA7cXvrI3aWCv/Mnq0246G87k3Bp/BVFjukR/DLRAuKkijFzanxdu?=
 =?us-ascii?Q?hc7DdGpE1hXQSPWqx4Vc+9WoZVbQb9W6s3RIX64nA+d7J+DTomsOdG/Ci8rg?=
 =?us-ascii?Q?MmWe9WKHrF5uOwZoae7Ew2YqmGORW+pBEIqb0++NYOf8wOb0V1ytaRIWLUmc?=
 =?us-ascii?Q?JA03qaDNVMMho6TSxwXei2L8QldYTsg7JZtlY3Pea2SeQJYrGLS44168vuLf?=
 =?us-ascii?Q?Bnm8JpZVyb4PEhOZnIJx5F3kXYAY6f0dYVhd2Jii/ctp6WnCFhelNLzZ3BWZ?=
 =?us-ascii?Q?qM4DY+fucVqgbcjmAe+D/S45zFj7HB1nQe/mw2qTPEmMB18BAnmKNFtKr7C9?=
 =?us-ascii?Q?ZF7MNHy9USmnJbovYU8sSUYOE9cff8xc559p04OgRpwZavg9RAN7MUCtXuqB?=
 =?us-ascii?Q?8flnePv60nqGjkN8QLBOYK1MYSj8Zl+YGIHmR3DUYtuhJ8MyNFpC2x0l/PTj?=
 =?us-ascii?Q?X4a13xqa5Ni3mbguJgX8M281j6HMqA6SupGvQt4po8gJzHpJpfpMJUz3AZAy?=
 =?us-ascii?Q?DDCbLxPVN6kGhwt+IOZ9K5tB9NrG/CXM6UsgoJS6bc+SQlVLSVXnnwNhmMmL?=
 =?us-ascii?Q?FP4laPsY3nUj0uf9uqi52Udqh/3yKt3axNZ1x0NPfO52IIc0vOuCZFx3hX4b?=
 =?us-ascii?Q?Rh48FkGnm/pQhpKylLHP2ABLS9rLH9Vaprf5I1QFDAQ/qSGocL9CPuEbulDx?=
 =?us-ascii?Q?zjQdep2SxeldBc1B37raL2lo75y9EXggN27tYllP90OCb62mXw+JvAZFmBoU?=
 =?us-ascii?Q?cfXsgU5VGA0zZNj7Bs7tWEQNH5UiFXvivkchCrSDFxBb5vIed9c4wvPXWMD0?=
 =?us-ascii?Q?NvjXTH4L5HH8fNjnAE/Kv4FgT/GPvEAkUdezyzrv5kvil6JyQrGfaHom+3uP?=
 =?us-ascii?Q?iQaGJuFTl6Ie3lqFGWc=3D?=
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 1ffae849-a1f7-45c7-c8c7-08dbfb68bbb2
X-MS-Exchange-CrossTenant-AuthSource: AM6PR04MB4838.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 12 Dec 2023 23:19:05.5735
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: fU4FgG9kP0h7cpa7Tux7RiI2LgmgpRZkMhppCgE9dd9wU7hIzQ9ilJ8cF/LmNyrj609+k9A9Hcso8GnDz/dcfw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AS8PR04MB8359
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
	DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,
	SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable
	autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on fry.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (fry.vger.email [0.0.0.0]); Tue, 12 Dec 2023 15:19:55 -0800 (PST)

On Tue, Dec 12, 2023 at 04:54:26PM -0600, Rob Herring wrote:
> On Tue, Dec 12, 2023 at 10:19:13PM +0530, Manivannan Sadhasivam wrote:
> > On Mon, Dec 11, 2023 at 04:58:30PM -0500, Frank Li wrote:
> > > Refactors the clock handling logic in the imx6 PCI driver by adding
> > > HAS_CLK_* bitmask define for drvdata::flags . Simplifies the code and makes
> > > it more maintainable, as future additions of SOC support will only require
> > > straightforward changes. The drvdata::flags and a bitmask ensures a cleaner
> > > and more scalable switch-case structure for handling clocks.
> > > 
> > 
> > Is there any necessity to validate each clock in the driver? I mean, can't you
> > just rely on devicetree to provide enough clocks for the functioning of the PCIe
> > controller?
> > 
> > If you can rely on devicetree (everyone should in an ideal world), then you can
> > just use devm_clk_bulk_get_all() to get all available clocks for the SoC and
> > just enable/disable whatever is available.
> 
> Or just use the *_get_optional() variants of functions. They return NULL 
> such that subsequent calls are just NOPs if the resource is not present. 
> Of course, they aren't really optional on any given platform in this 
> case, but does that really matter.

I think it'd better use devm_clk_bulk_get() by passed down a clk name list
.clk_names = {"pcie_aux", ...},

So it will not silient when dts are not matched requirement. It is not
complex because only check once at probe.

Frank

> 
> There isn't an optional variant for phys, but it can be added.
> 
> > 
> > This will greatly simplify the code.
> > 
> > Only downside of this approach is, if the devicetree is not supplying enough
> > clocks, then it will be difficult to find why PCIe is not working. But this also
> > means that the devicetree is screwed.
> 
> A sufficient schema should prevent that... That's what they're for, not 
> just torturing people to learn json-schema. :)
> 
> Rob