drivers: serial: lis3mdl: support for SPI #75858
Conversation
zephyrbot
requested review from
jeppenodgaard,
MaureenHelm,
teburd,
tristan-google,
ubieda and
yperess
|
The current Kconfig default is to disable support for die temperature measurement and breaks backwards comparability. Should I enable it by default? |
|
Hello @DeMurlot, and thank you very much for your first pull request to the Zephyr project! |
I think it is safest to keep it as is. If it is changed I think that change should have its own commit, and maybe probably be mentioned in the 4.0 migration guide. |
|
The commits needs a body as described in: |
|
|
||
| __ASSERT_NO_MSG(chan == SENSOR_CHAN_ALL); | ||
|
|
||
| /* fetch magnetometer sample */ | ||
| if (i2c_burst_read_dt(&config->i2c, LIS3MDL_REG_SAMPLE_START, | ||
| (uint8_t *)buf, 8) < 0) { | ||
| (uint8_t *)buf, 6) < 0) { |
There was a problem hiding this comment.
But where are you reading the two temperature bytes? I see nothing under ifdefs here below...
| @@ -85,11 +91,12 @@ int lis3mdl_sample_fetch(const struct device *dev, enum sensor_channel chan) | |||
| LOG_DBG("Failed to fetch temperature sample."); | |||
| return -EIO; | |||
| } | |||
| drv_data->temp_sample = sys_le16_to_cpu(buf[3]); | |||
There was a problem hiding this comment.
where did you read these two bytes?
EDIT:
Sorry, I saw the code
| @@ -0,0 +1,8 @@ | |||
| # Copyright (c) 2017, Linaro Limited | |||
There was a problem hiding this comment.
2024 and maybe not Linaro
| @@ -0,0 +1,8 @@ | |||
| # Copyright (c) 2017, Linaro Limited | |||
drivers/sensor/st/lis3mdl/lis3mdl.c
Outdated
| static struct lis3mdl_data lis3mdl_data_##inst; \ | ||
| static struct lis3mdl_config lis3mdl_config_##inst = \ | ||
| LIS3MDL_CONFIG_I2C(inst); \ | ||
| LIS3MDL_DEVICE_INIT(inst) |
There was a problem hiding this comment.
this macro is common between i2c and spi, so maybe you can call only once in LIS3MDL_DEFINE() as some other st driver do.
| .bus_init = lis3mdl_i2c_init, \ | ||
| .bus_cfg.i2c = I2C_DT_SPEC_INST_GET(inst), \ | ||
| COND_CODE_1(DT_INST_NODE_HAS_PROP(inst, irq_gpios), \ | ||
| (LIS3MDL_CFG_IRQ(inst)), ()) \ |
There was a problem hiding this comment.
same for i2c and spi. Maybe call only once?
|
@DeMurlot I guess you may also want to add the spi test for the sensor in |
The LIS3MDL sensor driver is slowed down by an additional transaction to fetch the die temperature. This commit allowes to enable/disable this via a KConfig option. Default is enables in order to preserve backwards compatability. Signed-off-by: De Murlot <demurlotpierre@gmail.com>
The LIS3MDL sensor driver does not support SPI. This commit adds support for usage through SPI. Signed-off-by: De Murlot <demurlotpierre@gmail.com>
|
This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time. |
This pull request add two things to the lis3mdl driver
The first part is heavily inspired by the lsm6dsl's way of supporting both I2C and SPI.