Message ID | 20200210105108.1128-1-linux.amoon@gmail.com (mailing list archive) |
---|---|
Headers | show |
Series | Add support for suspend clk for Exynos5422 SoC | expand |
Hi All, Sorry typo this patch series should be PATCHv1 and not PATCHv3 -Anand On Mon, 10 Feb 2020 at 16:21, Anand Moon <linux.amoon@gmail.com> 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/ > > This patch series tries to enable suspend clk using > exynos dwc3 driver, for this I have added new > compatible string "samsung,exynos5420-dwusb3" > so that we could add new suspend clk in addition > to the core clk. exynos dwc3 driver will help > enable/disable these clk. > > -Anand > > Anand Moon (3): > devicetree: bindings: exynos: Add new compatible for Exynos5420 dwc3 > clocks support > ARM: dts: exynos: Add missing usbdrd3 suspend clk > usb: dwc3: exynos: Add support for Exynos5422 suspend clk > > Documentation/devicetree/bindings/usb/exynos-usb.txt | 4 +++- > arch/arm/boot/dts/exynos5420.dtsi | 8 ++++---- > arch/arm/boot/dts/exynos54xx.dtsi | 4 ++-- > drivers/usb/dwc3/dwc3-exynos.c | 9 +++++++++ > 4 files changed, 18 insertions(+), 7 deletions(-) > > -- > 2.25.0 >
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/ > 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 > This patch series tries to enable suspend clk using > exynos dwc3 driver, for this I have added new > compatible string "samsung,exynos5420-dwusb3" > so that we could add new suspend clk in addition > to the core clk. exynos dwc3 driver will help > enable/disable these clk. > > -Anand > > Anand Moon (3): > devicetree: bindings: exynos: Add new compatible for Exynos5420 dwc3 > clocks support > ARM: dts: exynos: Add missing usbdrd3 suspend clk > usb: dwc3: exynos: Add support for Exynos5422 suspend clk > > Documentation/devicetree/bindings/usb/exynos-usb.txt | 4 +++- > arch/arm/boot/dts/exynos5420.dtsi | 8 ++++---- > arch/arm/boot/dts/exynos54xx.dtsi | 4 ++-- > drivers/usb/dwc3/dwc3-exynos.c | 9 +++++++++ > 4 files changed, 18 insertions(+), 7 deletions(-) > > -- > 2.25.0 >
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. 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. -Anand
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