linux-kernel - Re: [PATCH 54/57] power: ab8500_charger: Use USBLink1Status Register

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20120928025659.GM5040@lizard>
Date:	Thu, 27 Sep 2012 19:56:59 -0700
From:	Anton Vorontsov <anton.vorontsov@...aro.org>
To:	mathieu.poirier@...aro.org
Cc:	linux-kernel@...r.kernel.org, dwmw2@...radead.org
Subject: Re: [PATCH 54/57] power: ab8500_charger: Use USBLink1Status Register

On Tue, Sep 25, 2012 at 10:12:51AM -0600, mathieu.poirier@...aro.org wrote:
> From: Marcus Cooper <marcus.xm.cooper@...ricsson.com>
> 
> The newer AB's such as the AB8505, AB9540 etc include a
> USBLink1 Status register which detects a larger range of
> external devices. This should be used instead of the
> USBLine Status register.
> 
> Signed-off-by: Marcus Cooper <marcus.xm.cooper@...ricsson.com>
> Signed-off-by: Mathieu Poirier <mathieu.poirier@...aro.org>
> Reviewed-by: Hakan BERG <hakan.berg@...ricsson.com>
> Reviewed-by: Yang QU <yang.qu@...ricsson.com>
> Reviewed-by: Jonas ABERG <jonas.aberg@...ricsson.com>
> ---
>  drivers/power/ab8500_charger.c |   22 ++++++++++++++++------
>  1 files changed, 16 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/power/ab8500_charger.c b/drivers/power/ab8500_charger.c
> index 3a97012..7f8f362 100644
> --- a/drivers/power/ab8500_charger.c
> +++ b/drivers/power/ab8500_charger.c
> @@ -2258,8 +2258,13 @@ static void ab8500_charger_usb_link_status_work(struct work_struct *work)
>  	 * to start the charging process. but by jumping
>  	 * thru a few hoops it can be forced to start.
>  	 */
> -	ret = abx500_get_register_interruptible(di->dev, AB8500_USB,
> -			AB8500_USB_LINE_STAT_REG, &val);
> +	if (is_ab8500(di->parent))
> +		ret = abx500_get_register_interruptible(di->dev, AB8500_USB,
> +					AB8500_USB_LINE_STAT_REG, &val);
> +	else
> +		ret = abx500_get_register_interruptible(di->dev, AB8500_USB,
> +					AB8500_USB_LINK1_STAT_REG, &val);

How about

	int reg = is_ab8500(di->parent) ? AB8500_USB_LINE_STAT_REG :
					  AB8500_USB_LINK1_STAT_REG;

	ret = abx500_get_register_interruptible(di->dev, AB8500_USB, reg, &val);

Shorter, clearer, and precisely fits into 80 columns -- must be good. :-)

> +
>  	if (ret >= 0)
>  		dev_dbg(di->dev, "UsbLineStatus register = 0x%02x\n", val);
>  	else
> @@ -2299,10 +2304,15 @@ static void ab8500_charger_usb_link_status_work(struct work_struct *work)
>  						AB8500_MCH_IPT_CURLVL_REG,
>  						0x01, 0x00);
>  			/*Check link status*/
> -			ret = abx500_get_register_interruptible(di->dev,
> -						AB8500_USB,
> -						AB8500_USB_LINE_STAT_REG,
> -						&val);
> +			if (is_ab8500(di->parent))
> +				ret = abx500_get_register_interruptible(di->dev,
> +					AB8500_USB, AB8500_USB_LINE_STAT_REG,
> +					&val);
> +			else
> +				ret = abx500_get_register_interruptible(di->dev,
> +					AB8500_USB, AB8500_USB_LINK1_STAT_REG,
> +					&val);
> +

Same here. Actually, isn't it exactly the same as above? If so, then just
factor it into its own function.

>  			dev_dbg(di->dev, "USB link status= 0x%02x\n",
>  				(val & link_status) >> USB_LINK_STATUS_SHIFT);
>  			di->invalid_charger_detect_state = 2;
> -- 
> 1.7.5.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/