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;
Date:   Mon, 26 Nov 2018 16:26:37 +0100
From:   Thierry Reding <thierry.reding@gmail.com>
To:     Hao Zhang <hao5781286@gmail.com>
Cc:     robh+dt@kernel.org, mark.rutland@arm.com,
        maxime.ripard@bootlin.com, wens@csie.org, mturquette@baylibre.com,
        sboyd@kernel.org, linux-gpio@vger.kernel.org,
        linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org, linux-pwm@vger.kernel.org,
        linux-sunxi@googlegroups.com
Subject: Re: [PATCH v3 4/6] DEV: CLK: add function to check the using clock
 name of driver.
Message-ID: <20181126152637.GB19710@ulmo>
References: <20181125162118.GA5358@arx-s1>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="7iMSBzlTiPOCCT2k"
Content-Disposition: inline
In-Reply-To: <20181125162118.GA5358@arx-s1>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


--7iMSBzlTiPOCCT2k
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Mon, Nov 26, 2018 at 12:21:18AM +0800, Hao Zhang wrote:
> In some situation we want to check clock whether is we want
> and after the driver been probed use to change different clock source.
>=20
> Signed-off-by: Hao Zhang <hao5781286@gmail.com>
> ---
>  drivers/clk/clk.c            | 6 ++++++
>  include/linux/clk-provider.h | 1 +
>  2 files changed, 7 insertions(+)

This doesn't seem right. If you're going to change to to a different
clock source anyway, why would you even want to know what it was set
before that? Isn't that completely irrelevant?

But if you really need this, perhaps a combination of clk_get() and
clk_is_match() would be a better approach? Perhaps best to let Mike
and Stephen share their feelings about this kind of API.

> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index d31055a..3d2c2cd 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -3466,6 +3466,12 @@ static int devm_clk_hw_match(struct device *dev, v=
oid *res, void *data)
>  	return hw =3D=3D data;
>  }
> =20
> +bool devm_clk_name_match(struct clk *clk, const char *string)
> +{
> +	return match_string(&clk->con_id, 1, string) =3D=3D 0;
> +}
> +EXPORT_SYMBOL_GPL(devm_clk_name_match);

Also, there's nothing about this that would be device-managed, so the
devm_ prefix on the function name is misleading.

>  /**
>   * devm_clk_unregister - resource managed clk_unregister()
>   * @clk: clock to unregister
> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> index 08b1aa7..5cd2eed 100644
> --- a/include/linux/clk-provider.h
> +++ b/include/linux/clk-provider.h
> @@ -764,6 +764,7 @@ struct clk *devm_clk_register(struct device *dev, str=
uct clk_hw *hw);
>  int __must_check clk_hw_register(struct device *dev, struct clk_hw *hw);
>  int __must_check devm_clk_hw_register(struct device *dev, struct clk_hw =
*hw);
> =20
> +bool devm_clk_name_match(struct clk *clk, const char *string);
>  void clk_unregister(struct clk *clk);
>  void devm_clk_unregister(struct device *dev, struct clk *clk);

You use this from clock consumers, so clk-provider.h is not the right
place for it. Also, you'll want to add a dummy implementation for the
case where CONFIG_COMMON_CLK is not set.

Thierry

--7iMSBzlTiPOCCT2k
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCAAdFiEEiOrDCAFJzPfAjcif3SOs138+s6EFAlv8EKkACgkQ3SOs138+
s6EqSA/+KecW29TvcRN+tsDP+2qTYxrw9IhCvgGgkvwWizFuMIY1kjajRebUe9j0
3jxWJ6hLrw1E2sL4ZwMOZJnw6GI2ApMg7PXy2R6F/awZ6vMOB2FE1BUTDWLBYP5v
2PHAy+kvDli+Gl74PCZy58BnOSZyncRP9FtoBJDZ4jx/LM8JGcIhqq0ybi0OTQjA
Q8ZEvPTdBZ8kS8PsjT6zSYIaUtYw7ejsemBPORaHFHJlHokUkfA2kMGJVhHsGnZH
eMngTXRf3q4U3Tx1reukBa3EfDuYRGl2OVVGKPGp1l6HuOpoAPjuGCA9ASP5bd3N
72OsNdbzp2kIU+BZIrmgnjQ6DQ0jQKpcHA7iomKYw/Qku8Sntu35Yj11LnF+3bJK
dR0rlTg2XV+GPigvGifh2WhFcXJ8C+CJKHmcaR3gyaigS8bYaymWe16HvKdPklSz
2tKo6pUlMKTHf5oYtTTNtlBNgGRjOuCjt23keuwrse8willdrHxdR9pCE0vUL3iN
StIfJTdq4FSJx+dsZBXKB4MUQ8oNckm9lEwZbI4jHaNi35eoRy3z7NrA07FQ7DLn
x9nGhvayD5xjYfJyzhk/XEC3w1FPxWIVmlP+v5IBNR28ei2EF/UZAEjN8uDy1Vul
UvT6cGuYmdMaw8x+tEXyrLMJI1iwDWDUwk8QV8/lurjsDfsEOxw=
=1aiA
-----END PGP SIGNATURE-----

--7iMSBzlTiPOCCT2k--