Received-SPF: pass (google.com: domain of linux-kernel+bounces-78344-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240223-saradcv2-chan-mask-v1-0-84b06a0f623a@theobroma-systems.com>
In-Reply-To: <20240223-saradcv2-chan-mask-v1-0-84b06a0f623a@theobroma-systems.com>
From: Andy Shevchenko <andy.shevchenko@gmail.com>
Date: Fri, 23 Feb 2024 15:01:30 +0200
Message-ID: <CAHp75VfmsDeTP97srRJU09gA988xw68+ZHsXvXT3W_wv1HEauA@mail.gmail.com>
Subject: Re: [PATCH 0/3] iio: adc: rockchip_saradc: fix bitmasking and remove
 custom logic for getting reset
To: Quentin Schulz <foss+kernel@0leil.net>
Cc: Jonathan Cameron <jic23@kernel.org>, Lars-Peter Clausen <lars@metafoo.de>, Heiko Stuebner <heiko@sntech.de>, 
	AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>, 
	Shreeya Patel <shreeya.patel@collabora.com>, Simon Xue <xxm@rock-chips.com>, 
	Philipp Zabel <p.zabel@pengutronix.de>, Jonathan Cameron <Jonathan.Cameron@huawei.com>, 
	linux-iio@vger.kernel.org, linux-arm-kernel@lists.infradead.org, 
	linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org, 
	Quentin Schulz <quentin.schulz@theobroma-systems.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Fri, Feb 23, 2024 at 2:46=E2=80=AFPM Quentin Schulz <foss+kernel@0leil.n=
et> 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@gmail.com>

--=20
With Best Regards,
Andy Shevchenko