You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/11/26 14:44:53 UTC

[GitHub] [nuttx] TimJTi opened a new pull request, #7702: Fix SAMA5D2 ADC and TSD problems

TimJTi opened a new pull request, #7702:
URL: https://github.com/apache/nuttx/pull/7702

   ## Summary
   
   Ensures ADC and TSD work together. TSD now works on SAMA5D2 and should still be OK for other SAMA5Dx family members
   
   ## Impact
   None expected
   
   ## Testing
   Custom board with ATSAMA5D2-D1M + 5" resistive TSD. Kconfig checked out for SAMA5D3 and SAMA5D4 families but no hardware to check these
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] TimJTi commented on pull request #7702: Fix SAMA5D2 ADC and TSD problems

Posted by GitBox <gi...@apache.org>.
TimJTi commented on PR #7702:
URL: https://github.com/apache/nuttx/pull/7702#issuecomment-1331231285

   Think its broken. Will have to leave it until tomorrow. Use GitHub they said - it's really good they said.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] TimJTi commented on a diff in pull request #7702: Fix SAMA5D2 ADC and TSD problems

Posted by GitBox <gi...@apache.org>.
TimJTi commented on code in PR #7702:
URL: https://github.com/apache/nuttx/pull/7702#discussion_r1038296668


##########
arch/arm/src/sama5/sam_adc.c:
##########
@@ -865,7 +935,7 @@ static void sam_adc_endconversion(void *arg)
   int ret;
 
   DEBUGASSERT(priv != NULL);
-  ainfo("pending=%08x\n", priv->pending);
+  ainfo("pending=%08lx\n", priv->pending);

Review Comment:
   still has %08lx in master, don't know why it shows wrong for you



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] xiaoxiang781216 commented on pull request #7702: Fix SAMA5D2 ADC and TSD problems

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #7702:
URL: https://github.com/apache/nuttx/pull/7702#issuecomment-1328115856

   @TimJTi it's better to squash the style fix into the first patch


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] pkarashchenko commented on a diff in pull request #7702: Fix SAMA5D2 ADC and TSD problems

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #7702:
URL: https://github.com/apache/nuttx/pull/7702#discussion_r1038302910


##########
arch/arm/src/sama5/sam_adc.c:
##########
@@ -865,7 +935,7 @@ static void sam_adc_endconversion(void *arg)
   int ret;
 
   DEBUGASSERT(priv != NULL);
-  ainfo("pending=%08x\n", priv->pending);
+  ainfo("pending=%08lx\n", priv->pending);

Review Comment:
   I mean, that we need to change to PRIx32



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] TimJTi commented on pull request #7702: Fix SAMA5D2 ADC and TSD problems

Posted by GitBox <gi...@apache.org>.
TimJTi commented on PR #7702:
URL: https://github.com/apache/nuttx/pull/7702#issuecomment-1331988715

   It's not broken! Wrong defconfig (I have TOOOO many, coping with all the arch fixes, trying to keep them separate!).
   
   Fingers crossed....


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] xiaoxiang781216 merged pull request #7702: Fix SAMA5D2 ADC and TSD problems

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged PR #7702:
URL: https://github.com/apache/nuttx/pull/7702


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] TimJTi commented on pull request #7702: Fix SAMA5D2 ADC and TSD problems

Posted by GitBox <gi...@apache.org>.
TimJTi commented on PR #7702:
URL: https://github.com/apache/nuttx/pull/7702#issuecomment-1328088672

   Have made this changes. Will squash after any other changes requested.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] xiaoxiang781216 commented on pull request #7702: Fix SAMA5D2 ADC and TSD problems

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #7702:
URL: https://github.com/apache/nuttx/pull/7702#issuecomment-1328166358

   > > @TimJTi it's better to squash the style fix into the first patch
   > 
   > Ok! Thought it might be better to wait for any other comments.
   > 
   > Do I still need to do this?
   
   You don't need split the change into two patches, since it can be very easy to compare the delta from this button:
   <img width="559" alt="image" src="https://user-images.githubusercontent.com/18066964/204118690-d209b004-191a-4f7c-8416-ef429b4ea666.png">
   
   https://github.com/apache/nuttx/pull/7702/files/248072e02c04a7b0332e3f43c176204f1f2d9076..HEAD
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] TimJTi commented on a diff in pull request #7702: Fix SAMA5D2 ADC and TSD problems

Posted by GitBox <gi...@apache.org>.
TimJTi commented on code in PR #7702:
URL: https://github.com/apache/nuttx/pull/7702#discussion_r1033438592


##########
arch/arm/src/sama5/Kconfig:
##########
@@ -3246,11 +3247,12 @@ config SAMA5_ADC_ANARCH
 	default n
 	---help---
 		This option allows you to select different gain, offset, and single
-		vs. differential modes for each channel.
+		vs. differential modes for each channel, depending on ARCH.
 
 if SAMA5_ADC_ANARCH
 
 menu "Channel gain"
+    depends on ARCH_CHIP_SAMA5D

Review Comment:
   This has the same indentation as, say, line 816 and 925. What is wrong?



##########
arch/arm/src/sama5/Kconfig:
##########
@@ -3387,6 +3389,7 @@ config SAMA5_ADC_GAIN11
 endmenu # Channel gain
 
 menu "Channel offsets"
+    depends on ARCH_CHIP_SAMA5D3

Review Comment:
   Ditto



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] TimJTi commented on a diff in pull request #7702: Fix SAMA5D2 ADC and TSD problems

Posted by GitBox <gi...@apache.org>.
TimJTi commented on code in PR #7702:
URL: https://github.com/apache/nuttx/pull/7702#discussion_r1032815391


##########
arch/arm/src/sama5/sam_tsd.c:
##########
@@ -630,7 +680,7 @@ static void sam_tsd_bottomhalf(void *arg)
       pressr = sam_adc_getreg(priv->adc, SAM_ADC_PRESSR);
 #endif
       /* Discard any bad readings.  This check may not be necessary. */
-
+#if 1

Review Comment:
   I forgot, yes - I always search for the #if 0 I use when I'm suspicious of a bit of code being a problem, but will add a search for #if 1 in future!



##########
arch/arm/src/sama5/sam_tsd.c:
##########
@@ -1551,21 +1628,44 @@ static void sam_tsd_initialize(struct sam_tsd_s *priv)
 
   /* Enable pen contact detection */
 
+  regval  = sam_adc_getreg(priv->adc, SAM_ADC_TSMR);
   regval |= ADC_TSMR_PENDET;
   sam_adc_putreg(priv->adc, SAM_ADC_TSMR, regval);
 
   /* Set up pen debounce time */
 
   sam_tsd_debounce(priv, BOARD_TSD_DEBOUNCE);
 
+  /* configure pen sensitivity */

