linux-kernel - Re: [PATCH v4 3/6] drm: Add driver for Solomon SSD130x OLED displays

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: <YgaFc++EP3/hv+iA@smile.fi.intel.com>
Date:   Fri, 11 Feb 2022 17:49:07 +0200
From:   Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To:     Javier Martinez Canillas <javierm@...hat.com>
Cc:     linux-kernel@...r.kernel.org, linux-fbdev@...r.kernel.org,
        Geert Uytterhoeven <geert@...ux-m68k.org>,
        Maxime Ripard <maxime@...no.tech>,
        Daniel Vetter <daniel.vetter@...ll.ch>,
        dri-devel@...ts.freedesktop.org,
        Thomas Zimmermann <tzimmermann@...e.de>,
        Sam Ravnborg <sam@...nborg.org>,
        Noralf Trønnes <noralf@...nnes.org>,
        Daniel Vetter <daniel@...ll.ch>,
        David Airlie <airlied@...ux.ie>,
        Lee Jones <lee.jones@...aro.org>,
        Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Maxime Ripard <mripard@...nel.org>,
        Thierry Reding <thierry.reding@...il.com>,
        Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>, linux-pwm@...r.kernel.org
Subject: Re: [PATCH v4 3/6] drm: Add driver for Solomon SSD130x OLED displays

On Fri, Feb 11, 2022 at 01:05:57PM +0100, Javier Martinez Canillas wrote:
> On 2/11/22 12:33, Andy Shevchenko wrote:
> > On Fri, Feb 11, 2022 at 10:19:24AM +0100, Javier Martinez Canillas wrote:

...

> >> + * Helper to write command (SSD130X_COMMAND). The fist variadic argument
> >> + * is the command to write and the following are the command options.
> > 
> > This is not correct explanation. Please, rephrase to show that _each_ of the
> > options is sent with a preceding command.
> >
> 
> It's a correct explanation IMO from the caller point of view. The first argument
> is the command sent (i.e: SSD130X_SET_ADDRESS_MODE) and the next ones are the
> the command options (i.e: SSD130X_SET_ADDRESS_MODE_HORIZONTAL).
> 
> The fact that each command and options are preceding with a SSD130X_COMMAND
> value is part of the protocol of the device and a detail that's abstracted
> away by this helper function to the callers.

My previous suggestion about bulk transaction was purely based on this
(misinterpreted) description. Can we make sure somehow that another reader
don't trap into the same?

...


> >> +	bl = devm_backlight_device_register(dev, dev_name(dev), dev, ssd130x,
> >> +					    &ssd130xfb_bl_ops, NULL);
> >> +	if (IS_ERR(bl)) {
> > 
> >> +		ret = PTR_ERR(bl);
> >> +		dev_err_probe(dev, ret, "Unable to register backlight device\n");
> >> +		return ERR_PTR(ret);
> > 
> > 		dev_err_probe(dev, PTR_ERR(bl), "Unable to register backlight device\n");
> > 		return bl;
> > 
> > ?
> 
> No, because this function's return value is a struct ssd130x_device pointer,
> not a struct backlight_device pointer.

	return ERR_CAST(bl);

-- 
With Best Regards,
Andy Shevchenko