[v1,2/2] usb: cdns3: cdns3-plat: Add clk and reset init

Message ID 20230502081805.112149-3-minda.chen@starfivetech.com
State New
Headers
Series Add clock and reset in cdns3 platform |

Commit Message

Minda Chen May 2, 2023, 8:18 a.m. UTC
  Add gereric clk and reset init codes to Cadence USBSS
controller. The codes has been tested by starfive vf2
board.

Signed-off-by: Minda Chen <minda.chen@starfivetech.com>
---
 drivers/usb/cdns3/cdns3-plat.c | 52 ++++++++++++++++++++++++++++++++++
 drivers/usb/cdns3/core.h       |  3 ++
 2 files changed, 55 insertions(+)
  

Comments

Roger Quadros May 4, 2023, 12:45 p.m. UTC | #1
Hi,

On 02/05/2023 11:18, Minda Chen wrote:
> Add gereric clk and reset init codes to Cadence USBSS
> controller. The codes has been tested by starfive vf2
> board.
> 
> Signed-off-by: Minda Chen <minda.chen@starfivetech.com>
> ---
>  drivers/usb/cdns3/cdns3-plat.c | 52 ++++++++++++++++++++++++++++++++++
>  drivers/usb/cdns3/core.h       |  3 ++
>  2 files changed, 55 insertions(+)
> 
> diff --git a/drivers/usb/cdns3/cdns3-plat.c b/drivers/usb/cdns3/cdns3-plat.c
> index 2bc5d094548b..1820844c74d2 100644
> --- a/drivers/usb/cdns3/cdns3-plat.c
> +++ b/drivers/usb/cdns3/cdns3-plat.c
> @@ -12,11 +12,13 @@
>   *         Roger Quadros <rogerq@ti.com>
>   */
>  
> +#include <linux/clk.h>
>  #include <linux/module.h>
>  #include <linux/irq.h>
>  #include <linux/kernel.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/reset.h>
>  
>  #include "core.h"
>  #include "gadget-export.h"
> @@ -43,6 +45,34 @@ static void set_phy_power_off(struct cdns *cdns)
>  	phy_power_off(cdns->usb2_phy);
>  }
>  
> +static int cdns3_clk_rst_init(struct cdns *cdns)
> +{
> +	int ret;
> +
> +	if (cdns->num_clks) {
> +		ret = clk_bulk_prepare_enable(cdns->num_clks, cdns->clks);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	ret = reset_control_deassert(cdns->resets);
> +	if (ret && cdns->num_clks)
> +		goto err_clk_init;

if (ret)
	goto err_clk_init;

> +
> +	return ret;
> +
> +err_clk_init:
> +	clk_bulk_disable_unprepare(cdns->num_clks, cdns->clks);

if (cdns->num_clks)
	clk_bulk_disable_unprepare(cdns->num_clks, cdns->clks);

This way is more nicer I think.

> +	return ret;
> +}
> +
> +static void cdns3_clk_rst_deinit(struct cdns *cdns)
> +{
> +	reset_control_assert(cdns->resets);
> +	if (cdns->num_clks)
> +		clk_bulk_disable_unprepare(cdns->num_clks, cdns->clks);
> +}
> +
>  /**
>   * cdns3_plat_probe - probe for cdns3 core device
>   * @pdev: Pointer to cdns3 core platform device
> @@ -116,6 +146,16 @@ static int cdns3_plat_probe(struct platform_device *pdev)
>  		cdns->wakeup_irq = 0x0;
>  	}
>  
> +	ret = devm_clk_bulk_get_all(dev, &cdns->clks);
> +	if (ret < 0)
> +		return ret;
> +
> +	cdns->num_clks = ret;
> +
> +	cdns->resets = devm_reset_control_array_get_optional_exclusive(dev);
> +	if (IS_ERR(cdns->resets))
> +		return PTR_ERR(cdns->resets);
> +
>  	cdns->usb2_phy = devm_phy_optional_get(dev, "cdns3,usb2-phy");
>  	if (IS_ERR(cdns->usb2_phy))
>  		return PTR_ERR(cdns->usb2_phy);
> @@ -128,6 +168,10 @@ static int cdns3_plat_probe(struct platform_device *pdev)
>  	if (IS_ERR(cdns->usb3_phy))
>  		return PTR_ERR(cdns->usb3_phy);
>  
> +	ret = cdns3_clk_rst_init(cdns);
> +	if (ret)
> +		return ret;
> +
>  	ret = phy_init(cdns->usb3_phy);
>  	if (ret)
>  		goto err_phy3_init;
> @@ -165,6 +209,7 @@ static int cdns3_plat_probe(struct platform_device *pdev)
>  	phy_exit(cdns->usb3_phy);
>  err_phy3_init:
>  	phy_exit(cdns->usb2_phy);
> +	cdns3_clk_rst_deinit(cdns);
>  
>  	return ret;
>  }
> @@ -187,6 +232,8 @@ static int cdns3_plat_remove(struct platform_device *pdev)
>  	set_phy_power_off(cdns);
>  	phy_exit(cdns->usb2_phy);
>  	phy_exit(cdns->usb3_phy);
> +	cdns3_clk_rst_deinit(cdns);
> +
>  	return 0;
>  }
>  
> @@ -220,6 +267,8 @@ static int cdns3_controller_suspend(struct device *dev, pm_message_t msg)
>  
>  	cdns3_set_platform_suspend(cdns->dev, true, wakeup);
>  	set_phy_power_off(cdns);
> +	if (!PMSG_IS_AUTO(msg))
> +		cdns3_clk_rst_deinit(cdns);

If you reset the controller here all state will be lost.
How is it expected to work on system resume?

>  	spin_lock_irqsave(&cdns->lock, flags);
>  	cdns->in_lpm = true;
>  	spin_unlock_irqrestore(&cdns->lock, flags);
> @@ -237,6 +286,9 @@ static int cdns3_controller_resume(struct device *dev, pm_message_t msg)
>  	if (!cdns->in_lpm)
>  		return 0;
>  
> +	if (!PMSG_IS_AUTO(msg))
> +		cdns3_clk_rst_init(cdns);
> +
>  	if (cdns_power_is_lost(cdns)) {
>  		phy_exit(cdns->usb2_phy);
>  		ret = phy_init(cdns->usb2_phy);
> diff --git a/drivers/usb/cdns3/core.h b/drivers/usb/cdns3/core.h
> index 2d332a788871..b894768ee485 100644
> --- a/drivers/usb/cdns3/core.h
> +++ b/drivers/usb/cdns3/core.h
> @@ -111,6 +111,9 @@ struct cdns {
>  	struct mutex			mutex;
>  	enum usb_dr_mode		dr_mode;
>  	struct usb_role_switch		*role_sw;
> +	struct reset_control *resets;
> +	struct clk_bulk_data *clks;
> +	int num_clks;
>  	bool				in_lpm;
>  	bool				wakeup_pending;
>  	struct cdns3_platform_data	*pdata;

cheers,
-roger
  
Minda Chen May 6, 2023, 1:50 a.m. UTC | #2
On 2023/5/4 20:45, Roger Quadros wrote:
> Hi,
> 
> On 02/05/2023 11:18, Minda Chen wrote:
>> Add gereric clk and reset init codes to Cadence USBSS
>> controller. The codes has been tested by starfive vf2
>> board.
>> 
>> Signed-off-by: Minda Chen <minda.chen@starfivetech.com>
>> ---
>>  drivers/usb/cdns3/cdns3-plat.c | 52 ++++++++++++++++++++++++++++++++++
>>  drivers/usb/cdns3/core.h       |  3 ++
>>  2 files changed, 55 insertions(+)
>> 
>> diff --git a/drivers/usb/cdns3/cdns3-plat.c b/drivers/usb/cdns3/cdns3-plat.c
>> index 2bc5d094548b..1820844c74d2 100644
>> --- a/drivers/usb/cdns3/cdns3-plat.c
>> +++ b/drivers/usb/cdns3/cdns3-plat.c
>> @@ -12,11 +12,13 @@
>>   *         Roger Quadros <rogerq@ti.com>
>>   */
>>  
>> +#include <linux/clk.h>
>>  #include <linux/module.h>
>>  #include <linux/irq.h>
>>  #include <linux/kernel.h>
>>  #include <linux/platform_device.h>
>>  #include <linux/pm_runtime.h>
>> +#include <linux/reset.h>
>>  
>>  #include "core.h"
>>  #include "gadget-export.h"
>> @@ -43,6 +45,34 @@ static void set_phy_power_off(struct cdns *cdns)
>>  	phy_power_off(cdns->usb2_phy);
>>  }
>>  
>> +static int cdns3_clk_rst_init(struct cdns *cdns)
>> +{
>> +	int ret;
>> +
>> +	if (cdns->num_clks) {
>> +		ret = clk_bulk_prepare_enable(cdns->num_clks, cdns->clks);
>> +		if (ret)
>> +			return ret;
>> +	}
>> +
>> +	ret = reset_control_deassert(cdns->resets);
>> +	if (ret && cdns->num_clks)
>> +		goto err_clk_init;
> 
> if (ret)
> 	goto err_clk_init;
> 
>> +
>> +	return ret;
>> +
>> +err_clk_init:
>> +	clk_bulk_disable_unprepare(cdns->num_clks, cdns->clks);
> 
> if (cdns->num_clks)
> 	clk_bulk_disable_unprepare(cdns->num_clks, cdns->clks);
> 
> This way is more nicer I think.
> 
OK, thanks.
>> +	return ret;
>> +}
>> +
>> +static void cdns3_clk_rst_deinit(struct cdns *cdns)
>> +{
>> +	reset_control_assert(cdns->resets);
>> +	if (cdns->num_clks)
>> +		clk_bulk_disable_unprepare(cdns->num_clks, cdns->clks);
>> +}
>> +
>>  /**
>>   * cdns3_plat_probe - probe for cdns3 core device
>>   * @pdev: Pointer to cdns3 core platform device
>> @@ -116,6 +146,16 @@ static int cdns3_plat_probe(struct platform_device *pdev)
>>  		cdns->wakeup_irq = 0x0;
>>  	}
>>  
>> +	ret = devm_clk_bulk_get_all(dev, &cdns->clks);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	cdns->num_clks = ret;
>> +
>> +	cdns->resets = devm_reset_control_array_get_optional_exclusive(dev);
>> +	if (IS_ERR(cdns->resets))
>> +		return PTR_ERR(cdns->resets);
>> +
>>  	cdns->usb2_phy = devm_phy_optional_get(dev, "cdns3,usb2-phy");
>>  	if (IS_ERR(cdns->usb2_phy))
>>  		return PTR_ERR(cdns->usb2_phy);
>> @@ -128,6 +168,10 @@ static int cdns3_plat_probe(struct platform_device *pdev)
>>  	if (IS_ERR(cdns->usb3_phy))
>>  		return PTR_ERR(cdns->usb3_phy);
>>  
>> +	ret = cdns3_clk_rst_init(cdns);
>> +	if (ret)
>> +		return ret;
>> +
>>  	ret = phy_init(cdns->usb3_phy);
>>  	if (ret)
>>  		goto err_phy3_init;
>> @@ -165,6 +209,7 @@ static int cdns3_plat_probe(struct platform_device *pdev)
>>  	phy_exit(cdns->usb3_phy);
>>  err_phy3_init:
>>  	phy_exit(cdns->usb2_phy);
>> +	cdns3_clk_rst_deinit(cdns);
>>  
>>  	return ret;
>>  }
>> @@ -187,6 +232,8 @@ static int cdns3_plat_remove(struct platform_device *pdev)
>>  	set_phy_power_off(cdns);
>>  	phy_exit(cdns->usb2_phy);
>>  	phy_exit(cdns->usb3_phy);
>> +	cdns3_clk_rst_deinit(cdns);
>> +
>>  	return 0;
>>  }
>>  
>> @@ -220,6 +267,8 @@ static int cdns3_controller_suspend(struct device *dev, pm_message_t msg)
>>  
>>  	cdns3_set_platform_suspend(cdns->dev, true, wakeup);
>>  	set_phy_power_off(cdns);
>> +	if (!PMSG_IS_AUTO(msg))
>> +		cdns3_clk_rst_deinit(cdns);
> 
> If you reset the controller here all state will be lost.
> How is it expected to work on system resume?
> 
I will delete them. This is platform codes. 
>>  	spin_lock_irqsave(&cdns->lock, flags);
>>  	cdns->in_lpm = true;
>>  	spin_unlock_irqrestore(&cdns->lock, flags);
>> @@ -237,6 +286,9 @@ static int cdns3_controller_resume(struct device *dev, pm_message_t msg)
>>  	if (!cdns->in_lpm)
>>  		return 0;
>>  
>> +	if (!PMSG_IS_AUTO(msg))
>> +		cdns3_clk_rst_init(cdns);
>> +
>>  	if (cdns_power_is_lost(cdns)) {
>>  		phy_exit(cdns->usb2_phy);
>>  		ret = phy_init(cdns->usb2_phy);
>> diff --git a/drivers/usb/cdns3/core.h b/drivers/usb/cdns3/core.h
>> index 2d332a788871..b894768ee485 100644
>> --- a/drivers/usb/cdns3/core.h
>> +++ b/drivers/usb/cdns3/core.h
>> @@ -111,6 +111,9 @@ struct cdns {
>>  	struct mutex			mutex;
>>  	enum usb_dr_mode		dr_mode;
>>  	struct usb_role_switch		*role_sw;
>> +	struct reset_control *resets;
>> +	struct clk_bulk_data *clks;
>> +	int num_clks;
>>  	bool				in_lpm;
>>  	bool				wakeup_pending;
>>  	struct cdns3_platform_data	*pdata;
> 
> cheers,
> -roger
  

Patch

diff --git a/drivers/usb/cdns3/cdns3-plat.c b/drivers/usb/cdns3/cdns3-plat.c
index 2bc5d094548b..1820844c74d2 100644
--- a/drivers/usb/cdns3/cdns3-plat.c
+++ b/drivers/usb/cdns3/cdns3-plat.c
@@ -12,11 +12,13 @@ 
  *         Roger Quadros <rogerq@ti.com>
  */
 
