Re: [PATCH v6 3/8] tda9950: use cec_notifier_cec_adap_(un)register
On 8/13/19 1:32 PM, Russell King - ARM Linux admin wrote: > On Tue, Aug 13, 2019 at 01:02:35PM +0200, Dariusz Marcinkiewicz wrote: >> Use the new cec_notifier_cec_adap_(un)register() functions to >> (un)register the notifier for the CEC adapter. >> >> Signed-off-by: Dariusz Marcinkiewicz >> Signed-off-by: Hans Verkuil >> Tested-by: Hans Verkuil >> --- >> drivers/gpu/drm/i2c/tda9950.c | 12 ++-- >> 1 file changed, 6 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/gpu/drm/i2c/tda9950.c b/drivers/gpu/drm/i2c/tda9950.c >> index 8039fc0d83db4..a5a75bdeb7a5f 100644 >> --- a/drivers/gpu/drm/i2c/tda9950.c >> +++ b/drivers/gpu/drm/i2c/tda9950.c >> @@ -420,7 +420,8 @@ static int tda9950_probe(struct i2c_client *client, >> priv->hdmi = glue->parent; >> >> priv->adap = cec_allocate_adapter(_cec_ops, priv, "tda9950", >> - CEC_CAP_DEFAULTS, >> + CEC_CAP_DEFAULTS | >> + CEC_CAP_CONNECTOR_INFO, >>CEC_MAX_LOG_ADDRS); >> if (IS_ERR(priv->adap)) >> return PTR_ERR(priv->adap); >> @@ -457,13 +458,14 @@ static int tda9950_probe(struct i2c_client *client, >> if (ret < 0) >> return ret; >> >> -priv->notify = cec_notifier_get(priv->hdmi); >> +priv->notify = cec_notifier_cec_adap_register(priv->hdmi, NULL, >> + priv->adap); >> if (!priv->notify) >> return -ENOMEM; >> >> ret = cec_register_adapter(priv->adap, priv->hdmi); >> if (ret < 0) { >> -cec_notifier_put(priv->notify); >> +cec_notifier_cec_adap_unregister(priv->notify); >> return ret; >> } >> >> @@ -473,8 +475,6 @@ static int tda9950_probe(struct i2c_client *client, >> */ >> devm_remove_action(dev, tda9950_cec_del, priv); >> >> -cec_register_cec_notifier(priv->adap, priv->notify); >> - >> return 0; >> } >> >> @@ -482,8 +482,8 @@ static int tda9950_remove(struct i2c_client *client) >> { >> struct tda9950_priv *priv = i2c_get_clientdata(client); >> >> +cec_notifier_cec_adap_unregister(priv->notify); >> cec_unregister_adapter(priv->adap); >> -cec_notifier_put(priv->notify); > > It looks weird to have an unexpectedly different ordering of > unregistration from the registration path - normally, unregistration > is the reverse order of initialisation. > > In the initialisation path, it seems that we register the notifier > and _then_ the adapter. Here, we unregister the notifier and then > the adapter rather than what would normally be expected. Why is > this? I suspect there will be drivers created that do this the > "normal" way round, so if this is a requirement, it needs to be made > plainly obvious. It's not a requirement, it just feels better to do it in this order since cec_unregister_adapter will in general also delete the adapter (unless an application keeps the cec device open). So the order is actually: allocate_adapter, then register notifier and: unregister notifier, then unregister (and typically delete) adapter Regards, Hans > >> >> return 0; >> } >> -- >> 2.23.0.rc1.153.gdeed80330f-goog >> >> >
Re: [PATCH v6 3/8] tda9950: use cec_notifier_cec_adap_(un)register
On Tue, Aug 13, 2019 at 01:02:35PM +0200, Dariusz Marcinkiewicz wrote: > Use the new cec_notifier_cec_adap_(un)register() functions to > (un)register the notifier for the CEC adapter. > > Signed-off-by: Dariusz Marcinkiewicz > Signed-off-by: Hans Verkuil > Tested-by: Hans Verkuil > --- > drivers/gpu/drm/i2c/tda9950.c | 12 ++-- > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i2c/tda9950.c b/drivers/gpu/drm/i2c/tda9950.c > index 8039fc0d83db4..a5a75bdeb7a5f 100644 > --- a/drivers/gpu/drm/i2c/tda9950.c > +++ b/drivers/gpu/drm/i2c/tda9950.c > @@ -420,7 +420,8 @@ static int tda9950_probe(struct i2c_client *client, > priv->hdmi = glue->parent; > > priv->adap = cec_allocate_adapter(_cec_ops, priv, "tda9950", > - CEC_CAP_DEFAULTS, > + CEC_CAP_DEFAULTS | > + CEC_CAP_CONNECTOR_INFO, > CEC_MAX_LOG_ADDRS); > if (IS_ERR(priv->adap)) > return PTR_ERR(priv->adap); > @@ -457,13 +458,14 @@ static int tda9950_probe(struct i2c_client *client, > if (ret < 0) > return ret; > > - priv->notify = cec_notifier_get(priv->hdmi); > + priv->notify = cec_notifier_cec_adap_register(priv->hdmi, NULL, > + priv->adap); > if (!priv->notify) > return -ENOMEM; > > ret = cec_register_adapter(priv->adap, priv->hdmi); > if (ret < 0) { > - cec_notifier_put(priv->notify); > + cec_notifier_cec_adap_unregister(priv->notify); > return ret; > } > > @@ -473,8 +475,6 @@ static int tda9950_probe(struct i2c_client *client, >*/ > devm_remove_action(dev, tda9950_cec_del, priv); > > - cec_register_cec_notifier(priv->adap, priv->notify); > - > return 0; > } > > @@ -482,8 +482,8 @@ static int tda9950_remove(struct i2c_client *client) > { > struct tda9950_priv *priv = i2c_get_clientdata(client); > > + cec_notifier_cec_adap_unregister(priv->notify); > cec_unregister_adapter(priv->adap); > - cec_notifier_put(priv->notify); It looks weird to have an unexpectedly different ordering of unregistration from the registration path - normally, unregistration is the reverse order of initialisation. In the initialisation path, it seems that we register the notifier and _then_ the adapter. Here, we unregister the notifier and then the adapter rather than what would normally be expected. Why is this? I suspect there will be drivers created that do this the "normal" way round, so if this is a requirement, it needs to be made plainly obvious. > > return 0; > } > -- > 2.23.0.rc1.153.gdeed80330f-goog > > -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up
[PATCH v6 3/8] tda9950: use cec_notifier_cec_adap_(un)register
Use the new cec_notifier_cec_adap_(un)register() functions to (un)register the notifier for the CEC adapter. Signed-off-by: Dariusz Marcinkiewicz Signed-off-by: Hans Verkuil Tested-by: Hans Verkuil --- drivers/gpu/drm/i2c/tda9950.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i2c/tda9950.c b/drivers/gpu/drm/i2c/tda9950.c index 8039fc0d83db4..a5a75bdeb7a5f 100644 --- a/drivers/gpu/drm/i2c/tda9950.c +++ b/drivers/gpu/drm/i2c/tda9950.c @@ -420,7 +420,8 @@ static int tda9950_probe(struct i2c_client *client, priv->hdmi = glue->parent; priv->adap = cec_allocate_adapter(_cec_ops, priv, "tda9950", - CEC_CAP_DEFAULTS, + CEC_CAP_DEFAULTS | + CEC_CAP_CONNECTOR_INFO, CEC_MAX_LOG_ADDRS); if (IS_ERR(priv->adap)) return PTR_ERR(priv->adap); @@ -457,13 +458,14 @@ static int tda9950_probe(struct i2c_client *client, if (ret < 0) return ret; - priv->notify = cec_notifier_get(priv->hdmi); + priv->notify = cec_notifier_cec_adap_register(priv->hdmi, NULL, + priv->adap); if (!priv->notify) return -ENOMEM; ret = cec_register_adapter(priv->adap, priv->hdmi); if (ret < 0) { - cec_notifier_put(priv->notify); + cec_notifier_cec_adap_unregister(priv->notify); return ret; } @@ -473,8 +475,6 @@ static int tda9950_probe(struct i2c_client *client, */ devm_remove_action(dev, tda9950_cec_del, priv); - cec_register_cec_notifier(priv->adap, priv->notify); - return 0; } @@ -482,8 +482,8 @@ static int tda9950_remove(struct i2c_client *client) { struct tda9950_priv *priv = i2c_get_clientdata(client); + cec_notifier_cec_adap_unregister(priv->notify); cec_unregister_adapter(priv->adap); - cec_notifier_put(priv->notify); return 0; } -- 2.23.0.rc1.153.gdeed80330f-goog