Re: [PATCH 4/6] platform/x86/uniwill: Make uniwill_dmi_table accessible in probe

[Armin Wolf <W_Armin@gmx.de>]

Am 18.11.25 um 14:01 schrieb Werner Sembach:

>
> Am 18.11.25 um 12:16 schrieb Armin Wolf:
>> Am 17.11.25 um 14:24 schrieb Werner Sembach:
>>
>>> Move uniwill_dmi_table up and remove __intconst to make it also 
>>> accessible
>>> in the probe function.
>>
>> NAK, i expect the DMI table to become very large in the future, so 
>> not marking it
>> as __initconst will waste a sizeable amount of memory.
>>
>> Luckily i am also currently working on extending the DMI table to 
>> contain additional
>> configuration data like power limits and callbacks for 
>> device-specific initialization.
>>
>> I can share the patch with you if you want. It would allow us to 
>> discard the DMI table
>> after module initialization while still allowing for device-specific 
>> initialization
>> callbacks.
> this callback needs a way to probe for presence of a Nvidia card e.g. 
> by reading the ec

The patch i am talking about allows DMI table entries to provide a "device descriptor" that
basically is a struct consisting of vendor-specific data and a callback that is executed by
the platform driver when probing the EC. This would allow you to read the special register.
Said device descriptor will replace the supported_features variable.

I will send you the patch tomorrow.

Thanks,
Armin Wolf

>> Thanks,
>> Armin Wolf
>>
>>> Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
>>> ---
>>>   drivers/platform/x86/uniwill/uniwill-acpi.c | 344 
>>> ++++++++++----------
>>>   1 file changed, 172 insertions(+), 172 deletions(-)
>>>
>>> diff --git a/drivers/platform/x86/uniwill/uniwill-acpi.c 
>>> b/drivers/platform/x86/uniwill/uniwill-acpi.c
>>> index de3417d9d1ac0..9412783698685 100644
>>> --- a/drivers/platform/x86/uniwill/uniwill-acpi.c
>>> +++ b/drivers/platform/x86/uniwill/uniwill-acpi.c
>>> @@ -1405,178 +1405,7 @@ static int uniwill_ec_init(struct 
>>> uniwill_data *data)
>>>       return devm_add_action_or_reset(data->dev, 
>>> uniwill_disable_manual_control, data);
>>>   }
>>>   -static int uniwill_probe(struct platform_device *pdev)
>>> -{
>>> -    struct uniwill_data *data;
>>> -    struct regmap *regmap;
>>> -    acpi_handle handle;
>>> -    int ret;
>>> -
>>> -    handle = ACPI_HANDLE(&pdev->dev);
>>> -    if (!handle)
>>> -        return -ENODEV;
>>> -
>>> -    data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
>>> -    if (!data)
>>> -        return -ENOMEM;
>>> -
>>> -    data->dev = &pdev->dev;
>>> -    data->handle = handle;
>>> -    platform_set_drvdata(pdev, data);
>>> -
>>> -    regmap = devm_regmap_init(&pdev->dev, &uniwill_ec_bus, data, 
>>> &uniwill_ec_config);
>>> -    if (IS_ERR(regmap))
>>> -        return PTR_ERR(regmap);
>>> -
>>> -    data->regmap = regmap;
>>> -    ret = devm_mutex_init(&pdev->dev, &data->super_key_lock);
>>> -    if (ret < 0)
>>> -        return ret;
>>> -
>>> -    ret = uniwill_ec_init(data);
>>> -    if (ret < 0)
>>> -        return ret;
>>> -
>>> -    ret = uniwill_battery_init(data);
>>> -    if (ret < 0)
>>> -        return ret;
>>> -
>>> -    ret = uniwill_led_init(data);
>>> -    if (ret < 0)
>>> -        return ret;
>>> -
>>> -    ret = uniwill_hwmon_init(data);
>>> -    if (ret < 0)
>>> -        return ret;
>>> -
>>> -    ret = uniwill_nvidia_ctgp_init(data);
>>> -    if (ret < 0)
>>> -        return ret;
>>> -
>>> -    return uniwill_input_init(data);
>>> -}
>>> -
>>> -static void uniwill_shutdown(struct platform_device *pdev)
>>> -{
>>> -    struct uniwill_data *data = platform_get_drvdata(pdev);
>>> -
>>> -    regmap_clear_bits(data->regmap, EC_ADDR_AP_OEM, 
>>> ENABLE_MANUAL_CTRL);
>>> -}
>>> -
>>> -static int uniwill_suspend_keyboard(struct uniwill_data *data)
>>> -{
>>> -    if (!(supported_features & UNIWILL_FEATURE_SUPER_KEY_TOGGLE))
>>> -        return 0;
>>> -
>>> -    /*
>>> -     * The EC_ADDR_SWITCH_STATUS is marked as volatile, so we have 
>>> to restore it
>>> -     * ourselves.
>>> -     */
>>> -    return regmap_read(data->regmap, EC_ADDR_SWITCH_STATUS, 
>>> &data->last_switch_status);
>>> -}
>>> -
>>> -static int uniwill_suspend_battery(struct uniwill_data *data)
>>> -{
>>> -    if (!(supported_features & UNIWILL_FEATURE_BATTERY))
>>> -        return 0;
>>> -
>>> -    /*
>>> -     * Save the current charge limit in order to restore it during 
>>> resume.
>>> -     * We cannot use the regmap code for that since this register 
>>> needs to
>>> -     * be declared as volatile due to CHARGE_CTRL_REACHED.
>>> -     */
>>> -    return regmap_read(data->regmap, EC_ADDR_CHARGE_CTRL, 
>>> &data->last_charge_ctrl);
>>> -}
>>> -
>>> -static int uniwill_suspend(struct device *dev)
>>> -{
>>> -    struct uniwill_data *data = dev_get_drvdata(dev);
>>> -    int ret;
>>> -
>>> -    ret = uniwill_suspend_keyboard(data);
>>> -    if (ret < 0)
>>> -        return ret;
>>> -
>>> -    ret = uniwill_suspend_battery(data);
>>> -    if (ret < 0)
>>> -        return ret;
>>> -
>>> -    regcache_cache_only(data->regmap, true);
>>> -    regcache_mark_dirty(data->regmap);
>>> -
>>> -    return 0;
>>> -}
>>> -
>>> -static int uniwill_resume_keyboard(struct uniwill_data *data)
>>> -{
>>> -    unsigned int value;
>>> -    int ret;
>>> -
>>> -    if (!(supported_features & UNIWILL_FEATURE_SUPER_KEY_TOGGLE))
>>> -        return 0;
>>> -
>>> -    ret = regmap_read(data->regmap, EC_ADDR_SWITCH_STATUS, &value);
>>> -    if (ret < 0)
>>> -        return ret;
>>> -
>>> -    if ((data->last_switch_status & SUPER_KEY_LOCK_STATUS) == 
>>> (value & SUPER_KEY_LOCK_STATUS))
>>> -        return 0;
>>> -
>>> -    return regmap_write_bits(data->regmap, EC_ADDR_TRIGGER, 
>>> TRIGGER_SUPER_KEY_LOCK,
>>> -                 TRIGGER_SUPER_KEY_LOCK);
>>> -}
>>> -
>>> -static int uniwill_resume_battery(struct uniwill_data *data)
>>> -{
>>> -    if (!(supported_features & UNIWILL_FEATURE_BATTERY))
>>> -        return 0;
>>> -
>>> -    return regmap_update_bits(data->regmap, EC_ADDR_CHARGE_CTRL, 
>>> CHARGE_CTRL_MASK,
>>> -                  data->last_charge_ctrl);
>>> -}
>>> -
>>> -static int uniwill_resume(struct device *dev)
>>> -{
>>> -    struct uniwill_data *data = dev_get_drvdata(dev);
>>> -    int ret;
>>> -
>>> -    regcache_cache_only(data->regmap, false);
>>> -
>>> -    ret = regcache_sync(data->regmap);
>>> -    if (ret < 0)
>>> -        return ret;
>>> -
>>> -    ret = uniwill_resume_keyboard(data);
>>> -    if (ret < 0)
>>> -        return ret;
>>> -
>>> -    return uniwill_resume_battery(data);
>>> -}
>>> -
>>> -static DEFINE_SIMPLE_DEV_PM_OPS(uniwill_pm_ops, uniwill_suspend, 
>>> uniwill_resume);
>>> -
>>> -/*
>>> - * We only use the DMI table for auoloading because the ACPI device 
>>> itself
>>> - * does not guarantee that the underlying EC implementation is 
>>> supported.
>>> - */
>>> -static const struct acpi_device_id uniwill_id_table[] = {
>>> -    { "INOU0000" },
>>> -    { },
>>> -};
>>> -
>>> -static struct platform_driver uniwill_driver = {
>>> -    .driver = {
>>> -        .name = DRIVER_NAME,
>>> -        .dev_groups = uniwill_groups,
>>> -        .probe_type = PROBE_PREFER_ASYNCHRONOUS,
>>> -        .acpi_match_table = uniwill_id_table,
>>> -        .pm = pm_sleep_ptr(&uniwill_pm_ops),
>>> -    },
>>> -    .probe = uniwill_probe,
>>> -    .shutdown = uniwill_shutdown,
>>> -};
>>> -
>>> -static const struct dmi_system_id uniwill_dmi_table[] __initconst = {
>>> +static const struct dmi_system_id uniwill_dmi_table[] = {
>>>       {
>>>           .ident = "XMG FUSION 15",
>>>           .matches = {
>>> @@ -1936,6 +1765,177 @@ static const struct dmi_system_id 
>>> uniwill_dmi_table[] __initconst = {
>>>   };
>>>   MODULE_DEVICE_TABLE(dmi, uniwill_dmi_table);
>>>   +static int uniwill_probe(struct platform_device *pdev)
>>> +{
>>> +    struct uniwill_data *data;
>>> +    struct regmap *regmap;
>>> +    acpi_handle handle;
>>> +    int ret;
>>> +
>>> +    handle = ACPI_HANDLE(&pdev->dev);
>>> +    if (!handle)
>>> +        return -ENODEV;
>>> +
>>> +    data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
>>> +    if (!data)
>>> +        return -ENOMEM;
>>> +
>>> +    data->dev = &pdev->dev;
>>> +    data->handle = handle;
>>> +    platform_set_drvdata(pdev, data);
>>> +
>>> +    regmap = devm_regmap_init(&pdev->dev, &uniwill_ec_bus, data, 
>>> &uniwill_ec_config);
>>> +    if (IS_ERR(regmap))
>>> +        return PTR_ERR(regmap);
>>> +
>>> +    data->regmap = regmap;
>>> +    ret = devm_mutex_init(&pdev->dev, &data->super_key_lock);
>>> +    if (ret < 0)
>>> +        return ret;
>>> +
>>> +    ret = uniwill_ec_init(data);
>>> +    if (ret < 0)
>>> +        return ret;
>>> +
>>> +    ret = uniwill_battery_init(data);
>>> +    if (ret < 0)
>>> +        return ret;
>>> +
>>> +    ret = uniwill_led_init(data);
>>> +    if (ret < 0)
>>> +        return ret;
>>> +
>>> +    ret = uniwill_hwmon_init(data);
>>> +    if (ret < 0)
>>> +        return ret;
>>> +
>>> +    ret = uniwill_nvidia_ctgp_init(data);
>>> +    if (ret < 0)
>>> +        return ret;
>>> +
>>> +    return uniwill_input_init(data);
>>> +}
>>> +
>>> +static void uniwill_shutdown(struct platform_device *pdev)
>>> +{
>>> +    struct uniwill_data *data = platform_get_drvdata(pdev);
>>> +
>>> +    regmap_clear_bits(data->regmap, EC_ADDR_AP_OEM, 
>>> ENABLE_MANUAL_CTRL);
>>> +}
>>> +
>>> +static int uniwill_suspend_keyboard(struct uniwill_data *data)
>>> +{
>>> +    if (!(supported_features & UNIWILL_FEATURE_SUPER_KEY_TOGGLE))
>>> +        return 0;
>>> +
>>> +    /*
>>> +     * The EC_ADDR_SWITCH_STATUS is marked as volatile, so we have 
>>> to restore it
>>> +     * ourselves.
>>> +     */
>>> +    return regmap_read(data->regmap, EC_ADDR_SWITCH_STATUS, 
>>> &data->last_switch_status);
>>> +}
>>> +
>>> +static int uniwill_suspend_battery(struct uniwill_data *data)
>>> +{
>>> +    if (!(supported_features & UNIWILL_FEATURE_BATTERY))
>>> +        return 0;
>>> +
>>> +    /*
>>> +     * Save the current charge limit in order to restore it during 
>>> resume.
>>> +     * We cannot use the regmap code for that since this register 
>>> needs to
>>> +     * be declared as volatile due to CHARGE_CTRL_REACHED.
>>> +     */
>>> +    return regmap_read(data->regmap, EC_ADDR_CHARGE_CTRL, 
>>> &data->last_charge_ctrl);
>>> +}
>>> +
>>> +static int uniwill_suspend(struct device *dev)
>>> +{
>>> +    struct uniwill_data *data = dev_get_drvdata(dev);
>>> +    int ret;
>>> +
>>> +    ret = uniwill_suspend_keyboard(data);
>>> +    if (ret < 0)
>>> +        return ret;
>>> +
>>> +    ret = uniwill_suspend_battery(data);
>>> +    if (ret < 0)
>>> +        return ret;
>>> +
>>> +    regcache_cache_only(data->regmap, true);
>>> +    regcache_mark_dirty(data->regmap);
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +static int uniwill_resume_keyboard(struct uniwill_data *data)
>>> +{
>>> +    unsigned int value;
>>> +    int ret;
>>> +
>>> +    if (!(supported_features & UNIWILL_FEATURE_SUPER_KEY_TOGGLE))
>>> +        return 0;
>>> +
>>> +    ret = regmap_read(data->regmap, EC_ADDR_SWITCH_STATUS, &value);
>>> +    if (ret < 0)
>>> +        return ret;
>>> +
>>> +    if ((data->last_switch_status & SUPER_KEY_LOCK_STATUS) == 
>>> (value & SUPER_KEY_LOCK_STATUS))
>>> +        return 0;
>>> +
>>> +    return regmap_write_bits(data->regmap, EC_ADDR_TRIGGER, 
>>> TRIGGER_SUPER_KEY_LOCK,
>>> +                 TRIGGER_SUPER_KEY_LOCK);
>>> +}
>>> +
>>> +static int uniwill_resume_battery(struct uniwill_data *data)
>>> +{
>>> +    if (!(supported_features & UNIWILL_FEATURE_BATTERY))
>>> +        return 0;
>>> +
>>> +    return regmap_update_bits(data->regmap, EC_ADDR_CHARGE_CTRL, 
>>> CHARGE_CTRL_MASK,
>>> +                  data->last_charge_ctrl);
>>> +}
>>> +
>>> +static int uniwill_resume(struct device *dev)
>>> +{
>>> +    struct uniwill_data *data = dev_get_drvdata(dev);
>>> +    int ret;
>>> +
>>> +    regcache_cache_only(data->regmap, false);
>>> +
>>> +    ret = regcache_sync(data->regmap);
>>> +    if (ret < 0)
>>> +        return ret;
>>> +
>>> +    ret = uniwill_resume_keyboard(data);
>>> +    if (ret < 0)
>>> +        return ret;
>>> +
>>> +    return uniwill_resume_battery(data);
>>> +}
>>> +
>>> +static DEFINE_SIMPLE_DEV_PM_OPS(uniwill_pm_ops, uniwill_suspend, 
>>> uniwill_resume);
>>> +
>>> +/*
>>> + * We only use the DMI table for auoloading because the ACPI device 
>>> itself
>>> + * does not guarantee that the underlying EC implementation is 
>>> supported.
>>> + */
>>> +static const struct acpi_device_id uniwill_id_table[] = {
>>> +    { "INOU0000" },
>>> +    { },
>>> +};
>>> +
>>> +static struct platform_driver uniwill_driver = {
>>> +    .driver = {
>>> +        .name = DRIVER_NAME,
>>> +        .dev_groups = uniwill_groups,
>>> +        .probe_type = PROBE_PREFER_ASYNCHRONOUS,
>>> +        .acpi_match_table = uniwill_id_table,
>>> +        .pm = pm_sleep_ptr(&uniwill_pm_ops),
>>> +    },
>>> +    .probe = uniwill_probe,
>>> +    .shutdown = uniwill_shutdown,
>>> +};
>>> +
>>>   static int __init uniwill_init(void)
>>>   {
>>>       const struct dmi_system_id *id;
>