lkml.org 
[lkml]   [2019]   [Jun]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [v3 2/4] drm/panel: support for BOE tv101wum-nl6 wuxga dsi video mode panel
    Hi Jitao.

    Driver looks good, one detail.

    > +
    > +struct panel_desc {
    > + const struct drm_display_mode *modes;
    > + unsigned int bpc;
    > +
    > + /**
    > + * @width: width (in millimeters) of the panel's active display area
    > + * @height: height (in millimeters) of the panel's active display area
    > + */
    > + struct {
    > + unsigned int width;
    > + unsigned int height;
    > + } size;
    Maybe name these width_mm and height_mm.
    Then they have the same name as where they are copied to,
    and it is explicit documented that it is in mm.

    The extra indirection with a struct is not needed in display_mode,
    maybe drop it here too?
    > +
    > + unsigned long mode_flags;
    > + enum mipi_dsi_pixel_format format;
    > + const struct panel_init_cmd *init_cmds;
    > + unsigned int lanes;
    > +};
    > +
    ...
    > +static int boe_panel_unprepare(struct drm_panel *panel)
    > +{
    > + struct boe_panel *boe = to_boe_panel(panel);
    > + int ret;
    > +
    > + if (!boe->prepared)
    > + return 0;
    > +
    > + ret = boe_panel_off(boe);
    > + if (ret < 0) {
    > + dev_err(panel->dev, "failed to set panel off: %d\n", ret);
    > + return ret;
    > + }
    > +
    > + msleep(150);
    > + if (boe->enable_gpio)
    > + gpiod_set_value(boe->enable_gpio, 0);
    Everywhere boe->enable_gpio is used it is checked like above.
    Bot boe->enable_gpio in a mandatory property so it must be present.
    The driver error out in probe if not present, so this check seems
    redundandt?

    Everything else looks really good.

    With the above fixed / considered:
    Reviewed-by: Sam Ravnborg <sam@ravnborg.org>

    Sam

    \
     
     \ /
      Last update: 2019-06-26 09:20    [W:3.075 / U:1.080 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site