On Mon, Mar 16, 2020 at 7:01 AM Daniel Baluta <daniel.baluta@xxxxxxxxx> wrote: > > Thanks Rob for review. See my comments inline: > > <snip> > > > > +# SPDX-License-Identifier: GPL-2.0 > > > > Dual license new bindings please: > > > > (GPL-2.0-only OR BSD-2-Clause) > > Ok, will do. > > > > > > +%YAML 1.2 > > > +--- > > > +$id: http://devicetree.org/schemas/sound/fsl,dai.yaml# > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > + > > > +title: Generic CPU FSL DAI driver for resource management > > > > Bindings are for h/w devices, not drivers. > > Indeed. I think I will change it to something like this. > > title: 'FSL CPU DAI for resource management' > > The explanation are already in patch 2/2 of this series but let e > explain again what I'm > trying to do here and let me know if this makes sense to you. > > Digital Audio Interface device (DAI) are configured by the firmware > running on the DSP. The only > trouble we have is that we cannot easily handle 'resources' like: > clocks, pinctrl, power domains from > firmware. > > This is because our architecture is like this: > > M core [running System Controller Firmware] > | > | > A core [Linux]<----> DSP core [SOF firmware] > > In theory, it is possible for DSP core to communicate with M core, but > this needs a huge > amount of work in order to make it work. We have this on our plans for > the future, > but we are now trying to do resource management from A core because > the infrastructure is already in place. When you change things in the future, Linux gets to keep supporting both ways of doing things? I'd rather just support one way. > So, the curent driver introduced in this series acts like a Generic > resource driver for DAI device. We can > have multiple types of DAIs but most of them need the same types of > resources (clocks, pinctrl, pm) sof > for this reason I made it generic. > > > > > > > + > > > +maintainers: > > > + - Daniel Baluta <daniel.baluta@xxxxxxx> > > > + > > > +description: | > > > + On platforms with a DSP we need to split the resource handling between > > > + Application Processor (AP) and DSP. On platforms where the DSP doesn't > > > + have an easy access to resources, the AP will take care of > > > + configuring them. Resources handled by this generic driver are: clocks, > > > + power domains, pinctrl. > > > > The DT should define a DSP node with resources that are part of the > > DSP. What setup the AP has to do should be implied by the compatible > > string and possibly what resources are described. > > We already have a DSP node: Documentation/devicetree/bindings/dsp/fsl,dsp.yaml > but I thought that the resources attached to DAIs are separated from > the resources > attached to the DSP device. I'd agree if the DAI was fully described in the DT. > In the great scheme of ALSA we usually have things like this: > > FE <-----> BE > > In the SOF world FE are defined by topology framework. Back ends are > defined by the machine driver: > > On the BE side we have: > - codec -> this is the specific code > - platform -> this is the DSP > - cpu -> this is our Generic DAI device > > Now, I'm wondering if we can get rid of cpu here and make platform > node (dsp) take care of every > resource (this looks not natural). I would think about how the DT will look when the DSP manages all these resources itself and how the kernel drivers evolve. I think perhaps if you can get rid of the DT part and just define the resources in the driver, then the future transition would be easier. > Perhaps Mark, Liam or Pierre can help me with this. > > > > > > Or maybe the audio portion of the DSP is a child node... > > > > > + > > > +properties: > > > + '#sound-dai-cells': > > > + const: 0 > > > + > > > + compatible: > > > + enum: > > > + - fsl,esai-dai > > > + - fsl,sai-dai > > > > Not very specific. There's only 2 versions of the DSP and ways it is > > integrated? > > As I said above this is not about the DSP, but about the Digital Audio > Intraface. On i.MX > NXP boards we have two types of DAIs: SAI and ESAI. > > <snip> > > > > + pinctrl-0: > > > + description: Should specify pin control groups used for this controller. > > > + > > > + pinctrl-names: > > > + const: default > > > > pinctrl properties are implicitly allowed an don't have to be listed > > here. > > Great. > > > > > > + > > > + power-domains: > > > + $ref: '/schemas/types.yaml#/definitions/phandle-array' > > > > Don't need a type. > > > > > + description: > > > + List of phandles and PM domain specifiers, as defined by bindings of the > > > + PM domain provider. > > > > Don't need to re-define common properties. > > > > You do need to say how many power domains (maxItems: 1?). > > We support multiple power domains, so technically there is no upper > limit. What should I put here in this case? There's an upper limit in the h/w so there should be some sort of limit. > > > + fsl,dai-index: > > > + $ref: '/schemas/types.yaml#/definitions/uint32' > > > + description: Physical DAI index, must match the index from topology file > > > > Sorry, we don't do indexes in DT. > > > > What's a topology file? > > Topology files are binary blobs that contain the description of an > audio pipeline. They are built > are written in a specific format and compiled with alsa-tplg tools in userspace. > > Then loaded via firmware interface inside the kernel. Sounds like a kernel-userspace issue that has nothing to do with DT. How do other platforms deal with mulitple DAIs? Rob