On Wednesday, February 7, 2018 10:31:01 AM CET Rafael J. Wysocki wrote: > On Tuesday, February 6, 2018 11:37:54 PM CET Andy Shevchenko wrote: > > Yeah, that's fixed in the branch. > > > > P.S. Apologies for top-posting, wrote from phone in order to not waste time > > > > On Tuesday, February 6, 2018, <Mario.Limonciello@xxxxxxxx> wrote: > > > > > > -----Original Message----- > > > > From: platform-driver-x86-owner@xxxxxxxxxxxxxxx [mailto: > > > platform-driver-x86- > > > > owner@xxxxxxxxxxxxxxx] On Behalf Of Andy Shevchenko > > > > Sent: Monday, February 5, 2018 10:18 AM > > > > To: Darren Hart <dvhart@xxxxxxxxxxxxx> > > > > Cc: platform-driver-x86@xxxxxxxxxxxxxxx; AceLan Kao > > > > <acelan.kao@xxxxxxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx; > > > > pali.rohar@xxxxxxxxx; Limonciello, Mario <Mario_Limonciello@xxxxxxxx>; > > > Rafael > > > > Wysocki <rjw@xxxxxxxxxxxxx> > > > > Subject: Re: [RFC, PATCH v1] platform/x86: intel-vbtn: Convert to pure > > > ACPI driver > > > > > > > > On Wed, 2018-01-31 at 13:37 -0800, Darren Hart wrote: > > > > > On Wed, Jan 31, 2018 at 08:26:14PM +0200, Andy Shevchenko wrote: > > > > > > Remove code duplication by converting driver to pure ACPI one. > > > > > > > > > > > > > I have fixed couple issues, but... > > > > > > > > > +Rafael > > > > > > > > > > I don't see any reason *not* to do this, it seems the acpi driver > > > > > model provides > > > > > some of the boilerplate stuff we were doing manually before as a > > > > > platform > > > > > device. I'm scratching my head wondering why we did this as a > > > > > platform-driver in > > > > > the first place now... > > It looks like I have overlooked this. > > Generally speaking, binding drivers to ACPI device objects is a bad idea, > because it is sort of what binding drivers to struct device_node in OF would > be. It was an unfortunate design choice for struct acpi_device to be based > on struct device in the first place, but undoing it is not worth the effort > and pain. > > In some instances (PCI, PNP, I2C, SPI etc) there is a device object to bind a > driver to already and the corresponding struct acpi_device is the "companion" > of it. In those cases binding a driver to the struct acpi_device itself > clearly doesn't make sense. > > In the cases like the $subject one it is less clear, but for symmetry and > general confusion avoidance it is better to have a struct device representing > the "physical" thing *in* *addition* to the struct acpi_device and use the > latter as the "companion" of it. That's why we have the platform device in > this particular case. > > Please drop this patch, it doesn't go in the right direction IMO. BTW, please CC patches with ACPI material to linux-acpi. They are less likely to be overlooked by me then and the others on that list may have opinions on them too.
