On Mon, 12 Aug 2024, at 7:37 PM, Ilpo Järvinen wrote:
> On Sun, 11 Aug 2024, Luke Jones wrote:
>
> > On Tue, 6 Aug 2024, at 10:16 PM, Ilpo Järvinen wrote:
> > > On Tue, 6 Aug 2024, Luke D. Jones wrote:
> > >
> > > > The fw_attributes_class provides a much cleaner interface to all of the
> > > > attributes introduced to asus-wmi. This patch moves all of these extra
> > > > attributes over to fw_attributes_class, and shifts the bulk of these
> > > > definitions to a new kernel module to reduce the clutter of asus-wmi
> > > > with the intention of deprecating the asus-wmi attributes in future.
> > > >
> > > > The work applies only to WMI methods which don't have a clearly defined
> > > > place within the sysfs and as a result ended up lumped together in
> > > > /sys/devices/platform/asus-nb-wmi/ with no standard API.
> > > >
> > > > Where possible the fw attrs now implement defaults, min, max, scalar,
> > > > choices, etc. As en example dgpu_disable becomes:
> > > >
> > > > /sys/class/firmware-attributes/asus-armoury/attributes/dgpu_disable/
> > > > ├── current_value
> > > > ├── display_name
> > > > ├── possible_values
> > > > └── type
> > > >
> > > > as do other attributes.
> > > >
> > > > Signed-off-by: Luke D. Jones <luke@xxxxxxxxxx>
>
> > > > +static ssize_t attr_int_store(struct kobject *kobj, struct kobj_attribute *attr,
> > > > + const char *buf, size_t count, u32 min, u32 max, u32 *store_value,
> > > > + u32 wmi_dev);
> > > > +
> > > > +static ssize_t int_type_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
> > >
> > > inline missing
> > >
> > > > +{
> > > > + return sysfs_emit(buf, "integer\n");
> > >
> > > Lukas Wunner might have done something to make emitting constant strings
> > > easier, please check out if that's already in mainline.
> >
> > I'm not sure what I'm looking for here. Searching my current pdx86 pull
> > isn't returning anything likely and I can't find anything on lore.
>
> I think that was done mostly outside of pdx86, here it is:
>
> https://lore.kernel.org/all/cover.1713608122.git.lukas@xxxxxxxxx/
>
> So DEVICE_STRING_ATTR_RO() seems the way to go, I think.
Thanks. I did try this but the signature is different to the bulk of what I;ve done already. I don't have the time for it yet and will revisit it later.
>
> > > > + * On success the return value is 0, and the retval is a valid value returned
> > > > + * by the successful WMI function call. An error value is returned only if the
> > > > + * WMI function failed, or if it returns "unsupported" which is typically a 0
> > > > + * (no return, and no 'supported' bit set), or a 0xFFFFFFFE (~1) which if not
> > > > + * caught here can result in unexpected behaviour later.
> > > > + */
> > > > +int asus_wmi_get_devstate_dsts(u32 dev_id, u32 *retval)
> > > > +{
> > > > + int err;
> > > > +
> > > > + err = asus_wmi_evaluate_method(ASUS_WMI_METHODID_DSTS, dev_id, 0, retval);
> > > > + if (err)
> > > > + return err;
> > > > + /* Be explicit about retval */
> > > > + if (*retval == 0xFFFFFFFE || *retval == 0)
> > >
> > > Please name the literals with defines.
> >
> > Should have been ASUS_WMI_UNSUPPORTED_METHOD :)
>
> Is one define enough if you have two ways to indicate "unsupported"?
This function body wasn't correct. It now is and that retval == 0 is removed.
>
> > > > + return -ENODEV;
> > > > +
> > > > + return 0;
> > > > +}
> > > > +EXPORT_SYMBOL_GPL(asus_wmi_get_devstate_dsts);
> > > > +
> > > > +/**
> > > > + * asus_wmi_set_devstate() - Set the WMI function state.
> > > > + * @dev_id: The WMI function to call.
> > > > + * @ctrl_param: The argument to be used for this WMI function.
> > > > + * @retval: A pointer to where to store the value returned from WMI.
> > > > + *
> > > > + * The returned WMI function state if not checked here for error as
> > > > + * asus_wmi_set_devstate() is not called unless first paired with a call to
> > > > + * asus_wmi_get_devstate_dsts() to check that the WMI function is supported.
> > > > + *
> > > > + * On success the return value is 0, and the retval is a valid value returned
> > > > + * by the successful WMI function call. An error value is returned only if the
> > > > + * WMI function failed.
> > > > + */
> > > > +int asus_wmi_set_devstate(u32 dev_id, u32 ctrl_param, u32 *retval)
> > > > {
> > > > return asus_wmi_evaluate_method(ASUS_WMI_METHODID_DEVS, dev_id,
> > > > ctrl_param, retval);
> > > > }
> > > > +EXPORT_SYMBOL_GPL(asus_wmi_set_devstate);
> > >
> > > Namespace exports.
> >
> > I'm sorry, I don't understand.
>
> Use EXPORT_SYMBOL_NS_GPL(). The driver using the functions, will then do
> MODULE_IMPORT_NS() to pair with the namespace defined by the
> EXPORT_SYMBOL_NS_GPL().
Got it. I did wonder about this while reading a few kernel books (rather old now).
>
> --
> i.
>
