Re: [PATCH v2 1/2] isp1704_charger: allow board specific powering routine

[Heikki Krogerus <heikki.krogerus-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>]

Hi,

Add Anton Vorontsov <cbouatmailru-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> to your v3. This will
need ack from him, or this needs to go to him. In this case I guess we
are only dealing with RX51 stuff, so maybe this should go to Tony.

On Mon, Mar 28, 2011 at 09:51:38AM +0300, Kalle Jokiniemi wrote:
> The ISP1704/1707 chip can be put to full power down
> state by asserting the CHIP_SEL line. This patch enables
> platform or board specific hooks to put the device into
> power down mode in case not needed.
> 
> These patches are preparatio for enabling this powering
> routine in n900 (rx-51) devices.
> 
> Thanks to Heikki Krogerus for helping out with the patch.
> 
> Signed-off-by: Kalle Jokiniemi <kalle.jokiniemi-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
> Cc: Heikki Krogerus <heikki.krogerus-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/power/isp1704_charger.c       |   26 ++++++++++++++++++++++++++
>  include/linux/power/isp1704_charger.h |   29 +++++++++++++++++++++++++++++
>  2 files changed, 55 insertions(+), 0 deletions(-)
>  create mode 100644 include/linux/power/isp1704_charger.h
> 
> diff --git a/drivers/power/isp1704_charger.c b/drivers/power/isp1704_charger.c
> index 2ad9b14..c796b9f 100644
> --- a/drivers/power/isp1704_charger.c
> +++ b/drivers/power/isp1704_charger.c
> @@ -33,6 +33,7 @@
>  #include <linux/usb/ulpi.h>
>  #include <linux/usb/ch9.h>
>  #include <linux/usb/gadget.h>
> +#include <linux/power/isp1704_charger.h>
>  
>  /* Vendor specific Power Control register */
>  #define ISP1704_PWR_CTRL		0x3d
> @@ -63,6 +64,7 @@ struct isp1704_charger {
>  	char			model[8];
>  	unsigned		present:1;
>  	unsigned		online:1;
> +	unsigned		init_done;

Do we need this?

>  	unsigned		current_max;
>  
>  	/* temp storage variables */
> @@ -71,6 +73,18 @@ struct isp1704_charger {
>  };
>  
>  /*
> + * Disable/enable the power from the isp1704 if a function for it
> + * has been provided with platform data.
> + */
> +static void isp1704_charger_set_power(struct isp1704_charger *isp, bool on)
> +{
> +	struct isp1704_charger_data	*board = isp->dev->platform_data;
> +
> +	if (board->set_power)
> +		board->set_power(on);
> +}
> +
> +/*
>   * Determine is the charging port DCP (dedicated charger) or CDP (Host/HUB
>   * chargers).
>   *
> @@ -222,6 +236,9 @@ static void isp1704_charger_work(struct work_struct *data)
>  
>  	mutex_lock(&lock);
>  
> +	if (event != USB_EVENT_NONE)
> +		isp1704_charger_set_power(isp, 1);
> +
>  	switch (event) {
>  	case USB_EVENT_VBUS:
>  		isp->online = true;
> @@ -269,6 +286,9 @@ static void isp1704_charger_work(struct work_struct *data)
>  		 */
>  		if (isp->otg->gadget)
>  			usb_gadget_disconnect(isp->otg->gadget);
> +		/* If we're initialized, we can power down the isp */
> +		if (isp->init_done)
> +			isp1704_charger_set_power(isp, 0);
>  		break;
>  	case USB_EVENT_ENUMERATED:
>  		if (isp->present)
> @@ -394,6 +414,8 @@ static int __devinit isp1704_charger_probe(struct platform_device *pdev)
>  	isp->dev = &pdev->dev;
>  	platform_set_drvdata(pdev, isp);
>  
> +	isp1704_charger_set_power(isp, 1);
> +
>  	ret = isp1704_test_ulpi(isp);
>  	if (ret < 0)
>  		goto fail1;
> @@ -437,8 +459,11 @@ static int __devinit isp1704_charger_probe(struct platform_device *pdev)
>  	if ((ret & ULPI_INT_VBUS_VALID) && !isp->otg->default_a) {
>  		isp->event = USB_EVENT_VBUS;
>  		schedule_work(&isp->work);
> +	} else {
> +		isp1704_charger_set_power(isp, 0);
>  	}

I think the transceiver can be powered down even if we just scheduled
the work. This will basically cause hw reset on the transceiver which
is only a good thing IMO. Drop the else condition.

> +	isp->init_done = 1;
>  	return 0;
>  fail2:
>  	power_supply_unregister(&isp->psy);
> @@ -459,6 +484,7 @@ static int __devexit isp1704_charger_remove(struct platform_device *pdev)
>  	otg_unregister_notifier(isp->otg, &isp->nb);
>  	power_supply_unregister(&isp->psy);
>  	otg_put_transceiver(isp->otg);
> +	isp1704_charger_set_power(isp, 0);
>  	kfree(isp);
>  
>  	return 0;

-- 
heikki
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html