* [PATCH v7 1/5] dt-bindings: media: add Maxim MAX96717 GMSL2 Serializer
2024-04-30 13:19 [PATCH v7 0/5] Add support for MAX96714/F and MAX96717/F GMSL2 ser/des Julien Massot
@ 2024-04-30 13:19 ` Julien Massot
2024-04-30 13:19 ` [PATCH v7 2/5] dt-bindings: media: add Maxim MAX96714 GMSL2 Deserializer Julien Massot
` (5 subsequent siblings)
6 siblings, 0 replies; 14+ messages in thread
From: Julien Massot @ 2024-04-30 13:19 UTC (permalink / raw)
To: linux-media, sakari.ailus
Cc: devicetree, kernel, linux-kernel, mchehab, robh+dt,
krzysztof.kozlowski+dt, conor+dt, Julien Massot, Conor Dooley
Add DT bindings for Maxim MAX96717 GMSL2 Serializer.
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Julien Massot <julien.massot@collabora.com>
---
Change since v6:
- Remove mention to C-PHY therefore remove the bus-type property
since the device only D-PHY.
Change since v5:
- Reverse the fallback MAX96717 can fallback to MAX96717F
- Use const instead of enum for MAX96717F compatible
Change since v4:
- Add compatible for MAX96717 and use it as a fallback for MAX96717F
- Remove extra '|' for decriptions
- Reference 'i2c-gate' instead of 'i2c-controller'
Change since v3:
- Renamed file to maxim,max96717.yaml dropped the 'f' suffix
- Added lane-polarities and bus type properties to the CSI endpoint
Change since v2:
- remove reg description
- add data lanes min/maxItems
- Use generic node name
---
.../bindings/media/i2c/maxim,max96717.yaml | 157 ++++++++++++++++++
1 file changed, 157 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/i2c/maxim,max96717.yaml
diff --git a/Documentation/devicetree/bindings/media/i2c/maxim,max96717.yaml b/Documentation/devicetree/bindings/media/i2c/maxim,max96717.yaml
new file mode 100644
index 000000000000..d1e8ba6e368e
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/maxim,max96717.yaml
@@ -0,0 +1,157 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2024 Collabora Ltd.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/i2c/maxim,max96717.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MAX96717 CSI-2 to GMSL2 Serializer
+
+maintainers:
+ - Julien Massot <julien.massot@collabora.com>
+
+description:
+ The MAX96717 serializer converts MIPI CSI-2 D-PHY formatted input
+ into GMSL2 serial outputs. The device allows the GMSL2 link to
+ simultaneously transmit bidirectional control-channel data while forward
+ video transmissions are in progress. The MAX96717 can connect to one
+ remotely located deserializer using industry-standard coax or STP
+ interconnects. The device cans operate in pixel or tunnel mode. In pixel mode
+ the MAX96717 can select the MIPI datatype, while the tunnel mode forward all the MIPI
+ data received by the serializer.
+ The MAX96717 supports Reference Over Reverse (channel),
+ to generate a clock output for the sensor from the GMSL reverse channel.
+
+ The GMSL2 serial link operates at a fixed rate of 3Gbps or 6Gbps in the
+ forward direction and 187.5Mbps in the reverse direction.
+ MAX96717F only supports a fixed rate of 3Gbps in the forward direction.
+
+properties:
+ compatible:
+ oneOf:
+ - const: maxim,max96717f
+ - items:
+ - enum:
+ - maxim,max96717
+ - const: maxim,max96717f
+
+ '#gpio-cells':
+ const: 2
+ description:
+ First cell is the GPIO pin number, second cell is the flags. The GPIO pin
+ number must be in range of [0, 10].
+
+ gpio-controller: true
+
+ '#clock-cells':
+ const: 0
+
+ reg:
+ maxItems: 1
+
+ ports:
+ $ref: /schemas/graph.yaml#/properties/ports
+
+ properties:
+ port@0:
+ $ref: /schemas/graph.yaml#/$defs/port-base
+ unevaluatedProperties: false
+ description: CSI-2 Input port
+
+ properties:
+ endpoint:
+ $ref: /schemas/media/video-interfaces.yaml#
+ unevaluatedProperties: false
+
+ properties:
+ data-lanes:
+ minItems: 1
+ maxItems: 4
+
+ lane-polarities:
+ minItems: 1
+ maxItems: 5
+
+ required:
+ - data-lanes
+
+ port@1:
+ $ref: /schemas/graph.yaml#/properties/port
+ unevaluatedProperties: false
+ description: GMSL Output port
+
+ required:
+ - port@1
+
+ i2c-gate:
+ $ref: /schemas/i2c/i2c-gate.yaml
+ unevaluatedProperties: false
+ description:
+ The MAX96717 will forward the I2C requests from the
+ incoming GMSL2 link. Therefore, it supports an i2c-gate
+ subnode to configure a sensor.
+
+required:
+ - compatible
+ - reg
+ - ports
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ #include <dt-bindings/media/video-interfaces.h>
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ serializer: serializer@40 {
+ compatible = "maxim,max96717f";
+ reg = <0x40>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ #clock-cells = <0>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ max96717f_csi_in: endpoint {
+ data-lanes = <1 2 3 4>;
+ remote-endpoint = <&sensor_out>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ max96917f_gmsl_out: endpoint {
+ remote-endpoint = <&deser_gmsl_in>;
+ };
+ };
+ };
+
+ i2c-gate {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ sensor@10 {
+ compatible = "st,st-vgxy61";
+ reg = <0x10>;
+ reset-gpios = <&serializer 0 GPIO_ACTIVE_LOW>;
+ clocks = <&serializer>;
+ VCORE-supply = <&v1v2>;
+ VDDIO-supply = <&v1v8>;
+ VANA-supply = <&v2v8>;
+ port {
+ sensor_out: endpoint {
+ data-lanes = <1 2 3 4>;
+ remote-endpoint = <&max96717f_csi_in>;
+ };
+ };
+ };
+ };
+ };
+ };
+...
--
2.44.0
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH v7 2/5] dt-bindings: media: add Maxim MAX96714 GMSL2 Deserializer
2024-04-30 13:19 [PATCH v7 0/5] Add support for MAX96714/F and MAX96717/F GMSL2 ser/des Julien Massot
2024-04-30 13:19 ` [PATCH v7 1/5] dt-bindings: media: add Maxim MAX96717 GMSL2 Serializer Julien Massot
@ 2024-04-30 13:19 ` Julien Massot
2024-04-30 13:19 ` [PATCH v7 3/5] media: i2c: add MAX96717 driver Julien Massot
` (4 subsequent siblings)
6 siblings, 0 replies; 14+ messages in thread
From: Julien Massot @ 2024-04-30 13:19 UTC (permalink / raw)
To: linux-media, sakari.ailus
Cc: devicetree, kernel, linux-kernel, mchehab, robh+dt,
krzysztof.kozlowski+dt, conor+dt, Julien Massot, Conor Dooley
Add DT bindings for Maxim MAX96714 GMSL2 Deserializer.
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Julien Massot <julien.massot@collabora.com>
---
Change since v6:
- Remove 'bus-type' serializer property in the example
Change since v5:
- Reverse the fallback MAX96714 can fallback to MAX96714F
- Use const instead of enum for MAX96714F compatible
Change since v4:
- Add compatible for MAX96714 and use it as a fallback for MAX96714F
- Remove extra '|' for decriptions
- Reference 'i2c-gate' instead of 'i2c-controller'
Change since v3:
- Renamed file to maxim,max96714.yaml dropped the 'f' suffix
- Removed mention to C-PHY since it's not supported by MAX96714 deserializers
- Removed bus-type requirement on CSI endpoint since the device only support D-PHY
- Removed the clock-lanes property in the dt example
Change since v2:
- remove reg description
- rename enable gpio to powerdown
- use generic node name: i2c, serializer, deserializer
---
.../bindings/media/i2c/maxim,max96714.yaml | 174 ++++++++++++++++++
1 file changed, 174 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/i2c/maxim,max96714.yaml
diff --git a/Documentation/devicetree/bindings/media/i2c/maxim,max96714.yaml b/Documentation/devicetree/bindings/media/i2c/maxim,max96714.yaml
new file mode 100644
index 000000000000..3ace50e11921
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/maxim,max96714.yaml
@@ -0,0 +1,174 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2024 Collabora Ltd.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/i2c/maxim,max96714.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Maxim MAX96714 GMSL2 to CSI-2 Deserializer
+
+maintainers:
+ - Julien Massot <julien.massot@collabora.com>
+
+description:
+ The MAX96714 deserializer converts GMSL2 serial inputs into MIPI
+ CSI-2 D-PHY formatted output. The device allows the GMSL2 link to
+ simultaneously transmit bidirectional control-channel data while forward
+ video transmissions are in progress. The MAX96714 can connect to one
+ remotely located serializer using industry-standard coax or STP
+ interconnects. The device cans operate in pixel or tunnel mode. In pixel mode
+ the MAX96714 can select individual video stream, while the tunnel mode forward all
+ the MIPI data received by the serializer.
+
+ The GMSL2 serial link operates at a fixed rate of 3Gbps or 6Gbps in the
+ forward direction and 187.5Mbps in the reverse direction.
+ MAX96714F only supports a fixed rate of 3Gbps in the forward direction.
+
+properties:
+ compatible:
+ oneOf:
+ - const: maxim,max96714f
+ - items:
+ - enum:
+ - maxim,max96714
+ - const: maxim,max96714f
+
+ reg:
+ maxItems: 1
+
+ powerdown-gpios:
+ maxItems: 1
+ description:
+ Specifier for the GPIO connected to the PWDNB pin.
+
+ ports:
+ $ref: /schemas/graph.yaml#/properties/ports
+
+ properties:
+ port@0:
+ $ref: /schemas/graph.yaml#/properties/port
+ unevaluatedProperties: false
+ description: GMSL Input
+ properties:
+ endpoint:
+ $ref: /schemas/media/video-interfaces.yaml#
+ unevaluatedProperties: false
+ description:
+ Endpoint for GMSL2-Link port.
+
+ port@1:
+ $ref: /schemas/graph.yaml#/$defs/port-base
+ unevaluatedProperties: false
+ description: CSI-2 Output port
+
+ properties:
+ endpoint:
+ $ref: /schemas/media/video-interfaces.yaml#
+ unevaluatedProperties: false
+
+ properties:
+ data-lanes:
+ minItems: 1
+ maxItems: 4
+
+ lane-polarities:
+ minItems: 1
+ maxItems: 5
+
+ link-frequencies:
+ maxItems: 1
+
+ required:
+ - data-lanes
+
+ required:
+ - port@1
+
+ i2c-gate:
+ $ref: /schemas/i2c/i2c-gate.yaml
+ unevaluatedProperties: false
+ description:
+ The MAX96714 will pass through and forward the I2C requests from the
+ incoming I2C bus over the GMSL2 link. Therefore it supports an i2c-gate
+ subnode to configure a serializer.
+
+ port0-poc-supply:
+ description: Regulator providing Power over Coax for the GMSL port
+
+required:
+ - compatible
+ - reg
+ - ports
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ #include <dt-bindings/media/video-interfaces.h>
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ deserializer@28 {
+ compatible = "maxim,max96714f";
+ reg = <0x28>;
+ powerdown-gpios = <&main_gpio0 37 GPIO_ACTIVE_LOW>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ port@0 {
+ reg = <0>;
+ max96714_gmsl_in: endpoint {
+ remote-endpoint = <&max96917f_gmsl_out>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ max96714_csi_out: endpoint {
+ data-lanes = <1 2 3 4>;
+ link-frequencies = /bits/ 64 <400000000>;
+ remote-endpoint = <&csi_in>;
+ };
+ };
+ };
+
+ i2c-gate {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ serializer@40 {
+ compatible = "maxim,max96717f";
+ reg = <0x40>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ #clock-cells = <0>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ max96717f_csi_in: endpoint {
+ data-lanes = <1 2>;
+ lane-polarities = <1 0 1>;
+ remote-endpoint = <&sensor_out>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ max96917f_gmsl_out: endpoint {
+ remote-endpoint = <&max96714_gmsl_in>;
+ };
+ };
+ };
+ };
+ };
+ };
+ };
+...
--
2.44.0
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH v7 3/5] media: i2c: add MAX96717 driver
2024-04-30 13:19 [PATCH v7 0/5] Add support for MAX96714/F and MAX96717/F GMSL2 ser/des Julien Massot
2024-04-30 13:19 ` [PATCH v7 1/5] dt-bindings: media: add Maxim MAX96717 GMSL2 Serializer Julien Massot
2024-04-30 13:19 ` [PATCH v7 2/5] dt-bindings: media: add Maxim MAX96714 GMSL2 Deserializer Julien Massot
@ 2024-04-30 13:19 ` Julien Massot
2024-05-31 13:56 ` Sakari Ailus
2024-06-03 7:29 ` Tommaso Merciai
2024-04-30 13:19 ` [PATCH v7 4/5] media: i2c: add MAX96714 driver Julien Massot
` (3 subsequent siblings)
6 siblings, 2 replies; 14+ messages in thread
From: Julien Massot @ 2024-04-30 13:19 UTC (permalink / raw)
To: linux-media, sakari.ailus
Cc: devicetree, kernel, linux-kernel, mchehab, robh+dt,
krzysztof.kozlowski+dt, conor+dt, Julien Massot
This driver handles the MAX96717 serializer in tunnel mode.
All incoming CSI traffic will be tunneled through the GMSL2
link.
The MAX96717 driver can handle MAX96717 and MAX96717F variants
with the same "maxim,max96717f" compatible.
Signed-off-by: Julien Massot <julien.massot@collabora.com>
---
Change since v6:
- Kconfig select MEDIA_CONTROLLER, V4L2_FWNODE and VIDEO_V4L2_SUBDEV_API
- rename 'REG3' register to 'MAX96717_REG3'
- Initialized 'ret' variable in 'max96717_gpiochip_probe'
- remove max96714_v4l2_notifier_unregister and call the function directly
- Do not store private pointer with i2c_set_clientdata since the v4l2-i2c
uses it to store the subdev pointer
- use dev_err_probe at gpio chip initialization
Change since v5:
- set the driver compatible back to MAX96717F that can be used as a fallback for MAX96717
Change since v4:
- make the driver compatible with MAX96717 instead of MAX96717F
- Add the device id for the MAX96717
- remove hw_data structure for now, it can be usefull later for handling different serializers e.g max9295
Change since v3:
- Maintainers: align to the new binding path
- Kconfig: better describe the symbol
- store the v4l2_mbus_config_mipi_csi2 structure instead of the full endpoint in the driver private structure
- use MAX96717_PAD_SINK/SOURCE instead of 0/1 for pad intialization
- Removed incorrect call to fwnode_handle_put(priv->sd.fwnode)
- Use unsigned int instead of u8
- Allocate clk name out of the clk struct initialization
- fixed multiline comment
- Removed one unnecessary goto at the end of the probe function
Change since v2:
- Use CCI helpers instead of recoding register access
- add missing bitfield header
---
MAINTAINERS | 7 +
drivers/media/i2c/Kconfig | 17 +
drivers/media/i2c/Makefile | 1 +
drivers/media/i2c/max96717.c | 928 +++++++++++++++++++++++++++++++++++
4 files changed, 953 insertions(+)
create mode 100644 drivers/media/i2c/max96717.c
diff --git a/MAINTAINERS b/MAINTAINERS
index eea74166a2d9..cfaa904ace59 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13426,6 +13426,13 @@ S: Maintained
F: Documentation/devicetree/bindings/media/i2c/maxim,max96712.yaml
F: drivers/staging/media/max96712/max96712.c
+MAX96717 GMSL2 SERIALIZER DRIVER
+M: Julien Massot <julien.massot@collabora.com>
+L: linux-media@vger.kernel.org
+S: Maintained
+F: Documentation/devicetree/bindings/media/i2c/maxim,max96717.yaml
+F: drivers/media/i2c/max96717.c
+
MAX9860 MONO AUDIO VOICE CODEC DRIVER
M: Peter Rosin <peda@axentia.se>
L: alsa-devel@alsa-project.org (moderated for non-subscribers)
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index c6d3ee472d81..9918195e09ba 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -1575,6 +1575,23 @@ config VIDEO_DS90UB960
Device driver for the Texas Instruments DS90UB960
FPD-Link III Deserializer and DS90UB9702 FPD-Link IV Deserializer.
+config VIDEO_MAX96717
+ tristate "Maxim MAX96717 GMSL2 Serializer support"
+ depends on OF && I2C && VIDEO_DEV && COMMON_CLK
+ select I2C_MUX
+ select MEDIA_CONTROLLER
+ select GPIOLIB
+ select V4L2_CCI_I2C
+ select V4L2_FWNODE
+ select VIDEO_V4L2_SUBDEV_API
+ help
+ Device driver for the Maxim MAX96717 GMSL2 Serializer.
+ MAX96717 serializers convert video on a MIPI CSI-2
+ input to a GMSL2 output.
+
+ To compile this driver as a module, choose M here: the
+ module will be called max96717.
+
endmenu
endif # VIDEO_DEV
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index dfbe6448b549..9e007116f929 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -64,6 +64,7 @@ obj-$(CONFIG_VIDEO_LM3646) += lm3646.o
obj-$(CONFIG_VIDEO_M52790) += m52790.o
obj-$(CONFIG_VIDEO_MAX9271_LIB) += max9271.o
obj-$(CONFIG_VIDEO_MAX9286) += max9286.o
+obj-$(CONFIG_VIDEO_MAX96717) += max96717.o
obj-$(CONFIG_VIDEO_ML86V7667) += ml86v7667.o
obj-$(CONFIG_VIDEO_MSP3400) += msp3400.o
obj-$(CONFIG_VIDEO_MT9M001) += mt9m001.o
diff --git a/drivers/media/i2c/max96717.c b/drivers/media/i2c/max96717.c
new file mode 100644
index 000000000000..1ea76f922bdb
--- /dev/null
+++ b/drivers/media/i2c/max96717.c
@@ -0,0 +1,928 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Maxim GMSL2 Serializer Driver
+ *
+ * Copyright (C) 2024 Collabora Ltd.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/delay.h>
+#include <linux/fwnode.h>
+#include <linux/gpio/driver.h>
+#include <linux/i2c-mux.h>
+#include <linux/i2c.h>
+#include <linux/regmap.h>
+
+#include <media/v4l2-cci.h>
+#include <media/v4l2-fwnode.h>
+#include <media/v4l2-subdev.h>
+
+#define MAX96717_DEVICE_ID 0xbf
+#define MAX96717F_DEVICE_ID 0xc8
+#define MAX96717_PORTS 2
+#define MAX96717_PAD_SINK 0
+#define MAX96717_PAD_SOURCE 1
+
+#define MAX96717_DEFAULT_CLKOUT_RATE 24000000UL
+
+/* DEV */
+#define MAX96717_REG3 CCI_REG8(0x3)
+#define MAX96717_RCLKSEL GENMASK(1, 0)
+#define RCLKSEL_REF_PLL CCI_REG8(0x3)
+#define MAX96717_REG6 CCI_REG8(0x6)
+#define RCLKEN BIT(5)
+#define MAX96717_DEV_ID CCI_REG8(0xd)
+#define MAX96717_DEV_REV CCI_REG8(0xe)
+#define MAX96717_DEV_REV_MASK GENMASK(3, 0)
+
+/* VID_TX Z */
+#define MAX96717_VIDEO_TX2 CCI_REG8(0x112)
+#define MAX96717_VIDEO_PCLKDET BIT(7)
+
+/* GPIO */
+#define MAX96717_NUM_GPIO 11
+#define MAX96717_GPIO_REG_A(gpio) CCI_REG8(0x2be + (gpio) * 3)
+#define MAX96717_GPIO_OUT BIT(4)
+#define MAX96717_GPIO_IN BIT(3)
+#define MAX96717_GPIO_RX_EN BIT(2)
+#define MAX96717_GPIO_TX_EN BIT(1)
+#define MAX96717_GPIO_OUT_DIS BIT(0)
+
+/* FRONTTOP */
+/* MAX96717 only have CSI port 'B' */
+#define MAX96717_FRONTOP0 CCI_REG8(0x308)
+#define MAX96717_START_PORT_B BIT(5)
+
+/* MIPI_RX */
+#define MAX96717_MIPI_RX1 CCI_REG8(0x331)
+#define MAX96717_MIPI_LANES_CNT GENMASK(5, 4)
+#define MAX96717_MIPI_RX2 CCI_REG8(0x332) /* phy1 Lanes map */
+#define MAX96717_PHY2_LANES_MAP GENMASK(7, 4)
+#define MAX96717_MIPI_RX3 CCI_REG8(0x333) /* phy2 Lanes map */
+#define MAX96717_PHY1_LANES_MAP GENMASK(3, 0)
+#define MAX96717_MIPI_RX4 CCI_REG8(0x334) /* phy1 lane polarities */
+#define MAX96717_PHY1_LANES_POL GENMASK(6, 4)
+#define MAX96717_MIPI_RX5 CCI_REG8(0x335) /* phy2 lane polarities */
+#define MAX96717_PHY2_LANES_POL GENMASK(2, 0)
+
+/* MIPI_RX_EXT */
+#define MAX96717_MIPI_RX_EXT11 CCI_REG8(0x383)
+#define MAX96717_TUN_MODE BIT(7)
+
+/* REF_VTG */
+#define REF_VTG0 CCI_REG8(0x3f0)
+#define REFGEN_PREDEF_EN BIT(6)
+#define REFGEN_PREDEF_FREQ_MASK GENMASK(5, 4)
+#define REFGEN_PREDEF_FREQ_ALT BIT(3)
+#define REFGEN_RST BIT(1)
+#define REFGEN_EN BIT(0)
+
+/* MISC */
+#define PIO_SLEW_1 CCI_REG8(0x570)
+
+struct max96717_priv {
+ struct i2c_client *client;
+ struct regmap *regmap;
+ struct i2c_mux_core *mux;
+ struct v4l2_mbus_config_mipi_csi2 mipi_csi2;
+ struct v4l2_subdev sd;
+ struct media_pad pads[MAX96717_PORTS];
+ struct v4l2_async_notifier notifier;
+ struct v4l2_subdev *source_sd;
+ u16 source_sd_pad;
+ u64 enabled_source_streams;
+ u8 pll_predef_index;
+ struct clk_hw clk_hw;
+ struct gpio_chip gpio_chip;
+};
+
+static inline struct max96717_priv *sd_to_max96717(struct v4l2_subdev *sd)
+{
+ return container_of(sd, struct max96717_priv, sd);
+}
+
+static inline struct max96717_priv *clk_hw_to_max96717(struct clk_hw *hw)
+{
+ return container_of(hw, struct max96717_priv, clk_hw);
+}
+
+static int max96717_i2c_mux_select(struct i2c_mux_core *mux, u32 chan)
+{
+ return 0;
+}
+
+static int max96717_i2c_mux_init(struct max96717_priv *priv)
+{
+ priv->mux = i2c_mux_alloc(priv->client->adapter, &priv->client->dev,
+ 1, 0, I2C_MUX_LOCKED | I2C_MUX_GATE,
+ max96717_i2c_mux_select, NULL);
+ if (!priv->mux)
+ return -ENOMEM;
+
+ return i2c_mux_add_adapter(priv->mux, 0, 0, 0);
+}
+
+static inline int max96717_start_csi(struct max96717_priv *priv, bool start)
+{
+ return cci_update_bits(priv->regmap, MAX96717_FRONTOP0,
+ MAX96717_START_PORT_B,
+ start ? MAX96717_START_PORT_B : 0, NULL);
+}
+
+static int max96717_gpiochip_get(struct gpio_chip *gpiochip,
+ unsigned int offset)
+{
+ struct max96717_priv *priv = gpiochip_get_data(gpiochip);
+ u64 val;
+ int ret;
+
+ ret = cci_read(priv->regmap, MAX96717_GPIO_REG_A(offset),
+ &val, NULL);
+ if (ret)
+ return ret;
+
+ if (val & MAX96717_GPIO_OUT_DIS)
+ return !!(val & MAX96717_GPIO_IN);
+ else
+ return !!(val & MAX96717_GPIO_OUT);
+}
+
+static void max96717_gpiochip_set(struct gpio_chip *gpiochip,
+ unsigned int offset, int value)
+{
+ struct max96717_priv *priv = gpiochip_get_data(gpiochip);
+
+ cci_update_bits(priv->regmap, MAX96717_GPIO_REG_A(offset),
+ MAX96717_GPIO_OUT, MAX96717_GPIO_OUT, NULL);
+}
+
+static int max96717_gpio_get_direction(struct gpio_chip *gpiochip,
+ unsigned int offset)
+{
+ struct max96717_priv *priv = gpiochip_get_data(gpiochip);
+ u64 val;
+ int ret;
+
+ ret = cci_read(priv->regmap, MAX96717_GPIO_REG_A(offset), &val, NULL);
+ if (ret < 0)
+ return ret;
+
+ return !!(val & MAX96717_GPIO_OUT_DIS);
+}
+
+static int max96717_gpio_direction_out(struct gpio_chip *gpiochip,
+ unsigned int offset, int value)
+{
+ struct max96717_priv *priv = gpiochip_get_data(gpiochip);
+
+ return cci_update_bits(priv->regmap, MAX96717_GPIO_REG_A(offset),
+ MAX96717_GPIO_OUT_DIS | MAX96717_GPIO_OUT,
+ value ? MAX96717_GPIO_OUT : 0, NULL);
+}
+
+static int max96717_gpio_direction_in(struct gpio_chip *gpiochip,
+ unsigned int offset)
+{
+ struct max96717_priv *priv = gpiochip_get_data(gpiochip);
+
+ return cci_update_bits(priv->regmap, MAX96717_GPIO_REG_A(offset),
+ MAX96717_GPIO_OUT_DIS, MAX96717_GPIO_OUT_DIS,
+ NULL);
+}
+
+static int max96717_gpiochip_probe(struct max96717_priv *priv)
+{
+ struct device *dev = &priv->client->dev;
+ struct gpio_chip *gc = &priv->gpio_chip;
+ int i, ret = 0;
+
+ gc->label = dev_name(dev);
+ gc->parent = dev;
+ gc->owner = THIS_MODULE;
+ gc->ngpio = MAX96717_NUM_GPIO;
+ gc->base = -1;
+ gc->can_sleep = true;
+ gc->get_direction = max96717_gpio_get_direction;
+ gc->direction_input = max96717_gpio_direction_in;
+ gc->direction_output = max96717_gpio_direction_out;
+ gc->set = max96717_gpiochip_set;
+ gc->get = max96717_gpiochip_get;
+ gc->of_gpio_n_cells = 2;
+
+ /* Disable GPIO forwarding */
+ for (i = 0; i < gc->ngpio; i++)
+ cci_update_bits(priv->regmap, MAX96717_GPIO_REG_A(i),
+ MAX96717_GPIO_RX_EN | MAX96717_GPIO_TX_EN,
+ 0, &ret);
+
+ if (ret)
+ return ret;
+
+ ret = devm_gpiochip_add_data(dev, gc, priv);
+ if (ret) {
+ dev_err(dev, "Unable to create gpio_chip\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+static int _max96717_set_routing(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *state,
+ struct v4l2_subdev_krouting *routing)
+{
+ static const struct v4l2_mbus_framefmt format = {
+ .width = 1280,
+ .height = 1080,
+ .code = MEDIA_BUS_FMT_Y8_1X8,
+ .field = V4L2_FIELD_NONE,
+ };
+ int ret;
+
+ ret = v4l2_subdev_routing_validate(sd, routing,
+ V4L2_SUBDEV_ROUTING_ONLY_1_TO_1);
+ if (ret)
+ return ret;
+
+ ret = v4l2_subdev_set_routing_with_fmt(sd, state, routing, &format);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static int max96717_set_routing(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *state,
+ enum v4l2_subdev_format_whence which,
+ struct v4l2_subdev_krouting *routing)
+{
+ struct max96717_priv *priv = sd_to_max96717(sd);
+
+ if (which == V4L2_SUBDEV_FORMAT_ACTIVE && priv->enabled_source_streams)
+ return -EBUSY;
+
+ return _max96717_set_routing(sd, state, routing);
+}
+
+static int max96717_set_fmt(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *state,
+ struct v4l2_subdev_format *format)
+{
+ struct max96717_priv *priv = sd_to_max96717(sd);
+ struct v4l2_mbus_framefmt *fmt;
+ u64 stream_source_mask;
+
+ if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE &&
+ priv->enabled_source_streams)
+ return -EBUSY;
+
+ /* No transcoding, source and sink formats must match. */
+ if (format->pad == MAX96717_PAD_SOURCE)
+ return v4l2_subdev_get_fmt(sd, state, format);
+
+ /* Set sink format */
+ fmt = v4l2_subdev_state_get_format(state, format->pad, format->stream);
+ if (!fmt)
+ return -EINVAL;
+
+ *fmt = format->format;
+
+ /* Propagate to source format */
+ fmt = v4l2_subdev_state_get_opposite_stream_format(state, format->pad,
+ format->stream);
+ if (!fmt)
+ return -EINVAL;
+ *fmt = format->format;
+
+ stream_source_mask = BIT(format->stream);
+
+ return v4l2_subdev_state_xlate_streams(state, MAX96717_PAD_SOURCE,
+ MAX96717_PAD_SINK,
+ &stream_source_mask);
+}
+
+static int max96717_init_state(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *state)
+{
+ struct v4l2_subdev_route routes[] = {
+ {
+ .sink_pad = MAX96717_PAD_SINK,
+ .sink_stream = 0,
+ .source_pad = MAX96717_PAD_SOURCE,
+ .source_stream = 0,
+ .flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE,
+ },
+ };
+ struct v4l2_subdev_krouting routing = {
+ .num_routes = ARRAY_SIZE(routes),
+ .routes = routes,
+ };
+
+ return _max96717_set_routing(sd, state, &routing);
+}
+
+static bool max96717_pipe_pclkdet(struct max96717_priv *priv)
+{
+ u64 val = 0;
+
+ cci_read(priv->regmap, MAX96717_VIDEO_TX2, &val, NULL);
+
+ return val & MAX96717_VIDEO_PCLKDET;
+}
+
+static int max96717_log_status(struct v4l2_subdev *sd)
+{
+ struct max96717_priv *priv = sd_to_max96717(sd);
+ struct device *dev = &priv->client->dev;
+
+ dev_info(dev, "Serializer: max96717\n");
+ dev_info(dev, "Pipe: pclkdet:%d\n", max96717_pipe_pclkdet(priv));
+
+ return 0;
+}
+
+static int max96717_enable_streams(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *state, u32 pad,
+ u64 streams_mask)
+{
+ struct max96717_priv *priv = sd_to_max96717(sd);
+ struct device *dev = &priv->client->dev;
+ u64 sink_streams;
+ int ret;
+
+ sink_streams = v4l2_subdev_state_xlate_streams(state,
+ MAX96717_PAD_SOURCE,
+ MAX96717_PAD_SINK,
+ &streams_mask);
+
+ if (!priv->enabled_source_streams)
+ max96717_start_csi(priv, true);
+
+ ret = v4l2_subdev_enable_streams(priv->source_sd, priv->source_sd_pad,
+ sink_streams);
+ if (ret) {
+ dev_err(dev, "Fail to start streams:%llu on remote subdev\n",
+ sink_streams);
+ goto stop_csi;
+ }
+
+ priv->enabled_source_streams |= streams_mask;
+
+ return 0;
+
+stop_csi:
+ if (!priv->enabled_source_streams)
+ max96717_start_csi(priv, false);
+ return ret;
+}
+
+static int max96717_disable_streams(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *state, u32 pad,
+ u64 streams_mask)
+{
+ struct max96717_priv *priv = sd_to_max96717(sd);
+ u64 sink_streams;
+ int ret;
+
+ sink_streams = v4l2_subdev_state_xlate_streams(state,
+ MAX96717_PAD_SOURCE,
+ MAX96717_PAD_SINK,
+ &streams_mask);
+
+ ret = v4l2_subdev_disable_streams(priv->source_sd, priv->source_sd_pad,
+ sink_streams);
+ if (ret)
+ return ret;
+
+ priv->enabled_source_streams &= ~streams_mask;
+
+ if (!priv->enabled_source_streams)
+ max96717_start_csi(priv, false);
+
+ return 0;
+}
+
+static const struct v4l2_subdev_pad_ops max96717_pad_ops = {
+ .enable_streams = max96717_enable_streams,
+ .disable_streams = max96717_disable_streams,
+ .set_routing = max96717_set_routing,
+ .get_fmt = v4l2_subdev_get_fmt,
+ .set_fmt = max96717_set_fmt,
+};
+
+static const struct v4l2_subdev_core_ops max96717_subdev_core_ops = {
+ .log_status = max96717_log_status,
+};
+
+static const struct v4l2_subdev_internal_ops max96717_internal_ops = {
+ .init_state = max96717_init_state,
+};
+
+static const struct v4l2_subdev_ops max96717_subdev_ops = {
+ .core = &max96717_subdev_core_ops,
+ .pad = &max96717_pad_ops,
+};
+
+static const struct media_entity_operations max96717_entity_ops = {
+ .link_validate = v4l2_subdev_link_validate,
+};
+
+static int max96717_notify_bound(struct v4l2_async_notifier *notifier,
+ struct v4l2_subdev *source_subdev,
+ struct v4l2_async_connection *asd)
+{
+ struct max96717_priv *priv = sd_to_max96717(notifier->sd);
+ struct device *dev = &priv->client->dev;
+ int ret;
+
+ ret = media_entity_get_fwnode_pad(&source_subdev->entity,
+ source_subdev->fwnode,
+ MEDIA_PAD_FL_SOURCE);
+ if (ret < 0) {
+ dev_err(dev, "Failed to find pad for %s\n",
+ source_subdev->name);
+ return ret;
+ }
+
+ priv->source_sd = source_subdev;
+ priv->source_sd_pad = ret;
+
+ ret = media_create_pad_link(&source_subdev->entity, priv->source_sd_pad,
+ &priv->sd.entity, 0,
+ MEDIA_LNK_FL_ENABLED |
+ MEDIA_LNK_FL_IMMUTABLE);
+ if (ret) {
+ dev_err(dev, "Unable to link %s:%u -> %s:0\n",
+ source_subdev->name, priv->source_sd_pad,
+ priv->sd.name);
+ return ret;
+ }
+
+ return 0;
+}
+
+static const struct v4l2_async_notifier_operations max96717_notify_ops = {
+ .bound = max96717_notify_bound,
+};
+
+static int max96717_v4l2_notifier_register(struct max96717_priv *priv)
+{
+ struct device *dev = &priv->client->dev;
+ struct v4l2_async_connection *asd;
+ struct fwnode_handle *ep_fwnode;
+ int ret;
+
+ ep_fwnode = fwnode_graph_get_endpoint_by_id(dev_fwnode(dev),
+ MAX96717_PAD_SINK, 0, 0);
+ if (!ep_fwnode) {
+ dev_err(dev, "No graph endpoint\n");
+ return -ENODEV;
+ }
+
+ v4l2_async_subdev_nf_init(&priv->notifier, &priv->sd);
+
+ asd = v4l2_async_nf_add_fwnode_remote(&priv->notifier, ep_fwnode,
+ struct v4l2_async_connection);
+
+ fwnode_handle_put(ep_fwnode);
+
+ if (IS_ERR(asd)) {
+ dev_err(dev, "Failed to add subdev: %ld", PTR_ERR(asd));
+ v4l2_async_nf_cleanup(&priv->notifier);
+ return PTR_ERR(asd);
+ }
+
+ priv->notifier.ops = &max96717_notify_ops;
+
+ ret = v4l2_async_nf_register(&priv->notifier);
+ if (ret) {
+ dev_err(dev, "Failed to register subdev_notifier");
+ v4l2_async_nf_cleanup(&priv->notifier);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int max96717_subdev_init(struct max96717_priv *priv)
+{
+ struct device *dev = &priv->client->dev;
+ int ret;
+
+ v4l2_i2c_subdev_init(&priv->sd, priv->client, &max96717_subdev_ops);
+ priv->sd.internal_ops = &max96717_internal_ops;
+
+ priv->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_STREAMS;
+ priv->sd.entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
+ priv->sd.entity.ops = &max96717_entity_ops;
+
+ priv->pads[MAX96717_PAD_SINK].flags = MEDIA_PAD_FL_SINK;
+ priv->pads[MAX96717_PAD_SOURCE].flags = MEDIA_PAD_FL_SOURCE;
+
+ ret = media_entity_pads_init(&priv->sd.entity, 2, priv->pads);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to init pads\n");
+
+ ret = v4l2_subdev_init_finalize(&priv->sd);
+ if (ret) {
+ dev_err_probe(dev, ret,
+ "v4l2 subdev init finalized failed\n");
+ goto err_entity_cleanup;
+ }
+ ret = max96717_v4l2_notifier_register(priv);
+ if (ret) {
+ dev_err_probe(dev, ret,
+ "v4l2 subdev notifier register failed\n");
+ goto err_free_state;
+ }
+
+ ret = v4l2_async_register_subdev(&priv->sd);
+ if (ret) {
+ dev_err_probe(dev, ret, "v4l2_async_register_subdev error\n");
+ goto err_unreg_notif;
+ }
+
+ return 0;
+
+err_unreg_notif:
+ v4l2_async_nf_unregister(&priv->notifier);
+ v4l2_async_nf_cleanup(&priv->notifier);
+err_free_state:
+ v4l2_subdev_cleanup(&priv->sd);
+err_entity_cleanup:
+ media_entity_cleanup(&priv->sd.entity);
+
+ return ret;
+}
+
+static void max96717_subdev_uninit(struct max96717_priv *priv)
+{
+ v4l2_async_unregister_subdev(&priv->sd);
+ v4l2_async_nf_unregister(&priv->notifier);
+ v4l2_async_nf_cleanup(&priv->notifier);
+ v4l2_subdev_cleanup(&priv->sd);
+ media_entity_cleanup(&priv->sd.entity);
+}
+
+struct max96717_pll_predef_freq {
+ unsigned long freq;
+ bool is_alt;
+ u8 val;
+};
+
+static const struct max96717_pll_predef_freq max96717_predef_freqs[] = {
+ { 13500000, true, 0 }, { 19200000, false, 0 },
+ { 24000000, true, 1 }, { 27000000, false, 1 },
+ { 37125000, false, 2 }, { 74250000, false, 3 },
+};
+
+static unsigned long
+max96717_clk_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
+{
+ struct max96717_priv *priv = clk_hw_to_max96717(hw);
+
+ return max96717_predef_freqs[priv->pll_predef_index].freq;
+}
+
+static unsigned int max96717_clk_find_best_index(struct max96717_priv *priv,
+ unsigned long rate)
+{
+ unsigned int i, idx;
+ unsigned long diff_new, diff_old;
+
+ diff_old = U32_MAX;
+ idx = 0;
+
+ for (i = 0; i < ARRAY_SIZE(max96717_predef_freqs); i++) {
+ diff_new = abs(rate - max96717_predef_freqs[i].freq);
+ if (diff_new < diff_old) {
+ diff_old = diff_new;
+ idx = i;
+ }
+ }
+
+ return idx;
+}
+
+static long max96717_clk_round_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long *parent_rate)
+{
+ struct max96717_priv *priv = clk_hw_to_max96717(hw);
+ struct device *dev = &priv->client->dev;
+ unsigned int idx;
+
+ idx = max96717_clk_find_best_index(priv, rate);
+
+ if (rate != max96717_predef_freqs[idx].freq) {
+ dev_warn(dev, "Request CLK freq:%lu, found CLK freq:%lu\n",
+ rate, max96717_predef_freqs[idx].freq);
+ }
+
+ return max96717_predef_freqs[idx].freq;
+}
+
+static int max96717_clk_set_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long parent_rate)
+{
+ struct max96717_priv *priv = clk_hw_to_max96717(hw);
+ unsigned int val, idx;
+ int ret = 0;
+
+ idx = max96717_clk_find_best_index(priv, rate);
+
+ val = FIELD_PREP(REFGEN_PREDEF_FREQ_MASK,
+ max96717_predef_freqs[idx].val);
+
+ if (max96717_predef_freqs[idx].is_alt)
+ val |= REFGEN_PREDEF_FREQ_ALT;
+
+ val |= REFGEN_RST | REFGEN_PREDEF_EN;
+
+ cci_write(priv->regmap, REF_VTG0, val, &ret);
+ cci_update_bits(priv->regmap, REF_VTG0, REFGEN_RST | REFGEN_EN,
+ REFGEN_EN, &ret);
+ if (ret)
+ return ret;
+
+ priv->pll_predef_index = idx;
+
+ return 0;
+}
+
+static int max96717_clk_prepare(struct clk_hw *hw)
+{
+ struct max96717_priv *priv = clk_hw_to_max96717(hw);
+
+ return cci_update_bits(priv->regmap, MAX96717_REG6, RCLKEN,
+ RCLKEN, NULL);
+}
+
+static void max96717_clk_unprepare(struct clk_hw *hw)
+{
+ struct max96717_priv *priv = clk_hw_to_max96717(hw);
+
+ cci_update_bits(priv->regmap, MAX96717_REG6, RCLKEN, 0, NULL);
+}
+
+static const struct clk_ops max96717_clk_ops = {
+ .prepare = max96717_clk_prepare,
+ .unprepare = max96717_clk_unprepare,
+ .set_rate = max96717_clk_set_rate,
+ .recalc_rate = max96717_clk_recalc_rate,
+ .round_rate = max96717_clk_round_rate,
+};
+
+static int max96717_register_clkout(struct max96717_priv *priv)
+{
+ struct device *dev = &priv->client->dev;
+ struct clk_init_data init = { .ops = &max96717_clk_ops };
+ int ret;
+
+ init.name = kasprintf(GFP_KERNEL, "max96717.%s.clk_out",
+ dev_name(dev));
+ if (!init.name)
+ return -ENOMEM;
+
+ /* RCLKSEL Reference PLL output */
+ ret = cci_update_bits(priv->regmap, MAX96717_REG3, MAX96717_RCLKSEL,
+ MAX96717_RCLKSEL, NULL);
+ /* MFP4 fastest slew rate */
+ cci_update_bits(priv->regmap, PIO_SLEW_1, BIT(5) | BIT(4), 0, &ret);
+ if (ret)
+ goto free_init_name;
+
+ priv->clk_hw.init = &init;
+
+ /* Initialize to 24 MHz */
+ ret = max96717_clk_set_rate(&priv->clk_hw,
+ MAX96717_DEFAULT_CLKOUT_RATE, 0);
+ if (ret < 0)
+ goto free_init_name;
+
+ ret = devm_clk_hw_register(dev, &priv->clk_hw);
+ kfree(init.name);
+ if (ret)
+ return dev_err_probe(dev, ret, "Cannot register clock HW\n");
+
+ ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get,
+ &priv->clk_hw);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "Cannot add OF clock provider\n");
+
+ return 0;
+
+free_init_name:
+ kfree(init.name);
+ return ret;
+}
+
+static int max96717_init_csi_lanes(struct max96717_priv *priv)
+{
+ struct v4l2_mbus_config_mipi_csi2 *mipi = &priv->mipi_csi2;
+ unsigned long lanes_used = 0;
+ unsigned int nlanes, lane, val = 0;
+ int ret;
+
+ nlanes = mipi->num_data_lanes;
+
+ ret = cci_update_bits(priv->regmap, MAX96717_MIPI_RX1,
+ MAX96717_MIPI_LANES_CNT,
+ FIELD_PREP(MAX96717_MIPI_LANES_CNT,
+ nlanes - 1), NULL);
+
+ /* lanes polarity */
+ for (lane = 0; lane < nlanes + 1; lane++) {
+ if (!mipi->lane_polarities[lane])
+ continue;
+ /* Clock lane */
+ if (lane == 0)
+ val |= BIT(2);
+ else if (lane < 3)
+ val |= BIT(lane - 1);
+ else
+ val |= BIT(lane);
+ }
+
+ cci_update_bits(priv->regmap, MAX96717_MIPI_RX5,
+ MAX96717_PHY2_LANES_POL,
+ FIELD_PREP(MAX96717_PHY2_LANES_POL, val), &ret);
+
+ cci_update_bits(priv->regmap, MAX96717_MIPI_RX4,
+ MAX96717_PHY1_LANES_POL,
+ FIELD_PREP(MAX96717_PHY1_LANES_POL,
+ val >> 3), &ret);
+ /* lanes mapping */
+ for (lane = 0, val = 0; lane < nlanes; lane++) {
+ val |= (mipi->data_lanes[lane] - 1) << (lane * 2);
+ lanes_used |= BIT(mipi->data_lanes[lane] - 1);
+ }
+
+ /*
+ * Unused lanes need to be mapped as well to not have
+ * the same lanes mapped twice.
+ */
+ for (; lane < 4; lane++) {
+ unsigned int idx = find_first_zero_bit(&lanes_used, 4);
+
+ val |= idx << (lane * 2);
+ lanes_used |= BIT(idx);
+ }
+
+ cci_update_bits(priv->regmap, MAX96717_MIPI_RX3,
+ MAX96717_PHY1_LANES_MAP,
+ FIELD_PREP(MAX96717_PHY1_LANES_MAP, val), &ret);
+
+ return cci_update_bits(priv->regmap, MAX96717_MIPI_RX2,
+ MAX96717_PHY2_LANES_MAP,
+ FIELD_PREP(MAX96717_PHY2_LANES_MAP, val >> 4),
+ &ret);
+}
+
+static int max96717_hw_init(struct max96717_priv *priv)
+{
+ struct device *dev = &priv->client->dev;
+ u64 dev_id, val;
+ int ret;
+
+ ret = cci_read(priv->regmap, MAX96717_DEV_ID, &dev_id, NULL);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "Fail to read the device id\n");
+
+ if (dev_id != MAX96717_DEVICE_ID && dev_id != MAX96717F_DEVICE_ID)
+ return dev_err_probe(dev, -EOPNOTSUPP,
+ "Unsupported device id got %x\n", (u8)dev_id);
+
+ ret = cci_read(priv->regmap, MAX96717_DEV_REV, &val, NULL);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "Fail to read device revision");
+
+ dev_dbg(dev, "Found %x (rev %lx)\n", (u8)dev_id,
+ (u8)val & MAX96717_DEV_REV_MASK);
+
+ ret = cci_read(priv->regmap, MAX96717_MIPI_RX_EXT11, &val, NULL);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "Fail to read mipi rx extension");
+
+ if (!(val & MAX96717_TUN_MODE))
+ return dev_err_probe(dev, -EOPNOTSUPP,
+ "Only supporting tunnel mode");
+
+ return max96717_init_csi_lanes(priv);
+}
+
+static int max96717_parse_dt(struct max96717_priv *priv)
+{
+ struct device *dev = &priv->client->dev;
+ struct v4l2_fwnode_endpoint vep = {
+ .bus_type = V4L2_MBUS_CSI2_DPHY
+ };
+ struct fwnode_handle *ep_fwnode;
+ unsigned char num_data_lanes;
+ int ret;
+
+ ep_fwnode = fwnode_graph_get_endpoint_by_id(dev_fwnode(dev),
+ MAX96717_PAD_SINK, 0, 0);
+ if (!ep_fwnode)
+ return dev_err_probe(dev, -ENOENT, "no endpoint found\n");
+
+ ret = v4l2_fwnode_endpoint_parse(ep_fwnode, &vep);
+
+ fwnode_handle_put(ep_fwnode);
+
+ if (ret < 0)
+ return dev_err_probe(dev, ret, "Failed to parse sink endpoint");
+
+ num_data_lanes = vep.bus.mipi_csi2.num_data_lanes;
+ if (num_data_lanes < 1 || num_data_lanes > 4)
+ return dev_err_probe(dev, -EINVAL,
+ "Invalid data lanes must be 1 to 4\n");
+
+ memcpy(&priv->mipi_csi2, &vep.bus.mipi_csi2, sizeof(priv->mipi_csi2));
+
+ return 0;
+}
+
+static int max96717_probe(struct i2c_client *client)
+{
+ struct device *dev = &client->dev;
+ struct max96717_priv *priv;
+ int ret;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->client = client;
+ priv->regmap = devm_cci_regmap_init_i2c(client, 16);
+ if (IS_ERR(priv->regmap)) {
+ ret = PTR_ERR(priv->regmap);
+ return dev_err_probe(dev, ret, "Failed to init regmap\n");
+ }
+
+ ret = max96717_parse_dt(priv);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to parse the dt\n");
+
+ ret = max96717_hw_init(priv);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "Failed to initialize the hardware\n");
+
+ ret = max96717_gpiochip_probe(priv);
+ if (ret)
+ return dev_err_probe(&client->dev, ret,
+ "Failed to init gpiochip\n");
+
+ ret = max96717_register_clkout(priv);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to register clkout\n");
+
+ ret = max96717_subdev_init(priv);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "Failed to initialize v4l2 subdev\n");
+
+ ret = max96717_i2c_mux_init(priv);
+ if (ret) {
+ dev_err_probe(dev, ret, "failed to add remote i2c adapter\n");
+ max96717_subdev_uninit(priv);
+ }
+
+ return ret;
+}
+
+static void max96717_remove(struct i2c_client *client)
+{
+ struct v4l2_subdev *sd = i2c_get_clientdata(client);
+ struct max96717_priv *priv = sd_to_max96717(sd);
+
+ max96717_subdev_uninit(priv);
+ i2c_mux_del_adapters(priv->mux);
+}
+
+static const struct of_device_id max96717_of_ids[] = {
+ { .compatible = "maxim,max96717f" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, max96717_of_ids);
+
+static struct i2c_driver max96717_i2c_driver = {
+ .driver = {
+ .name = "max96717",
+ .of_match_table = max96717_of_ids,
+ },
+ .probe = max96717_probe,
+ .remove = max96717_remove,
+};
+
+module_i2c_driver(max96717_i2c_driver);
+
+MODULE_DESCRIPTION("Maxim GMSL2 MAX96717 Serializer Driver");
+MODULE_AUTHOR("Julien Massot <julien.massot@collabora.com>");
+MODULE_LICENSE("GPL");
--
2.44.0
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v7 3/5] media: i2c: add MAX96717 driver
2024-04-30 13:19 ` [PATCH v7 3/5] media: i2c: add MAX96717 driver Julien Massot
@ 2024-05-31 13:56 ` Sakari Ailus
2024-06-03 7:29 ` Tommaso Merciai
1 sibling, 0 replies; 14+ messages in thread
From: Sakari Ailus @ 2024-05-31 13:56 UTC (permalink / raw)
To: Julien Massot
Cc: linux-media, devicetree, kernel, linux-kernel, mchehab, robh+dt,
krzysztof.kozlowski+dt, conor+dt
Hi Julien,
A few fairly minor comments below... they probably apply to the other
driver, too.
On Tue, Apr 30, 2024 at 03:19:29PM +0200, Julien Massot wrote:
> This driver handles the MAX96717 serializer in tunnel mode.
> All incoming CSI traffic will be tunneled through the GMSL2
> link.
>
> The MAX96717 driver can handle MAX96717 and MAX96717F variants
> with the same "maxim,max96717f" compatible.
>
> Signed-off-by: Julien Massot <julien.massot@collabora.com>
> ---
> Change since v6:
> - Kconfig select MEDIA_CONTROLLER, V4L2_FWNODE and VIDEO_V4L2_SUBDEV_API
> - rename 'REG3' register to 'MAX96717_REG3'
> - Initialized 'ret' variable in 'max96717_gpiochip_probe'
> - remove max96714_v4l2_notifier_unregister and call the function directly
> - Do not store private pointer with i2c_set_clientdata since the v4l2-i2c
> uses it to store the subdev pointer
> - use dev_err_probe at gpio chip initialization
>
> Change since v5:
> - set the driver compatible back to MAX96717F that can be used as a fallback for MAX96717
>
> Change since v4:
> - make the driver compatible with MAX96717 instead of MAX96717F
> - Add the device id for the MAX96717
> - remove hw_data structure for now, it can be usefull later for handling different serializers e.g max9295
>
> Change since v3:
> - Maintainers: align to the new binding path
> - Kconfig: better describe the symbol
> - store the v4l2_mbus_config_mipi_csi2 structure instead of the full endpoint in the driver private structure
> - use MAX96717_PAD_SINK/SOURCE instead of 0/1 for pad intialization
> - Removed incorrect call to fwnode_handle_put(priv->sd.fwnode)
> - Use unsigned int instead of u8
> - Allocate clk name out of the clk struct initialization
> - fixed multiline comment
> - Removed one unnecessary goto at the end of the probe function
>
> Change since v2:
> - Use CCI helpers instead of recoding register access
> - add missing bitfield header
> ---
> MAINTAINERS | 7 +
> drivers/media/i2c/Kconfig | 17 +
> drivers/media/i2c/Makefile | 1 +
> drivers/media/i2c/max96717.c | 928 +++++++++++++++++++++++++++++++++++
> 4 files changed, 953 insertions(+)
> create mode 100644 drivers/media/i2c/max96717.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index eea74166a2d9..cfaa904ace59 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -13426,6 +13426,13 @@ S: Maintained
> F: Documentation/devicetree/bindings/media/i2c/maxim,max96712.yaml
> F: drivers/staging/media/max96712/max96712.c
>
> +MAX96717 GMSL2 SERIALIZER DRIVER
> +M: Julien Massot <julien.massot@collabora.com>
> +L: linux-media@vger.kernel.org
> +S: Maintained
> +F: Documentation/devicetree/bindings/media/i2c/maxim,max96717.yaml
> +F: drivers/media/i2c/max96717.c
> +
> MAX9860 MONO AUDIO VOICE CODEC DRIVER
> M: Peter Rosin <peda@axentia.se>
> L: alsa-devel@alsa-project.org (moderated for non-subscribers)
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index c6d3ee472d81..9918195e09ba 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -1575,6 +1575,23 @@ config VIDEO_DS90UB960
> Device driver for the Texas Instruments DS90UB960
> FPD-Link III Deserializer and DS90UB9702 FPD-Link IV Deserializer.
>
> +config VIDEO_MAX96717
> + tristate "Maxim MAX96717 GMSL2 Serializer support"
> + depends on OF && I2C && VIDEO_DEV && COMMON_CLK
> + select I2C_MUX
> + select MEDIA_CONTROLLER
> + select GPIOLIB
> + select V4L2_CCI_I2C
> + select V4L2_FWNODE
> + select VIDEO_V4L2_SUBDEV_API
> + help
> + Device driver for the Maxim MAX96717 GMSL2 Serializer.
> + MAX96717 serializers convert video on a MIPI CSI-2
> + input to a GMSL2 output.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called max96717.
> +
> endmenu
>
> endif # VIDEO_DEV
> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> index dfbe6448b549..9e007116f929 100644
> --- a/drivers/media/i2c/Makefile
> +++ b/drivers/media/i2c/Makefile
> @@ -64,6 +64,7 @@ obj-$(CONFIG_VIDEO_LM3646) += lm3646.o
> obj-$(CONFIG_VIDEO_M52790) += m52790.o
> obj-$(CONFIG_VIDEO_MAX9271_LIB) += max9271.o
> obj-$(CONFIG_VIDEO_MAX9286) += max9286.o
> +obj-$(CONFIG_VIDEO_MAX96717) += max96717.o
> obj-$(CONFIG_VIDEO_ML86V7667) += ml86v7667.o
> obj-$(CONFIG_VIDEO_MSP3400) += msp3400.o
> obj-$(CONFIG_VIDEO_MT9M001) += mt9m001.o
> diff --git a/drivers/media/i2c/max96717.c b/drivers/media/i2c/max96717.c
> new file mode 100644
> index 000000000000..1ea76f922bdb
> --- /dev/null
> +++ b/drivers/media/i2c/max96717.c
> @@ -0,0 +1,928 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Maxim GMSL2 Serializer Driver
> + *
> + * Copyright (C) 2024 Collabora Ltd.
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>
> +#include <linux/delay.h>
> +#include <linux/fwnode.h>
> +#include <linux/gpio/driver.h>
> +#include <linux/i2c-mux.h>
> +#include <linux/i2c.h>
> +#include <linux/regmap.h>
> +
> +#include <media/v4l2-cci.h>
> +#include <media/v4l2-fwnode.h>
> +#include <media/v4l2-subdev.h>
> +
> +#define MAX96717_DEVICE_ID 0xbf
> +#define MAX96717F_DEVICE_ID 0xc8
> +#define MAX96717_PORTS 2
> +#define MAX96717_PAD_SINK 0
> +#define MAX96717_PAD_SOURCE 1
> +
> +#define MAX96717_DEFAULT_CLKOUT_RATE 24000000UL
> +
> +/* DEV */
> +#define MAX96717_REG3 CCI_REG8(0x3)
> +#define MAX96717_RCLKSEL GENMASK(1, 0)
> +#define RCLKSEL_REF_PLL CCI_REG8(0x3)
> +#define MAX96717_REG6 CCI_REG8(0x6)
> +#define RCLKEN BIT(5)
> +#define MAX96717_DEV_ID CCI_REG8(0xd)
> +#define MAX96717_DEV_REV CCI_REG8(0xe)
> +#define MAX96717_DEV_REV_MASK GENMASK(3, 0)
> +
> +/* VID_TX Z */
> +#define MAX96717_VIDEO_TX2 CCI_REG8(0x112)
> +#define MAX96717_VIDEO_PCLKDET BIT(7)
> +
> +/* GPIO */
> +#define MAX96717_NUM_GPIO 11
> +#define MAX96717_GPIO_REG_A(gpio) CCI_REG8(0x2be + (gpio) * 3)
> +#define MAX96717_GPIO_OUT BIT(4)
> +#define MAX96717_GPIO_IN BIT(3)
> +#define MAX96717_GPIO_RX_EN BIT(2)
> +#define MAX96717_GPIO_TX_EN BIT(1)
> +#define MAX96717_GPIO_OUT_DIS BIT(0)
> +
> +/* FRONTTOP */
> +/* MAX96717 only have CSI port 'B' */
> +#define MAX96717_FRONTOP0 CCI_REG8(0x308)
> +#define MAX96717_START_PORT_B BIT(5)
> +
> +/* MIPI_RX */
> +#define MAX96717_MIPI_RX1 CCI_REG8(0x331)
> +#define MAX96717_MIPI_LANES_CNT GENMASK(5, 4)
> +#define MAX96717_MIPI_RX2 CCI_REG8(0x332) /* phy1 Lanes map */
> +#define MAX96717_PHY2_LANES_MAP GENMASK(7, 4)
> +#define MAX96717_MIPI_RX3 CCI_REG8(0x333) /* phy2 Lanes map */
> +#define MAX96717_PHY1_LANES_MAP GENMASK(3, 0)
> +#define MAX96717_MIPI_RX4 CCI_REG8(0x334) /* phy1 lane polarities */
> +#define MAX96717_PHY1_LANES_POL GENMASK(6, 4)
> +#define MAX96717_MIPI_RX5 CCI_REG8(0x335) /* phy2 lane polarities */
> +#define MAX96717_PHY2_LANES_POL GENMASK(2, 0)
> +
> +/* MIPI_RX_EXT */
> +#define MAX96717_MIPI_RX_EXT11 CCI_REG8(0x383)
> +#define MAX96717_TUN_MODE BIT(7)
> +
> +/* REF_VTG */
> +#define REF_VTG0 CCI_REG8(0x3f0)
> +#define REFGEN_PREDEF_EN BIT(6)
> +#define REFGEN_PREDEF_FREQ_MASK GENMASK(5, 4)
> +#define REFGEN_PREDEF_FREQ_ALT BIT(3)
> +#define REFGEN_RST BIT(1)
> +#define REFGEN_EN BIT(0)
> +
> +/* MISC */
> +#define PIO_SLEW_1 CCI_REG8(0x570)
> +
> +struct max96717_priv {
> + struct i2c_client *client;
> + struct regmap *regmap;
> + struct i2c_mux_core *mux;
> + struct v4l2_mbus_config_mipi_csi2 mipi_csi2;
> + struct v4l2_subdev sd;
> + struct media_pad pads[MAX96717_PORTS];
> + struct v4l2_async_notifier notifier;
> + struct v4l2_subdev *source_sd;
> + u16 source_sd_pad;
> + u64 enabled_source_streams;
> + u8 pll_predef_index;
> + struct clk_hw clk_hw;
> + struct gpio_chip gpio_chip;
> +};
> +
> +static inline struct max96717_priv *sd_to_max96717(struct v4l2_subdev *sd)
> +{
> + return container_of(sd, struct max96717_priv, sd);
> +}
> +
> +static inline struct max96717_priv *clk_hw_to_max96717(struct clk_hw *hw)
> +{
> + return container_of(hw, struct max96717_priv, clk_hw);
> +}
> +
> +static int max96717_i2c_mux_select(struct i2c_mux_core *mux, u32 chan)
> +{
> + return 0;
> +}
> +
> +static int max96717_i2c_mux_init(struct max96717_priv *priv)
> +{
> + priv->mux = i2c_mux_alloc(priv->client->adapter, &priv->client->dev,
> + 1, 0, I2C_MUX_LOCKED | I2C_MUX_GATE,
> + max96717_i2c_mux_select, NULL);
> + if (!priv->mux)
> + return -ENOMEM;
> +
> + return i2c_mux_add_adapter(priv->mux, 0, 0, 0);
> +}
> +
> +static inline int max96717_start_csi(struct max96717_priv *priv, bool start)
> +{
> + return cci_update_bits(priv->regmap, MAX96717_FRONTOP0,
> + MAX96717_START_PORT_B,
> + start ? MAX96717_START_PORT_B : 0, NULL);
> +}
> +
> +static int max96717_gpiochip_get(struct gpio_chip *gpiochip,
> + unsigned int offset)
> +{
> + struct max96717_priv *priv = gpiochip_get_data(gpiochip);
> + u64 val;
> + int ret;
> +
> + ret = cci_read(priv->regmap, MAX96717_GPIO_REG_A(offset),
> + &val, NULL);
> + if (ret)
> + return ret;
> +
> + if (val & MAX96717_GPIO_OUT_DIS)
> + return !!(val & MAX96717_GPIO_IN);
> + else
> + return !!(val & MAX96717_GPIO_OUT);
> +}
> +
> +static void max96717_gpiochip_set(struct gpio_chip *gpiochip,
> + unsigned int offset, int value)
> +{
> + struct max96717_priv *priv = gpiochip_get_data(gpiochip);
> +
> + cci_update_bits(priv->regmap, MAX96717_GPIO_REG_A(offset),
> + MAX96717_GPIO_OUT, MAX96717_GPIO_OUT, NULL);
> +}
> +
> +static int max96717_gpio_get_direction(struct gpio_chip *gpiochip,
> + unsigned int offset)
> +{
> + struct max96717_priv *priv = gpiochip_get_data(gpiochip);
> + u64 val;
> + int ret;
> +
> + ret = cci_read(priv->regmap, MAX96717_GPIO_REG_A(offset), &val, NULL);
> + if (ret < 0)
> + return ret;
> +
> + return !!(val & MAX96717_GPIO_OUT_DIS);
> +}
> +
> +static int max96717_gpio_direction_out(struct gpio_chip *gpiochip,
> + unsigned int offset, int value)
> +{
> + struct max96717_priv *priv = gpiochip_get_data(gpiochip);
> +
> + return cci_update_bits(priv->regmap, MAX96717_GPIO_REG_A(offset),
> + MAX96717_GPIO_OUT_DIS | MAX96717_GPIO_OUT,
> + value ? MAX96717_GPIO_OUT : 0, NULL);
> +}
> +
> +static int max96717_gpio_direction_in(struct gpio_chip *gpiochip,
> + unsigned int offset)
> +{
> + struct max96717_priv *priv = gpiochip_get_data(gpiochip);
> +
> + return cci_update_bits(priv->regmap, MAX96717_GPIO_REG_A(offset),
> + MAX96717_GPIO_OUT_DIS, MAX96717_GPIO_OUT_DIS,
> + NULL);
> +}
> +
> +static int max96717_gpiochip_probe(struct max96717_priv *priv)
> +{
> + struct device *dev = &priv->client->dev;
> + struct gpio_chip *gc = &priv->gpio_chip;
> + int i, ret = 0;
> +
> + gc->label = dev_name(dev);
> + gc->parent = dev;
> + gc->owner = THIS_MODULE;
> + gc->ngpio = MAX96717_NUM_GPIO;
> + gc->base = -1;
> + gc->can_sleep = true;
> + gc->get_direction = max96717_gpio_get_direction;
> + gc->direction_input = max96717_gpio_direction_in;
> + gc->direction_output = max96717_gpio_direction_out;
> + gc->set = max96717_gpiochip_set;
> + gc->get = max96717_gpiochip_get;
> + gc->of_gpio_n_cells = 2;
> +
> + /* Disable GPIO forwarding */
> + for (i = 0; i < gc->ngpio; i++)
> + cci_update_bits(priv->regmap, MAX96717_GPIO_REG_A(i),
> + MAX96717_GPIO_RX_EN | MAX96717_GPIO_TX_EN,
> + 0, &ret);
> +
> + if (ret)
> + return ret;
> +
> + ret = devm_gpiochip_add_data(dev, gc, priv);
> + if (ret) {
> + dev_err(dev, "Unable to create gpio_chip\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int _max96717_set_routing(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state,
> + struct v4l2_subdev_krouting *routing)
> +{
> + static const struct v4l2_mbus_framefmt format = {
> + .width = 1280,
> + .height = 1080,
> + .code = MEDIA_BUS_FMT_Y8_1X8,
> + .field = V4L2_FIELD_NONE,
> + };
> + int ret;
> +
> + ret = v4l2_subdev_routing_validate(sd, routing,
> + V4L2_SUBDEV_ROUTING_ONLY_1_TO_1);
> + if (ret)
> + return ret;
> +
> + ret = v4l2_subdev_set_routing_with_fmt(sd, state, routing, &format);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> +static int max96717_set_routing(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state,
> + enum v4l2_subdev_format_whence which,
> + struct v4l2_subdev_krouting *routing)
> +{
> + struct max96717_priv *priv = sd_to_max96717(sd);
> +
> + if (which == V4L2_SUBDEV_FORMAT_ACTIVE && priv->enabled_source_streams)
> + return -EBUSY;
> +
> + return _max96717_set_routing(sd, state, routing);
> +}
> +
> +static int max96717_set_fmt(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state,
> + struct v4l2_subdev_format *format)
> +{
> + struct max96717_priv *priv = sd_to_max96717(sd);
> + struct v4l2_mbus_framefmt *fmt;
> + u64 stream_source_mask;
> +
> + if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE &&
> + priv->enabled_source_streams)
> + return -EBUSY;
> +
> + /* No transcoding, source and sink formats must match. */
> + if (format->pad == MAX96717_PAD_SOURCE)
> + return v4l2_subdev_get_fmt(sd, state, format);
> +
> + /* Set sink format */
> + fmt = v4l2_subdev_state_get_format(state, format->pad, format->stream);
> + if (!fmt)
> + return -EINVAL;
> +
> + *fmt = format->format;
> +
> + /* Propagate to source format */
> + fmt = v4l2_subdev_state_get_opposite_stream_format(state, format->pad,
> + format->stream);
> + if (!fmt)
> + return -EINVAL;
> + *fmt = format->format;
> +
> + stream_source_mask = BIT(format->stream);
> +
> + return v4l2_subdev_state_xlate_streams(state, MAX96717_PAD_SOURCE,
> + MAX96717_PAD_SINK,
> + &stream_source_mask);
> +}
> +
> +static int max96717_init_state(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state)
> +{
> + struct v4l2_subdev_route routes[] = {
> + {
> + .sink_pad = MAX96717_PAD_SINK,
> + .sink_stream = 0,
> + .source_pad = MAX96717_PAD_SOURCE,
> + .source_stream = 0,
> + .flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE,
> + },
> + };
> + struct v4l2_subdev_krouting routing = {
> + .num_routes = ARRAY_SIZE(routes),
> + .routes = routes,
> + };
> +
> + return _max96717_set_routing(sd, state, &routing);
> +}
> +
> +static bool max96717_pipe_pclkdet(struct max96717_priv *priv)
> +{
> + u64 val = 0;
> +
> + cci_read(priv->regmap, MAX96717_VIDEO_TX2, &val, NULL);
> +
> + return val & MAX96717_VIDEO_PCLKDET;
> +}
> +
> +static int max96717_log_status(struct v4l2_subdev *sd)
> +{
> + struct max96717_priv *priv = sd_to_max96717(sd);
> + struct device *dev = &priv->client->dev;
> +
> + dev_info(dev, "Serializer: max96717\n");
> + dev_info(dev, "Pipe: pclkdet:%d\n", max96717_pipe_pclkdet(priv));
> +
> + return 0;
> +}
> +
> +static int max96717_enable_streams(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state, u32 pad,
> + u64 streams_mask)
> +{
> + struct max96717_priv *priv = sd_to_max96717(sd);
> + struct device *dev = &priv->client->dev;
> + u64 sink_streams;
> + int ret;
> +
> + sink_streams = v4l2_subdev_state_xlate_streams(state,
> + MAX96717_PAD_SOURCE,
> + MAX96717_PAD_SINK,
> + &streams_mask);
> +
> + if (!priv->enabled_source_streams)
> + max96717_start_csi(priv, true);
> +
> + ret = v4l2_subdev_enable_streams(priv->source_sd, priv->source_sd_pad,
> + sink_streams);
> + if (ret) {
> + dev_err(dev, "Fail to start streams:%llu on remote subdev\n",
> + sink_streams);
Doesn't v4l2_subdev_enable_streams() print this?
> + goto stop_csi;
> + }
> +
> + priv->enabled_source_streams |= streams_mask;
> +
> + return 0;
> +
> +stop_csi:
> + if (!priv->enabled_source_streams)
> + max96717_start_csi(priv, false);
A newline would be nice here.
> + return ret;
> +}
> +
> +static int max96717_disable_streams(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state, u32 pad,
> + u64 streams_mask)
> +{
> + struct max96717_priv *priv = sd_to_max96717(sd);
> + u64 sink_streams;
> + int ret;
> +
> + sink_streams = v4l2_subdev_state_xlate_streams(state,
> + MAX96717_PAD_SOURCE,
> + MAX96717_PAD_SINK,
> + &streams_mask);
> +
> + ret = v4l2_subdev_disable_streams(priv->source_sd, priv->source_sd_pad,
> + sink_streams);
> + if (ret)
> + return ret;
> +
> + priv->enabled_source_streams &= ~streams_mask;
> +
> + if (!priv->enabled_source_streams)
> + max96717_start_csi(priv, false);
> +
> + return 0;
> +}
> +
> +static const struct v4l2_subdev_pad_ops max96717_pad_ops = {
> + .enable_streams = max96717_enable_streams,
> + .disable_streams = max96717_disable_streams,
> + .set_routing = max96717_set_routing,
> + .get_fmt = v4l2_subdev_get_fmt,
> + .set_fmt = max96717_set_fmt,
> +};
> +
> +static const struct v4l2_subdev_core_ops max96717_subdev_core_ops = {
> + .log_status = max96717_log_status,
> +};
> +
> +static const struct v4l2_subdev_internal_ops max96717_internal_ops = {
> + .init_state = max96717_init_state,
> +};
> +
> +static const struct v4l2_subdev_ops max96717_subdev_ops = {
> + .core = &max96717_subdev_core_ops,
> + .pad = &max96717_pad_ops,
> +};
> +
> +static const struct media_entity_operations max96717_entity_ops = {
> + .link_validate = v4l2_subdev_link_validate,
> +};
> +
> +static int max96717_notify_bound(struct v4l2_async_notifier *notifier,
> + struct v4l2_subdev *source_subdev,
> + struct v4l2_async_connection *asd)
> +{
> + struct max96717_priv *priv = sd_to_max96717(notifier->sd);
> + struct device *dev = &priv->client->dev;
> + int ret;
> +
> + ret = media_entity_get_fwnode_pad(&source_subdev->entity,
> + source_subdev->fwnode,
> + MEDIA_PAD_FL_SOURCE);
> + if (ret < 0) {
> + dev_err(dev, "Failed to find pad for %s\n",
> + source_subdev->name);
> + return ret;
> + }
> +
> + priv->source_sd = source_subdev;
> + priv->source_sd_pad = ret;
> +
> + ret = media_create_pad_link(&source_subdev->entity, priv->source_sd_pad,
> + &priv->sd.entity, 0,
> + MEDIA_LNK_FL_ENABLED |
> + MEDIA_LNK_FL_IMMUTABLE);
> + if (ret) {
> + dev_err(dev, "Unable to link %s:%u -> %s:0\n",
> + source_subdev->name, priv->source_sd_pad,
> + priv->sd.name);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static const struct v4l2_async_notifier_operations max96717_notify_ops = {
> + .bound = max96717_notify_bound,
> +};
> +
> +static int max96717_v4l2_notifier_register(struct max96717_priv *priv)
> +{
> + struct device *dev = &priv->client->dev;
> + struct v4l2_async_connection *asd;
> + struct fwnode_handle *ep_fwnode;
> + int ret;
> +
> + ep_fwnode = fwnode_graph_get_endpoint_by_id(dev_fwnode(dev),
> + MAX96717_PAD_SINK, 0, 0);
> + if (!ep_fwnode) {
> + dev_err(dev, "No graph endpoint\n");
> + return -ENODEV;
> + }
> +
> + v4l2_async_subdev_nf_init(&priv->notifier, &priv->sd);
> +
> + asd = v4l2_async_nf_add_fwnode_remote(&priv->notifier, ep_fwnode,
> + struct v4l2_async_connection);
> +
> + fwnode_handle_put(ep_fwnode);
> +
> + if (IS_ERR(asd)) {
> + dev_err(dev, "Failed to add subdev: %ld", PTR_ERR(asd));
> + v4l2_async_nf_cleanup(&priv->notifier);
> + return PTR_ERR(asd);
> + }
> +
> + priv->notifier.ops = &max96717_notify_ops;
> +
> + ret = v4l2_async_nf_register(&priv->notifier);
> + if (ret) {
> + dev_err(dev, "Failed to register subdev_notifier");
> + v4l2_async_nf_cleanup(&priv->notifier);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int max96717_subdev_init(struct max96717_priv *priv)
> +{
> + struct device *dev = &priv->client->dev;
> + int ret;
> +
> + v4l2_i2c_subdev_init(&priv->sd, priv->client, &max96717_subdev_ops);
> + priv->sd.internal_ops = &max96717_internal_ops;
> +
> + priv->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_STREAMS;
> + priv->sd.entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
> + priv->sd.entity.ops = &max96717_entity_ops;
> +
> + priv->pads[MAX96717_PAD_SINK].flags = MEDIA_PAD_FL_SINK;
> + priv->pads[MAX96717_PAD_SOURCE].flags = MEDIA_PAD_FL_SOURCE;
> +
> + ret = media_entity_pads_init(&priv->sd.entity, 2, priv->pads);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to init pads\n");
> +
> + ret = v4l2_subdev_init_finalize(&priv->sd);
> + if (ret) {
> + dev_err_probe(dev, ret,
> + "v4l2 subdev init finalized failed\n");
> + goto err_entity_cleanup;
> + }
> + ret = max96717_v4l2_notifier_register(priv);
> + if (ret) {
> + dev_err_probe(dev, ret,
> + "v4l2 subdev notifier register failed\n");
> + goto err_free_state;
> + }
> +
> + ret = v4l2_async_register_subdev(&priv->sd);
> + if (ret) {
> + dev_err_probe(dev, ret, "v4l2_async_register_subdev error\n");
> + goto err_unreg_notif;
> + }
> +
> + return 0;
> +
> +err_unreg_notif:
> + v4l2_async_nf_unregister(&priv->notifier);
> + v4l2_async_nf_cleanup(&priv->notifier);
> +err_free_state:
> + v4l2_subdev_cleanup(&priv->sd);
> +err_entity_cleanup:
> + media_entity_cleanup(&priv->sd.entity);
> +
> + return ret;
> +}
> +
> +static void max96717_subdev_uninit(struct max96717_priv *priv)
> +{
> + v4l2_async_unregister_subdev(&priv->sd);
> + v4l2_async_nf_unregister(&priv->notifier);
> + v4l2_async_nf_cleanup(&priv->notifier);
> + v4l2_subdev_cleanup(&priv->sd);
> + media_entity_cleanup(&priv->sd.entity);
> +}
> +
> +struct max96717_pll_predef_freq {
> + unsigned long freq;
> + bool is_alt;
> + u8 val;
> +};
> +
> +static const struct max96717_pll_predef_freq max96717_predef_freqs[] = {
> + { 13500000, true, 0 }, { 19200000, false, 0 },
> + { 24000000, true, 1 }, { 27000000, false, 1 },
> + { 37125000, false, 2 }, { 74250000, false, 3 },
> +};
> +
> +static unsigned long
> +max96717_clk_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
> +{
> + struct max96717_priv *priv = clk_hw_to_max96717(hw);
> +
> + return max96717_predef_freqs[priv->pll_predef_index].freq;
> +}
> +
> +static unsigned int max96717_clk_find_best_index(struct max96717_priv *priv,
> + unsigned long rate)
> +{
> + unsigned int i, idx;
> + unsigned long diff_new, diff_old;
> +
> + diff_old = U32_MAX;
> + idx = 0;
Please initialise in declaration (or loop below).
> +
> + for (i = 0; i < ARRAY_SIZE(max96717_predef_freqs); i++) {
> + diff_new = abs(rate - max96717_predef_freqs[i].freq);
> + if (diff_new < diff_old) {
> + diff_old = diff_new;
> + idx = i;
> + }
> + }
> +
> + return idx;
> +}
> +
> +static long max96717_clk_round_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long *parent_rate)
> +{
> + struct max96717_priv *priv = clk_hw_to_max96717(hw);
> + struct device *dev = &priv->client->dev;
> + unsigned int idx;
> +
> + idx = max96717_clk_find_best_index(priv, rate);
> +
> + if (rate != max96717_predef_freqs[idx].freq) {
> + dev_warn(dev, "Request CLK freq:%lu, found CLK freq:%lu\n",
> + rate, max96717_predef_freqs[idx].freq);
> + }
> +
> + return max96717_predef_freqs[idx].freq;
> +}
> +
> +static int max96717_clk_set_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long parent_rate)
> +{
> + struct max96717_priv *priv = clk_hw_to_max96717(hw);
> + unsigned int val, idx;
> + int ret = 0;
> +
> + idx = max96717_clk_find_best_index(priv, rate);
> +
> + val = FIELD_PREP(REFGEN_PREDEF_FREQ_MASK,
> + max96717_predef_freqs[idx].val);
> +
> + if (max96717_predef_freqs[idx].is_alt)
> + val |= REFGEN_PREDEF_FREQ_ALT;
> +
> + val |= REFGEN_RST | REFGEN_PREDEF_EN;
> +
> + cci_write(priv->regmap, REF_VTG0, val, &ret);
> + cci_update_bits(priv->regmap, REF_VTG0, REFGEN_RST | REFGEN_EN,
> + REFGEN_EN, &ret);
> + if (ret)
> + return ret;
> +
> + priv->pll_predef_index = idx;
> +
> + return 0;
> +}
> +
> +static int max96717_clk_prepare(struct clk_hw *hw)
> +{
> + struct max96717_priv *priv = clk_hw_to_max96717(hw);
> +
> + return cci_update_bits(priv->regmap, MAX96717_REG6, RCLKEN,
> + RCLKEN, NULL);
> +}
> +
> +static void max96717_clk_unprepare(struct clk_hw *hw)
> +{
> + struct max96717_priv *priv = clk_hw_to_max96717(hw);
> +
> + cci_update_bits(priv->regmap, MAX96717_REG6, RCLKEN, 0, NULL);
> +}
> +
> +static const struct clk_ops max96717_clk_ops = {
> + .prepare = max96717_clk_prepare,
> + .unprepare = max96717_clk_unprepare,
> + .set_rate = max96717_clk_set_rate,
> + .recalc_rate = max96717_clk_recalc_rate,
> + .round_rate = max96717_clk_round_rate,
> +};
> +
> +static int max96717_register_clkout(struct max96717_priv *priv)
> +{
> + struct device *dev = &priv->client->dev;
> + struct clk_init_data init = { .ops = &max96717_clk_ops };
> + int ret;
> +
> + init.name = kasprintf(GFP_KERNEL, "max96717.%s.clk_out",
> + dev_name(dev));
Fits on the previous line.
> + if (!init.name)
> + return -ENOMEM;
> +
> + /* RCLKSEL Reference PLL output */
> + ret = cci_update_bits(priv->regmap, MAX96717_REG3, MAX96717_RCLKSEL,
> + MAX96717_RCLKSEL, NULL);
> + /* MFP4 fastest slew rate */
> + cci_update_bits(priv->regmap, PIO_SLEW_1, BIT(5) | BIT(4), 0, &ret);
> + if (ret)
> + goto free_init_name;
> +
> + priv->clk_hw.init = &init;
> +
> + /* Initialize to 24 MHz */
> + ret = max96717_clk_set_rate(&priv->clk_hw,
> + MAX96717_DEFAULT_CLKOUT_RATE, 0);
> + if (ret < 0)
> + goto free_init_name;
> +
> + ret = devm_clk_hw_register(dev, &priv->clk_hw);
> + kfree(init.name);
> + if (ret)
> + return dev_err_probe(dev, ret, "Cannot register clock HW\n");
> +
> + ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get,
> + &priv->clk_hw);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "Cannot add OF clock provider\n");
> +
> + return 0;
> +
> +free_init_name:
> + kfree(init.name);
> + return ret;
> +}
> +
> +static int max96717_init_csi_lanes(struct max96717_priv *priv)
> +{
> + struct v4l2_mbus_config_mipi_csi2 *mipi = &priv->mipi_csi2;
> + unsigned long lanes_used = 0;
> + unsigned int nlanes, lane, val = 0;
> + int ret;
> +
> + nlanes = mipi->num_data_lanes;
> +
> + ret = cci_update_bits(priv->regmap, MAX96717_MIPI_RX1,
> + MAX96717_MIPI_LANES_CNT,
> + FIELD_PREP(MAX96717_MIPI_LANES_CNT,
> + nlanes - 1), NULL);
> +
> + /* lanes polarity */
> + for (lane = 0; lane < nlanes + 1; lane++) {
> + if (!mipi->lane_polarities[lane])
> + continue;
> + /* Clock lane */
> + if (lane == 0)
> + val |= BIT(2);
> + else if (lane < 3)
> + val |= BIT(lane - 1);
> + else
> + val |= BIT(lane);
> + }
> +
> + cci_update_bits(priv->regmap, MAX96717_MIPI_RX5,
> + MAX96717_PHY2_LANES_POL,
> + FIELD_PREP(MAX96717_PHY2_LANES_POL, val), &ret);
> +
> + cci_update_bits(priv->regmap, MAX96717_MIPI_RX4,
> + MAX96717_PHY1_LANES_POL,
> + FIELD_PREP(MAX96717_PHY1_LANES_POL,
> + val >> 3), &ret);
> + /* lanes mapping */
> + for (lane = 0, val = 0; lane < nlanes; lane++) {
> + val |= (mipi->data_lanes[lane] - 1) << (lane * 2);
> + lanes_used |= BIT(mipi->data_lanes[lane] - 1);
> + }
> +
> + /*
> + * Unused lanes need to be mapped as well to not have
> + * the same lanes mapped twice.
> + */
> + for (; lane < 4; lane++) {
> + unsigned int idx = find_first_zero_bit(&lanes_used, 4);
It'd be nice to use a macro here instead of "4". Same elsewhere.
> +
> + val |= idx << (lane * 2);
> + lanes_used |= BIT(idx);
> + }
> +
> + cci_update_bits(priv->regmap, MAX96717_MIPI_RX3,
> + MAX96717_PHY1_LANES_MAP,
> + FIELD_PREP(MAX96717_PHY1_LANES_MAP, val), &ret);
> +
> + return cci_update_bits(priv->regmap, MAX96717_MIPI_RX2,
> + MAX96717_PHY2_LANES_MAP,
> + FIELD_PREP(MAX96717_PHY2_LANES_MAP, val >> 4),
> + &ret);
> +}
> +
> +static int max96717_hw_init(struct max96717_priv *priv)
> +{
> + struct device *dev = &priv->client->dev;
> + u64 dev_id, val;
> + int ret;
> +
> + ret = cci_read(priv->regmap, MAX96717_DEV_ID, &dev_id, NULL);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "Fail to read the device id\n");
> +
> + if (dev_id != MAX96717_DEVICE_ID && dev_id != MAX96717F_DEVICE_ID)
> + return dev_err_probe(dev, -EOPNOTSUPP,
> + "Unsupported device id got %x\n", (u8)dev_id);
> +
> + ret = cci_read(priv->regmap, MAX96717_DEV_REV, &val, NULL);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "Fail to read device revision");
> +
> + dev_dbg(dev, "Found %x (rev %lx)\n", (u8)dev_id,
> + (u8)val & MAX96717_DEV_REV_MASK);
> +
> + ret = cci_read(priv->regmap, MAX96717_MIPI_RX_EXT11, &val, NULL);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "Fail to read mipi rx extension");
> +
> + if (!(val & MAX96717_TUN_MODE))
> + return dev_err_probe(dev, -EOPNOTSUPP,
> + "Only supporting tunnel mode");
> +
> + return max96717_init_csi_lanes(priv);
> +}
> +
> +static int max96717_parse_dt(struct max96717_priv *priv)
> +{
> + struct device *dev = &priv->client->dev;
> + struct v4l2_fwnode_endpoint vep = {
> + .bus_type = V4L2_MBUS_CSI2_DPHY
> + };
Fits on a single line.
> + struct fwnode_handle *ep_fwnode;
> + unsigned char num_data_lanes;
> + int ret;
> +
> + ep_fwnode = fwnode_graph_get_endpoint_by_id(dev_fwnode(dev),
> + MAX96717_PAD_SINK, 0, 0);
> + if (!ep_fwnode)
> + return dev_err_probe(dev, -ENOENT, "no endpoint found\n");
> +
> + ret = v4l2_fwnode_endpoint_parse(ep_fwnode, &vep);
> +
> + fwnode_handle_put(ep_fwnode);
> +
> + if (ret < 0)
> + return dev_err_probe(dev, ret, "Failed to parse sink endpoint");
> +
> + num_data_lanes = vep.bus.mipi_csi2.num_data_lanes;
> + if (num_data_lanes < 1 || num_data_lanes > 4)
> + return dev_err_probe(dev, -EINVAL,
> + "Invalid data lanes must be 1 to 4\n");
> +
> + memcpy(&priv->mipi_csi2, &vep.bus.mipi_csi2, sizeof(priv->mipi_csi2));
priv->mipi_csi2 = vep.bus.mipi_csi2;
> +
> + return 0;
> +}
> +
> +static int max96717_probe(struct i2c_client *client)
> +{
> + struct device *dev = &client->dev;
> + struct max96717_priv *priv;
> + int ret;
> +
> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + priv->client = client;
> + priv->regmap = devm_cci_regmap_init_i2c(client, 16);
> + if (IS_ERR(priv->regmap)) {
> + ret = PTR_ERR(priv->regmap);
> + return dev_err_probe(dev, ret, "Failed to init regmap\n");
> + }
> +
> + ret = max96717_parse_dt(priv);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to parse the dt\n");
> +
> + ret = max96717_hw_init(priv);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "Failed to initialize the hardware\n");
> +
> + ret = max96717_gpiochip_probe(priv);
> + if (ret)
> + return dev_err_probe(&client->dev, ret,
> + "Failed to init gpiochip\n");
> +
> + ret = max96717_register_clkout(priv);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to register clkout\n");
> +
> + ret = max96717_subdev_init(priv);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "Failed to initialize v4l2 subdev\n");
> +
> + ret = max96717_i2c_mux_init(priv);
> + if (ret) {
> + dev_err_probe(dev, ret, "failed to add remote i2c adapter\n");
> + max96717_subdev_uninit(priv);
> + }
> +
> + return ret;
> +}
> +
> +static void max96717_remove(struct i2c_client *client)
> +{
> + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> + struct max96717_priv *priv = sd_to_max96717(sd);
> +
> + max96717_subdev_uninit(priv);
> + i2c_mux_del_adapters(priv->mux);
> +}
> +
> +static const struct of_device_id max96717_of_ids[] = {
> + { .compatible = "maxim,max96717f" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, max96717_of_ids);
> +
> +static struct i2c_driver max96717_i2c_driver = {
> + .driver = {
> + .name = "max96717",
> + .of_match_table = max96717_of_ids,
> + },
> + .probe = max96717_probe,
> + .remove = max96717_remove,
> +};
> +
> +module_i2c_driver(max96717_i2c_driver);
> +
> +MODULE_DESCRIPTION("Maxim GMSL2 MAX96717 Serializer Driver");
> +MODULE_AUTHOR("Julien Massot <julien.massot@collabora.com>");
> +MODULE_LICENSE("GPL");
--
Kind regards,
Sakari Ailus
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v7 3/5] media: i2c: add MAX96717 driver
2024-04-30 13:19 ` [PATCH v7 3/5] media: i2c: add MAX96717 driver Julien Massot
2024-05-31 13:56 ` Sakari Ailus
@ 2024-06-03 7:29 ` Tommaso Merciai
1 sibling, 0 replies; 14+ messages in thread
From: Tommaso Merciai @ 2024-06-03 7:29 UTC (permalink / raw)
To: Julien Massot
Cc: linux-media, sakari.ailus, devicetree, kernel, linux-kernel,
mchehab, robh+dt, krzysztof.kozlowski+dt, conor+dt
Hi Julien,
On Tue, Apr 30, 2024 at 03:19:29PM +0200, Julien Massot wrote:
> This driver handles the MAX96717 serializer in tunnel mode.
> All incoming CSI traffic will be tunneled through the GMSL2
> link.
>
> The MAX96717 driver can handle MAX96717 and MAX96717F variants
> with the same "maxim,max96717f" compatible.
>
> Signed-off-by: Julien Massot <julien.massot@collabora.com>
> ---
> Change since v6:
> - Kconfig select MEDIA_CONTROLLER, V4L2_FWNODE and VIDEO_V4L2_SUBDEV_API
> - rename 'REG3' register to 'MAX96717_REG3'
> - Initialized 'ret' variable in 'max96717_gpiochip_probe'
> - remove max96714_v4l2_notifier_unregister and call the function directly
> - Do not store private pointer with i2c_set_clientdata since the v4l2-i2c
> uses it to store the subdev pointer
> - use dev_err_probe at gpio chip initialization
>
> Change since v5:
> - set the driver compatible back to MAX96717F that can be used as a fallback for MAX96717
>
> Change since v4:
> - make the driver compatible with MAX96717 instead of MAX96717F
> - Add the device id for the MAX96717
> - remove hw_data structure for now, it can be usefull later for handling different serializers e.g max9295
>
> Change since v3:
> - Maintainers: align to the new binding path
> - Kconfig: better describe the symbol
> - store the v4l2_mbus_config_mipi_csi2 structure instead of the full endpoint in the driver private structure
> - use MAX96717_PAD_SINK/SOURCE instead of 0/1 for pad intialization
> - Removed incorrect call to fwnode_handle_put(priv->sd.fwnode)
> - Use unsigned int instead of u8
> - Allocate clk name out of the clk struct initialization
> - fixed multiline comment
> - Removed one unnecessary goto at the end of the probe function
>
> Change since v2:
> - Use CCI helpers instead of recoding register access
> - add missing bitfield header
> ---
> MAINTAINERS | 7 +
> drivers/media/i2c/Kconfig | 17 +
> drivers/media/i2c/Makefile | 1 +
> drivers/media/i2c/max96717.c | 928 +++++++++++++++++++++++++++++++++++
> 4 files changed, 953 insertions(+)
> create mode 100644 drivers/media/i2c/max96717.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index eea74166a2d9..cfaa904ace59 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -13426,6 +13426,13 @@ S: Maintained
> F: Documentation/devicetree/bindings/media/i2c/maxim,max96712.yaml
> F: drivers/staging/media/max96712/max96712.c
>
> +MAX96717 GMSL2 SERIALIZER DRIVER
> +M: Julien Massot <julien.massot@collabora.com>
> +L: linux-media@vger.kernel.org
> +S: Maintained
> +F: Documentation/devicetree/bindings/media/i2c/maxim,max96717.yaml
> +F: drivers/media/i2c/max96717.c
> +
> MAX9860 MONO AUDIO VOICE CODEC DRIVER
> M: Peter Rosin <peda@axentia.se>
> L: alsa-devel@alsa-project.org (moderated for non-subscribers)
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index c6d3ee472d81..9918195e09ba 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -1575,6 +1575,23 @@ config VIDEO_DS90UB960
> Device driver for the Texas Instruments DS90UB960
> FPD-Link III Deserializer and DS90UB9702 FPD-Link IV Deserializer.
>
> +config VIDEO_MAX96717
> + tristate "Maxim MAX96717 GMSL2 Serializer support"
> + depends on OF && I2C && VIDEO_DEV && COMMON_CLK
> + select I2C_MUX
> + select MEDIA_CONTROLLER
> + select GPIOLIB
> + select V4L2_CCI_I2C
> + select V4L2_FWNODE
> + select VIDEO_V4L2_SUBDEV_API
> + help
> + Device driver for the Maxim MAX96717 GMSL2 Serializer.
> + MAX96717 serializers convert video on a MIPI CSI-2
> + input to a GMSL2 output.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called max96717.
> +
> endmenu
>
> endif # VIDEO_DEV
> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> index dfbe6448b549..9e007116f929 100644
> --- a/drivers/media/i2c/Makefile
> +++ b/drivers/media/i2c/Makefile
> @@ -64,6 +64,7 @@ obj-$(CONFIG_VIDEO_LM3646) += lm3646.o
> obj-$(CONFIG_VIDEO_M52790) += m52790.o
> obj-$(CONFIG_VIDEO_MAX9271_LIB) += max9271.o
> obj-$(CONFIG_VIDEO_MAX9286) += max9286.o
> +obj-$(CONFIG_VIDEO_MAX96717) += max96717.o
> obj-$(CONFIG_VIDEO_ML86V7667) += ml86v7667.o
> obj-$(CONFIG_VIDEO_MSP3400) += msp3400.o
> obj-$(CONFIG_VIDEO_MT9M001) += mt9m001.o
> diff --git a/drivers/media/i2c/max96717.c b/drivers/media/i2c/max96717.c
> new file mode 100644
> index 000000000000..1ea76f922bdb
> --- /dev/null
> +++ b/drivers/media/i2c/max96717.c
> @@ -0,0 +1,928 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Maxim GMSL2 Serializer Driver
> + *
> + * Copyright (C) 2024 Collabora Ltd.
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>
> +#include <linux/delay.h>
> +#include <linux/fwnode.h>
> +#include <linux/gpio/driver.h>
> +#include <linux/i2c-mux.h>
> +#include <linux/i2c.h>
> +#include <linux/regmap.h>
> +
> +#include <media/v4l2-cci.h>
> +#include <media/v4l2-fwnode.h>
> +#include <media/v4l2-subdev.h>
> +
> +#define MAX96717_DEVICE_ID 0xbf
> +#define MAX96717F_DEVICE_ID 0xc8
> +#define MAX96717_PORTS 2
> +#define MAX96717_PAD_SINK 0
> +#define MAX96717_PAD_SOURCE 1
> +
> +#define MAX96717_DEFAULT_CLKOUT_RATE 24000000UL
> +
> +/* DEV */
> +#define MAX96717_REG3 CCI_REG8(0x3)
> +#define MAX96717_RCLKSEL GENMASK(1, 0)
> +#define RCLKSEL_REF_PLL CCI_REG8(0x3)
> +#define MAX96717_REG6 CCI_REG8(0x6)
> +#define RCLKEN BIT(5)
> +#define MAX96717_DEV_ID CCI_REG8(0xd)
> +#define MAX96717_DEV_REV CCI_REG8(0xe)
> +#define MAX96717_DEV_REV_MASK GENMASK(3, 0)
> +
> +/* VID_TX Z */
> +#define MAX96717_VIDEO_TX2 CCI_REG8(0x112)
> +#define MAX96717_VIDEO_PCLKDET BIT(7)
> +
> +/* GPIO */
> +#define MAX96717_NUM_GPIO 11
> +#define MAX96717_GPIO_REG_A(gpio) CCI_REG8(0x2be + (gpio) * 3)
> +#define MAX96717_GPIO_OUT BIT(4)
> +#define MAX96717_GPIO_IN BIT(3)
> +#define MAX96717_GPIO_RX_EN BIT(2)
> +#define MAX96717_GPIO_TX_EN BIT(1)
> +#define MAX96717_GPIO_OUT_DIS BIT(0)
> +
> +/* FRONTTOP */
> +/* MAX96717 only have CSI port 'B' */
> +#define MAX96717_FRONTOP0 CCI_REG8(0x308)
> +#define MAX96717_START_PORT_B BIT(5)
> +
> +/* MIPI_RX */
> +#define MAX96717_MIPI_RX1 CCI_REG8(0x331)
> +#define MAX96717_MIPI_LANES_CNT GENMASK(5, 4)
> +#define MAX96717_MIPI_RX2 CCI_REG8(0x332) /* phy1 Lanes map */
> +#define MAX96717_PHY2_LANES_MAP GENMASK(7, 4)
> +#define MAX96717_MIPI_RX3 CCI_REG8(0x333) /* phy2 Lanes map */
> +#define MAX96717_PHY1_LANES_MAP GENMASK(3, 0)
> +#define MAX96717_MIPI_RX4 CCI_REG8(0x334) /* phy1 lane polarities */
> +#define MAX96717_PHY1_LANES_POL GENMASK(6, 4)
> +#define MAX96717_MIPI_RX5 CCI_REG8(0x335) /* phy2 lane polarities */
> +#define MAX96717_PHY2_LANES_POL GENMASK(2, 0)
> +
> +/* MIPI_RX_EXT */
> +#define MAX96717_MIPI_RX_EXT11 CCI_REG8(0x383)
> +#define MAX96717_TUN_MODE BIT(7)
> +
> +/* REF_VTG */
> +#define REF_VTG0 CCI_REG8(0x3f0)
> +#define REFGEN_PREDEF_EN BIT(6)
> +#define REFGEN_PREDEF_FREQ_MASK GENMASK(5, 4)
> +#define REFGEN_PREDEF_FREQ_ALT BIT(3)
> +#define REFGEN_RST BIT(1)
> +#define REFGEN_EN BIT(0)
> +
> +/* MISC */
> +#define PIO_SLEW_1 CCI_REG8(0x570)
> +
> +struct max96717_priv {
> + struct i2c_client *client;
> + struct regmap *regmap;
> + struct i2c_mux_core *mux;
> + struct v4l2_mbus_config_mipi_csi2 mipi_csi2;
> + struct v4l2_subdev sd;
> + struct media_pad pads[MAX96717_PORTS];
> + struct v4l2_async_notifier notifier;
> + struct v4l2_subdev *source_sd;
> + u16 source_sd_pad;
> + u64 enabled_source_streams;
> + u8 pll_predef_index;
> + struct clk_hw clk_hw;
> + struct gpio_chip gpio_chip;
> +};
> +
> +static inline struct max96717_priv *sd_to_max96717(struct v4l2_subdev *sd)
> +{
> + return container_of(sd, struct max96717_priv, sd);
> +}
> +
> +static inline struct max96717_priv *clk_hw_to_max96717(struct clk_hw *hw)
> +{
> + return container_of(hw, struct max96717_priv, clk_hw);
> +}
> +
> +static int max96717_i2c_mux_select(struct i2c_mux_core *mux, u32 chan)
> +{
> + return 0;
> +}
> +
> +static int max96717_i2c_mux_init(struct max96717_priv *priv)
> +{
> + priv->mux = i2c_mux_alloc(priv->client->adapter, &priv->client->dev,
> + 1, 0, I2C_MUX_LOCKED | I2C_MUX_GATE,
> + max96717_i2c_mux_select, NULL);
> + if (!priv->mux)
> + return -ENOMEM;
> +
> + return i2c_mux_add_adapter(priv->mux, 0, 0, 0);
Rebasing the driver on top of linux 6.10.0-rc1 I'm getting the following
error:
error: too many arguments to function ‘i2c_mux_add_adapter’
Please fix that: i2c_mux_add_adapter(priv->mux, 0, 0);
Same for the max96714.c driver.
Thanks & Regards,
Tommaso
> +}
> +
> +static inline int max96717_start_csi(struct max96717_priv *priv, bool start)
> +{
> + return cci_update_bits(priv->regmap, MAX96717_FRONTOP0,
> + MAX96717_START_PORT_B,
> + start ? MAX96717_START_PORT_B : 0, NULL);
> +}
> +
> +static int max96717_gpiochip_get(struct gpio_chip *gpiochip,
> + unsigned int offset)
> +{
> + struct max96717_priv *priv = gpiochip_get_data(gpiochip);
> + u64 val;
> + int ret;
> +
> + ret = cci_read(priv->regmap, MAX96717_GPIO_REG_A(offset),
> + &val, NULL);
> + if (ret)
> + return ret;
> +
> + if (val & MAX96717_GPIO_OUT_DIS)
> + return !!(val & MAX96717_GPIO_IN);
> + else
> + return !!(val & MAX96717_GPIO_OUT);
> +}
> +
> +static void max96717_gpiochip_set(struct gpio_chip *gpiochip,
> + unsigned int offset, int value)
> +{
> + struct max96717_priv *priv = gpiochip_get_data(gpiochip);
> +
> + cci_update_bits(priv->regmap, MAX96717_GPIO_REG_A(offset),
> + MAX96717_GPIO_OUT, MAX96717_GPIO_OUT, NULL);
> +}
> +
> +static int max96717_gpio_get_direction(struct gpio_chip *gpiochip,
> + unsigned int offset)
> +{
> + struct max96717_priv *priv = gpiochip_get_data(gpiochip);
> + u64 val;
> + int ret;
> +
> + ret = cci_read(priv->regmap, MAX96717_GPIO_REG_A(offset), &val, NULL);
> + if (ret < 0)
> + return ret;
> +
> + return !!(val & MAX96717_GPIO_OUT_DIS);
> +}
> +
> +static int max96717_gpio_direction_out(struct gpio_chip *gpiochip,
> + unsigned int offset, int value)
> +{
> + struct max96717_priv *priv = gpiochip_get_data(gpiochip);
> +
> + return cci_update_bits(priv->regmap, MAX96717_GPIO_REG_A(offset),
> + MAX96717_GPIO_OUT_DIS | MAX96717_GPIO_OUT,
> + value ? MAX96717_GPIO_OUT : 0, NULL);
> +}
> +
> +static int max96717_gpio_direction_in(struct gpio_chip *gpiochip,
> + unsigned int offset)
> +{
> + struct max96717_priv *priv = gpiochip_get_data(gpiochip);
> +
> + return cci_update_bits(priv->regmap, MAX96717_GPIO_REG_A(offset),
> + MAX96717_GPIO_OUT_DIS, MAX96717_GPIO_OUT_DIS,
> + NULL);
> +}
> +
> +static int max96717_gpiochip_probe(struct max96717_priv *priv)
> +{
> + struct device *dev = &priv->client->dev;
> + struct gpio_chip *gc = &priv->gpio_chip;
> + int i, ret = 0;
> +
> + gc->label = dev_name(dev);
> + gc->parent = dev;
> + gc->owner = THIS_MODULE;
> + gc->ngpio = MAX96717_NUM_GPIO;
> + gc->base = -1;
> + gc->can_sleep = true;
> + gc->get_direction = max96717_gpio_get_direction;
> + gc->direction_input = max96717_gpio_direction_in;
> + gc->direction_output = max96717_gpio_direction_out;
> + gc->set = max96717_gpiochip_set;
> + gc->get = max96717_gpiochip_get;
> + gc->of_gpio_n_cells = 2;
> +
> + /* Disable GPIO forwarding */
> + for (i = 0; i < gc->ngpio; i++)
> + cci_update_bits(priv->regmap, MAX96717_GPIO_REG_A(i),
> + MAX96717_GPIO_RX_EN | MAX96717_GPIO_TX_EN,
> + 0, &ret);
> +
> + if (ret)
> + return ret;
> +
> + ret = devm_gpiochip_add_data(dev, gc, priv);
> + if (ret) {
> + dev_err(dev, "Unable to create gpio_chip\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int _max96717_set_routing(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state,
> + struct v4l2_subdev_krouting *routing)
> +{
> + static const struct v4l2_mbus_framefmt format = {
> + .width = 1280,
> + .height = 1080,
> + .code = MEDIA_BUS_FMT_Y8_1X8,
> + .field = V4L2_FIELD_NONE,
> + };
> + int ret;
> +
> + ret = v4l2_subdev_routing_validate(sd, routing,
> + V4L2_SUBDEV_ROUTING_ONLY_1_TO_1);
> + if (ret)
> + return ret;
> +
> + ret = v4l2_subdev_set_routing_with_fmt(sd, state, routing, &format);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> +static int max96717_set_routing(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state,
> + enum v4l2_subdev_format_whence which,
> + struct v4l2_subdev_krouting *routing)
> +{
> + struct max96717_priv *priv = sd_to_max96717(sd);
> +
> + if (which == V4L2_SUBDEV_FORMAT_ACTIVE && priv->enabled_source_streams)
> + return -EBUSY;
> +
> + return _max96717_set_routing(sd, state, routing);
> +}
> +
> +static int max96717_set_fmt(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state,
> + struct v4l2_subdev_format *format)
> +{
> + struct max96717_priv *priv = sd_to_max96717(sd);
> + struct v4l2_mbus_framefmt *fmt;
> + u64 stream_source_mask;
> +
> + if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE &&
> + priv->enabled_source_streams)
> + return -EBUSY;
> +
> + /* No transcoding, source and sink formats must match. */
> + if (format->pad == MAX96717_PAD_SOURCE)
> + return v4l2_subdev_get_fmt(sd, state, format);
> +
> + /* Set sink format */
> + fmt = v4l2_subdev_state_get_format(state, format->pad, format->stream);
> + if (!fmt)
> + return -EINVAL;
> +
> + *fmt = format->format;
> +
> + /* Propagate to source format */
> + fmt = v4l2_subdev_state_get_opposite_stream_format(state, format->pad,
> + format->stream);
> + if (!fmt)
> + return -EINVAL;
> + *fmt = format->format;
> +
> + stream_source_mask = BIT(format->stream);
> +
> + return v4l2_subdev_state_xlate_streams(state, MAX96717_PAD_SOURCE,
> + MAX96717_PAD_SINK,
> + &stream_source_mask);
> +}
> +
> +static int max96717_init_state(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state)
> +{
> + struct v4l2_subdev_route routes[] = {
> + {
> + .sink_pad = MAX96717_PAD_SINK,
> + .sink_stream = 0,
> + .source_pad = MAX96717_PAD_SOURCE,
> + .source_stream = 0,
> + .flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE,
> + },
> + };
> + struct v4l2_subdev_krouting routing = {
> + .num_routes = ARRAY_SIZE(routes),
> + .routes = routes,
> + };
> +
> + return _max96717_set_routing(sd, state, &routing);
> +}
> +
> +static bool max96717_pipe_pclkdet(struct max96717_priv *priv)
> +{
> + u64 val = 0;
> +
> + cci_read(priv->regmap, MAX96717_VIDEO_TX2, &val, NULL);
> +
> + return val & MAX96717_VIDEO_PCLKDET;
> +}
> +
> +static int max96717_log_status(struct v4l2_subdev *sd)
> +{
> + struct max96717_priv *priv = sd_to_max96717(sd);
> + struct device *dev = &priv->client->dev;
> +
> + dev_info(dev, "Serializer: max96717\n");
> + dev_info(dev, "Pipe: pclkdet:%d\n", max96717_pipe_pclkdet(priv));
> +
> + return 0;
> +}
> +
> +static int max96717_enable_streams(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state, u32 pad,
> + u64 streams_mask)
> +{
> + struct max96717_priv *priv = sd_to_max96717(sd);
> + struct device *dev = &priv->client->dev;
> + u64 sink_streams;
> + int ret;
> +
> + sink_streams = v4l2_subdev_state_xlate_streams(state,
> + MAX96717_PAD_SOURCE,
> + MAX96717_PAD_SINK,
> + &streams_mask);
> +
> + if (!priv->enabled_source_streams)
> + max96717_start_csi(priv, true);
> +
> + ret = v4l2_subdev_enable_streams(priv->source_sd, priv->source_sd_pad,
> + sink_streams);
> + if (ret) {
> + dev_err(dev, "Fail to start streams:%llu on remote subdev\n",
> + sink_streams);
> + goto stop_csi;
> + }
> +
> + priv->enabled_source_streams |= streams_mask;
> +
> + return 0;
> +
> +stop_csi:
> + if (!priv->enabled_source_streams)
> + max96717_start_csi(priv, false);
> + return ret;
> +}
> +
> +static int max96717_disable_streams(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state, u32 pad,
> + u64 streams_mask)
> +{
> + struct max96717_priv *priv = sd_to_max96717(sd);
> + u64 sink_streams;
> + int ret;
> +
> + sink_streams = v4l2_subdev_state_xlate_streams(state,
> + MAX96717_PAD_SOURCE,
> + MAX96717_PAD_SINK,
> + &streams_mask);
> +
> + ret = v4l2_subdev_disable_streams(priv->source_sd, priv->source_sd_pad,
> + sink_streams);
> + if (ret)
> + return ret;
> +
> + priv->enabled_source_streams &= ~streams_mask;
> +
> + if (!priv->enabled_source_streams)
> + max96717_start_csi(priv, false);
> +
> + return 0;
> +}
> +
> +static const struct v4l2_subdev_pad_ops max96717_pad_ops = {
> + .enable_streams = max96717_enable_streams,
> + .disable_streams = max96717_disable_streams,
> + .set_routing = max96717_set_routing,
> + .get_fmt = v4l2_subdev_get_fmt,
> + .set_fmt = max96717_set_fmt,
> +};
> +
> +static const struct v4l2_subdev_core_ops max96717_subdev_core_ops = {
> + .log_status = max96717_log_status,
> +};
> +
> +static const struct v4l2_subdev_internal_ops max96717_internal_ops = {
> + .init_state = max96717_init_state,
> +};
> +
> +static const struct v4l2_subdev_ops max96717_subdev_ops = {
> + .core = &max96717_subdev_core_ops,
> + .pad = &max96717_pad_ops,
> +};
> +
> +static const struct media_entity_operations max96717_entity_ops = {
> + .link_validate = v4l2_subdev_link_validate,
> +};
> +
> +static int max96717_notify_bound(struct v4l2_async_notifier *notifier,
> + struct v4l2_subdev *source_subdev,
> + struct v4l2_async_connection *asd)
> +{
> + struct max96717_priv *priv = sd_to_max96717(notifier->sd);
> + struct device *dev = &priv->client->dev;
> + int ret;
> +
> + ret = media_entity_get_fwnode_pad(&source_subdev->entity,
> + source_subdev->fwnode,
> + MEDIA_PAD_FL_SOURCE);
> + if (ret < 0) {
> + dev_err(dev, "Failed to find pad for %s\n",
> + source_subdev->name);
> + return ret;
> + }
> +
> + priv->source_sd = source_subdev;
> + priv->source_sd_pad = ret;
> +
> + ret = media_create_pad_link(&source_subdev->entity, priv->source_sd_pad,
> + &priv->sd.entity, 0,
> + MEDIA_LNK_FL_ENABLED |
> + MEDIA_LNK_FL_IMMUTABLE);
> + if (ret) {
> + dev_err(dev, "Unable to link %s:%u -> %s:0\n",
> + source_subdev->name, priv->source_sd_pad,
> + priv->sd.name);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static const struct v4l2_async_notifier_operations max96717_notify_ops = {
> + .bound = max96717_notify_bound,
> +};
> +
> +static int max96717_v4l2_notifier_register(struct max96717_priv *priv)
> +{
> + struct device *dev = &priv->client->dev;
> + struct v4l2_async_connection *asd;
> + struct fwnode_handle *ep_fwnode;
> + int ret;
> +
> + ep_fwnode = fwnode_graph_get_endpoint_by_id(dev_fwnode(dev),
> + MAX96717_PAD_SINK, 0, 0);
> + if (!ep_fwnode) {
> + dev_err(dev, "No graph endpoint\n");
> + return -ENODEV;
> + }
> +
> + v4l2_async_subdev_nf_init(&priv->notifier, &priv->sd);
> +
> + asd = v4l2_async_nf_add_fwnode_remote(&priv->notifier, ep_fwnode,
> + struct v4l2_async_connection);
> +
> + fwnode_handle_put(ep_fwnode);
> +
> + if (IS_ERR(asd)) {
> + dev_err(dev, "Failed to add subdev: %ld", PTR_ERR(asd));
> + v4l2_async_nf_cleanup(&priv->notifier);
> + return PTR_ERR(asd);
> + }
> +
> + priv->notifier.ops = &max96717_notify_ops;
> +
> + ret = v4l2_async_nf_register(&priv->notifier);
> + if (ret) {
> + dev_err(dev, "Failed to register subdev_notifier");
> + v4l2_async_nf_cleanup(&priv->notifier);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int max96717_subdev_init(struct max96717_priv *priv)
> +{
> + struct device *dev = &priv->client->dev;
> + int ret;
> +
> + v4l2_i2c_subdev_init(&priv->sd, priv->client, &max96717_subdev_ops);
> + priv->sd.internal_ops = &max96717_internal_ops;
> +
> + priv->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_STREAMS;
> + priv->sd.entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
> + priv->sd.entity.ops = &max96717_entity_ops;
> +
> + priv->pads[MAX96717_PAD_SINK].flags = MEDIA_PAD_FL_SINK;
> + priv->pads[MAX96717_PAD_SOURCE].flags = MEDIA_PAD_FL_SOURCE;
> +
> + ret = media_entity_pads_init(&priv->sd.entity, 2, priv->pads);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to init pads\n");
> +
> + ret = v4l2_subdev_init_finalize(&priv->sd);
> + if (ret) {
> + dev_err_probe(dev, ret,
> + "v4l2 subdev init finalized failed\n");
> + goto err_entity_cleanup;
> + }
> + ret = max96717_v4l2_notifier_register(priv);
> + if (ret) {
> + dev_err_probe(dev, ret,
> + "v4l2 subdev notifier register failed\n");
> + goto err_free_state;
> + }
> +
> + ret = v4l2_async_register_subdev(&priv->sd);
> + if (ret) {
> + dev_err_probe(dev, ret, "v4l2_async_register_subdev error\n");
> + goto err_unreg_notif;
> + }
> +
> + return 0;
> +
> +err_unreg_notif:
> + v4l2_async_nf_unregister(&priv->notifier);
> + v4l2_async_nf_cleanup(&priv->notifier);
> +err_free_state:
> + v4l2_subdev_cleanup(&priv->sd);
> +err_entity_cleanup:
> + media_entity_cleanup(&priv->sd.entity);
> +
> + return ret;
> +}
> +
> +static void max96717_subdev_uninit(struct max96717_priv *priv)
> +{
> + v4l2_async_unregister_subdev(&priv->sd);
> + v4l2_async_nf_unregister(&priv->notifier);
> + v4l2_async_nf_cleanup(&priv->notifier);
> + v4l2_subdev_cleanup(&priv->sd);
> + media_entity_cleanup(&priv->sd.entity);
> +}
> +
> +struct max96717_pll_predef_freq {
> + unsigned long freq;
> + bool is_alt;
> + u8 val;
> +};
> +
> +static const struct max96717_pll_predef_freq max96717_predef_freqs[] = {
> + { 13500000, true, 0 }, { 19200000, false, 0 },
> + { 24000000, true, 1 }, { 27000000, false, 1 },
> + { 37125000, false, 2 }, { 74250000, false, 3 },
> +};
> +
> +static unsigned long
> +max96717_clk_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
> +{
> + struct max96717_priv *priv = clk_hw_to_max96717(hw);
> +
> + return max96717_predef_freqs[priv->pll_predef_index].freq;
> +}
> +
> +static unsigned int max96717_clk_find_best_index(struct max96717_priv *priv,
> + unsigned long rate)
> +{
> + unsigned int i, idx;
> + unsigned long diff_new, diff_old;
> +
> + diff_old = U32_MAX;
> + idx = 0;
> +
> + for (i = 0; i < ARRAY_SIZE(max96717_predef_freqs); i++) {
> + diff_new = abs(rate - max96717_predef_freqs[i].freq);
> + if (diff_new < diff_old) {
> + diff_old = diff_new;
> + idx = i;
> + }
> + }
> +
> + return idx;
> +}
> +
> +static long max96717_clk_round_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long *parent_rate)
> +{
> + struct max96717_priv *priv = clk_hw_to_max96717(hw);
> + struct device *dev = &priv->client->dev;
> + unsigned int idx;
> +
> + idx = max96717_clk_find_best_index(priv, rate);
> +
> + if (rate != max96717_predef_freqs[idx].freq) {
> + dev_warn(dev, "Request CLK freq:%lu, found CLK freq:%lu\n",
> + rate, max96717_predef_freqs[idx].freq);
> + }
> +
> + return max96717_predef_freqs[idx].freq;
> +}
> +
> +static int max96717_clk_set_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long parent_rate)
> +{
> + struct max96717_priv *priv = clk_hw_to_max96717(hw);
> + unsigned int val, idx;
> + int ret = 0;
> +
> + idx = max96717_clk_find_best_index(priv, rate);
> +
> + val = FIELD_PREP(REFGEN_PREDEF_FREQ_MASK,
> + max96717_predef_freqs[idx].val);
> +
> + if (max96717_predef_freqs[idx].is_alt)
> + val |= REFGEN_PREDEF_FREQ_ALT;
> +
> + val |= REFGEN_RST | REFGEN_PREDEF_EN;
> +
> + cci_write(priv->regmap, REF_VTG0, val, &ret);
> + cci_update_bits(priv->regmap, REF_VTG0, REFGEN_RST | REFGEN_EN,
> + REFGEN_EN, &ret);
> + if (ret)
> + return ret;
> +
> + priv->pll_predef_index = idx;
> +
> + return 0;
> +}
> +
> +static int max96717_clk_prepare(struct clk_hw *hw)
> +{
> + struct max96717_priv *priv = clk_hw_to_max96717(hw);
> +
> + return cci_update_bits(priv->regmap, MAX96717_REG6, RCLKEN,
> + RCLKEN, NULL);
> +}
> +
> +static void max96717_clk_unprepare(struct clk_hw *hw)
> +{
> + struct max96717_priv *priv = clk_hw_to_max96717(hw);
> +
> + cci_update_bits(priv->regmap, MAX96717_REG6, RCLKEN, 0, NULL);
> +}
> +
> +static const struct clk_ops max96717_clk_ops = {
> + .prepare = max96717_clk_prepare,
> + .unprepare = max96717_clk_unprepare,
> + .set_rate = max96717_clk_set_rate,
> + .recalc_rate = max96717_clk_recalc_rate,
> + .round_rate = max96717_clk_round_rate,
> +};
> +
> +static int max96717_register_clkout(struct max96717_priv *priv)
> +{
> + struct device *dev = &priv->client->dev;
> + struct clk_init_data init = { .ops = &max96717_clk_ops };
> + int ret;
> +
> + init.name = kasprintf(GFP_KERNEL, "max96717.%s.clk_out",
> + dev_name(dev));
> + if (!init.name)
> + return -ENOMEM;
> +
> + /* RCLKSEL Reference PLL output */
> + ret = cci_update_bits(priv->regmap, MAX96717_REG3, MAX96717_RCLKSEL,
> + MAX96717_RCLKSEL, NULL);
> + /* MFP4 fastest slew rate */
> + cci_update_bits(priv->regmap, PIO_SLEW_1, BIT(5) | BIT(4), 0, &ret);
> + if (ret)
> + goto free_init_name;
> +
> + priv->clk_hw.init = &init;
> +
> + /* Initialize to 24 MHz */
> + ret = max96717_clk_set_rate(&priv->clk_hw,
> + MAX96717_DEFAULT_CLKOUT_RATE, 0);
> + if (ret < 0)
> + goto free_init_name;
> +
> + ret = devm_clk_hw_register(dev, &priv->clk_hw);
> + kfree(init.name);
> + if (ret)
> + return dev_err_probe(dev, ret, "Cannot register clock HW\n");
> +
> + ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get,
> + &priv->clk_hw);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "Cannot add OF clock provider\n");
> +
> + return 0;
> +
> +free_init_name:
> + kfree(init.name);
> + return ret;
> +}
> +
> +static int max96717_init_csi_lanes(struct max96717_priv *priv)
> +{
> + struct v4l2_mbus_config_mipi_csi2 *mipi = &priv->mipi_csi2;
> + unsigned long lanes_used = 0;
> + unsigned int nlanes, lane, val = 0;
> + int ret;
> +
> + nlanes = mipi->num_data_lanes;
> +
> + ret = cci_update_bits(priv->regmap, MAX96717_MIPI_RX1,
> + MAX96717_MIPI_LANES_CNT,
> + FIELD_PREP(MAX96717_MIPI_LANES_CNT,
> + nlanes - 1), NULL);
> +
> + /* lanes polarity */
> + for (lane = 0; lane < nlanes + 1; lane++) {
> + if (!mipi->lane_polarities[lane])
> + continue;
> + /* Clock lane */
> + if (lane == 0)
> + val |= BIT(2);
> + else if (lane < 3)
> + val |= BIT(lane - 1);
> + else
> + val |= BIT(lane);
> + }
> +
> + cci_update_bits(priv->regmap, MAX96717_MIPI_RX5,
> + MAX96717_PHY2_LANES_POL,
> + FIELD_PREP(MAX96717_PHY2_LANES_POL, val), &ret);
> +
> + cci_update_bits(priv->regmap, MAX96717_MIPI_RX4,
> + MAX96717_PHY1_LANES_POL,
> + FIELD_PREP(MAX96717_PHY1_LANES_POL,
> + val >> 3), &ret);
> + /* lanes mapping */
> + for (lane = 0, val = 0; lane < nlanes; lane++) {
> + val |= (mipi->data_lanes[lane] - 1) << (lane * 2);
> + lanes_used |= BIT(mipi->data_lanes[lane] - 1);
> + }
> +
> + /*
> + * Unused lanes need to be mapped as well to not have
> + * the same lanes mapped twice.
> + */
> + for (; lane < 4; lane++) {
> + unsigned int idx = find_first_zero_bit(&lanes_used, 4);
> +
> + val |= idx << (lane * 2);
> + lanes_used |= BIT(idx);
> + }
> +
> + cci_update_bits(priv->regmap, MAX96717_MIPI_RX3,
> + MAX96717_PHY1_LANES_MAP,
> + FIELD_PREP(MAX96717_PHY1_LANES_MAP, val), &ret);
> +
> + return cci_update_bits(priv->regmap, MAX96717_MIPI_RX2,
> + MAX96717_PHY2_LANES_MAP,
> + FIELD_PREP(MAX96717_PHY2_LANES_MAP, val >> 4),
> + &ret);
> +}
> +
> +static int max96717_hw_init(struct max96717_priv *priv)
> +{
> + struct device *dev = &priv->client->dev;
> + u64 dev_id, val;
> + int ret;
> +
> + ret = cci_read(priv->regmap, MAX96717_DEV_ID, &dev_id, NULL);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "Fail to read the device id\n");
> +
> + if (dev_id != MAX96717_DEVICE_ID && dev_id != MAX96717F_DEVICE_ID)
> + return dev_err_probe(dev, -EOPNOTSUPP,
> + "Unsupported device id got %x\n", (u8)dev_id);
> +
> + ret = cci_read(priv->regmap, MAX96717_DEV_REV, &val, NULL);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "Fail to read device revision");
> +
> + dev_dbg(dev, "Found %x (rev %lx)\n", (u8)dev_id,
> + (u8)val & MAX96717_DEV_REV_MASK);
> +
> + ret = cci_read(priv->regmap, MAX96717_MIPI_RX_EXT11, &val, NULL);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "Fail to read mipi rx extension");
> +
> + if (!(val & MAX96717_TUN_MODE))
> + return dev_err_probe(dev, -EOPNOTSUPP,
> + "Only supporting tunnel mode");
> +
> + return max96717_init_csi_lanes(priv);
> +}
> +
> +static int max96717_parse_dt(struct max96717_priv *priv)
> +{
> + struct device *dev = &priv->client->dev;
> + struct v4l2_fwnode_endpoint vep = {
> + .bus_type = V4L2_MBUS_CSI2_DPHY
> + };
> + struct fwnode_handle *ep_fwnode;
> + unsigned char num_data_lanes;
> + int ret;
> +
> + ep_fwnode = fwnode_graph_get_endpoint_by_id(dev_fwnode(dev),
> + MAX96717_PAD_SINK, 0, 0);
> + if (!ep_fwnode)
> + return dev_err_probe(dev, -ENOENT, "no endpoint found\n");
> +
> + ret = v4l2_fwnode_endpoint_parse(ep_fwnode, &vep);
> +
> + fwnode_handle_put(ep_fwnode);
> +
> + if (ret < 0)
> + return dev_err_probe(dev, ret, "Failed to parse sink endpoint");
> +
> + num_data_lanes = vep.bus.mipi_csi2.num_data_lanes;
> + if (num_data_lanes < 1 || num_data_lanes > 4)
> + return dev_err_probe(dev, -EINVAL,
> + "Invalid data lanes must be 1 to 4\n");
> +
> + memcpy(&priv->mipi_csi2, &vep.bus.mipi_csi2, sizeof(priv->mipi_csi2));
> +
> + return 0;
> +}
> +
> +static int max96717_probe(struct i2c_client *client)
> +{
> + struct device *dev = &client->dev;
> + struct max96717_priv *priv;
> + int ret;
> +
> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + priv->client = client;
> + priv->regmap = devm_cci_regmap_init_i2c(client, 16);
> + if (IS_ERR(priv->regmap)) {
> + ret = PTR_ERR(priv->regmap);
> + return dev_err_probe(dev, ret, "Failed to init regmap\n");
> + }
> +
> + ret = max96717_parse_dt(priv);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to parse the dt\n");
> +
> + ret = max96717_hw_init(priv);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "Failed to initialize the hardware\n");
> +
> + ret = max96717_gpiochip_probe(priv);
> + if (ret)
> + return dev_err_probe(&client->dev, ret,
> + "Failed to init gpiochip\n");
> +
> + ret = max96717_register_clkout(priv);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to register clkout\n");
> +
> + ret = max96717_subdev_init(priv);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "Failed to initialize v4l2 subdev\n");
> +
> + ret = max96717_i2c_mux_init(priv);
> + if (ret) {
> + dev_err_probe(dev, ret, "failed to add remote i2c adapter\n");
> + max96717_subdev_uninit(priv);
> + }
> +
> + return ret;
> +}
> +
> +static void max96717_remove(struct i2c_client *client)
> +{
> + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> + struct max96717_priv *priv = sd_to_max96717(sd);
> +
> + max96717_subdev_uninit(priv);
> + i2c_mux_del_adapters(priv->mux);
> +}
> +
> +static const struct of_device_id max96717_of_ids[] = {
> + { .compatible = "maxim,max96717f" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, max96717_of_ids);
> +
> +static struct i2c_driver max96717_i2c_driver = {
> + .driver = {
> + .name = "max96717",
> + .of_match_table = max96717_of_ids,
> + },
> + .probe = max96717_probe,
> + .remove = max96717_remove,
> +};
> +
> +module_i2c_driver(max96717_i2c_driver);
> +
> +MODULE_DESCRIPTION("Maxim GMSL2 MAX96717 Serializer Driver");
> +MODULE_AUTHOR("Julien Massot <julien.massot@collabora.com>");
> +MODULE_LICENSE("GPL");
> --
> 2.44.0
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v7 4/5] media: i2c: add MAX96714 driver
2024-04-30 13:19 [PATCH v7 0/5] Add support for MAX96714/F and MAX96717/F GMSL2 ser/des Julien Massot
` (2 preceding siblings ...)
2024-04-30 13:19 ` [PATCH v7 3/5] media: i2c: add MAX96717 driver Julien Massot
@ 2024-04-30 13:19 ` Julien Massot
2024-04-30 13:19 ` [PATCH v7 5/5] drivers: media: max96717: stop the csi receiver before the source Julien Massot
` (2 subsequent siblings)
6 siblings, 0 replies; 14+ messages in thread
From: Julien Massot @ 2024-04-30 13:19 UTC (permalink / raw)
To: linux-media, sakari.ailus
Cc: devicetree, kernel, linux-kernel, mchehab, robh+dt,
krzysztof.kozlowski+dt, conor+dt, Julien Massot
This driver handles the MAX96714 deserializer in tunnel mode.
The CSI output will replicate all the CSI traffic forwarded by
the remote serializer.
The MAX96714 driver can handle MAX96714 and MAX96714F variants
with the same "maxim,max96714f" compatible.
Signed-off-by: Julien Massot <julien.massot@collabora.com>
---
Change since v6:
- Kconfig select MEDIA_CONTROLLER, V4L2_FWNODE and VIDEO_V4L2_SUBDEV_API
- Make sure patgen is not enabled/disabled while streaming
- Remove the null check for the source subdev in disable_steams callback
- some minor cleanup, remove extra semicolon blank line, reduce variable scope,
fix Multi-line comment
- remove max96714_v4l2_notifier_unregister and call the function directly
- remove v4l2_set_clientdata, same as max96717
Change since v5:
- set the driver compatible back to MAX96714F that can be used as a fallback for MAX96714
Change since v4:
- make the driver compatible with MAX96714 instead of MAX96714F
- Add the device id for the MAX96714
Change since v3:
- Maintainers: align to the new binding path
- Kconfig: better describe the symbol
- Aligned the macro
- Store the v4l2_mbus_config_mipi_csi2 structure instead of the full endpoint in the driver private structure
- moved ret variables declaration at last
- Removed extra new lines
- Return the v4l2_subdev_set_routing_with_fmt result in _max96714_set_routing
- Use v4l2_subdev_s_stream_helper instead of the custom max96714_s_stream function
- Removed unnecessary check 'if (priv->tx_link_freq < 0)' in create_subdev since dt is already validated in max96714_parse_dt_txport
- Use div_u64 to fix compilation on 32 bits platforms
- Specify D-PHY for fwnode endpoint parsing since the MAX96714 only supports D-PHY
- Simplify parse_dt function by parsing first the tx_port so that we no longer have to call fwnode_handle_put(priv->rxport.source.ep_fwnode);
- Do not initialize regmap twice
- Use unsigned int instead of u8
Change since v2:
- Use CCI helpers instead of recoding register access
- add missing bitfield header
- Add pattern generator so the deserializer can be tested without a serializer/sensor
---
MAINTAINERS | 7 +
drivers/media/i2c/Kconfig | 17 +
drivers/media/i2c/Makefile | 1 +
drivers/media/i2c/max96714.c | 1024 ++++++++++++++++++++++++++++++++++
4 files changed, 1049 insertions(+)
create mode 100644 drivers/media/i2c/max96714.c
diff --git a/MAINTAINERS b/MAINTAINERS
index cfaa904ace59..fddd1a29589a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13426,6 +13426,13 @@ S: Maintained
F: Documentation/devicetree/bindings/media/i2c/maxim,max96712.yaml
F: drivers/staging/media/max96712/max96712.c
+MAX96714 GMSL2 DESERIALIZER DRIVER
+M: Julien Massot <julien.massot@collabora.com>
+L: linux-media@vger.kernel.org
+S: Maintained
+F: Documentation/devicetree/bindings/media/i2c/maxim,max96714.yaml
+F: drivers/media/i2c/max96714.c
+
MAX96717 GMSL2 SERIALIZER DRIVER
M: Julien Massot <julien.massot@collabora.com>
L: linux-media@vger.kernel.org
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 9918195e09ba..f3ab7330d5ec 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -1575,6 +1575,23 @@ config VIDEO_DS90UB960
Device driver for the Texas Instruments DS90UB960
FPD-Link III Deserializer and DS90UB9702 FPD-Link IV Deserializer.
+config VIDEO_MAX96714
+ tristate "Maxim MAX96714 GMSL2 deserializer"
+ depends on OF && I2C && VIDEO_DEV
+ select I2C_MUX
+ select MEDIA_CONTROLLER
+ select GPIOLIB
+ select V4L2_CCI_I2C
+ select V4L2_FWNODE
+ select VIDEO_V4L2_SUBDEV_API
+ help
+ Device driver for the Maxim MAX96714 GMSL2 Deserializer.
+ MAX96714 deserializers convert a GMSL2 input to MIPI CSI-2
+ output.
+
+ To compile this driver as a module, choose M here: the
+ module will be called max96714.
+
config VIDEO_MAX96717
tristate "Maxim MAX96717 GMSL2 Serializer support"
depends on OF && I2C && VIDEO_DEV && COMMON_CLK
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index 9e007116f929..7c794441eaff 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -64,6 +64,7 @@ obj-$(CONFIG_VIDEO_LM3646) += lm3646.o
obj-$(CONFIG_VIDEO_M52790) += m52790.o
obj-$(CONFIG_VIDEO_MAX9271_LIB) += max9271.o
obj-$(CONFIG_VIDEO_MAX9286) += max9286.o
+obj-$(CONFIG_VIDEO_MAX96714) += max96714.o
obj-$(CONFIG_VIDEO_MAX96717) += max96717.o
obj-$(CONFIG_VIDEO_ML86V7667) += ml86v7667.o
obj-$(CONFIG_VIDEO_MSP3400) += msp3400.o
diff --git a/drivers/media/i2c/max96714.c b/drivers/media/i2c/max96714.c
new file mode 100644
index 000000000000..220d3923e552
--- /dev/null
+++ b/drivers/media/i2c/max96714.c
@@ -0,0 +1,1024 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Maxim GMSL2 Deserializer Driver
+ *
+ * Copyright (C) 2024 Collabora Ltd.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bitops.h>
+#include <linux/fwnode.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/i2c-mux.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+
+#include <media/v4l2-cci.h>
+#include <media/v4l2-ctrls.h>
+#include <media/v4l2-fwnode.h>
+#include <media/v4l2-subdev.h>
+
+#define MAX96714_DEVICE_ID 0xc9
+#define MAX96714F_DEVICE_ID 0xca
+#define MAX96714_NPORTS 2
+#define MAX96714_PAD_SINK 0
+#define MAX96714_PAD_SOURCE 1
+
+/* DEV */
+#define MAX96714_REG13 CCI_REG8(0x0d)
+#define MAX96714_DEV_REV CCI_REG8(0x0e)
+#define MAX96714_DEV_REV_MASK GENMASK(3, 0)
+#define MAX96714_LINK_LOCK CCI_REG8(0x13)
+#define MAX96714_LINK_LOCK_BIT BIT(3)
+#define MAX96714_IO_CHK0 CCI_REG8(0x38)
+#define MAX96714_PATTERN_CLK_FREQ GENMASK(1, 0)
+/* VID_RX */
+#define MAX96714_VIDEO_RX8 CCI_REG8(0x11a)
+#define MAX96714_VID_LOCK BIT(6)
+
+/* VRX_PATGEN_0 */
+#define MAX96714_PATGEN_0 CCI_REG8(0x240)
+#define MAX96714_PATGEN_1 CCI_REG8(0x241)
+#define MAX96714_PATGEN_MODE GENMASK(5, 4)
+#define MAX96714_PATGEN_VS_DLY CCI_REG24(0x242)
+#define MAX96714_PATGEN_VS_HIGH CCI_REG24(0x245)
+#define MAX96714_PATGEN_VS_LOW CCI_REG24(0x248)
+#define MAX96714_PATGEN_V2H CCI_REG24(0x24b)
+#define MAX96714_PATGEN_HS_HIGH CCI_REG16(0x24e)
+#define MAX96714_PATGEN_HS_LOW CCI_REG16(0x250)
+#define MAX96714_PATGEN_HS_CNT CCI_REG16(0x252)
+#define MAX96714_PATGEN_V2D CCI_REG24(0x254)
+#define MAX96714_PATGEN_DE_HIGH CCI_REG16(0x257)
+#define MAX96714_PATGEN_DE_LOW CCI_REG16(0x259)
+#define MAX96714_PATGEN_DE_CNT CCI_REG16(0x25B)
+#define MAX96714_PATGEN_GRAD_INC CCI_REG8(0x25d)
+#define MAX96714_PATGEN_CHKB_COLOR_A CCI_REG24(0x25E)
+#define MAX96714_PATGEN_CHKB_COLOR_B CCI_REG24(0x261)
+#define MAX96714_PATGEN_CHKB_RPT_CNT_A CCI_REG8(0x264)
+#define MAX96714_PATGEN_CHKB_RPT_CNT_B CCI_REG8(0x265)
+#define MAX96714_PATGEN_CHKB_ALT CCI_REG8(0x266)
+/* BACKTOP */
+#define MAX96714_BACKTOP25 CCI_REG8(0x320)
+#define CSI_DPLL_FREQ_MASK GENMASK(4, 0)
+
+/* MIPI_PHY */
+#define MAX96714_MIPI_PHY0 CCI_REG8(0x330)
+#define MAX96714_FORCE_CSI_OUT BIT(7)
+#define MAX96714_MIPI_STDBY_N CCI_REG8(0x332)
+#define MAX96714_MIPI_STDBY_MASK GENMASK(5, 4)
+#define MAX96714_MIPI_LANE_MAP CCI_REG8(0x333)
+#define MAX96714_MIPI_POLARITY CCI_REG8(0x335)
+#define MAX96714_MIPI_POLARITY_MASK GENMASK(5, 0)
+
+/* MIPI_TX */
+#define MAX96714_MIPI_LANE_CNT CCI_REG8(0x44a)
+#define MAX96714_CSI2_LANE_CNT_MASK GENMASK(7, 6)
+#define MAX96714_MIPI_TX52 CCI_REG8(0x474)
+#define MAX96714_TUN_EN BIT(0)
+
+#define MHZ(v) ((u32)((v) * 1000000U))
+
+enum max96714_vpg_mode {
+ MAX96714_VPG_DISABLED = 0,
+ MAX96714_VPG_CHECKERBOARD = 1,
+ MAX96714_VPG_GRADIENT = 2,
+};
+
+struct max96714_rxport {
+ struct {
+ struct v4l2_subdev *sd;
+ u16 pad;
+ struct fwnode_handle *ep_fwnode;
+ } source;
+ struct regulator *poc;
+};
+
+struct max96714_txport {
+ struct v4l2_fwnode_endpoint vep;
+};
+
+struct max96714_priv {
+ struct i2c_client *client;
+ struct regmap *regmap;
+ struct gpio_desc *pd_gpio;
+ struct max96714_rxport rxport;
+ struct i2c_mux_core *mux;
+ u64 enabled_source_streams;
+ struct v4l2_subdev sd;
+ struct media_pad pads[MAX96714_NPORTS];
+ struct v4l2_mbus_config_mipi_csi2 mipi_csi2;
+ struct v4l2_ctrl_handler ctrl_handler;
+ struct v4l2_async_notifier notifier;
+ s64 tx_link_freq;
+ enum max96714_vpg_mode pattern;
+};
+
+static inline struct max96714_priv *sd_to_max96714(struct v4l2_subdev *sd)
+{
+ return container_of(sd, struct max96714_priv, sd);
+}
+
+static int max96714_enable_tx_port(struct max96714_priv *priv)
+{
+ return cci_update_bits(priv->regmap, MAX96714_MIPI_STDBY_N,
+ MAX96714_MIPI_STDBY_MASK,
+ MAX96714_MIPI_STDBY_MASK, NULL);
+}
+
+static int max96714_disable_tx_port(struct max96714_priv *priv)
+{
+ return cci_update_bits(priv->regmap, MAX96714_MIPI_STDBY_N,
+ MAX96714_MIPI_STDBY_MASK, 0, NULL);
+}
+
+static bool max96714_tx_port_enabled(struct max96714_priv *priv)
+{
+ u64 val;
+
+ cci_read(priv->regmap, MAX96714_MIPI_STDBY_N, &val, NULL);
+
+ return val & MAX96714_MIPI_STDBY_MASK;
+}
+
+static int max96714_apply_patgen_timing(struct max96714_priv *priv,
+ struct v4l2_subdev_state *state)
+{
+ struct v4l2_mbus_framefmt *fmt =
+ v4l2_subdev_state_get_format(state, MAX96714_PAD_SOURCE);
+ const u32 h_active = fmt->width;
+ const u32 h_fp = 88;
+ const u32 h_sw = 44;
+ const u32 h_bp = 148;
+ u32 h_tot;
+ const u32 v_active = fmt->height;
+ const u32 v_fp = 4;
+ const u32 v_sw = 5;
+ const u32 v_bp = 36;
+ u32 v_tot;
+ int ret = 0;
+
+ h_tot = h_active + h_fp + h_sw + h_bp;
+ v_tot = v_active + v_fp + v_sw + v_bp;
+
+ /* 75 Mhz pixel clock */
+ cci_update_bits(priv->regmap, MAX96714_IO_CHK0,
+ MAX96714_PATTERN_CLK_FREQ, 1, &ret);
+
+ dev_info(&priv->client->dev, "height: %d width: %d\n", fmt->height,
+ fmt->width);
+
+ cci_write(priv->regmap, MAX96714_PATGEN_VS_DLY, 0, &ret);
+ cci_write(priv->regmap, MAX96714_PATGEN_VS_HIGH, v_sw * h_tot, &ret);
+ cci_write(priv->regmap, MAX96714_PATGEN_VS_LOW,
+ (v_active + v_fp + v_bp) * h_tot, &ret);
+ cci_write(priv->regmap, MAX96714_PATGEN_HS_HIGH, h_sw, &ret);
+ cci_write(priv->regmap, MAX96714_PATGEN_HS_LOW, h_active + h_fp + h_bp,
+ &ret);
+ cci_write(priv->regmap, MAX96714_PATGEN_V2D,
+ h_tot * (v_sw + v_bp) + (h_sw + h_bp), &ret);
+ cci_write(priv->regmap, MAX96714_PATGEN_HS_CNT, v_tot, &ret);
+ cci_write(priv->regmap, MAX96714_PATGEN_DE_HIGH, h_active, &ret);
+ cci_write(priv->regmap, MAX96714_PATGEN_DE_LOW, h_fp + h_sw + h_bp,
+ &ret);
+ cci_write(priv->regmap, MAX96714_PATGEN_DE_CNT, v_active, &ret);
+ /* B G R */
+ cci_write(priv->regmap, MAX96714_PATGEN_CHKB_COLOR_A, 0xfecc00, &ret);
+ /* B G R */
+ cci_write(priv->regmap, MAX96714_PATGEN_CHKB_COLOR_B, 0x006aa7, &ret);
+ cci_write(priv->regmap, MAX96714_PATGEN_CHKB_RPT_CNT_A, 0x3c, &ret);
+ cci_write(priv->regmap, MAX96714_PATGEN_CHKB_RPT_CNT_B, 0x3c, &ret);
+ cci_write(priv->regmap, MAX96714_PATGEN_CHKB_ALT, 0x3c, &ret);
+ cci_write(priv->regmap, MAX96714_PATGEN_GRAD_INC, 0x10, &ret);
+
+ return ret;
+}
+
+static int max96714_apply_patgen(struct max96714_priv *priv,
+ struct v4l2_subdev_state *state)
+{
+ unsigned int val;
+ int ret = 0;
+
+ if (priv->pattern)
+ ret = max96714_apply_patgen_timing(priv, state);
+
+ cci_write(priv->regmap, MAX96714_PATGEN_0, priv->pattern ? 0xfb : 0,
+ &ret);
+
+ val = FIELD_PREP(MAX96714_PATGEN_MODE, priv->pattern);
+ cci_update_bits(priv->regmap, MAX96714_PATGEN_1, MAX96714_PATGEN_MODE,
+ val, &ret);
+ return ret;
+}
+
+static int max96714_s_ctrl(struct v4l2_ctrl *ctrl)
+{
+ struct max96714_priv *priv =
+ container_of(ctrl->handler, struct max96714_priv, ctrl_handler);
+ int ret;
+
+ switch (ctrl->id) {
+ case V4L2_CID_TEST_PATTERN:
+ if (priv->enabled_source_streams)
+ return -EBUSY;
+ priv->pattern = ctrl->val;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ ret = cci_update_bits(priv->regmap, MAX96714_MIPI_PHY0,
+ MAX96714_FORCE_CSI_OUT,
+ priv->pattern ? MAX96714_FORCE_CSI_OUT : 0, NULL);
+
+ /* Pattern generator doesn't work with tunnel mode */
+ return cci_update_bits(priv->regmap, MAX96714_MIPI_TX52,
+ MAX96714_TUN_EN,
+ priv->pattern ? 0 : MAX96714_TUN_EN, &ret);
+}
+
+static const char * const max96714_test_pattern[] = {
+ "Disabled",
+ "Checkerboard",
+ "Gradient"
+};
+
+static const struct v4l2_ctrl_ops max96714_ctrl_ops = {
+ .s_ctrl = max96714_s_ctrl,
+};
+
+static int max96714_enable_streams(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *state,
+ u32 source_pad, u64 streams_mask)
+{
+ struct max96714_priv *priv = sd_to_max96714(sd);
+ u64 sink_streams;
+ int ret;
+
+ if (!priv->enabled_source_streams)
+ max96714_enable_tx_port(priv);
+
+ ret = max96714_apply_patgen(priv, state);
+ if (ret)
+ goto err;
+
+ if (!priv->pattern) {
+ if (!priv->rxport.source.sd) {
+ ret = -ENODEV;
+ goto err;
+ }
+
+ sink_streams =
+ v4l2_subdev_state_xlate_streams(state,
+ MAX96714_PAD_SOURCE,
+ MAX96714_PAD_SINK,
+ &streams_mask);
+
+ ret = v4l2_subdev_enable_streams(priv->rxport.source.sd,
+ priv->rxport.source.pad,
+ sink_streams);
+ if (ret)
+ goto err;
+ }
+
+ priv->enabled_source_streams |= streams_mask;
+
+ return 0;
+
+err:
+ if (!priv->enabled_source_streams)
+ max96714_disable_tx_port(priv);
+
+ return ret;
+}
+
+static int max96714_disable_streams(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *state,
+ u32 source_pad, u64 streams_mask)
+{
+ struct max96714_priv *priv = sd_to_max96714(sd);
+ u64 sink_streams;
+
+ if (!priv->pattern) {
+ int ret;
+
+ sink_streams =
+ v4l2_subdev_state_xlate_streams(state,
+ MAX96714_PAD_SOURCE,
+ MAX96714_PAD_SINK,
+ &streams_mask);
+
+ ret = v4l2_subdev_disable_streams(priv->rxport.source.sd,
+ priv->rxport.source.pad,
+ sink_streams);
+ if (ret)
+ return ret;
+ }
+
+ priv->enabled_source_streams &= ~streams_mask;
+
+ if (!priv->enabled_source_streams)
+ max96714_disable_tx_port(priv);
+
+ return 0;
+}
+
+static int max96714_set_fmt(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *state,
+ struct v4l2_subdev_format *format)
+{
+ struct max96714_priv *priv = sd_to_max96714(sd);
+ struct v4l2_mbus_framefmt *fmt;
+
+ if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE &&
+ priv->enabled_source_streams)
+ return -EBUSY;
+
+ /* No transcoding, source and sink formats must match. */
+ if (format->pad == MAX96714_PAD_SOURCE)
+ return v4l2_subdev_get_fmt(sd, state, format);
+
+ fmt = v4l2_subdev_state_get_format(state, format->pad, format->stream);
+ if (!fmt)
+ return -EINVAL;
+
+ *fmt = format->format;
+
+ fmt = v4l2_subdev_state_get_opposite_stream_format(state, format->pad,
+ format->stream);
+ if (!fmt)
+ return -EINVAL;
+
+ *fmt = format->format;
+
+ return 0;
+}
+
+static int _max96714_set_routing(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *state,
+ enum v4l2_subdev_format_whence which,
+ struct v4l2_subdev_krouting *routing)
+{
+ static const struct v4l2_mbus_framefmt format = {
+ .width = 1280,
+ .height = 1080,
+ .code = MEDIA_BUS_FMT_Y8_1X8,
+ .field = V4L2_FIELD_NONE,
+ };
+ int ret;
+
+ /*
+ * Note: we can only support up to V4L2_FRAME_DESC_ENTRY_MAX, until
+ * frame desc is made dynamically allocated.
+ */
+ if (routing->num_routes > V4L2_FRAME_DESC_ENTRY_MAX)
+ return -EINVAL;
+
+ ret = v4l2_subdev_routing_validate(sd, routing,
+ V4L2_SUBDEV_ROUTING_ONLY_1_TO_1);
+ if (ret)
+ return ret;
+
+ return v4l2_subdev_set_routing_with_fmt(sd, state, routing, &format);
+}
+
+static int max96714_set_routing(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *state,
+ enum v4l2_subdev_format_whence which,
+ struct v4l2_subdev_krouting *routing)
+{
+ struct max96714_priv *priv = sd_to_max96714(sd);
+
+ if (which == V4L2_SUBDEV_FORMAT_ACTIVE && priv->enabled_source_streams)
+ return -EBUSY;
+
+ return _max96714_set_routing(sd, state, which, routing);
+}
+
+static int max96714_init_state(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *state)
+{
+ struct v4l2_subdev_route routes[] = {
+ {
+ .sink_pad = MAX96714_PAD_SINK,
+ .sink_stream = 0,
+ .source_pad = MAX96714_PAD_SOURCE,
+ .source_stream = 0,
+ .flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE,
+ }
+ };
+ struct v4l2_subdev_krouting routing = {
+ .num_routes = ARRAY_SIZE(routes),
+ .routes = routes,
+ };
+
+ return _max96714_set_routing(sd, state, V4L2_SUBDEV_FORMAT_ACTIVE,
+ &routing);
+}
+
+static const struct v4l2_subdev_pad_ops max96714_pad_ops = {
+ .enable_streams = max96714_enable_streams,
+ .disable_streams = max96714_disable_streams,
+
+ .set_routing = max96714_set_routing,
+ .get_fmt = v4l2_subdev_get_fmt,
+ .set_fmt = max96714_set_fmt,
+};
+
+static bool max96714_link_locked(struct max96714_priv *priv)
+{
+ u64 val = 0;
+
+ cci_read(priv->regmap, MAX96714_LINK_LOCK, &val, NULL);
+
+ return val & MAX96714_LINK_LOCK_BIT;
+}
+
+static void max96714_link_status(struct max96714_priv *priv)
+{
+ struct device *dev = &priv->client->dev;
+
+ dev_info(dev, "Link locked:%d\n", max96714_link_locked(priv));
+}
+
+static bool max96714_pipe_locked(struct max96714_priv *priv)
+{
+ u64 val;
+
+ cci_read(priv->regmap, MAX96714_VIDEO_RX8, &val, NULL);
+
+ return val & MAX96714_VID_LOCK;
+}
+
+static void max96714_pipe_status(struct max96714_priv *priv)
+{
+ struct device *dev = &priv->client->dev;
+
+ dev_info(dev, "Pipe vidlock:%d\n", max96714_pipe_locked(priv));
+}
+
+static void max96714_csi_status(struct max96714_priv *priv)
+{
+ struct device *dev = &priv->client->dev;
+ u64 freq = 0;
+
+ cci_read(priv->regmap, MAX96714_BACKTOP25, &freq, NULL);
+ freq = FIELD_GET(CSI_DPLL_FREQ_MASK, freq);
+
+ dev_info(dev, "CSI controller DPLL freq:%u00MHz CSIPHY enabled:%d\n",
+ (u8)freq, max96714_tx_port_enabled(priv));
+}
+
+static int max96714_log_status(struct v4l2_subdev *sd)
+{
+ struct max96714_priv *priv = sd_to_max96714(sd);
+ struct device *dev = &priv->client->dev;
+
+ dev_info(dev, "Deserializer: max96714\n");
+
+ max96714_link_status(priv);
+ max96714_pipe_status(priv);
+ max96714_csi_status(priv);
+
+ return 0;
+}
+
+static const struct v4l2_subdev_core_ops max96714_subdev_core_ops = {
+ .log_status = max96714_log_status,
+};
+
+static const struct v4l2_subdev_video_ops max96714_video_ops = {
+ .s_stream = v4l2_subdev_s_stream_helper,
+};
+
+static const struct v4l2_subdev_internal_ops max96714_internal_ops = {
+ .init_state = max96714_init_state,
+};
+
+static const struct v4l2_subdev_ops max96714_subdev_ops = {
+ .video = &max96714_video_ops,
+ .core = &max96714_subdev_core_ops,
+ .pad = &max96714_pad_ops,
+};
+
+static const struct media_entity_operations max96714_entity_ops = {
+ .link_validate = v4l2_subdev_link_validate,
+};
+
+static int max96714_notify_bound(struct v4l2_async_notifier *notifier,
+ struct v4l2_subdev *subdev,
+ struct v4l2_async_connection *asd)
+{
+ struct max96714_priv *priv = sd_to_max96714(notifier->sd);
+ struct device *dev = &priv->client->dev;
+ int ret;
+
+ ret = media_entity_get_fwnode_pad(&subdev->entity,
+ priv->rxport.source.ep_fwnode,
+ MEDIA_PAD_FL_SOURCE);
+ if (ret < 0) {
+ dev_err(dev, "Failed to find pad for %s\n", subdev->name);
+ return ret;
+ }
+
+ priv->rxport.source.sd = subdev;
+ priv->rxport.source.pad = ret;
+
+ ret = media_create_pad_link(&priv->rxport.source.sd->entity,
+ priv->rxport.source.pad, &priv->sd.entity,
+ MAX96714_PAD_SINK,
+ MEDIA_LNK_FL_ENABLED |
+ MEDIA_LNK_FL_IMMUTABLE);
+ if (ret) {
+ dev_err(dev, "Unable to link %s:%u -> %s:%u\n",
+ priv->rxport.source.sd->name, priv->rxport.source.pad,
+ priv->sd.name, MAX96714_PAD_SINK);
+ return ret;
+ }
+
+ return 0;
+}
+
+static const struct v4l2_async_notifier_operations max96714_notify_ops = {
+ .bound = max96714_notify_bound,
+};
+
+static int max96714_v4l2_notifier_register(struct max96714_priv *priv)
+{
+ struct device *dev = &priv->client->dev;
+ struct max96714_rxport *rxport = &priv->rxport;
+ struct v4l2_async_connection *asd;
+ int ret;
+
+ if (!rxport->source.ep_fwnode)
+ return 0;
+
+ v4l2_async_subdev_nf_init(&priv->notifier, &priv->sd);
+
+ asd = v4l2_async_nf_add_fwnode(&priv->notifier,
+ rxport->source.ep_fwnode,
+ struct v4l2_async_connection);
+ if (IS_ERR(asd)) {
+ dev_err(dev, "Failed to add subdev: %pe", asd);
+ v4l2_async_nf_cleanup(&priv->notifier);
+ return PTR_ERR(asd);
+ }
+
+ priv->notifier.ops = &max96714_notify_ops;
+
+ ret = v4l2_async_nf_register(&priv->notifier);
+ if (ret) {
+ dev_err(dev, "Failed to register subdev_notifier");
+ v4l2_async_nf_cleanup(&priv->notifier);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int max96714_create_subdev(struct max96714_priv *priv)
+{
+ struct device *dev = &priv->client->dev;
+ int ret;
+
+ v4l2_i2c_subdev_init(&priv->sd, priv->client, &max96714_subdev_ops);
+ priv->sd.internal_ops = &max96714_internal_ops;
+
+ v4l2_ctrl_handler_init(&priv->ctrl_handler, 1);
+ priv->sd.ctrl_handler = &priv->ctrl_handler;
+
+ v4l2_ctrl_new_int_menu(&priv->ctrl_handler, NULL, V4L2_CID_LINK_FREQ,
+ 0, 0, &priv->tx_link_freq);
+ v4l2_ctrl_new_std_menu_items(&priv->ctrl_handler,
+ &max96714_ctrl_ops,
+ V4L2_CID_TEST_PATTERN,
+ ARRAY_SIZE(max96714_test_pattern) - 1,
+ 0, 0, max96714_test_pattern);
+ if (priv->ctrl_handler.error) {
+ ret = priv->ctrl_handler.error;
+ goto err_free_ctrl;
+ }
+
+ priv->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_STREAMS;
+ priv->sd.entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
+ priv->sd.entity.ops = &max96714_entity_ops;
+
+ priv->pads[MAX96714_PAD_SINK].flags = MEDIA_PAD_FL_SINK;
+ priv->pads[MAX96714_PAD_SOURCE].flags = MEDIA_PAD_FL_SOURCE;
+
+ ret = media_entity_pads_init(&priv->sd.entity,
+ MAX96714_NPORTS,
+ priv->pads);
+ if (ret)
+ goto err_free_ctrl;
+
+ priv->sd.state_lock = priv->sd.ctrl_handler->lock;
+
+ ret = v4l2_subdev_init_finalize(&priv->sd);
+ if (ret)
+ goto err_entity_cleanup;
+
+ ret = max96714_v4l2_notifier_register(priv);
+ if (ret) {
+ dev_err(dev, "v4l2 subdev notifier register failed: %d\n", ret);
+ goto err_subdev_cleanup;
+ }
+
+ ret = v4l2_async_register_subdev(&priv->sd);
+ if (ret) {
+ dev_err(dev, "v4l2_async_register_subdev error: %d\n", ret);
+ goto err_unreg_notif;
+ }
+
+ return 0;
+
+err_unreg_notif:
+ v4l2_async_nf_unregister(&priv->notifier);
+ v4l2_async_nf_cleanup(&priv->notifier);
+err_subdev_cleanup:
+ v4l2_subdev_cleanup(&priv->sd);
+err_entity_cleanup:
+ media_entity_cleanup(&priv->sd.entity);
+err_free_ctrl:
+ v4l2_ctrl_handler_free(&priv->ctrl_handler);
+
+ return ret;
+};
+
+static void max96714_destroy_subdev(struct max96714_priv *priv)
+{
+ v4l2_async_nf_unregister(&priv->notifier);
+ v4l2_async_nf_cleanup(&priv->notifier);
+ v4l2_async_unregister_subdev(&priv->sd);
+
+ v4l2_subdev_cleanup(&priv->sd);
+
+ media_entity_cleanup(&priv->sd.entity);
+ v4l2_ctrl_handler_free(&priv->ctrl_handler);
+}
+
+static int max96714_i2c_mux_select(struct i2c_mux_core *mux, u32 chan)
+{
+ return 0;
+}
+
+static int max96714_i2c_mux_init(struct max96714_priv *priv)
+{
+ priv->mux = i2c_mux_alloc(priv->client->adapter, &priv->client->dev,
+ 1, 0, I2C_MUX_LOCKED | I2C_MUX_GATE,
+ max96714_i2c_mux_select, NULL);
+ if (!priv->mux)
+ return -ENOMEM;
+
+ return i2c_mux_add_adapter(priv->mux, 0, 0, 0);
+}
+
+static int max96714_init_tx_port(struct max96714_priv *priv)
+{
+ struct v4l2_mbus_config_mipi_csi2 *mipi;
+ unsigned long lanes_used = 0;
+ unsigned int val, lane;
+ int ret;
+
+ ret = max96714_disable_tx_port(priv);
+
+ mipi = &priv->mipi_csi2;
+ val = div_u64(priv->tx_link_freq * 2, MHZ(100));
+
+ cci_update_bits(priv->regmap, MAX96714_BACKTOP25,
+ CSI_DPLL_FREQ_MASK, val, &ret);
+
+ val = FIELD_PREP(MAX96714_CSI2_LANE_CNT_MASK, mipi->num_data_lanes - 1);
+ cci_update_bits(priv->regmap, MAX96714_MIPI_LANE_CNT,
+ MAX96714_CSI2_LANE_CNT_MASK, val, &ret);
+
+ /* lanes polarity */
+ val = 0;
+ for (lane = 0; lane < mipi->num_data_lanes + 1; lane++) {
+ if (!mipi->lane_polarities[lane])
+ continue;
+ if (lane == 0)
+ /* clock lane */
+ val |= BIT(5);
+ else if (lane < 3)
+ /* Lane D0 and D1 */
+ val |= BIT(lane - 1);
+ else
+ /* D2 and D3 */
+ val |= BIT(lane);
+ }
+
+ cci_update_bits(priv->regmap, MAX96714_MIPI_POLARITY,
+ MAX96714_MIPI_POLARITY_MASK, val, &ret);
+
+ /* lanes mapping */
+ val = 0;
+ for (lane = 0; lane < mipi->num_data_lanes; lane++) {
+ val |= (mipi->data_lanes[lane] - 1) << (lane * 2);
+ lanes_used |= BIT(mipi->data_lanes[lane] - 1);
+ }
+
+ /*
+ * Unused lanes need to be mapped as well to not have
+ * the same lanes mapped twice.
+ */
+ for (; lane < 4; lane++) {
+ unsigned int idx = find_first_zero_bit(&lanes_used, 4);
+
+ val |= idx << (lane * 2);
+ lanes_used |= BIT(idx);
+ }
+
+ return cci_write(priv->regmap, MAX96714_MIPI_LANE_MAP, val, &ret);
+}
+
+static int max96714_rxport_enable_poc(struct max96714_priv *priv)
+{
+ struct max96714_rxport *rxport = &priv->rxport;
+
+ if (!rxport->poc)
+ return 0;
+
+ return regulator_enable(rxport->poc);
+}
+
+static int max96714_rxport_disable_poc(struct max96714_priv *priv)
+{
+ struct max96714_rxport *rxport = &priv->rxport;
+
+ if (!rxport->poc)
+ return 0;
+
+ return regulator_disable(rxport->poc);
+}
+
+static int max96714_parse_dt_txport(struct max96714_priv *priv)
+{
+ struct device *dev = &priv->client->dev;
+ struct v4l2_fwnode_endpoint vep = {
+ .bus_type = V4L2_MBUS_CSI2_DPHY
+ };
+ struct fwnode_handle *ep_fwnode;
+ u32 num_data_lanes;
+ int ret;
+
+ ep_fwnode = fwnode_graph_get_endpoint_by_id(dev_fwnode(dev),
+ MAX96714_PAD_SOURCE, 0, 0);
+ if (!ep_fwnode)
+ return -EINVAL;
+
+ ret = v4l2_fwnode_endpoint_alloc_parse(ep_fwnode, &vep);
+ fwnode_handle_put(ep_fwnode);
+ if (ret) {
+ dev_err(dev, "tx: failed to parse endpoint data\n");
+ return -EINVAL;
+ }
+
+ if (vep.nr_of_link_frequencies != 1) {
+ ret = -EINVAL;
+ goto err_free_vep;
+ }
+
+ priv->tx_link_freq = vep.link_frequencies[0];
+ /* Min 50MHz, Max 1250MHz, 50MHz step */
+ if (priv->tx_link_freq < MHZ(50) || priv->tx_link_freq > MHZ(1250) ||
+ (u32)priv->tx_link_freq % MHZ(50)) {
+ dev_err(dev, "tx: invalid link frequency\n");
+ ret = -EINVAL;
+ goto err_free_vep;
+ }
+
+ num_data_lanes = vep.bus.mipi_csi2.num_data_lanes;
+ if (num_data_lanes < 1 || num_data_lanes > 4) {
+ dev_err(dev,
+ "tx: invalid number of data lanes must be 1 to 4\n");
+ ret = -EINVAL;
+ goto err_free_vep;
+ }
+
+ memcpy(&priv->mipi_csi2, &vep.bus.mipi_csi2, sizeof(priv->mipi_csi2));
+
+err_free_vep:
+ v4l2_fwnode_endpoint_free(&vep);
+
+ return ret;
+}
+
+static int max96714_parse_dt_rxport(struct max96714_priv *priv)
+{
+ static const char *poc_name = "port0-poc";
+ struct max96714_rxport *rxport = &priv->rxport;
+ struct device *dev = &priv->client->dev;
+ struct fwnode_handle *ep_fwnode;
+ int ret;
+
+ ep_fwnode = fwnode_graph_get_endpoint_by_id(dev_fwnode(dev),
+ MAX96714_PAD_SINK, 0, 0);
+ if (!ep_fwnode)
+ return -ENOENT;
+
+ rxport->source.ep_fwnode = fwnode_graph_get_remote_endpoint(ep_fwnode);
+ fwnode_handle_put(ep_fwnode);
+
+ if (!rxport->source.ep_fwnode) {
+ dev_err(dev, "rx: no remote endpoint\n");
+ return -EINVAL;
+ }
+
+ rxport->poc = devm_regulator_get_optional(dev, poc_name);
+ if (IS_ERR(rxport->poc)) {
+ ret = PTR_ERR(rxport->poc);
+ if (ret == -ENODEV) {
+ rxport->poc = NULL;
+ } else {
+ dev_err(dev, "rx: failed to get POC supply: %d\n", ret);
+ goto err_put_source_ep_fwnode;
+ }
+ }
+
+ return 0;
+
+err_put_source_ep_fwnode:
+ fwnode_handle_put(rxport->source.ep_fwnode);
+ return ret;
+}
+
+static int max96714_parse_dt(struct max96714_priv *priv)
+{
+ int ret;
+
+ ret = max96714_parse_dt_txport(priv);
+ if (ret)
+ return ret;
+
+ ret = max96714_parse_dt_rxport(priv);
+ /*
+ * The deserializer can create a test pattern even if the
+ * rx port is not connected to a serializer.
+ */
+ if (ret && ret == -ENOENT)
+ ret = 0;
+
+ return ret;
+}
+
+static int max96714_enable_core_hw(struct max96714_priv *priv)
+{
+ struct device *dev = &priv->client->dev;
+ u64 val;
+ int ret;
+
+ if (priv->pd_gpio) {
+ /* wait min 2 ms for reset to complete */
+ gpiod_set_value_cansleep(priv->pd_gpio, 1);
+ fsleep(2000);
+ gpiod_set_value_cansleep(priv->pd_gpio, 0);
+ /* wait min 2 ms for power up to finish */
+ fsleep(2000);
+ }
+
+ ret = cci_read(priv->regmap, MAX96714_REG13, &val, NULL);
+ if (ret) {
+ dev_err_probe(dev, ret, "Cannot read first register, abort\n");
+ goto err_pd_gpio;
+ }
+
+ if (val != MAX96714_DEVICE_ID && val != MAX96714F_DEVICE_ID) {
+ dev_err(dev, "Unsupported device id expected %x got %x\n",
+ MAX96714F_DEVICE_ID, (u8)val);
+ ret = -EOPNOTSUPP;
+ goto err_pd_gpio;
+ }
+
+ ret = cci_read(priv->regmap, MAX96714_DEV_REV, &val, NULL);
+ if (ret)
+ goto err_pd_gpio;
+
+ dev_dbg(dev, "Found %x (rev %lx)\n", MAX96714F_DEVICE_ID,
+ (u8)val & MAX96714_DEV_REV_MASK);
+
+ ret = cci_read(priv->regmap, MAX96714_MIPI_TX52, &val, NULL);
+ if (ret)
+ goto err_pd_gpio;
+
+ if (!(val & MAX96714_TUN_EN)) {
+ dev_err(dev, "Only supporting tunnel mode");
+ ret = -EOPNOTSUPP;
+ goto err_pd_gpio;
+ }
+
+ return 0;
+
+err_pd_gpio:
+ gpiod_set_value_cansleep(priv->pd_gpio, 1);
+ return ret;
+}
+
+static void max96714_disable_core_hw(struct max96714_priv *priv)
+{
+ gpiod_set_value_cansleep(priv->pd_gpio, 1);
+}
+
+static int max96714_get_hw_resources(struct max96714_priv *priv)
+{
+ struct device *dev = &priv->client->dev;
+
+ priv->regmap = devm_cci_regmap_init_i2c(priv->client, 16);
+ if (IS_ERR(priv->regmap))
+ return PTR_ERR(priv->regmap);
+
+ priv->pd_gpio =
+ devm_gpiod_get_optional(dev, "powerdown", GPIOD_OUT_HIGH);
+ if (IS_ERR(priv->pd_gpio))
+ return dev_err_probe(dev, PTR_ERR(priv->pd_gpio),
+ "Cannot get powerdown GPIO\n");
+ return 0;
+}
+
+static int max96714_probe(struct i2c_client *client)
+{
+ struct device *dev = &client->dev;
+ struct max96714_priv *priv;
+ int ret;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->client = client;
+
+ ret = max96714_get_hw_resources(priv);
+ if (ret)
+ return ret;
+
+ ret = max96714_enable_core_hw(priv);
+ if (ret)
+ return ret;
+
+ ret = max96714_parse_dt(priv);
+ if (ret)
+ goto err_disable_core_hw;
+
+ max96714_init_tx_port(priv);
+
+ ret = max96714_rxport_enable_poc(priv);
+ if (ret)
+ goto err_free_ports;
+
+ ret = max96714_i2c_mux_init(priv);
+ if (ret)
+ goto err_disable_poc;
+
+ ret = max96714_create_subdev(priv);
+ if (ret)
+ goto err_del_mux;
+
+ return 0;
+
+err_del_mux:
+ i2c_mux_del_adapters(priv->mux);
+err_disable_poc:
+ max96714_rxport_disable_poc(priv);
+err_free_ports:
+ fwnode_handle_put(priv->rxport.source.ep_fwnode);
+err_disable_core_hw:
+ max96714_disable_core_hw(priv);
+
+ return ret;
+}
+
+static void max96714_remove(struct i2c_client *client)
+{
+ struct v4l2_subdev *sd = i2c_get_clientdata(client);
+ struct max96714_priv *priv = sd_to_max96714(sd);
+
+ max96714_destroy_subdev(priv);
+ i2c_mux_del_adapters(priv->mux);
+ max96714_rxport_disable_poc(priv);
+ fwnode_handle_put(priv->rxport.source.ep_fwnode);
+ max96714_disable_core_hw(priv);
+ gpiod_set_value_cansleep(priv->pd_gpio, 1);
+}
+
+static const struct of_device_id max96714_of_ids[] = {
+ { .compatible = "maxim,max96714f" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, max96714_of_ids);
+
+static struct i2c_driver max96714_i2c_driver = {
+ .driver = {
+ .name = "max96714",
+ .of_match_table = max96714_of_ids,
+ },
+ .probe = max96714_probe,
+ .remove = max96714_remove,
+};
+
+module_i2c_driver(max96714_i2c_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Maxim Integrated GMSL2 Deserializers Driver");
+MODULE_AUTHOR("Julien Massot <julien.massot@collabora.com>");
--
2.44.0
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH v7 5/5] drivers: media: max96717: stop the csi receiver before the source
2024-04-30 13:19 [PATCH v7 0/5] Add support for MAX96714/F and MAX96717/F GMSL2 ser/des Julien Massot
` (3 preceding siblings ...)
2024-04-30 13:19 ` [PATCH v7 4/5] media: i2c: add MAX96714 driver Julien Massot
@ 2024-04-30 13:19 ` Julien Massot
2024-05-17 14:16 ` [PATCH v7 0/5] Add support for MAX96714/F and MAX96717/F GMSL2 ser/des Tommaso Merciai
2024-06-06 13:34 ` Tomi Valkeinen
6 siblings, 0 replies; 14+ messages in thread
From: Julien Massot @ 2024-04-30 13:19 UTC (permalink / raw)
To: linux-media, sakari.ailus
Cc: devicetree, kernel, linux-kernel, mchehab, robh+dt,
krzysztof.kozlowski+dt, conor+dt, Julien Massot
Stopping the CSI source before stopping the serializer
CSI port may make the serializer not respond.
Then all the next writes to the device will fail.
max96717 1-0040: Error writing reg 0x0308: -121
max96717 1-0040: Error writing reg 0x0006: -121
Fix that by stopping the CSI receiver first and then CSI source.
Seen on max96717f revision 4.
Signed-off-by: Julien Massot <julien.massot@collabora.com>
---
drivers/media/i2c/max96717.c | 23 +++++++++++------------
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/drivers/media/i2c/max96717.c b/drivers/media/i2c/max96717.c
index 1ea76f922bdb..499408837aef 100644
--- a/drivers/media/i2c/max96717.c
+++ b/drivers/media/i2c/max96717.c
@@ -384,24 +384,23 @@ static int max96717_disable_streams(struct v4l2_subdev *sd,
{
struct max96717_priv *priv = sd_to_max96717(sd);
u64 sink_streams;
- int ret;
+
+ /*
+ * Stop the CSI receiver first then the source,
+ * otherwise the device may become unresponsive
+ * while holding the I2C bus low.
+ */
+ priv->enabled_source_streams &= ~streams_mask;
+ if (!priv->enabled_source_streams)
+ max96717_start_csi(priv, false);
sink_streams = v4l2_subdev_state_xlate_streams(state,
MAX96717_PAD_SOURCE,
MAX96717_PAD_SINK,
&streams_mask);
- ret = v4l2_subdev_disable_streams(priv->source_sd, priv->source_sd_pad,
- sink_streams);
- if (ret)
- return ret;
-
- priv->enabled_source_streams &= ~streams_mask;
-
- if (!priv->enabled_source_streams)
- max96717_start_csi(priv, false);
-
- return 0;
+ return v4l2_subdev_disable_streams(priv->source_sd, priv->source_sd_pad,
+ sink_streams);
}
static const struct v4l2_subdev_pad_ops max96717_pad_ops = {
--
2.44.0
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v7 0/5] Add support for MAX96714/F and MAX96717/F GMSL2 ser/des
2024-04-30 13:19 [PATCH v7 0/5] Add support for MAX96714/F and MAX96717/F GMSL2 ser/des Julien Massot
` (4 preceding siblings ...)
2024-04-30 13:19 ` [PATCH v7 5/5] drivers: media: max96717: stop the csi receiver before the source Julien Massot
@ 2024-05-17 14:16 ` Tommaso Merciai
2024-06-06 13:34 ` Tomi Valkeinen
6 siblings, 0 replies; 14+ messages in thread
From: Tommaso Merciai @ 2024-05-17 14:16 UTC (permalink / raw)
To: Julien Massot
Cc: linux-media, sakari.ailus, devicetree, kernel, linux-kernel,
mchehab, robh+dt, krzysztof.kozlowski+dt, conor+dt, mhecht73,
martin.hecht
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
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v7 0/5] Add support for MAX96714/F and MAX96717/F GMSL2 ser/des
2024-04-30 13:19 [PATCH v7 0/5] Add support for MAX96714/F and MAX96717/F GMSL2 ser/des Julien Massot
` (5 preceding siblings ...)
2024-05-17 14:16 ` [PATCH v7 0/5] Add support for MAX96714/F and MAX96717/F GMSL2 ser/des Tommaso Merciai
@ 2024-06-06 13:34 ` Tomi Valkeinen
2024-06-06 15:24 ` Laurent Pinchart
2024-06-07 9:01 ` Julien Massot
6 siblings, 2 replies; 14+ messages in thread
From: Tomi Valkeinen @ 2024-06-06 13:34 UTC (permalink / raw)
To: Julien Massot, sakari.ailus, Laurent Pinchart
Cc: devicetree, kernel, linux-kernel, mchehab, robh+dt,
krzysztof.kozlowski+dt, linux-media, conor+dt
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
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v7 0/5] Add support for MAX96714/F and MAX96717/F GMSL2 ser/des
2024-06-06 13:34 ` Tomi Valkeinen
@ 2024-06-06 15:24 ` Laurent Pinchart
2024-06-06 15:55 ` Tomi Valkeinen
2024-06-07 9:27 ` Julien Massot
2024-06-07 9:01 ` Julien Massot
1 sibling, 2 replies; 14+ messages in thread
From: Laurent Pinchart @ 2024-06-06 15:24 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Julien Massot, sakari.ailus, devicetree, kernel, linux-kernel,
mchehab, robh+dt, krzysztof.kozlowski+dt, linux-media, conor+dt
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.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v7 0/5] Add support for MAX96714/F and MAX96717/F GMSL2 ser/des
2024-06-06 15:24 ` Laurent Pinchart
@ 2024-06-06 15:55 ` Tomi Valkeinen
2024-06-07 9:27 ` Julien Massot
1 sibling, 0 replies; 14+ messages in thread
From: Tomi Valkeinen @ 2024-06-06 15:55 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Julien Massot, sakari.ailus, devicetree, kernel, linux-kernel,
mchehab, robh+dt, krzysztof.kozlowski+dt, linux-media, conor+dt
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v7 0/5] Add support for MAX96714/F and MAX96717/F GMSL2 ser/des
2024-06-06 15:24 ` Laurent Pinchart
2024-06-06 15:55 ` Tomi Valkeinen
@ 2024-06-07 9:27 ` Julien Massot
1 sibling, 0 replies; 14+ messages in thread
From: Julien Massot @ 2024-06-07 9:27 UTC (permalink / raw)
To: Laurent Pinchart, Tomi Valkeinen
Cc: sakari.ailus, devicetree, kernel, linux-kernel, mchehab, robh+dt,
krzysztof.kozlowski+dt, linux-media, conor+dt
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.
--
Julien
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v7 0/5] Add support for MAX96714/F and MAX96717/F GMSL2 ser/des
2024-06-06 13:34 ` Tomi Valkeinen
2024-06-06 15:24 ` Laurent Pinchart
@ 2024-06-07 9:01 ` Julien Massot
1 sibling, 0 replies; 14+ messages in thread
From: Julien Massot @ 2024-06-07 9:01 UTC (permalink / raw)
To: Tomi Valkeinen, sakari.ailus, Laurent Pinchart
Cc: devicetree, kernel, linux-kernel, mchehab, robh+dt,
krzysztof.kozlowski+dt, linux-media, conor+dt
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
>
--
Julien
^ permalink raw reply [flat|nested] 14+ messages in thread