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] [day] [month] [year] [list]
Date:   Fri, 12 Apr 2019 09:48:01 +0000
From:   <orito.takao@...ionext.com>
To:     <ulf.hansson@...aro.org>
CC:     <robh+dt@...nel.org>, <mark.rutland@....com>,
        <linux-mmc@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <masami.hiramatsu@...aro.org>,
        <jaswinder.singh@...aro.org>, <sugaya.taichi@...ionext.com>,
        <kasai.kazuhiro@...ionext.com>, <kanematsu.shinji@...ionext.com>
Subject: Re: [Milbeaut PATCH v0.4 2/2] mmc: sdhci_f_sdh30: Add support for
 Milbeaut M10V host controller


Hi

Thank you for you comment.

> On Tue, 26 Mar 2019 at 05:56, Takao Orito <orito.takao@...ionext.com> wrote:
> >
> > SD Host controller on Milbeaut is consist of two controller parts.
> > One is core controller F_SDH30, this is similar to sdhci-fujitsu
> > controller.
> > Another is bridge controller.
> > This bridge controller is not compatible with sdhci-fujitsu controller.
> > This is special for Milbeaut series. This has some functions.
> > For example, reset control, clock enable/select for SDR50/25/12, set
> > property of SD physical pins, retuning control, set capabilityies.
> >
> > This bridge controller requires special procedures at reset or clock
> > enablement or change for further tuning of clock.
> >
> > This new driver uses GPIO descriptor from platform data to switch
> > the power.
> >
> > Signed-off-by: Takao Orito <orito.takao@...ionext.com>
> 
> Looks like you should add a new variant driver instead of extending
> sdhci_f_sdh30.c.
> 
> If there are some registers bits or some other minor things that you
> want to share among these variants, you can always do that via a
> header file.

Ok, I see.
I will think to prepare a new variant driver with new common header file
to be shared with sdhci_f_sdh30.c

Thanks
Orito

