| 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: <YdzEJijn2JbSFzxF@antec>
Date: Tue, 11 Jan 2022 08:41:26 +0900
From: Stafford Horne <shorne@...il.com>
To: "Gabriel L. Somlo" <gsomlo@...il.com>
Cc: linux-kernel@...r.kernel.org, robh+dt@...nel.org,
devicetree@...r.kernel.org, ulf.hansson@...aro.org,
linux-mmc@...r.kernel.org, kgugala@...micro.com,
mholenko@...micro.com, krakoczy@...micro.com,
mdudek@...ernships.antmicro.com, paulus@...abs.org, joel@....id.au,
geert@...ux-m68k.org, david.abdurachmanov@...ive.com,
florent@...oy-digital.fr, rdunlap@...radead.org,
andy.shevchenko@...il.com, hdanton@...a.com
Subject: Re: [PATCH v11 3/3] mmc: Add driver for LiteX's LiteSDCard interface
On Mon, Jan 10, 2022 at 05:39:36PM -0500, Gabriel L. Somlo wrote:
> On Tue, Jan 11, 2022 at 07:15:42AM +0900, Stafford Horne wrote:
> > On Sun, Jan 09, 2022 at 06:20:03PM -0500, Gabriel Somlo wrote:
> > > LiteX (https://github.com/enjoy-digital/litex) is a SoC framework
> > > that targets FPGAs. LiteSDCard is a small footprint, configurable
> > > SDCard core commonly used in LiteX designs.
> > >
> > > The driver was first written in May 2020 and has been maintained
> > > cooperatively by the LiteX community. Thanks to all contributors!
> > >
> > > Co-developed-by: Kamil Rakoczy <krakoczy@...micro.com>
> > > Signed-off-by: Kamil Rakoczy <krakoczy@...micro.com>
> > > Co-developed-by: Maciej Dudek <mdudek@...ernships.antmicro.com>
> > > Signed-off-by: Maciej Dudek <mdudek@...ernships.antmicro.com>
> > > Co-developed-by: Paul Mackerras <paulus@...abs.org>
> > > Signed-off-by: Paul Mackerras <paulus@...abs.org>
> > > Signed-off-by: Gabriel Somlo <gsomlo@...il.com>
> > > Reviewed-by: Joel Stanley <joel@....id.au>
> > > Reviewed-by: Andy Shevchenko <andy.shevchenko@...il.com>
> > > ---
> > >
> > ...
> > > +static int litex_mmc_probe(struct platform_device *pdev)
> > > +{
> > > + struct device *dev = &pdev->dev;
> > > + struct litex_mmc_host *host;
> > > + struct mmc_host *mmc;
> > > + struct clk *clk;
> > > + int ret;
> > > +
> > > + /*
> > > + * NOTE: defaults to max_[req,seg]_size=PAGE_SIZE, max_blk_size=512,
> > > + * and max_blk_count accordingly set to 8;
> > > + * If for some reason we need to modify max_blk_count, we must also
> > > + * re-calculate `max_[req,seg]_size = max_blk_size * max_blk_count;`
> > > + */
> > > + mmc = mmc_alloc_host(sizeof(struct litex_mmc_host), dev);
> > > + if (!mmc)
> > > + return -ENOMEM;
> > > +
> > > + ret = devm_add_action_or_reset(dev, litex_mmc_free_host_wrapper, mmc);
> > > + if (ret)
> > > + return dev_err_probe(dev, ret,
> > > + "Can't register mmc_free_host action\n");
> > > +
> > > + host = mmc_priv(mmc);
> > > + host->mmc = mmc;
> > > +
> > > + /* Initialize clock source */
> > > + clk = devm_clk_get(dev, NULL);
> > > + if (IS_ERR(clk))
> > > + return dev_err_probe(dev, PTR_ERR(clk), "can't get clock\n");
> > > + host->ref_clk = clk_get_rate(clk);
> > > + host->sd_clk = 0;
> > > +
> > > + /*
> > > + * LiteSDCard only supports 4-bit bus width; therefore, we MUST inject
> > > + * a SET_BUS_WIDTH (acmd6) before the very first data transfer, earlier
> > > + * than when the mmc subsystem would normally get around to it!
> > > + */
> > > + host->is_bus_width_set = false;
> > > + host->app_cmd = false;
> > > +
> > > + /* LiteSDCard can support 64-bit DMA addressing */
> > > + ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
> > > + if (ret)
> > > + return ret;
> > > +
> > > + host->buf_size = mmc->max_req_size * 2;
> > > + host->buffer = dmam_alloc_coherent(dev, host->buf_size,
> > > + &host->dma, GFP_KERNEL);
> > > + if (host->buffer == NULL)
> > > + return -ENOMEM;
> > > +
> > > + host->sdphy = devm_platform_ioremap_resource_byname(pdev, "phy");
> > > + if (IS_ERR(host->sdphy))
> > > + return PTR_ERR(host->sdphy);
> > > +
> > > + host->sdcore = devm_platform_ioremap_resource_byname(pdev, "core");
> > > + if (IS_ERR(host->sdcore))
> > > + return PTR_ERR(host->sdcore);
> > > +
> > > + host->sdreader = devm_platform_ioremap_resource_byname(pdev, "reader");
> > > + if (IS_ERR(host->sdreader))
> > > + return PTR_ERR(host->sdreader);
> > > +
> > > + host->sdwriter = devm_platform_ioremap_resource_byname(pdev, "writer");
> > > + if (IS_ERR(host->sdwriter))
> > > + return PTR_ERR(host->sdwriter);
> > > +
> > > + /* Ensure DMA bus masters are disabled */
> > > + litex_write8(host->sdreader + LITEX_BLK2MEM_ENA, 0);
> > > + litex_write8(host->sdwriter + LITEX_MEM2BLK_ENA, 0);
> > > +
> > > + init_completion(&host->cmd_done);
> > > + ret = litex_mmc_irq_init(pdev, host);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + /* Allow full generic 2.7-3.6V range; no software tuning available */
> > > + mmc->ocr_avail = LITEX_MMC_OCR;
> > > +
> > > + mmc->ops = &litex_mmc_ops;
> > > +
> > > + /*
> > > + * Set default sd_clk frequency range based on empirical observations
> > > + * of LiteSDCard gateware behavior on typical SDCard media
> > > + */
> > > + mmc->f_min = 12.5e6;
> > > + mmc->f_max = 50e6;
> > > +
> > > + ret = mmc_of_parse(mmc);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + /* Force 4-bit bus_width (only width supported by hardware) */
> > > + mmc->caps &= ~MMC_CAP_8_BIT_DATA;
> > > + mmc->caps |= MMC_CAP_4_BIT_DATA;
> > > +
> > > + /* Set default capabilities */
> > > + mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY |
> > > + MMC_CAP_DRIVER_TYPE_D |
> > > + MMC_CAP_CMD23;
> > > + mmc->caps2 |= MMC_CAP2_NO_WRITE_PROTECT |
> > > + MMC_CAP2_NO_SDIO |
> > > + MMC_CAP2_NO_MMC;
> > > +
> > > + platform_set_drvdata(pdev, host);
> >
> > One more thing here. Or somewhere, should we add:
> >
> > dev_info(dev, "Litex MMC controller initialized");
> >
> > I was having a hard time debugging probing of this and having no printk's made
> > it a bit difficult.
> >
> > Though I was able to get most of the debug statements I needed using:
> >
> > "debug initcall_debug dyndbg=\"file drivers/* +p\" loglevel=8"
> >
> > -Stafford
> >
> > > + return mmc_add_host(mmc);
>
> I'd prefer to declare victory *after* calling mmc_add_host(), so if
> there are no objections I'd prefer to do the following in v12:
>
> ...
> platform_set_drvdata(pdev, host);
>
> - return mmc_add_host(mmc);
> + ret = mmc_add_host(mmc);
> + if (ret)
> + return dev_err_probe(dev, ret, "LiteX MMC probe failed!\n");
> +
> + dev_info(dev, "LiteX MMC controller initialized.\n");
> +
> + return 0;
> }
Yes, that is what I was thinking too.
-Stafford
Powered by blists - more mailing lists