linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] mmc: cavium: Fix MMC supply switching for cards
@ 2021-11-17 22:27 Wojciech Bartczak
  2021-11-17 22:28 ` [PATCH 1/2] mmc: cavium: Fix voltage reg. switching for card slots Wojciech Bartczak
  2021-11-17 22:28 ` [PATCH 2/2] dt-bindings: mmc: Add vmmc/vqmmc for Cavium driver Wojciech Bartczak
  0 siblings, 2 replies; 5+ messages in thread
From: Wojciech Bartczak @ 2021-11-17 22:27 UTC (permalink / raw)
  To: linux-mmc
  Cc: ulf.hansson, beanhuo, tanxiaofei, robh+dt, devicetree,
	linux-kernel, wbartczak

Following series of patches fix code related to switching
voltage regulators used by multiple cards in Octeon/OcteonTX2
systems.

Change is necessary to support cards with different voltages,
which is common case in modern configuration where the mix of SD cards
and MMC memory chips is used as main storage.

Aside custom system, the change is required to support reference designs
for CN96xx and CN98xx systems.

First patch contains mid-size rework of do_switch() routine that
includes:
- separation of vmmc/vqmmc switch logic
- separation of register accesses to functions for better readibility
- forming do_switch() as high-level function that controls the flow of
  switch operation.

Second patch adds extended description of device-tree usage to
enable code contained in former patch.

Wojciech Bartczak (2):
  mmc: cavium: Fix voltage reg. switching for card slots
  dt-bindings: mmc: Add vmmc/vqmmc for Cavium driver

 .../devicetree/bindings/mmc/cavium-mmc.txt    |  47 ++++-
 drivers/mmc/host/cavium.c                     | 168 ++++++++++++++++--
 drivers/mmc/host/cavium.h                     |   8 +-
 3 files changed, 202 insertions(+), 21 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/2] mmc: cavium: Fix voltage reg. switching for card slots
  2021-11-17 22:27 [PATCH 0/2] mmc: cavium: Fix MMC supply switching for cards Wojciech Bartczak
@ 2021-11-17 22:28 ` Wojciech Bartczak
  2021-11-19  3:50   ` kernel test robot
  2021-11-17 22:28 ` [PATCH 2/2] dt-bindings: mmc: Add vmmc/vqmmc for Cavium driver Wojciech Bartczak
  1 sibling, 1 reply; 5+ messages in thread
From: Wojciech Bartczak @ 2021-11-17 22:28 UTC (permalink / raw)
  To: linux-mmc
  Cc: ulf.hansson, beanhuo, tanxiaofei, robh+dt, devicetree,
	linux-kernel, wbartczak

Following commit enables usage of vqmmc and vmmc regulators
for devices with multiple voltages.
The MMC block is capable to control up to three devices at the same
time. However, only vmmc regulator has been used in the code.
The change is necessary to support SD and MMC devices
in single system. SD cards uses usually 3.3V regulators,
while faster MMC devices uses lower volatges (1.8V or 1.2V).

The switch operation logic remains unchanged. The sequence
has been rewritten to separate logic, register accesses and
voltage reguators manipulation.

Signed-off-by: Wojciech Bartczak <wbartczak@marvell.com>
---
 drivers/mmc/host/cavium.c | 168 ++++++++++++++++++++++++++++++++++----
 drivers/mmc/host/cavium.h |   8 +-
 2 files changed, 157 insertions(+), 19 deletions(-)

diff --git a/drivers/mmc/host/cavium.c b/drivers/mmc/host/cavium.c
index 95a41983c6c0..35d0ea3e3dd4 100644
--- a/drivers/mmc/host/cavium.c
+++ b/drivers/mmc/host/cavium.c
@@ -193,36 +193,166 @@ static int get_bus_id(u64 reg)
 	return FIELD_GET(GENMASK_ULL(61, 60), reg);
 }
 
-/*
- * We never set the switch_exe bit since that would interfere
- * with the commands send by the MMC core.
+/**
+ * pre_switch - Switch voltage regulators
+ * @prev: Current slot, can be NULL
+ * @next: Next slot
+ *
+ * Switch between regulators used by slots. This call has to be done
+ * after call to pre_switch().
  */
-static void do_switch(struct cvm_mmc_host *host, u64 emm_switch)
+static void switch_vqmmc(struct cvm_mmc_slot *prev, struct cvm_mmc_slot *next)
 {
-	int retries = 100;
-	u64 rsp_sts;
-	int bus_id;
+	bool same_vqmmc = false;
+	struct regulator *next_vqmmc;
+	struct regulator *prev_vqmmc;
+
+	next_vqmmc = next->mmc->supply.vqmmc;
+	if (prev) {
+		prev_vqmmc = prev->mmc->supply.vqmmc;
+		/* Prev slot and next slot share vqmmc? */
+		same_vqmmc = prev_vqmmc == next_vqmmc;
+		/* Disable old regulator, if not the same */
+		if (!same_vqmmc && !IS_ERR_OR_NULL(prev_vqmmc))
+			regulator_disable(prev_vqmmc);
+	}
 
-	/*
-	 * Modes setting only taken from slot 0. Work around that hardware
-	 * issue by first switching to slot 0.
-	 */
-	bus_id = get_bus_id(emm_switch);
-	clear_bus_id(&emm_switch);
-	writeq(emm_switch, host->base + MIO_EMM_SWITCH(host));
+	/* Enable regulator for next slot */
+	if (!same_vqmmc && !IS_ERR_OR_NULL(next_vqmmc)) {
+		int ret;
 
-	set_bus_id(&emm_switch, bus_id);
-	writeq(emm_switch, host->base + MIO_EMM_SWITCH(host));
+		ret = regulator_enable(next_vqmmc);
+		if (ret)
+			dev_err(mmc_classdev(next->mmc),
+				"Can't enable vqmmc, error (%d)!\n", ret);
+	}
+}
+
+/**
+ * pre_switch - Start switch operation between slots
+ * @prev: Current slot, can be NULL
+ * @next: Next slot
+ *
+ * MMC block for octeontx2 shares data lines among the three card slots.
+ * To avoid transient states on DAT[0..7] and CLK lines set
+ * CMDn to tri-state when VQMMC is switched.
+ */
+static void pre_switch(struct cvm_mmc_slot *prev, struct cvm_mmc_slot *next)
+{
+	struct cvm_mmc_host *host; /* Common block for all slots */
+
+	host = next->host;
+	if (host->use_vqmmc)
+		/* Disable clock and data lines on all slots */
+		writeq(1ull << 3, host->base + MIO_EMM_CFG(host));
+
+	if (prev) {
+		/* Store current slot settings */
+		prev->cached_switch = readq(host->base + MIO_EMM_SWITCH(host));
+		prev->cached_rca = readq(host->base + MIO_EMM_RCA(host));
+	}
+}
+
+/**
+ * post_switch - Finish switch operation
+ * @prev: Current slot
+ * @next: Next slot
+ *
+ * Function finishes switch operation by enabling selcted slot
+ * and recovering its settigs.
+ */
+static void post_switch(struct cvm_mmc_slot *prev, struct cvm_mmc_slot *next)
+{
+	struct cvm_mmc_host *host; /* Common block for all slots */
+
+	host = next->host;
+	if (host->use_vqmmc)
+		/* Enable clock and data lines for current slot */
+		writeq(1ull << next->bus_id, host->base + MIO_EMM_CFG(host));
+
+	/* Read back values set during switch */
+	next->cached_switch = readq(host->base + MIO_EMM_SWITCH(host));
+	/* Set RCA value */
+	writeq(next->cached_rca, host->base + MIO_EMM_RCA(host));
+}
 
-	/* wait for the switch to finish */
+/**
+ * mode_switch - Performs low level switch operation
+ * @host: Common hardware block for all slots
+ * @emm_switch: register value to use for the operation
+ *
+ * Function finishes switch operation by enabling selcted slot
+ * and recovering its settigs.
+ */
+static inline void mode_switch(struct cvm_mmc_host *host, u64 emm_switch)
+{
+	unsigned int retries = MODE_SWITCH_RETRIES;
+	u64 rsp_sts;
+
+	writeq(emm_switch, host->base + MIO_EMM_SWITCH(host));
+	/* Wait for switch completion */
 	do {
 		rsp_sts = readq(host->base + MIO_EMM_RSP_STS(host));
 		if (!(rsp_sts & MIO_EMM_RSP_STS_SWITCH_VAL))
 			break;
 		udelay(10);
 	} while (--retries);
+}
+
+/**
+ * do_switch - Performs switch operation
+ * @host: Common hardware block for all slots
+ * @emm_switch: register value to use for the operation
+ *
+ * Function performs switch operation between slost on hardware level.
+ * It is also called when the same slot changes mode, class or bus width.
+ * Switch requires sometimes to change current slot, store some of the slot
+ * information. Additionally, the volatge regulator for a card has to be
+ * switched too. Function uses use_vqmmc quirk for hardware.
+ * The quirk is related to errata that requires to go over slot 0
+ * for all switch operations, otherwise no parameters are accepted.
+ */
+static void do_switch(struct cvm_mmc_host *host, u64 emm_switch)
+{
+	struct cvm_mmc_slot *next, *prev;
+	int bus_id;
+	bool slot_changed;
+
+	/* Initially previous slot is not set */
+	prev = NULL;
+	/* Read back bus id and get slot. Check does bus_id != last_slot */
+	bus_id = FIELD_GET(MIO_EMM_SWITCH_BUS_ID, emm_switch);
+	next = host->slot[bus_id];
+	slot_changed = host->last_slot != bus_id;
+	if (host->last_slot != -1)
+		prev = host->slot[host->last_slot];
+
+	/* Prepare to slot switch, switch voltage regulators */
+	if (slot_changed) {
+		pre_switch(prev, next);
+		switch_vqmmc(prev, next);
+	}
 
+	/*
+	 * Modes setting only taken from slot 0. Work around that hardware
+	 * issue by first switching to slot 0.
+	 */
+	if (bus_id) {
+		u64 emm_switch0;
+		/* Clear bus_id and switch through bus_id 0 */
+		emm_switch0 = emm_switch & (~MIO_EMM_SWITCH_BUS_ID);
+		mode_switch(host, emm_switch0);
+	}
+
+	/* Switch with target bus_id */
+	mode_switch(host, emm_switch);
 	check_switch_errors(host);
+
+	if (slot_changed)
+		post_switch(prev, next);
+
+	/* Set current slot id */
+	host->last_slot = bus_id;
 }
 
 static bool switch_val_changed(struct cvm_mmc_slot *slot, u64 new_val)
@@ -972,7 +1102,7 @@ static int cvm_mmc_of_parse(struct device *dev, struct cvm_mmc_slot *slot)
 	 * Legacy Octeon firmware has no regulator entry, fall-back to
 	 * a hard-coded voltage to get a sane OCR.
 	 */
-	if (IS_ERR(mmc->supply.vmmc))
+	if (IS_ERR_OR_NULL(mmc->supply.vmmc))
 		mmc->ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34;
 
 	/* Common MMC bindings */
@@ -1058,6 +1188,8 @@ int cvm_mmc_of_slot_probe(struct device *dev, struct cvm_mmc_host *host)
 
 	host->acquire_bus(host);
 	host->slot[id] = slot;
+	host->use_vqmmc = host->use_vqmmc ||
+			  !IS_ERR_OR_NULL(slot->mmc->supply.vqmmc);
 	cvm_mmc_switch_to(slot);
 	cvm_mmc_init_lowlevel(slot);
 	host->release_bus(host);
diff --git a/drivers/mmc/host/cavium.h b/drivers/mmc/host/cavium.h
index f3eea5eaa678..a581e0462c7e 100644
--- a/drivers/mmc/host/cavium.h
+++ b/drivers/mmc/host/cavium.h
@@ -56,6 +56,7 @@ struct cvm_mmc_host {
 	struct device *dev;
 	void __iomem *base;
 	void __iomem *dma_base;
+
 	int reg_off;
 	int reg_off_dma;
 	u64 emm_cfg;
@@ -70,6 +71,7 @@ struct cvm_mmc_host {
 	bool use_sg;
 
 	bool has_ciu3;
+	bool use_vqmmc;  /* Force disable slot over switch */
 	bool big_dma_addr;
 	bool need_irq_handler_lock;
 	spinlock_t irq_handler_lock;
@@ -206,10 +208,14 @@ struct cvm_mmc_cr_mods {
 #define MIO_EMM_SWITCH_CLK_HI		GENMASK_ULL(31, 16)
 #define MIO_EMM_SWITCH_CLK_LO		GENMASK_ULL(15, 0)
 
+/* Additional defines */
+#define MODE_SWITCH_RETRIES	100
+
 /* Protoypes */
 irqreturn_t cvm_mmc_interrupt(int irq, void *dev_id);
 int cvm_mmc_of_slot_probe(struct device *dev, struct cvm_mmc_host *host);
 int cvm_mmc_of_slot_remove(struct cvm_mmc_slot *slot);
 extern const char *cvm_mmc_irq_names[];
 
-#endif
+#endif /* _CAVIUM_MMC_H_ */
+
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/2] dt-bindings: mmc: Add vmmc/vqmmc for Cavium driver
  2021-11-17 22:27 [PATCH 0/2] mmc: cavium: Fix MMC supply switching for cards Wojciech Bartczak
  2021-11-17 22:28 ` [PATCH 1/2] mmc: cavium: Fix voltage reg. switching for card slots Wojciech Bartczak
@ 2021-11-17 22:28 ` Wojciech Bartczak
  2021-11-23 11:51   ` Ulf Hansson
  1 sibling, 1 reply; 5+ messages in thread
From: Wojciech Bartczak @ 2021-11-17 22:28 UTC (permalink / raw)
  To: linux-mmc
  Cc: ulf.hansson, beanhuo, tanxiaofei, robh+dt, devicetree,
	linux-kernel, wbartczak

Octeon/OcteonTX MMC supports up to 3 card slots.
Each slot can support SD or MMC cards with various speed.
However, any level-shifting to accommodate different signal voltages
for the cards is done by external hardware, under control of an optional
vqmmc regulator object, typically controlled by gpio.
The details of device-tree control of MMC signals via GPIO at reset
is available in this file.

If any mmc-slots have a vqmmc-supply property,
take it as a warning that we must switch carefully between
slots (unless they have the same vqmmc object), tri-stating
MMC signals to avoid any transient states as level-shifters
are enabled/disabled, by zeroing MIO_EMM_CFG[bus_id].

There's no need to list vqmmc property if all the mmc-slots
on a board run at same signal voltage, and have same width.
In this case the old behavior, enabling all probed slots in
MIO_EMM_CFG, allows faster slot-switching.

Signed-off-by: Wojciech Bartczak <wbartczak@marvell.com>
---
 .../devicetree/bindings/mmc/cavium-mmc.txt    | 47 ++++++++++++++++++-
 1 file changed, 45 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/mmc/cavium-mmc.txt b/Documentation/devicetree/bindings/mmc/cavium-mmc.txt
index 1433e6201dff..d0b750e23332 100644
--- a/Documentation/devicetree/bindings/mmc/cavium-mmc.txt
+++ b/Documentation/devicetree/bindings/mmc/cavium-mmc.txt
@@ -28,6 +28,46 @@ Deprecated properties:
 - power-gpios : use vmmc-supply instead
 - cavium,octeon-6130-mmc-slot : use mmc-slot instead
 
+GPIO control via vmmc-supply & vqmmc-supply:
+  Two types of regulator object can be specified as mmc properties,
+  typically regulator-fixed controlled by GPIO pins.
+
+  Octeon/OcteonTX chips commonly use GPIO8 as an MMC-reset pin.
+  In systems which may boot from MMC, it starts as input, and is gently
+  pulled up/down by board logic to indicate the active sense of the
+  signal. Chip reset then drives the signal in the opposite direction
+  to effect a reset of target devices.
+  Device tree should model this with a vmmc-supply regulator, gated by
+  GPIO8, so GPIO8 is driven in the non-reset direction when MMC devices
+  are probed, and held there until rmmod/shutdown/suspend.
+  This allows a warm reboot to reset the MMC devices.
+
+  Octeon/OcteonTX MMC supports up to 3 mmc slots, but any
+  level-shifting to accommodate different signal voltages is
+  done by external hardware, under control of an optional
+  vqmmc regulator object, typically controlled by GPIO.
+
+  If any mmc-slots have a vqmmc-supply property, it is taken as a warning
+  that we must switch carefully between slots (unless they have the same
+  vqmmc object), tri-stating MMC signals to avoid any transient states
+  as level-shifters are enabled/disabled.
+
+  Even when so-called bi-directional level shifters are used,
+  this technique should be employed when using different bus-widths
+  on different slots, disabling level shifters to avoid presenting
+  non-uniform impedance across DATA0-7 & CMD when non-selected
+  4-wide slots are left enabled, while accessing 8-wide targets.
+
+  Note that it's not possible to specify multiple regulators
+  controlled by same GPIO pin, but with different active state.
+  If one GPIO line is require to switch voltage/routing between
+  different mmc-slots, specify a vqmmc-supply on one slot, but
+  not the other. The regulator_disable call on leaving that slot
+  will implicitly switch the state to support the unmarked slot.
+
+  There's no need to list vqmmc-supply if all the mmc-slots on
+  a board run at same voltage, and have same width.
+
 Examples:
 	mmc_1_4: mmc@1,4 {
 		compatible = "cavium,thunder-8390-mmc";
@@ -40,7 +80,8 @@ Examples:
 			compatible = "mmc-slot";
 			reg = <0>;
 			vmmc-supply = <&mmc_supply_3v3>;
-			max-frequency = <42000000>;
+			vqmmc-supply = <&vqmmc_3v3>;
+			max-frequency = <52000000>;
 			bus-width = <4>;
 			cap-sd-highspeed;
 		};
