On 5/29/19 1:15 PM, Ian Arkver wrote: > Hi Marek, > > On 29/05/2019 12:09, Marek Vasut wrote: >> On 5/29/19 1:04 PM, Ian Arkver wrote: >>> Hi, >>> >>> On 29/05/2019 11:41, Marek Vasut wrote: >>>> On 5/29/19 8:28 AM, Jacopo Mondi wrote: >>>> >>>> [...] >>>> >>>>>>>>> [1] >>>>>>>>> https://www.renesas.com/eu/en/products/audio-video/video-decoders-encoders/video-decoders/device/ISL79987.html >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>>> +Required Properties: >>>>>>>>>> +- compatible: value should be "isil,isl79987" >>>>>>> >>>>>>> And here you might want to have 2 different compatibles for 79987 >>>>>>> and >>>>>>> 79988. >>>>>> >>>>>> The 79988 is not supported yet, do we want to have it in the binding >>>>>> doc? >>>>>> >>>>> >>>>> I got mislead by the isl7998x naming scheme you used... >>>>> >>>>> I would say that's up to you, the two chips seems very similar, >>>>> and it might make sense to provide bindings that support both. At the >>>>> same time, as long as the here defined bindings does not prevent >>>>> future expansions to include the ISL79988, its support could be safely >>>>> post-poned. In that case please s/isl7998x/isl79987/ in this document >>>>> and do not mention BT565 in the description. >>>> >>>> Right >>>> >>>>>> [...] >>>>>> >>>>>>>>> I see from the example you only support one output port? How do >>>>>>>>> you >>>>>>>>> model the input ones. >>>>>>>> >>>>>>>> I don't . Do we model analog inputs now somehow ? >>>>>>> >>>>>>> I really think so, please see: >>>>>>> Documentation/devicetree/bindings/display/connector/analog-tv-connector.txt >>>>>>> >>>>>>> >>>>>>> >>>>>>> And as an example of a board device tree using connectors to model >>>>>>> analog input see how the cvbs input on Salvator-X is described: >>>>>>> >>>>>>> cvbs-in { >>>>>>> compatible = "composite-video-connector"; >>>>>>> label = "CVBS IN"; >>>>>>> >>>>>>> port { >>>>>>> cvbs_con: endpoint { >>>>>>> remote-endpoint = <&adv7482_ain7>; >>>>>>> }; >>>>>>> }; >>>>>>> }; >>>>>>> >>>>>>> I think you should provide 4 input ports, where to connect input >>>>>>> from >>>>>>> the analog connectors, and derive the number of enabled inputs from >>>>>>> the number of endpoints connected to an active remote. >>>>>> >>>>>> Deriving the number of active physical inputs from some existing >>>>>> binding >>>>>> makes sense. >>>>>> >>>>>> However unlike the adv7482, the isl79987 does not support >>>>>> remapping the >>>>>> physical inputs to ADCs in the chip. It does support some >>>>>> remapping of >>>>>> physical inputs to MIPI CSI2 channels, but that's probably not very >>>>>> useful. >>>>>> >>>>> >>>>> I understand, but I will now use against you the argument you have >>>>> correctly pointed out here below that DT should describe hardware, and >>>>> the hardware has indeed 4 input ports.. >>>> >>>> My question here is whether it makes sense to describe the ports >>>> even if >>>> they cannot be muxed to different ADC. Does it ? >>> >>> Each input port can be either differential CVBS or single ended with a >>> 2:1 input select mux. It would be nice to be able to describe this. >> >> Where do you see that ? > > Bits 0 and 1 of each channel page's Differential Clamping Control 4 > (0x39, ISL7998x_REG_Px_DEC_DIFF_CLMP_CTL_4). > > I don't think you change it from the default (single ended on the first > input). I don't, since I have no way to test the differential mode of operation. [...]
