| Message ID | 20170316161736.339-3-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | Rafael Wysocki |
| Headers | show |
Hi, On Thu, Mar 16, 2017 at 05:17:36PM +0100, Hans de Goede wrote: > On some devices with an axp288 pmic setting vbus path based on the > id-pin is handled by an ACPI _AIE interrupt on the gpio and the > INT3496 device is disabled. > > Instead of returning -EPROBE_DEFER on these devices waiting for the > never to show up INT3496 device, check for its presence and only > request and monitor the matching extcon if the device is there, > otherwise let the firmware handle the vbus path control. > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > drivers/power/supply/axp288_charger.c | 24 +++++++++++++++--------- > 1 file changed, 15 insertions(+), 9 deletions(-) > > diff --git a/drivers/power/supply/axp288_charger.c b/drivers/power/supply/axp288_charger.c > index d3bf4b2..6c4ad66 100644 > --- a/drivers/power/supply/axp288_charger.c > +++ b/drivers/power/supply/axp288_charger.c > @@ -14,6 +14,7 @@ > * GNU General Public License for more details. > */ > > +#include <linux/acpi.h> > #include <linux/module.h> > #include <linux/device.h> > #include <linux/regmap.h> > @@ -114,7 +115,8 @@ > #define ILIM_3000MA 3000 /* 3000mA */ > > #define AXP288_EXTCON_DEV_NAME "axp288_extcon" > -#define USB_HOST_EXTCON_DEV_NAME "INT3496:00" > +#define USB_HOST_EXTCON_HID "INT3496" > +#define USB_HOST_EXTCON_NAME "INT3496:00" > > static const unsigned int cable_ids[] = > { EXTCON_CHG_USB_SDP, EXTCON_CHG_USB_CDP, EXTCON_CHG_USB_DCP }; > @@ -808,10 +810,12 @@ static int axp288_charger_probe(struct platform_device *pdev) > return -EPROBE_DEFER; > } > > - info->otg.cable = extcon_get_extcon_dev(USB_HOST_EXTCON_DEV_NAME); > - if (info->otg.cable == NULL) { > - dev_dbg(dev, "EXTCON_USB_HOST is not ready, probe deferred\n"); > - return -EPROBE_DEFER; > + if (acpi_dev_present(USB_HOST_EXTCON_HID)) { I may have missed something, but as far as I can see axp288 also supports DT based init, so that should be handled here. -- Sebastian
Hi, On Mon, Mar 20, 2017 at 9:30 AM, Sebastian Reichel <sre@kernel.org> wrote: > Hi, > > On Thu, Mar 16, 2017 at 05:17:36PM +0100, Hans de Goede wrote: >> On some devices with an axp288 pmic setting vbus path based on the >> id-pin is handled by an ACPI _AIE interrupt on the gpio and the >> INT3496 device is disabled. >> >> Instead of returning -EPROBE_DEFER on these devices waiting for the >> never to show up INT3496 device, check for its presence and only >> request and monitor the matching extcon if the device is there, >> otherwise let the firmware handle the vbus path control. >> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >> --- >> drivers/power/supply/axp288_charger.c | 24 +++++++++++++++--------- >> 1 file changed, 15 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/power/supply/axp288_charger.c b/drivers/power/supply/axp288_charger.c >> index d3bf4b2..6c4ad66 100644 >> --- a/drivers/power/supply/axp288_charger.c >> +++ b/drivers/power/supply/axp288_charger.c >> @@ -14,6 +14,7 @@ >> * GNU General Public License for more details. >> */ >> >> +#include <linux/acpi.h> >> #include <linux/module.h> >> #include <linux/device.h> >> #include <linux/regmap.h> >> @@ -114,7 +115,8 @@ >> #define ILIM_3000MA 3000 /* 3000mA */ >> >> #define AXP288_EXTCON_DEV_NAME "axp288_extcon" >> -#define USB_HOST_EXTCON_DEV_NAME "INT3496:00" >> +#define USB_HOST_EXTCON_HID "INT3496" >> +#define USB_HOST_EXTCON_NAME "INT3496:00" >> >> static const unsigned int cable_ids[] = >> { EXTCON_CHG_USB_SDP, EXTCON_CHG_USB_CDP, EXTCON_CHG_USB_DCP }; >> @@ -808,10 +810,12 @@ static int axp288_charger_probe(struct platform_device *pdev) >> return -EPROBE_DEFER; >> } >> >> - info->otg.cable = extcon_get_extcon_dev(USB_HOST_EXTCON_DEV_NAME); >> - if (info->otg.cable == NULL) { >> - dev_dbg(dev, "EXTCON_USB_HOST is not ready, probe deferred\n"); >> - return -EPROBE_DEFER; >> + if (acpi_dev_present(USB_HOST_EXTCON_HID)) { > > I may have missed something, but as far as I can see axp288 also > supports DT based init, so that should be handled here. I doubt that will happen. AXP288 was designed specifically for Intel Bay Trail platforms, which AFAIK use ACPI. For the other AXP series PMIC we have another battery driver for them coming in. If you look in drivers/mfd/axp20x.c and drivers/mfd/axp20x-i2c.c, you'll see that AXP288 is only instantiated from ACPI, and all the axp288 named drivers are only instantiated for the AXP288 mfd device. Regards ChenYu
Hi, On Mon, Mar 20, 2017 at 04:56:18PM +0800, Chen-Yu Tsai wrote: > On Mon, Mar 20, 2017 at 9:30 AM, Sebastian Reichel <sre@kernel.org> wrote: > > On Thu, Mar 16, 2017 at 05:17:36PM +0100, Hans de Goede wrote: > >> On some devices with an axp288 pmic setting vbus path based on the > >> id-pin is handled by an ACPI _AIE interrupt on the gpio and the > >> INT3496 device is disabled. > >> > >> Instead of returning -EPROBE_DEFER on these devices waiting for the > >> never to show up INT3496 device, check for its presence and only > >> request and monitor the matching extcon if the device is there, > >> otherwise let the firmware handle the vbus path control. > >> > >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> > >> --- > >> drivers/power/supply/axp288_charger.c | 24 +++++++++++++++--------- > >> 1 file changed, 15 insertions(+), 9 deletions(-) > >> > >> diff --git a/drivers/power/supply/axp288_charger.c b/drivers/power/supply/axp288_charger.c > >> index d3bf4b2..6c4ad66 100644 > >> --- a/drivers/power/supply/axp288_charger.c > >> +++ b/drivers/power/supply/axp288_charger.c > >> @@ -14,6 +14,7 @@ > >> * GNU General Public License for more details. > >> */ > >> > >> +#include <linux/acpi.h> > >> #include <linux/module.h> > >> #include <linux/device.h> > >> #include <linux/regmap.h> > >> @@ -114,7 +115,8 @@ > >> #define ILIM_3000MA 3000 /* 3000mA */ > >> > >> #define AXP288_EXTCON_DEV_NAME "axp288_extcon" > >> -#define USB_HOST_EXTCON_DEV_NAME "INT3496:00" > >> +#define USB_HOST_EXTCON_HID "INT3496" > >> +#define USB_HOST_EXTCON_NAME "INT3496:00" > >> > >> static const unsigned int cable_ids[] = > >> { EXTCON_CHG_USB_SDP, EXTCON_CHG_USB_CDP, EXTCON_CHG_USB_DCP }; > >> @@ -808,10 +810,12 @@ static int axp288_charger_probe(struct platform_device *pdev) > >> return -EPROBE_DEFER; > >> } > >> > >> - info->otg.cable = extcon_get_extcon_dev(USB_HOST_EXTCON_DEV_NAME); > >> - if (info->otg.cable == NULL) { > >> - dev_dbg(dev, "EXTCON_USB_HOST is not ready, probe deferred\n"); > >> - return -EPROBE_DEFER; > >> + if (acpi_dev_present(USB_HOST_EXTCON_HID)) { > > > > I may have missed something, but as far as I can see axp288 also > > supports DT based init, so that should be handled here. > > I doubt that will happen. AXP288 was designed specifically for Intel > Bay Trail platforms, which AFAIK use ACPI. For the other AXP series > PMIC we have another battery driver for them coming in. > > If you look in drivers/mfd/axp20x.c and drivers/mfd/axp20x-i2c.c, > you'll see that AXP288 is only instantiated from ACPI, and all > the axp288 named drivers are only instantiated for the AXP288 > mfd device. Ok. I just looked briefly at drivers/mfd/axp20x.c and saw, that it contained of_compatible entries. On a closer look axp288 does not have them, so: Acked-by: Sebastian Reichel <sre@kernel.org> -- Sebastian
diff --git a/drivers/power/supply/axp288_charger.c b/drivers/power/supply/axp288_charger.c index d3bf4b2..6c4ad66 100644 --- a/drivers/power/supply/axp288_charger.c +++ b/drivers/power/supply/axp288_charger.c @@ -14,6 +14,7 @@ * GNU General Public License for more details. */ +#include <linux/acpi.h> #include <linux/module.h> #include <linux/device.h> #include <linux/regmap.h> @@ -114,7 +115,8 @@ #define ILIM_3000MA 3000 /* 3000mA */ #define AXP288_EXTCON_DEV_NAME "axp288_extcon" -#define USB_HOST_EXTCON_DEV_NAME "INT3496:00" +#define USB_HOST_EXTCON_HID "INT3496" +#define USB_HOST_EXTCON_NAME "INT3496:00" static const unsigned int cable_ids[] = { EXTCON_CHG_USB_SDP, EXTCON_CHG_USB_CDP, EXTCON_CHG_USB_DCP }; @@ -808,10 +810,12 @@ static int axp288_charger_probe(struct platform_device *pdev) return -EPROBE_DEFER; } - info->otg.cable = extcon_get_extcon_dev(USB_HOST_EXTCON_DEV_NAME); - if (info->otg.cable == NULL) { - dev_dbg(dev, "EXTCON_USB_HOST is not ready, probe deferred\n"); - return -EPROBE_DEFER; + if (acpi_dev_present(USB_HOST_EXTCON_HID)) { + info->otg.cable = extcon_get_extcon_dev(USB_HOST_EXTCON_NAME); + if (info->otg.cable == NULL) { + dev_dbg(dev, "EXTCON_USB_HOST is not ready, probe deferred\n"); + return -EPROBE_DEFER; + } } platform_set_drvdata(pdev, info); @@ -850,11 +854,13 @@ static int axp288_charger_probe(struct platform_device *pdev) /* Register for OTG notification */ INIT_WORK(&info->otg.work, axp288_charger_otg_evt_worker); info->otg.id_nb.notifier_call = axp288_charger_handle_otg_evt; - ret = devm_extcon_register_notifier(&pdev->dev, info->otg.cable, + if (info->otg.cable) { + ret = devm_extcon_register_notifier(&pdev->dev, info->otg.cable, EXTCON_USB_HOST, &info->otg.id_nb); - if (ret) { - dev_err(dev, "failed to register EXTCON_USB_HOST notifier\n"); - return ret; + if (ret) { + dev_err(dev, "failed to register EXTCON_USB_HOST notifier\n"); + return ret; + } } schedule_work(&info->otg.work);
On some devices with an axp288 pmic setting vbus path based on the id-pin is handled by an ACPI _AIE interrupt on the gpio and the INT3496 device is disabled. Instead of returning -EPROBE_DEFER on these devices waiting for the never to show up INT3496 device, check for its presence and only request and monitor the matching extcon if the device is there, otherwise let the firmware handle the vbus path control. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/power/supply/axp288_charger.c | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-)