[PATCH 2/9] mmc: meson-gx: minor improvements in meson_mmc_set_ios

[PATCH 2/9] mmc: meson-gx: minor improvements in meson_mmc_set_ios

[Heiner Kallweit]

val isn't used in the switch clause and afterwards there's an
identical statement. So remove it.

In case of an unexpected bus width the error message indicates
the intention to set the bus width to 4 and to go on.
So remove the return statement. This return statement also
conflicts with "setting to 4" because nothing would be set
actually before returning.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index da3cce31..38edc60d 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -379,7 +379,6 @@ static void meson_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	meson_mmc_clk_set(host, ios->clock);
 
 	/* Bus width */
-	val = readl(host->regs + SD_EMMC_CFG);
 	switch (ios->bus_width) {
 	case MMC_BUS_WIDTH_1:
 		bus_width = CFG_BUS_WIDTH_1;
@@ -394,7 +393,6 @@ static void meson_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 		dev_err(host->dev, "Invalid ios->bus_width: %u.  Setting to 4.\n",
 			ios->bus_width);
 		bus_width = CFG_BUS_WIDTH_4;
-		return;
 	}
 
 	val = readl(host->regs + SD_EMMC_CFG);
-- 
2.11.0

Re: [PATCH 2/9] mmc: meson-gx: minor improvements in meson_mmc_set_ios

[Jaehoon Chung]

On 02/01/2017 05:57 AM, Heiner Kallweit wrote:
> val isn't used in the switch clause and afterwards there's an
> identical statement. So remove it.
> 
> In case of an unexpected bus width the error message indicates
> the intention to set the bus width to 4 and to go on.
> So remove the return statement. This return statement also
> conflicts with "setting to 4" because nothing would be set
> actually before returning.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  drivers/mmc/host/meson-gx-mmc.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> index da3cce31..38edc60d 100644
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -379,7 +379,6 @@ static void meson_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>  	meson_mmc_clk_set(host, ios->clock);
>  
>  	/* Bus width */
> -	val = readl(host->regs + SD_EMMC_CFG);
>  	switch (ios->bus_width) {
>  	case MMC_BUS_WIDTH_1:
>  		bus_width = CFG_BUS_WIDTH_1;
> @@ -394,7 +393,6 @@ static void meson_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>  		dev_err(host->dev, "Invalid ios->bus_width: %u.  Setting to 4.\n",
>  			ios->bus_width);
>  		bus_width = CFG_BUS_WIDTH_4;
> -		return;

It's a different question..why does meson-gx-mmc use 4bit buswidth for invalid bus-with? not using 1bit-buswidth?

Best Regards,
Jaehoon Chung

>  	}
>  
>  	val = readl(host->regs + SD_EMMC_CFG);
> 

Re: [PATCH 2/9] mmc: meson-gx: minor improvements in meson_mmc_set_ios

[Heiner Kallweit]

Am 01.02.2017 um 12:55 schrieb Jaehoon Chung:
> On 02/01/2017 05:57 AM, Heiner Kallweit wrote:
>> val isn't used in the switch clause and afterwards there's an
>> identical statement. So remove it.
>>
>> In case of an unexpected bus width the error message indicates
>> the intention to set the bus width to 4 and to go on.
>> So remove the return statement. This return statement also
>> conflicts with "setting to 4" because nothing would be set
>> actually before returning.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>>  drivers/mmc/host/meson-gx-mmc.c | 2 --
>>  1 file changed, 2 deletions(-)
>>
>> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
>> index da3cce31..38edc60d 100644
>> --- a/drivers/mmc/host/meson-gx-mmc.c
>> +++ b/drivers/mmc/host/meson-gx-mmc.c
>> @@ -379,7 +379,6 @@ static void meson_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>>  	meson_mmc_clk_set(host, ios->clock);
>>  
>>  	/* Bus width */
>> -	val = readl(host->regs + SD_EMMC_CFG);
>>  	switch (ios->bus_width) {
>>  	case MMC_BUS_WIDTH_1:
>>  		bus_width = CFG_BUS_WIDTH_1;
>> @@ -394,7 +393,6 @@ static void meson_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>>  		dev_err(host->dev, "Invalid ios->bus_width: %u.  Setting to 4.\n",
>>  			ios->bus_width);
>>  		bus_width = CFG_BUS_WIDTH_4;
>> -		return;
> 
> It's a different question..why does meson-gx-mmc use 4bit buswidth for invalid bus-with? not using 1bit-buswidth?
> 
I think this question is best addressed to Kevin as original author.

> Best Regards,
> Jaehoon Chung
> 
>>  	}
>>  
>>  	val = readl(host->regs + SD_EMMC_CFG);
>>
> 
> 

Re: [PATCH 2/9] mmc: meson-gx: minor improvements in meson_mmc_set_ios

[Kevin Hilman]

Heiner Kallweit <hkallweit1@gmail.com> writes:

> Am 01.02.2017 um 12:55 schrieb Jaehoon Chung:
>> On 02/01/2017 05:57 AM, Heiner Kallweit wrote:
>>> val isn't used in the switch clause and afterwards there's an
>>> identical statement. So remove it.
>>>
>>> In case of an unexpected bus width the error message indicates
>>> the intention to set the bus width to 4 and to go on.
>>> So remove the return statement. This return statement also
>>> conflicts with "setting to 4" because nothing would be set
>>> actually before returning.
>>>
>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>>> ---
>>>  drivers/mmc/host/meson-gx-mmc.c | 2 --
>>>  1 file changed, 2 deletions(-)
>>>
>>> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
>>> index da3cce31..38edc60d 100644
>>> --- a/drivers/mmc/host/meson-gx-mmc.c
>>> +++ b/drivers/mmc/host/meson-gx-mmc.c
>>> @@ -379,7 +379,6 @@ static void meson_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>>>  	meson_mmc_clk_set(host, ios->clock);
>>>  
>>>  	/* Bus width */
>>> -	val = readl(host->regs + SD_EMMC_CFG);
>>>  	switch (ios->bus_width) {
>>>  	case MMC_BUS_WIDTH_1:
>>>  		bus_width = CFG_BUS_WIDTH_1;
>>> @@ -394,7 +393,6 @@ static void meson_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>>>  		dev_err(host->dev, "Invalid ios->bus_width: %u.  Setting to 4.\n",
>>>  			ios->bus_width);
>>>  		bus_width = CFG_BUS_WIDTH_4;
>>> -		return;
>> 
>> It's a different question..why does meson-gx-mmc use 4bit buswidth for invalid bus-with? not using 1bit-buswidth?
>> 
> I think this question is best addressed to Kevin as original author.
>

Because that was the fall-back in the vendor driver that this was based
on.

I'm not really an MMC expert, so I don't know if that's the right thing
to do.

Kevin

[PATCH 3/9] mmc: meson-gx: improve meson_mmc_clk_set

[Heiner Kallweit]

The following changes are quite small, therefore I combined them in
one patch.

- ret doesn't need to be initialized with 0
- use standard !clk_rate notation to check for a zero value
- If clk_rate is zero we return here. Therefore all further checks
  in this function for clk_rate != 0 are not needed.
- switch from dev_warn to dev_err if the clock can't be set
- If due to clock source and available divider values the requested
  frequency isn't matched exactly (always the case if requested
  frequency is 52 MHz), then just print the differing values as
  debug message and not as warning.
- Also remove ret from the message as it is always 0.
- In the case of actual frequency not exactly matching the requested
  one set mmc->actual_clock to the requested frequency.
  So far mmc->actual_clock wasn't set at all in this case.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 36 +++++++++++++++++++-----------------
 1 file changed, 19 insertions(+), 17 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 38edc60d..529a4f22 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -179,7 +179,7 @@ struct sd_emmc_desc {
 static int meson_mmc_clk_set(struct meson_host *host, unsigned long clk_rate)
 {
 	struct mmc_host *mmc = host->mmc;
-	int ret = 0;
+	int ret;
 	u32 cfg;
 
 	if (clk_rate) {
@@ -202,29 +202,31 @@ static int meson_mmc_clk_set(struct meson_host *host, unsigned long clk_rate)
 	dev_dbg(host->dev, "change clock rate %u -> %lu\n",
 		mmc->actual_clock, clk_rate);
 
-	if (clk_rate == 0) {
+	if (!clk_rate) {
 		mmc->actual_clock = 0;
 		return 0;
 	}
 
 	ret = clk_set_rate(host->cfg_div_clk, clk_rate);
-	if (ret)
-		dev_warn(host->dev, "Unable to set cfg_div_clk to %lu. ret=%d\n",
-			 clk_rate, ret);
-	else if (clk_rate && clk_rate != clk_get_rate(host->cfg_div_clk))
-		dev_warn(host->dev, "divider requested rate %lu != actual rate %lu: ret=%d\n",
-			 clk_rate, clk_get_rate(host->cfg_div_clk), ret);
-	else
-		mmc->actual_clock = clk_rate;
-
-	/* (re)start clock, if non-zero */
-	if (!ret && clk_rate) {
-		cfg = readl(host->regs + SD_EMMC_CFG);
-		cfg &= ~CFG_STOP_CLOCK;
-		writel(cfg, host->regs + SD_EMMC_CFG);
+	if (ret) {
+		dev_err(host->dev, "Unable to set cfg_div_clk to %lu. ret=%d\n",
+			clk_rate, ret);
+		return ret;
 	}
 
-	return ret;
+	if (clk_rate != clk_get_rate(host->cfg_div_clk))
+		dev_dbg(host->dev,
+			"divider requested rate %lu != actual rate %lu\n",
+			clk_rate, clk_get_rate(host->cfg_div_clk));
+
+	mmc->actual_clock = clk_rate;
+
+	/* (re)start clock */
+	cfg = readl(host->regs + SD_EMMC_CFG);
+	cfg &= ~CFG_STOP_CLOCK;
+	writel(cfg, host->regs + SD_EMMC_CFG);
+
+	return 0;
 }
 
 /*
-- 
2.11.0

Re: [PATCH 3/9] mmc: meson-gx: improve meson_mmc_clk_set

[Jaehoon Chung]

On 02/01/2017 05:58 AM, Heiner Kallweit wrote:
> The following changes are quite small, therefore I combined them in
> one patch.
> 
> - ret doesn't need to be initialized with 0
> - use standard !clk_rate notation to check for a zero value
> - If clk_rate is zero we return here. Therefore all further checks
>   in this function for clk_rate != 0 are not needed.
> - switch from dev_warn to dev_err if the clock can't be set
> - If due to clock source and available divider values the requested
>   frequency isn't matched exactly (always the case if requested
>   frequency is 52 MHz), then just print the differing values as
>   debug message and not as warning.
> - Also remove ret from the message as it is always 0.
> - In the case of actual frequency not exactly matching the requested
>   one set mmc->actual_clock to the requested frequency.
>   So far mmc->actual_clock wasn't set at all in this case.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  drivers/mmc/host/meson-gx-mmc.c | 36 +++++++++++++++++++-----------------
>  1 file changed, 19 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> index 38edc60d..529a4f22 100644
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -179,7 +179,7 @@ struct sd_emmc_desc {
>  static int meson_mmc_clk_set(struct meson_host *host, unsigned long clk_rate)
>  {
>  	struct mmc_host *mmc = host->mmc;
> -	int ret = 0;
> +	int ret;
>  	u32 cfg;
>  
>  	if (clk_rate) {
> @@ -202,29 +202,31 @@ static int meson_mmc_clk_set(struct meson_host *host, unsigned long clk_rate)
>  	dev_dbg(host->dev, "change clock rate %u -> %lu\n",
>  		mmc->actual_clock, clk_rate);
>  
> -	if (clk_rate == 0) {
> +	if (!clk_rate) {
>  		mmc->actual_clock = 0;
>  		return 0;
>  	}
>  
>  	ret = clk_set_rate(host->cfg_div_clk, clk_rate);
> -	if (ret)
> -		dev_warn(host->dev, "Unable to set cfg_div_clk to %lu. ret=%d\n",
> -			 clk_rate, ret);
> -	else if (clk_rate && clk_rate != clk_get_rate(host->cfg_div_clk))
> -		dev_warn(host->dev, "divider requested rate %lu != actual rate %lu: ret=%d\n",
> -			 clk_rate, clk_get_rate(host->cfg_div_clk), ret);
> -	else
> -		mmc->actual_clock = clk_rate;
> -
> -	/* (re)start clock, if non-zero */
> -	if (!ret && clk_rate) {
> -		cfg = readl(host->regs + SD_EMMC_CFG);
> -		cfg &= ~CFG_STOP_CLOCK;
> -		writel(cfg, host->regs + SD_EMMC_CFG);
> +	if (ret) {
> +		dev_err(host->dev, "Unable to set cfg_div_clk to %lu. ret=%d\n",
> +			clk_rate, ret);
> +		return ret;
>  	}
>  
> -	return ret;
> +	if (clk_rate != clk_get_rate(host->cfg_div_clk))
> +		dev_dbg(host->dev,
> +			"divider requested rate %lu != actual rate %lu\n",
> +			clk_rate, clk_get_rate(host->cfg_div_clk));
> +
> +	mmc->actual_clock = clk_rate;

debug message is described actual_rate as clk_get_rate()..
What is correct "actual_clock"?

Best Regards,
Jaehoon Chung

> +
> +	/* (re)start clock */
> +	cfg = readl(host->regs + SD_EMMC_CFG);
> +	cfg &= ~CFG_STOP_CLOCK;
> +	writel(cfg, host->regs + SD_EMMC_CFG);
> +
> +	return 0;
>  }
>  
>  /*
> 

Re: [PATCH 3/9] mmc: meson-gx: improve meson_mmc_clk_set

[Heiner Kallweit]

Am 01.02.2017 um 13:24 schrieb Jaehoon Chung:
> On 02/01/2017 05:58 AM, Heiner Kallweit wrote:
>> The following changes are quite small, therefore I combined them in
>> one patch.
>>
>> - ret doesn't need to be initialized with 0
>> - use standard !clk_rate notation to check for a zero value
>> - If clk_rate is zero we return here. Therefore all further checks
>>   in this function for clk_rate != 0 are not needed.
>> - switch from dev_warn to dev_err if the clock can't be set
>> - If due to clock source and available divider values the requested
>>   frequency isn't matched exactly (always the case if requested
>>   frequency is 52 MHz), then just print the differing values as
>>   debug message and not as warning.
>> - Also remove ret from the message as it is always 0.
>> - In the case of actual frequency not exactly matching the requested
>>   one set mmc->actual_clock to the requested frequency.
>>   So far mmc->actual_clock wasn't set at all in this case.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>>  drivers/mmc/host/meson-gx-mmc.c | 36 +++++++++++++++++++-----------------
>>  1 file changed, 19 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
>> index 38edc60d..529a4f22 100644
>> --- a/drivers/mmc/host/meson-gx-mmc.c
>> +++ b/drivers/mmc/host/meson-gx-mmc.c
>> @@ -179,7 +179,7 @@ struct sd_emmc_desc {
>>  static int meson_mmc_clk_set(struct meson_host *host, unsigned long clk_rate)
>>  {
>>  	struct mmc_host *mmc = host->mmc;
>> -	int ret = 0;
>> +	int ret;
>>  	u32 cfg;
>>  
>>  	if (clk_rate) {
>> @@ -202,29 +202,31 @@ static int meson_mmc_clk_set(struct meson_host *host, unsigned long clk_rate)
>>  	dev_dbg(host->dev, "change clock rate %u -> %lu\n",
>>  		mmc->actual_clock, clk_rate);
>>  
>> -	if (clk_rate == 0) {
>> +	if (!clk_rate) {
>>  		mmc->actual_clock = 0;
>>  		return 0;
>>  	}
>>  
>>  	ret = clk_set_rate(host->cfg_div_clk, clk_rate);
>> -	if (ret)
>> -		dev_warn(host->dev, "Unable to set cfg_div_clk to %lu. ret=%d\n",
>> -			 clk_rate, ret);
>> -	else if (clk_rate && clk_rate != clk_get_rate(host->cfg_div_clk))
>> -		dev_warn(host->dev, "divider requested rate %lu != actual rate %lu: ret=%d\n",
>> -			 clk_rate, clk_get_rate(host->cfg_div_clk), ret);
>> -	else
>> -		mmc->actual_clock = clk_rate;
>> -
>> -	/* (re)start clock, if non-zero */
>> -	if (!ret && clk_rate) {
>> -		cfg = readl(host->regs + SD_EMMC_CFG);
>> -		cfg &= ~CFG_STOP_CLOCK;
>> -		writel(cfg, host->regs + SD_EMMC_CFG);
>> +	if (ret) {
>> +		dev_err(host->dev, "Unable to set cfg_div_clk to %lu. ret=%d\n",
>> +			clk_rate, ret);
>> +		return ret;
>>  	}
>>  
>> -	return ret;
>> +	if (clk_rate != clk_get_rate(host->cfg_div_clk))
>> +		dev_dbg(host->dev,
>> +			"divider requested rate %lu != actual rate %lu\n",
>> +			clk_rate, clk_get_rate(host->cfg_div_clk));
>> +
>> +	mmc->actual_clock = clk_rate;
> 
> debug message is described actual_rate as clk_get_rate()..
> What is correct "actual_clock"?
> 
Indeed this might be a little misleading.
"actual_clock" is a member of struct mmc_host and for now we have to live
with this name. However the meaning in our context is: "currently requested
clock rate".
What we call "actual rate" in the message is the effective clock rate and
might differ from the requested one, depending on available parent clocks
and divider values.

Regards, Heiner

> Best Regards,
> Jaehoon Chung
> 
>> +
>> +	/* (re)start clock */
>> +	cfg = readl(host->regs + SD_EMMC_CFG);
>> +	cfg &= ~CFG_STOP_CLOCK;
>> +	writel(cfg, host->regs + SD_EMMC_CFG);
>> +
>> +	return 0;
>>  }
>>  
>>  /*
>>
> 
> 

Re: [PATCH 3/9] mmc: meson-gx: improve meson_mmc_clk_set

[Ulf Hansson]

[...]

>>> +    if (clk_rate != clk_get_rate(host->cfg_div_clk))
>>> +            dev_dbg(host->dev,
>>> +                    "divider requested rate %lu != actual rate %lu\n",
>>> +                    clk_rate, clk_get_rate(host->cfg_div_clk));
>>> +
>>> +    mmc->actual_clock = clk_rate;
>>
>> debug message is described actual_rate as clk_get_rate()..
>> What is correct "actual_clock"?
>>
> Indeed this might be a little misleading.
> "actual_clock" is a member of struct mmc_host and for now we have to live
> with this name. However the meaning in our context is: "currently requested
> clock rate".

I think this is wrong. The intent with mmc->actual_clock is to show
what the real rate of the clock is, not the requested rate.

The requested clock rate is already available via "mmc->ios.clock".

Hopes this clarifies it.

[...]

Kind regards
Uffe

[PATCH 4/9] mmc: meson-gx: improve meson_mmc_irq_thread

[Heiner Kallweit]

Remove unneeded variable ret and simplify the if block.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 529a4f22..4ce4c640 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -670,7 +670,6 @@ static irqreturn_t meson_mmc_irq_thread(int irq, void *dev_id)
 	struct mmc_command *cmd = host->cmd;
 	struct mmc_data *data;
 	unsigned int xfer_bytes;
-	int ret = IRQ_HANDLED;
 
 	if (WARN_ON(!mrq))
 		return IRQ_NONE;
@@ -679,14 +678,12 @@ static irqreturn_t meson_mmc_irq_thread(int irq, void *dev_id)
 		return IRQ_NONE;
 
 	data = cmd->data;
-	if (data) {
+	if (data && data->flags & MMC_DATA_READ) {
 		xfer_bytes = data->blksz * data->blocks;
-		if (data->flags & MMC_DATA_READ) {
-			WARN_ON(xfer_bytes > host->bounce_buf_size);
-			sg_copy_from_buffer(data->sg, data->sg_len,
-					    host->bounce_buf, xfer_bytes);
-			data->bytes_xfered = xfer_bytes;
-		}
+		WARN_ON(xfer_bytes > host->bounce_buf_size);
+		sg_copy_from_buffer(data->sg, data->sg_len,
+				    host->bounce_buf, xfer_bytes);
+		data->bytes_xfered = xfer_bytes;
 	}
 
 	meson_mmc_read_resp(host->mmc, cmd);
@@ -695,7 +692,7 @@ static irqreturn_t meson_mmc_irq_thread(int irq, void *dev_id)
 	else
 		meson_mmc_start_cmd(host->mmc, data->stop);
 
-	return ret;
+	return IRQ_HANDLED;
 }
 
 /*
-- 
2.11.0

[PATCH 5/9] mmc: meson-gx: improve interrupt handling

[Heiner Kallweit]

Disabling and immediately re-enabling interrupts in meson_mmc_request
doesn't provide a benefit. Instead enable interrupts in probe already.
And disable interrupts in remove, this was missing so far.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 4ce4c640..0fba23d2 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -546,11 +546,6 @@ static void meson_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
 	/* Stop execution */
 	writel(0, host->regs + SD_EMMC_START);
 
-	/* clear, ack, enable all interrupts */
-	writel(0, host->regs + SD_EMMC_IRQ_EN);
-	writel(IRQ_EN_MASK, host->regs + SD_EMMC_STATUS);
-	writel(IRQ_EN_MASK, host->regs + SD_EMMC_IRQ_EN);
-
 	host->mrq = mrq;
 
 	if (mrq->sbc)
@@ -777,8 +772,8 @@ static int meson_mmc_probe(struct platform_device *pdev)
 	writel(0, host->regs + SD_EMMC_START);
 
 	/* clear, ack, enable all interrupts */
-	writel(0, host->regs + SD_EMMC_IRQ_EN);
 	writel(IRQ_EN_MASK, host->regs + SD_EMMC_STATUS);
+	writel(IRQ_EN_MASK, host->regs + SD_EMMC_IRQ_EN);
 
 	ret = devm_request_threaded_irq(&pdev->dev, host->irq,
 					meson_mmc_irq, meson_mmc_irq_thread,
@@ -816,6 +811,9 @@ static int meson_mmc_remove(struct platform_device *pdev)
 	if (WARN_ON(!host))
 		return 0;
 
+	/* disable interrupts */
+	writel(0, host->regs + SD_EMMC_IRQ_EN);
+
 	if (host->bounce_buf)
 		dma_free_coherent(host->dev, host->bounce_buf_size,
 				  host->bounce_buf, host->bounce_dma_addr);
-- 
2.11.0

[PATCH 6/9] mmc: meson-gx: set max block count and request size

[Heiner Kallweit]

So far max_blk_count isn't set what results in a default of value 8
to be used (PAGE_SIZE / block size).

Block length field has 9 bits, so set max_blk_count to 2^9-1 = 511.
In addition set max_req_size because max_blk_count is also limited
by max_req_size / block_size.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Acked-by: Kevin Hilman <khilman@baylibre.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 0fba23d2..883263c5 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -781,6 +781,9 @@ static int meson_mmc_probe(struct platform_device *pdev)
 	if (ret)
 		goto free_host;
 
+	mmc->max_blk_count = CMD_CFG_LENGTH_MASK;
+	mmc->max_req_size = mmc->max_blk_count * mmc->max_blk_size;
+
 	/* data bounce buffer */
 	host->bounce_buf_size = SZ_512K;
 	host->bounce_buf =
-- 
2.11.0

[PATCH 7/9] mmc: meson-gx: reduce bounce buffer size

[Heiner Kallweit]

A bounce buffer of 512K isn't needed as the max request size is
511 * 512 byte.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 883263c5..48ce4ba4 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -785,7 +785,7 @@ static int meson_mmc_probe(struct platform_device *pdev)
 	mmc->max_req_size = mmc->max_blk_count * mmc->max_blk_size;
 
 	/* data bounce buffer */
-	host->bounce_buf_size = SZ_512K;
+	host->bounce_buf_size = mmc->max_req_size;
 	host->bounce_buf =
 		dma_alloc_coherent(host->dev, host->bounce_buf_size,
 				   &host->bounce_dma_addr, GFP_KERNEL);
-- 
2.11.0

[PATCH 8/9] mmc: meson-gx: remove unneeded checks in remove

[Heiner Kallweit]

The remove callback is called only if probe finished successfully.
Therefore these checks are not needed.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 48ce4ba4..bc787444 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -811,15 +811,11 @@ static int meson_mmc_remove(struct platform_device *pdev)
 {
 	struct meson_host *host = dev_get_drvdata(&pdev->dev);
 
-	if (WARN_ON(!host))
-		return 0;
-
 	/* disable interrupts */
 	writel(0, host->regs + SD_EMMC_IRQ_EN);
 
-	if (host->bounce_buf)
-		dma_free_coherent(host->dev, host->bounce_buf_size,
-				  host->bounce_buf, host->bounce_dma_addr);
+	dma_free_coherent(host->dev, host->bounce_buf_size,
+			  host->bounce_buf, host->bounce_dma_addr);
 
 	clk_disable_unprepare(host->cfg_div_clk);
 	clk_disable_unprepare(host->core_clk);
-- 
2.11.0

[PATCH 9/9] mmc: meson-gx: add support for HS400 mode

[Heiner Kallweit]

Add support for HS400 mode.

Successfully tested on a Odroid C2 (S905 GXBB).

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index bc787444..83a94cf5 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -83,6 +83,7 @@
 #define   CFG_RC_CC_MASK 0xf
 #define   CFG_STOP_CLOCK BIT(22)
 #define   CFG_CLK_ALWAYS_ON BIT(18)
+#define   CFG_CHK_DS BIT(20)
 #define   CFG_AUTO_CLK BIT(23)
 
 #define SD_EMMC_STATUS 0x48
@@ -412,6 +413,16 @@ static void meson_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	val &= ~(CFG_RC_CC_MASK << CFG_RC_CC_SHIFT);
 	val |= ilog2(SD_EMMC_CFG_CMD_GAP) << CFG_RC_CC_SHIFT;
 
+	val &= ~CFG_DDR;
+	if (ios->timing == MMC_TIMING_UHS_DDR50 ||
+	    ios->timing == MMC_TIMING_MMC_DDR52 ||
+	    ios->timing == MMC_TIMING_MMC_HS400)
+		val |= CFG_DDR;
+
+	val &= ~CFG_CHK_DS;
+	if (ios->timing == MMC_TIMING_MMC_HS400)
+		val |= CFG_CHK_DS;
+
 	writel(val, host->regs + SD_EMMC_CFG);
 
 	if (val != orig)
-- 
2.11.0