Re: [PATCH v3 3/6] phy: amlogic: Add Amlogic A1 USB2 PHY Driver

From: Hanjie Lin <hanjie.lin@amlogic.com>
To: Chunfeng Yun <chunfeng.yun@mediatek.com>
Cc: Jerome Brunet <jbrunet@baylibre.com>,
	Neil Armstrong <narmstrong@baylibre.com>,
	Rob Herring <robh@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Kevin Hilman <khilman@baylibre.com>,
	Yue Wang <yue.wang@amlogic.com>,
	<linux-amlogic@lists.infradead.org>,
	<linux-arm-kernel@lists.infradead.org>,
	<linux-usb@vger.kernel.org>, <devicetree@vger.kernel.org>,
	Carlo Caione <carlo@caione.org>,
	Michael Turquette <mturquette@baylibre.com>,
	Stephen Boyd <sboyd@kernel.org>,
	Martin Blumenstingl <martin.blumenstingl@googlemail.com>,
	Liang Yang <liang.yang@amlogic.com>,
	Jianxin Pan <jianxin.pan@amlogic.com>,
	Qiufang Dai <qiufang.dai@amlogic.com>,
	Jian Hu <jian.hu@amlogic.com>,
	Victor Wan <victor.wan@amlogic.com>,
	Xingyu Chen <xingyu.chen@amlogic.com>
Subject: Re: [PATCH v3 3/6] phy: amlogic: Add Amlogic A1 USB2 PHY Driver
Date: Thu, 2 Jan 2020 08:12:56 +0800	[thread overview]
Message-ID: <edacb7ff-65b4-20a8-533f-71c980dcbef2@amlogic.com> (raw)
In-Reply-To: <1577501595.21256.8.camel@mhfsdcap03>

On 2019/12/28 10:53, Chunfeng Yun wrote:
> On Fri, 2019-12-27 at 14:36 +0800, Hanjie Lin wrote:
>> This adds support for the USB2 PHY found in the Amlogic A1 SoC Family.
>>
>> It supports host mode only.
>>
>> Signed-off-by: Hanjie Lin <hanjie.lin@amlogic.com>
>> Signed-off-by: Yue Wang <yue.wang@amlogic.com>
>> ---
>>  drivers/phy/amlogic/phy-meson-g12a-usb2.c | 93 +++++++++++++++++++++----------
>>  1 file changed, 64 insertions(+), 29 deletions(-)
>>
>> diff --git a/drivers/phy/amlogic/phy-meson-g12a-usb2.c b/drivers/phy/amlogic/phy-meson-g12a-usb2.c
>> index 9065ffc..a564747 100644
>> --- a/drivers/phy/amlogic/phy-meson-g12a-usb2.c
>> +++ b/drivers/phy/amlogic/phy-meson-g12a-usb2.c
>> @@ -146,11 +146,17 @@
> [...]
>> +	priv->soc_id = (enum meson_soc_id)of_device_get_match_data(&pdev->dev);
>> +
>>  	priv->regmap = devm_regmap_init_mmio(dev, base,
>>  					     &phy_meson_g12a_usb2_regmap_conf);
>>  	if (IS_ERR(priv->regmap))
>>  		return PTR_ERR(priv->regmap);
>>  
>> -	priv->clk = devm_clk_get(dev, "xtal");
>> -	if (IS_ERR(priv->clk))
>> -		return PTR_ERR(priv->clk);
>> +	if (priv->soc_id == MESON_SOC_G12A) {
>> +		priv->clk = devm_clk_get(dev, "xtal");
>> +		if (IS_ERR(priv->clk))
>> +			return PTR_ERR(priv->clk);
>> +	}
> How about use devm_clk_get_optional(), then make it as optional clock
> also in dt-binding
>>  
> 
>>  
> 

Hi Chunfeng 

Actually, there is a "xtal_usb_phy" clock in A1 ctrl driver, it seems it's
better to be in the A1 phy driver.

I will move that clock here in next version.

Thanks,

Hanjie