[PATCH v2 09/12] phy: socionext: Add UniPhier USB3 PHY driver

[Kunihiko Hayashi]

Hi Marek,

Thank you for reviewing.

On 2023/02/02 6:54, Marek Vasut wrote:
> On 2/1/23 02:13, Kunihiko Hayashi wrote:
>> Add USB3 PHY driver support to control clocks and resets for the phy.
>>
>> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko at socionext.com>
>> ---
>>    configs/uniphier_v8_defconfig             |  1 +
>>    drivers/phy/socionext/Kconfig             |  7 ++
>>    drivers/phy/socionext/Makefile            |  1 +
>>    drivers/phy/socionext/phy-uniphier-usb3.c | 89 +++++++++++++++++++++++
>>    4 files changed, 98 insertions(+)
>>    create mode 100644 drivers/phy/socionext/phy-uniphier-usb3.c
>>
>> diff --git a/configs/uniphier_v8_defconfig b/configs/uniphier_v8_defconfig
>> index 6a0e2666cf..5cfa9fc01d 100644
>> --- a/configs/uniphier_v8_defconfig
>> +++ b/configs/uniphier_v8_defconfig
>> @@ -78,3 +78,4 @@ CONFIG_USB_DWC3=y
>>    CONFIG_USB_DWC3_UNIPHIER=y
>>    CONFIG_PANIC_HANG=y
>>    CONFIG_FDT_FIXUP_PARTITIONS=y
>> +CONFIG_PHY_UNIPHIER_USB3=y
> 
> This should be in 12/12 patch, right ?

Yes, this should be.

>> diff --git a/drivers/phy/socionext/Kconfig b/drivers/phy/socionext/Kconfig
>> index bcd579e98e..fc63f4c042 100644
>> --- a/drivers/phy/socionext/Kconfig
>> +++ b/drivers/phy/socionext/Kconfig
>> @@ -10,3 +10,10 @@ config PHY_UNIPHIER_PCIE
>>    	help
>>    	  Enable this to support PHY implemented in PCIe controller
>>    	  on UniPhier SoCs.
>> +
>> +config PHY_UNIPHIER_USB3
>> +	bool "UniPhier USB3 PHY driver"
>> +	depends on PHY && ARCH_UNIPHIER
> 
> Maybe 'default y if SOMETHING' here, so you won't need to adjust defconfig ?

OK. this phy is mandatory for the controller.

>> +	help
>> +	  Enable this to support PHY implemented in USB3 controller
>> +	  on UniPhier SoCs.
>> diff --git a/drivers/phy/socionext/Makefile
>> b/drivers/phy/socionext/Makefile
>> index 5484360b70..94d3aa68cf 100644
>> --- a/drivers/phy/socionext/Makefile
>> +++ b/drivers/phy/socionext/Makefile
>> @@ -4,3 +4,4 @@
>>    #
>>
>>    obj-$(CONFIG_PHY_UNIPHIER_PCIE)	+= phy-uniphier-pcie.o
>> +obj-$(CONFIG_PHY_UNIPHIER_USB3)	+= phy-uniphier-usb3.o
>> diff --git a/drivers/phy/socionext/phy-uniphier-usb3.c
>> b/drivers/phy/socionext/phy-uniphier-usb3.c
>> new file mode 100644
>> index 0000000000..a2e44a52c1
>> --- /dev/null
>> +++ b/drivers/phy/socionext/phy-uniphier-usb3.c
>> @@ -0,0 +1,89 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * phy_uniphier_usb3.c - Socionext UniPhier Usb3 PHY driver
>> + * Copyright 2019-2021 Socionext, Inc.
> 
> 2023 instead of 2021 .

I'll fix it.

>> + */
>> +
>> +#include <common.h>
>> +#include <dm.h>
>> +#include <generic-phy.h>
>> +
>> +#include <clk.h>
>> +#include <reset.h>
>> +
>> +struct uniphier_usb3phy_priv {
>> +	struct clk_bulk clks;
>> +	struct reset_ctl_bulk rsts;
>> +};
>> +
>> +static int uniphier_usb3phy_init(struct phy *phy)
>> +{
>> +	struct uniphier_usb3phy_priv *priv = dev_get_priv(phy->dev);
>> +	int ret;
>> +
>> +	ret = clk_enable_bulk(&priv->clks);
>> +	if (ret) {
>> +		clk_release_bulk(&priv->clks);
>> +		return ret;
>> +	}
>> +
>> +	ret = reset_deassert_bulk(&priv->rsts);
>> +	if (ret) {
>> +		reset_release_bulk(&priv->rsts);
> 
> You have to disable clock here too.

Yes. I'll add it.

>> +		return ret;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int uniphier_usb3phy_probe(struct udevice *dev)
>> +{
>> +	struct uniphier_usb3phy_priv *priv = dev_get_priv(dev);
>> +	int ret;
>> +
>> +	ret = clk_get_bulk(dev, &priv->clks);
>> +	if (ret) {
>> +		if (ret != -ENOSYS && ret != -ENOENT) {
>> +			printf("Failed to get clocks\n");
>> +			return ret;
>> +		}
>> +	}
>> +
>> +	ret = reset_get_bulk(dev, &priv->rsts);
>> +	if (ret) {
>> +		if (ret != -ENOSYS && ret != -ENOENT) {
>> +			printf("Failed to get resets\n");
> 
> You have to release resets here too (use fail path, i.e. goto err_clock).

I see. I'll consider adding release or using "devm" function.

Thank you,

---
Best Regards
Kunihiko Hayashi