Re: [PATCH v4 1/2] Add DT bindings YAML schema for PWM fan controller of LGM SoC

2020-07-13 Thread Tanwar, Rahul 


Hi Rob,

On 14/7/2020 12:46 am, Rob Herring wrote:
> On Tue, Jun 30, 2020 at 03:55:31PM +0800, Rahul Tanwar wrote:
>> Intel's LGM(Lightning Mountain) SoC contains a PWM fan controller
>> which is only used to control the fan attached to the system. This
>> PWM controller does not have any other consumer other than fan.
>> Add DT bindings documentation for this PWM fan controller.
>>
>> Signed-off-by: Rahul Tanwar 
>> ---
>>  .../devicetree/bindings/pwm/intel,lgm-pwm.yaml | 50 
>> ++
>>  1 file changed, 50 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/pwm/intel,lgm-pwm.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/pwm/intel,lgm-pwm.yaml 
>> b/Documentation/devicetree/bindings/pwm/intel,lgm-pwm.yaml
>> new file mode 100644
>> index ..120bf3d85a24
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/pwm/intel,lgm-pwm.yaml
>> @@ -0,0 +1,50 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/pwm/intel,lgm-pwm.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: LGM SoC PWM fan controller
>> +
>> +maintainers:
>> +  - Rahul Tanwar 
>> +
>> +properties:
>> +  compatible:
>> +const: intel,lgm-pwm
>> +
>> +  reg:
>> +maxItems: 1
>> +
>> +  clocks:
>> +maxItems: 1
>> +
>> +  resets:
>> +maxItems: 1
>> +
>> +  intel,fan-wire:
>> +$ref: '/schemas/types.yaml#/definitions/uint32'
>> +description: Specifies fan mode. Default when unspecified is 2.
>> +
>> +  intel,max-rpm:
>> +$ref: '/schemas/types.yaml#/definitions/uint32'
>> +description:
>> +  Specifies maximum RPM of fan attached to the system.
>> +  Default when unspecified is 4000.
> These are properties of the fan, not the PWM. And probably if you 
> need these properties then others would too, so they should be 
> common. Look at the pwm-fan.txt binding.

I checked pwm-fan.txt. I don't find any common property which matches
our fan properties of fan wire mode & max-rpm. Are you suggesting to
add these properties additionally in pwm-fan.txt as new common properties
and then use newly added generic name in our driver?

Also, we have a dedicated PWM fan controller. So do you think this driver
belongs to drivers/hwmon instead of drivers/pwm? Thanks.

Regards,
Rahul 

Re: [PATCH v4 1/2] Add DT bindings YAML schema for PWM fan controller of LGM SoC

2020-07-13 Thread Rob Herring 
On Tue, Jun 30, 2020 at 03:55:31PM +0800, Rahul Tanwar wrote:
> Intel's LGM(Lightning Mountain) SoC contains a PWM fan controller
> which is only used to control the fan attached to the system. This
> PWM controller does not have any other consumer other than fan.
> Add DT bindings documentation for this PWM fan controller.
> 
> Signed-off-by: Rahul Tanwar 
> ---
>  .../devicetree/bindings/pwm/intel,lgm-pwm.yaml | 50 
> ++
>  1 file changed, 50 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pwm/intel,lgm-pwm.yaml
> 
> diff --git a/Documentation/devicetree/bindings/pwm/intel,lgm-pwm.yaml 
> b/Documentation/devicetree/bindings/pwm/intel,lgm-pwm.yaml
> new file mode 100644
> index ..120bf3d85a24
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pwm/intel,lgm-pwm.yaml
> @@ -0,0 +1,50 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pwm/intel,lgm-pwm.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: LGM SoC PWM fan controller
> +
> +maintainers:
> +  - Rahul Tanwar 
> +
> +properties:
> +  compatible:
> +const: intel,lgm-pwm
> +
> +  reg:
> +maxItems: 1
> +
> +  clocks:
> +maxItems: 1
> +
> +  resets:
> +maxItems: 1
> +
> +  intel,fan-wire:
> +$ref: '/schemas/types.yaml#/definitions/uint32'
> +description: Specifies fan mode. Default when unspecified is 2.
> +
> +  intel,max-rpm:
> +$ref: '/schemas/types.yaml#/definitions/uint32'
> +description:
> +  Specifies maximum RPM of fan attached to the system.
> +  Default when unspecified is 4000.

These are properties of the fan, not the PWM. And probably if you 
need these properties then others would too, so they should be 
common. Look at the pwm-fan.txt binding.

Rob

[PATCH v4 1/2] Add DT bindings YAML schema for PWM fan controller of LGM SoC

2020-06-30 Thread Rahul Tanwar 
Intel's LGM(Lightning Mountain) SoC contains a PWM fan controller
which is only used to control the fan attached to the system. This
PWM controller does not have any other consumer other than fan.
Add DT bindings documentation for this PWM fan controller.

Signed-off-by: Rahul Tanwar 
---
 .../devicetree/bindings/pwm/intel,lgm-pwm.yaml | 50 ++
 1 file changed, 50 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pwm/intel,lgm-pwm.yaml

diff --git a/Documentation/devicetree/bindings/pwm/intel,lgm-pwm.yaml 
b/Documentation/devicetree/bindings/pwm/intel,lgm-pwm.yaml
new file mode 100644
index ..120bf3d85a24
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/intel,lgm-pwm.yaml
@@ -0,0 +1,50 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pwm/intel,lgm-pwm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: LGM SoC PWM fan controller
+
+maintainers:
+  - Rahul Tanwar 
+
+properties:
+  compatible:
+const: intel,lgm-pwm
+
+  reg:
+maxItems: 1
+
+  clocks:
+maxItems: 1
+
+  resets:
+maxItems: 1
+
+  intel,fan-wire:
+$ref: '/schemas/types.yaml#/definitions/uint32'
+description: Specifies fan mode. Default when unspecified is 2.
+
+  intel,max-rpm:
+$ref: '/schemas/types.yaml#/definitions/uint32'
+description:
+  Specifies maximum RPM of fan attached to the system.
+  Default when unspecified is 4000.
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - resets
+
+additionalProperties: false
+
+examples:
+  - |
+pwm: pwm@e0d0 {
+compatible = "intel,lgm-pwm";
+reg = <0xe0d0 0x30>;
+clocks = < 126>;
+resets = < 0x30 21>;
+};
-- 
2.11.0