linux-kernel - Re: [PATCH v6 2/2] drm/panel: Add JDI LT070ME05000 WUXGA DSI Panel

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: <CACvgo537wN_8+yGHrB09U240iJObkKqy28kigUSBoct4H=RZsg@mail.gmail.com>
Date:	Sun, 19 Jun 2016 00:42:54 +0100
From:	Emil Velikov <emil.l.velikov@...il.com>
To:	Vinay Simha <simhavcs@...il.com>
Cc:	Archit Taneja <archit.taneja@...il.com>,
	Rob Clark <robdclark@...il.com>,
	Sumit Semwal <sumit.semwal@...aro.org>,
	John Stultz <john.stultz@...aro.org>,
	Thierry Reding <thierry.reding@...il.com>,
	David Airlie <airlied@...ux.ie>,
	open list <linux-kernel@...r.kernel.org>,
	"open list:DRM PANEL DRIVERS" <dri-devel@...ts.freedesktop.org>
Subject: Re: [PATCH v6 2/2] drm/panel: Add JDI LT070ME05000 WUXGA DSI Panel

On 18 June 2016 at 07:59, Vinay Simha <simhavcs@...il.com> wrote:
> On Sat, Jun 18, 2016 at 5:58 AM, Emil Velikov <emil.l.velikov@...il.com> wrote:
>> Hi Vinay,
>>
>> On 17 June 2016 at 19:23, Vinay Simha BN <simhavcs@...il.com> wrote:
>>
>>> v6:
>>>  * emil review comments incorporated
>>>    PANEL_NUM_REGULATORS dropped, return ret added at necessary
>>>    places, if checks dropped for backlight and gpios
>>
>> Looks like some of my suggestions went below the radar. Did you miss
>> them or I've I lost the plot somewhere ? In case of the latter don't
>> be shy to point out :-)
>>
>>
>>> +struct jdi_panel {
>>> +       struct drm_panel base;
>>> +       struct mipi_dsi_device *dsi;
>>> +
>>> +       struct regulator_bulk_data supplies[3];
>>> +
>> struct regulator_bulk_data supplies[ARRAY_SIZE(regulator_names)];
>>
>>
>>> +static int jdi_panel_off(struct jdi_panel *jdi)
>>> +{
>>> +       struct mipi_dsi_device *dsi = jdi->dsi;
>>> +       int ret;
>>> +
>>> +       dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
>>> +
>>> +       ret = mipi_dsi_dcs_set_display_off(dsi);
>>> +       if (ret < 0)
>>> +               return ret;
>>> +
>> IMHO neither this function nor its caller jdi_panel_unprepare() should
>> stop in the middle/return prematurely.
>>
>> Or in other words - one should change the function return type to void
>> and drop the returns.
>>
>>
>>> +static int jdi_panel_unprepare(struct drm_panel *panel)
>>> +{
>>> +       struct jdi_panel *jdi = to_jdi_panel(panel);
>>> +       struct device *dev = &jdi->dsi->dev;
>>> +       int ret;
>>> +
>>> +       if (!jdi->prepared)
>>> +               return 0;
>>> +
>>> +       ret = jdi_panel_off(jdi);
>>> +       if (ret) {
>>> +               dev_err(panel->dev, "failed to set panel off: %d\n", ret);
>>> +               return ret;
>> As suggested above, drop this return.
>>
> i can make the function void for jdi_panel_off  and drop the return,
> but i cannot make void for jdi_panel_unprepare,
> since drm_panel_prepare requires 0 or negative value.
>
Seems like I wasn't clear enough - all you want here is to drop the
spurious return.

Regards,
Emil