On 12/20, Vivek Gautam wrote: > Qualcomm SOCs have QMP phy controller that provides support > to a number of controller, viz. PCIe, UFS, and USB. > Add a new driver, based on generic phy framework, for this > phy controller. > > Signed-off-by: Vivek Gautam <vivek.gautam@xxxxxxxxxxxxxx> > Tested-by: Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx> > --- > > + > +static struct phy *qcom_qmp_phy_xlate(struct device *dev, > + struct of_phandle_args *args) > +{ > + struct qcom_qmp_phy *qphy = dev_get_drvdata(dev); > + int i; > + > + if (WARN_ON(args->args[0] >= qphy->cfg->nlanes)) > + return ERR_PTR(-ENODEV); > + > + for (i = 0; i < qphy->cfg->nlanes; i++) > + /* phys[i]->index */ > + if (i == args->args[0]) > + return qphy->phys[i]->phy; What's the loop for? If args->arg[0] < qphy->cfg->nlanes then we should be able to directly index the qphy->phys array with that number and return it. > + > + return ERR_PTR(-ENODEV); > +} > + [...] > + > +/* > + * The <s>_pipe_clksrc generated by PHY goes to the GCC that gate > + * controls it. The <s>_pipe_clk coming out of the GCC is requested > + * by the PHY driver for its operations. > + * We register the <s>_pipe_clksrc here. The gcc driver takes care > + * of assigning this <s>_pipe_clksrc as parent to <s>_pipe_clk. > + * Below picture shows this relationship. > + * > + * +--------------+ > + * | PHY block |<<---------------------------------------+ > + * | | | > + * | +-------+ | +-----+ | > + * I/P---^-->| PLL |--^--->pipe_clksrc--->| GCC |--->pipe_clk---+ > + * clk | +-------+ | +-----+ > + * +--------------+ There are mixed tabs and spaces in this diagram causing confusion in my editor. Please make it only spaces so the picture comes out correctly. > + * > + */ > +static int phy_pipe_clk_register(struct qcom_qmp_phy *qphy, int id) > +{ > + char clk_name[MAX_PROP_NAME]; I'm not sure MAX_PROP_NAME is the same as some max clk name but ok. We should be able to calculate that the maximum is length of usb3_phy_pipe_clk_src for now though? > + struct clk *clk; > + > + memset(&clk_name, 0, sizeof(clk_name)); > + switch (qphy->cfg->type) { > + case PHY_TYPE_USB3: > + snprintf(clk_name, MAX_PROP_NAME, "usb3_phy_pipe_clk_src"); > + break; > + case PHY_TYPE_PCIE: > + snprintf(clk_name, MAX_PROP_NAME, "pcie_%d_pipe_clk_src", id); > + break; > + default: > + return -EINVAL; > + } > + > + /* controllers using QMP phys use 125MHz pipe clock interface */ > + clk = clk_register_fixed_rate(qphy->dev, clk_name, NULL, 0, 125000000); I was hoping you would be able to calculate the actual output rate by reading hardware. This is ok too though. Just please use clk_hw_register_fixed_rate() instead. And you'll probably need some sort of devm() usage here to handle probe failure, so I would probably roll my own and allocate a fixed_rate clk structure and set the rate/name directly and then call devm_clk_hw_register(). > + > + return PTR_ERR_OR_ZERO(clk); -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html