Skip to content

drivers: stm32_temp stm32h5 device must disable icache to access cal #57178

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 2, 2023
Merged

drivers: stm32_temp stm32h5 device must disable icache to access cal #57178

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
57bbce3
drivers: stm32_temp stm32h5 device must disable icache to access cal
tu-brian Apr 24, 2023
3c5f6b2
dts: Add die_temp sensor to stm32h5
tu-brian Apr 24, 2023
e36870e
boards: Enable die temp sensor on stm32h573i_dk
tu-brian Apr 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions boards/arm/stm32h573i_dk/stm32h573i_dk.dts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
sw0 = &user_button;
watchdog0 = &iwdg;
spi-flash0 = &mx25lm51245;
die-temp0 = &die_temp;
};
};

Expand Down Expand Up @@ -236,3 +237,7 @@
};
};
};

&die_temp {
status = "okay";
};
13 changes: 13 additions & 0 deletions drivers/sensor/stm32_temp/stm32_temp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@
#include <zephyr/drivers/sensor.h>
#include <zephyr/drivers/adc.h>
#include <zephyr/logging/log.h>
#if defined(CONFIG_SOC_SERIES_STM32H5X)
#include <stm32_ll_icache.h>
#endif /* CONFIG_SOC_SERIES_STM32H5X */

LOG_MODULE_REGISTER(stm32_temp, CONFIG_SENSOR_LOG_LEVEL);
#define CAL_RES 12
Expand Down Expand Up @@ -98,6 +101,11 @@ static int stm32_temp_channel_get(const struct device *dev, enum sensor_channel
}

#if HAS_CALIBRATION

#if defined(CONFIG_SOC_SERIES_STM32H5X)
LL_ICACHE_Disable();
#endif /* CONFIG_SOC_SERIES_STM32H5X */

temp = ((float)data->raw * adc_ref_internal(data->adc)) / cfg->cal_vrefanalog;
temp -= (*cfg->cal1_addr >> cfg->ts_cal_shift);
#if HAS_SINGLE_CALIBRATION
Expand All @@ -110,6 +118,11 @@ static int stm32_temp_channel_get(const struct device *dev, enum sensor_channel
temp /= ((*cfg->cal2_addr - *cfg->cal1_addr) >> cfg->ts_cal_shift);
#endif
temp += cfg->cal1_temp;

#if defined(CONFIG_SOC_SERIES_STM32H5X)
LL_ICACHE_Enable();
#endif /* CONFIG_SOC_SERIES_STM32H5X */

Copy link
Collaborator

Choose a reason for hiding this comment

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

I know it should be enabled, but... shouldn't be restored to original state? (i.e. enabled if was enabled, but disable if it was disabled)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I Get your point, but for this fix I simply ported what was done in 8d5ff8c to here. When searching the codebase for LL_CACHE I can't find any other place where the check before enable is implemented

Copy link
Collaborator

Choose a reason for hiding this comment

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

I Get your point, but for this fix I simply ported what was done in 8d5ff8c to here. When searching the codebase for LL_CACHE I can't find any other place where the check before enable is implemented

Ok I see. Sorry for the noise. By the way, another point... This is instruction cache, so what does it mean reading 0x8fff814-0x8fff818 addresses? Is it some code fetched from that location? Can you comment about it a little bit more in the commit header? Sorry again if the question is pointless...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@FRASTM Can you please explain why the fix you did in 8d5ff8c solved the bus error on reading the read-only area?

Copy link
Collaborator

@FRASTM FRASTM May 2, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@FRASTM Can you please explain why the fix you did in 8d5ff8c solved the bus error on reading the read-only area?

If I remember, I got the idea from https://github.com/STMicroelectronics/STM32CubeH5/tree/main/Projects/NUCLEO-H563ZI/Examples_LL/UTILS/UTILS_ReadDeviceInfo

Copy link
Collaborator

Choose a reason for hiding this comment

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

(For future readers)

On STM32H5, the ICACHE block is interposed on C-bus between Cortex-M33 and FLASH. (See RM0492 §2.1.1 Fast C-bus)

The ICACHE determines whether an access is cacheable or not based on the AHB attribute, which depends on MPU configuration. (§8.4.6 Cacheable and non-cacheable traffic).

When a cacheable transaction is received from the Cortex-M33, if the requested data is not present in cache, a cache miss occurs and a cache line refill (128-bit access via burst) is performed (§8.4.7 Cacheable accesses).

However, all accesses to OTP and RO regions must be done with caching disabled (§7.3.2 FLASH signals); indeed, accesses to this region must be 16 or 32-bit sized, or a bus error is raised (§7.5.9 One-time-programmable and read-only memory protections - table 38 OTP/RO access constraints).

By disabling the ICACHE, we ensure the manufacturing (RO) flash region is accessed by 16/32-bit size as it should.

#else
/* Sensor value in millivolts */
int32_t mv = data->raw * adc_ref_internal(data->adc) / 0x0FFF;
Expand Down
12 changes: 12 additions & 0 deletions dts/arm/st/h5/stm32h5.dtsi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,18 @@
status = "disabled";
};
};

die_temp: dietemp {
compatible = "st,stm32-temp-cal";
ts-cal1-addr = <0x08fff814>;
ts-cal2-addr = <0x08fff818>;
ts-cal1-temp = <30>;
ts-cal2-temp = <130>;
ts-cal-vrefanalog = <3300>;
ts-cal-resolution = <12>;
io-channels = <&adc1 16>;
status = "disabled";
};
};

&nvic {
Expand Down