On Thu, May 08, 2014 at 03:29:30PM +0100, Lee Jones wrote:

quoted

Please document the units for these fields here. It looks like you're
using picoseconds.

Can't we leave this open to interpretation?  For instance, it's more
convenient for our driver to handle these as nano second values.

No, their values will be determined by the nand_base core, and we must
have something consistent for drivers to rely on. However, I don't
really have a hard preference on nanoseconds versus picoseconds. If we
see that many of the values reach low-digit nanosecons, or fractional
nanoseconds, it probably makes sense to have the higher resolution.

quoted

quoted

+struct nand_sdr_timings {

...

quoted

quoted

+	u32 tCS_min;

	u32 tCSD_min;

Is this a suggested addition, Lee? I agree with Boris that this looks
like a DDR mode, which should not be covered here.

quoted

quoted

+	u32 tDH_min;
+	u32 tDS_min;
+	u32 tFEAT_max;
+	u32 tIR_min;
+	u32 tITC_max;

	u32 tR_max;

Same here, is this a suggested new field? If you need it, then we can
follow up like Boris suggested, with a different method, since tR is not
part of the electrical parameters of the timing mode.

...

quoted

quoted

+};
+
 #endif /* __LINUX_MTD_NAND_H */

Brian