Re: [PATCH v6 3/5] i3c: master: svc: Add basic HDR mode support

From: Andy Shevchenko <andriy.shevchenko@intel.com>
To: Frank Li <Frank.li@nxp.com>
Cc: "Alexandre Belloni" <alexandre.belloni@bootlin.com>,
	"Miquel Raynal" <miquel.raynal@bootlin.com>,
	"Jonathan Cameron" <jic23@kernel.org>,
	"David Lechner" <dlechner@baylibre.com>,
	"Nuno Sá" <nuno.sa@analog.com>,
	"Andy Shevchenko" <andy@kernel.org>,
	"Rob Herring" <robh@kernel.org>,
	"Krzysztof Kozlowski" <krzk+dt@kernel.org>,
	"Conor Dooley" <conor+dt@kernel.org>,
	linux-i3c@lists.infradead.org, linux-kernel@vger.kernel.org,
	imx@lists.linux.dev, linux-iio@vger.kernel.org,
	joshua.yeong@starfivetech.com, devicetree@vger.kernel.org,
	"Carlos Song" <carlos.song@nxp.com>
Subject: Re: [PATCH v6 3/5] i3c: master: svc: Add basic HDR mode support
Date: Thu, 23 Oct 2025 21:25:00 +0300	[thread overview]
Message-ID: <aPpy_FNh0DB_1x-w@smile.fi.intel.com> (raw)
In-Reply-To: <aPpIkqjeWsi7xAAn@lizhi-Precision-Tower-5810>

On Thu, Oct 23, 2025 at 11:24:02AM -0400, Frank Li wrote:
> On Thu, Oct 23, 2025 at 11:29:53AM +0300, Andy Shevchenko wrote:
> > On Tue, Oct 14, 2025 at 12:40:02PM -0400, Frank Li wrote:

...

> > > +	readl_poll_timeout(master->regs + SVC_I3C_MSTATUS, reg,
> > > +			   SVC_I3C_MSTATUS_MCTRLDONE(reg), 0, 1000);
> >
> > No error checks? Why is it okay?
> > Why is the first parameter 0 while it's not an _atomic() call?
> >
> > > +	udelay(1);
> >
> > No explanations given. Also is it really need to be atomic? If not, use
> > fsleep() and it will choose the best suitable API under the hood.
> 
> It is in atomic context. I will add comments.

Not only, the call that's used in the code from iopoll.h is wrong in such a
case. Haven't you tested this with debug atomic context?

-- 
With Best Regards,
Andy Shevchenko