lkml.org 
[lkml]   [2017]   [Nov]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[RFC][PATCH] drm: adv7511/33: Fix adv7511_cec_init() failure handling
    Date
    From: Arnd Bergmann <arnd@arndb.de>

    An otherwise correct cleanup patch from Dan Carpenter turned a broken
    failure handling from a feature patch by Hans Verkuil into a kernel
    Oops, so bisection points to commit 7af35b0addbc ("drm/kirin: Checking
    for IS_ERR() instead of NULL") rather than 3b1b975003e4 ("drm:
    adv7511/33: add HDMI CEC support").

    I've managed to piece together several partial problems, though
    I'm still struggling with the bigger picture:

    adv7511_probe() registers a drm_bridge structure that was allocated
    with devm_kzalloc(). It calls adv7511_cec_init(), which fails for an
    unknown reason, which in turn triggers the registered structure to be
    removed.

    Elsewhere, kirin_drm_platform_probe() gets called, which calls
    of_graph_get_remote_node(), and that returns NULL. Before Dan's
    patch we would go on with a NULL pointer here and register that,
    now kirin_drm_platform_probe() fails with -ENODEV.

    In a third driver, dsi_parse_dt() calls drm_of_find_panel_or_bridge(),
    which after not finding a panel goes on to call of_drm_find_bridge(),
    and that crashes due to the earlier list corruption.

    This addresses the first issue by making sure that adv7511_probe()
    does not completely fail when the adv7511_cec_init() function fails,
    and instead we just disable the CEC feature. This avoids having the
    driver entirely fail to load if just the CEC initialization fails.

    Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org>
    Cc: Xinliang Liu <xinliang.liu@linaro.org>
    Cc: Dan Carpenter <dan.carpenter@oracle.com>
    Cc: Sean Paul <seanpaul@chromium.org>
    Cc: Hans Verkuil <hans.verkuil@cisco.com>
    Cc: Archit Taneja <architt@codeaurora.org>
    Link: https://bugs.linaro.org/show_bug.cgi?id=3345
    Link: https://lkft.validation.linaro.org/scheduler/job/48017#L3551
    Fixes: 7af35b0addbc ("drm/kirin: Checking for IS_ERR() instead of NULL")
    Fixes: 3b1b975003e4 ("drm: adv7511/33: add HDMI CEC support")
    Signed-off-by: Arnd Bergmann <arnd@arndb.de>
    [jstultz: Reworked so when adv7511_cec_init() fails, we disable the feature instead
    of disabling the entire driver, which causes graphics to not funciton]
    Signed-off-by: John Stultz <john.stultz@linaro.org>
    ---
    Just wanted to send out my rework of Arnd's patch here.
    Feedback would be welcome.

    thanks
    -john

    drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 8 ++++----
    1 file changed, 4 insertions(+), 4 deletions(-)

    diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
    index 0e14f15..939c3b9 100644
    --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
    +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
    @@ -1203,12 +1203,12 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id)

    #ifdef CONFIG_DRM_I2C_ADV7511_CEC
    ret = adv7511_cec_init(dev, adv7511, offset);
    - if (ret)
    - goto err_unregister_cec;
    #else
    - regmap_write(adv7511->regmap, ADV7511_REG_CEC_CTRL + offset,
    - ADV7511_CEC_CTRL_POWER_DOWN);
    + ret = 1;
    #endif
    + if (ret)
    + regmap_write(adv7511->regmap, ADV7511_REG_CEC_CTRL + offset,
    + ADV7511_CEC_CTRL_POWER_DOWN);

    return 0;

    --
    2.7.4
    \
     
     \ /
      Last update: 2017-11-17 17:18    [W:5.337 / U:0.056 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site