| 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
| ||
|
Date: Mon, 30 Oct 2023 08:39:24 +0100
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
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][2/4] mmc: Add Synopsys DesignWare mmc cmdq host driver
On 30/10/2023 07:27, Jyan Chou wrote:
> We implemented cmdq feature on Synopsys DesignWare mmc driver.
> The difference between dw_mmc.c and dw_mmc_cqe.c were distinct
> register definitions, mmc user flow and the addition of cmdq.
>
> New version of User Guide had modify mmc driver's usage flow,
> we may need to renew code to precisely follow user guide.
>
> More over, We added a wait status function to satisfy synopsys
> user guide's description, since this flow might be specific in
> synopsys host driver only.
>
> Signed-off-by: Jyan Chou <jyanchou@...ltek.com>
>
> —--
> v3 -> v4:
> - Modify dma mode selection and dma addressing bit to statisfy
> linux coding style.
>
I asked to fix several coding style issues so it will look a bit as
matching Linux coding style. I don't see improvements.
Please read carefully, more than once, the Linux coding style. Then
document in changelog what you fixed. If you document nothing, means you
ignored the feedback.
Fix every warning from checkpatch --strict. Then document in changelog
what you fixed. If you document nothing, means you ignored the feedback.
> +
> + if (!host->bus_hz) {
> + dev_err(host->dev,
> + "Platform data must supply bus speed\n");
> + ret = -ENODEV;
> + goto err_clk_ciu;
> + }
> +
> + if (!IS_ERR(host->pdata->rstc)) {
> + reset_control_assert(host->pdata->rstc);
> + usleep_range(10, 50);
> + reset_control_deassert(host->pdata->rstc);
> + }
> +
> + timer_setup(&host->timer, dw_mci_cqe_cto_timer, 0);
> +
> + spin_lock_init(&host->lock);
> + spin_lock_init(&host->irq_lock);
> + init_rwsem(&host->cr_rw_sem);
> + tasklet_init(&host->tasklet, dw_mci_cqe_tasklet_func, (unsigned long)host);
> +
> + /*pio mode's parameters should be initialized here*/
Nothing improved.
> +
> + /*Initialize the eMMC IP related attribute*/
> + dw_mci_cqe_setup(host);
> +
> + dw_mci_cqe_init_dma(host);
> +
> + /* This flag will be set 1 when doing tuning,
Nothing improved.
> + * we add this flag because
> + * some vendors might use other cmd instead of 21
> + * to tune phase under high speed interface.
> + * we use this flag to recognize if the system is under tuning stage.
> + */
> + host->tuning = 0;
> +
> + /*Timing_setting is to avoid sending command
Nothing improved.
> + *before setting phase in hs200, hs400
> + */
> + host->current_speed = 0;
> +
> + /*Do the rest of init for specific*/
> + if (drv_data && drv_data->init) {
> + ret = drv_data->init(host);
> + if (ret) {
> + dev_err(host->dev,
> + "implementation specific init failed\n");
> + goto err_dmaunmap;
> + }
> + }
> +
> + ret = dw_mci_cqe_init_slot(host);
> + if (ret) {
> + dev_err(host->dev, "slot 0 init failed\n");
> + goto err_dmaunmap;
> + }
> +
> + ret = devm_request_irq(host->dev, host->irq, dw_mci_cqe_interrupt,
> + host->irq_flags, "dw-mci-cqe", host);
> + if (ret)
> + goto err_dmaunmap;
> +
> + /*After the slot initialization,
Nothing improved.
> + *now we have mmc data and can initialize cmdq if user enabled
> + */
> + dw_mci_cqhci_init(host);
> +
> + return 0;
> +
> +err_dmaunmap:
> + if (!IS_ERR(host->pdata->rstc))
> + reset_control_assert(host->pdata->rstc);
> +err_clk_ciu:
> + clk_disable_unprepare(host->ciu_clk);
> +
> +err_clk_biu:
> + clk_disable_unprepare(host->biu_clk);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(dw_mci_cqe_probe);
> +
> +void dw_mci_cqe_remove(struct dw_mci *host)
> +{
> + dev_dbg(host->dev, "remove slot\n");
Nothing improved.
> + if (host->slot)
> + dw_mci_cqe_cleanup_slot(host->slot);
> +
> + if (!IS_ERR(host->pdata->rstc))
> + reset_control_assert(host->pdata->rstc);
> +
> + clk_disable_unprepare(host->ciu_clk);
> + clk_disable_unprepare(host->biu_clk);
> +}
> +EXPORT_SYMBOL(dw_mci_cqe_remove);
> +
> +#ifdef CONFIG_PM
> +int dw_mci_cqe_runtime_suspend(struct device *dev)
> +{
> + struct dw_mci *host = dev_get_drvdata(dev);
> + int ret = 0;
> +
> + if (host->pdata && (host->pdata->caps2 & MMC_CAP2_CQE)) {
> + if (host->slot) {
> + dev_info(host->dev, "cqe suspend\n");
Nothing improved.
> + ret = cqhci_suspend(host->slot->mmc);
> + if (ret) {
> + dev_err(host->dev, "cqe suspend failed\n");
Nothing improved.
> + return ret;
> + }
> + }
> + }
> +
> + clk_disable_unprepare(host->ciu_clk);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(dw_mci_cqe_runtime_suspend);
> +
> +int dw_mci_cqe_runtime_resume(struct device *dev)
> +{
> + struct dw_mci *host = dev_get_drvdata(dev);
> + const struct dw_mci_drv_data *drv_data = host->drv_data;
> + int ret = 0;
> +
> + clk_prepare_enable(host->ciu_clk);
> +
> + dw_mci_cqe_setup(host);
> + if (drv_data && drv_data->init) {
> + ret = drv_data->init(host);
> + if (ret)
> + dev_err(host->dev, "implementation specific init failed\n");
> + }
> +
> + init_completion(host->int_waiting);
> +
> + if (host->pdata && (host->pdata->caps2 & MMC_CAP2_CQE)) {
> + if (host->slot) {
> + dev_info(host->dev, "cqe resume\n");
> + ret = cqhci_resume(host->slot->mmc);
> + if (ret)
> + dev_err(host->dev, "cqe resume failed\n");
Nothing improved.
> + }
> + }
> +
> + dw_mci_cqe_setup_bus(host->slot, true);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(dw_mci_cqe_runtime_resume);
> +#endif /* CONFIG_PM */
> +
> +static int __init dw_mci_cqe_init(void)
> +{
> + pr_info("Synopsys Designware Multimedia Card Interface Driver\n");
Nothing improved.
> + return 0;
> +}
> +
> +static void __exit dw_mci_cqe_exit(void)
> +{
> +}
> +
> +module_init(dw_mci_cqe_init);
> +module_exit(dw_mci_cqe_exit);
This part of code is just useless.
Best regards,
Krzysztof
Powered by blists - more mailing lists