All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
To: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
	Peter Chen <Peter.Chen@nxp.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: usb: chipidea: Grab the (legacy) USB PHY by phandle first
Date: Wed, 16 Jan 2019 15:22:40 +0100	[thread overview]
Message-ID: <b42e37ab06ae3efa244a35df801e2126fd692bb7.camel@bootlin.com> (raw)

Hi,

On Wed, 2019-01-16 at 14:44 +0100, Thomas Petazzoni wrote:
> Well prior to your code, there was already a possibility for both
> ci->phy and ci->usb_phy to be valid. I don't think it's really useful
> to avoid the fallback when a generic PHY has already been found, it's
> confusing. If really you want to clarify that, it should be:
> 
> 	/* Let's first try to find a generic PHY */
> 	ci->phy = devm_phy_get(dev->parent, "usb-phy");
> 	if (IS_ERR(ci->phy)) {
> 		/* Fall back to legacy USB PHY */
> 		ci->usb_phy = devm_usb_get_phy_by_phandle(dev->parent, "phys", 0);
> 		if (IS_ERR(ci->usb_phy))
> 			ci->usb_phy = devm_usb_get_phy(dev->parent, USB_PHY_TYPE_USB2);
> 	}
> 
> With that, you would only have either ci->phy or ci->usb_phy be valid,
> and never both. With  your change, you can have ci->phy and ci->usb_phy
> both be valid if the legacy USB PHY was found using
> devm_usb_get_phy_by_phandle(), but not if we fell back to
> devm_usb_get_phy().

Okay that makes sense, your suggestion is indeed more consistent with
the existing behavior. I'll go with that in the next revision!

Cheers,

Paul

WARNING: multiple messages have this Message-ID (diff)
From: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
To: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
	Peter Chen <Peter.Chen@nxp.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: Re: [PATCH] usb: chipidea: Grab the (legacy) USB PHY by phandle first
Date: Wed, 16 Jan 2019 15:22:40 +0100	[thread overview]
Message-ID: <b42e37ab06ae3efa244a35df801e2126fd692bb7.camel@bootlin.com> (raw)
In-Reply-To: <20190116144429.7df1d0c5@windsurf>

Hi,

On Wed, 2019-01-16 at 14:44 +0100, Thomas Petazzoni wrote:
> Well prior to your code, there was already a possibility for both
> ci->phy and ci->usb_phy to be valid. I don't think it's really useful
> to avoid the fallback when a generic PHY has already been found, it's
> confusing. If really you want to clarify that, it should be:
> 
> 	/* Let's first try to find a generic PHY */
> 	ci->phy = devm_phy_get(dev->parent, "usb-phy");
> 	if (IS_ERR(ci->phy)) {
> 		/* Fall back to legacy USB PHY */
> 		ci->usb_phy = devm_usb_get_phy_by_phandle(dev->parent, "phys", 0);
> 		if (IS_ERR(ci->usb_phy))
> 			ci->usb_phy = devm_usb_get_phy(dev->parent, USB_PHY_TYPE_USB2);
> 	}
> 
> With that, you would only have either ci->phy or ci->usb_phy be valid,
> and never both. With  your change, you can have ci->phy and ci->usb_phy
> both be valid if the legacy USB PHY was found using
> devm_usb_get_phy_by_phandle(), but not if we fell back to
> devm_usb_get_phy().

Okay that makes sense, your suggestion is indeed more consistent with
the existing behavior. I'll go with that in the next revision!

Cheers,

Paul

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com


             reply	other threads:[~2019-01-16 14:22 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-16 14:22 Paul Kocialkowski [this message]
2019-01-16 14:22 ` [PATCH] usb: chipidea: Grab the (legacy) USB PHY by phandle first Paul Kocialkowski
  -- strict thread matches above, loose matches on Subject: below --
2019-01-18  2:18 Peter Chen
2019-01-18  2:18 ` [PATCH] " Peter Chen
2019-01-17 16:38 Paul Kocialkowski
2019-01-17 16:38 ` [PATCH] " Paul Kocialkowski
2019-01-17  6:44 Peter Chen
2019-01-17  6:44 ` [PATCH] " Peter Chen
2019-01-16 13:44 Thomas Petazzoni
2019-01-16 13:44 ` [PATCH] " Thomas Petazzoni
2019-01-16 13:30 Paul Kocialkowski
2019-01-16 13:30 ` [PATCH] " Paul Kocialkowski
2019-01-16 10:53 Thomas Petazzoni
2019-01-16 10:53 ` [PATCH] " Thomas Petazzoni
2019-01-16 10:10 Paul Kocialkowski
2019-01-16 10:10 ` [PATCH] " Paul Kocialkowski

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=b42e37ab06ae3efa244a35df801e2126fd692bb7.camel@bootlin.com \
    --to=paul.kocialkowski@bootlin.com \
    --cc=Peter.Chen@nxp.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=thomas.petazzoni@bootlin.com \
    /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.