Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1628232 > unrolled thread

[PATCH 1/5] extcon: Allow extcon drivers to specify the extcon name

Back to article view | Back to linux.kernel

Contents

  [PATCH 1/5] extcon: Allow extcon drivers to specify the extcon name Hans de Goede <hdegoede@redhat.com> - 2017-04-21 15:10 +0200
    [PATCH 5/5] extcon: intel-cht-wc: Add support for monitoring external USB Type-C controller Hans de Goede <hdegoede@redhat.com> - 2017-04-21 15:10 +0200
      Re: [PATCH 5/5] extcon: intel-cht-wc: Add support for monitoring  external USB Type-C controller Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-04-21 21:00 +0200
      Re: [PATCH 5/5] extcon: intel-cht-wc: Add support for monitoring  external USB Type-C controller Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2017-04-24 16:40 +0200

#1628232 — [PATCH 1/5] extcon: Allow extcon drivers to specify the extcon name

The parent device name is not necessarily always useful, e.g.
with i2c devices it may simply be e.g.: "0-0022" and it also depends
on the i2c-bus number which depends on probe ordering.

This commit allows drivers to set their own, more useful name,
avoiding the problems with some i2c-device names.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/extcon/extcon.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
index f422a78..f8d3c1b 100644
--- a/drivers/extcon/extcon.c
+++ b/drivers/extcon/extcon.c
@@ -1117,7 +1117,8 @@ int extcon_dev_register(struct extcon_dev *edev)
 	edev->dev.class = extcon_class;
 	edev->dev.release = extcon_dev_release;
 
-	edev->name = dev_name(edev->dev.parent);
+	if (!edev->name)
+		edev->name = dev_name(edev->dev.parent);
 	if (IS_ERR_OR_NULL(edev->name)) {
 		dev_err(&edev->dev,
 			"extcon device name is null\n");
-- 
2.9.3

[toc] | [next] | [standalone]

#1628233 — [PATCH 5/5] extcon: intel-cht-wc: Add support for monitoring external USB Type-C controller

On some boards the Whiskey Cove PMIC is combined with an external USB
Type-C controller, in this case extcon consumers should use the Type-C
extcon state, except when the USB Type-C controller detects a current
limit of 500 mA which may indicate USB-C to USB-A cable at which point
the extcon consumer should use the Whiskey Cove's BC-1.2 detection's
state.

Since the Type-C controller info is incomplete and needs to be
supplemented with the BC1.2 detection result in some cases, this
commit makes the intel-cht-wc extcon driver monitor the extcon device
registered by the Type-C controller and report that as our extcon state
except when BC-1.2 detection should be used. This allows extcon
consumers on these boards to keep monitoring only the intel-cht-wc extcon
and then get complete extcon info from that, which combines the Type-C
and BC-1.2 info.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/extcon/extcon-intel-cht-wc.c | 96 ++++++++++++++++++++++++++++++------
 1 file changed, 80 insertions(+), 16 deletions(-)

diff --git a/drivers/extcon/extcon-intel-cht-wc.c b/drivers/extcon/extcon-intel-cht-wc.c
index 684f6b2..e510188 100644
--- a/drivers/extcon/extcon-intel-cht-wc.c
+++ b/drivers/extcon/extcon-intel-cht-wc.c
@@ -15,6 +15,7 @@
  * more details.
  */
 
+#include <linux/acpi.h>
 #include <linux/extcon.h>
 #include <linux/interrupt.h>
 #include <linux/kernel.h>
@@ -88,6 +89,7 @@ static const unsigned int cht_wc_extcon_cables[] = {
 	EXTCON_CHG_USB_CDP,
 	EXTCON_CHG_USB_DCP,
 	EXTCON_CHG_USB_ACA,
+	EXTCON_CHG_USB_FAST,
 	EXTCON_NONE,
 };
 
@@ -95,6 +97,9 @@ struct cht_wc_extcon_data {
 	struct device *dev;
 	struct regmap *regmap;
 	struct extcon_dev *edev;
+	struct extcon_dev *usbc;
+	struct notifier_block usbc_nb;
+	struct work_struct work;
 	unsigned int previous_cable;
 	bool usb_host;
 };
@@ -224,8 +229,32 @@ static void cht_wc_extcon_set_state(struct cht_wc_extcon_data *ext,
 	extcon_set_state_sync(ext->edev, EXTCON_USB_HOST, ext->usb_host);
 }
 
-static void cht_wc_extcon_pwrsrc_event(struct cht_wc_extcon_data *ext)
+static void cht_wc_extcon_usb_c_event(struct work_struct *work)
 {
+	struct cht_wc_extcon_data *ext =
+		container_of(work, struct cht_wc_extcon_data, work);
+	unsigned int cable = EXTCON_NONE;
+
+	if (extcon_get_state(ext->usbc, EXTCON_CHG_USB_SDP) > 0) {
+		/*
+		 * USB-C to USB-A cable ? Try BC1.2 charger detection from
+		 * WC PMIC, ignore charger detection errors.
+		 */
+		cable = cht_wc_extcon_get_charger(ext, true);
+	} else if (extcon_get_state(ext->usbc, EXTCON_CHG_USB_CDP) > 0) {
+		cable = EXTCON_CHG_USB_CDP;
+	} else if (extcon_get_state(ext->usbc, EXTCON_CHG_USB_FAST) > 0) {
+		cable = EXTCON_CHG_USB_FAST;
+	}
+
+	ext->usb_host = (extcon_get_state(ext->usbc, EXTCON_USB_HOST) > 0);
+	cht_wc_extcon_set_state(ext, cable);
+}
+
+static void cht_wc_extcon_pwrsrc_event(struct work_struct *work)
+{
+	struct cht_wc_extcon_data *ext =
+		container_of(work, struct cht_wc_extcon_data, work);
 	int ret, pwrsrc_sts, id;
 	unsigned int cable = EXTCON_NONE;
 	/* Ignore errors in host mode, as the 5v boost converter is on then */
@@ -258,7 +287,7 @@ static irqreturn_t cht_wc_extcon_isr(int irq, void *data)
 		return IRQ_NONE;
 	}
 
-	cht_wc_extcon_pwrsrc_event(ext);
+	schedule_work(&ext->work);
 
 	ret = regmap_write(ext->regmap, CHT_WC_PWRSRC_IRQ, irqs);
 	if (ret) {
@@ -269,6 +298,17 @@ static irqreturn_t cht_wc_extcon_isr(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
+static int cht_wc_extcon_usbc_evt(struct notifier_block *nb,
+				  unsigned long event, void *param)
+{
+	struct cht_wc_extcon_data *ext =
+		container_of(nb, struct cht_wc_extcon_data, usbc_nb);
+
+	schedule_work(&ext->work);
+
+	return NOTIFY_OK;
+}
+
 static int cht_wc_extcon_sw_control(struct cht_wc_extcon_data *ext, bool enable)
 {
 	int ret, mask, val;
@@ -300,6 +340,15 @@ static int cht_wc_extcon_probe(struct platform_device *pdev)
 	ext->regmap = pmic->regmap;
 	ext->previous_cable = EXTCON_NONE;
 
+	if (acpi_dev_present("INT33FE", NULL, -1)) {
+		ext->usbc = extcon_get_extcon_dev("fusb302");
+		if (!ext->usbc)
+			return -EPROBE_DEFER;
+
+		dev_info(&pdev->dev,
+			 "Using FUSB302 extcon for USB Type-C cable info\n");
+	}
+
 	/* Initialize extcon device */
 	ext->edev = devm_extcon_dev_allocate(ext->dev, cht_wc_extcon_cables);
 	if (IS_ERR(ext->edev))
@@ -335,25 +384,40 @@ static int cht_wc_extcon_probe(struct platform_device *pdev)
 	/* Route D+ and D- to PMIC for initial charger detection */
 	cht_wc_extcon_set_phymux(ext, MUX_SEL_PMIC);
 
-	/* Get initial state */
-	cht_wc_extcon_pwrsrc_event(ext);
-
-	ret = devm_request_threaded_irq(ext->dev, irq, NULL, cht_wc_extcon_isr,
-					IRQF_ONESHOT, pdev->name, ext);
-	if (ret) {
-		dev_err(ext->dev, "Error requesting interrupt: %d\n", ret);
-		goto disable_sw_control;
-	}
+	if (ext->usbc) {
+		INIT_WORK(&ext->work, cht_wc_extcon_usb_c_event);
+		ext->usbc_nb.notifier_call = cht_wc_extcon_usbc_evt;
+		ret = devm_extcon_register_notifier_all(&pdev->dev, ext->usbc,
+							&ext->usbc_nb);
+		if (ret) {
+			dev_err(&pdev->dev,
+				"Error registering usbc extcon notifier: %d\n",
+				ret);
+			goto disable_sw_control;
+		}
+	} else {
+		INIT_WORK(&ext->work, cht_wc_extcon_pwrsrc_event);
+		ret = devm_request_threaded_irq(ext->dev, irq, NULL,
+						cht_wc_extcon_isr, IRQF_ONESHOT,
+						pdev->name, ext);
+		if (ret) {
+			dev_err(ext->dev, "Error requesting irq: %d\n", ret);
+			goto disable_sw_control;
+		}
 
-	/* Unmask irqs */
-	ret = regmap_write(ext->regmap, CHT_WC_PWRSRC_IRQ_MASK,
+		/* Unmask irqs */
+		ret = regmap_write(ext->regmap, CHT_WC_PWRSRC_IRQ_MASK,
 			   (int)~(CHT_WC_PWRSRC_VBUS | CHT_WC_PWRSRC_ID_GND |
 				  CHT_WC_PWRSRC_ID_FLOAT));
-	if (ret) {
-		dev_err(ext->dev, "Error writing irq-mask: %d\n", ret);
-		goto disable_sw_control;
+		if (ret) {
+			dev_err(ext->dev, "Error writing irq-mask: %d\n", ret);
+			goto disable_sw_control;
+		}
 	}
 
+	/* Get initial state */
+	schedule_work(&ext->work);
+
 	platform_set_drvdata(pdev, ext);
 
 	return 0;
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1628506 — Re: [PATCH 5/5] extcon: intel-cht-wc: Add support for monitoring external USB Type-C controller

+Cc: Heikki,

On Fri, Apr 21, 2017 at 4:01 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> On some boards the Whiskey Cove PMIC is combined with an external USB
> Type-C controller, in this case extcon consumers should use the Type-C
> extcon state, except when the USB Type-C controller detects a current
> limit of 500 mA which may indicate USB-C to USB-A cable at which point
> the extcon consumer should use the Whiskey Cove's BC-1.2 detection's
> state.
>
> Since the Type-C controller info is incomplete and needs to be
> supplemented with the BC1.2 detection result in some cases, this
> commit makes the intel-cht-wc extcon driver monitor the extcon device
> registered by the Type-C controller and report that as our extcon state
> except when BC-1.2 detection should be used. This allows extcon
> consumers on these boards to keep monitoring only the intel-cht-wc extcon
> and then get complete extcon info from that, which combines the Type-C
> and BC-1.2 info.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/extcon/extcon-intel-cht-wc.c | 96 ++++++++++++++++++++++++++++++------
>  1 file changed, 80 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/extcon/extcon-intel-cht-wc.c b/drivers/extcon/extcon-intel-cht-wc.c
> index 684f6b2..e510188 100644
> --- a/drivers/extcon/extcon-intel-cht-wc.c
> +++ b/drivers/extcon/extcon-intel-cht-wc.c
> @@ -15,6 +15,7 @@
>   * more details.
>   */
>
> +#include <linux/acpi.h>
>  #include <linux/extcon.h>
>  #include <linux/interrupt.h>
>  #include <linux/kernel.h>
> @@ -88,6 +89,7 @@ static const unsigned int cht_wc_extcon_cables[] = {
>         EXTCON_CHG_USB_CDP,
>         EXTCON_CHG_USB_DCP,
>         EXTCON_CHG_USB_ACA,
> +       EXTCON_CHG_USB_FAST,
>         EXTCON_NONE,
>  };
>
> @@ -95,6 +97,9 @@ struct cht_wc_extcon_data {
>         struct device *dev;
>         struct regmap *regmap;
>         struct extcon_dev *edev;
> +       struct extcon_dev *usbc;
> +       struct notifier_block usbc_nb;
> +       struct work_struct work;
>         unsigned int previous_cable;
>         bool usb_host;
>  };
> @@ -224,8 +229,32 @@ static void cht_wc_extcon_set_state(struct cht_wc_extcon_data *ext,
>         extcon_set_state_sync(ext->edev, EXTCON_USB_HOST, ext->usb_host);
>  }
>
> -static void cht_wc_extcon_pwrsrc_event(struct cht_wc_extcon_data *ext)
> +static void cht_wc_extcon_usb_c_event(struct work_struct *work)
>  {
> +       struct cht_wc_extcon_data *ext =
> +               container_of(work, struct cht_wc_extcon_data, work);
> +       unsigned int cable = EXTCON_NONE;
> +
> +       if (extcon_get_state(ext->usbc, EXTCON_CHG_USB_SDP) > 0) {
> +               /*
> +                * USB-C to USB-A cable ? Try BC1.2 charger detection from
> +                * WC PMIC, ignore charger detection errors.
> +                */
> +               cable = cht_wc_extcon_get_charger(ext, true);
> +       } else if (extcon_get_state(ext->usbc, EXTCON_CHG_USB_CDP) > 0) {
> +               cable = EXTCON_CHG_USB_CDP;
> +       } else if (extcon_get_state(ext->usbc, EXTCON_CHG_USB_FAST) > 0) {
> +               cable = EXTCON_CHG_USB_FAST;
> +       }
> +
> +       ext->usb_host = (extcon_get_state(ext->usbc, EXTCON_USB_HOST) > 0);
> +       cht_wc_extcon_set_state(ext, cable);
> +}
> +
> +static void cht_wc_extcon_pwrsrc_event(struct work_struct *work)
> +{
> +       struct cht_wc_extcon_data *ext =
> +               container_of(work, struct cht_wc_extcon_data, work);
>         int ret, pwrsrc_sts, id;
>         unsigned int cable = EXTCON_NONE;
>         /* Ignore errors in host mode, as the 5v boost converter is on then */
> @@ -258,7 +287,7 @@ static irqreturn_t cht_wc_extcon_isr(int irq, void *data)
>                 return IRQ_NONE;
>         }
>
> -       cht_wc_extcon_pwrsrc_event(ext);
> +       schedule_work(&ext->work);
>
>         ret = regmap_write(ext->regmap, CHT_WC_PWRSRC_IRQ, irqs);
>         if (ret) {
> @@ -269,6 +298,17 @@ static irqreturn_t cht_wc_extcon_isr(int irq, void *data)
>         return IRQ_HANDLED;
>  }
>
> +static int cht_wc_extcon_usbc_evt(struct notifier_block *nb,
> +                                 unsigned long event, void *param)
> +{
> +       struct cht_wc_extcon_data *ext =
> +               container_of(nb, struct cht_wc_extcon_data, usbc_nb);
> +
> +       schedule_work(&ext->work);
> +
> +       return NOTIFY_OK;
> +}
> +
>  static int cht_wc_extcon_sw_control(struct cht_wc_extcon_data *ext, bool enable)
>  {
>         int ret, mask, val;
> @@ -300,6 +340,15 @@ static int cht_wc_extcon_probe(struct platform_device *pdev)
>         ext->regmap = pmic->regmap;
>         ext->previous_cable = EXTCON_NONE;
>
> +       if (acpi_dev_present("INT33FE", NULL, -1)) {
> +               ext->usbc = extcon_get_extcon_dev("fusb302");
> +               if (!ext->usbc)
> +                       return -EPROBE_DEFER;
> +
> +               dev_info(&pdev->dev,
> +                        "Using FUSB302 extcon for USB Type-C cable info\n");
> +       }
> +
>         /* Initialize extcon device */
>         ext->edev = devm_extcon_dev_allocate(ext->dev, cht_wc_extcon_cables);
>         if (IS_ERR(ext->edev))
> @@ -335,25 +384,40 @@ static int cht_wc_extcon_probe(struct platform_device *pdev)
>         /* Route D+ and D- to PMIC for initial charger detection */
>         cht_wc_extcon_set_phymux(ext, MUX_SEL_PMIC);
>
> -       /* Get initial state */
> -       cht_wc_extcon_pwrsrc_event(ext);
> -
> -       ret = devm_request_threaded_irq(ext->dev, irq, NULL, cht_wc_extcon_isr,
> -                                       IRQF_ONESHOT, pdev->name, ext);
> -       if (ret) {
> -               dev_err(ext->dev, "Error requesting interrupt: %d\n", ret);
> -               goto disable_sw_control;
> -       }
> +       if (ext->usbc) {
> +               INIT_WORK(&ext->work, cht_wc_extcon_usb_c_event);
> +               ext->usbc_nb.notifier_call = cht_wc_extcon_usbc_evt;
> +               ret = devm_extcon_register_notifier_all(&pdev->dev, ext->usbc,
> +                                                       &ext->usbc_nb);
> +               if (ret) {
> +                       dev_err(&pdev->dev,
> +                               "Error registering usbc extcon notifier: %d\n",
> +                               ret);
> +                       goto disable_sw_control;
> +               }
> +       } else {
> +               INIT_WORK(&ext->work, cht_wc_extcon_pwrsrc_event);
> +               ret = devm_request_threaded_irq(ext->dev, irq, NULL,
> +                                               cht_wc_extcon_isr, IRQF_ONESHOT,
> +                                               pdev->name, ext);
> +               if (ret) {
> +                       dev_err(ext->dev, "Error requesting irq: %d\n", ret);
> +                       goto disable_sw_control;
> +               }
>
> -       /* Unmask irqs */
> -       ret = regmap_write(ext->regmap, CHT_WC_PWRSRC_IRQ_MASK,
> +               /* Unmask irqs */
> +               ret = regmap_write(ext->regmap, CHT_WC_PWRSRC_IRQ_MASK,
>                            (int)~(CHT_WC_PWRSRC_VBUS | CHT_WC_PWRSRC_ID_GND |
>                                   CHT_WC_PWRSRC_ID_FLOAT));
> -       if (ret) {
> -               dev_err(ext->dev, "Error writing irq-mask: %d\n", ret);
> -               goto disable_sw_control;
> +               if (ret) {
> +                       dev_err(ext->dev, "Error writing irq-mask: %d\n", ret);
> +                       goto disable_sw_control;
> +               }
>         }
>
> +       /* Get initial state */
> +       schedule_work(&ext->work);
> +
>         platform_set_drvdata(pdev, ext);
>
>         return 0;
> --
> 2.9.3
>



-- 
With Best Regards,
Andy Shevchenko

[toc] | [prev] | [next] | [standalone]

#1629646 — Re: [PATCH 5/5] extcon: intel-cht-wc: Add support for monitoring external USB Type-C controller

Hi Hans,

On Fri, Apr 21, 2017 at 03:01:19PM +0200, Hans de Goede wrote:
> On some boards the Whiskey Cove PMIC is combined with an external USB
> Type-C controller, in this case extcon consumers should use the Type-C
> extcon state, except when the USB Type-C controller detects a current
> limit of 500 mA which may indicate USB-C to USB-A cable at which point
> the extcon consumer should use the Whiskey Cove's BC-1.2 detection's
> state.

Doesn't this mean you have several extcon_dev registered for a
single port? I'm not completely sure how extcon framework is meant to
be used, but shouldn't a single extcon_dev represent all the types of
connectors a port is meant to support?

> Since the Type-C controller info is incomplete and needs to be
> supplemented with the BC1.2 detection result in some cases, this
> commit makes the intel-cht-wc extcon driver monitor the extcon device
> registered by the Type-C controller and report that as our extcon state
> except when BC-1.2 detection should be used. This allows extcon
> consumers on these boards to keep monitoring only the intel-cht-wc extcon
> and then get complete extcon info from that, which combines the Type-C
> and BC-1.2 info.

I don't really understand why does this need to be done in such a
complex manner? Both components should just report the result and be
done with it, and there is no need for any coupling between the two.
If there is a consumer for the power, the driver for it can decide on
its own the limits and maximum power from the two sources.

> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/extcon/extcon-intel-cht-wc.c | 96 ++++++++++++++++++++++++++++++------

I did not realize we had this driver. It really should register a psy.
We need to be able to advertise the results of the detection to the
consumers in order to support for example boards that have a discrete
charger ic or battery attached to the SoC instead of PMIC, and without
being forced to use board/platform specific quirks like this in the
driver.

This driver should just report the result of the detection forward and
let the psy framework take care of notifying the other components,
consumers and whatnot, if there are any.


Thanks,

-- 
heikki

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web