Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1440522 > unrolled thread

Re: [PATCH v7 2/2] drm/panel: Add JDI LT070ME05000 WUXGA DSI Panel

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v7 2/2] drm/panel: Add JDI LT070ME05000 WUXGA DSI Panel Thierry Reding <thierry.reding@gmail.com> - 2016-07-11 14:30 +0200
    Re: [PATCH v7 2/2] drm/panel: Add JDI LT070ME05000 WUXGA DSI Panel Vinay Simha <simhavcs@gmail.com> - 2016-07-11 14:40 +0200
      Re: [PATCH v7 2/2] drm/panel: Add JDI LT070ME05000 WUXGA DSI Panel Emil Velikov <emil.l.velikov@gmail.com> - 2016-07-11 16:20 +0200

#1440522 — Re: [PATCH v7 2/2] drm/panel: Add JDI LT070ME05000 WUXGA DSI Panel

[Multipart message — attachments visible in raw view] — view raw

On Sun, Jul 03, 2016 at 11:52:11PM +0100, Emil Velikov wrote:
> On 28 June 2016 at 16:59, Vinay Simha <simhavcs@gmail.com> wrote:
> > hi,
> >
> > Any further comments or reviews?
> >
> You still haven't covered my earlier suggestions, as such I cannot
> give you a r-b :-( They are not blockers by any means, but it'll be
> up-to the maintainer to ack/pick this up.
> 
> Thierry ?

Vinay, please address Emil's comments and resend. I also see that you
stopped sending the device tree bindings patch at some point. Please do
include that when you repost as I don't seem to be able to find a recent
version anywhere.

Thierry

[toc] | [next] | [standalone]

#1440559

emil,

As you had suggested to drop the spurious returns in
jdi_panel_unprepare and drop the return itself.
But as i had mentioned earlier , we cannot drop the return function
and void for jdi_panel_unprepare , since the drm fun* requires int as
return type (drm_panel_funcs .unprepare).

please do re-point out if i had still missed anything.

fyi,
v7:
 * emil review comments incorporated
   added ARRAY_SIZE in struct, regulator_bulk_disable in poweroff,
   gpios checks dropped.
   some returns cannot be dropped, since drm panel framework return
   type required.

On Mon, Jul 11, 2016 at 5:50 PM, Thierry Reding
<thierry.reding@gmail.com> wrote:
> On Sun, Jul 03, 2016 at 11:52:11PM +0100, Emil Velikov wrote:
>> On 28 June 2016 at 16:59, Vinay Simha <simhavcs@gmail.com> wrote:
>> > hi,
>> >
>> > Any further comments or reviews?
>> >
>> You still haven't covered my earlier suggestions, as such I cannot
>> give you a r-b :-( They are not blockers by any means, but it'll be
>> up-to the maintainer to ack/pick this up.
>>
>> Thierry ?
>
> Vinay, please address Emil's comments and resend. I also see that you
> stopped sending the device tree bindings patch at some point. Please do
> include that when you repost as I don't seem to be able to find a recent
> version anywhere.
>
> Thierry



-- 
Regards,

Vinay Simha.B.N.

[toc] | [prev] | [next] | [standalone]

#1440644

On 11 July 2016 at 13:31, Vinay Simha <simhavcs@gmail.com> wrote:
> emil,
>
> As you had suggested to drop the spurious returns in
> jdi_panel_unprepare and drop the return itself.
> But as i had mentioned earlier , we cannot drop the return function
> and void for jdi_panel_unprepare , since the drm fun* requires int as
> return type (drm_panel_funcs .unprepare).
>
> please do re-point out if i had still missed anything.
>
Simple steps:
 - forget/ignore anything you know about the driver for a moment.
 - note: existing DRM interfaces cannot be changed.
 - teardown path(s) should _not_ return prematurely. be that within
the function itself or any of the functions that it uses.

Now put the above into practise:
 - jdi_panel_unprepare, should _not_ return if jdi_panel_off fails.
 - similarly, _everything_ in jdi_panel_off should be executed, hence
one can drop the return type all together.
 - bonus points for inlining the {2,4} line helpers
jdi_panel_{on,off}. don't bother with this if it makes things too
complex/confusing from your POV.

If the above is still ambiguous, just copy/pasta from panel-sharp-lq101r1sx01.c.

Regards,
Emil

P.S. Please avoid top-posting where possible.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web