[PATCH u-boot-marvell] mmc: mv_sdhci: call mmc_of_parse()

[PATCH u-boot-marvell] mmc: mv_sdhci: call mmc_of_parse()

[Marek Behún]

This is needed to parse more capabilities such as `non-removable`.

Commit da18c62b6e6a ("mmc: sdhci: Implement SDHCI card detect") caused
a regression on Turris Omnia, because mv_sdhci driver did not fill out
host_caps from device-tree.

Signed-off-by: Marek Beh?n <marek.behun@nic.cz>
Fixes: da18c62b6e6a ("mmc: sdhci: Implement SDHCI card detect")
Cc: pali at kernel.org
Cc: Baruch Siach <baruch@tkos.co.il>
Cc: Stefan Roese <sr@denx.de>
---
 drivers/mmc/mv_sdhci.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/mmc/mv_sdhci.c b/drivers/mmc/mv_sdhci.c
index 556dd38046..4dc4a0d2be 100644
--- a/drivers/mmc/mv_sdhci.c
+++ b/drivers/mmc/mv_sdhci.c
@@ -118,6 +118,10 @@ static int mv_sdhci_probe(struct udevice *dev)
 	host->mmc->dev = dev;
 	host->mmc->priv = host;
 
+	ret = mmc_of_parse(dev, &plat->cfg);
+	if (ret)
+		return ret;
+
 	ret = sdhci_setup_cfg(&plat->cfg, host, 0, 0);
 	if (ret)
 		return ret;
-- 
2.26.2

[PATCH u-boot-marvell] mmc: mv_sdhci: call mmc_of_parse()

[Baruch Siach]

Hi Marek,

On Tue, Feb 02 2021, Marek Beh?n wrote:
> This is needed to parse more capabilities such as `non-removable`.
>
> Commit da18c62b6e6a ("mmc: sdhci: Implement SDHCI card detect") caused
> a regression on Turris Omnia, because mv_sdhci driver did not fill out
> host_caps from device-tree.

I posted a similar patch earlier today:

  https://patchwork.ozlabs.org/project/uboot/patch/7dcd24e8d0149618cf686c47cce6728a64dffe2b.1612248184.git.baruch at tkos.co.il/

> Signed-off-by: Marek Beh?n <marek.behun@nic.cz>
> Fixes: da18c62b6e6a ("mmc: sdhci: Implement SDHCI card detect")

My patch is missing this tag, though.

baruch

> Cc: pali at kernel.org
> Cc: Baruch Siach <baruch@tkos.co.il>
> Cc: Stefan Roese <sr@denx.de>
> ---
>  drivers/mmc/mv_sdhci.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/mmc/mv_sdhci.c b/drivers/mmc/mv_sdhci.c
> index 556dd38046..4dc4a0d2be 100644
> --- a/drivers/mmc/mv_sdhci.c
> +++ b/drivers/mmc/mv_sdhci.c
> @@ -118,6 +118,10 @@ static int mv_sdhci_probe(struct udevice *dev)
>  	host->mmc->dev = dev;
>  	host->mmc->priv = host;
>  
> +	ret = mmc_of_parse(dev, &plat->cfg);
> +	if (ret)
> +		return ret;
> +
>  	ret = sdhci_setup_cfg(&plat->cfg, host, 0, 0);
>  	if (ret)
>  		return ret;


-- 
                                                     ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -

[PATCH u-boot-marvell] mmc: mv_sdhci: call mmc_of_parse()

[Marek Behún]

> Hi Marek,
> I posted a similar patch earlier today:
> > Fixes: da18c62b6e6a ("mmc: sdhci: Implement SDHCI card detect")  
> My patch is missing this tag, though.
> baruch

OK, lets leave it on Stefan to decide which one to apply.
Marek

[PATCH u-boot-marvell] mmc: mv_sdhci: call mmc_of_parse()

[Jaehoon Chung]

On 2/3/21 2:37 AM, Marek Beh?n wrote:
> This is needed to parse more capabilities such as `non-removable`.
> 
> Commit da18c62b6e6a ("mmc: sdhci: Implement SDHCI card detect") caused
> a regression on Turris Omnia, because mv_sdhci driver did not fill out
> host_caps from device-tree.
> 
> Signed-off-by: Marek Beh?n <marek.behun@nic.cz>
> Fixes: da18c62b6e6a ("mmc: sdhci: Implement SDHCI card detect")

I don't think that this patch is for fixing its commit.

Best Regards,
Jaehoon Chung

> Cc: pali at kernel.org
> Cc: Baruch Siach <baruch@tkos.co.il>
> Cc: Stefan Roese <sr@denx.de>
> ---
>  drivers/mmc/mv_sdhci.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/mmc/mv_sdhci.c b/drivers/mmc/mv_sdhci.c
> index 556dd38046..4dc4a0d2be 100644
> --- a/drivers/mmc/mv_sdhci.c
> +++ b/drivers/mmc/mv_sdhci.c
> @@ -118,6 +118,10 @@ static int mv_sdhci_probe(struct udevice *dev)
>  	host->mmc->dev = dev;
>  	host->mmc->priv = host;
>  
> +	ret = mmc_of_parse(dev, &plat->cfg);
> +	if (ret)
> +		return ret;
> +
>  	ret = sdhci_setup_cfg(&plat->cfg, host, 0, 0);
>  	if (ret)
>  		return ret;
> 

[PATCH u-boot-marvell] mmc: mv_sdhci: call mmc_of_parse()

[Marek Behun]

On Wed, 3 Feb 2021 08:32:45 +0900
Jaehoon Chung <jh80.chung@samsung.com> wrote:

> On 2/3/21 2:37 AM, Marek Beh?n wrote:
> > This is needed to parse more capabilities such as `non-removable`.
> > 
> > Commit da18c62b6e6a ("mmc: sdhci: Implement SDHCI card detect") caused
> > a regression on Turris Omnia, because mv_sdhci driver did not fill out
> > host_caps from device-tree.
> > 
> > Signed-off-by: Marek Beh?n <marek.behun@nic.cz>
> > Fixes: da18c62b6e6a ("mmc: sdhci: Implement SDHCI card detect")  
> 
> I don't think that this patch is for fixing its commit.
> 
> Best Regards,
> Jaehoon Chung

Commit da18c62b6e6a causes the regression. The Fixes tag, as I
understand, should link to commit with which the regression first
occured, so that if someone wanted to backport my patch to previous
version of U-Boot, they would know that they only have to do it if
their repository contains that commit.

Marek

[PATCH u-boot-marvell] mmc: mv_sdhci: call mmc_of_parse()

[Jaehoon Chung]

On 2/3/21 10:41 PM, Marek Behun wrote:
> On Wed, 3 Feb 2021 08:32:45 +0900
> Jaehoon Chung <jh80.chung@samsung.com> wrote:
> 
>> On 2/3/21 2:37 AM, Marek Beh?n wrote:
>>> This is needed to parse more capabilities such as `non-removable`.
>>>
>>> Commit da18c62b6e6a ("mmc: sdhci: Implement SDHCI card detect") caused
>>> a regression on Turris Omnia, because mv_sdhci driver did not fill out
>>> host_caps from device-tree.
>>>
>>> Signed-off-by: Marek Beh?n <marek.behun@nic.cz>
>>> Fixes: da18c62b6e6a ("mmc: sdhci: Implement SDHCI card detect")  
>>
>> I don't think that this patch is for fixing its commit.
>>
>> Best Regards,
>> Jaehoon Chung
> 
> Commit da18c62b6e6a causes the regression. The Fixes tag, as I
> understand, should link to commit with which the regression first
> occured, so that if someone wanted to backport my patch to previous
> version of U-Boot, they would know that they only have to do it if
> their repository contains that commit.

