soundwire/SOF: add SoundWire Interface support for AMD SOF stack

[vijendarmukunda]

We have redesigned existing platform device creation logic for SoundWire managers (folder path :sound/soc/amd/ps ) and Implemented generic functions for SoundWire manager probe, start and exit sequence which are common for both Legacy(NO DSP enabled) and SOF stack.

We have also prepared legacy driver patches (folder path: sound/soc/amd/ps), as this PR is targeted for pushing SoundWire/SOF stack changes, we haven't included in this PR. We will push legacy patches for upstream review for alsa devel list once this PR review is completed and merged.

We took Intel code as a reference for implementing generic functions for SoundWire managers.
We are working on generic SoundWire machine driver for AMD platforms. There are some functions implemented in intel SoundWire machine driver code, we want to use it in our machine driver code. we will start a separate thread for the discussion.

Firmware changes are submitted for review. Below is the PR.
thesofproject/sof#8472

[vijendarmukunda]

will fix build dependency error and post v2 set.

[vijendarmukunda]

posted new patch set after fixing build dependency errors.

[vijendarmukunda]

@plbossart, @ranj063
Firmware related changes are got merged. Could you please review this PR?

[plbossart]

SOFCI TEST

[plbossart]

Quite a few changes needed but at a high-level
a) some ACPI sharing is possible
b) there is no need for a startup() step I believe, no need to mimick what Intel did to follow specific power management flows.

[vijendarmukunda]

@plbossart : fixed review comments and posted new patch set. Could you please review the patch set?

[plbossart]

please check with your lawyers. The code is directly inspired from Intel and released with a GPL-2.0-only license.

// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)

There is no '2.0 or later' in the initial Intel code.

[vijendarmukunda]

we will revisit license terms

[plbossart]

so why do you need to implement the startup callback? Everything can be moved to sdw_amd_probe(&sdw_res);

[vijendarmukunda]

As mentioned, earlier, its not possible to move startup callback to sdw_amd_probe(). It will break the interrupt handling functionality in parent driver code as in parent driver irq handler , it will try to deference using sdw_ctx structure. If we move this code to sdw_amd_probe(), we won't get soundwire context structure for handling Soundwire manager interrupts which results in kernel panic. i.e if we move the startup code to sdw_amd_probe() which returns ctx , once the link gets started, we get slave attach interrupt and corresponding SoundWire interrupt will be reported in parent driver IRQ handler.

For example, if we get SoundWire interrupt on SW0 link, in this case in parent driver irq handler code, we will retrieve amd_manger structure from ctx pdev get driver data as shown below.

if (val & ACP_SDW0_IRQ_MASK) {
amd_manager = dev_get_drvdata(&adata->sdw->pdev[0]->dev);

if startup is combined in sdw_amd_probe() call, as ctx is not returned to parent driver, it will result in kernel panic.

[plbossart]

Sorry, not following any of this.

	sdw = sdw_amd_probe(&sdw_res);
	if (!sdw) {
		dev_err(sdev->dev, "SoundWire probe failed\n");
		return -EINVAL;
	}

	/* save context */
	acp_data->sdw = sdw;
	return sdw_amd_startup(acp_data->sdw);

why can't you pass acp_data in the probe and store the context inside of sdw_amd_probe?

You are trying to stick to an Intel definition of interfaces which makes no sense for you.

[vijendarmukunda]

1. acp_data is not a common private data structure for legacy stack and sof stack. It will be different one for legacy stack.
2. We want to abstract usage of parent driver structures in amd_init.c file.
3. I think its still possible, passing parent driver private data structure sdw member to the probe function and assign it in probe function itself followed by invoking start sequence in the same probe call.
4. We want to divide whole probe and start functionality in two functions to increase the readability and scalability and we want to leave a place holder so that in future if start() functionality needs to move to some other place for example after FW loading, there will no option left with this approach.
5. For example, we need to support a SOF scenario. In case of firmware load failure, there is no point in starting the soundwire managers. In this case if we don't have startup sequence separately, we can't support it.

We don't see any major issue in exporting one more function(start() function) other than probe().
Please note our intention is not to replicate everything in Intel style.

[plbossart]

You really want to start the links as early as possible to make sure the peripherals are enumerated in parallel with the rest of firmware loading.

I can't force you to listen or change your code, but at this point you really don't have a good technical rationale for splitting probe/start. I am not the final reviewer so others might have a different opinion.

[vijendarmukunda]

@plbossart : We accept your review comments. We will do the code modifications and respin the patch set and let you know the test results.

[bardliao]

I agree with @plbossart. sdw_amd_startup() is only used here. It is not necessary to have a separated startup function.

[vijendarmukunda]

@bardliao : will make the changes as per @plbossart comments will combine probe and start sequence and remove separate export function.

[vijendarmukunda]

Posted new patch set after fixing review comments.

[plbossart]

two minor points can be improved, see below.

[vijendarmukunda]

pushed new patch set after fixing review comments

[plbossart]

@bardliao can you take a look?

[bardliao]

LGTM

[plbossart]

@vijendarmukunda probably best to wait until next year and the rc1 to send those patches upstream.

[vijendarmukunda]

@plbossart : will wait till rc1 release. I do have one more question. How can i send patch series which has to go two sub systems?
First patch ( ASoC/soundwire: implement generic api for scanning amd soundwire controller) should be pushed to asoc/soundwire tree.
On top of SoundWire patches should be pushed.

If I push only SoundWire patches then i need to wait till SoundWire patches got reflected in AsoC tree which blocks submitting
ASoC/SOF patches which are part of #4699.

Is it possible to send whole patch series at one go so that two subsystems will pick the patch series?

[plbossart]

The dependency between the two subsystem is not easy to navigate.

You have to CC: both Mark Brown and Vinod Koul , clearly spell out the dependency in the cover letter and let them figure things out. One of the two will provide his Acked-by tag and the other will apply the patches.