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