Received-SPF: pass (google.com: domain of linux-kernel+bounces-86549-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Precedence: bulk
MIME-Version: 1.0
References: <20240223143833.1509961-1-guanyulin@google.com>
 <a299118d-eeec-40b4-9a3d-48dc40f34e12@gmail.com> <CAOuDEK3wP6zhEwgUn5zSedtwTYVFaJeBfeXkSg897EhpGP9=ig@mail.gmail.com>
 <3208c5b9-5286-48d1-81ab-cc3b2bc4303e@gmail.com> <CAOuDEK39Bdru5wAbxW-g2c=POgRxZwdQzPO5uNXP96AfSyA6pw@mail.gmail.com>
 <2024022749-campus-multiply-d657@gregkh>
In-Reply-To: <2024022749-campus-multiply-d657@gregkh>
From: Guan-Yu Lin <guanyulin@google.com>
Date: Thu, 29 Feb 2024 18:27:00 +0800
Message-ID: <CAOuDEK3e0y_5HB3VkZoyYY8kzoWAJ3w9y7RcLJy7tSatsc+A6w@mail.gmail.com>
Subject: Re: [PATCH v3] PM / core: conditionally skip system pm in
 device/driver model
To: Greg KH <gregkh@linuxfoundation.org>
Cc: Florian Fainelli <f.fainelli@gmail.com>, rafael@kernel.org, pavel@ucw.cz, 
	len.brown@intel.com, andriy.shevchenko@linux.intel.com, rdunlap@infradead.org, 
	james@equiv.tech, broonie@kernel.org, james.clark@arm.com, 
	masahiroy@kernel.org, linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Tue, Feb 27, 2024 at 5:15=E2=80=AFPM Greg KH <gregkh@linuxfoundation.org=
> wrote:
>
> On Tue, Feb 27, 2024 at 04:56:00PM +0800, Guan-Yu Lin wrote:
> > On Tue, Feb 27, 2024 at 2:40=E2=80=AFAM Florian Fainelli <f.fainelli@gm=
ail.com> wrote:
> > >
> > > On 2/26/24 02:28, Guan-Yu Lin wrote:
> > > > On Sat, Feb 24, 2024 at 2:20=E2=80=AFAM Florian Fainelli <f.fainell=
i@gmail.com> wrote:
> > > >>
> > > >> On 2/23/24 06:38, Guan-Yu Lin wrote:
> > > >>> In systems with a main processor and a co-processor, asynchronous
> > > >>> controller management can lead to conflicts.  One example is the =
main
> > > >>> processor attempting to suspend a device while the co-processor i=
s
> > > >>> actively using it. To address this, we introduce a new sysfs entr=
y
> > > >>> called "conditional_skip". This entry allows the system to select=
ively
> > > >>> skip certain device power management state transitions. To use th=
is
> > > >>> feature, set the value in "conditional_skip" to indicate the type=
 of
> > > >>> state transition you want to avoid.  Please review /Documentation=
/ABI/
> > > >>> testing/sysfs-devices-power for more detailed information.
> > > >>
> > > >> This looks like a poor way of dealing with a lack of adequate reso=
urce
> > > >> tracking from Linux on behalf of the co-processor(s) and I really =
do not
> > > >> understand how someone is supposed to use that in a way that works=
.
> > > >>
> > > >> Cannot you use a HW maintained spinlock between your host processo=
r and
> > > >> the co-processor such that they can each claim exclusive access to=
 the
> > > >> hardware and you can busy-wait until one or the other is done usin=
g the
> > > >> device? How is your partitioning between host processor owned bloc=
ks and
> > > >> co-processor(s) owned blocks? Is it static or is it dynamic?
> > > >> --
> > > >> Florian
> > > >>
> > > >
> > > > This patch enables devices to selectively participate in system pow=
er
> > > > transitions. This is crucial when multiple processors, managed by
> > > > different operating system kernels, share the same controller. One
> > > > processor shouldn't enforce the same power transition procedures on
> > > > the controller =E2=80=93 another processor might be using it at tha=
t moment.
> > > > While a spinlock is necessary for synchronizing controller access, =
we
> > > > still need to add the flexibility to dynamically customize power
> > > > transition behavior for each device. And that's what this patch is
> > > > trying to do.
> > > > In our use case, the host processor and co-processor are managed by
> > > > separate operating system kernels. This arrangement is static.
> > >
> > > OK, so now the question is whether the peripheral is entirely visible=
 to
> > > Linux, or is it entirely owned by the co-processor, or is there a
> > > combination of both and the usage of the said device driver is dynami=
c
> > > between Linux and your co-processor?
> > >
> > > A sysfs entry does not seem like the appropriate way to described whi=
ch
> > > states need to be skipped and which ones can remain under control of
> > > Linux, you would have to use your firmware's description for that (AC=
PI,
> > > Device Tree, etc.) such that you have a more comprehensive solution t=
hat
> > > can span a bigger scope.
> > > --
> > > Florian
> > >
> >
> > We anticipate that control of the peripheral (e.g., controller) will
> > be shared between operating system kernels. Each kernel will need its
> > own driver for peripheral communication. To accommodate different
> > tasks, the operating system managing the peripheral can change
> > dynamically at runtime.
>
> That sounds like a nightmare of control and handling, how are you going
> to do any of that?  Where is the code for that?
>

Since the peripheral can issue different types of interrupts, we plan
to assign the handling of those interrupts to separate operating
system kernels. Additionally, only one operating system kernel will
have the privilege to issue commands to the peripheral. We think that
this could resolve potential conflicts.

> > We dynamically select the operating system kernel controlling the
> > target peripheral based on the task at hand, which looks more like a
> > software behavior rather than hardware behavior to me. I agree that we
> > might need a firmware description for "whether another operating
> > system exists for this peripheral", but we also need to store the
> > information about "whether another operating system is actively using
> > this peripheral". To me, the latter one looks more like a sysfs entry
> > rather than a firmware description as it's not determined statically.
>
> So you want to download different firmware to the device depending on
> "something".  What is going to control that "something"?  Is that coming
> from the kernel, or from userspace?  If userspace, why is any of this an
> issue and just load whatever firmware you decide at that point in time?
> Why does the kernel care?
>
> confused,
>
> greg k-h

In our design, no single firmware can fully control or communicate
with the peripheral. Different functions of the peripheral are
supported by different operating system kernels. Therefore, we should
keep both firmwares active to use the peripheral effectively.