Re: [PATCH 4/6] ACPI: Execute platform _OSC also with query bit clear

From: "Rafael J. Wysocki" <rafael@kernel.org>
To: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: "Limonciello, Mario" <Mario.Limonciello@dell.com>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	"open list:ULTRA-WIDEBAND (UWB) SUBSYSTEM:" 
	<linux-usb@vger.kernel.org>,
	Michael Jamet <michael.jamet@intel.com>,
	Yehezkel Bernat <YehezkelShB@gmail.com>,
	Andreas Noever <andreas.noever@gmail.com>,
	Lukas Wunner <lukas@wunner.de>,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>,
	Christian Kellner <christian@kellner.me>,
	Len Brown <lenb@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	ACPI Devel Maling List <linux-acpi@vger.kernel.org>
Subject: Re: [PATCH 4/6] ACPI: Execute platform _OSC also with query bit clear
Date: Wed, 27 Jan 2021 14:50:00 +0100	[thread overview]
Message-ID: <CAJZ5v0gQv2iKbyWLgEActwv1_ea0cMkT5dwvKBGqRtZE3Wo6NQ@mail.gmail.com> (raw)
In-Reply-To: <20210127124935.GC2542@lahna.fi.intel.com>

On Wed, Jan 27, 2021 at 1:49 PM Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
>
> On Tue, Jan 26, 2021 at 10:43:32PM +0000, Limonciello, Mario wrote:
> > > I would put that information into the changelog.
> >
> > Thanks, @Mika Westerberg can you collapse that in when you re-spin the
> > series?
>
> Sure.
>
> > >
> > > Moreover, have you looked at acpi_pci_osc_control_set()?
> > >
> > > What it does is analogous to what you are proposing, but a bit
> > > different, and I would like to preserve consistency between _OSC use
> > > cases.
> > >
> > > So would it be possible to adjust the _SB _OSC evaluation flow to
> > > follow the PCI _OSC one?  That is, if any control bits are there, pass
> > > them along with the last evaluation of _OSC with the query flag clear.
> > > Or is the latter defective and if so then why?
> >
> > Basically the only difference is another line cloning OSC_CONTROL_DWORD from
> > capbuf_ret to capbuf?
> >
> > Yes, this actually sounds like it better adheres to the spec to me.
> >
> > Quoting spec:
> > " If the OS is granted control of a feature in the Control Field in one call to
> > _OSC, then it must preserve the set state of that bit (requesting that feature)
> > in all subsequent calls."
>
> However, the platform wide _OSC does not actually have this
> OSC_CONTROL_DWORD at all ;-)

Right.

> I think what we do in this patch is already equivalent to what the PCI
> _OSC is doing:
>
>   1. Query bit set _OSC
>   2. Take the returned OSC_SUPPORT_DWORD buffer and
>   3. Pass it to the _OSC with query bit clear.

Yes, it is.

Given the way the USB4 _OSC protocol is defined (which admittedly
confused me somewhat), the code changes in this patch are fine by me.

Thanks and sorry for the confusion.