Hi Ard,

I'm glad you're back :)

czw., 8 lis 2018 o 12:06 Ard Biesheuvel <ard.biesheu...@linaro.org> napisaƂ(a):
>
> On 8 November 2018 at 02:57, Marcin Wojtas <m...@semihalf.com> wrote:
> > From: Tomasz Michalec <t...@semihalf.com>
> >
> > Some SD Host Controllers use different values in Host Control 2 Register
> > to select UHS Mode. This patch adds a new UhsSignaling type routine to
> > the NotifyPhase of the SdMmcOverride protocol.
> >
> > UHS signaling configuration is moved to a common, default routine
> > (SdMmcHcUhsSignaling). After it is executed, the protocol producer
> > can override the values if needed..
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Marcin Wojtas <m...@semihalf.com>
> > ---
> >  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h |  32 +++++
> >  MdeModulePkg/Include/Protocol/SdMmcOverride.h    |  17 +++
> >  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c  | 136 +++++++++++++-------
> >  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c    |  31 ++++-
> >  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c |  66 ++++++++++
> >  5 files changed, 225 insertions(+), 57 deletions(-)
> >
> > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h 
> > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> > index 7e3f588..1a11d51 100644
> > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> > @@ -63,6 +63,21 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, 
> > EITHER EXPRESS OR IMPLIED.
> >  #define SD_MMC_HC_CTRL_VER            0xFE
> >
> >  //
> > +// SD Host Controller bits to HOST_CTRL2 register
> > +//
> > +#define SD_MMC_HC_CTRL_UHS_MASK       0x0007
> > +#define SD_MMC_HC_CTRL_UHS_SDR12      0x0000
> > +#define SD_MMC_HC_CTRL_UHS_SDR25      0x0001
> > +#define SD_MMC_HC_CTRL_UHS_SDR50      0x0002
> > +#define SD_MMC_HC_CTRL_UHS_SDR104     0x0003
> > +#define SD_MMC_HC_CTRL_UHS_DDR50      0x0004
> > +#define SD_MMC_HC_CTRL_MMC_LEGACY     0x0000
> > +#define SD_MMC_HC_CTRL_MMC_HS_SDR     0x0001
> > +#define SD_MMC_HC_CTRL_MMC_HS_DDR     0x0004
> > +#define SD_MMC_HC_CTRL_MMC_HS200      0x0003
> > +#define SD_MMC_HC_CTRL_MMC_HS400      0x0005
> > +
> > +//
> >  // The transfer modes supported by SD Host Controller
> >  // Simplified Spec 3.0 Table 1-2
> >  //
> > @@ -518,4 +533,21 @@ SdMmcHcInitTimeoutCtrl (
> >    IN UINT8                  Slot
> >    );
> >
> > +/**
> > +  Set SD Host Controller control 2 registry according to selected speed.
> > +
> > +  @param[in] PciIo          The PCI IO protocol instance.
> > +  @param[in] Slot           The slot number of the SD card to send the 
> > command to.
> > +  @param[in] Timing         The timing to select.
> > +
> > +  @retval EFI_SUCCESS       The timing is set successfully.
> > +  @retval Others            The timing isn't set successfully.
> > +**/
> > +EFI_STATUS
> > +SdMmcHcUhsSignaling (
> > +  IN EFI_PCI_IO_PROTOCOL    *PciIo,
> > +  IN UINT8                  Slot,
> > +  IN SD_MMC_BUS_MODE        Timing
> > +  );
> > +
> >  #endif
> > diff --git a/MdeModulePkg/Include/Protocol/SdMmcOverride.h 
> > b/MdeModulePkg/Include/Protocol/SdMmcOverride.h
> > index 8a7669e..f948bef 100644
> > --- a/MdeModulePkg/Include/Protocol/SdMmcOverride.h
> > +++ b/MdeModulePkg/Include/Protocol/SdMmcOverride.h
> > @@ -26,11 +26,28 @@
> >
> >  typedef struct _EDKII_SD_MMC_OVERRIDE EDKII_SD_MMC_OVERRIDE;
> >
> > +//
> > +// Bus timing modes
> > +//
> > +typedef enum {
> > +  SdMmcUhsSdr12,
> > +  SdMmcUhsSdr25,
> > +  SdMmcUhsSdr50,
> > +  SdMmcUhsSdr104,
> > +  SdMmcUhsDdr50,
> > +  SdMmcMmcLegacy,
> > +  SdMmcMmcHsSdr,
> > +  SdMmcMmcHsDdr,
> > +  SdMmcMmcHs200,
> > +  SdMmcMmcHs400,
> > +} SD_MMC_BUS_MODE;
> > +
> >  typedef enum {
> >    EdkiiSdMmcResetPre,
> >    EdkiiSdMmcResetPost,
> >    EdkiiSdMmcInitHostPre,
> >    EdkiiSdMmcInitHostPost,
> > +  EdkiiSdMmcUhsSignaling,
> >  } EDKII_SD_MMC_PHASE_TYPE;
> >
> >  /**
> > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c 
> > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> > index c5fd214..473df8d 100755
> > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> > @@ -740,10 +740,13 @@ EmmcSwitchToHighSpeed (
> >    IN UINT8                              BusWidth
> >    )
> >  {
> > -  EFI_STATUS          Status;
> > -  UINT8               HsTiming;
> > -  UINT8               HostCtrl1;
> > -  UINT8               HostCtrl2;
> > +  EFI_STATUS              Status;
> > +  UINT8                   HsTiming;
> > +  UINT8                   HostCtrl1;
> > +  SD_MMC_BUS_MODE         Timing;
> > +  SD_MMC_HC_PRIVATE_DATA  *Private;
> > +
> > +  Private = SD_MMC_HC_PRIVATE_FROM_THIS (PassThru);
> >
> >    Status = EmmcSwitchBusWidth (PciIo, PassThru, Slot, Rca, IsDdr, 
> > BusWidth);
> >    if (EFI_ERROR (Status)) {
> > @@ -758,29 +761,37 @@ EmmcSwitchToHighSpeed (
> >      return Status;
> >    }
> >
> > -  //
> > -  // Clean UHS Mode Select field of Host Control 2 reigster before update
> > -  //
> > -  HostCtrl2 = (UINT8)~0x7;
> > -  Status = SdMmcHcAndMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof 
> > (HostCtrl2), &HostCtrl2);
> > -  if (EFI_ERROR (Status)) {
> > -    return Status;
> > -  }
> > -  //
> > -  // Set UHS Mode Select field of Host Control 2 reigster to SDR12/25/50
> > -  //
> >    if (IsDdr) {
> > -    HostCtrl2 = BIT2;
> > +    Timing = SdMmcMmcHsDdr;
> >    } else if (ClockFreq == 52) {
> > -    HostCtrl2 = BIT0;
> > +    Timing = SdMmcMmcHsSdr;
> >    } else {
> > -    HostCtrl2 = 0;
> > +    Timing = SdMmcMmcLegacy;
> >    }
> > -  Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof 
> > (HostCtrl2), &HostCtrl2);
> > +
> > +  Status = SdMmcHcUhsSignaling (PciIo, Slot, Timing);
> >    if (EFI_ERROR (Status)) {
> >      return Status;
> >    }
> >
> > +  if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
> > +    Status = mOverride->NotifyPhase (
> > +                          Private->ControllerHandle,
> > +                          Slot,
> > +                          EdkiiSdMmcUhsSignaling,
> > +                          &Timing
> > +                          );
> > +    if (EFI_ERROR (Status)) {
> > +      DEBUG ((
> > +        DEBUG_ERROR,
> > +        "%a: SD/MMC uhs signaling notifier callback failed - %r\n",
> > +        __FUNCTION__,
> > +        Status
> > +        ));
> > +      return Status;
> > +    }
> > +  }
> > +
>
> Can we move the override handling into SdMmcHcUhsSignaling()? That
> way, we no longer have to duplicate it 4 times.

Sure! I should've come up on this on my own...

Thanks,
Marcin

>
> >    HsTiming = 1;
> >    Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, 
> > ClockFreq);
> >
> > @@ -814,10 +825,13 @@ EmmcSwitchToHS200 (
> >    IN UINT8                              BusWidth
> >    )
> >  {
> > -  EFI_STATUS          Status;
> > -  UINT8               HsTiming;
> > -  UINT8               HostCtrl2;
> > -  UINT16              ClockCtrl;
> > +  EFI_STATUS               Status;
> > +  UINT8                    HsTiming;
> > +  UINT16                   ClockCtrl;
> > +  SD_MMC_BUS_MODE          Timing;
> > +  SD_MMC_HC_PRIVATE_DATA  *Private;
> > +
> > +  Private = SD_MMC_HC_PRIVATE_FROM_THIS (PassThru);
> >
> >    if ((BusWidth != 4) && (BusWidth != 8)) {
> >      return EFI_INVALID_PARAMETER;
> > @@ -837,22 +851,32 @@ EmmcSwitchToHS200 (
> >    if (EFI_ERROR (Status)) {
> >      return Status;
> >    }
> > -  //
> > -  // Clean UHS Mode Select field of Host Control 2 reigster before update
> > -  //
> > -  HostCtrl2 = (UINT8)~0x7;
> > -  Status = SdMmcHcAndMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof 
> > (HostCtrl2), &HostCtrl2);
> > +
> > +  Timing = SdMmcMmcHs200;
> > +
> > +  Status = SdMmcHcUhsSignaling (PciIo, Slot, Timing);
> >    if (EFI_ERROR (Status)) {
> >      return Status;
> >    }
> > -  //
> > -  // Set UHS Mode Select field of Host Control 2 reigster to SDR104
> > -  //
> > -  HostCtrl2 = BIT0 | BIT1;
> > -  Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof 
> > (HostCtrl2), &HostCtrl2);
> > -  if (EFI_ERROR (Status)) {
> > -    return Status;
> > +
> > +  if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
> > +    Status = mOverride->NotifyPhase (
> > +                          Private->ControllerHandle,
> > +                          Slot,
> > +                          EdkiiSdMmcUhsSignaling,
> > +                          &Timing
> > +                          );
> > +    if (EFI_ERROR (Status)) {
> > +      DEBUG ((
> > +        DEBUG_ERROR,
> > +        "%a: SD/MMC uhs signaling notifier callback failed - %r\n",
> > +        __FUNCTION__,
> > +        Status
> > +        ));
> > +      return Status;
> > +    }
> >    }
> > +
> >    //
> >    // Wait Internal Clock Stable in the Clock Control register to be 1 
> > before set SD Clock Enable bit
> >    //
> > @@ -910,9 +934,12 @@ EmmcSwitchToHS400 (
> >    IN UINT32                             ClockFreq
> >    )
> >  {
> > -  EFI_STATUS          Status;
> > -  UINT8               HsTiming;
> > -  UINT8               HostCtrl2;
> > +  EFI_STATUS                 Status;
> > +  UINT8                      HsTiming;
> > +  SD_MMC_BUS_MODE            Timing;
> > +  SD_MMC_HC_PRIVATE_DATA     *Private;
> > +
> > +  Private = SD_MMC_HC_PRIVATE_FROM_THIS (PassThru);
> >
> >    Status = EmmcSwitchToHS200 (PciIo, PassThru, Slot, Rca, ClockFreq, 8);
> >    if (EFI_ERROR (Status)) {
> > @@ -933,21 +960,30 @@ EmmcSwitchToHS400 (
> >    if (EFI_ERROR (Status)) {
> >      return Status;
> >    }
> > -  //
> > -  // Clean UHS Mode Select field of Host Control 2 reigster before update
> > -  //
> > -  HostCtrl2 = (UINT8)~0x7;
> > -  Status = SdMmcHcAndMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof 
> > (HostCtrl2), &HostCtrl2);
> > +
> > +  Timing = SdMmcMmcHs400;
> > +
> > +  Status = SdMmcHcUhsSignaling (PciIo, Slot, Timing);
> >    if (EFI_ERROR (Status)) {
> >      return Status;
> >    }
> > -  //
> > -  // Set UHS Mode Select field of Host Control 2 reigster to HS400
> > -  //
> > -  HostCtrl2 = BIT0 | BIT2;
> > -  Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof 
> > (HostCtrl2), &HostCtrl2);
> > -  if (EFI_ERROR (Status)) {
> > -    return Status;
> > +
> > +  if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
> > +    Status = mOverride->NotifyPhase (
> > +                          Private->ControllerHandle,
> > +                          Slot,
> > +                          EdkiiSdMmcUhsSignaling,
> > +                          &Timing
> > +                          );
> > +    if (EFI_ERROR (Status)) {
> > +      DEBUG ((
> > +        DEBUG_ERROR,
> > +        "%a: SD/MMC uhs signaling notifier callback failed - %r\n",
> > +        __FUNCTION__,
> > +        Status
> > +        ));
> > +      return Status;
> > +    }
> >    }
> >
> >    HsTiming = 3;
> > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c 
> > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> > index 6ee9ed7..850ad26 100644
> > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> > @@ -784,8 +784,8 @@ SdCardSetBusMode (
> >    UINT8                        BusWidth;
> >    UINT8                        AccessMode;
> >    UINT8                        HostCtrl1;
> > -  UINT8                        HostCtrl2;
> >    UINT8                        SwitchResp[64];
> > +  SD_MMC_BUS_MODE              Timing;
> >    SD_MMC_HC_PRIVATE_DATA       *Private;
> >
> >    Private = SD_MMC_HC_PRIVATE_FROM_THIS (PassThru);
> > @@ -817,18 +817,23 @@ SdCardSetBusMode (
> >    if (S18A && (Capability->Sdr104 != 0) && ((SwitchResp[13] & BIT3) != 0)) 
> > {
> >      ClockFreq = 208;
> >      AccessMode = 3;
> > +    Timing = SdMmcUhsSdr104;
> >    } else if (S18A && (Capability->Sdr50 != 0) && ((SwitchResp[13] & BIT2) 
> > != 0)) {
> >      ClockFreq = 100;
> >      AccessMode = 2;
> > +    Timing = SdMmcUhsSdr50;
> >    } else if (S18A && (Capability->Ddr50 != 0) && ((SwitchResp[13] & BIT4) 
> > != 0)) {
> >      ClockFreq = 50;
> >      AccessMode = 4;
> > +    Timing = SdMmcUhsDdr50;
> >    } else if ((SwitchResp[13] & BIT1) != 0) {
> >      ClockFreq = 50;
> >      AccessMode = 1;
> > +    Timing = SdMmcUhsSdr25;
> >    } else {
> >      ClockFreq = 25;
> >      AccessMode = 0;
> > +    Timing = SdMmcUhsSdr12;
> >    }
> >
> >    Status = SdCardSwitch (PassThru, Slot, AccessMode, 0xF, 0xF, 0xF, TRUE, 
> > SwitchResp);
> > @@ -854,15 +859,27 @@ SdCardSetBusMode (
> >      }
> >    }
> >
> > -  HostCtrl2 = (UINT8)~0x7;
> > -  Status = SdMmcHcAndMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof 
> > (HostCtrl2), &HostCtrl2);
> > +  Status = SdMmcHcUhsSignaling (PciIo, Slot, Timing);
> >    if (EFI_ERROR (Status)) {
> >      return Status;
> >    }
> > -  HostCtrl2 = AccessMode;
> > -  Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof 
> > (HostCtrl2), &HostCtrl2);
> > -  if (EFI_ERROR (Status)) {
> > -    return Status;
> > +
> > +  if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
> > +    Status = mOverride->NotifyPhase (
> > +                          Private->ControllerHandle,
> > +                          Slot,
> > +                          EdkiiSdMmcUhsSignaling,
> > +                          &Timing
> > +                          );
> > +    if (EFI_ERROR (Status)) {
> > +      DEBUG ((
> > +        DEBUG_ERROR,
> > +        "%a: SD/MMC uhs signaling notifier callback failed - %r\n",
> > +        __FUNCTION__,
> > +        Status
> > +        ));
> > +      return Status;
> > +    }
> >    }
> >
> >    Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000, *Capability);
> > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c 
> > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> > index 923c55b..85aa625 100644
> > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> > @@ -1138,6 +1138,72 @@ SdMmcHcInitHost (
> >  }
> >
> >  /**
> > +  Set SD Host Controler control 2 registry according to selected speed.
> > +
> > +  @param[in] PciIo          The PCI IO protocol instance.
> > +  @param[in] Slot           The slot number of the SD card to send the 
> > command to.
> > +  @param[in] Timing         The timing to select.
> > +
> > +  @retval EFI_SUCCESS       The timing is set successfully.
> > +  @retval Others            The timing isn't set successfully.
> > +**/
> > +EFI_STATUS
> > +SdMmcHcUhsSignaling (
> > +  IN EFI_PCI_IO_PROTOCOL    *PciIo,
> > +  IN UINT8                  Slot,
> > +  IN SD_MMC_BUS_MODE        Timing
> > +  )
> > +{
> > +  EFI_STATUS                 Status;
> > +  UINT8                      HostCtrl2;
> > +
> > +  HostCtrl2 = (UINT8)~SD_MMC_HC_CTRL_UHS_MASK;
> > +  Status = SdMmcHcAndMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof 
> > (HostCtrl2), &HostCtrl2);
> > +  if (EFI_ERROR (Status)) {
> > +    return Status;
> > +  }
> > +
> > +  switch (Timing) {
> > +    case SdMmcUhsSdr12:
> > +      HostCtrl2 = SD_MMC_HC_CTRL_UHS_SDR12;
> > +      break;
> > +    case SdMmcUhsSdr25:
> > +      HostCtrl2 = SD_MMC_HC_CTRL_UHS_SDR25;
> > +      break;
> > +    case SdMmcUhsSdr50:
> > +      HostCtrl2 = SD_MMC_HC_CTRL_UHS_SDR50;
> > +      break;
> > +    case SdMmcUhsSdr104:
> > +      HostCtrl2 = SD_MMC_HC_CTRL_UHS_SDR104;
> > +      break;
> > +    case SdMmcUhsDdr50:
> > +      HostCtrl2 = SD_MMC_HC_CTRL_UHS_DDR50;
> > +      break;
> > +    case SdMmcMmcLegacy:
> > +      HostCtrl2 = SD_MMC_HC_CTRL_MMC_LEGACY;
> > +      break;
> > +    case SdMmcMmcHsSdr:
> > +      HostCtrl2 = SD_MMC_HC_CTRL_MMC_HS_SDR;
> > +      break;
> > +    case SdMmcMmcHsDdr:
> > +      HostCtrl2 = SD_MMC_HC_CTRL_MMC_HS_DDR;
> > +      break;
> > +    case SdMmcMmcHs200:
> > +      HostCtrl2 = SD_MMC_HC_CTRL_MMC_HS200;
> > +      break;
> > +    case SdMmcMmcHs400:
> > +      HostCtrl2 = SD_MMC_HC_CTRL_MMC_HS400;
> > +      break;
> > +    default:
> > +     HostCtrl2 = 0;
> > +     break;
> > +  }
> > +  Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof 
> > (HostCtrl2), &HostCtrl2);
> > +
> > +  return Status;
> > +}
> > +
> > +/**
> >    Turn on/off LED.
> >
> >    @param[in] PciIo          The PCI IO protocol instance.
> > --
> > 2.7.4
> >
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to