| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <327ae9d5-8683-488f-7970-4983e2fec51d@st.com>
Date: Thu, 22 Oct 2020 14:56:17 +0000
From: Hugues FRUCHET <hugues.fruchet@...com>
To: Sakari Ailus <sakari.ailus@...ux.intel.com>,
Jacopo Mondi <jacopo@...ndi.org>
CC: Alexandre TORGUE <alexandre.torgue@...com>,
Mauro Carvalho Chehab <mchehab@...nel.org>,
Hans Verkuil <hverkuil@...all.nl>,
Rob Herring <robh+dt@...nel.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-media@...r.kernel.org" <linux-media@...r.kernel.org>,
"linux-stm32@...md-mailman.stormreply.com"
<linux-stm32@...md-mailman.stormreply.com>,
Alain VOLMAT <alain.volmat@...com>,
Yannick FERTRE <yannick.fertre@...com>,
Philippe CORNU <philippe.cornu@...com>
Subject: Re: [PATCH v4 2/2] media: dt-bindings: media: st,stm32-dcmi: Add
support of BT656
Hi Sakari,
+ Jacopo for his work on ov772x binding related to BT656
On 10/21/20 11:40 PM, Sakari Ailus wrote:
> Hi Hugues,
>
> On Wed, Oct 21, 2020 at 02:24:08PM +0000, Hugues FRUCHET wrote:
>> Hi Sakari,
>>
>> On 10/21/20 3:00 PM, Sakari Ailus wrote:
>>> Hi Hugues,
>>>
>>> On Tue, Oct 20, 2020 at 12:14:49PM +0200, Hugues Fruchet wrote:
>>>> Add support of BT656 parallel bus mode in DCMI.
>>>> This mode is enabled when hsync-active & vsync-active
>>>> fields are not specified.
>>>>
>>>> Signed-off-by: Hugues Fruchet <hugues.fruchet@...com>
>>>> ---
>>>> .../devicetree/bindings/media/st,stm32-dcmi.yaml | 30 ++++++++++++++++++++++
>>>> 1 file changed, 30 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml b/Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml
>>>> index 3fe778c..1ee521a 100644
>>>> --- a/Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml
>>>> +++ b/Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml
>>>> @@ -44,6 +44,36 @@ properties:
>>>> bindings defined in
>>>> Documentation/devicetree/bindings/media/video-interfaces.txt.
>>>>
>>>> + properties:
>>>> + endpoint:
>>>> + type: object
>>>> +
>>>> + properties:
>>>> + bus-width: true
>>>> +
>>>> + hsync-active:
>>>> + description:
>>>> + If both HSYNC and VSYNC polarities are not specified, BT656
>>>> + embedded synchronization is selected.
>>>> + default: 0
>>>> +
>>>> + vsync-active:
>>>> + description:
>>>> + If both HSYNC and VSYNC polarities are not specified, BT656
>>>> + embedded synchronization is selected.
>>>> + default: 0
>>>
>>> Should I understand this as if the polarities were not specified, BT.656
>>> will be used?
>>
>> Yes, this is what is documented in video-interfaces.txt:
>> "
>> Note, that if HSYNC and VSYNC polarities are not specified, embedded
>> synchronization may be required, where supported.
>> "
>> and
>> "
>> /* If hsync-active/vsync-active are missing,
>> embedded BT.656 sync is used */
>> hsync-active = <0>; /* Active low */
>> vsync-active = <0>; /* Active low */
>> "
>> and I found also this in
>> Documentation/devicetree/bindings/media/renesas,vin.yaml
>> "
>> hsync-active:
>> description:
>> If both HSYNC and VSYNC polarities are not specified,
>> embedded
>> synchronization is selected.
>> default: 1
>>
>> vsync-active:
>> description:
>> If both HSYNC and VSYNC polarities are not specified,
>> embedded
>> synchronization is selected.
>> default: 1
>
> Having the defaults leads to somewhat weird behaviour: specifying the
> default value on either property changes the bus type.
>
>> "
>>
>> In the other hand I've found few occurences of "bus-type"
>> (marvell,mmp2-ccic.yaml), it is why I asked you if "bus-type" is the new
>> way to go versus previous way to signal BT656 (without hsync/vsync) ?
>> As explained previously, I prefer this last way for backward compatibility.
>
> If you have a default for bus-type (BT.601), this won't be a problem.
>
> The old DT bindings were somewhat, well, opportunistic. The v4l2-of
> framework-let did its best and sometimes it worked. The behaviour is still
> supported but not encouraged in new bindings.
>
OK, so let's go for the new way.
I've found an interesting patch from Jacopo that is of great help:
https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20200910162055.614089-4-jacopo+renesas@jmondi.org/
Here is a draft proposal before I push a new version, please comment:
properties:
bus-type:
enum: [5, 6]
default: 5
bus-width:
enum: [8, 10, 12, 14]
default: 8
hsync-active:
enum: [0, 1]
default: 0
vsync-active:
enum: [0, 1]
default: 0
pclk-sample:
enum: [0, 1]
default: 0
remote-endpoint: true
allOf:
- if:
properties:
bus-type:
const: 6
then:
properties:
hsync-active: false
vsync-active: false
bus-width:
enum: [8]
required:
- remote-endpoint
unevaluatedProperties: false
Unfortunately, the "default: 5" for bus-type is not working !!
If we don't specify "bus-type" in example, dt_binding_check is failing
as if default was 6, it's hardly understandable (see below) !
port {
dcmi_0: endpoint {
remote-endpoint = <&ov5640_0>;
bus-width = <10>;
hsync-active = <0>;
vsync-active = <0>;
pclk-sample = <1>;
};
=> this should be OK but error claimed:
DTC
Documentation/devicetree/bindings/media/st,stm32-dcmi.example.dt.yaml
CHECK
Documentation/devicetree/bindings/media/st,stm32-dcmi.example.dt.yaml
Documentation/devicetree/bindings/media/st,stm32-dcmi.example.dt.yaml:
dcmi@...06000: port:endpoint:vsync-active: False schema does not allow [[0]]
dcmi@...06000: port:endpoint:hsync-active: False schema does not allow [[0]]
dcmi@...06000: port:endpoint:bus-width:0:0: 10 is not one of [8]
From schema: Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml
=> if "bus-type" is explicitly set to 5, all is fine (see below) !
port {
dcmi_0: endpoint {
remote-endpoint = <&ov5640_0>;
bus-type = <5>;
bus-width = <10>;
hsync-active = <0>;
vsync-active = <0>;
pclk-sample = <1>;
};
};
DTC
Documentation/devicetree/bindings/media/st,stm32-dcmi.example.dt.yaml
CHECK
Documentation/devicetree/bindings/media/st,stm32-dcmi.example.dt.yaml
~/.../media_tree$
>>
>>
>> The bindings previously documented BT.601 (parallel) only, so
>>> it was somewhat ambigious to begin with. Is there a risk of interpreting
>>> old BT.601 bindings as BT.656?
>> I don't think so.
>>
>> With bus-type property, I believe you could
>>> avoid at least that risk.
>> yes but as explained, I'll prefer not to amend current boards device
>> tree files.
>
> I don't think it matters from this point of view --- you can have a
> default bus-type.
>
>>
>>>
>>> Also not specifying at least one of the default values leads to BT.656
>>> without bus-type. That could be addressed by removing the defaults.
>>>
>> I'm new to yaml, I've taken that from renesas,vin.yaml. Should I just
>> drop the "default: 1" lines ?
>
> That's one option, yes. Then you have to have those for BT.601 and it's no
> longer ambiguous.
>
BR,
Hugues.
Powered by blists - more mailing lists