Re: [PATCH] netifd: add support of GRE tunnel ignore-df option

2023-04-25 Thread Denis K
It seems that we have two options: all is good with the tunnel path,
so in this case we use default "pmtudisc noignore-df". Or something
wrong and ICMP replies with "fragmentation needed" do not come back,
so in this case we use "nopmtudisc ignore-df". But I would leave the
possibility of selective control of enabling of these options, just in
case. So, I agree with Philip.

пн, 24 апр. 2023 г. в 17:33, Philip Prindeville
:
>
> I'm not sure you want it to be unconditional.
>
> When I wrote the option for netlink and iproute2, it was for some very 
> specialized scenarios.
>
>
> > On Apr 24, 2023, at 3:14 AM, Stefan Hellermann  
> > wrote:
> >
> > I have an easier patch in my private repo, maybe it's enough without a new 
> > configuration option? I'm using it in production for a gretap link which is 
> > bridged on both sides to MTU 1500 ethernet links.
> >
> > See https://bugzilla.kernel.org/show_bug.cgi?id=14837, which seems to say 
> > you always have to use both options: ignore-df nopmtudisc
> >
> > Signed-off-by: Stefan Hellermann 
> >
> > --- a/system-linux.c
> > +++ b/system-linux.c
> > @@ -3496,6 +3496,7 @@ static int system_add_gre_tunnel(const c
> >  ttl = 64;
> >
> >  nla_put_u8(nlm, IFLA_GRE_PMTUDISC, set_df ? 1 : 0);
> > +nla_put_u8(nlm, IFLA_GRE_IGNORE_DF, set_df ? 0 : 1);
> >
> >  nla_put_u8(nlm, IFLA_GRE_TOS, tos);
> >  }
> >
> > Am 17.04.23 um 11:29 schrieb Denis Kalashnikov:
> >> This is useful for GRE TAP tunnel when tunnel is added to a br-lan bridge.
> >> In this case you need to create it with "nopmtudisc ignore-df". Otherwise
> >> large IP-packets with DF=1 (TCP-data, large pings) will be silently dropped
> >> (since DF=1 but stack failed to send ICMP "need fragmentation" back). But 
> >> with
> >> "ignore-df" packets with DF=1 will be fragmented.
> >>
> >> Signed-off-by: Denis Kalashnikov 
> >> ---
> >>  system-linux.c | 5 +
> >>  system.c   | 1 +
> >>  system.h   | 1 +
> >>  3 files changed, 7 insertions(+)
> >>
> >> diff --git a/system-linux.c b/system-linux.c
> >> index e4041fb..4397460 100644
> >> --- a/system-linux.c
> >> +++ b/system-linux.c
> >> @@ -3500,6 +3500,11 @@ static int system_add_gre_tunnel(const char *name, 
> >> const char *kind,
> >> nla_put_u8(nlm, IFLA_GRE_PMTUDISC, set_df ? 1 : 0);
> >>  + if ((cur = tb[TUNNEL_ATTR_IGNORE_DF])) {
> >> + nla_put_u8(nlm, IFLA_GRE_IGNORE_DF,
> >> + blobmsg_get_bool(cur));
> >> + }
> >> +
> >>   nla_put_u8(nlm, IFLA_GRE_TOS, tos);
> >>   }
> >>  diff --git a/system.c b/system.c
> >> index 32597c1..e773245 100644
> >> --- a/system.c
> >> +++ b/system.c
> >> @@ -21,6 +21,7 @@ static const struct blobmsg_policy 
> >> tunnel_attrs[__TUNNEL_ATTR_MAX] = {
> >>   [TUNNEL_ATTR_REMOTE] = { .name = "remote", .type = BLOBMSG_TYPE_STRING },
> >>   [TUNNEL_ATTR_MTU] = { .name = "mtu", .type = BLOBMSG_TYPE_INT32 },
> >>   [TUNNEL_ATTR_DF] = { .name = "df", .type = BLOBMSG_TYPE_BOOL },
> >> + [TUNNEL_ATTR_IGNORE_DF] = { .name = "ignore-df", .type = 
> >> BLOBMSG_TYPE_BOOL },
> >>   [TUNNEL_ATTR_TTL] = { .name = "ttl", .type = BLOBMSG_TYPE_INT32 },
> >>   [TUNNEL_ATTR_TOS] = { .name = "tos", .type = BLOBMSG_TYPE_STRING },
> >>   [TUNNEL_ATTR_LINK] = { .name = "link", .type = BLOBMSG_TYPE_STRING },
> >> diff --git a/system.h b/system.h
> >> index 1f7037d..a7a713d 100644
> >> --- a/system.h
> >> +++ b/system.h
> >> @@ -29,6 +29,7 @@ enum tunnel_param {
> >>   TUNNEL_ATTR_LOCAL,
> >>   TUNNEL_ATTR_MTU,
> >>   TUNNEL_ATTR_DF,
> >> + TUNNEL_ATTR_IGNORE_DF,
> >>   TUNNEL_ATTR_TTL,
> >>   TUNNEL_ATTR_TOS,
> >>   TUNNEL_ATTR_LINK,
> > ___
> > openwrt-devel mailing list
> > openwrt-devel@lists.openwrt.org
> > https://lists.openwrt.org/mailman/listinfo/openwrt-devel
>

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


Re: [PATCH v2 1/2] ath79: add support for reset key on MikroTik RB912UAG-2HPnD

2022-01-17 Thread Denis K
> I'm seeing this in the bootlogs when using this patch:
>
> [5.183305] gpio-latch gpio_latch: failed to get gpio 7: -517
> [5.235889] rb91x-nand nand_gpio: failed to get gpios: -517

It's okay. The gpio-latch probe function seems to be called before the
rb91x-key probe, but it also returns  EPROBE_DEFER (-517), so it will
be called later. I've tested master with reset key patch and Koen's
patch that sets ref clock freq to 2500. All is working: nand,
leds, key and Gigabit Ethernet (setting 100 -- pings okay, setting
back 1000 -- pings still okay).

Should I delete from my rb91x-key patch support for kernel 5.4 and submit v2?

Thanks,
Denis

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


Re: [PATCH v2 1/2] ath79: add support for reset key on MikroTik RB912UAG-2HPnD

2021-12-10 Thread Denis K
> Added to my staging tree.
Thank you, Koen! It is very good news for me this Friday night.

Regards, Denis

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


Re: [PATCH v2 1/2] ath79: add support for reset key on MikroTik RB912UAG-2HPnD

2021-12-10 Thread Denis K
Mikrotik RB912UAG needs to be better supported in ath79, imho
and I'm working on it. Reset key, Gigabit Ethernet, mPCIe slot, UART
-- all need to be fixed. This patch adds support for reset key, next
one I'm working on will fix mPCIe slot. No ideas about how to fix
Gigabit, sigh.

Guys, please tell me what should I change in this patch to make
it useful for upstream?

Regards, Denis Kalashnikov

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


ath79: MikroTik RB912UAG: not working Wi-Fi card in mPCIe slot

2021-12-10 Thread Denis K
> Thomas Hühn  wrote:
>
> We have 5 Mikrotik 912UAG in our Freifunk Network and just build and updated
> them today with latest tunk .. we moved vom ar71xx to ath79 successfully,
> so far so good .. thx for your upstream work!
>
> The only thing that is not working: we can not get your 2nd wifi card in the
> mPCIe port up and running. Only the USB Port is working. We have tried
> several things to switch from USB to mPCIe .. without success:
>
> * change the dts file ar9342_mikrotik_routerboard-912uag-2hpnd.dts in
>   the gpio-export section to have usb_power output = 0 and pcie_power = 1
>   mPCIe port is powered with 3,3V (measured on in 2 & 4) but mPCIe WiFi
>   cards (ath9k - mikrotik) are not detected, tried to rescan the PCI bus
>   as well
> * change on a running 912UAG i the /sys/class/gpio/power_usb
>   ../power-pcie/values ... not WiFi Card detection
> * compare the running und working mPCIe port ar71xx image on the router ...
>   found out there is gpio52 used, but different latch setup .. our good
>   gues was that GPIO20 could be responsible for the usb switch for the mPCIe
>   port .. add gpio20 to the export folder and set it to 1.. but no WiFi card
>   detection
>
> Who has some troubleshooting tips for how to disable the usb port and enable
> the mPCIe port?

I've done some tests with RB912UAG-2HPnD and R11e-5HacT.
It seems that you need to set power-pcie gpio before pci controller
driver starts. Since it doesn't support hotplug or rescan (I suppose).
gpio-export starts after, so I've used this temporarily hack in ssr
node in dts (gpio-hog):

ssr: ssr@1 {
compatible = "fairchild,74hc595";
gpio-controller;
#gpio-cells = <2>;
registers-number = <1>;
reg = <1>;
spi-max-frequency = <5000>;

power_pcie {
gpio-hog;
gpios = <7 0>;
output-high;
};

power_usb {
gpio-hog;
gpios = <6 0>;
output-low;
};
};

root@OpenWrt:~# lspci
00:00.0 Class 0280: 168c:003c

This workaround works, but it is better to make pci rescan working.

power_usb and power_pcie gpio lines are on the Serial Shift Register (ssr).
It seems that RouterBoot left both =1. But SSR while init set them =0. Then
the PCI controller (arch/mips/pci-ar724x) starts... and says: "PCIe link is
down", remembers this in the internal link_up field and never re-check
AR724x_PCI_RESET_LINK_UP register bit. When link_up is false, pci read/write
handlers always return error. When I patched it to actually check link status
before reading, I had kernel panic with Device bus error when trying to read
Vendor ID from config register (see "MIPS: pci-ar724x: avoid data bus error
due to a missing PCIe module" Gabor Juhos patch). It seems that we need to
somehow reinit the PCI host controller.

Does anyone have any ideas?

Regards, Denis

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


Re: [PATCH 2/2] ath79: mikrotik: Change the moment of routerboot partition parser init

2021-11-16 Thread Denis K
Thank you for your help, Thibaut. I think we've found the right way of
initializing routerboot. I've sent a v2 of the patch.

Denis

вт, 16 нояб. 2021 г. в 13:20, Thibaut :
>
> Hi,
>
> > Le 16 nov. 2021 à 10:46, Denis K  a écrit :
> >
> > I got it! _exit macro of rb_hardconfig_init seems to cause this kernel
> > panic. Here is a patch.
>
> Thanks; indeed that’s logical: functions marked as __exit are discarded when 
> the module is built into the kernel: they’re no longer available when the 
> mtd_notifier executes the remove() callback. I missed that. Since these are 
> very small, I agree it’s ok to remove the __exit qualifier.
>
> I see the same rationale for removing the __init qualifier from the init 
> functions since the mtd notifier could be called at any point during kernel 
> execution, but that bothers me a bit more because these are somewhat large 
> functions. Then again it’s a tradeoff between doing the right thing (using a 
> notifier) vs using a workaround (late_initcall()). I think it’s acceptable.
>
> Anyway, I’ve refactored the patch with your changes and I added a note and 
> edited the commit message about the point above; see attachment below. 
> Hopefully this one is good to go :)
>
> You are the author of this patch and you did all the heavy lifting (thanks 
> again for that), so if you have no further changes/comments I’d suggest you 
> submit this as a v2. I’ll ACK it then (please CC-me - this should be 
> automatic with my SoB-line added).
>
> Thanks,
> Thibaut
>

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


Re: [PATCH 2/2] ath79: mikrotik: Change the moment of routerboot partition parser init

2021-11-16 Thread Denis K
I got it! _exit macro of rb_hardconfig_init seems to cause this kernel
panic. Here is a patch.

Regards, Denis.
From e042f7d0a72ff9b69fd34ab2f3bac2b698da776f Mon Sep 17 00:00:00 2001
From: Denis Kalashnikov 
Date: Mon, 15 Nov 2021 11:27:14 +0300
Subject: [PATCH] Fixes to generic: platform/mikrotik: use MTD notifier

---
 .../files/drivers/platform/mikrotik/rb_hardconfig.c  | 9 -
 .../files/drivers/platform/mikrotik/rb_softconfig.c  | 7 +++
 .../generic/files/drivers/platform/mikrotik/routerboot.c | 1 +
 .../generic/files/drivers/platform/mikrotik/routerboot.h | 8 
 4 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/target/linux/generic/files/drivers/platform/mikrotik/rb_hardconfig.c b/target/linux/generic/files/drivers/platform/mikrotik/rb_hardconfig.c
index 1f076a9666..5ae798b3ce 100644
--- a/target/linux/generic/files/drivers/platform/mikrotik/rb_hardconfig.c
+++ b/target/linux/generic/files/drivers/platform/mikrotik/rb_hardconfig.c
@@ -676,7 +676,7 @@ static ssize_t hc_wlan_data_bin_read(struct file *filp, struct kobject *kobj,
 	return count;
 }
 
-int __init rb_hardconfig_init(struct kobject *rb_kobj, struct mtd_info *mtd)
+int rb_hardconfig_init(struct kobject *rb_kobj, struct mtd_info *mtd)
 {
 	struct kobject *hc_wlan_kobj;
 	size_t bytes_read, buflen, outlen;
@@ -689,8 +689,7 @@ int __init rb_hardconfig_init(struct kobject *rb_kobj, struct mtd_info *mtd)
 	hc_kobj = NULL;
 	hc_wlan_kobj = NULL;
 
-	ret = __get_mtd_device(mtd);
-	if (IS_ERR(ret))
+	if (__get_mtd_device(mtd))
 		return -ENODEV;
 
 	hc_buflen = mtd->size;
@@ -816,10 +815,10 @@ fail:
 	return ret;
 }
 
-void __exit rb_hardconfig_exit(void)
+void rb_hardconfig_exit(void)
 {
 	kobject_put(hc_kobj);
 	hc_kobj = NULL;
 	kfree(hc_buf);
-	hc_buff = NULL;
+	hc_buf = NULL;
 }
diff --git a/target/linux/generic/files/drivers/platform/mikrotik/rb_softconfig.c b/target/linux/generic/files/drivers/platform/mikrotik/rb_softconfig.c
index 3bbf4038ae..575ef60e33 100644
--- a/target/linux/generic/files/drivers/platform/mikrotik/rb_softconfig.c
+++ b/target/linux/generic/files/drivers/platform/mikrotik/rb_softconfig.c
@@ -705,7 +705,7 @@ mtdfail:
 
 static struct kobj_attribute sc_kattrcommit = __ATTR(commit, RB_SC_RMODE|RB_SC_WMODE, sc_commit_show, sc_commit_store);
 
-int __init rb_softconfig_init(struct kobject *rb_kobj, struct mtd_info *mtd)
+int rb_softconfig_init(struct kobject *rb_kobj, struct mtd_info *mtd)
 {
 	size_t bytes_read, buflen;
 	const u8 *buf;
@@ -715,8 +715,7 @@ int __init rb_softconfig_init(struct kobject *rb_kobj, struct mtd_info *mtd)
 	sc_buf = NULL;
 	sc_kobj = NULL;
 
-	ret = __get_mtd_device(mtd);
-	if (IS_ERR(ret))
+	if (__get_mtd_device(mtd))
 		return -ENODEV;
 
 	sc_buflen = mtd->size;
@@ -797,7 +796,7 @@ fail:
 	return ret;
 }
 
-void __exit rb_softconfig_exit(void)
+void rb_softconfig_exit(void)
 {
 	kobject_put(sc_kobj);
 	sc_kobj = NULL;
diff --git a/target/linux/generic/files/drivers/platform/mikrotik/routerboot.c b/target/linux/generic/files/drivers/platform/mikrotik/routerboot.c
index 13ecfab2ba..9cc080779a 100644
--- a/target/linux/generic/files/drivers/platform/mikrotik/routerboot.c
+++ b/target/linux/generic/files/drivers/platform/mikrotik/routerboot.c
@@ -13,6 +13,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "routerboot.h"
 
diff --git a/target/linux/generic/files/drivers/platform/mikrotik/routerboot.h b/target/linux/generic/files/drivers/platform/mikrotik/routerboot.h
index 747136b2c1..e858a524af 100644
--- a/target/linux/generic/files/drivers/platform/mikrotik/routerboot.h
+++ b/target/linux/generic/files/drivers/platform/mikrotik/routerboot.h
@@ -25,11 +25,11 @@
 int routerboot_tag_find(const u8 *bufhead, const size_t buflen, const u16 tag_id, u16 *pld_ofs, u16 *pld_len);
 int routerboot_rle_decode(const u8 *in, size_t inlen, u8 *out, size_t *outlen);
 
-int __init rb_hardconfig_init(struct kobject *rb_kobj, struct mtd_info *mtd);
-void __exit rb_hardconfig_exit(void);
+int rb_hardconfig_init(struct kobject *rb_kobj, struct mtd_info *mtd);
+void rb_hardconfig_exit(void);
 
-int __init rb_softconfig_init(struct kobject *rb_kobj, struct mtd_info *mtd);
-void __exit rb_softconfig_exit(void);
+int rb_softconfig_init(struct kobject *rb_kobj, struct mtd_info *mtd);
+void rb_softconfig_exit(void);
 
 ssize_t routerboot_tag_show_string(const u8 *pld, u16 pld_len, char *buf);
 ssize_t routerboot_tag_show_u32s(const u8 *pld, u16 pld_len, char *buf);
-- 
2.31.1

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


Re: [PATCH 2/2] ath79: mikrotik: Change the moment of routerboot partition parser init

2021-11-15 Thread Denis K
I've tested it (on 5.4 and 5.10). On reboot I've had this kernel panic:

[ 1028.460043] Reserved instruction in kernel code[#1]:
[ 1028.465191] CPU: 0 PID: 2406 Comm: procd Not tainted 5.4.158 #0
[ 1028.471300] $ 0   :  0001  
[ 1028.476702] $ 4   : 8342118c 806adf50  
[ 1028.482095] $ 8   : 83812ab0  83812ad4 0002
[ 1028.487490] $12   : fffd 0402 80731904 0040
[ 1028.492883] $16   : 8073503c 83419000 807318ec 8073
[ 1028.498278] $20   : 8073 808a 806a9efc 806a9eec
[ 1028.503671] $24   :  
[ 1028.509065] $28   : 83c42000 83c43b80 83cefc00 803c1fd0
[ 1028.514459] Hi: 00474bff
[ 1028.517426] Lo: b47b3346
[ 1028.520400] epc   : 80764000 0x80764000
[ 1028.524371] ra: 803c1fd0 del_mtd_device+0x68/0x100
[ 1028.529673] Status: 1100dc03KERNEL EXL IE
[ 1028.533994] Cause : 00800028 (ExcCode 0a)
[ 1028.538135] PrId  : 0001974c (MIPS 74Kc)
[ 1028.542181] Modules linked in: ath9k ath9k_common pppoe ppp_async
iptable_nat ath9k_hw ath xt_state xt_nat xt_conntrack xt_REDIRECT
xt_MASQUERADE xt_FLOWOFFLOAD pppox ppp_generic nf_nat nf_flow_table_hw
nf_flow_table nf_conntrack mac80211 ipt_REJECT cfg80211 xt_time
xt_tcpudp xt_multiport xt_mark xt_mac xt_limit xt_comment xt_TCPMSS
xt_LOG slhc nf_reject_ipv4 nf_log_ipv4 nf_defrag_ipv6 nf_defrag_ipv4
iptable_mangle iptable_filter ip_tables crc_ccitt compat evdev
input_core nf_log_ipv6 nf_log_common ip6table_mangle ip6table_filter
ip6_tables ip6t_REJECT x_tables nf_reject_ipv6 sha256_generic
libsha256 seqiv jitterentropy_rng drbg hmac ghash_generic gf128mul gcm
ctr cmac ccm fsl_mph_dr_of ehci_platform ehci_fsl ehci_hcd
gpio_button_hotplug usbcore nls_base usb_common aead cryptomgr
crypto_null crypto_hash
[ 1028.615764] Process procd (pid: 2406, threadinfo=2c3273bc,
task=f31e3962, tls=77ef0dcc)
[ 1028.624021] Stack :  802183b4 8068 8066ecb0 80731768
83419000 80731978 
[ 1028.632649] 8073 803c4a5c 8073 0001 83c09400
802150e8 000b0054 83418400
[ 1028.641278] 83ff4000 80731978  803c4a1c 83ff1360
 0001 808a
[ 1028.649906] 83ff1360 80731768 83d99880 80731978 
803c63a4 806a9efc 806a9eec
[ 1028.658535] 83cefc00 83d01d40 83d99880 80733b18 
80733b18 0044 803c2248
[ 1028.667163] ...
[ 1028.669683] Call Trace:
[ 1028.669685]
[ 1028.673757] [<802183b4>] sysfs_remove_files+0x38/0x5c
[ 1028.679019] [<803c4a5c>] __mtd_del_partition+0xa8/0x100
[ 1028.684439] [<802150e8>] __kernfs_remove.part.0+0x1e4/0x318
[ 1028.690204] [<803c4a1c>] __mtd_del_partition+0x68/0x100
[ 1028.695604] [<803c63a4>] del_mtd_partitions+0x78/0xf0
[ 1028.700826] [<803c2248>] mtd_device_unregister+0x28/0x5c
[ 1028.706314] [<803a1c80>] __device_release_driver+0x178/0x214
[ 1028.712159] [<8039fdf0>] klist_devices_put+0x0/0x8
[ 1028.717105] [<803a1d48>] device_release_driver+0x2c/0x44
[ 1028.722589] [<803a0f68>] bus_remove_device+0x154/0x168
[ 1028.727905] [<803b7944>] __unregister+0x0/0x20
[ 1028.732522] [<8039cd80>] device_del+0x15c/0x458
[ 1028.737228] [<803428e0>] spi_sync_transfer.constprop.0+0x60/0x6c
[ 1028.743438] [<803b7348>] spi_complete+0x0/0x8
[ 1028.747936] [<803b7944>] __unregister+0x0/0x20
[ 1028.752524] [<803b7910>] spi_unregister_device+0x40/0x74
[ 1028.758007] [<803b7954>] __unregister+0x10/0x20
[ 1028.762685] [<803b7944>] __unregister+0x0/0x20
[ 1028.767277] [<8039bd78>] device_for_each_child+0x50/0xa4
[ 1028.772778] [<803b903c>] spi_unregister_controller+0x3c/0x170
[ 1028.778724] [<803bdb90>] ath79_spi_remove+0x1c/0x78
[ 1028.783766] [<8039f658>] device_shutdown+0x13c/0x1f4
[ 1028.788897] [<800a4814>] blocking_notifier_call_chain+0x74/0xe8
[ 1028.795019] [<800a5d6c>] kernel_restart+0x40/0xac
[ 1028.799877] [<800a5ed8>] __do_sys_reboot+0x100/0x214
[ 1028.805037] [<8012fdc8>] filemap_map_pages+0x3a0/0x3d0
[ 1028.810352] [<801617e4>] handle_mm_fault+0x89c/0xcf0
[ 1028.815520] [<8006fdec>] do_page_fault+0xb4/0x4b8
[ 1028.820379] [<8006e18c>] syscall_common+0x34/0x58
[ 1028.825233]
[ 1028.826769] Code:      <7f454c46> 01020100
    00030008  0001
[ 1028.836839]
[ 1028.838458] ---[ end trace 37f524f87b727ac2 ]---
[ 1028.843240] Kernel panic - not syncing: Fatal exception
[ 1028.848639] Rebooting in 3 seconds..

If I comment out calls of rb_hardconfig_exit and rb_softconfig_exit in
routerboot_mtd_notifier_remove, there is no panic. I'm trying to
determine why this is happening.

Regards, Denis

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


Re: [PATCH 2/2] ath79: mikrotik: Change the moment of routerboot partition parser init

2021-11-12 Thread Denis K
> OK, let’s do that as a workaround and hope that deferred probes won’t overlap 
> past late_initcall()
What about using register_mtd_user function: routerboot can call
rb_hardconfig_init and rb_softconfig_init when mtd subsystem notifies
it about a new mtd device? It works.

---
 .../drivers/platform/mikrotik/rb_hardconfig.c | 12 --
 .../drivers/platform/mikrotik/rb_softconfig.c | 12 --
 .../drivers/platform/mikrotik/routerboot.c| 24 +--
 .../drivers/platform/mikrotik/routerboot.h|  5 ++--
 4 files changed, 30 insertions(+), 23 deletions(-)

diff --git 
a/target/linux/generic/files/drivers/platform/mikrotik/rb_hardconfig.c
b/target/linux/generic/files/drivers/platform/mikrotik/rb_hardconfig.c
index e6a6928896..724851474e 100644
--- a/target/linux/generic/files/drivers/platform/mikrotik/rb_hardconfig.c
+++ b/target/linux/generic/files/drivers/platform/mikrotik/rb_hardconfig.c
@@ -676,10 +676,9 @@ static ssize_t hc_wlan_data_bin_read(struct file
*filp, struct kobject *kobj,
 return count;
 }

-int __init rb_hardconfig_init(struct kobject *rb_kobj)
+int __init rb_hardconfig_init(struct kobject *rb_kobj, struct mtd_info *mtd)
 {
 struct kobject *hc_wlan_kobj;
-struct mtd_info *mtd;
 size_t bytes_read, buflen, outlen;
 const u8 *buf;
 void *outbuf;
@@ -690,20 +689,19 @@ int __init rb_hardconfig_init(struct kobject *rb_kobj)
 hc_kobj = NULL;
 hc_wlan_kobj = NULL;

-// TODO allow override
-mtd = get_mtd_device_nm(RB_MTD_HARD_CONFIG);
-if (IS_ERR(mtd))
+ret = __get_mtd_device(mtd);
+if (IS_ERR(ret))
 return -ENODEV;

 hc_buflen = mtd->size;
 hc_buf = kmalloc(hc_buflen, GFP_KERNEL);
 if (!hc_buf) {
-put_mtd_device(mtd);
+__put_mtd_device(mtd);
 return -ENOMEM;
 }

 ret = mtd_read(mtd, 0, hc_buflen, _read, hc_buf);
-put_mtd_device(mtd);
+__put_mtd_device(mtd);

 if (ret)
 goto fail;
diff --git 
a/target/linux/generic/files/drivers/platform/mikrotik/rb_softconfig.c
b/target/linux/generic/files/drivers/platform/mikrotik/rb_softconfig.c
index 070bd32d5a..59c42e4cf4 100644
--- a/target/linux/generic/files/drivers/platform/mikrotik/rb_softconfig.c
+++ b/target/linux/generic/files/drivers/platform/mikrotik/rb_softconfig.c
@@ -705,9 +705,8 @@ mtdfail:

 static struct kobj_attribute sc_kattrcommit = __ATTR(commit,
RB_SC_RMODE|RB_SC_WMODE, sc_commit_show, sc_commit_store);

-int __init rb_softconfig_init(struct kobject *rb_kobj)
+int __init rb_softconfig_init(struct kobject *rb_kobj, struct mtd_info *mtd)
 {
-struct mtd_info *mtd;
 size_t bytes_read, buflen;
 const u8 *buf;
 int i, ret;
@@ -716,20 +715,19 @@ int __init rb_softconfig_init(struct kobject *rb_kobj)
 sc_buf = NULL;
 sc_kobj = NULL;

-// TODO allow override
-mtd = get_mtd_device_nm(RB_MTD_SOFT_CONFIG);
-if (IS_ERR(mtd))
+ret = __get_mtd_device(mtd);
+if (IS_ERR(ret))
 return -ENODEV;

 sc_buflen = mtd->size;
 sc_buf = kmalloc(sc_buflen, GFP_KERNEL);
 if (!sc_buf) {
-put_mtd_device(mtd);
+__put_mtd_device(mtd);
 return -ENOMEM;
 }

 ret = mtd_read(mtd, 0, sc_buflen, _read, sc_buf);
-put_mtd_device(mtd);
+__put_mtd_device(mtd);

 if (ret)
 goto fail;
diff --git a/target/linux/generic/files/drivers/platform/mikrotik/routerboot.c
b/target/linux/generic/files/drivers/platform/mikrotik/routerboot.c
index 4c8c0bfac5..77f39709ce 100644
--- a/target/linux/generic/files/drivers/platform/mikrotik/routerboot.c
+++ b/target/linux/generic/files/drivers/platform/mikrotik/routerboot.c
@@ -160,25 +160,35 @@ fail:
 return ret;
 }

+static void routerboot_mtd_notifier_add(struct mtd_info *mtd)
+{
+if (mtd->type != MTD_NORFLASH)
+return;
+if (!strcmp(mtd->name, RB_MTD_HARD_CONFIG)) {
+rb_hardconfig_init(rb_kobj, mtd);
+} else if (!strcmp(mtd->name, RB_MTD_SOFT_CONFIG)) {
+rb_softconfig_init(rb_kobj, mtd);
+}
+}
+
+static struct mtd_notifier routerboot_mtd_notifier = {
+.add = routerboot_mtd_notifier_add,
+};
+
 static int __init routerboot_init(void)
 {
 rb_kobj = kobject_create_and_add("mikrotik", firmware_kobj);
 if (!rb_kobj)
 return -ENOMEM;

-/*
- * We ignore the following return values and always register.
- * These init() routines are designed so that their failed state is
- * always manageable by the corresponding exit() calls.
- */
-rb_hardconfig_init(rb_kobj);
-rb_softconfig_init(rb_kobj);
+register_mtd_user(_mtd_notifier);

 return 0;
 }

 static void __exit routerboot_exit(void)
 {
+unregister_mtd_user(_mtd_notifier);
 rb_softconfig_exit();
 rb_hardconfig_exit();
 kobject_put(rb_kobj);// recursive afaict
diff --git a/target/linux/generic/files/drivers/platform/mikrotik/routerboot.h
b/target/linux/generic/files/drivers/platform/mikrotik/routerboot.h
index 

Re: [PATCH 2/2] ath79: mikrotik: Change the moment of routerboot partition parser init

2021-11-11 Thread Denis K
Hello, Thibaut!

> because the driver depends on SPI routines, it should be linked and loaded 
> after SPI
> rb_hardconfig should never be loaded where it is: platform drivers (where it 
> is currently hooked)
are linked much later in kernel

Maybe the order of init functions of SPI and routerboot depends on a link
order. But SPI probe function can be called in any time: before or after
routerboot init. Adding one more driver -- a new rb91x-key somehow
shifts init timings so SPI _probe_ (not init) now is called after routerboot
init. I suppose that SPI probe is called later now due to several deferred
probes (spi, gpio-latch and rb91x-nand), you can see this in the attached
dmesg:

[0.348321] gpio-latch gpio_latch: failed to get gpio 7: -517
[0.355504] gpio-rb91x-key gpio_key: probe ok
...
[0.405835] rb91x-nand nand_gpio: failed to get gpios: -517
...
[0.806348] rb_hardconfig_init: get_mtd_device_nm returns error: -19
...
[0.909300] gpio-latch gpio_latch: probe ok
[0.927785] spi-nor spi0.0: w25x05 (64 Kbytes)
[0.932811] 1 routerbootpart partitions found on MTD device spi0.0
[0.939189] Creating 1 MTD partitions on "spi0.0":
[0.944193] 0x-0x0001 : "partitions"
[0.952617] 4 routerbootpart partitions found on MTD device partitions
[0.959369] Creating 4 MTD partitions on "partitions":
[0.964750] 0x-0xc000 : "routerboot"
[0.971481] 0xc000-0xd000 : "hard_config"
[0.978215] 0xd000-0xe000 : "bios"
[0.984340] 0xe000-0xf000 : "soft_config"
[0.991016] spi-nor spi0.0: probe ok
...
[1.118306] rb91x-nand nand_gpio: probe ok

> I see this is using kernel 5.10 but afaict master is still using 5.4 for 
> ath79.
> Maybe this is related?
The same thing is when kernel 5.4 is used (see the attachment).

I see two ways to eliminate this race condition:
1) late_initcall of routerboot module, only a single line need to be changed,
2) Somehow add a probe function to routerboot module. May be it should be
called from routerboot_partitions_parse function (from
mtd/parsers/routerbootpart.c).

Regards, Denis.


dmesg.5.4.gz
Description: application/gzip
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [PATCH 2/2] ath79: mikrotik: Change the moment of routerboot partition parser init

2021-11-10 Thread Denis K
Hello, Thibaut! Here is a dmesg output as an attachment.

I've added printk into rb_hardconfig_init:
[0.806327] rb_hardconfig_init: get_mtd_device_nm returns error: -19


dmesg.gz
Description: application/gzip
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [PATCH 2/2] ath79: mikrotik: Change the moment of routerboot partition parser init

2021-11-10 Thread Denis K
> Hi, wouldn't it be better for the rb_hardconfig_init to return
> -EPROBE_DEFER so the
> the core can try probing later?
Hello, Robert! I've tried what you suggested (return -EPROBE_DEFER
from rb_hardconfig_init and routerboot_init). It doesn't work.
do_one_initcall() from linux-5.4.158/init/main.c calls module init
function and it doesn't check return code and recall it later.

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


Re: [RFC v2 3/3] ath79: add support for Mikrotik RouterBoard 912G

2021-05-26 Thread Denis K
> I am just curious, why did you place beeper and LEDs in the root node,
> but specify latch and NAND as the AHB bus child nodes?
Ack, you are right.

> > +   /* NAND control gpios */
> > +   read-gpios = <_latch 3 GPIO_ACTIVE_HIGH>;
> > +   rdy-gpios  = <_latch 4 GPIO_ACTIVE_HIGH>;
> > +   nce-gpios  = <_latch 5 GPIO_ACTIVE_LOW>; /* Chip 
> > Enable */
> > +   cle-gpios  = <_latch 6 GPIO_ACTIVE_HIGH>; /* Command 
> > Latch */
> > +   ale-gpios  = <_latch 7 GPIO_ACTIVE_HIGH>; /* Address 
> > Latch */
> > +   nrw-gpios  = < 12 GPIO_ACTIVE_LOW>; /* Read/Write */
>
> Why did you use so complex set of properties for configuration? You
> anyway add comments to a large portion of properties. So why do not
> follow the 'gpio-control-nand' driver approach and simply use a single
> 'gpios' array?
I'll fix this in v3.

Denis

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


Re: [RFC v2 3/3] ath79: add support for Mikrotik RouterBoard 912G

2021-05-26 Thread Denis K
Hello Sergey

> the device tree has a dualistic
> nature, on one hand it is a place for driver configuration, on the
> other hand it is a way to document board stuff interconnections
I agree with you here, but I don't think that I should insist on this.

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


Re: [RFC v2 3/3] ath79: add support for Mikrotik RouterBoard 912G

2021-05-26 Thread Denis K
Hello Adrian

> > + {
> > + status = "okay";
> > +
> > + compatible = "qca,ar7100-spi";
> > +
> > + cs-gpios = <0>, <_latch 0 GPIO_ACTIVE_LOW>;
>
> Are you sure you need cs-gpios here? We've removed it everywhere else (for 
>  node):
> https://github.com/openwrt/openwrt/commit/e5f81ea3fe79ad484d454f5959814b3a1b094dcb

On this board for the serial shift register a separate gpio line is
used as nCS, not the SoC SPI controller dedicated nCS line. So,
without cs-gpios DTS property the serial shift register doesn't work.

Denis.

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