Re: [PATCH v2] platform/x86: intel-hid: Add support for Device Specific Methods

[Srinivas Pandruvada]

On Fri, 2018-07-06 at 16:59 -0700, Darren Hart wrote:
> On Mon, Jul 02, 2018 at 05:06:14PM +0300, Andy Shevchenko wrote:
> > On Mon, Jul 2, 2018 at 4:51 PM,  <Mario.Limonciello@xxxxxxxx>
> > wrote:
> > 
> > > > > So there are some customers who will have issue with power
> > > > > button
> > > > > without this patch, so it should be also marked for stable
> > > > > also.
> > > > > Also this may be a candidate for 4.18-rcX.
> > > > > 
> > > > 
> > > > I'm not sure Greg will take this selling point for rather big
> > > > patch.
> > > > From changelog, honestly, I don't see any regression
> > > > description,
> > > > looks like "it wasn't working before change anyway".
> > > > 
> > > 
> > > Just for adding some context.
> > > 
> > > Some platforms have moved to different interface in ASL in FW
> > > upgrade
> > > due to deficiencies/bugs present with old interface.  So yes it's
> > > platform FW
> > > change in behavior that leads to Linux kernel
> > > regression.  Windows driver has supported
> > > both interfaces for a long time.  Linux kernel however doesn't
> > > support this interface until now.
> > > 
> > > > For now, I pushed this to my review and testing queue as is,
> > > > thanks!
> > > 
> > > If not stable I think it would at least be ideal to try to bring
> > > this to 4.18-rcX if possible for
> > > compatibility with more platforms that will come with this other
> > > interface instead.
> > 
> > Citing Linus:
> > 
> > --- 8< --- 8< ---
> > So please, people, the "fixes" during the rc series really should
> > be
> > things that are _regressions_. If it used to work, and it no longer
> > does,
> > then fixing that is a good and proper fix. Or if something oopses
> > or has a
> > security implication, then the fix for that is a real fix.
> > 
> > But if it's something that has never worked, even if it "fixes"
> > some
> > behavior, then it's new development, and that should come in during
> > the
> > merge window. Just because you think it's a "fix" doesn't mean that
> > it
> > really is one, at least in the "during the rc series" sense.
> > --- 8< --- 8< ---
> > 
> > So, if we can sell him that it used to work and firmware fix is a
> > Linux regression I'm fine.
> > 
> > Darren, what do you think?
> 
> So if I understand this correctly, we have this timeline.
> 
> Linux v4.16
> - Machine A works
> Linux v4.17
> - Machine A works
> - Machine A updates firmware
> - Machine A stops working
> Linux v4.18
> - Machine A still doesn't work
> 
> So it is not a *Linux kernel* regression.
> 
> The patch is too large for standard stable rules.
> 
> It is a regression from any user's perspective - the machine worked,
> they followed good digital hygiene and updated their firmware, and
> now
> it doesn't. This user will now think twice before they update their
> BIOS
> again, since it may fundamentally change the platform, rather than
> committing to only fixing things below the OS Interface layer. :-(
> 
> The risk of course is that this introduces new bugs - and as with
> anything that still uses _DSM (sigh, why?) that is quite possible due
> to
> the opaque interface. Very unfortunate to see _DSM ADDED to a
> previously
> _DSM free implementation. Linus is right, this is not a fix, this is
> feature development.
> 
> I strongly advocate for vendors to have more control over their
> drivers,
> but this scenario really frustrates me. I don't think I can justify
> this
> to Linus as a fix. But before we just say "no" (because hey, I want
> these fixes available as early as possible too), let's ask Rafael if
> he
> has an opinion or if there is precedent for this in his experience
> with
> ACPI drivers in general:
> 
> Rafael?
> 
> Finally, we can also just ask Linus. The firmware update broke the
> power
> button, we can get it working again by supporting the new API with
> this
> patch... see what he says.
Mario can add more.
But I think Dell has released a BIOS fix, so that power button can
still work using non _DSM way. So I think we can wait for normal
release cycle.

Thanks,
Srinivas

>