Devicetree
 help / color / mirror / Atom feed
* [PATCH v3 2/4] iio: chemical: scd30: add I2C interface driver
From: Tomasz Duszynski @ 2020-06-02 16:47 UTC (permalink / raw)
  To: linux-iio
  Cc: linux-kernel, devicetree, robh+dt, jic23, andy.shevchenko, pmeerw,
	Tomasz Duszynski
In-Reply-To: <20200602164723.28858-1-tomasz.duszynski@octakon.com>

Add I2C interface driver for the SCD30 sensor.

Signed-off-by: Tomasz Duszynski <tomasz.duszynski@octakon.com>
---
 MAINTAINERS                      |   1 +
 drivers/iio/chemical/Kconfig     |  11 +++
 drivers/iio/chemical/Makefile    |   1 +
 drivers/iio/chemical/scd30_i2c.c | 134 +++++++++++++++++++++++++++++++
 4 files changed, 147 insertions(+)
 create mode 100644 drivers/iio/chemical/scd30_i2c.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 41a509cca6f1..13aed3473b7e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15142,6 +15142,7 @@ M:	Tomasz Duszynski <tomasz.duszynski@octakon.com>
 S:	Maintained
 F:	drivers/iio/chemical/scd30.h
 F:	drivers/iio/chemical/scd30_core.c
+F:	drivers/iio/chemical/scd30_i2c.c
 
 SENSIRION SPS30 AIR POLLUTION SENSOR DRIVER
 M:	Tomasz Duszynski <tduszyns@gmail.com>
diff --git a/drivers/iio/chemical/Kconfig b/drivers/iio/chemical/Kconfig
index 99e852b67e55..970d34888c2e 100644
--- a/drivers/iio/chemical/Kconfig
+++ b/drivers/iio/chemical/Kconfig
@@ -96,6 +96,17 @@ config SCD30_CORE
 	  To compile this driver as a module, choose M here: the module will
 	  be called scd30_core.
 
+config SCD30_I2C
+	tristate "SCD30 carbon dioxide sensor I2C driver"
+	depends on SCD30_CORE && I2C
+	select CRC8
+	help
+	  Say Y here to build support for the Sensirion SCD30 I2C interface
+	  driver.
+
+	  To compile this driver as a module, choose M here: the module will
+	  be called scd30_i2c.
+
 config SENSIRION_SGP30
 	tristate "Sensirion SGPxx gas sensors"
 	depends on I2C
diff --git a/drivers/iio/chemical/Makefile b/drivers/iio/chemical/Makefile
index c9804b041ecd..0966ca34e34b 100644
--- a/drivers/iio/chemical/Makefile
+++ b/drivers/iio/chemical/Makefile
@@ -13,6 +13,7 @@ obj-$(CONFIG_CCS811)		+= ccs811.o
 obj-$(CONFIG_IAQCORE)		+= ams-iaq-core.o
 obj-$(CONFIG_PMS7003) += pms7003.o
 obj-$(CONFIG_SCD30_CORE) += scd30_core.o
+obj-$(CONFIG_SCD30_I2C) += scd30_i2c.o
 obj-$(CONFIG_SENSIRION_SGP30)	+= sgp30.o
 obj-$(CONFIG_SPS30) += sps30.o
 obj-$(CONFIG_VZ89X)		+= vz89x.o
diff --git a/drivers/iio/chemical/scd30_i2c.c b/drivers/iio/chemical/scd30_i2c.c
new file mode 100644
index 000000000000..a6b532b83669
--- /dev/null
+++ b/drivers/iio/chemical/scd30_i2c.c
@@ -0,0 +1,134 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Sensirion SCD30 carbon dioxide sensor i2c driver
+ *
+ * Copyright (c) 2020 Tomasz Duszynski <tomasz.duszynski@octakon.com>
+ *
+ * I2C slave address: 0x61
+ */
+#include <linux/crc8.h>
+#include <linux/device.h>
+#include <linux/errno.h>
+#include <linux/i2c.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/types.h>
+#include <asm/unaligned.h>
+
+#include "scd30.h"
+
+#define SCD30_I2C_MAX_BUF_SIZE 18
+#define SCD30_I2C_CRC8_POLYNOMIAL 0x31
+
+static u16 scd30_i2c_cmd_lookup_tbl[] = {
+	[CMD_START_MEAS] = 0x0010,
+	[CMD_STOP_MEAS] = 0x0104,
+	[CMD_MEAS_INTERVAL] = 0x4600,
+	[CMD_MEAS_READY] = 0x0202,
+	[CMD_READ_MEAS] = 0x0300,
+	[CMD_ASC] = 0x5306,
+	[CMD_FRC] = 0x5204,
+	[CMD_TEMP_OFFSET] = 0x5403,
+	[CMD_FW_VERSION] = 0xd100,
+	[CMD_RESET] = 0xd304,
+};
+
+DECLARE_CRC8_TABLE(scd30_i2c_crc8_tbl);
+
+static int scd30_i2c_xfer(struct scd30_state *state, char *txbuf, int txsize,
+			  char *rxbuf, int rxsize)
+{
+	struct i2c_client *client = to_i2c_client(state->dev);
+	int ret;
+
+	/*
+	 * repeated start is not supported hence instead of sending two i2c
+	 * messages in a row we send one by one
+	 */
+	ret = i2c_master_send(client, txbuf, txsize);
+	if (ret != txsize)
+		return ret < 0 ? ret : -EIO;
+
+	if (!rxbuf)
+		return 0;
+
+	ret = i2c_master_recv(client, rxbuf, rxsize);
+	if (ret != rxsize)
+		return ret < 0 ? ret : -EIO;
+
+	return 0;
+}
+
+static int scd30_i2c_command(struct scd30_state *state, enum scd30_cmd cmd,
+			     u16 arg, void *response, int size)
+{
+	char crc, buf[SCD30_I2C_MAX_BUF_SIZE], *rsp = response;
+	int i, ret;
+
+	put_unaligned_be16(scd30_i2c_cmd_lookup_tbl[cmd], buf);
+	i = 2;
+
+	if (rsp) {
+		/* each two bytes are followed by a crc8 */
+		size += size / 2;
+	} else {
+		put_unaligned_be16(arg, buf + i);
+		crc = crc8(scd30_i2c_crc8_tbl, buf + i, 2, CRC8_INIT_VALUE);
+		i += 2;
+		buf[i] = crc;
+		i += 1;
+
+		/* commands below don't take an argument */
+		if ((cmd == CMD_STOP_MEAS) || (cmd == CMD_RESET))
+			i -= 3;
+	}
+
+	ret = scd30_i2c_xfer(state, buf, i, buf, size);
+	if (ret)
+		return ret;
+
+	/* validate received data and strip off crc bytes */
+	for (i = 0; i < size; i += 3) {
+		crc = crc8(scd30_i2c_crc8_tbl, buf + i, 2, CRC8_INIT_VALUE);
+		if (crc != buf[i + 2]) {
+			dev_err(state->dev, "data integrity check failed\n");
+			return -EIO;
+		}
+
+		*rsp++ = buf[i];
+		*rsp++ = buf[i + 1];
+	}
+
+	return 0;
+}
+
+static int scd30_i2c_probe(struct i2c_client *client)
+{
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
+		return -EOPNOTSUPP;
+
+	crc8_populate_msb(scd30_i2c_crc8_tbl, SCD30_I2C_CRC8_POLYNOMIAL);
+
+	return scd30_probe(&client->dev, client->irq, client->name, NULL,
+			   scd30_i2c_command);
+}
+
+static const struct of_device_id scd30_i2c_of_match[] = {
+	{ .compatible = "sensirion,scd30" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, scd30_i2c_of_match);
+
+static struct i2c_driver scd30_i2c_driver = {
+	.driver = {
+		.name = KBUILD_MODNAME,
+		.of_match_table = scd30_i2c_of_match,
+		.pm = &scd30_pm_ops,
+	},
+	.probe_new = scd30_i2c_probe,
+};
+module_i2c_driver(scd30_i2c_driver);
+
+MODULE_AUTHOR("Tomasz Duszynski <tomasz.duszynski@octakon.com>");
+MODULE_DESCRIPTION("Sensirion SCD30 carbon dioxide sensor i2c driver");
+MODULE_LICENSE("GPL v2");
-- 
2.26.2


^ permalink raw reply related

* [PATCH net-next v5 3/4] dt-bindings: net: Add RGMII internal delay for DP83869
From: Dan Murphy @ 2020-06-02 16:45 UTC (permalink / raw)
  To: andrew, f.fainelli, hkallweit1, davem, robh
  Cc: netdev, linux-kernel, devicetree, Dan Murphy
In-Reply-To: <20200602164522.3276-1-dmurphy@ti.com>

Add the internal delay values into the header and update the binding
with the internal delay properties.

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
 .../devicetree/bindings/net/ti,dp83869.yaml      | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/ti,dp83869.yaml b/Documentation/devicetree/bindings/net/ti,dp83869.yaml
index 5b69ef03bbf7..71e90a3e4652 100644
--- a/Documentation/devicetree/bindings/net/ti,dp83869.yaml
+++ b/Documentation/devicetree/bindings/net/ti,dp83869.yaml
@@ -8,7 +8,7 @@ $schema: "http://devicetree.org/meta-schemas/core.yaml#"
 title: TI DP83869 ethernet PHY
 
 allOf:
-  - $ref: "ethernet-controller.yaml#"
+  - $ref: "ethernet-phy.yaml#"
 
 maintainers:
   - Dan Murphy <dmurphy@ti.com>
@@ -64,6 +64,18 @@ properties:
        Operational mode for the PHY.  If this is not set then the operational
        mode is set by the straps. see dt-bindings/net/ti-dp83869.h for values
 
+  rx-internal-delay-ps:
+    description: Delay is in pico seconds
+    enum: [ 250, 500, 750, 1000, 1250, 1500, 1750, 2000, 2250, 2500, 2750, 3000,
+            3250, 3500, 3750, 4000 ]
+    default: 2000
+
+  tx-internal-delay-ps:
+    description: Delay is in pico seconds
+    enum: [ 250, 500, 750, 1000, 1250, 1500, 1750, 2000, 2250, 2500, 2750, 3000,
+            3250, 3500, 3750, 4000 ]
+    default: 2000
+
 required:
   - reg
 
@@ -80,5 +92,7 @@ examples:
         ti,op-mode = <DP83869_RGMII_COPPER_ETHERNET>;
         ti,max-output-impedance = "true";
         ti,clk-output-sel = <DP83869_CLK_O_SEL_CHN_A_RCLK>;
+        rx-internal-delay-ps = <2000>;
+        tx-internal-delay-ps = <2000>;
       };
     };
-- 
2.26.2


^ permalink raw reply related

* [PATCH net-next v5 4/4] net: dp83869: Add RGMII internal delay configuration
From: Dan Murphy @ 2020-06-02 16:45 UTC (permalink / raw)
  To: andrew, f.fainelli, hkallweit1, davem, robh
  Cc: netdev, linux-kernel, devicetree, Dan Murphy
In-Reply-To: <20200602164522.3276-1-dmurphy@ti.com>

Add RGMII internal delay configuration for Rx and Tx.

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
 drivers/net/phy/dp83869.c | 82 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 79 insertions(+), 3 deletions(-)

diff --git a/drivers/net/phy/dp83869.c b/drivers/net/phy/dp83869.c
index cfb22a21a2e6..ba1e3d599888 100644
--- a/drivers/net/phy/dp83869.c
+++ b/drivers/net/phy/dp83869.c
@@ -64,6 +64,10 @@
 #define DP83869_RGMII_TX_CLK_DELAY_EN		BIT(1)
 #define DP83869_RGMII_RX_CLK_DELAY_EN		BIT(0)
 
+/* RGMIIDCTL */
+#define DP83869_RGMII_CLK_DELAY_SHIFT		4
+#define DP83869_CLK_DELAY_DEF			7
+
 /* STRAP_STS1 bits */
 #define DP83869_STRAP_OP_MODE_MASK		GENMASK(2, 0)
 #define DP83869_STRAP_STS1_RESERVED		BIT(11)
@@ -78,9 +82,6 @@
 #define DP83869_PHYCR_FIFO_DEPTH_MASK	GENMASK(15, 12)
 #define DP83869_PHYCR_RESERVED_MASK	BIT(11)
 
-/* RGMIIDCTL bits */
-#define DP83869_RGMII_TX_CLK_DELAY_SHIFT	4
-
 /* IO_MUX_CFG bits */
 #define DP83869_IO_MUX_CFG_IO_IMPEDANCE_CTRL	0x1f
 
@@ -99,6 +100,10 @@
 #define DP83869_OP_MODE_MII			BIT(5)
 #define DP83869_SGMII_RGMII_BRIDGE		BIT(6)
 
+static const int dp83869_internal_delay[] = {250, 500, 750, 1000, 1250, 1500,
+					     1750, 2000, 2250, 2500, 2750, 3000,
+					     3250, 3500, 3750, 4000};
+
 enum {
 	DP83869_PORT_MIRRORING_KEEP,
 	DP83869_PORT_MIRRORING_EN,
@@ -108,6 +113,8 @@ enum {
 struct dp83869_private {
 	int tx_fifo_depth;
 	int rx_fifo_depth;
+	s32 rx_id_delay;
+	s32 tx_id_delay;
 	int io_impedance;
 	int port_mirroring;
 	bool rxctrl_strap_quirk;
@@ -232,6 +239,22 @@ static int dp83869_of_init(struct phy_device *phydev)
 				 &dp83869->tx_fifo_depth))
 		dp83869->tx_fifo_depth = DP83869_PHYCR_FIFO_DEPTH_4_B_NIB;
 
+	ret = of_property_read_u32(of_node, "rx-internal-delay-ps",
+				   &dp83869->rx_id_delay);
+	if (ret) {
+		dp83869->rx_id_delay =
+				dp83869_internal_delay[DP83869_CLK_DELAY_DEF];
+		ret = 0;
+	}
+
+	ret = of_property_read_u32(of_node, "tx-internal-delay-ps",
+				   &dp83869->tx_id_delay);
+	if (ret) {
+		dp83869->tx_id_delay =
+				dp83869_internal_delay[DP83869_CLK_DELAY_DEF];
+		ret = 0;
+	}
+
 	return ret;
 }
 #else
@@ -367,10 +390,35 @@ static int dp83869_configure_mode(struct phy_device *phydev,
 	return ret;
 }
 
+static int dp83869_get_delay(struct phy_device *phydev)
+{
+	struct dp83869_private *dp83869 = phydev->priv;
+	int delay_size = ARRAY_SIZE(dp83869_internal_delay);
+	int tx_delay;
+	int rx_delay;
+
+	tx_delay = phy_get_delay_index(phydev, &dp83869_internal_delay[0],
+				       delay_size, dp83869->tx_id_delay);
+	if (tx_delay < 0) {
+		phydev_err(phydev, "Tx internal delay is invalid\n");
+		return tx_delay;
+	}
+
+	rx_delay = phy_get_delay_index(phydev, &dp83869_internal_delay[0],
+				       delay_size, dp83869->rx_id_delay);
+	if (rx_delay < 0) {
+		phydev_err(phydev, "Rx internal delay is invalid\n");
+		return rx_delay;
+	}
+
+	return rx_delay | tx_delay << DP83869_RGMII_CLK_DELAY_SHIFT;
+}
+
 static int dp83869_config_init(struct phy_device *phydev)
 {
 	struct dp83869_private *dp83869 = phydev->priv;
 	int ret, val;
+	int delay;
 
 	ret = dp83869_configure_mode(phydev, dp83869);
 	if (ret)
@@ -394,6 +442,34 @@ static int dp83869_config_init(struct phy_device *phydev)
 				     dp83869->clk_output_sel <<
 				     DP83869_IO_MUX_CFG_CLK_O_SEL_SHIFT);
 
+	if (phy_interface_is_rgmii(phydev)) {
+		delay = dp83869_get_delay(phydev);
+		if (delay < 0)
+			return delay;
+
+		ret = phy_write_mmd(phydev, DP83869_DEVADDR, DP83869_RGMIIDCTL,
+				    delay);
+		if (ret)
+			return ret;
+
+		val = phy_read_mmd(phydev, DP83869_DEVADDR, DP83869_RGMIICTL);
+		val &= ~(DP83869_RGMII_TX_CLK_DELAY_EN |
+			 DP83869_RGMII_RX_CLK_DELAY_EN);
+
+		if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
+			val |= (DP83869_RGMII_TX_CLK_DELAY_EN |
+				DP83869_RGMII_RX_CLK_DELAY_EN);
+
+		if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID)
+			val |= DP83869_RGMII_TX_CLK_DELAY_EN;
+
+		if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID)
+			val |= DP83869_RGMII_RX_CLK_DELAY_EN;
+
+		ret = phy_write_mmd(phydev, DP83869_DEVADDR, DP83869_RGMIICTL,
+				    val);
+	}
+
 	return ret;
 }
 
-- 
2.26.2


^ permalink raw reply related

* [PATCH net-next v5 2/4] net: phy: Add a helper to return the index for of the internal delay
From: Dan Murphy @ 2020-06-02 16:45 UTC (permalink / raw)
  To: andrew, f.fainelli, hkallweit1, davem, robh
  Cc: netdev, linux-kernel, devicetree, Dan Murphy
In-Reply-To: <20200602164522.3276-1-dmurphy@ti.com>

Add a helper function that will return the index in the array for the
passed in internal delay value.  The helper requires the array, size and
delay value.

The helper will then return the index for the exact match or return the
index for the index to the closest smaller value.

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
 drivers/net/phy/phy_device.c | 51 ++++++++++++++++++++++++++++++++++++
 include/linux/phy.h          |  2 ++
 2 files changed, 53 insertions(+)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 04946de74fa0..5d4e7520b15e 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -2657,6 +2657,57 @@ void phy_get_pause(struct phy_device *phydev, bool *tx_pause, bool *rx_pause)
 }
 EXPORT_SYMBOL(phy_get_pause);
 
+/**
+ * phy_get_delay_index - returns the index of the internal delay
+ * @phydev: phy_device struct
+ * @delay_values: array of delays the PHY supports
+ * @size: the size of the delay array
+ * @int_delay: the internal delay to be looked up
+ *
+ * Returns the index within the array of internal delay passed in.
+ * The array must be in ascending order.
+ * Return errno if the delay is invalid or cannot be found.
+ */
+s32 phy_get_delay_index(struct phy_device *phydev, const int *delay_values,
+			int size, int int_delay)
+{
+	int i;
+
+	if (int_delay < 0)
+		return -EINVAL;
+
+	if (size <= 0)
+		return -EINVAL;
+
+	if (int_delay < delay_values[0] || int_delay > delay_values[size - 1]) {
+		phydev_err(phydev, "Delay %d is out of range\n", int_delay);
+		return -EINVAL;
+	}
+
+	if (int_delay == delay_values[0])
+		return 0;
+
+	for (i = 1; i < size; i++) {
+		if (int_delay == delay_values[i])
+			return i;
+
+		/* Find an approximate index by looking up the table */
+		if (int_delay > delay_values[i - 1] &&
+		    int_delay < delay_values[i]) {
+			if (int_delay - delay_values[i - 1] <
+			    delay_values[i] - int_delay)
+				return i - 1;
+			else
+				return i;
+		}
+	}
+
+	phydev_err(phydev, "error finding internal delay index for %d\n",
+		   int_delay);
+	return -EINVAL;
+}
+EXPORT_SYMBOL(phy_get_delay_index);
+
 static bool phy_drv_supports_irq(struct phy_driver *phydrv)
 {
 	return phydrv->config_intr && phydrv->ack_interrupt;
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 8c05d0fb5c00..a4327e6fd356 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1430,6 +1430,8 @@ void phy_set_asym_pause(struct phy_device *phydev, bool rx, bool tx);
 bool phy_validate_pause(struct phy_device *phydev,
 			struct ethtool_pauseparam *pp);
 void phy_get_pause(struct phy_device *phydev, bool *tx_pause, bool *rx_pause);
+int phy_get_delay_index(struct phy_device *phydev, const int *delay_values,
+			int size, int delay);
 void phy_resolve_pause(unsigned long *local_adv, unsigned long *partner_adv,
 		       bool *tx_pause, bool *rx_pause);
 
-- 
2.26.2


^ permalink raw reply related

* [PATCH net-next v5 1/4] dt-bindings: net: Add tx and rx internal delays
From: Dan Murphy @ 2020-06-02 16:45 UTC (permalink / raw)
  To: andrew, f.fainelli, hkallweit1, davem, robh
  Cc: netdev, linux-kernel, devicetree, Dan Murphy
In-Reply-To: <20200602164522.3276-1-dmurphy@ti.com>

tx-internal-delays and rx-internal-delays are a common setting for RGMII
capable devices.

These properties are used when the phy-mode or phy-controller is set to
rgmii-id, rgmii-rxid or rgmii-txid.  These modes indicate to the
controller that the PHY will add the internal delay for the connection.

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
 .../devicetree/bindings/net/ethernet-phy.yaml       | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
index 9b1f1147ca36..edd0245d132b 100644
--- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
+++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
@@ -162,6 +162,19 @@ properties:
     description:
       Specifies a reference to a node representing a SFP cage.
 
+
+  rx-internal-delay-ps:
+    $ref: /schemas/types.yaml#definitions/uint32
+    description: |
+      RGMII Receive PHY Clock Delay defined in pico seconds.  This is used for
+      PHY's that have configurable RX internal delays.
+
+  tx-internal-delay-ps:
+    $ref: /schemas/types.yaml#definitions/uint32
+    description: |
+      RGMII Transmit PHY Clock Delay defined in pico seconds.  This is used for
+      PHY's that have configurable TX internal delays.
+
 required:
   - reg
 
-- 
2.26.2


^ permalink raw reply related

* [PATCH net-next v5 0/4] RGMII Internal delay common property
From: Dan Murphy @ 2020-06-02 16:45 UTC (permalink / raw)
  To: andrew, f.fainelli, hkallweit1, davem, robh
  Cc: netdev, linux-kernel, devicetree, Dan Murphy

Hello

The RGMII internal delay is a common setting found in most RGMII capable PHY
devices.  It was found that many vendor specific device tree properties exist
to do the same function. This creates a common property to be used for PHY's
that have tunable internal delays for the Rx and Tx paths.

Dan Murphy (4):
  dt-bindings: net: Add tx and rx internal delays
  net: phy: Add a helper to return the index for of the internal delay
  dt-bindings: net: Add RGMII internal delay for DP83869
  net: dp83869: Add RGMII internal delay configuration

 .../devicetree/bindings/net/ethernet-phy.yaml | 13 +++
 .../devicetree/bindings/net/ti,dp83869.yaml   | 16 +++-
 drivers/net/phy/dp83869.c                     | 82 ++++++++++++++++++-
 drivers/net/phy/phy_device.c                  | 51 ++++++++++++
 include/linux/phy.h                           |  2 +
 5 files changed, 160 insertions(+), 4 deletions(-)

-- 
2.26.2


^ permalink raw reply

* [RESEND PATCH] dt-bindings: property-units: Add picoseconds type
From: Dan Murphy @ 2020-06-02 16:42 UTC (permalink / raw)
  To: robh; +Cc: linux-kernel, devicetree, Dan Murphy

Add the '-ps' picosecond unit suffix for property names.

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
 Documentation/devicetree/bindings/property-units.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/property-units.txt b/Documentation/devicetree/bindings/property-units.txt
index e9b8360b3288..00094070bdac 100644
--- a/Documentation/devicetree/bindings/property-units.txt
+++ b/Documentation/devicetree/bindings/property-units.txt
@@ -17,6 +17,7 @@ Time/Frequency
 -ms		: millisecond
 -us		: microsecond
 -ns		: nanosecond
+-ps		: picosecond
 
 Distance
 ----------------------------------------
-- 
2.26.2


^ permalink raw reply related

* Re: [PATCH] dt-bindings: property-units: Add picoseconds type
From: Dan Murphy @ 2020-06-02 16:40 UTC (permalink / raw)
  To: robh; +Cc: devicetree
In-Reply-To: <20200602163900.2556-1-dmurphy@ti.com>

Rob

On 6/2/20 11:39 AM, Dan Murphy wrote:
> Add the '-ps' picosecond unit suffix for property names.
>
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
>   Documentation/devicetree/bindings/property-units.txt | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/Documentation/devicetree/bindings/property-units.txt b/Documentation/devicetree/bindings/property-units.txt
> index e9b8360b3288..00094070bdac 100644
> --- a/Documentation/devicetree/bindings/property-units.txt
> +++ b/Documentation/devicetree/bindings/property-units.txt
> @@ -17,6 +17,7 @@ Time/Frequency
>   -ms		: millisecond
>   -us		: microsecond
>   -ns		: nanosecond
> +-ps		: picosecond
>   
>   Distance
>   ----------------------------------------

Need to resend I missed the linux-kernel on the maintainers.

Dan


^ permalink raw reply

* [PATCH] dt-bindings: property-units: Add picoseconds type
From: Dan Murphy @ 2020-06-02 16:39 UTC (permalink / raw)
  To: robh; +Cc: devicetree, Dan Murphy

Add the '-ps' picosecond unit suffix for property names.

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
 Documentation/devicetree/bindings/property-units.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/property-units.txt b/Documentation/devicetree/bindings/property-units.txt
index e9b8360b3288..00094070bdac 100644
--- a/Documentation/devicetree/bindings/property-units.txt
+++ b/Documentation/devicetree/bindings/property-units.txt
@@ -17,6 +17,7 @@ Time/Frequency
 -ms		: millisecond
 -us		: microsecond
 -ns		: nanosecond
+-ps		: picosecond
 
 Distance
 ----------------------------------------
-- 
2.26.2


^ permalink raw reply related

* Re: [PATCH] ARM: dts: AM33xx-l4: add gpio-ranges
From: Drew Fustini @ 2020-06-02 16:34 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Grygorii Strashko, linux-omap, linux-kernel, Benoît Cousson,
	Rob Herring, devicetree, Santosh Shilimkar, Suman Anna,
	Haojian Zhuang, Linus Walleij, linux-gpio, jkridner,
	robertcnelson
In-Reply-To: <20200602135155.GE37466@atomide.com>

On Tue, Jun 02, 2020 at 06:51:55AM -0700, Tony Lindgren wrote:
> * Grygorii Strashko <grygorii.strashko@ti.com> [200602 13:44]:
> > 
> > 
> > On 02/06/2020 16:14, Drew Fustini wrote:
> > > Add gpio-ranges properties to the gpio controller nodes.
> > > 
> > > These gpio-ranges were created based on "Table 9-10. CONTROL_MODULE
> > > REGISTERS" in the  "AM335x Technical Reference Manual" [0] and "Table
> > > 4-2. Pin Attributes" in the "AM335x Sitara Processor datasheet" [1].
> > > A csv file with this data is available for reference [2].
> > 
> > It will be good if you can explain not only "what" is changed, but
> > also "why" it's needed in commit message.
> 
> Also, please check (again) that this is the same for all the am3
> variants. For omap3, we had different pad assignments even between
> SoC revisions. Different pad routings should be easy to deal with
> in the dts if needed though.
> 
> Regards,
> 
> Tony

It appears that the only usage of am33xx-l4.dtsi is for am335x for which
specific parts mentioned in those dtsi files are 3352, 3358, and 3359.

$ git grep am33xx-l4.dtsi 
arch/arm/boot/dts/am33xx.dtsi:#include "am33xx-l4.dtsi"
$ git grep -l '#include "am33xx.dtsi"' arch/ |wc -l
27
$ git grep -l '#include "am33xx.dtsi"' arch/ |grep -v am335x |wc -l
0

Also, it appears that the only AM33xx parts that actually exist are [0]:

AM3351, AM3352, AM3354, AM3356, AM3357, AM3358, AM3359

I clicked on the datasheet link for each product page and while the URL
has the specific part number in it [1], they all end up loading the
exact same PDF. The header states:

"AM3359, AM3358, AM3357, AM3356, AM3354, AM3352, AM3351
SPRS717L – OCTOBER 2011 – REVISED MARCH 2020"

Thus, I do believe all SoC's using am33xx-l4.dtsi would have the same
memory map for the pin control registers and the same relationshop from
pin to gpio line.  For example, GPMC_A0 has mode 7 and it is labeled
gpio1_16.  conf_gpmc_a0 is at offset 840h which makes it pin 16.

Maybe am33xx-l4.dtsi should have actually been named am335x-l4.dtsi?

Though I suppose there is no point in changing that now.

thanks,
drew

[0] http://www.ti.com/processors/sitara-arm/am335x-cortex-a8/overview.html
[1] https://www.ti.com/lit/ds/symlink/am3359.pdf

^ permalink raw reply

* Re: [PATCH v5 11/11] PCI: qcom: Add Force GEN1 support
From: Bjorn Helgaas @ 2020-06-02 16:32 UTC (permalink / raw)
  To: Ansuel Smith
  Cc: Rob Herring, Sham Muthayyan, Rob Herring, Andy Gross,
	Bjorn Andersson, Bjorn Helgaas, Mark Rutland, Stanimir Varbanov,
	Lorenzo Pieralisi, Andrew Murray, Philipp Zabel, linux-arm-msm,
	linux-pci, devicetree, linux-kernel
In-Reply-To: <20200602115353.20143-12-ansuelsmth@gmail.com>

On Tue, Jun 02, 2020 at 01:53:52PM +0200, Ansuel Smith wrote:
> From: Sham Muthayyan <smuthayy@codeaurora.org>
> 
> Add Force GEN1 support needed in some ipq8064 board that needs to limit
> some PCIe line to gen1 for some hardware limitation. This is set by the
> max-link-speed binding and needed by some soc based on ipq8064. (for
> example Netgear R7800 router)
> 
> Signed-off-by: Sham Muthayyan <smuthayy@codeaurora.org>
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 259b627bf890..0ce15d53c46e 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -27,6 +27,7 @@
>  #include <linux/slab.h>
>  #include <linux/types.h>
>  
> +#include "../../pci.h"
>  #include "pcie-designware.h"
>  
>  #define PCIE20_PARF_SYS_CTRL			0x00
> @@ -99,6 +100,8 @@
>  #define PCIE20_v3_PARF_SLV_ADDR_SPACE_SIZE	0x358
>  #define SLV_ADDR_SPACE_SZ			0x10000000
>  
> +#define PCIE20_LNK_CONTROL2_LINK_STATUS2	0xa0
> +
>  #define DEVICE_TYPE_RC				0x4
>  
>  #define QCOM_PCIE_2_1_0_MAX_SUPPLY	3
> @@ -195,6 +198,7 @@ struct qcom_pcie {
>  	struct phy *phy;
>  	struct gpio_desc *reset;
>  	const struct qcom_pcie_ops *ops;
> +	int gen;
>  };
>  
>  #define to_qcom_pcie(x)		dev_get_drvdata((x)->dev)
> @@ -395,6 +399,11 @@ static int qcom_pcie_init_2_1_0(struct qcom_pcie *pcie)
>  	/* wait for clock acquisition */
>  	usleep_range(1000, 1500);
>  
> +	if (pcie->gen == 1) {
> +		val = readl(pci->dbi_base + PCIE20_LNK_CONTROL2_LINK_STATUS2);
> +		val |= 1;

Is this the same bit that's visible in config space as
PCI_EXP_LNKSTA_CLS_2_5GB?  Why not use that #define?

There are a bunch of other #defines in this file that look like
redefinitions of standard things:

  #define PCIE20_COMMAND_STATUS                   0x04
    Looks like PCI_COMMAND

  #define CMD_BME_VAL                             0x4
    Looks like PCI_COMMAND_MASTER

  #define PCIE20_DEVICE_CONTROL2_STATUS2          0x98
    Looks like (PCIE20_CAP + PCI_EXP_DEVCTL2)

  #define PCIE_CAP_CPL_TIMEOUT_DISABLE            0x10
    Looks like PCI_EXP_DEVCTL2_COMP_TMOUT_DIS

  #define PCIE20_CAP                              0x70
    This one is obviously device-specific

  #define PCIE20_CAP_LINK_CAPABILITIES            (PCIE20_CAP + 0xC)
    Looks like (PCIE20_CAP + PCI_EXP_LNKCAP)

  #define PCIE20_CAP_ACTIVE_STATE_LINK_PM_SUPPORT (BIT(10) | BIT(11))
    Looks like PCI_EXP_LNKCAP_ASPMS

  #define PCIE20_CAP_LINK_1                       (PCIE20_CAP + 0x14)
  #define PCIE_CAP_LINK1_VAL                      0x2FD7F
    This looks like PCIE20_CAP_LINK_1 should be (PCIE20_CAP +
    PCI_EXP_SLTCAP), but "CAP_LINK_1" doesn't sound like the Slot
    Capabilities register, and I don't know what PCIE_CAP_LINK1_VAL
    means.

> +		writel(val, pci->dbi_base + PCIE20_LNK_CONTROL2_LINK_STATUS2);
> +	}
>  
>  	/* Set the Max TLP size to 2K, instead of using default of 4K */
>  	writel(CFG_REMOTE_RD_REQ_BRIDGE_SIZE_2K,
> @@ -1397,6 +1406,10 @@ static int qcom_pcie_probe(struct platform_device *pdev)
>  		goto err_pm_runtime_put;
>  	}
>  
> +	pcie->gen = of_pci_get_max_link_speed(pdev->dev.of_node);
> +	if (pcie->gen < 0)
> +		pcie->gen = 2;
> +
>  	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "parf");
>  	pcie->parf = devm_ioremap_resource(dev, res);
>  	if (IS_ERR(pcie->parf)) {
> -- 
> 2.25.1
> 

^ permalink raw reply

* [PATCH 3/3] ARM: dts: NSP: Add support for Cisco Meraki MX65(W)
From: Matthew Hagan @ 2020-06-02 16:11 UTC (permalink / raw)
  Cc: Matthew Hagan, Florian Fainelli, Rob Herring, Ray Jui,
	Scott Branden, bcm-kernel-feedback-list, devicetree,
	linux-arm-kernel
In-Reply-To: <cover.1591114003.git.mnhagan88@gmail.com>

Hardware Info
-------------

Processor	- Broadcom BCM58625 dual-core @ 1.2 GHz
DDR3 RAM	- 2GB (4x SK Hynix H5TC4G83CFR)
Flash		- 1GB (Micron MT29F8G08ABACA)
Switches	- Broadcom BCM58625, 2x Qualcomm Atheros QCA8337
Ports		- 12 Ports
Wireless(MX65W)	- Broadcom BCM43520KMLG (2X)
Serial Port	- 115200 8n1
USB		- 1x 2.0

Tested with Kernel 5.4. PCIe is inactive on non-wireless MX65.

Note: The QCA8337 switches are connected to ports 4 and 5 of the BCM58625
SRAB, which need be set to SGMII mode.

Signed-off-by: Matthew Hagan <mnhagan88@gmail.com>
---
 arch/arm/boot/dts/bcm958625-mx65.dts   |  15 ++
 arch/arm/boot/dts/bcm958625-mx65w.dts  |  23 ++
 arch/arm/boot/dts/bcm958625-mx65x.dtsi | 321 +++++++++++++++++++++++++
 3 files changed, 359 insertions(+)
 create mode 100644 arch/arm/boot/dts/bcm958625-mx65.dts
 create mode 100644 arch/arm/boot/dts/bcm958625-mx65w.dts
 create mode 100644 arch/arm/boot/dts/bcm958625-mx65x.dtsi

diff --git a/arch/arm/boot/dts/bcm958625-mx65.dts b/arch/arm/boot/dts/bcm958625-mx65.dts
new file mode 100644
index 000000000000..af161d268824
--- /dev/null
+++ b/arch/arm/boot/dts/bcm958625-mx65.dts
@@ -0,0 +1,15 @@
+// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
+/*
+ * Device Tree Bindings for Cisco Meraki MX65.
+ *
+ * Copyright (C) 2020 Matthew Hagan <mnhagan88@gmail.com>
+ */
+
+/dts-v1/;
+
+#include "bcm958625-mx65x.dtsi"
+
+/ {
+	model = "Cisco Meraki MX65";
+	compatible = "meraki,mx65", "brcm,bcm58625", "brcm,nsp";
+};
diff --git a/arch/arm/boot/dts/bcm958625-mx65w.dts b/arch/arm/boot/dts/bcm958625-mx65w.dts
new file mode 100644
index 000000000000..67933ca7b598
--- /dev/null
+++ b/arch/arm/boot/dts/bcm958625-mx65w.dts
@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
+/*
+ * Device Tree Bindings for Cisco Meraki MX65W.
+ *
+ * Copyright (C) 2020 Matthew Hagan <mnhagan88@gmail.com>
+ */
+
+/dts-v1/;
+
+#include "bcm958625-mx65x.dtsi"
+
+/ {
+	model = "Cisco Meraki MX65W";
+	compatible = "meraki,mx65w", "brcm,bcm58625", "brcm,nsp";
+};
+
+&pcie0 {
+        status = "okay";
+};
+
+&pcie1 {
+        status = "okay";
+};
diff --git a/arch/arm/boot/dts/bcm958625-mx65x.dtsi b/arch/arm/boot/dts/bcm958625-mx65x.dtsi
new file mode 100644
index 000000000000..f69949be501e
--- /dev/null
+++ b/arch/arm/boot/dts/bcm958625-mx65x.dtsi
@@ -0,0 +1,321 @@
+// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
+/*
+ * Device Tree Bindings for Cisco Meraki MX65 series (Alamo).
+ *
+ * Copyright (C) 2020 Matthew Hagan <mnhagan88@gmail.com>
+ */
+
+/dts-v1/;
+
+#include "bcm958625-mx6x-common.dtsi"
+
+#include <dt-bindings/input/input.h>
+
+/ {
+	aliases {
+		mdio-mux-mmio = &mdiomux0;
+	};
+
+	leds {
+		compatible = "gpio-leds";
+
+		power_orange {
+			label = "power:orange";
+			gpios = <&gpioa 3 GPIO_ACTIVE_HIGH>;
+			default-state = "on";
+		};
+
+		lan_leds {
+			label = "lan:leds";
+			gpios = <&gpioa 12 GPIO_ACTIVE_HIGH>;
+		};
+
+		wan1_right {
+			label = "wan1:right";
+			gpios = <&gpioa 24 GPIO_ACTIVE_LOW>;
+		};
+
+		wan1_left {
+			label = "wan1:left";
+			gpios = <&gpioa 25 GPIO_ACTIVE_LOW>;
+		};
+
+		wan2_right {
+			label = "wan2:right";
+			gpios = <&gpioa 26 GPIO_ACTIVE_LOW>;
+		};
+
+		wan2_left {
+			label = "wan2:left";
+			gpios = <&gpioa 27 GPIO_ACTIVE_LOW>;
+		};
+
+		power_white {
+			label = "power:white";
+			gpios = <&gpioa 31 GPIO_ACTIVE_HIGH>;
+		};
+	};
+
+	gpio-buttons {
+		compatible = "gpio-keys-polled";
+		autorepeat;
+		poll-interval = <20>;
+
+		reset {
+			label = "reset";
+			linux,code = <KEY_RESTART>;
+			gpios = <&gpioa 8 GPIO_ACTIVE_LOW>;
+		};
+	};
+
+	mdio: mdio@18032000 {
+                compatible = "brcm,iproc-mdio";
+                reg = <0x18032000 0x8>;
+                #size-cells = <0>;
+                #address-cells = <1>;
+        };
+
+	mdiomux0: mdio-mux {
+		compatible = "mdio-mux-mmioreg";
+		reg = <0x18032000 0x4>;
+		mux-mask = <0x200>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		mdio-parent-bus = <&mdio>;
+
+		mdio_int: mdio@0 {
+			reg = <0x0>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+	};
+		mdio_ext: mdio@200 {
+			reg = <0x200>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};
+	};
+
+	mdio-mii-mux {
+		compatible = "mdio-mux-mmioreg";
+		reg = <0x1803f1c0 0x4>;
+		mux-mask = <0x2000>;
+		mdio-parent-bus = <&mdio_ext>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		mdio@0 {
+			reg = <0x0>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			phy_port6: phy@0 {
+				reg = <0>;
+			};
+
+			phy_port7: phy@1 {
+				reg = <1>;
+			};
+
+			phy_port8: phy@2 {
+				reg = <2>;
+			};
+
+			phy_port9: phy@3 {
+				reg = <3>;
+			};
+
+			phy_port10: phy@4 {
+				reg = <4>;
+			};
+
+			switch@10 {
+				compatible = "qca,qca8337";
+				#address-cells = <1>;
+				#size-cells = <0>;
+				reg = <0x10>;
+				dsa,member = <1 0>;
+
+				ports {
+					#address-cells = <1>;
+					#size-cells = <0>;
+					port@0 {
+						reg = <0>;
+						label = "cpu";
+						ethernet = <&sgmii1>;
+						phy-mode = "sgmii";
+						fixed-link {
+							speed = <1000>;
+							full-duplex;
+						};
+					};
+
+					port@1 {
+						reg = <1>;
+						label = "lan8";
+						phy-handle = <&phy_port6>;
+					};
+
+					port@2 {
+						reg = <2>;
+						label = "lan9";
+						phy-handle = <&phy_port7>;
+					};
+
+					port@3 {
+						reg = <3>;
+						label = "lan10";
+						phy-handle = <&phy_port8>;
+					};
+
+					port@4 {
+						reg = <4>;
+						label = "lan11";
+						phy-handle = <&phy_port9>;
+					};
+
+					port@5 {
+						reg = <5>;
+						label = "lan12";
+						phy-handle = <&phy_port10>;
+					};
+				};
+			};
+		};
+
+		mdio-mii@2000 {
+			reg = <0x2000>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			phy_port1: phy@0 {
+				reg = <0>;
+			};
+
+			phy_port2: phy@1 {
+				reg = <1>;
+			};
+
+			phy_port3: phy@2 {
+				reg = <2>;
+			};
+
+			phy_port4: phy@3 {
+				reg = <3>;
+			};
+
+			phy_port5: phy@4 {
+				reg = <4>;
+			};
+
+			switch@10 {
+				compatible = "qca,qca8337";
+				#address-cells = <1>;
+				#size-cells = <0>;
+				reg = <0x10>;
+				dsa,member = <2 0>;
+
+				ports {
+					#address-cells = <1>;
+					#size-cells = <0>;
+					port@0 {
+						reg = <0>;
+						label = "cpu";
+						ethernet = <&sgmii0>;
+						phy-mode = "sgmii";
+						fixed-link {
+							speed = <1000>;
+							full-duplex;
+						};
+					};
+
+					port@1 {
+						reg = <1>;
+						label = "lan3";
+						phy-handle = <&phy_port1>;
+					};
+
+					port@2 {
+						reg = <2>;
+						label = "lan4";
+						phy-handle = <&phy_port2>;
+					};
+
+					port@3 {
+						reg = <3>;
+						label = "lan5";
+						phy-handle = <&phy_port3>;
+					};
+
+					port@4 {
+						reg = <4>;
+						label = "lan6";
+						phy-handle = <&phy_port4>;
+					};
+
+					port@5 {
+						reg = <5>;
+						label = "lan7";
+						phy-handle = <&phy_port5>;
+					};
+				};
+			};
+		};
+	};
+};
+
+&srab {
+	compatible = "brcm,bcm58625-srab", "brcm,nsp-srab";
+	status = "okay";
+	dsa,member = <0 0>;
+
+	ports {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		port@0 {
+			label = "wan1";
+			reg = <0>;
+		};
+
+		port@1 {
+			label = "wan2";
+			reg = <1>;
+		};
+
+		sgmii0: port@4 {
+			label = "sw0";
+			reg = <4>;
+			fixed-link {
+			       speed = <1000>;
+			       full-duplex;
+		      };
+		};
+
+		sgmii1: port@5 {
+			label = "sw1";
+			reg = <5>;
+			fixed-link {
+				speed = <1000>;
+				full-duplex;
+			};
+		};
+
+		port@8 {
+			ethernet = <&amac2>;
+			label = "cpu";
+			reg = <8>;
+			fixed-link {
+				speed = <1000>;
+				full-duplex;
+			};
+		};
+	};
+};
+
+&pinctrl {
+	mdio_sel: mdio {
+		function = "mdio";
+		groups = "mdio_grp";
+	};
+};
-- 
2.25.4


^ permalink raw reply related

* [PATCH 0/3] ARM: dts: NSP: Add support for Cisco Meraki NSP devices
From: Matthew Hagan @ 2020-06-02 16:11 UTC (permalink / raw)
  Cc: Matthew Hagan, Florian Fainelli, Rob Herring, Ray Jui,
	Scott Branden, bcm-kernel-feedback-list, devicetree,
	linux-arm-kernel

This patch set adds support for the Meraki MX64(W) and MX65(W) security 
devices. There are four devices in total, all using the same basic hardware.

The MX64 series has five ethernet ports connected to the BCM SRAB. The MX65
series has two ports conected to the SRAB, and two QCA8337 switches connected 
by SGMII to SRAB ports 4 and 5, each providing five additional ports.

The W variants of these devices have two BCM43520s on the PCIe bus. On the
non-wireless variants PCIe is inactive, hence separate dts files.

1/3 contains common bindings for both Meraki devices.
2/3 contains MX64 specific bindings.
3/3 contains MX65 specific bindings.

Note that Chris Packham's "[PATCH 2/2] ARM: dts: NSP: avoid unnecessary probe
deferrals" is also necessary.

Thanks,
Matthew

Matthew Hagan (3):
  ARM: dts: NSP: Add common bindings for Meraki MX64/65
  ARM: dts: NSP: Add support for Cisco Meraki MX64(W)
  ARM: dts: NSP: Add support for Cisco Meraki MX65(W)

 arch/arm/boot/dts/bcm958625-mx64.dts         |  15 +
 arch/arm/boot/dts/bcm958625-mx64w.dts        |  23 ++
 arch/arm/boot/dts/bcm958625-mx64x.dtsi       | 136 ++++++++
 arch/arm/boot/dts/bcm958625-mx65.dts         |  15 +
 arch/arm/boot/dts/bcm958625-mx65w.dts        |  23 ++
 arch/arm/boot/dts/bcm958625-mx65x.dtsi       | 321 +++++++++++++++++++
 arch/arm/boot/dts/bcm958625-mx6x-common.dtsi | 172 ++++++++++
 7 files changed, 705 insertions(+)
 create mode 100644 arch/arm/boot/dts/bcm958625-mx64.dts
 create mode 100644 arch/arm/boot/dts/bcm958625-mx64w.dts
 create mode 100644 arch/arm/boot/dts/bcm958625-mx64x.dtsi
 create mode 100644 arch/arm/boot/dts/bcm958625-mx65.dts
 create mode 100644 arch/arm/boot/dts/bcm958625-mx65w.dts
 create mode 100644 arch/arm/boot/dts/bcm958625-mx65x.dtsi
 create mode 100644 arch/arm/boot/dts/bcm958625-mx6x-common.dtsi

-- 
2.25.4


^ permalink raw reply

* [PATCH 2/3] ARM: dts: NSP: Add support for Cisco Meraki MX64(W)
From: Matthew Hagan @ 2020-06-02 16:11 UTC (permalink / raw)
  Cc: Matthew Hagan, Florian Fainelli, Rob Herring, Ray Jui,
	Scott Branden, bcm-kernel-feedback-list, devicetree,
	linux-arm-kernel
In-Reply-To: <cover.1591114003.git.mnhagan88@gmail.com>

Hardware Info
-------------

Processor	- Broadcom BCM58625 dual-core @ 1.2 GHz
DDR3 RAM	- 2GB (4x SK Hynix H5TC4G83CFR)
Flash		- 1GB (Micron MT29F8G08ABACA)
Switch		- Broadcom BCM58625
Wireless(MX64W)	- Broadcom BCM43520KMLG (2x)
Ports		- 5 Ports
Serial Port	- 115200 8n1
USB		- 1x 2.0

Tested with Kernel 5.4. PCIe is inactive on the non-wireless MX64.

Signed-off-by: Matthew Hagan <mnhagan88@gmail.com>
---
 arch/arm/boot/dts/bcm958625-mx64.dts   |  15 +++
 arch/arm/boot/dts/bcm958625-mx64w.dts  |  23 +++++
 arch/arm/boot/dts/bcm958625-mx64x.dtsi | 136 +++++++++++++++++++++++++
 3 files changed, 174 insertions(+)
 create mode 100644 arch/arm/boot/dts/bcm958625-mx64.dts
 create mode 100644 arch/arm/boot/dts/bcm958625-mx64w.dts
 create mode 100644 arch/arm/boot/dts/bcm958625-mx64x.dtsi

diff --git a/arch/arm/boot/dts/bcm958625-mx64.dts b/arch/arm/boot/dts/bcm958625-mx64.dts
new file mode 100644
index 000000000000..ec1017b8bf68
--- /dev/null
+++ b/arch/arm/boot/dts/bcm958625-mx64.dts
@@ -0,0 +1,15 @@
+// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
+/*
+ * Device Tree Bindings for Cisco Meraki MX64.
+ *
+ * Copyright (C) 2020 Matthew Hagan <mnhagan88@gmail.com>
+ */
+
+/dts-v1/;
+
+#include "bcm958625-mx64x.dtsi"
+
+/ {
+	model = "Cisco Meraki MX64";
+	compatible = "meraki,mx64", "brcm,bcm58625", "brcm,nsp";
+};
diff --git a/arch/arm/boot/dts/bcm958625-mx64w.dts b/arch/arm/boot/dts/bcm958625-mx64w.dts
new file mode 100644
index 000000000000..a3fbf0fed218
--- /dev/null
+++ b/arch/arm/boot/dts/bcm958625-mx64w.dts
@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
+/*
+ * Device Tree Bindings for Cisco Meraki MX64W.
+ *
+ * Copyright (C) 2020 Matthew Hagan <mnhagan88@gmail.com>
+ */
+
+/dts-v1/;
+
+#include "bcm958625-mx64x.dtsi"
+
+/ {
+	model = "Cisco Meraki MX64W";
+	compatible = "meraki,mx64w", "brcm,bcm58625", "brcm,nsp";
+};
+
+&pcie0 {
+        status = "okay";
+};
+
+&pcie1 {
+        status = "okay";
+};
diff --git a/arch/arm/boot/dts/bcm958625-mx64x.dtsi b/arch/arm/boot/dts/bcm958625-mx64x.dtsi
new file mode 100644
index 000000000000..4be3dd314beb
--- /dev/null
+++ b/arch/arm/boot/dts/bcm958625-mx64x.dtsi
@@ -0,0 +1,136 @@
+// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
+/*
+ * Device Tree Bindings for Cisco Meraki MX64 series (Kingpin).
+ *
+ * Copyright (C) 2020 Matthew Hagan <mnhagan88@gmail.com>
+ */
+
+/dts-v1/;
+
+#include "bcm958625-mx6x-common.dtsi"
+
+#include <dt-bindings/input/input.h>
+
+/ {
+	leds {
+		compatible = "gpio-leds";
+
+		power_orange {
+			label = "power:orange";
+			gpios = <&gpioa 0 GPIO_ACTIVE_LOW>;
+			default-state = "on";
+		};
+
+		lan1_right {
+			label = "lan1:right";
+			gpios = <&gpioa 18 GPIO_ACTIVE_LOW>;
+		};
+
+		lan1_left {
+			label = "lan1:left";
+			gpios = <&gpioa 19 GPIO_ACTIVE_LOW>;
+		};
+
+		lan2_right {
+			label = "lan2:right";
+			gpios = <&gpioa 20 GPIO_ACTIVE_LOW>;
+		};
+
+		lan2_left {
+			label = "lan2:left";
+			gpios = <&gpioa 24 GPIO_ACTIVE_LOW>;
+		};
+
+		lan3_right {
+			label = "lan3:right";
+			gpios = <&gpioa 25 GPIO_ACTIVE_LOW>;
+		};
+
+		lan3_left {
+			label = "lan3:left";
+			gpios = <&gpioa 26 GPIO_ACTIVE_LOW>;
+		};
+
+		lan4_right {
+			label = "lan4:right";
+			gpios = <&gpioa 27 GPIO_ACTIVE_LOW>;
+		};
+
+		lan4_left {
+			label = "lan4:left";
+			gpios = <&gpioa 28 GPIO_ACTIVE_LOW>;
+		};
+
+		wan0_right {
+			label = "wan0:right";
+			gpios = <&gpioa 29 GPIO_ACTIVE_LOW>;
+		};
+
+		wan0_left {
+			label = "wan0:left";
+			gpios = <&gpioa 30 GPIO_ACTIVE_LOW>;
+		};
+
+		power_white {
+			label = "power:white";
+			gpios = <&gpioa 31 GPIO_ACTIVE_HIGH>;
+		};
+	};
+
+	gpio-buttons {
+		compatible = "gpio-keys-polled";
+		autorepeat;
+		poll-interval = <20>;
+
+		reset {
+			label = "reset";
+			linux,code = <KEY_RESTART>;
+			gpios = <&gpioa 6 GPIO_ACTIVE_LOW>;
+		};
+	};
+};
+
+&srab {
+	compatible = "brcm,bcm58625-srab", "brcm,nsp-srab";
+	status = "okay";
+
+	ports {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		port@0 {
+			label = "lan1";
+			reg = <0>;
+		};
+
+		port@1 {
+			label = "lan2";
+			reg = <1>;
+		};
+
+		port@2 {
+			label = "lan3";
+			reg = <2>;
+		};
+
+		port@3 {
+			label = "lan4";
+			reg = <3>;
+		};
+
+		port@4 {
+			label = "wan0";
+			reg = <4>;
+		};
+
+		port@8 {
+			ethernet = <&amac2>;
+			label = "cpu";
+			reg = <8>;
+			fixed-link {
+				speed = <1000>;
+				full-duplex;
+			};
+		};
+	};
+};
-- 
2.25.4


^ permalink raw reply related

* [PATCH 1/3] ARM: dts: NSP: Add common bindings for Meraki MX64/65
From: Matthew Hagan @ 2020-06-02 16:11 UTC (permalink / raw)
  Cc: Matthew Hagan, Florian Fainelli, Rob Herring, Ray Jui,
	Scott Branden, bcm-kernel-feedback-list, devicetree,
	linux-arm-kernel
In-Reply-To: <cover.1591114003.git.mnhagan88@gmail.com>

Signed-off-by: Matthew Hagan <mnhagan88@gmail.com>
---
 arch/arm/boot/dts/bcm958625-mx6x-common.dtsi | 172 +++++++++++++++++++
 1 file changed, 172 insertions(+)
 create mode 100644 arch/arm/boot/dts/bcm958625-mx6x-common.dtsi

diff --git a/arch/arm/boot/dts/bcm958625-mx6x-common.dtsi b/arch/arm/boot/dts/bcm958625-mx6x-common.dtsi
new file mode 100644
index 000000000000..1e253dd0941a
--- /dev/null
+++ b/arch/arm/boot/dts/bcm958625-mx6x-common.dtsi
@@ -0,0 +1,172 @@
+// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
+/*
+ * Common Bindings for Cisco Meraki MX64 (Kingpin) and MX65 (Alamo) devices.
+ *
+ * Copyright (C) 2020 Matthew Hagan <mnhagan88@gmail.com>
+ */
+
+/dts-v1/;
+
+#include "bcm-nsp.dtsi"
+#include <dt-bindings/gpio/gpio.h>
+
+/ {
+	chosen {
+		stdout-path = "serial0:115200n8";
+	};
+
+	memory {
+		device_type = "memory";
+		reg = <0x60000000 0x80000000>;
+	};
+
+	pwm-leds {
+		compatible = "pwm-leds";
+
+		red {
+			label = "pwm:led:red";
+			pwms = <&pwm 1 50000>;
+		};
+
+		green {
+			label = "pwm:led:green";
+			pwms = <&pwm 2 50000>;
+		};
+
+		blue {
+			label = "pwm:led:blue";
+			pwms = <&pwm 3 50000>;
+		};
+	};
+};
+
+&L2 {
+        arm,io-coherent;
+        prefetch-data = <1>;
+        prefetch-instr = <1>;
+};
+
+&uart0 {
+	clock-frequency = <62500000>;
+	status = "okay";
+};
+
+&i2c0 {
+	status = "okay";
+	eeprom: at24@50 {
+		compatible = "atmel,24c64";
+		pagesize = <32>;
+		reg = <0x50>;
+	};
+};
+
+&amac2 {
+	status = "okay";
+};
+
+&nand {
+	nandcs@0 {
+		compatible = "brcm,nandcs";
+		reg = <0>;
+		nand-on-flash-bbt;
+
+		#address-cells = <1>;
+		#size-cells = <1>;
+
+		nand-ecc-strength = <24>;
+		nand-ecc-step-size = <1024>;
+
+		brcm,nand-oob-sector-size = <27>;
+
+		partition@0 {
+			label = "U-boot";
+			reg = <0x00 0x80000>;
+			read-only;
+		};
+
+		partition@80000 {
+			label = "Shmoo";
+			reg = <0x80000 0x80000>;
+			read-only;
+		};
+
+		partition@100000 {
+			label = "bootkernel1";
+			reg = <0x100000 0x300000>;
+		};
+
+		partition@400000 {
+			label = "senao_nvram";
+			reg = <0x400000 0x100000>;
+		};
+
+		partition@500000 {
+			label = "bootkernel2";
+			reg = <0x500000 0x300000>;
+		};
+
+		partition@800000 {
+			label = "ubi";
+			reg = <0x800000 0x3f700000>;
+		};
+	};
+};
+
+&ehci0 {
+	status = "okay";
+};
+
+&ohci0 {
+	status = "okay";
+};
+
+&pwm {
+	status = "okay";
+	#pwm-cells = <2>;
+	chan0 {
+		channel = <1>;
+		active_low = <1>;
+		};
+	chan1 {
+		channel = <2>;
+		active_low = <1>;
+	};
+	chan2 {
+		channel = <3>;
+		active_low = <1>;
+	};
+};
+
+&ccbtimer1 {
+	status = "disabled";
+};
+
+&pinctrl {
+	pinctrl-names = "default";
+	pinctrl-0 = <&nand_sel>, <&gpiobs>, <&pwmc>;
+
+	nand_sel: nand_sel {
+		function = "nand";
+		groups = "nand_grp";
+	};
+
+	gpiobs: gpiobs {
+		function = "gpio_b";
+		groups = "gpio_b_0_grp", "gpio_b_1_grp", "gpio_b_2_grp",
+			 "gpio_b_3_grp";
+	};
+
+	pwmc: pwmc {
+		function = "pwm";
+		groups = "pwm0_grp", "pwm1_grp", "pwm2_grp", "pwm3_grp";
+	};
+
+	i2c_sel: i2c {
+		function = "i2c";
+		groups = "i2c_grp";
+	};
+};
+
+&sata_phy {
+	status = "disabled";
+};
-- 
2.25.4


^ permalink raw reply related

* Re: [v2] drm/msm: add shutdown support for display platform_driver
From: Sai Prakash Ranjan @ 2020-06-02 16:10 UTC (permalink / raw)
  To: Emil Velikov
  Cc: Krishna Manikandan, ML dri-devel, linux-arm-msm, freedreno,
	devicetree, Linux-Kernel@Vger. Kernel. Org, Sean Paul, kalyan_t,
	Kristian H . Kristensen, mka, devicetree-owner
In-Reply-To: <CACvgo50b+m2+onak=AZfgihkBXEP9POjMR52087v==-puLdkQQ@mail.gmail.com>

Hi Emil,

On 2020-06-02 21:09, Emil Velikov wrote:
> On Tue, 2 Jun 2020 at 15:49, Sai Prakash Ranjan
> <saiprakash.ranjan@codeaurora.org> wrote:
>> 
>> Hi Emil,
>> 
>> On 2020-06-02 19:43, Emil Velikov wrote:
>> > Hi Krishna,
>> >
>> > On Tue, 2 Jun 2020 at 08:17, Krishna Manikandan
>> > <mkrishn@codeaurora.org> wrote:
>> >>
>> >> Define shutdown callback for display drm driver,
>> >> so as to disable all the CRTCS when shutdown
>> >> notification is received by the driver.
>> >>
>> >> This change will turn off the timing engine so
>> >> that no display transactions are requested
>> >> while mmu translations are getting disabled
>> >> during reboot sequence.
>> >>
>> >> Signed-off-by: Krishna Manikandan <mkrishn@codeaurora.org>
>> >>
>> > AFAICT atomics is setup in msm_drm_ops::bind and shutdown in
>> > msm_drm_ops::unbind.
>> >
>> > Are you saying that unbind never triggers? If so, then we should
>> > really fix that instead, since this patch seems more like a
>> > workaround.
>> >
>> 
>> Which path do you suppose that the unbind should be called from, 
>> remove
>> callback? Here we are talking about the drivers which are builtin, 
>> where
>> remove callbacks are not called from the driver core during
>> reboot/shutdown,
>> instead shutdown callbacks are called which needs to be defined in 
>> order
>> to
>> trigger unbind. So AFAICS there is nothing to be fixed.
>> 
> Interesting - in drm effectively only drm panels implement .shutdown.
> So my naive assumption was that .remove was used implicitly by core,
> as part of the shutdown process. Yet that's not the case, so it seems
> that many drivers could use some fixes.
> 
> Then again, that's an existing problem which is irrelevant for msm.
> -Emil

To give more context, we are actually targeting the clients/consumers
of SMMU/IOMMU here because we have to make sure that before the supplier
(SMMU) shuts down, its consumers/clients need to be shutdown properly.
Now the ordering of this is taken care in the SMMU driver via 
device_link
which makes sure that consumer shutdown callbacks are called first, but 
we
need to define shutdown callbacks for all its consumers to make sure we
actually shutdown the clients or else it would invite the crashes during 
reboot
which in this case was seen for display.

Thanks,
Sai

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a 
member
of Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply

* Re: [PATCH 1/2] arm64: dts: Add a device tree for the Librem5 phone
From: Martin Kepplinger @ 2020-06-02 15:44 UTC (permalink / raw)
  To: Marco Felsch
  Cc: robh, kernel, shawnguo, s.hauer, kernel, festevam, linux-imx,
	mchehab, Anson.Huang, agx, angus, devicetree, linux-kernel,
	linux-arm-kernel
In-Reply-To: <20200527093538.xqtdoybl5hx27ccv@pengutronix.de>

Hi Marco,

On 27.05.20 11:35, Marco Felsch wrote:
> Hi Martin,
> 
> On 20-05-14 17:57, Martin Kepplinger wrote:
>> From: "Angus Ainslie (Purism)" <angus@akkea.ca>
>>
>> Add a devicetree description for the Librem 5 phone. The early batches
>> that have been sold are supported as well as the mass-produced device
>> available later this year, see https://puri.sm/products/librem-5/
>>
>> This boots to a working console with working WWAN modem, wifi usdhc,
>> IMU sensor device, proximity sensor, haptic motor, gpio keys, GNSS and LEDs.
>>
>> Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm>
>> Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
>> Signed-off-by: Guido Günther <agx@sigxcpu.org>
>> ---
>>  arch/arm64/boot/dts/freescale/Makefile        |    1 +
>>  .../boot/dts/freescale/imx8mq-librem5.dts     | 1174 +++++++++++++++++
>>  2 files changed, 1175 insertions(+)
>>  create mode 100644 arch/arm64/boot/dts/freescale/imx8mq-librem5.dts
>>
>> diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
>> index cd38d04da5a7..342579121f98 100644
>> --- a/arch/arm64/boot/dts/freescale/Makefile
>> +++ b/arch/arm64/boot/dts/freescale/Makefile
>> @@ -34,6 +34,7 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mn-ddr4-evk.dtb
>>  dtb-$(CONFIG_ARCH_MXC) += imx8mp-evk.dtb
>>  dtb-$(CONFIG_ARCH_MXC) += imx8mq-evk.dtb
>>  dtb-$(CONFIG_ARCH_MXC) += imx8mq-hummingboard-pulse.dtb
>> +dtb-$(CONFIG_ARCH_MXC) += imx8mq-librem5.dtb
>>  dtb-$(CONFIG_ARCH_MXC) += imx8mq-librem5-devkit.dtb
>>  dtb-$(CONFIG_ARCH_MXC) += imx8mq-nitrogen.dtb
>>  dtb-$(CONFIG_ARCH_MXC) += imx8mq-phanbell.dtb
>> diff --git a/arch/arm64/boot/dts/freescale/imx8mq-librem5.dts b/arch/arm64/boot/dts/freescale/imx8mq-librem5.dts
>> new file mode 100644
>> index 000000000000..95c105b4c120
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/freescale/imx8mq-librem5.dts
>> @@ -0,0 +1,1174 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Copyright 2018-2020 Purism SPC
>> + */
>> +
>> +/dts-v1/;
>> +
>> +#include "dt-bindings/input/input.h"
>> +#include "dt-bindings/pwm/pwm.h"
>> +#include "dt-bindings/usb/pd.h"
>> +#include "imx8mq.dtsi"
>> +
>> +/ {
>> +	model = "Purism Librem 5";
>> +	compatible = "purism,librem5", "fsl,imx8mq";
>> +
>> +	backlight_dsi: backlight-dsi {
>> +		compatible = "led-backlight";
>> +		leds = <&led_backlight>;
>> +		brightness-levels = <255>;
>> +		default-brightness-level = <100>;
>> +	};
>> +
>> +	bm818_codec: sound-wwan-codec {
>> +		compatible = "broadmobi,bm818", "option,gtm601";
>> +		#sound-dai-cells = <0>;
>> +	};
> 
> Please sort the node names alpabetical.
> 
>> +
>> +	chosen {
>> +		stdout-path = &uart1;
>> +	};
>> +
>> +	gpio-keys {
>> +		compatible = "gpio-keys";
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&pinctrl_keys>, <&pinctrl_hp>;
>> +
>> +		hp-det {
>> +			label = "HP_DET";
>> +			gpios = <&gpio3 9 GPIO_ACTIVE_HIGH>;
>> +			wakeup-source;
>> +			linux,code = <KEY_HP>;
> 
> Nit: I would add the wakeup-source behind the linux,code.
> 
>> +		};
>> +
>> +		vol-down {
>> +			label = "VOL_DOWN";
>> +			gpios = <&gpio1 17 GPIO_ACTIVE_LOW>;
>> +			linux,code = <KEY_VOLUMEDOWN>;
>> +		};
>> +
>> +		vol-up {
>> +			label = "VOL_UP";
>> +			gpios = <&gpio1 16 GPIO_ACTIVE_LOW>;
>> +			linux,code = <KEY_VOLUMEUP>;
>> +		};
>> +	};
>> +
>> +	pwmleds {
>> +		compatible = "pwm-leds";
>> +
>> +		blue {
>> +			label = "phone:blue:front";
>> +			max-brightness = <248>;
>> +			pwms = <&pwm2 0 50000>;
>> +		};
>> +
>> +		green {
>> +			label = "phone:green:front";
>> +			max-brightness = <248>;
>> +			pwms = <&pwm4 0 50000>;
>> +		};
>> +
>> +		red {
>> +			label = "phone:red:front";
>> +			max-brightness = <248>;
>> +			pwms = <&pwm3 0 50000>;
>> +		};
>> +	};
>> +
>> +	pmic_osc: clock-pmic {
>> +		compatible = "fixed-clock";
>> +		#clock-cells = <0>;
>> +		clock-frequency = <32768>;
>> +		clock-output-names = "pmic_osc";
>> +	};
> 
> Please sort nodes alphabetical.
> 
>> +
>> +	reg_audio_pwr_en: regulator-audio-pwr-en {
>> +		compatible = "regulator-fixed";
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&pinctrl_audiopwr>;
>> +		regulator-name = "AUDIO_PWR_EN";
>> +		regulator-min-microvolt = <1800000>;
>> +		regulator-max-microvolt = <1800000>;
>> +		gpio = <&gpio1 4 GPIO_ACTIVE_HIGH>;
>> +		enable-active-high;
>> +		regulator-always-on;
> 
> Why should this regulator be always on? The wm8962.c driver can handle
> the regualtor enable/disable.

It can indeed be handled by the driver here. We keep it always on
because of issues with the display stack that are not yet part of this
description.

> 
>> +	};
>> +
>> +	reg_aud_1v8: regulator-audio-v1v8 {
> 				^
> 		     regulator-audio-1v8?
> 
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "aud_1v8";
> 
> Is it intended to use capitalized and no-capitalized regulator-name's?
> 
>> +		regulator-min-microvolt = <1800000>;
>> +		regulator-max-microvolt = <1800000>;
>> +		vin-supply = <&reg_audio_pwr_en>;
>> +	};
> 
> Can we squash regulator-audio-pwr-en and regulator-audio-v1v8?

I think we can.

> 
>> +
>> +	reg_gnss: regulator-gnss {
>> +		compatible = "regulator-fixed";
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&pinctrl_gnsspwr>;
>> +		regulator-name = "GNSS";
>> +		regulator-min-microvolt = <3300000>;
>> +		regulator-max-microvolt = <3300000>;
>> +		gpio = <&gpio3 12 GPIO_ACTIVE_HIGH>;
>> +		enable-active-high;
>> +	};
>> +
>> +	reg_hub: regulator-hub {
>> +		compatible = "regulator-fixed";
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&pinctrl_hub_pwr>;
>> +		regulator-name = "HUB";
>> +		regulator-min-microvolt = <3300000>;
>> +		regulator-max-microvolt = <3300000>;
>> +		gpio = <&gpio1 14 GPIO_ACTIVE_HIGH>;
>> +		enable-active-high;
>> +	};
>> +
>> +	reg_lcd_1v8: regulator-lcd-1v8 {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "lcd_1v8";
>> +		regulator-min-microvolt = <1800000>;
>> +		regulator-max-microvolt = <1800000>;
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&pinctrl_dsien>;
>> +		vin-supply = <&reg_vdd_1v8>;
>> +		enable-active-high;
>> +		gpio = <&gpio1 5 GPIO_ACTIVE_HIGH>;
>> +	};
> 
> This regulator is never used.
> 
>> +
>> +	reg_lcd_3v4: regulator-lcd-3v4 {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "lcd_3v4";
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&pinctrl_dsibiasen>;
>> +		vin-supply = <&reg_vsys_3v4>;
>> +		enable-active-high;
>> +		gpio = <&gpio1 20 GPIO_ACTIVE_HIGH>;
>> +	};
>> +
>> +	reg_vdd_sen: regulator-vdd-sen {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "vdd_sen";
>> +		regulator-min-microvolt = <3300000>;
>> +		regulator-max-microvolt = <3300000>;
>> +	};
>> +
>> +	reg_vdd_3v3: regulator-vdd-3v3 {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "vdd_3v3";
>> +		regulator-min-microvolt = <3300000>;
>> +		regulator-max-microvolt = <3300000>;
>> +	};
>> +
>> +	reg_vdd_1v8: regulator-vdd-1v8 {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "vdd_1v8";
>> +		regulator-min-microvolt = <1800000>;
>> +		regulator-max-microvolt = <1800000>;
>> +	};
>> +
>> +	reg_vsys_3v4: regulator-vsys-3v4 {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "vsys_3v4";
>> +		regulator-min-microvolt = <3400000>;
>> +		regulator-max-microvolt = <3400000>;
>> +		regulator-always-on;
>> +	};
>> +
>> +	reg_3v3_wifi: regulator-3v3-wifi {
> 			^
> 	reg_wifi_3v3: regulator-wifi-3v3?
> 
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "3v3_wifi";
>> +		regulator-min-microvolt = <3300000>;
>> +		regulator-max-microvolt = <3300000>;
>> +	};
>> +
>> +	sound {
>> +		compatible = "simple-audio-card";
>> +		simple-audio-card,name = "wm8962";
>> +		simple-audio-card,format = "i2s";
>> +		simple-audio-card,widgets =
>> +			"Headphone", "Headphone",
>> +			"Microphone", "Headset Mic",
>> +			"Microphone", "Digital Mic",
>> +			"Speaker", "Speaker";
>> +		simple-audio-card,routing =
>> +			"Headphone", "HPOUTL",
>> +			"Headphone", "HPOUTR",
>> +			"Speaker", "SPKOUTL",
>> +			"Speaker", "SPKOUTR",
>> +			"Headset Mic", "MICBIAS",
>> +			"IN3R", "Headset Mic",
>> +			"DMICDAT", "Digital Mic";
>> +		simple-audio-card,cpu {
>> +			sound-dai = <&sai2>;
>> +		};
>> +		simple-audio-card,codec {
>> +			sound-dai = <&codec>;
>> +			clocks = <&clk IMX8MQ_CLK_SAI2_ROOT>;
>> +			frame-master;
>> +			bitclock-master;
>> +		};
>> +	};
>> +
>> +	sound-wwan {
>> +		compatible = "simple-audio-card";
>> +		simple-audio-card,name = "MODEM";
>> +		simple-audio-card,format = "i2s";
>> +
>> +		simple-audio-card,cpu {
>> +			sound-dai = <&sai6>;
>> +			frame-inversion;
>> +		};
>> +
>> +		telephony_link_master: simple-audio-card,codec {
> 			^
> 		useless phandle?
>> +			sound-dai = <&bm818_codec>;
>> +			frame-master;
>> +			bitclock-master;
>> +		};
>> +	};
>> +
>> +	vibrator {
>> +		compatible = "pwm-vibrator";
>> +		pwms = <&pwm1 0 1000000000 0>;
>> +		pwm-names = "enable";
>> +		vcc-supply = <&reg_vdd_3v3>;
>> +	};
>> +};
>> +
>> +&A53_0 {
>> +	cpu-supply = <&buck2_reg>;
>> +};
>> +
>> +&A53_1 {
>> +	cpu-supply = <&buck2_reg>;
>> +};
>> +
>> +&A53_2 {
>> +	cpu-supply = <&buck2_reg>;
>> +};
>> +
>> +&A53_3 {
>> +	cpu-supply = <&buck2_reg>;
>> +};
>> +
>> +&clk {
>> +	assigned-clocks = <&clk IMX8MQ_AUDIO_PLL1>, <&clk IMX8MQ_AUDIO_PLL2>;
>> +	assigned-clock-rates = <786432000>, <722534400>;
>> +};
> 
> Either I would bundle all clock settings here or within the sai nodes.

You're right, I'll try to move them.

> 
>> +
>> +&ddrc {
>> +	operating-points-v2 = <&ddrc_opp_table>;
>> +
>> +	ddrc_opp_table: ddrc-opp-table {
>> +		compatible = "operating-points-v2";
>> +
>> +		opp-25M {
>> +			opp-hz = /bits/ 64 <25000000>;
>> +		};
>> +		opp-100M {
>> +			opp-hz = /bits/ 64 <100000000>;
>> +		};
>> +		opp-800M {
>> +			opp-hz = /bits/ 64 <800000000>;
>> +		};
>> +	};
>> +};
>> +
>> +&dphy {
>> +	status = "okay";
>> +};
>> +
>> +&ecspi1 {
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&pinctrl_ecspi1>;
>> +	cs-gpios = <&gpio5 9 GPIO_ACTIVE_HIGH>;
> 
> Missmatch with the pinctrl_ecspi1?
> 
>> +	status = "okay";
> 
> Status is always the last property.
> 
>> +	#address-cells = <1>;
>> +	#size-cells = <0>;
>> +
>> +	nor_flash: flash@0 {
>> +		compatible = "jedec,spi-nor";
>> +		spi-max-frequency = <1000000>;
>> +		reg = <0>;
>> +	};
>> +};
>> +
>> +&gpio1 {
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&pinctrl_pmic_5v>;
>> +
>> +	pmic-5v {
>> +		gpio-hog;
>> +		gpio = <&gpio1 1 GPIO_ACTIVE_HIGH>;
>> +		input;
>> +	};
>> +};
>> +
>> +&iomuxc {
>> +	pinctrl_audiopwr: audiopwrgrp {
>> +		fsl,pins = <
>> +			/* AUDIO_POWER_EN_3V3 */
>> +			MX8MQ_IOMUXC_GPIO1_IO04_GPIO1_IO4	0x83
>> +		>;
>> +	};
>> +
>> +	pinctrl_bl: blgrp {
>> +		fsl,pins = <
>> +			/* BACKLINGE_EN */
>> +			MX8MQ_IOMUXC_NAND_DQS_GPIO3_IO14	0x83
>> +		>;
>> +	};
>> +
>> +	pinctrl_charger_in: chargeringrp {
>> +		fsl,pins = <
>> +			/* CHRG_INT */
>> +			MX8MQ_IOMUXC_NAND_CE2_B_GPIO3_IO3	0x80
>> +			/* CHG_STATUS_B */
>> +			MX8MQ_IOMUXC_NAND_ALE_GPIO3_IO0		0x80
>> +		>;
>> +	};
>> +
>> +	pinctrl_dsibiasen: dsibiasengrp {
>> +		fsl,pins = <
>> +			/* DSI_BIAS_EN */
>> +			MX8MQ_IOMUXC_ENET_TD1_GPIO1_IO20	0x83
>> +		>;
>> +	};
>> +
>> +	pinctrl_dsien: dsiengrp {
>> +		fsl,pins = <
>> +			/* DSI_EN_3V3 */
>> +			MX8MQ_IOMUXC_GPIO1_IO05_GPIO1_IO5	0x83
>> +		>;
>> +	};
>> +
>> +	pinctrl_ecspi1: spi1grp {
>> +		fsl,pins = <
>> +			MX8MQ_IOMUXC_ECSPI1_MOSI_ECSPI1_MOSI	0x83
>> +			MX8MQ_IOMUXC_ECSPI1_MISO_ECSPI1_MISO	0x83
>> +			MX8MQ_IOMUXC_ECSPI1_SS0_GPIO5_IO9	0x19
>> +			MX8MQ_IOMUXC_ECSPI1_SCLK_ECSPI1_SCLK	0x83
>> +			/* SPI_SS1 */
>> +			MX8MQ_IOMUXC_UART4_RXD_GPIO5_IO28	0x19
>> +		>;
>> +	};
>> +
>> +	pinctrl_gauge: gaugegrp {
>> +		fsl,pins = <
>> +			/* BAT_LOW */
>> +			MX8MQ_IOMUXC_SAI5_RXC_GPIO3_IO20	0x80
>> +		>;
>> +	};
>> +
>> +	pinctrl_gnsspwr: gnsspwrgrp {
>> +		fsl,pins = <
>> +			/* GPS3V3_EN */
>> +			MX8MQ_IOMUXC_NAND_DATA06_GPIO3_IO12	0x83
>> +		>;
>> +	};
>> +
>> +	pinctrl_haptic: hapticgrp {
>> +		fsl,pins = <
>> +			/* MOTO */
>> +			MX8MQ_IOMUXC_SPDIF_EXT_CLK_PWM1_OUT	0x83
>> +		>;
>> +	};
>> +
>> +	pinctrl_hp: hpgrp {
>> +		fsl,pins = <
>> +			/* HEADPHONE_DET_1V8 */
>> +			MX8MQ_IOMUXC_NAND_DATA03_GPIO3_IO9	0x180
>> +		>;
>> +	};
>> +
>> +	pinctrl_hub_pwr: hubpwrgrp {
>> +		fsl,pins = <
>> +			/* HUB_PWR_3V3_EN */
>> +			MX8MQ_IOMUXC_GPIO1_IO14_GPIO1_IO14	0x83
>> +		>;
>> +	};
>> +
>> +	pinctrl_i2c1: i2c1grp {
>> +		fsl,pins = <
>> +			MX8MQ_IOMUXC_I2C1_SCL_I2C1_SCL		0x40000026
>> +			MX8MQ_IOMUXC_I2C1_SDA_I2C1_SDA		0x40000026
>> +		>;
>> +	};
>> +
>> +	pinctrl_i2c2: i2c2grp {
>> +		fsl,pins = <
>> +			MX8MQ_IOMUXC_I2C2_SCL_I2C2_SCL		0x40000026
>> +			MX8MQ_IOMUXC_I2C2_SDA_I2C2_SDA		0x40000026
>> +		>;
>> +	};
>> +
>> +	pinctrl_i2c3: i2c3grp {
>> +		fsl,pins = <
>> +			MX8MQ_IOMUXC_I2C3_SCL_I2C3_SCL		0x40000026
>> +			MX8MQ_IOMUXC_I2C3_SDA_I2C3_SDA		0x40000026
>> +		>;
>> +	};
>> +
>> +	pinctrl_i2c4: i2c4grp {
>> +		fsl,pins = <
>> +			MX8MQ_IOMUXC_I2C4_SCL_I2C4_SCL		0x40000026
>> +			MX8MQ_IOMUXC_I2C4_SDA_I2C4_SDA		0x40000026
>> +		>;
>> +	};
>> +
>> +	pinctrl_keys: keysgrp {
>> +		fsl,pins = <
>> +			/* 4G_WAKE */
>> +			MX8MQ_IOMUXC_NAND_RE_B_GPIO3_IO15	0x80
>> +			/* PWR_KEY */
>> +			MX8MQ_IOMUXC_NAND_CLE_GPIO3_IO5		0x01C0
> 
> gpio3 5/15 are never used was this intended?
> 
>> +			/* VOL- */
>> +			MX8MQ_IOMUXC_ENET_MDIO_GPIO1_IO17	0x01C0
>> +			/* VOL+ */
>> +			MX8MQ_IOMUXC_ENET_MDC_GPIO1_IO16	0x01C0
>> +		>;
>> +	};
>> +
>> +	pinctrl_led_b: ledbgrp {
>> +		fsl,pins = <
>> +			/* LED_B */
>> +			MX8MQ_IOMUXC_GPIO1_IO13_PWM2_OUT	0x06
>> +		>;
>> +	};
>> +
>> +	pinctrl_led_g: ledggrp {
>> +		fsl,pins = <
>> +			/* LED_G */
>> +			MX8MQ_IOMUXC_SAI3_MCLK_PWM4_OUT		0x06
>> +		>;
>> +	};
>> +
>> +	pinctrl_led_r: ledrgrp {
>> +		fsl,pins = <
>> +			/* LED_R */
>> +			MX8MQ_IOMUXC_SPDIF_TX_PWM3_OUT		0x06
>> +		>;
>> +	};
>> +
>> +	pinctrl_mag: maggrp {
>> +		fsl,pins = <
>> +			/* INT_MAG */
>> +			MX8MQ_IOMUXC_SAI5_RXD1_GPIO3_IO22	0x80
>> +		>;
>> +	};
>> +
>> +	pinctrl_pmic: pmicgrp {
>> +		fsl,pins = <
>> +			/* PMIC_NINT */
>> +			MX8MQ_IOMUXC_GPIO1_IO07_GPIO1_IO7	0x80
>> +		>;
>> +	};
>> +
>> +	pinctrl_pmic_5v: pmic5vgrp {
>> +		fsl,pins = <
>> +			/* PMIC_5V */
>> +			MX8MQ_IOMUXC_GPIO1_IO01_GPIO1_IO1	0x80
>> +		>;
>> +	};
>> +
>> +	pinctrl_prox: proxgrp {
>> +		fsl,pins = <
>> +			/* INT_LIGHT */
>> +			MX8MQ_IOMUXC_NAND_DATA01_GPIO3_IO7	0x80
>> +		>;
>> +	};
>> +
>> +	pinctrl_rtc: rtcgrp {
>> +		fsl,pins = <
>> +			/* RTC_INT */
>> +			MX8MQ_IOMUXC_GPIO1_IO09_GPIO1_IO9	0x80
>> +		>;
>> +	};
>> +
>> +	pinctrl_sai2: sai2grp {
>> +		fsl,pins = <
>> +			MX8MQ_IOMUXC_SAI2_TXD0_SAI2_TX_DATA0	0xd6
>> +			MX8MQ_IOMUXC_SAI2_TXFS_SAI2_TX_SYNC	0xd6
>> +			MX8MQ_IOMUXC_SAI2_MCLK_SAI2_MCLK	0xd6
>> +			MX8MQ_IOMUXC_SAI2_RXD0_SAI2_RX_DATA0	0xd6
>> +			MX8MQ_IOMUXC_SAI2_TXC_SAI2_TX_BCLK	0xd6
>> +		>;
>> +	};
>> +
>> +	pinctrl_sai6: sai6grp {
>> +		fsl,pins = <
>> +			MX8MQ_IOMUXC_SAI1_RXD5_SAI6_RX_DATA0	0xd6
>> +			MX8MQ_IOMUXC_SAI1_RXD6_SAI6_RX_SYNC	0xd6
>> +			MX8MQ_IOMUXC_SAI1_TXD4_SAI6_RX_BCLK	0xd6
>> +			MX8MQ_IOMUXC_SAI1_TXD5_SAI6_TX_DATA0	0xd6
>> +		>;
>> +	};
>> +
>> +	pinctrl_tcpc: tcpcgrp {
>> +		fsl,pins = <
>> +			/* TCPC_INT */
>> +			MX8MQ_IOMUXC_GPIO1_IO10_GPIO1_IO10	0x01C0
>> +		>;
>> +	};
>> +
>> +	pinctrl_typec: typecgrp {
>> +		fsl,pins = <
>> +			/* TYPEC_MUX_EN */
>> +			MX8MQ_IOMUXC_GPIO1_IO11_GPIO1_IO11	0x83
>> +		>;
>> +	};
>> +
>> +	pinctrl_uart1: uart1grp {
>> +		fsl,pins = <
>> +			MX8MQ_IOMUXC_UART1_RXD_UART1_DCE_RX	0x49
>> +			MX8MQ_IOMUXC_UART1_TXD_UART1_DCE_TX	0x49
>> +		>;
>> +	};
>> +
>> +	pinctrl_uart2: uart2grp {
>> +		fsl,pins = <
>> +			MX8MQ_IOMUXC_UART2_TXD_UART2_DCE_TX	0x49
>> +			MX8MQ_IOMUXC_UART2_RXD_UART2_DCE_RX	0x49
>> +		>;
>> +	};
>> +
>> +	pinctrl_uart3: uart3grp {
>> +		fsl,pins = <
>> +			MX8MQ_IOMUXC_UART3_RXD_UART3_DCE_RX	0x49
>> +			MX8MQ_IOMUXC_UART3_TXD_UART3_DCE_TX	0x49
>> +		>;
>> +	};
>> +
>> +	pinctrl_uart4: uart4grp {
>> +		fsl,pins = <
>> +			MX8MQ_IOMUXC_ECSPI2_SCLK_UART4_DCE_RX		0x49
>> +			MX8MQ_IOMUXC_ECSPI2_MOSI_UART4_DCE_TX		0x49
>> +			MX8MQ_IOMUXC_ECSPI2_MISO_UART4_DCE_CTS_B	0x49
>> +			MX8MQ_IOMUXC_ECSPI2_SS0_UART4_DCE_RTS_B		0x49
>> +		>;
>> +	};
>> +
>> +	pinctrl_usdhc1: usdhc1grp {
>> +		fsl,pins = <
>> +			MX8MQ_IOMUXC_SD1_CLK_USDHC1_CLK			0x83
>> +			MX8MQ_IOMUXC_SD1_CMD_USDHC1_CMD			0xc3
>> +			MX8MQ_IOMUXC_SD1_DATA0_USDHC1_DATA0		0xc3
>> +			MX8MQ_IOMUXC_SD1_DATA1_USDHC1_DATA1		0xc3
>> +			MX8MQ_IOMUXC_SD1_DATA2_USDHC1_DATA2		0xc3
>> +			MX8MQ_IOMUXC_SD1_DATA3_USDHC1_DATA3		0xc3
>> +			MX8MQ_IOMUXC_SD1_DATA4_USDHC1_DATA4		0xc3
>> +			MX8MQ_IOMUXC_SD1_DATA5_USDHC1_DATA5		0xc3
>> +			MX8MQ_IOMUXC_SD1_DATA6_USDHC1_DATA6		0xc3
>> +			MX8MQ_IOMUXC_SD1_DATA7_USDHC1_DATA7		0xc3
>> +			MX8MQ_IOMUXC_SD1_STROBE_USDHC1_STROBE		0x83
>> +			MX8MQ_IOMUXC_SD1_RESET_B_USDHC1_RESET_B		0xc1
>> +		>;
>> +	};
>> +
>> +	pinctrl_usdhc1_100mhz: usdhc1grp100mhz {
>> +		fsl,pins = <
>> +			MX8MQ_IOMUXC_SD1_CLK_USDHC1_CLK			0x8d
>> +			MX8MQ_IOMUXC_SD1_CMD_USDHC1_CMD			0xcd
>> +			MX8MQ_IOMUXC_SD1_DATA0_USDHC1_DATA0		0xcd
>> +			MX8MQ_IOMUXC_SD1_DATA1_USDHC1_DATA1		0xcd
>> +			MX8MQ_IOMUXC_SD1_DATA2_USDHC1_DATA2		0xcd
>> +			MX8MQ_IOMUXC_SD1_DATA3_USDHC1_DATA3		0xcd
>> +			MX8MQ_IOMUXC_SD1_DATA4_USDHC1_DATA4		0xcd
>> +			MX8MQ_IOMUXC_SD1_DATA5_USDHC1_DATA5		0xcd
>> +			MX8MQ_IOMUXC_SD1_DATA6_USDHC1_DATA6		0xcd
>> +			MX8MQ_IOMUXC_SD1_DATA7_USDHC1_DATA7		0xcd
>> +			MX8MQ_IOMUXC_SD1_STROBE_USDHC1_STROBE		0x8d
>> +			MX8MQ_IOMUXC_SD1_RESET_B_USDHC1_RESET_B		0xc1
>> +		>;
>> +	};
>> +
>> +	pinctrl_usdhc1_200mhz: usdhc1grp200mhz {
>> +		fsl,pins = <
>> +			MX8MQ_IOMUXC_SD1_CLK_USDHC1_CLK			0x9f
>> +			MX8MQ_IOMUXC_SD1_CMD_USDHC1_CMD			0xdf
>> +			MX8MQ_IOMUXC_SD1_DATA0_USDHC1_DATA0		0xdf
>> +			MX8MQ_IOMUXC_SD1_DATA1_USDHC1_DATA1		0xdf
>> +			MX8MQ_IOMUXC_SD1_DATA2_USDHC1_DATA2		0xdf
>> +			MX8MQ_IOMUXC_SD1_DATA3_USDHC1_DATA3		0xdf
>> +			MX8MQ_IOMUXC_SD1_DATA4_USDHC1_DATA4		0xdf
>> +			MX8MQ_IOMUXC_SD1_DATA5_USDHC1_DATA5		0xdf
>> +			MX8MQ_IOMUXC_SD1_DATA6_USDHC1_DATA6		0xdf
>> +			MX8MQ_IOMUXC_SD1_DATA7_USDHC1_DATA7		0xdf
>> +			MX8MQ_IOMUXC_SD1_STROBE_USDHC1_STROBE		0x9f
>> +			MX8MQ_IOMUXC_SD1_RESET_B_USDHC1_RESET_B		0xc1
>> +		>;
>> +	};
>> +
>> +	pinctrl_usdhc2: usdhc2grp {
>> +		fsl,pins = <
>> +			MX8MQ_IOMUXC_SD2_CD_B_USDHC2_CD_B	0x80
>> +			MX8MQ_IOMUXC_SD2_CLK_USDHC2_CLK		0x83
>> +			MX8MQ_IOMUXC_SD2_CMD_USDHC2_CMD		0xc3
>> +			MX8MQ_IOMUXC_SD2_DATA0_USDHC2_DATA0	0xc3
>> +			MX8MQ_IOMUXC_SD2_DATA1_USDHC2_DATA1	0xc3
>> +			MX8MQ_IOMUXC_SD2_DATA2_USDHC2_DATA2	0xc3
>> +			MX8MQ_IOMUXC_SD2_DATA3_USDHC2_DATA3	0xc3
>> +			MX8MQ_IOMUXC_SD2_RESET_B_USDHC2_RESET_B 0xc1
>> +		>;
>> +	};
>> +
>> +	pinctrl_usdhc2_100mhz: usdhc2grp100mhz {
>> +		fsl,pins = <
>> +			MX8MQ_IOMUXC_SD2_CD_B_USDHC2_CD_B	0x80
>> +			MX8MQ_IOMUXC_SD2_CLK_USDHC2_CLK		0x8d
>> +			MX8MQ_IOMUXC_SD2_CMD_USDHC2_CMD		0xcd
>> +			MX8MQ_IOMUXC_SD2_DATA0_USDHC2_DATA0	0xcd
>> +			MX8MQ_IOMUXC_SD2_DATA1_USDHC2_DATA1	0xcd
>> +			MX8MQ_IOMUXC_SD2_DATA2_USDHC2_DATA2	0xcd
>> +			MX8MQ_IOMUXC_SD2_DATA3_USDHC2_DATA3	0xcd
>> +			MX8MQ_IOMUXC_SD2_RESET_B_USDHC2_RESET_B 0xc1
>> +		>;
>> +	};
>> +
>> +	pinctrl_usdhc2_200mhz: usdhc2grp200mhz {
>> +		fsl,pins = <
>> +			MX8MQ_IOMUXC_SD2_CD_B_USDHC2_CD_B	0x80
>> +			MX8MQ_IOMUXC_SD2_CLK_USDHC2_CLK		0x9f
>> +			MX8MQ_IOMUXC_SD2_CMD_USDHC2_CMD		0xcf
>> +			MX8MQ_IOMUXC_SD2_DATA0_USDHC2_DATA0	0xcf
>> +			MX8MQ_IOMUXC_SD2_DATA1_USDHC2_DATA1	0xcf
>> +			MX8MQ_IOMUXC_SD2_DATA2_USDHC2_DATA2	0xcf
>> +			MX8MQ_IOMUXC_SD2_DATA3_USDHC2_DATA3	0xcf
>> +			MX8MQ_IOMUXC_SD2_RESET_B_USDHC2_RESET_B 0xc1
>> +		>;
>> +	};
>> +
>> +	pinctrl_wdog: wdoggrp {
>> +		fsl,pins = <
>> +			/* nWDOG */
>> +			MX8MQ_IOMUXC_GPIO1_IO02_WDOG1_WDOG_B	0x1f
>> +		>;
>> +	};
>> +};
>> +
>> +&i2c1 {
>> +	clock-frequency = <387000>;
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&pinctrl_i2c1>;
>> +	status = "okay";
>> +
>> +	typec_pd: usb-pd@3f {
>> +		compatible = "ti,tps6598x";
>> +		reg = <0x3f>;
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&pinctrl_typec>, <&pinctrl_tcpc>;
>> +		interrupt-parent = <&gpio1>;
>> +		interrupts = <10 IRQ_TYPE_LEVEL_LOW>;
>> +
>> +		connector {
>> +			ports {
>> +				#address-cells = <1>;
>> +				#size-cells = <0>;
>> +
>> +				port@0 {
>> +					reg = <0>;
>> +
>> +					usb_con_hs: endpoint {
>> +						remote-endpoint = <&typec_hs>;
>> +					};
>> +				};
>> +
>> +				port@1 {
>> +					reg = <1>;
>> +
>> +					usb_con_ss: endpoint {
>> +						remote-endpoint = <&typec_ss>;
>> +					};
>> +				};
>> +			};
>> +		};
>> +	};
>> +
>> +	pmic: pmic@4b {
>> +		compatible = "rohm,bd71837";
>> +		reg = <0x4b>;
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&pinctrl_pmic>;
>> +		clocks = <&pmic_osc>;
>> +		clock-names = "osc";
>> +		clock-output-names = "pmic_clk";
>> +		interrupt-parent = <&gpio1>;
>> +		interrupts = <7 GPIO_ACTIVE_LOW>;
>> +		interrupt-names = "irq";
>> +		rohm,reset-snvs-powered;
>> +
>> +		regulators {
>> +			buck1_reg: BUCK1 {
>> +				regulator-name = "buck1";
>> +				regulator-min-microvolt = <700000>;
>> +				regulator-max-microvolt = <1300000>;
>> +				regulator-ramp-delay = <1250>;
>> +				rohm,dvs-run-voltage = <900000>;
>> +				rohm,dvs-idle-voltage = <850000>;
>> +				rohm,dvs-suspend-voltage = <800000>;
>> +				regulator-always-on;
>> +			};
>> +
>> +			buck2_reg: BUCK2 {
>> +				regulator-name = "buck2";
>> +				regulator-min-microvolt = <700000>;
>> +				regulator-max-microvolt = <1300000>;
>> +				regulator-ramp-delay = <1250>;
>> +				rohm,dvs-run-voltage = <1000000>;
>> +				rohm,dvs-idle-voltage = <900000>;
>> +				regulator-always-on;
>> +			};
>> +
>> +			buck3_reg: BUCK3 {
>> +				regulator-name = "buck3";
>> +				regulator-min-microvolt = <700000>;
>> +				regulator-max-microvolt = <1300000>;
>> +				rohm,dvs-run-voltage = <900000>;
>> +				regulator-always-on;
>> +			};
>> +
>> +			buck4_reg: BUCK4 {
>> +				regulator-name = "buck4";
>> +				regulator-min-microvolt = <700000>;
>> +				regulator-max-microvolt = <1300000>;
>> +				rohm,dvs-run-voltage = <1000000>;
>> +			};
>> +
>> +			buck5_reg: BUCK5 {
>> +				regulator-name = "buck5";
>> +				regulator-min-microvolt = <700000>;
>> +				regulator-max-microvolt = <1350000>;
>> +				regulator-always-on;
>> +			};
>> +
>> +			buck6_reg: BUCK6 {
>> +				regulator-name = "buck6";
>> +				regulator-min-microvolt = <3000000>;
>> +				regulator-max-microvolt = <3300000>;
>> +				regulator-always-on;
>> +			};
>> +
>> +			buck7_reg: BUCK7 {
>> +				regulator-name = "buck7";
>> +				regulator-min-microvolt = <1605000>;
>> +				regulator-max-microvolt = <1995000>;
>> +				regulator-always-on;
>> +			};
>> +
>> +			buck8_reg: BUCK8 {
>> +				regulator-name = "buck8";
>> +				regulator-min-microvolt = <800000>;
>> +				regulator-max-microvolt = <1400000>;
>> +				regulator-always-on;
>> +			};
>> +
>> +			ldo1_reg: LDO1 {
>> +				regulator-name = "ldo1";
>> +				regulator-min-microvolt = <3000000>;
>> +				regulator-max-microvolt = <3300000>;
>> +				/* leave on for snvs power button */
>> +				regulator-always-on;
>> +			};
>> +
>> +			ldo2_reg: LDO2 {
>> +				regulator-name = "ldo2";
>> +				regulator-min-microvolt = <900000>;
>> +				regulator-max-microvolt = <900000>;
>> +				/* leave on for snvs power button */
>> +				regulator-always-on;
>> +			};
>> +
>> +			ldo3_reg: LDO3 {
>> +				regulator-name = "ldo3";
>> +				regulator-min-microvolt = <1800000>;
>> +				regulator-max-microvolt = <3300000>;
>> +				regulator-always-on;
>> +			};
>> +
>> +			ldo4_reg: LDO4 {
>> +				regulator-name = "ldo4";
>> +				regulator-min-microvolt = <900000>;
>> +				regulator-max-microvolt = <1800000>;
>> +				regulator-always-on;
>> +			};
>> +
>> +			ldo5_reg: LDO5 {
>> +				/* VDD_PHY_0V9 - MIPI and HDMI domains */
>> +				regulator-name = "ldo5";
>> +				regulator-min-microvolt = <1800000>;
>> +				regulator-max-microvolt = <3300000>;
>> +				regulator-always-on;
>> +			};
>> +
>> +			ldo6_reg: LDO6 {
>> +				/* VDD_PHY_0V9 - MIPI, HDMI and USB domains */
>> +				regulator-name = "ldo6";
>> +				regulator-min-microvolt = <900000>;
>> +				regulator-max-microvolt = <1800000>;
>> +				regulator-always-on;
>> +			};
>> +
>> +			ldo7_reg: LDO7 {
>> +				/* VDD_PHY_3V3 - USB domain */
>> +				regulator-name = "ldo7";
>> +				regulator-min-microvolt = <1800000>;
>> +				regulator-max-microvolt = <3300000>;
>> +				regulator-always-on;
>> +			};
> 
> Out of curiosity, why did you marked all regulators as
> regulator-always-on? I thought the librem5 is a smartphone.

It is. But we just currently see an unstable system in case we allow
turning most of them off. We do for the VPU, and should (afaik already)
be able to do so for the GPU. After more work in the drivers the mipi
regulator(s) should be able to be turned off. So we'll look at more and
more of these over time.

> 
>> +		};
>> +	};
>> +
>> +	rtc@68 {
>> +		compatible = "microcrystal,rv4162";
>> +		reg = <0x68>;
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&pinctrl_rtc>;
>> +		interrupt-parent = <&gpio1>;
>> +		interrupts = <9 IRQ_TYPE_LEVEL_LOW>;
>> +	};
>> +};
>> +
>> +&i2c2 {
>> +	clock-frequency = <387000>;
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&pinctrl_i2c2>;
>> +	status = "okay";
>> +
>> +	magnetometer@1e	{
>> +		compatible = "st,lsm9ds1-magn";
>> +		reg = <0x1e>;
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&pinctrl_mag>;
>> +		interrupt-parent = <&gpio3>;
>> +		interrupts = <22 IRQ_TYPE_LEVEL_HIGH>;
>> +		vdd-supply = <&reg_vdd_sen>;
>> +		vddio-supply = <&reg_vdd_1v8>;
>> +	};
>> +
>> +	regulator@3e {
>> +		compatible = "tps65132";
>> +		reg = <0x3e>;
>> +		reg_lcd_avdd: outp {
>> +			regulator-name = "lcd_avdd";
>> +			vin-supply = <&reg_lcd_3v4>;
>> +		};
>> +
>> +		reg_lcd_avee: outn {
>> +			regulator-name = "lcd_avee";
>> +			vin-supply = <&reg_lcd_3v4>;
>> +		};
> 		both phandles are not used.
>> +	};
>> +
>> +	flash@53 {
>> +		compatible = "lm3560";
>> +		reg = <0x53>;
>> +		#address-cells = <1>;
>> +		#size-cells = <0>;
>> +		flash@0 {
>> +			reg = <0x0>;
>> +			flash-timeout-us = <150000>;
>> +			flash-max-microamp = <320000>;
>> +			led-max-microamp = <60000>;
>> +			label = "lm3560:flash";
>> +		};
>> +
>> +		torch@1 {
>> +			reg = <0x1>;
>> +			led-max-microamp = <10000>;
>> +			label = "lm3560:torch";
>> +		};
>> +
>> +	};
>> +
>> +	prox@60 {
>> +		compatible = "vishay,vcnl4040";
>> +		reg = <0x60>;
>> +		pinctrl-0 = <&pinctrl_prox>;
>> +		interrupt-parent = <&gpio3>;
>> +		interrupts = <7 IRQ_TYPE_LEVEL_LOW>;
>> +	};
>> +
>> +	accel-gyro@6a	{
>> +		compatible = "st,lsm9ds1-imu";
>> +		reg = <0x6a>;
>> +		vdd-supply = <&reg_vdd_sen>;
>> +		vddio-supply = <&reg_vdd_1v8>;
>> +		mount-matrix =  "1",  "0",  "0",
>> +				"0",  "1",  "0",
>> +				"0",  "0", "-1";
>> +	};
>> +};
>> +
>> +&i2c3 {
>> +	clock-frequency = <387000>;
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&pinctrl_i2c3>;
>> +	status = "okay";
>> +
>> +	codec: wm8962@1a {
> 
> Please use generic names.
> 
>> +		compatible = "wlf,wm8962";
>> +		reg = <0x1a>; // 0x4a is the test address
>> +		clocks = <&clk IMX8MQ_CLK_SAI2_ROOT>;
>> +		assigned-clocks = <&clk IMX8MQ_CLK_SAI2>;
>> +		assigned-clock-parents = <&clk IMX8MQ_AUDIO_PLL1_OUT>;
>> +		assigned-clock-rates = <24576000>;
>> +		#sound-dai-cells = <0>;
>> +		mic-cfg = <0x200>;
>> +		DCVDD-supply = <&reg_aud_1v8>;
>> +		DBVDD-supply = <&reg_aud_1v8>;
>> +		AVDD-supply = <&reg_aud_1v8>;
>> +		CPVDD-supply = <&reg_aud_1v8>;
>> +		MICVDD-supply = <&reg_aud_1v8>;
>> +		PLLVDD-supply = <&reg_aud_1v8>;
>> +		SPKVDD1-supply = <&reg_vsys_3v4>;
>> +		SPKVDD2-supply = <&reg_vsys_3v4>;
>> +		gpio-cfg = <
>> +			0x0000 /* n/c */
>> +			0x0001 /* gpio2, 1: default */
>> +			0x0013 /* gpio3, 2: dmicclk */
>> +			0x0000 /* n/c, 3: default */
>> +			0x8014 /* gpio5, 4: dmic_dat */
>> +			0x0000 /* gpio6, 5: default */
>> +		>;
>> +		status = "okay";
> 
> status can be dropped
> 
>> +	};
>> +
>> +	backlight@36 {
>> +		compatible = "ti,lm36922";
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&pinctrl_bl>;
>> +		reg = <0x36>;
>> +		#address-cells = <1>;
>> +		#size-cells = <0>;
>> +
>> +		enable-gpios = <&gpio3 14 GPIO_ACTIVE_HIGH>;
>> +		vled-supply = <&reg_vsys_3v4>;
>> +		ti,ovp-microvolt = <25000000>;
>> +
>> +		led_backlight: led@0 {
>> +			reg = <0>;
>> +			label = "white:backlight_cluster";
>> +			linux,default-trigger = "backlight";
>> +			led-max-microamp = <20000>;
>> +		};
>> +	};
>> +
>> +	touchscreen@38 {
>> +		compatible = "edt,edt-ft5506";
>> +		reg = <0x38>;
>> +		interrupt-parent = <&gpio1>;
>> +		interrupts = <27 IRQ_TYPE_EDGE_FALLING>;
> 
> You need to mux the irq gpio.
> 
>> +		irq-gpios = <&gpio1 27 GPIO_ACTIVE_HIGH>;
> 
> irq-gpios is not supported by the driver. We only have a
> wake/reset-gpio.
> 
>> +		touchscreen-size-x = <720>;
>> +		touchscreen-size-y = <1440>;
>> +	};
>> +};
>> +
>> +&i2c4 {
>> +	clock-frequency = <387000>;
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&pinctrl_i2c4>;
>> +	status = "okay";
>> +
>> +	bat: fuel-gauge@36 {
>> +		compatible = "maxim,max17055";
>> +		interrupt-parent = <&gpio3>;
>> +		interrupts = <20 IRQ_TYPE_LEVEL_LOW>;
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&pinctrl_gauge>;
>> +		reg = <0x36>;
> 
> Please check that "reg" is always the 2nd property after the
> "compatible".
> 
>> +		maxim,over-heat-temp = <700>;
>> +		maxim,over-volt = <4500>;
>> +		maxim,rsns-microohm = <5000>;
>> +	};
>> +
>> +	charger@6a { /* bq25895 */
>> +		compatible = "ti,bq25890";
> 
> The compatible should be "ti,bq25895" if it is a bq25895. So we can drop
> the comment too.
> 
>> +		reg = <0x6a>;
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&pinctrl_charger_in>;
>> +		interrupt-parent = <&gpio3>;
>> +		interrupts = <3 IRQ_TYPE_EDGE_FALLING>;
>> +		phys = <&usb3_phy0>;
>> +		ti,battery-regulation-voltage = <4192000>; /* 4.192V */
>> +		ti,charge-current = <1600000>; /* 1.6A */
>> +		ti,termination-current = <66000>;  /* 66mA */
>> +		ti,precharge-current = <130000>; /* 130mA */
>> +		ti,minimum-sys-voltage = <3700000>; /* 3.7V */
>> +		ti,boost-voltage = <5000000>; /* 5V */
>> +		ti,boost-max-current = <50000>; /* 50mA */
>> +		ti,use-vinmin-threshold = <1>; /* enable VINDPM */
>> +		ti,vinmin-threshold = <3900000>; /* 3.9V */
> 
> I would only mention the units within a comment because comments like
> this begin to divergence after you fix something.
> 
> Regards,
>   Marco
> 

Basically I try to follow all your suggestions that I haven't commented
here, too.

thank you very much for this in-depth review. That helps a lot and I try
to put together a second revision soon.

                             martin


^ permalink raw reply

* Re: [v2] drm/msm: add shutdown support for display platform_driver
From: Emil Velikov @ 2020-06-02 15:39 UTC (permalink / raw)
  To: Sai Prakash Ranjan
  Cc: Krishna Manikandan, ML dri-devel, linux-arm-msm, freedreno,
	devicetree, Linux-Kernel@Vger. Kernel. Org, Sean Paul, kalyan_t,
	Kristian H . Kristensen, mka, devicetree-owner
In-Reply-To: <cd61dd742e73b89794fc1b812d9fdcd9@codeaurora.org>

On Tue, 2 Jun 2020 at 15:49, Sai Prakash Ranjan
<saiprakash.ranjan@codeaurora.org> wrote:
>
> Hi Emil,
>
> On 2020-06-02 19:43, Emil Velikov wrote:
> > Hi Krishna,
> >
> > On Tue, 2 Jun 2020 at 08:17, Krishna Manikandan
> > <mkrishn@codeaurora.org> wrote:
> >>
> >> Define shutdown callback for display drm driver,
> >> so as to disable all the CRTCS when shutdown
> >> notification is received by the driver.
> >>
> >> This change will turn off the timing engine so
> >> that no display transactions are requested
> >> while mmu translations are getting disabled
> >> during reboot sequence.
> >>
> >> Signed-off-by: Krishna Manikandan <mkrishn@codeaurora.org>
> >>
> > AFAICT atomics is setup in msm_drm_ops::bind and shutdown in
> > msm_drm_ops::unbind.
> >
> > Are you saying that unbind never triggers? If so, then we should
> > really fix that instead, since this patch seems more like a
> > workaround.
> >
>
> Which path do you suppose that the unbind should be called from, remove
> callback? Here we are talking about the drivers which are builtin, where
> remove callbacks are not called from the driver core during
> reboot/shutdown,
> instead shutdown callbacks are called which needs to be defined in order
> to
> trigger unbind. So AFAICS there is nothing to be fixed.
>
Interesting - in drm effectively only drm panels implement .shutdown.
So my naive assumption was that .remove was used implicitly by core,
as part of the shutdown process. Yet that's not the case, so it seems
that many drivers could use some fixes.

Then again, that's an existing problem which is irrelevant for msm.
-Emil

^ permalink raw reply

* Re: [PATCH resend 0/2] dts: keystone-k2g-evm: Display support
From: santosh.shilimkar @ 2020-06-02 15:18 UTC (permalink / raw)
  To: Tomi Valkeinen, Jyri Sarha, dri-devel, ssantosh, linux-arm-kernel,
	devicetree
  Cc: mark.rutland, praneeth, robh+dt, peter.ujfalusi, laurent.pinchart
In-Reply-To: <973b69f2-bbe1-3c1b-615f-751bb8d5d83e@ti.com>

On 6/2/20 1:13 AM, Tomi Valkeinen wrote:
> Hi Santosh,
> 
> On 14/02/2020 19:40, santosh.shilimkar@oracle.com wrote:
>> On 2/14/20 1:22 AM, Jyri Sarha wrote:
>>> Resend because the earlier recipient list was wrong.
>>>
>>> Now that drm/tidss is queued for mainline, lets add display support for
>>> k2g-evm. There is no hurry since tidss is out only in v5.7, but it
>>> should not harm to have the dts changes in place before that.
>>>
>>> Jyri Sarha (2):
>>>    ARM: dts: keystone-k2g: Add DSS node
>>>    ARM: dts: keystone-k2g-evm: add HDMI video support
>>>
>>>   arch/arm/boot/dts/keystone-k2g-evm.dts | 101 +++++++++++++++++++++++++
>>>   arch/arm/boot/dts/keystone-k2g.dtsi    |  22 ++++++
>>>   2 files changed, 123 insertions(+)
>>>
>> Ok. Will add this to the next queue.
> 
> What happened with this one? It used to be in linux-next, but now I 
> don't see it anymore.
> 
Pull request [1] was sent during last merge window. Thought it was
picked up but doesn't seems to be. Have sent question to arm-soc
maintainers.

Regards,
Santosh

[1] https://www.spinics.net/lists/arm-kernel/msg791224.html

^ permalink raw reply

* Re: [PATCH v3 104/105] dt-bindings: display: vc4: hdmi: Add BCM2711 HDMI controllers bindings
From: Maxime Ripard @ 2020-06-02 15:08 UTC (permalink / raw)
  To: Rob Herring
  Cc: Nicolas Saenz Julienne, Eric Anholt, dri-devel, linux-rpi-kernel,
	bcm-kernel-feedback-list, linux-arm-kernel, linux-kernel,
	Dave Stevenson, Tim Gover, Phil Elwell, devicetree
In-Reply-To: <20200529181833.GA2685451@bogus>

[-- Attachment #1: Type: text/plain, Size: 3676 bytes --]

Hi Rob,

On Fri, May 29, 2020 at 12:18:33PM -0600, Rob Herring wrote:
> On Wed, May 27, 2020 at 05:49:14PM +0200, Maxime Ripard wrote:
> > The HDMI controllers found in the BCM2711 SoC need some adjustments to the
> > bindings, especially since the registers have been shuffled around in more
> > register ranges.
> > 
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: devicetree@vger.kernel.org
> > Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> > ---
> >  Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml | 109 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 109 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml b/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml
> > new file mode 100644
> > index 000000000000..6091fe3d315b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml
> > @@ -0,0 +1,109 @@
> > +# SPDX-License-Identifier: GPL-2.0
> 
> Dual license...
> 
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/display/brcm,bcm2711-hdmi.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Broadcom BCM2711 HDMI Controller Device Tree Bindings
> > +
> > +maintainers:
> > +  - Eric Anholt <eric@anholt.net>
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - brcm,bcm2711-hdmi0
> > +      - brcm,bcm2711-hdmi1
> 
> What's the difference between the 2 blocks? 

The register layout and the lane mapping in the PHY change a bit.

> > +
> > +  reg:
> > +    items:
> > +      - description: HDMI controller register range
> > +      - description: DVP register range
> > +      - description: HDMI PHY register range
> > +      - description: Rate Manager register range
> > +      - description: Packet RAM register range
> > +      - description: Metadata RAM register range
> > +      - description: CSC register range
> > +      - description: CEC register range
> > +      - description: HD register range
> > +
> > +  reg-names:
> > +    items:
> > +      - const: hdmi
> > +      - const: dvp
> > +      - const: phy
> > +      - const: rm
> > +      - const: packet
> > +      - const: metadata
> > +      - const: csc
> > +      - const: cec
> > +      - const: hd
> > +
> > +  clocks:
> > +    description: The HDMI state machine clock
> > +
> > +  clock-names:
> > +    const: hdmi
> > +
> > +  ddc:
> > +    allOf:
> > +      - $ref: /schemas/types.yaml#/definitions/phandle
> > +    description: >
> > +      Phandle of the I2C controller used for DDC EDID probing
> 
> Goes in the connector.
> 
> And isn't the standard name ddc-i2c-bus?
> 
> > +
> > +  hpd-gpios:
> > +    description: >
> > +      The GPIO pin for the HDMI hotplug detect (if it doesn't appear
> > +      as an interrupt/status bit in the HDMI controller itself)
> 
> Goes in the connector.

If this was an entirely new binding, I would agree, but this is not
really the case here.

We discussed it already for the v2, and this binding is essentially the
same one than the bcm2835 HDMI controller.

I initially sent a patch adding conditionnals for the clocks and regs
differences too, and you asked to split the binding into a separate file
to simplify it a bit.

Supporting both the old binding, and the new one based on the connector
is going to make the code significantly more complicated, and I'm not
really sure why we would here.

Thanks!
Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply

* Re: [PATCH v3 006/105] dt-bindings: display: Convert VC4 bindings to schemas
From: Maxime Ripard @ 2020-06-02 15:00 UTC (permalink / raw)
  To: Rob Herring
  Cc: Nicolas Saenz Julienne, Eric Anholt, dri-devel, linux-rpi-kernel,
	bcm-kernel-feedback-list, linux-arm-kernel, linux-kernel,
	Dave Stevenson, Tim Gover, Phil Elwell, devicetree
In-Reply-To: <20200527191211.GA2559189@bogus>

[-- Attachment #1: Type: text/plain, Size: 4929 bytes --]

Hi Rob,

On Wed, May 27, 2020 at 01:12:11PM -0600, Rob Herring wrote:
> On Wed, May 27, 2020 at 05:47:36PM +0200, Maxime Ripard wrote:
> > The BCM283x SoCs have a display pipeline composed of several controllers
> > with device tree bindings that are supported by Linux.
> > 
> > Now that we have the DT validation in place, let's split into separate
> > files and convert the device tree bindings for those controllers to
> > schemas.
> > 
> > This is just a 1:1 conversion though, and some bindings were incomplete so
> > it results in example validation warnings that are going to be addressed in
> > the following patches.
> > 
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: devicetree@vger.kernel.org
> > Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> > ---
> >  Documentation/devicetree/bindings/display/brcm,bcm-vc4.txt              | 174 +------------------------------------------------------------------------
> >  Documentation/devicetree/bindings/display/brcm,bcm2835-dpi.yaml         |  66 +++++++++++++++++++++++++++-
> >  Documentation/devicetree/bindings/display/brcm,bcm2835-dsi0.yaml        |  73 ++++++++++++++++++++++++++++++-
> >  Documentation/devicetree/bindings/display/brcm,bcm2835-hdmi.yaml        |  75 +++++++++++++++++++++++++++++++-
> >  Documentation/devicetree/bindings/display/brcm,bcm2835-hvs.yaml         |  37 +++++++++++++++-
> >  Documentation/devicetree/bindings/display/brcm,bcm2835-pixelvalve0.yaml |  40 +++++++++++++++++-
> >  Documentation/devicetree/bindings/display/brcm,bcm2835-txp.yaml         |  37 +++++++++++++++-
> >  Documentation/devicetree/bindings/display/brcm,bcm2835-v3d.yaml         |  42 +++++++++++++++++-
> >  Documentation/devicetree/bindings/display/brcm,bcm2835-vc4.yaml         |  34 ++++++++++++++-
> >  Documentation/devicetree/bindings/display/brcm,bcm2835-vec.yaml         |  44 ++++++++++++++++++-
> >  MAINTAINERS                                                             |   2 +-
> >  11 files changed, 449 insertions(+), 175 deletions(-)
> >  delete mode 100644 Documentation/devicetree/bindings/display/brcm,bcm-vc4.txt
> >  create mode 100644 Documentation/devicetree/bindings/display/brcm,bcm2835-dpi.yaml
> >  create mode 100644 Documentation/devicetree/bindings/display/brcm,bcm2835-dsi0.yaml
> >  create mode 100644 Documentation/devicetree/bindings/display/brcm,bcm2835-hdmi.yaml
> >  create mode 100644 Documentation/devicetree/bindings/display/brcm,bcm2835-hvs.yaml
> >  create mode 100644 Documentation/devicetree/bindings/display/brcm,bcm2835-pixelvalve0.yaml
> >  create mode 100644 Documentation/devicetree/bindings/display/brcm,bcm2835-txp.yaml
> >  create mode 100644 Documentation/devicetree/bindings/display/brcm,bcm2835-v3d.yaml
> >  create mode 100644 Documentation/devicetree/bindings/display/brcm,bcm2835-vc4.yaml
> >  create mode 100644 Documentation/devicetree/bindings/display/brcm,bcm2835-vec.yaml
> 
> 
> > diff --git a/Documentation/devicetree/bindings/display/brcm,bcm2835-dsi0.yaml b/Documentation/devicetree/bindings/display/brcm,bcm2835-dsi0.yaml
> > new file mode 100644
> > index 000000000000..3887675f844e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/brcm,bcm2835-dsi0.yaml
> > @@ -0,0 +1,73 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/display/brcm,bcm2835-dsi0.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Broadcom VC4 (VideoCore4) DSI Controller
> > +
> > +maintainers:
> > +  - Eric Anholt <eric@anholt.net>
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - brcm,bcm2835-dsi0
> > +      - brcm,bcm2835-dsi1
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    items:
> > +      - description: The DSI PLL clock feeding the DSI analog PHY
> > +      - description: The DSI ESC clock
> > +      - description: The DSI pixel clock
> > +
> > +  clock-output-names: true
> > +    # FIXME: The meta-schemas don't seem to allow it for now
> > +    # items:
> > +    #   - description: The DSI byte clock for the PHY
> > +    #   - description: The DSI DDR2 clock
> > +    #   - description: The DSI DDR clock
> 
> Doesn't pattern work for you?
> 
> pattern: '^dsi[0-1]_byte$'

That's not really what I was trying to achieve. I don't think
clock-output-names should hardcode the values it expect, since the whole
point is to let the "user" (ie the DT) control the clock names. If these
were to be fixed, it wouldn't even be here in the first place.

I just wanted to have a description of the clocks to provide a name for,
but it looks like clock-output-names can't have an items below. I looked
at why, couldn't really find a reason, and forgot to tell you about it,
sorry

> Either way,
> 
> Reviewed-by: Rob Herring <robh@kernel.org>

Thanks!
Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply

* Re: [PATCH v1 2/6] drm: panel-simple: add Seiko 70WVW2T 7" simple panel
From: Emil Velikov @ 2020-06-02 14:55 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Sam Ravnborg,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Sebastian Reichel, dri-devel, Bjorn Andersson, Thierry Reding,
	Søren Andersen
In-Reply-To: <CAD=FV=VSyODjtVtEe6H46U6xNraD2LUUi+xt8cxraaqXom=64g@mail.gmail.com>

On Tue, 2 Jun 2020 at 01:31, Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Mon, Jun 1, 2020 at 1:33 AM Sam Ravnborg <sam@ravnborg.org> wrote:
> >
> > The Seiko 70WVW2T is a discontinued product, but may be used somewhere.
> > Tested on a proprietary product.
> >
> > Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> > Cc: Søren Andersen <san@skov.dk>
> > Cc: Thierry Reding <thierry.reding@gmail.com>
> > Cc: Sam Ravnborg <sam@ravnborg.org>
> > ---
> >  drivers/gpu/drm/panel/panel-simple.c | 28 ++++++++++++++++++++++++++++
> >  1 file changed, 28 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
> > index b067f66cea0e..8624bb80108c 100644
> > --- a/drivers/gpu/drm/panel/panel-simple.c
> > +++ b/drivers/gpu/drm/panel/panel-simple.c
> > @@ -3194,6 +3194,31 @@ static const struct panel_desc shelly_sca07010_bfn_lnn = {
> >         .bus_format = MEDIA_BUS_FMT_RGB666_1X18,
> >  };
> >
> > +static const struct drm_display_mode sii_70wvw2t_mode = {
> > +       .clock = 33000,
> > +       .hdisplay = 800,
> > +       .hsync_start = 800 + 256,
> > +       .hsync_end = 800 + 256 + 0,
> > +       .htotal = 800 + 256 + 0 + 0,
> > +       .vdisplay = 480,
> > +       .vsync_start = 480 + 0,
> > +       .vsync_end = 480 + 0 + 0,
> > +       .vtotal = 480 + 0 + 0 + 45,
>
> Important to have a "vrefresh"?
>
Ville posted a series (most of which already landed) getting removing
vrefresh all together. The overall idea is to compute it, in the rare
case it's needed.


>
> > +       .flags = DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC,
> > +};
> > +
> > +static const struct panel_desc sii_70wvw2t = {
> > +       .modes = &sii_70wvw2t_mode,
> > +       .num_modes = 1,
>
> Do we want "bpc = 6"?
>
The largest user of bpc is userspace - the data gets copied via the ioctls.

A secondary, and quite limited, user are drivers exposing the "max
bpc" connector property. From a quick look: amdgpu, the synopsys
dw-hdmi bridge and i915 do so. In case the data missing, atomics
assume a max 8 bpc.

>
> > +       .size = {
> > +               .width = 152,
> > +               .height = 91,
> > +       },
> > +       .bus_format = MEDIA_BUS_FMT_RGB888_1X24,
>
> Should this be a 666 format?  Random internet-found data sheet says
> 262K colors...

Good catch. Would be nice to have a spec sheet link (even if random)
in the commit message.

HTH
-Emil

^ permalink raw reply

* Re: [v2] drm/msm: add shutdown support for display platform_driver
From: Sai Prakash Ranjan @ 2020-06-02 14:49 UTC (permalink / raw)
  To: Emil Velikov
  Cc: Krishna Manikandan, ML dri-devel, linux-arm-msm, freedreno,
	devicetree, Linux-Kernel@Vger. Kernel. Org, Sean Paul, kalyan_t,
	Kristian H . Kristensen, mka, devicetree-owner
In-Reply-To: <CACvgo50eb5_jp_6B5tkV9cX_X2_y2Xnavu+wvUUhHN5FsV9hiw@mail.gmail.com>

Hi Emil,

On 2020-06-02 19:43, Emil Velikov wrote:
> Hi Krishna,
> 
> On Tue, 2 Jun 2020 at 08:17, Krishna Manikandan 
> <mkrishn@codeaurora.org> wrote:
>> 
>> Define shutdown callback for display drm driver,
>> so as to disable all the CRTCS when shutdown
>> notification is received by the driver.
>> 
>> This change will turn off the timing engine so
>> that no display transactions are requested
>> while mmu translations are getting disabled
>> during reboot sequence.
>> 
>> Signed-off-by: Krishna Manikandan <mkrishn@codeaurora.org>
>> 
> AFAICT atomics is setup in msm_drm_ops::bind and shutdown in
> msm_drm_ops::unbind.
> 
> Are you saying that unbind never triggers? If so, then we should
> really fix that instead, since this patch seems more like a
> workaround.
> 

Which path do you suppose that the unbind should be called from, remove
callback? Here we are talking about the drivers which are builtin, where
remove callbacks are not called from the driver core during 
reboot/shutdown,
instead shutdown callbacks are called which needs to be defined in order 
to
trigger unbind. So AFAICS there is nothing to be fixed.

msm_pdev_shutdown()
  platform_drv_shutdown()
   device_shutdown()
    kernel_restart_prepare()
     kernel_restart()
      __arm64_sys_reboot()

Thanks,
Sai

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a 
member
of Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply

* Re: [PATCH v8 0/5] support reserving crashkernel above 4G on arm64 kdump
From: John Donnelly @ 2020-06-02 14:41 UTC (permalink / raw)
  To: Prabhakar Kushwaha
  Cc: Bhupesh Sharma, Chen Zhou, Simon Horman, Devicetree List,
	Baoquan He, Will Deacon, Linux Doc Mailing List, Catalin Marinas,
	kexec mailing list, Linux Kernel Mailing List, Rob Herring,
	Ingo Molnar, Arnd Bergmann, guohanjun, James Morse,
	Thomas Gleixner, Prabhakar Kushwaha, RuiRui Yang,
	linux-arm-kernel
In-Reply-To: <CAJ2QiJJE-jeRL1HPUZCwi1LtV9CBMmYrsOaS6vX1R1sJ6Z1t8g@mail.gmail.com>



> On Jun 2, 2020, at 12:38 AM, Prabhakar Kushwaha <prabhakar.pkin@gmail.com> wrote:
> 
> On Tue, Jun 2, 2020 at 3:29 AM John Donnelly <john.p.donnelly@oracle.com> wrote:
>> 
>> Hi .  See below !
>> 
>>> On Jun 1, 2020, at 4:02 PM, Bhupesh Sharma <bhsharma@redhat.com> wrote:
>>> 
>>> Hi John,
>>> 
>>> On Tue, Jun 2, 2020 at 1:01 AM John Donnelly <John.P.donnelly@oracle.com> wrote:
>>>> 
>>>> Hi,
>>>> 
>>>> 
>>>> On 6/1/20 7:02 AM, Prabhakar Kushwaha wrote:
>>>>> Hi Chen,
>>>>> 
>>>>> On Thu, May 21, 2020 at 3:05 PM Chen Zhou <chenzhou10@huawei.com> wrote:
>>>>>> This patch series enable reserving crashkernel above 4G in arm64.
>>>>>> 
>>>>>> There are following issues in arm64 kdump:
>>>>>> 1. We use crashkernel=X to reserve crashkernel below 4G, which will fail
>>>>>> when there is no enough low memory.
>>>>>> 2. Currently, crashkernel=Y@X can be used to reserve crashkernel above 4G,
>>>>>> in this case, if swiotlb or DMA buffers are required, crash dump kernel
>>>>>> will boot failure because there is no low memory available for allocation.
>>>>>> 
>>>>>> To solve these issues, introduce crashkernel=X,low to reserve specified
>>>>>> size low memory.
>>>>>> Crashkernel=X tries to reserve memory for the crash dump kernel under
>>>>>> 4G. If crashkernel=Y,low is specified simultaneously, reserve spcified
>>>>>> size low memory for crash kdump kernel devices firstly and then reserve
>>>>>> memory above 4G.
>>>>>> 
>>>>>> When crashkernel is reserved above 4G in memory, that is, crashkernel=X,low
>>>>>> is specified simultaneously, kernel should reserve specified size low memory
>>>>>> for crash dump kernel devices. So there may be two crash kernel regions, one
>>>>>> is below 4G, the other is above 4G.
>>>>>> In order to distinct from the high region and make no effect to the use of
>>>>>> kexec-tools, rename the low region as "Crash kernel (low)", and add DT property
>>>>>> "linux,low-memory-range" to crash dump kernel's dtb to pass the low region.
>>>>>> 
>>>>>> Besides, we need to modify kexec-tools:
>>>>>> arm64: kdump: add another DT property to crash dump kernel's dtb(see [1])
>>>>>> 
>>>>>> The previous changes and discussions can be retrieved from:
>>>>>> 
>>>>>> Changes since [v7]
>>>>>> - Move x86 CRASH_ALIGN to 2M
>>>>>> Suggested by Dave and do some test, move x86 CRASH_ALIGN to 2M.
>>>>>> - Update Documentation/devicetree/bindings/chosen.txt
>>>>>> Add corresponding documentation to Documentation/devicetree/bindings/chosen.txt suggested by Arnd.
>>>>>> - Add Tested-by from Jhon and pk
>>>>>> 
>>>>>> Changes since [v6]
>>>>>> - Fix build errors reported by kbuild test robot.
>>>>>> 
>>>>>> Changes since [v5]
>>>>>> - Move reserve_crashkernel_low() into kernel/crash_core.c.
>>>>>> - Delete crashkernel=X,high.
>>>>>> - Modify crashkernel=X,low.
>>>>>> If crashkernel=X,low is specified simultaneously, reserve spcified size low
>>>>>> memory for crash kdump kernel devices firstly and then reserve memory above 4G.
>>>>>> In addition, rename crashk_low_res as "Crash kernel (low)" for arm64, and then
>>>>>> pass to crash dump kernel by DT property "linux,low-memory-range".
>>>>>> - Update Documentation/admin-guide/kdump/kdump.rst.
>>>>>> 
>>>>>> Changes since [v4]
>>>>>> - Reimplement memblock_cap_memory_ranges for multiple ranges by Mike.
>>>>>> 
>>>>>> Changes since [v3]
>>>>>> - Add memblock_cap_memory_ranges back for multiple ranges.
>>>>>> - Fix some compiling warnings.
>>>>>> 
>>>>>> Changes since [v2]
>>>>>> - Split patch "arm64: kdump: support reserving crashkernel above 4G" as
>>>>>> two. Put "move reserve_crashkernel_low() into kexec_core.c" in a separate
>>>>>> patch.
>>>>>> 
>>>>>> Changes since [v1]:
>>>>>> - Move common reserve_crashkernel_low() code into kernel/kexec_core.c.
>>>>>> - Remove memblock_cap_memory_ranges() i added in v1 and implement that
>>>>>> in fdt_enforce_memory_region().
>>>>>> There are at most two crash kernel regions, for two crash kernel regions
>>>>>> case, we cap the memory range [min(regs[*].start), max(regs[*].end)]
>>>>>> and then remove the memory range in the middle.
>>>>>> 
>>>>>> [1]: https://urldefense.com/v3/__http://lists.infradead.org/pipermail/kexec/2020-May/025128.html__;!!GqivPVa7Brio!LnTSARkCt0V0FozR0KmqooaH5ADtdXvs3mPdP3KRVqALmvSK2VmCkIPIhsaxbvpn1uM1$
>>>>>> [v1]: https://urldefense.com/v3/__https://lkml.org/lkml/2019/4/2/1174__;!!GqivPVa7Brio!LnTSARkCt0V0FozR0KmqooaH5ADtdXvs3mPdP3KRVqALmvSK2VmCkIPIhsaxbt0xN9PE$
>>>>>> [v2]: https://urldefense.com/v3/__https://lkml.org/lkml/2019/4/9/86__;!!GqivPVa7Brio!LnTSARkCt0V0FozR0KmqooaH5ADtdXvs3mPdP3KRVqALmvSK2VmCkIPIhsaxbub7yUQH$
>>>>>> [v3]: https://urldefense.com/v3/__https://lkml.org/lkml/2019/4/9/306__;!!GqivPVa7Brio!LnTSARkCt0V0FozR0KmqooaH5ADtdXvs3mPdP3KRVqALmvSK2VmCkIPIhsaxbnc4zPPV$
>>>>>> [v4]: https://urldefense.com/v3/__https://lkml.org/lkml/2019/4/15/273__;!!GqivPVa7Brio!LnTSARkCt0V0FozR0KmqooaH5ADtdXvs3mPdP3KRVqALmvSK2VmCkIPIhsaxbvsAsZLu$
>>>>>> [v5]: https://urldefense.com/v3/__https://lkml.org/lkml/2019/5/6/1360__;!!GqivPVa7Brio!LnTSARkCt0V0FozR0KmqooaH5ADtdXvs3mPdP3KRVqALmvSK2VmCkIPIhsaxbl24n-79$
>>>>>> [v6]: https://urldefense.com/v3/__https://lkml.org/lkml/2019/8/30/142__;!!GqivPVa7Brio!LnTSARkCt0V0FozR0KmqooaH5ADtdXvs3mPdP3KRVqALmvSK2VmCkIPIhsaxbs7r8G2a$
>>>>>> [v7]: https://urldefense.com/v3/__https://lkml.org/lkml/2019/12/23/411__;!!GqivPVa7Brio!LnTSARkCt0V0FozR0KmqooaH5ADtdXvs3mPdP3KRVqALmvSK2VmCkIPIhsaxbiFUH90G$
>>>>>> 
>>>>>> Chen Zhou (5):
>>>>>>  x86: kdump: move reserve_crashkernel_low() into crash_core.c
>>>>>>  arm64: kdump: reserve crashkenel above 4G for crash dump kernel
>>>>>>  arm64: kdump: add memory for devices by DT property, low-memory-range
>>>>>>  kdump: update Documentation about crashkernel on arm64
>>>>>>  dt-bindings: chosen: Document linux,low-memory-range for arm64 kdump
>>>>>> 
>>>>> We are getting "warn_alloc" [1] warning during boot of kdump kernel
>>>>> with bootargs as [2] of primary kernel.
>>>>> This error observed on ThunderX2  ARM64 platform.
>>>>> 
>>>>> It is observed with latest upstream tag (v5.7-rc3) with this patch set
>>>>> and https://urldefense.com/v3/__https://lists.infradead.org/pipermail/kexec/2020-May/025128.html__;!!GqivPVa7Brio!LnTSARkCt0V0FozR0KmqooaH5ADtdXvs3mPdP3KRVqALmvSK2VmCkIPIhsaxbiIAAlzu$
>>>>> Also **without** this patch-set
>>>>> "https://urldefense.com/v3/__https://www.spinics.net/lists/arm-kernel/msg806882.html__;!!GqivPVa7Brio!LnTSARkCt0V0FozR0KmqooaH5ADtdXvs3mPdP3KRVqALmvSK2VmCkIPIhsaxbjC6ujMA$"
>>>>> 
>>>>> This issue comes whenever crashkernel memory is reserved after 0xc000_0000.
>>>>> More details discussed earlier in
>>>>> https://urldefense.com/v3/__https://www.spinics.net/lists/arm-kernel/msg806882.html__;!!GqivPVa7Brio!LnTSARkCt0V0FozR0KmqooaH5ADtdXvs3mPdP3KRVqALmvSK2VmCkIPIhsaxbjC6ujMA$  without any
>>>>> solution
>>>>> 
>>>>> This patch-set is expected to solve similar kind of issue.
>>>>> i.e. low memory is only targeted for DMA, swiotlb; So above mentioned
>>>>> observation should be considered/fixed. .
>>>>> 
>>>>> --pk
>>>>> 
>>>>> [1]
>>>>> [   30.366695] DMI: Cavium Inc. Saber/Saber, BIOS
>>>>> TX2-FW-Release-3.1-build_01-2803-g74253a541a mm/dd/yyyy
>>>>> [   30.367696] NET: Registered protocol family 16
>>>>> [   30.369973] swapper/0: page allocation failure: order:6,
>>>>> mode:0x1(GFP_DMA), nodemask=(null),cpuset=/,mems_allowed=0
>>>>> [   30.369980] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.7.0-rc3+ #121
>>>>> [   30.369981] Hardware name: Cavium Inc. Saber/Saber, BIOS
>>>>> TX2-FW-Release-3.1-build_01-2803-g74253a541a mm/dd/yyyy
>>>>> [   30.369984] Call trace:
>>>>> [   30.369989]  dump_backtrace+0x0/0x1f8
>>>>> [   30.369991]  show_stack+0x20/0x30
>>>>> [   30.369997]  dump_stack+0xc0/0x10c
>>>>> [   30.370001]  warn_alloc+0x10c/0x178
>>>>> [   30.370004]  __alloc_pages_slowpath.constprop.111+0xb10/0xb50
>>>>> [   30.370006]  __alloc_pages_nodemask+0x2b4/0x300
>>>>> [   30.370008]  alloc_page_interleave+0x24/0x98
>>>>> [   30.370011]  alloc_pages_current+0xe4/0x108
>>>>> [   30.370017]  dma_atomic_pool_init+0x44/0x1a4
>>>>> [   30.370020]  do_one_initcall+0x54/0x228
>>>>> [   30.370027]  kernel_init_freeable+0x228/0x2cc
>>>>> [   30.370031]  kernel_init+0x1c/0x110
>>>>> [   30.370034]  ret_from_fork+0x10/0x18
>>>>> [   30.370036] Mem-Info:
>>>>> [   30.370064] active_anon:0 inactive_anon:0 isolated_anon:0
>>>>> [   30.370064]  active_file:0 inactive_file:0 isolated_file:0
>>>>> [   30.370064]  unevictable:0 dirty:0 writeback:0 unstable:0
>>>>> [   30.370064]  slab_reclaimable:34 slab_unreclaimable:4438
>>>>> [   30.370064]  mapped:0 shmem:0 pagetables:14 bounce:0
>>>>> [   30.370064]  free:1537719 free_pcp:219 free_cma:0
>>>>> [   30.370070] Node 0 active_anon:0kB inactive_anon:0kB
>>>>> active_file:0kB inactive_file:0kB unevictable:0kB isolated(anon):0kB
>>>>> isolated(file):0kB mapped:0kB dirty:0kB writeback:0kB shmem:0kB
>>>>> shmem_thp: 0kB shmem_pmdmapped: 0kB anon_thp: 0kB writeback_tmp:0kB
>>>>> unstable:0kB all_unreclaimable? no
>>>>> [   30.370073] Node 1 active_anon:0kB inactive_anon:0kB
>>>>> active_file:0kB inactive_file:0kB unevictable:0kB isolated(anon):0kB
>>>>> isolated(file):0kB mapped:0kB dirty:0kB writeback:0kB shmem:0kB
>>>>> shmem_thp: 0kB shmem_pmdmapped: 0kB anon_thp: 0kB writeback_tmp:0kB
>>>>> unstable:0kB all_unreclaimable? no
>>>>> [   30.370079] Node 0 DMA free:0kB min:0kB low:0kB high:0kB
>>>>> reserved_highatomic:0KB active_anon:0kB inactive_anon:0kB
>>>>> active_file:0kB inactive_file:0kB unevictable:0kB writepending:0kB
>>>>> present:128kB managed:0kB mlocked:0kB kernel_stack:0kB pagetables:0kB
>>>>> bounce:0kB free_pcp:0kB local_pcp:0kB free_cma:0kB
>>>>> [   30.370084] lowmem_reserve[]: 0 250 6063 6063
>>>>> [   30.370090] Node 0 DMA32 free:256000kB min:408kB low:664kB
>>>>> high:920kB reserved_highatomic:0KB active_anon:0kB inactive_anon:0kB
>>>>> active_file:0kB inactive_file:0kB unevictable:0kB writepending:0kB
>>>>> present:269700kB managed:256000kB mlocked:0kB kernel_stack:0kB
>>>>> pagetables:0kB bounce:0kB free_pcp:0kB local_pcp:0kB free_cma:0kB
>>>>> [   30.370094] lowmem_reserve[]: 0 0 5813 5813
>>>>> [   30.370100] Node 0 Normal free:5894876kB min:9552kB low:15504kB
>>>>> high:21456kB reserved_highatomic:0KB active_anon:0kB inactive_anon:0kB
>>>>> active_file:0kB inactive_file:0kB unevictable:0kB writepending:0kB
>>>>> present:8388608kB managed:5953112kB mlocked:0kB kernel_stack:21672kB
>>>>> pagetables:56kB bounce:0kB free_pcp:876kB local_pcp:176kB free_cma:0kB
>>>>> [   30.370104] lowmem_reserve[]: 0 0 0 0
>>>>> [   30.370107] Node 0 DMA: 0*4kB 0*8kB 0*16kB 0*32kB 0*64kB 0*128kB
>>>>> 0*256kB 0*512kB 0*1024kB 0*2048kB 0*4096kB = 0kB
>>>>> [   30.370113] Node 0 DMA32: 0*4kB 0*8kB 0*16kB 0*32kB 0*64kB 0*128kB
>>>>> 0*256kB 0*512kB 0*1024kB 1*2048kB (M) 62*4096kB (M) = 256000kB
>>>>> [   30.370119] Node 0 Normal: 2*4kB (M) 3*8kB (ME) 2*16kB (UE) 3*32kB
>>>>> (UM) 1*64kB (U) 2*128kB (M) 2*256kB (ME) 3*512kB (ME) 3*1024kB (ME)
>>>>> 3*2048kB (UME) 1436*4096kB (M) = 5893600kB
>>>>> [   30.370129] Node 0 hugepages_total=0 hugepages_free=0
>>>>> hugepages_surp=0 hugepages_size=1048576kB
>>>>> [   30.370130] 0 total pagecache pages
>>>>> [   30.370132] 0 pages in swap cache
>>>>> [   30.370134] Swap cache stats: add 0, delete 0, find 0/0
>>>>> [   30.370135] Free swap  = 0kB
>>>>> [   30.370136] Total swap = 0kB
>>>>> [   30.370137] 2164609 pages RAM
>>>>> [   30.370139] 0 pages HighMem/MovableOnly
>>>>> [   30.370140] 612331 pages reserved
>>>>> [   30.370141] 0 pages hwpoisoned
>>>>> [   30.370143] DMA: failed to allocate 256 KiB pool for atomic
>>>>> coherent allocation
>>>> 
>>>> 
>>>> During my testing I saw the same error and Chen's  solution corrected it .
>>> 
>>> Which combination you are using on your side? I am using Prabhakar's
>>> suggested environment and can reproduce the issue
>>> with or without Chen's crashkernel support above 4G patchset.
>>> 
>>> I am also using a ThunderX2 platform with latest makedumpfile code and
>>> kexec-tools (with the suggested patch
>>> <https://urldefense.com/v3/__https://lists.infradead.org/pipermail/kexec/2020-May/025128.html__;!!GqivPVa7Brio!J6lUig58-Gw6TKZnEEYzEeSU36T-1SqlB1kImU00xtX_lss5Tx-JbUmLE9TJC3foXBLg$ >).
>>> 
>>> Thanks,
>>> Bhupesh
>> 
>> 
>> I did this activity 5 months ago and I have moved on to other activities. My DMA failures were related to PCI devices that could not be enumerated because  low-DMA space was not  available when crashkernel was moved above 4G; I don’t recall the exact platform.
>> 
>> 
>> 
>> For this failure ,
>> 
>>>>> DMA: failed to allocate 256 KiB pool for atomic
>>>>> coherent allocation
>> 
>> 
>> Is due to :
>> 
>> 
>> 3618082c
>> ("arm64 use both ZONE_DMA and ZONE_DMA32")
>> 
>> With the introduction of ZONE_DMA to support the Raspberry DMA
>> region below 1G, the crashkernel is placed in the upper 4G
>> ZONE_DMA_32 region. Since the crashkernel does not have access
>> to the ZONE_DMA region, it prints out call trace during bootup.
>> 
>> It is due to having this CONFIG item  ON  :
>> 
>> 
>> CONFIG_ZONE_DMA=y
>> 
>> Turning off ZONE_DMA fixes a issue and Raspberry PI 4 will
>> use the device tree to specify memory below 1G.
>> 
>> 
> 
> Disabling ZONE_DMA is temporary solution.  We may need proper solution


Perhaps the Raspberry platform configuration dependencies need separated  from “server class” Arm  equipment ?  Or auto-configured on boot ?  Consult an expert ;-) 



> 
>> I would like to see Chen’s feature added , perhaps as EXPERIMENTAL,  so we can get some configuration testing done on it.   It corrects having a DMA zone in low memory while crash-kernel is above 4GB.  This has been going on for a year now.
> 
> I will also like this patch to be added in Linux as early as possible.
> 
> Issue mentioned by me happens with or without this patch.
> 
> This patch-set can consider fixing because it uses low memory for DMA
> & swiotlb only.
> We can consider restricting crashkernel within the required range like below
> 
> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> index 7f9e5a6dc48c..bd67b90d35bd 100644
> --- a/kernel/crash_core.c
> +++ b/kernel/crash_core.c
> @@ -354,7 +354,7 @@ int __init reserve_crashkernel_low(void)
>                        return 0;
>        }
> 
> -       low_base = memblock_find_in_range(0, 1ULL << 32, low_size, CRASH_ALIGN);
> +       low_base = memblock_find_in_range(0,0xc0000000, low_size, CRASH_ALIGN);
>        if (!low_base) {
>                pr_err("Cannot reserve %ldMB crashkernel low memory,
> please try smaller size.\n",
>                       (unsigned long)(low_size >> 20));
> 
> 

    I suspect  0xc0000000  would need to be a CONFIG item  and not hard-coded.
    
    

> Similar change can be considered for scenario "without" this patch.
> But it will decrease memory availability for crashkernel.
> Hence increase the failure probability of crashkernel reservation.
> 
> --pk


^ permalink raw reply

* Re: [PATCH V2 1/2] hwmon: pwm-fan: Add profile support and add remove module support
From: Guenter Roeck @ 2020-06-02 14:28 UTC (permalink / raw)
  To: Sandipan Patra
  Cc: treding, jonathanh, kamil, jdelvare, robh+dt, u.kleine-koenig,
	bbasu, bbiswas, kyarlagadda, linux-pwm, linux-hwmon, devicetree,
	linux-tegra, linux-kernel
In-Reply-To: <1590992354-12623-1-git-send-email-spatra@nvidia.com>

On Mon, Jun 01, 2020 at 11:49:13AM +0530, Sandipan Patra wrote:
> Add support for profiles mode settings.
> This allows different fan settings for trip point temp/hyst/pwm.
> Tegra194 has multiple fan-profiles support.
> 
> Signed-off-by: Sandipan Patra <spatra@nvidia.com>

The subject says "remove module support". What is this supposed to be
about ?

The code adds support for multiple cooling "profiles" but, unless I am
really missing something, no means to actually select a profile.
This adds a lot of complexity to the code with zero value.

Either case, and I may have mentioned this before, functionality like this
should really reside in the thermal core and not in individual drivers.

> ---
> 
> PATCH V2:
> 	Cleaned pwm_fan_remove support as it is not required.
> 
>  drivers/hwmon/pwm-fan.c | 92 ++++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 80 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c
> index 30b7b3e..1d2a416 100644
> --- a/drivers/hwmon/pwm-fan.c
> +++ b/drivers/hwmon/pwm-fan.c
> @@ -3,8 +3,10 @@
>   * pwm-fan.c - Hwmon driver for fans connected to PWM lines.
>   *
>   * Copyright (c) 2014 Samsung Electronics Co., Ltd.
> + * Copyright (c) 2020, NVIDIA Corporation.
>   *
>   * Author: Kamil Debski <k.debski@samsung.com>
> + * Author: Sandipan Patra <spatra@nvidia.com>

You can not claim authorship for a driver by adding a few lines of code
to it.

>   */
>  
>  #include <linux/hwmon.h>
> @@ -21,6 +23,8 @@
>  #include <linux/timer.h>
>  
>  #define MAX_PWM 255
> +/* Based on OF max device tree node name length */
> +#define MAX_PROFILE_NAME_LENGTH	31
>  
>  struct pwm_fan_ctx {
>  	struct mutex lock;
> @@ -38,6 +42,12 @@ struct pwm_fan_ctx {
>  	unsigned int pwm_fan_state;
>  	unsigned int pwm_fan_max_state;
>  	unsigned int *pwm_fan_cooling_levels;
> +
> +	unsigned int pwm_fan_profiles;
> +	const char **fan_profile_names;
> +	unsigned int **fan_profile_cooling_levels;
> +	unsigned int fan_current_profile;
> +
>  	struct thermal_cooling_device *cdev;
>  };
>  
> @@ -227,28 +237,86 @@ static int pwm_fan_of_get_cooling_data(struct device *dev,
>  				       struct pwm_fan_ctx *ctx)
>  {
>  	struct device_node *np = dev->of_node;
> +	struct device_node *base_profile = NULL;
> +	struct device_node *profile_np = NULL;
> +	const char *default_profile = NULL;
>  	int num, i, ret;
>  
> -	if (!of_find_property(np, "cooling-levels", NULL))
> -		return 0;
> +	num = of_property_count_u32_elems(np, "cooling-levels");
> +	if (num <= 0) {
> +		base_profile = of_get_child_by_name(np, "profiles");

You can not just add new properties like this without documenting it
and getting approval by devicetree maintainers.

Guenter

> +		if (!base_profile) {
> +			dev_err(dev, "Wrong Data\n");
> +			return -EINVAL;
> +		}
> +	}
> +
> +	if (base_profile) {
> +		ctx->pwm_fan_profiles =
> +			of_get_available_child_count(base_profile);
>  
> -	ret = of_property_count_u32_elems(np, "cooling-levels");
> -	if (ret <= 0) {
> -		dev_err(dev, "Wrong data!\n");
> -		return ret ? : -EINVAL;
> +		if (ctx->pwm_fan_profiles <= 0) {
> +			dev_err(dev, "Profiles used but not defined\n");
> +			return -EINVAL;
> +		}
> +
> +		ctx->fan_profile_names = devm_kzalloc(dev,
> +			sizeof(const char *) * ctx->pwm_fan_profiles,
> +							GFP_KERNEL);
> +		ctx->fan_profile_cooling_levels = devm_kzalloc(dev,
> +			sizeof(int *) * ctx->pwm_fan_profiles,
> +							GFP_KERNEL);
> +
> +		if (!ctx->fan_profile_names
> +				|| !ctx->fan_profile_cooling_levels)
> +			return -ENOMEM;
> +
> +		ctx->fan_current_profile = 0;
> +		i = 0;
> +		for_each_available_child_of_node(base_profile, profile_np) {
> +			num = of_property_count_u32_elems(profile_np,
> +							"cooling-levels");
> +			if (num <= 0) {
> +				dev_err(dev, "No data in cooling-levels inside profile node!\n");
> +				return -EINVAL;
> +			}
> +
> +			of_property_read_string(profile_np, "name",
> +						&ctx->fan_profile_names[i]);
> +			if (default_profile &&
> +				!strncmp(default_profile,
> +				ctx->fan_profile_names[i],
> +				MAX_PROFILE_NAME_LENGTH))
> +				ctx->fan_current_profile = i;
> +
> +			ctx->fan_profile_cooling_levels[i] =
> +				devm_kzalloc(dev, sizeof(int) * num,
> +							GFP_KERNEL);
> +			if (!ctx->fan_profile_cooling_levels[i])
> +				return -ENOMEM;
> +
> +			of_property_read_u32_array(profile_np, "cooling-levels",
> +				ctx->fan_profile_cooling_levels[i], num);
> +			i++;
> +		}
>  	}
>  
> -	num = ret;
>  	ctx->pwm_fan_cooling_levels = devm_kcalloc(dev, num, sizeof(u32),
>  						   GFP_KERNEL);
>  	if (!ctx->pwm_fan_cooling_levels)
>  		return -ENOMEM;
>  
> -	ret = of_property_read_u32_array(np, "cooling-levels",
> -					 ctx->pwm_fan_cooling_levels, num);
> -	if (ret) {
> -		dev_err(dev, "Property 'cooling-levels' cannot be read!\n");
> -		return ret;
> +	if (base_profile) {
> +		memcpy(ctx->pwm_fan_cooling_levels,
> +		  ctx->fan_profile_cooling_levels[ctx->fan_current_profile],
> +						num);
> +	} else {
> +		ret = of_property_read_u32_array(np, "cooling-levels",
> +				ctx->pwm_fan_cooling_levels, num);
> +		if (ret) {
> +			dev_err(dev, "Property 'cooling-levels' cannot be read!\n");
> +			return -EINVAL;
> +		}
>  	}
>  
>  	for (i = 0; i < num; i++) {

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox