linux-kernel - Re: [PATCH] drm/mipi-dsi: Introduce macros to create mipi_dsi_*

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] [day] [month] [year] [list]

Message-ID: <CAD=FV=WiRfyteOKwhDm6-bP1yrRWqggDqqPr5=Exw6x9zzPYFg@mail.gmail.com>
Date: Mon, 22 Jul 2024 09:58:53 -0700
From: Doug Anderson <dianders@...omium.org>
To: Tejas Vipin <tejasvipin76@...il.com>
Cc: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>, maarten.lankhorst@...ux.intel.com, 
	mripard@...nel.org, tzimmermann@...e.de, airlied@...il.com, daniel@...ll.ch, 
	dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] drm/mipi-dsi: Introduce macros to create mipi_dsi_*_multi functions

Hi,

On Fri, Jul 19, 2024 at 10:19 PM Tejas Vipin <tejasvipin76@...il.com> wrote:
>
> On 7/19/24 10:29 PM, Doug Anderson wrote:
> > Hi,
> >
> > On Wed, Jul 17, 2024 at 3:07 AM Dmitry Baryshkov
> > <dmitry.baryshkov@...aro.org> wrote:
> >>
> >>>> However it might be better to go other way arround.
> >>>> Do we want for all the drivers to migrate to _multi()-kind of API? If
> >>>> so, what about renaming the multi and non-multi functions accordingly
> >>>> and making the old API a wrapper around the multi() function?
> >>>>
> >>>
> >>> I think this would be good. For the wrapper to make a multi() function
> >>> non-multi, what do you think about a macro that would just pass a
> >>> default dsi_ctx to the multi() func and return on error? In this case
> >>> it would also be good to let the code fill inline instead of generating
> >>> a whole new function imo.
> >>>
> >>> So in this scenario all the mipi dsi functions would be multi functions,
> >>> and a function could be called non-multi like MACRO_NAME(func) at the
> >>> call site.
> >>
> >> Sounds good to me. I'd suggest to wait for a day or two for further
> >> feedback / comments from other developers.
> >
> > I don't totally hate the idea of going full-multi and just having
> > macros to wrap anyone who hasn't converted, but I'd be curious how
> > much driver bloat this will cause for drivers that aren't converted.
> > Would the compiler do a good job optimizing here? Maybe we don't care
> > if we just want everyone to switch over? If nothing else, it might
> > make sense to at least keep both versions for the very generic
> > functions (mipi_dsi_generic_write_multi and
> > mipi_dsi_dcs_write_buffer_multi)
> >
> > ...oh, but wait. We probably have the non-multi versions wrap the
> > _multi ones. One of the things about the _multi functions is that they
> > are also "chatty" and print errors. They're designed for the use case
> > of a pile of init code where any error is fatal and needs to be
> > printed. I suspect that somewhere along the way someone will want to
> > be able to call these functions and not have them print errors...
> >
>
> I think what would be interesting is if we had "chatty" member as a
> part of mipi_dsi_multi_context as a check for if dev_err should run.
> That way, we could make any function not chatty. If we did this, is
> there any reason for a driver to not switch over to using the multi
> functions?

I'd be OK with that. My preference would be that "chatty" would be the
zero-initialized value just to make that slightly more efficient and
preserve existing behavior. So I guess the member would be something
like "quiet" and when 0 (false) it wouldn't be quiet.


> > Maybe going with Dmitry's suggested syntax is the best option here?
> > Depending on how others feel, we could potentially even get rid of the
> > special error message and just stringify the function name for the
> > error message?
> >
> The problem with any macro solution that defines new multi functions is
> that it creates a lot of bloat. Defining the function through macros
> might be smaller than defining them manually, but there are still twice
> as many function declarations as there would be if we went all multi.
> The generated code is still just as big as if we manually defined
> everything. I think a macro that defines functions should be more of a
> last resort if we don't have any other options.

Ah, somehow I was thinking that Dmitry's solution was conflated with
switching back to a macro. I haven't prototyped it, but I thought
Dmitry's primary complaint was that the syntax for declaring the
"_multi" function was a bit dodgy and I'd expect that using a macro
would solve that.

In any case, I'm good with keeping away from macros / bloating things.

-Doug