All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Alexander Stein <alexander.stein@systec-electronic.com>
To: Cory Tusar <cory.tusar@pid1solutions.com>
Cc: robh+dt@kernel.org, pawel.moll@arm.com, mark.rutland@arm.com,
	ijc+devicetree@hellion.org.uk, galak@codeaurora.org,
	shawnguo@kernel.org, kernel@pengutronix.de, han.xu@freescale.com,
	dwmw2@infradead.org, computersforpeace@gmail.com,
	stefan@agner.ch, linux@arm.linux.org.uk,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-mtd@lists.infradead.org, andrew@lunn.ch
Subject: Re: [PATCH v1 3/7] mtd: fsl-quadspi: Support both 24- and 32-bit addressed commands.
Date: Tue, 14 Jul 2015 16:11:19 +0200	[thread overview]
Message-ID: <1991808.GxeN5ENdEz@ws-stein> (raw)
In-Reply-To: <1436386881-28088-4-git-send-email-cory.tusar@pid1solutions.com>

Hello Cory,

On Wednesday 08 July 2015 16:21:17, Cory Tusar wrote:
> The current fsl-quadspi implementation assumes that all connected
> devices are of the same size and type.  This commit adds lookup table
> entries for both 24- and 32-bit addressed variants of the read, sector
> erase, and page program operations as a precursor to later changes which
> generalize the flash layout parsing logic and allow for non-contiguous
> and non-homogeneous chip combinations.
> 
> Signed-off-by: Cory Tusar <cory.tusar@pid1solutions.com>
> 
> [...]
>  /* Get the SEQID for the command */
> -static int fsl_qspi_get_seqid(struct fsl_qspi *q, u8 cmd)
> +static int fsl_qspi_get_seqid(struct fsl_qspi *q, u8 cmd, u8 addr_width)
>  {
>  	switch (cmd) {
>  	case SPINOR_OP_READ_1_1_4:
> -		return SEQID_QUAD_READ;
> +		if (addr_width == 3)
> +			return SEQID_QUAD_READ_24;
> +		if (addr_width == 4)
> +			return SEQID_QUAD_READ_32;
> +		dev_err(q->dev, "Unsupported addr_width (%d) for cmd 0x%.2x\n",
> +			addr_width, cmd);
> +		break;

Does this actually work? What if q->nor[0].read_opcode is SPINOR_OP_READ4_1_1_4 (set in spi-nor.c for flashes using 4 byte address commands)?

>  	case SPINOR_OP_WREN:
>  		return SEQID_WREN;
>  	case SPINOR_OP_WRDI:
> @@ -403,11 +394,23 @@ static int fsl_qspi_get_seqid(struct fsl_qspi *q, u8 cmd)
>  	case SPINOR_OP_RDSR:
>  		return SEQID_RDSR;
>  	case SPINOR_OP_SE:
> -		return SEQID_SE;
> +		if (addr_width == 3)
> +			return SEQID_SE_24;
> +		if (addr_width == 4)
> +			return SEQID_SE_32;
> +		dev_err(q->dev, "Unsupported addr_width (%d) for cmd 0x%.2x\n",
> +			addr_width, cmd);
> +		break;

Same as for read command.

>  	case SPINOR_OP_CHIP_ERASE:
>  		return SEQID_CHIP_ERASE;
>  	case SPINOR_OP_PP:
> -		return SEQID_PP;
> +		if (addr_width == 3)
> +			return SEQID_PP_24;
> +		if (addr_width == 4)
> +			return SEQID_PP_32;
> +		dev_err(q->dev, "Unsupported addr_width (%d) for cmd 0x%.2x\n",
> +			addr_width, cmd);
> +		break;

See above too.

>  	case SPINOR_OP_RDID:
>  		return SEQID_RDID;
>  	case SPINOR_OP_WRSR:
> @@ -456,7 +459,7 @@ fsl_qspi_runcmd(struct fsl_qspi *q, u8 cmd, unsigned int addr, int len)
>  	} while (1);
>  
>  	/* trigger the LUT now */
> -	seqid = fsl_qspi_get_seqid(q, cmd);
> +	seqid = fsl_qspi_get_seqid(q, cmd, q->nor[0].addr_width);
>  	writel((seqid << QUADSPI_IPCR_SEQID_SHIFT) | len, base + QUADSPI_IPCR);
>  
>  	/* Wait for the interrupt. */
> @@ -601,9 +604,10 @@ static void fsl_qspi_init_abh_read(struct fsl_qspi *q)
>  	writel(0, base + QUADSPI_BUF2IND);
>  
>  	/* Set the default lut sequence for AHB Read. */
> -	seqid = fsl_qspi_get_seqid(q, q->nor[0].read_opcode);
> +	seqid = fsl_qspi_get_seqid(q, q->nor[0].read_opcode,
> +			q->nor[0].addr_width);
>  	writel(seqid << QUADSPI_BFGENCR_SEQID_SHIFT,

Those changes can then be skipped you explicitly do switch/case for 4 byte address commands.

Best reagrds,
Alexander
-- 
Dipl.-Inf. Alexander Stein

SYS TEC electronic GmbH
Am Windrad 2
08468 Heinsdorfergrund
Tel.: 03765 38600-1156
Fax: 03765 38600-4100
Email: alexander.stein@systec-electronic.com
Website: www.systec-electronic.com
 
Managing Director: Dipl.-Phys. Siegmar Schmidt
Commercial registry: Amtsgericht Chemnitz, HRB 28082


WARNING: multiple messages have this Message-ID (diff)
From: alexander.stein@systec-electronic.com (Alexander Stein)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v1 3/7] mtd: fsl-quadspi: Support both 24- and 32-bit addressed commands.
Date: Tue, 14 Jul 2015 16:11:19 +0200	[thread overview]
Message-ID: <1991808.GxeN5ENdEz@ws-stein> (raw)
In-Reply-To: <1436386881-28088-4-git-send-email-cory.tusar@pid1solutions.com>

Hello Cory,

On Wednesday 08 July 2015 16:21:17, Cory Tusar wrote:
> The current fsl-quadspi implementation assumes that all connected
> devices are of the same size and type.  This commit adds lookup table
> entries for both 24- and 32-bit addressed variants of the read, sector
> erase, and page program operations as a precursor to later changes which
> generalize the flash layout parsing logic and allow for non-contiguous
> and non-homogeneous chip combinations.
> 
> Signed-off-by: Cory Tusar <cory.tusar@pid1solutions.com>
> 
> [...]
>  /* Get the SEQID for the command */
> -static int fsl_qspi_get_seqid(struct fsl_qspi *q, u8 cmd)
> +static int fsl_qspi_get_seqid(struct fsl_qspi *q, u8 cmd, u8 addr_width)
>  {
>  	switch (cmd) {
>  	case SPINOR_OP_READ_1_1_4:
> -		return SEQID_QUAD_READ;
> +		if (addr_width == 3)
> +			return SEQID_QUAD_READ_24;
> +		if (addr_width == 4)
> +			return SEQID_QUAD_READ_32;
> +		dev_err(q->dev, "Unsupported addr_width (%d) for cmd 0x%.2x\n",
> +			addr_width, cmd);
> +		break;

Does this actually work? What if q->nor[0].read_opcode is SPINOR_OP_READ4_1_1_4 (set in spi-nor.c for flashes using 4 byte address commands)?

>  	case SPINOR_OP_WREN:
>  		return SEQID_WREN;
>  	case SPINOR_OP_WRDI:
> @@ -403,11 +394,23 @@ static int fsl_qspi_get_seqid(struct fsl_qspi *q, u8 cmd)
>  	case SPINOR_OP_RDSR:
>  		return SEQID_RDSR;
>  	case SPINOR_OP_SE:
> -		return SEQID_SE;
> +		if (addr_width == 3)
> +			return SEQID_SE_24;
> +		if (addr_width == 4)
> +			return SEQID_SE_32;
> +		dev_err(q->dev, "Unsupported addr_width (%d) for cmd 0x%.2x\n",
> +			addr_width, cmd);
> +		break;

Same as for read command.

>  	case SPINOR_OP_CHIP_ERASE:
>  		return SEQID_CHIP_ERASE;
>  	case SPINOR_OP_PP:
> -		return SEQID_PP;
> +		if (addr_width == 3)
> +			return SEQID_PP_24;
> +		if (addr_width == 4)
> +			return SEQID_PP_32;
> +		dev_err(q->dev, "Unsupported addr_width (%d) for cmd 0x%.2x\n",
> +			addr_width, cmd);
> +		break;

See above too.

>  	case SPINOR_OP_RDID:
>  		return SEQID_RDID;
>  	case SPINOR_OP_WRSR:
> @@ -456,7 +459,7 @@ fsl_qspi_runcmd(struct fsl_qspi *q, u8 cmd, unsigned int addr, int len)
>  	} while (1);
>  
>  	/* trigger the LUT now */
> -	seqid = fsl_qspi_get_seqid(q, cmd);
> +	seqid = fsl_qspi_get_seqid(q, cmd, q->nor[0].addr_width);
>  	writel((seqid << QUADSPI_IPCR_SEQID_SHIFT) | len, base + QUADSPI_IPCR);
>  
>  	/* Wait for the interrupt. */
> @@ -601,9 +604,10 @@ static void fsl_qspi_init_abh_read(struct fsl_qspi *q)
>  	writel(0, base + QUADSPI_BUF2IND);
>  
>  	/* Set the default lut sequence for AHB Read. */
> -	seqid = fsl_qspi_get_seqid(q, q->nor[0].read_opcode);
> +	seqid = fsl_qspi_get_seqid(q, q->nor[0].read_opcode,
> +			q->nor[0].addr_width);
>  	writel(seqid << QUADSPI_BFGENCR_SEQID_SHIFT,

Those changes can then be skipped you explicitly do switch/case for 4 byte address commands.

Best reagrds,
Alexander
-- 
Dipl.-Inf. Alexander Stein

SYS TEC electronic GmbH
Am Windrad 2
08468 Heinsdorfergrund
Tel.: 03765 38600-1156
Fax: 03765 38600-4100
Email: alexander.stein@systec-electronic.com
Website: www.systec-electronic.com
 
Managing Director: Dipl.-Phys. Siegmar Schmidt
Commercial registry: Amtsgericht Chemnitz, HRB 28082

  reply	other threads:[~2015-07-14 14:11 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-08 20:21 [PATCH v1 0/7] fsl-quadspi: Allow additional device combinations Cory Tusar
2015-07-08 20:21 ` Cory Tusar
2015-07-08 20:21 ` [PATCH v1 1/7] ARM: dts: vf610: Add missing QuadSPI register mapping and names Cory Tusar
2015-07-08 20:21   ` Cory Tusar
2015-07-08 20:21   ` Cory Tusar
2015-07-08 20:21 ` [PATCH v1 2/7] ARM: dts: vfxxx: Include support for qspi1 functionality Cory Tusar
2015-07-08 20:21   ` Cory Tusar
2015-07-08 20:21 ` [PATCH v1 3/7] mtd: fsl-quadspi: Support both 24- and 32-bit addressed commands Cory Tusar
2015-07-08 20:21   ` Cory Tusar
2015-07-14 14:11   ` Alexander Stein [this message]
2015-07-14 14:11     ` Alexander Stein
2015-11-20 19:24   ` Brian Norris
2015-11-20 19:24     ` Brian Norris
2015-11-20 19:24     ` Brian Norris
2015-11-20 19:38     ` Cory Tusar
2015-11-20 19:38       ` Cory Tusar
2015-07-08 20:21 ` [PATCH v1 4/7] mtd: fsl-quadspi: Use per-device clk_rate Cory Tusar
2015-07-08 20:21   ` Cory Tusar
2015-07-08 20:21   ` Cory Tusar
2015-07-08 20:21 ` [PATCH v1 5/7] mtd: fsl-quadspi: Allow non-contiguous flash layouts Cory Tusar
2015-07-08 20:21   ` Cory Tusar
2015-07-08 20:21 ` [PATCH v1 6/7] mtd: spi-nor: Add support for Micron MT25QL02GC serial flash Cory Tusar
2015-07-08 20:21   ` Cory Tusar
2015-07-08 20:21   ` Cory Tusar
2015-07-08 20:21 ` [PATCH v1 7/7] ARM: dts: vf610-twr: Enable QSPI and map flash devices Cory Tusar
2015-07-08 20:21   ` Cory Tusar
2015-07-14  3:33 ` [PATCH v1 0/7] fsl-quadspi: Allow additional device combinations Shawn Guo
2015-07-14  3:33   ` Shawn Guo
2015-07-14  3:33   ` Shawn Guo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1991808.GxeN5ENdEz@ws-stein \
    --to=alexander.stein@systec-electronic.com \
    --cc=andrew@lunn.ch \
    --cc=computersforpeace@gmail.com \
    --cc=cory.tusar@pid1solutions.com \
    --cc=devicetree@vger.kernel.org \
    --cc=dwmw2@infradead.org \
    --cc=galak@codeaurora.org \
    --cc=han.xu@freescale.com \
    --cc=ijc+devicetree@hellion.org.uk \
    --cc=kernel@pengutronix.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=linux@arm.linux.org.uk \
    --cc=mark.rutland@arm.com \
    --cc=pawel.moll@arm.com \
    --cc=robh+dt@kernel.org \
    --cc=shawnguo@kernel.org \
    --cc=stefan@agner.ch \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.