On Fri, Oct 04, 2019 at 01:04:24PM +0200, Hans Verkuil wrote:
> It is possible for one HDMI connector to have multiple CEC adapters. The
> typical real-world scenario is that where one adapter is used when the device
> is in standby, and one that's better/smarter when the device is powered up.
>
> The cec-notifier changes were made with that in mind, but I missed that in
> order to support this you need to tell cec_notifier_cec_adap_unregister()
> which adapter you are unregistering from the notifier.
>
> Add this additional argument. It is currently unused, but once all drivers
> use this, the CEC core will be adapted for these use-cases.
>
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xxxxxxxxx>
> ---
> This patch should go in via the drm subsystem since all CEC adapters in the
> drm subsystem have been converted to use cec_notifier_cec_adap_unregister().
> The media subsystem still has older drm drivers that weren't converted to use
> cec_notifier_cec_adap_unregister().
>
> This will only be a problem if a new CEC adapter driver is added to the media
> subsystem for v5.5, but I am not aware of any plans for that. Should it happen,
> then that just means that the media subsystem needs to resolve a fairly trivial
> merge conflict.
>
> Ville, Mauro, can you review/ack?
Looks harmless enough to me :)
Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c
> index ac1e001d0882..70ab4fbdc23e 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c
> @@ -285,7 +285,7 @@ static int dw_hdmi_cec_probe(struct platform_device *pdev)
>
> ret = cec_register_adapter(cec->adap, pdev->dev.parent);
> if (ret < 0) {
> - cec_notifier_cec_adap_unregister(cec->notify);
> + cec_notifier_cec_adap_unregister(cec->notify, cec->adap);
> return ret;
> }
>
> @@ -302,7 +302,7 @@ static int dw_hdmi_cec_remove(struct platform_device *pdev)
> {
> struct dw_hdmi_cec *cec = platform_get_drvdata(pdev);
>
> - cec_notifier_cec_adap_unregister(cec->notify);
> + cec_notifier_cec_adap_unregister(cec->notify, cec->adap);
> cec_unregister_adapter(cec->adap);
>
> return 0;
> diff --git a/drivers/gpu/drm/i2c/tda9950.c b/drivers/gpu/drm/i2c/tda9950.c
> index a5a75bdeb7a5..5b03fdd1eaa4 100644
> --- a/drivers/gpu/drm/i2c/tda9950.c
> +++ b/drivers/gpu/drm/i2c/tda9950.c
> @@ -465,7 +465,7 @@ static int tda9950_probe(struct i2c_client *client,
>
> ret = cec_register_adapter(priv->adap, priv->hdmi);
> if (ret < 0) {
> - cec_notifier_cec_adap_unregister(priv->notify);
> + cec_notifier_cec_adap_unregister(priv->notify, priv->adap);
> return ret;
> }
>
> @@ -482,7 +482,7 @@ static int tda9950_remove(struct i2c_client *client)
> {
> struct tda9950_priv *priv = i2c_get_clientdata(client);
>
> - cec_notifier_cec_adap_unregister(priv->notify);
> + cec_notifier_cec_adap_unregister(priv->notify, priv->adap);
> cec_unregister_adapter(priv->adap);
>
> return 0;
> diff --git a/drivers/media/cec/cec-notifier.c b/drivers/media/cec/cec-notifier.c
> index 4d82a5522072..7cf42b133dbc 100644
> --- a/drivers/media/cec/cec-notifier.c
> +++ b/drivers/media/cec/cec-notifier.c
> @@ -153,13 +153,14 @@ cec_notifier_cec_adap_register(struct device *hdmi_dev, const char *conn_name,
> }
> EXPORT_SYMBOL_GPL(cec_notifier_cec_adap_register);
>
> -void cec_notifier_cec_adap_unregister(struct cec_notifier *n)
> +void cec_notifier_cec_adap_unregister(struct cec_notifier *n,
> + struct cec_adapter *adap)
> {
> if (!n)
> return;
>
> mutex_lock(&n->lock);
> - n->cec_adap->notifier = NULL;
> + adap->notifier = NULL;
Maybe assert that the notifier and adapter know each other?
> n->cec_adap = NULL;
> n->callback = NULL;
> mutex_unlock(&n->lock);
> diff --git a/drivers/media/platform/cros-ec-cec/cros-ec-cec.c b/drivers/media/platform/cros-ec-cec/cros-ec-cec.c
> index 4a3b3810fd89..f048e8994785 100644
> --- a/drivers/media/platform/cros-ec-cec/cros-ec-cec.c
> +++ b/drivers/media/platform/cros-ec-cec/cros-ec-cec.c
> @@ -314,7 +314,8 @@ static int cros_ec_cec_probe(struct platform_device *pdev)
> return 0;
>
> out_probe_notify:
> - cec_notifier_cec_adap_unregister(cros_ec_cec->notify);
> + cec_notifier_cec_adap_unregister(cros_ec_cec->notify,
> + cros_ec_cec->adap);
> out_probe_adapter:
> cec_delete_adapter(cros_ec_cec->adap);
> return ret;
> @@ -335,7 +336,8 @@ static int cros_ec_cec_remove(struct platform_device *pdev)
> return ret;
> }
>
> - cec_notifier_cec_adap_unregister(cros_ec_cec->notify);
> + cec_notifier_cec_adap_unregister(cros_ec_cec->notify,
> + cros_ec_cec->adap);
> cec_unregister_adapter(cros_ec_cec->adap);
>
> return 0;
> diff --git a/drivers/media/platform/meson/ao-cec-g12a.c b/drivers/media/platform/meson/ao-cec-g12a.c
> index 3b39e875292e..70f875b4a01e 100644
> --- a/drivers/media/platform/meson/ao-cec-g12a.c
> +++ b/drivers/media/platform/meson/ao-cec-g12a.c
> @@ -736,7 +736,7 @@ static int meson_ao_cec_g12a_probe(struct platform_device *pdev)
> clk_disable_unprepare(ao_cec->core);
>
> out_probe_notify:
> - cec_notifier_cec_adap_unregister(ao_cec->notify);
> + cec_notifier_cec_adap_unregister(ao_cec->notify, ao_cec->adap);
>
> out_probe_adapter:
> cec_delete_adapter(ao_cec->adap);
> @@ -752,7 +752,7 @@ static int meson_ao_cec_g12a_remove(struct platform_device *pdev)
>
> clk_disable_unprepare(ao_cec->core);
>
> - cec_notifier_cec_adap_unregister(ao_cec->notify);
> + cec_notifier_cec_adap_unregister(ao_cec->notify, ao_cec->adap);
>
> cec_unregister_adapter(ao_cec->adap);
>
> diff --git a/drivers/media/platform/meson/ao-cec.c b/drivers/media/platform/meson/ao-cec.c
> index 64ed549bf012..92859a6d006f 100644
> --- a/drivers/media/platform/meson/ao-cec.c
> +++ b/drivers/media/platform/meson/ao-cec.c
> @@ -688,7 +688,7 @@ static int meson_ao_cec_probe(struct platform_device *pdev)
> clk_disable_unprepare(ao_cec->core);
>
> out_probe_notify:
> - cec_notifier_cec_adap_unregister(ao_cec->notify);
> + cec_notifier_cec_adap_unregister(ao_cec->notify, ao_cec->adap);
>
> out_probe_adapter:
> cec_delete_adapter(ao_cec->adap);
> @@ -704,7 +704,7 @@ static int meson_ao_cec_remove(struct platform_device *pdev)
>
> clk_disable_unprepare(ao_cec->core);
>
> - cec_notifier_cec_adap_unregister(ao_cec->notify);
> + cec_notifier_cec_adap_unregister(ao_cec->notify, ao_cec->adap);
> cec_unregister_adapter(ao_cec->adap);
>
> return 0;
> diff --git a/drivers/media/platform/s5p-cec/s5p_cec.c b/drivers/media/platform/s5p-cec/s5p_cec.c
> index 6ddcc35b0bbd..2a3e7ffefe0a 100644
> --- a/drivers/media/platform/s5p-cec/s5p_cec.c
> +++ b/drivers/media/platform/s5p-cec/s5p_cec.c
> @@ -239,7 +239,7 @@ static int s5p_cec_probe(struct platform_device *pdev)
> return 0;
>
> err_notifier:
> - cec_notifier_cec_adap_unregister(cec->notifier);
> + cec_notifier_cec_adap_unregister(cec->notifier, cec->adap);
>
> err_delete_adapter:
> cec_delete_adapter(cec->adap);
> @@ -250,7 +250,7 @@ static int s5p_cec_remove(struct platform_device *pdev)
> {
> struct s5p_cec_dev *cec = platform_get_drvdata(pdev);
>
> - cec_notifier_cec_adap_unregister(cec->notifier);
> + cec_notifier_cec_adap_unregister(cec->notifier, cec->adap);
> cec_unregister_adapter(cec->adap);
> pm_runtime_disable(&pdev->dev);
> return 0;
> diff --git a/drivers/media/platform/seco-cec/seco-cec.c b/drivers/media/platform/seco-cec/seco-cec.c
> index 9cd60fe1867c..54b0d51e9c55 100644
> --- a/drivers/media/platform/seco-cec/seco-cec.c
> +++ b/drivers/media/platform/seco-cec/seco-cec.c
> @@ -671,7 +671,7 @@ static int secocec_probe(struct platform_device *pdev)
> return ret;
>
> err_notifier:
> - cec_notifier_cec_adap_unregister(secocec->notifier);
> + cec_notifier_cec_adap_unregister(secocec->notifier, secocec->cec_adap);
> err_delete_adapter:
> cec_delete_adapter(secocec->cec_adap);
> err:
> @@ -692,7 +692,7 @@ static int secocec_remove(struct platform_device *pdev)
>
> dev_dbg(&pdev->dev, "IR disabled");
> }
> - cec_notifier_cec_adap_unregister(secocec->notifier);
> + cec_notifier_cec_adap_unregister(secocec->notifier, secocec->cec_adap);
> cec_unregister_adapter(secocec->cec_adap);
>
> release_region(BRA_SMB_BASE_ADDR, 7);
> diff --git a/drivers/media/platform/sti/cec/stih-cec.c b/drivers/media/platform/sti/cec/stih-cec.c
> index 8118c7365d3f..f0c73e64b586 100644
> --- a/drivers/media/platform/sti/cec/stih-cec.c
> +++ b/drivers/media/platform/sti/cec/stih-cec.c
> @@ -359,7 +359,7 @@ static int stih_cec_probe(struct platform_device *pdev)
> return 0;
>
> err_notifier:
> - cec_notifier_cec_adap_unregister(cec->notifier);
> + cec_notifier_cec_adap_unregister(cec->notifier, cec->adap);
>
> err_delete_adapter:
> cec_delete_adapter(cec->adap);
> @@ -370,7 +370,7 @@ static int stih_cec_remove(struct platform_device *pdev)
> {
> struct stih_cec *cec = platform_get_drvdata(pdev);
>
> - cec_notifier_cec_adap_unregister(cec->notifier);
> + cec_notifier_cec_adap_unregister(cec->notifier, cec->adap);
> cec_unregister_adapter(cec->adap);
>
> return 0;
> diff --git a/drivers/media/platform/tegra-cec/tegra_cec.c b/drivers/media/platform/tegra-cec/tegra_cec.c
> index a632602131f2..a99caac59f44 100644
> --- a/drivers/media/platform/tegra-cec/tegra_cec.c
> +++ b/drivers/media/platform/tegra-cec/tegra_cec.c
> @@ -409,7 +409,7 @@ static int tegra_cec_probe(struct platform_device *pdev)
> return 0;
>
> err_notifier:
> - cec_notifier_cec_adap_unregister(cec->notifier);
> + cec_notifier_cec_adap_unregister(cec->notifier, cec->adap);
> err_adapter:
> cec_delete_adapter(cec->adap);
> err_clk:
> @@ -423,7 +423,7 @@ static int tegra_cec_remove(struct platform_device *pdev)
>
> clk_disable_unprepare(cec->clk);
>
> - cec_notifier_cec_adap_unregister(cec->notifier);
> + cec_notifier_cec_adap_unregister(cec->notifier, cec->adap);
> cec_unregister_adapter(cec->adap);
>
> return 0;
> diff --git a/include/media/cec-notifier.h b/include/media/cec-notifier.h
> index f161f8a493ac..985afea1ee36 100644
> --- a/include/media/cec-notifier.h
> +++ b/include/media/cec-notifier.h
> @@ -93,8 +93,10 @@ cec_notifier_cec_adap_register(struct device *hdmi_dev, const char *conn_name,
> * cec_notifier_cec_adap_unregister - decrease refcount and delete when the
> * refcount reaches 0.
> * @n: notifier. If NULL, then this function does nothing.
> + * @adap: the cec adapter that registered this notifier.
> */
> -void cec_notifier_cec_adap_unregister(struct cec_notifier *n);
> +void cec_notifier_cec_adap_unregister(struct cec_notifier *n,
> + struct cec_adapter *adap);
>
> /**
> * cec_notifier_set_phys_addr - set a new physical address.
> @@ -160,7 +162,8 @@ cec_notifier_cec_adap_register(struct device *hdmi_dev, const char *conn_name,
> return (struct cec_notifier *)0xdeadfeed;
> }
>
> -static inline void cec_notifier_cec_adap_unregister(struct cec_notifier *n)
> +static inline void cec_notifier_cec_adap_unregister(struct cec_notifier *n,
> + struct cec_adapter *adap)
> {
> }
--
Ville Syrjälä
Intel
