linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] cec: add cec_adapter to cec_notifier_cec_adap_unregister()
@ 2019-10-04 11:04 Hans Verkuil
  2019-10-04 12:55 ` Ville Syrjälä
  2019-10-07  9:45 ` Mauro Carvalho Chehab
  0 siblings, 2 replies; 3+ messages in thread
From: Hans Verkuil @ 2019-10-04 11:04 UTC (permalink / raw)
  To: Maling list - DRI developers, Linux Media Mailing List
  Cc: Dariusz Marcinkiewicz, Ville Syrjälä,
	Mauro Carvalho Chehab

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@xs4all.nl>
---
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?
---
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;
 	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)
 {
 }


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] cec: add cec_adapter to cec_notifier_cec_adap_unregister()
  2019-10-04 11:04 [PATCH] cec: add cec_adapter to cec_notifier_cec_adap_unregister() Hans Verkuil
@ 2019-10-04 12:55 ` Ville Syrjälä
  2019-10-07  9:45 ` Mauro Carvalho Chehab
  1 sibling, 0 replies; 3+ messages in thread
From: Ville Syrjälä @ 2019-10-04 12:55 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Maling list - DRI developers, Linux Media Mailing List,
	Dariusz Marcinkiewicz, Mauro Carvalho Chehab

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@xs4all.nl>
> ---
> 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@linux.intel.com>

> ---
> 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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] cec: add cec_adapter to cec_notifier_cec_adap_unregister()
  2019-10-04 11:04 [PATCH] cec: add cec_adapter to cec_notifier_cec_adap_unregister() Hans Verkuil
  2019-10-04 12:55 ` Ville Syrjälä
@ 2019-10-07  9:45 ` Mauro Carvalho Chehab
  1 sibling, 0 replies; 3+ messages in thread
From: Mauro Carvalho Chehab @ 2019-10-07  9:45 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Maling list - DRI developers, Linux Media Mailing List,
	Dariusz Marcinkiewicz, Ville Syrjälä

Em Fri, 4 Oct 2019 13:04:24 +0200
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

> 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@xs4all.nl>
> ---
> 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?

Yeah, feel free to merge this via DRM:

Acked-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>


> ---
> 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;
>  	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)
>  {
>  }
> 



Thanks,
Mauro

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-10-07  9:45 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-10-04 11:04 [PATCH] cec: add cec_adapter to cec_notifier_cec_adap_unregister() Hans Verkuil
2019-10-04 12:55 ` Ville Syrjälä
2019-10-07  9:45 ` Mauro Carvalho Chehab

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).