@@ -49,9 +90,11 @@ Examples:
 			compatible = "mmc-slot";
 			reg = <1>;
 			vmmc-supply = <&mmc_supply_3v3>;
-			max-frequency = <42000000>;
+			vqmmc-supply = <&vqmmc_1v8>;
+			max-frequency = <100000000>;
 			bus-width = <8>;
 			cap-mmc-highspeed;
 			non-removable;
 		};
 	};
+
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] mmc: cavium: Fix voltage reg. switching for card slots
  2021-11-17 22:28 ` [PATCH 1/2] mmc: cavium: Fix voltage reg. switching for card slots Wojciech Bartczak
@ 2021-11-19  3:50   ` kernel test robot
  0 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2021-11-19  3:50 UTC (permalink / raw)
  To: Wojciech Bartczak, linux-mmc
  Cc: kbuild-all, ulf.hansson, beanhuo, tanxiaofei, robh+dt,
	devicetree, linux-kernel, wbartczak

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

Hi Wojciech,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on robh/for-next]
[also build test WARNING on linus/master v5.16-rc1 next-20211118]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Wojciech-Bartczak/mmc-cavium-Fix-MMC-supply-switching-for-cards/20211118-063028
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: ia64-buildonly-randconfig-r005-20211119 (attached as .config)
compiler: ia64-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/1a0194dde9d7f7c97c8257103835b04cbaf50f3b
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Wojciech-Bartczak/mmc-cavium-Fix-MMC-supply-switching-for-cards/20211118-063028
        git checkout 1a0194dde9d7f7c97c8257103835b04cbaf50f3b
        # save the attached .config to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=ia64 SHELL=/bin/bash drivers/mmc/host/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/mmc/host/cavium.c:205: warning: expecting prototype for pre_switch(). Prototype was for switch_vqmmc() instead


vim +205 drivers/mmc/host/cavium.c

ba3869ff32e4a6 Jan Glauber       2017-03-30  195  
1a0194dde9d7f7 Wojciech Bartczak 2021-11-17  196  /**
1a0194dde9d7f7 Wojciech Bartczak 2021-11-17  197   * pre_switch - Switch voltage regulators
1a0194dde9d7f7 Wojciech Bartczak 2021-11-17  198   * @prev: Current slot, can be NULL
1a0194dde9d7f7 Wojciech Bartczak 2021-11-17  199   * @next: Next slot
1a0194dde9d7f7 Wojciech Bartczak 2021-11-17  200   *
1a0194dde9d7f7 Wojciech Bartczak 2021-11-17  201   * Switch between regulators used by slots. This call has to be done
1a0194dde9d7f7 Wojciech Bartczak 2021-11-17  202   * after call to pre_switch().
ba3869ff32e4a6 Jan Glauber       2017-03-30  203   */
1a0194dde9d7f7 Wojciech Bartczak 2021-11-17  204  static void switch_vqmmc(struct cvm_mmc_slot *prev, struct cvm_mmc_slot *next)
ba3869ff32e4a6 Jan Glauber       2017-03-30 @205  {
1a0194dde9d7f7 Wojciech Bartczak 2021-11-17  206  	bool same_vqmmc = false;
1a0194dde9d7f7 Wojciech Bartczak 2021-11-17  207  	struct regulator *next_vqmmc;
1a0194dde9d7f7 Wojciech Bartczak 2021-11-17  208  	struct regulator *prev_vqmmc;
1a0194dde9d7f7 Wojciech Bartczak 2021-11-17  209  
1a0194dde9d7f7 Wojciech Bartczak 2021-11-17  210  	next_vqmmc = next->mmc->supply.vqmmc;
1a0194dde9d7f7 Wojciech Bartczak 2021-11-17  211  	if (prev) {
1a0194dde9d7f7 Wojciech Bartczak 2021-11-17  212  		prev_vqmmc = prev->mmc->supply.vqmmc;
1a0194dde9d7f7 Wojciech Bartczak 2021-11-17  213  		/* Prev slot and next slot share vqmmc? */
1a0194dde9d7f7 Wojciech Bartczak 2021-11-17  214  		same_vqmmc = prev_vqmmc == next_vqmmc;
1a0194dde9d7f7 Wojciech Bartczak 2021-11-17  215  		/* Disable old regulator, if not the same */
1a0194dde9d7f7 Wojciech Bartczak 2021-11-17  216  		if (!same_vqmmc && !IS_ERR_OR_NULL(prev_vqmmc))
1a0194dde9d7f7 Wojciech Bartczak 2021-11-17  217  			regulator_disable(prev_vqmmc);
1a0194dde9d7f7 Wojciech Bartczak 2021-11-17  218  	}
ba3869ff32e4a6 Jan Glauber       2017-03-30  219  
1a0194dde9d7f7 Wojciech Bartczak 2021-11-17  220  	/* Enable regulator for next slot */
1a0194dde9d7f7 Wojciech Bartczak 2021-11-17  221  	if (!same_vqmmc && !IS_ERR_OR_NULL(next_vqmmc)) {
1a0194dde9d7f7 Wojciech Bartczak 2021-11-17  222  		int ret;
1a0194dde9d7f7 Wojciech Bartczak 2021-11-17  223  
1a0194dde9d7f7 Wojciech Bartczak 2021-11-17  224  		ret = regulator_enable(next_vqmmc);
1a0194dde9d7f7 Wojciech Bartczak 2021-11-17  225  		if (ret)
1a0194dde9d7f7 Wojciech Bartczak 2021-11-17  226  			dev_err(mmc_classdev(next->mmc),
1a0194dde9d7f7 Wojciech Bartczak 2021-11-17  227  				"Can't enable vqmmc, error (%d)!\n", ret);
1a0194dde9d7f7 Wojciech Bartczak 2021-11-17  228  	}
1a0194dde9d7f7 Wojciech Bartczak 2021-11-17  229  }
1a0194dde9d7f7 Wojciech Bartczak 2021-11-17  230  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 34903 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] dt-bindings: mmc: Add vmmc/vqmmc for Cavium driver
  2021-11-17 22:28 ` [PATCH 2/2] dt-bindings: mmc: Add vmmc/vqmmc for Cavium driver Wojciech Bartczak
@ 2021-11-23 11:51   ` Ulf Hansson
  0 siblings, 0 replies; 5+ messages in thread
From: Ulf Hansson @ 2021-11-23 11:51 UTC (permalink / raw)
  To: Wojciech Bartczak
  Cc: linux-mmc, beanhuo, tanxiaofei, robh+dt, devicetree, linux-kernel

On Wed, 17 Nov 2021 at 23:28, Wojciech Bartczak <wbartczak@marvell.com> wrote:
>
> Octeon/OcteonTX MMC supports up to 3 card slots.
> Each slot can support SD or MMC cards with various speed.
> However, any level-shifting to accommodate different signal voltages
> for the cards is done by external hardware, under control of an optional
> vqmmc regulator object, typically controlled by gpio.
> The details of device-tree control of MMC signals via GPIO at reset
> is available in this file.
>
> If any mmc-slots have a vqmmc-supply property,
> take it as a warning that we must switch carefully between
> slots (unless they have the same vqmmc object), tri-stating
> MMC signals to avoid any transient states as level-shifters
> are enabled/disabled, by zeroing MIO_EMM_CFG[bus_id].
>
> There's no need to list vqmmc property if all the mmc-slots
> on a board run at same signal voltage, and have same width.
> In this case the old behavior, enabling all probed slots in
> MIO_EMM_CFG, allows faster slot-switching.

Overall, I suggest you avoid mixing software behaviour with the HW
description. The DT bindings should describe the way the HW works, not
the software.

This applies further down to the description of the new properties as
well. But please, don't get me wrong, I appreciate the detailed
explanations, just make sure it's more focused on describing the HW.

>
> Signed-off-by: Wojciech Bartczak <wbartczak@marvell.com>
> ---
>  .../devicetree/bindings/mmc/cavium-mmc.txt    | 47 ++++++++++++++++++-
>  1 file changed, 45 insertions(+), 2 deletions(-)

We have moved to describe DT bindings in the yaml format. I would
appreciate it if you make that conversion and then extend it with the
new things, rather than continue to extend the txt based one.

Kind regards
Uffe

>
> diff --git a/Documentation/devicetree/bindings/mmc/cavium-mmc.txt b/Documentation/devicetree/bindings/mmc/cavium-mmc.txt
> index 1433e6201dff..d0b750e23332 100644
> --- a/Documentation/devicetree/bindings/mmc/cavium-mmc.txt
> +++ b/Documentation/devicetree/bindings/mmc/cavium-mmc.txt
> @@ -28,6 +28,46 @@ Deprecated properties:
>  - power-gpios : use vmmc-supply instead
>  - cavium,octeon-6130-mmc-slot : use mmc-slot instead
>
> +GPIO control via vmmc-supply & vqmmc-supply:
> +  Two types of regulator object can be specified as mmc properties,
> +  typically regulator-fixed controlled by GPIO pins.
> +
> +  Octeon/OcteonTX chips commonly use GPIO8 as an MMC-reset pin.
> +  In systems which may boot from MMC, it starts as input, and is gently
> +  pulled up/down by board logic to indicate the active sense of the
> +  signal. Chip reset then drives the signal in the opposite direction
> +  to effect a reset of target devices.
> +  Device tree should model this with a vmmc-supply regulator, gated by
> +  GPIO8, so GPIO8 is driven in the non-reset direction when MMC devices
> +  are probed, and held there until rmmod/shutdown/suspend.
> +  This allows a warm reboot to reset the MMC devices.
> +
> +  Octeon/OcteonTX MMC supports up to 3 mmc slots, but any
> +  level-shifting to accommodate different signal voltages is
> +  done by external hardware, under control of an optional
> +  vqmmc regulator object, typically controlled by GPIO.
> +
> +  If any mmc-slots have a vqmmc-supply property, it is taken as a warning
> +  that we must switch carefully between slots (unless they have the same
> +  vqmmc object), tri-stating MMC signals to avoid any transient states
> +  as level-shifters are enabled/disabled.
> +
> +  Even when so-called bi-directional level shifters are used,
> +  this technique should be employed when using different bus-widths
> +  on different slots, disabling level shifters to avoid presenting
> +  non-uniform impedance across DATA0-7 & CMD when non-selected
> +  4-wide slots are left enabled, while accessing 8-wide targets.
> +
> +  Note that it's not possible to specify multiple regulators
> +  controlled by same GPIO pin, but with different active state.
> +  If one GPIO line is require to switch voltage/routing between
> +  different mmc-slots, specify a vqmmc-supply on one slot, but
> +  not the other. The regulator_disable call on leaving that slot
> +  will implicitly switch the state to support the unmarked slot.
> +
> +  There's no need to list vqmmc-supply if all the mmc-slots on
> +  a board run at same voltage, and have same width.
> +
>  Examples:
>         mmc_1_4: mmc@1,4 {
>                 compatible = "cavium,thunder-8390-mmc";
> @@ -40,7 +80,8 @@ Examples:
>                         compatible = "mmc-slot";
>                         reg = <0>;
>                         vmmc-supply = <&mmc_supply_3v3>;
> -                       max-frequency = <42000000>;
> +                       vqmmc-supply = <&vqmmc_3v3>;
> +                       max-frequency = <52000000>;
>                         bus-width = <4>;
>                         cap-sd-highspeed;
>                 };
> @@ -49,9 +90,11 @@ Examples:
>                         compatible = "mmc-slot";
>                         reg = <1>;
>                         vmmc-supply = <&mmc_supply_3v3>;
> -                       max-frequency = <42000000>;
> +                       vqmmc-supply = <&vqmmc_1v8>;
> +                       max-frequency = <100000000>;
>                         bus-width = <8>;
>                         cap-mmc-highspeed;
>                         non-removable;
>                 };
>         };
> +
> --
> 2.17.1
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-11-23 11:52 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-17 22:27 [PATCH 0/2] mmc: cavium: Fix MMC supply switching for cards Wojciech Bartczak
2021-11-17 22:28 ` [PATCH 1/2] mmc: cavium: Fix voltage reg. switching for card slots Wojciech Bartczak
2021-11-19  3:50   ` kernel test robot
2021-11-17 22:28 ` [PATCH 2/2] dt-bindings: mmc: Add vmmc/vqmmc for Cavium driver Wojciech Bartczak
2021-11-23 11:51   ` Ulf Hansson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).