All of lore.kernel.org
 help / color / mirror / Atom feed
From: kernel test robot <lkp@intel.com>
To: "Nylon Chen" <nylon.chen@sifive.com>,
	"Conor Dooley" <conor@kernel.org>,
	"Rob Herring" <robh@kernel.org>,
	"Krzysztof Kozlowski" <krzk@kernel.org>,
	"Paul Walmsley" <paul.walmsley@sifive.com>,
	"Palmer Dabbelt" <palmer@dabbelt.com>,
	"Albert Ou" <aou@eecs.berkeley.edu>,
	"Samuel Holland" <samuel.holland@sifive.com>,
	"Uwe Kleine-König" <ukleinek@kernel.org>
Cc: llvm@lists.linux.dev, oe-kbuild-all@lists.linux.dev,
	linux-riscv@lists.infradead.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-pwm@vger.kernel.org,
	Nylon Chen <nylon.chen@sifive.com>, Zong Li <zong.li@sifive.com>
Subject: Re: [PATCH v13 4/5] pwm: sifive: Fix rounding issues in apply and get_state functions
Date: Thu, 8 May 2025 03:23:02 +0800	[thread overview]
Message-ID: <202505080303.dBfU5YMS-lkp@intel.com> (raw)
In-Reply-To: <20250423090446.294846-5-nylon.chen@sifive.com>

Hi Nylon,

kernel test robot noticed the following build errors:

