Re: [PATCH v3] media: rc: pwm-ir-tx: Switch to atomic PWM API

Re: [PATCH v3] media: rc: pwm-ir-tx: Switch to atomic PWM API

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 1812 bytes --]

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 <lkp@intel.com>

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@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 32140 bytes --]

Re: [PATCH v3] media: rc: pwm-ir-tx: Switch to atomic PWM API

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 2289 bytes --]

On Wed, Oct 27, 2021 at 02:07:19PM +0800, kernel test robot wrote:
> 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 <lkp@intel.com>
> 
> All errors (new ones prefixed by >>, old ones prefixed by <<):
> 
> >> 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

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v3] media: rc: pwm-ir-tx: Switch to atomic PWM API

[Sean Young]

On Wed, Oct 27, 2021 at 08:15:52AM +0200, Uwe Kleine-König wrote:
> On Wed, Oct 27, 2021 at 02:07:19PM +0800, kernel test robot wrote:
> > If you fix the issue, kindly add following tag as appropriate
> > Reported-by: kernel test robot <lkp@intel.com>
> > 
> > All errors (new ones prefixed by >>, old ones prefixed by <<):
> > 
> > >> 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

Re: [PATCH v3] media: rc: pwm-ir-tx: Switch to atomic PWM API

[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.

> Em qua., 27 de out. de 2021 às 04:32, Sean Young <sean@mess.org> escreveu:
>>
>> On Wed, Oct 27, 2021 at 08:15:52AM +0200, Uwe Kleine-König wrote:
>> > On Wed, Oct 27, 2021 at 02:07:19PM +0800, kernel test robot wrote:
>> > > If you fix the issue, kindly add following tag as appropriate
>> > > Reported-by: kernel test robot <lkp@intel.com>
>> > >
>> > > All errors (new ones prefixed by >>, old ones prefixed by <<):
>> > >
>> > > >> 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

Re: [PATCH v3] media: rc: pwm-ir-tx: Switch to atomic PWM API

[Sean Young]

Hi Maíra,

On Wed, Oct 27, 2021 at 09:43:47AM -0300, Maíra Canal wrote:
> [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

> > Em qua., 27 de out. de 2021 às 04:32, Sean Young <sean@mess.org> escreveu:
> >>
> >> On Wed, Oct 27, 2021 at 08:15:52AM +0200, Uwe Kleine-König wrote:
> >> > On Wed, Oct 27, 2021 at 02:07:19PM +0800, kernel test robot wrote:
> >> > > If you fix the issue, kindly add following tag as appropriate
> >> > > Reported-by: kernel test robot <lkp@intel.com>
> >> > >
> >> > > All errors (new ones prefixed by >>, old ones prefixed by <<):
> >> > >
> >> > > >> 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