> -----Original Message----- > From: Darren Hart [mailto:dvhart@xxxxxxxxxxxxx] > Sent: Wednesday, September 6, 2017 3:10 PM > To: Limonciello, Mario <Mario_Limonciello@xxxxxxxx> > Cc: yehezkel.bernat@xxxxxxxxx; mika.westerberg@xxxxxxxxxxxxxxx; linux- > kernel@xxxxxxxxxxxxxxx; platform-driver-x86@xxxxxxxxxxxxxxx; > hughsient@xxxxxxxxx > Subject: Re: Fwd: [PATCH] Add driver to force WMI Thunderbolt controller power > status > > On Wed, Sep 06, 2017 at 07:49:32PM +0000, Mario.Limonciello@xxxxxxxx wrote: > > > -----Original Message----- > > > From: Bernat, Yehezkel [mailto:yehezkel.bernat@xxxxxxxxx] > > > Sent: Wednesday, September 6, 2017 2:41 PM > > > To: Limonciello, Mario <Mario_Limonciello@xxxxxxxx> > > > Cc: mika.westerberg@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; platform- > > > driver-x86@xxxxxxx; dvhart@xxxxxxxxxxxxx; hughsient@xxxxxxxxx > > > Subject: Re: Fwd: [PATCH] Add driver to force WMI Thunderbolt controller > power > > > status > > > > > > > > > > Current implementations of Intel Thunderbolt controllers will go > > > > into a low power mode when not in use. > > > > > > > > Many machines containing these controllers also have a GPIO wired up > > > > that can force the controller awake. This is offered via a ACPI-WMI > > > > interface intended to be manipulated by a userspace utility. > > > > > > > > > > This mechanism is provided by Intel to OEMs to include in BIOS. > > > > It uses an industry wide GUID that is populated in a separate _WDG > > > > entry with no binary MOF. > > > > > > > > This interface allow software such as fwupd to wake up thunderbolt > > > > controllers to query the firmware version or flash new firmware. > > > > > > As this is a Thunderbolt specific function, maybe it's better to be > > > exposed from the Thunderbolt driver? > > > > > > > I thought about this too, but the thunderbolt driver won't load if the > > controller doesn't exist in the first place, whereas this is a platform > > BIOS feature. I'll be interested to hear if Mika has a different perspective > > on if this should live in the TBT driver and the proper way to do it. > > > > The other question I had about this was if the typical use case involves the OS, > or if the firmware update (for example) would be performed as part of the > general platform firmware update (from the UEFI update utility). > > > > > > > > + > > > > +static DEVICE_ATTR_WO(force_power); > > > > + > > > > > > I'm not sure what is the convention for permissions for this type of > > > attributes but I feel like this worth being root-only writable, as it > > > can be used to power-off the controller in the middle of a FW update, > > > for example. > > > > Yeah I think you're right. I'll adjust it in a follow up patch if this is the > > correct way to go afterall. > > > Ahhhrg, that was something I meant to follow up on as I was discussing using the > macros with Mario previously, and I forgot. Sorry about that Mario. > I double checked and with the way it's done right now, permissions are: --w------- Looks like the macro DTRT without me needing to override.