diff mbox

[v2,1/5] media: cec-notifier: Get notifier by device and connector name

Message ID 1526395342-15481-2-git-send-email-narmstrong@baylibre.com (mailing list archive)
State New, archived
Headers show

Commit Message

Neil Armstrong May 15, 2018, 2:42 p.m. UTC
In non device-tree world, we can need to get the notifier by the driver
name directly and eventually defer probe if not yet created.

This patch adds a variant of the get function by using the device name
instead and will not create a notifier if not yet created.

But the i915 driver exposes at least 2 HDMI connectors, this patch also
adds the possibility to add a connector name tied to the notifier device
to form a tuple and associate different CEC controllers for each HDMI
connectors.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/media/cec/cec-notifier.c | 11 ++++++++---
 include/media/cec-notifier.h     | 27 ++++++++++++++++++++++++---
 2 files changed, 32 insertions(+), 6 deletions(-)

Comments

Hans Verkuil May 15, 2018, 3:22 p.m. UTC | #1
On 05/15/2018 04:42 PM, Neil Armstrong wrote:
> In non device-tree world, we can need to get the notifier by the driver
> name directly and eventually defer probe if not yet created.
> 
> This patch adds a variant of the get function by using the device name
> instead and will not create a notifier if not yet created.
> 
> But the i915 driver exposes at least 2 HDMI connectors, this patch also
> adds the possibility to add a connector name tied to the notifier device
> to form a tuple and associate different CEC controllers for each HDMI
> connectors.

The patch looks good, but I'm curious about this paragraph above.

Was this tested with devices with more than one HDMI output? Or only on
laptops with a single physical HDMI output? If there are two or more
outputs then I guess it is the HW designer that decides with output gets
CEC support?

Regards,

	Hans

> 
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  drivers/media/cec/cec-notifier.c | 11 ++++++++---
>  include/media/cec-notifier.h     | 27 ++++++++++++++++++++++++---
>  2 files changed, 32 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/media/cec/cec-notifier.c b/drivers/media/cec/cec-notifier.c
> index 16dffa0..dd2078b 100644
> --- a/drivers/media/cec/cec-notifier.c
> +++ b/drivers/media/cec/cec-notifier.c
> @@ -21,6 +21,7 @@ struct cec_notifier {
>  	struct list_head head;
>  	struct kref kref;
>  	struct device *dev;
> +	const char *conn;
>  	struct cec_adapter *cec_adap;
>  	void (*callback)(struct cec_adapter *adap, u16 pa);
>  
> @@ -30,13 +31,14 @@ struct cec_notifier {
>  static LIST_HEAD(cec_notifiers);
>  static DEFINE_MUTEX(cec_notifiers_lock);
>  
> -struct cec_notifier *cec_notifier_get(struct device *dev)
> +struct cec_notifier *cec_notifier_get_conn(struct device *dev, const char *conn)
>  {
>  	struct cec_notifier *n;
>  
>  	mutex_lock(&cec_notifiers_lock);
>  	list_for_each_entry(n, &cec_notifiers, head) {
> -		if (n->dev == dev) {
> +		if (n->dev == dev &&
> +		    (!conn || !strcmp(n->conn, conn))) {
>  			kref_get(&n->kref);
>  			mutex_unlock(&cec_notifiers_lock);
>  			return n;
> @@ -46,6 +48,8 @@ struct cec_notifier *cec_notifier_get(struct device *dev)
>  	if (!n)
>  		goto unlock;
>  	n->dev = dev;
> +	if (conn)
> +		n->conn = kstrdup(conn, GFP_KERNEL);
>  	n->phys_addr = CEC_PHYS_ADDR_INVALID;
>  	mutex_init(&n->lock);
>  	kref_init(&n->kref);
> @@ -54,7 +58,7 @@ struct cec_notifier *cec_notifier_get(struct device *dev)
>  	mutex_unlock(&cec_notifiers_lock);
>  	return n;
>  }
> -EXPORT_SYMBOL_GPL(cec_notifier_get);
> +EXPORT_SYMBOL_GPL(cec_notifier_get_conn);
>  
>  static void cec_notifier_release(struct kref *kref)
>  {
> @@ -62,6 +66,7 @@ static void cec_notifier_release(struct kref *kref)
>  		container_of(kref, struct cec_notifier, kref);
>  
>  	list_del(&n->head);
> +	kfree(n->conn);
>  	kfree(n);
>  }
>  
> diff --git a/include/media/cec-notifier.h b/include/media/cec-notifier.h
> index cf0add7..814eeef 100644
> --- a/include/media/cec-notifier.h
> +++ b/include/media/cec-notifier.h
> @@ -20,8 +20,10 @@ struct cec_notifier;
>  #if IS_REACHABLE(CONFIG_CEC_CORE) && IS_ENABLED(CONFIG_CEC_NOTIFIER)
>  
>  /**
> - * cec_notifier_get - find or create a new cec_notifier for the given device.
> + * cec_notifier_get_conn - find or create a new cec_notifier for the given
> + * device and connector tuple.
>   * @dev: device that sends the events.
> + * @conn: the connector name from which the event occurs
>   *
>   * If a notifier for device @dev already exists, then increase the refcount
>   * and return that notifier.
> @@ -31,7 +33,8 @@ struct cec_notifier;
>   *
>   * Return NULL if the memory could not be allocated.
>   */
> -struct cec_notifier *cec_notifier_get(struct device *dev);
> +struct cec_notifier *cec_notifier_get_conn(struct device *dev,
> +					   const char *conn);
>  
>  /**
>   * cec_notifier_put - decrease refcount and delete when the refcount reaches 0.
> @@ -85,7 +88,8 @@ void cec_register_cec_notifier(struct cec_adapter *adap,
>  			       struct cec_notifier *notifier);
>  
>  #else
> -static inline struct cec_notifier *cec_notifier_get(struct device *dev)
> +static inline struct cec_notifier *cec_notifier_get_conn(struct device *dev,
> +							 const char *conn)
>  {
>  	/* A non-NULL pointer is expected on success */
>  	return (struct cec_notifier *)0xdeadfeed;
> @@ -121,6 +125,23 @@ static inline void cec_register_cec_notifier(struct cec_adapter *adap,
>  #endif
>  
>  /**
> + * cec_notifier_get - find or create a new cec_notifier for the given device.
> + * @dev: device that sends the events.
> + *
> + * If a notifier for device @dev already exists, then increase the refcount
> + * and return that notifier.
> + *
> + * If it doesn't exist, then allocate a new notifier struct and return a
> + * pointer to that new struct.
> + *
> + * Return NULL if the memory could not be allocated.
> + */
> +static inline struct cec_notifier *cec_notifier_get(struct device *dev)
> +{
> +	return cec_notifier_get_conn(dev, NULL);
> +}
> +
> +/**
>   * cec_notifier_phys_addr_invalidate() - set the physical address to INVALID
>   *
>   * @n: the CEC notifier
>
Neil Armstrong May 15, 2018, 4:10 p.m. UTC | #2
On 15/05/2018 17:22, Hans Verkuil wrote:
> On 05/15/2018 04:42 PM, Neil Armstrong wrote:
>> In non device-tree world, we can need to get the notifier by the driver
>> name directly and eventually defer probe if not yet created.
>>
>> This patch adds a variant of the get function by using the device name
>> instead and will not create a notifier if not yet created.
>>
>> But the i915 driver exposes at least 2 HDMI connectors, this patch also
>> adds the possibility to add a connector name tied to the notifier device
>> to form a tuple and associate different CEC controllers for each HDMI
>> connectors.
> 
> The patch looks good, but I'm curious about this paragraph above.
> 
> Was this tested with devices with more than one HDMI output? Or only on
> laptops with a single physical HDMI output? If there are two or more
> outputs then I guess it is the HW designer that decides with output gets
> CEC support?

The driver exposes 2 HDMI connectors (I suspect one is connected to the USB-C alt mode mux along the DP port),
and only one connected has the CEC line connected to the Embedded Controller.

Neil

> 
> Regards,
> 
> 	Hans
> 
>>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>>  drivers/media/cec/cec-notifier.c | 11 ++++++++---
>>  include/media/cec-notifier.h     | 27 ++++++++++++++++++++++++---
>>  2 files changed, 32 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/media/cec/cec-notifier.c b/drivers/media/cec/cec-notifier.c
>> index 16dffa0..dd2078b 100644
>> --- a/drivers/media/cec/cec-notifier.c
>> +++ b/drivers/media/cec/cec-notifier.c
>> @@ -21,6 +21,7 @@ struct cec_notifier {
>>  	struct list_head head;
>>  	struct kref kref;
>>  	struct device *dev;
>> +	const char *conn;
>>  	struct cec_adapter *cec_adap;
>>  	void (*callback)(struct cec_adapter *adap, u16 pa);
>>  
>> @@ -30,13 +31,14 @@ struct cec_notifier {
>>  static LIST_HEAD(cec_notifiers);
>>  static DEFINE_MUTEX(cec_notifiers_lock);
>>  
>> -struct cec_notifier *cec_notifier_get(struct device *dev)
>> +struct cec_notifier *cec_notifier_get_conn(struct device *dev, const char *conn)
>>  {
>>  	struct cec_notifier *n;
>>  
>>  	mutex_lock(&cec_notifiers_lock);
>>  	list_for_each_entry(n, &cec_notifiers, head) {
>> -		if (n->dev == dev) {
>> +		if (n->dev == dev &&
>> +		    (!conn || !strcmp(n->conn, conn))) {
>>  			kref_get(&n->kref);
>>  			mutex_unlock(&cec_notifiers_lock);
>>  			return n;
>> @@ -46,6 +48,8 @@ struct cec_notifier *cec_notifier_get(struct device *dev)
>>  	if (!n)
>>  		goto unlock;
>>  	n->dev = dev;
>> +	if (conn)
>> +		n->conn = kstrdup(conn, GFP_KERNEL);
>>  	n->phys_addr = CEC_PHYS_ADDR_INVALID;
>>  	mutex_init(&n->lock);
>>  	kref_init(&n->kref);
>> @@ -54,7 +58,7 @@ struct cec_notifier *cec_notifier_get(struct device *dev)
>>  	mutex_unlock(&cec_notifiers_lock);
>>  	return n;
>>  }
>> -EXPORT_SYMBOL_GPL(cec_notifier_get);
>> +EXPORT_SYMBOL_GPL(cec_notifier_get_conn);
>>  
>>  static void cec_notifier_release(struct kref *kref)
>>  {
>> @@ -62,6 +66,7 @@ static void cec_notifier_release(struct kref *kref)
>>  		container_of(kref, struct cec_notifier, kref);
>>  
>>  	list_del(&n->head);
>> +	kfree(n->conn);
>>  	kfree(n);
>>  }
>>  
>> diff --git a/include/media/cec-notifier.h b/include/media/cec-notifier.h
>> index cf0add7..814eeef 100644
>> --- a/include/media/cec-notifier.h
>> +++ b/include/media/cec-notifier.h
>> @@ -20,8 +20,10 @@ struct cec_notifier;
>>  #if IS_REACHABLE(CONFIG_CEC_CORE) && IS_ENABLED(CONFIG_CEC_NOTIFIER)
>>  
>>  /**
>> - * cec_notifier_get - find or create a new cec_notifier for the given device.
>> + * cec_notifier_get_conn - find or create a new cec_notifier for the given
>> + * device and connector tuple.
>>   * @dev: device that sends the events.
>> + * @conn: the connector name from which the event occurs
>>   *
>>   * If a notifier for device @dev already exists, then increase the refcount
>>   * and return that notifier.
>> @@ -31,7 +33,8 @@ struct cec_notifier;
>>   *
>>   * Return NULL if the memory could not be allocated.
>>   */
>> -struct cec_notifier *cec_notifier_get(struct device *dev);
>> +struct cec_notifier *cec_notifier_get_conn(struct device *dev,
>> +					   const char *conn);
>>  
>>  /**
>>   * cec_notifier_put - decrease refcount and delete when the refcount reaches 0.
>> @@ -85,7 +88,8 @@ void cec_register_cec_notifier(struct cec_adapter *adap,
>>  			       struct cec_notifier *notifier);
>>  
>>  #else
>> -static inline struct cec_notifier *cec_notifier_get(struct device *dev)
>> +static inline struct cec_notifier *cec_notifier_get_conn(struct device *dev,
>> +							 const char *conn)
>>  {
>>  	/* A non-NULL pointer is expected on success */
>>  	return (struct cec_notifier *)0xdeadfeed;
>> @@ -121,6 +125,23 @@ static inline void cec_register_cec_notifier(struct cec_adapter *adap,
>>  #endif
>>  
>>  /**
>> + * cec_notifier_get - find or create a new cec_notifier for the given device.
>> + * @dev: device that sends the events.
>> + *
>> + * If a notifier for device @dev already exists, then increase the refcount
>> + * and return that notifier.
>> + *
>> + * If it doesn't exist, then allocate a new notifier struct and return a
>> + * pointer to that new struct.
>> + *
>> + * Return NULL if the memory could not be allocated.
>> + */
>> +static inline struct cec_notifier *cec_notifier_get(struct device *dev)
>> +{
>> +	return cec_notifier_get_conn(dev, NULL);
>> +}
>> +
>> +/**
>>   * cec_notifier_phys_addr_invalidate() - set the physical address to INVALID
>>   *
>>   * @n: the CEC notifier
>>
>
diff mbox

Patch

diff --git a/drivers/media/cec/cec-notifier.c b/drivers/media/cec/cec-notifier.c
index 16dffa0..dd2078b 100644
--- a/drivers/media/cec/cec-notifier.c
+++ b/drivers/media/cec/cec-notifier.c
@@ -21,6 +21,7 @@  struct cec_notifier {
 	struct list_head head;
 	struct kref kref;
 	struct device *dev;
+	const char *conn;
 	struct cec_adapter *cec_adap;
 	void (*callback)(struct cec_adapter *adap, u16 pa);
 
@@ -30,13 +31,14 @@  struct cec_notifier {
 static LIST_HEAD(cec_notifiers);
 static DEFINE_MUTEX(cec_notifiers_lock);
 
-struct cec_notifier *cec_notifier_get(struct device *dev)
+struct cec_notifier *cec_notifier_get_conn(struct device *dev, const char *conn)
 {
 	struct cec_notifier *n;
 
 	mutex_lock(&cec_notifiers_lock);
 	list_for_each_entry(n, &cec_notifiers, head) {
-		if (n->dev == dev) {
+		if (n->dev == dev &&
+		    (!conn || !strcmp(n->conn, conn))) {
 			kref_get(&n->kref);
 			mutex_unlock(&cec_notifiers_lock);
 			return n;
@@ -46,6 +48,8 @@  struct cec_notifier *cec_notifier_get(struct device *dev)
 	if (!n)
 		goto unlock;
 	n->dev = dev;
+	if (conn)
+		n->conn = kstrdup(conn, GFP_KERNEL);
 	n->phys_addr = CEC_PHYS_ADDR_INVALID;
 	mutex_init(&n->lock);
 	kref_init(&n->kref);
@@ -54,7 +58,7 @@  struct cec_notifier *cec_notifier_get(struct device *dev)
 	mutex_unlock(&cec_notifiers_lock);
 	return n;
 }
-EXPORT_SYMBOL_GPL(cec_notifier_get);
+EXPORT_SYMBOL_GPL(cec_notifier_get_conn);
 
 static void cec_notifier_release(struct kref *kref)
 {
@@ -62,6 +66,7 @@  static void cec_notifier_release(struct kref *kref)
 		container_of(kref, struct cec_notifier, kref);
 
 	list_del(&n->head);
+	kfree(n->conn);
 	kfree(n);
 }
 
diff --git a/include/media/cec-notifier.h b/include/media/cec-notifier.h
index cf0add7..814eeef 100644
--- a/include/media/cec-notifier.h
+++ b/include/media/cec-notifier.h
@@ -20,8 +20,10 @@  struct cec_notifier;
 #if IS_REACHABLE(CONFIG_CEC_CORE) && IS_ENABLED(CONFIG_CEC_NOTIFIER)
 
 /**
- * cec_notifier_get - find or create a new cec_notifier for the given device.
+ * cec_notifier_get_conn - find or create a new cec_notifier for the given
+ * device and connector tuple.
  * @dev: device that sends the events.
+ * @conn: the connector name from which the event occurs
  *
  * If a notifier for device @dev already exists, then increase the refcount
  * and return that notifier.
@@ -31,7 +33,8 @@  struct cec_notifier;
  *
  * Return NULL if the memory could not be allocated.
  */
-struct cec_notifier *cec_notifier_get(struct device *dev);
+struct cec_notifier *cec_notifier_get_conn(struct device *dev,
+					   const char *conn);
 
 /**
  * cec_notifier_put - decrease refcount and delete when the refcount reaches 0.
@@ -85,7 +88,8 @@  void cec_register_cec_notifier(struct cec_adapter *adap,
 			       struct cec_notifier *notifier);
 
 #else
-static inline struct cec_notifier *cec_notifier_get(struct device *dev)
+static inline struct cec_notifier *cec_notifier_get_conn(struct device *dev,
+							 const char *conn)
 {
 	/* A non-NULL pointer is expected on success */
 	return (struct cec_notifier *)0xdeadfeed;
@@ -121,6 +125,23 @@  static inline void cec_register_cec_notifier(struct cec_adapter *adap,
 #endif
 
 /**
+ * cec_notifier_get - find or create a new cec_notifier for the given device.
+ * @dev: device that sends the events.
+ *
+ * If a notifier for device @dev already exists, then increase the refcount
+ * and return that notifier.
+ *
+ * If it doesn't exist, then allocate a new notifier struct and return a
+ * pointer to that new struct.
+ *
+ * Return NULL if the memory could not be allocated.
+ */
+static inline struct cec_notifier *cec_notifier_get(struct device *dev)
+{
+	return cec_notifier_get_conn(dev, NULL);
+}
+
+/**
  * cec_notifier_phys_addr_invalidate() - set the physical address to INVALID
  *
  * @n: the CEC notifier