[PATCH 5/5] drivers: mtd: add Samsung SoC OneNAND driver
Marek Szyprowski
m.szyprowski at samsung.com
Tue Apr 27 03:30:29 EDT 2010
Hello,
On Tuesday, April 27, 2010 7:19 AM Ben Dooks wrote:
> ...
> > +static int s3c_read_reg(int offset)
> > +{
> > + return readl(onenand->base + offset);
> > +}
> > +
> > +static void s3c_write_reg(int value, int offset)
> > +{
> > + writel(value, onenand->base + offset);
> > +}
>
> I can sort of see why you've done this, it would be interesting to see
> if the compiler is continually reloading onenand->base or not (currently
> too lazy to try this myself).
I will check this, but probably changing these functions to 'static inline'
would solve the problem for sure.
> ...
> > + r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + if (!r) {
> > + dev_err(&pdev->dev, "no resource defined\n");
> > + return -ENXIO;
>
> If i rember correctly, -ENXIO will make the driver layer fail to report
> any binding issue. However there seems to be a group of people who feel
> that using filesystem errors such as -ENOENT inappropriate for drivers...
>
> Not sure what to recommend here, maybe just leave as is.
I will change it to -ENOENT. Most the drivers returns it if get_resource()
fails. I don't think that this really matters anyway...
> ...
> > +static int __init s3c_onenand_init(void)
> > +{
> > + return platform_driver_register(&s3c_onenand_driver);
> > +}
> > +
> > +static void __exit s3c_onenand_exit(void)
> > +{
> > + platform_driver_unregister(&s3c_onenand_driver);
> > +}
> > +
> > +module_init(s3c_onenand_init);
> > +module_exit(s3c_onenand_exit);
> > +
> > +MODULE_LICENSE("GPL");
> > +MODULE_AUTHOR("Kyungmin Park <kyungmin.park at samsung.com>");
> > +MODULE_DESCRIPTION("Samsung OneNAND controller support");
> > +MODULE_ALIAS("platform:samsung-onenand");
>
> don't think you need a MODULE_ALIAS when you have a MODULE_DEVICE_TABLE
> decleration, the module tools should support reading this table for use
> with the autoloader.
Ok. Thanks for your hints.
> Don't see any show stopping problems with this. It would be nice to
> sort out as many of the issues raised before final submission.
I will do all required cleanup and send the new patch series soon. Thanks for
your detailed review.
Best regards
--
Marek Szyprowski
Samsung Poland R&D Center
More information about the linux-arm-kernel
mailing list