You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "slorquet (via GitHub)" <gi...@apache.org> on 2023/06/13 15:32:28 UTC

[GitHub] [nuttx] slorquet opened a new pull request, #9530: Smartcard for stm32h7 uart

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

   ## Summary
   This pull request is intended to show my progress into developing smartcard support for stm32h7.
   It is an experimental draft for comment and review.
   
   ## Testing
   A test app is required to exercise the new ioctls.
   


-- 
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] slorquet commented on a diff in pull request #9530: Smartcard for stm32h7 uart

Posted by "slorquet (via GitHub)" <gi...@apache.org>.
slorquet commented on code in PR #9530:
URL: https://github.com/apache/nuttx/pull/9530#discussion_r1229162693


##########
arch/arm/src/stm32h7/stm32_serial.c:
##########
@@ -2660,6 +2719,201 @@ static int up_ioctl(struct file *filep, int cmd, unsigned long arg)
      break;
 #endif
 
+#ifdef CONFIG_STM32H7_USART_SMARTCARD
+    case TIOCGSMARTCARD:
+      {
+        struct serial_smartcard_s *sc = (struct serial_smartcard_s *)arg;
+
+        /* Determine argument validity */
+
+        if (!sc)
+          {
+            ret = -EINVAL;
+            break;
+          }
+
+        /* Determine if this uart supports smartcard mode */
+
+        if (!priv->sc_supported)
+          {
+            ret = -ENOTTY;
+            break;
+          }
+
+        memcpy(sc, &priv->smartcard, sizeof(struct serial_smartcard_s));
+        break;
+      }
+
+    case TIOCSSMARTCARD:
+      {
+        struct serial_smartcard_s *sc = (struct serial_smartcard_s *)arg;
+        uint32_t cr1;
+        uint32_t cr1_ue;
+        uint32_t cr2;
+        uint32_t cr3;
+        uint32_t gtpr;
+        irqstate_t flags;
+
+        /* Determine argument validity */
+
+        if (!sc)
+          {
+            ret = -EINVAL;
+            break;
+          }
+
+        /* Determine if this uart supports smartcard mode */
+
+        if (!priv->sc_supported)
+          {
+            ret = -ENOTTY;
+            break;
+          }
+
+        memcpy(&priv->smartcard, sc, sizeof(struct serial_smartcard_s));
+
+        flags = enter_critical_section();
+
+        /* Get the original state of UE */
+
+        cr1  = up_serialin(priv, STM32_USART_CR1_OFFSET);
+        cr1_ue = cr1 & USART_CR1_UE;
+        cr1 &= ~USART_CR1_UE;
+
+        /* Disable UE, config can only be written when UE=0 */
+
+        up_serialout(priv, STM32_USART_CR1_OFFSET, cr1);
+
+        /* Read smart card related registers */
+
+        cr2  = up_serialin(priv, STM32_USART_CR2_OFFSET);
+        cr3  = up_serialin(priv, STM32_USART_CR3_OFFSET);
+        gtpr = up_serialin(priv, STM32_USART_GTPR_OFFSET);
+
+        /* Apply settings */
+
+        if (priv->smartcard.enabled)
+          {
+            /* User has requested to enable smart card mode */
+
+            cr3 |= USART_CR3_SCEN | USART_CR3_NACK; /* Always enable NACK */
+
+            /* Smart card mode does not use RX, but uses CLK */
+
+            stm32_unconfiggpio(priv->rx_gpio);
+            if (!priv->ck_gpio)
+              {
+                ret = -EIO;
+                break;
+              }
+
+            stm32_configgpio(priv->ck_gpio);
+
+            /* Note: TX gpio has to be open drain */
+
+            if (priv->smartcard.clken)
+              {
+                /* Enable smart card clock */
+
+                cr2 |=   USART_CR2_CLKEN | USART_CR2_LBCL;
+                cr2 &= ~(USART_CR2_CPOL  | USART_CR2_CPHA);
+              }
+            else
+              {
+                cr2 &= ~USART_CR2_CLKEN; /* Disable smart card clock */
+              }
+
+            if (priv->smartcard.inverse)
+              {
+                /* Enable inverse convention */
+
+                cr2 |= USART_CR2_MSBFIRST | USART_CR2_DATAINV;
+              }
+            else
+              {
+                /* Enable direct convention */
+
+                cr3 &= ~(USART_CR2_MSBFIRST | USART_CR2_DATAINV);
+              }
+
+            if (priv->smartcard.clkout)
+              {
+                /* Compute prescaler to match request,
+                 * then return the actual value.
+                 */
+
+                uint8_t clkdiv = 1;
+                uint32_t divided = priv->apbclock >> 1;
+                while ((divided > priv->smartcard.clkout) && (clkdiv < 31))
+                  {
+                    clkdiv   += 1;
+                    divided >>= 1;
+                  }
+
+                priv->smartcard.clkout = priv->apbclock / (clkdiv << 1);
+                gtpr = (gtpr & ~USART_GTPR_PSC_MASK) |
+                       (clkdiv << USART_GTPR_PSC_SHIFT);
+              }
+
+            if (priv->smartcard.etu)
+              {
+                uint8_t clkdiv;
+                uint32_t etuclock;
+
+                /* Get the current prescaler from gtpr, either the original
+                 * value, or the one that was just updated
+                 */
+
+                clkdiv = (gtpr & USART_GTPR_PSC_MASK) >>
+                         USART_GTPR_PSC_SHIFT;
+                clkdiv <<= 1; /* Actual divider is twice the reg contents */
+
+                /* The refernce clock for the ETU is the divided clock */
+
+                etuclock = priv->apbclock / clkdiv;
+
+                /* Compute baud rate from clock divider and ETU */
+
+                priv->baud      = etuclock / priv->smartcard.etu;
+                priv->bits      = 8;
+                priv->parity    = 2; /* even */
+                priv->stopbits2 = TRUE;
+                up_set_format(dev);
+              }
+
+            if (priv->smartcard.cgt)
+              {
+                gtpr = (gtpr & ~USART_GTPR_GT_MASK) |
+                       (sc->cgt << USART_GTPR_GT_SHIFT);
+              }
+          }
+        else
+          {
+            /* User has requested to disable smart card mode */
+
+            cr3 &= ~USART_CR3_SCEN;
+            cr2 &= ~USART_CR2_CLKEN; /* Disable smart card clock */
+
+            /* Re-enable RX and disable CLK */
+
+            stm32_configgpio  (priv->rx_gpio);

Review Comment:
   Please tell me globally if that kind of alignment within consecutive lines is a thing I should continue or not.
   
   I personally find it has cosmetic value and has no drawbacks, but I can avoid 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] acassis commented on pull request #9530: Smartcard for stm32h7 uart

Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis commented on PR #9530:
URL: https://github.com/apache/nuttx/pull/9530#issuecomment-1593541126

   > @slorquet A few questions:
   > 
   > Is this for ISO7816? Would it be better to make is a separate driver?
   
   @slorquet I think @davids5 suggestion makes sense!
   
   The 1wire implementation for STM32 is also uses the UART peripheral, but it is a separated file:
   arch/arm/src/stm32/stm32_1wire.c
   
   So, this way you could create a drivers/smartcard/iso7816.c that uses this driver as a lowerhalf


-- 
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 #9530: Smartcard for stm32h7 uart

Posted by "pkarashchenko (via GitHub)" <gi...@apache.org>.
pkarashchenko commented on code in PR #9530:
URL: https://github.com/apache/nuttx/pull/9530#discussion_r1228678794


##########
arch/arm/src/stm32h7/stm32_serial.c:
##########
@@ -2660,6 +2719,201 @@ static int up_ioctl(struct file *filep, int cmd, unsigned long arg)
      break;
 #endif
 
+#ifdef CONFIG_STM32H7_USART_SMARTCARD
+    case TIOCGSMARTCARD:
+      {
+        struct serial_smartcard_s *sc = (struct serial_smartcard_s *)arg;
+
+        /* Determine argument validity */
+
+        if (!sc)
+          {
+            ret = -EINVAL;
+            break;
+          }
+
+        /* Determine if this uart supports smartcard mode */
+
+        if (!priv->sc_supported)
+          {
+            ret = -ENOTTY;
+            break;
+          }
+
+        memcpy(sc, &priv->smartcard, sizeof(struct serial_smartcard_s));
+        break;
+      }
+
+    case TIOCSSMARTCARD:
+      {
+        struct serial_smartcard_s *sc = (struct serial_smartcard_s *)arg;
+        uint32_t cr1;
+        uint32_t cr1_ue;
+        uint32_t cr2;
+        uint32_t cr3;
+        uint32_t gtpr;
+        irqstate_t flags;
+
+        /* Determine argument validity */
+
+        if (!sc)
+          {
+            ret = -EINVAL;
+            break;
+          }
+
+        /* Determine if this uart supports smartcard mode */
+
+        if (!priv->sc_supported)
+          {
+            ret = -ENOTTY;
+            break;
+          }
+
+        memcpy(&priv->smartcard, sc, sizeof(struct serial_smartcard_s));
+
+        flags = enter_critical_section();
+
+        /* Get the original state of UE */
+
+        cr1  = up_serialin(priv, STM32_USART_CR1_OFFSET);
+        cr1_ue = cr1 & USART_CR1_UE;
+        cr1 &= ~USART_CR1_UE;
+
+        /* Disable UE, config can only be written when UE=0 */
+
+        up_serialout(priv, STM32_USART_CR1_OFFSET, cr1);
+
+        /* Read smart card related registers */
+
+        cr2  = up_serialin(priv, STM32_USART_CR2_OFFSET);
+        cr3  = up_serialin(priv, STM32_USART_CR3_OFFSET);
+        gtpr = up_serialin(priv, STM32_USART_GTPR_OFFSET);
+
+        /* Apply settings */
+
+        if (priv->smartcard.enabled)
+          {
+            /* User has requested to enable smart card mode */
+
+            cr3 |= USART_CR3_SCEN | USART_CR3_NACK; /* Always enable NACK */
+
+            /* Smart card mode does not use RX, but uses CLK */
+
+            stm32_unconfiggpio(priv->rx_gpio);
+            if (!priv->ck_gpio)
+              {
+                ret = -EIO;
+                break;
+              }
+
+            stm32_configgpio(priv->ck_gpio);
+
+            /* Note: TX gpio has to be open drain */
+
+            if (priv->smartcard.clken)
+              {
+                /* Enable smart card clock */
+
+                cr2 |=   USART_CR2_CLKEN | USART_CR2_LBCL;
+                cr2 &= ~(USART_CR2_CPOL  | USART_CR2_CPHA);
+              }
+            else
+              {
+                cr2 &= ~USART_CR2_CLKEN; /* Disable smart card clock */
+              }
+
+            if (priv->smartcard.inverse)
+              {
+                /* Enable inverse convention */
+
+                cr2 |= USART_CR2_MSBFIRST | USART_CR2_DATAINV;
+              }
+            else
+              {
+                /* Enable direct convention */
+
+                cr3 &= ~(USART_CR2_MSBFIRST | USART_CR2_DATAINV);
+              }
+
+            if (priv->smartcard.clkout)
+              {
+                /* Compute prescaler to match request,
+                 * then return the actual value.
+                 */
+
+                uint8_t clkdiv = 1;
+                uint32_t divided = priv->apbclock >> 1;
+                while ((divided > priv->smartcard.clkout) && (clkdiv < 31))
+                  {
+                    clkdiv   += 1;
+                    divided >>= 1;
+                  }
+
+                priv->smartcard.clkout = priv->apbclock / (clkdiv << 1);
+                gtpr = (gtpr & ~USART_GTPR_PSC_MASK) |
+                       (clkdiv << USART_GTPR_PSC_SHIFT);
+              }
+
+            if (priv->smartcard.etu)
+              {
+                uint8_t clkdiv;
+                uint32_t etuclock;
+
+                /* Get the current prescaler from gtpr, either the original
+                 * value, or the one that was just updated
+                 */
+
+                clkdiv = (gtpr & USART_GTPR_PSC_MASK) >>
+                         USART_GTPR_PSC_SHIFT;
+                clkdiv <<= 1; /* Actual divider is twice the reg contents */
+
+                /* The refernce clock for the ETU is the divided clock */
+
+                etuclock = priv->apbclock / clkdiv;
+
+                /* Compute baud rate from clock divider and ETU */
+
+                priv->baud      = etuclock / priv->smartcard.etu;
+                priv->bits      = 8;
+                priv->parity    = 2; /* even */
+                priv->stopbits2 = TRUE;
+                up_set_format(dev);
+              }
+
+            if (priv->smartcard.cgt)
+              {
+                gtpr = (gtpr & ~USART_GTPR_GT_MASK) |
+                       (sc->cgt << USART_GTPR_GT_SHIFT);
+              }
+          }
+        else
+          {
+            /* User has requested to disable smart card mode */
+
+            cr3 &= ~USART_CR3_SCEN;
+            cr2 &= ~USART_CR2_CLKEN; /* Disable smart card clock */
+
+            /* Re-enable RX and disable CLK */
+
+            stm32_configgpio  (priv->rx_gpio);

Review Comment:
   ```suggestion
               stm32_configgpio(priv->rx_gpio);
   ```



##########
arch/arm/src/stm32h7/stm32_serial.c:
##########
@@ -2660,6 +2719,201 @@ static int up_ioctl(struct file *filep, int cmd, unsigned long arg)
      break;
 #endif
 
+#ifdef CONFIG_STM32H7_USART_SMARTCARD
+    case TIOCGSMARTCARD:
+      {
+        struct serial_smartcard_s *sc = (struct serial_smartcard_s *)arg;
+
+        /* Determine argument validity */
+
+        if (!sc)
+          {
+            ret = -EINVAL;
+            break;
+          }
+
+        /* Determine if this uart supports smartcard mode */
+
+        if (!priv->sc_supported)
+          {
+            ret = -ENOTTY;
+            break;
+          }
+
+        memcpy(sc, &priv->smartcard, sizeof(struct serial_smartcard_s));
+        break;
+      }
+
+    case TIOCSSMARTCARD:
+      {
+        struct serial_smartcard_s *sc = (struct serial_smartcard_s *)arg;
+        uint32_t cr1;
+        uint32_t cr1_ue;
+        uint32_t cr2;
+        uint32_t cr3;
+        uint32_t gtpr;
+        irqstate_t flags;
+
+        /* Determine argument validity */
+
+        if (!sc)
+          {
+            ret = -EINVAL;
+            break;
+          }
+
+        /* Determine if this uart supports smartcard mode */
+
+        if (!priv->sc_supported)
+          {
+            ret = -ENOTTY;
+            break;
+          }
+
+        memcpy(&priv->smartcard, sc, sizeof(struct serial_smartcard_s));
+
+        flags = enter_critical_section();
+
+        /* Get the original state of UE */
+
+        cr1  = up_serialin(priv, STM32_USART_CR1_OFFSET);
+        cr1_ue = cr1 & USART_CR1_UE;
+        cr1 &= ~USART_CR1_UE;
+
+        /* Disable UE, config can only be written when UE=0 */
+
+        up_serialout(priv, STM32_USART_CR1_OFFSET, cr1);
+
+        /* Read smart card related registers */
+
+        cr2  = up_serialin(priv, STM32_USART_CR2_OFFSET);
+        cr3  = up_serialin(priv, STM32_USART_CR3_OFFSET);
+        gtpr = up_serialin(priv, STM32_USART_GTPR_OFFSET);
+
+        /* Apply settings */
+
+        if (priv->smartcard.enabled)
+          {
+            /* User has requested to enable smart card mode */
+
+            cr3 |= USART_CR3_SCEN | USART_CR3_NACK; /* Always enable NACK */
+
+            /* Smart card mode does not use RX, but uses CLK */
+
+            stm32_unconfiggpio(priv->rx_gpio);
+            if (!priv->ck_gpio)
+              {
+                ret = -EIO;
+                break;
+              }
+
+            stm32_configgpio(priv->ck_gpio);
+
+            /* Note: TX gpio has to be open drain */
+
+            if (priv->smartcard.clken)
+              {
+                /* Enable smart card clock */
+
+                cr2 |=   USART_CR2_CLKEN | USART_CR2_LBCL;
+                cr2 &= ~(USART_CR2_CPOL  | USART_CR2_CPHA);
+              }
+            else
+              {
+                cr2 &= ~USART_CR2_CLKEN; /* Disable smart card clock */
+              }
+
+            if (priv->smartcard.inverse)
+              {
+                /* Enable inverse convention */
+
+                cr2 |= USART_CR2_MSBFIRST | USART_CR2_DATAINV;
+              }
+            else
+              {
+                /* Enable direct convention */
+
+                cr3 &= ~(USART_CR2_MSBFIRST | USART_CR2_DATAINV);
+              }
+
+            if (priv->smartcard.clkout)
+              {
+                /* Compute prescaler to match request,
+                 * then return the actual value.
+                 */
+
+                uint8_t clkdiv = 1;
+                uint32_t divided = priv->apbclock >> 1;
+                while ((divided > priv->smartcard.clkout) && (clkdiv < 31))
+                  {
+                    clkdiv   += 1;

Review Comment:
   ```suggestion
                       clkdiv++;
   ```



##########
arch/arm/src/stm32h7/stm32_serial.c:
##########
@@ -2660,6 +2719,201 @@ static int up_ioctl(struct file *filep, int cmd, unsigned long arg)
      break;
 #endif
 
+#ifdef CONFIG_STM32H7_USART_SMARTCARD
+    case TIOCGSMARTCARD:
+      {
+        struct serial_smartcard_s *sc = (struct serial_smartcard_s *)arg;
+
+        /* Determine argument validity */
+
+        if (!sc)
+          {
+            ret = -EINVAL;
+            break;
+          }
+
+        /* Determine if this uart supports smartcard mode */
+
+        if (!priv->sc_supported)
+          {
+            ret = -ENOTTY;
+            break;
+          }
+
+        memcpy(sc, &priv->smartcard, sizeof(struct serial_smartcard_s));
+        break;
+      }
+
+    case TIOCSSMARTCARD:
+      {
+        struct serial_smartcard_s *sc = (struct serial_smartcard_s *)arg;
+        uint32_t cr1;
+        uint32_t cr1_ue;
+        uint32_t cr2;
+        uint32_t cr3;
+        uint32_t gtpr;
+        irqstate_t flags;
+
+        /* Determine argument validity */
+
+        if (!sc)
+          {
+            ret = -EINVAL;
+            break;
+          }
+
+        /* Determine if this uart supports smartcard mode */
+
+        if (!priv->sc_supported)
+          {
+            ret = -ENOTTY;
+            break;
+          }
+
+        memcpy(&priv->smartcard, sc, sizeof(struct serial_smartcard_s));
+
+        flags = enter_critical_section();
+
+        /* Get the original state of UE */
+
+        cr1  = up_serialin(priv, STM32_USART_CR1_OFFSET);
+        cr1_ue = cr1 & USART_CR1_UE;
+        cr1 &= ~USART_CR1_UE;

Review Comment:
   ```suggestion
           cr1    = up_serialin(priv, STM32_USART_CR1_OFFSET);
           cr1_ue = cr1 & USART_CR1_UE;
           cr1   &= ~USART_CR1_UE;
   ```



##########
arch/arm/src/stm32h7/stm32_serial.c:
##########
@@ -2660,6 +2719,201 @@ static int up_ioctl(struct file *filep, int cmd, unsigned long arg)
      break;
 #endif
 
+#ifdef CONFIG_STM32H7_USART_SMARTCARD
+    case TIOCGSMARTCARD:
+      {
+        struct serial_smartcard_s *sc = (struct serial_smartcard_s *)arg;
+
+        /* Determine argument validity */
+
+        if (!sc)
+          {
+            ret = -EINVAL;
+            break;
+          }
+
+        /* Determine if this uart supports smartcard mode */
+
+        if (!priv->sc_supported)
+          {
+            ret = -ENOTTY;
+            break;
+          }
+
+        memcpy(sc, &priv->smartcard, sizeof(struct serial_smartcard_s));
+        break;
+      }
+
+    case TIOCSSMARTCARD:
+      {
+        struct serial_smartcard_s *sc = (struct serial_smartcard_s *)arg;
+        uint32_t cr1;
+        uint32_t cr1_ue;
+        uint32_t cr2;
+        uint32_t cr3;
+        uint32_t gtpr;
+        irqstate_t flags;
+
+        /* Determine argument validity */
+
+        if (!sc)
+          {
+            ret = -EINVAL;
+            break;
+          }
+
+        /* Determine if this uart supports smartcard mode */
+
+        if (!priv->sc_supported)
+          {
+            ret = -ENOTTY;
+            break;
+          }
+
+        memcpy(&priv->smartcard, sc, sizeof(struct serial_smartcard_s));
+
+        flags = enter_critical_section();
+
+        /* Get the original state of UE */
+
+        cr1  = up_serialin(priv, STM32_USART_CR1_OFFSET);
+        cr1_ue = cr1 & USART_CR1_UE;
+        cr1 &= ~USART_CR1_UE;
+
+        /* Disable UE, config can only be written when UE=0 */
+
+        up_serialout(priv, STM32_USART_CR1_OFFSET, cr1);
+
+        /* Read smart card related registers */
+
+        cr2  = up_serialin(priv, STM32_USART_CR2_OFFSET);
+        cr3  = up_serialin(priv, STM32_USART_CR3_OFFSET);
+        gtpr = up_serialin(priv, STM32_USART_GTPR_OFFSET);
+
+        /* Apply settings */
+
+        if (priv->smartcard.enabled)
+          {
+            /* User has requested to enable smart card mode */
+
+            cr3 |= USART_CR3_SCEN | USART_CR3_NACK; /* Always enable NACK */
+
+            /* Smart card mode does not use RX, but uses CLK */
+
+            stm32_unconfiggpio(priv->rx_gpio);
+            if (!priv->ck_gpio)
+              {
+                ret = -EIO;
+                break;
+              }
+
+            stm32_configgpio(priv->ck_gpio);
+
+            /* Note: TX gpio has to be open drain */
+
+            if (priv->smartcard.clken)
+              {
+                /* Enable smart card clock */
+
+                cr2 |=   USART_CR2_CLKEN | USART_CR2_LBCL;
+                cr2 &= ~(USART_CR2_CPOL  | USART_CR2_CPHA);
+              }
+            else
+              {
+                cr2 &= ~USART_CR2_CLKEN; /* Disable smart card clock */
+              }
+
+            if (priv->smartcard.inverse)
+              {
+                /* Enable inverse convention */
+
+                cr2 |= USART_CR2_MSBFIRST | USART_CR2_DATAINV;
+              }
+            else
+              {
+                /* Enable direct convention */
+
+                cr3 &= ~(USART_CR2_MSBFIRST | USART_CR2_DATAINV);
+              }
+
+            if (priv->smartcard.clkout)
+              {
+                /* Compute prescaler to match request,
+                 * then return the actual value.
+                 */
+
+                uint8_t clkdiv = 1;
+                uint32_t divided = priv->apbclock >> 1;
+                while ((divided > priv->smartcard.clkout) && (clkdiv < 31))
+                  {
+                    clkdiv   += 1;
+                    divided >>= 1;
+                  }
+
+                priv->smartcard.clkout = priv->apbclock / (clkdiv << 1);
+                gtpr = (gtpr & ~USART_GTPR_PSC_MASK) |
+                       (clkdiv << USART_GTPR_PSC_SHIFT);
+              }
+
+            if (priv->smartcard.etu)
+              {
+                uint8_t clkdiv;
+                uint32_t etuclock;
+
+                /* Get the current prescaler from gtpr, either the original
+                 * value, or the one that was just updated
+                 */
+
+                clkdiv = (gtpr & USART_GTPR_PSC_MASK) >>
+                         USART_GTPR_PSC_SHIFT;
+                clkdiv <<= 1; /* Actual divider is twice the reg contents */
+
+                /* The refernce clock for the ETU is the divided clock */
+
+                etuclock = priv->apbclock / clkdiv;
+
+                /* Compute baud rate from clock divider and ETU */
+
+                priv->baud      = etuclock / priv->smartcard.etu;
+                priv->bits      = 8;
+                priv->parity    = 2; /* even */
+                priv->stopbits2 = TRUE;

Review Comment:
   ```suggestion
                   priv->stopbits2 = true;
   ```



##########
arch/arm/src/stm32h7/stm32_serial.c:
##########
@@ -2660,6 +2719,201 @@ static int up_ioctl(struct file *filep, int cmd, unsigned long arg)
      break;
 #endif
 
+#ifdef CONFIG_STM32H7_USART_SMARTCARD
+    case TIOCGSMARTCARD:
+      {
+        struct serial_smartcard_s *sc = (struct serial_smartcard_s *)arg;
+
+        /* Determine argument validity */
+
+        if (!sc)
+          {
+            ret = -EINVAL;
+            break;
+          }
+
+        /* Determine if this uart supports smartcard mode */
+
+        if (!priv->sc_supported)
+          {
+            ret = -ENOTTY;
+            break;
+          }
+
+        memcpy(sc, &priv->smartcard, sizeof(struct serial_smartcard_s));
+        break;
+      }
+
+    case TIOCSSMARTCARD:
+      {
+        struct serial_smartcard_s *sc = (struct serial_smartcard_s *)arg;
+        uint32_t cr1;
+        uint32_t cr1_ue;
+        uint32_t cr2;
+        uint32_t cr3;
+        uint32_t gtpr;
+        irqstate_t flags;
+
+        /* Determine argument validity */
+
+        if (!sc)
+          {
+            ret = -EINVAL;
+            break;
+          }
+
+        /* Determine if this uart supports smartcard mode */
+
+        if (!priv->sc_supported)
+          {
+            ret = -ENOTTY;
+            break;
+          }
+
+        memcpy(&priv->smartcard, sc, sizeof(struct serial_smartcard_s));
+
+        flags = enter_critical_section();
+
+        /* Get the original state of UE */
+
+        cr1  = up_serialin(priv, STM32_USART_CR1_OFFSET);
+        cr1_ue = cr1 & USART_CR1_UE;
+        cr1 &= ~USART_CR1_UE;
+
+        /* Disable UE, config can only be written when UE=0 */
+
+        up_serialout(priv, STM32_USART_CR1_OFFSET, cr1);
+
+        /* Read smart card related registers */
+
+        cr2  = up_serialin(priv, STM32_USART_CR2_OFFSET);
+        cr3  = up_serialin(priv, STM32_USART_CR3_OFFSET);
+        gtpr = up_serialin(priv, STM32_USART_GTPR_OFFSET);
+
+        /* Apply settings */
+
+        if (priv->smartcard.enabled)
+          {
+            /* User has requested to enable smart card mode */
+
+            cr3 |= USART_CR3_SCEN | USART_CR3_NACK; /* Always enable NACK */
+
+            /* Smart card mode does not use RX, but uses CLK */
+
+            stm32_unconfiggpio(priv->rx_gpio);
+            if (!priv->ck_gpio)
+              {
+                ret = -EIO;
+                break;
+              }
+
+            stm32_configgpio(priv->ck_gpio);
+
+            /* Note: TX gpio has to be open drain */
+
+            if (priv->smartcard.clken)
+              {
+                /* Enable smart card clock */
+
+                cr2 |=   USART_CR2_CLKEN | USART_CR2_LBCL;
+                cr2 &= ~(USART_CR2_CPOL  | USART_CR2_CPHA);
+              }
+            else
+              {
+                cr2 &= ~USART_CR2_CLKEN; /* Disable smart card clock */
+              }
+
+            if (priv->smartcard.inverse)
+              {
+                /* Enable inverse convention */
+
+                cr2 |= USART_CR2_MSBFIRST | USART_CR2_DATAINV;
+              }
+            else
+              {
+                /* Enable direct convention */
+
+                cr3 &= ~(USART_CR2_MSBFIRST | USART_CR2_DATAINV);
+              }
+
+            if (priv->smartcard.clkout)
+              {
+                /* Compute prescaler to match request,
+                 * then return the actual value.
+                 */
+
+                uint8_t clkdiv = 1;
+                uint32_t divided = priv->apbclock >> 1;
+                while ((divided > priv->smartcard.clkout) && (clkdiv < 31))
+                  {
+                    clkdiv   += 1;
+                    divided >>= 1;
+                  }
+
+                priv->smartcard.clkout = priv->apbclock / (clkdiv << 1);
+                gtpr = (gtpr & ~USART_GTPR_PSC_MASK) |
+                       (clkdiv << USART_GTPR_PSC_SHIFT);
+              }
+
+            if (priv->smartcard.etu)
+              {
+                uint8_t clkdiv;
+                uint32_t etuclock;
+
+                /* Get the current prescaler from gtpr, either the original
+                 * value, or the one that was just updated
+                 */
+
+                clkdiv = (gtpr & USART_GTPR_PSC_MASK) >>
+                         USART_GTPR_PSC_SHIFT;
+                clkdiv <<= 1; /* Actual divider is twice the reg contents */
+
+                /* The refernce clock for the ETU is the divided clock */
+
+                etuclock = priv->apbclock / clkdiv;
+
+                /* Compute baud rate from clock divider and ETU */
+
+                priv->baud      = etuclock / priv->smartcard.etu;
+                priv->bits      = 8;
+                priv->parity    = 2; /* even */
+                priv->stopbits2 = TRUE;
+                up_set_format(dev);
+              }
+
+            if (priv->smartcard.cgt)
+              {
+                gtpr = (gtpr & ~USART_GTPR_GT_MASK) |
+                       (sc->cgt << USART_GTPR_GT_SHIFT);
+              }
+          }
+        else
+          {
+            /* User has requested to disable smart card mode */
+
+            cr3 &= ~USART_CR3_SCEN;
+            cr2 &= ~USART_CR2_CLKEN; /* Disable smart card clock */
+
+            /* Re-enable RX and disable CLK */
+
+            stm32_configgpio  (priv->rx_gpio);
+            stm32_unconfiggpio(priv->ck_gpio);
+          }
+
+        /* Apply new settings */
+
+        up_serialout(priv, STM32_USART_CR3_OFFSET , cr2);
+        up_serialout(priv, STM32_USART_CR3_OFFSET , cr3);

Review Comment:
   ```suggestion
           up_serialout(priv, STM32_USART_CR3_OFFSET, cr2);
           up_serialout(priv, STM32_USART_CR3_OFFSET, cr3);
   ```



-- 
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] slorquet commented on pull request #9530: Smartcard for stm32h7 uart

Posted by "slorquet (via GitHub)" <gi...@apache.org>.
slorquet commented on PR #9530:
URL: https://github.com/apache/nuttx/pull/9530#issuecomment-1590666855

   Thanks @pkarashchenko for the extensive proofreading. I'm waiting more confirmation for the alignments.
   
   @davids5 : Yes, it implements ISO7816, but the framework this goes into is PC/SC, not just ISO7816.
   And no, it should not be a separate driver at this point.
   
   This is merely ISO7816 support for the stm32 uart, just like the uart also has LIN SPI or RS485 modes. This is extremely hardware specific, and its just a low level driver.
   
   I want to make it look like PC/SC since this is the de-facto standard for smart cards in C, already implemented in Windows and Linux. It has real value to help people port smart card programs.
   
   In PC/SC, you can have many types of drivers. This is usually handled by the CCID driver, which describes a way to access USB readers. But CCID also support serial attached readers, and proprietary drivers could be implemented in any way possible.
   
   These drivers are then managed by userspace PC/SC daemon that handles access to drivers from application programs.
   
   so to sum up, smart cards are not accessed as devices from user space, but only through a management daemon that hides the details of actual reader access and implementation.
   
   NuttX already has support for usb host, so it could support USB smart card readers via ccid. Thats why I'm keeping this architecture, so USB readers can be handled by the PC/SC daemon in the future. This means the stm 32 uart side of things does not need to have an easy to use user interface.
   
   It's a long text so I can add precisions and details to make it more clear.


-- 
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] slorquet commented on pull request #9530: Smartcard for stm32h7 uart

Posted by "slorquet (via GitHub)" <gi...@apache.org>.
slorquet commented on PR #9530:
URL: https://github.com/apache/nuttx/pull/9530#issuecomment-1596779785

   Yes, I am worrying that it will be a lot more validation work.
   
   These days I'm back to this development. I still don't get clock output from the clock pin.
   
   (I am using a small trivial test app to exercise this driver)


-- 
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] slorquet commented on a diff in pull request #9530: Smartcard for stm32h7 uart

Posted by "slorquet (via GitHub)" <gi...@apache.org>.
slorquet commented on code in PR #9530:
URL: https://github.com/apache/nuttx/pull/9530#discussion_r1229160968


##########
arch/arm/src/stm32h7/stm32_serial.c:
##########
@@ -2660,6 +2719,201 @@ static int up_ioctl(struct file *filep, int cmd, unsigned long arg)
      break;
 #endif
 
+#ifdef CONFIG_STM32H7_USART_SMARTCARD
+    case TIOCGSMARTCARD:
+      {
+        struct serial_smartcard_s *sc = (struct serial_smartcard_s *)arg;
+
+        /* Determine argument validity */
+
+        if (!sc)
+          {
+            ret = -EINVAL;
+            break;
+          }
+
+        /* Determine if this uart supports smartcard mode */
+
+        if (!priv->sc_supported)
+          {
+            ret = -ENOTTY;
+            break;
+          }
+
+        memcpy(sc, &priv->smartcard, sizeof(struct serial_smartcard_s));
+        break;
+      }
+
+    case TIOCSSMARTCARD:
+      {
+        struct serial_smartcard_s *sc = (struct serial_smartcard_s *)arg;
+        uint32_t cr1;
+        uint32_t cr1_ue;
+        uint32_t cr2;
+        uint32_t cr3;
+        uint32_t gtpr;
+        irqstate_t flags;
+
+        /* Determine argument validity */
+
+        if (!sc)
+          {
+            ret = -EINVAL;
+            break;
+          }
+
+        /* Determine if this uart supports smartcard mode */
+
+        if (!priv->sc_supported)
+          {
+            ret = -ENOTTY;
+            break;
+          }
+
+        memcpy(&priv->smartcard, sc, sizeof(struct serial_smartcard_s));
+
+        flags = enter_critical_section();
+
+        /* Get the original state of UE */
+
+        cr1  = up_serialin(priv, STM32_USART_CR1_OFFSET);
+        cr1_ue = cr1 & USART_CR1_UE;
+        cr1 &= ~USART_CR1_UE;
+
+        /* Disable UE, config can only be written when UE=0 */
+
+        up_serialout(priv, STM32_USART_CR1_OFFSET, cr1);
+
+        /* Read smart card related registers */
+
+        cr2  = up_serialin(priv, STM32_USART_CR2_OFFSET);
+        cr3  = up_serialin(priv, STM32_USART_CR3_OFFSET);
+        gtpr = up_serialin(priv, STM32_USART_GTPR_OFFSET);
+
+        /* Apply settings */
+
+        if (priv->smartcard.enabled)
+          {
+            /* User has requested to enable smart card mode */
+
+            cr3 |= USART_CR3_SCEN | USART_CR3_NACK; /* Always enable NACK */
+
+            /* Smart card mode does not use RX, but uses CLK */
+
+            stm32_unconfiggpio(priv->rx_gpio);
+            if (!priv->ck_gpio)
+              {
+                ret = -EIO;
+                break;
+              }
+
+            stm32_configgpio(priv->ck_gpio);
+
+            /* Note: TX gpio has to be open drain */
+
+            if (priv->smartcard.clken)
+              {
+                /* Enable smart card clock */
+
+                cr2 |=   USART_CR2_CLKEN | USART_CR2_LBCL;
+                cr2 &= ~(USART_CR2_CPOL  | USART_CR2_CPHA);
+              }
+            else
+              {
+                cr2 &= ~USART_CR2_CLKEN; /* Disable smart card clock */
+              }
+
+            if (priv->smartcard.inverse)
+              {
+                /* Enable inverse convention */
+
+                cr2 |= USART_CR2_MSBFIRST | USART_CR2_DATAINV;
+              }
+            else
+              {
+                /* Enable direct convention */
+
+                cr3 &= ~(USART_CR2_MSBFIRST | USART_CR2_DATAINV);
+              }
+
+            if (priv->smartcard.clkout)
+              {
+                /* Compute prescaler to match request,
+                 * then return the actual value.
+                 */
+
+                uint8_t clkdiv = 1;
+                uint32_t divided = priv->apbclock >> 1;
+                while ((divided > priv->smartcard.clkout) && (clkdiv < 31))
+                  {
+                    clkdiv   += 1;
+                    divided >>= 1;
+                  }
+
+                priv->smartcard.clkout = priv->apbclock / (clkdiv << 1);
+                gtpr = (gtpr & ~USART_GTPR_PSC_MASK) |
+                       (clkdiv << USART_GTPR_PSC_SHIFT);
+              }
+
+            if (priv->smartcard.etu)
+              {
+                uint8_t clkdiv;
+                uint32_t etuclock;
+
+                /* Get the current prescaler from gtpr, either the original
+                 * value, or the one that was just updated
+                 */
+
+                clkdiv = (gtpr & USART_GTPR_PSC_MASK) >>
+                         USART_GTPR_PSC_SHIFT;
+                clkdiv <<= 1; /* Actual divider is twice the reg contents */
+
+                /* The refernce clock for the ETU is the divided clock */
+
+                etuclock = priv->apbclock / clkdiv;
+
+                /* Compute baud rate from clock divider and ETU */
+
+                priv->baud      = etuclock / priv->smartcard.etu;
+                priv->bits      = 8;
+                priv->parity    = 2; /* even */
+                priv->stopbits2 = TRUE;
+                up_set_format(dev);
+              }
+
+            if (priv->smartcard.cgt)
+              {
+                gtpr = (gtpr & ~USART_GTPR_GT_MASK) |
+                       (sc->cgt << USART_GTPR_GT_SHIFT);
+              }
+          }
+        else
+          {
+            /* User has requested to disable smart card mode */
+
+            cr3 &= ~USART_CR3_SCEN;
+            cr2 &= ~USART_CR2_CLKEN; /* Disable smart card clock */
+
+            /* Re-enable RX and disable CLK */
+
+            stm32_configgpio  (priv->rx_gpio);
+            stm32_unconfiggpio(priv->ck_gpio);
+          }
+
+        /* Apply new settings */
+
+        up_serialout(priv, STM32_USART_CR3_OFFSET , cr2);
+        up_serialout(priv, STM32_USART_CR3_OFFSET , cr3);

Review Comment:
   The goal was to ensure alignment of the commas. Is that kind of alignment not important for readability? Checkpatch did not notify me about 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] slorquet commented on a diff in pull request #9530: Smartcard for stm32h7 uart

Posted by "slorquet (via GitHub)" <gi...@apache.org>.
slorquet commented on code in PR #9530:
URL: https://github.com/apache/nuttx/pull/9530#discussion_r1229159287


##########
include/nuttx/serial/tioctl.h:
##########
@@ -202,6 +202,23 @@
 
 #define TIOCSLINID      _TIOC(0x0037) /* Master send one LIN header with specified LIN identifier: uint8_t */
 
+/* Smart Card Support */
+
+#define TIOCSSMARTCARD  _TIOC(0x0038) /* Set smart card mode */
+#define TIOCGSMARTCARD  _TIOC(0x0039) /* Get Smart card mode */
+
+struct serial_smartcard_s
+  {
+    uint8_t enabled : 1; /* Enable smart card mode */
+    uint8_t clken   : 1; /* Enable smart card clock */
+    uint8_t inverse : 1; /* Use inverse convention */
+    uint32_t clkout;     /* Clock output frequency requested,
+                          * updated with actual value, which is the largest
+                          * value inferior to the request */
+    uint16_t etu;        /* Elementary time unit, eg baud rate in clock counts */
+    uint8_t  cgt;        /* Character guard time in ETU units */
+  };

Review Comment:
   done, thanks



-- 
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] slorquet commented on a diff in pull request #9530: Smartcard for stm32h7 uart

Posted by "slorquet (via GitHub)" <gi...@apache.org>.
slorquet commented on code in PR #9530:
URL: https://github.com/apache/nuttx/pull/9530#discussion_r1229164233


##########
arch/arm/src/stm32h7/stm32_serial.c:
##########
@@ -2660,6 +2719,201 @@ static int up_ioctl(struct file *filep, int cmd, unsigned long arg)
      break;
 #endif
 
+#ifdef CONFIG_STM32H7_USART_SMARTCARD
+    case TIOCGSMARTCARD:
+      {
+        struct serial_smartcard_s *sc = (struct serial_smartcard_s *)arg;
+
+        /* Determine argument validity */
+
+        if (!sc)
+          {
+            ret = -EINVAL;
+            break;
+          }
+
+        /* Determine if this uart supports smartcard mode */
+
+        if (!priv->sc_supported)
+          {
+            ret = -ENOTTY;
+            break;
+          }
+
+        memcpy(sc, &priv->smartcard, sizeof(struct serial_smartcard_s));
+        break;
+      }
+
+    case TIOCSSMARTCARD:
+      {
+        struct serial_smartcard_s *sc = (struct serial_smartcard_s *)arg;
+        uint32_t cr1;
+        uint32_t cr1_ue;
+        uint32_t cr2;
+        uint32_t cr3;
+        uint32_t gtpr;
+        irqstate_t flags;
+
+        /* Determine argument validity */
+
+        if (!sc)
+          {
+            ret = -EINVAL;
+            break;
+          }
+
+        /* Determine if this uart supports smartcard mode */
+
+        if (!priv->sc_supported)
+          {
+            ret = -ENOTTY;
+            break;
+          }
+
+        memcpy(&priv->smartcard, sc, sizeof(struct serial_smartcard_s));
+
+        flags = enter_critical_section();
+
+        /* Get the original state of UE */
+
+        cr1  = up_serialin(priv, STM32_USART_CR1_OFFSET);
+        cr1_ue = cr1 & USART_CR1_UE;
+        cr1 &= ~USART_CR1_UE;
+
+        /* Disable UE, config can only be written when UE=0 */
+
+        up_serialout(priv, STM32_USART_CR1_OFFSET, cr1);
+
+        /* Read smart card related registers */
+
+        cr2  = up_serialin(priv, STM32_USART_CR2_OFFSET);
+        cr3  = up_serialin(priv, STM32_USART_CR3_OFFSET);
+        gtpr = up_serialin(priv, STM32_USART_GTPR_OFFSET);
+
+        /* Apply settings */
+
+        if (priv->smartcard.enabled)
+          {
+            /* User has requested to enable smart card mode */
+
+            cr3 |= USART_CR3_SCEN | USART_CR3_NACK; /* Always enable NACK */
+
+            /* Smart card mode does not use RX, but uses CLK */
+
+            stm32_unconfiggpio(priv->rx_gpio);
+            if (!priv->ck_gpio)
+              {
+                ret = -EIO;
+                break;
+              }
+
+            stm32_configgpio(priv->ck_gpio);
+
+            /* Note: TX gpio has to be open drain */
+
+            if (priv->smartcard.clken)
+              {
+                /* Enable smart card clock */
+
+                cr2 |=   USART_CR2_CLKEN | USART_CR2_LBCL;
+                cr2 &= ~(USART_CR2_CPOL  | USART_CR2_CPHA);
+              }
+            else
+              {
+                cr2 &= ~USART_CR2_CLKEN; /* Disable smart card clock */
+              }
+
+            if (priv->smartcard.inverse)
+              {
+                /* Enable inverse convention */
+
+                cr2 |= USART_CR2_MSBFIRST | USART_CR2_DATAINV;
+              }
+            else
+              {
+                /* Enable direct convention */
+
+                cr3 &= ~(USART_CR2_MSBFIRST | USART_CR2_DATAINV);
+              }
+
+            if (priv->smartcard.clkout)
+              {
+                /* Compute prescaler to match request,
+                 * then return the actual value.
+                 */
+
+                uint8_t clkdiv = 1;
+                uint32_t divided = priv->apbclock >> 1;
+                while ((divided > priv->smartcard.clkout) && (clkdiv < 31))
+                  {
+                    clkdiv   += 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] slorquet commented on a diff in pull request #9530: Smartcard for stm32h7 uart

Posted by "slorquet (via GitHub)" <gi...@apache.org>.
slorquet commented on code in PR #9530:
URL: https://github.com/apache/nuttx/pull/9530#discussion_r1229164897


##########
arch/arm/src/stm32h7/stm32_serial.c:
##########
@@ -2660,6 +2719,201 @@ static int up_ioctl(struct file *filep, int cmd, unsigned long arg)
      break;
 #endif
 
+#ifdef CONFIG_STM32H7_USART_SMARTCARD
+    case TIOCGSMARTCARD:
+      {
+        struct serial_smartcard_s *sc = (struct serial_smartcard_s *)arg;
+
+        /* Determine argument validity */
+
+        if (!sc)
+          {
+            ret = -EINVAL;
+            break;
+          }
+
+        /* Determine if this uart supports smartcard mode */
+
+        if (!priv->sc_supported)
+          {
+            ret = -ENOTTY;
+            break;
+          }
+
+        memcpy(sc, &priv->smartcard, sizeof(struct serial_smartcard_s));
+        break;
+      }
+
+    case TIOCSSMARTCARD:
+      {
+        struct serial_smartcard_s *sc = (struct serial_smartcard_s *)arg;
+        uint32_t cr1;
+        uint32_t cr1_ue;
+        uint32_t cr2;
+        uint32_t cr3;
+        uint32_t gtpr;
+        irqstate_t flags;
+
+        /* Determine argument validity */
+
+        if (!sc)
+          {
+            ret = -EINVAL;
+            break;
+          }
+
+        /* Determine if this uart supports smartcard mode */
+
+        if (!priv->sc_supported)
+          {
+            ret = -ENOTTY;
+            break;
+          }
+
+        memcpy(&priv->smartcard, sc, sizeof(struct serial_smartcard_s));
+
+        flags = enter_critical_section();
+
+        /* Get the original state of UE */
+
+        cr1  = up_serialin(priv, STM32_USART_CR1_OFFSET);
+        cr1_ue = cr1 & USART_CR1_UE;
+        cr1 &= ~USART_CR1_UE;

Review Comment:
   same cosmetic alignment issue, I'm waiting feedback from the project before I update this globally.



-- 
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 #9530: Smartcard for stm32h7 uart

Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis commented on code in PR #9530:
URL: https://github.com/apache/nuttx/pull/9530#discussion_r1228585795


##########
include/nuttx/serial/tioctl.h:
##########
@@ -202,6 +202,23 @@
 
 #define TIOCSLINID      _TIOC(0x0037) /* Master send one LIN header with specified LIN identifier: uint8_t */
 
+/* Smart Card Support */
+
+#define TIOCSSMARTCARD  _TIOC(0x0038) /* Set smart card mode */
+#define TIOCGSMARTCARD  _TIOC(0x0039) /* Get Smart card mode */
+
+struct serial_smartcard_s
+  {
+    uint8_t enabled : 1; /* Enable smart card mode */
+    uint8_t clken   : 1; /* Enable smart card clock */
+    uint8_t inverse : 1; /* Use inverse convention */
+    uint32_t clkout;     /* Clock output frequency requested,
+                          * updated with actual value, which is the largest
+                          * value inferior to the request */
+    uint16_t etu;        /* Elementary time unit, eg baud rate in clock counts */
+    uint8_t  cgt;        /* Character guard time in ETU units */
+  };

Review Comment:
   Please align { and } to start of the line, see "struct winsize" below for reference!



-- 
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] slorquet commented on a diff in pull request #9530: Smartcard for stm32h7 uart

Posted by "slorquet (via GitHub)" <gi...@apache.org>.
slorquet commented on code in PR #9530:
URL: https://github.com/apache/nuttx/pull/9530#discussion_r1229162693


##########
arch/arm/src/stm32h7/stm32_serial.c:
##########
@@ -2660,6 +2719,201 @@ static int up_ioctl(struct file *filep, int cmd, unsigned long arg)
      break;
 #endif
 
+#ifdef CONFIG_STM32H7_USART_SMARTCARD
+    case TIOCGSMARTCARD:
+      {
+        struct serial_smartcard_s *sc = (struct serial_smartcard_s *)arg;
+
+        /* Determine argument validity */
+
+        if (!sc)
+          {
+            ret = -EINVAL;
+            break;
+          }
+
+        /* Determine if this uart supports smartcard mode */
+
+        if (!priv->sc_supported)
+          {
+            ret = -ENOTTY;
+            break;
+          }
+
+        memcpy(sc, &priv->smartcard, sizeof(struct serial_smartcard_s));
+        break;
+      }
+
+    case TIOCSSMARTCARD:
+      {
+        struct serial_smartcard_s *sc = (struct serial_smartcard_s *)arg;
+        uint32_t cr1;
+        uint32_t cr1_ue;
+        uint32_t cr2;
+        uint32_t cr3;
+        uint32_t gtpr;
+        irqstate_t flags;
+
+        /* Determine argument validity */
+
+        if (!sc)
+          {
+            ret = -EINVAL;
+            break;
+          }
+
+        /* Determine if this uart supports smartcard mode */
+
+        if (!priv->sc_supported)
+          {
+            ret = -ENOTTY;
+            break;
+          }
+
+        memcpy(&priv->smartcard, sc, sizeof(struct serial_smartcard_s));
+
+        flags = enter_critical_section();
+
+        /* Get the original state of UE */
+
+        cr1  = up_serialin(priv, STM32_USART_CR1_OFFSET);
+        cr1_ue = cr1 & USART_CR1_UE;
+        cr1 &= ~USART_CR1_UE;
+
+        /* Disable UE, config can only be written when UE=0 */
+
+        up_serialout(priv, STM32_USART_CR1_OFFSET, cr1);
+
+        /* Read smart card related registers */
+
+        cr2  = up_serialin(priv, STM32_USART_CR2_OFFSET);
+        cr3  = up_serialin(priv, STM32_USART_CR3_OFFSET);
+        gtpr = up_serialin(priv, STM32_USART_GTPR_OFFSET);
+
+        /* Apply settings */
+
+        if (priv->smartcard.enabled)
+          {
+            /* User has requested to enable smart card mode */
+
+            cr3 |= USART_CR3_SCEN | USART_CR3_NACK; /* Always enable NACK */
+
+            /* Smart card mode does not use RX, but uses CLK */
+
+            stm32_unconfiggpio(priv->rx_gpio);
+            if (!priv->ck_gpio)
+              {
+                ret = -EIO;
+                break;
+              }
+
+            stm32_configgpio(priv->ck_gpio);
+
+            /* Note: TX gpio has to be open drain */
+
+            if (priv->smartcard.clken)
+              {
+                /* Enable smart card clock */
+
+                cr2 |=   USART_CR2_CLKEN | USART_CR2_LBCL;
+                cr2 &= ~(USART_CR2_CPOL  | USART_CR2_CPHA);
+              }
+            else
+              {
+                cr2 &= ~USART_CR2_CLKEN; /* Disable smart card clock */
+              }
+
+            if (priv->smartcard.inverse)
+              {
+                /* Enable inverse convention */
+
+                cr2 |= USART_CR2_MSBFIRST | USART_CR2_DATAINV;
+              }
+            else
+              {
+                /* Enable direct convention */
+
+                cr3 &= ~(USART_CR2_MSBFIRST | USART_CR2_DATAINV);
+              }
+
+            if (priv->smartcard.clkout)
+              {
+                /* Compute prescaler to match request,
+                 * then return the actual value.
+                 */
+
+                uint8_t clkdiv = 1;
+                uint32_t divided = priv->apbclock >> 1;
+                while ((divided > priv->smartcard.clkout) && (clkdiv < 31))
+                  {
+                    clkdiv   += 1;
+                    divided >>= 1;
+                  }
+
+                priv->smartcard.clkout = priv->apbclock / (clkdiv << 1);
+                gtpr = (gtpr & ~USART_GTPR_PSC_MASK) |
+                       (clkdiv << USART_GTPR_PSC_SHIFT);
+              }
+
+            if (priv->smartcard.etu)
+              {
+                uint8_t clkdiv;
+                uint32_t etuclock;
+
+                /* Get the current prescaler from gtpr, either the original
+                 * value, or the one that was just updated
+                 */
+
+                clkdiv = (gtpr & USART_GTPR_PSC_MASK) >>
+                         USART_GTPR_PSC_SHIFT;
+                clkdiv <<= 1; /* Actual divider is twice the reg contents */
+
+                /* The refernce clock for the ETU is the divided clock */
+
+                etuclock = priv->apbclock / clkdiv;
+
+                /* Compute baud rate from clock divider and ETU */
+
+                priv->baud      = etuclock / priv->smartcard.etu;
+                priv->bits      = 8;
+                priv->parity    = 2; /* even */
+                priv->stopbits2 = TRUE;
+                up_set_format(dev);
+              }
+
+            if (priv->smartcard.cgt)
+              {
+                gtpr = (gtpr & ~USART_GTPR_GT_MASK) |
+                       (sc->cgt << USART_GTPR_GT_SHIFT);
+              }
+          }
+        else
+          {
+            /* User has requested to disable smart card mode */
+
+            cr3 &= ~USART_CR3_SCEN;
+            cr2 &= ~USART_CR2_CLKEN; /* Disable smart card clock */
+
+            /* Re-enable RX and disable CLK */
+
+            stm32_configgpio  (priv->rx_gpio);

Review Comment:
   Please tell me globally if that kind of alignment within consecutive lines is a thing I should continue or not.
   
   I personally find it has cosmetic value and has no drawbacks, but I can avoid it if someone hates it and insist.



##########
arch/arm/src/stm32h7/stm32_serial.c:
##########
@@ -2660,6 +2719,201 @@ static int up_ioctl(struct file *filep, int cmd, unsigned long arg)
      break;
 #endif
 
+#ifdef CONFIG_STM32H7_USART_SMARTCARD
+    case TIOCGSMARTCARD:
+      {
+        struct serial_smartcard_s *sc = (struct serial_smartcard_s *)arg;
+
+        /* Determine argument validity */
+
+        if (!sc)
+          {
+            ret = -EINVAL;
+            break;
+          }
+
+        /* Determine if this uart supports smartcard mode */
+
+        if (!priv->sc_supported)
+          {
+            ret = -ENOTTY;
+            break;
+          }
+
+        memcpy(sc, &priv->smartcard, sizeof(struct serial_smartcard_s));
+        break;
+      }
+
+    case TIOCSSMARTCARD:
+      {
+        struct serial_smartcard_s *sc = (struct serial_smartcard_s *)arg;
+        uint32_t cr1;
+        uint32_t cr1_ue;
+        uint32_t cr2;
+        uint32_t cr3;
+        uint32_t gtpr;
+        irqstate_t flags;
+
+        /* Determine argument validity */
+
+        if (!sc)
+          {
+            ret = -EINVAL;
+            break;
+          }
+
+        /* Determine if this uart supports smartcard mode */
+
+        if (!priv->sc_supported)
+          {
+            ret = -ENOTTY;
+            break;
+          }
+
+        memcpy(&priv->smartcard, sc, sizeof(struct serial_smartcard_s));
+
+        flags = enter_critical_section();
+
+        /* Get the original state of UE */
+
+        cr1  = up_serialin(priv, STM32_USART_CR1_OFFSET);
+        cr1_ue = cr1 & USART_CR1_UE;
+        cr1 &= ~USART_CR1_UE;
+
+        /* Disable UE, config can only be written when UE=0 */
+
+        up_serialout(priv, STM32_USART_CR1_OFFSET, cr1);
+
+        /* Read smart card related registers */
+
+        cr2  = up_serialin(priv, STM32_USART_CR2_OFFSET);
+        cr3  = up_serialin(priv, STM32_USART_CR3_OFFSET);
+        gtpr = up_serialin(priv, STM32_USART_GTPR_OFFSET);
+
+        /* Apply settings */
+
+        if (priv->smartcard.enabled)
+          {
+            /* User has requested to enable smart card mode */
+
+            cr3 |= USART_CR3_SCEN | USART_CR3_NACK; /* Always enable NACK */
+
+            /* Smart card mode does not use RX, but uses CLK */
+
+            stm32_unconfiggpio(priv->rx_gpio);
+            if (!priv->ck_gpio)
+              {
+                ret = -EIO;
+                break;
+              }
+
+            stm32_configgpio(priv->ck_gpio);
+
+            /* Note: TX gpio has to be open drain */
+
+            if (priv->smartcard.clken)
+              {
+                /* Enable smart card clock */
+
+                cr2 |=   USART_CR2_CLKEN | USART_CR2_LBCL;
+                cr2 &= ~(USART_CR2_CPOL  | USART_CR2_CPHA);
+              }
+            else
+              {
+                cr2 &= ~USART_CR2_CLKEN; /* Disable smart card clock */
+              }
+
+            if (priv->smartcard.inverse)
+              {
+                /* Enable inverse convention */
+
+                cr2 |= USART_CR2_MSBFIRST | USART_CR2_DATAINV;
+              }
+            else
+              {
+                /* Enable direct convention */
+
+                cr3 &= ~(USART_CR2_MSBFIRST | USART_CR2_DATAINV);
+              }
+
+            if (priv->smartcard.clkout)
+              {
+                /* Compute prescaler to match request,
+                 * then return the actual value.
+                 */
+
+                uint8_t clkdiv = 1;
+                uint32_t divided = priv->apbclock >> 1;
+                while ((divided > priv->smartcard.clkout) && (clkdiv < 31))
+                  {
+                    clkdiv   += 1;
+                    divided >>= 1;
+                  }
+
+                priv->smartcard.clkout = priv->apbclock / (clkdiv << 1);
+                gtpr = (gtpr & ~USART_GTPR_PSC_MASK) |
+                       (clkdiv << USART_GTPR_PSC_SHIFT);
+              }
+
+            if (priv->smartcard.etu)
+              {
+                uint8_t clkdiv;
+                uint32_t etuclock;
+
+                /* Get the current prescaler from gtpr, either the original
+                 * value, or the one that was just updated
+                 */
+
+                clkdiv = (gtpr & USART_GTPR_PSC_MASK) >>
+                         USART_GTPR_PSC_SHIFT;
+                clkdiv <<= 1; /* Actual divider is twice the reg contents */
+
+                /* The refernce clock for the ETU is the divided clock */
+
+                etuclock = priv->apbclock / clkdiv;
+
+                /* Compute baud rate from clock divider and ETU */
+
+                priv->baud      = etuclock / priv->smartcard.etu;
+                priv->bits      = 8;
+                priv->parity    = 2; /* even */
+                priv->stopbits2 = TRUE;
+                up_set_format(dev);
+              }
+
+            if (priv->smartcard.cgt)
+              {
+                gtpr = (gtpr & ~USART_GTPR_GT_MASK) |
+                       (sc->cgt << USART_GTPR_GT_SHIFT);
+              }
+          }
+        else
+          {
+            /* User has requested to disable smart card mode */
+
+            cr3 &= ~USART_CR3_SCEN;
+            cr2 &= ~USART_CR2_CLKEN; /* Disable smart card clock */
+
+            /* Re-enable RX and disable CLK */
+
+            stm32_configgpio  (priv->rx_gpio);

Review Comment:
   Please tell me globally if that kind of alignment within consecutive lines is a thing I should continue or not.
   
   I personally find it has cosmetic value and has no drawbacks, but I can avoid it if someone hates it and insists.



-- 
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] slorquet commented on a diff in pull request #9530: Smartcard for stm32h7 uart

Posted by "slorquet (via GitHub)" <gi...@apache.org>.
slorquet commented on code in PR #9530:
URL: https://github.com/apache/nuttx/pull/9530#discussion_r1229163683


##########
arch/arm/src/stm32h7/stm32_serial.c:
##########
@@ -2660,6 +2719,201 @@ static int up_ioctl(struct file *filep, int cmd, unsigned long arg)
      break;
 #endif
 
+#ifdef CONFIG_STM32H7_USART_SMARTCARD
+    case TIOCGSMARTCARD:
+      {
+        struct serial_smartcard_s *sc = (struct serial_smartcard_s *)arg;
+
+        /* Determine argument validity */
+
+        if (!sc)
+          {
+            ret = -EINVAL;
+            break;
+          }
+
+        /* Determine if this uart supports smartcard mode */
+
+        if (!priv->sc_supported)
+          {
+            ret = -ENOTTY;
+            break;
+          }
+
+        memcpy(sc, &priv->smartcard, sizeof(struct serial_smartcard_s));
+        break;
+      }
+
+    case TIOCSSMARTCARD:
+      {
+        struct serial_smartcard_s *sc = (struct serial_smartcard_s *)arg;
+        uint32_t cr1;
+        uint32_t cr1_ue;
+        uint32_t cr2;
+        uint32_t cr3;
+        uint32_t gtpr;
+        irqstate_t flags;
+
+        /* Determine argument validity */
+
+        if (!sc)
+          {
+            ret = -EINVAL;
+            break;
+          }
+
+        /* Determine if this uart supports smartcard mode */
+
+        if (!priv->sc_supported)
+          {
+            ret = -ENOTTY;
+            break;
+          }
+
+        memcpy(&priv->smartcard, sc, sizeof(struct serial_smartcard_s));
+
+        flags = enter_critical_section();
+
+        /* Get the original state of UE */
+
+        cr1  = up_serialin(priv, STM32_USART_CR1_OFFSET);
+        cr1_ue = cr1 & USART_CR1_UE;
+        cr1 &= ~USART_CR1_UE;
+
+        /* Disable UE, config can only be written when UE=0 */
+
+        up_serialout(priv, STM32_USART_CR1_OFFSET, cr1);
+
+        /* Read smart card related registers */
+
+        cr2  = up_serialin(priv, STM32_USART_CR2_OFFSET);
+        cr3  = up_serialin(priv, STM32_USART_CR3_OFFSET);
+        gtpr = up_serialin(priv, STM32_USART_GTPR_OFFSET);
+
+        /* Apply settings */
+
+        if (priv->smartcard.enabled)
+          {
+            /* User has requested to enable smart card mode */
+
+            cr3 |= USART_CR3_SCEN | USART_CR3_NACK; /* Always enable NACK */
+
+            /* Smart card mode does not use RX, but uses CLK */
+
+            stm32_unconfiggpio(priv->rx_gpio);
+            if (!priv->ck_gpio)
+              {
+                ret = -EIO;
+                break;
+              }
+
+            stm32_configgpio(priv->ck_gpio);
+
+            /* Note: TX gpio has to be open drain */
+
+            if (priv->smartcard.clken)
+              {
+                /* Enable smart card clock */
+
+                cr2 |=   USART_CR2_CLKEN | USART_CR2_LBCL;
+                cr2 &= ~(USART_CR2_CPOL  | USART_CR2_CPHA);
+              }
+            else
+              {
+                cr2 &= ~USART_CR2_CLKEN; /* Disable smart card clock */
+              }
+
+            if (priv->smartcard.inverse)
+              {
+                /* Enable inverse convention */
+
+                cr2 |= USART_CR2_MSBFIRST | USART_CR2_DATAINV;
+              }
+            else
+              {
+                /* Enable direct convention */
+
+                cr3 &= ~(USART_CR2_MSBFIRST | USART_CR2_DATAINV);
+              }
+
+            if (priv->smartcard.clkout)
+              {
+                /* Compute prescaler to match request,
+                 * then return the actual value.
+                 */
+
+                uint8_t clkdiv = 1;
+                uint32_t divided = priv->apbclock >> 1;
+                while ((divided > priv->smartcard.clkout) && (clkdiv < 31))
+                  {
+                    clkdiv   += 1;
+                    divided >>= 1;
+                  }
+
+                priv->smartcard.clkout = priv->apbclock / (clkdiv << 1);
+                gtpr = (gtpr & ~USART_GTPR_PSC_MASK) |
+                       (clkdiv << USART_GTPR_PSC_SHIFT);
+              }
+
+            if (priv->smartcard.etu)
+              {
+                uint8_t clkdiv;
+                uint32_t etuclock;
+
+                /* Get the current prescaler from gtpr, either the original
+                 * value, or the one that was just updated
+                 */
+
+                clkdiv = (gtpr & USART_GTPR_PSC_MASK) >>
+                         USART_GTPR_PSC_SHIFT;
+                clkdiv <<= 1; /* Actual divider is twice the reg contents */
+
+                /* The refernce clock for the ETU is the divided clock */
+
+                etuclock = priv->apbclock / clkdiv;
+
+                /* Compute baud rate from clock divider and ETU */
+
+                priv->baud      = etuclock / priv->smartcard.etu;
+                priv->bits      = 8;
+                priv->parity    = 2; /* even */
+                priv->stopbits2 = TRUE;

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] davids5 commented on pull request #9530: Smartcard for stm32h7 uart

Posted by "davids5 (via GitHub)" <gi...@apache.org>.
davids5 commented on PR #9530:
URL: https://github.com/apache/nuttx/pull/9530#issuecomment-1589772099

   @slorquet 
   A few questions:
   
   Is this for ISO7816? 
   Would it be better to make is a separate  driver?


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