Skip to content

Conversation

@dbaluta
Copy link
Collaborator

@dbaluta dbaluta commented Oct 14, 2019

This is just to get your opinion on two matters regarding having a generic approach for ACPI / DT platforms in SOF.

There are two patches in the series each one tackling a different problem.

  1. ASoC: SOF: Introduce generic machine driver probing mechanism

Currently SOF probes machine driver by manually creating a platform device. While this will just work fine for DT platforms also, it is not really necessary. We can describe the machine driver inside a DT node and then create a device platform from that node by calling of_platform_populate.

Because Intel uses platform_device_register / ARM uses of_platform_populate we need a function to abstract these two operations. Thus we introduce a new SOF operation: sof_machine_register which each platform will implement with specific code.

  1. ASoC: SOF: Introduce generic machine driver descriptor

snd_sof_pdata should really be generic. For this reason we replace snd_soc_acpi_mach with snd_soc_fw_mach. The latter will be an union containing specific machine descriptors for ACPI / DT.

The code is far from being complete or proper tested but I need to understand if this is the correct direction.

Cc: @groncarolo @paulstelian97 @jlacla

SOF probes machine driver by creating its associated device.

On Intel platforms the machine driver is probed by creating
a platform device and the information describing the machine
driver is syntethically generated via tabels of snd_soc_acpi_mach
structures.

On ARM platforms (specifically on i.MX) the machine driver can
be described via a DT node.

In both cases the operation of probing the machine driver is
implemented via generic function snd_sof_machine_register.
This function calls into device specific machine_register function.

This patch does the following:

(1) Introduce snd_sof_machine_register
(2) Replace current creation of machine driver by
snd_sof_machine_register
(3) Introduce a generic function that creates a platform_device
and use it for Intel platforms
(4) Introduce imx8_machine_register which expects that the machine
driver for i.MX platforms is described inside SOF DSP node.

Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
snd_soc_fw_mach will be the generic machine driver descriptor.
It contains the type of the machine driver (e.g ACPI/DT) and
a pointer to the specific machine driver acpi/of.

On i.MX platforms machine driver description will be read from Device
Tree, so we don't actually need nothing more than a pointer to
the corresponding device_node. We will use it to read any machine
driver property.

Because the nocodec implementation is generic enough we can keep
the existing ACPI approach even for i.MX8.

Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
Copy link
Member

@plbossart plbossart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No real objections, but @ranj063 's work on MFD is going to change the way we deal with machine drivers, i.e. we may have more that one machine driver loaded for the same ACPI/PCI ID.
In short, we should probably pause a bit here and avoid going too fast since we will have conflicting changes going in at the same time


/* machine driver probing */
int (*machine_register)(struct snd_sof_dev *sof_dev); /* mandatory */

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be optional?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think yes, although I haven't seen any platform which doesn't have a "codec" scenario.


static int imx8_machine_register(struct snd_sof_dev *sdev)
{
return devm_of_platform_populate(sdev->dev);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is this supposed to do?

Copy link
Collaborator Author

@dbaluta dbaluta Oct 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The SOF DSP DT will look like this:


dsp {
       compatible = "fsl,imx8qxp-dsp";
       reg = <0x596e8000 0x88000>;
       clocks = <..>
       mach_drv: mach_drv {
                compatible = "simple-scu-audio-card";
                simple-audio-card,widgets =<..>
                simple-audio-card,routing =<..>
                simple-audio-card,dai-link {
                        format = "i2s";
                        codec {
                                sound-dai = <&cs42888>;
                       };
                };
        };

devm_of_platform_populate - walks the device tree and creates devices from nodes.

Now, it starts from DSP root node and it will find mach_drv child node. It reads the node and creates a platform_device from it. It is similar with platform_device_register_data but it reads the information from DT instead of having them passed via pdata argument of platform_device_register_data.

@dbaluta
Copy link
Collaborator Author

dbaluta commented Oct 15, 2019

@plbossart sure lets wait for @ranj063 PR to get in and then I will rebase my PR.

@dbaluta
Copy link
Collaborator Author

dbaluta commented Nov 15, 2019

patches already taken by @ranj063 in #1478

@dbaluta dbaluta deleted the topic/sof-dev branch August 10, 2020 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants