On 18/04/16 11:32, Mark Brown wrote: > On Sun, Apr 17, 2016 at 09:36:10AM +0100, Jonathan Cameron wrote: >> On 11/04/16 16:08, Crestez Dan Leonard wrote: > > Please leave blank lines between paragraphs, it makes things much easier > to read. > >>> Would it be >>> acceptable to just expand the REGMASK_* into a large or-ing of (1 << >>> MAX44000_REG_*)? Then it would be clear in the source what's going on >>> but binary will be the same. > >> Would be interesting to see but I doubt the optimized code will be that >> different, and the switch is pretty much the 'standard' way of handling >> these long register lists cleanly. > >> Often it comes down to doing things the way people expect them to >> be done as that makes review easier for a tiny possible cost in >> run time. > > You can also specify ranges of registers if the map mostly has large > blocks of contiguous registers, a switch statement tends to be easier > and is probably more efficient for most register maps. > >>>>>> + .use_single_rw = 1, >>>>>> + .cache_type = REGCACHE_FLAT, > >>>> This always seems like a good idea, but tends to cause issues. >>>> FLAT is really only meant for very high performance devices, you >>>> are probably better with something else here. If you are doing this >>>> deliberately to make the below writes actually occur, then please >>>> add a comment here. > >>> I used REGCACHE_FLAT because my device has a very small number of >>> registers and I assume it uses less memory. Honestly it would make >>> sense for regmap to include a REGCACHE_AUTO cache_type and pick the >>> cache implementation automatically based on number of registers. > >> I've fallen for that one in the past as well. AUTO would indeed >> be good if it was easy to do. > > It's extremely easy to do. Unless you've got a good reason to do > anything else you should always be using an rbtree. The core would > never select anything else. > >>> Yes. It would not work otherwise since the regmap cache is explicitly >>> initialized with my listed defaults. >>> As far as I can tell regmap_write will always write to the hardware. > >> Interesting and counter intuitive if true... > > No, if the driver asked to write then we write. If the driver wants to > do a read/modify/write cycle it should use regmap_update_bits(). > >>> If the device had a reset command I should have used that, right? >>> What is happening is that I am implementing a reset command in >>> software. > >> Not necessarily. Lots of drivers don't - but instead have their interfaces >> reflect their current state on startup. Reset's are often there to get >> the internal state of the device cleaned up if it is in an unknowable state >> rather than to get the defaults to any particular state. They are always >> read from the hardware or a known good cache when queried from userspace >> anyway. > > That's not entirely it. Doing a reset is often faster than rewriting > the entire register map and is more robust against undocumented > registers or things the driver didn't think about which means that > the behaviour is going to be more consistent. Hmm. Fair enough on the undocumented register argument... > -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
