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/01/18 02:23:34 UTC

[GitHub] [incubator-nuttx] SPRESENSE opened a new pull request #5256: Add usbdev serial use boardid

SPRESENSE opened a new pull request #5256:
URL: https://github.com/apache/incubator-nuttx/pull/5256


   ## Summary
   
   Add use board unique ID as a USB device serial number support.
   
   ## Impact
   
   USB devices (MSC, Composite, PL2303, CDCACM, CDCECM, RNDIS, ADB)
   
   ## Testing
   
   Runs on Sony Spresense board with USBMSC configuration, and confirmed that board unique ID is set at iSerial field from connected PC.


-- 
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 change in pull request #5256: Add usbdev serial use boardid

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5256:
URL: https://github.com/apache/incubator-nuttx/pull/5256#discussion_r787666136



##########
File path: include/nuttx/board.h
##########
@@ -429,6 +429,23 @@ int board_composite_initialize(int port);
 FAR void *board_composite_connect(int port, int configid);
 #endif
 
+/****************************************************************************
+ * Name:  board_usbdev_serialstr
+ *
+ * Description:
+ *   Use board unique serial number string to iSerialNumber field in the
+ *   device descriptor. This is for determining the board when multiple
+ *   boards on the same host.
+ *
+ * Returned Value:
+ *   The board unique serial number string.
+ *
+ ****************************************************************************/
+
+#if defined(CONFIG_BOARD_USBDEV_SERIALSTR)
+FAR const char *board_usbdev_serialstr(void);

Review comment:
       But should we give a more general name here? It's too bad to couple serial string to usbdev. I could prefer that:
   ```
   #if defined(CONFIG_BOARD_UNIQUESTR)
   FAR const char *board_uniquestr(void);
   #endif
   ```
   and provide a default implementation on top of board_uniqueid if no special need.




-- 
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 change in pull request #5256: Add usbdev serial use boardid

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5256:
URL: https://github.com/apache/incubator-nuttx/pull/5256#discussion_r787666136



##########
File path: include/nuttx/board.h
##########
@@ -429,6 +429,23 @@ int board_composite_initialize(int port);
 FAR void *board_composite_connect(int port, int configid);
 #endif
 
+/****************************************************************************
+ * Name:  board_usbdev_serialstr
+ *
+ * Description:
+ *   Use board unique serial number string to iSerialNumber field in the
+ *   device descriptor. This is for determining the board when multiple
+ *   boards on the same host.
+ *
+ * Returned Value:
+ *   The board unique serial number string.
+ *
+ ****************************************************************************/
+
+#if defined(CONFIG_BOARD_USBDEV_SERIALSTR)
+FAR const char *board_usbdev_serialstr(void);

Review comment:
       But should we give a more general name here? It's too bad to couple serial string to usbdev. I could prefer that:
   #if defined(CONFIG_BOARD_UNIQUESTR)
   FAR const char *board_uniquestr(void);
   #endif
   and provide a defined implementation on top of board_uniqueid.




-- 
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] SPRESENSE commented on a change in pull request #5256: Add usbdev serial use boardid

Posted by GitBox <gi...@apache.org>.
SPRESENSE commented on a change in pull request #5256:
URL: https://github.com/apache/incubator-nuttx/pull/5256#discussion_r786509547



##########
File path: boards/arm/cxd56xx/common/src/cxd56_usbdevserialstr.c
##########
@@ -0,0 +1,52 @@
+/****************************************************************************
+ * boards/arm/cxd56xx/common/src/cxd56_usbdevserialstr.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdio.h>
+
+#include "cxd56_uid.h"
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+#ifdef CONFIG_BOARD_USBDEV_SERIALSTR
+
+static char g_serialstr[CONFIG_BOARDCTL_UNIQUEID_SIZE * 2 + 1];
+
+FAR const char *board_usbdev_serialstr(void)
+{
+  uint8_t uid[CONFIG_BOARDCTL_UNIQUEID_SIZE];
+
+  cxd56_get_uniqueid(uid);
+
+  snprintf(g_serialstr, CONFIG_BOARDCTL_UNIQUEID_SIZE * 2 + 1,

Review comment:
       I've 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] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5256: Add usbdev serial use boardid

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5256:
URL: https://github.com/apache/incubator-nuttx/pull/5256#discussion_r786610383



##########
File path: include/nuttx/board.h
##########
@@ -429,6 +429,23 @@ int board_composite_initialize(int port);
 FAR void *board_composite_connect(int port, int configid);
 #endif
 
+/****************************************************************************
+ * Name:  board_usbdev_serialstr
+ *
+ * Description:
+ *   Use board unique serial number string to iSerialNumber field in the
+ *   device descriptor. This is for determining the board when multiple
+ *   boards on the same host.
+ *
+ * Returned Value:
+ *   The board unique serial number string.
+ *
+ ****************************************************************************/
+
+#if defined(CONFIG_BOARD_USBDEV_SERIALSTR)
+FAR const char *board_usbdev_serialstr(void);

