* [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 an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.