All of lore.kernel.org
 help / color / mirror / Atom feed
* A3720 - Disable slot when eMMC is not present
@ 2020-12-08 10:08 ` Pali Rohár
  2020-12-09 10:01   ` Jaehoon Chung
  2020-12-14 12:38   ` Andre Heider
  0 siblings, 2 replies; 6+ messages in thread
From: Pali Rohár @ 2020-12-08 10:08 UTC (permalink / raw)
  To: u-boot

Hello! I looked at what is initialized and enabled for sd/emmc slots and
I found out that comphy mmc needs to be enabled if at least one slot is
used (e.g. SD card) and then remaining part is slot initialization in
xenon driver.

I wrote quick patch to disable slot when unregistering mmc device from
U-Boot and also unregister emmc on A3720 from U-Boot when it is not
present. But I have not tested it yet.

What do you think about it?

diff --git a/board/Marvell/mvebu_armada-37xx/board.c b/board/Marvell/mvebu_armada-37xx/board.c
index f67b04b78c..1b9e7520cc 100644
--- a/board/Marvell/mvebu_armada-37xx/board.c
+++ b/board/Marvell/mvebu_armada-37xx/board.c
@@ -5,6 +5,7 @@
 
 #include <common.h>
 #include <dm.h>
+#include <dm/device-internal.h>
 #include <env.h>
 #include <i2c.h>
 #include <init.h>
@@ -84,12 +85,10 @@ int board_init(void)
 #ifdef CONFIG_BOARD_LATE_INIT
 int board_late_init(void)
 {
+	struct udevice *dev;
 	struct mmc *mmc_dev;
 	bool ddr4, emmc;
 
-	if (env_get("fdtfile"))
-		return 0;
-
 	if (!of_machine_is_compatible("globalscale,espressobin"))
 		return 0;
 
@@ -101,6 +100,16 @@ int board_late_init(void)
 	mmc_dev = find_mmc_device(1);
 	emmc = (mmc_dev && mmc_init(mmc_dev) == 0);
 
+	/* if eMMC is not present then remove it from DM */
+	if (!emmc && mmc_dev) {
+		dev = mmc_dev->dev;
+		device_remove(dev, DM_REMOVE_NORMAL);
+		device_unbind(dev);
+	}
+
+	if (env_get("fdtfile"))
+		return 0;
+
 	if (ddr4 && emmc)
 		env_set("fdtfile", "marvell/armada-3720-espressobin-v7-emmc.dtb");
 	else if (ddr4)
diff --git a/drivers/mmc/xenon_sdhci.c b/drivers/mmc/xenon_sdhci.c
index 6ce9d00d0a..da9882cbf6 100644
--- a/drivers/mmc/xenon_sdhci.c
+++ b/drivers/mmc/xenon_sdhci.c
@@ -350,6 +350,16 @@ static void xenon_mmc_enable_slot(struct sdhci_host *host, u8 slot)
 	sdhci_writel(host, var, SDHC_SYS_OP_CTRL);
 }
 
+/* Disable specific slot */
+static void xenon_mmc_disable_slot(struct sdhci_host *host, u8 slot)
+{
+	u32 var;
+
+	var = sdhci_readl(host, SDHC_SYS_OP_CTRL);
+	var &= ~(SLOT_MASK(slot) << SLOT_ENABLE_SHIFT);
+	sdhci_writel(host, var, SDHC_SYS_OP_CTRL);
+}
+
 /* Enable Parallel Transfer Mode */
 static void xenon_mmc_enable_parallel_tran(struct sdhci_host *host, u8 slot)
 {
@@ -515,6 +525,14 @@ static int xenon_sdhci_probe(struct udevice *dev)
 	return ret;
 }
 
+static int xenon_sdhci_remove(struct udevice *dev)
+{
+	struct sdhci_host *host = dev_get_priv(dev);
+
+	xenon_mmc_disable_slot(host, XENON_MMC_SLOT_ID_HYPERION);
+	return 0;
+}
+
 static int xenon_sdhci_ofdata_to_platdata(struct udevice *dev)
 {
 	struct sdhci_host *host = dev_get_priv(dev);
@@ -564,6 +582,7 @@ U_BOOT_DRIVER(xenon_sdhci_drv) = {
 	.ops		= &sdhci_ops,
 	.bind		= xenon_sdhci_bind,
 	.probe		= xenon_sdhci_probe,
+	.remove		= xenon_sdhci_remove,
 	.priv_auto_alloc_size = sizeof(struct xenon_sdhci_priv),
 	.platdata_auto_alloc_size = sizeof(struct xenon_sdhci_plat),
 };

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* A3720 - Disable slot when eMMC is not present
  2020-12-08 10:08 ` A3720 - Disable slot when eMMC is not present Pali Rohár
