linux-kernel - Re: [PATCH v1 2/2] This patch adds a new DRM bridge driver for the Lontium LT9611C chip.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <e2c40d56-e0aa-45fa-958d-97dcf4f92a6c@kernel.org>
Date: Thu, 4 Sep 2025 13:04:28 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: 杨孙运 <yangsunyun1993@...il.com>,
 Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
Cc: syyang <syyang@...tium.com>, robh@...nel.org, krzk+dt@...nel.org,
 conor+dt@...nel.org, andrzej.hajda@...el.com, neil.armstrong@...aro.org,
 rfoss@...nel.org, Laurent.pinchart@...asonboard.com, jonas@...boo.se,
 jernej.skrabec@...il.com, devicetree@...r.kernel.org,
 dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 2/2] This patch adds a new DRM bridge driver for the
 Lontium LT9611C chip.

On 04/09/2025 12:48, 杨孙运 wrote:
>>> +
>>> +static void lt9611c_cleanup_resources(struct lt9611c *lt9611c)
>>> +{
>>> +     struct device *dev = lt9611c->dev;
>>> +
>>> +     if (lt9611c->work_inited) {
>>> +             cancel_work_sync(&lt9611c->work);
>>> +             lt9611c->work_inited = false;
>>> +             dev_err(dev, "work cancelled\n");
>>
>> Why???
>>
> ?? I don't understand.

You need to explain why that line - printing error - should be here. And
focus on "WHY" part.

> 
>>> +     }
>>> +
>>> +     if (lt9611c->bridge_added) {
>>> +             drm_bridge_remove(&lt9611c->bridge);
>>> +             lt9611c->bridge_added = false;
>>> +             dev_err(dev, "DRM bridge removed\n");
>>> +     }
>>> +
>>> +     if (lt9611c->regulators_enabled) {
>>> +             regulator_bulk_disable(ARRAY_SIZE(lt9611c->supplies), lt9611c->supplies);
>>> +             lt9611c->regulators_enabled = false;
>>> +             dev_err(dev, "regulators disabled\n");
>>> +     }
>>> +
>>> +     if (lt9611c->audio_pdev)
>>> +             lt9611c_audio_exit(lt9611c);
>>> +
>>> +     if (lt9611c->fw) {
>>
>> You definitely don't need firmware when the bridge is up and running.
>>
> The previous text has already described the working logic of the firmware.
> 
>>> +             release_firmware(lt9611c->fw);
>>> +             lt9611c->fw = NULL;
>>> +             dev_err(dev, "firmware released\n");
>>> +     }
>>> +
>>> +     if (lt9611c->dsi0_node) {
>>> +             of_node_put(lt9611c->dsi0_node);
>>> +             lt9611c->dsi0_node = NULL;
>>> +             dev_err(dev, "dsi0 node released\n");

Your driver is way, way to noisy.

Please read coding style - what does it say about driver being silent?

Best regards,
Krzysztof