[PATCH v8 0/3] Add of_regulator_get_optional() and Fix MTK Power Domain Driver

[PATCH v8 0/3] Add of_regulator_get_optional() and Fix MTK Power Domain Driver

[Chen-Yu Tsai]

Hi folks,

This series is split off from my "DT hardware prober" series [1].

Changes since v7:
- Added stub versions for of_regulator_get_optional() for !CONFIG_OF
  and !CONFIG_REGULATOR
- Added new patches for devres version and converting MTK pmdomain
  driver

At ELCE, Sebastian told me about his recent work on adding regulator
supply support to the Rockchip power domain driver [2], how the MediaTek
driver has been using the existing devm_regulator_get() API and
reassigning different device nodes to the device doing the lookup, and
how the new of_regulator_get_optional() is the proper fit for this.

Patch 1 adds a new of_regulator_get_optional() function to look up
regulator supplies using device tree nodes.

Patch 2 adds a devres version of the aforementioned function at
Sebastian's request for the two power domain drivers.

Patch 3 converts the MediaTek power domain driver to use function.


Each of the latter two patches depend on the previous one at build time.
Mark, would it be possible for you to put the two regulator patches
on an immutable branch / tag? Otherwise we could have Ulf ack the
pmdomain patch and merge it through your tree. Sebastian was fine
with converting the rockchip pmdomain some time later.

Thanks
ChenYu


[1] https://lore.kernel.org/all/20240911072751.365361-1-wenst@chromium.org/
[2] https://lore.kernel.org/all/20240919091834.83572-1-sebastian.reichel@collabora.com/

Chen-Yu Tsai (3):
  regulator: Add of_regulator_get_optional() for pure DT regulator
    lookup
  regulator: Add devres version of of_regulator_get_optional()
  pmdomain: mediatek: Use OF-specific regulator API to get power domain
    supply

 drivers/pmdomain/mediatek/mtk-pm-domains.c | 12 +----
 drivers/regulator/core.c                   |  4 +-
 drivers/regulator/devres.c                 | 39 +++++++++++++++++
 drivers/regulator/internal.h               | 18 +++++---
 drivers/regulator/of_regulator.c           | 51 +++++++++++++++++++---
 include/linux/regulator/consumer.h         | 37 ++++++++++++++++
 6 files changed, 136 insertions(+), 25 deletions(-)


base-commit: 2b7275670032a98cba266bd1b8905f755b3e650f
-- 
2.46.0.792.g87dc391469-goog

[PATCH v8 1/3] regulator: Add of_regulator_get_optional() for pure DT regulator lookup

[Chen-Yu Tsai]

The to-be-introduced I2C component prober needs to enable regulator
supplies (and toggle GPIO pins) for the various components it intends
to probe. To support this, a new "pure DT lookup" method for getting
regulator supplies is needed, since the device normally requesting
the supply won't get created until after the component is probed to
be available.

Add a new of_regulator_get_optional() function for this. This mirrors
the existing regulator_get_optional() function, but is OF-specific.
The underlying code that supports the existing regulator_get*()
functions has been reworked in previous patches to support this
specific case.

Also convert an existing usage of "dev && dev->of_node" to
"dev_of_node(dev)".

Link: https://lore.kernel.org/all/20231220203537.83479-2-jernej.skrabec@gmail.com/ [1]
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
Changes since v7:
- Added stub version for !CONFIG_OF and !CONFIG_REGULATOR

