On Mon, Jul 11, 2022 at 7:06 PM Laurent Pinchart
<laurent.pinchart@xxxxxxxxxxxxxxxx> wrote:
>
> The Image Sensing Interface (ISI) combines image processing pipelines
> with DMA engines to process and capture frames originating from a
> variety of sources. The inputs to the ISI go through Pixel Link
> interfaces, and their number and nature is SoC-dependent. They cover
> both capture interfaces (MIPI CSI-2 RX, HDMI RX) and memory inputs.
>
> Signed-off-by: Christian Hemp <c.hemp@xxxxxxxxx>
> Signed-off-by: Dong Aisheng <aisheng.dong@xxxxxxx>
> Signed-off-by: Guoniu Zhou <guoniu.zhou@xxxxxxx>
> Signed-off-by: Jacopo Mondi <jacopo@xxxxxxxxxx>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> Signed-off-by: Stefan Riedmueller <s.riedmueller@xxxxxxxxx>
> ---
> MAINTAINERS | 7 +
> drivers/media/platform/nxp/Kconfig | 2 +
> drivers/media/platform/nxp/Makefile | 1 +
> drivers/media/platform/nxp/imx8-isi/Kconfig | 22 +
> drivers/media/platform/nxp/imx8-isi/Makefile | 9 +
> .../platform/nxp/imx8-isi/imx8-isi-core.c | 646 +++++++
> .../platform/nxp/imx8-isi/imx8-isi-core.h | 394 +++++
> .../platform/nxp/imx8-isi/imx8-isi-crossbar.c | 529 ++++++
> .../platform/nxp/imx8-isi/imx8-isi-debug.c | 109 ++
> .../media/platform/nxp/imx8-isi/imx8-isi-hw.c | 651 +++++++
> .../platform/nxp/imx8-isi/imx8-isi-m2m.c | 858 ++++++++++
> .../platform/nxp/imx8-isi/imx8-isi-pipe.c | 867 ++++++++++
> .../platform/nxp/imx8-isi/imx8-isi-regs.h | 418 +++++
> .../platform/nxp/imx8-isi/imx8-isi-video.c | 1513 +++++++++++++++++
> 14 files changed, 6026 insertions(+)
> create mode 100644 drivers/media/platform/nxp/imx8-isi/Kconfig
> create mode 100644 drivers/media/platform/nxp/imx8-isi/Makefile
> create mode 100644 drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> create mode 100644 drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> create mode 100644 drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
> create mode 100644 drivers/media/platform/nxp/imx8-isi/imx8-isi-debug.c
> create mode 100644 drivers/media/platform/nxp/imx8-isi/imx8-isi-hw.c
> create mode 100644 drivers/media/platform/nxp/imx8-isi/imx8-isi-m2m.c
> create mode 100644 drivers/media/platform/nxp/imx8-isi/imx8-isi-pipe.c
> create mode 100644 drivers/media/platform/nxp/imx8-isi/imx8-isi-regs.h
> create mode 100644 drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c
>
(snip)
Laurent,
Thank you for this series.
> +
> +/* -----------------------------------------------------------------------------
> + * IRQ
> + */
> +
> +u32 mxc_isi_channel_irq_status(struct mxc_isi_pipe *pipe, bool clear)
> +{
> + u32 status;
> +
> + status = mxc_isi_read(pipe, CHNL_STS);
> + if (clear)
> + mxc_isi_write(pipe, CHNL_STS, status);
> +
> + return status;
> +}
> +
> +void mxc_isi_channel_irq_clear(struct mxc_isi_pipe *pipe)
> +{
> + mxc_isi_write(pipe, CHNL_STS, 0xffffffff);
> +}
> +
I was reading through the TRM for both Nano and Plus, and it seems
like there are a few bits in CHNL_STS that are always 0, but we're
setting them 1. Bit 0-7, 11-15 an 24 all show 0. The TRM says to
write a 1 to any bit that's set in order to clear it, and
mxc_isi_channel_irq_status can do this. Why not just have
mxc_isi_channel_irq_clear call mxc_isi_channel_irq_status(pipe, true)?
It seems clearer to me than writing a hard-coded hex value. This
way, it's only clearing any set bits and not arbitrarily writing 1's
to bit locations that might not be desired..
(snip)
> --
> Regards,
>
> Laurent Pinchart
>
