Hi, On Wed, Dec 05, 2018 at 10:57:12AM +0800, Baolin Wang wrote: > Hi Rob, > On Wed, 5 Dec 2018 at 05:52, Rob Herring <robh@xxxxxxxxxx> wrote: > > > > On Fri, Nov 16, 2018 at 07:01:12PM +0800, Baolin Wang wrote: > > > The struct extcon_specific_cable_nb and related APIs are deprecated now, > > > so we should use new method to get one extcon device and register extcon > > > notifier. > > > > > > Signed-off-by: Baolin Wang <baolin.wang@xxxxxxxxxx> > > > --- > > > .../bindings/power/supply/charger-manager.txt | 6 +-- > > > > Bindings should be a separate patch. > > Sure. > > > > > > drivers/power/supply/charger-manager.c | 51 ++++++++------------ > > > include/linux/power/charger-manager.h | 2 +- > > > 3 files changed, 23 insertions(+), 36 deletions(-) > > > > > > diff --git a/Documentation/devicetree/bindings/power/supply/charger-manager.txt b/Documentation/devicetree/bindings/power/supply/charger-manager.txt > > > index ec4fe9d..315b0cb 100644 > > > --- a/Documentation/devicetree/bindings/power/supply/charger-manager.txt > > > +++ b/Documentation/devicetree/bindings/power/supply/charger-manager.txt > > > @@ -11,7 +11,7 @@ Required properties : > > > - cm-regulator-name : name of charger regulator > > > - subnode <cable> : > > > - cm-cable-name : name of charger cable > > > - - cm-cable-extcon : name of extcon dev > > > + - extcon : phandles to external connector devices > > > > Somewhat less terrible, but not really. I consider extcon binding itself > > deprecated. I suspect 'charger-manager' as a whole probably needs to be > > reworked. This also is not a backwards compatible change. Right, charger-manager is a big hack. The DT node does not represent any hardware. The feature should be integrated into the power-supply core and work without any special DT bindings. I don't have the time to work on this, though. > We are do some optimization for 'charger-manager', like this patch > did, we are trying to remove the deprecated extcon API. > And now no one use the incorrect 'cm-cable-extcon' property on > mainline, so no need worry backwards compatibility. As far as I can see there is no charger-manager user in mainline at all. -- Sebastian > > > (optional) - cm-cable-min : minimum current of cable > > > (optional) - cm-cable-max : maximum current of cable > > > > > > @@ -66,13 +66,13 @@ Example : > > > cm-regulator-name = "chg-reg"; > > > cable@0 { > > > cm-cable-name = "USB"; > > > - cm-cable-extcon = "extcon-dev.0"; > > > + extcon = <&extcon_usb>; > > > cm-cable-min = <475000>; > > > cm-cable-max = <500000>; > > > }; > > > cable@1 { > > > cm-cable-name = "TA"; > > > - cm-cable-extcon = "extcon-dev.0"; > > > + extcon = <&extcon_usb>; > > > cm-cable-min = <650000>; > > > cm-cable-max = <675000>; > > > }; > > > diff --git a/drivers/power/supply/charger-manager.c b/drivers/power/supply/charger-manager.c > > > index dc0c9a6..4f28c03 100644 > > > --- a/drivers/power/supply/charger-manager.c > > > +++ b/drivers/power/supply/charger-manager.c > > > @@ -1207,12 +1207,11 @@ static int charger_extcon_init(struct charger_manager *cm, > > > */ > > > INIT_WORK(&cable->wq, charger_extcon_work); > > > cable->nb.notifier_call = charger_extcon_notifier; > > > - ret = extcon_register_interest(&cable->extcon_dev, > > > - cable->extcon_name, cable->name, &cable->nb); > > > - if (ret < 0) { > > > - pr_info("Cannot register extcon_dev for %s(cable: %s)\n", > > > - cable->extcon_name, cable->name); > > > - } > > > + ret = devm_extcon_register_notifier(cm->dev, cable->extcon_dev, > > > + EXTCON_USB, &cable->nb); > > > + if (ret < 0) > > > + dev_err(cm->dev, "Cannot register extcon_dev for (cable: %s)\n", > > > + cable->name); > > > > > > return ret; > > > } > > > @@ -1589,15 +1588,25 @@ static struct charger_desc *of_cm_parse_desc(struct device *dev) > > > for_each_child_of_node(child, _child) { > > > of_property_read_string(_child, > > > "cm-cable-name", &cables->name); > > > - of_property_read_string(_child, > > > - "cm-cable-extcon", > > > - &cables->extcon_name); > > > of_property_read_u32(_child, > > > "cm-cable-min", > > > &cables->min_uA); > > > of_property_read_u32(_child, > > > "cm-cable-max", > > > &cables->max_uA); > > > + > > > + if (of_property_read_bool(_child, "extcon")) { > > > + struct device_node *np; > > > + > > > + np = of_parse_phandle(_child, "extcon", 0); > > > + if (!np) > > > + return ERR_PTR(-ENODEV); > > > + > > > + cables->extcon_dev = extcon_find_edev_by_node(np); > > > + of_node_put(np); > > > + if (IS_ERR(cables->extcon_dev)) > > > + return ERR_PTR(PTR_ERR(cables->extcon_dev)); > > > + } > > > cables++; > > > } > > > } > > > @@ -1625,7 +1634,6 @@ static int charger_manager_probe(struct platform_device *pdev) > > > struct charger_desc *desc = cm_get_drv_data(pdev); > > > struct charger_manager *cm; > > > int ret, i = 0; > > > - int j = 0; > > > union power_supply_propval val; > > > struct power_supply *fuel_gauge; > > > struct power_supply_config psy_cfg = {}; > > > @@ -1823,19 +1831,8 @@ static int charger_manager_probe(struct platform_device *pdev) > > > &charger->attr_g); > > > } > > > err_reg_extcon: > > > - for (i = 0; i < desc->num_charger_regulators; i++) { > > > - struct charger_regulator *charger; > > > - > > > - charger = &desc->charger_regulators[i]; > > > - for (j = 0; j < charger->num_cables; j++) { > > > - struct charger_cable *cable = &charger->cables[j]; > > > - /* Remove notifier block if only edev exists */ > > > - if (cable->extcon_dev.edev) > > > - extcon_unregister_interest(&cable->extcon_dev); > > > - } > > > - > > > + for (i = 0; i < desc->num_charger_regulators; i++) > > > regulator_put(desc->charger_regulators[i].consumer); > > > - } > > > > > > power_supply_unregister(cm->charger_psy); > > > > > > @@ -1847,7 +1844,6 @@ static int charger_manager_remove(struct platform_device *pdev) > > > struct charger_manager *cm = platform_get_drvdata(pdev); > > > struct charger_desc *desc = cm->desc; > > > int i = 0; > > > - int j = 0; > > > > > > /* Remove from the list */ > > > mutex_lock(&cm_list_mtx); > > > @@ -1857,15 +1853,6 @@ static int charger_manager_remove(struct platform_device *pdev) > > > cancel_work_sync(&setup_polling); > > > cancel_delayed_work_sync(&cm_monitor_work); > > > > > > - for (i = 0 ; i < desc->num_charger_regulators ; i++) { > > > - struct charger_regulator *charger > > > - = &desc->charger_regulators[i]; > > > - for (j = 0 ; j < charger->num_cables ; j++) { > > > - struct charger_cable *cable = &charger->cables[j]; > > > - extcon_unregister_interest(&cable->extcon_dev); > > > - } > > > - } > > > - > > > for (i = 0 ; i < desc->num_charger_regulators ; i++) > > > regulator_put(desc->charger_regulators[i].consumer); > > > > > > diff --git a/include/linux/power/charger-manager.h b/include/linux/power/charger-manager.h > > > index c4fa907..e4d0269 100644 > > > --- a/include/linux/power/charger-manager.h > > > +++ b/include/linux/power/charger-manager.h > > > @@ -66,7 +66,7 @@ struct charger_cable { > > > const char *name; > > > > > > /* The charger-manager use Extcon framework */ > > > - struct extcon_specific_cable_nb extcon_dev; > > > + struct extcon_dev *extcon_dev; > > > struct work_struct wq; > > > struct notifier_block nb; > > > > > > -- > > > 1.7.9.5 > > > > > > > -- > Baolin Wang > Best Regards
Attachment:
signature.asc
Description: PGP signature