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.
> > > + * 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"?
> > > + 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().
--
i.
