2021-06-11 08:36:32

by Srinivas Kandagatla

[permalink] [raw]
Subject: [PATCH 4/9] nvmem: sprd: Fix an error message

From: Christophe JAILLET <[email protected]>

'ret' is known to be 0 here.
The expected error status is stored in 'status', so use it instead.

Also change %d in %u, because status is an u32, not a int.

Fixes: 096030e7f449 ("nvmem: sprd: Add Spreadtrum SoCs eFuse support")
Signed-off-by: Christophe JAILLET <[email protected]>
Acked-by: Chunyan Zhang <[email protected]>
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
drivers/nvmem/sprd-efuse.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nvmem/sprd-efuse.c b/drivers/nvmem/sprd-efuse.c
index 5d394559edf2..e3e721d4c205 100644
--- a/drivers/nvmem/sprd-efuse.c
+++ b/drivers/nvmem/sprd-efuse.c
@@ -234,7 +234,7 @@ static int sprd_efuse_raw_prog(struct sprd_efuse *efuse, u32 blk, bool doub,
status = readl(efuse->base + SPRD_EFUSE_ERR_FLAG);
if (status) {
dev_err(efuse->dev,
- "write error status %d of block %d\n", ret, blk);
+ "write error status %u of block %d\n", status, blk);

writel(SPRD_EFUSE_ERR_CLR_MASK,
efuse->base + SPRD_EFUSE_ERR_CLR);
--
2.21.0


2021-06-11 08:59:12

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 4/9] nvmem: sprd: Fix an error message

On Fri, Jun 11, 2021 at 09:33:43AM +0100, Srinivas Kandagatla wrote:
> From: Christophe JAILLET <[email protected]>
>
> 'ret' is known to be 0 here.
> The expected error status is stored in 'status', so use it instead.
>
> Also change %d in %u, because status is an u32, not a int.
>
> Fixes: 096030e7f449 ("nvmem: sprd: Add Spreadtrum SoCs eFuse support")
> Signed-off-by: Christophe JAILLET <[email protected]>
> Acked-by: Chunyan Zhang <[email protected]>
> Signed-off-by: Srinivas Kandagatla <[email protected]>
> ---
> drivers/nvmem/sprd-efuse.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/nvmem/sprd-efuse.c b/drivers/nvmem/sprd-efuse.c
> index 5d394559edf2..e3e721d4c205 100644
> --- a/drivers/nvmem/sprd-efuse.c
> +++ b/drivers/nvmem/sprd-efuse.c
> @@ -234,7 +234,7 @@ static int sprd_efuse_raw_prog(struct sprd_efuse *efuse, u32 blk, bool doub,
> status = readl(efuse->base + SPRD_EFUSE_ERR_FLAG);
> if (status) {
> dev_err(efuse->dev,
> - "write error status %d of block %d\n", ret, blk);
> + "write error status %u of block %d\n", status, blk);

Shouldn't this be %pe and not %u?

thanks,

greg k-h

2021-06-11 09:09:22

by Srinivas Kandagatla

[permalink] [raw]
Subject: Re: [PATCH 4/9] nvmem: sprd: Fix an error message



On 11/06/2021 09:56, Greg KH wrote:
> On Fri, Jun 11, 2021 at 09:33:43AM +0100, Srinivas Kandagatla wrote:
>> From: Christophe JAILLET <[email protected]>
>>
>> 'ret' is known to be 0 here.
>> The expected error status is stored in 'status', so use it instead.
>>
>> Also change %d in %u, because status is an u32, not a int.
>>
>> Fixes: 096030e7f449 ("nvmem: sprd: Add Spreadtrum SoCs eFuse support")
>> Signed-off-by: Christophe JAILLET <[email protected]>
>> Acked-by: Chunyan Zhang <[email protected]>
>> Signed-off-by: Srinivas Kandagatla <[email protected]>
>> ---
>> drivers/nvmem/sprd-efuse.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/nvmem/sprd-efuse.c b/drivers/nvmem/sprd-efuse.c
>> index 5d394559edf2..e3e721d4c205 100644
>> --- a/drivers/nvmem/sprd-efuse.c
>> +++ b/drivers/nvmem/sprd-efuse.c
>> @@ -234,7 +234,7 @@ static int sprd_efuse_raw_prog(struct sprd_efuse *efuse, u32 blk, bool doub,
>> status = readl(efuse->base + SPRD_EFUSE_ERR_FLAG);
>> if (status) {
>> dev_err(efuse->dev,
>> - "write error status %d of block %d\n", ret, blk);
>> + "write error status %u of block %d\n", status, blk);
>
> Shouldn't this be %pe and not %u?

This is not error pointer its status value read back from a register.

I think %u should be good here.

--srini
>
> thanks,
>
> greg k-h
>

2021-06-11 09:19:41

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 4/9] nvmem: sprd: Fix an error message

On Fri, Jun 11, 2021 at 10:05:40AM +0100, Srinivas Kandagatla wrote:
>
>
> On 11/06/2021 09:56, Greg KH wrote:
> > On Fri, Jun 11, 2021 at 09:33:43AM +0100, Srinivas Kandagatla wrote:
> > > From: Christophe JAILLET <[email protected]>
> > >
> > > 'ret' is known to be 0 here.
> > > The expected error status is stored in 'status', so use it instead.
> > >
> > > Also change %d in %u, because status is an u32, not a int.
> > >
> > > Fixes: 096030e7f449 ("nvmem: sprd: Add Spreadtrum SoCs eFuse support")
> > > Signed-off-by: Christophe JAILLET <[email protected]>
> > > Acked-by: Chunyan Zhang <[email protected]>
> > > Signed-off-by: Srinivas Kandagatla <[email protected]>
> > > ---
> > > drivers/nvmem/sprd-efuse.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/nvmem/sprd-efuse.c b/drivers/nvmem/sprd-efuse.c
> > > index 5d394559edf2..e3e721d4c205 100644
> > > --- a/drivers/nvmem/sprd-efuse.c
> > > +++ b/drivers/nvmem/sprd-efuse.c
> > > @@ -234,7 +234,7 @@ static int sprd_efuse_raw_prog(struct sprd_efuse *efuse, u32 blk, bool doub,
> > > status = readl(efuse->base + SPRD_EFUSE_ERR_FLAG);
> > > if (status) {
> > > dev_err(efuse->dev,
> > > - "write error status %d of block %d\n", ret, blk);
> > > + "write error status %u of block %d\n", status, blk);
> >
> > Shouldn't this be %pe and not %u?
>
> This is not error pointer its status value read back from a register.
>
> I think %u should be good here.

Argh, you are right, my fault. For some reason I thought this worked
for integers as well. Don't we have such a printk modifier somewhere to
turn error values into strings? Let me dig...


thanks

greg k-h

2021-06-11 09:47:21

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 4/9] nvmem: sprd: Fix an error message

On Fri, Jun 11, 2021 at 11:17:50AM +0200, Greg KH wrote:
> On Fri, Jun 11, 2021 at 10:05:40AM +0100, Srinivas Kandagatla wrote:
> >
> >
> > On 11/06/2021 09:56, Greg KH wrote:
> > > On Fri, Jun 11, 2021 at 09:33:43AM +0100, Srinivas Kandagatla wrote:
> > > > From: Christophe JAILLET <[email protected]>
> > > >
> > > > 'ret' is known to be 0 here.
> > > > The expected error status is stored in 'status', so use it instead.
> > > >
> > > > Also change %d in %u, because status is an u32, not a int.
> > > >
> > > > Fixes: 096030e7f449 ("nvmem: sprd: Add Spreadtrum SoCs eFuse support")
> > > > Signed-off-by: Christophe JAILLET <[email protected]>
> > > > Acked-by: Chunyan Zhang <[email protected]>
> > > > Signed-off-by: Srinivas Kandagatla <[email protected]>
> > > > ---
> > > > drivers/nvmem/sprd-efuse.c | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/nvmem/sprd-efuse.c b/drivers/nvmem/sprd-efuse.c
> > > > index 5d394559edf2..e3e721d4c205 100644
> > > > --- a/drivers/nvmem/sprd-efuse.c
> > > > +++ b/drivers/nvmem/sprd-efuse.c
> > > > @@ -234,7 +234,7 @@ static int sprd_efuse_raw_prog(struct sprd_efuse *efuse, u32 blk, bool doub,
> > > > status = readl(efuse->base + SPRD_EFUSE_ERR_FLAG);
> > > > if (status) {
> > > > dev_err(efuse->dev,
> > > > - "write error status %d of block %d\n", ret, blk);
> > > > + "write error status %u of block %d\n", status, blk);
> > >
> > > Shouldn't this be %pe and not %u?
> >
> > This is not error pointer its status value read back from a register.
> >
> > I think %u should be good here.
>
> Argh, you are right, my fault. For some reason I thought this worked
> for integers as well. Don't we have such a printk modifier somewhere to
> turn error values into strings? Let me dig...

Ah, errname() will do it.

Looks like no one uses it, so nevermind, sorry for the noise. I'll go
apply this one now.

thanks,

greg k-h

2021-06-11 09:52:07

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 4/9] nvmem: sprd: Fix an error message

On Fri, Jun 11, 2021 at 11:45:25AM +0200, Greg KH wrote:
> On Fri, Jun 11, 2021 at 11:17:50AM +0200, Greg KH wrote:
> > On Fri, Jun 11, 2021 at 10:05:40AM +0100, Srinivas Kandagatla wrote:
> > >
> > >
> > > On 11/06/2021 09:56, Greg KH wrote:
> > > > On Fri, Jun 11, 2021 at 09:33:43AM +0100, Srinivas Kandagatla wrote:
> > > > > From: Christophe JAILLET <[email protected]>
> > > > >
> > > > > 'ret' is known to be 0 here.
> > > > > The expected error status is stored in 'status', so use it instead.
> > > > >
> > > > > Also change %d in %u, because status is an u32, not a int.
> > > > >
> > > > > Fixes: 096030e7f449 ("nvmem: sprd: Add Spreadtrum SoCs eFuse support")
> > > > > Signed-off-by: Christophe JAILLET <[email protected]>
> > > > > Acked-by: Chunyan Zhang <[email protected]>
> > > > > Signed-off-by: Srinivas Kandagatla <[email protected]>
> > > > > ---
> > > > > drivers/nvmem/sprd-efuse.c | 2 +-
> > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/nvmem/sprd-efuse.c b/drivers/nvmem/sprd-efuse.c
> > > > > index 5d394559edf2..e3e721d4c205 100644
> > > > > --- a/drivers/nvmem/sprd-efuse.c
> > > > > +++ b/drivers/nvmem/sprd-efuse.c
> > > > > @@ -234,7 +234,7 @@ static int sprd_efuse_raw_prog(struct sprd_efuse *efuse, u32 blk, bool doub,
> > > > > status = readl(efuse->base + SPRD_EFUSE_ERR_FLAG);
> > > > > if (status) {
> > > > > dev_err(efuse->dev,
> > > > > - "write error status %d of block %d\n", ret, blk);
> > > > > + "write error status %u of block %d\n", status, blk);
> > > >
> > > > Shouldn't this be %pe and not %u?
> > >
> > > This is not error pointer its status value read back from a register.
> > >
> > > I think %u should be good here.
> >
> > Argh, you are right, my fault. For some reason I thought this worked
> > for integers as well. Don't we have such a printk modifier somewhere to
> > turn error values into strings? Let me dig...
>
> Ah, errname() will do it.
>
> Looks like no one uses it, so nevermind, sorry for the noise. I'll go
> apply this one now.

Does not apply to my tree :(

2021-06-11 10:11:56

by Srinivas Kandagatla

[permalink] [raw]
Subject: Re: [PATCH 4/9] nvmem: sprd: Fix an error message



On 11/06/2021 10:45, Greg KH wrote:
> On Fri, Jun 11, 2021 at 11:17:50AM +0200, Greg KH wrote:
>> On Fri, Jun 11, 2021 at 10:05:40AM +0100, Srinivas Kandagatla wrote:
>>>
>>>
>>> On 11/06/2021 09:56, Greg KH wrote:
>>>> On Fri, Jun 11, 2021 at 09:33:43AM +0100, Srinivas Kandagatla wrote:
>>>>> From: Christophe JAILLET <[email protected]>
>>>>>
>>>>> 'ret' is known to be 0 here.
>>>>> The expected error status is stored in 'status', so use it instead.
>>>>>
>>>>> Also change %d in %u, because status is an u32, not a int.
>>>>>
>>>>> Fixes: 096030e7f449 ("nvmem: sprd: Add Spreadtrum SoCs eFuse support")
>>>>> Signed-off-by: Christophe JAILLET <[email protected]>
>>>>> Acked-by: Chunyan Zhang <[email protected]>
>>>>> Signed-off-by: Srinivas Kandagatla <[email protected]>
>>>>> ---
>>>>> drivers/nvmem/sprd-efuse.c | 2 +-
>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/nvmem/sprd-efuse.c b/drivers/nvmem/sprd-efuse.c
>>>>> index 5d394559edf2..e3e721d4c205 100644
>>>>> --- a/drivers/nvmem/sprd-efuse.c
>>>>> +++ b/drivers/nvmem/sprd-efuse.c
>>>>> @@ -234,7 +234,7 @@ static int sprd_efuse_raw_prog(struct sprd_efuse *efuse, u32 blk, bool doub,
>>>>> status = readl(efuse->base + SPRD_EFUSE_ERR_FLAG);
>>>>> if (status) {
>>>>> dev_err(efuse->dev,
>>>>> - "write error status %d of block %d\n", ret, blk);
>>>>> + "write error status %u of block %d\n", status, blk);
>>>>
>>>> Shouldn't this be %pe and not %u?
>>>
>>> This is not error pointer its status value read back from a register.
>>>
>>> I think %u should be good here.
>>
>> Argh, you are right, my fault. For some reason I thought this worked
>> for integers as well. Don't we have such a printk modifier somewhere to
>> turn error values into strings? Let me dig...
>
> Ah, errname() will do it.
>
Thanks for looking this up,
looks like errname is more of stringfy rather than translating it in to
proper description like strerror() does.

It would do something like:
errname(EIO) -> "EIO"
errname(-EIO) -> "-EIO"


> Looks like no one uses it, so nevermind, sorry for the noise. I'll go
> apply this one now.


Noticed that It does not apply to your tree, I will resend it along with
other patch.


thanks,
srini
>
> thanks,
>
> greg k-h
>

2021-06-11 10:14:32

by Christophe JAILLET

[permalink] [raw]
Subject: Re: [PATCH 4/9] nvmem: sprd: Fix an error message


Le 11/06/2021 à 11:45, Greg KH a écrit :
> On Fri, Jun 11, 2021 at 11:17:50AM +0200, Greg KH wrote:
>> On Fri, Jun 11, 2021 at 10:05:40AM +0100, Srinivas Kandagatla wrote:
>>>
>>> On 11/06/2021 09:56, Greg KH wrote:
>>>> On Fri, Jun 11, 2021 at 09:33:43AM +0100, Srinivas Kandagatla wrote:
>>>>> From: Christophe JAILLET <[email protected]>
>>>>>
>>>>> 'ret' is known to be 0 here.
>>>>> The expected error status is stored in 'status', so use it instead.
>>>>>
>>>>> Also change %d in %u, because status is an u32, not a int.
>>>>>
>>>>> Fixes: 096030e7f449 ("nvmem: sprd: Add Spreadtrum SoCs eFuse support")
>>>>> Signed-off-by: Christophe JAILLET <[email protected]>
>>>>> Acked-by: Chunyan Zhang <[email protected]>
>>>>> Signed-off-by: Srinivas Kandagatla <[email protected]>
>>>>> ---
>>>>> drivers/nvmem/sprd-efuse.c | 2 +-
>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/nvmem/sprd-efuse.c b/drivers/nvmem/sprd-efuse.c
>>>>> index 5d394559edf2..e3e721d4c205 100644
>>>>> --- a/drivers/nvmem/sprd-efuse.c
>>>>> +++ b/drivers/nvmem/sprd-efuse.c
>>>>> @@ -234,7 +234,7 @@ static int sprd_efuse_raw_prog(struct sprd_efuse *efuse, u32 blk, bool doub,
>>>>> status = readl(efuse->base + SPRD_EFUSE_ERR_FLAG);
>>>>> if (status) {
>>>>> dev_err(efuse->dev,
>>>>> - "write error status %d of block %d\n", ret, blk);
>>>>> + "write error status %u of block %d\n", status, blk);
>>>> Shouldn't this be %pe and not %u?
>>> This is not error pointer its status value read back from a register.
>>>
>>> I think %u should be good here.
>> Argh, you are right, my fault. For some reason I thought this worked
>> for integers as well. Don't we have such a printk modifier somewhere to
>> turn error values into strings? Let me dig...
> Ah, errname() will do it.
>
> Looks like no one uses it, so nevermind, sorry for the noise. I'll go
> apply this one now.
>
> thanks,
>
> greg k-h

Hi,

errname() depends on CONFIG_SYMBOLIC_ERRNAME.
Is this widely used?
If not, using errname() directly would loose the error code.
(note that %pe already deals with it)

Dan Capenter already spoke about a potential %e extension, but I don't
think anyone did anything yet.

CJ

2021-06-11 10:18:55

by Srinivas Kandagatla

[permalink] [raw]
Subject: Re: [PATCH 4/9] nvmem: sprd: Fix an error message



On 11/06/2021 10:48, Greg KH wrote:
> On Fri, Jun 11, 2021 at 11:45:25AM +0200, Greg KH wrote:
>> On Fri, Jun 11, 2021 at 11:17:50AM +0200, Greg KH wrote:
>>> On Fri, Jun 11, 2021 at 10:05:40AM +0100, Srinivas Kandagatla wrote:
>>>>
>>>>
>>>> On 11/06/2021 09:56, Greg KH wrote:
>>>>> On Fri, Jun 11, 2021 at 09:33:43AM +0100, Srinivas Kandagatla wrote:
>>>>>> From: Christophe JAILLET <[email protected]>
>>>>>>
>>>>>> 'ret' is known to be 0 here.
>>>>>> The expected error status is stored in 'status', so use it instead.
>>>>>>
>>>>>> Also change %d in %u, because status is an u32, not a int.
>>>>>>
>>>>>> Fixes: 096030e7f449 ("nvmem: sprd: Add Spreadtrum SoCs eFuse support")
>>>>>> Signed-off-by: Christophe JAILLET <[email protected]>
>>>>>> Acked-by: Chunyan Zhang <[email protected]>
>>>>>> Signed-off-by: Srinivas Kandagatla <[email protected]>
>>>>>> ---
>>>>>> drivers/nvmem/sprd-efuse.c | 2 +-
>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/nvmem/sprd-efuse.c b/drivers/nvmem/sprd-efuse.c
>>>>>> index 5d394559edf2..e3e721d4c205 100644
>>>>>> --- a/drivers/nvmem/sprd-efuse.c
>>>>>> +++ b/drivers/nvmem/sprd-efuse.c
>>>>>> @@ -234,7 +234,7 @@ static int sprd_efuse_raw_prog(struct sprd_efuse *efuse, u32 blk, bool doub,
>>>>>> status = readl(efuse->base + SPRD_EFUSE_ERR_FLAG);
>>>>>> if (status) {
>>>>>> dev_err(efuse->dev,
>>>>>> - "write error status %d of block %d\n", ret, blk);
>>>>>> + "write error status %u of block %d\n", status, blk);
>>>>>
>>>>> Shouldn't this be %pe and not %u?
>>>>
>>>> This is not error pointer its status value read back from a register.
>>>>
>>>> I think %u should be good here.
>>>
>>> Argh, you are right, my fault. For some reason I thought this worked
>>> for integers as well. Don't we have such a printk modifier somewhere to
>>> turn error values into strings? Let me dig...
>>
>> Ah, errname() will do it.
>>
>> Looks like no one uses it, so nevermind, sorry for the noise. I'll go
>> apply this one now.
>
> Does not apply to my tree :(

Looks like its already in your tree since May 7th.

--srini
>

2021-06-11 10:19:01

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 4/9] nvmem: sprd: Fix an error message

On Fri, Jun 11, 2021 at 12:10:58PM +0200, Marion & Christophe JAILLET wrote:
>
> Le 11/06/2021 ? 11:45, Greg KH a ?crit?:
> > On Fri, Jun 11, 2021 at 11:17:50AM +0200, Greg KH wrote:
> > > On Fri, Jun 11, 2021 at 10:05:40AM +0100, Srinivas Kandagatla wrote:
> > > >
> > > > On 11/06/2021 09:56, Greg KH wrote:
> > > > > On Fri, Jun 11, 2021 at 09:33:43AM +0100, Srinivas Kandagatla wrote:
> > > > > > From: Christophe JAILLET <[email protected]>
> > > > > >
> > > > > > 'ret' is known to be 0 here.
> > > > > > The expected error status is stored in 'status', so use it instead.
> > > > > >
> > > > > > Also change %d in %u, because status is an u32, not a int.
> > > > > >
> > > > > > Fixes: 096030e7f449 ("nvmem: sprd: Add Spreadtrum SoCs eFuse support")
> > > > > > Signed-off-by: Christophe JAILLET <[email protected]>
> > > > > > Acked-by: Chunyan Zhang <[email protected]>
> > > > > > Signed-off-by: Srinivas Kandagatla <[email protected]>
> > > > > > ---
> > > > > > drivers/nvmem/sprd-efuse.c | 2 +-
> > > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/drivers/nvmem/sprd-efuse.c b/drivers/nvmem/sprd-efuse.c
> > > > > > index 5d394559edf2..e3e721d4c205 100644
> > > > > > --- a/drivers/nvmem/sprd-efuse.c
> > > > > > +++ b/drivers/nvmem/sprd-efuse.c
> > > > > > @@ -234,7 +234,7 @@ static int sprd_efuse_raw_prog(struct sprd_efuse *efuse, u32 blk, bool doub,
> > > > > > status = readl(efuse->base + SPRD_EFUSE_ERR_FLAG);
> > > > > > if (status) {
> > > > > > dev_err(efuse->dev,
> > > > > > - "write error status %d of block %d\n", ret, blk);
> > > > > > + "write error status %u of block %d\n", status, blk);
> > > > > Shouldn't this be %pe and not %u?
> > > > This is not error pointer its status value read back from a register.
> > > >
> > > > I think %u should be good here.
> > > Argh, you are right, my fault. For some reason I thought this worked
> > > for integers as well. Don't we have such a printk modifier somewhere to
> > > turn error values into strings? Let me dig...
> > Ah, errname() will do it.
> >
> > Looks like no one uses it, so nevermind, sorry for the noise. I'll go
> > apply this one now.
> >
> > thanks,
> >
> > greg k-h
>
> Hi,
>
> errname() depends on CONFIG_SYMBOLIC_ERRNAME.
> Is this widely used?

It is set by default if you enable CONFIG_PRINTK

> If not, using errname() directly would loose the error code.
> (note that %pe already deals with it)
>
> Dan Capenter already spoke about a potential %e extension, but I don't think
> anyone did anything yet.

That would be a fun and simple beginner task for someone to do. Maybe
I'll mention it to this new round of interns that have just started...

thanks,

greg k-h

2021-06-11 10:22:56

by Srinivas Kandagatla

[permalink] [raw]
Subject: Re: [PATCH 4/9] nvmem: sprd: Fix an error message



On 11/06/2021 11:17, Greg KH wrote:
> On Fri, Jun 11, 2021 at 12:10:58PM +0200, Marion & Christophe JAILLET wrote:
>>
>> Le 11/06/2021 à 11:45, Greg KH a écrit :
>>> On Fri, Jun 11, 2021 at 11:17:50AM +0200, Greg KH wrote:
>>>> On Fri, Jun 11, 2021 at 10:05:40AM +0100, Srinivas Kandagatla wrote:
>>>>>
>>>>> On 11/06/2021 09:56, Greg KH wrote:
>>>>>> On Fri, Jun 11, 2021 at 09:33:43AM +0100, Srinivas Kandagatla wrote:
>>>>>>> From: Christophe JAILLET <[email protected]>
>>>>>>>
>>>>>>> 'ret' is known to be 0 here.
>>>>>>> The expected error status is stored in 'status', so use it instead.
>>>>>>>
>>>>>>> Also change %d in %u, because status is an u32, not a int.
>>>>>>>
>>>>>>> Fixes: 096030e7f449 ("nvmem: sprd: Add Spreadtrum SoCs eFuse support")
>>>>>>> Signed-off-by: Christophe JAILLET <[email protected]>
>>>>>>> Acked-by: Chunyan Zhang <[email protected]>
>>>>>>> Signed-off-by: Srinivas Kandagatla <[email protected]>
>>>>>>> ---
>>>>>>> drivers/nvmem/sprd-efuse.c | 2 +-
>>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/drivers/nvmem/sprd-efuse.c b/drivers/nvmem/sprd-efuse.c
>>>>>>> index 5d394559edf2..e3e721d4c205 100644
>>>>>>> --- a/drivers/nvmem/sprd-efuse.c
>>>>>>> +++ b/drivers/nvmem/sprd-efuse.c
>>>>>>> @@ -234,7 +234,7 @@ static int sprd_efuse_raw_prog(struct sprd_efuse *efuse, u32 blk, bool doub,
>>>>>>> status = readl(efuse->base + SPRD_EFUSE_ERR_FLAG);
>>>>>>> if (status) {
>>>>>>> dev_err(efuse->dev,
>>>>>>> - "write error status %d of block %d\n", ret, blk);
>>>>>>> + "write error status %u of block %d\n", status, blk);
>>>>>> Shouldn't this be %pe and not %u?
>>>>> This is not error pointer its status value read back from a register.
>>>>>
>>>>> I think %u should be good here.
>>>> Argh, you are right, my fault. For some reason I thought this worked
>>>> for integers as well. Don't we have such a printk modifier somewhere to
>>>> turn error values into strings? Let me dig...
>>> Ah, errname() will do it.
>>>
>>> Looks like no one uses it, so nevermind, sorry for the noise. I'll go
>>> apply this one now.
>>>
>>> thanks,
>>>
>>> greg k-h
>>
>> Hi,
>>
>> errname() depends on CONFIG_SYMBOLIC_ERRNAME.
>> Is this widely used?
>
> It is set by default if you enable CONFIG_PRINTK
>
>> If not, using errname() directly would loose the error code.
>> (note that %pe already deals with it)
>>
>> Dan Capenter already spoke about a potential %e extension, but I don't think
>> anyone did anything yet.
>
> That would be a fun and simple beginner task for someone to do. Maybe
> I'll mention it to this new round of interns that have just started...

There seems to be some work on this side in the past.

http://lkml.iu.edu/hypermail/linux/kernel/1309.2/01027.html

This should be a good starting point.


--srini
>
> thanks,
>
> greg k-h
>