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: <20231125113617.4d626bb2@jic23-huawei>
Date:   Sat, 25 Nov 2023 11:36:17 +0000
From:   Jonathan Cameron <jic23@...nel.org>
To:     Anshul Dalal <anshulusr@...il.com>
Cc:     linux-kernel@...r.kernel.org, linux-iio@...r.kernel.org,
        devicetree@...r.kernel.org, Conor Dooley <conor+dt@...nel.org>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        linux-kernel-mentees@...ts.linuxfoundation.org,
        Shuah Khan <skhan@...uxfoundation.org>
Subject: Re: [PATCH 1/2] dt-bindings: iio: dac: add MCP4821

On Fri, 17 Nov 2023 13:00:37 +0530
Anshul Dalal <anshulusr@...il.com> wrote:

> Adds support for MCP48xx series of DACs.
> 
> Datasheet:
>   [MCP48x1] https://ww1.microchip.com/downloads/en/DeviceDoc/22244B.pdf
>   [MCP48x2] https://ww1.microchip.com/downloads/en/DeviceDoc/20002249B.pdf
> 
> Signed-off-by: Anshul Dalal <anshulusr@...il.com>
Hi Anshul,

Usually we mark vdd-supply as required given I guess device doesn't work
without a supply. Obviously we don't actually have to provide it in a binding
if the supply is always on and we are fine with a stub regulator being
provided by the regulator subsystem.

There was some discussion about this a while back and conclusion was
mark them required in bindings anyway.  We haven't yet updated this in all
the older IIO bindings and it's a minor thing, but given the build warning
on patch 2 you are going around again so might as well tidy that up!

Jonathan


> ---
>  .../bindings/iio/dac/microchip,mcp4821.yaml   | 63 +++++++++++++++++++
>  1 file changed, 63 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/dac/microchip,mcp4821.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/dac/microchip,mcp4821.yaml b/Documentation/devicetree/bindings/iio/dac/microchip,mcp4821.yaml
> new file mode 100644
> index 000000000000..904de15300bd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/dac/microchip,mcp4821.yaml
> @@ -0,0 +1,63 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/dac/microchip,mcp4821.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Microchip MCP4821 and similar DACs
> +
> +description: |
> +  Supports MCP48x1 (single channel) and MCP48x2 (dual channel) series of DACs.
> +  Device supports simplex communication over SPI in Mode 0,1 and Mode 1,1.
> +
> +  +---------+--------------+-------------+
> +  | Device  |  Resolution  |   Channels  |
> +  |---------|--------------|-------------|
> +  | MCP4801 |     8-bit    |      1      |
> +  | MCP4811 |    10-bit    |      1      |
> +  | MCP4821 |    12-bit    |      1      |
> +  | MCP4802 |     8-bit    |      2      |
> +  | MCP4812 |    10-bit    |      2      |
> +  | MCP4822 |    12-bit    |      2      |
> +  +---------+--------------+-------------+
> +
> +  Datasheet:
> +    MCP48x1: https://ww1.microchip.com/downloads/en/DeviceDoc/22244B.pdf
> +    MCP48x2: https://ww1.microchip.com/downloads/en/DeviceDoc/20002249B.pdf
> +
> +maintainers:
> +  - Anshul Dalal <anshulusr@...il.com>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - microchip,mcp4801
> +      - microchip,mcp4811
> +      - microchip,mcp4821
> +      - microchip,mcp4802
> +      - microchip,mcp4812
> +      - microchip,mcp4822

Whilst I understand the reasoning of keeping these grouped by number of channels,
I'd still rather see them in numeric order here and probably also in the table above.
Given that grouping by resolution rather than channels would also be a valid choice,
I don't see a strong reason to keep them out of order.

Also, manufacturers often get creative with numbering (when they run of out of digits
for example - maybe they'll do a 16 channel variant one day and then be stuck) so
trying to group things is often a loosing game long term!

Jonathan


>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