On Mon, May 13, 2019 at 12:39:55AM +0200, Ondřej Jirman wrote: > > + /* > > + * clkin = 24MHz > > + * T acquire = clkin / (SUN50I_THS_CTRL0_T_ACQ + 1) > > + * = 20us > > + */ > > + regmap_write(tmdev->regmap, SUN50I_THS_CTRL0, > > + SUN50I_THS_CTRL0_T_ACQ(479)); > > + /* average over 4 samples */ > > + regmap_write(tmdev->regmap, SUN50I_H6_THS_MFC, > > + SUN50I_THS_FILTER_EN | > > + SUN50I_THS_FILTER_TYPE(1)); > > + /* period = (SUN50I_H6_THS_PC_TEMP_PERIOD + 1) * 4096 / clkin; ~10ms */ > > + regmap_write(tmdev->regmap, SUN50I_H6_THS_PC, > > + SUN50I_H6_THS_PC_TEMP_PERIOD(58)); > > Also this math is not all that clear: > > period = (SUN50I_H6_THS_PC_TEMP_PERIOD + 1) * 4096 / clkin; ~10ms > > SUN50I_H6_THS_PC_TEMP_PERIOD is a macro with an argument. So how does > this work? > > Also, related to this, I've noticed that you removed the interrupt > processing from the original driver. Without that you have to make sure > that OF contains non-zero polling-delay and polling-delay-passive. > > Nonzero values are necessary for enabling polling mode of the tz core, > otherwise tz core will not read values periodically from your driver. > > You should documment it in the DT bindings, too. Or keep the interrupt > handling for THS. If there's interrupts for this in the H6, yeah we should use them over polling. Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com