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;
From:   Phil Edworthy <phil.edworthy@renesas.com>
To:     Stephen Boyd <sboyd@kernel.org>,
        Michael Turquette <mturquette@baylibre.com>
CC:     "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-clk@vger.kernel.org" <linux-clk@vger.kernel.org>
Subject: RE: [PATCH v2] clk: Document of_parse_clkspec() some more
Thread-Topic: [PATCH v2] clk: Document of_parse_clkspec() some more
Thread-Index: AQHVXFQhjzSS11Xl9kyvJ1FqF5kDOqcOz06Q
Date:   Tue, 27 Aug 2019 10:53:54 +0000
Message-ID: <TY1PR01MB1769DCBAD9B79AA2AD36F241F5A00@TY1PR01MB1769.jpnprd01.prod.outlook.com>
References: <20190826212042.48642-1-sboyd@kernel.org>
In-Reply-To: <20190826212042.48642-1-sboyd@kernel.org>
Accept-Language: en-GB, en-US
Content-Language: en-US
received-spf: None (protection.outlook.com: renesas.com does not designate
 permitted sender hosts)
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 61374bd9-7ea1-46b5-e65e-08d72adcdaca
X-MS-Exchange-CrossTenant-originalarrivaltime: 27 Aug 2019 10:53:54.6947
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 53d82571-da19-47e4-9cb4-625a166a4a2a
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: dlm+9yTlIlN3AKLoe+2jmAWJtgX0Yettyn+VmaaTh3W7RWXdVH0+o/Hf8+s5MIyYiUtCxIgQ6Wl/yVqhQXW6UzAhwplhyZkRCbSQ2NSkMBs=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: TY1PR01MB1692
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Stephen,

On 26 August 2019 22:21 Stephen Boyd wrote:
> The return value of of_parse_clkspec() is peculiar. If the function is
> called with a NULL argument for 'name' it will return -ENOENT, but if
> it's called with a non-NULL argument for 'name' it will return -EINVAL.
> This peculiarity is documented by commit 5c56dfe63b6e ("clk: Add comment
> about __of_clk_get_by_name() error values").
>=20
> Let's further document this function so that it's clear what the return
> value is and how to use the arguments to parse clk specifiers.
>=20
> Cc: Phil Edworthy <phil.edworthy@renesas.com>
> Signed-off-by: Stephen Boyd <sboyd@kernel.org>

Thanks, this is much better than my comment!

Reviewed-by: Phil Edworthy <phil.edworthy@renesas.com>

> ---
>  drivers/clk/clk.c | 43 +++++++++++++++++++++++++++++++++++++------
>  1 file changed, 37 insertions(+), 6 deletions(-)
>=20
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index c0990703ce54..5c6585eb35d4 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -4316,12 +4316,43 @@ void devm_of_clk_del_provider(struct device
> *dev)
>  }
>  EXPORT_SYMBOL(devm_of_clk_del_provider);
>=20
> -/*
> - * Beware the return values when np is valid, but no clock provider is f=
ound.
> - * If name =3D=3D NULL, the function returns -ENOENT.
> - * If name !=3D NULL, the function returns -EINVAL. This is because
> - * of_parse_phandle_with_args() is called even if
> of_property_match_string()
> - * returns an error.
> +/**
> + * of_parse_clkspec() - Parse a DT clock specifier for a given device no=
de
> + * @np: device node to parse clock specifier from
> + * @index: index of phandle to parse clock out of. If index < 0, @name i=
s
> used
> + * @name: clock name to find and parse. If name is NULL, the index is us=
ed
> + * @out_args: Result of parsing the clock specifier
> + *
> + * Parses a device node's "clocks" and "clock-names" properties to find =
the
> + * phandle and cells for the index or name that is desired. The resultin=
g
> clock
> + * specifier is placed into @out_args, or an errno is returned when ther=
e's a
> + * parsing error. The @index argument is ignored if @name is non-NULL.
> + *
> + * Example:
> + *
> + * phandle1: clock-controller@1 {
> + *	#clock-cells =3D <2>;
> + * }
> + *
> + * phandle2: clock-controller@2 {
> + *	#clock-cells =3D <1>;
> + * }
> + *
> + * clock-consumer@3 {
> + *	clocks =3D <&phandle1 1 2 &phandle2 3>;
> + *	clock-names =3D "name1", "name2";
> + * }
> + *
> + * To get a device_node for `clock-controller@2' node you may call this
> + * function a few different ways:
> + *
> + *   of_parse_clkspec(clock-consumer@3, -1, "name2", &args);
> + *   of_parse_clkspec(clock-consumer@3, 1, NULL, &args);
> + *   of_parse_clkspec(clock-consumer@3, 1, "name2", &args);
> + *
> + * Return: 0 upon successfully parsing the clock specifier. Otherwise, -
> ENOENT
> + * if @name is NULL or -EINVAL if @name is non-NULL and it can't be foun=
d
> in
> + * the "clock-names" property of @np.
>   */
>  static int of_parse_clkspec(const struct device_node *np, int index,
>  			    const char *name, struct of_phandle_args
> *out_args)
> --
> Sent by a computer through tubes