Review Comment:
   OK



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] pkarashchenko commented on a diff in pull request #7702: Fix SAMA5D2 ADC and TSD problems

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #7702:
URL: https://github.com/apache/nuttx/pull/7702#discussion_r1038369485


##########
arch/arm/src/sama5/sam_adc.c:
##########
@@ -865,7 +935,7 @@ static void sam_adc_endconversion(void *arg)
   int ret;
 
   DEBUGASSERT(priv != NULL);
-  ainfo("pending=%08x\n", priv->pending);
+  ainfo("pending=%08lx\n", priv->pending);

Review Comment:
   That is a standard way of printing stdint.h types. The print macro are located in inttypes.h



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] TimJTi commented on a diff in pull request #7702: Fix SAMA5D2 ADC and TSD problems

Posted by GitBox <gi...@apache.org>.
TimJTi commented on code in PR #7702:
URL: https://github.com/apache/nuttx/pull/7702#discussion_r1032816407


##########
arch/arm/src/sama5/sam_tsd.c:
##########
@@ -630,7 +680,7 @@ static void sam_tsd_bottomhalf(void *arg)
       pressr = sam_adc_getreg(priv->adc, SAM_ADC_PRESSR);
 #endif
       /* Discard any bad readings.  This check may not be necessary. */
-
+#if 1

Review Comment:
   done



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] TimJTi commented on a diff in pull request #7702: Fix SAMA5D2 ADC and TSD problems

Posted by GitBox <gi...@apache.org>.
TimJTi commented on code in PR #7702:
URL: https://github.com/apache/nuttx/pull/7702#discussion_r1038458296


##########
arch/arm/src/sama5/sam_adc.c:
##########
@@ -865,7 +935,7 @@ static void sam_adc_endconversion(void *arg)
   int ret;
 
   DEBUGASSERT(priv != NULL);
-  ainfo("pending=%08x\n", priv->pending);
+  ainfo("pending=%08lx\n", priv->pending);

Review Comment:
   Or, at least, there's no "need" to use C99 features in arch files even if we can?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] TimJTi commented on pull request #7702: Fix SAMA5D2 ADC and TSD problems

Posted by GitBox <gi...@apache.org>.
TimJTi commented on PR #7702:
URL: https://github.com/apache/nuttx/pull/7702#issuecomment-1334495014

   Since this is closed I can't just commit the suggestions here, so I'll do a new PR in the next few days to fix these style issues. Although I suppose I shouldn't rely on checkpatch.sh to fix my lack of meticulousness, but shouldn't it find these sorts of things?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] TimJTi commented on a diff in pull request #7702: Fix SAMA5D2 ADC and TSD problems

Posted by GitBox <gi...@apache.org>.
TimJTi commented on code in PR #7702:
URL: https://github.com/apache/nuttx/pull/7702#discussion_r1038285593


##########
arch/arm/src/sama5/sam_adc.c:
##########
@@ -1335,7 +1417,7 @@ static int sam_adc_settimer(struct sam_adc_s *priv, uint32_t frequency,
   priv->tc = sam_tc_allocate(channel, mode);
   if (!priv->tc)
     {
-      aerr("ERROR: Failed to allocate channel %d mode %08" PRIx32 "\n",
+      aerr("ERROR: Failed to allocate channel %d mode %08x\n",
             channel, mode);

Review Comment:
   Don't know...I guess it threw an error at some point and I changed it, but it seems OK now. I was having quite a bit of grief with version control....



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] TimJTi commented on a diff in pull request #7702: Fix SAMA5D2 ADC and TSD problems

Posted by GitBox <gi...@apache.org>.
TimJTi commented on code in PR #7702:
URL: https://github.com/apache/nuttx/pull/7702#discussion_r1038765865


##########
arch/arm/src/sama5/sam_adc.c:
##########
@@ -865,7 +935,7 @@ static void sam_adc_endconversion(void *arg)
   int ret;
 
   DEBUGASSERT(priv != NULL);
-  ainfo("pending=%08x\n", priv->pending);
+  ainfo("pending=%08lx\n", priv->pending);

Review Comment:
   That discussion is slightly beyond me, but I'm trying to understand if using %08lx is actually WRONG rather than just a difference in approach. We *can* use C99 in arch but don't HAVE to? I have always used the %08x type formatters, probably because I learnt C back in the 1980's (K&R!).
   
   I am not trying to be difficult, but need to make sure that any PR I submit 100% meets the rules, ideally first time! If it's "just" a suggestion that it is "better" to do something using a different method I am always happy to listen and learn, but need to know if that's what it is.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] TimJTi commented on a diff in pull request #7702: Fix SAMA5D2 ADC and TSD problems

Posted by GitBox <gi...@apache.org>.
TimJTi commented on code in PR #7702:
URL: https://github.com/apache/nuttx/pull/7702#discussion_r1033442040


##########
arch/arm/src/sama5/Kconfig:
##########
@@ -3712,6 +3763,17 @@ config SAMA5_ADC_TIOA_BOTH
 
 endchoice # ADTRG edge
 endif # SAMA5_ADC_TIOATRIG
+
+if SAMA5_ADC_PERIODIC_TRIG
+
+config SAMA5_ADC_TRIGGER_PERIOD
+	int "ADC Periodic Trigger Rate, useconds"
+	default 50000
+	---help---
+    This setting determines the periodic sample trigger rate in useconds.

Review Comment:
   yep, this was wrong.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] xiaoxiang781216 commented on pull request #7702: Fix SAMA5D2 ADC and TSD problems

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #7702:
URL: https://github.com/apache/nuttx/pull/7702#issuecomment-1331144126

   @TimJTi please squash your patch again.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] TimJTi commented on a diff in pull request #7702: Fix SAMA5D2 ADC and TSD problems

Posted by GitBox <gi...@apache.org>.
TimJTi commented on code in PR #7702:
URL: https://github.com/apache/nuttx/pull/7702#discussion_r1038292534


##########
arch/arm/src/sama5/sam_adc.c:
##########
@@ -1069,10 +1137,13 @@ static void sam_adc_reset(struct adc_dev_s *dev)
 
   /* Reset gain, offset, differential modes */
 
+#if defined (ATSAMA5D3)
   sam_adc_putreg(priv, SAM_ADC_CGR, 0);
+  #endif
+
   sam_adc_putreg(priv, SAM_ADC_COR, 0);
 
-#ifndef CONFIG_SAMA5_ADC_SWTRIG
+#if !defined CONFIG_SAMA5_ADC_SWTRIG && !defined CONFIG_SAMA5_TSD

Review Comment:
   done



##########
arch/arm/src/sama5/sam_adc.c:
##########
@@ -1069,10 +1137,13 @@ static void sam_adc_reset(struct adc_dev_s *dev)
 
   /* Reset gain, offset, differential modes */
 
