[PATCH] mtd: blktrans: change blktrans_getgeo rerurn value

[PATCH] mtd: blktrans: change blktrans_getgeo rerurn value

[Wenlin Kang]

Modify function blktrans_getgeo()'s return value to -ENXIO when
dev->tr->getgeo == NULL.

We shouldn't make the return value to 0 when dev->tr->getgeo == NULL,
because the function blktrans_getgeo() has an output value "hd_geometry"
which is usually used by some application, if return 0, it will make some
application get the wrong information.

Signed-off-by: Wenlin Kang <wenlin.kang@windriver.com>
---
 drivers/mtd/mtd_blkdevs.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
index 2b0c5287..f8bb16e 100644
--- a/drivers/mtd/mtd_blkdevs.c
+++ b/drivers/mtd/mtd_blkdevs.c
@@ -273,7 +273,7 @@ static int blktrans_getgeo(struct block_device *bdev, struct hd_geometry *geo)
 	if (!dev->mtd)
 		goto unlock;
 
-	ret = dev->tr->getgeo ? dev->tr->getgeo(dev, geo) : 0;
+	ret = dev->tr->getgeo ? dev->tr->getgeo(dev, geo) : -ENXIO;
 unlock:
 	mutex_unlock(&dev->lock);
 	blktrans_dev_put(dev);
-- 
1.7.9.5

Re: [PATCH] mtd: blktrans: change blktrans_getgeo rerurn value

[Brian Norris]

Hi Wenlin,

In the subject:

s/rerurn/return/

On Wed, May 13, 2015 at 02:29:16PM +0800, Wenlin Kang wrote:
> Modify function blktrans_getgeo()'s return value to -ENXIO when
> dev->tr->getgeo == NULL.
> 
> We shouldn't make the return value to 0 when dev->tr->getgeo == NULL,
> because the function blktrans_getgeo() has an output value "hd_geometry"
> which is usually used by some application, if return 0, it will make some
> application get the wrong information.
> 
> Signed-off-by: Wenlin Kang <wenlin.kang@windriver.com>
> ---
>  drivers/mtd/mtd_blkdevs.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
> index 2b0c5287..f8bb16e 100644
> --- a/drivers/mtd/mtd_blkdevs.c
> +++ b/drivers/mtd/mtd_blkdevs.c
> @@ -273,7 +273,7 @@ static int blktrans_getgeo(struct block_device *bdev, struct hd_geometry *geo)
>  	if (!dev->mtd)
>  		goto unlock;
>  
> -	ret = dev->tr->getgeo ? dev->tr->getgeo(dev, geo) : 0;
> +	ret = dev->tr->getgeo ? dev->tr->getgeo(dev, geo) : -ENXIO;

Good catch. I don't think ENXIO is correct in this case, though. Maybe
-EOPNOTSUPP or -ENOSYS? The latter might make more sense I guess.

>  unlock:
>  	mutex_unlock(&dev->lock);
>  	blktrans_dev_put(dev);

Thanks,
Brian

Re: [PATCH] mtd: blktrans: change blktrans_getgeo rerurn value

[nick]

On 2015-05-20 03:33 PM, Brian Norris wrote:
> Hi Wenlin,
> 
> In the subject:
> 
> s/rerurn/return/
> 
> On Wed, May 13, 2015 at 02:29:16PM +0800, Wenlin Kang wrote:
>> Modify function blktrans_getgeo()'s return value to -ENXIO when
>> dev->tr->getgeo == NULL.
>>
>> We shouldn't make the return value to 0 when dev->tr->getgeo == NULL,
>> because the function blktrans_getgeo() has an output value "hd_geometry"
>> which is usually used by some application, if return 0, it will make some
>> application get the wrong information.
>>
>> Signed-off-by: Wenlin Kang <wenlin.kang@windriver.com>
>> ---
>>  drivers/mtd/mtd_blkdevs.c |    2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
>> index 2b0c5287..f8bb16e 100644
>> --- a/drivers/mtd/mtd_blkdevs.c
>> +++ b/drivers/mtd/mtd_blkdevs.c
>> @@ -273,7 +273,7 @@ static int blktrans_getgeo(struct block_device *bdev, struct hd_geometry *geo)
>>  	if (!dev->mtd)
>>  		goto unlock;
>>  
>> -	ret = dev->tr->getgeo ? dev->tr->getgeo(dev, geo) : 0;
>> +	ret = dev->tr->getgeo ? dev->tr->getgeo(dev, geo) : -ENXIO;
> 
> Good catch. I don't think ENXIO is correct in this case, though. Maybe
> -EOPNOTSUPP or -ENOSYS? The latter might make more sense I guess.
> 
I would recommend -EOPNOTSUPP as this as nothing to do with unimplemented 
functions or hardware support. This is just unsupported due to the value
being NULL and therefore the hardware support is not there.
Just My Option,
Nick
>>  unlock:
>>  	mutex_unlock(&dev->lock);
>>  	blktrans_dev_put(dev);
> 
> Thanks,
> Brian
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
> 

Re: [PATCH] mtd: blktrans: change blktrans_getgeo rerurn value

[Wenlin Kang]

[-- Attachment #1: Type: text/plain, Size: 2084 bytes --]

On 2015年05月21日 03:47, nick wrote:
>
> On 2015-05-20 03:33 PM, Brian Norris wrote:
>> Hi Wenlin,
>>
>> In the subject:
>>
>> s/rerurn/return/

    Thanks for pointing out this, I will modify it.

>>
>> On Wed, May 13, 2015 at 02:29:16PM +0800, Wenlin Kang wrote:
>>> Modify function blktrans_getgeo()'s return value to -ENXIO when
>>> dev->tr->getgeo == NULL.
>>>
>>> We shouldn't make the return value to 0 when dev->tr->getgeo == NULL,
>>> because the function blktrans_getgeo() has an output value "hd_geometry"
>>> which is usually used by some application, if return 0, it will make some
>>> application get the wrong information.
>>>
>>> Signed-off-by: Wenlin Kang <wenlin.kang@windriver.com>
>>> ---
>>>   drivers/mtd/mtd_blkdevs.c |    2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
>>> index 2b0c5287..f8bb16e 100644
>>> --- a/drivers/mtd/mtd_blkdevs.c
>>> +++ b/drivers/mtd/mtd_blkdevs.c
>>> @@ -273,7 +273,7 @@ static int blktrans_getgeo(struct block_device *bdev, struct hd_geometry *geo)
>>>   	if (!dev->mtd)
>>>   		goto unlock;
>>>   
>>> -	ret = dev->tr->getgeo ? dev->tr->getgeo(dev, geo) : 0;
>>> +	ret = dev->tr->getgeo ? dev->tr->getgeo(dev, geo) : -ENXIO;
>> Good catch. I don't think ENXIO is correct in this case, though. Maybe
>> -EOPNOTSUPP or -ENOSYS? The latter might make more sense I guess.
>>
> I would recommend -EOPNOTSUPP as this as nothing to do with unimplemented
> functions or hardware support. This is just unsupported due to the value
> being NULL and therefore the hardware support is not there.
> Just My Option,
> Nick

As you said, -EOPNOTSUPP might be better,thanks.


Hi Brian

I have remade the patch and attached it, would you please check it 
again? thanks.


>>>   unlock:
>>>   	mutex_unlock(&dev->lock);
>>>   	blktrans_dev_put(dev);
>> Thanks,
>> Brian
>>
>> ______________________________________________________
>> Linux MTD discussion mailing list
>> http://lists.infradead.org/mailman/listinfo/linux-mtd/
>>
>


-- 
Thanks,
Wenlin Kang


[-- Attachment #2: 0001-mtd-blktrans-change-blktrans_getgeo-return-value.patch --]
[-- Type: text/x-diff, Size: 1216 bytes --]

>From d8fa6589438a6992ed13b91c0124ed6cd8114eff Mon Sep 17 00:00:00 2001
From: Wenlin Kang <wenlin.kang@windriver.com>
Date: Thu, 21 May 2015 14:11:58 +0800
Subject: [PATCH] mtd: blktrans: change blktrans_getgeo return value

Modify function blktrans_getgeo()'s return value to -EOPNOTSUPP when
dev->tr->getgeo == NULL.

We shouldn't make the return value to 0 when dev->tr->getgeo == NULL,
because the function blktrans_getgeo() has an output value "hd_geometry"
which is usually used by some application, if return 0, it will make some
application get the wrong information.

Signed-off-by: Wenlin Kang <wenlin.kang@windriver.com>
---
 drivers/mtd/mtd_blkdevs.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
index 2b0c5287..2012216 100644
--- a/drivers/mtd/mtd_blkdevs.c
+++ b/drivers/mtd/mtd_blkdevs.c
@@ -273,7 +273,7 @@ static int blktrans_getgeo(struct block_device *bdev, struct hd_geometry *geo)
 	if (!dev->mtd)
 		goto unlock;
 
-	ret = dev->tr->getgeo ? dev->tr->getgeo(dev, geo) : 0;
+	ret = dev->tr->getgeo ? dev->tr->getgeo(dev, geo) : -EOPNOTSUPP;
 unlock:
 	mutex_unlock(&dev->lock);
 	blktrans_dev_put(dev);
-- 
1.7.9.5

Re: [PATCH] mtd: blktrans: change blktrans_getgeo rerurn value

[Brian Norris]

On Thu, May 21, 2015 at 02:49:38PM +0800, Wenlin Kang wrote:
> On 2015年05月21日 03:47, nick wrote:
> >On 2015-05-20 03:33 PM, Brian Norris wrote:
> >>On Wed, May 13, 2015 at 02:29:16PM +0800, Wenlin Kang wrote:
> >>>Modify function blktrans_getgeo()'s return value to -ENXIO when
> >>>dev->tr->getgeo == NULL.
> >>>
> >>>We shouldn't make the return value to 0 when dev->tr->getgeo == NULL,
> >>>because the function blktrans_getgeo() has an output value "hd_geometry"
> >>>which is usually used by some application, if return 0, it will make some
> >>>application get the wrong information.
> >>>
> >>>Signed-off-by: Wenlin Kang <wenlin.kang@windriver.com>
> >>>---
> >>>  drivers/mtd/mtd_blkdevs.c |    2 +-
> >>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>>diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
> >>>index 2b0c5287..f8bb16e 100644
> >>>--- a/drivers/mtd/mtd_blkdevs.c
> >>>+++ b/drivers/mtd/mtd_blkdevs.c
> >>>@@ -273,7 +273,7 @@ static int blktrans_getgeo(struct block_device *bdev, struct hd_geometry *geo)
> >>>  	if (!dev->mtd)
> >>>  		goto unlock;
> >>>-	ret = dev->tr->getgeo ? dev->tr->getgeo(dev, geo) : 0;
> >>>+	ret = dev->tr->getgeo ? dev->tr->getgeo(dev, geo) : -ENXIO;
> >>Good catch. I don't think ENXIO is correct in this case, though. Maybe
> >>-EOPNOTSUPP or -ENOSYS? The latter might make more sense I guess.
> >>
> >I would recommend -EOPNOTSUPP as this as nothing to do with unimplemented
> >functions or hardware support. This is just unsupported due to the value
> >being NULL and therefore the hardware support is not there.
> >Just My Option,
> >Nick
> 
> As you said, -EOPNOTSUPP might be better,thanks.

Nick's opinion is irrelevant here:

https://lkml.org/lkml/2014/8/4/206

> I have remade the patch and attached it, would you please check it
> again? thanks.

Please send patches inline (e.g., via git-send-email), not as
attachments. But this is pretty trivial, and it's what I already tested.

So, applied to l2-mtd.git.

Brian

Re: [PATCH] mtd: blktrans: change blktrans_getgeo rerurn value

[Richard Weinberger]

On Wed, May 20, 2015 at 9:33 PM, Brian Norris
<computersforpeace@gmail.com> wrote:
> Hi Wenlin,
>
> In the subject:
>
> s/rerurn/return/
>
> On Wed, May 13, 2015 at 02:29:16PM +0800, Wenlin Kang wrote:
>> Modify function blktrans_getgeo()'s return value to -ENXIO when
>> dev->tr->getgeo == NULL.
>>
>> We shouldn't make the return value to 0 when dev->tr->getgeo == NULL,
>> because the function blktrans_getgeo() has an output value "hd_geometry"
>> which is usually used by some application, if return 0, it will make some
>> application get the wrong information.
>>
>> Signed-off-by: Wenlin Kang <wenlin.kang@windriver.com>
>> ---
>>  drivers/mtd/mtd_blkdevs.c |    2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
>> index 2b0c5287..f8bb16e 100644
>> --- a/drivers/mtd/mtd_blkdevs.c
>> +++ b/drivers/mtd/mtd_blkdevs.c
>> @@ -273,7 +273,7 @@ static int blktrans_getgeo(struct block_device *bdev, struct hd_geometry *geo)
>>       if (!dev->mtd)
>>               goto unlock;
>>
>> -     ret = dev->tr->getgeo ? dev->tr->getgeo(dev, geo) : 0;
>> +     ret = dev->tr->getgeo ? dev->tr->getgeo(dev, geo) : -ENXIO;
>
> Good catch. I don't think ENXIO is correct in this case, though. Maybe
> -EOPNOTSUPP or -ENOSYS? The latter might make more sense I guess.

I'd vote for -ENOTTY as this is what HDIO_GETGEIO returns
if the function is not implemented and blktrans_getgeo()
is only a wrapper around that.

See https://www.kernel.org/doc/Documentation/ioctl/hdio.txt
and block/ioctl.c.

-- 
Thanks,
//richard

Re: [PATCH] mtd: blktrans: change blktrans_getgeo rerurn value

[Wenlin Kang]

On 2015年05月21日 15:37, Brian Norris wrote:
> On Thu, May 21, 2015 at 02:49:38PM +0800, Wenlin Kang wrote:
>> On 2015年05月21日 03:47, nick wrote:
>>> On 2015-05-20 03:33 PM, Brian Norris wrote:
>>>> On Wed, May 13, 2015 at 02:29:16PM +0800, Wenlin Kang wrote:
>>>>> Modify function blktrans_getgeo()'s return value to -ENXIO when
>>>>> dev->tr->getgeo == NULL.
>>>>>
>>>>> We shouldn't make the return value to 0 when dev->tr->getgeo == NULL,
>>>>> because the function blktrans_getgeo() has an output value "hd_geometry"
>>>>> which is usually used by some application, if return 0, it will make some
>>>>> application get the wrong information.
>>>>>
>>>>> Signed-off-by: Wenlin Kang <wenlin.kang@windriver.com>
>>>>> ---
>>>>>   drivers/mtd/mtd_blkdevs.c |    2 +-
>>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
>>>>> index 2b0c5287..f8bb16e 100644
>>>>> --- a/drivers/mtd/mtd_blkdevs.c
>>>>> +++ b/drivers/mtd/mtd_blkdevs.c
>>>>> @@ -273,7 +273,7 @@ static int blktrans_getgeo(struct block_device *bdev, struct hd_geometry *geo)
>>>>>   	if (!dev->mtd)
>>>>>   		goto unlock;
>>>>> -	ret = dev->tr->getgeo ? dev->tr->getgeo(dev, geo) : 0;
>>>>> +	ret = dev->tr->getgeo ? dev->tr->getgeo(dev, geo) : -ENXIO;
>>>> Good catch. I don't think ENXIO is correct in this case, though. Maybe
>>>> -EOPNOTSUPP or -ENOSYS? The latter might make more sense I guess.
>>>>
>>> I would recommend -EOPNOTSUPP as this as nothing to do with unimplemented
>>> functions or hardware support. This is just unsupported due to the value
>>> being NULL and therefore the hardware support is not there.
>>> Just My Option,
>>> Nick
>> As you said, -EOPNOTSUPP might be better,thanks.
> Nick's opinion is irrelevant here:
>
> https://lkml.org/lkml/2014/8/4/206
>
>> I have remade the patch and attached it, would you please check it
>> again? thanks.
> Please send patches inline (e.g., via git-send-email), not as
> attachments. But this is pretty trivial, and it's what I already tested.
>
> So, applied to l2-mtd.git.
>
> Brian
>

OK, thanks, I have resent the patch to you and list by git send-email, 
please check it.




-- 
Thanks,
Wenlin Kang

Re: [PATCH] mtd: blktrans: change blktrans_getgeo rerurn value

[Brian Norris]

On Thu, May 21, 2015 at 09:56:21AM +0200, Richard Weinberger wrote:
> On Wed, May 20, 2015 at 9:33 PM, Brian Norris
> <computersforpeace@gmail.com> wrote:
> > On Wed, May 13, 2015 at 02:29:16PM +0800, Wenlin Kang wrote:
> >> Modify function blktrans_getgeo()'s return value to -ENXIO when
> >> dev->tr->getgeo == NULL.
> >>
> >> We shouldn't make the return value to 0 when dev->tr->getgeo == NULL,
> >> because the function blktrans_getgeo() has an output value "hd_geometry"
> >> which is usually used by some application, if return 0, it will make some
> >> application get the wrong information.
> >>
> >> Signed-off-by: Wenlin Kang <wenlin.kang@windriver.com>
> >> ---
> >>  drivers/mtd/mtd_blkdevs.c |    2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
> >> index 2b0c5287..f8bb16e 100644
> >> --- a/drivers/mtd/mtd_blkdevs.c
> >> +++ b/drivers/mtd/mtd_blkdevs.c
> >> @@ -273,7 +273,7 @@ static int blktrans_getgeo(struct block_device *bdev, struct hd_geometry *geo)
> >>       if (!dev->mtd)
> >>               goto unlock;
> >>
> >> -     ret = dev->tr->getgeo ? dev->tr->getgeo(dev, geo) : 0;
> >> +     ret = dev->tr->getgeo ? dev->tr->getgeo(dev, geo) : -ENXIO;
> >
> > Good catch. I don't think ENXIO is correct in this case, though. Maybe
> > -EOPNOTSUPP or -ENOSYS? The latter might make more sense I guess.
> 
> I'd vote for -ENOTTY as this is what HDIO_GETGEIO returns
> if the function is not implemented and blktrans_getgeo()
> is only a wrapper around that.

I suppose that makes more sense.

> See https://www.kernel.org/doc/Documentation/ioctl/hdio.txt

But this only mentions EINVAL...

> and block/ioctl.c.

Anyway, I pushed EOPNOTSUPP, but I can fix that up if we agree.

Brian

Re: [PATCH] mtd: blktrans: change blktrans_getgeo rerurn value

[Wenlin Kang]

On 2015年05月21日 16:05, Brian Norris wrote:
> On Thu, May 21, 2015 at 09:56:21AM +0200, Richard Weinberger wrote:
>> On Wed, May 20, 2015 at 9:33 PM, Brian Norris
>> <computersforpeace@gmail.com> wrote:
>>> On Wed, May 13, 2015 at 02:29:16PM +0800, Wenlin Kang wrote:
>>>> Modify function blktrans_getgeo()'s return value to -ENXIO when
>>>> dev->tr->getgeo == NULL.
>>>>
>>>> We shouldn't make the return value to 0 when dev->tr->getgeo == NULL,
>>>> because the function blktrans_getgeo() has an output value "hd_geometry"
>>>> which is usually used by some application, if return 0, it will make some
>>>> application get the wrong information.
>>>>
>>>> Signed-off-by: Wenlin Kang <wenlin.kang@windriver.com>
>>>> ---
>>>>   drivers/mtd/mtd_blkdevs.c |    2 +-
>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
>>>> index 2b0c5287..f8bb16e 100644
>>>> --- a/drivers/mtd/mtd_blkdevs.c
>>>> +++ b/drivers/mtd/mtd_blkdevs.c
>>>> @@ -273,7 +273,7 @@ static int blktrans_getgeo(struct block_device *bdev, struct hd_geometry *geo)
>>>>        if (!dev->mtd)
>>>>                goto unlock;
>>>>
>>>> -     ret = dev->tr->getgeo ? dev->tr->getgeo(dev, geo) : 0;
>>>> +     ret = dev->tr->getgeo ? dev->tr->getgeo(dev, geo) : -ENXIO;
>>> Good catch. I don't think ENXIO is correct in this case, though. Maybe
>>> -EOPNOTSUPP or -ENOSYS? The latter might make more sense I guess.
>> I'd vote for -ENOTTY as this is what HDIO_GETGEIO returns
>> if the function is not implemented and blktrans_getgeo()
>> is only a wrapper around that.
> I suppose that makes more sense.
>
>> See https://www.kernel.org/doc/Documentation/ioctl/hdio.txt
> But this only mentions EINVAL...
>
>> and block/ioctl.c.
> Anyway, I pushed EOPNOTSUPP, but I can fix that up if we agree.
>
> Brian
>
>

Hi  Brian

I just read block/compat_ioctl.c, in that I found the follow,

  51 static int compat_hdio_getgeo(struct gendisk *disk, struct 
block_device *bdev,
  52             struct compat_hd_geometry __user *ugeo)
  53 {
  54     struct hd_geometry geo;
  55     int ret;
  56
  57     if (!ugeo)
  58         return -EINVAL;
  59     if (!disk->fops->getgeo)
  60         return -ENOTTY;

So, as Richard said, -ENOTTY should also be OK, and before last
commit(048d87199566663e4edc4880df3703c04bcf41d9), it's  -ENOTTY, so I 
think you can modify it,
or let know whether I need to resent it again.




-- 
Thanks,
Wenlin Kang

Re: [PATCH] mtd: blktrans: change blktrans_getgeo rerurn value

[Richard Weinberger]

Am 21.05.2015 um 10:05 schrieb Brian Norris:
> On Thu, May 21, 2015 at 09:56:21AM +0200, Richard Weinberger wrote:
>> On Wed, May 20, 2015 at 9:33 PM, Brian Norris
>> <computersforpeace@gmail.com> wrote:
>>> On Wed, May 13, 2015 at 02:29:16PM +0800, Wenlin Kang wrote:
>>>> Modify function blktrans_getgeo()'s return value to -ENXIO when
>>>> dev->tr->getgeo == NULL.
>>>>
>>>> We shouldn't make the return value to 0 when dev->tr->getgeo == NULL,
>>>> because the function blktrans_getgeo() has an output value "hd_geometry"
>>>> which is usually used by some application, if return 0, it will make some
>>>> application get the wrong information.
>>>>
>>>> Signed-off-by: Wenlin Kang <wenlin.kang@windriver.com>
>>>> ---
>>>>  drivers/mtd/mtd_blkdevs.c |    2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
>>>> index 2b0c5287..f8bb16e 100644
>>>> --- a/drivers/mtd/mtd_blkdevs.c
>>>> +++ b/drivers/mtd/mtd_blkdevs.c
>>>> @@ -273,7 +273,7 @@ static int blktrans_getgeo(struct block_device *bdev, struct hd_geometry *geo)
>>>>       if (!dev->mtd)
>>>>               goto unlock;
>>>>
>>>> -     ret = dev->tr->getgeo ? dev->tr->getgeo(dev, geo) : 0;
>>>> +     ret = dev->tr->getgeo ? dev->tr->getgeo(dev, geo) : -ENXIO;
>>>
>>> Good catch. I don't think ENXIO is correct in this case, though. Maybe
>>> -EOPNOTSUPP or -ENOSYS? The latter might make more sense I guess.
>>
>> I'd vote for -ENOTTY as this is what HDIO_GETGEIO returns
>> if the function is not implemented and blktrans_getgeo()
>> is only a wrapper around that.
> 
> I suppose that makes more sense.
> 
>> See https://www.kernel.org/doc/Documentation/ioctl/hdio.txt
> 
> But this only mentions EINVAL...

Because ENOTTY is covered by ioctl().
If you request a non-existing function this error code will
be returned.

Thanks,
//richard

[PATCH] mtd: blktrans: use better error code for unimplemented ioctl()

[Brian Norris]

In commit 50183936254b ("mtd: blktrans: change blktrans_getgeo return
value") we fixed the problem that ioctl(HDIO_GETGEO) might return 0
(success) for mtdblock devices which did not implement the feature and
would leave a blank (zero) result.

But now, let's get the error code right. Other code paths on this ioctl
tend to use -ENOTTY to notify the user that the ioctl() is not supported
for the device, so let's use that instead of -EOPNOTSUPP.

Suggested-by: Richard Weinberger <richard@nod.at>
Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
Against l2-mtd.git (linux-next)

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

diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
index c545c9325acf..41acc507b22e 100644
--- a/drivers/mtd/mtd_blkdevs.c
+++ b/drivers/mtd/mtd_blkdevs.c
@@ -278,7 +278,7 @@ static int blktrans_getgeo(struct block_device *bdev, struct hd_geometry *geo)
 	if (!dev->mtd)
 		goto unlock;
 
-	ret = dev->tr->getgeo ? dev->tr->getgeo(dev, geo) : -EOPNOTSUPP;
+	ret = dev->tr->getgeo ? dev->tr->getgeo(dev, geo) : -ENOTTY;
 unlock:
 	mutex_unlock(&dev->lock);
 	blktrans_dev_put(dev);
-- 
1.9.1

Re: [PATCH] mtd: blktrans: use better error code for unimplemented ioctl()

[Brian Norris]

On Thu, May 21, 2015 at 10:54:31AM -0700, Brian Norris wrote:
> In commit 50183936254b ("mtd: blktrans: change blktrans_getgeo return
> value") we fixed the problem that ioctl(HDIO_GETGEO) might return 0
> (success) for mtdblock devices which did not implement the feature and
> would leave a blank (zero) result.
> 
> But now, let's get the error code right. Other code paths on this ioctl
> tend to use -ENOTTY to notify the user that the ioctl() is not supported
> for the device, so let's use that instead of -EOPNOTSUPP.
> 
> Suggested-by: Richard Weinberger <richard@nod.at>
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> ---
> Against l2-mtd.git (linux-next)

Applied.