Re: [PATCH] staging: rtl8188eu: Fix spelling mistake

2018-08-22 Thread Dan Carpenter 
On Tue, Aug 21, 2018 at 07:14:28AM +0530, Bhaskar Singh wrote:
> This patch fix spelling mistakes in TODO.
> 

Btw, it helps when you say which word you're changing, otherwise it
takes a while to spot the difference.  We changed "HGz" to "GHz".

Probably someone smarter than I am would have spotted it faster...

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v4 0/4] Fix debug macros and their usages

2018-08-22 Thread Nishad Kamdar 
This patchset fixes the four debug macros N_MSG, ERR_MSG, INIT_MSG and
IRQ_MSG. Each patch fixes one particular macro and its usages.

For N_MSG, replaces printk with dev_ without __func__ or __LINE__
or current->comm and current->pid. Removes the do {} while(0) loop for
the single statement macro.

For ERR_MSG and IRQ_MSG, makes the same changes as those for N_MSG, but
further drops the macros and replaces their usages with dev_.

Removes the INIT_MSG macro and its usages.

Changes in v4:
  - Create multiple patches, one for each type of macro being
deleted/changed.

Changes in v3:
  - Replace usages of ERR_MSG and IRQ_MSG with dev_err() in code itself.
  - Remove all INIT_MSG usages.
  - Drop ERR_MSG, INIT_MSG and IRQ_MSG from dbg.h.

Changes in v2:
  - Replace printk with dev_.
  - Remove __func__, __LINE__, current->comm, current->pid from arguments.
  - Remove the do {} while(0) loop from these macros.
  - Modify commit message to include other changes.
-
Nishad Kamdar (4):
  staging: mt7621-mmc: Fix debug macro N_MSG
  staging: mt7621-mmc: Fix debug macro ERR_MSG and its usages
  staging: mt7621-mmc: Remove macro INIT_MSG and its usages
  staging: mt7621-mmc: Fix debug macro IRQ_MSG and its usages

 drivers/staging/mt7621-mmc/dbg.h |  36 +--
 drivers/staging/mt7621-mmc/sd.c  | 180 ---
 2 files changed, 121 insertions(+), 95 deletions(-)

-- 
2.17.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v4 1/4] staging: mt7621-mmc: Fix debug macro N_MSG

2018-08-22 Thread Nishad Kamdar 
This patch fixes the debug macro N_MSG. Replaces printk with
dev_ without __func__ or __LINE__ or current->comm and
current->pid. Removes the do {} while(0) loop for the single
statement macro. Issue found by checkpatch.

Signed-off-by: Nishad Kamdar 
---
 drivers/staging/mt7621-mmc/dbg.h | 11 ---
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/mt7621-mmc/dbg.h b/drivers/staging/mt7621-mmc/dbg.h
index 2f2c56b73987..c56fb896617a 100644
--- a/drivers/staging/mt7621-mmc/dbg.h
+++ b/drivers/staging/mt7621-mmc/dbg.h
@@ -104,13 +104,10 @@ do { \
 
 #define N_MSG(evt, fmt, args...)
 /*
-do {\
-if ((DBG_EVT_##evt) & sd_debug_zone[host->id]) { \
-printk(KERN_ERR TAG"%d -> "fmt" <- %s() : L<%d> PID<%s><0x%x>\n", \
-host->id,  ##args , __FUNCTION__, __LINE__, current->comm, 
current->pid);  \
-} \
-} while(0)
-*/
+ *if ((DBG_EVT_##evt) & sd_debug_zone[host->id]) { \
+ *dev_err(mmc_dev(host->mmc), "%d -> " fmt "\n", host->id, ##args) \
+ *}
+ */
 
 #define ERR_MSG(fmt, args...) \
 do { \
-- 
2.17.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v4 2/4] staging: mt7621-mmc: Fix debug macro ERR_MSG and its usages

2018-08-22 Thread Nishad Kamdar 
Replace all usages of ERR_MSG with with dev_ without __func__
or __LINE__ or current->comm and current->pid. Remove the do {}
while(0) loop for the single statement macro. Drop ERR_MSG from dbg.h.
Issue found by checkpatch.

Signed-off-by: Nishad Kamdar 
---
 drivers/staging/mt7621-mmc/dbg.h |   6 --
 drivers/staging/mt7621-mmc/sd.c  | 128 ++-
 2 files changed, 90 insertions(+), 44 deletions(-)

diff --git a/drivers/staging/mt7621-mmc/dbg.h b/drivers/staging/mt7621-mmc/dbg.h
index c56fb896617a..71295df59ed0 100644
--- a/drivers/staging/mt7621-mmc/dbg.h
+++ b/drivers/staging/mt7621-mmc/dbg.h
@@ -109,12 +109,6 @@ do { \
  *}
  */
 
-#define ERR_MSG(fmt, args...) \
-do { \
-   printk(KERN_ERR TAG"%d -> "fmt" <- %s() : L<%d> PID<%s><0x%x>\n", \
-  host->id,  ##args, __FUNCTION__, __LINE__, current->comm, 
current->pid); \
-} while (0);
-
 #if 1
 //defined CONFIG_MTK_MMC_CD_POLL
 #define INIT_MSG(fmt, args...)
diff --git a/drivers/staging/mt7621-mmc/sd.c b/drivers/staging/mt7621-mmc/sd.c
index 04d23cc7cd4a..6b2c72fc61f2 100644
--- a/drivers/staging/mt7621-mmc/sd.c
+++ b/drivers/staging/mt7621-mmc/sd.c
@@ -466,7 +466,8 @@ static void msdc_set_mclk(struct msdc_host *host, int ddr, 
unsigned int hz)
//u8  clksrc = hw->clk_src;
 
if (!hz) { // set mmc system clock to 0 ?
-   //ERR_MSG("set mclk to 0!!!");
+   //dev_err(mmc_dev(host->mmc), "%d -> set mclk to 0!!!\n",
+   //host->id);
msdc_reset_hw(host);
return;
}
@@ -521,7 +522,7 @@ static void msdc_abort_data(struct msdc_host *host)
 {
struct mmc_command *stop = host->mrq->stop;
 
-   ERR_MSG("Need to Abort.");
+   dev_err(mmc_dev(host->mmc), "%d -> Need to Abort.\n", host->id);
 
msdc_reset_hw(host);
msdc_clr_fifo(host);
@@ -530,7 +531,8 @@ static void msdc_abort_data(struct msdc_host *host)
// need to check FIFO count 0 ?
 
if (stop) {  /* try to stop, but may not success */
-   ERR_MSG("stop when abort CMD<%d>", stop->opcode);
+   dev_err(mmc_dev(host->mmc), "%d -> stop when abort CMD<%d>\n",
+   host->id, stop->opcode);
(void)msdc_do_command(host, stop, 0, CMD_TIMEOUT);
}
 
@@ -688,13 +690,17 @@ static void msdc_pm(pm_message_t state, void *data)
 
} else if (evt == PM_EVENT_RESUME || evt == PM_EVENT_USER_RESUME) {
if (!host->suspend) {
-   //ERR_MSG("warning: already resume");
+   //dev_err(mmc_dev(host->mmc),
+   //"%d -> warning: already resume\n",
+   //host->id);
return;
}
 
/* No PM resume when USR suspend */
if (evt == PM_EVENT_RESUME && host->pm_state.event == 
PM_EVENT_USER_SUSPEND) {
-   ERR_MSG("PM Resume when in USR Suspend");   
/* won't happen. */
+   dev_err(mmc_dev(host->mmc),
+   "%d -> PM Resume when in USR Suspend\n",
+   host->id); /* won't happen. */
return;
}
 
@@ -812,7 +818,9 @@ static unsigned int msdc_command_start(struct msdc_host   
*host,
break;
 
if (time_after(jiffies, tmo)) {
-   ERR_MSG("XXX cmd_busy timeout: before CMD<%d>", 
opcode);
+   dev_err(mmc_dev(host->mmc),
+   "%d -> XXX cmd_busy timeout: before 
CMD<%d>\n",
+   host->id, opcode);
cmd->error = -ETIMEDOUT;
msdc_reset_hw(host);
goto end;
@@ -823,7 +831,9 @@ static unsigned int msdc_command_start(struct msdc_host   
*host,
if (!sdc_is_busy())
break;
if (time_after(jiffies, tmo)) {
-   ERR_MSG("XXX sdc_busy timeout: before CMD<%d>", 
opcode);
+   dev_err(mmc_dev(host->mmc),
+   "%d -> XXX sdc_busy timeout: before 
CMD<%d>\n",
+   host->id, opcode);
cmd->error = -ETIMEDOUT;
msdc_reset_hw(host);
goto end;
@@ -862,7 +872,9 @@ static unsigned int msdc_command_resp(struct msdc_host   
*host,
 
spin_unlock(&host->lock);
if (!wait_for_completion_timeout(&host->cmd_done, 10 * timeout)) {
-   ERR_MSG("XXX CMD<%d> wait_for_completion timeout ARG<0x%.8x>", 
opcode, cmd->arg);
+   dev_err(mmc_dev(host->mmc),
+   "%d -> XXX CMD<%d> wait_for_completion timeout 
ARG

[PATCH v4 3/4] staging: mt7621-mmc: Remove macro INIT_MSG and its usages

2018-08-22 Thread Nishad Kamdar 
Removed all usages of INIT_MSG and dropped it from dbg.h.

Signed-off-by: Nishad Kamdar 
---
 drivers/staging/mt7621-mmc/dbg.h |  7 ---
 drivers/staging/mt7621-mmc/sd.c  | 16 
 2 files changed, 23 deletions(-)

diff --git a/drivers/staging/mt7621-mmc/dbg.h b/drivers/staging/mt7621-mmc/dbg.h
index 71295df59ed0..8d2c16450ef5 100644
--- a/drivers/staging/mt7621-mmc/dbg.h
+++ b/drivers/staging/mt7621-mmc/dbg.h
@@ -111,15 +111,8 @@ do { \
 
 #if 1
 //defined CONFIG_MTK_MMC_CD_POLL
-#define INIT_MSG(fmt, args...)
 #define IRQ_MSG(fmt, args...)
 #else
-#define INIT_MSG(fmt, args...) \
-do { \
-   printk(KERN_ERR TAG"%d -> "fmt" <- %s() : L<%d> PID<%s><0x%x>\n", \
-  host->id,  ##args, __FUNCTION__, __LINE__, current->comm, 
current->pid); \
-} while (0);
-
 /* PID in ISR in not corrent */
 #define IRQ_MSG(fmt, args...) \
 do { \
diff --git a/drivers/staging/mt7621-mmc/sd.c b/drivers/staging/mt7621-mmc/sd.c
index 6b2c72fc61f2..327c1cd7fd04 100644
--- a/drivers/staging/mt7621-mmc/sd.c
+++ b/drivers/staging/mt7621-mmc/sd.c
@@ -187,12 +187,10 @@ static u32 hclks[] = {5000}; /* +/- by chhung */
 //
 #define msdc_vcore_on(host) \
do {\
-   INIT_MSG("[+]VMC ref. count<%d>", ++host->pwr_ref); \
(void)hwPowerOn(MT65XX_POWER_LDO_VMC, VOL_3300, "SD");  \
} while (0)
 #define msdc_vcore_off(host) \
do {\
-   INIT_MSG("[-]VMC ref. count<%d>", --host->pwr_ref); \
(void)hwPowerDown(MT65XX_POWER_LDO_VMC, "SD");  \
} while (0)
 
@@ -439,7 +437,6 @@ static void msdc_select_clksrc(struct msdc_host *host, 
unsigned char clksrc)
u32 val;
 
BUG_ON(clksrc > 3);
-   INIT_MSG("set clock source to <%d>", clksrc);
 
val = readl(host->base + MSDC_CLKSRC_REG);
if (readl(host->base + MSDC_ECO_VER) >= 4) {
@@ -510,10 +507,6 @@ static void msdc_set_mclk(struct msdc_host *host, int ddr, 
unsigned int hz)
host->mclk = hz;
msdc_set_timeout(host, host->timeout_ns, host->timeout_clks); // need?
 
-   INIT_MSG("");
-   INIT_MSG("!!! Set<%dKHz> Source<%dKHz> -> sclk<%dKHz>", hz / 1000, hclk 
/ 1000, sclk / 1000);
-   INIT_MSG("");
-
msdc_irq_restore(flags);
 }
 
@@ -671,12 +664,6 @@ static void msdc_pm(pm_message_t state, void *data)
struct msdc_host *host = (struct msdc_host *)data;
int evt = state.event;
 
-   if (evt == PM_EVENT_USER_RESUME || evt == PM_EVENT_USER_SUSPEND) {
-   INIT_MSG("USR_%s: suspend<%d> power<%d>",
-   evt == PM_EVENT_USER_RESUME ? "EVENT_USER_RESUME" : 
"EVENT_USER_SUSPEND",
-   host->suspend, host->power_mode);
-   }
-
if (evt == PM_EVENT_SUSPEND || evt == PM_EVENT_USER_SUSPEND) {
if (host->suspend) /* already suspend */  /* default 0*/
return;
@@ -1762,7 +1749,6 @@ static void msdc_ops_set_ios(struct mmc_host *mmc, struct 
mmc_ios *ios)
if (host->mclk != ios->clock) {
if (ios->clock > 2500) {
//if (!(host->hw->flags & MSDC_REMOVABLE)) {
-   INIT_MSG("SD data latch edge<%d>", MSDC_SMPL_FALLING);
sdr_set_field(host->base + MSDC_IOCON, MSDC_IOCON_RSPL,
  MSDC_SMPL_FALLING);
sdr_set_field(host->base + MSDC_IOCON, MSDC_IOCON_DSPL,
@@ -1815,7 +1801,6 @@ static int msdc_ops_get_cd(struct mmc_host *mmc)
return 1;
 #else
host->card_inserted = (host->pm_state.event == 
PM_EVENT_USER_RESUME) ? 1 : 0;
-   INIT_MSG("sdio ops_get_cd<%d>", host->card_inserted);
return host->card_inserted;
 #endif
}
@@ -1839,7 +1824,6 @@ static int msdc_ops_get_cd(struct mmc_host *mmc)
present = 0; /* TODO? Check DAT3 pins for card detection */
}
 
-   INIT_MSG("ops_get_cd return<%d>", present);
return present;
 }
 
-- 
2.17.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v4 4/4] staging: mt7621-mmc: Fix debug macro IRQ_MSG and its usages

2018-08-22 Thread Nishad Kamdar 
Replace all usages of IRQ_MSG with with dev_ without __func__
or __LINE__ or current->comm and current->pid. Remove the do {}
while(0) loop for the single statement macro. Drop IRQ_MSG from dbg.h.
Issue found by checkpatch.

Signed-off-by: Nishad Kamdar 
---
 drivers/staging/mt7621-mmc/dbg.h | 12 ---
 drivers/staging/mt7621-mmc/sd.c  | 36 
 2 files changed, 27 insertions(+), 21 deletions(-)

diff --git a/drivers/staging/mt7621-mmc/dbg.h b/drivers/staging/mt7621-mmc/dbg.h
index 8d2c16450ef5..5458dae5dc03 100644
--- a/drivers/staging/mt7621-mmc/dbg.h
+++ b/drivers/staging/mt7621-mmc/dbg.h
@@ -109,18 +109,6 @@ do { \
  *}
  */
 
-#if 1
-//defined CONFIG_MTK_MMC_CD_POLL
-#define IRQ_MSG(fmt, args...)
-#else
-/* PID in ISR in not corrent */
-#define IRQ_MSG(fmt, args...) \
-do { \
-   printk(KERN_ERR TAG"%d -> "fmt" <- %s() : L<%d>\n", \
-  host->id,  ##args, __FUNCTION__, __LINE__);  \
-} while (0);
-#endif
-
 void msdc_debug_proc_init(void);
 
 #if 0 /* --- chhung */
diff --git a/drivers/staging/mt7621-mmc/sd.c b/drivers/staging/mt7621-mmc/sd.c
index 327c1cd7fd04..5cb6bc36e78b 100644
--- a/drivers/staging/mt7621-mmc/sd.c
+++ b/drivers/staging/mt7621-mmc/sd.c
@@ -420,7 +420,9 @@ static void msdc_tasklet_card(struct work_struct *work)
mmc_detect_change(host->mmc, msecs_to_jiffies(20));
}
 
-   IRQ_MSG("card found<%s>", inserted ? "inserted" : "removed");
+   dev_err(mmc_dev(host->mmc),
+   "%d -> card found<%s>\n",
+   host->id, inserted ? "inserted" : "removed");
 #endif
 
spin_unlock(&host->lock);
@@ -1858,14 +1860,17 @@ static irqreturn_t msdc_irq(int irq, void *dev_id)
if (intsts & MSDC_INT_CDSC) {
if (host->mmc->caps & MMC_CAP_NEEDS_POLL)
return IRQ_HANDLED;
-   IRQ_MSG("MSDC_INT_CDSC irq<0x%.8x>", intsts);
+   dev_err(mmc_dev(host->mmc),
+   "%d -> MSDC_INT_CDSC irq<0x%.8x>\n", host->id, intsts);
schedule_delayed_work(&host->card_delaywork, HZ);
/* tuning when plug card ? */
}
 
/* sdio interrupt */
if (intsts & MSDC_INT_SDIOIRQ) {
-   IRQ_MSG("XXX MSDC_INT_SDIOIRQ");  /* seems not sdio irq */
+   dev_err(mmc_dev(host->mmc),
+   "%d -> XXX MSDC_INT_SDIOIRQ\n",
+   host->id); /* seems not sdio irq */
//mmc_signal_sdio_irq(host->mmc);
}
 
@@ -1883,10 +1888,15 @@ static irqreturn_t msdc_irq(int irq, void *dev_id)
msdc_clr_int();
 
if (intsts & MSDC_INT_DATTMO) {
-   IRQ_MSG("XXX CMD<%d> MSDC_INT_DATTMO", 
host->mrq->cmd->opcode);
+   dev_err(mmc_dev(host->mmc),
+   "%d -> XXX CMD<%d> MSDC_INT_DATTMO\n",
+   host->id, host->mrq->cmd->opcode);
data->error = -ETIMEDOUT;
} else if (intsts & MSDC_INT_DATCRCERR) {
-   IRQ_MSG("XXX CMD<%d> MSDC_INT_DATCRCERR, 
SDC_DCRC_STS<0x%x>", host->mrq->cmd->opcode, readl(host->base + SDC_DCRC_STS));
+   dev_err(mmc_dev(host->mmc),
+   "%d -> XXX CMD<%d> MSDC_INT_DATCRCERR, 
SDC_DCRC_STS<0x%x>\n",
+   host->id, host->mrq->cmd->opcode,
+   readl(host->base + SDC_DCRC_STS);
data->error = -EIO;
}
 
@@ -1919,15 +1929,23 @@ static irqreturn_t msdc_irq(int irq, void *dev_id)
}
} else if ((intsts & MSDC_INT_RSPCRCERR) || (intsts & 
MSDC_INT_ACMDCRCERR)) {
if (intsts & MSDC_INT_ACMDCRCERR)
-   IRQ_MSG("XXX CMD<%d> MSDC_INT_ACMDCRCERR", 
cmd->opcode);
+   dev_err(mmc_dev(host->mmc),
+   "%d -> XXX CMD<%d> 
MSDC_INT_ACMDCRCERR\n",
+   host->id, cmd->opcode);
else
-   IRQ_MSG("XXX CMD<%d> MSDC_INT_RSPCRCERR", 
cmd->opcode);
+   dev_err(mmc_dev(host->mmc),
+   "%d -> XXX CMD<%d> 
MSDC_INT_RSPCRCERR\n",
+   host->id, cmd->opcode);
cmd->error = -EIO;
} else if ((intsts & MSDC_INT_CMDTMO) || (intsts & 
MSDC_INT_ACMDTMO)) {
if (intsts & MSDC_INT_ACMDTMO)
-   IRQ_MSG("XXX CMD<%d> MSDC_INT_ACMDTMO", 
cmd->opcode);
+   dev_err(mmc_dev(host->mmc),
+   "%d -> XXX CMD<%d> MSDC_INT_ACMDTMO\n",
+

Re: [PATCH] staging: rtl8188eu: Fix spelling mistake

2018-08-22 Thread Bhaskar Singh 
On Wed, Aug 22, 2018 at 11:16:36AM +0300, Dan Carpenter wrote:
> On Tue, Aug 21, 2018 at 07:14:28AM +0530, Bhaskar Singh wrote:
> > This patch fix spelling mistakes in TODO.
> > 
> 
> Btw, it helps when you say which word you're changing, otherwise it
> takes a while to spot the difference.  We changed "HGz" to "GHz".
> 
> Probably someone smarter than I am would have spotted it faster...
> 
> regards,
> dan carpenter
>

Apologies for the inconvenience and thanks a lot for your suggestion.

I will definitely follow that.

Should I send the patch again?

Thanks
Bhaskar Singh

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v4 1/4] staging: mt7621-mmc: Fix debug macro N_MSG

2018-08-22 Thread Dan Carpenter 
On Wed, Aug 22, 2018 at 02:04:55PM +0530, Nishad Kamdar wrote:
> This patch fixes the debug macro N_MSG. Replaces printk with
> dev_ without __func__ or __LINE__ or current->comm and
> current->pid. Removes the do {} while(0) loop for the single
> statement macro. Issue found by checkpatch.
> 
> Signed-off-by: Nishad Kamdar 
> ---
>  drivers/staging/mt7621-mmc/dbg.h | 11 ---
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/staging/mt7621-mmc/dbg.h 
> b/drivers/staging/mt7621-mmc/dbg.h
> index 2f2c56b73987..c56fb896617a 100644
> --- a/drivers/staging/mt7621-mmc/dbg.h
> +++ b/drivers/staging/mt7621-mmc/dbg.h
> @@ -104,13 +104,10 @@ do { \
>  
>  #define N_MSG(evt, fmt, args...)
>  /*
> -do {\
> -if ((DBG_EVT_##evt) & sd_debug_zone[host->id]) { \
> -printk(KERN_ERR TAG"%d -> "fmt" <- %s() : L<%d> PID<%s><0x%x>\n", \
> -host->id,  ##args , __FUNCTION__, __LINE__, current->comm, 
> current->pid);\
> -} \
> -} while(0)
> -*/
> + *if ((DBG_EVT_##evt) & sd_debug_zone[host->id]) { \
> + *dev_err(mmc_dev(host->mmc), "%d -> " fmt "\n", host->id, ##args) \
> + *}
> + */

I don't understand what you're trying to do here.  You just commented
out the macro and turned it into a no-op.  That's not what the patch
description says.

To me the original code seems fine.

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v4 2/4] staging: mt7621-mmc: Fix debug macro ERR_MSG and its usages

2018-08-22 Thread Dan Carpenter 
On Wed, Aug 22, 2018 at 02:13:07PM +0530, Nishad Kamdar wrote:
> diff --git a/drivers/staging/mt7621-mmc/sd.c b/drivers/staging/mt7621-mmc/sd.c
> index 04d23cc7cd4a..6b2c72fc61f2 100644
> --- a/drivers/staging/mt7621-mmc/sd.c
> +++ b/drivers/staging/mt7621-mmc/sd.c
> @@ -466,7 +466,8 @@ static void msdc_set_mclk(struct msdc_host *host, int 
> ddr, unsigned int hz)
>   //u8  clksrc = hw->clk_src;
>  
>   if (!hz) { // set mmc system clock to 0 ?
> - //ERR_MSG("set mclk to 0!!!");
> + //dev_err(mmc_dev(host->mmc), "%d -> set mclk to 0!!!\n",
> + //host->id);

Just delete commented out code.

>   msdc_reset_hw(host);
>   return;
>   }

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: rtl8188eu: Fix spelling mistake

2018-08-22 Thread Dan Carpenter 
On Wed, Aug 22, 2018 at 02:35:32PM +0530, Bhaskar Singh wrote:
> On Wed, Aug 22, 2018 at 11:16:36AM +0300, Dan Carpenter wrote:
> > On Tue, Aug 21, 2018 at 07:14:28AM +0530, Bhaskar Singh wrote:
> > > This patch fix spelling mistakes in TODO.
> > > 
> > 
> > Btw, it helps when you say which word you're changing, otherwise it
> > takes a while to spot the difference.  We changed "HGz" to "GHz".
> > 
> > Probably someone smarter than I am would have spotted it faster...
> > 
> > regards,
> > dan carpenter
> >
> 
> Apologies for the inconvenience and thanks a lot for your suggestion.
> 
> I will definitely follow that.
> 
> Should I send the patch again?

No.  It's fine.

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v4 1/4] staging: mt7621-mmc: Fix debug macro N_MSG

2018-08-22 Thread Nishad Kamdar 
On Wed, Aug 22, 2018 at 12:09:36PM +0300, Dan Carpenter wrote:
> On Wed, Aug 22, 2018 at 02:04:55PM +0530, Nishad Kamdar wrote:
> > This patch fixes the debug macro N_MSG. Replaces printk with
> > dev_ without __func__ or __LINE__ or current->comm and
> > current->pid. Removes the do {} while(0) loop for the single
> > statement macro. Issue found by checkpatch.
> > 
> > Signed-off-by: Nishad Kamdar 
> > ---
> >  drivers/staging/mt7621-mmc/dbg.h | 11 ---
> >  1 file changed, 4 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/staging/mt7621-mmc/dbg.h 
> > b/drivers/staging/mt7621-mmc/dbg.h
> > index 2f2c56b73987..c56fb896617a 100644
> > --- a/drivers/staging/mt7621-mmc/dbg.h
> > +++ b/drivers/staging/mt7621-mmc/dbg.h
> > @@ -104,13 +104,10 @@ do { \
> >  
> >  #define N_MSG(evt, fmt, args...)
> >  /*
> > -do {\
> > -if ((DBG_EVT_##evt) & sd_debug_zone[host->id]) { \
> > -printk(KERN_ERR TAG"%d -> "fmt" <- %s() : L<%d> PID<%s><0x%x>\n", \
> > -host->id,  ##args , __FUNCTION__, __LINE__, current->comm, 
> > current->pid);  \
> > -} \
> > -} while(0)
> > -*/
> > + *if ((DBG_EVT_##evt) & sd_debug_zone[host->id]) { \
> > + *dev_err(mmc_dev(host->mmc), "%d -> " fmt "\n", host->id, ##args) \
> > + *}
> > + */
> 
> I don't understand what you're trying to do here.  You just commented
> out the macro and turned it into a no-op.  That's not what the patch
> description says.
> 
> To me the original code seems fine.
> 
> regards,
> dan carpenter
>

Ok. But the macro definition was already commented.
I just changed printk to dev_err and gave it a better
block comment format.
Should I leave the older version as it is ?

Thanks for the review.

regards,
nishad
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v4 2/4] staging: mt7621-mmc: Fix debug macro ERR_MSG and its usages

2018-08-22 Thread Nishad Kamdar 
On Wed, Aug 22, 2018 at 12:13:42PM +0300, Dan Carpenter wrote:
> On Wed, Aug 22, 2018 at 02:13:07PM +0530, Nishad Kamdar wrote:
> > diff --git a/drivers/staging/mt7621-mmc/sd.c 
> > b/drivers/staging/mt7621-mmc/sd.c
> > index 04d23cc7cd4a..6b2c72fc61f2 100644
> > --- a/drivers/staging/mt7621-mmc/sd.c
> > +++ b/drivers/staging/mt7621-mmc/sd.c
> > @@ -466,7 +466,8 @@ static void msdc_set_mclk(struct msdc_host *host, int 
> > ddr, unsigned int hz)
> > //u8  clksrc = hw->clk_src;
> >  
> > if (!hz) { // set mmc system clock to 0 ?
> > -   //ERR_MSG("set mclk to 0!!!");
> > +   //dev_err(mmc_dev(host->mmc), "%d -> set mclk to 0!!!\n",
> > +   //host->id);
> 
> Just delete commented out code.
> 
> > msdc_reset_hw(host);
> > return;
> > }
> 
> regards,
> dan carpenter
> 

Ok, I'll do that.

Thanks for the review.

regards,
nishad
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v4 1/4] staging: mt7621-mmc: Fix debug macro N_MSG

2018-08-22 Thread Greg Kroah-Hartman 
On Wed, Aug 22, 2018 at 04:40:56PM +0530, Nishad Kamdar wrote:
> On Wed, Aug 22, 2018 at 12:09:36PM +0300, Dan Carpenter wrote:
> > On Wed, Aug 22, 2018 at 02:04:55PM +0530, Nishad Kamdar wrote:
> > > This patch fixes the debug macro N_MSG. Replaces printk with
> > > dev_ without __func__ or __LINE__ or current->comm and
> > > current->pid. Removes the do {} while(0) loop for the single
> > > statement macro. Issue found by checkpatch.
> > > 
> > > Signed-off-by: Nishad Kamdar 
> > > ---
> > >  drivers/staging/mt7621-mmc/dbg.h | 11 ---
> > >  1 file changed, 4 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/drivers/staging/mt7621-mmc/dbg.h 
> > > b/drivers/staging/mt7621-mmc/dbg.h
> > > index 2f2c56b73987..c56fb896617a 100644
> > > --- a/drivers/staging/mt7621-mmc/dbg.h
> > > +++ b/drivers/staging/mt7621-mmc/dbg.h
> > > @@ -104,13 +104,10 @@ do { \
> > >  
> > >  #define N_MSG(evt, fmt, args...)
> > >  /*
> > > -do {\
> > > -if ((DBG_EVT_##evt) & sd_debug_zone[host->id]) { \
> > > -printk(KERN_ERR TAG"%d -> "fmt" <- %s() : L<%d> 
> > > PID<%s><0x%x>\n", \
> > > -host->id,  ##args , __FUNCTION__, __LINE__, current->comm, 
> > > current->pid);\
> > > -} \
> > > -} while(0)
> > > -*/
> > > + *if ((DBG_EVT_##evt) & sd_debug_zone[host->id]) { \
> > > + *dev_err(mmc_dev(host->mmc), "%d -> " fmt "\n", host->id, ##args) \
> > > + *}
> > > + */
> > 
> > I don't understand what you're trying to do here.  You just commented
> > out the macro and turned it into a no-op.  That's not what the patch
> > description says.
> > 
> > To me the original code seems fine.
> > 
> > regards,
> > dan carpenter
> >
> 
> Ok. But the macro definition was already commented.
> I just changed printk to dev_err and gave it a better
> block comment format.
> Should I leave the older version as it is ?

No, delete the whole thing if it is not being used, don't "convert" code
that is commented out.

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v4 1/4] staging: mt7621-mmc: Fix debug macro N_MSG

2018-08-22 Thread Dan Carpenter 
On Wed, Aug 22, 2018 at 04:40:56PM +0530, Nishad Kamdar wrote:
> On Wed, Aug 22, 2018 at 12:09:36PM +0300, Dan Carpenter wrote:
> > On Wed, Aug 22, 2018 at 02:04:55PM +0530, Nishad Kamdar wrote:
> > > This patch fixes the debug macro N_MSG. Replaces printk with
> > > dev_ without __func__ or __LINE__ or current->comm and
> > > current->pid. Removes the do {} while(0) loop for the single
> > > statement macro. Issue found by checkpatch.
> > > 
> > > Signed-off-by: Nishad Kamdar 
> > > ---
> > >  drivers/staging/mt7621-mmc/dbg.h | 11 ---
> > >  1 file changed, 4 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/drivers/staging/mt7621-mmc/dbg.h 
> > > b/drivers/staging/mt7621-mmc/dbg.h
> > > index 2f2c56b73987..c56fb896617a 100644
> > > --- a/drivers/staging/mt7621-mmc/dbg.h
> > > +++ b/drivers/staging/mt7621-mmc/dbg.h
> > > @@ -104,13 +104,10 @@ do { \
> > >  
> > >  #define N_MSG(evt, fmt, args...)
> > >  /*
> > > -do {\
> > > -if ((DBG_EVT_##evt) & sd_debug_zone[host->id]) { \
> > > -printk(KERN_ERR TAG"%d -> "fmt" <- %s() : L<%d> 
> > > PID<%s><0x%x>\n", \
> > > -host->id,  ##args , __FUNCTION__, __LINE__, current->comm, 
> > > current->pid);\
> > > -} \
> > > -} while(0)
> > > -*/
> > > + *if ((DBG_EVT_##evt) & sd_debug_zone[host->id]) { \
> > > + *dev_err(mmc_dev(host->mmc), "%d -> " fmt "\n", host->id, ##args) \
> > > + *}
> > > + */
> > 
> > I don't understand what you're trying to do here.  You just commented
> > out the macro and turned it into a no-op.  That's not what the patch
> > description says.
> > 
> > To me the original code seems fine.
> > 
> > regards,
> > dan carpenter
> >
> 
> Ok. But the macro definition was already commented.
> I just changed printk to dev_err and gave it a better
> block comment format.
> Should I leave the older version as it is ?

Oh...  Yeah.  You're right.  Sorry about that.

We normally just delete dead code without even thinking hard about it.
It turns out that if we delete all the N_MSG() code, it's actually quite
a bit.  But probably it's the right thing to do, instead of hanging on
to it like a hoarder.

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v7 0/8] Cedrus driver for the Allwinner Video Engine, using media requests

2018-08-22 Thread Ezequiel Garcia 
Hey Paul,

On Thu, 2018-08-09 at 11:04 +0200, Paul Kocialkowski wrote:
> This is the seventh iteration of the updated Cedrus driver,
> that supports the Video Engine found on most Allwinner SoCs, starting
> with the A10. It was tested on the A13, A20, A33 and H3.
> 
> The initial version of this driver[0] was originally written and
> submitted by Florent Revest using a previous version of the request API
> that is necessary to provide coherency between controls and the buffers
> they apply to.
> 
> The driver was adapted to use the latest version of the media request
> API[1], as submitted by Hans Verkuil. Media request API support is a
> hard requirement for the Cedrus driver.
> 
> The driver itself currently only supports MPEG2 and more codecs will be
> added eventually. The default output frame format provided by the Video
> Engine is a multi-planar tiled YUV format (based on NV12). A specific
> format is introduced in the V4L2 API to describe it. Starting with the
> A33, the Video Engine can also output untiled YUV formats.
> 
> This implementation is based on the significant work that was conducted
> by various members of the linux-sunxi community for understanding and
> documenting the Video Engine's innards.
> 
> In addition to the media requests API, the following series are required
> for Cedrus:
> * vicodec: the Virtual Codec driver
> * allwinner: a64: add SRAM controller / system control
> * SRAM patches from the Cedrus VPU driver series version 5
> 
> Changes since v6:
> * Reworked MPEG2 controls to stick closer to the bitstream;
> * Updated controls documentation accordingly and added requested fixes;
> * Renamed tiled format to V4L2_PIX_FMT_SUNXI_TILED_NV12;
> * Added various minor driver fixes based on Hans' feedback;
> * Fixed dst frame alignment based on Jernej's feedback and tests;
> * Removed set bits for the disabled secondary output.
> 
> Changes since v5:
> * Added MPEG2 quantization matrices definitions and support;
> * Cleaned up registers definitions;
> * Moved the driver to staging as requested;
> 

I tried to find the reason for moving this driver to staging,
but couldn't find it in the discussion.

If there's a legitimate reason, shouldn't you add a TODO file?

Thanks,
Eze
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 01/23] mtd: rawnand: plat_nand: Pass a nand_chip object to all platform_nand_ctrl hooks

2018-08-22 Thread Robert Jarzmik 
Boris Brezillon  writes:

> Let's make the raw NAND API consistent by patching all helpers and
> hooks to take a nand_chip object instead of an mtd_info one or
> remove the mtd_info object when both are passed.
>
> In order to do that, we first need to update the platform_nand_ctrl
> hooks to take a nand_chip object instead of an mtd_info.
>
> We had temporary plat_nand_xxx() wrappers to the do the mtd -> chip
> conversion, but those will be dropped when doing the patching nand_chip
> hooks to take a nand_chip object.
>
> Signed-off-by: Boris Brezillon 
For mach-pxa:
Acked-by: Robert Jarzmik 

Cheers.

--
Robert
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 01/23] mtd: rawnand: plat_nand: Pass a nand_chip object to all platform_nand_ctrl hooks

2018-08-22 Thread Boris Brezillon 
On Fri, 17 Aug 2018 18:09:00 +0200
Boris Brezillon  wrote:

> Let's make the raw NAND API consistent by patching all helpers and
> hooks to take a nand_chip object instead of an mtd_info one or
> remove the mtd_info object when both are passed.
> 
> In order to do that, we first need to update the platform_nand_ctrl
> hooks to take a nand_chip object instead of an mtd_info.
> 
> We had temporary plat_nand_xxx() wrappers to the do the mtd -> chip

 ^ add

> conversion, but those will be dropped when doing the patching nand_chip

 ^ s/doing the//

> hooks to take a nand_chip object.

Will fix those typos in v2.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] hv_netvsc: Fix a deadlock by getting rtnl_lock earlier in netvsc_probe()

2018-08-22 Thread Dexuan Cui 


This patch fixes the race between netvsc_probe() and
rndis_set_subchannel(), which can cause a deadlock.

Fixes: 8195b1396ec8 ("hv_netvsc: fix deadlock on hotplug")
Signed-off-by: Dexuan Cui 
Cc: Stephen Hemminger 
Cc: K. Y. Srinivasan 
Cc: Haiyang Zhang 
---
 drivers/net/hyperv/netvsc_drv.c | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)


FYI: these are the related 3 paths which show the deadlock:

#1:
Workqueue: hv_vmbus_con vmbus_onmessage_work [hv_vmbus]
Call Trace:
 schedule
 schedule_preempt_disabled
 __mutex_lock
 __device_attach
 bus_probe_device
 device_add
 vmbus_device_register
 vmbus_onoffer
 vmbus_onmessage_work
 process_one_work
 worker_thread
 kthread
 ret_from_fork

#2:
schedule
 schedule_preempt_disabled
 __mutex_lock
 netvsc_probe
 vmbus_probe
 really_probe
 __driver_attach
 bus_for_each_dev
 driver_attach_async
 async_run_entry_fn
 process_one_work
 worker_thread
 kthread
 ret_from_fork

#3:
Workqueue: events netvsc_subchan_work [hv_netvsc]
Call Trace:
 schedule
 rndis_set_subchannel
 netvsc_subchan_work
 process_one_work
 worker_thread
 kthread
 ret_from_fork

Before path #1 finishes, path #2 can start to run, because just before
the "bus_probe_device(dev);" in device_add() in path #1, there is a line
"object_uevent(&dev->kobj, KOBJ_ADD);", so systemd-udevd can
immediately try to load hv_netvsc and hence path #2 can start to run.

diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 1121a1ec..4fd14a0 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -2206,6 +2206,16 @@ static int netvsc_probe(struct hv_device *dev,
 
memcpy(net->dev_addr, device_info.mac_adr, ETH_ALEN);
 
+   /* We must get rtnl_lock before scheduling nvdev->subchan_work,
+* otherwise netvsc_subchan_work() can get rtnl_lock first and wait
+* all subchannels to show up, but that may not happen because
+* netvsc_probe() can't get rtnl_lock and as a result vmbus_onoffer()
+* -> ... -> device_add() -> ... -> __device_attach() can't get
+* the device lock, so all the subchannels can't be processed --
+* finally netvsc_subchan_work() hangs for ever.
+*/
+   rtnl_lock();
+
if (nvdev->num_chn > 1)
schedule_work(&nvdev->subchan_work);
 
@@ -2224,7 +2234,6 @@ static int netvsc_probe(struct hv_device *dev,
else
net->max_mtu = ETH_DATA_LEN;
 
-   rtnl_lock();
ret = register_netdevice(net);
if (ret != 0) {
pr_err("Unable to register netdev.\n");
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: KASAN: null-ptr-deref Write in binder_update_page_range

2018-08-22 Thread Minchan Kim 
Hi,

On Wed, Aug 22, 2018 at 03:07:04PM +0900, Dae R. Jeong wrote:
> Reporting the crash: KASAN: null-ptr-deref Write in binder_update_page_range
> 
> This crash has been found in v4.18-rc3 using RaceFuzzer (a modified
> version of Syzkaller), which we describe more at the end of this
> report.
> 
> Our analysis shows that the race occurs when invoking two syscalls
> concurrently, mmap$binder() and ioctl$BINDER_WRITE_READ. More
> specifically, since two code lines `alloc->vma = vma;` and
> `alloc->vma_vm_mm = vma->vm_mm;` in binder_alloc_mmap_handler() is not
> an atomic operation during mmap$binder() syscall, there is a time
> window that `alloc->vma` is assigned but `alloc->vma_vm_mm` isn't
> assigned. It causes the null pointer dereference in
> binder_alloc_new_buf_locked() since it checks whether `alloc->vma` is
> NULL, but it doesn't check that `alloc->vma_vm_mm` is NULL. More
> details on the thread interleaving and the crash log are follows.
> 
> 
> Thread interleaving:
> CPU0 (binder_alloc_mmap_handler)  CPU1 
> (binder_alloc_new_buf_locked)
> = =
> // drivers/android/binder_alloc.c
> // #L718 (v4.18-rc3)
> alloc->vma = vma;
>   // 
> drivers/android/binder_alloc.c
>   // #L346 (v4.18-rc3)
>   if (alloc->vma == NULL) {
>   ...
>   // alloc->vma is not NULL 
> at this point
>   return ERR_PTR(-ESRCH);
>   }
>   ...
>   // #L438
>   binder_update_page_range(alloc, 
> 0,
>   (void 
> *)PAGE_ALIGN((uintptr_t)buffer->data),
>   end_page_addr);
> 
>   // In 
> binder_update_page_range() #L218
>   // But still alloc->vma_vm_mm 
> is NULL here
>   if (need_mm && 
> mmget_not_zero(alloc->vma_vm_mm))
> alloc->vma_vm_mm = vma->vm_mm;
> 
> 
> Crash Log:
> ==
> BUG: KASAN: null-ptr-deref in __atomic_add_unless 
> include/asm-generic/atomic-instrumented.h:89 [inline]
> BUG: KASAN: null-ptr-deref in atomic_add_unless include/linux/atomic.h:533 
> [inline]
> BUG: KASAN: null-ptr-deref in mmget_not_zero include/linux/sched/mm.h:75 
> [inline]
> BUG: KASAN: null-ptr-deref in binder_update_page_range+0xece/0x18e0 
> drivers/android/binder_alloc.c:218
> Write of size 4 at addr 0058 by task syz-executor0/11184
> 
> CPU: 1 PID: 11184 Comm: syz-executor0 Not tainted 4.18.0-rc3 #1
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
> rel-1.8.2-0-g33fbe13 by qemu-project.org 04/01/2014
> Call Trace:
>  __dump_stack lib/dump_stack.c:77 [inline]
>  dump_stack+0x16e/0x22c lib/dump_stack.c:113
>  kasan_report_error mm/kasan/report.c:352 [inline]
>  kasan_report+0x163/0x380 mm/kasan/report.c:412
>  check_memory_region_inline mm/kasan/kasan.c:260 [inline]
>  check_memory_region+0x140/0x1a0 mm/kasan/kasan.c:267
>  kasan_check_write+0x14/0x20 mm/kasan/kasan.c:278
>  __atomic_add_unless include/asm-generic/atomic-instrumented.h:89 [inline]
>  atomic_add_unless include/linux/atomic.h:533 [inline]
>  mmget_not_zero include/linux/sched/mm.h:75 [inline]
>  binder_update_page_range+0xece/0x18e0 drivers/android/binder_alloc.c:218
>  binder_alloc_new_buf_locked drivers/android/binder_alloc.c:443 [inline]
>  binder_alloc_new_buf+0x467/0xc30 drivers/android/binder_alloc.c:513
>  binder_transaction+0x125b/0x4fb0 drivers/android/binder.c:2957
>  binder_thread_write+0xc08/0x2770 drivers/android/binder.c:3528
>  binder_ioctl_write_read.isra.39+0x24f/0x8e0 drivers/android/binder.c:4456
>  binder_ioctl+0xa86/0xf34 drivers/android/binder.c:4596
>  vfs_ioctl fs/ioctl.c:46 [inline]
>  do_vfs_ioctl+0x154/0xd40 fs/ioctl.c:686
>  ksys_ioctl+0x94/0xb0 fs/ioctl.c:701
>  __do_sys_ioctl fs/ioctl.c:708 [inline]
>  __se_sys_ioctl fs/ioctl.c:706 [inline]
>  __x64_sys_ioctl+0x43/0x50 fs/ioctl.c:706
>  do_syscall_64+0x167/0x4b0 arch/x86/entry/common.c:290
>  entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0033:0x456469
> Code: 1d ba fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7 48 
> 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 
> 83 eb b9 fb ff c3 66 2e 0f 1f 84 00 00 00 00
> RSP: 002b:7f575f268b28 EFLAGS: 0246 ORIG_RAX: 0010
> RAX: ffda RBX: 0072bfa0 RCX: 00456469
> RDX: 23c0 RSI: c0306201 RDI: 0016
> RBP: 000