Re: [PATCH v2 2/2] phy: mtk-mipi-csi: add driver for CSI phy

From: AngeloGioacchino Del Regno
Date: Mon May 15 2023 - 10:34:11 EST

Il 15/05/23 16:07, Julien Stephan ha scritto:
On Mon, May 15, 2023 at 02:22:52PM +0200, AngeloGioacchino Del Regno wrote:
+#define CSIxB_OFFSET 0x1000

What if we grab two (or three?) iospaces from devicetree?

- base (global)
- csi_a
- csi_b

That would make it possible to maybe eventually extend this driver to more
versions (older or newer) of the CSI PHY IP without putting fixes offsets
inside of platform data structures and such.

Hi Angelo,
The register bank of the CSI port is divided into 2:
* from base address to base + 0x1000 (port A)
* from base + 0x1000 to base +0x2000 (port B)
Some CSI port can be configured in 4D1C mode (4 data + 1 clock) using
the whole register bank from base to base + 0x2000 or in 2D1C mode (2 data +
1 clock) and use either port A or port B.

For example mt8365 has CSI0 that can be used either in 4D1C mode or in
2 * 2D1C and CSI1 which can use only 4D1C mode

2D1C mode can not be tested and is not implemented in the driver so
I guess adding csi_a and csi_b reg value may be confusing?

What do you think?

Ok so we're talking about two data lanes per CSI port... it may still be
beneficial to split the two register regions as

reg-names = "csi-a", "csi-b"; (whoops, I actually used underscores before,
and that was a mistake, sorry!)

....but that would be actually good only if we are expecting to get a CSI
PHY in the future with four data lanes per port.

If you do *not* expect at all such a CSI PHY, or you do *not* expect such
a PHY to ever be compatible with this driver (read as: if you expect such
a PHY to be literally completely different from this one), then it would
not change much to have the registers split in two.

Another case in which it would make sense is if we were to get a PHY that
provides more than two CSI ports: in that case, we'd avoid platform data
machinery to check the number of actual ports in the IP, as we would be
just checking how many register regions we were given from the devicetree,
meaning that if we got "csi-a", "csi-b", "csi-c", "csi-d", we have four

Besides, another thing to think about is... yes you cannot test nor implement
2D1C mode in your submission, but this doesn't mean that others won't ever be
interested in this and that other people won't be actually implementing that;
Providing them with the right initial driver structure will surely make things
easier, encouraging other people from the community to spend their precious
time on the topic.