@ 2020-12-09 10:01   ` Jaehoon Chung
  2020-12-09 10:25     ` Pali Rohár
  2020-12-14 12:38   ` Andre Heider
  1 sibling, 1 reply; 6+ messages in thread
From: Jaehoon Chung @ 2020-12-09 10:01 UTC (permalink / raw)
  To: u-boot

Hi,

On 12/8/20 7:08 PM, Pali Roh?r wrote:
> Hello! I looked at what is initialized and enabled for sd/emmc slots and
> I found out that comphy mmc needs to be enabled if at least one slot is
> used (e.g. SD card) and then remaining part is slot initialization in
> xenon driver.
> 
> I wrote quick patch to disable slot when unregistering mmc device from
> U-Boot and also unregister emmc on A3720 from U-Boot when it is not
> present. But I have not tested it yet.
> > What do you think about it?

Logically, it's possible to disable slot. But i'm not sure because of not having its board.
Just minor question.. Does it support multi slot per one IP?

Best Regards,
Jaehoon Chung

> 
> diff --git a/board/Marvell/mvebu_armada-37xx/board.c b/board/Marvell/mvebu_armada-37xx/board.c
> index f67b04b78c..1b9e7520cc 100644
> --- a/board/Marvell/mvebu_armada-37xx/board.c
> +++ b/board/Marvell/mvebu_armada-37xx/board.c
> @@ -5,6 +5,7 @@
>  
>  #include <common.h>
>  #include <dm.h>
> +#include <dm/device-internal.h>
>  #include <env.h>
>  #include <i2c.h>
>  #include <init.h>
> @@ -84,12 +85,10 @@ int board_init(void)
>  #ifdef CONFIG_BOARD_LATE_INIT
>  int board_late_init(void)
>  {
> +	struct udevice *dev;
>  	struct mmc *mmc_dev;
>  	bool ddr4, emmc;
>  
> -	if (env_get("fdtfile"))
> -		return 0;
> -
>  	if (!of_machine_is_compatible("globalscale,espressobin"))
>  		return 0;
>  
> @@ -101,6 +100,16 @@ int board_late_init(void)
>  	mmc_dev = find_mmc_device(1);
>  	emmc = (mmc_dev && mmc_init(mmc_dev) == 0);
>  
> +	/* if eMMC is not present then remove it from DM */
> +	if (!emmc && mmc_dev) {
> +		dev = mmc_dev->dev;
> +		device_remove(dev, DM_REMOVE_NORMAL);
> +		device_unbind(dev);
> +	}
> +
> +	if (env_get("fdtfile"))
> +		return 0;
> +
>  	if (ddr4 && emmc)
>  		env_set("fdtfile", "marvell/armada-3720-espressobin-v7-emmc.dtb");
>  	else if (ddr4)
> diff --git a/drivers/mmc/xenon_sdhci.c b/drivers/mmc/xenon_sdhci.c
> index 6ce9d00d0a..da9882cbf6 100644
> --- a/drivers/mmc/xenon_sdhci.c
> +++ b/drivers/mmc/xenon_sdhci.c
> @@ -350,6 +350,16 @@ static void xenon_mmc_enable_slot(struct sdhci_host *host, u8 slot)
>  	sdhci_writel(host, var, SDHC_SYS_OP_CTRL);
>  }
>  
> +/* Disable specific slot */
> +static void xenon_mmc_disable_slot(struct sdhci_host *host, u8 slot)
> +{
> +	u32 var;
> +
> +	var = sdhci_readl(host, SDHC_SYS_OP_CTRL);
> +	var &= ~(SLOT_MASK(slot) << SLOT_ENABLE_SHIFT);
> +	sdhci_writel(host, var, SDHC_SYS_OP_CTRL);
> +}
> +
>  /* Enable Parallel Transfer Mode */
>  static void xenon_mmc_enable_parallel_tran(struct sdhci_host *host, u8 slot)
>  {
> @@ -515,6 +525,14 @@ static int xenon_sdhci_probe(struct udevice *dev)
>  	return ret;
>  }
>  
> +static int xenon_sdhci_remove(struct udevice *dev)
> +{
> +	struct sdhci_host *host = dev_get_priv(dev);
> +
> +	xenon_mmc_disable_slot(host, XENON_MMC_SLOT_ID_HYPERION);
> +	return 0;
> +}
> +
>  static int xenon_sdhci_ofdata_to_platdata(struct udevice *dev)
>  {
>  	struct sdhci_host *host = dev_get_priv(dev);
> @@ -564,6 +582,7 @@ U_BOOT_DRIVER(xenon_sdhci_drv) = {
>  	.ops		= &sdhci_ops,
>  	.bind		= xenon_sdhci_bind,
>  	.probe		= xenon_sdhci_probe,
> +	.remove		= xenon_sdhci_remove,
>  	.priv_auto_alloc_size = sizeof(struct xenon_sdhci_priv),
>  	.platdata_auto_alloc_size = sizeof(struct xenon_sdhci_plat),
>  };
> 
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread
* A3720 - Disable slot when eMMC is not present
  2020-12-09 10:01   ` Jaehoon Chung
@ 2020-12-09 10:25     ` Pali Rohár
  2020-12-09 21:31       ` Jaehoon Chung
  0 siblings, 1 reply; 6+ messages in thread
From: Pali Rohár @ 2020-12-09 10:25 UTC (permalink / raw)
  To: u-boot

On Wednesday 09 December 2020 19:01:19 Jaehoon Chung wrote:
> Hi,
> 
> On 12/8/20 7:08 PM, Pali Roh?r wrote:
> > Hello! I looked at what is initialized and enabled for sd/emmc slots and
> > I found out that comphy mmc needs to be enabled if at least one slot is
> > used (e.g. SD card) and then remaining part is slot initialization in
> > xenon driver.
> > 
> > I wrote quick patch to disable slot when unregistering mmc device from
> > U-Boot and also unregister emmc on A3720 from U-Boot when it is not
> > present. But I have not tested it yet.
> > > What do you think about it?
> 
> Logically, it's possible to disable slot. But i'm not sure because of not having its board.
> Just minor question.. Does it support multi slot per one IP?

A3720 has only one slot with index XENON_MMC_SLOT_ID_HYPERION.

A3720 has two mmc/sd IPs at two different address spaces, so uSD and
eMMC do not share config address space.

> Best Regards,
> Jaehoon Chung
> 
> > 
> > diff --git a/board/Marvell/mvebu_armada-37xx/board.c b/board/Marvell/mvebu_armada-37xx/board.c
> > index f67b04b78c..1b9e7520cc 100644
> > --- a/board/Marvell/mvebu_armada-37xx/board.c
> > +++ b/board/Marvell/mvebu_armada-37xx/board.c
> > @@ -5,6 +5,7 @@
> >  
> >  #include <common.h>
> >  #include <dm.h>
> > +#include <dm/device-internal.h>
> >  #include <env.h>
> >  #include <i2c.h>
> >  #include <init.h>
> > @@ -84,12 +85,10 @@ int board_init(void)
> >  #ifdef CONFIG_BOARD_LATE_INIT
> >  int board_late_init(void)
> >  {
> > +	struct udevice *dev;
> >  	struct mmc *mmc_dev;
> >  	bool ddr4, emmc;
> >  
> > -	if (env_get("fdtfile"))
> > -		return 0;
> > -
> >  	if (!of_machine_is_compatible("globalscale,espressobin"))
> >  		return 0;
> >  
> > @@ -101,6 +100,16 @@ int board_late_init(void)
> >  	mmc_dev = find_mmc_device(1);
> >  	emmc = (mmc_dev && mmc_init(mmc_dev) == 0);
> >  
> > +	/* if eMMC is not present then remove it from DM */
> > +	if (!emmc && mmc_dev) {
> > +		dev = mmc_dev->dev;
> > +		device_remove(dev, DM_REMOVE_NORMAL);
> > +		device_unbind(dev);
> > +	}
> > +
> > +	if (env_get("fdtfile"))
> > +		return 0;
> > +
> >  	if (ddr4 && emmc)
> >  		env_set("fdtfile", "marvell/armada-3720-espressobin-v7-emmc.dtb");
> >  	else if (ddr4)
> > diff --git a/drivers/mmc/xenon_sdhci.c b/drivers/mmc/xenon_sdhci.c
> > index 6ce9d00d0a..da9882cbf6 100644
> > --- a/drivers/mmc/xenon_sdhci.c
> > +++ b/drivers/mmc/xenon_sdhci.c
> > @@ -350,6 +350,16 @@ static void xenon_mmc_enable_slot(struct sdhci_host *host, u8 slot)
> >  	sdhci_writel(host, var, SDHC_SYS_OP_CTRL);
> >  }
> >  
> > +/* Disable specific slot */
> > +static void xenon_mmc_disable_slot(struct sdhci_host *host, u8 slot)
> > +{
> > +	u32 var;
> > +
> > +	var = sdhci_readl(host, SDHC_SYS_OP_CTRL);
> > +	var &= ~(SLOT_MASK(slot) << SLOT_ENABLE_SHIFT);
> > +	sdhci_writel(host, var, SDHC_SYS_OP_CTRL);
> > +}
> > +
> >  /* Enable Parallel Transfer Mode */
> >  static void xenon_mmc_enable_parallel_tran(struct sdhci_host *host, u8 slot)
> >  {
> > @@ -515,6 +525,14 @@ static int xenon_sdhci_probe(struct udevice *dev)
> >  	return ret;
> >  }
> >  
> > +static int xenon_sdhci_remove(struct udevice *dev)
> > +{
> > +	struct sdhci_host *host = dev_get_priv(dev);
> > +
> > +	xenon_mmc_disable_slot(host, XENON_MMC_SLOT_ID_HYPERION);
> > +	return 0;
> > +}
> > +
> >  static int xenon_sdhci_ofdata_to_platdata(struct udevice *dev)
> >  {
> >  	struct sdhci_host *host = dev_get_priv(dev);
> > @@ -564,6 +582,7 @@ U_BOOT_DRIVER(xenon_sdhci_drv) = {
> >  	.ops		= &sdhci_ops,
> >  	.bind		= xenon_sdhci_bind,
> >  	.probe		= xenon_sdhci_probe,
> > +	.remove		= xenon_sdhci_remove,
> >  	.priv_auto_alloc_size = sizeof(struct xenon_sdhci_priv),
> >  	.platdata_auto_alloc_size = sizeof(struct xenon_sdhci_plat),
> >  };
> > 
> > 
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread
* A3720 - Disable slot when eMMC is not present
  2020-12-09 10:25     ` Pali Rohár
@ 2020-12-09 21:31       ` Jaehoon Chung
  0 siblings, 0 replies; 6+ messages in thread
From: Jaehoon Chung @ 2020-12-09 21:31 UTC (permalink / raw)
  To: u-boot

On 12/9/20 7:25 PM, Pali Roh?r wrote:
> On Wednesday 09 December 2020 19:01:19 Jaehoon Chung wrote:
>> Hi,
>>
>> On 12/8/20 7:08 PM, Pali Roh?r wrote:
>>> Hello! I looked at what is initialized and enabled for sd/emmc slots and
>>> I found out that comphy mmc needs to be enabled if at least one slot is
>>> used (e.g. SD card) and then remaining part is slot initialization in
>>> xenon driver.
>>>
>>> I wrote quick patch to disable slot when unregistering mmc device from
>>> U-Boot and also unregister emmc on A3720 from U-Boot when it is not
>>> present. But I have not tested it yet.
>>>> What do you think about it?
>>
>> Logically, it's possible to disable slot. But i'm not sure because of not having its board.
>> Just minor question.. Does it support multi slot per one IP?
> 
> A3720 has only one slot with index XENON_MMC_SLOT_ID_HYPERION.
> 
> A3720 has two mmc/sd IPs at two different address spaces, so uSD and
> eMMC do not share config address space.

Thanks for explanation kindly. I asked because didn't see a real case that multi slot is supported. 

Best Regards,
Jaehoon Chung

> 
>> Best Regards,
>> Jaehoon Chung
>>
>>>
>>> diff --git a/board/Marvell/mvebu_armada-37xx/board.c b/board/Marvell/mvebu_armada-37xx/board.c
>>> index f67b04b78c..1b9e7520cc 100644
>>> --- a/board/Marvell/mvebu_armada-37xx/board.c
>>> +++ b/board/Marvell/mvebu_armada-37xx/board.c
>>> @@ -5,6 +5,7 @@
>>>  
>>>  #include <common.h>
>>>  #include <dm.h>
>>> +#include <dm/device-internal.h>
>>>  #include <env.h>
>>>  #include <i2c.h>
>>>  #include <init.h>
>>> @@ -84,12 +85,10 @@ int board_init(void)
>>>  #ifdef CONFIG_BOARD_LATE_INIT
>>>  int board_late_init(void)
>>>  {
>>> +	struct udevice *dev;
>>>  	struct mmc *mmc_dev;
>>>  	bool ddr4, emmc;
>>>  
>>> -	if (env_get("fdtfile"))
>>> -		return 0;
>>> -
>>>  	if (!of_machine_is_compatible("globalscale,espressobin"))
>>>  		return 0;
>>>  
>>> @@ -101,6 +100,16 @@ int board_late_init(void)
>>>  	mmc_dev = find_mmc_device(1);
>>>  	emmc = (mmc_dev && mmc_init(mmc_dev) == 0);
>>>  
>>> +	/* if eMMC is not present then remove it from DM */
>>> +	if (!emmc && mmc_dev) {
>>> +		dev = mmc_dev->dev;
>>> +		device_remove(dev, DM_REMOVE_NORMAL);
>>> +		device_unbind(dev);
>>> +	}
>>> +
>>> +	if (env_get("fdtfile"))
>>> +		return 0;
>>> +
>>>  	if (ddr4 && emmc)
>>>  		env_set("fdtfile", "marvell/armada-3720-espressobin-v7-emmc.dtb");
>>>  	else if (ddr4)
>>> diff --git a/drivers/mmc/xenon_sdhci.c b/drivers/mmc/xenon_sdhci.c
>>> index 6ce9d00d0a..da9882cbf6 100644
>>> --- a/drivers/mmc/xenon_sdhci.c
>>> +++ b/drivers/mmc/xenon_sdhci.c
>>> @@ -350,6 +350,16 @@ static void xenon_mmc_enable_slot(struct sdhci_host *host, u8 slot)
>>>  	sdhci_writel(host, var, SDHC_SYS_OP_CTRL);
>>>  }
>>>  
>>> +/* Disable specific slot */
>>> +static void xenon_mmc_disable_slot(struct sdhci_host *host, u8 slot)
>>> +{
>>> +	u32 var;
>>> +
>>> +	var = sdhci_readl(host, SDHC_SYS_OP_CTRL);
>>> +	var &= ~(SLOT_MASK(slot) << SLOT_ENABLE_SHIFT);
>>> +	sdhci_writel(host, var, SDHC_SYS_OP_CTRL);
>>> +}
>>> +
>>>  /* Enable Parallel Transfer Mode */
>>>  static void xenon_mmc_enable_parallel_tran(struct sdhci_host *host, u8 slot)
>>>  {
>>> @@ -515,6 +525,14 @@ static int xenon_sdhci_probe(struct udevice *dev)
>>>  	return ret;
>>>  }
>>>  
>>> +static int xenon_sdhci_remove(struct udevice *dev)
>>> +{
>>> +	struct sdhci_host *host = dev_get_priv(dev);
>>> +
>>> +	xenon_mmc_disable_slot(host, XENON_MMC_SLOT_ID_HYPERION);
>>> +	return 0;
>>> +}
>>> +
>>>  static int xenon_sdhci_ofdata_to_platdata(struct udevice *dev)
>>>  {
>>>  	struct sdhci_host *host = dev_get_priv(dev);
>>> @@ -564,6 +582,7 @@ U_BOOT_DRIVER(xenon_sdhci_drv) = {
>>>  	.ops		= &sdhci_ops,
>>>  	.bind		= xenon_sdhci_bind,
>>>  	.probe		= xenon_sdhci_probe,
>>> +	.remove		= xenon_sdhci_remove,
>>>  	.priv_auto_alloc_size = sizeof(struct xenon_sdhci_priv),
>>>  	.platdata_auto_alloc_size = sizeof(struct xenon_sdhci_plat),
>>>  };
>>>
>>>
>>
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread
* A3720 - Disable slot when eMMC is not present
  2020-12-08 10:08 ` A3720 - Disable slot when eMMC is not present Pali Rohár
  2020-12-09 10:01   ` Jaehoon Chung
@ 2020-12-14 12:38   ` Andre Heider
  2020-12-15 12:48     ` Pali Rohár
  1 sibling, 1 reply; 6+ messages in thread
From: Andre Heider @ 2020-12-14 12:38 UTC (permalink / raw)
  To: u-boot

On 08/12/2020 11:08, Pali Roh?r wrote:
> Hello! I looked at what is initialized and enabled for sd/emmc slots and
> I found out that comphy mmc needs to be enabled if at least one slot is
> used (e.g. SD card) and then remaining part is slot initialization in
> xenon driver.
> 
> I wrote quick patch to disable slot when unregistering mmc device from
> U-Boot and also unregister emmc on A3720 from U-Boot when it is not
> present. But I have not tested it yet.
> 
> What do you think about it?

I haven't tested this yet, but it looks like it should leave the hw in a 
state as it was on a non-emmc board before the emmc/non-emmc merge. So I 
like the idea!

You can merge your xenon_mmc_disable_slot() with xenon_mmc_enable_slot() 
though, and instead add a 'bool enable' as with xenon_mmc_set_acg() above.

Thanks,
Andre

> 
> diff --git a/board/Marvell/mvebu_armada-37xx/board.c b/board/Marvell/mvebu_armada-37xx/board.c
> index f67b04b78c..1b9e7520cc 100644
> --- a/board/Marvell/mvebu_armada-37xx/board.c
> +++ b/board/Marvell/mvebu_armada-37xx/board.c
> @@ -5,6 +5,7 @@
>   
>   #include <common.h>
>   #include <dm.h>
> +#include <dm/device-internal.h>
>   #include <env.h>
>   #include <i2c.h>
>   #include <init.h>
> @@ -84,12 +85,10 @@ int board_init(void)
>   #ifdef CONFIG_BOARD_LATE_INIT
>   int board_late_init(void)
>   {
> +	struct udevice *dev;
>   	struct mmc *mmc_dev;
>   	bool ddr4, emmc;
>   
> -	if (env_get("fdtfile"))
> -		return 0;
> -
>   	if (!of_machine_is_compatible("globalscale,espressobin"))
>   		return 0;
>   
> @@ -101,6 +100,16 @@ int board_late_init(void)
>   	mmc_dev = find_mmc_device(1);
>   	emmc = (mmc_dev && mmc_init(mmc_dev) == 0);
>   
> +	/* if eMMC is not present then remove it from DM */
> +	if (!emmc && mmc_dev) {
> +		dev = mmc_dev->dev;
> +		device_remove(dev, DM_REMOVE_NORMAL);
> +		device_unbind(dev);
> +	}
> +
> +	if (env_get("fdtfile"))
> +		return 0;
> +
>   	if (ddr4 && emmc)
>   		env_set("fdtfile", "marvell/armada-3720-espressobin-v7-emmc.dtb");
>   	else if (ddr4)
> diff --git a/drivers/mmc/xenon_sdhci.c b/drivers/mmc/xenon_sdhci.c
> index 6ce9d00d0a..da9882cbf6 100644
> --- a/drivers/mmc/xenon_sdhci.c
> +++ b/drivers/mmc/xenon_sdhci.c
> @@ -350,6 +350,16 @@ static void xenon_mmc_enable_slot(struct sdhci_host *host, u8 slot)
>   	sdhci_writel(host, var, SDHC_SYS_OP_CTRL);
>   }
>   
> +/* Disable specific slot */
> +static void xenon_mmc_disable_slot(struct sdhci_host *host, u8 slot)
> +{
> +	u32 var;
> +
> +	var = sdhci_readl(host, SDHC_SYS_OP_CTRL);
> +	var &= ~(SLOT_MASK(slot) << SLOT_ENABLE_SHIFT);
> +	sdhci_writel(host, var, SDHC_SYS_OP_CTRL);
> +}
> +
>   /* Enable Parallel Transfer Mode */
>   static void xenon_mmc_enable_parallel_tran(struct sdhci_host *host, u8 slot)
>   {
> @@ -515,6 +525,14 @@ static int xenon_sdhci_probe(struct udevice *dev)
>   	return ret;
>   }
>   
> +static int xenon_sdhci_remove(struct udevice *dev)
> +{
> +	struct sdhci_host *host = dev_get_priv(dev);
> +
> +	xenon_mmc_disable_slot(host, XENON_MMC_SLOT_ID_HYPERION);
> +	return 0;
> +}
> +
>   static int xenon_sdhci_ofdata_to_platdata(struct udevice *dev)
>   {
>   	struct sdhci_host *host = dev_get_priv(dev);
> @@ -564,6 +582,7 @@ U_BOOT_DRIVER(xenon_sdhci_drv) = {
>   	.ops		= &sdhci_ops,
>   	.bind		= xenon_sdhci_bind,
>   	.probe		= xenon_sdhci_probe,
> +	.remove		= xenon_sdhci_remove,
>   	.priv_auto_alloc_size = sizeof(struct xenon_sdhci_priv),
>   	.platdata_auto_alloc_size = sizeof(struct xenon_sdhci_plat),
>   };
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread
* A3720 - Disable slot when eMMC is not present
  2020-12-14 12:38   ` Andre Heider
@ 2020-12-15 12:48     ` Pali Rohár
  0 siblings, 0 replies; 6+ messages in thread
From: Pali Rohár @ 2020-12-15 12:48 UTC (permalink / raw)
  To: u-boot

Hello!

On Monday 14 December 2020 13:38:07 Andre Heider wrote:
> On 08/12/2020 11:08, Pali Roh?r wrote:
> > Hello! I looked at what is initialized and enabled for sd/emmc slots and
> > I found out that comphy mmc needs to be enabled if at least one slot is
> > used (e.g. SD card) and then remaining part is slot initialization in
> > xenon driver.
> > 
> > I wrote quick patch to disable slot when unregistering mmc device from
> > U-Boot and also unregister emmc on A3720 from U-Boot when it is not
> > present. But I have not tested it yet.
> > 
> > What do you think about it?
> 
> I haven't tested this yet, but it looks like it should leave the hw in a
> state as it was on a non-emmc board before the emmc/non-emmc merge. So I
> like the idea!

Now I have tested this patch on Espressobin v5 without eMMC and mmc
device was correctly removed, it even does not show in 'mmc list'
output. From uSD card is working fine as before.

I do not observe any slowdown of init sequence. And I still have a
problem to catch output and press key to stop uboot booting :D So I
think initialization is still very fast.

> You can merge your xenon_mmc_disable_slot() with xenon_mmc_enable_slot()
> though, and instead add a 'bool enable' as with xenon_mmc_set_acg() above.

I was thinking about it and I chosen separate function as separated it
is also in the linux kernel driver.

But I have no problem to choose any variant. Stefan (or any other
maintainer of this sdhc code), please let me know what style would you
like to see or if you have any other comments for changes.

> Thanks,
> Andre
> 
> > 
> > diff --git a/board/Marvell/mvebu_armada-37xx/board.c b/board/Marvell/mvebu_armada-37xx/board.c
> > index f67b04b78c..1b9e7520cc 100644
> > --- a/board/Marvell/mvebu_armada-37xx/board.c
> > +++ b/board/Marvell/mvebu_armada-37xx/board.c
> > @@ -5,6 +5,7 @@
> >   #include <common.h>
> >   #include <dm.h>
> > +#include <dm/device-internal.h>
> >   #include <env.h>
> >   #include <i2c.h>
> >   #include <init.h>
> > @@ -84,12 +85,10 @@ int board_init(void)
> >   #ifdef CONFIG_BOARD_LATE_INIT
> >   int board_late_init(void)
> >   {
> > +	struct udevice *dev;
> >   	struct mmc *mmc_dev;
> >   	bool ddr4, emmc;
> > -	if (env_get("fdtfile"))
> > -		return 0;
> > -
> >   	if (!of_machine_is_compatible("globalscale,espressobin"))
> >   		return 0;
> > @@ -101,6 +100,16 @@ int board_late_init(void)
> >   	mmc_dev = find_mmc_device(1);
> >   	emmc = (mmc_dev && mmc_init(mmc_dev) == 0);
> > +	/* if eMMC is not present then remove it from DM */
> > +	if (!emmc && mmc_dev) {
> > +		dev = mmc_dev->dev;
> > +		device_remove(dev, DM_REMOVE_NORMAL);
> > +		device_unbind(dev);
> > +	}
> > +
> > +	if (env_get("fdtfile"))
> > +		return 0;
> > +
> >   	if (ddr4 && emmc)
> >   		env_set("fdtfile", "marvell/armada-3720-espressobin-v7-emmc.dtb");
> >   	else if (ddr4)
> > diff --git a/drivers/mmc/xenon_sdhci.c b/drivers/mmc/xenon_sdhci.c
> > index 6ce9d00d0a..da9882cbf6 100644
> > --- a/drivers/mmc/xenon_sdhci.c
> > +++ b/drivers/mmc/xenon_sdhci.c
> > @@ -350,6 +350,16 @@ static void xenon_mmc_enable_slot(struct sdhci_host *host, u8 slot)
> >   	sdhci_writel(host, var, SDHC_SYS_OP_CTRL);
> >   }
> > +/* Disable specific slot */
> > +static void xenon_mmc_disable_slot(struct sdhci_host *host, u8 slot)
> > +{
> > +	u32 var;
> > +
> > +	var = sdhci_readl(host, SDHC_SYS_OP_CTRL);
> > +	var &= ~(SLOT_MASK(slot) << SLOT_ENABLE_SHIFT);
> > +	sdhci_writel(host, var, SDHC_SYS_OP_CTRL);
> > +}
> > +
> >   /* Enable Parallel Transfer Mode */
> >   static void xenon_mmc_enable_parallel_tran(struct sdhci_host *host, u8 slot)
> >   {
> > @@ -515,6 +525,14 @@ static int xenon_sdhci_probe(struct udevice *dev)
> >   	return ret;
> >   }
> > +static int xenon_sdhci_remove(struct udevice *dev)
> > +{
> > +	struct sdhci_host *host = dev_get_priv(dev);
> > +
> > +	xenon_mmc_disable_slot(host, XENON_MMC_SLOT_ID_HYPERION);
> > +	return 0;
> > +}
> > +
> >   static int xenon_sdhci_ofdata_to_platdata(struct udevice *dev)
> >   {
> >   	struct sdhci_host *host = dev_get_priv(dev);
> > @@ -564,6 +582,7 @@ U_BOOT_DRIVER(xenon_sdhci_drv) = {
> >   	.ops		= &sdhci_ops,
> >   	.bind		= xenon_sdhci_bind,
> >   	.probe		= xenon_sdhci_probe,
> > +	.remove		= xenon_sdhci_remove,
> >   	.priv_auto_alloc_size = sizeof(struct xenon_sdhci_priv),
> >   	.platdata_auto_alloc_size = sizeof(struct xenon_sdhci_plat),
> >   };
> > 
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-12-15 12:48 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20201208100846epcas1p2a8e9be0a318b840262ddfb0377ea0f06@epcas1p2.samsung.com>
2020-12-08 10:08 ` A3720 - Disable slot when eMMC is not present Pali Rohár
2020-12-09 10:01   ` Jaehoon Chung
2020-12-09 10:25     ` Pali Rohár
2020-12-09 21:31       ` Jaehoon Chung
2020-12-14 12:38   ` Andre Heider
2020-12-15 12:48     ` Pali Rohár

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.