media: rc: pwm-ir-tx: Switch to atomic PWM API

PATCH Superseded Maíra Canal 4 participants 8 messages Oct 24, 2021 – Oct 27, 2021

Subsystems (4)

Media (V4L/DVB) PWM RC-core / lirc framework PWM IR Transmitter

lore

Share & tools

lore

Maíra Canal

author

2021-10-24 10:33 UTC #0 lore raw

PATCH v3 1/1 media: rc: pwm-ir-tx: Switch to atomic PWM API

Remove legacy PWM interface (pwm_config, pwm_enable, pwm_disable) and replace it for the atomic PWM API.

Signed-off-by Maíra Canal <[email protected]>

---
V1 -> V2: Assign variables directly and simplify conditional statement V2 -> V3: Fix declaration of undeclared variable

---
 drivers/media/rc/pwm-ir-tx.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

drivers/media/rc/pwm-ir-tx.c +9 -9

index 4bc28d2c9cc9..e1f348a962e8 100644
@@ -53,22 +53,21 @@ 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;
-	int i, duty, period;
+	struct pwm_state state;
+	int i;
 	ktime_t edge;
 	long delta;
 
-	period = DIV_ROUND_CLOSEST(NSEC_PER_SEC, pwm_ir->carrier);
-	duty = DIV_ROUND_CLOSEST(pwm_ir->duty_cycle * period, 100);
+	pwm_init_state(pwm, &state);
 
-	pwm_config(pwm, duty, period);
+	state.period = DIV_ROUND_CLOSEST(NSEC_PER_SEC, pwm_ir->carrier);
+	state.duty_cycle = DIV_ROUND_CLOSEST(pwm_ir->duty_cycle * state.period, 100);
 
 	edge = ktime_get();
 
 	for (i = 0; i < count; i++) {
-		if (i % 2) // space
-			pwm_disable(pwm);
-		else
-			pwm_enable(pwm);
+		state.enabled = !(i % 2);
+		pwm_apply_state(pwm, &state);
 
 		edge = ktime_add_us(edge, txbuf[i]);
 		delta = ktime_us_delta(edge, ktime_get());

@@ -76,7 +75,8 @@ static int pwm_ir_tx(struct rc_dev *dev, unsigned int *txbuf,
 			usleep_range(delta, delta + 10);
 	}
 
-	pwm_disable(pwm);
+	state.enabled = false;
+	pwm_apply_state(pwm, &state);
 
 	return count;
 }

Signature · 1 line

2.31.1

Uwe Kleine-König

Acked-by on v3 1/1

2021-10-25 06:08 UTC #1 lore raw

Hello,

Quoted 46 lines · Maíra Canal

Remove legacy PWM interface (pwm_config, pwm_enable, pwm_disable) and replace it for the atomic PWM API.

Signed-off-by Maíra Canal <[email protected]>

---
V1 -> V2: Assign variables directly and simplify conditional statement V2 -> V3: Fix declaration of undeclared variable

---
 drivers/media/rc/pwm-ir-tx.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

drivers/media/rc/pwm-ir-tx.c +7 -8

index 4bc28d2c9cc9..e1f348a962e8 100644
@@ -53,22 +53,21 @@ 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;
-	int i, duty, period;
+	struct pwm_state state;
+	int i;
 	ktime_t edge;
 	long delta;
 
-	period = DIV_ROUND_CLOSEST(NSEC_PER_SEC, pwm_ir->carrier);
-	duty = DIV_ROUND_CLOSEST(pwm_ir->duty_cycle * period, 100);
+	pwm_init_state(pwm, &state);
 
-	pwm_config(pwm, duty, period);
+	state.period = DIV_ROUND_CLOSEST(NSEC_PER_SEC, pwm_ir->carrier);
+	state.duty_cycle = DIV_ROUND_CLOSEST(pwm_ir->duty_cycle * state.period, 100);
 
 	edge = ktime_get();
 
 	for (i = 0; i < count; i++) {
-		if (i % 2) // space
-			pwm_disable(pwm);
-		else
-			pwm_enable(pwm);
+		state.enabled = !(i % 2);
+		pwm_apply_state(pwm, &state);
 
 		edge = ktime_add_us(edge, txbuf[i]);
 		delta = ktime_us_delta(edge, ktime_get());

drivers/media/rc/pwm-ir-tx.c static int pwm_ir_tx(struct rc_dev *dev, unsigned int *txbuf, · line 79

Quoted 6 lines

drivers/media/rc/pwm-ir-tx.c +2 -1

@@ -76,7 +75,8 @@ static int pwm_ir_tx(struct rc_dev *dev, unsigned int *txbuf,
 			usleep_range(delta, delta + 10);
 	}
 
-	pwm_disable(pwm);
+	state.enabled = false;
+	pwm_apply_state(pwm, &state);

I would have added a struct pwm_state to struct pwm_ir and then would call pwm_init_state() only once in .probe(). But that's subjective if you like it better or not, so do what you prefer.

The other changes look fine, so:

Acked-by Uwe Kleine-König <[email protected]>

Best regards
Uwe

Signature · 2 lines

Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

kernel test robot

build bot

2021-10-27 06:07 UTC #2 lore raw

In reply to Maíra Canal

Thank you for the patch! Yet something to improve:

Hi "Maíra,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on media-tree/master]
[also build test ERROR on v5.15-rc7 next-20211026]
[If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Ma-ra-Canal/media-rc-pwm-ir-tx-Switch-to-atomic-PWM-API/20211024-183502
base:   git://linuxtv.org/media_tree.git master
config: riscv-randconfig-r004-20211027 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 5db7568a6a1fcb408eb8988abdaff2a225a8eb72)

reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# install riscv cross compiling tool for clang build
# apt-get install binutils-riscv64-linux-gnu
# https://github.com/0day-ci/linux/commit/abea850df0b6436083fcaa097ad3029a27aa62bb
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Ma-ra-Canal/media-rc-pwm-ir-tx-Switch-to-atomic-PWM-API/20211024-183502 git checkout abea850df0b6436083fcaa097ad3029a27aa62bb
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=riscv

If you fix the issue, kindly add following tag as appropriate

Reported-by kernel test robot <[email protected]>

All errors (new ones prefixed by >>, old ones prefixed by <<):

>> ERROR: modpost: "__udivdi3" [drivers/media/rc/pwm-ir-tx.ko] undefined!

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

Uwe Kleine-König

2021-10-27 06:15 UTC #3 lore raw

Quoted 26 lines · kernel test robot

Hi "Maíra,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on media-tree/master]
[also build test ERROR on v5.15-rc7 next-20211026]
[If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:    https://github.com/0day-ci/linux/commits/Ma-ra-Canal/media-rc-pwm-ir-tx-Switch-to-atomic-PWM-API/20211024-183502
base:   git://linuxtv.org/media_tree.git master
config: riscv-randconfig-r004-20211027 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 5db7568a6a1fcb408eb8988abdaff2a225a8eb72)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# install riscv cross compiling tool for clang build
# apt-get install binutils-riscv64-linux-gnu
# https://github.com/0day-ci/linux/commit/abea850df0b6436083fcaa097ad3029a27aa62bb
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Ma-ra-Canal/media-rc-pwm-ir-tx-Switch-to-atomic-PWM-API/20211024-183502 git checkout abea850df0b6436083fcaa097ad3029a27aa62bb
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=riscv

Quoted 6 lines

If you fix the issue, kindly add following tag as appropriate
Reported-by kernel test robot <[email protected]>
All errors (new ones prefixed by >>, old ones prefixed by <<):
Quoted 1 line
Quoted 1 line
ERROR: modpost: "__udivdi3" [drivers/media/rc/pwm-ir-tx.ko] undefined!

This comes from the line:

	state.duty_cycle = DIV_ROUND_CLOSEST(pwm_ir->duty_cycle * state.period, 100);

where DIV_ROUND_CLOSEST expands to a normal division but state.period is a u64. So this should use DIV64_U64_ROUND_CLOSEST I guess.

Best regards
Uwe

Signature · 2 lines

Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

Sean Young

maintainer

2021-10-27 07:32 UTC #4 lore raw

Quoted 6 lines · Uwe Kleine-König

Quoted 5 lines · kernel test robot
If you fix the issue, kindly add following tag as appropriate
Reported-by kernel test robot <[email protected]>
All errors (new ones prefixed by >>, old ones prefixed by <<):

Quoted 8 lines

Quoted 1 line
Quoted 1 line
Quoted 1 line
ERROR: modpost: "__udivdi3" [drivers/media/rc/pwm-ir-tx.ko] undefined!
This comes from the line:
	state.duty_cycle = DIV_ROUND_CLOSEST(pwm_ir->duty_cycle * state.period, 100);
where DIV_ROUND_CLOSEST expands to a normal division but state.period is a u64. So this should use DIV64_U64_ROUND_CLOSEST I guess.

DIV64_U64_ROUND_CLOSEST is for dividing a u64 with a u64. We're dividing by 100 here so this is not necessary.

It should use DIV_ROUND_CLOSEST_ULL, however it might be nicer to use:

	pwm_set_relative_duty_cycle(&state, pwm_ir->duty_cycle, 100);

Thanks

Sean

Maíra Canal

author

2021-10-27 12:38 UTC #5 lore raw

Thank you for the feedback! I appreciate that! I'm new at the kernel and I got a little confused about how to send the new patch. Should I send a v4 of this patch or just send a new patch fixing this issue? I'm sorry about the question and thank you for your attention.

Em qua., 27 de out. de 2021 às 04:32, Sean Young <[email protected]> escreveu:

Quoted 29 lines · Uwe Kleine-König

Quoted 7 lines · kernel test robot
Quoted 6 lines
If you fix the issue, kindly add following tag as appropriate
Reported-by kernel test robot <[email protected]>
All errors (new ones prefixed by >>, old ones prefixed by <<):
Quoted 1 line
Quoted 1 line
ERROR: modpost: "__udivdi3" [drivers/media/rc/pwm-ir-tx.ko]
undefined!
Quoted 4 lines
This comes from the line:
      state.duty_cycle = DIV_ROUND_CLOSEST(pwm_ir->duty_cycle *
state.period, 100);
Quoted 3 lines
where DIV_ROUND_CLOSEST expands to a normal division but state.period is a u64. So this should use DIV64_U64_ROUND_CLOSEST I guess.
DIV64_U64_ROUND_CLOSEST is for dividing a u64 with a u64. We're dividing by 100 here so this is not necessary.
It should use DIV_ROUND_CLOSEST_ULL, however it might be nicer to use:
        pwm_set_relative_duty_cycle(&state, pwm_ir->duty_cycle, 100);
Thanks
Sean

Maíra Canal

author

2021-10-27 12:43 UTC #6 lore raw

[resend it in Plain Text]
Thank you for the feedback! I appreciate that! I'm new at the kernel and I got a little confused about how to send the new patch. Should I send a v4 of this patch or just send a new patch fixing this issue? I'm sorry about the question and thank you for your attention.

Quoted 28 lines

Em qua., 27 de out. de 2021 às 04:32, Sean Young <[email protected]> escreveu:
Quoted 27 lines · Uwe Kleine-König
Quoted 6 lines · kernel test robot
Quoted 5 lines
If you fix the issue, kindly add following tag as appropriate
Reported-by kernel test robot <[email protected]>
All errors (new ones prefixed by >>, old ones prefixed by <<):
Quoted 8 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
ERROR: modpost: "__udivdi3" [drivers/media/rc/pwm-ir-tx.ko] undefined!
This comes from the line:
      state.duty_cycle = DIV_ROUND_CLOSEST(pwm_ir->duty_cycle * state.period, 100);
where DIV_ROUND_CLOSEST expands to a normal division but state.period is a u64. So this should use DIV64_U64_ROUND_CLOSEST I guess.
DIV64_U64_ROUND_CLOSEST is for dividing a u64 with a u64. We're dividing by 100 here so this is not necessary.
It should use DIV_ROUND_CLOSEST_ULL, however it might be nicer to use:
        pwm_set_relative_duty_cycle(&state, pwm_ir->duty_cycle, 100);
Thanks
Sean

Sean Young

maintainer

2021-10-27 12:48 UTC #7 lore raw

Hi Maíra,

Quoted 5 lines · Maíra Canal

[resend it in Plain Text]
Thank you for the feedback! I appreciate that! I'm new at the kernel and I got a little confused about how to send the new patch. Should I send a v4 of this patch or just send a new patch fixing this issue? I'm sorry about the question and thank you for your attention.

Please send out a v4 with the problem fixed.

Also top-posting is deprecated on linux mailing lists.

Thanks,

Sean

Quoted 28 lines

Quoted 28 lines
Em qua., 27 de out. de 2021 às 04:32, Sean Young <[email protected]> escreveu:
Quoted 27 lines · Uwe Kleine-König
Quoted 6 lines · kernel test robot
Quoted 5 lines
If you fix the issue, kindly add following tag as appropriate
Reported-by kernel test robot <[email protected]>
All errors (new ones prefixed by >>, old ones prefixed by <<):
Quoted 8 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
ERROR: modpost: "__udivdi3" [drivers/media/rc/pwm-ir-tx.ko] undefined!
This comes from the line:
      state.duty_cycle = DIV_ROUND_CLOSEST(pwm_ir->duty_cycle * state.period, 100);
where DIV_ROUND_CLOSEST expands to a normal division but state.period is a u64. So this should use DIV64_U64_ROUND_CLOSEST I guess.
DIV64_U64_ROUND_CLOSEST is for dividing a u64 with a u64. We're dividing by 100 here so this is not necessary.
It should use DIV_ROUND_CLOSEST_ULL, however it might be nicer to use:
        pwm_set_relative_duty_cycle(&state, pwm_ir->duty_cycle, 100);
Thanks
Sean