Skip to content

Conversation

@plbossart
Copy link
Member

This PR adds yet another set of cleanups
a) all the helpers are now exposed as hw_ops callbacks
b) the auxiliary device code is now completely generic.

This will allow for easier introduction of new hw_ops implementations in the future without throwing away all the goodness of the auxiliary device handling. Since there will be a tighter coupling with HDaudio, it's likely that future hw_ops are implemented in the SOF driver directly.

bardliao
bardliao previously approved these changes Nov 2, 2022
@plbossart
Copy link
Member Author

I'll rebase when PR #3933 is merged, and remove the draft status.

Before introducing new hardware with completely different register
spaces and programming sequences, we need to abstract some of the
existing routines in hw_ops that will be platform-specific. For now we
only use the 'cnl' ops - after the first Intel platform with SoundWire
capabilities.

Rather than one big intrusive patch, hw_ops are introduced in this
patch so show the dependencies between drivers. Follow-up patches will
introduce callbacks for debugfs, power and bus management.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
No functionality change, only add indirection for debugfs helpers.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
No functionality change, only add indirection for DAI registration
helper.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
No functionality change, only add indirection for bus management
helpers.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
No functionality change, only add indirection for link power
management helpers.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
No functionality change, only add indirection for in-band wake
management helpers.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
The auxdevice layer is completely generic, it should be split from
intel.c which is only geared to the 'cnl' hw_ops now.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
@plbossart
Copy link
Member Author

v2 - comments from @RanderWang addressed

