diff mbox

[05/12] mmc: sdhci: add a pre voltage switch callback function

Message ID 1465456218-28354-6-git-send-email-gregory.clement@free-electrons.com (mailing list archive)
State New, archived
Headers show

Commit Message

Gregory CLEMENT June 9, 2016, 7:10 a.m. UTC 
From: Victor Gu <xigu@marvell.com>

Some host controller such as Xenon needs additional setting when
switching signal voltage in eMMC mode. They also need to re-enable
internal clock before a voltage switch.

This commit adds a callback routine "voltage_switch_pre" in the struct
sdhci_ops, which is used by some host controllers which need re-enable
the internal clock before a voltage switch.

[gregory.clement@free-electrons.com: split the initial commit and
reformulate the log]

Signed-off-by: Victor Gu <xigu@marvell.com>
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 drivers/mmc/host/sdhci.c | 4 ++++
 drivers/mmc/host/sdhci.h | 1 +
 2 files changed, 5 insertions(+)

Comments

Adrian Hunter June 13, 2016, 8:46 a.m. UTC | #1 
On 09/06/16 10:10, Gregory CLEMENT wrote:
> From: Victor Gu <xigu@marvell.com>
> 
> Some host controller such as Xenon needs additional setting when
> switching signal voltage in eMMC mode. They also need to re-enable
> internal clock before a voltage switch.
> 
> This commit adds a callback routine "voltage_switch_pre" in the struct
> sdhci_ops, which is used by some host controllers which need re-enable
> the internal clock before a voltage switch.

Don't want to add sdhci host ops for the "do something before a mmc host op"
case.

Instead, export sdhci_start_signal_voltage_switch() and  hook
host->mmc_host_ops.start_signal_voltage_switch. Then in sdhci-xenon.c:

int xenon__start_signal_voltage_switch(struct mmc_host *mmc,
				       struct mmc_ios *ios)
{
	blah blah
	return sdhci_start_signal_voltage_switch(mmc, ios);
}


> 
> [gregory.clement@free-electrons.com: split the initial commit and
> reformulate the log]
> 
> Signed-off-by: Victor Gu <xigu@marvell.com>
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> ---
>  drivers/mmc/host/sdhci.c | 4 ++++
>  drivers/mmc/host/sdhci.h | 1 +
>  2 files changed, 5 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index b07219426d39..cad03ffa9d9b 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -1729,6 +1729,10 @@ static int sdhci_start_signal_voltage_switch(struct mmc_host *mmc,
>  	if (host->version < SDHCI_SPEC_300)
>  		return 0;
>  
> +	/* Some controller need to do more before switching */
> +	if (host->ops->voltage_switch_pre)
> +		host->ops->voltage_switch_pre(host);
> +
>  	ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
>  
>  	switch (ios->signal_voltage) {
> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> index 49c6c5b0e33b..6bec1b0368d2 100644
> --- a/drivers/mmc/host/sdhci.h
> +++ b/drivers/mmc/host/sdhci.h
> @@ -550,6 +550,7 @@ struct sdhci_ops {
>  					 unsigned int max_dtr, int host_drv,
>  					 int card_drv, int *drv_type);
>  	void	(*init_card)(struct sdhci_host *host, struct mmc_card *card);
> +	void	(*voltage_switch_pre)(struct sdhci_host *host);
>  };
>  
>  #ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS
> 

--
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
Hu Ziji June 13, 2016, 2:04 p.m. UTC | #2 
SGkgQWRyaWFuLA0KDQogIFRoYW5rcyBhIGxvdCBmb3IgdGhlIHN1Z2dlc3Rpb24uIEknbSBmcm9t
IE1hcnZlbGwgWGVub24gU0RIQyB0ZWFtLg0KICBNb3N0IG9mIHNkaGNpIGhvc3Qgb3BzIGFyZSBz
dGlsbCB1c2VkIGluIG91ciBkcml2ZXIuIFdlIGp1c3QgY2hhbmdlIG9yIGFkZCBmZXcgb3BzLiBU
aHVzLCBpZiB3ZSBjcmVhdGUgb3VyIG93biBtbWNfaG9zdF9vcHMsIG1hbnkgc3RhdGljIHNkaGNp
IGhvc3Qgb3BzIHdpbGwgYmUgZXhwb3J0ZWQuIEkgd291bGQgbGlrZSB0byBrbm93IHdoZXRoZXIg
c3VjaCBhIG1vZGlmaWNhdGlvbiBjYW4gYmUgYWNjZXB0ZWQgb3Igbm90Lg0KDQogICBJIG5vdGlj
ZSB0aGF0IGNvbW11bml0eSBpcyBpbXByb3Zpbmcgc2RoY2kuIFRodXMgYWxsIHRoZSBzdGF0aWMg
c2RoY2kgb3BzIGNhbGxlZCBvdXRzaWRlIHRoZSBzZGhjaS5jIHdpbGwgYmUgZXhwb3J0ZWQuIElz
IG15IHVuZGVyc3RhbmRpbmcgY29ycmVjdD8NCiAgIFRoYW5rIHlvdS4NCg0KQmVzdCByZWdhcmRz
LA0KSHUgWmlqaQ0KDQo+PiDU2iAyMDE2xOo21MIxM8jVo6wxNjo1MqOsQWRyaWFuIEh1bnRlciA8
YWRyaWFuLmh1bnRlckBpbnRlbC5jb20+INC0tcCjug0KPj4gDQo+PiBPbiAwOS8wNi8xNiAxMDox
MCwgR3JlZ29yeSBDTEVNRU5UIHdyb3RlOg0KPj4gRnJvbTogVmljdG9yIEd1IDx4aWd1QG1hcnZl
bGwuY29tPg0KPj4gDQo+PiBTb21lIGhvc3QgY29udHJvbGxlciBzdWNoIGFzIFhlbm9uIG5lZWRz
IGFkZGl0aW9uYWwgc2V0dGluZyB3aGVuDQo+PiBzd2l0Y2hpbmcgc2lnbmFsIHZvbHRhZ2UgaW4g
ZU1NQyBtb2RlLiBUaGV5IGFsc28gbmVlZCB0byByZS1lbmFibGUNCj4+IGludGVybmFsIGNsb2Nr
IGJlZm9yZSBhIHZvbHRhZ2Ugc3dpdGNoLg0KPj4gDQo+PiBUaGlzIGNvbW1pdCBhZGRzIGEgY2Fs
bGJhY2sgcm91dGluZSAidm9sdGFnZV9zd2l0Y2hfcHJlIiBpbiB0aGUgc3RydWN0DQo+PiBzZGhj
aV9vcHMsIHdoaWNoIGlzIHVzZWQgYnkgc29tZSBob3N0IGNvbnRyb2xsZXJzIHdoaWNoIG5lZWQg
cmUtZW5hYmxlDQo+PiB0aGUgaW50ZXJuYWwgY2xvY2sgYmVmb3JlIGEgdm9sdGFnZSBzd2l0Y2gu
DQo+IA0KPiBEb24ndCB3YW50IHRvIGFkZCBzZGhjaSBob3N0IG9wcyBmb3IgdGhlICJkbyBzb21l
dGhpbmcgYmVmb3JlIGEgbW1jIGhvc3Qgb3AiDQo+IGNhc2UuDQo+IA0KPiBJbnN0ZWFkLCBleHBv
cnQgc2RoY2lfc3RhcnRfc2lnbmFsX3ZvbHRhZ2Vfc3dpdGNoKCkgYW5kICBob29rDQo+IGhvc3Qt
Pm1tY19ob3N0X29wcy5zdGFydF9zaWduYWxfdm9sdGFnZV9zd2l0Y2guIFRoZW4gaW4gc2RoY2kt
eGVub24uYzoNCj4gDQo+IGludCB4ZW5vbl9fc3RhcnRfc2lnbmFsX3ZvbHRhZ2Vfc3dpdGNoKHN0
cnVjdCBtbWNfaG9zdCAqbW1jLA0KPiAgICAgICAgICAgICAgICAgICAgICBzdHJ1Y3QgbW1jX2lv
cyAqaW9zKQ0KPiB7DQo+ICAgYmxhaCBibGFoDQo+ICAgcmV0dXJuIHNkaGNpX3N0YXJ0X3NpZ25h
bF92b2x0YWdlX3N3aXRjaChtbWMsIGlvcyk7DQo+IH0NCj4gDQo+IA0KPj4gDQo+PiBbZ3JlZ29y
eS5jbGVtZW50QGZyZWUtZWxlY3Ryb25zLmNvbTogc3BsaXQgdGhlIGluaXRpYWwgY29tbWl0IGFu
ZA0KPj4gcmVmb3JtdWxhdGUgdGhlIGxvZ10NCj4+IA0KPj4gU2lnbmVkLW9mZi1ieTogVmljdG9y
IEd1IDx4aWd1QG1hcnZlbGwuY29tPg0KPj4gU2lnbmVkLW9mZi1ieTogTWFyY2luIFdvanRhcyA8
bXdAc2VtaWhhbGYuY29tPg0KPj4gU2lnbmVkLW9mZi1ieTogR3JlZ29yeSBDTEVNRU5UIDxncmVn
b3J5LmNsZW1lbnRAZnJlZS1lbGVjdHJvbnMuY29tPg0KPj4gLS0tDQo+PiBkcml2ZXJzL21tYy9o
b3N0L3NkaGNpLmMgfCA0ICsrKysNCj4+IGRyaXZlcnMvbW1jL2hvc3Qvc2RoY2kuaCB8IDEgKw0K
Pj4gMiBmaWxlcyBjaGFuZ2VkLCA1IGluc2VydGlvbnMoKykNCj4+IA0KPj4gZGlmZiAtLWdpdCBh
L2RyaXZlcnMvbW1jL2hvc3Qvc2RoY2kuYyBiL2RyaXZlcnMvbW1jL2hvc3Qvc2RoY2kuYw0KPj4g
aW5kZXggYjA3MjE5NDI2ZDM5Li5jYWQwM2ZmYTlkOWIgMTAwNjQ0DQo+PiAtLS0gYS9kcml2ZXJz
L21tYy9ob3N0L3NkaGNpLmMNCj4+ICsrKyBiL2RyaXZlcnMvbW1jL2hvc3Qvc2RoY2kuYw0KPj4g
QEAgLTE3MjksNiArMTcyOSwxMCBAQCBzdGF0aWMgaW50IHNkaGNpX3N0YXJ0X3NpZ25hbF92b2x0
YWdlX3N3aXRjaChzdHJ1Y3QgbW1jX2hvc3QgKm1tYywNCj4+ICAgaWYgKGhvc3QtPnZlcnNpb24g
PCBTREhDSV9TUEVDXzMwMCkNCj4+ICAgICAgIHJldHVybiAwOw0KPj4gDQo+PiArICAgIC8qIFNv
bWUgY29udHJvbGxlciBuZWVkIHRvIGRvIG1vcmUgYmVmb3JlIHN3aXRjaGluZyAqLw0KPj4gKyAg
ICBpZiAoaG9zdC0+b3BzLT52b2x0YWdlX3N3aXRjaF9wcmUpDQo+PiArICAgICAgICBob3N0LT5v
cHMtPnZvbHRhZ2Vfc3dpdGNoX3ByZShob3N0KTsNCj4+ICsNCj4+ICAgY3RybCA9IHNkaGNpX3Jl
YWR3KGhvc3QsIFNESENJX0hPU1RfQ09OVFJPTDIpOw0KPj4gDQo+PiAgIHN3aXRjaCAoaW9zLT5z
aWduYWxfdm9sdGFnZSkgew0KPj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvbW1jL2hvc3Qvc2RoY2ku
aCBiL2RyaXZlcnMvbW1jL2hvc3Qvc2RoY2kuaA0KPj4gaW5kZXggNDljNmM1YjBlMzNiLi42YmVj
MWIwMzY4ZDIgMTAwNjQ0DQo+PiAtLS0gYS9kcml2ZXJzL21tYy9ob3N0L3NkaGNpLmgNCj4+ICsr
KyBiL2RyaXZlcnMvbW1jL2hvc3Qvc2RoY2kuaA0KPj4gQEAgLTU1MCw2ICs1NTAsNyBAQCBzdHJ1
Y3Qgc2RoY2lfb3BzIHsNCj4+ICAgICAgICAgICAgICAgICAgICB1bnNpZ25lZCBpbnQgbWF4X2R0
ciwgaW50IGhvc3RfZHJ2LA0KPj4gICAgICAgICAgICAgICAgICAgIGludCBjYXJkX2RydiwgaW50
ICpkcnZfdHlwZSk7DQo+PiAgIHZvaWQgICAgKCppbml0X2NhcmQpKHN0cnVjdCBzZGhjaV9ob3N0
ICpob3N0LCBzdHJ1Y3QgbW1jX2NhcmQgKmNhcmQpOw0KPj4gKyAgICB2b2lkICAgICgqdm9sdGFn
ZV9zd2l0Y2hfcHJlKShzdHJ1Y3Qgc2RoY2lfaG9zdCAqaG9zdCk7DQo+PiB9Ow0KPj4gDQo+PiAj
aWZkZWYgQ09ORklHX01NQ19TREhDSV9JT19BQ0NFU1NPUlMNCj4gDQo+IC0tDQo+IFRvIHVuc3Vi
c2NyaWJlIGZyb20gdGhpcyBsaXN0OiBzZW5kIHRoZSBsaW5lICJ1bnN1YnNjcmliZSBsaW51eC1t
bWMiIGluDQo+IHRoZSBib2R5IG9mIGEgbWVzc2FnZSB0byBtYWpvcmRvbW9Admdlci5rZXJuZWwu
b3JnDQo+IE1vcmUgbWFqb3Jkb21vIGluZm8gYXQgIGh0dHA6Ly92Z2VyLmtlcm5lbC5vcmcvbWFq
b3Jkb21vLWluZm8uaHRtbA0K
--
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
Adrian Hunter June 14, 2016, 7:07 a.m. UTC | #3 
On 13/06/16 17:04, Ziji Hu wrote:
> Hi Adrian,
> 
> Thanks a lot for the suggestion. I'm from Marvell Xenon SDHC team. Most
> of sdhci host ops are still used in our driver. We just change or add few
> ops. Thus, if we create our own mmc_host_ops, many static sdhci host ops
> will be exported. I would like to know whether such a modification can be
> accepted or not.
> 
> I notice that community is improving sdhci. Thus all the static sdhci ops
> called outside the sdhci.c will be exported. Is my understanding
> correct? Thank you.

The aim is to make sdhci more like a library and give more control to the
drivers.  That will mean more sdhci functions get exported, particularly mmc
host ops like sdhci_set_ios() or sdhci_start_signal_voltage_switch().

> 
> Best regards, Hu Ziji
> 
>>> 在 2016年6月13日，16:52，Adrian Hunter <adrian.hunter@intel.com> 写道：
>>> 
>>> On 09/06/16 10:10, Gregory CLEMENT wrote: From: Victor Gu
>>> <xigu@marvell.com>
>>> 
>>> Some host controller such as Xenon needs additional setting when 
>>> switching signal voltage in eMMC mode. They also need to re-enable 
>>> internal clock before a voltage switch.
>>> 
>>> This commit adds a callback routine "voltage_switch_pre" in the
>>> struct sdhci_ops, which is used by some host controllers which need
>>> re-enable the internal clock before a voltage switch.
>> 
>> Don't want to add sdhci host ops for the "do something before a mmc
>> host op" case.
>> 
>> Instead, export sdhci_start_signal_voltage_switch() and  hook 
>> host->mmc_host_ops.start_signal_voltage_switch. Then in sdhci-xenon.c:
>> 
>> int xenon__start_signal_voltage_switch(struct mmc_host *mmc, struct
>> mmc_ios *ios) { blah blah return sdhci_start_signal_voltage_switch(mmc,
>> ios); }
>> 
>> 
>>> 
>>> [gregory.clement@free-electrons.com: split the initial commit and 
>>> reformulate the log]
>>> 
>>> Signed-off-by: Victor Gu <xigu@marvell.com> Signed-off-by: Marcin
>>> Wojtas <mw@semihalf.com> Signed-off-by: Gregory CLEMENT
>>> <gregory.clement@free-electrons.com> --- drivers/mmc/host/sdhci.c | 4
>>> ++++ drivers/mmc/host/sdhci.h | 1 + 2 files changed, 5 insertions(+)
>>> 
>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c 
>>> index b07219426d39..cad03ffa9d9b 100644 ---
>>> a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -1729,6
>>> +1729,10 @@ static int sdhci_start_signal_voltage_switch(struct
>>> mmc_host *mmc, if (host->version < SDHCI_SPEC_300) return 0;
>>> 
>>> +    /* Some controller need to do more before switching */ +    if
>>> (host->ops->voltage_switch_pre) +
>>> host->ops->voltage_switch_pre(host); + ctrl = sdhci_readw(host,
>>> SDHCI_HOST_CONTROL2);
>>> 
>>> switch (ios->signal_voltage) { diff --git a/drivers/mmc/host/sdhci.h
>>> b/drivers/mmc/host/sdhci.h index 49c6c5b0e33b..6bec1b0368d2 100644 
>>> --- a/drivers/mmc/host/sdhci.h +++ b/drivers/mmc/host/sdhci.h @@
>>> -550,6 +550,7 @@ struct sdhci_ops { unsigned int max_dtr, int
>>> host_drv, int card_drv, int *drv_type); void    (*init_card)(struct
>>> sdhci_host *host, struct mmc_card *card); +    void
>>> (*voltage_switch_pre)(struct sdhci_host *host); };
>>> 
>>> #ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS
>> 
>> -- 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

--
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
Gregory CLEMENT June 14, 2016, 7:59 a.m. UTC | #4 
Hi Adrian,
 
 On lun., juin 13 2016, Adrian Hunter <adrian.hunter@intel.com> wrote:

> On 09/06/16 10:10, Gregory CLEMENT wrote:
>> From: Victor Gu <xigu@marvell.com>
>> 
>> Some host controller such as Xenon needs additional setting when
>> switching signal voltage in eMMC mode. They also need to re-enable
>> internal clock before a voltage switch.
>> 
>> This commit adds a callback routine "voltage_switch_pre" in the struct
>> sdhci_ops, which is used by some host controllers which need re-enable
>> the internal clock before a voltage switch.
>
> Don't want to add sdhci host ops for the "do something before a mmc host op"
> case.
>
> Instead, export sdhci_start_signal_voltage_switch() and  hook
> host->mmc_host_ops.start_signal_voltage_switch. Then in sdhci-xenon.c:
>
> int xenon__start_signal_voltage_switch(struct mmc_host *mmc,
> 				       struct mmc_ios *ios)
> {
> 	blah blah
> 	return sdhci_start_signal_voltage_switch(mmc, ios);
> }

OK I see your point.

Thanks,

Gregory

>
>
>> 
>> [gregory.clement@free-electrons.com: split the initial commit and
>> reformulate the log]
>> 
>> Signed-off-by: Victor Gu <xigu@marvell.com>
>> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>> ---
>>  drivers/mmc/host/sdhci.c | 4 ++++
>>  drivers/mmc/host/sdhci.h | 1 +
>>  2 files changed, 5 insertions(+)
>> 
>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>> index b07219426d39..cad03ffa9d9b 100644
>> --- a/drivers/mmc/host/sdhci.c
>> +++ b/drivers/mmc/host/sdhci.c
>> @@ -1729,6 +1729,10 @@ static int sdhci_start_signal_voltage_switch(struct mmc_host *mmc,
>>  	if (host->version < SDHCI_SPEC_300)
>>  		return 0;
>>  
>> +	/* Some controller need to do more before switching */
>> +	if (host->ops->voltage_switch_pre)
>> +		host->ops->voltage_switch_pre(host);
>> +
>>  	ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
>>  
>>  	switch (ios->signal_voltage) {
>> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
>> index 49c6c5b0e33b..6bec1b0368d2 100644
>> --- a/drivers/mmc/host/sdhci.h
>> +++ b/drivers/mmc/host/sdhci.h
>> @@ -550,6 +550,7 @@ struct sdhci_ops {
>>  					 unsigned int max_dtr, int host_drv,
>>  					 int card_drv, int *drv_type);
>>  	void	(*init_card)(struct sdhci_host *host, struct mmc_card *card);
>> +	void	(*voltage_switch_pre)(struct sdhci_host *host);
>>  };
>>  
>>  #ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS
>> 
>
diff mbox

Patch

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index b07219426d39..cad03ffa9d9b 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1729,6 +1729,10 @@  static int sdhci_start_signal_voltage_switch(struct mmc_host *mmc,
 	if (host->version < SDHCI_SPEC_300)
 		return 0;
 
+	/* Some controller need to do more before switching */
+	if (host->ops->voltage_switch_pre)
+		host->ops->voltage_switch_pre(host);
+
 	ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
 
 	switch (ios->signal_voltage) {
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 49c6c5b0e33b..6bec1b0368d2 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -550,6 +550,7 @@  struct sdhci_ops {
 					 unsigned int max_dtr, int host_drv,
 					 int card_drv, int *drv_type);
 	void	(*init_card)(struct sdhci_host *host, struct mmc_card *card);
+	void	(*voltage_switch_pre)(struct sdhci_host *host);
 };
 
 #ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS