Re: [PATCH v3 01/13] PCI: imx6: Simplify clock handling by using HAS_CLK_* bitmask

[Frank Li]

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