Review comment:
       can we reuse board_uniqueid? we can simply format the binary to string.




-- 
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] jerpelea merged pull request #5256: Add usbdev serial use boardid

Posted by GitBox <gi...@apache.org>.
jerpelea merged pull request #5256:
URL: https://github.com/apache/incubator-nuttx/pull/5256


   


-- 
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 change in pull request #5256: Add usbdev serial use boardid

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5256:
URL: https://github.com/apache/incubator-nuttx/pull/5256#discussion_r787666136



##########
File path: include/nuttx/board.h
##########
@@ -429,6 +429,23 @@ int board_composite_initialize(int port);
 FAR void *board_composite_connect(int port, int configid);
 #endif
 
+/****************************************************************************
+ * Name:  board_usbdev_serialstr
+ *
+ * Description:
+ *   Use board unique serial number string to iSerialNumber field in the
+ *   device descriptor. This is for determining the board when multiple
+ *   boards on the same host.
+ *
+ * Returned Value:
+ *   The board unique serial number string.
+ *
+ ****************************************************************************/
+
+#if defined(CONFIG_BOARD_USBDEV_SERIALSTR)
+FAR const char *board_usbdev_serialstr(void);

Review comment:
       But should we give a more general name here? It's too bad to couple serial string to usbdev. I could prefer that:
   ```
   #if defined(CONFIG_BOARD_UNIQUESTR)
   FAR const char *board_uniquestr(void);
   #endif
   ```
   and provide a defined implementation on top of board_uniqueid if no special need.




-- 
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] SPRESENSE commented on a change in pull request #5256: Add usbdev serial use boardid

Posted by GitBox <gi...@apache.org>.
SPRESENSE commented on a change in pull request #5256:
URL: https://github.com/apache/incubator-nuttx/pull/5256#discussion_r787266566



##########
File path: include/nuttx/board.h
##########
@@ -429,6 +429,23 @@ int board_composite_initialize(int port);
 FAR void *board_composite_connect(int port, int configid);
 #endif
 
+/****************************************************************************
+ * Name:  board_usbdev_serialstr
+ *
+ * Description:
+ *   Use board unique serial number string to iSerialNumber field in the
+ *   device descriptor. This is for determining the board when multiple
+ *   boards on the same host.
+ *
+ * Returned Value:
+ *   The board unique serial number string.
+ *
+ ****************************************************************************/
+
+#if defined(CONFIG_BOARD_USBDEV_SERIALSTR)
+FAR const char *board_usbdev_serialstr(void);

Review comment:
       Yes, but I don't recommend to use board_uniqueid directly. Because USB serial number can use any string, `MYDEV112233` for example. This new API is for possible to do that.




-- 
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 change in pull request #5256: Add usbdev serial use boardid

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5256:
URL: https://github.com/apache/incubator-nuttx/pull/5256#discussion_r786495378



##########
File path: drivers/usbdev/usbmsc_scsi.c
##########
@@ -173,6 +173,8 @@ static int    usbmsc_cmdstatusstate(FAR struct usbmsc_dev_s *priv);
  * Private Data
  ****************************************************************************/
 
+static const char *g_productrevision = "0101";

Review comment:
       So this is hardcoded? Why?

##########
File path: boards/arm/cxd56xx/common/src/cxd56_usbdevserialstr.c
##########
@@ -0,0 +1,52 @@
+/****************************************************************************
+ * boards/arm/cxd56xx/common/src/cxd56_usbdevserialstr.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdio.h>
+
+#include "cxd56_uid.h"
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+#ifdef CONFIG_BOARD_USBDEV_SERIALSTR
+
+static char g_serialstr[CONFIG_BOARDCTL_UNIQUEID_SIZE * 2 + 1];
+
+FAR const char *board_usbdev_serialstr(void)
+{
+  uint8_t uid[CONFIG_BOARDCTL_UNIQUEID_SIZE];
+
+  cxd56_get_uniqueid(uid);
+
+  snprintf(g_serialstr, CONFIG_BOARDCTL_UNIQUEID_SIZE * 2 + 1,

Review comment:
       ```suggestion
     snprintf(g_serialstr, sizeof(g_serialstr),
   ```




-- 
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] SPRESENSE commented on a change in pull request #5256: Add usbdev serial use boardid

Posted by GitBox <gi...@apache.org>.
SPRESENSE commented on a change in pull request #5256:
URL: https://github.com/apache/incubator-nuttx/pull/5256#discussion_r786509297



##########
File path: drivers/usbdev/usbmsc_scsi.c
##########
@@ -173,6 +173,8 @@ static int    usbmsc_cmdstatusstate(FAR struct usbmsc_dev_s *priv);
  * Private Data
  ****************************************************************************/
 
+static const char *g_productrevision = "0101";

Review comment:
       I don't know about the details of this SCSI field, but USBMSC does not affected when this field is any values. So I wrote it in hard coded anyway.
   




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