[PATCH 1/4] extcon: axp288: Remove unused extcon_nb struct member

[PATCH 1/4] extcon: axp288: Remove unused extcon_nb struct member

[Hans de Goede]

Remove the unused extcon_nb struct member.

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

diff --git a/drivers/extcon/extcon-axp288.c b/drivers/extcon/extcon-axp288.c
index 981fba56bc18..ba185e9ebb82 100644
--- a/drivers/extcon/extcon-axp288.c
+++ b/drivers/extcon/extcon-axp288.c
@@ -107,7 +107,6 @@ struct axp288_extcon_info {
 	struct gpio_desc *gpio_mux_cntl;
 	int irq[EXTCON_IRQ_END];
 	struct extcon_dev *edev;
-	struct notifier_block extcon_nb;
 	unsigned int previous_cable;
 };
 
-- 
2.14.3

[PATCH 2/4] extcon: axp288: Remove unused platform data

[Hans de Goede]

This is not used / set anywhere in the tree.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/extcon/extcon-axp288.c | 35 +----------------------------------
 include/linux/mfd/axp20x.h     |  5 -----
 2 files changed, 1 insertion(+), 39 deletions(-)

diff --git a/drivers/extcon/extcon-axp288.c b/drivers/extcon/extcon-axp288.c
index ba185e9ebb82..386afb7d1160 100644
--- a/drivers/extcon/extcon-axp288.c
+++ b/drivers/extcon/extcon-axp288.c
@@ -24,8 +24,6 @@
 #include <linux/notifier.h>
 #include <linux/extcon-provider.h>
 #include <linux/regmap.h>
-#include <linux/gpio.h>
-#include <linux/gpio/consumer.h>
 #include <linux/mfd/axp20x.h>
 
 /* Power source status register */
@@ -79,11 +77,6 @@ enum axp288_extcon_reg {
 	AXP288_BC_DET_STAT_REG		= 0x2f,
 };
 
