All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] clk: Clean up optional helpers, and add API docs to HTML
@ 2021-12-01 18:43 Sean Anderson
  2021-12-01 18:43 ` [PATCH 1/5] clk: Rename clk_get_optional_nodev Sean Anderson
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Sean Anderson @ 2021-12-01 18:43 UTC (permalink / raw)
  To: u-boot
  Cc: Dario Binacchi, Simon Glass, Lukasz Majewski, Sean Anderson,
	Heinrich Schuchardt, Michal Simek, Neil Armstrong,
	u-boot-amlogic

This cleans up the various optional helpers for clocks, and adds a new one.
While we're at it, also convert the existing API docs to our HTML documentation.


Sean Anderson (5):
  clk: Rename clk_get_optional_nodev
  clk: Inline clk_get_*_optional
  clk: Add client API to HTML docs
  clk: Add driver API to HTML docs
  clk: Add clk_get_by_name_optional

 doc/api/clk.rst            |  19 +++
 doc/api/index.rst          |   1 +
 drivers/clk/clk-uclass.c   |  21 ---
 drivers/clk/clk_zynq.c     |   5 +-
 drivers/phy/phy-mtk-tphy.c |   8 +-
 drivers/rng/meson-rng.c    |   4 +-
 include/clk-uclass.h       | 187 +++++++++++++----------
 include/clk.h              | 306 +++++++++++++++++++++----------------
 8 files changed, 310 insertions(+), 241 deletions(-)
 create mode 100644 doc/api/clk.rst

-- 
2.33.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/5] clk: Rename clk_get_optional_nodev
  2021-12-01 18:43 [PATCH 0/5] clk: Clean up optional helpers, and add API docs to HTML Sean Anderson
@ 2021-12-01 18:43 ` Sean Anderson
  2021-12-01 18:43 ` [PATCH 2/5] clk: Inline clk_get_*_optional Sean Anderson
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Sean Anderson @ 2021-12-01 18:43 UTC (permalink / raw)
  To: u-boot; +Cc: Dario Binacchi, Simon Glass, Lukasz Majewski, Sean Anderson

This normalizes the name of this accessor function to put "_optional" last.

Signed-off-by: Sean Anderson <seanga2@gmail.com>
---

 drivers/clk/clk-uclass.c   | 3 ++-
 drivers/phy/phy-mtk-tphy.c | 8 ++++----
 include/clk.h              | 7 ++++---
 3 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
index 493018b33e..18699d69e3 100644
--- a/drivers/clk/clk-uclass.c
+++ b/drivers/clk/clk-uclass.c
@@ -432,7 +432,8 @@ int clk_get_by_name_nodev(ofnode node, const char *name, struct clk *clk)
 	return clk_get_by_index_nodev(node, index, clk);
 }
 
-int clk_get_optional_nodev(ofnode node, const char *name, struct clk *clk)
+int clk_get_by_name_nodev_optional(ofnode node, const char *name,
+				   struct clk *clk)
 {
 	int ret;
 
diff --git a/drivers/phy/phy-mtk-tphy.c b/drivers/phy/phy-mtk-tphy.c
index 824244b852..2dd964f7b2 100644
--- a/drivers/phy/phy-mtk-tphy.c
+++ b/drivers/phy/phy-mtk-tphy.c
@@ -723,13 +723,13 @@ static int mtk_tphy_probe(struct udevice *dev)
 		tphy->phys[index] = instance;
 		index++;
 
-		err = clk_get_optional_nodev(subnode, "ref",
-					     &instance->ref_clk);
+		err = clk_get_by_name_nodev_optional(subnode, "ref",
+						     &instance->ref_clk);
 		if (err)
 			return err;
 
-		err = clk_get_optional_nodev(subnode, "da_ref",
-					     &instance->da_ref_clk);
+		err = clk_get_by_name_nodev_optional(subnode, "da_ref",
+						     &instance->da_ref_clk);
 		if (err)
 			return err;
 	}
diff --git a/include/clk.h b/include/clk.h
index a928879b12..943c87f3c3 100644
--- a/include/clk.h
+++ b/include/clk.h
@@ -194,7 +194,7 @@ int clk_get_by_name(struct udevice *dev, const char *name, struct clk *clk);
 int clk_get_by_name_nodev(ofnode node, const char *name, struct clk *clk);
 
 /**
- * clk_get_optional_nodev - Get/request an optinonal clock by name
+ * clk_get_by_name_nodev_optional - Get/request an optinonal clock by name
  *		without a device.
  * @node:	The client ofnode.
  * @name:	The name of the clock to request.
@@ -206,7 +206,8 @@ int clk_get_by_name_nodev(ofnode node, const char *name, struct clk *clk);
  * no clock producer, in this case, skip the error number -ENODATA, and
  * the function returns 0.
  */
-int clk_get_optional_nodev(ofnode node, const char *name, struct clk *clk);
+int clk_get_by_name_nodev_optional(ofnode node, const char *name,
+				   struct clk *clk);
 
 /**
  * devm_clk_get - lookup and obtain a managed reference to a clock producer.
@@ -291,7 +292,7 @@ clk_get_by_name_nodev(ofnode node, const char *name, struct clk *clk)
 }
 
 static inline int
-clk_get_optional_nodev(ofnode node, const char *name, struct clk *clk)
+clk_get_by_name_nodev_optional(ofnode node, const char *name, struct clk *clk)
 {
 	return -ENOSYS;
 }
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/5] clk: Inline clk_get_*_optional
  2021-12-01 18:43 [PATCH 0/5] clk: Clean up optional helpers, and add API docs to HTML Sean Anderson
  2021-12-01 18:43 ` [PATCH 1/5] clk: Rename clk_get_optional_nodev Sean Anderson
@ 2021-12-01 18:43 ` Sean Anderson
  2021-12-01 18:43 ` [PATCH 3/5] clk: Add client API to HTML docs Sean Anderson
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Sean Anderson @ 2021-12-01 18:43 UTC (permalink / raw)
  To: u-boot; +Cc: Dario Binacchi, Simon Glass, Lukasz Majewski, Sean Anderson

The optional varients of clk_get_* functions are just simple wrappers.
Reduce code size a bit by inlining them. On platforms where it is not used
(most of them), it will not be compiled in any more. On platforms where
they are used, the inlined branch should not cause any significant growth.

Signed-off-by: Sean Anderson <seanga2@gmail.com>
---

 drivers/clk/clk-uclass.c | 22 ---------------
 include/clk.h            | 58 ++++++++++++++++++++++++----------------
 2 files changed, 35 insertions(+), 45 deletions(-)

diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
index 18699d69e3..168fb67019 100644
--- a/drivers/clk/clk-uclass.c
+++ b/drivers/clk/clk-uclass.c
@@ -432,18 +432,6 @@ int clk_get_by_name_nodev(ofnode node, const char *name, struct clk *clk)
 	return clk_get_by_index_nodev(node, index, clk);
 }
 
-int clk_get_by_name_nodev_optional(ofnode node, const char *name,
-				   struct clk *clk)
-{
-	int ret;
-
-	ret = clk_get_by_name_nodev(node, name, clk);
-	if (ret == -ENODATA)
-		return 0;
-
-	return ret;
-}
-
 int clk_release_all(struct clk *clk, int count)
 {
 	int i, ret;
@@ -824,16 +812,6 @@ struct clk *devm_clk_get(struct udevice *dev, const char *id)
 	return clk;
 }
 
-struct clk *devm_clk_get_optional(struct udevice *dev, const char *id)
-{
-	struct clk *clk = devm_clk_get(dev, id);
-
-	if (PTR_ERR(clk) == -ENODATA)
-		return NULL;
-
-	return clk;
-}
-
 void devm_clk_put(struct udevice *dev, struct clk *clk)
 {
 	int rc;
diff --git a/include/clk.h b/include/clk.h
index 943c87f3c3..c3d4cc0ecf 100644
--- a/include/clk.h
+++ b/include/clk.h
@@ -193,22 +193,6 @@ int clk_get_by_name(struct udevice *dev, const char *name, struct clk *clk);
  */
 int clk_get_by_name_nodev(ofnode node, const char *name, struct clk *clk);
 
-/**
- * clk_get_by_name_nodev_optional - Get/request an optinonal clock by name
- *		without a device.
- * @node:	The client ofnode.
- * @name:	The name of the clock to request.
- * @name:	The name of the clock to request, within the client's list of
- *		clocks.
- * @clock:	A pointer to a clock struct to initialize.
- *
- * Behaves the same as clk_get_by_name_nodev() except where there is
- * no clock producer, in this case, skip the error number -ENODATA, and
- * the function returns 0.
- */
-int clk_get_by_name_nodev_optional(ofnode node, const char *name,
-				   struct clk *clk);
-
 /**
  * devm_clk_get - lookup and obtain a managed reference to a clock producer.
  * @dev: device for clock "consumer"
@@ -238,7 +222,16 @@ struct clk *devm_clk_get(struct udevice *dev, const char *id);
  * Behaves the same as devm_clk_get() except where there is no clock producer.
  * In this case, instead of returning -ENOENT, the function returns NULL.
  */
-struct clk *devm_clk_get_optional(struct udevice *dev, const char *id);
+static inline struct clk *devm_clk_get_optional(struct udevice *dev,
+						const char *id)
+{
+	struct clk *clk = devm_clk_get(dev, id);
+
+	if (PTR_ERR(clk) == -ENODATA)
+		return NULL;
+
+	return clk;
+}
 
 /**
  * clk_release_all() - Disable (turn off)/Free an array of previously
@@ -291,18 +284,37 @@ clk_get_by_name_nodev(ofnode node, const char *name, struct clk *clk)
 	return -ENOSYS;
 }
 
-static inline int
-clk_get_by_name_nodev_optional(ofnode node, const char *name, struct clk *clk)
-{
-	return -ENOSYS;
-}
-
 static inline int clk_release_all(struct clk *clk, int count)
 {
 	return -ENOSYS;
 }
 #endif
 
+/**
+ * clk_get_by_name_nodev_optional - Get/request an optinonal clock by name
+ *		without a device.
+ * @node:	The client ofnode.
+ * @name:	The name of the clock to request.
+ * @name:	The name of the clock to request, within the client's list of
+ *		clocks.
+ * @clock:	A pointer to a clock struct to initialize.
+ *
+ * Behaves the same as clk_get_by_name_nodev() except where there is
+ * no clock producer, in this case, skip the error number -ENODATA, and
+ * the function returns 0.
+ */
+static inline int clk_get_by_name_nodev_optional(ofnode node, const char *name,
+						 struct clk *clk)
+{
+	int ret;
+
+	ret = clk_get_by_name_nodev(node, name, clk);
+	if (ret == -ENODATA)
+		return 0;
+
+	return ret;
+}
+
 /**
  * enum clk_defaults_stage - What stage clk_set_defaults() is called at
  * @CLK_DEFAULTS_PRE: Called before probe. Setting of defaults for clocks owned
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/5] clk: Add client API to HTML docs
  2021-12-01 18:43 [PATCH 0/5] clk: Clean up optional helpers, and add API docs to HTML Sean Anderson
  2021-12-01 18:43 ` [PATCH 1/5] clk: Rename clk_get_optional_nodev Sean Anderson
  2021-12-01 18:43 ` [PATCH 2/5] clk: Inline clk_get_*_optional Sean Anderson
@ 2021-12-01 18:43 ` Sean Anderson
  2021-12-01 18:57   ` Heinrich Schuchardt
  2021-12-01 18:43 ` [PATCH 4/5] clk: Add driver " Sean Anderson
  2021-12-01 18:43 ` [PATCH 5/5] clk: Add clk_get_by_name_optional Sean Anderson
  4 siblings, 1 reply; 12+ messages in thread
From: Sean Anderson @ 2021-12-01 18:43 UTC (permalink / raw)
  To: u-boot
  Cc: Dario Binacchi, Simon Glass, Lukasz Majewski, Sean Anderson,
	Heinrich Schuchardt

This converts the existing client (aka clk.h) documentation to kernel doc
format, and adds it to the HTML docs. I have tried to preserve existing
comments as much as possible, refraining from semantic changes.

Signed-off-by: Sean Anderson <seanga2@gmail.com>
---

 doc/api/clk.rst   |  13 +++
 doc/api/index.rst |   1 +
 include/clk.h     | 229 +++++++++++++++++++++++-----------------------
 3 files changed, 129 insertions(+), 114 deletions(-)
 create mode 100644 doc/api/clk.rst

diff --git a/doc/api/clk.rst b/doc/api/clk.rst
new file mode 100644
index 0000000000..7eb3b5645a
--- /dev/null
+++ b/doc/api/clk.rst
@@ -0,0 +1,13 @@
+.. SPDX-License-Identifier: GPL-2.0+
+
+Clock API
+=========
+
+.. kernel-doc:: include/clk.h
+   :doc: Overview
+
+Client API
+----------
+
+.. kernel-doc:: include/clk.h
+   :internal:
diff --git a/doc/api/index.rst b/doc/api/index.rst
index 806c7385a6..83d4f3e5c1 100644
--- a/doc/api/index.rst
+++ b/doc/api/index.rst
@@ -6,6 +6,7 @@ U-Boot API documentation
 .. toctree::
    :maxdepth: 2
 
+   clk
    dfu
    efi
    getopt
diff --git a/include/clk.h b/include/clk.h
index c3d4cc0ecf..103ef16bf9 100644
--- a/include/clk.h
+++ b/include/clk.h
@@ -14,6 +14,8 @@
 #include <linux/types.h>
 
 /**
+ * DOC: Overview
+ *
  * A clock is a hardware signal that oscillates autonomously at a specific
  * frequency and duty cycle. Most hardware modules require one or more clock
  * signal to drive their operation. Clock signals are typically generated
@@ -34,22 +36,22 @@ struct udevice;
 
 /**
  * struct clk - A handle to (allowing control of) a single clock.
- *
- * Clients provide storage for clock handles. The content of the structure is
- * managed solely by the clock API and clock drivers. A clock struct is
- * initialized by "get"ing the clock struct. The clock struct is passed to all
- * other clock APIs to identify which clock signal to operate upon.
- *
  * @dev: The device which implements the clock signal.
  * @rate: The clock rate (in HZ).
- * @flags: Flags used across common clock structure (e.g. CLK_)
+ * @flags: Flags used across common clock structure (e.g. %CLK_)
  *         Clock IP blocks specific flags (i.e. mux, div, gate, etc) are defined
- *         in struct's for those devices (e.g. struct clk_mux).
+ *         in struct's for those devices (e.g. &struct clk_mux).
+ * @enable_count: The number of times this clock has been enabled.
  * @id: The clock signal ID within the provider.
  * @data: An optional data field for scenarios where a single integer ID is not
  *	  sufficient. If used, it can be populated through an .of_xlate op and
  *	  processed during the various clock ops.
  *
+ * Clients provide storage for clock handles. The content of the structure is
+ * managed solely by the clock API and clock drivers. A clock struct is
+ * initialized by "get"ing the clock struct. The clock struct is passed to all
+ * other clock APIs to identify which clock signal to operate upon.
+ *
  * Should additional information to identify and configure any clock signal
  * for any provider be required in the future, the struct could be expanded to
  * either (a) add more fields to allow clock providers to store additional
@@ -72,15 +74,14 @@ struct clk {
 
 /**
  * struct clk_bulk - A handle to (allowing control of) a bulk of clocks.
+ * @clks: An array of clock handles.
+ * @count: The number of clock handles in the clks array.
  *
  * Clients provide storage for the clock bulk. The content of the structure is
  * managed solely by the clock API. A clock bulk struct is
  * initialized by "get"ing the clock bulk struct.
  * The clock bulk struct is passed to all other bulk clock APIs to apply
  * the API to all the clock in the bulk struct.
- *
- * @clks: An array of clock handles.
- * @count: The number of clock handles in the clks array.
  */
 struct clk_bulk {
 	struct clk *clks;
@@ -91,16 +92,19 @@ struct clk_bulk {
 struct phandle_1_arg;
 /**
  * clk_get_by_phandle() - Get a clock by its phandle information (of-platadata)
+ * @dev: Device containing the phandle
+ * @cells: Phandle info
+ * @clk: A pointer to a clock struct to initialise
  *
  * This function is used when of-platdata is enabled.
  *
  * This looks up a clock using the phandle info. With dtoc, each phandle in the
- * 'clocks' property is transformed into an idx representing the device. For
- * example:
+ * 'clocks' property is transformed into an idx representing the device.
+ * For example::
  *
  *	clocks = <&dpll_mpu_ck 23>;
  *
- * might result in:
+ * might result in::
  *
  *	.clocks = {1, {23}},},
  *
@@ -109,16 +113,17 @@ struct phandle_1_arg;
  * this example it would return a clock containing the 'dpll_mpu_ck' device and
  * the clock ID 23.
  *
- * @dev: Device containing the phandle
- * @cells: Phandle info
- * @clock: A pointer to a clock struct to initialise
- * @return 0 if OK, or a negative error code.
+ * Return: 0 if OK, or a negative error code.
  */
 int clk_get_by_phandle(struct udevice *dev, const struct phandle_1_arg *cells,
 		       struct clk *clk);
 
 /**
  * clk_get_by_index() - Get/request a clock by integer index.
+ * @dev:	The client device.
+ * @index:	The index of the clock to request, within the client's list of
+ *		clocks.
+ * @clk:	A pointer to a clock struct to initialize.
  *
  * This looks up and requests a clock. The index is relative to the client
  * device; each device is assumed to have n clocks associated with it somehow,
@@ -126,30 +131,28 @@ int clk_get_by_phandle(struct udevice *dev, const struct phandle_1_arg *cells,
  * device clock indices to provider clocks may be via device-tree properties,
  * board-provided mapping tables, or some other mechanism.
  *
- * @dev:	The client device.
- * @index:	The index of the clock to request, within the client's list of
- *		clocks.
- * @clock	A pointer to a clock struct to initialize.
- * @return 0 if OK, or a negative error code.
+ * Return: 0 if OK, or a negative error code.
  */
 int clk_get_by_index(struct udevice *dev, int index, struct clk *clk);
 
 /**
- * clk_get_by_index_nodev - Get/request a clock by integer index
- * without a device.
+ * clk_get_by_index_nodev() - Get/request a clock by integer index without a
+ *                            device.
+ * @node:	The client ofnode.
+ * @index:	The index of the clock to request, within the client's list of
+ *		clocks.
+ * @clk:	A pointer to a clock struct to initialize.
  *
  * This is a version of clk_get_by_index() that does not use a device.
  *
- * @node:	The client ofnode.
- * @index:	The index of the clock to request, within the client's list of
- *		clocks.
- * @clock	A pointer to a clock struct to initialize.
- * @return 0 if OK, or a negative error code.
+ * Return: 0 if OK, or a negative error code.
  */
 int clk_get_by_index_nodev(ofnode node, int index, struct clk *clk);
 
 /**
- * clk_get_bulk - Get/request all clocks of a device.
+ * clk_get_bulk() - Get/request all clocks of a device.
+ * @dev:	The client device.
+ * @bulk:	A pointer to a clock bulk struct to initialize.
  *
  * This looks up and requests all clocks of the client device; each device is
  * assumed to have n clocks associated with it somehow, and this function finds
@@ -157,14 +160,16 @@ int clk_get_by_index_nodev(ofnode node, int index, struct clk *clk);
  * device clock indices to provider clocks may be via device-tree properties,
  * board-provided mapping tables, or some other mechanism.
  *
- * @dev:	The client device.
- * @bulk	A pointer to a clock bulk struct to initialize.
- * @return 0 if OK, or a negative error code.
+ * Return: 0 if OK, or a negative error code.
  */
 int clk_get_bulk(struct udevice *dev, struct clk_bulk *bulk);
 
 /**
- * clk_get_by_name - Get/request a clock by name.
+ * clk_get_by_name() - Get/request a clock by name.
+ * @dev:	The client device.
+ * @name:	The name of the clock to request, within the client's list of
+ *		clocks.
+ * @clk:	A pointer to a clock struct to initialize.
  *
  * This looks up and requests a clock. The name is relative to the client
  * device; each device is assumed to have n clocks associated with it somehow,
@@ -172,55 +177,51 @@ int clk_get_bulk(struct udevice *dev, struct clk_bulk *bulk);
  * device clock names to provider clocks may be via device-tree properties,
  * board-provided mapping tables, or some other mechanism.
  *
- * @dev:	The client device.
- * @name:	The name of the clock to request, within the client's list of
- *		clocks.
- * @clock:	A pointer to a clock struct to initialize.
- * @return 0 if OK, or a negative error code.
+ * Return: 0 if OK, or a negative error code.
  */
 int clk_get_by_name(struct udevice *dev, const char *name, struct clk *clk);
 
 /**
  * clk_get_by_name_nodev - Get/request a clock by name without a device.
+ * @node:	The client ofnode.
+ * @name:	The name of the clock to request, within the client's list of
+ *		clocks.
+ * @clk:	A pointer to a clock struct to initialize.
  *
  * This is a version of clk_get_by_name() that does not use a device.
  *
- * @node:	The client ofnode.
- * @name:	The name of the clock to request, within the client's list of
- *		clocks.
- * @clock:	A pointer to a clock struct to initialize.
- * @return 0 if OK, or a negative error code.
+ * Return: 0 if OK, or a negative error code.
  */
 int clk_get_by_name_nodev(ofnode node, const char *name, struct clk *clk);
 
 /**
- * devm_clk_get - lookup and obtain a managed reference to a clock producer.
+ * devm_clk_get() - lookup and obtain a managed reference to a clock producer.
  * @dev: device for clock "consumer"
  * @id: clock consumer ID
  *
- * Returns a struct clk corresponding to the clock producer, or
- * valid IS_ERR() condition containing errno.  The implementation
- * uses @dev and @id to determine the clock consumer, and thereby
- * the clock producer.  (IOW, @id may be identical strings, but
- * clk_get may return different clock producers depending on @dev.)
+ * The implementation uses @dev and @id to determine the clock consumer, and
+ * thereby the clock producer. (IOW, @id may be identical strings, but clk_get
+ * may return different clock producers depending on @dev.)
  *
  * Drivers must assume that the clock source is not enabled.
  *
- * devm_clk_get should not be called from within interrupt context.
- *
  * The clock will automatically be freed when the device is unbound
  * from the bus.
+ *
+ * Return:
+ * a struct clk corresponding to the clock producer, or
+ * valid IS_ERR() condition containing errno
  */
 struct clk *devm_clk_get(struct udevice *dev, const char *id);
 
 /**
- * devm_clk_get_optional - lookup and obtain a managed reference to an optional
- *			   clock producer.
+ * devm_clk_get_optional() - lookup and obtain a managed reference to an
+ *                           optional clock producer.
  * @dev: device for clock "consumer"
  * @id: clock consumer ID
  *
  * Behaves the same as devm_clk_get() except where there is no clock producer.
- * In this case, instead of returning -ENOENT, the function returns NULL.
+ * In this case, instead of returning -%ENOENT, the function returns NULL.
  */
 static inline struct clk *devm_clk_get_optional(struct udevice *dev,
 						const char *id)
@@ -236,14 +237,15 @@ static inline struct clk *devm_clk_get_optional(struct udevice *dev,
 /**
  * clk_release_all() - Disable (turn off)/Free an array of previously
  * requested clocks.
- *
- * For each clock contained in the clock array, this function will check if
- * clock has been previously requested and then will disable and free it.
- *
  * @clk:	A clock struct array that was previously successfully
  *		requested by clk_request/get_by_*().
- * @count	Number of clock contained in the array
- * @return zero on success, or -ve error code.
+ * @count:	Number of clock contained in the array
+ *
+ * For each clock contained in the clock array, this function will check if
+ * clock has been previously requested and then will disable and free it.
+ *
+ *
+ * Return: zero on success, or -ve error code.
  */
 int clk_release_all(struct clk *clk, int count);
 
@@ -297,10 +299,10 @@ static inline int clk_release_all(struct clk *clk, int count)
  * @name:	The name of the clock to request.
  * @name:	The name of the clock to request, within the client's list of
  *		clocks.
- * @clock:	A pointer to a clock struct to initialize.
+ * @clk:	A pointer to a clock struct to initialize.
  *
  * Behaves the same as clk_get_by_name_nodev() except where there is
- * no clock producer, in this case, skip the error number -ENODATA, and
+ * no clock producer, in this case, skip the error number -%ENODATA, and
  * the function returns 0.
  */
 static inline int clk_get_by_name_nodev_optional(ofnode node, const char *name,
@@ -340,12 +342,13 @@ enum clk_defaults_stage {
 
 #if CONFIG_IS_ENABLED(OF_REAL) && CONFIG_IS_ENABLED(CLK)
 /**
- * clk_set_defaults - Process 'assigned-{clocks/clock-parents/clock-rates}'
+ * clk_set_defaults - Process ``assigned-{clocks/clock-parents/clock-rates}``
  *                    properties to configure clocks
- *
  * @dev:        A device to process (the ofnode associated with this device
  *              will be processed).
  * @stage:	The stage of the probing process this function is called during.
+ *
+ * Return: zero on success, or -ve error code.
  */
 int clk_set_defaults(struct udevice *dev, enum clk_defaults_stage stage);
 #else
@@ -358,13 +361,13 @@ static inline int clk_set_defaults(struct udevice *dev, int stage)
 /**
  * clk_release_bulk() - Disable (turn off)/Free an array of previously
  * requested clocks in a clock bulk struct.
+ * @bulk:	A clock bulk struct that was previously successfully
+ *		requested by clk_get_bulk().
  *
  * For each clock contained in the clock bulk struct, this function will check
  * if clock has been previously requested and then will disable and free it.
  *
- * @clk:	A clock bulk struct that was previously successfully
- *		requested by clk_get_bulk().
- * @return zero on success, or -ve error code.
+ * Return: zero on success, or -ve error code.
  */
 static inline int clk_release_bulk(struct clk_bulk *bulk)
 {
@@ -373,134 +376,134 @@ static inline int clk_release_bulk(struct clk_bulk *bulk)
 
 #if CONFIG_IS_ENABLED(CLK)
 /**
- * clk_request - Request a clock by provider-specific ID.
+ * clk_request() - Request a clock by provider-specific ID.
+ * @dev:	The clock provider device.
+ * @clk:	A pointer to a clock struct to initialize. The caller must
+ *		have already initialized any field in this struct which the
+ *		clock provider uses to identify the clock.
  *
  * This requests a clock using a provider-specific ID. Generally, this function
  * should not be used, since clk_get_by_index/name() provide an interface that
  * better separates clients from intimate knowledge of clock providers.
  * However, this function may be useful in core SoC-specific code.
  *
- * @dev:	The clock provider device.
- * @clock:	A pointer to a clock struct to initialize. The caller must
- *		have already initialized any field in this struct which the
- *		clock provider uses to identify the clock.
- * @return 0 if OK, or a negative error code.
+ * Return: 0 if OK, or a negative error code.
  */
 int clk_request(struct udevice *dev, struct clk *clk);
 
 /**
- * clk_free - Free a previously requested clock.
- *
- * @clock:	A clock struct that was previously successfully requested by
+ * clk_free() - Free a previously requested clock.
+ * @clk:	A clock struct that was previously successfully requested by
  *		clk_request/get_by_*().
- * @return 0 if OK, or a negative error code.
+ *
+ * Return: 0 if OK, or a negative error code.
  */
 int clk_free(struct clk *clk);
 
 /**
  * clk_get_rate() - Get current clock rate.
- *
  * @clk:	A clock struct that was previously successfully requested by
  *		clk_request/get_by_*().
- * @return clock rate in Hz, or -ve error code.
+ *
+ * Return: clock rate in Hz, or -ve error code.
  */
 ulong clk_get_rate(struct clk *clk);
 
 /**
  * clk_get_parent() - Get current clock's parent.
- *
  * @clk:	A clock struct that was previously successfully requested by
  *		clk_request/get_by_*().
- * @return pointer to parent's struct clk, or error code passed as pointer
+ *
+ * Return: pointer to parent's struct clk, or error code passed as pointer
  */
 struct clk *clk_get_parent(struct clk *clk);
 
 /**
  * clk_get_parent_rate() - Get parent of current clock rate.
- *
  * @clk:	A clock struct that was previously successfully requested by
  *		clk_request/get_by_*().
- * @return clock rate in Hz, or -ve error code.
+ *
+ * Return: clock rate in Hz, or -ve error code.
  */
 long long clk_get_parent_rate(struct clk *clk);
 
 /**
  * clk_round_rate() - Adjust a rate to the exact rate a clock can provide
+ * @clk: A clock struct that was previously successfully requested by
+ *       clk_request/get_by_*().
+ * @rate: desired clock rate in Hz.
  *
  * This answers the question "if I were to pass @rate to clk_set_rate(),
  * what clock rate would I end up with?" without changing the hardware
- * in any way.  In other words:
+ * in any way. In other words::
  *
  *   rate = clk_round_rate(clk, r);
  *
- * and:
+ * and::
  *
  *   rate = clk_set_rate(clk, r);
  *
  * are equivalent except the former does not modify the clock hardware
  * in any way.
  *
- * @clk: A clock struct that was previously successfully requested by
- *       clk_request/get_by_*().
- * @rate: desired clock rate in Hz.
- * @return rounded rate in Hz, or -ve error code.
+ * Return: rounded rate in Hz, or -ve error code.
  */
 ulong clk_round_rate(struct clk *clk, ulong rate);
 
 /**
  * clk_set_rate() - Set current clock rate.
- *
  * @clk:	A clock struct that was previously successfully requested by
  *		clk_request/get_by_*().
  * @rate:	New clock rate in Hz.
- * @return new rate, or -ve error code.
+ *
+ * Return: new rate, or -ve error code.
  */
 ulong clk_set_rate(struct clk *clk, ulong rate);
 
 /**
  * clk_set_parent() - Set current clock parent.
- *
  * @clk:	A clock struct that was previously successfully requested by
  *		clk_request/get_by_*().
  * @parent:	A clock struct that was previously successfully requested by
  *		clk_request/get_by_*().
- * @return new rate, or -ve error code.
+ *
+ * Return: new rate, or -ve error code.
  */
 int clk_set_parent(struct clk *clk, struct clk *parent);
 
 /**
  * clk_enable() - Enable (turn on) a clock.
- *
  * @clk:	A clock struct that was previously successfully requested by
  *		clk_request/get_by_*().
- * @return zero on success, or -ve error code.
+ *
+ * Return: zero on success, or -ve error code.
  */
 int clk_enable(struct clk *clk);
 
 /**
  * clk_enable_bulk() - Enable (turn on) all clocks in a clock bulk struct.
- *
  * @bulk:	A clock bulk struct that was previously successfully requested
  *		by clk_get_bulk().
- * @return zero on success, or -ve error code.
+ *
+ * Return: zero on success, or -ve error code.
  */
 int clk_enable_bulk(struct clk_bulk *bulk);
 
 /**
  * clk_disable() - Disable (turn off) a clock.
- *
  * @clk:	A clock struct that was previously successfully requested by
  *		clk_request/get_by_*().
- * @return zero on success, or -ve error code.
+ *
+ * Return: zero on success, or -ve error code.
  */
 int clk_disable(struct clk *clk);
 
 /**
  * clk_disable_bulk() - Disable (turn off) all clocks in a clock bulk struct.
- *
  * @bulk:	A clock bulk struct that was previously successfully requested
  *		by clk_get_bulk().
- * @return zero on success, or -ve error code.
+ *
+ * Return: zero on success, or -ve error code.
  */
 int clk_disable_bulk(struct clk_bulk *bulk);
 
@@ -509,30 +512,28 @@ int clk_disable_bulk(struct clk_bulk *bulk);
  * @p: clk compared against q
  * @q: clk compared against p
  *
- * Returns true if the two struct clk pointers both point to the same hardware
- * clock node.
- *
- * Returns false otherwise. Note that two NULL clks are treated as matching.
+ * Return:
+ * %true if the two struct clk pointers both point to the same hardware clock
+ * node, and %false otherwise. Note that two %NULL clks are treated as matching.
  */
 bool clk_is_match(const struct clk *p, const struct clk *q);
 
 /**
  * clk_get_by_id() - Get the clock by its ID
- *
  * @id:	The clock ID to search for
  *
  * @clkp:	A pointer to clock struct that has been found among added clocks
  *              to UCLASS_CLK
- * @return zero on success, or -ENOENT on error
+ *
+ * Return: zero on success, or -%ENOENT on error
  */
 int clk_get_by_id(ulong id, struct clk **clkp);
 
 /**
  * clk_dev_binded() - Check whether the clk has a device binded
+ * @clk:	A pointer to the clk
  *
- * @clk		A pointer to the clk
- *
- * @return true on binded, or false on no
+ * Return: %true if binded, or %false
  */
 bool clk_dev_binded(struct clk *clk);
 
@@ -616,9 +617,9 @@ static inline bool clk_dev_binded(struct clk *clk)
 
 /**
  * clk_valid() - check if clk is valid
- *
  * @clk:	the clock to check
- * @return true if valid, or false
+ *
+ * Return: %true if valid, or %false
  */
 static inline bool clk_valid(struct clk *clk)
 {
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 4/5] clk: Add driver API to HTML docs
  2021-12-01 18:43 [PATCH 0/5] clk: Clean up optional helpers, and add API docs to HTML Sean Anderson
                   ` (2 preceding siblings ...)
  2021-12-01 18:43 ` [PATCH 3/5] clk: Add client API to HTML docs Sean Anderson
@ 2021-12-01 18:43 ` Sean Anderson
  2021-12-01 18:57   ` Heinrich Schuchardt
  2021-12-01 18:43 ` [PATCH 5/5] clk: Add clk_get_by_name_optional Sean Anderson
  4 siblings, 1 reply; 12+ messages in thread
From: Sean Anderson @ 2021-12-01 18:43 UTC (permalink / raw)
  To: u-boot
  Cc: Dario Binacchi, Simon Glass, Lukasz Majewski, Sean Anderson,
	Heinrich Schuchardt

This converts the existing driver API docs (clk-uclass.h) to kernel doc
format and adds them to the HTML documentation. Because the kernel doc
sphinx converter does not handle functions in structs very well, the
individual methods are documented separately. This is primarily inspired by
the phylink documentation [1], which uses this trick extensively.

[1] https://www.kernel.org/doc/html/latest/networking/kapi.html#c.phylink_mac_ops

Signed-off-by: Sean Anderson <seanga2@gmail.com>
---

 doc/api/clk.rst      |   6 ++
 include/clk-uclass.h | 187 +++++++++++++++++++++++++------------------
 2 files changed, 115 insertions(+), 78 deletions(-)

diff --git a/doc/api/clk.rst b/doc/api/clk.rst
index 7eb3b5645a..7c27066928 100644
--- a/doc/api/clk.rst
+++ b/doc/api/clk.rst
@@ -11,3 +11,9 @@ Client API
 
 .. kernel-doc:: include/clk.h
    :internal:
+
+Driver API
+----------
+
+.. kernel-doc:: include/clk-uclass.h
+   :internal:
diff --git a/include/clk-uclass.h b/include/clk-uclass.h
index 50e8681b55..c18ecfdf5f 100644
--- a/include/clk-uclass.h
+++ b/include/clk-uclass.h
@@ -16,96 +16,127 @@ struct ofnode_phandle_args;
 
 /**
  * struct clk_ops - The functions that a clock driver must implement.
+ * @of_xlate: Translate a client's device-tree (OF) clock specifier.
+ * @request: Request a translated clock.
+ * @rfree: Free a previously requested clock.
+ * @round_rate: Adjust a rate to the exact rate a clock can provide.
+ * @get_rate: Get current clock rate.
+ * @set_rate: Set current clock rate.
+ * @set_parent: Set current clock parent
+ * @enable: Enable a clock.
+ * @disable: Disable a clock.
+ *
+ * The individual methods are described more fully below.
  */
 struct clk_ops {
-	/**
-	 * of_xlate - Translate a client's device-tree (OF) clock specifier.
-	 *
-	 * The clock core calls this function as the first step in implementing
-	 * a client's clk_get_by_*() call.
-	 *
-	 * If this function pointer is set to NULL, the clock core will use a
-	 * default implementation, which assumes #clock-cells = <1>, and that
-	 * the DT cell contains a simple integer clock ID.
-	 *
-	 * At present, the clock API solely supports device-tree. If this
-	 * changes, other xxx_xlate() functions may be added to support those
-	 * other mechanisms.
-	 *
-	 * @clock:	The clock struct to hold the translation result.
-	 * @args:	The clock specifier values from device tree.
-	 * @return 0 if OK, or a negative error code.
-	 */
 	int (*of_xlate)(struct clk *clock,
 			struct ofnode_phandle_args *args);
-	/**
-	 * request - Request a translated clock.
-	 *
-	 * The clock core calls this function as the second step in
-	 * implementing a client's clk_get_by_*() call, following a successful
-	 * xxx_xlate() call, or as the only step in implementing a client's
-	 * clk_request() call.
-	 *
-	 * @clock:	The clock struct to request; this has been fille in by
-	 *		a previoux xxx_xlate() function call, or by the caller
-	 *		of clk_request().
-	 * @return 0 if OK, or a negative error code.
-	 */
 	int (*request)(struct clk *clock);
-	/**
-	 * rfree - Free a previously requested clock.
-	 *
-	 * This is the implementation of the client clk_free() API.
-	 *
-	 * @clock:	The clock to free.
-	 * @return 0 if OK, or a negative error code.
-	 */
 	int (*rfree)(struct clk *clock);
-	/**
-	 * round_rate() - Adjust a rate to the exact rate a clock can provide.
-	 *
-	 * @clk:	The clock to manipulate.
-	 * @rate:	Desidered clock rate in Hz.
-	 * @return rounded rate in Hz, or -ve error code.
-	 */
 	ulong (*round_rate)(struct clk *clk, ulong rate);
-	/**
-	 * get_rate() - Get current clock rate.
-	 *
-	 * @clk:	The clock to query.
-	 * @return clock rate in Hz, or -ve error code
-	 */
 	ulong (*get_rate)(struct clk *clk);
-	/**
-	 * set_rate() - Set current clock rate.
-	 *
-	 * @clk:	The clock to manipulate.
-	 * @rate:	New clock rate in Hz.
-	 * @return new rate, or -ve error code.
-	 */
 	ulong (*set_rate)(struct clk *clk, ulong rate);
-	/**
-	 * set_parent() - Set current clock parent
-	 *
-	 * @clk:        The clock to manipulate.
-	 * @parent:     New clock parent.
-	 * @return zero on success, or -ve error code.
-	 */
 	int (*set_parent)(struct clk *clk, struct clk *parent);
-	/**
-	 * enable() - Enable a clock.
-	 *
-	 * @clk:	The clock to manipulate.
-	 * @return zero on success, or -ve error code.
-	 */
 	int (*enable)(struct clk *clk);
-	/**
-	 * disable() - Disable a clock.
-	 *
-	 * @clk:	The clock to manipulate.
-	 * @return zero on success, or -ve error code.
-	 */
 	int (*disable)(struct clk *clk);
 };
 
+#if 0 /* For documentation only */
+/**
+ * of_xlate() - Translate a client's device-tree (OF) clock specifier.
+ * @clock:	The clock struct to hold the translation result.
+ * @args:	The clock specifier values from device tree.
+ *
+ * The clock core calls this function as the first step in implementing
+ * a client's clk_get_by_*() call.
+ *
+ * If this function pointer is set to NULL, the clock core will use a
+ * default implementation, which assumes #clock-cells = <1>, and that
+ * the DT cell contains a simple integer clock ID.
+ *
+ * At present, the clock API solely supports device-tree. If this
+ * changes, other xxx_xlate() functions may be added to support those
+ * other mechanisms.
+ *
+ * Return: 0 if OK, or a negative error code.
+ */
+int of_xlate(struct clk *clock, struct ofnode_phandle_args *args);
+
+/**
+ * request() - Request a translated clock.
+ *
+ * The clock core calls this function as the second step in
+ * implementing a client's clk_get_by_*() call, following a successful
+ * xxx_xlate() call, or as the only step in implementing a client's
+ * clk_request() call.
+ *
+ * @clock:	The clock struct to request; this has been fille in by
+ *		a previoux xxx_xlate() function call, or by the caller
+ *		of clk_request().
+ * Return: 0 if OK, or a negative error code.
+ */
+int request(struct clk *clock);
+
+/**
+ * rfree() - Free a previously requested clock.
+ *
+ * This is the implementation of the client clk_free() API.
+ *
+ * @clock:	The clock to free.
+ * Return: 0 if OK, or a negative error code.
+ */
+int rfree(struct clk *clock);
+
+/**
+ * round_rate() - Adjust a rate to the exact rate a clock can provide.
+ *
+ * @clk:	The clock to manipulate.
+ * @rate:	Desidered clock rate in Hz.
+ * Return: rounded rate in Hz, or -ve error code.
+ */
+ulong round_rate(struct clk *clk, ulong rate);
+
+/**
+ * get_rate() - Get current clock rate.
+ *
+ * @clk:	The clock to query.
+ * Return: clock rate in Hz, or -ve error code
+ */
+ulong get_rate(struct clk *clk);
+
+/**
+ * set_rate() - Set current clock rate.
+ *
+ * @clk:	The clock to manipulate.
+ * @rate:	New clock rate in Hz.
+ * Return: new rate, or -ve error code.
+ */
+ulong set_rate(struct clk *clk, ulong rate);
+
+/**
+ * set_parent() - Set current clock parent
+ *
+ * @clk:        The clock to manipulate.
+ * @parent:     New clock parent.
+ * Return: zero on success, or -ve error code.
+ */
+int set_parent(struct clk *clk, struct clk *parent);
+
+/**
+ * enable() - Enable a clock.
+ *
+ * @clk:	The clock to manipulate.
+ * Return: zero on success, or -ve error code.
+ */
+int enable(struct clk *clk);
+
+/**
+ * disable() - Disable a clock.
+ *
+ * @clk:	The clock to manipulate.
+ * Return: zero on success, or -ve error code.
+ */
+int disable(struct clk *clk);
+#endif
+
 #endif
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 5/5] clk: Add clk_get_by_name_optional
  2021-12-01 18:43 [PATCH 0/5] clk: Clean up optional helpers, and add API docs to HTML Sean Anderson
                   ` (3 preceding siblings ...)
  2021-12-01 18:43 ` [PATCH 4/5] clk: Add driver " Sean Anderson
@ 2021-12-01 18:43 ` Sean Anderson
  2021-12-02 10:35   ` Neil Armstrong
  2021-12-02 13:43   ` Simon Glass
  4 siblings, 2 replies; 12+ messages in thread
From: Sean Anderson @ 2021-12-01 18:43 UTC (permalink / raw)
  To: u-boot
  Cc: Dario Binacchi, Simon Glass, Lukasz Majewski, Sean Anderson,
	Michal Simek, Neil Armstrong, u-boot-amlogic

This adds a helper function for clk_get_by_name in cases where the clock is
optional. Hopefully this helps point driver writers in the right direction.
Also convert some existing users.

Signed-off-by: Sean Anderson <seanga2@gmail.com>
---

 drivers/clk/clk_zynq.c  |  5 +++--
 drivers/rng/meson-rng.c |  4 ++--
 include/clk.h           | 24 ++++++++++++++++++++++++
 3 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/clk_zynq.c b/drivers/clk/clk_zynq.c
index 18915c3e04..e80500e382 100644
--- a/drivers/clk/clk_zynq.c
+++ b/drivers/clk/clk_zynq.c
@@ -472,8 +472,9 @@ static int zynq_clk_probe(struct udevice *dev)
 
 	for (i = 0; i < 2; i++) {
 		sprintf(name, "gem%d_emio_clk", i);
-		ret = clk_get_by_name(dev, name, &priv->gem_emio_clk[i]);
-		if (ret < 0 && ret != -ENODATA) {
+		ret = clk_get_by_name_optional(dev, name,
+					       &priv->gem_emio_clk[i]);
+		if (ret) {
 			dev_err(dev, "failed to get %s clock\n", name);
 			return ret;
 		}
diff --git a/drivers/rng/meson-rng.c b/drivers/rng/meson-rng.c
index 5a4f45ad5a..e0a1e8c7e0 100644
--- a/drivers/rng/meson-rng.c
+++ b/drivers/rng/meson-rng.c
@@ -91,8 +91,8 @@ static int meson_rng_of_to_plat(struct udevice *dev)
 		return -ENODEV;
 
 	/* Get optional "core" clock */
-	err = clk_get_by_name(dev, "core", &pdata->clk);
-	if (err && err != -ENODATA)
+	err = clk_get_by_name_optional(dev, "core", &pdata->clk);
+	if (err)
 		return err;
 
 	return 0;
diff --git a/include/clk.h b/include/clk.h
index 103ef16bf9..36721188d0 100644
--- a/include/clk.h
+++ b/include/clk.h
@@ -292,6 +292,30 @@ static inline int clk_release_all(struct clk *clk, int count)
 }
 #endif
 
+/**
+ * clk_get_by_name_optional() - Get/request a optional clock by name.
+ * @dev:	The client device.
+ * @name:	The name of the clock to request, within the client's list of
+ *		clocks.
+ * @clk:	A pointer to a clock struct to initialize.
+ *
+ * Behaves the same as clk_get_by_name(), except when there is no clock
+ * provider. In the latter case, return 0.
+ *
+ * Return: 0 if OK, or a negative error code.
+ */
+static inline int clk_get_by_name_optional(struct udevice *dev,
+					   const char *name, struct clk *clk)
+{
+	int ret;
+
+	ret = clk_get_by_name_optional(dev, name, clk);
+	if (ret == -ENODATA)
+		return 0;
+
+	return ret;
+}
+
 /**
  * clk_get_by_name_nodev_optional - Get/request an optinonal clock by name
  *		without a device.
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/5] clk: Add client API to HTML docs
  2021-12-01 18:43 ` [PATCH 3/5] clk: Add client API to HTML docs Sean Anderson
@ 2021-12-01 18:57   ` Heinrich Schuchardt
  0 siblings, 0 replies; 12+ messages in thread
From: Heinrich Schuchardt @ 2021-12-01 18:57 UTC (permalink / raw)
  To: Sean Anderson, u-boot; +Cc: Dario Binacchi, Simon Glass, Lukasz Majewski

On 12/1/21 19:43, Sean Anderson wrote:
> This converts the existing client (aka clk.h) documentation to kernel doc
> format, and adds it to the HTML docs. I have tried to preserve existing
> comments as much as possible, refraining from semantic changes.
>
> Signed-off-by: Sean Anderson<seanga2@gmail.com>

make htmldocs shows no problems

Tested-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 4/5] clk: Add driver API to HTML docs
  2021-12-01 18:43 ` [PATCH 4/5] clk: Add driver " Sean Anderson
@ 2021-12-01 18:57   ` Heinrich Schuchardt
  0 siblings, 0 replies; 12+ messages in thread
From: Heinrich Schuchardt @ 2021-12-01 18:57 UTC (permalink / raw)
  To: Sean Anderson, u-boot; +Cc: Dario Binacchi, Simon Glass, Lukasz Majewski

On 12/1/21 19:43, Sean Anderson wrote:
> This converts the existing driver API docs (clk-uclass.h) to kernel doc
> format and adds them to the HTML documentation. Because the kernel doc
> sphinx converter does not handle functions in structs very well, the
> individual methods are documented separately. This is primarily inspired by
> the phylink documentation [1], which uses this trick extensively.
>
> [1]https://www.kernel.org/doc/html/latest/networking/kapi.html#c.phylink_mac_ops
>
> Signed-off-by: Sean Anderson<seanga2@gmail.com>


make htmldocs shows no problems

Tested-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 5/5] clk: Add clk_get_by_name_optional
  2021-12-01 18:43 ` [PATCH 5/5] clk: Add clk_get_by_name_optional Sean Anderson
@ 2021-12-02 10:35   ` Neil Armstrong
  2021-12-02 13:43   ` Simon Glass
  1 sibling, 0 replies; 12+ messages in thread
From: Neil Armstrong @ 2021-12-02 10:35 UTC (permalink / raw)
  To: Sean Anderson, u-boot
  Cc: Dario Binacchi, Simon Glass, Lukasz Majewski, Michal Simek,
	u-boot-amlogic

On 01/12/2021 19:43, Sean Anderson wrote:
> This adds a helper function for clk_get_by_name in cases where the clock is
> optional. Hopefully this helps point driver writers in the right direction.
> Also convert some existing users.
> 
> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> ---
> 
>  drivers/clk/clk_zynq.c  |  5 +++--
>  drivers/rng/meson-rng.c |  4 ++--
>  include/clk.h           | 24 ++++++++++++++++++++++++
>  3 files changed, 29 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/clk/clk_zynq.c b/drivers/clk/clk_zynq.c
> index 18915c3e04..e80500e382 100644
> --- a/drivers/clk/clk_zynq.c
> +++ b/drivers/clk/clk_zynq.c
> @@ -472,8 +472,9 @@ static int zynq_clk_probe(struct udevice *dev)
>  
>  	for (i = 0; i < 2; i++) {
>  		sprintf(name, "gem%d_emio_clk", i);
> -		ret = clk_get_by_name(dev, name, &priv->gem_emio_clk[i]);
> -		if (ret < 0 && ret != -ENODATA) {
> +		ret = clk_get_by_name_optional(dev, name,
> +					       &priv->gem_emio_clk[i]);
> +		if (ret) {
>  			dev_err(dev, "failed to get %s clock\n", name);
>  			return ret;
>  		}
> diff --git a/drivers/rng/meson-rng.c b/drivers/rng/meson-rng.c
> index 5a4f45ad5a..e0a1e8c7e0 100644
> --- a/drivers/rng/meson-rng.c
> +++ b/drivers/rng/meson-rng.c
> @@ -91,8 +91,8 @@ static int meson_rng_of_to_plat(struct udevice *dev)
>  		return -ENODEV;
>  
>  	/* Get optional "core" clock */
> -	err = clk_get_by_name(dev, "core", &pdata->clk);
> -	if (err && err != -ENODATA)
> +	err = clk_get_by_name_optional(dev, "core", &pdata->clk);
> +	if (err)
>  		return err;
>  
>  	return 0;
> diff --git a/include/clk.h b/include/clk.h
> index 103ef16bf9..36721188d0 100644
> --- a/include/clk.h
> +++ b/include/clk.h
> @@ -292,6 +292,30 @@ static inline int clk_release_all(struct clk *clk, int count)
>  }
>  #endif
>  
> +/**
> + * clk_get_by_name_optional() - Get/request a optional clock by name.
> + * @dev:	The client device.
> + * @name:	The name of the clock to request, within the client's list of
> + *		clocks.
> + * @clk:	A pointer to a clock struct to initialize.
> + *
> + * Behaves the same as clk_get_by_name(), except when there is no clock
> + * provider. In the latter case, return 0.
> + *
> + * Return: 0 if OK, or a negative error code.
> + */
> +static inline int clk_get_by_name_optional(struct udevice *dev,
> +					   const char *name, struct clk *clk)
> +{
> +	int ret;
> +
> +	ret = clk_get_by_name_optional(dev, name, clk);
> +	if (ret == -ENODATA)
> +		return 0;
> +
> +	return ret;
> +}
> +
>  /**
>   * clk_get_by_name_nodev_optional - Get/request an optinonal clock by name
>   *		without a device.
> 


For meson-rng:
Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 5/5] clk: Add clk_get_by_name_optional
  2021-12-01 18:43 ` [PATCH 5/5] clk: Add clk_get_by_name_optional Sean Anderson
  2021-12-02 10:35   ` Neil Armstrong
@ 2021-12-02 13:43   ` Simon Glass
  2021-12-02 14:25     ` Sean Anderson
  1 sibling, 1 reply; 12+ messages in thread
From: Simon Glass @ 2021-12-02 13:43 UTC (permalink / raw)
  To: Sean Anderson
  Cc: u-boot, Dario Binacchi, Lukasz Majewski, Michal Simek,
	Neil Armstrong, u-boot-amlogic

Hi Sean,

On Wed, 1 Dec 2021 at 11:43, Sean Anderson <seanga2@gmail.com> wrote:
>
> This adds a helper function for clk_get_by_name in cases where the clock is
> optional. Hopefully this helps point driver writers in the right direction.
> Also convert some existing users.
>
> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> ---
>
>  drivers/clk/clk_zynq.c  |  5 +++--
>  drivers/rng/meson-rng.c |  4 ++--
>  include/clk.h           | 24 ++++++++++++++++++++++++
>  3 files changed, 29 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/clk/clk_zynq.c b/drivers/clk/clk_zynq.c
> index 18915c3e04..e80500e382 100644
> --- a/drivers/clk/clk_zynq.c
> +++ b/drivers/clk/clk_zynq.c
> @@ -472,8 +472,9 @@ static int zynq_clk_probe(struct udevice *dev)
>
>         for (i = 0; i < 2; i++) {
>                 sprintf(name, "gem%d_emio_clk", i);
> -               ret = clk_get_by_name(dev, name, &priv->gem_emio_clk[i]);
> -               if (ret < 0 && ret != -ENODATA) {
> +               ret = clk_get_by_name_optional(dev, name,
> +                                              &priv->gem_emio_clk[i]);
> +               if (ret) {
>                         dev_err(dev, "failed to get %s clock\n", name);
>                         return ret;
>                 }
> diff --git a/drivers/rng/meson-rng.c b/drivers/rng/meson-rng.c
> index 5a4f45ad5a..e0a1e8c7e0 100644
> --- a/drivers/rng/meson-rng.c
> +++ b/drivers/rng/meson-rng.c
> @@ -91,8 +91,8 @@ static int meson_rng_of_to_plat(struct udevice *dev)
>                 return -ENODEV;
>
>         /* Get optional "core" clock */
> -       err = clk_get_by_name(dev, "core", &pdata->clk);
> -       if (err && err != -ENODATA)
> +       err = clk_get_by_name_optional(dev, "core", &pdata->clk);
> +       if (err)
>                 return err;
>
>         return 0;
> diff --git a/include/clk.h b/include/clk.h
> index 103ef16bf9..36721188d0 100644
> --- a/include/clk.h
> +++ b/include/clk.h
> @@ -292,6 +292,30 @@ static inline int clk_release_all(struct clk *clk, int count)
>  }
>  #endif
>
> +/**
> + * clk_get_by_name_optional() - Get/request a optional clock by name.

Can I suggest we rename this to ..._opt as it is shorter?

> + * @dev:       The client device.
> + * @name:      The name of the clock to request, within the client's list of
> + *             clocks.
> + * @clk:       A pointer to a clock struct to initialize.
> + *
> + * Behaves the same as clk_get_by_name(), except when there is no clock
> + * provider. In the latter case, return 0.
> + *
> + * Return: 0 if OK, or a negative error code.
> + */
> +static inline int clk_get_by_name_optional(struct udevice *dev,
> +                                          const char *name, struct clk *clk)
> +{
> +       int ret;
> +
> +       ret = clk_get_by_name_optional(dev, name, clk);
> +       if (ret == -ENODATA)
> +               return 0;
> +
> +       return ret;
> +}
> +
>  /**
>   * clk_get_by_name_nodev_optional - Get/request an optinonal clock by name
>   *             without a device.
> --
> 2.33.0
>

Regards,
SImon

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 5/5] clk: Add clk_get_by_name_optional
  2021-12-02 13:43   ` Simon Glass
@ 2021-12-02 14:25     ` Sean Anderson
  2021-12-02 15:59       ` Simon Glass
  0 siblings, 1 reply; 12+ messages in thread
From: Sean Anderson @ 2021-12-02 14:25 UTC (permalink / raw)
  To: Simon Glass
  Cc: u-boot, Dario Binacchi, Lukasz Majewski, Michal Simek,
	Neil Armstrong, u-boot-amlogic

Hi Simon,

On 12/2/21 8:43 AM, Simon Glass wrote:
> Hi Sean,
> 
> On Wed, 1 Dec 2021 at 11:43, Sean Anderson <seanga2@gmail.com> wrote:
>>
>> This adds a helper function for clk_get_by_name in cases where the clock is
>> optional. Hopefully this helps point driver writers in the right direction.
>> Also convert some existing users.
>>
>> Signed-off-by: Sean Anderson <seanga2@gmail.com>
>> ---
>>
>>   drivers/clk/clk_zynq.c  |  5 +++--
>>   drivers/rng/meson-rng.c |  4 ++--
>>   include/clk.h           | 24 ++++++++++++++++++++++++
>>   3 files changed, 29 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/clk/clk_zynq.c b/drivers/clk/clk_zynq.c
>> index 18915c3e04..e80500e382 100644
>> --- a/drivers/clk/clk_zynq.c
>> +++ b/drivers/clk/clk_zynq.c
>> @@ -472,8 +472,9 @@ static int zynq_clk_probe(struct udevice *dev)
>>
>>          for (i = 0; i < 2; i++) {
>>                  sprintf(name, "gem%d_emio_clk", i);
>> -               ret = clk_get_by_name(dev, name, &priv->gem_emio_clk[i]);
>> -               if (ret < 0 && ret != -ENODATA) {
>> +               ret = clk_get_by_name_optional(dev, name,
>> +                                              &priv->gem_emio_clk[i]);
>> +               if (ret) {
>>                          dev_err(dev, "failed to get %s clock\n", name);
>>                          return ret;
>>                  }
>> diff --git a/drivers/rng/meson-rng.c b/drivers/rng/meson-rng.c
>> index 5a4f45ad5a..e0a1e8c7e0 100644
>> --- a/drivers/rng/meson-rng.c
>> +++ b/drivers/rng/meson-rng.c
>> @@ -91,8 +91,8 @@ static int meson_rng_of_to_plat(struct udevice *dev)
>>                  return -ENODEV;
>>
>>          /* Get optional "core" clock */
>> -       err = clk_get_by_name(dev, "core", &pdata->clk);
>> -       if (err && err != -ENODATA)
>> +       err = clk_get_by_name_optional(dev, "core", &pdata->clk);
>> +       if (err)
>>                  return err;
>>
>>          return 0;
>> diff --git a/include/clk.h b/include/clk.h
>> index 103ef16bf9..36721188d0 100644
>> --- a/include/clk.h
>> +++ b/include/clk.h
>> @@ -292,6 +292,30 @@ static inline int clk_release_all(struct clk *clk, int count)
>>   }
>>   #endif
>>
>> +/**
>> + * clk_get_by_name_optional() - Get/request a optional clock by name.
> 
> Can I suggest we rename this to ..._opt as it is shorter?

This follows the general trend of suffixing _optional. For example (and
several of these are borrowed from Linux):

	clk_get_optional
	reset_control_bulk_get_optional_exclusive
	gpiod_get_optional
	platform_get_irq_byname_optional

and particularly, the Linux clock subsystem uses _optional and not _opt
as a suffix. While some of these names can get fairly long-winded, I
think we should go for consistency here.

--Sean

>> + * @dev:       The client device.
>> + * @name:      The name of the clock to request, within the client's list of
>> + *             clocks.
>> + * @clk:       A pointer to a clock struct to initialize.
>> + *
>> + * Behaves the same as clk_get_by_name(), except when there is no clock
>> + * provider. In the latter case, return 0.
>> + *
>> + * Return: 0 if OK, or a negative error code.
>> + */
>> +static inline int clk_get_by_name_optional(struct udevice *dev,
>> +                                          const char *name, struct clk *clk)
>> +{
>> +       int ret;
>> +
>> +       ret = clk_get_by_name_optional(dev, name, clk);
>> +       if (ret == -ENODATA)
>> +               return 0;
>> +
>> +       return ret;
>> +}
>> +
>>   /**
>>    * clk_get_by_name_nodev_optional - Get/request an optinonal clock by name
>>    *             without a device.
>> --
>> 2.33.0
>>
> 
> Regards,
> SImon
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 5/5] clk: Add clk_get_by_name_optional
  2021-12-02 14:25     ` Sean Anderson
@ 2021-12-02 15:59       ` Simon Glass
  0 siblings, 0 replies; 12+ messages in thread
From: Simon Glass @ 2021-12-02 15:59 UTC (permalink / raw)
  To: Sean Anderson
  Cc: u-boot, Dario Binacchi, Lukasz Majewski, Michal Simek,
	Neil Armstrong, u-boot-amlogic

Hi Sean,

On Thu, 2 Dec 2021 at 07:25, Sean Anderson <seanga2@gmail.com> wrote:
>
> Hi Simon,
>
> On 12/2/21 8:43 AM, Simon Glass wrote:
> > Hi Sean,
> >
> > On Wed, 1 Dec 2021 at 11:43, Sean Anderson <seanga2@gmail.com> wrote:
> >>
> >> This adds a helper function for clk_get_by_name in cases where the clock is
> >> optional. Hopefully this helps point driver writers in the right direction.
> >> Also convert some existing users.
> >>
> >> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> >> ---
> >>
> >>   drivers/clk/clk_zynq.c  |  5 +++--
> >>   drivers/rng/meson-rng.c |  4 ++--
> >>   include/clk.h           | 24 ++++++++++++++++++++++++
> >>   3 files changed, 29 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/clk/clk_zynq.c b/drivers/clk/clk_zynq.c
> >> index 18915c3e04..e80500e382 100644
> >> --- a/drivers/clk/clk_zynq.c
> >> +++ b/drivers/clk/clk_zynq.c
> >> @@ -472,8 +472,9 @@ static int zynq_clk_probe(struct udevice *dev)
> >>
> >>          for (i = 0; i < 2; i++) {
> >>                  sprintf(name, "gem%d_emio_clk", i);
> >> -               ret = clk_get_by_name(dev, name, &priv->gem_emio_clk[i]);
> >> -               if (ret < 0 && ret != -ENODATA) {
> >> +               ret = clk_get_by_name_optional(dev, name,
> >> +                                              &priv->gem_emio_clk[i]);
> >> +               if (ret) {
> >>                          dev_err(dev, "failed to get %s clock\n", name);
> >>                          return ret;
> >>                  }
> >> diff --git a/drivers/rng/meson-rng.c b/drivers/rng/meson-rng.c
> >> index 5a4f45ad5a..e0a1e8c7e0 100644
> >> --- a/drivers/rng/meson-rng.c
> >> +++ b/drivers/rng/meson-rng.c
> >> @@ -91,8 +91,8 @@ static int meson_rng_of_to_plat(struct udevice *dev)
> >>                  return -ENODEV;
> >>
> >>          /* Get optional "core" clock */
> >> -       err = clk_get_by_name(dev, "core", &pdata->clk);
> >> -       if (err && err != -ENODATA)
> >> +       err = clk_get_by_name_optional(dev, "core", &pdata->clk);
> >> +       if (err)
> >>                  return err;
> >>
> >>          return 0;
> >> diff --git a/include/clk.h b/include/clk.h
> >> index 103ef16bf9..36721188d0 100644
> >> --- a/include/clk.h
> >> +++ b/include/clk.h
> >> @@ -292,6 +292,30 @@ static inline int clk_release_all(struct clk *clk, int count)
> >>   }
> >>   #endif
> >>
> >> +/**
> >> + * clk_get_by_name_optional() - Get/request a optional clock by name.
> >
> > Can I suggest we rename this to ..._opt as it is shorter?
>
> This follows the general trend of suffixing _optional. For example (and
> several of these are borrowed from Linux):
>
>         clk_get_optional
>         reset_control_bulk_get_optional_exclusive
>         gpiod_get_optional
>         platform_get_irq_byname_optional
>
> and particularly, the Linux clock subsystem uses _optional and not _opt
> as a suffix. While some of these names can get fairly long-winded, I
> think we should go for consistency here.

Yes I agree.

I do wish people would consider brevity as these names are pretty
hopeless for readability.

Regards,
Simon

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2021-12-02 15:59 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-01 18:43 [PATCH 0/5] clk: Clean up optional helpers, and add API docs to HTML Sean Anderson
2021-12-01 18:43 ` [PATCH 1/5] clk: Rename clk_get_optional_nodev Sean Anderson
2021-12-01 18:43 ` [PATCH 2/5] clk: Inline clk_get_*_optional Sean Anderson
2021-12-01 18:43 ` [PATCH 3/5] clk: Add client API to HTML docs Sean Anderson
2021-12-01 18:57   ` Heinrich Schuchardt
2021-12-01 18:43 ` [PATCH 4/5] clk: Add driver " Sean Anderson
2021-12-01 18:57   ` Heinrich Schuchardt
2021-12-01 18:43 ` [PATCH 5/5] clk: Add clk_get_by_name_optional Sean Anderson
2021-12-02 10:35   ` Neil Armstrong
2021-12-02 13:43   ` Simon Glass
2021-12-02 14:25     ` Sean Anderson
2021-12-02 15:59       ` Simon Glass

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.