On Mon, Mar 10, 2025 at 12:18:25PM +0100, Stanislaw Gruszka wrote:
> 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 ?
In the former case I'd keep data related to an array index on the same
line, they're often more readable like that. It's not a strict rule. In the
latter case wrapping after first && may be more readable than either of the
two.
--
Sakari Ailus
