On Sun March 24 2013 11:07:03 Mauro Carvalho Chehab wrote: > Em Mon, 18 Mar 2013 15:12:03 +0100 > Hans Verkuil <hverk...@xs4all.nl> escreveu: > > > From: Hans Verkuil <hans.verk...@cisco.com> > > > > This ioctl is defined as IOW, so pass the argument as const. > > > > Signed-off-by: Hans Verkuil <hans.verk...@cisco.com> > > Acked-by: Guennadi Liakhovetski <g.liakhovet...@gmx.de> > > Acked-by: Lad, Prabhakar <prabhakar.cse...@gmail.com> > > ... > > > 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) > > { > > - 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); > > I'm not convinced about the changes on ivtv. Why do you need volatile there?
It was using volatile before and as I told Laurent as well, I don't want to mix changing volatile removal with adding const support. > Also, as you're doing changes there that aren't that trivial, and are not > just "add const argument", please split those non-trivial ivtv changes into > a separate patch, and properly describe what you're doing and why. OK, and I'll add a separate patch in that case to remove the volatile part. > Also, having it on a separate patch helps to bisect it, if it ever brings > any problem. Regards, Hans -- 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