From: "Jon Medhurst (Tixy)" <tixy@linaro.org>
To: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
Nicolas Pitre <nicolas.pitre@linaro.org>,
Samuel Ortiz <sameo@linux.intel.com>,
Pawel Moll <Pawel.Moll@arm.com>,
Sudeep KarkadaNagesha <Sudeep.KarkadaNagesha@arm.com>,
"devicetree-discuss@lists.ozlabs.org"
<devicetree-discuss@lists.ozlabs.org>,
Amit Kucheria <amit.kucheria@linaro.org>,
Achin Gupta <Achin.Gupta@arm.com>
Subject: Re: [RFC PATCH 2/3] drivers: mfd: vexpress: add timeout API to vexpress config interface
Date: Mon, 03 Jun 2013 13:03:50 +0100 [thread overview]
Message-ID: <1370261030.16073.5.camel@linaro1.home> (raw)
In-Reply-To: <20130603115219.GA22821@e102568-lin.cambridge.arm.com>
On Mon, 2013-06-03 at 12:52 +0100, Lorenzo Pieralisi wrote:
> On Mon, Jun 03, 2013 at 11:15:32AM +0100, Jon Medhurst (Tixy) wrote:
> > On Fri, 2013-05-24 at 13:53 +0100, Lorenzo Pieralisi wrote:
> > > In case some transactions to the Serial Power Controller (SPC) are lost owing
> > > to multiple operations handled at once by the M3 controller the OS needs to
> > > rely on a configuration API that can time out so that failures do not result
> > > in an unusable system.
> > >
> > > This patch adds a timeout API to the vexpress config programming interface,
> > > and refactors the existing read/write functions so that they can be reused
> > > seamlessly on top of the newly defined API.
> >
> > Isn't one of the main purposes of the config interface to serialise
> > transactions to the config bus, so why would the SPC be handling
> > multiple transactions at once? And if we can in fact loose transactions
> > doesn't this mean we get random failures in the system? E.g. if this
> > happened at boot in vexpress_spc_populate_opps then cpufreq will fail.
>
> It has more to do with firmware carrying out background operations like
> powering up a cluster when a DVFS is requested.
Would that make it drop transactions or just take a longer time to get
around to servicing them?
> I can prepare a v2 with timeout interface dropped and extensively test that
> one, I do not think we should add the required complexity that you describe
> below for something that should never happen.
>
> > Also, I think the code implementing timeouts is broken, see below.
>
> I will have a look asap and repost a v2 accordingly.
Thanks, I'll hold off any further review the current patches then.
--
Tixy
WARNING: multiple messages have this Message-ID (diff)
From: "Jon Medhurst (Tixy)" <tixy@linaro.org>
To: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Nicolas Pitre <nicolas.pitre@linaro.org>,
Samuel Ortiz <sameo@linux.intel.com>,
Pawel Moll <Pawel.Moll@arm.com>,
Sudeep KarkadaNagesha <Sudeep.KarkadaNagesha@arm.com>,
"devicetree-discuss@lists.ozlabs.org"
<devicetree-discuss@lists.ozlabs.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Amit Kucheria <amit.kucheria@linaro.org>,
Achin Gupta <Achin.Gupta@arm.com>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>
Subject: Re: [RFC PATCH 2/3] drivers: mfd: vexpress: add timeout API to vexpress config interface
Date: Mon, 03 Jun 2013 13:03:50 +0100 [thread overview]
Message-ID: <1370261030.16073.5.camel@linaro1.home> (raw)
In-Reply-To: <20130603115219.GA22821@e102568-lin.cambridge.arm.com>
On Mon, 2013-06-03 at 12:52 +0100, Lorenzo Pieralisi wrote:
> On Mon, Jun 03, 2013 at 11:15:32AM +0100, Jon Medhurst (Tixy) wrote:
> > On Fri, 2013-05-24 at 13:53 +0100, Lorenzo Pieralisi wrote:
> > > In case some transactions to the Serial Power Controller (SPC) are lost owing
> > > to multiple operations handled at once by the M3 controller the OS needs to
> > > rely on a configuration API that can time out so that failures do not result
> > > in an unusable system.
> > >
> > > This patch adds a timeout API to the vexpress config programming interface,
> > > and refactors the existing read/write functions so that they can be reused
> > > seamlessly on top of the newly defined API.
> >
> > Isn't one of the main purposes of the config interface to serialise
> > transactions to the config bus, so why would the SPC be handling
> > multiple transactions at once? And if we can in fact loose transactions
> > doesn't this mean we get random failures in the system? E.g. if this
> > happened at boot in vexpress_spc_populate_opps then cpufreq will fail.
>
> It has more to do with firmware carrying out background operations like
> powering up a cluster when a DVFS is requested.
Would that make it drop transactions or just take a longer time to get
around to servicing them?
> I can prepare a v2 with timeout interface dropped and extensively test that
> one, I do not think we should add the required complexity that you describe
> below for something that should never happen.
>
> > Also, I think the code implementing timeouts is broken, see below.
>
> I will have a look asap and repost a v2 accordingly.
Thanks, I'll hold off any further review the current patches then.
--
Tixy
WARNING: multiple messages have this Message-ID (diff)
From: tixy@linaro.org (Jon Medhurst (Tixy))
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC PATCH 2/3] drivers: mfd: vexpress: add timeout API to vexpress config interface
Date: Mon, 03 Jun 2013 13:03:50 +0100 [thread overview]
Message-ID: <1370261030.16073.5.camel@linaro1.home> (raw)
In-Reply-To: <20130603115219.GA22821@e102568-lin.cambridge.arm.com>
On Mon, 2013-06-03 at 12:52 +0100, Lorenzo Pieralisi wrote:
> On Mon, Jun 03, 2013 at 11:15:32AM +0100, Jon Medhurst (Tixy) wrote:
> > On Fri, 2013-05-24 at 13:53 +0100, Lorenzo Pieralisi wrote:
> > > In case some transactions to the Serial Power Controller (SPC) are lost owing
> > > to multiple operations handled at once by the M3 controller the OS needs to
> > > rely on a configuration API that can time out so that failures do not result
> > > in an unusable system.
> > >
> > > This patch adds a timeout API to the vexpress config programming interface,
> > > and refactors the existing read/write functions so that they can be reused
> > > seamlessly on top of the newly defined API.
> >
> > Isn't one of the main purposes of the config interface to serialise
> > transactions to the config bus, so why would the SPC be handling
> > multiple transactions at once? And if we can in fact loose transactions
> > doesn't this mean we get random failures in the system? E.g. if this
> > happened at boot in vexpress_spc_populate_opps then cpufreq will fail.
>
> It has more to do with firmware carrying out background operations like
> powering up a cluster when a DVFS is requested.
Would that make it drop transactions or just take a longer time to get
around to servicing them?
> I can prepare a v2 with timeout interface dropped and extensively test that
> one, I do not think we should add the required complexity that you describe
> below for something that should never happen.
>
> > Also, I think the code implementing timeouts is broken, see below.
>
> I will have a look asap and repost a v2 accordingly.
Thanks, I'll hold off any further review the current patches then.
--
Tixy
next prev parent reply other threads:[~2013-06-03 12:05 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-05-24 12:53 [RFC PATCH 0/3] drivers: mfd: Versatile Express SPC support Lorenzo Pieralisi
2013-05-24 12:53 ` Lorenzo Pieralisi
2013-05-24 12:53 ` Lorenzo Pieralisi
2013-05-24 12:53 ` [RFC PATCH 1/3] drivers: mfd: refactor the vexpress config bridge API Lorenzo Pieralisi
2013-05-24 12:53 ` Lorenzo Pieralisi
2013-05-24 12:53 ` Lorenzo Pieralisi
2013-05-24 12:53 ` [RFC PATCH 2/3] drivers: mfd: vexpress: add timeout API to vexpress config interface Lorenzo Pieralisi
2013-05-24 12:53 ` Lorenzo Pieralisi
2013-06-03 10:15 ` Jon Medhurst (Tixy)
2013-06-03 10:15 ` Jon Medhurst (Tixy)
2013-06-03 10:15 ` Jon Medhurst (Tixy)
2013-06-03 11:52 ` Lorenzo Pieralisi
2013-06-03 11:52 ` Lorenzo Pieralisi
2013-06-03 11:52 ` Lorenzo Pieralisi
2013-06-03 12:03 ` Jon Medhurst (Tixy) [this message]
2013-06-03 12:03 ` Jon Medhurst (Tixy)
2013-06-03 12:03 ` Jon Medhurst (Tixy)
2013-06-03 13:15 ` Lorenzo Pieralisi
2013-06-03 13:15 ` Lorenzo Pieralisi
2013-06-03 13:15 ` Lorenzo Pieralisi
2013-05-24 12:53 ` [RFC PATCH 3/3] drivers: mfd: vexpress: add Serial Power Controller (SPC) support Lorenzo Pieralisi
2013-05-24 12:53 ` Lorenzo Pieralisi
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=1370261030.16073.5.camel@linaro1.home \
--to=tixy@linaro.org \
--cc=Achin.Gupta@arm.com \
--cc=Pawel.Moll@arm.com \
--cc=Sudeep.KarkadaNagesha@arm.com \
--cc=amit.kucheria@linaro.org \
--cc=devicetree-discuss@lists.ozlabs.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lorenzo.pieralisi@arm.com \
--cc=nicolas.pitre@linaro.org \
--cc=sameo@linux.intel.com \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.