Re: [PATCH v2 2/4] mtd: nand: convert printk() to pr_*()

[Artem Bityutskiy <dedekind1@gmail.com>]

On Mon, 2011-06-13 at 11:24 -0700, Brian Norris wrote:
> On Thu, Jun 9, 2011 at 9:03 AM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> > On Thu, 2011-06-09 at 09:00 -0700, Brian Norris wrote:
> >> On Thu, Jun 9, 2011 at 12:44 AM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> >> > Wait, and why are we using pr_* while it is better to use dbg_* ? :-)
> >>
> >> Just so we're on the same page...did you say "dbg_*" when you really
> >> meant "dev_*"?
> >
> > Yes, I meant dev_*
> >
> >> Anyway, the answer is: I'm mostly trying to do as little shakeup as
> >> possible, so "printk" could translate pretty directly to the "pr_*"
> >> with shorter code, etc. If it makes as much (or more) sense to have
> >> the device name on all the prints, then I can just as well do it that
> >> way.
> >
> > Yes, I think it makes much more sense to specify for which device (out
> > of possibly many!) this message belongs.
> 
> I've been testing some of the "dev_*" printing, and it seems as if our
> mtd_info structs never have fully initialized "device" fields (i.e.,
> mtd->dev.driver, mtd->dev.bus, mtd->dev.class, mtd->dev.init_name,
> etc. are never filled in with anything meaningful). That means that
> our dev_* messages do not have anything to work from and simply print
> " (null): " references before our strings instead of device
> driver/name information, for example:
> 
>  (null): ONFI flash detected

Ouch, good finding.

> 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;
> 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.

> Is there some better way to initialize these device values? Or should
> we scrap the whole dev_* thing since MTD doesn't match the device
> model well enough?

I think there is no reason why it would not match the model, so of
course fixing this is preferable...

-- 
Best Regards,
Artem Bityutskiy