| Message ID | 20240430131931.166012-1-julien.massot@collabora.com |
|---|---|
| Headers | show |
| Series | Add support for MAX96714/F and MAX96717/F GMSL2 ser/des | expand |
Hi Julien, On Tue, Apr 30, 2024 at 03:19:26PM +0200, Julien Massot wrote: > Change since v6: > - Remove mention of C-PHY for MAX96717, this serializer is D-PHY only > - Remove bus-type requirement for MAX96717 > - Minor changes requested by Sakari > - Workaround a MAX96717 issue, which occurs when stopping > the CSI source before stopping the MAX96717 CSI receiver. > > Power management is not included in this patchset. The GMSL link is > not always resuming when the deserializer is suspended without > suspending the serializer. > > Change since v5: > - Reverse fallback logic: max9671{4,7} can fallback to max9671{4,7}F > - use const instead of enum for max9671{4,7}f compatible as suggested > > Change since v4: > - Add support for MAX96717 and MAX96714 and use them as a fallback for > MAX96717F and MAX96714F respectively > - The drivers are now compatible with MAX96717 and MAX96714 since no change in > the logic is needed > - Reference 'i2c-gate' instead of 'i2c-controller' in the bindings > > Change since v3: > - bindings > - Renamed bindings to drop the 'f' suffix > - Add bus type to MAX96717 and remove from MAX9674 > - Add lane-polarities to both bindings > > - drivers > - Address changes requested by Sakari in v3 > - use v4l2_subdev_s_stream_helper for MAX96714 > - do not init regmap twice in the MAX96714 driver > - Fix compilations on 32 bits platforms > > Change since v2: > - Convert drivers to use CCI helpers > - Use generic node name > - Use 'powerdown' as gpio name instead of 'enable' > - Add pattern generator support for MAX96714 > > These patches add support for Maxim MAX96714F deserializer and > MAX96717F serializer. > > MAX96714F has one GMSL2 input port and one CSI2 4 lanes output port, > MAX96717F has one CSI2 input port and one GMSL2 output port. > > The drivers support the tunnel mode where all the > CSI2 traffic coming from an imager is replicated through the deserializer > output port. > > Both MAX96714F and MAX96717F are limited to a 3Gbps forward link rate > leaving a maximum of 2.6Gbps for the video payload. Thanks for your great work! :) I test your series with the following hw: alvium camera (GM2-319c) -> max96717 (non f variant) -> max96716a -> imx8mp-msc-sm2s-ep2 board Note: max96716a is pretty similar to max96714. I change only: #define MAX96714_DEVICE_ID 0xb6 #define MAX96714F_DEVICE_ID 0xbe And swapping lanes as you suggest: max96714_csi0_out: endpoint { data-lanes = <3 4 1 2>; link-frequencies = /bits/ 64 <750000000>; remote-endpoint = <&mipi_csi_0_in>; }; On my current setup csi2 to gmsl2 board is always powered on then, I have ERRB pin that is triggered at uboot lvl because is not handled. I think we can andle later this case I can suggest to clear/unclear the ERRB_EN bit of REG5(0x5) in the very beginning of the probe. Apart of this All is working properly on my side and also on Martin side (that is in CC) :) I'm able to stream properly using: # SETUP TOPOLOGY LINKS media-ctl --links "'alvium-csi2 3-003c':0->'max96717 6-0040':0[1]" media-ctl --links "'max96717 6-0040':1->'max96714 3-0028':0[1]" media-ctl --links "'max96714 3-0028':1->'csis-32e40000.csi':0[1]" media-ctl --links "'csis-32e40000.csi':1->'crossbar':0[1]" media-ctl --links "'crossbar':3->'mxc_isi.0':0[1]" media-ctl --links "'mxc_isi.0':1->'mxc_isi.0.capture':0[1]" # SETUP TOPOLOGY ENTITIES media-ctl -d /dev/media0 --set-v4l2 '"alvium-csi2 3-003c":0[fmt:UYVY8_1X16/1280x800@1/60 field:none]' media-ctl -d /dev/media0 --set-v4l2 '"max96717 6-0040":0[fmt:UYVY8_1X16/1280x800 field:none]' media-ctl -d /dev/media0 --set-v4l2 '"max96714 3-0028":0[fmt:UYVY8_1X16/1280x800 field:none]' media-ctl -d /dev/media0 --set-v4l2 '"csis-32e40000.csi":0[fmt:UYVY8_1X16/1280x800 field:none]' media-ctl -d /dev/media0 --set-v4l2 '"crossbar":0[fmt:UYVY8_1X16/1280x800 field:none]' media-ctl -d /dev/media0 --set-v4l2 '"mxc_isi.0":0[fmt:UYVY8_1X16/1280x800 field:none]' gst-launch-1.0 v4l2src io-mode=dmabuf blocksize=76800 ! video/x-raw,format=YUY2,width=1280,height=800 ! videoconvert ! waylandsink sync=false # TOPOLOGY Media controller API version 6.9.0 Media device information ------------------------ driver mxc-isi model FSL Capture Media Device serial bus info platform:32e00000.isi hw revision 0x0 driver version 6.9.0 Device topology - entity 1: crossbar (5 pads, 4 links, 0 routes) type V4L2 subdev subtype Unknown flags 0 device node name /dev/v4l-subdev0 pad0: Sink [stream:0 fmt:UYVY8_1X16/1280x800 field:none] <- "csis-32e40000.csi":1 [ENABLED,IMMUTABLE] pad1: Sink [stream:0 fmt:UYVY8_1X16/1920x1080 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:lim-range] pad2: Sink <- "mxc_isi.output":0 [ENABLED,IMMUTABLE] pad3: Source [stream:0 fmt:UYVY8_1X16/1280x800 field:none] -> "mxc_isi.0":0 [ENABLED,IMMUTABLE] pad4: Source [stream:0 fmt:UYVY8_1X16/1920x1080 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:lim-range] -> "mxc_isi.1":0 [ENABLED,IMMUTABLE] - entity 7: mxc_isi.0 (2 pads, 2 links, 0 routes) type V4L2 subdev subtype Unknown flags 0 device node name /dev/v4l-subdev1 pad0: Sink [stream:0 fmt:UYVY8_1X16/1280x800 field:none compose.bounds:(0,0)/1280x800 compose:(0,0)/1280x800] <- "crossbar":3 [ENABLED,IMMUTABLE] pad1: Source [stream:0 fmt:YUV8_1X24/1280x800 field:none colorspace:jpeg xfer:srgb ycbcr:601 quantization:full-range crop.bounds:(0,0)/1280x800 crop:(0,0)/1280x800] -> "mxc_isi.0.capture":0 [ENABLED,IMMUTABLE] - entity 10: mxc_isi.0.capture (1 pad, 1 link) type Node subtype V4L flags 0 device node name /dev/video0 pad0: Sink <- "mxc_isi.0":1 [ENABLED,IMMUTABLE] - entity 18: mxc_isi.1 (2 pads, 2 links, 0 routes) type V4L2 subdev subtype Unknown flags 0 device node name /dev/v4l-subdev2 pad0: Sink [stream:0 fmt:UYVY8_1X16/1920x1080 field:none colorspace:jpeg xfer:srgb ycbcr:601 quantization:full-range compose.bounds:(0,0)/1920x1080 compose:(0,0)/1920x1080] <- "crossbar":4 [ENABLED,IMMUTABLE] pad1: Source [stream:0 fmt:YUV8_1X24/1920x1080 field:none colorspace:jpeg xfer:srgb ycbcr:601 quantization:full-range crop.bounds:(0,0)/1920x1080 crop:(0,0)/1920x1080] -> "mxc_isi.1.capture":0 [ENABLED,IMMUTABLE] - entity 21: mxc_isi.1.capture (1 pad, 1 link) type Node subtype V4L flags 0 device node name /dev/video1 pad0: Sink <- "mxc_isi.1":1 [ENABLED,IMMUTABLE] - entity 29: mxc_isi.output (1 pad, 1 link) type Node subtype V4L flags 0 pad0: Source -> "crossbar":2 [ENABLED,IMMUTABLE] - entity 36: csis-32e40000.csi (2 pads, 2 links, 0 routes) type V4L2 subdev subtype Unknown flags 0 device node name /dev/v4l-subdev3 pad0: Sink [stream:0 fmt:UYVY8_1X16/1280x800 field:none] <- "max96714 3-0028":1 [ENABLED] pad1: Source [stream:0 fmt:UYVY8_1X16/1280x800 field:none] -> "crossbar":0 [ENABLED,IMMUTABLE] - entity 41: max96714 3-0028 (2 pads, 2 links, 0 routes) type V4L2 subdev subtype Unknown flags 0 device node name /dev/v4l-subdev4 pad0: Sink [stream:0 fmt:UYVY8_1X16/1280x800 field:none] <- "max96717 6-0040":1 [ENABLED,IMMUTABLE] pad1: Source [stream:0 fmt:UYVY8_1X16/1280x800 field:none] -> "csis-32e40000.csi":0 [ENABLED] - entity 46: max96717 6-0040 (2 pads, 2 links, 0 routes) type V4L2 subdev subtype Unknown flags 0 device node name /dev/v4l-subdev5 pad0: Sink [stream:0 fmt:UYVY8_1X16/1280x800 field:none] <- "alvium-csi2 3-003c":0 [ENABLED,IMMUTABLE] pad1: Source [stream:0 fmt:UYVY8_1X16/1280x800 field:none] -> "max96714 3-0028":0 [ENABLED,IMMUTABLE] - entity 51: alvium-csi2 3-003c (1 pad, 1 link, 0 routes) type V4L2 subdev subtype Sensor flags 0 device node name /dev/v4l-subdev6 pad0: Source [stream:0 fmt:UYVY8_1X16/1280x800@1/60 field:none crop.bounds:(0,0)/2064x1544 crop:(0,0)/640x480] -> "max96717 6-0040":0 [ENABLED,IMMUTABLE] All this test was done on top of linux-media tree 6.9.0-rc2. Here my tags for all the series: Reviewed-by: Tommaso Merciai <tomm.merciai@gmail.com> Tested-by: Tommaso Merciai <tomm.merciai@gmail.com> Thanks again for your great work. Hope this series will be merged soon :) Regards, Tommaso > > Julien Massot (9): > dt-bindings: media: add Maxim MAX96717 GMSL2 Serializer > dt-bindings: media: add Maxim MAX96714 GMSL2 Deserializer > media: i2c: add MAX96717 driver > media: i2c: add MAX96714 driver > drivers: media: max96717: stop the csi receiver before the source > > .../bindings/media/i2c/maxim,max96714.yaml | 174 +++ > .../bindings/media/i2c/maxim,max96717.yaml | 157 +++ > MAINTAINERS | 14 + > drivers/media/i2c/Kconfig | 34 + > drivers/media/i2c/Makefile | 2 + > drivers/media/i2c/max96714.c | 1024 +++++++++++++++++ > drivers/media/i2c/max96717.c | 927 +++++++++++++++ > 7 files changed, 2332 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/i2c/maxim,max96714.yaml > create mode 100644 Documentation/devicetree/bindings/media/i2c/maxim,max96717.yaml > create mode 100644 drivers/media/i2c/max96714.c > create mode 100644 drivers/media/i2c/max96717.c > > -- > 2.44.0 > >
Hi, On 30/04/2024 16:19, Julien Massot wrote: > Change since v6: > - Remove mention of C-PHY for MAX96717, this serializer is D-PHY only > - Remove bus-type requirement for MAX96717 > - Minor changes requested by Sakari > - Workaround a MAX96717 issue, which occurs when stopping > the CSI source before stopping the MAX96717 CSI receiver. > > Power management is not included in this patchset. The GMSL link is > not always resuming when the deserializer is suspended without > suspending the serializer. > > Change since v5: > - Reverse fallback logic: max9671{4,7} can fallback to max9671{4,7}F > - use const instead of enum for max9671{4,7}f compatible as suggested > > Change since v4: > - Add support for MAX96717 and MAX96714 and use them as a fallback for > MAX96717F and MAX96714F respectively > - The drivers are now compatible with MAX96717 and MAX96714 since no change in > the logic is needed > - Reference 'i2c-gate' instead of 'i2c-controller' in the bindings > > Change since v3: > - bindings > - Renamed bindings to drop the 'f' suffix > - Add bus type to MAX96717 and remove from MAX9674 > - Add lane-polarities to both bindings > > - drivers > - Address changes requested by Sakari in v3 > - use v4l2_subdev_s_stream_helper for MAX96714 > - do not init regmap twice in the MAX96714 driver > - Fix compilations on 32 bits platforms > > Change since v2: > - Convert drivers to use CCI helpers > - Use generic node name > - Use 'powerdown' as gpio name instead of 'enable' > - Add pattern generator support for MAX96714 > > These patches add support for Maxim MAX96714F deserializer and > MAX96717F serializer. > > MAX96714F has one GMSL2 input port and one CSI2 4 lanes output port, > MAX96717F has one CSI2 input port and one GMSL2 output port. > > The drivers support the tunnel mode where all the > CSI2 traffic coming from an imager is replicated through the deserializer > output port. > > Both MAX96714F and MAX96717F are limited to a 3Gbps forward link rate > leaving a maximum of 2.6Gbps for the video payload. (I see this mail turned out to be a collection of thoughts rather than clear questions... Bear with me =)) I know I'm very late to this party, and perhaps these topics have already been discussed, but as I will most likely be doing some GMSL work in the future I wanted to ask these questions. My main questions/concerns are related to the i2c and the representation of the links in the DT. First, I know these particular devices are one input, one output serializer and deserializer, so there's not much to do wrt. i2c translation/gating. But even here I wonder how does one support a case where a single local i2c bus would have two deserializer devices (with different i2c addresses), connected to two identical camera modules? Controlling the deserializers would work fine, but as the serializers and the remote peripherals (sensor) would answer to identical i2c addresses, it would conflict and not work. If I understand the HW docs right, a way (maybe there are others?) to handle this would be: - deser probes, but keeps the link disabled by default - deser reads the initial serializer i2c address from the DT, but also a new address which we want the serializer to have (which doesn't conflict with the other serializer) - deser enables the link and immediately (how to be sure the other deser driver doesn't do this at the same time?) sends a write to the serializer's DEV_ADDR, changing the serializer's i2c address. - deser can now add the serializer linux i2c device, so that the serializer can probe - the serializer should prevent any remote i2c transactions until it has written the SRC_A/B and DST_A/B registers, to get translation for the remote peripherals (or maybe the deser driver should do this part too). Am I on the right track with the above? Now, maybe having such a HW config, two deserializers on a single i2c bus, doesn't happen in real life, but this issue comes up with multi-port deserializers. And while those deserializers are different devices than what's added in this series, the serializers used may be the same as here. This means the serializer drivers and DT bindings should be such that multi-port deserializers can be supported. As I said, I'm late (and new) to this party, and struggling to consume and understand all the related specs and drivers, so I hope you can give some insight into how all this might be implemented in the future =). Have you looked at the FPD-Link drivers (ds90ub9xx)? The i2c management is a bit different with those (with my current understanding, a bit saner...), but I wonder if similar style would help here, or if the i2c-atr could be utilized. It would be nice (but I guess not really mandatory in any way) to have similar style in DT bindings for all ser-des solutions. To summarize the i2c management on both FPD-Link and GMSL (if I have understood it right): In FPD-Link the deserializer does it all: it has registers for the serializer i2c aliases, and for i2c address translation (per port). So when the deser probes, it can program suitable i2c addresses (based on data from DT), which will be the addresses visible on the main i2c bus, and thus there are never any conflicts. In addition to that, the drivers utilize i2c-atr, which means that new linux i2c busses are created for each serializer. E.g. the deser might be, say, on i2c bus 4, and also the serializers, via their i2c aliases, would be accessible bus 4. When the serializer drivers probe they will create new i2c busses with i2c-atr. So with a 4 port deserializer we might get i2c busses 5, 6, 7 and 8. The linux i2c devices for remote peripherals (sensors mainly) would be created on these busses with their real i2c addresses. When a sensor driver does an i2c write to its device, the i2c-atr will catch the write, change the address according to the translation table, and do an actual write on the i2c bus 4. This would result in the deser HW to catch this write, switch the address back to the "real" one, and send it to the appropriate serializer, which would then send the i2c transaction on its i2c bus. In GMSL the deser just forwards everything it sees on the i2c bus, if a port is enabled. The deser has no other support related to i2c. The serializers have DEV_ADDR register which can be used to change the address the serializers respond to, and the serializers also have i2c translation for two remote peripherals. But if the i2c translation is used, it would mean that, say, the sensor driver would need to use the "virtual" address, not the real one to communicate with the sensor device, which doesn't sound right... You have used i2c-gate for both the deser and the ser. I don't have experience with i2c-gate, but how can we manage the serializer i2c address and the i2c address translation with it? One difference with the FPD-Link and this series' DT bindings is that I have a "links" node in the deser, instead of just adding the serializers under an i2c node. In FPD-Link case this allowed me to better represent the hardware and the configuration needed. So... Perhaps my bottom line question is: do we need something similar to what the FPD-Link uses (links, i2c-atr) to fully support GMSL devices? And also, if we merge the DT bindings in this series, will we have gone into a corner wrt. how we can manage the i2c? Tomi
On Thu, Jun 06, 2024 at 04:34:19PM +0300, Tomi Valkeinen wrote: > Hi, > > On 30/04/2024 16:19, Julien Massot wrote: > > Change since v6: > > - Remove mention of C-PHY for MAX96717, this serializer is D-PHY only > > - Remove bus-type requirement for MAX96717 > > - Minor changes requested by Sakari > > - Workaround a MAX96717 issue, which occurs when stopping > > the CSI source before stopping the MAX96717 CSI receiver. > > > > Power management is not included in this patchset. The GMSL link is > > not always resuming when the deserializer is suspended without > > suspending the serializer. > > > > Change since v5: > > - Reverse fallback logic: max9671{4,7} can fallback to max9671{4,7}F > > - use const instead of enum for max9671{4,7}f compatible as suggested > > > > Change since v4: > > - Add support for MAX96717 and MAX96714 and use them as a fallback for > > MAX96717F and MAX96714F respectively > > - The drivers are now compatible with MAX96717 and MAX96714 since no change in > > the logic is needed > > - Reference 'i2c-gate' instead of 'i2c-controller' in the bindings > > > > Change since v3: > > - bindings > > - Renamed bindings to drop the 'f' suffix > > - Add bus type to MAX96717 and remove from MAX9674 > > - Add lane-polarities to both bindings > > > > - drivers > > - Address changes requested by Sakari in v3 > > - use v4l2_subdev_s_stream_helper for MAX96714 > > - do not init regmap twice in the MAX96714 driver > > - Fix compilations on 32 bits platforms > > > > Change since v2: > > - Convert drivers to use CCI helpers > > - Use generic node name > > - Use 'powerdown' as gpio name instead of 'enable' > > - Add pattern generator support for MAX96714 > > > > These patches add support for Maxim MAX96714F deserializer and > > MAX96717F serializer. > > > > MAX96714F has one GMSL2 input port and one CSI2 4 lanes output port, > > MAX96717F has one CSI2 input port and one GMSL2 output port. > > > > The drivers support the tunnel mode where all the > > CSI2 traffic coming from an imager is replicated through the deserializer > > output port. > > > > Both MAX96714F and MAX96717F are limited to a 3Gbps forward link rate > > leaving a maximum of 2.6Gbps for the video payload. > > (I see this mail turned out to be a collection of thoughts rather than > clear questions... Bear with me =)) > > I know I'm very late to this party, and perhaps these topics have > already been discussed, but as I will most likely be doing some GMSL > work in the future I wanted to ask these questions. My main > questions/concerns are related to the i2c and the representation of the > links in the DT. > > First, I know these particular devices are one input, one output > serializer and deserializer, so there's not much to do wrt. i2c > translation/gating. But even here I wonder how does one support a case > where a single local i2c bus would have two deserializer devices (with > different i2c addresses), connected to two identical camera modules? > > Controlling the deserializers would work fine, but as the serializers > and the remote peripherals (sensor) would answer to identical i2c > addresses, it would conflict and not work. > > If I understand the HW docs right, a way (maybe there are others?) to > handle this would be: > - deser probes, but keeps the link disabled by default I don't know if the GMSL2 deserializers typically start with the link enabled or disabled by default, but I assume you mean here that early in the probe sequence the driver would disable the link if it's enabled by default. Note that the forward (from serializer to deserializer, carrying video and I2C "replies") and reverse (from deserializer to serializer, carrying I2C "requests") can be controlled separately in GMSL1. I don't know if GMSL2 allows doing the same. It would be good to be precise in the discussions. > - deser reads the initial serializer i2c address from the DT, but also a > new address which we want the serializer to have (which doesn't conflict > with the other serializer) There's also the devices behind the serializer that we need to consider. There will typically be one (a camera sensor), but possibly multiple (microcontrollers, EEPROMs, ISPs, ...) such devices. With GMSL1, the serializer has the ability to perform address translation for up to two addresses, plus the ability to reprogram the serializer address. If we end up having more than two devices behind the serializers, address translation won't be good enough. For GMSL1, we decided not to reprogram the serializer address, but instead to implement an I2C mux in the deserializer driver. The deserializer would disable all reverse links, and enable them selectively through the I2C mux API. This ensured that only one reverse link would be enabled at a time per deserializer. It didn't address the issue of multiple deserializers on the same I2C bus. There's also the issue of power management to consider. Power to the cameras and deserializers could be cut when they're unused. We need to ensure they can come back up with I2C conflicts, as they would be reset to their default address. I don't know if this is a solvable problem in the generic case with GMSL1 and GMSL2. > - deser enables the link and immediately (how to be sure the other deser > driver doesn't do this at the same time?) sends a write to the > serializer's DEV_ADDR, changing the serializer's i2c address. We faced a similar issue when we started working on the MAX9286 driver (a quad GMSL1 deserializer). Our test platform had two MAX9286 on the same I2C bus (for a total of 8 cameras), and all cameras were identical. The initial driver implementation posted to the list ([1]) included a mechanism to handle this problem: /* * We can have multiple MAX9286 instances on the same physical I2C * bus, and I2C children behind ports of separate MAX9286 instances * having the same I2C address. As the MAX9286 starts by default with * all ports enabled, we need to disable all ports on all MAX9286 * instances before proceeding to further initialize the devices and * instantiate children. * * Start by just disabling all channels on the current device. Then, * if all other MAX9286 on the parent bus have been probed, proceed * to initialize them all, including the current one. */ max9286_i2c_mux_close(dev); /* * The MAX9286 initialises with auto-acknowledge enabled by default. * This means that if multiple MAX9286 devices are connected to an I2C * bus, another MAX9286 could ack I2C transfers meant for a device on * the other side of the GMSL links for this MAX9286 (such as a * MAX9271). To prevent that disable auto-acknowledge early on; it * will be enabled later as needed. */ max9286_configure_i2c(dev, false); ret = device_for_each_child(client->dev.parent, &client->dev, max9286_is_bound); if (ret) return 0; dev_dbg(&client->dev, "All max9286 probed: start initialization sequence\n"); ret = device_for_each_child(client->dev.parent, NULL, max9286_init); [1] https://lore.kernel.org/all/20180605233435.18102-3-kieran.bingham+renesas@ideasonboard.com/ This was considered as a hack and dropped, limiting support to a single MAX9286 on a given I2C bus. I think we should revive that discussion, and implement a generic mechanism to handle synchronized initialization at probe time, synchronized operation of muxes across multiple deserializers (if we decide to go that way for GMSL1), and synchronized power up at runtime (again if we decide we can handle runtime power management). > - deser can now add the serializer linux i2c device, so that the > serializer can probe I'm a bit concerned about having the deserializer driver writing to a serializer register. If possible, I'd like that to be performed by the serializer driver when it probes. Power management needs to be taken into account here (if we decide to support it). > - the serializer should prevent any remote i2c transactions until it has > written the SRC_A/B and DST_A/B registers, to get translation for the > remote peripherals (or maybe the deser driver should do this part too). > > Am I on the right track with the above? As explained above, we went the I2C mux way. I think address translation would make sense to explore, but we may need to support falling back to a mux if there are too many devices behind the serializers. > Now, maybe having such a HW config, two deserializers on a single i2c > bus, doesn't happen in real life, It did, we cried about it, and the world didn't care. Maybe we didn't sacrifice the right goat to the right god, but I'm pretty sure we'll run out of goats and/or gods before we run out of "interesting" hardware designs. > but this issue comes up with > multi-port deserializers. And while those deserializers are different > devices than what's added in this series, the serializers used may be > the same as here. This means the serializer drivers and DT bindings > should be such that multi-port deserializers can be supported. I fully agree, the DT bindings need to consider more than just the particular serializers and deserializers that this series covers. > As I said, I'm late (and new) to this party, and struggling to consume > and understand all the related specs and drivers, so I hope you can give > some insight into how all this might be implemented in the future =). > > Have you looked at the FPD-Link drivers (ds90ub9xx)? The i2c management > is a bit different with those (with my current understanding, a bit > saner...), but I wonder if similar style would help here, or if the > i2c-atr could be utilized. It would be nice (but I guess not really > mandatory in any way) to have similar style in DT bindings for all > ser-des solutions. > > To summarize the i2c management on both FPD-Link and GMSL (if I have > understood it right): > > In FPD-Link the deserializer does it all: it has registers for the > serializer i2c aliases, and for i2c address translation (per port). So > when the deser probes, it can program suitable i2c addresses (based on > data from DT), which will be the addresses visible on the main i2c bus, > and thus there are never any conflicts. That's much nicer than the GMSL architecture in my opinion. > In addition to that, the drivers utilize i2c-atr, which means that new > linux i2c busses are created for each serializer. E.g. the deser might > be, say, on i2c bus 4, and also the serializers, via their i2c aliases, > would be accessible bus 4. When the serializer drivers probe they will > create new i2c busses with i2c-atr. So with a 4 port deserializer we > might get i2c busses 5, 6, 7 and 8. The linux i2c devices for remote > peripherals (sensors mainly) would be created on these busses with their > real i2c addresses. When a sensor driver does an i2c write to its > device, the i2c-atr will catch the write, change the address according > to the translation table, and do an actual write on the i2c bus 4. This > would result in the deser HW to catch this write, switch the address > back to the "real" one, and send it to the appropriate serializer, which > would then send the i2c transaction on its i2c bus. > > In GMSL the deser just forwards everything it sees on the i2c bus, if a > port is enabled. The deser has no other support related to i2c. The > serializers have DEV_ADDR register which can be used to change the > address the serializers respond to, and the serializers also have i2c > translation for two remote peripherals. In addition to that, the deserializer (at least the MAX9286) has support for auto-ack. When enabled, it will automatically ack any I2C write, when the I2C reverse channel is available for the forward channel isn't. It's a plug-and-pray approach, used to write serializer registers related to the I2C forward channel configuration. One issue with this is that any I2C write on the bus seen by the deserializer will be acked by it, even if it is for a completely unrelated device. > But if the i2c translation is used, it would mean that, say, the sensor > driver would need to use the "virtual" address, not the real one to > communicate with the sensor device, which doesn't sound right... How so ? With FPD-Link, with ATR is enabled, doesn't the sensor driver also use the "virtual" (as in host-visible) I2C address instead of the real one (as in the address used on the bus physically connected to the sensor) ? > You have used i2c-gate for both the deser and the ser. I don't have > experience with i2c-gate, but how can we manage the serializer i2c > address and the i2c address translation with it? > > One difference with the FPD-Link and this series' DT bindings is that I > have a "links" node in the deser, instead of just adding the serializers > under an i2c node. In FPD-Link case this allowed me to better represent > the hardware and the configuration needed. > > So... Perhaps my bottom line question is: do we need something similar > to what the FPD-Link uses (links, i2c-atr) to fully support GMSL > devices? And also, if we merge the DT bindings in this series, will we > have gone into a corner wrt. how we can manage the i2c? For consistency, I would like to keep the bindings as close as possible to each other when there is no reason to do otherwise. Of course, we already have GMSL1 and FPD-Link bindings that are not identical... Given the backward compatibility of GMSL2 with GMSL1, we may need to stay closer to the GMSL1 bindings than to the FPD-Link bindings. Of course, any feature not available in the GMSL1 bindings that we would need to design and implement can mimick the FPD-Link bindings.
On 06/06/2024 18:24, Laurent Pinchart wrote: >> But if the i2c translation is used, it would mean that, say, the sensor >> driver would need to use the "virtual" address, not the real one to >> communicate with the sensor device, which doesn't sound right... > > How so ? With FPD-Link, with ATR is enabled, doesn't the sensor driver > also use the "virtual" (as in host-visible) I2C address instead of the > real one (as in the address used on the bus physically connected to the > sensor) ? No. If we, say, have a sensor hardware that responds to address 0x30, we create a new "virtual" or "remote" i2c-bus (let's say i2c-10), on which there's the sensor with address 0x30. So for the driver and this i2c-bus, everything looks just like the sensor would be connected "normally" to the SoC's i2c bus. So, we have i2c-10 bus with sensor@30. This is what the userspace sees, and how the driver sees it. And if we have, say, 2 identical cameras behind two links, we have i2c-10 with sensor@30 and i2c-11 with sensor@30. When the sensor driver does an i2c transaction, the i2c-atr driver will catch that transaction before it goes to HW. It will replace the address 30 in the message with the appropriate alias (say, 50), and issue a HW transaction on the SoC's i2c bus where the deserializer resides. The deserializer sees a message to address 50, and knows that it's a message to link 0 with alias 30 (based on the programmed translation table). It takes the message, replaces 50 with 30, and sends it to the serializer on link 0. The serializer will then transmit that message on its i2c master, which will then be received by the sensor@30. On a reply, the same happens but in reverse. Tomi
Hi Tomi, On 6/6/24 3:34 PM, Tomi Valkeinen wrote: > Hi, > > On 30/04/2024 16:19, Julien Massot wrote: >> Change since v6: >> - Remove mention of C-PHY for MAX96717, this serializer is D-PHY only >> - Remove bus-type requirement for MAX96717 >> - Minor changes requested by Sakari >> - Workaround a MAX96717 issue, which occurs when stopping >> the CSI source before stopping the MAX96717 CSI receiver. >> >> Power management is not included in this patchset. The GMSL link is >> not always resuming when the deserializer is suspended without >> suspending the serializer. >> >> Change since v5: >> - Reverse fallback logic: max9671{4,7} can fallback to max9671{4,7}F >> - use const instead of enum for max9671{4,7}f compatible as suggested >> >> Change since v4: >> - Add support for MAX96717 and MAX96714 and use them as a fallback for >> MAX96717F and MAX96714F respectively >> - The drivers are now compatible with MAX96717 and MAX96714 since no >> change in >> the logic is needed >> - Reference 'i2c-gate' instead of 'i2c-controller' in the bindings >> >> Change since v3: >> - bindings >> - Renamed bindings to drop the 'f' suffix >> - Add bus type to MAX96717 and remove from MAX9674 >> - Add lane-polarities to both bindings >> >> - drivers >> - Address changes requested by Sakari in v3 >> - use v4l2_subdev_s_stream_helper for MAX96714 >> - do not init regmap twice in the MAX96714 driver >> - Fix compilations on 32 bits platforms >> >> Change since v2: >> - Convert drivers to use CCI helpers >> - Use generic node name >> - Use 'powerdown' as gpio name instead of 'enable' >> - Add pattern generator support for MAX96714 >> >> These patches add support for Maxim MAX96714F deserializer and >> MAX96717F serializer. >> >> MAX96714F has one GMSL2 input port and one CSI2 4 lanes output port, >> MAX96717F has one CSI2 input port and one GMSL2 output port. >> >> The drivers support the tunnel mode where all the >> CSI2 traffic coming from an imager is replicated through the deserializer >> output port. >> >> Both MAX96714F and MAX96717F are limited to a 3Gbps forward link rate >> leaving a maximum of 2.6Gbps for the video payload. > > (I see this mail turned out to be a collection of thoughts rather than > clear questions... Bear with me =)) > > I know I'm very late to this party, and perhaps these topics have > already been discussed, but as I will most likely be doing some GMSL > work in the future I wanted to ask these questions. My main > questions/concerns are related to the i2c and the representation of the > links in the DT. > > First, I know these particular devices are one input, one output > serializer and deserializer, so there's not much to do wrt. i2c > translation/gating. But even here I wonder how does one support a case > where a single local i2c bus would have two deserializer devices (with > different i2c addresses), connected to two identical camera modules? > > Controlling the deserializers would work fine, but as the serializers > and the remote peripherals (sensor) would answer to identical i2c > addresses, it would conflict and not work. > > If I understand the HW docs right, a way (maybe there are others?) to > handle this would be: > - deser probes, but keeps the link disabled by default > - deser reads the initial serializer i2c address from the DT, but also a > new address which we want the serializer to have (which doesn't conflict > with the other serializer) > - deser enables the link and immediately (how to be sure the other deser > driver doesn't do this at the same time?) sends a write to the > serializer's DEV_ADDR, changing the serializer's i2c address. > - deser can now add the serializer linux i2c device, so that the > serializer can probe > - the serializer should prevent any remote i2c transactions until it has > written the SRC_A/B and DST_A/B registers, to get translation for the > remote peripherals (or maybe the deser driver should do this part too). > > Am I on the right track with the above? Yes this is the recommended way, and at least the only one I know from Analog device https://www.analog.com/media/en/technical-documentation/user-guides/gmsl2-general-user-guide.pdf 6.2.3.1.4.1Camera Setup – Two Serializers to One Deserializer If we faced a scenario where we need to rewrite the serializers addresses, then we will need a way to synchronize the link startup and probing the serializers one by one to rewrite the I2C address. > > Now, maybe having such a HW config, two deserializers on a single i2c > bus, doesn't happen in real life, but this issue comes up with > multi-port deserializers. And while those deserializers are different > devices than what's added in this series, the serializers used may be > the same as here. This means the serializer drivers and DT bindings > should be such that multi-port deserializers can be supported. The serializer is supporting i2c-atr as well so the dt-binding can be improved to handle this scenario perhaps in an exclusive way. (not using i2c-gate and i2c-atr at the same time) > > As I said, I'm late (and new) to this party, and struggling to consume > and understand all the related specs and drivers, so I hope you can give > some insight into how all this might be implemented in the future =). > > Have you looked at the FPD-Link drivers (ds90ub9xx)? The i2c management > is a bit different with those (with my current understanding, a bit > saner...), but I wonder if similar style would help here, or if the > i2c-atr could be utilized. It would be nice (but I guess not really > mandatory in any way) to have similar style in DT bindings for all > ser-des solutions. > > To summarize the i2c management on both FPD-Link and GMSL (if I have > understood it right): > > In FPD-Link the deserializer does it all: it has registers for the > serializer i2c aliases, and for i2c address translation (per port). So > when the deser probes, it can program suitable i2c addresses (based on > data from DT), which will be the addresses visible on the main i2c bus, > and thus there are never any conflicts. > > In addition to that, the drivers utilize i2c-atr, which means that new > linux i2c busses are created for each serializer. E.g. the deser might > be, say, on i2c bus 4, and also the serializers, via their i2c aliases, > would be accessible bus 4. When the serializer drivers probe they will > create new i2c busses with i2c-atr. So with a 4 port deserializer we > might get i2c busses 5, 6, 7 and 8. The linux i2c devices for remote > peripherals (sensors mainly) would be created on these busses with their > real i2c addresses. When a sensor driver does an i2c write to its > device, the i2c-atr will catch the write, change the address according > to the translation table, and do an actual write on the i2c bus 4. This > would result in the deser HW to catch this write, switch the address > back to the "real" one, and send it to the appropriate serializer, which > would then send the i2c transaction on its i2c bus. > > In GMSL the deser just forwards everything it sees on the i2c bus, if a > port is enabled. The deser has no other support related to i2c. The > serializers have DEV_ADDR register which can be used to change the > address the serializers respond to, and the serializers also have i2c > translation for two remote peripherals. That's correct, the deser also have the DIS_REM_CC configuration, to not propagate the I2C requests on a particular link. As I understand from the datasheet this settings require a link reset to be applied, so we can't use it as a select/unselect method for an I2C mux. > > But if the i2c translation is used, it would mean that, say, the sensor > driver would need to use the "virtual" address, not the real one to > communicate with the sensor device, which doesn't sound right... > > You have used i2c-gate for both the deser and the ser. I don't have > experience with i2c-gate, but how can we manage the serializer i2c > address and the i2c address translation with it? If we want to add support for I2C ATR on the serializer side then we may want to declare the device on another node than 'i2c-gate', 'i2c-atr' for example. > > One difference with the FPD-Link and this series' DT bindings is that I > have a "links" node in the deser, instead of just adding the serializers > under an i2c node. In FPD-Link case this allowed me to better represent > the hardware and the configuration needed. > > So... Perhaps my bottom line question is: do we need something similar > to what the FPD-Link uses (links, i2c-atr) to fully support GMSL > devices? And also, if we merge the DT bindings in this series, will we > have gone into a corner wrt. how we can manage the i2c? Fully supporting the GMSL2 devices is an ambitious task that this patchset doesn't address. I wanted to first tackle a simple scenario, where we don't need links nodes. Dual and Quad GMSL devices will probably deserve their own bindings and drivers, and at this point we can discuss if there is a requirement to increase the complexity of the binding. We can of course add an i2c-atr node to the MAX96717 binding, without breaking the dt-binding compatibility. About the links node, as you know the GMSL2 deserializer doesn't allow to write and i2c-alias or a per link I2C control. We can add later a per link configuration e.g: - GMSL1 backward compatibility - Pixel/tunnel mode - Forward channel rate 6/3 Gbps We can choose to add those configurations either in a link node, similar to FPD Link devices or directly as a port properties, no strong opinion on that. > > Tomi >
Hi Laurent, On 6/6/24 5:24 PM, Laurent Pinchart wrote: > On Thu, Jun 06, 2024 at 04:34:19PM +0300, Tomi Valkeinen wrote: >> Hi, >> >> On 30/04/2024 16:19, Julien Massot wrote: >>> Change since v6: >>> - Remove mention of C-PHY for MAX96717, this serializer is D-PHY only >>> - Remove bus-type requirement for MAX96717 >>> - Minor changes requested by Sakari >>> - Workaround a MAX96717 issue, which occurs when stopping >>> the CSI source before stopping the MAX96717 CSI receiver. >>> >>> Power management is not included in this patchset. The GMSL link is >>> not always resuming when the deserializer is suspended without >>> suspending the serializer. >>> >>> Change since v5: >>> - Reverse fallback logic: max9671{4,7} can fallback to max9671{4,7}F >>> - use const instead of enum for max9671{4,7}f compatible as suggested >>> >>> Change since v4: >>> - Add support for MAX96717 and MAX96714 and use them as a fallback for >>> MAX96717F and MAX96714F respectively >>> - The drivers are now compatible with MAX96717 and MAX96714 since no change in >>> the logic is needed >>> - Reference 'i2c-gate' instead of 'i2c-controller' in the bindings >>> >>> Change since v3: >>> - bindings >>> - Renamed bindings to drop the 'f' suffix >>> - Add bus type to MAX96717 and remove from MAX9674 >>> - Add lane-polarities to both bindings >>> >>> - drivers >>> - Address changes requested by Sakari in v3 >>> - use v4l2_subdev_s_stream_helper for MAX96714 >>> - do not init regmap twice in the MAX96714 driver >>> - Fix compilations on 32 bits platforms >>> >>> Change since v2: >>> - Convert drivers to use CCI helpers >>> - Use generic node name >>> - Use 'powerdown' as gpio name instead of 'enable' >>> - Add pattern generator support for MAX96714 >>> >>> These patches add support for Maxim MAX96714F deserializer and >>> MAX96717F serializer. >>> >>> MAX96714F has one GMSL2 input port and one CSI2 4 lanes output port, >>> MAX96717F has one CSI2 input port and one GMSL2 output port. >>> >>> The drivers support the tunnel mode where all the >>> CSI2 traffic coming from an imager is replicated through the deserializer >>> output port. >>> >>> Both MAX96714F and MAX96717F are limited to a 3Gbps forward link rate >>> leaving a maximum of 2.6Gbps for the video payload. >> >> (I see this mail turned out to be a collection of thoughts rather than >> clear questions... Bear with me =)) >> >> I know I'm very late to this party, and perhaps these topics have >> already been discussed, but as I will most likely be doing some GMSL >> work in the future I wanted to ask these questions. My main >> questions/concerns are related to the i2c and the representation of the >> links in the DT. >> >> First, I know these particular devices are one input, one output >> serializer and deserializer, so there's not much to do wrt. i2c >> translation/gating. But even here I wonder how does one support a case >> where a single local i2c bus would have two deserializer devices (with >> different i2c addresses), connected to two identical camera modules? >> >> Controlling the deserializers would work fine, but as the serializers >> and the remote peripherals (sensor) would answer to identical i2c >> addresses, it would conflict and not work. >> >> If I understand the HW docs right, a way (maybe there are others?) to >> handle this would be: >> - deser probes, but keeps the link disabled by default > > I don't know if the GMSL2 deserializers typically start with the link > enabled or disabled by default, but I assume you mean here that early in > the probe sequence the driver would disable the link if it's enabled by > default. > > Note that the forward (from serializer to deserializer, carrying video > and I2C "replies") and reverse (from deserializer to serializer, > carrying I2C "requests") can be controlled separately in GMSL1. I don't > know if GMSL2 allows doing the same. It would be good to be precise in > the discussions. > >> - deser reads the initial serializer i2c address from the DT, but also a >> new address which we want the serializer to have (which doesn't conflict >> with the other serializer) > > There's also the devices behind the serializer that we need to consider. > There will typically be one (a camera sensor), but possibly multiple > (microcontrollers, EEPROMs, ISPs, ...) such devices. With GMSL1, the > serializer has the ability to perform address translation for up to two > addresses, plus the ability to reprogram the serializer address. If we > end up having more than two devices behind the serializers, address > translation won't be good enough. > > For GMSL1, we decided not to reprogram the serializer address, but > instead to implement an I2C mux in the deserializer driver. The > deserializer would disable all reverse links, and enable them > selectively through the I2C mux API. This ensured that only one reverse > link would be enabled at a time per deserializer. It didn't address the > issue of multiple deserializers on the same I2C bus. > > There's also the issue of power management to consider. Power to the > cameras and deserializers could be cut when they're unused. We need to > ensure they can come back up with I2C conflicts, as they would be reset > to their default address. I don't know if this is a solvable problem in > the generic case with GMSL1 and GMSL2. I don't know either for GMSL1 serializers, but for the GMSL2 one there is a sleep mode on the serializer which retains some register configuration such as the I2C address. I don't know how this mode can be used since we need a close relationship between the des and ser to wake it up. > >> - deser enables the link and immediately (how to be sure the other deser >> driver doesn't do this at the same time?) sends a write to the >> serializer's DEV_ADDR, changing the serializer's i2c address. > > We faced a similar issue when we started working on the MAX9286 driver > (a quad GMSL1 deserializer). Our test platform had two MAX9286 on the > same I2C bus (for a total of 8 cameras), and all cameras were identical. > > The initial driver implementation posted to the list ([1]) included a > mechanism to handle this problem: > > /* > * We can have multiple MAX9286 instances on the same physical I2C > * bus, and I2C children behind ports of separate MAX9286 instances > * having the same I2C address. As the MAX9286 starts by default with > * all ports enabled, we need to disable all ports on all MAX9286 > * instances before proceeding to further initialize the devices and > * instantiate children. > * > * Start by just disabling all channels on the current device. Then, > * if all other MAX9286 on the parent bus have been probed, proceed > * to initialize them all, including the current one. > */ > max9286_i2c_mux_close(dev); > > /* > * The MAX9286 initialises with auto-acknowledge enabled by default. > * This means that if multiple MAX9286 devices are connected to an I2C > * bus, another MAX9286 could ack I2C transfers meant for a device on > * the other side of the GMSL links for this MAX9286 (such as a > * MAX9271). To prevent that disable auto-acknowledge early on; it > * will be enabled later as needed. > */ > max9286_configure_i2c(dev, false); > > ret = device_for_each_child(client->dev.parent, &client->dev, > max9286_is_bound); > if (ret) > return 0; > > dev_dbg(&client->dev, > "All max9286 probed: start initialization sequence\n"); > ret = device_for_each_child(client->dev.parent, NULL, > max9286_init); > > [1] https://lore.kernel.org/all/20180605233435.18102-3-kieran.bingham+renesas@ideasonboard.com/ > > This was considered as a hack and dropped, limiting support to a single > MAX9286 on a given I2C bus. I think we should revive that discussion, > and implement a generic mechanism to handle synchronized initialization > at probe time, synchronized operation of muxes across multiple > deserializers (if we decide to go that way for GMSL1), and synchronized > power up at runtime (again if we decide we can handle runtime power > management). > >> - deser can now add the serializer linux i2c device, so that the >> serializer can probe > > I'm a bit concerned about having the deserializer driver writing to a > serializer register. If possible, I'd like that to be performed by the > serializer driver when it probes. Power management needs to be taken > into account here (if we decide to support it). > >> - the serializer should prevent any remote i2c transactions until it has >> written the SRC_A/B and DST_A/B registers, to get translation for the >> remote peripherals (or maybe the deser driver should do this part too). >> >> Am I on the right track with the above? > > As explained above, we went the I2C mux way. I think address translation > would make sense to explore, but we may need to support falling back to > a mux if there are too many devices behind the serializers. > >> Now, maybe having such a HW config, two deserializers on a single i2c >> bus, doesn't happen in real life, > > It did, we cried about it, and the world didn't care. Maybe we didn't > sacrifice the right goat to the right god, but I'm pretty sure we'll run > out of goats and/or gods before we run out of "interesting" hardware > designs. > >> but this issue comes up with >> multi-port deserializers. And while those deserializers are different >> devices than what's added in this series, the serializers used may be >> the same as here. This means the serializer drivers and DT bindings >> should be such that multi-port deserializers can be supported. > > I fully agree, the DT bindings need to consider more than just the > particular serializers and deserializers that this series covers. > >> As I said, I'm late (and new) to this party, and struggling to consume >> and understand all the related specs and drivers, so I hope you can give >> some insight into how all this might be implemented in the future =). >> >> Have you looked at the FPD-Link drivers (ds90ub9xx)? The i2c management >> is a bit different with those (with my current understanding, a bit >> saner...), but I wonder if similar style would help here, or if the >> i2c-atr could be utilized. It would be nice (but I guess not really >> mandatory in any way) to have similar style in DT bindings for all >> ser-des solutions. >> >> To summarize the i2c management on both FPD-Link and GMSL (if I have >> understood it right): >> >> In FPD-Link the deserializer does it all: it has registers for the >> serializer i2c aliases, and for i2c address translation (per port). So >> when the deser probes, it can program suitable i2c addresses (based on >> data from DT), which will be the addresses visible on the main i2c bus, >> and thus there are never any conflicts. > > That's much nicer than the GMSL architecture in my opinion. > >> In addition to that, the drivers utilize i2c-atr, which means that new >> linux i2c busses are created for each serializer. E.g. the deser might >> be, say, on i2c bus 4, and also the serializers, via their i2c aliases, >> would be accessible bus 4. When the serializer drivers probe they will >> create new i2c busses with i2c-atr. So with a 4 port deserializer we >> might get i2c busses 5, 6, 7 and 8. The linux i2c devices for remote >> peripherals (sensors mainly) would be created on these busses with their >> real i2c addresses. When a sensor driver does an i2c write to its >> device, the i2c-atr will catch the write, change the address according >> to the translation table, and do an actual write on the i2c bus 4. This >> would result in the deser HW to catch this write, switch the address >> back to the "real" one, and send it to the appropriate serializer, which >> would then send the i2c transaction on its i2c bus. >> >> In GMSL the deser just forwards everything it sees on the i2c bus, if a >> port is enabled. The deser has no other support related to i2c. The >> serializers have DEV_ADDR register which can be used to change the >> address the serializers respond to, and the serializers also have i2c >> translation for two remote peripherals. > > In addition to that, the deserializer (at least the MAX9286) has support > for auto-ack. When enabled, it will automatically ack any I2C write, > when the I2C reverse channel is available for the forward channel isn't. > It's a plug-and-pray approach, used to write serializer registers > related to the I2C forward channel configuration. One issue with this is > that any I2C write on the bus seen by the deserializer will be acked by > it, even if it is for a completely unrelated device. > >> But if the i2c translation is used, it would mean that, say, the sensor >> driver would need to use the "virtual" address, not the real one to >> communicate with the sensor device, which doesn't sound right... > > How so ? With FPD-Link, with ATR is enabled, doesn't the sensor driver > also use the "virtual" (as in host-visible) I2C address instead of the > real one (as in the address used on the bus physically connected to the > sensor) ? > >> You have used i2c-gate for both the deser and the ser. I don't have >> experience with i2c-gate, but how can we manage the serializer i2c >> address and the i2c address translation with it? >> >> One difference with the FPD-Link and this series' DT bindings is that I >> have a "links" node in the deser, instead of just adding the serializers >> under an i2c node. In FPD-Link case this allowed me to better represent >> the hardware and the configuration needed. >> >> So... Perhaps my bottom line question is: do we need something similar >> to what the FPD-Link uses (links, i2c-atr) to fully support GMSL >> devices? And also, if we merge the DT bindings in this series, will we >> have gone into a corner wrt. how we can manage the i2c? > > For consistency, I would like to keep the bindings as close as possible > to each other when there is no reason to do otherwise. Of course, we > already have GMSL1 and FPD-Link bindings that are not identical... Given > the backward compatibility of GMSL2 with GMSL1, we may need to stay > closer to the GMSL1 bindings than to the FPD-Link bindings. Of course, > any feature not available in the GMSL1 bindings that we would need to > design and implement can mimick the FPD-Link bindings. The difference I'm seeing with the max9286 binding is that the deserializer declare the serializer in the i2c-gate node instead of a i2c-mux. But this device has only one port, and we don't have a way to only transmit the i2c requests on a particular link.
Change since v6: - Remove mention of C-PHY for MAX96717, this serializer is D-PHY only - Remove bus-type requirement for MAX96717 - Minor changes requested by Sakari - Workaround a MAX96717 issue, which occurs when stopping the CSI source before stopping the MAX96717 CSI receiver. Power management is not included in this patchset. The GMSL link is not always resuming when the deserializer is suspended without suspending the serializer. Change since v5: - Reverse fallback logic: max9671{4,7} can fallback to max9671{4,7}F - use const instead of enum for max9671{4,7}f compatible as suggested Change since v4: - Add support for MAX96717 and MAX96714 and use them as a fallback for MAX96717F and MAX96714F respectively - The drivers are now compatible with MAX96717 and MAX96714 since no change in the logic is needed - Reference 'i2c-gate' instead of 'i2c-controller' in the bindings Change since v3: - bindings - Renamed bindings to drop the 'f' suffix - Add bus type to MAX96717 and remove from MAX9674 - Add lane-polarities to both bindings - drivers - Address changes requested by Sakari in v3 - use v4l2_subdev_s_stream_helper for MAX96714 - do not init regmap twice in the MAX96714 driver - Fix compilations on 32 bits platforms Change since v2: - Convert drivers to use CCI helpers - Use generic node name - Use 'powerdown' as gpio name instead of 'enable' - Add pattern generator support for MAX96714 These patches add support for Maxim MAX96714F deserializer and MAX96717F serializer. MAX96714F has one GMSL2 input port and one CSI2 4 lanes output port, MAX96717F has one CSI2 input port and one GMSL2 output port. The drivers support the tunnel mode where all the CSI2 traffic coming from an imager is replicated through the deserializer output port. Both MAX96714F and MAX96717F are limited to a 3Gbps forward link rate leaving a maximum of 2.6Gbps for the video payload. Julien Massot (9): dt-bindings: media: add Maxim MAX96717 GMSL2 Serializer dt-bindings: media: add Maxim MAX96714 GMSL2 Deserializer media: i2c: add MAX96717 driver media: i2c: add MAX96714 driver drivers: media: max96717: stop the csi receiver before the source .../bindings/media/i2c/maxim,max96714.yaml | 174 +++ .../bindings/media/i2c/maxim,max96717.yaml | 157 +++ MAINTAINERS | 14 + drivers/media/i2c/Kconfig | 34 + drivers/media/i2c/Makefile | 2 + drivers/media/i2c/max96714.c | 1024 +++++++++++++++++ drivers/media/i2c/max96717.c | 927 +++++++++++++++ 7 files changed, 2332 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/i2c/maxim,max96714.yaml create mode 100644 Documentation/devicetree/bindings/media/i2c/maxim,max96717.yaml create mode 100644 drivers/media/i2c/max96714.c create mode 100644 drivers/media/i2c/max96717.c