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

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

[-- 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.