Re: [PATCH] arm64: dts: rockchip: Fix rk3328-roc-cc sdmmcio-regulator

[Robin Murphy <robin.murphy@arm.com>]

On 05/02/2020 4:14 pm, Adam Van Ymeren wrote:
[...]
>>> Calling regmap_write seems wrong, as we end up setting all bits in the register, so this should probably be regmap_update_bits.  The top 16-bits are write-enable for the lower 16-bits, but I can't find documentation if it works to set both the write enable bit and the target bit at the same time.
>> data = (val ? BIT(bit) : 0) | BIT(bit + 16); handles setting both the
>> bit and the write bit.
> Right I saw that, I was more wondering if it's legal to set both in the
> same operation, or if the chip requires you to set the write bit, and
> then the data bit in a subsequent write.

The point of this particular hardware idiom is that the mask indicates 
which data bits to update, and both mask and data are part of a single 
write, thus there is no need for a non-atomic read-modify-write 
sequence. For example:

- register value is 0x00000000
- write 0xffffffff (mask all set, data all 1s)
- register value is now 0x0000ffff
- write 0x00090000 (mask bits 0 and 3 set, corresponding data bit values 0)
- register value is now 0x0000fff6


FWIW I've confirmed on my box that there doesn't seem to be any problem 
with the grf-gpio driver itself - setting the value to 1 or 0 from 
userspace shows up as the enable pin on the audio line driver (per the 
RK3328 reference design) going high and low respectively.

One thing I did notice, though, is that GPIO_MUTE seems to have some 
inherent coupling to the analog codec, as the value automatically goes 
high when starting to play audio, and low again when stopping (but can 
still be manually toggled in between). Thus unless there's some secret 
to disabling that behaviour then it might not be safe to enable analog 
audio on these ROC-CC boards for fear of messing up peoples' SD cards.

Robin.