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/14 18:08:12 UTC

[GitHub] [incubator-nuttx] TimJTi opened a new pull request, #7595: SAMA5Dx ADC and TSD fixes

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

   ## Summary
   Refer to issue #7586
   
   Ensures ADC and TSD work together. TSD now works on SAMA5D2 and should be OK, still, for other SAMA5D familiy members
   
   ## Impact
   
   Hopefully none, other than allowing these peripherals to actually work
   
   ## Testing
   tested on custom board with SAMA5D27-D5M processor with 5" resistive touch LCD and 6 analog input channels
   
   ## Outstanding
   As per issue #7586 I am not 100% happy that interrupts are never up_disabled or detached and welcome advice and suggestions
   


-- 
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] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #7595: SAMA5Dx ADC and TSD fixes

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


##########
arch/arm/src/sama5/hardware/sam_adc.h:
##########
@@ -355,7 +368,11 @@
 #define ADC_INT_NOPEN              (1 << 30) /* Bit 30: No Pen Contact Interrupt */
 #define ADC_SR_PENS                (1 << 31) /* Bit 31: Pen detect Status (SR only) */
 
-#define ADC_INT_ALL                (0xe7f00fff)
+#if defined (ATSAMA5D3)
+#  define ADC_INT_ALL                (0xe7f00fff)

Review Comment:
   ```suggestion
   #  define ADC_INT_ALL              (0xe7f00fff)
   ```



-- 
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] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #7595: SAMA5Dx ADC and TSD fixes

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


##########
arch/arm/src/sama5/hardware/sam_adc.h:
##########
@@ -484,7 +501,12 @@
 /* Channel Data Register */
 
 #define ADC_CDR_DATA_SHIFT         (0)       /* Bits 0-11: Converted Data */
+
+#if defined (ATSAMA5D2)
+#define ADC_CDR_DATA_MASK          (0x3fff << ADC_CDR_DATA_SHIFT)

Review Comment:
   ```suggestion
   #  define ADC_CDR_DATA_MASK        (0x3fff << ADC_CDR_DATA_SHIFT)
   ```



##########
arch/arm/src/sama5/hardware/sam_adc.h:
##########
@@ -484,7 +501,12 @@
 /* Channel Data Register */
 
 #define ADC_CDR_DATA_SHIFT         (0)       /* Bits 0-11: Converted Data */
+
+#if defined (ATSAMA5D2)
+#define ADC_CDR_DATA_MASK          (0x3fff << ADC_CDR_DATA_SHIFT)
+#else
 #define ADC_CDR_DATA_MASK          (0xfff << ADC_CDR_DATA_SHIFT)

Review Comment:
   ```suggestion
   #  define ADC_CDR_DATA_MASK        (0xfff << ADC_CDR_DATA_SHIFT)
   ```



-- 
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] [incubator-nuttx] TimJTi commented on a diff in pull request #7595: SAMA5Dx ADC and TSD fixes

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


##########
arch/arm/src/sama5/Kconfig:
##########
@@ -3615,6 +3615,16 @@ config SAMA5_ADC_SWTRIG
 	---help---
 		A-to-D Conversion is initiated only by software via an ioctl()
 
+config SAMA5_ADC_PERIODIC_TRIG
+	bool "Periodic trigger"
+	---help---
+		A-to-D Conversion is triggered periodically
+
+config SAMA5_ADC_CONTINUOUS_TRIG

Review Comment:
   Think I may have included an incorrect Kconfig - I'll check.



-- 
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] [incubator-nuttx] TimJTi commented on a diff in pull request #7595: SAMA5Dx ADC and TSD fixes

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


##########
arch/arm/src/sama5/Kconfig:
##########
@@ -3712,6 +3722,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:
   Ah, OK. Still learning!



-- 
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] [incubator-nuttx] TimJTi commented on pull request #7595: SAMA5Dx ADC and TSD fixes

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

   Failing check is how the file was before my changes. Not looked at what happens if I move the #include, and open to suggestions.
   


-- 
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 closed pull request #7595: SAMA5Dx ADC and TSD fixes

Posted by GitBox <gi...@apache.org>.
TimJTi closed pull request #7595: SAMA5Dx ADC and TSD fixes
URL: https://github.com/apache/nuttx/pull/7595


-- 
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] [incubator-nuttx] TimJTi commented on pull request #7595: SAMA5Dx ADC and TSD fixes

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

   first check fails with:
   
   [check: arch/arm/src/samv7/sam_pwm.c#L489]
   Missing blank line after comment
   
   Not even edited that file, and can't see anything wrong with it at that line. It passes checkpatch.sh


-- 
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] [incubator-nuttx] TimJTi commented on a diff in pull request #7595: SAMA5Dx ADC and TSD fixes

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


##########
arch/arm/src/sama5/sam_tsd.c:
##########
@@ -1655,7 +1735,11 @@ 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 */
+  memset(priv, 0, sizeof(struct sam_tsd_s));

Review Comment:
   Thank you for pointing all these out. Should I do the squash before or after I make these corrections? Git is not my strong point!



-- 
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] [incubator-nuttx] TimJTi commented on a diff in pull request #7595: SAMA5Dx ADC and TSD fixes

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


##########
arch/arm/src/sama5/Kconfig:
##########
@@ -3615,6 +3615,16 @@ config SAMA5_ADC_SWTRIG
 	---help---
 		A-to-D Conversion is initiated only by software via an ioctl()
 
+config SAMA5_ADC_PERIODIC_TRIG
+	bool "Periodic trigger"
+	---help---
+		A-to-D Conversion is triggered periodically
+
+config SAMA5_ADC_CONTINUOUS_TRIG

Review Comment:
   No - the modes are different. 
   
   Periodic means the ADC is triggered regularly at a rate set in the Trigger register, using the value (now) set in Kconfig via SAMA5_ADC_TRIGGER_PERIOD.
   
   Continuous means it will start a new conversion sequence as soon as the current one finishes. 



-- 
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] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #7595: SAMA5Dx ADC and TSD fixes

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


##########
arch/arm/src/sama5/hardware/sam_adc.h:
##########
@@ -501,6 +523,12 @@
 #define ADC_ACR_PENDETSENS_MASK    (3 << ADC_ACR_PENDETSENS_SHIFT)
 #  define ADC_ACR_PENDETSENS(n)    ((uint32_t)(n) << ADC_ACR_PENDETSENS_SHIFT)
 
+#if defined (ATSAMA5D2)
+#define ADC_ACR_IBTL_SHIFT         (8) /* Bits 8-9: ADC Bias Current Control */

Review Comment:
   ```suggestion
   #  define ADC_ACR_IBTL_SHIFT       (8) /* Bits 8-9: ADC Bias Current 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] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #7595: SAMA5Dx ADC and TSD fixes

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


##########
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
 
+#if (defined CONFIG_SAMA5_ADC && defined CONFIG_SAMA5_ADC_SWTRIG) || !defined CONFIG_SAMA5_ADC

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



-- 
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] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #7595: SAMA5Dx ADC and TSD fixes

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


##########
arch/arm/src/sama5/sam_tsd.c:
##########
@@ -1655,7 +1735,11 @@ 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 */
+  memset(priv, 0, sizeof(struct sam_tsd_s));

Review Comment:
   squash first and then make the change. Please use --amend option after you finish:
   git commit --amend



-- 
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] [incubator-nuttx] TimJTi commented on a diff in pull request #7595: SAMA5Dx ADC and TSD fixes

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


##########
arch/arm/src/sama5/Kconfig:
##########
@@ -3615,6 +3615,16 @@ config SAMA5_ADC_SWTRIG
 	---help---
 		A-to-D Conversion is initiated only by software via an ioctl()
 
+config SAMA5_ADC_PERIODIC_TRIG
+	bool "Periodic trigger"
+	---help---
+		A-to-D Conversion is triggered periodically
+
+config SAMA5_ADC_CONTINUOUS_TRIG

Review Comment:
   OK...may be a quirk of Kconfig but the selections actually work as being mutually exclusive



-- 
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] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #7595: SAMA5Dx ADC and TSD fixes

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


##########
arch/arm/src/sama5/sam_adc.c:
##########
@@ -497,20 +498,11 @@ static const struct adc_ops_s g_adcops =
 
 /* ADC internal state */
 
-static struct sam_adc_s g_adcpriv =
-{
-  .lock        = NXMUTEX_INITIALIZER,
-};
+static struct sam_adc_s g_adcpriv;
 
 /* ADC device instance */
 
-static struct adc_dev_s g_adcdev =
-{
-#ifdef SAMA5_ADC_HAVE_CHANNELS
-  .ad_ops      = &g_adcops,
-#endif
-  .ad_priv     = &g_adcpriv,
-};
+static struct adc_dev_s g_adcdev;

Review Comment:
   why change



##########
arch/arm/src/sama5/sam_adc.c:
##########
@@ -497,20 +498,11 @@ static const struct adc_ops_s g_adcops =
 
 /* ADC internal state */
 
-static struct sam_adc_s g_adcpriv =
-{
-  .lock        = NXMUTEX_INITIALIZER,
-};
+static struct sam_adc_s g_adcpriv;

Review Comment:
   why change



##########
arch/arm/src/sama5/Kconfig:
##########
@@ -3615,6 +3615,16 @@ config SAMA5_ADC_SWTRIG
 	---help---
 		A-to-D Conversion is initiated only by software via an ioctl()
 
+config SAMA5_ADC_PERIODIC_TRIG
+	bool "Periodic trigger"
+	---help---
+		A-to-D Conversion is triggered periodically
+
+config SAMA5_ADC_CONTINUOUS_TRIG

Review Comment:
   does SAMA5_ADC_PERIODIC_TRIG and SAMA5_ADC_CONTINUOUS_TRIG work at the same 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] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #7595: SAMA5Dx ADC and TSD fixes

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


##########
arch/arm/src/sama5/Kconfig:
##########
@@ -3615,6 +3615,16 @@ config SAMA5_ADC_SWTRIG
 	---help---
 		A-to-D Conversion is initiated only by software via an ioctl()
 
+config SAMA5_ADC_PERIODIC_TRIG
+	bool "Periodic trigger"
+	---help---
+		A-to-D Conversion is triggered periodically
+
+config SAMA5_ADC_CONTINUOUS_TRIG

Review Comment:
   If so you should use choice/endchoice like this:
   https://github.com/apache/incubator-nuttx/blob/master/Kconfig#L89-L105
   to avoid the user select both options.



-- 
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 #7595: SAMA5Dx ADC and TSD fixes

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


##########
arch/arm/src/sama5/sam_tsd.c:
##########
@@ -245,12 +257,7 @@ static const struct file_operations g_tsdops =
 
 /* The driver state structure is pre-allocated. */
 
-static struct sam_tsd_s g_tsd =
-{
-  .threshx = INVALID_THRESHOLD,
-  .threshy = INVALID_THRESHOLD,
-  .waitsem = SEM_INITIALIZER(0),
-};
+static struct sam_tsd_s g_tsd;

Review Comment:
   No idea...it seems my changes are conflicting with other changes so either my rebase/merge/whatever its called, to master  at that time, or later, who knows didn't do what I thought. This is SOOOO hard for me but really trying to make this code work so others can use it if they ever want to as it never would have worked at all in the real world.
   
   I will give up on this PR, do may changes again, and make a clean one.



-- 
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 #7595: SAMA5Dx ADC and TSD fixes

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

   Closing as my lack of github skills have made this PR a waste of 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] [incubator-nuttx] TimJTi commented on pull request #7595: SAMA5Dx ADC and TSD fixes

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

   I've had a "version control" issue and the files submitted don't properly build without other changes. I will push further changes and change it back from "draft" once I'm happier.
   
   Thanks for the input - my first ever PR to NuttX was always going to be traumatic!
   


-- 
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] [incubator-nuttx] xiaoxiang781216 commented on pull request #7595: SAMA5Dx ADC and TSD fixes

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

   > first check fails with:
   > 
   > [check: arch/arm/src/samv7/sam_pwm.c#L489] Missing blank line after comment
   > 
   > Not even edited that file, and can't see anything wrong with it at that line. It passes checkpatch.sh
   
   Here is fix: https://github.com/apache/incubator-nuttx/pull/7608


-- 
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] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #7595: SAMA5Dx ADC and TSD fixes

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


##########
arch/arm/src/sama5/hardware/sam_adc.h:
##########
@@ -501,6 +523,12 @@
 #define ADC_ACR_PENDETSENS_MASK    (3 << ADC_ACR_PENDETSENS_SHIFT)
 #  define ADC_ACR_PENDETSENS(n)    ((uint32_t)(n) << ADC_ACR_PENDETSENS_SHIFT)
 
+#if defined (ATSAMA5D2)
+#define ADC_ACR_IBTL_SHIFT         (8) /* Bits 8-9: ADC Bias Current Control */
+#define ADC_ACR_IBCTL_MASK         (3 << ADC_ACR_IBTL_SHIFT)

Review Comment:
   ```suggestion
   #  define ADC_ACR_IBCTL_MASK       (3 << ADC_ACR_IBTL_SHIFT)
   ```



-- 
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] [incubator-nuttx] pkarashchenko commented on a diff in pull request #7595: SAMA5Dx ADC and TSD fixes

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


##########
arch/arm/src/sama5/Kconfig:
##########
@@ -3712,6 +3722,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:
   please use TABs only in Kconfig files



##########
arch/arm/src/sama5/hardware/sam_adc.h:
##########
@@ -119,6 +120,9 @@
 
 #ifdef ATSAMA5D3
 #  define SAM_ADC_CGR              (SAM_TSADC_VBASE+SAM_ADC_CGR_OFFSET)
+#endif
+
+#if defined (ATSAMA5D3) || defined (ATSAMA5D2)

Review Comment:
   ditto



##########
arch/arm/src/sama5/sam_tsd.c:
##########
@@ -245,12 +257,7 @@ static const struct file_operations g_tsdops =
 
 /* The driver state structure is pre-allocated. */
 
-static struct sam_tsd_s g_tsd =
-{
-  .threshx = INVALID_THRESHOLD,
-  .threshy = INVALID_THRESHOLD,
-  .waitsem = SEM_INITIALIZER(0),
-};
+static struct sam_tsd_s g_tsd;

Review Comment:
   why this is changed?



##########
arch/arm/src/sama5/hardware/sam_adc.h:
##########
@@ -36,7 +36,7 @@
 
 /* General definitions ******************************************************/
 
-#if defined(ATSAMA5D3)
+#if defined(ATSAMA5D3) || defined (ATSAMA5D2)

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



##########
arch/arm/src/sama5/hardware/sam_adc.h:
##########
@@ -129,7 +133,7 @@
 #define SAM_ADC_CDR3               (SAM_TSADC_VBASE+SAM_ADC_CDR3_OFFSET)
 #define SAM_ADC_CDR4               (SAM_TSADC_VBASE+SAM_ADC_CDR4_OFFSET)
 
-#ifdef ATSAMA5D3
+#if defined (ATSAMA5D3) || defined (ATSAMA5D2)

Review Comment:
   ditto



##########
arch/arm/src/sama5/hardware/sam_adc.h:
##########
@@ -204,21 +208,30 @@
 #  define ADC_MR_STARTUP_960       (15 << ADC_MR_STARTUP_SHIFT) /* 960 periods of ADCClock */
 
 #ifdef ATSAMA5D3
-#  define ADC_MR_SETTLING_SHIFT    (20)      /* Bits 20-21: Analog Settling Time */
+#  define ADC_MR_SETTLING_SHIFT    (20)      /* Bits 20-21: Analog Settling Time           */
 #  define ADC_MR_SETTLING_MASK     (15 << ADC_MR_SETTLING_SHIFT)
-#    define ADC_MR_SETTLING_3      (0 << ADC_MR_SETTLING_SHIFT) /* 3 periods of ADCClock */
-#    define ADC_MR_SETTLING_5      (1 << ADC_MR_SETTLING_SHIFT) /* 5 periods of ADCClock */
-#    define ADC_MR_SETTLING_9      (2 << ADC_MR_SETTLING_SHIFT) /* 9 periods of ADCClock */
-#    define ADC_MR_SETTLING_17     (3 << ADC_MR_SETTLING_SHIFT) /* 17 periods of ADCClock */
+#    define ADC_MR_SETTLING_3      (0 << ADC_MR_SETTLING_SHIFT) /* 3 periods of ADCClock   */
+#    define ADC_MR_SETTLING_5      (1 << ADC_MR_SETTLING_SHIFT) /* 5 periods of ADCClock   */
+#    define ADC_MR_SETTLING_9      (2 << ADC_MR_SETTLING_SHIFT) /* 9 periods of ADCClock   */
+#    define ADC_MR_SETTLING_17     (3 << ADC_MR_SETTLING_SHIFT) /* 17 periods of ADCClock  */
+#else
+#  define ADC_MR_SETTLING_SHIFT    (20)      /* Not present in SAMA5D2 or SAMA5D4          */
+#  define ADC_MR_SETTLING_MASK     (0)
+#    define ADC_MR_SETTLING_3      (0 << ADC_MR_SETTLING_SHIFT) /* n/a periods of ADCClock */
+#    define ADC_MR_SETTLING_5      (0 << ADC_MR_SETTLING_SHIFT) /* n/a periods of ADCClock */
+#    define ADC_MR_SETTLING_9      (0 << ADC_MR_SETTLING_SHIFT) /* n/a periods of ADCClock */
+#    define ADC_MR_SETTLING_17     (0 << ADC_MR_SETTLING_SHIFT) /* n/a periods of ADCClock */
+#endif
 
+#if defined (ATSAMA5D3) || defined (ATSAMA5D2)

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



##########
arch/arm/src/sama5/sam_adc.c:
##########
@@ -2132,6 +2247,9 @@ struct adc_dev_s *sam_adc_initialize(void)
                 ADC_MR_SETTLING_MASK);
       regval |= (ADC_MR_STARTUP_512 | ADC_MR_TRACKTIM(0) |
                 ADC_MR_SETTLING_17);
+#if defined ATSAMA5D2

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



##########
arch/arm/src/sama5/hardware/sam_adc.h:
##########
@@ -204,21 +208,30 @@
 #  define ADC_MR_STARTUP_960       (15 << ADC_MR_STARTUP_SHIFT) /* 960 periods of ADCClock */
 
 #ifdef ATSAMA5D3
-#  define ADC_MR_SETTLING_SHIFT    (20)      /* Bits 20-21: Analog Settling Time */
+#  define ADC_MR_SETTLING_SHIFT    (20)      /* Bits 20-21: Analog Settling Time           */
 #  define ADC_MR_SETTLING_MASK     (15 << ADC_MR_SETTLING_SHIFT)
-#    define ADC_MR_SETTLING_3      (0 << ADC_MR_SETTLING_SHIFT) /* 3 periods of ADCClock */
-#    define ADC_MR_SETTLING_5      (1 << ADC_MR_SETTLING_SHIFT) /* 5 periods of ADCClock */
-#    define ADC_MR_SETTLING_9      (2 << ADC_MR_SETTLING_SHIFT) /* 9 periods of ADCClock */
-#    define ADC_MR_SETTLING_17     (3 << ADC_MR_SETTLING_SHIFT) /* 17 periods of ADCClock */
+#    define ADC_MR_SETTLING_3      (0 << ADC_MR_SETTLING_SHIFT) /* 3 periods of ADCClock   */
+#    define ADC_MR_SETTLING_5      (1 << ADC_MR_SETTLING_SHIFT) /* 5 periods of ADCClock   */
+#    define ADC_MR_SETTLING_9      (2 << ADC_MR_SETTLING_SHIFT) /* 9 periods of ADCClock   */
+#    define ADC_MR_SETTLING_17     (3 << ADC_MR_SETTLING_SHIFT) /* 17 periods of ADCClock  */
+#else
+#  define ADC_MR_SETTLING_SHIFT    (20)      /* Not present in SAMA5D2 or SAMA5D4          */
+#  define ADC_MR_SETTLING_MASK     (0)
+#    define ADC_MR_SETTLING_3      (0 << ADC_MR_SETTLING_SHIFT) /* n/a periods of ADCClock */
+#    define ADC_MR_SETTLING_5      (0 << ADC_MR_SETTLING_SHIFT) /* n/a periods of ADCClock */
+#    define ADC_MR_SETTLING_9      (0 << ADC_MR_SETTLING_SHIFT) /* n/a periods of ADCClock */
+#    define ADC_MR_SETTLING_17     (0 << ADC_MR_SETTLING_SHIFT) /* n/a periods of ADCClock */
+#endif
 
+#if defined (ATSAMA5D3) || defined (ATSAMA5D2)
 #  define ADC_MR_ANACH             (1 << 23) /* Bit 23: Analog Change */
 #endif
 
 #define ADC_MR_TRACKTIM_SHIFT      (24)      /* Bits 24-27: Tracking Time */
 #define ADC_MR_TRACKTIM_MASK       (15 << ADC_MR_TRACKTIM_SHIFT)
 #  define ADC_MR_TRACKTIM(n)       ((uint32_t)(n) << ADC_MR_TRACKTIM_SHIFT)
 
-#ifdef ATSAMA5D3
+#if defined (ATSAMA5D3) || defined (ATSAMA5D2)

Review Comment:
   ditto



##########
arch/arm/src/sama5/hardware/sam_adc.h:
##########
@@ -67,17 +67,18 @@
 
 #ifdef ATSAMA5D3
 #  define SAM_ADC_CGR_OFFSET       0x0048 /* Channel Gain Register */
-#  define SAM_ADC_COR_OFFSET       0x004c /* Channel Offset Register */
 #endif
 
+#define SAM_ADC_COR_OFFSET         0x004c /* Channel Offset Register */
+
 #define SAM_ADC_CDR_OFFSET(n)      (0x0050+((n)<<2))
 #define SAM_ADC_CDR0_OFFSET        0x0050 /* Channel Data Register 0 */
 #define SAM_ADC_CDR1_OFFSET        0x0054 /* Channel Data Register 1 */
 #define SAM_ADC_CDR2_OFFSET        0x0058 /* Channel Data Register 2 */
 #define SAM_ADC_CDR3_OFFSET        0x005c /* Channel Data Register 3 */
 #define SAM_ADC_CDR4_OFFSET        0x0060 /* Channel Data Register 4 */
 
-#ifdef ATSAMA5D3
+#if defined (ATSAMA5D3) || defined (ATSAMA5D2)

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



##########
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
   ```



##########
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
+#endif
+
+#ifndef BOARD_TSD_PENDETSENS
+# define BOARD_TSD_PENDETSENS 0

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



##########
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
+#endif
+
+#ifndef BOARD_TSD_PENDETSENS
+# define BOARD_TSD_PENDETSENS 0
+#endif
+
+#if !defined BOARD_TSD_IBCTL && defined ATSAMA5D2
+# define BOARD_TSD_IBCTL 0

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



##########
arch/arm/src/sama5/sam_tsd.c:
##########
@@ -1291,6 +1349,8 @@ static void sam_tsd_tracking(struct sam_tsd_s *priv, uint32_t time)
   uint32_t tracktim;
   uint32_t regval;
 
+#if defined (ATSAMA5D4)

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



##########
arch/arm/src/sama5/sam_tsd.c:
##########
@@ -1551,21 +1609,42 @@ 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

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



-- 
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] [incubator-nuttx] TimJTi commented on pull request #7595: SAMA5Dx ADC and TSD fixes

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

   Build error due to unused variable. Will correct but will wait on any other comments first.


-- 
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] [incubator-nuttx] xiaoxiang781216 commented on pull request #7595: SAMA5Dx ADC and TSD fixes

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

   @TimJTi please squash your patch into one:
   ```
   git rebase --interactive HEAD~3
   change the last two pick to squash
   git push -f origin
   ```


-- 
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] [incubator-nuttx] TimJTi commented on a diff in pull request #7595: SAMA5Dx ADC and TSD fixes

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


##########
arch/arm/src/sama5/Kconfig:
##########
@@ -3615,6 +3615,16 @@ config SAMA5_ADC_SWTRIG
 	---help---
 		A-to-D Conversion is initiated only by software via an ioctl()
 
+config SAMA5_ADC_PERIODIC_TRIG
+	bool "Periodic trigger"
+	---help---
+		A-to-D Conversion is triggered periodically
+
+config SAMA5_ADC_CONTINUOUS_TRIG

Review Comment:
   I see other errors in what was there before, and not all modes are actually covered or handled logically, so I will rework Kconfig and sam_adc.c to fully and properly support the peripheral.



-- 
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] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #7595: SAMA5Dx ADC and TSD fixes

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


##########
arch/arm/src/sama5/hardware/sam_adc.h:
##########
@@ -355,7 +368,11 @@
 #define ADC_INT_NOPEN              (1 << 30) /* Bit 30: No Pen Contact Interrupt */
 #define ADC_SR_PENS                (1 << 31) /* Bit 31: Pen detect Status (SR only) */
 
-#define ADC_INT_ALL                (0xe7f00fff)
+#if defined (ATSAMA5D3)
+#  define ADC_INT_ALL                (0xe7f00fff)
+#elif defined (ATSAMA5D2)
+#  define ADC_INT_ALL                (0x67780fff)

Review Comment:
   ```suggestion
   #  define ADC_INT_ALL              (0x67780fff)
   ```



-- 
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] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #7595: SAMA5Dx ADC and TSD fixes

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


##########
arch/arm/src/sama5/sam_tsd.c:
##########
@@ -1655,7 +1735,11 @@ 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 */
+  memset(priv, 0, sizeof(struct sam_tsd_s));

Review Comment:
   remove memset



-- 
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