Re: [PATCH net-next v2 2/2] net: ethernet: mediatek: support custom GMAC label

2021-04-20 Thread Ilya Lipnitskiy
On Tue, Apr 20, 2021 at 12:51 PM Rob Herring  wrote:
>
> On Mon, Apr 19, 2021 at 08:46:59AM -0700, Ilya Lipnitskiy wrote:
> > The MAC device name can now be set within DTS file instead of always
> > being "ethX". This is helpful for DSA to clearly label the DSA master
> > device and distinguish it from DSA slave ports.
> >
> > For example, some devices, such as the Ubiquiti EdgeRouter X, may have
> > ports labeled ethX. Labeling the master GMAC with a different prefix
> > than DSA ports helps with clarity.
> >
> > Suggested-by: René van Dorst 
> > Signed-off-by: Ilya Lipnitskiy 
> > ---
> >  drivers/net/ethernet/mediatek/mtk_eth_soc.c | 6 --
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c 
> > b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > index 6b00c12c6c43..df3cda63a8c5 100644
> > --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > @@ -2845,6 +2845,7 @@ static const struct net_device_ops mtk_netdev_ops = {
> >
> >  static int mtk_add_mac(struct mtk_eth *eth, struct device_node *np)
> >  {
> > + const char *label = of_get_property(np, "label", NULL);
> >   const __be32 *_id = of_get_property(np, "reg", NULL);
> >   phy_interface_t phy_mode;
> >   struct phylink *phylink;
> > @@ -2867,9 +2868,10 @@ static int mtk_add_mac(struct mtk_eth *eth, struct 
> > device_node *np)
> >   return -EINVAL;
> >   }
> >
> > - eth->netdev[id] = alloc_etherdev(sizeof(*mac));
> > + eth->netdev[id] = alloc_netdev(sizeof(*mac), label ? label : "eth%d",
> > +NET_NAME_UNKNOWN, ether_setup);
>
> 'label' is generally supposed to correspond to the sticker for the
> device connector for a human to id. I can't really tell if that's the
> case here. I don't see how 'gmacX' vs. 'ethX' maps to DSA master vs.
> slave.
The ports on devices such as Ubiquiti ER-X are named eth0 through
eth4, all of them DSA slaves. The gmac (DSA master) is hard-coded to
eth0 without this change.
>
> I don't think this should be handled within a specific driver either. If
> we're going to have a way to name things, then fix it in
> alloc_etherdev().
>
> It can also be argued that device naming for userspace is a userspace
> (udev) problem.
Yeah, that is a valid argument. We can drop this changeset if the
agreement is that it doesn't belong in the specific driver or the
kernel-space at all.
Some discussion (and device picture) here if you are interested:
https://github.com/openwrt/openwrt/pull/3971

Ilya


[PATCH] MIPS: pci-legacy: revert "use generic pci_enable_resources"

2021-04-20 Thread Ilya Lipnitskiy
This mostly reverts commit 99bca615d895 ("MIPS: pci-legacy: use generic
pci_enable_resources"). Fixes regressions such as:
  ata_piix :00:0a.1: can't enable device: BAR 0 [io  0x01f0-0x01f7] not
claimed
  ata_piix: probe of :00:0a.1 failed with error -22

The only changes from the strict revert are to fix checkpatch errors:
  ERROR: spaces required around that '=' (ctx:VxV)
  #33: FILE: arch/mips/pci/pci-legacy.c:252:
  + for (idx=0; idx < PCI_NUM_RESOURCES; idx++) {
^

  ERROR: do not use assignment in if condition
  #67: FILE: arch/mips/pci/pci-legacy.c:284:
  + if ((err = pcibios_enable_resources(dev, mask)) < 0)

Reported-by: Guenter Roeck 
Signed-off-by: Ilya Lipnitskiy 
---
Guenter, sorry about this - let's just revert this for now to minimize
turmoil to the legacy PCI code. Obviously, this needs more testing
before applying. Thanks for your report!

 arch/mips/pci/pci-legacy.c | 38 +-
 1 file changed, 37 insertions(+), 1 deletion(-)

diff --git a/arch/mips/pci/pci-legacy.c b/arch/mips/pci/pci-legacy.c
index c24226ea0a6e..468722c8a5c6 100644
--- a/arch/mips/pci/pci-legacy.c
+++ b/arch/mips/pci/pci-legacy.c
@@ -241,9 +241,45 @@ static int __init pcibios_init(void)
 
 subsys_initcall(pcibios_init);
 
+static int pcibios_enable_resources(struct pci_dev *dev, int mask)
+{
+   u16 cmd, old_cmd;
+   int idx;
+   struct resource *r;
+
+   pci_read_config_word(dev, PCI_COMMAND, );
+   old_cmd = cmd;
+   for (idx = 0; idx < PCI_NUM_RESOURCES; idx++) {
+   /* Only set up the requested stuff */
+   if (!(mask & (1<resource[idx];
+   if (!(r->flags & (IORESOURCE_IO | IORESOURCE_MEM)))
+   continue;
+   if ((idx == PCI_ROM_RESOURCE) &&
+   (!(r->flags & IORESOURCE_ROM_ENABLE)))
+   continue;
+   if (!r->start && r->end) {
+   pci_err(dev,
+   "can't enable device: resource collisions\n");
+   return -EINVAL;
+   }
+   if (r->flags & IORESOURCE_IO)
+   cmd |= PCI_COMMAND_IO;
+   if (r->flags & IORESOURCE_MEM)
+   cmd |= PCI_COMMAND_MEMORY;
+   }
+   if (cmd != old_cmd) {
+   pci_info(dev, "enabling device (%04x -> %04x)\n", old_cmd, cmd);
+   pci_write_config_word(dev, PCI_COMMAND, cmd);
+   }
+   return 0;
+}
+
 int pcibios_enable_device(struct pci_dev *dev, int mask)
 {
-   int err = pci_enable_resources(dev, mask);
+   int err = pcibios_enable_resources(dev, mask);
 
if (err < 0)
return err;
-- 
2.31.1



Re: [PATCH] dt-bindings: net: mediatek: support MT7621 SoC

2021-04-19 Thread Ilya Lipnitskiy
On Sun, Apr 18, 2021 at 11:24 PM Bjørn Mork  wrote:
>
> Ilya Lipnitskiy  writes:
>
> > Add missing binding documentation for SoC support that has been in place
> > since v5.1
> >
> > Fixes: 889bcbdeee57 ("net: ethernet: mediatek: support MT7621 SoC ethernet 
> > hardware")
> > Cc: Bjørn Mork 
> > Signed-off-by: Ilya Lipnitskiy 
> > ---
> >  Documentation/devicetree/bindings/net/mediatek-net.txt | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/Documentation/devicetree/bindings/net/mediatek-net.txt 
> > b/Documentation/devicetree/bindings/net/mediatek-net.txt
> > index 72d03e07cf7c..950ef6af20b1 100644
> > --- a/Documentation/devicetree/bindings/net/mediatek-net.txt
> > +++ b/Documentation/devicetree/bindings/net/mediatek-net.txt
> > @@ -10,6 +10,7 @@ Required properties:
> >  - compatible: Should be
> >   "mediatek,mt2701-eth": for MT2701 SoC
> >   "mediatek,mt7623-eth", "mediatek,mt2701-eth": for MT7623 SoC
> > + "mediatek,mt7621-eth": for MT7621 SoC
> >   "mediatek,mt7622-eth": for MT7622 SoC
> >   "mediatek,mt7629-eth": for MT7629 SoC
> >   "ralink,rt5350-eth": for Ralink Rt5350F and MT7628/88 SoC
>
>
> Thanks for taking care of this!
>
> Note, however, that this compatible value is defined in
> Documentation/devicetree/bindings/net/ralink,rt2880-net.txt
>
> I believe that file should go away. These two files are both documenting
> the same compatible property AFAICS.
Removed along with two others in
https://lore.kernel.org/lkml/20210420024222.101615-1-ilya.lipnits...@gmail.com/T/#u

Ilya


[PATCH] dt-bindings: net: mediatek/ralink: remove unused bindings

2021-04-19 Thread Ilya Lipnitskiy
Revert commit 663148e48a66 ("Documentation: DT: net: add docs for
ralink/mediatek SoC ethernet binding")

No in-tree drivers use the compatible strings present in these bindings,
and some have been superseded by DSA-capable mtk_eth_soc driver, so
remove these obsolete bindings.

Cc: John Crispin 
Signed-off-by: Ilya Lipnitskiy 
---
 .../bindings/net/mediatek,mt7620-gsw.txt  | 24 
 .../bindings/net/ralink,rt2880-net.txt| 59 ---
 .../bindings/net/ralink,rt3050-esw.txt| 30 --
 3 files changed, 113 deletions(-)
 delete mode 100644 
Documentation/devicetree/bindings/net/mediatek,mt7620-gsw.txt
 delete mode 100644 Documentation/devicetree/bindings/net/ralink,rt2880-net.txt
 delete mode 100644 Documentation/devicetree/bindings/net/ralink,rt3050-esw.txt

diff --git a/Documentation/devicetree/bindings/net/mediatek,mt7620-gsw.txt 
b/Documentation/devicetree/bindings/net/mediatek,mt7620-gsw.txt
deleted file mode 100644
index 358fed2fab43..
--- a/Documentation/devicetree/bindings/net/mediatek,mt7620-gsw.txt
+++ /dev/null
@@ -1,24 +0,0 @@
-Mediatek Gigabit Switch
-===
-
-The mediatek gigabit switch can be found on Mediatek SoCs (mt7620, mt7621).
-
-Required properties:
-- compatible: Should be "mediatek,mt7620-gsw" or "mediatek,mt7621-gsw"
-- reg: Address and length of the register set for the device
-- interrupts: Should contain the gigabit switches interrupt
-- resets: Should contain the gigabit switches resets
-- reset-names: Should contain the reset names "gsw"
-
-Example:
-
-gsw@1011 {
-   compatible = "ralink,mt7620-gsw";
-   reg = <0x1011 8000>;
-
-   resets = < 23>;
-   reset-names = "gsw";
-
-   interrupt-parent = <>;
-   interrupts = <17>;
-};
diff --git a/Documentation/devicetree/bindings/net/ralink,rt2880-net.txt 
b/Documentation/devicetree/bindings/net/ralink,rt2880-net.txt
deleted file mode 100644
index 9fe1a0a22e44..
--- a/Documentation/devicetree/bindings/net/ralink,rt2880-net.txt
+++ /dev/null
@@ -1,59 +0,0 @@
-Ralink Frame Engine Ethernet controller
-===
-
-The Ralink frame engine ethernet controller can be found on Ralink and
-Mediatek SoCs (RT288x, RT3x5x, RT366x, RT388x, rt5350, mt7620, mt7621, mt76x8).
-
-Depending on the SoC, there is a number of ports connected to the CPU port
-directly and/or via a (gigabit-)switch.
-
-* Ethernet controller node
-
-Required properties:
-- compatible: Should be one of "ralink,rt2880-eth", "ralink,rt3050-eth",
-  "ralink,rt3050-eth", "ralink,rt3883-eth", "ralink,rt5350-eth",
-  "mediatek,mt7620-eth", "mediatek,mt7621-eth"
-- reg: Address and length of the register set for the device
-- interrupts: Should contain the frame engines interrupt
-- resets: Should contain the frame engines resets
-- reset-names: Should contain the reset names "fe". If a switch is present
-  "esw" is also required.
-
-
-* Ethernet port node
-
-Required properties:
-- compatible: Should be "ralink,eth-port"
-- reg: The number of the physical port
-- phy-handle: reference to the node describing the phy
-
-Example:
-
-mdio-bus {
-   ...
-   phy0: ethernet-phy@0 {
-   phy-mode = "mii";
-   reg = <0>;
-   };
-};
-
-ethernet@40 {
-   compatible = "ralink,rt2880-eth";
-   reg = <0x0040 1>;
-
-   #address-cells = <1>;
-   #size-cells = <0>;
-
-   resets = < 18>;
-   reset-names = "fe";
-
-   interrupt-parent = <>;
-   interrupts = <5>;
-
-   port@0 {
-   compatible = "ralink,eth-port";
-   reg = <0>;
-   phy-handle = <>;
-   };
-
-};
diff --git a/Documentation/devicetree/bindings/net/ralink,rt3050-esw.txt 
b/Documentation/devicetree/bindings/net/ralink,rt3050-esw.txt
deleted file mode 100644
index 87e315856efa..
--- a/Documentation/devicetree/bindings/net/ralink,rt3050-esw.txt
+++ /dev/null
@@ -1,30 +0,0 @@
-Ralink Fast Ethernet Embedded Switch
-
-
-The ralink fast ethernet embedded switch can be found on Ralink and Mediatek
-SoCs (RT3x5x, RT5350, MT76x8).
-
-Required properties:
-- compatible: Should be "ralink,rt3050-esw"
-- reg: Address and length of the register set for the device
-- interrupts: Should contain the embedded switches interrupt
-- resets: Should contain the embedded switches resets
-- reset-names: Should contain the reset names "esw"
-
-Optional properties:
-- ralink,portmap: can be used to choose if the default switch setup is
-  w or w
-- ralink,led_polarity: override the active high/low settings of the leds
-
-Example:
-
-esw@1011 {
-   compatible = "ralink,rt3050-esw";
-   reg = <0x1011 8000>;
-
-   resets = < 23>;
-   reset-names = "esw";
-
-   interrupt-parent = <>;
-   interrupts = <17>;
-};
-- 
2.31.1



[PATCH net-next v2 1/2] dt-bindings: net: mediatek: add optional GMAC labels

2021-04-19 Thread Ilya Lipnitskiy
Document the mediatek ethernet driver change that adds support for
custom labels and provide an example.

Signed-off-by: Ilya Lipnitskiy 
---
 Documentation/devicetree/bindings/net/mediatek-net.txt | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/mediatek-net.txt 
b/Documentation/devicetree/bindings/net/mediatek-net.txt
index 72d03e07cf7c..500bf9351010 100644
--- a/Documentation/devicetree/bindings/net/mediatek-net.txt
+++ b/Documentation/devicetree/bindings/net/mediatek-net.txt
@@ -51,6 +51,10 @@ Required properties:
is equal to 0 and the MAC uses fixed-link to connect
with internal switch such as MT7530.
 
+Optional properties:
+- label: overrides the default netdevice name. Useful when a custom name for 
the
+   DSA master interface is desired.
+
 Example:
 
 eth: ethernet@1b10 {
@@ -74,12 +78,14 @@ eth: ethernet@1b10 {
 
gmac1: mac@0 {
compatible = "mediatek,eth-mac";
+   label = "gmac1";
reg = <0>;
phy-handle = <>;
};
 
gmac2: mac@1 {
compatible = "mediatek,eth-mac";
+   label = "gmac2";
reg = <1>;
phy-handle = <>;
};
-- 
2.31.1



[PATCH net-next v2 2/2] net: ethernet: mediatek: support custom GMAC label

2021-04-19 Thread Ilya Lipnitskiy
The MAC device name can now be set within DTS file instead of always
being "ethX". This is helpful for DSA to clearly label the DSA master
device and distinguish it from DSA slave ports.

For example, some devices, such as the Ubiquiti EdgeRouter X, may have
ports labeled ethX. Labeling the master GMAC with a different prefix
than DSA ports helps with clarity.

Suggested-by: René van Dorst 
Signed-off-by: Ilya Lipnitskiy 
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c 
b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 6b00c12c6c43..df3cda63a8c5 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -2845,6 +2845,7 @@ static const struct net_device_ops mtk_netdev_ops = {
 
 static int mtk_add_mac(struct mtk_eth *eth, struct device_node *np)
 {
+   const char *label = of_get_property(np, "label", NULL);
const __be32 *_id = of_get_property(np, "reg", NULL);
phy_interface_t phy_mode;
struct phylink *phylink;
@@ -2867,9 +2868,10 @@ static int mtk_add_mac(struct mtk_eth *eth, struct 
device_node *np)
return -EINVAL;
}
 
-   eth->netdev[id] = alloc_etherdev(sizeof(*mac));
+   eth->netdev[id] = alloc_netdev(sizeof(*mac), label ? label : "eth%d",
+  NET_NAME_UNKNOWN, ether_setup);
if (!eth->netdev[id]) {
-   dev_err(eth->dev, "alloc_etherdev failed\n");
+   dev_err(eth->dev, "alloc_netdev failed\n");
return -ENOMEM;
}
mac = netdev_priv(eth->netdev[id]);
-- 
2.31.1



[PATCH net-next v2 0/2] net: ethernet: mediatek: support custom GMAC label

2021-04-19 Thread Ilya Lipnitskiy
Add support for specifying GMAC label via DTS. Useful when it is desired
to use a master DSA interface name that is different from the "eth%d"
pattern.

v2:
  - Use alloc_netdev instead of alloc_etherdev followed by rename

Ilya Lipnitskiy (2):
  dt-bindings: net: mediatek: add optional GMAC labels
  net: ethernet: mediatek: support custom GMAC label

 Documentation/devicetree/bindings/net/mediatek-net.txt | 6 ++
 drivers/net/ethernet/mediatek/mtk_eth_soc.c| 6 --
 2 files changed, 10 insertions(+), 2 deletions(-)

-- 
2.31.1



Re: [PATCH] dt-bindings: net: mediatek: support MT7621 SoC

2021-04-19 Thread Ilya Lipnitskiy
On Sun, Apr 18, 2021 at 11:24 PM Bjørn Mork  wrote:
>
> Ilya Lipnitskiy  writes:
>
> > Add missing binding documentation for SoC support that has been in place
> > since v5.1
> >
> > Fixes: 889bcbdeee57 ("net: ethernet: mediatek: support MT7621 SoC ethernet 
> > hardware")
> > Cc: Bjørn Mork 
> > Signed-off-by: Ilya Lipnitskiy 
> > ---
> >  Documentation/devicetree/bindings/net/mediatek-net.txt | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/Documentation/devicetree/bindings/net/mediatek-net.txt 
> > b/Documentation/devicetree/bindings/net/mediatek-net.txt
> > index 72d03e07cf7c..950ef6af20b1 100644
> > --- a/Documentation/devicetree/bindings/net/mediatek-net.txt
> > +++ b/Documentation/devicetree/bindings/net/mediatek-net.txt
> > @@ -10,6 +10,7 @@ Required properties:
> >  - compatible: Should be
> >   "mediatek,mt2701-eth": for MT2701 SoC
> >   "mediatek,mt7623-eth", "mediatek,mt2701-eth": for MT7623 SoC
> > + "mediatek,mt7621-eth": for MT7621 SoC
> >   "mediatek,mt7622-eth": for MT7622 SoC
> >   "mediatek,mt7629-eth": for MT7629 SoC
> >   "ralink,rt5350-eth": for Ralink Rt5350F and MT7628/88 SoC
>
>
> Thanks for taking care of this!
>
> Note, however, that this compatible value is defined in
> Documentation/devicetree/bindings/net/ralink,rt2880-net.txt
Good point. I don't think there is a driver in-tree for that binding.
It looks like commit 663148e48a66 ("Documentation: DT: net: add docs
for ralink/mediatek SoC ethernet binding") should just be reverted and
the three documents (mediatek,mt7620-gsw.txt; ralink,rt2880-net.txt;
ralink,rt3050-esw.txt) removed. Any objections?

Ilya


Re: [PATCH net-next 2/2] net: ethernet: mediatek: support custom GMAC label

2021-04-19 Thread Ilya Lipnitskiy
On Mon, Apr 19, 2021 at 5:15 AM Andrew Lunn  wrote:
>
> On Sun, Apr 18, 2021 at 09:03:52PM -0700, Ilya Lipnitskiy wrote:
> > The MAC device name can now be set within DTS file instead of always
> > being "ethX". This is helpful for DSA to clearly label the DSA master
> > device and distinguish it from DSA slave ports.
> >
> > For example, some devices, such as the Ubiquiti EdgeRouter X, may have
> > ports labeled ethX. Labeling the master GMAC with a different prefix
> > than DSA ports helps with clarity.
> >
> > Suggested-by: René van Dorst 
> > Signed-off-by: Ilya Lipnitskiy 
> > ---
> >  drivers/net/ethernet/mediatek/mtk_eth_soc.c | 4 
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c 
> > b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > index 6b00c12c6c43..4c0ce4fb7735 100644
> > --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > @@ -2845,6 +2845,7 @@ static const struct net_device_ops mtk_netdev_ops = {
> >
> >  static int mtk_add_mac(struct mtk_eth *eth, struct device_node *np)
> >  {
> > + const char *label = of_get_property(np, "label", NULL);
> >   const __be32 *_id = of_get_property(np, "reg", NULL);
> >   phy_interface_t phy_mode;
> >   struct phylink *phylink;
> > @@ -2940,6 +2941,9 @@ static int mtk_add_mac(struct mtk_eth *eth, struct 
> > device_node *np)
> >   else
> >   eth->netdev[id]->max_mtu = MTK_MAX_RX_LENGTH_2K - 
> > MTK_RX_ETH_HLEN;
> >
> > + if (label)
> > + strscpy(eth->netdev[id]->name, label, IFNAMSIZ);
>
> It is better to use alloc_netdev_mqs() so you get validation the name
> is unique.
It doesn't look like the name validation happens until the netdev is
registered, and it does not get registered at alloc, right?

I do agree that it's better to use the correct name in the first place
instead of renaming, regardless, so using alloc_netdev_mqs() seems
like a better solution - I'll make the change.

Ilya


[PATCH net-next 2/2] net: ethernet: mediatek: support custom GMAC label

2021-04-18 Thread Ilya Lipnitskiy
The MAC device name can now be set within DTS file instead of always
being "ethX". This is helpful for DSA to clearly label the DSA master
device and distinguish it from DSA slave ports.

For example, some devices, such as the Ubiquiti EdgeRouter X, may have
ports labeled ethX. Labeling the master GMAC with a different prefix
than DSA ports helps with clarity.

Suggested-by: René van Dorst 
Signed-off-by: Ilya Lipnitskiy 
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c 
b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 6b00c12c6c43..4c0ce4fb7735 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -2845,6 +2845,7 @@ static const struct net_device_ops mtk_netdev_ops = {
 
 static int mtk_add_mac(struct mtk_eth *eth, struct device_node *np)
 {
+   const char *label = of_get_property(np, "label", NULL);
const __be32 *_id = of_get_property(np, "reg", NULL);
phy_interface_t phy_mode;
struct phylink *phylink;
@@ -2940,6 +2941,9 @@ static int mtk_add_mac(struct mtk_eth *eth, struct 
device_node *np)
else
eth->netdev[id]->max_mtu = MTK_MAX_RX_LENGTH_2K - 
MTK_RX_ETH_HLEN;
 
+   if (label)
+   strscpy(eth->netdev[id]->name, label, IFNAMSIZ);
+
return 0;
 
 free_netdev:
-- 
2.31.1



[PATCH net-next 1/2] dt-bindings: net: mediatek: add optional GMAC labels

2021-04-18 Thread Ilya Lipnitskiy
Document the mediatek ethernet driver change that adds support for
custom labels and provide an example.

Signed-off-by: Ilya Lipnitskiy 
---
 Documentation/devicetree/bindings/net/mediatek-net.txt | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/mediatek-net.txt 
b/Documentation/devicetree/bindings/net/mediatek-net.txt
index 72d03e07cf7c..500bf9351010 100644
--- a/Documentation/devicetree/bindings/net/mediatek-net.txt
+++ b/Documentation/devicetree/bindings/net/mediatek-net.txt
@@ -51,6 +51,10 @@ Required properties:
is equal to 0 and the MAC uses fixed-link to connect
with internal switch such as MT7530.
 
+Optional properties:
+- label: overrides the default netdevice name. Useful when a custom name for 
the
+   DSA master interface is desired.
+
 Example:
 
 eth: ethernet@1b10 {
@@ -74,12 +78,14 @@ eth: ethernet@1b10 {
 
gmac1: mac@0 {
compatible = "mediatek,eth-mac";
+   label = "gmac1";
reg = <0>;
phy-handle = <>;
};
 
gmac2: mac@1 {
compatible = "mediatek,eth-mac";
+   label = "gmac2";
reg = <1>;
phy-handle = <>;
};
-- 
2.31.1



[PATCH net-next 0/2] net: ethernet: mediatek: support custom GMAC label

2021-04-18 Thread Ilya Lipnitskiy
Add support for specifying GMAC label via DTS. Useful when it is desired
to use a master DSA interface name that is different from the "eth%d"
pattern.

Ilya Lipnitskiy (2):
  dt-bindings: net: mediatek: add optional GMAC labels
  net: ethernet: mediatek: support custom GMAC label

 Documentation/devicetree/bindings/net/mediatek-net.txt | 6 ++
 drivers/net/ethernet/mediatek/mtk_eth_soc.c| 4 
 2 files changed, 10 insertions(+)

-- 
2.31.1



[PATCH] dt-bindings: net: mediatek: support MT7621 SoC

2021-04-18 Thread Ilya Lipnitskiy
Add missing binding documentation for SoC support that has been in place
since v5.1

Fixes: 889bcbdeee57 ("net: ethernet: mediatek: support MT7621 SoC ethernet 
hardware")
Cc: Bjørn Mork 
Signed-off-by: Ilya Lipnitskiy 
---
 Documentation/devicetree/bindings/net/mediatek-net.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/net/mediatek-net.txt 
b/Documentation/devicetree/bindings/net/mediatek-net.txt
index 72d03e07cf7c..950ef6af20b1 100644
--- a/Documentation/devicetree/bindings/net/mediatek-net.txt
+++ b/Documentation/devicetree/bindings/net/mediatek-net.txt
@@ -10,6 +10,7 @@ Required properties:
 - compatible: Should be
"mediatek,mt2701-eth": for MT2701 SoC
"mediatek,mt7623-eth", "mediatek,mt2701-eth": for MT7623 SoC
+   "mediatek,mt7621-eth": for MT7621 SoC
"mediatek,mt7622-eth": for MT7622 SoC
"mediatek,mt7629-eth": for MT7629 SoC
"ralink,rt5350-eth": for Ralink Rt5350F and MT7628/88 SoC
-- 
2.31.1



[PATCH net-next,v3] net: ethernet: mediatek: ppe: fix busy wait loop

2021-04-15 Thread Ilya Lipnitskiy
The intention is for the loop to timeout if the body does not succeed.
The current logic calls time_is_before_jiffies(timeout) which is false
until after the timeout, so the loop body never executes.

Fix by using readl_poll_timeout as a more standard and less error-prone
solution.

Fixes: ba37b7caf1ed ("net: ethernet: mtk_eth_soc: add support for initializing 
the PPE")
Signed-off-by: Ilya Lipnitskiy 
Cc: Felix Fietkau 
Reviewed-by: Andrew Lunn 
---
v2:
  - Use readl_poll_timeout as suggested by Andrew Lunn
v3:
  - Remove unused #include's

 drivers/net/ethernet/mediatek/mtk_ppe.c | 20 +---
 drivers/net/ethernet/mediatek/mtk_ppe.h |  1 +
 2 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_ppe.c 
b/drivers/net/ethernet/mediatek/mtk_ppe.c
index 71e1ccea6e72..3ad10c793308 100644
--- a/drivers/net/ethernet/mediatek/mtk_ppe.c
+++ b/drivers/net/ethernet/mediatek/mtk_ppe.c
@@ -2,9 +2,8 @@
 /* Copyright (C) 2020 Felix Fietkau  */
 
 #include 
-#include 
-#include 
 #include 
+#include 
 #include 
 #include 
 #include "mtk_ppe.h"
@@ -44,18 +43,17 @@ static u32 ppe_clear(struct mtk_ppe *ppe, u32 reg, u32 val)
 
 static int mtk_ppe_wait_busy(struct mtk_ppe *ppe)
 {
-   unsigned long timeout = jiffies + HZ;
-
-   while (time_is_before_jiffies(timeout)) {
-   if (!(ppe_r32(ppe, MTK_PPE_GLO_CFG) & MTK_PPE_GLO_CFG_BUSY))
-   return 0;
+   int ret;
+   u32 val;
 
-   usleep_range(10, 20);
-   }
+   ret = readl_poll_timeout(ppe->base + MTK_PPE_GLO_CFG, val,
+!(val & MTK_PPE_GLO_CFG_BUSY),
+20, MTK_PPE_WAIT_TIMEOUT_US);
 
-   dev_err(ppe->dev, "PPE table busy");
+   if (ret)
+   dev_err(ppe->dev, "PPE table busy");
 
-   return -ETIMEDOUT;
+   return ret;
 }
 
 static void mtk_ppe_cache_clear(struct mtk_ppe *ppe)
diff --git a/drivers/net/ethernet/mediatek/mtk_ppe.h 
b/drivers/net/ethernet/mediatek/mtk_ppe.h
index 51bd5e75bbbd..242fb8f2ae65 100644
--- a/drivers/net/ethernet/mediatek/mtk_ppe.h
+++ b/drivers/net/ethernet/mediatek/mtk_ppe.h
@@ -12,6 +12,7 @@
 #define MTK_PPE_ENTRIES_SHIFT  3
 #define MTK_PPE_ENTRIES(1024 << MTK_PPE_ENTRIES_SHIFT)
 #define MTK_PPE_HASH_MASK  (MTK_PPE_ENTRIES - 1)
+#define MTK_PPE_WAIT_TIMEOUT_US100
 
 #define MTK_FOE_IB1_UNBIND_TIMESTAMP   GENMASK(7, 0)
 #define MTK_FOE_IB1_UNBIND_PACKETS GENMASK(23, 8)
-- 
2.31.1



[PATCH net-next,v2] net: ethernet: mediatek: ppe: fix busy wait loop

2021-04-15 Thread Ilya Lipnitskiy
The intention is for the loop to timeout if the body does not succeed.
The current logic calls time_is_before_jiffies(timeout) which is false
until after the timeout, so the loop body never executes.

Fix by using readl_poll_timeout as a more standard and less error-prone
solution.

Fixes: ba37b7caf1ed ("net: ethernet: mtk_eth_soc: add support for initializing 
the PPE")
Signed-off-by: Ilya Lipnitskiy 
Cc: Felix Fietkau 
---
 drivers/net/ethernet/mediatek/mtk_ppe.c | 18 +-
 drivers/net/ethernet/mediatek/mtk_ppe.h |  1 +
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_ppe.c 
b/drivers/net/ethernet/mediatek/mtk_ppe.c
index 71e1ccea6e72..f4b3fc0eeb50 100644
--- a/drivers/net/ethernet/mediatek/mtk_ppe.c
+++ b/drivers/net/ethernet/mediatek/mtk_ppe.c
@@ -5,6 +5,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include "mtk_ppe.h"
@@ -44,18 +45,17 @@ static u32 ppe_clear(struct mtk_ppe *ppe, u32 reg, u32 val)
 
 static int mtk_ppe_wait_busy(struct mtk_ppe *ppe)
 {
-   unsigned long timeout = jiffies + HZ;
-
-   while (time_is_before_jiffies(timeout)) {
-   if (!(ppe_r32(ppe, MTK_PPE_GLO_CFG) & MTK_PPE_GLO_CFG_BUSY))
-   return 0;
+   int ret;
+   u32 val;
 
-   usleep_range(10, 20);
-   }
+   ret = readl_poll_timeout(ppe->base + MTK_PPE_GLO_CFG, val,
+!(val & MTK_PPE_GLO_CFG_BUSY),
+20, MTK_PPE_WAIT_TIMEOUT_US);
 
-   dev_err(ppe->dev, "PPE table busy");
+   if (ret)
+   dev_err(ppe->dev, "PPE table busy");
 
-   return -ETIMEDOUT;
+   return ret;
 }
 
 static void mtk_ppe_cache_clear(struct mtk_ppe *ppe)
diff --git a/drivers/net/ethernet/mediatek/mtk_ppe.h 
b/drivers/net/ethernet/mediatek/mtk_ppe.h
index 51bd5e75bbbd..242fb8f2ae65 100644
--- a/drivers/net/ethernet/mediatek/mtk_ppe.h
+++ b/drivers/net/ethernet/mediatek/mtk_ppe.h
@@ -12,6 +12,7 @@
 #define MTK_PPE_ENTRIES_SHIFT  3
 #define MTK_PPE_ENTRIES(1024 << MTK_PPE_ENTRIES_SHIFT)
 #define MTK_PPE_HASH_MASK  (MTK_PPE_ENTRIES - 1)
+#define MTK_PPE_WAIT_TIMEOUT_US100
 
 #define MTK_FOE_IB1_UNBIND_TIMESTAMP   GENMASK(7, 0)
 #define MTK_FOE_IB1_UNBIND_PACKETS GENMASK(23, 8)
-- 
2.31.1



[PATCH net-next] net: ethernet: mediatek: ppe: fix busy wait loop

2021-04-15 Thread Ilya Lipnitskiy
The intention is for the loop to timeout if the body does not succeed.
The current logic calls time_is_before_jiffies(timeout) which is false
until after the timeout, so the loop body never executes.

time_is_after_jiffies(timeout) will return true until timeout is less
than jiffies, which is the intended behavior here.

Fixes: ba37b7caf1ed ("net: ethernet: mtk_eth_soc: add support for initializing 
the PPE")
Signed-off-by: Ilya Lipnitskiy 
Cc: Felix Fietkau 
---
 drivers/net/ethernet/mediatek/mtk_ppe.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_ppe.c 
b/drivers/net/ethernet/mediatek/mtk_ppe.c
index 71e1ccea6e72..af3c266297aa 100644
--- a/drivers/net/ethernet/mediatek/mtk_ppe.c
+++ b/drivers/net/ethernet/mediatek/mtk_ppe.c
@@ -46,7 +46,7 @@ static int mtk_ppe_wait_busy(struct mtk_ppe *ppe)
 {
unsigned long timeout = jiffies + HZ;
 
-   while (time_is_before_jiffies(timeout)) {
+   while (time_is_after_jiffies(timeout)) {
if (!(ppe_r32(ppe, MTK_PPE_GLO_CFG) & MTK_PPE_GLO_CFG_BUSY))
return 0;
 
-- 
2.31.1



Re: [PATCH] staging: mt7621-pci: stop using of_pci_range_to_resource

2021-04-13 Thread Ilya Lipnitskiy
Hi Sergio,

Just as an aside, are you planning to move staging/mt7621-pci into
arch/mips/pci at some point? This driver seems more maintained (by
you!) than many in-tree drivers...

Ilya

On Sat, Apr 10, 2021 at 12:23 PM Sergio Paracuellos
 wrote:
>
> Hi Ilya,
>
> On Sat, Apr 10, 2021 at 7:33 PM Ilya Lipnitskiy
>  wrote:
> >
> > The logic here was already overriding the erroneous IO addresses
> > returned from of_pci_range_to_resource, which is the bulk of the logic.
> >
> > So stop using it altogether and initialize the fields explicitly, as
> > done in aeba3731b150 ("powerpc/pci: Fix IO space breakage after
> > of_pci_range_to_resource() change").
> >
> > Signed-off-by: Ilya Lipnitskiy 
> > Cc: Sergio Paracuellos 
> > ---
> >  drivers/staging/mt7621-pci/pci-mt7621.c | 11 ++-
> >  1 file changed, 6 insertions(+), 5 deletions(-)
>
> Looks good to me, thanks! I have also tested this in gnubee pc1
> platform with no regressions at all when io bars are assigned:
>
> [   16.378956] mt7621-pci 1e14.pcie: host bridge /pcie@1e14 ranges:
> [   16.392405] mt7621-pci 1e14.pcie:  MEM
> 0x006000..0x006fff -> 0x00
> [   16.408796] mt7621-pci 1e14.pcie:   IO
> 0x001e16..0x001e16 -> 0x00
> [   16.425264] mt7621-pci-phy 1e149000.pcie-phy: PHY for 0xbe149000
> (dual port = 1)
> [   16.440452] mt7621-pci-phy 1e14a000.pcie-phy: PHY for 0xbe14a000
> (dual port = 0)
> [   16.678713] mt7621-pci 1e14.pcie: PCIE0 enabled
> [   16.688435] mt7621-pci 1e14.pcie: PCIE1 enabled
> [   16.698160] mt7621-pci 1e14.pcie: PCIE2 enabled
> [   16.707886] mt7621-pci 1e14.pcie: PCI coherence region base:
> 0x6000, mask/settings: 0xf002
> [   16.726623] mt7621-pci 1e14.pcie: PCI host bridge to bus :00
> [   16.739309] pci_bus :00: root bus resource [io  0x1e16-0x1e16]
> [   16.753008] pci_bus :00: root bus resource [mem 0x6000-0x6fff]
> [   16.766709] pci_bus :00: root bus resource [bus 00-ff]
> [   16.777649] pci_bus :00: root bus resource [mem
> 0x6000-0x6fff] (bus address [0x-0x0fff])
> [   16.797986] pci :00:00.0: [0e8d:0801] type 01 class 0x060400
> [   16.809973] pci :00:00.0: reg 0x10: [mem 0x-0x7fff]
> [   16.822467] pci :00:00.0: reg 0x14: initial BAR value 0x 
> invalid
> [   16.836511] pci :00:00.0: reg 0x14: [mem size 0x0001]
> [   16.848048] pci :00:00.0: supports D1
> [   16.856051] pci :00:00.0: PME# supported from D0 D1 D3hot
> [   16.867943] pci :00:01.0: [0e8d:0801] type 01 class 0x060400
> [   16.879960] pci :00:01.0: reg 0x10: [mem 0x-0x7fff]
> [   16.892454] pci :00:01.0: reg 0x14: initial BAR value 0x 
> invalid
> [   16.906497] pci :00:01.0: reg 0x14: [mem size 0x0001]
> [   16.918040] pci :00:01.0: supports D1
> [   16.926031] pci :00:01.0: PME# supported from D0 D1 D3hot
> [   16.937903] pci :00:02.0: [0e8d:0801] type 01 class 0x060400
> [   16.949915] pci :00:02.0: reg 0x10: [mem 0x-0x7fff]
> [   16.962412] pci :00:02.0: reg 0x14: initial BAR value 0x 
> invalid
> [   16.976466] pci :00:02.0: reg 0x14: [mem size 0x0001]
> [   16.987991] pci :00:02.0: supports D1
> [   16.995986] pci :00:02.0: PME# supported from D0 D1 D3hot
> [   17.008716] pci :00:00.0: bridge configuration invalid ([bus
> 00-00]), reconfiguring
> [   17.024695] pci :00:01.0: bridge configuration invalid ([bus
> 00-00]), reconfiguring
> [   17.040654] pci :00:02.0: bridge configuration invalid ([bus
> 00-00]), reconfiguring
> [   17.056868] pci :01:00.0: [1b21:0611] type 00 class 0x010185
> [   17.068882] pci :01:00.0: reg 0x10: [io  0x-0x0007]
> [   17.080003] pci :01:00.0: reg 0x14: [io  0x-0x0003]
> [   17.091115] pci :01:00.0: reg 0x18: [io  0x-0x0007]
> [   17.102238] pci :01:00.0: reg 0x1c: [io  0x-0x0003]
> [   17.113350] pci :01:00.0: reg 0x20: [io  0x-0x000f]
> [   17.124463] pci :01:00.0: reg 0x24: initial BAR value 0x 
> invalid
> [   17.138508] pci :01:00.0: reg 0x24: [mem size 0x0200]
> [   17.150115] pci :01:00.0: 2.000 Gb/s available PCIe bandwidth,
> limited by 2.5 GT/s PCIe x1 link at :00:00.0 (capable of 4.000
> Gb/s with 5.0 GT/s PCIe x1 link)
> [   17.204594] pci :00:00.0: PCI bridge to [bus 01-ff]
> [   17.215109] pci :00:00.0:   bridge window [io  0x-0x0fff]
> [   17.227257] pci :00:00.0:   bridge window [mem 0x6000-0x600f]
> [   17.240785] pci :00:00.0:   bridge window [mem
> 0x6000-0x600f pref]
> [   17.255183] 

[PATCH v2 6/8] MIPS: pci-legacy: remove redundant info messages

2021-04-13 Thread Ilya Lipnitskiy
Remove the following pci-legacy message:
  PCI host bridge /pci@44/host-bridge ranges:
   MEM 0x2000..0x2fff
IO 0x0046..0x0046

It is followed shortly by the same data from pci_register_host_bridge:
  PCI host bridge to bus :00
  pci_bus :00: root bus resource [mem 0x2000-0x2fff]
  pci_bus :00: root bus resource [io  0x46-0x46]

Signed-off-by: Ilya Lipnitskiy 
---
 arch/mips/pci/pci-legacy.c | 7 ---
 1 file changed, 7 deletions(-)

diff --git a/arch/mips/pci/pci-legacy.c b/arch/mips/pci/pci-legacy.c
index 3a909194284a..ec3f52ade72d 100644
--- a/arch/mips/pci/pci-legacy.c
+++ b/arch/mips/pci/pci-legacy.c
@@ -140,7 +140,6 @@ void pci_load_of_ranges(struct pci_controller *hose, struct 
device_node *node)
struct of_pci_range range;
struct of_pci_range_parser parser;
 
-   pr_info("PCI host bridge %pOF ranges:\n", node);
hose->of_node = node;
 
if (of_pci_range_parser_init(, node))
@@ -151,18 +150,12 @@ void pci_load_of_ranges(struct pci_controller *hose, 
struct device_node *node)
 
switch (range.flags & IORESOURCE_TYPE_BITS) {
case IORESOURCE_IO:
-   pr_info("  IO 0x%016llx..0x%016llx\n",
-   range.cpu_addr,
-   range.cpu_addr + range.size - 1);
hose->io_map_base =
(unsigned long)ioremap(range.cpu_addr,
   range.size);
res = hose->io_resource;
break;
case IORESOURCE_MEM:
-   pr_info(" MEM 0x%016llx..0x%016llx\n",
-   range.cpu_addr,
-   range.cpu_addr + range.size - 1);
res = hose->mem_resource;
break;
}
-- 
2.31.1



[PATCH v2 8/8] MIPS: pci-legacy: use generic pci_enable_resources

2021-04-13 Thread Ilya Lipnitskiy
Follow the reasoning from commit 842de40d93e0 ("PCI: add generic
pci_enable_resources()"):

  The only functional difference from the MIPS version is that the
  generic one uses "!r->parent" to check for resource collisions
  instead of "!r->start && r->end".

That should have no effect on any pci-legacy driver.

Suggested-by: Bjorn Helgaas 
Signed-off-by: Ilya Lipnitskiy 
---
 arch/mips/pci/pci-legacy.c | 40 ++
 1 file changed, 2 insertions(+), 38 deletions(-)

diff --git a/arch/mips/pci/pci-legacy.c b/arch/mips/pci/pci-legacy.c
index 78c22987bef0..c24226ea0a6e 100644
--- a/arch/mips/pci/pci-legacy.c
+++ b/arch/mips/pci/pci-legacy.c
@@ -241,47 +241,11 @@ static int __init pcibios_init(void)
 
 subsys_initcall(pcibios_init);
 
-static int pcibios_enable_resources(struct pci_dev *dev, int mask)
-{
-   u16 cmd, old_cmd;
-   int idx;
-   struct resource *r;
-
-   pci_read_config_word(dev, PCI_COMMAND, );
-   old_cmd = cmd;
-   for (idx=0; idx < PCI_NUM_RESOURCES; idx++) {
-   /* Only set up the requested stuff */
-   if (!(mask & (1<resource[idx];
-   if (!(r->flags & (IORESOURCE_IO | IORESOURCE_MEM)))
-   continue;
-   if ((idx == PCI_ROM_RESOURCE) &&
-   (!(r->flags & IORESOURCE_ROM_ENABLE)))
-   continue;
-   if (!r->start && r->end) {
-   pci_err(dev,
-   "can't enable device: resource collisions\n");
-   return -EINVAL;
-   }
-   if (r->flags & IORESOURCE_IO)
-   cmd |= PCI_COMMAND_IO;
-   if (r->flags & IORESOURCE_MEM)
-   cmd |= PCI_COMMAND_MEMORY;
-   }
-   if (cmd != old_cmd) {
-   pci_info(dev, "enabling device (%04x -> %04x)\n", old_cmd, cmd);
-   pci_write_config_word(dev, PCI_COMMAND, cmd);
-   }
-   return 0;
-}
-
 int pcibios_enable_device(struct pci_dev *dev, int mask)
 {
-   int err;
+   int err = pci_enable_resources(dev, mask);
 
-   if ((err = pcibios_enable_resources(dev, mask)) < 0)
+   if (err < 0)
return err;
 
return pcibios_plat_dev_init(dev);
-- 
2.31.1



[PATCH v2 3/8] MIPS: pci-rt3883: trivial: remove unused variable

2021-04-13 Thread Ilya Lipnitskiy
Fixes the following compiler warning:
  warning: unused variable 'flags' [-Wunused-variable]

Fixes: e5067c718b3a ("MIPS: pci-rt3883: Remove odd locking in PCI config space 
access code")
Signed-off-by: Ilya Lipnitskiy 
Acked-by: Sergey Ryazanov 
Cc: triv...@kernel.org
---
 arch/mips/pci/pci-rt3883.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/arch/mips/pci/pci-rt3883.c b/arch/mips/pci/pci-rt3883.c
index 0ac6346026d0..e422f78db5bc 100644
--- a/arch/mips/pci/pci-rt3883.c
+++ b/arch/mips/pci/pci-rt3883.c
@@ -100,7 +100,6 @@ static u32 rt3883_pci_read_cfg32(struct 
rt3883_pci_controller *rpc,
   unsigned bus, unsigned slot,
   unsigned func, unsigned reg)
 {
-   unsigned long flags;
u32 address;
u32 ret;
 
@@ -116,7 +115,6 @@ static void rt3883_pci_write_cfg32(struct 
rt3883_pci_controller *rpc,
 unsigned bus, unsigned slot,
 unsigned func, unsigned reg, u32 val)
 {
-   unsigned long flags;
u32 address;
 
address = rt3883_pci_get_cfgaddr(bus, slot, func, reg);
@@ -229,7 +227,6 @@ static int rt3883_pci_config_read(struct pci_bus *bus, 
unsigned int devfn,
  int where, int size, u32 *val)
 {
struct rt3883_pci_controller *rpc;
-   unsigned long flags;
u32 address;
u32 data;
 
@@ -263,7 +260,6 @@ static int rt3883_pci_config_write(struct pci_bus *bus, 
unsigned int devfn,
   int where, int size, u32 val)
 {
struct rt3883_pci_controller *rpc;
-   unsigned long flags;
u32 address;
u32 data;
 
-- 
2.31.1



[PATCH v2 7/8] MIPS: pci-legacy: remove busn_resource field

2021-04-13 Thread Ilya Lipnitskiy
No drivers set the busn_resource field in the pci_controller struct.
Commit 7ee214b540d9 ("MIPS: PCI: Remove unused busn_offset") almost
removed it over 3 years ago. Remove it for good to free up memory and
eliminate messages like:
  pci_bus :00: root bus resource [??? 0x flags 0x0]

Signed-off-by: Ilya Lipnitskiy 
Cc: Bjorn Helgaas 
---
 arch/mips/include/asm/pci.h | 1 -
 arch/mips/pci/pci-legacy.c  | 1 -
 2 files changed, 2 deletions(-)

diff --git a/arch/mips/include/asm/pci.h b/arch/mips/include/asm/pci.h
index 6f48649201c5..9ffc8192adae 100644
--- a/arch/mips/include/asm/pci.h
+++ b/arch/mips/include/asm/pci.h
@@ -38,7 +38,6 @@ struct pci_controller {
struct resource *io_resource;
unsigned long io_offset;
unsigned long io_map_base;
-   struct resource *busn_resource;
 
 #ifndef CONFIG_PCI_DOMAINS_GENERIC
unsigned int index;
diff --git a/arch/mips/pci/pci-legacy.c b/arch/mips/pci/pci-legacy.c
index ec3f52ade72d..78c22987bef0 100644
--- a/arch/mips/pci/pci-legacy.c
+++ b/arch/mips/pci/pci-legacy.c
@@ -89,7 +89,6 @@ static void pcibios_scanbus(struct pci_controller *hose)
hose->mem_resource, hose->mem_offset);
pci_add_resource_offset(,
hose->io_resource, hose->io_offset);
-   pci_add_resource(, hose->busn_resource);
list_splice_init(, >windows);
bridge->dev.parent = NULL;
bridge->sysdata = hose;
-- 
2.31.1



[PATCH v2 5/8] MIPS: pci-legacy: stop using of_pci_range_to_resource

2021-04-13 Thread Ilya Lipnitskiy
Mirror commit aeba3731b150 ("powerpc/pci: Fix IO space breakage after
of_pci_range_to_resource() change").

Most MIPS platforms do not define PCI_IOBASE, nor implement
pci_address_to_pio(). Moreover, IO_SPACE_LIMIT is 0x for most MIPS
platforms. of_pci_range_to_resource passes the _start address_ of the IO
range into pci_address_to_pio, which then checks it against
IO_SPACE_LIMIT and fails, because for MIPS platforms that use
pci-legacy (pci-lantiq, pci-rt3883, pci-mt7620), IO ranges start much
higher than 0x.

In fact, pci-mt7621 in staging already works around this problem, see
commit 09dd629eeabb ("staging: mt7621-pci: fix io space and properly set
resource limits")

So just stop using of_pci_range_to_resource, which does not work for
MIPS.

Fixes PCI errors like:
  pci_bus :00: root bus resource [io  0x]

Fixes: 0b0b0893d49b ("of/pci: Fix the conversion of IO ranges into IO 
resources")
Signed-off-by: Ilya Lipnitskiy 
Cc: Liviu Dudau 
---
 arch/mips/pci/pci-legacy.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/mips/pci/pci-legacy.c b/arch/mips/pci/pci-legacy.c
index 39052de915f3..3a909194284a 100644
--- a/arch/mips/pci/pci-legacy.c
+++ b/arch/mips/pci/pci-legacy.c
@@ -166,8 +166,13 @@ void pci_load_of_ranges(struct pci_controller *hose, 
struct device_node *node)
res = hose->mem_resource;
break;
}
-   if (res != NULL)
-   of_pci_range_to_resource(, node, res);
+   if (res != NULL) {
+   res->name = node->full_name;
+   res->flags = range.flags;
+   res->start = range.cpu_addr;
+   res->end = range.cpu_addr + range.size - 1;
+   res->parent = res->child = res->sibling = NULL;
+   }
}
 }
 
-- 
2.31.1



[PATCH v2 1/8] MIPS: pci-rt2880: fix slot 0 configuration

2021-04-13 Thread Ilya Lipnitskiy
pci_fixup_irqs() used to call pcibios_map_irq on every PCI device, which
for RT2880 included bus 0 slot 0. After pci_fixup_irqs() got removed,
only slots/funcs with devices attached would be called. While arguably
the right thing, that left no chance for this driver to ever initialize
slot 0, effectively bricking PCI and USB on RT2880 devices such as the
Belkin F5D8235-4 v1.

Slot 0 configuration needs to happen after PCI bus enumeration, but
before any device at slot 0x11 (func 0 or 1) is talked to. That was
determined empirically by testing on a Belkin F5D8235-4 v1 device. A
minimal BAR 0 config write followed by read, then setting slot 0
PCI_COMMAND to MASTER | IO | MEMORY is all that seems to be required for
proper functionality.

Tested by ensuring that full- and high-speed USB devices get enumerated
on the Belkin F5D8235-4 v1 (with an out of tree DTS file from OpenWrt).

Fixes: 04c81c7293df ("MIPS: PCI: Replace pci_fixup_irqs() call with host bridge 
IRQ mapping hooks")
Signed-off-by: Ilya Lipnitskiy 
Cc: Lorenzo Pieralisi 
Cc: Tobias Wolf 
Cc:  # v4.14+
---
 arch/mips/pci/pci-rt2880.c | 37 -
 1 file changed, 24 insertions(+), 13 deletions(-)

diff --git a/arch/mips/pci/pci-rt2880.c b/arch/mips/pci/pci-rt2880.c
index e1f12e398136..f1538d2be89e 100644
--- a/arch/mips/pci/pci-rt2880.c
+++ b/arch/mips/pci/pci-rt2880.c
@@ -180,7 +180,6 @@ static inline void rt2880_pci_write_u32(unsigned long reg, 
u32 val)
 
 int pcibios_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
 {
-   u16 cmd;
int irq = -1;
 
if (dev->bus->number != 0)
@@ -188,8 +187,6 @@ int pcibios_map_irq(const struct pci_dev *dev, u8 slot, u8 
pin)
 
switch (PCI_SLOT(dev->devfn)) {
case 0x00:
-   rt2880_pci_write_u32(PCI_BASE_ADDRESS_0, 0x0800);
-   (void) rt2880_pci_read_u32(PCI_BASE_ADDRESS_0);
break;
case 0x11:
irq = RT288X_CPU_IRQ_PCI;
@@ -201,16 +198,6 @@ int pcibios_map_irq(const struct pci_dev *dev, u8 slot, u8 
pin)
break;
}
 
-   pci_write_config_byte((struct pci_dev *) dev,
-   PCI_CACHE_LINE_SIZE, 0x14);
-   pci_write_config_byte((struct pci_dev *) dev, PCI_LATENCY_TIMER, 0xFF);
-   pci_read_config_word((struct pci_dev *) dev, PCI_COMMAND, );
-   cmd |= PCI_COMMAND_MASTER | PCI_COMMAND_IO | PCI_COMMAND_MEMORY |
-   PCI_COMMAND_INVALIDATE | PCI_COMMAND_FAST_BACK |
-   PCI_COMMAND_SERR | PCI_COMMAND_WAIT | PCI_COMMAND_PARITY;
-   pci_write_config_word((struct pci_dev *) dev, PCI_COMMAND, cmd);
-   pci_write_config_byte((struct pci_dev *) dev, PCI_INTERRUPT_LINE,
- dev->irq);
return irq;
 }
 
@@ -251,6 +238,30 @@ static int rt288x_pci_probe(struct platform_device *pdev)
 
 int pcibios_plat_dev_init(struct pci_dev *dev)
 {
+   static bool slot0_init;
+
+   /*
+* Nobody seems to initialize slot 0, but this platform requires it, so
+* do it once when some other slot is being enabled. The PCI subsystem
+* should configure other slots properly, so no need to do anything
+* special for those.
+*/
+   if (!slot0_init && dev->bus->number == 0) {
+   u16 cmd;
+   u32 bar0;
+
+   slot0_init = true;
+
+   pci_bus_write_config_dword(dev->bus, 0, PCI_BASE_ADDRESS_0,
+  0x0800);
+   pci_bus_read_config_dword(dev->bus, 0, PCI_BASE_ADDRESS_0,
+ );
+
+   pci_bus_read_config_word(dev->bus, 0, PCI_COMMAND, );
+   cmd |= PCI_COMMAND_MASTER | PCI_COMMAND_IO | PCI_COMMAND_MEMORY;
+   pci_bus_write_config_word(dev->bus, 0, PCI_COMMAND, cmd);
+   }
+
return 0;
 }
 
-- 
2.31.1



[PATCH v2 4/8] MIPS: pci-rt3883: more accurate DT error messages

2021-04-13 Thread Ilya Lipnitskiy
Existing strings do not make sense: one is always NULL and the other
refers to the wrong parent node.

Signed-off-by: Ilya Lipnitskiy 
---
 arch/mips/pci/pci-rt3883.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/arch/mips/pci/pci-rt3883.c b/arch/mips/pci/pci-rt3883.c
index e422f78db5bc..aebd4964ea34 100644
--- a/arch/mips/pci/pci-rt3883.c
+++ b/arch/mips/pci/pci-rt3883.c
@@ -431,8 +431,7 @@ static int rt3883_pci_probe(struct platform_device *pdev)
 
if (!rpc->intc_of_node) {
dev_err(dev, "%pOF has no %s child node",
-   rpc->intc_of_node,
-   "interrupt controller");
+   np, "interrupt controller");
return -EINVAL;
}
 
@@ -446,8 +445,7 @@ static int rt3883_pci_probe(struct platform_device *pdev)
 
if (!rpc->pci_controller.of_node) {
dev_err(dev, "%pOF has no %s child node",
-   rpc->intc_of_node,
-   "PCI host bridge");
+   np, "PCI host bridge");
err = -EINVAL;
goto err_put_intc_node;
}
-- 
2.31.1



[PATCH v2 0/8] MIPS: fixes for PCI legacy drivers (rt2880, rt3883)

2021-04-13 Thread Ilya Lipnitskiy
One major fix for rt2880-pci in the first patch - fixes breakage that
existed since v4.14.

Other more minor fixes, cleanups, and improvements that either free up
memory, make dmesg messages clearer, or remove redundant dmesg output.

v2:
- Do not use internal pci-rt2880 config read and write functions after
  the device has been registered with the PCI subsystem to avoid races.
  Use safe pci_bus_{read,write}_config_{d}word wrappers instead.

Ilya Lipnitskiy (8):
  MIPS: pci-rt2880: fix slot 0 configuration
  MIPS: pci-rt2880: remove unneeded locks
  MIPS: pci-rt3883: trivial: remove unused variable
  MIPS: pci-rt3883: more accurate DT error messages
  MIPS: pci-legacy: stop using of_pci_range_to_resource
  MIPS: pci-legacy: remove redundant info messages
  MIPS: pci-legacy: remove busn_resource field
  MIPS: pci-legacy: use generic pci_enable_resources

 arch/mips/include/asm/pci.h |  1 -
 arch/mips/pci/pci-legacy.c  | 57 ++---
 arch/mips/pci/pci-rt2880.c  | 50 
 arch/mips/pci/pci-rt3883.c  | 10 ++-
 4 files changed, 35 insertions(+), 83 deletions(-)

-- 
2.31.1



[PATCH v2 2/8] MIPS: pci-rt2880: remove unneeded locks

2021-04-13 Thread Ilya Lipnitskiy
Mirror pci-rt3883 fix from commit e5067c718b3a ("MIPS: pci-rt3883:
Remove odd locking in PCI config space access code"). pci-rt2880 shares
the driver layout with pci-rt3883 and the same reasons apply.

Caller (generic PCI code) already does proper locking, so no need to add
another one here. Local PCI read/write functions are never called
simultaneously, also they do not require synchronization with the PCI
controller ops, since they are used before the controller registration.

Suggested-by: Sergey Ryazanov 
Signed-off-by: Ilya Lipnitskiy 
---
 arch/mips/pci/pci-rt2880.c | 13 -
 1 file changed, 13 deletions(-)

diff --git a/arch/mips/pci/pci-rt2880.c b/arch/mips/pci/pci-rt2880.c
index f1538d2be89e..e9dd01431f21 100644
--- a/arch/mips/pci/pci-rt2880.c
+++ b/arch/mips/pci/pci-rt2880.c
@@ -41,7 +41,6 @@
 #define RT2880_PCI_REG_ARBCTL  0x80
 
 static void __iomem *rt2880_pci_base;
-static DEFINE_SPINLOCK(rt2880_pci_lock);
 
 static u32 rt2880_pci_reg_read(u32 reg)
 {
@@ -63,17 +62,14 @@ static inline u32 rt2880_pci_get_cfgaddr(unsigned int bus, 
unsigned int slot,
 static int rt2880_pci_config_read(struct pci_bus *bus, unsigned int devfn,
  int where, int size, u32 *val)
 {
-   unsigned long flags;
u32 address;
u32 data;
 
address = rt2880_pci_get_cfgaddr(bus->number, PCI_SLOT(devfn),
 PCI_FUNC(devfn), where);
 
-   spin_lock_irqsave(_pci_lock, flags);
rt2880_pci_reg_write(address, RT2880_PCI_REG_CONFIG_ADDR);
data = rt2880_pci_reg_read(RT2880_PCI_REG_CONFIG_DATA);
-   spin_unlock_irqrestore(_pci_lock, flags);
 
switch (size) {
case 1:
@@ -93,14 +89,12 @@ static int rt2880_pci_config_read(struct pci_bus *bus, 
unsigned int devfn,
 static int rt2880_pci_config_write(struct pci_bus *bus, unsigned int devfn,
   int where, int size, u32 val)
 {
-   unsigned long flags;
u32 address;
u32 data;
 
address = rt2880_pci_get_cfgaddr(bus->number, PCI_SLOT(devfn),
 PCI_FUNC(devfn), where);
 
-   spin_lock_irqsave(_pci_lock, flags);
rt2880_pci_reg_write(address, RT2880_PCI_REG_CONFIG_ADDR);
data = rt2880_pci_reg_read(RT2880_PCI_REG_CONFIG_DATA);
 
@@ -119,7 +113,6 @@ static int rt2880_pci_config_write(struct pci_bus *bus, 
unsigned int devfn,
}
 
rt2880_pci_reg_write(data, RT2880_PCI_REG_CONFIG_DATA);
-   spin_unlock_irqrestore(_pci_lock, flags);
 
return PCIBIOS_SUCCESSFUL;
 }
@@ -151,31 +144,25 @@ static struct pci_controller rt2880_pci_controller = {
 
 static inline u32 rt2880_pci_read_u32(unsigned long reg)
 {
-   unsigned long flags;
u32 address;
u32 ret;
 
address = rt2880_pci_get_cfgaddr(0, 0, 0, reg);
 
-   spin_lock_irqsave(_pci_lock, flags);
rt2880_pci_reg_write(address, RT2880_PCI_REG_CONFIG_ADDR);
ret = rt2880_pci_reg_read(RT2880_PCI_REG_CONFIG_DATA);
-   spin_unlock_irqrestore(_pci_lock, flags);
 
return ret;
 }
 
 static inline void rt2880_pci_write_u32(unsigned long reg, u32 val)
 {
-   unsigned long flags;
u32 address;
 
address = rt2880_pci_get_cfgaddr(0, 0, 0, reg);
 
-   spin_lock_irqsave(_pci_lock, flags);
rt2880_pci_reg_write(address, RT2880_PCI_REG_CONFIG_ADDR);
rt2880_pci_reg_write(val, RT2880_PCI_REG_CONFIG_DATA);
-   spin_unlock_irqrestore(_pci_lock, flags);
 }
 
 int pcibios_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
-- 
2.31.1



Re: [PATCH 2/8] MIPS: pci-rt2880: remove unneeded locks

2021-04-13 Thread Ilya Lipnitskiy
Hi Sergey,

On Tue, Apr 13, 2021 at 6:40 AM Sergey Ryazanov  wrote:
>
> On Tue, Apr 13, 2021 at 4:28 PM Sergey Ryazanov  
> wrote:
> > On Tue, Apr 13, 2021 at 9:22 AM Ilya Lipnitskiy
> >  wrote:
> > > Mirror pci-rt3883 fix from commit e5067c718b3a ("MIPS: pci-rt3883:
> > > Remove odd locking in PCI config space access code"). pci-rt2880 shares
> > > the driver layout with pci-rt3883 and the same reasons apply.
> > >
> > > Caller (generic PCI code) already does proper locking, so no need to add
> > > another one here. Local PCI read/write functions are never called
> > > simultaneously, also they do not require synchronization with the PCI
> > > controller ops, since they are used before the controller registration.
> > >
> > > Suggested-by: Sergey Ryazanov 
> > > Signed-off-by: Ilya Lipnitskiy 
> > > ---
> > >  arch/mips/pci/pci-rt2880.c | 13 -
> > >  1 file changed, 13 deletions(-)
> > >
> > > diff --git a/arch/mips/pci/pci-rt2880.c b/arch/mips/pci/pci-rt2880.c
> > > index 19f7860fb28b..b4ee07cbcf2a 100644
> > > --- a/arch/mips/pci/pci-rt2880.c
> > > +++ b/arch/mips/pci/pci-rt2880.c
> > > @@ -41,7 +41,6 @@
> > >  #define RT2880_PCI_REG_ARBCTL  0x80
> > >
> > >  static void __iomem *rt2880_pci_base;
> > > -static DEFINE_SPINLOCK(rt2880_pci_lock);
> > >
> > >  static u32 rt2880_pci_reg_read(u32 reg)
> > >  {
> > > @@ -63,7 +62,6 @@ static inline u32 rt2880_pci_get_cfgaddr(unsigned int 
> > > bus, unsigned int slot,
> > >  static int rt2880_pci_config_read(struct pci_bus *bus, unsigned int 
> > > devfn,
> > >   int where, int size, u32 *val)
> > >  {
> > > -   unsigned long flags;
> > > u32 address;
> > > u32 data;
> > > int busn = 0;
> > > @@ -74,10 +72,8 @@ static int rt2880_pci_config_read(struct pci_bus *bus, 
> > > unsigned int devfn,
> > > address = rt2880_pci_get_cfgaddr(busn, PCI_SLOT(devfn), 
> > > PCI_FUNC(devfn),
> > >  where);
> > >
> > > -   spin_lock_irqsave(_pci_lock, flags);
> > > rt2880_pci_reg_write(address, RT2880_PCI_REG_CONFIG_ADDR);
> > > data = rt2880_pci_reg_read(RT2880_PCI_REG_CONFIG_DATA);
> > > -   spin_unlock_irqrestore(_pci_lock, flags);
> > >
> > > switch (size) {
> > > case 1:
> > > @@ -97,7 +93,6 @@ static int rt2880_pci_config_read(struct pci_bus *bus, 
> > > unsigned int devfn,
> > >  static int rt2880_pci_config_write(struct pci_bus *bus, unsigned int 
> > > devfn,
> > >int where, int size, u32 val)
> > >  {
> > > -   unsigned long flags;
> > > u32 address;
> > > u32 data;
> > > int busn = 0;
> > > @@ -108,7 +103,6 @@ static int rt2880_pci_config_write(struct pci_bus 
> > > *bus, unsigned int devfn,
> > > address = rt2880_pci_get_cfgaddr(busn, PCI_SLOT(devfn), 
> > > PCI_FUNC(devfn),
> > >  where);
> > >
> > > -   spin_lock_irqsave(_pci_lock, flags);
> > > rt2880_pci_reg_write(address, RT2880_PCI_REG_CONFIG_ADDR);
> > > data = rt2880_pci_reg_read(RT2880_PCI_REG_CONFIG_DATA);
> > >
> > > @@ -127,7 +121,6 @@ static int rt2880_pci_config_write(struct pci_bus 
> > > *bus, unsigned int devfn,
> > > }
> > >
> > > rt2880_pci_reg_write(data, RT2880_PCI_REG_CONFIG_DATA);
> > > -   spin_unlock_irqrestore(_pci_lock, flags);
> > >
> > > return PCIBIOS_SUCCESSFUL;
> > >  }
> > > @@ -159,31 +152,25 @@ static struct pci_controller rt2880_pci_controller 
> > > = {
> > >
> > >  static inline u32 rt2880_pci_read_u32(unsigned long reg)
> > >  {
> > > -   unsigned long flags;
> > > u32 address;
> > > u32 ret;
> > >
> > > address = rt2880_pci_get_cfgaddr(0, 0, 0, reg);
> > >
> > > -   spin_lock_irqsave(_pci_lock, flags);
> > > rt2880_pci_reg_write(address, RT2880_PCI_REG_CONFIG_ADDR);
> > > ret = rt2880_pci_reg_read(RT2880_PCI_REG_CONFIG_DATA);
> > > -   spin_unlock_irqrestore(_pci_lock, flags);
> > >
> > >   

Re: [PATCH] MIPS: fix memory reservation for non-usermem setups

2021-04-13 Thread Ilya Lipnitskiy
On Mon, Apr 12, 2021 at 11:45 PM Ilya Lipnitskiy
 wrote:
>
> Hi Thomas,
>
> On Tue, Apr 6, 2021 at 6:18 AM Thomas Bogendoerfer
>  wrote:
> >
> > On Sat, Apr 03, 2021 at 07:02:13PM -0700, Ilya Lipnitskiy wrote:
> > > Hi Mike,
> > >
> > > On Tue, Mar 16, 2021 at 11:33 PM Mike Rapoport  wrote:
> > > >
> > > > Hi Ilya,
> > > >
> > > > On Tue, Mar 16, 2021 at 10:10:09PM -0700, Ilya Lipnitskiy wrote:
> > > > > Hi Thomas,
> > > > >
> > > > > On Fri, Mar 12, 2021 at 7:19 AM Thomas Bogendoerfer
> > > > >  wrote:
> > > > > >
> > > > > > On Sun, Mar 07, 2021 at 11:40:30AM -0800, Ilya Lipnitskiy wrote:
> > > > > > > From: Tobias Wolf 
> > > > > > >
> > > > > > > Commit 67a3ba25aa95 ("MIPS: Fix incorrect mem=X@Y handling") 
> > > > > > > introduced a new
> > > > > > > issue for rt288x where "PHYS_OFFSET" is 0x0 but the calculated 
> > > > > > > "ramstart" is
> > > > > > > not. As the prerequisite of custom memory map has been removed, 
> > > > > > > this results
> > > > > > > in the full memory range of 0x0 - 0x800 to be marked as 
> > > > > > > reserved for this
> > > > > > > platform.
> > > > > >
> > > > > > and where is the problem here ?
> > > > > Turns out this was already attempted to be upstreamed - not clear why
> > > > > it wasn't merged. Context:
> > > > > https://lore.kernel.org/linux-mips/6504517.U6H5IhoIOn@loki/
> > > > >
> > > > > I hope the thread above helps you understand the problem.
> > > >
> > > > The memory initialization was a bit different then. Do you still see the
> > > > same problem?
> > > Thanks for asking. I obtained a RT2880 device and gave it a try. It
> > > hangs at boot without this patch, however selecting
> >
> > can you provide debug logs with memblock=debug for both good and bad
> > kernels ? I'm curious what's the reason for failing allocation...
> Sorry for taking a while to respond. See attached.
> FWIW, it seems these are the lines that stand out in hang.log:
> [0.00] memblock_reserve: [0x-0x07ff] 
> setup_arch+0x214/0x5d8
> [0.00] Wasting 1048576 bytes for tracking 32768 unused pages
> ...
> [0.00]  reserved[0x0][0x-0x087137aa], 0x087137ab
> bytes flags: 0x0
Just to be clear, good.log is mips-next tip (dbd815c0dcca) and
hang.log is the same with MIPS_AUTO_PFN_OFFSET _NOT_ selected.

Ilya


Re: [PATCH] MIPS: fix memory reservation for non-usermem setups

2021-04-13 Thread Ilya Lipnitskiy
Hi Thomas,

On Tue, Apr 6, 2021 at 6:18 AM Thomas Bogendoerfer
 wrote:
>
> On Sat, Apr 03, 2021 at 07:02:13PM -0700, Ilya Lipnitskiy wrote:
> > Hi Mike,
> >
> > On Tue, Mar 16, 2021 at 11:33 PM Mike Rapoport  wrote:
> > >
> > > Hi Ilya,
> > >
> > > On Tue, Mar 16, 2021 at 10:10:09PM -0700, Ilya Lipnitskiy wrote:
> > > > Hi Thomas,
> > > >
> > > > On Fri, Mar 12, 2021 at 7:19 AM Thomas Bogendoerfer
> > > >  wrote:
> > > > >
> > > > > On Sun, Mar 07, 2021 at 11:40:30AM -0800, Ilya Lipnitskiy wrote:
> > > > > > From: Tobias Wolf 
> > > > > >
> > > > > > Commit 67a3ba25aa95 ("MIPS: Fix incorrect mem=X@Y handling") 
> > > > > > introduced a new
> > > > > > issue for rt288x where "PHYS_OFFSET" is 0x0 but the calculated 
> > > > > > "ramstart" is
> > > > > > not. As the prerequisite of custom memory map has been removed, 
> > > > > > this results
> > > > > > in the full memory range of 0x0 - 0x800 to be marked as 
> > > > > > reserved for this
> > > > > > platform.
> > > > >
> > > > > and where is the problem here ?
> > > > Turns out this was already attempted to be upstreamed - not clear why
> > > > it wasn't merged. Context:
> > > > https://lore.kernel.org/linux-mips/6504517.U6H5IhoIOn@loki/
> > > >
> > > > I hope the thread above helps you understand the problem.
> > >
> > > The memory initialization was a bit different then. Do you still see the
> > > same problem?
> > Thanks for asking. I obtained a RT2880 device and gave it a try. It
> > hangs at boot without this patch, however selecting
>
> can you provide debug logs with memblock=debug for both good and bad
> kernels ? I'm curious what's the reason for failing allocation...
Sorry for taking a while to respond. See attached.
FWIW, it seems these are the lines that stand out in hang.log:
[0.00] memblock_reserve: [0x-0x07ff] setup_arch+0x214/0x5d8
[0.00] Wasting 1048576 bytes for tracking 32768 unused pages
...
[0.00]  reserved[0x0][0x-0x087137aa], 0x087137ab
bytes flags: 0x0

Ilya
[0.00] Linux version 5.12.0-rc2+ (builder@buildhost) (mipsel-openwrt-linux-musl-gcc (OpenWrt GCC 7.5.0 r4-7145a72d3ce2) 7.5.0, GNU ld (GNU Binutils) 2.31.1) #4 Mon Apr 12 23:41:18 PDT 2021
[0.00] SoC Type: Ralink RT2880 id:2 rev:1
[0.00] printk: bootconsole [early0] enabled
[0.00] CPU0 revision is: 0001906c (MIPS 4KEc)
[0.00] MIPS: machine is Belkin F5D8235 v1
[0.00] memblock_reserve: [0x085d84a8-0x085d9f5e] setup_arch+0x14c/0x5c0
[0.00] memblock_reserve: [0x0800-0x0871378f] setup_arch+0x220/0x5c0
[0.00] Initrd not found or empty - disabling initrd
[0.00] memblock_alloc_try_nid: 6839 bytes align=0x40 nid=-1 from=0x max_addr=0x early_init_dt_alloc_memory_arch+0x40/0x84
[0.00] memblock_reserve: [0x087137c0-0x08715276] memblock_alloc_range_nid+0xf0/0x184
[0.00] memblock_alloc_try_nid: 21180 bytes align=0x4 nid=-1 from=0x max_addr=0x early_init_dt_alloc_memory_arch+0x40/0x84
[0.00] memblock_reserve: [0x08715278-0x0871a533] memblock_alloc_range_nid+0xf0/0x184
[0.00] memblock_alloc_try_nid: 27 bytes align=0x4 nid=-1 from=0x max_addr=0x early_init_dt_alloc_memory_arch+0x40/0x84
[0.00] memblock_reserve: [0x08713790-0x087137aa] memblock_alloc_range_nid+0xf0/0x184
[0.00] memblock_reserve: [0x08526000-0x08525fff] setup_arch+0x390/0x5c0
[0.00] memblock_alloc_try_nid: 32 bytes align=0x10 nid=-1 from=0x max_addr=0x setup_arch+0x4ec/0x5c0
[0.00] memblock_reserve: [0x0871a540-0x0871a55f] memblock_alloc_range_nid+0xf0/0x184
[0.00] Primary instruction cache 16kB, VIPT, 4-way, linesize 16 bytes.
[0.00] Primary data cache 16kB, 4-way, VIPT, no aliases, linesize 16 bytes
[0.00] Zone ranges:
[0.00]   Normal   [mem 0x0800-0x09ff]
[0.00] Movable zone start for each node
[0.00] Early memory node ranges
[0.00]   node   0: [mem 0x0800-0x09ff]
[0.00] Initmem setup node 0 [mem 0x0800-0x09ff]
[0.00] memblock_alloc_try_nid: 262144 bytes align=0x10 nid=0 from=0x max_addr=0x alloc_node_mem_map.constprop.145+0x6c/0xd0
[0.00] memblock_reserve: [0x0871a560-0x0875a55f] memblock_alloc_range_nid+0xf0/0x184
[0.00] memblock_alloc_try_nid: 4 bytes align=0x10 nid=0 from=0x max_addr=0x000

[PATCH 5/8] MIPS: pci-legacy: stop using of_pci_range_to_resource

2021-04-13 Thread Ilya Lipnitskiy
Mirror commit aeba3731b150 ("powerpc/pci: Fix IO space breakage after
of_pci_range_to_resource() change").

Most MIPS platforms do not define PCI_IOBASE, nor implement
pci_address_to_pio(). Moreover, IO_SPACE_LIMIT is 0x for most MIPS
platforms. of_pci_range_to_resource passes the _start address_ of the IO
range into pci_address_to_pio, which then checks it against
IO_SPACE_LIMIT and fails, because for MIPS platforms that use
pci-legacy (pci-lantiq, pci-rt3883, pci-mt7620), IO ranges start much
higher than 0x.

In fact, pci-mt7621 in staging already works around this problem, see
commit 09dd629eeabb ("staging: mt7621-pci: fix io space and properly set
resource limits")

So just stop using of_pci_range_to_resource, which does not work for
MIPS.

Fixes PCI errors like:
  pci_bus :00: root bus resource [io  0x]

Fixes: 0b0b0893d49b ("of/pci: Fix the conversion of IO ranges into IO 
resources")
Signed-off-by: Ilya Lipnitskiy 
Cc: Liviu Dudau 
---
 arch/mips/pci/pci-legacy.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/mips/pci/pci-legacy.c b/arch/mips/pci/pci-legacy.c
index 39052de915f3..3a909194284a 100644
--- a/arch/mips/pci/pci-legacy.c
+++ b/arch/mips/pci/pci-legacy.c
@@ -166,8 +166,13 @@ void pci_load_of_ranges(struct pci_controller *hose, 
struct device_node *node)
res = hose->mem_resource;
break;
}
-   if (res != NULL)
-   of_pci_range_to_resource(, node, res);
+   if (res != NULL) {
+   res->name = node->full_name;
+   res->flags = range.flags;
+   res->start = range.cpu_addr;
+   res->end = range.cpu_addr + range.size - 1;
+   res->parent = res->child = res->sibling = NULL;
+   }
}
 }
 
-- 
2.31.1



[PATCH 8/8] MIPS: pci-legacy: use generic pci_enable_resources

2021-04-13 Thread Ilya Lipnitskiy
Follow the reasoning from commit 842de40d93e0 ("PCI: add generic
pci_enable_resources()"):

  The only functional difference from the MIPS version is that the
  generic one uses "!r->parent" to check for resource collisions
  instead of "!r->start && r->end".

That should have no effect on any pci-legacy driver.

Suggested-by: Bjorn Helgaas 
Signed-off-by: Ilya Lipnitskiy 
---
 arch/mips/pci/pci-legacy.c | 40 ++
 1 file changed, 2 insertions(+), 38 deletions(-)

diff --git a/arch/mips/pci/pci-legacy.c b/arch/mips/pci/pci-legacy.c
index 78c22987bef0..c24226ea0a6e 100644
--- a/arch/mips/pci/pci-legacy.c
+++ b/arch/mips/pci/pci-legacy.c
@@ -241,47 +241,11 @@ static int __init pcibios_init(void)
 
 subsys_initcall(pcibios_init);
 
-static int pcibios_enable_resources(struct pci_dev *dev, int mask)
-{
-   u16 cmd, old_cmd;
-   int idx;
-   struct resource *r;
-
-   pci_read_config_word(dev, PCI_COMMAND, );
-   old_cmd = cmd;
-   for (idx=0; idx < PCI_NUM_RESOURCES; idx++) {
-   /* Only set up the requested stuff */
-   if (!(mask & (1<resource[idx];
-   if (!(r->flags & (IORESOURCE_IO | IORESOURCE_MEM)))
-   continue;
-   if ((idx == PCI_ROM_RESOURCE) &&
-   (!(r->flags & IORESOURCE_ROM_ENABLE)))
-   continue;
-   if (!r->start && r->end) {
-   pci_err(dev,
-   "can't enable device: resource collisions\n");
-   return -EINVAL;
-   }
-   if (r->flags & IORESOURCE_IO)
-   cmd |= PCI_COMMAND_IO;
-   if (r->flags & IORESOURCE_MEM)
-   cmd |= PCI_COMMAND_MEMORY;
-   }
-   if (cmd != old_cmd) {
-   pci_info(dev, "enabling device (%04x -> %04x)\n", old_cmd, cmd);
-   pci_write_config_word(dev, PCI_COMMAND, cmd);
-   }
-   return 0;
-}
-
 int pcibios_enable_device(struct pci_dev *dev, int mask)
 {
-   int err;
+   int err = pci_enable_resources(dev, mask);
 
-   if ((err = pcibios_enable_resources(dev, mask)) < 0)
+   if (err < 0)
return err;
 
return pcibios_plat_dev_init(dev);
-- 
2.31.1



[PATCH 1/8] MIPS: pci-rt2880: fix slot 0 configuration

2021-04-13 Thread Ilya Lipnitskiy
pci_fixup_irqs() used to call pcibios_map_irq on every PCI device, which
for RT2880 included bus 0 slot 0. After pci_fixup_irqs() got removed,
only slots/funcs with devices attached would be called. While arguably
the right thing, that left no chance for this driver to ever initialize
slot 0, effectively bricking PCI and USB on RT2880 devices such as the
Belkin F5D8235-4 v1.

Slot 0 configuration needs to happen after PCI bus enumeration, but
before any device at slot 0x11 (func 0 or 1) is talked to. That was
determined empirically by testing on a Belkin F5D8235-4 v1 device. A
minimal BAR 0 config write followed by read, then setting slot 0
PCI_COMMAND to MASTER | IO | MEMORY is all that seems to be required for
proper functionality.

Tested by ensuring that full- and high-speed USB devices get enumerated
on the Belkin F5D8235-4 v1 (with an out of tree DTS file from OpenWrt).

Fixes: 04c81c7293df ("MIPS: PCI: Replace pci_fixup_irqs() call with host bridge 
IRQ mapping hooks")
Signed-off-by: Ilya Lipnitskiy 
Cc: Lorenzo Pieralisi 
Cc: Tobias Wolf 
Cc:  # v4.14+
---
 arch/mips/pci/pci-rt2880.c | 50 +-
 1 file changed, 33 insertions(+), 17 deletions(-)

diff --git a/arch/mips/pci/pci-rt2880.c b/arch/mips/pci/pci-rt2880.c
index e1f12e398136..19f7860fb28b 100644
--- a/arch/mips/pci/pci-rt2880.c
+++ b/arch/mips/pci/pci-rt2880.c
@@ -66,9 +66,13 @@ static int rt2880_pci_config_read(struct pci_bus *bus, 
unsigned int devfn,
unsigned long flags;
u32 address;
u32 data;
+   int busn = 0;
 
-   address = rt2880_pci_get_cfgaddr(bus->number, PCI_SLOT(devfn),
-PCI_FUNC(devfn), where);
+   if (bus)
+   busn = bus->number;
+
+   address = rt2880_pci_get_cfgaddr(busn, PCI_SLOT(devfn), PCI_FUNC(devfn),
+where);
 
spin_lock_irqsave(_pci_lock, flags);
rt2880_pci_reg_write(address, RT2880_PCI_REG_CONFIG_ADDR);
@@ -96,9 +100,13 @@ static int rt2880_pci_config_write(struct pci_bus *bus, 
unsigned int devfn,
unsigned long flags;
u32 address;
u32 data;
+   int busn = 0;
+
+   if (bus)
+   busn = bus->number;
 
-   address = rt2880_pci_get_cfgaddr(bus->number, PCI_SLOT(devfn),
-PCI_FUNC(devfn), where);
+   address = rt2880_pci_get_cfgaddr(busn, PCI_SLOT(devfn), PCI_FUNC(devfn),
+where);
 
spin_lock_irqsave(_pci_lock, flags);
rt2880_pci_reg_write(address, RT2880_PCI_REG_CONFIG_ADDR);
@@ -180,7 +188,6 @@ static inline void rt2880_pci_write_u32(unsigned long reg, 
u32 val)
 
 int pcibios_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
 {
-   u16 cmd;
int irq = -1;
 
if (dev->bus->number != 0)
@@ -188,8 +195,6 @@ int pcibios_map_irq(const struct pci_dev *dev, u8 slot, u8 
pin)
 
switch (PCI_SLOT(dev->devfn)) {
case 0x00:
-   rt2880_pci_write_u32(PCI_BASE_ADDRESS_0, 0x0800);
-   (void) rt2880_pci_read_u32(PCI_BASE_ADDRESS_0);
break;
case 0x11:
irq = RT288X_CPU_IRQ_PCI;
@@ -201,16 +206,6 @@ int pcibios_map_irq(const struct pci_dev *dev, u8 slot, u8 
pin)
break;
}
 
-   pci_write_config_byte((struct pci_dev *) dev,
-   PCI_CACHE_LINE_SIZE, 0x14);
-   pci_write_config_byte((struct pci_dev *) dev, PCI_LATENCY_TIMER, 0xFF);
-   pci_read_config_word((struct pci_dev *) dev, PCI_COMMAND, );
-   cmd |= PCI_COMMAND_MASTER | PCI_COMMAND_IO | PCI_COMMAND_MEMORY |
-   PCI_COMMAND_INVALIDATE | PCI_COMMAND_FAST_BACK |
-   PCI_COMMAND_SERR | PCI_COMMAND_WAIT | PCI_COMMAND_PARITY;
-   pci_write_config_word((struct pci_dev *) dev, PCI_COMMAND, cmd);
-   pci_write_config_byte((struct pci_dev *) dev, PCI_INTERRUPT_LINE,
- dev->irq);
return irq;
 }
 
@@ -251,6 +246,27 @@ static int rt288x_pci_probe(struct platform_device *pdev)
 
 int pcibios_plat_dev_init(struct pci_dev *dev)
 {
+   static bool slot0_init;
+
+   /*
+* Nobody seems to initialize slot 0, but this platform requires it, so
+* do it once when some other slot is being enabled. The PCI subsystem
+* should configure other slots properly, so no need to do anything
+* special for those.
+*/
+   if (!slot0_init) {
+   u32 cmd;
+
+   slot0_init = true;
+
+   rt2880_pci_write_u32(PCI_BASE_ADDRESS_0, 0x0800);
+   (void) rt2880_pci_read_u32(PCI_BASE_ADDRESS_0);
+
+   rt2880_pci_config_read(NULL, 0, PCI_COMMAND, 2, );
+   cmd |= PCI_COMMAND_MASTER | PCI_COMMAND_IO | PCI_COMMAND_MEMORY;
+   rt2880_pci_config_write(NULL, 0, PCI_COMMAND, 2, cmd);
+   }
+
return 0;
 }
 
-- 
2.31.1



[PATCH 2/8] MIPS: pci-rt2880: remove unneeded locks

2021-04-13 Thread Ilya Lipnitskiy
Mirror pci-rt3883 fix from commit e5067c718b3a ("MIPS: pci-rt3883:
Remove odd locking in PCI config space access code"). pci-rt2880 shares
the driver layout with pci-rt3883 and the same reasons apply.

Caller (generic PCI code) already does proper locking, so no need to add
another one here. Local PCI read/write functions are never called
simultaneously, also they do not require synchronization with the PCI
controller ops, since they are used before the controller registration.

Suggested-by: Sergey Ryazanov 
Signed-off-by: Ilya Lipnitskiy 
---
 arch/mips/pci/pci-rt2880.c | 13 -
 1 file changed, 13 deletions(-)

diff --git a/arch/mips/pci/pci-rt2880.c b/arch/mips/pci/pci-rt2880.c
index 19f7860fb28b..b4ee07cbcf2a 100644
--- a/arch/mips/pci/pci-rt2880.c
+++ b/arch/mips/pci/pci-rt2880.c
@@ -41,7 +41,6 @@
 #define RT2880_PCI_REG_ARBCTL  0x80
 
 static void __iomem *rt2880_pci_base;
-static DEFINE_SPINLOCK(rt2880_pci_lock);
 
 static u32 rt2880_pci_reg_read(u32 reg)
 {
@@ -63,7 +62,6 @@ static inline u32 rt2880_pci_get_cfgaddr(unsigned int bus, 
unsigned int slot,
 static int rt2880_pci_config_read(struct pci_bus *bus, unsigned int devfn,
  int where, int size, u32 *val)
 {
-   unsigned long flags;
u32 address;
u32 data;
int busn = 0;
@@ -74,10 +72,8 @@ static int rt2880_pci_config_read(struct pci_bus *bus, 
unsigned int devfn,
address = rt2880_pci_get_cfgaddr(busn, PCI_SLOT(devfn), PCI_FUNC(devfn),
 where);
 
-   spin_lock_irqsave(_pci_lock, flags);
rt2880_pci_reg_write(address, RT2880_PCI_REG_CONFIG_ADDR);
data = rt2880_pci_reg_read(RT2880_PCI_REG_CONFIG_DATA);
-   spin_unlock_irqrestore(_pci_lock, flags);
 
switch (size) {
case 1:
@@ -97,7 +93,6 @@ static int rt2880_pci_config_read(struct pci_bus *bus, 
unsigned int devfn,
 static int rt2880_pci_config_write(struct pci_bus *bus, unsigned int devfn,
   int where, int size, u32 val)
 {
-   unsigned long flags;
u32 address;
u32 data;
int busn = 0;
@@ -108,7 +103,6 @@ static int rt2880_pci_config_write(struct pci_bus *bus, 
unsigned int devfn,
address = rt2880_pci_get_cfgaddr(busn, PCI_SLOT(devfn), PCI_FUNC(devfn),
 where);
 
-   spin_lock_irqsave(_pci_lock, flags);
rt2880_pci_reg_write(address, RT2880_PCI_REG_CONFIG_ADDR);
data = rt2880_pci_reg_read(RT2880_PCI_REG_CONFIG_DATA);
 
@@ -127,7 +121,6 @@ static int rt2880_pci_config_write(struct pci_bus *bus, 
unsigned int devfn,
}
 
rt2880_pci_reg_write(data, RT2880_PCI_REG_CONFIG_DATA);
-   spin_unlock_irqrestore(_pci_lock, flags);
 
return PCIBIOS_SUCCESSFUL;
 }
@@ -159,31 +152,25 @@ static struct pci_controller rt2880_pci_controller = {
 
 static inline u32 rt2880_pci_read_u32(unsigned long reg)
 {
-   unsigned long flags;
u32 address;
u32 ret;
 
address = rt2880_pci_get_cfgaddr(0, 0, 0, reg);
 
-   spin_lock_irqsave(_pci_lock, flags);
rt2880_pci_reg_write(address, RT2880_PCI_REG_CONFIG_ADDR);
ret = rt2880_pci_reg_read(RT2880_PCI_REG_CONFIG_DATA);
-   spin_unlock_irqrestore(_pci_lock, flags);
 
return ret;
 }
 
 static inline void rt2880_pci_write_u32(unsigned long reg, u32 val)
 {
-   unsigned long flags;
u32 address;
 
address = rt2880_pci_get_cfgaddr(0, 0, 0, reg);
 
-   spin_lock_irqsave(_pci_lock, flags);
rt2880_pci_reg_write(address, RT2880_PCI_REG_CONFIG_ADDR);
rt2880_pci_reg_write(val, RT2880_PCI_REG_CONFIG_DATA);
-   spin_unlock_irqrestore(_pci_lock, flags);
 }
 
 int pcibios_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
-- 
2.31.1



[PATCH 6/8] MIPS: pci-legacy: remove redundant info messages

2021-04-13 Thread Ilya Lipnitskiy
Remove the following pci-legacy message:
  PCI host bridge /pci@44/host-bridge ranges:
   MEM 0x2000..0x2fff
IO 0x0046..0x0046

It is followed shortly by the same data from pci_register_host_bridge:
  PCI host bridge to bus :00
  pci_bus :00: root bus resource [mem 0x2000-0x2fff]
  pci_bus :00: root bus resource [io  0x46-0x46]

Signed-off-by: Ilya Lipnitskiy 
---
 arch/mips/pci/pci-legacy.c | 7 ---
 1 file changed, 7 deletions(-)

diff --git a/arch/mips/pci/pci-legacy.c b/arch/mips/pci/pci-legacy.c
index 3a909194284a..ec3f52ade72d 100644
--- a/arch/mips/pci/pci-legacy.c
+++ b/arch/mips/pci/pci-legacy.c
@@ -140,7 +140,6 @@ void pci_load_of_ranges(struct pci_controller *hose, struct 
device_node *node)
struct of_pci_range range;
struct of_pci_range_parser parser;
 
-   pr_info("PCI host bridge %pOF ranges:\n", node);
hose->of_node = node;
 
if (of_pci_range_parser_init(, node))
@@ -151,18 +150,12 @@ void pci_load_of_ranges(struct pci_controller *hose, 
struct device_node *node)
 
switch (range.flags & IORESOURCE_TYPE_BITS) {
case IORESOURCE_IO:
-   pr_info("  IO 0x%016llx..0x%016llx\n",
-   range.cpu_addr,
-   range.cpu_addr + range.size - 1);
hose->io_map_base =
(unsigned long)ioremap(range.cpu_addr,
   range.size);
res = hose->io_resource;
break;
case IORESOURCE_MEM:
-   pr_info(" MEM 0x%016llx..0x%016llx\n",
-   range.cpu_addr,
-   range.cpu_addr + range.size - 1);
res = hose->mem_resource;
break;
}
-- 
2.31.1



[PATCH 3/8] MIPS: pci-rt3883: trivial: remove unused variable

2021-04-13 Thread Ilya Lipnitskiy
Fixes the following compiler warning:
  warning: unused variable 'flags' [-Wunused-variable]

Fixes: e5067c718b3a ("MIPS: pci-rt3883: Remove odd locking in PCI config space 
access code")
Signed-off-by: Ilya Lipnitskiy 
Cc: Sergey Ryazanov 
Cc: triv...@kernel.org
---
 arch/mips/pci/pci-rt3883.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/arch/mips/pci/pci-rt3883.c b/arch/mips/pci/pci-rt3883.c
index 0ac6346026d0..e422f78db5bc 100644
--- a/arch/mips/pci/pci-rt3883.c
+++ b/arch/mips/pci/pci-rt3883.c
@@ -100,7 +100,6 @@ static u32 rt3883_pci_read_cfg32(struct 
rt3883_pci_controller *rpc,
   unsigned bus, unsigned slot,
   unsigned func, unsigned reg)
 {
-   unsigned long flags;
u32 address;
u32 ret;
 
@@ -116,7 +115,6 @@ static void rt3883_pci_write_cfg32(struct 
rt3883_pci_controller *rpc,
 unsigned bus, unsigned slot,
 unsigned func, unsigned reg, u32 val)
 {
-   unsigned long flags;
u32 address;
 
address = rt3883_pci_get_cfgaddr(bus, slot, func, reg);
@@ -229,7 +227,6 @@ static int rt3883_pci_config_read(struct pci_bus *bus, 
unsigned int devfn,
  int where, int size, u32 *val)
 {
struct rt3883_pci_controller *rpc;
-   unsigned long flags;
u32 address;
u32 data;
 
@@ -263,7 +260,6 @@ static int rt3883_pci_config_write(struct pci_bus *bus, 
unsigned int devfn,
   int where, int size, u32 val)
 {
struct rt3883_pci_controller *rpc;
-   unsigned long flags;
u32 address;
u32 data;
 
-- 
2.31.1



[PATCH 4/8] MIPS: pci-rt3883: more accurate DT error messages

2021-04-13 Thread Ilya Lipnitskiy
Existing strings do not make sense: one is always NULL and the other
refers to the wrong parent node.

Signed-off-by: Ilya Lipnitskiy 
---
 arch/mips/pci/pci-rt3883.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/arch/mips/pci/pci-rt3883.c b/arch/mips/pci/pci-rt3883.c
index e422f78db5bc..aebd4964ea34 100644
--- a/arch/mips/pci/pci-rt3883.c
+++ b/arch/mips/pci/pci-rt3883.c
@@ -431,8 +431,7 @@ static int rt3883_pci_probe(struct platform_device *pdev)
 
if (!rpc->intc_of_node) {
dev_err(dev, "%pOF has no %s child node",
-   rpc->intc_of_node,
-   "interrupt controller");
+   np, "interrupt controller");
return -EINVAL;
}
 
@@ -446,8 +445,7 @@ static int rt3883_pci_probe(struct platform_device *pdev)
 
if (!rpc->pci_controller.of_node) {
dev_err(dev, "%pOF has no %s child node",
-   rpc->intc_of_node,
-   "PCI host bridge");
+   np, "PCI host bridge");
err = -EINVAL;
goto err_put_intc_node;
}
-- 
2.31.1



[PATCH 7/8] MIPS: pci-legacy: remove busn_resource field

2021-04-13 Thread Ilya Lipnitskiy
No drivers set the busn_resource field in the pci_controller struct.
Commit 7ee214b540d9 ("MIPS: PCI: Remove unused busn_offset") almost
removed it over 3 years ago. Remove it for good to free up memory and
eliminate messages like:
  pci_bus :00: root bus resource [??? 0x flags 0x0]

Signed-off-by: Ilya Lipnitskiy 
Cc: Bjorn Helgaas 
---
 arch/mips/include/asm/pci.h | 1 -
 arch/mips/pci/pci-legacy.c  | 1 -
 2 files changed, 2 deletions(-)

diff --git a/arch/mips/include/asm/pci.h b/arch/mips/include/asm/pci.h
index 6f48649201c5..9ffc8192adae 100644
--- a/arch/mips/include/asm/pci.h
+++ b/arch/mips/include/asm/pci.h
@@ -38,7 +38,6 @@ struct pci_controller {
struct resource *io_resource;
unsigned long io_offset;
unsigned long io_map_base;
-   struct resource *busn_resource;
 
 #ifndef CONFIG_PCI_DOMAINS_GENERIC
unsigned int index;
diff --git a/arch/mips/pci/pci-legacy.c b/arch/mips/pci/pci-legacy.c
index ec3f52ade72d..78c22987bef0 100644
--- a/arch/mips/pci/pci-legacy.c
+++ b/arch/mips/pci/pci-legacy.c
@@ -89,7 +89,6 @@ static void pcibios_scanbus(struct pci_controller *hose)
hose->mem_resource, hose->mem_offset);
pci_add_resource_offset(,
hose->io_resource, hose->io_offset);
-   pci_add_resource(, hose->busn_resource);
list_splice_init(, >windows);
bridge->dev.parent = NULL;
bridge->sysdata = hose;
-- 
2.31.1



[PATCH 0/8] MIPS: Fixes for PCI legacy drivers (rt2880, rt3883)

2021-04-13 Thread Ilya Lipnitskiy
One major fix for rt2880-pci in the first patch - fixes breakage that
existed since v4.14.

Other more minor fixes, cleanups, and improvements that either free up
memory, make dmesg messages clearer, or remove redundant dmesg output.

Ilya Lipnitskiy (8):
  MIPS: pci-rt2880: fix slot 0 configuration
  MIPS: pci-rt2880: remove unneeded locks
  MIPS: pci-rt3883: trivial: remove unused variable
  MIPS: pci-rt3883: more accurate DT error messages
  MIPS: pci-legacy: stop using of_pci_range_to_resource
  MIPS: pci-legacy: remove redundant info messages
  MIPS: pci-legacy: remove busn_resource field
  MIPS: pci-legacy: use generic pci_enable_resources

 arch/mips/include/asm/pci.h |  1 -
 arch/mips/pci/pci-legacy.c  | 57 ++---
 arch/mips/pci/pci-rt2880.c  | 63 +++--
 arch/mips/pci/pci-rt3883.c  | 10 ++
 4 files changed, 44 insertions(+), 87 deletions(-)

-- 
2.31.1



[PATCH] staging: mt7621-pci: stop using of_pci_range_to_resource

2021-04-10 Thread Ilya Lipnitskiy
The logic here was already overriding the erroneous IO addresses
returned from of_pci_range_to_resource, which is the bulk of the logic.

So stop using it altogether and initialize the fields explicitly, as
done in aeba3731b150 ("powerpc/pci: Fix IO space breakage after
of_pci_range_to_resource() change").

Signed-off-by: Ilya Lipnitskiy 
Cc: Sergio Paracuellos 
---
 drivers/staging/mt7621-pci/pci-mt7621.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/mt7621-pci/pci-mt7621.c 
b/drivers/staging/mt7621-pci/pci-mt7621.c
index 1781c1dcf5b4..115250115f10 100644
--- a/drivers/staging/mt7621-pci/pci-mt7621.c
+++ b/drivers/staging/mt7621-pci/pci-mt7621.c
@@ -301,18 +301,19 @@ static int mt7621_pci_parse_request_of_pci_ranges(struct 
pci_host_bridge *host)
 
/*
 * IO_SPACE_LIMIT for MIPS is 0x but this platform uses IO at
-* upper address 0x001e16 so we have to get the resource from
-* the DT because when it has been requested it failed and has been
-* removed from bridge->dma_ranges and bridge->windows. So parse it
-* and remap it manually to make things work.
+* upper address 0x001e16. of_pci_range_to_resource does not work
+* well for MIPS platforms that don't define PCI_IOBASE, so set the IO
+* resource manually instead.
 */
+   pcie->io.name = node->full_name;
+   pcie->io.parent = pcie->io.child = pcie->io.sibling = NULL;
for_each_of_pci_range(, ) {
switch (range.flags & IORESOURCE_TYPE_BITS) {
case IORESOURCE_IO:
pcie->io_map_base =
(unsigned long)ioremap(range.cpu_addr,
   range.size);
-   of_pci_range_to_resource(, node, >io);
+   pcie->io.flags = range.flags;
pcie->io.start = range.cpu_addr;
pcie->io.end = range.cpu_addr + range.size - 1;
set_io_port_base(pcie->io_map_base);
-- 
2.31.1



Re: [PATCH v2] of: property: fw_devlink: do not link ".*,nr-gpios"

2021-04-07 Thread Ilya Lipnitskiy
On Tue, Apr 6, 2021 at 6:24 PM Saravana Kannan  wrote:
>
> On Tue, Apr 6, 2021 at 6:10 PM Rob Herring  wrote:
> >
> > On Tue, Apr 6, 2021 at 7:46 PM Saravana Kannan  wrote:
> > >
> > > On Tue, Apr 6, 2021 at 5:34 PM Rob Herring  wrote:
> > > >
> > > > On Tue, Apr 06, 2021 at 04:09:10PM -0700, Saravana Kannan wrote:
> > > > > On Mon, Apr 5, 2021 at 3:26 PM Ilya Lipnitskiy
> > > > >  wrote:
> > > > > >
> > > > > > [,]nr-gpios property is used by some GPIO drivers[0] to 
> > > > > > indicate
> > > > > > the number of GPIOs present on a system, not define a GPIO. 
> > > > > > nr-gpios is
> > > > > > not configured by #gpio-cells and can't be parsed along with other
> > > > > > "*-gpios" properties.
> > > > > >
> > > > > > nr-gpios without the "," prefix is not allowed by the DT
> > > > > > spec[1], so only add exception for the ",nr-gpios" suffix and let 
> > > > > > the
> > > > > > error message continue being printed for non-compliant 
> > > > > > implementations.
> > > > > >
> > > > > > [0]: nr-gpios is referenced in 
> > > > > > Documentation/devicetree/bindings/gpio:
> > > > > >  - gpio-adnp.txt
> > > > > >  - gpio-xgene-sb.txt
> > > > > >  - gpio-xlp.txt
> > > > > >  - snps,dw-apb-gpio.yaml
> > > > > >
> > > > > > [1]:
> > > > > > Link: 
> > > > > > https://github.com/devicetree-org/dt-schema/blob/cb53a16a1eb3e2169ce170c071e47940845ec26e/schemas/gpio/gpio-consumer.yaml#L20
> > > > > >
> > > > > > Fixes errors such as:
> > > > > >   OF: /palmbus@30/gpio@600: could not find phandle
> > > > > >
> > > > > > Fixes: 7f00be96f125 ("of: property: Add device link support for 
> > > > > > interrupt-parent, dmas and -gpio(s)")
> > > > > > Signed-off-by: Ilya Lipnitskiy 
> > > > > > Cc: Saravana Kannan 
> > > > > > Cc:  # 5.5.x
> > > > > > ---
> > > > > >  drivers/of/property.c | 11 ++-
> > > > > >  1 file changed, 10 insertions(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/drivers/of/property.c b/drivers/of/property.c
> > > > > > index 2046ae311322..1793303e84ac 100644
> > > > > > --- a/drivers/of/property.c
> > > > > > +++ b/drivers/of/property.c
> > > > > > @@ -1281,7 +1281,16 @@ DEFINE_SIMPLE_PROP(pinctrl7, "pinctrl-7", 
> > > > > > NULL)
> > > > > >  DEFINE_SIMPLE_PROP(pinctrl8, "pinctrl-8", NULL)
> > > > > >  DEFINE_SUFFIX_PROP(regulators, "-supply", NULL)
> > > > > >  DEFINE_SUFFIX_PROP(gpio, "-gpio", "#gpio-cells")
> > > > > > -DEFINE_SUFFIX_PROP(gpios, "-gpios", "#gpio-cells")
> > > > > > +
> > > > > > +static struct device_node *parse_gpios(struct device_node *np,
> > > > > > +  const char *prop_name, int 
> > > > > > index)
> > > > > > +{
> > > > > > +   if (!strcmp_suffix(prop_name, ",nr-gpios"))
> > > > > > +   return NULL;
> > > > >
> > > > > Ah I somehow missed this patch. This gives a blanked exception for
> > > > > vendor,nr-gpios. I'd prefer explicit exceptions for all the instances
> > > > > of ",nr-gpios" we are grandfathering in. Any future additions should
> > > > > be rejected. Can we do that please?
> > > > >
> > > > > Rob, you okay with making this list more explicit?
> > > >
> > > > Not the kernel's job IMO. A schema is the right way to handle that.
> > >
> > > Ok, that's fine by me. Btw, let's land this in driver-core? I've made
> > > changes there and this might cause conflicts. Not sure.
> >
> > It merges with linux-next fine. You'll need to resend this to Greg if
> > you want to do that.
> >
> > Reviewed-by: Rob Herring 
>
> Hi Greg,
>
> Can you pull this into driver-core please?
Do you want me to re-spin on top of driver-core? The patch is
currently based on dt/next in robh/linux.git

Ilya


[PATCH v3] MIPS: add support for buggy MT7621S core detection

2021-04-07 Thread Ilya Lipnitskiy
Most MT7621 SoCs have 2 cores, which is detected and supported properly
by CPS.

Unfortunately, MT7621 SoC has a less common S variant with only one core.
On MT7621S, GCR_CONFIG still reports 2 cores, which leads to hangs when
starting SMP. CPULAUNCH registers can be used in that case to detect the
absence of the second core and override the GCR_CONFIG PCORES field.

Rework a long-standing OpenWrt patch to override the value of
mips_cps_numcores on single-core MT7621 systems.

Tested on a dual-core MT7621 device (Ubiquiti ER-X) and a single-core
MT7621 device (Netgear R6220).

Original 4.14 OpenWrt patch:
Link: 
https://git.openwrt.org/?p=openwrt/openwrt.git;a=commitdiff;h=4cdbc90a376dd0555201c1434a2081e055e9ceb7
Current 5.10 OpenWrt patch:
Link: 
https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/ramips/patches-5.10/320-mt7621-core-detect-hack.patch;h=c63f0f4c1ec742e24d8480e80553863744b58f6a;hb=10267e17299806f9885d086147878f6c492cb904

Suggested-by: Felix Fietkau 
Signed-off-by: Ilya Lipnitskiy 
---
 arch/mips/include/asm/mips-cps.h | 23 ++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/arch/mips/include/asm/mips-cps.h b/arch/mips/include/asm/mips-cps.h
index fd43d876892e..35fb8ee6dd33 100644
--- a/arch/mips/include/asm/mips-cps.h
+++ b/arch/mips/include/asm/mips-cps.h
@@ -10,6 +10,8 @@
 #include 
 #include 
 
+#include 
+
 extern unsigned long __cps_access_bad_size(void)
__compiletime_error("Bad size for CPS accessor");
 
@@ -165,11 +167,30 @@ static inline uint64_t mips_cps_cluster_config(unsigned 
int cluster)
  */
 static inline unsigned int mips_cps_numcores(unsigned int cluster)
 {
+   unsigned int ncores;
+
if (!mips_cm_present())
return 0;
 
/* Add one before masking to handle 0xff indicating no cores */
-   return (mips_cps_cluster_config(cluster) + 1) & CM_GCR_CONFIG_PCORES;
+   ncores = (mips_cps_cluster_config(cluster) + 1) & CM_GCR_CONFIG_PCORES;
+
+   if (IS_ENABLED(CONFIG_SOC_MT7621)) {
+   struct cpulaunch *launch;
+
+   /*
+* Ralink MT7621S SoC is single core, but the GCR_CONFIG method
+* always reports 2 cores. Check the second core's LAUNCH_FREADY
+* flag to detect if the second core is missing. This method
+* only works before the core has been started.
+*/
+   launch = (struct cpulaunch *)CKSEG0ADDR(CPULAUNCH);
+   launch += 2; /* MT7621 has 2 VPEs per core */
+   if (!(launch->flags & LAUNCH_FREADY))
+   ncores = 1;
+   }
+
+   return ncores;
 }
 
 /**
-- 
2.31.1



Re: [PATCH] MIPS: add support for buggy MT7621S core detection

2021-04-07 Thread Ilya Lipnitskiy
On Wed, Apr 7, 2021 at 6:49 AM Maciej W. Rozycki  wrote:
>
> On Mon, 5 Apr 2021, Ilya Lipnitskiy wrote:
>
> > Thanks for the comments. Including asm/bugs.h in asm/mips-cps.h led to
> > some circular dependencies when I tried it, but I will try again based
> > on your feedback - indeed it would be much cleaner to have this logic
> > in mips_cps_numcores. The only wrinkle is that mips_cps_numcores may
> > return a different value on MT7621 after the cores have started due to
> > CPULAUNCH flags changing, but nobody calls mips_cps_numcores later
> > anyway, so it's a moot point today. I will clean up the change and
> > resend.
>
>  Hmm, I don't know this system, but by the look of the code it queries
> launch[2], which I gather refers to the VPE #0 of an inexistent core #1,
> so why would the structure change given that there is no corresponding
> silicon?
The structure would change only on the dual-core flavor of MT7621, the
single-core would always report 1 core, but on the dual-core, if
somebody were to call mips_cps_numcores after the second core had
already started, mips_cps_numcores would return 1 instead of 2. I
think it may be feasible to fix it by checking other flags, but there
is no use case for that today, so I'd rather keep this hacky logic to
a minimum.

Ilya


Re: [PATCH] of: property: do not create device links from *nr-gpios

2021-04-06 Thread Ilya Lipnitskiy
On Tue, Apr 6, 2021 at 10:40 AM Rob Herring  wrote:
>
> On Mon, Apr 05, 2021 at 01:18:56PM -0700, Saravana Kannan wrote:
> > On Mon, Apr 5, 2021 at 1:10 PM Ilya Lipnitskiy
> >  wrote:
> > >
> > > Hi Saravana,
> > >
> > > On Mon, Apr 5, 2021 at 1:01 PM Saravana Kannan  
> > > wrote:
> > > >
> > > > On Sun, Apr 4, 2021 at 8:14 PM Ilya Lipnitskiy
> > > >  wrote:
> > > > >
> > > > > [,]nr-gpios property is used by some GPIO drivers[0] to 
> > > > > indicate
> > > > > the number of GPIOs present on a system, not define a GPIO. nr-gpios 
> > > > > is
> > > > > not configured by #gpio-cells and can't be parsed along with other
> > > > > "*-gpios" properties.
> > > > >
> > > > > scripts/dtc/checks.c also has a special case for nr-gpio{s}. However,
> > > > > nr-gpio is not really special, so we only need to fix nr-gpios suffix
> > > > > here.
> > > >
> > > > The only example of this that I see is "snps,nr-gpios".
> > > arch/arm64/boot/dts/apm/apm-shadowcat.dtsi uses "apm,nr-gpios", with
> > > parsing code in drivers/gpio/gpio-xgene-sb.c. There is also code in
> > > drivers/gpio/gpio-adnp.c and drivers/gpio/gpio-mockup.c using
> > > "nr-gpios" without any vendor prefix.
> >
> > Ah ok. I just grepped the DT files. I'm not sure what Rob's position
> > is on supporting DT files not in upstream. Thanks for the
> > clarification.
>
> If it's something we had documented, then we have to support it
Do I read this correctly as a sort-of Ack of my proposed [PATCH v2] in
this thread, since it aligns the code with the published DT schema?

Ilya


[PATCH v2] MIPS: add support for buggy MT7621S core detection

2021-04-05 Thread Ilya Lipnitskiy
Most MT7621 SoCs have 2 cores, which is detected and supported properly
by CPS.

Unfortunately, MT7621 SoC has a less common S variant with only one core.
On MT7621S, GCR_CONFIG still reports 2 cores, which leads to hangs when
starting SMP. CPULAUNCH registers can be used in that case to detect the
absence of the second core and override the GCR_CONFIG PCORES field.

Rework a long-standing OpenWrt patch to override the value of
mips_cps_numcores on single-core MT7621 systems.

Tested on a dual-core MT7621 device (Ubiquiti ER-X) and a single-core
MT7621 device (Netgear R6220).

Original 4.14 OpenWrt patch:
Link: 
https://git.openwrt.org/?p=openwrt/openwrt.git;a=commitdiff;h=4cdbc90a376dd0555201c1434a2081e055e9ceb7
Current 5.10 OpenWrt patch:
Link: 
https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/ramips/patches-5.10/320-mt7621-core-detect-hack.patch;h=c63f0f4c1ec742e24d8480e80553863744b58f6a;hb=10267e17299806f9885d086147878f6c492cb904

Suggested-by: Felix Fietkau 
Signed-off-by: Ilya Lipnitskiy 
---
 arch/mips/include/asm/mips-cps.h | 22 +-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/arch/mips/include/asm/mips-cps.h b/arch/mips/include/asm/mips-cps.h
index fd43d876892e..9f495ffef2b7 100644
--- a/arch/mips/include/asm/mips-cps.h
+++ b/arch/mips/include/asm/mips-cps.h
@@ -10,6 +10,8 @@
 #include 
 #include 
 
+#include 
+
 extern unsigned long __cps_access_bad_size(void)
__compiletime_error("Bad size for CPS accessor");
 
@@ -165,11 +167,29 @@ static inline uint64_t mips_cps_cluster_config(unsigned 
int cluster)
  */
 static inline unsigned int mips_cps_numcores(unsigned int cluster)
 {
+   struct cpulaunch *launch;
+   unsigned int ncores;
+
if (!mips_cm_present())
return 0;
 
/* Add one before masking to handle 0xff indicating no cores */
-   return (mips_cps_cluster_config(cluster) + 1) & CM_GCR_CONFIG_PCORES;
+   ncores = (mips_cps_cluster_config(cluster) + 1) & CM_GCR_CONFIG_PCORES;
+
+   if (IS_ENABLED(CONFIG_SOC_MT7621)) {
+   /*
+* Ralink MT7621S SoC is single core, but the GCR_CONFIG method
+* always reports 2 cores. Check the second core's LAUNCH_FREADY
+* flag to detect if the second core is missing. This method
+* only works before the core has been started.
+*/
+   launch = (struct cpulaunch *)CKSEG0ADDR(CPULAUNCH);
+   launch += 2; /* MT7621 has 2 VPEs per core */
+   if (!(launch->flags & LAUNCH_FREADY))
+   ncores = 1;
+   }
+
+   return ncores;
 }
 
 /**
-- 
2.31.1



Re: [PATCH] MIPS: add support for buggy MT7621S core detection

2021-04-05 Thread Ilya Lipnitskiy
On Mon, Apr 5, 2021 at 6:22 PM Maciej W. Rozycki  wrote:
>
> On Fri, 2 Apr 2021, Ilya Lipnitskiy wrote:
>
> > diff --git a/arch/mips/include/asm/bugs.h b/arch/mips/include/asm/bugs.h
> > index d72dc6e1cf3c..d32f0c4e61f7 100644
> > --- a/arch/mips/include/asm/bugs.h
> > +++ b/arch/mips/include/asm/bugs.h
> > @@ -50,4 +51,21 @@ static inline int r4k_daddiu_bug(void)
> >   return daddiu_bug != 0;
> >  }
> >
> > +static inline void cm_gcr_pcores_bug(unsigned int *ncores)
> > +{
> > + struct cpulaunch *launch;
> > +
> > + if (!IS_ENABLED(CONFIG_SOC_MT7621) || !ncores)
> > + return;
> > +
> > + /*
> > +  * Ralink MT7621S SoC is single core, but GCR_CONFIG always reports 2 
> > cores.
>
>  Overlong line.
>
> > diff --git a/arch/mips/kernel/smp-cps.c b/arch/mips/kernel/smp-cps.c
> > index bcd6a944b839..e1e9c11e8a7c 100644
> > --- a/arch/mips/kernel/smp-cps.c
> > +++ b/arch/mips/kernel/smp-cps.c
> > @@ -60,6 +61,7 @@ static void __init cps_smp_setup(void)
> >   pr_cont("{");
> >
> >   ncores = mips_cps_numcores(cl);
> > + cm_gcr_pcores_bug();
> >   for (c = 0; c < ncores; c++) {
> >   core_vpes = core_vpe_count(cl, c);
> >
> > @@ -170,6 +172,7 @@ static void __init cps_prepare_cpus(unsigned int 
> > max_cpus)
> >
> >   /* Allocate core boot configuration structs */
> >   ncores = mips_cps_numcores(0);
> > + cm_gcr_pcores_bug();
>
>  Why called at each `mips_cps_numcores' call site rather than within the
> callee?  Also weird inefficient interface: why isn't `ncores' passed by
> value for a new value to be returned?
Thanks for the comments. Including asm/bugs.h in asm/mips-cps.h led to
some circular dependencies when I tried it, but I will try again based
on your feedback - indeed it would be much cleaner to have this logic
in mips_cps_numcores. The only wrinkle is that mips_cps_numcores may
return a different value on MT7621 after the cores have started due to
CPULAUNCH flags changing, but nobody calls mips_cps_numcores later
anyway, so it's a moot point today. I will clean up the change and
resend.

Ilya


[PATCH v2] of: property: fw_devlink: do not link ".*,nr-gpios"

2021-04-05 Thread Ilya Lipnitskiy
[,]nr-gpios property is used by some GPIO drivers[0] to indicate
the number of GPIOs present on a system, not define a GPIO. nr-gpios is
not configured by #gpio-cells and can't be parsed along with other
"*-gpios" properties.

nr-gpios without the "," prefix is not allowed by the DT
spec[1], so only add exception for the ",nr-gpios" suffix and let the
error message continue being printed for non-compliant implementations.

[0]: nr-gpios is referenced in Documentation/devicetree/bindings/gpio:
 - gpio-adnp.txt
 - gpio-xgene-sb.txt
 - gpio-xlp.txt
 - snps,dw-apb-gpio.yaml

[1]:
Link: 
https://github.com/devicetree-org/dt-schema/blob/cb53a16a1eb3e2169ce170c071e47940845ec26e/schemas/gpio/gpio-consumer.yaml#L20

Fixes errors such as:
  OF: /palmbus@30/gpio@600: could not find phandle

Fixes: 7f00be96f125 ("of: property: Add device link support for 
interrupt-parent, dmas and -gpio(s)")
Signed-off-by: Ilya Lipnitskiy 
Cc: Saravana Kannan 
Cc:  # 5.5.x
---
 drivers/of/property.c | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/of/property.c b/drivers/of/property.c
index 2046ae311322..1793303e84ac 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -1281,7 +1281,16 @@ DEFINE_SIMPLE_PROP(pinctrl7, "pinctrl-7", NULL)
 DEFINE_SIMPLE_PROP(pinctrl8, "pinctrl-8", NULL)
 DEFINE_SUFFIX_PROP(regulators, "-supply", NULL)
 DEFINE_SUFFIX_PROP(gpio, "-gpio", "#gpio-cells")
-DEFINE_SUFFIX_PROP(gpios, "-gpios", "#gpio-cells")
+
+static struct device_node *parse_gpios(struct device_node *np,
+  const char *prop_name, int index)
+{
+   if (!strcmp_suffix(prop_name, ",nr-gpios"))
+   return NULL;
+
+   return parse_suffix_prop_cells(np, prop_name, index, "-gpios",
+  "#gpio-cells");
+}
 
 static struct device_node *parse_iommu_maps(struct device_node *np,
const char *prop_name, int index)
-- 
2.31.1



Re: [PATCH] of: property: do not create device links from *nr-gpios

2021-04-05 Thread Ilya Lipnitskiy
On Mon, Apr 5, 2021 at 1:19 PM Saravana Kannan  wrote:
>
> On Mon, Apr 5, 2021 at 1:10 PM Ilya Lipnitskiy
>  wrote:
> >
> > Hi Saravana,
> >
> > On Mon, Apr 5, 2021 at 1:01 PM Saravana Kannan  wrote:
> > >
> > > On Sun, Apr 4, 2021 at 8:14 PM Ilya Lipnitskiy
> > >  wrote:
> > > >
> > > > [,]nr-gpios property is used by some GPIO drivers[0] to indicate
> > > > the number of GPIOs present on a system, not define a GPIO. nr-gpios is
> > > > not configured by #gpio-cells and can't be parsed along with other
> > > > "*-gpios" properties.
> > > >
> > > > scripts/dtc/checks.c also has a special case for nr-gpio{s}. However,
> > > > nr-gpio is not really special, so we only need to fix nr-gpios suffix
> > > > here.
> > >
> > > The only example of this that I see is "snps,nr-gpios".
> > arch/arm64/boot/dts/apm/apm-shadowcat.dtsi uses "apm,nr-gpios", with
> > parsing code in drivers/gpio/gpio-xgene-sb.c. There is also code in
> > drivers/gpio/gpio-adnp.c and drivers/gpio/gpio-mockup.c using
> > "nr-gpios" without any vendor prefix.
>
> Ah ok. I just grepped the DT files. I'm not sure what Rob's position
> is on supporting DT files not in upstream. Thanks for the
> clarification.
For the offending drivers and docs that don't have any dts/dtsi files
in-tree, can we just "sed -i 's/nr-gpios/ngpios'" and call it good?

> Looks like even the DT spec has an exception only for vendor,nr and not just 
> nr.
> https://github.com/devicetree-org/dt-schema/blob/master/schemas/gpio/gpio-consumer.yaml#L20
Thanks for linking the spec. I can re-spin the patch with ",nr-gpios"
as the special suffix to align with the spec.

Ilya


Re: [PATCH] of: property: do not create device links from *nr-gpios

2021-04-05 Thread Ilya Lipnitskiy
Hi Saravana,

On Mon, Apr 5, 2021 at 1:01 PM Saravana Kannan  wrote:
>
> On Sun, Apr 4, 2021 at 8:14 PM Ilya Lipnitskiy
>  wrote:
> >
> > [,]nr-gpios property is used by some GPIO drivers[0] to indicate
> > the number of GPIOs present on a system, not define a GPIO. nr-gpios is
> > not configured by #gpio-cells and can't be parsed along with other
> > "*-gpios" properties.
> >
> > scripts/dtc/checks.c also has a special case for nr-gpio{s}. However,
> > nr-gpio is not really special, so we only need to fix nr-gpios suffix
> > here.
>
> The only example of this that I see is "snps,nr-gpios".
arch/arm64/boot/dts/apm/apm-shadowcat.dtsi uses "apm,nr-gpios", with
parsing code in drivers/gpio/gpio-xgene-sb.c. There is also code in
drivers/gpio/gpio-adnp.c and drivers/gpio/gpio-mockup.c using
"nr-gpios" without any vendor prefix.

I personally don't think causing regressions is good for any reason,
so I think we need to fix this in stable releases. The patch can be
reverted when nr-gpios is no longer special. The logic here should
also be aligned with scripts/dtc/checks.c, I actually submitted a
patch to warn about "nr-gpios" only and not "nr-gpio" in dtc as well:
https://www.spinics.net/lists/devicetree-compiler/msg03619.html

Ilya


[PATCH] of: property: do not create device links from *nr-gpios

2021-04-04 Thread Ilya Lipnitskiy
[,]nr-gpios property is used by some GPIO drivers[0] to indicate
the number of GPIOs present on a system, not define a GPIO. nr-gpios is
not configured by #gpio-cells and can't be parsed along with other
"*-gpios" properties.

scripts/dtc/checks.c also has a special case for nr-gpio{s}. However,
nr-gpio is not really special, so we only need to fix nr-gpios suffix
here.

[0]: nr-gpios is referenced in Documentation/devicetree/bindings/gpio:
 - gpio-adnp.txt
 - gpio-xgene-sb.txt
 - gpio-xlp.txt
 - snps,dw-apb-gpio.yaml

Fixes errors such as:
  OF: /palmbus@30/gpio@600: could not find phandle

Call Trace:
  of_phandle_iterator_next+0x8c/0x16c
  __of_parse_phandle_with_args+0x38/0xb8
  of_parse_phandle_with_args+0x28/0x3c
  parse_suffix_prop_cells+0x80/0xac
  parse_gpios+0x20/0x2c
  of_link_to_suppliers+0x18c/0x288
  of_link_to_suppliers+0x1fc/0x288
  device_add+0x4e0/0x734
  of_platform_device_create_pdata+0xb8/0xfc
  of_platform_bus_create+0x170/0x214
  of_platform_populate+0x88/0xf4
  __dt_register_buses+0xbc/0xf0
  plat_of_setup+0x1c/0x34

Fixes: 7f00be96f125 ("of: property: Add device link support for 
interrupt-parent, dmas and -gpio(s)")
Signed-off-by: Ilya Lipnitskiy 
Cc: Saravana Kannan 
Cc:  # 5.5.x
---
 drivers/of/property.c | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/of/property.c b/drivers/of/property.c
index 2bb3158c9e43..24672c295603 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -1271,7 +1271,16 @@ DEFINE_SIMPLE_PROP(pinctrl8, "pinctrl-8", NULL)
 DEFINE_SIMPLE_PROP(remote_endpoint, "remote-endpoint", NULL)
 DEFINE_SUFFIX_PROP(regulators, "-supply", NULL)
 DEFINE_SUFFIX_PROP(gpio, "-gpio", "#gpio-cells")
-DEFINE_SUFFIX_PROP(gpios, "-gpios", "#gpio-cells")
+
+static struct device_node *parse_gpios(struct device_node *np,
+  const char *prop_name, int index)
+{
+   if (!strcmp_suffix(prop_name, "nr-gpios"))
+   return NULL;
+
+   return parse_suffix_prop_cells(np, prop_name, index, "-gpios",
+  "#gpio-cells");
+}
 
 static struct device_node *parse_iommu_maps(struct device_node *np,
const char *prop_name, int index)
-- 
2.31.1



[PATCH] MIPS: ralink: rt288x: select MIPS_AUTO_PFN_OFFSET

2021-04-03 Thread Ilya Lipnitskiy
RT288X systems may have a non-zero ramstart causing problems with memory
reservations and boot hangs, as well as messages like:
  Wasting 1048576 bytes for tracking 32768 unused pages

Both are alleviated by selecting MIPS_AUTO_PFN_OFFSET for such
platforms.

Tested on a Belkin F5D8235 v1 RT2880 device.

Link: 
https://lore.kernel.org/linux-mips/20180820233111.xww5232dxbuouf4n@pburton-laptop/

Signed-off-by: Ilya Lipnitskiy 
Cc: Mike Rapoport 
---
 arch/mips/ralink/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/mips/ralink/Kconfig b/arch/mips/ralink/Kconfig
index c20c44788b62..7c6de7f8484e 100644
--- a/arch/mips/ralink/Kconfig
+++ b/arch/mips/ralink/Kconfig
@@ -31,6 +31,7 @@ choice
 
config SOC_RT288X
bool "RT288x"
+   select MIPS_AUTO_PFN_OFFSET
select MIPS_L1_CACHE_SHIFT_4
select HAVE_LEGACY_CLK
select HAVE_PCI
-- 
2.31.1



Re: [PATCH] MIPS: fix memory reservation for non-usermem setups

2021-04-03 Thread Ilya Lipnitskiy
Hi Mike,

On Tue, Mar 16, 2021 at 11:33 PM Mike Rapoport  wrote:
>
> Hi Ilya,
>
> On Tue, Mar 16, 2021 at 10:10:09PM -0700, Ilya Lipnitskiy wrote:
> > Hi Thomas,
> >
> > On Fri, Mar 12, 2021 at 7:19 AM Thomas Bogendoerfer
> >  wrote:
> > >
> > > On Sun, Mar 07, 2021 at 11:40:30AM -0800, Ilya Lipnitskiy wrote:
> > > > From: Tobias Wolf 
> > > >
> > > > Commit 67a3ba25aa95 ("MIPS: Fix incorrect mem=X@Y handling") introduced 
> > > > a new
> > > > issue for rt288x where "PHYS_OFFSET" is 0x0 but the calculated 
> > > > "ramstart" is
> > > > not. As the prerequisite of custom memory map has been removed, this 
> > > > results
> > > > in the full memory range of 0x0 - 0x800 to be marked as reserved 
> > > > for this
> > > > platform.
> > >
> > > and where is the problem here ?
> > Turns out this was already attempted to be upstreamed - not clear why
> > it wasn't merged. Context:
> > https://lore.kernel.org/linux-mips/6504517.U6H5IhoIOn@loki/
> >
> > I hope the thread above helps you understand the problem.
>
> The memory initialization was a bit different then. Do you still see the
> same problem?
Thanks for asking. I obtained a RT2880 device and gave it a try. It
hangs at boot without this patch, however selecting
MIPS_AUTO_PFN_OFFSET fixes it. Also, no more messages like "Wasting
1048576 bytes for tracking 32768 unused pages". MIPS_AUTO_PFN_OFFSET
was suggested by Paul Burton here:
https://lore.kernel.org/linux-mips/20180820233111.xww5232dxbuouf4n@pburton-laptop/

I will supersede this patch with one that simply selects
MIPS_AUTO_PFN_OFFSET for this SOC.

Ilya


[PATCH] MIPS: add support for buggy MT7621S core detection

2021-04-03 Thread Ilya Lipnitskiy
Most MT7621 SoCs have 2 cores, which is detected and supported properly
by CPS.

Unfortunately, MT7621 SoC has a less common S variant with only one core.
On MT7621S, GCR_CONFIG still reports 2 cores, which leads to hangs when
starting SMP. CPULAUNCH registers can be used in that case to detect the
absence of the second core and override the GCR_CONFIG PCORES field.

Rework a long-standing OpenWrt patch to override the value of
mips_cps_numcores on single-core MT7621 systems.

Tested on a dual-core MT7621 device (Ubiquiti ER-X) and a single-core
MT7621 device (Netgear R6220).

Original 4.14 OpenWrt patch:
Link: 
https://git.openwrt.org/?p=openwrt/openwrt.git;a=commitdiff;h=4cdbc90a376dd0555201c1434a2081e055e9ceb7
Current 5.10 OpenWrt patch:
Link: 
https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/ramips/patches-5.10/320-mt7621-core-detect-hack.patch;h=c63f0f4c1ec742e24d8480e80553863744b58f6a;hb=10267e17299806f9885d086147878f6c492cb904

Suggested-by: Felix Fietkau 
Signed-off-by: Ilya Lipnitskiy 
---
 arch/mips/include/asm/bugs.h | 18 ++
 arch/mips/kernel/smp-cps.c   |  3 +++
 2 files changed, 21 insertions(+)

diff --git a/arch/mips/include/asm/bugs.h b/arch/mips/include/asm/bugs.h
index d72dc6e1cf3c..d32f0c4e61f7 100644
--- a/arch/mips/include/asm/bugs.h
+++ b/arch/mips/include/asm/bugs.h
@@ -16,6 +16,7 @@
 
 #include 
 #include 
+#include 
 
 extern int daddiu_bug;
 
@@ -50,4 +51,21 @@ static inline int r4k_daddiu_bug(void)
return daddiu_bug != 0;
 }
 
+static inline void cm_gcr_pcores_bug(unsigned int *ncores)
+{
+   struct cpulaunch *launch;
+
+   if (!IS_ENABLED(CONFIG_SOC_MT7621) || !ncores)
+   return;
+
+   /*
+* Ralink MT7621S SoC is single core, but GCR_CONFIG always reports 2 
cores.
+* Use legacy amon method to detect if the second core is missing.
+*/
+   launch = (struct cpulaunch *)CKSEG0ADDR(CPULAUNCH);
+   launch += 2; /* MT7621 has 2 VPEs per core */
+   if (!(launch->flags & LAUNCH_FREADY))
+   *ncores = 1;
+}
+
 #endif /* _ASM_BUGS_H */
diff --git a/arch/mips/kernel/smp-cps.c b/arch/mips/kernel/smp-cps.c
index bcd6a944b839..e1e9c11e8a7c 100644
--- a/arch/mips/kernel/smp-cps.c
+++ b/arch/mips/kernel/smp-cps.c
@@ -15,6 +15,7 @@
 #include 
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -60,6 +61,7 @@ static void __init cps_smp_setup(void)
pr_cont("{");
 
ncores = mips_cps_numcores(cl);
+   cm_gcr_pcores_bug();
for (c = 0; c < ncores; c++) {
core_vpes = core_vpe_count(cl, c);
 
@@ -170,6 +172,7 @@ static void __init cps_prepare_cpus(unsigned int max_cpus)
 
/* Allocate core boot configuration structs */
ncores = mips_cps_numcores(0);
+   cm_gcr_pcores_bug();
mips_cps_core_bootcfg = kcalloc(ncores, sizeof(*mips_cps_core_bootcfg),
GFP_KERNEL);
if (!mips_cps_core_bootcfg) {
-- 
2.31.1



Re: exec error: BUG: Bad rss-counter

2021-03-30 Thread Ilya Lipnitskiy
On Tue, Mar 30, 2021 at 9:11 AM Linus Torvalds
 wrote:
>
> On Mon, Mar 29, 2021 at 9:56 PM Zhou Yanjie  wrote:
> >
> > On 2021/3/29 上午10:48, Ilya Lipnitskiy wrote:
> > >
> > > Try:
> > > diff --git a/mm/memory.c b/mm/memory.c
> > > index c8e357627318..1fd753245369 100644
> > > --- a/mm/memory.c
> > > +++ b/mm/memory.c
> > > @@ -166,7 +166,7 @@ static int __init init_zero_pfn(void)
> > >  zero_pfn = page_to_pfn(ZERO_PAGE(0));
> > >  return 0;
> > >   }
> > > -core_initcall(init_zero_pfn);
> > > +early_initcall(init_zero_pfn);
> >
> > It works, thanks!
>
> Looks good to me - init_zero_pfn() can be called early, because it
> depends on paging_init() will should have happened long before any
> initcalls in setup_arch().
>
> Ilya, mind sending a signed-off version with a nice commit message,
> and I'll apply it.
Sorry, I could have done better linking it to this thread - I actually
did submit it recently - please see
https://lkml.kernel.org/r/20210330044208.8305-1-ilya.lipnits...@gmail.com

Ilya


[PATCH v3] mm: fix race by making init_zero_pfn() early_initcall

2021-03-29 Thread Ilya Lipnitskiy
There are code paths that rely on zero_pfn to be fully initialized
before core_initcall. For example, wq_sysfs_init() is a core_initcall
function that eventually results in a call to kernel_execve, which
causes a page fault with a subsequent mmput. If zero_pfn is not
initialized by then it may not get cleaned up properly and result in an
error:
  BUG: Bad rss-counter state mm:(ptrval) type:MM_ANONPAGES val:1

Here is an analysis of the race as seen on a MIPS device. On this
particular MT7621 device (Ubiquiti ER-X), zero_pfn is PFN 0 until
initialized, at which point it becomes PFN 5120:
  1. wq_sysfs_init calls into kobject_uevent_env at core_initcall:
   [<80340dc8>] kobject_uevent_env+0x7e4/0x7ec
   [<8033f8b8>] kset_register+0x68/0x88
   [<803cf824>] bus_register+0xdc/0x34c
   [<803cfac8>] subsys_virtual_register+0x34/0x78
   [<8086afb0>] wq_sysfs_init+0x1c/0x4c
   [<80001648>] do_one_initcall+0x50/0x1a8
   [<8086503c>] kernel_init_freeable+0x230/0x2c8
   [<8066bca0>] kernel_init+0x10/0x100
   [<80003038>] ret_from_kernel_thread+0x14/0x1c

  2. kobject_uevent_env() calls call_usermodehelper_exec() which executes
 kernel_execve asynchronously.

  3. Memory allocations in kernel_execve cause a page fault, bumping the
 MM reference counter:
   [<8015adb4>] add_mm_counter_fast+0xb4/0xc0
   [<80160d58>] handle_mm_fault+0x6e4/0xea0
   [<80158aa4>] __get_user_pages.part.78+0x190/0x37c
   [<8015992c>] __get_user_pages_remote+0x128/0x360
   [<801a6d9c>] get_arg_page+0x34/0xa0
   [<801a7394>] copy_string_kernel+0x194/0x2a4
   [<801a880c>] kernel_execve+0x11c/0x298
   [<800420f4>] call_usermodehelper_exec_async+0x114/0x194

  4. In case zero_pfn has not been initialized yet, zap_pte_range does
 not decrement the MM_ANONPAGES RSS counter and the BUG message is
 triggered shortly afterwards when __mmdrop checks the ref counters:
   [<800285e8>] __mmdrop+0x98/0x1d0
   [<801a6de8>] free_bprm+0x44/0x118
   [<801a86a8>] kernel_execve+0x160/0x1d8
   [<800420f4>] call_usermodehelper_exec_async+0x114/0x194
   [<80003198>] ret_from_kernel_thread+0x14/0x1c

To avoid races such as described above, initialize init_zero_pfn at
early_initcall level. Depending on the architecture, ZERO_PAGE is either
constant or gets initialized even earlier, at paging_init, so there is
no issue with initializing zero_pfn earlier.

Discussion: 
https://lkml.kernel.org/r/CALCv0x2YqOXEAy2Q=hafjhHCtTHVodChv1qpM=niaxopqeb...@mail.gmail.com

Signed-off-by: Ilya Lipnitskiy 
Cc: Hugh Dickins 
Cc: "Eric W. Biederman" 
Cc: sta...@vger.kernel.org
---
 mm/memory.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/memory.c b/mm/memory.c
index 5c3b29d3af66..e66b11ac1659 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -166,7 +166,7 @@ static int __init init_zero_pfn(void)
zero_pfn = page_to_pfn(ZERO_PAGE(0));
return 0;
 }
-core_initcall(init_zero_pfn);
+early_initcall(init_zero_pfn);
 
 void mm_trace_rss_stat(struct mm_struct *mm, int member, long count)
 {
-- 
2.31.0



[PATCH v2] mm: fix race by making init_zero_pfn() early_initcall

2021-03-28 Thread Ilya Lipnitskiy
There are code paths that rely on zero_pfn to be fully initialized
before core_initcall. For example, wq_sysfs_init() is a core_initcall
function that eventually results in a call to kernel_execve, which
causes a page fault with a subsequent mmput. If zero_pfn is not
initialized by then it may not get cleaned up properly and result in an
error:
  BUG: Bad rss-counter state mm:(ptrval) type:MM_ANONPAGES val:1

Here is an analysis of the race as seen on a MIPS device. On this
particular MT7621 device (Ubiquiti ER-X), zero_pfn is PFN 0 until
initialized, at which point it becomes PFN 5120:
  1. wq_sysfs_init calls into kobject_uevent_env at core_initcall:
   [<80340dc8>] kobject_uevent_env+0x7e4/0x7ec
   [<8033f8b8>] kset_register+0x68/0x88
   [<803cf824>] bus_register+0xdc/0x34c
   [<803cfac8>] subsys_virtual_register+0x34/0x78
   [<8086afb0>] wq_sysfs_init+0x1c/0x4c
   [<80001648>] do_one_initcall+0x50/0x1a8
   [<8086503c>] kernel_init_freeable+0x230/0x2c8
   [<8066bca0>] kernel_init+0x10/0x100
   [<80003038>] ret_from_kernel_thread+0x14/0x1c

  2. kobject_uevent_env() calls call_usermodehelper_exec() which executes
 kernel_execve asynchronously.

  3. Memory allocations in kernel_execve cause a page fault, bumping the
 MM reference counter:
   [<8015adb4>] add_mm_counter_fast+0xb4/0xc0
   [<80160d58>] handle_mm_fault+0x6e4/0xea0
   [<80158aa4>] __get_user_pages.part.78+0x190/0x37c
   [<8015992c>] __get_user_pages_remote+0x128/0x360
   [<801a6d9c>] get_arg_page+0x34/0xa0
   [<801a7394>] copy_string_kernel+0x194/0x2a4
   [<801a880c>] kernel_execve+0x11c/0x298
   [<800420f4>] call_usermodehelper_exec_async+0x114/0x194

  4. In case zero_pfn has not been initialized yet, zap_pte_range does
 not decrement the MM_ANONPAGES RSS counter and the BUG message is
 triggered shortly afterwards when __mmdrop checks the ref counters:
   [<800285e8>] __mmdrop+0x98/0x1d0
   [<801a6de8>] free_bprm+0x44/0x118
   [<801a86a8>] kernel_execve+0x160/0x1d8
   [<800420f4>] call_usermodehelper_exec_async+0x114/0x194
   [<80003198>] ret_from_kernel_thread+0x14/0x1c

To avoid races such as described above, initialize init_zero_pfn at
early_initcall level. Depending on the architecture, ZERO_PAGE is either
constant or gets initialized even earlier, at paging_init, so there is
no issue with initializing zero_pfn earlier.

ML discussion: 
https://lore.kernel.org/lkml/CALCv0x2YqOXEAy2Q=hafjhHCtTHVodChv1qpM=niaxopqeb...@mail.gmail.com/

Signed-off-by: Ilya Lipnitskiy 
Cc: "Eric W. Biederman" 
Cc: sta...@vger.kernel.org
---
 mm/memory.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/memory.c b/mm/memory.c
index 46ef306375bd..a8bbc4fc121f 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -166,7 +166,7 @@ static int __init init_zero_pfn(void)
zero_pfn = page_to_pfn(ZERO_PAGE(0));
return 0;
 }
-core_initcall(init_zero_pfn);
+early_initcall(init_zero_pfn);
 
 void mm_trace_rss_stat(struct mm_struct *mm, int member, long count)
 {
-- 
2.31.0



[PATCH] mm: fix race by making init_zero_pfn() early_initcall

2021-03-28 Thread Ilya Lipnitskiy
There are code paths that rely on zero_pfn to be fully initialized
before core_initcall. For example, wq_sysfs_init() is a core_initcall
function that eventually results in a call to kernel_execve, which
causes a page fault with a subsequent mmput. If zero_pfn is not
initialized by then it may not get cleaned up properly and result in an
error:
  BUG: Bad rss-counter state mm:(ptrval) type:MM_ANONPAGES val:1

Here is an analysis of the race as seen on a MIPS device. On this
particular MT7621 device (Ubiquiti ER-X), zero_pfn is PFN 0 until
initialized, at which point it becomes PFN 5120:
  1. wq_sysfs_init calls into kobject_uevent_env at core_initcall:
   [<80340dc8>] kobject_uevent_env+0x7e4/0x7ec
   [<8033f8b8>] kset_register+0x68/0x88
   [<803cf824>] bus_register+0xdc/0x34c
   [<803cfac8>] subsys_virtual_register+0x34/0x78
   [<8086afb0>] wq_sysfs_init+0x1c/0x4c
   [<80001648>] do_one_initcall+0x50/0x1a8
   [<8086503c>] kernel_init_freeable+0x230/0x2c8
   [<8066bca0>] kernel_init+0x10/0x100
   [<80003038>] ret_from_kernel_thread+0x14/0x1c

  2. kobject_uevent_env() calls call_usermodehelper_exec() which executes
 kernel_execve asynchronously.

  3. Memory allocations in kernel_execve cause a page fault, bumping the
 MM reference counter:
   [<8015adb4>] add_mm_counter_fast+0xb4/0xc0
   [<80160d58>] handle_mm_fault+0x6e4/0xea0
   [<80158aa4>] __get_user_pages.part.78+0x190/0x37c
   [<8015992c>] __get_user_pages_remote+0x128/0x360
   [<801a6d9c>] get_arg_page+0x34/0xa0
   [<801a7394>] copy_string_kernel+0x194/0x2a4
   [<801a880c>] kernel_execve+0x11c/0x298
   [<800420f4>] call_usermodehelper_exec_async+0x114/0x194

  4. In case zero_pfn has not been initialized yet, zap_pte_range does
 not decrement the MM_ANONPAGES RSS counter and the BUG message is
 triggered shortly afterwards when __mmdrop checks the ref counters:
   [<800285e8>] __mmdrop+0x98/0x1d0
   [<801a6de8>] free_bprm+0x44/0x118
   [<801a86a8>] kernel_execve+0x160/0x1d8
   [<800420f4>] call_usermodehelper_exec_async+0x114/0x194
   [<80003198>] ret_from_kernel_thread+0x14/0x1c

To avoid races such as described above, initialize init_zero_pfn at
early_initcall level. Depending on the architecture, ZERO_PAGE is either
constant or gets initialized even earlier, at paging_init, so there is
no issue with initializing zero_pfn earlier.

ML discussion: 
https://lore.kernel.org/lkml/CALCv0x2YqOXEAy2Q=hafjhHCtTHVodChv1qpM=niaxopqeb...@mail.gmail.com/

Signed-off-by: Ilya Lipnitskiy 
Cc: "Eric W. Biederman" 
---
 mm/memory.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/memory.c b/mm/memory.c
index 46ef306375bd..a8bbc4fc121f 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -166,7 +166,7 @@ static int __init init_zero_pfn(void)
zero_pfn = page_to_pfn(ZERO_PAGE(0));
return 0;
 }
-core_initcall(init_zero_pfn);
+early_initcall(init_zero_pfn);
 
 void mm_trace_rss_stat(struct mm_struct *mm, int member, long count)
 {
-- 
2.31.0



Re: exec error: BUG: Bad rss-counter

2021-03-28 Thread Ilya Lipnitskiy
On Sat, Mar 20, 2021 at 8:59 AM Zhou Yanjie  wrote:
>
> Hi Ilya,
>
> On 2021/3/3 下午11:55, Ilya Lipnitskiy wrote:
> > On Wed, Mar 3, 2021 at 7:50 AM Eric W. Biederman  
> > wrote:
> >> Ilya Lipnitskiy  writes:
> >>
> >>> On Tue, Mar 2, 2021 at 11:37 AM Eric W. Biederman  
> >>> wrote:
> >>>> Ilya Lipnitskiy  writes:
> >>>>
> >>>>> On Mon, Mar 1, 2021 at 12:43 PM Eric W. Biederman 
> >>>>>  wrote:
> >>>>>> Ilya Lipnitskiy  writes:
> >>>>>>
> >>>>>>> Eric, All,
> >>>>>>>
> >>>>>>> The following error appears when running Linux 5.10.18 on an embedded
> >>>>>>> MIPS mt7621 target:
> >>>>>>> [0.301219] BUG: Bad rss-counter state mm:(ptrval) 
> >>>>>>> type:MM_ANONPAGES val:1
> >>>>>>>
> >>>>>>> Being a very generic error, I started digging and added a stack dump
> >>>>>>> before the BUG:
> >>>>>>> Call Trace:
> >>>>>>> [<80008094>] show_stack+0x30/0x100
> >>>>>>> [<8033b238>] dump_stack+0xac/0xe8
> >>>>>>> [<800285e8>] __mmdrop+0x98/0x1d0
> >>>>>>> [<801a6de8>] free_bprm+0x44/0x118
> >>>>>>> [<801a86a8>] kernel_execve+0x160/0x1d8
> >>>>>>> [<800420f4>] call_usermodehelper_exec_async+0x114/0x194
> >>>>>>> [<80003198>] ret_from_kernel_thread+0x14/0x1c
> >>>>>>>
> >>>>>>> So that's how I got to looking at fs/exec.c and noticed quite a few
> >>>>>>> changes last year. Turns out this message only occurs once very early
> >>>>>>> at boot during the very first call to kernel_execve. current->mm is
> >>>>>>> NULL at this stage, so acct_arg_size() is effectively a no-op.
> >>>>>> If you believe this is a new error you could bisect the kernel
> >>>>>> to see which change introduced the behavior you are seeing.
> >>>>>>
> >>>>>>> More digging, and I traced the RSS counter increment to:
> >>>>>>> [<8015adb4>] add_mm_counter_fast+0xb4/0xc0
> >>>>>>> [<80160d58>] handle_mm_fault+0x6e4/0xea0
> >>>>>>> [<80158aa4>] __get_user_pages.part.78+0x190/0x37c
> >>>>>>> [<8015992c>] __get_user_pages_remote+0x128/0x360
> >>>>>>> [<801a6d9c>] get_arg_page+0x34/0xa0
> >>>>>>> [<801a7394>] copy_string_kernel+0x194/0x2a4
> >>>>>>> [<801a880c>] kernel_execve+0x11c/0x298
> >>>>>>> [<800420f4>] call_usermodehelper_exec_async+0x114/0x194
> >>>>>>> [<80003198>] ret_from_kernel_thread+0x14/0x1c
> >>>>>>>
> >>>>>>> In fact, I also checked vma_pages(bprm->vma) and lo and behold it is 
> >>>>>>> set to 1.
> >>>>>>>
> >>>>>>> How is fs/exec.c supposed to handle implied RSS increments that happen
> >>>>>>> due to page faults when discarding the bprm structure? In this case,
> >>>>>>> the bug-generating kernel_execve call never succeeded, it returned -2,
> >>>>>>> but I didn't trace exactly what failed.
> >>>>>> Unless I am mistaken any left over pages should be purged by exit_mmap
> >>>>>> which is called by mmput before mmput calls mmdrop.
> >>>>> Good to know. Some more digging and I can say that we hit this error
> >>>>> when trying to unmap PFN 0 (is_zero_pfn(pfn) returns TRUE,
> >>>>> vm_normal_page returns NULL, zap_pte_range does not decrement
> >>>>> MM_ANONPAGES RSS counter). Is my understanding correct that PFN 0 is
> >>>>> usable, but special? Or am I totally off the mark here?
> >>>> It would be good to know if that is the page that get_user_pages_remote
> >>>> returned to copy_string_kernel.  The zero page that is always zero,
> >>>> should never be returned when a writable mapping is desired.
> >>> Indeed, pfn 0 is returned from get_arg_page: (page is 0x809cf000,
> >>> page_to_pfn(page) is 0) and it is the same page that is being freed and 
> >>> no

Re: exec error: BUG: Bad rss-counter

2021-03-28 Thread Ilya Lipnitskiy
On Wed, Mar 3, 2021 at 7:50 AM Eric W. Biederman  wrote:
>
> Ilya Lipnitskiy  writes:
>
> > On Tue, Mar 2, 2021 at 11:37 AM Eric W. Biederman  
> > wrote:
> >>
> >> Ilya Lipnitskiy  writes:
> >>
> >> > On Mon, Mar 1, 2021 at 12:43 PM Eric W. Biederman 
> >> >  wrote:
> >> >>
> >> >> Ilya Lipnitskiy  writes:
> >> >>
> >> >> > Eric, All,
> >> >> >
> >> >> > The following error appears when running Linux 5.10.18 on an embedded
> >> >> > MIPS mt7621 target:
> >> >> > [0.301219] BUG: Bad rss-counter state mm:(ptrval) 
> >> >> > type:MM_ANONPAGES val:1
> >> >> >
> >> >> > Being a very generic error, I started digging and added a stack dump
> >> >> > before the BUG:
> >> >> > Call Trace:
> >> >> > [<80008094>] show_stack+0x30/0x100
> >> >> > [<8033b238>] dump_stack+0xac/0xe8
> >> >> > [<800285e8>] __mmdrop+0x98/0x1d0
> >> >> > [<801a6de8>] free_bprm+0x44/0x118
> >> >> > [<801a86a8>] kernel_execve+0x160/0x1d8
> >> >> > [<800420f4>] call_usermodehelper_exec_async+0x114/0x194
> >> >> > [<80003198>] ret_from_kernel_thread+0x14/0x1c
> >> >> >
> >> >> > So that's how I got to looking at fs/exec.c and noticed quite a few
> >> >> > changes last year. Turns out this message only occurs once very early
> >> >> > at boot during the very first call to kernel_execve. current->mm is
> >> >> > NULL at this stage, so acct_arg_size() is effectively a no-op.
> >> >>
> >> >> If you believe this is a new error you could bisect the kernel
> >> >> to see which change introduced the behavior you are seeing.
> >> >>
> >> >> > More digging, and I traced the RSS counter increment to:
> >> >> > [<8015adb4>] add_mm_counter_fast+0xb4/0xc0
> >> >> > [<80160d58>] handle_mm_fault+0x6e4/0xea0
> >> >> > [<80158aa4>] __get_user_pages.part.78+0x190/0x37c
> >> >> > [<8015992c>] __get_user_pages_remote+0x128/0x360
> >> >> > [<801a6d9c>] get_arg_page+0x34/0xa0
> >> >> > [<801a7394>] copy_string_kernel+0x194/0x2a4
> >> >> > [<801a880c>] kernel_execve+0x11c/0x298
> >> >> > [<800420f4>] call_usermodehelper_exec_async+0x114/0x194
> >> >> > [<80003198>] ret_from_kernel_thread+0x14/0x1c
> >> >> >
> >> >> > In fact, I also checked vma_pages(bprm->vma) and lo and behold it is 
> >> >> > set to 1.
> >> >> >
> >> >> > How is fs/exec.c supposed to handle implied RSS increments that happen
> >> >> > due to page faults when discarding the bprm structure? In this case,
> >> >> > the bug-generating kernel_execve call never succeeded, it returned -2,
> >> >> > but I didn't trace exactly what failed.
> >> >>
> >> >> Unless I am mistaken any left over pages should be purged by exit_mmap
> >> >> which is called by mmput before mmput calls mmdrop.
> >> > Good to know. Some more digging and I can say that we hit this error
> >> > when trying to unmap PFN 0 (is_zero_pfn(pfn) returns TRUE,
> >> > vm_normal_page returns NULL, zap_pte_range does not decrement
> >> > MM_ANONPAGES RSS counter). Is my understanding correct that PFN 0 is
> >> > usable, but special? Or am I totally off the mark here?
> >>
> >> It would be good to know if that is the page that get_user_pages_remote
> >> returned to copy_string_kernel.  The zero page that is always zero,
> >> should never be returned when a writable mapping is desired.
> >
> > Indeed, pfn 0 is returned from get_arg_page: (page is 0x809cf000,
> > page_to_pfn(page) is 0) and it is the same page that is being freed and not
> > refcounted in mmput/zap_pte_range. Confirmed with good old printk. Also,
> > ZERO_PAGE(0)==0x809fc000 -> PFN 5120.
> >
> > I think I have found the problem though, after much digging and thanks to 
> > all
> > the information provided. init_zero_pfn() gets called too late (after
> > the call to
> > is_zero_pfn(0) from mmput returns true), until then zero_pfn == 0, and 
> > after,
> > zero_pfn == 5120. Boom.
&g

[PATCH v3] crypto: mips: add poly1305-core.S to .gitignore

2021-03-27 Thread Ilya Lipnitskiy
poly1305-core.S is an auto-generated file, so it should be ignored.

Fixes: a11d055e7a64 ("crypto: mips/poly1305 - incorporate OpenSSL/CRYPTOGAMS 
optimized implementation")
Signed-off-by: Ilya Lipnitskiy 
Cc: Ard Biesheuvel 
---
 arch/mips/crypto/.gitignore | 2 ++
 1 file changed, 2 insertions(+)
 create mode 100644 arch/mips/crypto/.gitignore

diff --git a/arch/mips/crypto/.gitignore b/arch/mips/crypto/.gitignore
new file mode 100644
index ..0d47d4f21c6d
--- /dev/null
+++ b/arch/mips/crypto/.gitignore
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0-only
+poly1305-core.S
-- 
2.31.0



[PATCH v2] crypto: mips: add poly1305-core.S to .gitignore

2021-03-27 Thread Ilya Lipnitskiy
poly1305-core.S is an auto-generated file, so it should be ignored.

Fixes: a11d055e7a64 ("crypto: mips/poly1305 - incorporate OpenSSL/CRYPTOGAMS 
optimized implementation")
Signed-off-by: Ilya Lipnitskiy 
Cc: Ard Biesheuvel 
---
 arch/mips/crypto/.gitignore | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 arch/mips/crypto/.gitignore

diff --git a/arch/mips/crypto/.gitignore b/arch/mips/crypto/.gitignore
new file mode 100644
index ..4882ba199071
--- /dev/null
+++ b/arch/mips/crypto/.gitignore
@@ -0,0 +1 @@
+poly1305-core.S
-- 
2.31.0



[PATCH] crypto: mips: add poly1305-core.S to .gitignore

2021-03-27 Thread Ilya Lipnitskiy
poly1305-core.S is an auto-generated file, so it should be ignored.

Signed-off-by: Ilya Lipnitskiy 
Cc: Ard Biesheuvel 
---
 arch/mips/crypto/.gitignore | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 arch/mips/crypto/.gitignore

diff --git a/arch/mips/crypto/.gitignore b/arch/mips/crypto/.gitignore
new file mode 100644
index ..4882ba199071
--- /dev/null
+++ b/arch/mips/crypto/.gitignore
@@ -0,0 +1 @@
+poly1305-core.S
-- 
2.31.0



Re: [PATCH] MIPS: ralink: mt7621: add memory detection support

2021-03-27 Thread Ilya Lipnitskiy
On Sat, Mar 27, 2021 at 2:06 PM Thomas Bogendoerfer
 wrote:
>
> On Sat, Mar 27, 2021 at 09:35:43AM -0700, Ilya Lipnitskiy wrote:
> > On Sat, Mar 27, 2021 at 2:46 AM Thomas Bogendoerfer
> >  wrote:
> > >
> > > On Thu, Mar 25, 2021 at 07:45:23PM -0700, Ilya Lipnitskiy wrote:
> > > > On Thu, Mar 25, 2021 at 3:01 AM Thomas Bogendoerfer
> > > >  wrote:
> > > > >
> > > > > On Tue, Mar 16, 2021 at 10:59:02PM -0700, Ilya Lipnitskiy wrote:
> > > > > > From: Chuanhong Guo 
> > > > > >
> > > > > > mt7621 has the following memory map:
> > > > > > 0x0-0x1c00: lower 448m memory
> > > > > > 0x1c00-0x200: peripheral registers
> > > > > > 0x2000-0x240: higher 64m memory
> > > > > >
> > > > > > detect_memory_region in arch/mips/kernel/setup.c only adds the first
> > > > > > memory region and isn't suitable for 512m memory detection because
> > > > > > it may accidentally read the memory area for peripheral registers.
> > > > > >
> > > > > > This commit adds memory detection capability for mt7621:
> > > > > >   1. Add the highmem area when 512m is detected.
> > > > > >   2. Guard memcmp from accessing peripheral registers:
> > > > > >  This only happens when a user decided to change kernel load 
> > > > > > address
> > > > > >  to 256m or higher address. Since this is a quite unusual case, 
> > > > > > we
> > > > > >  just skip 512m testing and return 256m as memory size.
> > > > > >
> > > > > > [...]
> > > > >
> > > > > I get
> > > > >
> > > > > WARNING: modpost: vmlinux.o(.text+0x132c): Section mismatch in 
> > > > > reference from the function prom_soc_init() to the function 
> > > > > .init.text:mt7621_memory_detect()
> > > > > The function prom_soc_init() references
> > > > > the function __init mt7621_memory_detect().
> > > > > This is often because prom_soc_init lacks a __init
> > > > > annotation or the annotation of mt7621_memory_detect is wrong.
> > > > >
> > > > > Can you please fix this ?
> > > > Thanks, I will fix it. Having trouble reproducing the error, but I
> > > > clearly see the issue. Are you building on a MIPS target or
> > > > cross-compiling (I'm cross-compiling and no errors).
> > >
> > > I'm cross compiling, I can provide you the .config, if you want.
> > Yeah, that would help. I spent quite a bit of time trying to reproduce
> > - tried different options with GCC 8 and GCC 10 to no avail. Maybe you
> > are using clang?
>
> no, but an older gcc (gcc version 6.1.1 20160621 (Red Hat Cross 6.1.1-2) 
> (GCC)).
> config is attached.
Thanks, disabling CONFIG_LD_DEAD_CODE_DATA_ELIMINATION reproduced it
even with GCC 10. Fixed in
https://lore.kernel.org/linux-arm-kernel/20210327053840.471155-1-ilya.lipnits...@gmail.com/

- Ilya


Re: [PATCH] MIPS: ralink: mt7621: add memory detection support

2021-03-27 Thread Ilya Lipnitskiy
On Sat, Mar 27, 2021 at 2:46 AM Thomas Bogendoerfer
 wrote:
>
> On Thu, Mar 25, 2021 at 07:45:23PM -0700, Ilya Lipnitskiy wrote:
> > On Thu, Mar 25, 2021 at 3:01 AM Thomas Bogendoerfer
> >  wrote:
> > >
> > > On Tue, Mar 16, 2021 at 10:59:02PM -0700, Ilya Lipnitskiy wrote:
> > > > From: Chuanhong Guo 
> > > >
> > > > mt7621 has the following memory map:
> > > > 0x0-0x1c00: lower 448m memory
> > > > 0x1c00-0x200: peripheral registers
> > > > 0x2000-0x240: higher 64m memory
> > > >
> > > > detect_memory_region in arch/mips/kernel/setup.c only adds the first
> > > > memory region and isn't suitable for 512m memory detection because
> > > > it may accidentally read the memory area for peripheral registers.
> > > >
> > > > This commit adds memory detection capability for mt7621:
> > > >   1. Add the highmem area when 512m is detected.
> > > >   2. Guard memcmp from accessing peripheral registers:
> > > >  This only happens when a user decided to change kernel load address
> > > >  to 256m or higher address. Since this is a quite unusual case, we
> > > >  just skip 512m testing and return 256m as memory size.
> > > >
> > > > [...]
> > >
> > > I get
> > >
> > > WARNING: modpost: vmlinux.o(.text+0x132c): Section mismatch in reference 
> > > from the function prom_soc_init() to the function 
> > > .init.text:mt7621_memory_detect()
> > > The function prom_soc_init() references
> > > the function __init mt7621_memory_detect().
> > > This is often because prom_soc_init lacks a __init
> > > annotation or the annotation of mt7621_memory_detect is wrong.
> > >
> > > Can you please fix this ?
> > Thanks, I will fix it. Having trouble reproducing the error, but I
> > clearly see the issue. Are you building on a MIPS target or
> > cross-compiling (I'm cross-compiling and no errors).
>
> I'm cross compiling, I can provide you the .config, if you want.
Yeah, that would help. I spent quite a bit of time trying to reproduce
- tried different options with GCC 8 and GCC 10 to no avail. Maybe you
are using clang?

Ilya


[PATCH net-next,v2] net: dsa: mt7530: clean up core and TRGMII clock setup

2021-03-27 Thread Ilya Lipnitskiy
Three minor changes:

- When disabling PLL, there is no need to call core_write_mmd_indirect
  directly, use the core_write wrapper instead like the rest of the code
  in the function does. This change helps with consistency and
  readability. Move the comment to the definition of
  core_read_mmd_indirect where it belongs.

- Disable both core and TRGMII Tx clocks prior to reconfiguring.
  Previously, only the core clock was disabled, but not TRGMII Tx clock.
  So disable both, then configure them, then re-enable both, for
  consistency.

- The core clock enable bit (REG_GSWCK_EN) is written redundantly three
  times. Simplify the code and only write the register only once at the
  end of clock reconfiguration to enable both core and TRGMII Tx clocks.

Tested on Ubiquiti ER-X running the GMAC0 and MT7530 in TRGMII mode.

Signed-off-by: Ilya Lipnitskiy 
---
 drivers/net/dsa/mt7530.c | 29 +
 1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index c442a5885fca..2bd1bab71497 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -67,6 +67,11 @@ static const struct mt7530_mib_desc mt7530_mib[] = {
MIB_DESC(1, 0xb8, "RxArlDrop"),
 };
 
+/* Since phy_device has not yet been created and
+ * phy_{read,write}_mmd_indirect is not available, we provide our own
+ * core_{read,write}_mmd_indirect with core_{clear,write,set} wrappers
+ * to complete this function.
+ */
 static int
 core_read_mmd_indirect(struct mt7530_priv *priv, int prtad, int devad)
 {
@@ -435,19 +440,13 @@ mt7530_pad_clk_setup(struct dsa_switch *ds, 
phy_interface_t interface)
mt7530_write(priv, MT7530_TRGMII_TD_ODT(i),
 TD_DM_DRVP(8) | TD_DM_DRVN(8));
 
-   /* Setup core clock for MT7530 */
-   /* Disable MT7530 core clock */
-   core_clear(priv, CORE_TRGMII_GSW_CLK_CG, REG_GSWCK_EN);
+   /* Disable MT7530 core and TRGMII Tx clocks */
+   core_clear(priv, CORE_TRGMII_GSW_CLK_CG,
+  REG_GSWCK_EN | REG_TRGMIICK_EN);
 
-   /* Disable PLL, since phy_device has not yet been created
-* provided for phy_[read,write]_mmd_indirect is called, we
-* provide our own core_write_mmd_indirect to complete this
-* function.
-*/
-   core_write_mmd_indirect(priv,
-   CORE_GSWPLL_GRP1,
-   MDIO_MMD_VEND2,
-   0);
+   /* Setup core clock for MT7530 */
+   /* Disable PLL */
+   core_write(priv, CORE_GSWPLL_GRP1, 0);
 
/* Set core clock into 500Mhz */
core_write(priv, CORE_GSWPLL_GRP2,
@@ -460,11 +459,7 @@ mt7530_pad_clk_setup(struct dsa_switch *ds, 
phy_interface_t interface)
   RG_GSWPLL_POSDIV_200M(2) |
   RG_GSWPLL_FBKDIV_200M(32));
 
-   /* Enable MT7530 core clock */
-   core_set(priv, CORE_TRGMII_GSW_CLK_CG, REG_GSWCK_EN);
-
/* Setup the MT7530 TRGMII Tx Clock */
-   core_set(priv, CORE_TRGMII_GSW_CLK_CG, REG_GSWCK_EN);
core_write(priv, CORE_PLL_GROUP5, RG_LCDDS_PCW_NCPO1(ncpo1));
core_write(priv, CORE_PLL_GROUP6, RG_LCDDS_PCW_NCPO0(0));
core_write(priv, CORE_PLL_GROUP10, RG_LCDDS_SSC_DELTA(ssc_delta));
@@ -478,6 +473,8 @@ mt7530_pad_clk_setup(struct dsa_switch *ds, phy_interface_t 
interface)
core_write(priv, CORE_PLL_GROUP7,
   RG_LCDDS_PCW_NCPO_CHG | RG_LCCDS_C(3) |
   RG_LCDDS_PWDB | RG_LCDDS_ISO_EN);
+
+   /* Enable MT7530 core and TRGMII Tx clocks */
core_set(priv, CORE_TRGMII_GSW_CLK_CG,
 REG_GSWCK_EN | REG_TRGMIICK_EN);
 
-- 
2.31.0



[PATCH net-next] net: dsa: mt7530: clean up core and TRGMII clock setup

2021-03-26 Thread Ilya Lipnitskiy
Three minor changes:

- When disabling PLL, there is no need to call core_write_mmd_indirect
  directly, use the core_write wrapper instead like the rest of the code
  in the function does. This change helps with consistency and
  readability. Move the comment to the definition of
  core_write_mmd_indirect where it belongs.

- Disable both core and TRGMII Tx clocks prior to reconfiguring.
  Previously, only the core clock was disabled, but not TRGMII Tx clock.
  So disable both, then configure them, then re-enable both, for
  consistency.

- The core clock enable bit (REG_GSWCK_EN) is written redundantly three
  times. Simplify the code and only write the register only once at the
  end of clock reconfiguration to enable both core and TRGMII Tx clocks.

Tested on Ubiquiti ER-X running the GMAC0 and MT7530 in TRGMII mode.

Signed-off-by: Ilya Lipnitskiy 
---
 drivers/net/dsa/mt7530.c | 29 +
 1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index c442a5885fca..d779f40d46d9 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -67,6 +67,11 @@ static const struct mt7530_mib_desc mt7530_mib[] = {
MIB_DESC(1, 0xb8, "RxArlDrop"),
 };
 
+/* Since phy_device has not yet been created and
+ * phy_[read,write]_mmd_indirect is not available, we provide our own
+ * core_write_mmd_indirect with core_{clear,write,set} wrappers to
+ * complete this function.
+ */
 static int
 core_read_mmd_indirect(struct mt7530_priv *priv, int prtad, int devad)
 {
@@ -435,19 +440,13 @@ mt7530_pad_clk_setup(struct dsa_switch *ds, 
phy_interface_t interface)
mt7530_write(priv, MT7530_TRGMII_TD_ODT(i),
 TD_DM_DRVP(8) | TD_DM_DRVN(8));
 
-   /* Setup core clock for MT7530 */
-   /* Disable MT7530 core clock */
-   core_clear(priv, CORE_TRGMII_GSW_CLK_CG, REG_GSWCK_EN);
+   /* Disable MT7530 core and TRGMII Tx clocks */
+   core_clear(priv, CORE_TRGMII_GSW_CLK_CG,
+  REG_GSWCK_EN | REG_TRGMIICK_EN);
 
-   /* Disable PLL, since phy_device has not yet been created
-* provided for phy_[read,write]_mmd_indirect is called, we
-* provide our own core_write_mmd_indirect to complete this
-* function.
-*/
-   core_write_mmd_indirect(priv,
-   CORE_GSWPLL_GRP1,
-   MDIO_MMD_VEND2,
-   0);
+   /* Setup core clock for MT7530 */
+   /* Disable PLL */
+   core_write(priv, CORE_GSWPLL_GRP1, 0);
 
/* Set core clock into 500Mhz */
core_write(priv, CORE_GSWPLL_GRP2,
@@ -460,11 +459,7 @@ mt7530_pad_clk_setup(struct dsa_switch *ds, 
phy_interface_t interface)
   RG_GSWPLL_POSDIV_200M(2) |
   RG_GSWPLL_FBKDIV_200M(32));
 
-   /* Enable MT7530 core clock */
-   core_set(priv, CORE_TRGMII_GSW_CLK_CG, REG_GSWCK_EN);
-
/* Setup the MT7530 TRGMII Tx Clock */
-   core_set(priv, CORE_TRGMII_GSW_CLK_CG, REG_GSWCK_EN);
core_write(priv, CORE_PLL_GROUP5, RG_LCDDS_PCW_NCPO1(ncpo1));
core_write(priv, CORE_PLL_GROUP6, RG_LCDDS_PCW_NCPO0(0));
core_write(priv, CORE_PLL_GROUP10, RG_LCDDS_SSC_DELTA(ssc_delta));
@@ -478,6 +473,8 @@ mt7530_pad_clk_setup(struct dsa_switch *ds, phy_interface_t 
interface)
core_write(priv, CORE_PLL_GROUP7,
   RG_LCDDS_PCW_NCPO_CHG | RG_LCCDS_C(3) |
   RG_LCDDS_PWDB | RG_LCDDS_ISO_EN);
+
+   /* Enable MT7530 core and TRGMII Tx clocks */
core_set(priv, CORE_TRGMII_GSW_CLK_CG,
 REG_GSWCK_EN | REG_TRGMIICK_EN);
 
-- 
2.31.0



[PATCH 2/2] MIPS: ralink: mt7621: add memory detection support

2021-03-26 Thread Ilya Lipnitskiy
From: Chuanhong Guo 

mt7621 has the following memory map:
0x0-0x1c00: lower 448m memory
0x1c00-0x200: peripheral registers
0x2000-0x240: higher 64m memory

detect_memory_region in arch/mips/kernel/setup.c only adds the first
memory region and isn't suitable for 512m memory detection because
it may accidentally read the memory area for peripheral registers.

This commit adds memory detection capability for mt7621:
  1. Add the highmem area when 512m is detected.
  2. Guard memcmp from accessing peripheral registers:
 This only happens when a user decided to change kernel load address
 to 256m or higher address. Since this is a quite unusual case, we
 just skip 512m testing and return 256m as memory size.

Signed-off-by: Chuanhong Guo 
[Minor commit message reword, make mt7621_memory_detect static]
Signed-off-by: Ilya Lipnitskiy 
---
 arch/mips/include/asm/mach-ralink/mt7621.h |  7 +++---
 arch/mips/ralink/common.h  |  1 +
 arch/mips/ralink/mt7621.c  | 29 +++---
 arch/mips/ralink/of.c  |  2 ++
 4 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/arch/mips/include/asm/mach-ralink/mt7621.h 
b/arch/mips/include/asm/mach-ralink/mt7621.h
index e1af1ba50bd8..6bbf082dd149 100644
--- a/arch/mips/include/asm/mach-ralink/mt7621.h
+++ b/arch/mips/include/asm/mach-ralink/mt7621.h
@@ -24,9 +24,10 @@
 #define CHIP_REV_VER_SHIFT 8
 #define CHIP_REV_ECO_MASK  0xf
 
-#define MT7621_DRAM_BASE0x0
-#define MT7621_DDR2_SIZE_MIN   32
-#define MT7621_DDR2_SIZE_MAX   256
+#define MT7621_LOWMEM_BASE 0x0
+#define MT7621_LOWMEM_MAX_SIZE 0x1C00
+#define MT7621_HIGHMEM_BASE0x2000
+#define MT7621_HIGHMEM_SIZE0x400
 
 #define MT7621_CHIP_NAME0  0x3637544D
 #define MT7621_CHIP_NAME1  0x20203132
diff --git a/arch/mips/ralink/common.h b/arch/mips/ralink/common.h
index 49ae370d023d..87fc16751281 100644
--- a/arch/mips/ralink/common.h
+++ b/arch/mips/ralink/common.h
@@ -17,6 +17,7 @@ struct ralink_soc_info {
unsigned long mem_size;
unsigned long mem_size_min;
unsigned long mem_size_max;
+   void (*mem_detect)(void);
 };
 extern struct ralink_soc_info soc_info;
 
diff --git a/arch/mips/ralink/mt7621.c b/arch/mips/ralink/mt7621.c
index ec87ce561049..6b3db98894cb 100644
--- a/arch/mips/ralink/mt7621.c
+++ b/arch/mips/ralink/mt7621.c
@@ -9,7 +9,9 @@
 #include 
 #include 
 #include 
+#include 
 
+#include 
 #include 
 #include 
 #include 
@@ -49,6 +51,8 @@
 #define MT7621_GPIO_MODE_SDHCI_SHIFT   18
 #define MT7621_GPIO_MODE_SDHCI_GPIO1
 
+static void *detect_magic __initdata = detect_memory_region;
+
 static struct rt2880_pmx_func uart1_grp[] =  { FUNC("uart1", 0, 1, 2) };
 static struct rt2880_pmx_func i2c_grp[] =  { FUNC("i2c", 0, 3, 2) };
 static struct rt2880_pmx_func uart3_grp[] = {
@@ -110,6 +114,26 @@ phys_addr_t mips_cpc_default_phys_base(void)
panic("Cannot detect cpc address");
 }
 
+static void __init mt7621_memory_detect(void)
+{
+   void *dm = _magic;
+   phys_addr_t size;
+
+   for (size = 32 * SZ_1M; size < 256 * SZ_1M; size <<= 1) {
+   if (!__builtin_memcmp(dm, dm + size, sizeof(detect_magic)))
+   break;
+   }
+
+   if ((size == 256 * SZ_1M) &&
+   (CPHYSADDR(dm + size) < MT7621_LOWMEM_MAX_SIZE) &&
+   __builtin_memcmp(dm, dm + size, sizeof(detect_magic))) {
+   memblock_add(MT7621_LOWMEM_BASE, MT7621_LOWMEM_MAX_SIZE);
+   memblock_add(MT7621_HIGHMEM_BASE, MT7621_HIGHMEM_SIZE);
+   } else {
+   memblock_add(MT7621_LOWMEM_BASE, size);
+   }
+}
+
 void __init ralink_of_remap(void)
 {
rt_sysc_membase = plat_of_remap_node("mtk,mt7621-sysc");
@@ -194,10 +218,7 @@ void __init prom_soc_init(struct ralink_soc_info *soc_info)
(rev >> CHIP_REV_VER_SHIFT) & CHIP_REV_VER_MASK,
(rev & CHIP_REV_ECO_MASK));
 
-   soc_info->mem_size_min = MT7621_DDR2_SIZE_MIN;
-   soc_info->mem_size_max = MT7621_DDR2_SIZE_MAX;
-   soc_info->mem_base = MT7621_DRAM_BASE;
-
+   soc_info->mem_detect = mt7621_memory_detect;
rt2880_pinmux_data = mt7621_pinmux_data;
 
soc_dev_init(soc_info, rev);
diff --git a/arch/mips/ralink/of.c b/arch/mips/ralink/of.c
index 8286c3521476..0c5de07da097 100644
--- a/arch/mips/ralink/of.c
+++ b/arch/mips/ralink/of.c
@@ -78,6 +78,8 @@ void __init plat_mem_setup(void)
of_scan_flat_dt(early_init_dt_find_memory, NULL);
if (memory_dtb)
of_scan_flat_dt(early_init_dt_scan_memory, NULL);
+   else if (soc_info.mem_detect)
+   soc_info.mem_detect();
else if (soc_info.mem_size)
memblock_add(soc_info.mem_base, soc_info.mem_size * SZ_1M);
else
-- 
2.31.0



[PATCH 1/2] MIPS: ralink: annotate prom_soc_init() with __init

2021-03-26 Thread Ilya Lipnitskiy
prom_soc_init is only called from prom_init in arch/mips/ralink/prom.c,
which is already annotated with __init, so annotate prom_soc_init with
__init too.

Signed-off-by: Ilya Lipnitskiy 
---
 arch/mips/ralink/common.h | 2 +-
 arch/mips/ralink/mt7620.c | 2 +-
 arch/mips/ralink/mt7621.c | 2 +-
 arch/mips/ralink/rt288x.c | 2 +-
 arch/mips/ralink/rt305x.c | 2 +-
 arch/mips/ralink/rt3883.c | 2 +-
 6 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/mips/ralink/common.h b/arch/mips/ralink/common.h
index 4bc65b7a3241..49ae370d023d 100644
--- a/arch/mips/ralink/common.h
+++ b/arch/mips/ralink/common.h
@@ -27,7 +27,7 @@ extern void ralink_clk_add(const char *dev, unsigned long 
rate);
 
 extern void ralink_rst_init(void);
 
-extern void prom_soc_init(struct ralink_soc_info *soc_info);
+extern void __init prom_soc_init(struct ralink_soc_info *soc_info);
 
 __iomem void *plat_of_remap_node(const char *node);
 
diff --git a/arch/mips/ralink/mt7620.c b/arch/mips/ralink/mt7620.c
index fcf010038054..53a5969e61af 100644
--- a/arch/mips/ralink/mt7620.c
+++ b/arch/mips/ralink/mt7620.c
@@ -639,7 +639,7 @@ mt7628_dram_init(struct ralink_soc_info *soc_info)
}
 }
 
-void prom_soc_init(struct ralink_soc_info *soc_info)
+void __init prom_soc_init(struct ralink_soc_info *soc_info)
 {
void __iomem *sysc = (void __iomem *) KSEG1ADDR(MT7620_SYSC_BASE);
unsigned char *name = NULL;
diff --git a/arch/mips/ralink/mt7621.c b/arch/mips/ralink/mt7621.c
index ca0ac607b0f3..ec87ce561049 100644
--- a/arch/mips/ralink/mt7621.c
+++ b/arch/mips/ralink/mt7621.c
@@ -146,7 +146,7 @@ static void soc_dev_init(struct ralink_soc_info *soc_info, 
u32 rev)
}
 }
 
-void prom_soc_init(struct ralink_soc_info *soc_info)
+void __init prom_soc_init(struct ralink_soc_info *soc_info)
 {
void __iomem *sysc = (void __iomem *) KSEG1ADDR(MT7621_SYSC_BASE);
unsigned char *name = NULL;
diff --git a/arch/mips/ralink/rt288x.c b/arch/mips/ralink/rt288x.c
index 3f096897858c..34083c70ec68 100644
--- a/arch/mips/ralink/rt288x.c
+++ b/arch/mips/ralink/rt288x.c
@@ -77,7 +77,7 @@ void __init ralink_of_remap(void)
panic("Failed to remap core resources");
 }
 
-void prom_soc_init(struct ralink_soc_info *soc_info)
+void __init prom_soc_init(struct ralink_soc_info *soc_info)
 {
void __iomem *sysc = (void __iomem *) KSEG1ADDR(RT2880_SYSC_BASE);
const char *name;
diff --git a/arch/mips/ralink/rt305x.c b/arch/mips/ralink/rt305x.c
index 496f966c05f9..c5b63c142705 100644
--- a/arch/mips/ralink/rt305x.c
+++ b/arch/mips/ralink/rt305x.c
@@ -214,7 +214,7 @@ void __init ralink_of_remap(void)
panic("Failed to remap core resources");
 }
 
-void prom_soc_init(struct ralink_soc_info *soc_info)
+void __init prom_soc_init(struct ralink_soc_info *soc_info)
 {
void __iomem *sysc = (void __iomem *) KSEG1ADDR(RT305X_SYSC_BASE);
unsigned char *name;
diff --git a/arch/mips/ralink/rt3883.c b/arch/mips/ralink/rt3883.c
index 8f3fe3106708..ff91f3531ad0 100644
--- a/arch/mips/ralink/rt3883.c
+++ b/arch/mips/ralink/rt3883.c
@@ -113,7 +113,7 @@ void __init ralink_of_remap(void)
panic("Failed to remap core resources");
 }
 
-void prom_soc_init(struct ralink_soc_info *soc_info)
+void __init prom_soc_init(struct ralink_soc_info *soc_info)
 {
void __iomem *sysc = (void __iomem *) KSEG1ADDR(RT3883_SYSC_BASE);
const char *name;
-- 
2.31.0



Re: [PATCH] MIPS: ralink: mt7621: add memory detection support

2021-03-25 Thread Ilya Lipnitskiy
On Thu, Mar 25, 2021 at 3:01 AM Thomas Bogendoerfer
 wrote:
>
> On Tue, Mar 16, 2021 at 10:59:02PM -0700, Ilya Lipnitskiy wrote:
> > From: Chuanhong Guo 
> >
> > mt7621 has the following memory map:
> > 0x0-0x1c00: lower 448m memory
> > 0x1c00-0x200: peripheral registers
> > 0x2000-0x240: higher 64m memory
> >
> > detect_memory_region in arch/mips/kernel/setup.c only adds the first
> > memory region and isn't suitable for 512m memory detection because
> > it may accidentally read the memory area for peripheral registers.
> >
> > This commit adds memory detection capability for mt7621:
> >   1. Add the highmem area when 512m is detected.
> >   2. Guard memcmp from accessing peripheral registers:
> >  This only happens when a user decided to change kernel load address
> >  to 256m or higher address. Since this is a quite unusual case, we
> >  just skip 512m testing and return 256m as memory size.
> >
> > [...]
>
> I get
>
> WARNING: modpost: vmlinux.o(.text+0x132c): Section mismatch in reference from 
> the function prom_soc_init() to the function .init.text:mt7621_memory_detect()
> The function prom_soc_init() references
> the function __init mt7621_memory_detect().
> This is often because prom_soc_init lacks a __init
> annotation or the annotation of mt7621_memory_detect is wrong.
>
> Can you please fix this ?
Thanks, I will fix it. Having trouble reproducing the error, but I
clearly see the issue. Are you building on a MIPS target or
cross-compiling (I'm cross-compiling and no errors).

Ilya


Re: [PATCH net-next,v2 1/3] net: dsa: mt7530: setup core clock even in TRGMII mode

2021-03-23 Thread Ilya Lipnitskiy
On Tue, Mar 23, 2021 at 6:33 PM Ilya Lipnitskiy
 wrote:
>
> On Thu, Mar 11, 2021 at 9:41 AM Andrew Lunn  wrote:
> >
> > On Wed, Mar 10, 2021 at 06:09:52PM -0800, Ilya Lipnitskiy wrote:
> > > A recent change to MIPS ralink reset logic made it so mt7530 actually
> > > resets the switch on platforms such as mt7621 (where bit 2 is the reset
> > > line for the switch). That exposed an issue where the switch would not
> > > function properly in TRGMII mode after a reset.
> > >
> > > Reconfigure core clock in TRGMII mode to fix the issue.
> > >
> > > Tested on Ubiquiti ER-X (MT7621) with TRGMII mode enabled.
> >
> > Please don't submit the same patch to net and net-next.  Anything
> > which is accepted into net, will get merged into net-next about a week
> > later. If your other two patches depend on this patch, you need to
> > wait for the merge to happen, then submit them.
> I don't mind waiting, but it's been more than a week now. When is the
> next merge of net-next into net planned to happen?
Oops, I meant net into net-next...

Ilya


Re: [PATCH net-next,v2 1/3] net: dsa: mt7530: setup core clock even in TRGMII mode

2021-03-23 Thread Ilya Lipnitskiy
On Thu, Mar 11, 2021 at 9:41 AM Andrew Lunn  wrote:
>
> On Wed, Mar 10, 2021 at 06:09:52PM -0800, Ilya Lipnitskiy wrote:
> > A recent change to MIPS ralink reset logic made it so mt7530 actually
> > resets the switch on platforms such as mt7621 (where bit 2 is the reset
> > line for the switch). That exposed an issue where the switch would not
> > function properly in TRGMII mode after a reset.
> >
> > Reconfigure core clock in TRGMII mode to fix the issue.
> >
> > Tested on Ubiquiti ER-X (MT7621) with TRGMII mode enabled.
>
> Please don't submit the same patch to net and net-next.  Anything
> which is accepted into net, will get merged into net-next about a week
> later. If your other two patches depend on this patch, you need to
> wait for the merge to happen, then submit them.
I don't mind waiting, but it's been more than a week now. When is the
next merge of net-next into net planned to happen?

Ilya


[PATCH] MIPS: ralink: mt7621: add memory detection support

2021-03-17 Thread Ilya Lipnitskiy
From: Chuanhong Guo 

mt7621 has the following memory map:
0x0-0x1c00: lower 448m memory
0x1c00-0x200: peripheral registers
0x2000-0x240: higher 64m memory

detect_memory_region in arch/mips/kernel/setup.c only adds the first
memory region and isn't suitable for 512m memory detection because
it may accidentally read the memory area for peripheral registers.

This commit adds memory detection capability for mt7621:
  1. Add the highmem area when 512m is detected.
  2. Guard memcmp from accessing peripheral registers:
 This only happens when a user decided to change kernel load address
 to 256m or higher address. Since this is a quite unusual case, we
 just skip 512m testing and return 256m as memory size.

Signed-off-by: Chuanhong Guo 
[Minor commit message reword, make mt7621_memory_detect static]
Signed-off-by: Ilya Lipnitskiy 
---
 arch/mips/include/asm/mach-ralink/mt7621.h |  7 +++---
 arch/mips/ralink/common.h  |  1 +
 arch/mips/ralink/mt7621.c  | 29 +++---
 arch/mips/ralink/of.c  |  2 ++
 4 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/arch/mips/include/asm/mach-ralink/mt7621.h 
b/arch/mips/include/asm/mach-ralink/mt7621.h
index e1af1ba50bd8..6bbf082dd149 100644
--- a/arch/mips/include/asm/mach-ralink/mt7621.h
+++ b/arch/mips/include/asm/mach-ralink/mt7621.h
@@ -24,9 +24,10 @@
 #define CHIP_REV_VER_SHIFT 8
 #define CHIP_REV_ECO_MASK  0xf
 
-#define MT7621_DRAM_BASE0x0
-#define MT7621_DDR2_SIZE_MIN   32
-#define MT7621_DDR2_SIZE_MAX   256
+#define MT7621_LOWMEM_BASE 0x0
+#define MT7621_LOWMEM_MAX_SIZE 0x1C00
+#define MT7621_HIGHMEM_BASE0x2000
+#define MT7621_HIGHMEM_SIZE0x400
 
 #define MT7621_CHIP_NAME0  0x3637544D
 #define MT7621_CHIP_NAME1  0x20203132
diff --git a/arch/mips/ralink/common.h b/arch/mips/ralink/common.h
index 4bc65b7a3241..113dca5ac129 100644
--- a/arch/mips/ralink/common.h
+++ b/arch/mips/ralink/common.h
@@ -17,6 +17,7 @@ struct ralink_soc_info {
unsigned long mem_size;
unsigned long mem_size_min;
unsigned long mem_size_max;
+   void (*mem_detect)(void);
 };
 extern struct ralink_soc_info soc_info;
 
diff --git a/arch/mips/ralink/mt7621.c b/arch/mips/ralink/mt7621.c
index ca0ac607b0f3..d6616b0ad610 100644
--- a/arch/mips/ralink/mt7621.c
+++ b/arch/mips/ralink/mt7621.c
@@ -9,7 +9,9 @@
 #include 
 #include 
 #include 
+#include 
 
+#include 
 #include 
 #include 
 #include 
@@ -49,6 +51,8 @@
 #define MT7621_GPIO_MODE_SDHCI_SHIFT   18
 #define MT7621_GPIO_MODE_SDHCI_GPIO1
 
+static void *detect_magic __initdata = detect_memory_region;
+
 static struct rt2880_pmx_func uart1_grp[] =  { FUNC("uart1", 0, 1, 2) };
 static struct rt2880_pmx_func i2c_grp[] =  { FUNC("i2c", 0, 3, 2) };
 static struct rt2880_pmx_func uart3_grp[] = {
@@ -110,6 +114,26 @@ phys_addr_t mips_cpc_default_phys_base(void)
panic("Cannot detect cpc address");
 }
 
+static void __init mt7621_memory_detect(void)
+{
+   void *dm = _magic;
+   phys_addr_t size;
+
+   for (size = 32 * SZ_1M; size < 256 * SZ_1M; size <<= 1) {
+   if (!__builtin_memcmp(dm, dm + size, sizeof(detect_magic)))
+   break;
+   }
+
+   if ((size == 256 * SZ_1M) &&
+   (CPHYSADDR(dm + size) < MT7621_LOWMEM_MAX_SIZE) &&
+   __builtin_memcmp(dm, dm + size, sizeof(detect_magic))) {
+   memblock_add(MT7621_LOWMEM_BASE, MT7621_LOWMEM_MAX_SIZE);
+   memblock_add(MT7621_HIGHMEM_BASE, MT7621_HIGHMEM_SIZE);
+   } else {
+   memblock_add(MT7621_LOWMEM_BASE, size);
+   }
+}
+
 void __init ralink_of_remap(void)
 {
rt_sysc_membase = plat_of_remap_node("mtk,mt7621-sysc");
@@ -194,10 +218,7 @@ void prom_soc_init(struct ralink_soc_info *soc_info)
(rev >> CHIP_REV_VER_SHIFT) & CHIP_REV_VER_MASK,
(rev & CHIP_REV_ECO_MASK));
 
-   soc_info->mem_size_min = MT7621_DDR2_SIZE_MIN;
-   soc_info->mem_size_max = MT7621_DDR2_SIZE_MAX;
-   soc_info->mem_base = MT7621_DRAM_BASE;
-
+   soc_info->mem_detect = mt7621_memory_detect;
rt2880_pinmux_data = mt7621_pinmux_data;
 
soc_dev_init(soc_info, rev);
diff --git a/arch/mips/ralink/of.c b/arch/mips/ralink/of.c
index 8286c3521476..0c5de07da097 100644
--- a/arch/mips/ralink/of.c
+++ b/arch/mips/ralink/of.c
@@ -78,6 +78,8 @@ void __init plat_mem_setup(void)
of_scan_flat_dt(early_init_dt_find_memory, NULL);
if (memory_dtb)
of_scan_flat_dt(early_init_dt_scan_memory, NULL);
+   else if (soc_info.mem_detect)
+   soc_info.mem_detect();
else if (soc_info.mem_size)
memblock_add(soc_info.mem_base, soc_info.mem_size * SZ_1M);
else
-- 
2.31.0



Re: [PATCH] MIPS: fix memory reservation for non-usermem setups

2021-03-16 Thread Ilya Lipnitskiy
Hi Thomas,

On Fri, Mar 12, 2021 at 7:19 AM Thomas Bogendoerfer
 wrote:
>
> On Sun, Mar 07, 2021 at 11:40:30AM -0800, Ilya Lipnitskiy wrote:
> > From: Tobias Wolf 
> >
> > Commit 67a3ba25aa95 ("MIPS: Fix incorrect mem=X@Y handling") introduced a 
> > new
> > issue for rt288x where "PHYS_OFFSET" is 0x0 but the calculated "ramstart" is
> > not. As the prerequisite of custom memory map has been removed, this results
> > in the full memory range of 0x0 - 0x800 to be marked as reserved for 
> > this
> > platform.
>
> and where is the problem here ?
Turns out this was already attempted to be upstreamed - not clear why
it wasn't merged. Context:
https://lore.kernel.org/linux-mips/6504517.U6H5IhoIOn@loki/

I hope the thread above helps you understand the problem.

>
> Thomas.
>
> --
> Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
> good idea.[ RFC1925, 2.3 ]
Ilya


Re: [PATCH] MIPS: ralink: select WEAK_REORDERING_BEYOND_LLSC on MT7621

2021-03-15 Thread Ilya Lipnitskiy
Hi Thomas,

On Fri, Mar 12, 2021 at 2:34 AM Thomas Bogendoerfer
 wrote:
>
> On Sun, Mar 07, 2021 at 11:08:06AM -0800, Ilya Lipnitskiy wrote:
> > Upstream a long-standing OpenWrt patch for RALINK MT7621 SoC. Selecting
> > WEAK_REORDERING_BEYOND_LLSC fixes random kernel hangs. This bug and fix
> > was reported by MediaTek WCN division [0].
> >
> > [0]: 
> > https://lists.infradead.org/pipermail/lede-commits/2017-August/004537.html
>
> I don't see the Mediatek WCN reporting there. Looking at the 1004K user
> manual I couldn't find a notice about such behaviour. So this looks
> more like a fix for papering over the real bug by just introducing
> a few more syncs. Could you please point me to where this is
> really documented ?
You may actually be right. After doing some testing we may not need
this change after all. Let me drop this patch for now. FYI, the
following commit may have fixed LL SC issues this change was covering
up: https://lore.kernel.org/lkml/20190613134933.048961...@infradead.org/

>
> Thomas.
>
> --
> Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
> good idea.[ RFC1925, 2.3 ]
Ilya


[PATCH net,v2] net: dsa: mt7530: setup core clock even in TRGMII mode

2021-03-12 Thread Ilya Lipnitskiy
A recent change to MIPS ralink reset logic made it so mt7530 actually
resets the switch on platforms such as mt7621 (where bit 2 is the reset
line for the switch). That exposed an issue where the switch would not
function properly in TRGMII mode after a reset.

Reconfigure core clock in TRGMII mode to fix the issue.

Tested on Ubiquiti ER-X (MT7621) with TRGMII mode enabled.

Fixes: 3f9ef7785a9c ("MIPS: ralink: manage low reset lines")
Signed-off-by: Ilya Lipnitskiy 
---
 drivers/net/dsa/mt7530.c | 52 +++-
 1 file changed, 25 insertions(+), 27 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index f06f5fa2f898..9871d7cff93a 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -436,34 +436,32 @@ mt7530_pad_clk_setup(struct dsa_switch *ds, 
phy_interface_t interface)
 TD_DM_DRVP(8) | TD_DM_DRVN(8));
 
/* Setup core clock for MT7530 */
-   if (!trgint) {
-   /* Disable MT7530 core clock */
-   core_clear(priv, CORE_TRGMII_GSW_CLK_CG, REG_GSWCK_EN);
-
-   /* Disable PLL, since phy_device has not yet been created
-* provided for phy_[read,write]_mmd_indirect is called, we
-* provide our own core_write_mmd_indirect to complete this
-* function.
-*/
-   core_write_mmd_indirect(priv,
-   CORE_GSWPLL_GRP1,
-   MDIO_MMD_VEND2,
-   0);
-
-   /* Set core clock into 500Mhz */
-   core_write(priv, CORE_GSWPLL_GRP2,
-  RG_GSWPLL_POSDIV_500M(1) |
-  RG_GSWPLL_FBKDIV_500M(25));
+   /* Disable MT7530 core clock */
+   core_clear(priv, CORE_TRGMII_GSW_CLK_CG, REG_GSWCK_EN);
 
-   /* Enable PLL */
-   core_write(priv, CORE_GSWPLL_GRP1,
-  RG_GSWPLL_EN_PRE |
-  RG_GSWPLL_POSDIV_200M(2) |
-  RG_GSWPLL_FBKDIV_200M(32));
-
-   /* Enable MT7530 core clock */
-   core_set(priv, CORE_TRGMII_GSW_CLK_CG, REG_GSWCK_EN);
-   }
+   /* Disable PLL, since phy_device has not yet been created
+* provided for phy_[read,write]_mmd_indirect is called, we
+* provide our own core_write_mmd_indirect to complete this
+* function.
+*/
+   core_write_mmd_indirect(priv,
+   CORE_GSWPLL_GRP1,
+   MDIO_MMD_VEND2,
+   0);
+
+   /* Set core clock into 500Mhz */
+   core_write(priv, CORE_GSWPLL_GRP2,
+  RG_GSWPLL_POSDIV_500M(1) |
+  RG_GSWPLL_FBKDIV_500M(25));
+
+   /* Enable PLL */
+   core_write(priv, CORE_GSWPLL_GRP1,
+  RG_GSWPLL_EN_PRE |
+  RG_GSWPLL_POSDIV_200M(2) |
+  RG_GSWPLL_FBKDIV_200M(32));
+
+   /* Enable MT7530 core clock */
+   core_set(priv, CORE_TRGMII_GSW_CLK_CG, REG_GSWCK_EN);
 
/* Setup the MT7530 TRGMII Tx Clock */
core_set(priv, CORE_TRGMII_GSW_CLK_CG, REG_GSWCK_EN);
-- 
2.30.2



Re: [PATCH 3/3] net: dsa: mt7530: setup core clock even in TRGMII mode

2021-03-10 Thread Ilya Lipnitskiy
Hi Florian,

On Wed, Mar 10, 2021 at 7:20 PM Florian Fainelli  wrote:
>
>
>
> On 3/10/2021 7:17 PM, Ilya Lipnitskiy wrote:
> > Hi Vladimir,
> >
> > On Wed, Mar 10, 2021 at 3:10 PM Vladimir Oltean  wrote:
> >>
> >> Hello Ilya,
> >>
> >> On Wed, Mar 10, 2021 at 01:14:20PM -0800, Ilya Lipnitskiy wrote:
> >>> 3f9ef7785a9c ("MIPS: ralink: manage low reset lines") made it so mt7530
> >>> actually resets the switch on platforms such as mt7621 (where bit 2 is
> >>> the reset line for the switch). That exposed an issue where the switch
> >>> would not function properly in TRGMII mode after a reset.
> >>>
> >>> Reconfigure core clock in TRGMII mode to fix the issue.
> >>>
> >>> Also, disable both core and TRGMII Tx clocks prior to reconfiguring.
> >>> Previously, only the core clock was disabled, but not TRGMII Tx clock.
> >>>
> >>> Tested on Ubiquity ER-X (MT7621) with TRGMII mode enabled.
> >>>
> >>> Signed-off-by: Ilya Lipnitskiy 
> >>> ---
> >>
> >> For the networking subsystem there are two git trees, "net" for bugfixes
> >> and "net-next" for new features, and we specify the target tree using
> >> git send-email --subject-prefix="PATCH net-next".
> >>
> >> I assume you would like the v5.12 kernel to actually be functional on
> >> the Ubiquiti ER-X switch, so I would recommend keeping this patch
> >> minimal and splitting it out from the current series, and targeting it
> >> towards the "net" tree, which will eventually get merged into one of the
> >> v5.12 rc's and then into the final version. The other patches won't go
> >> into v5.12 but into v5.13, hence the "next" name.
> > I thought I figured it out - now I'm confused. Can you explain why
> > https://patchwork.kernel.org/project/netdevbpf/patch/20210311012108.7190-1-ilya.lipnits...@gmail.com/
> > is marked as supeseded?
>
> That looks like a mistake on the maintainer side, I do not believe that
> patch should be Superseded since you just submitted it.
Thanks for taking a look. I thought maybe I did something wrong with
submitting the same patch to net and net-next, but the net-next series
(https://patchwork.kernel.org/project/netdevbpf/patch/20210311020954.842341-1-ilya.lipnits...@gmail.com/)
depends on it, so the way I did it made the most sense for me. Let me
know if I did something wrong.
> --
> Florian


Re: [PATCH 3/3] net: dsa: mt7530: setup core clock even in TRGMII mode

2021-03-10 Thread Ilya Lipnitskiy
Hi Vladimir,

On Wed, Mar 10, 2021 at 3:10 PM Vladimir Oltean  wrote:
>
> Hello Ilya,
>
> On Wed, Mar 10, 2021 at 01:14:20PM -0800, Ilya Lipnitskiy wrote:
> > 3f9ef7785a9c ("MIPS: ralink: manage low reset lines") made it so mt7530
> > actually resets the switch on platforms such as mt7621 (where bit 2 is
> > the reset line for the switch). That exposed an issue where the switch
> > would not function properly in TRGMII mode after a reset.
> >
> > Reconfigure core clock in TRGMII mode to fix the issue.
> >
> > Also, disable both core and TRGMII Tx clocks prior to reconfiguring.
> > Previously, only the core clock was disabled, but not TRGMII Tx clock.
> >
> > Tested on Ubiquity ER-X (MT7621) with TRGMII mode enabled.
> >
> > Signed-off-by: Ilya Lipnitskiy 
> > ---
>
> For the networking subsystem there are two git trees, "net" for bugfixes
> and "net-next" for new features, and we specify the target tree using
> git send-email --subject-prefix="PATCH net-next".
>
> I assume you would like the v5.12 kernel to actually be functional on
> the Ubiquiti ER-X switch, so I would recommend keeping this patch
> minimal and splitting it out from the current series, and targeting it
> towards the "net" tree, which will eventually get merged into one of the
> v5.12 rc's and then into the final version. The other patches won't go
> into v5.12 but into v5.13, hence the "next" name.
I thought I figured it out - now I'm confused. Can you explain why
https://patchwork.kernel.org/project/netdevbpf/patch/20210311012108.7190-1-ilya.lipnits...@gmail.com/
is marked as supeseded?
>
> Also add these lines in your .gitconfig:
>
> [core]
> abbrev = 12
> [pretty]
> fixes = Fixes: %h (\"%s\")
>
> and run:
>
> git show 3f9ef7785a9c --pretty=fixes
> Fixes: 3f9ef7785a9c ("MIPS: ralink: manage low reset lines")
>
> and paste that "Fixes:" line in the commit message, right above your
> Signed-off-by: tag.


Re: [PATCH 3/3] net: dsa: mt7530: setup core clock even in TRGMII mode

2021-03-10 Thread Ilya Lipnitskiy
Hi Vladimir,

On Wed, Mar 10, 2021 at 3:10 PM Vladimir Oltean  wrote:
>
> Hello Ilya,
>
> On Wed, Mar 10, 2021 at 01:14:20PM -0800, Ilya Lipnitskiy wrote:
> > 3f9ef7785a9c ("MIPS: ralink: manage low reset lines") made it so mt7530
> > actually resets the switch on platforms such as mt7621 (where bit 2 is
> > the reset line for the switch). That exposed an issue where the switch
> > would not function properly in TRGMII mode after a reset.
> >
> > Reconfigure core clock in TRGMII mode to fix the issue.
> >
> > Also, disable both core and TRGMII Tx clocks prior to reconfiguring.
> > Previously, only the core clock was disabled, but not TRGMII Tx clock.
> >
> > Tested on Ubiquity ER-X (MT7621) with TRGMII mode enabled.
> >
> > Signed-off-by: Ilya Lipnitskiy 
> > ---
>
> For the networking subsystem there are two git trees, "net" for bugfixes
> and "net-next" for new features, and we specify the target tree using
> git send-email --subject-prefix="PATCH net-next".
>
> I assume you would like the v5.12 kernel to actually be functional on
> the Ubiquiti ER-X switch, so I would recommend keeping this patch
> minimal and splitting it out from the current series, and targeting it
> towards the "net" tree, which will eventually get merged into one of the
> v5.12 rc's and then into the final version. The other patches won't go
> into v5.12 but into v5.13, hence the "next" name.
I think I knew this, but didn't think it through. Thanks for your
guidance. I have submitted a single patch to "net" and that same patch
and two more to "net-next" - hopefully that looks better, but I'm sure
I have more to learn still.

>
> Also add these lines in your .gitconfig:
>
> [core]
> abbrev = 12
> [pretty]
> fixes = Fixes: %h (\"%s\")
>
> and run:
>
> git show 3f9ef7785a9c --pretty=fixes
> Fixes: 3f9ef7785a9c ("MIPS: ralink: manage low reset lines")
>
> and paste that "Fixes:" line in the commit message, right above your
> Signed-off-by: tag.


[PATCH net-next,v2 2/3] net: dsa: mt7530: clean up redundant clock enables

2021-03-10 Thread Ilya Lipnitskiy
Two minor changes:

- In RGMII mode, the REG_GSWCK_EN bit of CORE_TRGMII_GSW_CLK_CG gets
  set three times in a row. In TRGMII mode, two times. Simplify the code
  and only set it once for both modes.

- When disabling PLL, there is no need to call core_write_mmd_indirect
  directly, use the core_write wrapper instead like the rest of the code
  in the function does. This change helps with consistency and
  readability.

Signed-off-by: Ilya Lipnitskiy 
---
 drivers/net/dsa/mt7530.c | 11 +++
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 9871d7cff93a..80a35caf920e 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -444,10 +444,7 @@ mt7530_pad_clk_setup(struct dsa_switch *ds, 
phy_interface_t interface)
 * provide our own core_write_mmd_indirect to complete this
 * function.
 */
-   core_write_mmd_indirect(priv,
-   CORE_GSWPLL_GRP1,
-   MDIO_MMD_VEND2,
-   0);
+   core_write(priv, CORE_GSWPLL_GRP1, 0);
 
/* Set core clock into 500Mhz */
core_write(priv, CORE_GSWPLL_GRP2,
@@ -460,11 +457,7 @@ mt7530_pad_clk_setup(struct dsa_switch *ds, 
phy_interface_t interface)
   RG_GSWPLL_POSDIV_200M(2) |
   RG_GSWPLL_FBKDIV_200M(32));
 
-   /* Enable MT7530 core clock */
-   core_set(priv, CORE_TRGMII_GSW_CLK_CG, REG_GSWCK_EN);
-
/* Setup the MT7530 TRGMII Tx Clock */
-   core_set(priv, CORE_TRGMII_GSW_CLK_CG, REG_GSWCK_EN);
core_write(priv, CORE_PLL_GROUP5, RG_LCDDS_PCW_NCPO1(ncpo1));
core_write(priv, CORE_PLL_GROUP6, RG_LCDDS_PCW_NCPO0(0));
core_write(priv, CORE_PLL_GROUP10, RG_LCDDS_SSC_DELTA(ssc_delta));
@@ -478,6 +471,8 @@ mt7530_pad_clk_setup(struct dsa_switch *ds, phy_interface_t 
interface)
core_write(priv, CORE_PLL_GROUP7,
   RG_LCDDS_PCW_NCPO_CHG | RG_LCCDS_C(3) |
   RG_LCDDS_PWDB | RG_LCDDS_ISO_EN);
+
+   /* Enable MT7530 core and TRGMII Tx clocks */
core_set(priv, CORE_TRGMII_GSW_CLK_CG,
 REG_GSWCK_EN | REG_TRGMIICK_EN);
 
-- 
2.30.2



[PATCH net-next,v2 1/3] net: dsa: mt7530: setup core clock even in TRGMII mode

2021-03-10 Thread Ilya Lipnitskiy
A recent change to MIPS ralink reset logic made it so mt7530 actually
resets the switch on platforms such as mt7621 (where bit 2 is the reset
line for the switch). That exposed an issue where the switch would not
function properly in TRGMII mode after a reset.

Reconfigure core clock in TRGMII mode to fix the issue.

Tested on Ubiquiti ER-X (MT7621) with TRGMII mode enabled.

Fixes: 3f9ef7785a9c ("MIPS: ralink: manage low reset lines")
Signed-off-by: Ilya Lipnitskiy 
---
 drivers/net/dsa/mt7530.c | 52 +++-
 1 file changed, 25 insertions(+), 27 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index f06f5fa2f898..9871d7cff93a 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -436,34 +436,32 @@ mt7530_pad_clk_setup(struct dsa_switch *ds, 
phy_interface_t interface)
 TD_DM_DRVP(8) | TD_DM_DRVN(8));
 
/* Setup core clock for MT7530 */
-   if (!trgint) {
-   /* Disable MT7530 core clock */
-   core_clear(priv, CORE_TRGMII_GSW_CLK_CG, REG_GSWCK_EN);
-
-   /* Disable PLL, since phy_device has not yet been created
-* provided for phy_[read,write]_mmd_indirect is called, we
-* provide our own core_write_mmd_indirect to complete this
-* function.
-*/
-   core_write_mmd_indirect(priv,
-   CORE_GSWPLL_GRP1,
-   MDIO_MMD_VEND2,
-   0);
-
-   /* Set core clock into 500Mhz */
-   core_write(priv, CORE_GSWPLL_GRP2,
-  RG_GSWPLL_POSDIV_500M(1) |
-  RG_GSWPLL_FBKDIV_500M(25));
+   /* Disable MT7530 core clock */
+   core_clear(priv, CORE_TRGMII_GSW_CLK_CG, REG_GSWCK_EN);
 
-   /* Enable PLL */
-   core_write(priv, CORE_GSWPLL_GRP1,
-  RG_GSWPLL_EN_PRE |
-  RG_GSWPLL_POSDIV_200M(2) |
-  RG_GSWPLL_FBKDIV_200M(32));
-
-   /* Enable MT7530 core clock */
-   core_set(priv, CORE_TRGMII_GSW_CLK_CG, REG_GSWCK_EN);
-   }
+   /* Disable PLL, since phy_device has not yet been created
+* provided for phy_[read,write]_mmd_indirect is called, we
+* provide our own core_write_mmd_indirect to complete this
+* function.
+*/
+   core_write_mmd_indirect(priv,
+   CORE_GSWPLL_GRP1,
+   MDIO_MMD_VEND2,
+   0);
+
+   /* Set core clock into 500Mhz */
+   core_write(priv, CORE_GSWPLL_GRP2,
+  RG_GSWPLL_POSDIV_500M(1) |
+  RG_GSWPLL_FBKDIV_500M(25));
+
+   /* Enable PLL */
+   core_write(priv, CORE_GSWPLL_GRP1,
+  RG_GSWPLL_EN_PRE |
+  RG_GSWPLL_POSDIV_200M(2) |
+  RG_GSWPLL_FBKDIV_200M(32));
+
+   /* Enable MT7530 core clock */
+   core_set(priv, CORE_TRGMII_GSW_CLK_CG, REG_GSWCK_EN);
 
/* Setup the MT7530 TRGMII Tx Clock */
core_set(priv, CORE_TRGMII_GSW_CLK_CG, REG_GSWCK_EN);
-- 
2.30.2



[PATCH net-next,v2 3/3] net: dsa: mt7530: disable TRGMII clock at reconfigure

2021-03-10 Thread Ilya Lipnitskiy
Disable both core and TRGMII Tx clocks prior to reconfiguring.
Previously, only the core clock was disabled, but not TRGMII Tx clock.
So disable both, then configure them, then re-enable both, for
consistency.

Reword the comment about core_write_mmd_indirect for clarity.

Tested on Ubiquiti ER-X running the GMAC and MT7530 in TRGMII mode.

Signed-off-by: Ilya Lipnitskiy 
---
 drivers/net/dsa/mt7530.c | 19 +++
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 80a35caf920e..7ef5e7c23e05 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -435,15 +435,18 @@ mt7530_pad_clk_setup(struct dsa_switch *ds, 
phy_interface_t interface)
mt7530_write(priv, MT7530_TRGMII_TD_ODT(i),
 TD_DM_DRVP(8) | TD_DM_DRVN(8));
 
-   /* Setup core clock for MT7530 */
-   /* Disable MT7530 core clock */
-   core_clear(priv, CORE_TRGMII_GSW_CLK_CG, REG_GSWCK_EN);
-
-   /* Disable PLL, since phy_device has not yet been created
-* provided for phy_[read,write]_mmd_indirect is called, we
-* provide our own core_write_mmd_indirect to complete this
-* function.
+   /* Since phy_device has not yet been created and
+* phy_[read,write]_mmd_indirect is not available, we provide our own
+* core_write_mmd_indirect with core_{clear,write,set} wrappers to
+* complete this function.
 */
+
+   /* Disable MT7530 core and TRGMII Tx clocks */
+   core_clear(priv, CORE_TRGMII_GSW_CLK_CG,
+  REG_GSWCK_EN | REG_TRGMIICK_EN);
+
+   /* Setup core clock for MT7530 */
+   /* Disable PLL */
core_write(priv, CORE_GSWPLL_GRP1, 0);
 
/* Set core clock into 500Mhz */
-- 
2.30.2



[PATCH net] net: dsa: mt7530: setup core clock even in TRGMII mode

2021-03-10 Thread Ilya Lipnitskiy
A recent change to MIPS ralink reset logic made it so mt7530 actually
resets the switch on platforms such as mt7621 (where bit 2 is the reset
line for the switch). That exposed an issue where the switch would not
function properly in TRGMII mode after a reset.

Reconfigure core clock in TRGMII mode to fix the issue.

Tested on Ubiquiti ER-X (MT7621) with TRGMII mode enabled.

Fixes: 3f9ef7785a9c ("MIPS: ralink: manage low reset lines")
Signed-off-by: Ilya Lipnitskiy 
---
 drivers/net/dsa/mt7530.c | 52 +++-
 1 file changed, 25 insertions(+), 27 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index f06f5fa2f898..9871d7cff93a 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -436,34 +436,32 @@ mt7530_pad_clk_setup(struct dsa_switch *ds, 
phy_interface_t interface)
 TD_DM_DRVP(8) | TD_DM_DRVN(8));
 
/* Setup core clock for MT7530 */
-   if (!trgint) {
-   /* Disable MT7530 core clock */
-   core_clear(priv, CORE_TRGMII_GSW_CLK_CG, REG_GSWCK_EN);
-
-   /* Disable PLL, since phy_device has not yet been created
-* provided for phy_[read,write]_mmd_indirect is called, we
-* provide our own core_write_mmd_indirect to complete this
-* function.
-*/
-   core_write_mmd_indirect(priv,
-   CORE_GSWPLL_GRP1,
-   MDIO_MMD_VEND2,
-   0);
-
-   /* Set core clock into 500Mhz */
-   core_write(priv, CORE_GSWPLL_GRP2,
-  RG_GSWPLL_POSDIV_500M(1) |
-  RG_GSWPLL_FBKDIV_500M(25));
+   /* Disable MT7530 core clock */
+   core_clear(priv, CORE_TRGMII_GSW_CLK_CG, REG_GSWCK_EN);
 
-   /* Enable PLL */
-   core_write(priv, CORE_GSWPLL_GRP1,
-  RG_GSWPLL_EN_PRE |
-  RG_GSWPLL_POSDIV_200M(2) |
-  RG_GSWPLL_FBKDIV_200M(32));
-
-   /* Enable MT7530 core clock */
-   core_set(priv, CORE_TRGMII_GSW_CLK_CG, REG_GSWCK_EN);
-   }
+   /* Disable PLL, since phy_device has not yet been created
+* provided for phy_[read,write]_mmd_indirect is called, we
+* provide our own core_write_mmd_indirect to complete this
+* function.
+*/
+   core_write_mmd_indirect(priv,
+   CORE_GSWPLL_GRP1,
+   MDIO_MMD_VEND2,
+   0);
+
+   /* Set core clock into 500Mhz */
+   core_write(priv, CORE_GSWPLL_GRP2,
+  RG_GSWPLL_POSDIV_500M(1) |
+  RG_GSWPLL_FBKDIV_500M(25));
+
+   /* Enable PLL */
+   core_write(priv, CORE_GSWPLL_GRP1,
+  RG_GSWPLL_EN_PRE |
+  RG_GSWPLL_POSDIV_200M(2) |
+  RG_GSWPLL_FBKDIV_200M(32));
+
+   /* Enable MT7530 core clock */
+   core_set(priv, CORE_TRGMII_GSW_CLK_CG, REG_GSWCK_EN);
 
/* Setup the MT7530 TRGMII Tx Clock */
core_set(priv, CORE_TRGMII_GSW_CLK_CG, REG_GSWCK_EN);
-- 
2.30.2



[PATCH 2/3] net: dsa: mt7530: use core_write wrapper

2021-03-10 Thread Ilya Lipnitskiy
When disabling PLL, there is no need to call core_write_mmd_indirect
directly, use the core_write wrapper instead like the rest of the code
in the function does. This change helps with consistency and
readability.

Signed-off-by: Ilya Lipnitskiy 
---
 drivers/net/dsa/mt7530.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index e785f80f966b..b106ea816778 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -445,10 +445,7 @@ mt7530_pad_clk_setup(struct dsa_switch *ds, 
phy_interface_t interface)
 * provide our own core_write_mmd_indirect to complete this
 * function.
 */
-   core_write_mmd_indirect(priv,
-   CORE_GSWPLL_GRP1,
-   MDIO_MMD_VEND2,
-   0);
+   core_write(priv, CORE_GSWPLL_GRP1, 0);
 
/* Set core clock into 500Mhz */
core_write(priv, CORE_GSWPLL_GRP2,
-- 
2.30.1



[PATCH 3/3] net: dsa: mt7530: setup core clock even in TRGMII mode

2021-03-10 Thread Ilya Lipnitskiy
3f9ef7785a9c ("MIPS: ralink: manage low reset lines") made it so mt7530
actually resets the switch on platforms such as mt7621 (where bit 2 is
the reset line for the switch). That exposed an issue where the switch
would not function properly in TRGMII mode after a reset.

Reconfigure core clock in TRGMII mode to fix the issue.

Also, disable both core and TRGMII Tx clocks prior to reconfiguring.
Previously, only the core clock was disabled, but not TRGMII Tx clock.

Tested on Ubiquity ER-X (MT7621) with TRGMII mode enabled.

Signed-off-by: Ilya Lipnitskiy 
---
 drivers/net/dsa/mt7530.c | 44 
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index b106ea816778..7ef5e7c23e05 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -435,30 +435,30 @@ mt7530_pad_clk_setup(struct dsa_switch *ds, 
phy_interface_t interface)
mt7530_write(priv, MT7530_TRGMII_TD_ODT(i),
 TD_DM_DRVP(8) | TD_DM_DRVN(8));
 
-   /* Setup core clock for MT7530 */
-   if (!trgint) {
-   /* Disable MT7530 core clock */
-   core_clear(priv, CORE_TRGMII_GSW_CLK_CG, REG_GSWCK_EN);
-
-   /* Disable PLL, since phy_device has not yet been created
-* provided for phy_[read,write]_mmd_indirect is called, we
-* provide our own core_write_mmd_indirect to complete this
-* function.
-*/
-   core_write(priv, CORE_GSWPLL_GRP1, 0);
-
-   /* Set core clock into 500Mhz */
-   core_write(priv, CORE_GSWPLL_GRP2,
-  RG_GSWPLL_POSDIV_500M(1) |
-  RG_GSWPLL_FBKDIV_500M(25));
+   /* Since phy_device has not yet been created and
+* phy_[read,write]_mmd_indirect is not available, we provide our own
+* core_write_mmd_indirect with core_{clear,write,set} wrappers to
+* complete this function.
+*/
 
-   /* Enable PLL */
-   core_write(priv, CORE_GSWPLL_GRP1,
-  RG_GSWPLL_EN_PRE |
-  RG_GSWPLL_POSDIV_200M(2) |
-  RG_GSWPLL_FBKDIV_200M(32));
+   /* Disable MT7530 core and TRGMII Tx clocks */
+   core_clear(priv, CORE_TRGMII_GSW_CLK_CG,
+  REG_GSWCK_EN | REG_TRGMIICK_EN);
 
-   }
+   /* Setup core clock for MT7530 */
+   /* Disable PLL */
+   core_write(priv, CORE_GSWPLL_GRP1, 0);
+
+   /* Set core clock into 500Mhz */
+   core_write(priv, CORE_GSWPLL_GRP2,
+  RG_GSWPLL_POSDIV_500M(1) |
+  RG_GSWPLL_FBKDIV_500M(25));
+
+   /* Enable PLL */
+   core_write(priv, CORE_GSWPLL_GRP1,
+  RG_GSWPLL_EN_PRE |
+  RG_GSWPLL_POSDIV_200M(2) |
+  RG_GSWPLL_FBKDIV_200M(32));
 
/* Setup the MT7530 TRGMII Tx Clock */
core_write(priv, CORE_PLL_GROUP5, RG_LCDDS_PCW_NCPO1(ncpo1));
-- 
2.30.1



[PATCH 1/3] net: dsa: mt7530: remove redundant clock enables

2021-03-10 Thread Ilya Lipnitskiy
In RGMII mode, the REG_GSWCK_EN bit of CORE_TRGMII_GSW_CLK_CG gets
set three times in a row. In TRGMII mode, two times. Simplify the code
and only set it once for both modes.

Signed-off-by: Ilya Lipnitskiy 
---
 drivers/net/dsa/mt7530.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index f06f5fa2f898..e785f80f966b 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -461,12 +461,9 @@ mt7530_pad_clk_setup(struct dsa_switch *ds, 
phy_interface_t interface)
   RG_GSWPLL_POSDIV_200M(2) |
   RG_GSWPLL_FBKDIV_200M(32));
 
-   /* Enable MT7530 core clock */
-   core_set(priv, CORE_TRGMII_GSW_CLK_CG, REG_GSWCK_EN);
}
 
/* Setup the MT7530 TRGMII Tx Clock */
-   core_set(priv, CORE_TRGMII_GSW_CLK_CG, REG_GSWCK_EN);
core_write(priv, CORE_PLL_GROUP5, RG_LCDDS_PCW_NCPO1(ncpo1));
core_write(priv, CORE_PLL_GROUP6, RG_LCDDS_PCW_NCPO0(0));
core_write(priv, CORE_PLL_GROUP10, RG_LCDDS_SSC_DELTA(ssc_delta));
@@ -480,6 +477,8 @@ mt7530_pad_clk_setup(struct dsa_switch *ds, phy_interface_t 
interface)
core_write(priv, CORE_PLL_GROUP7,
   RG_LCDDS_PCW_NCPO_CHG | RG_LCCDS_C(3) |
   RG_LCDDS_PWDB | RG_LCDDS_ISO_EN);
+
+   /* Enable MT7530 core and TRGMII Tx clocks */
core_set(priv, CORE_TRGMII_GSW_CLK_CG,
 REG_GSWCK_EN | REG_TRGMIICK_EN);
 
-- 
2.30.1



[PATCH] MIPS: fix memory reservation for non-usermem setups

2021-03-07 Thread Ilya Lipnitskiy
From: Tobias Wolf 

Commit 67a3ba25aa95 ("MIPS: Fix incorrect mem=X@Y handling") introduced a new
issue for rt288x where "PHYS_OFFSET" is 0x0 but the calculated "ramstart" is
not. As the prerequisite of custom memory map has been removed, this results
in the full memory range of 0x0 - 0x800 to be marked as reserved for this
platform.

This patch adds the originally intended prerequisite again.

This patch has been present in OpenWrt tree for over 2 years:
https://git.openwrt.org/?p=openwrt/openwrt.git;a=commit;h=93bfafb8dc209f153022796d9e747149e66cc29e

Fixes: 67a3ba25aa95 ("MIPS: Fix incorrect mem=X@Y handling")
Signed-off-by: Tobias Wolf 
[Reword commit message]
Signed-off-by: Ilya Lipnitskiy 
Cc: Marcin Nowakowski 
Cc: linux-m...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: sta...@vger.kernel.org
---
 arch/mips/kernel/setup.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
index 279be0153f8b..97e3a0db651b 100644
--- a/arch/mips/kernel/setup.c
+++ b/arch/mips/kernel/setup.c
@@ -251,6 +251,8 @@ static unsigned long __init init_initrd(void)
  * Initialize the bootmem allocator. It also setup initrd related data
  * if needed.
  */
+static int usermem __initdata;
+
 #if defined(CONFIG_SGI_IP27) || (defined(CONFIG_CPU_LOONGSON64) && 
defined(CONFIG_NUMA))
 
 static void __init bootmem_init(void)
@@ -290,7 +292,7 @@ static void __init bootmem_init(void)
/*
 * Reserve any memory between the start of RAM and PHYS_OFFSET
 */
-   if (ramstart > PHYS_OFFSET)
+   if (usermem && ramstart > PHYS_OFFSET)
memblock_reserve(PHYS_OFFSET, ramstart - PHYS_OFFSET);
 
if (PFN_UP(ramstart) > ARCH_PFN_OFFSET) {
@@ -338,8 +340,6 @@ static void __init bootmem_init(void)
 
 #endif /* CONFIG_SGI_IP27 */
 
-static int usermem __initdata;
-
 static int __init early_parse_mem(char *p)
 {
phys_addr_t start, size;
-- 
2.30.1



[PATCH] MIPS: ralink: select WEAK_REORDERING_BEYOND_LLSC on MT7621

2021-03-07 Thread Ilya Lipnitskiy
Upstream a long-standing OpenWrt patch for RALINK MT7621 SoC. Selecting
WEAK_REORDERING_BEYOND_LLSC fixes random kernel hangs. This bug and fix
was reported by MediaTek WCN division [0].

[0]: https://lists.infradead.org/pipermail/lede-commits/2017-August/004537.html

Signed-off-by: Ilya Lipnitskiy 
Cc: linux-media...@lists.infradead.org
Cc: linux-m...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 arch/mips/ralink/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/mips/ralink/Kconfig b/arch/mips/ralink/Kconfig
index c20c44788b62..31671bbf26ff 100644
--- a/arch/mips/ralink/Kconfig
+++ b/arch/mips/ralink/Kconfig
@@ -62,6 +62,7 @@ choice
select CLKSRC_MIPS_GIC
select HAVE_PCI if PCI_MT7621
select SOC_BUS
+   select WEAK_REORDERING_BEYOND_LLSC
 endchoice
 
 choice
-- 
2.30.1



[PATCH] MIPS: ralink: make RALINK_ILL_ACC symbol visible

2021-03-06 Thread Ilya Lipnitskiy
The illegal access driver is optional - it is informational and does not
provide critical functionality. Furthermore, it is currently not
functional on RT5350 SoCs, so a user may choose to omit non-functional
code on that platform. The default is kept at 'y' for backwards
compatibility. This change only makes the symbol visible and adds a
brief description.

Signed-off-by: Ilya Lipnitskiy 
Cc: linux-m...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 arch/mips/ralink/Kconfig | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/mips/ralink/Kconfig b/arch/mips/ralink/Kconfig
index c10d8b233ab1..c20c44788b62 100644
--- a/arch/mips/ralink/Kconfig
+++ b/arch/mips/ralink/Kconfig
@@ -9,9 +9,14 @@ config CLKEVT_RT3352
select CLKSRC_MMIO
 
 config RALINK_ILL_ACC
-   bool
+   bool "Illegal memory access IRQ driver"
depends on SOC_RT305X
default y
+   help
+ This enables notifications of illegal memory access events.
+
+ RT305x/RT5350 SoCs have a special IRQ that fires upon an illegal 
memory
+ access.
 
 config IRQ_INTC
bool
-- 
2.30.1



[PATCH] MIPS: pci-mt7620: fix PLL lock check

2021-03-06 Thread Ilya Lipnitskiy
Upstream a long-standing OpenWrt patch [0] that fixes MT7620 PCIe PLL
lock check. The existing code checks the wrong register bit: PPLL_SW_SET
is not defined in PPLL_CFG1 and bit 31 of PPLL_CFG1 is marked as reserved
in the MT7620 Programming Guide. The correct bit to check for PLL lock
is PPLL_LD (bit 23).

Also reword the error message for clarity.

Without this change it is unlikely that this driver ever worked with
mainline kernel.

[0]: https://lists.infradead.org/pipermail/lede-commits/2017-July/004441.html

Signed-off-by: Ilya Lipnitskiy 
Cc: John Crispin 
Cc: linux-m...@vger.kernel.org
Cc: linux-media...@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Cc: sta...@vger.kernel.org
---
 arch/mips/pci/pci-mt7620.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/mips/pci/pci-mt7620.c b/arch/mips/pci/pci-mt7620.c
index d36061603752..e032932348d6 100644
--- a/arch/mips/pci/pci-mt7620.c
+++ b/arch/mips/pci/pci-mt7620.c
@@ -30,6 +30,7 @@
 #define RALINK_GPIOMODE0x60
 
 #define PPLL_CFG1  0x9c
+#define PPLL_LDBIT(23)
 
 #define PPLL_DRV   0xa0
 #define PDRV_SW_SETBIT(31)
@@ -239,8 +240,8 @@ static int mt7620_pci_hw_init(struct platform_device *pdev)
rt_sysc_m32(0, RALINK_PCIE0_CLK_EN, RALINK_CLKCFG1);
mdelay(100);
 
-   if (!(rt_sysc_r32(PPLL_CFG1) & PDRV_SW_SET)) {
-   dev_err(>dev, "MT7620 PPLL unlock\n");
+   if (!(rt_sysc_r32(PPLL_CFG1) & PPLL_LD)) {
+   dev_err(>dev, "pcie PLL not locked, aborting init\n");
reset_control_assert(rstpcie0);
rt_sysc_m32(RALINK_PCIE0_CLK_EN, 0, RALINK_CLKCFG1);
return -1;
-- 
2.30.1



Re: exec error: BUG: Bad rss-counter

2021-03-03 Thread Ilya Lipnitskiy
On Wed, Mar 3, 2021 at 7:50 AM Eric W. Biederman  wrote:
>
> Ilya Lipnitskiy  writes:
>
> > On Tue, Mar 2, 2021 at 11:37 AM Eric W. Biederman  
> > wrote:
> >>
> >> Ilya Lipnitskiy  writes:
> >>
> >> > On Mon, Mar 1, 2021 at 12:43 PM Eric W. Biederman 
> >> >  wrote:
> >> >>
> >> >> Ilya Lipnitskiy  writes:
> >> >>
> >> >> > Eric, All,
> >> >> >
> >> >> > The following error appears when running Linux 5.10.18 on an embedded
> >> >> > MIPS mt7621 target:
> >> >> > [0.301219] BUG: Bad rss-counter state mm:(ptrval) 
> >> >> > type:MM_ANONPAGES val:1
> >> >> >
> >> >> > Being a very generic error, I started digging and added a stack dump
> >> >> > before the BUG:
> >> >> > Call Trace:
> >> >> > [<80008094>] show_stack+0x30/0x100
> >> >> > [<8033b238>] dump_stack+0xac/0xe8
> >> >> > [<800285e8>] __mmdrop+0x98/0x1d0
> >> >> > [<801a6de8>] free_bprm+0x44/0x118
> >> >> > [<801a86a8>] kernel_execve+0x160/0x1d8
> >> >> > [<800420f4>] call_usermodehelper_exec_async+0x114/0x194
> >> >> > [<80003198>] ret_from_kernel_thread+0x14/0x1c
> >> >> >
> >> >> > So that's how I got to looking at fs/exec.c and noticed quite a few
> >> >> > changes last year. Turns out this message only occurs once very early
> >> >> > at boot during the very first call to kernel_execve. current->mm is
> >> >> > NULL at this stage, so acct_arg_size() is effectively a no-op.
> >> >>
> >> >> If you believe this is a new error you could bisect the kernel
> >> >> to see which change introduced the behavior you are seeing.
> >> >>
> >> >> > More digging, and I traced the RSS counter increment to:
> >> >> > [<8015adb4>] add_mm_counter_fast+0xb4/0xc0
> >> >> > [<80160d58>] handle_mm_fault+0x6e4/0xea0
> >> >> > [<80158aa4>] __get_user_pages.part.78+0x190/0x37c
> >> >> > [<8015992c>] __get_user_pages_remote+0x128/0x360
> >> >> > [<801a6d9c>] get_arg_page+0x34/0xa0
> >> >> > [<801a7394>] copy_string_kernel+0x194/0x2a4
> >> >> > [<801a880c>] kernel_execve+0x11c/0x298
> >> >> > [<800420f4>] call_usermodehelper_exec_async+0x114/0x194
> >> >> > [<80003198>] ret_from_kernel_thread+0x14/0x1c
> >> >> >
> >> >> > In fact, I also checked vma_pages(bprm->vma) and lo and behold it is 
> >> >> > set to 1.
> >> >> >
> >> >> > How is fs/exec.c supposed to handle implied RSS increments that happen
> >> >> > due to page faults when discarding the bprm structure? In this case,
> >> >> > the bug-generating kernel_execve call never succeeded, it returned -2,
> >> >> > but I didn't trace exactly what failed.
> >> >>
> >> >> Unless I am mistaken any left over pages should be purged by exit_mmap
> >> >> which is called by mmput before mmput calls mmdrop.
> >> > Good to know. Some more digging and I can say that we hit this error
> >> > when trying to unmap PFN 0 (is_zero_pfn(pfn) returns TRUE,
> >> > vm_normal_page returns NULL, zap_pte_range does not decrement
> >> > MM_ANONPAGES RSS counter). Is my understanding correct that PFN 0 is
> >> > usable, but special? Or am I totally off the mark here?
> >>
> >> It would be good to know if that is the page that get_user_pages_remote
> >> returned to copy_string_kernel.  The zero page that is always zero,
> >> should never be returned when a writable mapping is desired.
> >
> > Indeed, pfn 0 is returned from get_arg_page: (page is 0x809cf000,
> > page_to_pfn(page) is 0) and it is the same page that is being freed and not
> > refcounted in mmput/zap_pte_range. Confirmed with good old printk. Also,
> > ZERO_PAGE(0)==0x809fc000 -> PFN 5120.
> >
> > I think I have found the problem though, after much digging and thanks to 
> > all
> > the information provided. init_zero_pfn() gets called too late (after
> > the call to
> > is_zero_pfn(0) from mmput returns true), until then zero_pfn == 0, and 
> > after,
> > zero_pfn == 

Re: exec error: BUG: Bad rss-counter

2021-03-03 Thread Ilya Lipnitskiy
On Tue, Mar 2, 2021 at 10:56 AM Linus Torvalds
 wrote:
>
> On Mon, Mar 1, 2021 at 11:59 PM Ilya Lipnitskiy
>  wrote:
> >
> > Good to know. Some more digging and I can say that we hit this error
> > when trying to unmap PFN 0 (is_zero_pfn(pfn) returns TRUE,
> > vm_normal_page returns NULL, zap_pte_range does not decrement
> > MM_ANONPAGES RSS counter). Is my understanding correct that PFN 0 is
> > usable, but special? Or am I totally off the mark here?
>
> PFN 0 should be usable - depending on architecture, of course - and
> shouldn't even be special in any way.
>
> is_zero_pfn(pfn) is *not* meant to test for pfn being 0 - it's meant
> to test for the pfn pointing to the special zero-filled page. The two
> _could_ be the same thing, of course, but generally are not (h8300
> seems to say "we use pfn 0 as the zero page" if I read things right).
>
> In fact, there can be many zero-filled pages - architectures with
> virtually mapped caches that want cache coloring have multiple
> contiguous zero-filled pages and then map in the right one based on
> virtual address. I'm not sure why it would matter (the zero-page is
> always mapped read-only, so any physical aliases should be a
> non-issue), but whatever..
>
> > Here is the (optimized) stack trace when the counter does not get 
> > decremented:
> > [<8015b078>] vm_normal_page+0x114/0x1a8
>
> Yes, if "is_zero_pfn()" returns true, then it won't be considered a
> normal page, and is not refcounted.
>
> But that should only trigger for pfn == zero_pfn, and zero_pfn should
> be initialized to
>
> zero_pfn = page_to_pfn(ZERO_PAGE(0));
>
> so it _sounds_ like you possibly have something odd going on with ZERO_PAGE.
Thanks for explaining this - I have figured out that zero_pfn gets set
a little late (see other response for details). Until init_zero_pfn()
is called, zero_pfn==0, after - zero_pfn==5120. Seems somewhat bad,
unless my system is breaking rules by checking zero_pfn before it was
initialized ;)
>
> Yes, one architecture does actually make pfn 0 _be_ the zero page, but
> you said MIPS, and that does do the page coloring games, and has
>
>#define ZERO_PAGE(vaddr) \
> (virt_to_page((void *)(empty_zero_page + (((unsigned
> long)(vaddr)) & zero_page_mask
>
> where zero_page_mask is the page colorign mask, and empty_zero_page is
> allocated in setup_zero_pages() fairly early in mem_init() (again, it
> allocates multiple pages depending on the page ordering - see that
> horrible virtual cache thing with cpu_has_vce).
>
> So PFN 0 shouldn't be an issue at all.
>
> Of course, since you said this was an embedded MIPS platform, maybe
> it's one of the broken ones with virtual caches and cpu_has_vce is
> set. I'm not sure how much testing that has gotten lately. MOST of the
> later MIPS architectures walked away from the pure virtual cache
> setups.
FWIW, here is the CPU info from my platform, and cpu_has_vce is not set:

system type : MediaTek MT7621 ver:1 eco:3
machine : Ubiquiti EdgeRouter X
processor   : 0
cpu model   : MIPS 1004Kc V2.15
BogoMIPS: 581.63
wait instruction: yes
microsecond timers  : yes
tlb_entries : 32
extra interrupt vector  : yes
hardware watchpoint : yes, count: 4, address/irw mask: [0x0ffc,
0x0ffc, 0x0ffb, 0x0ffb]
isa : mips1 mips2 mips32r1 mips32r2
ASEs implemented: mips16 dsp mt
Options implemented : tlb 4kex 4k_cache prefetch mcheck ejtag llsc
pindexed_dcache userlocal vint perf_cntr_intr_bit cdmm perf
shadow register sets: 1
kscratch registers  : 0
package : 0
core: 0
VPE : 0
VCED exceptions : not available
VCEI exceptions : not available

>
>   Linus
Ilya


Re: exec error: BUG: Bad rss-counter

2021-03-03 Thread Ilya Lipnitskiy
On Tue, Mar 2, 2021 at 11:37 AM Eric W. Biederman  wrote:
>
> Ilya Lipnitskiy  writes:
>
> > On Mon, Mar 1, 2021 at 12:43 PM Eric W. Biederman  
> > wrote:
> >>
> >> Ilya Lipnitskiy  writes:
> >>
> >> > Eric, All,
> >> >
> >> > The following error appears when running Linux 5.10.18 on an embedded
> >> > MIPS mt7621 target:
> >> > [0.301219] BUG: Bad rss-counter state mm:(ptrval) type:MM_ANONPAGES 
> >> > val:1
> >> >
> >> > Being a very generic error, I started digging and added a stack dump
> >> > before the BUG:
> >> > Call Trace:
> >> > [<80008094>] show_stack+0x30/0x100
> >> > [<8033b238>] dump_stack+0xac/0xe8
> >> > [<800285e8>] __mmdrop+0x98/0x1d0
> >> > [<801a6de8>] free_bprm+0x44/0x118
> >> > [<801a86a8>] kernel_execve+0x160/0x1d8
> >> > [<800420f4>] call_usermodehelper_exec_async+0x114/0x194
> >> > [<80003198>] ret_from_kernel_thread+0x14/0x1c
> >> >
> >> > So that's how I got to looking at fs/exec.c and noticed quite a few
> >> > changes last year. Turns out this message only occurs once very early
> >> > at boot during the very first call to kernel_execve. current->mm is
> >> > NULL at this stage, so acct_arg_size() is effectively a no-op.
> >>
> >> If you believe this is a new error you could bisect the kernel
> >> to see which change introduced the behavior you are seeing.
> >>
> >> > More digging, and I traced the RSS counter increment to:
> >> > [<8015adb4>] add_mm_counter_fast+0xb4/0xc0
> >> > [<80160d58>] handle_mm_fault+0x6e4/0xea0
> >> > [<80158aa4>] __get_user_pages.part.78+0x190/0x37c
> >> > [<8015992c>] __get_user_pages_remote+0x128/0x360
> >> > [<801a6d9c>] get_arg_page+0x34/0xa0
> >> > [<801a7394>] copy_string_kernel+0x194/0x2a4
> >> > [<801a880c>] kernel_execve+0x11c/0x298
> >> > [<800420f4>] call_usermodehelper_exec_async+0x114/0x194
> >> > [<80003198>] ret_from_kernel_thread+0x14/0x1c
> >> >
> >> > In fact, I also checked vma_pages(bprm->vma) and lo and behold it is set 
> >> > to 1.
> >> >
> >> > How is fs/exec.c supposed to handle implied RSS increments that happen
> >> > due to page faults when discarding the bprm structure? In this case,
> >> > the bug-generating kernel_execve call never succeeded, it returned -2,
> >> > but I didn't trace exactly what failed.
> >>
> >> Unless I am mistaken any left over pages should be purged by exit_mmap
> >> which is called by mmput before mmput calls mmdrop.
> > Good to know. Some more digging and I can say that we hit this error
> > when trying to unmap PFN 0 (is_zero_pfn(pfn) returns TRUE,
> > vm_normal_page returns NULL, zap_pte_range does not decrement
> > MM_ANONPAGES RSS counter). Is my understanding correct that PFN 0 is
> > usable, but special? Or am I totally off the mark here?
>
> It would be good to know if that is the page that get_user_pages_remote
> returned to copy_string_kernel.  The zero page that is always zero,
> should never be returned when a writable mapping is desired.

Indeed, pfn 0 is returned from get_arg_page: (page is 0x809cf000,
page_to_pfn(page) is 0) and it is the same page that is being freed and not
refcounted in mmput/zap_pte_range. Confirmed with good old printk. Also,
ZERO_PAGE(0)==0x809fc000 -> PFN 5120.

I think I have found the problem though, after much digging and thanks to all
the information provided. init_zero_pfn() gets called too late (after
the call to
is_zero_pfn(0) from mmput returns true), until then zero_pfn == 0, and after,
zero_pfn == 5120. Boom.

So PFN 0 is special, but only for a little bit, enough for something
on my system
to call kernel_execve :)

Question: is my system not supposed to be calling kernel_execve this
early or does
init_zero_pfn() need to happen earlier? init_zero_pfn is currently a
core_initcall.

>
> > Here is the (optimized) stack trace when the counter does not get 
> > decremented:
> > [<8015b078>] vm_normal_page+0x114/0x1a8
> > [<8015dc98>] unmap_page_range+0x388/0xacc
> > [<8015e5a0>] unmap_vmas+0x6c/0x98
> > [<80166194>] exit_mmap+0xd8/0x1ac
> > [<800290c0>] mmput+0x58/0xf8
> > [<801a6f8c>] free_bprm+0x2c/0xc4
> > [<801a8890>] kernel_execve+0x160/0x1d8
> > [<800420e0>] call_usermodehelper_exec_async+0x114/0x194
>

Re: exec error: BUG: Bad rss-counter

2021-03-02 Thread Ilya Lipnitskiy
On Mon, Mar 1, 2021 at 12:43 PM Eric W. Biederman  wrote:
>
> Ilya Lipnitskiy  writes:
>
> > Eric, All,
> >
> > The following error appears when running Linux 5.10.18 on an embedded
> > MIPS mt7621 target:
> > [0.301219] BUG: Bad rss-counter state mm:(ptrval) type:MM_ANONPAGES 
> > val:1
> >
> > Being a very generic error, I started digging and added a stack dump
> > before the BUG:
> > Call Trace:
> > [<80008094>] show_stack+0x30/0x100
> > [<8033b238>] dump_stack+0xac/0xe8
> > [<800285e8>] __mmdrop+0x98/0x1d0
> > [<801a6de8>] free_bprm+0x44/0x118
> > [<801a86a8>] kernel_execve+0x160/0x1d8
> > [<800420f4>] call_usermodehelper_exec_async+0x114/0x194
> > [<80003198>] ret_from_kernel_thread+0x14/0x1c
> >
> > So that's how I got to looking at fs/exec.c and noticed quite a few
> > changes last year. Turns out this message only occurs once very early
> > at boot during the very first call to kernel_execve. current->mm is
> > NULL at this stage, so acct_arg_size() is effectively a no-op.
>
> If you believe this is a new error you could bisect the kernel
> to see which change introduced the behavior you are seeing.
>
> > More digging, and I traced the RSS counter increment to:
> > [<8015adb4>] add_mm_counter_fast+0xb4/0xc0
> > [<80160d58>] handle_mm_fault+0x6e4/0xea0
> > [<80158aa4>] __get_user_pages.part.78+0x190/0x37c
> > [<8015992c>] __get_user_pages_remote+0x128/0x360
> > [<801a6d9c>] get_arg_page+0x34/0xa0
> > [<801a7394>] copy_string_kernel+0x194/0x2a4
> > [<801a880c>] kernel_execve+0x11c/0x298
> > [<800420f4>] call_usermodehelper_exec_async+0x114/0x194
> > [<80003198>] ret_from_kernel_thread+0x14/0x1c
> >
> > In fact, I also checked vma_pages(bprm->vma) and lo and behold it is set to 
> > 1.
> >
> > How is fs/exec.c supposed to handle implied RSS increments that happen
> > due to page faults when discarding the bprm structure? In this case,
> > the bug-generating kernel_execve call never succeeded, it returned -2,
> > but I didn't trace exactly what failed.
>
> Unless I am mistaken any left over pages should be purged by exit_mmap
> which is called by mmput before mmput calls mmdrop.
Good to know. Some more digging and I can say that we hit this error
when trying to unmap PFN 0 (is_zero_pfn(pfn) returns TRUE,
vm_normal_page returns NULL, zap_pte_range does not decrement
MM_ANONPAGES RSS counter). Is my understanding correct that PFN 0 is
usable, but special? Or am I totally off the mark here?

Here is the (optimized) stack trace when the counter does not get decremented:
[<8015b078>] vm_normal_page+0x114/0x1a8
[<8015dc98>] unmap_page_range+0x388/0xacc
[<8015e5a0>] unmap_vmas+0x6c/0x98
[<80166194>] exit_mmap+0xd8/0x1ac
[<800290c0>] mmput+0x58/0xf8
[<801a6f8c>] free_bprm+0x2c/0xc4
[<801a8890>] kernel_execve+0x160/0x1d8
[<800420e0>] call_usermodehelper_exec_async+0x114/0x194
[<80003198>] ret_from_kernel_thread+0x14/0x1c

>
> AKA it looks very very fishy this happens and this does not look like
> an execve error.
I think you are right, I'm probably wrong to bother you. However,
since the thread is already started, let me add linux-mm here :)
>
> On the other hand it would be good to know why kernel_execve is failing.
> Then the error handling paths could be scrutinized, and we can check to
> see if everything that should happen on an error path does.
I can check on this, but likely it's the init system not doing things
quite in the right order on my platform, or something similar. The
error is ENOENT from do_open_execat().
>
> > Interestingly, this "BUG:" message is timing-dependent. If I wait a
> > bit before calling free_bprm after bprm_execve the message seems to go
> > away (there are 3 other cores running and calling into kernel_execve
> > at the same time, so there is that). The error also only ever happens
> > once (probably because no more page faults happen?).
> >
> > I don't know enough to propose a proper fix here. Is it decrementing
> > the bprm->mm RSS counter to account for that page fault? Or is
> > current->mm being NULL a bigger problem?
>
> This is call_usermode_helper calls kernel_execve from a kernel thread
> forked by kthreadd.  Which means current->mm == NULL is expected, and
> current->active_mm == _mm.
>
> Similarly I bprm->mm having an incremented RSS counter appears correct.
>
> The question is why doesn't that count get consistently cleaned up.
>
> > Apologies in advance, but I have looked hard and do not see a clear
> >

Re: [PATCH 4.19 209/247] staging/mt7621-dma: mtk-hsdma.c->hsdma-mt7621.c

2021-03-01 Thread Ilya Lipnitskiy
Sorry, I should have been more clear in my original email, but we
don't strictly need this in 4.19 as the check only became fatal in
5.10 (actually, before 5.10, but 5.10 is the first stable release with
it). Feel free to take or omit this from 4.19.

On Mon, Mar 1, 2021 at 8:45 AM Greg Kroah-Hartman
 wrote:
>
> From: Ilya Lipnitskiy 
>
> commit 1f92798cbe7fe923479cff754dd06dd23d352e36 upstream.
>
> Also use KBUILD_MODNAME for module name.
>
> This driver is only used by RALINK MIPS MT7621 SoCs. Tested by building
> against that target using OpenWrt with Linux 5.10.10.
>
> Fixes the following error:
> error: the following would cause module name conflict:
>   drivers/dma/mediatek/mtk-hsdma.ko
>   drivers/staging/mt7621-dma/mtk-hsdma.ko
>
> Cc: sta...@vger.kernel.org
> Cc: Masahiro Yamada 
> Signed-off-by: Ilya Lipnitskiy 
> Link: 
> https://lore.kernel.org/r/20210130034507.2115280-1-ilya.lipnits...@gmail.com
> Signed-off-by: Greg Kroah-Hartman 
> ---
>  drivers/staging/mt7621-dma/Makefile   |2
>  drivers/staging/mt7621-dma/hsdma-mt7621.c |  771 
> ++
>  drivers/staging/mt7621-dma/mtk-hsdma.c|  771 
> --
>  3 files changed, 772 insertions(+), 772 deletions(-)
>  rename drivers/staging/mt7621-dma/{mtk-hsdma.c => hsdma-mt7621.c} (99%)
>
> --- a/drivers/staging/mt7621-dma/Makefile
> +++ b/drivers/staging/mt7621-dma/Makefile
> @@ -1,4 +1,4 @@
>  obj-$(CONFIG_DMA_RALINK) += ralink-gdma.o
> -obj-$(CONFIG_MTK_HSDMA) += mtk-hsdma.o
> +obj-$(CONFIG_MTK_HSDMA) += hsdma-mt7621.o
>
>  ccflags-y += -I$(srctree)/drivers/dma
> --- /dev/null
> +++ b/drivers/staging/mt7621-dma/hsdma-mt7621.c
> @@ -0,0 +1,771 @@
> +/*
> + *  Copyright (C) 2015, Michael Lee 
> + *  MTK HSDMA support
> + *
> + *  This program is free software; you can redistribute it and/or modify it
> + *  under  the terms of the GNU General Public License as published 
> by the
> + *  Free Software Foundation;  either version 2 of the License, or (at your
> + *  option) any later version.
> + *
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "virt-dma.h"
> +
> +#define HSDMA_BASE_OFFSET  0x800
> +
> +#define HSDMA_REG_TX_BASE  0x00
> +#define HSDMA_REG_TX_CNT   0x04
> +#define HSDMA_REG_TX_CTX   0x08
> +#define HSDMA_REG_TX_DTX   0x0c
> +#define HSDMA_REG_RX_BASE  0x100
> +#define HSDMA_REG_RX_CNT   0x104
> +#define HSDMA_REG_RX_CRX   0x108
> +#define HSDMA_REG_RX_DRX   0x10c
> +#define HSDMA_REG_INFO 0x200
> +#define HSDMA_REG_GLO_CFG  0x204
> +#define HSDMA_REG_RST_CFG  0x208
> +#define HSDMA_REG_DELAY_INT0x20c
> +#define HSDMA_REG_FREEQ_THRES  0x210
> +#define HSDMA_REG_INT_STATUS   0x220
> +#define HSDMA_REG_INT_MASK 0x228
> +#define HSDMA_REG_SCH_Q01  0x280
> +#define HSDMA_REG_SCH_Q23  0x284
> +
> +#define HSDMA_DESCS_MAX0xfff
> +#define HSDMA_DESCS_NUM8
> +#define HSDMA_DESCS_MASK   (HSDMA_DESCS_NUM - 1)
> +#define HSDMA_NEXT_DESC(x) (((x) + 1) & HSDMA_DESCS_MASK)
> +
> +/* HSDMA_REG_INFO */
> +#define HSDMA_INFO_INDEX_MASK  0xf
> +#define HSDMA_INFO_INDEX_SHIFT 24
> +#define HSDMA_INFO_BASE_MASK   0xff
> +#define HSDMA_INFO_BASE_SHIFT  16
> +#define HSDMA_INFO_RX_MASK 0xff
> +#define HSDMA_INFO_RX_SHIFT8
> +#define HSDMA_INFO_TX_MASK 0xff
> +#define HSDMA_INFO_TX_SHIFT0
> +
> +/* HSDMA_REG_GLO_CFG */
> +#define HSDMA_GLO_TX_2B_OFFSET BIT(31)
> +#define HSDMA_GLO_CLK_GATE BIT(30)
> +#define HSDMA_GLO_BYTE_SWAPBIT(29)
> +#define HSDMA_GLO_MULTI_DMABIT(10)
> +#define HSDMA_GLO_TWO_BUF  BIT(9)
> +#define HSDMA_GLO_32B_DESC BIT(8)
> +#define HSDMA_GLO_BIG_ENDIAN   BIT(7)
> +#define HSDMA_GLO_TX_DONE  BIT(6)
> +#define HSDMA_GLO_BT_MASK  0x3
> +#define HSDMA_GLO_BT_SHIFT 4
> +#define HSDMA_GLO_RX_BUSY  BIT(3)
> +#define HSDMA_GLO_RX_DMA   BIT(2)
> +#define HSDMA_GLO_TX_BUSY  BIT(1)
> +#define HSDMA_GLO_TX_DMA   BIT(0)
> +
> +#define HSDMA_BT_SIZE_16BYTES  (0 << HSDMA_GLO_BT_SHIFT)
> +#define HSDMA_BT_SIZE_32BYTES  (1 <&l

Re: [PATCH 5.4 280/340] staging/mt7621-dma: mtk-hsdma.c->hsdma-mt7621.c

2021-03-01 Thread Ilya Lipnitskiy
Sorry, I should have been more clear in my original email, but we
don't strictly need this in 5.4 as the check only became fatal in 5.10
(actually, before 5.10, but 5.10 is the first stable release with it).
Feel free to take or omit this from 5.4.

On Mon, Mar 1, 2021 at 9:00 AM Greg Kroah-Hartman
 wrote:
>
> From: Ilya Lipnitskiy 
>
> commit 1f92798cbe7fe923479cff754dd06dd23d352e36 upstream.
>
> Also use KBUILD_MODNAME for module name.
>
> This driver is only used by RALINK MIPS MT7621 SoCs. Tested by building
> against that target using OpenWrt with Linux 5.10.10.
>
> Fixes the following error:
> error: the following would cause module name conflict:
>   drivers/dma/mediatek/mtk-hsdma.ko
>   drivers/staging/mt7621-dma/mtk-hsdma.ko
>
> Cc: sta...@vger.kernel.org
> Cc: Masahiro Yamada 
> Signed-off-by: Ilya Lipnitskiy 
> Link: 
> https://lore.kernel.org/r/20210130034507.2115280-1-ilya.lipnits...@gmail.com
> Signed-off-by: Greg Kroah-Hartman 
> ---
>  drivers/staging/mt7621-dma/Makefile   |2
>  drivers/staging/mt7621-dma/hsdma-mt7621.c |  762 
> ++
>  drivers/staging/mt7621-dma/mtk-hsdma.c|  762 
> --
>  3 files changed, 763 insertions(+), 763 deletions(-)
>  rename drivers/staging/mt7621-dma/{mtk-hsdma.c => hsdma-mt7621.c} (99%)
>
> --- a/drivers/staging/mt7621-dma/Makefile
> +++ b/drivers/staging/mt7621-dma/Makefile
> @@ -1,4 +1,4 @@
>  # SPDX-License-Identifier: GPL-2.0
> -obj-$(CONFIG_MTK_HSDMA) += mtk-hsdma.o
> +obj-$(CONFIG_MTK_HSDMA) += hsdma-mt7621.o
>
>  ccflags-y += -I$(srctree)/drivers/dma
> --- /dev/null
> +++ b/drivers/staging/mt7621-dma/hsdma-mt7621.c
> @@ -0,0 +1,762 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + *  Copyright (C) 2015, Michael Lee 
> + *  MTK HSDMA support
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "virt-dma.h"
> +
> +#define HSDMA_BASE_OFFSET  0x800
> +
> +#define HSDMA_REG_TX_BASE  0x00
> +#define HSDMA_REG_TX_CNT   0x04
> +#define HSDMA_REG_TX_CTX   0x08
> +#define HSDMA_REG_TX_DTX   0x0c
> +#define HSDMA_REG_RX_BASE  0x100
> +#define HSDMA_REG_RX_CNT   0x104
> +#define HSDMA_REG_RX_CRX   0x108
> +#define HSDMA_REG_RX_DRX   0x10c
> +#define HSDMA_REG_INFO 0x200
> +#define HSDMA_REG_GLO_CFG  0x204
> +#define HSDMA_REG_RST_CFG  0x208
> +#define HSDMA_REG_DELAY_INT0x20c
> +#define HSDMA_REG_FREEQ_THRES  0x210
> +#define HSDMA_REG_INT_STATUS   0x220
> +#define HSDMA_REG_INT_MASK 0x228
> +#define HSDMA_REG_SCH_Q01  0x280
> +#define HSDMA_REG_SCH_Q23  0x284
> +
> +#define HSDMA_DESCS_MAX0xfff
> +#define HSDMA_DESCS_NUM8
> +#define HSDMA_DESCS_MASK   (HSDMA_DESCS_NUM - 1)
> +#define HSDMA_NEXT_DESC(x) (((x) + 1) & HSDMA_DESCS_MASK)
> +
> +/* HSDMA_REG_INFO */
> +#define HSDMA_INFO_INDEX_MASK  0xf
> +#define HSDMA_INFO_INDEX_SHIFT 24
> +#define HSDMA_INFO_BASE_MASK   0xff
> +#define HSDMA_INFO_BASE_SHIFT  16
> +#define HSDMA_INFO_RX_MASK 0xff
> +#define HSDMA_INFO_RX_SHIFT8
> +#define HSDMA_INFO_TX_MASK 0xff
> +#define HSDMA_INFO_TX_SHIFT0
> +
> +/* HSDMA_REG_GLO_CFG */
> +#define HSDMA_GLO_TX_2B_OFFSET BIT(31)
> +#define HSDMA_GLO_CLK_GATE BIT(30)
> +#define HSDMA_GLO_BYTE_SWAPBIT(29)
> +#define HSDMA_GLO_MULTI_DMABIT(10)
> +#define HSDMA_GLO_TWO_BUF  BIT(9)
> +#define HSDMA_GLO_32B_DESC BIT(8)
> +#define HSDMA_GLO_BIG_ENDIAN   BIT(7)
> +#define HSDMA_GLO_TX_DONE  BIT(6)
> +#define HSDMA_GLO_BT_MASK  0x3
> +#define HSDMA_GLO_BT_SHIFT 4
> +#define HSDMA_GLO_RX_BUSY  BIT(3)
> +#define HSDMA_GLO_RX_DMA   BIT(2)
> +#define HSDMA_GLO_TX_BUSY  BIT(1)
> +#define HSDMA_GLO_TX_DMA   BIT(0)
> +
> +#define HSDMA_BT_SIZE_16BYTES  (0 << HSDMA_GLO_BT_SHIFT)
> +#define HSDMA_BT_SIZE_32BYTES  (1 << HSDMA_GLO_BT_SHIFT)
> +#define HSDMA_BT_SIZE_64BYTES  (2 << HSDMA_GLO_BT_SHIFT)
> +#define HSDMA_BT_SIZE_128BYTES (3 << HSDMA_GLO_BT_SHIFT)
> +
> +#define HSDMA_GLO_DEFAULT  (HSDMA_GLO_MULTI_DMA | \
> +   

exec error: BUG: Bad rss-counter

2021-02-28 Thread Ilya Lipnitskiy
Eric, All,

The following error appears when running Linux 5.10.18 on an embedded
MIPS mt7621 target:
[0.301219] BUG: Bad rss-counter state mm:(ptrval) type:MM_ANONPAGES val:1

Being a very generic error, I started digging and added a stack dump
before the BUG:
Call Trace:
[<80008094>] show_stack+0x30/0x100
[<8033b238>] dump_stack+0xac/0xe8
[<800285e8>] __mmdrop+0x98/0x1d0
[<801a6de8>] free_bprm+0x44/0x118
[<801a86a8>] kernel_execve+0x160/0x1d8
[<800420f4>] call_usermodehelper_exec_async+0x114/0x194
[<80003198>] ret_from_kernel_thread+0x14/0x1c

So that's how I got to looking at fs/exec.c and noticed quite a few
changes last year. Turns out this message only occurs once very early
at boot during the very first call to kernel_execve. current->mm is
NULL at this stage, so acct_arg_size() is effectively a no-op.

More digging, and I traced the RSS counter increment to:
[<8015adb4>] add_mm_counter_fast+0xb4/0xc0
[<80160d58>] handle_mm_fault+0x6e4/0xea0
[<80158aa4>] __get_user_pages.part.78+0x190/0x37c
[<8015992c>] __get_user_pages_remote+0x128/0x360
[<801a6d9c>] get_arg_page+0x34/0xa0
[<801a7394>] copy_string_kernel+0x194/0x2a4
[<801a880c>] kernel_execve+0x11c/0x298
[<800420f4>] call_usermodehelper_exec_async+0x114/0x194
[<80003198>] ret_from_kernel_thread+0x14/0x1c

In fact, I also checked vma_pages(bprm->vma) and lo and behold it is set to 1.

How is fs/exec.c supposed to handle implied RSS increments that happen
due to page faults when discarding the bprm structure? In this case,
the bug-generating kernel_execve call never succeeded, it returned -2,
but I didn't trace exactly what failed.

Interestingly, this "BUG:" message is timing-dependent. If I wait a
bit before calling free_bprm after bprm_execve the message seems to go
away (there are 3 other cores running and calling into kernel_execve
at the same time, so there is that). The error also only ever happens
once (probably because no more page faults happen?).

I don't know enough to propose a proper fix here. Is it decrementing
the bprm->mm RSS counter to account for that page fault? Or is
current->mm being NULL a bigger problem?

Apologies in advance, but I have looked hard and do not see a clear
resolution for this even in the latest kernel code.

- Ilya