Re: [PATCH v6] platform/x86: samsung-galaxybook: Add samsung-galaxybook driver

["Kurt Borja" <kuurtb@gmail.com>]

On Fri Jan 17, 2025 at 7:38 PM -05, Joshua Grisham wrote:
> Hi Ilpo!
>
> Den fre 17 jan. 2025 kl 18:36 skrev Ilpo Järvinen
> <ilpo.jarvinen@linux.intel.com>:
> >
> > > +     err = devm_platform_profile_register(&galaxybook->profile_handler);
> > > +     if (err)
> > > +             return err;
> >
> > As you might already know, I've in the meantime merged the Kurt's big
> > platform_profile series so these need to the be rebased on top of that.
> >
> > --
> >  i.
> >
>
> Yes thank you! Actually I see that there is also an update to
> firmware-attributes, i8042 filter, and platform_profile on for-next
> that all now need to be implemented in this driver since the v6 of the
> patch was posted. I have a working version drafted and will send it
> shortly (along with the comment folding and spacing requests you
> made).
>
> One question and maybe more directed to Kurt regarding the new

Hi Joshua,

> platform_profile interface is that (similar to how I did it with the
> above v6 implementation), it still feels cleanest to locally track the
> different "choices" within samsung-galaxybook since it can vary

You are guaranteed by the platform_profile API to only receive selected
choices in the profile_set callback and there is no limitations to the
profile_get one so I don't see the need to store the choices in any way.

> depending on the model, and this get_performance_mode_profile()
> function needs to be able to evaluate during runtime what choices are
> set vs not.. so in my draft for v7 I have opted to add a private data
> member for platform_profile_choices and set it up during "init" of the
> platform driver pretty much exactly like how is done here in v6, and
> then during the new platform_profile probe I am just copying the
> choices set from the private member to the choices given in the probe

You should just directly set choices in the probe, that is it's intended
purpose. No need to set it first and then transfer them in the probe.

> callback. I hope this will make sense but please do take an extra look
> at this when I post v7 and see if it looks ok or if there is a better
> way to do this (again keeping in mind that setting/getting during
> runtime will need to be aware of what bits were set up).

The platform_profile class takes care of this internally.

>
> Thank you again!

If you still find this confusing or have questions let me know! I also
left a couple of suggestions in the new version.

~ Kurt

>
> Best regards,
> Joshua