-enum axp288_mux_select {
-	EXTCON_GPIO_MUX_SEL_PMIC = 0,
-	EXTCON_GPIO_MUX_SEL_SOC,
-};
-
 enum axp288_extcon_irq {
 	VBUS_FALLING_IRQ = 0,
 	VBUS_RISING_IRQ,
@@ -104,7 +97,6 @@ struct axp288_extcon_info {
 	struct device *dev;
 	struct regmap *regmap;
 	struct regmap_irq_chip_data *regmap_irqc;
-	struct gpio_desc *gpio_mux_cntl;
 	int irq[EXTCON_IRQ_END];
 	struct extcon_dev *edev;
 	unsigned int previous_cable;
@@ -196,15 +188,6 @@ static int axp288_handle_chrg_det_event(struct axp288_extcon_info *info)
 	}
 
 no_vbus:
-	/*
-	 * If VBUS is absent Connect D+/D- lines to PMIC for BC
-	 * detection. Else connect them to SOC for USB communication.
-	 */
-	if (info->gpio_mux_cntl)
-		gpiod_set_value(info->gpio_mux_cntl,
-			vbus_attach ? EXTCON_GPIO_MUX_SEL_SOC
-					: EXTCON_GPIO_MUX_SEL_PMIC);
-
 	extcon_set_state_sync(info->edev, info->previous_cable, false);
 	if (info->previous_cable == EXTCON_CHG_USB_SDP)
 		extcon_set_state_sync(info->edev, EXTCON_USB, false);
@@ -252,8 +235,7 @@ static int axp288_extcon_probe(struct platform_device *pdev)
 {
 	struct axp288_extcon_info *info;
 	struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
-	struct axp288_extcon_pdata *pdata = pdev->dev.platform_data;
-	int ret, i, pirq, gpio;
+	int ret, i, pirq;
 
 	info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
 	if (!info)
@@ -263,8 +245,6 @@ static int axp288_extcon_probe(struct platform_device *pdev)
 	info->regmap = axp20x->regmap;
 	info->regmap_irqc = axp20x->regmap_irqc;
 	info->previous_cable = EXTCON_NONE;
-	if (pdata)
-		info->gpio_mux_cntl = pdata->gpio_mux_cntl;
 
 	platform_set_drvdata(pdev, info);
 
@@ -285,19 +265,6 @@ static int axp288_extcon_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	/* Set up gpio control for USB Mux */
-	if (info->gpio_mux_cntl) {
-		gpio = desc_to_gpio(info->gpio_mux_cntl);
-		ret = devm_gpio_request(&pdev->dev, gpio, "USB_MUX");
-		if (ret < 0) {
-			dev_err(&pdev->dev,
-				"failed to request the gpio=%d\n", gpio);
-			return ret;
-		}
-		gpiod_direction_output(info->gpio_mux_cntl,
-						EXTCON_GPIO_MUX_SEL_PMIC);
-	}
-
 	for (i = 0; i < EXTCON_IRQ_END; i++) {
 		pirq = platform_get_irq(pdev, i);
 		info->irq[i] = regmap_irq_get_virq(info->regmap_irqc, pirq);
diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
index 78dc85365c4f..080798f17ece 100644
--- a/include/linux/mfd/axp20x.h
+++ b/include/linux/mfd/axp20x.h
@@ -645,11 +645,6 @@ struct axp20x_dev {
 	const struct regmap_irq_chip	*regmap_irq_chip;
 };
 
-struct axp288_extcon_pdata {
-	/* GPIO pin control to switch D+/D- lines b/w PMIC and SOC */
-	struct gpio_desc *gpio_mux_cntl;
-};
-
 /* generic helper function for reading 9-16 bit wide regs */
 static inline int axp20x_read_variable_width(struct regmap *regmap,
 	unsigned int reg, unsigned int width)
-- 
2.14.3

[PATCH 3/4] extcon: axp288: Redo charger type dection a couple of seconds after probe()

[Hans de Goede]

The axp288 extcon code depends on other drivers to do things like mux the
data lines, enable/disable vbus based on the id-pin, etc.

Sometimes the BIOS has not set these things up correctly resulting in the
initial charger cable type detection giving a wrong result and we end up
not charging or charging at only 0.5A.

This commit starts a second charger-detection cycle a couple of seconds
after the first one finishes, giving the other drivers time to load and
do their thing.

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

diff --git a/drivers/extcon/extcon-axp288.c b/drivers/extcon/extcon-axp288.c
index 386afb7d1160..cc7c35c7ff02 100644
--- a/drivers/extcon/extcon-axp288.c
+++ b/drivers/extcon/extcon-axp288.c
@@ -1,6 +1,7 @@
 /*
  * extcon-axp288.c - X-Power AXP288 PMIC extcon cable detection driver
  *
+ * Copyright (C) 2016-2017 Hans de Goede <hdegoede@redhat.com>
  * Copyright (C) 2015 Intel Corporation
  * Author: Ramakrishna Pallala <ramakrishna.pallala@intel.com>
  *
@@ -97,9 +98,11 @@ struct axp288_extcon_info {
 	struct device *dev;
 	struct regmap *regmap;
 	struct regmap_irq_chip_data *regmap_irqc;
+	struct delayed_work det_work;
 	int irq[EXTCON_IRQ_END];
 	struct extcon_dev *edev;
 	unsigned int previous_cable;
+	bool first_detect_done;
 };
 
 /* Power up/down reason string array */
@@ -137,6 +140,25 @@ static void axp288_extcon_log_rsi(struct axp288_extcon_info *info)
 	regmap_write(info->regmap, AXP288_PS_BOOT_REASON_REG, clear_mask);
 }
 
+static void axp288_chrg_detect_complete(struct axp288_extcon_info *info)
+{
+	/*
+	 * We depend on other drivers to do things like mux the data lines,
+	 * enable/disable vbus based on the id-pin, etc. Sometimes the BIOS has
+	 * not set these things up correctly resulting in the initial charger
+	 * cable type detection giving a wrong result and we end up not charging
+	 * or charging at only 0.5A.
+	 *
+	 * So we schedule a second cable type detection after 2 seconds to
+	 * give the other drivers time to load and do their thing.
+	 */
+	if (!info->first_detect_done) {
+		queue_delayed_work(system_wq, &info->det_work,
+				   msecs_to_jiffies(2000));
+		info->first_detect_done = true;
+	}
+}
+
 static int axp288_handle_chrg_det_event(struct axp288_extcon_info *info)
 {
 	int ret, stat, cfg, pwr_stat;
@@ -201,6 +223,8 @@ static int axp288_handle_chrg_det_event(struct axp288_extcon_info *info)
 		info->previous_cable = cable;
 	}
 
+	axp288_chrg_detect_complete(info);
+
 	return 0;
 
 dev_det_ret:
@@ -222,8 +246,11 @@ static irqreturn_t axp288_extcon_isr(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
-static void axp288_extcon_enable(struct axp288_extcon_info *info)
+static void axp288_extcon_det_work(struct work_struct *work)
 {
+	struct axp288_extcon_info *info =
+		container_of(work, struct axp288_extcon_info, det_work.work);
+
 	regmap_update_bits(info->regmap, AXP288_BC_GLOBAL_REG,
 						BC_GLOBAL_RUN, 0);
 	/* Enable the charger detection logic */
@@ -245,6 +272,7 @@ static int axp288_extcon_probe(struct platform_device *pdev)
 	info->regmap = axp20x->regmap;
 	info->regmap_irqc = axp20x->regmap_irqc;
 	info->previous_cable = EXTCON_NONE;
+	INIT_DELAYED_WORK(&info->det_work, axp288_extcon_det_work);
 
 	platform_set_drvdata(pdev, info);
 
@@ -287,7 +315,7 @@ static int axp288_extcon_probe(struct platform_device *pdev)
 	}
 
 	/* Start charger cable type detection */
-	axp288_extcon_enable(info);
+	queue_delayed_work(system_wq, &info->det_work, 0);
 
 	return 0;
 }
-- 
2.14.3

[PATCH 4/4] extcon: axp288: Handle reserved charger-type values better

[Hans de Goede]

According to the data sheets all the values not handled in the
switch-case are "reserved". Update the dev_warn message to reflect
this and set the cable-type to EXTCON_CHG_USB_SDP (so max 500mA
current draw) as safe default.

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

diff --git a/drivers/extcon/extcon-axp288.c b/drivers/extcon/extcon-axp288.c
index cc7c35c7ff02..d60c615f709f 100644
--- a/drivers/extcon/extcon-axp288.c
+++ b/drivers/extcon/extcon-axp288.c
@@ -205,8 +205,8 @@ static int axp288_handle_chrg_det_event(struct axp288_extcon_info *info)
 		cable = EXTCON_CHG_USB_DCP;
 		break;
 	default:
-		dev_warn(info->dev,
-			"disconnect or unknown or ID event\n");
+		dev_warn(info->dev, "unknown (reserved) bc detect result\n");
+		cable = EXTCON_CHG_USB_SDP;
 	}
 
 no_vbus:
-- 
2.14.3

Re: [PATCH 2/4] extcon: axp288: Remove unused platform data

[Chanwoo Choi]

+ MFD Maintainer (Lee Jones <lee.jones@linaro.org>)

Hi Hans,

You better to use the scripts/get_maintainer.pl for the mailing list.
I added the MFD maintainer.

This patch looks good to me.
Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>


Best Regards,
Chanwoo Choi

On 2017년 12월 22일 21:36, Hans de Goede wrote:
> This is not used / set anywhere in the tree.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/extcon/extcon-axp288.c | 35 +----------------------------------
>  include/linux/mfd/axp20x.h     |  5 -----
>  2 files changed, 1 insertion(+), 39 deletions(-)
> 
> diff --git a/drivers/extcon/extcon-axp288.c b/drivers/extcon/extcon-axp288.c
> index ba185e9ebb82..386afb7d1160 100644
> --- a/drivers/extcon/extcon-axp288.c
> +++ b/drivers/extcon/extcon-axp288.c
> @@ -24,8 +24,6 @@
>  #include <linux/notifier.h>
>  #include <linux/extcon-provider.h>
>  #include <linux/regmap.h>
> -#include <linux/gpio.h>
> -#include <linux/gpio/consumer.h>
>  #include <linux/mfd/axp20x.h>
>  
>  /* Power source status register */
> @@ -79,11 +77,6 @@ enum axp288_extcon_reg {
>  	AXP288_BC_DET_STAT_REG		= 0x2f,
>  };
>  
> -enum axp288_mux_select {
> -	EXTCON_GPIO_MUX_SEL_PMIC = 0,
> -	EXTCON_GPIO_MUX_SEL_SOC,
> -};
> -
>  enum axp288_extcon_irq {
>  	VBUS_FALLING_IRQ = 0,
>  	VBUS_RISING_IRQ,
> @@ -104,7 +97,6 @@ struct axp288_extcon_info {
>  	struct device *dev;
>  	struct regmap *regmap;
>  	struct regmap_irq_chip_data *regmap_irqc;
> -	struct gpio_desc *gpio_mux_cntl;
>  	int irq[EXTCON_IRQ_END];
>  	struct extcon_dev *edev;
>  	unsigned int previous_cable;
> @@ -196,15 +188,6 @@ static int axp288_handle_chrg_det_event(struct axp288_extcon_info *info)
>  	}
>  
>  no_vbus:
> -	/*
> -	 * If VBUS is absent Connect D+/D- lines to PMIC for BC
> -	 * detection. Else connect them to SOC for USB communication.
> -	 */
> -	if (info->gpio_mux_cntl)
> -		gpiod_set_value(info->gpio_mux_cntl,
> -			vbus_attach ? EXTCON_GPIO_MUX_SEL_SOC
> -					: EXTCON_GPIO_MUX_SEL_PMIC);
> -
>  	extcon_set_state_sync(info->edev, info->previous_cable, false);
>  	if (info->previous_cable == EXTCON_CHG_USB_SDP)
>  		extcon_set_state_sync(info->edev, EXTCON_USB, false);
> @@ -252,8 +235,7 @@ static int axp288_extcon_probe(struct platform_device *pdev)
>  {
>  	struct axp288_extcon_info *info;
>  	struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
> -	struct axp288_extcon_pdata *pdata = pdev->dev.platform_data;
> -	int ret, i, pirq, gpio;
> +	int ret, i, pirq;
>  
>  	info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
>  	if (!info)
> @@ -263,8 +245,6 @@ static int axp288_extcon_probe(struct platform_device *pdev)
>  	info->regmap = axp20x->regmap;
>  	info->regmap_irqc = axp20x->regmap_irqc;
>  	info->previous_cable = EXTCON_NONE;
> -	if (pdata)
> -		info->gpio_mux_cntl = pdata->gpio_mux_cntl;
>  
>  	platform_set_drvdata(pdev, info);
>  
> @@ -285,19 +265,6 @@ static int axp288_extcon_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> -	/* Set up gpio control for USB Mux */
> -	if (info->gpio_mux_cntl) {
> -		gpio = desc_to_gpio(info->gpio_mux_cntl);
> -		ret = devm_gpio_request(&pdev->dev, gpio, "USB_MUX");
> -		if (ret < 0) {
> -			dev_err(&pdev->dev,
> -				"failed to request the gpio=%d\n", gpio);
> -			return ret;
> -		}
> -		gpiod_direction_output(info->gpio_mux_cntl,
> -						EXTCON_GPIO_MUX_SEL_PMIC);
> -	}
> -
>  	for (i = 0; i < EXTCON_IRQ_END; i++) {
>  		pirq = platform_get_irq(pdev, i);
>  		info->irq[i] = regmap_irq_get_virq(info->regmap_irqc, pirq);
> diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
> index 78dc85365c4f..080798f17ece 100644
> --- a/include/linux/mfd/axp20x.h
> +++ b/include/linux/mfd/axp20x.h
> @@ -645,11 +645,6 @@ struct axp20x_dev {
>  	const struct regmap_irq_chip	*regmap_irq_chip;
>  };
>  
> -struct axp288_extcon_pdata {
> -	/* GPIO pin control to switch D+/D- lines b/w PMIC and SOC */
> -	struct gpio_desc *gpio_mux_cntl;
> -};
> -
>  /* generic helper function for reading 9-16 bit wide regs */
>  static inline int axp20x_read_variable_width(struct regmap *regmap,
>  	unsigned int reg, unsigned int width)
> 

Re: [PATCH 1/4] extcon: axp288: Remove unused extcon_nb struct member

[Chanwoo Choi]

On 2017년 12월 22일 21:36, Hans de Goede wrote:
> Remove the unused extcon_nb struct member.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/extcon/extcon-axp288.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/extcon/extcon-axp288.c b/drivers/extcon/extcon-axp288.c
> index 981fba56bc18..ba185e9ebb82 100644
> --- a/drivers/extcon/extcon-axp288.c
> +++ b/drivers/extcon/extcon-axp288.c
> @@ -107,7 +107,6 @@ struct axp288_extcon_info {
>  	struct gpio_desc *gpio_mux_cntl;
>  	int irq[EXTCON_IRQ_END];
>  	struct extcon_dev *edev;
> -	struct notifier_block extcon_nb;
>  	unsigned int previous_cable;
>  };
>  
> 

Looks good to me.
Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Re: [PATCH 3/4] extcon: axp288: Redo charger type dection a couple of seconds after probe()

[Chanwoo Choi]

Hi Hans,

s/dection/detection on patch title.

On 2017년 12월 22일 21:36, Hans de Goede wrote:
> The axp288 extcon code depends on other drivers to do things like mux the
> data lines, enable/disable vbus based on the id-pin, etc.
> 
> Sometimes the BIOS has not set these things up correctly resulting in the
> initial charger cable type detection giving a wrong result and we end up
> not charging or charging at only 0.5A.
> 
> This commit starts a second charger-detection cycle a couple of seconds
> after the first one finishes, giving the other drivers time to load and
> do their thing.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/extcon/extcon-axp288.c | 32 ++++++++++++++++++++++++++++++--
>  1 file changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/extcon/extcon-axp288.c b/drivers/extcon/extcon-axp288.c
> index 386afb7d1160..cc7c35c7ff02 100644
> --- a/drivers/extcon/extcon-axp288.c
> +++ b/drivers/extcon/extcon-axp288.c
> @@ -1,6 +1,7 @@
>  /*
>   * extcon-axp288.c - X-Power AXP288 PMIC extcon cable detection driver
>   *
> + * Copyright (C) 2016-2017 Hans de Goede <hdegoede@redhat.com>
>   * Copyright (C) 2015 Intel Corporation
>   * Author: Ramakrishna Pallala <ramakrishna.pallala@intel.com>
>   *
> @@ -97,9 +98,11 @@ struct axp288_extcon_info {
>  	struct device *dev;
>  	struct regmap *regmap;
>  	struct regmap_irq_chip_data *regmap_irqc;
> +	struct delayed_work det_work;
>  	int irq[EXTCON_IRQ_END];
>  	struct extcon_dev *edev;
>  	unsigned int previous_cable;
> +	bool first_detect_done;

The first_detect_done is used only one time in the axp288_handle_chrg_det_event().
The other function don't use it. So, you better to define and use
'static bool first_detect_done' in the axp288_handle_chrg_det_event()
instead of defining the 'first_detect_done' in the struct axp288_extcon_info.

>  };
>  
>  /* Power up/down reason string array */
> @@ -137,6 +140,25 @@ static void axp288_extcon_log_rsi(struct axp288_extcon_info *info)
>  	regmap_write(info->regmap, AXP288_PS_BOOT_REASON_REG, clear_mask);
>  }
>  
> +static void axp288_chrg_detect_complete(struct axp288_extcon_info *info)
> +{
> +	/*
> +	 * We depend on other drivers to do things like mux the data lines,
> +	 * enable/disable vbus based on the id-pin, etc. Sometimes the BIOS has
> +	 * not set these things up correctly resulting in the initial charger
> +	 * cable type detection giving a wrong result and we end up not charging
> +	 * or charging at only 0.5A.
> +	 *
> +	 * So we schedule a second cable type detection after 2 seconds to
> +	 * give the other drivers time to load and do their thing.
> +	 */
> +	if (!info->first_detect_done) {
> +		queue_delayed_work(system_wq, &info->det_work,
> +				   msecs_to_jiffies(2000));
> +		info->first_detect_done = true;
> +	}
> +}

The axp288_chrg_detect_complete() is only used in the axp288_handle_chrg_det_event()
and axp288_chrg_detect_complete() is not a complicate. I think that you don't need
to make the separate function. I'd like you to add the 

> +
>  static int axp288_handle_chrg_det_event(struct axp288_extcon_info *info)
>  {
>  	int ret, stat, cfg, pwr_stat;
> @@ -201,6 +223,8 @@ static int axp288_handle_chrg_det_event(struct axp288_extcon_info *info)
>  		info->previous_cable = cable;
>  	}
>  
> +	axp288_chrg_detect_complete(info);

As I commented, you better to add the code directly instead of separate function.

> +
>  	return 0;
>  
>  dev_det_ret:
> @@ -222,8 +246,11 @@ static irqreturn_t axp288_extcon_isr(int irq, void *data)
>  	return IRQ_HANDLED;
>  }
>  
> -static void axp288_extcon_enable(struct axp288_extcon_info *info)
> +static void axp288_extcon_det_work(struct work_struct *work)
>  {
> +	struct axp288_extcon_info *info =
> +		container_of(work, struct axp288_extcon_info, det_work.work);
> +
>  	regmap_update_bits(info->regmap, AXP288_BC_GLOBAL_REG,
>  						BC_GLOBAL_RUN, 0);
>  	/* Enable the charger detection logic */
> @@ -245,6 +272,7 @@ static int axp288_extcon_probe(struct platform_device *pdev)
>  	info->regmap = axp20x->regmap;
>  	info->regmap_irqc = axp20x->regmap_irqc;
>  	info->previous_cable = EXTCON_NONE;
> +	INIT_DELAYED_WORK(&info->det_work, axp288_extcon_det_work);
>  
>  	platform_set_drvdata(pdev, info);
>  
> @@ -287,7 +315,7 @@ static int axp288_extcon_probe(struct platform_device *pdev)
>  	}
>  
>  	/* Start charger cable type detection */
> -	axp288_extcon_enable(info);
> +	queue_delayed_work(system_wq, &info->det_work, 0);
>  
>  	return 0;
>  }
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Re: [PATCH 4/4] extcon: axp288: Handle reserved charger-type values better

[Chanwoo Choi]

Hi Hans,

On 2017년 12월 22일 21:36, Hans de Goede wrote:
> According to the data sheets all the values not handled in the
> switch-case are "reserved". Update the dev_warn message to reflect
> this and set the cable-type to EXTCON_CHG_USB_SDP (so max 500mA
> current draw) as safe default.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/extcon/extcon-axp288.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/extcon/extcon-axp288.c b/drivers/extcon/extcon-axp288.c
> index cc7c35c7ff02..d60c615f709f 100644
> --- a/drivers/extcon/extcon-axp288.c
> +++ b/drivers/extcon/extcon-axp288.c
> @@ -205,8 +205,8 @@ static int axp288_handle_chrg_det_event(struct axp288_extcon_info *info)
>  		cable = EXTCON_CHG_USB_DCP;
>  		break;
>  	default:
> -		dev_warn(info->dev,
> -			"disconnect or unknown or ID event\n");
> +		dev_warn(info->dev, "unknown (reserved) bc detect result\n");
> +		cable = EXTCON_CHG_USB_SDP;
>  	}
>  
>  no_vbus:
> 

Looks good to me.
Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Re: [PATCH 2/4] extcon: axp288: Remove unused platform data

[Lee Jones]

On Tue, 02 Jan 2018, Chanwoo Choi wrote:

> + MFD Maintainer (Lee Jones <lee.jones@linaro.org>)
> 
> Hi Hans,
> 
> You better to use the scripts/get_maintainer.pl for the mailing list.
> I added the MFD maintainer.
> 
> This patch looks good to me.
> Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
> 
> 
> Best Regards,
> Chanwoo Choi
> 
> On 2017년 12월 22일 21:36, Hans de Goede wrote:
> > This is not used / set anywhere in the tree.
> > 
> > Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> > ---
> >  drivers/extcon/extcon-axp288.c | 35 +----------------------------------
> >  include/linux/mfd/axp20x.h     |  5 -----

Acked-by: Lee Jones <lee.jones@linaro.org>

-- 
Lee Jones
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 3/4] extcon: axp288: Redo charger type dection a couple of seconds after probe()

[Hans de Goede]

Hi,

On 02-01-18 01:54, Chanwoo Choi wrote:
> Hi Hans,
> 
> s/dection/detection on patch title.

Thank you for all the reviews.

I've fixed the typo in my personal tree.

> On 2017년 12월 22일 21:36, Hans de Goede wrote:
>> The axp288 extcon code depends on other drivers to do things like mux the
>> data lines, enable/disable vbus based on the id-pin, etc.
>>
>> Sometimes the BIOS has not set these things up correctly resulting in the
>> initial charger cable type detection giving a wrong result and we end up
>> not charging or charging at only 0.5A.
>>
>> This commit starts a second charger-detection cycle a couple of seconds
>> after the first one finishes, giving the other drivers time to load and
>> do their thing.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   drivers/extcon/extcon-axp288.c | 32 ++++++++++++++++++++++++++++++--
>>   1 file changed, 30 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/extcon/extcon-axp288.c b/drivers/extcon/extcon-axp288.c
>> index 386afb7d1160..cc7c35c7ff02 100644
>> --- a/drivers/extcon/extcon-axp288.c
>> +++ b/drivers/extcon/extcon-axp288.c
>> @@ -1,6 +1,7 @@
>>   /*
>>    * extcon-axp288.c - X-Power AXP288 PMIC extcon cable detection driver
>>    *
>> + * Copyright (C) 2016-2017 Hans de Goede <hdegoede@redhat.com>
>>    * Copyright (C) 2015 Intel Corporation
>>    * Author: Ramakrishna Pallala <ramakrishna.pallala@intel.com>
>>    *
>> @@ -97,9 +98,11 @@ struct axp288_extcon_info {
>>   	struct device *dev;
>>   	struct regmap *regmap;
>>   	struct regmap_irq_chip_data *regmap_irqc;
>> +	struct delayed_work det_work;
>>   	int irq[EXTCON_IRQ_END];
>>   	struct extcon_dev *edev;
>>   	unsigned int previous_cable;
>> +	bool first_detect_done;
> 
> The first_detect_done is used only one time in the axp288_handle_chrg_det_event().
> The other function don't use it. So, you better to define and use
> 'static bool first_detect_done' in the axp288_handle_chrg_det_event()
> instead of defining the 'first_detect_done' in the struct axp288_extcon_info.

But what if a device has 2 axp288 PMICs (unlikely I know) then only the
first one to check this will do the re-detect 2 seconds later, unless they
race, which is bad in itself too.

In general using static function variables is a bad idea and should be
avoided, it does not work when their are multiple instances of the device
and it is racy. So sorry but I'm not going to make this change.

> 
>>   };
>>   
>>   /* Power up/down reason string array */
>> @@ -137,6 +140,25 @@ static void axp288_extcon_log_rsi(struct axp288_extcon_info *info)
>>   	regmap_write(info->regmap, AXP288_PS_BOOT_REASON_REG, clear_mask);
>>   }
>>   
>> +static void axp288_chrg_detect_complete(struct axp288_extcon_info *info)
>> +{
>> +	/*
>> +	 * We depend on other drivers to do things like mux the data lines,
>> +	 * enable/disable vbus based on the id-pin, etc. Sometimes the BIOS has
>> +	 * not set these things up correctly resulting in the initial charger
>> +	 * cable type detection giving a wrong result and we end up not charging
>> +	 * or charging at only 0.5A.
>> +	 *
>> +	 * So we schedule a second cable type detection after 2 seconds to
>> +	 * give the other drivers time to load and do their thing.
>> +	 */
>> +	if (!info->first_detect_done) {
>> +		queue_delayed_work(system_wq, &info->det_work,
>> +				   msecs_to_jiffies(2000));
>> +		info->first_detect_done = true;
>> +	}
>> +}
> 
> The axp288_chrg_detect_complete() is only used in the axp288_handle_chrg_det_event()
> and axp288_chrg_detect_complete() is not a complicate. I think that you don't need
> to make the separate function. I'd like you to add the
> 
>> +
>>   static int axp288_handle_chrg_det_event(struct axp288_extcon_info *info)
>>   {
>>   	int ret, stat, cfg, pwr_stat;
>> @@ -201,6 +223,8 @@ static int axp288_handle_chrg_det_event(struct axp288_extcon_info *info)
>>   		info->previous_cable = cable;
>>   	}
>>   
>> +	axp288_chrg_detect_complete(info);
> 
> As I commented, you better to add the code directly instead of separate function.

