All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 6/6 v3] configs: ls1028a: enable networking options in rds, qds defconfig
From: Alex Marginean @ 2019-06-20 14:48 UTC (permalink / raw)
  To: u-boot
In-Reply-To: <20190620144828.11899-1-alexandru.marginean@nxp.com>

Enables ethernet, MDIO, PHY drivers for LS1028A RDB and QDS.

Signed-off-by: Alex Marginean <alexm.osslist@gmail.com>
---
 configs/ls1028aqds_tfa_defconfig | 5 ++++-
 configs/ls1028ardb_tfa_defconfig | 2 ++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/configs/ls1028aqds_tfa_defconfig b/configs/ls1028aqds_tfa_defconfig
index 7982ce4157..2ea594c43a 100644
--- a/configs/ls1028aqds_tfa_defconfig
+++ b/configs/ls1028aqds_tfa_defconfig
@@ -41,10 +41,13 @@ CONFIG_SPI_FLASH_SPANSION=y
 CONFIG_SPI_FLASH_STMICRO=y
 # CONFIG_SPI_FLASH_USE_4K_SECTORS is not set
 CONFIG_PHYLIB=y
+CONFIG_PHY_AQUANTIA=y
 CONFIG_PHY_ATHEROS=y
+CONFIG_PHY_VITESSE=y
 CONFIG_DM_ETH=y
-CONFIG_PHY_GIGE=y
+CONFIG_DM_MDIO=y
 CONFIG_E1000=y
+CONFIG_FSL_ENETC=y
 CONFIG_PCI=y
 CONFIG_DM_PCI=y
 CONFIG_DM_PCI_COMPAT=y
diff --git a/configs/ls1028ardb_tfa_defconfig b/configs/ls1028ardb_tfa_defconfig
index c65e37df79..3f5bc2e139 100644
--- a/configs/ls1028ardb_tfa_defconfig
+++ b/configs/ls1028ardb_tfa_defconfig
@@ -43,8 +43,10 @@ CONFIG_SPI_FLASH_STMICRO=y
 CONFIG_PHYLIB=y
 CONFIG_PHY_ATHEROS=y
 CONFIG_DM_ETH=y
+CONFIG_DM_MDIO=y
 CONFIG_PHY_GIGE=y
 CONFIG_E1000=y
+CONFIG_FSL_ENETC=y
 CONFIG_PCI=y
 CONFIG_DM_PCI=y
 CONFIG_DM_PCI_COMPAT=y
-- 
2.17.1

^ permalink raw reply related

* [U-Boot] [PATCH 5/6 v3] arm: dts: ls1028a updates for network interfaces
From: Alex Marginean @ 2019-06-20 14:48 UTC (permalink / raw)
  To: u-boot
In-Reply-To: <20190620144828.11899-1-alexandru.marginean@nxp.com>

Defines LS1028A RDB SGMII port, QDS RGMII port.

Signed-off-by: Alex Marginean <alexm.osslist@gmail.com>
---
 arch/arm/dts/fsl-ls1028a-qds.dts | 13 +++++++++++++
 arch/arm/dts/fsl-ls1028a-rdb.dts | 13 +++++++++++++
 arch/arm/dts/fsl-ls1028a.dtsi    | 24 ++++++++++++++++++++++++
 3 files changed, 50 insertions(+)

diff --git a/arch/arm/dts/fsl-ls1028a-qds.dts b/arch/arm/dts/fsl-ls1028a-qds.dts
index 46a0419d77..94d0aa0f95 100644
--- a/arch/arm/dts/fsl-ls1028a-qds.dts
+++ b/arch/arm/dts/fsl-ls1028a-qds.dts
@@ -86,3 +86,16 @@
 &usb2 {
 	status = "okay";
 };
+
+&enetc1 {
+	status = "okay";
+	phy-mode = "rgmii";
+	phy-handle = <&qds_phy0>;
+};
+
+&mdio0 {
+	status = "okay";
+	qds_phy0: phy at 5 {
+		reg = <5>;
+	};
+};
diff --git a/arch/arm/dts/fsl-ls1028a-rdb.dts b/arch/arm/dts/fsl-ls1028a-rdb.dts
index 932cfa2275..052538937b 100644
--- a/arch/arm/dts/fsl-ls1028a-rdb.dts
+++ b/arch/arm/dts/fsl-ls1028a-rdb.dts
@@ -86,3 +86,16 @@
 &usb2 {
 	status = "okay";
 };
+
+&enetc0 {
+	status = "okay";
+	phy-mode = "sgmii";
+	phy-handle = <&rdb_phy0>;
+};
+
+&mdio0 {
+	status = "okay";
+	rdb_phy0: phy at 2 {
+		reg = <2>;
+	};
+};
diff --git a/arch/arm/dts/fsl-ls1028a.dtsi b/arch/arm/dts/fsl-ls1028a.dtsi
index b2b55398bb..613b362b50 100644
--- a/arch/arm/dts/fsl-ls1028a.dtsi
+++ b/arch/arm/dts/fsl-ls1028a.dtsi
@@ -117,6 +117,30 @@
 		#size-cells = <2>;
 		device_type = "pci";
 		ranges= <0x82000000 0x0 0x00000000 0x1 0xf8000000 0x0 0x160000>;
+		enetc0: pci at 0,0 {
+			reg = <0x000000 0 0 0 0>;
+			status = "disabled";
+		};
+		enetc1: pci at 0,1 {
+			reg = <0x000100 0 0 0 0>;
+			status = "disabled";
+		};
+		enetc2: pci at 0,2 {
+			reg = <0x000200 0 0 0 0>;
+			status = "okay";
+			phy-mode = "internal";
+		};
+		mdio0: pci at 0,3 {
+			#address-cells=<0>;
+			#size-cells=<1>;
+			reg = <0x000300 0 0 0 0>;
+			status = "disabled";
+		};
+		enetc6: pci at 0,6 {
+			reg = <0x000600 0 0 0 0>;
+			status = "okay";
+			phy-mode = "internal";
+		};
 	};
 
 	i2c0: i2c at 2000000 {
-- 
2.17.1

^ permalink raw reply related

* [U-Boot] [PATCH 4/6 v3] drivers: net: apply serdes configuration for ENETC Ethernet interfaces
From: Alex Marginean @ 2019-06-20 14:48 UTC (permalink / raw)
  To: u-boot
In-Reply-To: <20190620144828.11899-1-alexandru.marginean@nxp.com>

Ethernet interfaces using serial protocols go through the serdes block
integrated in the SoC.  This is accessed over dedicated internal MDIOs
which are part of the Ethernet PCI functions.  Set up serdes at _start,
along with other protocol specific port/MAC configuration.
MDIO code is shared with enetc_mdio, read/write functions are exported
from fsl_enetc_mdio for this reason.

Signed-off-by: Alex Marginean <alexm.osslist@gmail.com>
---
 drivers/net/fsl_enetc.c      | 148 ++++++++++++++++++++++++++++++++++-
 drivers/net/fsl_enetc.h      |  38 +++++++++
 drivers/net/fsl_enetc_mdio.c |   8 +-
 3 files changed, 189 insertions(+), 5 deletions(-)

diff --git a/drivers/net/fsl_enetc.c b/drivers/net/fsl_enetc.c
index f14b484b26..da533a1e5d 100644
--- a/drivers/net/fsl_enetc.c
+++ b/drivers/net/fsl_enetc.c
@@ -39,6 +39,152 @@ static int enetc_bind(struct udevice *dev)
 	return 0;
 }
 
+/* MDIO wrappers, we're using these to drive internal MDIO to get to serdes */
+static int enetc_mdio_read(struct mii_dev *bus, int addr, int devad, int reg)
+{
+	struct enetc_mdio_priv priv;
+
+	priv.regs_base = bus->priv;
+	return enetc_mdio_read_priv(&priv, addr, devad, reg);
+}
+
+static int enetc_mdio_write(struct mii_dev *bus, int addr, int devad, int reg,
+			    u16 val)
+{
+	struct enetc_mdio_priv priv;
+
+	priv.regs_base = bus->priv;
+	return enetc_mdio_write_priv(&priv, addr, devad, reg, val);
+}
+
+/* only interfaces that can pin out through serdes have internal MDIO */
+static bool enetc_has_imdio(struct udevice *dev)
+{
+	struct enetc_priv *priv = dev_get_priv(dev);
+
+	return !!(priv->imdio.priv);
+}
+
+/* set up serdes for SGMII */
+static int enetc_init_sgmii(struct udevice *dev)
+{
+	struct enetc_priv *priv = dev_get_priv(dev);
+
+	if (!enetc_has_imdio(dev))
+		return 0;
+
+	/* Set to SGMII mode, use AN */
+	enetc_mdio_write(&priv->imdio, ENETC_PCS_PHY_ADDR, MDIO_DEVAD_NONE,
+			 ENETC_PCS_IF_MODE, ENETC_PCS_IF_MODE_SGMII_AN);
+
+	/* Dev ability - SGMII */
+	enetc_mdio_write(&priv->imdio, ENETC_PCS_PHY_ADDR, MDIO_DEVAD_NONE,
+			 ENETC_PCS_DEV_ABILITY, ENETC_PCS_DEV_ABILITY_SGMII);
+
+	/* Adjust link timer for SGMII */
+	enetc_mdio_write(&priv->imdio, ENETC_PCS_PHY_ADDR, MDIO_DEVAD_NONE,
+			 ENETC_PCS_LINK_TIMER1, ENETC_PCS_LINK_TIMER1_VAL);
+	enetc_mdio_write(&priv->imdio, ENETC_PCS_PHY_ADDR, MDIO_DEVAD_NONE,
+			 ENETC_PCS_LINK_TIMER2, ENETC_PCS_LINK_TIMER2_VAL);
+
+	/* restart PCS AN */
+	enetc_mdio_write(&priv->imdio, ENETC_PCS_PHY_ADDR, MDIO_DEVAD_NONE,
+			 ENETC_PCS_CR,
+			 ENETC_PCS_CR_RESET_AN | ENETC_PCS_CR_DEF_VAL);
+
+	return 0;
+}
+
+/* set up MAC for RGMII */
+static int enetc_init_rgmii(struct udevice *dev)
+{
+	struct enetc_priv *priv = dev_get_priv(dev);
+	u32 if_mode;
+
+	/* enable RGMII AN */
+	if_mode = enetc_read_port(priv, ENETC_PM_IF_MODE);
+	if_mode |= ENETC_PM_IF_MODE_AN_ENA;
+	enetc_write_port(priv, ENETC_PM_IF_MODE, if_mode);
+
+	return 0;
+}
+
+/* set up MAC and serdes for SXGMII */
+static int enetc_init_sxgmii(struct udevice *dev)
+{
+	struct enetc_priv *priv = dev_get_priv(dev);
+	u32 if_mode;
+
+	/* set ifmode to (US)XGMII */
+	if_mode = enetc_read_port(priv, ENETC_PM_IF_MODE);
+	if_mode &= ~ENETC_PM_IF_IFMODE_MASK;
+	enetc_write_port(priv, ENETC_PM_IF_MODE, if_mode);
+
+	if (!enetc_has_imdio(dev))
+		return 0;
+
+	/* Dev ability - SXGMII */
+	enetc_mdio_write(&priv->imdio, ENETC_PCS_PHY_ADDR, ENETC_PCS_DEVAD_REPL,
+			 ENETC_PCS_DEV_ABILITY, ENETC_PCS_DEV_ABILITY_SXGMII);
+
+	/* Restart PCS AN */
+	enetc_mdio_write(&priv->imdio, ENETC_PCS_PHY_ADDR, ENETC_PCS_DEVAD_REPL,
+			 ENETC_PCS_CR,
+			 ENETC_PCS_CR_LANE_RESET | ENETC_PCS_CR_RESET_AN);
+
+	return 0;
+}
+
+/* Apply protocol specific configuration to MAC, serdes as needed */
+static void enetc_start_pcs(struct udevice *dev)
+{
+	struct enetc_priv *priv = dev_get_priv(dev);
+	const char *if_str;
+
+	priv->if_type = PHY_INTERFACE_MODE_NONE;
+
+	/* check internal mdio capability, not all ports need it */
+	if (enetc_read_port(priv, ENETC_PCAPR0) & ENETC_PCAPRO_MDIO) {
+		/*
+		 * set up internal MDIO, this is part of ETH PCI function and is
+		 * used to access serdes / internal SoC PHYs.
+		 * We don't currently register it as a MDIO bus as it goes away
+		 * when the interface is removed, so it can't practically be
+		 * used in the console.
+		 */
+		priv->imdio.read = enetc_mdio_read;
+		priv->imdio.write = enetc_mdio_write;
+		priv->imdio.priv = priv->port_regs + ENETC_PM_IMDIO_BASE;
+		strncpy(priv->imdio.name, dev->name, MDIO_NAME_LEN);
+	}
+
+	if (!ofnode_valid(dev->node)) {
+		enetc_dbg(dev, "no enetc ofnode found, skipping PCS set-up\n");
+		return;
+	}
+
+	if_str = ofnode_read_string(dev->node, "phy-mode");
+	if (if_str)
+		priv->if_type = phy_get_interface_by_name(if_str);
+	else
+		enetc_dbg(dev,
+			  "phy-mode property not found, defaulting to SGMII\n");
+	if (priv->if_type < 0)
+		priv->if_type = PHY_INTERFACE_MODE_NONE;
+
+	switch (priv->if_type) {
+	case PHY_INTERFACE_MODE_SGMII:
+		enetc_init_sgmii(dev);
+		break;
+	case PHY_INTERFACE_MODE_RGMII:
+		enetc_init_rgmii(dev);
+		break;
+	case PHY_INTERFACE_MODE_XGMII:
+		enetc_init_sxgmii(dev);
+		break;
+	};
+}
+
 /* Configure the actual/external ethernet PHY, if one is found */
 static void enetc_start_phy(struct udevice *dev)
 {
@@ -303,7 +449,7 @@ static int enetc_start(struct udevice *dev)
 	enetc_setup_tx_bdr(dev);
 	enetc_setup_rx_bdr(dev);
 
-	priv->if_type = PHY_INTERFACE_MODE_NONE;
+	enetc_start_pcs(dev);
 	enetc_start_phy(dev);
 
 	return 0;
diff --git a/drivers/net/fsl_enetc.h b/drivers/net/fsl_enetc.h
index fbb9dfa15e..ac8d38da92 100644
--- a/drivers/net/fsl_enetc.h
+++ b/drivers/net/fsl_enetc.h
@@ -61,6 +61,8 @@ enum enetc_bdr_type {TX, RX};
 #define ENETC_PSIPMMR			0x0018
 #define ENETC_PSIPMAR0			0x0100
 #define ENETC_PSIPMAR1			0x0104
+#define ENETC_PCAPR0			0x0900
+#define  ENETC_PCAPRO_MDIO		BIT(11)
 #define ENETC_PSICFGR(n)		(0x0940 + (n) * 0x10)
 #define  ENETC_PSICFGR_SET_TXBDR(val)	((val) & 0xff)
 #define  ENETC_PSICFGR_SET_RXBDR(val)	(((val) & 0xff) << 16)
@@ -70,6 +72,11 @@ enum enetc_bdr_type {TX, RX};
 #define  ENETC_PM_CC_RX_TX_EN		0x8813
 #define ENETC_PM_MAXFRM			0x8014
 #define  ENETC_RX_MAXFRM_SIZE		PKTSIZE_ALIGN
+#define ENETC_PM_IMDIO_BASE		0x8030
+#define ENETC_PM_IF_MODE		0x8300
+#define  ENETC_PM_IF_MODE_RG		BIT(2)
+#define  ENETC_PM_IF_MODE_AN_ENA	BIT(15)
+#define  ENETC_PM_IF_IFMODE_MASK	GENMASK(1, 0)
 
 /* buffer descriptors count must be multiple of 8 and aligned to 128 bytes */
 #define ENETC_BD_CNT		CONFIG_SYS_RX_ETH_BUFFER
@@ -146,6 +153,7 @@ struct enetc_priv {
 	struct bd_ring rx_bdr;
 
 	int if_type;
+	struct mii_dev imdio;
 };
 
 /* register accessors */
@@ -168,6 +176,27 @@ struct enetc_priv {
 #define enetc_bdr_write(priv, t, n, off, val) \
 			enetc_write(priv, ENETC_BDR(t, n, off), val)
 
+/* PCS / internal SoC PHY ID, it defaults to 0 on all interfaces */
+#define ENETC_PCS_PHY_ADDR	0
+
+/* PCS registers */
+#define ENETC_PCS_CR			0x00
+#define ENETC_PCS_CR_RESET_AN		 0x1200
+#define ENETC_PCS_CR_DEF_VAL		 0x0140
+#define ENETC_PCS_CR_LANE_RESET		 0x8000
+#define ENETC_PCS_DEV_ABILITY		0x04
+#define ENETC_PCS_DEV_ABILITY_SGMII	 0x4001
+#define ENETC_PCS_DEV_ABILITY_SXGMII	 0x5001
+#define ENETC_PCS_LINK_TIMER1		0x12
+#define ENETC_PCS_LINK_TIMER1_VAL	 0x06a0
+#define ENETC_PCS_LINK_TIMER2		0x13
+#define ENETC_PCS_LINK_TIMER2_VAL	 0x0003
+#define ENETC_PCS_IF_MODE		0x14
+#define ENETC_PCS_IF_MODE_SGMII_AN	 0x0003
+
+/* PCS replicator block for USXGMII */
+#define ENETC_PCS_DEVAD_REPL		0x1f
+
 /* ENETC external MDIO registers */
 #define ENETC_MDIO_BASE		0x1c00
 #define ENETC_MDIO_CFG		0x00
@@ -186,4 +215,13 @@ struct enetc_mdio_priv {
 	void *regs_base;
 };
 
+/*
+ * these functions are implemented by ENETC_MDIO and are re-used by ENETC driver
+ * to drive serdes / internal SoC PHYs
+ */
+int enetc_mdio_read_priv(struct enetc_mdio_priv *priv, int addr, int devad,
+			 int reg);
+int enetc_mdio_write_priv(struct enetc_mdio_priv *priv, int addr, int devad,
+			  int reg, u16 val);
+
 #endif /* _ENETC_H */
diff --git a/drivers/net/fsl_enetc_mdio.c b/drivers/net/fsl_enetc_mdio.c
index 46afac06d7..60d21537b8 100644
--- a/drivers/net/fsl_enetc_mdio.c
+++ b/drivers/net/fsl_enetc_mdio.c
@@ -21,8 +21,8 @@ static void enetc_mdio_wait_bsy(struct enetc_mdio_priv *priv)
 		cpu_relax();
 }
 
-static int enetc_mdio_read_priv(struct enetc_mdio_priv *priv, int addr,
-				int devad, int reg)
+int enetc_mdio_read_priv(struct enetc_mdio_priv *priv, int addr, int devad,
+			 int reg)
 {
 	if (devad == MDIO_DEVAD_NONE)
 		enetc_write(priv, ENETC_MDIO_CFG, ENETC_EMDIO_CFG_C22);
@@ -51,8 +51,8 @@ static int enetc_mdio_read_priv(struct enetc_mdio_priv *priv, int addr,
 	return enetc_read(priv, ENETC_MDIO_DATA);
 }
 
-static int enetc_mdio_write_priv(struct enetc_mdio_priv *priv, int addr,
-				 int devad, int reg, u16 val)
+int enetc_mdio_write_priv(struct enetc_mdio_priv *priv, int addr, int devad,
+			  int reg, u16 val)
 {
 	if (devad == MDIO_DEVAD_NONE)
 		enetc_write(priv, ENETC_MDIO_CFG, ENETC_EMDIO_CFG_C22);
-- 
2.17.1

^ permalink raw reply related

* [U-Boot] [PATCH 3/6 v3] drivers: net: add NXP ENETC MDIO driver
From: Alex Marginean @ 2019-06-20 14:48 UTC (permalink / raw)
  To: u-boot
In-Reply-To: <20190620144828.11899-1-alexandru.marginean@nxp.com>

Adds a driver for the MDIO interface currently integrated in LS1028A SoC.
This MDIO interface is shared by multiple ethernet interfaces and is
presented as a stand-alone PCI function on the SoC ECAM.
Ethernet has a functional dependency on MDIO, for simplicity there is a
single config option for both.

Signed-off-by: Alex Marginean <alexm.osslist@gmail.com>
---
 drivers/net/Kconfig          |   2 +-
 drivers/net/Makefile         |   2 +-
 drivers/net/fsl_enetc.c      |  57 ++++++++++++++
 drivers/net/fsl_enetc.h      |  21 +++++
 drivers/net/fsl_enetc_mdio.c | 149 +++++++++++++++++++++++++++++++++++
 5 files changed, 229 insertions(+), 2 deletions(-)
 create mode 100644 drivers/net/fsl_enetc_mdio.c

diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 802eadf99d..4d85fb1716 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -590,7 +590,7 @@ config HIGMACV300_ETH
 
 config FSL_ENETC
 	bool "NXP ENETC Ethernet controller"
-	depends on DM_PCI && DM_ETH
+	depends on DM_PCI && DM_ETH && DM_MDIO
 	help
 	  This driver supports the NXP ENETC Ethernet controller found on some
 	  of the NXP SoCs.
diff --git a/drivers/net/Makefile b/drivers/net/Makefile
index b9a6bd6297..282d6d220f 100644
--- a/drivers/net/Makefile
+++ b/drivers/net/Makefile
@@ -79,4 +79,4 @@ obj-y += mscc_eswitch/
 obj-$(CONFIG_HIGMACV300_ETH) += higmacv300.o
 obj-$(CONFIG_MDIO_SANDBOX) += mdio_sandbox.o
 obj-$(CONFIG_MDIO_MUX_SANDBOX) += mdio_mux_sandbox.o
-obj-$(CONFIG_FSL_ENETC) += fsl_enetc.o
+obj-$(CONFIG_FSL_ENETC) += fsl_enetc.o fsl_enetc_mdio.o
diff --git a/drivers/net/fsl_enetc.c b/drivers/net/fsl_enetc.c
index 58ba63ceb1..f14b484b26 100644
--- a/drivers/net/fsl_enetc.c
+++ b/drivers/net/fsl_enetc.c
@@ -10,6 +10,7 @@
 #include <memalign.h>
 #include <asm/io.h>
 #include <pci.h>
+#include <miiphy.h>
 
 #include "fsl_enetc.h"
 
@@ -38,6 +39,59 @@ static int enetc_bind(struct udevice *dev)
 	return 0;
 }
 
+/* Configure the actual/external ethernet PHY, if one is found */
+static void enetc_start_phy(struct udevice *dev)
+{
+	struct enetc_priv *priv = dev_get_priv(dev);
+	struct udevice *miidev;
+	struct phy_device *phy;
+	u32 phandle, phy_id;
+	ofnode phy_node;
+	int supported;
+
+	if (!ofnode_valid(dev->node)) {
+		enetc_dbg(dev, "no enetc ofnode found, skipping PHY set-up\n");
+		return;
+	}
+
+	if (ofnode_read_u32(dev->node, "phy-handle", &phandle)) {
+		enetc_dbg(dev, "phy-handle not found, skipping PHY set-up\n");
+		return;
+	}
+
+	phy_node = ofnode_get_by_phandle(phandle);
+	if (!ofnode_valid(phy_node)) {
+		enetc_dbg(dev, "invalid phy node, skipping PHY set-up\n");
+		return;
+	}
+	enetc_dbg(dev, "phy node: %s\n", ofnode_get_name(phy_node));
+
+	if (ofnode_read_u32(phy_node, "reg", &phy_id)) {
+		enetc_dbg(dev,
+			  "missing reg in PHY node, skipping PHY set-up\n");
+		return;
+	}
+
+	if (uclass_get_device_by_ofnode(UCLASS_MDIO,
+					ofnode_get_parent(phy_node),
+					&miidev)) {
+		enetc_dbg(dev, "can't find MDIO bus for node %s\n",
+			  ofnode_get_name(ofnode_get_parent(phy_node)));
+		return;
+	}
+
+	phy = dm_mdio_phy_connect(miidev, phy_id, dev, priv->if_type);
+	if (!phy) {
+		enetc_dbg(dev, "dm_mdio_phy_connect returned null\n");
+		return;
+	}
+
+	supported = GENMASK(6, 0); /* speeds up to 1G & AN */
+	phy->advertising = phy->supported & supported;
+	phy_config(phy);
+	phy_startup(phy);
+}
+
 /*
  * Probe ENETC driver:
  * - initialize port and station interface BARs
@@ -249,6 +303,9 @@ static int enetc_start(struct udevice *dev)
 	enetc_setup_tx_bdr(dev);
 	enetc_setup_rx_bdr(dev);
 
+	priv->if_type = PHY_INTERFACE_MODE_NONE;
+	enetc_start_phy(dev);
+
 	return 0;
 }
 
diff --git a/drivers/net/fsl_enetc.h b/drivers/net/fsl_enetc.h
index 155ecc895b..fbb9dfa15e 100644
--- a/drivers/net/fsl_enetc.h
+++ b/drivers/net/fsl_enetc.h
@@ -11,6 +11,7 @@
 
 /* PCI function IDs */
 #define PCI_DEVICE_ID_ENETC_ETH		0xE100
+#define PCI_DEVICE_ID_ENETC_MDIO	0xEE01
 
 /* ENETC Ethernet controller registers */
 /* Station interface register offsets */
@@ -143,6 +144,8 @@ struct enetc_priv {
 	/* Rx/Tx buffer descriptor rings info */
 	struct bd_ring tx_bdr;
 	struct bd_ring rx_bdr;
+
+	int if_type;
 };
 
 /* register accessors */
@@ -165,4 +168,22 @@ struct enetc_priv {
 #define enetc_bdr_write(priv, t, n, off, val) \
 			enetc_write(priv, ENETC_BDR(t, n, off), val)
 
+/* ENETC external MDIO registers */
+#define ENETC_MDIO_BASE		0x1c00
+#define ENETC_MDIO_CFG		0x00
+#define  ENETC_EMDIO_CFG_C22	0x00809508
+#define  ENETC_EMDIO_CFG_C45	0x00809548
+#define  ENETC_EMDIO_CFG_RD_ER	BIT(1)
+#define  ENETC_EMDIO_CFG_BSY	BIT(0)
+#define ENETC_MDIO_CTL		0x04
+#define  ENETC_MDIO_CTL_READ	BIT(15)
+#define ENETC_MDIO_DATA		0x08
+#define ENETC_MDIO_STAT		0x0c
+
+#define ENETC_MDIO_READ_ERR	0xffff
+
+struct enetc_mdio_priv {
+	void *regs_base;
+};
+
 #endif /* _ENETC_H */
diff --git a/drivers/net/fsl_enetc_mdio.c b/drivers/net/fsl_enetc_mdio.c
new file mode 100644
index 0000000000..46afac06d7
--- /dev/null
+++ b/drivers/net/fsl_enetc_mdio.c
@@ -0,0 +1,149 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * ENETC ethernet controller driver
+ * Copyright 2019 NXP
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <errno.h>
+#include <pci.h>
+#include <miiphy.h>
+#include <asm/io.h>
+#include <asm/processor.h>
+#include <miiphy.h>
+
+#include "fsl_enetc.h"
+
+static void enetc_mdio_wait_bsy(struct enetc_mdio_priv *priv)
+{
+	while (enetc_read(priv, ENETC_MDIO_CFG) & ENETC_EMDIO_CFG_BSY)
+		cpu_relax();
+}
+
+static int enetc_mdio_read_priv(struct enetc_mdio_priv *priv, int addr,
+				int devad, int reg)
+{
+	if (devad == MDIO_DEVAD_NONE)
+		enetc_write(priv, ENETC_MDIO_CFG, ENETC_EMDIO_CFG_C22);
+	else
+		enetc_write(priv, ENETC_MDIO_CFG, ENETC_EMDIO_CFG_C45);
+	enetc_mdio_wait_bsy(priv);
+
+	if (devad == MDIO_DEVAD_NONE) {
+		enetc_write(priv, ENETC_MDIO_CTL, ENETC_MDIO_CTL_READ |
+			    (addr << 5) | reg);
+	} else {
+		enetc_write(priv, ENETC_MDIO_CTL, (addr << 5) + devad);
+		enetc_mdio_wait_bsy(priv);
+
+		enetc_write(priv, ENETC_MDIO_STAT, reg);
+		enetc_mdio_wait_bsy(priv);
+
+		enetc_write(priv, ENETC_MDIO_CTL, ENETC_MDIO_CTL_READ |
+			    (addr << 5) | devad);
+	}
+
+	enetc_mdio_wait_bsy(priv);
+	if (enetc_read(priv, ENETC_MDIO_CFG) & ENETC_EMDIO_CFG_RD_ER)
+		return ENETC_MDIO_READ_ERR;
+
+	return enetc_read(priv, ENETC_MDIO_DATA);
+}
+
+static int enetc_mdio_write_priv(struct enetc_mdio_priv *priv, int addr,
+				 int devad, int reg, u16 val)
+{
+	if (devad == MDIO_DEVAD_NONE)
+		enetc_write(priv, ENETC_MDIO_CFG, ENETC_EMDIO_CFG_C22);
+	else
+		enetc_write(priv, ENETC_MDIO_CFG, ENETC_EMDIO_CFG_C45);
+	enetc_mdio_wait_bsy(priv);
+
+	if (devad != MDIO_DEVAD_NONE) {
+		enetc_write(priv, ENETC_MDIO_CTL, (addr << 5) + devad);
+		enetc_write(priv, ENETC_MDIO_STAT, reg);
+	} else {
+		enetc_write(priv, ENETC_MDIO_CTL, (addr << 5) + reg);
+	}
+	enetc_mdio_wait_bsy(priv);
+
+	enetc_write(priv, ENETC_MDIO_DATA, val);
+	enetc_mdio_wait_bsy(priv);
+
+	return 0;
+}
+
+/* DM wrappers */
+static int dm_enetc_mdio_read(struct udevice *dev, int addr, int devad, int reg)
+{
+	struct enetc_mdio_priv *priv = dev_get_priv(dev);
+
+	return enetc_mdio_read_priv(priv, addr, devad, reg);
+}
+
+static int dm_enetc_mdio_write(struct udevice *dev, int addr, int devad,
+			       int reg, u16 val)
+{
+	struct enetc_mdio_priv *priv = dev_get_priv(dev);
+
+	return enetc_mdio_write_priv(priv, addr, devad, reg, val);
+}
+
+static const struct mdio_ops enetc_mdio_ops = {
+	.read = dm_enetc_mdio_read,
+	.write = dm_enetc_mdio_write,
+};
+
+static int enetc_mdio_bind(struct udevice *dev)
+{
+	char name[16];
+	static int eth_num_devices;
+
+	/*
+	 * prefer using PCI function numbers to number interfaces, but these
+	 * are only available if dts nodes are present.  For PCI they are
+	 * optional, handle that case too.  Just in case some nodes are present
+	 * and some are not, use different naming scheme - enetc-N based on
+	 * PCI function # and enetc#N based on interface count
+	 */
+	if (ofnode_valid(dev->node))
+		sprintf(name, "emdio-%u", PCI_FUNC(pci_get_devfn(dev)));
+	else
+		sprintf(name, "emdio#%u", eth_num_devices++);
+	device_set_name(dev, name);
+
+	return 0;
+}
+
+static int enetc_mdio_probe(struct udevice *dev)
+{
+	struct enetc_mdio_priv *priv = dev_get_priv(dev);
+
+	priv->regs_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0);
+	if (!priv->regs_base) {
+		enetc_dbg(dev, "failed to map BAR0\n");
+		return -EINVAL;
+	}
+
+	priv->regs_base += ENETC_MDIO_BASE;
+
+	dm_pci_clrset_config16(dev, PCI_COMMAND, 0, PCI_COMMAND_MEMORY);
+
+	return 0;
+}
+
+U_BOOT_DRIVER(enetc_mdio) = {
+	.name	= "enetc_mdio",
+	.id	= UCLASS_MDIO,
+	.bind	= enetc_mdio_bind,
+	.probe	= enetc_mdio_probe,
+	.ops	= &enetc_mdio_ops,
+	.priv_auto_alloc_size = sizeof(struct enetc_mdio_priv),
+};
+
+static struct pci_device_id enetc_mdio_ids[] = {
+	{ PCI_DEVICE(PCI_VENDOR_ID_FREESCALE, PCI_DEVICE_ID_ENETC_MDIO) },
+};
+
+U_BOOT_PCI_DEVICE(enetc_mdio, enetc_mdio_ids);
-- 
2.17.1

^ permalink raw reply related

* [U-Boot] [PATCH 2/6 v3] drivers: net: add NXP ENETC ethernet driver
From: Alex Marginean @ 2019-06-20 14:48 UTC (permalink / raw)
  To: u-boot
In-Reply-To: <20190620144828.11899-1-alexandru.marginean@nxp.com>

Adds a driver for NXP ENETC ethernet controller currently integrated in
LS1028A.  ENETC is a fairly straight-forward BD ring device and interfaces
are presented as PCI EPs on the SoC ECAM.

Signed-off-by: Catalin Horghidan <catalin.horghidan@nxp.com>
Signed-off-by: Alex Marginean <alexm.osslist@gmail.com>
---
 drivers/net/Kconfig     |   7 +
 drivers/net/Makefile    |   1 +
 drivers/net/fsl_enetc.c | 380 ++++++++++++++++++++++++++++++++++++++++
 drivers/net/fsl_enetc.h | 168 ++++++++++++++++++
 4 files changed, 556 insertions(+)
 create mode 100644 drivers/net/fsl_enetc.c
 create mode 100644 drivers/net/fsl_enetc.h

diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 403df5e960..802eadf99d 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -588,4 +588,11 @@ config HIGMACV300_ETH
 	  This driver supports HIGMACV300 Ethernet controller found on
 	  HiSilicon SoCs.
 
+config FSL_ENETC
+	bool "NXP ENETC Ethernet controller"
+	depends on DM_PCI && DM_ETH
+	help
+	  This driver supports the NXP ENETC Ethernet controller found on some
+	  of the NXP SoCs.
+
 endif # NETDEVICES
diff --git a/drivers/net/Makefile b/drivers/net/Makefile
index a3706ca27b..b9a6bd6297 100644
--- a/drivers/net/Makefile
+++ b/drivers/net/Makefile
@@ -79,3 +79,4 @@ obj-y += mscc_eswitch/
 obj-$(CONFIG_HIGMACV300_ETH) += higmacv300.o
 obj-$(CONFIG_MDIO_SANDBOX) += mdio_sandbox.o
 obj-$(CONFIG_MDIO_MUX_SANDBOX) += mdio_mux_sandbox.o
+obj-$(CONFIG_FSL_ENETC) += fsl_enetc.o
diff --git a/drivers/net/fsl_enetc.c b/drivers/net/fsl_enetc.c
new file mode 100644
index 0000000000..58ba63ceb1
--- /dev/null
+++ b/drivers/net/fsl_enetc.c
@@ -0,0 +1,380 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * ENETC ethernet controller driver
+ * Copyright 2017-2019 NXP
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <errno.h>
+#include <memalign.h>
+#include <asm/io.h>
+#include <pci.h>
+
+#include "fsl_enetc.h"
+
+/*
+ * Bind the device:
+ * - set a more explicit name on the interface
+ */
+static int enetc_bind(struct udevice *dev)
+{
+	char name[16];
+	static int eth_num_devices;
+
+	/*
+	 * prefer using PCI function numbers to number interfaces, but these
+	 * are only available if dts nodes are present.  For PCI they are
+	 * optional, handle that case too.  Just in case some nodes are present
+	 * and some are not, use different naming scheme - enetc-N based on
+	 * PCI function # and enetc#N based on interface count
+	 */
+	if (ofnode_valid(dev->node))
+		sprintf(name, "enetc-%u", PCI_FUNC(pci_get_devfn(dev)));
+	else
+		sprintf(name, "enetc#%u", eth_num_devices++);
+	device_set_name(dev, name);
+
+	return 0;
+}
+
+/*
+ * Probe ENETC driver:
+ * - initialize port and station interface BARs
+ */
+static int enetc_probe(struct udevice *dev)
+{
+	struct enetc_priv *priv = dev_get_priv(dev);
+
+	if (ofnode_valid(dev->node) && !ofnode_is_available(dev->node)) {
+		enetc_dbg(dev, "interface disabled\n");
+		return -ENODEV;
+	}
+
+	priv->enetc_txbd = memalign(ENETC_BD_ALIGN,
+				    sizeof(struct enetc_tx_bd) * ENETC_BD_CNT);
+	priv->enetc_rxbd = memalign(ENETC_BD_ALIGN,
+				    sizeof(union enetc_rx_bd) * ENETC_BD_CNT);
+
+	if (!priv->enetc_txbd || !priv->enetc_rxbd) {
+		/* free should be able to handle NULL, just free all pointers */
+		free(priv->enetc_txbd);
+		free(priv->enetc_rxbd);
+
+		return -ENOMEM;
+	}
+
+	/* initialize register */
+	priv->regs_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0);
+	if (!priv->regs_base) {
+		enetc_dbg(dev, "failed to map BAR0\n");
+		return -EINVAL;
+	}
+	priv->port_regs = priv->regs_base + ENETC_PORT_REGS_OFF;
+
+	dm_pci_clrset_config16(dev, PCI_COMMAND, 0, PCI_COMMAND_MEMORY);
+
+	return 0;
+}
+
+/*
+ * Remove the driver from an interface:
+ * - free up allocated memory
+ */
+static int enetc_remove(struct udevice *dev)
+{
+	struct enetc_priv *priv = dev_get_priv(dev);
+
+	free(priv->enetc_txbd);
+	free(priv->enetc_rxbd);
+
+	return 0;
+}
+
+/* ENETC Port MAC address registers, accepts big-endian format */
+static void enetc_set_primary_mac_addr(struct enetc_priv *priv, const u8 *addr)
+{
+	u16 lower = *(const u16 *)(addr + 4);
+	u32 upper = *(const u32 *)addr;
+
+	enetc_write_port(priv, ENETC_PSIPMAR0, upper);
+	enetc_write_port(priv, ENETC_PSIPMAR1, lower);
+}
+
+/* Configure port parameters (# of rings, frame size, enable port) */
+static void enetc_enable_si_port(struct enetc_priv *priv)
+{
+	u32 val;
+
+	/* set Rx/Tx BDR count */
+	val = ENETC_PSICFGR_SET_TXBDR(ENETC_TX_BDR_CNT);
+	val |= ENETC_PSICFGR_SET_RXBDR(ENETC_RX_BDR_CNT);
+	enetc_write_port(priv, ENETC_PSICFGR(0), val);
+	/* set Rx max frame size */
+	enetc_write_port(priv, ENETC_PM_MAXFRM, ENETC_RX_MAXFRM_SIZE);
+	/* enable MAC port */
+	enetc_write_port(priv, ENETC_PM_CC, ENETC_PM_CC_RX_TX_EN);
+	/* enable port */
+	enetc_write_port(priv, ENETC_PMR, ENETC_PMR_SI0_EN);
+	/* set SI cache policy */
+	enetc_write(priv, ENETC_SICAR0,
+		    ENETC_SICAR_RD_CFG | ENETC_SICAR_WR_CFG);
+	/* enable SI */
+	enetc_write(priv, ENETC_SIMR, ENETC_SIMR_EN);
+}
+
+/* returns DMA address for a given buffer index */
+static inline u64 enetc_rxb_address(struct udevice *dev, int i)
+{
+	return cpu_to_le64(dm_pci_virt_to_mem(dev, net_rx_packets[i]));
+}
+
+/*
+ * Setup a single Tx BD Ring (ID = 0):
+ * - set Tx buffer descriptor address
+ * - set the BD count
+ * - initialize the producer and consumer index
+ */
+static void enetc_setup_tx_bdr(struct udevice *dev)
+{
+	struct enetc_priv *priv = dev_get_priv(dev);
+	struct bd_ring *tx_bdr = &priv->tx_bdr;
+	u64 tx_bd_add = (u64)priv->enetc_txbd;
+
+	/* used later to advance to the next Tx BD */
+	tx_bdr->bd_count = ENETC_BD_CNT;
+	tx_bdr->next_prod_idx = 0;
+	tx_bdr->next_cons_idx = 0;
+	tx_bdr->cons_idx = priv->regs_base +
+				ENETC_BDR(TX, ENETC_TX_BDR_ID, ENETC_TBCIR);
+	tx_bdr->prod_idx = priv->regs_base +
+				ENETC_BDR(TX, ENETC_TX_BDR_ID, ENETC_TBPIR);
+
+	/* set Tx BD address */
+	enetc_bdr_write(priv, TX, ENETC_TX_BDR_ID, ENETC_TBBAR0,
+			lower_32_bits(tx_bd_add));
+	enetc_bdr_write(priv, TX, ENETC_TX_BDR_ID, ENETC_TBBAR1,
+			upper_32_bits(tx_bd_add));
+	/* set Tx 8 BD count */
+	enetc_bdr_write(priv, TX, ENETC_TX_BDR_ID, ENETC_TBLENR,
+			tx_bdr->bd_count);
+
+	/* reset both producer/consumer indexes */
+	enetc_write_reg(tx_bdr->cons_idx, tx_bdr->next_cons_idx);
+	enetc_write_reg(tx_bdr->prod_idx, tx_bdr->next_prod_idx);
+
+	/* enable TX ring */
+	enetc_bdr_write(priv, TX, ENETC_TX_BDR_ID, ENETC_TBMR, ENETC_TBMR_EN);
+}
+
+/*
+ * Setup a single Rx BD Ring (ID = 0):
+ * - set Rx buffer descriptors address (one descriptor per buffer)
+ * - set buffer size as max frame size
+ * - enable Rx ring
+ * - reset consumer and producer indexes
+ * - set buffer for each descriptor
+ */
+static void enetc_setup_rx_bdr(struct udevice *dev)
+{
+	struct enetc_priv *priv = dev_get_priv(dev);
+	struct bd_ring *rx_bdr = &priv->rx_bdr;
+	u64 rx_bd_add = (u64)priv->enetc_rxbd;
+	int i;
+
+	/* used later to advance to the next BD produced by ENETC HW */
+	rx_bdr->bd_count = ENETC_BD_CNT;
+	rx_bdr->next_prod_idx = 0;
+	rx_bdr->next_cons_idx = 0;
+	rx_bdr->cons_idx = priv->regs_base +
+				ENETC_BDR(RX, ENETC_RX_BDR_ID, ENETC_RBCIR);
+	rx_bdr->prod_idx = priv->regs_base +
+				ENETC_BDR(RX, ENETC_RX_BDR_ID, ENETC_RBPIR);
+
+	/* set Rx BD address */
+	enetc_bdr_write(priv, RX, ENETC_RX_BDR_ID, ENETC_RBBAR0,
+			lower_32_bits(rx_bd_add));
+	enetc_bdr_write(priv, RX, ENETC_RX_BDR_ID, ENETC_RBBAR1,
+			upper_32_bits(rx_bd_add));
+	/* set Rx BD count (multiple of 8) */
+	enetc_bdr_write(priv, RX, ENETC_RX_BDR_ID, ENETC_RBLENR,
+			rx_bdr->bd_count);
+	/* set Rx buffer  size */
+	enetc_bdr_write(priv, RX, ENETC_RX_BDR_ID, ENETC_RBBSR, PKTSIZE_ALIGN);
+
+	/* fill Rx BD */
+	memset(priv->enetc_rxbd, 0,
+	       rx_bdr->bd_count * sizeof(union enetc_rx_bd));
+	for (i = 0; i < rx_bdr->bd_count; i++) {
+		priv->enetc_rxbd[i].w.addr = enetc_rxb_address(dev, i);
+		/* each RX buffer must be aligned to 64B */
+		WARN_ON(priv->enetc_rxbd[i].w.addr & (ARCH_DMA_MINALIGN - 1));
+	}
+
+	/* reset producer (ENETC owned) and consumer (SW owned) index */
+	enetc_write_reg(rx_bdr->cons_idx, rx_bdr->next_cons_idx);
+	enetc_write_reg(rx_bdr->prod_idx, rx_bdr->next_prod_idx);
+
+	/* enable Rx ring */
+	enetc_bdr_write(priv, RX, ENETC_RX_BDR_ID, ENETC_RBMR, ENETC_RBMR_EN);
+}
+
+/*
+ * Start ENETC interface:
+ * - perform FLR
+ * - enable access to port and SI registers
+ * - set mac address
+ * - setup TX/RX buffer descriptors
+ * - enable Tx/Rx rings
+ */
+static int enetc_start(struct udevice *dev)
+{
+	struct eth_pdata *plat = dev_get_platdata(dev);
+	struct enetc_priv *priv = dev_get_priv(dev);
+
+	/* reset and enable the PCI device */
+	dm_pci_flr(dev);
+	dm_pci_clrset_config16(dev, PCI_COMMAND, 0,
+			       PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER);
+
+	if (!is_valid_ethaddr(plat->enetaddr)) {
+		enetc_dbg(dev, "invalid MAC address, generate random ...\n");
+		net_random_ethaddr(plat->enetaddr);
+	}
+	enetc_set_primary_mac_addr(priv, plat->enetaddr);
+
+	enetc_enable_si_port(priv);
+
+	/* setup Tx/Rx buffer descriptors */
+	enetc_setup_tx_bdr(dev);
+	enetc_setup_rx_bdr(dev);
+
+	return 0;
+}
+
+/*
+ * Stop the network interface:
+ * - just quiesce it, we can wipe all configuration as _start starts from
+ * scratch each time
+ */
+static void enetc_stop(struct udevice *dev)
+{
+	/* FLR is sufficient to quiesce the device */
+	dm_pci_flr(dev);
+}
+
+/*
+ * ENETC transmit packet:
+ * - check if Tx BD ring is full
+ * - set buffer/packet address (dma address)
+ * - set final fragment flag
+ * - try while producer index equals consumer index or timeout
+ */
+static int enetc_send(struct udevice *dev, void *packet, int length)
+{
+	struct enetc_priv *priv = dev_get_priv(dev);
+	struct bd_ring *txr = &priv->tx_bdr;
+	void *nv_packet = (void *)packet;
+	int tries = ENETC_POLL_TRIES;
+	u32 pi, ci;
+
+	pi = txr->next_prod_idx;
+	ci = enetc_read_reg(txr->cons_idx) & ENETC_BDR_IDX_MASK;
+	/* Tx ring is full when */
+	if (((pi + 1) % txr->bd_count) == ci) {
+		enetc_dbg(dev, "Tx BDR full\n");
+		return -ETIMEDOUT;
+	}
+	enetc_dbg(dev, "TxBD[%d]send: pkt_len=%d, buff @0x%x%08x\n", pi, length,
+		  upper_32_bits((u64)nv_packet), lower_32_bits((u64)nv_packet));
+
+	/* prepare Tx BD */
+	memset(&priv->enetc_txbd[pi], 0x0, sizeof(struct enetc_tx_bd));
+	priv->enetc_txbd[pi].addr =
+		cpu_to_le64(dm_pci_virt_to_mem(dev, nv_packet));
+	priv->enetc_txbd[pi].buf_len = cpu_to_le16(length);
+	priv->enetc_txbd[pi].frm_len = cpu_to_le16(length);
+	priv->enetc_txbd[pi].flags = cpu_to_le16(ENETC_TXBD_FLAGS_F);
+	dmb();
+	/* send frame: increment producer index */
+	pi = (pi + 1) % txr->bd_count;
+	txr->next_prod_idx = pi;
+	enetc_write_reg(txr->prod_idx, pi);
+	while ((--tries >= 0) &&
+	       (pi != (enetc_read_reg(txr->cons_idx) & ENETC_BDR_IDX_MASK)))
+		udelay(10);
+
+	return tries > 0 ? 0 : -ETIMEDOUT;
+}
+
+/*
+ * Receive frame:
+ * - wait for the next BD to get ready bit set
+ * - clean up the descriptor
+ * - move on and indicate to HW that the cleaned BD is available for Rx
+ */
+static int enetc_recv(struct udevice *dev, int flags, uchar **packetp)
+{
+	struct enetc_priv *priv = dev_get_priv(dev);
+	struct bd_ring *rxr = &priv->rx_bdr;
+	int tries = ENETC_POLL_TRIES;
+	int pi = rxr->next_prod_idx;
+	int ci = rxr->next_cons_idx;
+	u32 status;
+	int len;
+	u8 rdy;
+
+	do {
+		dmb();
+		status = le32_to_cpu(priv->enetc_rxbd[pi].r.lstatus);
+		/* check if current BD is ready to be consumed */
+		rdy = ENETC_RXBD_STATUS_R(status);
+	} while (--tries >= 0 && !rdy);
+
+	if (!rdy)
+		return -EAGAIN;
+
+	dmb();
+	len = le16_to_cpu(priv->enetc_rxbd[pi].r.buf_len);
+	*packetp = (uchar *)enetc_rxb_address(dev, pi);
+	enetc_dbg(dev, "RxBD[%d]: len=%d err=%d pkt=0x%x%08x\n", pi, len,
+		  ENETC_RXBD_STATUS_ERRORS(status),
+		  upper_32_bits((u64)*packetp), lower_32_bits((u64)*packetp));
+
+	/* BD clean up and advance to next in ring */
+	memset(&priv->enetc_rxbd[pi], 0, sizeof(union enetc_rx_bd));
+	priv->enetc_rxbd[pi].w.addr = enetc_rxb_address(dev, pi);
+	rxr->next_prod_idx = (pi + 1) % rxr->bd_count;
+	ci = (ci + 1) % rxr->bd_count;
+	rxr->next_cons_idx = ci;
+	dmb();
+	/* free up the slot in the ring for HW */
+	enetc_write_reg(rxr->cons_idx, ci);
+
+	return len;
+}
+
+static const struct eth_ops enetc_ops = {
+	.start	= enetc_start,
+	.send	= enetc_send,
+	.recv	= enetc_recv,
+	.stop	= enetc_stop,
+};
+
+U_BOOT_DRIVER(eth_enetc) = {
+	.name	= "enetc_eth",
+	.id	= UCLASS_ETH,
+	.bind	= enetc_bind,
+	.probe	= enetc_probe,
+	.remove = enetc_remove,
+	.ops	= &enetc_ops,
+	.priv_auto_alloc_size = sizeof(struct enetc_priv),
+	.platdata_auto_alloc_size = sizeof(struct eth_pdata),
+};
+
+static struct pci_device_id enetc_ids[] = {
+	{ PCI_DEVICE(PCI_VENDOR_ID_FREESCALE, PCI_DEVICE_ID_ENETC_ETH) },
+	{}
+};
+
+U_BOOT_PCI_DEVICE(eth_enetc, enetc_ids);
diff --git a/drivers/net/fsl_enetc.h b/drivers/net/fsl_enetc.h
new file mode 100644
index 0000000000..155ecc895b
--- /dev/null
+++ b/drivers/net/fsl_enetc.h
@@ -0,0 +1,168 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * ENETC ethernet controller driver
+ * Copyright 2017-2019 NXP
+ */
+
+#ifndef _ENETC_H
+#define _ENETC_H
+
+#define enetc_dbg(dev, fmt, args...)	debug("%s:" fmt, dev->name, ##args)
+
+/* PCI function IDs */
+#define PCI_DEVICE_ID_ENETC_ETH		0xE100
+
+/* ENETC Ethernet controller registers */
+/* Station interface register offsets */
+#define ENETC_SIMR		0x000
+#define  ENETC_SIMR_EN		BIT(31)
+#define ENETC_SICAR0		0x040
+/* write cache cfg: snoop, no allocate, data & BD coherent */
+#define  ENETC_SICAR_WR_CFG	0x6767
+/* read cache cfg: coherent copy, look up, don't alloc in cache */
+#define  ENETC_SICAR_RD_CFG	0x27270000
+#define ENETC_SIROCT		0x300
+#define ENETC_SIRFRM		0x308
+#define ENETC_SITOCT		0x320
+#define ENETC_SITFRM		0x328
+
+/* Rx/Tx Buffer Descriptor Ring registers */
+enum enetc_bdr_type {TX, RX};
+#define ENETC_BDR(type, n, off)	(0x8000 + (type) * 0x100 + (n) * 0x200 + (off))
+#define ENETC_BDR_IDX_MASK	0xffff
+
+/* Rx BDR reg offsets */
+#define ENETC_RBMR		0x00
+#define  ENETC_RBMR_EN		BIT(31)
+#define ENETC_RBBSR		0x08
+/* initial consumer index for Rx BDR */
+#define ENETC_RBCIR		0x0c
+#define ENETC_RBBAR0		0x10
+#define ENETC_RBBAR1		0x14
+#define ENETC_RBPIR		0x18
+#define ENETC_RBLENR		0x20
+
+/* Tx BDR reg offsets */
+#define ENETC_TBMR		0x00
+#define  ENETC_TBMR_EN		BIT(31)
+#define ENETC_TBBAR0		0x10
+#define ENETC_TBBAR1		0x14
+#define ENETC_TBPIR		0x18
+#define ENETC_TBCIR		0x1c
+#define ENETC_TBLENR		0x20
+
+/* Port registers offset */
+#define ENETC_PORT_REGS_OFF		0x10000
+
+/* Port registers */
+#define ENETC_PMR			0x0000
+#define  ENETC_PMR_SI0_EN		BIT(16)
+#define ENETC_PSIPMMR			0x0018
+#define ENETC_PSIPMAR0			0x0100
+#define ENETC_PSIPMAR1			0x0104
+#define ENETC_PSICFGR(n)		(0x0940 + (n) * 0x10)
+#define  ENETC_PSICFGR_SET_TXBDR(val)	((val) & 0xff)
+#define  ENETC_PSICFGR_SET_RXBDR(val)	(((val) & 0xff) << 16)
+/* MAC configuration */
+#define ENETC_PM_CC			0x8008
+#define  ENETC_PM_CC_DEFAULT		0x0810
+#define  ENETC_PM_CC_RX_TX_EN		0x8813
+#define ENETC_PM_MAXFRM			0x8014
+#define  ENETC_RX_MAXFRM_SIZE		PKTSIZE_ALIGN
+
+/* buffer descriptors count must be multiple of 8 and aligned to 128 bytes */
+#define ENETC_BD_CNT		CONFIG_SYS_RX_ETH_BUFFER
+#define ENETC_BD_ALIGN		128
+
+/* single pair of Rx/Tx rings */
+#define ENETC_RX_BDR_CNT	1
+#define ENETC_TX_BDR_CNT	1
+#define ENETC_RX_BDR_ID		0
+#define ENETC_TX_BDR_ID		0
+
+/* Tx buffer descriptor */
+struct enetc_tx_bd {
+	__le64 addr;
+	__le16 buf_len;
+	__le16 frm_len;
+	__le16 err_csum;
+	__le16 flags;
+};
+
+#define ENETC_TXBD_FLAGS_F	BIT(15)
+#define ENETC_POLL_TRIES	32000
+
+/* Rx buffer descriptor */
+union enetc_rx_bd {
+	/* SW provided BD format */
+	struct {
+		__le64 addr;
+		u8 reserved[8];
+	} w;
+
+	/* ENETC returned BD format */
+	struct {
+		__le16 inet_csum;
+		__le16 parse_summary;
+		__le32 rss_hash;
+		__le16 buf_len;
+		__le16 vlan_opt;
+		union {
+			struct {
+				__le16 flags;
+				__le16 error;
+			};
+			__le32 lstatus;
+		};
+	} r;
+};
+
+#define ENETC_RXBD_STATUS_R(status)		(((status) >> 30) & 0x1)
+#define ENETC_RXBD_STATUS_F(status)		(((status) >> 31) & 0x1)
+#define ENETC_RXBD_STATUS_ERRORS(status)	(((status) >> 16) & 0xff)
+#define ENETC_RXBD_STATUS(flags)		((flags) << 16)
+
+/* Tx/Rx ring info */
+struct bd_ring {
+	void *cons_idx;
+	void *prod_idx;
+	/* next BD index to use */
+	int next_prod_idx;
+	int next_cons_idx;
+	int bd_count;
+};
+
+/* ENETC private structure */
+struct enetc_priv {
+	struct enetc_tx_bd *enetc_txbd;
+	union enetc_rx_bd *enetc_rxbd;
+
+	void *regs_base; /* base ENETC registers */
+	void *port_regs; /* base ENETC port registers */
+
+	/* Rx/Tx buffer descriptor rings info */
+	struct bd_ring tx_bdr;
+	struct bd_ring rx_bdr;
+};
+
+/* register accessors */
+#define enetc_read_reg(x)	readl((x))
+#define enetc_write_reg(x, val)	writel((val), (x))
+#define enetc_read(priv, off)	enetc_read_reg((priv)->regs_base + (off))
+#define enetc_write(priv, off, v) \
+			enetc_write_reg((priv)->regs_base + (off), v)
+
+/* port register accessors */
+#define enetc_port_regs(priv, off) ((priv)->port_regs + (off))
+#define enetc_read_port(priv, off) \
+			enetc_read_reg(enetc_port_regs((priv), (off)))
+#define enetc_write_port(priv, off, v) \
+			enetc_write_reg(enetc_port_regs((priv), (off)), v)
+
+/* BDR register accessors, see ENETC_BDR() */
+#define enetc_bdr_read(priv, t, n, off) \
+			enetc_read(priv, ENETC_BDR(t, n, off))
+#define enetc_bdr_write(priv, t, n, off, val) \
+			enetc_write(priv, ENETC_BDR(t, n, off), val)
+
+#endif /* _ENETC_H */
-- 
2.17.1

^ permalink raw reply related

* RE: Patch "net: phylink: set the autoneg state in phylink_phy_change" has been added to the 5.1-stable tree
From: Ioana Ciornei @ 2019-06-20 14:48 UTC (permalink / raw)
  To: gregkh@linuxfoundation.org, stable@vger.kernel.org
  Cc: stable-commits@vger.kernel.org, Russell King - ARM Linux admin,
	David Miller
In-Reply-To: <156094908410230@kroah.com>


> Subject: Patch "net: phylink: set the autoneg state in phylink_phy_change" has
> been added to the 5.1-stable tree
> 
> 
> This is a note to let you know that I've just added the patch titled
> 
>     net: phylink: set the autoneg state in phylink_phy_change
> 
> to the 5.1-stable tree which can be found at:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/stable-queue.git 
> 
> The filename of the patch is:
>      net-phylink-set-the-autoneg-state-in-phylink_phy_change.patch
> and it can be found in the queue-5.1 subdirectory.
> 
> If you, or anyone else, feels it should not be added to the stable tree, please let
> <stable@vger.kernel.org> know about it.

Hi all,

Sorry for the late response but this patch should not be added to stables trees since it was already reverted in net-next.
More information can be found at: https://marc.info/?l=linux-netdev&m=156104162206869&w=2

Thanks,
Ioana

> 
> 
> From foo@baz Wed 19 Jun 2019 02:33:45 PM CEST
> From: Ioana Ciornei <ioana.ciornei@nxp.com>
> Date: Thu, 13 Jun 2019 09:37:51 +0300
> Subject: net: phylink: set the autoneg state in phylink_phy_change
> 
> From: Ioana Ciornei <ioana.ciornei@nxp.com>
> 
> [ Upstream commit ef7bfa84725d891bbdb88707ed55b2cbf94942bb ]
> 
> The phy_state field of phylink should carry only valid information especially
> when this can be passed to the .mac_config callback.
> Update the an_enabled field with the autoneg state in the phylink_phy_change
> function.
> 
> Fixes: 9525ae83959b ("phylink: add phylink infrastructure")
> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  drivers/net/phy/phylink.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> --- a/drivers/net/phy/phylink.c
> +++ b/drivers/net/phy/phylink.c
> @@ -638,6 +638,7 @@ static void phylink_phy_change(struct ph
>  		pl->phy_state.pause |= MLO_PAUSE_ASYM;
>  	pl->phy_state.interface = phydev->interface;
>  	pl->phy_state.link = up;
> +	pl->phy_state.an_enabled = phydev->autoneg;
>  	mutex_unlock(&pl->state_mutex);
> 
>  	phylink_run_resolve(pl);
> 
> 
> Patches currently in stable-queue which might be from ioana.ciornei@nxp.com
> are
> 
> queue-5.1/net-phylink-set-the-autoneg-state-in-phylink_phy_change.patch

^ permalink raw reply

* [U-Boot] [PATCH 1/6 v3] include: configs: ls1028a: set SYS_RX_ETH_BUFFER to 8
From: Alex Marginean @ 2019-06-20 14:48 UTC (permalink / raw)
  To: u-boot
In-Reply-To: <20190620144828.11899-1-alexandru.marginean@nxp.com>

LS1028A ethernet interfaces work with at least 8 BDs, set number of buffers
to match that.

Signed-off-by: Alex Marginean <alexm.osslist@gmail.com>
---
 include/configs/ls1028a_common.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/configs/ls1028a_common.h b/include/configs/ls1028a_common.h
index 0db86396e9..f9d2602afd 100644
--- a/include/configs/ls1028a_common.h
+++ b/include/configs/ls1028a_common.h
@@ -197,4 +197,8 @@
 #define CONFIG_SYS_EEPROM_PAGE_WRITE_BITS	3
 #define CONFIG_SYS_EEPROM_PAGE_WRITE_DELAY_MS	5
 
+/* Ethernet */
+/* smallest ENETC BD ring has 8 entries */
+#define CONFIG_SYS_RX_ETH_BUFFER		8
+
 #endif /* __L1028A_COMMON_H */
-- 
2.17.1

^ permalink raw reply related

* [U-Boot] [PATCH v3 0/6] ls1028a: networking support
From: Alex Marginean @ 2019-06-20 14:48 UTC (permalink / raw)
  To: u-boot

This patch set introduces ENETC ethernet and MDIO drivers, integrated in LS1028A
NXP device and enables networking on the two existing boards.
The SGMII interface labeled MAC0 is now functional on the reference design board
(RDB).  The RDB also features four switch ports, not enabled yet.
On the development system (QDS) the RGMII port is now functional.  Add-on cards
are not yet enabled.

Depends on:
    - DM MDIO: https://patchwork.ozlabs.org/patch/1109368/
    - PCI EA and FLR: https://patchwork.ozlabs.org/patch/1111608/

Changes in v3:
    - Added cover letter
    - Added serdes (SoC internal PHYs) configuration code
    - Use a single config option for ethernet and MDIO drivers as they are
    cross dependent
    - Use net_rx_packets for Rx and set SYS_RX_ETH_BUFFER to 8, minimum BD ring
    size supported by ENETC
    - MDIO driver now resides in its own file, read/write functions are
    reused by ethernet driver to access serdes
    - PCI IDs for our functions are no longer defined in pci_ids.h
    - several cosmetic updates, static functions, comments

Alex Marginean (6):
  include: configs: ls1028a: set SYS_RX_ETH_BUFFER to 8
  drivers: net: add NXP ENETC ethernet driver
  drivers: net: add NXP ENETC MDIO driver
  drivers: net: apply serdes configuration for ENETC Ethernet interfaces
  arm: dts: ls1028a updates for network interfaces
  configs: ls1028a: enable networking options in rds, qds defconfig

 arch/arm/dts/fsl-ls1028a-qds.dts |  13 +
 arch/arm/dts/fsl-ls1028a-rdb.dts |  13 +
 arch/arm/dts/fsl-ls1028a.dtsi    |  24 ++
 configs/ls1028aqds_tfa_defconfig |   5 +-
 configs/ls1028ardb_tfa_defconfig |   2 +
 drivers/net/Kconfig              |   7 +
 drivers/net/Makefile             |   1 +
 drivers/net/fsl_enetc.c          | 583 +++++++++++++++++++++++++++++++
 drivers/net/fsl_enetc.h          | 227 ++++++++++++
 drivers/net/fsl_enetc_mdio.c     | 149 ++++++++
 include/configs/ls1028a_common.h |   4 +
 11 files changed, 1027 insertions(+), 1 deletion(-)
 create mode 100644 drivers/net/fsl_enetc.c
 create mode 100644 drivers/net/fsl_enetc.h
 create mode 100644 drivers/net/fsl_enetc_mdio.c

-- 
2.17.1

^ permalink raw reply

* Re: [Freedreno] [PATCH 5/5 v3] drm/msm/mdp5: Use the interconnect API
From: Jeffrey Hugo @ 2019-06-20 14:48 UTC (permalink / raw)
  To: Rob Clark
  Cc: open list:DRM PANEL DRIVERS, Rob Clark, freedreno,
	Boris Brezillon, David Airlie, MSM, lkml, Mamta Shukla, Sean Paul,
	Daniel Vetter, Sean Paul, Georgi Djakov
In-Reply-To: <20190618221022.28749-1-robdclark@gmail.com>

On Tue, Jun 18, 2019 at 4:10 PM Rob Clark <robdclark@gmail.com> wrote:
>
> From: Georgi Djakov <georgi.djakov@linaro.org>
>
> The interconnect API provides an interface for consumer drivers to
> express their bandwidth needs in the SoC. This data is aggregated
> and the on-chip interconnect hardware is configured to the most
> appropriate power/performance profile.
>
> Use the API to configure the interconnects and request bandwidth
> between DDR and the display hardware (MDP port(s) and rotator
> downscaler).
>
> v2: update the path names to be consistent with dpu, handle the NULL
>     path case, updated commit msg from Georgi.
> v3: split out icc setup into it's own function, and rework logic
>     slightly so no interconnect paths is not fatal.
>
> Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
> Signed-off-by: Rob Clark <robdclark@chromium.org>

Looks good to me.
Reviewed-By: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>

^ permalink raw reply

* [Qemu-devel] [PATCH v1 1/1] tcg/riscv: Fix RISC-VH host build failure
From: Alistair Francis @ 2019-06-20 14:04 UTC (permalink / raw)
  To: qemu-devel, qemu-riscv
  Cc: alistair23, richard.henderson, palmer, alistair.francis

Commit 269bd5d8 "cpu: Move the softmmu tlb to CPUNegativeOffsetState'
broke the RISC-V host build as there are two variables that are used but
not defined.

This patch renames the undefined variables mask_off and table_off to the
existing (but unused) mask_ofs and table_ofs variables.

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 tcg/riscv/tcg-target.inc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tcg/riscv/tcg-target.inc.c b/tcg/riscv/tcg-target.inc.c
index 1f0ae64aae..3e76bf5738 100644
--- a/tcg/riscv/tcg-target.inc.c
+++ b/tcg/riscv/tcg-target.inc.c
@@ -980,8 +980,8 @@ static void tcg_out_tlb_load(TCGContext *s, TCGReg addrl,
     int table_ofs = fast_ofs + offsetof(CPUTLBDescFast, table);
     TCGReg mask_base = TCG_AREG0, table_base = TCG_AREG0;
 
-    tcg_out_ld(s, TCG_TYPE_PTR, TCG_REG_TMP0, mask_base, mask_off);
-    tcg_out_ld(s, TCG_TYPE_PTR, TCG_REG_TMP1, table_base, table_off);
+    tcg_out_ld(s, TCG_TYPE_PTR, TCG_REG_TMP0, mask_base, mask_ofs);
+    tcg_out_ld(s, TCG_TYPE_PTR, TCG_REG_TMP1, table_base, table_ofs);
 
     tcg_out_opc_imm(s, OPC_SRLI, TCG_REG_TMP2, addrl,
                     TARGET_PAGE_BITS - CPU_TLB_ENTRY_BITS);
-- 
2.22.0



^ permalink raw reply related

* [PATCH RFC v1 7/7] serial: imx: get rid of imx_uart_rts_auto()
From: Sergey Organov @ 2019-06-20 14:47 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: Pengutronix Kernel Team, NXP Linux Team, linux-arm-kernel,
	linux-serial, Uwe Kleine-König
In-Reply-To: <1561042073-617-1-git-send-email-sorganov@gmail.com>

Called in only one place, for RS232, it only obscures things, as it
doesn't go well with 2 similar named functions,
imx_uart_rts_inactive() and imx_uart_rts_active(), that both are
RS485-specific.

Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
 drivers/tty/serial/imx.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 171347d..a5e80a0 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -402,13 +402,6 @@ static void imx_uart_rts_inactive(struct imx_port *sport, u32 *ucr2)
 	mctrl_gpio_set(sport->gpios, sport->port.mctrl);
 }
 
-/* called with port.lock taken and irqs caller dependent */
-static void imx_uart_rts_auto(struct imx_port *sport, u32 *ucr2)
-{
-	if (*ucr2 & UCR2_CTS)
-		*ucr2 |= UCR2_CTSC;
-}
-
 /* called with port.lock taken and irqs off */
 static void imx_uart_start_rx(struct uart_port *port)
 {
@@ -1598,8 +1591,10 @@ imx_uart_set_termios(struct uart_port *port, struct ktermios *termios,
 		else
 			imx_uart_rts_inactive(sport, &ucr2);
 
-	} else if (termios->c_cflag & CRTSCTS)
-		imx_uart_rts_auto(sport, &ucr2);
+	} else if (termios->c_cflag & CRTSCTS) {
+		if (ucr2 & UCR2_CTS)
+			ucr2 |= UCR2_CTSC;
+	}
 
 	if (termios->c_cflag & CRTSCTS)
 		ucr2 &= ~UCR2_IRTS;
-- 
2.10.0.1.g57b01a3

^ permalink raw reply related

* [PATCH RFC v1 6/7] serial: imx: set_mctrl(): correctly restore autoRTS state
From: Sergey Organov @ 2019-06-20 14:47 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: Pengutronix Kernel Team, NXP Linux Team, linux-arm-kernel,
	linux-serial, Uwe Kleine-König
In-Reply-To: <1561042073-617-1-git-send-email-sorganov@gmail.com>

imx_uart_set_mctrl() happened to set UCR2_CTSC bit whenever TIOCM_RTS
was set, no matter if RTS/CTS handshake is enabled or not. Now fixed by
turning handshake on only when CRTSCTS bit for the port is set.

Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
 drivers/tty/serial/imx.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 4867f80..171347d 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -970,10 +970,19 @@ static void imx_uart_set_mctrl(struct uart_port *port, unsigned int mctrl)
 	if (!(port->rs485.flags & SER_RS485_ENABLED)) {
 		u32 ucr2;
 
+		/*
+		 * Turn off autoRTS (UCR2_CTSC) if RTS is lowered and restore
+		 * autoRTS setting if RTS is raised. Inverted UCR2_IRTS is set
+		 * if and only if CRTSCTS bit is set for the port, so we use it
+		 * to get the state to restore to.
+		 */
 		ucr2 = imx_uart_readl(sport, UCR2);
 		ucr2 &= ~(UCR2_CTS | UCR2_CTSC);
-		if (mctrl & TIOCM_RTS)
-			ucr2 |= UCR2_CTS | UCR2_CTSC;
+		if (mctrl & TIOCM_RTS) {
+			ucr2 |= UCR2_CTS;
+			if (!(ucr2 & UCR2_IRTS))
+				ucr2 |= UCR2_CTSC;
+		}
 		imx_uart_writel(sport, ucr2, UCR2);
 	}
 
-- 
2.10.0.1.g57b01a3

^ permalink raw reply related

* [PATCH RFC v1 5/7] serial: imx: set_termios(): do not enable autoRTS if RTS is unset
From: Sergey Organov @ 2019-06-20 14:47 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: Pengutronix Kernel Team, NXP Linux Team, linux-arm-kernel,
	linux-serial, Uwe Kleine-König
In-Reply-To: <1561042073-617-1-git-send-email-sorganov@gmail.com>

set_termios() shouldn't set UCR2_CTSC bit if UCR2_CTS (=TIOCM_RTS) is
cleared. Added corresponding check in imx_uart_rts_auto() to fix this.

Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
 drivers/tty/serial/imx.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index e0f5365..4867f80 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -405,7 +405,8 @@ static void imx_uart_rts_inactive(struct imx_port *sport, u32 *ucr2)
 /* called with port.lock taken and irqs caller dependent */
 static void imx_uart_rts_auto(struct imx_port *sport, u32 *ucr2)
 {
-	*ucr2 |= UCR2_CTSC;
+	if (*ucr2 & UCR2_CTS)
+		*ucr2 |= UCR2_CTSC;
 }
 
 /* called with port.lock taken and irqs off */
-- 
2.10.0.1.g57b01a3

^ permalink raw reply related

* [PATCH RFC v1 4/7] serial: imx: set_termios(): preserve RTS state
From: Sergey Organov @ 2019-06-20 14:47 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: Pengutronix Kernel Team, NXP Linux Team, linux-arm-kernel,
	linux-serial, Uwe Kleine-König
In-Reply-To: <1561042073-617-1-git-send-email-sorganov@gmail.com>

imx_set_termios() cleared RTS on every call, now fixed.

Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
 drivers/tty/serial/imx.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 17e2322..e0f5365 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -1563,7 +1563,14 @@ imx_uart_set_termios(struct uart_port *port, struct ktermios *termios,
 
 	spin_lock_irqsave(&sport->port.lock, flags);
 
-	ucr2 = UCR2_SRST | UCR2_IRTS;
+	/*
+	 * Read current UCR2 and save it for future use, then clear all the bits
+	 * except those we will or may need to preserve.
+	 */
+	old_ucr2 = imx_uart_readl(sport, UCR2);
+	ucr2 = old_ucr2 & (UCR2_TXEN | UCR2_RXEN | UCR2_ATEN | UCR2_CTS);
+
+	ucr2 |= UCR2_SRST | UCR2_IRTS;
 	if ((termios->c_cflag & CSIZE) == CS8)
 		ucr2 |= UCR2_WS;
 
@@ -1632,7 +1639,6 @@ imx_uart_set_termios(struct uart_port *port, struct ktermios *termios,
 	imx_uart_writel(sport,
 			old_ucr1 & ~(UCR1_TXMPTYEN | UCR1_RRDYEN | UCR1_RTSDEN),
 			UCR1);
-	old_ucr2 = imx_uart_readl(sport, UCR2);
 	imx_uart_writel(sport, old_ucr2 & ~UCR2_ATEN, UCR2);
 
 	while (!(imx_uart_readl(sport, USR2) & USR2_TXDC))
@@ -1640,7 +1646,6 @@ imx_uart_set_termios(struct uart_port *port, struct ktermios *termios,
 
 	/* then, disable everything */
 	imx_uart_writel(sport, old_ucr2 & ~(UCR2_TXEN | UCR2_RXEN | UCR2_ATEN), UCR2);
-	old_ucr2 &= (UCR2_TXEN | UCR2_RXEN | UCR2_ATEN);
 
 	/* custom-baudrate handling */
 	div = sport->port.uartclk / (baud * 16);
@@ -1678,8 +1683,7 @@ imx_uart_set_termios(struct uart_port *port, struct ktermios *termios,
 
 	imx_uart_writel(sport, old_ucr1, UCR1);
 
-	/* set the parity, stop bits and data size */
-	imx_uart_writel(sport, ucr2 | old_ucr2, UCR2);
+	imx_uart_writel(sport, ucr2, UCR2);
 
 	if (UART_ENABLE_MS(&sport->port, termios->c_cflag))
 		imx_uart_enable_ms(&sport->port);
-- 
2.10.0.1.g57b01a3

^ permalink raw reply related

* [PATCH RFC v1 3/7] serial: imx: set_termios(): clarify RTS/CTS bits calculation
From: Sergey Organov @ 2019-06-20 14:47 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: Pengutronix Kernel Team, NXP Linux Team, linux-arm-kernel,
	linux-serial, Uwe Kleine-König
In-Reply-To: <1561042073-617-1-git-send-email-sorganov@gmail.com>

Avoid repeating the same code for rs485 twice.

Make it obvious we clear CRTSCTS bit in termios->c_cflag whenever
sport->have_rtscts is false.

Make it obvious we clear UCR2_IRTS whenever CRTSCTS is set.

Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
 drivers/tty/serial/imx.c | 36 +++++++++++++-----------------------
 1 file changed, 13 insertions(+), 23 deletions(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 87802fd..17e2322 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -1567,35 +1567,25 @@ imx_uart_set_termios(struct uart_port *port, struct ktermios *termios,
 	if ((termios->c_cflag & CSIZE) == CS8)
 		ucr2 |= UCR2_WS;
 
-	if (termios->c_cflag & CRTSCTS) {
-		if (sport->have_rtscts) {
-			ucr2 &= ~UCR2_IRTS;
+	if (!sport->have_rtscts)
+		termios->c_cflag &= ~CRTSCTS;
 
-			if (port->rs485.flags & SER_RS485_ENABLED) {
-				/*
-				 * RTS is mandatory for rs485 operation, so keep
-				 * it under manual control and keep transmitter
-				 * disabled.
-				 */
-				if (port->rs485.flags &
-				    SER_RS485_RTS_AFTER_SEND)
-					imx_uart_rts_active(sport, &ucr2);
-				else
-					imx_uart_rts_inactive(sport, &ucr2);
-			} else {
-				imx_uart_rts_auto(sport, &ucr2);
-			}
-		} else {
-			termios->c_cflag &= ~CRTSCTS;
-		}
-	} else if (port->rs485.flags & SER_RS485_ENABLED) {
-		/* disable transmitter */
+	if (port->rs485.flags & SER_RS485_ENABLED) {
+		/*
+		 * RTS is mandatory for rs485 operation, so keep
+		 * it under manual control and keep transmitter
+		 * disabled.
+		 */
 		if (port->rs485.flags & SER_RS485_RTS_AFTER_SEND)
 			imx_uart_rts_active(sport, &ucr2);
 		else
 			imx_uart_rts_inactive(sport, &ucr2);
-	}
 
+	} else if (termios->c_cflag & CRTSCTS)
+		imx_uart_rts_auto(sport, &ucr2);
+
+	if (termios->c_cflag & CRTSCTS)
+		ucr2 &= ~UCR2_IRTS;
 
 	if (termios->c_cflag & CSTOPB)
 		ucr2 |= UCR2_STPB;
-- 
2.10.0.1.g57b01a3

^ permalink raw reply related

* [PATCH RFC v1 2/7] serial: imx: set_termios(): factor-out 'ucr2' initial value
From: Sergey Organov @ 2019-06-20 14:47 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: Pengutronix Kernel Team, NXP Linux Team, linux-arm-kernel,
	linux-serial, Uwe Kleine-König
In-Reply-To: <1561042073-617-1-git-send-email-sorganov@gmail.com>

Set common bits in a separate statement to make initialization
explicit and not repeat the common part.

Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
 drivers/tty/serial/imx.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 1055124..87802fd 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -1563,10 +1563,9 @@ imx_uart_set_termios(struct uart_port *port, struct ktermios *termios,
 
 	spin_lock_irqsave(&sport->port.lock, flags);
 
+	ucr2 = UCR2_SRST | UCR2_IRTS;
 	if ((termios->c_cflag & CSIZE) == CS8)
-		ucr2 = UCR2_WS | UCR2_SRST | UCR2_IRTS;
-	else
-		ucr2 = UCR2_SRST | UCR2_IRTS;
+		ucr2 |= UCR2_WS;
 
 	if (termios->c_cflag & CRTSCTS) {
 		if (sport->have_rtscts) {
-- 
2.10.0.1.g57b01a3

^ permalink raw reply related

* [PATCH RFC v1 1/7] serial: imx: fix locking in set_termios()
From: Sergey Organov @ 2019-06-20 14:47 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: Pengutronix Kernel Team, NXP Linux Team, linux-arm-kernel,
	linux-serial, Uwe Kleine-König
In-Reply-To: <1561042073-617-1-git-send-email-sorganov@gmail.com>

imx_uart_set_termios() called imx_uart_rts_active(), or
imx_uart_rts_inactive() before taking port->port.lock.

As a consequence, sport->port.mctrl that these functions modify
could have been changed without holding port->port.lock.

Moved locking of port->port.lock above the calls to fix the issue.

Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
 drivers/tty/serial/imx.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index dff75dc..1055124 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -383,6 +383,7 @@ static void imx_uart_ucrs_restore(struct imx_port *sport,
 }
 #endif
 
+/* called with port.lock taken and irqs caller dependent */
 static void imx_uart_rts_active(struct imx_port *sport, u32 *ucr2)
 {
 	*ucr2 &= ~(UCR2_CTSC | UCR2_CTS);
@@ -391,6 +392,7 @@ static void imx_uart_rts_active(struct imx_port *sport, u32 *ucr2)
 	mctrl_gpio_set(sport->gpios, sport->port.mctrl);
 }
 
+/* called with port.lock taken and irqs caller dependent */
 static void imx_uart_rts_inactive(struct imx_port *sport, u32 *ucr2)
 {
 	*ucr2 &= ~UCR2_CTSC;
@@ -400,6 +402,7 @@ static void imx_uart_rts_inactive(struct imx_port *sport, u32 *ucr2)
 	mctrl_gpio_set(sport->gpios, sport->port.mctrl);
 }
 
+/* called with port.lock taken and irqs caller dependent */
 static void imx_uart_rts_auto(struct imx_port *sport, u32 *ucr2)
 {
 	*ucr2 |= UCR2_CTSC;
@@ -1550,6 +1553,16 @@ imx_uart_set_termios(struct uart_port *port, struct ktermios *termios,
 		old_csize = CS8;
 	}
 
+	del_timer_sync(&sport->timer);
+
+	/*
+	 * Ask the core to calculate the divisor for us.
+	 */
+	baud = uart_get_baud_rate(port, termios, old, 50, port->uartclk / 16);
+	quot = uart_get_divisor(port, baud);
+
+	spin_lock_irqsave(&sport->port.lock, flags);
+
 	if ((termios->c_cflag & CSIZE) == CS8)
 		ucr2 = UCR2_WS | UCR2_SRST | UCR2_IRTS;
 	else
@@ -1593,16 +1606,6 @@ imx_uart_set_termios(struct uart_port *port, struct ktermios *termios,
 			ucr2 |= UCR2_PROE;
 	}
 
-	del_timer_sync(&sport->timer);
-
-	/*
-	 * Ask the core to calculate the divisor for us.
-	 */
-	baud = uart_get_baud_rate(port, termios, old, 50, port->uartclk / 16);
-	quot = uart_get_divisor(port, baud);
-
-	spin_lock_irqsave(&sport->port.lock, flags);
-
 	sport->port.read_status_mask = 0;
 	if (termios->c_iflag & INPCK)
 		sport->port.read_status_mask |= (URXD_FRMERR | URXD_PRERR);
-- 
2.10.0.1.g57b01a3

^ permalink raw reply related

* [PATCH RFC v1 0/7] serial: imx: fix RTS and RTS/CTS handling
From: Sergey Organov @ 2019-06-20 14:47 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: Pengutronix Kernel Team, NXP Linux Team, linux-arm-kernel,
	linux-serial, Uwe Kleine-König
In-Reply-To: <20190614072801.3187-1-s.hauer@pengutronix.de>

The patches are not tested yet, so the RFC in the header. I'll re-roll
without RFC once Sasha Hauer tests them.

Sasha, in addition to already discussed fixes, I've also reordered 2
patches so that the sequence makes sense.

Changelog:

  v1:

      * Fixed in "serial: imx: set_termios(): preserve RTS state"

-+	ucr2 = UCR2_SRST | UCR2_IRTS;
++	ucr2 |= UCR2_SRST | UCR2_IRTS;
      
        as noticed by Lothar Waßmann <LW@KARO-electronics.de>

      * Fixed in "serial: imx: set_termios(): preserve RTS state"
      
-+	ucr2 = old_ucr2 & (UCR2_TXEN | UCR2_RXEN | UCR2_ATEN | UCR2_CTSC);
++	ucr2 = old_ucr2 & (UCR2_TXEN | UCR2_RXEN | UCR2_ATEN | UCR2_CTS);

        as the fix for the problem found by Sascha Hauer
        <s.hauer@pengutronix.de>

      * Reordered:

        serial: imx: set_termios(): preserve RTS state
        serial: imx: set_termios(): do not enable autoRTS if RTS is unset

        as the latter makes sense only provided the former is already applied.
      

Sergey Organov (7):
  serial: imx: fix locking in set_termios()
  serial: imx: set_termios(): factor-out 'ucr2' initial value
  serial: imx: set_termios(): clarify RTS/CTS bits calculation
  serial: imx: set_termios(): preserve RTS state
  serial: imx: set_termios(): do not enable autoRTS if RTS is unset
  serial: imx: set_mctrl(): correctly restore autoRTS state
  serial: imx: get rid of imx_uart_rts_auto()

 drivers/tty/serial/imx.c | 93 ++++++++++++++++++++++++------------------------
 1 file changed, 47 insertions(+), 46 deletions(-)

--
2.10.0.1.g57b01a3

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] drm/self_refresh: Fix possible NULL deref in failure path
From: Sean Paul @ 2019-06-20 14:47 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Zain Wang, Maxime Ripard, Sam Ravnborg, Jose Souza, Tomasz Figa,
	David Airlie, Sean Paul, dri-devel, Dan Carpenter, Sean Paul
In-Reply-To: <20190620112855.GQ12905@phenom.ffwll.local>

On Thu, Jun 20, 2019 at 01:28:55PM +0200, Daniel Vetter wrote:
> On Wed, Jun 19, 2019 at 02:19:47PM -0400, Sean Paul wrote:
> > From: Sean Paul <seanpaul@chromium.org>
> > 
> > If state allocation fails, we still try to give back the reference on
> > it. Also initialize ret in case the crtc is not enabled and we hit the
> > eject button.
> > 
> > Fixes: 1452c25b0e60 ("drm: Add helpers to kick off self refresh mode in drivers")
> > Cc: Daniel Vetter <daniel@ffwll.ch>
> > Cc: Jose Souza <jose.souza@intel.com>
> > Cc: Zain Wang <wzz@rock-chips.com>
> > Cc: Tomasz Figa <tfiga@chromium.org>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Sam Ravnborg <sam@ravnborg.org>
> > Cc: Sean Paul <seanpaul@chromium.org>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Maxime Ripard <maxime.ripard@bootlin.com>
> > Cc: Sean Paul <sean@poorly.run>
> > Cc: David Airlie <airlied@linux.ie>
> > Cc: dri-devel@lists.freedesktop.org
> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> > Signed-off-by: Sean Paul <seanpaul@chromium.org>
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 

Applied to -misc-next, thanks!

Sean

> > ---
> >  drivers/gpu/drm/drm_self_refresh_helper.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_self_refresh_helper.c b/drivers/gpu/drm/drm_self_refresh_helper.c
> > index e0d2ad1f070cb..4b9424a8f1f1c 100644
> > --- a/drivers/gpu/drm/drm_self_refresh_helper.c
> > +++ b/drivers/gpu/drm/drm_self_refresh_helper.c
> > @@ -69,14 +69,14 @@ static void drm_self_refresh_helper_entry_work(struct work_struct *work)
> >  	struct drm_connector *conn;
> >  	struct drm_connector_state *conn_state;
> >  	struct drm_crtc_state *crtc_state;
> > -	int i, ret;
> > +	int i, ret = 0;
> >  
> >  	drm_modeset_acquire_init(&ctx, 0);
> >  
> >  	state = drm_atomic_state_alloc(dev);
> >  	if (!state) {
> >  		ret = -ENOMEM;
> > -		goto out;
> > +		goto out_drop_locks;
> >  	}
> >  
> >  retry:
> > @@ -116,6 +116,8 @@ static void drm_self_refresh_helper_entry_work(struct work_struct *work)
> >  	}
> >  
> >  	drm_atomic_state_put(state);
> > +
> > +out_drop_locks:
> >  	drm_modeset_drop_locks(&ctx);
> >  	drm_modeset_acquire_fini(&ctx);
> >  }
> > -- 
> > Sean Paul, Software Engineer, Google / Chromium OS
> > 
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* [PATCH] pwm: meson: fix the G12A AO clock parents order
From: Neil Armstrong @ 2019-06-20 14:46 UTC (permalink / raw)
  To: thierry.reding
  Cc: linux-pwm, Neil Armstrong, martin.blumenstingl, linux-kernel,
	linux-amlogic, linux-arm-kernel

The Amlogic G12A and G12B Documentation is wrong, the AO xtal and clk81
clock source order is reversed, and validated when adding DVFS support by
using the PWM AO D output to control the CPU supply voltage.

The vendor tree also uses the reversed xtal and clk81 order at [1].

[1] https://github.com/hardkernel/linux/blob/odroidn2-4.9.y/drivers/amlogic/pwm/pwm_meson.c#L462

Fixes: f41efceb46e6 ("pwm: meson: Add clock source configuration for Meson G12A")
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/pwm/pwm-meson.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c
index 5e65b042c240..e15d045947bb 100644
--- a/drivers/pwm/pwm-meson.c
+++ b/drivers/pwm/pwm-meson.c
@@ -453,8 +453,17 @@ static const struct meson_pwm_data pwm_axg_ao_data = {
 	.num_parents = ARRAY_SIZE(pwm_axg_ao_parent_names),
 };
 
+static const char * const pwm_g12a_ao_ab_parent_names[] = {
+	"xtal", "aoclk81", "fclk_div4", "fclk_div5"
+};
+
+static const struct meson_pwm_data pwm_g12a_ao_ab_data = {
+	.parent_names = pwm_g12a_ao_ab_parent_names,
+	.num_parents = ARRAY_SIZE(pwm_g12a_ao_ab_parent_names),
+};
+
 static const char * const pwm_g12a_ao_cd_parent_names[] = {
-	"aoclk81", "xtal",
+	"xtal", "aoclk81",
 };
 
 static const struct meson_pwm_data pwm_g12a_ao_cd_data = {
@@ -498,7 +507,7 @@ static const struct of_device_id meson_pwm_matches[] = {
 	},
 	{
 		.compatible = "amlogic,meson-g12a-ao-pwm-ab",
-		.data = &pwm_axg_ao_data
+		.data = &pwm_g12a_ao_ab_data
 	},
 	{
 		.compatible = "amlogic,meson-g12a-ao-pwm-cd",
-- 
2.21.0


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply related

* RE: [PATCH 2/3] crypto: inside-secure - add support for PCI based FPGA development board
From: Pascal Van Leeuwen @ 2019-06-20 14:47 UTC (permalink / raw)
  To: Antoine Tenart
  Cc: Pascal van Leeuwen, linux-crypto@vger.kernel.org,
	herbert@gondor.apana.org.au, davem@davemloft.net
In-Reply-To: <20190620130609.GA4642@kwain>

> -----Original Message-----
> From: Antoine Tenart <antoine.tenart@bootlin.com>
> Sent: Thursday, June 20, 2019 3:06 PM
> To: Pascal Van Leeuwen <pvanleeuwen@insidesecure.com>
> Cc: Antoine Tenart <antoine.tenart@bootlin.com>; Pascal van Leeuwen <pascalvanl@gmail.com>; linux-crypto@vger.kernel.org;
> herbert@gondor.apana.org.au; davem@davemloft.net
> Subject: Re: [PATCH 2/3] crypto: inside-secure - add support for PCI based FPGA development board
> 
> Hi Pascal,
> 
> On Wed, Jun 19, 2019 at 02:22:19PM +0000, Pascal Van Leeuwen wrote:
> > > From: Antoine Tenart <antoine.tenart@bootlin.com>
> > > On Tue, Jun 18, 2019 at 07:56:23AM +0200, Pascal van Leeuwen wrote:
> > > >
> > > >  			/* Fallback to the old firmware location for the
> > > > @@ -294,6 +291,9 @@ static int safexcel_hw_init(struct safexcel_crypto_priv *priv)
> > > >
> > > > +	dev_info(priv->dev, "EIP(1)97 HW init: burst size %d beats, using %d pipe(s) and %d
> > > ring(s)",
> > > > +			16, priv->config.pes, priv->config.rings);
> > >
> > > Adding custom messages in the kernel log has to be done carefully.
> > > Although it's not considered stable it could be difficult to rework
> > > later on. Also, if all driver were to print custom messages the log
> > > would be very hard to read. But you can also argue that a single message
> > > when probing a driver is also done in other drivers.
> > >
> > Hmm ... don't know what the rules for logging are exactly, but from my
> > perspective, I'm dealing with a zillion different HW configurations so
> > some feedback whether the driver detected the *correct* HW parameters -
> > or actually, whether I stuffed the correct image into my FPGA :o) - is
> > very convenient to have. And not just for my local development, but also
> > to debug deployments in the field at customer sites.
> 
> I understand it can be convenient, it's just a matter of having a
> logging message for you that will end up in many builds for many users.
> They do not necessarily have the same needs. So it's a matter of
> compromise, one or two messages at boot can be OK, more is likely to
> become an issue.
> 
OK, got it. So I have to stuff all my logging into one or two very long lines :-P
(just kidding)

> > Also, looking at my log, there's other drivers that are similarly
> > (or even more) verbose.
> 
> There are *always* other examples in the kernel :)
> 
> > If it's a really considered a problem, I could make it dev_dbg?
> 
> Sure. I would say adding one message at boot like this one seems OK. But
> the others would need to be debugging messages.
> 
OK, then that's what I'll really do.

> > > For this one particularly, the probe could fail later on. So if we were
> > > to add this output, it should be done at the very end of the probe.
> > >
> > I'm in doubt about this one. I understand that you want to reduce the
> > logging in that case, but at the same time that message can convey
> > information as to WHY the probing fails later on ...
> 
> If the drivers fails to probe, there will be other messages. In that
> case is this one really needed? I'm not sure.
> 
> > i.e. if it detects, say, 4 pipes on a device that, in fact, only has
> > 2, then that may be the very reason for the FW init to fail later on.
> 
> In case of failure you'll need anyway to debug and understand what's
> going on. By adding new prints, or enabling debugging messages.
> 
If it fails for me locally, I can do that. If it somehow fails "in the field",
I think most people won't be able to recompile their own Linux
kernel with debug messages let alone add their own debug messages.

Anyway, I'll just make everything dev_dbg to avoid further discussion.

> > > > -static int safexcel_request_ring_irq(struct platform_device *pdev, const char *name,
> > > > +static int safexcel_request_ring_irq(void *pdev, int irqid,
> > > > +				     int is_pci_dev,
> > >
> > > You could probably use the DEVICE_IS_PCI flag instead.
> > >
> > I'm not currently passing priv to that function, so I can't access
> > priv->version directly. I could pass a priv pointer instead and use
> > the flag, but passing a bool constant seemed to be more efficient?
> 
> That right, whatever you prefer then.
> 
Ok, then I'll leave it as it is.

> > > > +static int safexcel_probe_generic(void *pdev,
> > > > +				  struct safexcel_crypto_priv *priv,
> > > > +				  int is_pci_dev)
> > > >  {
> > >
> > > > +	if (IS_ENABLED(CONFIG_PCI) && (priv->version & DEVICE_IS_PCI)) {
> > >
> > > DEVICE_IS_PCI should be set in ->flags, not ->version.
> > >
> > As far as I could tell from the existing code, "version" was
> > a highlevel indication of the device *type* (EIP97/EIP197B/D)
> > while flags was more about low-level *features*.
> 
> Using the same argument I would say the way the device is accessed is
> very run-time dependent, and it has nothing to do with the version of
> the h/w (the same engine could be accessed through different ways given
> the platform).
> 
> Maybe the name 'feature' is too specific, I agree with you.
> 
> > So in my humble opinion, version was the correct location, it
> > is just a confusing name. (i.e. you can have many *versions*
> > of an EIP197B, for instance ...)
> 
> That would be an issue with the driver. We named the 'version' given the
> knowledge we had of the h/w, it might not be specific enough. Or maybe
> you can think of this as being a "family of engine versions". The idea
> is the version is what the h/w is capable of, not how it's being
> wired/accessed.
> 

Well ... I want to avoid the whole discussion about the naming of the
variable (which can be trivially changed) and what the intention may
have been,  if you allow me.

Fact is ... this variable is what receives .data / .driver_data from the 
OF or PCI match table. So it is a means of conveying a value that is 
specific to the table entry that was matched. No more,  no less.
In "your" device tree case you want to distinguish between 
Armada 39x, Armada 7K/8K and Armada 9K. In "my" PCI case I
want to potentially distinguish multiple FPGA boards/images.

It wouldn't make much sense to me to do the vendor/subvendor/
device/subdevice decoding all over again in my probe routine.
So what exactly is so very wrong with the way I'm doing this?

> > > > +		/*
> > > > +		 * Request MSI vectors for global + 1 per ring -
> > > > +		 * or just 1 for older dev images
> > > > +		 */
> > > > +		struct pci_dev *pci_pdev = pdev;
> > > > +
> > > > +		ret = pci_alloc_irq_vectors(pci_pdev,
> > > > +					    priv->config.rings + 1,
> > > > +					    priv->config.rings + 1,
> > > > +					    PCI_IRQ_MSI|PCI_IRQ_MSIX);
> > >
> > > You need a space before and after the | here.
> > >
> > Ok, will add (checkpatch did not complain though)
> 
> It may complain with --strict on this one. (Well, I'm sure many --strict
> tests won't pass on the existing upstream code, no worries :)).
> 
> > > > +		if (ret < 0) {
> > > > +			dev_err(dev, "Failed to allocate PCI MSI interrupts");
> > >
> > > Do not forget the \n at the end of the string.
> > >
> > Actually, I removed all "\n"'s from my messages as I
> > understood they are not needed? If they are really needed,
> > I can add them to all log strings again ... anyone else?
> 
> The simple answer is: everybody set the \n, so at least for consistency
> we should have them.
> 
I did a bit of side-investigation to conclude the same. So I already added them.

> > > > @@ -1189,13 +1249,12 @@ static int safexcel_remove(struct platform_device *pdev)
> > > >  		.compatible = "inside-secure,safexcel-eip197d",
> > > >  		.data = (void *)EIP197D,
> > > >  	},
> > > > +	/* For backward compatibiliry and intended for generic use */
> > > >  	{
> > > > -		/* Deprecated. Kept for backward compatibility. */
> > > >  		.compatible = "inside-secure,safexcel-eip97",
> > > >  		.data = (void *)EIP97IES,
> > > >  	},
> > > >  	{
> > > > -		/* Deprecated. Kept for backward compatibility. */
> > > >  		.compatible = "inside-secure,safexcel-eip197",
> > > >  		.data = (void *)EIP197B,
> > > >  	},
> > >
> > > I'm not sure about this. The compatible should describe what the
> > > hardware is, and the driver can then decide if it has special things to
> > > do or not. It is not used to configure the driver to be used with a
> > > generic use or not.
> > >
> > > Do you have a practical reason to do this?
> >
> > I have to admit I don't fully understand how these compatible
> > strings work. All I wanted to achieve is provide some generic
> > device tree entry to point to this driver, to be used for
> > devices other than Marvell. No need to convey b/d that way
> > (or even eip97/197, for that matter) as that can all be probed.
> 
> Compatibles are used in device trees, which intend to be a description
> of the hardware (not the configuration of how the hardware should be
> used). So we can't have a compatible being a restricted configuration
> use of a given hardware. But I think here you're right, and there is
> room for a more generic eip197 compatible: the b/d versions only have
> few differences and are part of the same family, so we can have a very
> specific compatible plus a "family" one. Something like:
> 
>   compatible = "inside-secure,safexcel-eip197d", "inside-secure,safexcel-eip197";
> 
> This would need to be in a separated patch, and this should be
> documented in:
> Documentation/devicetree/bindings/crypto/inside-secure-safexcel.txt
> 
Ok, then I'll leave that part untouched for now.
(I only changed the comments anyway ...)

> > > > +		/* enable the device */
> > > > +		rc = pcim_enable_device(pdev);
> > > > +		if (rc) {
> > > > +			dev_err(dev, "pci_enable_device() failed");
> > >
> > > Please use error messages describing the issue rather than printing the
> > > function names. (There are other examples).
> > >
> > But ... that IS the issue, isn't it? Well, apart from the actual
> > return code. What message text would you suggest?
> 
> Yes, it is the issue. We generally use a sentence instead of function
> names (something alongside s/_/ / :)).
> 
Fine, will do.

> > > > +			/* HW reset FPGA dev board */
> > > > +			// assert reset
> > > > +			writel(1, priv->base + XILINX_GPIO_BASE);
> > > > +			wmb(); /* maintain strict ordering for accesses here */
> > > > +			// deassert reset
> > > > +			writel(0, priv->base + XILINX_GPIO_BASE);
> > > > +			wmb(); /* maintain strict ordering for accesses here */
> > >
> > > It seems the driver here access to a GPIO controller, to assert and
> > > de-assert reset, which is outside the crypto engine i/o range. If true,
> > > this is not acceptable in the upstream kernel: those GPIO or reset
> > > should be accessed through the dedicated in-kernel API and be
> > > implemented in separate drivers (maybe a reset driver here).
> > >
> > Hmmm ... this is some *local* GPIO controller *inside*
> > the FPGA device (i.e. doesn't even leave the silicon die).
> > It's inside the range of the (single!) PCIE device, it's not
> > a seperate device and thus cannot be managed as such ...
> >
> > Effectively, it's just an MMIO mapped register no different
> > from any other slave accessible register.
> > If I had given it a less explicit name, nobody would've cared.
> 
> OK, that makes sense. If the register accessed is within the register
> bank of the crypto engine you can keep this.
> 
> > > > +static struct pci_driver crypto_is_pci_driver = {
> > > > +	.name          = "crypto-safexcel",
> > > > +	.id_table      = crypto_is_pci_ids,
> > > > +	.probe         = crypto_is_pci_probe,
> > > > +	.remove        = crypto_is_pci_remove,
> > > > +};
> > >
> > > More generally, you should protect all the PCI specific functions and
> > > definitions between #ifdef.
> > >
> > I asked the mailing list and the answer was I should NOT use #ifdef,
> > but instead use IS_ENABLED to *only* remove relevant function bodies.
> > Which is exactly what I did (or tried to do, anyway).
> 
> My bad, I realise there's a mistake in my comment. That should have
> been: you should protect all the PCI specific functions and definitions
> with #if IS_ENABLED(...). When part of a function should be excluded
> you can use if(IS_ENABLED(...)), but if the entire function can be left
> out, #if is the way to go.
> 
Ok  #if instead of if or #ifdef,that makes sense.
So can I just put all the PCI stuff into one big #if then?

Thanks,

Pascal van Leeuwen
Silicon IP Architect, Multi-Protocol Engines @ Inside Secure
www.insidesecure.com


^ permalink raw reply

* Re: No invalid histogram error
From: Zanussi, Tom @ 2019-06-20 14:47 UTC (permalink / raw)
  To: Masami Hiramatsu; +Cc: linux-kernel, Steven Rostedt
In-Reply-To: <20190620211737.993b09619af1fc58222549b4@kernel.org>

Hi Masami,

On 6/20/2019 7:17 AM, Masami Hiramatsu wrote:
> Hi Tom,
>
> I'm trying to use histogram on a synthetic event, but faced an odd situation.
>
> There is a synthetic event, which has foo and bar.
>
> /sys/kernel/debug/tracing # cat synthetic_events
> testevent	int foo; int bar
>
> And when I tried to add hist on trigger, both foo and bar can be used as below.
>
> /sys/kernel/debug/tracing # echo "hist:keys=bar" > events/synthetic/testevent/trigger
> /sys/kernel/debug/tracing # echo "hist:keys=foo" > events/synthetic/testevent/trigger
>
> And, when I missed to specify the sort key, it failed
>
> /sys/kernel/debug/tracing # echo "hist:keys=foo:sort=bar" > events/synthetic/testevent/trigger
> sh: write error: Invalid argument
>
> But no error on error_log file.
>
> /sys/kernel/debug/tracing # cat error_log
> /sys/kernel/debug/tracing #
>
> Could you add some error message? What about below?
>
> [ 5422.134656] hist:synthetic:testevent: error: Sort key must be one of keys.
>    Command: keys=foo;sort=bar
>                                 ^


Sure, I'll submit a fix for this.  Thanks for pointing it out.


Tom


> Thank you,
>
>

^ permalink raw reply

* Re: [PATCH V6 04/27] PCI: tegra: Mask AFI_INTR in runtime suspend
From: Manikanta Maddireddy @ 2019-06-20 14:46 UTC (permalink / raw)
  To: Lorenzo Pieralisi
  Cc: thierry.reding, bhelgaas, robh+dt, mark.rutland, jonathanh,
	vidyas, linux-tegra, linux-pci, devicetree
In-Reply-To: <20190620142702.GA31996@e121166-lin.cambridge.arm.com>



On 20-Jun-19 7:57 PM, Lorenzo Pieralisi wrote:
> On Tue, Jun 18, 2019 at 11:31:43PM +0530, Manikanta Maddireddy wrote:
>> AFI_INTR is unmasked in tegra_pcie_enable_controller(), mask it to avoid
>> unwanted interrupts raised by AFI after pex_rst is asserted.
>>
>> Following sequence triggers such scenario,
>>  - tegra_pcie_remove() triggers runtime suspend
>>  - pex_rst is asserted in runtime suspend
>>  - PRSNT_MAP bit field in RP_PRIV_MISC register changes from EP_PRSNT to
>>    EP_ABSNT
>>  - This is sensed by AFI and triggers "Slot present pin change" interrupt
>>  - tegra_pcie_isr() function accesses AFI register when runtime suspend
>>    is going through power off sequence
>>
>> rmmod pci-tegra
>>  pci_generic_config_write32: 108 callbacks suppressed
>>  pci_bus 0002:00: 2-byte config write to 0002:00:02.0 offset 0x4c may corrupt adjacent RW1C bits
>>  pci_bus 0002:00: 2-byte config write to 0002:00:02.0 offset 0x9c may corrupt adjacent RW1C bits
>>  pci_bus 0002:00: 2-byte config write to 0002:00:02.0 offset 0x88 may corrupt adjacent RW1C bits
>>  pci_bus 0002:00: 2-byte config write to 0002:00:02.0 offset 0x90 may corrupt adjacent RW1C bits
>>  pci_bus 0002:00: 2-byte config write to 0002:00:02.0 offset 0x4 may corrupt adjacent RW1C bits
>>  igb 0002:04:00.1: removed PHC on enP2p4s0f1
>>  igb 0002:04:00.0: removed PHC on enP2p4s0f0
>>  pci_bus 0002:00: 2-byte config write to 0002:00:01.0 offset 0x4c may corrupt adjacent RW1C bits
>>  pci_bus 0002:00: 2-byte config write to 0002:00:01.0 offset 0x9c may corrupt adjacent RW1C bits
>>  pci_bus 0002:00: 2-byte config write to 0002:00:01.0 offset 0x88 may corrupt adjacent RW1C bits
>>  pci_bus 0002:00: 2-byte config write to 0002:00:01.0 offset 0x90 may corrupt adjacent RW1C bits
>>  pci_bus 0002:00: 2-byte config write to 0002:00:01.0 offset 0x4 may corrupt adjacent RW1C bits
>>  rcu: INFO: rcu_preempt self-detected stall on CPU
>>  SError Interrupt on CPU0, code 0xbf000002 -- SError
>>  CPU: 0 PID: 0 Comm: swapper/0 Tainted: G        W         5.1.0-rc3-next-20190405-00027-gcd8110499e6f-dirty #42
>>  Hardware name: NVIDIA Jetson TX1 Developer Kit (DT)
>>  pstate: 20000085 (nzCv daIf -PAN -UAO)
>>  pc : tegra_pcie_isr+0x58/0x178 [pci_tegra]
>>  lr : tegra_pcie_isr+0x40/0x178 [pci_tegra]
>>  sp : ffff000010003da0
>>  x29: ffff000010003da0 x28: 0000000000000000
>>  x27: ffff8000f9e61000 x26: ffff000010fbf420
>>  x25: ffff000011427f93 x24: ffff8000fa600410
>>  x23: ffff00001129d000 x22: ffff00001129d000
>>  x21: ffff8000f18bf3c0 x20: 0000000000000070
>>  x19: 00000000ffffffff x18: 0000000000000000
>>  x17: 0000000000000000 x16: 0000000000000000
>>  x15: 0000000000000000 x14: ffff000008d40a48
>>  x13: ffff000008d40a30 x12: ffff000008d40a20
>>  x11: ffff000008d40a10 x10: ffff000008d40a00
>>  x9 : ffff000008d409e8 x8 : ffff000008d40ae8
>>  x7 : ffff000008d40ad0 x6 : ffff000010003e58
>>  x5 : ffff8000fac00248 x4 : 0000000000000000
>>  x3 : ffff000008d40b08 x2 : fffffffffffffff8
>>  x1 : ffff000008d3f4e8 x0 : 00000000ffffffff
>>  Kernel panic - not syncing: Asynchronous SError Interrupt
>>  CPU: 0 PID: 0 Comm: swapper/0 Tainted: G        W         5.1.0-rc3-next-20190405-00027-gcd8110499e6f-dirty #42
>>  Hardware name: NVIDIA Jetson TX1 Developer Kit (DT)
>>  Call trace:
>>   dump_backtrace+0x0/0x158
>>   show_stack+0x14/0x20
>>   dump_stack+0xa8/0xcc
>>   panic+0x140/0x2f4
>>   nmi_panic+0x6c/0x70
>>   arm64_serror_panic+0x74/0x80
>>   __pte_error+0x0/0x28
>>   el1_error+0x84/0xf8
>>   tegra_pcie_isr+0x58/0x178 [pci_tegra]
>>   __handle_irq_event_percpu+0x70/0x198
>>   handle_irq_event_percpu+0x34/0x88
>>   handle_irq_event+0x48/0x78
>>   handle_fasteoi_irq+0xb4/0x190
>>   generic_handle_irq+0x24/0x38
>>   __handle_domain_irq+0x5c/0xb8
>>   gic_handle_irq+0x58/0xa8
>>   el1_irq+0xb8/0x180
>>   cpuidle_enter_state+0x138/0x358
>>   cpuidle_enter+0x18/0x20
>>   call_cpuidle+0x1c/0x48
>>   do_idle+0x230/0x2d0
>>   cpu_startup_entry+0x20/0x28
>>   rest_init+0xd4/0xe0
>>   arch_call_rest_init+0xc/0x14
>>   start_kernel+0x444/0x470
>>
>> Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com>
>> Acked-by: Thierry Reding <treding@nvidia.com>
>> ---
>> V6: No change
>>
>> V5:
>> * Added blank line before block-style comment
>>
>> V4: No change
>>
>> V3:
>> * Update the commit log and comment to reflect why this fix is required
>> * MSI interrupt is not disabled
>>
>> V2: This is new patch in V2
>>
>>  drivers/pci/controller/pci-tegra.c | 15 +++++++++++++++
>>  1 file changed, 15 insertions(+)
>>
>> diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c
>> index bb3c0af9c830..0453bfb2726e 100644
>> --- a/drivers/pci/controller/pci-tegra.c
>> +++ b/drivers/pci/controller/pci-tegra.c
>> @@ -1622,6 +1622,15 @@ static int tegra_pcie_disable_msi(struct tegra_pcie *pcie)
>>  	return 0;
>>  }
>>  
>> +static void tegra_pcie_disable_interrupts(struct tegra_pcie *pcie)
>> +{
>> +	u32 value;
>> +
>> +	value = afi_readl(pcie, AFI_INTR_MASK);
>> +	value &= ~AFI_INTR_MASK_INT_MASK;
>> +	afi_writel(pcie, value, AFI_INTR_MASK);
>> +}
>> +
>>  static int tegra_pcie_get_xbar_config(struct tegra_pcie *pcie, u32 lanes,
>>  				      u32 *xbar)
>>  {
>> @@ -2467,6 +2476,12 @@ static int __maybe_unused tegra_pcie_pm_suspend(struct device *dev)
>>  
>>  	tegra_pcie_disable_ports(pcie);
>>  
>> +	/*
>> +	 * AFI_INTR is unmasked in tegra_pcie_enable_controller(), mask it to
>> +	 * avoid unwanted interrupts raised by AFI after pex_rst is asserted.
>> +	 */
>> +	tegra_pcie_disable_interrupts(pcie);
> When do you re-enable it ? I assume it is enabled by default for
> a reason, so if you disable on suspend you renable it on resume.
>
> Please explain or I will drop this patch from the series.
>
> Lorenzo

Power on reset value of AFI_INTR_MASK_INT_MASK is 0, it is not enabled by default.
In suspend AFI reset is asserted, so in resume AFI programming has to be done
again including AFI_INTR_MASK_INT_MASK. Even if I don't disable here, in resume
it has to be enabled after bringing AFI out of reset.

tegra_pcie_pm_resume() -> tegra_pcie_enable_controller() will enable it.

Manikanta 

>
>> +
>>  	if (pcie->soc->program_uphy) {
>>  		err = tegra_pcie_phy_power_off(pcie);
>>  		if (err < 0)
>> -- 
>> 2.17.1
>>


^ permalink raw reply

* Re: [PATCH] backlight: gpio-backlight: Set power state instead of brightness at probe
From: Peter Ujfalusi @ 2019-06-20 14:47 UTC (permalink / raw)
  To: Daniel Thompson, Paul Kocialkowski, dri-devel, linux-fbdev,
	linux-kernel
  Cc: Jingoo Han, Lee Jones, Laurent Pinchart, Thomas Petazzoni,
	Bartlomiej Zolnierkiewicz
In-Reply-To: <29712212-0567-702b-fbc9-c0f37806d84c@linaro.org>

Daniel,

On 20/06/2019 16.56, Daniel Thompson wrote:
> On 18/06/2019 13:58, Paul Kocialkowski wrote:
>> Hi,
>>
>> On Fri, 2019-05-17 at 17:05 +0200, Paul Kocialkowski wrote:
>>> On a trivial gpio-backlight setup with a panel using the backlight but
>>> no boot software to enable it beforehand, we fall in a case where the
>>> backlight is disabled (not just blanked) and thus remains disabled when
>>> the panel gets enabled.
>>>
>>> Setting gbl->def_value via the device-tree prop allows enabling the
>>> backlight in this situation, but it will be unblanked straight away,
>>> in compliance with the binding. This does not work well when there
>>> was no
>>> boot software to display something before, since we really need to
>>> unblank
>>> by the time the panel is enabled, not before.
>>>
>>> Resolve the situation by setting the brightness to 1 at probe and
>>> managing the power state accordingly, a bit like it's done in
>>> pwm-backlight.
>>
>> Any feedback on this? I was under the impression that it could be quite
>> controversial, as it implies that the backlight can no longer be
>> enabled without a bound panel (which IMO makes good sense but could be
>> a matter to debate).
> 
> My apologies. This patch brought on such severe deja-vu I got rather
> confused. Then when I went digging I've also dropped the ball on the
> same feature previously.
> 
> Peter Ujfalusi provided a similar patch to yours but with a slightly
> different implementation:
> https://lore.kernel.org/patchwork/patch/1002359/
> 
> On the whole I think it is important to read the GPIO pin since
> otherwise we swap problems when there bootloader does setup the
> backlight for problems where it does not.
> 
> The thing I don't get is why both patches try to avoid setting the
> backlight brightness from def_value. Simple displays, especially
> monochrome ones are perfectly readable with the backlight off... zero
> brightness is not a "bad" value.

Because we might have non monochrome displays where the display is not
readable when the backlight is off and for the sake of to be consistent?
Flickering backlight is not really a nice thing during boot.

> Not sure if Peter is still willing to rev his version of this code
> (given how badly we neglected him previously) or whether you want to try
> and combine both ideas.

Nothing special, things sometimes got overlooked.
I should have been nagging you about it ;)

I think the v2 patch from me should apply just fine and it has the gpio
read as well, if not, then I might not be able to resend as I'm out of
office for a while

- Péter


> 
> 
> Daniel.
> 
> 
>>
>> Cheers,
>>
>> Paul
>>
>>> Fixes: 8b770e3c9824 ("backlight: Add GPIO-based backlight driver")
>>> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
>>> ---
>>>   drivers/video/backlight/gpio_backlight.c | 19 ++++++++++++++++++-
>>>   1 file changed, 18 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/video/backlight/gpio_backlight.c
>>> b/drivers/video/backlight/gpio_backlight.c
>>> index e470da95d806..c9cb97fa13d0 100644
>>> --- a/drivers/video/backlight/gpio_backlight.c
>>> +++ b/drivers/video/backlight/gpio_backlight.c
>>> @@ -57,6 +57,21 @@ static const struct backlight_ops
>>> gpio_backlight_ops = {
>>>       .check_fb    = gpio_backlight_check_fb,
>>>   };
>>>   +static int gpio_backlight_initial_power_state(struct
>>> gpio_backlight *gbl)
>>> +{
>>> +    struct device_node *node = gbl->dev->of_node;
>>> +
>>> +    /* If we absolutely want the backlight enabled at boot. */
>>> +    if (gbl->def_value)
>>> +        return FB_BLANK_UNBLANK;
>>> +
>>> +    /* If there's no panel to unblank the backlight later. */
>>> +    if (!node || !node->phandle)
>>> +        return FB_BLANK_UNBLANK;
>>> +
>>> +    return FB_BLANK_POWERDOWN;
>>> +}
>>> +
>>>   static int gpio_backlight_probe_dt(struct platform_device *pdev,
>>>                      struct gpio_backlight *gbl)
>>>   {
>>> @@ -142,7 +157,9 @@ static int gpio_backlight_probe(struct
>>> platform_device *pdev)
>>>           return PTR_ERR(bl);
>>>       }
>>>   -    bl->props.brightness = gbl->def_value;
>>> +    bl->props.brightness = 1;
>>> +    bl->props.power = gpio_backlight_initial_power_state(gbl);
>>> +
>>>       backlight_update_status(bl);
>>>         platform_set_drvdata(pdev, bl);
> 


Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

^ permalink raw reply

* [PATCH] pwm: meson: fix the G12A AO clock parents order
From: Neil Armstrong @ 2019-06-20 14:46 UTC (permalink / raw)
  To: thierry.reding
  Cc: linux-pwm, Neil Armstrong, martin.blumenstingl, linux-kernel,
	linux-amlogic, linux-arm-kernel

The Amlogic G12A and G12B Documentation is wrong, the AO xtal and clk81
clock source order is reversed, and validated when adding DVFS support by
using the PWM AO D output to control the CPU supply voltage.

The vendor tree also uses the reversed xtal and clk81 order at [1].

[1] https://github.com/hardkernel/linux/blob/odroidn2-4.9.y/drivers/amlogic/pwm/pwm_meson.c#L462

Fixes: f41efceb46e6 ("pwm: meson: Add clock source configuration for Meson G12A")
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/pwm/pwm-meson.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c
index 5e65b042c240..e15d045947bb 100644
--- a/drivers/pwm/pwm-meson.c
+++ b/drivers/pwm/pwm-meson.c
@@ -453,8 +453,17 @@ static const struct meson_pwm_data pwm_axg_ao_data = {
 	.num_parents = ARRAY_SIZE(pwm_axg_ao_parent_names),
 };
 
+static const char * const pwm_g12a_ao_ab_parent_names[] = {
+	"xtal", "aoclk81", "fclk_div4", "fclk_div5"
+};
+
+static const struct meson_pwm_data pwm_g12a_ao_ab_data = {
+	.parent_names = pwm_g12a_ao_ab_parent_names,
+	.num_parents = ARRAY_SIZE(pwm_g12a_ao_ab_parent_names),
+};
+
 static const char * const pwm_g12a_ao_cd_parent_names[] = {
-	"aoclk81", "xtal",
+	"xtal", "aoclk81",
 };
 
 static const struct meson_pwm_data pwm_g12a_ao_cd_data = {
@@ -498,7 +507,7 @@ static const struct of_device_id meson_pwm_matches[] = {
 	},
 	{
 		.compatible = "amlogic,meson-g12a-ao-pwm-ab",
-		.data = &pwm_axg_ao_data
+		.data = &pwm_g12a_ao_ab_data
 	},
 	{
 		.compatible = "amlogic,meson-g12a-ao-pwm-cd",
-- 
2.21.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related


This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.