On Tue, Dec 20, 2022 at 08:36:52AM -0800, matthew.gerlach@xxxxxxxxxxxxxxx wrote: > From: Matthew Gerlach <matthew.gerlach@xxxxxxxxxxxxxxx> > > Add a Device Feature List (DFL) bus driver for the Altera > 16550 implementation of UART. In general the code here looks good to me, but one thing to discuss due to comment to the previous patch(es). ... > + u64 *p; > + > + p = dfh_find_param(dfl_dev, DFHv1_PARAM_ID_CLK_FRQ); > + if (!p) > + return dev_err_probe(dev, -EINVAL, "missing CLK_FRQ param\n"); > + > + p++; > + uart->port.uartclk = *p; So, here and the below is using always the second u64 from the returned data. Does it mean: - we always skip the first u64 from the returned buffer and hence... (see below) - we may actually return the second u64 as a plain number (not a pointer) from (an additional?) API? In such case we would not need to take care about this p++; lines here and there. - we have fixed length of the data, returned by find_param(), i.e. 2 u64 words? -- With Best Regards, Andy Shevchenko