I would prefer to keep this as a separate function, that keeps the main
flow of the axp288_handle_chrg_det_event function a lot more readable IMHO.

axp288_handle_chrg_det_event already has a non trivial code flow adding more
conditional code to it only makes it harder to read.

But if you insist I can move the code inside the function for v2. Note that
this will not make a difference for the code generated by the compiler, the
compiler will auto-inline it anyways.


> 
>> +
>>   	return 0;
>>   
>>   dev_det_ret:
>> @@ -222,8 +246,11 @@ static irqreturn_t axp288_extcon_isr(int irq, void *data)
>>   	return IRQ_HANDLED;
>>   }
>>   
>> -static void axp288_extcon_enable(struct axp288_extcon_info *info)
>> +static void axp288_extcon_det_work(struct work_struct *work)
>>   {
>> +	struct axp288_extcon_info *info =
>> +		container_of(work, struct axp288_extcon_info, det_work.work);
>> +
>>   	regmap_update_bits(info->regmap, AXP288_BC_GLOBAL_REG,
>>   						BC_GLOBAL_RUN, 0);
>>   	/* Enable the charger detection logic */
>> @@ -245,6 +272,7 @@ static int axp288_extcon_probe(struct platform_device *pdev)
>>   	info->regmap = axp20x->regmap;
>>   	info->regmap_irqc = axp20x->regmap_irqc;
>>   	info->previous_cable = EXTCON_NONE;
>> +	INIT_DELAYED_WORK(&info->det_work, axp288_extcon_det_work);
>>   
>>   	platform_set_drvdata(pdev, info);
>>   
>> @@ -287,7 +315,7 @@ static int axp288_extcon_probe(struct platform_device *pdev)
>>   	}
>>   
>>   	/* Start charger cable type detection */
>> -	axp288_extcon_enable(info);
>> +	queue_delayed_work(system_wq, &info->det_work, 0);
>>   
>>   	return 0;
>>   }
>>
> 
> 

