From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S941660AbdAGSlV (ORCPT ); Sat, 7 Jan 2017 13:41:21 -0500 Received: from smtp.codeaurora.org ([198.145.29.96]:35234 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S935500AbdAGSlQ (ORCPT ); Sat, 7 Jan 2017 13:41:16 -0500 MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Sun, 08 Jan 2017 00:11:14 +0530 From: vivek.gautam@codeaurora.org To: Bjorn Andersson Cc: robh+dt@kernel.org, kishon@ti.com, sboyd@codeaurora.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, mark.rutland@arm.com, srinivas.kandagatla@linaro.org, linux-arm-msm@vger.kernel.org, linux-arm-msm-owner@vger.kernel.org Subject: Re: [PATCH v3 4/4] phy: qcom-qmp: new qmp phy driver for qcom-chipsets In-Reply-To: <20170106211722.GT10531@minitux> References: <1482253431-23160-1-git-send-email-vivek.gautam@codeaurora.org> <1482253431-23160-5-git-send-email-vivek.gautam@codeaurora.org> <20170106071834.GP10531@minitux> <20170106211722.GT10531@minitux> Message-ID: <2e93e810adbc252cd630d89bb753ec81@codeaurora.org> User-Agent: Roundcube Webmail/1.2.1 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2017-01-07 02:47, Bjorn Andersson wrote: > 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. That's correct. The qcom_qmp_phy structure represents the QMP phy block as a whole and not the individual phy lane instances. These phy lanes are represented by qcom_phy_desc, and are the actual PHYs in Linux eyes. > > 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. Sure, this sounds good to me - "struct qmp" and "struct qmp_phy" (will call the respective variables as qblk for qmp block (struct qmp) and qphy for struct qmp_phy). Thanks for pointing out. Will change them. Regards Vivek > > Regards, > Bjorn > > -- > To unsubscribe from this list: send the line "unsubscribe > linux-arm-msm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html