On Tue, 2016-06-28 at 14:35 +0530, Rajneesh Bhardwaj wrote:
> On Mon, Jun 13, 2016 at 08:56:49PM +0300, Andy Shevchenko wrote:
> > This patch does the following:
> > - refactors code to use recently introduced
> > DEFINE_DEBUGFS_ATTRIBUTE() macro
> > - makes absence of DEBUG_FS non-fatal error
> > - counter_val = pmc_core_reg_read(pmcdev,
> > - SPT_PMC_SLP_S0_RES_COUNTER_
> > OFFSET);
> > - seq_printf(s, "%u\n",
> > pmc_core_adjust_slp_s0_step(counter_val));
> > + value = pmc_core_reg_read(pmcdev,
> > SPT_PMC_SLP_S0_RES_COUNTER_OFFSET);
> > + *val = pmc_core_adjust_slp_s0_step(value);
> >
>
> We were ensuring a new line for printing the output value with
> seq_printf
> whereas in this case the output is printed along with the shell
> prompt.
> Sometimes this could be harder to read.
This would be a difference in the behaviour to a generic assumption
(since this new macro is going to be used by plenty of simple attributes
under debugfs). If you still consider it's inappropriate I would
recommend to propose a fix globally then.
Regarding to change behaviour of the current implementation I think
Darren can tell his opinion. (For me at least pros and cons of new
format quite obvious).
>
> > +struct dentry;
> > +
>
> Why do we need this ?
Since we are using pointer to undefined type.
>
> > /**
> > * struct pmc_dev - pmc device structure
> > * @base_addr: comtains pmc base address
> > @@ -42,9 +45,7 @@
> > struct pmc_dev {
> > u32 base_addr;
> > void __iomem *regbase;
> > -#if IS_ENABLED(CONFIG_DEBUG_FS)
> > struct dentry *dbgfs_dir;
> > -#endif /* CONFIG_DEBUG_FS */
--
Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
Intel Finland Oy
--
To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