+#if defined (ATSAMA5D3)

Review Comment:
   done



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] xiaoxiang781216 commented on a diff in pull request #7702: Fix SAMA5D2 ADC and TSD problems

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #7702:
URL: https://github.com/apache/nuttx/pull/7702#discussion_r1038769231


##########
arch/arm/src/sama5/sam_adc.c:
##########
@@ -865,7 +935,7 @@ static void sam_adc_endconversion(void *arg)
   int ret;
 
   DEBUGASSERT(priv != NULL);
-  ainfo("pending=%08x\n", priv->pending);
+  ainfo("pending=%08lx\n", priv->pending);

Review Comment:
   > That discussion is slightly beyond me, but I'm trying to understand if using %08lx is actually WRONG rather than just a difference in approach.
   
   no prefix mean int, l prefix is for long int. But int32_t may map to int or long int which depends on arch and toolchain(#7476), so it's wrong to either use l prefix or no prefix for int32_t. To make programmer life easier, the standard introduce PRI[d|u|x]32 as the int32_t format specifier.
   
   > We _can_ use C99 in arch but don't HAVE to? I have always used the %08x type formatters, probably because I learnt C back in the 1980's (K&R!).
   > 
   
   So the rule is simple:
   int: no prefix
   long: l prefix
   long long: ll prefix
   int32_t: PRI[o||d|u|x]32
   Here has some discussion:
   https://stackoverflow.com/questions/3168275/printf-format-specifiers-for-uint32-t-and-size-t?rq=1
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] TimJTi commented on a diff in pull request #7702: Fix SAMA5D2 ADC and TSD problems

Posted by GitBox <gi...@apache.org>.
TimJTi commented on code in PR #7702:
URL: https://github.com/apache/nuttx/pull/7702#discussion_r1038320009


##########
arch/arm/src/sama5/sam_adc.c:
##########
@@ -1728,7 +1890,8 @@ static void sam_adc_gain(struct sam_adc_s *priv)
   /* Set GAIN0 only.  GAIN0 will be used for all channels. */
 
   sam_adc_putreg(priv, SAM_ADC_CGR, ADC_CGR_GAIN0(CONFIG_SAMA5_ADC_GAIN));
-#endif
+#endif /* CONFIG_SAMA5_ADC_ANARCH */

Review Comment:
   Ah, OK - yes, I see the associated #ifdef is (correctly) indented. Makes sense. Sorry!
   
   Done.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] pkarashchenko commented on a diff in pull request #7702: Fix SAMA5D2 ADC and TSD problems

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #7702:
URL: https://github.com/apache/nuttx/pull/7702#discussion_r1038523774


##########
arch/arm/src/sama5/sam_adc.c:
##########
@@ -865,7 +935,7 @@ static void sam_adc_endconversion(void *arg)
   int ret;
 
   DEBUGASSERT(priv != NULL);
-  ainfo("pending=%08x\n", priv->pending);
+  ainfo("pending=%08lx\n", priv->pending);

Review Comment:
   Here is some reference discussion, so you can better understand what I'm taking about https://github.com/apache/nuttx/pull/7476#issuecomment-1296318701
   
   Just never mind, I will fix it some time later when will get some free time.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] TimJTi commented on pull request #7702: Fix SAMA5D2 ADC and TSD problems

Posted by GitBox <gi...@apache.org>.
TimJTi commented on PR #7702:
URL: https://github.com/apache/nuttx/pull/7702#issuecomment-1329028386

   Github desktop does NOT help. I have done a crash course (i.e. read the help page!) and used command line interactive and hope I haven't made things worse...


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] TimJTi commented on pull request #7702: Fix SAMA5D2 ADC and TSD problems

Posted by GitBox <gi...@apache.org>.
TimJTi commented on PR #7702:
URL: https://github.com/apache/nuttx/pull/7702#issuecomment-1328971864

   And github has broken it...well its broken me, for sure. Can't squash changes so its a right mess.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] TimJTi commented on pull request #7702: Fix SAMA5D2 ADC and TSD problems

Posted by GitBox <gi...@apache.org>.
TimJTi commented on PR #7702:
URL: https://github.com/apache/nuttx/pull/7702#issuecomment-1328120305

   > @TimJTi it's better to squash the style fix into the first patch
   
   Ok! Thought it might be better to wait for any other comments.
   
   Do I still need to do this?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] TimJTi commented on pull request #7702: Fix SAMA5D2 ADC and TSD problems

Posted by GitBox <gi...@apache.org>.
TimJTi commented on PR #7702:
URL: https://github.com/apache/nuttx/pull/7702#issuecomment-1328359988

   I use tabs. Searched for double spaces. Don’t know what to do.On 27 Nov 2022, at 22:20, Petro Karashchenko ***@***.***> wrote:
   @pkarashchenko commented on this pull request.
   
   
   
   In arch/arm/src/sama5/Kconfig:
   > @@ -3230,6 +3230,7 @@ config SAMA5_ADC_DMASAMPLES
    
    config SAMA5_ADC_AUTOCALIB
    	bool "ADC auto-calibration"
   +    depends on ARCH_CHIP_SAMA5D3
   
   TABs
   
   
   
   In arch/arm/src/sama5/Kconfig:
   >  
    if SAMA5_ADC_ANARCH
    
    menu "Channel gain"
   +    depends on ARCH_CHIP_SAMA5D
   
   TABs
   
   
   
   In arch/arm/src/sama5/Kconfig:
   > @@ -3387,6 +3389,7 @@ config SAMA5_ADC_GAIN11
    endmenu # Channel gain
    
    menu "Channel offsets"
   +    depends on ARCH_CHIP_SAMA5D3
   
   TABs
   
   
   
   In arch/arm/src/sama5/Kconfig:
   > @@ -3487,6 +3490,7 @@ config SAMA5_ADC_OFFSET11
    endmenu # Channel offsets
    
    menu "Channel differential mode"
   +    depends on ARCH_CHIP_SAMA5D2 || ARCH_CHIP_SAMA5D3
   
   TABs
   
   
   
   In arch/arm/src/sama5/Kconfig:
   >  
    config SAMA5_ADC_OFFSET
    	bool "Offset"
    	default n
   +    depends on ARCH_CHIP_SAMA5D3
   
   TABs
   
   
   
   In arch/arm/src/sama5/Kconfig:
   > +  bool "PWM Event trigger"
   +  depends on SAMA5_PWM
   +  ---help---
   +    A-to-D Conversion is initiated from the PWM event lines
   +
   +config SAMA5_ADC_RTCOUT
   +  bool "RTC Out trigger"
   +  depends on SAMA5_RTC
   +  depends on ARCH_CHIP_SAMA5D2
   +  ---help---
   +    A-to-D Conversion is initiated from the RTC output
   +
   
   TABs
   
   
   
   In arch/arm/src/sama5/Kconfig:
   > +if SAMA5_ADC_PWMTRIG
   +
   +choice
   +  prompt "PWM Event Line Selection"
   +  default SAMA5_ADC_PWMTRIG_LINE0
   +
   +config SAMA5_ADC_PWM_TRIG_LINE0
   +  bool "PWM event Line 0"
   +  ---help---
   +    Trigger A-to-D conversion on PWM event line 0
   +
   +config SAMA5_ADC_PWM_TRIG_LINE1
   +  bool "PWM event Line 1"
   +  ---help---
   +    Trigger A-to-D conversion on PWM event line 1
   +
   +endchoice # PWM Event Line Selection
   +endif # SAMA5_ADC_PWMTRIG
   +
   +if SAMA5_ADC_ADTRG || SAMA5_ADC_TIOATRIG || SAMA5_ADC_PWMTRIG || SAMA5_ADC_RTCOUT
   +
   
   TABs
   
   
   
   In arch/arm/src/sama5/Kconfig:
   > @@ -3712,6 +3763,17 @@ config SAMA5_ADC_TIOA_BOTH
    
    endchoice # ADTRG edge
    endif # SAMA5_ADC_TIOATRIG
   +
   +if SAMA5_ADC_PERIODIC_TRIG
   +
   +config SAMA5_ADC_TRIGGER_PERIOD
   +	int "ADC Periodic Trigger Rate, useconds"
   +	default 50000
   +	---help---
   +    This setting determines the periodic sample trigger rate in useconds.
   
   TABs
   
   —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***>


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] TimJTi commented on a diff in pull request #7702: Fix SAMA5D2 ADC and TSD problems

Posted by GitBox <gi...@apache.org>.
TimJTi commented on code in PR #7702:
URL: https://github.com/apache/nuttx/pull/7702#discussion_r1032815416


##########
arch/arm/src/sama5/sam_tsd.c:
##########
@@ -1551,21 +1628,44 @@ static void sam_tsd_initialize(struct sam_tsd_s *priv)
 
   /* Enable pen contact detection */
 
+  regval  = sam_adc_getreg(priv->adc, SAM_ADC_TSMR);
   regval |= ADC_TSMR_PENDET;
   sam_adc_putreg(priv->adc, SAM_ADC_TSMR, regval);
 
   /* Set up pen debounce time */
 
   sam_tsd_debounce(priv, BOARD_TSD_DEBOUNCE);
 
+  /* configure pen sensitivity */
+
+  regval = sam_adc_getreg(priv->adc, SAM_ADC_ACR);
+  regval &= ~ADC_ACR_PENDETSENS_MASK;
+  regval |= ADC_ACR_PENDETSENS(BOARD_TSD_PENDETSENS);
+#if defined(ATSAMA5D2)
+  regval &= ~ADC_ACR_IBCTL_MASK;
+  regval |= ADC_ACR_IBCTL(BOARD_TSD_IBCTL);
+#endif
+  sam_adc_putreg(priv->adc, SAM_ADC_ACR, regval);
+
+#ifdef SAMA5_TSD_PENDET_TRIG_ALLOWED
   /* Configure pen interrupt generation */
 
   regval  = sam_adc_getreg(priv->adc, SAM_ADC_TRGR);
   regval &= ~ADC_TRGR_TRGMOD_MASK;
   regval |= ADC_TRGR_TRGMOD_PEN;
   sam_adc_putreg(priv->adc, SAM_ADC_TRGR, regval);
+#endif
 
   sam_adc_putreg(priv->adc, SAM_ADC_IER, ADC_INT_PEN);
+
+#ifdef CONFIG_SAMA5_TSD_AUTOCALIB
+  /* perform a ts calibration */

Review Comment:
   OK



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] TimJTi commented on pull request #7702: Fix SAMA5D2 ADC and TSD problems

Posted by GitBox <gi...@apache.org>.
TimJTi commented on PR #7702:
URL: https://github.com/apache/nuttx/pull/7702#issuecomment-1331174736

   Aarggghhhh...I did, but must have not pushed it. SORRY AGAIN!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] TimJTi commented on a diff in pull request #7702: Fix SAMA5D2 ADC and TSD problems

Posted by GitBox <gi...@apache.org>.
TimJTi commented on code in PR #7702:
URL: https://github.com/apache/nuttx/pull/7702#discussion_r1038782007


##########
arch/arm/src/sama5/sam_adc.c:
##########
@@ -865,7 +935,7 @@ static void sam_adc_endconversion(void *arg)
   int ret;
 
   DEBUGASSERT(priv != NULL);
-  ainfo("pending=%08x\n", priv->pending);
+  ainfo("pending=%08lx\n", priv->pending);

Review Comment:
   Thanks! Because the SAMA5s are 32 bit devices (and I have only ever written serious code for 32 bit devices) the %08lx is technically correct I think? But I understand that anything that makes code clearer to any coder is a good thing. I will adopt this and VERY much appreciate the clear explanation :)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] TimJTi commented on a diff in pull request #7702: Fix SAMA5D2 ADC and TSD problems

Posted by GitBox <gi...@apache.org>.
TimJTi commented on code in PR #7702:
URL: https://github.com/apache/nuttx/pull/7702#discussion_r1038318332


##########
arch/arm/src/sama5/sam_adc.c:
##########
@@ -865,7 +935,7 @@ static void sam_adc_endconversion(void *arg)
   int ret;
 
   DEBUGASSERT(priv != NULL);
-  ainfo("pending=%08x\n", priv->pending);
+  ainfo("pending=%08lx\n", priv->pending);

Review Comment:
   I've never used that macro. Is it preferred for NuttX? There are plenty of instances of %08x and %08lx littered through the codebase.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] pkarashchenko commented on a diff in pull request #7702: Fix SAMA5D2 ADC and TSD problems

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #7702:
URL: https://github.com/apache/nuttx/pull/7702#discussion_r1037576176


##########
arch/arm/src/sama5/sam_adc.c:
##########
@@ -865,7 +935,7 @@ static void sam_adc_endconversion(void *arg)
   int ret;
 
   DEBUGASSERT(priv != NULL);
-  ainfo("pending=%08x\n", priv->pending);
+  ainfo("pending=%08lx\n", priv->pending);

Review Comment:
   PRIx32



##########
arch/arm/src/sama5/sam_adc.c:
##########
@@ -2050,9 +2213,17 @@ struct adc_dev_s *sam_adc_initialize(void)
       /* Initialize the public ADC device data structure */
 
 #ifdef SAMA5_ADC_HAVE_CHANNELS
+      g_adcdev.ad_ops  = &g_adcops;
       priv->dev = &g_adcdev;
 #endif
 
