[Letux-kernel] [PATCH] arm: boot: dts: lc15: hardcode txs02612 mux to eMMC

Nick Elsmore nicholaselsmore at gmail.com
Tue Aug 11 20:38:36 CEST 2020

Hi Niklaus,

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.

The current implementation actually only works safely if there is no device
present when the root host is probed.  This way the device is not
registered as a card on the root host and is only registered on the new
host which gets added by the txs02612 driver.  IMHO this is dangerous.

> 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.
Definitely agree here.  This method is not good, particularly for trying to
get submitted into mainline.  This was mainly a discussion of a method
which may work with the current mmc architecture, but it poses its own
issues as you mentioned.

> 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.
Yes this would need to be added to the mmc host code.  We would need to
iterate through the devices registered as mmc_host_class.  The pointer to
mmc_host_class is not accessible outside of the host subsystem so the API
would need to be extended to allow this.

> 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...
Agreed.  I think we will have a difficult time getting support here into
mainline as it will require some significant changes to the mmc host
architecture.  Perhaps this could be done simply by adding a new dts
property to the standard mmc host node like you suggested, such as
"mmc-host-port" or "mmc-host-muxed" it just probes the child node (mux
driver) and passes its ops to the mux driver rather than performing card
initialization directly.  However, mainline maintainers would likely prefer
that both mmc cards present on the downstream side of the mux show up as
child devices of the root host rather than on new hosts which aren't
properly represented in the class tree.

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.
The issue I am having with it is it makes it particularly difficult to
manage the boot process when trying to use a rootfs on the eMMC (or uSD).
Basically when booting from either the eMMC or the uSD, the bootloader and
the kernel need to align.  If we use a script in the initramfs to manage
this, it would need to detect the boot device selected by the bootloader
(which reflects the boot device selected by the user using the shoulder
button on the Pyra).  Many gpio drivers do not properly expose pin
direction and value from userspace until they are set from userspace when
accessed via sysfs.  In this case, a script cannot reliably get the state
of the pin as set by the bootloader from userspace to determine the correct
device to select after initialization of all downstream devices.  Granted I
have not verified whether or not this works properly on the omap5 driver, I
should and will.

Overall, this should be handled by the kernel.  I tried to do a small
modification to the txs02612 driver to grab the gpio without modifying it
using the GPIOD_ASIS flag rather than GPIOD_OUT_HIGH and reading the state
as set by U-Boot.  After initialization of the new hosts (for eMMC and
uSD), I tried setting it back to the value chosen by the bootloader as this
reflects the boot device selected by the user.  This does not work properly
since the initialization procedure of the slots, when mmc_add_host is
called, is handled asynchronously so I cannot properly synchronize
selection of the correct device after allocation (the request op is also
switching the gpio during the mmc_add_host procedure).

I am coming at this from a standpoint of trying to get the Pyra to a
release ready state.  This involves getting the software to a point where
it works in an intuitive manner to an end user.  A very common use-case for
the Pyra (and the state in which it will ship) is to boot off the eMMC.
The current management of the mmc mux makes this difficult to configure and
involves synchronization of the boot selection in the bootloader,
initramfs, and kernel which is quite complicated and makes modification of
this difficult.  It seems sensible to me to limit functionality to the eMMC
for now until a safe method which is properly managed by the kernel can be
merged in.

Another option here is to disable all control of the selection gpio in the
kernel and initramfs so the pin remains in the state set by the
bootloader.  This way, the eMMC/uSD selection is determined at boot time
and isn't changed at runtime.

I do recognize that the change proposed modifies the device tree structure
in omap5-letux-cortex15-common.dtsi which may impact devices other than the
Pyra.  If this is the case, it would make sense to move these changes into
a Pyra-specific device tree to leave the device tree of other devices using
this include unaffected.

Letux-kernel mailing list
Letux-kernel at openphoenux.org
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.goldelico.com/pipermail/letux-kernel/attachments/20200811/83d0fa88/attachment.htm>

More information about the Letux-kernel mailing list