RE: [EXT] config_default vs subtargets having their own config_?

2023-03-05 Thread Andy Tang
Hi Tim,

Thanks for doing this. 
I suggest to have config_ for each subtargs in the subtarget dirs..

I have sent the patches for adding the imx8 platform.
But it can't be accepted due to the firmware issue.
Basically the DDR firmware for im8x was released by a self-extracted binary.
When extracted, customer need to accept the EULA which is not accepted by 
OpenWRT community.

Have you solved this problem?

BR,
Andy

> -Original Message-
> From: Tim Harvey 
> Sent: 2023年3月4日 8:20
> To: OpenWrt Development List 
> Cc: Piotr Dymacz ; Andy Tang 
> Subject: [EXT] config_default vs subtargets having their own config_?
> 
> Caution: EXT Email
> 
> Greetings,
> 
> I'm working on cortexa53 support for target/linux/imx for imx8 support and
> am not quite clear how best to take care of kernel config. There is a lot of
> differences in kernel config for
> cortexa7/cortexa9/cortexa53 with regards to IMX SoC's.
> 
> I see some targets have config_default files in the subtarget dirs and others
> have config_.
> 
> What is the recommended way of defining kernel config per subtarget and
> how is it recommended to come up with those split up config files?
> 
> Best Regards,
> 
> Tim
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


[sdwalker/sdwalker.github.io] d7a764: This week's update

2023-03-05 Thread Stephen Walker via openwrt-devel
The sender domain has a DMARC Reject/Quarantine policy which disallows
sending mailing list messages using the original "From" header.

To mitigate this problem, the original message has been wrapped
automatically by the mailing list software.--- Begin Message ---
  Branch: refs/heads/master
  Home:   https://github.com/sdwalker/sdwalker.github.io
  Commit: d7a76403bed484c337cc19d632ef811b0154c3f8
  
https://github.com/sdwalker/sdwalker.github.io/commit/d7a76403bed484c337cc19d632ef811b0154c3f8
  Author: Stephen Walker 
  Date:   2023-03-05 (Sun, 05 Mar 2023)

  Changed paths:
M uscan/index-21.02.html
M uscan/index-22.03.html
M uscan/index.html

  Log Message:
  ---
  This week's update



--- End Message ---
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [PATCH v2] mvebu: add support for Fortinet FortiGate 50E

2023-03-05 Thread INAGAKI Hiroshi

Hi Hauke,

thank you for your review.

On 2023/03/06 1:42, Hauke Mehrtens wrote:

On 3/1/23 17:01, INAGAKI Hiroshi wrote:

Fortinet FortiGate 50E (FG-50E) is a UTM, based on Armada 385 (88F6820).




Notes:

- All "SPEED" LEDs(Green/Amber) of LAN and 1000M "SPEED" LEDs(Green) of
   WAN1/2 are connected to GPIO expander. There is no way to indicate
   link speed of networking device, so those LEDs cannot be used like
   stock firmware.


I think you can use the ledtrig-netdev to activate the LEDs on link up 
if they are connected to the nxp,pca9555 GPIO extender.


I didn't think about it since "LINK/ACT" LEDs already indicate link up, 
but it certainly might be a good alternative for "SPEED" LEDs.





- Both colors of Bi-color LEDs on the front panel cannot be turned on at
   the same time.

- "PWR" and "Logo" LEDs are connected to power source directory.

- The following partitions are added for OpenWrt.
   These partitions are contained in "uboot" partition (0x0-0x1f) on
   stock firmware.

   - "firmware-info"
   - "dtb"
   - "u-boot-env"
   - "board-info"


.

  +define Device/fortinet_fg-50e
+  DEVICE_VENDOR := Fortinet
+  DEVICE_MODEL := FortiGate 50E
+  SOC := armada-385
+  KERNEL := kernel-bin | append-dtb
+  KERNEL_INITRAMFS := kernel-bin | append-dtb | fortigate-header | \
+    gzip-filename FGT50E
+  KERNEL_SIZE := 6144k
+  DEVICE_DTS := armada-385-fortinet-fg-50e
+  IMAGE/sysupgrade.bin := append-rootfs | pad-rootfs | \
+    sysupgrade-tar rootfs=@ | append-metadata
+  DEVICE_PACKAGES := kmod-hwmon-nct7802


Why don't you add the driver for the GPIO extender kmod-gpio-pca953x 
here?


CONFIG_GPIO_PCA953X (and irq option) is enabled in mvebu/config-5.15[1], 
so I didn't add that package.


[1]: 
https://github.com/openwrt/openwrt/blob/a03076cc392b67c8342aac2017f8ac903c983e59/target/linux/mvebu/config-5.15#L191-L192





+endef
+TARGET_DEVICES += fortinet_fg-50e
+
  define Device/globalscale_mirabox
    $(Device/NAND-512K)
    DEVICE_VENDOR := Globalscale




Regards,
Hiroshi

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


[PATCH uqmi v1] uqmi: avoid gcc false error reporting (storing the address of local variable 'complete' in '*req.complete')

2023-03-05 Thread Peter Seiderer
Avoid gcc false error reporting (req->complete is later reset to NULL
in case of use of local complete):

  dev.c:217:23: error: storing the address of local variable 'complete' in 
'*req.complete' [-Werror=dangling-pointer=]
217 | req->complete = 
| ~~^~~

Signed-off-by: Peter Seiderer 
---
 dev.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/dev.c b/dev.c
index bd10207..b8ac273 100644
--- a/dev.c
+++ b/dev.c
@@ -203,6 +203,15 @@ void qmi_request_cancel(struct qmi_dev *qmi, struct 
qmi_request *req)
__qmi_request_complete(qmi, req, NULL);
 }
 
+/* avoid gcc false error reporting:
+ *   dev.c:217:23: error: storing the address of local variable ‘complete’ in 
‘*req.complete’ [-Werror=dangling-pointer=]
+ * 217 | req->complete = 
+ * | ~~^~~
+ */
+#if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 6)
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wdangling-pointer"
+#endif
 int qmi_request_wait(struct qmi_dev *qmi, struct qmi_request *req)
 {
bool complete = false;
@@ -231,6 +240,9 @@ int qmi_request_wait(struct qmi_dev *qmi, struct 
qmi_request *req)
 
return req->ret;
 }
+#if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 6)
+#pragma GCC diagnostic pop
+#endif
 
 struct qmi_connect_request {
struct qmi_request req;
-- 
2.39.2

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [PATCH uqmi] fix uloop initialization

2023-03-05 Thread Hauke Mehrtens

Hi Leon,

Please add a prefix for which application with patch is next time.
git format-patch origin/master --subject-prefix="PATCH uqmi"


On 11/27/22 09:38, Leon M. Busch-George wrote:

uloop_init is already called in main.
uloop_done is just missing.

Signed-off-by: Leon M. Busch-George 
---
  dev.c  | 2 --
  main.c | 2 ++
  2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/dev.c b/dev.c
index bd10207..9eb7209 100644
--- a/dev.c
+++ b/dev.c
@@ -353,8 +353,6 @@ int qmi_device_open(struct qmi_dev *qmi, const char *path)
struct ustream *us = >sf.stream;
int fd;
  
-	uloop_init();

-
fd = open(path, O_RDWR | O_EXCL | O_NONBLOCK | O_NOCTTY);
if (fd < 0)
return -1;


I am not familiar with the uqmi code. Should we move the uloop handling 
completely to the dev.c file?



diff --git a/main.c b/main.c
index aa4634c..e3ccf08 100644
--- a/main.c
+++ b/main.c
@@ -168,5 +168,7 @@ int main(int argc, char **argv)
  
  	qmi_device_close();
  
+	uloop_done();

+
return ret;
  }


Hauke


___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [ubus PATCH] libubus: remove global variables

2023-03-05 Thread Hauke Mehrtens

Hi Simon,

On 1/5/23 15:30, Simon Tate wrote:

Remove the use of global blob_buf and blob_attr variables to allow
for better thread safety with a ctx per thread on client invoke
and sends.

Add the same variables to within each calling function's scope,
encapsulating the memory usage there.

Fixes a multithreaded use case and has been verified 10,000
threads multiple times running invokes and send events.


I like this approach. The disadvantage is that we have more mallocs in 
the code now. Before the global b variable had a pointer to the heap 
which was reused, now we always start with a small buffer on the heap 
and then have to realloc to increase it and free it up when the function 
terminates.


The OpenWrt applications do not make much use of pthreads, but if your 
application is such a change is helpful. I think we can effort the extra 
overhead of the extra mallocs and frees.


I also found there many style problems and some problems in error paths 
in the code.



Signed-off-by: Simon Tate 
---
  cli.c  | 38 --
  libubus-internal.h |  2 +-
  libubus-io.c   | 10 ---
  libubus-obj.c  | 63 ++-
  libubus-req.c  | 44 +-
  libubus-sub.c  | 13 ++---
  libubus.c  | 67 ++
  7 files changed, 161 insertions(+), 76 deletions(-)

diff --git a/cli.c b/cli.c
index 81591ec..e6d7a1b 100644
--- a/cli.c
+++ b/cli.c
@@ -16,7 +16,6 @@
  #include 
  #include "libubus.h"
  
-static struct blob_buf b;

  static int listen_timeout;
  static int timeout = 30;
  static bool simple_output = false;
@@ -140,18 +139,29 @@ static int ubus_cli_call(struct ubus_context *ctx, int 
argc, char **argv)
if (argc < 2 || argc > 3)
return -2;
  
+	struct blob_buf b = { 0 };

blob_buf_init(, 0);
if (argc == 3 && !blobmsg_add_json_from_string(, argv[2])) {
if (!simple_output)
fprintf(stderr, "Failed to parse message data\n");
-   return -1;
+   ret = -1;
+   goto error;
}
  
  	ret = ubus_lookup_id(ctx, argv[0], );

-   if (ret)
-   return ret;
+   if (ret) {
+   goto error;
+   }


Please do not use brackets for only one statement. This is part of the 
Linux kernel code style which is used here. There are multiple places 
where you do not have to add them.



+
+   ret = ubus_invoke(ctx, id, argv[1], b.head, receive_call_result_data, 
NULL, timeout * 1000);
+   if (ret) {
+   goto error;
+   }
  
-	return ubus_invoke(ctx, id, argv[1], b.head, receive_call_result_data, NULL, timeout * 1000);

+error:
+   blob_buf_free();
+
+   return ret;
  }
  
  struct cli_listen_data {

@@ -265,15 +275,23 @@ static int ubus_cli_send(struct ubus_context *ctx, int 
argc, char **argv)
if (argc < 1 || argc > 2)
return -2;
  
+	int ret = UBUS_STATUS_OK;


Initialization is not needed here.
Please move the "int ret;" to the beginning of the function.


+
+   struct blob_buf b = { 0 };
blob_buf_init(, 0);
  
  	if (argc == 2 && !blobmsg_add_json_from_string(, argv[1])) {

if (!simple_output)
fprintf(stderr, "Failed to parse message data\n");
-   return -1;
+   ret = -1;
+   goto error;
}
  
-	return ubus_send_event(ctx, argv[0], b.head);

+   ret = ubus_send_event(ctx, argv[0], b.head);
+
+error:
+   blob_buf_free();
+   return ret;
  }
  
  struct cli_wait_data {

@@ -428,6 +446,7 @@ ubus_cli_get_monitor_data(struct blob_attr *data)
struct blob_attr *tb[UBUS_ATTR_MAX];
int i;
  
+	struct blob_buf b = { 0 };

blob_buf_init(, 0);
blob_parse(data, tb, policy, UBUS_ATTR_MAX);
  
@@ -454,7 +473,10 @@ ubus_cli_get_monitor_data(struct blob_attr *data)

}
}
  
-	return blobmsg_format_json(b.head, true);

+   char* ret = blobmsg_format_json(b.head, true);
+
+   blob_buf_free();
+   return ret;
  }
  
  static void

diff --git a/libubus-internal.h b/libubus-internal.h
index 24477a0..5b23668 100644
--- a/libubus-internal.h
+++ b/libubus-internal.h
@@ -17,7 +17,7 @@
  extern struct blob_buf b;
  extern const struct ubus_method watch_method;
  
-struct blob_attr **ubus_parse_msg(struct blob_attr *msg, size_t len);

+void ubus_parse_msg(struct blob_attr *msg, size_t len, struct blob_attr 
**attrbuf, size_t attrbuf_len);
  bool ubus_validate_hdr(struct ubus_msghdr *hdr);
  void ubus_handle_data(struct uloop_fd *u, unsigned int events);
  int ubus_send_msg(struct ubus_context *ctx, uint32_t seq,
diff --git a/libubus-io.c b/libubus-io.c
index 3561ac4..143d3be 100644
--- a/libubus-io.c
+++ b/libubus-io.c
@@ -41,12 +41,10 @@ static const struct blob_attr_info 
ubus_policy[UBUS_ATTR_MAX] = {

Re: [PATCH v2] mvebu: add support for Fortinet FortiGate 50E

2023-03-05 Thread Hauke Mehrtens

On 3/1/23 17:01, INAGAKI Hiroshi wrote:

Fortinet FortiGate 50E (FG-50E) is a UTM, based on Armada 385 (88F6820).




Notes:

- All "SPEED" LEDs(Green/Amber) of LAN and 1000M "SPEED" LEDs(Green) of
   WAN1/2 are connected to GPIO expander. There is no way to indicate
   link speed of networking device, so those LEDs cannot be used like
   stock firmware.


I think you can use the ledtrig-netdev to activate the LEDs on link up 
if they are connected to the nxp,pca9555 GPIO extender.



- Both colors of Bi-color LEDs on the front panel cannot be turned on at
   the same time.

- "PWR" and "Logo" LEDs are connected to power source directory.

- The following partitions are added for OpenWrt.
   These partitions are contained in "uboot" partition (0x0-0x1f) on
   stock firmware.

   - "firmware-info"
   - "dtb"
   - "u-boot-env"
   - "board-info"


.
  
+define Device/fortinet_fg-50e

+  DEVICE_VENDOR := Fortinet
+  DEVICE_MODEL := FortiGate 50E
+  SOC := armada-385
+  KERNEL := kernel-bin | append-dtb
+  KERNEL_INITRAMFS := kernel-bin | append-dtb | fortigate-header | \
+gzip-filename FGT50E
+  KERNEL_SIZE := 6144k
+  DEVICE_DTS := armada-385-fortinet-fg-50e
+  IMAGE/sysupgrade.bin := append-rootfs | pad-rootfs | \
+sysupgrade-tar rootfs=@ | append-metadata
+  DEVICE_PACKAGES := kmod-hwmon-nct7802


Why don't you add the driver for the GPIO extender kmod-gpio-pca953x here?


+endef
+TARGET_DEVICES += fortinet_fg-50e
+
  define Device/globalscale_mirabox
$(Device/NAND-512K)
DEVICE_VENDOR := Globalscale



___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel