Re: [PATCH v14 17/18] media: i2c: ds90ub953: Restructure clkout management

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux