* [PATCH v7 03/13] hwmon: pwm-fan: Use 64-bit division macros for period and duty cycle [not found] <cover.1583782035.git.gurus@codeaurora.org> @ 2020-03-09 19:35 ` Guru Das Srinagesh 2020-03-09 21:48 ` Guenter Roeck 0 siblings, 1 reply; 6+ messages in thread From: Guru Das Srinagesh @ 2020-03-09 19:35 UTC (permalink / raw) To: linux-pwm Cc: Thierry Reding, Uwe Kleine-König, Subbaraman Narayanamurthy, linux-kernel, Guru Das Srinagesh, Kamil Debski, Bartlomiej Zolnierkiewicz, Jean Delvare, Guenter Roeck, Liam Girdwood, Mark Brown, linux-hwmon Because period and duty cycle are defined in the PWM framework structs as ints with units of nanoseconds, the maximum time duration that can be set is limited to ~2.147 seconds. Redefining them as u64 values will enable larger time durations to be set. As a first step, prepare drivers to handle the switch to u64 period and duty_cycle by replacing division operations involving pwm period and duty cycle with their 64-bit equivalents as appropriate. The actual switch to u64 period and duty_cycle follows as a separate patch. Where the dividend is 64-bit but the divisor is 32-bit, use *_ULL macros: - DIV_ROUND_UP_ULL - DIV_ROUND_CLOSEST_ULL - div_u64 Where the divisor is 64-bit (dividend may be 32-bit or 64-bit), use DIV64_* macros: - DIV64_U64_ROUND_CLOSEST - div64_u64 Cc: Kamil Debski <kamil@wypas.org> Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> Cc: Jean Delvare <jdelvare@suse.com> Cc: Guenter Roeck <linux@roeck-us.net> Cc: Liam Girdwood <lgirdwood@gmail.com> Cc: Mark Brown <broonie@kernel.org> Cc: linux-hwmon@vger.kernel.org Signed-off-by: Guru Das Srinagesh <gurus@codeaurora.org> --- drivers/hwmon/pwm-fan.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c index 42ffd2e..283423a 100644 --- a/drivers/hwmon/pwm-fan.c +++ b/drivers/hwmon/pwm-fan.c @@ -437,7 +437,7 @@ static int pwm_fan_resume(struct device *dev) return 0; pwm_get_args(ctx->pwm, &pargs); - duty = DIV_ROUND_UP(ctx->pwm_value * (pargs.period - 1), MAX_PWM); + duty = DIV_ROUND_UP_ULL(ctx->pwm_value * (pargs.period - 1), MAX_PWM); ret = pwm_config(ctx->pwm, duty, pargs.period); if (ret) return ret; -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v7 03/13] hwmon: pwm-fan: Use 64-bit division macros for period and duty cycle 2020-03-09 19:35 ` [PATCH v7 03/13] hwmon: pwm-fan: Use 64-bit division macros for period and duty cycle Guru Das Srinagesh @ 2020-03-09 21:48 ` Guenter Roeck 2020-03-10 12:08 ` Uwe Kleine-König 0 siblings, 1 reply; 6+ messages in thread From: Guenter Roeck @ 2020-03-09 21:48 UTC (permalink / raw) To: Guru Das Srinagesh Cc: linux-pwm, Thierry Reding, Uwe Kleine-König, Subbaraman Narayanamurthy, linux-kernel, Kamil Debski, Bartlomiej Zolnierkiewicz, Jean Delvare, Liam Girdwood, Mark Brown, linux-hwmon On Mon, Mar 09, 2020 at 12:35:06PM -0700, Guru Das Srinagesh wrote: > Because period and duty cycle are defined in the PWM framework structs > as ints with units of nanoseconds, the maximum time duration that can be > set is limited to ~2.147 seconds. Redefining them as u64 values will > enable larger time durations to be set. > > As a first step, prepare drivers to handle the switch to u64 period and > duty_cycle by replacing division operations involving pwm period and duty cycle > with their 64-bit equivalents as appropriate. The actual switch to u64 period > and duty_cycle follows as a separate patch. > > Where the dividend is 64-bit but the divisor is 32-bit, use *_ULL > macros: > - DIV_ROUND_UP_ULL > - DIV_ROUND_CLOSEST_ULL > - div_u64 > > Where the divisor is 64-bit (dividend may be 32-bit or 64-bit), use > DIV64_* macros: > - DIV64_U64_ROUND_CLOSEST > - div64_u64 > There is no explanation why this is necessary. What is the use case ? It is hard to imagine a real-world use case with a duty cycle of more than 2 seconds. Guenter ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v7 03/13] hwmon: pwm-fan: Use 64-bit division macros for period and duty cycle 2020-03-09 21:48 ` Guenter Roeck @ 2020-03-10 12:08 ` Uwe Kleine-König 2020-03-10 15:05 ` Guenter Roeck 0 siblings, 1 reply; 6+ messages in thread From: Uwe Kleine-König @ 2020-03-10 12:08 UTC (permalink / raw) To: Guenter Roeck Cc: Guru Das Srinagesh, linux-pwm, Thierry Reding, Subbaraman Narayanamurthy, linux-kernel, Kamil Debski, Bartlomiej Zolnierkiewicz, Jean Delvare, Liam Girdwood, Mark Brown, linux-hwmon Hello Guenter, On Mon, Mar 09, 2020 at 02:48:22PM -0700, Guenter Roeck wrote: > On Mon, Mar 09, 2020 at 12:35:06PM -0700, Guru Das Srinagesh wrote: > > Because period and duty cycle are defined in the PWM framework structs > > as ints with units of nanoseconds, the maximum time duration that can be > > set is limited to ~2.147 seconds. Redefining them as u64 values will > > enable larger time durations to be set. > > > > As a first step, prepare drivers to handle the switch to u64 period and > > duty_cycle by replacing division operations involving pwm period and duty cycle > > with their 64-bit equivalents as appropriate. The actual switch to u64 period > > and duty_cycle follows as a separate patch. > > > > Where the dividend is 64-bit but the divisor is 32-bit, use *_ULL > > macros: > > - DIV_ROUND_UP_ULL > > - DIV_ROUND_CLOSEST_ULL > > - div_u64 > > > > Where the divisor is 64-bit (dividend may be 32-bit or 64-bit), use > > DIV64_* macros: > > - DIV64_U64_ROUND_CLOSEST > > - div64_u64 > > There is no explanation why this is necessary. What is the use case ? > It is hard to imagine a real-world use case with a duty cycle of more > than 2 seconds. When my Laptop is in suspend there is an LED that blinks with a period of approximately 5 seconds. (To be fair, the brightness is more a sinus than a rectangle, but still.) Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ | ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v7 03/13] hwmon: pwm-fan: Use 64-bit division macros for period and duty cycle 2020-03-10 12:08 ` Uwe Kleine-König @ 2020-03-10 15:05 ` Guenter Roeck 2020-03-10 22:24 ` Guru Das Srinagesh 0 siblings, 1 reply; 6+ messages in thread From: Guenter Roeck @ 2020-03-10 15:05 UTC (permalink / raw) To: Uwe Kleine-König Cc: Guru Das Srinagesh, linux-pwm, Thierry Reding, Subbaraman Narayanamurthy, linux-kernel, Kamil Debski, Bartlomiej Zolnierkiewicz, Jean Delvare, Liam Girdwood, Mark Brown, linux-hwmon On 3/10/20 5:08 AM, Uwe Kleine-König wrote: > Hello Guenter, > > On Mon, Mar 09, 2020 at 02:48:22PM -0700, Guenter Roeck wrote: >> On Mon, Mar 09, 2020 at 12:35:06PM -0700, Guru Das Srinagesh wrote: >>> Because period and duty cycle are defined in the PWM framework structs >>> as ints with units of nanoseconds, the maximum time duration that can be >>> set is limited to ~2.147 seconds. Redefining them as u64 values will >>> enable larger time durations to be set. >>> >>> As a first step, prepare drivers to handle the switch to u64 period and >>> duty_cycle by replacing division operations involving pwm period and duty cycle >>> with their 64-bit equivalents as appropriate. The actual switch to u64 period >>> and duty_cycle follows as a separate patch. >>> >>> Where the dividend is 64-bit but the divisor is 32-bit, use *_ULL >>> macros: >>> - DIV_ROUND_UP_ULL >>> - DIV_ROUND_CLOSEST_ULL >>> - div_u64 >>> >>> Where the divisor is 64-bit (dividend may be 32-bit or 64-bit), use >>> DIV64_* macros: >>> - DIV64_U64_ROUND_CLOSEST >>> - div64_u64 >> >> There is no explanation why this is necessary. What is the use case ? >> It is hard to imagine a real-world use case with a duty cycle of more >> than 2 seconds. > > When my Laptop is in suspend there is an LED that blinks with a period > of approximately 5 seconds. (To be fair, the brightness is more a sinus > than a rectangle, but still.) > I don't see support in the LED subsystem to utilize the PWM framework directly for blinking. Plus, you say yourself that it isn't a _real_ use case, just a theoretic one. Either case, the reason / use case for this series should be explained in the summary patch. That is what it is for. That case is not made. Guenter ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v7 03/13] hwmon: pwm-fan: Use 64-bit division macros for period and duty cycle 2020-03-10 15:05 ` Guenter Roeck @ 2020-03-10 22:24 ` Guru Das Srinagesh 2020-03-10 22:57 ` Guenter Roeck 0 siblings, 1 reply; 6+ messages in thread From: Guru Das Srinagesh @ 2020-03-10 22:24 UTC (permalink / raw) To: Guenter Roeck Cc: Uwe Kleine-König, linux-pwm, Thierry Reding, Subbaraman Narayanamurthy, linux-kernel, Kamil Debski, Bartlomiej Zolnierkiewicz, Jean Delvare, Liam Girdwood, Mark Brown, linux-hwmon On Tue, Mar 10, 2020 at 08:05:58AM -0700, Guenter Roeck wrote: > I don't see support in the LED subsystem to utilize the PWM framework directly > for blinking. Plus, you say yourself that it isn't a _real_ use case, just a > theoretic one. An example use case is a mobile phone OEM that wishes to set a period of 5 seconds or more for, say, a low battery slow blinking indication - currently this is not possible. The PWM framework not having direct support for blinking should not be a concern if the PWM peripheral being controlled supports it via register writes. > Either case, the reason / use case for this series should be explained > in the summary patch. That is what it is for. That case is not made. Will upload a new patchset adding more details in the summary patch. Thank you. Guru Das. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v7 03/13] hwmon: pwm-fan: Use 64-bit division macros for period and duty cycle 2020-03-10 22:24 ` Guru Das Srinagesh @ 2020-03-10 22:57 ` Guenter Roeck 0 siblings, 0 replies; 6+ messages in thread From: Guenter Roeck @ 2020-03-10 22:57 UTC (permalink / raw) To: Guru Das Srinagesh Cc: Uwe Kleine-König, linux-pwm, Thierry Reding, Subbaraman Narayanamurthy, linux-kernel, Kamil Debski, Bartlomiej Zolnierkiewicz, Jean Delvare, Liam Girdwood, Mark Brown, linux-hwmon On 3/10/20 3:24 PM, Guru Das Srinagesh wrote: > On Tue, Mar 10, 2020 at 08:05:58AM -0700, Guenter Roeck wrote: >> I don't see support in the LED subsystem to utilize the PWM framework directly >> for blinking. Plus, you say yourself that it isn't a _real_ use case, just a >> theoretic one. > > An example use case is a mobile phone OEM that wishes to set a period of > 5 seconds or more for, say, a low battery slow blinking indication - currently > this is not possible. The PWM framework not having direct support for > blinking should not be a concern if the PWM peripheral being controlled > supports it via register writes. > >> Either case, the reason / use case for this series should be explained >> in the summary patch. That is what it is for. That case is not made. > > Will upload a new patchset adding more details in the summary patch. > Well, let's assume that this is a real use case. Please also add information about alternatives considered, such as keeping the second-part of the period in a separate variable. Guenter ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-03-10 22:57 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <cover.1583782035.git.gurus@codeaurora.org> 2020-03-09 19:35 ` [PATCH v7 03/13] hwmon: pwm-fan: Use 64-bit division macros for period and duty cycle Guru Das Srinagesh 2020-03-09 21:48 ` Guenter Roeck 2020-03-10 12:08 ` Uwe Kleine-König 2020-03-10 15:05 ` Guenter Roeck 2020-03-10 22:24 ` Guru Das Srinagesh 2020-03-10 22:57 ` Guenter Roeck
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).