RE: [PATCH v2 1/3] platform/x86: Add Intel Input Output Manager (IOM) driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Prashant,

...

> > > > +
> > > > +	reg = iom->regbar + IOM_PORT_STATUS_OFFSET + IOM_REG_LEN *
> > > port;
> > > > +
> > > > +	*status = ioread32(reg);
> > >
> > > Perhaps just inline reg within the parentheses?
> >
> > Kept this way to increase readability. Let me know if you feel
> > strongly towards inline reg.
> 
> I'd rather this be inlined, you save a couple lines from the variable declaration,
> and IMO we're not gaining much in terms of readability by declaring this
> separately.
> 

Ack
(at least to me, it was more readable)

...

> >
> > > > +
> > > > +	return 0;
> > > > +}
> > > > +EXPORT_SYMBOL_GPL(intel_iom_port_status);
> > > > +
> > > > +static int intel_iom_probe(struct platform_device *pdev) {
> > > > +	void __iomem *addr;
> > > > +
> > > > +	/* only one IOM device is supported */
> > >
> > > Minor nit: s/only/Only
> >
> > And then I may need to end the comment with a period.
> > Let me know if you feel strongly.
> Yes, let's capitalize and add the period. Let's try to use the right punctuation
> where possible.
> 

Ack
Will take care of this as part of v3.




[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux