Re: [PATCH v4 3/5] dt-bindings: media: add cat24c208 bindings

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux