| Message ID | 20221124200007.390901-2-hdegoede@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | platform/x86: int3472/discrete: Make it work with IPU6 | expand |
On Thu, Nov 24, 2022 at 10:20 PM Hans de Goede <hdegoede@redhat.com> wrote: > On 11/24/22 21:09, Andy Shevchenko wrote: > > On Thu, Nov 24, 2022 at 10:00 PM Hans de Goede <hdegoede@redhat.com> wrote: ... > >> +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"; > > > > default: > > return "unknown"; > > > > ? > > > >> + } > >> + > >> + return "unknown"; > >> +} > > > > In the passed some compiler versions complained about the non-void > function not ending with a return statement. > > I guess I can give your variant a try (I agree it is more readable) > and of we get compiler warnings we can switch back. > > I'll fix this up in the next version or when merging this, > depending on further feedback on the series. I believe it's not the case for a long time. (Esp. when the kernel requires GCC 5.1 as bare minimum). We have a lot of (relatively) new code that uses switch-cases like I suggested and I have heard none of the complains from anybody, including all CIs crawling through the kernel code.
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: >
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:
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> --- drivers/platform/x86/intel/int3472/discrete.c | 31 ++++++++++++++----- 1 file changed, 23 insertions(+), 8 deletions(-)