[PATCH 1/1]nand/denali: Add runtime pm framework for MRST Denali NAND controller

[Dong, Chuanxiao]

Hi Artem
Thanks for your comment. Except the DENALI_PM macro problem, do you have any other comment
about the implement of runtime pm?

> -----Original Message-----
> From: Artem Bityutskiy [mailto:dedekind1 at gmail.com]
> Sent: Monday, August 30, 2010 8:43 PM
> To: Dong, Chuanxiao
> Cc: dwmw2 at infradead.org; Van De Ven, Arjan; linux-mtd at lists.infradead.org
> Subject: Re: [PATCH 1/1]nand/denali: Add runtime pm framework for MRST Denali
> NAND controller
> 
> Hi,
> 
> On Thu, 2010-08-26 at 16:12 +0800, Chuanxiao.Dong wrote:
> > +static struct dev_pm_ops denali_pm = {
> > +	.runtime_suspend = denali_runtime_suspend,
> > +	.runtime_resume = denali_runtime_resume,
> > +	.runtime_idle = denali_runtime_idle,
> > +};
> > +#define DENALI_PM (&denali_pm)
> 
> I think it is confusing to use this style. Please, do not define this
> DENALI_PM macro, unless this is a standard practice, which I doubt, but
> not sure.
> 
> ... snip ...
> 
> > +#else
> > +#define DENALI_PM NULL
> Ditto.
> > +static inline void denali_timer_add(struct denali_nand_info *denali) {}
> > +static inline void denali_timer_func(unsigned long ptr) {}
> > +#endif
> 
> #endif /* !CONFIG_PM_RUNTIME */
> 
> .. snip ..
> 
> >  static struct pci_driver denali_pci_driver = {
> > +	.driver = {
> > +		.pm = DENALI_PM,
> > +	},
> 
> This is the only place where you use this macro, just do
> 
> #ifdef CONFIG_PM_RUNTIME
> 	.driver = {
> 		.pm = DENALI_PM,
> 	},
> #endif
> 
> And it will be more readable.
> 
> --
> Best Regards,
> Artem Bityutskiy (Битюцкий Артём)