Skip to content

Conversation

@cederom
Copy link
Contributor

@cederom cederom commented Nov 29, 2025

Summary

PR delivered by kerogit over mailing list.

This patch adds support for TC74 Tiny Serial Digital Thermal Sensor from Microchip. It is a standalone chip connected via I2C/SMBus. Documentation for sensor is provided.

IOCTL call used for power management borrows IOCTL code from another driver (ISL29023).

Depends-on: #17403, #17404.

Impact

drivers/sensors: Adds support for TC74 Tiny Serial Digital Thermal Sensor from Microchip + Documentation.

Testing

The driver was tested on AVR128DA28 chip using simple application that read the temperature from it and also switched it to/from standby state.

nsh> tc74_test
tc74_test [4:100]
nsh> Starting TC74 test
Temperature read: 23
Temperature read: 23

@github-actions github-actions bot added Area: Documentation Improvements or additions to documentation Area: Sensors Sensors issues Size: XL The size of the change in this PR is very large. Consider breaking down the PR into smaller pieces. labels Nov 29, 2025
Copy link
Contributor

@linguini1 linguini1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel bad but should this not be a "uORB" sensor (uORB in quotes because it's just the new sensor framework)? This is what we're asking for from contributors going forward and I think it's counterproductive to keep merging legacy interfaces.

#ifndef __ASSEMBLY__

typedef enum
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These should be in CAPS and also maybe a have a prefix unique to the sensor.

@cederom
Copy link
Contributor Author

cederom commented Nov 29, 2025

I feel bad but should this not be a "uORB" sensor (uORB in quotes because it's just the new sensor framework)? This is what we're asking for from contributors going forward and I think it's counterproductive to keep merging legacy interfaces.

Thanks @linguini1, feedback forwarded to mailing list with additional references :-)

This patch adds support for TC74 Tiny Serial Digital Thermal Sensor
from Microchip. It is a standalone chip connected via I2C/SMBus.

The driver was tested on AVR128DA28 chip using simple application
that read the temperature from it and also switched it to/from
standby state.

IOCTL call used for power management borrows IOCTL code
from another driver (ISL29023).

Signed-off-by: Kerogit <[email protected]>
@cederom
Copy link
Contributor Author

cederom commented Nov 29, 2025

Response from kerogit:

the TC74Ax driver not being uORB - well, I was not sure whether to submit the driver at all exactly because it is not done the new way. But then I noticed another such driver was merged in August so I posted it. As for the reasons why the driver is not done the new way, there are two:

First - I did not read the documentation. Over my previous submissions, I was never able to find any howto for writing drivers. As in "if you want to make driver for X, do this." (Do not take this as a criticism please, I was thinking about writing such howto but every time I was done with a feature, it seemed to me that there is actually nothing to write about because everything is simple, clear and pretty obvious.) So I got into the habit of simply looking at other code that does the same thing and adapting it. Only after the driver was almost done, I found the documents you linked.

The other reason - and the reason why I did not decide to rewrite the driver - is that unless I misunderstood the documentation, the new way uses floats to exchange data and seems more complex overall. My intended use case is currently 8-bit AVR and I am even not able to compile code that has floats in it. (That can be probably fixed easily by upgrading to newer gcc.) Moreover, floats are expensive to handle. So to put it simple, if I did the driver this way, it would not be of much use to me.

All in all, if the driver gets rejected because it uses deprecated interface, that would be completely understandable from my point of view. However, I have to say that I am not planning to rewrite it in any forseeable future (maybe if it proved easy to have it configurable for
both interfaces - judging from bmp180 driver it could be - but I don't think I'll find the time even in such case.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: Documentation Improvements or additions to documentation Area: Sensors Sensors issues Size: XL The size of the change in this PR is very large. Consider breaking down the PR into smaller pieces.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants