[PATCH] v4l: subdev: Add pad config allocator and init
Add a new subdev operation to initialize a subdev pad config array, and
a helper function to allocate and initialize the array. This can be used
by bridge drivers to implement try format based on subdev pad
operations.
Signed-off-by: Laurent Pinchart laurent.pinch...@linaro.org
Acked-by: Vaibhav Hiremath vaibhav.hirem...@linaro.org
---
drivers/media/v4l2-core/v4l2-subdev.c | 19 ++-
include/media/v4l2-subdev.h | 3 +++
2 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/drivers/media/v4l2-core/v4l2-subdev.c
b/drivers/media/v4l2-core/v4l2-subdev.c
index 63596063b213..d594fe566be2 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -35,7 +35,7 @@
static int subdev_fh_init(struct v4l2_subdev_fh *fh, struct v4l2_subdev *sd)
{
#if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)
- fh-pad = kzalloc(sizeof(*fh-pad) * sd-entity.num_pads, GFP_KERNEL);
+ fh-pad = v4l2_subdev_alloc_pad_config(sd);
if (fh-pad == NULL)
return -ENOMEM;
#endif
@@ -569,6 +569,23 @@ int v4l2_subdev_link_validate(struct media_link *link)
sink, link, source_fmt, sink_fmt);
}
EXPORT_SYMBOL_GPL(v4l2_subdev_link_validate);
+
+struct v4l2_subdev_pad_config *v4l2_subdev_alloc_pad_config(struct v4l2_subdev
*sd)
+{
+ struct v4l2_subdev_pad_config *cfg;
+
+ if (!sd-entity.num_pads)
+ return NULL;
+
+ cfg = kcalloc(sd-entity.num_pads, sizeof(*cfg), GFP_KERNEL);
+ if (!cfg)
+ return NULL;
+
+ v4l2_subdev_call(sd, pad, init_cfg, cfg);
+
+ return cfg;
+}
+EXPORT_SYMBOL_GPL(v4l2_subdev_alloc_pad_config);
#endif /* CONFIG_MEDIA_CONTROLLER */
void v4l2_subdev_init(struct v4l2_subdev *sd, const struct v4l2_subdev_ops
*ops)
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index 8f5da73dacff..7860d67574f5 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -483,6 +483,8 @@ struct v4l2_subdev_pad_config {
* may be adjusted by the subdev driver to device
capabilities.
*/
struct v4l2_subdev_pad_ops {
+ void (*init_cfg)(struct v4l2_subdev *sd,
+struct v4l2_subdev_pad_config *cfg);
int (*enum_mbus_code)(struct v4l2_subdev *sd,
struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_mbus_code_enum *code);
@@ -675,6 +677,7 @@ int v4l2_subdev_link_validate_default(struct v4l2_subdev
*sd,
struct v4l2_subdev_format *source_fmt,
struct v4l2_subdev_format *sink_fmt);
int v4l2_subdev_link_validate(struct media_link *link);
+struct v4l2_subdev_pad_config *v4l2_subdev_alloc_pad_config(struct v4l2_subdev
*sd);
#endif /* CONFIG_MEDIA_CONTROLLER */
void v4l2_subdev_init(struct v4l2_subdev *sd,
const struct v4l2_subdev_ops *ops);
--
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
Re: [PATCH 2/5] v4l: flash: Make v4l2_flash_init() and v4l2_flash_release() functions always
Hi Sakari,
Thank you for the patch.
On Wednesday 20 May 2015 02:04:02 Sakari Ailus wrote:
If CONFIG_V4L2_FLASH_LED_CLASS wasn't defined, v4l2_flash_init() and
v4l2_flash_release() were empty macros. This will lead to compiler warnings
in form of unused variables if the variables are not used for other
purposes.
Instead, implement v4l2_flash_init() and v4l2_flash_release() as static
inline functions.
Signed-off-by: Sakari Ailus sakari.ai...@iki.fi
Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
---
include/media/v4l2-flash.h | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/include/media/v4l2-flash.h b/include/media/v4l2-flash.h
index 945fa08..67a2cbf 100644
--- a/include/media/v4l2-flash.h
+++ b/include/media/v4l2-flash.h
@@ -138,8 +138,16 @@ struct v4l2_flash *v4l2_flash_init(struct
led_classdev_flash *fled_cdev, void v4l2_flash_release(struct v4l2_flash
*v4l2_flash);
#else
-#define v4l2_flash_init(fled_cdev, ops, config) (NULL)
-#define v4l2_flash_release(v4l2_flash)
+static inline struct v4l2_flash *v4l2_flash_init(
+ struct led_classdev_flash *fled_cdev, const struct v4l2_flash_ops *ops,
+ struct v4l2_flash_config *config)
+{
+ return NULL;
+}
+
+static inline void v4l2_flash_release(struct v4l2_flash *v4l2_flash)
+{
+}
#endif /* CONFIG_V4L2_FLASH_LED_CLASS */
#endif /* _V4L2_FLASH_H */
--
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
Re: [PATCH 08/20] media: soc_camera pad-aware driver initialisation
Hi Hans,
On Thursday 21 May 2015 07:55:10 Hans Verkuil wrote:
On 05/20/2015 06:39 PM, William Towle wrote:
Add detection of source pad number for drivers aware of the media
controller API, so that soc_camera/rcar_vin can create device nodes
to support a driver such as adv7604.c (for HDMI on Lager) underneath.
Signed-off-by: William Towle william.to...@codethink.co.uk
Reviewed-by: Rob Taylor rob.tay...@codethink.co.uk
---
drivers/media/platform/soc_camera/rcar_vin.c |4
drivers/media/platform/soc_camera/soc_camera.c | 27 ++-
include/media/soc_camera.h |1 +
3 files changed, 31 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/soc_camera/rcar_vin.c
b/drivers/media/platform/soc_camera/rcar_vin.c index 0f67646..b4e9b43
100644
--- a/drivers/media/platform/soc_camera/rcar_vin.c
+++ b/drivers/media/platform/soc_camera/rcar_vin.c
@@ -1364,8 +1364,12 @@ static int rcar_vin_get_formats(struct
soc_camera_device *icd, unsigned int idx,
struct v4l2_mbus_framefmt *mf = fmt.format;
struct v4l2_rect rect;
struct device *dev = icd-parent;
+ struct media_pad *remote_pad;
int shift;
+ remote_pad = media_entity_remote_pad(
+ icd-vdev-entity.pads[0]);
+ fmt.pad = remote_pad-index;
This won't work if CONFIG_MEDIA_CONTROLLER isn't defined. All these media
calls would all have to be under #ifdef CONFIG_MEDIA_CONTROLLER.
Unfortunately, if it is not defined, then you still have no way of finding
the source pad.
Laurent, do you think if it would make sense to add a new subdev core op
that will return the default source pad (I'm saying 'default' in case there
are more) of a subdev? That way it can be used in non-MC drivers. We never
needed the source pad before, but now we do, and this op only needs to be
implemented if the default source pad != 0.
I'm not too fond of that. Is there something wrong with the method implemented
in this patch ? Is the dependency on CONFIG_MEDIA_CONTROLLER an issue ?
ret = v4l2_subdev_call(sd, pad, get_fmt, NULL, fmt);
if (ret 0)
return ret;
diff --git a/drivers/media/platform/soc_camera/soc_camera.c
b/drivers/media/platform/soc_camera/soc_camera.c index d708df4..126d645
100644
--- a/drivers/media/platform/soc_camera/soc_camera.c
+++ b/drivers/media/platform/soc_camera/soc_camera.c
@@ -1293,6 +1293,7 @@ static int soc_camera_probe_finish(struct
soc_camera_device *icd)
.which = V4L2_SUBDEV_FORMAT_ACTIVE,
};
struct v4l2_mbus_framefmt *mf = fmt.format;
+ int src_pad_idx = -1;
int ret;
sd-grp_id = soc_camera_grp_id(icd);
@@ -1311,7 +1312,25 @@ static int soc_camera_probe_finish(struct
soc_camera_device *icd)
}
/* At this point client .probe() should have run already */
- ret = soc_camera_init_user_formats(icd);
+ ret = media_entity_init(icd-vdev-entity, 1, icd-pad, 0);
+ if (!ret) {
+ for (src_pad_idx = 0; src_pad_idx sd-entity.num_pads;
+ src_pad_idx++)
+ if (sd-entity.pads[src_pad_idx].flags
+ == MEDIA_PAD_FL_SOURCE)
+ break;
+
+ if (src_pad_idx sd-entity.num_pads) {
+ if (!media_entity_create_link(
+ icd-vdev-entity, 0,
+ sd-entity, src_pad_idx,
+ MEDIA_LNK_FL_IMMUTABLE |
+ MEDIA_LNK_FL_ENABLED)) {
+ ret = soc_camera_init_user_formats(icd);
+ }
+ }
+ }
+
if (ret 0)
goto eusrfmt;
@@ -1322,6 +1341,7 @@ static int soc_camera_probe_finish(struct
soc_camera_device *icd)
goto evidstart;
/* Try to improve our guess of a reasonable window format */
+ fmt.pad = src_pad_idx;
if (!v4l2_subdev_call(sd, pad, get_fmt, NULL, fmt)) {
icd-user_width = mf-width;
icd-user_height= mf-height;
@@ -1335,6 +1355,7 @@ static int soc_camera_probe_finish(struct
soc_camera_device *icd)
evidstart:
soc_camera_free_user_formats(icd);
eusrfmt:
+ media_entity_cleanup(icd-vdev-entity);
soc_camera_remove_device(icd);
return ret;
@@ -1856,6 +1877,10 @@ static int soc_camera_remove(struct
soc_camera_device *icd)
if (icd-num_user_formats)
soc_camera_free_user_formats(icd);
+ if (icd-vdev-entity.num_pads) {
+ media_entity_cleanup(icd-vdev-entity);
+ }
+
if (icd-clk) {
/* For the synchronous case */
v4l2_clk_unregister(icd-clk);
diff --git
Re: [PATCH 09/20] media: rcar_vin: Use correct pad number in try_fmt
Hi Hans,
On Thursday 21 May 2015 07:58:18 Hans Verkuil wrote:
On 05/20/2015 06:39 PM, William Towle wrote:
From: Rob Taylor rob.tay...@codethink.co.uk
Fix rcar_vin_try_fmt to use the correct pad number when calling the
subdev set_fmt. Previously pad number 0 was always used, resulting in
EINVAL if the subdev cares about the pad number (e.g. ADV7612).
Signed-off-by: William Towle Taylor rob.tay...@codethink.co.uk
Reviewed-by: Rob Taylor rob.tay...@codethink.co.uk
---
drivers/media/platform/soc_camera/rcar_vin.c | 29 +---
1 file changed, 19 insertions(+), 10 deletions(-)
diff --git a/drivers/media/platform/soc_camera/rcar_vin.c
b/drivers/media/platform/soc_camera/rcar_vin.c index b4e9b43..571ab20
100644
--- a/drivers/media/platform/soc_camera/rcar_vin.c
+++ b/drivers/media/platform/soc_camera/rcar_vin.c
@@ -1707,12 +1707,13 @@ static int rcar_vin_try_fmt(struct
soc_camera_device *icd,
const struct soc_camera_format_xlate *xlate;
struct v4l2_pix_format *pix = f-fmt.pix;
struct v4l2_subdev *sd = soc_camera_to_subdev(icd);
- struct v4l2_subdev_pad_config pad_cfg;
+ struct v4l2_subdev_pad_config pad_cfg[sd-entity.num_pads];
Same problem: this relies on the presence of CONFIG_MEDIA_CONTROLLER.
This array can also get large which is bad when it is on the stack.
Laurent, I remember that you had plans to add an op that would allocate
and initialize this for you. Any progress on that?
You have it in your mailbox :-)
struct v4l2_subdev_format format = {
.which = V4L2_SUBDEV_FORMAT_TRY,
};
struct v4l2_mbus_framefmt *mf = format.format;
__u32 pixfmt = pix-pixelformat;
+ struct media_pad *remote_pad;
int width, height;
int ret;
@@ -1744,17 +1745,24 @@ static int rcar_vin_try_fmt(struct
soc_camera_device *icd,
mf-code = xlate-code;
mf-colorspace = pix-colorspace;
- ret = v4l2_device_call_until_err(sd-v4l2_dev,
soc_camera_grp_id(icd),
-pad, set_fmt, pad_cfg, format);
+ remote_pad = media_entity_remote_pad(
+ icd-vdev-entity.pads[0]);
+ format.pad = remote_pad-index;
+
+ ret = v4l2_device_call_until_err(sd-v4l2_dev,
+ soc_camera_grp_id(icd), pad,
+ set_fmt, pad_cfg,
+ format);
if (ret 0)
return ret;
- /* Adjust only if VIN cannot scale */
- if (pix-width mf-width * 2)
- pix-width = mf-width * 2;
- if (pix-height mf-height * 3)
- pix-height = mf-height * 3;
-
+ /* In case the driver has adjusted 'fmt' to match the
+* resolution of the live stream, 'pix' needs to pass this
+* change out so that the buffer userland creates for the
+* captured image/video has these dimensions
+*/
+ pix-width = mf-width;
+ pix-height = mf-height;
pix-field = mf-field;
pix-colorspace = mf-colorspace;
@@ -1769,9 +1777,10 @@ static int rcar_vin_try_fmt(struct
soc_camera_device *icd,
*/
mf-width = VIN_MAX_WIDTH;
mf-height = VIN_MAX_HEIGHT;
+ format.pad = remote_pad-index;
ret = v4l2_device_call_until_err(sd-v4l2_dev,
soc_camera_grp_id(icd),
-pad, set_fmt, pad_cfg,
+pad, set_fmt, pad_cfg,
format);
if (ret 0) {
dev_err(icd-parent,
--
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
Re: [PATCH 1/5] v4l: async: Add a pointer to of_node to struct v4l2_subdev, match it
Hi Sakari,
Thank you for the patch.
On Wednesday 20 May 2015 02:04:01 Sakari Ailus wrote:
V4L2 async sub-devices are currently matched (OF case) based on the struct
device_node pointer in struct device. LED devices may have more than one
LED, and in that case the OF node to match is not directly the device's
node, but a LED's node.
Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
Signed-off-by: Sakari Ailus sakari.ai...@iki.fi
---
drivers/media/v4l2-core/v4l2-async.c | 34 +++---
drivers/media/v4l2-core/v4l2-device.c |3 +++
include/media/v4l2-subdev.h |2 ++
3 files changed, 27 insertions(+), 12 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-async.c
b/drivers/media/v4l2-core/v4l2-async.c index 85a6a34..bcdd140 100644
--- a/drivers/media/v4l2-core/v4l2-async.c
+++ b/drivers/media/v4l2-core/v4l2-async.c
@@ -22,10 +22,10 @@
#include media/v4l2-device.h
#include media/v4l2-subdev.h
-static bool match_i2c(struct device *dev, struct v4l2_async_subdev *asd)
+static bool match_i2c(struct v4l2_subdev *sd, struct v4l2_async_subdev
*asd) {
#if IS_ENABLED(CONFIG_I2C)
- struct i2c_client *client = i2c_verify_client(dev);
+ struct i2c_client *client = i2c_verify_client(sd-dev);
return client
asd-match.i2c.adapter_id == client-adapter-nr
asd-match.i2c.address == client-addr;
@@ -34,14 +34,27 @@ static bool match_i2c(struct device *dev, struct
v4l2_async_subdev *asd) #endif
}
-static bool match_devname(struct device *dev, struct v4l2_async_subdev
*asd) +static bool match_devname(struct v4l2_subdev *sd,
+ struct v4l2_async_subdev *asd)
{
- return !strcmp(asd-match.device_name.name, dev_name(dev));
+ return !strcmp(asd-match.device_name.name, dev_name(sd-dev));
}
-static bool match_of(struct device *dev, struct v4l2_async_subdev *asd)
+static bool match_of(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd)
{
- return dev-of_node == asd-match.of.node;
+ struct device_node *of_node =
+ sd-of_node ? sd-of_node : sd-dev-of_node;
+
+ return of_node == asd-match.of.node;
+}
+
+static bool match_custom(struct v4l2_subdev *sd, struct v4l2_async_subdev
*asd) +{
+ if (!asd-match.custom.match)
+ /* Match always */
+ return true;
+
+ return asd-match.custom.match(sd-dev, asd);
}
static LIST_HEAD(subdev_list);
@@ -51,17 +64,14 @@ static DEFINE_MUTEX(list_lock);
static struct v4l2_async_subdev *v4l2_async_belongs(struct
v4l2_async_notifier *notifier, struct v4l2_subdev *sd)
{
+ bool (*match)(struct v4l2_subdev *, struct v4l2_async_subdev *);
struct v4l2_async_subdev *asd;
- bool (*match)(struct device *, struct v4l2_async_subdev *);
list_for_each_entry(asd, notifier-waiting, list) {
/* bus_type has been verified valid before */
switch (asd-match_type) {
case V4L2_ASYNC_MATCH_CUSTOM:
- match = asd-match.custom.match;
- if (!match)
- /* Match always */
- return asd;
+ match = match_custom;
break;
case V4L2_ASYNC_MATCH_DEVNAME:
match = match_devname;
@@ -79,7 +89,7 @@ static struct v4l2_async_subdev *v4l2_async_belongs(struct
v4l2_async_notifier * }
/* match cannot be NULL here */
- if (match(sd-dev, asd))
+ if (match(sd, asd))
return asd;
}
diff --git a/drivers/media/v4l2-core/v4l2-device.c
b/drivers/media/v4l2-core/v4l2-device.c index 5b0a30b..a741c6c 100644
--- a/drivers/media/v4l2-core/v4l2-device.c
+++ b/drivers/media/v4l2-core/v4l2-device.c
@@ -157,6 +157,9 @@ int v4l2_device_register_subdev(struct v4l2_device
*v4l2_dev, /* Warn if we apparently re-register a subdev */
WARN_ON(sd-v4l2_dev != NULL);
+ if (!sd-of_node sd-dev)
+ sd-of_node = sd-dev-of_node;
+
/*
* The reason to acquire the module here is to avoid unloading
* a module of sub-device which is registered to a media
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index 8f5da73..5c51987 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -603,6 +603,8 @@ struct v4l2_subdev {
struct video_device *devnode;
/* pointer to the physical device, if any */
struct device *dev;
+ /* A device_node of the sub-device, iff not dev-of_node. */
iff ?
I think we need to define usage of this field more precisely. If the subdev
driver doesn't initialize it it will be set in v4l2_device_register_subdev(),
meaning that there's part of the lifetime of the subdev during which the field
will be NULL even though the subdev's device has a DT node. Could
cron job: media_tree daily build: ERRORS
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date: Sun May 24 04:00:17 CEST 2015 git branch: test git hash: 2a80f296422a01178d0a993479369e94f5830127 gcc version:i686-linux-gcc (GCC) 5.1.0 sparse version: v0.5.0-44-g40791b9 smatch version: 0.4.1-3153-g7d56ab3 host hardware: x86_64 host os:4.0.0-3.slh.1-amd64 linux-git-arm-at91: OK linux-git-arm-davinci: WARNINGS linux-git-arm-exynos: OK linux-git-arm-mx: OK linux-git-arm-omap: OK linux-git-arm-omap1: OK linux-git-arm-pxa: OK linux-git-blackfin-bf561: WARNINGS linux-git-i686: WARNINGS linux-git-m32r: OK linux-git-mips: WARNINGS linux-git-powerpc64: OK linux-git-sh: OK linux-git-x86_64: OK linux-2.6.32.27-i686: OK linux-2.6.33.7-i686: OK linux-2.6.34.7-i686: OK linux-2.6.35.9-i686: OK linux-2.6.36.4-i686: OK linux-2.6.37.6-i686: OK linux-2.6.38.8-i686: OK linux-2.6.39.4-i686: OK linux-3.0.60-i686: OK linux-3.1.10-i686: ERRORS linux-3.2.37-i686: ERRORS linux-3.3.8-i686: ERRORS linux-3.4.27-i686: OK linux-3.5.7-i686: OK linux-3.6.11-i686: OK linux-3.7.4-i686: OK linux-3.8-i686: OK linux-3.9.2-i686: OK linux-3.10.1-i686: OK linux-3.11.1-i686: OK linux-3.12.23-i686: OK linux-3.13.11-i686: OK linux-3.14.9-i686: OK linux-3.15.2-i686: OK linux-3.16.7-i686: WARNINGS linux-3.17.8-i686: WARNINGS linux-3.18.7-i686: WARNINGS linux-3.19-i686: WARNINGS linux-4.0-i686: WARNINGS linux-4.1-rc1-i686: WARNINGS linux-2.6.32.27-x86_64: OK linux-2.6.33.7-x86_64: OK linux-2.6.34.7-x86_64: OK linux-2.6.35.9-x86_64: OK linux-2.6.36.4-x86_64: OK linux-2.6.37.6-x86_64: OK linux-2.6.38.8-x86_64: OK linux-2.6.39.4-x86_64: OK linux-3.0.60-x86_64: OK linux-3.1.10-x86_64: ERRORS linux-3.2.37-x86_64: ERRORS linux-3.3.8-x86_64: ERRORS linux-3.4.27-x86_64: OK linux-3.5.7-x86_64: OK linux-3.6.11-x86_64: OK linux-3.7.4-x86_64: OK linux-3.8-x86_64: OK linux-3.9.2-x86_64: OK linux-3.10.1-x86_64: OK linux-3.11.1-x86_64: OK linux-3.12.23-x86_64: OK linux-3.13.11-x86_64: OK linux-3.14.9-x86_64: OK linux-3.15.2-x86_64: OK linux-3.16.7-x86_64: OK linux-3.17.8-x86_64: OK linux-3.18.7-x86_64: OK linux-3.19-x86_64: OK linux-4.0-x86_64: WARNINGS linux-4.1-rc1-x86_64: WARNINGS apps: OK spec-git: OK sparse: WARNINGS smatch: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Sunday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Sunday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/media.html -- 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 v3 1/7] rc: rc-ir-raw: Add scancode encoder callback
On 22 May 2015 at 13:33, David Härdeman da...@hardeman.nu wrote:
On 2015-05-22 07:27, Antti Seppälä wrote:
On 21 May 2015 at 22:40, David Härdeman da...@hardeman.nu wrote:
On Thu, May 21, 2015 at 05:22:08PM +0300, Antti Seppälä wrote:
On 21 May 2015 at 15:30, David Härdeman da...@hardeman.nu wrote:
On 2015-05-21 13:51, Antti Seppälä wrote:
On 21 May 2015 at 12:14, David Härdeman da...@hardeman.nu wrote:
I'm talking about ir_raw_encode_scancode() which is entirely broken
in
its
current state. It will, given more than one enabled protocol, encode
a
scancode to pulse/space events according to the rules of a randomly
chosen
protocol. That random selection will be influenced by things like
*module
load order* (independent of the separate fact that passing multiple
protocols to it is completely bogus in the first place).
To be clear: the same scancode may be encoded differently depending
on if
you've load the nec decoder before or after the rc5 decoder! That
kind of
behavior can't go into a release kernel (Mauro...).
So... if the ir_raw_handler_list is sorted to eliminate the randomness
caused by module load ordering you will be happy (or happier)?
No, cause it's a horrible hack. And the caller of ir_raw_handler_list()
still has no idea of knowing (given more than one protocol) which
protocol a
given scancode will be encoded according to.
Okay, so how about demuxing the first protocol before handing them
to encoder callback? Simply something like below:
- if (handler-protocols protocols handler-encode) {
+ if (handler-protocols ffs(protocols)
handler-encode) {
Now the behavior is well-defined even when multiple protocols are
selected.
Your patchset introduced ir_raw_encode_scancode() as well as the only
two callers of that function:
drivers/media/rc/nuvoton-cir.c
drivers/media/rc/rc-loopback.c
I realize that the sysfs wakeup_protocols file (which bakes several
protocols into one label) makes defining *the* protocol difficult, but
if you're going to add hacks like this, keep them to the sole driver
using the API rather than the core API itself.
That is, change nvt_ir_raw_change_wakeup_protocol() so that it picks a
single protocol, no matter how many are passed to it (the img-ir driver
already sets a precedent here so it wouldn't be an API change to change
to a set of protocols which might be different than what the user
suggested). (Also...yes, that'll make supporting several versions of
e.g. RC6 impossible with the current sysfs code).
I think that approach too is far from perfect as it leaves us with
questions such as: How do we let the user know what variant of
protocol the label rc-6 really means? If in nvt we hardcode it to
mean RC6-0-16 and a new driver cames along which chooses
RC_TYPE_RC6_6A_24 how do we tell the user that the implementations
differ? What if the scancode that was fed was really RC_TYPE_RC6_MCE?
I never claimed it was perfect.
For another (short-term, per-driver) solution, see the winbond-cir driver.
Heh, funny you should mention that... Back in late 2013/early 2014 I
submitted a patch for nuvoton which was modeled after winbond-cir. The
feedback from that could be summarized as:
- Scancodes should be used instead of raw pulse/spaces (the initial
version of the patch worked without encoding)
- Encoders should be generalized for others to use them too
- Sysfs -api should be used instead of module parameters
So the suggestions were a pretty much the exact opposite of what
winbond-cir does.
If only there were a sysfs api to set the exact variant life would be
simpler...
Yes, and your patches made it harder to get to a sane solution.
Then change both ir_raw_encode_scancode() to take a single protocol enum
and change the *encode function pointer in struct ir_raw_handler to also
take a single protocol enum.
That way encoders won't have to guess (using scanmasks...!?) what
protocol they should encode to.
And Mauro...I strongly suggest you revert all of this encoding stuff
until this has been fixed...it's broken.
Let me shortly recap the points made so far about the encoding stuff:
* If user enables multiple wakeup_protocols then the first matching
one will be used to do the encoding. First being the module that was
loaded first.
That in itself would be a good enough reason to revert the patches.
* Currently the encoders use heuristics to determine the intended
protocol variant from the scancode that was fed and from the length of
the scanmask. This causes the programmer using the encoder to not know
which exact variant will be used (until after the encoding is done
when the information can be made available if needed).
First, currently implies that the heuristics can later be changed. They
can't once this becomes part of a released kernel (not without breaking the
kernel API).
Second, how do you plan to pass the information about the
Re: [PATCH v8 8/8] DT: samsung-fimc: Add examples for samsung,flash-led property
Hi Sylwester and Jacek, On Thu, May 21, 2015 at 03:28:40PM +0200, Sylwester Nawrocki wrote: flash-leds = flash_xx image_sensor_x, ...; One more matter to consider: xenon flash devices. How about samsung,camera-flashes (and ti,camera-flashes)? After pondering this awhile, I'm ok with removing the vendor prefix as well. Let me know what you think. -- Kind regards, Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk -- 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
