lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CACT4zj_LRArVUS4HsP134+NMm2T-PRgFGpwxmcrK87V-5rqhqg@mail.gmail.com>
Date:   Mon, 15 May 2023 08:01:44 +0800
From:   Ben Chuang <benchuanggli@...il.com>
To:     Victor Shih <victorshihgli@...il.com>
Cc:     ulf.hansson@...aro.org, adrian.hunter@...el.com,
        linux-mmc@...r.kernel.org, linux-kernel@...r.kernel.org,
        HL.Liu@...esyslogic.com.tw, Greg.tu@...esyslogic.com.tw,
        takahiro.akashi@...aro.org, dlunev@...omium.org,
        Victor Shih <victor.shih@...esyslogic.com.tw>,
        Ben Chuang <ben.chuang@...esyslogic.com.tw>
Subject: Re: [PATCH V1 1/3] mmc: sdhci-pci-gli: Add Genesys Logic GL9767 support

Hi Victor,

One thing, get in the habit of running checkpatch.pl before submitting patches.

checkpatch.pl says.
"From:/Signed-off-by: email address mismatch: 'From: Victor Shih
<victorshihgli@...il.com>' != 'Signed-off-by: Victor Shih
<victor.shih@...esyslogic.com.tw>'"

It reminds you that maybe the first line of the email body needs a
line "Form: Victor Shih <victor.shih@...esyslogic.com.tw>'".

man git-format-patch and see --form option

Best regards,
Ben


On Fri, May 12, 2023 at 6:47 PM Victor Shih <victorshihgli@...il.com> wrote:
>
> Add support for the GL9767 chipset. GL9767 supports
> SD3 mode likes UHS-I SDR50, SDR104.
> Enable MSI interrupt for GL9767. Some platform do not
> support PCI INTx and devices can not work without
> interrupt.
>
> Signed-off-by: Ben Chuang <ben.chuang@...esyslogic.com.tw>
> Signed-off-by: Victor Shih <victor.shih@...esyslogic.com.tw>
> ---
>  drivers/mmc/host/sdhci-pci-core.c |   1 +
>  drivers/mmc/host/sdhci-pci-gli.c  | 132 ++++++++++++++++++++++++++++++
>  drivers/mmc/host/sdhci-pci.h      |   2 +
>  3 files changed, 135 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
> index 01975d145200..1c2572c0f012 100644
> --- a/drivers/mmc/host/sdhci-pci-core.c
> +++ b/drivers/mmc/host/sdhci-pci-core.c
> @@ -1903,6 +1903,7 @@ static const struct pci_device_id pci_ids[] = {
>         SDHCI_PCI_DEVICE(GLI, 9750, gl9750),
>         SDHCI_PCI_DEVICE(GLI, 9755, gl9755),
>         SDHCI_PCI_DEVICE(GLI, 9763E, gl9763e),
> +       SDHCI_PCI_DEVICE(GLI, 9767, gl9767),
>         SDHCI_PCI_DEVICE_CLASS(AMD, SYSTEM_SDHCI, PCI_CLASS_MASK, amd),
>         /* Generic SD host controller */
>         {PCI_DEVICE_CLASS(SYSTEM_SDHCI, PCI_CLASS_MASK)},
> diff --git a/drivers/mmc/host/sdhci-pci-gli.c b/drivers/mmc/host/sdhci-pci-gli.c
> index 633a8ee8f8c5..300512740cb0 100644
> --- a/drivers/mmc/host/sdhci-pci-gli.c
> +++ b/drivers/mmc/host/sdhci-pci-gli.c
> @@ -149,6 +149,32 @@
>  #define PCI_GLI_9755_PM_CTRL     0xFC
>  #define   PCI_GLI_9755_PM_STATE    GENMASK(1, 0)
>
> +#define SDHCI_GLI_9767_GM_BURST_SIZE                   0x510
> +#define   SDHCI_GLI_9767_GM_BURST_SIZE_AXI_ALWAYS_SET    BIT(8)
> +
> +#define PCIE_GLI_9767_VHS      0x884
> +#define   GLI_9767_VHS_REV       GENMASK(19, 16)
> +#define   GLI_9767_VHS_REV_R     0x0
> +#define   GLI_9767_VHS_REV_M     0x1
> +#define   GLI_9767_VHS_REV_W     0x2
> +
> +#define PCIE_GLI_9767_PWR_MACRO_CTL                                    0x8D0
> +#define   PCIE_GLI_9767_PWR_MACRO_CTL_LOW_VOLTAGE                        GENMASK(3, 0)
> +#define   PCIE_GLI_9767_PWR_MACRO_CTL_LD0_LOW_OUTPUT_VOLTAGE             GENMASK(15, 12)
> +#define   PCIE_GLI_9767_PWR_MACRO_CTL_LD0_LOW_OUTPUT_VOLTAGE_VALUE       0x7
> +#define   PCIE_GLI_9767_PWR_MACRO_CTL_RCLK_AMPLITUDE_CTL                 GENMASK(29, 28)
> +#define   PCIE_GLI_9767_PWR_MACRO_CTL_RCLK_AMPLITUDE_CTL_VALUE           0x3
> +
> +#define PCIE_GLI_9767_SCR                              0x8E0
> +#define   PCIE_GLI_9767_SCR_AUTO_AXI_W_BURST             BIT(6)
> +#define   PCIE_GLI_9767_SCR_AUTO_AXI_R_BURST             BIT(7)
> +#define   PCIE_GLI_9767_SCR_AXI_REQ                      BIT(9)
> +#define   PCIE_GLI_9767_SCR_CARD_DET_PWR_SAVING_EN       BIT(10)
> +#define   PCIE_GLI_9767_SCR_SYSTEM_CLK_SELECT_MODE0      BIT(16)
> +#define   PCIE_GLI_9767_SCR_SYSTEM_CLK_SELECT_MODE1      BIT(17)
> +#define   PCIE_GLI_9767_SCR_CORE_PWR_D3_OFF              BIT(21)
> +#define   PCIE_GLI_9767_SCR_CFG_RST_DATA_LINK_DOWN       BIT(30)
> +
>  #define GLI_MAX_TUNING_LOOP 40
>
>  /* Genesys Logic chipset */
> @@ -693,6 +719,61 @@ static void gl9755_hw_setting(struct sdhci_pci_slot *slot)
>         gl9755_wt_off(pdev);
>  }
>
> +static void gli_set_9767(struct sdhci_host *host)
> +{
> +       u32 value;
> +
> +       value = sdhci_readl(host, SDHCI_GLI_9767_GM_BURST_SIZE);
> +       value &= ~SDHCI_GLI_9767_GM_BURST_SIZE_AXI_ALWAYS_SET;
> +       sdhci_writel(host, value, SDHCI_GLI_9767_GM_BURST_SIZE);
> +}
> +
> +static void gl9767_hw_setting(struct sdhci_pci_slot *slot)
> +{
> +       struct pci_dev *pdev = slot->chip->pdev;
> +       u32 value;
> +
> +       pci_read_config_dword(pdev, PCIE_GLI_9767_VHS, &value);
> +       value &= ~GLI_9767_VHS_REV;
> +       value |= FIELD_PREP(GLI_9767_VHS_REV, GLI_9767_VHS_REV_W);
> +       pci_write_config_dword(pdev, PCIE_GLI_9767_VHS, value);
> +
> +       pci_read_config_dword(pdev, PCIE_GLI_9767_PWR_MACRO_CTL, &value);
> +       value &= ~(PCIE_GLI_9767_PWR_MACRO_CTL_LOW_VOLTAGE |
> +                  PCIE_GLI_9767_PWR_MACRO_CTL_LD0_LOW_OUTPUT_VOLTAGE |
> +                  PCIE_GLI_9767_PWR_MACRO_CTL_RCLK_AMPLITUDE_CTL);
> +
> +       value |= PCIE_GLI_9767_PWR_MACRO_CTL_LOW_VOLTAGE |
> +                FIELD_PREP(PCIE_GLI_9767_PWR_MACRO_CTL_LD0_LOW_OUTPUT_VOLTAGE,
> +                           PCIE_GLI_9767_PWR_MACRO_CTL_LD0_LOW_OUTPUT_VOLTAGE_VALUE) |
> +                FIELD_PREP(PCIE_GLI_9767_PWR_MACRO_CTL_RCLK_AMPLITUDE_CTL,
> +                           PCIE_GLI_9767_PWR_MACRO_CTL_RCLK_AMPLITUDE_CTL_VALUE);
> +       pci_write_config_dword(pdev, PCIE_GLI_9767_PWR_MACRO_CTL, value);
> +
> +       pci_read_config_dword(pdev, PCIE_GLI_9767_SCR, &value);
> +       value &= ~(PCIE_GLI_9767_SCR_SYSTEM_CLK_SELECT_MODE0 |
> +                  PCIE_GLI_9767_SCR_SYSTEM_CLK_SELECT_MODE1 |
> +                  PCIE_GLI_9767_SCR_CFG_RST_DATA_LINK_DOWN);
> +
> +       value |= PCIE_GLI_9767_SCR_AUTO_AXI_W_BURST |
> +                PCIE_GLI_9767_SCR_AUTO_AXI_R_BURST |
> +                PCIE_GLI_9767_SCR_AXI_REQ |
> +                PCIE_GLI_9767_SCR_CARD_DET_PWR_SAVING_EN |
> +                PCIE_GLI_9767_SCR_CORE_PWR_D3_OFF;
> +       pci_write_config_dword(pdev, PCIE_GLI_9767_SCR, value);
> +
> +       pci_read_config_dword(pdev, PCIE_GLI_9767_VHS, &value);
> +       value &= ~GLI_9767_VHS_REV;
> +       value |= FIELD_PREP(GLI_9767_VHS_REV, GLI_9767_VHS_REV_R);
> +       pci_write_config_dword(pdev, PCIE_GLI_9767_VHS, value);
> +}
> +
> +static void sdhci_gl9767_reset(struct sdhci_host *host, u8 mask)
> +{
> +       sdhci_reset(host, mask);
> +       gli_set_9767(host);
> +}
> +
>  static int gli_probe_slot_gl9750(struct sdhci_pci_slot *slot)
>  {
>         struct sdhci_host *host = slot->host;
> @@ -717,6 +798,19 @@ static int gli_probe_slot_gl9755(struct sdhci_pci_slot *slot)
>         return 0;
>  }
>
> +static int gli_probe_slot_gl9767(struct sdhci_pci_slot *slot)
> +{
> +       struct sdhci_host *host = slot->host;
> +
> +       gli_set_9767(host);
> +       gl9767_hw_setting(slot);
> +       gli_pcie_enable_msi(slot);
> +       slot->host->mmc->caps2 |= MMC_CAP2_NO_SDIO;
> +       sdhci_enable_v4_mode(host);
> +
> +       return 0;
> +}
> +
>  static void sdhci_gli_voltage_switch(struct sdhci_host *host)
>  {
>         /*
> @@ -740,6 +834,25 @@ static void sdhci_gli_voltage_switch(struct sdhci_host *host)
>         usleep_range(100000, 110000);
>  }
>
> +static void sdhci_gl9767_voltage_switch(struct sdhci_host *host)
> +{
> +       /*
> +        * According to Section 3.6.1 signal voltage switch procedure in
> +        * SD Host Controller Simplified Spec. 4.20, steps 6~8 are as
> +        * follows:
> +        * (6) Set 1.8V Signal Enable in the Host Control 2 register.
> +        * (7) Wait 5ms. 1.8V voltage regulator shall be stable within this
> +        *     period.
> +        * (8) If 1.8V Signal Enable is cleared by Host Controller, go to
> +        *     step (12).
> +        *
> +        * Wait 5ms after set 1.8V signal enable in Host Control 2 register
> +        * to ensure 1.8V signal enable bit is set by GL9767.
> +        *
> +        */
> +       usleep_range(5000, 5500);
> +}
> +
>  static void sdhci_gl9750_reset(struct sdhci_host *host, u8 mask)
>  {
>         sdhci_reset(host, mask);
> @@ -1150,3 +1263,22 @@ const struct sdhci_pci_fixes sdhci_gl9763e = {
>  #endif
>         .add_host       = gl9763e_add_host,
>  };
> +
> +static const struct sdhci_ops sdhci_gl9767_ops = {
> +       .set_clock              = sdhci_set_clock,
> +       .enable_dma             = sdhci_pci_enable_dma,
> +       .set_bus_width          = sdhci_set_bus_width,
> +       .reset                  = sdhci_gl9767_reset,
> +       .set_uhs_signaling      = sdhci_set_uhs_signaling,
> +       .voltage_switch         = sdhci_gl9767_voltage_switch,
> +};
> +
> +const struct sdhci_pci_fixes sdhci_gl9767 = {
> +       .quirks         = SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC,
> +       .quirks2        = SDHCI_QUIRK2_BROKEN_DDR50,
> +       .probe_slot     = gli_probe_slot_gl9767,
> +       .ops            = &sdhci_gl9767_ops,
> +#ifdef CONFIG_PM_SLEEP
> +       .resume         = sdhci_pci_gli_resume,
> +#endif
> +};
> diff --git a/drivers/mmc/host/sdhci-pci.h b/drivers/mmc/host/sdhci-pci.h
> index 3661a224fb04..9c8863956381 100644
> --- a/drivers/mmc/host/sdhci-pci.h
> +++ b/drivers/mmc/host/sdhci-pci.h
> @@ -76,6 +76,7 @@
>  #define PCI_DEVICE_ID_GLI_9755         0x9755
>  #define PCI_DEVICE_ID_GLI_9750         0x9750
>  #define PCI_DEVICE_ID_GLI_9763E                0xe763
> +#define PCI_DEVICE_ID_GLI_9767         0x9767
>
>  /*
>   * PCI device class and mask
> @@ -195,5 +196,6 @@ extern const struct sdhci_pci_fixes sdhci_o2;
>  extern const struct sdhci_pci_fixes sdhci_gl9750;
>  extern const struct sdhci_pci_fixes sdhci_gl9755;
>  extern const struct sdhci_pci_fixes sdhci_gl9763e;
> +extern const struct sdhci_pci_fixes sdhci_gl9767;
>
>  #endif /* __SDHCI_PCI_H */
> --
> 2.25.1
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