* [PATCH v4 3/5] dt-bindings: media: add cat24c208 bindings
[not found] <20221111132906.2212662-1-hljunggr@cisco.com>
@ 2022-11-11 13:29 ` Erling Ljunggren
2022-11-16 20:07 ` Rob Herring
0 siblings, 1 reply; 6+ messages in thread
From: Erling Ljunggren @ 2022-11-11 13:29 UTC (permalink / raw)
To: linux-media; +Cc: Erling Ljunggren, devicetree
Add devicetree bindings for new cat24c208 EDID EEPROM driver.
Signed-off-by: Erling Ljunggren <hljunggr@cisco.com>
---
.../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@xs4all.nl>
+
+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
+
+required:
+ - compatible
+ - reg
+ - input-connector
+
+additionalProperties: false
+
+examples:
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ cat24c208@31 {
+ compatible = "onnn,cat24c208";
+ reg = <0x31>;
+ input-connector = <&hdmi_connector_in>;
+ };
+ };
--
2.38.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v4 3/5] dt-bindings: media: add cat24c208 bindings
2022-11-11 13:29 ` [PATCH v4 3/5] dt-bindings: media: add cat24c208 bindings Erling Ljunggren
@ 2022-11-16 20:07 ` Rob Herring
2022-11-18 10:34 ` Hans Verkuil
0 siblings, 1 reply; 6+ messages in thread
From: Rob Herring @ 2022-11-16 20:07 UTC (permalink / raw)
To: Erling Ljunggren; +Cc: linux-media, devicetree
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@cisco.com>
> ---
> .../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@xs4all.nl>
> +
> +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.
Reading the datasheet, I don't see anything special about accessing the
EEPROM from the host (DSP) side. Wouldn't the default at24 driver work?
It exposes regmap and nvmem.
Rob
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 3/5] dt-bindings: media: add cat24c208 bindings
2022-11-16 20:07 ` Rob Herring
@ 2022-11-18 10:34 ` Hans Verkuil
2022-11-18 15:20 ` Rob Herring
0 siblings, 1 reply; 6+ messages in thread
From: Hans Verkuil @ 2022-11-18 10:34 UTC (permalink / raw)
To: Rob Herring, Erling Ljunggren; +Cc: linux-media, devicetree
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@cisco.com>
>> ---
>> .../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@xs4all.nl>
>> +
>> +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.
>
> Reading the datasheet, I don't see anything special about accessing the
> EEPROM from the host (DSP) side. Wouldn't the default at24 driver work?
> It exposes regmap and nvmem.
No. It is not a regular EEPROM, it is dedicated to store EDIDs. It has to
correctly toggle the HPD line and inform other drivers (specifically HDMI CEC)
of EDID updates.
I don't see how the at24 could work: besides the eeprom i2c address it has to
deal with two additional i2c devices: the segment address and the config
address of the device itself. Writing to the eeprom from the host side requires
a write to the segment address followed by a write to the eeprom part itself,
and that's not something the at24 can do. And it is also something very specific
to the VESA E-DDC standard (freely downloadable from vesa.org).
Note that historically the first EDID EEPROMs most likely did work like the
at24, but as EDID sizes grew beyond 256 bytes the E-DDC standard was created
and that departed from the normal EEPROMs.
Regards,
Hans
>
> Rob
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 3/5] dt-bindings: media: add cat24c208 bindings
2022-11-18 10:34 ` Hans Verkuil
@ 2022-11-18 15:20 ` Rob Herring
2022-11-18 15:46 ` Hans Verkuil
0 siblings, 1 reply; 6+ messages in thread
From: Rob Herring @ 2022-11-18 15:20 UTC (permalink / raw)
To: Hans Verkuil, Bartosz Golaszewski
Cc: Erling Ljunggren, linux-media, devicetree
+Bartosz
On Fri, Nov 18, 2022 at 4:34 AM Hans Verkuil <hverkuil@xs4all.nl> 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@cisco.com>
> >> ---
> >> .../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@xs4all.nl>
> >> +
> >> +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.
We've always started bindings without connector nodes and just shove
properties into whatever node we do have. Then things evolve to be
more complicated with different h/w and that doesn't work. Start with
a connector even if you think it is overkill.
> > Reading the datasheet, I don't see anything special about accessing the
> > EEPROM from the host (DSP) side. Wouldn't the default at24 driver work?
> > It exposes regmap and nvmem.
>
> No. It is not a regular EEPROM, it is dedicated to store EDIDs. It has to
> correctly toggle the HPD line and inform other drivers (specifically HDMI CEC)
> of EDID updates.
>
> I don't see how the at24 could work: besides the eeprom i2c address it has to
> deal with two additional i2c devices: the segment address and the config
> address of the device itself. Writing to the eeprom from the host side requires
> a write to the segment address followed by a write to the eeprom part itself,
> and that's not something the at24 can do. And it is also something very specific
> to the VESA E-DDC standard (freely downloadable from vesa.org).
The addressing is different on the DSP (as the datasheet calls it)
side compared to the DDC side which has the EDID_SEL signal. Linux
only sees the DSP side, right? Looking at it a bit more, it looks like
the segment addressing is different from at24 which uses an i2c
address per segment. It is similar to ee1004 SPD where the segment is
selected by a write to a 2nd i2c address.
> Note that historically the first EDID EEPROMs most likely did work like the
> at24, but as EDID sizes grew beyond 256 bytes the E-DDC standard was created
> and that departed from the normal EEPROMs.
What happens if/when you have more than 1 part to support? Why not
provide a regmap or nvmem to the EDID storage and then the backend
device can be anything. I could imagine we could have a s/w
implementation.
Anyways, I don't really care if you do any of that now, but I still
think the binding is backwards. It's the connector you are managing,
not just an EEPROM, so you should bind to the connector and from there
gather all the resources you need to do that (EEPROM, HPD).
Rob
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 3/5] dt-bindings: media: add cat24c208 bindings
2022-11-18 15:20 ` Rob Herring
@ 2022-11-18 15:46 ` Hans Verkuil
2022-11-18 17:12 ` Rob Herring
0 siblings, 1 reply; 6+ messages in thread
From: Hans Verkuil @ 2022-11-18 15:46 UTC (permalink / raw)
To: Rob Herring, Bartosz Golaszewski
Cc: Erling Ljunggren, linux-media, devicetree
On 11/18/22 16:20, Rob Herring wrote:
> +Bartosz
>
> On Fri, Nov 18, 2022 at 4:34 AM Hans Verkuil <hverkuil@xs4all.nl> 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@cisco.com>
>>>> ---
>>>> .../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@xs4all.nl>
>>>> +
>>>> +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?
Or do you mean that the hdmi-connector node should point to the EDID
driver? In other words, a new property has to be added there to support
a reference to an EDID device (the hdmi connector bindings are currently written
for HDMI output and never used with HDMI input drivers).
Regards,
Hans
>
> We've always started bindings without connector nodes and just shove
> properties into whatever node we do have. Then things evolve to be
> more complicated with different h/w and that doesn't work. Start with
> a connector even if you think it is overkill.
>
>>> Reading the datasheet, I don't see anything special about accessing the
>>> EEPROM from the host (DSP) side. Wouldn't the default at24 driver work?
>>> It exposes regmap and nvmem.
>>
>> No. It is not a regular EEPROM, it is dedicated to store EDIDs. It has to
>> correctly toggle the HPD line and inform other drivers (specifically HDMI CEC)
>> of EDID updates.
>>
>> I don't see how the at24 could work: besides the eeprom i2c address it has to
>> deal with two additional i2c devices: the segment address and the config
>> address of the device itself. Writing to the eeprom from the host side requires
>> a write to the segment address followed by a write to the eeprom part itself,
>> and that's not something the at24 can do. And it is also something very specific
>> to the VESA E-DDC standard (freely downloadable from vesa.org).
>
> The addressing is different on the DSP (as the datasheet calls it)
> side compared to the DDC side which has the EDID_SEL signal. Linux
> only sees the DSP side, right? Looking at it a bit more, it looks like
> the segment addressing is different from at24 which uses an i2c
> address per segment. It is similar to ee1004 SPD where the segment is
> selected by a write to a 2nd i2c address.
>
>> Note that historically the first EDID EEPROMs most likely did work like the
>> at24, but as EDID sizes grew beyond 256 bytes the E-DDC standard was created
>> and that departed from the normal EEPROMs.
>
> What happens if/when you have more than 1 part to support? Why not
> provide a regmap or nvmem to the EDID storage and then the backend
> device can be anything. I could imagine we could have a s/w
> implementation.
>
> Anyways, I don't really care if you do any of that now, but I still
> think the binding is backwards. It's the connector you are managing,
> not just an EEPROM, so you should bind to the connector and from there
> gather all the resources you need to do that (EEPROM, HPD).
>
> Rob
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 3/5] dt-bindings: media: add cat24c208 bindings
2022-11-18 15:46 ` Hans Verkuil
@ 2022-11-18 17:12 ` Rob Herring
0 siblings, 0 replies; 6+ messages in thread
From: Rob Herring @ 2022-11-18 17:12 UTC (permalink / raw)
To: Hans Verkuil
Cc: Bartosz Golaszewski, Erling Ljunggren, linux-media, devicetree
On Fri, Nov 18, 2022 at 9:46 AM Hans Verkuil <hverkuil@xs4all.nl> wrote:
>
> On 11/18/22 16:20, Rob Herring wrote:
> > +Bartosz
> >
> > On Fri, Nov 18, 2022 at 4:34 AM Hans Verkuil <hverkuil@xs4all.nl> 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@cisco.com>
> >>>> ---
> >>>> .../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@xs4all.nl>
> >>>> +
> >>>> +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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-11-18 17:13 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20221111132906.2212662-1-hljunggr@cisco.com>
2022-11-11 13:29 ` [PATCH v4 3/5] dt-bindings: media: add cat24c208 bindings Erling Ljunggren
2022-11-16 20:07 ` Rob Herring
2022-11-18 10:34 ` Hans Verkuil
2022-11-18 15:20 ` Rob Herring
2022-11-18 15:46 ` Hans Verkuil
2022-11-18 17:12 ` Rob Herring
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).