| Message ID | 20250305105314.2009637-1-alexander.stein@ew.tq-group.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v2,1/1] hwmon: (gpio-fan) Add regulator support | expand |
On 3/5/25 02:53, Alexander Stein wrote: > FANs might be supplied by a regulator which needs to be enabled as well. > This is implemented using runtime PM. Every time speed_index changes from > 0 to non-zero and vise versa RPM is resumed or suspended. > Intitial RPM state is determined by initial value of speed_index. > > Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com> > --- > Patch 1 & 2 from v1 [1] have already been applied, although number 2 [2] is not > yet showing in next-20250305. Patches 3 & 4 (just removing comments) from v1 > have been dropped, so only this patch remains. > > Changes in v2: > * Make regulator non-optional > > [1] https://lore.kernel.org/all/20250210145934.761280-1-alexander.stein@ew.tq-group.com/ > [2] https://web.git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git/commit/?h=hwmon-next&id=9fee7d19bab635f89223cc40dfd2c8797fdc4988 > --- > drivers/hwmon/gpio-fan.c | 81 ++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 77 insertions(+), 4 deletions(-) > > diff --git a/drivers/hwmon/gpio-fan.c b/drivers/hwmon/gpio-fan.c > index cee3fa146d69a..db918d6858325 100644 > --- a/drivers/hwmon/gpio-fan.c > +++ b/drivers/hwmon/gpio-fan.c > @@ -20,6 +20,9 @@ > #include <linux/gpio/consumer.h> > #include <linux/of.h> > #include <linux/of_platform.h> > +#include <linux/pm.h> > +#include <linux/pm_runtime.h> > +#include <linux/regulator/consumer.h> > #include <linux/thermal.h> > > struct gpio_fan_speed { > @@ -42,6 +45,7 @@ struct gpio_fan_data { > bool pwm_enable; > struct gpio_desc *alarm_gpio; > struct work_struct alarm_work; > + struct regulator *supply; > }; > > /* > @@ -125,13 +129,38 @@ static int __get_fan_ctrl(struct gpio_fan_data *fan_data) > } > > /* Must be called with fan_data->lock held, except during initialization. */ > -static void set_fan_speed(struct gpio_fan_data *fan_data, int speed_index) > +static int set_fan_speed(struct gpio_fan_data *fan_data, int speed_index) What is the point of changing this function to return an error, but not checking for that error in the calling code ? > { > if (fan_data->speed_index == speed_index) > - return; > + return 0; > + > + if (fan_data->speed_index == 0 && speed_index > 0) { > + int ret; > + > + ret = pm_runtime_resume_and_get(fan_data->dev); > + if (ret < 0) { > + dev_err(fan_data->dev, > + "Failed to runtime_get device: %d\n", ret); Please drop that noise. If you think you need the messages, make it debug messages. > + return ret; > + } > + } > > __set_fan_ctrl(fan_data, fan_data->speed[speed_index].ctrl_val); > + > + if (fan_data->speed_index > 0 && speed_index == 0) { > + int ret; > + > + ret = pm_runtime_put_sync(fan_data->dev); > + if (ret < 0) { > + dev_err(fan_data->dev, > + "Failed to runtime_put device: %d\n", ret); > + return ret; > + } > + } > + > fan_data->speed_index = speed_index; > + > + return 0; > } > > static int get_fan_speed_index(struct gpio_fan_data *fan_data) > @@ -499,6 +528,8 @@ static void gpio_fan_stop(void *data) > mutex_lock(&fan_data->lock); > set_fan_speed(data, 0); > mutex_unlock(&fan_data->lock); > + > + pm_runtime_disable(fan_data->dev); > } > > static int gpio_fan_probe(struct platform_device *pdev) > @@ -521,6 +552,11 @@ static int gpio_fan_probe(struct platform_device *pdev) > platform_set_drvdata(pdev, fan_data); > mutex_init(&fan_data->lock); > > + fan_data->supply = devm_regulator_get(dev, "fan"); > + if (IS_ERR(fan_data->supply)) > + return dev_err_probe(dev, PTR_ERR(fan_data->supply), > + "Failed to get fan-supply"); > + > /* Configure control GPIOs if available. */ > if (fan_data->gpios && fan_data->num_gpios > 0) { > if (!fan_data->speed || fan_data->num_speed <= 1) > @@ -548,6 +584,17 @@ static int gpio_fan_probe(struct platform_device *pdev) > return err; > } > > + pm_runtime_set_suspended(&pdev->dev); > + pm_runtime_enable(&pdev->dev); > + /* If current GPIO state is active, mark RPM as active as well */ > + if (fan_data->speed_index > 0) { > + int ret; > + > + ret = pm_runtime_resume_and_get(&pdev->dev); > + if (ret) > + return ret; > + } > + > /* Optional cooling device register for Device tree platforms */ > fan_data->cdev = devm_thermal_of_cooling_device_register(dev, np, > "gpio-fan", fan_data, &gpio_fan_cool_ops); > @@ -568,6 +615,28 @@ static void gpio_fan_shutdown(struct platform_device *pdev) > } > } > > +static int gpio_fan_runtime_suspend(struct device *dev) > +{ > + struct gpio_fan_data *fan_data = dev_get_drvdata(dev); > + int ret = 0; > + > + if (fan_data->supply) > + ret = regulator_disable(fan_data->supply); > + > + return ret; > +} > + > +static int gpio_fan_runtime_resume(struct device *dev) > +{ > + struct gpio_fan_data *fan_data = dev_get_drvdata(dev); > + int ret = 0; > + > + if (fan_data->supply) > + ret = regulator_enable(fan_data->supply); > + > + return ret; > +} > + > static int gpio_fan_suspend(struct device *dev) > { > struct gpio_fan_data *fan_data = dev_get_drvdata(dev); > @@ -595,14 +664,18 @@ static int gpio_fan_resume(struct device *dev) > return 0; > } > > -static DEFINE_SIMPLE_DEV_PM_OPS(gpio_fan_pm, gpio_fan_suspend, gpio_fan_resume); > +static const struct dev_pm_ops gpio_fan_pm = { > + RUNTIME_PM_OPS(gpio_fan_runtime_suspend, > + gpio_fan_runtime_resume, NULL) > + SYSTEM_SLEEP_PM_OPS(gpio_fan_suspend, gpio_fan_resume) > +}; > > static struct platform_driver gpio_fan_driver = { > .probe = gpio_fan_probe, > .shutdown = gpio_fan_shutdown, > .driver = { > .name = "gpio-fan", > - .pm = pm_sleep_ptr(&gpio_fan_pm), > + .pm = pm_ptr(&gpio_fan_pm), > .of_match_table = of_gpio_fan_match, > }, > };
diff --git a/drivers/hwmon/gpio-fan.c b/drivers/hwmon/gpio-fan.c index cee3fa146d69a..db918d6858325 100644 --- a/drivers/hwmon/gpio-fan.c +++ b/drivers/hwmon/gpio-fan.c @@ -20,6 +20,9 @@ #include <linux/gpio/consumer.h> #include <linux/of.h> #include <linux/of_platform.h> +#include <linux/pm.h> +#include <linux/pm_runtime.h> +#include <linux/regulator/consumer.h> #include <linux/thermal.h> struct gpio_fan_speed { @@ -42,6 +45,7 @@ struct gpio_fan_data { bool pwm_enable; struct gpio_desc *alarm_gpio; struct work_struct alarm_work; + struct regulator *supply; }; /* @@ -125,13 +129,38 @@ static int __get_fan_ctrl(struct gpio_fan_data *fan_data) } /* Must be called with fan_data->lock held, except during initialization. */ -static void set_fan_speed(struct gpio_fan_data *fan_data, int speed_index) +static int set_fan_speed(struct gpio_fan_data *fan_data, int speed_index) { if (fan_data->speed_index == speed_index) - return; + return 0; + + if (fan_data->speed_index == 0 && speed_index > 0) { + int ret; + + ret = pm_runtime_resume_and_get(fan_data->dev); + if (ret < 0) { + dev_err(fan_data->dev, + "Failed to runtime_get device: %d\n", ret); + return ret; + } + } __set_fan_ctrl(fan_data, fan_data->speed[speed_index].ctrl_val); + + if (fan_data->speed_index > 0 && speed_index == 0) { + int ret; + + ret = pm_runtime_put_sync(fan_data->dev); + if (ret < 0) { + dev_err(fan_data->dev, + "Failed to runtime_put device: %d\n", ret); + return ret; + } + } + fan_data->speed_index = speed_index; + + return 0; } static int get_fan_speed_index(struct gpio_fan_data *fan_data) @@ -499,6 +528,8 @@ static void gpio_fan_stop(void *data) mutex_lock(&fan_data->lock); set_fan_speed(data, 0); mutex_unlock(&fan_data->lock); + + pm_runtime_disable(fan_data->dev); } static int gpio_fan_probe(struct platform_device *pdev) @@ -521,6 +552,11 @@ static int gpio_fan_probe(struct platform_device *pdev) platform_set_drvdata(pdev, fan_data); mutex_init(&fan_data->lock); + fan_data->supply = devm_regulator_get(dev, "fan"); + if (IS_ERR(fan_data->supply)) + return dev_err_probe(dev, PTR_ERR(fan_data->supply), + "Failed to get fan-supply"); + /* Configure control GPIOs if available. */ if (fan_data->gpios && fan_data->num_gpios > 0) { if (!fan_data->speed || fan_data->num_speed <= 1) @@ -548,6 +584,17 @@ static int gpio_fan_probe(struct platform_device *pdev) return err; } + pm_runtime_set_suspended(&pdev->dev); + pm_runtime_enable(&pdev->dev); + /* If current GPIO state is active, mark RPM as active as well */ + if (fan_data->speed_index > 0) { + int ret; + + ret = pm_runtime_resume_and_get(&pdev->dev); + if (ret) + return ret; + } + /* Optional cooling device register for Device tree platforms */ fan_data->cdev = devm_thermal_of_cooling_device_register(dev, np, "gpio-fan", fan_data, &gpio_fan_cool_ops); @@ -568,6 +615,28 @@ static void gpio_fan_shutdown(struct platform_device *pdev) } } +static int gpio_fan_runtime_suspend(struct device *dev) +{ + struct gpio_fan_data *fan_data = dev_get_drvdata(dev); + int ret = 0; + + if (fan_data->supply) + ret = regulator_disable(fan_data->supply); + + return ret; +} + +static int gpio_fan_runtime_resume(struct device *dev) +{ + struct gpio_fan_data *fan_data = dev_get_drvdata(dev); + int ret = 0; + + if (fan_data->supply) + ret = regulator_enable(fan_data->supply); + + return ret; +} + static int gpio_fan_suspend(struct device *dev) { struct gpio_fan_data *fan_data = dev_get_drvdata(dev); @@ -595,14 +664,18 @@ static int gpio_fan_resume(struct device *dev) return 0; } -static DEFINE_SIMPLE_DEV_PM_OPS(gpio_fan_pm, gpio_fan_suspend, gpio_fan_resume); +static const struct dev_pm_ops gpio_fan_pm = { + RUNTIME_PM_OPS(gpio_fan_runtime_suspend, + gpio_fan_runtime_resume, NULL) + SYSTEM_SLEEP_PM_OPS(gpio_fan_suspend, gpio_fan_resume) +}; static struct platform_driver gpio_fan_driver = { .probe = gpio_fan_probe, .shutdown = gpio_fan_shutdown, .driver = { .name = "gpio-fan", - .pm = pm_sleep_ptr(&gpio_fan_pm), + .pm = pm_ptr(&gpio_fan_pm), .of_match_table = of_gpio_fan_match, }, };
FANs might be supplied by a regulator which needs to be enabled as well. This is implemented using runtime PM. Every time speed_index changes from 0 to non-zero and vise versa RPM is resumed or suspended. Intitial RPM state is determined by initial value of speed_index. Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com> --- Patch 1 & 2 from v1 [1] have already been applied, although number 2 [2] is not yet showing in next-20250305. Patches 3 & 4 (just removing comments) from v1 have been dropped, so only this patch remains. Changes in v2: * Make regulator non-optional [1] https://lore.kernel.org/all/20250210145934.761280-1-alexander.stein@ew.tq-group.com/ [2] https://web.git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git/commit/?h=hwmon-next&id=9fee7d19bab635f89223cc40dfd2c8797fdc4988 --- drivers/hwmon/gpio-fan.c | 81 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 77 insertions(+), 4 deletions(-)