mmc: core: Fix error return code in sd_read_ext_regs()

  Fix to return a negative error code from the error handling
case instead of 0, as done elsewhere in this function.

Fixes: c784f92769ae ("mmc: core: Read the SD function extension registers for power management")
Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
 drivers/mmc/core/sd.c | 1 +
 1 file changed, 1 insertion(+)
  

On Wed, 30 Nov 2022 at 10:29, Zhen Lei <thunder.leizhen@huawei.com> wrote:
>
> Fix to return a negative error code from the error handling
> case instead of 0, as done elsewhere in this function.
>
> Fixes: c784f92769ae ("mmc: core: Read the SD function extension registers for power management")
> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
> ---
>  drivers/mmc/core/sd.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
> index 3662bf5320ce56d..7b64f76f0179ca8 100644
> --- a/drivers/mmc/core/sd.c
> +++ b/drivers/mmc/core/sd.c
> @@ -1277,6 +1277,7 @@ static int sd_read_ext_regs(struct mmc_card *card)
>         if (rev != 0 || len > 512) {
>                 pr_warn("%s: non-supported SD ext reg layout\n",
>                         mmc_hostname(card->host));
> +               err = -EOPNOTSUPP;

The original intent was to not return an error code. Simply, because
the card remains functional and all but the new features from the SD
function extensions registers can still be used.

Perhaps, we should update the comment a few lines above to better
reflect that this is in-fact what we intend here.

>                 goto out;
>         }
>

Kind regards
Uffe
  

On 2022/11/30 18:04, Ulf Hansson wrote:
> On Wed, 30 Nov 2022 at 10:29, Zhen Lei <thunder.leizhen@huawei.com> wrote:
>>
>> Fix to return a negative error code from the error handling
>> case instead of 0, as done elsewhere in this function.
>>
>> Fixes: c784f92769ae ("mmc: core: Read the SD function extension registers for power management")
>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
>> ---
>>  drivers/mmc/core/sd.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
>> index 3662bf5320ce56d..7b64f76f0179ca8 100644
>> --- a/drivers/mmc/core/sd.c
>> +++ b/drivers/mmc/core/sd.c
>> @@ -1277,6 +1277,7 @@ static int sd_read_ext_regs(struct mmc_card *card)
>>         if (rev != 0 || len > 512) {
>>                 pr_warn("%s: non-supported SD ext reg layout\n",
>>                         mmc_hostname(card->host));
>> +               err = -EOPNOTSUPP;
> 
> The original intent was to not return an error code. Simply, because
> the card remains functional and all but the new features from the SD
> function extensions registers can still be used.

OK, I got it.

> 
> Perhaps, we should update the comment a few lines above to better
> reflect that this is in-fact what we intend here.

How about also add 'warning' to the output? The other two outputs
contain the 'error' keyword.

> 
>>                 goto out;
>>         }
>>
> 
> Kind regards
> Uffe
> .
>
  

On Wed, 30 Nov 2022 at 12:18, Leizhen (ThunderTown)
<thunder.leizhen@huawei.com> wrote:
>
>
>
> On 2022/11/30 18:04, Ulf Hansson wrote:
> > On Wed, 30 Nov 2022 at 10:29, Zhen Lei <thunder.leizhen@huawei.com> wrote:
> >>
> >> Fix to return a negative error code from the error handling
> >> case instead of 0, as done elsewhere in this function.
> >>
> >> Fixes: c784f92769ae ("mmc: core: Read the SD function extension registers for power management")
> >> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
> >> ---
> >>  drivers/mmc/core/sd.c | 1 +
> >>  1 file changed, 1 insertion(+)
> >>
> >> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
> >> index 3662bf5320ce56d..7b64f76f0179ca8 100644
> >> --- a/drivers/mmc/core/sd.c
> >> +++ b/drivers/mmc/core/sd.c
> >> @@ -1277,6 +1277,7 @@ static int sd_read_ext_regs(struct mmc_card *card)
> >>         if (rev != 0 || len > 512) {
> >>                 pr_warn("%s: non-supported SD ext reg layout\n",
> >>                         mmc_hostname(card->host));
> >> +               err = -EOPNOTSUPP;
> >
> > The original intent was to not return an error code. Simply, because
> > the card remains functional and all but the new features from the SD
> > function extensions registers can still be used.
>
> OK, I got it.
>
> >
> > Perhaps, we should update the comment a few lines above to better
> > reflect that this is in-fact what we intend here.
>
> How about also add 'warning' to the output? The other two outputs
> contain the 'error' keyword.

There is already a pr_warn(), or is there something you want to add to that?

>
> >
> >>                 goto out;
> >>         }
> >>

Kind regards
Uffe
  

On 2022/11/30 19:33, Ulf Hansson wrote:
> On Wed, 30 Nov 2022 at 12:18, Leizhen (ThunderTown)
> <thunder.leizhen@huawei.com> wrote:
>>
>>
>>
>> On 2022/11/30 18:04, Ulf Hansson wrote:
>>> On Wed, 30 Nov 2022 at 10:29, Zhen Lei <thunder.leizhen@huawei.com> wrote:
>>>>
>>>> Fix to return a negative error code from the error handling
>>>> case instead of 0, as done elsewhere in this function.
>>>>
>>>> Fixes: c784f92769ae ("mmc: core: Read the SD function extension registers for power management")
>>>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
>>>> ---
>>>>  drivers/mmc/core/sd.c | 1 +
>>>>  1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
>>>> index 3662bf5320ce56d..7b64f76f0179ca8 100644
>>>> --- a/drivers/mmc/core/sd.c
>>>> +++ b/drivers/mmc/core/sd.c
>>>> @@ -1277,6 +1277,7 @@ static int sd_read_ext_regs(struct mmc_card *card)
>>>>         if (rev != 0 || len > 512) {
>>>>                 pr_warn("%s: non-supported SD ext reg layout\n",
>>>>                         mmc_hostname(card->host));
>>>> +               err = -EOPNOTSUPP;
>>>
>>> The original intent was to not return an error code. Simply, because
>>> the card remains functional and all but the new features from the SD
>>> function extensions registers can still be used.
>>
>> OK, I got it.
>>
>>>
>>> Perhaps, we should update the comment a few lines above to better
>>> reflect that this is in-fact what we intend here.
>>
>> How about also add 'warning' to the output? The other two outputs
>> contain the 'error' keyword.
> 
> There is already a pr_warn(), or is there something you want to add to that?

No, I don't have anything else to add.

I just saw the other two error outputs are also use pr_warn(). Maybe they
need to be changed to pr_err().

> 
>>
>>>
>>>>                 goto out;
>>>>         }
>>>>
> 
> Kind regards
> Uffe
> .
>
  

On Wed, 30 Nov 2022 at 12:45, Leizhen (ThunderTown)
<thunder.leizhen@huawei.com> wrote:
>
>
>
> On 2022/11/30 19:33, Ulf Hansson wrote:
> > On Wed, 30 Nov 2022 at 12:18, Leizhen (ThunderTown)
> > <thunder.leizhen@huawei.com> wrote:
> >>
> >>
> >>
> >> On 2022/11/30 18:04, Ulf Hansson wrote:
> >>> On Wed, 30 Nov 2022 at 10:29, Zhen Lei <thunder.leizhen@huawei.com> wrote:
> >>>>
> >>>> Fix to return a negative error code from the error handling
> >>>> case instead of 0, as done elsewhere in this function.
> >>>>
> >>>> Fixes: c784f92769ae ("mmc: core: Read the SD function extension registers for power management")
> >>>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
> >>>> ---
> >>>>  drivers/mmc/core/sd.c | 1 +
> >>>>  1 file changed, 1 insertion(+)
> >>>>
> >>>> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
> >>>> index 3662bf5320ce56d..7b64f76f0179ca8 100644
> >>>> --- a/drivers/mmc/core/sd.c
> >>>> +++ b/drivers/mmc/core/sd.c
> >>>> @@ -1277,6 +1277,7 @@ static int sd_read_ext_regs(struct mmc_card *card)
> >>>>         if (rev != 0 || len > 512) {
> >>>>                 pr_warn("%s: non-supported SD ext reg layout\n",
> >>>>                         mmc_hostname(card->host));
> >>>> +               err = -EOPNOTSUPP;
> >>>
> >>> The original intent was to not return an error code. Simply, because
> >>> the card remains functional and all but the new features from the SD
> >>> function extensions registers can still be used.
> >>
> >> OK, I got it.
> >>
> >>>
> >>> Perhaps, we should update the comment a few lines above to better
> >>> reflect that this is in-fact what we intend here.
> >>
> >> How about also add 'warning' to the output? The other two outputs
> >> contain the 'error' keyword.
> >
> > There is already a pr_warn(), or is there something you want to add to that?
>
> No, I don't have anything else to add.
>
> I just saw the other two error outputs are also use pr_warn(). Maybe they
> need to be changed to pr_err().

Aha, that's a good point! We should change those to pr_err().

Kind regards
Uffe