Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20200131153440.20870-1-calvin.johnson@nxp.com> <20200131153440.20870-6-calvin.johnson@nxp.com>
In-Reply-To: <20200131153440.20870-6-calvin.johnson@nxp.com>
From:   Andy Shevchenko <andy.shevchenko@gmail.com>
Date:   Fri, 31 Jan 2020 17:57:34 +0200
Message-ID: <CAHp75Vf+UVMS1WOQ3KB8DiSB5KD2oFQs5ZmJ5yDPi=3i4uZVRg@mail.gmail.com>
Subject: Re: [PATCH v1 5/7] device property: Introduce fwnode_phy_is_fixed_link()
To:     Calvin Johnson <calvin.johnson@nxp.com>
Cc:     linux.cj@gmail.com, Jon Nettleton <jon@solid-run.com>,
        Russell King - ARM Linux <linux@armlinux.org.uk>,
        Makarand Pawagi <makarand.pawagi@nxp.com>,
        cristian.sovaiala@nxp.com, laurentiu.tudor@nxp.com,
        ioana.ciornei@nxp.com, V.Sethi@nxp.com, pankaj.bansal@nxp.com,
        "Rajesh V . Bikkina" <rajesh.bikkina@nxp.com>,
        Calvin Johnson <calvin.johnson@oss.nxp.com>,
        Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
        Dmitry Torokhov <dmitry.torokhov@gmail.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Heikki Krogerus <heikki.krogerus@linux.intel.com>,
        "Rafael J. Wysocki" <rafael@kernel.org>,
        Sakari Ailus <sakari.ailus@linux.intel.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, Jan 31, 2020 at 5:38 PM Calvin Johnson <calvin.johnson@nxp.com> wrote:
>
> From: Calvin Johnson <calvin.johnson@oss.nxp.com>
>
> Introduce fwnode_phy_is_fixed_link() function that an Ethernet driver
> can call on its PHY phandle to find out whether it's a fixed link PHY
> or not.

> +/*
> + * fwnode_phy_is_fixed_link()
> + */

Please, do a full kernel doc description.

> +bool fwnode_phy_is_fixed_link(struct fwnode_handle *fwnode)
> +{
> +       struct fwnode_handle *fixed_node;
> +       int len, err;
> +       const char *managed;
> +
> +       fixed_node = fwnode_get_named_child_node(fwnode, "fixed-link");
> +       if (fixed_node)
> +               return fixed_node;
> +
> +       err = fwnode_property_read_string(fixed_node, "managed", &managed);

> +       if (err == 0 && strcmp(managed, "auto") != 0)
> +               return true;
> +
> +       return false;

Maybe other way around?

  if (err)
    return false;

  return !strcmp(managed, "auto");

?

Same pattern perhaps for the patch where you introduce fwnode_get_phy_mode().

> +}

-- 
With Best Regards,
Andy Shevchenko