[auto build test ERROR on robh/for-next]
[also build test ERROR on linus/master v6.15-rc5 next-20250507]
[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#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Nylon-Chen/riscv-dts-sifive-unleashed-unmatched-Remove-PWM-controlled-LED-s-active-low-properties/20250423-165906
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link:    https://lore.kernel.org/r/20250423090446.294846-5-nylon.chen%40sifive.com
patch subject: [PATCH v13 4/5] pwm: sifive: Fix rounding issues in apply and get_state functions
config: hexagon-allmodconfig (https://download.01.org/0day-ci/archive/20250508/202505080303.dBfU5YMS-lkp@intel.com/config)
compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250508/202505080303.dBfU5YMS-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202505080303.dBfU5YMS-lkp@intel.com/

All error/warnings (new ones prefixed by >>):

>> drivers/pwm/pwm-sifive.c:161:2: warning: comparison of distinct pointer types ('typeof ((frac)) *' (aka 'unsigned int *') and 'uint64_t *' (aka 'unsigned long long *')) [-Wcompare-distinct-pointer-types]
     161 |         do_div(frac, state->period);
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/asm-generic/div64.h:183:28: note: expanded from macro 'do_div'
     183 |         (void)(((typeof((n)) *)0) == ((uint64_t *)0));  \
         |                ~~~~~~~~~~~~~~~~~~ ^  ~~~~~~~~~~~~~~~
>> drivers/pwm/pwm-sifive.c:161:2: error: incompatible pointer types passing 'u32 *' (aka 'unsigned int *') to parameter of type 'uint64_t *' (aka 'unsigned long long *') [-Werror,-Wincompatible-pointer-types]
     161 |         do_div(frac, state->period);
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/asm-generic/div64.h:199:22: note: expanded from macro 'do_div'
     199 |                 __rem = __div64_32(&(n), __base);       \
         |                                    ^~~~
   include/asm-generic/div64.h:174:38: note: passing argument to parameter 'dividend' here
     174 | extern uint32_t __div64_32(uint64_t *dividend, uint32_t divisor);
         |                                      ^
>> drivers/pwm/pwm-sifive.c:161:2: warning: shift count >= width of type [-Wshift-count-overflow]
     161 |         do_div(frac, state->period);
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/asm-generic/div64.h:195:25: note: expanded from macro 'do_div'
     195 |         } else if (likely(((n) >> 32) == 0)) {          \
         |                                ^  ~~
   include/linux/compiler.h:76:40: note: expanded from macro 'likely'
      76 | # define likely(x)      __builtin_expect(!!(x), 1)
         |                                             ^
   2 warnings and 1 error generated.


vim +161 drivers/pwm/pwm-sifive.c

   131	
   132	static int pwm_sifive_apply(struct pwm_chip *chip, struct pwm_device *pwm,
   133				    const struct pwm_state *state)
   134	{
   135		struct pwm_sifive_ddata *ddata = pwm_sifive_chip_to_ddata(chip);
   136		struct pwm_state cur_state;
   137		unsigned int duty_cycle;
   138		unsigned long long num;
   139		bool enabled;
   140		int ret = 0;
   141		u32 frac, inactive;
   142	
   143		if (state->polarity != PWM_POLARITY_NORMAL)
   144			return -EINVAL;
   145	
   146		cur_state = pwm->state;
   147		enabled = cur_state.enabled;
   148	
   149		duty_cycle = state->duty_cycle;
   150		if (!state->enabled)
   151			duty_cycle = 0;
   152	
   153		/*
   154		 * The problem of output producing mixed setting as mentioned at top,
   155		 * occurs here. To minimize the window for this problem, we are
   156		 * calculating the register values first and then writing them
   157		 * consecutively
   158		 */
   159		num = (u64)duty_cycle * (1U << PWM_SIFIVE_CMPWIDTH);
   160		frac = num;
 > 161		do_div(frac, state->period);
   162		/* The hardware cannot generate a 0% duty cycle */
   163		frac = min(frac, (1U << PWM_SIFIVE_CMPWIDTH) - 1);
   164		inactive = (1U << PWM_SIFIVE_CMPWIDTH) - 1 - frac;
   165	
   166		mutex_lock(&ddata->lock);
   167		if (state->period != ddata->approx_period) {
   168			/*
   169			 * Don't let a 2nd user change the period underneath the 1st user.
   170			 * However if ddate->approx_period == 0 this is the first time we set
   171			 * any period, so let whoever gets here first set the period so other
   172			 * users who agree on the period won't fail.
   173			 */
   174			if (ddata->user_count != 1 && ddata->approx_period) {
   175				mutex_unlock(&ddata->lock);
   176				return -EBUSY;
   177			}
   178			ddata->approx_period = state->period;
   179			pwm_sifive_update_clock(ddata, clk_get_rate(ddata->clk));
   180		}
   181		mutex_unlock(&ddata->lock);
   182	
   183		/*
   184		 * If the PWM is enabled the clk is already on. So only enable it
   185		 * conditionally to have it on exactly once afterwards independent of
   186		 * the PWM state.
   187		 */
   188		if (!enabled) {
   189			ret = clk_enable(ddata->clk);
   190			if (ret) {
   191				dev_err(pwmchip_parent(chip), "Enable clk failed\n");
   192				return ret;
   193			}
   194		}
   195	
   196		writel(inactive, ddata->regs + PWM_SIFIVE_PWMCMP(pwm->hwpwm));
   197	
   198		if (!state->enabled)
   199			clk_disable(ddata->clk);
   200	
   201		return 0;
   202	}
   203	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

WARNING: multiple messages have this Message-ID (diff)
From: kernel test robot <lkp@intel.com>
To: "Nylon Chen" <nylon.chen@sifive.com>,
	"Conor Dooley" <conor@kernel.org>,
	"Rob Herring" <robh@kernel.org>,
	"Krzysztof Kozlowski" <krzk@kernel.org>,
	"Paul Walmsley" <paul.walmsley@sifive.com>,
	"Palmer Dabbelt" <palmer@dabbelt.com>,
	"Albert Ou" <aou@eecs.berkeley.edu>,
	"Samuel Holland" <samuel.holland@sifive.com>,
	"Uwe Kleine-König" <ukleinek@kernel.org>
Cc: llvm@lists.linux.dev, oe-kbuild-all@lists.linux.dev,
	linux-riscv@lists.infradead.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-pwm@vger.kernel.org,
	Nylon Chen <nylon.chen@sifive.com>, Zong Li <zong.li@sifive.com>
Subject: Re: [PATCH v13 4/5] pwm: sifive: Fix rounding issues in apply and get_state functions
Date: Thu, 8 May 2025 03:23:02 +0800	[thread overview]
Message-ID: <202505080303.dBfU5YMS-lkp@intel.com> (raw)
In-Reply-To: <20250423090446.294846-5-nylon.chen@sifive.com>

Hi Nylon,

kernel test robot noticed the following build errors:

[auto build test ERROR on robh/for-next]
[also build test ERROR on linus/master v6.15-rc5 next-20250507]
[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#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Nylon-Chen/riscv-dts-sifive-unleashed-unmatched-Remove-PWM-controlled-LED-s-active-low-properties/20250423-165906
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link:    https://lore.kernel.org/r/20250423090446.294846-5-nylon.chen%40sifive.com
patch subject: [PATCH v13 4/5] pwm: sifive: Fix rounding issues in apply and get_state functions
config: hexagon-allmodconfig (https://download.01.org/0day-ci/archive/20250508/202505080303.dBfU5YMS-lkp@intel.com/config)
compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250508/202505080303.dBfU5YMS-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202505080303.dBfU5YMS-lkp@intel.com/

All error/warnings (new ones prefixed by >>):

>> drivers/pwm/pwm-sifive.c:161:2: warning: comparison of distinct pointer types ('typeof ((frac)) *' (aka 'unsigned int *') and 'uint64_t *' (aka 'unsigned long long *')) [-Wcompare-distinct-pointer-types]
     161 |         do_div(frac, state->period);
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/asm-generic/div64.h:183:28: note: expanded from macro 'do_div'
     183 |         (void)(((typeof((n)) *)0) == ((uint64_t *)0));  \
         |                ~~~~~~~~~~~~~~~~~~ ^  ~~~~~~~~~~~~~~~
>> drivers/pwm/pwm-sifive.c:161:2: error: incompatible pointer types passing 'u32 *' (aka 'unsigned int *') to parameter of type 'uint64_t *' (aka 'unsigned long long *') [-Werror,-Wincompatible-pointer-types]
     161 |         do_div(frac, state->period);
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/asm-generic/div64.h:199:22: note: expanded from macro 'do_div'
     199 |                 __rem = __div64_32(&(n), __base);       \
         |                                    ^~~~
   include/asm-generic/div64.h:174:38: note: passing argument to parameter 'dividend' here
     174 | extern uint32_t __div64_32(uint64_t *dividend, uint32_t divisor);
         |                                      ^
>> drivers/pwm/pwm-sifive.c:161:2: warning: shift count >= width of type [-Wshift-count-overflow]
     161 |         do_div(frac, state->period);
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/asm-generic/div64.h:195:25: note: expanded from macro 'do_div'
     195 |         } else if (likely(((n) >> 32) == 0)) {          \
         |                                ^  ~~
   include/linux/compiler.h:76:40: note: expanded from macro 'likely'
      76 | # define likely(x)      __builtin_expect(!!(x), 1)
         |                                             ^
   2 warnings and 1 error generated.


vim +161 drivers/pwm/pwm-sifive.c

   131	
   132	static int pwm_sifive_apply(struct pwm_chip *chip, struct pwm_device *pwm,
   133				    const struct pwm_state *state)
   134	{
   135		struct pwm_sifive_ddata *ddata = pwm_sifive_chip_to_ddata(chip);
   136		struct pwm_state cur_state;
   137		unsigned int duty_cycle;
   138		unsigned long long num;
   139		bool enabled;
   140		int ret = 0;
   141		u32 frac, inactive;
   142	
   143		if (state->polarity != PWM_POLARITY_NORMAL)
   144			return -EINVAL;
   145	
   146		cur_state = pwm->state;
   147		enabled = cur_state.enabled;
   148	
   149		duty_cycle = state->duty_cycle;
   150		if (!state->enabled)
   151			duty_cycle = 0;
   152	
   153		/*
   154		 * The problem of output producing mixed setting as mentioned at top,
   155		 * occurs here. To minimize the window for this problem, we are
   156		 * calculating the register values first and then writing them
   157		 * consecutively
   158		 */
   159		num = (u64)duty_cycle * (1U << PWM_SIFIVE_CMPWIDTH);
   160		frac = num;
 > 161		do_div(frac, state->period);
   162		/* The hardware cannot generate a 0% duty cycle */
   163		frac = min(frac, (1U << PWM_SIFIVE_CMPWIDTH) - 1);
   164		inactive = (1U << PWM_SIFIVE_CMPWIDTH) - 1 - frac;
   165	
   166		mutex_lock(&ddata->lock);
   167		if (state->period != ddata->approx_period) {
   168			/*
   169			 * Don't let a 2nd user change the period underneath the 1st user.
   170			 * However if ddate->approx_period == 0 this is the first time we set
   171			 * any period, so let whoever gets here first set the period so other
   172			 * users who agree on the period won't fail.
   173			 */
   174			if (ddata->user_count != 1 && ddata->approx_period) {
   175				mutex_unlock(&ddata->lock);
   176				return -EBUSY;
   177			}
   178			ddata->approx_period = state->period;
   179			pwm_sifive_update_clock(ddata, clk_get_rate(ddata->clk));
   180		}
   181		mutex_unlock(&ddata->lock);
   182	
   183		/*
   184		 * If the PWM is enabled the clk is already on. So only enable it
   185		 * conditionally to have it on exactly once afterwards independent of
   186		 * the PWM state.
   187		 */
   188		if (!enabled) {
   189			ret = clk_enable(ddata->clk);
   190			if (ret) {
   191				dev_err(pwmchip_parent(chip), "Enable clk failed\n");
   192				return ret;
   193			}
   194		}
   195	
   196		writel(inactive, ddata->regs + PWM_SIFIVE_PWMCMP(pwm->hwpwm));
   197	
   198		if (!state->enabled)
   199			clk_disable(ddata->clk);
   200	
   201		return 0;
   202	}
   203	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

  reply	other threads:[~2025-05-07 19:24 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-23  9:04 [PATCH v13 0/5] Change PWM-controlled LED pin active mode and algorithm Nylon Chen
2025-04-23  9:04 ` Nylon Chen
2025-04-23  9:04 ` [PATCH v13 1/5] riscv: dts: sifive: unleashed/unmatched: Remove PWM controlled LED's active-low properties Nylon Chen
2025-04-23  9:04   ` Nylon Chen
2025-04-23  9:04 ` [PATCH v13 2/5] pwm: sifive: change the PWM algorithm Nylon Chen
2025-04-23  9:04   ` Nylon Chen
2025-04-23  9:04 ` [PATCH v13 3/5] pwm: sifive: Fix the error in the idempotent test within the pwm_apply_state_debug function Nylon Chen
2025-04-23  9:04   ` Nylon Chen
2025-04-23  9:04 ` [PATCH v13 4/5] pwm: sifive: Fix rounding issues in apply and get_state functions Nylon Chen
2025-04-23  9:04   ` Nylon Chen
2025-05-07 19:23   ` kernel test robot [this message]
2025-05-07 19:23     ` kernel test robot
2025-04-23  9:04 ` [PATCH v13 5/5] pwm: sifive: clarify inverted compare logic in comments Nylon Chen
2025-04-23  9:04   ` Nylon Chen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=202505080303.dBfU5YMS-lkp@intel.com \
    --to=lkp@intel.com \
    --cc=aou@eecs.berkeley.edu \
    --cc=conor@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=krzk@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pwm@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=llvm@lists.linux.dev \
    --cc=nylon.chen@sifive.com \
    --cc=oe-kbuild-all@lists.linux.dev \
    --cc=palmer@dabbelt.com \
    --cc=paul.walmsley@sifive.com \
    --cc=robh@kernel.org \
    --cc=samuel.holland@sifive.com \
    --cc=ukleinek@kernel.org \
    --cc=zong.li@sifive.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.