| Message ID | 20231208143359.469049-1-julien.massot@collabora.com |
|---|---|
| Headers | show |
| Series | Add support for MAX96714F and MAX96717F GMSL2 ser/des | expand |
On 08/12/2023 15:33, Julien Massot wrote: > Add DT bindings for Maxim MAX96714F GMSL2 Deserializer. > > Signed-off-by: Julien Massot <julien.massot@collabora.com> Please use scripts/get_maintainers.pl to get a list of necessary people and lists to CC (and consider --no-git-fallback argument). It might happen, that command when run on an older kernel, gives you outdated entries. Therefore please be sure you base your patches on recent Linux kernel. Limited review follows, as robot already pointed out issues... > --- > .../bindings/media/i2c/maxim,max96714f.yaml | 163 ++++++++++++++++++ > 1 file changed, 163 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/i2c/maxim,max96714f.yaml > > diff --git a/Documentation/devicetree/bindings/media/i2c/maxim,max96714f.yaml b/Documentation/devicetree/bindings/media/i2c/maxim,max96714f.yaml > new file mode 100644 > index 000000000000..8a2a06e7e279 > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/i2c/maxim,max96714f.yaml > @@ -0,0 +1,163 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +# Copyright (C) 2023 Collabora Ltd. > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/media/i2c/maxim,max96714f.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: GMSL2 to CSI-2 Deserializer > + > +maintainers: > + - Julien Massot <julien.massot@collabora.com> > + > +description: | > + The MAX96714F deserializer converts GMSL2 serial inputs into MIPI > + CSI-2 D-PHY or C-PHY formatted output. The device allows the GMSL2 link to > + simultaneously transmit bidirectional control-channel data while forward > + video transmissions are in progress. The MAX96714F 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 MAX96714F 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 in the > + forward direction and 187.5Mbps in the reverse direction. > + > +properties: > + compatible: > + const: maxim,max96714f > + > + reg: > + description: I2C device address Drop > + maxItems: 1 > + > + enable-gpios: true > + > + 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 > + > + properties: > + endpoint: > + $ref: /schemas/media/video-interfaces.yaml# > + unevaluatedProperties: false > + > + properties: > + data-lanes: true > + bus-type: > + enum: > + - 4 # MEDIA_BUS_TYPE_CSI2_DPHY > + > + required: > + - data-lanes > + - bus-type > + > + required: > + - port@1 > + > + i2c-gate: > + $ref: /schemas/i2c/i2c-controller.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 a particular port > + > +required: > + - compatible > + - reg > + - ports > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/gpio/gpio.h> > + #include <dt-bindings/media/video-interfaces.h> > + > + main_i2c2 { Node names should be generic. See also an explanation and list of examples (not exhaustive) in DT specification: https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation > + #address-cells = <1>; > + #size-cells = <0>; > + i2c-gate { > + #address-cells = <1>; > + #size-cells = <0>; > + > + gmsl-serializer@40 { > + compatible = "maxim,max96717f"; > + reg = <0x40>; Not documented. > +... Best regards, Krzysztof
On 08/12/2023 15:33, Julien Massot wrote: > This driver handle the MAX96714 deserializer in tunnel mode. > The CSI output will replicate all the CSI traffic capture by > the remote serializer. > > Signed-off-by: Julien Massot <julien.massot@collabora.com> ... > +static int max96714_get_hw_resources(struct max96714_priv *priv) > +{ > + struct device *dev = &priv->client->dev; > + > + priv->regmap = devm_regmap_init_i2c(priv->client, > + &max96714_regmap_config); > + if (IS_ERR(priv->regmap)) > + return PTR_ERR(priv->regmap); > + > + priv->gpiod_pwdn = devm_gpiod_get_optional(&priv->client->dev, "enable", > + GPIOD_OUT_HIGH); > + if (IS_ERR(priv->gpiod_pwdn)) > + return dev_err_probe(dev, PTR_ERR(priv->gpiod_pwdn), A powerdown GPIO is not an enable GPIO. Please use correct name - see gpio-consumers-common.yaml The same applies to other patch and potentially all others... Best regards, Krzysztof
Hi Krzysztof, Thanks for the review. On 12/8/23 18:16, Krzysztof Kozlowski wrote: > On 08/12/2023 15:33, Julien Massot wrote: >> Add DT bindings for Maxim MAX96714F GMSL2 Deserializer. >> >> Signed-off-by: Julien Massot <julien.massot@collabora.com> > > Please use scripts/get_maintainers.pl to get a list of necessary people > and lists to CC (and consider --no-git-fallback argument). It might > happen, that command when run on an older kernel, gives you outdated > entries. Therefore please be sure you base your patches on recent Linux > kernel. Will do > > Limited review follows, as robot already pointed out issues.. > >> --- >> .../bindings/media/i2c/maxim,max96714f.yaml | 163 ++++++++++++++++++ >> 1 file changed, 163 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/media/i2c/maxim,max96714f.yaml >> >> diff --git a/Documentation/devicetree/bindings/media/i2c/maxim,max96714f.yaml b/Documentation/devicetree/bindings/media/i2c/maxim,max96714f.yaml >> new file mode 100644 >> index 000000000000..8a2a06e7e279 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/media/i2c/maxim,max96714f.yaml >> @@ -0,0 +1,163 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +# Copyright (C) 2023 Collabora Ltd. >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/media/i2c/maxim,max96714f.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: GMSL2 to CSI-2 Deserializer >> + >> +maintainers: >> + - Julien Massot <julien.massot@collabora.com> >> + >> +description: | >> + The MAX96714F deserializer converts GMSL2 serial inputs into MIPI >> + CSI-2 D-PHY or C-PHY formatted output. The device allows the GMSL2 link to >> + simultaneously transmit bidirectional control-channel data while forward >> + video transmissions are in progress. The MAX96714F 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 MAX96714F 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 in the >> + forward direction and 187.5Mbps in the reverse direction. >> + >> +properties: >> + compatible: >> + const: maxim,max96714f >> + >> + reg: >> + description: I2C device address > > Drop Ok > >> + maxItems: 1 >> + >> + enable-gpios: true >> + >> + 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 >> + >> + properties: >> + endpoint: >> + $ref: /schemas/media/video-interfaces.yaml# >> + unevaluatedProperties: false >> + >> + properties: >> + data-lanes: true >> + bus-type: >> + enum: >> + - 4 # MEDIA_BUS_TYPE_CSI2_DPHY >> + >> + required: >> + - data-lanes >> + - bus-type >> + >> + required: >> + - port@1 >> + >> + i2c-gate: >> + $ref: /schemas/i2c/i2c-controller.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 a particular port >> + >> +required: >> + - compatible >> + - reg >> + - ports >> + >> +additionalProperties: false >> + >> +examples: >> + - | >> + #include <dt-bindings/gpio/gpio.h> >> + #include <dt-bindings/media/video-interfaces.h> >> + >> + main_i2c2 { > > Node names should be generic. See also an explanation and list of > examples (not exhaustive) in DT specification: > https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation Ok V3 will use generic node names. > > >> + #address-cells = <1>; >> + #size-cells = <0>; > >> + i2c-gate { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + gmsl-serializer@40 { >> + compatible = "maxim,max96717f"; >> + reg = <0x40>; > > Not documented. Ok I will re-post max96717f binding first and then the max96714f binding. Thank you,