diff --git a/drivers/soundwire/intel.h b/drivers/soundwire/intel.h
index 7eb48e459f6b..de9883313c8f 100644
--- a/drivers/soundwire/intel.h
+++ b/drivers/soundwire/intel.h
@@ -54,96 +54,88 @@ struct sdw_intel {
 
 #define INTEL_MASTER_RESET_ITERATIONS	10
 
+#define SDW_INTEL_CHECK_OPS(sdw, cb)	((sdw) && (sdw)->link_res && (sdw)->link_res->hw_ops && \
+					 (sdw)->link_res->hw_ops->cb)
+#define SDW_INTEL_OPS(sdw, cb)		((sdw)->link_res->hw_ops->cb)
+
 static inline void sdw_intel_debugfs_init(struct sdw_intel *sdw)
 {
-	if (sdw && sdw->link_res && sdw->link_res->hw_ops &&
-	    sdw->link_res->hw_ops->debugfs_init)
-		sdw->link_res->hw_ops->debugfs_init(sdw);
+	if (SDW_INTEL_CHECK_OPS(sdw, debugfs_init))
+		SDW_INTEL_OPS(sdw, debugfs_init)(sdw);
 }
 
 static inline void sdw_intel_debugfs_exit(struct sdw_intel *sdw)
 {
-	if (sdw && sdw->link_res && sdw->link_res->hw_ops &&
-	    sdw->link_res->hw_ops->debugfs_exit)
-		sdw->link_res->hw_ops->debugfs_exit(sdw);
+	if (SDW_INTEL_CHECK_OPS(sdw, debugfs_exit))
+		SDW_INTEL_OPS(sdw, debugfs_exit)(sdw);
 }
 
 static inline int sdw_intel_register_dai(struct sdw_intel *sdw)
 {
-	if (sdw && sdw->link_res &&  sdw->link_res->hw_ops &&
-	    sdw->link_res->hw_ops->register_dai)
-		return sdw->link_res->hw_ops->register_dai(sdw);
+	if (SDW_INTEL_CHECK_OPS(sdw, register_dai))
+		return SDW_INTEL_OPS(sdw, register_dai)(sdw);
 	return -ENOTSUPP;
 }
 
 static inline void sdw_intel_check_clock_stop(struct sdw_intel *sdw)
 {
-	if (sdw && sdw->link_res &&  sdw->link_res->hw_ops &&
-	    sdw->link_res->hw_ops->check_clock_stop)
-		sdw->link_res->hw_ops->check_clock_stop(sdw);
+	if (SDW_INTEL_CHECK_OPS(sdw, check_clock_stop))
+		SDW_INTEL_OPS(sdw, check_clock_stop)(sdw);
 }
 
 static inline int sdw_intel_start_bus(struct sdw_intel *sdw)
 {
-	if (sdw && sdw->link_res &&  sdw->link_res->hw_ops &&
-	    sdw->link_res->hw_ops->start_bus)
-		return sdw->link_res->hw_ops->start_bus(sdw);
+	if (SDW_INTEL_CHECK_OPS(sdw, start_bus))
+		return SDW_INTEL_OPS(sdw, start_bus)(sdw);
 	return -ENOTSUPP;
 }
 
 static inline int sdw_intel_start_bus_after_reset(struct sdw_intel *sdw)
 {
-	if (sdw && sdw->link_res &&  sdw->link_res->hw_ops &&
-	    sdw->link_res->hw_ops->start_bus_after_reset)
-		return sdw->link_res->hw_ops->start_bus_after_reset(sdw);
+	if (SDW_INTEL_CHECK_OPS(sdw, start_bus_after_reset))
+		return SDW_INTEL_OPS(sdw, start_bus_after_reset)(sdw);
 	return -ENOTSUPP;
 }
 
 static inline int sdw_intel_start_bus_after_clock_stop(struct sdw_intel *sdw)
 {
-	if (sdw && sdw->link_res &&  sdw->link_res->hw_ops &&
-	    sdw->link_res->hw_ops->start_bus_after_clock_stop)
-		return sdw->link_res->hw_ops->start_bus_after_clock_stop(sdw);
+	if (SDW_INTEL_CHECK_OPS(sdw, start_bus_after_clock_stop))
+		return SDW_INTEL_OPS(sdw, start_bus_after_clock_stop)(sdw);
 	return -ENOTSUPP;
 }
 
 static inline int sdw_intel_stop_bus(struct sdw_intel *sdw, bool clock_stop)
 {
-	if (sdw && sdw->link_res &&  sdw->link_res->hw_ops &&
-	    sdw->link_res->hw_ops->stop_bus)
-		return sdw->link_res->hw_ops->stop_bus(sdw, clock_stop);
+	if (SDW_INTEL_CHECK_OPS(sdw, stop_bus))
+		return SDW_INTEL_OPS(sdw, stop_bus)(sdw, clock_stop);
 	return -ENOTSUPP;
 }
 
 static inline int sdw_intel_link_power_up(struct sdw_intel *sdw)
 {
-	if (sdw && sdw->link_res && sdw->link_res->hw_ops &&
-	    sdw->link_res->hw_ops->link_power_up)
-		return sdw->link_res->hw_ops->link_power_up(sdw);
+	if (SDW_INTEL_CHECK_OPS(sdw, link_power_up))
+		return SDW_INTEL_OPS(sdw, link_power_up)(sdw);
 	return -ENOTSUPP;
 }
 
 static inline int sdw_intel_link_power_down(struct sdw_intel *sdw)
 {
-	if (sdw && sdw->link_res &&  sdw->link_res->hw_ops &&
-	    sdw->link_res->hw_ops->link_power_down)
-		return sdw->link_res->hw_ops->link_power_down(sdw);
+	if (SDW_INTEL_CHECK_OPS(sdw, link_power_down))
+		return SDW_INTEL_OPS(sdw, link_power_down)(sdw);
 	return -ENOTSUPP;
 }
 
 static inline int sdw_intel_shim_check_wake(struct sdw_intel *sdw)
 {
-	if (sdw && sdw->link_res &&  sdw->link_res->hw_ops &&
-	    sdw->link_res->hw_ops->shim_check_wake)
-		return sdw->link_res->hw_ops->shim_check_wake(sdw);
+	if (SDW_INTEL_CHECK_OPS(sdw, shim_check_wake))
+		return SDW_INTEL_OPS(sdw, shim_check_wake)(sdw);
 	return -ENOTSUPP;
 }
 
 static inline void sdw_intel_shim_wake(struct sdw_intel *sdw, bool wake_enable)
 {
-	if (sdw && sdw->link_res &&  sdw->link_res->hw_ops &&
-	    sdw->link_res->hw_ops->shim_wake)
-		sdw->link_res->hw_ops->shim_wake(sdw, wake_enable);
+	if (SDW_INTEL_CHECK_OPS(sdw, shim_wake))
+		SDW_INTEL_OPS(sdw, shim_wake)(sdw, wake_enable);
 }
 
 #endif /* __SDW_INTEL_LOCAL_H */
diff --git a/drivers/soundwire/intel_auxdevice.c b/drivers/soundwire/intel_auxdevice.c
index 5f192b0cc79e..96c6b2112feb 100644
--- a/drivers/soundwire/intel_auxdevice.c
+++ b/drivers/soundwire/intel_auxdevice.c
@@ -613,7 +613,6 @@ static int __maybe_unused intel_resume_runtime(struct device *dev)
 			return ret;
 		}
 
-
 	} else if (clock_stop_quirks & SDW_INTEL_CLK_STOP_BUS_RESET) {
 		ret = sdw_intel_link_power_up(sdw);
 		if (ret) {

Copy link

@RanderWang RanderWang left a comment

Choose a reason for hiding this comment

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

LGTM

@plbossart plbossart merged commit b01304b into thesofproject:topic/sof-dev Nov 10, 2022
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.

4 participants