power: supply: axp20x_usb_power: Init work before enabling IRQs
diff mbox series

Message ID 20210124152421.39693-1-samuel@sholland.org
State New, archived
Headers show
Series
  • power: supply: axp20x_usb_power: Init work before enabling IRQs
Related show

Commit Message

Samuel Holland Jan. 24, 2021, 3:24 p.m. UTC
The IRQ handler calls mod_delayed_work() on power->vbus_detect. However,
that work item is not initialized until after the IRQs are enabled. If
an IRQ is already pending when the driver is probed, the driver calls
mod_delayed_work() on an uninitialized work item, which causes an oops.

Fixes: bcfb7ae3f50b ("power: supply: axp20x_usb_power: Only poll while offline")
Signed-off-by: Samuel Holland <samuel@sholland.org>
---
 drivers/power/supply/axp20x_usb_power.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Chen-Yu Tsai Jan. 25, 2021, 3:41 a.m. UTC | #1
On Sun, Jan 24, 2021 at 11:24 PM Samuel Holland <samuel@sholland.org> wrote:
>
> The IRQ handler calls mod_delayed_work() on power->vbus_detect. However,
> that work item is not initialized until after the IRQs are enabled. If
> an IRQ is already pending when the driver is probed, the driver calls
> mod_delayed_work() on an uninitialized work item, which causes an oops.
>
> Fixes: bcfb7ae3f50b ("power: supply: axp20x_usb_power: Only poll while offline")
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---
>  drivers/power/supply/axp20x_usb_power.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/power/supply/axp20x_usb_power.c b/drivers/power/supply/axp20x_usb_power.c
> index 20817a49110b..02aba3da271a 100644
> --- a/drivers/power/supply/axp20x_usb_power.c
> +++ b/drivers/power/supply/axp20x_usb_power.c
> @@ -711,20 +711,21 @@ static int axp20x_usb_power_probe(struct platform_device *pdev)
>                              struct_size(power, irqs, axp_data->num_irq_names),
>                              GFP_KERNEL);
>         if (!power)
>                 return -ENOMEM;
>
>         platform_set_drvdata(pdev, power);
>
>         power->axp20x_id = axp_data->axp20x_id;
>         power->regmap = axp20x->regmap;
>         power->num_irqs = axp_data->num_irq_names;
> +       INIT_DELAYED_WORK(&power->vbus_detect, axp20x_usb_power_poll_vbus);

Nit:
Since axp20x_usb_power_poll_vbus() calls power_supply_changed() on
power->supply, it would make more sense to have INIT_DELAYED_WORK()
after devm_power_supply_register(), just so things are ordered correctly.
In practice this has no effect on the end result, since by the time the
interrupts are enabled the power supply would have been registered.

ChenYu

>         if (power->axp20x_id == AXP202_ID) {
>                 /* Enable vbus valid checking */
>                 ret = regmap_update_bits(power->regmap, AXP20X_VBUS_MON,
>                                          AXP20X_VBUS_MON_VBUS_VALID,
>                                          AXP20X_VBUS_MON_VBUS_VALID);
>                 if (ret)
>                         return ret;
>
>                 if (IS_ENABLED(CONFIG_AXP20X_ADC))
> @@ -763,21 +764,20 @@ static int axp20x_usb_power_probe(struct platform_device *pdev)
>                 ret = devm_request_any_context_irq(&pdev->dev, power->irqs[i],
>                                                    axp20x_usb_power_irq, 0,
>                                                    DRVNAME, power);
>                 if (ret < 0) {
>                         dev_err(&pdev->dev, "Error requesting %s IRQ: %d\n",
>                                 axp_data->irq_names[i], ret);
>                         return ret;
>                 }
>         }
>
> -       INIT_DELAYED_WORK(&power->vbus_detect, axp20x_usb_power_poll_vbus);
>         if (axp20x_usb_vbus_needs_polling(power))
>                 queue_delayed_work(system_power_efficient_wq, &power->vbus_detect, 0);
>
>         return 0;
>  }
>
>  static int axp20x_usb_power_remove(struct platform_device *pdev)
>  {
>         struct axp20x_usb_power *power = platform_get_drvdata(pdev);
>
> --
> 2.26.2
>
Sebastian Reichel Jan. 28, 2021, 12:36 a.m. UTC | #2
Hi,

On Sun, Jan 24, 2021 at 09:24:21AM -0600, Samuel Holland wrote:
> The IRQ handler calls mod_delayed_work() on power->vbus_detect. However,
> that work item is not initialized until after the IRQs are enabled. If
> an IRQ is already pending when the driver is probed, the driver calls
> mod_delayed_work() on an uninitialized work item, which causes an oops.
> 
> Fixes: bcfb7ae3f50b ("power: supply: axp20x_usb_power: Only poll while offline")
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---

Thanks, queued.

-- Sebastian

>  drivers/power/supply/axp20x_usb_power.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/power/supply/axp20x_usb_power.c b/drivers/power/supply/axp20x_usb_power.c
> index 20817a49110b..02aba3da271a 100644
> --- a/drivers/power/supply/axp20x_usb_power.c
> +++ b/drivers/power/supply/axp20x_usb_power.c
> @@ -711,20 +711,21 @@ static int axp20x_usb_power_probe(struct platform_device *pdev)
>  			     struct_size(power, irqs, axp_data->num_irq_names),
>  			     GFP_KERNEL);
>  	if (!power)
>  		return -ENOMEM;
>  
>  	platform_set_drvdata(pdev, power);
>  
>  	power->axp20x_id = axp_data->axp20x_id;
>  	power->regmap = axp20x->regmap;
>  	power->num_irqs = axp_data->num_irq_names;
> +	INIT_DELAYED_WORK(&power->vbus_detect, axp20x_usb_power_poll_vbus);
>  
>  	if (power->axp20x_id == AXP202_ID) {
>  		/* Enable vbus valid checking */
>  		ret = regmap_update_bits(power->regmap, AXP20X_VBUS_MON,
>  					 AXP20X_VBUS_MON_VBUS_VALID,
>  					 AXP20X_VBUS_MON_VBUS_VALID);
>  		if (ret)
>  			return ret;
>  
>  		if (IS_ENABLED(CONFIG_AXP20X_ADC))
> @@ -763,21 +764,20 @@ static int axp20x_usb_power_probe(struct platform_device *pdev)
>  		ret = devm_request_any_context_irq(&pdev->dev, power->irqs[i],
>  						   axp20x_usb_power_irq, 0,
>  						   DRVNAME, power);
>  		if (ret < 0) {
>  			dev_err(&pdev->dev, "Error requesting %s IRQ: %d\n",
>  				axp_data->irq_names[i], ret);
>  			return ret;
>  		}
>  	}
>  
> -	INIT_DELAYED_WORK(&power->vbus_detect, axp20x_usb_power_poll_vbus);
>  	if (axp20x_usb_vbus_needs_polling(power))
>  		queue_delayed_work(system_power_efficient_wq, &power->vbus_detect, 0);
>  
>  	return 0;
>  }
>  
>  static int axp20x_usb_power_remove(struct platform_device *pdev)
>  {
>  	struct axp20x_usb_power *power = platform_get_drvdata(pdev);
>  
> -- 
> 2.26.2
>

Patch
diff mbox series

diff --git a/drivers/power/supply/axp20x_usb_power.c b/drivers/power/supply/axp20x_usb_power.c
index 20817a49110b..02aba3da271a 100644
--- a/drivers/power/supply/axp20x_usb_power.c
+++ b/drivers/power/supply/axp20x_usb_power.c
@@ -711,20 +711,21 @@  static int axp20x_usb_power_probe(struct platform_device *pdev)
 			     struct_size(power, irqs, axp_data->num_irq_names),
 			     GFP_KERNEL);
 	if (!power)
 		return -ENOMEM;
 
 	platform_set_drvdata(pdev, power);
 
 	power->axp20x_id = axp_data->axp20x_id;
 	power->regmap = axp20x->regmap;
 	power->num_irqs = axp_data->num_irq_names;
+	INIT_DELAYED_WORK(&power->vbus_detect, axp20x_usb_power_poll_vbus);
 
 	if (power->axp20x_id == AXP202_ID) {
 		/* Enable vbus valid checking */
 		ret = regmap_update_bits(power->regmap, AXP20X_VBUS_MON,
 					 AXP20X_VBUS_MON_VBUS_VALID,
 					 AXP20X_VBUS_MON_VBUS_VALID);
 		if (ret)
 			return ret;
 
 		if (IS_ENABLED(CONFIG_AXP20X_ADC))
@@ -763,21 +764,20 @@  static int axp20x_usb_power_probe(struct platform_device *pdev)
 		ret = devm_request_any_context_irq(&pdev->dev, power->irqs[i],
 						   axp20x_usb_power_irq, 0,
 						   DRVNAME, power);
 		if (ret < 0) {
 			dev_err(&pdev->dev, "Error requesting %s IRQ: %d\n",
 				axp_data->irq_names[i], ret);
 			return ret;
 		}
 	}
 
-	INIT_DELAYED_WORK(&power->vbus_detect, axp20x_usb_power_poll_vbus);
 	if (axp20x_usb_vbus_needs_polling(power))
 		queue_delayed_work(system_power_efficient_wq, &power->vbus_detect, 0);
 
 	return 0;
 }
 
 static int axp20x_usb_power_remove(struct platform_device *pdev)
 {
 	struct axp20x_usb_power *power = platform_get_drvdata(pdev);