[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1c242d0114554b72f8f0b1f6507973a0dea50b3d.camel@calian.com>
Date: Sat, 15 Jan 2022 01:04:36 +0000
From: Robert Hancock <robert.hancock@...ian.com>
To: "sean.anderson@...o.com" <sean.anderson@...o.com>,
"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"bjagadee@...eaurora.org" <bjagadee@...eaurora.org>,
"balbi@...nel.org" <balbi@...nel.org>,
"baruch@...s.co.il" <baruch@...s.co.il>
Subject: Re: [PATCH 2/6] usb: dwc3: Get clocks individually
On Fri, 2022-01-14 at 18:39 -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.
Another approach would be to keep the bulk get and prepare_enable and just
search through the clocks in the bulk data to find the "ref" clock, i.e.
something like:
for (i = 0; i < dwc->num_clks; ++i)
if (!strcmp("ref", dwc->clks[i].id)) {
ref_clk_rate = clk_get_rate(dwc->clks[i].clk);
break;
}
That's probably simpler than all the extra complexity to get each of the clocks
individually and release them in the right order.
>
> Signed-off-by: Sean Anderson <sean.anderson@...o.com>
> ---
>
> drivers/usb/dwc3/core.c | 62 +++++++++++++++++++++++++++++++++--------
> drivers/usb/dwc3/core.h | 5 ++--
> 2 files changed, 53 insertions(+), 14 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..32dfcf3a83d5 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -1134,8 +1134,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;
>
--
Robert Hancock
Senior Hardware Designer, Calian Advanced Technologies
www.calian.com
Powered by blists - more mailing lists