drivers: sensor: add F75303 temperature sensor driver #60833
Conversation
panikiel
requested review from
nashif,
carlescufi,
galak and
MaureenHelm
as code owners
zephyrbot
added
area: Devicetree Binding
| f75303_emul_set_reg(fixture->target, F75303_LOCAL_TEMP_H, temp_local >> 3); | ||
| f75303_emul_set_reg(fixture->target, F75303_LOCAL_TEMP_L, (temp_local & 7) << 5); |
There was a problem hiding this comment.
Thanks for creating an emulator and test! However instead of having the test write register values directly into the emulator, you can use the emulator sensor backend API.
The generic sensor test will then validate your new temperature sensor an emulator.
Your emulator code needs to implement the 'set_channel` API, which will internally write the correct registers corresponding to the set operation.
See the akm09918c emulator for an example.
drivers/sensor/f75303/f75303_emul.h
Outdated
There was a problem hiding this comment.
These public routines shouldn't be needed once you use the emulator backend model.
|
Note that commit f05fae4 doesn't appear to have any changed from your previous commit. |
Yeah sorry, that was an accidental merge commit |
str4t0m
changed the title
| } | ||
| res = f75303_fetch_remote2(dev); | ||
| break; | ||
| case SENSOR_CHAN_AMBIENT_TEMP: |
There was a problem hiding this comment.
This sort of thing is exactly why we need to support channel type and index pairs and not just channel type
There was a problem hiding this comment.
Are there any open issues/proposals to to do this?
This could also be modeled as a multi-function device, with each temperature sensor nested under a parent device. But that seems a little heavyweight.
There was a problem hiding this comment.
It's being discussed in #61163, though to the best of my knowledge, there is no dedicated issue about this.
There was a problem hiding this comment.
MFD for that seems incredibly overkill for this, device specific channels seems reasonable to close the gap and get the code in, easy enough to change it to subchannel later.
| } | ||
| res = f75303_fetch_remote2(dev); | ||
| break; | ||
| case SENSOR_CHAN_AMBIENT_TEMP: |
There was a problem hiding this comment.
Are there any open issues/proposals to to do this?
This could also be modeled as a multi-function device, with each temperature sensor nested under a parent device. But that seems a little heavyweight.
drivers/sensor/f75303/f75303.c
Outdated
| val->val1 = sample >> 3; | ||
| val->val2 = (sample & 7) * 125000; |
There was a problem hiding this comment.
Please add a comment documenting this conversion.
There was a problem hiding this comment.
Would also be nice to add some constants for the numbers, e.g.
#define F75303_TEMP_SAMPLE_INT_SHIFT (3)
#define F75303_TEMP_SAMPLE_FRAC_MASK GENMASK(2,0)
#define F75303_TEMP_SAMPLE_MICROCELSIUS_PER_BIT (125000)
These don't need to go in the header as neither tests nor applications need these.
Add vendor prefix for Feature Integration Technology Inc. Signed-off-by: Paweł Anikiel <[email protected]>
Add driver for F75303 temperature sensor IC. Signed-off-by: Paweł Anikiel <[email protected]>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hi @semihalf-anikiel-pawel!
Congratulations on getting your very first Zephyr pull request merged 🎉🥳. This is a fantastic achievement, and we're thrilled to have you as part of our community!
To celebrate this milestone and showcase your contribution, we'd love to award you the Zephyr Technical Contributor badge. If you're interested, please claim your badge by filling out this form: Claim Your Zephyr Badge.
Thank you for your valuable input, and we look forward to seeing more of your contributions in the future! 🪁
Add driver for F75303 temperature sensor IC.