Received-SPF: pass (google.com: domain of linux-kernel+bounces-159062-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Date: Thu, 25 Apr 2024 22:10:00 +0300
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: Sui Jingfeng <sui.jingfeng@linux.dev>
Cc: Neil Armstrong <neil.armstrong@linaro.org>,
	Randy Dunlap <rdunlap@infradead.org>,
	dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	Jessica Zhang <quic_jesszhan@quicinc.com>,
	Sam Ravnborg <sam@ravnborg.org>,
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Maxime Ripard <mripard@kernel.org>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	David Airlie <airlied@gmail.com>, Daniel Vetter <daniel@ffwll.ch>
Subject: Re: [v1,1/3] drm/panel: ili9341: Correct use of device property APIs
Message-ID: <ZiqqiAztCaiAgI8e@smile.fi.intel.com>
References: <20240425142706.2440113-2-andriy.shevchenko@linux.intel.com>
 <c2d41916-0b6c-43b5-98eb-514eb0511f84@linux.dev>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <c2d41916-0b6c-43b5-98eb-514eb0511f84@linux.dev>
Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo

On Fri, Apr 26, 2024 at 02:08:16AM +0800, Sui Jingfeng wrote:
> On 2024/4/25 22:26, Andy Shevchenko wrote:
> > It seems driver missed the point of proper use of device property APIs.
> > Correct this by updating headers and calls respectively.
> 
> You are using the 'seems' here exactly saying that you are not 100% sure.
> 
> Please allow me to tell you the truth: This patch again has ZERO effect.
> It fix nothing. And this patch is has the risks to be wrong.

Huh?! Really, stop commenting the stuff you do not understand.

> Simple because the "ili9341_probe() ---> ili9341_dbi_prob()" code path
> is DT dependent.
> 
> First of all, the devm_of_find_backlight() is called in ili9341_dbi_probe()
> under *non-DT* environment, devm_of_find_backlight() is just a just a
> no-op and will return NULL. NULL is not an error code, so ili9341_dbi_probe()
> won't rage quit. But the several side effect is that the backlight will
> NOT works at all.

Is it a problem?

> It is actually considered as fatal bug for *panels* if the backlight of
> it is not light up, at least the brightness of *won't* be able to adjust.
> What's worse, if there is no sane platform setup code at the firmware
> or boot loader stage to set a proper initial state. The screen is complete
> dark. Even though the itself panel is refreshing framebuffers, it can not
> be seen by human's eye. Simple because of no backlight.

Can you imagine that I may have different hardware that considered
this is non-fatal error?

> Second, the ili9341_dbi_probe() requires additional device properties to
> be able to works very well on the rotation screen case. See the calling
> of "device_property_read_u32(dev, "rotation", &rotation)" in
> ili9341_dbi_probe() function.

Yes, exactly, and how does it object the purpose of this patch?

> Combine with those two factors, it is actually can conclude that the
> panel-ilitek-ili9394 driver has the *implicit* dependency on 'OF'.
> Removing the 'OF' dependency from its Kconfig just trigger the
> leakage of such risks.

What?!

> My software node related patches can help to reduce part of the potential
> risks, but it still need some extra work. And it is not landed yet.

Your patch has nothing to do with this series.

-- 
With Best Regards,
Andy Shevchenko