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/07/19 22:06:18 UTC
[GitHub] [incubator-nuttx] ALTracer opened a new pull request, #6640: net/slip: Rename and clarify orphaned Kconfig options
ALTracer opened a new pull request, #6640:
URL: https://github.com/apache/incubator-nuttx/pull/6640
## Summary
Rename the SLIP options to be consistent with the driver source.
2d7c072723902aa763d3851f6399c39cbb1851fe deleted them from drivers/net/Kconfig as "duplicates" despite them having slightly different names
(SLIP_STACKSIZE vs NET_SLIP_STACKSIZE). See https://github.com/apache/incubator-nuttx/blob/b10658653ba4567191ffcd98e0e4c1a3df3f0db0/drivers/net/slip.c#L68
```c
#ifndef CONFIG_NET_SLIP_STACKSIZE
# define CONFIG_NET_SLIP_STACKSIZE 2048
#endif
#ifndef CONFIG_NET_SLIP_DEFPRIO
# define CONFIG_NET_SLIP_DEFPRIO 128
#endif
```
but https://github.com/apache/incubator-nuttx/blob/b10658653ba4567191ffcd98e0e4c1a3df3f0db0/net/Kconfig#L207
This reserves 2048 bytes by default (not even CONFIG_DEFAULT_TASK_SIZE) for each of 2 threads,
even though I see typically only 300-400 bytes used on armv6-m UART 115200 link responding to 50..250-byte ICMP pings.
Make kthread stacksize configurable again.
I don't feel like these options truly belonged in drivers/net/Kconfig with external MACs, either.
## Impact
Boards using SLIP. Currently in-tree only sim:tcpblaster and olimex-lpc1766stk:slip-httpd.
## Testing
nucleo-h743zi2: builds without errors.
Milandr mdr32: builds and runs with reduced stack footprint (rxslip+txslip 4k->1.5k).
--
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 pull request #6640: net/slip: Rename and clarify orphaned Kconfig options
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #6640:
URL: https://github.com/apache/incubator-nuttx/pull/6640#issuecomment-1189819427
Seems like `TUN_NINTERFACES` also should be renamed to `NET_TUN_NINTERFACES` based on this strategy
--
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] ALTracer commented on pull request #6640: net/slip: Rename and clarify orphaned Kconfig options
Posted by GitBox <gi...@apache.org>.
ALTracer commented on PR #6640:
URL: https://github.com/apache/incubator-nuttx/pull/6640#issuecomment-1190329341
> @ALTracer somehow your search missed `CONFIG_NET_W5500_NINTERFACES`
That's because my workdir is on release/10.3, and W5500 was only recently added to master.
--
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 merged pull request #6640: net/slip: Rename and clarify orphaned Kconfig options
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged PR #6640:
URL: https://github.com/apache/incubator-nuttx/pull/6640
--
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] ALTracer commented on pull request #6640: net/slip: Rename and clarify orphaned Kconfig options
Posted by GitBox <gi...@apache.org>.
ALTracer commented on PR #6640:
URL: https://github.com/apache/incubator-nuttx/pull/6640#issuecomment-1190285198
> Seems like `TUN_NINTERFACES` also should be renamed to `NET_TUN_NINTERFACES` based on this strategy or we should get back from `NET_SLIP_NINTERFACES` to `SLIP_NINTERFACES`
@pkarashchenko That corresponds to drivers/net/tun.c and isn't broken like SLIP was. Do you want the rename just for consistency? Nothing else using the NINTERFACES keys uses the NET_ prefix apart from NET_SLIP and NET_TUN.
Only ENC28J60 and ENCX24J600 (has a copy-paste error in Kconfig) are actual external NICs with SPI interface, DM9X uses some MMIO on ntosd-dm320:nsh.
```sh
$ find . -name Kconfig | xargs -n1 grep -l NINTERFACES
./drivers/lcd/Kconfig
./drivers/net/Kconfig
./drivers/wireless/ieee80211/bcm43xxx/Kconfig
./net/Kconfig
$ find . -name Kconfig | xargs -n1 grep NINTERFACES
config P14201_NINTERFACES
config UG9664HSWAG01_NINTERFACES
config SSD1351_NINTERFACES
config PCD8544_NINTERFACES
config ST7565_NINTERFACES
config ST7567_NINTERFACES
config UG2864AMBAG01_NINTERFACES
config MEMLCD_NINTERFACES
config LCD_ILI9340_NINTERFACES
depends on LCD_ILI9340_NINTERFACES = 1 || LCD_ILI9340_NINTERFACES = 2
depends on LCD_ILI9340_NINTERFACES = 2
config LCD_ILI9341_NINTERFACES
depends on LCD_ILI9341_NINTERFACES = 1 || LCD_ILI9341_NINTERFACES = 2
depends on LCD_ILI9341_NINTERFACES = 2
config DM9X_NINTERFACES
config ENC28J60_NINTERFACES
config ENC28J60_NINTERFACES
config IEEE80211_BROADCOM_NINTERFACES
config NET_SLIP_NINTERFACES
config NET_TUN_NINTERFACES
```
CONFIG_TUN_NINTERFACES is used in C source code. I could make a patch replacing these with CONFIG_NET_TUN_INTERFACES:
```diff
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 61ad6696d0..2bae04620e 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -91,12 +91,12 @@
#define TUNWORK LPWORK
-/* CONFIG_TUN_NINTERFACES determines the number of physical interfaces
+/* CONFIG_NET_TUN_NINTERFACES determines the number of physical interfaces
* that will be supported.
*/
-#ifndef CONFIG_TUN_NINTERFACES
-# define CONFIG_TUN_NINTERFACES 1
+#ifndef CONFIG_NET_TUN_NINTERFACES
+# define CONFIG_NET_TUN_NINTERFACES 1
#endif
/* Make sure that packet buffers include in configured guard size and are an
@@ -227,7 +227,7 @@ static int tun_poll(FAR struct file *filep, FAR struct pollfd *fds,
****************************************************************************/
static struct tun_driver_s g_tun;
-static struct tun_device_s g_tun_devices[CONFIG_TUN_NINTERFACES];
+static struct tun_device_s g_tun_devices[CONFIG_NET_TUN_NINTERFACES];
static const struct file_operations g_tun_file_ops =
{
@@ -1442,7 +1442,7 @@ static int tun_ioctl(FAR struct file *filep, int cmd, unsigned long arg)
}
for (intf = 0;
- intf < CONFIG_TUN_NINTERFACES && !(free_tuns & 1);
+ intf < CONFIG_NET_TUN_NINTERFACES && !(free_tuns & 1);
intf++, free_tuns >>= 1);
ret = tun_dev_init(&g_tun_devices[intf], filep,
@@ -1489,7 +1489,7 @@ int tun_initialize(void)
{
nxsem_init(&g_tun.waitsem, 0, 1);
- g_tun.free_tuns = (1 << CONFIG_TUN_NINTERFACES) - 1;
+ g_tun.free_tuns = (1 << CONFIG_NET_TUN_NINTERFACES) - 1;
register_driver("/dev/tun", &g_tun_file_ops, 0644, &g_tun);
return OK;
diff --git a/net/Kconfig b/net/Kconfig
index 7288e5f3cc..ae90a9a1b0 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -229,7 +229,7 @@ menuconfig NET_TUN
if NET_TUN
-config TUN_NINTERFACES
+config NET_TUN_NINTERFACES
int "Number of TUN interfaces"
default 1
range 1 8
```
Or I could rename NET_SLIP_NINTERFACES back to SLIP_NINTERFACES:
```diff
diff --git a/drivers/net/slip.c b/drivers/net/slip.c
index a4c4608929..de033a4396 100644
--- a/drivers/net/slip.c
+++ b/drivers/net/slip.c
@@ -88,12 +88,12 @@
# error "CONFIG_NET_SLIP_PKTSIZE >= 296 is required"
#endif
-/* CONFIG_NET_SLIP_NINTERFACES determines the number of physical interfaces
+/* CONFIG_SLIP_NINTERFACES determines the number of physical interfaces
* that will be supported.
*/
-#ifndef CONFIG_NET_SLIP_NINTERFACES
-# define CONFIG_NET_SLIP_NINTERFACES 1
+#ifndef CONFIG_SLIP_NINTERFACES
+# define CONFIG_SLIP_NINTERFACES 1
#endif
/* SLIP special character codes ********************************************/
@@ -143,11 +143,11 @@ struct slip_driver_s
* Private Data
****************************************************************************/
-/* We really should get rid of CONFIG_NET_SLIP_NINTERFACES and, instead,
+/* We really should get rid of CONFIG_SLIP_NINTERFACES and, instead,
* kmm_malloc() new interface instances as needed.
*/
-static struct slip_driver_s g_slip[CONFIG_NET_SLIP_NINTERFACES];
+static struct slip_driver_s g_slip[CONFIG_SLIP_NINTERFACES];
/****************************************************************************
* Private Function Prototypes
@@ -445,7 +445,7 @@ static int slip_txtask(int argc, FAR char *argv[])
int ret;
nerr("index: %d\n", index);
- DEBUGASSERT(index < CONFIG_NET_SLIP_NINTERFACES);
+ DEBUGASSERT(index < CONFIG_SLIP_NINTERFACES);
/* Get our private data structure instance and wake up the waiting
* initialization logic.
@@ -667,7 +667,7 @@ static int slip_rxtask(int argc, FAR char *argv[])
int ch;
nerr("index: %d\n", index);
- DEBUGASSERT(index < CONFIG_NET_SLIP_NINTERFACES);
+ DEBUGASSERT(index < CONFIG_SLIP_NINTERFACES);
/* Get our private data structure instance and wake up the waiting
* initialization logic.
@@ -978,7 +978,7 @@ int slip_initialize(int intf, FAR const char *devname)
/* Get the interface structure associated with this interface number. */
- DEBUGASSERT(intf < CONFIG_NET_SLIP_NINTERFACES);
+ DEBUGASSERT(intf < CONFIG_SLIP_NINTERFACES);
priv = &g_slip[intf];
/* Initialize the driver structure */
diff --git a/net/Kconfig b/net/Kconfig
index 7288e5f3cc..af38419b15 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -196,7 +196,7 @@ menuconfig NET_SLIP
if NET_SLIP
-config NET_SLIP_NINTERFACES
+config SLIP_NINTERFACES
int "Number of SLIP interfaces"
default 1
---help---
```
@xiaoxiang781216 Which of the two patch variants do you like better?
_I wish GitHub Markdown dialect supported code spoilers._
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #6640: net/slip: Rename and clarify orphaned Kconfig options
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #6640:
URL: https://github.com/apache/incubator-nuttx/pull/6640#issuecomment-1189838641
@ALTracer could you create a new patch for 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 pull request #6640: net/slip: Rename and clarify orphaned Kconfig options
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #6640:
URL: https://github.com/apache/incubator-nuttx/pull/6640#issuecomment-1190294718
let's keep the things like they are now. I see that variants are inconsistent anyway: `CONFIG_DM9X_NINTERFACES` but `CONFIG_NET_W5500_NINTERFACES`. Some are `NET_` prefixed, but other are not, so let's keep working things.
--
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 pull request #6640: net/slip: Rename and clarify orphaned Kconfig options
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #6640:
URL: https://github.com/apache/incubator-nuttx/pull/6640#issuecomment-1190299463
@ALTracer somehow your search missed `CONFIG_NET_W5500_NINTERFACES`
--
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