linux-kernel - Re: [PATCH 2/5] dt-bindings: Add bindings for the Analog Devices ADP5585

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: Wed, 22 May 2024 09:20:54 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Laurent Pinchart <laurent.pinchart@...asonboard.com>
Cc: linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
 linux-gpio@...r.kernel.org, linux-pwm@...r.kernel.org,
 Alexandru Ardelean <alexandru.ardelean@...log.com>,
 Bartosz Golaszewski <brgl@...ev.pl>, Conor Dooley <conor+dt@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Lee Jones <lee@...nel.org>,
 Linus Walleij <linus.walleij@...aro.org>, Rob Herring <robh@...nel.org>,
 Uwe Kleine-König <ukleinek@...nel.org>
Subject: Re: [PATCH 2/5] dt-bindings: Add bindings for the Analog Devices
 ADP5585

On 22/05/2024 08:57, Krzysztof Kozlowski wrote:
> On 21/05/2024 21:43, Laurent Pinchart wrote:
>> Hi Krzysztof,
>>
>> On Tue, May 21, 2024 at 09:05:50PM +0200, Krzysztof Kozlowski wrote:
>>> On 20/05/2024 21:59, Laurent Pinchart wrote:
>>>> The ADP5585 is a 10/11 input/output port expander with a built in keypad
>>>> matrix decoder, programmable logic, reset generator, and PWM generator.
>>>> These bindings model the device as an MFD, and support the GPIO expander
>>>> and PWM functions.
>>>>
>>>> These bindings support the GPIO and PWM functions.
>>>>
>>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@...asonboard.com>
>>>> ---
>>>> I've limited the bindings to GPIO and PWM as I lack hardware to design,
>>>> implement and test the rest of the features the chip supports.
>>>> ---
>>>>  .../bindings/gpio/adi,adp5585-gpio.yaml       |  36 ++++++
>>>>  .../devicetree/bindings/mfd/adi,adp5585.yaml  | 117 ++++++++++++++++++
>>>>  .../bindings/pwm/adi,adp5585-pwm.yaml         |  35 ++++++
>>>>  MAINTAINERS                                   |   7 ++
>>>>  4 files changed, 195 insertions(+)
>>>>  create mode 100644 Documentation/devicetree/bindings/gpio/adi,adp5585-gpio.yaml
>>>>  create mode 100644 Documentation/devicetree/bindings/mfd/adi,adp5585.yaml
>>>>  create mode 100644 Documentation/devicetree/bindings/pwm/adi,adp5585-pwm.yaml
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/gpio/adi,adp5585-gpio.yaml b/Documentation/devicetree/bindings/gpio/adi,adp5585-gpio.yaml
>>>> new file mode 100644
>>>> index 000000000000..210e4d53e764
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/gpio/adi,adp5585-gpio.yaml
>>>> @@ -0,0 +1,36 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/gpio/adi,adp5585-gpio.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: Analog Devices ADP5585 GPIO Expander
>>>> +
>>>> +maintainers:
>>>> +  - Laurent Pinchart <laurent.pinchart@...asonboard.com>
>>>> +
>>>> +description: |
>>>> +  The Analog Devices ADP5585 has up to 11 GPIOs represented by a "gpio" child
>>>> +  node of the parent MFD device. See
>>>> +  Documentation/devicetree/bindings/mfd/adi,adp5585.yaml for further details as
>>>> +  well as an example.
>>>> +
>>>> +properties:
>>>> +  compatible:
>>>> +    const: adi,adp5585-gpio
>>>> +
>>>> +  gpio-controller: true
>>>> +
>>>> +  '#gpio-cells':
>>>> +    const: 2
>>>> +
>>>> +  gpio-reserved-ranges: true
>>>
>>> There are no resources here, so new compatible is not really warranted.
>>> Squash the node into parent.
>>
>> Child nodes seem (to me) to be the standard way to model functions in
>> MFD devices. Looking at mfd_add_device(), for OF-based systems, the
>> function iterates over child nodes. I don't mind going a different
> 
> Only to assign of node, which could be skipped as well.
> 
>> routes, could you indicate what you have in mind, perhaps pointing to an
>> existing driver as an example ?
> 
> Most of them? OK, let's take the last added driver in MFD directory:
> cirrus,cs42l43
> It has three children and only two nodes, because only these two devices
> actually need/use/benefit the subnodes.

BTW, if these devices have their own I2C addresses or are reusable
blocks, then of course their existence as child nodes is warranted.

Similarly if these devices had additional subnodes, like pinctrl or SPI
controller.

Best regards,
Krzysztof