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
