[PATCH 0/3] platform/x86: dell-pc: Transition to faux device

[PATCH 0/3] platform/x86: dell-pc: Transition to faux device

[Kurt Borja]

Hi all,

In commit 

	35fa2d88ca94 ("driver core: add a faux bus for use when a simple device/bus is needed")

the new faux bus was introduced. It was designed for cases where a
module needs a fake parent device.

When I saw this, dell-pc immediately came to mind so I wrote this small
patchset.

Compile tested only.

Signed-off-by: Kurt Borja <kuurtb@gmail.com>
---
Kurt Borja (3):
      platform/x86: dell-pc: Propagate errors when detecting feature support
      platform/x86: dell-pc: Use non-atomic bitmap operations
      platform/x86: dell-pc: Transition to faux device

 drivers/platform/x86/dell/dell-pc.c | 67 +++++++++++--------------------------
 1 file changed, 19 insertions(+), 48 deletions(-)
---
base-commit: 70081121e24cacbef8b3be849cc13bea31f8a158
change-id: 20250327-dell-faux-52e24510a574

Best regards,
-- 
 ~ Kurt

[PATCH 1/3] platform/x86: dell-pc: Propagate errors when detecting feature support

[Kurt Borja]

The dell-pc module only supports the thermal management Dell SMBIOS
feature, therefore it is pointless to have it loaded if this is not
available.

Signed-off-by: Kurt Borja <kuurtb@gmail.com>
---
 drivers/platform/x86/dell/dell-pc.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/drivers/platform/x86/dell/dell-pc.c b/drivers/platform/x86/dell/dell-pc.c
index 483240bb36e77d2118dfcbd6bf271e12e82e042f..38f198a7330006333b01787a9934b8eb146ce75e 100644
--- a/drivers/platform/x86/dell/dell-pc.c
+++ b/drivers/platform/x86/dell/dell-pc.c
@@ -146,11 +146,6 @@ static int thermal_get_supported_modes(int *supported_bits)
 
 	dell_fill_request(&buffer, 0x0, 0, 0, 0);
 	ret = dell_send_request(&buffer, CLASS_INFO, SELECT_THERMAL_MANAGEMENT);
-	/* Thermal function not supported */
-	if (ret == -ENXIO) {
-		*supported_bits = 0;
-		return 0;
-	}
 	if (ret)
 		return ret;
 	*supported_bits = FIELD_GET(DELL_THERMAL_SUPPORTED, buffer.output[1]);
@@ -255,16 +250,12 @@ static int thermal_init(void)
 	struct device *ppdev;
 	int ret;
 
-	/* If thermal commands are not supported, exit without error */
 	if (!dell_smbios_class_is_supported(CLASS_INFO))
-		return 0;
+		return -ENODEV;
 
-	/* If thermal modes are not supported, exit without error */
 	ret = thermal_get_supported_modes(&supported_modes);
 	if (ret < 0)
 		return ret;
-	if (!supported_modes)
-		return 0;
 
 	platform_device = platform_device_register_simple("dell-pc", PLATFORM_DEVID_NONE, NULL, 0);
 	if (IS_ERR(platform_device))
@@ -297,7 +288,6 @@ static int __init dell_init(void)
 	if (!dmi_check_system(dell_device_table))
 		return -ENODEV;
 
-	/* Do not fail module if thermal modes not supported, just skip */
 	ret = thermal_init();
 	if (ret)
 		goto fail_thermal;

-- 
2.49.0

[PATCH 2/3] platform/x86: dell-pc: Use non-atomic bitmap operations

[Kurt Borja]

The choices bitmap belongs only to this thread, therefore we can use the
non-atomic version of set_bit().

Signed-off-by: Kurt Borja <kuurtb@gmail.com>
---
 drivers/platform/x86/dell/dell-pc.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/x86/dell/dell-pc.c b/drivers/platform/x86/dell/dell-pc.c
index 38f198a7330006333b01787a9934b8eb146ce75e..794924913be0c6f13ed4aed8b01ffd21f1d34dea 100644
--- a/drivers/platform/x86/dell/dell-pc.c
+++ b/drivers/platform/x86/dell/dell-pc.c
@@ -11,6 +11,7 @@
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #include <linux/bitfield.h>
+#include <linux/bitops.h>
 #include <linux/bits.h>
 #include <linux/dmi.h>
 #include <linux/err.h>
@@ -228,13 +229,13 @@ static int thermal_platform_profile_get(struct device *dev,
 static int thermal_platform_profile_probe(void *drvdata, unsigned long *choices)
 {
 	if (supported_modes & DELL_QUIET)
-		set_bit(PLATFORM_PROFILE_QUIET, choices);
+		__set_bit(PLATFORM_PROFILE_QUIET, choices);
 	if (supported_modes & DELL_COOL_BOTTOM)
-		set_bit(PLATFORM_PROFILE_COOL, choices);
+		__set_bit(PLATFORM_PROFILE_COOL, choices);
 	if (supported_modes & DELL_BALANCED)
-		set_bit(PLATFORM_PROFILE_BALANCED, choices);
+		__set_bit(PLATFORM_PROFILE_BALANCED, choices);
 	if (supported_modes & DELL_PERFORMANCE)
-		set_bit(PLATFORM_PROFILE_PERFORMANCE, choices);
+		__set_bit(PLATFORM_PROFILE_PERFORMANCE, choices);
 
 	return 0;
 }

-- 
2.49.0

[PATCH 3/3] platform/x86: dell-pc: Transition to faux device

[Kurt Borja]

Use a faux device parent for registering the platform_profile instead of
a "fake" platform device.

The faux bus is a minimalistic, single driver bus designed for this
purpose.

Signed-off-by: Kurt Borja <kuurtb@gmail.com>
---
 drivers/platform/x86/dell/dell-pc.c | 46 +++++++++++--------------------------
 1 file changed, 13 insertions(+), 33 deletions(-)

diff --git a/drivers/platform/x86/dell/dell-pc.c b/drivers/platform/x86/dell/dell-pc.c
index 794924913be0c6f13ed4aed8b01ffd21f1d34dea..48cc7511905a62d2828e3a7b593b3d2dae893e34 100644
--- a/drivers/platform/x86/dell/dell-pc.c
+++ b/drivers/platform/x86/dell/dell-pc.c
@@ -13,18 +13,18 @@
 #include <linux/bitfield.h>
 #include <linux/bitops.h>
 #include <linux/bits.h>
+#include <linux/device/faux.h>
 #include <linux/dmi.h>
 #include <linux/err.h>
 #include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/platform_profile.h>
-#include <linux/platform_device.h>
 #include <linux/slab.h>
 
 #include "dell-smbios.h"
 
-static struct platform_device *platform_device;
+static struct faux_device *dell_pc_fdev;
 static int supported_modes;
 
 static const struct dmi_system_id dell_device_table[] __initconst = {
@@ -246,7 +246,7 @@ static const struct platform_profile_ops dell_pc_platform_profile_ops = {
 	.profile_set = thermal_platform_profile_set,
 };
 
-static int thermal_init(void)
+static int dell_pc_faux_probe(struct faux_device *fdev)
 {
 	struct device *ppdev;
 	int ret;
@@ -258,51 +258,31 @@ static int thermal_init(void)
 	if (ret < 0)
 		return ret;
 
-	platform_device = platform_device_register_simple("dell-pc", PLATFORM_DEVID_NONE, NULL, 0);
-	if (IS_ERR(platform_device))
-		return PTR_ERR(platform_device);
+	ppdev = devm_platform_profile_register(&fdev->dev, "dell-pc", NULL,
+					       &dell_pc_platform_profile_ops);
 
-	ppdev = devm_platform_profile_register(&platform_device->dev, "dell-pc",
-					       NULL, &dell_pc_platform_profile_ops);
-	if (IS_ERR(ppdev)) {
-		ret = PTR_ERR(ppdev);
-		goto cleanup_platform_device;
-	}
-
-	return 0;
-
-cleanup_platform_device:
-	platform_device_unregister(platform_device);
-
-	return ret;
+	return PTR_ERR_OR_ZERO(ppdev);
 }
 
-static void thermal_cleanup(void)
-{
-	platform_device_unregister(platform_device);
-}
+static const struct faux_device_ops dell_pc_faux_ops = {
+	.probe = dell_pc_faux_probe,
+};
 
 static int __init dell_init(void)
 {
-	int ret;
-
 	if (!dmi_check_system(dell_device_table))
 		return -ENODEV;
 
-	ret = thermal_init();
-	if (ret)
-		goto fail_thermal;
+	dell_pc_fdev = faux_device_create("dell-pc", NULL, &dell_pc_faux_ops);
+	if (!dell_pc_fdev)
+		return -ENODEV;
 
 	return 0;
-
-fail_thermal:
-	thermal_cleanup();
-	return ret;
 }
 
 static void __exit dell_exit(void)
 {
-	thermal_cleanup();
+	faux_device_destroy(dell_pc_fdev);
 }
 
 module_init(dell_init);

-- 
2.49.0

Re: [PATCH 3/3] platform/x86: dell-pc: Transition to faux device

[Ilpo Järvinen]

On Fri, 11 Apr 2025, Kurt Borja wrote:

> Use a faux device parent for registering the platform_profile instead of
> a "fake" platform device.
> 
> The faux bus is a minimalistic, single driver bus designed for this
> purpose.

Hi Kurt, Hans & Greg,

I'm not sure about this change. So dell-pc not a platform device but
a "fake".

I'm not saying this is wrong, but feel I'm a bit just lost where the 
dividing line is. Is it just because this driver only happens to call
dell_send_request(), etc., not contains that low-level access code within? 
Or is that dell-smbios "fake" too?

> Signed-off-by: Kurt Borja <kuurtb@gmail.com>
> ---
>  drivers/platform/x86/dell/dell-pc.c | 46 +++++++++++--------------------------
>  1 file changed, 13 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/platform/x86/dell/dell-pc.c b/drivers/platform/x86/dell/dell-pc.c
> index 794924913be0c6f13ed4aed8b01ffd21f1d34dea..48cc7511905a62d2828e3a7b593b3d2dae893e34 100644
> --- a/drivers/platform/x86/dell/dell-pc.c
> +++ b/drivers/platform/x86/dell/dell-pc.c
> @@ -13,18 +13,18 @@
>  #include <linux/bitfield.h>
>  #include <linux/bitops.h>
>  #include <linux/bits.h>
> +#include <linux/device/faux.h>
>  #include <linux/dmi.h>
>  #include <linux/err.h>
>  #include <linux/init.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/platform_profile.h>
> -#include <linux/platform_device.h>
>  #include <linux/slab.h>
>  
>  #include "dell-smbios.h"
>  
> -static struct platform_device *platform_device;
> +static struct faux_device *dell_pc_fdev;
>  static int supported_modes;
>  
>  static const struct dmi_system_id dell_device_table[] __initconst = {
> @@ -246,7 +246,7 @@ static const struct platform_profile_ops dell_pc_platform_profile_ops = {
>  	.profile_set = thermal_platform_profile_set,
>  };
>  
> -static int thermal_init(void)
> +static int dell_pc_faux_probe(struct faux_device *fdev)
>  {
>  	struct device *ppdev;
>  	int ret;
> @@ -258,51 +258,31 @@ static int thermal_init(void)
>  	if (ret < 0)
>  		return ret;
>  
> -	platform_device = platform_device_register_simple("dell-pc", PLATFORM_DEVID_NONE, NULL, 0);
> -	if (IS_ERR(platform_device))
> -		return PTR_ERR(platform_device);
> +	ppdev = devm_platform_profile_register(&fdev->dev, "dell-pc", NULL,
> +					       &dell_pc_platform_profile_ops);
>  
> -	ppdev = devm_platform_profile_register(&platform_device->dev, "dell-pc",
> -					       NULL, &dell_pc_platform_profile_ops);
> -	if (IS_ERR(ppdev)) {
> -		ret = PTR_ERR(ppdev);
> -		goto cleanup_platform_device;
> -	}
> -
> -	return 0;
> -
> -cleanup_platform_device:
> -	platform_device_unregister(platform_device);
> -
> -	return ret;
> +	return PTR_ERR_OR_ZERO(ppdev);
>  }
>  
> -static void thermal_cleanup(void)
> -{
> -	platform_device_unregister(platform_device);
> -}
> +static const struct faux_device_ops dell_pc_faux_ops = {
> +	.probe = dell_pc_faux_probe,
> +};
>  
>  static int __init dell_init(void)
>  {
> -	int ret;
> -
>  	if (!dmi_check_system(dell_device_table))
>  		return -ENODEV;
>  
> -	ret = thermal_init();
> -	if (ret)
> -		goto fail_thermal;
> +	dell_pc_fdev = faux_device_create("dell-pc", NULL, &dell_pc_faux_ops);
> +	if (!dell_pc_fdev)
> +		return -ENODEV;
>  
>  	return 0;
> -
> -fail_thermal:
> -	thermal_cleanup();
> -	return ret;
>  }
>  
>  static void __exit dell_exit(void)
>  {
> -	thermal_cleanup();
> +	faux_device_destroy(dell_pc_fdev);
>  }
>  
>  module_init(dell_init);
> 
> 

-- 
 i.

Re: [PATCH 3/3] platform/x86: dell-pc: Transition to faux device

[Hans de Goede]

Hi Ilpo,

On 23-Apr-25 3:27 PM, Ilpo Järvinen wrote:
> On Fri, 11 Apr 2025, Kurt Borja wrote:
> 
>> Use a faux device parent for registering the platform_profile instead of
>> a "fake" platform device.
>>
>> The faux bus is a minimalistic, single driver bus designed for this
>> purpose.
> 
> Hi Kurt, Hans & Greg,
> 
> I'm not sure about this change. So dell-pc not a platform device but
> a "fake".

Arguably the dell-pc driver does not need a struct device at all,
since it just exports /sys/firmware/acpi/platform_profile sysfs
interface by using the relevant Dell SMBIOS interfaces for this.

As such maybe we should just completely get rid of the whole
struct device here?

If we do decide to keep the struct device, then since the struct device
seems to just be there to tie the lifetime of the platform_profile
handler to, I guess that calling it a faux device is fair.

> I'm not saying this is wrong, but feel I'm a bit just lost where the 
> dividing line is.

In this case it seems to be clear that this is a faux device,
but I do agree that sometimes the line can be a bit blurry.

Regards,

Hans




> Is it just because this driver only happens to call
> dell_send_request(), etc., not contains that low-level access code within? 
> Or is that dell-smbios "fake" too?
> 
>> Signed-off-by: Kurt Borja <kuurtb@gmail.com>
>> ---
>>  drivers/platform/x86/dell/dell-pc.c | 46 +++++++++++--------------------------
>>  1 file changed, 13 insertions(+), 33 deletions(-)
>>
>> diff --git a/drivers/platform/x86/dell/dell-pc.c b/drivers/platform/x86/dell/dell-pc.c
>> index 794924913be0c6f13ed4aed8b01ffd21f1d34dea..48cc7511905a62d2828e3a7b593b3d2dae893e34 100644
>> --- a/drivers/platform/x86/dell/dell-pc.c
>> +++ b/drivers/platform/x86/dell/dell-pc.c
>> @@ -13,18 +13,18 @@
>>  #include <linux/bitfield.h>
>>  #include <linux/bitops.h>
>>  #include <linux/bits.h>
>> +#include <linux/device/faux.h>
>>  #include <linux/dmi.h>
>>  #include <linux/err.h>
>>  #include <linux/init.h>
>>  #include <linux/kernel.h>
>>  #include <linux/module.h>
>>  #include <linux/platform_profile.h>
>> -#include <linux/platform_device.h>
>>  #include <linux/slab.h>
>>  
>>  #include "dell-smbios.h"
>>  
>> -static struct platform_device *platform_device;
>> +static struct faux_device *dell_pc_fdev;
>>  static int supported_modes;
>>  
>>  static const struct dmi_system_id dell_device_table[] __initconst = {
>> @@ -246,7 +246,7 @@ static const struct platform_profile_ops dell_pc_platform_profile_ops = {
>>  	.profile_set = thermal_platform_profile_set,
>>  };
>>  
>> -static int thermal_init(void)
>> +static int dell_pc_faux_probe(struct faux_device *fdev)
>>  {
>>  	struct device *ppdev;
>>  	int ret;
>> @@ -258,51 +258,31 @@ static int thermal_init(void)
>>  	if (ret < 0)
>>  		return ret;
>>  
>> -	platform_device = platform_device_register_simple("dell-pc", PLATFORM_DEVID_NONE, NULL, 0);
>> -	if (IS_ERR(platform_device))
>> -		return PTR_ERR(platform_device);
>> +	ppdev = devm_platform_profile_register(&fdev->dev, "dell-pc", NULL,
>> +					       &dell_pc_platform_profile_ops);
>>  
>> -	ppdev = devm_platform_profile_register(&platform_device->dev, "dell-pc",
>> -					       NULL, &dell_pc_platform_profile_ops);
>> -	if (IS_ERR(ppdev)) {
>> -		ret = PTR_ERR(ppdev);
>> -		goto cleanup_platform_device;
>> -	}
>> -
>> -	return 0;
>> -
>> -cleanup_platform_device:
>> -	platform_device_unregister(platform_device);
>> -
>> -	return ret;
>> +	return PTR_ERR_OR_ZERO(ppdev);
>>  }
>>  
>> -static void thermal_cleanup(void)
>> -{
>> -	platform_device_unregister(platform_device);
>> -}
>> +static const struct faux_device_ops dell_pc_faux_ops = {
>> +	.probe = dell_pc_faux_probe,
>> +};
>>  
>>  static int __init dell_init(void)
>>  {
>> -	int ret;
>> -
>>  	if (!dmi_check_system(dell_device_table))
>>  		return -ENODEV;
>>  
>> -	ret = thermal_init();
>> -	if (ret)
>> -		goto fail_thermal;
>> +	dell_pc_fdev = faux_device_create("dell-pc", NULL, &dell_pc_faux_ops);
>> +	if (!dell_pc_fdev)
>> +		return -ENODEV;
>>  
>>  	return 0;
>> -
>> -fail_thermal:
>> -	thermal_cleanup();
>> -	return ret;
>>  }
>>  
>>  static void __exit dell_exit(void)
>>  {
>> -	thermal_cleanup();
>> +	faux_device_destroy(dell_pc_fdev);
>>  }
>>  
>>  module_init(dell_init);
>>
>>
> 

Re: [PATCH 3/3] platform/x86: dell-pc: Transition to faux device

[Greg Kroah-Hartman]

On Wed, Apr 23, 2025 at 03:44:56PM +0200, Hans de Goede wrote:
> Hi Ilpo,
> 
> On 23-Apr-25 3:27 PM, Ilpo Järvinen wrote:
> > On Fri, 11 Apr 2025, Kurt Borja wrote:
> > 
> >> Use a faux device parent for registering the platform_profile instead of
> >> a "fake" platform device.
> >>
> >> The faux bus is a minimalistic, single driver bus designed for this
> >> purpose.
> > 
> > Hi Kurt, Hans & Greg,
> > 
> > I'm not sure about this change. So dell-pc not a platform device but
> > a "fake".
> 
> Arguably the dell-pc driver does not need a struct device at all,
> since it just exports /sys/firmware/acpi/platform_profile sysfs
> interface by using the relevant Dell SMBIOS interfaces for this.
> 
> As such maybe we should just completely get rid of the whole
> struct device here?
> 
> If we do decide to keep the struct device, then since the struct device
> seems to just be there to tie the lifetime of the platform_profile
> handler to, I guess that calling it a faux device is fair.
> 
> > I'm not saying this is wrong, but feel I'm a bit just lost where the 
> > dividing line is.
> 
> In this case it seems to be clear that this is a faux device,
> but I do agree that sometimes the line can be a bit blurry.

If a device needs access to platform resources, then it is a platform
device.  If not, then it is not.  Not too complex :)

But (you knew there was a but), many drivers want to detach their
ability to create a device, and have a driver bind to them, in a
different "place" in the kernel.  For many of those, they have (ab)used
the platform driver/device api to achieve this, despite them not being a
platform device at all.  For these, we can't convert them directly to
use faux bus, as it's not as simple of a conversion and in some places,
doesn't work well.  So let's leave those alone for now, but not take any
more of them going forward in the future.

hope this helps,

greg k-h

Re: [PATCH 3/3] platform/x86: dell-pc: Transition to faux device

[Lyndon Sanche]

On Wed, Apr 23, 2025, at 7:59 AM, Greg Kroah-Hartman wrote:
> On Wed, Apr 23, 2025 at 03:44:56PM +0200, Hans de Goede wrote:
>> 
>> Arguably the dell-pc driver does not need a struct device at all,
>> since it just exports /sys/firmware/acpi/platform_profile sysfs
>> interface by using the relevant Dell SMBIOS interfaces for this.
>> 
>> As such maybe we should just completely get rid of the whole
>> struct device here?
>> 
>> If we do decide to keep the struct device, then since the struct device
>> seems to just be there to tie the lifetime of the platform_profile
>> handler to, I guess that calling it a faux device is fair.

I am curious to see what this would look like. If we can get away with not using a struct for this functionality I think that is a good way to keep it simple.

>
> If a device needs access to platform resources, then it is a platform
> device.  If not, then it is not.  Not too complex :)
>
> But (you knew there was a but), many drivers want to detach their
> ability to create a device, and have a driver bind to them, in a
> different "place" in the kernel.  For many of those, they have (ab)used
> the platform driver/device api to achieve this, despite them not being a
> platform device at all.  For these, we can't convert them directly to
> use faux bus, as it's not as simple of a conversion and in some places,
> doesn't work well.  So let's leave those alone for now, but not take any
> more of them going forward in the future.

Right now we are just using the platform device to register a platform profile handler. If we only need the faux bus for that then I think that is a good way to go. I will look into this new faux bus a bit more.

I'll see if I can find some time this evening to run this patch.

Thanks,

Lyndon

Re: [PATCH 3/3] platform/x86: dell-pc: Transition to faux device

[Kurt Borja]

Hi all,

On Wed Apr 23, 2025 at 10:44 AM -03, Hans de Goede wrote:
> Hi Ilpo,
>
> On 23-Apr-25 3:27 PM, Ilpo Järvinen wrote:
>> On Fri, 11 Apr 2025, Kurt Borja wrote:
>> 
>>> Use a faux device parent for registering the platform_profile instead of
>>> a "fake" platform device.
>>>
>>> The faux bus is a minimalistic, single driver bus designed for this
>>> purpose.
>> 
>> Hi Kurt, Hans & Greg,
>> 
>> I'm not sure about this change. So dell-pc not a platform device but
>> a "fake".
>
> Arguably the dell-pc driver does not need a struct device at all,
> since it just exports /sys/firmware/acpi/platform_profile sysfs
> interface by using the relevant Dell SMBIOS interfaces for this.
>
> As such maybe we should just completely get rid of the whole
> struct device here?
>
> If we do decide to keep the struct device, then since the struct device
> seems to just be there to tie the lifetime of the platform_profile
> handler to, I guess that calling it a faux device is fair.

I think it's important to mention that a parent device is required to
register a platform profile, see [1].

I guess we could get away with removing the device altogether from here,
but that would require to find another suitable parent device. The
obvious choice would be the `dell-smbios` device, however that would
require exporting it in the first place.

For some reason, exporting devices doesn't seem right to me, so IMO a
faux device is a good choice here.

Another solution that would make more sense, lifetime wise, is to turn
this into an aux driver and let `dell-smbios` create the matching aux
device. I could do this, but I think it's overly complicated.

>
>> I'm not saying this is wrong, but feel I'm a bit just lost where the 
>> dividing line is.
>
> In this case it seems to be clear that this is a faux device,
> but I do agree that sometimes the line can be a bit blurry.
>
> Regards,
>
> Hans
>
>
>
>
>> Is it just because this driver only happens to call
>> dell_send_request(), etc., not contains that low-level access code within? 
>> Or is that dell-smbios "fake" too?

IMO `dell-smbios` is "fake" too? It is there only to expose either the
WMI or the SMM backend through a single sysfs interface.

I think a more natural design for `dell-smbios` would be an aux driver
that exposed it's interface through a class device. Maybe I'm wrong in
this regard though.

[1] https://elixir.bootlin.com/linux/v6.15-rc3/source/drivers/acpi/platform_profile.c#L556

-- 
 ~ Kurt

>> 
>>> Signed-off-by: Kurt Borja <kuurtb@gmail.com>
>>> ---
>>>  drivers/platform/x86/dell/dell-pc.c | 46 +++++++++++--------------------------
>>>  1 file changed, 13 insertions(+), 33 deletions(-)
>>>
>>> diff --git a/drivers/platform/x86/dell/dell-pc.c b/drivers/platform/x86/dell/dell-pc.c
>>> index 794924913be0c6f13ed4aed8b01ffd21f1d34dea..48cc7511905a62d2828e3a7b593b3d2dae893e34 100644
>>> --- a/drivers/platform/x86/dell/dell-pc.c
>>> +++ b/drivers/platform/x86/dell/dell-pc.c
>>> @@ -13,18 +13,18 @@
>>>  #include <linux/bitfield.h>
>>>  #include <linux/bitops.h>
>>>  #include <linux/bits.h>
>>> +#include <linux/device/faux.h>
>>>  #include <linux/dmi.h>
>>>  #include <linux/err.h>
>>>  #include <linux/init.h>
>>>  #include <linux/kernel.h>
>>>  #include <linux/module.h>
>>>  #include <linux/platform_profile.h>
>>> -#include <linux/platform_device.h>
>>>  #include <linux/slab.h>
>>>  
>>>  #include "dell-smbios.h"
>>>  
>>> -static struct platform_device *platform_device;
>>> +static struct faux_device *dell_pc_fdev;
>>>  static int supported_modes;
>>>  
>>>  static const struct dmi_system_id dell_device_table[] __initconst = {
>>> @@ -246,7 +246,7 @@ static const struct platform_profile_ops dell_pc_platform_profile_ops = {
>>>  	.profile_set = thermal_platform_profile_set,
>>>  };
>>>  
>>> -static int thermal_init(void)
>>> +static int dell_pc_faux_probe(struct faux_device *fdev)
>>>  {
>>>  	struct device *ppdev;
>>>  	int ret;
>>> @@ -258,51 +258,31 @@ static int thermal_init(void)
>>>  	if (ret < 0)
>>>  		return ret;
>>>  
>>> -	platform_device = platform_device_register_simple("dell-pc", PLATFORM_DEVID_NONE, NULL, 0);
>>> -	if (IS_ERR(platform_device))
>>> -		return PTR_ERR(platform_device);
>>> +	ppdev = devm_platform_profile_register(&fdev->dev, "dell-pc", NULL,
>>> +					       &dell_pc_platform_profile_ops);
>>>  
>>> -	ppdev = devm_platform_profile_register(&platform_device->dev, "dell-pc",
>>> -					       NULL, &dell_pc_platform_profile_ops);
>>> -	if (IS_ERR(ppdev)) {
>>> -		ret = PTR_ERR(ppdev);
>>> -		goto cleanup_platform_device;
>>> -	}
>>> -
>>> -	return 0;
>>> -
>>> -cleanup_platform_device:
>>> -	platform_device_unregister(platform_device);
>>> -
>>> -	return ret;
>>> +	return PTR_ERR_OR_ZERO(ppdev);
>>>  }
>>>  
>>> -static void thermal_cleanup(void)
>>> -{
>>> -	platform_device_unregister(platform_device);
>>> -}
>>> +static const struct faux_device_ops dell_pc_faux_ops = {
>>> +	.probe = dell_pc_faux_probe,
>>> +};
>>>  
>>>  static int __init dell_init(void)
>>>  {
>>> -	int ret;
>>> -
>>>  	if (!dmi_check_system(dell_device_table))
>>>  		return -ENODEV;
>>>  
>>> -	ret = thermal_init();
>>> -	if (ret)
>>> -		goto fail_thermal;
>>> +	dell_pc_fdev = faux_device_create("dell-pc", NULL, &dell_pc_faux_ops);
>>> +	if (!dell_pc_fdev)
>>> +		return -ENODEV;
>>>  
>>>  	return 0;
>>> -
>>> -fail_thermal:
>>> -	thermal_cleanup();
>>> -	return ret;
>>>  }
>>>  
>>>  static void __exit dell_exit(void)
>>>  {
>>> -	thermal_cleanup();
>>> +	faux_device_destroy(dell_pc_fdev);
>>>  }
>>>  
>>>  module_init(dell_init);
>>>
>>>
>> 

Re: [PATCH 3/3] platform/x86: dell-pc: Transition to faux device

[Hans de Goede]

Hi Kurt,

On 23-Apr-25 6:14 PM, Kurt Borja wrote:
> Hi all,
> 
> On Wed Apr 23, 2025 at 10:44 AM -03, Hans de Goede wrote:
>> Hi Ilpo,
>>
>> On 23-Apr-25 3:27 PM, Ilpo Järvinen wrote:
>>> On Fri, 11 Apr 2025, Kurt Borja wrote:
>>>
>>>> Use a faux device parent for registering the platform_profile instead of
>>>> a "fake" platform device.
>>>>
>>>> The faux bus is a minimalistic, single driver bus designed for this
>>>> purpose.
>>>
>>> Hi Kurt, Hans & Greg,
>>>
>>> I'm not sure about this change. So dell-pc not a platform device but
>>> a "fake".
>>
>> Arguably the dell-pc driver does not need a struct device at all,
>> since it just exports /sys/firmware/acpi/platform_profile sysfs
>> interface by using the relevant Dell SMBIOS interfaces for this.
>>
>> As such maybe we should just completely get rid of the whole
>> struct device here?
>>
>> If we do decide to keep the struct device, then since the struct device
>> seems to just be there to tie the lifetime of the platform_profile
>> handler to, I guess that calling it a faux device is fair.
> 
> I think it's important to mention that a parent device is required to
> register a platform profile, see [1].

Ah ok, that is new, I guess that was changed with the new support
for registering multiple platform-profile handlers.

> I guess we could get away with removing the device altogether from here,
> but that would require to find another suitable parent device. The
> obvious choice would be the `dell-smbios` device, however that would
> require exporting it in the first place.
> 
> For some reason, exporting devices doesn't seem right to me, so IMO a
> faux device is a good choice here.

Agreed.

> Another solution that would make more sense, lifetime wise, is to turn
> this into an aux driver and let `dell-smbios` create the matching aux
> device. I could do this, but I think it's overly complicated.

Yes that does seem overly complicated, lets just go with the faux
device.

Regards,

Hans




>>> Is it just because this driver only happens to call
>>> dell_send_request(), etc., not contains that low-level access code within? 
>>> Or is that dell-smbios "fake" too?
> 
> IMO `dell-smbios` is "fake" too? It is there only to expose either the
> WMI or the SMM backend through a single sysfs interface.
> 
> I think a more natural design for `dell-smbios` would be an aux driver
> that exposed it's interface through a class device. Maybe I'm wrong in
> this regard though.
> 
> [1] https://elixir.bootlin.com/linux/v6.15-rc3/source/drivers/acpi/platform_profile.c#L556
> 

Re: [PATCH 3/3] platform/x86: dell-pc: Transition to faux device

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 2939 bytes --]

On Wed, 23 Apr 2025, Hans de Goede wrote:
> On 23-Apr-25 6:14 PM, Kurt Borja wrote:
> > On Wed Apr 23, 2025 at 10:44 AM -03, Hans de Goede wrote:
> >> On 23-Apr-25 3:27 PM, Ilpo Järvinen wrote:
> >>> On Fri, 11 Apr 2025, Kurt Borja wrote:
> >>>
> >>>> Use a faux device parent for registering the platform_profile instead of
> >>>> a "fake" platform device.
> >>>>
> >>>> The faux bus is a minimalistic, single driver bus designed for this
> >>>> purpose.
> >>>
> >>> Hi Kurt, Hans & Greg,
> >>>
> >>> I'm not sure about this change. So dell-pc not a platform device but
> >>> a "fake".
> >>
> >> Arguably the dell-pc driver does not need a struct device at all,
> >> since it just exports /sys/firmware/acpi/platform_profile sysfs
> >> interface by using the relevant Dell SMBIOS interfaces for this.
> >>
> >> As such maybe we should just completely get rid of the whole
> >> struct device here?
> >>
> >> If we do decide to keep the struct device, then since the struct device
> >> seems to just be there to tie the lifetime of the platform_profile
> >> handler to, I guess that calling it a faux device is fair.
> > 
> > I think it's important to mention that a parent device is required to
> > register a platform profile, see [1].
> 
> Ah ok, that is new, I guess that was changed with the new support
> for registering multiple platform-profile handlers.
> 
> > I guess we could get away with removing the device altogether from here,
> > but that would require to find another suitable parent device. The
> > obvious choice would be the `dell-smbios` device, however that would
> > require exporting it in the first place.
> > 
> > For some reason, exporting devices doesn't seem right to me, so IMO a
> > faux device is a good choice here.
> 
> Agreed.
> 
> > Another solution that would make more sense, lifetime wise, is to turn
> > this into an aux driver and let `dell-smbios` create the matching aux
> > device.

Well, that was what caused part of my confusion / uncertainty here as
I could see that aux bus between these two drivers. Obviously, it's not 
there currently but conceptually this relationship looks what full-blown 
aux bus was supposed to solve.

The other part was that as per Greg's simple classification, certainly 
this driver needs to access platform resources. BUT, that access is routed 
through another driver which is a case his answer/classification did not 
cover.

> > I could do this, but I think it's overly complicated.
>
> Yes that does seem overly complicated, lets just go with the faux
> device.

Okay. In part, this was also to check whether replacing full-blown aux bus 
with faux should be considered another kind of "abuse". I've no problem 
with accepting faux for cases like this as I see these as policy / 
convention decision more than one being right and another wrong. :-)

Thanks to all who answered.

-- 
 i.

Re: [PATCH 0/3] platform/x86: dell-pc: Transition to faux device

[Ilpo Järvinen]

On Fri, 11 Apr 2025 11:36:40 -0300, Kurt Borja wrote:

> In commit
> 
> 	35fa2d88ca94 ("driver core: add a faux bus for use when a simple device/bus is needed")
> 
> the new faux bus was introduced. It was designed for cases where a
> module needs a fake parent device.
> 
> [...]


Thank you for your contribution, it has been applied to my local
review-ilpo-next branch. Note it will show up in the public
platform-drivers-x86/review-ilpo-next branch only once I've pushed my
local branch there, which might take a while.

The list of commits applied:
[1/3] platform/x86: dell-pc: Propagate errors when detecting feature support
      commit: 4630b99d2e93a91b304f498c4d543c002fb78ca5
[2/3] platform/x86: dell-pc: Use non-atomic bitmap operations
      commit: 48e21e0226a9325fc75145840d289113fb0c27bc
[3/3] platform/x86: dell-pc: Transition to faux device
      commit: 99fb11d1edb2104f976a672a0863f1ea1ea27398

--
 i.

Re: [PATCH 3/3] platform/x86: dell-pc: Transition to faux device

[Kurt Borja]

[-- Attachment #1.1.1: Type: text/plain, Size: 3551 bytes --]

On Thu Apr 24, 2025 at 8:57 AM -03, Ilpo Järvinen wrote:
> On Wed, 23 Apr 2025, Hans de Goede wrote:
>> On 23-Apr-25 6:14 PM, Kurt Borja wrote:
>> > On Wed Apr 23, 2025 at 10:44 AM -03, Hans de Goede wrote:
>> >> On 23-Apr-25 3:27 PM, Ilpo Järvinen wrote:
>> >>> On Fri, 11 Apr 2025, Kurt Borja wrote:
>> >>>
>> >>>> Use a faux device parent for registering the platform_profile instead of
>> >>>> a "fake" platform device.
>> >>>>
>> >>>> The faux bus is a minimalistic, single driver bus designed for this
>> >>>> purpose.
>> >>>
>> >>> Hi Kurt, Hans & Greg,
>> >>>
>> >>> I'm not sure about this change. So dell-pc not a platform device but
>> >>> a "fake".
>> >>
>> >> Arguably the dell-pc driver does not need a struct device at all,
>> >> since it just exports /sys/firmware/acpi/platform_profile sysfs
>> >> interface by using the relevant Dell SMBIOS interfaces for this.
>> >>
>> >> As such maybe we should just completely get rid of the whole
>> >> struct device here?
>> >>
>> >> If we do decide to keep the struct device, then since the struct device
>> >> seems to just be there to tie the lifetime of the platform_profile
>> >> handler to, I guess that calling it a faux device is fair.
>> > 
>> > I think it's important to mention that a parent device is required to
>> > register a platform profile, see [1].
>> 
>> Ah ok, that is new, I guess that was changed with the new support
>> for registering multiple platform-profile handlers.
>> 
>> > I guess we could get away with removing the device altogether from here,
>> > but that would require to find another suitable parent device. The
>> > obvious choice would be the `dell-smbios` device, however that would
>> > require exporting it in the first place.
>> > 
>> > For some reason, exporting devices doesn't seem right to me, so IMO a
>> > faux device is a good choice here.
>> 
>> Agreed.
>> 
>> > Another solution that would make more sense, lifetime wise, is to turn
>> > this into an aux driver and let `dell-smbios` create the matching aux
>> > device.
>
> Well, that was what caused part of my confusion / uncertainty here as
> I could see that aux bus between these two drivers. Obviously, it's not 
> there currently but conceptually this relationship looks what full-blown 
> aux bus was supposed to solve.
>
> The other part was that as per Greg's simple classification, certainly 
> this driver needs to access platform resources. BUT, that access is routed 
> through another driver which is a case his answer/classification did not 
> cover.

Perhaps it didn't cover it because, as you mentioned, this falls under
the aux bus use cases.

>
>> > I could do this, but I think it's overly complicated.
>>
>> Yes that does seem overly complicated, lets just go with the faux
>> device.
>
> Okay. In part, this was also to check whether replacing full-blown aux bus 
> with faux should be considered another kind of "abuse". I've no problem 
> with accepting faux for cases like this as I see these as policy / 
> convention decision more than one being right and another wrong. :-)

Now that you put it that way, I guess this still is kind of "abusive",
but is still an improvement over creating a full platform device.

Nevertheless, although this driver do access platform resources, it does
it completely detached from the "dell-smbios" device. The only use of
the platform device here was `&pdev->dev` :p

>
> Thanks to all who answered.

Thanks for your review!

-- 
 ~ Kurt

[-- Attachment #1.2: 57E3B6585920A69A.asc --]
[-- Type: application/pgp-keys, Size: 4349 bytes --]

-----BEGIN PGP PUBLIC KEY BLOCK-----

mQINBGgKemsBEADlBa3abjsoEof7YAphRayi3YrBgIp06UinSkhCkv7g4daNUUvz
D8clrNHZ6GCt4DXEjbbApQZaZSwn+/e0Gt7FAK8HYftVir158tftWk8JPqN3cZaH
a15B+0svYc9bBsmSZyMNsllIBOE3h4oSqIVrPuEWEqK+6cCaccj95zwYjsJ/T4XR
AlReC13r1tQ4zOOii9LgULfqXHmUkrxxvzLLB7ZAgsxEIT1nN+zJvPxCeNGjqY1B
ID0l5AWyrtp3bQFrcZN/RDlpDwS0kopvZaYxCL7ZxOoZtwv9Tz6gP30Wrcx5JGQq
zvRpmU++TYkNBTi/gXwUTbibTT9ARF5/IERnOrDe73Ylz770yP4fS4Vp9epMphjN
J59Fdk0TaksL6tAckZ3RVcLkjBIj7asXf9tqM6GP4uX4lZvc9AWtCbFEFjwYATru
h1fczHbq7x+Escgc3baob5WpAvDRV2U6aFeYJw8T2SqHEr9O7MPHV9GdvOKarks1
PzjxP+29vX0+ySR7kPcl3duuJcxPutBK/4tyYgimEABYuKvZgfKZVBtEDPXy3Sx/
meWxCb4aihSCFvoeMZQJijNrMa/VHJPjzxKzjDDtCLmZsSeDjXGjOEzEalO4HtHp
nSymg8urfrrRtHZK5yQ++h2a8XctnPkrU7r0L1BoA5E54uqPGjgoYo4n9QARAQAB
tB1LdXJ0IEJvcmphIDxrdXVydGJAZ21haWwuY29tPokCVwQTAQgAQQIbAQULCQgH
AgIiAgYVCgkICwIEFgIDAQIeBwIXgBYhBFTTvhcK73d5g8PGO1fjtlhZIKaaBQJo
CnxxBQkE6GeGAAoJEFfjtlhZIKaa/GUP/R3wfzFRK+SnTo5tbJARBG9eEzLcIcLk
+aC840gz13oLWD0HWs90DjqNOz4PfPYWyEljedg33ecgQi5A0T4uE3PLoczB1cGm
UKbNwEU3URnzxRGm8e2S1CyayUQBFlNaJlScokLYoJbxpeFNCbUTMXuayono6vlz
mOGFflie99u6RsfaTG+QoC2uRPvWcpapUPFPswbOcoYKs+joDlevAChLv/DzqVTn
coLgTQTUI5tDkFPP1D8QToxfR4jBCvBslFrP+veKZEyUrvHKggNQEHBXzfCNYWfz
m5O4zYQbTK45ITyEfcFweJHjXDfw1PVppfzgT5Bx0OpCNylj7NE5whTpRnKTipn+
/WnNryDCnRfTvAQ/ssAp0UsC4eGfGgH9HO+jmTB2rKFsSjtZCm/wiTTJ+q8gWRzG
y8WXBau1NntAQ9zI5SZs+sbJmSMqrt7SB/NGb67vNt3jenWc2XiIeZ4XYzSLqpTP
8UIa959jDj+DJBRYBJAPHCan1/0qE0dx9cDgDxOUqQM0dzjw4e/PX2Px4UdW/Sn7
hDbDpAF1xi+t51K4fc1ewATLlnURddmIToZfbfqSnB12AI9ljoYNNzhNPcYbviQV
HS7cU8Vyuxao1oy3njxDWMrkRF8p105o6yveAqILqrj+0hQKycchw3qqdvxQ+wVJ
JaKk4wpR1Jm6uDgEaAp7kBIKKwYBBAGXVQEFAQEHQOwklpBpjcr2GFlZhbGaWZLs
b04yNqi+0PQGgJQ1xH1rAwEIB4kCNgQYAQgAIBYhBFTTvhcK73d5g8PGO1fjtlhZ
IKaaBQJoCnuQAhsMAAoJEFfjtlhZIKaaXHQP/2M2yaOLUvm9ycz909kgSLow/9Qh
ztffxyO2GDDeXzUB9g1iCY5ssdgSiR5ygxrzwqjZwuAGk2voE6D9zRvwlCjUe/Yc
/IU6bSwwLEoa5O772EnbIzJvVEaLY3bYtviNNJixkAkRB0KX8V/fkgy2MEKTl1cO
rtoj3UKX1XTdgeh26X/2qS93iAKsUur2268U5EQsSf2A8UB38XYu48FGGQkW6aAS
4pONJV50WsiFGfZpiQbSa7XCj5pz5ba+vGSC/OHymCMAb+gRP3jdD0h35HXEkzQC
RTYzxWdfnWcBHgCl4SQN3pAnZp898/ay4+3rAg/T7eAcW3RTgU/HUEG24GzqNHJf
Lb5xtD9DBpjkhujlIo1Ggzl5APItcCRxDM3P0lCw8V+gkK5cKSfL/kh73qmb4KXq
4Em9I2m2xg2E6dQw7p4cksWUDbkoKbGMI52S/q6PQLN4i5IM2FBBIpZzFJFm7Hlq
40vHGwQyc61M4kNRhMX5e6ctdVE6Q7PbF64MQ3oY+sSTMc2sg6g1/cYdKseTzAK3
A14sw61CbDcnarU/v/fXDn5GsnhMByxHyGcTpxKar+OZxapHI0hJsJPIj01Ylqq7
PEu4Nqhmtmz5ubHF+aSR+PxaRi2uUUgzZnZfqmZle4GnQ0D8vyE7zM8mI78I/NGw
BLElwcXPsQBf3G5auDMEaAp7txYJKwYBBAHaRw8BAQdAy7v1AAo9uvDYGAxCfnrw
nXKW3QHPfQp9reFXSlDxtDOJAq0EGAEIACAWIQRU074XCu93eYPDxjtX47ZYWSCm
mgUCaAp7twIbAgCBCRBX47ZYWSCmmnYgBBkWCgAdFiEEh2Ci9uJabu1OwFXfFmBD
OPSf1GYFAmgKe7cACgkQFmBDOPSf1GaznAEAzDC4NSbsiPck4j4K+vBh4VjUW3Jd
M05VC5KqoEs4KbgBAI4XrcqkDhjDoVnudL2CMRClkLBZdCIacm5kVeKZQgUM45EQ
AKQbwP3P7kZt52oo5xZV1yrap7VfvBnjpIwNsjT/4p1W5Wwzriet5ximkm6tf/ub
FWlE22WWjtBmed9fGR5NZoGowPAjt/rFN2zOsFQWdOApeUlzzmVMLNZjp030h3Xg
oNPRP3W7Cb1FocGIgJgcJ5UsKKoTTCa0jebjnMshen+nlr4JI6dINeCRYw1nh+9b
yr1UoatS35vHJLBaUkLuBv8XUPRmKK7rkhhvyVv/NKfIcia+Mr6h5BVOls7T3mD0
7OFbpqbdOHva/xvL/66DUp5riJ3CzvOLaQXMiCHsxvymwJ6Iajxvl26YmZ7Tyavt
v+koowloExuVoGRtdMv+oP1mnTeidIx1Qn+i0a9H3QWSfnbPD8pEoDoDVrFubxdM
FIwEZ4kc9rUXXvu8sVhCM9N/YSam7zEAOUhQJNUGbNpK8HgL32kaTKJxkUQB571P
F8CgGPE6RNoEnA/9IQIKCEL2x03N0evnGktiXgpFsKzsZUrMHlyRplh3kkPHytAG
PcY6KR5P4rh2TYaO+ZAklCyLF0mbno8BSVq/liuzfC4yCkh/tYh8uXG+ezNllaBz
g3RSp7b0MeiiDNGOdHlgouhcpRIS4R/RFarkO4Z26TX+NVIv1H8pLji3/gcaXWDb
NSsfJULbL+IE/Yfr8Nj8Rt5YJZwhHmxMIVwuhCi9JH+IuDMEaAp7xxYJKwYBBAHa
Rw8BAQdAKkxKhQUR39+VwLvyev1AR1l1ardbn+qtj2PicyY0CRGJAjYEGAEIACAW
IQRU074XCu93eYPDxjtX47ZYWSCmmgUCaAp7xwIbIAAKCRBX47ZYWSCmmgllD/4h
f+MD3lHN/z4kI8zjKJ4sh+6wq79Mb1lGZyFgNm4ZW8TYJVN6G/CJN/Y/AFwX0izj
Qamzs6A5FZf6EFR4lkbFLFFU8QtC3NeddlC8Rq9C409wrbR+G/Qp78KWyvyVMeEI
U0Qk4mtmGmrnkKpii116O5uof3W2eVzYry9AlWNk0qP2DZ/BxYL8XBH+mkLPyLKa
O60fXflRL+616kd5IAffwZq97mWU9GSkmeY984D+cqXrqWCXx1UlHz+qzszzAkzH
muz2NRWloicZ0MrcHw5ftDvIzY1hIRdRcHdk3tLDpXxkZldDDS43apGLGeDf3rHz
QHgDtCSx1fT3OL2PT8nZPki3mnWJ/KwX6u2rrErP92OYPX1ACcwQ3uKNlvevZkXU
amaRoIrZD65EDobgokCwwfoK0Ga+8I0oftFKBV5CZFixl9hf8XzjWK6eif8QUk38
fhheO3PpIAxh8+pHzpdIjEAJI3ycbdYhFRXprN610K37cXmwljrD/YhiyUHLAIHf
9RepyH30UJLiwVNMHOZlRnp4GaKEkpGP0tSE2ZoSGWuCw7BzOQmjTnRrow41SmWN
CG13hiGSG3hvTwkgOYr7fJQydFqQd+NOnp3aDc8r+2josRWp9MiwlK2euRkTygsy
wOBUvzk73mjU+O6iYOw7hhDbXKcZ3HQ/5mh5b8FCzA==
=GRvV
-----END PGP PUBLIC KEY BLOCK-----

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 3/3] platform/x86: dell-pc: Transition to faux device

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 4022 bytes --]

On Fri, 25 Apr 2025, Kurt Borja wrote:

> On Thu Apr 24, 2025 at 8:57 AM -03, Ilpo Järvinen wrote:
> > On Wed, 23 Apr 2025, Hans de Goede wrote:
> >> On 23-Apr-25 6:14 PM, Kurt Borja wrote:
> >> > On Wed Apr 23, 2025 at 10:44 AM -03, Hans de Goede wrote:
> >> >> On 23-Apr-25 3:27 PM, Ilpo Järvinen wrote:
> >> >>> On Fri, 11 Apr 2025, Kurt Borja wrote:
> >> >>>
> >> >>>> Use a faux device parent for registering the platform_profile instead of
> >> >>>> a "fake" platform device.
> >> >>>>
> >> >>>> The faux bus is a minimalistic, single driver bus designed for this
> >> >>>> purpose.
> >> >>>
> >> >>> Hi Kurt, Hans & Greg,
> >> >>>
> >> >>> I'm not sure about this change. So dell-pc not a platform device but
> >> >>> a "fake".
> >> >>
> >> >> Arguably the dell-pc driver does not need a struct device at all,
> >> >> since it just exports /sys/firmware/acpi/platform_profile sysfs
> >> >> interface by using the relevant Dell SMBIOS interfaces for this.
> >> >>
> >> >> As such maybe we should just completely get rid of the whole
> >> >> struct device here?
> >> >>
> >> >> If we do decide to keep the struct device, then since the struct device
> >> >> seems to just be there to tie the lifetime of the platform_profile
> >> >> handler to, I guess that calling it a faux device is fair.
> >> > 
> >> > I think it's important to mention that a parent device is required to
> >> > register a platform profile, see [1].
> >> 
> >> Ah ok, that is new, I guess that was changed with the new support
> >> for registering multiple platform-profile handlers.
> >> 
> >> > I guess we could get away with removing the device altogether from here,
> >> > but that would require to find another suitable parent device. The
> >> > obvious choice would be the `dell-smbios` device, however that would
> >> > require exporting it in the first place.
> >> > 
> >> > For some reason, exporting devices doesn't seem right to me, so IMO a
> >> > faux device is a good choice here.
> >> 
> >> Agreed.
> >> 
> >> > Another solution that would make more sense, lifetime wise, is to turn
> >> > this into an aux driver and let `dell-smbios` create the matching aux
> >> > device.
> >
> > Well, that was what caused part of my confusion / uncertainty here as
> > I could see that aux bus between these two drivers. Obviously, it's not 
> > there currently but conceptually this relationship looks what full-blown 
> > aux bus was supposed to solve.
> >
> > The other part was that as per Greg's simple classification, certainly 
> > this driver needs to access platform resources. BUT, that access is routed 
> > through another driver which is a case his answer/classification did not 
> > cover.
> 
> Perhaps it didn't cover it because, as you mentioned, this falls under
> the aux bus use cases.
> 
> >
> >> > I could do this, but I think it's overly complicated.
> >>
> >> Yes that does seem overly complicated, lets just go with the faux
> >> device.
> >
> > Okay. In part, this was also to check whether replacing full-blown aux bus 
> > with faux should be considered another kind of "abuse". I've no problem 
> > with accepting faux for cases like this as I see these as policy / 
> > convention decision more than one being right and another wrong. :-)
> 
> Now that you put it that way, I guess this still is kind of "abusive",
> but is still an improvement over creating a full platform device.
> 
> Nevertheless, although this driver do access platform resources, it does
> it completely detached from the "dell-smbios" device. The only use of
> the platform device here was `&pdev->dev` :p

Perhaps "completely detached" is just a synonym for the common assumption 
that there can be only one and abuse of statics, which are generally 
frowned upon but also commonly used in platform drivers regardless. So 
there are these caveats in that supposed complete detatchedness.

But yeah, in some case it makes things easier.

-- 
 i.