* [PATCH 01/10] scsi: ufs: wrap the i/o access operations
2013-06-26 17:09 [PATCH RESEND 00/10] scsi: ufs: link start-up rework and other fixes Santosh Y
@ 2013-06-26 17:09 ` Santosh Y
2013-06-26 17:09 ` [PATCH 02/10] scsi: ufs: amend interrupt configuration Santosh Y
` (8 subsequent siblings)
9 siblings, 0 replies; 18+ messages in thread
From: Santosh Y @ 2013-06-26 17:09 UTC (permalink / raw)
To: james.bottomley; +Cc: linux-scsi, vinholikatti, Seungwon Jeon, Santosh Y
From: Seungwon Jeon <tgih.jun@samsung.com>
Simplify operations with hiding mmio_base.
Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
Tested-by: Maya Erez <merez@codeaurora.org>
Signed-off-by: Santosh Y <santoshsy@gmail.com>
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index c32a478..871c2f0 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;
}
@@ -243,18 +242,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;
}
}
@@ -267,12 +263,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);
}
/**
@@ -281,7 +275,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);
}
/**
@@ -292,7 +286,7 @@ static inline void ufshcd_hba_start(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;
}
/**
@@ -304,8 +298,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);
}
/**
@@ -329,8 +322,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;
@@ -347,16 +339,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);
}
/**
@@ -408,16 +397,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;
}
}
@@ -703,7 +691,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"
@@ -748,7 +736,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;
@@ -870,14 +858,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);
@@ -1169,8 +1157,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++) {
@@ -1244,9 +1231,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;
}
@@ -1264,7 +1249,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);
}
@@ -1305,15 +1290,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);
@@ -1378,8 +1362,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 6b99a42..807dd2d 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -186,6 +186,11 @@ struct ufs_hba {
u32 errors;
};
+#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 *);
@@ -196,7 +201,7 @@ void ufshcd_remove(struct ufs_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);
}
#endif /* End of Header */
--
1.8.3.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 02/10] scsi: ufs: amend interrupt configuration
2013-06-26 17:09 [PATCH RESEND 00/10] scsi: ufs: link start-up rework and other fixes Santosh Y
2013-06-26 17:09 ` [PATCH 01/10] scsi: ufs: wrap the i/o access operations Santosh Y
@ 2013-06-26 17:09 ` Santosh Y
2013-06-26 17:09 ` [PATCH 03/10] scsi: ufs: remove version check before IS reg clear Santosh Y
` (7 subsequent siblings)
9 siblings, 0 replies; 18+ messages in thread
From: Santosh Y @ 2013-06-26 17:09 UTC (permalink / raw)
To: james.bottomley; +Cc: linux-scsi, vinholikatti, Seungwon Jeon, Santosh Y
From: Seungwon Jeon <tgih.jun@samsung.com>
It makes interrupt setting more flexible especially
for disabling. And wrong bit mask is fixed for ver 1.0.
[17:16] is added for mask.
Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
Tested-by: Maya Erez <merez@codeaurora.org>
Signed-off-by: Santosh Y <santoshsy@gmail.com>
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 871c2f0..1f1e085 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -35,6 +35,10 @@
#include "ufshcd.h"
+#define UFSHCD_ENABLE_INTRS (UTP_TRANSFER_REQ_COMPL |\
+ UTP_TASK_REQ_COMPL |\
+ UFSHCD_ERROR_MASK)
+
enum {
UFSHCD_MAX_CHANNEL = 0,
UFSHCD_MAX_ID = 1,
@@ -64,6 +68,20 @@ enum {
};
/**
+ * ufshcd_get_intr_mask - Get the interrupt bit mask
+ * @hba - Pointer to adapter instance
+ *
+ * Returns interrupt bit mask per version
+ */
+static inline u32 ufshcd_get_intr_mask(struct ufs_hba *hba)
+{
+ if (hba->ufs_version == UFSHCI_VERSION_10)
+ return INTERRUPT_MASK_ALL_VER_10;
+ else
+ return INTERRUPT_MASK_ALL_VER_11;
+}
+
+/**
* ufshcd_get_ufs_version - Get the UFS version supported by the HBA
* @hba - Pointer to adapter instance
*
@@ -389,25 +407,45 @@ static int ufshcd_map_sg(struct ufshcd_lrb *lrbp)
}
/**
- * ufshcd_int_config - enable/disable interrupts
+ * ufshcd_enable_intr - enable interrupts
* @hba: per adapter instance
- * @option: interrupt option
+ * @intrs: interrupt bits
*/
-static void ufshcd_int_config(struct ufs_hba *hba, u32 option)
+static void ufshcd_enable_intr(struct ufs_hba *hba, u32 intrs)
{
- switch (option) {
- case UFSHCD_INT_ENABLE:
- ufshcd_writel(hba, hba->int_enable_mask, REG_INTERRUPT_ENABLE);
- break;
- case UFSHCD_INT_DISABLE:
- if (hba->ufs_version == UFSHCI_VERSION_10)
- ufshcd_writel(hba, INTERRUPT_DISABLE_MASK_10,
- REG_INTERRUPT_ENABLE);
- else
- ufshcd_writel(hba, INTERRUPT_DISABLE_MASK_11,
- REG_INTERRUPT_ENABLE);
- break;
+ u32 set = ufshcd_readl(hba, REG_INTERRUPT_ENABLE);
+
+ if (hba->ufs_version == UFSHCI_VERSION_10) {
+ u32 rw;
+ rw = set & INTERRUPT_MASK_RW_VER_10;
+ set = rw | ((set ^ intrs) & intrs);
+ } else {
+ set |= intrs;
+ }
+
+ ufshcd_writel(hba, set, REG_INTERRUPT_ENABLE);
+}
+
+/**
+ * ufshcd_disable_intr - disable interrupts
+ * @hba: per adapter instance
+ * @intrs: interrupt bits
+ */
+static void ufshcd_disable_intr(struct ufs_hba *hba, u32 intrs)
+{
+ u32 set = ufshcd_readl(hba, REG_INTERRUPT_ENABLE);
+
+ if (hba->ufs_version == UFSHCI_VERSION_10) {
+ u32 rw;
+ rw = (set & INTERRUPT_MASK_RW_VER_10) &
+ ~(intrs & INTERRUPT_MASK_RW_VER_10);
+ set = rw | ((set & intrs) & ~INTERRUPT_MASK_RW_VER_10);
+
+ } else {
+ set &= ~intrs;
}
+
+ ufshcd_writel(hba, set, REG_INTERRUPT_ENABLE);
}
/**
@@ -709,8 +747,7 @@ static int ufshcd_dme_link_startup(struct ufs_hba *hba)
uic_cmd->argument3 = 0;
/* enable UIC related interrupts */
- hba->int_enable_mask |= UIC_COMMAND_COMPL;
- ufshcd_int_config(hba, UFSHCD_INT_ENABLE);
+ ufshcd_enable_intr(hba, UIC_COMMAND_COMPL);
/* sending UIC commands to controller */
ufshcd_send_uic_command(hba, uic_cmd);
@@ -757,13 +794,7 @@ static int ufshcd_make_hba_operational(struct ufs_hba *hba)
}
/* Enable required interrupts */
- hba->int_enable_mask |= (UTP_TRANSFER_REQ_COMPL |
- UIC_ERROR |
- UTP_TASK_REQ_COMPL |
- DEVICE_FATAL_ERROR |
- CONTROLLER_FATAL_ERROR |
- SYSTEM_BUS_FATAL_ERROR);
- ufshcd_int_config(hba, UFSHCD_INT_ENABLE);
+ ufshcd_enable_intr(hba, UFSHCD_ENABLE_INTRS);
/* Configure interrupt aggregation */
ufshcd_config_int_aggr(hba, INT_AGGR_CONFIG);
@@ -1570,7 +1601,7 @@ static void ufshcd_hba_free(struct ufs_hba *hba)
void ufshcd_remove(struct ufs_hba *hba)
{
/* disable interrupts */
- ufshcd_int_config(hba, UFSHCD_INT_DISABLE);
+ ufshcd_disable_intr(hba, hba->intr_mask);
ufshcd_hba_stop(hba);
ufshcd_hba_free(hba);
@@ -1628,6 +1659,9 @@ int ufshcd_init(struct device *dev, struct ufs_hba **hba_handle,
/* Get UFS version supported by the controller */
hba->ufs_version = ufshcd_get_ufs_version(hba);
+ /* Get Interrupt bit mask per version */
+ hba->intr_mask = ufshcd_get_intr_mask(hba);
+
/* Allocate memory for host memory space */
err = ufshcd_memory_alloc(hba);
if (err) {
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 807dd2d..4213600 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -139,7 +139,7 @@ struct ufshcd_lrb {
* @ufshcd_tm_wait_queue: wait queue for task management
* @tm_condition: condition variable for task management
* @ufshcd_state: UFSHCD states
- * @int_enable_mask: Interrupt Mask Bits
+ * @intr_mask: Interrupt Mask Bits
* @uic_workq: Work queue for UIC completion handling
* @feh_workq: Work queue for fatal controller error handling
* @errors: HBA errors
@@ -176,7 +176,7 @@ struct ufs_hba {
unsigned long tm_condition;
u32 ufshcd_state;
- u32 int_enable_mask;
+ u32 intr_mask;
/* Work Queues */
struct work_struct uic_workq;
diff --git a/drivers/scsi/ufs/ufshci.h b/drivers/scsi/ufs/ufshci.h
index 0c16484..d5c5f14 100644
--- a/drivers/scsi/ufs/ufshci.h
+++ b/drivers/scsi/ufs/ufshci.h
@@ -232,10 +232,11 @@ enum {
/* Interrupt disable masks */
enum {
/* Interrupt disable mask for UFSHCI v1.0 */
- INTERRUPT_DISABLE_MASK_10 = 0xFFFF,
+ INTERRUPT_MASK_ALL_VER_10 = 0x30FFF,
+ INTERRUPT_MASK_RW_VER_10 = 0x30000,
/* Interrupt disable mask for UFSHCI v1.1 */
- INTERRUPT_DISABLE_MASK_11 = 0x0,
+ INTERRUPT_MASK_ALL_VER_11 = 0x31FFF,
};
/*
--
1.8.3.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 03/10] scsi: ufs: remove version check before IS reg clear
2013-06-26 17:09 [PATCH RESEND 00/10] scsi: ufs: link start-up rework and other fixes Santosh Y
2013-06-26 17:09 ` [PATCH 01/10] scsi: ufs: wrap the i/o access operations Santosh Y
2013-06-26 17:09 ` [PATCH 02/10] scsi: ufs: amend interrupt configuration Santosh Y
@ 2013-06-26 17:09 ` Santosh Y
2013-06-26 17:09 ` [PATCH 04/10] scsi: ufs: rework link start-up process Santosh Y
` (6 subsequent siblings)
9 siblings, 0 replies; 18+ messages in thread
From: Santosh Y @ 2013-06-26 17:09 UTC (permalink / raw)
To: james.bottomley; +Cc: linux-scsi, vinholikatti, Seungwon Jeon, Santosh Y
From: Seungwon Jeon <tgih.jun@samsung.com>
There is no need to check the version to clear
the interrupt status. And the order is changed
prior to actual handling.
Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
Tested-by: Maya Erez <merez@codeaurora.org>
Signed-off-by: Santosh Y <santoshsy@gmail.com>
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 1f1e085..2e02483 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -1324,11 +1324,8 @@ static irqreturn_t ufshcd_intr(int irq, void *__hba)
intr_status = ufshcd_readl(hba, REG_INTERRUPT_STATUS);
if (intr_status) {
+ ufshcd_writel(hba, intr_status, REG_INTERRUPT_STATUS);
ufshcd_sl_intr(hba, intr_status);
-
- /* If UFSHCI 1.0 then clear interrupt status register */
- if (hba->ufs_version == UFSHCI_VERSION_10)
- ufshcd_writel(hba, intr_status, REG_INTERRUPT_STATUS);
retval = IRQ_HANDLED;
}
spin_unlock(hba->host->host_lock);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 04/10] scsi: ufs: rework link start-up process
2013-06-26 17:09 [PATCH RESEND 00/10] scsi: ufs: link start-up rework and other fixes Santosh Y
` (2 preceding siblings ...)
2013-06-26 17:09 ` [PATCH 03/10] scsi: ufs: remove version check before IS reg clear Santosh Y
@ 2013-06-26 17:09 ` Santosh Y
2013-06-26 17:09 ` [PATCH 05/10] scsi: ufs: Fix the response UPIU length setting Santosh Y
` (5 subsequent siblings)
9 siblings, 0 replies; 18+ messages in thread
From: Santosh Y @ 2013-06-26 17:09 UTC (permalink / raw)
To: james.bottomley
Cc: linux-scsi, vinholikatti, Seungwon Jeon, Sujit Reddy Thumma, Santosh Y
From: Seungwon Jeon <tgih.jun@samsung.com>
Link start-up requires long time with multiphase handshakes
between UFS host and device. This affects driver's probe time.
This patch let link start-up run asynchronously. Link start-up
will be executed at the end of prove separately.
Along with this change, the following is worked.
Defined completion time of uic command to avoid a permanent wait.
Added mutex to guarantee of uic command at a time.
Adapted some sequence of controller initialization after link statup
according to HCI standard.
Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
Signed-off-by: Sujit Reddy Thumma <sthumma@codeaurora.org>
Tested-by: Maya Erez <merez@codeaurora.org>
Signed-off-by: Santosh Y <santoshsy@gmail.com>
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 2e02483..48a7645 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -33,11 +33,15 @@
* this program.
*/
+#include <linux/async.h>
+
#include "ufshcd.h"
#define UFSHCD_ENABLE_INTRS (UTP_TRANSFER_REQ_COMPL |\
UTP_TASK_REQ_COMPL |\
UFSHCD_ERROR_MASK)
+/* UIC command timeout, unit: ms */
+#define UIC_CMD_TIMEOUT 500
enum {
UFSHCD_MAX_CHANNEL = 0,
@@ -349,24 +353,122 @@ static inline void ufshcd_hba_capabilities(struct ufs_hba *hba)
}
/**
- * ufshcd_send_uic_command - Send UIC commands to unipro layers
+ * ufshcd_ready_for_uic_cmd - Check if controller is ready
+ * to accept UIC commands
* @hba: per adapter instance
- * @uic_command: UIC command
+ * Return true on success, else false
+ */
+static inline bool ufshcd_ready_for_uic_cmd(struct ufs_hba *hba)
+{
+ if (ufshcd_readl(hba, REG_CONTROLLER_STATUS) & UIC_COMMAND_READY)
+ return true;
+ else
+ return false;
+}
+
+/**
+ * ufshcd_dispatch_uic_cmd - Dispatch UIC commands to unipro layers
+ * @hba: per adapter instance
+ * @uic_cmd: UIC command
+ *
+ * Mutex must be held.
*/
static inline void
-ufshcd_send_uic_command(struct ufs_hba *hba, struct uic_command *uic_cmnd)
+ufshcd_dispatch_uic_cmd(struct ufs_hba *hba, struct uic_command *uic_cmd)
{
+ WARN_ON(hba->active_uic_cmd);
+
+ hba->active_uic_cmd = uic_cmd;
+
/* Write Args */
- 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);
+ ufshcd_writel(hba, uic_cmd->argument1, REG_UIC_COMMAND_ARG_1);
+ ufshcd_writel(hba, uic_cmd->argument2, REG_UIC_COMMAND_ARG_2);
+ ufshcd_writel(hba, uic_cmd->argument3, REG_UIC_COMMAND_ARG_3);
/* Write UIC Cmd */
- ufshcd_writel(hba, uic_cmnd->command & COMMAND_OPCODE_MASK,
+ ufshcd_writel(hba, uic_cmd->command & COMMAND_OPCODE_MASK,
REG_UIC_COMMAND);
}
/**
+ * ufshcd_wait_for_uic_cmd - Wait complectioin of UIC command
+ * @hba: per adapter instance
+ * @uic_command: UIC command
+ *
+ * Must be called with mutex held.
+ * Returns 0 only if success.
+ */
+static int
+ufshcd_wait_for_uic_cmd(struct ufs_hba *hba, struct uic_command *uic_cmd)
+{
+ int ret;
+ unsigned long flags;
+
+ if (wait_for_completion_timeout(&uic_cmd->done,
+ msecs_to_jiffies(UIC_CMD_TIMEOUT)))
+ ret = uic_cmd->argument2 & MASK_UIC_COMMAND_RESULT;
+ else
+ ret = -ETIMEDOUT;
+
+ spin_lock_irqsave(hba->host->host_lock, flags);
+ hba->active_uic_cmd = NULL;
+ spin_unlock_irqrestore(hba->host->host_lock, flags);
+
+ return ret;
+}
+
+/**
+ * __ufshcd_send_uic_cmd - Send UIC commands and retrieve the result
+ * @hba: per adapter instance
+ * @uic_cmd: UIC command
+ *
+ * Identical to ufshcd_send_uic_cmd() expect mutex. Must be called
+ * with mutex held.
+ * Returns 0 only if success.
+ */
+static int
+__ufshcd_send_uic_cmd(struct ufs_hba *hba, struct uic_command *uic_cmd)
+{
+ int ret;
+ unsigned long flags;
+
+ if (!ufshcd_ready_for_uic_cmd(hba)) {
+ dev_err(hba->dev,
+ "Controller not ready to accept UIC commands\n");
+ return -EIO;
+ }
+
+ init_completion(&uic_cmd->done);
+
+ spin_lock_irqsave(hba->host->host_lock, flags);
+ ufshcd_dispatch_uic_cmd(hba, uic_cmd);
+ spin_unlock_irqrestore(hba->host->host_lock, flags);
+
+ ret = ufshcd_wait_for_uic_cmd(hba, uic_cmd);
+
+ return ret;
+}
+
+/**
+ * ufshcd_send_uic_cmd - Send UIC commands and retrieve the result
+ * @hba: per adapter instance
+ * @uic_cmd: UIC command
+ *
+ * Returns 0 only if success.
+ */
+static int
+ufshcd_send_uic_cmd(struct ufs_hba *hba, struct uic_command *uic_cmd)
+{
+ int ret;
+
+ mutex_lock(&hba->uic_cmd_mutex);
+ ret = __ufshcd_send_uic_cmd(hba, uic_cmd);
+ mutex_unlock(&hba->uic_cmd_mutex);
+
+ return ret;
+}
+
+/**
* ufshcd_map_sg - Map scatter-gather list to prdt
* @lrbp - pointer to local reference block
*
@@ -725,34 +827,16 @@ static void ufshcd_host_memory_configure(struct ufs_hba *hba)
*/
static int ufshcd_dme_link_startup(struct ufs_hba *hba)
{
- struct uic_command *uic_cmd;
- unsigned long flags;
+ struct uic_command uic_cmd = {0};
+ int ret;
- /* check if controller is ready to accept UIC commands */
- if ((ufshcd_readl(hba, REG_CONTROLLER_STATUS) &
- UIC_COMMAND_READY) == 0x0) {
- dev_err(hba->dev,
- "Controller not ready"
- " to accept UIC commands\n");
- return -EIO;
- }
+ uic_cmd.command = UIC_CMD_DME_LINK_STARTUP;
- spin_lock_irqsave(hba->host->host_lock, flags);
-
- /* form UIC command */
- uic_cmd = &hba->active_uic_cmd;
- uic_cmd->command = UIC_CMD_DME_LINK_STARTUP;
- uic_cmd->argument1 = 0;
- uic_cmd->argument2 = 0;
- uic_cmd->argument3 = 0;
-
- /* enable UIC related interrupts */
- ufshcd_enable_intr(hba, UIC_COMMAND_COMPL);
-
- /* sending UIC commands to controller */
- ufshcd_send_uic_command(hba, uic_cmd);
- spin_unlock_irqrestore(hba->host->host_lock, flags);
- return 0;
+ ret = ufshcd_send_uic_cmd(hba, &uic_cmd);
+ if (ret)
+ dev_err(hba->dev,
+ "dme-link-startup: error code %d\n", ret);
+ return ret;
}
/**
@@ -761,9 +845,10 @@ static int ufshcd_dme_link_startup(struct ufs_hba *hba)
*
* To bring UFS host controller to operational state,
* 1. Check if device is present
- * 2. Configure run-stop-registers
- * 3. Enable required interrupts
- * 4. Configure interrupt aggregation
+ * 2. Enable required interrupts
+ * 3. Configure interrupt aggregation
+ * 4. Program UTRL and UTMRL base addres
+ * 5. Configure run-stop-registers
*
* Returns 0 on success, non-zero value on failure
*/
@@ -780,6 +865,22 @@ static int ufshcd_make_hba_operational(struct ufs_hba *hba)
goto out;
}
+ /* Enable required interrupts */
+ ufshcd_enable_intr(hba, UFSHCD_ENABLE_INTRS);
+
+ /* Configure interrupt aggregation */
+ ufshcd_config_int_aggr(hba, INT_AGGR_CONFIG);
+
+ /* Configure UTRL and UTMRL base address registers */
+ 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);
+
/*
* UCRDY, UTMRLDY and UTRLRDY bits must be 1
* DEI, HEI bits must be 0
@@ -793,17 +894,11 @@ static int ufshcd_make_hba_operational(struct ufs_hba *hba)
goto out;
}
- /* Enable required interrupts */
- ufshcd_enable_intr(hba, UFSHCD_ENABLE_INTRS);
-
- /* Configure interrupt aggregation */
- ufshcd_config_int_aggr(hba, INT_AGGR_CONFIG);
-
if (hba->ufshcd_state == UFSHCD_STATE_RESET)
scsi_unblock_requests(hba->host);
hba->ufshcd_state = UFSHCD_STATE_OPERATIONAL;
- scsi_scan_host(hba->host);
+
out:
return err;
}
@@ -872,34 +967,28 @@ static int ufshcd_hba_enable(struct ufs_hba *hba)
}
/**
- * ufshcd_initialize_hba - start the initialization process
+ * ufshcd_link_startup - Initialize unipro link startup
* @hba: per adapter instance
*
- * 1. Enable the controller via ufshcd_hba_enable.
- * 2. Program the Transfer Request List Address with the starting address of
- * UTRDL.
- * 3. Program the Task Management Request List Address with starting address
- * of UTMRDL.
- *
- * Returns 0 on success, non-zero value on failure.
+ * Returns 0 for success, non-zero in case of failure
*/
-static int ufshcd_initialize_hba(struct ufs_hba *hba)
+static int ufshcd_link_startup(struct ufs_hba *hba)
{
- if (ufshcd_hba_enable(hba))
- return -EIO;
+ int ret;
- /* Configure UTRL and UTMRL base address registers */
- 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);
+ /* enable UIC related interrupts */
+ ufshcd_enable_intr(hba, UIC_COMMAND_COMPL);
+
+ ret = ufshcd_dme_link_startup(hba);
+ if (ret)
+ goto out;
+
+ ret = ufshcd_make_hba_operational(hba);
- /* Initialize unipro link startup procedure */
- return ufshcd_dme_link_startup(hba);
+out:
+ if (ret)
+ dev_err(hba->dev, "link startup failed %d\n", ret);
+ return ret;
}
/**
@@ -939,12 +1028,19 @@ static int ufshcd_do_reset(struct ufs_hba *hba)
hba->outstanding_reqs = 0;
hba->outstanding_tasks = 0;
- /* start the initialization process */
- if (ufshcd_initialize_hba(hba)) {
+ /* Host controller enable */
+ if (ufshcd_hba_enable(hba)) {
dev_err(hba->dev,
"Reset: Controller initialization failed\n");
return FAILED;
}
+
+ if (ufshcd_link_startup(hba)) {
+ dev_err(hba->dev,
+ "Reset: Link start-up failed\n");
+ return FAILED;
+ }
+
return SUCCESS;
}
@@ -1176,6 +1272,19 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
}
/**
+ * ufshcd_uic_cmd_compl - handle completion of uic command
+ * @hba: per adapter instance
+ */
+static void ufshcd_uic_cmd_compl(struct ufs_hba *hba)
+{
+ if (hba->active_uic_cmd) {
+ hba->active_uic_cmd->argument2 |=
+ ufshcd_get_uic_cmd_result(hba);
+ complete(&hba->active_uic_cmd->done);
+ }
+}
+
+/**
* ufshcd_transfer_req_compl - handle SCSI and query command completion
* @hba: per adapter instance
*/
@@ -1215,28 +1324,6 @@ static void ufshcd_transfer_req_compl(struct ufs_hba *hba)
}
/**
- * ufshcd_uic_cc_handler - handle UIC command completion
- * @work: pointer to a work queue structure
- *
- * Returns 0 on success, non-zero value on failure
- */
-static void ufshcd_uic_cc_handler (struct work_struct *work)
-{
- struct ufs_hba *hba;
-
- hba = container_of(work, struct ufs_hba, uic_workq);
-
- if ((hba->active_uic_cmd.command == UIC_CMD_DME_LINK_STARTUP) &&
- !(ufshcd_get_uic_cmd_result(hba))) {
-
- if (ufshcd_make_hba_operational(hba))
- dev_err(hba->dev,
- "cc: hba not operational state\n");
- return;
- }
-}
-
-/**
* ufshcd_fatal_err_handler - handle fatal errors
* @hba: per adapter instance
*/
@@ -1297,7 +1384,7 @@ static void ufshcd_sl_intr(struct ufs_hba *hba, u32 intr_status)
ufshcd_err_handler(hba);
if (intr_status & UIC_COMMAND_COMPL)
- schedule_work(&hba->uic_workq);
+ ufshcd_uic_cmd_compl(hba);
if (intr_status & UTP_TASK_REQ_COMPL)
ufshcd_tmc_handler(hba);
@@ -1520,6 +1607,21 @@ out:
return err;
}
+/**
+ * ufshcd_async_scan - asynchronous execution for link startup
+ * @data: data pointer to pass to this function
+ * @cookie: cookie data
+ */
+static void ufshcd_async_scan(void *data, async_cookie_t cookie)
+{
+ struct ufs_hba *hba = (struct ufs_hba *)data;
+ int ret;
+
+ ret = ufshcd_link_startup(hba);
+ if (!ret)
+ scsi_scan_host(hba->host);
+}
+
static struct scsi_host_template ufshcd_driver_template = {
.module = THIS_MODULE,
.name = UFSHCD,
@@ -1681,9 +1783,11 @@ int ufshcd_init(struct device *dev, struct ufs_hba **hba_handle,
init_waitqueue_head(&hba->ufshcd_tm_wait_queue);
/* Initialize work queues */
- INIT_WORK(&hba->uic_workq, ufshcd_uic_cc_handler);
INIT_WORK(&hba->feh_workq, ufshcd_fatal_err_handler);
+ /* Initialize UIC command mutex */
+ mutex_init(&hba->uic_cmd_mutex);
+
/* IRQ registration */
err = request_irq(irq, ufshcd_intr, IRQF_SHARED, UFSHCD, hba);
if (err) {
@@ -1704,14 +1808,17 @@ int ufshcd_init(struct device *dev, struct ufs_hba **hba_handle,
goto out_free_irq;
}
- /* Initialization routine */
- err = ufshcd_initialize_hba(hba);
+ /* Host controller enable */
+ err = ufshcd_hba_enable(hba);
if (err) {
- dev_err(hba->dev, "Initialization failed\n");
+ dev_err(hba->dev, "Host controller enable failed\n");
goto out_remove_scsi_host;
}
+
*hba_handle = hba;
+ async_schedule(ufshcd_async_scan, hba);
+
return 0;
out_remove_scsi_host:
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 4213600..49590ee 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -51,6 +51,7 @@
#include <linux/bitops.h>
#include <linux/pm_runtime.h>
#include <linux/clk.h>
+#include <linux/completion.h>
#include <asm/irq.h>
#include <asm/byteorder.h>
@@ -75,6 +76,7 @@
* @argument3: UIC command argument 3
* @cmd_active: Indicate if UIC command is outstanding
* @result: UIC command result
+ * @done: UIC command completion
*/
struct uic_command {
u32 command;
@@ -83,6 +85,7 @@ struct uic_command {
u32 argument3;
int cmd_active;
int result;
+ struct completion done;
};
/**
@@ -136,11 +139,11 @@ struct ufshcd_lrb {
* @ufs_version: UFS Version to which controller complies
* @irq: Irq number of the controller
* @active_uic_cmd: handle of active UIC command
+ * @uic_cmd_mutex: mutex for uic command
* @ufshcd_tm_wait_queue: wait queue for task management
* @tm_condition: condition variable for task management
* @ufshcd_state: UFSHCD states
* @intr_mask: Interrupt Mask Bits
- * @uic_workq: Work queue for UIC completion handling
* @feh_workq: Work queue for fatal controller error handling
* @errors: HBA errors
*/
@@ -171,7 +174,9 @@ struct ufs_hba {
u32 ufs_version;
unsigned int irq;
- struct uic_command active_uic_cmd;
+ struct uic_command *active_uic_cmd;
+ struct mutex uic_cmd_mutex;
+
wait_queue_head_t ufshcd_tm_wait_queue;
unsigned long tm_condition;
@@ -179,7 +184,6 @@ struct ufs_hba {
u32 intr_mask;
/* Work Queues */
- struct work_struct uic_workq;
struct work_struct feh_workq;
/* HBA Errors */
--
1.8.3.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 05/10] scsi: ufs: Fix the response UPIU length setting
2013-06-26 17:09 [PATCH RESEND 00/10] scsi: ufs: link start-up rework and other fixes Santosh Y
` (3 preceding siblings ...)
2013-06-26 17:09 ` [PATCH 04/10] scsi: ufs: rework link start-up process Santosh Y
@ 2013-06-26 17:09 ` Santosh Y
2013-06-26 17:09 ` [PATCH 06/10] scsi: ufs: use devres functions for ufshcd Santosh Y
` (4 subsequent siblings)
9 siblings, 0 replies; 18+ messages in thread
From: Santosh Y @ 2013-06-26 17:09 UTC (permalink / raw)
To: james.bottomley
Cc: linux-scsi, vinholikatti, Sujit Reddy Thumma, Maya Erez, Santosh Y
From: Sujit Reddy Thumma <sthumma@codeaurora.org>
The response UPIU length should be in DWORD and not in bytes.
Signed-off-by: Maya Erez <merez@codeaurora.org>
Signed-off-by: Sujit Reddy Thumma <sthumma@codeaurora.org>
Signed-off-by: Santosh Y <santoshsy@gmail.com>
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 48a7645..2230f14 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -802,7 +802,7 @@ static void ufshcd_host_memory_configure(struct ufs_hba *hba)
utrdlp[i].prd_table_offset =
cpu_to_le16((prdt_offset >> 2));
utrdlp[i].response_upiu_length =
- cpu_to_le16(ALIGNED_UPIU_SIZE);
+ cpu_to_le16(ALIGNED_UPIU_SIZE >> 2);
hba->lrb[i].utr_descriptor_ptr = (utrdlp + i);
hba->lrb[i].ucd_cmd_ptr =
--
1.8.3.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 06/10] scsi: ufs: use devres functions for ufshcd
2013-06-26 17:09 [PATCH RESEND 00/10] scsi: ufs: link start-up rework and other fixes Santosh Y
` (4 preceding siblings ...)
2013-06-26 17:09 ` [PATCH 05/10] scsi: ufs: Fix the response UPIU length setting Santosh Y
@ 2013-06-26 17:09 ` Santosh Y
2013-06-27 4:31 ` [PATCH v2 " Seungwon Jeon
2013-06-26 17:09 ` [PATCH 07/10] ufshcd-pltfrm: add missing empty slot in ufs_of_match[] Santosh Y
` (3 subsequent siblings)
9 siblings, 1 reply; 18+ messages in thread
From: Santosh Y @ 2013-06-26 17:09 UTC (permalink / raw)
To: james.bottomley; +Cc: linux-scsi, vinholikatti, Seungwon Jeon, Santosh Y
From: Seungwon Jeon <tgih.jun@samsung.com>
This patch replaces normal calls for resource allocation with devm_*()
derivative functions. It makes resource freeing simpler.
Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
Signed-off-by: Santosh Y <santoshsy@gmail.com>
diff --git a/drivers/scsi/ufs/ufshcd-pci.c b/drivers/scsi/ufs/ufshcd-pci.c
index 5cb1d75..48be39a 100644
--- a/drivers/scsi/ufs/ufshcd-pci.c
+++ b/drivers/scsi/ufs/ufshcd-pci.c
@@ -92,7 +92,6 @@ static void ufshcd_pci_remove(struct pci_dev *pdev)
struct ufs_hba *hba = pci_get_drvdata(pdev);
disable_irq(pdev->irq);
- free_irq(pdev->irq, hba);
ufshcd_remove(hba);
pci_release_regions(pdev);
pci_set_drvdata(pdev, NULL);
diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c
index 3db2ee1..0e48827 100644
--- a/drivers/scsi/ufs/ufshcd-pltfrm.c
+++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
@@ -33,9 +33,10 @@
* this program.
*/
-#include "ufshcd.h"
#include <linux/platform_device.h>
+#include "ufshcd.h"
+
#ifdef CONFIG_PM
/**
* ufshcd_pltfrm_suspend - suspend power management function
@@ -97,62 +98,45 @@ static int ufshcd_pltfrm_probe(struct platform_device *pdev)
struct ufs_hba *hba;
void __iomem *mmio_base;
struct resource *mem_res;
- struct resource *irq_res;
- resource_size_t mem_size;
- int err;
+ int irq, err;
struct device *dev = &pdev->dev;
mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!mem_res) {
- dev_err(&pdev->dev,
- "Memory resource not available\n");
+ dev_err(dev, "Memory resource not available\n");
err = -ENODEV;
- goto out_error;
+ goto out;
}
- mem_size = resource_size(mem_res);
- if (!request_mem_region(mem_res->start, mem_size, "ufshcd")) {
- dev_err(&pdev->dev,
- "Cannot reserve the memory resource\n");
- err = -EBUSY;
- goto out_error;
+ mmio_base = devm_ioremap_resource(dev, mem_res);
+ if (IS_ERR(mmio_base)) {
+ dev_err(dev, "memory map failed\n");
+ err = PTR_ERR(mmio_base);
+ goto out;
}
- mmio_base = ioremap_nocache(mem_res->start, mem_size);
- if (!mmio_base) {
- dev_err(&pdev->dev, "memory map failed\n");
- err = -ENOMEM;
- goto out_release_regions;
- }
-
- irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
- if (!irq_res) {
- dev_err(&pdev->dev, "IRQ resource not available\n");
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0) {
+ dev_err(dev, "IRQ resource not available\n");
err = -ENODEV;
- goto out_iounmap;
+ goto out;
}
err = dma_set_coherent_mask(dev, dev->coherent_dma_mask);
if (err) {
- dev_err(&pdev->dev, "set dma mask failed\n");
- goto out_iounmap;
+ dev_err(dev, "set dma mask failed\n");
+ goto out;
}
- err = ufshcd_init(&pdev->dev, &hba, mmio_base, irq_res->start);
+ err = ufshcd_init(dev, &hba, mmio_base, irq);
if (err) {
- dev_err(&pdev->dev, "Intialization failed\n");
- goto out_iounmap;
+ dev_err(dev, "Intialization failed\n");
+ goto out;
}
platform_set_drvdata(pdev, hba);
- return 0;
-
-out_iounmap:
- iounmap(mmio_base);
-out_release_regions:
- release_mem_region(mem_res->start, mem_size);
-out_error:
+out:
return err;
}
@@ -164,26 +148,10 @@ out_error:
*/
static int ufshcd_pltfrm_remove(struct platform_device *pdev)
{
- struct resource *mem_res;
- resource_size_t mem_size;
struct ufs_hba *hba = platform_get_drvdata(pdev);
disable_irq(hba->irq);
-
- /* Some buggy controllers raise interrupt after
- * the resources are removed. So first we unregister the
- * irq handler and then the resources used by driver
- */
-
- free_irq(hba->irq, hba);
ufshcd_remove(hba);
- mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!mem_res)
- dev_err(&pdev->dev, "ufshcd: Memory resource not available\n");
- else {
- mem_size = resource_size(mem_res);
- release_mem_region(mem_res->start, mem_size);
- }
return 0;
}
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 2230f14..255f5be 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -1789,7 +1789,7 @@ int ufshcd_init(struct device *dev, struct ufs_hba **hba_handle,
mutex_init(&hba->uic_cmd_mutex);
/* IRQ registration */
- err = request_irq(irq, ufshcd_intr, IRQF_SHARED, UFSHCD, hba);
+ err = devm_request_irq(dev, irq, ufshcd_intr, IRQF_SHARED, UFSHCD, hba);
if (err) {
dev_err(hba->dev, "request irq failed\n");
goto out_lrb_free;
@@ -1799,13 +1799,13 @@ int ufshcd_init(struct device *dev, struct ufs_hba **hba_handle,
err = scsi_init_shared_tag_map(host, host->can_queue);
if (err) {
dev_err(hba->dev, "init shared queue failed\n");
- goto out_free_irq;
+ goto out_lrb_free;
}
err = scsi_add_host(host, hba->dev);
if (err) {
dev_err(hba->dev, "scsi_add_host failed\n");
- goto out_free_irq;
+ goto out_lrb_free;
}
/* Host controller enable */
@@ -1823,8 +1823,6 @@ int ufshcd_init(struct device *dev, struct ufs_hba **hba_handle,
out_remove_scsi_host:
scsi_remove_host(hba->host);
-out_free_irq:
- free_irq(irq, hba);
out_lrb_free:
ufshcd_free_hba_memory(hba);
out_disable:
--
1.8.3.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v2 06/10] scsi: ufs: use devres functions for ufshcd
2013-06-26 17:09 ` [PATCH 06/10] scsi: ufs: use devres functions for ufshcd Santosh Y
@ 2013-06-27 4:31 ` Seungwon Jeon
2013-07-29 19:17 ` Santosh Y
0 siblings, 1 reply; 18+ messages in thread
From: Seungwon Jeon @ 2013-06-27 4:31 UTC (permalink / raw)
To: 'James Bottomley'; +Cc: linux-scsi, 'Santosh Y', vinholikatti
This patch replaces normal calls for resource allocation with devm_*()
derivative functions. It makes resource freeing simpler.
Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
Signed-off-by: Santosh Y <santoshsy@gmail.com>
---
Change in v2:
[NOTE: There are no conflicts with the following series(07~10)]
- Remove iounmap which is remained.
- Apply devres to ufshcd_memory_alloc[dmam_alloc_coherent, devm_kzalloc]
Accordingly, 'free' related functions are removed.
drivers/scsi/ufs/ufshcd-pci.c | 1 -
drivers/scsi/ufs/ufshcd-pltfrm.c | 72 +++++++++-----------------------
drivers/scsi/ufs/ufshcd.c | 86 ++++++++-----------------------------
3 files changed, 39 insertions(+), 120 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd-pci.c b/drivers/scsi/ufs/ufshcd-pci.c
index 5cb1d75..48be39a 100644
--- a/drivers/scsi/ufs/ufshcd-pci.c
+++ b/drivers/scsi/ufs/ufshcd-pci.c
@@ -92,7 +92,6 @@ static void ufshcd_pci_remove(struct pci_dev *pdev)
struct ufs_hba *hba = pci_get_drvdata(pdev);
disable_irq(pdev->irq);
- free_irq(pdev->irq, hba);
ufshcd_remove(hba);
pci_release_regions(pdev);
pci_set_drvdata(pdev, NULL);
diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c
index 3db2ee1..0e48827 100644
--- a/drivers/scsi/ufs/ufshcd-pltfrm.c
+++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
@@ -33,9 +33,10 @@
* this program.
*/
-#include "ufshcd.h"
#include <linux/platform_device.h>
+#include "ufshcd.h"
+
#ifdef CONFIG_PM
/**
* ufshcd_pltfrm_suspend - suspend power management function
@@ -97,62 +98,45 @@ static int ufshcd_pltfrm_probe(struct platform_device *pdev)
struct ufs_hba *hba;
void __iomem *mmio_base;
struct resource *mem_res;
- struct resource *irq_res;
- resource_size_t mem_size;
- int err;
+ int irq, err;
struct device *dev = &pdev->dev;
mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!mem_res) {
- dev_err(&pdev->dev,
- "Memory resource not available\n");
+ dev_err(dev, "Memory resource not available\n");
err = -ENODEV;
- goto out_error;
+ goto out;
}
- mem_size = resource_size(mem_res);
- if (!request_mem_region(mem_res->start, mem_size, "ufshcd")) {
- dev_err(&pdev->dev,
- "Cannot reserve the memory resource\n");
- err = -EBUSY;
- goto out_error;
+ mmio_base = devm_ioremap_resource(dev, mem_res);
+ if (IS_ERR(mmio_base)) {
+ dev_err(dev, "memory map failed\n");
+ err = PTR_ERR(mmio_base);
+ goto out;
}
- mmio_base = ioremap_nocache(mem_res->start, mem_size);
- if (!mmio_base) {
- dev_err(&pdev->dev, "memory map failed\n");
- err = -ENOMEM;
- goto out_release_regions;
- }
-
- irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
- if (!irq_res) {
- dev_err(&pdev->dev, "IRQ resource not available\n");
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0) {
+ dev_err(dev, "IRQ resource not available\n");
err = -ENODEV;
- goto out_iounmap;
+ goto out;
}
err = dma_set_coherent_mask(dev, dev->coherent_dma_mask);
if (err) {
- dev_err(&pdev->dev, "set dma mask failed\n");
- goto out_iounmap;
+ dev_err(dev, "set dma mask failed\n");
+ goto out;
}
- err = ufshcd_init(&pdev->dev, &hba, mmio_base, irq_res->start);
+ err = ufshcd_init(dev, &hba, mmio_base, irq);
if (err) {
- dev_err(&pdev->dev, "Intialization failed\n");
- goto out_iounmap;
+ dev_err(dev, "Intialization failed\n");
+ goto out;
}
platform_set_drvdata(pdev, hba);
- return 0;
-
-out_iounmap:
- iounmap(mmio_base);
-out_release_regions:
- release_mem_region(mem_res->start, mem_size);
-out_error:
+out:
return err;
}
@@ -164,26 +148,10 @@ out_error:
*/
static int ufshcd_pltfrm_remove(struct platform_device *pdev)
{
- struct resource *mem_res;
- resource_size_t mem_size;
struct ufs_hba *hba = platform_get_drvdata(pdev);
disable_irq(hba->irq);
-
- /* Some buggy controllers raise interrupt after
- * the resources are removed. So first we unregister the
- * irq handler and then the resources used by driver
- */
-
- free_irq(hba->irq, hba);
ufshcd_remove(hba);
- mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!mem_res)
- dev_err(&pdev->dev, "ufshcd: Memory resource not available\n");
- else {
- mem_size = resource_size(mem_res);
- release_mem_region(mem_res->start, mem_size);
- }
return 0;
}
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 2230f14..aba461f 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -191,38 +191,6 @@ static inline int ufshcd_get_uic_cmd_result(struct ufs_hba *hba)
}
/**
- * ufshcd_free_hba_memory - Free allocated memory for LRB, request
- * and task lists
- * @hba: Pointer to adapter instance
- */
-static inline void ufshcd_free_hba_memory(struct ufs_hba *hba)
-{
- size_t utmrdl_size, utrdl_size, ucdl_size;
-
- kfree(hba->lrb);
-
- if (hba->utmrdl_base_addr) {
- utmrdl_size = sizeof(struct utp_task_req_desc) * hba->nutmrs;
- dma_free_coherent(hba->dev, utmrdl_size,
- hba->utmrdl_base_addr, hba->utmrdl_dma_addr);
- }
-
- if (hba->utrdl_base_addr) {
- utrdl_size =
- (sizeof(struct utp_transfer_req_desc) * hba->nutrs);
- dma_free_coherent(hba->dev, utrdl_size,
- hba->utrdl_base_addr, hba->utrdl_dma_addr);
- }
-
- if (hba->ucdl_base_addr) {
- ucdl_size =
- (sizeof(struct utp_transfer_cmd_desc) * hba->nutrs);
- dma_free_coherent(hba->dev, ucdl_size,
- hba->ucdl_base_addr, hba->ucdl_dma_addr);
- }
-}
-
-/**
* ufshcd_is_valid_req_rsp - checks if controller TR response is valid
* @ucd_rsp_ptr: pointer to response UPIU
*
@@ -690,10 +658,10 @@ static int ufshcd_memory_alloc(struct ufs_hba *hba)
/* Allocate memory for UTP command descriptors */
ucdl_size = (sizeof(struct utp_transfer_cmd_desc) * hba->nutrs);
- hba->ucdl_base_addr = dma_alloc_coherent(hba->dev,
- ucdl_size,
- &hba->ucdl_dma_addr,
- GFP_KERNEL);
+ hba->ucdl_base_addr = dmam_alloc_coherent(hba->dev,
+ ucdl_size,
+ &hba->ucdl_dma_addr,
+ GFP_KERNEL);
/*
* UFSHCI requires UTP command descriptor to be 128 byte aligned.
@@ -713,10 +681,10 @@ static int ufshcd_memory_alloc(struct ufs_hba *hba)
* UFSHCI requires 1024 byte alignment of UTRD
*/
utrdl_size = (sizeof(struct utp_transfer_req_desc) * hba->nutrs);
- hba->utrdl_base_addr = dma_alloc_coherent(hba->dev,
- utrdl_size,
- &hba->utrdl_dma_addr,
- GFP_KERNEL);
+ hba->utrdl_base_addr = dmam_alloc_coherent(hba->dev,
+ utrdl_size,
+ &hba->utrdl_dma_addr,
+ GFP_KERNEL);
if (!hba->utrdl_base_addr ||
WARN_ON(hba->utrdl_dma_addr & (PAGE_SIZE - 1))) {
dev_err(hba->dev,
@@ -729,10 +697,10 @@ static int ufshcd_memory_alloc(struct ufs_hba *hba)
* UFSHCI requires 1024 byte alignment of UTMRD
*/
utmrdl_size = sizeof(struct utp_task_req_desc) * hba->nutmrs;
- hba->utmrdl_base_addr = dma_alloc_coherent(hba->dev,
- utmrdl_size,
- &hba->utmrdl_dma_addr,
- GFP_KERNEL);
+ hba->utmrdl_base_addr = dmam_alloc_coherent(hba->dev,
+ utmrdl_size,
+ &hba->utmrdl_dma_addr,
+ GFP_KERNEL);
if (!hba->utmrdl_base_addr ||
WARN_ON(hba->utmrdl_dma_addr & (PAGE_SIZE - 1))) {
dev_err(hba->dev,
@@ -741,14 +709,15 @@ static int ufshcd_memory_alloc(struct ufs_hba *hba)
}
/* Allocate memory for local reference block */
- hba->lrb = kcalloc(hba->nutrs, sizeof(struct ufshcd_lrb), GFP_KERNEL);
+ hba->lrb = devm_kzalloc(hba->dev,
+ hba->nutrs * sizeof(struct ufshcd_lrb),
+ GFP_KERNEL);
if (!hba->lrb) {
dev_err(hba->dev, "LRB Memory allocation failed\n");
goto out;
}
return 0;
out:
- ufshcd_free_hba_memory(hba);
return -ENOMEM;
}
@@ -1682,17 +1651,6 @@ int ufshcd_resume(struct ufs_hba *hba)
EXPORT_SYMBOL_GPL(ufshcd_resume);
/**
- * ufshcd_hba_free - free allocated memory for
- * host memory space data structures
- * @hba: per adapter instance
- */
-static void ufshcd_hba_free(struct ufs_hba *hba)
-{
- iounmap(hba->mmio_base);
- ufshcd_free_hba_memory(hba);
-}
-
-/**
* ufshcd_remove - de-allocate SCSI host and host memory space
* data structure memory
* @hba - per adapter instance
@@ -1701,9 +1659,7 @@ void ufshcd_remove(struct ufs_hba *hba)
{
/* disable interrupts */
ufshcd_disable_intr(hba, hba->intr_mask);
-
ufshcd_hba_stop(hba);
- ufshcd_hba_free(hba);
scsi_remove_host(hba->host);
scsi_host_put(hba->host);
@@ -1789,23 +1745,23 @@ int ufshcd_init(struct device *dev, struct ufs_hba **hba_handle,
mutex_init(&hba->uic_cmd_mutex);
/* IRQ registration */
- err = request_irq(irq, ufshcd_intr, IRQF_SHARED, UFSHCD, hba);
+ err = devm_request_irq(dev, irq, ufshcd_intr, IRQF_SHARED, UFSHCD, hba);
if (err) {
dev_err(hba->dev, "request irq failed\n");
- goto out_lrb_free;
+ goto out_disable;
}
/* Enable SCSI tag mapping */
err = scsi_init_shared_tag_map(host, host->can_queue);
if (err) {
dev_err(hba->dev, "init shared queue failed\n");
- goto out_free_irq;
+ goto out_disable;
}
err = scsi_add_host(host, hba->dev);
if (err) {
dev_err(hba->dev, "scsi_add_host failed\n");
- goto out_free_irq;
+ goto out_disable;
}
/* Host controller enable */
@@ -1823,10 +1779,6 @@ int ufshcd_init(struct device *dev, struct ufs_hba **hba_handle,
out_remove_scsi_host:
scsi_remove_host(hba->host);
-out_free_irq:
- free_irq(irq, hba);
-out_lrb_free:
- ufshcd_free_hba_memory(hba);
out_disable:
scsi_host_put(host);
out_error:
--
1.7.0.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v2 06/10] scsi: ufs: use devres functions for ufshcd
2013-06-27 4:31 ` [PATCH v2 " Seungwon Jeon
@ 2013-07-29 19:17 ` Santosh Y
0 siblings, 0 replies; 18+ messages in thread
From: Santosh Y @ 2013-07-29 19:17 UTC (permalink / raw)
To: Seungwon Jeon; +Cc: James Bottomley, linux-scsi, vinholikatti
On Thu, Jun 27, 2013 at 10:01 AM, Seungwon Jeon <tgih.jun@samsung.com> wrote:
> This patch replaces normal calls for resource allocation with devm_*()
> derivative functions. It makes resource freeing simpler.
>
> Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
> Signed-off-by: Santosh Y <santoshsy@gmail.com>
> ---
> Change in v2:
> [NOTE: There are no conflicts with the following series(07~10)]
> - Remove iounmap which is remained.
> - Apply devres to ufshcd_memory_alloc[dmam_alloc_coherent, devm_kzalloc]
> Accordingly, 'free' related functions are removed.
>
> drivers/scsi/ufs/ufshcd-pci.c | 1 -
> drivers/scsi/ufs/ufshcd-pltfrm.c | 72 +++++++++-----------------------
> drivers/scsi/ufs/ufshcd.c | 86 ++++++++-----------------------------
> 3 files changed, 39 insertions(+), 120 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd-pci.c b/drivers/scsi/ufs/ufshcd-pci.c
> index 5cb1d75..48be39a 100644
> --- a/drivers/scsi/ufs/ufshcd-pci.c
> +++ b/drivers/scsi/ufs/ufshcd-pci.c
> @@ -92,7 +92,6 @@ static void ufshcd_pci_remove(struct pci_dev *pdev)
> struct ufs_hba *hba = pci_get_drvdata(pdev);
>
> disable_irq(pdev->irq);
> - free_irq(pdev->irq, hba);
> ufshcd_remove(hba);
> pci_release_regions(pdev);
> pci_set_drvdata(pdev, NULL);
> diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c
> index 3db2ee1..0e48827 100644
> --- a/drivers/scsi/ufs/ufshcd-pltfrm.c
> +++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
> @@ -33,9 +33,10 @@
> * this program.
> */
>
> -#include "ufshcd.h"
> #include <linux/platform_device.h>
>
> +#include "ufshcd.h"
> +
> #ifdef CONFIG_PM
> /**
> * ufshcd_pltfrm_suspend - suspend power management function
> @@ -97,62 +98,45 @@ static int ufshcd_pltfrm_probe(struct platform_device *pdev)
> struct ufs_hba *hba;
> void __iomem *mmio_base;
> struct resource *mem_res;
> - struct resource *irq_res;
> - resource_size_t mem_size;
> - int err;
> + int irq, err;
> struct device *dev = &pdev->dev;
>
> mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> if (!mem_res) {
> - dev_err(&pdev->dev,
> - "Memory resource not available\n");
> + dev_err(dev, "Memory resource not available\n");
> err = -ENODEV;
> - goto out_error;
> + goto out;
> }
>
> - mem_size = resource_size(mem_res);
> - if (!request_mem_region(mem_res->start, mem_size, "ufshcd")) {
> - dev_err(&pdev->dev,
> - "Cannot reserve the memory resource\n");
> - err = -EBUSY;
> - goto out_error;
> + mmio_base = devm_ioremap_resource(dev, mem_res);
> + if (IS_ERR(mmio_base)) {
> + dev_err(dev, "memory map failed\n");
> + err = PTR_ERR(mmio_base);
> + goto out;
> }
>
> - mmio_base = ioremap_nocache(mem_res->start, mem_size);
> - if (!mmio_base) {
> - dev_err(&pdev->dev, "memory map failed\n");
> - err = -ENOMEM;
> - goto out_release_regions;
> - }
> -
> - irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> - if (!irq_res) {
> - dev_err(&pdev->dev, "IRQ resource not available\n");
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0) {
> + dev_err(dev, "IRQ resource not available\n");
> err = -ENODEV;
> - goto out_iounmap;
> + goto out;
> }
>
> err = dma_set_coherent_mask(dev, dev->coherent_dma_mask);
> if (err) {
> - dev_err(&pdev->dev, "set dma mask failed\n");
> - goto out_iounmap;
> + dev_err(dev, "set dma mask failed\n");
> + goto out;
> }
>
> - err = ufshcd_init(&pdev->dev, &hba, mmio_base, irq_res->start);
> + err = ufshcd_init(dev, &hba, mmio_base, irq);
> if (err) {
> - dev_err(&pdev->dev, "Intialization failed\n");
> - goto out_iounmap;
> + dev_err(dev, "Intialization failed\n");
> + goto out;
> }
>
> platform_set_drvdata(pdev, hba);
>
> - return 0;
> -
> -out_iounmap:
> - iounmap(mmio_base);
> -out_release_regions:
> - release_mem_region(mem_res->start, mem_size);
> -out_error:
> +out:
> return err;
> }
>
> @@ -164,26 +148,10 @@ out_error:
> */
> static int ufshcd_pltfrm_remove(struct platform_device *pdev)
> {
> - struct resource *mem_res;
> - resource_size_t mem_size;
> struct ufs_hba *hba = platform_get_drvdata(pdev);
>
> disable_irq(hba->irq);
> -
> - /* Some buggy controllers raise interrupt after
> - * the resources are removed. So first we unregister the
> - * irq handler and then the resources used by driver
> - */
> -
> - free_irq(hba->irq, hba);
> ufshcd_remove(hba);
> - mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - if (!mem_res)
> - dev_err(&pdev->dev, "ufshcd: Memory resource not available\n");
> - else {
> - mem_size = resource_size(mem_res);
> - release_mem_region(mem_res->start, mem_size);
> - }
> return 0;
> }
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 2230f14..aba461f 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -191,38 +191,6 @@ static inline int ufshcd_get_uic_cmd_result(struct ufs_hba *hba)
> }
>
> /**
> - * ufshcd_free_hba_memory - Free allocated memory for LRB, request
> - * and task lists
> - * @hba: Pointer to adapter instance
> - */
> -static inline void ufshcd_free_hba_memory(struct ufs_hba *hba)
> -{
> - size_t utmrdl_size, utrdl_size, ucdl_size;
> -
> - kfree(hba->lrb);
> -
> - if (hba->utmrdl_base_addr) {
> - utmrdl_size = sizeof(struct utp_task_req_desc) * hba->nutmrs;
> - dma_free_coherent(hba->dev, utmrdl_size,
> - hba->utmrdl_base_addr, hba->utmrdl_dma_addr);
> - }
> -
> - if (hba->utrdl_base_addr) {
> - utrdl_size =
> - (sizeof(struct utp_transfer_req_desc) * hba->nutrs);
> - dma_free_coherent(hba->dev, utrdl_size,
> - hba->utrdl_base_addr, hba->utrdl_dma_addr);
> - }
> -
> - if (hba->ucdl_base_addr) {
> - ucdl_size =
> - (sizeof(struct utp_transfer_cmd_desc) * hba->nutrs);
> - dma_free_coherent(hba->dev, ucdl_size,
> - hba->ucdl_base_addr, hba->ucdl_dma_addr);
> - }
> -}
> -
> -/**
> * ufshcd_is_valid_req_rsp - checks if controller TR response is valid
> * @ucd_rsp_ptr: pointer to response UPIU
> *
> @@ -690,10 +658,10 @@ static int ufshcd_memory_alloc(struct ufs_hba *hba)
>
> /* Allocate memory for UTP command descriptors */
> ucdl_size = (sizeof(struct utp_transfer_cmd_desc) * hba->nutrs);
> - hba->ucdl_base_addr = dma_alloc_coherent(hba->dev,
> - ucdl_size,
> - &hba->ucdl_dma_addr,
> - GFP_KERNEL);
> + hba->ucdl_base_addr = dmam_alloc_coherent(hba->dev,
> + ucdl_size,
> + &hba->ucdl_dma_addr,
> + GFP_KERNEL);
>
> /*
> * UFSHCI requires UTP command descriptor to be 128 byte aligned.
> @@ -713,10 +681,10 @@ static int ufshcd_memory_alloc(struct ufs_hba *hba)
> * UFSHCI requires 1024 byte alignment of UTRD
> */
> utrdl_size = (sizeof(struct utp_transfer_req_desc) * hba->nutrs);
> - hba->utrdl_base_addr = dma_alloc_coherent(hba->dev,
> - utrdl_size,
> - &hba->utrdl_dma_addr,
> - GFP_KERNEL);
> + hba->utrdl_base_addr = dmam_alloc_coherent(hba->dev,
> + utrdl_size,
> + &hba->utrdl_dma_addr,
> + GFP_KERNEL);
> if (!hba->utrdl_base_addr ||
> WARN_ON(hba->utrdl_dma_addr & (PAGE_SIZE - 1))) {
> dev_err(hba->dev,
> @@ -729,10 +697,10 @@ static int ufshcd_memory_alloc(struct ufs_hba *hba)
> * UFSHCI requires 1024 byte alignment of UTMRD
> */
> utmrdl_size = sizeof(struct utp_task_req_desc) * hba->nutmrs;
> - hba->utmrdl_base_addr = dma_alloc_coherent(hba->dev,
> - utmrdl_size,
> - &hba->utmrdl_dma_addr,
> - GFP_KERNEL);
> + hba->utmrdl_base_addr = dmam_alloc_coherent(hba->dev,
> + utmrdl_size,
> + &hba->utmrdl_dma_addr,
> + GFP_KERNEL);
> if (!hba->utmrdl_base_addr ||
> WARN_ON(hba->utmrdl_dma_addr & (PAGE_SIZE - 1))) {
> dev_err(hba->dev,
> @@ -741,14 +709,15 @@ static int ufshcd_memory_alloc(struct ufs_hba *hba)
> }
>
> /* Allocate memory for local reference block */
> - hba->lrb = kcalloc(hba->nutrs, sizeof(struct ufshcd_lrb), GFP_KERNEL);
> + hba->lrb = devm_kzalloc(hba->dev,
> + hba->nutrs * sizeof(struct ufshcd_lrb),
> + GFP_KERNEL);
> if (!hba->lrb) {
> dev_err(hba->dev, "LRB Memory allocation failed\n");
> goto out;
> }
> return 0;
> out:
> - ufshcd_free_hba_memory(hba);
> return -ENOMEM;
> }
>
> @@ -1682,17 +1651,6 @@ int ufshcd_resume(struct ufs_hba *hba)
> EXPORT_SYMBOL_GPL(ufshcd_resume);
>
> /**
> - * ufshcd_hba_free - free allocated memory for
> - * host memory space data structures
> - * @hba: per adapter instance
> - */
> -static void ufshcd_hba_free(struct ufs_hba *hba)
> -{
> - iounmap(hba->mmio_base);
> - ufshcd_free_hba_memory(hba);
> -}
> -
> -/**
> * ufshcd_remove - de-allocate SCSI host and host memory space
> * data structure memory
> * @hba - per adapter instance
> @@ -1701,9 +1659,7 @@ void ufshcd_remove(struct ufs_hba *hba)
> {
> /* disable interrupts */
> ufshcd_disable_intr(hba, hba->intr_mask);
> -
> ufshcd_hba_stop(hba);
> - ufshcd_hba_free(hba);
>
> scsi_remove_host(hba->host);
> scsi_host_put(hba->host);
> @@ -1789,23 +1745,23 @@ int ufshcd_init(struct device *dev, struct ufs_hba **hba_handle,
> mutex_init(&hba->uic_cmd_mutex);
>
> /* IRQ registration */
> - err = request_irq(irq, ufshcd_intr, IRQF_SHARED, UFSHCD, hba);
> + err = devm_request_irq(dev, irq, ufshcd_intr, IRQF_SHARED, UFSHCD, hba);
> if (err) {
> dev_err(hba->dev, "request irq failed\n");
> - goto out_lrb_free;
> + goto out_disable;
> }
>
> /* Enable SCSI tag mapping */
> err = scsi_init_shared_tag_map(host, host->can_queue);
> if (err) {
> dev_err(hba->dev, "init shared queue failed\n");
> - goto out_free_irq;
> + goto out_disable;
> }
>
> err = scsi_add_host(host, hba->dev);
> if (err) {
> dev_err(hba->dev, "scsi_add_host failed\n");
> - goto out_free_irq;
> + goto out_disable;
> }
>
> /* Host controller enable */
> @@ -1823,10 +1779,6 @@ int ufshcd_init(struct device *dev, struct ufs_hba **hba_handle,
>
> out_remove_scsi_host:
> scsi_remove_host(hba->host);
> -out_free_irq:
> - free_irq(irq, hba);
> -out_lrb_free:
> - ufshcd_free_hba_memory(hba);
> out_disable:
> scsi_host_put(host);
> out_error:
> --
> 1.7.0.4
>
>
Please resend the patch and re-base it on scsi 'misc' branch.
--
~Santosh
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 07/10] ufshcd-pltfrm: add missing empty slot in ufs_of_match[]
2013-06-26 17:09 [PATCH RESEND 00/10] scsi: ufs: link start-up rework and other fixes Santosh Y
` (5 preceding siblings ...)
2013-06-26 17:09 ` [PATCH 06/10] scsi: ufs: use devres functions for ufshcd Santosh Y
@ 2013-06-26 17:09 ` Santosh Y
2013-06-26 17:09 ` [PATCH 08/10] ufs: fix register address in UIC error interrupt handling Santosh Y
` (2 subsequent siblings)
9 siblings, 0 replies; 18+ messages in thread
From: Santosh Y @ 2013-06-26 17:09 UTC (permalink / raw)
To: james.bottomley
Cc: linux-scsi, vinholikatti, Akinobu Mita, James E.J. Bottomley, Santosh Y
From: Akinobu Mita <mita@fixstars.com>
of_match_table member in struct device_driver must be terminated by
empty slot as a sentinel.
Signed-off-by: Akinobu Mita <mita@fixstars.com>
Cc: Vinayak Holikatti <vinholikatti@gmail.com>
Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
Cc: linux-scsi@vger.kernel.org
Signed-off-by: Santosh Y <santoshsy@gmail.com>
diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c
index 0e48827..6829a16 100644
--- a/drivers/scsi/ufs/ufshcd-pltfrm.c
+++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
@@ -157,6 +157,7 @@ static int ufshcd_pltfrm_remove(struct platform_device *pdev)
static const struct of_device_id ufs_of_match[] = {
{ .compatible = "jedec,ufs-1.1"},
+ {},
};
static const struct dev_pm_ops ufshcd_dev_pm_ops = {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 08/10] ufs: fix register address in UIC error interrupt handling
2013-06-26 17:09 [PATCH RESEND 00/10] scsi: ufs: link start-up rework and other fixes Santosh Y
` (6 preceding siblings ...)
2013-06-26 17:09 ` [PATCH 07/10] ufshcd-pltfrm: add missing empty slot in ufs_of_match[] Santosh Y
@ 2013-06-26 17:09 ` Santosh Y
2013-06-26 17:09 ` [PATCH 09/10] ufshcd-pltfrm: remove unnecessary dma_set_coherent_mask() call Santosh Y
2013-06-26 17:09 ` [PATCH 10/10] ufs: fix DMA mask setting Santosh Y
9 siblings, 0 replies; 18+ messages in thread
From: Santosh Y @ 2013-06-26 17:09 UTC (permalink / raw)
To: james.bottomley
Cc: linux-scsi, vinholikatti, Akinobu Mita, James E.J. Bottomley, Santosh Y
From: Akinobu Mita <mita@fixstars.com>
In UIC error interrupt handling, it checks if UIC data link layer error
code indicates PA_INIT_ERROR in order to determine whether a fatal error
handling is needed or not.
But the code tries to read UIC data link layer error code from wrong
REG_UIC_ERROR_CODE_PHY_ADAPTER_LAYER, it should be
REG_UIC_ERROR_CODE_DATA_LINK_LAYER.
Signed-off-by: Akinobu Mita <mita@fixstars.com>
Cc: Vinayak Holikatti <vinholikatti@gmail.com>
Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
Cc: linux-scsi@vger.kernel.org
Signed-off-by: Santosh Y <santoshsy@gmail.com>
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 255f5be..19618c6 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -1349,7 +1349,7 @@ static void ufshcd_err_handler(struct ufs_hba *hba)
goto fatal_eh;
if (hba->errors & UIC_ERROR) {
- reg = ufshcd_readl(hba, REG_UIC_ERROR_CODE_PHY_ADAPTER_LAYER);
+ reg = ufshcd_readl(hba, REG_UIC_ERROR_CODE_DATA_LINK_LAYER);
if (reg & UIC_DATA_LINK_LAYER_ERROR_PA_INIT)
goto fatal_eh;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 09/10] ufshcd-pltfrm: remove unnecessary dma_set_coherent_mask() call
2013-06-26 17:09 [PATCH RESEND 00/10] scsi: ufs: link start-up rework and other fixes Santosh Y
` (7 preceding siblings ...)
2013-06-26 17:09 ` [PATCH 08/10] ufs: fix register address in UIC error interrupt handling Santosh Y
@ 2013-06-26 17:09 ` Santosh Y
2013-06-26 17:09 ` [PATCH 10/10] ufs: fix DMA mask setting Santosh Y
9 siblings, 0 replies; 18+ messages in thread
From: Santosh Y @ 2013-06-26 17:09 UTC (permalink / raw)
To: james.bottomley
Cc: linux-scsi, vinholikatti, Akinobu Mita, James E.J. Bottomley, Santosh Y
From: Akinobu Mita <mita@fixstars.com>
Changing the device coherent dma mask to the value that currently set
has no effect.
Signed-off-by: Akinobu Mita <mita@fixstars.com>
Cc: Vinayak Holikatti <vinholikatti@gmail.com>
Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
Cc: linux-scsi@vger.kernel.org
Signed-off-by: Santosh Y <santoshsy@gmail.com>
diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c
index 6829a16..c42db40 100644
--- a/drivers/scsi/ufs/ufshcd-pltfrm.c
+++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
@@ -122,12 +122,6 @@ static int ufshcd_pltfrm_probe(struct platform_device *pdev)
goto out;
}
- err = dma_set_coherent_mask(dev, dev->coherent_dma_mask);
- if (err) {
- dev_err(dev, "set dma mask failed\n");
- goto out;
- }
-
err = ufshcd_init(dev, &hba, mmio_base, irq);
if (err) {
dev_err(dev, "Intialization failed\n");
--
1.8.3.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 10/10] ufs: fix DMA mask setting
2013-06-26 17:09 [PATCH RESEND 00/10] scsi: ufs: link start-up rework and other fixes Santosh Y
` (8 preceding siblings ...)
2013-06-26 17:09 ` [PATCH 09/10] ufshcd-pltfrm: remove unnecessary dma_set_coherent_mask() call Santosh Y
@ 2013-06-26 17:09 ` Santosh Y
2013-06-28 20:37 ` James Bottomley
9 siblings, 1 reply; 18+ messages in thread
From: Santosh Y @ 2013-06-26 17:09 UTC (permalink / raw)
To: james.bottomley
Cc: linux-scsi, vinholikatti, Akinobu Mita, James E.J. Bottomley, Santosh Y
From: Akinobu Mita <mita@fixstars.com>
If the controller doesn't support 64-bit addressing mode, it must not
set the DMA mask to 64-bit. But it's unconditionally trying to set to
64-bit without checking 64-bit addressing support in the controller
capabilities.
It was correctly checked before commit 3b1d05807a9a68c6d0580e9248247a774a4d3be6
("[SCSI] ufs: Segregate PCI Specific Code"), this aims to restore
the correct behaviour.
To achieve this in a generic way, firstly we should push down the DMA
mask setting routine ufshcd_set_dma_mask() from PCI glue driver to core
driver in order to do it for both PCI glue driver and Platform glue
driver. Secondly, we should change pci_ DMA mapping API to dma_ DMA
mapping API because core driver is independent of glue drivers.
Lastly, we need to relax dma_set_mask(dev, DMA_BIT_MASK(32)) error check
for platform devices on ARM, which do not have a valid dma_mask pointer.
Signed-off-by: Akinobu Mita <mita@fixstars.com>
Cc: Vinayak Holikatti <vinholikatti@gmail.com>
Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
Cc: linux-scsi@vger.kernel.org
Signed-off-by: Santosh Y <santoshsy@gmail.com>
diff --git a/drivers/scsi/ufs/ufshcd-pci.c b/drivers/scsi/ufs/ufshcd-pci.c
index 48be39a..64d36eb 100644
--- a/drivers/scsi/ufs/ufshcd-pci.c
+++ b/drivers/scsi/ufs/ufshcd-pci.c
@@ -100,26 +100,6 @@ static void ufshcd_pci_remove(struct pci_dev *pdev)
}
/**
- * ufshcd_set_dma_mask - Set dma mask based on the controller
- * addressing capability
- * @pdev: PCI device structure
- *
- * Returns 0 for success, non-zero for failure
- */
-static int ufshcd_set_dma_mask(struct pci_dev *pdev)
-{
- int err;
-
- if (!pci_set_dma_mask(pdev, DMA_BIT_MASK(64))
- && !pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(64)))
- return 0;
- err = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
- if (!err)
- err = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(32));
- return err;
-}
-
-/**
* ufshcd_pci_probe - probe routine of the driver
* @pdev: pointer to PCI device handle
* @id: PCI device id
@@ -155,12 +135,6 @@ ufshcd_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
goto out_release_regions;
}
- err = ufshcd_set_dma_mask(pdev);
- if (err) {
- dev_err(&pdev->dev, "set dma mask failed\n");
- goto out_iounmap;
- }
-
err = ufshcd_init(&pdev->dev, &hba, mmio_base, pdev->irq);
if (err) {
dev_err(&pdev->dev, "Initialization failed\n");
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 19618c6..431ddb2 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -1711,6 +1711,25 @@ void ufshcd_remove(struct ufs_hba *hba)
EXPORT_SYMBOL_GPL(ufshcd_remove);
/**
+ * ufshcd_set_dma_mask - Set dma mask based on the controller
+ * addressing capability
+ * @hba: per adapter instance
+ *
+ * Returns 0 for success, non-zero for failure
+ */
+static int ufshcd_set_dma_mask(struct ufs_hba *hba)
+{
+ if (hba->capabilities & MASK_64_ADDRESSING_SUPPORT) {
+ if (!dma_set_mask(hba->dev, DMA_BIT_MASK(64)) &&
+ !dma_set_coherent_mask(hba->dev, DMA_BIT_MASK(64)))
+ return 0;
+ }
+ dma_set_mask(hba->dev, DMA_BIT_MASK(32));
+
+ return dma_set_coherent_mask(hba->dev, DMA_BIT_MASK(32));
+}
+
+/**
* ufshcd_init - Driver initialization routine
* @dev: pointer to device handle
* @hba_handle: driver private handle
@@ -1761,6 +1780,12 @@ int ufshcd_init(struct device *dev, struct ufs_hba **hba_handle,
/* Get Interrupt bit mask per version */
hba->intr_mask = ufshcd_get_intr_mask(hba);
+ err = ufshcd_set_dma_mask(hba);
+ if (err) {
+ dev_err(hba->dev, "set dma mask failed\n");
+ goto out_disable;
+ }
+
/* Allocate memory for host memory space */
err = ufshcd_memory_alloc(hba);
if (err) {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH 10/10] ufs: fix DMA mask setting
2013-06-26 17:09 ` [PATCH 10/10] ufs: fix DMA mask setting Santosh Y
@ 2013-06-28 20:37 ` James Bottomley
2013-06-29 5:40 ` Akinobu Mita
0 siblings, 1 reply; 18+ messages in thread
From: James Bottomley @ 2013-06-28 20:37 UTC (permalink / raw)
To: Santosh Y; +Cc: linux-scsi, vinholikatti, Akinobu Mita
On Wed, 2013-06-26 at 22:39 +0530, Santosh Y wrote:
> index 19618c6..431ddb2 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -1711,6 +1711,25 @@ void ufshcd_remove(struct ufs_hba *hba)
> EXPORT_SYMBOL_GPL(ufshcd_remove);
>
> /**
> + * ufshcd_set_dma_mask - Set dma mask based on the controller
> + * addressing capability
> + * @hba: per adapter instance
> + *
> + * Returns 0 for success, non-zero for failure
> + */
> +static int ufshcd_set_dma_mask(struct ufs_hba *hba)
> +{
> + if (hba->capabilities & MASK_64_ADDRESSING_SUPPORT) {
> + if (!dma_set_mask(hba->dev, DMA_BIT_MASK(64)) &&
> + !dma_set_coherent_mask(hba->dev, DMA_BIT_MASK(64)))
> + return 0;
> + }
> + dma_set_mask(hba->dev, DMA_BIT_MASK(32));
> +
> + return dma_set_coherent_mask(hba->dev, DMA_BIT_MASK(32));
> +}
This isn't right per the API spec. The guarantee is that if
dma_set_mask() succeeds then dma_set_coherent_mask of the same mask will
succeed, so this should read
int err;
if (hba->capabilities & MASK_64_ADDRESSING_SUPPORT) {
if (!dma_set_mask(hba->dev, DMA_BIT_MASK(64))) {
dma_set_coherent_mask(hba->dev, DMA_BIT_MASK(64)))
return 0;
}
}
err = dma_set_mask(hba->dev, DMA_BIT_MASK(32));
if (!err)
dma_set_coherent_mask(hba->dev, DMA_BIT_MASK(32));
return err;
James
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 10/10] ufs: fix DMA mask setting
2013-06-28 20:37 ` James Bottomley
@ 2013-06-29 5:40 ` Akinobu Mita
2013-08-10 6:08 ` Sujit Reddy Thumma
0 siblings, 1 reply; 18+ messages in thread
From: Akinobu Mita @ 2013-06-29 5:40 UTC (permalink / raw)
To: James Bottomley; +Cc: Santosh Y, linux-scsi, vinholikatti
2013/6/29 James Bottomley <James.Bottomley@hansenpartnership.com>:
> On Wed, 2013-06-26 at 22:39 +0530, Santosh Y wrote:
>> index 19618c6..431ddb2 100644
>> --- a/drivers/scsi/ufs/ufshcd.c
>> +++ b/drivers/scsi/ufs/ufshcd.c
>> @@ -1711,6 +1711,25 @@ void ufshcd_remove(struct ufs_hba *hba)
>> EXPORT_SYMBOL_GPL(ufshcd_remove);
>>
>> /**
>> + * ufshcd_set_dma_mask - Set dma mask based on the controller
>> + * addressing capability
>> + * @hba: per adapter instance
>> + *
>> + * Returns 0 for success, non-zero for failure
>> + */
>> +static int ufshcd_set_dma_mask(struct ufs_hba *hba)
>> +{
>> + if (hba->capabilities & MASK_64_ADDRESSING_SUPPORT) {
>> + if (!dma_set_mask(hba->dev, DMA_BIT_MASK(64)) &&
>> + !dma_set_coherent_mask(hba->dev, DMA_BIT_MASK(64)))
>> + return 0;
>> + }
>> + dma_set_mask(hba->dev, DMA_BIT_MASK(32));
>> +
>> + return dma_set_coherent_mask(hba->dev, DMA_BIT_MASK(32));
>> +}
>
> This isn't right per the API spec. The guarantee is that if
> dma_set_mask() succeeds then dma_set_coherent_mask of the same mask will
> succeed, so this should read
>
> int err;
>
> if (hba->capabilities & MASK_64_ADDRESSING_SUPPORT) {
> if (!dma_set_mask(hba->dev, DMA_BIT_MASK(64))) {
> dma_set_coherent_mask(hba->dev, DMA_BIT_MASK(64)))
> return 0;
> }
> }
> err = dma_set_mask(hba->dev, DMA_BIT_MASK(32));
> if (!err)
> dma_set_coherent_mask(hba->dev, DMA_BIT_MASK(32));
> return err;
Thanks for the explanation. I agree that this is the correct definision
of ufshcd_set_dma_mask().
The reason that I omitted the error check on dma_set_mask(DMA_BIT_MASK(32))
in the patch was that I was seeing that error due to the luck of
valid dev->dma_mask pointer on OF platform devices although
dma_supported(DMA_BIT_MASK(32)) returns true.
However, now I think that I should investigate more about dev->dma_mask
issue rather than jumping into short-circuit fixing in the driver. And
then I will revisit this.
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 10/10] ufs: fix DMA mask setting
2013-06-29 5:40 ` Akinobu Mita
@ 2013-08-10 6:08 ` Sujit Reddy Thumma
2013-08-15 14:48 ` Akinobu Mita
0 siblings, 1 reply; 18+ messages in thread
From: Sujit Reddy Thumma @ 2013-08-10 6:08 UTC (permalink / raw)
To: Akinobu Mita; +Cc: James Bottomley, Santosh Y, linux-scsi, vinholikatti
On 6/29/2013 11:10 AM, Akinobu Mita wrote:
> 2013/6/29 James Bottomley <James.Bottomley@hansenpartnership.com>:
>> On Wed, 2013-06-26 at 22:39 +0530, Santosh Y wrote:
>>> index 19618c6..431ddb2 100644
>>> --- a/drivers/scsi/ufs/ufshcd.c
>>> +++ b/drivers/scsi/ufs/ufshcd.c
>>> @@ -1711,6 +1711,25 @@ void ufshcd_remove(struct ufs_hba *hba)
>>> EXPORT_SYMBOL_GPL(ufshcd_remove);
>>>
>>> /**
>>> + * ufshcd_set_dma_mask - Set dma mask based on the controller
>>> + * addressing capability
>>> + * @hba: per adapter instance
>>> + *
>>> + * Returns 0 for success, non-zero for failure
>>> + */
>>> +static int ufshcd_set_dma_mask(struct ufs_hba *hba)
>>> +{
>>> + if (hba->capabilities & MASK_64_ADDRESSING_SUPPORT) {
>>> + if (!dma_set_mask(hba->dev, DMA_BIT_MASK(64)) &&
>>> + !dma_set_coherent_mask(hba->dev, DMA_BIT_MASK(64)))
>>> + return 0;
>>> + }
>>> + dma_set_mask(hba->dev, DMA_BIT_MASK(32));
>>> +
>>> + return dma_set_coherent_mask(hba->dev, DMA_BIT_MASK(32));
>>> +}
>>
>> This isn't right per the API spec. The guarantee is that if
>> dma_set_mask() succeeds then dma_set_coherent_mask of the same mask will
>> succeed, so this should read
>>
>> int err;
>>
>> if (hba->capabilities & MASK_64_ADDRESSING_SUPPORT) {
>> if (!dma_set_mask(hba->dev, DMA_BIT_MASK(64))) {
>> dma_set_coherent_mask(hba->dev, DMA_BIT_MASK(64)))
>> return 0;
>> }
>> }
>> err = dma_set_mask(hba->dev, DMA_BIT_MASK(32));
>> if (!err)
>> dma_set_coherent_mask(hba->dev, DMA_BIT_MASK(32));
>> return err;
>
> Thanks for the explanation. I agree that this is the correct definision
> of ufshcd_set_dma_mask().
>
> The reason that I omitted the error check on dma_set_mask(DMA_BIT_MASK(32))
> in the patch was that I was seeing that error due to the luck of
> valid dev->dma_mask pointer on OF platform devices although
> dma_supported(DMA_BIT_MASK(32)) returns true.
The popular trick implemented for device-tree probed devices is -
dev->dma_mask = &dev->coherent_dma_mask;
If you don't agree with this you can have something like -
dev->dma_mask = devm_kzalloc(dev, sizeof(*dev->dma_mask), GFP_KERNEL);
See platform_device_register_full();
>
> However, now I think that I should investigate more about dev->dma_mask
> issue rather than jumping into short-circuit fixing in the driver. And
> then I will revisit this.
--
Regards,
Sujit
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 10/10] ufs: fix DMA mask setting
2013-08-10 6:08 ` Sujit Reddy Thumma
@ 2013-08-15 14:48 ` Akinobu Mita
0 siblings, 0 replies; 18+ messages in thread
From: Akinobu Mita @ 2013-08-15 14:48 UTC (permalink / raw)
To: Sujit Reddy Thumma
Cc: Akinobu Mita, James Bottomley, Santosh Y, linux-scsi, vinholikatti
> On 6/29/2013 11:10 AM, Akinobu Mita wrote:
>> 2013/6/29 James Bottomley <James.Bottomley@hansenpartnership.com>:
>>> On Wed, 2013-06-26 at 22:39 +0530, Santosh Y wrote:
>>>> index 19618c6..431ddb2 100644
>>>> --- a/drivers/scsi/ufs/ufshcd.c
>>>> +++ b/drivers/scsi/ufs/ufshcd.c
>>>> @@ -1711,6 +1711,25 @@ void ufshcd_remove(struct ufs_hba *hba)
>>>> EXPORT_SYMBOL_GPL(ufshcd_remove);
>>>>
>>>> /**
>>>> + * ufshcd_set_dma_mask - Set dma mask based on the controller
>>>> + * addressing capability
>>>> + * @hba: per adapter instance
>>>> + *
>>>> + * Returns 0 for success, non-zero for failure
>>>> + */
>>>> +static int ufshcd_set_dma_mask(struct ufs_hba *hba)
>>>> +{
>>>> + if (hba->capabilities & MASK_64_ADDRESSING_SUPPORT) {
>>>> + if (!dma_set_mask(hba->dev, DMA_BIT_MASK(64)) &&
>>>> + !dma_set_coherent_mask(hba->dev, DMA_BIT_MASK(64)))
>>>> + return 0;
>>>> + }
>>>> + dma_set_mask(hba->dev, DMA_BIT_MASK(32));
>>>> +
>>>> + return dma_set_coherent_mask(hba->dev, DMA_BIT_MASK(32));
>>>> +}
>>>
>>> This isn't right per the API spec. The guarantee is that if
>>> dma_set_mask() succeeds then dma_set_coherent_mask of the same mask
>>> will
>>> succeed, so this should read
>>>
>>> int err;
>>>
>>> if (hba->capabilities & MASK_64_ADDRESSING_SUPPORT) {
>>> if (!dma_set_mask(hba->dev, DMA_BIT_MASK(64))) {
>>> dma_set_coherent_mask(hba->dev,
>>> DMA_BIT_MASK(64)))
>>> return 0;
>>> }
>>> }
>>> err = dma_set_mask(hba->dev, DMA_BIT_MASK(32));
>>> if (!err)
>>> dma_set_coherent_mask(hba->dev, DMA_BIT_MASK(32));
>>> return err;
>>
>> Thanks for the explanation. I agree that this is the correct definision
>> of ufshcd_set_dma_mask().
>>
>> The reason that I omitted the error check on
>> dma_set_mask(DMA_BIT_MASK(32))
>> in the patch was that I was seeing that error due to the luck of
>> valid dev->dma_mask pointer on OF platform devices although
>> dma_supported(DMA_BIT_MASK(32)) returns true.
>
> The popular trick implemented for device-tree probed devices is -
> dev->dma_mask = &dev->coherent_dma_mask;
This looks the right solution for now. I'll send revised patches that
include this trick in ufshcd-pltfrm and fixed ufshcd_set_dma_mask() as
James suggested above.
^ permalink raw reply [flat|nested] 18+ messages in thread