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]
Message-ID: <20221214205556.jdbcv3e4fkvm7f3y@SoMainline.org>
Date:   Wed, 14 Dec 2022 21:55:56 +0100
From:   Marijn Suijten <marijn.suijten@...ainline.org>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Cc:     phone-devel@...r.kernel.org, ~postmarketos/upstreaming@...ts.sr.ht,
        AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@...ainline.org>,
        Konrad Dybcio <konrad.dybcio@...ainline.org>,
        Martin Botka <martin.botka@...ainline.org>,
        Jami Kettunen <jami.kettunen@...ainline.org>,
        Jonathan Cameron <jic23@...nel.org>,
        Andy Gross <agross@...nel.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio <konrad.dybcio@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH] arm64: dts: qcom: Use labels with generic node names
 for ADC channels

On 2022-12-12 09:48:26, Krzysztof Kozlowski wrote:
> On 10/12/2022 17:54, Marijn Suijten wrote:
> > On 2022-12-10 12:02:03, Krzysztof Kozlowski wrote:
> >> On 09/12/2022 22:53, Marijn Suijten wrote:
> >>> As discussed in [1] the DT should use labels to describe ADC
> >>> channels, with generic node names, since the IIO drivers now
> >>> moved to the fwnode API where node names include the `@xx`
> >>> address suffix.
> >>> 
> >>> Especially for the ADC5 driver that uses extend_name - which
> >>> cannot be removed for compatibility reasons - this results in
> >>> sysfs files with the @xx name that wasn't previously present, and
> >>> leads to an unpleasant file-browsing experience.
> >>> 
> >>> Also remove all the unused channel labels in pm660.dtsi.
> >>> 
> >>> [1]:
> >>> https://lore.kernel.org/linux-arm-msm/20221106193018.270106-1-marijn.suijten@somainline.org/T/#u
> >>>
> >>>
> >>> 
> Signed-off-by: Marijn Suijten <marijn.suijten@...ainline.org>
> >> 
> >> The talk was in context of bindings, not about changing all
> >> existing users thus affecting DTS.
> > 
> > And as a consequence, DTS.  The already-merged transition from OF to 
> > fwnode resulted in `@xx` to be included in the ADC channel name - and
> > in the case of ADC5 even in sysfs filenames - so this seems like a 
> > necessary change to make.
> > 
> > At the very least I would have changed the bindings submitted or 
> > co-authored /by myself/ since I initially decided to rely on this
> > (now obviously) wrong behaviour, and should have used labels from the
> > get go.
> > 
> >> What's more, to me "skin-temp-thermistor" is quite generic name,
> >> maybe "thermistor" would be more and reflects the purpose of the
> >> node, so it was more or less fine.
> > 
> > Are you suggesting to not use "adc-chan", but "thermistor" as node
> > name (and still use skin_temp as label)?
> 
> No, I am just saying that some of the names were correct, so the
> reasoning in commit msg is not entirely accurate.

The commit msg doesn't state that any of these names was wrong, rather
that the label property should be used to convey a name to the driver.

If you think multiple /node names/ are acceptable, please discuss in
that direction.  Do we use a predetermined set of names (adc-chan@xx,
thermistor@xx, ...) or do you have another suggestion?

> > Or to keep the fully-written-out "thermistor" word in the label?
> 
> No, I don't refer to labels. Labels don't matter, they are being removed
> entirely during DTS build.

Ugh, I knew this was becoming a problem, even in an earlier IIO
discussions with Jonathan it was sometimes hard to keep apart:

I am /not/ referring to DTS labels, but to the `label =` node
attribute/property, which is read by the driver (which isn't removed,
how else would the driver fish this value out at runtime...).

After all none of this patch has any DTS labels, in fact a bunch of
unused ones in pm660 have been removed... but this patch /does add/ the
`label =` property to many nodes.

> >> Anyway I am against such changes without expressing it in the
> >> bindings.
> > 
> > As expressed in [1] I suggested and am all for locking this change
> > in via bindings, and you are right to expect that to have gone paired
> > with this patch.
> 
> Yes, I expect such changes to have both binding and DTS change together.
> 
> > 
> > I'll submit that as the leading patch to this in v2, with the
> > wildcard pattern changed to adc-chan (or something else pending the
> > discussion above), and should I then also require the label property
> > via `label: true`?
> 
> I don't think label is required.

My commit message explains why the ADC5/VADC driver should really
receive a label property instead of using the node name.

- Marijn

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