Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <3eba05be-a836-d362-4e06-2d9f102c4113@collabora.com>
Date:   Tue, 4 Apr 2023 19:32:11 +0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.9.0
Subject: Re: [PATCH 1/2] dt-bindings: hwmon: pwm-fan: Convert to DT schema
Content-Language: en-US
To:     Rob Herring <robh@kernel.org>
Cc:     Jean Delvare <jdelvare@suse.com>,
        Guenter Roeck <linux@roeck-us.net>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Heiko Stuebner <heiko@sntech.de>, linux-hwmon@vger.kernel.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org,
        linux-rockchip@lists.infradead.org, kernel@collabora.com
References: <20230403105052.426135-1-cristian.ciocaltea@collabora.com>
 <20230403105052.426135-2-cristian.ciocaltea@collabora.com>
 <20230404141749.GB3793612-robh@kernel.org>
From:   Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
In-Reply-To: <20230404141749.GB3793612-robh@kernel.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 4/4/23 17:17, Rob Herring wrote:
> On Mon, Apr 03, 2023 at 01:50:51PM +0300, Cristian Ciocaltea wrote:
>> Convert the PWM fan bindings to DT schema format.
>>
>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>> ---
>>  .../devicetree/bindings/hwmon/pwm-fan.txt     |  68 +----------
>>  .../devicetree/bindings/hwmon/pwm-fan.yaml    | 109 ++++++++++++++++++
>>  2 files changed, 110 insertions(+), 67 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/hwmon/pwm-fan.yaml
>>

[...]

>> diff --git a/Documentation/devicetree/bindings/hwmon/pwm-fan.yaml b/Documentation/devicetree/bindings/hwmon/pwm-fan.yaml
>> new file mode 100644
>> index 000000000000..448b48ec5d73
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/hwmon/pwm-fan.yaml
>> @@ -0,0 +1,109 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/hwmon/pwm-fan.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Fan connected to PWM lines
>> +
>> +maintainers:
>> +  - Jean Delvare <jdelvare@suse.com>
>> +  - Guenter Roeck <linux@roeck-us.net>
>> +
>> +properties:
>> +  compatible:
>> +    const: pwm-fan
>> +
>> +  cooling-levels:
>> +    description:
>> +      PWM duty cycle values in a range from 0 to 255 which correspond to
> 
> Don't put constraints in plain text:
> 
> items:
>   maximum: 255
> 
>> +      thermal cooling states.
>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> 
> Unfortunately, looks like we've wound up with same property with 2 
> differing types. A problem for another day...
> 
>> +
>> +  fan-supply:
>> +    description: Phandle to the regulator that provides power to the fan.
>> +
>> +  interrupts:
>> +    description:
>> +      This contains an interrupt specifier for each fan tachometer output
>> +      connected to an interrupt source. The output signal must generate a
>> +      defined number of interrupts per fan revolution, which require that
>> +      it must be self resetting edge interrupts.
> 
> How many entries? I'm not sure how more than 1 makes sense.

The 3rd example taken from the original binding uses 2 entries. So far
it seems there are no use cases requiring more than one, so I'm going to
limit this to 5 (the driver doesn't enforce a limit).

>> +
>> +  pulses-per-revolution:
>> +    description:
>> +      Define the number of pulses per fan revolution for each tachometer
>> +      input as an integer (default is 2 interrupts per revolution).
> 
> default: 2
> 
>> +      The value must be greater than zero.
> 
> minimum: 1
> 
> maximum: ??? I assume there's some practical limit here much less than 
> 2^32.

Will set it to 4, as suggested by Guenter.

>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> 
> Isn't this a scalar? 

As in the case of interrupts, the 3rd example uses 2 entries. Will set
the same limit (5).

>> +
>> +  pwms:
>> +    description: The PWM that is used to control the fan.
>> +    maxItems: 1
>> +
>> +  pwm-names: true
>> +
>> +  "#cooling-cells":
>> +    description: The PWM fan can be referenced as a cooling-device.
> 
> Not that useful. What would be is what's in the 2 cells.

Will describe its usage according to thermal-cooling-devices binding.

Thanks for reviewing,
Cristian