Re: [edk2-platforms: PATCH] Marvell/Drivers: XenonDxe: Explicitly disable HS400

["Leif Lindholm" <leif.lindholm@linaro.org>]

On Thu, Jul 11, 2019 at 10:07:27AM +0200, Marcin Wojtas wrote:
> Hi Leif,
> 
> śr., 26 cze 2019 o 11:58 Marcin Wojtas <mw@semihalf.com> napisał(a):
> >
> > Hi Leif,
> >
> > śr., 26 cze 2019 o 11:31 Leif Lindholm <leif.lindholm@linaro.org> napisał(a):
> > >
> > > On Wed, Jun 26, 2019 at 09:04:14AM +0200, Marcin Wojtas wrote:
> > > > Ensure that in case of SlowMode or 3.3V operation,
> > > > also the HS400 capability will be disabled in the
> > > > SdMmc driver.
> > > >
> > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > >
> > > Well done on keeping this tag. But I'm thinking we need to do that
> > > relicensing sooner rather than later, and drop the tag.
> >
> > I left it, as this file is still not 2-clause SPDX tagged.
> >
> > >
> > >
> > > However, can you clarify what problem this solves?
> > >
> >
> > On another SoC revision, the capability register marks HS400 support
> > as enabled. However the interface itself is powered with 3.3V and it
> > turned out that my flags in SdMmcOverride driver did not cover this
> > case, which resulted in an unsuccessful EmmcSwitchToHS400 () execution
> > - it shouldn't be done at all.
> >
> 
> Did you have a chance to see my explanation? Should I repost?

Sorry, yes. Explanation is fine. If you can update the commit message
and drop the Contributed-under, I will push this once we get the
licenses updated.

Best Regards,

Leif

> Best regards,
> Marcin
> 
> > > /
> > >     Leif
> > >
> > > > Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> > > > ---
> > > >  Silicon/Marvell/Drivers/SdMmc/XenonDxe/XenonSdhci.h         | 1 +
> > > >  Silicon/Marvell/Drivers/SdMmc/XenonDxe/XenonSdMmcOverride.c | 5 +++--
> > > >  2 files changed, 4 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/Silicon/Marvell/Drivers/SdMmc/XenonDxe/XenonSdhci.h b/Silicon/Marvell/Drivers/SdMmc/XenonDxe/XenonSdhci.h
> > > > index 8bf1835..2d7c7f0 100644
> > > > --- a/Silicon/Marvell/Drivers/SdMmc/XenonDxe/XenonSdhci.h
> > > > +++ b/Silicon/Marvell/Drivers/SdMmc/XenonDxe/XenonSdhci.h
> > > > @@ -82,6 +82,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> > > >  #define SDHC_CAP_SDR50                BIT32
> > > >  #define SDHC_CAP_SDR104               BIT33
> > > >  #define SDHC_CAP_DDR50                BIT34
> > > > +#define SDHC_CAP_HS400                BIT63
> > > >  #define SDHC_MAX_CURRENT_CAP          0x0048
> > > >  #define SDHC_FORCE_EVT_AUTO_CMD       0x0050
> > > >  #define SDHC_FORCE_EVT_ERR_INT        0x0052
> > > > diff --git a/Silicon/Marvell/Drivers/SdMmc/XenonDxe/XenonSdMmcOverride.c b/Silicon/Marvell/Drivers/SdMmc/XenonDxe/XenonSdMmcOverride.c
> > > > index 7a9266e..55ebcf8 100644
> > > > --- a/Silicon/Marvell/Drivers/SdMmc/XenonDxe/XenonSdMmcOverride.c
> > > > +++ b/Silicon/Marvell/Drivers/SdMmc/XenonDxe/XenonSdMmcOverride.c
> > > > @@ -357,7 +357,8 @@ XenonSdMmcCapability (
> > > >      Capability &= ~(UINT64)(SDHC_CAP_VOLTAGE_33 | SDHC_CAP_VOLTAGE_30);
> > > >    } else {
> > > >      Capability &= ~(UINT64)(SDHC_CAP_SDR104 | SDHC_CAP_DDR50 |
> > > > -                            SDHC_CAP_SDR50 | SDHC_CAP_VOLTAGE_18);
> > > > +                            SDHC_CAP_SDR50 | SDHC_CAP_HS400 |
> > > > +                            SDHC_CAP_VOLTAGE_18);
> > > >    }
> > > >
> > > >    if (!SdMmcDesc.Xenon8BitBusEnabled) {
> > > > @@ -365,7 +366,7 @@ XenonSdMmcCapability (
> > > >    }
> > > >
> > > >    if (SdMmcDesc.XenonSlowModeEnabled) {
> > > > -    Capability &= ~(UINT64)(SDHC_CAP_SDR104 | SDHC_CAP_DDR50);
> > > > +    Capability &= ~(UINT64)(SDHC_CAP_SDR104 | SDHC_CAP_DDR50 | SDHC_CAP_HS400);
> > > >    }
> > > >
> > > >    Capability &= ~(UINT64)(SDHC_CAP_SLOT_TYPE_MASK);
> > > > --
> > > > 2.7.4
> > > >