ppisa commented on code in PR #12081:
URL: https://github.com/apache/nuttx/pull/12081#discussion_r1567252446
##########
arch/arm/src/samv7/sam_qencoder.c:
##########
@@ -270,11 +316,139 @@ static int sam_reset(struct qe_lowerhalf_s *lower)
static int sam_ioctl(struct qe_lowerhalf_s *lower, int cmd,
unsigned long arg)
{
- /* No ioctl commands supported */
-
+#ifdef CONFIG_SAMV7_QENCODER_ENABLE_GETINDEX
+ switch (cmd)
+ {
+ case QEIOC_GETINDEX:
+ {
+ /* Call the qeindex function */
+
+ sam_qeindex(lower, (struct qe_index_s *) arg);
+ return OK;
+ }
+
+ default:
+ {
+ return -ENOTTY;
+ }
+ }
+#else
return -ENOTTY;
+#endif
}
+#ifdef CONFIG_SAMV7_QENCODER_ENABLE_GETINDEX
+/****************************************************************************
+ * Name: sam_qe_pos_16to32b
+ *
+ * Description:
+ * An inline function performing the extension of current position.
+ * Last reading is saved to priv->last_pos.
+ *
+ ****************************************************************************/
+
+static inline int32_t sam_qe_pos_16to32b(struct qe_lowerhalf_s *lower,
+ uint32_t current_pos)
+{
+ struct sam_lowerhalf_s *priv = (struct sam_lowerhalf_s *) lower;
+
+ uint32_t new_pos = *(volatile uint32_t *) &priv->last_pos;
+ new_pos += (int16_t) (current_pos - new_pos);
+ *(volatile uint32_t *) &priv->last_pos = new_pos;
+
+ return (int32_t) new_pos;
+}
+#endif
+
+#ifdef CONFIG_SAMV7_QENCODER_ENABLE_GETINDEX
+/****************************************************************************
+ * Name: sam_qe_indx_pos_16to32b
+ *
+ * Description:
+ * An inline function performing the extension of the last index position.
+ * Last reading is saved to priv->last_index.
+ *
+ ****************************************************************************/
+
+static inline int32_t sam_qe_indx_pos_16to32b(struct qe_lowerhalf_s *lower,
+ uint32_t current_indx_pos)
+{
+ struct sam_lowerhalf_s *priv = (struct sam_lowerhalf_s *) lower;
+
+ uint32_t new_index = *(volatile uint32_t *) &priv->last_pos;
Review Comment:
The option is to protect IOCTL operations access by critical section, but it
is not necessary in this case under above and later described conditions. Code
is written such way that for example high priority PMSM control task can read
position by IOCT and position can be accessed by low priority task for
monitoring purposes in parallel. No locking is necessary but what is mandatory
that subtraction and following addition is done against the same value last_pos
snapshot inside each invocation of sam_qe_pos_16to32b(). The last_pos field can
be changed to atomic and atomic access can be used but it is not necessary and
barrier instructions are overhead.
There are next assumptions which allows correct operation
1. the position is read by some of the threads more often than is the time
for which position changes by +/- 32k increments for maximal considered speed.
2. no thread (even some very low priority one) stays inside
sam_qe_pos_16to32b() longer than is the time equivalent for +/- 32k position
change
Under these assumptions there is not necessary any additional locking. If
these assumptions are not met then there is problem which can be solved only in
hardware, may be there is some option to cascade counters etc. 32-bit counters
found on some other MCUs are even better. But I actual solution fits our needs,
has minimal performance impact and I expect that it would fit 99+% of needs of
others.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
