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

[Krzysztof Kozlowski <krzk@kernel.org>]

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@ideasonboard.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@ideasonboard.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