[PATCH] sdhci: expose the SDCLK frq in sys ios

[PATCH] sdhci: expose the SDCLK frq in sys ios

[Giuseppe CAVALLARO]

This patch is to expose the SDCLK frequency in the ios /sys entry.

For example, if the max clk for a normal speed card is 20MHz
this will be reported in /sys/kernel/debug/mmc0/ios instead of
the real SDCLK frequency (calculated as Baseclock / divisor;
divisor used for programming the Clock Control Register).

Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
---
 drivers/mmc/host/sdhci.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 0e02cc1..6bdf64f 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1045,6 +1045,7 @@ static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
 	int div = 0; /* Initialized for compiler warning */
 	u16 clk = 0;
 	unsigned long timeout;
+	unsigned int new_hc_clock = host->max_clk;
 
 	if (clock == host->clock)
 		return;
@@ -1134,7 +1135,10 @@ static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
 	sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
 
 out:
-	host->clock = clock;
+	if (div)
+		new_hc_clock = host->max_clk / div;
+
+	host->clock = new_hc_clock;
 }
 
 static void sdhci_set_power(struct sdhci_host *host, unsigned short power)
@@ -1293,6 +1297,9 @@ static void sdhci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 
 	sdhci_set_clock(host, ios->clock);
 
+	/* Clock provided to the HC */
+	ios->clock = host->clock;
+
 	if (ios->power_mode == MMC_POWER_OFF)
 		sdhci_set_power(host, -1);
 	else
-- 
1.7.4.4

Re: [PATCH] sdhci: expose the SDCLK frq in sys ios

[Adrian Hunter]

On 19/10/11 16:45, Giuseppe CAVALLARO wrote:
> This patch is to expose the SDCLK frequency in the ios /sys entry.
> 
> For example, if the max clk for a normal speed card is 20MHz
> this will be reported in /sys/kernel/debug/mmc0/ios instead of
> the real SDCLK frequency (calculated as Baseclock / divisor;
> divisor used for programming the Clock Control Register).
> 
> Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> ---
>  drivers/mmc/host/sdhci.c |    9 ++++++++-
>  1 files changed, 8 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 0e02cc1..6bdf64f 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -1045,6 +1045,7 @@ static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
>  	int div = 0; /* Initialized for compiler warning */
>  	u16 clk = 0;
>  	unsigned long timeout;
> +	unsigned int new_hc_clock = host->max_clk;
>  
>  	if (clock == host->clock)
>  		return;
> @@ -1134,7 +1135,10 @@ static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
>  	sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
>  
>  out:
> -	host->clock = clock;
> +	if (div)
> +		new_hc_clock = host->max_clk / div;

At this point div is the value written to the register not the
divisor

> +
> +	host->clock = new_hc_clock;
>  }
>  
>  static void sdhci_set_power(struct sdhci_host *host, unsigned short power)
> @@ -1293,6 +1297,9 @@ static void sdhci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>  
>  	sdhci_set_clock(host, ios->clock);
>  
> +	/* Clock provided to the HC */
> +	ios->clock = host->clock;

It would be nicer to keep ios->clock as the target
frequency and add a new member to struct mmc_host for
the actual frequency.

> +
>  	if (ios->power_mode == MMC_POWER_OFF)
>  		sdhci_set_power(host, -1);
>  	else

Re: [PATCH] sdhci: expose the SDCLK frq in sys ios

[Giuseppe CAVALLARO]

Hello Adrian

On 10/20/2011 8:22 AM, Adrian Hunter wrote:
> On 19/10/11 16:45, Giuseppe CAVALLARO wrote:
>> This patch is to expose the SDCLK frequency in the ios /sys entry.
>>
>> For example, if the max clk for a normal speed card is 20MHz
>> this will be reported in /sys/kernel/debug/mmc0/ios instead of
>> the real SDCLK frequency (calculated as Baseclock / divisor;
>> divisor used for programming the Clock Control Register).
>>
>> Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
>> ---
>>  drivers/mmc/host/sdhci.c |    9 ++++++++-
>>  1 files changed, 8 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>> index 0e02cc1..6bdf64f 100644
>> --- a/drivers/mmc/host/sdhci.c
>> +++ b/drivers/mmc/host/sdhci.c
>> @@ -1045,6 +1045,7 @@ static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
>>  	int div = 0; /* Initialized for compiler warning */
>>  	u16 clk = 0;
>>  	unsigned long timeout;
>> +	unsigned int new_hc_clock = host->max_clk;
>>  
>>  	if (clock == host->clock)
>>  		return;
>> @@ -1134,7 +1135,10 @@ static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
>>  	sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
>>  
>>  out:
>> -	host->clock = clock;
>> +	if (div)
>> +		new_hc_clock = host->max_clk / div;
> 
> At this point div is the value written to the register not the
> divisor

Hmm, you are right. I'll fix it.

>> +
>> +	host->clock = new_hc_clock;
>>  }
>>  
>>  static void sdhci_set_power(struct sdhci_host *host, unsigned short power)
>> @@ -1293,6 +1297,9 @@ static void sdhci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>>  
>>  	sdhci_set_clock(host, ios->clock);
>>  
>> +	/* Clock provided to the HC */
>> +	ios->clock = host->clock;
> 
> It would be nicer to keep ios->clock as the target
> frequency and add a new member to struct mmc_host for
> the actual frequency.

Why not! I like it. I'll rework the patch and send it again.
Just a question: do I have to dump both in /sys/.../ios?
If yes, maybe it's worth using: card_clock and hc_clock as names.
What do you think?

Regards
Peppe

> 
>> +
>>  	if (ios->power_mode == MMC_POWER_OFF)
>>  		sdhci_set_power(host, -1);
>>  	else
> 
> 

Re: [PATCH] sdhci: expose the SDCLK frq in sys ios

[Adrian Hunter]

On 20/10/11 09:31, Giuseppe CAVALLARO wrote:
> Hello Adrian
> 
> On 10/20/2011 8:22 AM, Adrian Hunter wrote:
>> On 19/10/11 16:45, Giuseppe CAVALLARO wrote:
>>> This patch is to expose the SDCLK frequency in the ios /sys entry.
>>>
>>> For example, if the max clk for a normal speed card is 20MHz
>>> this will be reported in /sys/kernel/debug/mmc0/ios instead of
>>> the real SDCLK frequency (calculated as Baseclock / divisor;
>>> divisor used for programming the Clock Control Register).
>>>
>>> Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
>>> ---
>>>  drivers/mmc/host/sdhci.c |    9 ++++++++-
>>>  1 files changed, 8 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>>> index 0e02cc1..6bdf64f 100644
>>> --- a/drivers/mmc/host/sdhci.c
>>> +++ b/drivers/mmc/host/sdhci.c
>>> @@ -1045,6 +1045,7 @@ static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
>>>  	int div = 0; /* Initialized for compiler warning */
>>>  	u16 clk = 0;
>>>  	unsigned long timeout;
>>> +	unsigned int new_hc_clock = host->max_clk;
>>>  
>>>  	if (clock == host->clock)
>>>  		return;
>>> @@ -1134,7 +1135,10 @@ static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
>>>  	sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
>>>  
>>>  out:
>>> -	host->clock = clock;
>>> +	if (div)
>>> +		new_hc_clock = host->max_clk / div;
>>
>> At this point div is the value written to the register not the
>> divisor
> 
> Hmm, you are right. I'll fix it.
> 
>>> +
>>> +	host->clock = new_hc_clock;
>>>  }
>>>  
>>>  static void sdhci_set_power(struct sdhci_host *host, unsigned short power)
>>> @@ -1293,6 +1297,9 @@ static void sdhci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>>>  
>>>  	sdhci_set_clock(host, ios->clock);
>>>  
>>> +	/* Clock provided to the HC */
>>> +	ios->clock = host->clock;
>>
>> It would be nicer to keep ios->clock as the target
>> frequency and add a new member to struct mmc_host for
>> the actual frequency.
> 
> Why not! I like it. I'll rework the patch and send it again.
> Just a question: do I have to dump both in /sys/.../ios?
> If yes, maybe it's worth using: card_clock and hc_clock as names.
> What do you think?

For me hc_clock is confusing.  Potentially the host controller has
lots of clocks e.g. interface clock, functional clock, debounce clock

Also, I would not rename anything.  You could add

	/sys/kernel/debug/mmcx/actual_clock

or amend /sys/kernel/debug/mmcx/ios e.g.

clock:          26000000 Hz (actual 25000000 Hz)
vdd:            17 (2.9 ~ 3.0 V)
bus mode:       2 (push-pull)
chip select:    0 (don't care)
power mode:     2 (on)
bus width:      2 (4 bits)
timing spec:    0 (legacy)



> 
> Regards
> Peppe
> 
>>
>>> +
>>>  	if (ios->power_mode == MMC_POWER_OFF)
>>>  		sdhci_set_power(host, -1);
>>>  	else
>>
>>
> 
> 

Re: [PATCH] sdhci: expose the SDCLK frq in sys ios

[Giuseppe CAVALLARO]

On 10/20/2011 9:38 AM, Adrian Hunter wrote:
> On 20/10/11 09:31, Giuseppe CAVALLARO wrote:
>> Hello Adrian
>>
>> On 10/20/2011 8:22 AM, Adrian Hunter wrote:
>>> On 19/10/11 16:45, Giuseppe CAVALLARO wrote:
>>>> This patch is to expose the SDCLK frequency in the ios /sys entry.
>>>>
>>>> For example, if the max clk for a normal speed card is 20MHz
>>>> this will be reported in /sys/kernel/debug/mmc0/ios instead of
>>>> the real SDCLK frequency (calculated as Baseclock / divisor;
>>>> divisor used for programming the Clock Control Register).
>>>>
>>>> Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
>>>> ---
>>>>  drivers/mmc/host/sdhci.c |    9 ++++++++-
>>>>  1 files changed, 8 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>>>> index 0e02cc1..6bdf64f 100644
>>>> --- a/drivers/mmc/host/sdhci.c
>>>> +++ b/drivers/mmc/host/sdhci.c
>>>> @@ -1045,6 +1045,7 @@ static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
>>>>  	int div = 0; /* Initialized for compiler warning */
>>>>  	u16 clk = 0;
>>>>  	unsigned long timeout;
>>>> +	unsigned int new_hc_clock = host->max_clk;
>>>>  
>>>>  	if (clock == host->clock)
>>>>  		return;
>>>> @@ -1134,7 +1135,10 @@ static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
>>>>  	sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
>>>>  
>>>>  out:
>>>> -	host->clock = clock;
>>>> +	if (div)
>>>> +		new_hc_clock = host->max_clk / div;
>>>
>>> At this point div is the value written to the register not the
>>> divisor
>>
>> Hmm, you are right. I'll fix it.
>>
>>>> +
>>>> +	host->clock = new_hc_clock;
>>>>  }
>>>>  
>>>>  static void sdhci_set_power(struct sdhci_host *host, unsigned short power)
>>>> @@ -1293,6 +1297,9 @@ static void sdhci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>>>>  
>>>>  	sdhci_set_clock(host, ios->clock);
>>>>  
>>>> +	/* Clock provided to the HC */
>>>> +	ios->clock = host->clock;
>>>
>>> It would be nicer to keep ios->clock as the target
>>> frequency and add a new member to struct mmc_host for
>>> the actual frequency.
>>
>> Why not! I like it. I'll rework the patch and send it again.
>> Just a question: do I have to dump both in /sys/.../ios?
>> If yes, maybe it's worth using: card_clock and hc_clock as names.
>> What do you think?
> 
> For me hc_clock is confusing.  Potentially the host controller has
> lots of clocks e.g. interface clock, functional clock, debounce clock
> 
> Also, I would not rename anything.  You could add

Indeed, it 's better to not rename clock field for compatibility. ;-)

> 	/sys/kernel/debug/mmcx/actual_clock
> 
> or amend /sys/kernel/debug/mmcx/ios e.g.
> 
> clock:          26000000 Hz (actual 25000000 Hz)

ok I prefer the latter.

Thanks
Peppe

> vdd:            17 (2.9 ~ 3.0 V)
> bus mode:       2 (push-pull)
> chip select:    0 (don't care)
> power mode:     2 (on)
> bus width:      2 (4 bits)
> timing spec:    0 (legacy)



> 
> 
> 
>>
>> Regards
>> Peppe
>>
>>>
>>>> +
>>>>  	if (ios->power_mode == MMC_POWER_OFF)
>>>>  		sdhci_set_power(host, -1);
>>>>  	else
>>>
>>>
>>
>>
> 
> 

[PATCH] mmc: debugfs: expose the SDCLK frq in sys ios (v2)

[Giuseppe CAVALLARO]

This patch is to expose the actual SDCLK frequency in
/sys/kernel/debug/mmcX/ios entry.

For example, if the max clk for a normal speed card is 20MHz
this is reported in /sys/kernel/debug/mmcX/ios.
Unfortunately the actual SDCLK frequency (i.e. Baseclock / divisor)
is not reported at all: for example, in that case, on Arasan HC,
it should be 48/4=12 (MHz).

Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
---
 drivers/mmc/core/debugfs.c |    2 ++
 drivers/mmc/host/sdhci.c   |    7 +++++++
 include/linux/mmc/host.h   |    2 ++
 3 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/core/debugfs.c b/drivers/mmc/core/debugfs.c
index 998797e..0b9a4aa 100644
--- a/drivers/mmc/core/debugfs.c
+++ b/drivers/mmc/core/debugfs.c
@@ -46,6 +46,8 @@ static int mmc_ios_show(struct seq_file *s, void *data)
 	const char *str;
 
 	seq_printf(s, "clock:\t\t%u Hz\n", ios->clock);
+	if (host->actual_clock)
+		seq_printf(s, "actual clock:\t%u Hz\n", host->actual_clock);
 	seq_printf(s, "vdd:\t\t%u ", ios->vdd);
 	if ((1 << ios->vdd) & MMC_VDD_165_195)
 		seq_printf(s, "(1.65 - 1.95 V)\n");
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 0e02cc1..a0d61b0 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1043,6 +1043,7 @@ static void sdhci_finish_command(struct sdhci_host *host)
 static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
 {
 	int div = 0; /* Initialized for compiler warning */
+	int actual_div = 0;
 	u16 clk = 0;
 	unsigned long timeout;
 
@@ -1088,6 +1089,7 @@ static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
 				clk = SDHCI_PROG_CLOCK_MODE;
 				div--;
 			}
+			actual_div = div;
 		} else {
 			/* Version 3.00 divisors must be a multiple of 2. */
 			if (host->max_clk <= clock)
@@ -1099,6 +1101,7 @@ static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
 						break;
 				}
 			}
+			actual_div = div;
 			div >>= 1;
 		}
 	} else {
@@ -1107,9 +1110,13 @@ static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
 			if ((host->max_clk / div) <= clock)
 				break;
 		}
+			actual_div = div;
 		div >>= 1;
 	}
 
+	if (actual_div)
+		host->mmc->actual_clock = host->max_clk / actual_div;
+
 	clk |= (div & SDHCI_DIV_MASK) << SDHCI_DIVIDER_SHIFT;
 	clk |= ((div & SDHCI_DIV_HI_MASK) >> SDHCI_DIV_MASK_LEN)
 		<< SDHCI_DIVIDER_HI_SHIFT;
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 1d09562..d815311 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -302,6 +302,8 @@ struct mmc_host {
 
 	struct mmc_async_req	*areq;		/* active async req */
 
+	unsigned int		actual_clock;	/* Actual HC clock rate */
+
 	unsigned long		private[0] ____cacheline_aligned;
 };
 
-- 
1.7.4.4

Re: [PATCH] mmc: debugfs: expose the SDCLK frq in sys ios (v2)

[Adrian Hunter]

On 21/10/11 11:16, Giuseppe CAVALLARO wrote:
> This patch is to expose the actual SDCLK frequency in
> /sys/kernel/debug/mmcX/ios entry.
>
> For example, if the max clk for a normal speed card is 20MHz
> this is reported in /sys/kernel/debug/mmcX/ios.
> Unfortunately the actual SDCLK frequency (i.e. Baseclock / divisor)
> is not reported at all: for example, in that case, on Arasan HC,
> it should be 48/4=12 (MHz).
>
> Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> ---
>  drivers/mmc/core/debugfs.c |    2 ++
>  drivers/mmc/host/sdhci.c   |    7 +++++++
>  include/linux/mmc/host.h   |    2 ++
>  3 files changed, 11 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/mmc/core/debugfs.c b/drivers/mmc/core/debugfs.c
> index 998797e..0b9a4aa 100644
> --- a/drivers/mmc/core/debugfs.c
> +++ b/drivers/mmc/core/debugfs.c
> @@ -46,6 +46,8 @@ static int mmc_ios_show(struct seq_file *s, void *data)
>  	const char *str;
>  
>  	seq_printf(s, "clock:\t\t%u Hz\n", ios->clock);
> +	if (host->actual_clock)
> +		seq_printf(s, "actual clock:\t%u Hz\n", host->actual_clock);
>  	seq_printf(s, "vdd:\t\t%u ", ios->vdd);
>  	if ((1 << ios->vdd) & MMC_VDD_165_195)
>  		seq_printf(s, "(1.65 - 1.95 V)\n");
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 0e02cc1..a0d61b0 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -1043,6 +1043,7 @@ static void sdhci_finish_command(struct sdhci_host *host)
>  static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
>  {
>  	int div = 0; /* Initialized for compiler warning */
> +	int actual_div = 0;
>  	u16 clk = 0;
>  	unsigned long timeout;

What if the clock is gated i.e. zero?  Maybe add

	host->mmc->actual_clock = 0;


>  
> @@ -1088,6 +1089,7 @@ static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
>  				clk = SDHCI_PROG_CLOCK_MODE;
>  				div--;
>  			}
> +			actual_div = div;

What about the clock multiplier?

>  		} else {
>  			/* Version 3.00 divisors must be a multiple of 2. */
>  			if (host->max_clk <= clock)
> @@ -1099,6 +1101,7 @@ static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
>  						break;
>  				}
>  			}
> +			actual_div = div;
>  			div >>= 1;
>  		}
>  	} else {
> @@ -1107,9 +1110,13 @@ static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
>  			if ((host->max_clk / div) <= clock)
>  				break;
>  		}
> +			actual_div = div;
>  		div >>= 1;
>  	}
>  
> +	if (actual_div)
> +		host->mmc->actual_clock = host->max_clk / actual_div;
> +
>  	clk |= (div & SDHCI_DIV_MASK) << SDHCI_DIVIDER_SHIFT;
>  	clk |= ((div & SDHCI_DIV_HI_MASK) >> SDHCI_DIV_MASK_LEN)
>  		<< SDHCI_DIVIDER_HI_SHIFT;
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index 1d09562..d815311 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -302,6 +302,8 @@ struct mmc_host {
>  
>  	struct mmc_async_req	*areq;		/* active async req */
>  
> +	unsigned int		actual_clock;	/* Actual HC clock rate */
> +
>  	unsigned long		private[0] ____cacheline_aligned;
>  };
>  

Re: [PATCH] mmc: debugfs: expose the SDCLK frq in sys ios (v2)

[Giuseppe CAVALLARO]

Hello Adrian

On 10/25/2011 10:40 AM, Adrian Hunter wrote:
> On 21/10/11 11:16, Giuseppe CAVALLARO wrote:
>> This patch is to expose the actual SDCLK frequency in
>> /sys/kernel/debug/mmcX/ios entry.
>>
>> For example, if the max clk for a normal speed card is 20MHz
>> this is reported in /sys/kernel/debug/mmcX/ios.
>> Unfortunately the actual SDCLK frequency (i.e. Baseclock / divisor)
>> is not reported at all: for example, in that case, on Arasan HC,
>> it should be 48/4=12 (MHz).
>>
>> Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
>> ---
>>  drivers/mmc/core/debugfs.c |    2 ++
>>  drivers/mmc/host/sdhci.c   |    7 +++++++
>>  include/linux/mmc/host.h   |    2 ++
>>  3 files changed, 11 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/mmc/core/debugfs.c b/drivers/mmc/core/debugfs.c
>> index 998797e..0b9a4aa 100644
>> --- a/drivers/mmc/core/debugfs.c
>> +++ b/drivers/mmc/core/debugfs.c
>> @@ -46,6 +46,8 @@ static int mmc_ios_show(struct seq_file *s, void *data)
>>  	const char *str;
>>  
>>  	seq_printf(s, "clock:\t\t%u Hz\n", ios->clock);
>> +	if (host->actual_clock)
>> +		seq_printf(s, "actual clock:\t%u Hz\n", host->actual_clock);
>>  	seq_printf(s, "vdd:\t\t%u ", ios->vdd);
>>  	if ((1 << ios->vdd) & MMC_VDD_165_195)
>>  		seq_printf(s, "(1.65 - 1.95 V)\n");
>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>> index 0e02cc1..a0d61b0 100644
>> --- a/drivers/mmc/host/sdhci.c
>> +++ b/drivers/mmc/host/sdhci.c
>> @@ -1043,6 +1043,7 @@ static void sdhci_finish_command(struct sdhci_host *host)
>>  static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
>>  {
>>  	int div = 0; /* Initialized for compiler warning */
>> +	int actual_div = 0;
>>  	u16 clk = 0;
>>  	unsigned long timeout;
> 
> What if the clock is gated i.e. zero?  Maybe add
> 
> 	host->mmc->actual_clock = 0;

ok

> 
> 
>>  
>> @@ -1088,6 +1089,7 @@ static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
>>  				clk = SDHCI_PROG_CLOCK_MODE;
>>  				div--;
>>  			}
>> +			actual_div = div;
> 
> What about the clock multiplier?

I hadn't the SDHCI 3.0 spec. I'm looking at it now and I'll provide a
new patch asap.

> 
>>  		} else {
>>  			/* Version 3.00 divisors must be a multiple of 2. */
>>  			if (host->max_clk <= clock)
>> @@ -1099,6 +1101,7 @@ static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
>>  						break;
>>  				}
>>  			}
>> +			actual_div = div;
>>  			div >>= 1;
>>  		}
>>  	} else {
>> @@ -1107,9 +1110,13 @@ static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
>>  			if ((host->max_clk / div) <= clock)
>>  				break;
>>  		}
>> +			actual_div = div;
>>  		div >>= 1;
>>  	}
>>  
>> +	if (actual_div)
>> +		host->mmc->actual_clock = host->max_clk / actual_div;
>> +
>>  	clk |= (div & SDHCI_DIV_MASK) << SDHCI_DIVIDER_SHIFT;
>>  	clk |= ((div & SDHCI_DIV_HI_MASK) >> SDHCI_DIV_MASK_LEN)
>>  		<< SDHCI_DIVIDER_HI_SHIFT;
>> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
>> index 1d09562..d815311 100644
>> --- a/include/linux/mmc/host.h
>> +++ b/include/linux/mmc/host.h
>> @@ -302,6 +302,8 @@ struct mmc_host {
>>  
>>  	struct mmc_async_req	*areq;		/* active async req */
>>  
>> +	unsigned int		actual_clock;	/* Actual HC clock rate */
>> +
>>  	unsigned long		private[0] ____cacheline_aligned;
>>  };
>>  
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[PATCH] mmc: debugfs: expose the SDCLK frq in sys ios (v3)

[Giuseppe CAVALLARO]

This patch is to expose the actual SDCLK frequency in
/sys/kernel/debug/mmcX/ios entry.

For example, if the max clk for a normal speed card is 20MHz
this is reported in /sys/kernel/debug/mmcX/ios.
Unfortunately the actual SDCLK frequency (i.e. Baseclock / divisor)
is not reported at all: for example, in that case, on Arasan HC,
it should be 48/4=12 (MHz).

Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
---
 drivers/mmc/core/debugfs.c |    2 ++
 drivers/mmc/host/sdhci.c   |   10 ++++++++++
 include/linux/mmc/host.h   |    2 ++
 3 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/core/debugfs.c b/drivers/mmc/core/debugfs.c
index 998797e..0b9a4aa 100644
--- a/drivers/mmc/core/debugfs.c
+++ b/drivers/mmc/core/debugfs.c
@@ -46,6 +46,8 @@ static int mmc_ios_show(struct seq_file *s, void *data)
 	const char *str;
 
 	seq_printf(s, "clock:\t\t%u Hz\n", ios->clock);
+	if (host->actual_clock)
+		seq_printf(s, "actual clock:\t%u Hz\n", host->actual_clock);
 	seq_printf(s, "vdd:\t\t%u ", ios->vdd);
 	if ((1 << ios->vdd) & MMC_VDD_165_195)
 		seq_printf(s, "(1.65 - 1.95 V)\n");
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 0e02cc1..6bfdec5 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1043,9 +1043,12 @@ static void sdhci_finish_command(struct sdhci_host *host)
 static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
 {
 	int div = 0; /* Initialized for compiler warning */
+	int real_div = div, clk_mul = 1;
 	u16 clk = 0;
 	unsigned long timeout;
 
+	host->mmc->actual_clock = 0;
+
 	if (clock == host->clock)
 		return;
 
@@ -1086,6 +1089,8 @@ static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
 				 * Control register.
 				 */
 				clk = SDHCI_PROG_CLOCK_MODE;
+				real_div = div;
+				clk_mul = host->clk_mul;
 				div--;
 			}
 		} else {
@@ -1099,6 +1104,7 @@ static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
 						break;
 				}
 			}
+			real_div = div;
 			div >>= 1;
 		}
 	} else {
@@ -1107,9 +1113,13 @@ static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
 			if ((host->max_clk / div) <= clock)
 				break;
 		}
+		real_div = div;
 		div >>= 1;
 	}
 
+	if (real_div)
+		host->mmc->actual_clock = (host->max_clk * clk_mul) / real_div;
+
 	clk |= (div & SDHCI_DIV_MASK) << SDHCI_DIVIDER_SHIFT;
 	clk |= ((div & SDHCI_DIV_HI_MASK) >> SDHCI_DIV_MASK_LEN)
 		<< SDHCI_DIVIDER_HI_SHIFT;
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 1d09562..d815311 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -302,6 +302,8 @@ struct mmc_host {
 
 	struct mmc_async_req	*areq;		/* active async req */
 
+	unsigned int		actual_clock;	/* Actual HC clock rate */
+
 	unsigned long		private[0] ____cacheline_aligned;
 };
 
-- 
1.7.4.4

Re: [PATCH] mmc: debugfs: expose the SDCLK frq in sys ios (v3)

[Sebastian Rasmussen]

Hi!

> +++ b/drivers/mmc/host/sdhci.c
> @@ -1107,9 +1113,13 @@ static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
>                        if ((host->max_clk / div) <= clock)
>                                break;
>                }
> +               real_div = div;
>                div >>= 1;
>        }
>
> +       if (real_div)
> +               host->mmc->actual_clock = (host->max_clk * clk_mul) / real_div;

What triggered me to look at this is that usually a divider of 0 means
that the clock is not divided at all, and your code seemed to set
host->mmc->actual_clock to 0 in this case. After further inspection I
realize that real_div appears to always be >= 1 which means that the
if-statement is unnecessary. Or did I miss something?

 / Sebastian

PS. I don't have a good SDHCI spec, is it publicly available
somewhere? I have only found Pierre Ossman's old wiki on
archive.org...

Re: [PATCH] mmc: debugfs: expose the SDCLK frq in sys ios (v3)

[Adrian Hunter]

On 27/10/11 09:16, Giuseppe CAVALLARO wrote:
> This patch is to expose the actual SDCLK frequency in
> /sys/kernel/debug/mmcX/ios entry.
> 
> For example, if the max clk for a normal speed card is 20MHz
> this is reported in /sys/kernel/debug/mmcX/ios.
> Unfortunately the actual SDCLK frequency (i.e. Baseclock / divisor)
> is not reported at all: for example, in that case, on Arasan HC,
> it should be 48/4=12 (MHz).
> 
> Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> ---
>  drivers/mmc/core/debugfs.c |    2 ++
>  drivers/mmc/host/sdhci.c   |   10 ++++++++++
>  include/linux/mmc/host.h   |    2 ++
>  3 files changed, 14 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/mmc/core/debugfs.c b/drivers/mmc/core/debugfs.c
> index 998797e..0b9a4aa 100644
> --- a/drivers/mmc/core/debugfs.c
> +++ b/drivers/mmc/core/debugfs.c
> @@ -46,6 +46,8 @@ static int mmc_ios_show(struct seq_file *s, void *data)
>  	const char *str;
>  
>  	seq_printf(s, "clock:\t\t%u Hz\n", ios->clock);
> +	if (host->actual_clock)
> +		seq_printf(s, "actual clock:\t%u Hz\n", host->actual_clock);
>  	seq_printf(s, "vdd:\t\t%u ", ios->vdd);
>  	if ((1 << ios->vdd) & MMC_VDD_165_195)
>  		seq_printf(s, "(1.65 - 1.95 V)\n");
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 0e02cc1..6bfdec5 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -1043,9 +1043,12 @@ static void sdhci_finish_command(struct sdhci_host *host)
>  static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
>  {
>  	int div = 0; /* Initialized for compiler warning */
> +	int real_div = div, clk_mul = 1;
>  	u16 clk = 0;
>  	unsigned long timeout;
>  
> +	host->mmc->actual_clock = 0;
> +

This needs to come after "if (clock == host->clock) return;"

>  	if (clock == host->clock)
>  		return;
>  
> @@ -1086,6 +1089,8 @@ static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
>  				 * Control register.
>  				 */
>  				clk = SDHCI_PROG_CLOCK_MODE;
> +				real_div = div;
> +				clk_mul = host->clk_mul;
>  				div--;
>  			}
>  		} else {
> @@ -1099,6 +1104,7 @@ static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
>  						break;
>  				}
>  			}
> +			real_div = div;
>  			div >>= 1;
>  		}
>  	} else {
> @@ -1107,9 +1113,13 @@ static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
>  			if ((host->max_clk / div) <= clock)
>  				break;
>  		}
> +		real_div = div;
>  		div >>= 1;
>  	}
>  
> +	if (real_div)
> +		host->mmc->actual_clock = (host->max_clk * clk_mul) / real_div;
> +
>  	clk |= (div & SDHCI_DIV_MASK) << SDHCI_DIVIDER_SHIFT;
>  	clk |= ((div & SDHCI_DIV_HI_MASK) >> SDHCI_DIV_MASK_LEN)
>  		<< SDHCI_DIVIDER_HI_SHIFT;
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index 1d09562..d815311 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -302,6 +302,8 @@ struct mmc_host {
>  
>  	struct mmc_async_req	*areq;		/* active async req */
>  
> +	unsigned int		actual_clock;	/* Actual HC clock rate */
> +
>  	unsigned long		private[0] ____cacheline_aligned;
>  };
>  

Re: [PATCH] mmc: debugfs: expose the SDCLK frq in sys ios (v3)

[Giuseppe CAVALLARO]

On 11/2/2011 11:38 AM, Adrian Hunter wrote:
> On 27/10/11 09:16, Giuseppe CAVALLARO wrote:
>> This patch is to expose the actual SDCLK frequency in
>> /sys/kernel/debug/mmcX/ios entry.
>>
>> For example, if the max clk for a normal speed card is 20MHz
>> this is reported in /sys/kernel/debug/mmcX/ios.
>> Unfortunately the actual SDCLK frequency (i.e. Baseclock / divisor)
>> is not reported at all: for example, in that case, on Arasan HC,
>> it should be 48/4=12 (MHz).
>>
>> Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
>> ---
>>  drivers/mmc/core/debugfs.c |    2 ++
>>  drivers/mmc/host/sdhci.c   |   10 ++++++++++
>>  include/linux/mmc/host.h   |    2 ++
>>  3 files changed, 14 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/mmc/core/debugfs.c b/drivers/mmc/core/debugfs.c
>> index 998797e..0b9a4aa 100644
>> --- a/drivers/mmc/core/debugfs.c
>> +++ b/drivers/mmc/core/debugfs.c
>> @@ -46,6 +46,8 @@ static int mmc_ios_show(struct seq_file *s, void *data)
>>  	const char *str;
>>  
>>  	seq_printf(s, "clock:\t\t%u Hz\n", ios->clock);
>> +	if (host->actual_clock)
>> +		seq_printf(s, "actual clock:\t%u Hz\n", host->actual_clock);
>>  	seq_printf(s, "vdd:\t\t%u ", ios->vdd);
>>  	if ((1 << ios->vdd) & MMC_VDD_165_195)
>>  		seq_printf(s, "(1.65 - 1.95 V)\n");
>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>> index 0e02cc1..6bfdec5 100644
>> --- a/drivers/mmc/host/sdhci.c
>> +++ b/drivers/mmc/host/sdhci.c
>> @@ -1043,9 +1043,12 @@ static void sdhci_finish_command(struct sdhci_host *host)
>>  static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
>>  {
>>  	int div = 0; /* Initialized for compiler warning */
>> +	int real_div = div, clk_mul = 1;
>>  	u16 clk = 0;
>>  	unsigned long timeout;
>>  
>> +	host->mmc->actual_clock = 0;
>> +
> 
> This needs to come after "if (clock == host->clock) return;"

fixed in V4
thx
peppe

> 
>>  	if (clock == host->clock)
>>  		return;
>>  
>> @@ -1086,6 +1089,8 @@ static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
>>  				 * Control register.
>>  				 */
>>  				clk = SDHCI_PROG_CLOCK_MODE;
>> +				real_div = div;
>> +				clk_mul = host->clk_mul;
>>  				div--;
>>  			}
>>  		} else {
>> @@ -1099,6 +1104,7 @@ static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
>>  						break;
>>  				}
>>  			}
>> +			real_div = div;
>>  			div >>= 1;
>>  		}
>>  	} else {
>> @@ -1107,9 +1113,13 @@ static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
>>  			if ((host->max_clk / div) <= clock)
>>  				break;
>>  		}
>> +		real_div = div;
>>  		div >>= 1;
>>  	}
>>  
>> +	if (real_div)
>> +		host->mmc->actual_clock = (host->max_clk * clk_mul) / real_div;
>> +
>>  	clk |= (div & SDHCI_DIV_MASK) << SDHCI_DIVIDER_SHIFT;
>>  	clk |= ((div & SDHCI_DIV_HI_MASK) >> SDHCI_DIV_MASK_LEN)
>>  		<< SDHCI_DIVIDER_HI_SHIFT;
>> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
>> index 1d09562..d815311 100644
>> --- a/include/linux/mmc/host.h
>> +++ b/include/linux/mmc/host.h
>> @@ -302,6 +302,8 @@ struct mmc_host {
>>  
>>  	struct mmc_async_req	*areq;		/* active async req */
>>  
>> +	unsigned int		actual_clock;	/* Actual HC clock rate */
>> +
>>  	unsigned long		private[0] ____cacheline_aligned;
>>  };
>>  
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH] mmc: debugfs: expose the SDCLK frq in sys ios (v3)

[Giuseppe CAVALLARO]

On 10/28/2011 12:02 AM, Sebastian Rasmussen wrote:
> Hi!
> 
>> +++ b/drivers/mmc/host/sdhci.c
>> @@ -1107,9 +1113,13 @@ static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
>>                        if ((host->max_clk / div) <= clock)
>>                                break;
>>                }
>> +               real_div = div;
>>                div >>= 1;
>>        }
>>
>> +       if (real_div)
>> +               host->mmc->actual_clock = (host->max_clk * clk_mul) / real_div;
> 
> What triggered me to look at this is that usually a divider of 0 means
> that the clock is not divided at all, and your code seemed to set
> host->mmc->actual_clock to 0 in this case. After further inspection I
> realize that real_div appears to always be >= 1 which means that the
> if-statement is unnecessary. Or did I miss something?

fixed in v4
thx
peppe

> 
>  / Sebastian
> 
> PS. I don't have a good SDHCI spec, is it publicly available
> somewhere? I have only found Pierre Ossman's old wiki on
> archive.org...
>