diff mbox series

[U-Boot] dm: mmc: socfpga: call dwmci_probe()

Message ID 20180306080723.28405-1-linux-kernel-dev@beckhoff.com
State Accepted
Commit 55118ec90c316daed6a50e28da618de4545647b0
Delegated to: Jaehoon Chung
Headers show
Series [U-Boot] dm: mmc: socfpga: call dwmci_probe() | expand

Commit Message

linux-kernel-dev March 6, 2018, 8:07 a.m. UTC
From: Patrick Bruenn <p.bruenn@beckhoff.com>

On a socfpga_cyclone5 based board the SD card, was never powered up. For
other dw_mmc based SoCs dwmci_probe() is called in the platform specific
probe(). It seems this call is missing for socfpga_dw_mmc.

With this change DWMCI_PWREN is set by dmwci_init().

Signed-off-by: Patrick Bruenn <p.bruenn@beckhoff.com>
---

 drivers/mmc/socfpga_dw_mmc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jaehoon Chung March 8, 2018, 2:17 a.m. UTC | #1
On 03/06/2018 05:07 PM, linux-kernel-dev@beckhoff.com wrote:
> From: Patrick Bruenn <p.bruenn@beckhoff.com>
> 
> On a socfpga_cyclone5 based board the SD card, was never powered up. For
> other dw_mmc based SoCs dwmci_probe() is called in the platform specific
> probe(). It seems this call is missing for socfpga_dw_mmc.
> 
> With this change DWMCI_PWREN is set by dmwci_init().
> 
> Signed-off-by: Patrick Bruenn <p.bruenn@beckhoff.com>

Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>

Will apply this patch before releasing v2018.03.
(I have a problem about accessing git.denx.de. After fixing my problem, will resend email about applying.)

Thanks.

Best Regards,
Jaehoon Chung

> ---
> 
>  drivers/mmc/socfpga_dw_mmc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/socfpga_dw_mmc.c b/drivers/mmc/socfpga_dw_mmc.c
> index 759686ccd6..c5fce8f09d 100644
> --- a/drivers/mmc/socfpga_dw_mmc.c
> +++ b/drivers/mmc/socfpga_dw_mmc.c
> @@ -124,7 +124,7 @@ static int socfpga_dwmmc_probe(struct udevice *dev)
>  	upriv->mmc = host->mmc;
>  	host->mmc->dev = dev;
>  
> -	return 0;
> +	return dwmci_probe(dev);
>  }
>  
>  static int socfpga_dwmmc_bind(struct udevice *dev)
>
Marek Vasut March 8, 2018, 3:12 a.m. UTC | #2
On 03/08/2018 03:17 AM, Jaehoon Chung wrote:
> On 03/06/2018 05:07 PM, linux-kernel-dev@beckhoff.com wrote:
>> From: Patrick Bruenn <p.bruenn@beckhoff.com>
>>
>> On a socfpga_cyclone5 based board the SD card, was never powered up. For
>> other dw_mmc based SoCs dwmci_probe() is called in the platform specific
>> probe(). It seems this call is missing for socfpga_dw_mmc.
>>
>> With this change DWMCI_PWREN is set by dmwci_init().
>>
>> Signed-off-by: Patrick Bruenn <p.bruenn@beckhoff.com>
> 
> Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>
> 
> Will apply this patch before releasing v2018.03.
> (I have a problem about accessing git.denx.de. After fixing my problem, will resend email about applying.)

DWMMC works on SoCFPGA for me (tested on rc4), so I don't understand 
what this patch is trying to fix. I'd prefer if you did not hastily 
apply this.
Jaehoon Chung March 8, 2018, 3:56 a.m. UTC | #3
On 03/08/2018 12:12 PM, Marek Vasut wrote:
> On 03/08/2018 03:17 AM, Jaehoon Chung wrote:
>> On 03/06/2018 05:07 PM, linux-kernel-dev@beckhoff.com wrote:
>>> From: Patrick Bruenn <p.bruenn@beckhoff.com>
>>>
>>> On a socfpga_cyclone5 based board the SD card, was never powered up. For
>>> other dw_mmc based SoCs dwmci_probe() is called in the platform specific
>>> probe(). It seems this call is missing for socfpga_dw_mmc.
>>>
>>> With this change DWMCI_PWREN is set by dmwci_init().
>>>
>>> Signed-off-by: Patrick Bruenn <p.bruenn@beckhoff.com>
>>
>> Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>
>>
>> Will apply this patch before releasing v2018.03.
>> (I have a problem about accessing git.denx.de. After fixing my problem, will resend email about applying.)
> 
> DWMMC works on SoCFPGA for me (tested on rc4), so I don't understand what this patch is trying to fix. I'd prefer if you did not hastily apply this.

It's my misunderstanding. When i checked more. I think that Marek is right.
Thanks Marek for pointing out.

Best Regards,
Jaehoon Chung

> 
> 
>
Patrick Brünn March 8, 2018, 5:39 a.m. UTC | #4
>From: Jaehoon Chung [mailto:jh80.chung@samsung.com]

>Sent: Donnerstag, 8. März 2018 04:57

>On 03/08/2018 12:12 PM, Marek Vasut wrote:

>> On 03/08/2018 03:17 AM, Jaehoon Chung wrote:

>>> On 03/06/2018 05:07 PM, linux-kernel-dev@beckhoff.com wrote:

>>>> From: Patrick Bruenn <p.bruenn@beckhoff.com>

>>>>

>>>> On a socfpga_cyclone5 based board the SD card, was never powered up.

>For

>>>> other dw_mmc based SoCs dwmci_probe() is called in the platform

>specific

>>>> probe(). It seems this call is missing for socfpga_dw_mmc.

>>>>

>>>> With this change DWMCI_PWREN is set by dmwci_init().

>>>>

>>>> Signed-off-by: Patrick Bruenn <p.bruenn@beckhoff.com>

>>>

>>> Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>

>>>

>>> Will apply this patch before releasing v2018.03.

>>> (I have a problem about accessing git.denx.de. After fixing my problem,

>will resend email about applying.)

>>

>> DWMMC works on SoCFPGA for me (tested on rc4), so I don't understand

>what this patch is trying to fix. I'd prefer if you did not hastily apply this.

>

>It's my misunderstanding. When i checked more. I think that Marek is right.

>Thanks Marek for pointing out.

>

Okay, but do you have any hint what I am doing wrong? My board (cx8100 not mainline, yet) is based on socfpga_cyclone5. And on my board " dwmci_writel(host, DWMCI_PWREN, 1);" is never called, because dwmci_init() is never called.
As far as I can see with CONFIG_DM_MMC enabled dwmci_init() should be called by dwmci_probe().

exynos  and rockchip do call dwmci_probe() within exynos/rockchip_dwmmc_probe().
but socfpga_dwmmc_probe() is missing this call. So I looked deeper but found no place for socfpga platform to call dwmci_probe() or dwmci_init().
What am I missing?

Maybe this diff helps:
u-boot$ diff configs/socfpga_cx8100_defconfig configs/socfpga_cyclone5_defconfig
2d1
< CONFIG_API=y
6c5
< CONFIG_TARGET_SOCFPGA_CX8100=y
---
> CONFIG_TARGET_SOCFPGA_CYCLONE5_SOCDK=y

8c7,8
< CONFIG_DEFAULT_DEVICE_TREE="socfpga_cyclone5_cx8100"
---
> CONFIG_DEFAULT_DEVICE_TREE="socfpga_cyclone5_socdk"

> CONFIG_DISTRO_DEFAULTS=y

9a10
> # CONFIG_USE_BOOTCOMMAND is not set

13c14
< CONFIG_DEFAULT_FDT_FILE="socfpga_cyclone5_cx8100.dtb"
---
> CONFIG_DEFAULT_FDT_FILE="socfpga_cyclone5_socdk.dtb"

16d16
< CONFIG_BOARD_EARLY_INIT_F=y
20,21d19
< CONFIG_HUSH_PARSER=y
< CONFIG_CMD_BOOTZ=y
29d26
< CONFIG_CMD_PART=y
34,37d30
< CONFIG_CMD_DHCP=y
< CONFIG_CMD_PXE=y
< CONFIG_CMD_MII=y
< CONFIG_CMD_PING=y
39d31
< CONFIG_CMD_EXT4=y
41,42d32
< CONFIG_CMD_FAT=y
< CONFIG_CMD_FS_GENERIC=y
54,56d43
< CONFIG_CMD_LED=y
< CONFIG_LED=y
< CONFIG_LED_GPIO=y
60a48
> CONFIG_SPI_FLASH_MACRONIX=y


Thanks for your time and patience,
Patrick

Beckhoff Automation GmbH & Co. KG | Managing Director: Dipl. Phys. Hans Beckhoff
Registered office: Verl, Germany | Register court: Guetersloh HRA 7075
Patrick Brünn March 9, 2018, 6:51 a.m. UTC | #5
>From: Patrick Brünn

>Sent: Donnerstag, 8. März 2018 06:39

>>From: Jaehoon Chung [mailto:jh80.chung@samsung.com]

>>Sent: Donnerstag, 8. März 2018 04:57

>>On 03/08/2018 12:12 PM, Marek Vasut wrote:

>>> On 03/08/2018 03:17 AM, Jaehoon Chung wrote:

>>>> On 03/06/2018 05:07 PM, linux-kernel-dev@beckhoff.com wrote:

>>>>> From: Patrick Bruenn <p.bruenn@beckhoff.com>

>>>>>

>>>>> On a socfpga_cyclone5 based board the SD card, was never powered

>up.

>>For

>>>>> other dw_mmc based SoCs dwmci_probe() is called in the platform

>>specific

>>>>> probe(). It seems this call is missing for socfpga_dw_mmc.

>>>>>

>>>>> With this change DWMCI_PWREN is set by dmwci_init().

>>>>>

>>>>> Signed-off-by: Patrick Bruenn <p.bruenn@beckhoff.com>

>>>>

>>>> Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>

>>>>

>>>> Will apply this patch before releasing v2018.03.

>>>> (I have a problem about accessing git.denx.de. After fixing my problem,

>>will resend email about applying.)

>>>

>>> DWMMC works on SoCFPGA for me (tested on rc4), so I don't understand

>>what this patch is trying to fix. I'd prefer if you did not hastily apply this.

>>

>>It's my misunderstanding. When i checked more. I think that Marek is right.

>>Thanks Marek for pointing out.

>>

>Okay, but do you have any hint what I am doing wrong? My board (cx8100 not

>mainline, yet) is based on socfpga_cyclone5. And on my board "

>dwmci_writel(host, DWMCI_PWREN, 1);" is never called, because

>dwmci_init() is never called.

>As far as I can see with CONFIG_DM_MMC enabled dwmci_init() should be

>called by dwmci_probe().

>

>exynos  and rockchip do call dwmci_probe() within

>exynos/rockchip_dwmmc_probe().

>but socfpga_dwmmc_probe() is missing this call. So I looked deeper but found

>no place for socfpga platform to call dwmci_probe() or dwmci_init().

>What am I missing?

>

I got an idea, what might be the difference between my board and your boards.
I suspect you use U-BOOT SPL without CONFIG_DM_MMC set, so
dwmci_init() is called indirectly by mmc_start_init().
Now, when your main U-Boot (with CONFIG_DM_MMC) is launched,
everything is already configured and it isn't necessary to call dwmci_init() again.
On my board the Altera MPL is used (and I can't replace it).  Which seems to
disable DWMCI_PWREN before launching U-Boot.
If my assumption is correct, I still think it is a U-Boot bug to assume code like in
dwmci_init() was already run before U-Boot gets in control.
Besides exynos/rockchip_dw_mmc don't have this precondition requirement.

Please take your time to look deeper into this issue, before deciding anything.
I don't think we need to rush this into the next release, as normal mainline
boards are "accidently" not affected.

Thanks and regards,
Patrick

Beckhoff Automation GmbH & Co. KG | Managing Director: Dipl. Phys. Hans Beckhoff
Registered office: Verl, Germany | Register court: Guetersloh HRA 7075
Jaehoon Chung March 9, 2018, 8:24 a.m. UTC | #6
Dear Patrick,

On 03/09/2018 03:51 PM, Patrick Brünn wrote:
>> From: Patrick Brünn
>> Sent: Donnerstag, 8. März 2018 06:39
>>> From: Jaehoon Chung [mailto:jh80.chung@samsung.com]
>>> Sent: Donnerstag, 8. März 2018 04:57
>>> On 03/08/2018 12:12 PM, Marek Vasut wrote:
>>>> On 03/08/2018 03:17 AM, Jaehoon Chung wrote:
>>>>> On 03/06/2018 05:07 PM, linux-kernel-dev@beckhoff.com wrote:
>>>>>> From: Patrick Bruenn <p.bruenn@beckhoff.com>
>>>>>>
>>>>>> On a socfpga_cyclone5 based board the SD card, was never powered
>> up.
>>> For
>>>>>> other dw_mmc based SoCs dwmci_probe() is called in the platform
>>> specific
>>>>>> probe(). It seems this call is missing for socfpga_dw_mmc.
>>>>>>
>>>>>> With this change DWMCI_PWREN is set by dmwci_init().
>>>>>>
>>>>>> Signed-off-by: Patrick Bruenn <p.bruenn@beckhoff.com>
>>>>>
>>>>> Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>
>>>>>
>>>>> Will apply this patch before releasing v2018.03.
>>>>> (I have a problem about accessing git.denx.de. After fixing my problem,
>>> will resend email about applying.)
>>>>
>>>> DWMMC works on SoCFPGA for me (tested on rc4), so I don't understand
>>> what this patch is trying to fix. I'd prefer if you did not hastily apply this.
>>>
>>> It's my misunderstanding. When i checked more. I think that Marek is right.
>>> Thanks Marek for pointing out.
>>>
>> Okay, but do you have any hint what I am doing wrong? My board (cx8100 not
>> mainline, yet) is based on socfpga_cyclone5. And on my board "
>> dwmci_writel(host, DWMCI_PWREN, 1);" is never called, because
>> dwmci_init() is never called.
>> As far as I can see with CONFIG_DM_MMC enabled dwmci_init() should be
>> called by dwmci_probe().
>>
>> exynos  and rockchip do call dwmci_probe() within
>> exynos/rockchip_dwmmc_probe().
>> but socfpga_dwmmc_probe() is missing this call. So I looked deeper but found
>> no place for socfpga platform to call dwmci_probe() or dwmci_init().
>> What am I missing?
>>
> I got an idea, what might be the difference between my board and your boards.
> I suspect you use U-BOOT SPL without CONFIG_DM_MMC set, so
> dwmci_init() is called indirectly by mmc_start_init().

Right, it's difference with CONFIG_DM_MMC. I had checked this.

> Now, when your main U-Boot (with CONFIG_DM_MMC) is launched,
> everything is already configured and it isn't necessary to call dwmci_init() again.
> On my board the Altera MPL is used (and I can't replace it).  Which seems to
> disable DWMCI_PWREN before launching U-Boot.
> If my assumption is correct, I still think it is a U-Boot bug to assume code like in
> dwmci_init() was already run before U-Boot gets in control.
> Besides exynos/rockchip_dw_mmc don't have this precondition requirement.
> 
> Please take your time to look deeper into this issue, before deciding anything.
> I don't think we need to rush this into the next release, as normal mainline
> boards are "accidently" not affected.

Sure. I will check this patch. Before applying this, will check more carefully. 
I agreed about Marek's opinion "did not hastily apply this."
And i think that your approach also is right.

Best Regards,
Jaehoon Chung

> 
> Thanks and regards,
> Patrick
> 
> Beckhoff Automation GmbH & Co. KG | Managing Director: Dipl. Phys. Hans Beckhoff
> Registered office: Verl, Germany | Register court: Guetersloh HRA 7075
> 
>
Marek Vasut March 9, 2018, 6:12 p.m. UTC | #7
On 03/09/2018 07:51 AM, Patrick Brünn wrote:
>> From: Patrick Brünn
>> Sent: Donnerstag, 8. März 2018 06:39
>>> From: Jaehoon Chung [mailto:jh80.chung@samsung.com]
>>> Sent: Donnerstag, 8. März 2018 04:57
>>> On 03/08/2018 12:12 PM, Marek Vasut wrote:
>>>> On 03/08/2018 03:17 AM, Jaehoon Chung wrote:
>>>>> On 03/06/2018 05:07 PM, linux-kernel-dev@beckhoff.com wrote:
>>>>>> From: Patrick Bruenn <p.bruenn@beckhoff.com>
>>>>>>
>>>>>> On a socfpga_cyclone5 based board the SD card, was never powered
>> up.
>>> For
>>>>>> other dw_mmc based SoCs dwmci_probe() is called in the platform
>>> specific
>>>>>> probe(). It seems this call is missing for socfpga_dw_mmc.
>>>>>>
>>>>>> With this change DWMCI_PWREN is set by dmwci_init().
>>>>>>
>>>>>> Signed-off-by: Patrick Bruenn <p.bruenn@beckhoff.com>
>>>>>
>>>>> Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>
>>>>>
>>>>> Will apply this patch before releasing v2018.03.
>>>>> (I have a problem about accessing git.denx.de. After fixing my problem,
>>> will resend email about applying.)
>>>>
>>>> DWMMC works on SoCFPGA for me (tested on rc4), so I don't understand
>>> what this patch is trying to fix. I'd prefer if you did not hastily apply this.
>>>
>>> It's my misunderstanding. When i checked more. I think that Marek is right.
>>> Thanks Marek for pointing out.
>>>
>> Okay, but do you have any hint what I am doing wrong? My board (cx8100 not
>> mainline, yet) is based on socfpga_cyclone5. And on my board "
>> dwmci_writel(host, DWMCI_PWREN, 1);" is never called, because
>> dwmci_init() is never called.
>> As far as I can see with CONFIG_DM_MMC enabled dwmci_init() should be
>> called by dwmci_probe().
>>
>> exynos  and rockchip do call dwmci_probe() within
>> exynos/rockchip_dwmmc_probe().
>> but socfpga_dwmmc_probe() is missing this call. So I looked deeper but found
>> no place for socfpga platform to call dwmci_probe() or dwmci_init().
>> What am I missing?
>>
> I got an idea, what might be the difference between my board and your boards.
> I suspect you use U-BOOT SPL without CONFIG_DM_MMC set, so
> dwmci_init() is called indirectly by mmc_start_init().
> Now, when your main U-Boot (with CONFIG_DM_MMC) is launched,
> everything is already configured and it isn't necessary to call dwmci_init() again.

Yes, that's the only supported configuration -- U-Boot SPL and U-Boot. 
Any such mix-and-match setups are NOT supported.

> On my board the Altera MPL is used (and I can't replace it).  Which seems to
> disable DWMCI_PWREN before launching U-Boot.
> If my assumption is correct, I still think it is a U-Boot bug to assume code like in
> dwmci_init() was already run before U-Boot gets in control.
> Besides exynos/rockchip_dw_mmc don't have this precondition requirement.

We're too close to the release and it's not broken in mainline, it's 
only broken if you use this horrible shitty MPL configuration, which is 
not supported.

So we should revisit this patch after release, after it can be tested 
properly and checked if it doesn't break standard setups.

> Please take your time to look deeper into this issue, before deciding anything.
> I don't think we need to rush this into the next release, as normal mainline
> boards are "accidently" not affected.

Sounds good
Marek Vasut March 9, 2018, 6:13 p.m. UTC | #8
On 03/09/2018 09:24 AM, Jaehoon Chung wrote:
> Dear Patrick,
> 
> On 03/09/2018 03:51 PM, Patrick Brünn wrote:
>>> From: Patrick Brünn
>>> Sent: Donnerstag, 8. März 2018 06:39
>>>> From: Jaehoon Chung [mailto:jh80.chung@samsung.com]
>>>> Sent: Donnerstag, 8. März 2018 04:57
>>>> On 03/08/2018 12:12 PM, Marek Vasut wrote:
>>>>> On 03/08/2018 03:17 AM, Jaehoon Chung wrote:
>>>>>> On 03/06/2018 05:07 PM, linux-kernel-dev@beckhoff.com wrote:
>>>>>>> From: Patrick Bruenn <p.bruenn@beckhoff.com>
>>>>>>>
>>>>>>> On a socfpga_cyclone5 based board the SD card, was never powered
>>> up.
>>>> For
>>>>>>> other dw_mmc based SoCs dwmci_probe() is called in the platform
>>>> specific
>>>>>>> probe(). It seems this call is missing for socfpga_dw_mmc.
>>>>>>>
>>>>>>> With this change DWMCI_PWREN is set by dmwci_init().
>>>>>>>
>>>>>>> Signed-off-by: Patrick Bruenn <p.bruenn@beckhoff.com>
>>>>>>
>>>>>> Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>
>>>>>>
>>>>>> Will apply this patch before releasing v2018.03.
>>>>>> (I have a problem about accessing git.denx.de. After fixing my problem,
>>>> will resend email about applying.)
>>>>>
>>>>> DWMMC works on SoCFPGA for me (tested on rc4), so I don't understand
>>>> what this patch is trying to fix. I'd prefer if you did not hastily apply this.
>>>>
>>>> It's my misunderstanding. When i checked more. I think that Marek is right.
>>>> Thanks Marek for pointing out.
>>>>
>>> Okay, but do you have any hint what I am doing wrong? My board (cx8100 not
>>> mainline, yet) is based on socfpga_cyclone5. And on my board "
>>> dwmci_writel(host, DWMCI_PWREN, 1);" is never called, because
>>> dwmci_init() is never called.
>>> As far as I can see with CONFIG_DM_MMC enabled dwmci_init() should be
>>> called by dwmci_probe().
>>>
>>> exynos  and rockchip do call dwmci_probe() within
>>> exynos/rockchip_dwmmc_probe().
>>> but socfpga_dwmmc_probe() is missing this call. So I looked deeper but found
>>> no place for socfpga platform to call dwmci_probe() or dwmci_init().
>>> What am I missing?
>>>
>> I got an idea, what might be the difference between my board and your boards.
>> I suspect you use U-BOOT SPL without CONFIG_DM_MMC set, so
>> dwmci_init() is called indirectly by mmc_start_init().
> 
> Right, it's difference with CONFIG_DM_MMC. I had checked this.
> 
>> Now, when your main U-Boot (with CONFIG_DM_MMC) is launched,
>> everything is already configured and it isn't necessary to call dwmci_init() again.
>> On my board the Altera MPL is used (and I can't replace it).  Which seems to
>> disable DWMCI_PWREN before launching U-Boot.
>> If my assumption is correct, I still think it is a U-Boot bug to assume code like in
>> dwmci_init() was already run before U-Boot gets in control.
>> Besides exynos/rockchip_dw_mmc don't have this precondition requirement.
>>
>> Please take your time to look deeper into this issue, before deciding anything.
>> I don't think we need to rush this into the next release, as normal mainline
>> boards are "accidently" not affected.
> 
> Sure. I will check this patch. Before applying this, will check more carefully.
> I agreed about Marek's opinion "did not hastily apply this."
> And i think that your approach also is right.

Please queue for -next , NOT this release.
Jaehoon Chung March 13, 2018, 5:01 a.m. UTC | #9
On 03/10/2018 03:13 AM, Marek Vasut wrote:
> On 03/09/2018 09:24 AM, Jaehoon Chung wrote:
>> Dear Patrick,
>>
>> On 03/09/2018 03:51 PM, Patrick Brünn wrote:
>>>> From: Patrick Brünn
>>>> Sent: Donnerstag, 8. März 2018 06:39
>>>>> From: Jaehoon Chung [mailto:jh80.chung@samsung.com]
>>>>> Sent: Donnerstag, 8. März 2018 04:57
>>>>> On 03/08/2018 12:12 PM, Marek Vasut wrote:
>>>>>> On 03/08/2018 03:17 AM, Jaehoon Chung wrote:
>>>>>>> On 03/06/2018 05:07 PM, linux-kernel-dev@beckhoff.com wrote:
>>>>>>>> From: Patrick Bruenn <p.bruenn@beckhoff.com>
>>>>>>>>
>>>>>>>> On a socfpga_cyclone5 based board the SD card, was never powered
>>>> up.
>>>>> For
>>>>>>>> other dw_mmc based SoCs dwmci_probe() is called in the platform
>>>>> specific
>>>>>>>> probe(). It seems this call is missing for socfpga_dw_mmc.
>>>>>>>>
>>>>>>>> With this change DWMCI_PWREN is set by dmwci_init().
>>>>>>>>
>>>>>>>> Signed-off-by: Patrick Bruenn <p.bruenn@beckhoff.com>
>>>>>>>
>>>>>>> Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>
>>>>>>>
>>>>>>> Will apply this patch before releasing v2018.03.
>>>>>>> (I have a problem about accessing git.denx.de. After fixing my problem,
>>>>> will resend email about applying.)
>>>>>>
>>>>>> DWMMC works on SoCFPGA for me (tested on rc4), so I don't understand
>>>>> what this patch is trying to fix. I'd prefer if you did not hastily apply this.
>>>>>
>>>>> It's my misunderstanding. When i checked more. I think that Marek is right.
>>>>> Thanks Marek for pointing out.
>>>>>
>>>> Okay, but do you have any hint what I am doing wrong? My board (cx8100 not
>>>> mainline, yet) is based on socfpga_cyclone5. And on my board "
>>>> dwmci_writel(host, DWMCI_PWREN, 1);" is never called, because
>>>> dwmci_init() is never called.
>>>> As far as I can see with CONFIG_DM_MMC enabled dwmci_init() should be
>>>> called by dwmci_probe().
>>>>
>>>> exynos  and rockchip do call dwmci_probe() within
>>>> exynos/rockchip_dwmmc_probe().
>>>> but socfpga_dwmmc_probe() is missing this call. So I looked deeper but found
>>>> no place for socfpga platform to call dwmci_probe() or dwmci_init().
>>>> What am I missing?
>>>>
>>> I got an idea, what might be the difference between my board and your boards.
>>> I suspect you use U-BOOT SPL without CONFIG_DM_MMC set, so
>>> dwmci_init() is called indirectly by mmc_start_init().
>>
>> Right, it's difference with CONFIG_DM_MMC. I had checked this.
>>
>>> Now, when your main U-Boot (with CONFIG_DM_MMC) is launched,
>>> everything is already configured and it isn't necessary to call dwmci_init() again.
>>> On my board the Altera MPL is used (and I can't replace it).  Which seems to
>>> disable DWMCI_PWREN before launching U-Boot.
>>> If my assumption is correct, I still think it is a U-Boot bug to assume code like in
>>> dwmci_init() was already run before U-Boot gets in control.
>>> Besides exynos/rockchip_dw_mmc don't have this precondition requirement.
>>>
>>> Please take your time to look deeper into this issue, before deciding anything.
>>> I don't think we need to rush this into the next release, as normal mainline
>>> boards are "accidently" not affected.
>>
>> Sure. I will check this patch. Before applying this, will check more carefully.
>> I agreed about Marek's opinion "did not hastily apply this."
>> And i think that your approach also is right.
> 
> Please queue for -next , NOT this release.

Yes. I understood what you want. Thanks!

> 
> 
>
Jaehoon Chung April 26, 2018, 10:30 a.m. UTC | #10
Hi,

On 03/06/2018 05:07 PM, linux-kernel-dev@beckhoff.com wrote:
> From: Patrick Bruenn <p.bruenn@beckhoff.com>
> 
> On a socfpga_cyclone5 based board the SD card, was never powered up. For
> other dw_mmc based SoCs dwmci_probe() is called in the platform specific
> probe(). It seems this call is missing for socfpga_dw_mmc.
> 
> With this change DWMCI_PWREN is set by dmwci_init().

Sorry for late. Applied to u-boot-mmc. Thanks!

Best Regards,
Jaehoon Chung

> 
> Signed-off-by: Patrick Bruenn <p.bruenn@beckhoff.com>
> Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>
> ---
> 
>  drivers/mmc/socfpga_dw_mmc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/socfpga_dw_mmc.c b/drivers/mmc/socfpga_dw_mmc.c
> index 759686ccd6..c5fce8f09d 100644
> --- a/drivers/mmc/socfpga_dw_mmc.c
> +++ b/drivers/mmc/socfpga_dw_mmc.c
> @@ -124,7 +124,7 @@ static int socfpga_dwmmc_probe(struct udevice *dev)
>  	upriv->mmc = host->mmc;
>  	host->mmc->dev = dev;
>  
> -	return 0;
> +	return dwmci_probe(dev);
>  }
>  
>  static int socfpga_dwmmc_bind(struct udevice *dev)
>
Marek Vasut April 26, 2018, 12:01 p.m. UTC | #11
On 04/26/2018 12:30 PM, Jaehoon Chung wrote:
> Hi,
> 
> On 03/06/2018 05:07 PM, linux-kernel-dev@beckhoff.com wrote:
>> From: Patrick Bruenn <p.bruenn@beckhoff.com>
>>
>> On a socfpga_cyclone5 based board the SD card, was never powered up. For
>> other dw_mmc based SoCs dwmci_probe() is called in the platform specific
>> probe(). It seems this call is missing for socfpga_dw_mmc.
>>
>> With this change DWMCI_PWREN is set by dmwci_init().
> 
> Sorry for late. Applied to u-boot-mmc. Thanks!

This is for next, right ?
Jaehoon Chung April 27, 2018, 8:35 a.m. UTC | #12
Dear Marek,

On 04/26/2018 09:01 PM, Marek Vasut wrote:
> On 04/26/2018 12:30 PM, Jaehoon Chung wrote:
>> Hi,
>>
>> On 03/06/2018 05:07 PM, linux-kernel-dev@beckhoff.com wrote:
>>> From: Patrick Bruenn <p.bruenn@beckhoff.com>
>>>
>>> On a socfpga_cyclone5 based board the SD card, was never powered up. For
>>> other dw_mmc based SoCs dwmci_probe() is called in the platform specific
>>> probe(). It seems this call is missing for socfpga_dw_mmc.
>>>
>>> With this change DWMCI_PWREN is set by dmwci_init().
>>
>> Sorry for late. Applied to u-boot-mmc. Thanks!
> 
> This is for next, right ?

If possible, i want to apply this for v2018.05.
I know it's too late. It's my fault.
If not possible, i will rebase mmc repository with fixing patches.
And other patches will move to next branch.

Will make a decision until today. And Will send the PR for v2081.05.

Best Regards,
Jaehoon Chung

>
Marek Vasut April 27, 2018, 8:48 a.m. UTC | #13
On 04/27/2018 10:35 AM, Jaehoon Chung wrote:
> Dear Marek,
> 
> On 04/26/2018 09:01 PM, Marek Vasut wrote:
>> On 04/26/2018 12:30 PM, Jaehoon Chung wrote:
>>> Hi,
>>>
>>> On 03/06/2018 05:07 PM, linux-kernel-dev@beckhoff.com wrote:
>>>> From: Patrick Bruenn <p.bruenn@beckhoff.com>
>>>>
>>>> On a socfpga_cyclone5 based board the SD card, was never powered up. For
>>>> other dw_mmc based SoCs dwmci_probe() is called in the platform specific
>>>> probe(). It seems this call is missing for socfpga_dw_mmc.
>>>>
>>>> With this change DWMCI_PWREN is set by dmwci_init().
>>>
>>> Sorry for late. Applied to u-boot-mmc. Thanks!
>>
>> This is for next, right ?
> 
> If possible, i want to apply this for v2018.05.
> I know it's too late. It's my fault.

Yes it is, and if it breaks something, there's very little time to find
out or test it.

> If not possible, i will rebase mmc repository with fixing patches.
> And other patches will move to next branch.

Yes

> Will make a decision until today. And Will send the PR for v2081.05.

I am happy to take this via socfpga-next if you cannot submit a PR right
after the release for some reason.
Jaehoon Chung April 30, 2018, 6:37 a.m. UTC | #14
On 04/27/2018 05:48 PM, Marek Vasut wrote:
> On 04/27/2018 10:35 AM, Jaehoon Chung wrote:
>> Dear Marek,
>>
>> On 04/26/2018 09:01 PM, Marek Vasut wrote:
>>> On 04/26/2018 12:30 PM, Jaehoon Chung wrote:
>>>> Hi,
>>>>
>>>> On 03/06/2018 05:07 PM, linux-kernel-dev@beckhoff.com wrote:
>>>>> From: Patrick Bruenn <p.bruenn@beckhoff.com>
>>>>>
>>>>> On a socfpga_cyclone5 based board the SD card, was never powered up. For
>>>>> other dw_mmc based SoCs dwmci_probe() is called in the platform specific
>>>>> probe(). It seems this call is missing for socfpga_dw_mmc.
>>>>>
>>>>> With this change DWMCI_PWREN is set by dmwci_init().
>>>>
>>>> Sorry for late. Applied to u-boot-mmc. Thanks!
>>>
>>> This is for next, right ?
>>
>> If possible, i want to apply this for v2018.05.
>> I know it's too late. It's my fault.
> 
> Yes it is, and if it breaks something, there's very little time to find
> out or test it.
> 
>> If not possible, i will rebase mmc repository with fixing patches.
>> And other patches will move to next branch.
> 
> Yes

Will do. This patch will move to next branch.

> 
>> Will make a decision until today. And Will send the PR for v2081.05.
> 
> I am happy to take this via socfpga-next if you cannot submit a PR right
> after the release for some reason.

I will do.

Best Regards,
Jaehoon Chung

>
diff mbox series

Patch

diff --git a/drivers/mmc/socfpga_dw_mmc.c b/drivers/mmc/socfpga_dw_mmc.c
index 759686ccd6..c5fce8f09d 100644
--- a/drivers/mmc/socfpga_dw_mmc.c
+++ b/drivers/mmc/socfpga_dw_mmc.c
@@ -124,7 +124,7 @@  static int socfpga_dwmmc_probe(struct udevice *dev)
 	upriv->mmc = host->mmc;
 	host->mmc->dev = dev;
 
-	return 0;
+	return dwmci_probe(dev);
 }
 
 static int socfpga_dwmmc_bind(struct udevice *dev)