All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
To: Mikko Perttunen <mperttunen-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Cc: balbi-l0cyMroinI0@public.gmane.org,
	stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org,
	linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [PATCH 1/4] usb: phy: tegra: Add support for device tree-based vbus regulator control
Date: Wed, 26 Jun 2013 11:14:35 -0600	[thread overview]
Message-ID: <51CB217B.1040308@wwwdotorg.org> (raw)
In-Reply-To: <1372240781-1017-2-git-send-email-mperttunen-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

On 06/26/2013 03:59 AM, Mikko Perttunen wrote:
> After this patch, usb vbus regulators for tegra usb phy devices can be specified
> with the device tree attribute vbus-supply = <&x> where x is a regulator defined
> in the device tree.

> diff --git a/drivers/usb/phy/phy-tegra-usb.c b/drivers/usb/phy/phy-tegra-usb.c

> @@ -250,12 +251,24 @@ static int utmip_pad_open(struct tegra_usb_phy *phy)
>  		return PTR_ERR(phy->pad_clk);
>  	}
>  
> +	phy->vbus = devm_regulator_get(phy->dev, "vbus");
> +	/* On some boards, the VBUS regulator doesn't need to be controlled */
> +	if (IS_ERR(phy->vbus)) {
> +		if (PTR_ERR(phy->vbus) == -ENODEV) {
> +			dev_notice(phy->dev, "no vbus regulator");
> +			phy->vbus = NULL;
> +		} else {
> +			return PTR_ERR(phy->vbus);
> +		}
> +	}

I think this code should be added to some more core initialization
function; IIRC, there are separate utmip_pad_open() and some other
function for ULPI mode, and in the future there may be more for HSIC, etc.

For the error-handling, I think it'd be better to do:

* If property doesn't exist, set phy->vbus to some error value, e.g.
ERR_PTR(-ENODEV).
* If property does exist, call devm_regulator_get().
** If devm_regulator_get() returned any error, return it.

Or, does devm_regulator_get() return -ENODEV if-and-only-if the
vbus-supply DT property does not exist?

and ...

> @@ -280,6 +293,14 @@ static void utmip_pad_power_on(struct tegra_usb_phy *phy)
>  	spin_unlock_irqrestore(&utmip_pad_lock, flags);
>  
>  	clk_disable_unprepare(phy->pad_clk);
> +
> +	if (phy->vbus) {

Here, check if (IS_ERR(phy->vbus) instead. The reason is if
devm_regulator_get() returns either a valid value or an error-pointer,
then NULL could in theory be a valid value (it's up the the regulator
API to determine that), and hence this code shouldn't assume that it can
use NULL to represent "no regulator".

  parent reply	other threads:[~2013-06-26 17:14 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-26  9:59 [PATCH 0/4] usb: tegra: Replace nvidia,vbus-gpio property with vbus-supply Mikko Perttunen
     [not found] ` <1372240781-1017-1-git-send-email-mperttunen-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2013-06-26  9:59   ` [PATCH 1/4] usb: phy: tegra: Add support for device tree-based vbus regulator control Mikko Perttunen
     [not found]     ` <1372240781-1017-2-git-send-email-mperttunen-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2013-06-26 17:14       ` Stephen Warren [this message]
     [not found]         ` <51CB217B.1040308-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2013-06-27  7:20           ` Mikko Perttunen
2013-06-27 16:15             ` Stephen Warren
     [not found]               ` <51CC6506.4000503-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2013-06-28  7:17                 ` Peter De Schrijver
2013-06-26  9:59   ` [PATCH 2/4] usb: host: tegra: Remove direct vbus regulator control using GPIOs Mikko Perttunen
     [not found]     ` <1372240781-1017-3-git-send-email-mperttunen-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2013-06-26 17:16       ` Stephen Warren
2013-06-26  9:59   ` [PATCH 3/4] Documentation: devicetree: phy-tegra-usb: Add vbus-supply property for host mode PHYs Mikko Perttunen
     [not found]     ` <1372240781-1017-4-git-send-email-mperttunen-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2013-06-26 17:17       ` Stephen Warren
2013-06-26  9:59   ` [PATCH 4/4] ARM: dts: tegra20: Remove obsolete vbus-gpio properties Mikko Perttunen
     [not found]     ` <1372240781-1017-5-git-send-email-mperttunen-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2013-06-26 10:17       ` Felipe Balbi
     [not found]         ` <20130626101758.GU12640-S8G//mZuvNWo5Im9Ml3/Zg@public.gmane.org>
2013-06-26 10:22           ` Mikko Perttunen
2013-06-26 10:23             ` Felipe Balbi
2013-06-26 17:19       ` Stephen Warren
     [not found]         ` <51CB22B1.2000903-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2013-06-27  6:49           ` Felipe Balbi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=51CB217B.1040308@wwwdotorg.org \
    --to=swarren-3lzwwm7+weoh9zmkesr00q@public.gmane.org \
    --cc=balbi-l0cyMroinI0@public.gmane.org \
    --cc=gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org \
    --cc=linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=mperttunen-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org \
    --cc=stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.