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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Mon, 12 Feb 2024 09:24:13 +0100
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc: devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 Miguel Ojeda <ojeda@...nel.org>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
 Conor Dooley <conor+dt@...nel.org>, Robin van der Gracht
 <robin@...tonic.nl>, Paul Burton <paulburton@...nel.org>,
 Geert Uytterhoeven <geert+renesas@...der.be>
Subject: Re: [PATCH v1 14/15] dt-bindings: auxdisplay: Add Maxim MAX6958/6959

On 09/02/2024 14:59, Andy Shevchenko wrote:
> On Fri, Feb 09, 2024 at 09:02:44AM +0100, Krzysztof Kozlowski wrote:
>> On 08/02/2024 19:48, Andy Shevchenko wrote:
>>> Add initial device tree documentation for Maxim MAX6958/6959.
> 
> ...
> 
>> Please describe the device, e.g. bus/interface.
> 
> OK.
> 
> ...
> 
>>> +properties:
>>> +  compatible:
>>> +    const: maxim,max6959
>>
>> Your title said also max6958, so I would expect it to be here as well.
>> Cam be followed by 6959 fallback compatible, if they are compatible.
> 
> Same question as I asked before, why should we have them separated?
> The hardware features can be autodetected. What's the reason for (unneeded
> in my opinion and duplicative) compatible?

And which part of device description in the binding, or at least commit
msg but better description, explained it?

For every unexplained deviation from common rules - and documenting
compatibles is explicitly asked in writing bindings document - you will
get questions from reviewers...

Please add this information to description.

> 
> ...
> 
>>> +  reg:
>>> +    maxItems: 1
>>
>> No power supplies? No reset pins?
> 
> No power supplies, no reset pins. At least there is no as such in
> the datasheet. Do you see them there?

How do I know? I don't have datasheets and I don't have really time to
investigate each datasheet of every device people send bindings for.
Several people make mistakes of not putting such stuff because "driver
does not need it", so how can I know that here it was not the case?

> 
> ...
> 
>>> +examples:
>>> +  - |
>>> +    i2c {
>>> +            #address-cells = <1>;
>>
>> Use 4 spaces for example indentation. 2 is also fine.
> 
> Sure. Btw, this is copy&pasted from the existing YAML. Are you going to
> fix them?

I fixed several of them. At some point I might fix all of them, but
that's lower priority. I wished I have all the time for this :)

> 
>>> +            #size-cells = <0>;
>>> +
>>> +            max6959: max6959@38 {
>>
>> Node names should be generic. See also an explanation and list of
> 
> (Same remark: it's a pattern from the existing code. Are you going to fix
>  that?)
> 

Same answer.

Best regards,
Krzysztof


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