[PATCH v3] backlight: lp855x: 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 <redacted>
---
V1 -> V2: Initialize variable and simply conditional loop
V2 -> V3: Fix assignment of NULL variable
---
 drivers/video/backlight/lp855x_bl.c | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/drivers/video/backlight/lp855x_bl.c b/drivers/video/backlight/lp855x_bl.c
index e94932c69f54..627a31d05691 100644
--- a/drivers/video/backlight/lp855x_bl.c
+++ b/drivers/video/backlight/lp855x_bl.c

@@ -233,9 +233,8 @@ static int lp855x_configure(struct lp855x *lp)
 
 static void lp855x_pwm_ctrl(struct lp855x *lp, int br, int max_br)
 {
-	unsigned int period = lp->pdata->period_ns;
-	unsigned int duty = br * period / max_br;
-	struct pwm_device *pwm;
+	struct pwm_device *pwm = NULL;
+	struct pwm_state state;
 
 	/* request pwm device with the consumer name */
 	if (!lp->pwm) {

@@ -245,18 +244,16 @@ static void lp855x_pwm_ctrl(struct lp855x *lp, int br, int max_br)
 
 		lp->pwm = pwm;
 
-		/*
-		 * FIXME: pwm_apply_args() should be removed when switching to
-		 * the atomic PWM API.
-		 */
-		pwm_apply_args(pwm);
+		pwm_init_state(lp->pwm, &state);
+		state.period = lp->pdata->period_ns;
 	}
 
-	pwm_config(lp->pwm, duty, period);
-	if (duty)
-		pwm_enable(lp->pwm);
-	else
-		pwm_disable(lp->pwm);
+	pwm_get_state(lp->pwm, &state);
+
+	state.duty_cycle = br * state.period / max_br;
+	state.enabled = state.duty_cycle;
+
+	pwm_apply_state(lp->pwm, &state);
 }
 
 static int lp855x_bl_update_status(struct backlight_device *bl)

-- 
2.31.1

Hi "Maíra,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on lee-backlight/for-backlight-next]
[also build test ERROR on v5.15-rc7 next-20211029]
[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/backlight-lp855x-Switch-to-atomic-PWM-API/20211030-213551
base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/backlight.git for-backlight-next
config: i386-randconfig-a002-20211031 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/f4d53f4d51c78636fc4fd34aecbdcbd1f83f656e
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Ma-ra-Canal/backlight-lp855x-Switch-to-atomic-PWM-API/20211030-213551
        git checkout f4d53f4d51c78636fc4fd34aecbdcbd1f83f656e
        # save the attached .config to linux build tree
        mkdir build_dir
        make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <redacted>

All errors (new ones prefixed by >>):

   ld: drivers/video/backlight/lp855x_bl.o: in function `lp855x_pwm_ctrl':

quoted

drivers/video/backlight/lp855x_bl.c:253: undefined reference to `__udivdi3'

vim +253 drivers/video/backlight/lp855x_bl.c

   233	
   234	static void lp855x_pwm_ctrl(struct lp855x *lp, int br, int max_br)
   235	{
   236		struct pwm_device *pwm = NULL;
   237		struct pwm_state state;
   238	
   239		/* request pwm device with the consumer name */
   240		if (!lp->pwm) {
   241			pwm = devm_pwm_get(lp->dev, lp->chipname);
   242			if (IS_ERR(pwm))
   243				return;
   244	
   245			lp->pwm = pwm;
   246	
   247			pwm_init_state(lp->pwm, &state);
   248			state.period = lp->pdata->period_ns;
   249		}
   250	
   251		pwm_get_state(lp->pwm, &state);
   252	
 > 253		state.duty_cycle = br * state.period / max_br;
   254		state.enabled = state.duty_cycle;
   255	
   256		pwm_apply_state(lp->pwm, &state);
   257	}
   258	

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

Hi "Maíra,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on lee-backlight/for-backlight-next]
[also build test ERROR on v5.15-rc7 next-20211029]
[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/backlight-lp855x-Switch-to-atomic-PWM-API/20211030-213551
base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/backlight.git for-backlight-next
config: m68k-randconfig-r005-20211028 (attached as .config)
compiler: m68k-linux-gcc (GCC) 11.2.0
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
        # https://github.com/0day-ci/linux/commit/f4d53f4d51c78636fc4fd34aecbdcbd1f83f656e
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Ma-ra-Canal/backlight-lp855x-Switch-to-atomic-PWM-API/20211030-213551
        git checkout f4d53f4d51c78636fc4fd34aecbdcbd1f83f656e
        # save the attached .config to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=m68k SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <redacted>

All errors (new ones prefixed by >>):

   m68k-linux-ld: arch/m68k/kernel/machine_kexec.o: in function `machine_kexec':
   machine_kexec.c:(.text+0x154): undefined reference to `m68k_mmutype'
   m68k-linux-ld: machine_kexec.c:(.text+0x15c): undefined reference to `m68k_cputype'
   m68k-linux-ld: arch/m68k/kernel/relocate_kernel.o:(.m68k_fixup+0x0): undefined reference to `M68K_FIXUP_MEMOFFSET'
   m68k-linux-ld: arch/m68k/kernel/relocate_kernel.o:(.m68k_fixup+0x8): undefined reference to `M68K_FIXUP_MEMOFFSET'
   m68k-linux-ld: arch/m68k/kernel/uboot.o: in function `process_uboot_commandline':
   uboot.c:(.init.text+0x2e): undefined reference to `_init_sp'
   m68k-linux-ld: drivers/video/backlight/lp855x_bl.o: in function `lp855x_bl_update_status':

quoted

lp855x_bl.c:(.text+0x15c): undefined reference to `__udivdi3'

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

Hi Maíra,

On Sat, Oct 30, 2021 at 3:35 PM Maíra Canal [off-list ref] wrote:

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

Signed-off-by: Maíra Canal <redacted>

Thanks for your patch!

Above is a 64-by-32 division, which should not be open-coded, as
that will cause link failures on 32-bit platform (cfr. "undefined
reference to `__udivdi3'", as reported by the kernel test robot).
Please use div_u64() instead.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Em ter., 2 de nov. de 2021 às 05:39, Geert Uytterhoeven
[off-list ref] escreveu:

Hi Maíra,

On Sat, Oct 30, 2021 at 3:35 PM Maíra Canal [off-list ref] wrote:

quoted

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

Signed-off-by: Maíra Canal <redacted>

Thanks for your patch!

quoted

--- a/drivers/video/backlight/lp855x_bl.c
+++ b/drivers/video/backlight/lp855x_bl.c

@@ -233,9 +233,8 @@ static int lp855x_configure(struct lp855x *lp)

 static void lp855x_pwm_ctrl(struct lp855x *lp, int br, int max_br)
 {
-       unsigned int period = lp->pdata->period_ns;
-       unsigned int duty = br * period / max_br;
-       struct pwm_device *pwm;
+       struct pwm_device *pwm = NULL;
+       struct pwm_state state;

        /* request pwm device with the consumer name */
        if (!lp->pwm) {

@@ -245,18 +244,16 @@ static void lp855x_pwm_ctrl(struct lp855x *lp, int br, int max_br)

                lp->pwm = pwm;

-               /*
-                * FIXME: pwm_apply_args() should be removed when switching to
-                * the atomic PWM API.
-                */
-               pwm_apply_args(pwm);
+               pwm_init_state(lp->pwm, &state);
+               state.period = lp->pdata->period_ns;
        }

-       pwm_config(lp->pwm, duty, period);
-       if (duty)
-               pwm_enable(lp->pwm);
-       else
-               pwm_disable(lp->pwm);
+       pwm_get_state(lp->pwm, &state);
+
+       state.duty_cycle = br * state.period / max_br;

Above is a 64-by-32 division, which should not be open-coded, as
that will cause link failures on 32-bit platform (cfr. "undefined
reference to `__udivdi3'", as reported by the kernel test robot).
Please use div_u64() instead.

Hi Geert,

Thank you for the suggestion! I fixed this problem a bit differently
and submitted the v4. I made use of the PWM API and applied the
pwm_set_relative_duty_cycle function, which solved this division
problem.

quoted

+       state.enabled = state.duty_cycle;
+
+       pwm_apply_state(lp->pwm, &state);
 }

 static int lp855x_bl_update_status(struct backlight_device *bl)

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds