Re: [PATCH] octeon: fix unstable eMMC on ER-8

2022-08-09 Thread Yu Zhao 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 ---
On Mon, Aug 1, 2022 at 1:50 AM Yu Zhao  wrote:
>
> Two ER-8 devices complained rootfs corruption and failed to finish
> booting [1] after they were flashed to OpenWrt. The problem was always
> reproducible unless those device were powered on after they had been
> off for a while. The two devices seem to be in different hardware
> versions but neither has a clear marking of version.

Following up on this -- I'm hoping to get this patch into the next rc.

Thanks.

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


Re: [RFC PATCH 2/7] realtek: add switch core MFD driver

2022-08-09 Thread Sander Vanheule
Hi,

On Tue, 2022-08-09 at 17:57 +0200, Olliver Schinagl wrote:
> Hey Sander,
> 
> On 29-07-2022 22:19, Sander Vanheule wrote:
> > On Fri, 2022-07-29 at 14:58 +0200, Olliver Schinagl wrote:
> > > Hey Sander,
> > > 
> > > On 16-07-2022 21:09, Sander Vanheule wrote:
> > > > Realtek managed switch SoCs such as the RTL8380 consist of a MIPS CPU
> > > > with a number of basic peripherals, and an ethernet switch peripheral.
> > > > Besides performing ethernet related tasks, this switch core also
> > > > provides SoC management features. These switch core features are badly
> > > > separated, and cannot be divided into distinct IO ranges.
> > > > 
> > > > This MFD core driver is intended to manage the switch core regmap, and
> > > > to expose some limited features that don't warrant their own driver.
> > > > These include SoC identification and system LED management.
> > > > 
> > > > Signed-off-by: Sander Vanheule 
> > > > ---
> > > >    .../drivers/mfd/realtek-switchcore.c  | 244
> > > > ++
> > > >    ...0-mfd-add-Realtek-switch-core-driver.patch |  50 
> > > >    target/linux/realtek/rtl838x/config-5.10  |   2 +
> > > >    3 files changed, 296 insertions(+)
> > > >    create mode 100644 target/linux/realtek/files-
> > > > 5.10/drivers/mfd/realtek-switchcore.c
> > > >    create mode 100644 target/linux/realtek/patches-5.10/200-mfd-add-
> > > > Realtek-switch-
> > > > core-driver.patch
> > > > 
> > > > diff --git a/target/linux/realtek/files-5.10/drivers/mfd/realtek-
> > > > switchcore.c
> > > > b/target/linux/realtek/files-5.10/drivers/mfd/realtek-switchcore.c
> > > > new file mode 100644
> > > > index ..5b3f6eee6fe2
> > > > --- /dev/null
> > > > +++ b/target/linux/realtek/files-5.10/drivers/mfd/realtek-switchcore.c
> > > > @@ -0,0 +1,244 @@
> > > > +// SPDX-License-Identifier: GPL-2.0-or-later
> > > > +
> > > > +#include 
> > > Probably want bitops.h here as well, as BIT() and GENMASK() come from
> > > there
> > Will add.
> > 
> > > > +#include 
> > > > +#include 
> > > > +#include 
> > > > +#include 
> > > > +#include 
> > > > +#include 
> > > > +#include 
> > > > +#include 
> > > > +#include 
> > > > +
> > > > +struct realtek_switchcore_ctrl;
> > > I take it a switchcore is the generic name, and an rtl8380 etc just
> > > implements one? Makes sense if so.
> > Switchcore is the name of the ASIC peripheral that handles the networking,
> > and is really
> > the system controller as well, managing pin muxes and such. RTL838x (Maple)
> > SoC have one
> > register layout, RTL839x (Cypress) has another set. In one generation
> > (RTL83xx, RTL93xx)
> > the provided features are usually similar, but every SoC type really
> > requires its own
> > implementation.
> > 
> > > > +
> > > > +struct realtek_switchcore_data {
> > > > +   struct reg_field sys_led_field;
> > > > +   const struct mfd_cell *mfd_devices;
> > > > +   unsigned int mfd_device_count;
> > > > +   void (*probe_model_name)(const struct realtek_switchcore_ctrl
> > > > *ctrl);
> > > > +};
> > > > +
> > > > +struct realtek_switchcore_ctrl {
> > > > +   struct device *dev;
> > > > +   struct regmap *map;
> > > > +   const struct realtek_switchcore_data *data;
> > > > +   struct led_classdev sys_led;
> > > > +   struct regmap_field *sys_led_field;
> > > > +   bool active_low;
> > > > +};
> > > > +
> > > > +/*
> > > > + * Model name probe
> > > > + *
> > > > + * Reads the family-specific MODEL_NAME_INFO register
> > > > + * to identify the SoC model and revision
> > > > + */
> > > any reasons to put the defines close to the function rather then before
> > > the code as is commonly done? I'm not saying it's a bad idea; it just
> > > not what is done normally ...
> > I put the RTL838x defines here, because I was planning to put the
> > information for one SoC
> > type together. The register definitions could go into a separate MFD header
> > though, but I
> > kept them here for now to more easily maintain an overview.
> > 
> > > > +#define RTL8380_REG_MODEL_NAME_INFO0x00d4
> > > > +#define RTL8380_REG_CHIP_INFO  0x00d8
> > > > +#define MODEL_NAME_CHAR_XLATE(val) ((val) ? 'A' + (val) - 1
> > > > : '\0')
> > > > +#define MODEL_NAME_CHAR(reg, msb,
> > > > lsb) (MODEL_NAME_CHAR_XLATE(FIELD_GET(GENMASK((msb), (lsb)),
> > > > (val
> > > > +
> > These macro's are certainly not SoC-specific, and (following conventions)
> > should indeed be
> > higher up in the file.
> > 
> > > > +#define RTL8380_REG_INT_RW_CTRL0x0058
> > > _personally_ i prefer alignment with spaces here so that any changes in
> > > tab with (for indentation) is not affecting the alignment here.
> > > > +
> > > > +static void rtl8380_probe_model_name(const struct
> > > > realtek_switchcore_ctrl *ctrl)
> > > > +{
> > > > +    char model_char[4] = {0, 0, 0, 0};
> > > While I love being explicit, what if you have [20] :)
> > Then I shouldn't be putti

Re: [RFC PATCH 2/7] realtek: add switch core MFD driver

2022-08-09 Thread Olliver Schinagl

Hey Sander,

On 29-07-2022 22:19, Sander Vanheule wrote:

On Fri, 2022-07-29 at 14:58 +0200, Olliver Schinagl wrote:

Hey Sander,

On 16-07-2022 21:09, Sander Vanheule wrote:

Realtek managed switch SoCs such as the RTL8380 consist of a MIPS CPU
with a number of basic peripherals, and an ethernet switch peripheral.
Besides performing ethernet related tasks, this switch core also
provides SoC management features. These switch core features are badly
separated, and cannot be divided into distinct IO ranges.

This MFD core driver is intended to manage the switch core regmap, and
to expose some limited features that don't warrant their own driver.
These include SoC identification and system LED management.

Signed-off-by: Sander Vanheule 
---
   .../drivers/mfd/realtek-switchcore.c  | 244 ++
   ...0-mfd-add-Realtek-switch-core-driver.patch |  50 
   target/linux/realtek/rtl838x/config-5.10  |   2 +
   3 files changed, 296 insertions(+)
   create mode 100644 
target/linux/realtek/files-5.10/drivers/mfd/realtek-switchcore.c
   create mode 100644 
target/linux/realtek/patches-5.10/200-mfd-add-Realtek-switch-
core-driver.patch

diff --git a/target/linux/realtek/files-5.10/drivers/mfd/realtek-switchcore.c
b/target/linux/realtek/files-5.10/drivers/mfd/realtek-switchcore.c
new file mode 100644
index ..5b3f6eee6fe2
--- /dev/null
+++ b/target/linux/realtek/files-5.10/drivers/mfd/realtek-switchcore.c
@@ -0,0 +1,244 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include 

Probably want bitops.h here as well, as BIT() and GENMASK() come from there

Will add.


+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+struct realtek_switchcore_ctrl;

I take it a switchcore is the generic name, and an rtl8380 etc just
implements one? Makes sense if so.

Switchcore is the name of the ASIC peripheral that handles the networking, and 
is really
the system controller as well, managing pin muxes and such. RTL838x (Maple) SoC 
have one
register layout, RTL839x (Cypress) has another set. In one generation (RTL83xx, 
RTL93xx)
the provided features are usually similar, but every SoC type really requires 
its own
implementation.


+
+struct realtek_switchcore_data {
+   struct reg_field sys_led_field;
+   const struct mfd_cell *mfd_devices;
+   unsigned int mfd_device_count;
+   void (*probe_model_name)(const struct realtek_switchcore_ctrl *ctrl);
+};
+
+struct realtek_switchcore_ctrl {
+   struct device *dev;
+   struct regmap *map;
+   const struct realtek_switchcore_data *data;
+   struct led_classdev sys_led;
+   struct regmap_field *sys_led_field;
+   bool active_low;
+};
+
+/*
+ * Model name probe
+ *
+ * Reads the family-specific MODEL_NAME_INFO register
+ * to identify the SoC model and revision
+ */

any reasons to put the defines close to the function rather then before
the code as is commonly done? I'm not saying it's a bad idea; it just
not what is done normally ...

I put the RTL838x defines here, because I was planning to put the information 
for one SoC
type together. The register definitions could go into a separate MFD header 
though, but I
kept them here for now to more easily maintain an overview.


+#define RTL8380_REG_MODEL_NAME_INFO0x00d4
+#define RTL8380_REG_CHIP_INFO  0x00d8
+#define MODEL_NAME_CHAR_XLATE(val) ((val) ? 'A' + (val) - 1 : '\0')
+#define MODEL_NAME_CHAR(reg, msb,
lsb) (MODEL_NAME_CHAR_XLATE(FIELD_GET(GENMASK((msb), (lsb)), (val
+

These macro's are certainly not SoC-specific, and (following conventions) 
should indeed be
higher up in the file.


+#define RTL8380_REG_INT_RW_CTRL0x0058

_personally_ i prefer alignment with spaces here so that any changes in
tab with (for indentation) is not affecting the alignment here.

+
+static void rtl8380_probe_model_name(const struct realtek_switchcore_ctrl 
*ctrl)
+{
+    char model_char[4] = {0, 0, 0, 0};

While I love being explicit, what if you have [20] :)

Then I shouldn't be putting this on the stack either, when this becomes too 
big. But for
all current SoC series, 3 is the maximum number of characters that can be 
defined in the
model name (plus NULL termination)
I was being a smart-ass; but you answerd yourself below, that {} is 
probably better as your initializer.



+    char model_char[4] = { 0x00 };

is probably easier ;)

memset() would also work, but I should be able to use {} if we just want to 
empty-
initialise a const-size array.

sure, but the { 0x00 }, initializer auto-repeats itself for all entries.




+    char chip_rev;
+    u32 model_id;
+    u32 val = 0;
+
+    regmap_read(ctrl->map, RTL8380_REG_MODEL_NAME_INFO, &val);
+    model_id = FIELD_GET(GENMASK(31, 16), val);
+    model_char[0] = MODEL_NAME_CHAR(val, 15, 11);
+    model_char[1] = MODEL_NAME_CHAR(val, 10, 6);
+    model_char[2] = MODEL_NAME_CHAR(val, 5, 1);
+
+    /

Re: Bridge-vlan bug? (mt7621/DSA)

2022-08-09 Thread Felix Fietkau


On 09.08.22 15:13, Thibaut wrote:



Le 6 août 2022 à 11:58, Thibaut  a écrit :



Le 6 août 2022 à 00:50, Mark Mentovai  a écrit :

Thibaut wrote:

I’m experiencing a strange bug on Yuncore AX820 (mt7621/mt7905/mt7975, 
DSA-enabled) when using a bridge-vlan setup. This bug affects at least OpenWRT 
22.03.0-rc6.

I’m not sure whether this bug is related to this particular SoC or only to DSA 
as I was unable to test with another DSA-enabled device (I don’t have any). 
However this bug does not affect e.g. QCA non-DSA devices.

I’m running out of ideas on how to further debug this problem, so feel free to 
guide me if more information is needed. Please CC-me in replies.


This sounds very similar to the problem I experienced with the work-in-progress 
DSA patches for ipq40xx:

https://github.com/openwrt/openwrt/pull/4721#issuecomment-971162067

This kernel patch explains the situation fairly well:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d5f19486cee79d04c054427577ac96ed123706db

But the fix isn’t operative unless the switch driver opts in via 
assisted_learning_on_cpu_port. There were also comments from around that time 
that there may still be trouble with untagged traffic.

There’s a bit of discussion about this issue in the comments around there on 
the pull request. Hopefully you’ll find it helpful. It should at least get you 
oriented in the right direction, even if it’s not a fix for your untagged use 
case.


Thanks a lot for these details. Based on your input and looking at our current 
5.10 source and the current upstream, it seems this might have already been 
fixed upstream:

https://github.com/torvalds/linux/commit/0b69c54c74bcb60e834013ccaf596caf05156a8e

I’ll check if this can be backported without too much fuss.


Backport submitted:
http://patchwork.ozlabs.org/project/openwrt/patch/20220809125947.31775-1-ha...@slashdirt.org/

It fixes the issue and applies cleanly to 22.03: I have run tests in both 
master and 22.03.

I will now check if I can make it work in 21.02 which is also affected.

Please take a look at
https://git.openwrt.org/3e0daca6447c3d5b9eb6d24ecb8e52f256f385cc

The changes were backported once already and reverted due to issues.
See also: https://github.com/openwrt/openwrt/issues/9420

- Felix

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


[PATCH] kernel: backport mt7530 dsa bridge-vlan fixes

2022-08-09 Thread Thibaut VARÈNE
Backport patches:
0b69c54c74bc ("net: dsa: mt7530: enable assisted learning on CPU port")
6087175b7991 ("net: dsa: mt7530: use independent VLAN learning on VLAN-unaware 
bridges)

These patches fix outstanding issues with bridge-vlan setups. In
particular, prior to this change setups involving a bridge-vlan with
untagged ethernet ports would see wireless clients roaming to the AP
experience connection drops lasting 2-5mn as the stale FDB entries
persist.

See the discussion here:
http://lists.openwrt.org/pipermail/openwrt-devel/2022-August/039175.html

Signed-off-by: Thibaut VARÈNE 
---
CCing DENG Qingfang  as the upstream author: I had to
massage the patch a little bit to apply to older 5.10 code base.

Note: this cherry-picks with no conflict into openwrt-22.03, and I have
also successfully tested this backport there: I would thus recommend this
be cherry-picked into 22.03.
---
 ...enable-assisted-learning-on-CPU-port.patch |  83 ++
 ...LAN-learning-on-VLAN-unaware-bridges.patch | 272 ++
 2 files changed, 355 insertions(+)
 create mode 100644 
target/linux/generic/backport-5.10/797-v5.15-0001-net-dsa-mt7530-enable-assisted-learning-on-CPU-port.patch
 create mode 100644 
target/linux/generic/backport-5.10/797-v5.15-0002-net-dsa-mt7530-use-independent-VLAN-learning-on-VLAN-unaware-bridges.patch

diff --git 
a/target/linux/generic/backport-5.10/797-v5.15-0001-net-dsa-mt7530-enable-assisted-learning-on-CPU-port.patch
 
b/target/linux/generic/backport-5.10/797-v5.15-0001-net-dsa-mt7530-enable-assisted-learning-on-CPU-port.patch
new file mode 100644
index 00..a474527ca9
--- /dev/null
+++ 
b/target/linux/generic/backport-5.10/797-v5.15-0001-net-dsa-mt7530-enable-assisted-learning-on-CPU-port.patch
@@ -0,0 +1,83 @@
+From 0b69c54c74bcb60e834013ccaf596caf05156a8e Mon Sep 17 00:00:00 2001
+From: DENG Qingfang 
+Date: Wed, 4 Aug 2021 00:04:01 +0800
+Subject: [PATCH] net: dsa: mt7530: enable assisted learning on CPU port
+
+Consider the following bridge configuration, where bond0 is not
+offloaded:
+
+ +-- br0 --+
+/ /   | \
+   / /|  \
+  /  || bond0
+ /   || /   \
+   swp0 swp1 swp2 swp3 swp4
+ ..   .
+ ..   .
+ AB   C
+
+Address learning is enabled on offloaded ports (swp0~2) and the CPU
+port, so when client A sends a packet to C, the following will happen:
+
+1. The switch learns that client A can be reached at swp0.
+2. The switch probably already knows that client C can be reached at the
+   CPU port, so it forwards the packet to the CPU.
+3. The bridge core knows client C can be reached at bond0, so it
+   forwards the packet back to the switch.
+4. The switch learns that client A can be reached at the CPU port.
+5. The switch forwards the packet to either swp3 or swp4, according to
+   the packet's tag.
+
+That makes client A's MAC address flap between swp0 and the CPU port. If
+client B sends a packet to A, it is possible that the packet is
+forwarded to the CPU. With offload_fwd_mark = 1, the bridge core won't
+forward it back to the switch, resulting in packet loss.
+
+As we have the assisted_learning_on_cpu_port in DSA core now, enable
+that and disable hardware learning on the CPU port.
+
+Signed-off-by: DENG Qingfang 
+Reviewed-by: Vladimir Oltean 
+Signed-off-by: David S. Miller 
+---
+ drivers/net/dsa/mt7530.c | 14 --
+ 1 file changed, 8 insertions(+), 6 deletions(-)
+
+--- a/drivers/net/dsa/mt7530.c
 b/drivers/net/dsa/mt7530.c
+@@ -2026,6 +2026,7 @@ mt7530_setup(struct dsa_switch *ds)
+*/
+   dn = dsa_to_port(ds, MT7530_CPU_PORT)->master->dev.of_node->parent;
+   ds->configure_vlan_while_not_filtering = true;
++  ds->assisted_learning_on_cpu_port = true;
+   ds->mtu_enforcement_ingress = true;
+ 
+   if (priv->id == ID_MT7530) {
+@@ -2096,6 +2097,9 @@ mt7530_setup(struct dsa_switch *ds)
+   mt7530_rmw(priv, MT7530_PCR_P(i), PCR_MATRIX_MASK,
+  PCR_MATRIX_CLR);
+ 
++  /* Disable learning by default on all ports */
++  mt7530_set(priv, MT7530_PSC_P(i), SA_DIS);
++
+   if (dsa_is_cpu_port(ds, i)) {
+   ret = mt753x_cpu_port_enable(ds, i);
+   if (ret)
+@@ -2257,6 +2261,9 @@ mt7531_setup(struct dsa_switch *ds)
+   mt7530_rmw(priv, MT7530_PCR_P(i), PCR_MATRIX_MASK,
+  PCR_MATRIX_CLR);
+ 
++  /* Disable learning by default on all ports */
++  mt7530_set(priv, MT7530_PSC_P(i), SA_DIS);
++
+   mt7530_set(priv, MT7531_DBG_CNT(i), MT7531_DIS_CLR);
+ 
+   if (dsa_is_cpu_port(ds, i)) {
+@@ -2272,6 +2279,7 @@ mt7531_setup(struct dsa_switch *ds)
+   }
+ 
+   ds->configure_vlan_while_not_filtering = true;
++  ds->assisted_learning_on_cpu_port = true;
+   ds->mtu_enforcement_ingress = true;
+ 
+   /* Flush the FDB table */
diff --