Fix for issue #12104 (STM32 can_init_freq() ignores frequency)

[mprse]

Summary of changes

This PR fixes issue #12104.

Currently can_init_freq()/can_init_freq_direct() ignores given frequency and sets default CAN frequency.
This is a bug introduces while adding the static pin-map support for STM CAN. This PR fixes this problem and optimizes the code.

Impact of changes

Migration actions required

Documentation

---

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

@jeromecoutant @bikeNomad

---

[kjbracey]

Fix looks correct, but looking at code I wonder why can_init above doesn't just call can_init_freq(obj, rd, td, 100000), just as CAN_INIT_DIRECT calls CAN_INIT_FREQ_DIRECT.

[mprse]

@kjbracey-arm optimized can_init() as suggested.

[bikeNomad]

Looks good to me.

[jeromecoutant]

CAN_INIT_DIRECT definition seems to be unused ?

[kjbracey]

CAN_INIT_DIRECT definition seems to be unused ?

You're half right - CAN_INIT_DIRECT is defined as either _can_init_direct or can_init_direct, depending on whether the target is fully static pinmap enabled.

If the latter, then can_init_direct is part of the HAL API and is required.

If the former, then no, we don't need _can_init_direct, as we actually use _can_init_freq_direct.

So the macro magic of CAN_INIT_DIRECT could be dropped. Only need it for CAN_INIT_FREQ_DIRECT - the core.

[kjbracey]

@mrpse - could you respond to comments above?

[mprse]

Sorry, guys. I was off today.

Generally, I agree with @kjbracey-arm.

All can_init, can_init_direct, can_init_freq, can_init_freq_direct are required by HAL API:

mbed-os/hal/can_api.h

Lines 70 to 73 in e2ee381

	void can_init(can_t *obj, PinName rd, PinName td);
	void can_init_direct(can_t *obj, const can_pinmap_t *pinmap);
	void can_init_freq(can_t *obj, PinName rd, PinName td, int hz);
	void can_init_freq_direct(can_t *obj, const can_pinmap_t *pinmap, int hz);

When the target is not ready for static pin-map then we are using the following weak direct functions (it simply calls regular init):

mbed-os/hal/static_pinmap.cpp

Lines 76 to 85 in 9209b98

	#if DEVICE_CAN
	MBED_WEAK void can_init_freq_direct(can_t *obj, const can_pinmap_t *pinmap, int hz)
	{
	can_init_freq(obj, pinmap->rd_pin, pinmap->td_pin, hz);
	}
	MBED_WEAK void can_init_direct(can_t *obj, const can_pinmap_t *pinmap)
	{
	can_init(obj, pinmap->rd_pin, pinmap->td_pin);
	}

The problem is when the same HAL implementation is for targets that support and does not support static pin-map (like in this case). That is why we are using this trick with INIT_DIRECT macros. When all STM targets will be ready for the static pin map we can remove these macros.

Regarding CAN_INIT_DIRECT it looks like it can be removed since we do not use it. I will look closer into it tomorrow.

[mprse]

Dropped CAN_INIT_DIRECT macro.

[adbridge]

@mprse could you please add a little more detail to the summary of changes. For fixes we would expect to see a short summary of what the actual issue is and then some detail about how it has actually been fixed. This makes it much easier for people to see what has changed and why. Thanks.

[adbridge]

I have started the CI in the interim

[mbed-ci]

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 1
Build artifacts

[mprse]

@mprse could you please add a little more detail to the summary of changes. For fixes we would expect to see a short summary of what the actual issue is and then some detail about how it has actually been fixed. This makes it much easier for people to see what has changed and why. Thanks.

Updated PR description as requested.