RE: [PATCH 2/5] scsi: ufs: wrap the i/o access operations

[Seungwon Jeon <tgih.jun@samsung.com>]

Hi,

On Thursday, April 25, 2013, Sujit Reddy Thumma wrote:
> On 4/24/2013 9:36 PM, Seungwon Jeon wrote:
> > Simplify operations with hiding mmio_base.
> >
> > Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
> > ---
> >   drivers/scsi/ufs/ufshcd.c |  106 +++++++++++++++++++--------------------------
> >   drivers/scsi/ufs/ufshcd.h |    5 ++
> >   2 files changed, 49 insertions(+), 62 deletions(-)
> >
> > diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
> > index 1680394..6728450 100644
> > --- a/drivers/scsi/ufs/ufshcd.h
> > +++ b/drivers/scsi/ufs/ufshcd.h
> > @@ -190,4 +190,9 @@ int ufshcd_init(struct device *, struct ufs_hba ** , void __iomem * ,
> >   			unsigned int);
> >   void ufshcd_remove(struct ufs_hba *);
> >
> > +#define ufshcd_writel(hba, reg, val)	\
> 
> Let this be consistent with writel() arguments - "val" as second arg and
> "reg" as third?
You got a point there.
When considering an array of arguments in two functions and value part can be some long expression,
I think it seems more coherent.
	ufshcd_readl(hba, reg);
	ufshcd_writel(hba, reg, val);
How about keeping these?

Thanks,
Seungwon Jeon

> 
> > +	writel((val), (hba)->mmio_base + (reg))
> > +#define ufshcd_readl(hba, reg)	\
> > +	readl((hba)->mmio_base + (reg))
> > +
> >   #endif /* End of Header */
> >
> 
> --
> Regards,
> Sujit
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html