drivers: support for NXP PCA9685 I2C 16-channel, 12-bit PWM controller

[gschorcht]

Contribution description

This PR adds a driver vor the NXP PCA9685 16-channel, 12-bit PWM controller that is connected via I2C. Although the controller is optimized for LED control, the 12-bit resolution also allows the control of servos with a resolution of 4 us at 60 Hz refresh signal.

The following features of the PCA9685 are supported by the driver:

• 16 channels with 12-bit resolution
• Refresh rates from 24 Hz to 1526 Hz with internal 25 MHz oscillator
• Totem pole outputs with 25 mA as sink and 10 mA as source at 5V
• Software programmable open-drain output selection
• Inverted outputs
• Active LOW Output Enable (OE) input pin
• External clock input with max. 50 MHz

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

• functions have the prefix pca9685_ and
• functions require an additional parameter, the pointer to the PWM device of type #pca9685_t.

Update:
The driver is also compatible with the PWM extension API in PR #10533 so that up to 64 PCA9685 PWM devices can be used as PWM extensions with the standard peripheral PWM interface.
To get it merged soon, all PWM extension API code was removed.

Testing procedure

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

make -C tests/driver_pca9685 BOARD=... flash term

Connect a logic analyzer and execute the following tests:

1. The signals should alter with frequency of 100 Hz and the duty cycles should be left aligned.

   init 0 0 100 1000
   set 0 0 100
   set 0 1 900
   

2. The signals should alter with frequency of 500 Hz and the duty cycles should be right aligned.

   init 0 1 500 100
   set 0 0 10
   set 0 1 90
   

3. The signals should alter with frequency of 1000 Hz and the duty cycles should be center aligned.

   init 0 2 1000 100
   set 0 0 10
   set 0 1 90
   

Issues/PRs references

The PR is related to #10533 but does not depend on it. As long as module extend_pwm is not enabled, it is working without #10533.

[stale]

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.

[mrusme]

Can this be merged in a way it becomes partially functional and continue with the fancy stuff as soon as the PR this is depending on are ready?

[gschorcht]

Can this be merged in a way it becomes partially functional and continue with the fancy stuff as soon as the PR this is depending on are ready?

From my point of view the driver is ready and could be used.

The initial plan was to merge the PWM extension API in PR #10533 first. If you take a look to the test application you can see that PWM extender channels could be used exactly like MCU pwm channels when the module extend_pwm is enabled what would be pretty cool. Unfortunately, we still can't find and agreement for extension APIs. But, if module extend_pwm is not enabled, it can be used with current interface.

That is, you could checkout the PR and you could use it. I will rebase it later today to the current master. I could also remove the extender parts for the moment, if it helps to merge it. It would be great, if you would test it.

[gschorcht]

@mrusme I have rebased it to current master and it should be working. Now, I have to find someone (maybe @benpicco) who review it. Since you have probably the hardware, it would help if you could test it.

[gschorcht]

@benpicco Thanks for reviewing. I'm really impressed, so fast and so deep in detail.

[benpicco]

Overall I think this is good, just some minor nits. There are more efficient ways to set those config bits, but since it only happens on init I don't think it's a problem.

Sorry for the spotty updates, I should use the review feature next time to add them all at one.

@mrusme can you give it a whirl and see if it works for you?

[benpicco]

I'd squash drivers: makefile support for PCA9685 PWMs and drivers/pca9685: PWM extension API code removed into drivers: support for PCA9685 PWMs.

When the PWM extension code gets finalised you couldn't simply revert 998c862 but would make a new commit anyway.

[gschorcht]

I'd squash drivers: makefile support for PCA9685 PWMs and drivers/pca9685: PWM extension API code removed into drivers: support for PCA9685 PWMs.

When the PWM extension code gets finalised you couldn't simply revert 998c862 but would make a new commit anyway.

I will try, but I'm not sure whether it works.

[gschorcht]

I'd squash drivers: makefile support for PCA9685 PWMs and drivers/pca9685: PWM extension API code removed into drivers: support for PCA9685 PWMs.
When the PWM extension code gets finalised you couldn't simply revert 998c862 but would make a new commit anyway.

I will try, but I'm not sure whether it works.

No, it leads to conflicts. I would like to leave it as is.

[gschorcht]

I could squash drivers: makefile support for PCA9685 PWMs and drivers: support for PCA9685 PWMs. But squashing drivers/pca9685: PWM extension API code removed leads to conflicts since it contains reverting changes of other commits.

[gschorcht]

Ok, I solved the conflicts by splitting commit drivers/pca9685: PWM extension API code removed into separate fixups of the respective commits and squashed these fixups afterwards.

[benpicco]

The code looks good now.
I'd still like @mrusme to give it a try, but if he doesn't respond we can merge it without him 😉

Another rebase is needed though.

[mrusme]

Hey there, thanks for the patience and sorry for the delay! I've tested this and all looks good from a general perspective. However, I've experienced a slight issue here which is probably only relevant for me - it does however make this driver unusable to me. Let me explain:

For every PWM action a write is being performed. Write then acquires I2C, sets writes the data to the registers and releases the I2C again. That's basically how most drivers run and it's totally fine. In my case, I need to set multiple channels at once, with either identical or different computed values. Hence the poor-man's-driver I wrote for the PCA contains a small hack which allows me to specify whether I2C should be kept acquired. If I was to release the I2C channel between setting PWM for the multiple channels I could end up with a different thread/device acquiring the I2C and blocking it for a certain amount of time, leaving me with a couple of channels set to the new PWM values, while the rest of the channels are still running on the old values.

So, long story short: This driver can be merged as it looks good. However, unfortunately I won't be able to use it in this form, because of the issue that I just described.

[gschorcht]

@mrusme The problem is that the built-in PWM driver API doesn't allow to synchronize the pwm_set calls for different channels. For compatibility reason, the extender driver realizes pwm_set function does it in the same way. However, beside the requirement to be compatible with the built-in PWM driver API, the extener driver could implement additional features, e.g., to set an option and to define an additional function which allows to synchronize channel settings by aquiring the I2C bus as long as it is released explicitly.

I would really appreciate, if you would provide a proposal for an extension of this PR.

BTW, I already knew the problem of channel synchronisation and was thinking about to propose an extension of the built-in PWM driver API.

[gschorcht]

@mrusme

However, beside the requirement to be compatible with the built-in PWM driver API, the extener driver could implement additional features, e.g., to set an option and to define an additional function which allows to synchronize channel settings by aquiring the I2C bus as long as it is released explicitly.

I could imagine to add a member bool sync_setting to struct pca9685_params_t which is set to false by default. This member can be set in parameters to true, if synchronized setting is required. If synchronized setting is required, the first call of pca9685_pwm_set aquires the I2C bus. An additional function pca9685_set_sync releases the I2C bus.

With this approach, the extender can be used as defined by the built-in PWM API, but the user has the option to use synchronized setting too.

[benpicco]

I'd rather prefer to merge a driver that is fit for a real-world use case, but it seems there is no easy solution for that.

So I think it will be easier for everyone to propose changes to synchronized PWM output (atomic PWM setting? 😉) on top of the mainline tree.

The driver works - extending the API to allow synchronized settings of multiple outputs is for another PR.

[benpicco]

Turns out the compiler will optimize this all away - neat!