Remove STM's qspi_api trace which is verbose and slows down operations

[LDong-Arm]

Summary of changes

While tracing can be useful, trace in STM's qspi_api HAL is verbose and slows down operations - printing takes way longer than most storage operations.

Since HAL implementations are fundamental, it should have very low overhead and be assumed to work reliable without the need for tracing. STM qspi_api is the only place in targets/ where mbed-trace is used, so this PR removes it.

Impact of changes

No more traces generated from STM's qspi_api implementation, when mbed-trace is enabled.

Migration actions required

None.

Documentation

None.

---

Pull request type

[x] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

---

Test results

[] No Tests required for this change (E.g docs only update)
[x] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

---

Reviewers

@ARMmbed/mbed-os-hal @mbed-os-core @ARMmbed/team-st-mcd

---

[ciarmcom]

@LDong-Arm, thank you for your changes.
@ARMmbed/mbed-os-core @ARMmbed/mbed-os-maintainers please review.

[jeromecoutant]

Impact of changes

No more traces generated from STM's qspi_api implementation, when mbed-trace is enabled.

You have to explain me this...
If you don't want trace, you disable trace...

If you find it too much verbose, you can change some info trace to debug

[0xc0170]

info is way to generous here. However, the trace that is being removed is only printing arguments and the name of the function being executed. The same could be done on the driver level (another question would be if we really need this type of tracing in the driver, but that is for a review in another issue/pull request).

[jeromecoutant]

info is way to generous here.

OK, so just replace tr_info in qspi_command_transfer by tr_debug

[LDong-Arm]

@jeromecoutant @0xc0170 I'm happy to change it to tr_debug().
But the main concern is consistency (instead of verbosity). There are many targets and HAL implementations in targets/ - SPI, OSPI, TRNG, Flash, etc., but why is mbed-trace only present in this single file (QSPI)? My initial intention was to make them consistent.

[kjbracey]

Part of the answer is mbed-trace is newer than most HALs, so most sorted out their porting infrastructure without it, and didn't update.

When something new is progressing slowly through the system without an active conversion push, you don't want to back it out on the grounds of consistency. (But that natural response is a good reason to do a conversion push if you mean it).

Other answer is that most people don't want to see debug from subsystems they're not working on, such as the HAL, but there isn't a per-subsystem/file switch for it.

Linux trace gets this right in that its printk "debug" level is not all-or-nothing in filters, like other levels, but has to be activated for specific files. We could do that using the mbed-trace group (ideally with some macro magic to allow compiler exclusion).

Best current compromise is to make sure this debug is at debug level, and expect normal non-developer operation (like Greentea tests) to set global filter level to info or higher to avoid it.

[LDong-Arm]

Other answer is that most people don't want to see debug from subsystems they're not working on, such as the HAL, but there isn't a per-subsystem/file switch for it.

This is the key problem. I want to debug the project I work on by setting trace level to debug, without traces from other modules.
IMO each module should have a config to enable/disable its own traces, like lwipstack:

mbed-os/connectivity/lwipstack/mbed_lib.json

Lines 57 to 60 in 78c8d84

	"enable-ppp-trace": {
	"help": "Enable trace support for PPP interfaces (obsolete: use netsocket/ppp configuration instead)",
	"value": false
	},

So now I propose one "HAL tracing" config that affects all HALs of all targets (for now, it's only relevant to ST's QSPI).

[kjbracey]

There is group filtering in mbed-trace you can use. I'm not sure how many options it offers though - can you say "all except X,Y,Z" or "only X,Y,Z"?

I know you can't do "all info, and debug only from X". That's the main omission. Plus the fact that there's no compile-time group exclusion.

There should be no need for every component to invent its own local switch - the intent was that mbed-trace would handle this centrally using the groups, it's just that the API is not fleshed out enough.

[LDong-Arm]

@kjbracey-arm That's a good idea, I'll try it.

[0xc0170]

@kjbracey-arm That's a good idea, I'll try it.

@LDong-Arm Any update for this PR?

[LDong-Arm]

@kjbracey-arm That's a good idea, I'll try it.

@LDong-Arm Any update for this PR?

I'll close this PR and create a new one to change tr_info to tr_debug, because GitHub won't let me rename my branch (remove_qspi_trace) while keeping the same PR.