On Tue, Aug 27, 2024 at 11:25:48AM +0530, Siddharth Vadapalli wrote: > The ACSPCIE module is capable of driving the reference clock required by > the PCIe Endpoint device. It is an alternative to on-board and external > reference clock generators. Enabling the output from the ACSPCIE module's > PAD IO Buffers requires clearing the "PAD IO disable" bits of the > ACSPCIE_PROXY_CTRL register in the CTRL_MMR register space. > > Add support to enable the ACSPCIE reference clock output using the optional > device-tree property "ti,syscon-acspcie-proxy-ctrl". > > Signed-off-by: Siddharth Vadapalli <s-vadapalli@xxxxxx> > --- > > v2: > https://lore.kernel.org/r/20240729092855.1945700-3-s-vadapalli@xxxxxx/ > Changes since v2: > - Rebased patch on next-20240826. > > v1: > https://lore.kernel.org/r/20240715120936.1150314-4-s-vadapalli@xxxxxx/ > Changes since v1: > - Addressed Bjorn's feedback at: > https://lore.kernel.org/r/20240725211841.GA859405@bhelgaas/ > with the following changes: > 1) Updated $subject and commit message to indicate that this patch > enables ACSPCIE reference clock output if the DT property is present. > 2) Updated macro and comments to indicate that the BITS correspond to > disabling ACSPCIE output, due to which clearing them enables the > reference clock output. > 3) Replaced "PAD" with "refclk" both in the function name and in the > error prints. > 4) Wrapped lines to be within the 80 character limit to match the rest > of the driver. > > drivers/pci/controller/cadence/pci-j721e.c | 38 ++++++++++++++++++++++ > 1 file changed, 38 insertions(+) > > diff --git a/drivers/pci/controller/cadence/pci-j721e.c b/drivers/pci/controller/cadence/pci-j721e.c > index 85718246016b..ed42b2229483 100644 > --- a/drivers/pci/controller/cadence/pci-j721e.c > +++ b/drivers/pci/controller/cadence/pci-j721e.c > @@ -44,6 +44,7 @@ enum link_status { > #define J721E_MODE_RC BIT(7) > #define LANE_COUNT(n) ((n) << 8) > > +#define ACSPCIE_PAD_DISABLE_MASK GENMASK(1, 0) > #define GENERATION_SEL_MASK GENMASK(1, 0) > > struct j721e_pcie { > @@ -220,6 +221,34 @@ static int j721e_pcie_set_lane_count(struct j721e_pcie *pcie, > return ret; > } > > +static int j721e_enable_acspcie_refclk(struct j721e_pcie *pcie, > + struct regmap *syscon) > +{ > + struct device *dev = pcie->cdns_pcie->dev; > + struct device_node *node = dev->of_node; > + u32 mask = ACSPCIE_PAD_DISABLE_MASK; > + struct of_phandle_args args; > + u32 val; > + int ret; > + > + ret = of_parse_phandle_with_fixed_args(node, > + "ti,syscon-acspcie-proxy-ctrl", > + 1, 0, &args); > + if (!ret) { > + /* Clear PAD IO disable bits to enable refclk output */ > + val = ~(args.args[0]); > + ret = regmap_update_bits(syscon, 0, mask, val); > + if (ret) > + dev_err(dev, "failed to enable ACSPCIE refclk: %d\n", > + ret); > + } else { > + dev_err(dev, > + "ti,syscon-acspcie-proxy-ctrl has invalid arguments\n"); > + } I should have mentioned this the first time, but this would be easier to read if structured as: ret = of_parse_phandle_with_fixed_args(...); if (ret) { dev_err(...); return ret; } /* Clear PAD IO disable bits to enable refclk output */ val = ~(args.args[0]); ret = regmap_update_bits(syscon, 0, mask, val); if (ret) { dev_err(...); return ret; } return 0; > + return ret; > +} > + > static int j721e_pcie_ctrl_init(struct j721e_pcie *pcie) > { > struct device *dev = pcie->cdns_pcie->dev; > @@ -259,6 +288,15 @@ static int j721e_pcie_ctrl_init(struct j721e_pcie *pcie) > return ret; > } > > + /* Enable ACSPCIE refclk output if the optional property exists */ > + syscon = syscon_regmap_lookup_by_phandle_optional(node, > + "ti,syscon-acspcie-proxy-ctrl"); > + if (syscon) { > + ret = j721e_enable_acspcie_refclk(pcie, syscon); > + if (ret) > + return ret; > + } > + > return 0; Not as dramatic here, but I think the following would be a little simpler since the final "return" isn't used for two purposes ((1) syscon property absent, (2) syscon present and refclk successfully enabled): syscon = syscon_regmap_lookup_by_phandle_optional(...); if (!syscon) return 0; return j721e_enable_acspcie_refclk(...); > } > > -- > 2.40.1 >
