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

[Werner Sembach <wse@tuxedocomputers.com>]

Am 18.11.25 um 14:35 schrieb Armin Wolf:
> 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.
Sounds like it would work for my usecase
>
> I will send you the patch tomorrow.
Thx
>
> 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;
>>