[PATCH-v2 0/3] mmc: sdhci-pxav3: Fix tabbing issue

[PATCH-v2 0/3] mmc: sdhci-pxav3: Fix tabbing issue

[Vaibhav Hiremath]

Trivial patch-series, which fixes the tabbing issue in the driver,
uses the BIT macro for bit fields and prints notice on -EPROBE_DEFER
in sdhci_add_host() function on regulator unavailability.

V1 => V2
========
 - Fixed all comments from Joe, mostly alignment changes
 - Separated BIT macro usage patch into new one.
 - changed error to kernel notice for EPROBE_DEFER in sdhci_add_host()

Note: This patch-series should get merged before another series -

[PATCH-v2 0/7] mmc: sdhci-pxav3: Enable support for PXA1928 SDCHI controller

Vaibhav Hiremath (3):
  mmc: sdhci-pxav3: Fix tabbing issue
  mmc: sdhci-pxav3: Use BIT macro for bit field definitions
  mmc: sdhci: print notice on -EPROBE_DEFER in sdhci_add_host() fn

 drivers/mmc/host/sdhci-pxav3.c | 46 +++++++++++++++++++++---------------------
 drivers/mmc/host/sdhci.c       |  5 ++++-
 2 files changed, 27 insertions(+), 24 deletions(-)

-- 
1.9.1

[PATCH-v2 1/3] mmc: sdhci-pxav3: Fix tabbing issue

[Vaibhav Hiremath]

There were some coding style issues where spaces have been used instead
of tabs, for example, in macro definitions, alignment of function
declarations/definitions, etc...

This patch fixes all such occurrences in the code.

Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
---
 drivers/mmc/host/sdhci-pxav3.c | 46 +++++++++++++++++++++---------------------
 1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/drivers/mmc/host/sdhci-pxav3.c b/drivers/mmc/host/sdhci-pxav3.c
index 946d37f..7a07177 100644
--- a/drivers/mmc/host/sdhci-pxav3.c
+++ b/drivers/mmc/host/sdhci-pxav3.c
@@ -39,24 +39,24 @@
 #include "sdhci.h"
 #include "sdhci-pltfm.h"
 
-#define PXAV3_RPM_DELAY_MS     50
+#define PXAV3_RPM_DELAY_MS		50
 
-#define SD_CLOCK_BURST_SIZE_SETUP		0x10A
-#define SDCLK_SEL	0x100
-#define SDCLK_DELAY_SHIFT	9
-#define SDCLK_DELAY_MASK	0x1f
+#define SD_CLOCK_BURST_SIZE_SETUP	0x10A
+#define SDCLK_SEL			0x100
+#define  SDCLK_DELAY_SHIFT		9
+#define  SDCLK_DELAY_MASK		0x1f
 
-#define SD_CFG_FIFO_PARAM       0x100
-#define SDCFG_GEN_PAD_CLK_ON	(1<<6)
-#define SDCFG_GEN_PAD_CLK_CNT_MASK	0xFF
-#define SDCFG_GEN_PAD_CLK_CNT_SHIFT	24
+#define SD_CFG_FIFO_PARAM		0x100
+#define  SDCFG_GEN_PAD_CLK_ON		(1<<6)
+#define  SDCFG_GEN_PAD_CLK_CNT_MASK	0xFF
+#define  SDCFG_GEN_PAD_CLK_CNT_SHIFT	24
 
-#define SD_SPI_MODE          0x108
-#define SD_CE_ATA_1          0x10C
+#define SD_SPI_MODE			0x108
+#define SD_CE_ATA_1			0x10C
 
-#define SD_CE_ATA_2          0x10E
-#define SDCE_MISC_INT		(1<<2)
-#define SDCE_MISC_INT_EN	(1<<1)
+#define SD_CE_ATA_2			0x10E
+#define  SDCE_MISC_INT			(1<<2)
+#define  SDCE_MISC_INT_EN		(1<<1)
 
 struct sdhci_pxa {
 	struct clk *clk_core;
@@ -284,7 +284,7 @@ static void pxav3_set_uhs_signaling(struct sdhci_host *host, unsigned int uhs)
 	 * FE-2946959
 	 */
 	if (pxa->sdio3_conf_reg) {
-		u8 reg_val  = readb(pxa->sdio3_conf_reg);
+		u8 reg_val = readb(pxa->sdio3_conf_reg);
 
 		if (uhs == MMC_TIMING_UHS_SDR50 ||
 		    uhs == MMC_TIMING_UHS_DDR50) {
@@ -304,20 +304,20 @@ static void pxav3_set_uhs_signaling(struct sdhci_host *host, unsigned int uhs)
 }
 
 static const struct sdhci_ops pxav3_sdhci_ops = {
-	.set_clock = sdhci_set_clock,
-	.platform_send_init_74_clocks = pxav3_gen_init_74_clocks,
-	.get_max_clock = sdhci_pltfm_clk_get_max_clock,
-	.set_bus_width = sdhci_set_bus_width,
-	.reset = pxav3_reset,
-	.set_uhs_signaling = pxav3_set_uhs_signaling,
+	.set_clock			= sdhci_set_clock,
+	.platform_send_init_74_clocks	= pxav3_gen_init_74_clocks,
+	.get_max_clock			= sdhci_pltfm_clk_get_max_clock,
+	.set_bus_width			= sdhci_set_bus_width,
+	.reset				= pxav3_reset,
+	.set_uhs_signaling		= pxav3_set_uhs_signaling,
 };
 
 static struct sdhci_pltfm_data sdhci_pxav3_pdata = {
-	.quirks = SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK
+	.quirks	= SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK
 		| SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC
 		| SDHCI_QUIRK_32BIT_ADMA_SIZE
 		| SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
-	.ops = &pxav3_sdhci_ops,
+	.ops	= &pxav3_sdhci_ops,
 };
 
 #ifdef CONFIG_OF
-- 
1.9.1

Re: [PATCH-v2 1/3] mmc: sdhci-pxav3: Fix tabbing issue

[Jisheng Zhang]

On Mon, 7 Sep 2015 17:01:09 +0530
Vaibhav Hiremath <vaibhav.hiremath@linaro.org> wrote:

> There were some coding style issues where spaces have been used instead
> of tabs, for example, in macro definitions, alignment of function
> declarations/definitions, etc...
> 
> This patch fixes all such occurrences in the code.
> 
> Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
> ---
>  drivers/mmc/host/sdhci-pxav3.c | 46 +++++++++++++++++++++---------------------
>  1 file changed, 23 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-pxav3.c b/drivers/mmc/host/sdhci-pxav3.c
> index 946d37f..7a07177 100644
> --- a/drivers/mmc/host/sdhci-pxav3.c
> +++ b/drivers/mmc/host/sdhci-pxav3.c
> @@ -39,24 +39,24 @@
>  #include "sdhci.h"
>  #include "sdhci-pltfm.h"
>  
> -#define PXAV3_RPM_DELAY_MS     50
> +#define PXAV3_RPM_DELAY_MS		50
>  
> -#define SD_CLOCK_BURST_SIZE_SETUP		0x10A
> -#define SDCLK_SEL	0x100
> -#define SDCLK_DELAY_SHIFT	9
> -#define SDCLK_DELAY_MASK	0x1f
> +#define SD_CLOCK_BURST_SIZE_SETUP	0x10A
> +#define SDCLK_SEL			0x100
> +#define  SDCLK_DELAY_SHIFT		9
> +#define  SDCLK_DELAY_MASK		0x1f
>  
> -#define SD_CFG_FIFO_PARAM       0x100
> -#define SDCFG_GEN_PAD_CLK_ON	(1<<6)
> -#define SDCFG_GEN_PAD_CLK_CNT_MASK	0xFF
> -#define SDCFG_GEN_PAD_CLK_CNT_SHIFT	24
> +#define SD_CFG_FIFO_PARAM		0x100
> +#define  SDCFG_GEN_PAD_CLK_ON		(1<<6)
> +#define  SDCFG_GEN_PAD_CLK_CNT_MASK	0xFF
> +#define  SDCFG_GEN_PAD_CLK_CNT_SHIFT	24
>  
> -#define SD_SPI_MODE          0x108
> -#define SD_CE_ATA_1          0x10C
> +#define SD_SPI_MODE			0x108
> +#define SD_CE_ATA_1			0x10C
>  
> -#define SD_CE_ATA_2          0x10E
> -#define SDCE_MISC_INT		(1<<2)
> -#define SDCE_MISC_INT_EN	(1<<1)
> +#define SD_CE_ATA_2			0x10E
> +#define  SDCE_MISC_INT			(1<<2)
> +#define  SDCE_MISC_INT_EN		(1<<1)
>  
>  struct sdhci_pxa {
>  	struct clk *clk_core;
> @@ -284,7 +284,7 @@ static void pxav3_set_uhs_signaling(struct sdhci_host *host, unsigned int uhs)
>  	 * FE-2946959
>  	 */
>  	if (pxa->sdio3_conf_reg) {
> -		u8 reg_val  = readb(pxa->sdio3_conf_reg);
> +		u8 reg_val = readb(pxa->sdio3_conf_reg);
>  
>  		if (uhs == MMC_TIMING_UHS_SDR50 ||
>  		    uhs == MMC_TIMING_UHS_DDR50) {
> @@ -304,20 +304,20 @@ static void pxav3_set_uhs_signaling(struct sdhci_host *host, unsigned int uhs)
>  }
>  
>  static const struct sdhci_ops pxav3_sdhci_ops = {
> -	.set_clock = sdhci_set_clock,
> -	.platform_send_init_74_clocks = pxav3_gen_init_74_clocks,
> -	.get_max_clock = sdhci_pltfm_clk_get_max_clock,
> -	.set_bus_width = sdhci_set_bus_width,
> -	.reset = pxav3_reset,
> -	.set_uhs_signaling = pxav3_set_uhs_signaling,
> +	.set_clock			= sdhci_set_clock,
> +	.platform_send_init_74_clocks	= pxav3_gen_init_74_clocks,
> +	.get_max_clock			= sdhci_pltfm_clk_get_max_clock,
> +	.set_bus_width			= sdhci_set_bus_width,
> +	.reset				= pxav3_reset,
> +	.set_uhs_signaling		= pxav3_set_uhs_signaling,

IMHO, this is not an improvement, previous style is fine and I saw
such style in kernel here and there

>  };
>  
>  static struct sdhci_pltfm_data sdhci_pxav3_pdata = {
> -	.quirks = SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK
> +	.quirks	= SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK
>  		| SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC
>  		| SDHCI_QUIRK_32BIT_ADMA_SIZE
>  		| SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
> -	.ops = &pxav3_sdhci_ops,
> +	.ops	= &pxav3_sdhci_ops,

ditto

>  };
>  
>  #ifdef CONFIG_OF

Re: [PATCH-v2 1/3] mmc: sdhci-pxav3: Fix tabbing issue

[Vaibhav Hiremath]

On Monday 07 September 2015 05:26 PM, Jisheng Zhang wrote:
> On Mon, 7 Sep 2015 17:01:09 +0530
> Vaibhav Hiremath <vaibhav.hiremath-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>
>> There were some coding style issues where spaces have been used instead
>> of tabs, for example, in macro definitions, alignment of function
>> declarations/definitions, etc...
>>
>> This patch fixes all such occurrences in the code.
>>
>> Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> ---
>>   drivers/mmc/host/sdhci-pxav3.c | 46 +++++++++++++++++++++---------------------
>>   1 file changed, 23 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci-pxav3.c b/drivers/mmc/host/sdhci-pxav3.c
>> index 946d37f..7a07177 100644
>> --- a/drivers/mmc/host/sdhci-pxav3.c
>> +++ b/drivers/mmc/host/sdhci-pxav3.c
>> @@ -39,24 +39,24 @@
>>   #include "sdhci.h"
>>   #include "sdhci-pltfm.h"
>>
>> -#define PXAV3_RPM_DELAY_MS     50
>> +#define PXAV3_RPM_DELAY_MS		50
>>
>> -#define SD_CLOCK_BURST_SIZE_SETUP		0x10A
>> -#define SDCLK_SEL	0x100
>> -#define SDCLK_DELAY_SHIFT	9
>> -#define SDCLK_DELAY_MASK	0x1f
>> +#define SD_CLOCK_BURST_SIZE_SETUP	0x10A
>> +#define SDCLK_SEL			0x100
>> +#define  SDCLK_DELAY_SHIFT		9
>> +#define  SDCLK_DELAY_MASK		0x1f
>>
>> -#define SD_CFG_FIFO_PARAM       0x100
>> -#define SDCFG_GEN_PAD_CLK_ON	(1<<6)
>> -#define SDCFG_GEN_PAD_CLK_CNT_MASK	0xFF
>> -#define SDCFG_GEN_PAD_CLK_CNT_SHIFT	24
>> +#define SD_CFG_FIFO_PARAM		0x100
>> +#define  SDCFG_GEN_PAD_CLK_ON		(1<<6)
>> +#define  SDCFG_GEN_PAD_CLK_CNT_MASK	0xFF
>> +#define  SDCFG_GEN_PAD_CLK_CNT_SHIFT	24
>>
>> -#define SD_SPI_MODE          0x108
>> -#define SD_CE_ATA_1          0x10C
>> +#define SD_SPI_MODE			0x108
>> +#define SD_CE_ATA_1			0x10C
>>
>> -#define SD_CE_ATA_2          0x10E
>> -#define SDCE_MISC_INT		(1<<2)
>> -#define SDCE_MISC_INT_EN	(1<<1)
>> +#define SD_CE_ATA_2			0x10E
>> +#define  SDCE_MISC_INT			(1<<2)
>> +#define  SDCE_MISC_INT_EN		(1<<1)
>>
>>   struct sdhci_pxa {
>>   	struct clk *clk_core;
>> @@ -284,7 +284,7 @@ static void pxav3_set_uhs_signaling(struct sdhci_host *host, unsigned int uhs)
>>   	 * FE-2946959
>>   	 */
>>   	if (pxa->sdio3_conf_reg) {
>> -		u8 reg_val  = readb(pxa->sdio3_conf_reg);
>> +		u8 reg_val = readb(pxa->sdio3_conf_reg);
>>
>>   		if (uhs == MMC_TIMING_UHS_SDR50 ||
>>   		    uhs == MMC_TIMING_UHS_DDR50) {
>> @@ -304,20 +304,20 @@ static void pxav3_set_uhs_signaling(struct sdhci_host *host, unsigned int uhs)
>>   }
>>
>>   static const struct sdhci_ops pxav3_sdhci_ops = {
>> -	.set_clock = sdhci_set_clock,
>> -	.platform_send_init_74_clocks = pxav3_gen_init_74_clocks,
>> -	.get_max_clock = sdhci_pltfm_clk_get_max_clock,
>> -	.set_bus_width = sdhci_set_bus_width,
>> -	.reset = pxav3_reset,
>> -	.set_uhs_signaling = pxav3_set_uhs_signaling,
>> +	.set_clock			= sdhci_set_clock,
>> +	.platform_send_init_74_clocks	= pxav3_gen_init_74_clocks,
>> +	.get_max_clock			= sdhci_pltfm_clk_get_max_clock,
>> +	.set_bus_width			= sdhci_set_bus_width,
>> +	.reset				= pxav3_reset,
>> +	.set_uhs_signaling		= pxav3_set_uhs_signaling,
>
> IMHO, this is not an improvement, previous style is fine and I saw
> such style in kernel here and there
>

I had received feedback to use above style on different patch-series.
And I also think this looks more clean and readable.

Thanks,
Vaibhav
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH-v2 2/3] mmc: sdhci-pxav3: Use BIT macro for bit field definitions

[Vaibhav Hiremath]

Instead of using shift operation use BIT macro for bit field
definitions.

Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
---
 drivers/mmc/host/sdhci-pxav3.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/sdhci-pxav3.c b/drivers/mmc/host/sdhci-pxav3.c
index 7a07177..a8a8c94 100644
--- a/drivers/mmc/host/sdhci-pxav3.c
+++ b/drivers/mmc/host/sdhci-pxav3.c
@@ -47,7 +47,7 @@
 #define  SDCLK_DELAY_MASK		0x1f
 
 #define SD_CFG_FIFO_PARAM		0x100
-#define  SDCFG_GEN_PAD_CLK_ON		(1<<6)
+#define  SDCFG_GEN_PAD_CLK_ON		BIT(6)
 #define  SDCFG_GEN_PAD_CLK_CNT_MASK	0xFF
 #define  SDCFG_GEN_PAD_CLK_CNT_SHIFT	24
 
@@ -55,8 +55,8 @@
 #define SD_CE_ATA_1			0x10C
 
 #define SD_CE_ATA_2			0x10E
-#define  SDCE_MISC_INT			(1<<2)
-#define  SDCE_MISC_INT_EN		(1<<1)
+#define  SDCE_MISC_INT			BIT(2)
+#define  SDCE_MISC_INT_EN		BIT(1)
 
 struct sdhci_pxa {
 	struct clk *clk_core;
-- 
1.9.1

[PATCH-v2 3/3] mmc: sdhci: print notice on -EPROBE_DEFER in sdhci_add_host() fn

[Vaibhav Hiremath]

This patch adds kernel notice/message to sdhci_host_add() fn on
-EPROBE_DEFER.

Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/mmc/host/sdhci.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 1dbe932..37a5cd5 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -3130,8 +3130,11 @@ int sdhci_add_host(struct sdhci_host *host)
 		mmc->caps |= MMC_CAP_NEEDS_POLL;
 
 	/* If there are external regulators, get them */
-	if (mmc_regulator_get_supply(mmc) == -EPROBE_DEFER)
+	if (mmc_regulator_get_supply(mmc) == -EPROBE_DEFER) {
+		pr_notice("%s: regulator supply unavailable, deferring probe.\n",
+				mmc_hostname(mmc));
 		return -EPROBE_DEFER;
+	}
 
 	/* If vqmmc regulator and no 1.8V signalling, then there's no UHS */
 	if (!IS_ERR(mmc->supply.vqmmc)) {
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH-v2 0/3] mmc: sdhci-pxav3: Fix tabbing issue

[Ulf Hansson]

On 7 September 2015 at 13:31, Vaibhav Hiremath
<vaibhav.hiremath-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> Trivial patch-series, which fixes the tabbing issue in the driver,
> uses the BIT macro for bit fields and prints notice on -EPROBE_DEFER
> in sdhci_add_host() function on regulator unavailability.
>
> V1 => V2
> ========
>  - Fixed all comments from Joe, mostly alignment changes
>  - Separated BIT macro usage patch into new one.
>  - changed error to kernel notice for EPROBE_DEFER in sdhci_add_host()
>
> Note: This patch-series should get merged before another series -
>
> [PATCH-v2 0/7] mmc: sdhci-pxav3: Enable support for PXA1928 SDCHI controller
>
> Vaibhav Hiremath (3):
>   mmc: sdhci-pxav3: Fix tabbing issue
>   mmc: sdhci-pxav3: Use BIT macro for bit field definitions
>   mmc: sdhci: print notice on -EPROBE_DEFER in sdhci_add_host() fn
>
>  drivers/mmc/host/sdhci-pxav3.c | 46 +++++++++++++++++++++---------------------
>  drivers/mmc/host/sdhci.c       |  5 ++++-
>  2 files changed, 27 insertions(+), 24 deletions(-)
>
> --
> 1.9.1
>

Hi Vaibhav,

FYI, I won't be picking up any of these patches. Primarily because I
don't think they improves the code and other people also seems to
agree to that.

Regarding patch1 and similar patches which deals with only fixing
checkpatch warnings/errors. In most cases I don't like such changes,
as they makes it harder to use "git blame" when you want to find out
which commit that introduced a change.

Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH-v2 0/3] mmc: sdhci-pxav3: Fix tabbing issue

[Vaibhav Hiremath]

On Tuesday 15 September 2015 05:58 PM, Ulf Hansson wrote:
> On 7 September 2015 at 13:31, Vaibhav Hiremath
> <vaibhav.hiremath@linaro.org> wrote:
>> Trivial patch-series, which fixes the tabbing issue in the driver,
>> uses the BIT macro for bit fields and prints notice on -EPROBE_DEFER
>> in sdhci_add_host() function on regulator unavailability.
>>
>> V1 => V2
>> ========
>>   - Fixed all comments from Joe, mostly alignment changes
>>   - Separated BIT macro usage patch into new one.
>>   - changed error to kernel notice for EPROBE_DEFER in sdhci_add_host()
>>
>> Note: This patch-series should get merged before another series -
>>
>> [PATCH-v2 0/7] mmc: sdhci-pxav3: Enable support for PXA1928 SDCHI controller
>>
>> Vaibhav Hiremath (3):
>>    mmc: sdhci-pxav3: Fix tabbing issue
>>    mmc: sdhci-pxav3: Use BIT macro for bit field definitions
>>    mmc: sdhci: print notice on -EPROBE_DEFER in sdhci_add_host() fn
>>
>>   drivers/mmc/host/sdhci-pxav3.c | 46 +++++++++++++++++++++---------------------
>>   drivers/mmc/host/sdhci.c       |  5 ++++-
>>   2 files changed, 27 insertions(+), 24 deletions(-)
>>
>> --
>> 1.9.1
>>
>
> Hi Vaibhav,
>
> FYI, I won't be picking up any of these patches. Primarily because I
> don't think they improves the code and other people also seems to
> agree to that.
>
> Regarding patch1 and similar patches which deals with only fixing
> checkpatch warnings/errors. In most cases I don't like such changes,
> as they makes it harder to use "git blame" when you want to find out
> which commit that introduced a change.
>


Honestly, I also agree with you.
But recently I have been told to do these changed to get my
main/feature changes accepted :)

No issues, I am OK to drop them.
These are just beautification changes and as you rightly
said it makes 'git blame' difficult to find original change commit.


Thanks,
Vaibhav