[PATCHv7 10/11] regulator: expose regulator_find_closest_bigger

Sebastian Reichel posted 11 patches 2 years, 6 months ago
[PATCHv7 10/11] regulator: expose regulator_find_closest_bigger
Posted by Sebastian Reichel 2 years, 6 months ago
Expose and document the table lookup logic used by
regulator_set_ramp_delay_regmap, so that it can be
reused for devices that cannot be configured via
regulator_set_ramp_delay_regmap.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/regulator/helpers.c      | 22 ++++++++++++++++++----
 include/linux/regulator/driver.h |  2 ++
 2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/drivers/regulator/helpers.c b/drivers/regulator/helpers.c
index ad2237a95572..586f42e378ee 100644
--- a/drivers/regulator/helpers.c
+++ b/drivers/regulator/helpers.c
@@ -902,8 +902,21 @@ bool regulator_is_equal(struct regulator *reg1, struct regulator *reg2)
 }
 EXPORT_SYMBOL_GPL(regulator_is_equal);
 
-static int find_closest_bigger(unsigned int target, const unsigned int *table,
-			       unsigned int num_sel, unsigned int *sel)
+/**
+ * regulator_find_closest_bigger - helper to find offset in ramp delay table
+ *
+ * @target: targeted ramp_delay
+ * @table: table with supported ramp delays
+ * @num_sel: number of entries in the table
+ * @sel: Pointer to store table offset
+ *
+ * This is the internal helper used by regulator_set_ramp_delay_regmap to
+ * map ramp delay to register value. It should only be used directly if
+ * regulator_set_ramp_delay_regmap cannot handle a specific device setup
+ * (e.g. because the value is split over multiple registers).
+ */
+int regulator_find_closest_bigger(unsigned int target, const unsigned int *table,
+				  unsigned int num_sel, unsigned int *sel)
 {
 	unsigned int s, tmp, max, maxsel = 0;
 	bool found = false;
@@ -933,6 +946,7 @@ static int find_closest_bigger(unsigned int target, const unsigned int *table,
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(regulator_find_closest_bigger);
 
 /**
  * regulator_set_ramp_delay_regmap - set_ramp_delay() helper
@@ -951,8 +965,8 @@ int regulator_set_ramp_delay_regmap(struct regulator_dev *rdev, int ramp_delay)
 	if (WARN_ON(!rdev->desc->n_ramp_values || !rdev->desc->ramp_delay_table))
 		return -EINVAL;
 
-	ret = find_closest_bigger(ramp_delay, rdev->desc->ramp_delay_table,
-				  rdev->desc->n_ramp_values, &sel);
+	ret = regulator_find_closest_bigger(ramp_delay, rdev->desc->ramp_delay_table,
+					    rdev->desc->n_ramp_values, &sel);
 
 	if (ret) {
 		dev_warn(rdev_get_dev(rdev),
diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
index d3b4a3d4514a..c6ef7d68eb9a 100644
--- a/include/linux/regulator/driver.h
+++ b/include/linux/regulator/driver.h
@@ -758,6 +758,8 @@ int regulator_set_current_limit_regmap(struct regulator_dev *rdev,
 				       int min_uA, int max_uA);
 int regulator_get_current_limit_regmap(struct regulator_dev *rdev);
 void *regulator_get_init_drvdata(struct regulator_init_data *reg_init_data);
+int regulator_find_closest_bigger(unsigned int target, const unsigned int *table,
+				  unsigned int num_sel, unsigned int *sel);
 int regulator_set_ramp_delay_regmap(struct regulator_dev *rdev, int ramp_delay);
 int regulator_sync_voltage_rdev(struct regulator_dev *rdev);
 
-- 
2.39.2
Re: [PATCHv7 10/11] regulator: expose regulator_find_closest_bigger
Posted by Sebastian Reichel 2 years, 6 months ago
Hi Mark and Liam,

On Tue, Mar 07, 2023 at 04:36:16PM +0100, Sebastian Reichel wrote:
> Expose and document the table lookup logic used by
> regulator_set_ramp_delay_regmap, so that it can be
> reused for devices that cannot be configured via
> regulator_set_ramp_delay_regmap.
> 
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---

Can you please Ack and the following patch?
(they needs to go through MFD tree)

Thanks,

-- Sebastian

>  drivers/regulator/helpers.c      | 22 ++++++++++++++++++----
>  include/linux/regulator/driver.h |  2 ++
>  2 files changed, 20 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/regulator/helpers.c b/drivers/regulator/helpers.c
> index ad2237a95572..586f42e378ee 100644
> --- a/drivers/regulator/helpers.c
> +++ b/drivers/regulator/helpers.c
> @@ -902,8 +902,21 @@ bool regulator_is_equal(struct regulator *reg1, struct regulator *reg2)
>  }
>  EXPORT_SYMBOL_GPL(regulator_is_equal);
>  
> -static int find_closest_bigger(unsigned int target, const unsigned int *table,
> -			       unsigned int num_sel, unsigned int *sel)
> +/**
> + * regulator_find_closest_bigger - helper to find offset in ramp delay table
> + *
> + * @target: targeted ramp_delay
> + * @table: table with supported ramp delays
> + * @num_sel: number of entries in the table
> + * @sel: Pointer to store table offset
> + *
> + * This is the internal helper used by regulator_set_ramp_delay_regmap to
> + * map ramp delay to register value. It should only be used directly if
> + * regulator_set_ramp_delay_regmap cannot handle a specific device setup
> + * (e.g. because the value is split over multiple registers).
> + */
> +int regulator_find_closest_bigger(unsigned int target, const unsigned int *table,
> +				  unsigned int num_sel, unsigned int *sel)
>  {
>  	unsigned int s, tmp, max, maxsel = 0;
>  	bool found = false;
> @@ -933,6 +946,7 @@ static int find_closest_bigger(unsigned int target, const unsigned int *table,
>  
>  	return 0;
>  }
> +EXPORT_SYMBOL_GPL(regulator_find_closest_bigger);
>  
>  /**
>   * regulator_set_ramp_delay_regmap - set_ramp_delay() helper
> @@ -951,8 +965,8 @@ int regulator_set_ramp_delay_regmap(struct regulator_dev *rdev, int ramp_delay)
>  	if (WARN_ON(!rdev->desc->n_ramp_values || !rdev->desc->ramp_delay_table))
>  		return -EINVAL;
>  
> -	ret = find_closest_bigger(ramp_delay, rdev->desc->ramp_delay_table,
> -				  rdev->desc->n_ramp_values, &sel);
> +	ret = regulator_find_closest_bigger(ramp_delay, rdev->desc->ramp_delay_table,
> +					    rdev->desc->n_ramp_values, &sel);
>  
>  	if (ret) {
>  		dev_warn(rdev_get_dev(rdev),
> diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
> index d3b4a3d4514a..c6ef7d68eb9a 100644
> --- a/include/linux/regulator/driver.h
> +++ b/include/linux/regulator/driver.h
> @@ -758,6 +758,8 @@ int regulator_set_current_limit_regmap(struct regulator_dev *rdev,
>  				       int min_uA, int max_uA);
>  int regulator_get_current_limit_regmap(struct regulator_dev *rdev);
>  void *regulator_get_init_drvdata(struct regulator_init_data *reg_init_data);
> +int regulator_find_closest_bigger(unsigned int target, const unsigned int *table,
> +				  unsigned int num_sel, unsigned int *sel);
>  int regulator_set_ramp_delay_regmap(struct regulator_dev *rdev, int ramp_delay);
>  int regulator_sync_voltage_rdev(struct regulator_dev *rdev);
>  
> -- 
> 2.39.2
> 
> 
> -- 
> To unsubscribe, send mail to kernel-unsubscribe@lists.collabora.co.uk.
Re: [PATCHv7 10/11] regulator: expose regulator_find_closest_bigger
Posted by Mark Brown 2 years, 6 months ago
On Thu, Mar 16, 2023 at 02:34:53AM +0100, Sebastian Reichel wrote:
> On Tue, Mar 07, 2023 at 04:36:16PM +0100, Sebastian Reichel wrote:

> > Expose and document the table lookup logic used by
> > regulator_set_ramp_delay_regmap, so that it can be
> > reused for devices that cannot be configured via
> > regulator_set_ramp_delay_regmap.
> > 
> > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>

> Can you please Ack and the following patch?

Please don't send content free pings and please allow a reasonable time
for review.  People get busy, go on holiday, attend conferences and so 
on so unless there is some reason for urgency (like critical bug fixes)
please allow at least a couple of weeks for review.  If there have been
review comments then people may be waiting for those to be addressed.

Sending content free pings adds to the mail volume (if they are seen at
all) which is often the problem and since they can't be reviewed
directly if something has gone wrong you'll have to resend the patches
anyway, so sending again is generally a better approach though there are
some other maintainers who like them - if in doubt look at how patches
for the subsystem are normally handled.

> (they needs to go through MFD tree)

That seems entirely non-obvious from the subject.