Messages in this thread | | | Subject | Re: [PATCH] pwm: Use div64_ul instead of do_div | From | Nicolas Ferre <> | Date | Thu, 18 Nov 2021 14:19:37 +0100 |
| |
On 18/11/2021 at 11:09, Russell King (Oracle) wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On Wed, Nov 17, 2021 at 12:24:00PM +0100, Uwe Kleine-König wrote: >> Hello, >> >> On Wed, Nov 17, 2021 at 02:04:26AM +0000, cgel.zte@gmail.com wrote: >>> From: Changcheng Deng <deng.changcheng@zte.com.cn> >>> >>> do_div() does a 64-by-32 division. If the divisor is unsigned long, using >>> div64_ul can avoid truncation to 32-bit. >> >> After some research I understood your commit log. I'd write: >> >> do_div() does a 64-by-32 division. Here the divsor is an >> unsigned long which on some platforms is 64 bit wide. So use >> div64_ul instead of do_div to avoid a possible truncation. >> >> The priority of this patch seems to be low, as the device seems to exist >> only on (32bit) arm. > > ... where unsigned long is 32-bit. > > In any case, for this to overflow, we would need to have a clock in > excess of 2^32-1 Hz, or around 4GHz - and if we had such a situation > on 32-bit devices, we need to change the type for holding the frequency > in the clk API, and probably a lot of code in the CCF as well. > > Unless there is a real reason for this change, I would suggest leaving > the code as is - there is absolutely no point in making these divisions > more expensive unless there is a real reason.
Thanks for the technical demonstration Russell. With this in mind: NACK to the patch, sorry Changcheng Deng.
Best regards, Nicolas
-- Nicolas Ferre
| |