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

[merez@codeaurora.org]

Tested-by: Maya Erez <merez@codeaurora.org>

I also tend to agree with Sujit on the order of the wrappers parameters.

Thanks,
Maya

> On 4/26/2013 10:36 AM, Seungwon Jeon wrote:
>> 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?
>
> I somehow tend to agree with what Sujit suggested. Its good to be
> consitent with writel() for better code readability.
>
>>
>> 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
>> --
>> 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
>
> --
> 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
>


-- 
Maya Erez
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation