RE: [PATCH - v1 4/6] V4L - vpfe_capture bug fix and enhancements
On Wed, Dec 16, 2009 at 13:11:57, Hans Verkuil wrote:
On Wednesday 16 December 2009 00:37:52 Karicheri, Muralidharan wrote:
Hans,
I remember there was a comment against an earlier patch that asks
for combining such statements since it makes the function appear
as big. Not sure who had made that comment. That is the reason you
find code like this in this patch. It was initially done with multiple
OR statements to construct the value to be written to the register as you
stated below as
val = bc-horz.win_count_calc
ISIF_HORZ_BC_WIN_COUNT_MASK;
val |= !!bc-horz.base_win_sel_calc
ISIF_HORZ_BC_WIN_SEL_SHIFT;
I have checked few other drivers such as bt819.c ir-kbd-i2c.c,
mt9m111.c etc, where similar statements are found, but they have used
hardcoded values masks which makes it appears less complex. But I
like to reduce magic numbers as much possible in the code.
Personally I have mixed feelings about the use for symbolic names for things
like this. The problem is that they do not help me understanding the code.
The names tend to be long, leading to these broken up lines, and if I want
to know how the bits are distributed in the value I continuously have to
do the look up in the header containing these defines.
Frankly, I have a similar problem with using symbolic names for registers.
Every time I need to look up a register in the datasheet I first need to
look up the register number the register name maps to. All datasheets seem
to be organized around the register addresses and there rarely is a datasheet
that has an index of symbolic names.
Using hard-coded numbers together with a well written comment tends to be much
more readable in my opinion. I don't really think there is anything magic
about
these numbers: these are exactly the numbers that I need to know whenever I
have to deal with the datasheet. Having to go through a layer of obfuscation
is just plain irritating to me.
However, I seem to be a minority when it comes to this and I've given up
arguing for this...
Note that all this assumes that the datasheet is publicly available. If it
is a reversed engineered driver or based on NDA datasheets only, then the
symbolic names may be all there is to make you understand what is going on.
[...]
That seems overkill. I actually think it can be improved a lot by visually
grouping the lines:
val = (bc-horz.win_count_calc
ISIF_HORZ_BC_WIN_COUNT_MASK) |
((!!bc-horz.base_win_sel_calc)
ISIF_HORZ_BC_WIN_SEL_SHIFT) |
((!!bc-horz.clamp_pix_limit)
ISIF_HORZ_BC_PIX_LIMIT_SHIFT) |
((bc-horz.win_h_sz_calc
ISIF_HORZ_BC_WIN_H_SIZE_MASK)
ISIF_HORZ_BC_WIN_H_SIZE_SHIFT) |
((bc-horz.win_v_sz_calc
ISIF_HORZ_BC_WIN_V_SIZE_MASK)
ISIF_HORZ_BC_WIN_V_SIZE_SHIFT);
Now I can at least see the various values that are ORed.
I liked this piece of code from drivers/mtd/nand/s3c2410.c. Serves as
a good template to do this sort of thing.
#define S3C2410_NFCONF_TACLS(x)((x)8)
#define S3C2410_NFCONF_TWRPH0(x) ((x)4)
#define S3C2410_NFCONF_TWRPH1(x) ((x)0)
[Okay, spaces around '', please :)]
[...]
if (plat != NULL) {
tacls = s3c_nand_calc_rate(plat-tacls, clkrate, tacls_max);
twrph0 = s3c_nand_calc_rate(plat-twrph0, clkrate, 8);
twrph1 = s3c_nand_calc_rate(plat-twrph1, clkrate, 8);
}
[...]
mask = (S3C2410_NFCONF_TACLS(3) |
S3C2410_NFCONF_TWRPH0(7) |
S3C2410_NFCONF_TWRPH1(7));
set = S3C2410_NFCONF_EN;
set |= S3C2410_NFCONF_TACLS(tacls - 1);
set |= S3C2410_NFCONF_TWRPH0(twrph0 - 1);
set |= S3C2410_NFCONF_TWRPH1(twrph1 - 1);
[...]
cfg = readl(info-regs + S3C2410_NFCONF);
cfg = ~mask;
cfg |= set;
writel(cfg, info-regs + S3C2410_NFCONF);
And Murali said:
Huh? That does not explain why apparently bc-horz.win_h_sz_calc can be
larger
than ISIF_HORZ_BC_WIN_H_SIZE_MASK.
because the values come from the user and since we can't use the enum
for the types, I have to make sure the value is within range. Other way
to do is to check the value in the validate() function. I am inclined to
do the validation so that the statements with masks can be removed while
setting it in
the register.
Agree fully here. Either a separate validate function or
an if check before using the values. Results with masking
or without masking are both unpredictable.
Thanks,
Sekhar
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More
RE: [PATCH - v1 4/6] V4L - vpfe_capture bug fix and enhancements
hans, Yes, isif_config_bclamp() set values in the register. Huh? That does not explain why apparently bc-horz.win_h_sz_calc can be larger than ISIF_HORZ_BC_WIN_H_SIZE_MASK. because the values come from the user and since we can't use the enum for the types, I have to make sure the value is within range. Other way to do is to check the value in the validate() function. I am inclined to do the validation so that the statements with masks can be removed while setting it in the register. Regards, Hans It would be interesting to know if people know of good ways of making awkward code like this more elegant (or at least less awkward). Regards, Hans -- Hans Verkuil - video4linux developer - sponsored by TANDBERG -- Hans Verkuil - video4linux developer - sponsored by TANDBERG -- 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 - v1 4/6] V4L - vpfe_capture bug fix and enhancements
On Thursday 10 December 2009 18:00:29 m-kariche...@ti.com wrote:
From: Muralidharan Karicheri m-kariche...@ti.com
Added a experimental IOCTL, to read the CCDC parameters.
Default handler was not getting the original pointer from the core.
So a wrapper function added to handle the default handler properly.
Also fixed a bug in the probe() in the linux-next tree
Signed-off-by: Muralidharan Karicheri m-kariche...@ti.com
---
drivers/media/video/davinci/vpfe_capture.c | 119
+---
include/media/davinci/vpfe_capture.h | 12 ++-
2 files changed, 81 insertions(+), 50 deletions(-)
diff --git a/drivers/media/video/davinci/vpfe_capture.c
b/drivers/media/video/davinci/vpfe_capture.c
index 091750e..8c6d856 100644
--- a/drivers/media/video/davinci/vpfe_capture.c
+++ b/drivers/media/video/davinci/vpfe_capture.c
@@ -759,12 +759,83 @@ static unsigned int vpfe_poll(struct file *file,
poll_table *wait)
return 0;
}
+static long vpfe_param_handler(struct file *file, void *priv,
+ int cmd, void *param)
+{
+ struct vpfe_device *vpfe_dev = video_drvdata(file);
+ int ret = 0;
+
+ v4l2_dbg(1, debug, vpfe_dev-v4l2_dev, vpfe_param_handler\n);
+
+ if (NULL == param) {
+ v4l2_dbg(1, debug, vpfe_dev-v4l2_dev,
+ Invalid user ptr\n);
Shouldn't there be an error return here? It looks weird.
+ }
+
+ if (vpfe_dev-started) {
+ /* only allowed if streaming is not started */
+ v4l2_err(vpfe_dev-v4l2_dev, device already started\n);
+ return -EBUSY;
+ }
+
+
+ switch (cmd) {
+ case VPFE_CMD_S_CCDC_RAW_PARAMS:
+ v4l2_warn(vpfe_dev-v4l2_dev,
+ VPFE_CMD_S_CCDC_RAW_PARAMS: experimental ioctl\n);
+ ret = mutex_lock_interruptible(vpfe_dev-lock);
+ if (ret)
+ return ret;
+ ret = ccdc_dev-hw_ops.set_params(param);
+ if (ret) {
+ v4l2_dbg(1, debug, vpfe_dev-v4l2_dev,
+ Error in setting parameters in CCDC\n);
+ goto unlock_out;
+ }
+
+ if (vpfe_get_ccdc_image_format(vpfe_dev, vpfe_dev-fmt) 0) {
+ v4l2_err(vpfe_dev-v4l2_dev,
+ Invalid image format at CCDC\n);
+ ret = -EINVAL;
+ }
+unlock_out:
+ mutex_unlock(vpfe_dev-lock);
+ break;
+ case VPFE_CMD_G_CCDC_RAW_PARAMS:
+ v4l2_warn(vpfe_dev-v4l2_dev,
+ VPFE_CMD_G_CCDC_RAW_PARAMS: experimental ioctl\n);
+ if (!ccdc_dev-hw_ops.get_params) {
+ ret = -EINVAL;
+ break;
+ }
+ ret = ccdc_dev-hw_ops.get_params(param);
+ if (ret) {
+ v4l2_dbg(1, debug, vpfe_dev-v4l2_dev,
+ Error in getting parameters from CCDC\n);
+ }
+ break;
+
+ default:
+ ret = -EINVAL;
Please add a break statement here.
+ }
+ return ret;
+}
+
+static long vpfe_ioctl(struct file *file, unsigned int cmd, unsigned long
arg)
+{
+ if (cmd == VPFE_CMD_S_CCDC_RAW_PARAMS ||
+ cmd == VPFE_CMD_G_CCDC_RAW_PARAMS)
+ return vpfe_param_handler(file, file-private_data, cmd,
+ (void *)arg);
+ return video_ioctl2(file, cmd, arg);
+}
+
/* vpfe capture driver file operations */
static const struct v4l2_file_operations vpfe_fops = {
.owner = THIS_MODULE,
.open = vpfe_open,
.release = vpfe_release,
- .unlocked_ioctl = video_ioctl2,
+ .unlocked_ioctl = vpfe_ioctl,
.mmap = vpfe_mmap,
.poll = vpfe_poll
};
@@ -1682,50 +1753,6 @@ unlock_out:
return ret;
}
-
-static long vpfe_param_handler(struct file *file, void *priv,
- int cmd, void *param)
-{
- struct vpfe_device *vpfe_dev = video_drvdata(file);
- int ret = 0;
-
- v4l2_dbg(1, debug, vpfe_dev-v4l2_dev, vpfe_param_handler\n);
-
- if (vpfe_dev-started) {
- /* only allowed if streaming is not started */
- v4l2_err(vpfe_dev-v4l2_dev, device already started\n);
- return -EBUSY;
- }
-
- ret = mutex_lock_interruptible(vpfe_dev-lock);
- if (ret)
- return ret;
-
- switch (cmd) {
- case VPFE_CMD_S_CCDC_RAW_PARAMS:
- v4l2_warn(vpfe_dev-v4l2_dev,
- VPFE_CMD_S_CCDC_RAW_PARAMS: experimental ioctl\n);
- ret = ccdc_dev-hw_ops.set_params(param);
- if (ret) {
- v4l2_err(vpfe_dev-v4l2_dev,
- Error in setting parameters in CCDC\n);
- goto unlock_out;
- }
- if
Re: [PATCH - v1 4/6] V4L - vpfe_capture bug fix and enhancements
Note that the other patches from this series are fine as far as I am concerned. One general note: I always have difficulties with constructions like this: + val = (bc-horz.win_count_calc + ISIF_HORZ_BC_WIN_COUNT_MASK) | + ((!!bc-horz.base_win_sel_calc) + ISIF_HORZ_BC_WIN_SEL_SHIFT) | + ((!!bc-horz.clamp_pix_limit) + ISIF_HORZ_BC_PIX_LIMIT_SHIFT) | + ((bc-horz.win_h_sz_calc + ISIF_HORZ_BC_WIN_H_SIZE_MASK) + ISIF_HORZ_BC_WIN_H_SIZE_SHIFT) | + ((bc-horz.win_v_sz_calc + ISIF_HORZ_BC_WIN_V_SIZE_MASK) + ISIF_HORZ_BC_WIN_V_SIZE_SHIFT); It's just about impossible for me to parse. And some of the patches in this series are full of such constructs. Unfortunately, I do not have a magic bullet solution. In some cases I suspect that a static inline function can help. In other cases it might help to split it up in smaller parts. For example: u32 tmp; val = bc-horz.win_count_calc ISIF_HORZ_BC_WIN_COUNT_MASK; val |= !!bc-horz.base_win_sel_calc ISIF_HORZ_BC_WIN_SEL_SHIFT; val |= !!bc-horz.clamp_pix_limit ISIF_HORZ_BC_PIX_LIMIT_SHIFT; tmp = bc-horz.win_h_sz_calc ISIF_HORZ_BC_WIN_H_SIZE_MASK; val |= tmp ISIF_HORZ_BC_WIN_H_SIZE_SHIFT; tmp = bc-horz.win_v_sz_calc ISIF_HORZ_BC_WIN_V_SIZE_MASK; val |= tmp ISIF_HORZ_BC_WIN_V_SIZE_SHIFT; Of course, in this particular piece of code from the function isif_config_bclamp() I am also wondering why bc-horz.win_h_sz_calc and bc-horz.win_v_sz_calc need to be ANDed anyway. I would expect that to happen when these values are set. But I did not look at this in-depth, so I may well have missed some subtlety here. It would be interesting to know if people know of good ways of making awkward code like this more elegant (or at least less awkward). Regards, Hans -- Hans Verkuil - video4linux developer - sponsored by TANDBERG -- 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 - v1 4/6] V4L - vpfe_capture bug fix and enhancements
Hans, I remember there was a comment against an earlier patch that asks for combining such statements since it makes the function appear as big. Not sure who had made that comment. That is the reason you find code like this in this patch. It was initially done with multiple OR statements to construct the value to be written to the register as you stated below as val = bc-horz.win_count_calc ISIF_HORZ_BC_WIN_COUNT_MASK; val |= !!bc-horz.base_win_sel_calc ISIF_HORZ_BC_WIN_SEL_SHIFT; I have checked few other drivers such as bt819.c ir-kbd-i2c.c, mt9m111.c etc, where similar statements are found, but they have used hardcoded values masks which makes it appears less complex. But I like to reduce magic numbers as much possible in the code. I think what I can do is to combine maximum of 2 such expressions in a statement which might make it less complex to read. Such as val = (bc-horz.win_count_calc ISIF_HORZ_BC_WIN_COUNT_MASK) | ((!!bc-horz.base_win_sel_calc) ISIF_HORZ_BC_WIN_SEL_SHIFT); val |= (!!bc-horz.clamp_pix_limit) ISIF_HORZ_BC_PIX_LIMIT_SHIFT) | ((bc-horz.win_h_sz_calc ISIF_HORZ_BC_WIN_H_SIZE_MASK) ISIF_HORZ_BC_WIN_H_SIZE_SHIFT); val |= ((bc-horz.win_v_sz_calc ISIF_HORZ_BC_WIN_V_SIZE_MASK) ISIF_HORZ_BC_WIN_V_SIZE_SHIFT); Also to make the line fits in 80 characters, I will consider reducing the number of characters in #define names such as val = (bc-horz.win_count_calc HZ_BC_WIN_CNT_MASK) | ((!!bc-horz.base_win_sel_calc) HZ_BC_WIN_SEL_SHIFT) | (!!bc-horz.clamp_pix_limit) HZ_BC_PIX_LIMIT_SHIFT); Let me know if you don't agree. Of course, in this particular piece of code from the function isif_config_bclamp() I am also wondering why bc-horz.win_h_sz_calc and bc-horz.win_v_sz_calc need to be ANDed anyway. I would expect that to happen when these values are set. But I did not look at this in-depth, so I may well have missed some subtlety here. Yes, isif_config_bclamp() set values in the register. It would be interesting to know if people know of good ways of making awkward code like this more elegant (or at least less awkward). Regards, Hans -- Hans Verkuil - video4linux developer - sponsored by TANDBERG -- 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 - v1 4/6] V4L - vpfe_capture bug fix and enhancements
On Wednesday 16 December 2009 00:37:52 Karicheri, Muralidharan wrote: Hans, I remember there was a comment against an earlier patch that asks for combining such statements since it makes the function appear as big. Not sure who had made that comment. That is the reason you find code like this in this patch. It was initially done with multiple OR statements to construct the value to be written to the register as you stated below as val = bc-horz.win_count_calc ISIF_HORZ_BC_WIN_COUNT_MASK; val |= !!bc-horz.base_win_sel_calc ISIF_HORZ_BC_WIN_SEL_SHIFT; I have checked few other drivers such as bt819.c ir-kbd-i2c.c, mt9m111.c etc, where similar statements are found, but they have used hardcoded values masks which makes it appears less complex. But I like to reduce magic numbers as much possible in the code. Personally I have mixed feelings about the use for symbolic names for things like this. The problem is that they do not help me understanding the code. The names tend to be long, leading to these broken up lines, and if I want to know how the bits are distributed in the value I continuously have to do the look up in the header containing these defines. Frankly, I have a similar problem with using symbolic names for registers. Every time I need to look up a register in the datasheet I first need to look up the register number the register name maps to. All datasheets seem to be organized around the register addresses and there rarely is a datasheet that has an index of symbolic names. Using hard-coded numbers together with a well written comment tends to be much more readable in my opinion. I don't really think there is anything magic about these numbers: these are exactly the numbers that I need to know whenever I have to deal with the datasheet. Having to go through a layer of obfuscation is just plain irritating to me. However, I seem to be a minority when it comes to this and I've given up arguing for this... Note that all this assumes that the datasheet is publicly available. If it is a reversed engineered driver or based on NDA datasheets only, then the symbolic names may be all there is to make you understand what is going on. I think what I can do is to combine maximum of 2 such expressions in a statement which might make it less complex to read. Such as val = (bc-horz.win_count_calc ISIF_HORZ_BC_WIN_COUNT_MASK) | ((!!bc-horz.base_win_sel_calc) ISIF_HORZ_BC_WIN_SEL_SHIFT); val |= (!!bc-horz.clamp_pix_limit) ISIF_HORZ_BC_PIX_LIMIT_SHIFT) | ((bc-horz.win_h_sz_calc ISIF_HORZ_BC_WIN_H_SIZE_MASK) ISIF_HORZ_BC_WIN_H_SIZE_SHIFT); val |= ((bc-horz.win_v_sz_calc ISIF_HORZ_BC_WIN_V_SIZE_MASK) ISIF_HORZ_BC_WIN_V_SIZE_SHIFT); Also to make the line fits in 80 characters, I will consider reducing the number of characters in #define names such as val = (bc-horz.win_count_calc HZ_BC_WIN_CNT_MASK) | ((!!bc-horz.base_win_sel_calc) HZ_BC_WIN_SEL_SHIFT) | (!!bc-horz.clamp_pix_limit) HZ_BC_PIX_LIMIT_SHIFT); Let me know if you don't agree. That seems overkill. I actually think it can be improved a lot by visually grouping the lines: val = (bc-horz.win_count_calc ISIF_HORZ_BC_WIN_COUNT_MASK) | ((!!bc-horz.base_win_sel_calc) ISIF_HORZ_BC_WIN_SEL_SHIFT) | ((!!bc-horz.clamp_pix_limit) ISIF_HORZ_BC_PIX_LIMIT_SHIFT) | ((bc-horz.win_h_sz_calc ISIF_HORZ_BC_WIN_H_SIZE_MASK) ISIF_HORZ_BC_WIN_H_SIZE_SHIFT) | ((bc-horz.win_v_sz_calc ISIF_HORZ_BC_WIN_V_SIZE_MASK) ISIF_HORZ_BC_WIN_V_SIZE_SHIFT); Now I can at least see the various values that are ORed. Of course, in this particular piece of code from the function isif_config_bclamp() I am also wondering why bc-horz.win_h_sz_calc and bc-horz.win_v_sz_calc need to be ANDed anyway. I would expect that to happen when these values are set. But I did not look at this in-depth, so I may well have missed some subtlety here. Yes, isif_config_bclamp() set values in the register. Huh? That does not explain why apparently bc-horz.win_h_sz_calc can be larger than ISIF_HORZ_BC_WIN_H_SIZE_MASK. Regards, Hans It would be interesting to know if people know of good ways of making awkward code like this more elegant (or at least less awkward). Regards, Hans -- Hans Verkuil - video4linux developer - sponsored by TANDBERG -- Hans Verkuil - video4linux developer - sponsored by TANDBERG -- 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
[PATCH - v1 4/6] V4L - vpfe_capture bug fix and enhancements
From: Muralidharan Karicheri m-kariche...@ti.com
Added a experimental IOCTL, to read the CCDC parameters.
Default handler was not getting the original pointer from the core.
So a wrapper function added to handle the default handler properly.
Also fixed a bug in the probe() in the linux-next tree
Signed-off-by: Muralidharan Karicheri m-kariche...@ti.com
---
drivers/media/video/davinci/vpfe_capture.c | 119 +---
include/media/davinci/vpfe_capture.h | 12 ++-
2 files changed, 81 insertions(+), 50 deletions(-)
diff --git a/drivers/media/video/davinci/vpfe_capture.c
b/drivers/media/video/davinci/vpfe_capture.c
index 091750e..8c6d856 100644
--- a/drivers/media/video/davinci/vpfe_capture.c
+++ b/drivers/media/video/davinci/vpfe_capture.c
@@ -759,12 +759,83 @@ static unsigned int vpfe_poll(struct file *file,
poll_table *wait)
return 0;
}
+static long vpfe_param_handler(struct file *file, void *priv,
+ int cmd, void *param)
+{
+ struct vpfe_device *vpfe_dev = video_drvdata(file);
+ int ret = 0;
+
+ v4l2_dbg(1, debug, vpfe_dev-v4l2_dev, vpfe_param_handler\n);
+
+ if (NULL == param) {
+ v4l2_dbg(1, debug, vpfe_dev-v4l2_dev,
+ Invalid user ptr\n);
+ }
+
+ if (vpfe_dev-started) {
+ /* only allowed if streaming is not started */
+ v4l2_err(vpfe_dev-v4l2_dev, device already started\n);
+ return -EBUSY;
+ }
+
+
+ switch (cmd) {
+ case VPFE_CMD_S_CCDC_RAW_PARAMS:
+ v4l2_warn(vpfe_dev-v4l2_dev,
+ VPFE_CMD_S_CCDC_RAW_PARAMS: experimental ioctl\n);
+ ret = mutex_lock_interruptible(vpfe_dev-lock);
+ if (ret)
+ return ret;
+ ret = ccdc_dev-hw_ops.set_params(param);
+ if (ret) {
+ v4l2_dbg(1, debug, vpfe_dev-v4l2_dev,
+ Error in setting parameters in CCDC\n);
+ goto unlock_out;
+ }
+
+ if (vpfe_get_ccdc_image_format(vpfe_dev, vpfe_dev-fmt) 0) {
+ v4l2_err(vpfe_dev-v4l2_dev,
+ Invalid image format at CCDC\n);
+ ret = -EINVAL;
+ }
+unlock_out:
+ mutex_unlock(vpfe_dev-lock);
+ break;
+ case VPFE_CMD_G_CCDC_RAW_PARAMS:
+ v4l2_warn(vpfe_dev-v4l2_dev,
+ VPFE_CMD_G_CCDC_RAW_PARAMS: experimental ioctl\n);
+ if (!ccdc_dev-hw_ops.get_params) {
+ ret = -EINVAL;
+ break;
+ }
+ ret = ccdc_dev-hw_ops.get_params(param);
+ if (ret) {
+ v4l2_dbg(1, debug, vpfe_dev-v4l2_dev,
+ Error in getting parameters from CCDC\n);
+ }
+ break;
+
+ default:
+ ret = -EINVAL;
+ }
+ return ret;
+}
+
+static long vpfe_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
+{
+ if (cmd == VPFE_CMD_S_CCDC_RAW_PARAMS ||
+ cmd == VPFE_CMD_G_CCDC_RAW_PARAMS)
+ return vpfe_param_handler(file, file-private_data, cmd,
+(void *)arg);
+ return video_ioctl2(file, cmd, arg);
+}
+
/* vpfe capture driver file operations */
static const struct v4l2_file_operations vpfe_fops = {
.owner = THIS_MODULE,
.open = vpfe_open,
.release = vpfe_release,
- .unlocked_ioctl = video_ioctl2,
+ .unlocked_ioctl = vpfe_ioctl,
.mmap = vpfe_mmap,
.poll = vpfe_poll
};
@@ -1682,50 +1753,6 @@ unlock_out:
return ret;
}
-
-static long vpfe_param_handler(struct file *file, void *priv,
- int cmd, void *param)
-{
- struct vpfe_device *vpfe_dev = video_drvdata(file);
- int ret = 0;
-
- v4l2_dbg(1, debug, vpfe_dev-v4l2_dev, vpfe_param_handler\n);
-
- if (vpfe_dev-started) {
- /* only allowed if streaming is not started */
- v4l2_err(vpfe_dev-v4l2_dev, device already started\n);
- return -EBUSY;
- }
-
- ret = mutex_lock_interruptible(vpfe_dev-lock);
- if (ret)
- return ret;
-
- switch (cmd) {
- case VPFE_CMD_S_CCDC_RAW_PARAMS:
- v4l2_warn(vpfe_dev-v4l2_dev,
- VPFE_CMD_S_CCDC_RAW_PARAMS: experimental ioctl\n);
- ret = ccdc_dev-hw_ops.set_params(param);
- if (ret) {
- v4l2_err(vpfe_dev-v4l2_dev,
- Error in setting parameters in CCDC\n);
- goto unlock_out;
- }
- if (vpfe_get_ccdc_image_format(vpfe_dev, vpfe_dev-fmt) 0) {
- v4l2_err(vpfe_dev-v4l2_dev,
-
