Devicetree
 help / color / mirror / Atom feed
* [RFC PATCH net-next 2/4] net: phy: Add a helper to return the index for of the internal delay
From: Dan Murphy @ 2020-05-21 17:48 UTC (permalink / raw)
  To: andrew, f.fainelli, hkallweit1, davem, robh
  Cc: netdev, linux-kernel, devicetree, Dan Murphy
In-Reply-To: <20200521174834.3234-1-dmurphy@ti.com>

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

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

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

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 7481135d27ab..40f53b379d2b 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -2661,6 +2661,51 @@ void phy_get_pause(struct phy_device *phydev, bool *tx_pause, bool *rx_pause)
 }
 EXPORT_SYMBOL(phy_get_pause);
 
+/**
+ * phy_get_delay_index - returns the index of the internal delay
+ * @phydev: phy_device struct
+ * @delay_values: array of delays the PHY supports
+ * @size: the size of the delay array
+ * @delay: the delay to be looked up
+ *
+ * Returns the index within the array of internal delay passed in.
+ */
+int phy_get_delay_index(struct phy_device *phydev, int *delay_values, int size,
+			int delay)
+{
+	int i;
+
+	if (size <= 0)
+		return -EINVAL;
+
+	if (delay <= delay_values[0])
+		return 0;
+
+	if (delay > delay_values[size - 1])
+		return size - 1;
+
+	for (i = 0; i < size; i++) {
+		if (delay == delay_values[i])
+			return i;
+
+		/* Find an approximate index by looking up the table */
+		if (delay > delay_values[i - 1] &&
+		    delay < delay_values[i]) {
+			if (delay - delay_values[i - 1] < delay_values[i] - delay)
+				return i - 1;
+			else
+				return i;
+		}
+
+	}
+
+	phydev_err(phydev, "error finding internal delay index for %d\n",
+		   delay);
+
+	return -EINVAL;
+}
+EXPORT_SYMBOL(phy_get_delay_index);
+
 static bool phy_drv_supports_irq(struct phy_driver *phydrv)
 {
 	return phydrv->config_intr && phydrv->ack_interrupt;
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 467aa8bf9f64..78b7b77f9a67 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1406,6 +1406,8 @@ void phy_set_asym_pause(struct phy_device *phydev, bool rx, bool tx);
 bool phy_validate_pause(struct phy_device *phydev,
 			struct ethtool_pauseparam *pp);
 void phy_get_pause(struct phy_device *phydev, bool *tx_pause, bool *rx_pause);
+int phy_get_delay_index(struct phy_device *phydev, int *delay_values,
+			int size, int delay);
 void phy_resolve_pause(unsigned long *local_adv, unsigned long *partner_adv,
 		       bool *tx_pause, bool *rx_pause);
 
-- 
2.26.2


^ permalink raw reply related

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

Add RGMII internal delay configuration for Rx and Tx.

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

diff --git a/drivers/net/phy/dp83869.c b/drivers/net/phy/dp83869.c
index cfb22a21a2e6..40c34fefffe4 100644
--- a/drivers/net/phy/dp83869.c
+++ b/drivers/net/phy/dp83869.c
@@ -99,6 +99,14 @@
 #define DP83869_OP_MODE_MII			BIT(5)
 #define DP83869_SGMII_RGMII_BRIDGE		BIT(6)
 
+/* RGMIIDCTL bits */
+#define DP83869_RGMII_TX_CLK_DELAY_SHIFT	4
+#define DP83869_RGMII_CLK_DELAY_INV		0
+
+static int dp83869_internal_delay[] = {250, 500, 750, 1000, 1250, 1500, 1750,
+				       2000, 2250, 2500, 2750, 3000, 3250,
+				       3500, 3750, 4000};
+
 enum {
 	DP83869_PORT_MIRRORING_KEEP,
 	DP83869_PORT_MIRRORING_EN,
@@ -108,6 +116,8 @@ enum {
 struct dp83869_private {
 	int tx_fifo_depth;
 	int rx_fifo_depth;
+	u32 rx_id_delay;
+	u32 tx_id_delay;
 	int io_impedance;
 	int port_mirroring;
 	bool rxctrl_strap_quirk;
@@ -182,6 +192,7 @@ static int dp83869_of_init(struct phy_device *phydev)
 	struct dp83869_private *dp83869 = phydev->priv;
 	struct device *dev = &phydev->mdio.dev;
 	struct device_node *of_node = dev->of_node;
+	int delay_size = ARRAY_SIZE(dp83869_internal_delay);
 	int ret;
 
 	if (!of_node)
@@ -232,6 +243,26 @@ static int dp83869_of_init(struct phy_device *phydev)
 				 &dp83869->tx_fifo_depth))
 		dp83869->tx_fifo_depth = DP83869_PHYCR_FIFO_DEPTH_4_B_NIB;
 
+	dp83869->rx_id_delay = DP83869_RGMII_CLK_DELAY_INV;
+	ret = of_property_read_u32(of_node, "rx-internal-delay",
+				   &dp83869->rx_id_delay);
+	if (!ret && dp83869->rx_id_delay > dp83869_internal_delay[delay_size]) {
+		phydev_err(phydev,
+			   "rx-internal-delay value of %u out of range\n",
+			   dp83869->rx_id_delay);
+		return -EINVAL;
+	}
+
+	dp83869->tx_id_delay = DP83869_RGMII_CLK_DELAY_INV;
+	ret = of_property_read_u32(of_node, "tx-internal-delay",
+				   &dp83869->tx_id_delay);
+	if (!ret && dp83869->tx_id_delay > dp83869_internal_delay[delay_size]) {
+		phydev_err(phydev,
+			   "tx-internal-delay value of %u out of range\n",
+			   dp83869->tx_id_delay);
+		return -EINVAL;
+	}
+
 	return ret;
 }
 #else
@@ -270,6 +301,29 @@ static int dp83869_configure_rgmii(struct phy_device *phydev,
 	return ret;
 }
 
+static int dp83869_verify_rgmii_cfg(struct phy_device *phydev)
+{
+	struct dp83869_private *dp83869 = phydev->priv;
+
+	/* RX delay *must* be specified if internal delay of RX is used. */
+	if ((phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
+	     phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID) &&
+	     dp83869->rx_id_delay == DP83869_RGMII_CLK_DELAY_INV) {
+		phydev_err(phydev, "ti,rx-internal-delay must be specified\n");
+		return -EINVAL;
+	}
+
+	/* TX delay *must* be specified if internal delay of TX is used. */
+	if ((phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
+	     phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID) &&
+	     dp83869->tx_id_delay == DP83869_RGMII_CLK_DELAY_INV) {
+		phydev_err(phydev, "ti,tx-internal-delay must be specified\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int dp83869_configure_mode(struct phy_device *phydev,
 				  struct dp83869_private *dp83869)
 {
@@ -371,6 +425,12 @@ static int dp83869_config_init(struct phy_device *phydev)
 {
 	struct dp83869_private *dp83869 = phydev->priv;
 	int ret, val;
+	int delay_size = ARRAY_SIZE(dp83869_internal_delay);
+	int delay = 0;
+
+	ret = dp83869_verify_rgmii_cfg(phydev);
+	if (ret)
+		return ret;
 
 	ret = dp83869_configure_mode(phydev, dp83869);
 	if (ret)
@@ -394,6 +454,47 @@ static int dp83869_config_init(struct phy_device *phydev)
 				     dp83869->clk_output_sel <<
 				     DP83869_IO_MUX_CFG_CLK_O_SEL_SHIFT);
 
+	if (phy_interface_is_rgmii(phydev)) {
+		val = phy_read_mmd(phydev, DP83869_DEVADDR, DP83869_RGMIICTL);
+
+		val &= ~(DP83869_RGMII_TX_CLK_DELAY_EN | DP83869_RGMII_RX_CLK_DELAY_EN);
+		if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
+			val |= (DP83869_RGMII_TX_CLK_DELAY_EN | DP83869_RGMII_RX_CLK_DELAY_EN);
+
+		if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID)
+			val |= DP83869_RGMII_TX_CLK_DELAY_EN;
+
+		if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID)
+			val |= DP83869_RGMII_RX_CLK_DELAY_EN;
+
+		phy_write_mmd(phydev, DP83869_DEVADDR, DP83869_RGMIICTL, val);
+
+		if (dp83869->rx_id_delay) {
+			val = phy_get_delay_index(phydev,
+						  &dp83869_internal_delay[0],
+						  delay_size,
+						  dp83869->rx_id_delay);
+			if (val < 0)
+				return val;
+
+			delay |= val;
+		}
+
+		if (dp83869->tx_id_delay) {
+			val = phy_get_delay_index(phydev,
+						  &dp83869_internal_delay[0],
+						  delay_size,
+						  dp83869->tx_id_delay);
+			if (val < 0)
+				return val;
+
+			delay |= val << DP83869_RGMII_TX_CLK_DELAY_SHIFT;
+		}
+
+		phy_write_mmd(phydev, DP83869_DEVADDR, DP83869_RGMIIDCTL,
+			      delay);
+	}
+
 	return ret;
 }
 
-- 
2.26.2


^ permalink raw reply related

* [PATCH net-next v3 1/2] net: phy: dp83869: Update port-mirroring to read straps
From: Dan Murphy @ 2020-05-21 17:47 UTC (permalink / raw)
  To: andrew, f.fainelli, hkallweit1, davem
  Cc: netdev, linux-kernel, devicetree, Dan Murphy
In-Reply-To: <20200521174738.3151-1-dmurphy@ti.com>

The device tree may not have the property set for port mirroring
because the hardware may have it strapped. If the property is not in the
DT then check the straps and set the port mirroring bit appropriately.

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
 drivers/net/phy/dp83869.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/net/phy/dp83869.c b/drivers/net/phy/dp83869.c
index 7996a4aea8d2..073a0f7754a5 100644
--- a/drivers/net/phy/dp83869.c
+++ b/drivers/net/phy/dp83869.c
@@ -66,6 +66,7 @@
 
 /* STRAP_STS1 bits */
 #define DP83869_STRAP_STS1_RESERVED		BIT(11)
+#define DP83869_STRAP_MIRROR_ENABLED           BIT(12)
 
 /* PHYCTRL bits */
 #define DP83869_RX_FIFO_SHIFT	12
@@ -191,10 +192,18 @@ static int dp83869_of_init(struct phy_device *phydev)
 	else if (of_property_read_bool(of_node, "ti,min-output-impedance"))
 		dp83869->io_impedance = DP83869_IO_MUX_CFG_IO_IMPEDANCE_MIN;
 
-	if (of_property_read_bool(of_node, "enet-phy-lane-swap"))
+	if (of_property_read_bool(of_node, "enet-phy-lane-swap")) {
 		dp83869->port_mirroring = DP83869_PORT_MIRRORING_EN;
-	else
-		dp83869->port_mirroring = DP83869_PORT_MIRRORING_DIS;
+	} else {
+		/* If the lane swap is not in the DT then check the straps */
+		ret = phy_read_mmd(phydev, DP83869_DEVADDR, DP83869_STRAP_STS1);
+		if (ret < 0)
+			return ret;
+		if (ret & DP83869_STRAP_MIRROR_ENABLED)
+			dp83869->port_mirroring = DP83869_PORT_MIRRORING_EN;
+		else
+			dp83869->port_mirroring = DP83869_PORT_MIRRORING_DIS;
+	}
 
 	if (of_property_read_u32(of_node, "rx-fifo-depth",
 				 &dp83869->rx_fifo_depth))
-- 
2.26.2


^ permalink raw reply related

* [PATCH net-next v3 2/2] net: phy: dp83869: Set opmode from straps
From: Dan Murphy @ 2020-05-21 17:47 UTC (permalink / raw)
  To: andrew, f.fainelli, hkallweit1, davem
  Cc: netdev, linux-kernel, devicetree, Dan Murphy
In-Reply-To: <20200521174738.3151-1-dmurphy@ti.com>

If the op-mode for the device is not set in the device tree then set
the strapped op-mode and store it for later configuration.

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
 drivers/net/phy/dp83869.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/dp83869.c b/drivers/net/phy/dp83869.c
index 073a0f7754a5..cfb22a21a2e6 100644
--- a/drivers/net/phy/dp83869.c
+++ b/drivers/net/phy/dp83869.c
@@ -65,6 +65,7 @@
 #define DP83869_RGMII_RX_CLK_DELAY_EN		BIT(0)
 
 /* STRAP_STS1 bits */
+#define DP83869_STRAP_OP_MODE_MASK		GENMASK(2, 0)
 #define DP83869_STRAP_STS1_RESERVED		BIT(11)
 #define DP83869_STRAP_MIRROR_ENABLED           BIT(12)
 
@@ -161,6 +162,20 @@ static int dp83869_config_port_mirroring(struct phy_device *phydev)
 					  DP83869_CFG3_PORT_MIRROR_EN);
 }
 
+static int dp83869_set_strapped_mode(struct phy_device *phydev)
+{
+	struct dp83869_private *dp83869 = phydev->priv;
+	int val;
+
+	val = phy_read_mmd(phydev, DP83869_DEVADDR, DP83869_STRAP_STS1);
+	if (val < 0)
+		return val;
+
+	dp83869->mode = val & DP83869_STRAP_OP_MODE_MASK;
+
+	return 0;
+}
+
 #ifdef CONFIG_OF_MDIO
 static int dp83869_of_init(struct phy_device *phydev)
 {
@@ -185,6 +200,10 @@ static int dp83869_of_init(struct phy_device *phydev)
 		if (dp83869->mode < DP83869_RGMII_COPPER_ETHERNET ||
 		    dp83869->mode > DP83869_SGMII_COPPER_ETHERNET)
 			return -EINVAL;
+	} else {
+		ret = dp83869_set_strapped_mode(phydev);
+		if (ret)
+			return ret;
 	}
 
 	if (of_property_read_bool(of_node, "ti,max-output-impedance"))
@@ -218,7 +237,7 @@ static int dp83869_of_init(struct phy_device *phydev)
 #else
 static int dp83869_of_init(struct phy_device *phydev)
 {
-	return 0;
+	return dp83869_set_strapped_mode(phydev);
 }
 #endif /* CONFIG_OF_MDIO */
 
-- 
2.26.2


^ permalink raw reply related

* [PATCH net-next v3 0/2] DP83869 Enhancements
From: Dan Murphy @ 2020-05-21 17:47 UTC (permalink / raw)
  To: andrew, f.fainelli, hkallweit1, davem
  Cc: netdev, linux-kernel, devicetree, Dan Murphy

Hello

These are improvements to the DP83869 Ethernet PHY driver.  OP-mode and port
mirroring may be strapped on the device but the software only retrives these
settings from the device tree.  Reading the straps and initializing the
associated stored variables so when setting the PHY up and down the PHY's
configuration values will be retained.

Dan Murphy (2):
  net: phy: dp83869: Update port-mirroring to read straps
  net: phy: dp83869: Set opmode from straps

 drivers/net/phy/dp83869.c | 36 ++++++++++++++++++++++++++++++++----
 1 file changed, 32 insertions(+), 4 deletions(-)

-- 
2.26.2


^ permalink raw reply

* Re: [PATCH v3 00/16] spi: dw: Add generic DW DMA controller support
From: Andy Shevchenko @ 2020-05-21 17:46 UTC (permalink / raw)
  To: Serge Semin
  Cc: Mark Brown, Serge Semin, Georgy Vlasov, Ramil Zaripov,
	Alexey Malahov, Maxim Kaurkin, Pavel Parkhomenko,
	Ekaterina Skachko, Vadim Vlasov, Alexey Kolotnikov,
	Thomas Bogendoerfer, Paul Burton, Ralf Baechle, Arnd Bergmann,
	Rob Herring, linux-mips, linux-spi, devicetree, linux-kernel
In-Reply-To: <20200521012206.14472-1-Sergey.Semin@baikalelectronics.ru>

On Thu, May 21, 2020 at 04:21:50AM +0300, Serge Semin wrote:
> Baikal-T1 SoC provides a DW DMA controller to perform low-speed peripherals
> Mem-to-Dev and Dev-to-Mem transaction. This is also applicable to the DW
> APB SSI devices embedded into the SoC. Currently the DMA-based transfers
> are supported by the DW APB SPI driver only as a middle layer code for
> Intel MID/Elkhart PCI devices. Seeing the same code can be used for normal
> platform DMAC device we introduced a set of patches to fix it within this
> series.
> 
> First of all we need to add the Tx and Rx DMA channels support into the DW
> APB SSI binding. Then there are several fixes and cleanups provided as a
> initial preparation for the Generic DMA support integration: add Tx/Rx
> finish wait methods, clear DMAC register when done or stopped, Fix native
> CS being unset, enable interrupts in accordance with DMA xfer mode,
> discard static DW DMA slave structures, discard unused void priv pointer
> and dma_width member of the dw_spi structure, provide the DMA Tx/Rx burst
> length parametrisation and make sure it's optionally set in accordance
> with the DMA max-burst capability.
> 
> In order to have the DW APB SSI MMIO driver working with DMA we need to
> initialize the paddr field with the physical base address of the DW APB SSI
> registers space. Then we unpin the Intel MID specific code from the
> generic DMA one and placed it into the spi-dw-pci.c driver, which is a
> better place for it anyway. After that the naming cleanups are performed
> since the code is going to be used for a generic DMAC device. Finally the
> Generic DMA initialization can be added to the generic version of the
> DW APB SSI IP.
> 
> Last but not least we traditionally convert the legacy plain text-based
> dt-binding file with yaml-based one and as a cherry on a cake replace
> the manually written DebugFS registers read method with a ready-to-use
> for the same purpose regset32 DebugFS interface usage.

I have given tags where I agree with content and I left disputed ones untouched
(1st patch, nevertheless, is fine for me, but I'm waiting for Feng).

So, now it seems settled in a way that I will send couple of cleanups
afterwards to avoid blocking this series to go.

Thanks for your work!

> This patchset is rebased and tested on the spi/for-next (5.7-rc5):
> base-commit: fe9fce6b2cf3 ("Merge remote-tracking branch 'spi/for-5.8' into spi-next")
> 
> Link: https://lore.kernel.org/linux-spi/20200508132943.9826-1-Sergey.Semin@baikalelectronics.ru/
> Changelog v2:
> - Rebase on top of the spi repository for-next branch.
> - Move bindings conversion patch to the tail of the series.
> - Move fixes to the head of the series.
> - Apply as many changes as possible to be applied the Generic DMA
>   functionality support is added and the spi-dw-mid is moved to the
>   spi-dw-dma driver.
> - Discard patch "spi: dw: Fix dma_slave_config used partly uninitialized"
>   since the problem has already been fixed.
> - Add new patch "spi: dw: Discard unused void priv pointer".
> - Add new patch "spi: dw: Discard dma_width member of the dw_spi structure".
>   n_bytes member of the DW SPI data can be used instead.
> - Build the DMA functionality into the DW APB SSI core if required instead
>   of creating a separate kernel module.
> - Use conditional statement instead of the ternary operator in the ref
>   clock getter.
> 
> Link: https://lore.kernel.org/linux-spi/20200515104758.6934-1-Sergey.Semin@baikalelectronics.ru/
> Changelog v3:
> - Use spi_delay_exec() method to wait for the DMA operation completion.
> - Explicitly initialize the dw_dma_slave members on stack.
> - Discard the dws->fifo_len utilization in the Tx FIFO DMA threshold
>   setting from the patch where we just add the default burst length
>   constants.
> - Use min() method to calculate the optimal burst values.
> - Add new patch which moves the spi-dw.c source file to spi-dw-core.c in
>   order to preserve the DW APB SSI core driver name.
> - Add commas in the debugfs_reg32 structure initializer and after the last
>   entry of the dw_spi_dbgfs_regs array.
> 
> Co-developed-by: Georgy Vlasov <Georgy.Vlasov@baikalelectronics.ru>
> Signed-off-by: Georgy Vlasov <Georgy.Vlasov@baikalelectronics.ru>
> Co-developed-by: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
> Signed-off-by: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Maxim Kaurkin <Maxim.Kaurkin@baikalelectronics.ru>
> Cc: Pavel Parkhomenko <Pavel.Parkhomenko@baikalelectronics.ru>
> Cc: Ekaterina Skachko <Ekaterina.Skachko@baikalelectronics.ru>
> Cc: Vadim Vlasov <V.Vlasov@baikalelectronics.ru>
> Cc: Alexey Kolotnikov <Alexey.Kolotnikov@baikalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Paul Burton <paulburton@kernel.org>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: linux-mips@vger.kernel.org
> Cc: linux-spi@vger.kernel.org
> Cc: devicetree@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> 
> Serge Semin (16):
>   spi: dw: Add Tx/Rx finish wait methods to the MID DMA
>   spi: dw: Enable interrupts in accordance with DMA xfer mode
>   spi: dw: Discard static DW DMA slave structures
>   spi: dw: Discard unused void priv pointer
>   spi: dw: Discard dma_width member of the dw_spi structure
>   spi: dw: Parameterize the DMA Rx/Tx burst length
>   spi: dw: Use DMA max burst to set the request thresholds
>   spi: dw: Fix Rx-only DMA transfers
>   spi: dw: Add core suffix to the DW APB SSI core source file
>   spi: dw: Move Non-DMA code to the DW PCIe-SPI driver
>   spi: dw: Remove DW DMA code dependency from DW_DMAC_PCI
>   spi: dw: Add DW SPI DMA/PCI/MMIO dependency on the DW SPI core
>   spi: dw: Cleanup generic DW DMA code namings
>   spi: dw: Add DMA support to the DW SPI MMIO driver
>   spi: dw: Use regset32 DebugFS method to create regdump file
>   dt-bindings: spi: Convert DW SPI binding to DT schema
> 
>  .../bindings/spi/snps,dw-apb-ssi.txt          |  44 ---
>  .../bindings/spi/snps,dw-apb-ssi.yaml         | 127 ++++++++
>  .../devicetree/bindings/spi/spi-dw.txt        |  24 --
>  drivers/spi/Kconfig                           |  15 +-
>  drivers/spi/Makefile                          |   5 +-
>  drivers/spi/{spi-dw.c => spi-dw-core.c}       |  88 ++----
>  drivers/spi/{spi-dw-mid.c => spi-dw-dma.c}    | 276 +++++++++++-------
>  drivers/spi/spi-dw-mmio.c                     |   4 +
>  drivers/spi/spi-dw-pci.c                      |  50 +++-
>  drivers/spi/spi-dw.h                          |  33 ++-
>  10 files changed, 407 insertions(+), 259 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
>  create mode 100644 Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
>  delete mode 100644 Documentation/devicetree/bindings/spi/spi-dw.txt
>  rename drivers/spi/{spi-dw.c => spi-dw-core.c} (82%)
>  rename drivers/spi/{spi-dw-mid.c => spi-dw-dma.c} (53%)
> 
> -- 
> 2.25.1
> 

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply

* Re: [PATCH v2 0/3] Re-introduce TX FIFO resize for larger EP bursting
From: Wesley Cheng @ 2020-05-21 17:37 UTC (permalink / raw)
  To: agross, bjorn.andersson, robh+dt, balbi, gregkh
  Cc: linux-arm-msm, devicetree, linux-kernel, linux-usb, jackp
In-Reply-To: <1590050169-30747-1-git-send-email-wcheng@codeaurora.org>



On 5/21/2020 1:36 AM, Wesley Cheng wrote:
> Changes in V2:
>  - Modified TXFIFO resizing logic to ensure that each EP is reserved a
>    FIFO.
>  - Removed dev_dbg() prints and fixed typos from patches
>  - Added some more description on the dt-bindings commit message
>


> Reviewed-by: Felipe Balbi <balbi@kernel.org>
> Reviewed-by: Rob Herring <robh@kernel.org>
> 

Sorry, please disregard the Reviewed-by tags in the patches.  I added
those mistakenly.

> Currently, there is no functionality to allow for resizing the TXFIFOs, and
> relying on the HW default setting for the TXFIFO depth.  In most cases, the
> HW default is probably sufficient, but for USB compositions that contain
> multiple functions that require EP bursting, the default settings
> might not be enough.  Also to note, the current SW will assign an EP to a
> function driver w/o checking to see if the TXFIFO size for that particular
> EP is large enough. (this is a problem if there are multiple HW defined
> values for the TXFIFO size)
> 
> It is mentioned in the SNPS databook that a minimum of TX FIFO depth = 3
> is required for an EP that supports bursting.  Otherwise, there may be
> frequent occurences of bursts ending.  For high bandwidth functions,
> such as data tethering (protocols that support data aggregation), mass
> storage, and media transfer protocol (over FFS), the bMaxBurst value can be
> large, and a bigger TXFIFO depth may prove to be beneficial in terms of USB
> throughput. (which can be associated to system access latency, etc...)  It
> allows for a more consistent burst of traffic, w/o any interruptions, as
> data is readily available in the FIFO.
> 
> With testing done using the mass storage function driver, the results show
> that with a larger TXFIFO depth, the bandwidth increased significantly.
> 
> Test Parameters:
>  - Platform: Qualcomm SM8150
>  - bMaxBurst = 6
>  - USB req size = 256kB
>  - Num of USB reqs = 16
>  - USB Speed = Super-Speed
>  - Function Driver: Mass Storage (w/ ramdisk)
>  - Test Application: CrystalDiskMark
> 
> Results:
> 
> TXFIFO Depth = 3 max packets
> 
> Test Case | Data Size | AVG tput (in MB/s)
> -------------------------------------------
> Sequential|1 GB x     | 
> Read      |9 loops    | 193.60
> 	  |           | 195.86
>           |           | 184.77
>           |           | 193.60
> -------------------------------------------
> 
> TXFIFO Depth = 6 max packets
> 
> Test Case | Data Size | AVG tput (in MB/s)
> -------------------------------------------
> Sequential|1 GB x     | 
> Read      |9 loops    | 287.35
> 	  |           | 304.94
>           |           | 289.64
>           |           | 293.61
> -------------------------------------------
> 
> Wesley Cheng (3):
>   usb: dwc3: Resize TX FIFOs to meet EP bursting requirements
>   arm64: boot: dts: qcom: sm8150: Enable dynamic TX FIFO resize logic
>   dt-bindings: usb: dwc3: Add entry for tx-fifo-resize
> 
>  Documentation/devicetree/bindings/usb/dwc3.txt |   2 +-
>  arch/arm64/boot/dts/qcom/sm8150.dtsi           |   1 +
>  drivers/usb/dwc3/core.c                        |   2 +
>  drivers/usb/dwc3/core.h                        |   8 ++
>  drivers/usb/dwc3/ep0.c                         |  37 ++++++++-
>  drivers/usb/dwc3/gadget.c                      | 111 +++++++++++++++++++++++++
>  6 files changed, 159 insertions(+), 2 deletions(-)
> 

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply

* Re: [PATCH v3 03/16] spi: dw: Discard static DW DMA slave structures
From: Serge Semin @ 2020-05-21 17:29 UTC (permalink / raw)
  To: Mark Brown
  Cc: Serge Semin, Andy Shevchenko, Georgy Vlasov, Ramil Zaripov,
	Alexey Malahov, Thomas Bogendoerfer, Paul Burton, Ralf Baechle,
	Andy Shevchenko, Arnd Bergmann, Rob Herring, linux-mips,
	devicetree, Thomas Gleixner, Wan Ahmad Zainie, Jarkko Nikula,
	Clement Leger, linux-spi, Linux Kernel Mailing List
In-Reply-To: <20200521163904.GF4770@sirena.org.uk>

On Thu, May 21, 2020 at 05:39:04PM +0100, Mark Brown wrote:
> On Thu, May 21, 2020 at 07:02:32PM +0300, Andy Shevchenko wrote:
> > On Thu, May 21, 2020 at 6:58 PM Serge Semin
> 
> > > Let's leave the patch as is.
> 
> > Mark, should I send a partial revert afterwards in this case?
> > I'm not fully satisfied with it.
> 
> That might be a suitable way to keep the peace here.  You are clearly
> both much more passionate about this choice than I am.

Fine with me.)

-Sergey

^ permalink raw reply

* Re: [PATCH 1/2] spi: dw: add reset control
From: Serge Semin @ 2020-05-21 17:28 UTC (permalink / raw)
  To: Mark Brown
  Cc: Serge Semin, Dinh Nguyen, linux-kernel, devicetree, robh+dt,
	linux-spi, Liang Jin J, Andy Shevchenko, Lars Povlsen
In-Reply-To: <20200521171203.GH4770@sirena.org.uk>

On Thu, May 21, 2020 at 06:12:03PM +0100, Mark Brown wrote:
> On Thu, May 21, 2020 at 12:03:58PM -0500, Dinh Nguyen wrote:
> > Add mechanism to get the reset control and deassert it in order to bring
> > the IP out of reset.
> > 
> > Signed-off-by: Liang Jin J <liang.j.jin@ericsson.com>
> > Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
> 
> Is the From: correct here?
> 
> Also adding everyone else who's got patches in flight for this driver
> right now.
> 
> > ---
> >  drivers/spi/spi-dw-mmio.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c
> > index 384a3ab6dc2d..5c813e15ed89 100644
> > --- a/drivers/spi/spi-dw-mmio.c
> > +++ b/drivers/spi/spi-dw-mmio.c
> > @@ -20,6 +20,7 @@
> >  #include <linux/acpi.h>
> >  #include <linux/property.h>
> >  #include <linux/regmap.h>
> > +#include <linux/reset.h>
> >  
> >  #include "spi-dw.h"
> >  
> > @@ -30,6 +31,7 @@ struct dw_spi_mmio {
> >  	struct clk     *clk;
> >  	struct clk     *pclk;
> >  	void           *priv;
> > +	struct reset_control	*rstc;
> >  };
> >  
> >  #define MSCC_CPU_SYSTEM_CTRL_GENERAL_CTRL	0x24
> > @@ -145,6 +147,10 @@ static int dw_spi_mmio_probe(struct platform_device *pdev)
> >  	if (!dwsmmio)
> >  		return -ENOMEM;
> >  

> > +	dwsmmio->rstc = devm_reset_control_get_exclusive(&pdev->dev, "spi");

It would be better to use the optional-version of the reset getter:
devm_reset_control_get_optional_exclusive(), since for instance our DW APB SSI
doesn't have a dedicated reset line.

Note also that you don't need to check the rstc for being NULL. It's null if the
reset line is optional and isn't provided.

If we get to have this patch applied, then DT binding shall be also updated...
Though we haven't got it reviewed yet.(

-Sergey



> > +	if (!IS_ERR(dwsmmio->rstc))
> > +		reset_control_deassert(dwsmmio->rstc);
> > +
> >  	dws = &dwsmmio->dws;
> >  
> >  	/* Get basic io resource and map it */
> 
> Should we also undo the reset in error paths from probe()?  It's not the
> end of the world if we don't but...
> 
> > @@ -220,6 +226,9 @@ static int dw_spi_mmio_remove(struct platform_device *pdev)
> >  	clk_disable_unprepare(dwsmmio->pclk);
> >  	clk_disable_unprepare(dwsmmio->clk);
> >  
> > +	if (dwsmmio->rstc)
> > +		reset_control_assert(dwsmmio->rstc);
> > +
> >  	return 0;
> >  }
> >  
> > -- 
> > 2.17.1
> > 



^ permalink raw reply

* Re: [PATCH v2] arm: dts: am33xx-bone-common: add gpio-line-names
From: Grygorii Strashko @ 2020-05-21 17:26 UTC (permalink / raw)
  To: Robert Nelson
  Cc: Drew Fustini, Linus Walleij, Benoît Cousson, Tony Lindgren,
	Rob Herring, Linux-OMAP, devicetree, linux kernel, Jason Kridner
In-Reply-To: <CAOCHtYiw2jJuzbnW02FUmPy-xmmtErMmow46QQJUMs0VtX=cKg@mail.gmail.com>



On 21/05/2020 17:41, Robert Nelson wrote:
>> Not sure if it should be in am335x-bone-common.dtsi.
>>
>> For example:
>> am335x-boneblack.dts
>>    #include "am335x-bone-common.dtsi"
>>    #include "am335x-boneblack-common.dtsi" <-- hdmi defined only here
> 
> Ah crap, yeah that's a good point.. So if we stick it in...
> am335x-boneblack-common.dtsi
> 
> Then the Black-Wireless now has Ethernet...
> 
> am335x-boneblack-wireless.dts
> #include "am335x-bone-common.dtsi"
> #include "am335x-boneblack-common.dtsi"
> 
> It's going to be ugly, copy and paste mess, but i guess we might as
> well stick it in the device " am335x-boneblack.dts"?

Seems like.

Unfortunately, there is no way to partially overwrite "gpio-line-names"
property.

-- 
Best regards,
grygorii

^ permalink raw reply

* Re: [PATCH 2/2] dt-bindings: snps,dw-apb-ssi: add optional reset property
From: Mark Brown @ 2020-05-21 17:13 UTC (permalink / raw)
  To: Dinh Nguyen; +Cc: linux-kernel, devicetree, robh+dt, linux-spi
In-Reply-To: <20200521170359.20430-2-dinguyen@kernel.org>

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

On Thu, May 21, 2020 at 12:03:59PM -0500, Dinh Nguyen wrote:
> Add optional reset property.
> 
> Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
> ---
>  Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
> index 3ed08ee9feba..e01c557a9876 100644
> --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
> +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
> @@ -37,5 +37,7 @@ Example:
>  		num-cs = <2>;
>  		cs-gpios = <&gpio0 13 0>,
>  			   <&gpio0 14 0>;
> +		resets = <&rst SPIM0_RST>;
> +		reset-names = "spi";

This only adds the example, it doesn't add anything to the actual
binding documentation.

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

^ permalink raw reply

* Re: [PATCH 1/2] spi: dw: add reset control
From: Mark Brown @ 2020-05-21 17:12 UTC (permalink / raw)
  To: Dinh Nguyen
  Cc: linux-kernel, devicetree, robh+dt, linux-spi, Liang Jin J,
	Serge Semin, fancer.lancer, Andy Shevchenko, Lars Povlsen
In-Reply-To: <20200521170359.20430-1-dinguyen@kernel.org>

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

On Thu, May 21, 2020 at 12:03:58PM -0500, Dinh Nguyen wrote:
> Add mechanism to get the reset control and deassert it in order to bring
> the IP out of reset.
> 
> Signed-off-by: Liang Jin J <liang.j.jin@ericsson.com>
> Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>

Is the From: correct here?

Also adding everyone else who's got patches in flight for this driver
right now.

> ---
>  drivers/spi/spi-dw-mmio.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c
> index 384a3ab6dc2d..5c813e15ed89 100644
> --- a/drivers/spi/spi-dw-mmio.c
> +++ b/drivers/spi/spi-dw-mmio.c
> @@ -20,6 +20,7 @@
>  #include <linux/acpi.h>
>  #include <linux/property.h>
>  #include <linux/regmap.h>
> +#include <linux/reset.h>
>  
>  #include "spi-dw.h"
>  
> @@ -30,6 +31,7 @@ struct dw_spi_mmio {
>  	struct clk     *clk;
>  	struct clk     *pclk;
>  	void           *priv;
> +	struct reset_control	*rstc;
>  };
>  
>  #define MSCC_CPU_SYSTEM_CTRL_GENERAL_CTRL	0x24
> @@ -145,6 +147,10 @@ static int dw_spi_mmio_probe(struct platform_device *pdev)
>  	if (!dwsmmio)
>  		return -ENOMEM;
>  
> +	dwsmmio->rstc = devm_reset_control_get_exclusive(&pdev->dev, "spi");
> +	if (!IS_ERR(dwsmmio->rstc))
> +		reset_control_deassert(dwsmmio->rstc);
> +
>  	dws = &dwsmmio->dws;
>  
>  	/* Get basic io resource and map it */

Should we also undo the reset in error paths from probe()?  It's not the
end of the world if we don't but...

> @@ -220,6 +226,9 @@ static int dw_spi_mmio_remove(struct platform_device *pdev)
>  	clk_disable_unprepare(dwsmmio->pclk);
>  	clk_disable_unprepare(dwsmmio->clk);
>  
> +	if (dwsmmio->rstc)
> +		reset_control_assert(dwsmmio->rstc);
> +
>  	return 0;
>  }
>  
> -- 
> 2.17.1
> 

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

^ permalink raw reply

* Re: [RFC PATCH V4 1/4] media: v4l2-mem2mem: add v4l2_m2m_suspend, v4l2_m2m_resume
From: Tomasz Figa @ 2020-05-21 17:11 UTC (permalink / raw)
  To: Jerry-ch Chen, Hans Verkuil
  Cc: Hans Verkuil, laurent.pinchart+renesas, matthias.bgg, mchehab,
	pihsun, yuzhao, zwisler, linux-mediatek, linux-arm-kernel,
	Sean.Cheng, sj.huang, christie.yu, frederic.chen, jungo.lin,
	Rynn.Wu, linux-media, srv_heupstream, devicetree, Jerry-ch Chen
In-Reply-To: <20191204124732.10932-2-Jerry-Ch.chen@mediatek.com>

Hi Jerry,

On Wed, Dec 04, 2019 at 08:47:29PM +0800, Jerry-ch Chen wrote:
> From: Pi-Hsun Shih <pihsun@chromium.org>
> 
> Add two functions that can be used to stop new jobs from being queued /
> continue running queued job. This can be used while a driver using m2m
> helper is going to suspend / wake up from resume, and can ensure that
> there's no job running in suspend process.
> 
> BUG=b:143046833
> TEST=build
> 
> Signed-off-by: Pi-Hsun Shih <pihsun@chromium.org>
> Signed-off-by: Jerry-ch Chen <jerry-ch.chen@mediatek.corp-partner.google.com>
> ---
>  drivers/media/v4l2-core/v4l2-mem2mem.c | 40 ++++++++++++++++++++++++++
>  include/media/v4l2-mem2mem.h           | 22 ++++++++++++++
>  2 files changed, 62 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c b/drivers/media/v4l2-core/v4l2-mem2mem.c
> index 5bbdec55b7d7..76ba203e0035 100644
> --- a/drivers/media/v4l2-core/v4l2-mem2mem.c
> +++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
> @@ -47,6 +47,10 @@ module_param(debug, bool, 0644);
>  #define TRANS_ABORT		(1 << 2)
>  
>  
> +/* The job queue is not running new jobs */
> +#define QUEUE_PAUSED		(1 << 0)
> +
> +
>  /* Offset base for buffers on the destination queue - used to distinguish
>   * between source and destination buffers when mmapping - they receive the same
>   * offsets but for different queues */
> @@ -88,6 +92,7 @@ static const char * const m2m_entity_name[] = {
>   * @job_queue:		instances queued to run
>   * @job_spinlock:	protects job_queue
>   * @job_work:		worker to run queued jobs.
> + * @job_queue_flags:	flags of the queue status, %QUEUE_PAUSED.
>   * @m2m_ops:		driver callbacks
>   */
>  struct v4l2_m2m_dev {
> @@ -105,6 +110,7 @@ struct v4l2_m2m_dev {
>  	struct list_head	job_queue;
>  	spinlock_t		job_spinlock;
>  	struct work_struct	job_work;
> +	unsigned long		job_queue_flags;
>  
>  	const struct v4l2_m2m_ops *m2m_ops;
>  };
> @@ -267,6 +273,12 @@ static void v4l2_m2m_try_run(struct v4l2_m2m_dev *m2m_dev)
>  		return;
>  	}
>  
> +	if (m2m_dev->job_queue_flags & QUEUE_PAUSED) {
> +		spin_unlock_irqrestore(&m2m_dev->job_spinlock, flags);
> +		dprintk("Running new jobs is paused\n");
> +		return;
> +	}
> +
>  	m2m_dev->curr_ctx = list_first_entry(&m2m_dev->job_queue,
>  				   struct v4l2_m2m_ctx, queue);
>  	m2m_dev->curr_ctx->job_flags |= TRANS_RUNNING;
> @@ -447,6 +459,34 @@ void v4l2_m2m_job_finish(struct v4l2_m2m_dev *m2m_dev,
>  }
>  EXPORT_SYMBOL(v4l2_m2m_job_finish);
>  
> +void v4l2_m2m_suspend(struct v4l2_m2m_dev *m2m_dev)
> +{
> +	unsigned long flags;
> +	struct v4l2_m2m_ctx *curr_ctx;
> +
> +	spin_lock_irqsave(&m2m_dev->job_spinlock, flags);
> +	m2m_dev->job_queue_flags |= QUEUE_PAUSED;
> +	curr_ctx = m2m_dev->curr_ctx;
> +	spin_unlock_irqrestore(&m2m_dev->job_spinlock, flags);
> +
> +	if (curr_ctx)
> +		wait_event(curr_ctx->finished,
> +			   !(curr_ctx->job_flags & TRANS_RUNNING));
> +}
> +EXPORT_SYMBOL(v4l2_m2m_suspend);
> +
> +void v4l2_m2m_resume(struct v4l2_m2m_dev *m2m_dev)
> +{
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&m2m_dev->job_spinlock, flags);
> +	m2m_dev->job_queue_flags &= ~QUEUE_PAUSED;
> +	spin_unlock_irqrestore(&m2m_dev->job_spinlock, flags);
> +
> +	v4l2_m2m_try_run(m2m_dev);
> +}
> +EXPORT_SYMBOL(v4l2_m2m_resume);
> +
>  int v4l2_m2m_reqbufs(struct file *file, struct v4l2_m2m_ctx *m2m_ctx,
>  		     struct v4l2_requestbuffers *reqbufs)
>  {
> diff --git a/include/media/v4l2-mem2mem.h b/include/media/v4l2-mem2mem.h
> index 5467264771ec..119a195da390 100644
> --- a/include/media/v4l2-mem2mem.h
> +++ b/include/media/v4l2-mem2mem.h
> @@ -183,6 +183,28 @@ v4l2_m2m_buf_done(struct vb2_v4l2_buffer *buf, enum vb2_buffer_state state)
>  	vb2_buffer_done(&buf->vb2_buf, state);
>  }
>  
> +/**
> + * v4l2_m2m_suspend() - stop new jobs from being run and wait for current job
> + * to finish
> + *
> + * @m2m_dev: opaque pointer to the internal data to handle M2M context
> + *
> + * Called by a driver in the suspend hook. Stop new jobs from being run, and
> + * wait for current running job to finish.
> + */
> +void v4l2_m2m_suspend(struct v4l2_m2m_dev *m2m_dev);
> +
> +/**
> + * v4l2_m2m_resume() - resume job running and try to run a queued job
> + *
> + * @m2m_dev: opaque pointer to the internal data to handle M2M context
> + *
> + * Called by a driver in the resume hook. This reverts the operation of
> + * v4l2_m2m_suspend() and allows job to be run. Also try to run a queued job if
> + * there is any.
> + */
> +void v4l2_m2m_resume(struct v4l2_m2m_dev *m2m_dev);
> +
>  /**
>   * v4l2_m2m_reqbufs() - multi-queue-aware REQBUFS multiplexer
>   *
> -- 
> 2.18.0

Reviewed-by: Tomasz Figa <tfiga@chromium.org>

[Corrected Hans's email address.]
Hans, does this look good to you?

Best regards,
Tomasz


^ permalink raw reply

* [PATCH 2/2] dt-bindings: snps,dw-apb-ssi: add optional reset property
From: Dinh Nguyen @ 2020-05-21 17:03 UTC (permalink / raw)
  To: linux-kernel; +Cc: dinguyen, devicetree, broonie, robh+dt, linux-spi
In-Reply-To: <20200521170359.20430-1-dinguyen@kernel.org>

Add optional reset property.

Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
---
 Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
index 3ed08ee9feba..e01c557a9876 100644
--- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
+++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
@@ -37,5 +37,7 @@ Example:
 		num-cs = <2>;
 		cs-gpios = <&gpio0 13 0>,
 			   <&gpio0 14 0>;
+		resets = <&rst SPIM0_RST>;
+		reset-names = "spi";
 	};
 
-- 
2.17.1


^ permalink raw reply related

* [PATCH 1/2] spi: dw: add reset control
From: Dinh Nguyen @ 2020-05-21 17:03 UTC (permalink / raw)
  To: linux-kernel
  Cc: dinguyen, devicetree, broonie, robh+dt, linux-spi, Liang Jin J

Add mechanism to get the reset control and deassert it in order to bring
the IP out of reset.

Signed-off-by: Liang Jin J <liang.j.jin@ericsson.com>
Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
---
 drivers/spi/spi-dw-mmio.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c
index 384a3ab6dc2d..5c813e15ed89 100644
--- a/drivers/spi/spi-dw-mmio.c
+++ b/drivers/spi/spi-dw-mmio.c
@@ -20,6 +20,7 @@
 #include <linux/acpi.h>
 #include <linux/property.h>
 #include <linux/regmap.h>
+#include <linux/reset.h>
 
 #include "spi-dw.h"
 
@@ -30,6 +31,7 @@ struct dw_spi_mmio {
 	struct clk     *clk;
 	struct clk     *pclk;
 	void           *priv;
+	struct reset_control	*rstc;
 };
 
 #define MSCC_CPU_SYSTEM_CTRL_GENERAL_CTRL	0x24
@@ -145,6 +147,10 @@ static int dw_spi_mmio_probe(struct platform_device *pdev)
 	if (!dwsmmio)
 		return -ENOMEM;
 
+	dwsmmio->rstc = devm_reset_control_get_exclusive(&pdev->dev, "spi");
+	if (!IS_ERR(dwsmmio->rstc))
+		reset_control_deassert(dwsmmio->rstc);
+
 	dws = &dwsmmio->dws;
 
 	/* Get basic io resource and map it */
@@ -220,6 +226,9 @@ static int dw_spi_mmio_remove(struct platform_device *pdev)
 	clk_disable_unprepare(dwsmmio->pclk);
 	clk_disable_unprepare(dwsmmio->clk);
 
+	if (dwsmmio->rstc)
+		reset_control_assert(dwsmmio->rstc);
+
 	return 0;
 }
 
-- 
2.17.1


^ permalink raw reply related

* RE: [PATCH v9 10/10] arm64: dts: Add node for ufs exynos7
From: Alim Akhtar @ 2020-05-21 16:49 UTC (permalink / raw)
  To: 'Krzysztof Kozlowski'
  Cc: robh, devicetree, linux-scsi, avri.altman, martin.petersen,
	kwmad.kim, stanley.chu, cang, linux-samsung-soc, linux-arm-kernel,
	linux-kernel
In-Reply-To: <20200519071636.GA6971@kozik-lap>



> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: 19 May 2020 12:47
> To: Alim Akhtar <alim.akhtar@samsung.com>
> Cc: robh@kernel.org; devicetree@vger.kernel.org; linux-scsi@vger.kernel.org;
> avri.altman@wdc.com; martin.petersen@oracle.com;
> kwmad.kim@samsung.com; stanley.chu@mediatek.com;
> cang@codeaurora.org; linux-samsung-soc@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v9 10/10] arm64: dts: Add node for ufs exynos7
> 
> On Thu, May 14, 2020 at 06:09:14AM +0530, Alim Akhtar wrote:
> > Adding dt node foe UFS and UFS-PHY for exynos7 SoC.
> >
> > Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
> > Tested-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
> > ---
> >  .../boot/dts/exynos/exynos7-espresso.dts      |  4 ++
> >  arch/arm64/boot/dts/exynos/exynos7.dtsi       | 43 ++++++++++++++++++-
> >  2 files changed, 45 insertions(+), 2 deletions(-)
> 
> I will pick it up after all bindings get Rob's ack (or are picked up as well).  The
> second bindings patch are still pending on that.
> 
Thank Krzysztof,
Yes, one binding still awaiting Rob's ack, I have addressed his comment in this v9 series.
Hoping he will find some time to review the same.

> Best regards,
> Krzysztof



^ permalink raw reply

* Re: [PATCH v3 03/16] spi: dw: Discard static DW DMA slave structures
From: Andy Shevchenko @ 2020-05-21 16:44 UTC (permalink / raw)
  To: Mark Brown
  Cc: Serge Semin, Serge Semin, Georgy Vlasov, Ramil Zaripov,
	Alexey Malahov, Thomas Bogendoerfer, Paul Burton, Ralf Baechle,
	Andy Shevchenko, Arnd Bergmann, Rob Herring, linux-mips,
	devicetree, Thomas Gleixner, Wan Ahmad Zainie, Jarkko Nikula,
	Clement Leger, linux-spi, Linux Kernel Mailing List
In-Reply-To: <20200521163904.GF4770@sirena.org.uk>

On Thu, May 21, 2020 at 7:39 PM Mark Brown <broonie@kernel.org> wrote:
> On Thu, May 21, 2020 at 07:02:32PM +0300, Andy Shevchenko wrote:
> > On Thu, May 21, 2020 at 6:58 PM Serge Semin
>
> > > Let's leave the patch as is.
>
> > Mark, should I send a partial revert afterwards in this case?
> > I'm not fully satisfied with it.
>
> That might be a suitable way to keep the peace here.  You are clearly
> both much more passionate about this choice than I am.

Okay, will work for me, thanks!

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* Re: [PATCH v3 03/16] spi: dw: Discard static DW DMA slave structures
From: Mark Brown @ 2020-05-21 16:39 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Serge Semin, Serge Semin, Georgy Vlasov, Ramil Zaripov,
	Alexey Malahov, Thomas Bogendoerfer, Paul Burton, Ralf Baechle,
	Andy Shevchenko, Arnd Bergmann, Rob Herring, linux-mips,
	devicetree, Thomas Gleixner, Wan Ahmad Zainie, Jarkko Nikula,
	Clement Leger, linux-spi, Linux Kernel Mailing List
In-Reply-To: <CAHp75VcY9HG3=wft9VFWKJD9PG6TiSbmOyjgYZyue+z3PW_eCQ@mail.gmail.com>

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

On Thu, May 21, 2020 at 07:02:32PM +0300, Andy Shevchenko wrote:
> On Thu, May 21, 2020 at 6:58 PM Serge Semin

> > Let's leave the patch as is.

> Mark, should I send a partial revert afterwards in this case?
> I'm not fully satisfied with it.

That might be a suitable way to keep the peace here.  You are clearly
both much more passionate about this choice than I am.

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

^ permalink raw reply

* Re: [PATCH v11] arm64: dts: qcom: sc7180: Add WCN3990 WLAN module device node
From: Evan Green @ 2020-05-21 16:19 UTC (permalink / raw)
  To: Rakesh Pillai
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-arm Mailing List, LKML, linux-arm-msm
In-Reply-To: <1589946996-31264-1-git-send-email-pillair@codeaurora.org>

On Tue, May 19, 2020 at 8:57 PM Rakesh Pillai <pillair@codeaurora.org> wrote:
>
> Add device node for the ath10k SNOC platform driver probe
> and add resources required for WCN3990 on sc7180 soc.
>
> Signed-off-by: Rakesh Pillai <pillair@codeaurora.org>

Reviewed-by: Evan Green <evgreen@chromium.org>

^ permalink raw reply

* Re: [PATCH v8 14/14] media: platform: Add jpeg dec/enc feature
From: Tomasz Figa @ 2020-05-21 16:08 UTC (permalink / raw)
  To: Hans Verkuil, Xia Jiang
  Cc: Xia Jiang, Mauro Carvalho Chehab, Rob Herring, Matthias Brugger,
	Rick Chang, linux-media, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek, Marek Szyprowski,
	srv_heupstream, senozhatsky, mojahsu, drinkcat, maoguang.meng,
	sj.huang
In-Reply-To: <b62b303c-10cd-fdf6-52fa-90d63124487a@xs4all.nl>

On Mon, May 11, 2020 at 11:04:19AM +0200, Hans Verkuil wrote:
> On 03/04/2020 11:40, Xia Jiang wrote:
> > Add mtk jpeg encode v4l2 driver based on jpeg decode, because that jpeg
> > decode and encode have great similarities with function operation.
> > 
> > Signed-off-by: Xia Jiang <xia.jiang@mediatek.com>
> > ---
> > v8:jpeg encoder and decoder use separate callbacks instead of repeating
> >    the if/else in every callback.
> >    improve vidioc_try_fmt() implementation that can be shared by jpeg
> >    encoder and decoder.
> >    fix the bug of jpeg encoder s_selection implementation.
> >    cancel the state of the jpeg encoder.
> >    improve jpeg encoder and decoder set default params flow.
> >    put the clock names and other datas in a match_data struct.
> >    fix the bug of geting correctly quality value.
> >    do the all the bits' settings of one register in one function.
> >    move the code of mtk_jpeg_enc_reg.h to mtk_jpeg_enc_hw.h and delete
> >    mtk_jpeg_enc_reg.h.
> > 
> > v7: reverse spin lock and unlock operation in device run function for
> >     multi-instance.
> > 
> > v6: add space to arounding '+'.
> >     alignment 'struct mtk_jpeg_fmt *fmt' match open parenthesis.
> >     change 'mtk_jpeg_enc_set_encFormat' to 'mtk_jpeg_enc_set_enc_format'.
> >     make 'mtk_jpeg_ctrls_setup' to static prototype.
> >     delete unused variables 'jpeg'/'align_h'/'align_w'/'flags'.
> >     initialize 'yuv_format'/'enc_quality' variables.
> >     
> > v5: support crop for encoder and compose for decoder in s_selection and
> >     g_selection function.
> >     use clamp() to replace mtk_jpeg_bound_align_image() and round_up()
> >     to replace mtk_jpeg_align().
> >     delete jpeg_enc_param/mtk_jpeg_enc_param structure and
> >     mtk_jpeg_set_param(), program the registers directly based on
> >     the original V4L2 values.
> >     move macro definition about hw to mtk_jpeg_enc_reg.h.
> >     delete unnecessary V4L2 logs in driver.
> >     cancel spin lock and unlock operation in deviec run function.
> >     change jpeg enc register offset hex numberals from upercase to lowercase.
> > 
> > v4: split mtk_jpeg_try_fmt_mplane() to two functions, one for encoder,                                                      
> >     one for decoder.                                                          
> >     split mtk_jpeg_set_default_params() to two functions, one for                                                          
> >     encoder, one for decoder.                                                          
> >     add cropping support for encoder in g/s_selection ioctls.                                                          
> >     change exif mode support by using V4L2_JPEG_ACTIVE_MARKER_APP1.                                                         
> >     change MTK_JPEG_MAX_WIDTH/MTK_JPEG_MAX_HEIGH from 8192 to 65535 by                                                      
> >     specification.                                                          
> >     move width shifting operation behind aligning operation in                                                          
> >     mtk_jpeg_try_enc_fmt_mplane() for bug fix.                                                          
> >     fix user abuseing data_offset issue for DMABUF in                                                          
> >     mtk_jpeg_set_enc_src().                                                          
> >     fix kbuild warings: change MTK_JPEG_MIN_HEIGHT/MTK_JPEG_MAX_HEIGHT                                                      
> >                         and MTK_JPEG_MIN_WIDTH/MTK_JPEG_MAX_WIDTH from                                                      
> >                         'int' type to 'unsigned int' type.                                                          
> >                         fix msleadingly indented of 'else'.                                                                                                              
> > v3: delete Change-Id.                                                          
> >     only test once handler->error after the last v4l2_ctrl_new_std().                                                       
> >     seperate changes of v4l2-ctrls.c and v4l2-controls.h to new patch.                                                      
> > v2: fix compliance test fail, check created buffer size in driver.
> > ---
> >  drivers/media/platform/mtk-jpeg/Makefile      |    5 +-
> >  .../media/platform/mtk-jpeg/mtk_jpeg_core.c   | 1038 +++++++++++++----
> >  .../media/platform/mtk-jpeg/mtk_jpeg_core.h   |   51 +-
> >  .../media/platform/mtk-jpeg/mtk_jpeg_dec_hw.h |    7 +-
> >  .../media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c |  193 +++
> >  .../media/platform/mtk-jpeg/mtk_jpeg_enc_hw.h |  123 ++
> >  6 files changed, 1188 insertions(+), 229 deletions(-)
> >  create mode 100644 drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c
> >  create mode 100644 drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.h
> > 
> > diff --git a/drivers/media/platform/mtk-jpeg/Makefile b/drivers/media/platform/mtk-jpeg/Makefile
> > index 48516dcf96e6..76c33aad0f3f 100644
> > --- a/drivers/media/platform/mtk-jpeg/Makefile
> > +++ b/drivers/media/platform/mtk-jpeg/Makefile
> > @@ -1,3 +1,6 @@
> >  # SPDX-License-Identifier: GPL-2.0-only
> > -mtk_jpeg-objs := mtk_jpeg_core.o mtk_jpeg_dec_hw.o mtk_jpeg_dec_parse.o
> > +mtk_jpeg-objs := mtk_jpeg_core.o \
> > +		 mtk_jpeg_dec_hw.o \
> > +		 mtk_jpeg_dec_parse.o \
> > +		 mtk_jpeg_enc_hw.o
> >  obj-$(CONFIG_VIDEO_MEDIATEK_JPEG) += mtk_jpeg.o
> > diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > index 77a95185584c..18a759ce2c46 100644
> > --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > @@ -3,6 +3,7 @@
> >   * Copyright (c) 2016 MediaTek Inc.
> >   * Author: Ming Hsiu Tsai <minghsiu.tsai@mediatek.com>
> >   *         Rick Chang <rick.chang@mediatek.com>
> > + *         Xia Jiang <xia.jiang@mediatek.com>
> >   */
> >  
> >  #include <linux/clk.h>
> > @@ -23,11 +24,60 @@
> >  #include <media/videobuf2-dma-contig.h>
> >  #include <soc/mediatek/smi.h>
> >  
> > +#include "mtk_jpeg_enc_hw.h"
> >  #include "mtk_jpeg_dec_hw.h"
> >  #include "mtk_jpeg_core.h"
> >  #include "mtk_jpeg_dec_parse.h"
> >  
> > -static struct mtk_jpeg_fmt mtk_jpeg_formats[] = {
> > +static struct mtk_jpeg_fmt mtk_jpeg_enc_formats[] = {
> > +	{
> > +		.fourcc		= V4L2_PIX_FMT_JPEG,
> > +		.colplanes	= 1,
> > +		.flags		= MTK_JPEG_FMT_FLAG_ENC_CAPTURE,
> > +	},
> > +	{
> > +		.fourcc		= V4L2_PIX_FMT_NV12M,
> > +		.hw_format	= JPEG_ENC_YUV_FORMAT_NV12,
> > +		.h_sample	= {4, 4},
> > +		.v_sample	= {4, 2},
> > +		.colplanes	= 2,
> > +		.h_align	= 4,
> > +		.v_align	= 4,
> > +		.flags		= MTK_JPEG_FMT_FLAG_ENC_OUTPUT,
> > +	},
> > +	{
> > +		.fourcc		= V4L2_PIX_FMT_NV21M,
> > +		.hw_format	= JEPG_ENC_YUV_FORMAT_NV21,
> > +		.h_sample	= {4, 4},
> > +		.v_sample	= {4, 2},
> > +		.colplanes	= 2,
> > +		.h_align	= 4,
> > +		.v_align	= 4,
> > +		.flags		= MTK_JPEG_FMT_FLAG_ENC_OUTPUT,
> > +	},
> > +	{
> > +		.fourcc		= V4L2_PIX_FMT_YUYV,
> > +		.hw_format	= JPEG_ENC_YUV_FORMAT_YUYV,
> > +		.h_sample	= {8},
> > +		.v_sample	= {4},
> > +		.colplanes	= 1,
> > +		.h_align	= 5,
> > +		.v_align	= 3,
> > +		.flags		= MTK_JPEG_FMT_FLAG_ENC_OUTPUT,
> > +	},
> > +	{
> > +		.fourcc		= V4L2_PIX_FMT_YVYU,
> > +		.hw_format	= JPEG_ENC_YUV_FORMAT_YVYU,
> > +		.h_sample	= {8},
> > +		.v_sample	= {4},
> > +		.colplanes	= 1,
> > +		.h_align	= 5,
> > +		.v_align	= 3,
> > +		.flags		= MTK_JPEG_FMT_FLAG_ENC_OUTPUT,
> > +	},
> > +};
> > +
> > +static struct mtk_jpeg_fmt mtk_jpeg_dec_formats[] = {
> >  	{
> >  		.fourcc		= V4L2_PIX_FMT_JPEG,
> >  		.colplanes	= 1,
> > @@ -53,7 +103,8 @@ static struct mtk_jpeg_fmt mtk_jpeg_formats[] = {
> >  	},
> >  };
> >  
> > -#define MTK_JPEG_NUM_FORMATS ARRAY_SIZE(mtk_jpeg_formats)
> > +#define MTK_JPEG_ENC_NUM_FORMATS ARRAY_SIZE(mtk_jpeg_enc_formats)
> > +#define MTK_JPEG_DEC_NUM_FORMATS ARRAY_SIZE(mtk_jpeg_dec_formats)
> >  
> >  enum {
> >  	MTK_JPEG_BUF_FLAGS_INIT			= 0,
> > @@ -70,6 +121,11 @@ struct mtk_jpeg_src_buf {
> >  static int debug;
> >  module_param(debug, int, 0644);
> >  
> > +static inline struct mtk_jpeg_ctx *ctrl_to_ctx(struct v4l2_ctrl *ctrl)
> > +{
> > +	return container_of(ctrl->handler, struct mtk_jpeg_ctx, ctrl_hdl);
> > +}
> > +
> >  static inline struct mtk_jpeg_ctx *mtk_jpeg_fh_to_ctx(struct v4l2_fh *fh)
> >  {
> >  	return container_of(fh, struct mtk_jpeg_ctx, fh);
> > @@ -81,12 +137,25 @@ static inline struct mtk_jpeg_src_buf *mtk_jpeg_vb2_to_srcbuf(
> >  	return container_of(to_vb2_v4l2_buffer(vb), struct mtk_jpeg_src_buf, b);
> >  }
> >  
> > -static int mtk_jpeg_querycap(struct file *file, void *priv,
> > -			     struct v4l2_capability *cap)
> > +static int mtk_jpeg_enc_querycap(struct file *file, void *priv,
> > +				 struct v4l2_capability *cap)
> > +{
> > +	struct mtk_jpeg_dev *jpeg = video_drvdata(file);
> > +
> > +	strscpy(cap->driver, MTK_JPEG_NAME, sizeof(cap->driver));
> > +	strscpy(cap->card, MTK_JPEG_NAME " encoder", sizeof(cap->card));
> > +	snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:%s",
> > +		 dev_name(jpeg->dev));
> > +
> > +	return 0;
> > +}
> > +
> > +static int mtk_jpeg_dec_querycap(struct file *file, void *priv,
> > +				 struct v4l2_capability *cap)
> >  {
> >  	struct mtk_jpeg_dev *jpeg = video_drvdata(file);
> >  
> > -	strscpy(cap->driver, MTK_JPEG_NAME " decoder", sizeof(cap->driver));
> > +	strscpy(cap->driver, MTK_JPEG_NAME, sizeof(cap->driver));
> >  	strscpy(cap->card, MTK_JPEG_NAME " decoder", sizeof(cap->card));
> >  	snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:%s",
> >  		 dev_name(jpeg->dev));
> > @@ -94,6 +163,54 @@ static int mtk_jpeg_querycap(struct file *file, void *priv,
> >  	return 0;
> >  }
> >  
> > +static int vidioc_jpeg_enc_s_ctrl(struct v4l2_ctrl *ctrl)
> > +{
> > +	struct mtk_jpeg_ctx *ctx = ctrl_to_ctx(ctrl);
> > +
> > +	switch (ctrl->id) {
> > +	case V4L2_CID_JPEG_RESTART_INTERVAL:
> > +		ctx->restart_interval = ctrl->val;
> > +		break;
> > +	case V4L2_CID_JPEG_COMPRESSION_QUALITY:
> > +		ctx->enc_quality = ctrl->val;
> > +		break;
> > +	case V4L2_CID_JPEG_ACTIVE_MARKER:
> > +		ctx->enable_exif = ctrl->val & V4L2_JPEG_ACTIVE_MARKER_APP1 ?
> > +				   true : false;
> > +		break;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct v4l2_ctrl_ops mtk_jpeg_enc_ctrl_ops = {
> > +	.s_ctrl = vidioc_jpeg_enc_s_ctrl,
> > +};
> > +
> > +static int mtk_jpeg_enc_ctrls_setup(struct mtk_jpeg_ctx *ctx)
> > +{
> > +	const struct v4l2_ctrl_ops *ops = &mtk_jpeg_enc_ctrl_ops;
> > +	struct v4l2_ctrl_handler *handler = &ctx->ctrl_hdl;
> > +
> > +	v4l2_ctrl_handler_init(handler, 3);
> > +
> > +	v4l2_ctrl_new_std(handler, ops, V4L2_CID_JPEG_RESTART_INTERVAL, 0, 100,
> > +			  1, 0);
> > +	v4l2_ctrl_new_std(handler, ops, V4L2_CID_JPEG_COMPRESSION_QUALITY, 48,
> > +			  100, 1, 90);
> > +	v4l2_ctrl_new_std(handler, ops, V4L2_CID_JPEG_ACTIVE_MARKER, 0,
> > +			  V4L2_JPEG_ACTIVE_MARKER_APP1, 0, 0);
> > +
> > +	if (handler->error) {
> > +		v4l2_ctrl_handler_free(&ctx->ctrl_hdl);
> > +		return handler->error;
> > +	}
> > +
> > +	v4l2_ctrl_handler_setup(&ctx->ctrl_hdl);
> > +
> > +	return 0;
> > +}
> > +
> >  static int mtk_jpeg_enum_fmt(struct mtk_jpeg_fmt *mtk_jpeg_formats, int n,
> >  			     struct v4l2_fmtdesc *f, u32 type)
> >  {
> > @@ -115,117 +232,105 @@ static int mtk_jpeg_enum_fmt(struct mtk_jpeg_fmt *mtk_jpeg_formats, int n,
> >  	return 0;
> >  }
> >  
> > -static int mtk_jpeg_enum_fmt_vid_cap(struct file *file, void *priv,
> > -				     struct v4l2_fmtdesc *f)
> > +static int mtk_jpeg_enc_enum_fmt_vid_cap(struct file *file, void *priv,
> > +					 struct v4l2_fmtdesc *f)
> >  {
> > -	return mtk_jpeg_enum_fmt(mtk_jpeg_formats, MTK_JPEG_NUM_FORMATS, f,
> > +	return mtk_jpeg_enum_fmt(mtk_jpeg_enc_formats,
> > +				 MTK_JPEG_ENC_NUM_FORMATS, f,
> > +				 MTK_JPEG_FMT_FLAG_ENC_CAPTURE);
> > +}
> > +
> > +static int mtk_jpeg_dec_enum_fmt_vid_cap(struct file *file, void *priv,
> > +					 struct v4l2_fmtdesc *f)
> > +{
> > +	return mtk_jpeg_enum_fmt(mtk_jpeg_dec_formats,
> > +				 MTK_JPEG_DEC_NUM_FORMATS, f,
> >  				 MTK_JPEG_FMT_FLAG_DEC_CAPTURE);
> >  }
> >  
> > -static int mtk_jpeg_enum_fmt_vid_out(struct file *file, void *priv,
> > -				     struct v4l2_fmtdesc *f)
> > +static int mtk_jpeg_enc_enum_fmt_vid_out(struct file *file, void *priv,
> > +					 struct v4l2_fmtdesc *f)
> > +{
> > +	return mtk_jpeg_enum_fmt(mtk_jpeg_enc_formats,
> > +				 MTK_JPEG_ENC_NUM_FORMATS, f,
> > +				 MTK_JPEG_FMT_FLAG_ENC_OUTPUT);
> > +}
> > +
> > +static int mtk_jpeg_dec_enum_fmt_vid_out(struct file *file, void *priv,
> > +					 struct v4l2_fmtdesc *f)
> >  {
> > -	return mtk_jpeg_enum_fmt(mtk_jpeg_formats, MTK_JPEG_NUM_FORMATS, f,
> > -				 MTK_JPEG_FMT_FLAG_DEC_OUTPUT);
> > +	return mtk_jpeg_enum_fmt(mtk_jpeg_dec_formats, MTK_JPEG_DEC_NUM_FORMATS,
> > +				 f, MTK_JPEG_FMT_FLAG_DEC_OUTPUT);
> >  }
> 
> OK, so this patch is very hard to read because there are two independent changes
> taking place:
> 
> 1) rename existing functions/defines/variables with a _dec prefix to prepare
>    for the addition of the encoder feature.
> 
> 2) add the encoder feature.
> 
> Please split up this patch into two parts: one that does the rename and as much of
> the preparation to support both decoder and encoder without changing the
> functionality, and a second one that actually adds the new encoder feature.
> 
> In fact, once that's done it is likely that most of this patch series can be
> merged, even if there are still things that need to be changed for the last
> patch adding the encoder support. I see nothing objectionable in patches 1-10
> and 13. So merging those together with a new rename patch wouldn't be an issue,
> I think.
> 
> In any case, the diffs should be a lot cleaner and easier to review by splitting
> it up like that.

Agreed with Hans. The split will make it much easier to review this one
for me, so I'll wait for it. Thanks in advance.

Best regards,
Tomasz

^ permalink raw reply

* Re: [PATCH v3 03/16] spi: dw: Discard static DW DMA slave structures
From: Andy Shevchenko @ 2020-05-21 16:02 UTC (permalink / raw)
  To: Serge Semin
  Cc: Mark Brown, Serge Semin, Georgy Vlasov, Ramil Zaripov,
	Alexey Malahov, Thomas Bogendoerfer, Paul Burton, Ralf Baechle,
	Andy Shevchenko, Arnd Bergmann, Rob Herring, linux-mips,
	devicetree, Thomas Gleixner, Wan Ahmad Zainie, Jarkko Nikula,
	Clement Leger, linux-spi, Linux Kernel Mailing List
In-Reply-To: <20200521155804.qgaaqc23h2nll7r3@mobilestation>

On Thu, May 21, 2020 at 6:58 PM Serge Semin
<Sergey.Semin@baikalelectronics.ru> wrote:
> On Thu, May 21, 2020 at 04:51:43PM +0100, Mark Brown wrote:
> > On Thu, May 21, 2020 at 03:12:28PM +0300, Serge Semin wrote:
> >
> > > Well, for me both solutions are equal except mine consumes less stack memory.
> > > The only reason why your solution might be better is that if DW DMA driver or
> > > the DMA engine subsystem changed the dw_dma_slave structure instance passed to
> > > the dma_request_channel() method, which non of them do. So I'll leave this for
> > > Mark to decide. Mark, could you give us your final word about this?
> >
> > Honestly I'm struggling to care either way.  I guess saving a bit of
> > stack is potentially useful.
>
> Settled then.

With whom?

> Let's leave the patch as is.

Mark, should I send a partial revert afterwards in this case?
I'm not fully satisfied with it.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* Re: [PATCH v8 13/14] media: platform: Rename jpeg dec file name
From: Tomasz Figa @ 2020-05-21 16:02 UTC (permalink / raw)
  To: Xia Jiang
  Cc: Hans Verkuil, Mauro Carvalho Chehab, Rob Herring,
	Matthias Brugger, Rick Chang, linux-media, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek, Marek Szyprowski,
	srv_heupstream, senozhatsky, mojahsu, drinkcat, maoguang.meng,
	sj.huang
In-Reply-To: <20200403094033.8288-14-xia.jiang@mediatek.com>

Hi Xia,

On Fri, Apr 03, 2020 at 05:40:32PM +0800, Xia Jiang wrote:
> Rename the files which are for decode feature. This is preparing
> path since the jpeg enc patch will be added later.
> 
> Signed-off-by: Xia Jiang <xia.jiang@mediatek.com>
> ---
> v8: no changes
> 
> v7: no changes
> 
> v6: no changes
> 
> v5: no changes
> 
> v4: no changes
> 
> v3: no changes
> 
> v2: no changes
> ---
>  drivers/media/platform/mtk-jpeg/Makefile                      | 2 +-
>  drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c               | 4 ++--
>  .../platform/mtk-jpeg/{mtk_jpeg_hw.c => mtk_jpeg_dec_hw.c}    | 2 +-
>  .../platform/mtk-jpeg/{mtk_jpeg_hw.h => mtk_jpeg_dec_hw.h}    | 2 +-
>  .../mtk-jpeg/{mtk_jpeg_parse.c => mtk_jpeg_dec_parse.c}       | 2 +-
>  .../mtk-jpeg/{mtk_jpeg_parse.h => mtk_jpeg_dec_parse.h}       | 2 +-
>  .../platform/mtk-jpeg/{mtk_jpeg_reg.h => mtk_jpeg_dec_reg.h}  | 0
>  7 files changed, 7 insertions(+), 7 deletions(-)
>  rename drivers/media/platform/mtk-jpeg/{mtk_jpeg_hw.c => mtk_jpeg_dec_hw.c} (99%)
>  rename drivers/media/platform/mtk-jpeg/{mtk_jpeg_hw.h => mtk_jpeg_dec_hw.h} (98%)
>  rename drivers/media/platform/mtk-jpeg/{mtk_jpeg_parse.c => mtk_jpeg_dec_parse.c} (98%)
>  rename drivers/media/platform/mtk-jpeg/{mtk_jpeg_parse.h => mtk_jpeg_dec_parse.h} (92%)
>  rename drivers/media/platform/mtk-jpeg/{mtk_jpeg_reg.h => mtk_jpeg_dec_reg.h} (100%)
> 

Reviewed-by: Tomasz Figa <tfiga@chromium.org>

Best regards,
Tomasz

^ permalink raw reply

* Re: [PATCH v8 11/14] media: dt-bindings: Add jpeg enc device tree node document
From: Tomasz Figa @ 2020-05-21 16:00 UTC (permalink / raw)
  To: Xia Jiang
  Cc: Hans Verkuil, Mauro Carvalho Chehab, Rob Herring,
	Matthias Brugger, Rick Chang, linux-media, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek, Marek Szyprowski,
	srv_heupstream, senozhatsky, mojahsu, drinkcat, maoguang.meng,
	sj.huang, yong.wu
In-Reply-To: <20200403094033.8288-12-xia.jiang@mediatek.com>

Hi Xia,

On Fri, Apr 03, 2020 at 05:40:30PM +0800, Xia Jiang wrote:
> Add jpeg enc device tree node document
> 
> Reviewed-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Xia Jiang <xia.jiang@mediatek.com>
> ---
> v8: no changes
> 
> v7: no changes
> 
> v6: no changes
> 
> v5: no changes
> 
> v4: no changes
> 
> v3: change compatible to SoC specific compatible
> 
> v2: no changes
> ---
>  .../bindings/media/mediatek-jpeg-encoder.txt  | 37 +++++++++++++++++++
>  1 file changed, 37 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/mediatek-jpeg-encoder.txt
> 

Thank you for the patch. Please see my comments inline.

> diff --git a/Documentation/devicetree/bindings/media/mediatek-jpeg-encoder.txt b/Documentation/devicetree/bindings/media/mediatek-jpeg-encoder.txt
> new file mode 100644
> index 000000000000..fa8da699493b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/mediatek-jpeg-encoder.txt
> @@ -0,0 +1,37 @@
> +* MediaTek JPEG Encoder
> +
> +MediaTek JPEG Encoder is the JPEG encode hardware present in MediaTek SoCs
> +
> +Required properties:
> +- compatible : should be one of:
> +               "mediatek,mt2701-jpgenc"
> +               ...

What does this "..." mean?

> +               followed by "mediatek,mtk-jpgenc"
> +- reg : physical base address of the JPEG encoder registers and length of
> +  memory mapped region.
> +- interrupts : interrupt number to the interrupt controller.
> +- clocks: device clocks, see
> +  Documentation/devicetree/bindings/clock/clock-bindings.txt for details.
> +- clock-names: must contain "jpgenc". It is the clock of JPEG encoder.

nit: In principle the clocks should be named after the function the clock
performs on the consumer side, i.e. the JPEG block in this case, I guess
here it's just a generic clock that does everything, but I guess it comes
from somewhere. Is it the AHB clock or something? In that case it would
normally be called "ahb".

> +- power-domains: a phandle to the power domain, see
> +  Documentation/devicetree/bindings/power/power_domain.txt for details.
> +- mediatek,larb: must contain the local arbiters in the current SoCs, see
> +  Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.txt
> +  for details.

I believe this isn't necessary anymore, because larbs are added
automatically by the MTK IOMMU driver using device links. +CC Yong who
worked on that.

> +- iommus: should point to the respective IOMMU block with master port as
> +  argument, see Documentation/devicetree/bindings/iommu/mediatek,iommu.txt
> +  for details.
> +
> +Example:
> +	jpegenc: jpegenc@1500a000 {
> +		compatible = "mediatek,mt2701-jpgenc",
> +			     "mediatek,mtk-jpgenc";
> +		reg = <0 0x1500a000 0 0x1000>;
> +		interrupts = <GIC_SPI 141 IRQ_TYPE_LEVEL_LOW>;
> +		clocks =  <&imgsys CLK_IMG_VENC>;
> +		clock-names = "jpgenc";
> +		power-domains = <&scpsys MT2701_POWER_DOMAIN_ISP>;
> +		mediatek,larb = <&larb2>;

Ditto.

Best regards,
Tomasz

^ permalink raw reply

* Re: [PATCH V6 2/7] soc: qcom-geni-se: Add interconnect support to fix earlycon crash
From: Matthias Kaehlcke @ 2020-05-21 16:00 UTC (permalink / raw)
  To: Akash Asthana
  Cc: gregkh, agross, bjorn.andersson, wsa, broonie, mark.rutland,
	robh+dt, linux-i2c, linux-spi, devicetree, swboyd, mgautam,
	linux-arm-msm, linux-serial, dianders, msavaliy, evgreen
In-Reply-To: <1590049764-20912-3-git-send-email-akashast@codeaurora.org>

Hi Akash,

On Thu, May 21, 2020 at 01:59:19PM +0530, Akash Asthana wrote:
> QUP core clock is shared among all the SE drivers present on particular
> QUP wrapper, the system will reset(unclocked access) if earlycon used after
> QUP core clock is put to 0 from other SE drivers before real console comes
> up.
> 
> As earlycon can't vote for it's QUP core need, to fix this add ICC
> support to common/QUP wrapper driver and put vote for QUP core from
> probe on behalf of earlycon and remove vote during earlycon exit call.
> 
> Signed-off-by: Akash Asthana <akashast@codeaurora.org>
> Reported-by: Matthias Kaehlcke <mka@chromium.org>
> ---
> Change in V3:
>  - Add geni_remove_earlycon_icc_vote API that will be used by earlycon
>    exit function to remove ICC vote for earlyconsole.
>  - Remove suspend/resume hook for geni-se driver as we are no longer
>    removing earlyconsole ICC vote from system suspend, we are removing
>    from earlycon exit.
> 
> Change in V4:
>  - As per Matthias comment make 'earlycon_wrapper' as static structure.
> 
> Changes in V5:
>  - Vote for core path only after checking whether "qcom_geni" earlycon is
>    actually present or not by traversing over structure "console_drivers".
> 
> Changes in V6:
>  - As per Matthias's comment removed NULL check for console_drivers global
>    struct, added NULL check for earlycon_wrapper in _remove_earlycon_icc_vote
>    API
>  - Addressed nitpicks from Andy.
> 
>  drivers/soc/qcom/qcom-geni-se.c       | 68 +++++++++++++++++++++++++++++++++++
>  drivers/tty/serial/qcom_geni_serial.c |  7 ++++
>  include/linux/qcom-geni-se.h          |  2 ++
>  3 files changed, 77 insertions(+)
> 
> diff --git a/drivers/soc/qcom/qcom-geni-se.c b/drivers/soc/qcom/qcom-geni-se.c
> index 0b2526d..ac16bb1 100644
> --- a/drivers/soc/qcom/qcom-geni-se.c
> +++ b/drivers/soc/qcom/qcom-geni-se.c
> @@ -3,6 +3,7 @@
>  
>  #include <linux/acpi.h>
>  #include <linux/clk.h>
> +#include <linux/console.h>
>  #include <linux/slab.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/io.h>
> @@ -90,11 +91,14 @@ struct geni_wrapper {
>  	struct device *dev;
>  	void __iomem *base;
>  	struct clk_bulk_data ahb_clks[NUM_AHB_CLKS];
> +	struct geni_icc_path to_core;
>  };
>  
>  static const char * const icc_path_names[] = {"qup-core", "qup-config",
>  						"qup-memory"};
>  
> +static struct geni_wrapper *earlycon_wrapper;
> +
>  #define QUP_HW_VER_REG			0x4
>  
>  /* Common SE registers */
> @@ -812,11 +816,38 @@ int geni_icc_disable(struct geni_se *se)
>  }
>  EXPORT_SYMBOL(geni_icc_disable);
>  
> +void geni_remove_earlycon_icc_vote(void)
> +{
> +	struct geni_wrapper *wrapper;
> +	struct device_node *parent;
> +	struct device_node *child;
> +
> +	if (!earlycon_wrapper)
> +		return;
> +
> +	wrapper = earlycon_wrapper;
> +	parent = of_get_next_parent(wrapper->dev->of_node);
> +	for_each_child_of_node(parent, child) {
> +		if (!of_device_is_compatible(child, "qcom,geni-se-qup"))
> +			continue;
> +		wrapper = platform_get_drvdata(of_find_device_by_node(child));
> +		icc_put(wrapper->to_core.path);
> +		wrapper->to_core.path = NULL;
> +
> +	}
> +	of_node_put(parent);
> +
> +	earlycon_wrapper = NULL;
> +}
> +EXPORT_SYMBOL(geni_remove_earlycon_icc_vote);
> +
>  static int geni_se_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
>  	struct resource *res;
>  	struct geni_wrapper *wrapper;
> +	struct console __maybe_unused *bcon;
> +	bool __maybe_unused has_earlycon = false;
>  	int ret;
>  
>  	wrapper = devm_kzalloc(dev, sizeof(*wrapper), GFP_KERNEL);
> @@ -839,6 +870,43 @@ static int geni_se_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> +#ifdef CONFIG_SERIAL_EARLYCON
> +	for_each_console(bcon) {
> +		if (!strcmp(bcon->name, "qcom_geni")) {
> +			has_earlycon = true;
> +			break;
> +		}
> +	}
> +	if (!has_earlycon)
> +		goto exit;
> +
> +	wrapper->to_core.path = devm_of_icc_get(dev, "qup-core");
> +	if (IS_ERR(wrapper->to_core.path))
> +		return PTR_ERR(wrapper->to_core.path);
> +	/*
> +	 * Put minmal BW request on core clocks on behalf of early console.
> +	 * The vote will be removed earlycon exit function.
> +	 *
> +	 * Note: We are putting vote on each QUP wrapper instead only to which
> +	 * earlycon is connected because QUP core clock of different wrapper
> +	 * share same voltage domain. If core1 is put to 0, then core2 will
> +	 * also run at 0, if not voted. Default ICC vote will be removed ASA
> +	 * we touch any of the core clock.
> +	 * core1 = core2 = max(core1, core2)
> +	 */
> +	ret = icc_set_bw(wrapper->to_core.path, GENI_DEFAULT_BW,
> +				GENI_DEFAULT_BW);
> +	if (ret) {
> +		dev_err(&pdev->dev, "%s: ICC BW voting failed for core :%d\n",

nit: " ... core: %d\n".

Reviewed-by: Matthias Kaehlcke <mka@chromium.org>

^ permalink raw reply

* Re: [PATCH v3 03/16] spi: dw: Discard static DW DMA slave structures
From: Serge Semin @ 2020-05-21 15:58 UTC (permalink / raw)
  To: Mark Brown
  Cc: Serge Semin, Andy Shevchenko, Georgy Vlasov, Ramil Zaripov,
	Alexey Malahov, Thomas Bogendoerfer, Paul Burton, Ralf Baechle,
	Andy Shevchenko, Arnd Bergmann, Rob Herring, linux-mips,
	devicetree, Thomas Gleixner, Wan Ahmad Zainie, Jarkko Nikula,
	Clement Leger, linux-spi, Linux Kernel Mailing List
In-Reply-To: <20200521155143.GE4770@sirena.org.uk>

On Thu, May 21, 2020 at 04:51:43PM +0100, Mark Brown wrote:
> On Thu, May 21, 2020 at 03:12:28PM +0300, Serge Semin wrote:
> 
> > Well, for me both solutions are equal except mine consumes less stack memory.
> > The only reason why your solution might be better is that if DW DMA driver or
> > the DMA engine subsystem changed the dw_dma_slave structure instance passed to
> > the dma_request_channel() method, which non of them do. So I'll leave this for
> > Mark to decide. Mark, could you give us your final word about this?
> 
> Honestly I'm struggling to care either way.  I guess saving a bit of
> stack is potentially useful.

Settled then. Let's leave the patch as is. I suppose we've finally finished a
review except a question Feng asked to the patch:
[PATCH v3 01/16] spi: dw: Add Tx/Rx finish wait methods to the MID DMA

If you are ok with my responses, then the patchset is ready for you further
actions.

-Sergey

^ permalink raw reply


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