Hey Nicolas, Thanks for the review! On Thu, 28 Jan 2021 at 01:19, Nicolas Boichat <drinkcat@xxxxxxxxxxxx> wrote: > > On Wed, Jan 27, 2021 at 10:56 PM Robert Foss <robert.foss@xxxxxxxxxx> wrote: > > > > In order to support Qualcomm ISP hardware architectures that diverge > > from older architectures, the VFE subdevice driver needs to be refactored > > to better abstract the different ISP architectures. > > > > Gen1 represents the CAMSS ISP architecture. The ISP architecture developed > > after CAMSS, Titan, will be referred to as Gen2. > > > > Signed-off-by: Robert Foss <robert.foss@xxxxxxxxxx> > > --- > > [snip] > > diff --git a/drivers/media/platform/qcom/camss/camss-vfe-4-8.c b/drivers/media/platform/qcom/camss/camss-vfe-4-8.c > > new file mode 100644 > > index 000000000000..153e0e20664e > > --- /dev/null > > +++ b/drivers/media/platform/qcom/camss/camss-vfe-4-8.c > > [snip] > > +/* > > + * vfe_isr - VFE module interrupt handler > > + * @irq: Interrupt line > > + * @dev: VFE device > > + * > > + * Return IRQ_HANDLED on success > > + */ > > +static irqreturn_t vfe_isr(int irq, void *dev) > > +{ > > + struct vfe_device *vfe = dev; > > + u32 value0, value1; > > + int i, j; > > + > > + vfe->ops->isr_read(vfe, &value0, &value1); > > + > > + trace_printk("VFE: status0 = 0x%08x, status1 = 0x%08x\n", > > + value0, value1); > > Please do not use trace_printk in production code [1,2], it is only > meant for debug use. Consider using trace events, or dev_dbg. Ack, this is a copy paste error, I'll add a commit fixing all occurrences of this in the driver > > [1] https://elixir.bootlin.com/linux/v5.8/source/kernel/trace/trace.c#L3158 > [2] https://elixir.bootlin.com/linux/v5.8/source/include/linux/kernel.h#L766 > > > [snip]