Introduce new ADC API for DMA-based high sampling rate #175

ghost · 2017-12-12T14:15:13Z

Implement DMA API for ADC for F0, F1, F3, F4, L0 and L4 families. The DMA is used for maximize the ADC sampling rate. Measure the sampling rate for each ADC and verify that they match the datasheet. For example on L4 board measure a sampling rate of 5,33 Ms/s on the three ADCs. Also check that we can make two ADC conversions at the same by using the DMA with two differents ADCs.

LMESTM · 2017-12-12T15:25:39Z

cores/arduino/stm32/dma.c

+     "-'    "--                                                            
+
+----------------------------------------------------------------------------
+


Is ASCII art really needed ?

It's remove

LMESTM · 2017-12-12T15:26:31Z

cores/arduino/stm32/dma.c

+#endif
+
+extern ADC_HandleTypeDef AdcHandleTab[ADC_NUM];
+


dma.c file should not have any reference to ADC, so that it can later be re-used by oher devices

We will see if it possible in next release.

After study, it's not possible to remove this variable from DMA. The DMA link a peripheral, it need to have the information that is in the ADC structure. If we don't share this variable between ADC and DMA, DMA will not work.

The idea was to do the DMA allocation in dma.c and keep the dma actual configuration in each driver ADC and others later. Otherwise this file here will become huge and needs to be updated each time when you need to add DMA support for a new driver. If we make this file simpler, then up to each driver to add the calls to DMA based on the "allocated" DMA information. My point is not that the code is wrong, but rather the split between the files can be changed

If i understand well, dma_allocate() just give you the irqHandler and the channel to use, then all the initialization of the DMA will be done in analog.c file, in HAL_ADC_MspInit() function.

That would be the idea of the proposal indeed - please think about it and let let me know if you think it makes sense. Also I think that the DMA mapping table can be generic and could contain any kind of peripheral instances (ADC, serial, I2C, etc ... ) so the name and types should be generic
Depending on peripherals that are used, the DMA settings will differ (periph to mem, or mem to periph, etc.) so it is rather up to the peripheral driver to set these options.

Done, let me know if it's OK. I didn't rebase the git history yet if you want to go back.

LMESTM · 2017-12-12T15:33:21Z

cores/arduino/stm32/dma.c

+  __HAL_RCC_DMA2_CLK_ENABLE();
+#endif
+
+  channel = pinmap_find_dmaChannel(hadc->Instance, DmaMap_DMA);


Instead of DmaMap_DMA a generic table can be used, where any kind of devices Serial, ADC could be listed with corresponding ADC instance and channel. ...
Then the table could be parsed with a for loop instead of checks like (if (hadc->Instance == ADC3))

pinmap_find_dmaChannel is similar to other functions like pin_pinName or pinmap_find_function. Why should it be different? DmaMap_DMA contains all informations needed for DMA like PinMap_ADC contains all informations needed for ADC.

One important thing: PeripheralsPins.c contains arrays for pin mapping.
What a pin is able to do for a dedicated peripheral and how.
So adding this kind of array not linked to a pin is not correct.
+const DmaMap DmaMap_DMA[] = {

{ADC1, 1, DMA1_Channel1, DMA1_Channel1_IRQn , 1},

{ADC1, 1, DMA1_Channel2, DMA1_Ch2_3_DMA2_Ch1_2_IRQn, 2},

{ADC1, 2, DMA2_Channel5, DMA1_Ch4_7_DMA2_Ch3_5_IRQn, 5},

{NP, 0, NP, NP, 0}
+};

A DMA is link to a peripheral not a pin... i agree we that. Can we keep this table an put it in another file? Oterwise, what do you propose?

Currently, I do not reviewed this PR so I have no proposal. Just a note on PeripheralPins.c file usage.

I agree with proposal to keep the table that is needed but move it to dma.c as this is a table about dma channels and streams, not pins - is it ok ?

The table if different from one board to another. If we move it to dma.c, we will frame it with #ifdef. dma.c will become a huge file. Is it ok for you?

LMESTM · 2017-12-12T15:33:45Z

cores/arduino/stm32/dma.c

+
+  channel = pinmap_find_dmaChannel(hadc->Instance, DmaMap_DMA);
+  #ifdef ADC1_BASE
+  if (hadc->Instance == ADC1)


rather use a for loop over the DMA descritpion table for finding a match.

We need to know is the ADC exist before the "if" condition. Some device have only one ADC...
The same method is used in timer.c

LMESTM · 2017-12-12T15:35:10Z

cores/arduino/stm32/dma.c

+
+  /* Deinitialize  & Initialize the DMA for new transfer */
+  HAL_DMA_DeInit(&DmaHandleTab[indice]);
+  HAL_DMA_Init(&DmaHandleTab[indice]);


Once DMA instance and channel are found, the DMA init itself could be left in the ADC driver

Is it not better to do all the DMA things in DMA layer?

LMESTM · 2017-12-12T15:36:59Z

cores/arduino/stm32/dma.c

+  * @retval none
+  */
+void dma_init_adc(ADC_HandleTypeDef *hadc)
+{


As mentioned DMA PAI should not mention ADC at all.
instead of dma_init_adc I would rather consider a generic function like dma_allocate to look for the proper DMA instance and channel with a generic (void*) pointer to device. then the function would look for an instance match in the table with pinmap_find_dmaChannel.

dma_allocate create. A enum is create, we need it for identify the peripheral.

sorry I don't understand your answer.

My proposal is
replace : void dma_init_adc(ADC_HandleTypeDef *hadc)
by : void dma_init(void *any_handle_typedef)

so that it can later be used also by other devices than ADC

This is what we have done. But in dma_init() we also need an other parameter for know on what peripheral we work (ADC, UART). So the prototype is :
void dma_allocate(void *periph_handle, Peripheral periph);

LMESTM · 2017-12-12T15:38:51Z

cores/arduino/stm32/dma.c

+  * @retval none
+  */
+void dma_deinit_adc(ADC_HandleTypeDef *hadc)
+{


rathe have dma_free or dma_put to release a DMA handle.

dma_free() is create. The prototype is void dma_free(void *periph_handle, Peripheral perip).
Again, we need to know on which peripheral we work.

LMESTM · 2017-12-12T15:42:37Z

cores/arduino/stm32/dma.c

+  {
+    HAL_DMA_IRQHandler(AdcHandleTab[0].DMA_Handle); // ADC1
+  }
+  else


could be
elsif (AdcHandleTab[0].DMA_Handle != NULL)

I don't really understand what you mean. If and elseif can't have the same condition. I can put a condition on the elseif if that's what you want.

LMESTM · 2017-12-12T15:43:32Z

cores/arduino/stm32/dma.c

+  * @retval None
+  */
+
+#ifdef ADC1_BASE


Wouldn't it be better to keep generic name ?
DMA1_Channel1_IRQHandler

We can't keep generic name because the are different from one device to another. For example for F3 family this is DMA2_Channel1_IRQHandler and for F4 family this is DMA2_Stream1_IRQHandler. (Channel/Stream)

Ok I understand that - I just mean is it really needed that the name contains 'ADC'
it could be always DMA2_Stream1_IRQHandler anyway and redefined only for few families.

Ok done in next release.

LMESTM · 2017-12-12T15:49:23Z

cores/arduino/stm32/dma.c

+    defined(STM32F098xx)
+void DMA_ADC1_IRQHandler3(void)
+{
+  HAL_DMA_IRQHandler(AdcHandleTab[0].DMA_Handle);


Not checking handle is != NULL ?

Done. Null pointer is check.

LMESTM · 2017-12-12T15:53:09Z

cores/arduino/stm32/analog.c

  /*##-2- Configure peripheral GPIO ##########################################*/
+#ifdef ADC1_BASE


How is this part of change related to "prepare ADC DMA for F3 family" ?

It was a old commit, i rename it.

LMESTM

Ok for the new split asa it is started now. More changes are required I think. Please find more comments and proposals as well.

LMESTM · 2017-12-15T10:04:58Z

cores/arduino/stm32/PeripheralPins.h

@@ -71,5 +71,8 @@ extern const PinMap PinMap_Ethernet[];
 //*** QUADSPI ***
 extern const PinMap PinMap_QUADSPI[];

+//*** DMA ***
+extern const DmaMap DmaMap_DMA[];
+


as suggested by @fpistm the DmAMap should be declared and managed in dma.c, not in PeripheralPins as there is no pin in this map.

LMESTM · 2017-12-15T10:13:54Z

cores/arduino/stm32/dma.c

+      *channel = pinmap_find_dmaChannel(hadc->Instance, DmaMap_DMA);
+
+      /* Set clock as active */
+      channelNumber = pinmap_find_dmaChannelNumber(hadc->Instance, DmaMap_DMA);


this should not be pinmap_find_dmaChannelNumber but dma_find_dmaChannelNumber

LMESTM · 2017-12-15T10:14:00Z

cores/arduino/stm32/dma.c

+
+      /* Set clock as active */
+      channelNumber = pinmap_find_dmaChannelNumber(hadc->Instance, DmaMap_DMA);
+      dmaNumber     = pinmap_find_dma(hadc->Instance, DmaMap_DMA);


LMESTM · 2017-12-15T10:15:41Z

cores/arduino/stm32/dma.c

+void DMA2_Stream0_IRQHandler(void)
+{
+  #if defined(STM32F2xx) || defined(STM32F4xx) || defined(STM32F7xx)
+  if (AdcHandleTab[0].DMA_Handle != NULL)


from dma point of view this is a HandleTab, we don't need to to know it's a Adc one.

Where is the DMA_Handle table initiliazed with proper DMA_handle values ?

This is done by this function, when your link the DMA to a peripheral :
__HAL_LINKDMA(hadc, DMA_Handle, DmaHandleTab[indice]);

Ok same as previous - I have looked at __HAL_LINKDMA and this is now clearer. thanks

LMESTM · 2017-12-15T10:16:19Z

cores/arduino/stm32/pinmap.c

@@ -109,3 +109,39 @@ PinName pin_pinName(const PinMap* map) {
    return (PinName)NC;
  }
 }
+
+uint8_t pinmap_find_dma(void* peripheral, const DmaMap* dma) {


DMA functions that are not related to pins should be moved to dma.c file

LMESTM · 2017-12-15T10:51:48Z

cores/arduino/stm32/analog.c

 static PinName g_current_pin = NC;
+void (*cb[ADC_NUM])(void *user_data);
+static bool use_dma = false;


why isn't a "use_dma" for each ADC like use_dma[ADC_NUM] ?

Here we don't need a table. I explain myself. When you want to use ADC peripheral with or without DMA, the same function was called : HAL_ADC_MspInit(). In this function I do the DMA part only if the call come from the dma function (adc_read_value_dma). The boolean dma_use fulfilled this role.

so does it cover the case where ADC_1 is used with DMA and ADC_2 is used without DMA ?

LMESTM · 2017-12-15T10:52:59Z

cores/arduino/stm32/analog.c

-  */
+ADC_HandleTypeDef AdcHandleTab[ADC_NUM];
+static DMA_HandleTypeDef DmaHandleTab[ADC_NUM] = {NULL};
+static PinName g_current_pinTab[ADC_NUM];
 static PinName g_current_pin = NC;


looks like g_current_pin is not used anymore right ?

It's used for DAC function, I don't touch to this part. See for example function HAL_DAC_MspInit() or dac_write_value().

hmm ok - can we use the same DAC in DMA then without DMA ? Isn't it supposed to be the same pin .

Do you want to use g_current_pinTab for DAC too? I prefer to leave as it is because DAC and ADC are two different peripherals.

Ok - no please keep them separated if they're used for different purpose. But would be great to have at a least a comment to explain that one is used for ADC and the other one is used for DAC because as of now they have exactly the same name.

LMESTM · 2017-12-15T10:58:49Z

cores/arduino/stm32/analog.c

+  */
+void HAL_ADC_ConvCpltCallback(ADC_HandleTypeDef *AdcHandle)
+{
+  void *user_data = NULL;


what's the point of user_data if it is always NULL ?

LMESTM · 2017-12-15T10:59:42Z

cores/arduino/wiring_analog.c

@@ -61,6 +61,21 @@ uint32_t analogRead(uint32_t ulPin)
  return value;
 }

+//Start a DMA acquisition on ADC
+//the initialization of the analog PIN is done through this function
+void analogReadDma( uint32_t ulPin, uint32_t *pData, uint32_t lData, void (*callback)(void *user_data))


if we use a user_data parameter in the callback, then the user data paramrter must also be passed by application.

LMESTM · 2017-12-15T11:03:44Z

cores/arduino/stm32/analog.c

 static PinName g_current_pin = NC;
+void (*cb[ADC_NUM])(void *user_data);
+static bool use_dma = false;



Maybe it would be useful to have a global ADC structure starting which would contain the ADC handle, the DMA handle, cb and also use_dma I guess.

+ADC_HandleTypeDef AdcHandleTab[ADC_NUM];
+static DMA_HandleTypeDef DmaHandleTab[ADC_NUM] = {NULL};
+static PinName g_current_pinTab[ADC_NUM];
+void (*cb[ADC_NUM])(void *user_data);
+static bool use_dma = false;

If you define your structure in such way that ADC_HandleTypeDef is the first structure parameter, then you can also access the global structure based on the handle.

something like
typedef struct {
ADC_HandleTypeDef AdcHandle;
DMA_HandleTypeDef DmaHandle;
PinName current_pin;
void(*cb)(void user_data);
void user_data;
bool use_dma;
} adc_context;

static adc_context context(ADC_NUM];

then in MSp_Init/Deinit and also in HAL_ADC_ConvCpltCallback you can get all information like cb:

void HAL_ADC_ConvCpltCallback(ADC_HandleTypeDef AdcHandle) {
adc_context context = (adc_context *) AdcHandle;
if(context->cb != NULL)
context->cb(context->user_data);
}

Done. New release available.

LMESTM

Looks pretty good now. Only a few remaining comments or questions.

LMESTM · 2017-12-18T20:37:38Z

cores/arduino/stm32/dma.c

+  {ADC3, 2, DMA2_Stream1, DMA2_Stream1_IRQn, 1},
+  #endif
+  #endif
+};


isn't the table supposed to end with NP entry ?

LMESTM · 2017-12-18T20:46:14Z

cores/arduino/stm32/analog.c

+AdcContext context[ADC_NUM];
+
+/* DMA is use, or classic ADC  */
+static bool use_dma = false;


I still don"t understand why use_dma is not defined for each instance ... how does it work in case ADC1 is used in DMA mode wile ADC2 is used without DMA.

Ok right, i see where is the problem. My thing works only for initialization, but I have lost the use_dma information when I wat Deinit it. So yes it's better to make a use_dma for each instance.

LMESTM · 2017-12-18T20:53:45Z

cores/arduino/stm32/analog.h

@@ -47,6 +57,15 @@
 extern "C" {
 #endif

+typedef struct {
+  ADC_HandleTypeDef AdcHandle;


We're making sure that AdcHandle is the first struct inside AdcContext => add a comment to make sure this is always true.

LMESTM · 2017-12-18T20:56:34Z

cores/arduino/stm32/analog.c

+  */
+void HAL_ADC_ConvCpltCallback(ADC_HandleTypeDef *AdcHandle)
+{
+  /* Report to main program that ADC sequencer has reached its end */


Because we're making sure that ADC_HandleTypeDef is the first struct in the AdContext struct this is a trick to retrieve the AdcContext from the handle used in HAL.
you should be able to code as below:

AdcContext context = (AdcContext *) AdcHandle;
contex.cb(context.user_data);

ok. Done. New release available.

Signed-off-by: dhl <[email protected]>

LMESTM

Ok for this release.

LMESTM · 2017-12-20T10:51:28Z

Thanks @damienWi6labs
Can you provide also a few lines in the PR description of what was tested with this release ?

ghost · 2017-12-20T11:27:06Z

PR description update.

LMESTM · 2018-01-31T20:10:27Z

@fpistm - anything gating this PR ?

fpistm · 2018-01-31T20:36:43Z

My review of DMA API...

fpistm

Major remark is that ADC uses DMA service.
DMA service must be generic and must not be aware of any peripheral in its code. This is to the peripheral to provide the right config with the right handler.
For DMA, ADC instance or SPI one is the same.

Commits order have to be changed. DMA api must be introduced before using it.

Provide a Wiki page about this new API: analogReadDma in https://github.com/stm32duino/wiki/wiki/API
Provide a sketch example in STM32Examples repo or in the Wiki

fpistm · 2018-02-05T09:14:06Z

cores/arduino/stm32/analog.h

@@ -47,6 +57,16 @@
 extern "C" {
 #endif

+typedef struct {
+  ADC_HandleTypeDef AdcHandle;  /* CAUTION : This must be this first element of the struct */


Please, explain why in the comment

fpistm · 2018-02-05T10:07:41Z

cores/arduino/stm32/analog.c

 #else
 #error "ADC SAMPLINGTIME could not be defined"
 #endif

+// Use smaller clock prescaler to keep ADC conversion time as short as possible


Is it really true as this depends of the series and which source clock is used?
Ex:
F0 has no ADC_CLOCK_SYNC_PCLK_DIV1. So ADC_CLOCK_ASYNC_DIV1 will be used but this one is:
ADC asynchronous clock derived from ADC dedicated HSI while ADC_CLOCK_SYNC_PCLK_DIV2 is ADC synchronous clock derived from AHB clock divided by a prescaler of 2.

Moreover for L0: which is the best to use?
ADC_CLOCK_ASYNC_DIV1 or ADC_CLOCK_SYNC_PCLK_DIV1?
Note that ADC_CLOCK_SYNC_PCLK_DIV1 must be enabled only if PCLK has a 50% duty
clock cycle (APB prescaler configured inside the RCC must be bypassed and the system clock
must by 50% duty cycle)

fpistm · 2018-02-05T10:10:56Z

cores/arduino/stm32/analog.c

-  * @{
-  */
+/* Adc context                                                                */
+AdcContext context[ADC_NUM];


typo use upper case:
ADCContext
Table should be rename ADCContextMap to better identify it.
Then the structure have to be rename to know it is a structure typedef: ADCContext_t
ADCContext could be used for variable

fpistm · 2018-02-05T10:10:56Z

cores/arduino/stm32/analog.c

-/** @addtogroup STM32F4xx_System_Private_Variables
-  * @{
-  */
+/* Adc context                                                                */


typo use upper case for ADC:
ADC

fpistm · 2018-02-05T10:11:21Z

cores/arduino/stm32/analog.c

+/* Adc context                                                                */
+AdcContext context[ADC_NUM];
+
+/* GPIO pin use for DAC                                                       */


Only for DAC ?

fpistm · 2018-02-05T15:45:56Z

cores/arduino/stm32/dma.h

+typedef struct {
+    void* peripheral;
+    int8_t dmaNumber;
+    void* channel;


As the dmaNumber and channelNumber, it's possible to retrieve void* channel;
So dma_find_dmaChannel could be rewrote to return the correct one.
This will save some space.
Moreover, for struct memory alignement put channelNumber next to dmaNumber

fpistm · 2018-02-05T15:52:09Z

cores/arduino/stm32/dma.c

+  /* NVIC configuration for DMA Input data interrupt                          */
+  *irqHandler = dma_find_dmaIrqHandler(instance, DmaMap_DMA);
+
+#ifdef __HAL_RCC_DMA1_CLK_ENABLE


As the DMA instance used is know, enable only the right one.

fpistm · 2018-02-05T16:18:24Z

cores/arduino/stm32/analog.c

+    /*##-3- Ask DMA informations ###############################################*/
+    if (dma_allocate(hadc->Instance, &channel, &irqHandler))
+    {
+      /*##-4- Configure the DMA ##################################################*/


DMA configuration have to be done by the driver in dma.c
dma_init()

fpistm · 2018-02-05T16:21:26Z

cores/arduino/stm32/dma.c

+#include "dma.h"
+#include "analog.h"
+
+const DmaMap DmaMap_DMA[] = {


ADC DMA mapping have to be in analog part:
ADC1 , 1 ,1 --> ADC1 use DMA1 channel 1

In dma: only the dma handle should be stored

fpistm · 2018-02-05T16:28:28Z

cores/arduino/stm32/analog.c

+  if (context[indice].use_dma)
+  {
+    dma_free(hadc->Instance);
+    HAL_DMA_DeInit(hadc->DMA_Handle);


dma_free should be called after deinit ?

See my review

fpistm

Major remark is that ADC uses DMA service.
DMA service must be generic and must not be aware of any peripheral in its code. This is to the peripheral to provide the right config with the right handler.
For DMA, ADC instance or SPI one is the same.

Commits order have to be changed. DMA api must be introduced before using it.

Provide a Wiki page about this new API: analogReadDma in https://github.com/stm32duino/wiki/wiki/API
Provide a sketch example in STM32Examples repo or in the Wiki

fpistm · 2018-03-07T17:33:47Z

Hi @fprwi6labs
any news on this PR?

ghost · 2018-03-20T15:49:56Z

Hi @fpistm
This PR can be closed because the thread moved to PR #231.

fpistm · 2018-03-21T10:27:32Z

Hi @fprwi6labs,
Thanks.
Closed replaced by #231

ghost changed the title ~~Fix adc~~ Introduce new ADC API for DMA-based high sampling rate Dec 12, 2017

ghost requested review from LMESTM, fpistm and VVESTM December 12, 2017 15:21

ghost added the enhancement New feature or request label Dec 12, 2017

LMESTM reviewed Dec 12, 2017

View reviewed changes

LMESTM reviewed Dec 15, 2017

View reviewed changes

LMESTM reviewed Dec 18, 2017

View reviewed changes

damien16130 added 2 commits December 19, 2017 11:37

add all functions needed for using DMA in ADC layer

04499f4

Signed-off-by: dhl <[email protected]>

add dma file

5087b9f

Signed-off-by: dhl <[email protected]>

LMESTM previously approved these changes Dec 20, 2017

View reviewed changes

fpistm added the review on going label Jan 31, 2018

fpistm added this to the Next release milestone Feb 2, 2018

fpistm reviewed Feb 5, 2018

View reviewed changes

fpistm requested changes Feb 5, 2018

View reviewed changes

fpistm added the waiting feedback Further information is required label Mar 7, 2018

ghost mentioned this pull request Mar 20, 2018

Introduce new ADC API for DMA-based high sampling rate #231

Closed

fpistm closed this Mar 21, 2018

fpistm removed this from the Next release milestone Mar 21, 2018

fpistm added abandoned No more work on this and removed waiting feedback Further information is required labels Mar 21, 2018

fpistm removed the review on going label Mar 21, 2018

fpistm mentioned this pull request Jun 6, 2023

gh pr list do not show some PR cli/cli#7538

Closed

		"-' "--

		----------------------------------------------------------------------------

		/##-2- Configure peripheral GPIO ##########################################/
		#ifdef ADC1_BASE

Introduce new ADC API for DMA-based high sampling rate #175

Introduce new ADC API for DMA-based high sampling rate #175

Conversation

ghost commented Dec 12, 2017 • edited by ghost 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

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

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

LMESTM left a 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

LMESTM Dec 15, 2017 • 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LMESTM left a 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

LMESTM left a comment

Choose a reason for hiding this comment

LMESTM commented Dec 20, 2017

ghost commented Dec 20, 2017

LMESTM commented Jan 31, 2018

ghost commented Dec 12, 2017 •

edited by ghost

Loading

LMESTM Dec 15, 2017 •

edited

Loading