On Fri, Nov 18, 2022 at 9:46 AM Hans Verkuil <hverkuil@xxxxxxxxx> wrote: > > On 11/18/22 16:20, Rob Herring wrote: > > +Bartosz > > > > On Fri, Nov 18, 2022 at 4:34 AM Hans Verkuil <hverkuil@xxxxxxxxx> wrote: > >> > >> On 11/16/22 21:07, Rob Herring wrote: > >>> On Fri, Nov 11, 2022 at 02:29:04PM +0100, Erling Ljunggren wrote: > >>>> Add devicetree bindings for new cat24c208 EDID EEPROM driver. > >>>> > >>>> Signed-off-by: Erling Ljunggren <hljunggr@xxxxxxxxx> > >>>> --- > >>>> .../bindings/media/i2c/onnn,cat24c208.yaml | 46 +++++++++++++++++++ > >>>> 1 file changed, 46 insertions(+) > >>>> create mode 100644 Documentation/devicetree/bindings/media/i2c/onnn,cat24c208.yaml > >>>> > >>>> diff --git a/Documentation/devicetree/bindings/media/i2c/onnn,cat24c208.yaml b/Documentation/devicetree/bindings/media/i2c/onnn,cat24c208.yaml > >>>> new file mode 100644 > >>>> index 000000000000..492eecb3ab7c > >>>> --- /dev/null > >>>> +++ b/Documentation/devicetree/bindings/media/i2c/onnn,cat24c208.yaml > >>>> @@ -0,0 +1,46 @@ > >>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > >>>> +%YAML 1.2 > >>>> +--- > >>>> +$id: http://devicetree.org/schemas/media/i2c/onnn,cat24c208.yaml# > >>>> +$schema: http://devicetree.org/meta-schemas/core.yaml# > >>>> + > >>>> +title: ON Semiconductor CAT24C208 EDID EEPROM driver > >>>> + > >>>> +maintainers: > >>>> + - Hans Verkuil <hverkuil-cisco@xxxxxxxxx> > >>>> + > >>>> +description: | > >>>> + CAT24C208 is a dual port i2c EEPROM designed for EDID storage. > >>>> + > >>>> +properties: > >>>> + compatible: > >>>> + const: onnn,cat24c208 > >>>> + > >>>> + reg: > >>>> + maxItems: 1 > >>>> + > >>>> + input-connector: > >>>> + description: | > >>>> + Phandle to the video input connector, used to find > >>>> + the HPD gpio and the connector label, both optional. > >>>> + $ref: /schemas/types.yaml#/definitions/phandle > >>> > >>> The binding and driver feel the wrong way around to me. It seems > >>> like you should have a driver for the connector and it needs HPD GPIO, > >>> label, and EEPROM. The driver instead looks mostly like an EEPROM driver > >>> that hooks into a few connector properties. > >> > >> A device like this is typically used next to an HDMI receiver: the DDC > >> lines and the HPD line are connected to the EDID EEPROM, and the video > >> is handled by the HDMI receiver. > >> > >> Most HDMI receivers will have the EDID part integrated into the chip itself > >> (see e.g. the adv7604 driver), but that doesn't have to be the case. The EDID > >> can be completely separate, it doesn't matter for the receiver part. > >> > >> In our specific use-case there isn't even an HDMI receiver since the HDMI > >> video is passed through and this EDID EEPROM is used to help debug HDMI > >> issues by presenting custom EDIDs, similar to something like this: > >> > >> https://www.amazon.com/dp/B0722NVQHX > >> > >> The HPD line is controlled by the EDID part since it has to be low if there > >> is no EDID or pulled low for at least 100ms if the EDID is being modified. > > > > There is no HPD control on the EEPROM itself. So HPD does not belong > > in the EEPROM node. That is the fundamental problem with the binding. > > Perhaps there is a terminology issue here: the input-connector phandle > points to a connector (an hdmi-connector in our case, but it can also > be a dvi-connector for example). The driver gets the HPD gpio from > the connector node. Perhaps the example should be extended with the > hdmi-connector node? You do need to define a connector node. But 'hdmi-connector' is taken and means an hdmi output (though maybe it got used by someone for an input?). The input side has and does different things, so we should define a new compatible rather than complicate our lives trying to reuse and extend the current one. So you need 'hdmi-in-connector' compatible or something like that. You can possibly add this to the existing hdmi-connector schema depending on how much the existing properties may apply. For example, ddc-i2c-bus points to an i2c slave for a s/w EDID? > Or do you mean that the hdmi-connector node should point to the EDID > driver? DT does not have drivers, but yes, the HDMI connector node should point to the EEPROM h/w device. Rob
