You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by GitBox <gi...@apache.org> on 2021/10/07 12:26:59 UTC

[GitHub] [mynewt-core] kasjer opened a new pull request #2692: Add USB DFU functionality

kasjer opened a new pull request #2692:
URL: https://github.com/apache/mynewt-core/pull/2692


   This adds package with USB DFU functionality tailored to 
   fill SLOT1 (when build in application) or SLOT0 if build with mcuboot.
   
   once DFU is enabled standard utilities for firmware update can be used
   (dfu-util, dfu-tool or other that implement host side of DFU).
   
   This only exposes one image slot (not whole flash) and allows only writing to
   the device.
   DFU functionality of reading firmware is not implemented.


-- 
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@mynewt.apache.org

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



[GitHub] [mynewt-core] utzig commented on a change in pull request #2692: Add USB DFU functionality

Posted by GitBox <gi...@apache.org>.
utzig commented on a change in pull request #2692:
URL: https://github.com/apache/mynewt-core/pull/2692#discussion_r724161120



##########
File path: hw/usb/tinyusb/dfu/src/dfu.c
##########
@@ -0,0 +1,169 @@
+/*
+ * 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.
+ */
+
+#include <os/mynewt.h>
+
+#include <class/dfu/dfu_device.h>
+
+#include <bsp/bsp.h>
+#include <img_mgmt/img_mgmt.h>
+
+/*
+ * If DFU is activated from bootloader it writes to SLOT0.
+ * If application code handles DFU it writes to SLOT1 and marks slot as
+ * pending.
+ */
+#if MYNEWT_VAL(BOOT_LOADER)
+#define FIRMWARE_SLOT       FLASH_AREA_IMAGE_0
+#else
+#define FIRMWARE_SLOT       FLASH_AREA_IMAGE_1
+#endif
+
+#if MYNEWT_VAL(USBD_DFU_RESET_AFTER_DOWNLOAD)
+
+struct os_callout delayed_reset_callout;
+
+void
+delayed_reset_cb(struct os_event *event)
+{
+    hal_system_reset();
+}
+
+#endif
+
+/*
+ * DFU callbacks
+ * Note: alt is used as the partition number, in order to support multiple partitions like FLASH, EEPROM, etc.
+ */
+
+/*
+ * Invoked right before tud_dfu_download_cb() (state=DFU_DNBUSY) or tud_dfu_manifest_cb() (state=DFU_MANIFEST)
+ * Application return timeout in milliseconds (bwPollTimeout) for the next download/manifest operation.
+ * During this period, USB host won't try to communicate with us.
+ */
+uint32_t
+tud_dfu_get_timeout_cb(uint8_t alt, uint8_t state)
+{
+    if (state == DFU_DNBUSY) {
+        return MYNEWT_VAL(USBD_DFU_BLOCK_WRITE_TIME);
+    } else if (state == DFU_MANIFEST) {
+        /* since we don't buffer entire image and do any flashing in manifest stage */
+        return 0;
+    }

Review comment:
       Since this function returns `0` anyway, this `else if` might be removed?




-- 
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@mynewt.apache.org

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



[GitHub] [mynewt-core] utzig commented on a change in pull request #2692: Add USB DFU functionality

Posted by GitBox <gi...@apache.org>.
utzig commented on a change in pull request #2692:
URL: https://github.com/apache/mynewt-core/pull/2692#discussion_r724153676



##########
File path: hw/usb/tinyusb/dfu/syscfg.yml
##########
@@ -0,0 +1,85 @@
+#
+# 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.
+#
+
+syscfg.defs:
+    USBD_DFU_DESCRIPTOR_STRING:
+        description: String for DFU descriptor
+        value: NULL
+    USBD_DFU_SLOT_NAME:
+        description: Name of firmware slot that will be visible in DFU list.
+        value: '"SLOT1"'
+    USBD_DFU_BLOCK_SIZE:
+        description: >
+            Size of block that will be used during upgrade.
+            It must be equal of greater than minimum block size that can be
+            erased.
+            It is flash dependent. For SPIFLASH typical value is 4096.
+            For on-chip flash values can be different.
+        value:

Review comment:
       Hmmm, can a `value:` have no default value? Never tried it TBH...




-- 
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@mynewt.apache.org

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



[GitHub] [mynewt-core] kasjer merged pull request #2692: Add USB DFU functionality

Posted by GitBox <gi...@apache.org>.
kasjer merged pull request #2692:
URL: https://github.com/apache/mynewt-core/pull/2692


   


-- 
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@mynewt.apache.org

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



[GitHub] [mynewt-core] utzig commented on a change in pull request #2692: Add USB DFU functionality

Posted by GitBox <gi...@apache.org>.
utzig commented on a change in pull request #2692:
URL: https://github.com/apache/mynewt-core/pull/2692#discussion_r724153676



##########
File path: hw/usb/tinyusb/dfu/syscfg.yml
##########
@@ -0,0 +1,85 @@
+#
+# 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.
+#
+
+syscfg.defs:
+    USBD_DFU_DESCRIPTOR_STRING:
+        description: String for DFU descriptor
+        value: NULL
+    USBD_DFU_SLOT_NAME:
+        description: Name of firmware slot that will be visible in DFU list.
+        value: '"SLOT1"'
+    USBD_DFU_BLOCK_SIZE:
+        description: >
+            Size of block that will be used during upgrade.
+            It must be equal of greater than minimum block size that can be
+            erased.
+            It is flash dependent. For SPIFLASH typical value is 4096.
+            For on-chip flash values can be different.
+        value:

Review comment:
       Hmmm, can a `value:` have no default value? Never tried it TBH...

##########
File path: hw/usb/tinyusb/dfu/src/dfu.c
##########
@@ -0,0 +1,169 @@
+/*
+ * 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.
+ */
+
+#include <os/mynewt.h>
+
+#include <class/dfu/dfu_device.h>
+
+#include <bsp/bsp.h>
+#include <img_mgmt/img_mgmt.h>
+
+/*
+ * If DFU is activated from bootloader it writes to SLOT0.
+ * If application code handles DFU it writes to SLOT1 and marks slot as
+ * pending.
+ */
+#if MYNEWT_VAL(BOOT_LOADER)
+#define FIRMWARE_SLOT       FLASH_AREA_IMAGE_0
+#else
+#define FIRMWARE_SLOT       FLASH_AREA_IMAGE_1
+#endif
+
+#if MYNEWT_VAL(USBD_DFU_RESET_AFTER_DOWNLOAD)
+
+struct os_callout delayed_reset_callout;
+
+void
+delayed_reset_cb(struct os_event *event)
+{
+    hal_system_reset();
+}
+
+#endif
+
+/*
+ * DFU callbacks
+ * Note: alt is used as the partition number, in order to support multiple partitions like FLASH, EEPROM, etc.
+ */
+
+/*
+ * Invoked right before tud_dfu_download_cb() (state=DFU_DNBUSY) or tud_dfu_manifest_cb() (state=DFU_MANIFEST)
+ * Application return timeout in milliseconds (bwPollTimeout) for the next download/manifest operation.
+ * During this period, USB host won't try to communicate with us.
+ */
+uint32_t
+tud_dfu_get_timeout_cb(uint8_t alt, uint8_t state)
+{
+    if (state == DFU_DNBUSY) {
+        return MYNEWT_VAL(USBD_DFU_BLOCK_WRITE_TIME);
+    } else if (state == DFU_MANIFEST) {
+        /* since we don't buffer entire image and do any flashing in manifest stage */
+        return 0;
+    }

Review comment:
       Since this function returns `0` anyway, this `else if` might be removed?




-- 
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@mynewt.apache.org

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



[GitHub] [mynewt-core] sjanc commented on a change in pull request #2692: Add USB DFU functionality

Posted by GitBox <gi...@apache.org>.
sjanc commented on a change in pull request #2692:
URL: https://github.com/apache/mynewt-core/pull/2692#discussion_r724186285



##########
File path: hw/usb/tinyusb/dfu/pkg.yml
##########
@@ -0,0 +1,34 @@
+#
+# 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.
+#
+
+pkg.name: hw/usb/tinyusb/dfu
+pkg.description: DFU.

Review comment:
       just a nitpick that description could be more descriptive :-)




-- 
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@mynewt.apache.org

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



[GitHub] [mynewt-core] kasjer commented on a change in pull request #2692: Add USB DFU functionality

Posted by GitBox <gi...@apache.org>.
kasjer commented on a change in pull request #2692:
URL: https://github.com/apache/mynewt-core/pull/2692#discussion_r724157770



##########
File path: hw/usb/tinyusb/dfu/syscfg.yml
##########
@@ -0,0 +1,85 @@
+#
+# 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.
+#
+
+syscfg.defs:
+    USBD_DFU_DESCRIPTOR_STRING:
+        description: String for DFU descriptor
+        value: NULL
+    USBD_DFU_SLOT_NAME:
+        description: Name of firmware slot that will be visible in DFU list.
+        value: '"SLOT1"'
+    USBD_DFU_BLOCK_SIZE:
+        description: >
+            Size of block that will be used during upgrade.
+            It must be equal of greater than minimum block size that can be
+            erased.
+            It is flash dependent. For SPIFLASH typical value is 4096.
+            For on-chip flash values can be different.
+        value:

Review comment:
       It can moreover some time ago I made change to newt tool to allow setting value in package that is not BSP/APP/TARGET if it was defined but had not default value. Previously it was prohibited but it is useful if there is some generic package like MCU family that define a lot but some specific MCU can override settings this pattern was used in dialog MCU.

##########
File path: hw/usb/tinyusb/dfu/syscfg.yml
##########
@@ -0,0 +1,85 @@
+#
+# 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.
+#
+
+syscfg.defs:
+    USBD_DFU_DESCRIPTOR_STRING:
+        description: String for DFU descriptor
+        value: NULL
+    USBD_DFU_SLOT_NAME:
+        description: Name of firmware slot that will be visible in DFU list.
+        value: '"SLOT1"'
+    USBD_DFU_BLOCK_SIZE:
+        description: >
+            Size of block that will be used during upgrade.
+            It must be equal of greater than minimum block size that can be
+            erased.
+            It is flash dependent. For SPIFLASH typical value is 4096.
+            For on-chip flash values can be different.
+        value:

Review comment:
       For this particular case idea is like this: tusb_hw.h (files) define MCU specific default value for block size. It could be something like 4096 for Dialog MCU where external QSPI flash is used. For internal flashes erase size could have other constraints.
   syscfg value is added to allow user to fine tune buffer size that could be change to increase performance or reduce RAM usage.




-- 
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@mynewt.apache.org

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



[GitHub] [mynewt-core] sjanc commented on a change in pull request #2692: Add USB DFU functionality

Posted by GitBox <gi...@apache.org>.
sjanc commented on a change in pull request #2692:
URL: https://github.com/apache/mynewt-core/pull/2692#discussion_r724186285



##########
File path: hw/usb/tinyusb/dfu/pkg.yml
##########
@@ -0,0 +1,34 @@
+#
+# 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.
+#
+
+pkg.name: hw/usb/tinyusb/dfu
+pkg.description: DFU.

Review comment:
       just a nitpick that description could be more descriptive :-)




-- 
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@mynewt.apache.org

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



[GitHub] [mynewt-core] kasjer commented on a change in pull request #2692: Add USB DFU functionality

Posted by GitBox <gi...@apache.org>.
kasjer commented on a change in pull request #2692:
URL: https://github.com/apache/mynewt-core/pull/2692#discussion_r724164542



##########
File path: hw/usb/tinyusb/dfu/syscfg.yml
##########
@@ -0,0 +1,85 @@
+#
+# 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.
+#
+
+syscfg.defs:
+    USBD_DFU_DESCRIPTOR_STRING:
+        description: String for DFU descriptor
+        value: NULL
+    USBD_DFU_SLOT_NAME:
+        description: Name of firmware slot that will be visible in DFU list.
+        value: '"SLOT1"'
+    USBD_DFU_BLOCK_SIZE:
+        description: >
+            Size of block that will be used during upgrade.
+            It must be equal of greater than minimum block size that can be
+            erased.
+            It is flash dependent. For SPIFLASH typical value is 4096.
+            For on-chip flash values can be different.
+        value:

Review comment:
       For this particular case idea is like this: tusb_hw.h (files) define MCU specific default value for block size. It could be something like 4096 for Dialog MCU where external QSPI flash is used. For internal flashes erase size could have other constraints.
   syscfg value is added to allow user to fine tune buffer size that could be change to increase performance or reduce RAM usage.




-- 
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@mynewt.apache.org

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



[GitHub] [mynewt-core] kasjer commented on a change in pull request #2692: Add USB DFU functionality

Posted by GitBox <gi...@apache.org>.
kasjer commented on a change in pull request #2692:
URL: https://github.com/apache/mynewt-core/pull/2692#discussion_r724157770



##########
File path: hw/usb/tinyusb/dfu/syscfg.yml
##########
@@ -0,0 +1,85 @@
+#
+# 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.
+#
+
+syscfg.defs:
+    USBD_DFU_DESCRIPTOR_STRING:
+        description: String for DFU descriptor
+        value: NULL
+    USBD_DFU_SLOT_NAME:
+        description: Name of firmware slot that will be visible in DFU list.
+        value: '"SLOT1"'
+    USBD_DFU_BLOCK_SIZE:
+        description: >
+            Size of block that will be used during upgrade.
+            It must be equal of greater than minimum block size that can be
+            erased.
+            It is flash dependent. For SPIFLASH typical value is 4096.
+            For on-chip flash values can be different.
+        value:

Review comment:
       It can moreover some time ago I made change to newt tool to allow setting value in package that is not BSP/APP/TARGET if it was defined but had not default value. Previously it was prohibited but it is useful if there is some generic package like MCU family that define a lot but some specific MCU can override settings this pattern was used in dialog MCU.




-- 
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@mynewt.apache.org

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