On 06/09/2023 11:00, Laurent Pinchart wrote: >>> has a regulator@0. There are similar instances for clocks. >>> >>> I understand why it may not be a good idea, and how the root node is >>> indeed not a bus. In some cases, those regulators and clocks are grouped >>> in a regulators or clocks node that has a "simple-bus" compatible. I'm >>> not sure if that's a good idea, but at least it should validate. >>> >>> What's the best practice for discrete board-level clocks and regulators >>> in overlays ? How do we ensure that their node name will not conflict >>> with the board to which the overlay is attached ? >> >> Top-level nodes (so under /) do not have unit addresses. If they have - >> it's an error, because it is not a bus. Also, unit address requires reg. >> No reg? No unit address. DTC reports this as warnings as well. > > I agree with all that, but what's the recommended practice to add > top-level clocks and regulators in overlays, in a way that avoids > namespace clashes with the base board ? Whether you use regulator@0 or regulator-0, you have the same chances of clash. > >>>>> + orientation = <0>; >>>>> + rotation = <0>; >>>>> + >>>>> + thine,rx,data-lanes = <4 1 3 2>; >>>> >>>> NAK for this property. >>> >>> Please explain why. You commented very briefly in the bindings review, >>> and it wasn't clear to me if you were happy or not with the property, >>> and if not, why. >> >> Because it is duplicating endpoint. At least from the description. > > The THP7312 is an external ISP. At the hardware level, it has an input > side, with a CSI-2 receiver and an I2C master controller, and an output > side, with a CSI-2 transmitter and an I2C slave controller. A raw camera > sensor is connected on the input side, transmitting image data to the > THP7312, and being controlled over I2C by the firmware running on the > THP7312. From a Linux point of view, only the output side of the THP7312 > is visible, and the combination of the raw camera sensor and the THP7312 > acts as a smart camera sensor, producing YUV images. None of this was explained in the device description or property field. I probably judged to fast but it just looked like duplicated property. Then shouldn't it have two ports, even if camera side is not visible for the Linux? > > As there are two CSI-2 buses, the data lanes configuration needs to be > specified for both sides. On the output side, connected to the SoC and > visible to Linux, the bindings use a port node with an endpoint and the > standard data-lanes property. On the input side, which is invisible to > Linux, the bindings use the vendor-specific thine,rx,data-lanes > property. Its semantics is identical to the standard data-lanes > property, but it's not located in an endpoint as there's no port for the > input side. And how does the property support multiple sensors? What if they data lanes are also different between each other? Best regards, Krzysztof