> 
> Kind regards
> Uffe
> 
> > ---
> >  drivers/mmc/host/sdhci_f_sdh30.c | 324 +++++++++++++++++++++++++++++++++++----
> >  1 file changed, 296 insertions(+), 28 deletions(-)
> >
> > diff --git a/drivers/mmc/host/sdhci_f_sdh30.c b/drivers/mmc/host/sdhci_f_sdh30.c
> > index 485f759..994bafd 100644
> > --- a/drivers/mmc/host/sdhci_f_sdh30.c
> > +++ b/drivers/mmc/host/sdhci_f_sdh30.c
> > @@ -11,12 +11,15 @@
> >   */
> >
> >  #include <linux/acpi.h>
> > -#include <linux/err.h>
> > +#include <linux/clk.h>
> >  #include <linux/delay.h>
> > +#include <linux/err.h>
> > +#include <linux/bits.h>
> > +#include <linux/gpio/consumer.h>
> >  #include <linux/module.h>
> >  #include <linux/of.h>
> > +#include <linux/of_device.h>
> >  #include <linux/property.h>
> > -#include <linux/clk.h>
> >
> >  #include "sdhci-pltfm.h"
> >
> > @@ -43,14 +46,55 @@
> >
> >  #define F_SDH30_CMD_DAT_DELAY          0x200
> >
> > +/* milbeaut bridge controller register */
> > +#define MLB_SOFT_RESET         0x0200
> > +#define  MLB_SOFT_RESET_RSTX           BIT(0)
> > +
> > +#define MLB_WP_CD_LED_SET      0x0210
> > +#define  MLB_WP_CD_LED_SET_LED_INV  BIT(2)
> > +
> > +#define MLB_CR_SET                     0x0220
> > +#define  MLB_CR_SET_CR_TOCLKUNIT       BIT(24)
> > +#define  MLB_CR_SET_CR_TOCLKFREQ_SFT   (16)
> > +#define  MLB_CR_SET_CR_TOCLKFREQ_MASK  (0x3F << MLB_CR_SET_CR_TOCLKFREQ_SFT)
> > +#define  MLB_CR_SET_CR_BCLKFREQ_SFT    (8)
> > +#define  MLB_CR_SET_CR_BCLKFREQ_MASK   (0xFF << MLB_CR_SET_CR_BCLKFREQ_SFT)
> > +#define  MLB_CR_SET_CR_RTUNTIMER_SFT   (4)
> > +#define  MLB_CR_SET_CR_RTUNTIMER_MASK  (0xF << MLB_CR_SET_CR_RTUNTIMER_SFT)
> > +
> > +#define MLB_SD_TOCLK_I_DIV     16
> > +#define MLB_TOCLKFREQ_UNIT_THRES       16000000
> > +#define MLB_CAL_TOCLKFREQ_MHZ(rate)    (rate / MLB_SD_TOCLK_I_DIV / 1000000)
> > +#define MLB_CAL_TOCLKFREQ_KHZ(rate)    (rate / MLB_SD_TOCLK_I_DIV / 1000)
> > +#define MLB_TOCLKFREQ_MAX      63
> > +#define MLB_TOCLKFREQ_MIN       1
> > +
> > +#define MLB_SD_BCLK_I_DIV      4
> > +#define MLB_CAL_BCLKFREQ(rate) (rate / MLB_SD_BCLK_I_DIV / 1000000)
> > +#define MLB_BCLKFREQ_MAX               255
> > +#define MLB_BCLKFREQ_MIN                 1
> > +
> > +#define MLB_CDR_SET                    0x0230
> > +#define MLB_CDR_SET_CLK2POW16  3
> > +
> >  #define F_SDH30_MIN_CLOCK              400000
> >
> > +struct f_sdh30_config {
> > +       void (*reset)(struct sdhci_host *host, u8 mask);
> > +       void (*init)(struct sdhci_host *host);
> > +       void (*quirk_init)(struct sdhci_host *host);
> > +       void (*prepare_power)(struct sdhci_host *host, unsigned char mode,
> > +                                        unsigned short vdd);
> > +};
> > +
> >  struct f_sdhost_priv {
> >         struct clk *clk_iface;
> >         struct clk *clk;
> >         u32 vendor_hs200;
> >         struct device *dev;
> >         bool enable_cmd_dat_delay;
> > +       struct gpio_desc *power_gpio;
> > +       struct f_sdh30_config *config;
> >  };
> >
> >  static void sdhci_f_sdh30_soft_voltage_switch(struct sdhci_host *host)
> > @@ -86,7 +130,46 @@ static unsigned int sdhci_f_sdh30_get_min_clock(struct sdhci_host *host)
> >         return F_SDH30_MIN_CLOCK;
> >  }
> >
> > -static void sdhci_f_sdh30_reset(struct sdhci_host *host, u8 mask)
> > +static void sdhci_milbeaut_reset(struct sdhci_host *host, u8 mask)
> > +{
> > +       struct f_sdhost_priv *priv = sdhci_priv(host);
> > +       u16 clk;
> > +       u32 ctl;
> > +       ktime_t timeout;
> > +
> > +       clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL);
> > +       clk = (clk & ~SDHCI_CLOCK_CARD_EN) | SDHCI_CLOCK_INT_EN;
> > +       sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
> > +
> > +       sdhci_reset(host, mask);
> > +
> > +       clk |= SDHCI_CLOCK_CARD_EN;
> > +       sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
> > +
> > +       timeout = ktime_add_ms(ktime_get(), 10);
> > +       while (1) {
> > +               bool timedout = ktime_after(ktime_get(), timeout);
> > +
> > +               clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL);
> > +               if (clk & SDHCI_CLOCK_INT_STABLE)
> > +                       break;
> > +               if (timedout) {
> > +                       pr_err("%s: Internal clock never stabilised.\n",
> > +                               mmc_hostname(host->mmc));
> > +                       sdhci_dumpregs(host);
> > +                       return;
> > +               }
> > +               udelay(10);
> > +       }
> > +
> > +       if (priv->enable_cmd_dat_delay) {
> > +               ctl = sdhci_readl(host, F_SDH30_ESD_CONTROL);
> > +               ctl |= F_SDH30_CMD_DAT_DELAY;
> > +               sdhci_writel(host, ctl, F_SDH30_ESD_CONTROL);
> > +       }
> > +}
> > +
> > +static void sdhci_mb86s70_reset(struct sdhci_host *host, u8 mask)
> >  {
> >         struct f_sdhost_priv *priv = sdhci_priv(host);
> >         u32 ctl;
> > @@ -103,6 +186,51 @@ static void sdhci_f_sdh30_reset(struct sdhci_host *host, u8 mask)
> >         }
> >  }
> >
> > +static void sdhci_f_sdh30_reset(struct sdhci_host *host, u8 mask)
> > +{
> > +       struct f_sdhost_priv *priv = sdhci_priv(host);
> > +
> > +       priv->config->reset(host, mask);
> > +}
> > +
> > +static void sdhci_milbeaut_power_off(struct f_sdhost_priv *priv)
> > +{
> > +       gpiod_set_value(priv->power_gpio, 0);
> > +       udelay(1000);
> > +}
> > +
> > +static void sdhci_milbeaut_power_on(struct f_sdhost_priv *priv)
> > +{
> > +       gpiod_set_value(priv->power_gpio, 1);
> > +}
> > +
> > +static void sdhci_milbeaut_prepare_power(struct sdhci_host *host,
> > +                       unsigned char mode, unsigned short vdd)
> > +{
> > +       struct f_sdhost_priv *priv = sdhci_priv(host);
> > +
> > +       if (!IS_ERR(priv->power_gpio))
> > +               switch (mode) {
> > +               case MMC_POWER_UP:
> > +                       sdhci_milbeaut_power_on(priv);
> > +                       break;
> > +               case MMC_POWER_OFF:
> > +                       sdhci_milbeaut_power_off(priv);
> > +                       return;
> > +               }
> > +}
> > +
> > +static void sdhci_f_sdh30_set_power(struct sdhci_host *host,
> > +                               unsigned char mode, unsigned short vdd)
> > +{
> > +       struct f_sdhost_priv *priv = sdhci_priv(host);
> > +
> > +       if (priv->config->prepare_power)
> > +               priv->config->prepare_power(host, mode, vdd);
> > +
> > +       sdhci_set_power(host, mode, vdd);
> > +}
> > +
> >  static const struct sdhci_ops sdhci_f_sdh30_ops = {
> >         .voltage_switch = sdhci_f_sdh30_soft_voltage_switch,
> >         .get_min_clock = sdhci_f_sdh30_get_min_clock,
> > @@ -110,6 +238,156 @@ static void sdhci_f_sdh30_reset(struct sdhci_host *host, u8 mask)
> >         .set_clock = sdhci_set_clock,
> >         .set_bus_width = sdhci_set_bus_width,
> >         .set_uhs_signaling = sdhci_set_uhs_signaling,
> > +       .set_power = sdhci_f_sdh30_set_power,
> > +};
> > +
> > +static void sdhci_milbeaut_bridge_reset(struct sdhci_host *host, int reset_flag)
> > +{
> > +       if (reset_flag)
> > +               sdhci_writel(host, 0, MLB_SOFT_RESET);
> > +       else
> > +               sdhci_writel(host, MLB_SOFT_RESET_RSTX, MLB_SOFT_RESET);
> > +}
> > +
> > +static void sdhci_milbeaut_bridge_init(struct sdhci_host *host, int rate)
> > +{
> > +       u32 val, clk;
> > +
> > +       /* IO_SDIO_CR_SET should be set while reset */
> > +       val = sdhci_readl(host, MLB_CR_SET);
> > +       val &= ~(MLB_CR_SET_CR_TOCLKFREQ_MASK | MLB_CR_SET_CR_TOCLKUNIT |
> > +                               MLB_CR_SET_CR_BCLKFREQ_MASK);
> > +       if (rate >= MLB_TOCLKFREQ_UNIT_THRES) {
> > +               clk = MLB_CAL_TOCLKFREQ_MHZ(rate);
> > +               clk = min_t(u32, MLB_TOCLKFREQ_MAX, clk);
> > +               val |= MLB_CR_SET_CR_TOCLKUNIT |
> > +                       (clk << MLB_CR_SET_CR_TOCLKFREQ_SFT);
> > +       } else {
> > +               clk = MLB_CAL_TOCLKFREQ_KHZ(rate);
> > +               clk = min_t(u32, MLB_TOCLKFREQ_MAX, clk);
> > +               clk = max_t(u32, MLB_TOCLKFREQ_MIN, clk);
> > +               val |= clk << MLB_CR_SET_CR_TOCLKFREQ_SFT;
> > +       }
> > +
> > +       clk = MLB_CAL_BCLKFREQ(rate);
> > +       clk = min_t(u32, MLB_BCLKFREQ_MAX, clk);
> > +       clk = max_t(u32, MLB_BCLKFREQ_MIN, clk);
> > +       val |=  clk << MLB_CR_SET_CR_BCLKFREQ_SFT;
> > +
> > +       val &= ~MLB_CR_SET_CR_RTUNTIMER_MASK;
> > +
> > +       sdhci_writel(host, val, MLB_CR_SET);
> > +
> > +       sdhci_writel(host, MLB_CDR_SET_CLK2POW16, MLB_CDR_SET);
> > +
> > +       sdhci_writel(host, MLB_WP_CD_LED_SET_LED_INV, MLB_WP_CD_LED_SET);
> > +}
> > +
> > +static void sdhci_milbeaut_vendor_init(struct sdhci_host *host)
> > +{
> > +       struct f_sdhost_priv *priv = sdhci_priv(host);
> > +       u32 ctl;
> > +
> > +       ctl = sdhci_readl(host, F_SDH30_IO_CONTROL2);
> > +       ctl |= F_SDH30_CRES_O_DN;
> > +       sdhci_writel(host, ctl, F_SDH30_IO_CONTROL2);
> > +       ctl &= ~F_SDH30_MSEL_O_1_8;
> > +       sdhci_writel(host, ctl, F_SDH30_IO_CONTROL2);
> > +       ctl &= ~F_SDH30_CRES_O_DN;
> > +       sdhci_writel(host, ctl, F_SDH30_IO_CONTROL2);
> > +
> > +       ctl = sdhci_readw(host, F_SDH30_AHB_CONFIG);
> > +       ctl |= F_SDH30_SIN | F_SDH30_AHB_INCR_16 | F_SDH30_AHB_INCR_8 |
> > +                  F_SDH30_AHB_INCR_4;
> > +       ctl &= ~(F_SDH30_AHB_BIGED | F_SDH30_BUSLOCK_EN);
> > +       sdhci_writew(host, ctl, F_SDH30_AHB_CONFIG);
> > +
> > +       if (priv->enable_cmd_dat_delay) {
> > +               ctl = sdhci_readl(host, F_SDH30_ESD_CONTROL);
> > +               ctl |= F_SDH30_CMD_DAT_DELAY;
> > +               sdhci_writel(host, ctl, F_SDH30_ESD_CONTROL);
> > +       }
> > +}
> > +
> > +static void sdhci_milbeaut_init(struct sdhci_host *host)
> > +{
> > +       struct f_sdhost_priv *priv = sdhci_priv(host);
> > +       int rate = clk_get_rate(priv->clk);
> > +       u16 ctl;
> > +
> > +       sdhci_milbeaut_bridge_reset(host, 0);
> > +
> > +       ctl = sdhci_readw(host, SDHCI_CLOCK_CONTROL);
> > +       ctl &= ~(SDHCI_CLOCK_CARD_EN | SDHCI_CLOCK_INT_EN);
> > +       sdhci_writew(host, ctl, SDHCI_CLOCK_CONTROL);
> > +
> > +       sdhci_milbeaut_bridge_reset(host, 1);
> > +
> > +       priv->power_gpio = devm_gpiod_get_optional(priv->dev,
> > +                        "sni,mmc-power", GPIOD_OUT_LOW);
> > +       if (!IS_ERR(priv->power_gpio)) {
> > +               sdhci_milbeaut_power_off(priv);
> > +               msleep(20); /* need for card power off duration */
> > +       }
> > +
> > +       sdhci_milbeaut_bridge_init(host, rate);
> > +       sdhci_milbeaut_bridge_reset(host, 0);
> > +
> > +       sdhci_milbeaut_vendor_init(host);
> > +}
> > +
> > +static void sdhci_mb86s70_init(struct sdhci_host *host)
> > +{
> > +       struct f_sdhost_priv *priv = sdhci_priv(host);
> > +       u32 ctrl, reg;
> > +
> > +       /* init vendor specific regs */
> > +       ctrl = sdhci_readw(host, F_SDH30_AHB_CONFIG);
> > +       ctrl |= F_SDH30_SIN | F_SDH30_AHB_INCR_16 | F_SDH30_AHB_INCR_8 |
> > +               F_SDH30_AHB_INCR_4;
> > +       ctrl &= ~(F_SDH30_AHB_BIGED | F_SDH30_BUSLOCK_EN);
> > +       sdhci_writew(host, ctrl, F_SDH30_AHB_CONFIG);
> > +
> > +       reg = sdhci_readl(host, F_SDH30_ESD_CONTROL);
> > +       sdhci_writel(host, reg & ~F_SDH30_EMMC_RST, F_SDH30_ESD_CONTROL);
> > +       msleep(20);
> > +       sdhci_writel(host, reg | F_SDH30_EMMC_RST, F_SDH30_ESD_CONTROL);
> > +
> > +       reg = sdhci_readl(host, SDHCI_CAPABILITIES);
> > +       if (reg & SDHCI_CAN_DO_8BIT)
> > +               priv->vendor_hs200 = F_SDH30_EMMC_HS200;
> > +}
> > +
> > +static void sdhci_milbeaut_quirk_init(struct sdhci_host *host)
> > +{
> > +       host->quirks = SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC |
> > +                          SDHCI_QUIRK_INVERTED_WRITE_PROTECT |
> > +                          SDHCI_QUIRK_CLOCK_BEFORE_RESET |
> > +                          SDHCI_QUIRK_DELAY_AFTER_POWER;
> > +       host->quirks2 = SDHCI_QUIRK2_SUPPORT_SINGLE |
> > +                       SDHCI_QUIRK2_TUNING_WORK_AROUND |
> > +                       SDHCI_QUIRK2_PRESET_VALUE_BROKEN;
> > +}
> > +
> > +static void sdhci_mb86s70_quirk_init(struct sdhci_host *host)
> > +{
> > +       host->quirks = SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC |
> > +                          SDHCI_QUIRK_INVERTED_WRITE_PROTECT;
> > +       host->quirks2 = SDHCI_QUIRK2_SUPPORT_SINGLE |
> > +                       SDHCI_QUIRK2_TUNING_WORK_AROUND;
> > +}
> > +
> > +struct f_sdh30_config mb86s70_config = {
> > +       .reset = sdhci_mb86s70_reset,
> > +       .quirk_init = sdhci_mb86s70_quirk_init,
> > +       .init = sdhci_mb86s70_init,
> > +};
> > +
> > +struct f_sdh30_config milbeaut_config = {
> > +       .reset = sdhci_milbeaut_reset,
> > +       .prepare_power = sdhci_milbeaut_prepare_power,
> > +       .quirk_init = sdhci_milbeaut_quirk_init,
> > +       .init = sdhci_milbeaut_init,
> >  };
> >
> >  static int sdhci_f_sdh30_probe(struct platform_device *pdev)
> > @@ -117,9 +395,8 @@ static int sdhci_f_sdh30_probe(struct platform_device *pdev)
> >         struct sdhci_host *host;
> >         struct device *dev = &pdev->dev;
> >         struct resource *res;
> > -       int irq, ctrl = 0, ret = 0;
> > +       int irq, ret = 0;
> >         struct f_sdhost_priv *priv;
> > -       u32 reg = 0;
> >
> >         irq = platform_get_irq(pdev, 0);
> >         if (irq < 0) {
> > @@ -133,14 +410,12 @@ static int sdhci_f_sdh30_probe(struct platform_device *pdev)
> >
> >         priv = sdhci_priv(host);
> >         priv->dev = dev;
> > +       priv->config = (struct f_sdh30_config *)of_device_get_match_data(dev);
> > +       if (!priv->config)
> > +               /* this must be matched by ACPI, fall down to mb86s70 compat */
> > +               priv->config = &mb86s70_config;
> >
> > -       host->quirks = SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC |
> > -                      SDHCI_QUIRK_INVERTED_WRITE_PROTECT;
> > -       host->quirks2 = SDHCI_QUIRK2_SUPPORT_SINGLE |
> > -                       SDHCI_QUIRK2_TUNING_WORK_AROUND;
> > -
> > -       priv->enable_cmd_dat_delay = device_property_read_bool(dev,
> > -                                               "fujitsu,cmd-dat-delay-select");
> > +       priv->config->quirk_init(host);
> >
> >         ret = mmc_of_parse(host->mmc);
> >         if (ret)
> > @@ -183,21 +458,7 @@ static int sdhci_f_sdh30_probe(struct platform_device *pdev)
> >                         goto err_clk;
> >         }
> >
> > -       /* init vendor specific regs */
> > -       ctrl = sdhci_readw(host, F_SDH30_AHB_CONFIG);
> > -       ctrl |= F_SDH30_SIN | F_SDH30_AHB_INCR_16 | F_SDH30_AHB_INCR_8 |
> > -               F_SDH30_AHB_INCR_4;
> > -       ctrl &= ~(F_SDH30_AHB_BIGED | F_SDH30_BUSLOCK_EN);
> > -       sdhci_writew(host, ctrl, F_SDH30_AHB_CONFIG);
> > -
> > -       reg = sdhci_readl(host, F_SDH30_ESD_CONTROL);
> > -       sdhci_writel(host, reg & ~F_SDH30_EMMC_RST, F_SDH30_ESD_CONTROL);
> > -       msleep(20);
> > -       sdhci_writel(host, reg | F_SDH30_EMMC_RST, F_SDH30_ESD_CONTROL);
> > -
> > -       reg = sdhci_readl(host, SDHCI_CAPABILITIES);
> > -       if (reg & SDHCI_CAN_DO_8BIT)
> > -               priv->vendor_hs200 = F_SDH30_EMMC_HS200;
> > +       priv->config->init(host);
> >
> >         ret = sdhci_add_host(host);
> >         if (ret)
> > @@ -233,7 +494,14 @@ static int sdhci_f_sdh30_remove(struct platform_device *pdev)
> >
> >  #ifdef CONFIG_OF
> >  static const struct of_device_id f_sdh30_dt_ids[] = {
> > -       { .compatible = "fujitsu,mb86s70-sdhci-3.0" },
> > +       {
> > +               .compatible = "fujitsu,mb86s70-sdhci-3.0",
> > +               .data = &mb86s70_config,
> > +       },
> > +       {
> > +               .compatible = "socionext,milbeaut-m10v-sdhci-3.0",
> > +               .data = &milbeaut_config,
> > +       },
> >         { /* sentinel */ }
> >  };
> >  MODULE_DEVICE_TABLE(of, f_sdh30_dt_ids);
> > --
> > 1.9.1
> >
> >

-- 
Takao Orito <orito.takao@...ionext.com>

Powered by blists - more mailing lists