Re: [PATCH v12 4/4] power: wm831x_power: Support USB charger current limit management

[Mark Brown]

On Tue, Jun 21, 2016 at 02:50:27PM +0300, Felipe Balbi wrote:
> Mark Brown  writes:

> > The wm831x has no DT support currently.

> okay, perhaps its time to add it.

The only platform using it would need the DT connector overlays
completing in order to be able to convert to DT.  I'm really not
convinced it's worth the effort either.


signature.asc
Description: PGP signature

Re: [PATCH v12 4/4] power: wm831x_power: Support USB charger current limit management

[Felipe Balbi]

Hi,

Baolin Wang  writes:
> On 21 June 2016 at 19:03, Mark Brown  wrote:
>> On Tue, Jun 21, 2016 at 01:30:49PM +0300, Felipe Balbi wrote:
>>> Baolin Wang  writes:
>>> > @@ -607,8 +647,31 @@ static int wm831x_power_probe(struct platform_device 
>>> > *pdev)
>>> > }
>>> > }
>>
>>> > +   if (wm831x_pdata && wm831x_pdata->usb_gadget) {
>>> > +   power->usb_charger =
>>> > +   usb_charger_find_by_name(wm831x_pdata->usb_gadget);
>>
>>> the fact that you rely on strings and pass them via pdata is an
>>> indication that you don't have enough description of the HW. Seems like
>>> we need to come up with a set of DT properties which tie a charger to a
>>> UDC.
>>
>>> I'm thinking a phandle would be enough?
>>
>> The wm831x has no DT support currently.
>
> Another hand I suppose the usb charger is one virtual device and does
> not need be described from DT.

Right, I don't think that should be a device at all. But you can pass a
phandle to the UDC controller and use that to get to struct usb_gadget
from which you could reach ->charger.

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH v12 4/4] power: wm831x_power: Support USB charger current limit management

[Baolin Wang]

On 21 June 2016 at 19:53, Felipe Balbi  wrote:
>
> Hi,
>
> Baolin Wang  writes:
>> On 21 June 2016 at 19:03, Mark Brown  wrote:
>>> On Tue, Jun 21, 2016 at 01:30:49PM +0300, Felipe Balbi wrote:
 Baolin Wang  writes:
 > @@ -607,8 +647,31 @@ static int wm831x_power_probe(struct 
 > platform_device *pdev)
 > }
 > }
>>>
 > +   if (wm831x_pdata && wm831x_pdata->usb_gadget) {
 > +   power->usb_charger =
 > +   usb_charger_find_by_name(wm831x_pdata->usb_gadget);
>>>
 the fact that you rely on strings and pass them via pdata is an
 indication that you don't have enough description of the HW. Seems like
 we need to come up with a set of DT properties which tie a charger to a
 UDC.
>>>
 I'm thinking a phandle would be enough?
>>>
>>> The wm831x has no DT support currently.
>>
>> Another hand I suppose the usb charger is one virtual device and does
>> not need be described from DT.
>
> Right, I don't think that should be a device at all. But you can pass a
> phandle to the UDC controller and use that to get to struct usb_gadget
> from which you could reach ->charger.

Ah, make sense.

-- 
Baolin.wang
Best Regards
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v12 4/4] power: wm831x_power: Support USB charger current limit management

[Felipe Balbi]

Hi,

Mark Brown  writes:
> [ Unknown signature status ]
> On Tue, Jun 21, 2016 at 01:30:49PM +0300, Felipe Balbi wrote:
>> Baolin Wang  writes:
>> > @@ -607,8 +647,31 @@ static int wm831x_power_probe(struct platform_device 
>> > *pdev)
>> >}
>> >}
>
>> > +  if (wm831x_pdata && wm831x_pdata->usb_gadget) {
>> > +  power->usb_charger =
>> > +  usb_charger_find_by_name(wm831x_pdata->usb_gadget);
>
>> the fact that you rely on strings and pass them via pdata is an
>> indication that you don't have enough description of the HW. Seems like
>> we need to come up with a set of DT properties which tie a charger to a
>> UDC.
>
>> I'm thinking a phandle would be enough?
>
> The wm831x has no DT support currently.

okay, perhaps its time to add it.

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH v12 4/4] power: wm831x_power: Support USB charger current limit management

[Baolin Wang]

On 21 June 2016 at 19:03, Mark Brown  wrote:
> On Tue, Jun 21, 2016 at 01:30:49PM +0300, Felipe Balbi wrote:
>> Baolin Wang  writes:
>> > @@ -607,8 +647,31 @@ static int wm831x_power_probe(struct platform_device 
>> > *pdev)
>> > }
>> > }
>
>> > +   if (wm831x_pdata && wm831x_pdata->usb_gadget) {
>> > +   power->usb_charger =
>> > +   usb_charger_find_by_name(wm831x_pdata->usb_gadget);
>
>> the fact that you rely on strings and pass them via pdata is an
>> indication that you don't have enough description of the HW. Seems like
>> we need to come up with a set of DT properties which tie a charger to a
>> UDC.
>
>> I'm thinking a phandle would be enough?
>
> The wm831x has no DT support currently.

Another hand I suppose the usb charger is one virtual device and does
not need be described from DT.

-- 
Baolin.wang
Best Regards
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v12 4/4] power: wm831x_power: Support USB charger current limit management

[Mark Brown]

On Tue, Jun 21, 2016 at 01:30:49PM +0300, Felipe Balbi wrote:
> Baolin Wang  writes:
> > @@ -607,8 +647,31 @@ static int wm831x_power_probe(struct platform_device 
> > *pdev)
> > }
> > }

> > +   if (wm831x_pdata && wm831x_pdata->usb_gadget) {
> > +   power->usb_charger =
> > +   usb_charger_find_by_name(wm831x_pdata->usb_gadget);

> the fact that you rely on strings and pass them via pdata is an
> indication that you don't have enough description of the HW. Seems like
> we need to come up with a set of DT properties which tie a charger to a
> UDC.

> I'm thinking a phandle would be enough?

The wm831x has no DT support currently.


signature.asc
Description: PGP signature

Re: [PATCH v12 4/4] power: wm831x_power: Support USB charger current limit management

[Felipe Balbi]

Hi,

Baolin Wang  writes:
> @@ -607,8 +647,31 @@ static int wm831x_power_probe(struct platform_device 
> *pdev)
>   }
>   }
>  
> + if (wm831x_pdata && wm831x_pdata->usb_gadget) {
> + power->usb_charger =
> + usb_charger_find_by_name(wm831x_pdata->usb_gadget);

the fact that you rely on strings and pass them via pdata is an
indication that you don't have enough description of the HW. Seems like
we need to come up with a set of DT properties which tie a charger to a
UDC.

I'm thinking a phandle would be enough?

-- 
balbi


signature.asc
Description: PGP signature

[PATCH v12 4/4] power: wm831x_power: Support USB charger current limit management

[Baolin Wang]

Integrate with the newly added USB charger interface to limit the current
we draw from the USB input based on the input device configuration
identified by the USB stack, allowing us to charge more quickly from high
current inputs without drawing more current than specified from others.

Signed-off-by: Mark Brown 
Signed-off-by: Baolin Wang 
Acked-by: Lee Jones 
Acked-by: Charles Keepax 
Acked-by: Peter Chen 
Acked-by: Sebastian Reichel 
---
 drivers/power/wm831x_power.c |   69 ++
 include/linux/mfd/wm831x/pdata.h |3 ++
 2 files changed, 72 insertions(+)

diff --git a/drivers/power/wm831x_power.c b/drivers/power/wm831x_power.c
index 7082301..cef1812 100644
--- a/drivers/power/wm831x_power.c
+++ b/drivers/power/wm831x_power.c
@@ -13,6 +13,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -31,6 +32,8 @@ struct wm831x_power {
char usb_name[20];
char battery_name[20];
bool have_battery;
+   struct usb_charger *usb_charger;
+   struct notifier_block usb_notify;
 };
 
 static int wm831x_power_check_online(struct wm831x *wm831x, int supply,
@@ -125,6 +128,43 @@ static enum power_supply_property wm831x_usb_props[] = {
POWER_SUPPLY_PROP_VOLTAGE_NOW,
 };
 
+/* In milliamps */
+static const unsigned int wm831x_usb_limits[] = {
+   0,
+   2,
+   100,
+   500,
+   900,
+   1500,
+   1800,
+   550,
+};
+
+static int wm831x_usb_limit_change(struct notifier_block *nb,
+  unsigned long limit, void *data)
+{
+   struct wm831x_power *wm831x_power = container_of(nb,
+struct wm831x_power,
+usb_notify);
+   unsigned int i, best;
+
+   /* Find the highest supported limit */
+   best = 0;
+   for (i = 0; i < ARRAY_SIZE(wm831x_usb_limits); i++) {
+   if (limit >= wm831x_usb_limits[i] &&
+   wm831x_usb_limits[best] < wm831x_usb_limits[i])
+   best = i;
+   }
+
+   dev_dbg(wm831x_power->wm831x->dev,
+   "Limiting USB current to %umA", wm831x_usb_limits[best]);
+
+   wm831x_set_bits(wm831x_power->wm831x, WM831X_POWER_STATE,
+   WM831X_USB_ILIM_MASK, best);
+
+   return 0;
+}
+
 /*
  * Battery properties
  */
@@ -607,8 +647,31 @@ static int wm831x_power_probe(struct platform_device *pdev)
}
}
 
+   if (wm831x_pdata && wm831x_pdata->usb_gadget) {
+   power->usb_charger =
+   usb_charger_find_by_name(wm831x_pdata->usb_gadget);
+   if (IS_ERR(power->usb_charger)) {
+   ret = PTR_ERR(power->usb_charger);
+   dev_err(>dev,
+   "Failed to find USB gadget: %d\n", ret);
+   goto err_bat_irq;
+   }
+
+   power->usb_notify.notifier_call = wm831x_usb_limit_change;
+
+   ret = usb_charger_register_notify(power->usb_charger,
+ >usb_notify);
+   if (ret != 0) {
+   dev_err(>dev,
+   "Failed to register notifier: %d\n", ret);
+   goto err_usb_charger;
+   }
+   }
+
return ret;
 
+err_usb_charger:
+   /* put_device on charger */
 err_bat_irq:
--i;
for (; i >= 0; i--) {
@@ -637,6 +700,12 @@ static int wm831x_power_remove(struct platform_device 
*pdev)
struct wm831x *wm831x = wm831x_power->wm831x;
int irq, i;
 
+   if (wm831x_power->usb_charger) {
+   usb_charger_unregister_notify(wm831x_power->usb_charger,
+ _power->usb_notify);
+   /* Free charger */
+   }
+
for (i = 0; i < ARRAY_SIZE(wm831x_bat_irqs); i++) {
irq = wm831x_irq(wm831x, 
 platform_get_irq_byname(pdev,
diff --git a/include/linux/mfd/wm831x/pdata.h b/include/linux/mfd/wm831x/pdata.h
index dcc9631..5af8399 100644
--- a/include/linux/mfd/wm831x/pdata.h
+++ b/include/linux/mfd/wm831x/pdata.h
@@ -126,6 +126,9 @@ struct wm831x_pdata {
/** The driver should initiate a power off sequence during shutdown */
bool soft_shutdown;
 
+   /** dev_name of USB charger gadget to integrate with */
+   const char *usb_gadget;
+
int irq_base;
int gpio_base;
int gpio_defaults[WM831X_GPIO_NUM];
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe