[PATCH v2 06/14] mmc: dw_mmc: simplify optional reset handling

[PATCH v2 06/14] mmc: dw_mmc: simplify optional reset handling

[Philipp Zabel]

As of commit bb475230b8e5 ("reset: make optional functions really
optional"), the reset framework API calls use NULL pointers to describe
optional, non-present reset controls.

This allows to return errors from devm_reset_control_get_optional and to
call reset_control_(de)assert unconditionally.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/mmc/host/dw_mmc.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index a9ac0b4573131..3d62b0a1f81cb 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -2968,10 +2968,8 @@ static struct dw_mci_board *dw_mci_parse_dt(struct dw_mci *host)
 
 	/* find reset controller when exist */
 	pdata->rstc = devm_reset_control_get_optional(dev, "reset");
-	if (IS_ERR(pdata->rstc)) {
-		if (PTR_ERR(pdata->rstc) == -EPROBE_DEFER)
-			return ERR_PTR(-EPROBE_DEFER);
-	}
+	if (IS_ERR(pdata->rstc))
+		return ERR_CAST(pdata->rstc);
 
 	/* find out number of slots supported */
 	of_property_read_u32(np, "num-slots", &pdata->num_slots);
@@ -3100,7 +3098,7 @@ int dw_mci_probe(struct dw_mci *host)
 		}
 	}
 
-	if (!IS_ERR(host->pdata->rstc)) {
+	if (host->pdata->rstc) {
 		reset_control_assert(host->pdata->rstc);
 		usleep_range(10, 50);
 		reset_control_deassert(host->pdata->rstc);
@@ -3257,8 +3255,7 @@ int dw_mci_probe(struct dw_mci *host)
 	if (host->use_dma && host->dma_ops->exit)
 		host->dma_ops->exit(host);
 
-	if (!IS_ERR(host->pdata->rstc))
-		reset_control_assert(host->pdata->rstc);
+	reset_control_assert(host->pdata->rstc);
 
 err_clk_ciu:
 	clk_disable_unprepare(host->ciu_clk);
@@ -3290,8 +3287,7 @@ void dw_mci_remove(struct dw_mci *host)
 	if (host->use_dma && host->dma_ops->exit)
 		host->dma_ops->exit(host);
 
-	if (!IS_ERR(host->pdata->rstc))
-		reset_control_assert(host->pdata->rstc);
+	reset_control_assert(host->pdata->rstc);
 
 	clk_disable_unprepare(host->ciu_clk);
 	clk_disable_unprepare(host->biu_clk);
-- 
2.11.0

Re: [PATCH v2 06/14] mmc: dw_mmc: simplify optional reset handling

[Ulf Hansson]

On 15 March 2017 at 12:31, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> As of commit bb475230b8e5 ("reset: make optional functions really
> optional"), the reset framework API calls use NULL pointers to describe
> optional, non-present reset controls.
>
> This allows to return errors from devm_reset_control_get_optional and to
> call reset_control_(de)assert unconditionally.
>
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>

Thanks, applied for next!

Kind regards
Uffe

> ---
>  drivers/mmc/host/dw_mmc.c | 14 +++++---------
>  1 file changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index a9ac0b4573131..3d62b0a1f81cb 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -2968,10 +2968,8 @@ static struct dw_mci_board *dw_mci_parse_dt(struct dw_mci *host)
>
>         /* find reset controller when exist */
>         pdata->rstc = devm_reset_control_get_optional(dev, "reset");
> -       if (IS_ERR(pdata->rstc)) {
> -               if (PTR_ERR(pdata->rstc) == -EPROBE_DEFER)
> -                       return ERR_PTR(-EPROBE_DEFER);
> -       }
> +       if (IS_ERR(pdata->rstc))
> +               return ERR_CAST(pdata->rstc);
>
>         /* find out number of slots supported */
>         of_property_read_u32(np, "num-slots", &pdata->num_slots);
> @@ -3100,7 +3098,7 @@ int dw_mci_probe(struct dw_mci *host)
>                 }
>         }
>
> -       if (!IS_ERR(host->pdata->rstc)) {
> +       if (host->pdata->rstc) {
>                 reset_control_assert(host->pdata->rstc);
>                 usleep_range(10, 50);
>                 reset_control_deassert(host->pdata->rstc);
> @@ -3257,8 +3255,7 @@ int dw_mci_probe(struct dw_mci *host)
>         if (host->use_dma && host->dma_ops->exit)
>                 host->dma_ops->exit(host);
>
> -       if (!IS_ERR(host->pdata->rstc))
> -               reset_control_assert(host->pdata->rstc);
> +       reset_control_assert(host->pdata->rstc);
>
>  err_clk_ciu:
>         clk_disable_unprepare(host->ciu_clk);
> @@ -3290,8 +3287,7 @@ void dw_mci_remove(struct dw_mci *host)
>         if (host->use_dma && host->dma_ops->exit)
>                 host->dma_ops->exit(host);
>
> -       if (!IS_ERR(host->pdata->rstc))
> -               reset_control_assert(host->pdata->rstc);
> +       reset_control_assert(host->pdata->rstc);
>
>         clk_disable_unprepare(host->ciu_clk);
>         clk_disable_unprepare(host->biu_clk);
> --
> 2.11.0
>

Re: [PATCH v2 06/14] mmc: dw_mmc: simplify optional reset handling

[Andrzej Hajda]

Hi Philipp,

Todays next branch does not work with exynos5433-tm2 board.
I guess this patch causes regression. On MMC without reset controller I
get errors:
[    4.938222] dwmmc_exynos 15540000.mshc: platform data not available
[    4.943268] dwmmc_exynos: probe of 15540000.mshc failed with error -22
[    4.950184] dwmmc_exynos 15560000.mshc: platform data not available
[    4.955962] dwmmc_exynos: probe of 15560000.mshc failed with error -22

Commenting out reset controller get and error checks 'fixes' the issue.

On 15.03.2017 12:31, Philipp Zabel wrote:
> As of commit bb475230b8e5 ("reset: make optional functions really
> optional"), the reset framework API calls use NULL pointers to describe
> optional, non-present reset controls.
> 
> This allows to return errors from devm_reset_control_get_optional and to
> call reset_control_(de)assert unconditionally.
> 
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
>  drivers/mmc/host/dw_mmc.c | 14 +++++---------
>  1 file changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index a9ac0b4573131..3d62b0a1f81cb 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -2968,10 +2968,8 @@ static struct dw_mci_board *dw_mci_parse_dt(struct dw_mci *host)
>  
>  	/* find reset controller when exist */
>  	pdata->rstc = devm_reset_control_get_optional(dev, "reset");
> -	if (IS_ERR(pdata->rstc)) {
> -		if (PTR_ERR(pdata->rstc) == -EPROBE_DEFER)
> -			return ERR_PTR(-EPROBE_DEFER);
> -	}
> +	if (IS_ERR(pdata->rstc))
> +		return ERR_CAST(pdata->rstc);


With three lines above commented out it works.

Regards
Andrzej

Re: [PATCH v2 06/14] mmc: dw_mmc: simplify optional reset handling

[Philipp Zabel]

On Mon, 2017-03-20 at 10:22 +0100, Andrzej Hajda wrote:
> Hi Philipp,
> 
> Todays next branch does not work with exynos5433-tm2 board.
> I guess this patch causes regression. On MMC without reset controller I
> get errors:
> [    4.938222] dwmmc_exynos 15540000.mshc: platform data not available
> [    4.943268] dwmmc_exynos: probe of 15540000.mshc failed with error -22
> [    4.950184] dwmmc_exynos 15560000.mshc: platform data not available
> [    4.955962] dwmmc_exynos: probe of 15560000.mshc failed with error -22
> 
> Commenting out reset controller get and error checks 'fixes' the issue.
> 
> On 15.03.2017 12:31, Philipp Zabel wrote:
> > As of commit bb475230b8e5 ("reset: make optional functions really
> > optional"), the reset framework API calls use NULL pointers to describe
> > optional, non-present reset controls.
> > 
> > This allows to return errors from devm_reset_control_get_optional and to
> > call reset_control_(de)assert unconditionally.
> > 
> > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> > ---
> >  drivers/mmc/host/dw_mmc.c | 14 +++++---------
> >  1 file changed, 5 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> > index a9ac0b4573131..3d62b0a1f81cb 100644
> > --- a/drivers/mmc/host/dw_mmc.c
> > +++ b/drivers/mmc/host/dw_mmc.c
> > @@ -2968,10 +2968,8 @@ static struct dw_mci_board *dw_mci_parse_dt(struct dw_mci *host)
> >  
> >  	/* find reset controller when exist */
> >  	pdata->rstc = devm_reset_control_get_optional(dev, "reset");
> > -	if (IS_ERR(pdata->rstc)) {
> > -		if (PTR_ERR(pdata->rstc) == -EPROBE_DEFER)
> > -			return ERR_PTR(-EPROBE_DEFER);
> > -	}
> > +	if (IS_ERR(pdata->rstc))
> > +		return ERR_CAST(pdata->rstc);
> 
> 
> With three lines above commented out it works.

So devm_reset_control_get_optional returns -EINVAL. Why?

The mshc@15560000 node is compatible to "samsung,exynos7-dw-mshc-smu",
so that's dw_mmc-exynos.c calling dw_mci_pltfm_register, which then
calls dw_mci_probe, passing the original platform device as
host->dev = &pdev->dev, and I expect __of_reset_control_get being called
with a valid DT node (dev->of_node).
Since id is set to "reset", of_property_match_string(node,
"reset-names", id) should then be called and return -EINVAL because the
"reset-names" property does not exist. Then __of_reset_control_get
should return NULL because optional == true.
Some of this obviously doesn't happen, where am I wrong?

regards
Philipp

Re: [PATCH v2 06/14] mmc: dw_mmc: simplify optional reset handling

[Andrzej Hajda]

Hi Philipp,

On 20.03.2017 10:53, Philipp Zabel wrote:
> On Mon, 2017-03-20 at 10:22 +0100, Andrzej Hajda wrote:
>> Hi Philipp,
>>
>> Todays next branch does not work with exynos5433-tm2 board.
>> I guess this patch causes regression. On MMC without reset controller I
>> get errors:
>> [    4.938222] dwmmc_exynos 15540000.mshc: platform data not available
>> [    4.943268] dwmmc_exynos: probe of 15540000.mshc failed with error -22
>> [    4.950184] dwmmc_exynos 15560000.mshc: platform data not available
>> [    4.955962] dwmmc_exynos: probe of 15560000.mshc failed with error -22
>>
>> Commenting out reset controller get and error checks 'fixes' the issue.
>>
>> On 15.03.2017 12:31, Philipp Zabel wrote:
>>> As of commit bb475230b8e5 ("reset: make optional functions really
>>> optional"), the reset framework API calls use NULL pointers to describe
>>> optional, non-present reset controls.
>>>
>>> This allows to return errors from devm_reset_control_get_optional and to
>>> call reset_control_(de)assert unconditionally.
>>>
>>> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
>>> ---
>>>  drivers/mmc/host/dw_mmc.c | 14 +++++---------
>>>  1 file changed, 5 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>>> index a9ac0b4573131..3d62b0a1f81cb 100644
>>> --- a/drivers/mmc/host/dw_mmc.c
>>> +++ b/drivers/mmc/host/dw_mmc.c
>>> @@ -2968,10 +2968,8 @@ static struct dw_mci_board *dw_mci_parse_dt(struct dw_mci *host)
>>>  
>>>  	/* find reset controller when exist */
>>>  	pdata->rstc = devm_reset_control_get_optional(dev, "reset");
>>> -	if (IS_ERR(pdata->rstc)) {
>>> -		if (PTR_ERR(pdata->rstc) == -EPROBE_DEFER)
>>> -			return ERR_PTR(-EPROBE_DEFER);
>>> -	}
>>> +	if (IS_ERR(pdata->rstc))
>>> +		return ERR_CAST(pdata->rstc);
>>
>> With three lines above commented out it works.
> So devm_reset_control_get_optional returns -EINVAL. Why?
>
> The mshc@15560000 node is compatible to "samsung,exynos7-dw-mshc-smu",
> so that's dw_mmc-exynos.c calling dw_mci_pltfm_register, which then
> calls dw_mci_probe, passing the original platform device as
> host->dev = &pdev->dev, and I expect __of_reset_control_get being called
> with a valid DT node (dev->of_node).
> Since id is set to "reset", of_property_match_string(node,
> "reset-names", id) should then be called and return -EINVAL because the
> "reset-names" property does not exist. Then __of_reset_control_get
> should return NULL because optional == true.
> Some of this obviously doesn't happen, where am I wrong?


When RESET_CONTROLLER is not enabled dummy stubs return -ENOSUPP error [1].

[1]: http://lxr.free-electrons.com/source/include/linux/reset.h#L77

Regards
Andrzej




>
> regards
> Philipp
>
>
>
>

Re: [PATCH v2 06/14] mmc: dw_mmc: simplify optional reset handling

[Philipp Zabel]

Hi Andrzej,

On Mon, 2017-03-20 at 11:03 +0100, Andrzej Hajda wrote:
> Hi Philipp,
> 
> On 20.03.2017 10:53, Philipp Zabel wrote:
> > On Mon, 2017-03-20 at 10:22 +0100, Andrzej Hajda wrote:
> >> Hi Philipp,
> >>
> >> Todays next branch does not work with exynos5433-tm2 board.
> >> I guess this patch causes regression. On MMC without reset controller I
> >> get errors:
> >> [    4.938222] dwmmc_exynos 15540000.mshc: platform data not available
> >> [    4.943268] dwmmc_exynos: probe of 15540000.mshc failed with error -22

I was thrown off by this. Should maybe dw_mci_probe return the error
value reported by dw_mci_parse_dt instead of always returning -EINVAL?

> >> [    4.950184] dwmmc_exynos 15560000.mshc: platform data not available
> >> [    4.955962] dwmmc_exynos: probe of 15560000.mshc failed with error -22
> >>
> >> Commenting out reset controller get and error checks 'fixes' the issue.
> >>
> >> On 15.03.2017 12:31, Philipp Zabel wrote:
> >>> As of commit bb475230b8e5 ("reset: make optional functions really
> >>> optional"), the reset framework API calls use NULL pointers to describe
> >>> optional, non-present reset controls.
> >>>
> >>> This allows to return errors from devm_reset_control_get_optional and to
> >>> call reset_control_(de)assert unconditionally.
> >>>
> >>> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> >>> ---
> >>>  drivers/mmc/host/dw_mmc.c | 14 +++++---------
> >>>  1 file changed, 5 insertions(+), 9 deletions(-)
> >>>
> >>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> >>> index a9ac0b4573131..3d62b0a1f81cb 100644
> >>> --- a/drivers/mmc/host/dw_mmc.c
> >>> +++ b/drivers/mmc/host/dw_mmc.c
> >>> @@ -2968,10 +2968,8 @@ static struct dw_mci_board *dw_mci_parse_dt(struct dw_mci *host)
> >>>  
> >>>  	/* find reset controller when exist */
> >>>  	pdata->rstc = devm_reset_control_get_optional(dev, "reset");
> >>> -	if (IS_ERR(pdata->rstc)) {
> >>> -		if (PTR_ERR(pdata->rstc) == -EPROBE_DEFER)
> >>> -			return ERR_PTR(-EPROBE_DEFER);
> >>> -	}
> >>> +	if (IS_ERR(pdata->rstc))
> >>> +		return ERR_CAST(pdata->rstc);
> >>
> >> With three lines above commented out it works.
> > So devm_reset_control_get_optional returns -EINVAL. Why?
> >
> > The mshc@15560000 node is compatible to "samsung,exynos7-dw-mshc-smu",
> > so that's dw_mmc-exynos.c calling dw_mci_pltfm_register, which then
> > calls dw_mci_probe, passing the original platform device as
> > host->dev = &pdev->dev, and I expect __of_reset_control_get being called
> > with a valid DT node (dev->of_node).
> > Since id is set to "reset", of_property_match_string(node,
> > "reset-names", id) should then be called and return -EINVAL because the
> > "reset-names" property does not exist. Then __of_reset_control_get
> > should return NULL because optional == true.
> > Some of this obviously doesn't happen, where am I wrong?
> 
> 
> When RESET_CONTROLLER is not enabled dummy stubs return -ENOSUPP error [1].
> 
> [1]: http://lxr.free-electrons.com/source/include/linux/reset.h#L77

Thanks, I suppose that issue should be fixed by:

----------8<----------
diff --git a/include/linux/reset.h b/include/linux/reset.h
index 86b4ed75359e8..c905ff1c21ec6 100644
--- a/include/linux/reset.h
+++ b/include/linux/reset.h
@@ -74,14 +74,14 @@ static inline struct reset_control *__of_reset_control_get(
 					const char *id, int index, bool shared,
 					bool optional)
 {
-	return ERR_PTR(-ENOTSUPP);
+	return optional ? NULL : ERR_PTR(-ENOTSUPP);
 }
 
 static inline struct reset_control *__devm_reset_control_get(
 					struct device *dev, const char *id,
 					int index, bool shared, bool optional)
 {
-	return ERR_PTR(-ENOTSUPP);
+	return optional ? NULL : ERR_PTR(-ENOTSUPP);
 }
 
 #endif /* CONFIG_RESET_CONTROLLER */
---------->8----------

regards
Philipp

Re: [PATCH v2 06/14] mmc: dw_mmc: simplify optional reset handling

[Andrzej Hajda]

On 20.03.2017 11:27, Philipp Zabel wrote:
> Hi Andrzej,
>
> On Mon, 2017-03-20 at 11:03 +0100, Andrzej Hajda wrote:
>> Hi Philipp,
>>
>> On 20.03.2017 10:53, Philipp Zabel wrote:
>>> On Mon, 2017-03-20 at 10:22 +0100, Andrzej Hajda wrote:
>>>> Hi Philipp,
>>>>
>>>> Todays next branch does not work with exynos5433-tm2 board.
>>>> I guess this patch causes regression. On MMC without reset controller I
>>>> get errors:
>>>> [    4.938222] dwmmc_exynos 15540000.mshc: platform data not available
>>>> [    4.943268] dwmmc_exynos: probe of 15540000.mshc failed with error -22
> I was thrown off by this. Should maybe dw_mci_probe return the error
> value reported by dw_mci_parse_dt instead of always returning -EINVAL?
>
>>>> [    4.950184] dwmmc_exynos 15560000.mshc: platform data not available
>>>> [    4.955962] dwmmc_exynos: probe of 15560000.mshc failed with error -22
>>>>
>>>> Commenting out reset controller get and error checks 'fixes' the issue.
>>>>
>>>> On 15.03.2017 12:31, Philipp Zabel wrote:
>>>>> As of commit bb475230b8e5 ("reset: make optional functions really
>>>>> optional"), the reset framework API calls use NULL pointers to describe
>>>>> optional, non-present reset controls.
>>>>>
>>>>> This allows to return errors from devm_reset_control_get_optional and to
>>>>> call reset_control_(de)assert unconditionally.
>>>>>
>>>>> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
>>>>> ---
>>>>>  drivers/mmc/host/dw_mmc.c | 14 +++++---------
>>>>>  1 file changed, 5 insertions(+), 9 deletions(-)
>>>>>
>>>>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>>>>> index a9ac0b4573131..3d62b0a1f81cb 100644
>>>>> --- a/drivers/mmc/host/dw_mmc.c
>>>>> +++ b/drivers/mmc/host/dw_mmc.c
>>>>> @@ -2968,10 +2968,8 @@ static struct dw_mci_board *dw_mci_parse_dt(struct dw_mci *host)
>>>>>  
>>>>>  	/* find reset controller when exist */
>>>>>  	pdata->rstc = devm_reset_control_get_optional(dev, "reset");
>>>>> -	if (IS_ERR(pdata->rstc)) {
>>>>> -		if (PTR_ERR(pdata->rstc) == -EPROBE_DEFER)
>>>>> -			return ERR_PTR(-EPROBE_DEFER);
>>>>> -	}
>>>>> +	if (IS_ERR(pdata->rstc))
>>>>> +		return ERR_CAST(pdata->rstc);
>>>> With three lines above commented out it works.
>>> So devm_reset_control_get_optional returns -EINVAL. Why?
>>>
>>> The mshc@15560000 node is compatible to "samsung,exynos7-dw-mshc-smu",
>>> so that's dw_mmc-exynos.c calling dw_mci_pltfm_register, which then
>>> calls dw_mci_probe, passing the original platform device as
>>> host->dev = &pdev->dev, and I expect __of_reset_control_get being called
>>> with a valid DT node (dev->of_node).
>>> Since id is set to "reset", of_property_match_string(node,
>>> "reset-names", id) should then be called and return -EINVAL because the
>>> "reset-names" property does not exist. Then __of_reset_control_get
>>> should return NULL because optional == true.
>>> Some of this obviously doesn't happen, where am I wrong?
>>
>> When RESET_CONTROLLER is not enabled dummy stubs return -ENOSUPP error [1].
>>
>> [1]: http://lxr.free-electrons.com/source/include/linux/reset.h#L77
> Thanks, I suppose that issue should be fixed by:
>
> ----------8<----------
> diff --git a/include/linux/reset.h b/include/linux/reset.h
> index 86b4ed75359e8..c905ff1c21ec6 100644
> --- a/include/linux/reset.h
> +++ b/include/linux/reset.h
> @@ -74,14 +74,14 @@ static inline struct reset_control *__of_reset_control_get(
>  					const char *id, int index, bool shared,
>  					bool optional)
>  {
> -	return ERR_PTR(-ENOTSUPP);
> +	return optional ? NULL : ERR_PTR(-ENOTSUPP);
>  }
>  
>  static inline struct reset_control *__devm_reset_control_get(
>  					struct device *dev, const char *id,
>  					int index, bool shared, bool optional)
>  {
> -	return ERR_PTR(-ENOTSUPP);
> +	return optional ? NULL : ERR_PTR(-ENOTSUPP);
>  }
>  
>  #endif /* CONFIG_RESET_CONTROLLER */
> ---------->8----------

In dw_mmc.c file there are also unconditional calls to
reset_control_assert, with disabled RESET_CONTROLLER it will cause
unexpected WARNs.
Anyway if you change reset API as above I think you should remove all
warns from reset stubs, because NULL reset is valid, but these warns are
there for reason - contradiction.

Regards
Andrzej

>
> regards
> Philipp
>
>
>
>

Re: [PATCH v2 06/14] mmc: dw_mmc: simplify optional reset handling

[Philipp Zabel]

On Mon, 2017-03-20 at 11:49 +0100, Andrzej Hajda wrote:
> On 20.03.2017 11:27, Philipp Zabel wrote:
[...]
> > diff --git a/include/linux/reset.h b/include/linux/reset.h
> > index 86b4ed75359e8..c905ff1c21ec6 100644
> > --- a/include/linux/reset.h
> > +++ b/include/linux/reset.h
> > @@ -74,14 +74,14 @@ static inline struct reset_control *__of_reset_control_get(
> >  					const char *id, int index, bool shared,
> >  					bool optional)
> >  {
> > -	return ERR_PTR(-ENOTSUPP);
> > +	return optional ? NULL : ERR_PTR(-ENOTSUPP);
> >  }
> >  
> >  static inline struct reset_control *__devm_reset_control_get(
> >  					struct device *dev, const char *id,
> >  					int index, bool shared, bool optional)
> >  {
> > -	return ERR_PTR(-ENOTSUPP);
> > +	return optional ? NULL : ERR_PTR(-ENOTSUPP);
> >  }
> >  
> >  #endif /* CONFIG_RESET_CONTROLLER */
> > ---------->8----------
> 
> In dw_mmc.c file there are also unconditional calls to
> reset_control_assert, with disabled RESET_CONTROLLER it will cause
> unexpected WARNs.
> Anyway if you change reset API as above I think you should remove all
> warns from reset stubs, because NULL reset is valid, but these warns are
> there for reason - contradiction.

You are right, I have to let go of those, too.

regards
Philipp

Re: [PATCH v2 06/14] mmc: dw_mmc: simplify optional reset handling

[Ulf Hansson]

On 20 March 2017 at 12:00, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> On Mon, 2017-03-20 at 11:49 +0100, Andrzej Hajda wrote:
>> On 20.03.2017 11:27, Philipp Zabel wrote:
> [...]
>> > diff --git a/include/linux/reset.h b/include/linux/reset.h
>> > index 86b4ed75359e8..c905ff1c21ec6 100644
>> > --- a/include/linux/reset.h
>> > +++ b/include/linux/reset.h
>> > @@ -74,14 +74,14 @@ static inline struct reset_control *__of_reset_control_get(
>> >                                     const char *id, int index, bool shared,
>> >                                     bool optional)
>> >  {
>> > -   return ERR_PTR(-ENOTSUPP);
>> > +   return optional ? NULL : ERR_PTR(-ENOTSUPP);
>> >  }
>> >
>> >  static inline struct reset_control *__devm_reset_control_get(
>> >                                     struct device *dev, const char *id,
>> >                                     int index, bool shared, bool optional)
>> >  {
>> > -   return ERR_PTR(-ENOTSUPP);
>> > +   return optional ? NULL : ERR_PTR(-ENOTSUPP);
>> >  }
>> >
>> >  #endif /* CONFIG_RESET_CONTROLLER */
>> > ---------->8----------
>>
>> In dw_mmc.c file there are also unconditional calls to
>> reset_control_assert, with disabled RESET_CONTROLLER it will cause
>> unexpected WARNs.
>> Anyway if you change reset API as above I think you should remove all
>> warns from reset stubs, because NULL reset is valid, but these warns are
>> there for reason - contradiction.
>
> You are right, I have to let go of those, too.
>
> regards
> Philipp
>

Re: [PATCH v2 06/14] mmc: dw_mmc: simplify optional reset handling

[Ulf Hansson]

On 20 March 2017 at 12:00, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> On Mon, 2017-03-20 at 11:49 +0100, Andrzej Hajda wrote:
>> On 20.03.2017 11:27, Philipp Zabel wrote:
> [...]
>> > diff --git a/include/linux/reset.h b/include/linux/reset.h
>> > index 86b4ed75359e8..c905ff1c21ec6 100644
>> > --- a/include/linux/reset.h
>> > +++ b/include/linux/reset.h
>> > @@ -74,14 +74,14 @@ static inline struct reset_control *__of_reset_control_get(
>> >                                     const char *id, int index, bool shared,
>> >                                     bool optional)
>> >  {
>> > -   return ERR_PTR(-ENOTSUPP);
>> > +   return optional ? NULL : ERR_PTR(-ENOTSUPP);
>> >  }
>> >
>> >  static inline struct reset_control *__devm_reset_control_get(
>> >                                     struct device *dev, const char *id,
>> >                                     int index, bool shared, bool optional)
>> >  {
>> > -   return ERR_PTR(-ENOTSUPP);
>> > +   return optional ? NULL : ERR_PTR(-ENOTSUPP);
>> >  }
>> >
>> >  #endif /* CONFIG_RESET_CONTROLLER */
>> > ---------->8----------
>>
>> In dw_mmc.c file there are also unconditional calls to
>> reset_control_assert, with disabled RESET_CONTROLLER it will cause
>> unexpected WARNs.
>> Anyway if you change reset API as above I think you should remove all
>> warns from reset stubs, because NULL reset is valid, but these warns are
>> there for reason - contradiction.
>
> You are right, I have to let go of those, too.


Until fixed, I have dropped the three changes from my next branch
related to this. Please re-post when fixed.

Kind regards
Uffe

>
> regards
> Philipp
>

Re: [PATCH v2 06/14] mmc: dw_mmc: simplify optional reset handling

[Jaehoon Chung]

Hi All,

On 03/21/2017 03:10 AM, Ulf Hansson wrote:
> On 20 March 2017 at 12:00, Philipp Zabel <p.zabel@pengutronix.de> wrote:
>> On Mon, 2017-03-20 at 11:49 +0100, Andrzej Hajda wrote:
>>> On 20.03.2017 11:27, Philipp Zabel wrote:
>> [...]
>>>> diff --git a/include/linux/reset.h b/include/linux/reset.h
>>>> index 86b4ed75359e8..c905ff1c21ec6 100644
>>>> --- a/include/linux/reset.h
>>>> +++ b/include/linux/reset.h
>>>> @@ -74,14 +74,14 @@ static inline struct reset_control *__of_reset_control_get(
>>>>                                     const char *id, int index, bool shared,
>>>>                                     bool optional)
>>>>  {
>>>> -   return ERR_PTR(-ENOTSUPP);
>>>> +   return optional ? NULL : ERR_PTR(-ENOTSUPP);
>>>>  }
>>>>
>>>>  static inline struct reset_control *__devm_reset_control_get(
>>>>                                     struct device *dev, const char *id,
>>>>                                     int index, bool shared, bool optional)
>>>>  {
>>>> -   return ERR_PTR(-ENOTSUPP);
>>>> +   return optional ? NULL : ERR_PTR(-ENOTSUPP);
>>>>  }
>>>>
>>>>  #endif /* CONFIG_RESET_CONTROLLER */
>>>> ---------->8----------
>>>
>>> In dw_mmc.c file there are also unconditional calls to
>>> reset_control_assert, with disabled RESET_CONTROLLER it will cause
>>> unexpected WARNs.
>>> Anyway if you change reset API as above I think you should remove all
>>> warns from reset stubs, because NULL reset is valid, but these warns are
>>> there for reason - contradiction.
>>
>> You are right, I have to let go of those, too.
> 
> 
> Until fixed, I have dropped the three changes from my next branch
> related to this. Please re-post when fixed.

I missed this patch. If resend the patch, i will check.

Best Regards,
Jaehoon Chung

> 
> Kind regards
> Uffe
> 
>>
>> regards
>> Philipp
>>
> 
> 
>