Well, i don't think so. This patch is not a bug fix patch about commit da18c62b6e6a.
Just mv_sdhci driver didn't use mmc_of_parse() before. 
(If someone set to MMC_NON_REMOVABLE capabilities in host->cap, it should be working.)

Not need to add *Fixes*. Its commit is a generic callback function.
And mmc_of_parse() is also generic function to use Driver-model.

Best Regards,
Jaehoon Chung

> 
> Marek
> 

[PATCH u-boot-marvell] mmc: mv_sdhci: call mmc_of_parse()

[Marek Behún]

On Thu, 4 Feb 2021 07:18:23 +0900
Jaehoon Chung <jh80.chung@samsung.com> wrote:

> > Commit da18c62b6e6a causes the regression. The Fixes tag, as I
> > understand, should link to commit with which the regression first
> > occured, so that if someone wanted to backport my patch to previous
> > version of U-Boot, they would know that they only have to do it if
> > their repository contains that commit.  
> 
> Well, i don't think so. This patch is not a bug fix patch about
> commit da18c62b6e6a. Just mv_sdhci driver didn't use mmc_of_parse()
> before. (If someone set to MMC_NON_REMOVABLE capabilities in
> host->cap, it should be working.)
> 
> Not need to add *Fixes*. Its commit is a generic callback function.
> And mmc_of_parse() is also generic function to use Driver-model.

Lets apply Baruch's version. I don't really care if the Fixes tag is
there or not...

Marek

[PATCH u-boot-marvell] mmc: mv_sdhci: call mmc_of_parse()

[Jaehoon Chung]

Dear Marek,

On 2/5/21 1:38 AM, Marek Beh?n wrote:
> On Thu, 4 Feb 2021 07:18:23 +0900
> Jaehoon Chung <jh80.chung@samsung.com> wrote:
> 
>>> Commit da18c62b6e6a causes the regression. The Fixes tag, as I
>>> understand, should link to commit with which the regression first
>>> occured, so that if someone wanted to backport my patch to previous
>>> version of U-Boot, they would know that they only have to do it if
>>> their repository contains that commit.  
>>
>> Well, i don't think so. This patch is not a bug fix patch about
>> commit da18c62b6e6a. Just mv_sdhci driver didn't use mmc_of_parse()
>> before. (If someone set to MMC_NON_REMOVABLE capabilities in
>> host->cap, it should be working.)
>>
>> Not need to add *Fixes*. Its commit is a generic callback function.
>> And mmc_of_parse() is also generic function to use Driver-model.
> 
> Lets apply Baruch's version. I don't really care if the Fixes tag is
> there or not...

I don't have any objection with your patch. It was just my opinion. 
But it needs to choose one of them. Yours and Baruch's patch are same. 

If you want to add Fixes tags, i think that it has to add in Baruch's patch.
Because his patch was firstly posted than yours. 

Best Regards,
Jaehoon Chung

> 
> Marek
> 

[PATCH u-boot-marvell] mmc: mv_sdhci: call mmc_of_parse()

[Stefan Roese]

On 04.02.21 17:38, Marek Beh?n wrote:
> On Thu, 4 Feb 2021 07:18:23 +0900
> Jaehoon Chung <jh80.chung@samsung.com> wrote:
> 
>>> Commit da18c62b6e6a causes the regression. The Fixes tag, as I
>>> understand, should link to commit with which the regression first
>>> occured, so that if someone wanted to backport my patch to previous
>>> version of U-Boot, they would know that they only have to do it if
>>> their repository contains that commit.
>>
>> Well, i don't think so. This patch is not a bug fix patch about
>> commit da18c62b6e6a. Just mv_sdhci driver didn't use mmc_of_parse()
>> before. (If someone set to MMC_NON_REMOVABLE capabilities in
>> host->cap, it should be working.)
>>
>> Not need to add *Fixes*. Its commit is a generic callback function.
>> And mmc_of_parse() is also generic function to use Driver-model.
> 
> Lets apply Baruch's version. I don't really care if the Fixes tag is
> there or not...

I'll pull in Baruch's patch now.

Thanks,
Stefan