Regards,

Hans

Re: [PATCH 3/4] extcon: axp288: Redo charger type dection a couple of seconds after probe()

[Chanwoo Choi]

Hi Hans,

On 2018년 01월 03일 07:44, Hans de Goede wrote:
> Hi,
> 
> On 02-01-18 01:54, Chanwoo Choi wrote:
>> Hi Hans,
>>
>> s/dection/detection on patch title.
> 
> Thank you for all the reviews.
> 
> I've fixed the typo in my personal tree.
> 
>> On 2017년 12월 22일 21:36, Hans de Goede wrote:
>>> The axp288 extcon code depends on other drivers to do things like mux the
>>> data lines, enable/disable vbus based on the id-pin, etc.
>>>
>>> Sometimes the BIOS has not set these things up correctly resulting in the
>>> initial charger cable type detection giving a wrong result and we end up
>>> not charging or charging at only 0.5A.
>>>
>>> This commit starts a second charger-detection cycle a couple of seconds
>>> after the first one finishes, giving the other drivers time to load and
>>> do their thing.
>>>
>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>> ---
>>>   drivers/extcon/extcon-axp288.c | 32 ++++++++++++++++++++++++++++++--
>>>   1 file changed, 30 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/extcon/extcon-axp288.c b/drivers/extcon/extcon-axp288.c
>>> index 386afb7d1160..cc7c35c7ff02 100644
>>> --- a/drivers/extcon/extcon-axp288.c
>>> +++ b/drivers/extcon/extcon-axp288.c
>>> @@ -1,6 +1,7 @@
>>>   /*
>>>    * extcon-axp288.c - X-Power AXP288 PMIC extcon cable detection driver
>>>    *
>>> + * Copyright (C) 2016-2017 Hans de Goede <hdegoede@redhat.com>
>>>    * Copyright (C) 2015 Intel Corporation
>>>    * Author: Ramakrishna Pallala <ramakrishna.pallala@intel.com>
>>>    *
>>> @@ -97,9 +98,11 @@ struct axp288_extcon_info {
>>>       struct device *dev;
>>>       struct regmap *regmap;
>>>       struct regmap_irq_chip_data *regmap_irqc;
>>> +    struct delayed_work det_work;
>>>       int irq[EXTCON_IRQ_END];
>>>       struct extcon_dev *edev;
>>>       unsigned int previous_cable;
>>> +    bool first_detect_done;
>>
>> The first_detect_done is used only one time in the axp288_handle_chrg_det_event().
>> The other function don't use it. So, you better to define and use
>> 'static bool first_detect_done' in the axp288_handle_chrg_det_event()
>> instead of defining the 'first_detect_done' in the struct axp288_extcon_info.
> 
> But what if a device has 2 axp288 PMICs (unlikely I know) then only the
> first one to check this will do the re-detect 2 seconds later, unless they
> race, which is bad in itself too.
> 
> In general using static function variables is a bad idea and should be
> avoided, it does not work when their are multiple instances of the device
> and it is racy. So sorry but I'm not going to make this change.

You're right. It is my mistake. Please keep your way.

> 
>>
>>>   };
>>>     /* Power up/down reason string array */
>>> @@ -137,6 +140,25 @@ static void axp288_extcon_log_rsi(struct axp288_extcon_info *info)
>>>       regmap_write(info->regmap, AXP288_PS_BOOT_REASON_REG, clear_mask);
>>>   }
>>>   +static void axp288_chrg_detect_complete(struct axp288_extcon_info *info)
>>> +{
>>> +    /*
>>> +     * We depend on other drivers to do things like mux the data lines,
>>> +     * enable/disable vbus based on the id-pin, etc. Sometimes the BIOS has
>>> +     * not set these things up correctly resulting in the initial charger
>>> +     * cable type detection giving a wrong result and we end up not charging
>>> +     * or charging at only 0.5A.
>>> +     *
>>> +     * So we schedule a second cable type detection after 2 seconds to
>>> +     * give the other drivers time to load and do their thing.
>>> +     */
>>> +    if (!info->first_detect_done) {
>>> +        queue_delayed_work(system_wq, &info->det_work,
>>> +                   msecs_to_jiffies(2000));
>>> +        info->first_detect_done = true;
>>> +    }
>>> +}
>>
>> The axp288_chrg_detect_complete() is only used in the axp288_handle_chrg_det_event()
>> and axp288_chrg_detect_complete() is not a complicate. I think that you don't need
>> to make the separate function. I'd like you to add the
>>
>>> +
>>>   static int axp288_handle_chrg_det_event(struct axp288_extcon_info *info)
>>>   {
>>>       int ret, stat, cfg, pwr_stat;
>>> @@ -201,6 +223,8 @@ static int axp288_handle_chrg_det_event(struct axp288_extcon_info *info)
>>>           info->previous_cable = cable;
>>>       }
>>>   +    axp288_chrg_detect_complete(info);
>>
>> As I commented, you better to add the code directly instead of separate function.
> 
> I would prefer to keep this as a separate function, that keeps the main
> flow of the axp288_handle_chrg_det_event function a lot more readable IMHO.
> 
> axp288_handle_chrg_det_event already has a non trivial code flow adding more
> conditional code to it only makes it harder to read.
> 
> But if you insist I can move the code inside the function for v2. Note that
> this will not make a difference for the code generated by the compiler, the
> compiler will auto-inline it anyways.

I didn't mention the result from compiler. I focus on the readability.
On v1, the developer always check what to do in axp288_chrg_detect_complete()
even if this function is used only one time like '__init'.
Actually, axp288_chrg_detect_complete() is not complicate and short.

But, I will not be forced because it is not a critical issue.
If you want to keep the separate function, you just send v2 with only fixing the typo.

