| Message ID | 1446825570-30029-2-git-send-email-ludovic.desroches@atmel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Nov 06, 2015 at 04:59:29PM +0100, Ludovic Desroches wrote: > When there is a vmmc regulator, only SD Bus Power is set to 1 in the > Power Control Register. It means SD Bus Voltage Select field is set to 0 > that is a reserved value. The SD Host Controller specification says: > 'SD Bus Power: Before setting this bit, the SD Host Driver shall set SD > Bus Voltage Select.' and 'If the Host Driver selects an unsupported > voltage in the SD B?us Voltage Select field, the Host Controller may > ignore writes to SD Bus Power and keep its value at zero." > > Having an external regulator means the SD Bus Voltage Select is useless > but if the Host Controller strictly follows the specification then we > need to set a valid value. Ulf, What is your opinion about this patch? If the 'no regulator found' message is turned in debug message, I can get rid of my vmmc regulator but I really think that writing only SDHCI_POWER_ON is opposite to the sdhci spec. I would say that not setting the bus voltage is a quirk! Regards Ludovic > > Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com> > --- > drivers/mmc/host/sdhci.c | 7 ------- > 1 file changed, 7 deletions(-) > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > index ac97b46..0cfd7b2 100644 > --- a/drivers/mmc/host/sdhci.c > +++ b/drivers/mmc/host/sdhci.c > @@ -1278,13 +1278,6 @@ static void sdhci_set_power(struct sdhci_host *host, unsigned char mode, > spin_unlock_irq(&host->lock); > mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd); > spin_lock_irq(&host->lock); > - > - if (mode != MMC_POWER_OFF) > - sdhci_writeb(host, SDHCI_POWER_ON, SDHCI_POWER_CONTROL); > - else > - sdhci_writeb(host, 0, SDHCI_POWER_CONTROL); > - > - return; > } > > if (mode != MMC_POWER_OFF) { > -- > 2.5.0 > -- 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
On 9 November 2015 at 14:23, Ludovic Desroches <ludovic.desroches@atmel.com> wrote: > On Fri, Nov 06, 2015 at 04:59:29PM +0100, Ludovic Desroches wrote: >> When there is a vmmc regulator, only SD Bus Power is set to 1 in the >> Power Control Register. It means SD Bus Voltage Select field is set to 0 >> that is a reserved value. The SD Host Controller specification says: >> 'SD Bus Power: Before setting this bit, the SD Host Driver shall set SD >> Bus Voltage Select.' and 'If the Host Driver selects an unsupported >> voltage in the SD B?us Voltage Select field, the Host Controller may >> ignore writes to SD Bus Power and keep its value at zero." >> >> Having an external regulator means the SD Bus Voltage Select is useless >> but if the Host Controller strictly follows the specification then we >> need to set a valid value. > > Ulf, > > What is your opinion about this patch? > > If the 'no regulator found' message is turned in debug message, I can get > rid of my vmmc regulator but I really think that writing only I expect you mean vqmmc? > SDHCI_POWER_ON is opposite to the sdhci spec. I would say that not > setting the bus voltage is a quirk! I don't really follow. I read the SDHCI spec and the section for the Power Control Register. Bit 0 needs to be set when communicating with the card as it will for example enables the clock. Before setting bit0 you must decide what signal level to use, which is done by writing to bit 1->3. If SDHCI monitors the power state (MMC_POWER_UP|ON|OFF) when its ->set_ios() callback are invoked and in combination of using the ->start_signal_voltage_switch() callback to change the signal voltage level, this *should* work out nicely. Now, looking at the related code in sdhci, I am kind of surprised that it works. :-) Though, again I don't have the in-depth knowledge about sdhci. Kind regards Uffe -- 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
On Mon, Nov 09, 2015 at 03:12:46PM +0100, Ulf Hansson wrote: > On 9 November 2015 at 14:23, Ludovic Desroches > <ludovic.desroches@atmel.com> wrote: > > On Fri, Nov 06, 2015 at 04:59:29PM +0100, Ludovic Desroches wrote: > >> When there is a vmmc regulator, only SD Bus Power is set to 1 in the > >> Power Control Register. It means SD Bus Voltage Select field is set to 0 > >> that is a reserved value. The SD Host Controller specification says: > >> 'SD Bus Power: Before setting this bit, the SD Host Driver shall set SD > >> Bus Voltage Select.' and 'If the Host Driver selects an unsupported > >> voltage in the SD B?us Voltage Select field, the Host Controller may > >> ignore writes to SD Bus Power and keep its value at zero." > >> > >> Having an external regulator means the SD Bus Voltage Select is useless > >> but if the Host Controller strictly follows the specification then we > >> need to set a valid value. > > > > Ulf, > > > > What is your opinion about this patch? > > > > If the 'no regulator found' message is turned in debug message, I can get > > rid of my vmmc regulator but I really think that writing only > > I expect you mean vqmmc? I don't mean vmmc. In the sdhci_set_power function, we are using vmmc. I feel not confortable with it because the power control register contains 'SD Bus' fields so it should depend on vqmmc not vmmc. > > > SDHCI_POWER_ON is opposite to the sdhci spec. I would say that not > > setting the bus voltage is a quirk! > > I don't really follow. > > I read the SDHCI spec and the section for the Power Control Register. > Bit 0 needs to be set when communicating with the card as it will for > example enables the clock. Before setting bit0 you must decide what > signal level to use, which is done by writing to bit 1->3. > Right. But when having vmmc supply we do: sdhci_writeb(host, SDHCI_POWER_ON, SDHCI_POWER_CONTROL) or sdhci_writeb(host, 0, SDHCI_POWER_CONTROL) so we loose the signal level, isn't it? > If SDHCI monitors the power state (MMC_POWER_UP|ON|OFF) when its > ->set_ios() callback are invoked and in combination of using the > ->start_signal_voltage_switch() callback to change the signal voltage > level, this *should* work out nicely. > It is my turn to not follow! We write into the Power Control Register only in sdhci_set_power(). May I miss a callback or something else? sdhci_do_start_signal_voltage_switch doesn't modify the Power Control Register. > Now, looking at the related code in sdhci, I am kind of surprised that > it works. :-) Though, again I don't have the in-depth knowledge about > sdhci. > Me too, I am starting to dig into the sdhci spec and some points are not crystal clear. Regards Ludovic -- 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
On 9 November 2015 at 15:40, Ludovic Desroches <ludovic.desroches@atmel.com> wrote: > On Mon, Nov 09, 2015 at 03:12:46PM +0100, Ulf Hansson wrote: >> On 9 November 2015 at 14:23, Ludovic Desroches >> <ludovic.desroches@atmel.com> wrote: >> > On Fri, Nov 06, 2015 at 04:59:29PM +0100, Ludovic Desroches wrote: >> >> When there is a vmmc regulator, only SD Bus Power is set to 1 in the >> >> Power Control Register. It means SD Bus Voltage Select field is set to 0 >> >> that is a reserved value. The SD Host Controller specification says: >> >> 'SD Bus Power: Before setting this bit, the SD Host Driver shall set SD >> >> Bus Voltage Select.' and 'If the Host Driver selects an unsupported >> >> voltage in the SD B?us Voltage Select field, the Host Controller may >> >> ignore writes to SD Bus Power and keep its value at zero." >> >> >> >> Having an external regulator means the SD Bus Voltage Select is useless >> >> but if the Host Controller strictly follows the specification then we >> >> need to set a valid value. >> > >> > Ulf, >> > >> > What is your opinion about this patch? >> > >> > If the 'no regulator found' message is turned in debug message, I can get >> > rid of my vmmc regulator but I really think that writing only >> >> I expect you mean vqmmc? > > I don't mean vmmc. In the sdhci_set_power function, we are using vmmc. > I feel not confortable with it because the power control register > contains 'SD Bus' fields so it should depend on vqmmc not vmmc. > >> >> > SDHCI_POWER_ON is opposite to the sdhci spec. I would say that not >> > setting the bus voltage is a quirk! >> >> I don't really follow. >> >> I read the SDHCI spec and the section for the Power Control Register. >> Bit 0 needs to be set when communicating with the card as it will for >> example enables the clock. Before setting bit0 you must decide what >> signal level to use, which is done by writing to bit 1->3. >> > > Right. But when having vmmc supply we do: > sdhci_writeb(host, SDHCI_POWER_ON, SDHCI_POWER_CONTROL) or > sdhci_writeb(host, 0, SDHCI_POWER_CONTROL) so we loose the signal level, > isn't it? > >> If SDHCI monitors the power state (MMC_POWER_UP|ON|OFF) when its >> ->set_ios() callback are invoked and in combination of using the >> ->start_signal_voltage_switch() callback to change the signal voltage >> level, this *should* work out nicely. >> > > It is my turn to not follow! We write into the Power Control Register > only in sdhci_set_power(). May I miss a callback or something else? > sdhci_do_start_signal_voltage_switch doesn't modify the Power Control > Register. > >> Now, looking at the related code in sdhci, I am kind of surprised that >> it works. :-) Though, again I don't have the in-depth knowledge about >> sdhci. >> > > Me too, I am starting to dig into the sdhci spec and some points are > not crystal clear. > Okay, I am finally starting to understand some of your concern. According to the spec, the Power Control Register should control the signal voltage/bus voltage. As UHS mode was added to the spec, it seems like the Power Control Register couldn't cover all new cases, as why Host Control 2 register needed to be added. The Host Control 2 register, is what sdhci_do_start_signal_voltage_switch() uses to change the signal voltage level, which all makes sense to me. For sdhci_set_power(); it seems to use the Power Control Register to control the power to the card (VDD/VMMC). Indeed this looks *really* weird/wrong. I wonder if it's working because of luck, intentional violation of the SDHCI spec or because of special variants. Especially when looking into the case when you *don't* have a VMMC regulator several strange quirks exists in sdhci_set_power(). In the case when you *have* a VMMC, I think just setting/clearing bit 0 (SDHCI_POWER_ON) and then bail out, is probably working with modern HW because it's likely the only thing needed. Now, this discussion was interesting, but I forgot what problem you actually where trying to solve? :-) Kind regards Uffe -- 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
On Mon, Nov 09, 2015 at 05:00:46PM +0100, Ulf Hansson wrote: > On 9 November 2015 at 15:40, Ludovic Desroches > <ludovic.desroches@atmel.com> wrote: > > On Mon, Nov 09, 2015 at 03:12:46PM +0100, Ulf Hansson wrote: > >> On 9 November 2015 at 14:23, Ludovic Desroches > >> <ludovic.desroches@atmel.com> wrote: > >> > On Fri, Nov 06, 2015 at 04:59:29PM +0100, Ludovic Desroches wrote: > >> >> When there is a vmmc regulator, only SD Bus Power is set to 1 in the > >> >> Power Control Register. It means SD Bus Voltage Select field is set to 0 > >> >> that is a reserved value. The SD Host Controller specification says: > >> >> 'SD Bus Power: Before setting this bit, the SD Host Driver shall set SD > >> >> Bus Voltage Select.' and 'If the Host Driver selects an unsupported > >> >> voltage in the SD B?us Voltage Select field, the Host Controller may > >> >> ignore writes to SD Bus Power and keep its value at zero." > >> >> > >> >> Having an external regulator means the SD Bus Voltage Select is useless > >> >> but if the Host Controller strictly follows the specification then we > >> >> need to set a valid value. > >> > > >> > Ulf, > >> > > >> > What is your opinion about this patch? > >> > > >> > If the 'no regulator found' message is turned in debug message, I can get > >> > rid of my vmmc regulator but I really think that writing only > >> > >> I expect you mean vqmmc? > > > > I don't mean vmmc. In the sdhci_set_power function, we are using vmmc. > > I feel not confortable with it because the power control register > > contains 'SD Bus' fields so it should depend on vqmmc not vmmc. > > > >> > >> > SDHCI_POWER_ON is opposite to the sdhci spec. I would say that not > >> > setting the bus voltage is a quirk! > >> > >> I don't really follow. > >> > >> I read the SDHCI spec and the section for the Power Control Register. > >> Bit 0 needs to be set when communicating with the card as it will for > >> example enables the clock. Before setting bit0 you must decide what > >> signal level to use, which is done by writing to bit 1->3. > >> > > > > Right. But when having vmmc supply we do: > > sdhci_writeb(host, SDHCI_POWER_ON, SDHCI_POWER_CONTROL) or > > sdhci_writeb(host, 0, SDHCI_POWER_CONTROL) so we loose the signal level, > > isn't it? > > > >> If SDHCI monitors the power state (MMC_POWER_UP|ON|OFF) when its > >> ->set_ios() callback are invoked and in combination of using the > >> ->start_signal_voltage_switch() callback to change the signal voltage > >> level, this *should* work out nicely. > >> > > > > It is my turn to not follow! We write into the Power Control Register > > only in sdhci_set_power(). May I miss a callback or something else? > > sdhci_do_start_signal_voltage_switch doesn't modify the Power Control > > Register. > > > >> Now, looking at the related code in sdhci, I am kind of surprised that > >> it works. :-) Though, again I don't have the in-depth knowledge about > >> sdhci. > >> > > > > Me too, I am starting to dig into the sdhci spec and some points are > > not crystal clear. > > > > Okay, I am finally starting to understand some of your concern. > > According to the spec, the Power Control Register should control the > signal voltage/bus voltage. As UHS mode was added to the spec, it > seems like the Power Control Register couldn't cover all new cases, as > why Host Control 2 register needed to be added. The Host Control 2 > register, is what sdhci_do_start_signal_voltage_switch() uses to > change the signal voltage level, which all makes sense to me. > > For sdhci_set_power(); it seems to use the Power Control Register to > control the power to the card (VDD/VMMC). Indeed this looks *really* > weird/wrong. I wonder if it's working because of luck, intentional > violation of the SDHCI spec or because of special variants. > Yes but I don't know if we are doing something weird/wrong or if it is the naming of the 'SD Bus Voltage Select' which is weird/wrong. > Especially when looking into the case when you *don't* have a VMMC > regulator several strange quirks exists in sdhci_set_power(). > > In the case when you *have* a VMMC, I think just setting/clearing bit > 0 (SDHCI_POWER_ON) and then bail out, is probably working with modern > HW because it's likely the only thing needed. > > Now, this discussion was interesting, but I forgot what problem you > actually where trying to solve? :-) There is this discussion because of two things: - Fixing something I consider as a bug: when I have a VMMC, only setting/clearing bit 0. Our controller strictly obeys to the spec and check the 'SD Bus Voltage Select' field. Since we put a reserved value (000), the Power On is not performed. - I was trying to get help to understand what is this 'SD Bus Voltage'. For our controller and sdhci_set_power(), it seems to stand for VMMC. For me, everything concerning bus voltage is related to VQMMC, so I was disappointed. Regards Ludovic -- 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
Hi Ulf, On Mon, Nov 09, 2015 at 05:30:26PM +0100, Ludovic Desroches wrote: > On Mon, Nov 09, 2015 at 05:00:46PM +0100, Ulf Hansson wrote: [...] > > Now, this discussion was interesting, but I forgot what problem you > > actually where trying to solve? :-) > > There is this discussion because of two things: > - Fixing something I consider as a bug: when I have a VMMC, only > setting/clearing bit 0. Our controller strictly obeys to the spec and > check the 'SD Bus Voltage Select' field. Since we put a reserved value > (000), the Power On is not performed. > - I was trying to get help to understand what is this 'SD Bus Voltage'. > For our controller and sdhci_set_power(), it seems to stand for VMMC. > For me, everything concerning bus voltage is related to VQMMC, so I was > disappointed. Do you plan to take the patch for VMMC? If yes, I will send a new patch for the device tree (I'll only add vmmc, not vqmmc as discussed); if not, forget these two patches. Regards Ludovic -- 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
On 24 November 2015 at 10:23, Ludovic Desroches <ludovic.desroches@atmel.com> wrote: > Hi Ulf, > > On Mon, Nov 09, 2015 at 05:30:26PM +0100, Ludovic Desroches wrote: >> On Mon, Nov 09, 2015 at 05:00:46PM +0100, Ulf Hansson wrote: > > [...] > >> > Now, this discussion was interesting, but I forgot what problem you >> > actually where trying to solve? :-) >> >> There is this discussion because of two things: >> - Fixing something I consider as a bug: when I have a VMMC, only >> setting/clearing bit 0. Our controller strictly obeys to the spec and >> check the 'SD Bus Voltage Select' field. Since we put a reserved value >> (000), the Power On is not performed. >> - I was trying to get help to understand what is this 'SD Bus Voltage'. >> For our controller and sdhci_set_power(), it seems to stand for VMMC. >> For me, everything concerning bus voltage is related to VQMMC, so I was >> disappointed. > > Do you plan to take the patch for VMMC? If yes, I will send a new patch > for the device tree (I'll only add vmmc, not vqmmc as discussed); if > not, forget these two patches. > Which patch do you refer to for "VMMC"? Kind regards Uffe -- 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
On Tue, Nov 24, 2015 at 12:01:53PM +0100, Ulf Hansson wrote: > On 24 November 2015 at 10:23, Ludovic Desroches > <ludovic.desroches@atmel.com> wrote: > > Hi Ulf, > > > > On Mon, Nov 09, 2015 at 05:30:26PM +0100, Ludovic Desroches wrote: > >> On Mon, Nov 09, 2015 at 05:00:46PM +0100, Ulf Hansson wrote: > > > > [...] > > > >> > Now, this discussion was interesting, but I forgot what problem you > >> > actually where trying to solve? :-) > >> > >> There is this discussion because of two things: > >> - Fixing something I consider as a bug: when I have a VMMC, only > >> setting/clearing bit 0. Our controller strictly obeys to the spec and > >> check the 'SD Bus Voltage Select' field. Since we put a reserved value > >> (000), the Power On is not performed. > >> - I was trying to get help to understand what is this 'SD Bus Voltage'. > >> For our controller and sdhci_set_power(), it seems to stand for VMMC. > >> For me, everything concerning bus voltage is related to VQMMC, so I was > >> disappointed. > > > > Do you plan to take the patch for VMMC? If yes, I will send a new patch > > for the device tree (I'll only add vmmc, not vqmmc as discussed); if > > not, forget these two patches. > > > > Which patch do you refer to for "VMMC"? > This one to not write an invalid voltage in the power control register even if we have an external regulator for vmmc. Regards Ludovic -- 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
On 24 November 2015 at 14:12, Ludovic Desroches <ludovic.desroches@atmel.com> wrote: > On Tue, Nov 24, 2015 at 12:01:53PM +0100, Ulf Hansson wrote: >> On 24 November 2015 at 10:23, Ludovic Desroches >> <ludovic.desroches@atmel.com> wrote: >> > Hi Ulf, >> > >> > On Mon, Nov 09, 2015 at 05:30:26PM +0100, Ludovic Desroches wrote: >> >> On Mon, Nov 09, 2015 at 05:00:46PM +0100, Ulf Hansson wrote: >> > >> > [...] >> > >> >> > Now, this discussion was interesting, but I forgot what problem you >> >> > actually where trying to solve? :-) >> >> >> >> There is this discussion because of two things: >> >> - Fixing something I consider as a bug: when I have a VMMC, only >> >> setting/clearing bit 0. Our controller strictly obeys to the spec and >> >> check the 'SD Bus Voltage Select' field. Since we put a reserved value >> >> (000), the Power On is not performed. >> >> - I was trying to get help to understand what is this 'SD Bus Voltage'. >> >> For our controller and sdhci_set_power(), it seems to stand for VMMC. >> >> For me, everything concerning bus voltage is related to VQMMC, so I was >> >> disappointed. >> > >> > Do you plan to take the patch for VMMC? If yes, I will send a new patch >> > for the device tree (I'll only add vmmc, not vqmmc as discussed); if >> > not, forget these two patches. >> > >> >> Which patch do you refer to for "VMMC"? >> > > This one to not write an invalid voltage in the power control register > even if we have an external regulator for vmmc. > As I stated earlier, according to the SDHCI spec in the section for the Power Control Register. Bit 0 needs to be set when communicating with the card as it will for example enable the clock. I suspect if I apply your patch several sdhci variants would break, don't you think? Kind regards Uffe -- 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
On Tue, Nov 24, 2015 at 02:56:21PM +0100, Ulf Hansson wrote: > On 24 November 2015 at 14:12, Ludovic Desroches > <ludovic.desroches@atmel.com> wrote: > > On Tue, Nov 24, 2015 at 12:01:53PM +0100, Ulf Hansson wrote: > >> On 24 November 2015 at 10:23, Ludovic Desroches > >> <ludovic.desroches@atmel.com> wrote: > >> > Hi Ulf, > >> > > >> > On Mon, Nov 09, 2015 at 05:30:26PM +0100, Ludovic Desroches wrote: > >> >> On Mon, Nov 09, 2015 at 05:00:46PM +0100, Ulf Hansson wrote: > >> > > >> > [...] > >> > > >> >> > Now, this discussion was interesting, but I forgot what problem you > >> >> > actually where trying to solve? :-) > >> >> > >> >> There is this discussion because of two things: > >> >> - Fixing something I consider as a bug: when I have a VMMC, only > >> >> setting/clearing bit 0. Our controller strictly obeys to the spec and > >> >> check the 'SD Bus Voltage Select' field. Since we put a reserved value > >> >> (000), the Power On is not performed. > >> >> - I was trying to get help to understand what is this 'SD Bus Voltage'. > >> >> For our controller and sdhci_set_power(), it seems to stand for VMMC. > >> >> For me, everything concerning bus voltage is related to VQMMC, so I was > >> >> disappointed. > >> > > >> > Do you plan to take the patch for VMMC? If yes, I will send a new patch > >> > for the device tree (I'll only add vmmc, not vqmmc as discussed); if > >> > not, forget these two patches. > >> > > >> > >> Which patch do you refer to for "VMMC"? > >> > > > > This one to not write an invalid voltage in the power control register > > even if we have an external regulator for vmmc. > > > > As I stated earlier, according to the SDHCI spec in the section for > the Power Control Register. Bit 0 needs to be set when communicating > with the card as it will for > example enable the clock. > I am okay with bit 0. I don't want to change this part, it will be done later in sdhci_set_power(). My concern is only about bit 3-1, I want to go through the switch statement. > I suspect if I apply your patch several sdhci variants would break, > don't you think? I wouldn't sign it with my blood but I don't think so. It seems they don't care about the SD bus Voltage since they work with an unsupported voltage. Regards Ludovic -- 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
[...] >> > This one to not write an invalid voltage in the power control register >> > even if we have an external regulator for vmmc. >> > >> >> As I stated earlier, according to the SDHCI spec in the section for >> the Power Control Register. Bit 0 needs to be set when communicating >> with the card as it will for >> example enable the clock. >> > > I am okay with bit 0. I don't want to change this part, it will be done > later in sdhci_set_power(). My concern is only about bit 3-1, I want to > go through the switch statement. For those variants that have a VMMC and don't care about the other bits (1->3), it means executing code that isn't needed. Instead, as I have been telling people several times by now, let's convert the "sdhci core" into a library, so each variant can pick and do what suite them best. > >> I suspect if I apply your patch several sdhci variants would break, >> don't you think? > > I wouldn't sign it with my blood but I don't think so. It seems they > don't care about the SD bus Voltage since they work with an unsupported > voltage. You may very well be right, that it doesn't break anything. But in this case I really don't want to take the risk. As stated above, the proper solution would be that sdhci_set_power() should be split up in smaller pieces, where each piece may become a library function. Each host variant can then decide what to use. Future wise, that would mean when changing a library function, it will affect the subset of the sdhci variants that actually use it and not *all* sdhci variants. Moreover it will lead to optimized code. Kind regards Uffe -- 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
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index ac97b46..0cfd7b2 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -1278,13 +1278,6 @@ static void sdhci_set_power(struct sdhci_host *host, unsigned char mode, spin_unlock_irq(&host->lock); mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd); spin_lock_irq(&host->lock); - - if (mode != MMC_POWER_OFF) - sdhci_writeb(host, SDHCI_POWER_ON, SDHCI_POWER_CONTROL); - else - sdhci_writeb(host, 0, SDHCI_POWER_CONTROL); - - return; } if (mode != MMC_POWER_OFF) {
When there is a vmmc regulator, only SD Bus Power is set to 1 in the Power Control Register. It means SD Bus Voltage Select field is set to 0 that is a reserved value. The SD Host Controller specification says: 'SD Bus Power: Before setting this bit, the SD Host Driver shall set SD Bus Voltage Select.' and 'If the Host Driver selects an unsupported voltage in the SD B?us Voltage Select field, the Host Controller may ignore writes to SD Bus Power and keep its value at zero." Having an external regulator means the SD Bus Voltage Select is useless but if the Host Controller strictly follows the specification then we need to set a valid value. Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com> --- drivers/mmc/host/sdhci.c | 7 ------- 1 file changed, 7 deletions(-)