Re: [PATCH 1/1] smiapp: I2C address is the last part of the subdev name

2014-05-27 Thread Sakari Ailus

Hi Laurent,

Laurent Pinchart wrote:

Hi Sakari,

Thank you for the patch.

On Thursday 15 May 2014 10:56:42 Sakari Ailus wrote:

The I2C address of the sensor device was in the middle of the sub-device
name and not in the end as it should have been. The smiapp sub-device names
will change from e.g. vs6555 1-0010 pixel array to vs6555 pixel array
1-0010.

Signed-off-by: Sakari Ailus sakari.ai...@linux.intel.com
---
This was already supposed to be fixed by [media] smiapp: Use I2C adapter ID
and address in the sub-device name but the I2C address indeed was in the
middle of the sub-device name and not in the end as it should have been.


I don't mind much whether the I2C bus number is in the middle or at the end of
the name. The current vs6555 1-0010 pixel array value looks good to me, as
it means the pixel array of the vs6555 1-0010 sensor in English, but I'm
fine with vs6555 pixel array 1-0010 as well.

However, as discussed privately, I think we need to make sure that
applications don't rely on a specific format for the name. Names must be
unique, but should not otherwise be parsed by applications to extract device
location information (at least in my opinion). That information should be


I agree with that; my primary motivation with this patch is consistency: 
other drivers do the same, i.e. put the bus information at the end of 
the name. Still I don't think other drivers expose multiple sub-devices, 
so the question hasn't popped up before.


--
Kind regards,

Sakari Ailus
sakari.ai...@linux.intel.com
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/1] smiapp: I2C address is the last part of the subdev name

2014-05-26 Thread Laurent Pinchart
Hi Sakari,

Thank you for the patch.

On Thursday 15 May 2014 10:56:42 Sakari Ailus wrote:
 The I2C address of the sensor device was in the middle of the sub-device
 name and not in the end as it should have been. The smiapp sub-device names
 will change from e.g. vs6555 1-0010 pixel array to vs6555 pixel array
 1-0010.
 
 Signed-off-by: Sakari Ailus sakari.ai...@linux.intel.com
 ---
 This was already supposed to be fixed by [media] smiapp: Use I2C adapter ID
 and address in the sub-device name but the I2C address indeed was in the
 middle of the sub-device name and not in the end as it should have been.

I don't mind much whether the I2C bus number is in the middle or at the end of 
the name. The current vs6555 1-0010 pixel array value looks good to me, as 
it means the pixel array of the vs6555 1-0010 sensor in English, but I'm 
fine with vs6555 pixel array 1-0010 as well.

However, as discussed privately, I think we need to make sure that 
applications don't rely on a specific format for the name. Names must be 
unique, but should not otherwise be parsed by applications to extract device 
location information (at least in my opinion). That information should be 
reported through a separate new ioctl that would report all kind of static 
information about entities. We've discussed that ioctl on and off for years 
(including with ALSA developers), it's just a matter of implementing it.

  drivers/media/i2c/smiapp/smiapp-core.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)
 
 diff --git a/drivers/media/i2c/smiapp/smiapp-core.c
 b/drivers/media/i2c/smiapp/smiapp-core.c index db3d5a6..2413d3c 100644
 --- a/drivers/media/i2c/smiapp/smiapp-core.c
 +++ b/drivers/media/i2c/smiapp/smiapp-core.c
 @@ -2543,9 +2543,9 @@ static int smiapp_registered(struct v4l2_subdev
 *subdev) }
 
   snprintf(this-sd.name,
 -  sizeof(this-sd.name), %s %d-%4.4x %s,
 -  sensor-minfo.name, i2c_adapter_id(client-adapter),
 -  client-addr, _this-name);
 +  sizeof(this-sd.name), %s %s %d-%4.4x,
 +  sensor-minfo.name, _this-name,
 +  i2c_adapter_id(client-adapter), client-addr);
 
   this-sink_fmt.width =
   sensor-limits[SMIAPP_LIMIT_X_ADDR_MAX] + 1;

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/1] smiapp: I2C address is the last part of the subdev name

2014-05-15 Thread Sakari Ailus
The I2C address of the sensor device was in the middle of the sub-device
name and not in the end as it should have been. The smiapp sub-device names
will change from e.g. vs6555 1-0010 pixel array to vs6555 pixel array
1-0010.

Signed-off-by: Sakari Ailus sakari.ai...@linux.intel.com
---
This was already supposed to be fixed by [media] smiapp: Use I2C adapter ID
and address in the sub-device name but the I2C address indeed was in the
middle of the sub-device name and not in the end as it should have been.

 drivers/media/i2c/smiapp/smiapp-core.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/i2c/smiapp/smiapp-core.c 
b/drivers/media/i2c/smiapp/smiapp-core.c
index db3d5a6..2413d3c 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -2543,9 +2543,9 @@ static int smiapp_registered(struct v4l2_subdev *subdev)
}
 
snprintf(this-sd.name,
-sizeof(this-sd.name), %s %d-%4.4x %s,
-sensor-minfo.name, i2c_adapter_id(client-adapter),
-client-addr, _this-name);
+sizeof(this-sd.name), %s %s %d-%4.4x,
+sensor-minfo.name, _this-name,
+i2c_adapter_id(client-adapter), client-addr);
 
this-sink_fmt.width =
sensor-limits[SMIAPP_LIMIT_X_ADDR_MAX] + 1;
-- 
1.8.3.2

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html