mail archive of the barebox mailing list
 help / color / mirror / Atom feed
From: Trent Piepho <>
To: Sascha Hauer <>
Cc: "Daniel Brát" <>,
Subject: Re: [PATCH] phy: core: Make 'phy_optional_get' return NULL when not implemented
Date: Mon, 1 Nov 2021 12:04:36 -0700	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On Mon, Nov 1, 2021 at 2:01 AM Sascha Hauer <> wrote:
> On Wed, Oct 20, 2021 at 10:39:54AM +0200, Daniel Brát wrote:
> > Make 'phy_optional_get' return NULL instead of ERR_PTR(-ENOSYS) when the
> > CONFIG_GENERIC_PHY is not enabled. It makes more sense to return NULL instead
> > of straight up throwing a error since the function has 'optional' in its name.
> > This also fixes dwc2 usb driver which would previously fail inside its probe
> > function despite being able to function without a phy just fine.
> The phy is only optional as long as none is specified in the device
> tree. When there is one specified then it's no longer optional. We can't
> do the right thing here without checking the device tree. Given that
> it's simple to enable CONFIG_GENERIC_PHY I think this is the way to go.

But this force enables GENERIC_PHY when it's not needed.

There are commonly many device nodes in Linux dts files that are not
used by an enabled Barebox driver.  It's normal to turn off a driver
that might be or could be used.  Is it necessarily an error if a phy
is present in the dts but we don't wish to include support for it?

In this Barebox version of this code, I think phy_optional_get() only
returns NULL if there is some error finding the phy OF node, such as
there being none specified or some error in the validity of the dts
data.  Otherwise it's PROBE_DEFER.

In the Linux version, there are other ways to get NULL, such as the
phy being disabled:
        if (!of_device_is_available( {
               dev_warn(phy_provider->dev, "Requested PHY is disabled\n");
               phy = ERR_PTR(-ENODEV);  // phy_optional_get -> NULL

So it seems like the phy being optional, even if defined in this dts,
might be the intended behavior.

The stub version could check if a phy is in the dts and generate a
warning, if the goal is to reduce problems with people creating broken
configurations and then not understanding why their configuration does
not work.

struct phy *phy_optional_get(struct device_d *dev, const char *string)
        if (dev->device_node &&
            !of_parse_phandle_with_args(dev->device_node, "phys",
"#phy-cells", of_property_match_string(dev->device_node, "phy-names",
string), NULL))
                dev_warn(dev, "%s phy specified in device tree but
CONFIG_GENERIC_PHY support is not enabled", string);
        return NULL;

barebox mailing list

  parent reply	other threads:[~2021-11-01 19:06 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-20  8:39 Daniel Brát
2021-11-01  9:00 ` Sascha Hauer
     [not found]   ` <>
2021-11-01 19:04     ` Trent Piepho [this message]
2021-11-02  7:40     ` Sascha Hauer
2021-11-03 22:35       ` Trent Piepho
2021-11-04 20:02         ` Sascha Hauer
2021-11-04 21:40           ` Trent Piepho
2021-11-08  9:10             ` Sascha Hauer
2021-11-08 22:01               ` Trent Piepho
2021-11-11 13:09                 ` Sascha Hauer
2022-03-03  9:53 ` Sascha Hauer

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:

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

  git send-email \
    --in-reply-to='' \ \ \ \ \

* 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox