Bug#950578: linux-image-4.19.67-2-arm64: Add ACPI network interface support for RPi4 (patch attached)

classic Classic list List threaded Threaded
15 messages Options
Reply | Threaded
Open this post in threaded view
|

Bug#950578: linux-image-4.19.67-2-arm64: Add ACPI network interface support for RPi4 (patch attached)

Pete Batard
Package: src:linux
Version: 4.19.67-2
Severity: important
Tags: patch


Dear Maintainers,

While it is currently possible to boot and install Debian 10.x on the
Raspberry Pi 4 using the official vanilla ARM64 ISO images (through the
use of the latest EDK2 RPi4 firmware such as the one provided at [1]),
one of the major drawbacks that exists is that, because the EDK2
firmware currently only supports Linux boot in ACPI mode and also
because the native Debian kernel does not include the bcmgenet module at
all (module is currently disabled altogether), it is not possible to
perform a networked installation of Debian on the Raspberry Pi 4.

Due to the popularity of the platform, we therefore suggest that
bcmgenet should be enabled for the next Debian ARM64 10.x release as a
matter of priority, after the the attached patch has been applied to the
default kernel as some modifications are required for the network
interface to work in ACPI mode in coordination with the EDK2 firmware.

With bcmgenet is currently being disabled as a module, and with the
effort we made to ensure that the modifications applied have the least
impact possible on existing code (the existing Device Tree code paths
are left virtually untouched), we assert that the inclusion of this
patch should be low risk and therefore very desirable to have for the
potentially large number of RPi4 users, who should then be able to
perform netinst installation of Debian 10.x on their platform using the
vanilla ARM64 images.

Thanks and regards,

/Pete

[1] https://github.com/pftf/RPi4

-- System Information:
Debian Release: 10.2
   APT prefers stable-updates
   APT policy: (500, 'stable-updates'), (500, 'stable')
Architecture: arm64 (aarch64)

Kernel: Linux 4.19.87 (SMP w/4 CPU cores)
Kernel taint flags: TAINT_UNSIGNED_MODULE
Locale: LANG=en_IE.UTF-8, LC_CTYPE=en_IE.UTF-8 (charmap=UTF-8),
LANGUAGE=en_IE:en (charmap=UTF-8)
Shell: /bin/sh linked to /usr/bin/dash
Init: systemd (via /run/systemd/system)
LSM: AppArmor: enabled

From d2fecdc2828dc0256df7c10e02fc6e7aaf99ce9a Mon Sep 17 00:00:00 2001
From: Jeremy Linton <[hidden email]>
Date: Mon, 3 Feb 2020 17:39:14 +0000
Subject: [PATCH] net: bcmgenet: add ACPI bindings

This patch is needed to support the Raspberry Pi 4 network interface on
native Debian 4.19 kernels in ACPI mode. With this, network installation
of Debian 10.x, from vanilla ARM64 ISO images, should become possible
when using the official EDK2 UEFI firmware for the Raspberry Pi 4, which
currently requires the use of ACPI when booting Linux.

The changes covered by this patch can be summarized as follows:
* Since the 4.19 Genet driver private data structures are lacking this
  attribute compared to the 5.x version, and this is the least intrusive
  way of compensating for that, the max DMA burst length must be allowed
  to be overridden, when it is specified as an ACPI DSD parameter (which
  the UEFI firmware will provide accordingly).
* The MAC address must be allowed to be set from the hardware's UMAC
  registers (which the UEFI firmware will program accordingly).
  Note that we grouped the MAC initialization further down the code for
  readability and added random allocation on invalid MAC, rather than
  report an error, as this is what the current 5.x driver does.
* The relevant ACPI initialization structures must exist in the driver.

We also take this opportunity to add an entry for brcm,bcm2711-genet-v5
in the Device Tree list, which is how the Raspberry Pi Foundation
declares the network interface (though using it as such in a 4.19 kernel
will require the provision of the "brcm,max-dma-burst-size" attribute).

Signed-off-by: Pete Batard <[hidden email]>
---
 .../net/ethernet/broadcom/genet/bcmgenet.c    | 74 ++++++++++++++-----
 drivers/net/ethernet/broadcom/genet/bcmmii.c  | 33 ++++++++-
 drivers/net/phy/mdio-bcm-unimac.c             | 26 ++++++-
 3 files changed, 111 insertions(+), 22 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index 338d22380434..2b4b2906a9b9 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -10,6 +10,7 @@
 
 #define pr_fmt(fmt) "bcmgenet: " fmt
 
+#include <linux/acpi.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/sched.h>
@@ -2553,6 +2554,7 @@ static int bcmgenet_init_dma(struct bcmgenet_priv *priv)
  int ret;
  unsigned int i;
  struct enet_cb *cb;
+ u32 dma_max_burst_length;
 
  netif_dbg(priv, hw, priv->dev, "%s\n", __func__);
 
@@ -2584,8 +2586,13 @@ static int bcmgenet_init_dma(struct bcmgenet_priv *priv)
  cb->bd_addr = priv->tx_bds + i * DMA_DESC_SIZE;
  }
 
+ /* Set the max DMA burst length if specified. Else, use default */
+ if (!priv->pdev || device_property_read_u32(&priv->pdev->dev,
+    "brcm,max-dma-burst-size", &dma_max_burst_length) != 0)
+ dma_max_burst_length = DMA_MAX_BURST_LENGTH;
+
  /* Init rDma */
- bcmgenet_rdma_writel(priv, DMA_MAX_BURST_LENGTH, DMA_SCB_BURST_SIZE);
+ bcmgenet_rdma_writel(priv, dma_max_burst_length, DMA_SCB_BURST_SIZE);
 
  /* Initialize Rx queues */
  ret = bcmgenet_init_rx_queues(priv->dev);
@@ -2598,7 +2605,7 @@ static int bcmgenet_init_dma(struct bcmgenet_priv *priv)
  }
 
  /* Init tDma */
- bcmgenet_tdma_writel(priv, DMA_MAX_BURST_LENGTH, DMA_SCB_BURST_SIZE);
+ bcmgenet_tdma_writel(priv, dma_max_burst_length, DMA_SCB_BURST_SIZE);
 
  /* Initialize Tx queues */
  bcmgenet_init_tx_queues(priv->dev);
@@ -2783,6 +2790,21 @@ static void bcmgenet_set_hw_addr(struct bcmgenet_priv *priv,
  bcmgenet_umac_writel(priv, (addr[4] << 8) | addr[5], UMAC_MAC1);
 }
 
+static void bcmgenet_get_hw_addr(struct bcmgenet_priv *priv,
+ unsigned char *addr)
+{
+ u32 addr_tmp;
+
+ addr_tmp = bcmgenet_umac_readl(priv, UMAC_MAC0);
+ addr[0] = addr_tmp >> 24;
+ addr[1] = (addr_tmp >> 16) & 0xff;
+ addr[2] = (addr_tmp >> 8) & 0xff;
+ addr[3] = addr_tmp & 0xff;
+ addr_tmp = bcmgenet_umac_readl(priv, UMAC_MAC1);
+ addr[4] = (addr_tmp >> 8) & 0xff;
+ addr[5] = addr_tmp & 0xff;
+}
+
 /* Returns a reusable dma control register value */
 static u32 bcmgenet_dma_disable(struct bcmgenet_priv *priv)
 {
@@ -3433,10 +3455,17 @@ static const struct of_device_id bcmgenet_match[] = {
  { .compatible = "brcm,genet-v3", .data = (void *)GENET_V3 },
  { .compatible = "brcm,genet-v4", .data = (void *)GENET_V4 },
  { .compatible = "brcm,genet-v5", .data = (void *)GENET_V5 },
+ { .compatible = "brcm,bcm2711-genet-v5", .data = (void *)GENET_V5 },
  { },
 };
 MODULE_DEVICE_TABLE(of, bcmgenet_match);
 
+static const struct acpi_device_id genet_acpi_match[] = {
+ { "BCM6E4E", (kernel_ulong_t)GENET_V5 },
+ { },
+};
+MODULE_DEVICE_TABLE(acpi, genet_acpi_match);
+
 static int bcmgenet_probe(struct platform_device *pdev)
 {
  struct bcmgenet_platform_data *pd = pdev->dev.platform_data;
@@ -3444,7 +3473,7 @@ static int bcmgenet_probe(struct platform_device *pdev)
  const struct of_device_id *of_id = NULL;
  struct bcmgenet_priv *priv;
  struct net_device *dev;
- const void *macaddr;
+ const void *macaddr = NULL;
  struct resource *r;
  unsigned int i;
  int err = -EIO;
@@ -3474,17 +3503,6 @@ static int bcmgenet_probe(struct platform_device *pdev)
  goto err;
  }
 
- if (dn) {
- macaddr = of_get_mac_address(dn);
- if (!macaddr) {
- dev_err(&pdev->dev, "can't find MAC address\n");
- err = -EINVAL;
- goto err;
- }
- } else {
- macaddr = pd->mac_address;
- }
-
  r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
  priv->base = devm_ioremap_resource(&pdev->dev, r);
  if (IS_ERR(priv->base)) {
@@ -3496,7 +3514,6 @@ static int bcmgenet_probe(struct platform_device *pdev)
 
  SET_NETDEV_DEV(dev, &pdev->dev);
  dev_set_drvdata(&pdev->dev, dev);
- ether_addr_copy(dev->dev_addr, macaddr);
  dev->watchdog_timeo = 2 * HZ;
  dev->ethtool_ops = &bcmgenet_ethtool_ops;
  dev->netdev_ops = &bcmgenet_netdev_ops;
@@ -3525,8 +3542,11 @@ static int bcmgenet_probe(struct platform_device *pdev)
  priv->pdev = pdev;
  if (of_id)
  priv->version = (enum bcmgenet_version)of_id->data;
- else
+ else if (pd)
  priv->version = pd->genet_version;
+ else
+ priv->version = (enum bcmgenet_version)
+ device_get_match_data(&pdev->dev);
 
  priv->clk = devm_clk_get(&priv->pdev->dev, "enet");
  if (IS_ERR(priv->clk)) {
@@ -3559,10 +3579,27 @@ static int bcmgenet_probe(struct platform_device *pdev)
  /* If this is an internal GPHY, power it on now, before UniMAC is
  * brought out of reset as absolutely no UniMAC activity is allowed
  */
- if (dn && !of_property_read_string(dn, "phy-mode", &phy_mode_str) &&
-    !strcasecmp(phy_mode_str, "internal"))
+ if (device_property_read_string(&pdev->dev, "phy-mode",
+    &phy_mode_str) == 0 && !strcasecmp(phy_mode_str, "internal"))
  bcmgenet_power_up(priv, GENET_POWER_PASSIVE);
 
+ if (dn)
+ macaddr = of_get_mac_address(dn);
+ else if (pd)
+ macaddr = pd->mac_address;
+
+ if (IS_ERR_OR_NULL(macaddr) || !is_valid_ether_addr(macaddr)) {
+ if (has_acpi_companion(&pdev->dev))
+ bcmgenet_get_hw_addr(priv, dev->dev_addr);
+
+ if (!is_valid_ether_addr(dev->dev_addr)) {
+ dev_warn(&pdev->dev, "using random Ethernet MAC\n");
+ eth_hw_addr_random(dev);
+ }
+ } else {
+ ether_addr_copy(dev->dev_addr, macaddr);
+ }
+
  reset_umac(priv);
 
  err = bcmgenet_mii_init(dev);
@@ -3730,6 +3767,7 @@ static struct platform_driver bcmgenet_driver = {
  .name = "bcmgenet",
  .of_match_table = bcmgenet_match,
  .pm = &bcmgenet_pm_ops,
+ .acpi_match_table = ACPI_PTR(genet_acpi_match),
  },
 };
 module_platform_driver(bcmgenet_driver);
diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c b/drivers/net/ethernet/broadcom/genet/bcmmii.c
index b0592fd4135b..3b3d412b0e4a 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmmii.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c
@@ -8,7 +8,7 @@
  * published by the Free Software Foundation.
  */
 
-
+#include <linux/acpi.h>
 #include <linux/types.h>
 #include <linux/delay.h>
 #include <linux/wait.h>
@@ -277,7 +277,8 @@ int bcmgenet_mii_config(struct net_device *dev, bool init)
 int bcmgenet_mii_probe(struct net_device *dev)
 {
  struct bcmgenet_priv *priv = netdev_priv(dev);
- struct device_node *dn = priv->pdev->dev.of_node;
+ struct device *kdev = &priv->pdev->dev;
+ struct device_node *dn = kdev->of_node;
  struct phy_device *phydev;
  u32 phy_flags = 0;
  int ret;
@@ -299,6 +300,20 @@ int bcmgenet_mii_probe(struct net_device *dev)
  pr_err("could not attach to PHY\n");
  return -ENODEV;
  }
+ } else if (has_acpi_companion(kdev)) {
+ char phy_name[MII_BUS_ID_SIZE + 3];
+ char mdio_bus_id[MII_BUS_ID_SIZE];
+
+ snprintf(mdio_bus_id, MII_BUS_ID_SIZE, "%s-%d",
+ UNIMAC_MDIO_DRV_NAME, priv->pdev->id);
+ snprintf(phy_name, MII_BUS_ID_SIZE, PHY_ID_FMT, mdio_bus_id, 1);
+
+ phydev = phy_connect(dev, phy_name, bcmgenet_mii_setup,
+     priv->phy_interface);
+ if (!IS_ERR(phydev))
+ phydev->dev_flags = phy_flags;
+ else
+ return -ENODEV;
  } else {
  phydev = dev->phydev;
  phydev->dev_flags = phy_flags;
@@ -545,12 +560,24 @@ static int bcmgenet_mii_pd_init(struct bcmgenet_priv *priv)
  return 0;
 }
 
+static int bcmgenet_mii_acpi_init(struct bcmgenet_priv *priv)
+{
+ struct device *kdev = &priv->pdev->dev;
+ priv->phy_interface = device_get_phy_mode(kdev);
+ if (priv->phy_interface < 0)
+ priv->phy_interface = PHY_INTERFACE_MODE_RGMII;
+ return 0;
+}
+
 static int bcmgenet_mii_bus_init(struct bcmgenet_priv *priv)
 {
- struct device_node *dn = priv->pdev->dev.of_node;
+ struct device *kdev = &priv->pdev->dev;
+ struct device_node *dn = kdev->of_node;
 
  if (dn)
  return bcmgenet_mii_of_init(priv);
+ else if (has_acpi_companion(kdev))
+ return bcmgenet_mii_acpi_init(priv);
  else
  return bcmgenet_mii_pd_init(priv);
 }
diff --git a/drivers/net/phy/mdio-bcm-unimac.c b/drivers/net/phy/mdio-bcm-unimac.c
index df75efa96a7d..b71fbc42e324 100644
--- a/drivers/net/phy/mdio-bcm-unimac.c
+++ b/drivers/net/phy/mdio-bcm-unimac.c
@@ -294,9 +294,33 @@ static int unimac_mdio_probe(struct platform_device *pdev)
  goto out_mdio_free;
  }
 
+ if (!np) {
+ struct phy_device *phy_dev;
+ int phy_addr_check;
+
+ for (phy_addr_check = 0; phy_addr_check < PHY_MAX_ADDR; phy_addr_check++) {
+ phy_dev = get_phy_device(bus, phy_addr_check ,false);
+ if (IS_ERR(phy_dev)) {
+ if (phy_addr_check == PHY_MAX_ADDR - 1) {
+ dev_err(&pdev->dev, "MDIO phy search failed\n");
+ goto out_mdio_free;
+ }
+ } else {
+ dev_dbg(&pdev->dev, "MDIO phy success @ %d\n", phy_addr_check);
+ break;
+ }
+ }
+
+ if (phy_device_register(phy_dev)) {
+ dev_err(&pdev->dev, "MDIO phy register failed\n");
+ phy_device_free(phy_dev);
+ goto out_mdio_free;
+ }
+ }
+
  platform_set_drvdata(pdev, priv);
 
- dev_info(&pdev->dev, "Broadcom UniMAC MDIO bus at 0x%p\n", priv->base);
+ dev_info(&pdev->dev, "Broadcom UniMAC MDIO bus id=%s\n", bus->id);
 
  return 0;
 
--
2.21.0.windows.1

Reply | Threaded
Open this post in threaded view
|

Bug#950578: (no subject)

Pete Batard
Just gonna add that the latest UEFI Firmware, released today at
https://github.com/pftf/RPi4/releases, now contains all the elements
needed (ACPI binding and UMAC initialization) for the above patch to work.

Which means that, the only limiting factor for UEFI Debian 10.x netinst
on a Raspberry Pi 4 is that the kernel is missing the patch above.

Thanks,

/Pete

Reply | Threaded
Open this post in threaded view
|

Bug#950578: Updated patch

Pete Batard
Here's an updated version of the patch, since we switched to using
"rgmii-rxid" instead of "rgmii" for phy-mode in ACPI (which is what the
current Linux Device Tree also uses and which prevents packet loss and
network instability) and the 4.19 bcmmii driver is missing a case for
PHY_INTERFACE_MODE_RGMII_RXID.

We also use this opportunity to fix a possible warning in
bcmgenet_mii_acpi_init().

Once again, it would be greatly appreciated if this patch could be
applied as a matter of priority as it's all that stands between making
the next release of Debian net installable on a Raspberry Pi 4.

From 2d780b0bec254002221559e51cc0b6e5038d4f1a Mon Sep 17 00:00:00 2001
From: Jeremy Linton <[hidden email]>
Date: Mon, 3 Feb 2020 17:39:14 +0000
Subject: [PATCH] net: bcmgenet: add ACPI bindings

This patch is needed to support the Raspberry Pi 4 network interface on
native Debian 4.19 kernels in ACPI mode. With this, network installation
of Debian 10.x, from vanilla ARM64 ISO images, should become possible
when using the official EDK2 UEFI firmware for the Raspberry Pi 4, which
currently requires the use of ACPI when booting Linux.

The changes covered by this patch can be summarized as follows:
* Since the 4.19 Genet driver private data structures are lacking this
  attribute compared to the 5.x version, and this is the least intrusive
  way of compensating for that, the max DMA burst length must be allowed
  to be overridden, when it is specified as an ACPI DSD parameter (which
  the UEFI firmware will provide accordingly).
* The MAC address must be allowed to be set from the hardware's UMAC
  registers (which the UEFI firmware will program accordingly).
  Note that we grouped the MAC initialization further down the code for
  readability and added random allocation on invalid MAC, rather than
  report an error, as this is what the current 5.x driver does.
* The relevant ACPI initialization structures must exist in the driver.

We also take this opportunity to add an entry for brcm,bcm2711-genet-v5
in the Device Tree list, which is how the Raspberry Pi Foundation
declares the network interface (though using it as such in a 4.19 kernel
will require the provision of the "brcm,max-dma-burst-size" attribute).

Signed-off-by: Pete Batard <[hidden email]>
---
 .../net/ethernet/broadcom/genet/bcmgenet.c    | 74 ++++++++++++++-----
 drivers/net/ethernet/broadcom/genet/bcmmii.c  | 40 +++++++++-
 drivers/net/phy/mdio-bcm-unimac.c             | 26 ++++++-
 3 files changed, 118 insertions(+), 22 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index 338d22380434..2b4b2906a9b9 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -10,6 +10,7 @@
 
 #define pr_fmt(fmt) "bcmgenet: " fmt
 
+#include <linux/acpi.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/sched.h>
@@ -2553,6 +2554,7 @@ static int bcmgenet_init_dma(struct bcmgenet_priv *priv)
  int ret;
  unsigned int i;
  struct enet_cb *cb;
+ u32 dma_max_burst_length;
 
  netif_dbg(priv, hw, priv->dev, "%s\n", __func__);
 
@@ -2584,8 +2586,13 @@ static int bcmgenet_init_dma(struct bcmgenet_priv *priv)
  cb->bd_addr = priv->tx_bds + i * DMA_DESC_SIZE;
  }
 
+ /* Set the max DMA burst length if specified. Else, use default */
+ if (!priv->pdev || device_property_read_u32(&priv->pdev->dev,
+    "brcm,max-dma-burst-size", &dma_max_burst_length) != 0)
+ dma_max_burst_length = DMA_MAX_BURST_LENGTH;
+
  /* Init rDma */
- bcmgenet_rdma_writel(priv, DMA_MAX_BURST_LENGTH, DMA_SCB_BURST_SIZE);
+ bcmgenet_rdma_writel(priv, dma_max_burst_length, DMA_SCB_BURST_SIZE);
 
  /* Initialize Rx queues */
  ret = bcmgenet_init_rx_queues(priv->dev);
@@ -2598,7 +2605,7 @@ static int bcmgenet_init_dma(struct bcmgenet_priv *priv)
  }
 
  /* Init tDma */
- bcmgenet_tdma_writel(priv, DMA_MAX_BURST_LENGTH, DMA_SCB_BURST_SIZE);
+ bcmgenet_tdma_writel(priv, dma_max_burst_length, DMA_SCB_BURST_SIZE);
 
  /* Initialize Tx queues */
  bcmgenet_init_tx_queues(priv->dev);
@@ -2783,6 +2790,21 @@ static void bcmgenet_set_hw_addr(struct bcmgenet_priv *priv,
  bcmgenet_umac_writel(priv, (addr[4] << 8) | addr[5], UMAC_MAC1);
 }
 
+static void bcmgenet_get_hw_addr(struct bcmgenet_priv *priv,
+ unsigned char *addr)
+{
+ u32 addr_tmp;
+
+ addr_tmp = bcmgenet_umac_readl(priv, UMAC_MAC0);
+ addr[0] = addr_tmp >> 24;
+ addr[1] = (addr_tmp >> 16) & 0xff;
+ addr[2] = (addr_tmp >> 8) & 0xff;
+ addr[3] = addr_tmp & 0xff;
+ addr_tmp = bcmgenet_umac_readl(priv, UMAC_MAC1);
+ addr[4] = (addr_tmp >> 8) & 0xff;
+ addr[5] = addr_tmp & 0xff;
+}
+
 /* Returns a reusable dma control register value */
 static u32 bcmgenet_dma_disable(struct bcmgenet_priv *priv)
 {
@@ -3433,10 +3455,17 @@ static const struct of_device_id bcmgenet_match[] = {
  { .compatible = "brcm,genet-v3", .data = (void *)GENET_V3 },
  { .compatible = "brcm,genet-v4", .data = (void *)GENET_V4 },
  { .compatible = "brcm,genet-v5", .data = (void *)GENET_V5 },
+ { .compatible = "brcm,bcm2711-genet-v5", .data = (void *)GENET_V5 },
  { },
 };
 MODULE_DEVICE_TABLE(of, bcmgenet_match);
 
+static const struct acpi_device_id genet_acpi_match[] = {
+ { "BCM6E4E", (kernel_ulong_t)GENET_V5 },
+ { },
+};
+MODULE_DEVICE_TABLE(acpi, genet_acpi_match);
+
 static int bcmgenet_probe(struct platform_device *pdev)
 {
  struct bcmgenet_platform_data *pd = pdev->dev.platform_data;
@@ -3444,7 +3473,7 @@ static int bcmgenet_probe(struct platform_device *pdev)
  const struct of_device_id *of_id = NULL;
  struct bcmgenet_priv *priv;
  struct net_device *dev;
- const void *macaddr;
+ const void *macaddr = NULL;
  struct resource *r;
  unsigned int i;
  int err = -EIO;
@@ -3474,17 +3503,6 @@ static int bcmgenet_probe(struct platform_device *pdev)
  goto err;
  }
 
- if (dn) {
- macaddr = of_get_mac_address(dn);
- if (!macaddr) {
- dev_err(&pdev->dev, "can't find MAC address\n");
- err = -EINVAL;
- goto err;
- }
- } else {
- macaddr = pd->mac_address;
- }
-
  r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
  priv->base = devm_ioremap_resource(&pdev->dev, r);
  if (IS_ERR(priv->base)) {
@@ -3496,7 +3514,6 @@ static int bcmgenet_probe(struct platform_device *pdev)
 
  SET_NETDEV_DEV(dev, &pdev->dev);
  dev_set_drvdata(&pdev->dev, dev);
- ether_addr_copy(dev->dev_addr, macaddr);
  dev->watchdog_timeo = 2 * HZ;
  dev->ethtool_ops = &bcmgenet_ethtool_ops;
  dev->netdev_ops = &bcmgenet_netdev_ops;
@@ -3525,8 +3542,11 @@ static int bcmgenet_probe(struct platform_device *pdev)
  priv->pdev = pdev;
  if (of_id)
  priv->version = (enum bcmgenet_version)of_id->data;
- else
+ else if (pd)
  priv->version = pd->genet_version;
+ else
+ priv->version = (enum bcmgenet_version)
+ device_get_match_data(&pdev->dev);
 
  priv->clk = devm_clk_get(&priv->pdev->dev, "enet");
  if (IS_ERR(priv->clk)) {
@@ -3559,10 +3579,27 @@ static int bcmgenet_probe(struct platform_device *pdev)
  /* If this is an internal GPHY, power it on now, before UniMAC is
  * brought out of reset as absolutely no UniMAC activity is allowed
  */
- if (dn && !of_property_read_string(dn, "phy-mode", &phy_mode_str) &&
-    !strcasecmp(phy_mode_str, "internal"))
+ if (device_property_read_string(&pdev->dev, "phy-mode",
+    &phy_mode_str) == 0 && !strcasecmp(phy_mode_str, "internal"))
  bcmgenet_power_up(priv, GENET_POWER_PASSIVE);
 
+ if (dn)
+ macaddr = of_get_mac_address(dn);
+ else if (pd)
+ macaddr = pd->mac_address;
+
+ if (IS_ERR_OR_NULL(macaddr) || !is_valid_ether_addr(macaddr)) {
+ if (has_acpi_companion(&pdev->dev))
+ bcmgenet_get_hw_addr(priv, dev->dev_addr);
+
+ if (!is_valid_ether_addr(dev->dev_addr)) {
+ dev_warn(&pdev->dev, "using random Ethernet MAC\n");
+ eth_hw_addr_random(dev);
+ }
+ } else {
+ ether_addr_copy(dev->dev_addr, macaddr);
+ }
+
  reset_umac(priv);
 
  err = bcmgenet_mii_init(dev);
@@ -3730,6 +3767,7 @@ static struct platform_driver bcmgenet_driver = {
  .name = "bcmgenet",
  .of_match_table = bcmgenet_match,
  .pm = &bcmgenet_pm_ops,
+ .acpi_match_table = ACPI_PTR(genet_acpi_match),
  },
 };
 module_platform_driver(bcmgenet_driver);
diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c b/drivers/net/ethernet/broadcom/genet/bcmmii.c
index b0592fd4135b..b764444474d9 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmmii.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c
@@ -8,7 +8,7 @@
  * published by the Free Software Foundation.
  */
 
-
+#include <linux/acpi.h>
 #include <linux/types.h>
 #include <linux/delay.h>
 #include <linux/wait.h>
@@ -250,6 +250,11 @@ int bcmgenet_mii_config(struct net_device *dev, bool init)
  bcmgenet_sys_writel(priv,
     PORT_MODE_EXT_GPHY, SYS_PORT_CTRL);
  break;
+ case PHY_INTERFACE_MODE_RGMII_RXID:
+ phy_name = "external RGMII (RX delay)";
+ bcmgenet_sys_writel(priv,
+    PORT_MODE_EXT_GPHY, SYS_PORT_CTRL);
+ break;
  default:
  dev_err(kdev, "unknown phy mode: %d\n", priv->phy_interface);
  return -EINVAL;
@@ -277,7 +282,8 @@ int bcmgenet_mii_config(struct net_device *dev, bool init)
 int bcmgenet_mii_probe(struct net_device *dev)
 {
  struct bcmgenet_priv *priv = netdev_priv(dev);
- struct device_node *dn = priv->pdev->dev.of_node;
+ struct device *kdev = &priv->pdev->dev;
+ struct device_node *dn = kdev->of_node;
  struct phy_device *phydev;
  u32 phy_flags = 0;
  int ret;
@@ -299,6 +305,20 @@ int bcmgenet_mii_probe(struct net_device *dev)
  pr_err("could not attach to PHY\n");
  return -ENODEV;
  }
+ } else if (has_acpi_companion(kdev)) {
+ char phy_name[MII_BUS_ID_SIZE + 3];
+ char mdio_bus_id[MII_BUS_ID_SIZE];
+
+ snprintf(mdio_bus_id, MII_BUS_ID_SIZE, "%s-%d",
+ UNIMAC_MDIO_DRV_NAME, priv->pdev->id);
+ snprintf(phy_name, MII_BUS_ID_SIZE, PHY_ID_FMT, mdio_bus_id, 1);
+
+ phydev = phy_connect(dev, phy_name, bcmgenet_mii_setup,
+     priv->phy_interface);
+ if (!IS_ERR(phydev))
+ phydev->dev_flags = phy_flags;
+ else
+ return -ENODEV;
  } else {
  phydev = dev->phydev;
  phydev->dev_flags = phy_flags;
@@ -545,12 +565,26 @@ static int bcmgenet_mii_pd_init(struct bcmgenet_priv *priv)
  return 0;
 }
 
+static int bcmgenet_mii_acpi_init(struct bcmgenet_priv *priv)
+{
+ struct device *kdev = &priv->pdev->dev;
+ int phy_mode = device_get_phy_mode(kdev);
+ if (phy_mode < 0)
+ priv->phy_interface = PHY_INTERFACE_MODE_RGMII;
+ else
+ priv->phy_interface = phy_mode;
+ return 0;
+}
+
 static int bcmgenet_mii_bus_init(struct bcmgenet_priv *priv)
 {
- struct device_node *dn = priv->pdev->dev.of_node;
+ struct device *kdev = &priv->pdev->dev;
+ struct device_node *dn = kdev->of_node;
 
  if (dn)
  return bcmgenet_mii_of_init(priv);
+ else if (has_acpi_companion(kdev))
+ return bcmgenet_mii_acpi_init(priv);
  else
  return bcmgenet_mii_pd_init(priv);
 }
diff --git a/drivers/net/phy/mdio-bcm-unimac.c b/drivers/net/phy/mdio-bcm-unimac.c
index df75efa96a7d..b71fbc42e324 100644
--- a/drivers/net/phy/mdio-bcm-unimac.c
+++ b/drivers/net/phy/mdio-bcm-unimac.c
@@ -294,9 +294,33 @@ static int unimac_mdio_probe(struct platform_device *pdev)
  goto out_mdio_free;
  }
 
+ if (!np) {
+ struct phy_device *phy_dev;
+ int phy_addr_check;
+
+ for (phy_addr_check = 0; phy_addr_check < PHY_MAX_ADDR; phy_addr_check++) {
+ phy_dev = get_phy_device(bus, phy_addr_check ,false);
+ if (IS_ERR(phy_dev)) {
+ if (phy_addr_check == PHY_MAX_ADDR - 1) {
+ dev_err(&pdev->dev, "MDIO phy search failed\n");
+ goto out_mdio_free;
+ }
+ } else {
+ dev_dbg(&pdev->dev, "MDIO phy success @ %d\n", phy_addr_check);
+ break;
+ }
+ }
+
+ if (phy_device_register(phy_dev)) {
+ dev_err(&pdev->dev, "MDIO phy register failed\n");
+ phy_device_free(phy_dev);
+ goto out_mdio_free;
+ }
+ }
+
  platform_set_drvdata(pdev, priv);
 
- dev_info(&pdev->dev, "Broadcom UniMAC MDIO bus at 0x%p\n", priv->base);
+ dev_info(&pdev->dev, "Broadcom UniMAC MDIO bus id=%s\n", bus->id);
 
  return 0;
 
--
2.21.0.windows.1

Reply | Threaded
Open this post in threaded view
|

Bug#950578: (no subject)

Pete Batard
Just going to point out that the 5.x equivalent patch has now been
applied to Linux mainline as per:

https://lkml.kernel.org/lkml/20200224.153146.125327154283545636.davem@.../

Could we get some acknowledgement that this patchset is being looked at
for possible integration and isn't falling through the cracks?

This has been opened for more than a month now, and, notwithstanding the
patch update above, it's quite damaging for Raspberry Pi users that the
10.3 update didn't include these changes, as it would have enabled them
to install Debian from the vanilla 10.3 release ISO whereas Pi 4 users
will have to wait months before that happens...

So, can someone please look at this request and at least acknowledge it?

Thank you,

/Pete

Reply | Threaded
Open this post in threaded view
|

Bug#950578: fixed in linux 5.5.13-1

Pete Batard
In reply to this post by Pete Batard
Does that mean that the next Debian ISO installer will use Linux kernel
5.5.13.1 or later?

Because, if that is not the case, this bug is certainly not fixed at all
and must be reopened.

To reiterate, the problem is that the official netinst ISO cannot
perform a network installation of the Debian packages due to its use of
a 4.x kernel that is lacking an ACPI compatible Genet driver.

This means that the *installation* 4.x kernel (i.e. the default kernel
used by the installer, which, unlike the post installation kernel, is
not something that can be upgraded) must be patched with a retrofitted
Genet driver.

In other words, if this is considered fixed simply because the
additional Debian kernel package has been updated to latest mainline,
then this is *NOT* fixed at all, as the one and original problem of not
being able to perform a networked installation of Debian on a Raspberry
Pi 4 will remain.

I would therefore appreciate if you can please answer the following:

Will the next release of Debian ditch 4.19 kernels altogether and switch
to the 5.5.13-1 (or later) kernel for its installer?

Only if the answer to the question above is "yes" can the issue be
considered fixed.

But if the answer is no, then you must reopen this issue and apply the
proposed fix, as the 5.5.13.1 update will not address this issue at all.

Thank you,

/Pete

Reply | Threaded
Open this post in threaded view
|

Bug#950578: fixed in linux 5.5.13-1

Ben Hutchings-6
On Mon, 2020-03-30 at 09:44 +0100, Pete Batard wrote:

> Does that mean that the next Debian ISO installer will use Linux kernel
> 5.5.13.1 or later?
>
> Because, if that is not the case, this bug is certainly not fixed at all
> and must be reopened.
>
> To reiterate, the problem is that the official netinst ISO cannot
> perform a network installation of the Debian packages due to its use of
> a 4.x kernel that is lacking an ACPI compatible Genet driver.
>
> This means that the *installation* 4.x kernel (i.e. the default kernel
> used by the installer, which, unlike the post installation kernel, is
> not something that can be upgraded) must be patched with a retrofitted
> Genet driver.
[...]

This is why the bug tracking system records which versions are fixed,
and which are not.

Ben.

--
Ben Hutchings - Debian developer, member of kernel, installer and LTS teams

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Bug#950578: linux-image-5.5.0-1-armmp: enable Raspberry Pi 4 NIC module "bcmgenet"

Steven Shiau
In reply to this post by Pete Batard
Package: src:linux
Version: 5.5.13-2
Severity: normal

Dear Maintainer,
Since the module for the NIC of raspberry Pi 4 was enabled in Linux 5.5.13-2 arm64:

$ grep CONFIG_BCMGENET config-5.5.0-1-arm64
CONFIG_BCMGENET=m

However, it's not enabled in armhf:

$ grep CONFIG_BCMGENET config-5.5.0-1-armmp-lpae
# CONFIG_BCMGENET is not set

Is that possible you can enable it for armhf architecture in the next
release?
Some people still would like to use armhf for raspberry Pi 4.

Thank you very much.

Steven

--
Steven Shiau <steven _at_ stevenshiau org>
Public Key Server PGP Key ID: 4096R/163E3FB0
Fingerprint: EB1D D5BF 6F88 820B BCF5  356C 8E94 C9CD 163E 3FB0

Reply | Threaded
Open this post in threaded view
|

Bug#950578: Linux 5.5.0-1-arm64: kernel panic after module bcmgenet was loaded

Steven Shiau
In reply to this post by Pete Batard
Package: src:linux
Version: 5.5.13-2
Severity: normal

Dear Maintainer,

I created an arm64 live system for Raspberry Pi 4 using Debian
live-build, and it successfully booted into the initramfs.
However, after the network module bcmgenet was loaded, I got the kernel
panic.
Attached please find the output messages on the serial console.
If you need more info or tests, please let me know.

Thank you very much.

Steven

--
Steven Shiau <steven _at_ stevenshiau org>
Public Key Server PGP Key ID: 4096R/163E3FB0
Fingerprint: EB1D D5BF 6F88 820B BCF5  356C 8E94 C9CD 163E 3FB0


rpi4-kernel-panic.txt (31K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Bug#950578: Linux 5.5.0-1-arm64: kernel panic after module bcmgenet was loaded

Ben Hutchings-3
On Thu, 2020-04-09 at 11:43 +0800, Steven Shiau wrote:

> Package: src:linux
> Version: 5.5.13-2
> Severity: normal
>
> Dear Maintainer,
>
> I created an arm64 live system for Raspberry Pi 4 using Debian
> live-build, and it successfully booted into the initramfs.
> However, after the network module bcmgenet was loaded, I got the kernel
> panic.
> Attached please find the output messages on the serial console.
That's a WARNING not a panic.  And this needs to be a separate bug
report.

> If you need more info or tests, please let me know.

I don't see any sign of UEFI in the kernel log.  I think our kernel
only supports the RPi 4 running UEFI firmware.

Ben.

--
Ben Hutchings
73.46% of all statistics are made up.


signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Bug#950578: linux-image-4.19.67-2-arm64: Add ACPI network interface support for RPi4

Pete Batard
In reply to this post by Pete Batard
I would really like to get an update on this, because I really can't
understand what the holdup is, or why non related issues seem to be be
shoved into this bug, with the apparent end result of completely
distracting from the matter at hand.

This bug is about one thing and one thing only: Enabling Raspberry Pi 4
users to perform a netinstall using the next official aarch64 ISO of
Debian 10.x. Therefore it is only about backporting the Genet ACPI
driver into the 4.19 kernel, for which an effective backport patch was
actually submitted.

It is *NOT* about tracking whether the 5.x kernel packages have Genet
support. And it is *NOT* about troubleshooting network issues with the
5.x kernel.

The sole focus for the bug, as it was opened by the submitter (myself)
is to add Genet support to the kernel that is used by the Debian ISO
installer, and, seeing that no progress appears to have been made on
that front, despite the fact that a patch to *SOLVE* the reported issue
has been submitted along with the bug report, I would greatly appreciate
if we could reframe the problem and drop all references to 5.x genet
support as being linked to this bug, as it looks to me like this is
hindering the resolution of the one and only issue that prompted the
creation of this bug.

I would also greatly appreciate if this could actually be treated with
the level of urgency it requires on account of the following.

- As of March 2020, the Raspberry Pi Foundation announced that it had
sold 640 000 Raspberry Pi 4 units, and one can reasonably expect that 1
million units will have been sold by year's end, which clearly makes the
platform one of the most popular ARM64 targets, if not the most popular,
and therefore, one can reasonably expect many of its users to want to
install Debian 10.x on it. By not applying the proposed patch and
enabling netinst from official Debian 10.x ISOs as a matter of urgency,
Debian maintainers will be doing a major disservice to their users.

- The required patch to *SOLVE* the issue has been provided, so it's not
like Debian maintainers have to invest time to create the backport
themselves. And for the record, I did work with Jeremy Linton, the
person who upstreamed the main patch for 5.x kernels, and used the code
that he was in the process of submitting at that time to create the
backport (which is actually tailored for easy review by Debian
maintainers), so the attached patch was not produced in isolation.

- Though it may look that way at first glance, this patch is not being
requested because we are using a custom/toy bootloader. On the contrary,
the very reason why we can use the official aarch64 ISO is because we
are following industry standards pretty much to the letter. We are using
both ACPI and UEFI in a very official manner, and as a matter of fact,
the UEFI firmware that is meant to be used with the official ISOs is
fully integrated with the EDK2 [1]. So this is not a "it would be nice
if Debian could do this so that it would work with our custom
bootloader" but really a "If Debian is to follow industry standards for
the Raspberry Pi 4 and other UEFI platforms that use a Broadcom Genet
NIC, then it should provide the functionality requested above, for which
we have conveniently provided a patch".

So, if the integration of the proposed patch into the kernel used by the
next Debian ISO release is going to be delayed further, I would really
like the Debian maintainers to explain why that is the case.

We have been *EXCEEDINGLY* patient about this and waited in the hope
that Debian maintainers would understand the urgency, but, seeing that
Debian 10.4, which is planned to be released in 2 days, does not appear
to integrate the patch we proposed (either that or this bug tracker was
not updated as it should), I feel that we might as well tell the
thousands of Raspberry Pi 4 users who we are seeing downloading the UEFI
firmware in the hope of using it to install GNU/Linux, that they should
just forget about Debian, because it seems Debian maintainers have
either very little interest in ensuring that their OS can be installed
on what is, by far, be the most popular ARM64 platforms out there, or
have failed to grasp the implications of not applying the patch that can
solve this very important issue and which was proposed *MONTHS* ago...

Regards,

/Pete

[1]
https://github.com/tianocore/edk2-platforms/tree/master/Platform/RaspberryPi/RPi4

Reply | Threaded
Open this post in threaded view
|

Bug#950578: linux-image-4.19.67-2-arm64: Add ACPI network interface support for RPi4

Ben Hutchings-6
Pete Batard wrote:
> I would really like to get an update on this, because I really can't
> understand what the holdup is, or why non related issues seem to be be
> shoved into this bug, with the apparent end result of completely
> distracting from the matter at hand.
[...]
> It is *NOT* about tracking whether the 5.x kernel packages have Genet
> support. And it is *NOT* about troubleshooting network issues with the
> 5.x kernel.

The Debian bug tracking system is quite capable of recording *which*
versions an issue is found and fixed in.  So it's fine that this bug is
marked fixed in 5.x; we still know that it's unfixed in 4.19.

Also, there would be no point in enabling this driver in 4.19, only to
have people find on upgrade to the next version on Debian that we
didn't enable it there.  That's why we're concerned with both versions.

[...]
> I would also greatly appreciate if this could actually be treated with
> the level of urgency it requires on account of the following.

All "missing hardware support" bugs have severity "important".

[...]
> - The required patch to *SOLVE* the issue has been provided, so it's not
[...]

I acknowledge that you have provided a patch, which I appreciate, and
I'm sorry you haven't had a specific response to that yet.

Generally we want patches that correspond closely to upstream commits.
For 5.5 I had to backport these 6 commits:

ce69e2162f15 mdio_bus: Add generic mdio_find_bus()
480ded265205 net: bcmgenet: refactor phy mode configuration
6ef31c8bee5b net: bcmgenet: enable automatic phy discovery
99c6b06a37d4 net: bcmgenet: Initial bcmgenet ACPI support
26bd9cc64faf net: bcmgenet: Fetch MAC address from the adapter
ae200c26b32b net: bcmgenet: reduce severity of missing clock warnings

Can you please provide separate backports of these, instead of a single
patch where it's hard to see what changed?

Ben.

--
Ben Hutchings - Debian developer, member of kernel, installer and LTS teams

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Bug#950578: linux-image-4.19.67-2-arm64: Add ACPI network interface support for RPi4

Pete Batard
On 2020.05.07 13:45, Ben Hutchings wrote:
> Also, there would be no point in enabling this driver in 4.19, only to
> have people find on upgrade to the next version on Debian that we
> didn't enable it there.  That's why we're concerned with both versions.

Yes, I did consider that, but I feel that this is an issue that should
have been tracked separately (especially as, like we have seen, this
polluted this issue with unrelated queries), as it doesn't have the same
level of severity. Vanilla breakage vs optional breakage should not be
grouped together IMO. But of course, you're free to do what you want.

> [...]
>> I would also greatly appreciate if this could actually be treated with
>> the level of urgency it requires on account of the following.
>
> All "missing hardware support" bugs have severity "important".

And I will assert that bugs should be evaluated in terms of potential
users that are going to be impacted.

Support for a NIC that's used in niche hardware should not have the same
level of severity as support for a NIC that is used on hardware that
sold a quarter of a million units in less than a year.

> [...]
>> - The required patch to *SOLVE* the issue has been provided, so it's not
> [...]
>
> I acknowledge that you have provided a patch, which I appreciate, and
> I'm sorry you haven't had a specific response to that yet.
>
> Generally we want patches that correspond closely to upstream commits.
> For 5.5 I had to backport these 6 commits:
>
> ce69e2162f15 mdio_bus: Add generic mdio_find_bus()
> 480ded265205 net: bcmgenet: refactor phy mode configuration
> 6ef31c8bee5b net: bcmgenet: enable automatic phy discovery
> 99c6b06a37d4 net: bcmgenet: Initial bcmgenet ACPI support
> 26bd9cc64faf net: bcmgenet: Fetch MAC address from the adapter
> ae200c26b32b net: bcmgenet: reduce severity of missing clock warnings
>
> Can you please provide separate backports of these, instead of a single
> patch where it's hard to see what changed?

Sigh.

I specifically designed the patch I submitted for easy review and
integration, because there are missing elements from 4.x that are
present in 5.x, that we have to compensate for. I would rather not have
to split it, especially as I believe it should be included as a matter
of priority and we're simply adding delays.

If Debian 11 was planning to continue to use a 4.x kernel, I could see
some point in splitting the patch and ensuring, so that it *might* be
easier to maintain for many years to come. But, from what I gather,
Debian 11 will bump kernel major, so any work being done making the 4.x
backport (which is not that complex, sorry, especially as I made sure to
already group the code changes in a manner that makes it easier to
handle) easier to maintain in the long run seems like a waste of time,
even if 10.x may see long time support for a few more years...

If you had made that point a few months ago, I would have been more
inclined to do it, but at this stage, considering that it's too late for
10.4 anyway, and considering that I feel like I've wasted more than
enough time trying to push for this change to be included to help RPi4
Debian users, and that 2 releases have gone by without any progress, I
will respectfully decline to provide alterations to the submission
unless it has to do with something that effectively prevents its
integration, sorry.

Regards,

/Pete

Reply | Threaded
Open this post in threaded view
|

Bug#950578: linux-image-4.19.67-2-arm64: Add ACPI network interface support for RPi4

Ben Hutchings-6
On Thu, 2020-05-07 at 14:23 +0100, Pete Batard wrote:
[...]
> I specifically designed the patch I submitted for easy review and
> integration, because there are missing elements from 4.x that are
> present in 5.x, that we have to compensate for. I would rather not have
> to split it, especially as I believe it should be included as a matter
> of priority and we're simply adding delays.

I personally find it much easier to review backports in this form, and
it is the usual practice in Debian to backport changes in this form
where possible.

> If Debian 11 was planning to continue to use a 4.x kernel, I could see
> some point in splitting the patch and ensuring, so that it *might* be
> easier to maintain for many years to come.  But, from what I gather,
> Debian 11 will bump kernel major, so any work being done making the 4.x
> backport (which is not that complex, sorry, especially as I made sure to
> already group the code changes in a manner that makes it easier to
> handle) easier to maintain in the long run seems like a waste of time,
> even if 10.x may see long time support for a few more years...
[...]

Debian 10 will be supported for 4 more years, in fact.  During that
time this driver may well see other changes backported through the
stable process.  Based on past experience, I think it will be easier to
resolve any conflicts if our patches make smaller changes.

So please don't assume what's "easier to handle" for us.

Ben.

--
Ben Hutchings - Debian developer, member of kernel, installer and LTS teams

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Bug#950578: linux-image-4.19.67-2-arm64: Add ACPI network interface support for RPi4

Pete Batard
On 2020.05.09 22:20, Ben Hutchings wrote:

> On Thu, 2020-05-07 at 14:23 +0100, Pete Batard wrote:
> [...]
>> I specifically designed the patch I submitted for easy review and
>> integration, because there are missing elements from 4.x that are
>> present in 5.x, that we have to compensate for. I would rather not have
>> to split it, especially as I believe it should be included as a matter
>> of priority and we're simply adding delays.
>
> I personally find it much easier to review backports in this form, and
> it is the usual practice in Debian to backport changes in this form
> where possible.

Then you should have asked for that 3 months ago.

Sorry but that mistakes of not following up on the patch I submitted in
a reasonable timeframe, or understanding its context and priority level
is entirely on you.

I have tried all the means I knew to bring attention to this patch for
weeks that followed its submission (including posting about it in
various Debian mailing lists, including arm64 and debian-release), and
nothing happened, and especially nobody provided any inkling about the
submission needing to be split, so you genuinely have exhausted all of
the good will I had on that topic.

Moreover, if you are taking my refusal to split the patch as an excuse
not to produce your own effort to include it in the next release, then
you are simply corroborating the idea I have established that Debian
appears to have no clue about the level of importance it should actually
allocate to the underlying problem this is attempting to solve, and,
furthermore, that it may simply be looking for a way out of a situation
it mishandled.

In other words, I am waiting to see a real concerted effort on your side
before I decide whether I am willing to contribute any further, and
insistance on asking me to perform work that, if you understood the
actual severity of the situation, you should be more than willing to
perform yourself as a matter of urgency, simply indicates that you still
haven't understood the nature of the underlying situation. As a direct
result of that, I have good reasons to estimate that further involvement
from my side is simply not worth the trouble, because it appears that
you are still going to be bouncing, delaying or mishandling the ability
for RPi4 users to install Debian from vanilla Debian ISOs.

Therefore either you understand the importance of the problem, and are
willing to perform the work required to reorganize the submission
according to your *preferences*. Or you continue to hint that you don't,
and instead try to place the blame on someone from whom you have long
exhausted any willingness to contribute to something they have good
reasons to believe, from the continued unwillingness to take the
necessary actions on your side, is most likely going to amount to
*another* completely wasted effort.

In other words, I will need to see some real good will from Debian in
the matter of trying to salvage the current RPi4 situation before I
decide to get involved in trying to contribute again. Thus, in case this
was not clear, let me be explicit in saying that you are not going to
see any further contribution from me until you have made some visible
effort to remedy that.

Regards,

/Pete

Reply | Threaded
Open this post in threaded view
|

Bug#950578: [External] Bug#950578: linux-image-4.19.67-2-arm64: Add ACPI network interface support for RPi4

Mark Pearson
In reply to this post by Ben Hutchings-6
> From: Ben Hutchings <[hidden email]>
> Sent: Saturday, May 9, 2020 5:20 PM
>
> On Thu, 2020-05-07 at 14:23 +0100, Pete Batard wrote:
> [...]
<snip>

>
> > If Debian 11 was planning to continue to use a 4.x kernel, I could see
> > some point in splitting the patch and ensuring, so that it *might* be
> > easier to maintain for many years to come.  But, from what I gather,
> > Debian 11 will bump kernel major, so any work being done making the 4.x
> > backport (which is not that complex, sorry, especially as I made sure to
> > already group the code changes in a manner that makes it easier to
> > handle) easier to maintain in the long run seems like a waste of time,
> > even if 10.x may see long time support for a few more years...
> [...]
>
> Debian 10 will be supported for 4 more years, in fact.  During that
> time this driver may well see other changes backported through the
> stable process.  Based on past experience, I think it will be easier to
> resolve any conflicts if our patches make smaller changes.
>
> So please don't assume what's "easier to handle" for us.
>
> Ben.
>
I'm hesitant to post to this thread as I don't agree with all of Pete's points,
but this thread somewhat resonated, especially this last comment.
As someone who is still learning and finding their way through the process
myself - finding the preferred way of doing things and all the little details  
is hard - or at least that's my experience. The kernel handbook is OK, but
it doesn't cover this detail in my opinion

It would be really nice to have an idiots/beginners guide to what the best way
is to make the maintainers life easy - I have been stumbling my way through,
making plenty of mistakes and I'm sure I generate more headaches then I mean
to.

Having a guide explaining how to backport a patch cleanly from kernel.org
would be a really nice thing to have - down to best working practices with
salsa, all the bits of info that have to be added to the patch(es), using dch,
how to deal with patches that don't merge cleanly, git best practices etc. I'm
sure as a kernel maintainer you see the same mistake again and again and it
must be infuriating. Recommended workflows would be amazing - I'm still not
sure what the *best* way to work on the Debian kernel is (I have steps I use
but I had to figure them out myself and I suspect they could be better).

It would also be really nice to have a way to reasonably escalate things (with
a reason for the escalation) without pissing off people who are too busy to be
swamped with nag-emails (I've been told those are a huge no-no with the
kernel Debian community). I'd be OK with a "this is not a priority I will look at it
in N weeks" but having no insight into where you are in the queue or if you
have been missed is hard.

From my point of view I regularly get asked "when will X be available in Debian"
and I can never give an estimate. I cannot recommend to even expert Lenovo
customers to use Debian on our platforms - and that is frustrating because it is
a great distro.

Please note - I do not mean to sound like I am complaining. I believe part of
being in the community is to do things the way the community wants. I just
think the path for newcomers to contribute and make Debian better is not
an easy one and wanted to offer insight as to why. If you can point me at
what I'm missing that would be awesome (and slightly embarrassing as I've
looked for it at length)

Mark