diff mbox series

[v2] dt-bindings: trivial-devices: add onnn,adt7462

Message ID 20240923093800.892949-1-chanh@os.amperecomputing.com
State New
Headers show
Series [v2] dt-bindings: trivial-devices: add onnn,adt7462 | expand

Commit Message

Chanh Nguyen Sept. 23, 2024, 9:38 a.m. UTC
The adt7462 supports monitoring and controlling up to
four PWM Fan drive outputs and eight TACH inputs measures.
The adt7462 supports reading a single on chip temperature
sensor and three remote temperature sensors. There are up
to 13 voltage monitoring inputs.

Add device tree bindings for the adt7462 device.

Signed-off-by: Chanh Nguyen <chanh@os.amperecomputing.com>
---
Change in v2:
   - Add onnn,adt7462 to the list of trivial devices       [Guenter]
---
 Documentation/devicetree/bindings/trivial-devices.yaml | 2 ++
 1 file changed, 2 insertions(+)

Comments

Conor Dooley Sept. 23, 2024, 9:23 p.m. UTC | #1
On Mon, Sep 23, 2024 at 09:38:00AM +0000, Chanh Nguyen wrote:
> The adt7462 supports monitoring and controlling up to
> four PWM Fan drive outputs and eight TACH inputs measures.
> The adt7462 supports reading a single on chip temperature
> sensor and three remote temperature sensors. There are up
> to 13 voltage monitoring inputs.
> 
> Add device tree bindings for the adt7462 device.
> 
> Signed-off-by: Chanh Nguyen <chanh@os.amperecomputing.com>
> ---
> Change in v2:
>    - Add onnn,adt7462 to the list of trivial devices       [Guenter]

Is this really a trivial device? If it monitors and controls fans, how
come those do not need to be represented in the devicetree? How is it
possible to tell the difference between monitoring 1 and 4 fans without
the extra detail?

Curious,
Conor.

> ---
>  Documentation/devicetree/bindings/trivial-devices.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml
> index 0108d7507215..15f89d7ecf73 100644
> --- a/Documentation/devicetree/bindings/trivial-devices.yaml
> +++ b/Documentation/devicetree/bindings/trivial-devices.yaml
> @@ -311,6 +311,8 @@ properties:
>            - nuvoton,w83773g
>              # OKI ML86V7667 video decoder
>            - oki,ml86v7667
> +            # ON Semiconductor ADT7462 Temperature, Voltage Monitor and Fan Controller
> +          - onnn,adt7462
>              # 48-Lane, 12-Port PCI Express Gen 2 (5.0 GT/s) Switch
>            - plx,pex8648
>              # Pulsedlight LIDAR range-finding sensor
> -- 
> 2.43.0
>
Chanh Nguyen Sept. 24, 2024, 4:17 a.m. UTC | #2
On 24/09/2024 04:23, Conor Dooley wrote:
> On Mon, Sep 23, 2024 at 09:38:00AM +0000, Chanh Nguyen wrote:
>> The adt7462 supports monitoring and controlling up to
>> four PWM Fan drive outputs and eight TACH inputs measures.
>> The adt7462 supports reading a single on chip temperature
>> sensor and three remote temperature sensors. There are up
>> to 13 voltage monitoring inputs.
>>
>> Add device tree bindings for the adt7462 device.
>>
>> Signed-off-by: Chanh Nguyen <chanh@os.amperecomputing.com>
>> ---
>> Change in v2:
>>     - Add onnn,adt7462 to the list of trivial devices       [Guenter]
> 
> Is this really a trivial device? If it monitors and controls fans, how
> come those do not need to be represented in the devicetree? How is it
> possible to tell the difference between monitoring 1 and 4 fans without
> the extra detail?
> 

Hi Conor, Thank you for your comments!

The chip is old. The driver was added back in 2008.

Really, this is such an old chip that it would make more sense to just 
leave its driver alone unless there is a problem with it; this is 
viewpoint from Guenter.

I'm using the driver and the device tree with only the "compatible" and 
"reg" properties; now it's being good for me without any extra detail.

Guenter, Rob, Krzysztof, and I discussed making the decision to add this 
device to the list of trivial devices. You can get more information at 
thread 
https://lore.kernel.org/lkml/20240918220553.GA2216504-robh@kernel.org/T/ 
(Because the commit title changed between v1 and v2, it's so hard for 
everyone to find it. Sorry! I missed mentioning the link to pacth v1).

Guenter, who supported the driver development before, he suggested me 
add this device to the list of trivial devices.

Thanks,
Chanh

> Curious,
> Conor.
> 
>> ---
>>   Documentation/devicetree/bindings/trivial-devices.yaml | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml
>> index 0108d7507215..15f89d7ecf73 100644
>> --- a/Documentation/devicetree/bindings/trivial-devices.yaml
>> +++ b/Documentation/devicetree/bindings/trivial-devices.yaml
>> @@ -311,6 +311,8 @@ properties:
>>             - nuvoton,w83773g
>>               # OKI ML86V7667 video decoder
>>             - oki,ml86v7667
>> +            # ON Semiconductor ADT7462 Temperature, Voltage Monitor and Fan Controller
>> +          - onnn,adt7462
>>               # 48-Lane, 12-Port PCI Express Gen 2 (5.0 GT/s) Switch
>>             - plx,pex8648
>>               # Pulsedlight LIDAR range-finding sensor
>> -- 
>> 2.43.0
>>
Guenter Roeck Sept. 24, 2024, 5:39 a.m. UTC | #3
On 9/23/24 21:17, Chanh Nguyen wrote:
> On 24/09/2024 04:23, Conor Dooley wrote:
>> On Mon, Sep 23, 2024 at 09:38:00AM +0000, Chanh Nguyen wrote:
>>> The adt7462 supports monitoring and controlling up to
>>> four PWM Fan drive outputs and eight TACH inputs measures.
>>> The adt7462 supports reading a single on chip temperature
>>> sensor and three remote temperature sensors. There are up
>>> to 13 voltage monitoring inputs.
>>>
>>> Add device tree bindings for the adt7462 device.
>>>
>>> Signed-off-by: Chanh Nguyen <chanh@os.amperecomputing.com>
>>> ---
>>> Change in v2:
>>>     - Add onnn,adt7462 to the list of trivial devices       [Guenter]
>>
>> Is this really a trivial device? If it monitors and controls fans, how
>> come those do not need to be represented in the devicetree? How is it
>> possible to tell the difference between monitoring 1 and 4 fans without
>> the extra detail?
>>
> 
> Hi Conor, Thank you for your comments!
> 
> The chip is old. The driver was added back in 2008.
> 
> Really, this is such an old chip that it would make more sense to just leave its driver alone unless there is a problem with it; this is viewpoint from Guenter.
> 
> I'm using the driver and the device tree with only the "compatible" and "reg" properties; now it's being good for me without any extra detail.
> 
> Guenter, Rob, Krzysztof, and I discussed making the decision to add this device to the list of trivial devices. You can get more information at thread https://lore.kernel.org/lkml/20240918220553.GA2216504-robh@kernel.org/T/ (Because the commit title changed between v1 and v2, it's so hard for everyone to find it. Sorry! I missed mentioning the link to pacth v1).
> 
> Guenter, who supported the driver development before, he suggested me add this device to the list of trivial devices.
> 

Historically it was ok to add an entry into trivial devices and extending
it later if/when needed. That was still widely done at least until very
recently. Maybe that changed recently. If so, sorry for bringing up the idea.
I did not envision that this might be a problem.

Can you live with no devicetree entry at all for the chip ? That is of
course less than perfect, but it seems better than trying to design a
devicetree description for the chip only to never implement it.

Thanks,
Guenter
Conor Dooley Sept. 24, 2024, 8:20 a.m. UTC | #4
On Mon, Sep 23, 2024 at 10:39:20PM -0700, Guenter Roeck wrote:
> On 9/23/24 21:17, Chanh Nguyen wrote:
> > On 24/09/2024 04:23, Conor Dooley wrote:
> > > On Mon, Sep 23, 2024 at 09:38:00AM +0000, Chanh Nguyen wrote:
> > > > The adt7462 supports monitoring and controlling up to
> > > > four PWM Fan drive outputs and eight TACH inputs measures.
> > > > The adt7462 supports reading a single on chip temperature
> > > > sensor and three remote temperature sensors. There are up
> > > > to 13 voltage monitoring inputs.
> > > > 
> > > > Add device tree bindings for the adt7462 device.
> > > > 
> > > > Signed-off-by: Chanh Nguyen <chanh@os.amperecomputing.com>
> > > > ---
> > > > Change in v2:
> > > >     - Add onnn,adt7462 to the list of trivial devices       [Guenter]
> > > 
> > > Is this really a trivial device? If it monitors and controls fans, how
> > > come those do not need to be represented in the devicetree? How is it
> > > possible to tell the difference between monitoring 1 and 4 fans without
> > > the extra detail?
> > > 
> > 
> > Hi Conor, Thank you for your comments!
> > 
> > The chip is old. The driver was added back in 2008.
> > 
> > Really, this is such an old chip that it would make more sense to just leave its driver alone unless there is a problem with it; this is viewpoint from Guenter.
> > 
> > I'm using the driver and the device tree with only the "compatible" and "reg" properties; now it's being good for me without any extra detail.
> > 
> > Guenter, Rob, Krzysztof, and I discussed making the decision to add this device to the list of trivial devices. You can get more information at thread https://lore.kernel.org/lkml/20240918220553.GA2216504-robh@kernel.org/T/ (Because the commit title changed between v1 and v2, it's so hard for everyone to find it. Sorry! I missed mentioning the link to pacth v1).
> > 
> > Guenter, who supported the driver development before, he suggested me add this device to the list of trivial devices.
> > 
> 
> Historically it was ok to add an entry into trivial devices and extending
> it later if/when needed. That was still widely done at least until very
> recently. Maybe that changed recently. If so, sorry for bringing up the idea.
> I did not envision that this might be a problem.
> 
> Can you live with no devicetree entry at all for the chip ? That is of
> course less than perfect, but it seems better than trying to design a
> devicetree description for the chip only to never implement it.

Since it sounds like Krzysztof assented to it, I'll just leave it for
him to ack.
Rob Herring Sept. 24, 2024, 10:26 p.m. UTC | #5
On Tue, Sep 24, 2024 at 11:17:53AM +0700, Chanh Nguyen wrote:
> On 24/09/2024 04:23, Conor Dooley wrote:
> > On Mon, Sep 23, 2024 at 09:38:00AM +0000, Chanh Nguyen wrote:
> > > The adt7462 supports monitoring and controlling up to
> > > four PWM Fan drive outputs and eight TACH inputs measures.
> > > The adt7462 supports reading a single on chip temperature
> > > sensor and three remote temperature sensors. There are up
> > > to 13 voltage monitoring inputs.
> > > 
> > > Add device tree bindings for the adt7462 device.
> > > 
> > > Signed-off-by: Chanh Nguyen <chanh@os.amperecomputing.com>
> > > ---
> > > Change in v2:
> > >     - Add onnn,adt7462 to the list of trivial devices       [Guenter]
> > 
> > Is this really a trivial device? If it monitors and controls fans, how
> > come those do not need to be represented in the devicetree? How is it
> > possible to tell the difference between monitoring 1 and 4 fans without
> > the extra detail?
> > 
> 
> Hi Conor, Thank you for your comments!
> 
> The chip is old. The driver was added back in 2008.
> 
> Really, this is such an old chip that it would make more sense to just leave
> its driver alone unless there is a problem with it; this is viewpoint from
> Guenter.
> 
> I'm using the driver and the device tree with only the "compatible" and
> "reg" properties; now it's being good for me without any extra detail.
> 
> Guenter, Rob, Krzysztof, and I discussed making the decision to add this
> device to the list of trivial devices. You can get more information at
> thread
> https://lore.kernel.org/lkml/20240918220553.GA2216504-robh@kernel.org/T/
> (Because the commit title changed between v1 and v2, it's so hard for
> everyone to find it. Sorry! I missed mentioning the link to pacth v1).

It's fine. I'll apply this after the merge window.

Rob
Krzysztof Kozlowski Sept. 26, 2024, 6:50 a.m. UTC | #6
On 23/09/2024 11:38, Chanh Nguyen wrote:
> The adt7462 supports monitoring and controlling up to
> four PWM Fan drive outputs and eight TACH inputs measures.
> The adt7462 supports reading a single on chip temperature
> sensor and three remote temperature sensors. There are up
> to 13 voltage monitoring inputs.
> 
> Add device tree bindings for the adt7462 device.
> 
> Signed-off-by: Chanh Nguyen <chanh@os.amperecomputing.com>
> ---
> Change in v2:
>    - Add onnn,adt7462 to the list of trivial devices       [Guenter]
> ---
>  Documentation/devicetree/bindings/trivial-devices.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml
> index 0108d7507215..15f89d7ecf73 100644
> --- a/Documentation/devicetree/bindings/trivial-devices.yaml
> +++ b/Documentation/devicetree/bindings/trivial-devices.yaml
> @@ -311,6 +311,8 @@ properties:
>            - nuvoton,w83773g
>              # OKI ML86V7667 video decoder
>            - oki,ml86v7667
> +            # ON Semiconductor ADT7462 Temperature, Voltage Monitor and Fan Controller
> +          - onnn,adt7462

I suggest to add the compatible to the driver, for explicit matching via OF.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml
index 0108d7507215..15f89d7ecf73 100644
--- a/Documentation/devicetree/bindings/trivial-devices.yaml
+++ b/Documentation/devicetree/bindings/trivial-devices.yaml
@@ -311,6 +311,8 @@  properties:
           - nuvoton,w83773g
             # OKI ML86V7667 video decoder
           - oki,ml86v7667
+            # ON Semiconductor ADT7462 Temperature, Voltage Monitor and Fan Controller
+          - onnn,adt7462
             # 48-Lane, 12-Port PCI Express Gen 2 (5.0 GT/s) Switch
           - plx,pex8648
             # Pulsedlight LIDAR range-finding sensor