Re: [PATCH v1 3/3] media: intel/ipu6: Constify ipu6_buttress_ctrl structure

[Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>]

On Mon, Mar 10, 2025 at 10:30:40AM +0000, Sakari Ailus wrote:
> Hi Stanislaw,
> 
> On Mon, Mar 10, 2025 at 09:37:55AM +0100, Stanislaw Gruszka wrote:
> > Hi Sakari,
> > 
> > On Fri, Mar 07, 2025 at 07:45:03AM +0000, Sakari Ailus wrote:
> > > >  ipu6_bus_initialize_device(struct pci_dev *pdev, struct device *parent,
> > > > -			   void *pdata, struct ipu6_buttress_ctrl *ctrl,
> > > > +			   void *pdata, const struct ipu6_buttress_ctrl *ctrl,
> > > 
> > > pdata should be const, too, btw.
> > > 
> > > >  			   char *name);
> > > >  int ipu6_bus_add_device(struct ipu6_bus_device *adev);
> > > >  void ipu6_bus_del_devices(struct pci_dev *pdev);
> > > > diff --git a/drivers/media/pci/intel/ipu6/ipu6-buttress.c b/drivers/media/pci/intel/ipu6/ipu6-buttress.c
> > > > index 787fcbd1df09..f8fdc07a953c 100644
> > > > --- a/drivers/media/pci/intel/ipu6/ipu6-buttress.c
> > > > +++ b/drivers/media/pci/intel/ipu6/ipu6-buttress.c
> > > > @@ -443,7 +443,7 @@ irqreturn_t ipu6_buttress_isr_threaded(int irq, void *isp_ptr)
> > > >  	return ret;
> > > >  }
> > > >  
> > > > -int ipu6_buttress_power(struct device *dev, struct ipu6_buttress_ctrl *ctrl,
> > > > +int ipu6_buttress_power(struct device *dev, const struct ipu6_buttress_ctrl *ctrl,
> > > >  			bool on)
> > > 
> > > But this is over 80.
> > 
> > On official kernel doc the limit is 100 (with 80 being preferred).
> > I run chackpatch.pl on this patch and it was just fine.
> 
> The Media tree driver documentation suggests:
> 
> $ ./scripts/checkpatch.pl --strict --max-line-length=80

TBH, in context of ipu6 enforcing 80 characters instead of 100,
frequently makes more harm then good IMHO, for example:

const struct ipu6_isys_pixelformat ipu6_isys_pfmts[] = {
	{ V4L2_PIX_FMT_SBGGR12, 16, 12, MEDIA_BUS_FMT_SBGGR12_1X12,
	  IPU6_FW_ISYS_FRAME_FORMAT_RAW16 },
	{ V4L2_PIX_FMT_SGBRG12, 16, 12, MEDIA_BUS_FMT_SGBRG12_1X12,
	  IPU6_FW_ISYS_FRAME_FORMAT_RAW16 },
	{ V4L2_PIX_FMT_SGRBG12, 16, 12, MEDIA_BUS_FMT_SGRBG12_1X12,
	  IPU6_FW_ISYS_FRAME_FORMAT_RAW16 },
	{ V4L2_PIX_FMT_SRGGB12, 16, 12, MEDIA_BUS_FMT_SRGGB12_1X12,
vs:

const struct ipu6_isys_pixelformat ipu6_isys_pfmts[] = {
	{ V4L2_PIX_FMT_SBGGR12, 16, 12, MEDIA_BUS_FMT_SBGGR12_1X12, IPU6_FW_ISYS_FRAME_FORMAT_RAW16 },
	{ V4L2_PIX_FMT_SGBRG12, 16, 12, MEDIA_BUS_FMT_SGBRG12_1X12, IPU6_FW_ISYS_FRAME_FORMAT_RAW16 },
	{ V4L2_PIX_FMT_SGRBG12, 16, 12, MEDIA_BUS_FMT_SGRBG12_1X12, IPU6_FW_ISYS_FRAME_FORMAT_RAW16 },
	{ V4L2_PIX_FMT_SRGGB12, 16, 12, MEDIA_BUS_FMT_SRGGB12_1X12, IPU6_FW_ISYS_FRAME_FORMAT_RAW16 },


Or:
		if (type && ((!pfmt->is_meta &&
			      type != V4L2_BUF_TYPE_VIDEO_CAPTURE) ||
			     (pfmt->is_meta &&
			      type != V4L2_BUF_TYPE_META_CAPTURE)))
			continue;

vs:

		if (type && ((!pfmt->is_meta && type != V4L2_BUF_TYPE_VIDEO_CAPTURE) ||
			     (pfmt->is_meta && type != V4L2_BUF_TYPE_META_CAPTURE)))
			continue;


Do we really need 80 chars limit in ipu drivers ? 

Regards
Stanislaw