Changes since v6:
- Changed reference [1] to Link: tag
- Rebased on top of commit 401d078eaf2e ("regulator: of: Refactor
  of_get_*regulator() to decrease indentation")
- Exported of_regulator_get_optional()
- Changed commit message to focus on "of_regulator_get_optional()"
- Dropped change to of_regulator_bulk_get_all()

Changes since v5:
- Used "dev_of_node(dev)" instead of "dev->of_node"
- Replaced "dev_printk" with "dev_printk()" in kerneldoc mentions
- Fixed kerneldoc "Return" section format for of_regulator_get_optional()
- Fix @np parameter name in of_regulator_dev_lookup() kerneldoc

Changes since v4:
- Restore platform-agnostic regulator consumer code to original state
- Move OF-specific regulator code to of_regulator.c (separate patch)
- Split _regulator_get() into three parts for reuse (separate patch)
- Add OF-specific _of_regulator_get() function
- Rename regulator_of_get_optional() to of_regulator_get_optional() for
  consistency
- Make of_regulator_get_optional static, as it is only used internally
- Convert of_regulator_bulk_get_all()

Changes since v3:
- New patch
---
 drivers/regulator/core.c           |  4 +--
 drivers/regulator/internal.h       |  2 ++
 drivers/regulator/of_regulator.c   | 51 ++++++++++++++++++++++++++----
 include/linux/regulator/consumer.h | 20 ++++++++++++
 4 files changed, 69 insertions(+), 8 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 1179766811f5..d0b3879f2746 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1959,8 +1959,8 @@ static struct regulator_dev *regulator_dev_lookup(struct device *dev,
 	regulator_supply_alias(&dev, &supply);
 
 	/* first do a dt based lookup */
-	if (dev && dev->of_node) {
-		r = of_regulator_dev_lookup(dev, supply);
+	if (dev_of_node(dev)) {
+		r = of_regulator_dev_lookup(dev, dev_of_node(dev), supply);
 		if (!IS_ERR(r))
 			return r;
 		if (PTR_ERR(r) == -EPROBE_DEFER)
diff --git a/drivers/regulator/internal.h b/drivers/regulator/internal.h
index 5b43f802468d..f62cacbbc729 100644
--- a/drivers/regulator/internal.h
+++ b/drivers/regulator/internal.h
@@ -67,6 +67,7 @@ static inline struct regulator_dev *dev_to_rdev(struct device *dev)
 
 #ifdef CONFIG_OF
 struct regulator_dev *of_regulator_dev_lookup(struct device *dev,
+					      struct device_node *np,
 					      const char *supply);
 struct regulator_init_data *regulator_of_get_init_data(struct device *dev,
 			         const struct regulator_desc *desc,
@@ -82,6 +83,7 @@ bool of_check_coupling_data(struct regulator_dev *rdev);
 
 #else
 static inline struct regulator_dev *of_regulator_dev_lookup(struct device *dev,
+							    struct device_node *np,
 							    const char *supply)
 {
 	return ERR_PTR(-ENODEV);
diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c
index 3f490d81abc2..358c3ed791db 100644
--- a/drivers/regulator/of_regulator.c
+++ b/drivers/regulator/of_regulator.c
@@ -588,7 +588,8 @@ static struct device_node *of_get_child_regulator(struct device_node *parent,
 
 /**
  * of_get_regulator - get a regulator device node based on supply name
- * @dev: Device pointer for the consumer (of regulator) device
+ * @dev: Device pointer for dev_printk() messages
+ * @node: Device node pointer for supply property lookup
  * @supply: regulator supply name
  *
  * Extract the regulator device node corresponding to the supply name.
@@ -596,15 +597,16 @@ static struct device_node *of_get_child_regulator(struct device_node *parent,
  * Return: Pointer to the &struct device_node corresponding to the regulator
  *	   if found, or %NULL if not found.
  */
-static struct device_node *of_get_regulator(struct device *dev, const char *supply)
+static struct device_node *of_get_regulator(struct device *dev, struct device_node *node,
+					    const char *supply)
 {
 	struct device_node *regnode = NULL;
 	char prop_name[64]; /* 64 is max size of property name */
 
-	dev_dbg(dev, "Looking up %s-supply from device tree\n", supply);
+	dev_dbg(dev, "Looking up %s-supply from device node %pOF\n", supply, node);
 
 	snprintf(prop_name, 64, "%s-supply", supply);
-	regnode = of_parse_phandle(dev->of_node, prop_name, 0);
+	regnode = of_parse_phandle(node, prop_name, 0);
 	if (regnode)
 		return regnode;
 
@@ -628,6 +630,7 @@ static struct regulator_dev *of_find_regulator_by_node(struct device_node *np)
 /**
  * of_regulator_dev_lookup - lookup a regulator device with device tree only
  * @dev: Device pointer for regulator supply lookup.
+ * @np: Device node pointer for regulator supply lookup.
  * @supply: Supply name or regulator ID.
  *
  * Return: Pointer to the &struct regulator_dev on success, or ERR_PTR()
@@ -642,13 +645,13 @@ static struct regulator_dev *of_find_regulator_by_node(struct device_node *np)
  * * -%ENODEV if lookup fails permanently.
  * * -%EPROBE_DEFER if lookup could succeed in the future.
  */
-struct regulator_dev *of_regulator_dev_lookup(struct device *dev,
+struct regulator_dev *of_regulator_dev_lookup(struct device *dev, struct device_node *np,
 					      const char *supply)
 {
 	struct regulator_dev *r;
 	struct device_node *node;
 
-	node = of_get_regulator(dev, supply);
+	node = of_get_regulator(dev, np, supply);
 	if (node) {
 		r = of_find_regulator_by_node(node);
 		of_node_put(node);
@@ -665,6 +668,42 @@ struct regulator_dev *of_regulator_dev_lookup(struct device *dev,
 	return ERR_PTR(-ENODEV);
 }
 
+static struct regulator *_of_regulator_get(struct device *dev, struct device_node *node,
+					   const char *id, enum regulator_get_type get_type)
+{
+	struct regulator_dev *r;
+	int ret;
+
+	ret = _regulator_get_common_check(dev, id, get_type);
+	if (ret)
+		return ERR_PTR(ret);
+
+	r = of_regulator_dev_lookup(dev, node, id);
+	return _regulator_get_common(r, dev, id, get_type);
+}
+
+/**
+ * of_regulator_get_optional - get optional regulator via device tree lookup
+ * @dev: device used for dev_printk() messages
+ * @node: device node for regulator "consumer"
+ * @id: Supply name
+ *
+ * Return: pointer to struct regulator corresponding to the regulator producer,
+ *	   or PTR_ERR() encoded error number.
+ *
+ * This is intended for use by consumers that want to get a regulator
+ * supply directly from a device node, and can and want to deal with
+ * absence of such supplies. This will _not_ consider supply aliases.
+ * See regulator_dev_lookup().
+ */
+struct regulator *of_regulator_get_optional(struct device *dev,
+					    struct device_node *node,
+					    const char *id)
+{
+	return _of_regulator_get(dev, node, id, OPTIONAL_GET);
+}
+EXPORT_SYMBOL_GPL(of_regulator_get_optional);
+
 /*
  * Returns number of regulators coupled with rdev.
  */
diff --git a/include/linux/regulator/consumer.h b/include/linux/regulator/consumer.h
index b9ce521910a0..2b22f07e491c 100644
--- a/include/linux/regulator/consumer.h
+++ b/include/linux/regulator/consumer.h
@@ -168,6 +168,19 @@ int devm_regulator_get_enable_read_voltage(struct device *dev, const char *id);
 void regulator_put(struct regulator *regulator);
 void devm_regulator_put(struct regulator *regulator);
 
+#if IS_ENABLED(CONFIG_OF)
+struct regulator *__must_check of_regulator_get_optional(struct device *dev,
+							 struct device_node *node,
+							 const char *id);
+#else
+static inline struct regulator *__must_check of_regulator_get_optional(struct device *dev,
+								       struct device_node *node,
+								       const char *id)
+{
+	return ERR_PTR(-ENODEV);
+}
+#endif
+
 int regulator_register_supply_alias(struct device *dev, const char *id,
 				    struct device *alias_dev,
 				    const char *alias_id);
@@ -350,6 +363,13 @@ devm_regulator_get_optional(struct device *dev, const char *id)
 	return ERR_PTR(-ENODEV);
 }
 
+static inline struct regulator *__must_check of_regulator_get_optional(struct device *dev,
+								       struct device_node *node,
+								       const char *id)
+{
+	return ERR_PTR(-ENODEV);
+}
+
 static inline void regulator_put(struct regulator *regulator)
 {
 }
-- 
2.46.0.792.g87dc391469-goog

Re: [PATCH v8 1/3] regulator: Add of_regulator_get_optional() for pure DT regulator lookup

[Andy Shevchenko]

On Wed, Sep 25, 2024 at 05:38:04PM +0800, Chen-Yu Tsai wrote:
> The to-be-introduced I2C component prober needs to enable regulator
> supplies (and toggle GPIO pins) for the various components it intends
> to probe. To support this, a new "pure DT lookup" method for getting
> regulator supplies is needed, since the device normally requesting
> the supply won't get created until after the component is probed to
> be available.
> 
> Add a new of_regulator_get_optional() function for this. This mirrors
> the existing regulator_get_optional() function, but is OF-specific.
> The underlying code that supports the existing regulator_get*()
> functions has been reworked in previous patches to support this
> specific case.
> 
> Also convert an existing usage of "dev && dev->of_node" to
> "dev_of_node(dev)".

I thought I gave a tag already...
Whatever, FWIW,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

-- 
With Best Regards,
Andy Shevchenko

[PATCH v8 2/3] regulator: Add devres version of of_regulator_get_optional()

[Chen-Yu Tsai]

There are existing uses for a devres version of of_regulator_get_optional()
in power domain drivers. On MediaTek platforms, power domains may have
regulator supplies tied to them. The driver currently tries to use
devm_regulator_get() to not have to manage the lifecycle, but ends up
doing it in a very hacky way by replacing the device node of the power
domain controller device to the device node of the power domain that is
currently being registered, getting the supply, and reverting the device
node.

Provide a better API so that the hack can be replaced.

Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
Changes since v7:
- New patch
---
 drivers/regulator/devres.c         | 39 ++++++++++++++++++++++++++++++
 drivers/regulator/internal.h       | 16 +++++++-----
 drivers/regulator/of_regulator.c   |  4 +--
 include/linux/regulator/consumer.h | 17 +++++++++++++
 4 files changed, 68 insertions(+), 8 deletions(-)

diff --git a/drivers/regulator/devres.c b/drivers/regulator/devres.c
index 1b893cdd1aad..36164aec30e8 100644
--- a/drivers/regulator/devres.c
+++ b/drivers/regulator/devres.c
@@ -749,3 +749,42 @@ void *devm_regulator_irq_helper(struct device *dev,
 	return ptr;
 }
 EXPORT_SYMBOL_GPL(devm_regulator_irq_helper);
+
+#if IS_ENABLED(CONFIG_OF)
+static struct regulator *_devm_of_regulator_get(struct device *dev, struct device_node *node,
+						const char *id, int get_type)
+{
+	struct regulator **ptr, *regulator;
+
+	ptr = devres_alloc(devm_regulator_release, sizeof(*ptr), GFP_KERNEL);
+	if (!ptr)
+		return ERR_PTR(-ENOMEM);
+
+	regulator = _of_regulator_get(dev, node, id, get_type);
+	if (!IS_ERR(regulator)) {
+		*ptr = regulator;
+		devres_add(dev, ptr);
+	} else {
+		devres_free(ptr);
+	}
+
+	return regulator;
+}
+
+/**
+ * devm_of_regulator_get_optional - Resource managed of_regulator_get_optional()
+ * @dev: device used for dev_printk() messages and resource lifetime management
+ * @node: device node for regulator "consumer"
+ * @id:  supply name or regulator ID.
+ *
+ * Managed regulator_get_optional(). Regulators returned from this
+ * function are automatically regulator_put() on driver detach. See
+ * of_regulator_get_optional() for more information.
+ */
+struct regulator *devm_of_regulator_get_optional(struct device *dev, struct device_node *node,
+						 const char *id)
+{
+	return _devm_of_regulator_get(dev, node, id, OPTIONAL_GET);
+}
+EXPORT_SYMBOL_GPL(devm_of_regulator_get_optional);
+#endif
diff --git a/drivers/regulator/internal.h b/drivers/regulator/internal.h
index f62cacbbc729..b3d48dc38bc4 100644
--- a/drivers/regulator/internal.h
+++ b/drivers/regulator/internal.h
@@ -65,6 +65,13 @@ static inline struct regulator_dev *dev_to_rdev(struct device *dev)
 	return container_of(dev, struct regulator_dev, dev);
 }
 
+enum regulator_get_type {
+	NORMAL_GET,
+	EXCLUSIVE_GET,
+	OPTIONAL_GET,
+	MAX_GET_TYPE
+};
+
 #ifdef CONFIG_OF
 struct regulator_dev *of_regulator_dev_lookup(struct device *dev,
 					      struct device_node *np,
@@ -74,6 +81,9 @@ struct regulator_init_data *regulator_of_get_init_data(struct device *dev,
 				 struct regulator_config *config,
 				 struct device_node **node);
 
+struct regulator *_of_regulator_get(struct device *dev, struct device_node *node,
+				    const char *id, enum regulator_get_type get_type);
+
 struct regulator_dev *of_parse_coupled_regulator(struct regulator_dev *rdev,
 						 int index);
 
@@ -116,12 +126,6 @@ static inline bool of_check_coupling_data(struct regulator_dev *rdev)
 }
 
 #endif
-enum regulator_get_type {
-	NORMAL_GET,
-	EXCLUSIVE_GET,
-	OPTIONAL_GET,
-	MAX_GET_TYPE
-};
 
 int _regulator_get_common_check(struct device *dev, const char *id,
 				enum regulator_get_type get_type);
diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c
index 358c3ed791db..3d85762beda6 100644
--- a/drivers/regulator/of_regulator.c
+++ b/drivers/regulator/of_regulator.c
@@ -668,8 +668,8 @@ struct regulator_dev *of_regulator_dev_lookup(struct device *dev, struct device_
 	return ERR_PTR(-ENODEV);
 }
 
-static struct regulator *_of_regulator_get(struct device *dev, struct device_node *node,
-					   const char *id, enum regulator_get_type get_type)
+struct regulator *_of_regulator_get(struct device *dev, struct device_node *node,
+				    const char *id, enum regulator_get_type get_type)
 {
 	struct regulator_dev *r;
 	int ret;
diff --git a/include/linux/regulator/consumer.h b/include/linux/regulator/consumer.h
index 2b22f07e491c..8c3c372ad735 100644
--- a/include/linux/regulator/consumer.h
+++ b/include/linux/regulator/consumer.h
@@ -172,6 +172,9 @@ void devm_regulator_put(struct regulator *regulator);
 struct regulator *__must_check of_regulator_get_optional(struct device *dev,
 							 struct device_node *node,
 							 const char *id);
+struct regulator *__must_check devm_of_regulator_get_optional(struct device *dev,
+							      struct device_node *node,
+							      const char *id);
 #else
 static inline struct regulator *__must_check of_regulator_get_optional(struct device *dev,
 								       struct device_node *node,
@@ -179,6 +182,13 @@ static inline struct regulator *__must_check of_regulator_get_optional(struct de
 {
 	return ERR_PTR(-ENODEV);
 }
+
+static inline struct regulator *__must_check devm_of_regulator_get_optional(struct device *dev,
+									    struct device_node *node,
+									    const char *id)
+{
+	return ERR_PTR(-ENODEV);
+}
 #endif
 
 int regulator_register_supply_alias(struct device *dev, const char *id,
@@ -370,6 +380,13 @@ static inline struct regulator *__must_check of_regulator_get_optional(struct de
 	return ERR_PTR(-ENODEV);
 }
 
+static inline struct regulator *__must_check devm_of_regulator_get_optional(struct device *dev,
+									    struct device_node *node,
+									    const char *id)
+{
+	return ERR_PTR(-ENODEV);
+}
+
 static inline void regulator_put(struct regulator *regulator)
 {
 }
-- 
2.46.0.792.g87dc391469-goog

Re: [PATCH v8 2/3] regulator: Add devres version of of_regulator_get_optional()

[Andy Shevchenko]

On Wed, Sep 25, 2024 at 05:38:05PM +0800, Chen-Yu Tsai wrote:
> There are existing uses for a devres version of of_regulator_get_optional()
> in power domain drivers. On MediaTek platforms, power domains may have
> regulator supplies tied to them. The driver currently tries to use
> devm_regulator_get() to not have to manage the lifecycle, but ends up
> doing it in a very hacky way by replacing the device node of the power
> domain controller device to the device node of the power domain that is
> currently being registered, getting the supply, and reverting the device
> node.
> 
> Provide a better API so that the hack can be replaced.

...

> +#if IS_ENABLED(CONFIG_OF)

Do we really need this?

> +static struct regulator *_devm_of_regulator_get(struct device *dev, struct device_node *node,
> +						const char *id, int get_type)
> +{
> +	struct regulator **ptr, *regulator;
> +
> +	ptr = devres_alloc(devm_regulator_release, sizeof(*ptr), GFP_KERNEL);
> +	if (!ptr)
> +		return ERR_PTR(-ENOMEM);
> +
> +	regulator = _of_regulator_get(dev, node, id, get_type);
> +	if (!IS_ERR(regulator)) {
> +		*ptr = regulator;
> +		devres_add(dev, ptr);
> +	} else {
> +		devres_free(ptr);
> +	}
> +
> +	return regulator;

Why not using devm_add_action() / devm_add_action_or_reset()
(whichever suits better here)?

> +}

> +#endif

...

> +static inline struct regulator *__must_check devm_of_regulator_get_optional(struct device *dev,
> +									    struct device_node *node,
> +									    const char *id)

I don't know the conventions here, but I find better to have it as

static inline __must_check struct regulator *
devm_of_regulator_get_optional(struct device *dev, struct device_node *node, const char *id)

Similar to other stubs and declarations.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v8 2/3] regulator: Add devres version of of_regulator_get_optional()

[Chen-Yu Tsai]

On Wed, Sep 25, 2024 at 6:56 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Wed, Sep 25, 2024 at 05:38:05PM +0800, Chen-Yu Tsai wrote:
> > There are existing uses for a devres version of of_regulator_get_optional()
> > in power domain drivers. On MediaTek platforms, power domains may have
> > regulator supplies tied to them. The driver currently tries to use
> > devm_regulator_get() to not have to manage the lifecycle, but ends up
> > doing it in a very hacky way by replacing the device node of the power
> > domain controller device to the device node of the power domain that is
> > currently being registered, getting the supply, and reverting the device
> > node.
> >
> > Provide a better API so that the hack can be replaced.
>
> ...
>
> > +#if IS_ENABLED(CONFIG_OF)
>
> Do we really need this?

What's the point of going through devres_* stuff if we already know
_of_regulator_get() is going to fail anyway?

Also, _of_regulator_get() does not have a stub version for !CONFIG_OF.

> > +static struct regulator *_devm_of_regulator_get(struct device *dev, struct device_node *node,
> > +                                             const char *id, int get_type)
> > +{
> > +     struct regulator **ptr, *regulator;
> > +
> > +     ptr = devres_alloc(devm_regulator_release, sizeof(*ptr), GFP_KERNEL);
> > +     if (!ptr)
> > +             return ERR_PTR(-ENOMEM);
> > +
> > +     regulator = _of_regulator_get(dev, node, id, get_type);
> > +     if (!IS_ERR(regulator)) {
> > +             *ptr = regulator;
> > +             devres_add(dev, ptr);
> > +     } else {
> > +             devres_free(ptr);
> > +     }
> > +
> > +     return regulator;
>
> Why not using devm_add_action() / devm_add_action_or_reset()
> (whichever suits better here)?

Cargo cult from _devm_regulator_get() in this file. However since this is
meant to share the same release function, both functions need to use the
same mechanism.

I could also argue that this is not an action, but an allocation, and so
devres_alloc() seems to make more sense.


ChenYu

> > +}
>
> > +#endif
>
> ...
>
> > +static inline struct regulator *__must_check devm_of_regulator_get_optional(struct device *dev,
> > +                                                                         struct device_node *node,
> > +                                                                         const char *id)
>
> I don't know the conventions here, but I find better to have it as
>
> static inline __must_check struct regulator *
> devm_of_regulator_get_optional(struct device *dev, struct device_node *node, const char *id)
>
> Similar to other stubs and declarations.

I don't think there are any conventions. This file already has three types:

1. Wrap the line with the function name on the second line
2. Wrap the arguments; wrapped arguments aligned to the left parenthesis.
3. Wrap the arguments; wrapped arguments aligned with aribtrary number of
   tabs.

I prefer the way I have put them.

Re: [PATCH v8 2/3] regulator: Add devres version of of_regulator_get_optional()

[Andy Shevchenko]

On Thu, Sep 26, 2024 at 04:43:52PM +0800, Chen-Yu Tsai wrote:
> On Wed, Sep 25, 2024 at 6:56 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Wed, Sep 25, 2024 at 05:38:05PM +0800, Chen-Yu Tsai wrote:

...

> > > +#if IS_ENABLED(CONFIG_OF)
> >
> > Do we really need this?
> 
> What's the point of going through devres_* stuff if we already know
> _of_regulator_get() is going to fail anyway?

With devm_add_action*() this will be other way around and there are plenty of
APIs done this way. The ifdeffery is simply ugly in the code.

> Also, _of_regulator_get() does not have a stub version for !CONFIG_OF.

So, what prevents us from adding it?

> > > +static struct regulator *_devm_of_regulator_get(struct device *dev, struct device_node *node,
> > > +                                             const char *id, int get_type)
> > > +{
> > > +     struct regulator **ptr, *regulator;
> > > +
> > > +     ptr = devres_alloc(devm_regulator_release, sizeof(*ptr), GFP_KERNEL);
> > > +     if (!ptr)
> > > +             return ERR_PTR(-ENOMEM);
> > > +
> > > +     regulator = _of_regulator_get(dev, node, id, get_type);
> > > +     if (!IS_ERR(regulator)) {
> > > +             *ptr = regulator;
> > > +             devres_add(dev, ptr);
> > > +     } else {
> > > +             devres_free(ptr);
> > > +     }
> > > +
> > > +     return regulator;
> >
> > Why not using devm_add_action() / devm_add_action_or_reset()
> > (whichever suits better here)?
> 
> Cargo cult from _devm_regulator_get() in this file. However since this is
> meant to share the same release function, both functions need to use the
> same mechanism.
> 
> I could also argue that this is not an action, but an allocation, and so
> devres_alloc() seems to make more sense.

It's rather matter of the naming of the devm_add_action*() APIs, but again,
we have plenty of APIs using it when it's allocation and not strictly speaking
an action.

> > > +}
> >
> > > +#endif

...

> > > +static inline struct regulator *__must_check devm_of_regulator_get_optional(struct device *dev,
> > > +                                                                         struct device_node *node,
> > > +                                                                         const char *id)
> >
> > I don't know the conventions here, but I find better to have it as
> >
> > static inline __must_check struct regulator *
> > devm_of_regulator_get_optional(struct device *dev, struct device_node *node, const char *id)
> >
> > Similar to other stubs and declarations.
> 
> I don't think there are any conventions. This file already has three types:
> 
> 1. Wrap the line with the function name on the second line
> 2. Wrap the arguments; wrapped arguments aligned to the left parenthesis.
> 3. Wrap the arguments; wrapped arguments aligned with aribtrary number of
>    tabs.
> 
> I prefer the way I have put them.

The way you put it despite relaxed limit is slightly harder to read.
I don't remember many headers that do so-o indented parameters. Besides
your way defers the burden of resplit to the future in case one more parameter
needs to be added which will excess the 100 limit.

Also __must_check is somehow misplaced in my opinion (talking from my
experience and this can be simply checked by grepping other headers).

That said, I prefer the way I suggested or something alike.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v8 2/3] regulator: Add devres version of of_regulator_get_optional()

[Chen-Yu Tsai]

On Thu, Sep 26, 2024 at 8:26 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Thu, Sep 26, 2024 at 04:43:52PM +0800, Chen-Yu Tsai wrote:
> > On Wed, Sep 25, 2024 at 6:56 PM Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > > On Wed, Sep 25, 2024 at 05:38:05PM +0800, Chen-Yu Tsai wrote:
>
> ...
>
> > > > +#if IS_ENABLED(CONFIG_OF)
> > >
> > > Do we really need this?
> >
> > What's the point of going through devres_* stuff if we already know
> > _of_regulator_get() is going to fail anyway?
>
> With devm_add_action*() this will be other way around and there are plenty of
> APIs done this way. The ifdeffery is simply ugly in the code.

It's still extra code that doesn't get compiled out.

> > Also, _of_regulator_get() does not have a stub version for !CONFIG_OF.
>
> So, what prevents us from adding it?

Because there's no need if the only internal user isn't using it.

I could also move them over to of_regulator.c.

_of_regulator_get() stays internal to that file, and devm_regulator_release()
gets exposed instead.

Does that sound better?

> > > > +static struct regulator *_devm_of_regulator_get(struct device *dev, struct device_node *node,
> > > > +                                             const char *id, int get_type)
> > > > +{
> > > > +     struct regulator **ptr, *regulator;
> > > > +
> > > > +     ptr = devres_alloc(devm_regulator_release, sizeof(*ptr), GFP_KERNEL);
> > > > +     if (!ptr)
> > > > +             return ERR_PTR(-ENOMEM);
> > > > +
> > > > +     regulator = _of_regulator_get(dev, node, id, get_type);
> > > > +     if (!IS_ERR(regulator)) {
> > > > +             *ptr = regulator;
> > > > +             devres_add(dev, ptr);
> > > > +     } else {
> > > > +             devres_free(ptr);
> > > > +     }
> > > > +
> > > > +     return regulator;
> > >
> > > Why not using devm_add_action() / devm_add_action_or_reset()
> > > (whichever suits better here)?
> >
> > Cargo cult from _devm_regulator_get() in this file. However since this is
> > meant to share the same release function, both functions need to use the
> > same mechanism.
> >
> > I could also argue that this is not an action, but an allocation, and so
> > devres_alloc() seems to make more sense.
>
> It's rather matter of the naming of the devm_add_action*() APIs, but again,
> we have plenty of APIs using it when it's allocation and not strictly speaking
> an action.

OK. Still the mechanism used needs to match that of the existing API.
So devres_add() it is for now.

> > > > +}
> > >
> > > > +#endif
>
> ...
>
> > > > +static inline struct regulator *__must_check devm_of_regulator_get_optional(struct device *dev,
> > > > +                                                                         struct device_node *node,
> > > > +                                                                         const char *id)
> > >
> > > I don't know the conventions here, but I find better to have it as
> > >
> > > static inline __must_check struct regulator *
> > > devm_of_regulator_get_optional(struct device *dev, struct device_node *node, const char *id)
> > >
> > > Similar to other stubs and declarations.
> >
> > I don't think there are any conventions. This file already has three types:
> >
> > 1. Wrap the line with the function name on the second line
> > 2. Wrap the arguments; wrapped arguments aligned to the left parenthesis.
> > 3. Wrap the arguments; wrapped arguments aligned with aribtrary number of
> >    tabs.
> >
> > I prefer the way I have put them.
>
> The way you put it despite relaxed limit is slightly harder to read.
> I don't remember many headers that do so-o indented parameters. Besides
> your way defers the burden of resplit to the future in case one more parameter
> needs to be added which will excess the 100 limit.
>
> Also __must_check is somehow misplaced in my opinion (talking from my
> experience and this can be simply checked by grepping other headers).

Seems correct to me. It's between the return type and the function name.
From the coding style doc:

 __init void * __must_check action(enum magic value, size_t size, u8 count,
                                   char *fmt, ...) __printf(4, 5) __malloc;

The preferred order of elements for a function prototype is:

- storage class (below, ``static __always_inline``, noting that
``__always_inline``
  is technically an attribute but is treated like ``inline``)
- storage class attributes (here, ``__init`` -- i.e. section
declarations, but also
  things like ``__cold``)
- return type (here, ``void *``)
- return type attributes (here, ``__must_check``)
- function name (here, ``action``)
- function parameters (here, ``(enum magic value, size_t size, u8
count, char *fmt, ...)``,
  noting that parameter names should always be included)
- function parameter attributes (here, ``__printf(4, 5)``)
- function behavior attributes (here, ``__malloc``)


> That said, I prefer the way I suggested or something alike.

Two people arguing over style that is not clearly specified in the coding
style doc is probably wasting time. I'll use what `clang-format` gave:

static inline struct regulator *__must_check of_regulator_get_optional(
       struct device *dev, struct device_node *node, const char *id)
static inline struct regulator *__must_check devm_of_regulator_get_optional(
       struct device *dev, struct device_node *node, const char *id)


ChenYu

Re: [PATCH v8 2/3] regulator: Add devres version of of_regulator_get_optional()

[Andy Shevchenko]

On Fri, Sep 27, 2024 at 12:38:35PM +0800, Chen-Yu Tsai wrote:
> On Thu, Sep 26, 2024 at 8:26 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Thu, Sep 26, 2024 at 04:43:52PM +0800, Chen-Yu Tsai wrote:
> > > On Wed, Sep 25, 2024 at 6:56 PM Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > On Wed, Sep 25, 2024 at 05:38:05PM +0800, Chen-Yu Tsai wrote:

...

> > > > > +#if IS_ENABLED(CONFIG_OF)
> > > >
> > > > Do we really need this?
> > >
> > > What's the point of going through devres_* stuff if we already know
> > > _of_regulator_get() is going to fail anyway?
> >
> > With devm_add_action*() this will be other way around and there are plenty of
> > APIs done this way. The ifdeffery is simply ugly in the code.
> 
> It's still extra code that doesn't get compiled out.

Are you sure? In case of the stub the compiler should go with a "dead code
elimination" optimisation and get rid of most of it (yes, I admit that it might
be the overhead for the exporting a function which returns a constant).

> > > Also, _of_regulator_get() does not have a stub version for !CONFIG_OF.
> >
> > So, what prevents us from adding it?
> 
> Because there's no need if the only internal user isn't using it.
> 
> I could also move them over to of_regulator.c.
> 
> _of_regulator_get() stays internal to that file, and devm_regulator_release()
> gets exposed instead.
> 
> Does that sound better?

This sounds good to me!

...

> > > > > +static inline struct regulator *__must_check devm_of_regulator_get_optional(struct device *dev,
> > > > > +                                                                         struct device_node *node,
> > > > > +                                                                         const char *id)
> > > >
> > > > I don't know the conventions here, but I find better to have it as
> > > >
> > > > static inline __must_check struct regulator *
> > > > devm_of_regulator_get_optional(struct device *dev, struct device_node *node, const char *id)
> > > >
> > > > Similar to other stubs and declarations.
> > >
> > > I don't think there are any conventions. This file already has three types:
> > >
> > > 1. Wrap the line with the function name on the second line
> > > 2. Wrap the arguments; wrapped arguments aligned to the left parenthesis.
> > > 3. Wrap the arguments; wrapped arguments aligned with aribtrary number of
> > >    tabs.
> > >
> > > I prefer the way I have put them.
> >
> > The way you put it despite relaxed limit is slightly harder to read.
> > I don't remember many headers that do so-o indented parameters. Besides
> > your way defers the burden of resplit to the future in case one more parameter
> > needs to be added which will excess the 100 limit.
> >
> > Also __must_check is somehow misplaced in my opinion (talking from my
> > experience and this can be simply checked by grepping other headers).
> 
> Seems correct to me. It's between the return type and the function name.
> From the coding style doc:
> 
>  __init void * __must_check action(enum magic value, size_t size, u8 count,
>                                    char *fmt, ...) __printf(4, 5) __malloc;
> 
> The preferred order of elements for a function prototype is:
> 
> - storage class (below, ``static __always_inline``, noting that
> ``__always_inline``
>   is technically an attribute but is treated like ``inline``)
> - storage class attributes (here, ``__init`` -- i.e. section
> declarations, but also
>   things like ``__cold``)
> - return type (here, ``void *``)
> - return type attributes (here, ``__must_check``)
> - function name (here, ``action``)
> - function parameters (here, ``(enum magic value, size_t size, u8
> count, char *fmt, ...)``,
>   noting that parameter names should always be included)
> - function parameter attributes (here, ``__printf(4, 5)``)
> - function behavior attributes (here, ``__malloc``)
> 
> > That said, I prefer the way I suggested or something alike.
> 
> Two people arguing over style that is not clearly specified in the coding
> style doc is probably wasting time. I'll use what `clang-format` gave:
> 
> static inline struct regulator *__must_check of_regulator_get_optional(
>        struct device *dev, struct device_node *node, const char *id)
> static inline struct regulator *__must_check devm_of_regulator_get_optional(
>        struct device *dev, struct device_node *node, const char *id)

With all my hatred towards this clang-format "feature", i.e. open ended
parenthesis, this looks better than your original variant.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v8 3/3] pmdomain: mediatek: Use OF-specific regulator API to get power domain supply

[Chen-Yu Tsai]

The MediaTek power domain driver contains a hack that assigns the device
node of the power domain to the struct device of the power domain
controller in order to use the devres regulator API.

Now that there is a proper OF-specific regulator API, and even a devres
version, replace the hack with proper code.

This change is incompatible with incomplete device trees. Instead of
assigning the dummy regulator in cases where the power domain requires
a supply but the device tree does not provide one, the driver will just
error out. This will be seen on the MT8390 EVK, which is missing
supplies for the IMG_VCORE and CAM_VCORE domains. And likely all the
MediaTek EVBs, which have no power domain supplies specified. This is
however the correct behavior. If the power domain's supply is missing,
then it should not work. Relying on other parts of the system to keep
the unattached regulator enabled is likely to break in ways less easier
to understand.

Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
Changes since v7:
- New patch

The other option is to follow what Rockchip will be doing: getting the
regulator supply upon first use / enable [1]. This will result in less
breakage: only the power domain that is missing its supplies will fail
to be attached.

[1] https://lore.kernel.org/all/20240919091834.83572-6-sebastian.reichel@collabora.com/
---
 drivers/pmdomain/mediatek/mtk-pm-domains.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/drivers/pmdomain/mediatek/mtk-pm-domains.c b/drivers/pmdomain/mediatek/mtk-pm-domains.c
index 88406e9ac63c..3580913f25d3 100644
--- a/drivers/pmdomain/mediatek/mtk-pm-domains.c
+++ b/drivers/pmdomain/mediatek/mtk-pm-domains.c
@@ -353,7 +353,6 @@ generic_pm_domain *scpsys_add_one_domain(struct scpsys *scpsys, struct device_no
 {
 	const struct scpsys_domain_data *domain_data;
 	struct scpsys_domain *pd;
-	struct device_node *root_node = scpsys->dev->of_node;
 	struct device_node *smi_node;
 	struct property *prop;
 	const char *clk_name;
@@ -388,16 +387,7 @@ generic_pm_domain *scpsys_add_one_domain(struct scpsys *scpsys, struct device_no
 	pd->scpsys = scpsys;
 
 	if (MTK_SCPD_CAPS(pd, MTK_SCPD_DOMAIN_SUPPLY)) {
-		/*
-		 * Find regulator in current power domain node.
-		 * devm_regulator_get() finds regulator in a node and its child
-		 * node, so set of_node to current power domain node then change
-		 * back to original node after regulator is found for current
-		 * power domain node.
-		 */
-		scpsys->dev->of_node = node;
-		pd->supply = devm_regulator_get(scpsys->dev, "domain");
-		scpsys->dev->of_node = root_node;
+		pd->supply = devm_of_regulator_get_optional(scpsys->dev, node, "domain");
 		if (IS_ERR(pd->supply))
 			return dev_err_cast_probe(scpsys->dev, pd->supply,
 				      "%pOF: failed to get power supply.\n",
-- 
2.46.0.792.g87dc391469-goog

Re: [PATCH v8 3/3] pmdomain: mediatek: Use OF-specific regulator API to get power domain supply

[AngeloGioacchino Del Regno]

Il 25/09/24 11:38, Chen-Yu Tsai ha scritto:
> The MediaTek power domain driver contains a hack that assigns the device
> node of the power domain to the struct device of the power domain
> controller in order to use the devres regulator API.
> 
> Now that there is a proper OF-specific regulator API, and even a devres
> version, replace the hack with proper code.
> 
> This change is incompatible with incomplete device trees. Instead of
> assigning the dummy regulator in cases where the power domain requires
> a supply but the device tree does not provide one, the driver will just
> error out. This will be seen on the MT8390 EVK, which is missing
> supplies for the IMG_VCORE and CAM_VCORE domains. And likely all the
> MediaTek EVBs, which have no power domain supplies specified. This is
> however the correct behavior. If the power domain's supply is missing,
> then it should not work. Relying on other parts of the system to keep
> the unattached regulator enabled is likely to break in ways less easier
> to understand.
> 

Breaking something that was "working" (not really working though) before is a
hard thing to justify, but I feel like this is one of the cases in which we
have to swallow the pill.

This is a hack that I've always been angry about, and causing all sorts of
random issues on MediaTek SoCs from time to time... that was fixed multiple
times by hacking the previous hack which was needed for this hack to still
work in a way or another.

I am tempted to give you a R-b right now, but let me carefully test this on
multiple devices before that.

Meanwhile, I just wanted to say that I agree with you about this commit and
wanted to give a bit more context to people reading "this breaks things" so
that they can understand what's going on (and that we're not crazy! ..or at
least, not *that* much :-P).

Cheers,
Angelo

> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> ---
> Changes since v7:
> - New patch
> 
> The other option is to follow what Rockchip will be doing: getting the
> regulator supply upon first use / enable [1]. This will result in less
> breakage: only the power domain that is missing its supplies will fail
> to be attached.
> 
> [1] https://lore.kernel.org/all/20240919091834.83572-6-sebastian.reichel@collabora.com/
> ---
>   drivers/pmdomain/mediatek/mtk-pm-domains.c | 12 +-----------
>   1 file changed, 1 insertion(+), 11 deletions(-)
> 
> diff --git a/drivers/pmdomain/mediatek/mtk-pm-domains.c b/drivers/pmdomain/mediatek/mtk-pm-domains.c
> index 88406e9ac63c..3580913f25d3 100644
> --- a/drivers/pmdomain/mediatek/mtk-pm-domains.c
> +++ b/drivers/pmdomain/mediatek/mtk-pm-domains.c
> @@ -353,7 +353,6 @@ generic_pm_domain *scpsys_add_one_domain(struct scpsys *scpsys, struct device_no
>   {
>   	const struct scpsys_domain_data *domain_data;
>   	struct scpsys_domain *pd;
> -	struct device_node *root_node = scpsys->dev->of_node;
>   	struct device_node *smi_node;
>   	struct property *prop;
>   	const char *clk_name;
> @@ -388,16 +387,7 @@ generic_pm_domain *scpsys_add_one_domain(struct scpsys *scpsys, struct device_no
>   	pd->scpsys = scpsys;
>   
>   	if (MTK_SCPD_CAPS(pd, MTK_SCPD_DOMAIN_SUPPLY)) {
> -		/*
> -		 * Find regulator in current power domain node.
> -		 * devm_regulator_get() finds regulator in a node and its child
> -		 * node, so set of_node to current power domain node then change
> -		 * back to original node after regulator is found for current
> -		 * power domain node.
> -		 */
> -		scpsys->dev->of_node = node;
> -		pd->supply = devm_regulator_get(scpsys->dev, "domain");
> -		scpsys->dev->of_node = root_node;
> +		pd->supply = devm_of_regulator_get_optional(scpsys->dev, node, "domain");
>   		if (IS_ERR(pd->supply))
>   			return dev_err_cast_probe(scpsys->dev, pd->supply,
>   				      "%pOF: failed to get power supply.\n",

Re: [PATCH v8 0/3] Add of_regulator_get_optional() and Fix MTK Power Domain Driver

[Ulf Hansson]

On Wed, 25 Sept 2024 at 11:38, Chen-Yu Tsai <wenst@chromium.org> wrote:
>
> Hi folks,
>
> This series is split off from my "DT hardware prober" series [1].
>
> Changes since v7:
> - Added stub versions for of_regulator_get_optional() for !CONFIG_OF
>   and !CONFIG_REGULATOR
> - Added new patches for devres version and converting MTK pmdomain
>   driver
>
> At ELCE, Sebastian told me about his recent work on adding regulator
> supply support to the Rockchip power domain driver [2], how the MediaTek
> driver has been using the existing devm_regulator_get() API and
> reassigning different device nodes to the device doing the lookup, and
> how the new of_regulator_get_optional() is the proper fit for this.
>
> Patch 1 adds a new of_regulator_get_optional() function to look up
> regulator supplies using device tree nodes.
>
> Patch 2 adds a devres version of the aforementioned function at
> Sebastian's request for the two power domain drivers.
>
> Patch 3 converts the MediaTek power domain driver to use function.
>
>
> Each of the latter two patches depend on the previous one at build time.
> Mark, would it be possible for you to put the two regulator patches
> on an immutable branch / tag? Otherwise we could have Ulf ack the
> pmdomain patch and merge it through your tree. Sebastian was fine
> with converting the rockchip pmdomain some time later.

Thanks for providing some context!

In my opinion I would prefer an immutable branch/tag of the regulator
core changes, so I can carry the pmdomain changes for MTK through my
pmdomain tree, but also because I would prefer if Sebastian could make
the corresponding conversion for the Rockchip pmdomain driver. If this
can get queued soon, there is really no need to have an intermediate
step for Rockchip, I think.

Does it make sense - or do you prefer another way forward?

[...]

Kind regards
Uffe

Re: [PATCH v8 0/3] Add of_regulator_get_optional() and Fix MTK Power Domain Driver

[Chen-Yu Tsai]

On Thu, Sep 26, 2024 at 11:49 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> On Wed, 25 Sept 2024 at 11:38, Chen-Yu Tsai <wenst@chromium.org> wrote:
> >
> > Hi folks,
> >
> > This series is split off from my "DT hardware prober" series [1].
> >
> > Changes since v7:
> > - Added stub versions for of_regulator_get_optional() for !CONFIG_OF
> >   and !CONFIG_REGULATOR
> > - Added new patches for devres version and converting MTK pmdomain
> >   driver
> >
> > At ELCE, Sebastian told me about his recent work on adding regulator
> > supply support to the Rockchip power domain driver [2], how the MediaTek
> > driver has been using the existing devm_regulator_get() API and
> > reassigning different device nodes to the device doing the lookup, and
> > how the new of_regulator_get_optional() is the proper fit for this.
> >
> > Patch 1 adds a new of_regulator_get_optional() function to look up
> > regulator supplies using device tree nodes.
> >
> > Patch 2 adds a devres version of the aforementioned function at
> > Sebastian's request for the two power domain drivers.
> >
> > Patch 3 converts the MediaTek power domain driver to use function.
> >
> >
> > Each of the latter two patches depend on the previous one at build time.
> > Mark, would it be possible for you to put the two regulator patches
> > on an immutable branch / tag? Otherwise we could have Ulf ack the
> > pmdomain patch and merge it through your tree. Sebastian was fine
> > with converting the rockchip pmdomain some time later.
>
> Thanks for providing some context!
>
> In my opinion I would prefer an immutable branch/tag of the regulator
> core changes, so I can carry the pmdomain changes for MTK through my
> pmdomain tree, but also because I would prefer if Sebastian could make
> the corresponding conversion for the Rockchip pmdomain driver. If this
> can get queued soon, there is really no need to have an intermediate
> step for Rockchip, I think.
>
> Does it make sense - or do you prefer another way forward?

Makes sense to me!

ChenYu

Re: (subset) [PATCH v8 0/3] Add of_regulator_get_optional() and Fix MTK Power Domain Driver

[Mark Brown]

On Wed, 25 Sep 2024 17:38:03 +0800, Chen-Yu Tsai wrote:
> This series is split off from my "DT hardware prober" series [1].
> 
> Changes since v7:
> - Added stub versions for of_regulator_get_optional() for !CONFIG_OF
>   and !CONFIG_REGULATOR
> - Added new patches for devres version and converting MTK pmdomain
>   driver
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next

Thanks!

[1/3] regulator: Add of_regulator_get_optional() for pure DT regulator lookup
      commit: 5441b6975adc26aeaca316b9075e04a98238b1b3
[2/3] regulator: Add devres version of of_regulator_get_optional()
      commit: 36ec3f437227470568e5f460997f367f5446a34d

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark