UHS-I bus speed mode should be set last in UHS initialization

UHS-I bus speed mode should be set last in UHS initialization

[Subhash Jadavani]

Hi Chris, Arindam,

mmc_sd_init_uhs_card function sets the driver type, current limit and bus
speed mode on card as well as on host controller side. Currently bus speed
mode is set by sending CMD6 to card and setting timing mode in host
controller and then before initiating tuning sequence, it also tries to
set current limit by sending CMD6 to card which fails on our host
controller (due to data CRC or timeout errors) if bus speed mode is
SDR50/SDR104 mode.

So basically bus speed mode should be set only after current limit is set
in the card and immediately after setting the bus speed mode, tuning
sequence should be initiated. So following changes is needed. If it looks
fine then I can send you this change in patch.

diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c index
0da37eb..19634d5 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -610,13 +610,13 @@ static int mmc_sd_init_uhs_card(struct mmc_card *card)
        if (err)
                goto out;

-       /* Set bus speed mode of the card */
-       err = sd_set_bus_speed_mode(card, status);
+       /* Set current limit for the card */
+       err = sd_set_current_limit(card, status);
        if (err)
                goto out;

-       /* Set current limit for the card */
-       err = sd_set_current_limit(card, status);
+       /* Set bus speed mode of the card */
+       err = sd_set_bus_speed_mode(card, status);
        if (err)
                goto out;

Regards,
Subhash

-- 
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

RE: UHS-I bus speed mode should be set last in UHS initialization

[Nath, Arindam]

Adding Philip and Zhangfei.

Hi Subhash,

Does your controller follow some non-standard steps for initialization? I referred to Figure 3-14, section 3.9.4 of Physical Layer Spec v3.01, and it shows the order, setting Driver Strength Select, Bus Speed Mode, and then Current Limit. So please confirm back.

Thanks,
Arindam

> -----Original Message-----
> From: subhashj@codeaurora.org [mailto:subhashj@codeaurora.org]
> Sent: Tuesday, June 14, 2011 1:27 AM
> To: Nath, Arindam
> Subject: FW: UHS-I bus speed mode should be set last in UHS
> initialization
> 
> Hi Arindam,
> 
> Does this change make sense for you? If yes, then I can go ahead and
> post a
> patch for this.
> 
> Regards,
> Subhash
> 
> -----Original Message-----
> From: linux-mmc-owner@vger.kernel.org
> [mailto:linux-mmc-owner@vger.kernel.org] On Behalf Of Subhash Jadavani
> Sent: Monday, June 06, 2011 3:44 PM
> To: linux-mmc@vger.kernel.org
> Subject: UHS-I bus speed mode should be set last in UHS initialization
> 
> Hi Chris, Arindam,
> 
> mmc_sd_init_uhs_card function sets the driver type, current limit and
> bus
> speed mode on card as well as on host controller side. Currently bus
> speed
> mode is set by sending CMD6 to card and setting timing mode in host
> controller and then before initiating tuning sequence, it also tries to
> set current limit by sending CMD6 to card which fails on our host
> controller (due to data CRC or timeout errors) if bus speed mode is
> SDR50/SDR104 mode.
> 
> So basically bus speed mode should be set only after current limit is
> set
> in the card and immediately after setting the bus speed mode, tuning
> sequence should be initiated. So following changes is needed. If it
> looks
> fine then I can send you this change in patch.
> 
> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c index
> 0da37eb..19634d5 100644
> --- a/drivers/mmc/core/sd.c
> +++ b/drivers/mmc/core/sd.c
> @@ -610,13 +610,13 @@ static int mmc_sd_init_uhs_card(struct mmc_card
> *card)
>         if (err)
>                 goto out;
> 
> -       /* Set bus speed mode of the card */
> -       err = sd_set_bus_speed_mode(card, status);
> +       /* Set current limit for the card */
> +       err = sd_set_current_limit(card, status);
>         if (err)
>                 goto out;
> 
> -       /* Set current limit for the card */
> -       err = sd_set_current_limit(card, status);
> +       /* Set bus speed mode of the card */
> +       err = sd_set_bus_speed_mode(card, status);
>         if (err)
>                 goto out;
> 
> Regards,
> Subhash
> 
> --
> Sent by a consultant of the Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
> Forum.
> 
> 
> --
> 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: UHS-I bus speed mode should be set last in UHS initialization

[subhashj]

Hi Arindam,

> -----Original Message-----
> From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc-
> owner@vger.kernel.org] On Behalf Of Nath, Arindam
> Sent: Tuesday, June 14, 2011 12:00 AM
> To: subhashj@codeaurora.org
> Cc: Philip Rakity; zhangfei gao; cjb@laptop.org; linux-
> mmc@vger.kernel.org; Lu, Aaron
> Subject: RE: UHS-I bus speed mode should be set last in UHS
> initialization
> 
> Adding Philip and Zhangfei.
> 
> Hi Subhash,
> 
> Does your controller follow some non-standard steps for initialization?
> I referred to Figure 3-14, section 3.9.4 of Physical Layer Spec v3.01,
> and it shows the order, setting Driver Strength Select, Bus Speed Mode,
> and then Current Limit. So please confirm back.

Sorry, I don't know what do you mean by non standard steps for
initialization. Nowhere in v3.01 spec, it is mentioned that Bus speed mode
**must** be set before current limit. Figure 3-14 also doesn't indicate
this.

If you set the bus speed mode in sd_set_bus_speed_mode(), you are changing
the host controller timings as well and if it's SDR50/SDR104 mode, then
immediately after setting the host controller timing mode, we have to
execute tuning sequence but here before executing tuning sequence, you are
sending the CMD6 which involves the read data transfer from card which is
bound to fail without tuning and it does fail with our host controller.

Do let me know if this explanation make sense.

Regards,
Subhash

> 
> Thanks,
> Arindam
> 
> > -----Original Message-----
> > From: subhashj@codeaurora.org [mailto:subhashj@codeaurora.org]
> > Sent: Tuesday, June 14, 2011 1:27 AM
> > To: Nath, Arindam
> > Subject: FW: UHS-I bus speed mode should be set last in UHS
> > initialization
> >
> > Hi Arindam,
> >
> > Does this change make sense for you? If yes, then I can go ahead and
> > post a
> > patch for this.
> >
> > Regards,
> > Subhash
> >
> > -----Original Message-----
> > From: linux-mmc-owner@vger.kernel.org
> > [mailto:linux-mmc-owner@vger.kernel.org] On Behalf Of Subhash
> Jadavani
> > Sent: Monday, June 06, 2011 3:44 PM
> > To: linux-mmc@vger.kernel.org
> > Subject: UHS-I bus speed mode should be set last in UHS
> initialization
> >
> > Hi Chris, Arindam,
> >
> > mmc_sd_init_uhs_card function sets the driver type, current limit and
> > bus
> > speed mode on card as well as on host controller side. Currently bus
> > speed
> > mode is set by sending CMD6 to card and setting timing mode in host
> > controller and then before initiating tuning sequence, it also tries
> to
> > set current limit by sending CMD6 to card which fails on our host
> > controller (due to data CRC or timeout errors) if bus speed mode is
> > SDR50/SDR104 mode.
> >
> > So basically bus speed mode should be set only after current limit is
> > set
> > in the card and immediately after setting the bus speed mode, tuning
> > sequence should be initiated. So following changes is needed. If it
> > looks
> > fine then I can send you this change in patch.
> >
> > diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c index
> > 0da37eb..19634d5 100644
> > --- a/drivers/mmc/core/sd.c
> > +++ b/drivers/mmc/core/sd.c
> > @@ -610,13 +610,13 @@ static int mmc_sd_init_uhs_card(struct mmc_card
> > *card)
> >         if (err)
> >                 goto out;
> >
> > -       /* Set bus speed mode of the card */
> > -       err = sd_set_bus_speed_mode(card, status);
> > +       /* Set current limit for the card */
> > +       err = sd_set_current_limit(card, status);
> >         if (err)
> >                 goto out;
> >
> > -       /* Set current limit for the card */
> > -       err = sd_set_current_limit(card, status);
> > +       /* Set bus speed mode of the card */
> > +       err = sd_set_bus_speed_mode(card, status);
> >         if (err)
> >                 goto out;
> >
> > Regards,
> > Subhash
> >
> > --
> > Sent by a consultant of the Qualcomm Innovation Center, Inc.
> > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
> > Forum.
> >
> >
> > --
> > 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

RE: UHS-I bus speed mode should be set last in UHS initialization

[Nath, Arindam]

Hi Philip,

Can you please confirm what should be the correct implementation?

Thanks,
Arindam

> -----Original Message-----
> From: subhashj@codeaurora.org [mailto:subhashj@codeaurora.org]
> Sent: Wednesday, June 15, 2011 5:28 AM
> To: Nath, Arindam
> Cc: 'Philip Rakity'; 'zhangfei gao'; cjb@laptop.org; linux-
> mmc@vger.kernel.org; Lu, Aaron
> Subject: RE: UHS-I bus speed mode should be set last in UHS
> initialization
> 
> Hi Arindam,
> 
> > -----Original Message-----
> > From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc-
> > owner@vger.kernel.org] On Behalf Of Nath, Arindam
> > Sent: Tuesday, June 14, 2011 12:00 AM
> > To: subhashj@codeaurora.org
> > Cc: Philip Rakity; zhangfei gao; cjb@laptop.org; linux-
> > mmc@vger.kernel.org; Lu, Aaron
> > Subject: RE: UHS-I bus speed mode should be set last in UHS
> > initialization
> >
> > Adding Philip and Zhangfei.
> >
> > Hi Subhash,
> >
> > Does your controller follow some non-standard steps for
> initialization?
> > I referred to Figure 3-14, section 3.9.4 of Physical Layer Spec
> v3.01,
> > and it shows the order, setting Driver Strength Select, Bus Speed
> Mode,
> > and then Current Limit. So please confirm back.
> 
> Sorry, I don't know what do you mean by non standard steps for
> initialization. Nowhere in v3.01 spec, it is mentioned that Bus speed
> mode
> **must** be set before current limit. Figure 3-14 also doesn't indicate
> this.
> 
> If you set the bus speed mode in sd_set_bus_speed_mode(), you are
> changing
> the host controller timings as well and if it's SDR50/SDR104 mode, then
> immediately after setting the host controller timing mode, we have to
> execute tuning sequence but here before executing tuning sequence, you
> are
> sending the CMD6 which involves the read data transfer from card which
> is
> bound to fail without tuning and it does fail with our host controller.
> 
> Do let me know if this explanation make sense.
> 
> Regards,
> Subhash
> 
> >
> > Thanks,
> > Arindam
> >
> > > -----Original Message-----
> > > From: subhashj@codeaurora.org [mailto:subhashj@codeaurora.org]
> > > Sent: Tuesday, June 14, 2011 1:27 AM
> > > To: Nath, Arindam
> > > Subject: FW: UHS-I bus speed mode should be set last in UHS
> > > initialization
> > >
> > > Hi Arindam,
> > >
> > > Does this change make sense for you? If yes, then I can go ahead
> and
> > > post a
> > > patch for this.
> > >
> > > Regards,
> > > Subhash
> > >
> > > -----Original Message-----
> > > From: linux-mmc-owner@vger.kernel.org
> > > [mailto:linux-mmc-owner@vger.kernel.org] On Behalf Of Subhash
> > Jadavani
> > > Sent: Monday, June 06, 2011 3:44 PM
> > > To: linux-mmc@vger.kernel.org
> > > Subject: UHS-I bus speed mode should be set last in UHS
> > initialization
> > >
> > > Hi Chris, Arindam,
> > >
> > > mmc_sd_init_uhs_card function sets the driver type, current limit
> and
> > > bus
> > > speed mode on card as well as on host controller side. Currently
> bus
> > > speed
> > > mode is set by sending CMD6 to card and setting timing mode in host
> > > controller and then before initiating tuning sequence, it also
> tries
> > to
> > > set current limit by sending CMD6 to card which fails on our host
> > > controller (due to data CRC or timeout errors) if bus speed mode is
> > > SDR50/SDR104 mode.
> > >
> > > So basically bus speed mode should be set only after current limit
> is
> > > set
> > > in the card and immediately after setting the bus speed mode,
> tuning
> > > sequence should be initiated. So following changes is needed. If it
> > > looks
> > > fine then I can send you this change in patch.
> > >
> > > diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c index
> > > 0da37eb..19634d5 100644
> > > --- a/drivers/mmc/core/sd.c
> > > +++ b/drivers/mmc/core/sd.c
> > > @@ -610,13 +610,13 @@ static int mmc_sd_init_uhs_card(struct
> mmc_card
> > > *card)
> > >         if (err)
> > >                 goto out;
> > >
> > > -       /* Set bus speed mode of the card */
> > > -       err = sd_set_bus_speed_mode(card, status);
> > > +       /* Set current limit for the card */
> > > +       err = sd_set_current_limit(card, status);
> > >         if (err)
> > >                 goto out;
> > >
> > > -       /* Set current limit for the card */
> > > -       err = sd_set_current_limit(card, status);
> > > +       /* Set bus speed mode of the card */
> > > +       err = sd_set_bus_speed_mode(card, status);
> > >         if (err)
> > >                 goto out;
> > >
> > > Regards,
> > > Subhash
> > >
> > > --
> > > Sent by a consultant of the Qualcomm Innovation Center, Inc.
> > > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
> > > Forum.
> > >
> > >
> > > --
> > > 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
> 

Re: UHS-I bus speed mode should be set last in UHS initialization

[zhangfei gao]

On Wed, Jun 15, 2011 at 7:57 AM,  <subhashj@codeaurora.org> wrote:
> Hi Arindam,
>
>> -----Original Message-----
>> From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc-
>> owner@vger.kernel.org] On Behalf Of Nath, Arindam
>> Sent: Tuesday, June 14, 2011 12:00 AM
>> To: subhashj@codeaurora.org
>> Cc: Philip Rakity; zhangfei gao; cjb@laptop.org; linux-
>> mmc@vger.kernel.org; Lu, Aaron
>> Subject: RE: UHS-I bus speed mode should be set last in UHS
>> initialization
>>
>> Adding Philip and Zhangfei.
>>
>> Hi Subhash,
>>
>> Does your controller follow some non-standard steps for initialization?
>> I referred to Figure 3-14, section 3.9.4 of Physical Layer Spec v3.01,
>> and it shows the order, setting Driver Strength Select, Bus Speed Mode,
>> and then Current Limit. So please confirm back.
>
> Sorry, I don't know what do you mean by non standard steps for
> initialization. Nowhere in v3.01 spec, it is mentioned that Bus speed mode
> **must** be set before current limit. Figure 3-14 also doesn't indicate
> this.
>
> If you set the bus speed mode in sd_set_bus_speed_mode(), you are changing
> the host controller timings as well and if it's SDR50/SDR104 mode, then
> immediately after setting the host controller timing mode, we have to
> execute tuning sequence but here before executing tuning sequence, you are
> sending the CMD6 which involves the read data transfer from card which is
> bound to fail without tuning and it does fail with our host controller.
>
> Do let me know if this explanation make sense.

I think the concern does make sense.

Though Figure 3-14 and 4-6 describe Bus Speed Mode is selected ahead
of Current limit, the controller timing is changed in
sd_set_bus_speed_mode, which may cause following cmd6 fail.

However, the test here shows mmp2 does not care the sequence of
speed_mode and current_limit.

Thanks

RE: UHS-I bus speed mode should be set last in UHS initialization

[Nath, Arindam]

Hi Zhangfei,


> -----Original Message-----
> From: zhangfei gao [mailto:zhangfei.gao@gmail.com]
> Sent: Friday, June 17, 2011 12:15 PM
> To: subhashj@codeaurora.org; Nath, Arindam
> Cc: Philip Rakity; cjb@laptop.org; linux-mmc@vger.kernel.org; Lu, Aaron
> Subject: Re: UHS-I bus speed mode should be set last in UHS
> initialization
> 
> On Wed, Jun 15, 2011 at 7:57 AM,  <subhashj@codeaurora.org> wrote:
> > Hi Arindam,
> >
> >> -----Original Message-----
> >> From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc-
> >> owner@vger.kernel.org] On Behalf Of Nath, Arindam
> >> Sent: Tuesday, June 14, 2011 12:00 AM
> >> To: subhashj@codeaurora.org
> >> Cc: Philip Rakity; zhangfei gao; cjb@laptop.org; linux-
> >> mmc@vger.kernel.org; Lu, Aaron
> >> Subject: RE: UHS-I bus speed mode should be set last in UHS
> >> initialization
> >>
> >> Adding Philip and Zhangfei.
> >>
> >> Hi Subhash,
> >>
> >> Does your controller follow some non-standard steps for
> initialization?
> >> I referred to Figure 3-14, section 3.9.4 of Physical Layer Spec
> v3.01,
> >> and it shows the order, setting Driver Strength Select, Bus Speed
> Mode,
> >> and then Current Limit. So please confirm back.
> >
> > Sorry, I don't know what do you mean by non standard steps for
> > initialization. Nowhere in v3.01 spec, it is mentioned that Bus speed
> mode
> > **must** be set before current limit. Figure 3-14 also doesn't
> indicate
> > this.
> >
> > If you set the bus speed mode in sd_set_bus_speed_mode(), you are
> changing
> > the host controller timings as well and if it's SDR50/SDR104 mode,
> then
> > immediately after setting the host controller timing mode, we have to
> > execute tuning sequence but here before executing tuning sequence,
> you are
> > sending the CMD6 which involves the read data transfer from card
> which is
> > bound to fail without tuning and it does fail with our host
> controller.
> >
> > Do let me know if this explanation make sense.
> 
> I think the concern does make sense.
> 
> Though Figure 3-14 and 4-6 describe Bus Speed Mode is selected ahead
> of Current limit, the controller timing is changed in
> sd_set_bus_speed_mode, which may cause following cmd6 fail.
> 
> However, the test here shows mmp2 does not care the sequence of
> speed_mode and current_limit.
> 
> Thanks

We also verified that the changes suggested by Subhash's patch does not break things for our controller. But I am still waiting for Philip's confirmation.

Thanks,
Arindam