Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1425877
| From | Emil Velikov <emil.l.velikov@gmail.com> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH v6 2/2] drm/panel: Add JDI LT070ME05000 WUXGA DSI Panel |
| Date | 2016-06-19 01:50 +0200 |
| Message-ID | <rLy9Y-2oq-5@gated-at.bofh.it> (permalink) |
| References | <rL6GJ-1zJ-27@gated-at.bofh.it> <rL6GJ-1zJ-25@gated-at.bofh.it> <rLcj8-5aI-7@gated-at.bofh.it> <rLioy-Dj-7@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
On 18 June 2016 at 07:59, Vinay Simha <simhavcs@gmail.com> wrote:
> On Sat, Jun 18, 2016 at 5:58 AM, Emil Velikov <emil.l.velikov@gmail.com> wrote:
>> Hi Vinay,
>>
>> On 17 June 2016 at 19:23, Vinay Simha BN <simhavcs@gmail.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
Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread
[PATCH v6 2/2] drm/panel: Add JDI LT070ME05000 WUXGA DSI Panel Vinay Simha BN <simhavcs@gmail.com> - 2016-06-17 20:30 +0200
Re: [PATCH v6 2/2] drm/panel: Add JDI LT070ME05000 WUXGA DSI Panel Emil Velikov <emil.l.velikov@gmail.com> - 2016-06-18 02:30 +0200
Re: [PATCH v6 2/2] drm/panel: Add JDI LT070ME05000 WUXGA DSI Panel Vinay Simha <simhavcs@gmail.com> - 2016-06-18 09:00 +0200
Re: [PATCH v6 2/2] drm/panel: Add JDI LT070ME05000 WUXGA DSI Panel Emil Velikov <emil.l.velikov@gmail.com> - 2016-06-19 01:50 +0200
csiph-web