+#include <linux/clk.h>
 #include <linux/module.h>
 #include <linux/irq.h>
 #include <linux/kernel.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
+#include <linux/reset.h>
 
 #include "core.h"
 #include "gadget-export.h"
@@ -43,6 +45,34 @@  static void set_phy_power_off(struct cdns *cdns)
 	phy_power_off(cdns->usb2_phy);
 }
 
+static int cdns3_clk_rst_init(struct cdns *cdns)
+{
+	int ret;
+
+	if (cdns->num_clks) {
+		ret = clk_bulk_prepare_enable(cdns->num_clks, cdns->clks);
+		if (ret)
+			return ret;
+	}
+
+	ret = reset_control_deassert(cdns->resets);
+	if (ret && cdns->num_clks)
+		goto err_clk_init;
+
+	return ret;
+
+err_clk_init:
+	clk_bulk_disable_unprepare(cdns->num_clks, cdns->clks);
+	return ret;
+}
+
+static void cdns3_clk_rst_deinit(struct cdns *cdns)
+{
+	reset_control_assert(cdns->resets);
+	if (cdns->num_clks)
+		clk_bulk_disable_unprepare(cdns->num_clks, cdns->clks);
+}
+
 /**
  * cdns3_plat_probe - probe for cdns3 core device
  * @pdev: Pointer to cdns3 core platform device
@@ -116,6 +146,16 @@  static int cdns3_plat_probe(struct platform_device *pdev)
 		cdns->wakeup_irq = 0x0;
 	}
 
+	ret = devm_clk_bulk_get_all(dev, &cdns->clks);
+	if (ret < 0)
+		return ret;
+
+	cdns->num_clks = ret;
+
+	cdns->resets = devm_reset_control_array_get_optional_exclusive(dev);
+	if (IS_ERR(cdns->resets))
+		return PTR_ERR(cdns->resets);
+
 	cdns->usb2_phy = devm_phy_optional_get(dev, "cdns3,usb2-phy");
 	if (IS_ERR(cdns->usb2_phy))
 		return PTR_ERR(cdns->usb2_phy);
@@ -128,6 +168,10 @@  static int cdns3_plat_probe(struct platform_device *pdev)
 	if (IS_ERR(cdns->usb3_phy))
 		return PTR_ERR(cdns->usb3_phy);
 
+	ret = cdns3_clk_rst_init(cdns);
+	if (ret)
+		return ret;
+
 	ret = phy_init(cdns->usb3_phy);
 	if (ret)
 		goto err_phy3_init;
@@ -165,6 +209,7 @@  static int cdns3_plat_probe(struct platform_device *pdev)
 	phy_exit(cdns->usb3_phy);
 err_phy3_init:
 	phy_exit(cdns->usb2_phy);
+	cdns3_clk_rst_deinit(cdns);
 
 	return ret;
 }
@@ -187,6 +232,8 @@  static int cdns3_plat_remove(struct platform_device *pdev)
 	set_phy_power_off(cdns);
 	phy_exit(cdns->usb2_phy);
 	phy_exit(cdns->usb3_phy);
+	cdns3_clk_rst_deinit(cdns);
+
 	return 0;
 }
 
@@ -220,6 +267,8 @@  static int cdns3_controller_suspend(struct device *dev, pm_message_t msg)
 
 	cdns3_set_platform_suspend(cdns->dev, true, wakeup);
 	set_phy_power_off(cdns);
+	if (!PMSG_IS_AUTO(msg))
+		cdns3_clk_rst_deinit(cdns);
 	spin_lock_irqsave(&cdns->lock, flags);
 	cdns->in_lpm = true;
 	spin_unlock_irqrestore(&cdns->lock, flags);
@@ -237,6 +286,9 @@  static int cdns3_controller_resume(struct device *dev, pm_message_t msg)
 	if (!cdns->in_lpm)
 		return 0;
 
+	if (!PMSG_IS_AUTO(msg))
+		cdns3_clk_rst_init(cdns);
+
 	if (cdns_power_is_lost(cdns)) {
 		phy_exit(cdns->usb2_phy);
 		ret = phy_init(cdns->usb2_phy);
diff --git a/drivers/usb/cdns3/core.h b/drivers/usb/cdns3/core.h
index 2d332a788871..b894768ee485 100644
--- a/drivers/usb/cdns3/core.h
+++ b/drivers/usb/cdns3/core.h
@@ -111,6 +111,9 @@  struct cdns {
 	struct mutex			mutex;
 	enum usb_dr_mode		dr_mode;
 	struct usb_role_switch		*role_sw;
+	struct reset_control *resets;
+	struct clk_bulk_data *clks;
+	int num_clks;
 	bool				in_lpm;
 	bool				wakeup_pending;
 	struct cdns3_platform_data	*pdata;