On Fri 06 Jan 01:47 PST 2017, Vivek Gautam wrote: > > > +static int qcom_qmp_phy_com_init(struct qcom_qmp_phy *qphy) > > > +{ > > > + const struct qmp_phy_cfg *cfg = qphy->cfg; > > > + void __iomem *serdes = qphy->serdes; > > > + int ret; > > > + > > > + mutex_lock(&qphy->phy_mutex); > > > + if (qphy->init_count++) { > > > + mutex_unlock(&qphy->phy_mutex); > > > + return 0; > > > + } > > As far as I can see phy_init() and phy_exit() already keep reference > > count on the initialization and you only call this function from > > phy_ops->init, so you should be able to drop this. > This is an intermediary function that does the common block initialization. > PHYs like PCIe have a separate common block (apart from SerDes) > for all phy channels. We shouldn't program this common block > multiple times for each channel. That's why this init_count. > You're right! Unfortunately it took me several minutes to wrap my head around the phy vs multi-lane and I have a really hard time keeping "qcom_qmp_phy" and "qmp_phy_desc" apart throughout the driver. If I understand correctly the qcom_qmp_phy is the context representing a "QMP block", while this is a PHY block it's not actually the phy in Linux eyes. The qcom_phy_desc represents a "QMP lane", which in Linux eyes is the phys, but as we think of QMP as the PHY this confused me. How about naming them "struct qmp" and "struct qmp_lane" (or possibly qmp_phy) instead? That way we remove the confusion of QMP PHY vs Linux PHY and we make the lane part explicit. Regards, Bjorn -- 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