From: Shawn Lin <shawn.lin@rock-chips.com>
To: Jaehoon Chung <jh80.chung@samsung.com>,
Ulf Hansson <ulf.hansson@linaro.org>
Cc: shawn.lin@rock-chips.com, Seungwon Jeon <tgih.jun@samsung.com>,
linux-mmc@vger.kernel.org, linux-kernel@vger.kernel.org,
dianders@chromium.org
Subject: Re: [PATCH] mmc: dw_mmc: Fix coding style issues
Date: Mon, 3 Aug 2015 14:29:47 +0800 [thread overview]
Message-ID: <55BF0A5B.7040703@rock-chips.com> (raw)
In-Reply-To: <55BEE8FF.3030207@samsung.com>
On 2015-8-3 12:07, Jaehoon Chung wrote:
> Hi, Shawn.
>
> On 07/28/2015 12:06 PM, Shawn Lin wrote:
>> This patch fixes the following issues reported by checkpatch.pl:
>> - use -EINVAL instead of -ENOSYS, to fix warning message:
>> "ENOSYS means 'invalid syscall nr' and nothing else"
>> - split lines whose length is greater than 80 characters
>> - avoid quoted string split across lines
>> - use min_t instead of min, to fix warning message:
>> "min() should probably be min_t(int, cnt, host->part_buf_count)"
>
> Thanks for fixing coding style.
> But i will apply this patch(https://patchwork.kernel.org/patch/6672581/).
> So if you can fix with this patch, then it's helpful to me.
> If you can't, i will modify your patch before applying.
> How about?
>
Okay, I will fix with this patch and resend it.
>>
>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>> ---
>>
>> drivers/mmc/host/dw_mmc.c | 85 ++++++++++++++++++++++++++++++-----------------
>> 1 file changed, 54 insertions(+), 31 deletions(-)
>>
>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index 40e9d8e..6aede38 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -235,8 +235,8 @@ static u32 dw_mci_prepare_command(struct mmc_host *mmc, struct mmc_command *cmd)
>> struct dw_mci *host = slot->host;
>> const struct dw_mci_drv_data *drv_data = slot->host->drv_data;
>> u32 cmdr;
>> - cmd->error = -EINPROGRESS;
>>
>> + cmd->error = -EINPROGRESS;
>> cmdr = cmd->opcode;
>>
>> if (cmd->opcode == MMC_STOP_TRANSMISSION ||
>> @@ -371,6 +371,7 @@ static void dw_mci_start_command(struct dw_mci *host,
>> cmd->arg, cmd_flags);
>>
>> mci_writel(host, CMDARG, cmd->arg);
>> + /* Make sure CMDARG is configured before CMD */
>> wmb();
>> dw_mci_wait_while_busy(host, cmd_flags);
>>
>> @@ -380,6 +381,7 @@ static void dw_mci_start_command(struct dw_mci *host,
>> static inline void send_stop_abort(struct dw_mci *host, struct mmc_data *data)
>> {
>> struct mmc_command *stop = data->stop ? data->stop : &host->stop_abort;
>> +
>> dw_mci_start_command(host, stop, host->stop_cmdr);
>> }
>>
>> @@ -463,6 +465,7 @@ static void dw_mci_translate_sglist(struct dw_mci *host, struct mmc_data *data,
>> unsigned int sg_len)
>> {
>> int i;
>> +
>> if (host->dma_64bit_address == 1) {
>> struct idmac_desc_64addr *desc = host->sg_cpu;
>>
>> @@ -524,7 +527,7 @@ static void dw_mci_translate_sglist(struct dw_mci *host, struct mmc_data *data,
>> desc->des0 |= cpu_to_le32(IDMAC_DES0_LD);
>> }
>>
>> - wmb();
>> + wmb(); /* drain writebuffer */
>> }
>>
>> static void dw_mci_idmac_start_dma(struct dw_mci *host, unsigned int sg_len)
>> @@ -542,7 +545,7 @@ static void dw_mci_idmac_start_dma(struct dw_mci *host, unsigned int sg_len)
>> temp |= SDMMC_CTRL_USE_IDMAC;
>> mci_writel(host, CTRL, temp);
>>
>> - wmb();
>> + wmb(); /* drain writebuffer */
>>
>> /* Enable the IDMAC */
>> temp = mci_readl(host, BMOD);
>> @@ -589,7 +592,9 @@ static int dw_mci_idmac_init(struct dw_mci *host)
>> host->ring_size = PAGE_SIZE / sizeof(struct idmac_desc);
>>
>> /* Forward link the descriptor list */
>> - for (i = 0, p = host->sg_cpu; i < host->ring_size - 1; i++, p++) {
>> + for (i = 0, p = host->sg_cpu;
>> + i < host->ring_size - 1;
>> + i++, p++) {
>> p->des3 = cpu_to_le32(host->sg_dma +
>> (sizeof(struct idmac_desc) * (i + 1)));
>> p->des1 = 0;
>> @@ -718,7 +723,7 @@ static void dw_mci_adjust_fifoth(struct dw_mci *host, struct mmc_data *data)
>> u32 fifo_width = 1 << host->data_shift;
>> u32 blksz_depth = blksz / fifo_width, fifoth_val;
>> u32 msize = 0, rx_wmark = 1, tx_wmark, tx_wmark_invers;
>> - int idx = (sizeof(mszs) / sizeof(mszs[0])) - 1;
>> + int idx = ARRAY_SIZE(mszs) - 1;
>>
>> tx_wmark = (host->fifo_depth) / 2;
>> tx_wmark_invers = host->fifo_depth - tx_wmark;
>> @@ -843,6 +848,7 @@ static int dw_mci_submit_data_dma(struct dw_mci *host, struct mmc_data *data)
>> static void dw_mci_submit_data(struct dw_mci *host, struct mmc_data *data)
>> {
>> unsigned long irqflags;
>> + int flags = SG_MITER_ATOMIC;
>> u32 temp;
>>
>> data->error = -EINPROGRESS;
>> @@ -859,7 +865,6 @@ static void dw_mci_submit_data(struct dw_mci *host, struct mmc_data *data)
>> }
>>
>> if (dw_mci_submit_data_dma(host, data)) {
>> - int flags = SG_MITER_ATOMIC;
>> if (host->data->flags & MMC_DATA_READ)
>> flags |= SG_MITER_TO_SG;
>> else
>> @@ -906,6 +911,7 @@ static void mci_send_cmd(struct dw_mci_slot *slot, u32 cmd, u32 arg)
>> unsigned int cmd_status = 0;
>>
>> mci_writel(host, CMDARG, arg);
>> + /* Make sure CMDARG is configured before CMD */
>> wmb();
>> dw_mci_wait_while_busy(host, cmd);
>> mci_writel(host, CMD, SDMMC_CMD_START | cmd);
>> @@ -1019,6 +1025,7 @@ static void __dw_mci_start_request(struct dw_mci *host,
>>
>> if (data) {
>> dw_mci_submit_data(host, data);
>> + /* drain writebuffer */
>> wmb();
>
> I think good that you have consistency for above comment.
> wmb(); /* drain writebuffer */
> or
> /* drain writebuffer */
> wmb();
>
> What do you want?
>
> Best Regards,
> Jaehoon Chung
>
>> }
>>
>> @@ -1384,14 +1391,15 @@ static int dw_mci_execute_tuning(struct mmc_host *mmc, u32 opcode)
>> struct dw_mci_slot *slot = mmc_priv(mmc);
>> struct dw_mci *host = slot->host;
>> const struct dw_mci_drv_data *drv_data = host->drv_data;
>> - int err = -ENOSYS;
>> + int err = -EINVAL;
>>
>> if (drv_data && drv_data->execute_tuning)
>> err = drv_data->execute_tuning(slot);
>> return err;
>> }
>>
>> -static int dw_mci_prepare_hs400_tuning(struct mmc_host *mmc, struct mmc_ios *ios)
>> +static int dw_mci_prepare_hs400_tuning(struct mmc_host *mmc,
>> + struct mmc_ios *ios)
>> {
>> struct dw_mci_slot *slot = mmc_priv(mmc);
>> struct dw_mci *host = slot->host;
>> @@ -1743,7 +1751,7 @@ static int dw_mci_push_part_bytes(struct dw_mci *host, void *buf, int cnt)
>> /* pull first bytes from part_buf, only use during pull */
>> static int dw_mci_pull_part_bytes(struct dw_mci *host, void *buf, int cnt)
>> {
>> - cnt = min(cnt, (int)host->part_buf_count);
>> + cnt = min_t(int, cnt, host->part_buf_count);
>> if (cnt) {
>> memcpy(buf, (void *)&host->part_buf + host->part_buf_start,
>> cnt);
>> @@ -1769,6 +1777,7 @@ static void dw_mci_push_data16(struct dw_mci *host, void *buf, int cnt)
>> /* try and push anything in the part_buf */
>> if (unlikely(host->part_buf_count)) {
>> int len = dw_mci_push_part_bytes(host, buf, cnt);
>> +
>> buf += len;
>> cnt -= len;
>> if (host->part_buf_count == 2) {
>> @@ -1795,6 +1804,7 @@ static void dw_mci_push_data16(struct dw_mci *host, void *buf, int cnt)
>> #endif
>> {
>> u16 *pdata = buf;
>> +
>> for (; cnt >= 2; cnt -= 2)
>> mci_fifo_writew(host->fifo_reg, *pdata++);
>> buf = pdata;
>> @@ -1819,6 +1829,7 @@ static void dw_mci_pull_data16(struct dw_mci *host, void *buf, int cnt)
>> int len = min(cnt & -2, (int)sizeof(aligned_buf));
>> int items = len >> 1;
>> int i;
>> +
>> for (i = 0; i < items; ++i)
>> aligned_buf[i] = mci_fifo_readw(host->fifo_reg);
>> /* memcpy from aligned buffer into output buffer */
>> @@ -1830,6 +1841,7 @@ static void dw_mci_pull_data16(struct dw_mci *host, void *buf, int cnt)
>> #endif
>> {
>> u16 *pdata = buf;
>> +
>> for (; cnt >= 2; cnt -= 2)
>> *pdata++ = mci_fifo_readw(host->fifo_reg);
>> buf = pdata;
>> @@ -1848,6 +1860,7 @@ static void dw_mci_push_data32(struct dw_mci *host, void *buf, int cnt)
>> /* try and push anything in the part_buf */
>> if (unlikely(host->part_buf_count)) {
>> int len = dw_mci_push_part_bytes(host, buf, cnt);
>> +
>> buf += len;
>> cnt -= len;
>> if (host->part_buf_count == 4) {
>> @@ -1874,6 +1887,7 @@ static void dw_mci_push_data32(struct dw_mci *host, void *buf, int cnt)
>> #endif
>> {
>> u32 *pdata = buf;
>> +
>> for (; cnt >= 4; cnt -= 4)
>> mci_fifo_writel(host->fifo_reg, *pdata++);
>> buf = pdata;
>> @@ -1898,6 +1912,7 @@ static void dw_mci_pull_data32(struct dw_mci *host, void *buf, int cnt)
>> int len = min(cnt & -4, (int)sizeof(aligned_buf));
>> int items = len >> 2;
>> int i;
>> +
>> for (i = 0; i < items; ++i)
>> aligned_buf[i] = mci_fifo_readl(host->fifo_reg);
>> /* memcpy from aligned buffer into output buffer */
>> @@ -1909,6 +1924,7 @@ static void dw_mci_pull_data32(struct dw_mci *host, void *buf, int cnt)
>> #endif
>> {
>> u32 *pdata = buf;
>> +
>> for (; cnt >= 4; cnt -= 4)
>> *pdata++ = mci_fifo_readl(host->fifo_reg);
>> buf = pdata;
>> @@ -1927,6 +1943,7 @@ static void dw_mci_push_data64(struct dw_mci *host, void *buf, int cnt)
>> /* try and push anything in the part_buf */
>> if (unlikely(host->part_buf_count)) {
>> int len = dw_mci_push_part_bytes(host, buf, cnt);
>> +
>> buf += len;
>> cnt -= len;
>>
>> @@ -1954,6 +1971,7 @@ static void dw_mci_push_data64(struct dw_mci *host, void *buf, int cnt)
>> #endif
>> {
>> u64 *pdata = buf;
>> +
>> for (; cnt >= 8; cnt -= 8)
>> mci_fifo_writeq(host->fifo_reg, *pdata++);
>> buf = pdata;
>> @@ -1978,6 +1996,7 @@ static void dw_mci_pull_data64(struct dw_mci *host, void *buf, int cnt)
>> int len = min(cnt & -8, (int)sizeof(aligned_buf));
>> int items = len >> 3;
>> int i;
>> +
>> for (i = 0; i < items; ++i)
>> aligned_buf[i] = mci_fifo_readq(host->fifo_reg);
>>
>> @@ -1990,6 +2009,7 @@ static void dw_mci_pull_data64(struct dw_mci *host, void *buf, int cnt)
>> #endif
>> {
>> u64 *pdata = buf;
>> +
>> for (; cnt >= 8; cnt -= 8)
>> *pdata++ = mci_fifo_readq(host->fifo_reg);
>> buf = pdata;
>> @@ -2065,7 +2085,7 @@ static void dw_mci_read_data_pio(struct dw_mci *host, bool dto)
>> done:
>> sg_miter_stop(sg_miter);
>> host->sg = NULL;
>> - smp_wmb();
>> + smp_wmb(); /* drain writebuffer */
>> set_bit(EVENT_XFER_COMPLETE, &host->pending_events);
>> }
>>
>> @@ -2119,7 +2139,7 @@ static void dw_mci_write_data_pio(struct dw_mci *host)
>> done:
>> sg_miter_stop(sg_miter);
>> host->sg = NULL;
>> - smp_wmb();
>> + smp_wmb(); /* drain writebuffer */
>> set_bit(EVENT_XFER_COMPLETE, &host->pending_events);
>> }
>>
>> @@ -2128,6 +2148,7 @@ static void dw_mci_cmd_interrupt(struct dw_mci *host, u32 status)
>> if (!host->cmd_status)
>> host->cmd_status = status;
>>
>> + /* drain writebuffer */
>> smp_wmb();
>>
>> set_bit(EVENT_CMD_COMPLETE, &host->pending_events);
>> @@ -2192,7 +2213,7 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
>> if (pending & DW_MCI_CMD_ERROR_FLAGS) {
>> mci_writel(host, RINTSTS, DW_MCI_CMD_ERROR_FLAGS);
>> host->cmd_status = pending;
>> - smp_wmb();
>> + smp_wmb(); /* drain writebuffer */
>> set_bit(EVENT_CMD_COMPLETE, &host->pending_events);
>> }
>>
>> @@ -2200,7 +2221,7 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
>> /* if there is an error report DATA_ERROR */
>> mci_writel(host, RINTSTS, DW_MCI_DATA_ERROR_FLAGS);
>> host->data_status = pending;
>> - smp_wmb();
>> + smp_wmb(); /* drain writebuffer */
>> set_bit(EVENT_DATA_ERROR, &host->pending_events);
>> tasklet_schedule(&host->tasklet);
>> }
>> @@ -2209,7 +2230,7 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
>> mci_writel(host, RINTSTS, SDMMC_INT_DATA_OVER);
>> if (!host->data_status)
>> host->data_status = pending;
>> - smp_wmb();
>> + smp_wmb(); /* drain writebuffer */
>> if (host->dir_status == DW_MCI_RECV_STATUS) {
>> if (host->sg != NULL)
>> dw_mci_read_data_pio(host, true);
>> @@ -2473,8 +2494,8 @@ static void dw_mci_init_dma(struct dw_mci *host)
>> if (host->dma_ops->init && host->dma_ops->start &&
>> host->dma_ops->stop && host->dma_ops->cleanup) {
>> if (host->dma_ops->init(host)) {
>> - dev_err(host->dev, "%s: Unable to initialize "
>> - "DMA Controller.\n", __func__);
>> + dev_err(host->dev, "%s: Unable to initialize DMA Controller.\n",
>> + __func__);
>> goto no_dma;
>> }
>> } else {
>> @@ -2488,7 +2509,6 @@ static void dw_mci_init_dma(struct dw_mci *host)
>> no_dma:
>> dev_info(host->dev, "Using PIO mode.\n");
>> host->use_dma = 0;
>> - return;
>> }
>>
>> static bool dw_mci_ctrl_reset(struct dw_mci *host, u32 reset)
>> @@ -2542,6 +2562,7 @@ static bool dw_mci_reset(struct dw_mci *host)
>> if (host->use_dma) {
>> unsigned long timeout = jiffies + msecs_to_jiffies(500);
>> u32 status;
>> +
>> do {
>> status = mci_readl(host, STATUS);
>> if (!(status & SDMMC_STATUS_DMA_REQ))
>> @@ -2551,8 +2572,8 @@ static bool dw_mci_reset(struct dw_mci *host)
>>
>> if (status & SDMMC_STATUS_DMA_REQ) {
>> dev_err(host->dev,
>> - "%s: Timeout waiting for dma_req to "
>> - "clear during reset\n", __func__);
>> + "%s: Timeout waiting for dma_req to clear during reset\n",
>> + __func__);
>> goto ciu_out;
>> }
>>
>> @@ -2563,8 +2584,8 @@ static bool dw_mci_reset(struct dw_mci *host)
>> } else {
>> /* if the controller reset bit did clear, then set clock regs */
>> if (!(mci_readl(host, CTRL) & SDMMC_CTRL_RESET)) {
>> - dev_err(host->dev, "%s: fifo/dma reset bits didn't "
>> - "clear but ciu was reset, doing clock update\n",
>> + dev_err(host->dev,
>> + "%s: fifo/dma reset bits didn't clear but ciu was reset, doing clock update\n",
>> __func__);
>> goto ciu_out;
>> }
>> @@ -2625,8 +2646,8 @@ static struct dw_mci_board *dw_mci_parse_dt(struct dw_mci *host)
>> /* find out number of slots supported */
>> if (of_property_read_u32(dev->of_node, "num-slots",
>> &pdata->num_slots)) {
>> - dev_info(dev, "num-slots property not found, "
>> - "assuming 1 slot is available\n");
>> + dev_info(dev,
>> + "num-slots property not found, assuming 1 slot is available\n");
>> pdata->num_slots = 1;
>> }
>>
>> @@ -2636,8 +2657,8 @@ static struct dw_mci_board *dw_mci_parse_dt(struct dw_mci *host)
>> pdata->quirks |= of_quirks[idx].id;
>>
>> if (of_property_read_u32(np, "fifo-depth", &pdata->fifo_depth))
>> - dev_info(dev, "fifo-depth property not found, using "
>> - "value of FIFOTH register as default\n");
>> + dev_info(dev,
>> + "fifo-depth property not found, using value of FIFOTH register as default\n");
>>
>> of_property_read_u32(np, "card-detect-delay", &pdata->detect_delay_ms);
>>
>> @@ -2874,11 +2895,11 @@ int dw_mci_probe(struct dw_mci *host)
>> mci_writel(host, INTMASK, SDMMC_INT_CMD_DONE | SDMMC_INT_DATA_OVER |
>> SDMMC_INT_TXDR | SDMMC_INT_RXDR |
>> DW_MCI_ERROR_FLAGS);
>> - mci_writel(host, CTRL, SDMMC_CTRL_INT_ENABLE); /* Enable mci interrupt */
>> + /* Enable mci interrupt */
>> + mci_writel(host, CTRL, SDMMC_CTRL_INT_ENABLE);
>>
>> - dev_info(host->dev, "DW MMC controller at irq %d, "
>> - "%d bit host data width, "
>> - "%u deep fifo\n",
>> + dev_info(host->dev,
>> + "DW MMC controller at irq %d,%d bit host data width,%u deep fifo\n",
>> host->irq, width, fifo_size);
>>
>> /* We need at least one slot to succeed */
>> @@ -2893,8 +2914,9 @@ int dw_mci_probe(struct dw_mci *host)
>> if (init_slots) {
>> dev_info(host->dev, "%d slots initialized\n", init_slots);
>> } else {
>> - dev_dbg(host->dev, "attempted to initialize %d slots, "
>> - "but failed on all\n", host->num_slots);
>> + dev_dbg(host->dev,
>> + "attempted to initialize %d slots, but failed on all\n",
>> + host->num_slots);
>> goto err_dmaunmap;
>> }
>>
>> @@ -2992,6 +3014,7 @@ int dw_mci_resume(struct dw_mci *host)
>>
>> for (i = 0; i < host->num_slots; i++) {
>> struct dw_mci_slot *slot = host->slot[i];
>> +
>> if (!slot)
>> continue;
>> if (slot->mmc->pm_flags & MMC_PM_KEEP_POWER) {
>>
>
>
>
>
prev parent reply other threads:[~2015-08-03 6:29 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-07-28 3:06 [PATCH] mmc: dw_mmc: Fix coding style issues Shawn Lin
2015-08-03 4:07 ` Jaehoon Chung
2015-08-03 6:29 ` Shawn Lin [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=55BF0A5B.7040703@rock-chips.com \
--to=shawn.lin@rock-chips.com \
--cc=dianders@chromium.org \
--cc=jh80.chung@samsung.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mmc@vger.kernel.org \
--cc=tgih.jun@samsung.com \
--cc=ulf.hansson@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.