Re: [PATCH v10 22/30] rcar-vin: add group allocator functions

2018-02-13 Thread Laurent Pinchart
Hi Niklas,

Thank you for the patch.

On Monday, 29 January 2018 18:34:27 EET Niklas Söderlund wrote:
> In media controller mode all VIN instances needs to be part of the same
> media graph. There is also a need for each VIN instance to know about
> and in some cases be able to communicate with other VIN instances.
> 
> Add an allocator framework where the first VIN instance to be probed
> creates a shared data structure and registers a media device.
> Consecutive VINs insert themself into the global group.
> 
> Signed-off-by: Niklas Söderlund 
> ---
>  drivers/media/platform/rcar-vin/rcar-core.c | 177 -
>  drivers/media/platform/rcar-vin/rcar-vin.h  |  31 +
>  2 files changed, 206 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c
> b/drivers/media/platform/rcar-vin/rcar-core.c index
> 0c6960756c33f86c..4a64df5019ce45f7 100644
> --- a/drivers/media/platform/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> @@ -20,12 +20,177 @@
>  #include 
>  #include 
>  #include 
> +#include 
> 
>  #include 
>  #include 
> 
>  #include "rcar-vin.h"
> 
> +/* 
> + * Gen3 CSI2 Group Allocator
> + */
> +
> +/* FIXME:  This should if we find a system that supports more
> + * then one group for the whole system be replaced with a linked

s/then/than/

> + * list of groups. And eventually all of this should be replaced
> + * with a global device allocator API.
> + *
> + * But for now this works as on all supported systems there will
> + * be only one group for all instances.
> + */
> +
> +static DEFINE_MUTEX(rvin_group_lock);
> +static struct rvin_group *rvin_group_data;
> +
> +static void rvin_group_cleanup(struct rvin_group *group)
> +{
> + media_device_unregister(&group->mdev);
> + media_device_cleanup(&group->mdev);
> + mutex_destroy(&group->lock);
> +}
> +
> +static int rvin_group_init(struct rvin_group *group, struct rvin_dev *vin)
> +{
> + struct media_device *mdev = &group->mdev;
> + const struct of_device_id *match;
> + struct device_node *np;
> + int ret;
> +
> + mutex_init(&group->lock);
> +
> + /* Count number of VINs in the system */
> + group->count = 0;
> + for_each_matching_node(np, vin->dev->driver->of_match_table)
> + if (of_device_is_available(np))
> + group->count++;
> +
> + vin_dbg(vin, "found %u enabled VIN's in DT", group->count);
> +
> + mdev->dev = vin->dev;
> +
> + match = of_match_node(vin->dev->driver->of_match_table,
> +   vin->dev->of_node);
> +
> + strlcpy(mdev->driver_name, KBUILD_MODNAME, sizeof(mdev->driver_name));
> + strlcpy(mdev->model, match->compatible, sizeof(mdev->model));
> + snprintf(mdev->bus_info, sizeof(mdev->bus_info), "platform:%s",
> +  dev_name(mdev->dev));
> +
> + media_device_init(mdev);
> +
> + ret = media_device_register(&group->mdev);
> + if (ret)
> + rvin_group_cleanup(group);
> +
> + return ret;
> +}
> +
> +static void rvin_group_release(struct kref *kref)
> +{
> + struct rvin_group *group =
> + container_of(kref, struct rvin_group, refcount);
> +
> + mutex_lock(&rvin_group_lock);
> +
> + rvin_group_data = NULL;
> +
> + rvin_group_cleanup(group);
> +
> + kfree(group);
> +
> + mutex_unlock(&rvin_group_lock);
> +}
> +
> +static int rvin_group_get(struct rvin_dev *vin)
> +{
> + struct rvin_group *group;
> + u32 id;
> + int ret;
> +
> + /* Make sure VIN id is present and sane */
> + ret = of_property_read_u32(vin->dev->of_node, "renesas,id", &id);
> + if (ret) {
> + vin_err(vin, "%pOF: No renesas,id property found\n",
> + vin->dev->of_node);
> + return -EINVAL;
> + }
> +
> + if (id >= RCAR_VIN_NUM) {
> + vin_err(vin, "%pOF: Invalid renesas,id '%u'\n",
> + vin->dev->of_node, id);
> + return -EINVAL;
> + }

I'd move this out of this function to an OF parsing function, but we don't 
have one yet. Something to keep in mind for later.

> + /* Join or create a VIN group */
> + mutex_lock(&rvin_group_lock);
> + if (rvin_group_data) {
> + group = rvin_group_data;
> + kref_get(&group->refcount);
> + } else {
> + group = kzalloc(sizeof(*group), GFP_KERNEL);
> + if (!group) {
> + ret = -ENOMEM;
> + goto err_group;
> + }
> +
> + ret = rvin_group_init(group, vin);
> + if (ret) {
> + kfree(group);
> + vin_err(vin, "Failed to initialize group\n");
> + goto err_group;
> + }
> +
> + kref_init(&group->refcount);
> +
> + rvin_group_data = group;
> + }
> + mutex_unlock(&rvin

[PATCH v10 22/30] rcar-vin: add group allocator functions

2018-01-29 Thread Niklas Söderlund
In media controller mode all VIN instances needs to be part of the same
media graph. There is also a need for each VIN instance to know about
and in some cases be able to communicate with other VIN instances.

Add an allocator framework where the first VIN instance to be probed
creates a shared data structure and registers a media device.
Consecutive VINs insert themself into the global group.

Signed-off-by: Niklas Söderlund 
---
 drivers/media/platform/rcar-vin/rcar-core.c | 177 +++-
 drivers/media/platform/rcar-vin/rcar-vin.h  |  31 +
 2 files changed, 206 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/rcar-vin/rcar-core.c 
b/drivers/media/platform/rcar-vin/rcar-core.c
index 0c6960756c33f86c..4a64df5019ce45f7 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -20,12 +20,177 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
 
 #include "rcar-vin.h"
 
+/* 
-
+ * Gen3 CSI2 Group Allocator
+ */
+
+/* FIXME:  This should if we find a system that supports more
+ * then one group for the whole system be replaced with a linked
+ * list of groups. And eventually all of this should be replaced
+ * with a global device allocator API.
+ *
+ * But for now this works as on all supported systems there will
+ * be only one group for all instances.
+ */
+
+static DEFINE_MUTEX(rvin_group_lock);
+static struct rvin_group *rvin_group_data;
+
+static void rvin_group_cleanup(struct rvin_group *group)
+{
+   media_device_unregister(&group->mdev);
+   media_device_cleanup(&group->mdev);
+   mutex_destroy(&group->lock);
+}
+
+static int rvin_group_init(struct rvin_group *group, struct rvin_dev *vin)
+{
+   struct media_device *mdev = &group->mdev;
+   const struct of_device_id *match;
+   struct device_node *np;
+   int ret;
+
+   mutex_init(&group->lock);
+
+   /* Count number of VINs in the system */
+   group->count = 0;
+   for_each_matching_node(np, vin->dev->driver->of_match_table)
+   if (of_device_is_available(np))
+   group->count++;
+
+   vin_dbg(vin, "found %u enabled VIN's in DT", group->count);
+
+   mdev->dev = vin->dev;
+
+   match = of_match_node(vin->dev->driver->of_match_table,
+ vin->dev->of_node);
+
+   strlcpy(mdev->driver_name, KBUILD_MODNAME, sizeof(mdev->driver_name));
+   strlcpy(mdev->model, match->compatible, sizeof(mdev->model));
+   snprintf(mdev->bus_info, sizeof(mdev->bus_info), "platform:%s",
+dev_name(mdev->dev));
+
+   media_device_init(mdev);
+
+   ret = media_device_register(&group->mdev);
+   if (ret)
+   rvin_group_cleanup(group);
+
+   return ret;
+}
+
+static void rvin_group_release(struct kref *kref)
+{
+   struct rvin_group *group =
+   container_of(kref, struct rvin_group, refcount);
+
+   mutex_lock(&rvin_group_lock);
+
+   rvin_group_data = NULL;
+
+   rvin_group_cleanup(group);
+
+   kfree(group);
+
+   mutex_unlock(&rvin_group_lock);
+}
+
+static int rvin_group_get(struct rvin_dev *vin)
+{
+   struct rvin_group *group;
+   u32 id;
+   int ret;
+
+   /* Make sure VIN id is present and sane */
+   ret = of_property_read_u32(vin->dev->of_node, "renesas,id", &id);
+   if (ret) {
+   vin_err(vin, "%pOF: No renesas,id property found\n",
+   vin->dev->of_node);
+   return -EINVAL;
+   }
+
+   if (id >= RCAR_VIN_NUM) {
+   vin_err(vin, "%pOF: Invalid renesas,id '%u'\n",
+   vin->dev->of_node, id);
+   return -EINVAL;
+   }
+
+   /* Join or create a VIN group */
+   mutex_lock(&rvin_group_lock);
+   if (rvin_group_data) {
+   group = rvin_group_data;
+   kref_get(&group->refcount);
+   } else {
+   group = kzalloc(sizeof(*group), GFP_KERNEL);
+   if (!group) {
+   ret = -ENOMEM;
+   goto err_group;
+   }
+
+   ret = rvin_group_init(group, vin);
+   if (ret) {
+   kfree(group);
+   vin_err(vin, "Failed to initialize group\n");
+   goto err_group;
+   }
+
+   kref_init(&group->refcount);
+
+   rvin_group_data = group;
+   }
+   mutex_unlock(&rvin_group_lock);
+
+   /* Add VIN to group */
+   mutex_lock(&group->lock);
+
+   if (group->vin[id]) {
+   vin_err(vin, "Duplicate renesas,id property value %u\n", id);
+   ret = -EINVAL;
+   goto err_vin;
+   }
+
+   group->vin[id] = vin;
+
+   vin->id = id;
+   vin->group = group;
+   vin->v4l2_dev.mdev = &group->mdev;