Hi Hans, Thanks for the patch. On Friday 15 March 2013 11:27:24 Hans Verkuil wrote: > From: Hans Verkuil <hans.verkuil@xxxxxxxxx> > > This ioctl is defined as IOW, so pass the argument as const. > > Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxx> [snip] > diff --git a/drivers/media/pci/ivtv/ivtv-ioctl.c > b/drivers/media/pci/ivtv/ivtv-ioctl.c index 080f179..15e08aa 100644 > --- a/drivers/media/pci/ivtv/ivtv-ioctl.c > +++ b/drivers/media/pci/ivtv/ivtv-ioctl.c > @@ -711,49 +711,50 @@ static int ivtv_g_chip_ident(struct file *file, void > *fh, struct v4l2_dbg_chip_i } > > #ifdef CONFIG_VIDEO_ADV_DEBUG > -static int ivtv_itvc(struct ivtv *itv, unsigned int cmd, void *arg) > +static volatile u8 __iomem *ivtv_itvc_start(struct ivtv *itv, > + const struct v4l2_dbg_register *regs) As you use the proper __iomem accessors I don't think you need a volatile. > { > - struct v4l2_dbg_register *regs = arg; > - volatile u8 __iomem *reg_start; > - > - if (!capable(CAP_SYS_ADMIN)) > - return -EPERM; > if (regs->reg >= IVTV_REG_OFFSET && regs->reg < IVTV_REG_OFFSET + > IVTV_REG_SIZE) - reg_start = itv->reg_mem - IVTV_REG_OFFSET; > - else if (itv->has_cx23415 && regs->reg >= IVTV_DECODER_OFFSET && > + return itv->reg_mem - IVTV_REG_OFFSET; > + if (itv->has_cx23415 && regs->reg >= IVTV_DECODER_OFFSET && > regs->reg < IVTV_DECODER_OFFSET + IVTV_DECODER_SIZE) > - reg_start = itv->dec_mem - IVTV_DECODER_OFFSET; > - else if (regs->reg < IVTV_ENCODER_SIZE) > - reg_start = itv->enc_mem; > - else > - return -EINVAL; > - > - regs->size = 4; > - if (cmd == VIDIOC_DBG_G_REGISTER) > - regs->val = readl(regs->reg + reg_start); > - else > - writel(regs->val, regs->reg + reg_start); > - return 0; > + return itv->dec_mem - IVTV_DECODER_OFFSET; > + if (regs->reg < IVTV_ENCODER_SIZE) > + return itv->enc_mem; > + return NULL; > } > > static int ivtv_g_register(struct file *file, void *fh, struct > v4l2_dbg_register *reg) { > struct ivtv *itv = fh2id(fh)->itv; > > - if (v4l2_chip_match_host(®->match)) > - return ivtv_itvc(itv, VIDIOC_DBG_G_REGISTER, reg); > + if (v4l2_chip_match_host(®->match)) { > + volatile u8 __iomem *reg_start = ivtv_itvc_start(itv, reg); > + > + if (reg_start == NULL) > + return -EINVAL; > + reg->size = 4; > + reg->val = readl(reg->reg + reg_start); > + return 0; > + } > /* TODO: subdev errors should not be ignored, this should become a > subdev helper function. */ > ivtv_call_all(itv, core, g_register, reg); > return 0; > } > > -static int ivtv_s_register(struct file *file, void *fh, struct > v4l2_dbg_register *reg) +static int ivtv_s_register(struct file *file, void > *fh, const struct v4l2_dbg_register *reg) { > struct ivtv *itv = fh2id(fh)->itv; > > - if (v4l2_chip_match_host(®->match)) > - return ivtv_itvc(itv, VIDIOC_DBG_S_REGISTER, reg); > + if (v4l2_chip_match_host(®->match)) { > + volatile u8 __iomem *reg_start = ivtv_itvc_start(itv, reg); > + > + if (reg_start == NULL) > + return -EINVAL; > + writel(reg->val, reg->reg + reg_start); > + return 0; > + } > /* TODO: subdev errors should not be ignored, this should become a > subdev helper function. */ > ivtv_call_all(itv, core, s_register, reg); -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html