Re: Adding support for Nordic PDM Driver in Devicetree #nrf52832 #pdm #driver


Bolivar, Marti
 

Hi there; responses inline.

"Frederik David Damsgaard Popp via lists.zephyrproject.org"
<frdm=demant.com@...> writes:

As I understand it, I need to do at least the following things:

* Create the generic api as well as the api struct, located in
zephyr/include/drivers.
I'm not an audio expert, but are you sure the existing audio codec API
is not right?

I.e. do you really need to create a new generic API in include/drivers/,
not just implement API calls in drivers/audio/my_audio_driver.c or
something like that?

Maybe someone else can weigh in on this particular question. Either way,
some details on devicetree are below.

* Place the actual implementation in zephyr/drivers where the api struct is then connected to this implementation
Additionally, setup the config variables in the *Kconfig* file in the same directory, as well as connecting the source code using *CMakeLists.txt*
* Create a YAML driver binding in zephyr/dts/bindings.
* Bind the driver in the *nrf52832.dtsi* file, in order to make the SoC compatible with this driver.
* Enable the driver in the *nrf52_pca20020.dts* file, in order to make
the board able to use the driver.
Strictly speaking, actual drivers -- as in drivers/subsystem/my_driver.c
files -- are enabled using Kconfig, not devicetree. So
CONFIG_MY_DRIVER=y in Kconfig means "enable my driver," which means
something like "compile my_driver.c."

There is a historical relationship between individual 'struct device'
instances and Kconfig options which Zephyr is moving away from:

https://docs.zephyrproject.org/latest/guides/kconfig/tips.html#what-not-to-turn-into-kconfig-options

Please see discussion here for more details on where this is going:

https://github.com/zephyrproject-rtos/zephyr/issues/10621#issuecomment-618689638

However, the "moving away" is still work in progress, so I will try to
provide some more details from what I understand about the current
consensus. Clarifications and corrections from others welcome.

First, as you've read from the DT guide, a device driver should decide
what 'struct device' instances to allocate using the devicetree. In
particular, any devicetree node with the right compatible and status
'okay' should result in a struct device, *as long as the driver is
enabled*. The driver being enabled (or not) depends on whether
CONFIG_MY_DRIVER=y (or not).

It's definitely nice to make CONFIG_MY_DRIVER default to y if any node
with the right compatible has an "okay" status in the devicetree, so the
build works properly by default just by setting up the right devicetree
overlays which enable the relevant nodes.

However, people still want to be able to set CONFIG_MY_DRIVER=n to
disable a driver. There was another question on the list about
substituting a different driver for the same hardware earlier this week
which is a good use case for that ability.

So in your case, since you are asking about PDM on nRF52832, if some
node with compatible = "nordic,nrf-pdm" has status = "okay", *and* the
driver is enabled with CONFIG_MY_AUDIO_DRIVER=y (for whatever you choose
to call the Kconfig option), then you should create a struct device for
the node so applications can get the struct device and interact with it
using the right API.

It looks like there is already a dts/bindings/audio/nordic,nrf-pdm.yaml
file, though, so it doesn't seem like you need to define your own
binding. I didn't find any drivers for it in the upstream tree, though.

It's also totally OK, even recommended, to do something like this in the
Kconfig file defining CONFIG_MY_AUDIO_DRIVER:

# Workaround for not being able to have commas in macro arguments
DT_COMPAT_NORDIC_NRF_PDM := nordic,nrf-pdm

config MY_AUDIO_DRIVER
bool
default $(dt_compat_enabled,$(DT_COMPAT_NORDIC_NRF_PDM))
help
Enable my audio driver.

This uses the dt_compat_enabled() kconfigfunction to set the default
value of MY_AUDIO_DRIVER depending on if any nodes with compatible
"nordic,nrf-pdm" have status "okay" (i.e. if the compatible has any
enabled nodes, hence the name). That way, as long as the right driver
subsystem is enabled, your driver will be enabled by default whenever
the devicetree says the hardware is there and enabled.

In the driver subsystem CMakeLists.txt file, you should add something
like this to compile my_audio_driver.c based on the value of
CONFIG_MY_AUDIO_DRIVER:

zephyr_library_sources_ifdef(CONFIG_MY_AUDIO_DRIVER my_audio_driver.c)

See e.g. drivers/spi/CMakeLists.txt for an example; the right CMake
extension function to use can depend on how the CMakeLists.txt is set
up.

Finally, the source file my_audio_driver.c itself should decide what
struct devices to instantiate using the final devicetree, as you've read
about in the devicetree guide. See drivers/pwm/pwm_nrfx.c for an
nRF-based example that does the right thing in my opinion.

To summarize:

- The soc.dtsi file just declares that the chip has some hardware by
defining a node or nodes for it; that's a question of hardware, not
software, and drivers are software. The soc.dtsi file can set the
compatible property on a node to say what kind of hardware it
represents.

In some cases on Nordic hardware, a node's compatible is left for the
user to pick. This happens when one ID in the product specification's
instantiation table can be used in multiple ways, see e.g. i2c0 in
nrf52832.dtsi, which could be set to nordic,nrf-twi or nordic-nrf-twim
depending on what the user wants. That's not the case with the PDM
peripheral on nRF52832, though.

- The BOARD.dts file has the option of setting the status property for
some nodes if the board's maintainer wants to; setting status to
"disabled" usually means software should ignore that hardware
completely, even though it knows it's "there." But status = "okay"
doesn't say what software driver to use for that hardware, because
again DT is for describing hardware.

- Only Kconfig can say "compile this driver for this hardware." It's
good if Kconfig takes clues from the devicetree to decide when to do
that by default, but the user must remain in control and able to
override that default decision.


This is how far I have gotten yet, and was wondering if I have missed anything?
I know the tree is not consistent in implementing the above
"vision," but I hope the examples and links help. Please let me know if
that's not clear.

Additionally I would like to ask (and I don't know whether this is the
right forum), if there is any possibility that this is pushed to the
official upstream, if I succeed?
Patches are generally welcome, just make sure you follow the
contributing guide, and it can't hurt to ask around like you are doing
here. You never know what other people might be doing.

I would then make a small sample, and document in accordance with the
guidelines, so that others may also use it.
If you're planning on upstreaming this, I would make sure to figure out
who the right maintainers are to decide the API that should be used or
if a new API is really needed. Defining a new API is harder and takes
more work and coordination than writing a driver for an existing API.

The devel list is a good place to ask about this. You're doing the right
thing (but I don't know the answer about what API to use).

Thanks in advance!
You're welcome,
Martí

Join devel@lists.zephyrproject.org to automatically receive all group messages.