[stm32] Add comparator platform driver for f3 and l4 #41

rleh · 2018-07-15T17:20:17Z

Replaces the old comp/stm32/comp.hpp.in which neither compiled nor ever worked correctly.

Two examples for Nucleo-L432 and STM32F3-Discovery boards are added.

In the future (with data from PDF datasheets) I plan to improve the input-selection enums and to add support for more STM32 controllers.

Todo

Test in hardware
Add a connect<>(); method?

salkinium · 2018-07-15T18:00:32Z

src/modm/platform/comp/stm32/module.lb

+        if not device.identifier["name"] in ["31", "32", "33", "42", "43", "51", "52", "62"]:
+            return False
+
+    driver_type = device.get_driver("comp")["type"]


Check with has_driver() if there even is a comp on this target.

salkinium · 2018-07-15T18:31:28Z

You may only want to enable this module for F3 and L4:
https://2776-77459564-gh.circle-artifacts.com/0/log/stm32f051c4t.log

salkinium · 2018-07-15T21:29:27Z

src/modm/platform/comp/stm32/module.lb

+        return False
+
+    if not device.has_driver("comp"):
+        return False


Use this instead and make it the first check of the function. This notation checks if a driver of name comp and type startswith stm32 exists.

if not device.has_driver("comp:stm32*"): return False

Since this function is called for all targets, it is important to reject the module as early as possible, since the device.identifier has no rigid structure and may not have a family or name key and this code may therefore fail.

salkinium · 2018-07-15T21:29:45Z

examples/nucleo_l432kc/comp/main.cpp

@@ -0,0 +1,47 @@
+/*
+ * Copyright (c) 2016-2017, Niklas Hauser


I don't remember writing this example… 😜

salkinium · 2018-07-15T21:32:19Z

src/modm/platform/comp/stm32/base.hpp.in

+			LowPower		= 0b01 << 2,
+			MediumSpeed		= 0b10 << 2,
+			HighSpeed		= 0b11 << 2,
+			{% elif driver.type in ["stm32-tsmc90_cube"] -%}


These should be renamed in this file:
https://github.com/modm-io/modm-devices/blob/develop/tools/generator/dfg/stm32/stm_peripherals.py

Would it be a lot of work to check the binary compatiblity of all STM32 Comp devices and add them too?
You'd need to compare the peripheral memory map overview of all devices with this functionality.

I have no idea how the different comparator IPs should be grouped, so I don't think a renaming makes sense at the moment:

stm32-tsmc90_cube

stm32-tsmc90_h7_cube

stm32-tsmc90_orca512_cube

stm32-v1.0

stm32-v1.2

stm32-v1.3

stm32-v1.4

stm32-v3.4

stm32-v3.6

I don't think it makes sense to compare the memory maps. I prefer to wait a few weeks and compare the data of the register descriptions from the PDF data sheets.

The memory maps of L43x and F303/F3x8 look very similar, but some bits have other meaning: On L43x the HighSpeed mode is 0b00, on F303/F3x8 the same bit have to be 0b11 to enable the HighSpeed mode 😒

Data about the configurations of the input- or output- selection (etc.) bits is not present in the memory maps and CMSIS header files.

I prefer to wait a few weeks and compare the data of the register descriptions from the PDF data sheets.

Someone is optimistic 😳

salkinium · 2018-07-15T21:33:48Z

src/modm/platform/comp/stm32/module.lb

+        if not device.identifier["name"] in ["31", "32", "33", "42", "43", "51", "52", "62"]:
+            return False
+    else:
+        return False


Is filtering for has_driver + type not enough?

I would rather remove the driver type filter. Shall I remove it?

driver_type = device.get_driver("comp")["type"] if not driver_type in ["stm32-v1.3", "stm32-tsmc90_cube"]: print("Unsupported COMP driver type: ", driver_type) return False

No, it filters at least for the platform stm32.

Oh, this way you mean, yes, that's fine with me.

To remove it or to leave it there?

I mean using this instead of the current if not device.has_driver("comp:stm32*"):

driver_type = device.get_driver("comp")["type"] if not driver_type in ["stm32-v1.3", "stm32-tsmc90_cube"]: env.log.debug("Unsupported COMP driver type: ", driver_type) return False

Well, maybe get rid of the env.log.debug, module registration should not be logged, it would spam the log.

Without device.has_driver() devices without comp will fail because TypeError("'NoneType' object is not subscriptable",).

I would suggest:

if not device.has_driver("comp:stm32*"): return False if not device.get_driver("comp")["type"] in ["stm32-v1.3", "stm32-tsmc90_cube"]: return False

salkinium · 2018-07-15T21:42:27Z

src/modm/platform/comp/stm32/comp.hpp.in

+			Dac1Channel1	= 0b100 << 4,
+			GpioA4			= 0b100 << 4,
+			Dac1Channel2	= 0b101 << 4,
+			GpioA5			= 0b101 << 4,


What's the relation between this information and the one encoded in the gpio driver in the device file?
For the F303x{bcde}, I find this data in the files:

<gpio port="a" pin="4"> <signal driver="adc" instance="2" name="in1"/> <signal driver="comp" instance="1" name="inm"/> <signal driver="comp" instance="2" name="inm"/> <signal driver="comp" instance="3" name="inm"/> <signal driver="comp" instance="4" name="inm"/> <signal driver="comp" instance="5" name="inm"/> <signal driver="comp" instance="6" name="inm"/> <signal driver="comp" instance="7" name="inm"/> <signal device-size="b|c" driver="dac" name="out1"/> <signal device-size="d|e" driver="dac" instance="1" name="out1"/> <signal driver="opamp" instance="4" name="vinp"/> <signal driver="opamp" instance="4" name="vinp_sec"/> <signal af="2" driver="tim" instance="3" name="ch2"/> <signal af="3" driver="tsc" name="g2_io1"/> <signal af="5" driver="spi" instance="1" name="nss"/> <signal af="6" driver="i2s" instance="3" name="ws"/> <signal af="6" driver="spi" instance="3" name="nss"/> <signal af="7" driver="usart" instance="2" name="ck"/> </gpio> <gpio port="a" pin="5"> <signal driver="adc" instance="2" name="in2"/> <signal driver="comp" instance="1" name="inm"/> <signal driver="comp" instance="2" name="inm"/> <signal driver="comp" instance="3" name="inm"/> <signal driver="comp" instance="4" name="inm"/> <signal driver="comp" instance="5" name="inm"/> <signal driver="comp" instance="6" name="inm"/> <signal driver="comp" instance="7" name="inm"/> <signal device-size="b|c" driver="dac" name="out2"/> <signal device-size="d|e" driver="dac" instance="1" name="out2"/> <signal driver="opamp" instance="1" name="vinp"/> <signal driver="opamp" instance="1" name="vinp_sec"/> <signal driver="opamp" instance="2" name="vinm"/> <signal driver="opamp" instance="2" name="vinm_sec"/> <signal driver="opamp" instance="3" name="vinp"/> <signal driver="opamp" instance="3" name="vinp_sec"/> <signal af="1" driver="tim" instance="2" name="ch1"/> <signal af="1" driver="tim" instance="2" name="etr"/> <signal af="3" driver="tsc" name="g2_io2"/> <signal af="5" driver="spi" instance="1" name="sck"/> </gpio>

The Gpio signal data from the device files only contains the information that the gpio pin can be used as an input or output of a comparator instance.
Information about how to set the COMP*(E)SEL bits in the COMPx->CSR register is additionally needed to configure the comparator.

salkinium · 2018-07-15T21:48:14Z

src/modm/platform/comp/stm32/comp.hpp.in

+			Tim1BkIn2		= 0b0010 << 10,
+			Tim8BkIn		= 0b0011 << 10,
+			Tim8BkIn2		= 0b0100 << 10,
+			Tim1Or8BkIn2	= 0b0101 << 10,


Are… are these the table you need (from F3 refman)? zomg… 🙈

Yeah, these tables and/or the register descriptions...

@chris-durand is working on parsing the register description using pdfminer...

is working on parsing the register description using pdfminer...

Been there, done that. It didn't give me enough data last I tried, but maybe it's enough for this purpose.

rleh · 2018-07-15T23:20:28Z

Both examples (Nucleo-L432KC and STM32F3 DIscovery) have just been tested in hardware.

salkinium · 2018-07-15T23:25:50Z

src/modm/platform/comp/stm32/comp.hpp.in

+	{% if target.family == "f3" -%}
+	{% set polarity = "POL" %}
+	{% elif target.family == "l4" -%}
+	{% set polarity = "POLARITY" %}


This doesn't seem to be used anywhere.

salkinium · 2018-07-15T23:28:42Z

src/modm/platform/comp/stm32/comp.hpp.in

+		connect()
+		{
+			using Connector = GpioConnector<Peripheral::Comp{{ id }}, Signals...>;
+			Connector::connect();


FYI: Is the current connect behaviour correct? ie. no setAnalogInput()?

connect() { %% if signal.af | length setAlternateFunction({{ signal.af[0] }}); %% elif signal.driver.startswith("Adc") or signal.driver.startswith("Dac") disconnect(); setAnalogInput(); %% endif }

The reference manuals state:

The I/Os used as comparators inputs must be configured in analog mode in the GPIOs registers.
The comparator output can be connected to the I/Os using the alternate function channel given in “Alternate function mapping” table in the datasheet.

Sounds like we should do setAnalogInput() for the input pins.
But I can confirm the comparator inputs and outputs from the two examples are working correctly.

Hm, this doesn't generate anything for Comp Inputs right now, since they do not have any AF number in the modm-devices dataset. You should add or signal.driver.startswith("Comp") to the line to set analog mode for these too.

<signal driver="comp" instance="1" name="inp"/>

The Comp output has a AF, so it will set that instead.

<signal af="8" driver="comp" instance="2" name="out"/>

Is this to verbose?

if ... or (signal.driver.startswith("Comp") and signal.name.startswith("In")):

Checking signal.name is not necessary, since all comp signals without AF number are inputs. (I went through the same thought process for adc and dac peripherals 😋).

Verify yourself with this inside ext/modm-devices/devices/stm32/:

ack "<signal( af=\".*?\")? driver=\"comp\".*?>"

Oh, wait, that's not true

$ ack -h "<signal( af=\".*?\")? driver=\"comp\".*?>" | sort | uniq <signal af="12" driver="comp" instance="1" name="out"/> <signal af="12" driver="comp" instance="2" name="out"/> <signal af="13" driver="comp" instance="1" name="out"/> <signal af="13" driver="comp" instance="2" name="out"/> <signal af="2" driver="comp" instance="1" name="out"/> <signal af="3" driver="comp" instance="7" name="out"/> <signal af="6" driver="comp" instance="1" name="out"/> <signal af="7" driver="comp" instance="1" name="inm"/> <=== <signal af="7" driver="comp" instance="1" name="inp"/> <=== <signal af="7" driver="comp" instance="1" name="out"/> <signal af="7" driver="comp" instance="2" name="inm"/> <=== <signal af="7" driver="comp" instance="2" name="inp"/> <=== <signal af="7" driver="comp" instance="2" name="out"/> <signal af="7" driver="comp" instance="3" name="out"/> <signal af="7" driver="comp" instance="5" name="out"/> <signal af="7" driver="comp" instance="6" name="out"/> <signal af="8" driver="comp" instance="1" name="out"/> <signal af="8" driver="comp" instance="2" name="out"/> <signal af="8" driver="comp" instance="3" name="out"/> <signal af="8" driver="comp" instance="4" name="out"/> <signal af="8" driver="comp" instance="5" name="out"/> <signal af="8" driver="comp" instance="6" name="out"/> <signal driver="comp" instance="1" name="inm"/> <signal driver="comp" instance="1" name="inp"/> <signal driver="comp" instance="2" name="inm"/> <signal driver="comp" instance="2" name="inp"/> <signal driver="comp" instance="3" name="inm"/> <signal driver="comp" instance="3" name="inp"/> <signal driver="comp" instance="4" name="inm"/> <signal driver="comp" instance="4" name="inp"/> <signal driver="comp" instance="5" name="inm"/> <signal driver="comp" instance="5" name="inp"/> <signal driver="comp" instance="6" name="inm"/> <signal driver="comp" instance="6" name="inp"/> <signal driver="comp" instance="7" name="inm"/> <signal driver="comp" instance="7" name="inp"/>

Could be a bug in the DFG though… grrrr.

$ ack "<signal af=\".*?\" driver=\"comp\".*?name=\"in.*?>" stm32f0-58-8.xml 113: <signal af="7" driver="comp" instance="1" name="inm"/> 122: <signal af="7" driver="comp" instance="1" name="inp"/> 130: <signal af="7" driver="comp" instance="2" name="inm"/> 139: <signal af="7" driver="comp" instance="2" name="inp"/> 149: <signal af="7" driver="comp" instance="1" name="inm"/> 150: <signal af="7" driver="comp" instance="2" name="inm"/> 160: <signal af="7" driver="comp" instance="1" name="inm"/> 161: <signal af="7" driver="comp" instance="2" name="inm"/>

But still true enough for the F3 and L4 series, lol! 😎

salkinium

Thanks!

salkinium reviewed Jul 15, 2018

View reviewed changes

rleh force-pushed the feature/comp branch from 3d089fe to 7f92986 Compare July 15, 2018 18:05

rleh force-pushed the feature/comp branch 2 times, most recently from 1d0c8c6 to 5d3a198 Compare July 15, 2018 19:47

salkinium reviewed Jul 15, 2018

View reviewed changes

rleh force-pushed the feature/comp branch from 5d3a198 to 23e11ec Compare July 15, 2018 21:38

salkinium reviewed Jul 15, 2018

View reviewed changes

rleh force-pushed the feature/comp branch from 23e11ec to 1f79ea6 Compare July 15, 2018 22:46

salkinium previously approved these changes Jul 15, 2018

View reviewed changes

rleh dismissed salkinium’s stale review via a0bba5d July 16, 2018 00:17

rleh force-pushed the feature/comp branch 2 times, most recently from a0bba5d to 89dbbd5 Compare July 17, 2018 17:25

rleh added 3 commits July 17, 2018 20:08

[stm32] add comparator platform driver for f3 and l4

cff7e1b

[examples] Nucleo-L432KC comparator example

e75bbdd

[examples] STM32F3 discovery board comparator example

10fdc3f

rleh force-pushed the feature/comp branch from 89dbbd5 to 10fdc3f Compare July 17, 2018 18:08

salkinium approved these changes Jul 17, 2018

View reviewed changes

salkinium merged commit 10fdc3f into modm-io:develop Jul 17, 2018

rleh deleted the feature/comp branch August 25, 2018 22:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[stm32] Add comparator platform driver for f3 and l4 #41

[stm32] Add comparator platform driver for f3 and l4 #41

rleh commented Jul 15, 2018 •

edited

Loading

salkinium Jul 15, 2018

salkinium commented Jul 15, 2018

salkinium Jul 15, 2018

salkinium Jul 15, 2018

salkinium Jul 15, 2018

rleh Jul 15, 2018

salkinium Jul 15, 2018

salkinium Jul 15, 2018

rleh Jul 15, 2018 •

edited

Loading

salkinium Jul 15, 2018

salkinium Jul 15, 2018

rleh Jul 15, 2018

salkinium Jul 15, 2018

salkinium Jul 15, 2018

rleh Jul 16, 2018

salkinium Jul 15, 2018

rleh Jul 15, 2018

salkinium Jul 15, 2018

rleh Jul 15, 2018

salkinium Jul 15, 2018

rleh commented Jul 15, 2018

salkinium Jul 15, 2018

salkinium Jul 15, 2018

rleh Jul 15, 2018

salkinium Jul 16, 2018

rleh Jul 16, 2018

salkinium Jul 16, 2018

salkinium Jul 16, 2018

salkinium Jul 16, 2018

salkinium Jul 16, 2018

salkinium left a comment

[stm32] Add comparator platform driver for f3 and l4 #41

[stm32] Add comparator platform driver for f3 and l4 #41

Conversation

rleh commented Jul 15, 2018 • edited Loading

Todo

Choose a reason for hiding this comment

salkinium commented Jul 15, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rleh Jul 15, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rleh commented Jul 15, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

salkinium left a comment

Choose a reason for hiding this comment

rleh commented Jul 15, 2018 •

edited

Loading

rleh Jul 15, 2018 •

edited

Loading