Linux-SPI Archive on lore.kernel.org
 help / color / Atom feed
From: Matthias Kaehlcke <mka-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
To: Akash Asthana <akashast-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Cc: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org,
	agross-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
	wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org,
	broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	mark.rutland-5wv7dgnIgG8@public.gmane.org,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-spi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	swboyd-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org,
	mgautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org,
	evgreen-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org
Subject: Re: [PATCH V2 3/8] soc: qcom-geni-se: Add interconnect support to fix earlycon crash
Date: Tue, 17 Mar 2020 11:29:10 -0700
Message-ID: <20200317182910.GR144492@google.com> (raw)
In-Reply-To: <1f86fdf0-df7c-4e4a-d4d8-8b0162e52cb4-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>

Hi Akash,

On Tue, Mar 17, 2020 at 04:27:47PM +0530, Akash Asthana wrote:
> Hi Matthias,
> 
> On 3/14/2020 2:14 AM, Matthias Kaehlcke wrote:
> > Hi Akash,
> > 
> > On Fri, Mar 13, 2020 at 06:42:09PM +0530, Akash Asthana wrote:
> > > V1 patch@https://patchwork.kernel.org/patch/11386469/ caused SC7180 system
> > > to reset at boot time.
> > The v1 patch isn't relevant in the commit message, please just describe the
> > problem. Also the crash only occurs when earlycon is used.
> ok
> > 
> > > As QUP core clock is shared among all the SE drivers present on particular
> > > QUP wrapper, the reset seen is due to earlycon usage after QUP core clock
> > > is put to 0 from other SE drivers before real console comes up.
> > > 
> > > As earlycon can't vote for it's QUP core need, to fix this add ICC
> > > support to common/QUP wrapper driver and put vote for QUP core from
> > > probe on behalf of earlycon and remove vote during sys suspend.
> > Only removing the vote on suspend isn't ideal, the system might never get
> > suspended. That said I don't have a really good alternative suggestion.
> > 
> > One thing you could possibly do is to launch a delayed work, check
> > console_device() every second or so and remove the vote when it returns
> > non-NULL. Not claiming this would be a great solution ...
> > 
> > The cleanest solution might be a notifier when the early console is
> > unregistered, it seems somewhat over-engineered though ... Then again
> > other (future) uart drivers with interconnect support might run into
> > the same problem.
> 
> We are hitting this problem because QUP core clocks are shared among all the
> SE driver present in particular QUP wrapper, if other HW controllers has
> similar architecture we will hit this issue.
> 
> How about if we expose an API from common driver(geni-se) for putting QUP
> core BW vote to 0.
> 
> We call this from console probe just after uart_add_one_port call (console
> resources are enabled as part of this call) to put core quota to 0 on behalf
> of earlyconsole?

>From my notes from earlier debugging I have doubts this would work:

  There is a short window where the early console and the 'real' console coexist:

  [    3.858122] printk: console [ttyMSM0] enabled
  [    3.875692] printk: bootconsole [qcom_geni0] disabled

  The reset probably occurs when the early console tries to write, but the ICC
  is effectively disabled because ttyMSM0 and the other geni ports are runtime
  suspended.

> > > Signed-off-by: Akash Asthana <akashast-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> > > Reported-by: Matthias Kaehlcke <mka-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> > > ---
> > >   drivers/soc/qcom/qcom-geni-se.c | 41 +++++++++++++++++++++++++++++++++++++++++
> > >   1 file changed, 41 insertions(+)
> > > 
> > > diff --git a/drivers/soc/qcom/qcom-geni-se.c b/drivers/soc/qcom/qcom-geni-se.c
> > > index 7d622ea..d244dfc 100644
> > > --- a/drivers/soc/qcom/qcom-geni-se.c
> > > +++ b/drivers/soc/qcom/qcom-geni-se.c
> > > @@ -90,6 +90,7 @@ struct geni_wrapper {
> > >   	struct device *dev;
> > >   	void __iomem *base;
> > >   	struct clk_bulk_data ahb_clks[NUM_AHB_CLKS];
> > > +	struct icc_path *icc_path_geni_to_core;
> > >   };
> > >   #define QUP_HW_VER_REG			0x4
> > > @@ -747,11 +748,50 @@ static int geni_se_probe(struct platform_device *pdev)
> > >   		}
> > >   	}
> > > +#ifdef CONFIG_SERIAL_EARLYCON
> > > +	wrapper->icc_path_geni_to_core = devm_of_icc_get(dev, "qup-core");
> > > +	if (IS_ERR(wrapper->icc_path_geni_to_core))
> > > +		return PTR_ERR(wrapper->icc_path_geni_to_core);
> > > +	/*
> > > +	 * Put minmal BW request on core clocks on behalf of early console.
> > > +	 * The vote will be removed in suspend call.
> > > +	 */
> > > +	ret = icc_set_bw(wrapper->icc_path_geni_to_core, Bps_to_icc(1000),
> > > +			Bps_to_icc(1000));
> > > +	if (ret) {
> > > +		dev_err(&pdev->dev, "%s: ICC BW voting failed for core\n",
> > > +			__func__);
> > > +		return ret;
> > > +	}
> > What is ugly about this is that it's done for every QUP, not only the one
> > with the early console. Again, I don't have a good solution for it, maybe
> > it's a limitation we have to live with :(
> 
> There is one more limitation from QUP core side. Core clocks for both the
> QUP wrapper runs at same speed.
> 
> core2x_1 = core2x_2 = max(core2x_1, core2x_2);
> 
> So with above limitation and if we are removing early con vote from Core
> when real console comes up. It doesn't matter whether it's done for every
> QUP or the only with early console.

it's still sorta ugly at an abstraction level, but it seem we have to be
pragmatic here.

> > > +#endif
> > > +
> > >   	dev_set_drvdata(dev, wrapper);
> > >   	dev_dbg(dev, "GENI SE Driver probed\n");
> > >   	return devm_of_platform_populate(dev);
> > >   }
> > > +static int __maybe_unused geni_se_sys_suspend(struct device *dev)
> > > +{
> > > +	struct geni_wrapper *wrapper = dev_get_drvdata(dev);
> > > +	int ret;
> > > +
> > > +#ifdef CONFIG_SERIAL_EARLYCON
> > > +	ret = icc_set_bw(wrapper->icc_path_geni_to_core, 0, 0);
> > I think you only want to do this on the first suspend.
> Ok, I can add that logic using global variable.
> > 
> > Do we need to handle the case where no 'real' console is configured?
> > In this case the early console would be active forever and setting
> > the bandwidths to 0 might cause a similar crash than the one you are
> > trying to fix. Not sure if that's a real world use case, but wanted to
> > mention it. Maybe this is an argument of the notifier approach?
> We can't support earlycon without real console.
> 
> As earlyconsole doesn't do any kind of resource enablement(SE clocks,
> pinctrl, etc) it assumes that resources are already enabled from previous
> stages.
> 
> So if real console doesn't come up no one will vote for that SE clock, and
> it will be disabled from clk late_init call which will result into
> un-clocked access.

Ok, IIUC what you are saying is that earlycon can't work on its own after geni
initialization. Because it clearly can work before (otherwise what would be
its purpose?), supposedly because the bootloader configures the necessary
bits.

In any case the bottom line is that earlycon requires a real console to be
configured and there is no need to handle the corner case I brought up.

  parent reply index

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-13 13:12 [PATCH V2 0/8] Add interconnect support to QSPI and QUP drivers Akash Asthana
     [not found] ` <1584105134-13583-1-git-send-email-akashast-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2020-03-13 13:12   ` [PATCH V2 1/8] interconnect: Add devm_of_icc_get() as exported API for users Akash Asthana
2020-03-13 16:26     ` Matthias Kaehlcke
2020-03-27 23:02     ` Bjorn Andersson
2020-03-13 13:12   ` [PATCH V2 4/8] tty: serial: qcom_geni_serial: Add interconnect support Akash Asthana
     [not found]     ` <1584105134-13583-5-git-send-email-akashast-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2020-03-13 21:28       ` Matthias Kaehlcke
     [not found]         ` <20200313212833.GK144492-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2020-03-17 11:48           ` Akash Asthana
2020-03-17 19:08             ` Matthias Kaehlcke
2020-03-18 12:23               ` Akash Asthana
     [not found]                 ` <e95bd6e0-d1fd-9d13-47df-f7d08b443e37-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2020-03-19 20:42                   ` Matthias Kaehlcke
     [not found]                     ` <20200319204248.GA204494-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2020-03-20 10:35                       ` Akash Asthana
2020-03-13 13:12   ` [PATCH V2 5/8] i2c: i2c-qcom-geni: " Akash Asthana
2020-03-14  0:17     ` Matthias Kaehlcke
2020-03-17 11:51       ` Akash Asthana
2020-03-13 13:12   ` [PATCH V2 6/8] spi: spi-geni-qcom: " Akash Asthana
     [not found]     ` <1584105134-13583-7-git-send-email-akashast-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2020-03-13 13:16       ` Mark Brown
2020-03-17  9:35         ` Akash Asthana
     [not found]           ` <aa197568-3bac-6962-d39d-3261f68c0514-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2020-03-17 13:06             ` Mark Brown
2020-03-20 13:52               ` Akash Asthana
2020-03-14  0:41     ` Matthias Kaehlcke
     [not found]       ` <20200314004106.GM144492-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2020-03-17 12:11         ` Akash Asthana
2020-03-13 13:12 ` [PATCH V2 2/8] soc: qcom: geni: Support for ICC voting Akash Asthana
2020-03-13 16:42   ` Matthias Kaehlcke
     [not found]     ` <20200313164207.GH144492-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2020-03-17  9:58       ` Akash Asthana
2020-03-17 19:06   ` Evan Green
     [not found]     ` <74851dda-296d-cdc5-2449-b9ec59bbc057@codeaurora.org>
2020-03-20 16:45       ` Evan Green
2020-03-27  5:33         ` Akash Asthana
2020-03-13 13:12 ` [PATCH V2 3/8] soc: qcom-geni-se: Add interconnect support to fix earlycon crash Akash Asthana
2020-03-13 20:44   ` Matthias Kaehlcke
     [not found]     ` <20200313204441.GJ144492-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2020-03-17 10:57       ` Akash Asthana
     [not found]         ` <1f86fdf0-df7c-4e4a-d4d8-8b0162e52cb4-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2020-03-17 18:29           ` Matthias Kaehlcke [this message]
2020-03-18  8:54             ` Akash Asthana
2020-03-19 19:43               ` Matthias Kaehlcke
     [not found]                 ` <20200319194332.GA60149-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2020-03-20 10:22                   ` Akash Asthana
2020-03-20 16:30                     ` Evan Green
2020-03-27  5:04                       ` Akash Asthana
2020-03-27 23:23                       ` Bjorn Andersson
2020-03-31 10:55                         ` Akash Asthana
2020-03-17 19:08           ` Evan Green
2020-03-17 19:46             ` Doug Anderson
     [not found]             ` <CAE=gft5GcOeQ5kh1bGen_P0J98g2XaAJ7NrDsxkirDoLtL4GWg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2020-03-18 10:57               ` Akash Asthana
2020-03-18 16:22                 ` Evan Green
2020-03-13 13:12 ` [PATCH V2 7/8] spi: spi-qcom-qspi: Add interconnect support Akash Asthana
2020-03-14  0:58   ` Matthias Kaehlcke
     [not found]     ` <20200314005817.GN144492-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2020-03-17 12:13       ` Akash Asthana
2020-03-17 19:08         ` Evan Green
2020-03-18 13:48           ` Akash Asthana
     [not found]             ` <e2ee1a60-a379-5c78-355a-64aad451a944-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2020-03-18 16:30               ` Evan Green
2020-03-20  5:35                 ` Akash Asthana
2020-03-13 13:12 ` [PATCH V2 8/8] arm64: dts: sc7180: Add interconnect for QUP and QSPI Akash Asthana

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 \
    --in-reply-to=20200317182910.GR144492@google.com \
    --to=mka-f7+t8e8rja9g9huczpvpmw@public.gmane.org \
    --cc=agross-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=akashast-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org \
    --cc=bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
    --cc=broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org \
    --cc=evgreen-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org \
    --cc=gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org \
    --cc=linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-spi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=mark.rutland-5wv7dgnIgG8@public.gmane.org \
    --cc=mgautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org \
    --cc=robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=swboyd-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org \
    --cc=wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org \
    /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

Linux-SPI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-spi/0 linux-spi/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-spi linux-spi/ https://lore.kernel.org/linux-spi \
		linux-spi@vger.kernel.org
	public-inbox-index linux-spi

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-spi


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git