public inbox for linux-scsi@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 2/5] scsi: ufs: wrap the i/o access operations
@ 2013-04-24 16:06 Seungwon Jeon
  2013-04-25  4:49 ` Sujit Reddy Thumma
  2013-05-04  8:45 ` [PATCH v2 2/7] " Seungwon Jeon
  0 siblings, 2 replies; 7+ messages in thread
From: Seungwon Jeon @ 2013-04-24 16:06 UTC (permalink / raw)
  To: linux-scsi
  Cc: 'Vinayak Holikatti', 'Santosh Y',
	'James E.J. Bottomley'

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.c b/drivers/scsi/ufs/ufshcd.c
index 41b9639..b6c19b0 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -71,7 +71,7 @@ enum {
  */
 static inline u32 ufshcd_get_ufs_version(struct ufs_hba *hba)
 {
-	return readl(hba->mmio_base + REG_UFS_VERSION);
+	return ufshcd_readl(hba, REG_UFS_VERSION);
 }
 
 /**
@@ -130,8 +130,7 @@ static inline int ufshcd_get_tm_free_slot(struct ufs_hba *hba)
  */
 static inline void ufshcd_utrl_clear(struct ufs_hba *hba, u32 pos)
 {
-	writel(~(1 << pos),
-		(hba->mmio_base + REG_UTP_TRANSFER_REQ_LIST_CLEAR));
+	ufshcd_writel(hba, REG_UTP_TRANSFER_REQ_LIST_CLEAR, ~(1 << pos));
 }
 
 /**
@@ -165,7 +164,7 @@ static inline int ufshcd_get_lists_status(u32 reg)
  */
 static inline int ufshcd_get_uic_cmd_result(struct ufs_hba *hba)
 {
-	return readl(hba->mmio_base + REG_UIC_COMMAND_ARG_2) &
+	return ufshcd_readl(hba, REG_UIC_COMMAND_ARG_2) &
 	       MASK_UIC_COMMAND_RESULT;
 }
 
@@ -243,18 +242,14 @@ ufshcd_config_int_aggr(struct ufs_hba *hba, int option)
 {
 	switch (option) {
 	case INT_AGGR_RESET:
-		writel((INT_AGGR_ENABLE |
-			INT_AGGR_COUNTER_AND_TIMER_RESET),
-			(hba->mmio_base +
-			 REG_UTP_TRANSFER_REQ_INT_AGG_CONTROL));
+		ufshcd_writel(hba, REG_UTP_TRANSFER_REQ_INT_AGG_CONTROL,
+			      INT_AGGR_ENABLE | INT_AGGR_COUNTER_AND_TIMER_RESET);
 		break;
 	case INT_AGGR_CONFIG:
-		writel((INT_AGGR_ENABLE |
-			INT_AGGR_PARAM_WRITE |
-			INT_AGGR_COUNTER_THRESHOLD_VALUE |
-			INT_AGGR_TIMEOUT_VALUE),
-			(hba->mmio_base +
-			 REG_UTP_TRANSFER_REQ_INT_AGG_CONTROL));
+		ufshcd_writel(hba, REG_UTP_TRANSFER_REQ_INT_AGG_CONTROL,
+			      INT_AGGR_ENABLE | INT_AGGR_PARAM_WRITE |
+			      INT_AGGR_COUNTER_THRESHOLD_VALUE |
+			      INT_AGGR_TIMEOUT_VALUE);
 		break;
 	}
 }
@@ -267,12 +262,10 @@ ufshcd_config_int_aggr(struct ufs_hba *hba, int option)
  */
 static void ufshcd_enable_run_stop_reg(struct ufs_hba *hba)
 {
-	writel(UTP_TASK_REQ_LIST_RUN_STOP_BIT,
-	       (hba->mmio_base +
-		REG_UTP_TASK_REQ_LIST_RUN_STOP));
-	writel(UTP_TRANSFER_REQ_LIST_RUN_STOP_BIT,
-	       (hba->mmio_base +
-		REG_UTP_TRANSFER_REQ_LIST_RUN_STOP));
+	ufshcd_writel(hba, REG_UTP_TASK_REQ_LIST_RUN_STOP,
+		      UTP_TASK_REQ_LIST_RUN_STOP_BIT);
+	ufshcd_writel(hba, REG_UTP_TRANSFER_REQ_LIST_RUN_STOP,
+		      UTP_TRANSFER_REQ_LIST_RUN_STOP_BIT);
 }
 
 /**
@@ -281,7 +274,7 @@ static void ufshcd_enable_run_stop_reg(struct ufs_hba *hba)
  */
 static inline void ufshcd_hba_start(struct ufs_hba *hba)
 {
-	writel(CONTROLLER_ENABLE , (hba->mmio_base + REG_CONTROLLER_ENABLE));
+	ufshcd_writel(hba, REG_CONTROLLER_ENABLE, CONTROLLER_ENABLE);
 }
 
 /**
@@ -290,7 +283,7 @@ static inline void ufshcd_hba_start(struct ufs_hba *hba)
  */
 static inline void ufshcd_hba_stop(struct ufs_hba *hba)
 {
-	writel(CONTROLLER_DISABLE, (hba->mmio_base + REG_CONTROLLER_ENABLE));
+	ufshcd_writel(hba, REG_CONTROLLER_ENABLE, CONTROLLER_DISABLE);
 }
 
 /**
@@ -301,7 +294,7 @@ static inline void ufshcd_hba_stop(struct ufs_hba *hba)
  */
 static inline int ufshcd_is_hba_active(struct ufs_hba *hba)
 {
-	return (readl(hba->mmio_base + REG_CONTROLLER_ENABLE) & 0x1) ? 0 : 1;
+	return (ufshcd_readl(hba, REG_CONTROLLER_ENABLE) & 0x1) ? 0 : 1;
 }
 
 /**
@@ -313,8 +306,7 @@ static inline
 void ufshcd_send_command(struct ufs_hba *hba, unsigned int task_tag)
 {
 	__set_bit(task_tag, &hba->outstanding_reqs);
-	writel((1 << task_tag),
-	       (hba->mmio_base + REG_UTP_TRANSFER_REQ_DOOR_BELL));
+	ufshcd_writel(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL, 1 << task_tag);
 }
 
 /**
@@ -338,8 +330,7 @@ static inline void ufshcd_copy_sense_data(struct ufshcd_lrb *lrbp)
  */
 static inline void ufshcd_hba_capabilities(struct ufs_hba *hba)
 {
-	hba->capabilities =
-		readl(hba->mmio_base + REG_CONTROLLER_CAPABILITIES);
+	hba->capabilities = ufshcd_readl(hba, REG_CONTROLLER_CAPABILITIES);
 
 	/* nutrs and nutmrs are 0 based values */
 	hba->nutrs = (hba->capabilities & MASK_TRANSFER_REQUESTS_SLOTS) + 1;
@@ -356,16 +347,13 @@ static inline void
 ufshcd_send_uic_command(struct ufs_hba *hba, struct uic_command *uic_cmnd)
 {
 	/* Write Args */
-	writel(uic_cmnd->argument1,
-	      (hba->mmio_base + REG_UIC_COMMAND_ARG_1));
-	writel(uic_cmnd->argument2,
-	      (hba->mmio_base + REG_UIC_COMMAND_ARG_2));
-	writel(uic_cmnd->argument3,
-	      (hba->mmio_base + REG_UIC_COMMAND_ARG_3));
+	ufshcd_writel(hba, REG_UIC_COMMAND_ARG_1, uic_cmnd->argument1);
+	ufshcd_writel(hba, REG_UIC_COMMAND_ARG_2, uic_cmnd->argument2);
+	ufshcd_writel(hba, REG_UIC_COMMAND_ARG_3, uic_cmnd->argument3);
 
 	/* Write UIC Cmd */
-	writel((uic_cmnd->command & COMMAND_OPCODE_MASK),
-	       (hba->mmio_base + REG_UIC_COMMAND));
+	ufshcd_writel(hba, REG_UIC_COMMAND,
+		      uic_cmnd->command & COMMAND_OPCODE_MASK);
 }
 
 /**
@@ -417,16 +405,15 @@ static void ufshcd_int_config(struct ufs_hba *hba, u32 option)
 {
 	switch (option) {
 	case UFSHCD_INT_ENABLE:
-		writel(hba->int_enable_mask,
-		      (hba->mmio_base + REG_INTERRUPT_ENABLE));
+		ufshcd_writel(hba, REG_INTERRUPT_ENABLE, hba->int_enable_mask);
 		break;
 	case UFSHCD_INT_DISABLE:
 		if (hba->ufs_version == UFSHCI_VERSION_10)
-			writel(INTERRUPT_DISABLE_MASK_10,
-			      (hba->mmio_base + REG_INTERRUPT_ENABLE));
+			ufshcd_writel(hba, REG_INTERRUPT_ENABLE,
+				      INTERRUPT_DISABLE_MASK_10);
 		else
-			writel(INTERRUPT_DISABLE_MASK_11,
-			       (hba->mmio_base + REG_INTERRUPT_ENABLE));
+			ufshcd_writel(hba, REG_INTERRUPT_ENABLE,
+				      INTERRUPT_DISABLE_MASK_11);
 		break;
 	}
 }
@@ -712,7 +699,7 @@ static int ufshcd_dme_link_startup(struct ufs_hba *hba)
 	unsigned long flags;
 
 	/* check if controller is ready to accept UIC commands */
-	if (((readl(hba->mmio_base + REG_CONTROLLER_STATUS)) &
+	if (((ufshcd_readl(hba, REG_CONTROLLER_STATUS)) &
 	    UIC_COMMAND_READY) == 0x0) {
 		dev_err(hba->dev,
 			"Controller not ready"
@@ -757,7 +744,7 @@ static int ufshcd_make_hba_operational(struct ufs_hba *hba)
 	u32 reg;
 
 	/* check if device present */
-	reg = readl((hba->mmio_base + REG_CONTROLLER_STATUS));
+	reg = ufshcd_readl(hba, REG_CONTROLLER_STATUS);
 	if (!ufshcd_is_device_present(reg)) {
 		dev_err(hba->dev, "cc: Device not present\n");
 		err = -ENXIO;
@@ -879,14 +866,14 @@ static int ufshcd_initialize_hba(struct ufs_hba *hba)
 		return -EIO;
 
 	/* Configure UTRL and UTMRL base address registers */
-	writel(lower_32_bits(hba->utrdl_dma_addr),
-	       (hba->mmio_base + REG_UTP_TRANSFER_REQ_LIST_BASE_L));
-	writel(upper_32_bits(hba->utrdl_dma_addr),
-	       (hba->mmio_base + REG_UTP_TRANSFER_REQ_LIST_BASE_H));
-	writel(lower_32_bits(hba->utmrdl_dma_addr),
-	       (hba->mmio_base + REG_UTP_TASK_REQ_LIST_BASE_L));
-	writel(upper_32_bits(hba->utmrdl_dma_addr),
-	       (hba->mmio_base + REG_UTP_TASK_REQ_LIST_BASE_H));
+	ufshcd_writel(hba, REG_UTP_TRANSFER_REQ_LIST_BASE_L,
+		      lower_32_bits(hba->utrdl_dma_addr));
+	ufshcd_writel(hba, REG_UTP_TRANSFER_REQ_LIST_BASE_H,
+		      upper_32_bits(hba->utrdl_dma_addr));
+	ufshcd_writel(hba, REG_UTP_TASK_REQ_LIST_BASE_L,
+		      lower_32_bits(hba->utmrdl_dma_addr));
+	ufshcd_writel(hba, REG_UTP_TASK_REQ_LIST_BASE_H,
+		      upper_32_bits(hba->utmrdl_dma_addr));
 
 	/* Initialize unipro link startup procedure */
 	return ufshcd_dme_link_startup(hba);
@@ -1178,8 +1165,7 @@ static void ufshcd_transfer_req_compl(struct ufs_hba *hba)
 	int index;
 
 	lrb = hba->lrb;
-	tr_doorbell =
-		readl(hba->mmio_base + REG_UTP_TRANSFER_REQ_DOOR_BELL);
+	tr_doorbell = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL);
 	completed_reqs = tr_doorbell ^ hba->outstanding_reqs;
 
 	for (index = 0; index < hba->nutrs; index++) {
@@ -1253,9 +1239,7 @@ static void ufshcd_err_handler(struct ufs_hba *hba)
 		goto fatal_eh;
 
 	if (hba->errors & UIC_ERROR) {
-
-		reg = readl(hba->mmio_base +
-			    REG_UIC_ERROR_CODE_PHY_ADAPTER_LAYER);
+		reg = ufshcd_readl(hba, REG_UIC_ERROR_CODE_PHY_ADAPTER_LAYER);
 		if (reg & UIC_DATA_LINK_LAYER_ERROR_PA_INIT)
 			goto fatal_eh;
 	}
@@ -1273,7 +1257,7 @@ static void ufshcd_tmc_handler(struct ufs_hba *hba)
 {
 	u32 tm_doorbell;
 
-	tm_doorbell = readl(hba->mmio_base + REG_UTP_TASK_REQ_DOOR_BELL);
+	tm_doorbell = ufshcd_readl(hba, REG_UTP_TASK_REQ_DOOR_BELL);
 	hba->tm_condition = tm_doorbell ^ hba->outstanding_tasks;
 	wake_up_interruptible(&hba->ufshcd_tm_wait_queue);
 }
@@ -1314,15 +1298,14 @@ static irqreturn_t ufshcd_intr(int irq, void *__hba)
 	struct ufs_hba *hba = __hba;
 
 	spin_lock(hba->host->host_lock);
-	intr_status = readl(hba->mmio_base + REG_INTERRUPT_STATUS);
+	intr_status = ufshcd_readl(hba, REG_INTERRUPT_STATUS);
 
 	if (intr_status) {
 		ufshcd_sl_intr(hba, intr_status);
 
 		/* If UFSHCI 1.0 then clear interrupt status register */
 		if (hba->ufs_version == UFSHCI_VERSION_10)
-			writel(intr_status,
-			       (hba->mmio_base + REG_INTERRUPT_STATUS));
+			ufshcd_writel(hba, REG_INTERRUPT_STATUS, intr_status);
 		retval = IRQ_HANDLED;
 	}
 	spin_unlock(hba->host->host_lock);
@@ -1387,8 +1370,7 @@ ufshcd_issue_tm_cmd(struct ufs_hba *hba,
 
 	/* send command to the controller */
 	__set_bit(free_slot, &hba->outstanding_tasks);
-	writel((1 << free_slot),
-	       (hba->mmio_base + REG_UTP_TASK_REQ_DOOR_BELL));
+	ufshcd_writel(hba, REG_UTP_TASK_REQ_DOOR_BELL, 1 << free_slot);
 
 	spin_unlock_irqrestore(host->host_lock, flags);
 
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)	\
+	writel((val), (hba)->mmio_base + (reg))
+#define ufshcd_readl(hba, reg)	\
+	readl((hba)->mmio_base + (reg))
+
 #endif /* End of Header */
-- 
1.7.0.4



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/5] scsi: ufs: wrap the i/o access operations
  2013-04-24 16:06 [PATCH 2/5] scsi: ufs: wrap the i/o access operations Seungwon Jeon
@ 2013-04-25  4:49 ` Sujit Reddy Thumma
  2013-04-26  5:06   ` Seungwon Jeon
  2013-05-04  8:45 ` [PATCH v2 2/7] " Seungwon Jeon
  1 sibling, 1 reply; 7+ messages in thread
From: Sujit Reddy Thumma @ 2013-04-25  4:49 UTC (permalink / raw)
  To: Seungwon Jeon
  Cc: linux-scsi, 'Vinayak Holikatti', 'Santosh Y',
	'James E.J. Bottomley'

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.c b/drivers/scsi/ufs/ufshcd.c
> index 41b9639..b6c19b0 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -71,7 +71,7 @@ enum {
>    */
>   static inline u32 ufshcd_get_ufs_version(struct ufs_hba *hba)
>   {
> -	return readl(hba->mmio_base + REG_UFS_VERSION);
> +	return ufshcd_readl(hba, REG_UFS_VERSION);
>   }
>
>   /**
> @@ -130,8 +130,7 @@ static inline int ufshcd_get_tm_free_slot(struct ufs_hba *hba)
>    */
>   static inline void ufshcd_utrl_clear(struct ufs_hba *hba, u32 pos)
>   {
> -	writel(~(1 << pos),
> -		(hba->mmio_base + REG_UTP_TRANSFER_REQ_LIST_CLEAR));
> +	ufshcd_writel(hba, REG_UTP_TRANSFER_REQ_LIST_CLEAR, ~(1 << pos));
>   }
>
>   /**
> @@ -165,7 +164,7 @@ static inline int ufshcd_get_lists_status(u32 reg)
>    */
>   static inline int ufshcd_get_uic_cmd_result(struct ufs_hba *hba)
>   {
> -	return readl(hba->mmio_base + REG_UIC_COMMAND_ARG_2) &
> +	return ufshcd_readl(hba, REG_UIC_COMMAND_ARG_2) &
>   	       MASK_UIC_COMMAND_RESULT;
>   }
>
> @@ -243,18 +242,14 @@ ufshcd_config_int_aggr(struct ufs_hba *hba, int option)
>   {
>   	switch (option) {
>   	case INT_AGGR_RESET:
> -		writel((INT_AGGR_ENABLE |
> -			INT_AGGR_COUNTER_AND_TIMER_RESET),
> -			(hba->mmio_base +
> -			 REG_UTP_TRANSFER_REQ_INT_AGG_CONTROL));
> +		ufshcd_writel(hba, REG_UTP_TRANSFER_REQ_INT_AGG_CONTROL,
> +			      INT_AGGR_ENABLE | INT_AGGR_COUNTER_AND_TIMER_RESET);
>   		break;
>   	case INT_AGGR_CONFIG:
> -		writel((INT_AGGR_ENABLE |
> -			INT_AGGR_PARAM_WRITE |
> -			INT_AGGR_COUNTER_THRESHOLD_VALUE |
> -			INT_AGGR_TIMEOUT_VALUE),
> -			(hba->mmio_base +
> -			 REG_UTP_TRANSFER_REQ_INT_AGG_CONTROL));
> +		ufshcd_writel(hba, REG_UTP_TRANSFER_REQ_INT_AGG_CONTROL,
> +			      INT_AGGR_ENABLE | INT_AGGR_PARAM_WRITE |
> +			      INT_AGGR_COUNTER_THRESHOLD_VALUE |
> +			      INT_AGGR_TIMEOUT_VALUE);
>   		break;
>   	}
>   }
> @@ -267,12 +262,10 @@ ufshcd_config_int_aggr(struct ufs_hba *hba, int option)
>    */
>   static void ufshcd_enable_run_stop_reg(struct ufs_hba *hba)
>   {
> -	writel(UTP_TASK_REQ_LIST_RUN_STOP_BIT,
> -	       (hba->mmio_base +
> -		REG_UTP_TASK_REQ_LIST_RUN_STOP));
> -	writel(UTP_TRANSFER_REQ_LIST_RUN_STOP_BIT,
> -	       (hba->mmio_base +
> -		REG_UTP_TRANSFER_REQ_LIST_RUN_STOP));
> +	ufshcd_writel(hba, REG_UTP_TASK_REQ_LIST_RUN_STOP,
> +		      UTP_TASK_REQ_LIST_RUN_STOP_BIT);
> +	ufshcd_writel(hba, REG_UTP_TRANSFER_REQ_LIST_RUN_STOP,
> +		      UTP_TRANSFER_REQ_LIST_RUN_STOP_BIT);
>   }
>
>   /**
> @@ -281,7 +274,7 @@ static void ufshcd_enable_run_stop_reg(struct ufs_hba *hba)
>    */
>   static inline void ufshcd_hba_start(struct ufs_hba *hba)
>   {
> -	writel(CONTROLLER_ENABLE , (hba->mmio_base + REG_CONTROLLER_ENABLE));
> +	ufshcd_writel(hba, REG_CONTROLLER_ENABLE, CONTROLLER_ENABLE);
>   }
>
>   /**
> @@ -290,7 +283,7 @@ static inline void ufshcd_hba_start(struct ufs_hba *hba)
>    */
>   static inline void ufshcd_hba_stop(struct ufs_hba *hba)
>   {
> -	writel(CONTROLLER_DISABLE, (hba->mmio_base + REG_CONTROLLER_ENABLE));
> +	ufshcd_writel(hba, REG_CONTROLLER_ENABLE, CONTROLLER_DISABLE);
>   }
>
>   /**
> @@ -301,7 +294,7 @@ static inline void ufshcd_hba_stop(struct ufs_hba *hba)
>    */
>   static inline int ufshcd_is_hba_active(struct ufs_hba *hba)
>   {
> -	return (readl(hba->mmio_base + REG_CONTROLLER_ENABLE) & 0x1) ? 0 : 1;
> +	return (ufshcd_readl(hba, REG_CONTROLLER_ENABLE) & 0x1) ? 0 : 1;
>   }
>
>   /**
> @@ -313,8 +306,7 @@ static inline
>   void ufshcd_send_command(struct ufs_hba *hba, unsigned int task_tag)
>   {
>   	__set_bit(task_tag, &hba->outstanding_reqs);
> -	writel((1 << task_tag),
> -	       (hba->mmio_base + REG_UTP_TRANSFER_REQ_DOOR_BELL));
> +	ufshcd_writel(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL, 1 << task_tag);
>   }
>
>   /**
> @@ -338,8 +330,7 @@ static inline void ufshcd_copy_sense_data(struct ufshcd_lrb *lrbp)
>    */
>   static inline void ufshcd_hba_capabilities(struct ufs_hba *hba)
>   {
> -	hba->capabilities =
> -		readl(hba->mmio_base + REG_CONTROLLER_CAPABILITIES);
> +	hba->capabilities = ufshcd_readl(hba, REG_CONTROLLER_CAPABILITIES);
>
>   	/* nutrs and nutmrs are 0 based values */
>   	hba->nutrs = (hba->capabilities & MASK_TRANSFER_REQUESTS_SLOTS) + 1;
> @@ -356,16 +347,13 @@ static inline void
>   ufshcd_send_uic_command(struct ufs_hba *hba, struct uic_command *uic_cmnd)
>   {
>   	/* Write Args */
> -	writel(uic_cmnd->argument1,
> -	      (hba->mmio_base + REG_UIC_COMMAND_ARG_1));
> -	writel(uic_cmnd->argument2,
> -	      (hba->mmio_base + REG_UIC_COMMAND_ARG_2));
> -	writel(uic_cmnd->argument3,
> -	      (hba->mmio_base + REG_UIC_COMMAND_ARG_3));
> +	ufshcd_writel(hba, REG_UIC_COMMAND_ARG_1, uic_cmnd->argument1);
> +	ufshcd_writel(hba, REG_UIC_COMMAND_ARG_2, uic_cmnd->argument2);
> +	ufshcd_writel(hba, REG_UIC_COMMAND_ARG_3, uic_cmnd->argument3);
>
>   	/* Write UIC Cmd */
> -	writel((uic_cmnd->command & COMMAND_OPCODE_MASK),
> -	       (hba->mmio_base + REG_UIC_COMMAND));
> +	ufshcd_writel(hba, REG_UIC_COMMAND,
> +		      uic_cmnd->command & COMMAND_OPCODE_MASK);
>   }
>
>   /**
> @@ -417,16 +405,15 @@ static void ufshcd_int_config(struct ufs_hba *hba, u32 option)
>   {
>   	switch (option) {
>   	case UFSHCD_INT_ENABLE:
> -		writel(hba->int_enable_mask,
> -		      (hba->mmio_base + REG_INTERRUPT_ENABLE));
> +		ufshcd_writel(hba, REG_INTERRUPT_ENABLE, hba->int_enable_mask);
>   		break;
>   	case UFSHCD_INT_DISABLE:
>   		if (hba->ufs_version == UFSHCI_VERSION_10)
> -			writel(INTERRUPT_DISABLE_MASK_10,
> -			      (hba->mmio_base + REG_INTERRUPT_ENABLE));
> +			ufshcd_writel(hba, REG_INTERRUPT_ENABLE,
> +				      INTERRUPT_DISABLE_MASK_10);
>   		else
> -			writel(INTERRUPT_DISABLE_MASK_11,
> -			       (hba->mmio_base + REG_INTERRUPT_ENABLE));
> +			ufshcd_writel(hba, REG_INTERRUPT_ENABLE,
> +				      INTERRUPT_DISABLE_MASK_11);
>   		break;
>   	}
>   }
> @@ -712,7 +699,7 @@ static int ufshcd_dme_link_startup(struct ufs_hba *hba)
>   	unsigned long flags;
>
>   	/* check if controller is ready to accept UIC commands */
> -	if (((readl(hba->mmio_base + REG_CONTROLLER_STATUS)) &
> +	if (((ufshcd_readl(hba, REG_CONTROLLER_STATUS)) &
redundant bracket

>   	    UIC_COMMAND_READY) == 0x0) {
>   		dev_err(hba->dev,
>   			"Controller not ready"
> @@ -757,7 +744,7 @@ static int ufshcd_make_hba_operational(struct ufs_hba *hba)
>   	u32 reg;
>
>   	/* check if device present */
> -	reg = readl((hba->mmio_base + REG_CONTROLLER_STATUS));
> +	reg = ufshcd_readl(hba, REG_CONTROLLER_STATUS);
>   	if (!ufshcd_is_device_present(reg)) {
>   		dev_err(hba->dev, "cc: Device not present\n");
>   		err = -ENXIO;
> @@ -879,14 +866,14 @@ static int ufshcd_initialize_hba(struct ufs_hba *hba)
>   		return -EIO;
>
>   	/* Configure UTRL and UTMRL base address registers */
> -	writel(lower_32_bits(hba->utrdl_dma_addr),
> -	       (hba->mmio_base + REG_UTP_TRANSFER_REQ_LIST_BASE_L));
> -	writel(upper_32_bits(hba->utrdl_dma_addr),
> -	       (hba->mmio_base + REG_UTP_TRANSFER_REQ_LIST_BASE_H));
> -	writel(lower_32_bits(hba->utmrdl_dma_addr),
> -	       (hba->mmio_base + REG_UTP_TASK_REQ_LIST_BASE_L));
> -	writel(upper_32_bits(hba->utmrdl_dma_addr),
> -	       (hba->mmio_base + REG_UTP_TASK_REQ_LIST_BASE_H));
> +	ufshcd_writel(hba, REG_UTP_TRANSFER_REQ_LIST_BASE_L,
> +		      lower_32_bits(hba->utrdl_dma_addr));
> +	ufshcd_writel(hba, REG_UTP_TRANSFER_REQ_LIST_BASE_H,
> +		      upper_32_bits(hba->utrdl_dma_addr));
> +	ufshcd_writel(hba, REG_UTP_TASK_REQ_LIST_BASE_L,
> +		      lower_32_bits(hba->utmrdl_dma_addr));
> +	ufshcd_writel(hba, REG_UTP_TASK_REQ_LIST_BASE_H,
> +		      upper_32_bits(hba->utmrdl_dma_addr));
>
>   	/* Initialize unipro link startup procedure */
>   	return ufshcd_dme_link_startup(hba);
> @@ -1178,8 +1165,7 @@ static void ufshcd_transfer_req_compl(struct ufs_hba *hba)
>   	int index;
>
>   	lrb = hba->lrb;
> -	tr_doorbell =
> -		readl(hba->mmio_base + REG_UTP_TRANSFER_REQ_DOOR_BELL);
> +	tr_doorbell = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL);
>   	completed_reqs = tr_doorbell ^ hba->outstanding_reqs;
>
>   	for (index = 0; index < hba->nutrs; index++) {
> @@ -1253,9 +1239,7 @@ static void ufshcd_err_handler(struct ufs_hba *hba)
>   		goto fatal_eh;
>
>   	if (hba->errors & UIC_ERROR) {
> -
> -		reg = readl(hba->mmio_base +
> -			    REG_UIC_ERROR_CODE_PHY_ADAPTER_LAYER);
> +		reg = ufshcd_readl(hba, REG_UIC_ERROR_CODE_PHY_ADAPTER_LAYER);
>   		if (reg & UIC_DATA_LINK_LAYER_ERROR_PA_INIT)
>   			goto fatal_eh;
>   	}
> @@ -1273,7 +1257,7 @@ static void ufshcd_tmc_handler(struct ufs_hba *hba)
>   {
>   	u32 tm_doorbell;
>
> -	tm_doorbell = readl(hba->mmio_base + REG_UTP_TASK_REQ_DOOR_BELL);
> +	tm_doorbell = ufshcd_readl(hba, REG_UTP_TASK_REQ_DOOR_BELL);
>   	hba->tm_condition = tm_doorbell ^ hba->outstanding_tasks;
>   	wake_up_interruptible(&hba->ufshcd_tm_wait_queue);
>   }
> @@ -1314,15 +1298,14 @@ static irqreturn_t ufshcd_intr(int irq, void *__hba)
>   	struct ufs_hba *hba = __hba;
>
>   	spin_lock(hba->host->host_lock);
> -	intr_status = readl(hba->mmio_base + REG_INTERRUPT_STATUS);
> +	intr_status = ufshcd_readl(hba, REG_INTERRUPT_STATUS);
>
>   	if (intr_status) {
>   		ufshcd_sl_intr(hba, intr_status);
>
>   		/* If UFSHCI 1.0 then clear interrupt status register */
>   		if (hba->ufs_version == UFSHCI_VERSION_10)
> -			writel(intr_status,
> -			       (hba->mmio_base + REG_INTERRUPT_STATUS));
> +			ufshcd_writel(hba, REG_INTERRUPT_STATUS, intr_status);
>   		retval = IRQ_HANDLED;
>   	}
>   	spin_unlock(hba->host->host_lock);
> @@ -1387,8 +1370,7 @@ ufshcd_issue_tm_cmd(struct ufs_hba *hba,
>
>   	/* send command to the controller */
>   	__set_bit(free_slot, &hba->outstanding_tasks);
> -	writel((1 << free_slot),
> -	       (hba->mmio_base + REG_UTP_TASK_REQ_DOOR_BELL));
> +	ufshcd_writel(hba, REG_UTP_TASK_REQ_DOOR_BELL, 1 << free_slot);
>
>   	spin_unlock_irqrestore(host->host_lock, flags);
>
> 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?

> +	writel((val), (hba)->mmio_base + (reg))
> +#define ufshcd_readl(hba, reg)	\
> +	readl((hba)->mmio_base + (reg))
> +
>   #endif /* End of Header */
>

-- 
Regards,
Sujit

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH 2/5] scsi: ufs: wrap the i/o access operations
  2013-04-25  4:49 ` Sujit Reddy Thumma
@ 2013-04-26  5:06   ` Seungwon Jeon
  2013-04-30 11:23     ` Subhash Jadavani
  0 siblings, 1 reply; 7+ messages in thread
From: Seungwon Jeon @ 2013-04-26  5:06 UTC (permalink / raw)
  To: 'Sujit Reddy Thumma'
  Cc: linux-scsi, 'Vinayak Holikatti', 'Santosh Y',
	'James E.J. Bottomley'

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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/5] scsi: ufs: wrap the i/o access operations
  2013-04-26  5:06   ` Seungwon Jeon
@ 2013-04-30 11:23     ` Subhash Jadavani
  2013-05-01  7:49       ` merez
  0 siblings, 1 reply; 7+ messages in thread
From: Subhash Jadavani @ 2013-04-30 11:23 UTC (permalink / raw)
  To: Seungwon Jeon
  Cc: 'Sujit Reddy Thumma', linux-scsi,
	'Vinayak Holikatti', 'Santosh Y',
	'James E.J. Bottomley'

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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/5] scsi: ufs: wrap the i/o access operations
  2013-04-30 11:23     ` Subhash Jadavani
@ 2013-05-01  7:49       ` merez
  2013-05-02  7:00         ` Seungwon Jeon
  0 siblings, 1 reply; 7+ messages in thread
From: merez @ 2013-05-01  7:49 UTC (permalink / raw)
  To: Subhash Jadavani
  Cc: Seungwon Jeon, 'Sujit Reddy Thumma', linux-scsi,
	'Vinayak Holikatti', 'Santosh Y',
	'James E.J. Bottomley'

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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH 2/5] scsi: ufs: wrap the i/o access operations
  2013-05-01  7:49       ` merez
@ 2013-05-02  7:00         ` Seungwon Jeon
  0 siblings, 0 replies; 7+ messages in thread
From: Seungwon Jeon @ 2013-05-02  7:00 UTC (permalink / raw)
  To: merez, 'Subhash Jadavani'
  Cc: 'Sujit Reddy Thumma', linux-scsi,
	'Vinayak Holikatti', 'Santosh Y',
	'James E.J. Bottomley'

On Wednesday, May 01, 2013, merez wrote:
> Tested-by: Maya Erez <merez@codeaurora.org>
> 
> I also tend to agree with Sujit on the order of the wrappers parameters.
Okay, I'll take the views of all(Sujit, Subhash and you)

Thanks,
Seungwon Jeon
> 
> 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
> 
> --
> 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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v2 2/7] scsi: ufs: wrap the i/o access operations
  2013-04-24 16:06 [PATCH 2/5] scsi: ufs: wrap the i/o access operations Seungwon Jeon
  2013-04-25  4:49 ` Sujit Reddy Thumma
@ 2013-05-04  8:45 ` Seungwon Jeon
  1 sibling, 0 replies; 7+ messages in thread
From: Seungwon Jeon @ 2013-05-04  8:45 UTC (permalink / raw)
  To: linux-scsi
  Cc: 'Vinayak Holikatti', 'Santosh Y',
	'James E.J. Bottomley'

Simplify operations with hiding mmio_base.

Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
Tested-by: Maya Erez <merez@codeaurora.org>
---
 drivers/scsi/ufs/ufshcd.c |  107 +++++++++++++++++++--------------------------
 drivers/scsi/ufs/ufshcd.h |    5 ++
 2 files changed, 50 insertions(+), 62 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index d932239..af7b81b 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -71,7 +71,7 @@ enum {
  */
 static inline u32 ufshcd_get_ufs_version(struct ufs_hba *hba)
 {
-	return readl(hba->mmio_base + REG_UFS_VERSION);
+	return ufshcd_readl(hba, REG_UFS_VERSION);
 }
 
 /**
@@ -130,8 +130,7 @@ static inline int ufshcd_get_tm_free_slot(struct ufs_hba *hba)
  */
 static inline void ufshcd_utrl_clear(struct ufs_hba *hba, u32 pos)
 {
-	writel(~(1 << pos),
-		(hba->mmio_base + REG_UTP_TRANSFER_REQ_LIST_CLEAR));
+	ufshcd_writel(hba, ~(1 << pos), REG_UTP_TRANSFER_REQ_LIST_CLEAR);
 }
 
 /**
@@ -165,7 +164,7 @@ static inline int ufshcd_get_lists_status(u32 reg)
  */
 static inline int ufshcd_get_uic_cmd_result(struct ufs_hba *hba)
 {
-	return readl(hba->mmio_base + REG_UIC_COMMAND_ARG_2) &
+	return ufshcd_readl(hba, REG_UIC_COMMAND_ARG_2) &
 	       MASK_UIC_COMMAND_RESULT;
 }
 
@@ -238,18 +237,15 @@ ufshcd_config_int_aggr(struct ufs_hba *hba, int option)
 {
 	switch (option) {
 	case INT_AGGR_RESET:
-		writel((INT_AGGR_ENABLE |
-			INT_AGGR_COUNTER_AND_TIMER_RESET),
-			(hba->mmio_base +
-			 REG_UTP_TRANSFER_REQ_INT_AGG_CONTROL));
+		ufshcd_writel(hba, INT_AGGR_ENABLE |
+			      INT_AGGR_COUNTER_AND_TIMER_RESET,
+			      REG_UTP_TRANSFER_REQ_INT_AGG_CONTROL);
 		break;
 	case INT_AGGR_CONFIG:
-		writel((INT_AGGR_ENABLE |
-			INT_AGGR_PARAM_WRITE |
-			INT_AGGR_COUNTER_THRESHOLD_VALUE |
-			INT_AGGR_TIMEOUT_VALUE),
-			(hba->mmio_base +
-			 REG_UTP_TRANSFER_REQ_INT_AGG_CONTROL));
+		ufshcd_writel(hba, INT_AGGR_ENABLE | INT_AGGR_PARAM_WRITE |
+			      INT_AGGR_COUNTER_THRESHOLD_VALUE |
+			      INT_AGGR_TIMEOUT_VALUE,
+			      REG_UTP_TRANSFER_REQ_INT_AGG_CONTROL);
 		break;
 	}
 }
@@ -262,12 +258,10 @@ ufshcd_config_int_aggr(struct ufs_hba *hba, int option)
  */
 static void ufshcd_enable_run_stop_reg(struct ufs_hba *hba)
 {
-	writel(UTP_TASK_REQ_LIST_RUN_STOP_BIT,
-	       (hba->mmio_base +
-		REG_UTP_TASK_REQ_LIST_RUN_STOP));
-	writel(UTP_TRANSFER_REQ_LIST_RUN_STOP_BIT,
-	       (hba->mmio_base +
-		REG_UTP_TRANSFER_REQ_LIST_RUN_STOP));
+	ufshcd_writel(hba, UTP_TASK_REQ_LIST_RUN_STOP_BIT,
+		      REG_UTP_TASK_REQ_LIST_RUN_STOP);
+	ufshcd_writel(hba, UTP_TRANSFER_REQ_LIST_RUN_STOP_BIT,
+		      REG_UTP_TRANSFER_REQ_LIST_RUN_STOP);
 }
 
 /**
@@ -276,7 +270,7 @@ static void ufshcd_enable_run_stop_reg(struct ufs_hba *hba)
  */
 static inline void ufshcd_hba_start(struct ufs_hba *hba)
 {
-	writel(CONTROLLER_ENABLE , (hba->mmio_base + REG_CONTROLLER_ENABLE));
+	ufshcd_writel(hba, CONTROLLER_ENABLE, REG_CONTROLLER_ENABLE);
 }
 
 /**
@@ -285,7 +279,7 @@ static inline void ufshcd_hba_start(struct ufs_hba *hba)
  */
 static inline void ufshcd_hba_stop(struct ufs_hba *hba)
 {
-	writel(CONTROLLER_DISABLE, (hba->mmio_base + REG_CONTROLLER_ENABLE));
+	ufshcd_writel(hba, CONTROLLER_DISABLE, REG_CONTROLLER_ENABLE);
 }
 
 /**
@@ -296,7 +290,7 @@ static inline void ufshcd_hba_stop(struct ufs_hba *hba)
  */
 static inline int ufshcd_is_hba_active(struct ufs_hba *hba)
 {
-	return (readl(hba->mmio_base + REG_CONTROLLER_ENABLE) & 0x1) ? 0 : 1;
+	return (ufshcd_readl(hba, REG_CONTROLLER_ENABLE) & 0x1) ? 0 : 1;
 }
 
 /**
@@ -308,8 +302,7 @@ static inline
 void ufshcd_send_command(struct ufs_hba *hba, unsigned int task_tag)
 {
 	__set_bit(task_tag, &hba->outstanding_reqs);
-	writel((1 << task_tag),
-	       (hba->mmio_base + REG_UTP_TRANSFER_REQ_DOOR_BELL));
+	ufshcd_writel(hba, 1 << task_tag, REG_UTP_TRANSFER_REQ_DOOR_BELL);
 }
 
 /**
@@ -390,8 +383,7 @@ void ufshcd_copy_query_response(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
  */
 static inline void ufshcd_hba_capabilities(struct ufs_hba *hba)
 {
-	hba->capabilities =
-		readl(hba->mmio_base + REG_CONTROLLER_CAPABILITIES);
+	hba->capabilities = ufshcd_readl(hba, REG_CONTROLLER_CAPABILITIES);
 
 	/* nutrs and nutmrs are 0 based values */
 	hba->nutrs = (hba->capabilities & MASK_TRANSFER_REQUESTS_SLOTS) + 1;
@@ -408,16 +400,13 @@ static inline void
 ufshcd_send_uic_command(struct ufs_hba *hba, struct uic_command *uic_cmnd)
 {
 	/* Write Args */
-	writel(uic_cmnd->argument1,
-	      (hba->mmio_base + REG_UIC_COMMAND_ARG_1));
-	writel(uic_cmnd->argument2,
-	      (hba->mmio_base + REG_UIC_COMMAND_ARG_2));
-	writel(uic_cmnd->argument3,
-	      (hba->mmio_base + REG_UIC_COMMAND_ARG_3));
+	ufshcd_writel(hba, uic_cmnd->argument1, REG_UIC_COMMAND_ARG_1);
+	ufshcd_writel(hba, uic_cmnd->argument2, REG_UIC_COMMAND_ARG_2);
+	ufshcd_writel(hba, uic_cmnd->argument3, REG_UIC_COMMAND_ARG_3);
 
 	/* Write UIC Cmd */
-	writel((uic_cmnd->command & COMMAND_OPCODE_MASK),
-	       (hba->mmio_base + REG_UIC_COMMAND));
+	ufshcd_writel(hba, uic_cmnd->command & COMMAND_OPCODE_MASK,
+		      REG_UIC_COMMAND);
 }
 
 /**
@@ -469,16 +458,15 @@ static void ufshcd_int_config(struct ufs_hba *hba, u32 option)
 {
 	switch (option) {
 	case UFSHCD_INT_ENABLE:
-		writel(hba->int_enable_mask,
-		      (hba->mmio_base + REG_INTERRUPT_ENABLE));
+		ufshcd_writel(hba, hba->int_enable_mask, REG_INTERRUPT_ENABLE);
 		break;
 	case UFSHCD_INT_DISABLE:
 		if (hba->ufs_version == UFSHCI_VERSION_10)
-			writel(INTERRUPT_DISABLE_MASK_10,
-			      (hba->mmio_base + REG_INTERRUPT_ENABLE));
+			ufshcd_writel(hba, INTERRUPT_DISABLE_MASK_10,
+				      REG_INTERRUPT_ENABLE);
 		else
-			writel(INTERRUPT_DISABLE_MASK_11,
-			       (hba->mmio_base + REG_INTERRUPT_ENABLE));
+			ufshcd_writel(hba, INTERRUPT_DISABLE_MASK_11,
+				      REG_INTERRUPT_ENABLE);
 		break;
 	}
 }
@@ -949,7 +937,7 @@ static int ufshcd_dme_link_startup(struct ufs_hba *hba)
 	unsigned long flags;
 
 	/* check if controller is ready to accept UIC commands */
-	if (((readl(hba->mmio_base + REG_CONTROLLER_STATUS)) &
+	if ((ufshcd_readl(hba, REG_CONTROLLER_STATUS) &
 	    UIC_COMMAND_READY) == 0x0) {
 		dev_err(hba->dev,
 			"Controller not ready"
@@ -994,7 +982,7 @@ static int ufshcd_make_hba_operational(struct ufs_hba *hba)
 	u32 reg;
 
 	/* check if device present */
-	reg = readl((hba->mmio_base + REG_CONTROLLER_STATUS));
+	reg = ufshcd_readl(hba, REG_CONTROLLER_STATUS);
 	if (!ufshcd_is_device_present(reg)) {
 		dev_err(hba->dev, "cc: Device not present\n");
 		err = -ENXIO;
@@ -1116,14 +1104,14 @@ static int ufshcd_initialize_hba(struct ufs_hba *hba)
 		return -EIO;
 
 	/* Configure UTRL and UTMRL base address registers */
-	writel(lower_32_bits(hba->utrdl_dma_addr),
-	       (hba->mmio_base + REG_UTP_TRANSFER_REQ_LIST_BASE_L));
-	writel(upper_32_bits(hba->utrdl_dma_addr),
-	       (hba->mmio_base + REG_UTP_TRANSFER_REQ_LIST_BASE_H));
-	writel(lower_32_bits(hba->utmrdl_dma_addr),
-	       (hba->mmio_base + REG_UTP_TASK_REQ_LIST_BASE_L));
-	writel(upper_32_bits(hba->utmrdl_dma_addr),
-	       (hba->mmio_base + REG_UTP_TASK_REQ_LIST_BASE_H));
+	ufshcd_writel(hba, lower_32_bits(hba->utrdl_dma_addr),
+		      REG_UTP_TRANSFER_REQ_LIST_BASE_L);
+	ufshcd_writel(hba, upper_32_bits(hba->utrdl_dma_addr),
+		      REG_UTP_TRANSFER_REQ_LIST_BASE_H);
+	ufshcd_writel(hba, lower_32_bits(hba->utmrdl_dma_addr),
+		      REG_UTP_TASK_REQ_LIST_BASE_L);
+	ufshcd_writel(hba, upper_32_bits(hba->utmrdl_dma_addr),
+		      REG_UTP_TASK_REQ_LIST_BASE_H);
 
 	/* Initialize unipro link startup procedure */
 	return ufshcd_dme_link_startup(hba);
@@ -1436,8 +1424,7 @@ static void ufshcd_transfer_req_compl(struct ufs_hba *hba)
 	int index;
 
 	lrb = hba->lrb;
-	tr_doorbell =
-		readl(hba->mmio_base + REG_UTP_TRANSFER_REQ_DOOR_BELL);
+	tr_doorbell = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL);
 	completed_reqs = tr_doorbell ^ hba->outstanding_reqs;
 
 	for (index = 0; index < hba->nutrs; index++) {
@@ -1511,9 +1498,7 @@ static void ufshcd_err_handler(struct ufs_hba *hba)
 		goto fatal_eh;
 
 	if (hba->errors & UIC_ERROR) {
-
-		reg = readl(hba->mmio_base +
-			    REG_UIC_ERROR_CODE_PHY_ADAPTER_LAYER);
+		reg = ufshcd_readl(hba, REG_UIC_ERROR_CODE_PHY_ADAPTER_LAYER);
 		if (reg & UIC_DATA_LINK_LAYER_ERROR_PA_INIT)
 			goto fatal_eh;
 	}
@@ -1531,7 +1516,7 @@ static void ufshcd_tmc_handler(struct ufs_hba *hba)
 {
 	u32 tm_doorbell;
 
-	tm_doorbell = readl(hba->mmio_base + REG_UTP_TASK_REQ_DOOR_BELL);
+	tm_doorbell = ufshcd_readl(hba, REG_UTP_TASK_REQ_DOOR_BELL);
 	hba->tm_condition = tm_doorbell ^ hba->outstanding_tasks;
 	wake_up_interruptible(&hba->ufshcd_tm_wait_queue);
 }
@@ -1572,15 +1557,14 @@ static irqreturn_t ufshcd_intr(int irq, void *__hba)
 	struct ufs_hba *hba = __hba;
 
 	spin_lock(hba->host->host_lock);
-	intr_status = readl(hba->mmio_base + REG_INTERRUPT_STATUS);
+	intr_status = ufshcd_readl(hba, REG_INTERRUPT_STATUS);
 
 	if (intr_status) {
 		ufshcd_sl_intr(hba, intr_status);
 
 		/* If UFSHCI 1.0 then clear interrupt status register */
 		if (hba->ufs_version == UFSHCI_VERSION_10)
-			writel(intr_status,
-			       (hba->mmio_base + REG_INTERRUPT_STATUS));
+			ufshcd_writel(hba, intr_status, REG_INTERRUPT_STATUS);
 		retval = IRQ_HANDLED;
 	}
 	spin_unlock(hba->host->host_lock);
@@ -1645,8 +1629,7 @@ ufshcd_issue_tm_cmd(struct ufs_hba *hba,
 
 	/* send command to the controller */
 	__set_bit(free_slot, &hba->outstanding_tasks);
-	writel((1 << free_slot),
-	       (hba->mmio_base + REG_UTP_TASK_REQ_DOOR_BELL));
+	ufshcd_writel(hba, 1 << free_slot, REG_UTP_TASK_REQ_DOOR_BELL);
 
 	spin_unlock_irqrestore(host->host_lock, flags);
 
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 3a1052d..f4fa183 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -204,6 +204,11 @@ struct ufs_hba {
 	struct ufs_query query;
 };
 
+#define ufshcd_writel(hba, val, reg)	\
+	writel((val), (hba)->mmio_base + (reg))
+#define ufshcd_readl(hba, reg)	\
+	readl((hba)->mmio_base + (reg))
+
 int ufshcd_init(struct device *, struct ufs_hba ** , void __iomem * ,
 			unsigned int);
 void ufshcd_remove(struct ufs_hba *);
-- 
1.7.0.4



^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2013-05-04  8:45 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-24 16:06 [PATCH 2/5] scsi: ufs: wrap the i/o access operations Seungwon Jeon
2013-04-25  4:49 ` Sujit Reddy Thumma
2013-04-26  5:06   ` Seungwon Jeon
2013-04-30 11:23     ` Subhash Jadavani
2013-05-01  7:49       ` merez
2013-05-02  7:00         ` Seungwon Jeon
2013-05-04  8:45 ` [PATCH v2 2/7] " Seungwon Jeon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox