On Wed, Jul 24, 2024 at 07:18:39PM GMT, Marc Gonzalez wrote: > On 15/07/2024 16:42, Maxime Ripard wrote: > > On Mon, Jul 08, 2024 at 11:29:46PM GMT, Dmitry Baryshkov wrote: > >> On Mon, Jul 08, 2024 at 04:59:23PM GMT, Maxime Ripard wrote: > >>> On Mon, Jul 01, 2024 at 05:36:18PM GMT, Marc Gonzalez wrote: > >>>> On 01/07/2024 15:50, Maxime Ripard wrote: > >>>> > >>>>> On Thu, Jun 27, 2024 at 01:13:03PM GMT, Marc Gonzalez wrote: > >>>>> > >>>>>> TDP158 is an AC-coupled DVI / HDMI to TMDS level shifting Redriver. > >>>>>> It supports DVI 1.0, HDMI 1.4b and 2.0b. > >>>>>> It supports 4 TMDS channels, HPD, and a DDC interface. > >>>>>> It supports dual power supply rails (1.1V on VDD, 3.3V on VCC) > >>>>>> for power reduction. Several methods of power management are > >>>>>> implemented to reduce overall power consumption. > >>>>>> It supports fixed receiver EQ gain using I2C or pin strap to > >>>>>> compensate for different lengths input cable or board traces. > >>>>>> > >>>>>> Features > >>>>>> > >>>>>> - AC-coupled TMDS or DisplayPort dual-mode physical layer input > >>>>>> to HDMI 2.0b TMDS physical layer output supporting up to 6Gbps > >>>>>> data rate, compatible with HDMI 2.0b electrical parameters > >>>>>> - DisplayPort dual-mode standard version 1.1 > >>>>>> - Programmable fixed receiver equalizer up to 15.5dB > >>>>>> - Global or independent high speed lane control, pre-emphasis > >>>>>> and transmit swing, and slew rate control > >>>>>> - I2C or pin strap programmable > >>>>>> - Configurable as a DisplayPort redriver through I2C > >>>>>> - Full lane swap on main lanes > >>>>>> - Low power consumption (200 mW at 6Gbps, 8 mW in shutdown) > >>>>>> > >>>>>> https://www.ti.com/lit/ds/symlink/tdp158.pdf > >>>>>> > >>>>>> Signed-off-by: Marc Gonzalez <mgonzalez@xxxxxxxxxx> > >>>>>> --- > >>>>>> .../bindings/display/bridge/ti,tdp158.yaml | 51 ++++++++++++++++++++++ > >>>>>> 1 file changed, 51 insertions(+) > >>>>>> > >>>>>> diff --git a/Documentation/devicetree/bindings/display/bridge/ti,tdp158.yaml b/Documentation/devicetree/bindings/display/bridge/ti,tdp158.yaml > >>>>>> new file mode 100644 > >>>>>> index 0000000000000..21c8585c3bb2d > >>>>>> --- /dev/null > >>>>>> +++ b/Documentation/devicetree/bindings/display/bridge/ti,tdp158.yaml > >>>>>> @@ -0,0 +1,51 @@ > >>>>>> +# SPDX-License-Identifier: GPL-2.0-only > >>>>>> +%YAML 1.2 > >>>>>> +--- > >>>>>> +$id: http://devicetree.org/schemas/display/bridge/ti,tdp158.yaml# > >>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml# > >>>>>> + > >>>>>> +title: TI TDP158 HDMI to TMDS Redriver > >>>>>> + > >>>>>> +maintainers: > >>>>>> + - Arnaud Vrac <avrac@xxxxxxxxxx> > >>>>>> + - Pierre-Hugues Husson <phhusson@xxxxxxxxxx> > >>>>>> + > >>>>>> +properties: > >>>>>> + compatible: > >>>>>> + const: ti,tdp158 > >>>>>> + > >>>>>> + reg: > >>>>>> + description: I2C address of the device > >>>>>> + > >>>>>> + enable-gpios: > >>>>>> + description: GPIO controlling bridge enable > >>>>>> + > >>>>>> + vcc-supply: > >>>>>> + description: Power supply 3.3V > >>>>>> + > >>>>>> + vdd-supply: > >>>>>> + description: Power supply 1.1V > >>>>>> + > >>>>>> + ports: > >>>>>> + $ref: /schemas/graph.yaml#/properties/ports > >>>>>> + > >>>>>> + properties: > >>>>>> + port@0: > >>>>>> + $ref: /schemas/graph.yaml#/properties/port > >>>>>> + description: Bridge input > >>>>>> + > >>>>>> + port@1: > >>>>>> + $ref: /schemas/graph.yaml#/properties/port > >>>>>> + description: Bridge output > >>>>>> + > >>>>>> + required: > >>>>>> + - port@0 > >>>>>> + - port@1 > >>>>> > >>>>> The device supports DVI, HDMI or DP input, with various requirements and > >>>>> capabilities depending on the input. Your binding doesn't address that. > >>>>> > >>>>> Similarly, it can do lane-swapping, so we should probably have a > >>>>> property to describe what mapping we want to use. > >>>>> > >>>>> The i2c register access (and the whole behaviour of the device) is > >>>>> constrained on the I2C_EN pin status, and you can't read it from the > >>>>> device, so it's also something we need to have in the DT. > >>>> > >>>> We are using the device in its default configuration. > >>>> (Power on via OE, then it works as expected) > >>> > >>> I know, but that doesn't really matter for a binding. > >>> > >>>> Can we leave any additional properties to be defined by whomever needs > >>>> them in the future? > >>> > >>> If you can guarantee that doing so would be backward compatible, sure. > >>> But that means being able to answer those questions with a reasonable > >>> plan. > >> > >> I think proposed bindings are generic enough to cover other possible > >> usecases in future. > > > > I don't think it is. The current binding is for a I2C device that > > shouldn't be accessed through I2C. > > > > It's working for now because the driver doesn't do any access, so it's > > all great, but as soon as we add support for the other case, then we'll > > have to add a property that states that while it's an i2c device, it > > shouldn't be accessed. > > > > And adding such a property is a compatibility-breaking change. > > Why do you say: > "current binding is for a I2C device that > shouldn't be accessed through I2C" ? > > As a matter of fact, my debug code queries the device ID using > regmap_read() to make sure I set the correct I2C address. Please note: bingdings do not cover the driver at all. The driver might do whatever it wants. The bindings describe the hardware. > > It's not that the device "SHOULD NOT" be accessed. > > It's just that the driver DOES NOT NEED TO access the device, > simply because the default settings work fine. -- With best wishes Dmitry
