Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Tue, 24 Mar 2020 10:49:24 +0100
From:   Philipp Zabel <pza@pengutronix.de>
To:     Wesley Cheng <wcheng@codeaurora.org>
Cc:     agross@kernel.org, bjorn.andersson@linaro.org, kishon@ti.com,
        robh+dt@kernel.org, mark.rutland@arm.com,
        linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org,
        devicetree@vger.kernel.org
Subject: Re: [PATCH v2 2/4] phy: qcom-snps: Add SNPS USB PHY driver for QCOM
 based SOCs
Message-ID: <20200324094924.GA22281@pengutronix.de>
References: <1584994632-31193-1-git-send-email-wcheng@codeaurora.org>
 <1584994632-31193-3-git-send-email-wcheng@codeaurora.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1584994632-31193-3-git-send-email-wcheng@codeaurora.org>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Wesley,

On Mon, Mar 23, 2020 at 01:17:10PM -0700, Wesley Cheng wrote:
> This adds the SNPS FemtoPHY driver used in QCOM SOCs.  There
> are potentially multiple instances of this UTMI PHY on the
> SOC, all which can utilize this driver.
> 
> Signed-off-by: Wesley Cheng <wcheng@codeaurora.org>
> ---
>  drivers/phy/qualcomm/Kconfig             |  10 ++
>  drivers/phy/qualcomm/Makefile            |   1 +
>  drivers/phy/qualcomm/phy-qcom-snps-7nm.c | 294 +++++++++++++++++++++++++++++++
>  3 files changed, 305 insertions(+)
>  create mode 100644 drivers/phy/qualcomm/phy-qcom-snps-7nm.c
> 
[...]
> diff --git a/drivers/phy/qualcomm/phy-qcom-snps-7nm.c b/drivers/phy/qualcomm/phy-qcom-snps-7nm.c
> new file mode 100644
> index 0000000..8d4ba53
> --- /dev/null
> +++ b/drivers/phy/qualcomm/phy-qcom-snps-7nm.c
> @@ -0,0 +1,294 @@
[...]
> +static int qcom_snps_hsphy_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct qcom_snps_hsphy *hsphy;
> +	struct phy_provider *phy_provider;
> +	struct phy *generic_phy;
> +	struct resource *res;
> +	int ret, i;
> +	int num;
> +
> +	hsphy = devm_kzalloc(dev, sizeof(*hsphy), GFP_KERNEL);
> +	if (!hsphy)
> +		return -ENOMEM;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	hsphy->base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(hsphy->base))
> +		return PTR_ERR(hsphy->base);
> +
> +	hsphy->ref_clk = devm_clk_get(dev, "ref");
> +	if (IS_ERR(hsphy->ref_clk)) {
> +		ret = PTR_ERR(hsphy->ref_clk);
> +		if (ret != -EPROBE_DEFER)
> +			dev_err(dev, "failed to get ref clk, %d\n", ret);
> +		return ret;
> +	}
> +
> +	hsphy->phy_reset = devm_reset_control_get_by_index(&pdev->dev, 0);
> +	if (IS_ERR(hsphy->phy_reset)) {
> +		dev_err(dev, "failed to get phy core reset\n");
> +		return PTR_ERR(hsphy->phy_reset);
> +	}

There is only a single reset specified, so there is no need for
_by_index. Also please explicitly request exclusive reset control
for this driver, I suggest:

	hsphy->phy_reset = devm_reset_control_get_exclusive(&pdev->dev, NULL);

If you do want to prepare for future addition of other resets to the
bindings (but if so, why not specify those right now?), you should add
a reset-names property and request the reset control by id string
instead.

regards
Philipp