[PATCH v1 0/3] media: intel/ipu6: minor cleanups

[PATCH v1 0/3] media: intel/ipu6: minor cleanups

[Stanislaw Gruszka]

Few small cleanups

Stanislaw Gruszka (3):
  media: intel/ipu6: Remove unused IPU6_BUS_NAM
  media: intel/ipu6: Remove ipu6_buttress_ctrl started field
  media: intel/ipu6: Constify ipu6_buttress_ctrl structure

 drivers/media/pci/intel/ipu6/ipu6-bus.c      | 2 +-
 drivers/media/pci/intel/ipu6/ipu6-bus.h      | 6 ++----
 drivers/media/pci/intel/ipu6/ipu6-buttress.c | 4 +---
 drivers/media/pci/intel/ipu6/ipu6-buttress.h | 4 ++--
 4 files changed, 6 insertions(+), 10 deletions(-)

-- 
2.34.1

[PATCH v1 1/3] media: intel/ipu6: Remove unused IPU6_BUS_NAM

[Stanislaw Gruszka]

Remove unused define.

Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
---
 drivers/media/pci/intel/ipu6/ipu6-bus.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/media/pci/intel/ipu6/ipu6-bus.h b/drivers/media/pci/intel/ipu6/ipu6-bus.h
index bb4926dfdf08..ebf470806a74 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-bus.h
+++ b/drivers/media/pci/intel/ipu6/ipu6-bus.h
@@ -15,8 +15,6 @@
 struct firmware;
 struct pci_dev;
 
-#define IPU6_BUS_NAME	IPU6_NAME "-bus"
-
 struct ipu6_buttress_ctrl;
 
 struct ipu6_bus_device {
-- 
2.34.1

[PATCH v1 2/3] media: intel/ipu6: Remove ipu6_buttress_ctrl started field

[Stanislaw Gruszka]

We assign to ->started field but newer read back, the field can be removed.

Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
---
 drivers/media/pci/intel/ipu6/ipu6-buttress.c | 2 --
 drivers/media/pci/intel/ipu6/ipu6-buttress.h | 1 -
 2 files changed, 3 deletions(-)

diff --git a/drivers/media/pci/intel/ipu6/ipu6-buttress.c b/drivers/media/pci/intel/ipu6/ipu6-buttress.c
index d8db5aa5d528..787fcbd1df09 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-buttress.c
+++ b/drivers/media/pci/intel/ipu6/ipu6-buttress.c
@@ -478,8 +478,6 @@ int ipu6_buttress_power(struct device *dev, struct ipu6_buttress_ctrl *ctrl,
 		dev_err(&isp->pdev->dev,
 			"Change power status timeout with 0x%x\n", val);
 
-	ctrl->started = !ret && on;
-
 	mutex_unlock(&isp->buttress.power_mutex);
 
 	return ret;
diff --git a/drivers/media/pci/intel/ipu6/ipu6-buttress.h b/drivers/media/pci/intel/ipu6/ipu6-buttress.h
index 482978c2a09d..4b9763acdfdd 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-buttress.h
+++ b/drivers/media/pci/intel/ipu6/ipu6-buttress.h
@@ -26,7 +26,6 @@ struct ipu6_buttress_ctrl {
 	u32 freq_ctl, pwr_sts_shift, pwr_sts_mask, pwr_sts_on, pwr_sts_off;
 	unsigned int ratio;
 	unsigned int qos_floor;
-	bool started;
 };
 
 struct ipu6_buttress_ipc {
-- 
2.34.1

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

[Stanislaw Gruszka]

Make ipu6_buttress_ctrl constant since it is not modified
any longer.

Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
---
 drivers/media/pci/intel/ipu6/ipu6-bus.c      | 2 +-
 drivers/media/pci/intel/ipu6/ipu6-bus.h      | 4 ++--
 drivers/media/pci/intel/ipu6/ipu6-buttress.c | 2 +-
 drivers/media/pci/intel/ipu6/ipu6-buttress.h | 3 ++-
 4 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/media/pci/intel/ipu6/ipu6-bus.c b/drivers/media/pci/intel/ipu6/ipu6-bus.c
index 37d88ddb6ee7..5cee2748983b 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-bus.c
+++ b/drivers/media/pci/intel/ipu6/ipu6-bus.c
@@ -82,7 +82,7 @@ static void ipu6_bus_release(struct device *dev)
 
 struct ipu6_bus_device *
 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,
 			   char *name)
 {
 	struct auxiliary_device *auxdev;
diff --git a/drivers/media/pci/intel/ipu6/ipu6-bus.h b/drivers/media/pci/intel/ipu6/ipu6-bus.h
index ebf470806a74..b790f9cc37e3 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-bus.h
+++ b/drivers/media/pci/intel/ipu6/ipu6-bus.h
@@ -25,7 +25,7 @@ struct ipu6_bus_device {
 	void *pdata;
 	struct ipu6_mmu *mmu;
 	struct ipu6_device *isp;
-	struct ipu6_buttress_ctrl *ctrl;
+	const struct ipu6_buttress_ctrl *ctrl;
 	u64 dma_mask;
 	const struct firmware *fw;
 	struct sg_table fw_sgt;
@@ -48,7 +48,7 @@ struct ipu6_auxdrv_data {
 
 struct ipu6_bus_device *
 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,
 			   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)
 {
 	struct ipu6_device *isp = to_ipu6_bus_device(dev)->isp;
diff --git a/drivers/media/pci/intel/ipu6/ipu6-buttress.h b/drivers/media/pci/intel/ipu6/ipu6-buttress.h
index 4b9763acdfdd..cb008964f870 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-buttress.h
+++ b/drivers/media/pci/intel/ipu6/ipu6-buttress.h
@@ -65,7 +65,8 @@ int ipu6_buttress_map_fw_image(struct ipu6_bus_device *sys,
 			       struct sg_table *sgt);
 void ipu6_buttress_unmap_fw_image(struct ipu6_bus_device *sys,
 				  struct sg_table *sgt);
-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);
 bool ipu6_buttress_get_secure_mode(struct ipu6_device *isp);
 int ipu6_buttress_authenticate(struct ipu6_device *isp);
-- 
2.34.1

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

[Sakari Ailus]

Hi Stanislaw,

Thanks for the set. A few minor comments below.

On Thu, Mar 06, 2025 at 02:06:29PM +0100, Stanislaw Gruszka wrote:
> Make ipu6_buttress_ctrl constant since it is not modified
> any longer.

Fits on previous line.

> 
> Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
> ---
>  drivers/media/pci/intel/ipu6/ipu6-bus.c      | 2 +-
>  drivers/media/pci/intel/ipu6/ipu6-bus.h      | 4 ++--
>  drivers/media/pci/intel/ipu6/ipu6-buttress.c | 2 +-
>  drivers/media/pci/intel/ipu6/ipu6-buttress.h | 3 ++-
>  4 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/media/pci/intel/ipu6/ipu6-bus.c b/drivers/media/pci/intel/ipu6/ipu6-bus.c
> index 37d88ddb6ee7..5cee2748983b 100644
> --- a/drivers/media/pci/intel/ipu6/ipu6-bus.c
> +++ b/drivers/media/pci/intel/ipu6/ipu6-bus.c
> @@ -82,7 +82,7 @@ static void ipu6_bus_release(struct device *dev)
>  
>  struct ipu6_bus_device *
>  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,
>  			   char *name)
>  {
>  	struct auxiliary_device *auxdev;
> diff --git a/drivers/media/pci/intel/ipu6/ipu6-bus.h b/drivers/media/pci/intel/ipu6/ipu6-bus.h
> index ebf470806a74..b790f9cc37e3 100644
> --- a/drivers/media/pci/intel/ipu6/ipu6-bus.h
> +++ b/drivers/media/pci/intel/ipu6/ipu6-bus.h
> @@ -25,7 +25,7 @@ struct ipu6_bus_device {
>  	void *pdata;
>  	struct ipu6_mmu *mmu;
>  	struct ipu6_device *isp;
> -	struct ipu6_buttress_ctrl *ctrl;
> +	const struct ipu6_buttress_ctrl *ctrl;
>  	u64 dma_mask;
>  	const struct firmware *fw;
>  	struct sg_table fw_sgt;
> @@ -48,7 +48,7 @@ struct ipu6_auxdrv_data {
>  
>  struct ipu6_bus_device *
>  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.

>  {
>  	struct ipu6_device *isp = to_ipu6_bus_device(dev)->isp;
> diff --git a/drivers/media/pci/intel/ipu6/ipu6-buttress.h b/drivers/media/pci/intel/ipu6/ipu6-buttress.h
> index 4b9763acdfdd..cb008964f870 100644
> --- a/drivers/media/pci/intel/ipu6/ipu6-buttress.h
> +++ b/drivers/media/pci/intel/ipu6/ipu6-buttress.h
> @@ -65,7 +65,8 @@ int ipu6_buttress_map_fw_image(struct ipu6_bus_device *sys,
>  			       struct sg_table *sgt);
>  void ipu6_buttress_unmap_fw_image(struct ipu6_bus_device *sys,
>  				  struct sg_table *sgt);
> -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);

And this fits on the previous line, too.

>  bool ipu6_buttress_get_secure_mode(struct ipu6_device *isp);
>  int ipu6_buttress_authenticate(struct ipu6_device *isp);

-- 
Regards,

Sakari Ailus

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

[Stanislaw Gruszka]

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.

However clang-format change this to:

int ipu6_buttress_power(struct device *dev,
			const struct ipu6_buttress_ctrl *ctrl, bool on)

which is less than 80 characters. So I guess I need to use auto formatter
for lines I change (for whole file clang-format change lot unrelated things).

Regards
Stanislaw

Re: [PATCH v1 0/3] media: intel/ipu6: minor cleanups

[Hans de Goede]

Hi Stanislaw,

On 6-Mar-25 14:06, Stanislaw Gruszka wrote:
> Few small cleanups
> 
> Stanislaw Gruszka (3):
>   media: intel/ipu6: Remove unused IPU6_BUS_NAM
>   media: intel/ipu6: Remove ipu6_buttress_ctrl started field
>   media: intel/ipu6: Constify ipu6_buttress_ctrl structure

Thanks, all 3 patches look good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

for the series.

Regards,

Hans

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

[Sakari Ailus]

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

> 
> However clang-format change this to:
> 
> int ipu6_buttress_power(struct device *dev,
> 			const struct ipu6_buttress_ctrl *ctrl, bool on)
> 
> which is less than 80 characters. So I guess I need to use auto formatter
> for lines I change (for whole file clang-format change lot unrelated things).

-- 
Kind regards,

Sakari Ailus

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

[Stanislaw Gruszka]

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

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

[Sakari Ailus]

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