On Thu, Jul 02, 2020 at 04:22:34PM +0200, Andrzej Hajda wrote:
On 14.05.2020 13:30, Vincent Whitchurch wrote:
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_cec.c b/drivers/gpu/drm/bridge/adv7511/adv7511_cec.c index a20a45c0b353..ee0ed4fb67c1 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7511_cec.c +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_cec.c @@ -286,28 +286,17 @@ static const struct cec_adap_ops adv7511_cec_adap_ops = { .adap_transmit = adv7511_cec_adap_transmit, };
-static int adv7511_cec_parse_dt(struct device *dev, struct adv7511 *adv7511) -{
- adv7511->cec_clk = devm_clk_get(dev, "cec");
- if (IS_ERR(adv7511->cec_clk)) {
int ret = PTR_ERR(adv7511->cec_clk);adv7511->cec_clk = NULL;return ret;- }
- clk_prepare_enable(adv7511->cec_clk);
- adv7511->cec_clk_freq = clk_get_rate(adv7511->cec_clk);
- return 0;
-}
-int adv7511_cec_init(struct device *dev, struct adv7511 *adv7511) +void 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);
- int ret;
- if (ret)
goto err_cec_parse_dt;
if (!adv7511->cec_clk)
goto err_cec_no_clock;clk_prepare_enable(adv7511->cec_clk);
adv7511->cec_clk_freq = clk_get_rate(adv7511->cec_clk);
adv7511->cec_adap = cec_allocate_adapter(&adv7511_cec_adap_ops, adv7511, dev_name(dev), CEC_CAP_DEFAULTS, ADV7511_MAX_ADDRS);
@@ -334,7 +323,7 @@ int adv7511_cec_init(struct device *dev, struct adv7511 *adv7511) ret = cec_register_adapter(adv7511->cec_adap, dev); if (ret) goto err_cec_register;
- return 0;
return;
err_cec_register: cec_delete_adapter(adv7511->cec_adap);
@@ -342,8 +331,11 @@ int adv7511_cec_init(struct device *dev, struct adv7511 *adv7511) err_cec_alloc: dev_info(dev, "Initializing CEC failed with error %d, disabling CEC\n", ret); -err_cec_parse_dt:
- clk_disable_unprepare(adv7511->cec_clk);
- devm_clk_put(dev, adv7511->cec_clk);
- /* Ensure that adv7511_remove() doesn't attempt to disable it again. */
- adv7511->cec_clk = NULL;
Are you sure these three lines above are necessary? devm mechanism should free the clock and with failed probe remove should not be called.
This driver ignores all failures in CEC registration/initialisation and does not fail the probe for them. I find that odd, but it seems to be done like this on purpose (see commit 1b6fba458c0a2e8513 "drm/bridge: adv7511/33: Fix adv7511_cec_init() failure handling") and my patch preserves that behaviour.