On 03/05/2023 10:26, Naresh Solanki wrote: > Hi Krzysztof, > > On 24-04-2023 03:23 pm, Naresh Solanki wrote: >> Hi Krzysztof, >> >> On 24-04-2023 03:12 pm, Krzysztof Kozlowski wrote: >>> On 24/04/2023 11:18, Naresh Solanki wrote: >>> >>>>>> Changes in V2: >>>>>> - Update subject >>>>>> - Drop blank lines >>>>>> --- >>>>>> .../bindings/hwmon/maxim,max6639.yaml | 52 >>>>>> +++++++++++++++++++ >>>>>> 1 file changed, 52 insertions(+) >>>>>> create mode 100644 >>>>>> Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml >>>>>> >>>>>> diff --git >>>>>> a/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml >>>>>> b/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml >>>>>> new file mode 100644 >>>>>> index 000000000000..1aaedfd7cee0 >>>>>> --- /dev/null >>>>>> +++ b/Documentation/devicetree/bindings/hwmon/maxim,max6639.yaml >>>>>> @@ -0,0 +1,52 @@ >>>>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >>>>>> +%YAML 1.2 >>>>>> +--- >>>>>> +$id: http://devicetree.org/schemas/hwmon/maxim,max6639.yaml# >>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>>>>> + >>>>>> +title: Maxim max6639 >>>>> >>>>> What is this device? fan controller? >>>> Yes Fan controller. >>>> Do you want me to update the title here as: >>>> "Maxim MAC6639 2 channel fan controller & temperature monitor" ? >>> >>> Enough would be: >>> Maxim MAX6639 Fan Controller >> Ack >>> >>> >>>> >>>>> >>>>>> + >>>>>> +maintainers: >>>>>> + - Naresh Solanki <Naresh.Solanki@xxxxxxxxxxxxx> >>>>>> + >>>>>> +description: | >>>>>> + The MAX6639 is a 2-channel temperature monitor with dual, >>>>>> automatic, PWM >>>>>> + fan-speed controller. It monitors its own temperature and one >>>>>> external >>>>>> + diode-connected transistor or the temperatures of two external >>>>>> diode-connected >>>>>> + transistors, typically available in CPUs, FPGAs, or GPUs. >>>>>> + >>>>>> + Datasheets: >>>>>> + https://datasheets.maximintegrated.com/en/ds/MAX6639-MAX6639F.pdf >>>>>> + >>>>>> +properties: >>>>>> + compatible: >>>>>> + enum: >>>>>> + - maxim,max6639 >>>>>> + >>>>>> + reg: >>>>>> + maxItems: 1 >>>>>> + >>>>>> + '#address-cells': >>>>>> + const: 1 >>>>>> + >>>>>> + '#size-cells': >>>>>> + const: 0 >>>>> >>>>> Why do you need these two properties? >>>> Ack. Will remove them. >>>>> >>>>> Anyway, the binding looks incomplete. Where are the supplies? >>>>> Interrupts? >>>> This patch just adds basic support to the existing platform driver. >>>> Intention is to be able to call the driver from DT with basic >>>> initialization from driver the existing driver. >>> >>> Bindings should be rather complete. Here the datasheet is accessible and >>> few properties quite obvious, so I don't see a reason to skip them. >> I agree with you. But currently the driver which is already merged >> doesn't has it implemented. >> And will be working on separate patch to include this later. > Please let me know if this approach is acceptable, or if there are any > other suggestions or concerns that you have. You are adding new bindings, so what does the driver has to do with it? Best regards, Krzysztof
