Messages in this thread | | | Date | Mon, 19 Jun 2023 14:11:09 +0300 | Subject | Re: [PATCH v14 17/18] media: i2c: ds90ub953: Restructure clkout management | From | Tomi Valkeinen <> |
| |
On 19/06/2023 13:44, Andy Shevchenko wrote: > On Mon, Jun 19, 2023 at 01:14:34PM +0300, Tomi Valkeinen wrote: >> On 16/06/2023 17:37, Andy Shevchenko wrote: >>> On Fri, Jun 16, 2023 at 04:59:21PM +0300, Tomi Valkeinen wrote: >>>> Separate clkout calculations and register writes into two functions: >>>> ub953_calc_clkout_params and ub953_write_clkout_regs, and add a struct >>>> ub953_clkout_data that is used to store the clkout parameters. > > ... > >>>> +struct ub953_clkout_data { >>>> + u32 hs_div; >>>> + u32 m; >>>> + u32 n; >>> >>> Please, use struxt u32_fract instead of m/n. >> >> I'm not sure how that helps. The documentation talks about m and n. Using >> different terms will make it more difficult to compare the code and the >> docs. > > You can always add a comment. > > (For example in drivers/clk/clk-fractional-divider.c our documentation also > says about m/n, but most of the people understands that this is about > fractional divider and actually with properly spelled parameters it will > help others who are not experts in the CLK hardware.)
Yes, I would agree with you if this was a generic piece of code, but I don't see the reasoning as valid as this is specific to a piece of hardware. Here, I think, matching the code to the HW documentation is more important than possibly making the code more understandable to people who are not familiar with the HW.
Especially as for non-English people seeing "numerator" and "denominator" might require a check to figure out which one is which, whereas m and n are (I would guess) a bit more universal (or maybe it's just me).
Tomi
| |