Hi Nick,

Am 10.08.2020 um 21:34 schrieb Nick Elsmore <nicholaselsmore@gmail.com>:

There are some pretty serious issues regarding the current hardware strategy of the txs02612 mmc mux and compatibility with the Linux mmc stack.  I would first like to propose disabling its functionality with the following patch and supporting just the eMMC until a viable method of handling it can be found.  The reasoning is as follows.

The mmc subsystem does not directly support multiplexing like other subsystems do (ie i2c). 

Yes, there is no mux driver available yet. It has to be written.

The current strategy involves intercepting mmc request operations to the cards on the downstream side of the mux and switching the mux before calling the parent host's request op. 

Ideally the tx02612 device node would be a subnode of the root mmc host node.

Yes, that is how the hardware is (and must be).

  However, the mmc host driver does not directly probe child nodes without first performing a handshake on the bus.  Therefore the subdriver would only be probed if a valid device was connected in the default state of the mux at the time the root host is probed.  Therefore, in order to guarantee the txs02612 driver is probed, the bootloader and kernel would need to ensure that a valid device is connected on the SDIO lines to ensure the mux driver came up.  This should work as the tx02612 driver would be registered as a card on the root host and the tx02612 driver would allocate and register two new hosts for the eMMC and uSD which would properly go through the SDIO/SD/MMC initialization procedures when they are registered. 

Yes, this is the idea.

There also may be an issue if the default card used for detection goes through the full probe procedure and is registered on the root host AND the new host.  However, since the driver even being probed in the first place relies on the default state of the GPIO set by the bootloader, this implementation seems shaky to me.

Maybe there must be a new flag to prevent the code to go this way. I.e. if the mmc2 host is flagged as "mux-host-port" it should not do all the steps mentioned above but initialize the mux driver. And the mux driver must ensure that both ports being probed.


Another option is to register the tx02612 driver as a platform driver and link in the device_node in the dts in a method similar to the one below.  This way we can be sure the driver is always probed, regardless of the mmc handshake.

txs02612 {
     compatible = "ti,txs02612";
     select-gpio = <&gpio3 18 0>;    /* gpio3_82 soft-select */
     host = <&mmc2>;
     status = "disabled";
 };

Well, this is not the design patterns DT maintainers want to see. I had similar discussions with them e.g. for SGX or GPS. For GPS on the GTA04 we simply had a driver that referenced the UART where the GPS chip was connected to. Was working fine for 5 years - outside of mainline. But DTS maintainers demanded that the idea is that the processor is sitting like a spider in the net (or like an octopus)and going outwards is represented by child nodes. I.e. each interface is a "bus". Even an UART is a "bus", although you can only connect one client. And if a device is indirectly connected through some subsystem, the subsystem has to be a child node of the processor and the device a child of the subsystem.

Therefore I am quite sure that your proposal will never be accepted mainline (but we should ask Rob Herring).

Generally I have seen the tendency that drivers for new devices are first discussed from DTS perspective. So the first thing which is needed is a .yaml bindings document, then some DTS. And finally the driver code has to be adapted to make use of what it gets. It is no longer possible to fit the DTS to a quick and simple driver implementation.


With this sort of link, the mmc device_node can be grabbed using of_parse_phandle.  However, getting the device and then mmc_host from this device_node is not easy since the mmc subsystem doesn't currently provide a method of accessing host nodes from their of_node by iterating through the class devices like some other subsystems provide (ie gpio, fpga-mgr).  So far, I have not found a viable method to access the linked node as we don't have access to the correct resources within the mmc subsystem.  This method also poses the same issue exhibted by the first method, where the card may be registered on both the root host and the newly registered host.

It is possible by adding some list and registration code. There is a similar mechanism for referencing PHY devices.


The current implementation allows for manual switching between the eMMC and uSD from userspace using a sysfs hook.  This works, but I worry about its behavior when interfacing with the mmc subsystem.

It is a simple hack to make use of the DTS (as I think it must be).

  The subsystem is not aware of the switching and so this method will likely pose issues with regard to filesystem cache sync and similar. 

I have a simple switch script that does an eject, then switch and finally partprobe. So it should be "safe" - unless it is switched by accessing the gpio directly.
Of course this scheme does not allow to use both channels in parallel like the ideal solution would do.

The host is using polling to handle card detect, and so if the card disappears due to unsynchronized userland switching, it may view this as a card disconnect, even though the card is still powered.  Because of this, a number of risks are possible including improper handling of the card itself (potentially causing damage), improper or missed filesystem cache synchronization, and data corruption.

It is not much different than extracting an SD card from an SD reader without card detect switch, while it is mounted.

Fortunately for the Pyra the µSD card can't be extracted while the Pyra is powered (well it may be possible but is very unlikely) and the eMMC isn't swappable at all.


Until I or someone else can find a safe and (hopefully) graceful solution to this issue,

IMHO it is complex and goes deeper into the MMC code. A big task for a small function and device...

I am proposing to simply disable the tx02612 driver and hardcode the mux to the eMMC for now with the following patch.

Hm. Why do you want to disable it completely? For me the switching script works as it should. It may be possible to have it part of the initrd so that a boot choice screen can allow the user to select to boot from eMMC or µSD. If we ever have a better txs02612 driver we can replace this.

BR and thanks,
Nikolaus



Signed-off-by: Nick Elsmore <nicholaselsmore@gmail.com>
---
index d59d9fd4c8af..60822100bcdf 100644
--- a/arch/arm/boot/dts/omap5-letux-cortex15-common.dtsi
+++ b/arch/arm/boot/dts/omap5-letux-cortex15-common.dtsi
@@ -248,6 +248,7 @@ mmcmux: txs02612@0 {
 
 		#adress-cells = <1>;
 		#size-cells = <0>;
+		status = "disabled";
 
 		eMMC: port@0 {
 			reg = <0>;
@@ -268,9 +269,25 @@ / { /* eMMC / uSD-card */
 	txs02612 {
 		compatible = "ti,txs02612";
 		select-gpio = <&gpio3 18 0>;	/* gpio3_82 soft-select */
+		status = "disabled";
 	};
 };
 
+/* hardcode txs02612 mmc mux to eMMC input for now */
+&gpio3 {
+	txs02612_soft_control {
+		gpio-hog;
+		gpios = <18 GPIO_ACTIVE_HIGH>;
+		output-low;
+		label = "txs02612-soft-control";
+	};
+};
+
+&mmc3 { /* WiFi-SDIO */
+ /* has been predefined for a wl1271 in ompa5-common.dtsi */
+};
+
+
 &mmc3 { /* WiFi-SDIO */
 	/* has been predefined for a wl1271 in ompa5-common.dtsi */
 };
_______________________________________________
https://projects.goldelico.com/p/gta04-kernel/
Letux-kernel mailing list
Letux-kernel@openphoenux.org
http://lists.goldelico.com/mailman/listinfo.cgi/letux-kernel