> 
> 
>>
>>> +
>>>       return 0;
>>>     dev_det_ret:
>>> @@ -222,8 +246,11 @@ static irqreturn_t axp288_extcon_isr(int irq, void *data)
>>>       return IRQ_HANDLED;
>>>   }
>>>   -static void axp288_extcon_enable(struct axp288_extcon_info *info)
>>> +static void axp288_extcon_det_work(struct work_struct *work)
>>>   {
>>> +    struct axp288_extcon_info *info =
>>> +        container_of(work, struct axp288_extcon_info, det_work.work);
>>> +
>>>       regmap_update_bits(info->regmap, AXP288_BC_GLOBAL_REG,
>>>                           BC_GLOBAL_RUN, 0);
>>>       /* Enable the charger detection logic */
>>> @@ -245,6 +272,7 @@ static int axp288_extcon_probe(struct platform_device *pdev)
>>>       info->regmap = axp20x->regmap;
>>>       info->regmap_irqc = axp20x->regmap_irqc;
>>>       info->previous_cable = EXTCON_NONE;
>>> +    INIT_DELAYED_WORK(&info->det_work, axp288_extcon_det_work);
>>>         platform_set_drvdata(pdev, info);
>>>   @@ -287,7 +315,7 @@ static int axp288_extcon_probe(struct platform_device *pdev)
>>>       }
>>>         /* Start charger cable type detection */
>>> -    axp288_extcon_enable(info);
>>> +    queue_delayed_work(system_wq, &info->det_work, 0);
>>>         return 0;
>>>   }
>>>
>>
>>
> 
> Regards,
> 
> Hans
> 
> 
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Re: [PATCH 3/4] extcon: axp288: Redo charger type dection a couple of seconds after probe()

[Chanwoo Choi]

Hi Hans,

On 2018년 01월 03일 09:58, Chanwoo Choi wrote:
> Hi Hans,
> 
> On 2018년 01월 03일 07:44, Hans de Goede wrote:
>> Hi,
>>
>> On 02-01-18 01:54, Chanwoo Choi wrote:
>>> Hi Hans,
>>>
>>> s/dection/detection on patch title.
>>
>> Thank you for all the reviews.
>>
>> I've fixed the typo in my personal tree.
>>
>>> On 2017년 12월 22일 21:36, Hans de Goede wrote:
>>>> The axp288 extcon code depends on other drivers to do things like mux the
>>>> data lines, enable/disable vbus based on the id-pin, etc.
>>>>
>>>> Sometimes the BIOS has not set these things up correctly resulting in the
>>>> initial charger cable type detection giving a wrong result and we end up
>>>> not charging or charging at only 0.5A.
>>>>
>>>> This commit starts a second charger-detection cycle a couple of seconds
>>>> after the first one finishes, giving the other drivers time to load and
>>>> do their thing.
>>>>
>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>> ---
>>>>   drivers/extcon/extcon-axp288.c | 32 ++++++++++++++++++++++++++++++--
>>>>   1 file changed, 30 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/extcon/extcon-axp288.c b/drivers/extcon/extcon-axp288.c
>>>> index 386afb7d1160..cc7c35c7ff02 100644
>>>> --- a/drivers/extcon/extcon-axp288.c
>>>> +++ b/drivers/extcon/extcon-axp288.c
>>>> @@ -1,6 +1,7 @@
>>>>   /*
>>>>    * extcon-axp288.c - X-Power AXP288 PMIC extcon cable detection driver
>>>>    *
>>>> + * Copyright (C) 2016-2017 Hans de Goede <hdegoede@redhat.com>
>>>>    * Copyright (C) 2015 Intel Corporation
>>>>    * Author: Ramakrishna Pallala <ramakrishna.pallala@intel.com>
>>>>    *
>>>> @@ -97,9 +98,11 @@ struct axp288_extcon_info {
>>>>       struct device *dev;
>>>>       struct regmap *regmap;
>>>>       struct regmap_irq_chip_data *regmap_irqc;
>>>> +    struct delayed_work det_work;
>>>>       int irq[EXTCON_IRQ_END];
>>>>       struct extcon_dev *edev;
>>>>       unsigned int previous_cable;
>>>> +    bool first_detect_done;
>>>
>>> The first_detect_done is used only one time in the axp288_handle_chrg_det_event().
>>> The other function don't use it. So, you better to define and use
>>> 'static bool first_detect_done' in the axp288_handle_chrg_det_event()
>>> instead of defining the 'first_detect_done' in the struct axp288_extcon_info.
>>
>> But what if a device has 2 axp288 PMICs (unlikely I know) then only the
>> first one to check this will do the re-detect 2 seconds later, unless they
>> race, which is bad in itself too.
>>
>> In general using static function variables is a bad idea and should be
>> avoided, it does not work when their are multiple instances of the device
>> and it is racy. So sorry but I'm not going to make this change.
> 
> You're right. It is my mistake. Please keep your way.
> 
>>
>>>
>>>>   };
>>>>     /* Power up/down reason string array */
>>>> @@ -137,6 +140,25 @@ static void axp288_extcon_log_rsi(struct axp288_extcon_info *info)
>>>>       regmap_write(info->regmap, AXP288_PS_BOOT_REASON_REG, clear_mask);
>>>>   }
>>>>   +static void axp288_chrg_detect_complete(struct axp288_extcon_info *info)
>>>> +{
>>>> +    /*
>>>> +     * We depend on other drivers to do things like mux the data lines,
>>>> +     * enable/disable vbus based on the id-pin, etc. Sometimes the BIOS has
>>>> +     * not set these things up correctly resulting in the initial charger
>>>> +     * cable type detection giving a wrong result and we end up not charging
>>>> +     * or charging at only 0.5A.
>>>> +     *
>>>> +     * So we schedule a second cable type detection after 2 seconds to
>>>> +     * give the other drivers time to load and do their thing.
>>>> +     */
>>>> +    if (!info->first_detect_done) {
>>>> +        queue_delayed_work(system_wq, &info->det_work,
>>>> +                   msecs_to_jiffies(2000));
>>>> +        info->first_detect_done = true;
>>>> +    }
>>>> +}
>>>
>>> The axp288_chrg_detect_complete() is only used in the axp288_handle_chrg_det_event()
>>> and axp288_chrg_detect_complete() is not a complicate. I think that you don't need
>>> to make the separate function. I'd like you to add the
>>>
>>>> +
>>>>   static int axp288_handle_chrg_det_event(struct axp288_extcon_info *info)
>>>>   {
>>>>       int ret, stat, cfg, pwr_stat;
>>>> @@ -201,6 +223,8 @@ static int axp288_handle_chrg_det_event(struct axp288_extcon_info *info)
>>>>           info->previous_cable = cable;
>>>>       }
>>>>   +    axp288_chrg_detect_complete(info);
>>>
>>> As I commented, you better to add the code directly instead of separate function.
>>
>> I would prefer to keep this as a separate function, that keeps the main
>> flow of the axp288_handle_chrg_det_event function a lot more readable IMHO.
>>
>> axp288_handle_chrg_det_event already has a non trivial code flow adding more
>> conditional code to it only makes it harder to read.
>>
>> But if you insist I can move the code inside the function for v2. Note that
>> this will not make a difference for the code generated by the compiler, the
>> compiler will auto-inline it anyways.
> 
> I didn't mention the result from compiler. I focus on the readability.
> On v1, the developer always check what to do in axp288_chrg_detect_complete()
> even if this function is used only one time like '__init'.
> Actually, axp288_chrg_detect_complete() is not complicate and short.
> 
> But, I will not be forced because it is not a critical issue.
> If you want to keep the separate function, you just send v2 with only fixing the typo.

You don't need to send v2. I'll fix the typo and apply your patch v1
As I mentioned, it is not a critical issue. Thanks.

> 
>>
>>
>>>
>>>> +
>>>>       return 0;
>>>>     dev_det_ret:
>>>> @@ -222,8 +246,11 @@ static irqreturn_t axp288_extcon_isr(int irq, void *data)
>>>>       return IRQ_HANDLED;
>>>>   }
>>>>   -static void axp288_extcon_enable(struct axp288_extcon_info *info)
>>>> +static void axp288_extcon_det_work(struct work_struct *work)
>>>>   {
>>>> +    struct axp288_extcon_info *info =
>>>> +        container_of(work, struct axp288_extcon_info, det_work.work);
>>>> +
>>>>       regmap_update_bits(info->regmap, AXP288_BC_GLOBAL_REG,
>>>>                           BC_GLOBAL_RUN, 0);
>>>>       /* Enable the charger detection logic */
>>>> @@ -245,6 +272,7 @@ static int axp288_extcon_probe(struct platform_device *pdev)
>>>>       info->regmap = axp20x->regmap;
>>>>       info->regmap_irqc = axp20x->regmap_irqc;
>>>>       info->previous_cable = EXTCON_NONE;
>>>> +    INIT_DELAYED_WORK(&info->det_work, axp288_extcon_det_work);
>>>>         platform_set_drvdata(pdev, info);
>>>>   @@ -287,7 +315,7 @@ static int axp288_extcon_probe(struct platform_device *pdev)
>>>>       }
>>>>         /* Start charger cable type detection */
>>>> -    axp288_extcon_enable(info);
>>>> +    queue_delayed_work(system_wq, &info->det_work, 0);
>>>>         return 0;
>>>>   }
>>>>
>>>
>>>
>>
>> Regards,
>>
>> Hans
>>
>>
>>
> 
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Re: [PATCH 1/4] extcon: axp288: Remove unused extcon_nb struct member

[Chanwoo Choi]

Hi Hans,
On 2017년 12월 22일 21:36, Hans de Goede wrote:
> Remove the unused extcon_nb struct member.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/extcon/extcon-axp288.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/extcon/extcon-axp288.c b/drivers/extcon/extcon-axp288.c
> index 981fba56bc18..ba185e9ebb82 100644
> --- a/drivers/extcon/extcon-axp288.c
> +++ b/drivers/extcon/extcon-axp288.c
> @@ -107,7 +107,6 @@ struct axp288_extcon_info {
>  	struct gpio_desc *gpio_mux_cntl;
>  	int irq[EXTCON_IRQ_END];
>  	struct extcon_dev *edev;
> -	struct notifier_block extcon_nb;
>  	unsigned int previous_cable;
>  };
>  
> 

Applied them(patch1/2/3/4). Thanks.

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Re: [PATCH 2/4] extcon: axp288: Remove unused platform data

[Chanwoo Choi]

Dear Lee,

On 2018년 01월 02일 18:16, Lee Jones wrote:
> On Tue, 02 Jan 2018, Chanwoo Choi wrote:
> 
>> + MFD Maintainer (Lee Jones <lee.jones@linaro.org>)
>>
>> Hi Hans,
>>
>> You better to use the scripts/get_maintainer.pl for the mailing list.
>> I added the MFD maintainer.
>>
>> This patch looks good to me.
>> Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
>>
>>
>> Best Regards,
>> Chanwoo Choi
>>
>> On 2017년 12월 22일 21:36, Hans de Goede wrote:
>>> This is not used / set anywhere in the tree.
>>>
>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>> ---
>>>  drivers/extcon/extcon-axp288.c | 35 +----------------------------------
>>>  include/linux/mfd/axp20x.h     |  5 -----
> 
> Acked-by: Lee Jones <lee.jones@linaro.org>
> 

I already made the immutable branch[1] for both mfd and extcon.
[1] https://lkml.org/lkml/2017/12/15/133

I added the additional patch for 'include/linux/mfd/axp20x.h'
If you want to need to apply it on mfd tree, please apply them of immutable branch.


The following changes since commit 4fbd8d194f06c8a3fd2af1ce560ddb31f7ec8323:

  Linux 4.15-rc1 (2017-11-26 16:01:47 -0800)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/extcon.git ib-extcon-mfd-4.16

for you to fetch changes up to 9bf317e900a19a857eb9921c9441a92e89f40415:

  extcon: axp288: Remove unused platform data (2018-01-03 10:11:02 +0900)

----------------------------------------------------------------
Arvind Yadav (1):
      extcon: axp288:: Handle return value of platform_get_irq

Benson Leung (1):
      extcon: usbc-cros-ec: add support to notify USB type cables.

Hans de Goede (2):
      extcon: axp288: Remove unused extcon_nb struct member
      extcon: axp288: Remove unused platform data

 drivers/extcon/extcon-axp288.c       |  39 +---------
 drivers/extcon/extcon-usbc-cros-ec.c | 142 ++++++++++++++++++++++++++++++++++-
 include/linux/mfd/axp20x.h           |   5 --
 include/linux/mfd/cros_ec_commands.h |  17 +++++
 4 files changed, 159 insertions(+), 44 deletions(-)

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Re: [PATCH 1/4] extcon: axp288: Remove unused extcon_nb struct member

[Hans de Goede]

Hi,

On 03-01-18 02:17, Chanwoo Choi wrote:
> Hi Hans,
> On 2017년 12월 22일 21:36, Hans de Goede wrote:
>> Remove the unused extcon_nb struct member.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   drivers/extcon/extcon-axp288.c | 1 -
>>   1 file changed, 1 deletion(-)
>>
>> diff --git a/drivers/extcon/extcon-axp288.c b/drivers/extcon/extcon-axp288.c
>> index 981fba56bc18..ba185e9ebb82 100644
>> --- a/drivers/extcon/extcon-axp288.c
>> +++ b/drivers/extcon/extcon-axp288.c
>> @@ -107,7 +107,6 @@ struct axp288_extcon_info {
>>   	struct gpio_desc *gpio_mux_cntl;
>>   	int irq[EXTCON_IRQ_END];
>>   	struct extcon_dev *edev;
>> -	struct notifier_block extcon_nb;
>>   	unsigned int previous_cable;
>>   };
>>   
>>
> 
> Applied them(patch1/2/3/4). Thanks.

Great, thank you.

Regards,

Hans

Re: [PATCH 2/4] extcon: axp288: Remove unused platform data

[Lee Jones]

On Wed, 03 Jan 2018, Chanwoo Choi wrote:

> Dear Lee,
> 
> On 2018년 01월 02일 18:16, Lee Jones wrote:
> > On Tue, 02 Jan 2018, Chanwoo Choi wrote:
> > 
> >> + MFD Maintainer (Lee Jones <lee.jones@linaro.org>)
> >>
> >> Hi Hans,
> >>
> >> You better to use the scripts/get_maintainer.pl for the mailing list.
> >> I added the MFD maintainer.
> >>
> >> This patch looks good to me.
> >> Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
> >>
> >>
> >> Best Regards,
> >> Chanwoo Choi
> >>
> >> On 2017년 12월 22일 21:36, Hans de Goede wrote:
> >>> This is not used / set anywhere in the tree.
> >>>
> >>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> >>> ---
> >>>  drivers/extcon/extcon-axp288.c | 35 +----------------------------------
> >>>  include/linux/mfd/axp20x.h     |  5 -----
> > 
> > Acked-by: Lee Jones <lee.jones@linaro.org>
> > 
> 
> I already made the immutable branch[1] for both mfd and extcon.
> [1] https://lkml.org/lkml/2017/12/15/133
> 
> I added the additional patch for 'include/linux/mfd/axp20x.h'
> If you want to need to apply it on mfd tree, please apply them of immutable branch.
> 
> 
> The following changes since commit 4fbd8d194f06c8a3fd2af1ce560ddb31f7ec8323:
> 
>   Linux 4.15-rc1 (2017-11-26 16:01:47 -0800)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/extcon.git ib-extcon-mfd-4.16

How is this possible?  ib-extcon-mfd-4.16 is already immutable.

I believe you should have created a new tag:

  ib-extcon-mfd-4.16-1

... based on ib-extcon-mfd-4.16.

> for you to fetch changes up to 9bf317e900a19a857eb9921c9441a92e89f40415:
> 
>   extcon: axp288: Remove unused platform data (2018-01-03 10:11:02 +0900)
> 
> ----------------------------------------------------------------
> Arvind Yadav (1):
>       extcon: axp288:: Handle return value of platform_get_irq
> 
> Benson Leung (1):
>       extcon: usbc-cros-ec: add support to notify USB type cables.
> 
> Hans de Goede (2):
>       extcon: axp288: Remove unused extcon_nb struct member
>       extcon: axp288: Remove unused platform data
> 
>  drivers/extcon/extcon-axp288.c       |  39 +---------
>  drivers/extcon/extcon-usbc-cros-ec.c | 142 ++++++++++++++++++++++++++++++++++-
>  include/linux/mfd/axp20x.h           |   5 --
>  include/linux/mfd/cros_ec_commands.h |  17 +++++
>  4 files changed, 159 insertions(+), 44 deletions(-)
> 

-- 
Lee Jones
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 2/4] extcon: axp288: Remove unused platform data

[Chanwoo Choi]

On 2018년 01월 03일 18:37, Lee Jones wrote:
> On Wed, 03 Jan 2018, Chanwoo Choi wrote:
> 
>> Dear Lee,
>>
>> On 2018년 01월 02일 18:16, Lee Jones wrote:
>>> On Tue, 02 Jan 2018, Chanwoo Choi wrote:
>>>
>>>> + MFD Maintainer (Lee Jones <lee.jones@linaro.org>)
>>>>
>>>> Hi Hans,
>>>>
>>>> You better to use the scripts/get_maintainer.pl for the mailing list.
>>>> I added the MFD maintainer.
>>>>
>>>> This patch looks good to me.
>>>> Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
>>>>
>>>>
>>>> Best Regards,
>>>> Chanwoo Choi
>>>>
>>>> On 2017년 12월 22일 21:36, Hans de Goede wrote:
>>>>> This is not used / set anywhere in the tree.
>>>>>
>>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>>> ---
>>>>>  drivers/extcon/extcon-axp288.c | 35 +----------------------------------
>>>>>  include/linux/mfd/axp20x.h     |  5 -----
>>>
>>> Acked-by: Lee Jones <lee.jones@linaro.org>
>>>
>>
>> I already made the immutable branch[1] for both mfd and extcon.
>> [1] https://lkml.org/lkml/2017/12/15/133
>>
>> I added the additional patch for 'include/linux/mfd/axp20x.h'
>> If you want to need to apply it on mfd tree, please apply them of immutable branch.
>>
>>
>> The following changes since commit 4fbd8d194f06c8a3fd2af1ce560ddb31f7ec8323:
>>
>>   Linux 4.15-rc1 (2017-11-26 16:01:47 -0800)
>>
>> are available in the git repository at:
>>
>>   git://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/extcon.git ib-extcon-mfd-4.16
> 
> How is this possible?  ib-extcon-mfd-4.16 is already immutable.
> 
> I believe you should have created a new tag:
> 
>   ib-extcon-mfd-4.16-1
> 
> ... based on ib-extcon-mfd-4.16.

You're right.
I make the new immutable branch (ib-extcon-mfd-4.16-1) as following:


The following changes since commit c7eb47f9e45226571be31212f6efd4b307d3b59d:

  extcon: usbc-cros-ec: add support to notify USB type cables. (2017-12-15 17:21:49 +0900)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/extcon.git tags/ib-extcon-mfd-4.16-1

for you to fetch changes up to 9bf317e900a19a857eb9921c9441a92e89f40415:

  extcon: axp288: Remove unused platform data (2018-01-03 10:11:02 +0900)

----------------------------------------------------------------
Immutable branch for both MFD and EXTCON tree.

----------------------------------------------------------------
Arvind Yadav (1):
      extcon: axp288:: Handle return value of platform_get_irq

Hans de Goede (2):
      extcon: axp288: Remove unused extcon_nb struct member
      extcon: axp288: Remove unused platform data

 drivers/extcon/extcon-axp288.c | 39 ++++-----------------------------------
 include/linux/mfd/axp20x.h     |  5 -----
 2 files changed, 4 insertions(+), 40 deletions(-)


> 
>> for you to fetch changes up to 9bf317e900a19a857eb9921c9441a92e89f40415:
>>
>>   extcon: axp288: Remove unused platform data (2018-01-03 10:11:02 +0900)
>>
>> ----------------------------------------------------------------
>> Arvind Yadav (1):
>>       extcon: axp288:: Handle return value of platform_get_irq
>>
>> Benson Leung (1):
>>       extcon: usbc-cros-ec: add support to notify USB type cables.
>>
>> Hans de Goede (2):
>>       extcon: axp288: Remove unused extcon_nb struct member
>>       extcon: axp288: Remove unused platform data
>>
>>  drivers/extcon/extcon-axp288.c       |  39 +---------
>>  drivers/extcon/extcon-usbc-cros-ec.c | 142 ++++++++++++++++++++++++++++++++++-
>>  include/linux/mfd/axp20x.h           |   5 --
>>  include/linux/mfd/cros_ec_commands.h |  17 +++++
>>  4 files changed, 159 insertions(+), 44 deletions(-)
>>
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Re: [PATCH 2/4] extcon: axp288: Remove unused platform data

[Lee Jones]

> The following changes since commit c7eb47f9e45226571be31212f6efd4b307d3b59d:
> 
>   extcon: usbc-cros-ec: add support to notify USB type cables. (2017-12-15 17:21:49 +0900)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/extcon.git tags/ib-extcon-mfd-4.16-1
> 
> for you to fetch changes up to 9bf317e900a19a857eb9921c9441a92e89f40415:
> 
>   extcon: axp288: Remove unused platform data (2018-01-03 10:11:02 +0900)
> 
> ----------------------------------------------------------------
> Immutable branch for both MFD and EXTCON tree.
> 
> ----------------------------------------------------------------
> Arvind Yadav (1):
>       extcon: axp288:: Handle return value of platform_get_irq
> 
> Hans de Goede (2):
>       extcon: axp288: Remove unused extcon_nb struct member
>       extcon: axp288: Remove unused platform data
> 
>  drivers/extcon/extcon-axp288.c | 39 ++++-----------------------------------
>  include/linux/mfd/axp20x.h     |  5 -----
>  2 files changed, 4 insertions(+), 40 deletions(-)

Pulled, thanks Chanwoo.

-- 
Lee Jones
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog