Re: [PATCH v3] ACPI: platform-profile: Add platform profile support

From: Hans de Goede <hdegoede@redhat.com>
To: "Barnabás Pőcze" <pobrn@protonmail.com>
Cc: Mark Pearson <markpearson@lenovo.com>,
	"rjw@rjwysocki.net" <rjw@rjwysocki.net>,
	"mgross@linux.intel.com" <mgross@linux.intel.com>,
	"linux-acpi@vger.kernel.org" <linux-acpi@vger.kernel.org>,
	"mario.limonciello@dell.com" <mario.limonciello@dell.com>,
	"eliadevito@gmail.com" <eliadevito@gmail.com>,
	"hadess@hadess.net" <hadess@hadess.net>,
	"bberg@redhat.com" <bberg@redhat.com>,
	"platform-driver-x86@vger.kernel.org" 
	<platform-driver-x86@vger.kernel.org>,
	"dvhart@infradead.org" <dvhart@infradead.org>
Subject: Re: [PATCH v3] ACPI: platform-profile: Add platform profile support
Date: Sun, 22 Nov 2020 10:32:10 +0100	[thread overview]
Message-ID: <41f3e41a-07be-3276-40fe-5675a19f36ae@redhat.com> (raw)
In-Reply-To: <8c3pjwAeJVjl5ZLmnajCVHjcyBjoQeQTupHqZZostKJt3YZ0seAAvD-UIcBYFAFUNjsi0iWrEBNfX1l0tqN-4x07TGQbsiOMEqCpPAxgqJQ=@protonmail.com>

Hi,

On 11/21/20 10:18 PM, Barnabás Pőcze wrote:
> 2020. november 21., szombat 15:27 keltezéssel, Hans de Goede írta:
> 
>> [...]
>>> I just realized that the sysfs attributes are only created if a profile provider
>>> is registered, and it is removed when the provide unregisters itself. I believe
>>> it would be easier for system daemons if those attributes existed from module load
>>> to module unload since they can just just open the file and watch it using poll,
>>> select, etc. If it goes away when the provider unregisters itself, then I believe
>>> a more complicated mechanism (like inotify) would need to be implemented in the
>>> daemons to be notified when a new provider is registered. Thus my suggestion
>>> for the next iteration is to create the sysfs attributes on module load,
>>> and delete them on unload.
>>>
>>> What do you think?
>>
>> Actually I asked Mark to move this to the register / unregister time since
>> having a non functioning files in sysfs is a bit weird.
>> [...]
> 
> Ahh, I didn't know that, sorry. If a non-functioning sysfs attribute is a problem,
> then there is another option: `platform_profile_choices` is always present;
> if no provider is registered, it's empty. If a provider is (un)registered,
> then `platform_profile_choices` is sysfs_notify()-ed. `platform_profile`
> only exists while a provider is registered, so it is created on provider
> registration and unregistered on provider unregistration.

TBH, I don't like this suggestion. I would like to either want both of
them be present and report "none" (and -ENODEV on write in case of platform_profile),
or neither of them be present.

Note I do agree with you that userspace probably needs a way to find out when a
provider shows up. Which means we should probably go with always having both of
them present. But it would also be good to get some input from Bastien here,
as he is working on a userspace daemon using this API.

Regards,

Hans