| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <9097b2a4-b6b9-5fca-e039-0a17694b1143@xs4all.nl>
Date: Tue, 21 Nov 2017 09:17:43 +0100
From: Hans Verkuil <hverkuil@...all.nl>
To: Arnd Bergmann <arnd@...db.de>
Cc: David Airlie <airlied@...ux.ie>,
lkml <linux-kernel@...r.kernel.org>,
dri-devel@...ts.freedesktop.org,
Hans Verkuil <hans.verkuil@...co.com>,
Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
Dan Carpenter <dan.carpenter@...cle.com>,
Bhumika Goyal <bhumirks@...il.com>,
John Stultz <john.stultz@...aro.org>
Subject: [PATCHv3] drm: adv7511/33: Fix adv7511_cec_init() failure handling
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 <naresh.kamboju@...aro.org>
Cc: Xinliang Liu <xinliang.liu@...aro.org>
Cc: Dan Carpenter <dan.carpenter@...cle.com>
Cc: Sean Paul <seanpaul@...omium.org>
Cc: Archit Taneja <architt@...eaurora.org>
Cc: John Stultz <john.stultz@...aro.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: Hans Verkuil <hans.verkuil@...co.com>
Tested-by: Hans Verkuil <hans.verkuil@...co.com>
---
This rework of Arnd and John's patches goes a bit further 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. Also tested what
happens when probing is deferred due to missing cec clock.
John, can you test this again?
Changes since v2:
- reverted adv7511_cec_init back to an int function for EPROBE_DEFER
- incorporated Laurent's comments
- moved the adv7511_cec_init call up in the probe function to prevent
having to remove the bridge in case of an error.
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 b4efcbabf7f7..d034b2cb5eee 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
@@ -372,9 +372,18 @@ struct adv7511 {
};
#ifdef CONFIG_DRM_I2C_ADV7511_CEC
-int adv7511_cec_init(struct device *dev, struct adv7511 *adv7511,
- unsigned int offset);
+int adv7511_cec_init(struct device *dev, struct adv7511 *adv7511);
void adv7511_cec_irq_process(struct adv7511 *adv7511, unsigned int irq1);
+#else
+static inline int adv7511_cec_init(struct device *dev, struct adv7511 *adv7511)
+{
+ unsigned int offset = adv7511->type == ADV7533 ?
+ ADV7533_REG_CEC_OFFSET : 0;
+
+ regmap_write(adv7511->regmap, ADV7511_REG_CEC_CTRL + offset,
+ ADV7511_CEC_CTRL_POWER_DOWN);
+ return 0;
+}
#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..a20a45c0b353 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_cec.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_cec.c
@@ -300,18 +300,21 @@ 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)
+int adv7511_cec_init(struct device *dev, struct adv7511 *adv7511)
{
+ unsigned int offset = adv7511->type == ADV7533 ?
+ ADV7533_REG_CEC_OFFSET : 0;
int ret = adv7511_cec_parse_dt(dev, adv7511);
if (ret)
- return ret;
+ goto err_cec_parse_dt;
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 err_cec_alloc;
+ }
regmap_write(adv7511->regmap, ADV7511_REG_CEC_CTRL + offset, 0);
/* cec soft reset */
@@ -329,9 +332,18 @@ 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)
+ goto err_cec_register;
+ return 0;
+
+err_cec_register:
+ cec_delete_adapter(adv7511->cec_adap);
+ adv7511->cec_adap = NULL;
+err_cec_alloc:
+ dev_info(dev, "Initializing CEC failed with error %d, disabling CEC\n",
+ ret);
+err_cec_parse_dt:
+ regmap_write(adv7511->regmap, ADV7511_REG_CEC_CTRL + offset,
+ ADV7511_CEC_CTRL_POWER_DOWN);
+ return ret == -EPROBE_DEFER ? ret : 0;
}
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index 0e14f1572d05..efa29db5fc2b 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -1084,7 +1084,6 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
struct device *dev = &i2c->dev;
unsigned int main_i2c_addr = i2c->addr << 1;
unsigned int edid_i2c_addr = main_i2c_addr + 4;
- unsigned int offset;
unsigned int val;
int ret;
@@ -1192,24 +1191,16 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
if (adv7511->type == ADV7511)
adv7511_set_link_config(adv7511, &link_config);
+ ret = adv7511_cec_init(dev, adv7511);
+ if (ret)
+ goto err_unregister_cec;
+
adv7511->bridge.funcs = &adv7511_bridge_funcs;
adv7511->bridge.of_node = dev->of_node;
drm_bridge_add(&adv7511->bridge);
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
-
return 0;
err_unregister_cec:
Powered by blists - more mailing lists