linux-kernel - Re: [PATCH v3 3/7] drm/bridge: ti-sn65dsi86: use the auxiliary device creation helper

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <1jfrki17fz.fsf@starbuckisacylon.baylibre.com>
Date: Thu, 13 Feb 2025 11:10:24 +0100
From: Jerome Brunet <jbrunet@...libre.com>
To: Doug Anderson <dianders@...omium.org>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,  Dave Ertman
 <david.m.ertman@...el.com>,  Ira Weiny <ira.weiny@...el.com>,  "Rafael J.
 Wysocki" <rafael@...nel.org>,  Stephen Boyd <sboyd@...nel.org>,  Arnd
 Bergmann <arnd@...db.de>,  Danilo Krummrich <dakr@...nel.org>,  Conor
 Dooley <conor.dooley@...rochip.com>,  Daire McNamara
 <daire.mcnamara@...rochip.com>,  Philipp Zabel <p.zabel@...gutronix.de>,
  Andrzej Hajda <andrzej.hajda@...el.com>,  Neil Armstrong
 <neil.armstrong@...aro.org>,  Robert Foss <rfoss@...nel.org>,  Laurent
 Pinchart <Laurent.pinchart@...asonboard.com>,  Jonas Karlman
 <jonas@...boo.se>,  Jernej Skrabec <jernej.skrabec@...il.com>,  Maarten
 Lankhorst <maarten.lankhorst@...ux.intel.com>,  Maxime Ripard
 <mripard@...nel.org>,  Thomas Zimmermann <tzimmermann@...e.de>,  David
 Airlie <airlied@...il.com>,  Simona Vetter <simona@...ll.ch>,  Hans de
 Goede <hdegoede@...hat.com>,  Ilpo Järvinen
 <ilpo.jarvinen@...ux.intel.com>,  "Bryan O'Donoghue"
 <bryan.odonoghue@...aro.org>,  Vladimir Kondratiev
 <vladimir.kondratiev@...ileye.com>,  Gregory CLEMENT
 <gregory.clement@...tlin.com>,  Théo Lebrun
 <theo.lebrun@...tlin.com>,
  Michael Turquette <mturquette@...libre.com>,  Abel Vesa
 <abelvesa@...nel.org>,  Peng Fan <peng.fan@....com>,  Shawn Guo
 <shawnguo@...nel.org>,  Sascha Hauer <s.hauer@...gutronix.de>,
  Pengutronix Kernel Team <kernel@...gutronix.de>,  Fabio Estevam
 <festevam@...il.com>,  Kevin Hilman <khilman@...libre.com>,  Martin
 Blumenstingl <martin.blumenstingl@...glemail.com>,
  linux-kernel@...r.kernel.org,  linux-riscv@...ts.infradead.org,
  dri-devel@...ts.freedesktop.org,  platform-driver-x86@...r.kernel.org,
  linux-mips@...r.kernel.org,  linux-clk@...r.kernel.org,
  imx@...ts.linux.dev,  linux-arm-kernel@...ts.infradead.org,
  linux-amlogic@...ts.infradead.org
Subject: Re: [PATCH v3 3/7] drm/bridge: ti-sn65dsi86: use the auxiliary
 device creation helper

On Wed 12 Feb 2025 at 08:38, Doug Anderson <dianders@...omium.org> wrote:

> Hi,
>
> On Tue, Feb 11, 2025 at 9:28 AM Jerome Brunet <jbrunet@...libre.com> wrote:
>>
>> The auxiliary device creation of this driver is simple enough to
>> use the available auxiliary device creation helper.
>>
>> Use it and remove some boilerplate code.
>>
>> Signed-off-by: Jerome Brunet <jbrunet@...libre.com>
>> ---
>>  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 84 +++++++++--------------------------
>>  1 file changed, 20 insertions(+), 64 deletions(-)
>
> Thanks for creating the helpers and getting rid of some boilerplate!
> This conflicts with commit 574f5ee2c85a ("drm/bridge: ti-sn65dsi86:
> Fix multiple instances") which is in drm-next, though. Please resolve.

Noted. this is based on v6.14-rc1 ATM

>
> Since nothing here is urgent, I would assume patch #1 would land and
> then we'd just wait until it made it to mainline before landing the
> other patches in their respective trees?

That would simplest way to handle it I think. No rush.
I'll rebase when the time comes.

>
>
>> -static int ti_sn65dsi86_add_aux_device(struct ti_sn65dsi86 *pdata,
>> -                                      struct auxiliary_device **aux_out,
>> -                                      const char *name)
>> -{
>> -       struct device *dev = pdata->dev;
>> -       struct auxiliary_device *aux;
>> -       int ret;
>> -
>> -       aux = kzalloc(sizeof(*aux), GFP_KERNEL);
>> -       if (!aux)
>> -               return -ENOMEM;
>> -
>> -       aux->name = name;
>> -       aux->dev.parent = dev;
>> -       aux->dev.release = ti_sn65dsi86_aux_device_release;
>> -       device_set_of_node_from_dev(&aux->dev, dev);
>> -       ret = auxiliary_device_init(aux);
>> -       if (ret) {
>> -               kfree(aux);
>> -               return ret;
>> -       }
>> -       ret = devm_add_action_or_reset(dev, ti_sn65dsi86_uninit_aux, aux);
>> -       if (ret)
>> -               return ret;
>> -
>> -       ret = auxiliary_device_add(aux);
>> -       if (ret)
>> -               return ret;
>> -       ret = devm_add_action_or_reset(dev, ti_sn65dsi86_delete_aux, aux);
>> -       if (!ret)
>> -               *aux_out = aux;
>
> I notice that your new code has one fewer devm_add_action_or_reset()
> than the code here which you're replacing. That means it needs to call
> "uninit" explicitly in one extra place.

... but it needs one memory allocation less ;)

> It still seems clean enough,
> though, so I don't have any real objections to the way you're doing it
> there. ;-)

Both ways are valid indeed. Just a matter of personal taste I guess.

>
> -Doug

-- 
Jerome