drivers: support for Microchip MCP47xx DAC devices added #10518

gschorcht · 2018-11-29T16:14:46Z

Contribution description

This PR adds a driver which supports different Microchip MCP47xx DAC variants. Multiple MCP47xx DAC devices and different variants can be used at the same time.

Expander	Channels	Resolution
MCP4706	1	8-bit
MCP4716	1	10-bit
MCP4725	1	12-bit
MCP4726	1	12-bit
MCP4728	4	12-bit

The following features of MCP47xx DAC devices are not supported at the moment:

configuring and reading address bits to/from EEPROM
writing DAC channel output values to the EEPROM
setting the UDAC bit and using the LDAC pin for MCP4728

The driver interface is kept as compatible as possible with the peripheral DAC interface. The only differences are that

functions have the prefix mcp47xx_ and
functions require an additional parameter, the pointer to the MCP47xx device of type #mcp47xx_t.

Testing procedure

Please refer the test application in tests/driver_mcp47xx for an example on how to use the driver

make -C tests/driver_mcp47xx BOARD=...

Issues/PRs references

stale · 2019-08-10T03:08:16Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

stale · 2020-02-11T03:15:15Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

gschorcht · 2020-02-11T09:51:34Z

Still waiting for another PR.

stale · 2020-08-14T23:45:46Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

stale · 2021-03-19T23:33:31Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

benpicco

Looks good! Only a few clarifications:

benpicco · 2021-12-07T12:50:15Z

drivers/include/mcp47xx.h

+ */
+typedef struct {
+    mcp47xx_params_t params;              /**< device configuration parameters */
+    uint16_t values[MCP47XX_CHN_NUM_MAX]; /**< contains the last values set */


Suggested change

uint16_t values[MCP47XX_CHN_NUM_MAX]; /**< contains the last values set */

uint16_t values[MCP47XX_CHN_NUM_MAX]; /**< contains the last values set for persistence when device is powered off */

right?

Yes, will change it. May I squash directly?

Sure, squash

benpicco · 2021-12-07T12:55:52Z

drivers/include/mcp47xx.h

+ *
+ * @retval  none
+ */
+void mcp47xx_dac_get(mcp47xx_t *dev, dac_t chn, uint16_t *value);


I'm a bit surprised you implemented this when periph/dac API does not provide any dac_get().
Feel free to keep it though :)

benpicco · 2021-12-07T12:58:08Z

drivers/include/mcp47xx.h

+typedef struct {
+    const char *name;   /**< name of the MCP47xx device */
+    unsigned int dev;   /**< index of the MCP47xx device */
+    dac_t channel;      /**< channel of the MCP47xx device */


Yes, it is the same as for the perhiph/dac. The goal was to keep it as compatible as possible with periph/dac and to handle channels in same way when we intended to introduced the DAC extension API with PR #10512.

But dac_t would refer to a unique DAC device, not a channel of a DAC.
Just using uint8_t here would be less confusing IMHO (there also is no saul_dac_params_t yet)

No, dac_t refers a line, that is device + channel. It is also used in periph/dac to set a certain channel of a device. In this sense, it also makes sense to use it here.

When we started to introduce the extension API, we used exactly the same types for extenders to refer devices and channels.

Unfortunatly, we had not much luck with the extension interfaces as suggested in PR #10512 til now.

However, we can of course also use uint8_t, if there is an extension API at some point in future, the type dac_t can still be mapped to dev + chn.

We can still change it later if we go forward with the extension API. 😉
To an unsuspecting user like me the dac_t type here just adds confusion.

benpicco · 2021-12-07T17:52:10Z

Please squash!

gschorcht mentioned this pull request Dec 11, 2018

periph/dac: support for DAC extension API #10512

Draft

stale bot added the State: stale State: The issue / PR has no activity for >185 days label Aug 10, 2019

gschorcht added the State: waiting for other PR State: The PR requires another PR to be merged first label Aug 10, 2019

stale bot removed the State: stale State: The issue / PR has no activity for >185 days label Aug 10, 2019

gschorcht mentioned this pull request Aug 13, 2019

periph/gpio: enable API usage for expanders #9690

Open

5 tasks

stale bot added the State: stale State: The issue / PR has no activity for >185 days label Feb 11, 2020

stale bot removed the State: stale State: The issue / PR has no activity for >185 days label Feb 11, 2020

stale bot added the State: stale State: The issue / PR has no activity for >185 days label Aug 14, 2020

gschorcht removed the State: stale State: The issue / PR has no activity for >185 days label Aug 15, 2020

stale bot added the State: stale State: The issue / PR has no activity for >185 days label Mar 19, 2021

gschorcht removed the State: stale State: The issue / PR has no activity for >185 days label Mar 20, 2021

MrKevinWeiss added this to the Release 2021.07 milestone Jun 21, 2021

MrKevinWeiss removed this from the Release 2021.07 milestone Jul 15, 2021

gschorcht added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed State: waiting for other PR State: The PR requires another PR to be merged first labels Dec 6, 2021

gschorcht force-pushed the drivers_mcp47xx branch from 1824ce8 to 60184d2 Compare December 6, 2021 23:57

gschorcht requested review from aabadie, cgundogan, fjmolinas and jia200x as code owners December 6, 2021 23:57

gschorcht requested review from leandrolanzieri, MichelRottleuthner and smlng as code owners December 6, 2021 23:57

github-actions bot added Area: Kconfig Area: Kconfig integration Area: tests Area: tests and testing framework labels Dec 6, 2021

gschorcht force-pushed the drivers_mcp47xx branch 4 times, most recently from b248489 to 44f8486 Compare December 7, 2021 08:34

benpicco self-requested a review December 7, 2021 12:46

benpicco reviewed Dec 7, 2021

View reviewed changes

benpicco approved these changes Dec 7, 2021

View reviewed changes

benpicco added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Dec 7, 2021

gschorcht added 4 commits December 8, 2021 05:42

drivers/mcp47xx: add driver for MCP47xx DAC devices

499cbb9

drivers/mcp47xx: SAUL integration

033fa00

tests/driver_mcp47xx: add test app for MCP47xx driver

1aa3584

drivers/mcp47xx: add Kconfig

739fbf4

gschorcht force-pushed the drivers_mcp47xx branch from 3197bd2 to 739fbf4 Compare December 8, 2021 04:42

benpicco added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Feb 26, 2022

benpicco enabled auto-merge February 26, 2022 18:34

benpicco added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Feb 26, 2022

benpicco merged commit 6cfbec4 into RIOT-OS:master Feb 26, 2022

OlegHahm added this to the Release 2022.04 milestone Apr 25, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

drivers: support for Microchip MCP47xx DAC devices added #10518

drivers: support for Microchip MCP47xx DAC devices added #10518

gschorcht commented Nov 29, 2018 •

edited

Loading

stale bot commented Aug 10, 2019

stale bot commented Feb 11, 2020

gschorcht commented Feb 11, 2020

stale bot commented Aug 14, 2020

stale bot commented Mar 19, 2021

benpicco left a comment

benpicco Dec 7, 2021

gschorcht Dec 7, 2021 •

edited

Loading

benpicco Dec 7, 2021

benpicco Dec 7, 2021

benpicco Dec 7, 2021

gschorcht Dec 7, 2021

benpicco Dec 7, 2021

gschorcht Dec 7, 2021 •

edited

Loading

gschorcht Dec 7, 2021

gschorcht Dec 7, 2021 •

edited

Loading

gschorcht Dec 7, 2021

benpicco Dec 7, 2021

benpicco commented Dec 7, 2021

	uint16_t values[MCP47XX_CHN_NUM_MAX]; /*< contains the last values set /
	uint16_t values[MCP47XX_CHN_NUM_MAX]; /*< contains the last values set for persistence when device is powered off /

drivers: support for Microchip MCP47xx DAC devices added #10518

drivers: support for Microchip MCP47xx DAC devices added #10518

Conversation

gschorcht commented Nov 29, 2018 • edited Loading

Contribution description

Testing procedure

Issues/PRs references

stale bot commented Aug 10, 2019

stale bot commented Feb 11, 2020

gschorcht commented Feb 11, 2020

stale bot commented Aug 14, 2020

stale bot commented Mar 19, 2021

benpicco left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gschorcht Dec 7, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gschorcht Dec 7, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gschorcht Dec 7, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benpicco commented Dec 7, 2021

gschorcht commented Nov 29, 2018 •

edited

Loading

gschorcht Dec 7, 2021 •

edited

Loading

gschorcht Dec 7, 2021 •

edited

Loading

gschorcht Dec 7, 2021 •

edited

Loading