* [PATCH v6 1/4] pwm: rename pwm_apply_state() to pwm_apply_might_sleep()
[not found] <cover.1701248996.git.sean@mess.org>
@ 2023-11-29 9:13 ` Sean Young
2023-12-09 13:57 ` Uwe Kleine-König
2023-12-10 3:59 ` Dmitry Torokhov
2023-11-29 9:13 ` [PATCH v6 3/4] pwm: bcm2835: allow pwm driver to be used in atomic context Sean Young
1 sibling, 2 replies; 11+ messages in thread
From: Sean Young @ 2023-11-29 9:13 UTC (permalink / raw)
To: linux-media, linux-pwm, Ivaylo Dimitrov, Thierry Reding,
Uwe Kleine-König, Jonathan Corbet, Jani Nikula,
Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin, David Airlie,
Daniel Vetter, Javier Martinez Canillas, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Jean Delvare, Guenter Roeck,
Support Opensource, Dmitry Torokhov, Pavel Machek, Lee Jones,
Sean Young, Mauro Carvalho Chehab, Hans de Goede,
Ilpo Järvinen, Mark Gross, Liam Girdwood, Mark Brown,
Daniel Thompson, Jingoo Han, Helge Deller
Cc: Jani Nikula, linux-doc, linux-kernel, intel-gfx, dri-devel,
linux-hwmon, linux-input, linux-leds, platform-driver-x86,
linux-arm-kernel, linux-fbdev
In order to introduce a pwm api which can be used from atomic context,
we will need two functions for applying pwm changes:
int pwm_apply_might_sleep(struct pwm *, struct pwm_state *);
int pwm_apply_atomic(struct pwm *, struct pwm_state *);
This commit just deals with renaming pwm_apply_state(), a following
commit will introduce the pwm_apply_atomic() function.
Acked-by: Hans de Goede <hdegoede@redhat.com>
Acked-by: Jani Nikula <jani.nikula@intel.com>
Acked-by: Lee Jones <lee@kernel.org>
Signed-off-by: Sean Young <sean@mess.org>
---
Documentation/driver-api/pwm.rst | 8 +++---
.../gpu/drm/i915/display/intel_backlight.c | 6 ++--
drivers/gpu/drm/solomon/ssd130x.c | 2 +-
drivers/hwmon/pwm-fan.c | 8 +++---
drivers/input/misc/da7280.c | 4 +--
drivers/input/misc/pwm-beeper.c | 4 +--
drivers/input/misc/pwm-vibra.c | 8 +++---
drivers/leds/leds-pwm.c | 2 +-
drivers/leds/rgb/leds-pwm-multicolor.c | 4 +--
drivers/media/rc/pwm-ir-tx.c | 4 +--
drivers/platform/x86/lenovo-yogabook.c | 2 +-
drivers/pwm/core.c | 18 ++++++------
drivers/pwm/pwm-twl-led.c | 2 +-
drivers/pwm/pwm-vt8500.c | 2 +-
drivers/pwm/sysfs.c | 10 +++----
drivers/regulator/pwm-regulator.c | 4 +--
drivers/video/backlight/lm3630a_bl.c | 2 +-
drivers/video/backlight/lp855x_bl.c | 2 +-
drivers/video/backlight/pwm_bl.c | 12 ++++----
drivers/video/fbdev/ssd1307fb.c | 2 +-
include/linux/pwm.h | 28 +++++++++----------
21 files changed, 67 insertions(+), 67 deletions(-)
diff --git a/Documentation/driver-api/pwm.rst b/Documentation/driver-api/pwm.rst
index bb264490a87a..f1d8197c8c43 100644
--- a/Documentation/driver-api/pwm.rst
+++ b/Documentation/driver-api/pwm.rst
@@ -41,7 +41,7 @@ the getter, devm_pwm_get() and devm_fwnode_pwm_get(), also exist.
After being requested, a PWM has to be configured using::
- int pwm_apply_state(struct pwm_device *pwm, struct pwm_state *state);
+ int pwm_apply_might_sleep(struct pwm_device *pwm, struct pwm_state *state);
This API controls both the PWM period/duty_cycle config and the
enable/disable state.
@@ -57,13 +57,13 @@ If supported by the driver, the signal can be optimized, for example to improve
EMI by phase shifting the individual channels of a chip.
The pwm_config(), pwm_enable() and pwm_disable() functions are just wrappers
-around pwm_apply_state() and should not be used if the user wants to change
+around pwm_apply_might_sleep() and should not be used if the user wants to change
several parameter at once. For example, if you see pwm_config() and
pwm_{enable,disable}() calls in the same function, this probably means you
-should switch to pwm_apply_state().
+should switch to pwm_apply_might_sleep().
The PWM user API also allows one to query the PWM state that was passed to the
-last invocation of pwm_apply_state() using pwm_get_state(). Note this is
+last invocation of pwm_apply_might_sleep() using pwm_get_state(). Note this is
different to what the driver has actually implemented if the request cannot be
satisfied exactly with the hardware in use. There is currently no way for
consumers to get the actually implemented settings.
diff --git a/drivers/gpu/drm/i915/display/intel_backlight.c b/drivers/gpu/drm/i915/display/intel_backlight.c
index 2e8f17c04522..ff9b9918b0a1 100644
--- a/drivers/gpu/drm/i915/display/intel_backlight.c
+++ b/drivers/gpu/drm/i915/display/intel_backlight.c
@@ -274,7 +274,7 @@ static void ext_pwm_set_backlight(const struct drm_connector_state *conn_state,
struct intel_panel *panel = &to_intel_connector(conn_state->connector)->panel;
pwm_set_relative_duty_cycle(&panel->backlight.pwm_state, level, 100);
- pwm_apply_state(panel->backlight.pwm, &panel->backlight.pwm_state);
+ pwm_apply_might_sleep(panel->backlight.pwm, &panel->backlight.pwm_state);
}
static void
@@ -427,7 +427,7 @@ static void ext_pwm_disable_backlight(const struct drm_connector_state *old_conn
intel_backlight_set_pwm_level(old_conn_state, level);
panel->backlight.pwm_state.enabled = false;
- pwm_apply_state(panel->backlight.pwm, &panel->backlight.pwm_state);
+ pwm_apply_might_sleep(panel->backlight.pwm, &panel->backlight.pwm_state);
}
void intel_backlight_disable(const struct drm_connector_state *old_conn_state)
@@ -749,7 +749,7 @@ static void ext_pwm_enable_backlight(const struct intel_crtc_state *crtc_state,
pwm_set_relative_duty_cycle(&panel->backlight.pwm_state, level, 100);
panel->backlight.pwm_state.enabled = true;
- pwm_apply_state(panel->backlight.pwm, &panel->backlight.pwm_state);
+ pwm_apply_might_sleep(panel->backlight.pwm, &panel->backlight.pwm_state);
}
static void __intel_backlight_enable(const struct intel_crtc_state *crtc_state,
diff --git a/drivers/gpu/drm/solomon/ssd130x.c b/drivers/gpu/drm/solomon/ssd130x.c
index e0174f82e353..cce043a4a1dc 100644
--- a/drivers/gpu/drm/solomon/ssd130x.c
+++ b/drivers/gpu/drm/solomon/ssd130x.c
@@ -319,7 +319,7 @@ static int ssd130x_pwm_enable(struct ssd130x_device *ssd130x)
pwm_init_state(ssd130x->pwm, &pwmstate);
pwm_set_relative_duty_cycle(&pwmstate, 50, 100);
- pwm_apply_state(ssd130x->pwm, &pwmstate);
+ pwm_apply_might_sleep(ssd130x->pwm, &pwmstate);
/* Enable the PWM */
pwm_enable(ssd130x->pwm);
diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c
index 6e4516c2ab89..b67bc9e833c0 100644
--- a/drivers/hwmon/pwm-fan.c
+++ b/drivers/hwmon/pwm-fan.c
@@ -151,7 +151,7 @@ static int pwm_fan_power_on(struct pwm_fan_ctx *ctx)
}
state->enabled = true;
- ret = pwm_apply_state(ctx->pwm, state);
+ ret = pwm_apply_might_sleep(ctx->pwm, state);
if (ret) {
dev_err(ctx->dev, "failed to enable PWM\n");
goto disable_regulator;
@@ -181,7 +181,7 @@ static int pwm_fan_power_off(struct pwm_fan_ctx *ctx)
state->enabled = false;
state->duty_cycle = 0;
- ret = pwm_apply_state(ctx->pwm, state);
+ ret = pwm_apply_might_sleep(ctx->pwm, state);
if (ret) {
dev_err(ctx->dev, "failed to disable PWM\n");
return ret;
@@ -207,7 +207,7 @@ static int __set_pwm(struct pwm_fan_ctx *ctx, unsigned long pwm)
period = state->period;
state->duty_cycle = DIV_ROUND_UP(pwm * (period - 1), MAX_PWM);
- ret = pwm_apply_state(ctx->pwm, state);
+ ret = pwm_apply_might_sleep(ctx->pwm, state);
if (ret)
return ret;
ret = pwm_fan_power_on(ctx);
@@ -278,7 +278,7 @@ static int pwm_fan_update_enable(struct pwm_fan_ctx *ctx, long val)
state,
&enable_regulator);
- pwm_apply_state(ctx->pwm, state);
+ pwm_apply_might_sleep(ctx->pwm, state);
pwm_fan_switch_power(ctx, enable_regulator);
pwm_fan_update_state(ctx, 0);
}
diff --git a/drivers/input/misc/da7280.c b/drivers/input/misc/da7280.c
index ce82548916bb..c1fa75c0f970 100644
--- a/drivers/input/misc/da7280.c
+++ b/drivers/input/misc/da7280.c
@@ -352,7 +352,7 @@ static int da7280_haptic_set_pwm(struct da7280_haptic *haptics, bool enabled)
state.duty_cycle = period_mag_multi;
}
- error = pwm_apply_state(haptics->pwm_dev, &state);
+ error = pwm_apply_might_sleep(haptics->pwm_dev, &state);
if (error)
dev_err(haptics->dev, "Failed to apply pwm state: %d\n", error);
@@ -1175,7 +1175,7 @@ static int da7280_probe(struct i2c_client *client)
/* Sync up PWM state and ensure it is off. */
pwm_init_state(haptics->pwm_dev, &state);
state.enabled = false;
- error = pwm_apply_state(haptics->pwm_dev, &state);
+ error = pwm_apply_might_sleep(haptics->pwm_dev, &state);
if (error) {
dev_err(dev, "Failed to apply PWM state: %d\n", error);
return error;
diff --git a/drivers/input/misc/pwm-beeper.c b/drivers/input/misc/pwm-beeper.c
index 1e731d8397c6..5b9aedf4362f 100644
--- a/drivers/input/misc/pwm-beeper.c
+++ b/drivers/input/misc/pwm-beeper.c
@@ -39,7 +39,7 @@ static int pwm_beeper_on(struct pwm_beeper *beeper, unsigned long period)
state.period = period;
pwm_set_relative_duty_cycle(&state, 50, 100);
- error = pwm_apply_state(beeper->pwm, &state);
+ error = pwm_apply_might_sleep(beeper->pwm, &state);
if (error)
return error;
@@ -138,7 +138,7 @@ static int pwm_beeper_probe(struct platform_device *pdev)
/* Sync up PWM state and ensure it is off. */
pwm_init_state(beeper->pwm, &state);
state.enabled = false;
- error = pwm_apply_state(beeper->pwm, &state);
+ error = pwm_apply_might_sleep(beeper->pwm, &state);
if (error) {
dev_err(dev, "failed to apply initial PWM state: %d\n",
error);
diff --git a/drivers/input/misc/pwm-vibra.c b/drivers/input/misc/pwm-vibra.c
index acac79c488aa..3e5ed685ed8f 100644
--- a/drivers/input/misc/pwm-vibra.c
+++ b/drivers/input/misc/pwm-vibra.c
@@ -56,7 +56,7 @@ static int pwm_vibrator_start(struct pwm_vibrator *vibrator)
pwm_set_relative_duty_cycle(&state, vibrator->level, 0xffff);
state.enabled = true;
- err = pwm_apply_state(vibrator->pwm, &state);
+ err = pwm_apply_might_sleep(vibrator->pwm, &state);
if (err) {
dev_err(pdev, "failed to apply pwm state: %d\n", err);
return err;
@@ -67,7 +67,7 @@ static int pwm_vibrator_start(struct pwm_vibrator *vibrator)
state.duty_cycle = vibrator->direction_duty_cycle;
state.enabled = true;
- err = pwm_apply_state(vibrator->pwm_dir, &state);
+ err = pwm_apply_might_sleep(vibrator->pwm_dir, &state);
if (err) {
dev_err(pdev, "failed to apply dir-pwm state: %d\n", err);
pwm_disable(vibrator->pwm);
@@ -160,7 +160,7 @@ static int pwm_vibrator_probe(struct platform_device *pdev)
/* Sync up PWM state and ensure it is off. */
pwm_init_state(vibrator->pwm, &state);
state.enabled = false;
- err = pwm_apply_state(vibrator->pwm, &state);
+ err = pwm_apply_might_sleep(vibrator->pwm, &state);
if (err) {
dev_err(&pdev->dev, "failed to apply initial PWM state: %d\n",
err);
@@ -174,7 +174,7 @@ static int pwm_vibrator_probe(struct platform_device *pdev)
/* Sync up PWM state and ensure it is off. */
pwm_init_state(vibrator->pwm_dir, &state);
state.enabled = false;
- err = pwm_apply_state(vibrator->pwm_dir, &state);
+ err = pwm_apply_might_sleep(vibrator->pwm_dir, &state);
if (err) {
dev_err(&pdev->dev, "failed to apply initial PWM state: %d\n",
err);
diff --git a/drivers/leds/leds-pwm.c b/drivers/leds/leds-pwm.c
index 2b3bf1353b70..4e3936a39d0e 100644
--- a/drivers/leds/leds-pwm.c
+++ b/drivers/leds/leds-pwm.c
@@ -54,7 +54,7 @@ static int led_pwm_set(struct led_classdev *led_cdev,
led_dat->pwmstate.duty_cycle = duty;
led_dat->pwmstate.enabled = true;
- return pwm_apply_state(led_dat->pwm, &led_dat->pwmstate);
+ return pwm_apply_might_sleep(led_dat->pwm, &led_dat->pwmstate);
}
__attribute__((nonnull))
diff --git a/drivers/leds/rgb/leds-pwm-multicolor.c b/drivers/leds/rgb/leds-pwm-multicolor.c
index 46cd062b8b24..e1a81e0109e8 100644
--- a/drivers/leds/rgb/leds-pwm-multicolor.c
+++ b/drivers/leds/rgb/leds-pwm-multicolor.c
@@ -51,8 +51,8 @@ static int led_pwm_mc_set(struct led_classdev *cdev,
priv->leds[i].state.duty_cycle = duty;
priv->leds[i].state.enabled = duty > 0;
- ret = pwm_apply_state(priv->leds[i].pwm,
- &priv->leds[i].state);
+ ret = pwm_apply_might_sleep(priv->leds[i].pwm,
+ &priv->leds[i].state);
if (ret)
break;
}
diff --git a/drivers/media/rc/pwm-ir-tx.c b/drivers/media/rc/pwm-ir-tx.c
index c5f37c03af9c..cf51e2760975 100644
--- a/drivers/media/rc/pwm-ir-tx.c
+++ b/drivers/media/rc/pwm-ir-tx.c
@@ -68,7 +68,7 @@ static int pwm_ir_tx(struct rc_dev *dev, unsigned int *txbuf,
for (i = 0; i < count; i++) {
state.enabled = !(i % 2);
- pwm_apply_state(pwm, &state);
+ pwm_apply_might_sleep(pwm, &state);
edge = ktime_add_us(edge, txbuf[i]);
delta = ktime_us_delta(edge, ktime_get());
@@ -77,7 +77,7 @@ static int pwm_ir_tx(struct rc_dev *dev, unsigned int *txbuf,
}
state.enabled = false;
- pwm_apply_state(pwm, &state);
+ pwm_apply_might_sleep(pwm, &state);
return count;
}
diff --git a/drivers/platform/x86/lenovo-yogabook.c b/drivers/platform/x86/lenovo-yogabook.c
index b8d0239192cb..fd62bf746ebd 100644
--- a/drivers/platform/x86/lenovo-yogabook.c
+++ b/drivers/platform/x86/lenovo-yogabook.c
@@ -435,7 +435,7 @@ static int yogabook_pdev_set_kbd_backlight(struct yogabook_data *data, u8 level)
.enabled = level,
};
- pwm_apply_state(data->kbd_bl_pwm, &state);
+ pwm_apply_might_sleep(data->kbd_bl_pwm, &state);
gpiod_set_value(data->kbd_bl_led_enable, level ? 1 : 0);
return 0;
}
diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 29078486534d..fc92ba622e56 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -356,8 +356,8 @@ struct pwm_device *pwm_request_from_chip(struct pwm_chip *chip,
}
EXPORT_SYMBOL_GPL(pwm_request_from_chip);
-static void pwm_apply_state_debug(struct pwm_device *pwm,
- const struct pwm_state *state)
+static void pwm_apply_debug(struct pwm_device *pwm,
+ const struct pwm_state *state)
{
struct pwm_state *last = &pwm->last;
struct pwm_chip *chip = pwm->chip;
@@ -463,11 +463,11 @@ static void pwm_apply_state_debug(struct pwm_device *pwm,
}
/**
- * pwm_apply_state() - atomically apply a new state to a PWM device
+ * pwm_apply_might_sleep() - atomically apply a new state to a PWM device
* @pwm: PWM device
* @state: new state to apply
*/
-int pwm_apply_state(struct pwm_device *pwm, const struct pwm_state *state)
+int pwm_apply_might_sleep(struct pwm_device *pwm, const struct pwm_state *state)
{
struct pwm_chip *chip;
int err;
@@ -475,7 +475,7 @@ int pwm_apply_state(struct pwm_device *pwm, const struct pwm_state *state)
/*
* Some lowlevel driver's implementations of .apply() make use of
* mutexes, also with some drivers only returning when the new
- * configuration is active calling pwm_apply_state() from atomic context
+ * configuration is active calling pwm_apply_might_sleep() from atomic context
* is a bad idea. So make it explicit that calling this function might
* sleep.
*/
@@ -505,11 +505,11 @@ int pwm_apply_state(struct pwm_device *pwm, const struct pwm_state *state)
* only do this after pwm->state was applied as some
* implementations of .get_state depend on this
*/
- pwm_apply_state_debug(pwm, state);
+ pwm_apply_debug(pwm, state);
return 0;
}
-EXPORT_SYMBOL_GPL(pwm_apply_state);
+EXPORT_SYMBOL_GPL(pwm_apply_might_sleep);
/**
* pwm_capture() - capture and report a PWM signal
@@ -567,7 +567,7 @@ int pwm_adjust_config(struct pwm_device *pwm)
state.period = pargs.period;
state.polarity = pargs.polarity;
- return pwm_apply_state(pwm, &state);
+ return pwm_apply_might_sleep(pwm, &state);
}
/*
@@ -590,7 +590,7 @@ int pwm_adjust_config(struct pwm_device *pwm)
state.duty_cycle = state.period - state.duty_cycle;
}
- return pwm_apply_state(pwm, &state);
+ return pwm_apply_might_sleep(pwm, &state);
}
EXPORT_SYMBOL_GPL(pwm_adjust_config);
diff --git a/drivers/pwm/pwm-twl-led.c b/drivers/pwm/pwm-twl-led.c
index 625233f4703a..70eec1c66747 100644
--- a/drivers/pwm/pwm-twl-led.c
+++ b/drivers/pwm/pwm-twl-led.c
@@ -172,7 +172,7 @@ static int twl4030_pwmled_apply(struct pwm_chip *chip, struct pwm_device *pwm,
* We cannot skip calling ->config even if state->period ==
* pwm->state.period && state->duty_cycle == pwm->state.duty_cycle
* because we might have exited early in the last call to
- * pwm_apply_state because of !state->enabled and so the two values in
+ * pwm_apply_might_sleep because of !state->enabled and so the two values in
* pwm->state might not be configured in hardware.
*/
ret = twl4030_pwmled_config(pwm->chip, pwm,
diff --git a/drivers/pwm/pwm-vt8500.c b/drivers/pwm/pwm-vt8500.c
index 5568d5312d3c..3d1d11062558 100644
--- a/drivers/pwm/pwm-vt8500.c
+++ b/drivers/pwm/pwm-vt8500.c
@@ -206,7 +206,7 @@ static int vt8500_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
* We cannot skip calling ->config even if state->period ==
* pwm->state.period && state->duty_cycle == pwm->state.duty_cycle
* because we might have exited early in the last call to
- * pwm_apply_state because of !state->enabled and so the two values in
+ * pwm_apply_might_sleep because of !state->enabled and so the two values in
* pwm->state might not be configured in hardware.
*/
err = vt8500_pwm_config(pwm->chip, pwm, state->duty_cycle, state->period);
diff --git a/drivers/pwm/sysfs.c b/drivers/pwm/sysfs.c
index 8d1254761e4d..052ccadbdabf 100644
--- a/drivers/pwm/sysfs.c
+++ b/drivers/pwm/sysfs.c
@@ -62,7 +62,7 @@ static ssize_t period_store(struct device *child,
mutex_lock(&export->lock);
pwm_get_state(pwm, &state);
state.period = val;
- ret = pwm_apply_state(pwm, &state);
+ ret = pwm_apply_might_sleep(pwm, &state);
mutex_unlock(&export->lock);
return ret ? : size;
@@ -97,7 +97,7 @@ static ssize_t duty_cycle_store(struct device *child,
mutex_lock(&export->lock);
pwm_get_state(pwm, &state);
state.duty_cycle = val;
- ret = pwm_apply_state(pwm, &state);
+ ret = pwm_apply_might_sleep(pwm, &state);
mutex_unlock(&export->lock);
return ret ? : size;
@@ -144,7 +144,7 @@ static ssize_t enable_store(struct device *child,
goto unlock;
}
- ret = pwm_apply_state(pwm, &state);
+ ret = pwm_apply_might_sleep(pwm, &state);
unlock:
mutex_unlock(&export->lock);
@@ -194,7 +194,7 @@ static ssize_t polarity_store(struct device *child,
mutex_lock(&export->lock);
pwm_get_state(pwm, &state);
state.polarity = polarity;
- ret = pwm_apply_state(pwm, &state);
+ ret = pwm_apply_might_sleep(pwm, &state);
mutex_unlock(&export->lock);
return ret ? : size;
@@ -401,7 +401,7 @@ static int pwm_class_apply_state(struct pwm_export *export,
struct pwm_device *pwm,
struct pwm_state *state)
{
- int ret = pwm_apply_state(pwm, state);
+ int ret = pwm_apply_might_sleep(pwm, state);
/* release lock taken in pwm_class_get_state */
mutex_unlock(&export->lock);
diff --git a/drivers/regulator/pwm-regulator.c b/drivers/regulator/pwm-regulator.c
index 2aff6db748e2..698c420e0869 100644
--- a/drivers/regulator/pwm-regulator.c
+++ b/drivers/regulator/pwm-regulator.c
@@ -90,7 +90,7 @@ static int pwm_regulator_set_voltage_sel(struct regulator_dev *rdev,
pwm_set_relative_duty_cycle(&pstate,
drvdata->duty_cycle_table[selector].dutycycle, 100);
- ret = pwm_apply_state(drvdata->pwm, &pstate);
+ ret = pwm_apply_might_sleep(drvdata->pwm, &pstate);
if (ret) {
dev_err(&rdev->dev, "Failed to configure PWM: %d\n", ret);
return ret;
@@ -216,7 +216,7 @@ static int pwm_regulator_set_voltage(struct regulator_dev *rdev,
pwm_set_relative_duty_cycle(&pstate, dutycycle, duty_unit);
- ret = pwm_apply_state(drvdata->pwm, &pstate);
+ ret = pwm_apply_might_sleep(drvdata->pwm, &pstate);
if (ret) {
dev_err(&rdev->dev, "Failed to configure PWM: %d\n", ret);
return ret;
diff --git a/drivers/video/backlight/lm3630a_bl.c b/drivers/video/backlight/lm3630a_bl.c
index 8fcb62be597b..a3412c936ca2 100644
--- a/drivers/video/backlight/lm3630a_bl.c
+++ b/drivers/video/backlight/lm3630a_bl.c
@@ -180,7 +180,7 @@ static int lm3630a_pwm_ctrl(struct lm3630a_chip *pchip, int br, int br_max)
pchip->pwmd_state.enabled = pchip->pwmd_state.duty_cycle ? true : false;
- return pwm_apply_state(pchip->pwmd, &pchip->pwmd_state);
+ return pwm_apply_might_sleep(pchip->pwmd, &pchip->pwmd_state);
}
/* update and get brightness */
diff --git a/drivers/video/backlight/lp855x_bl.c b/drivers/video/backlight/lp855x_bl.c
index da1f124db69c..7075bfab59c4 100644
--- a/drivers/video/backlight/lp855x_bl.c
+++ b/drivers/video/backlight/lp855x_bl.c
@@ -234,7 +234,7 @@ static int lp855x_pwm_ctrl(struct lp855x *lp, int br, int max_br)
state.duty_cycle = div_u64(br * state.period, max_br);
state.enabled = state.duty_cycle;
- return pwm_apply_state(lp->pwm, &state);
+ return pwm_apply_might_sleep(lp->pwm, &state);
}
static int lp855x_bl_update_status(struct backlight_device *bl)
diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index 289bd9ce4d36..35c716e9043c 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -103,7 +103,7 @@ static int pwm_backlight_update_status(struct backlight_device *bl)
pwm_get_state(pb->pwm, &state);
state.duty_cycle = compute_duty_cycle(pb, brightness, &state);
state.enabled = true;
- pwm_apply_state(pb->pwm, &state);
+ pwm_apply_might_sleep(pb->pwm, &state);
pwm_backlight_power_on(pb);
} else {
@@ -120,7 +120,7 @@ static int pwm_backlight_update_status(struct backlight_device *bl)
* inactive output.
*/
state.enabled = !pb->power_supply && !pb->enable_gpio;
- pwm_apply_state(pb->pwm, &state);
+ pwm_apply_might_sleep(pb->pwm, &state);
}
if (pb->notify_after)
@@ -528,7 +528,7 @@ static int pwm_backlight_probe(struct platform_device *pdev)
if (!state.period && (data->pwm_period_ns > 0))
state.period = data->pwm_period_ns;
- ret = pwm_apply_state(pb->pwm, &state);
+ ret = pwm_apply_might_sleep(pb->pwm, &state);
if (ret) {
dev_err(&pdev->dev, "failed to apply initial PWM state: %d\n",
ret);
@@ -633,7 +633,7 @@ static void pwm_backlight_remove(struct platform_device *pdev)
pwm_get_state(pb->pwm, &state);
state.duty_cycle = 0;
state.enabled = false;
- pwm_apply_state(pb->pwm, &state);
+ pwm_apply_might_sleep(pb->pwm, &state);
if (pb->exit)
pb->exit(&pdev->dev);
@@ -649,7 +649,7 @@ static void pwm_backlight_shutdown(struct platform_device *pdev)
pwm_get_state(pb->pwm, &state);
state.duty_cycle = 0;
state.enabled = false;
- pwm_apply_state(pb->pwm, &state);
+ pwm_apply_might_sleep(pb->pwm, &state);
}
#ifdef CONFIG_PM_SLEEP
@@ -673,7 +673,7 @@ static int pwm_backlight_suspend(struct device *dev)
pwm_get_state(pb->pwm, &state);
state.duty_cycle = 0;
state.enabled = false;
- pwm_apply_state(pb->pwm, &state);
+ pwm_apply_might_sleep(pb->pwm, &state);
if (pb->notify_after)
pb->notify_after(pb->dev, 0);
diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index 5ae48e36fccb..1a4f90ea7d5a 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -347,7 +347,7 @@ static int ssd1307fb_init(struct ssd1307fb_par *par)
pwm_init_state(par->pwm, &pwmstate);
pwm_set_relative_duty_cycle(&pwmstate, 50, 100);
- pwm_apply_state(par->pwm, &pwmstate);
+ pwm_apply_might_sleep(par->pwm, &pwmstate);
/* Enable the PWM */
pwm_enable(par->pwm);
diff --git a/include/linux/pwm.h b/include/linux/pwm.h
index cda3597b84f2..5c5c456948a4 100644
--- a/include/linux/pwm.h
+++ b/include/linux/pwm.h
@@ -93,8 +93,8 @@ struct pwm_device {
* @state: state to fill with the current PWM state
*
* The returned PWM state represents the state that was applied by a previous call to
- * pwm_apply_state(). Drivers may have to slightly tweak that state before programming it to
- * hardware. If pwm_apply_state() was never called, this returns either the current hardware
+ * pwm_apply_might_sleep(). Drivers may have to slightly tweak that state before programming it to
+ * hardware. If pwm_apply_might_sleep() was never called, this returns either the current hardware
* state (if supported) or the default settings.
*/
static inline void pwm_get_state(const struct pwm_device *pwm,
@@ -158,20 +158,20 @@ static inline void pwm_get_args(const struct pwm_device *pwm,
}
/**
- * pwm_init_state() - prepare a new state to be applied with pwm_apply_state()
+ * pwm_init_state() - prepare a new state to be applied with pwm_apply_might_sleep()
* @pwm: PWM device
* @state: state to fill with the prepared PWM state
*
* This functions prepares a state that can later be tweaked and applied
- * to the PWM device with pwm_apply_state(). This is a convenient function
+ * to the PWM device with pwm_apply_might_sleep(). This is a convenient function
* that first retrieves the current PWM state and the replaces the period
* and polarity fields with the reference values defined in pwm->args.
* Once the function returns, you can adjust the ->enabled and ->duty_cycle
- * fields according to your needs before calling pwm_apply_state().
+ * fields according to your needs before calling pwm_apply_might_sleep().
*
* ->duty_cycle is initially set to zero to avoid cases where the current
* ->duty_cycle value exceed the pwm_args->period one, which would trigger
- * an error if the user calls pwm_apply_state() without adjusting ->duty_cycle
+ * an error if the user calls pwm_apply_might_sleep() without adjusting ->duty_cycle
* first.
*/
static inline void pwm_init_state(const struct pwm_device *pwm,
@@ -227,7 +227,7 @@ pwm_get_relative_duty_cycle(const struct pwm_state *state, unsigned int scale)
*
* pwm_init_state(pwm, &state);
* pwm_set_relative_duty_cycle(&state, 50, 100);
- * pwm_apply_state(pwm, &state);
+ * pwm_apply_might_sleep(pwm, &state);
*
* This functions returns -EINVAL if @duty_cycle and/or @scale are
* inconsistent (@scale == 0 or @duty_cycle > @scale).
@@ -307,7 +307,7 @@ struct pwm_chip {
#if IS_ENABLED(CONFIG_PWM)
/* PWM user APIs */
-int pwm_apply_state(struct pwm_device *pwm, const struct pwm_state *state);
+int pwm_apply_might_sleep(struct pwm_device *pwm, const struct pwm_state *state);
int pwm_adjust_config(struct pwm_device *pwm);
/**
@@ -335,7 +335,7 @@ static inline int pwm_config(struct pwm_device *pwm, int duty_ns,
state.duty_cycle = duty_ns;
state.period = period_ns;
- return pwm_apply_state(pwm, &state);
+ return pwm_apply_might_sleep(pwm, &state);
}
/**
@@ -356,7 +356,7 @@ static inline int pwm_enable(struct pwm_device *pwm)
return 0;
state.enabled = true;
- return pwm_apply_state(pwm, &state);
+ return pwm_apply_might_sleep(pwm, &state);
}
/**
@@ -375,7 +375,7 @@ static inline void pwm_disable(struct pwm_device *pwm)
return;
state.enabled = false;
- pwm_apply_state(pwm, &state);
+ pwm_apply_might_sleep(pwm, &state);
}
/* PWM provider APIs */
@@ -406,8 +406,8 @@ struct pwm_device *devm_fwnode_pwm_get(struct device *dev,
struct fwnode_handle *fwnode,
const char *con_id);
#else
-static inline int pwm_apply_state(struct pwm_device *pwm,
- const struct pwm_state *state)
+static inline int pwm_apply_might_sleep(struct pwm_device *pwm,
+ const struct pwm_state *state)
{
might_sleep();
return -ENOTSUPP;
@@ -524,7 +524,7 @@ static inline void pwm_apply_args(struct pwm_device *pwm)
state.period = pwm->args.period;
state.usage_power = false;
- pwm_apply_state(pwm, &state);
+ pwm_apply_might_sleep(pwm, &state);
}
struct pwm_lookup {
--
2.43.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v6 3/4] pwm: bcm2835: allow pwm driver to be used in atomic context
[not found] <cover.1701248996.git.sean@mess.org>
2023-11-29 9:13 ` [PATCH v6 1/4] pwm: rename pwm_apply_state() to pwm_apply_might_sleep() Sean Young
@ 2023-11-29 9:13 ` Sean Young
2023-11-29 17:47 ` Florian Fainelli
2023-12-08 16:22 ` Thierry Reding
1 sibling, 2 replies; 11+ messages in thread
From: Sean Young @ 2023-11-29 9:13 UTC (permalink / raw)
To: linux-media, linux-pwm, Ivaylo Dimitrov, Thierry Reding,
Uwe Kleine-König, Florian Fainelli,
Broadcom internal kernel review list, Ray Jui, Scott Branden
Cc: Sean Young, linux-rpi-kernel, linux-arm-kernel, linux-kernel
clk_get_rate() may do a mutex lock. Fetch the clock rate once, and prevent
rate changes using clk_rate_exclusive_get().
Signed-off-by: Sean Young <sean@mess.org>
---
drivers/pwm/pwm-bcm2835.c | 31 +++++++++++++++++++++----------
1 file changed, 21 insertions(+), 10 deletions(-)
diff --git a/drivers/pwm/pwm-bcm2835.c b/drivers/pwm/pwm-bcm2835.c
index 9777babd5b95..52748194a3da 100644
--- a/drivers/pwm/pwm-bcm2835.c
+++ b/drivers/pwm/pwm-bcm2835.c
@@ -28,6 +28,7 @@ struct bcm2835_pwm {
struct device *dev;
void __iomem *base;
struct clk *clk;
+ unsigned long rate;
};
static inline struct bcm2835_pwm *to_bcm2835_pwm(struct pwm_chip *chip)
@@ -63,17 +64,11 @@ static int bcm2835_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
{
struct bcm2835_pwm *pc = to_bcm2835_pwm(chip);
- unsigned long rate = clk_get_rate(pc->clk);
unsigned long long period_cycles;
u64 max_period;
u32 val;
- if (!rate) {
- dev_err(pc->dev, "failed to get clock rate\n");
- return -EINVAL;
- }
-
/*
* period_cycles must be a 32 bit value, so period * rate / NSEC_PER_SEC
* must be <= U32_MAX. As U32_MAX * NSEC_PER_SEC < U64_MAX the
@@ -88,13 +83,13 @@ static int bcm2835_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
* <=> period < ((U32_MAX * NSEC_PER_SEC + NSEC_PER_SEC/2) / rate
* <=> period <= ceil((U32_MAX * NSEC_PER_SEC + NSEC_PER_SEC/2) / rate) - 1
*/
- max_period = DIV_ROUND_UP_ULL((u64)U32_MAX * NSEC_PER_SEC + NSEC_PER_SEC / 2, rate) - 1;
+ max_period = DIV_ROUND_UP_ULL((u64)U32_MAX * NSEC_PER_SEC + NSEC_PER_SEC / 2, pc->rate) - 1;
if (state->period > max_period)
return -EINVAL;
/* set period */
- period_cycles = DIV_ROUND_CLOSEST_ULL(state->period * rate, NSEC_PER_SEC);
+ period_cycles = DIV_ROUND_CLOSEST_ULL(state->period * pc->rate, NSEC_PER_SEC);
/* don't accept a period that is too small */
if (period_cycles < PERIOD_MIN)
@@ -103,7 +98,7 @@ static int bcm2835_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
writel(period_cycles, pc->base + PERIOD(pwm->hwpwm));
/* set duty cycle */
- val = DIV_ROUND_CLOSEST_ULL(state->duty_cycle * rate, NSEC_PER_SEC);
+ val = DIV_ROUND_CLOSEST_ULL(state->duty_cycle * pc->rate, NSEC_PER_SEC);
writel(val, pc->base + DUTY(pwm->hwpwm));
/* set polarity */
@@ -151,14 +146,29 @@ static int bcm2835_pwm_probe(struct platform_device *pdev)
return dev_err_probe(&pdev->dev, PTR_ERR(pc->clk),
"clock not found\n");
+ ret = clk_rate_exclusive_get(pc->clk);
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret,
+ "fail to get exclusive rate\n");
+
+ pc->rate = clk_get_rate(pc->clk);
+ if (!pc->rate) {
+ clk_rate_exclusive_put(pc->clk);
+ return dev_err_probe(&pdev->dev, -EINVAL,
+ "failed to get clock rate\n");
+ }
+
pc->chip.dev = &pdev->dev;
pc->chip.ops = &bcm2835_pwm_ops;
+ pc->chip.atomic = true;
pc->chip.npwm = 2;
ret = devm_pwmchip_add(&pdev->dev, &pc->chip);
- if (ret < 0)
+ if (ret < 0) {
+ clk_rate_exclusive_put(pc->clk);
return dev_err_probe(&pdev->dev, ret,
"failed to add pwmchip\n");
+ }
return 0;
}
@@ -167,6 +177,7 @@ static int bcm2835_pwm_suspend(struct device *dev)
{
struct bcm2835_pwm *pc = dev_get_drvdata(dev);
+ clk_rate_exclusive_put(pc->clk);
clk_disable_unprepare(pc->clk);
return 0;
--
2.43.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v6 3/4] pwm: bcm2835: allow pwm driver to be used in atomic context
2023-11-29 9:13 ` [PATCH v6 3/4] pwm: bcm2835: allow pwm driver to be used in atomic context Sean Young
@ 2023-11-29 17:47 ` Florian Fainelli
2023-12-08 16:22 ` Thierry Reding
1 sibling, 0 replies; 11+ messages in thread
From: Florian Fainelli @ 2023-11-29 17:47 UTC (permalink / raw)
To: Sean Young, linux-media, linux-pwm, Ivaylo Dimitrov,
Thierry Reding, Uwe Kleine-König,
Broadcom internal kernel review list, Ray Jui, Scott Branden
Cc: linux-rpi-kernel, linux-arm-kernel, linux-kernel
[-- Attachment #1.1: Type: text/plain, Size: 284 bytes --]
On 11/29/23 01:13, Sean Young wrote:
> clk_get_rate() may do a mutex lock. Fetch the clock rate once, and prevent
> rate changes using clk_rate_exclusive_get().
>
> Signed-off-by: Sean Young <sean@mess.org>
Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
--
Florian
[-- Attachment #1.2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4221 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v6 3/4] pwm: bcm2835: allow pwm driver to be used in atomic context
2023-11-29 9:13 ` [PATCH v6 3/4] pwm: bcm2835: allow pwm driver to be used in atomic context Sean Young
2023-11-29 17:47 ` Florian Fainelli
@ 2023-12-08 16:22 ` Thierry Reding
2023-12-08 17:01 ` Sean Young
1 sibling, 1 reply; 11+ messages in thread
From: Thierry Reding @ 2023-12-08 16:22 UTC (permalink / raw)
To: Sean Young
Cc: linux-media, linux-pwm, Ivaylo Dimitrov, Uwe Kleine-König,
Florian Fainelli, Broadcom internal kernel review list, Ray Jui,
Scott Branden, linux-rpi-kernel, linux-arm-kernel, linux-kernel
[-- Attachment #1.1: Type: text/plain, Size: 573 bytes --]
On Wed, Nov 29, 2023 at 09:13:36AM +0000, Sean Young wrote:
> clk_get_rate() may do a mutex lock. Fetch the clock rate once, and prevent
> rate changes using clk_rate_exclusive_get().
>
> Signed-off-by: Sean Young <sean@mess.org>
> ---
> drivers/pwm/pwm-bcm2835.c | 31 +++++++++++++++++++++----------
> 1 file changed, 21 insertions(+), 10 deletions(-)
s/pwm/PWM/ in the subject. Although, I guess you could just drop the
"PWM" altogether because the subject prefix implies that this is for
PWM. Also, please capitalize after the subject prefix.
Thierry
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v6 3/4] pwm: bcm2835: allow pwm driver to be used in atomic context
2023-12-08 16:22 ` Thierry Reding
@ 2023-12-08 17:01 ` Sean Young
2023-12-08 17:20 ` Uwe Kleine-König
0 siblings, 1 reply; 11+ messages in thread
From: Sean Young @ 2023-12-08 17:01 UTC (permalink / raw)
To: Thierry Reding
Cc: linux-media, linux-pwm, Ivaylo Dimitrov, Uwe Kleine-König,
Florian Fainelli, Broadcom internal kernel review list, Ray Jui,
Scott Branden, linux-rpi-kernel, linux-arm-kernel, linux-kernel
On Fri, Dec 08, 2023 at 05:22:52PM +0100, Thierry Reding wrote:
> On Wed, Nov 29, 2023 at 09:13:36AM +0000, Sean Young wrote:
> > clk_get_rate() may do a mutex lock. Fetch the clock rate once, and prevent
> > rate changes using clk_rate_exclusive_get().
> >
> > Signed-off-by: Sean Young <sean@mess.org>
> > ---
> > drivers/pwm/pwm-bcm2835.c | 31 +++++++++++++++++++++----------
> > 1 file changed, 21 insertions(+), 10 deletions(-)
>
> s/pwm/PWM/ in the subject. Although, I guess you could just drop the
> "PWM" altogether because the subject prefix implies that this is for
> PWM.
$ git log --no-merges --oneline drivers/pwm/ | sed -r 's/^\w* ([^:]+): .*/\1/' | sort | uniq -c
1197 pwm
1 PWM
...
The vast majority of the commits use pwm: as a prefix, only one uses PWM:.
In fact if you look across the tree almost everywhere lower case is used
for the prefix.
I'm just trying to follow convention.
Having said that, I think the prefix is totally redundant, it is clear from
the commit files what they are affecting. I am not sure what it really adds.
> Also, please capitalize after the subject prefix.
$ git log --no-merges --oneline drivers/pwm/ | grep -E '^\w* ([^:]+): [A-Z]' | wc -l
217
$ git log --no-merges --oneline drivers/pwm/ | grep -E '^\w* ([^:]+): [a-z]' | wc -l
1069
Although not as clear, convention seems to be lower case for commits. The
first line of a commit is not really a sentence, there is no trailing
period.
I am happy to oblige, just wanted to point this out. Sorry if this starts
a bikeshed discussion.
Thanks,
Sean
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v6 3/4] pwm: bcm2835: allow pwm driver to be used in atomic context
2023-12-08 17:01 ` Sean Young
@ 2023-12-08 17:20 ` Uwe Kleine-König
2023-12-09 9:11 ` Sean Young
2023-12-11 14:17 ` Thierry Reding
0 siblings, 2 replies; 11+ messages in thread
From: Uwe Kleine-König @ 2023-12-08 17:20 UTC (permalink / raw)
To: Sean Young
Cc: Thierry Reding, linux-media, linux-pwm, Ivaylo Dimitrov,
Florian Fainelli, Broadcom internal kernel review list, Ray Jui,
Scott Branden, linux-rpi-kernel, linux-arm-kernel, linux-kernel
[-- Attachment #1.1: Type: text/plain, Size: 2576 bytes --]
On Fri, Dec 08, 2023 at 05:01:26PM +0000, Sean Young wrote:
> On Fri, Dec 08, 2023 at 05:22:52PM +0100, Thierry Reding wrote:
> > On Wed, Nov 29, 2023 at 09:13:36AM +0000, Sean Young wrote:
> > > clk_get_rate() may do a mutex lock. Fetch the clock rate once, and prevent
> > > rate changes using clk_rate_exclusive_get().
> > >
> > > Signed-off-by: Sean Young <sean@mess.org>
> > > ---
> > > drivers/pwm/pwm-bcm2835.c | 31 +++++++++++++++++++++----------
> > > 1 file changed, 21 insertions(+), 10 deletions(-)
> >
> > s/pwm/PWM/ in the subject. Although, I guess you could just drop the
> > "PWM" altogether because the subject prefix implies that this is for
> > PWM.
>
> $ git log --no-merges --oneline drivers/pwm/ | sed -r 's/^\w* ([^:]+): .*/\1/' | sort | uniq -c
> 1197 pwm
> 1 PWM
> ...
>
> The vast majority of the commits use pwm: as a prefix, only one uses PWM:.
> In fact if you look across the tree almost everywhere lower case is used
> for the prefix.
Thierry doesn't want you to change the subject prefix, but only the
second "pwm" to make it read:
pwm: bcm2835: allow PWM driver to be used in atomic context
While I understand Thierry here, I'm fine with a lowercase pwm here,
too. In my book a PWM in all uppercase is the type of hardware and pwm
in all lowercase is the framework's name. If you use "PWM driver" or
"pwm driver" then doesn't matter much.
>
> I'm just trying to follow convention.
>
> Having said that, I think the prefix is totally redundant, it is clear from
> the commit files what they are affecting. I am not sure what it really adds.
>
> > Also, please capitalize after the subject prefix.
>
> $ git log --no-merges --oneline drivers/pwm/ | grep -E '^\w* ([^:]+): [A-Z]' | wc -l
> 217
> $ git log --no-merges --oneline drivers/pwm/ | grep -E '^\w* ([^:]+): [a-z]' | wc -l
> 1069
Your matching things like:
pwm: pwm-tiehrpwm: Add support for configuring polarity of PWM
with the second command. These are perfectly fine as pwm-tiehrpwm is the
driver name and so shouldn't be capitalized. With a bit more care here,
we get:
$ git log --no-merges --oneline drivers/pwm/ | grep -E '^.+: [a-z][^:]*$' | wc -l
114
$ git log --no-merges --oneline drivers/pwm/ | grep -E '^.+: [A-Z][^:]*$' | wc -l
1167
And the newest of the 114 with a small letter is from 2013.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v6 3/4] pwm: bcm2835: allow pwm driver to be used in atomic context
2023-12-08 17:20 ` Uwe Kleine-König
@ 2023-12-09 9:11 ` Sean Young
2023-12-11 14:17 ` Thierry Reding
1 sibling, 0 replies; 11+ messages in thread
From: Sean Young @ 2023-12-09 9:11 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Thierry Reding, linux-media, linux-pwm, Ivaylo Dimitrov,
Florian Fainelli, Broadcom internal kernel review list, Ray Jui,
Scott Branden, linux-rpi-kernel, linux-arm-kernel, linux-kernel
On Fri, Dec 08, 2023 at 06:20:40PM +0100, Uwe Kleine-König wrote:
> On Fri, Dec 08, 2023 at 05:01:26PM +0000, Sean Young wrote:
> > On Fri, Dec 08, 2023 at 05:22:52PM +0100, Thierry Reding wrote:
> > > On Wed, Nov 29, 2023 at 09:13:36AM +0000, Sean Young wrote:
> > > > clk_get_rate() may do a mutex lock. Fetch the clock rate once, and prevent
> > > > rate changes using clk_rate_exclusive_get().
> > > >
> > > > Signed-off-by: Sean Young <sean@mess.org>
> > > > ---
> > > > drivers/pwm/pwm-bcm2835.c | 31 +++++++++++++++++++++----------
> > > > 1 file changed, 21 insertions(+), 10 deletions(-)
> > >
> > > s/pwm/PWM/ in the subject. Although, I guess you could just drop the
> > > "PWM" altogether because the subject prefix implies that this is for
> > > PWM.
> >
> > $ git log --no-merges --oneline drivers/pwm/ | sed -r 's/^\w* ([^:]+): .*/\1/' | sort | uniq -c
> > 1197 pwm
> > 1 PWM
> > ...
> >
> > The vast majority of the commits use pwm: as a prefix, only one uses PWM:.
> > In fact if you look across the tree almost everywhere lower case is used
> > for the prefix.
>
> Thierry doesn't want you to change the subject prefix, but only the
> second "pwm" to make it read:
>
> pwm: bcm2835: allow PWM driver to be used in atomic context
Ah of course, my bad.
> While I understand Thierry here, I'm fine with a lowercase pwm here,
> too. In my book a PWM in all uppercase is the type of hardware and pwm
> in all lowercase is the framework's name. If you use "PWM driver" or
> "pwm driver" then doesn't matter much.
>
>
> >
> > I'm just trying to follow convention.
> >
> > Having said that, I think the prefix is totally redundant, it is clear from
> > the commit files what they are affecting. I am not sure what it really adds.
> >
> > > Also, please capitalize after the subject prefix.
> >
> > $ git log --no-merges --oneline drivers/pwm/ | grep -E '^\w* ([^:]+): [A-Z]' | wc -l
> > 217
> > $ git log --no-merges --oneline drivers/pwm/ | grep -E '^\w* ([^:]+): [a-z]' | wc -l
> > 1069
>
> Your matching things like:
>
> pwm: pwm-tiehrpwm: Add support for configuring polarity of PWM
>
> with the second command. These are perfectly fine as pwm-tiehrpwm is the
> driver name and so shouldn't be capitalized. With a bit more care here,
> we get:
>
> $ git log --no-merges --oneline drivers/pwm/ | grep -E '^.+: [a-z][^:]*$' | wc -l
> 114
> $ git log --no-merges --oneline drivers/pwm/ | grep -E '^.+: [A-Z][^:]*$' | wc -l
> 1167
>
> And the newest of the 114 with a small letter is from 2013.
Again, I stand corrected.
Thanks for pointing it out, I will fix in the next version.
Sean
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v6 1/4] pwm: rename pwm_apply_state() to pwm_apply_might_sleep()
2023-11-29 9:13 ` [PATCH v6 1/4] pwm: rename pwm_apply_state() to pwm_apply_might_sleep() Sean Young
@ 2023-12-09 13:57 ` Uwe Kleine-König
2023-12-11 8:30 ` Sean Young
2023-12-10 3:59 ` Dmitry Torokhov
1 sibling, 1 reply; 11+ messages in thread
From: Uwe Kleine-König @ 2023-12-09 13:57 UTC (permalink / raw)
To: Sean Young
Cc: linux-media, linux-pwm, Ivaylo Dimitrov, Thierry Reding,
Jonathan Corbet, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
Tvrtko Ursulin, David Airlie, Daniel Vetter,
Javier Martinez Canillas, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Jean Delvare, Guenter Roeck,
Support Opensource, Dmitry Torokhov, Pavel Machek, Lee Jones,
Mauro Carvalho Chehab, Hans de Goede, Ilpo Järvinen,
Mark Gross, Liam Girdwood, Mark Brown, Daniel Thompson,
Jingoo Han, Helge Deller, Jani Nikula, linux-doc, linux-kernel,
intel-gfx, dri-devel, linux-hwmon, linux-input, linux-leds,
platform-driver-x86, linux-arm-kernel, linux-fbdev
[-- Attachment #1.1: Type: text/plain, Size: 976 bytes --]
On Wed, Nov 29, 2023 at 09:13:34AM +0000, Sean Young wrote:
> In order to introduce a pwm api which can be used from atomic context,
> we will need two functions for applying pwm changes:
>
> int pwm_apply_might_sleep(struct pwm *, struct pwm_state *);
> int pwm_apply_atomic(struct pwm *, struct pwm_state *);
>
> This commit just deals with renaming pwm_apply_state(), a following
> commit will introduce the pwm_apply_atomic() function.
>
> Acked-by: Hans de Goede <hdegoede@redhat.com>
> Acked-by: Jani Nikula <jani.nikula@intel.com>
> Acked-by: Lee Jones <lee@kernel.org>
> Signed-off-by: Sean Young <sean@mess.org>
Not a in-detail-review, but I just noticed again, that we have
K: pwm_(config|apply_state|ops)
in MAINTAINERS. That one needs adaption, too.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v6 1/4] pwm: rename pwm_apply_state() to pwm_apply_might_sleep()
2023-11-29 9:13 ` [PATCH v6 1/4] pwm: rename pwm_apply_state() to pwm_apply_might_sleep() Sean Young
2023-12-09 13:57 ` Uwe Kleine-König
@ 2023-12-10 3:59 ` Dmitry Torokhov
1 sibling, 0 replies; 11+ messages in thread
From: Dmitry Torokhov @ 2023-12-10 3:59 UTC (permalink / raw)
To: Sean Young
Cc: linux-media, linux-pwm, Ivaylo Dimitrov, Thierry Reding,
Uwe Kleine-König, Jonathan Corbet, Jani Nikula,
Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin, David Airlie,
Daniel Vetter, Javier Martinez Canillas, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Jean Delvare, Guenter Roeck,
Support Opensource, Pavel Machek, Lee Jones,
Mauro Carvalho Chehab, Hans de Goede, Ilpo Järvinen,
Mark Gross, Liam Girdwood, Mark Brown, Daniel Thompson,
Jingoo Han, Helge Deller, Jani Nikula, linux-doc, linux-kernel,
intel-gfx, dri-devel, linux-hwmon, linux-input, linux-leds,
platform-driver-x86, linux-arm-kernel, linux-fbdev
On Wed, Nov 29, 2023 at 09:13:34AM +0000, Sean Young wrote:
> drivers/input/misc/da7280.c | 4 +--
> drivers/input/misc/pwm-beeper.c | 4 +--
> drivers/input/misc/pwm-vibra.c | 8 +++---
Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> # for input
Thanks.
--
Dmitry
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v6 1/4] pwm: rename pwm_apply_state() to pwm_apply_might_sleep()
2023-12-09 13:57 ` Uwe Kleine-König
@ 2023-12-11 8:30 ` Sean Young
0 siblings, 0 replies; 11+ messages in thread
From: Sean Young @ 2023-12-11 8:30 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: linux-media, linux-pwm, Ivaylo Dimitrov, Thierry Reding,
Jonathan Corbet, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
Tvrtko Ursulin, David Airlie, Daniel Vetter,
Javier Martinez Canillas, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Jean Delvare, Guenter Roeck,
Support Opensource, Dmitry Torokhov, Pavel Machek, Lee Jones,
Mauro Carvalho Chehab, Hans de Goede, Ilpo Järvinen,
Mark Gross, Liam Girdwood, Mark Brown, Daniel Thompson,
Jingoo Han, Helge Deller, Jani Nikula, linux-doc, linux-kernel,
intel-gfx, dri-devel, linux-hwmon, linux-input, linux-leds,
platform-driver-x86, linux-arm-kernel, linux-fbdev
On Sat, Dec 09, 2023 at 02:57:42PM +0100, Uwe Kleine-König wrote:
> On Wed, Nov 29, 2023 at 09:13:34AM +0000, Sean Young wrote:
> > In order to introduce a pwm api which can be used from atomic context,
> > we will need two functions for applying pwm changes:
> >
> > int pwm_apply_might_sleep(struct pwm *, struct pwm_state *);
> > int pwm_apply_atomic(struct pwm *, struct pwm_state *);
> >
> > This commit just deals with renaming pwm_apply_state(), a following
> > commit will introduce the pwm_apply_atomic() function.
> >
> > Acked-by: Hans de Goede <hdegoede@redhat.com>
> > Acked-by: Jani Nikula <jani.nikula@intel.com>
> > Acked-by: Lee Jones <lee@kernel.org>
> > Signed-off-by: Sean Young <sean@mess.org>
>
> Not a in-detail-review, but I just noticed again, that we have
>
> K: pwm_(config|apply_state|ops)
>
> in MAINTAINERS. That one needs adaption, too.
Fixed in v7.
Thanks,
Sean
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v6 3/4] pwm: bcm2835: allow pwm driver to be used in atomic context
2023-12-08 17:20 ` Uwe Kleine-König
2023-12-09 9:11 ` Sean Young
@ 2023-12-11 14:17 ` Thierry Reding
1 sibling, 0 replies; 11+ messages in thread
From: Thierry Reding @ 2023-12-11 14:17 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Sean Young, linux-media, linux-pwm, Ivaylo Dimitrov,
Florian Fainelli, Broadcom internal kernel review list, Ray Jui,
Scott Branden, linux-rpi-kernel, linux-arm-kernel, linux-kernel
[-- Attachment #1.1: Type: text/plain, Size: 1938 bytes --]
On Fri, Dec 08, 2023 at 06:20:40PM +0100, Uwe Kleine-König wrote:
> On Fri, Dec 08, 2023 at 05:01:26PM +0000, Sean Young wrote:
> > On Fri, Dec 08, 2023 at 05:22:52PM +0100, Thierry Reding wrote:
> > > On Wed, Nov 29, 2023 at 09:13:36AM +0000, Sean Young wrote:
> > > > clk_get_rate() may do a mutex lock. Fetch the clock rate once, and prevent
> > > > rate changes using clk_rate_exclusive_get().
> > > >
> > > > Signed-off-by: Sean Young <sean@mess.org>
> > > > ---
> > > > drivers/pwm/pwm-bcm2835.c | 31 +++++++++++++++++++++----------
> > > > 1 file changed, 21 insertions(+), 10 deletions(-)
> > >
> > > s/pwm/PWM/ in the subject. Although, I guess you could just drop the
> > > "PWM" altogether because the subject prefix implies that this is for
> > > PWM.
> >
> > $ git log --no-merges --oneline drivers/pwm/ | sed -r 's/^\w* ([^:]+): .*/\1/' | sort | uniq -c
> > 1197 pwm
> > 1 PWM
> > ...
> >
> > The vast majority of the commits use pwm: as a prefix, only one uses PWM:.
> > In fact if you look across the tree almost everywhere lower case is used
> > for the prefix.
>
> Thierry doesn't want you to change the subject prefix, but only the
> second "pwm" to make it read:
>
> pwm: bcm2835: allow PWM driver to be used in atomic context
>
> While I understand Thierry here, I'm fine with a lowercase pwm here,
> too. In my book a PWM in all uppercase is the type of hardware and pwm
> in all lowercase is the framework's name. If you use "PWM driver" or
> "pwm driver" then doesn't matter much.
I'm not fine with a lowercase "pwm" in what is clearly text. Text should
be grammatically correct and PWM being an abbreviation it should be all
caps. The framework name is also PWM, not pwm. We use the lowercase pwm
as prefix because it represents the directory where the subsystem lives
and we usually don't use capitalization in file and directory names.
Thierry
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-12-11 15:31 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <cover.1701248996.git.sean@mess.org>
2023-11-29 9:13 ` [PATCH v6 1/4] pwm: rename pwm_apply_state() to pwm_apply_might_sleep() Sean Young
2023-12-09 13:57 ` Uwe Kleine-König
2023-12-11 8:30 ` Sean Young
2023-12-10 3:59 ` Dmitry Torokhov
2023-11-29 9:13 ` [PATCH v6 3/4] pwm: bcm2835: allow pwm driver to be used in atomic context Sean Young
2023-11-29 17:47 ` Florian Fainelli
2023-12-08 16:22 ` Thierry Reding
2023-12-08 17:01 ` Sean Young
2023-12-08 17:20 ` Uwe Kleine-König
2023-12-09 9:11 ` Sean Young
2023-12-11 14:17 ` Thierry Reding
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).