On Fri, Jun 28, 2019 at 05:12:46PM +0800, Leo Yan wrote:
On Fri, Jun 28, 2019 at 09:53:58AM +0100, Andrew Murray wrote:
[...]
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c
[...]
+#define PARAM_PM_SAVE_DISABLE 0 +#define PARAM_PM_SAVE_ENABLE 1 +#define PARAM_PM_SAVE_FIRMWARE 2
+static int pm_save_enable = PARAM_PM_SAVE_FIRMWARE; +module_param(pm_save_enable, int, 0644); +MODULE_PARM_DESC(pm_save_enable, "Save/restore state on power down: "
"0 = disabled, 1 = enabled, 2 = firmware");I understand if set pm_save_enable = 2 (firmware), then driver will depend on drvdata->pm_save_enable to make decision for context saving and restoring.
Maybe we can simplize to set pm_save_enable for binary value: 0 (disabled) or 1 (enabled). The reason is if we set the module parameter 'pm_save_enable = 1', then we can set every ETM device's drvdata->pm_save_enable in initialization phase. So in the probe function, we can use below code:
drvdata->pm_save_enable = pm_save_enable ? : etm4_needs_save_restore(dev);
This means that when the module parameter is set to 1, then we only save/restore if the firmware suggests it is needed.
Sorry, I seemingly can't read code today.
If the module parameter is set to 1, then we will always set every device 'drvdata->pm_save_enable' to 1. So in this case, the module parameter will override the firmware property and always save/restore contexts for ETM.
However - what happens on hardware that ignores the PU bit (and thus requires save/restore), yet it's firmware doesn't have the 'arm,coresight-needs-save-restore' property? There is no way to override the firmware and always save/restore.
Actually I suggested to give the module parameter with high priority and when the module parameter has set to 1, then it can override firmware 'arm,coresight-needs-save-restore' property.
If we set the module parameter in kernel command line or when load the module, its value can be used in the function etm4_probe(). So in the driver probing, it detects the module parameter is 1, then it can directly set every device 'drvdata->pm_save_enable' to 1. Thus we can always save/restore context for ignoring the PU bit case.
In any case, not only do we want to override the firmware to always save/restore. Sometimes we may also want to override the firmware to never save/restore (despite the firmware having the 'arm,coresight-needs-save-restore' flag present). For example to debug power management.
Thus with this current approach you can override the firmware to either enable or disable save/restore.
Thanks,
Andrew Murray
(It's also quite helpful for debugging to be able to change the module parameter at run time.)
[...]
Do we need to disable trace unit at the end of saving flow?
At the start of this function we take the OS lock, this has the effect of also disabling the trace. Therefore I don't think it's necessary to do more.
Okay, I read the comment and thanks for reminding.
Thanks, Leo Yan