Drive-by changes to axp20x drivers
cover Drive-by changes to axp20x drivers
Hello,
while trying to help a Debian user debugging why USB doesn't work in the installer on a bananapi, I discovered these patch opportunities. They are only build tested.
The first patch touches a different subsystem than the other three, but given that maintainership overlaps and the first two patches are identical in spirit, I still created a single series only. The two subsets are orthogonal, so can be picked up individually by their respective subsystem maintainers (and please do so assuming you're happy with the patches).
Best regards
Uwe
Uwe Kleine-König (4):
pinctrl: axp209: Drop check for disabled devices
power: supply: axp20x*: Drop check for disabled devices
power: supply: axp20x_usb: Introduce a helper variable for &pdev->dev
power: supply: axp20x_usb: Improve probe error reporting
drivers/pinctrl/pinctrl-axp209.c | 3 -- drivers/power/supply/axp20x_ac_power.c | 3 -- drivers/power/supply/axp20x_battery.c | 3 -- drivers/power/supply/axp20x_usb_power.c | 50 ++++++++++++------------- 4 files changed, 25 insertions(+), 34 deletions(-)
base-commit: 3f2425f5b5bbbdd991ca9cdfd5502e68d8895998
Signature · 1 line
2.47.3
PATCH 1/4 pinctrl: axp209: Drop check for disabled devices
Since commit 6b5c350648b8 ("mfd: mfd-core: Honour Device Tree's request to disable a child-device") the mfd core checks for disabled devices, so there is no use in doing that again in the mfd child driver.
Drop the check.
--- drivers/pinctrl/pinctrl-axp209.c | 3 --- 1 file changed, 3 deletions(-)
drivers/pinctrl/pinctrl-axp209.c +0 -3
@@ -442,9 +442,6 @@ static int axp20x_pctl_probe(struct platform_device *pdev)struct pinctrl_desc *pctrl_desc;int ret; -if (!of_device_is_available (pdev->dev.of_node)) -return -ENODEV ; -if (!axp20x) {dev_err (&pdev->dev,"Parent drvdata not set\n" );return -EINVAL ;
Signature · 1 line
2.47.3
Linus Walleij
maintainerRe: [PATCH 1/4] pinctrl: axp209: Drop check for disabled devices
Quoted 7 lines · Uwe Kleine-König
Since commit
6b5c350648b8("mfd: mfd-core: Honour Device Tree's request to disable a child-device") the mfd core checks for disabled devices, so there is no use in doing that again in the mfd child driver.Drop the check.
Signed-off-by Uwe Kleine-König <[email protected]>
This patch 1/4 applied to the pin control tree.
Yours,
Linus Walleij
PATCH 2/4 power: supply: axp20x*: Drop check for disabled devices
In reply to Uwe Kleine-KönigSince commit 6b5c350648b8 ("mfd: mfd-core: Honour Device Tree's request to disable a child-device") the mfd core checks for disabled devices, so there is no use in doing that again in the mfd child driver.
Drop the check in the two power drivers.
--- drivers/power/supply/axp20x_ac_power.c | 3 --- drivers/power/supply/axp20x_battery.c | 3 --- drivers/power/supply/axp20x_usb_power.c | 3 --- 3 files changed, 9 deletions(-)
drivers/power/supply/axp20x_ac_power.c +0 -3
@@ -326,9 +326,6 @@ static int axp20x_ac_power_probe(struct platform_device *pdev)const struct axp_data *axp_data;int i, irq, ret; -if (!of_device_is_available (pdev->dev.of_node)) -return -ENODEV ; -if (!axp20x) {dev_err (&pdev->dev,"Parent drvdata not set\n" );return -EINVAL ;
drivers/power/supply/axp20x_battery.c +0 -3
@@ -1097,9 +1097,6 @@ static int axp20x_power_probe(struct platform_device *pdev)struct device *dev = &pdev->dev;int ret; -if (!of_device_is_available (pdev->dev.of_node)) -return -ENODEV ; - axp20x_batt =devm_kzalloc (&pdev->dev,sizeof (*axp20x_batt),GFP_KERNEL );if (!axp20x_batt)
drivers/power/supply/axp20x_usb_power.c +0 -3
@@ -924,9 +924,6 @@ static int axp20x_usb_power_probe(struct platform_device *pdev)const struct axp_data *axp_data;int i, irq, ret; -if (!of_device_is_available (pdev->dev.of_node)) -return -ENODEV ; -if (!axp20x) {dev_err (&pdev->dev,"Parent drvdata not set\n" );return -EINVAL ;
Signature · 1 line
2.47.3
PATCH 3/4 power: supply: axp20x_usb: Introduce a helper variable for &pdev->dev
In reply to Uwe Kleine-KönigWith the next commit adding several new usages of &pdev->dev, a helper variable is well justified. To not clutter that commit, split out the introduction of the helper out in this separate change to simplify review.
--- drivers/power/supply/axp20x_usb_power.c | 34 ++++++++++++------------- 1 file changed, 17 insertions(+), 17 deletions(-)
drivers/power/supply/axp20x_usb_power.c +17 -17
@@ -918,70 +918,70 @@ static void axp20x_usb_power_parse_dt(struct device *dev,static int axp20x_usb_power_probe (struct platform_device *pdev) { -struct axp20x_dev *axp20x =dev_get_drvdata (pdev->dev.parent); +struct device *dev = &pdev->dev; +struct axp20x_dev *axp20x =dev_get_drvdata (dev->parent);struct power_supply_config psy_cfg = {};struct axp20x_usb_power *power;const struct axp_data *axp_data;int i, irq, ret;if (!axp20x) { -dev_err (&pdev->dev,"Parent drvdata not set\n" ); +dev_err (dev,"Parent drvdata not set\n" );return -EINVAL ; } - axp_data =of_device_get_match_data (&pdev->dev); + axp_data =of_device_get_match_data (dev); - power =devm_kzalloc (&pdev->dev, -struct_size (power, irqs, axp_data->num_irq_names), + power =devm_kzalloc (dev,struct_size (power, irqs, axp_data->num_irq_names),GFP_KERNEL );if (!power)return -ENOMEM ;platform_set_drvdata (pdev, power); - power->dev = &pdev->dev; + power->dev = dev; power->axp_data = axp_data; power->regmap = axp20x->regmap; power->num_irqs = axp_data->num_irq_names; - power->curr_lim_fld =devm_regmap_field_alloc (&pdev->dev, power->regmap, + power->curr_lim_fld =devm_regmap_field_alloc (dev, power->regmap, axp_data->curr_lim_fld);if (IS_ERR (power->curr_lim_fld))return PTR_ERR (power->curr_lim_fld); -axp20x_usb_power_parse_dt (&pdev->dev, power); +axp20x_usb_power_parse_dt (dev, power); - ret =axp20x_regmap_field_alloc_optional (&pdev->dev, power->regmap, + ret =axp20x_regmap_field_alloc_optional (dev, power->regmap, axp_data->vbus_valid_bit, &power->vbus_valid_bit);if (ret)return ret; - ret =axp20x_regmap_field_alloc_optional (&pdev->dev, power->regmap, + ret =axp20x_regmap_field_alloc_optional (dev, power->regmap, axp_data->vbus_mon_bit, &power->vbus_mon_bit);if (ret)return ret; - ret =axp20x_regmap_field_alloc_optional (&pdev->dev, power->regmap, + ret =axp20x_regmap_field_alloc_optional (dev, power->regmap, axp_data->usb_bc_en_bit, &power->usb_bc_en_bit);if (ret)return ret; - ret =axp20x_regmap_field_alloc_optional (&pdev->dev, power->regmap, + ret =axp20x_regmap_field_alloc_optional (dev, power->regmap, axp_data->usb_bc_det_fld, &power->usb_bc_det_fld);if (ret)return ret; - ret =axp20x_regmap_field_alloc_optional (&pdev->dev, power->regmap, + ret =axp20x_regmap_field_alloc_optional (dev, power->regmap, axp_data->vbus_disable_bit, &power->vbus_disable_bit);if (ret)return ret; - ret =devm_delayed_work_autocancel (&pdev->dev, &power->vbus_detect, + ret =devm_delayed_work_autocancel (dev, &power->vbus_detect, axp_data->axp20x_read_vbus);if (ret)return ret;
@@ -1008,10 +1008,10 @@ static int axp20x_usb_power_probe(struct platform_device *pdev)return ret; } - psy_cfg.fwnode =dev_fwnode (&pdev->dev); + psy_cfg.fwnode =dev_fwnode (dev); psy_cfg.drv_data = power; - power->supply =devm_power_supply_register (&pdev->dev, + power->supply =devm_power_supply_register (dev, axp_data->power_desc, &psy_cfg);if (IS_ERR (power->supply))
@@ -1024,7 +1024,7 @@ static int axp20x_usb_power_probe(struct platform_device *pdev)return irq; power->irqs[i] =regmap_irq_get_virq (axp20x->regmap_irqc, irq); - ret =devm_request_any_context_irq (&pdev->dev, power->irqs[i], + ret =devm_request_any_context_irq (dev, power->irqs[i], axp20x_usb_power_irq,0 ,DRVNAME , power);if (ret <0 )
Signature · 1 line
2.47.3
PATCH 4/4 power: supply: axp20x_usb: Improve probe error reporting
In reply to Uwe Kleine-KönigDon't be silent about probe fail issues. Just having axp20x-usb-power-supply in /sys/kernel/debug/devices_deferred without a reason is making it harder than necessary to debug the actual issue. So be a bit more cooperative and add an error message for the typical failure points.
Exit paths where the error is -ENOMEM and register write failures are skipped, these should be quite obvious even without a dedicated error message.
--- drivers/power/supply/axp20x_usb_power.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-)
drivers/power/supply/axp20x_usb_power.c +9 -6
@@ -925,10 +925,8 @@ static int axp20x_usb_power_probe(struct platform_device *pdev)const struct axp_data *axp_data;int i, irq, ret; -if (!axp20x) { -dev_err (dev,"Parent drvdata not set\n" ); -return -EINVAL ; - } +if (!axp20x) +return dev_err_probe (dev, -EINVAL ,"Parent drvdata not set\n" ); axp_data =of_device_get_match_data (dev);
@@ -998,7 +996,9 @@ static int axp20x_usb_power_probe(struct platform_device *pdev) ret = axp_data->axp20x_cfg_adc_reg (power);if (ret) -return ret; +return dev_err_probe (dev, ret, +"Failed to initialize for %s usage\n" , +IS_ENABLED (CONFIG_AXP20X_ADC ) ?"iio" :"adc" ); }if (power->usb_bc_en_bit) {
@@ -1015,12 +1015,14 @@ static int axp20x_usb_power_probe(struct platform_device *pdev) axp_data->power_desc, &psy_cfg);if (IS_ERR (power->supply)) -return PTR_ERR (power->supply); +return dev_err_probe (dev,PTR_ERR (power->supply), +"Failed to register power supply\n" );/* Request irqs after registering, as irqs may trigger immediately */ for (i =0 ; i < axp_data->num_irq_names; i++) { irq =platform_get_irq_byname (pdev, axp_data->irq_names[i]);if (irq <0 ) +/* platform_get_irq_byname() already produced an error message */ return irq; power->irqs[i] =regmap_irq_get_virq (axp20x->regmap_irqc, irq);
@@ -1028,6 +1030,7 @@ static int axp20x_usb_power_probe(struct platform_device *pdev) axp20x_usb_power_irq,0 , DRVNAME, power);if (ret <0 ) +/* devm_request_any_context_irq() already produced an error message */ return ret; }
Signature · 1 line
2.47.3
Quoted 14 lines · Uwe Kleine-König
Hello,
while trying to help a Debian user debugging why USB doesn't work in the installer on a bananapi, I discovered these patch opportunities. They are only build tested.
The first patch touches a different subsystem than the other three, but given that maintainership overlaps and the first two patches are identical in spirit, I still created a single series only. The two subsets are orthogonal, so can be picked up individually by their respective subsystem maintainers (and please do so assuming you're happy with the patches).
Quoted 8 lines
Best regards
UweUwe Kleine-König (4):
pinctrl: axp209: Drop check for disabled devices
power: supply: axp20x*: Drop check for disabled devices
power: supply:axp20x_usb: Introduce a helper variable for&pdev->dev
power: supply:axp20x_usb: Improve probe error reporting
Whole series is
Sebastian Reichel
AppliedRe: (subset) [PATCH 0/4] Drive-by changes to axp20x drivers
In reply to Uwe Kleine-KönigQuoted 4 lines · Uwe Kleine-König
while trying to help a Debian user debugging why USB doesn't work in the installer on a bananapi, I discovered these patch opportunities. They are only build tested.
Quoted 8 lines
The first patch touches a different subsystem than the other three, but given that maintainership overlaps and the first two patches are identical in spirit, I still created a single series only. The two subsets are orthogonal, so can be picked up individually by their respective subsystem maintainers (and please do so assuming you're happy with the patches).
[...]
Applied, thanks!
[2/4] power: supply: axp20x*: Drop check for disabled devices
commit: c9ed44ab230f4359cdb3a72263c65daf31bd8528
[3/4] power: supply: axp20x_usb: Introduce a helper variable for &pdev->dev
commit: da54acbc2ff2470c49259a91a5f48c4b0cce09d7
[4/4] power: supply: axp20x_usb: Improve probe error reporting
commit: f945606a8cb34c97d644ed487282f96641b05185Best regards,
Signature · 1 line
Sebastian Reichel <[email protected]>