linux-kernel - Re: [PATCHv2] drivers: spi: Add qspi flash controller

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]

Date:	Tue, 2 Jul 2013 13:16:08 +0300
From:	Felipe Balbi <balbi@...com>
To:	Sourav Poddar <sourav.poddar@...com>
CC:	<balbi@...com>, <broonie@...nel.org>,
	<spi-devel-general@...ts.sourceforge.net>,
	<grant.likely@...aro.org>, <rnayak@...com>,
	<linux-omap@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCHv2] drivers: spi: Add qspi flash controller

Hi,

On Tue, Jul 02, 2013 at 03:30:42PM +0530, Sourav Poddar wrote:
> >>+static int dra7xxx_qspi_setup(struct spi_device *spi)
> >>+{
> >>+	struct dra7xxx_qspi	*qspi =
> >>+			spi_master_get_devdata(spi->master);
> >>+
> >>+	int clk_div;
> >>+
> >>+	if (!qspi->spi_max_frequency)
> >>+		clk_div = 0;
> >won't this generate division by zero ?
> >
> Yes, Probably only an error should be thrown here. ?
> since min clk_div should be kept at 1.

right, if spi_max_frequency isn't passed, this is a broken DT binding.
Bail out.

> >>+	pm_runtime_get_sync(qspi->dev);
> >>+
> >>+	/* disable SCLK */
> >>+	dra7xxx_writel(qspi, dra7xxx_readl(qspi, QSPI_SPI_CLOCK_CNTRL_REG)
> >>+			&  ~QSPI_CLK_EN, QSPI_SPI_CLOCK_CNTRL_REG);
> >>+
> >>+	if (clk_div<  0) {

btw, add a space between clk_div and <

> >>+			dra7xxx_writel(qspi, *txbuf++, QSPI_SPI_DATA_REG);
> >>+			dra7xxx_writel(qspi, qspi->dc, QSPI_SPI_DC_REG);
> >>+			dra7xxx_writel(qspi, qspi->cmd | QSPI_WR_SNGL,
> >>+					QSPI_SPI_CMD_REG);
> >>+			status = dra7xxx_readl(qspi, QSPI_SPI_STATUS_REG);
> >>+			timeout = QSPI_TIMEOUT;
> >>+			while ((status&  QSPI_WC_BUSY) != QSPI_XFER_DONE) {
> >do you really need to poll ? No IRQ available ?
> >
> There is an interrupt available, I will try using that.

look at how i2c-omap.c synchronizes interrupt with the transfer_msg
code. It just uses a wait_for_completion().

> >>+static int dra7xxx_qspi_start_transfer_one(struct spi_master *master,
> >>+		struct spi_message *m)
> >>+{
> >>+	struct dra7xxx_qspi *qspi = spi_master_get_devdata(master);
> >>+	struct spi_device *spi = m->spi;
> >>+	struct spi_transfer *t;
> >>+	int status = 0;
> >>+	int flags = 0;
> >>+
> >>+	/* setup command reg */
> >>+	qspi->cmd = 0;
> >>+	qspi->cmd |= QSPI_WLEN(8);
> >>+	qspi->cmd |= QSPI_EN_CS(0);
> >>+	qspi->cmd |= 0xfff;
> Since, we dont know the number of frame lenght that need to be
> transferred and it comes from the spi framework, we keep the frame
> lenght to maximum.
> Then depending on the count value above in while loop, we terminate
> our trasnfer.

what ? seriously didn't get what you meant.

-- 
balbi

Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)