Re: [PATCH 2/2] mmc: dw_mmc: add resets support to dw_mmc

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




在 2016/3/29 16:23, zhangfei 写道:


On 03/29/2016 10:22 AM, Shawn Lin wrote:




        +               else if (IS_ERR(host->pdata)) {
                                 dev_err(host->dev, "platform data not
        available\n");
                                 return -EINVAL;
                         }
        @@ -3012,6 +3022,9 @@ int dw_mci_probe(struct dw_mci *host)
                         }
                 }

        +       if (host->pdata->rstc != NULL)
        +               reset_control_deassert(host->pdata->rstc);
        +


    sorry, I can't follow your intention here. Shouldn't it be something
    like "assert mmc -> may need delay -> deassert mmc". As your current
    code, nothing happend right?
Should be abstracted in reset driver.



The chip exits from bootloader with this bit asserted. And when entering
kernel, we only need to deassert.

In my current code, the driver deassert mmc in _probe(), and assert mmc
in _remove().

I catch your point. From the previous discussion, we add it to make sure
dw_mmc in good state after leaving bootloader to kernel. But My real
question is that you can assert it in  bootloader, so you can also
dessert it in bootloaer to make sure dw_mmc work fine when probing
in kernel. In that way, we don't need this patch?

uefi does not have exit point, and kernel may not assume mmc controller
state is always correct when boot.
If Uefi need copy Image from mmc, mmc controller is in working state.
When jump to kernel, uefi mmc driver can not recover itself.
If kernel assume mmc controller state is clean, mmc will be in abnormal
state.
Some controller will clear itself when set clock, however, hip660 does
not, it need special register to access.


yep, I understand your requirement.



More to think, Is it ok to match the behaviour of bootloader stage?
My bootloader doesn't assert the reset pin of dw_mmc, so it seams if
I want to fix you issue on kernel stage, I need a new round of
assert->delay->deassert.

The process like delay (if required) should be abstracted in reset driver.
reset framework just export reset_control_assert/reset_control_deassert
API.
Unfortunately not find clear description in Documentation/.
Suppose deassert is like start, while assert is like stop.


yes, no docs except dt-bindings..... So seems the usage of these two
APIs depends on the implementation of reset controller driver

drivers/reset/core.c
reset_control_deassert - deasserts the reset line
reset_control_assert - asserts the reset line

More example:
drivers/mmc/host/sdhci-st.c
drivers/mmc/host/sunxi-mmc.c
drivers/usb/host/ohci-platform.c
drivers/i2c/busses/i2c-mv64xxx.c

But I'm afraid I have to nack here....

Looking at these files:
drivers/dma/stm32-dma.c
drivers/net/ethernet/mediatek/mtk_eth_soc.c
drivers/devfreq/tegra-devfreq.c
drivers/crypto/rockchip/rk3288_crypto.c
drivers/thermal/rockchip_thermal.c
drivers/thermal/tegra_soctherm.c
drivers/i2c/busses/i2c-tegra.c
....

they just do the way like: assert->[delay](maybe abstracted)->deassert
I think these drivers are vendor specific, so they can do
the reset operations in consistent with the implementation of
their platforms' reset controller drivers.

But, dw_mmc is shared by many Socs. So It's not good to do it in
dw_mci_probe, otherwise you force my reset controller driver to be
implemented in the same way as yours..... Right?

How about move it to your drv_data->init callback?



Thanks






--
Best Regards
Shawn Lin

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux