| 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
| ||
|
Message-ID: <856809930bb84df5ae428beabb7f9a63@realtek.com>
Date: Thu, 2 Nov 2023 07:58:44 +0000
From: Jyan Chou [周芷安] <jyanchou@...ltek.com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
"ulf.hansson@...aro.org" <ulf.hansson@...aro.org>,
"adrian.hunter@...el.com" <adrian.hunter@...el.com>,
"jh80.chung@...sung.com" <jh80.chung@...sung.com>,
"riteshh@...eaurora.org" <riteshh@...eaurora.org>,
"robh+dt@...nel.org" <robh+dt@...nel.org>,
"krzysztof.kozlowski+dt@...aro.org"
<krzysztof.kozlowski+dt@...aro.org>,
"conor+dt@...nel.org" <conor+dt@...nel.org>,
"asutoshd@...eaurora.org" <asutoshd@...eaurora.org>,
"p.zabel@...gutronix.de" <p.zabel@...gutronix.de>
CC: "linux-mmc@...r.kernel.org" <linux-mmc@...r.kernel.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"arnd@...db.de" <arnd@...db.de>,
"briannorris@...omium.org" <briannorris@...omium.org>,
"doug@...morgal.com" <doug@...morgal.com>,
"tonyhuang.sunplus@...il.com" <tonyhuang.sunplus@...il.com>,
"abel.vesa@...aro.org" <abel.vesa@...aro.org>,
"william.qiu@...rfivetech.com" <william.qiu@...rfivetech.com>
Subject: RE: [PATCH V4][3/4] mmc: Add dw mobile mmc cmdq rtk driver
>> Add Realtek mmc driver to make good use Synopsys DesignWare mmc cmdq
>> host driver.
>>
>> Signed-off-by: Jyan Chou <jyanchou@...ltek.com>
>>
>> ---
>> v3 -> v4:
>> - Modify dma setting's code to fix linux coding style.
> And coding style of all other parts were ignored. You must fix it everywhere in your code.
Thanks for your remind, we checked other linux coding style issues.
...
>> +
>> +static void dw_mci_rtk_init_card(struct mmc_host *host, struct
>> +mmc_card *card) {
>> + /* In Realtek Platform, we need to attach eMMC card onto mmc host
>> + * during eMMC initialization because of the following reason:
>> + * When system cannot run the hs400, we need to down speed to hs200
>> + * and call mmc_hw_reset and modify the mmc card attribute through mmc host.
>> + * At this moment, system will show errors if host->card = NULL.
>> + */
>> + host->card = card;
>> +}
>> +
>> +static int dw_mci_rtk_parse_dt(struct dw_mci *host) {
>> + struct dw_mci_rtkemmc_host *priv;
>> + const u32 *prop;
>> + int size;
>> +
>> + priv = devm_kzalloc(host->dev, sizeof(struct
>> + dw_mci_rtkemmc_host), GFP_KERNEL);
> sizeof(*)
We had corrected it.
>> + if (!priv)
>> + return -ENOMEM;
>> +
>> + priv->pinctrl = devm_pinctrl_get(host->dev);
>> + if (IS_ERR(priv->pinctrl))
>> + dev_dbg(host->dev, "no pinctrl\n");
>> +
>> + priv->pins_default = pinctrl_lookup_state(priv->pinctrl,
>> + PINCTRL_STATE_DEFAULT);
>> + if (IS_ERR(priv->pins_default))
>> + dev_warn(host->dev, "could not get default state\n");
>> +
> So this is required by the driver but not by the bindings.
These pinctrl setting are required by the driver passing from the bindings.
>> + priv->pins_sdr50 = pinctrl_lookup_state(priv->pinctrl,
>> + "sdr50");
>> + if (IS_ERR(priv->pins_sdr50))
>> + dev_warn(host->dev, "could not get sdr50 state\n");
>> +
>> + priv->pins_hs200 = pinctrl_lookup_state(priv->pinctrl,
>> + "hs200");
>> + if (IS_ERR(priv->pins_hs200))
>> + dev_warn(host->dev, "could not get hs200 state\n");
>> +
>> + priv->pins_hs400 = pinctrl_lookup_state(priv->pinctrl,
>> + "hs400");
>> + if (IS_ERR(priv->pins_hs400))
>> + dev_warn(host->dev, "could not get hs400 state\n");
>> +
>> + priv->pins_tune0 = pinctrl_lookup_state(priv->pinctrl,
>> + "tune0");
>> + if (IS_ERR(priv->pins_tune0))
>> + dev_warn(host->dev, "could not get tune0 state\n");
>> +
>> + priv->pins_tune1 = pinctrl_lookup_state(priv->pinctrl,
>> + "tune1");
>> + if (IS_ERR(priv->pins_tune1))
>> + dev_warn(host->dev, "could not get tune1 state\n");
>> +
>> + priv->pins_tune2 = pinctrl_lookup_state(priv->pinctrl,
>> + "tune2");
>> + if (IS_ERR(priv->pins_tune2))
>> + dev_warn(host->dev, "could not get tune2 state\n");
>> +
>> + priv->pins_tune3 = pinctrl_lookup_state(priv->pinctrl,
>> + "tune3");
>> + if (IS_ERR(priv->pins_tune3))
>> + dev_warn(host->dev, "could not get tune3 state\n");
>> +
>> + priv->pins_tune4 = pinctrl_lookup_state(priv->pinctrl,
>> + "tune4");
>> +
>> + if (IS_ERR(priv->pins_tune4))
>> + dev_warn(host->dev, "could not get tune4 state\n");
>> +
>> + priv->vp0 = devm_clk_get(host->dev, "vp0");
>> + if (IS_ERR(priv->vp0))
>> + dev_err(host->dev, "could not get vp0 clk\n");
>> +
>> + priv->vp1 = devm_clk_get(host->dev, "vp1");
>> + if (IS_ERR(priv->vp1))
>> + dev_err(host->dev, "could not get vp1 clk\n");
> dev_err_probe. Everywhere where applicable.
We had replace dev_err to dev_err_probe for error prints during probe.
>> +
>> + priv->emmc_mode = 0;
>> + prop = of_get_property(host->dev->of_node, "speed-step", &size);
>> + if (prop) {
>> + priv->emmc_mode = of_read_number(prop, 1);
>> + dev_info(host->dev, "emmc mode : %d\n",
>> + priv->emmc_mode);
> Drop
Okay.
>> + } else {
>> + dev_info(host->dev, "use default emmc sdr50 mode !\n");
> Drop, why is this a problem?
Okay.
>> + }
>> +
>> + priv->is_cqe = 0;
>> + prop = of_get_property(host->dev->of_node, "cqe", &size);
>> + if (prop) {
>> + priv->is_cqe = of_read_number(prop, 1);
>> + dev_info(host->dev, "cmdq mode : %d\n", priv->is_cqe);
> Drop
Okay.
>> + } else {
>> + dev_info(host->dev, "use default eMMC legacy mode !\n");
> Drop
Okay.
>> + }
>> +
>> + prop = of_get_property(host->dev->of_node, "rdq-ctrl", &size);
>> + if (prop) {
>> + priv->rdq_ctrl = of_read_number(prop, 1);
>> + dev_info(host->dev, "get rdq-ctrl : %u\n",
>> + priv->rdq_ctrl);
> Drop
Okay.
>> + } else {
>> + priv->rdq_ctrl = 0;
>> + dev_info(host->dev, "no dqs_dly_tape switch node, use
>> + default 0x0 !!\n");
> Drop
Okay.
>> + }
>> +
>> + priv->m2tmx = syscon_regmap_lookup_by_phandle(host->dev->of_node, "realtek,m2tmx");
>> + if (IS_ERR_OR_NULL(priv->m2tmx))
>> + dev_err(host->dev, "can not get m2mtx node.\n");
>> +
>> + host->priv = priv;
>> +
>> + return 0;
>> +}
>> +
>> +static int dw_mci_rtk_init(struct dw_mci *host) {
>> + struct dw_mci_rtkemmc_host *priv = host->priv;
>> +
>> + host->pdata->caps2 = MMC_CAP2_NO_SDIO | MMC_CAP2_NO_SD;
>> +
>> + if (priv->emmc_mode >= 2)
>> + host->pdata->caps2 |= MMC_CAP2_HS200_1_8V_SDR;
>> + if (priv->emmc_mode >= 3) {
>> + host->pdata->caps |= MMC_CAP_1_8V_DDR;
>> + host->pdata->caps2 |= MMC_CAP2_HS400_1_8V;
>> + }
>> +
>> + if (priv->is_cqe > 0)
>> + host->pdata->caps2 |= (MMC_CAP2_CQE |
>> + MMC_CAP2_CQE_DCMD);
>> +
>> + host->irq_flags = IRQF_SHARED;
>> +
>> + mcq_writel(host, CP, 0x0);
>> +
>> + /*Enable L4 gated*/
> Read Linux coding style. Multiple times if needed.
Thanks for your remind.
>> + mcq_writel(host, OTHER1, mcq_readl(host, OTHER1) &
>> + ~(SDMMC_L4_GATED_DIS | SDMMC_L4_GATED_DIS1));
>> +
>> + mcq_writel(host, OTHER1, mcq_readl(host, OTHER1) &
>> + (~(SDMMC_DQS_CTRL_GATE_DIS |
>> + SDMMC_DBUS_MAS_GATING_DIS)));
>> +
>> + /*Set the eMMC wrapper little Endian*/
>> + mcq_writel(host, AHB, mcq_readl(host, AHB) | SDMMC_AHB_BIG);
>> +
>> + mcq_writel(host, OTHER1,
>> + mcq_readl(host, OTHER1) |
>> + SDMMC_STARK_CARD_STOP_ENABLE);
>> +
>> + /*set eMMC instead of nand*/
>> + regmap_update_bits_base(priv->m2tmx, SDMMC_NAND_DMA_SEL,
>> + SDMMC_SRAM_DMA_SEL, SDMMC_SRAM_DMA_SEL,
>> + NULL, false, true);
>> +
>> + /*Set the clk initial phase*/
>> + dw_mci_rtk_phase_tuning(host, 0, 0);
>> +
>> + return 0;
>> +}
>> +
>> +#ifdef CONFIG_PM
>> +static int dw_mci_rtk_suspend(struct device *dev) {
>> + struct dw_mci *host = dev_get_drvdata(dev);
>> + int ret = 0;
>> +
>> + ret = dw_mci_cqe_runtime_suspend(dev);
>> + mcq_writel(host, AHB, 0);
>> +
>> + return ret;
>> +}
>> +
>> +static int dw_mci_rtk_resume(struct device *dev) {
>> + struct dw_mci *host = dev_get_drvdata(dev);
>> + int ret = 0;
>> +
>> + mcq_writel(host, AHB, mcq_readl(host, AHB) | SDMMC_AHB_BIG);
>> + ret = dw_mci_cqe_runtime_resume(dev);
>> +
>> + return ret;
>> +}
>> +#else
>> +static int dw_mci_rtk_suspend(struct device *dev) {
>> + dev_info(dev, "User should enable CONFIG_PM kernel config\n");
> NAK, come on. I asked to drop it. Did you just ignore the feedback? Yep...
Okay.
>> +
>> + return 0;
>> +}
>> +
>> +static int dw_mci_rtk_resume(struct device *dev) {
>> + dev_info(dev, "User should enable CONFIG_PM kernel config\n");
> NAK
We had removed it.
>> +
>> + return 0;
>> +}
>> +#endif /*CONFIG_PM*/
>> +static const struct dev_pm_ops rtk_dev_pm_ops = {
>> + SET_SYSTEM_SLEEP_PM_OPS(dw_mci_rtk_suspend,
>> + dw_mci_rtk_resume)
>> + SET_RUNTIME_PM_OPS(dw_mci_cqe_runtime_suspend,
>> + dw_mci_cqe_runtime_resume,
>> + NULL)
>> +};
>> +
>> +static void dw_mci_rtk_shutdown(struct platform_device *pdev) {
>> + dev_info(&pdev->dev, "[eMMC] Shutdown\n");
> NAK
We had removed it.
>> + dw_mci_cqe_runtime_resume(&pdev->dev);
>> +}
>> +
>> +static unsigned long dw_mci_rtk_dwmmc_caps[1] = {
>> + MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA |
>> + MMC_CAP_SD_HIGHSPEED | MMC_CAP_MMC_HIGHSPEED |
>> + MMC_CAP_NONREMOVABLE | MMC_CAP_CMD23, };
>> +
>> +static const struct dw_mci_drv_data rtk_drv_data = {
>> + .caps = dw_mci_rtk_dwmmc_caps,
>> + .num_caps = ARRAY_SIZE(dw_mci_rtk_dwmmc_caps),
>> + .set_ios = dw_mci_rtk_set_ios,
>> + .execute_tuning = dw_mci_rtk_execute_tuning,
>> + .parse_dt = dw_mci_rtk_parse_dt,
>> + .init = dw_mci_rtk_init,
>> + .prepare_hs400_tuning = dw_mci_rtk_prepare_hs400_tuning,
>> + .hs400_complete = dw_mci_rtk_hs400_complete,
>> + .init_card = dw_mci_rtk_init_card,
>> +};
>> +
>> +static const struct of_device_id dw_mci_rtk_match[] = {
>> + { .compatible = "realtek,rtd1325-dw-cqe-emmc",
>> + .data = &rtk_drv_data },
>> + {},
>> +};
>> +MODULE_DEVICE_TABLE(of, dw_mci_rtk_match);
>> +
>> +int dw_mci_cqe_pltfm_register(struct platform_device *pdev,
>> + const struct dw_mci_drv_data *drv_data) {
>> + struct dw_mci *host;
>> + struct resource *regs;
>> +
>> + host = devm_kzalloc(&pdev->dev, sizeof(struct dw_mci),
>> + GFP_KERNEL);
> sizeof(*)
We had corrected it.
>> + if (!host)
>> + return -ENOMEM;
>> +
>> + host->irq = platform_get_irq(pdev, 0);
>> + if (host->irq < 0)
>> + return host->irq;
>> +
>> + host->drv_data = drv_data;
>> + host->pdev = pdev;
>> + host->dev = &pdev->dev;
>> + host->irq_flags = 0;
>> + host->pdata = pdev->dev.platform_data;
>> +
>> + regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + host->regs = devm_ioremap_resource(&pdev->dev, regs);
> Use helper for this.
> Open existing, recent drivers and use the them as template or some set of patterns. Sorry, but upstreaming your vendor code will be painful, because you started from some old, buggier version.
We had replaced platform_get_resource , devm_ioremap_resource by devm_platform_ioremap_resource.
>> + if (IS_ERR(host->regs))
>> + return PTR_ERR(host->regs);
>> +
>> + /* Get registers' physical base address */
>> + host->phy_regs = regs->start;
>> +
>> + platform_set_drvdata(pdev, host);
>> +
>> + return dw_mci_cqe_probe(host);
>> +}
>> +
>> +static int dw_mci_rtk_probe(struct platform_device *pdev) {
>> + const struct dw_mci_drv_data *drv_data;
>> + const struct of_device_id *match;
>> +
>> + if (!pdev->dev.of_node)
>> + return -ENODEV;
>> +
>> + match = of_match_node(dw_mci_rtk_match, pdev->dev.of_node);
>> + drv_data = match->data;
>> +
>> + return dw_mci_cqe_pltfm_register(pdev, drv_data); }
>> +
>> +int dw_mci_rtk_remove(struct platform_device *pdev) {
>> + struct dw_mci *host = platform_get_drvdata(pdev);
>> +
>> + dw_mci_cqe_remove(host);
>> + return 0;
Best regards,
Jyan
-----Original Message-----
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Sent: Monday, October 30, 2023 3:44 PM
To: Jyan Chou [周芷安] <jyanchou@...ltek.com>; ulf.hansson@...aro.org; adrian.hunter@...el.com; jh80.chung@...sung.com; riteshh@...eaurora.org; robh+dt@...nel.org; krzysztof.kozlowski+dt@...aro.org; conor+dt@...nel.org; asutoshd@...eaurora.org; p.zabel@...gutronix.de
Cc: linux-mmc@...r.kernel.org; devicetree@...r.kernel.org; linux-kernel@...r.kernel.org; arnd@...db.de; briannorris@...omium.org; doug@...morgal.com; tonyhuang.sunplus@...il.com; abel.vesa@...aro.org; william.qiu@...rfivetech.com
Subject: Re: [PATCH V4][3/4] mmc: Add dw mobile mmc cmdq rtk driver
External mail.
On 30/10/2023 07:27, Jyan Chou wrote:
> Add Realtek mmc driver to make good use Synopsys DesignWare mmc cmdq
> host driver.
>
> Signed-off-by: Jyan Chou <jyanchou@...ltek.com>
>
> ---
> v3 -> v4:
> - Modify dma setting's code to fix linux coding style.
And coding style of all other parts were ignored. You must fix it everywhere in your code.
...
> +
> +static void dw_mci_rtk_init_card(struct mmc_host *host, struct
> +mmc_card *card) {
> + /* In Realtek Platform, we need to attach eMMC card onto mmc host
> + * during eMMC initialization because of the following reason:
> + * When system cannot run the hs400, we need to down speed to hs200
> + * and call mmc_hw_reset and modify the mmc card attribute through mmc host.
> + * At this moment, system will show errors if host->card = NULL.
> + */
> + host->card = card;
> +}
> +
> +static int dw_mci_rtk_parse_dt(struct dw_mci *host) {
> + struct dw_mci_rtkemmc_host *priv;
> + const u32 *prop;
> + int size;
> +
> + priv = devm_kzalloc(host->dev, sizeof(struct
> + dw_mci_rtkemmc_host), GFP_KERNEL);
sizeof(*)
> + if (!priv)
> + return -ENOMEM;
> +
> + priv->pinctrl = devm_pinctrl_get(host->dev);
> + if (IS_ERR(priv->pinctrl))
> + dev_dbg(host->dev, "no pinctrl\n");
> +
> + priv->pins_default = pinctrl_lookup_state(priv->pinctrl,
> + PINCTRL_STATE_DEFAULT);
> + if (IS_ERR(priv->pins_default))
> + dev_warn(host->dev, "could not get default state\n");
> +
So this is required by the driver but not by the bindings.
> + priv->pins_sdr50 = pinctrl_lookup_state(priv->pinctrl,
> + "sdr50");
> + if (IS_ERR(priv->pins_sdr50))
> + dev_warn(host->dev, "could not get sdr50 state\n");
> +
> + priv->pins_hs200 = pinctrl_lookup_state(priv->pinctrl,
> + "hs200");
> + if (IS_ERR(priv->pins_hs200))
> + dev_warn(host->dev, "could not get hs200 state\n");
> +
> + priv->pins_hs400 = pinctrl_lookup_state(priv->pinctrl,
> + "hs400");
> + if (IS_ERR(priv->pins_hs400))
> + dev_warn(host->dev, "could not get hs400 state\n");
> +
> + priv->pins_tune0 = pinctrl_lookup_state(priv->pinctrl,
> + "tune0");
> + if (IS_ERR(priv->pins_tune0))
> + dev_warn(host->dev, "could not get tune0 state\n");
> +
> + priv->pins_tune1 = pinctrl_lookup_state(priv->pinctrl,
> + "tune1");
> + if (IS_ERR(priv->pins_tune1))
> + dev_warn(host->dev, "could not get tune1 state\n");
> +
> + priv->pins_tune2 = pinctrl_lookup_state(priv->pinctrl,
> + "tune2");
> + if (IS_ERR(priv->pins_tune2))
> + dev_warn(host->dev, "could not get tune2 state\n");
> +
> + priv->pins_tune3 = pinctrl_lookup_state(priv->pinctrl,
> + "tune3");
> + if (IS_ERR(priv->pins_tune3))
> + dev_warn(host->dev, "could not get tune3 state\n");
> +
> + priv->pins_tune4 = pinctrl_lookup_state(priv->pinctrl,
> + "tune4");
> +
> + if (IS_ERR(priv->pins_tune4))
> + dev_warn(host->dev, "could not get tune4 state\n");
> +
> + priv->vp0 = devm_clk_get(host->dev, "vp0");
> + if (IS_ERR(priv->vp0))
> + dev_err(host->dev, "could not get vp0 clk\n");
> +
> + priv->vp1 = devm_clk_get(host->dev, "vp1");
> + if (IS_ERR(priv->vp1))
> + dev_err(host->dev, "could not get vp1 clk\n");
dev_err_probe. Everywhere where applicable.
> +
> + priv->emmc_mode = 0;
> + prop = of_get_property(host->dev->of_node, "speed-step", &size);
> + if (prop) {
> + priv->emmc_mode = of_read_number(prop, 1);
> + dev_info(host->dev, "emmc mode : %d\n",
> + priv->emmc_mode);
Drop
> + } else {
> + dev_info(host->dev, "use default emmc sdr50 mode !\n");
Drop, why is this a problem?
> + }
> +
> + priv->is_cqe = 0;
> + prop = of_get_property(host->dev->of_node, "cqe", &size);
> + if (prop) {
> + priv->is_cqe = of_read_number(prop, 1);
> + dev_info(host->dev, "cmdq mode : %d\n", priv->is_cqe);
Drop
> + } else {
> + dev_info(host->dev, "use default eMMC legacy mode !\n");
Drop
> + }
> +
> + prop = of_get_property(host->dev->of_node, "rdq-ctrl", &size);
> + if (prop) {
> + priv->rdq_ctrl = of_read_number(prop, 1);
> + dev_info(host->dev, "get rdq-ctrl : %u\n",
> + priv->rdq_ctrl);
Drop
> + } else {
> + priv->rdq_ctrl = 0;
> + dev_info(host->dev, "no dqs_dly_tape switch node, use
> + default 0x0 !!\n");
Drop
> + }
> +
> + priv->m2tmx = syscon_regmap_lookup_by_phandle(host->dev->of_node, "realtek,m2tmx");
> + if (IS_ERR_OR_NULL(priv->m2tmx))
> + dev_err(host->dev, "can not get m2mtx node.\n");
> +
> + host->priv = priv;
> +
> + return 0;
> +}
> +
> +static int dw_mci_rtk_init(struct dw_mci *host) {
> + struct dw_mci_rtkemmc_host *priv = host->priv;
> +
> + host->pdata->caps2 = MMC_CAP2_NO_SDIO | MMC_CAP2_NO_SD;
> +
> + if (priv->emmc_mode >= 2)
> + host->pdata->caps2 |= MMC_CAP2_HS200_1_8V_SDR;
> + if (priv->emmc_mode >= 3) {
> + host->pdata->caps |= MMC_CAP_1_8V_DDR;
> + host->pdata->caps2 |= MMC_CAP2_HS400_1_8V;
> + }
> +
> + if (priv->is_cqe > 0)
> + host->pdata->caps2 |= (MMC_CAP2_CQE |
> + MMC_CAP2_CQE_DCMD);
> +
> + host->irq_flags = IRQF_SHARED;
> +
> + mcq_writel(host, CP, 0x0);
> +
> + /*Enable L4 gated*/
Read Linux coding style. Multiple times if needed.
> + mcq_writel(host, OTHER1, mcq_readl(host, OTHER1) &
> + ~(SDMMC_L4_GATED_DIS | SDMMC_L4_GATED_DIS1));
> +
> + mcq_writel(host, OTHER1, mcq_readl(host, OTHER1) &
> + (~(SDMMC_DQS_CTRL_GATE_DIS |
> + SDMMC_DBUS_MAS_GATING_DIS)));
> +
> + /*Set the eMMC wrapper little Endian*/
> + mcq_writel(host, AHB, mcq_readl(host, AHB) | SDMMC_AHB_BIG);
> +
> + mcq_writel(host, OTHER1,
> + mcq_readl(host, OTHER1) |
> + SDMMC_STARK_CARD_STOP_ENABLE);
> +
> + /*set eMMC instead of nand*/
> + regmap_update_bits_base(priv->m2tmx, SDMMC_NAND_DMA_SEL,
> + SDMMC_SRAM_DMA_SEL, SDMMC_SRAM_DMA_SEL,
> + NULL, false, true);
> +
> + /*Set the clk initial phase*/
> + dw_mci_rtk_phase_tuning(host, 0, 0);
> +
> + return 0;
> +}
> +
> +#ifdef CONFIG_PM
> +static int dw_mci_rtk_suspend(struct device *dev) {
> + struct dw_mci *host = dev_get_drvdata(dev);
> + int ret = 0;
> +
> + ret = dw_mci_cqe_runtime_suspend(dev);
> + mcq_writel(host, AHB, 0);
> +
> + return ret;
> +}
> +
> +static int dw_mci_rtk_resume(struct device *dev) {
> + struct dw_mci *host = dev_get_drvdata(dev);
> + int ret = 0;
> +
> + mcq_writel(host, AHB, mcq_readl(host, AHB) | SDMMC_AHB_BIG);
> + ret = dw_mci_cqe_runtime_resume(dev);
> +
> + return ret;
> +}
> +#else
> +static int dw_mci_rtk_suspend(struct device *dev) {
> + dev_info(dev, "User should enable CONFIG_PM kernel config\n");
NAK, come on. I asked to drop it. Did you just ignore the feedback? Yep...
> +
> + return 0;
> +}
> +
> +static int dw_mci_rtk_resume(struct device *dev) {
> + dev_info(dev, "User should enable CONFIG_PM kernel config\n");
NAK
> +
> + return 0;
> +}
> +#endif /*CONFIG_PM*/
> +static const struct dev_pm_ops rtk_dev_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(dw_mci_rtk_suspend,
> + dw_mci_rtk_resume)
> + SET_RUNTIME_PM_OPS(dw_mci_cqe_runtime_suspend,
> + dw_mci_cqe_runtime_resume,
> + NULL)
> +};
> +
> +static void dw_mci_rtk_shutdown(struct platform_device *pdev) {
> + dev_info(&pdev->dev, "[eMMC] Shutdown\n");
NAK
> + dw_mci_cqe_runtime_resume(&pdev->dev);
> +}
> +
> +static unsigned long dw_mci_rtk_dwmmc_caps[1] = {
> + MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA |
> + MMC_CAP_SD_HIGHSPEED | MMC_CAP_MMC_HIGHSPEED |
> + MMC_CAP_NONREMOVABLE | MMC_CAP_CMD23, };
> +
> +static const struct dw_mci_drv_data rtk_drv_data = {
> + .caps = dw_mci_rtk_dwmmc_caps,
> + .num_caps = ARRAY_SIZE(dw_mci_rtk_dwmmc_caps),
> + .set_ios = dw_mci_rtk_set_ios,
> + .execute_tuning = dw_mci_rtk_execute_tuning,
> + .parse_dt = dw_mci_rtk_parse_dt,
> + .init = dw_mci_rtk_init,
> + .prepare_hs400_tuning = dw_mci_rtk_prepare_hs400_tuning,
> + .hs400_complete = dw_mci_rtk_hs400_complete,
> + .init_card = dw_mci_rtk_init_card,
> +};
> +
> +static const struct of_device_id dw_mci_rtk_match[] = {
> + { .compatible = "realtek,rtd1325-dw-cqe-emmc",
> + .data = &rtk_drv_data },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, dw_mci_rtk_match);
> +
> +int dw_mci_cqe_pltfm_register(struct platform_device *pdev,
> + const struct dw_mci_drv_data *drv_data) {
> + struct dw_mci *host;
> + struct resource *regs;
> +
> + host = devm_kzalloc(&pdev->dev, sizeof(struct dw_mci),
> + GFP_KERNEL);
sizeof(*)
> + if (!host)
> + return -ENOMEM;
> +
> + host->irq = platform_get_irq(pdev, 0);
> + if (host->irq < 0)
> + return host->irq;
> +
> + host->drv_data = drv_data;
> + host->pdev = pdev;
> + host->dev = &pdev->dev;
> + host->irq_flags = 0;
> + host->pdata = pdev->dev.platform_data;
> +
> + regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + host->regs = devm_ioremap_resource(&pdev->dev, regs);
Use helper for this.
Open existing, recent drivers and use the them as template or some set of patterns. Sorry, but upstreaming your vendor code will be painful, because you started from some old, buggier version.
> + if (IS_ERR(host->regs))
> + return PTR_ERR(host->regs);
> +
> + /* Get registers' physical base address */
> + host->phy_regs = regs->start;
> +
> + platform_set_drvdata(pdev, host);
> +
> + return dw_mci_cqe_probe(host);
> +}
> +
> +static int dw_mci_rtk_probe(struct platform_device *pdev) {
> + const struct dw_mci_drv_data *drv_data;
> + const struct of_device_id *match;
> +
> + if (!pdev->dev.of_node)
> + return -ENODEV;
> +
> + match = of_match_node(dw_mci_rtk_match, pdev->dev.of_node);
> + drv_data = match->data;
> +
> + return dw_mci_cqe_pltfm_register(pdev, drv_data); }
> +
> +int dw_mci_rtk_remove(struct platform_device *pdev) {
> + struct dw_mci *host = platform_get_drvdata(pdev);
> +
> + dw_mci_cqe_remove(host);
> + return 0;
Best regards,
Krzysztof
Powered by blists - more mailing lists