Re: [PATCH v2 2/7] usb: dwc3: Get clocks individually

[Robert Hancock <robert.hancock@calian.com>]

On Tue, 2022-01-18 at 19:24 -0500, Sean Anderson wrote:
> Instead of grabbing all clocks in bulk, grab them individually. This will
> allow us to get the frequency or otherwise deal with discrete clocks. This
> may break some platforms if they use a clock which doesn't use one of the
> documented names.
> 
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
> ---
> 
> Changes in v2:
> - Document clock members
> 
>  drivers/usb/dwc3/core.c | 62 +++++++++++++++++++++++++++++++++--------
>  drivers/usb/dwc3/core.h | 10 ++++---
>  2 files changed, 56 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index f4c09951b517..699ab9abdc47 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -745,6 +745,38 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
>  	return 0;
>  }
>  
> +static int dwc3_clk_enable(struct dwc3 *dwc)
> +{
> +	int ret;
> +
> +	ret = clk_prepare_enable(dwc->bus_clk);
> +	if (ret)
> +		return ret;
> +
> +	ret = clk_prepare_enable(dwc->ref_clk);
> +	if (ret)
> +		goto disable_bus_clk;
> +
> +	ret = clk_prepare_enable(dwc->susp_clk);
> +	if (ret)
> +		goto disable_ref_clk;
> +
> +	return 0;
> +
> +disable_ref_clk:
> +	clk_disable_unprepare(dwc->ref_clk);
> +disable_bus_clk:
> +	clk_disable_unprepare(dwc->bus_clk);
> +	return ret;
> +}
> +
> +static void dwc3_clk_disable(struct dwc3 *dwc)
> +{
> +	clk_disable_unprepare(dwc->susp_clk);
> +	clk_disable_unprepare(dwc->ref_clk);
> +	clk_disable_unprepare(dwc->bus_clk);
> +}
> +
>  static void dwc3_core_exit(struct dwc3 *dwc)
>  {
>  	dwc3_event_buffers_cleanup(dwc);
> @@ -758,7 +790,7 @@ static void dwc3_core_exit(struct dwc3 *dwc)
>  	usb_phy_set_suspend(dwc->usb3_phy, 1);
>  	phy_power_off(dwc->usb2_generic_phy);
>  	phy_power_off(dwc->usb3_generic_phy);
> -	clk_bulk_disable_unprepare(dwc->num_clks, dwc->clks);
> +	dwc3_clk_disable(dwc);
>  	reset_control_assert(dwc->reset);
>  }
>  
> @@ -1605,25 +1637,31 @@ static int dwc3_probe(struct platform_device *pdev)
>  		return PTR_ERR(dwc->reset);
>  
>  	if (dev->of_node) {
> -		ret = devm_clk_bulk_get_all(dev, &dwc->clks);
> -		if (ret == -EPROBE_DEFER)
> -			return ret;
>  		/*
>  		 * Clocks are optional, but new DT platforms should support all
>  		 * clocks as required by the DT-binding.
>  		 */
> -		if (ret < 0)
> -			dwc->num_clks = 0;
> -		else
> -			dwc->num_clks = ret;
> +		dwc->bus_clk = devm_clk_get_optional(dev, "bus_early");
> +		if (IS_ERR(dwc->bus_clk))
> +			return dev_err_probe(dev, PTR_ERR(dwc->bus_clk),
> +					     "could not get bus clock\n");
>  
> +		dwc->ref_clk = devm_clk_get_optional(dev, "ref");
> +		if (IS_ERR(dwc->ref_clk))
> +			return dev_err_probe(dev, PTR_ERR(dwc->ref_clk),
> +					     "could not get ref clock\n");
> +
> +		dwc->susp_clk = devm_clk_get_optional(dev, "suspend");
> +		if (IS_ERR(dwc->susp_clk))
> +			return dev_err_probe(dev, PTR_ERR(dwc->susp_clk),
> +					     "could not get suspend clock\n");
>  	}
>  
>  	ret = reset_control_deassert(dwc->reset);
>  	if (ret)
>  		return ret;
>  
> -	ret = clk_bulk_prepare_enable(dwc->num_clks, dwc->clks);
> +	ret = dwc3_clk_enable(dwc);
>  	if (ret)
>  		goto assert_reset;
>  
> @@ -1711,7 +1749,7 @@ static int dwc3_probe(struct platform_device *pdev)
>  	pm_runtime_disable(&pdev->dev);
>  
>  disable_clks:
> -	clk_bulk_disable_unprepare(dwc->num_clks, dwc->clks);
> +	dwc3_clk_disable(dwc);
>  assert_reset:
>  	reset_control_assert(dwc->reset);
>  
> @@ -1755,7 +1793,7 @@ static int dwc3_core_init_for_resume(struct dwc3 *dwc)
>  	if (ret)
>  		return ret;
>  
> -	ret = clk_bulk_prepare_enable(dwc->num_clks, dwc->clks);
> +	ret = dwc3_clk_enable(dwc);
>  	if (ret)
>  		goto assert_reset;
>  
> @@ -1766,7 +1804,7 @@ static int dwc3_core_init_for_resume(struct dwc3 *dwc)
>  	return 0;
>  
>  disable_clks:
> -	clk_bulk_disable_unprepare(dwc->num_clks, dwc->clks);
> +	dwc3_clk_disable(dwc);
>  assert_reset:
>  	reset_control_assert(dwc->reset);
>  
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index e1cc3f7398fb..45cfa7d9f27a 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -978,8 +978,9 @@ struct dwc3_scratchpad_array {
>   * @eps: endpoint array
>   * @gadget: device side representation of the peripheral controller
>   * @gadget_driver: pointer to the gadget driver
> - * @clks: array of clocks
> - * @num_clks: number of clocks
> + * @bus_clk: clock for accessing the registers
> + * @ref_clk: reference clock
> + * @susp_clk: clock used when the SS phy is in low power (S3) state
>   * @reset: reset control
>   * @regs: base address for our registers
>   * @regs_size: address space size
> @@ -1134,8 +1135,9 @@ struct dwc3 {
>  	struct usb_gadget	*gadget;
>  	struct usb_gadget_driver *gadget_driver;
>  
> -	struct clk_bulk_data	*clks;
> -	int			num_clks;
> +	struct clk		*bus_clk;
> +	struct clk		*ref_clk;
> +	struct clk		*susp_clk;
>  
>  	struct reset_control	*reset;
>  

Reviewed-by: Robert Hancock <robert.hancock@calian.com>