[PATCH] mtd: mtdraw: drop ioctl callback for mtdraw device

[PATCH] mtd: mtdraw: drop ioctl callback for mtdraw device

[Sascha Hauer]

Do not call mtd_ioctl for mtdraw devices. mtd_ioctl will derefence
the priv pointer to a struct mtd_info whereas with mtdraw devices it will be
a struct mtdraw pointer. We do not need ioctls for mtdraw devices, so drop
it instead of fixing it.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mtd/mtdraw.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/mtd/mtdraw.c b/drivers/mtd/mtdraw.c
index 2acd51f..be34723 100644
--- a/drivers/mtd/mtdraw.c
+++ b/drivers/mtd/mtdraw.c
@@ -278,7 +278,6 @@ static const struct file_operations mtd_raw_fops = {
 	.read		= mtdraw_read,
 	.write		= mtdraw_write,
 	.erase		= mtdraw_erase,
-	.ioctl		= mtd_ioctl,
 	.lseek		= dev_lseek_default,
 };
 
-- 
1.8.4.3


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] mtd: mtdraw: drop ioctl callback for mtdraw device

[Robert Jarzmik]

Sascha Hauer <s.hauer@pengutronix.de> writes:

> Do not call mtd_ioctl for mtdraw devices. mtd_ioctl will derefence
> the priv pointer to a struct mtd_info whereas with mtdraw devices it will be
> a struct mtdraw pointer. We do not need ioctls for mtdraw devices, so drop
> it instead of fixing it.

Very true for the fix.
As to whether we need ioctls for raw mtd devices, we're loosing bad block
operations and memgetinfo.

Unfortunately that's a flaw with my split of core.c/mtdraw.c. The complete fix
would be to have mtd_ioctl split into :
 - mtd_ioctl : would call 
              _mtd_ioctl(struct mtd_info *info, int request, void *buf)
 - _mtd_ioctl : current code of ioctl handling
And add :
 - mtdraw_ioctl: would call _mtd_ioctl()

For the time being your patch is perfectly fine. Do you want me to add the split
+ mtdraw_ioctl() ? That's not a too big amount of work.

Cheers.

--
Robert

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] mtd: mtdraw: drop ioctl callback for mtdraw device

[Sascha Hauer]

On Tue, Dec 03, 2013 at 11:04:45PM +0100, Robert Jarzmik wrote:
> Sascha Hauer <s.hauer@pengutronix.de> writes:
> 
> > Do not call mtd_ioctl for mtdraw devices. mtd_ioctl will derefence
> > the priv pointer to a struct mtd_info whereas with mtdraw devices it will be
> > a struct mtdraw pointer. We do not need ioctls for mtdraw devices, so drop
> > it instead of fixing it.
> 
> Very true for the fix.
> As to whether we need ioctls for raw mtd devices, we're loosing bad block
> operations and memgetinfo.
> 
> Unfortunately that's a flaw with my split of core.c/mtdraw.c. The complete fix
> would be to have mtd_ioctl split into :
>  - mtd_ioctl : would call 
>               _mtd_ioctl(struct mtd_info *info, int request, void *buf)
>  - _mtd_ioctl : current code of ioctl handling
> And add :
>  - mtdraw_ioctl: would call _mtd_ioctl()
> 
> For the time being your patch is perfectly fine. Do you want me to add the split
> + mtdraw_ioctl() ? That's not a too big amount of work.

For MEMGETINFO we can directly call into _mtd_ioctl(), but for
MEM[SG]ETBADBLOCK the offsets would have to be corrected first. Is this
worth the effort? Otherwise we could do something like:

int mtdraw_ioctl(struct cdev *cdev, int request, void *buf)
{
	struct mtd_info *mtd = to_mtd(cdev);

	switch (request) {
	case MEMGETINFO:
		return mtd_memgetinfo(mtd, buf);
	default:
		return -EINVAL;
	}
}

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] mtd: mtdraw: drop ioctl callback for mtdraw device

[Robert Jarzmik]

Sascha Hauer <s.hauer@pengutronix.de> writes:

> On Tue, Dec 03, 2013 at 11:04:45PM +0100, Robert Jarzmik wrote:
> For MEMGETINFO we can directly call into _mtd_ioctl(), but for
> MEM[SG]ETBADBLOCK the offsets would have to be corrected first. Is this
> worth the effort? Otherwise we could do something like:
>
> int mtdraw_ioctl(struct cdev *cdev, int request, void *buf)
> {
> 	struct mtd_info *mtd = to_mtd(cdev);
>
> 	switch (request) {
> 	case MEMGETINFO:
> 		return mtd_memgetinfo(mtd, buf);
> 	default:
> 		return -EINVAL;
> 	}
> }

Yes, that looks like the right thing to do.
I don't think MEM[GS]ETBADBLOCK are used anywhere at this time for raw
devices. And I'm not convinced it is worth the effort just as you.

And let's be pragmatic : is somebody needs the badblock ioctls, he'll add them
:) By now, I'm happy with your patch.

Cheers.

-- 
Robert

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox