From: Stephen Boyd <email@example.com> To: Michael Turquette <firstname.lastname@example.org>, Taniya Das <email@example.com> Cc: Andy Gross <firstname.lastname@example.org>, David Brown <email@example.com>, Rajendra Nayak <firstname.lastname@example.org>, email@example.com, firstname.lastname@example.org, email@example.com, firstname.lastname@example.org Subject: Re: [PATCH v6] clk: qcom: Add lpass clock controller driver for SDM845 Date: Fri, 12 Oct 2018 10:35:44 -0700 [thread overview] Message-ID: <email@example.com> (raw) In-Reply-To: <firstname.lastname@example.org> Quoting Taniya Das (2018-10-09 23:12:27) > > > On 10/10/2018 2:22 AM, Stephen Boyd wrote: > > Quoting Taniya Das (2018-10-09 10:26:38) > >> Hello Stephen, > >> > >> On 10/8/2018 8:14 AM, Stephen Boyd wrote: > >>> Quoting Taniya Das (2018-10-04 05:02:26) > >>>> Add support for the lpass clock controller found on SDM845 based devices. > >>>> This would allow lpass peripheral loader drivers to control the clocks to > >>>> bring the subsystem out of reset. > >>>> LPASS clocks present on the global clock controller would be registered > >>>> with the clock framework based on the device tree flag. Also do not gate > >>>> these clocks if they are left unused. > >>> > >>> Why not gate them? This statement states what the code is doing, not why > >>> it's doing it which is the more crucial information that should be > >>> described in the commit text. Also, please add a comment about it to the > >>> code next to the flag. > >>> > >>> I am concerned that it doesn't make any sense though, so probably it > >>> shouldn't be marked as CLK_IGNORE_UNUSED and it's papering over some > >>> other larger bug that needs to be fixed. > >>> > >> > >> It does not have any bug, it is just that to access these lpass > >> registers we would need the GCC lpass registers to be enabled. I would > >> update the same in the commit text. > >> > >> During clock late_init these clocks should not be accessed to check the > >> clock status as they would result in unclocked access. The client would > >> request these clocks in the correct order and it would not have any issue. > >> > > > > That seems like the bug right there. If the LPASS registers can't be > > accessed unless the clks in GCC are enabled then this driver needs to > > turn the clks on before reading/writing registers. Marking the clks as > > ignore unused is skipping around the real problem. > > > > If the driver requests for the clocks they would maintain the order. But > if the clock late init call is invoked before the driver requests, there > is no way I could manage this dependency, that is the only reason to > mark them unused. > Which driver are we talking about here? The lpass clk driver? Presumably the lpass clk driver would request the GCC clks and turn them on in probe and then register any lpass clks. If the lpass clk driver probes bfeore late init, then the gcc clks will be enabled and everything works, and if the lpass clk driver probes after late init then the clks that can't be touched without gcc clks enabled won't be registered, and then they won't be touched. What goes wrong?
next prev parent reply other threads:[~2018-10-12 17:35 UTC|newest] Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-10-04 12:02 [PATCH v6] Add support for LPASS clock controller " Taniya Das 2018-10-04 12:02 ` [PATCH v6] clk: qcom: Add lpass clock controller driver " Taniya Das 2018-10-08 2:44 ` Stephen Boyd 2018-10-09 17:26 ` Taniya Das 2018-10-09 20:52 ` Stephen Boyd 2018-10-10 6:12 ` Taniya Das 2018-10-12 17:35 ` Stephen Boyd [this message] 2018-10-17 11:37 ` Taniya Das 2018-10-17 12:04 ` Taniya Das 2018-10-17 14:20 ` Stephen Boyd 2018-10-19 10:39 ` Taniya Das 2018-10-25 10:51 ` tdas 2018-10-29 18:44 ` Stephen Boyd
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --subject='Re: [PATCH v6] clk: qcom: Add lpass clock controller driver for SDM845' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).