linux-kernel - Re: [PATCH] iio: addac: ad74413: don't set DIN_SINK for functions other than digital input

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] [day] [month] [year] [list]

Message-ID: <20230528201301.68b31bca@jic23-huawei>
Date:   Sun, 28 May 2023 20:13:01 +0100
From:   Jonathan Cameron <jic23@...nel.org>
To:     Rasmus Villemoes <linux@...musvillemoes.dk>
Cc:     Nuno Sá <noname.nuno@...il.com>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Michael Hennerich <Michael.Hennerich@...log.com>,
        Cosmin Tanislav <cosmin.tanislav@...log.com>,
        Jonathan Cameron <Jonathan.Cameron@...wei.com>,
        linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] iio: addac: ad74413: don't set DIN_SINK for functions
 other than digital input

On Mon, 22 May 2023 10:44:11 +0200
Rasmus Villemoes <linux@...musvillemoes.dk> wrote:

> On 06/05/2023 20.16, Jonathan Cameron wrote:
> > On Thu, 4 May 2023 12:08:53 +0200
> > Rasmus Villemoes <linux@...musvillemoes.dk> wrote:
> >   
> >> On 04/05/2023 09.28, Nuno Sá wrote:  
> 
> >>> Can anyone have a working device by specifying that dt parameter
> >>> on a non digital channel (or expect something from having that parameter set)?
> >>> Or the only effect is to actually have some functions misbehaving?    
> >>
> >> The data sheet doesn't say that the DIN_SINK should have any effect for
> >> other functions, so I'm pretty sure it's only the latter: some functions
> >> misbehave.
> >>  
> >>> On the driver side, if it's never right to have
> >>> these settings together, then the patch is valid since if someone has this, his
> >>> configuration is broken anyways (maybe that's also a valid point for the
> >>> bindings)...    
> >>
> >> Yes, I do believe that it's a broken description (whether or not the
> >> bindings specify that), and drivers don't need to go out of their way to
> >> validate or fixup such brokenness. But in this particular case, there's
> >> really no extra burden on the driver to not put garbage in DIN_SINK when
> >> a not-digital-input function has been chosen (the patch is a two-liner
> >> with 'git show -w').  
> > 
> > If we can tighten the DT binding to rule out something that should not be
> > set than that would be good.  Tightening bindings is fine - we don't mind
> > validation of bindings failing on peoples DTs as long as we didn't 'break'
> > them actually working.  
> 
> Well, I'm afraid I don't have any idea how to spell that constraint in
> the yaml-language (help appreciated).

Lots of examples in tree of this sort of thing.   Look for a 
: false with something other than additionalProperties or unevaluatedProperties
Documentation/devicetree/bindings/iio/adc/adi,ad7476.yaml
for example.

In short you have an allOf block containing a list of rules, one of which
is a match on particular conditions to set particular properties to 'false'
which means that any attempt to have them set when that condition is met
results in an error from the dts checking scripts.

> 
> And I assume a dt binding update would be a separate patch anyway, so
> could you please consider applying this patch?
Fair enough. Applied to the fixes-togreg branch of iio.git.

Thanks,

Jonathan

> 
> Thanks,
> Rasmus
>