Re: [PATCHv3 0/3] Add support for suspend clk for Exynos5422 SoC

From: Krzysztof Kozlowski <krzk@kernel.org>
To: Anand Moon <linux.amoon@gmail.com>
Cc: Linux USB Mailing List <linux-usb@vger.kernel.org>,
	devicetree <devicetree@vger.kernel.org>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Rob Herring <robh+dt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Felipe Balbi <balbi@kernel.org>,
	Marek Szyprowski <m.szyprowski@samsung.com>,
	Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>,
	linux-samsung-soc@vger.kernel.org,
	Linux Kernel <linux-kernel@vger.kernel.org>
Subject: Re: [PATCHv3 0/3] Add support for suspend clk for Exynos5422 SoC
Date: Mon, 10 Feb 2020 20:02:05 +0100	[thread overview]
Message-ID: <20200210190205.GB16433@kozik-lap> (raw)
In-Reply-To: <CANAwSgT9aq123H-pO2u6iN2E8towsWUFcWDsA9TbVqP30j=10w@mail.gmail.com>

On Mon, Feb 10, 2020 at 10:38:52PM +0530, Anand Moon wrote:
> Hi Krzysztof,
> 
> On Mon, 10 Feb 2020 at 19:26, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >
> > On Mon, Feb 10, 2020 at 10:51:05AM +0000, Anand Moon wrote:
> > > Long time ago I tried to add suspend clk for dwc3 phy
> > > which was wrong appoch, see below.
> > >
> > > [0] https://lore.kernel.org/patchwork/patch/837635/
> > > [1] https://lore.kernel.org/patchwork/patch/837636/
> > >
> >
> 
> Thanks for your review comments.
> 
> > You ignored parts of my review from these previous patches. I asked for
> > describing WHY are you doing this and WHAT problem are you trying to
> > solve. I asked for this multiple times. Unfortunately I cannot find the
> > answers to my questions in this patchset...
> >
> > Best regards,
> > Krzysztof
> 
> I dont know how to resolve this issue, but I want to re-post
> some of my changes back for review. let me try again.
> 
> My future goal is to add #power-domain for FSYS and FSYS2
> which I am trying to resolve some issue.
> Also add run-time power management for USB3 drivers.

You can start by describing why FSYS and FSYS2 power domains cannot be
added right now. Maybe this patchset allows this later?

> 
> Here is the clk diagram for FSYS clk as per Exynos5422 user manual.
> [0] https://imgur.com/gallery/zAiBoyh
> 
> As per the USB 3.0 Architecture T I.
> 
> 2.13.1 PHY Power Management
> The SS PHY has power states P0, P1, P2, and P3, corresponding to the
> SS LPM states of U0, U1, U2,and U3. In the P3 state,SS PHY does not drive
> the default functional clock,instead, the *susp_clk* is used in its place.
> 
> So enable the suspend clk help control the power management
> states for the DWC3 controller.

That's too vague because clock usually cannot "help"... The wording is
wrong and the actual problem is not described.

I could guess from your description and driver behavior that SCLK has to
be on during USB DRD suspend.

Best regards,
Krzysztof