Messages in this thread | | | Subject | Re: [PATCH 3/3] regcache: flat: Add valid bit to this cache type | From | "Andrew F. Davis" <> | Date | Tue, 9 Jan 2018 11:47:19 -0600 |
| |
On 01/09/2018 10:24 AM, Mark Brown wrote: > On Mon, Jan 08, 2018 at 10:46:58AM -0600, Andrew F. Davis wrote: >> On 01/08/2018 10:36 AM, Mark Brown wrote: > >>> Users are supposed to ensure that the cache is fully initialized either >>> by supplying defaults or writing to all the registers. Adding reads is >>> problematic since we'd suddenly start reading from hardware which might >>> not like it. > >> This does not appear documented anywhere, and is counter to the basic >> definition of how a cache should work. > > That's a bit strong; we just used the supplied defaults from bss after > all! Feel free to contribute documentation. >
I meant it to be strong, a cache that never consults the backing storage and requires write-allocate or pre-warming to every single location is wrong, not just an odd type that requires more documentation.
>> If the hardware does not like reads then the registers should be marked >> as not readable in their regmap config, if they don't do this then they >> need fixed, not the cache scheme. > > If you have the time and enthusiasm to do the audit that ensures that > this happens then go for it; we can't just go and cause regressions for > users who have devices that are currently working fine even if it's only > by chance. >
I would like to help here, what would be involved?
A quick looks shows 42 instances of use of this cache type, of these only 8 do not define a readable table or have defaults for every register:
sound/soc/sunxi/sun8i-codec.c sound/soc/codecs/msm8916-wcd-digital.c sound/soc/atmel/atmel-classd.c drivers/pwm/pwm-fsl-ftm.c drivers/mfd/sec-core.c drivers/media/i2c/max2175.c drivers/iio/health/max30100.c drivers/i2c/muxes/i2c-mux-ltc4306.c
The are the only ones that should need to be looked at, and setting a table of all zero reg_defaults would restore the old behavior until each could be confirmed to be unaffected by this change.
>> For my device, I cannot know beforehand what is in some registers as >> they change from device to device, and if I do a read to find out it >> will *always* return 0 as this cache type is broken. Other types work as >> expected, fetching the real initial value. My only alternative is to >> turn off cache, read, turn on cache, write, then continue as before.. > > That's one of the expected mechanisms for drivers to use. > >>> Sounds like a good thing to do an audit and contribute fixes for... > >> My guess is most assumed what I did and they only got lucky as all their >> device registers were default 0 anyway. In which case the results before >> and after this patch will be the same. > > If your guess right that'll be easy. >
| |