[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAFQXuNa0-_kLpoy9EE3rrQvPZj0XPaSp8F6u7wYZb8TEJ72hcQ@mail.gmail.com>
Date: Thu, 4 Sep 2025 19:17:12 +0800
From: 杨孙运 <yangsunyun1993@...il.com>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>, 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.
Krzysztof Kozlowski <krzk@...nel.org> 于2025年9月4日周四 19:04写道:
>
> 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(<9611c->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.
>
I understand. Thks.
> >
> >>> + }
> >>> +
> >>> + if (lt9611c->bridge_added) {
> >>> + drm_bridge_remove(<9611c->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?
>
Thank you for your guidance. I am learning how to make the driver silent.
>
> Best regards,
> Krzysztof
Powered by blists - more mailing lists