+      g_adcdev.ad_priv = priv;
+
+      /* Initialize the private ADC device data structure */
+
+      nxmutex_init(&priv->lock);
+      priv->cb  = NULL;

Review Comment:
   ```suggestion
         priv->cb = NULL;
   ```
   



##########
arch/arm/src/sama5/sam_tsd.c:
##########
@@ -1318,7 +1382,11 @@ static void sam_tsd_tracking(struct sam_tsd_s *priv, uint32_t time)
           tracktim--;
         }
     }
-
+#elif defined (ATSAMA5D3)

Review Comment:
   ```suggestion
   #elif defined(ATSAMA5D3)
   ```
   



##########
arch/arm/src/sama5/sam_adc.c:
##########
@@ -2050,9 +2213,17 @@ struct adc_dev_s *sam_adc_initialize(void)
       /* Initialize the public ADC device data structure */
 
 #ifdef SAMA5_ADC_HAVE_CHANNELS
+      g_adcdev.ad_ops  = &g_adcops;

Review Comment:
   ```suggestion
         g_adcdev.ad_ops = &g_adcops;
   ```
   



##########
arch/arm/src/sama5/sam_tsd.c:
##########
@@ -137,6 +137,18 @@
 #  define MAX(a,b) (((a) > (b)) ? (a) : (b))
 #endif
 
+#ifndef BOARD_TSSCTIM
+# define BOARD_TSSCTIM 0

Review Comment:
   ```suggestion
   #  define BOARD_TSSCTIM 0
   ```
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] pkarashchenko commented on a diff in pull request #7702: Fix SAMA5D2 ADC and TSD problems

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #7702:
URL: https://github.com/apache/nuttx/pull/7702#discussion_r1037678110


##########
arch/arm/src/sama5/sam_adc.c:
##########
@@ -1069,10 +1137,13 @@ static void sam_adc_reset(struct adc_dev_s *dev)
 
   /* Reset gain, offset, differential modes */
 
+#if defined (ATSAMA5D3)

Review Comment:
   ```suggestion
   #if defined(ATSAMA5D3)
   ```



##########
arch/arm/src/sama5/sam_adc.c:
##########
@@ -1335,7 +1417,7 @@ static int sam_adc_settimer(struct sam_adc_s *priv, uint32_t frequency,
   priv->tc = sam_tc_allocate(channel, mode);
   if (!priv->tc)
     {
-      aerr("ERROR: Failed to allocate channel %d mode %08" PRIx32 "\n",
+      aerr("ERROR: Failed to allocate channel %d mode %08x\n",
             channel, mode);

Review Comment:
   Why this is changed? the original version seems to be correct?



##########
arch/arm/src/sama5/sam_adc.c:
##########
@@ -1069,10 +1137,13 @@ static void sam_adc_reset(struct adc_dev_s *dev)
 
   /* Reset gain, offset, differential modes */
 
+#if defined (ATSAMA5D3)
   sam_adc_putreg(priv, SAM_ADC_CGR, 0);
+  #endif
+
   sam_adc_putreg(priv, SAM_ADC_COR, 0);
 
-#ifndef CONFIG_SAMA5_ADC_SWTRIG
+#if !defined CONFIG_SAMA5_ADC_SWTRIG && !defined CONFIG_SAMA5_TSD

Review Comment:
   ```suggestion
   #if !defined(CONFIG_SAMA5_ADC_SWTRIG) && !defined(CONFIG_SAMA5_TSD)
   ```



##########
arch/arm/src/sama5/sam_tsd.c:
##########
@@ -1655,7 +1755,7 @@ int sam_tsd_register(struct sam_adc_s *adc, int minor)
 
   /* Initialize the touchscreen device driver instance */
 
-  priv->adc = adc; /* Save the ADC device handle */
+  priv->adc     = adc;               /* Save the ADC device handle    */

Review Comment:
   why this is changed?



##########
arch/arm/src/sama5/sam_adc.c:
##########
@@ -1728,7 +1890,8 @@ static void sam_adc_gain(struct sam_adc_s *priv)
   /* Set GAIN0 only.  GAIN0 will be used for all channels. */
 
   sam_adc_putreg(priv, SAM_ADC_CGR, ADC_CGR_GAIN0(CONFIG_SAMA5_ADC_GAIN));
-#endif
+#endif /* CONFIG_SAMA5_ADC_ANARCH */

Review Comment:
   ```suggestion
   #  endif /* CONFIG_SAMA5_ADC_ANARCH */
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] pkarashchenko commented on a diff in pull request #7702: Fix SAMA5D2 ADC and TSD problems

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #7702:
URL: https://github.com/apache/nuttx/pull/7702#discussion_r1038503990


##########
arch/arm/src/sama5/sam_adc.c:
##########
@@ -865,7 +935,7 @@ static void sam_adc_endconversion(void *arg)
   int ret;
 
   DEBUGASSERT(priv != NULL);
-  ainfo("pending=%08x\n", priv->pending);
+  ainfo("pending=%08lx\n", priv->pending);

Review Comment:
   NuttX has some C99 extensions generated. Like stdbool.h or stdint.h
   Since those are not compiler dependant and can be used for all archs



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] TimJTi commented on a diff in pull request #7702: Fix SAMA5D2 ADC and TSD problems

Posted by GitBox <gi...@apache.org>.
TimJTi commented on code in PR #7702:
URL: https://github.com/apache/nuttx/pull/7702#discussion_r1038291947


##########
arch/arm/src/sama5/sam_adc.c:
##########
@@ -1335,7 +1417,7 @@ static int sam_adc_settimer(struct sam_adc_s *priv, uint32_t frequency,
   priv->tc = sam_tc_allocate(channel, mode);
   if (!priv->tc)
     {
-      aerr("ERROR: Failed to allocate channel %d mode %08" PRIx32 "\n",
+      aerr("ERROR: Failed to allocate channel %d mode %08x\n",
             channel, mode);

Review Comment:
   Have changed it back



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] TimJTi commented on a diff in pull request #7702: Fix SAMA5D2 ADC and TSD problems

Posted by GitBox <gi...@apache.org>.
TimJTi commented on code in PR #7702:
URL: https://github.com/apache/nuttx/pull/7702#discussion_r1038295545


##########
arch/arm/src/sama5/sam_adc.c:
##########
@@ -2050,9 +2213,17 @@ struct adc_dev_s *sam_adc_initialize(void)
       /* Initialize the public ADC device data structure */
 
 #ifdef SAMA5_ADC_HAVE_CHANNELS
+      g_adcdev.ad_ops  = &g_adcops;
       priv->dev = &g_adcdev;
 #endif
 
+      g_adcdev.ad_priv = priv;
+
+      /* Initialize the private ADC device data structure */
+
+      nxmutex_init(&priv->lock);
+      priv->cb  = NULL;

Review Comment:
   done



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] TimJTi commented on a diff in pull request #7702: Fix SAMA5D2 ADC and TSD problems

Posted by GitBox <gi...@apache.org>.
TimJTi commented on code in PR #7702:
URL: https://github.com/apache/nuttx/pull/7702#discussion_r1038295217


##########
arch/arm/src/sama5/sam_adc.c:
##########
@@ -2050,9 +2213,17 @@ struct adc_dev_s *sam_adc_initialize(void)
       /* Initialize the public ADC device data structure */
 
 #ifdef SAMA5_ADC_HAVE_CHANNELS
+      g_adcdev.ad_ops  = &g_adcops;

Review Comment:
   done



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] TimJTi commented on a diff in pull request #7702: Fix SAMA5D2 ADC and TSD problems

Posted by GitBox <gi...@apache.org>.
TimJTi commented on code in PR #7702:
URL: https://github.com/apache/nuttx/pull/7702#discussion_r1038293171


##########
arch/arm/src/sama5/sam_tsd.c:
##########
@@ -1318,7 +1382,11 @@ static void sam_tsd_tracking(struct sam_tsd_s *priv, uint32_t time)
           tracktim--;
         }
     }
-
+#elif defined (ATSAMA5D3)

Review Comment:
   done



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] TimJTi commented on a diff in pull request #7702: Fix SAMA5D2 ADC and TSD problems

Posted by GitBox <gi...@apache.org>.
TimJTi commented on code in PR #7702:
URL: https://github.com/apache/nuttx/pull/7702#discussion_r1038454957


##########
arch/arm/src/sama5/sam_adc.c:
##########
@@ -865,7 +935,7 @@ static void sam_adc_endconversion(void *arg)
   int ret;
 
   DEBUGASSERT(priv != NULL);
-  ainfo("pending=%08x\n", priv->pending);
+  ainfo("pending=%08lx\n", priv->pending);

Review Comment:
   Been looking it up and I think inttypes.h is C99? Aren't we supposed to use C89?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] TimJTi commented on a diff in pull request #7702: Fix SAMA5D2 ADC and TSD problems

Posted by GitBox <gi...@apache.org>.
TimJTi commented on code in PR #7702:
URL: https://github.com/apache/nuttx/pull/7702#discussion_r1032815460


##########
arch/arm/src/sama5/sam_tsd.h:
##########
@@ -36,10 +36,15 @@
 
 /* Configuration ************************************************************/
 
-#ifdef CONFIG_SAMA_TSD_RXP
+#ifndef CONFIG_SAMA_TSD_RXP
 #  define CONFIG_SAMA_TSD_RXP 6
 #endif
 
+/* Only allow Pendet triggering in limited circumstances */

Review Comment:
   Surprised that checkpatch.sh missed that, but OK, of course



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] TimJTi commented on a diff in pull request #7702: Fix SAMA5D2 ADC and TSD problems

Posted by GitBox <gi...@apache.org>.
TimJTi commented on code in PR #7702:
URL: https://github.com/apache/nuttx/pull/7702#discussion_r1038289670


##########
arch/arm/src/sama5/sam_adc.c:
##########
@@ -1728,7 +1890,8 @@ static void sam_adc_gain(struct sam_adc_s *priv)
   /* Set GAIN0 only.  GAIN0 will be used for all channels. */
 
   sam_adc_putreg(priv, SAM_ADC_CGR, ADC_CGR_GAIN0(CONFIG_SAMA5_ADC_GAIN));
-#endif
+#endif /* CONFIG_SAMA5_ADC_ANARCH */

Review Comment:
   Can't find that set out in the Coding Style guide? Have I missed it?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] pkarashchenko commented on a diff in pull request #7702: Fix SAMA5D2 ADC and TSD problems

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #7702:
URL: https://github.com/apache/nuttx/pull/7702#discussion_r1033019078


##########
arch/arm/src/sama5/Kconfig:
##########
@@ -3230,6 +3230,7 @@ config SAMA5_ADC_DMASAMPLES
 
 config SAMA5_ADC_AUTOCALIB
 	bool "ADC auto-calibration"
+    depends on ARCH_CHIP_SAMA5D3

Review Comment:
   TABs



##########
arch/arm/src/sama5/Kconfig:
##########
@@ -3487,6 +3490,7 @@ config SAMA5_ADC_OFFSET11
 endmenu # Channel offsets
 
 menu "Channel differential mode"
+    depends on ARCH_CHIP_SAMA5D2 || ARCH_CHIP_SAMA5D3

Review Comment:
   TABs



##########
arch/arm/src/sama5/Kconfig:
##########
@@ -3246,11 +3247,12 @@ config SAMA5_ADC_ANARCH
 	default n
 	---help---
 		This option allows you to select different gain, offset, and single
-		vs. differential modes for each channel.
+		vs. differential modes for each channel, depending on ARCH.
 
 if SAMA5_ADC_ANARCH
 
 menu "Channel gain"
+    depends on ARCH_CHIP_SAMA5D

Review Comment:
   TABs



##########
arch/arm/src/sama5/Kconfig:
##########
@@ -3580,25 +3584,28 @@ if !SAMA5_ADC_ANARCH
 config SAMA5_ADC_GAIN
 	int "Analog gain"
 	default 1
-	depends on SAMA5_ADC_CHAN0
+	depends on ARCH_CHIP_SAMA5D3
 	range 0 3
 	---help---
 		Valid gain settings are {0, 1, 2, 3} which may be interpreted as
 		either {1, 1, 2, 4} if the channels are configured for single ended
 		mode or as {0.5, 1, 2, 2} if the channels are configured for
-		differential mode.
+		differential mode. Applies to all channels.
 
 config SAMA5_ADC_OFFSET
 	bool "Offset"
 	default n
+    depends on ARCH_CHIP_SAMA5D3

Review Comment:
   TABs



##########
arch/arm/src/sama5/Kconfig:
##########
@@ -3387,6 +3389,7 @@ config SAMA5_ADC_GAIN11
 endmenu # Channel gain
 
 menu "Channel offsets"
+    depends on ARCH_CHIP_SAMA5D3

Review Comment:
   TABs



##########
arch/arm/src/sama5/Kconfig:
##########
@@ -3712,6 +3763,17 @@ config SAMA5_ADC_TIOA_BOTH
 
 endchoice # ADTRG edge
 endif # SAMA5_ADC_TIOATRIG
+
+if SAMA5_ADC_PERIODIC_TRIG
+
+config SAMA5_ADC_TRIGGER_PERIOD
+	int "ADC Periodic Trigger Rate, useconds"
+	default 50000
+	---help---
+    This setting determines the periodic sample trigger rate in useconds.

Review Comment:
   TABs



##########
arch/arm/src/sama5/Kconfig:
##########
@@ -3627,12 +3644,45 @@ config SAMA5_ADC_TIOATRIG
 		A-to-D Conversion is initiated the A output from one of
 		Timer/Counter 0 channels.
 
+config SAMA5_ADC_PWMTRIG
+  bool "PWM Event trigger"
+  depends on SAMA5_PWM
+  ---help---
+    A-to-D Conversion is initiated from the PWM event lines
+
+config SAMA5_ADC_RTCOUT
+  bool "RTC Out trigger"
+  depends on SAMA5_RTC
+  depends on ARCH_CHIP_SAMA5D2
+  ---help---
+    A-to-D Conversion is initiated from the RTC output
+

Review Comment:
   TABs



##########
arch/arm/src/sama5/Kconfig:
##########
@@ -3627,12 +3644,45 @@ config SAMA5_ADC_TIOATRIG
 		A-to-D Conversion is initiated the A output from one of
 		Timer/Counter 0 channels.
 
+config SAMA5_ADC_PWMTRIG
+  bool "PWM Event trigger"
+  depends on SAMA5_PWM
+  ---help---
+    A-to-D Conversion is initiated from the PWM event lines
+
+config SAMA5_ADC_RTCOUT
+  bool "RTC Out trigger"
+  depends on SAMA5_RTC
+  depends on ARCH_CHIP_SAMA5D2
+  ---help---
+    A-to-D Conversion is initiated from the RTC output
+
 endchoice # Trigger mode
 
+if SAMA5_ADC_PWMTRIG
+
+choice
+  prompt "PWM Event Line Selection"
+  default SAMA5_ADC_PWMTRIG_LINE0
+
+config SAMA5_ADC_PWM_TRIG_LINE0
+  bool "PWM event Line 0"
+  ---help---
+    Trigger A-to-D conversion on PWM event line 0
+
+config SAMA5_ADC_PWM_TRIG_LINE1
+  bool "PWM event Line 1"
+  ---help---
+    Trigger A-to-D conversion on PWM event line 1
+
+endchoice # PWM Event Line Selection
+endif # SAMA5_ADC_PWMTRIG
+
+if SAMA5_ADC_ADTRG || SAMA5_ADC_TIOATRIG || SAMA5_ADC_PWMTRIG || SAMA5_ADC_RTCOUT
+

Review Comment:
   TABs



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] TimJTi commented on a diff in pull request #7702: Fix SAMA5D2 ADC and TSD problems

Posted by GitBox <gi...@apache.org>.
TimJTi commented on code in PR #7702:
URL: https://github.com/apache/nuttx/pull/7702#discussion_r1033439446


##########
arch/arm/src/sama5/Kconfig:
##########
@@ -3580,25 +3584,28 @@ if !SAMA5_ADC_ANARCH
 config SAMA5_ADC_GAIN
 	int "Analog gain"
 	default 1
-	depends on SAMA5_ADC_CHAN0
+	depends on ARCH_CHIP_SAMA5D3
 	range 0 3
 	---help---
 		Valid gain settings are {0, 1, 2, 3} which may be interpreted as
 		either {1, 1, 2, 4} if the channels are configured for single ended
 		mode or as {0.5, 1, 2, 2} if the channels are configured for
-		differential mode.
+		differential mode. Applies to all channels.
 
 config SAMA5_ADC_OFFSET
 	bool "Offset"
 	default n
+    depends on ARCH_CHIP_SAMA5D3

Review Comment:
   All lined up in VS Code. Have deleted and reinserted and its still lined up.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] TimJTi commented on a diff in pull request #7702: Fix SAMA5D2 ADC and TSD problems

Posted by GitBox <gi...@apache.org>.
TimJTi commented on code in PR #7702:
URL: https://github.com/apache/nuttx/pull/7702#discussion_r1037597230


##########
arch/arm/src/sama5/sam_adc.c:
##########
@@ -865,7 +935,7 @@ static void sam_adc_endconversion(void *arg)
   int ret;
 
   DEBUGASSERT(priv != NULL);
-  ainfo("pending=%08x\n", priv->pending);
+  ainfo("pending=%08lx\n", priv->pending);

Review Comment:
   I come across so many of these in existing code....



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] pkarashchenko commented on a diff in pull request #7702: Fix SAMA5D2 ADC and TSD problems

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #7702:
URL: https://github.com/apache/nuttx/pull/7702#discussion_r1037677855


##########
arch/arm/src/sama5/sam_adc.c:
##########
@@ -865,7 +935,7 @@ static void sam_adc_endconversion(void *arg)
   int ret;
 
   DEBUGASSERT(priv != NULL);
-  ainfo("pending=%08x\n", priv->pending);
+  ainfo("pending=%08lx\n", priv->pending);

Review Comment:
   We need to fix this



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] TimJTi commented on a diff in pull request #7702: Fix SAMA5D2 ADC and TSD problems

Posted by GitBox <gi...@apache.org>.
TimJTi commented on code in PR #7702:
URL: https://github.com/apache/nuttx/pull/7702#discussion_r1033438752


##########
arch/arm/src/sama5/Kconfig:
##########
@@ -3487,6 +3490,7 @@ config SAMA5_ADC_OFFSET11
 endmenu # Channel offsets
 
 menu "Channel differential mode"
+    depends on ARCH_CHIP_SAMA5D2 || ARCH_CHIP_SAMA5D3

Review Comment:
   Ditto



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] acassis commented on a diff in pull request #7702: Fix SAMA5D2 ADC and TSD problems

Posted by GitBox <gi...@apache.org>.
acassis commented on code in PR #7702:
URL: https://github.com/apache/nuttx/pull/7702#discussion_r1032808973


##########
arch/arm/src/sama5/sam_tsd.c:
##########
@@ -1551,21 +1628,44 @@ static void sam_tsd_initialize(struct sam_tsd_s *priv)
 
   /* Enable pen contact detection */
 
+  regval  = sam_adc_getreg(priv->adc, SAM_ADC_TSMR);
   regval |= ADC_TSMR_PENDET;
   sam_adc_putreg(priv->adc, SAM_ADC_TSMR, regval);
 
   /* Set up pen debounce time */
 
   sam_tsd_debounce(priv, BOARD_TSD_DEBOUNCE);
 
+  /* configure pen sensitivity */
+
+  regval = sam_adc_getreg(priv->adc, SAM_ADC_ACR);
+  regval &= ~ADC_ACR_PENDETSENS_MASK;
+  regval |= ADC_ACR_PENDETSENS(BOARD_TSD_PENDETSENS);
+#if defined(ATSAMA5D2)
+  regval &= ~ADC_ACR_IBCTL_MASK;
+  regval |= ADC_ACR_IBCTL(BOARD_TSD_IBCTL);
+#endif
+  sam_adc_putreg(priv->adc, SAM_ADC_ACR, regval);
+
+#ifdef SAMA5_TSD_PENDET_TRIG_ALLOWED
   /* Configure pen interrupt generation */
 
   regval  = sam_adc_getreg(priv->adc, SAM_ADC_TRGR);
   regval &= ~ADC_TRGR_TRGMOD_MASK;
   regval |= ADC_TRGR_TRGMOD_PEN;
   sam_adc_putreg(priv->adc, SAM_ADC_TRGR, regval);
+#endif
 
   sam_adc_putreg(priv->adc, SAM_ADC_IER, ADC_INT_PEN);
+
+#ifdef CONFIG_SAMA5_TSD_AUTOCALIB
+  /* perform a ts calibration */

Review Comment:
   ditto



##########
arch/arm/src/sama5/sam_tsd.c:
##########
@@ -1551,21 +1628,44 @@ static void sam_tsd_initialize(struct sam_tsd_s *priv)
 
   /* Enable pen contact detection */
 
+  regval  = sam_adc_getreg(priv->adc, SAM_ADC_TSMR);
   regval |= ADC_TSMR_PENDET;
   sam_adc_putreg(priv->adc, SAM_ADC_TSMR, regval);
 
   /* Set up pen debounce time */
 
   sam_tsd_debounce(priv, BOARD_TSD_DEBOUNCE);
 
+  /* configure pen sensitivity */

Review Comment:
   Please capitalized text as all other comments on this file 



##########
arch/arm/src/sama5/sam_tsd.h:
##########
@@ -36,10 +36,15 @@
 
 /* Configuration ************************************************************/
 
-#ifdef CONFIG_SAMA_TSD_RXP
+#ifndef CONFIG_SAMA_TSD_RXP
 #  define CONFIG_SAMA_TSD_RXP 6
 #endif
 
+/* Only allow Pendet triggering in limited circumstances */

Review Comment:
   Add space after comment



##########
arch/arm/src/sama5/sam_tsd.c:
##########
@@ -630,7 +680,7 @@ static void sam_tsd_bottomhalf(void *arg)
       pressr = sam_adc_getreg(priv->adc, SAM_ADC_PRESSR);
 #endif
       /* Discard any bad readings.  This check may not be necessary. */
-
+#if 1

Review Comment:
   Why to keep this "#if 1" ? Was it a test that succeed and you forgot to remove of #if 1? 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] TimJTi commented on a diff in pull request #7702: Fix SAMA5D2 ADC and TSD problems

Posted by GitBox <gi...@apache.org>.
TimJTi commented on code in PR #7702:
URL: https://github.com/apache/nuttx/pull/7702#discussion_r1038516692


##########
arch/arm/src/sama5/sam_adc.c:
##########
@@ -865,7 +935,7 @@ static void sam_adc_endconversion(void *arg)
   int ret;
 
   DEBUGASSERT(priv != NULL);
-  ainfo("pending=%08x\n", priv->pending);
+  ainfo("pending=%08lx\n", priv->pending);

Review Comment:
   Yes, but I want to differentiate between errors that I should fix, or preferences. I think this is in the preference category, so I don't want to spend time fixing issues in other peoples code if they're not actually an issue or error.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] xiaoxiang781216 commented on a diff in pull request #7702: Fix SAMA5D2 ADC and TSD problems

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #7702:
URL: https://github.com/apache/nuttx/pull/7702#discussion_r1038769231


##########
arch/arm/src/sama5/sam_adc.c:
##########
@@ -865,7 +935,7 @@ static void sam_adc_endconversion(void *arg)
   int ret;
 
   DEBUGASSERT(priv != NULL);
-  ainfo("pending=%08x\n", priv->pending);
+  ainfo("pending=%08lx\n", priv->pending);

Review Comment:
   > That discussion is slightly beyond me, but I'm trying to understand if using %08lx is actually WRONG rather than just a difference in approach.
   
   no prefix mean int, l prefix is for long int. But int32_t may map to int or long int which depends on arch and toolchain, so it's wrong to either use l prefix or no prefix for int32_t. To make programmer life easier, the standard introduce PRI[d|u|x]32 as the int32_t format specifier.
   
   > We _can_ use C99 in arch but don't HAVE to? I have always used the %08x type formatters, probably because I learnt C back in the 1980's (K&R!).
   > 
   
   So the rule is simple:
   int: no prefix
   long: l prefix
   long long: ll prefix
   int32_t: PRI[o||d|u|x]32
   Here has some discussion:
   https://stackoverflow.com/questions/3168275/printf-format-specifiers-for-uint32-t-and-size-t?rq=1
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] TimJTi commented on a diff in pull request #7702: Fix SAMA5D2 ADC and TSD problems

Posted by GitBox <gi...@apache.org>.
TimJTi commented on code in PR #7702:
URL: https://github.com/apache/nuttx/pull/7702#discussion_r1038291569


##########
arch/arm/src/sama5/sam_tsd.c:
##########
@@ -1655,7 +1755,7 @@ int sam_tsd_register(struct sam_adc_s *adc, int minor)
 
   /* Initialize the touchscreen device driver instance */
 
-  priv->adc = adc; /* Save the ADC device handle */
+  priv->adc     = adc;               /* Save the ADC device handle    */

Review Comment:
   no reason so keyboard error I guess...but checkpatch.sh didn't find it :(
   
   Have corrected.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] TimJTi commented on a diff in pull request #7702: Fix SAMA5D2 ADC and TSD problems

Posted by GitBox <gi...@apache.org>.
TimJTi commented on code in PR #7702:
URL: https://github.com/apache/nuttx/pull/7702#discussion_r1038294627


##########
arch/arm/src/sama5/sam_tsd.c:
##########
@@ -137,6 +137,18 @@
 #  define MAX(a,b) (((a) > (b)) ? (a) : (b))
 #endif
 
+#ifndef BOARD_TSSCTIM
+# define BOARD_TSSCTIM 0

Review Comment:
   done



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] TimJTi commented on a diff in pull request #7702: Fix SAMA5D2 ADC and TSD problems

Posted by GitBox <gi...@apache.org>.
TimJTi commented on code in PR #7702:
URL: https://github.com/apache/nuttx/pull/7702#discussion_r1037614508


##########
arch/arm/src/sama5/sam_adc.c:
##########
@@ -865,7 +935,7 @@ static void sam_adc_endconversion(void *arg)
   int ret;
 
   DEBUGASSERT(priv != NULL);
-  ainfo("pending=%08x\n", priv->pending);
+  ainfo("pending=%08lx\n", priv->pending);

Review Comment:
   But this one is already fixed...its not in master as of now?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org