[PATCH] backlight: mp3309c: Use pwm_apply_might_sleep()
pwm_apply_state() is deprecated since commit c748a6d77c06a ("pwm: Rename
pwm_apply_state() to pwm_apply_might_sleep()"). This is the final user
in the tree.
Signed-off-by: Sean Young
---
drivers/video/backlight/mp3309c.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/video/backlight/mp3309c.c
b/drivers/video/backlight/mp3309c.c
index 34d71259fac1d..b0d9aef6942b3 100644
--- a/drivers/video/backlight/mp3309c.c
+++ b/drivers/video/backlight/mp3309c.c
@@ -131,7 +131,7 @@ static int mp3309c_bl_update_status(struct backlight_device
*bl)
chip->pdata->levels[brightness],
chip->pdata->levels[chip->pdata->max_brightness]);
pwmstate.enabled = true;
- ret = pwm_apply_state(chip->pwmd, &pwmstate);
+ ret = pwm_apply_might_sleep(chip->pwmd, &pwmstate);
if (ret)
return ret;
@@ -393,7 +393,7 @@ static int mp3309c_probe(struct i2c_client *client)
chip->pdata->default_brightness,
chip->pdata->max_brightness);
pwmstate.enabled = true;
- ret = pwm_apply_state(chip->pwmd, &pwmstate);
+ ret = pwm_apply_might_sleep(chip->pwmd, &pwmstate);
if (ret)
return dev_err_probe(chip->dev, ret,
"error setting pwm device\n");
--
2.43.0
Re: (subset) linux-next: build failure after merge of the pwm tree
On Thu, Jan 04, 2024 at 05:02:41PM +0700, Bagas Sanjaya wrote:
> [also add Jingoo (additional backlight maintainer) and Linus]
>
> On Thu, Dec 21, 2023 at 07:34:57PM +0100, Thierry Reding wrote:
> > On Thu, Dec 21, 2023 at 12:58:01PM +, Lee Jones wrote:
> > > On Thu, 21 Dec 2023, Lee Jones wrote:
> > >
> > > > On Thu, 21 Dec 2023 16:58:05 +1100, Stephen Rothwell wrote:
> > > > > After merging the backlight tree, today's linux-next build (x86_64
> > > > > allmodconfig) failed like this:
> > > > >
> > > > > drivers/video/backlight/mp3309c.c: In function
> > > > > 'mp3309c_bl_update_status':
> > > > > drivers/video/backlight/mp3309c.c:134:23: error: implicit declaration
> > > > > of function 'pwm_apply_state'; did you mean 'pwm_apply_args'?
> > > > > [-Werror=implicit-function-declaration]
> > > > > 134 | ret = pwm_apply_state(chip->pwmd, &pwmstate);
> > > > > | ^~~
> > > > > | pwm_apply_args
> > > > >
> > > > > [...]
> > > >
> > > > Applied, thanks!
> > > >
> > > > [1/1] linux-next: build failure after merge of the pwm tree
> > > > commit: f7baa9ccef93ba1c36a8ecf58c2f4e86fb3181b9
> > >
> > > Actually it's:
> > >
> > > f7baa9ccef93b ("backlight: mp3309c: Rename pwm_apply_state() to
> > > pwm_apply_might_sleep()")
> > >
> > > But don't bank on the commit ID staying the same.
> >
> > This is likely going to break the build on your branch because
> > pwm_apply_might_sleep() is only available in the PWM tree right now. In
> > any case, I've now pushed a commit that adds pwm_apply_state() back as a
> > compatibility stub, so it should be okay for you to drop this if you
> > run into problems. It's always possible that somebody else wants to add
> > a new caller of pwm_apply_state() and in retrospect we should've
> > probably done this from the start, at least as a transitional measure
> > for one or two cycles.
> >
>
> Hi Lee and Thierry,
>
> I know that we're still on New Year vibes, so some things are not up to full
> steam for now; but since we're close to v6.7 release and v6.8 merge window,
> hence allow me to ask:
>
> Stephen Rothwell is still complaining about backlight tree build failure
> due to f7baa9ccef93b, yet it has not been fixed so far. Has the culprit
> been dropped/reverted as he requested? The worst case is the culprit slips
> through and become part of backlight PR and Linus will likely not happy
> with the build regression (maybe he had to fix by himself).
This should be fixed by 9a216587a03df, and on current linux-next I can't
reproduce the problem any more (x86_64 allmodconfig).
Thanks,
Sean
[PATCH v10 1/6] pwm: Rename pwm_apply_state() to pwm_apply_might_sleep()
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: Uwe Kleine-König
Acked-by: Guenter Roeck
Acked-by: Mark Brown
Acked-by: Dmitry Torokhov # for input
Acked-by: Hans de Goede
Acked-by: Jani Nikula
Acked-by: Lee Jones
Signed-off-by: Sean Young
---
Documentation/driver-api/pwm.rst | 8 +++---
MAINTAINERS | 2 +-
.../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 +--
22 files changed, 68 insertions(+), 68 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/MAINTAINERS b/MAINTAINERS
index 97f51d5ec1cf..c58480595220 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17576,7 +17576,7 @@ F: drivers/video/backlight/pwm_bl.c
F: include/dt-bindings/pwm/
F: include/linux/pwm.h
F: include/linux/pwm_backlight.h
-K: pwm_(config|apply_state|ops)
+K: pwm_(config|apply_might_sleep|ops)
PXA GPIO DRIVER
M: Robert Jarzmik
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->
[PATCH v9 1/6] pwm: Rename pwm_apply_state() to pwm_apply_might_sleep()
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: Uwe Kleine-König
Acked-by: Guenter Roeck
Acked-by: Mark Brown
Acked-by: Dmitry Torokhov # for input
Acked-by: Hans de Goede
Acked-by: Jani Nikula
Acked-by: Lee Jones
Signed-off-by: Sean Young
---
Documentation/driver-api/pwm.rst | 8 +++---
MAINTAINERS | 2 +-
.../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 +--
22 files changed, 68 insertions(+), 68 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/MAINTAINERS b/MAINTAINERS
index 97f51d5ec1cf..c58480595220 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17576,7 +17576,7 @@ F: drivers/video/backlight/pwm_bl.c
F: include/dt-bindings/pwm/
F: include/linux/pwm.h
F: include/linux/pwm_backlight.h
-K: pwm_(config|apply_state|ops)
+K: pwm_(config|apply_might_sleep|ops)
PXA GPIO DRIVER
M: Robert Jarzmik
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->
[PATCH v8 1/6] pwm: Rename pwm_apply_state() to pwm_apply_might_sleep()
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: Dmitry Torokhov # for input
Acked-by: Hans de Goede
Acked-by: Jani Nikula
Acked-by: Lee Jones
Signed-off-by: Sean Young
---
Documentation/driver-api/pwm.rst | 8 +++---
MAINTAINERS | 2 +-
.../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 +--
22 files changed, 68 insertions(+), 68 deletions(-)
diff --git a/Documentation/driver-api/pwm.rst b/Documentation/driver-api/pwm.rst
index bb264490a87a1..f1d8197c8c430 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/MAINTAINERS b/MAINTAINERS
index 97f51d5ec1cfd..c584805952209 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17576,7 +17576,7 @@ F: drivers/video/backlight/pwm_bl.c
F: include/dt-bindings/pwm/
F: include/linux/pwm.h
F: include/linux/pwm_backlight.h
-K: pwm_(config|apply_state|ops)
+K: pwm_(config|apply_might_sleep|ops)
PXA GPIO DRIVER
M: Robert Jarzmik
diff --git a/drivers/gpu/drm/i915/display/intel_backlight.c
b/drivers/gpu/drm/i915/display/intel_backlight.c
index 2e8f17c045222..ff9b9918b0a13 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(
Re: [PATCH v6 1/4] pwm: rename pwm_apply_state() to pwm_apply_might_sleep()
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 > > Acked-by: Jani Nikula > > Acked-by: Lee Jones > > Signed-off-by: Sean Young > > 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
[PATCH v7 1/4] pwm: Rename pwm_apply_state() to pwm_apply_might_sleep()
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: Dmitry Torokhov # for input
Acked-by: Hans de Goede
Acked-by: Jani Nikula
Acked-by: Lee Jones
Signed-off-by: Sean Young
---
Documentation/driver-api/pwm.rst | 8 +++---
MAINTAINERS | 2 +-
.../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 +--
22 files changed, 68 insertions(+), 68 deletions(-)
diff --git a/Documentation/driver-api/pwm.rst b/Documentation/driver-api/pwm.rst
index bb264490a87a1..f1d8197c8c430 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/MAINTAINERS b/MAINTAINERS
index 0d96db7e1a9e4..c2a9e0b5594e7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17585,7 +17585,7 @@ F: drivers/video/backlight/pwm_bl.c
F: include/dt-bindings/pwm/
F: include/linux/pwm.h
F: include/linux/pwm_backlight.h
-K: pwm_(config|apply_state|ops)
+K: pwm_(config|apply_might_sleep|ops)
PXA GPIO DRIVER
M: Robert Jarzmik
diff --git a/drivers/gpu/drm/i915/display/intel_backlight.c
b/drivers/gpu/drm/i915/display/intel_backlight.c
index 2e8f17c045222..ff9b9918b0a13 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(
[PATCH v6 1/4] pwm: rename pwm_apply_state() to pwm_apply_might_sleep()
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
Acked-by: Jani Nikula
Acked-by: Lee Jones
Signed-off-by: Sean Young
---
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_mi
Re: [PATCH v5 1/4] pwm: rename pwm_apply_state() to pwm_apply_cansleep()
On Fri, Nov 24, 2023 at 02:31:18PM +0100, Thierry Reding wrote: > On Sat, Nov 18, 2023 at 04:16:17PM +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_cansleep(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. > > Sorry, I still don't agree with that _cansleep suffix. I think it's the > wrong terminology. Just because something can sleep doesn't mean that it > ever will. "Might sleep" is much more accurate because it says exactly > what might happen and indicates what we're guarding against. Sorry, I forgot about this in the last round. I've renamed it _might_sleep in v6 which I'll post shortly. Sean
[PATCH v5 1/4] pwm: rename pwm_apply_state() to pwm_apply_cansleep()
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_cansleep(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
Acked-by: Jani Nikula
Signed-off-by: Sean Young
---
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 bb264490a87a1..5f6d1540dcd7e 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_cansleep(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_cansleep() 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_cansleep().
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_cansleep() 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 2e8f17c045222..cf516190cde8f 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_cansleep(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_cansleep(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_cansleep(panel->backlight.pwm, &a
[PATCH v4 1/4] pwm: rename pwm_apply_state() to pwm_apply_cansleep()
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_cansleep(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
Acked-by: Jani Nikula
Signed-off-by: Sean Young
---
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 | 6 ++--
drivers/video/fbdev/ssd1307fb.c | 2 +-
include/linux/pwm.h | 28 +--
21 files changed, 64 insertions(+), 64 deletions(-)
diff --git a/Documentation/driver-api/pwm.rst b/Documentation/driver-api/pwm.rst
index 3fdc95f7a1d1..ff1b73431b04 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_cansleep(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_cansleep() 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_cansleep().
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_cansleep() 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..cf516190cde8 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_cansleep(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_cansleep(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_cansleep(panel->backlight.pwm, &panel->back
Re: [PATCH v3 1/3] pwm: make it possible to apply pwm changes in atomic context
On Mon, Oct 23, 2023 at 02:34:17PM +0100, Daniel Thompson wrote: > On Sun, Oct 22, 2023 at 11:46:22AM +0100, Sean Young wrote: > > On Sat, Oct 21, 2023 at 11:08:22AM +0200, Hans de Goede wrote: > > > On 10/19/23 12:51, Uwe Kleine-König wrote: > > > > On Wed, Oct 18, 2023 at 03:57:48PM +0200, Hans de Goede wrote: > > > >> On 10/17/23 11:17, Sean Young wrote: > > > > I think it's very subjective if you consider this > > > > churn or not. > > > > > > I consider it churn because I don't think adding a postfix > > > for what is the default/expected behavior is a good idea > > > (with GPIOs not sleeping is the expected behavior). > > > > > > I agree that this is very subjective and very much goes > > > into the territory of bikeshedding. So please consider > > > the above my 2 cents on this and lets leave it at that. > > > > You have a valid point. Let's focus on having descriptive function names. > > For a couple of days I've been trying to resist the bikeshedding (esp. > given the changes to backlight are tiny) so I'll try to keep it as > brief as I can: > > 1. I dislike the do_it() and do_it_cansleep() pairing. It is >difficult to detect when a client driver calls do_it() by mistake. >In fact a latent bug of this nature can only be detected by runtime >testing with the small number of PWMs that do not support >configuration from an atomic context. > >In contrast do_it() and do_it_atomic()[*] means that although >incorrectly calling do_it() from an atomic context can be pretty >catastrophic it is also trivially detected (with any PWM driver) >simply by running with CONFIG_DEBUG_ATOMIC_SLEEP. > >No objections (beyond churn) to fully spelt out pairings such as >do_it_cansleep() and do_it_atomic()[*]! I must say I do like the look of this. Uwe, how do you feel about: pwm_apply_cansleep() and pwm_apply_atomic()? I know we've talked about pwm_apply_atomic in the past, however I think this this the best option I've seen so far. > 2. If there is an API rename can we make sure the patch contains no >other changes (e.g. don't introduce any new API in the same patch). >Seperating renames makes the patches easier to review! >It makes each one smaller and easier to review! Yes, this should have been separated out. Will fix for next version. Thanks, Sean
Re: [PATCH v3 1/3] pwm: make it possible to apply pwm changes in atomic context
Hi Hans, On Sat, Oct 21, 2023 at 11:08:22AM +0200, Hans de Goede wrote: > On 10/19/23 12:51, Uwe Kleine-König wrote: > > On Wed, Oct 18, 2023 at 03:57:48PM +0200, Hans de Goede wrote: > >> On 10/17/23 11:17, Sean Young wrote: > >>> Some drivers require sleeping, for example if the pwm device is connected > >>> over i2c. The pwm-ir-tx requires precise timing, and sleeping causes havoc > >>> with the generated IR signal when sleeping occurs. > >>> > >>> This patch makes it possible to use pwm when the driver does not sleep, > >>> by introducing the pwm_can_sleep() function. > >>> > >>> Signed-off-by: Sean Young > >> > >> I have no objection to this patch by itself, but it seems a bit > >> of unnecessary churn to change all current callers of pwm_apply_state() > >> to a new API. > > > > The idea is to improve the semantic of the function name, see > > https://lore.kernel.org/linux-pwm/20231013180449.mcdmklbsz2rly...@pengutronix.de > > for more context. > > Hmm, so the argument here is that the GPIO API has this, but GPIOs > generally speaking can be set atomically, so there not being able > to set it atomically is special. > > OTOH we have many many many other kernel functions which may sleep > and we don't all postfix them with _can_sleep. > > And for PWM controllers pwm_apply_state is IMHO sorta expected to > sleep. Many of these are attached over I2C so things will sleep, > others have a handshake to wait for the current dutycycle to > end before you can apply a second change on top of an earlier > change during the current dutycycle which often also involves > sleeping. > > So the natural/expeected thing for pwm_apply_state() is to sleep > and thus it does not need a postfix for this IMHO. Most pwm drivers look like they can be made to work in atomic context, I think. Like you say this is not the case for all of them. Whatever we choose to be the default for pwm_apply_state(), we should have a clear function name for the alternative. This is essentially why pam_apply_cansleep() was picked. The alternative to pwm_apply_cansleep() is to have a function name which implies it can be used from atomic context. However, pwm_apply_atomic() is not great because the "atomic" could be confused with the PWM atomic API, not the kernel process/atomic context. So what should the non-sleeping function be called then? - pwm_apply_cannotsleep() - pwm_apply_nosleep() - pwm_apply_nonsleeping() - pwm_apply_atomic_context() > > I think it's very subjective if you consider this > > churn or not. > > I consider it churn because I don't think adding a postfix > for what is the default/expected behavior is a good idea > (with GPIOs not sleeping is the expected behavior). > > I agree that this is very subjective and very much goes > into the territory of bikeshedding. So please consider > the above my 2 cents on this and lets leave it at that. You have a valid point. Let's focus on having descriptive function names. > > While it's nice to have every caller converted in a single > > step, I'd go for > > > > #define pwm_apply_state(pwm, state) pwm_apply_cansleep(pwm, state) > > > > , keep that macro for a while and convert all users step by step. This > > way we don't needlessly break oot code and the changes to convert to the > > new API can go via their usual trees without time pressure. > > I don't think there are enough users of pwm_apply_state() to warrant > such an exercise. > > So if people want to move ahead with the _can_sleep postfix addition > (still not a fan) here is my acked-by for the drivers/platform/x86 > changes, for merging this through the PWM tree in a single commit: > > Acked-by: Hans de Goede Thanks, Sean
[PATCH v3 1/3] pwm: make it possible to apply pwm changes in atomic context
Some drivers require sleeping, for example if the pwm device is connected
over i2c. The pwm-ir-tx requires precise timing, and sleeping causes havoc
with the generated IR signal when sleeping occurs.
This patch makes it possible to use pwm when the driver does not sleep,
by introducing the pwm_can_sleep() function.
Signed-off-by: Sean Young
---
Documentation/driver-api/pwm.rst | 16 +++-
.../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| 75 ++-
drivers/pwm/pwm-renesas-tpu.c | 1 -
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 | 6 +-
drivers/video/fbdev/ssd1307fb.c | 2 +-
include/linux/pwm.h | 57 ++
22 files changed, 147 insertions(+), 76 deletions(-)
diff --git a/Documentation/driver-api/pwm.rst b/Documentation/driver-api/pwm.rst
index 3fdc95f7a1d15..a2fb5f8f6e1f8 100644
--- a/Documentation/driver-api/pwm.rst
+++ b/Documentation/driver-api/pwm.rst
@@ -41,7 +41,15 @@ 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_cansleep(struct pwm_device *pwm, struct pwm_state *state);
+
+If the PWM support atomic mode, which can be determined with::
+
+bool pwm_is_atomic(struct pwm_device *pwm);
+
+Then the PWM can be configured with::
+
+ int pwm_apply(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 +65,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_cansleep() 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_cansleep().
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_cansleep() 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 2e8f17c045222..cf516190cde8f 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_cansleep(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_cansleep(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_stat
Re: [PATCH 000/141] Fix fall-through warnings for Clang
On Mon, Nov 23, 2020 at 07:58:06AM -0800, James Bottomley wrote: > On Mon, 2020-11-23 at 15:19 +0100, Miguel Ojeda wrote: > > On Sun, Nov 22, 2020 at 11:36 PM James Bottomley > > wrote: > > > It's not about the risk of the changes it's about the cost of > > > implementing them. Even if you discount the producer time (which > > > someone gets to pay for, and if I were the engineering manager, I'd > > > be unhappy about), the review/merge/rework time is pretty > > > significant in exchange for six minor bug fixes. Fine, when a new > > > compiler warning comes along it's certainly reasonable to see if we > > > can benefit from it and the fact that the compiler people think > > > it's worthwhile is enough evidence to assume this initially. But > > > at some point you have to ask whether that assumption is supported > > > by the evidence we've accumulated over the time we've been using > > > it. And if the evidence doesn't support it perhaps it is time to > > > stop the experiment. > > > > Maintainers routinely review 1-line trivial patches, not to mention > > internal API changes, etc. > > We're also complaining about the inability to recruit maintainers: > > https://www.theregister.com/2020/06/30/hard_to_find_linux_maintainers_says_torvalds/ > > And burn out: > > http://antirez.com/news/129 > > The whole crux of your argument seems to be maintainers' time isn't > important so we should accept all trivial patches ... I'm pushing back > on that assumption in two places, firstly the valulessness of the time > and secondly that all trivial patches are valuable. You're assuming burn out or recruitment problems is due to patch workload or too many "trivial" patches. In my experience, "other maintainers" is by far the biggest cause of burn out for my kernel maintenance work. Certainly arguing with a maintainer about some obviously-correct patch series must be a good example of this. Sean ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH 00/12] i2c: replace i2c_new_probed_device with an ERR_PTR variant
On Wed, Nov 06, 2019 at 10:50:18AM +0100, Wolfram Sang wrote: > From: Wolfram Sang > > In the on-going mission to let i2c_new_* calls return an ERR_PTR instead > of NULL, here is a series converting i2c_new_probed_device(). A new > function called i2c_new_scanned_device() is introduced with the new > retval, but for now, a compatibility helper is provided until all users > are converted. The rest of the patches convert all current in-tree > users. > > Note that these patches are RFC because I want feedback on the approach > and hopefully collect acks on the driver conversions. If all goes well, > I'll apply the first two patches for the next merge window. Then, once > this dependency is upstream, I'll resend this series with all issues > fixed and acks collected. The patches to drivers/media/pci/* are all IR related which have touched on/read over the years. So, for those: Acked-by: Sean Young > > Core changes tested on a Renesas Salvator-XS board (R-Car M3-N), driver > patches build tested by me and buildbot. > > Wolfram Sang (12): > i2c: replace i2c_new_probed_device with an ERR_PTR variant > i2c: icy: convert to i2c_new_scanned_device > macintosh: convert to i2c_new_scanned_device > platform: chrome: convert to i2c_new_scanned_device > video: fbdev: matrox: convert to i2c_new_scanned_device > input: mouse: convert to i2c_new_scanned_device > media: pci: cx23885: convert to i2c_new_scanned_device > media: pci: cx88: convert to i2c_new_scanned_device > media: pci: bt8xx: convert to i2c_new_scanned_device > media: pci: cx18: convert to i2c_new_scanned_device > media: pci: ivtv: convert to i2c_new_scanned_device > media: v4l2-core: convert to i2c_new_scanned_device > > Documentation/i2c/instantiating-devices.rst | 10 - > Documentation/i2c/writing-clients.rst | 8 +++ > drivers/i2c/busses/i2c-icy.c| 8 +++ > drivers/i2c/i2c-core-base.c | 25 - > drivers/input/mouse/psmouse-smbus.c | 8 --- > drivers/macintosh/therm_windtunnel.c| 4 ++-- > drivers/media/pci/bt8xx/bttv-input.c| 6 ++--- > drivers/media/pci/cx18/cx18-i2c.c | 2 +- > drivers/media/pci/cx23885/cx23885-i2c.c | 4 ++-- > drivers/media/pci/cx88/cx88-input.c | 2 +- > drivers/media/pci/ivtv/ivtv-i2c.c | 6 ++--- > drivers/media/pci/ivtv/ivtv-i2c.h | 2 +- > drivers/media/v4l2-core/v4l2-i2c.c | 10 - > drivers/platform/chrome/chromeos_laptop.c | 18 --- > drivers/video/fbdev/matrox/i2c-matroxfb.c | 4 ++-- > include/linux/i2c.h | 12 +++--- > 16 files changed, 76 insertions(+), 53 deletions(-) > > -- > 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 05/10] pwm: add PWM mode to pwm_config()
On Thu, Feb 22, 2018 at 02:01:16PM +0200, Claudiu Beznea wrote:
> Add PWM mode to pwm_config() function. The drivers which uses pwm_config()
> were adapted to this change.
>
> Signed-off-by: Claudiu Beznea
> ---
-snip-
> diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
> index 49265f02e772..a971b02ea021 100644
> --- a/drivers/media/rc/ir-rx51.c
> +++ b/drivers/media/rc/ir-rx51.c
> @@ -55,10 +55,13 @@ static int init_timing_params(struct ir_rx51 *ir_rx51)
> {
> struct pwm_device *pwm = ir_rx51->pwm;
> int duty, period = DIV_ROUND_CLOSEST(NSEC_PER_SEC, ir_rx51->freq);
> + struct pwm_caps caps = { };
>
> duty = DIV_ROUND_CLOSEST(ir_rx51->duty_cycle * period, 100);
>
> - pwm_config(pwm, duty, period);
> + pwm_get_caps(pwm->chip, pwm, &caps);
> +
> + pwm_config(pwm, duty, period, BIT(ffs(caps.modes) - 1));
>
> return 0;
> }
> diff --git a/drivers/media/rc/pwm-ir-tx.c b/drivers/media/rc/pwm-ir-tx.c
> index 27d0f5837a76..c630e1b450a3 100644
> --- a/drivers/media/rc/pwm-ir-tx.c
> +++ b/drivers/media/rc/pwm-ir-tx.c
> @@ -61,6 +61,7 @@ static int pwm_ir_tx(struct rc_dev *dev, unsigned int
> *txbuf,
> {
> struct pwm_ir *pwm_ir = dev->priv;
> struct pwm_device *pwm = pwm_ir->pwm;
> + struct pwm_caps caps = { };
> int i, duty, period;
> ktime_t edge;
> long delta;
> @@ -68,7 +69,9 @@ static int pwm_ir_tx(struct rc_dev *dev, unsigned int
> *txbuf,
> period = DIV_ROUND_CLOSEST(NSEC_PER_SEC, pwm_ir->carrier);
> duty = DIV_ROUND_CLOSEST(pwm_ir->duty_cycle * period, 100);
>
> - pwm_config(pwm, duty, period);
> + pwm_get_caps(pwm->chip, pwm, &caps);
> +
> + pwm_config(pwm, duty, period, BIT(ffs(caps.modes) - 1));
>
> edge = ktime_get();
>
The two PWM rc-core drivers need PWM_MODE(NORMAL), not the first available
mode that the device supports. If mode normal is not supported, then probe
should fail.
Sean
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] drm/bridge/sii8620: add remote control support
Hi, On Thu, Sep 21, 2017 at 12:46:04PM +0100, Sean Young wrote: > On Mon, Sep 18, 2017 at 04:37:52PM +0200, Hans Verkuil wrote: > > On 09/18/2017 04:15 PM, Maciej Purski wrote: > > > Hi Hans, > > > some time ago in reply to your email I described what messages does > > > the MHL driver receive and at what time intervals. > > > Regarding that information, do you think that a similar solution as > > > in [1] is required? Would it be OK, if I just set REP_DELAY and REP_PERIOD > > > to values, which I presented in my last email? > > > > Based on the timings you measured I would say that there is a 99% chance > > that MHL > > uses exactly the same "Press and Hold" mechanism as CEC. Since you already > > specify > > RC_PROTO_BIT_CEC in the driver, it will set REP_DELAY and REP_PERIOD to the > > right > > values automatically. > > > > You will have to implement the same code as in cec-adap.c for the key press > > handling, > > though. It's a bit tricky to get it right. > > > > Since you will have to do the same thing as I do in CEC, I wonder if it > > would make > > more sense to move this code to the RC core. Basically it ensures that > > repeat mode > > doesn't turn on until you get two identical key presses 550ms or less > > apart. This > > is independent of REP_DELAY which can be changed by userspace. > > > > Sean, what do you think? > > Yes, this makes sense. In fact, since IR protocols have different repeat > times, I would like to make this protocol dependant anyway. > > To be honest I find REP_DELAY of 500ms too long and REP_PERIOD of 125ms > too short; IOW it takes too long for a key to start repeating, and once > it starts repeating it is very hard to control. If I try to increase the > volume with my remote it first hardly changes at all and then after 500ms > the volume shoots up far too quickly, same thing when navigating menus. > > CEC dictates a delay period of 550ms, which is not great for the user IMO. > > Anyway I'll have a look at this over the weekend. I have started on this, but I haven't gotten it in a state where I'm happy to submit it. I hope to have it ready for v4.15; in the mean time, there is no reason to block this patch on this. Sean ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] drm/bridge/sii8620: add remote control support
On Mon, Sep 18, 2017 at 04:37:52PM +0200, Hans Verkuil wrote: > On 09/18/2017 04:15 PM, Maciej Purski wrote: > > Hi Hans, > > some time ago in reply to your email I described what messages does > > the MHL driver receive and at what time intervals. > > Regarding that information, do you think that a similar solution as > > in [1] is required? Would it be OK, if I just set REP_DELAY and REP_PERIOD > > to values, which I presented in my last email? > > Based on the timings you measured I would say that there is a 99% chance that > MHL > uses exactly the same "Press and Hold" mechanism as CEC. Since you already > specify > RC_PROTO_BIT_CEC in the driver, it will set REP_DELAY and REP_PERIOD to the > right > values automatically. > > You will have to implement the same code as in cec-adap.c for the key press > handling, > though. It's a bit tricky to get it right. > > Since you will have to do the same thing as I do in CEC, I wonder if it would > make > more sense to move this code to the RC core. Basically it ensures that repeat > mode > doesn't turn on until you get two identical key presses 550ms or less apart. > This > is independent of REP_DELAY which can be changed by userspace. > > Sean, what do you think? Yes, this makes sense. In fact, since IR protocols have different repeat times, I would like to make this protocol dependant anyway. To be honest I find REP_DELAY of 500ms too long and REP_PERIOD of 125ms too short; IOW it takes too long for a key to start repeating, and once it starts repeating it is very hard to control. If I try to increase the volume with my remote it first hardly changes at all and then after 500ms the volume shoots up far too quickly, same thing when navigating menus. CEC dictates a delay period of 550ms, which is not great for the user IMO. Anyway I'll have a look at this over the weekend. Sean ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] drm/bridge/sii8620: add remote control support
On Thu, Aug 24, 2017 at 10:58:07AM +0200, Maciej Purski wrote:
> MHL specification defines Remote Control Protocol(RCP) to
> send input events between MHL devices.
> The driver now recognizes RCP messages and reacts to them
> by reporting key events to input subsystem, allowing
> a user to control a device using TV remote control.
>
> Signed-off-by: Maciej Purski
> ---
>
> Changes in v2:
> - use RC subsystem (including CEC keymap)
> - RC device initialized in attach drm_bridge callback and
> removed in detach callback. This is necessary, because RC_CORE,
> which is needed during rc_dev init, is loaded after sii8620.
> DRM bridge is binded later which solves the problem.
> - add RC_CORE dependency
>
> Changes in v3:
> - fix error handling in init_rcp and in attach callback
>
> Changes in v4:
> - usage of rc-core API compatible with upcoming changes
> - fix error handling in init_rcp
> - fix commit message
Looks good.
Acked-by: Sean Young
> ---
> drivers/gpu/drm/bridge/Kconfig | 2 +-
> drivers/gpu/drm/bridge/sil-sii8620.c | 96
> ++--
> include/drm/bridge/mhl.h | 4 ++
> 3 files changed, 96 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> index adf9ae0..6ef901c 100644
> --- a/drivers/gpu/drm/bridge/Kconfig
> +++ b/drivers/gpu/drm/bridge/Kconfig
> @@ -71,7 +71,7 @@ config DRM_PARADE_PS8622
>
> config DRM_SIL_SII8620
> tristate "Silicon Image SII8620 HDMI/MHL bridge"
> - depends on OF
> + depends on OF && RC_CORE
> select DRM_KMS_HELPER
> help
> Silicon Image SII8620 HDMI/MHL bridge chip driver.
> diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c
> b/drivers/gpu/drm/bridge/sil-sii8620.c
> index 2d51a22..ecb26c4 100644
> --- a/drivers/gpu/drm/bridge/sil-sii8620.c
> +++ b/drivers/gpu/drm/bridge/sil-sii8620.c
> @@ -28,6 +28,8 @@
> #include
> #include
>
> +#include
> +
> #include "sil-sii8620.h"
>
> #define SII8620_BURST_BUF_LEN 288
> @@ -58,6 +60,7 @@ enum sii8620_mt_state {
> struct sii8620 {
> struct drm_bridge bridge;
> struct device *dev;
> + struct rc_dev *rc_dev;
> struct clk *clk_xtal;
> struct gpio_desc *gpio_reset;
> struct gpio_desc *gpio_int;
> @@ -431,6 +434,16 @@ static void sii8620_mt_rap(struct sii8620 *ctx, u8 code)
> sii8620_mt_msc_msg(ctx, MHL_MSC_MSG_RAP, code);
> }
>
> +static void sii8620_mt_rcpk(struct sii8620 *ctx, u8 code)
> +{
> + sii8620_mt_msc_msg(ctx, MHL_MSC_MSG_RCPK, code);
> +}
> +
> +static void sii8620_mt_rcpe(struct sii8620 *ctx, u8 code)
> +{
> + sii8620_mt_msc_msg(ctx, MHL_MSC_MSG_RCPE, code);
> +}
> +
> static void sii8620_mt_read_devcap_send(struct sii8620 *ctx,
> struct sii8620_mt_msg *msg)
> {
> @@ -1753,6 +1766,25 @@ static void sii8620_send_features(struct sii8620 *ctx)
> sii8620_write_buf(ctx, REG_MDT_XMIT_WRITE_PORT, buf, ARRAY_SIZE(buf));
> }
>
> +static bool sii8620_rcp_consume(struct sii8620 *ctx, u8 scancode)
> +{
> + bool pressed = !(scancode & MHL_RCP_KEY_RELEASED_MASK);
> +
> + scancode &= MHL_RCP_KEY_ID_MASK;
> +
> + if (!ctx->rc_dev) {
> + dev_dbg(ctx->dev, "RCP input device not initialized\n");
> + return false;
> + }
> +
> + if (pressed)
> + rc_keydown(ctx->rc_dev, RC_PROTO_CEC, scancode, 0);
> + else
> + rc_keyup(ctx->rc_dev);
> +
> + return true;
> +}
> +
> static void sii8620_msc_mr_set_int(struct sii8620 *ctx)
> {
> u8 ints[MHL_INT_SIZE];
> @@ -1804,19 +1836,25 @@ static void sii8620_msc_mt_done(struct sii8620 *ctx)
>
> static void sii8620_msc_mr_msc_msg(struct sii8620 *ctx)
> {
> - struct sii8620_mt_msg *msg = sii8620_msc_msg_first(ctx);
> + struct sii8620_mt_msg *msg;
> u8 buf[2];
>
> - if (!msg)
> - return;
> -
> sii8620_read_buf(ctx, REG_MSC_MR_MSC_MSG_RCVD_1ST_DATA, buf, 2);
>
> switch (buf[0]) {
> case MHL_MSC_MSG_RAPK:
> + msg = sii8620_msc_msg_first(ctx);
> + if (!msg)
> + return;
> msg->ret = buf[1];
> ctx->mt_state = MT_STATE_DONE;
> break;
> + case MHL_MSC_MSG_RCP:
> + if (!sii8620_rcp_consume(ctx, buf[1]))
> + sii8620_mt_rcpe(ctx,
> + MHL_RCPE_STATUS_INEFFECTIVE_KEY_CODE);
> + sii8620_mt_rcpk
Re: [PATCH v3] drm/bridge/sii8620: add remote control support
Hi,
Some review comments below.
On Wed, Aug 23, 2017 at 09:03:18AM +0200, Hans Verkuil wrote:
> Maciej,
>
> I'm cross-posting this to linux-media and the rc maintainer Sean Young.
>
> Note that various RC defines have been renamed in the upcoming 4.14. It might
> be
> better to base your code on top of https://git.linuxtv.org/media_tree.git/
> which has those patches merged.
>
> Regards,
>
> Hans
>
> On 08/21/2017 11:33 AM, Maciej Purski wrote:
> > MHL specification defines Remote Control Protocol(RCP) to
> > send input events between MHL devices.
> > The driver now recognizes RCP messages and reacts to them
> > by reporting key events to input subsystem, allowing
> > a user to control a device using TV remote control.
> >
> > Changes in v2:
> > - use RC subsystem (including CEC keymap)
> > - RC device initialized in attach drm_bridge callback and
> > removed in detach callback. This is necessary, because RC_CORE,
> > which is needed during rc_dev init, is loaded after sii8620.
> > DRM bridge is binded later which solves the problem.
> > - add RC_CORE dependency
> >
> > Changes in v3:
> > - fix error handling in init_rcp and in attach callback
> >
> > Signed-off-by: Maciej Purski
> > ---
> > drivers/gpu/drm/bridge/Kconfig | 2 +-
> > drivers/gpu/drm/bridge/sil-sii8620.c | 101
> > +--
> > include/drm/bridge/mhl.h | 4 ++
> > 3 files changed, 101 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> > index adf9ae0..6ef901c 100644
> > --- a/drivers/gpu/drm/bridge/Kconfig
> > +++ b/drivers/gpu/drm/bridge/Kconfig
> > @@ -71,7 +71,7 @@ config DRM_PARADE_PS8622
> >
> > config DRM_SIL_SII8620
> > tristate "Silicon Image SII8620 HDMI/MHL bridge"
> > - depends on OF
> > + depends on OF && RC_CORE
> > select DRM_KMS_HELPER
> > help
> > Silicon Image SII8620 HDMI/MHL bridge chip driver.
> > diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c
> > b/drivers/gpu/drm/bridge/sil-sii8620.c
> > index 2d51a22..e072bca 100644
> > --- a/drivers/gpu/drm/bridge/sil-sii8620.c
> > +++ b/drivers/gpu/drm/bridge/sil-sii8620.c
> > @@ -28,6 +28,8 @@
> > #include
> > #include
> >
> > +#include
> > +
> > #include "sil-sii8620.h"
> >
> > #define SII8620_BURST_BUF_LEN 288
> > @@ -58,6 +60,7 @@ enum sii8620_mt_state {
> > struct sii8620 {
> > struct drm_bridge bridge;
> > struct device *dev;
> > + struct rc_dev *rc_dev;
> > struct clk *clk_xtal;
> > struct gpio_desc *gpio_reset;
> > struct gpio_desc *gpio_int;
> > @@ -431,6 +434,16 @@ static void sii8620_mt_rap(struct sii8620 *ctx, u8
> > code)
> > sii8620_mt_msc_msg(ctx, MHL_MSC_MSG_RAP, code);
> > }
> >
> > +static void sii8620_mt_rcpk(struct sii8620 *ctx, u8 code)
> > +{
> > + sii8620_mt_msc_msg(ctx, MHL_MSC_MSG_RCPK, code);
> > +}
> > +
> > +static void sii8620_mt_rcpe(struct sii8620 *ctx, u8 code)
> > +{
> > + sii8620_mt_msc_msg(ctx, MHL_MSC_MSG_RCPE, code);
> > +}
> > +
> > static void sii8620_mt_read_devcap_send(struct sii8620 *ctx,
> > struct sii8620_mt_msg *msg)
> > {
> > @@ -1753,6 +1766,25 @@ static void sii8620_send_features(struct sii8620
> > *ctx)
> > sii8620_write_buf(ctx, REG_MDT_XMIT_WRITE_PORT, buf, ARRAY_SIZE(buf));
> > }
> >
> > +static bool sii8620_rcp_consume(struct sii8620 *ctx, u8 scancode)
> > +{
> > + bool pressed = !(scancode & MHL_RCP_KEY_RELEASED_MASK);
> > +
> > + scancode &= MHL_RCP_KEY_ID_MASK;
> > +
> > + if (!ctx->rc_dev) {
> > + dev_dbg(ctx->dev, "RCP input device not initialized\n");
> > + return false;
> > + }
> > +
> > + if (pressed)
> > + rc_keydown(ctx->rc_dev, RC_TYPE_CEC, scancode, 0);
> > + else
> > + rc_keyup(ctx->rc_dev);
> > +
> > + return true;
> > +}
> > +
> > static void sii8620_msc_mr_set_int(struct sii8620 *ctx)
> > {
> > u8 ints[MHL_INT_SIZE];
> > @@ -1804,19 +1836,25 @@ static void sii8620_msc_mt_done(struct sii8620 *ctx)
> >
> > static void sii8620_msc_mr_msc_msg(struct sii8620 *ctx)
> > {
> > - struct sii8620_mt_msg *msg = sii8620_msc_msg_first(ctx);
> > +
Re: [PATCH] drm/bridge/sii8620: add remote control support
Hi Maciej,
On Thu, Aug 03, 2017 at 10:28:04AM +0200, Hans Verkuil wrote:
> Hi Maciej,
>
> Unfortunately I do not have the MHL spec, but I was wondering what the
> relationship between RCP and CEC is. CEC has remote control support as
> well, so is RCP that subset of the CEC specification or is it completely
> separate?
>
> I'm CC-ing Sean Young and the linux-media mailinglist as well since Sean
> maintains the rc subsystem. Which you probably should use, but I'm not the
> expert on that.
Yes, I agree. Using rc core the keymap can be changed from user space, and
is defined elsewhere.
In include/media/rc-map.h you'll have to define a new protocol (unless the
protocol is CEC).
In drivers/media/rc/keymaps/ a new keymap should be defined.
Use devm_rc_allocate_device(RC_DRIVER_SCANCODE) to create the device; then
you'll need to at least set these members:
input_phys = DRIVER_NAME "/input0";
input_id.bustype = BUS_HOST;
map_name = RC_MAP_RCP;
allowed_protocols = RC_BIT_RCP;
driver_name = DRIVER_NAME;
device_name = DEVICE_NAME;
Then register with devm_rc_register_device(), and report keys with
rc_keydown() and rc_keyup().
> On 08/03/17 09:44, Maciej Purski wrote:
> > MHL specification defines Remote Control Protocol(RCP) to
> > send input events between MHL devices.
> > The driver now recognizes RCP messages and reacts to them
> > by reporting key events to input subsystem, allowing
> > a user to control a device using TV remote control.
> >
> > Signed-off-by: Maciej Purski
> > ---
> > drivers/gpu/drm/bridge/sil-sii8620.c | 188
> > ++-
> > include/drm/bridge/mhl.h | 4 +
> > 2 files changed, 187 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c
> > b/drivers/gpu/drm/bridge/sil-sii8620.c
> > index 2d51a22..7e75f2f 100644
> > --- a/drivers/gpu/drm/bridge/sil-sii8620.c
> > +++ b/drivers/gpu/drm/bridge/sil-sii8620.c
> > @@ -19,6 +19,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> > #include
> > @@ -58,6 +59,7 @@ enum sii8620_mt_state {
> > struct sii8620 {
> > struct drm_bridge bridge;
> > struct device *dev;
> > + struct input_dev *rcp_input_dev;
> > struct clk *clk_xtal;
> > struct gpio_desc *gpio_reset;
> > struct gpio_desc *gpio_int;
> > @@ -106,6 +108,82 @@ struct sii8620_mt_msg {
> > sii8620_cb continuation;
> > };
> >
> > +static struct {
> > + u16 key;
> > + u16 extra_key;
> > + bool autorepeat;
Again let the input layer handle autorepeat, see REP_DELAY and REP_PERIOD.
> > +} rcp_keymap[] = {
> > + [0x00] = { KEY_SELECT },
> > + [0x01] = { KEY_UP, 0, true },
> > + [0x02] = { KEY_DOWN, 0, true },
> > + [0x03] = { KEY_LEFT, 0, true },
> > + [0x04] = { KEY_RIGHT, 0, true },
> > +
> > + [0x05] = { KEY_RIGHT, KEY_UP, true },
> > + [0x06] = { KEY_RIGHT, KEY_DOWN, true },
> > + [0x07] = { KEY_LEFT, KEY_UP, true },
> > + [0x08] = { KEY_LEFT, KEY_DOWN, true },
This should be replaced with KEY_RIGHT_UP, RC_RIGHT_DOWN, etc.
> > +
> > + [0x09] = { KEY_MENU },
> > + [0x0A] = { KEY_UNKNOWN },
> > + [0x0B] = { KEY_UNKNOWN },
> > + [0x0C] = { KEY_BOOKMARKS },
> > + [0x0D] = { KEY_EXIT },
> > +
> > + [0x20] = { KEY_NUMERIC_0 },
> > + [0x21] = { KEY_NUMERIC_1 },
> > + [0x22] = { KEY_NUMERIC_2 },
> > + [0x23] = { KEY_NUMERIC_3 },
> > + [0x24] = { KEY_NUMERIC_4 },
> > + [0x25] = { KEY_NUMERIC_5 },
> > + [0x26] = { KEY_NUMERIC_6 },
> > + [0x27] = { KEY_NUMERIC_7 },
> > + [0x28] = { KEY_NUMERIC_8 },
> > + [0x29] = { KEY_NUMERIC_9 },
> > +
> > + [0x2A] = { KEY_DOT },
> > + [0x2B] = { KEY_ENTER },
> > + [0x2C] = { KEY_CLEAR },
> > +
> > + [0x30] = { KEY_CHANNELUP, 0, true },
> > + [0x31] = { KEY_CHANNELDOWN, 0, true },
> > +
> > + [0x33] = { KEY_SOUND },
> > + [0x35] = { KEY_PROGRAM }, /* Show Information */
> > +
> > + [0x37] = { KEY_PAGEUP, 0, true },
> > + [0x38] = { KEY_PAGEDOWN, 0, true },
> > +
> > + [0x41] = { KEY_VOLUMEUP, 0, true },
> > + [0x42] = { KEY_VOLUMEDOWN, 0, true },
> > + [0x43] = { KEY_MUTE },
> > + [0x44] = { KEY_PLAY },
> > + [0x45] = { KEY_STOP },
> > + [0x46] = { KEY_PLAYPAUSE }, /* Pause */
> > + [0x47] = { KEY_RECORD },
> > + [0x48] = { KEY_REWIND, 0, true },
> > + [0x49]
[PATCH v6 06/11] cec: add HDMI CEC framework
On Mon, May 04, 2015 at 07:32:59PM +0200, Kamil Debski wrote:
> From: Hans Verkuil
>
> The added HDMI CEC framework provides a generic kernel interface for
> HDMI CEC devices.
>
> Signed-off-by: Hans Verkuil
-snip-
> +int cec_create_adapter(struct cec_adapter *adap, const char *name, u32 caps)
> +{
> + int res = 0;
> +
> + adap->state = CEC_ADAP_STATE_DISABLED;
> + adap->name = name;
> + adap->phys_addr = 0x;
> + adap->capabilities = caps;
> + adap->version = CEC_VERSION_1_4;
> + adap->sequence = 0;
> + mutex_init(&adap->lock);
> + adap->kthread = kthread_run(cec_thread_func, adap, name);
> + init_waitqueue_head(&adap->kthread_waitq);
> + init_waitqueue_head(&adap->waitq);
> + if (IS_ERR(adap->kthread)) {
> + pr_err("cec-%s: kernel_thread() failed\n", name);
> + return PTR_ERR(adap->kthread);
> + }
> + if (caps) {
> + res = cec_devnode_register(&adap->devnode, adap->owner);
> + if (res)
> + kthread_stop(adap->kthread);
> + }
> + adap->recv_notifier = cec_receive_notify;
> +
> + /* Prepare the RC input device */
> + adap->rc = rc_allocate_device();
> + if (!adap->rc) {
> + pr_err("cec-%s: failed to allocate memory for rc_dev\n", name);
> + cec_devnode_unregister(&adap->devnode);
> + kthread_stop(adap->kthread);
> + return -ENOMEM;
> + }
> +
> + snprintf(adap->input_name, sizeof(adap->input_name), "RC for %s", name);
> + snprintf(adap->input_phys, sizeof(adap->input_phys), "%s/input0", name);
> + strncpy(adap->input_drv, name, sizeof(adap->input_drv));
> +
> + adap->rc->input_name = adap->input_name;
> + adap->rc->input_phys = adap->input_phys;
> + adap->rc->dev.parent = &adap->devnode.dev;
> + adap->rc->driver_name = adap->input_drv;
> + adap->rc->driver_type = RC_DRIVER_CEC;
> + adap->rc->allowed_protocols = RC_BIT_CEC;
> + adap->rc->priv = adap;
> + adap->rc->map_name = RC_MAP_CEC;
> + adap->rc->timeout = MS_TO_NS(100);
> +
rc->input_id is not populated. It would be nice if input_phys has some
resemblance to a physical path (like the output of usb_make_path() if it
is a usb device).
> + res = rc_register_device(adap->rc);
> +
> + if (res) {
> + pr_err("cec-%s: failed to prepare input device\n", name);
> + cec_devnode_unregister(&adap->devnode);
> + rc_free_device(adap->rc);
> + kthread_stop(adap->kthread);
> + }
> +
> + return res;
> +}
> +EXPORT_SYMBOL_GPL(cec_create_adapter);
> +
> +void cec_delete_adapter(struct cec_adapter *adap)
> +{
> + if (adap->kthread == NULL)
> + return;
> + kthread_stop(adap->kthread);
> + if (adap->kthread_config)
> + kthread_stop(adap->kthread_config);
> + adap->state = CEC_ADAP_STATE_DISABLED;
> + if (cec_devnode_is_registered(&adap->devnode))
> + cec_devnode_unregister(&adap->devnode);
I think you're missing a rc_unregister_device() here.
> +}
> +EXPORT_SYMBOL_GPL(cec_delete_adapter);
Sean
[RFC v2 3/7] cec: add new framework for cec support.
Hi Kamil, On Fri, Mar 06, 2015 at 05:14:50PM +0100, Kamil Debski wrote: > 3) As you suggested - load an empty keymap whenever the pass through mode is > enabled. > I am not that familiar with the RC core. Is there a simple way to switch to > an empty map > from the kernel? There is the ir_setkeytable function, but it is static in > rc-main.c, so it > cannot be used in other kernel modules. Any hints, Sean? Why is it problematic if keypresses are passed to the input layer? You can only set the default keymap for an rc-device from kernel space; from user space you can clear the table using input ioctl, see: http://git.linuxtv.org/cgit.cgi/v4l-utils.git/tree/utils/keytable/keytable.c#n1277 You can select MAP_EMPTY as the default keymap if that is appropriate; using ir-setkeytable(1) a different keymap can be selected. Sean
[RFC v2 3/7] cec: add new framework for cec support.
On Thu, Jan 22, 2015 at 05:04:35PM +0100, Kamil Debski wrote:
> Add the CEC framework.
-snip-
> +Remote control handling
> +---
> +
> +The CEC framework provides two ways of handling the key messages of remote
> +control. In the first case, the CEC framework will handle these messages and
> +provide the keypressed via the RC framework. In the second case the messages
> +related to the key down/up events are not parsed by the framework and are
> +passed to the userspace as raw messages.
> +
> +Switching between these modes is done with a special ioctl.
> +
> +#define CEC_G_KEY_PASSTHROUGH_IOR('a', 10, __u8)
> +#define CEC_S_KEY_PASSTHROUGH_IOW('a', 11, __u8)
> +#define CEC_KEY_PASSTHROUGH_DISABLE 0
> +#define CEC_KEY_PASSTHROUGH_ENABLE 1
This is ugly. This ioctl stops keypresses from going to rc-core. The cec
device is still registered with rc-core but no keys will be passed to it.
This could also be handled by loading an empty keymap; this way the input
layer will still receive scancodes but no keypresses.
> +static ssize_t cec_read(struct file *filp, char __user *buf,
> + size_t sz, loff_t *off)
> +{
> + struct cec_devnode *cecdev = cec_devnode_data(filp);
> +
> + if (!cec_devnode_is_registered(cecdev))
> + return -EIO;
> + return 0;
> +}
> +
> +static ssize_t cec_write(struct file *filp, const char __user *buf,
> + size_t sz, loff_t *off)
> +{
> + struct cec_devnode *cecdev = cec_devnode_data(filp);
> +
> + if (!cec_devnode_is_registered(cecdev))
> + return -EIO;
> + return 0;
> +}
Both read and write do nothing; they should either -ENOSYS or the fuctions
should be removed.
Sean
[RFC 1/6] cec: add new driver for cec support.
On Tue, Dec 23, 2014 at 03:32:17PM +0100, Kamil Debski wrote: > +There are still a few todo's, the main one being the remote control support > +feature of CEC. I need to research if that should be implemented via the > +standard kernel remote control support. I guess a new rc driver type RC_DRIVER_CEC should be introduced (existing types are RC_DRIVER_IR_RAW and RC_DRIVER_SCANCODE). rc_register_device() should not register the sysfs attributes specific for IR, but register sysfs attributes for cec like a link to the device. In addition there should be a new rc_type protocol RC_TYPE_CEC; now rc_keydown_notimeout() can be called for each key press. I guess a new keymap should exist too. HTH Sean
