Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1370689 > unrolled thread
| Started by | Ludovic Desroches <ludovic.desroches@atmel.com> |
|---|---|
| First post | 2016-04-04 17:30 +0200 |
| Last post | 2016-04-07 10:00 +0200 |
| Articles | 5 — 2 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[PATCH v2 1/3] mmc: sdhci: introduce sdhci_compute_clock_config Ludovic Desroches <ludovic.desroches@atmel.com> - 2016-04-04 17:30 +0200
Re: [PATCH v2 1/3] mmc: sdhci: introduce sdhci_compute_clock_config Adrian Hunter <adrian.hunter@intel.com> - 2016-04-06 14:50 +0200
Re: [PATCH v2 1/3] mmc: sdhci: introduce sdhci_compute_clock_config Ludovic Desroches <ludovic.desroches@atmel.com> - 2016-04-06 17:10 +0200
Re: [PATCH v2 1/3] mmc: sdhci: introduce sdhci_compute_clock_config Ludovic Desroches <ludovic.desroches@atmel.com> - 2016-04-06 17:20 +0200
Re: [PATCH v2 1/3] mmc: sdhci: introduce sdhci_compute_clock_config Adrian Hunter <adrian.hunter@intel.com> - 2016-04-07 10:00 +0200
| From | Ludovic Desroches <ludovic.desroches@atmel.com> |
|---|---|
| Date | 2016-04-04 17:30 +0200 |
| Subject | [PATCH v2 1/3] mmc: sdhci: introduce sdhci_compute_clock_config |
| Message-ID | <rkeBX-52d-7@gated-at.bofh.it> |
In order to remove the SDHCI_QUIRK2_NEED_DELAY_AFTER_INT_CLK_RST and to
reduce code duplication, put the code relative to the SD clock
configuration in a function which can be used by hosts for the
implementation of the set_clock() callback.
Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
---
drivers/mmc/host/sdhci.c | 33 ++++++++++++++++++++++-----------
drivers/mmc/host/sdhci.h | 1 +
2 files changed, 23 insertions(+), 11 deletions(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 6bd3d17..2c3dede 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1091,23 +1091,13 @@ static u16 sdhci_get_preset_value(struct sdhci_host *host)
return preset;
}
-void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
+u16 sdhci_compute_clock_config(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;
bool switch_base_clk = false;
- host->mmc->actual_clock = 0;
-
- sdhci_writew(host, 0, SDHCI_CLOCK_CONTROL);
- if (host->quirks2 & SDHCI_QUIRK2_NEED_DELAY_AFTER_INT_CLK_RST)
- mdelay(1);
-
- if (clock == 0)
- return;
-
if (host->version >= SDHCI_SPEC_300) {
if (host->preset_enabled) {
u16 pre_val;
@@ -1188,6 +1178,27 @@ clock_set:
clk |= (div & SDHCI_DIV_MASK) << SDHCI_DIVIDER_SHIFT;
clk |= ((div & SDHCI_DIV_HI_MASK) >> SDHCI_DIV_MASK_LEN)
<< SDHCI_DIVIDER_HI_SHIFT;
+
+ return clk;
+}
+EXPORT_SYMBOL_GPL(sdhci_compute_clock_config);
+
+void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
+{
+ u16 clk;
+ unsigned long timeout;
+
+ host->mmc->actual_clock = 0;
+
+ sdhci_writew(host, 0, SDHCI_CLOCK_CONTROL);
+ if (host->quirks2 & SDHCI_QUIRK2_NEED_DELAY_AFTER_INT_CLK_RST)
+ mdelay(1);
+
+ if (clock == 0)
+ return;
+
+ clk = sdhci_compute_clock_config(host, clock);
+
clk |= SDHCI_CLOCK_INT_EN;
sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 0f39f4f..23ddd6e 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -661,6 +661,7 @@ static inline bool sdhci_sdio_irq_enabled(struct sdhci_host *host)
return !!(host->flags & SDHCI_SDIO_IRQ_ENABLED);
}
+u16 sdhci_compute_clock_config(struct sdhci_host *host, unsigned int clock);
void sdhci_set_clock(struct sdhci_host *host, unsigned int clock);
void sdhci_set_power(struct sdhci_host *host, unsigned char mode,
unsigned short vdd);
--
2.5.0
[toc] | [next] | [standalone]
| From | Adrian Hunter <adrian.hunter@intel.com> |
|---|---|
| Date | 2016-04-06 14:50 +0200 |
| Message-ID | <rkV4d-3Qg-5@gated-at.bofh.it> |
| In reply to | #1370689 |
On 04/04/16 18:27, Ludovic Desroches wrote:
> In order to remove the SDHCI_QUIRK2_NEED_DELAY_AFTER_INT_CLK_RST and to
> reduce code duplication, put the code relative to the SD clock
> configuration in a function which can be used by hosts for the
> implementation of the set_clock() callback.
>
> Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
> ---
> drivers/mmc/host/sdhci.c | 33 ++++++++++++++++++++++-----------
> drivers/mmc/host/sdhci.h | 1 +
> 2 files changed, 23 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 6bd3d17..2c3dede 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -1091,23 +1091,13 @@ static u16 sdhci_get_preset_value(struct sdhci_host *host)
> return preset;
> }
>
> -void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
> +u16 sdhci_compute_clock_config(struct sdhci_host *host, unsigned int clock)
The function name 'sdhci_compute_clock_config' seems a bit long. How about
just sdhci_calc_clk.
Also it needs to calculate 'actual_clock' too, so it needs to do that and
return the value.
> {
> int div = 0; /* Initialized for compiler warning */
> int real_div = div, clk_mul = 1;
> u16 clk = 0;
> - unsigned long timeout;
> bool switch_base_clk = false;
>
> - host->mmc->actual_clock = 0;
> -
> - sdhci_writew(host, 0, SDHCI_CLOCK_CONTROL);
> - if (host->quirks2 & SDHCI_QUIRK2_NEED_DELAY_AFTER_INT_CLK_RST)
> - mdelay(1);
> -
> - if (clock == 0)
> - return;
> -
> if (host->version >= SDHCI_SPEC_300) {
> if (host->preset_enabled) {
> u16 pre_val;
> @@ -1188,6 +1178,27 @@ clock_set:
> clk |= (div & SDHCI_DIV_MASK) << SDHCI_DIVIDER_SHIFT;
> clk |= ((div & SDHCI_DIV_HI_MASK) >> SDHCI_DIV_MASK_LEN)
> << SDHCI_DIVIDER_HI_SHIFT;
> +
> + return clk;
> +}
> +EXPORT_SYMBOL_GPL(sdhci_compute_clock_config);
> +
> +void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
> +{
> + u16 clk;
> + unsigned long timeout;
> +
> + host->mmc->actual_clock = 0;
> +
> + sdhci_writew(host, 0, SDHCI_CLOCK_CONTROL);
> + if (host->quirks2 & SDHCI_QUIRK2_NEED_DELAY_AFTER_INT_CLK_RST)
> + mdelay(1);
> +
> + if (clock == 0)
> + return;
> +
> + clk = sdhci_compute_clock_config(host, clock);
> +
> clk |= SDHCI_CLOCK_INT_EN;
> sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
>
> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> index 0f39f4f..23ddd6e 100644
> --- a/drivers/mmc/host/sdhci.h
> +++ b/drivers/mmc/host/sdhci.h
> @@ -661,6 +661,7 @@ static inline bool sdhci_sdio_irq_enabled(struct sdhci_host *host)
> return !!(host->flags & SDHCI_SDIO_IRQ_ENABLED);
> }
>
> +u16 sdhci_compute_clock_config(struct sdhci_host *host, unsigned int clock);
> void sdhci_set_clock(struct sdhci_host *host, unsigned int clock);
> void sdhci_set_power(struct sdhci_host *host, unsigned char mode,
> unsigned short vdd);
>
[toc] | [prev] | [next] | [standalone]
| From | Ludovic Desroches <ludovic.desroches@atmel.com> |
|---|---|
| Date | 2016-04-06 17:10 +0200 |
| Message-ID | <rkXfI-5Ne-19@gated-at.bofh.it> |
| In reply to | #1372496 |
On Wed, Apr 06, 2016 at 03:37:28PM +0300, Adrian Hunter wrote:
> On 04/04/16 18:27, Ludovic Desroches wrote:
> > In order to remove the SDHCI_QUIRK2_NEED_DELAY_AFTER_INT_CLK_RST and to
> > reduce code duplication, put the code relative to the SD clock
> > configuration in a function which can be used by hosts for the
> > implementation of the set_clock() callback.
> >
> > Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
> > ---
> > drivers/mmc/host/sdhci.c | 33 ++++++++++++++++++++++-----------
> > drivers/mmc/host/sdhci.h | 1 +
> > 2 files changed, 23 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> > index 6bd3d17..2c3dede 100644
> > --- a/drivers/mmc/host/sdhci.c
> > +++ b/drivers/mmc/host/sdhci.c
> > @@ -1091,23 +1091,13 @@ static u16 sdhci_get_preset_value(struct sdhci_host *host)
> > return preset;
> > }
> >
> > -void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
> > +u16 sdhci_compute_clock_config(struct sdhci_host *host, unsigned int clock)
>
> The function name 'sdhci_compute_clock_config' seems a bit long. How about
> just sdhci_calc_clk.
Ok.
>
> Also it needs to calculate 'actual_clock' too, so it needs to do that and
> return the value.
>
actual_clock is updated at the end of the function.
Which value has to be returned? actual_clock or clock configuration?
Regards
Ludovic
> > {
> > int div = 0; /* Initialized for compiler warning */
> > int real_div = div, clk_mul = 1;
> > u16 clk = 0;
> > - unsigned long timeout;
> > bool switch_base_clk = false;
> >
> > - host->mmc->actual_clock = 0;
> > -
> > - sdhci_writew(host, 0, SDHCI_CLOCK_CONTROL);
> > - if (host->quirks2 & SDHCI_QUIRK2_NEED_DELAY_AFTER_INT_CLK_RST)
> > - mdelay(1);
> > -
> > - if (clock == 0)
> > - return;
> > -
> > if (host->version >= SDHCI_SPEC_300) {
> > if (host->preset_enabled) {
> > u16 pre_val;
> > @@ -1188,6 +1178,27 @@ clock_set:
> > clk |= (div & SDHCI_DIV_MASK) << SDHCI_DIVIDER_SHIFT;
> > clk |= ((div & SDHCI_DIV_HI_MASK) >> SDHCI_DIV_MASK_LEN)
> > << SDHCI_DIVIDER_HI_SHIFT;
> > +
> > + return clk;
> > +}
> > +EXPORT_SYMBOL_GPL(sdhci_compute_clock_config);
> > +
> > +void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
> > +{
> > + u16 clk;
> > + unsigned long timeout;
> > +
> > + host->mmc->actual_clock = 0;
> > +
> > + sdhci_writew(host, 0, SDHCI_CLOCK_CONTROL);
> > + if (host->quirks2 & SDHCI_QUIRK2_NEED_DELAY_AFTER_INT_CLK_RST)
> > + mdelay(1);
> > +
> > + if (clock == 0)
> > + return;
> > +
> > + clk = sdhci_compute_clock_config(host, clock);
> > +
> > clk |= SDHCI_CLOCK_INT_EN;
> > sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
> >
> > diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> > index 0f39f4f..23ddd6e 100644
> > --- a/drivers/mmc/host/sdhci.h
> > +++ b/drivers/mmc/host/sdhci.h
> > @@ -661,6 +661,7 @@ static inline bool sdhci_sdio_irq_enabled(struct sdhci_host *host)
> > return !!(host->flags & SDHCI_SDIO_IRQ_ENABLED);
> > }
> >
> > +u16 sdhci_compute_clock_config(struct sdhci_host *host, unsigned int clock);
> > void sdhci_set_clock(struct sdhci_host *host, unsigned int clock);
> > void sdhci_set_power(struct sdhci_host *host, unsigned char mode,
> > unsigned short vdd);
> >
>
[toc] | [prev] | [next] | [standalone]
| From | Ludovic Desroches <ludovic.desroches@atmel.com> |
|---|---|
| Date | 2016-04-06 17:20 +0200 |
| Message-ID | <rkXpn-5RF-19@gated-at.bofh.it> |
| In reply to | #1372591 |
On Wed, Apr 06, 2016 at 05:04:45PM +0200, Ludovic Desroches wrote:
> On Wed, Apr 06, 2016 at 03:37:28PM +0300, Adrian Hunter wrote:
> > On 04/04/16 18:27, Ludovic Desroches wrote:
> > > In order to remove the SDHCI_QUIRK2_NEED_DELAY_AFTER_INT_CLK_RST and to
> > > reduce code duplication, put the code relative to the SD clock
> > > configuration in a function which can be used by hosts for the
> > > implementation of the set_clock() callback.
> > >
> > > Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
> > > ---
> > > drivers/mmc/host/sdhci.c | 33 ++++++++++++++++++++++-----------
> > > drivers/mmc/host/sdhci.h | 1 +
> > > 2 files changed, 23 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> > > index 6bd3d17..2c3dede 100644
> > > --- a/drivers/mmc/host/sdhci.c
> > > +++ b/drivers/mmc/host/sdhci.c
> > > @@ -1091,23 +1091,13 @@ static u16 sdhci_get_preset_value(struct sdhci_host *host)
> > > return preset;
> > > }
> > >
> > > -void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
> > > +u16 sdhci_compute_clock_config(struct sdhci_host *host, unsigned int clock)
> >
> > The function name 'sdhci_compute_clock_config' seems a bit long. How about
> > just sdhci_calc_clk.
>
> Ok.
>
> >
> > Also it needs to calculate 'actual_clock' too, so it needs to do that and
> > return the value.
> >
>
> actual_clock is updated at the end of the function.
Maybe it makes more sens to keep it in sdhci_set_clock().
>
> Which value has to be returned? actual_clock or clock configuration?
>
> Regards
>
> Ludovic
>
> > > {
> > > int div = 0; /* Initialized for compiler warning */
> > > int real_div = div, clk_mul = 1;
> > > u16 clk = 0;
> > > - unsigned long timeout;
> > > bool switch_base_clk = false;
> > >
> > > - host->mmc->actual_clock = 0;
> > > -
> > > - sdhci_writew(host, 0, SDHCI_CLOCK_CONTROL);
> > > - if (host->quirks2 & SDHCI_QUIRK2_NEED_DELAY_AFTER_INT_CLK_RST)
> > > - mdelay(1);
> > > -
> > > - if (clock == 0)
> > > - return;
> > > -
> > > if (host->version >= SDHCI_SPEC_300) {
> > > if (host->preset_enabled) {
> > > u16 pre_val;
> > > @@ -1188,6 +1178,27 @@ clock_set:
> > > clk |= (div & SDHCI_DIV_MASK) << SDHCI_DIVIDER_SHIFT;
> > > clk |= ((div & SDHCI_DIV_HI_MASK) >> SDHCI_DIV_MASK_LEN)
> > > << SDHCI_DIVIDER_HI_SHIFT;
> > > +
> > > + return clk;
> > > +}
> > > +EXPORT_SYMBOL_GPL(sdhci_compute_clock_config);
> > > +
> > > +void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
> > > +{
> > > + u16 clk;
> > > + unsigned long timeout;
> > > +
> > > + host->mmc->actual_clock = 0;
> > > +
> > > + sdhci_writew(host, 0, SDHCI_CLOCK_CONTROL);
> > > + if (host->quirks2 & SDHCI_QUIRK2_NEED_DELAY_AFTER_INT_CLK_RST)
> > > + mdelay(1);
> > > +
> > > + if (clock == 0)
> > > + return;
> > > +
> > > + clk = sdhci_compute_clock_config(host, clock);
> > > +
> > > clk |= SDHCI_CLOCK_INT_EN;
> > > sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
> > >
> > > diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> > > index 0f39f4f..23ddd6e 100644
> > > --- a/drivers/mmc/host/sdhci.h
> > > +++ b/drivers/mmc/host/sdhci.h
> > > @@ -661,6 +661,7 @@ static inline bool sdhci_sdio_irq_enabled(struct sdhci_host *host)
> > > return !!(host->flags & SDHCI_SDIO_IRQ_ENABLED);
> > > }
> > >
> > > +u16 sdhci_compute_clock_config(struct sdhci_host *host, unsigned int clock);
> > > void sdhci_set_clock(struct sdhci_host *host, unsigned int clock);
> > > void sdhci_set_power(struct sdhci_host *host, unsigned char mode,
> > > unsigned short vdd);
> > >
> >
[toc] | [prev] | [next] | [standalone]
| From | Adrian Hunter <adrian.hunter@intel.com> |
|---|---|
| Date | 2016-04-07 10:00 +0200 |
| Message-ID | <rld19-xm-21@gated-at.bofh.it> |
| In reply to | #1372591 |
On 06/04/16 18:04, Ludovic Desroches wrote:
> On Wed, Apr 06, 2016 at 03:37:28PM +0300, Adrian Hunter wrote:
>> On 04/04/16 18:27, Ludovic Desroches wrote:
>>> In order to remove the SDHCI_QUIRK2_NEED_DELAY_AFTER_INT_CLK_RST and to
>>> reduce code duplication, put the code relative to the SD clock
>>> configuration in a function which can be used by hosts for the
>>> implementation of the set_clock() callback.
>>>
>>> Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
>>> ---
>>> drivers/mmc/host/sdhci.c | 33 ++++++++++++++++++++++-----------
>>> drivers/mmc/host/sdhci.h | 1 +
>>> 2 files changed, 23 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>>> index 6bd3d17..2c3dede 100644
>>> --- a/drivers/mmc/host/sdhci.c
>>> +++ b/drivers/mmc/host/sdhci.c
>>> @@ -1091,23 +1091,13 @@ static u16 sdhci_get_preset_value(struct sdhci_host *host)
>>> return preset;
>>> }
>>>
>>> -void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
>>> +u16 sdhci_compute_clock_config(struct sdhci_host *host, unsigned int clock)
>>
>> The function name 'sdhci_compute_clock_config' seems a bit long. How about
>> just sdhci_calc_clk.
>
> Ok.
>
>>
>> Also it needs to calculate 'actual_clock' too, so it needs to do that and
>> return the value.
>>
>
> actual_clock is updated at the end of the function.
>
> Which value has to be returned? actual_clock or clock configuration?
The function doesn't update the clock, so I don't think it should update
actual_clock either, just return it. i.e.
u16 sdhci_calc_clk(struct sdhci_host *host, unsigned int clock, unsigned int *actual_clock)
{
...
if (real_div)
*actual_clock = (host->max_clk * clk_mul) / real_div;
...
return clk;
}
then
clk = sdhci_calc_clk(host, clock, &host->mmc->actual_clock);
>
> Regards
>
> Ludovic
>
>>> {
>>> int div = 0; /* Initialized for compiler warning */
>>> int real_div = div, clk_mul = 1;
>>> u16 clk = 0;
>>> - unsigned long timeout;
>>> bool switch_base_clk = false;
>>>
>>> - host->mmc->actual_clock = 0;
>>> -
>>> - sdhci_writew(host, 0, SDHCI_CLOCK_CONTROL);
>>> - if (host->quirks2 & SDHCI_QUIRK2_NEED_DELAY_AFTER_INT_CLK_RST)
>>> - mdelay(1);
>>> -
>>> - if (clock == 0)
>>> - return;
>>> -
>>> if (host->version >= SDHCI_SPEC_300) {
>>> if (host->preset_enabled) {
>>> u16 pre_val;
>>> @@ -1188,6 +1178,27 @@ clock_set:
>>> clk |= (div & SDHCI_DIV_MASK) << SDHCI_DIVIDER_SHIFT;
>>> clk |= ((div & SDHCI_DIV_HI_MASK) >> SDHCI_DIV_MASK_LEN)
>>> << SDHCI_DIVIDER_HI_SHIFT;
>>> +
>>> + return clk;
>>> +}
>>> +EXPORT_SYMBOL_GPL(sdhci_compute_clock_config);
>>> +
>>> +void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
>>> +{
>>> + u16 clk;
>>> + unsigned long timeout;
>>> +
>>> + host->mmc->actual_clock = 0;
>>> +
>>> + sdhci_writew(host, 0, SDHCI_CLOCK_CONTROL);
>>> + if (host->quirks2 & SDHCI_QUIRK2_NEED_DELAY_AFTER_INT_CLK_RST)
>>> + mdelay(1);
>>> +
>>> + if (clock == 0)
>>> + return;
>>> +
>>> + clk = sdhci_compute_clock_config(host, clock);
>>> +
>>> clk |= SDHCI_CLOCK_INT_EN;
>>> sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
>>>
>>> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
>>> index 0f39f4f..23ddd6e 100644
>>> --- a/drivers/mmc/host/sdhci.h
>>> +++ b/drivers/mmc/host/sdhci.h
>>> @@ -661,6 +661,7 @@ static inline bool sdhci_sdio_irq_enabled(struct sdhci_host *host)
>>> return !!(host->flags & SDHCI_SDIO_IRQ_ENABLED);
>>> }
>>>
>>> +u16 sdhci_compute_clock_config(struct sdhci_host *host, unsigned int clock);
>>> void sdhci_set_clock(struct sdhci_host *host, unsigned int clock);
>>> void sdhci_set_power(struct sdhci_host *host, unsigned char mode,
>>> unsigned short vdd);
>>>
>>
>
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web