Hi,
On 5/24/23 05:51, bingbu.cao@xxxxxxxxx wrote:
> From: Hao Yao <hao.yao@xxxxxxxxx>
>
> When int3472 is loaded before GPIO driver, acpi_get_and_request_gpiod()
> failed but the returned gpio descriptor is not NULL, it will cause panic
> in later gpiod_put(), so set the gpio_desc to NULL in register error
> handling to avoid such crash.
>
> Signed-off-by: Hao Yao <hao.yao@xxxxxxxxx>
> Signed-off-by: Bingbu Cao <bingbu.cao@xxxxxxxxx>
For the clk-enable GPIO this is not really necessary:
void skl_int3472_unregister_clock(struct int3472_discrete_device *int3472)
{
if (!int3472->clock.cl)
return;
...
gpiod_put(int3472->clock.ena_gpio);
}
The int3472->clock.cl check causes the gpiod_put() to never get called.
But setting both GPIOs to NULL when we fail to get them is consistent,
so I have taken this as is:
Thank you for your patch, I've applied this patch to my fixes
branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=fixes
I will include this patch in my next fixes pull-req to Linus
for the current kernel development cycle.
Regards,
Hans
> ---
> .../x86/intel/int3472/clk_and_regulator.c | 16 +++++++++++-----
> 1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/platform/x86/intel/int3472/clk_and_regulator.c b/drivers/platform/x86/intel/int3472/clk_and_regulator.c
> index 1086c3d83494..d1088be5af78 100644
> --- a/drivers/platform/x86/intel/int3472/clk_and_regulator.c
> +++ b/drivers/platform/x86/intel/int3472/clk_and_regulator.c
> @@ -101,9 +101,12 @@ int skl_int3472_register_clock(struct int3472_discrete_device *int3472,
>
> int3472->clock.ena_gpio = acpi_get_and_request_gpiod(path, agpio->pin_table[0],
> "int3472,clk-enable");
> - if (IS_ERR(int3472->clock.ena_gpio))
> - return dev_err_probe(int3472->dev, PTR_ERR(int3472->clock.ena_gpio),
> - "getting clk-enable GPIO\n");
> + if (IS_ERR(int3472->clock.ena_gpio)) {
> + ret = PTR_ERR(int3472->clock.ena_gpio);
> + int3472->clock.ena_gpio = NULL;
> + return dev_err_probe(int3472->dev, ret,
> + "failed to get gpio for clock\n");
> + }
>
> if (polarity == GPIO_ACTIVE_LOW)
> gpiod_toggle_active_low(int3472->clock.ena_gpio);
> @@ -199,8 +202,11 @@ int skl_int3472_register_regulator(struct int3472_discrete_device *int3472,
> int3472->regulator.gpio = acpi_get_and_request_gpiod(path, agpio->pin_table[0],
> "int3472,regulator");
> if (IS_ERR(int3472->regulator.gpio)) {
> - dev_err(int3472->dev, "Failed to get regulator GPIO line\n");
> - return PTR_ERR(int3472->regulator.gpio);
> + ret = PTR_ERR(int3472->regulator.gpio);
> + int3472->regulator.gpio = NULL;
> +
> + return dev_err_probe(int3472->dev, ret,
> + "failed to get regulator gpio\n");
> }
>
> /* Ensure the pin is in output mode and non-active state */
