From mboxrd@z Thu Jan  1 00:00:00 1970
From: Patrick DELAUNAY <patrick.delaunay@st.com>
Date: Wed, 6 Nov 2019 18:03:23 +0000
Subject: [U-Boot] [RESEND PATCH 2/5] usb: host: dwc2: add support for clk
In-Reply-To: <77804378-5ea1-4b37-cb54-8fce77351d9b@denx.de>
References: <20191014080025.11245-1-patrick.delaunay@st.com>
 <20191014080025.11245-3-patrick.delaunay@st.com>
 <77804378-5ea1-4b37-cb54-8fce77351d9b@denx.de>
Message-ID: <fd4aa0d388ed49269628a7f8cc9a75d0@SFHDAG6NODE3.st.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Marek,

> From: Marek Vasut <marex@denx.de>
> Sent: mardi 15 octobre 2019 01:28
> 
> On 10/14/19 10:00 AM, Patrick Delaunay wrote:
> > Add support for clock with driver model.
> >
> 
> Same question as with the PHY -- is there now a mandatory dependency on the
> DM CLK ?

No I don't think.

Because the clk function are also stubbed in ./include/clk.h
CONFIG_IS_ENABLED(CLK)

But I don't 100% sure as I don't tested it on one platform without DM_CLK...
 
> [...]
> 
> > @@ -1403,6 +1429,7 @@ static int dwc2_usb_remove(struct udevice *dev)
> >  	dwc2_uninit_common(priv->regs);
> >
> >  	reset_release_bulk(&priv->resets);
> > +	clk_release_bulk(&priv->clks);
> 
> Shouldn't there be some clk_...disable() here ?

I don't like make clk_....disable() in U-Boot remove function because the clock
u-class don't managed a counter for each clock user (as it is done in kernel).

We have always a risk to deactivate a clock needed by a several device:
each driver (A&B) enable a common clock with U-Boot clock function, 
but the first clock disable (A) really deactivate the clock even it is still needed
by the other driver (B)

I use the same logical than dwc3 driver: clk_disable_bulk is not called.

static int dwc3_glue_remove(struct udevice *dev)
{
	struct dwc3_glue_data *glue = dev_get_platdata(dev);

	reset_release_bulk(&glue->resets);

	clk_release_bulk(&glue->clks);

	return 0;
}

Regards
Patrick