Re: [PATCH 1/3] platform/x86: int3472/discrete: Refactor GPIO to sensor mapping

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 11/25/22 17:00, Dan Scally wrote:
> Hi Hans
> 
> On 24/11/2022 20:00, Hans de Goede wrote:
>> Make the GPIO to sensor mapping more generic and fold the
>> INT3472_GPIO_TYPE_RESET and INT3472_GPIO_TYPE_POWERDOWN cases into
>> a single generic case.
>>
>> This is a preparation patch for further GPIO mapping changes.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
> 
> 
> Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
> 
> Tested-by: Daniel Scally <dan.scally@ideasonboard.com>

Thank you.

Note I have made some (not insignificant) changes to this patch for the v2
series which I'm working on, so I have decided to not add these tags
because of the changes.

Regards,

Hans



> 
>>   drivers/platform/x86/intel/int3472/discrete.c | 31 ++++++++++++++-----
>>   1 file changed, 23 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c
>> index 974a132db651..bc6c62f3f3bf 100644
>> --- a/drivers/platform/x86/intel/int3472/discrete.c
>> +++ b/drivers/platform/x86/intel/int3472/discrete.c
>> @@ -184,6 +184,24 @@ static int skl_int3472_map_gpio_to_clk(struct int3472_discrete_device *int3472,
>>       return 0;
>>   }
>>   +static const char *int3472_dsm_type_to_func(u8 type)
>> +{
>> +    switch (type) {
>> +    case INT3472_GPIO_TYPE_RESET:
>> +        return "reset";
>> +    case INT3472_GPIO_TYPE_POWERDOWN:
>> +        return "powerdown";
>> +    case INT3472_GPIO_TYPE_CLK_ENABLE:
>> +        return "clken";
>> +    case INT3472_GPIO_TYPE_PRIVACY_LED:
>> +        return "pled";
>> +    case INT3472_GPIO_TYPE_POWER_ENABLE:
>> +        return "power-enable";
>> +    }
>> +
>> +    return "unknown";
>> +}
>> +
>>   /**
>>    * skl_int3472_handle_gpio_resources: Map PMIC resources to consuming sensor
>>    * @ares: A pointer to a &struct acpi_resource
>> @@ -223,6 +241,7 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
>>       struct acpi_resource_gpio *agpio;
>>       union acpi_object *obj;
>>       const char *err_msg;
>> +    const char *func;
>>       int ret;
>>       u8 type;
>>   @@ -246,19 +265,15 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
>>         type = obj->integer.value & 0xff;
>>   +    func = int3472_dsm_type_to_func(type);
>> +
>>       switch (type) {
>>       case INT3472_GPIO_TYPE_RESET:
>> -        ret = skl_int3472_map_gpio_to_sensor(int3472, agpio, "reset",
>> -                             GPIO_ACTIVE_LOW);
>> -        if (ret)
>> -            err_msg = "Failed to map reset pin to sensor\n";
>> -
>> -        break;
>>       case INT3472_GPIO_TYPE_POWERDOWN:
>> -        ret = skl_int3472_map_gpio_to_sensor(int3472, agpio, "powerdown",
>> +        ret = skl_int3472_map_gpio_to_sensor(int3472, agpio, func,
>>                                GPIO_ACTIVE_LOW);
>>           if (ret)
>> -            err_msg = "Failed to map powerdown pin to sensor\n";
>> +            err_msg = "Failed to map GPIO pin to sensor\n";
>>             break;
>>       case INT3472_GPIO_TYPE_CLK_ENABLE:
>