[PATCH v2] staging: mt7621-mmc: Fix single statement macros in sd.c

[PATCH v2] staging: mt7621-mmc: Fix single statement macros in sd.c

[Nishad Kamdar]

This patch fixes a few single statement macros in sd.c.
It converts two macros to inline functions. It removes
five other macros and replaces their usages with calls to
the function being called in the macro definition.
Issue found by checkpatch.

Signed-off-by: Nishad Kamdar <nishadkamdar@gmail.com>
---
Changes in v2:
  - Convert msdc_gate_clk() and msdc_ungate_clk() to inline functions.
  - Delete msdc_irq_restore(), msdc_vcore_on(), msdc_vcore_off(),
    msdc_vdd_on() and msdc_vdd_off() and replace their usages directly
    with calls to the function being called by these macros.
---
 drivers/staging/mt7621-mmc/sd.c | 61 +++++++--------------------------
 1 file changed, 13 insertions(+), 48 deletions(-)

diff --git a/drivers/staging/mt7621-mmc/sd.c b/drivers/staging/mt7621-mmc/sd.c
index 7474f9ed7b5b..c36884e33a50 100644
--- a/drivers/staging/mt7621-mmc/sd.c
+++ b/drivers/staging/mt7621-mmc/sd.c
@@ -103,15 +103,15 @@ static int cd_active_low = 1;
 #if 0 /* --- by chhung */
 /* gate means clock power down */
 static int g_clk_gate = 0;
-#define msdc_gate_clock(id) \
-	do {					       \
-		g_clk_gate &= ~(1 << ((id) + PERI_MSDC0_PDN));	\
-	} while (0)
+static inline void msdc_gate_clock(int id)
+{
+	g_clk_gate &= ~(1 << ((id) + PERI_MSDC0_PDN));
+}
 /* not like power down register. 1 means clock on. */
-#define msdc_ungate_clock(id) \
-	do {					    \
-		g_clk_gate |= 1 << ((id) + PERI_MSDC0_PDN);	\
-	} while (0)
+static inline void msdc_ungate_clock(int id)
+{
+	g_clk_gate |= 1 << ((id) + PERI_MSDC0_PDN);
+}
 
 // do we need sync object or not
 void msdc_clk_status(int *status)
@@ -169,11 +169,6 @@ static void msdc_clr_fifo(struct msdc_host *host)
 		sdr_clr_bits(host->base + MSDC_INTEN, val);	\
 	} while (0)
 
-#define msdc_irq_restore(val) \
-	do {					\
-		sdr_set_bits(host->base + MSDC_INTEN, val);	\
-	} while (0)
-
 /* clock source for host: global */
 #if defined(CONFIG_SOC_MT7620)
 static u32 hclks[] = {48000000}; /* +/- by chhung */
@@ -181,32 +176,6 @@ static u32 hclks[] = {48000000}; /* +/- by chhung */
 static u32 hclks[] = {50000000}; /* +/- by chhung */
 #endif
 
-//============================================
-// the power for msdc host controller: global
-//    always keep the VMC on.
-//============================================
-#define msdc_vcore_on(host) \
-	do {								\
-		(void)hwPowerOn(MT65XX_POWER_LDO_VMC, VOL_3300, "SD");	\
-	} while (0)
-#define msdc_vcore_off(host) \
-	do {								\
-		(void)hwPowerDown(MT65XX_POWER_LDO_VMC, "SD");		\
-	} while (0)
-
-//====================================
-// the vdd output for card: global
-//   always keep the VMCH on.
-//====================================
-#define msdc_vdd_on(host) \
-	do {								\
-		(void)hwPowerOn(MT65XX_POWER_LDO_VMCH, VOL_3300, "SD"); \
-	} while (0)
-#define msdc_vdd_off(host) \
-	do {							\
-		(void)hwPowerDown(MT65XX_POWER_LDO_VMCH, "SD"); \
-	} while (0)
-
 #define sdc_is_busy()          (readl(host->base + SDC_STS) & SDC_STS_SDCBUSY)
 #define sdc_is_cmd_busy()      (readl(host->base + SDC_STS) & SDC_STS_CMDBUSY)
 
@@ -364,7 +333,7 @@ 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?
 
-	msdc_irq_restore(flags);
+	sdr_set_bits(host->base + MSDC_INTEN, flags);
 }
 
 /* Fix me. when need to abort */
@@ -453,11 +422,11 @@ void msdc_pin_reset(struct msdc_host *host, int mode)
 static void msdc_core_power(struct msdc_host *host, int on)
 {
 	if (on && host->core_power == 0) {
-		msdc_vcore_on(host);
+		(void)hwPowerOn(MT65XX_POWER_LDO_VMC, VOL_3300, "SD");
 		host->core_power = 1;
 		msleep(1);
 	} else if (!on && host->core_power == 1) {
-		msdc_vcore_off(host);
+		(void)hwPowerDown(MT65XX_POWER_LDO_VMC, "SD");
 		host->core_power = 0;
 		msleep(1);
 	}
@@ -478,10 +447,8 @@ static void msdc_card_power(struct msdc_host *host, int on)
 {
 	if (on) {
 		msdc_pin_config(host, MSDC_PIN_PULL_UP);
-		//msdc_vdd_on(host);  // need todo card detection.
 		msleep(1);
 	} else {
-		//msdc_vdd_off(host);
 		msdc_pin_config(host, MSDC_PIN_PULL_DOWN);
 		msleep(1);
 	}
@@ -1750,7 +1717,6 @@ static void msdc_enable_cd_irq(struct msdc_host *host, int enable)
 		 * shouldn't be turned off. Here adds a reference count to keep
 		 * the core power alive.
 		 */
-		//msdc_vcore_on(host); //did in msdc_init_hw()
 
 		if (hw->config_gpio_pin) /* NULL */
 			hw->config_gpio_pin(MSDC_CD_PIN, GPIO_PULL_UP);
@@ -1773,7 +1739,6 @@ static void msdc_enable_cd_irq(struct msdc_host *host, int enable)
 		/* Here decreases a reference count to core power since card
 		 * detection circuit is shutdown.
 		 */
-		//msdc_vcore_off(host);
 	}
 }
 
@@ -1783,11 +1748,11 @@ static void msdc_init_hw(struct msdc_host *host)
 
 	/* Power on */
 #if 0 /* --- by chhung */
-	msdc_vcore_on(host);
+	(void)hwPowerOn(MT65XX_POWER_LDO_VMC, VOL_3300, "SD");
 	msdc_pin_reset(host, MSDC_PIN_PULL_UP);
 	msdc_select_clksrc(host, hw->clk_src);
 	enable_clock(PERI_MSDC0_PDN + host->id, "SD");
-	msdc_vdd_on(host);
+	(void)hwPowerOn(MT65XX_POWER_LDO_VMCH, VOL_3300, "SD");
 #endif /* end of --- */
 	/* Configure to MMC/SD mode */
 	sdr_set_field(host->base + MSDC_CFG, MSDC_CFG_MODE, MSDC_SDMMC);
-- 
2.17.1

Re: [PATCH v2] staging: mt7621-mmc: Fix single statement macros in sd.c

[Joe Perches]

On Sun, 2018-09-23 at 15:08 +0530, Nishad Kamdar wrote:
> This patch fixes a few single statement macros in sd.c.
> It converts two macros to inline functions. It removes
> five other macros and replaces their usages with calls to
> the function being called in the macro definition.
> Issue found by checkpatch.
> 
> Signed-off-by: Nishad Kamdar <nishadkamdar@gmail.com>
> ---
> Changes in v2:
>   - Convert msdc_gate_clk() and msdc_ungate_clk() to inline functions.
>   - Delete msdc_irq_restore(), msdc_vcore_on(), msdc_vcore_off(),
>     msdc_vdd_on() and msdc_vdd_off() and replace their usages directly
>     with calls to the function being called by these macros.

Nishad, do please look again for uses of these functions
you are changing.

Please try removing all the #if 0 blocks instead, and then
see if there are also now unused functions from those removed
blocks that could also be removed.

And Greg, if you look at this, look at the odd license of
these files.

It's possible the license is incompatible with the GPL.

Re: [PATCH v2] staging: mt7621-mmc: Fix single statement macros in sd.c

[Nishad Kamdar]

On Sun, Sep 23, 2018 at 06:31:32AM -0700, Joe Perches wrote:
> On Sun, 2018-09-23 at 15:08 +0530, Nishad Kamdar wrote:
> > This patch fixes a few single statement macros in sd.c.
> > It converts two macros to inline functions. It removes
> > five other macros and replaces their usages with calls to
> > the function being called in the macro definition.
> > Issue found by checkpatch.
> > 
> > Signed-off-by: Nishad Kamdar <nishadkamdar@gmail.com>
> > ---
> > Changes in v2:
> >   - Convert msdc_gate_clk() and msdc_ungate_clk() to inline functions.
> >   - Delete msdc_irq_restore(), msdc_vcore_on(), msdc_vcore_off(),
> >     msdc_vdd_on() and msdc_vdd_off() and replace their usages directly
> >     with calls to the function being called by these macros.
> 
> Nishad, do please look again for uses of these functions
> you are changing.
> 
> Please try removing all the #if 0 blocks instead, and then
> see if there are also now unused functions from those removed
> blocks that could also be removed.
> 
> And Greg, if you look at this, look at the odd license of
> these files.
> 
> It's possible the license is incompatible with the GPL.
> 
Ok, I'll look into that.

thanks for the review.

regards,
nishad

Re: [PATCH v2] staging: mt7621-mmc: Fix single statement macros in sd.c

[Greg Kroah-Hartman]

On Sun, Sep 23, 2018 at 06:31:32AM -0700, Joe Perches wrote:
> On Sun, 2018-09-23 at 15:08 +0530, Nishad Kamdar wrote:
> > This patch fixes a few single statement macros in sd.c.
> > It converts two macros to inline functions. It removes
> > five other macros and replaces their usages with calls to
> > the function being called in the macro definition.
> > Issue found by checkpatch.
> > 
> > Signed-off-by: Nishad Kamdar <nishadkamdar@gmail.com>
> > ---
> > Changes in v2:
> >   - Convert msdc_gate_clk() and msdc_ungate_clk() to inline functions.
> >   - Delete msdc_irq_restore(), msdc_vcore_on(), msdc_vcore_off(),
> >     msdc_vdd_on() and msdc_vdd_off() and replace their usages directly
> >     with calls to the function being called by these macros.
> 
> Nishad, do please look again for uses of these functions
> you are changing.
> 
> Please try removing all the #if 0 blocks instead, and then
> see if there are also now unused functions from those removed
> blocks that could also be removed.
> 
> And Greg, if you look at this, look at the odd license of
> these files.
> 
> It's possible the license is incompatible with the GPL.

It is odd, but the GPL at the bottom of the file kind of implies it is
ok.  Given that it was published by mtk, I am assuming all is good, but
it would be a good idea to check with the authors to fix this up
properly.

thanks,

greg k-h

Re: [PATCH v2] staging: mt7621-mmc: Fix single statement macros in sd.c

[Joe Perches]

On Tue, 2018-09-25 at 20:49 +0200, Greg Kroah-Hartman wrote:
> On Sun, Sep 23, 2018 at 06:31:32AM -0700, Joe Perches wrote:
> > On Sun, 2018-09-23 at 15:08 +0530, Nishad Kamdar wrote:
> > > This patch fixes a few single statement macros in sd.c.
> > > It converts two macros to inline functions. It removes
> > > five other macros and replaces their usages with calls to
> > > the function being called in the macro definition.
> > > Issue found by checkpatch.
> > > 
> > > Signed-off-by: Nishad Kamdar <nishadkamdar@gmail.com>
> > > ---
> > > Changes in v2:
> > >   - Convert msdc_gate_clk() and msdc_ungate_clk() to inline functions.
> > >   - Delete msdc_irq_restore(), msdc_vcore_on(), msdc_vcore_off(),
> > >     msdc_vdd_on() and msdc_vdd_off() and replace their usages directly
> > >     with calls to the function being called by these macros.
> > 
> > Nishad, do please look again for uses of these functions
> > you are changing.
> > 
> > Please try removing all the #if 0 blocks instead, and then
> > see if there are also now unused functions from those removed
> > blocks that could also be removed.
> > 
> > And Greg, if you look at this, look at the odd license of
> > these files.
> > 
> > It's possible the license is incompatible with the GPL.
> 
> It is odd, but the GPL at the bottom of the file kind of implies it is
> ok.

Implications are not licenses.

> Given that it was published by mtk, I am assuming all is good, but
> it would be a good idea to check with the authors to fix this up
> properly.

The initial patch was submitted by
John Crispin <blogic@openwrt.org>

I do not know John's relationship to mediatek.

commit 8b634a9c7620b15691322cd53071122d2ab249a7
Author: John Crispin <blogic@openwrt.org>
Date:   Thu Mar 15 07:22:35 2018 +1100

John?  Any idea of the providence of these files?

I do not see anything like it in
https://git.openwrt.org/?p=openwrt/openwrt.git;a=tree;f=target/linux/mediatek;h=ad50ab7e407372a482aafb4183c4e49e25f93739;hb=refs/tags/v18.06.1

Re: [PATCH v2] staging: mt7621-mmc: Fix single statement macros in sd.c

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 2863 bytes --]

On Tue, Sep 25 2018, Joe Perches wrote:

> On Tue, 2018-09-25 at 20:49 +0200, Greg Kroah-Hartman wrote:
>> On Sun, Sep 23, 2018 at 06:31:32AM -0700, Joe Perches wrote:
>> > On Sun, 2018-09-23 at 15:08 +0530, Nishad Kamdar wrote:
>> > > This patch fixes a few single statement macros in sd.c.
>> > > It converts two macros to inline functions. It removes
>> > > five other macros and replaces their usages with calls to
>> > > the function being called in the macro definition.
>> > > Issue found by checkpatch.
>> > > 
>> > > Signed-off-by: Nishad Kamdar <nishadkamdar@gmail.com>
>> > > ---
>> > > Changes in v2:
>> > >   - Convert msdc_gate_clk() and msdc_ungate_clk() to inline functions.
>> > >   - Delete msdc_irq_restore(), msdc_vcore_on(), msdc_vcore_off(),
>> > >     msdc_vdd_on() and msdc_vdd_off() and replace their usages directly
>> > >     with calls to the function being called by these macros.
>> > 
>> > Nishad, do please look again for uses of these functions
>> > you are changing.
>> > 
>> > Please try removing all the #if 0 blocks instead, and then
>> > see if there are also now unused functions from those removed
>> > blocks that could also be removed.
>> > 
>> > And Greg, if you look at this, look at the odd license of
>> > these files.
>> > 
>> > It's possible the license is incompatible with the GPL.
>> 
>> It is odd, but the GPL at the bottom of the file kind of implies it is
>> ok.
>
> Implications are not licenses.
>
>> Given that it was published by mtk, I am assuming all is good, but
>> it would be a good idea to check with the authors to fix this up
>> properly.
>
> The initial patch was submitted by
> John Crispin <blogic@openwrt.org>

Actually it was submitted by me. I extracted it from libreCMC (I think -
it is in most of the various openWRT-like distros).
In libreCMC the patch has John listed as the Author, and I thought it
right to preserve that.

I believe the code comes from a code-dump made by Mediatek several years
ago.  It can be found at git://github.com/mqmaker/linux.git.
The code there contains (in drivers/mmc/host/mtk-mmc/sd.c) both the
copyright statement and the
 MODULE_LICENSE("GPL");
declaration.
I took this declaration as sufficient evidence that Mediatek
intentionally released it under the GPL.  The fact that the whole
code dump contains the GPL COPYING file is extra evidence.

NeilBrown


>
> I do not know John's relationship to mediatek.
>
> commit 8b634a9c7620b15691322cd53071122d2ab249a7
> Author: John Crispin <blogic@openwrt.org>
> Date:   Thu Mar 15 07:22:35 2018 +1100
>
> John?  Any idea of the providence of these files?
>
> I do not see anything like it in
> https://git.openwrt.org/?p=openwrt/openwrt.git;a=tree;f=target/linux/mediatek;h=ad50ab7e407372a482aafb4183c4e49e25f93739;hb=refs/tags/v18.06.1

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]