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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6fcf4997-9d88-7e86-70f7-52f9d296bc6e@rasmusvillemoes.dk>
Date:   Thu, 4 May 2023 12:08:53 +0200
From:   Rasmus Villemoes <linux@...musvillemoes.dk>
To:     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 <jic23@...nel.org>
Cc:     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 04/05/2023 09.28, Nuno Sá wrote:
> Hi Rasmus,
> 

> So, I'm not really that familiar with this part and, at this stage, I'm being
> lazy to check the datasheet. 

Well, the data sheet is not particularly helpful here, which is why I
ended up with this mess.

> My concern is about breaking some other users...

I highly doubt there are users yet (other than my customer); this
binding+driver implementation only just landed.

> So, does it make any sense for having drive-strength-microamp in a non digital
> input at all?

That's the problem with the data sheet, it doesn't really say that the
DIN_SINK register has any effect whatsoever when the channel function is
set to something other than digital input (either flavor). Perhaps it
does hint that setting it to something non-zero is probably not a good
idea, because DIN_SINK is automatically set to 0 whenever the channel
function is set/changed, so one needs a good reason to change DIN_SINK
afterwards.

We just experimentally found out that when we added the DIN_SINK to fix
the digital input functions, when we got around to testing the
resistance measurement function that ended up broken due to the non-zero
DIN_SINK.

> 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').

Rasmus