On 09/06/2023 10:57, Walter Harms wrote: > > while we are here .... > > perhaps INT_MAX from kernel.h ? > > int deviation = (1 << 30) - 1; > > the part before looks a bit strange > > if (ourport->info->has_divslot) { > unsigned long div = rate / req_baud; > > /* The UDIVSLOT register on the newer UARTs allows us to > * get a divisor adjustment of 1/16th on the baud clock. > * > * We don't keep the UDIVSLOT value (the 16ths we > * calculated by not multiplying the baud by 16) as it > * is easy enough to recalculate. > */ > > quot = div / 16; > baud = rate / div; > because > baud=rate/rate/req_baud = req_baud > can this be simplyfied ? (or is the numeric required ?) > > > Homebrew abs() kernel.h has a abs() can we use it here ? > > if (calc_deviation < 0) > calc_deviation = -calc_deviation; > > to the patch: > > + /* > + * If we find a better clk, release the previous one, if > + * any. > + */ > + if (!IS_ERR(*best_clk)) > + clk_put(*best_clk); > > the intentions are good. *best_clk is user supplied (and should be NULL) > filled & released in the next round but IMHO must be valid (is clk). > so no need to check. (ntl clk_put seems to handle NULL and ERR ) > if (!clk || WARN_ON_ONCE(IS_ERR(clk))) > return; Don't top-post. Anyway, I don't understand what you want to say here. Best regards, Krzysztof