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
