From patchwork Mon Nov 20 20:57:34 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans Verkuil X-Patchwork-Id: 10067275 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 2B144602B7 for ; Mon, 20 Nov 2017 20:57:44 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 1CCA52948C for ; Mon, 20 Nov 2017 20:57:44 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 111BD2949D; Mon, 20 Nov 2017 20:57:44 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 4D1712948C for ; Mon, 20 Nov 2017 20:57:43 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 6DC9389FC3; Mon, 20 Nov 2017 20:57:42 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from lb2-smtp-cloud9.xs4all.net (lb2-smtp-cloud9.xs4all.net [194.109.24.26]) by gabe.freedesktop.org (Postfix) with ESMTPS id 0C5FE89F6F for ; Mon, 20 Nov 2017 20:57:40 +0000 (UTC) Received: from [192.168.2.10] ([212.251.195.8]) by smtp-cloud9.xs4all.net with ESMTPA id Gt8Ie4p1xnIXbGt8MeQNf0; Mon, 20 Nov 2017 21:57:39 +0100 From: Hans Verkuil Subject: [PATCHv2] drm: adv7511/33: Fix adv7511_cec_init() failure handling To: Arnd Bergmann References: Message-ID: <8f5c59f4-1cbf-0198-663d-2410e3826579@xs4all.nl> Date: Mon, 20 Nov 2017 21:57:34 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: Content-Language: en-US X-CMAE-Envelope: MS4wfFPO/m+7y3c4Btr8fwhbh+5bwdAXspvAFeNQHO12h7UBjxzlSRuJFQw+if5YILNw3ZWgr1mD723LBB10n6bIx+gFat9KkBYAnsuIZsl15LAPUwy7c0Zr AaBeraUn1Z2nLIF4Q/9upL0/YOqwRzBH+JS4S4aXkG9Pyah8ZT582t7NHtbQYsat96Ds3tcvl1PbHpHPESTPep1NfyIxrVJZh0knzVHCukMJ/+RQj3EAXSC1 AUbJxUqLNg2+RZCL1/ou3k5smY+LiN0uIZaCidg8To4oGL0ErXwrmKMXIZcR4jy4H9F+UsRS6SWloELiscVBc7zq7pG0FG3sd2zzV/YwwuA= Cc: David Airlie , lkml , dri-devel@lists.freedesktop.org, Hans Verkuil , Laurent Pinchart , Dan Carpenter , Bhumika Goyal X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Virus-Scanned: ClamAV using ClamSMTP If the device tree for a board did not specify a cec clock, then adv7511_cec_init would return an error, which would cause adv7511_probe() to fail and thus there is no HDMI output. There is no need to have adv7511_probe() fail if the CEC initialization fails, so just change adv7511_cec_init() to a void function. In addition, adv7511_cec_init() should just return silently if the cec clock isn't found and show a message for any other errors. An otherwise correct cleanup patch from Dan Carpenter turned this broken failure handling 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"). Based on earlier patches from Arnd and John. Reported-by: Naresh Kamboju Cc: Xinliang Liu Cc: Dan Carpenter Cc: Sean Paul Cc: Archit Taneja Cc: John Stultz 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: Hans Verkuil Tested-by: Hans Verkuil Tested-by: John Stultz --- This rework of Arnd and John's patches goes a bit further and makes cec_init a void function and just silently exits if there is no cec clock defined in the dts. I'm sure that's the reason why the kirin board failed on this. BTW: if the kirin board DOES support cec, then it would be nice if it can be hooked up in the dts! Tested with my Dragonboard and Renesas Koelsch board. Change since my previous RFC PATCH: - added static inline adv7511_cec_init to avoid #ifdef in the probe function as suggested by John Stultz. Regards, Hans --- diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h b/drivers/gpu/drm/bridge/adv7511/adv7511.h index 543a5eb91624..16051bfa5578 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h @@ -374,9 +374,17 @@ struct adv7511 { }; #ifdef CONFIG_DRM_I2C_ADV7511_CEC -int adv7511_cec_init(struct device *dev, struct adv7511 *adv7511, - unsigned int offset); +void adv7511_cec_init(struct device *dev, struct adv7511 *adv7511, + unsigned int offset); void adv7511_cec_irq_process(struct adv7511 *adv7511, unsigned int irq1); +#else +static inline void adv7511_cec_init(struct device *dev, + struct adv7511 *adv7511, + unsigned int offset) +{ + regmap_write(adv7511->regmap, ADV7511_REG_CEC_CTRL + offset, + ADV7511_CEC_CTRL_POWER_DOWN); +} #endif #ifdef CONFIG_DRM_I2C_ADV7533 diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_cec.c b/drivers/gpu/drm/bridge/adv7511/adv7511_cec.c index b33d730e4d73..c1cd471d31fa 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7511_cec.c +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_cec.c @@ -300,18 +300,20 @@ static int adv7511_cec_parse_dt(struct device *dev, struct adv7511 *adv7511) return 0; } -int adv7511_cec_init(struct device *dev, struct adv7511 *adv7511, - unsigned int offset) +void adv7511_cec_init(struct device *dev, struct adv7511 *adv7511, + unsigned int offset) { int ret = adv7511_cec_parse_dt(dev, adv7511); if (ret) - return ret; + goto disable_cec; adv7511->cec_adap = cec_allocate_adapter(&adv7511_cec_adap_ops, adv7511, dev_name(dev), CEC_CAP_DEFAULTS, ADV7511_MAX_ADDRS); - if (IS_ERR(adv7511->cec_adap)) - return PTR_ERR(adv7511->cec_adap); + if (IS_ERR(adv7511->cec_adap)) { + ret = PTR_ERR(adv7511->cec_adap); + goto fail; + } regmap_write(adv7511->regmap, ADV7511_REG_CEC_CTRL + offset, 0); /* cec soft reset */ @@ -329,9 +331,15 @@ int adv7511_cec_init(struct device *dev, struct adv7511 *adv7511, ((adv7511->cec_clk_freq / 750000) - 1) << 2); ret = cec_register_adapter(adv7511->cec_adap, dev); - if (ret) { - cec_delete_adapter(adv7511->cec_adap); - adv7511->cec_adap = NULL; - } - return ret; + if (!ret) + return; + cec_delete_adapter(adv7511->cec_adap); + adv7511->cec_adap = NULL; + +fail: + dev_info(dev, "Initializing CEC failed with error %d, disabling CEC\n", + ret); +disable_cec: + regmap_write(adv7511->regmap, ADV7511_REG_CEC_CTRL + offset, + ADV7511_CEC_CTRL_POWER_DOWN); } diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index 3a33075dbb22..2eb465827bcd 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c @@ -1200,15 +1200,7 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id) adv7511_audio_init(dev, adv7511); offset = adv7511->type == ADV7533 ? ADV7533_REG_CEC_OFFSET : 0; - -#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); -#endif + adv7511_cec_init(dev, adv7511, offset); return 0;