[PATCH v2 2/4] mtd: nand: convert printk() to pr_*()
Brian Norris
computersforpeace at gmail.com
Wed Jul 6 14:51:19 EDT 2011
On Wed, Jun 22, 2011 at 2:12 AM, Dmitry Eremin-Solenikov
<dbaryshkov at gmail.com> wrote:
> Artem Bityutskiy wrote:
>
>> On Mon, 2011-06-13 at 11:24 -0700, Brian Norris wrote:
>>> I'm a little new to the Linux device model, so I'm not sure if it's
>>> safe and sensible to add lines to a driver's nand_probe function,
>>> like, for plat_nand.c:
>>> data->mtd.dev.driver = pdev->dev.driver;
>
> IIUC, we should not set driver, but it might be better to consult with
> more experienced DM hackers (like Greg Kroah-Hartman).
>
>>> data->mtd.dev.init_name = dev_name(&pdev->dev);
>>
>> I think so, I believe struct device was added to mtd by some non-MTD guy
>> to just make it complaint with one of interface changes which required
>> struct device. So please, go ahead and improve that a bit.
I've dug a little more on this, especially in the RFC + mailing list
conversation that led to adding the "mtd_info.dev" field. See here:
http://lists.infradead.org/pipermail/linux-mtd/2009-April/025142.html
It seems that, according to the following comment, the master MTD node
(which is passed to most of our functions) is never registered and so
does not have valid information for printing dev_* information:
/* NOTE: we don't arrange MTDs as a tree; it'd be error-prone
* to have the same data be in two different partitions.
*/
So it seems that, by "design," mtd->dev will never be suitable for a
dev_* message (at least when "mtd" is the master node), so instead of:
dev_info(&mtd->dev, "informational message\n");
we should do:
dev_info(mtd->dev.parent, "informational message\n");
This utilizes the physical device instead of the MTD device for debug messages.
Unfortunately, this kinda makes some of our code longer, rather than
more concise (not a huge deal, I guess). Also, it relies on a
specific, potentially-flawed device tree layout. Perhaps there should
be a "mtd_to_dev" helper (accompanying "dev_to_mtd") to abstract this
a little and leave room for future tree changes? For example:
static inline struct device *mtd_to_dev(struct mtd_info *mtd)
{
return mtd ? mtd->dev.parent : NULL;
}
That makes the previous example into:
dev_info(mtd_to_dev(mtd), "informational message\n");
I'll send out a patch set if any of this makes sense.
Brian
More information about the linux-mtd
mailing list