On Fri, 23 Feb 2024 15:01:30 +0200 Andy Shevchenko <andy.shevchenko@xxxxxxxxx> wrote: > On Fri, Feb 23, 2024 at 2:46 PM Quentin Schulz <foss+kernel@xxxxxxxxx> wrote: > > > > The mask for the channel selection is incorrect as it's specified to be > > 16b wide by is actually only 4. > > > > Also, the 16 lower bits in the SARADC_CONV_CON register are write > > protected. Whatever their value is can only be written to the hardware > > block if their associated bit in the higher 16 bits is set. Considering > > that the channel bitmask is 4b wide but that we can write e.g. 0 in > > there, we shouldn't use the value shifted by 16 as a mask but rather the > > bitmask for that value shifted by 16. This is currently NOT an issue > > because the only SoC with SARADCv2 IP is the RK3588 which has a reset > > defined in the SoC DTSI. When that is the case, the reset is asserted > > before every channel conversion is started. This means the registers are > > reset so effectively, we do not need to write zeros so the wrong mask > > still works because where we should be writing zeroes, there are already > > zeroes. However, let's fix this in case there comes a day there's an SoC > > which doesn't require to reset the controller before every channel > > conversion is started. > > > > Lastly, let's use the appropriate function from the reset subsystem > > for getting an optional exclusive reset instead of rolling out our own > > logic. > > > > Those three patches should not be changing any behavior. > > Nice series, I have the comments in patch 3, but no need to resend > until Jonathan asks for. He might address that whilst applying. > > Reviewed-by: Andy Shevchenko <andy.shevchenko@xxxxxxxxx> > I've take the series through the togreg branch as we are so near the merge windows. Note (despite Linus not liking it :)) I use links in my git tags so anyone really searching for Quentin can find the discussion. I'm usually a bit lazy on cleaning out what I would consider unnecessary CC tags but given Andy comment on these I've dropped the extra one. Patches 1 and 2 marked for stable. Jonathan
