* [PATCH v1 0/3] media: intel/ipu6: minor cleanups
@ 2025-03-06 13:06 Stanislaw Gruszka
2025-03-06 13:06 ` [PATCH v1 1/3] media: intel/ipu6: Remove unused IPU6_BUS_NAM Stanislaw Gruszka
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Stanislaw Gruszka @ 2025-03-06 13:06 UTC (permalink / raw)
To: linux-media; +Cc: Sakari Ailus, Bingbu Cao, 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
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v1 1/3] media: intel/ipu6: Remove unused IPU6_BUS_NAM 2025-03-06 13:06 [PATCH v1 0/3] media: intel/ipu6: minor cleanups Stanislaw Gruszka @ 2025-03-06 13:06 ` Stanislaw Gruszka 2025-03-06 13:06 ` [PATCH v1 2/3] media: intel/ipu6: Remove ipu6_buttress_ctrl started field Stanislaw Gruszka ` (2 subsequent siblings) 3 siblings, 0 replies; 10+ messages in thread From: Stanislaw Gruszka @ 2025-03-06 13:06 UTC (permalink / raw) To: linux-media; +Cc: Sakari Ailus, Bingbu Cao, 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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v1 2/3] media: intel/ipu6: Remove ipu6_buttress_ctrl started field 2025-03-06 13:06 [PATCH v1 0/3] media: intel/ipu6: minor cleanups Stanislaw Gruszka 2025-03-06 13:06 ` [PATCH v1 1/3] media: intel/ipu6: Remove unused IPU6_BUS_NAM Stanislaw Gruszka @ 2025-03-06 13:06 ` Stanislaw Gruszka 2025-03-06 13:06 ` [PATCH v1 3/3] media: intel/ipu6: Constify ipu6_buttress_ctrl structure Stanislaw Gruszka 2025-03-10 9:21 ` [PATCH v1 0/3] media: intel/ipu6: minor cleanups Hans de Goede 3 siblings, 0 replies; 10+ messages in thread From: Stanislaw Gruszka @ 2025-03-06 13:06 UTC (permalink / raw) To: linux-media; +Cc: Sakari Ailus, Bingbu Cao, 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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v1 3/3] media: intel/ipu6: Constify ipu6_buttress_ctrl structure 2025-03-06 13:06 [PATCH v1 0/3] media: intel/ipu6: minor cleanups Stanislaw Gruszka 2025-03-06 13:06 ` [PATCH v1 1/3] media: intel/ipu6: Remove unused IPU6_BUS_NAM Stanislaw Gruszka 2025-03-06 13:06 ` [PATCH v1 2/3] media: intel/ipu6: Remove ipu6_buttress_ctrl started field Stanislaw Gruszka @ 2025-03-06 13:06 ` Stanislaw Gruszka 2025-03-07 7:45 ` Sakari Ailus 2025-03-10 9:21 ` [PATCH v1 0/3] media: intel/ipu6: minor cleanups Hans de Goede 3 siblings, 1 reply; 10+ messages in thread From: Stanislaw Gruszka @ 2025-03-06 13:06 UTC (permalink / raw) To: linux-media; +Cc: Sakari Ailus, Bingbu Cao, 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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v1 3/3] media: intel/ipu6: Constify ipu6_buttress_ctrl structure 2025-03-06 13:06 ` [PATCH v1 3/3] media: intel/ipu6: Constify ipu6_buttress_ctrl structure Stanislaw Gruszka @ 2025-03-07 7:45 ` Sakari Ailus 2025-03-10 8:37 ` Stanislaw Gruszka 0 siblings, 1 reply; 10+ messages in thread From: Sakari Ailus @ 2025-03-07 7:45 UTC (permalink / raw) To: Stanislaw Gruszka; +Cc: linux-media, Bingbu Cao 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1 3/3] media: intel/ipu6: Constify ipu6_buttress_ctrl structure 2025-03-07 7:45 ` Sakari Ailus @ 2025-03-10 8:37 ` Stanislaw Gruszka 2025-03-10 10:30 ` Sakari Ailus 0 siblings, 1 reply; 10+ messages in thread From: Stanislaw Gruszka @ 2025-03-10 8:37 UTC (permalink / raw) To: Sakari Ailus; +Cc: linux-media, Bingbu Cao 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1 3/3] media: intel/ipu6: Constify ipu6_buttress_ctrl structure 2025-03-10 8:37 ` Stanislaw Gruszka @ 2025-03-10 10:30 ` Sakari Ailus 2025-03-10 11:18 ` Stanislaw Gruszka 0 siblings, 1 reply; 10+ messages in thread From: Sakari Ailus @ 2025-03-10 10:30 UTC (permalink / raw) To: Stanislaw Gruszka; +Cc: linux-media, Bingbu Cao 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1 3/3] media: intel/ipu6: Constify ipu6_buttress_ctrl structure 2025-03-10 10:30 ` Sakari Ailus @ 2025-03-10 11:18 ` Stanislaw Gruszka 2025-03-10 13:01 ` Sakari Ailus 0 siblings, 1 reply; 10+ messages in thread From: Stanislaw Gruszka @ 2025-03-10 11:18 UTC (permalink / raw) To: Sakari Ailus; +Cc: linux-media, Bingbu Cao 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1 3/3] media: intel/ipu6: Constify ipu6_buttress_ctrl structure 2025-03-10 11:18 ` Stanislaw Gruszka @ 2025-03-10 13:01 ` Sakari Ailus 0 siblings, 0 replies; 10+ messages in thread From: Sakari Ailus @ 2025-03-10 13:01 UTC (permalink / raw) To: Stanislaw Gruszka; +Cc: linux-media, Bingbu Cao 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1 0/3] media: intel/ipu6: minor cleanups 2025-03-06 13:06 [PATCH v1 0/3] media: intel/ipu6: minor cleanups Stanislaw Gruszka ` (2 preceding siblings ...) 2025-03-06 13:06 ` [PATCH v1 3/3] media: intel/ipu6: Constify ipu6_buttress_ctrl structure Stanislaw Gruszka @ 2025-03-10 9:21 ` Hans de Goede 3 siblings, 0 replies; 10+ messages in thread From: Hans de Goede @ 2025-03-10 9:21 UTC (permalink / raw) To: Stanislaw Gruszka, linux-media; +Cc: Sakari Ailus, Bingbu Cao 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2025-03-10 13:02 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-03-06 13:06 [PATCH v1 0/3] media: intel/ipu6: minor cleanups Stanislaw Gruszka 2025-03-06 13:06 ` [PATCH v1 1/3] media: intel/ipu6: Remove unused IPU6_BUS_NAM Stanislaw Gruszka 2025-03-06 13:06 ` [PATCH v1 2/3] media: intel/ipu6: Remove ipu6_buttress_ctrl started field Stanislaw Gruszka 2025-03-06 13:06 ` [PATCH v1 3/3] media: intel/ipu6: Constify ipu6_buttress_ctrl structure Stanislaw Gruszka 2025-03-07 7:45 ` Sakari Ailus 2025-03-10 8:37 ` Stanislaw Gruszka 2025-03-10 10:30 ` Sakari Ailus 2025-03-10 11:18 ` Stanislaw Gruszka 2025-03-10 13:01 ` Sakari Ailus 2025-03-10 9:21 ` [PATCH v1 0/3] media: intel/ipu6: minor cleanups Hans de Goede
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox