Re: [PATCH v11 3/3] MIPS: dts: jz4780/ci20: Add NEMC, BCH and NAND device tree nodes
On Mon, Jan 04, 2016 at 12:34:44PM +, Harvey Hunt wrote:
> diff --git a/arch/mips/boot/dts/ingenic/ci20.dts
> b/arch/mips/boot/dts/ingenic/ci20.dts
> index 9fcb9e7..782258c 100644
> --- a/arch/mips/boot/dts/ingenic/ci20.dts
> +++ b/arch/mips/boot/dts/ingenic/ci20.dts
As I noted on patch 1, you need to send this to linux-mips + Ralf.
> @@ -42,3 +42,66 @@
> &uart4 {
> status = "okay";
> };
> +
> +&nemc {
> + status = "okay";
> +
> + nandc: nand-controller@1 {
> + compatible = "ingenic,jz4780-nand";
> + reg = <1 0 0x100>;
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + ingenic,bch-controller = <&bch>;
> +
> + ingenic,nemc-tAS = <10>;
> + ingenic,nemc-tAH = <5>;
> + ingenic,nemc-tBP = <10>;
> + ingenic,nemc-tAW = <15>;
> + ingenic,nemc-tSTRV = <100>;
> +
> + nand@1 {
> + reg = <1>;
> +
> + nand-ecc-step-size = <1024>;
> + nand-ecc-strength = <24>;
> + nand-ecc-mode = "hw";
> + nand-on-flash-bbt;
> +
> + partitions {
> + #address-cells = <2>;
> + #size-cells = <2>;
This binding was updated, so you need:
compatible = "fixed-partitions";
Brian
> +
> + partition@0 {
> + label = "u-boot-spl";
> + reg = <0x0 0x0 0x0 0x80>;
> + };
> +
> + partition@0x80 {
> + label = "u-boot";
> + reg = <0x0 0x80 0x0 0x20>;
> + };
> +
> + partition@0xa0 {
> + label = "u-boot-env";
> + reg = <0x0 0xa0 0x0 0x20>;
> + };
> +
> + partition@0xc0 {
> + label = "boot";
> + reg = <0x0 0xc0 0x0 0x400>;
> + };
> +
> + partition@0x8c0 {
> + label = "system";
> + reg = <0x0 0x4c0 0x1 0xfb40>;
> + };
> + };
> + };
> + };
> +};
> +
> +&bch {
> + status = "okay";
> +};
Brian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v11 1/3] dt-bindings: binding for jz4780-{nand,bch}
On Mon, Jan 04, 2016 at 12:34:42PM +, Harvey Hunt wrote:
> From: Alex Smith
>
> Add DT bindings for NAND devices connected to the NEMC on JZ4780 SoCs,
> as well as the hardware BCH controller, used by the jz4780_{nand,bch}
> drivers.
>
> Signed-off-by: Alex Smith
> Cc: Zubair Lutfullah Kakakhel
> Cc: David Woodhouse
> Cc: Brian Norris
> Cc: linux-...@lists.infradead.org
> Cc: devicetree@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> Cc: r...@kernel.org
> Signed-off-by: Harvey Hunt
> Acked-by: Rob Herring
> Reviewed-by: Boris Brezillon
> ---
> v10 -> v11:
> - Added Boris Brezillon's Reviewed-By.
Applied just patch 1 for now. I had some small comments on patch 2. And
you need to send patch 3 to linux-mips and the MIPS maintainer.
Brian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5] mtd: support BB SRAM on ICP DAS LP-8x4x
On Sun, Dec 20, 2015 at 01:43:58PM +0300, Sergei Ianovich wrote: > On Sat, 2015-12-19 at 21:38 -0600, Rob Herring wrote: > > On Tue, Dec 15, 2015 at 09:58:53PM +0300, Sergei Ianovich wrote: > > > +Required properties: > > > +- compatible : should be "icpdas,sram-lp8x4x" > > > > No wildcards please. Otherwise looks fine. > > There is a similar review comment from Arnd Bergmann in the discussion > of `[PATCH v5] serial: support for 16550A serial ports on LP-8x4x`. > > I'll quote my latest clarification: > > ... This driver will support ports on LP-8081, ^^ So 8081 doesn't even match the wildcard scheme you give in the compatible string, proving the point of the Conventional Wisdom suggestion Rob gave... > > LP-8141, LP-8441, LP-8841. Last time I checked the vendor was announcing > > a series with 3 as the last digit. They use lp8x4x name, eg. in > > documentation like `LP-8x4x_ChangeLog.txt`. They ship their proprietary > > SDK in `lp8x4x_sdk_for_linux.tar`. All of this implies that it is a > > single board. > > I think the solution should be the same for all LP-8x4x drivers (IRQ, > SRAM, SERIAL, IIO). The rationale is described here: http://devicetree.org/Device_Tree_Usage#Understanding_the_compatible_Property Quote: > Warning: Don't use wildcard compatible values, like "fsl,mpc83xx-uart" > or similar. Silicon vendors will invariably make a change that breaks > your wildcard assumptions the moment it is too late to change it. > Instead, choose a specific silicon implementations and make all > subsequent silicon compatible with it. I don't think your circumstance is anything unique. Regards, Brian -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND PATCH] mtd: mtk-nor: adjust sequence of trigger function and assignment function
On Fri, Dec 18, 2015 at 11:02:40AM +0800, Bayi Cheng wrote: > Move write data register before excute command to avoid > missing first byte write to nor flash > > Signed-off-by: Bayi Cheng > --- > the previous patch didn't drop the Change-Id Applied to l2-mtd.git -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/5] mtd: nand: Fix support for NAND DMA prefetch
On Thu, Oct 15, 2015 at 12:37:23PM -0500, Franklin S Cooper Jr wrote: > NAND DMA prefetch has been broken for awhile and seems to have only > worked for SDMA based devices > > This patchset fixes DMA prefetch to work on both EDMA and SDMA devices > > Test on: > am335x gp evm > am437x gp evm > am37x gp evm > > This patchset depends on Roger Quadros recent v4 GPMC/NAND patchset > https://github.com/rogerq/linux.git > branch: for-v4.4/gpmc-v4 In what way does this depend on the other series? Can any of this be taken without it? If not, then perhaps it'd be good if Roger can roll this into his series? Just throwing out ideas. If you want to wait on Roger's series, that's fine. FWIW, the MTD stuff all looks OK to me: Acked-by: Brian Norris > Franklin S Cooper Jr (5): > mtd: nand: omap2: Support parsing dma channel information from DT > mtd: nand: omap2: Start dma request before enabling prefetch > mtd: nand: omap2: Fix high memory dma prefetch transfer > ARM: dts: am437x/am33xx/omap/dm816x: Add gpmc dma channel > ARM: OMAP2+: Update GPMC and NAND DT binding documentation > > .../bindings/memory-controllers/omap-gpmc.txt | 7 +- > .../devicetree/bindings/mtd/gpmc-nand.txt | 2 ++ > arch/arm/boot/dts/am33xx.dtsi | 2 ++ > arch/arm/boot/dts/am4372.dtsi | 2 ++ > arch/arm/boot/dts/dm816x.dtsi | 2 ++ > arch/arm/boot/dts/omap3.dtsi | 2 ++ > arch/arm/boot/dts/omap4.dtsi | 2 ++ > arch/arm/boot/dts/omap5.dtsi | 2 ++ > drivers/mtd/nand/omap2.c | 27 > +- > 9 files changed, 31 insertions(+), 17 deletions(-) > > -- > 2.6.1 > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH linux-next 2/5] mtd: spi-nor: fix Quad SPI mode support for Spansion, Micron and Macronix
On Mon, Dec 07, 2015 at 03:09:11PM +0100, Cyrille Pitchen wrote: > This patch reworks the support of Quad and Dual SPI protocols for Micron, > Spansion and Macronix Quad/Dual capable memories. Indeed, in the best > case, only Spansion memories are correctly supported by the current > spi-nor framework. ^^ Ah, so this is what I was struggling with at first. I agree that Micron looks broken. Quite possibly Macronix too. Unfortunately, I haven't had great test hardware for some of the quad modes. Especially not anything that supports generic SPI, and not completely on mainline. > 1 - Micron: > When their Quad SPI mode is enabled, Micron spi-nor memories expect all > commands to use the SPI 4-4-4 protocol. Also when the Dual SPI mode is > enabled, all commands must use the SPI 2-2-2 protocol. > > Before this patch, the spi-nor framework used to always enable the Quad > mode when the mode argument of spi_nor_scan() took the value SPI_NOR_QUAD. > That was not suited with drivers only supporting SPI 1-x-4 protocols but > not the 4-4-4 (e.g. the m25p80 driver). Also the SPI controller was not > notified about which SPI protocol to use to transfert command. We cannot > rely only on the op code: in Extended SPI mode the 0x6b command must use > the SPI 1-1-4 protocol whereas in Quad SPi mode the SPI 4-4-4 protocol > must be use instead. > > After this patch, the spi-nor framework uses the result of the > spi_nor_read_id() function to choose the right SPI protocol to be used. > If the reg_proto was set to SPI_PROTO_4_4_4, we already know that the Quad > SPI mode is already enabled and that the SPI controller supports the SPI > 4-4-4 protocol (otherwise it would have fail to read the JEDEC ID with the > 0xaf op code). For the very same reason, if the reg_proto was set to > SPI_PROTO_2_2_2, we already know that the Dual mode is already enabled and > that the SPI controller supports the SPI 2-2-2 protocol. > Otherwise we switch back to the Extended SPI protocol, which supports at > least the Fast Read commands: > - 1-1-1 (0x0b) > - Dual Output 1-1-2 (0x3b) > - Quad Output 1-1-4 (0x6b) > > We also safely set the number of dummy cycles to 8 for Fast Read commands > through the Volatile Configuration Register (VCR): some drivers (m25p80) > or SPI controllers only support a number of dummy cycles multiple of 8. > This number may have previouly been set to an unsupported value by an > early bootloader or at reset thanks to the Non-Volatile Configuration > Register. It's not clear to me how you're being safe with the dummy cycles at all. It seems like you're introducing new values that may be incompatible with drivers. That can be OK, but you have to give drivers a chance to opt-out... Maybe some kind of "host capability" flags? > Finally the XIP bit is always set in the VCR to disable the Continuous > Read mode as we don't want to care about mode cycles. > > 2 - Macronix: > When the QPI mode is enabled, all commands must use the SPI 4-4-4 protocol > and only the 0xeb op code is supported for Fast Read commands. > Before this patch, the spi-nor framework used to force the QPI mode but > used the 0x6b op code for Fast Read commands when the SPI controller > claims to support Quad SPI mode. > This patch uses the result of spi_nor_read_id() to guess whether the QPI > mode is both enable and supported by the SPI controller (otherwise it > would have failed to read the JEDEC ID with the 0xaf op code). > When the QPI mode is disabled, Macronix memories still support the > following Fast Read commands: > - 1-1-1 (0x0b) > - Dual Output 1-1-2 (0x3b) > - Quad Output 1-1-4 (0x6b) > So if the QPI mode has not already been enabled, there is not need to > enable it. We also avoid the 0xbb (Dual I/O 1-2-2) and 0xeb (Quad I/O > 1-4-4) op codes on purpose as we don't want to care about the value to set > in mode cycles not to enter the Continuous Read (Performance Enhance) > mode. > > As for Micron memories, the spi-nor framework now safely sets the number > of dummy cycles to 8 thanks to 2 volatile bits inside the Configuration > Register. > > 3 - Spansion: > As for Macronix, we avoid the 0xbb (Dual I/O 1-2-2) and 0xeb (Quad I/O > 1-4-4) op codes on purpose as we don't want to care about the value to set > in mode cycles not to enter in the Continuous Read mode. > > Besides, we only care about the Quad Enable bit inside the Configuration > Register (CR) when using Quad operations. In such a case, we first check > its state before trying to set it. Now we also notify the user about the > update of this non-volatile bit. > > We also check the Latency Code (LC) in CR to know the exact number of > dummy cycles to use when performing a Fast Read operation. Currently only > the 0x0b, 0x3b and 0x6b op codes are used to perform Fast Read operation > so the number of dummy cycles is always either 0 or 8. Hence no regression > should be introduced. > > Signed-off-by: Cyrille Pitchen > --- > drivers/mtd/spi-nor/spi-nor.c | 783 >
Re: [PATCH linux-next 1/5] mtd: spi-nor: properly detect the memory when it boots in Quad or Dual mode
(Hit send too early; a few more comments)
On Mon, Dec 07, 2015 at 03:09:10PM +0100, Cyrille Pitchen wrote:
> drivers/mtd/spi-nor/spi-nor.c | 52
> +++
> include/linux/mtd/spi-nor.h | 23 +--
> 2 files changed, 69 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 3b2460efc019..bf17736750c1 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -73,6 +73,11 @@ struct flash_info {
>
> #define JEDEC_MFR(info) ((info)->id[0])
>
> +struct read_id_config {
> + enum read_mode mode;
> + enum spi_protocol proto;
> +};
> +
> static const struct flash_info *spi_nor_match_id(const char *name);
>
> /*
> @@ -867,11 +872,16 @@ static const struct flash_info spi_nor_ids[] = {
> { },
> };
>
> -static const struct flash_info *spi_nor_read_id(struct spi_nor *nor)
> +static const struct flash_info *spi_nor_read_id(struct spi_nor *nor,
> + enum read_mode mode)
> {
> - int tmp;
> + int i, tmp;
> u8 id[SPI_NOR_MAX_ID_LEN];
> const struct flash_info *info;
> + static const struct read_id_config configs[] = {
> + {SPI_NOR_QUAD, SPI_PROTO_4_4_4},
> + {SPI_NOR_DUAL, SPI_PROTO_2_2_2}
> + };
>
> tmp = nor->read_reg(nor, SPINOR_OP_RDID, id, SPI_NOR_MAX_ID_LEN);
> if (tmp < 0) {
> @@ -879,6 +889,34 @@ static const struct flash_info *spi_nor_read_id(struct
> spi_nor *nor)
> return ERR_PTR(tmp);
> }
>
> + /* Special case for Micron/Macronix qspi nor. */
> + if ((id[0] == 0xff && id[1] == 0xff && id[2] == 0xff) ||
> + (id[0] == 0x00 && id[1] == 0x00 && id[2] == 0x00))
Is this specified anywhere, or is this just a heuristic, to guess
whether we're getting valid IDs? Do we know anything about what opcodes
look like ...
> + for (i = 0; i < ARRAY_SIZE(configs); ++i) {
> + if (configs[i].mode != mode)
> + continue;
> +
> + /* Set this protocol for all commands. */
> + nor->reg_proto = configs[i].proto;
> + nor->read_proto = configs[i].proto;
> + nor->write_proto = configs[i].proto;
> + nor->erase_proto = configs[i].proto;
> +
> + /*
> + * Multiple I/O Read ID only returns the Manufacturer ID
> + * (1 byte) and the Device ID (2 bytes). So we reset the
> + * remaining bytes.
> + */
Ugh, does that mean we get different IDs returned via the different
modes? That sounds like a disaster. What about all the flash that we
need 5+ bytes to differentiate? Just be glad we haven't come across any
Micron or Macronix like that yet?
> + memset(id, 0, sizeof(id));
> + tmp = nor->read_reg(nor, SPINOR_OP_MIO_RDID, id, 3);
Hmm, so you're passing implicit configuration data via the
nor->reg_proto field. So, spi-nor drivers now have to read that field
during every read_reg() invocation? Seems like either this should be:
(a) a driver hook/callback, so we can just reconfigure a handful of
times or
(b) part of a parameter that gets passed as part of the function
signature
> + if (tmp < 0) {
> + dev_dbg(nor->dev,
> + "error %d reading JEDEC ID Multi I/O\n",
> + tmp);
> + return ERR_PTR(tmp);
> + }
> + }
> +
> for (tmp = 0; tmp < ARRAY_SIZE(spi_nor_ids) - 1; tmp++) {
> info = &spi_nor_ids[tmp];
> if (info->id_len) {
[...]
Brian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH linux-next 1/5] mtd: spi-nor: properly detect the memory when it boots in Quad or Dual mode
Hi Cyrille,
On Mon, Dec 07, 2015 at 03:09:10PM +0100, Cyrille Pitchen wrote:
> The quad (or dual) mode of a spi-nor memory may be enabled at boot time by
> non-volatile bits in some setting register. Also such a mode may have
> already been enabled at early stage by some boot loader.
>
> Hence, we should not guess the spi-nor memory is always configured for the
> regular SPI 1-1-1 protocol.
>
> Micron and Macronix memories, once their Quad (or dual for Micron) mode
> enabled, no longer process the regular JEDEC Read ID (0x9f) command but
> instead reply to a new command: JEDEC Read ID Multiple I/O (0xaf).
> Besides, in Quad mode both memory manufacturers expect ALL commands to
> use the SPI 4-4-4 protocol. For Micron memories, enabling their Dual mode
> implies to use the SPI 2-2-2 protocol for ALL commands.
>
> Signed-off-by: Cyrille Pitchen
> ---
> drivers/mtd/spi-nor/spi-nor.c | 52
> +++
> include/linux/mtd/spi-nor.h | 23 +--
> 2 files changed, 69 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 3b2460efc019..bf17736750c1 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -73,6 +73,11 @@ struct flash_info {
>
> #define JEDEC_MFR(info) ((info)->id[0])
>
> +struct read_id_config {
> + enum read_mode mode;
> + enum spi_protocol proto;
> +};
> +
> static const struct flash_info *spi_nor_match_id(const char *name);
>
> /*
> @@ -867,11 +872,16 @@ static const struct flash_info spi_nor_ids[] = {
> { },
> };
>
> -static const struct flash_info *spi_nor_read_id(struct spi_nor *nor)
> +static const struct flash_info *spi_nor_read_id(struct spi_nor *nor,
> + enum read_mode mode)
It's unclear what you're trying to do with the 'read_mode' enum now.
(Admittedly it may not be clear in the current code either, given the
confusion we already have over Micron support.)
Would you care to document it better?
> {
> - int tmp;
> + int i, tmp;
> u8 id[SPI_NOR_MAX_ID_LEN];
> const struct flash_info *info;
> + static const struct read_id_config configs[] = {
> + {SPI_NOR_QUAD, SPI_PROTO_4_4_4},
> + {SPI_NOR_DUAL, SPI_PROTO_2_2_2}
> + };
>
> tmp = nor->read_reg(nor, SPINOR_OP_RDID, id, SPI_NOR_MAX_ID_LEN);
> if (tmp < 0) {
> @@ -879,6 +889,34 @@ static const struct flash_info *spi_nor_read_id(struct
> spi_nor *nor)
> return ERR_PTR(tmp);
> }
>
> + /* Special case for Micron/Macronix qspi nor. */
> + if ((id[0] == 0xff && id[1] == 0xff && id[2] == 0xff) ||
> + (id[0] == 0x00 && id[1] == 0x00 && id[2] == 0x00))
> + for (i = 0; i < ARRAY_SIZE(configs); ++i) {
> + if (configs[i].mode != mode)
> + continue;
> +
> + /* Set this protocol for all commands. */
> + nor->reg_proto = configs[i].proto;
> + nor->read_proto = configs[i].proto;
> + nor->write_proto = configs[i].proto;
> + nor->erase_proto = configs[i].proto;
Are these all fully independent? Do we really need 4 fields for this?
> +
> + /*
> + * Multiple I/O Read ID only returns the Manufacturer ID
> + * (1 byte) and the Device ID (2 bytes). So we reset the
> + * remaining bytes.
> + */
> + memset(id, 0, sizeof(id));
> + tmp = nor->read_reg(nor, SPINOR_OP_MIO_RDID, id, 3);
> + if (tmp < 0) {
> + dev_dbg(nor->dev,
> + "error %d reading JEDEC ID Multi I/O\n",
> + tmp);
> + return ERR_PTR(tmp);
> + }
> + }
> +
> for (tmp = 0; tmp < ARRAY_SIZE(spi_nor_ids) - 1; tmp++) {
> info = &spi_nor_ids[tmp];
> if (info->id_len) {
> @@ -1178,11 +1216,17 @@ int spi_nor_scan(struct spi_nor *nor, const char
> *name, enum read_mode mode)
> if (ret)
> return ret;
>
> + /* Reset SPI protocol for all commands */
> + nor->erase_proto = SPI_PROTO_1_1_1;
> + nor->read_proto = SPI_PROTO_1_1_1;
> + nor->write_proto = SPI_PROTO_1_1_1;
> + nor->reg_proto = SPI_PROTO_1_1_1;
> +
> if (name)
> info = spi_nor_match_id(name);
> /* Try to auto-detect if chip name wasn't specified or not found */
> if (!info)
> - info = spi_nor_read_id(nor);
> + info = spi_nor_read_id(nor, mode);
> if (IS_ERR_OR_NULL(info))
> return -ENOENT;
>
> @@ -1193,7 +1237,7 @@ int spi_nor_
Re: [PATCH linux-next 0/5] mtd: spi-nor: add driver for Atmel QSPI controller
On Thu, Dec 17, 2015 at 04:29:01PM -0800, Brian Norris wrote:
> On Tue, Dec 08, 2015 at 06:21:00AM +, Bean Huo 霍斌斌 wrote:
> OK, so I think your patch is broken:
>
> > > commit 548cd3ab54da ("mtd: spi-nor: Add quad I/O support for Micron
> > > SPI NOR")
>
> How did you test this? Specifically, this can't possibly have worked
> with a regular drivers/spi/ controllers, since:
>
> (a) you're enabling 4/4/4 (i.e., "Quad mode") on the flash but
> (b) m25p80_read() only sets .rx_bits for the data; i.e., it's using
> 1/1/4 (i.e., "Extended mode")
>
> I'm tempted to essentially revert that, as it looks essentially
> untested. It would be nice to have a cleaner baseline before trying to
> extend it with Cyrille's work.
>
> Cyrille, what do you think? Is my analysis at all correct here? (Sorry
> if this is addressed elsewhere; there's a lot of text in this
> conversation, but I'm getting hung up very early.) And if so, does it
> hurt to just drop Micron "Quad mode" (4/4/4)?
It looks like you address some of this in patch 2, where you (as I do)
claim that Micron support is broken. It seems to me that it could be
better to kill it than to try to fix it. But maybe that's just the
frustrated maintainer in me speaking.
Brian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH linux-next 0/5] mtd: spi-nor: add driver for Atmel QSPI controller
Hi Bean,
On Tue, Dec 08, 2015 at 06:21:00AM +, Bean Huo 霍斌斌 wrote:
> > -Original Message-
> > From: Brian Norris [mailto:computersforpe...@gmail.com]
> >
> > I'll admit I'm a little fuzzy on the differences between dual and quad
> > modes on
> > various flash manufacturers. Can you help clear it up for me?
>
> For Micron SPI NOR spi quad mode, means that Qaud I/O prototocol, it follows
> I/O
> Bus width is command-address-Data 4-4-4, at this time, DQ0,DQ1,DQ2,DQ3
> are all used to transfer address/command/data. For this maybe not the same
> between
> different flash manufactures. For example, for Spansion Qspi NOR, its all
> instructions are
> transferred from host to memory as a single bit serial sequence on the DQ0
> signal, even under
> Quad mode. Dual mode the same as Qaud mode scenario.
>
> for SPI NOR 1-1-4, means command and address are transferred on the DQ0,
> but for data, being transferred on DQ0,DQ1,DQ2,DQ3.For this, it is the same
> between different flash manufacturers. Of course, at this moment, SPI NOR
> should work under extended I/O mode.
OK, so to make these statements *much* shorter:
* Micron "Quad Mode" means putting the flash in a 4/4/4 mode
* Spansion (and all others?) are using 1/1/4 modes
Correct?
> > I think some of the comments on patch 2 help too, but I'll just comment here
> > for now.
> > It looks like the current driver has problems regarding the non 1-x-y modes
> > (e.g., 4-4-4), right? But I see that spi-nor.c never tries to send a 4_4_4
> > command; it only sets read_opcode to SPINOR_OP_READ_1_1_{1,2,4}. So is
> > this an oversight in patches like Bean's patch?
>
> For SPINOR_OP_READ_1_1_{1,2,4} commands, Spi NOR actually works under
> Extended I/O mode, not Quad mode. They just push Spi NOR output data by Quad
> mode,
> Command and address still following extended I/O mode.
The naming is confusing enough here... so in your words, "extended"
means 1/1/{1,2,4} (i.e., command, and maybe address, use 1 line, but
data goes on 4)? And "quad" means 4/4/4?
> For 4-4-4 I/O protocol, SPI NOR should change to Quad mode(just as my patch),
> of course, SPI controller should support this. for Micron Qspi NOR, under
> quad mode,
> all commands/address/data are transferred on DQ0,DQ1,DQ2,DQ3 signals. No
> matter what
> kind of command.
OK, so I think your patch is broken:
> > commit 548cd3ab54da ("mtd: spi-nor: Add quad I/O support for Micron
> > SPI NOR")
How did you test this? Specifically, this can't possibly have worked
with a regular drivers/spi/ controllers, since:
(a) you're enabling 4/4/4 (i.e., "Quad mode") on the flash but
(b) m25p80_read() only sets .rx_bits for the data; i.e., it's using
1/1/4 (i.e., "Extended mode")
I'm tempted to essentially revert that, as it looks essentially
untested. It would be nice to have a cleaner baseline before trying to
extend it with Cyrille's work.
Cyrille, what do you think? Is my analysis at all correct here? (Sorry
if this is addressed elsewhere; there's a lot of text in this
conversation, but I'm getting hung up very early.) And if so, does it
hurt to just drop Micron "Quad mode" (4/4/4)?
(AIUI, this won't exactly be a panacea, since you mention bootloaders
that start us off in quad mode, so we can't use single I/O 0x9f READ
ID.)
> > Why would we even need to enable quad modes like that, if we're not going
> > to send the 4-4-4 opcodes?
> I think, in order to high speed SPI NOR, after enable quad mode,
> SPINOR_OP_READ_1_1_{1,2,4} commands don't need any more, normal read command
> (0x03)
> Can implement as them.
OK. That's odd, but I guess it doesn't matter much. It just makes it a
little less obvious what's going on.
> > My next question (if my understanding is roughly correct) is, do we need the
> > 4-4-4 modes, and what risks come with them? I understand we can shorten
> > the command and address phases, but does that alone yield much
> > performance benefit? And I think the risk is that a given system might not
> > be
> > prepared for the flash to be in a 4-4-4 mode, if the boot code tries to use
> > 1-x-y
> > commands.
>
> As far as my current experience and knowledge, this still need to be enabled,
> especially for
> fast boot, and some IOT devices to store info into SPI NOR.
Do you have any data to about the speed? And you haven't addressed the
risks. There are definitely risks. Cyrille looks like he's trying to
address the risks (e.g., use volatile modes whenever possible), but it
doesn't seem that you are.
> For this patches, my current concern is that host side how to get different
> I/O protocol changes,
> and distinguish between different flash manufacturers I/O mode.
Brian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 0/7] ARM: dts: Add compatible property to "partitions" node
+ real linux-mtd mailing list
On Mon, Dec 14, 2015 at 1:43 PM, Brian Norris
wrote:
> On Mon, Dec 14, 2015 at 10:20:19PM +0100, Geert Uytterhoeven wrote:
>> On Mon, Dec 14, 2015 at 8:46 PM, Brian Norris
>> wrote:
>> > On Mon, Dec 14, 2015 at 08:11:37PM +0100, Geert Uytterhoeven wrote:
>> >> As of commit e488ca9f8d4f62c2 ("doc: dt: mtd: partitions: add compatible
>> >> property to "partitions" node"), the "partitions" subnode of an SPI
>> >> FLASH device node must have a compatible property. The partitions are no
>> >> longer detected if it is not present.
>> >
>> > For all patches:
>> >
>> > Acked-by: Brian Norris
>> >
>> >> I marked this series as "RFC", as the abovementioned commit is not yet
>> >> upstream. Ideally, the users should be fixed first, to avoid breakage.
>>
>> It breaks bisectability in mainline.
>
> The files you are fixing are not converted in mainline yet, are they?
>
>> > I'd differ on the last sentence. It's more important to straighten out
>> > the release features for 4.4 than to make sure that linux-next has
>> > perfect DTS files. So I'll submit whenever ready :)
>>
>> If the DTS changes go in first, bisectability is preserved.
>
> Are you asking me to delay fixing the DT binding until 4.5, just because
> your for-next branches can't be fixed? IOW, you want me to release the
> bad DT binding for a whole cycle?
>
> Brian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 0/7] ARM: dts: Add compatible property to "partitions" node
On Mon, Dec 14, 2015 at 10:20:19PM +0100, Geert Uytterhoeven wrote:
> On Mon, Dec 14, 2015 at 8:46 PM, Brian Norris
> wrote:
> > On Mon, Dec 14, 2015 at 08:11:37PM +0100, Geert Uytterhoeven wrote:
> >> As of commit e488ca9f8d4f62c2 ("doc: dt: mtd: partitions: add compatible
> >> property to "partitions" node"), the "partitions" subnode of an SPI
> >> FLASH device node must have a compatible property. The partitions are no
> >> longer detected if it is not present.
> >
> > For all patches:
> >
> > Acked-by: Brian Norris
> >
> >> I marked this series as "RFC", as the abovementioned commit is not yet
> >> upstream. Ideally, the users should be fixed first, to avoid breakage.
>
> It breaks bisectability in mainline.
The files you are fixing are not converted in mainline yet, are they?
> > I'd differ on the last sentence. It's more important to straighten out
> > the release features for 4.4 than to make sure that linux-next has
> > perfect DTS files. So I'll submit whenever ready :)
>
> If the DTS changes go in first, bisectability is preserved.
Are you asking me to delay fixing the DT binding until 4.5, just because
your for-next branches can't be fixed? IOW, you want me to release the
bad DT binding for a whole cycle?
Brian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 0/7] ARM: dts: Add compatible property to "partitions" node
On Mon, Dec 14, 2015 at 08:11:37PM +0100, Geert Uytterhoeven wrote:
> Hi,
>
> As of commit e488ca9f8d4f62c2 ("doc: dt: mtd: partitions: add compatible
> property to "partitions" node"), the "partitions" subnode of an SPI
> FLASH device node must have a compatible property. The partitions are no
> longer detected if it is not present.
For all patches:
Acked-by: Brian Norris
> I marked this series as "RFC", as the abovementioned commit is not yet
> upstream. Ideally, the users should be fixed first, to avoid breakage.
I'd differ on the last sentence. It's more important to straighten out
the release features for 4.4 than to make sure that linux-next has
perfect DTS files. So I'll submit whenever ready :)
FWIW, I plan to submit my pullreq this week, hopefully by
end-of-Wednesday. (There are other potential MTD fixes that haven't
stabilized. I may just send them separately.)
Regards,
Brian
> Tested on r8a7791/koelsch.
>
> Thanks!
>
> Geert Uytterhoeven (7):
> ARM: dt: mvebu: ix4-300d: Add compatible property to "partitions" node
> ARM: shmobile: bockw dts: Add compatible property to "partitions" node
> ARM: shmobile: lager dts: Add compatible property to "partitions" node
> ARM: shmobile: koelsch dts: Add compatible property to "partitions"
> node
> ARM: shmobile: porter dts: Add compatible property to "partitions"
> node
> ARM: shmobile: gose dts: Add compatible property to "partitions" node
> ARM: shmobile: silk dts: Add compatible property to "partitions" node
>
> arch/arm/boot/dts/armada-xp-lenovo-ix4-300d.dts | 1 +
> arch/arm/boot/dts/r8a7778-bockw.dts | 1 +
> arch/arm/boot/dts/r8a7790-lager.dts | 1 +
> arch/arm/boot/dts/r8a7791-koelsch.dts | 1 +
> arch/arm/boot/dts/r8a7791-porter.dts| 1 +
> arch/arm/boot/dts/r8a7793-gose.dts | 1 +
> arch/arm/boot/dts/r8a7794-silk.dts | 1 +
> 7 files changed, 7 insertions(+)
>
> --
> 1.9.1
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 --
> ge...@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like
> that.
> -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH for-4.4 2/2] doc: dt: mtd: partitions: add compatible property to "partitions" node
On Mon, Dec 14, 2015 at 03:49:14PM +0100, Geert Uytterhoeven wrote:
> On Wed, Dec 9, 2015 at 2:12 AM, Brian Norris
> wrote:
> > On Mon, Dec 07, 2015 at 09:58:35AM -0800, Brian Norris wrote:
> >> On Sat, Dec 05, 2015 at 12:45:36PM +0100, Jonas Gorski wrote:
> >> > On Fri, Dec 4, 2015 at 3:02 AM, Brian Norris
> >> > wrote:
> >> > > // proposed
> >> > > partitions {
> >> > > compatible = "partitions";
> >> >
> >> > "partitions" sounds mode like a device_type thing than a compatible
> >> > name, maybe "fixed-partitions"? IMHO that would describe better what
> >> > these are, and doesn't invite to think using compatible =
> >> > "arm,arm-flash-structure", "partitions"; is a good idea.
> >>
> >> "fixed-partitions" sounds OK to me. If no objections, I'll apply these
> >> patches, with (approximately) a:
> >>
> >> s/"partitions"/"fixed-partitions"/
> >
> > Pushed to linux-mtd.git with the above change.
>
> Aarghl, hadn't seen this patch before.
>
> This breaks the users that have already added the partitions subnodes
> (armada-xp-lenovo-ix4-300d.dts and a few shmobile).
Sorry, I checked Linus' master and not linux-next :(
> Will send patches to fix it...
Thanks.
Brian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 0/7] mtd: partitions: add of_match_table support
On Fri, Dec 11, 2015 at 09:44:37AM +0100, Geert Uytterhoeven wrote: > On Thu, Dec 10, 2015 at 9:54 PM, Brian Norris > wrote: > > IOW, I wouldn't expect MBR or GPT to work well on large raw NAND flash, > > and so I don't plan to do that sort of work myself. If you can provide > > some better argument for it, and some nice maintainable code to go with > > it, then of course it could be considered :) > > There's also NOR FLASH (e.g. SPI-NOR), which is what most boards I'm > working on have. OK. But these flash are often used for the boot firmware, no? And then, does the boot code have to be provided at one end of the flash (e.g., bottom)? If so, then something like MBR or GPT will likely not apply, since they reserve the first (and sometimes last) blocks of the medium. Brian -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 0/7] mtd: partitions: add of_match_table support
On Sat, Dec 05, 2015 at 12:35:42PM +0100, Jonas Gorski wrote:
> On Sat, Dec 5, 2015 at 6:19 AM, Brian Norris
> wrote:
> > Hi,
> >
> > There have been several discussions [1] about adding a device tree binding
> > for
> > associating flash devices with the partition parser(s) that are used on the
> > flash. There are a few reasons:
> >
> > (1) drivers shouldn't have to be encoding platform knowledge by listing
> > what
> > parsers might be used on a given system (this is the currently all
> > that's
> > supported)
> > (2) we can't just scan for all supported parsers (like the block system
> > does), since
> > there is a wide diversity of "formats" (no standardization), and it is
> > not
> > always safe or efficient to attempt to do so, particularly since many
> > of
> > them allow their data structures to be placed anywhere on the flash,
> > and
> > so require scanning the entire flash device to find them.
> >
> > So instead, let's support a new binding so that a device tree can specify
> > what
> > partition formats might be used. This seems like a reasonable choice (even
> > though it's not strictly a hardware description) because the flash layout /
> > partitioning is often very closely tied with the bootloader/firmware, at
> > production time.
>
> On a first glance this looks good to me, and looks easily extensible
> for application of non-complete partition parsers.
>
> E.g. for the "brcm,bcm6345-imagetag" we would want to actually do something
> like
>
> partitions {
>
>
> partition@0 {
> reg = <0x0 0x1>;
> label = "cfe";
> read-only;
> };
>
> partition@1 {
> reg = <0x1 0x3d>;
> label = "firmware";
> compatible = "brcm,bcm6345-imagetag";
> };
>
> partition@3e {
> reg = <0x3e 0x1>;
> label = "art";
> read-only;
> };
>
>partition@3f {
> reg = <0x3f 0x1>;
> label = "nvram";
> read-only;
> };
> };
>
> as the image tag can only specify the offsets and sizes of the rootfs
> and kernel parts, but not of any other parts.
I had your (and others') prior attempts and suggestions in mind when
planning this, and I agree that the binding looks extendible to cases
like that. I haven't yet worked out what a good MTD infrastructure for
that would look like, so I stuck with defining and implementing only
what I know use :)
> > Also, as an example first-use of this mechanism, I support Google's FMAP
> > flash
> > structure, used on Chrome OS devices.
> >
> > Note that this is an RFC, mainly for the reason noted in patch 6 ("RFC: mtd:
> > partitions: enable of_match_table matching"): the of_match_table support
> > won't
> > yet autoload a partition parser that is built as a module. I'm not quite
> > sure
> > if there's a lot of value in supporting MTD parsers as modules (block
> > partition
> > support can't be), but that is supported for "by-name" parser lookups in MTD
> > already, so I don't feel like dropping that feature yet. Tips or thoughts
> > are
> > particularly welcome on this aspect!
>
> I would assume a lot of the cases these would be a chicken-egg
> problem, you need the parser to be able to find and mount the rootfs,
> but you you need mount the rootfs to load the parser.
Not necessarily. One of my current use cases has a boot SPI NOR flash +
an eMMC rootfs. Modules can be loaded from eMMC.
BTW, I'm realizing that if partition parsers are forced to built-in
only, then we'd have to do the same for the MTD core (or at least, the
MTD core that handles partitioning). Not sure if that's a desirable
trade-off. (Again, block support is 'bool' in Kconfig, if we're trying
to compare.)
Brian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 0/7] mtd: partitions: add of_match_table support
On Sat, Dec 05, 2015 at 11:15:54AM +0100, Geert Uytterhoeven wrote: > On Sat, Dec 5, 2015 at 6:19 AM, Brian Norris > wrote: > > There have been several discussions [1] about adding a device tree binding > > for > > associating flash devices with the partition parser(s) that are used on the > > flash. There are a few reasons: > > > > (1) drivers shouldn't have to be encoding platform knowledge by listing > > what > > parsers might be used on a given system (this is the currently all > > that's > > supported) > > (2) we can't just scan for all supported parsers (like the block system > > does), since > > there is a wide diversity of "formats" (no standardization), and it is > > not > > always safe or efficient to attempt to do so, particularly since many > > of > > them allow their data structures to be placed anywhere on the flash, > > and > > so require scanning the entire flash device to find them. > > I read the second reason, but would it be useful to (partially) merge > block/partitions/ and drivers/mtd/partitions/, so I can use e.g. msdos > partitions > on an mtd device?? I kinda agree with Michal: is there a good use case? Really, MTD partitioning is not a highly-scalable design. Particularly, it's not typically that well-suited to large (read: unreliable) NAND flash, where fixing partitions at the raw flash level mostly serves to restrict UBI's ability to wear-level across the device. For that sort of case, it's best if people are using UBI volumes on a (mostly?) unpartitioned MTD, instead of using MTD partitions as the main separation mechanism. Also, most partition designs (either MTD or block) aren't very robust against bitflips, read disturb, etc. IOW, I wouldn't expect MBR or GPT to work well on large raw NAND flash, and so I don't plan to do that sort of work myself. If you can provide some better argument for it, and some nice maintainable code to go with it, then of course it could be considered :) Regards, Brian -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 3/7] doc: dt: mtd: partition: add on-flash format binding
On Mon, Dec 07, 2015 at 12:36:28PM +1100, David Gibson wrote:
> On Sat, Dec 05, 2015 at 10:33:30PM +0100, Michal Suchanek wrote:
> > On 5 December 2015 at 12:39, Jonas Gorski wrote:
> > > On Sat, Dec 5, 2015 at 6:19 AM, Brian Norris
> > > wrote:
> >
> > >> +
> > >> +Examples:
> > >> +
> > >> +flash@0 {
> > >> + partitions {
> > >> + compatible = "google,fmap";
> > >> + };
> > >> +};
> > >
> > > I wonder if this wouldn't be better served in a separate binding doc
> > > with its compatible name as the filename, like we do with
> > > driver^Whardware blocks, especially if we want to add more parsers.
> >
> >
> > I find that *very* counter productive for bindings that go to the same
> > node. You have a description of a node, and then suddenly there you
> > have another file with another description of the same node. Totally
> > awesome.
>
> I can't actually work out from that if you're agreeing with the
> original post or the first reply.
Perhaps I'm biased, but I think he was agreeing with the first reply.
(Particularly, "I find that *very* counter productive" uses the word
"that" to refer to "separate binding doc[s]".)
> > Also how do you plan to write partitioning schemes with parameters
> > like with non-zero offset of the partition table.
If you are directing this question at me: I don't have a specific plan
for it. MTD parsers don't currently take external input for this; many
scan the whole device, but some might also have conventions built into
the parser itself too, so this just gets hooked based on "compatible".
But if the need arose, I would hope we could work out a common binding.
> Presumably with properties in the patitions node. Not seeing the
> problem here.
I believe Michal is bringing up the (important, IMO) point that if
distinct partition types are being described in the same node, then any
use of additional properties *must* be closely coordinated. We can't
have two parsers "foo" and "bar" defining conflicting uses of the same
property in the same node, like this:
partitions {
compatible = "foo", "bar";
property-baz = ...; // e.g., reg = <...>;
};
where if "foo" is not found, we fall back to "bar". But what if "foo"
and "bar" use "property-baz" differently?
Having everything in one doc would help ensure that the entire
"partitions" binding is considered as a whole when extending it, in my
(and an in my interpretation of Michal's) opinion.
Brian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH linux-next (v2) 1/3] mtd: brcmnand: Add brcm,bcm6368-nand device tree binding
On Wed, Dec 09, 2015 at 01:01:42PM -0800, Florian Fainelli wrote:
> Le 09/12/2015 12:40, Simon Arlott a écrit :
> > Add device tree binding for NAND on the BCM6368.
> >
> > The BCM6368 has a NAND interrupt register with combined status and enable
> > registers. It also requires a clock, so add an optional clock to the
> > common brcmnand binding.
> >
>
> Reviewed-by: Florian Fainelli
Applied this and patches 2 and 3 to l2-mtd.git, with one small fix,
below.
> > Signed-off-by: Simon Arlott
> > ---
> > Changed "nand-intr-base" reg name to "nand-int-base".
> >
> > .../devicetree/bindings/mtd/brcm,brcmnand.txt | 32
> > ++
> > 1 file changed, 32 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt
> > b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt
> > index 4ff7128..ebfa6fc 100644
> > --- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt
> > +++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt
> > @@ -45,6 +45,8 @@ Required properties:
> > - #size-cells : <0>
> >
> > Optional properties:
> > +- clock : reference to the clock for the NAND
> > controller
> > +- clock-names : "nand" (required for the above clock)
> > - brcm,nand-has-wp : Some versions of this IP include a
> > write-protect
> >(WP) control bit. It is always available on
> > >=
> >v7.0. Use this property to describe the rare
> > @@ -72,6 +74,12 @@ we define additional 'compatible' properties and
> > associated register resources w
> > and enable registers
> > - reg-names: (required) "nand-int-base"
> >
> > + * "brcm,nand-bcm6368"
> > + - compatible: should contain "brcm,nand-bcm", "brcm,nand-bcm6368"
> > + - reg: (required) the 'NAND_INTR_BASE' register range, with combined
> > status
> > + and enable registers, and boot address registers
> > + - reg-names: (required) "nand-int-base"
> > +
> > * "brcm,nand-iproc"
> > - reg: (required) the "IDM" register range, for interrupt enable and
> > APB
> > bus access endianness configuration, and the "EXT" register range,
> > @@ -148,3 +156,27 @@ nand@f0442800 {
> > };
> > };
> > };
> > +
> > +nand@1200 {
> > + compatible = "brcm,nand-bcm63168", "brcm,nand-bcm6368",
> > + "brcm,brcmnand-v4.0", "brcm,brcmnand";
> > + reg = <0x1200 0x180>,
> > + <0x1600 0x200>,
> > + <0x10b0 0x10>;
> > + reg-names = "nand", "nand-cache", "nand-intr-base";
s/intr/int/
> > + interrupt-parent = <&periph_intc>;
> > + interrupts = <50>;
> > + clocks = <&periph_clk 20>;
> > + clock-names = "nand";
> > +
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + nand0: nandcs@0 {
> > + compatible = "brcm,nandcs";
> > + reg = <0>;
> > + nand-on-flash-bbt;
> > + nand-ecc-strength = <1>;
> > + nand-ecc-step-size = <512>;
> > + };
> > +};
> >
>
>
> --
> Florian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] mtd: brcmnand: Add brcm,bcm6368-nand device tree binding
On Fri, Dec 04, 2015 at 09:29:55PM -, Simon Arlott wrote: > On Fri, December 4, 2015 16:04, Jonas Gorski wrote: > > On Thu, Dec 3, 2015 at 12:41 AM, Simon Arlott wrote: > >> + * "brcm,nand-bcm6368" > >> + - compatible: should contain "brcm,nand-bcm", > >> "brcm,nand-bcm6368" > >> + - reg: (required) the 'NAND_INTR_BASE' register range, with combined > >> status > >> + and enable registers, and boot address registers > >> + - reg-names: (required) "nand-intr-base" > > > > Can't we use the same name as bcm63138, i.e. nand-int-base? > > Brian, > > Before I change this, is there anything else in the patch series that needs to > be changed? No, I think you covered my comments in your latest series: http://lists.infradead.org/pipermail/linux-mtd/2015-December/064004.html I don't know about Jonas's comments about using bcm6368, even though bcm6368 is a much older NAND core. I had similar thoughts when Florian first proposed it, but I'm not sure I have a much better suggestion. We're trying to describe two slightly different tracks of IP: the core NAND controller, which has a defined revision (2.x, 4.0, etc.), and the accessory interrupt bits, which are mostly constant across a product line / class of SoCs and aren't really versioned. So I guess I'm OK with the usage of the bcm6368 compatible string. Regards, Brian -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH for-4.4 2/2] doc: dt: mtd: partitions: add compatible property to "partitions" node
On Mon, Dec 07, 2015 at 09:58:35AM -0800, Brian Norris wrote:
> On Sat, Dec 05, 2015 at 12:45:36PM +0100, Jonas Gorski wrote:
> > On Fri, Dec 4, 2015 at 3:02 AM, Brian Norris
> > wrote:
> > > // proposed
> > > partitions {
> > > compatible = "partitions";
> >
> > "partitions" sounds mode like a device_type thing than a compatible
> > name, maybe "fixed-partitions"? IMHO that would describe better what
> > these are, and doesn't invite to think using compatible =
> > "arm,arm-flash-structure", "partitions"; is a good idea.
>
> "fixed-partitions" sounds OK to me. If no objections, I'll apply these
> patches, with (approximately) a:
>
> s/"partitions"/"fixed-partitions"/
Pushed to linux-mtd.git with the above change.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH linux-next 0/5] mtd: spi-nor: add driver for Atmel QSPI controller
+ Bean Huo
Hi Cyrille,
On Mon, Dec 07, 2015 at 03:09:09PM +0100, Cyrille Pitchen wrote:
> Hi all,
>
> this series of patches adds support to the Atmel QSPI controller available
> on sama5d2 SoCs. It was tested on a sama5d2 xplained ultra board with a
> Micron n25q128a13 QSPI memory and a at25df321a SPI memory.
>
> In order to use the Micron memory in its Quad SPI mode, the spi-nor
> framework needed to be patched to fix the support of Quad/Dual SPI
> protocols with some memory manufacturers such as Spansion, Micron and
> Macronix. There are many comments in the source code to explain the
> implementation choices based on the datasheets from memory manufacturers.
>
>
> This series was based and tested on linux-next-20151207
>
> 1 - Atmel QSPI + Micron n25q128a13 (atmel-quadspi.c driver)
>
> SPI 1-1-1: This mode was tested replacing SPI_NOR_QUAD by SPI_NOR_FAST as
>argument to spi_nor_scan() called from atmel_qspi_probe().
>
> SPI 1-1-4: Bootloaders (at91bootstrap/uboot) don't enable the Quad SPI
>mode of the Micron memory. When probed from Linux, the memory
>uses its Extended SPI mode and replies to the regular Read ID
>(0x9f) command.
>
> SPI 4-4-4: The romcode enabled the Quad SPI mode the of Micron memory
>before loading the at91bootstrap. When probed from Linux, the
>memory uses its Quad SPI mode and no longer replies to the
>regular Read ID (0x9f) command but instead to the Read ID
>Multiple I/O (0xaf) command. The memory expects ALL commands
>to use the SPI 4-4-4 protocol.
I'll admit I'm a little fuzzy on the differences between dual and quad
modes on various flash manufacturers. Can you help clear it up for me?
I think some of the comments on patch 2 help too, but I'll just comment
here for now.
It looks like the current driver has problems regarding the non 1-x-y
modes (e.g., 4-4-4), right? But I see that spi-nor.c never tries to
send a 4_4_4 command; it only sets read_opcode to
SPINOR_OP_READ_1_1_{1,2,4}. So is this an oversight in patches like
Bean's patch?
commit 548cd3ab54da ("mtd: spi-nor: Add quad I/O support for Micron
SPI NOR")
Why would we even need to enable quad modes like that, if we're not
going to send the 4-4-4 opcodes?
My next question (if my understanding is roughly correct) is, do we need
the 4-4-4 modes, and what risks come with them? I understand we can
shorten the command and address phases, but does that alone yield much
performance benefit? And I think the risk is that a given system might
not be prepared for the flash to be in a 4-4-4 mode, if the boot code
tries to use 1-x-y commands.
Also, I see a lot of good comments in patch 2 about Spansion vs.
Macronix vs. Micron memories. I wonder if previous developers have
completely tested their patches, or if they're just reading the
datasheets... so, what kind have testing have you done? Do you have
samples of all these flash to test?
Regards,
Brian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 4/7] mtd: add of_match_mtd_parser() and of_mtd_match_mtd_parser() helpers
On Sun, Dec 06, 2015 at 08:45:40PM -0600, Rob Herring wrote: > On Fri, Dec 4, 2015 at 11:19 PM, Brian Norris > wrote: > > drivers/of/of_mtd.c| 33 + > > BTW, this file should be moved to drivers/mtd/ at some point. How about s/at some point/now/ ? I can send a separate patch. It also seems like these should just get linked into the 'mtd' module (when CONFIG_OF=y) instead of having a tiny module for just a few functions. Why did files like this get placed here anyway? Is there a reason that there are things like of_net and of_pci here too? Brian -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH for-4.4 2/2] doc: dt: mtd: partitions: add compatible property to "partitions" node
On Sat, Dec 05, 2015 at 12:45:36PM +0100, Jonas Gorski wrote:
> On Fri, Dec 4, 2015 at 3:02 AM, Brian Norris
> wrote:
> > // proposed
> > partitions {
> > compatible = "partitions";
>
> "partitions" sounds mode like a device_type thing than a compatible
> name, maybe "fixed-partitions"? IMHO that would describe better what
> these are, and doesn't invite to think using compatible =
> "arm,arm-flash-structure", "partitions"; is a good idea.
"fixed-partitions" sounds OK to me. If no objections, I'll apply these
patches, with (approximately) a:
s/"partitions"/"fixed-partitions"/
Thanks,
Brian
> > #address-cells = ;
> > #size-cells = ;
> > partition@0 {
> > ...;
> > };
> > };
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH 4/7] mtd: add of_match_mtd_parser() and of_mtd_match_mtd_parser() helpers
Like the corresponding OF-based device/driver matching infrascture,
let's begin to support a mtd/partition-parser matching infrastructure.
Signed-off-by: Brian Norris
---
drivers/of/of_mtd.c| 33 +
include/linux/mtd/partitions.h | 2 ++
include/linux/of_mtd.h | 13 +
3 files changed, 48 insertions(+)
diff --git a/drivers/of/of_mtd.c b/drivers/of/of_mtd.c
index b7361ed70537..169d7500af5d 100644
--- a/drivers/of/of_mtd.c
+++ b/drivers/of/of_mtd.c
@@ -9,6 +9,7 @@
#include
#include
#include
+#include
#include
/**
@@ -117,3 +118,35 @@ bool of_get_nand_on_flash_bbt(struct device_node *np)
return of_property_read_bool(np, "nand-on-flash-bbt");
}
EXPORT_SYMBOL_GPL(of_get_nand_on_flash_bbt);
+
+static const struct of_device_id *of_match_mtd_parser(
+ struct mtd_part_parser *parser, struct device_node *np)
+{
+ if (!parser || !np)
+ return NULL;
+
+ return of_match_node(parser->of_match_table, np);
+}
+
+static struct device_node *mtd_get_partitions_of_node(struct mtd_info *master)
+{
+ struct device_node *np = mtd_get_of_node(master);
+
+ if (!np)
+ return NULL;
+
+ return of_get_child_by_name(np, "partitions");
+}
+
+bool of_mtd_match_mtd_parser(struct mtd_info *mtd,
+struct mtd_part_parser *parser)
+{
+ struct device_node *np = mtd_get_partitions_of_node(mtd);
+ bool ret;
+
+ ret = of_match_mtd_parser(parser, np) != NULL;
+ of_node_put(np);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(of_mtd_match_mtd_parser);
diff --git a/include/linux/mtd/partitions.h b/include/linux/mtd/partitions.h
index 70736e1e6c8f..2e68ef561a40 100644
--- a/include/linux/mtd/partitions.h
+++ b/include/linux/mtd/partitions.h
@@ -51,6 +51,7 @@ struct mtd_partition {
struct mtd_info;
struct device_node;
+struct of_device_id;
/**
* struct mtd_part_parser_data - used to pass data to MTD partition parsers.
@@ -69,6 +70,7 @@ struct mtd_part_parser {
struct list_head list;
struct module *owner;
const char *name;
+ const struct of_device_id *of_match_table;
int (*parse_fn)(struct mtd_info *, const struct mtd_partition **,
struct mtd_part_parser_data *);
void (*cleanup)(const struct mtd_partition *pparts, int nr_parts);
diff --git a/include/linux/of_mtd.h b/include/linux/of_mtd.h
index e266caa36402..781362d0be0c 100644
--- a/include/linux/of_mtd.h
+++ b/include/linux/of_mtd.h
@@ -9,6 +9,10 @@
#ifndef __LINUX_OF_MTD_H
#define __LINUX_OF_MTD_H
+#include
+
+struct mtd_part_parser;
+
#ifdef CONFIG_OF_MTD
#include
@@ -18,6 +22,9 @@ int of_get_nand_ecc_strength(struct device_node *np);
int of_get_nand_bus_width(struct device_node *np);
bool of_get_nand_on_flash_bbt(struct device_node *np);
+bool of_mtd_match_mtd_parser(struct mtd_info *mtd,
+struct mtd_part_parser *parser);
+
#else /* CONFIG_OF_MTD */
static inline int of_get_nand_ecc_mode(struct device_node *np)
@@ -45,6 +52,12 @@ static inline bool of_get_nand_on_flash_bbt(struct
device_node *np)
return false;
}
+static inline bool of_mtd_match_mtd_parser(struct mtd_info *mtd,
+ struct mtd_part_parser *parser)
+{
+ return false;
+}
+
#endif /* CONFIG_OF_MTD */
#endif /* __LINUX_OF_MTD_H */
--
2.6.0.rc2.230.g3dd15c0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH 2/7] mtd: move partition parsers' Kconfig under a sub-menu
For better organization. Signed-off-by: Brian Norris --- drivers/mtd/Kconfig| 134 + drivers/mtd/partitions/Kconfig | 131 2 files changed, 134 insertions(+), 131 deletions(-) create mode 100644 drivers/mtd/partitions/Kconfig diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig index 42cc953309f1..a06e80d24499 100644 --- a/drivers/mtd/Kconfig +++ b/drivers/mtd/Kconfig @@ -23,137 +23,9 @@ config MTD_TESTS WARNING: some of the tests will ERASE entire MTD device which they test. Do not use these tests unless you really know what you do. -config MTD_REDBOOT_PARTS - tristate "RedBoot partition table parsing" - ---help--- - RedBoot is a ROM monitor and bootloader which deals with multiple - 'images' in flash devices by putting a table one of the erase - blocks on the device, similar to a partition table, which gives - the offsets, lengths and names of all the images stored in the - flash. - - If you need code which can detect and parse this table, and register - MTD 'partitions' corresponding to each image in the table, enable - this option. - - You will still need the parsing functions to be called by the driver - for your particular device. It won't happen automatically. The - SA1100 map driver (CONFIG_MTD_SA1100) has an option for this, for - example. - -if MTD_REDBOOT_PARTS - -config MTD_REDBOOT_DIRECTORY_BLOCK - int "Location of RedBoot partition table" - default "-1" - ---help--- - This option is the Linux counterpart to the - CYGNUM_REDBOOT_FIS_DIRECTORY_BLOCK RedBoot compile time - option. - - The option specifies which Flash sectors holds the RedBoot - partition table. A zero or positive value gives an absolute - erase block number. A negative value specifies a number of - sectors before the end of the device. - - For example "2" means block number 2, "-1" means the last - block and "-2" means the penultimate block. - -config MTD_REDBOOT_PARTS_UNALLOCATED - bool "Include unallocated flash regions" - help - If you need to register each unallocated flash region as a MTD - 'partition', enable this option. - -config MTD_REDBOOT_PARTS_READONLY - bool "Force read-only for RedBoot system images" - help - If you need to force read-only for 'RedBoot', 'RedBoot Config' and - 'FIS directory' images, enable this option. - -endif # MTD_REDBOOT_PARTS - -config MTD_CMDLINE_PARTS - tristate "Command line partition table parsing" - depends on MTD - ---help--- - Allow generic configuration of the MTD partition tables via the kernel - command line. Multiple flash resources are supported for hardware where - different kinds of flash memory are available. - - You will still need the parsing functions to be called by the driver - for your particular device. It won't happen automatically. The - SA1100 map driver (CONFIG_MTD_SA1100) has an option for this, for - example. - - The format for the command line is as follows: - - mtdparts=[; := :[,] - := [@offset][][ro] - := unique id used in mapping driver/device - := standard linux memsize OR "-" to denote all - remaining space - := (NAME) - - Due to the way Linux handles the command line, no spaces are - allowed in the partition definition, including mtd id's and partition - names. - - Examples: - - 1 flash resource (mtd-id "sa1100"), with 1 single writable partition: - mtdparts=sa1100:- - - Same flash, but 2 named partitions, the first one being read-only: - mtdparts=sa1100:256k(ARMboot)ro,-(root) - - If unsure, say 'N'. - -config MTD_AFS_PARTS - tristate "ARM Firmware Suite partition parsing" - depends on (ARM || ARM64) - ---help--- - The ARM Firmware Suite allows the user to divide flash devices into - multiple 'images'. Each such image has a header containing its name - and offset/size etc. - - If you need code which can detect and parse these tables, and - register MTD 'partitions' corresponding to each image detected, - enable this option. - - You will still need the parsing functions to be called by the driver - for your particular device. It won't happen automatically. The - 'physmap' map driver (CONFIG_MTD_PHYSMAP) does this, for example. - -config MTD_OF_PARTS -
[RFC PATCH 1/7] mtd: move partition parsers to drivers/mtd/partitions/
For better organization.
Signed-off-by: Brian Norris
---
drivers/mtd/Makefile | 8 +---
drivers/mtd/partitions/Makefile| 7 +++
drivers/mtd/{ => partitions}/afs.c | 0
drivers/mtd/{ => partitions}/ar7part.c | 0
drivers/mtd/{ => partitions}/bcm47xxpart.c | 0
drivers/mtd/{ => partitions}/bcm63xxpart.c | 0
drivers/mtd/{ => partitions}/cmdlinepart.c | 0
drivers/mtd/{ => partitions}/ofpart.c | 0
drivers/mtd/{ => partitions}/redboot.c | 0
9 files changed, 8 insertions(+), 7 deletions(-)
create mode 100644 drivers/mtd/partitions/Makefile
rename drivers/mtd/{ => partitions}/afs.c (100%)
rename drivers/mtd/{ => partitions}/ar7part.c (100%)
rename drivers/mtd/{ => partitions}/bcm47xxpart.c (100%)
rename drivers/mtd/{ => partitions}/bcm63xxpart.c (100%)
rename drivers/mtd/{ => partitions}/cmdlinepart.c (100%)
rename drivers/mtd/{ => partitions}/ofpart.c (100%)
rename drivers/mtd/{ => partitions}/redboot.c (100%)
diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile
index 99bb9a1f6e16..1c0cd3b1c7c3 100644
--- a/drivers/mtd/Makefile
+++ b/drivers/mtd/Makefile
@@ -6,13 +6,7 @@
obj-$(CONFIG_MTD) += mtd.o
mtd-y := mtdcore.o mtdsuper.o mtdconcat.o mtdpart.o
mtdchar.o
-obj-$(CONFIG_MTD_OF_PARTS) += ofpart.o
-obj-$(CONFIG_MTD_REDBOOT_PARTS) += redboot.o
-obj-$(CONFIG_MTD_CMDLINE_PARTS) += cmdlinepart.o
-obj-$(CONFIG_MTD_AFS_PARTS)+= afs.o
-obj-$(CONFIG_MTD_AR7_PARTS)+= ar7part.o
-obj-$(CONFIG_MTD_BCM63XX_PARTS)+= bcm63xxpart.o
-obj-$(CONFIG_MTD_BCM47XX_PARTS)+= bcm47xxpart.o
+obj-y += partitions/
# 'Users' - code which presents functionality to userspace.
obj-$(CONFIG_MTD_BLKDEVS) += mtd_blkdevs.o
diff --git a/drivers/mtd/partitions/Makefile b/drivers/mtd/partitions/Makefile
new file mode 100644
index ..89822f2bfa59
--- /dev/null
+++ b/drivers/mtd/partitions/Makefile
@@ -0,0 +1,7 @@
+obj-$(CONFIG_MTD_OF_PARTS) += ofpart.o
+obj-$(CONFIG_MTD_REDBOOT_PARTS) += redboot.o
+obj-$(CONFIG_MTD_CMDLINE_PARTS) += cmdlinepart.o
+obj-$(CONFIG_MTD_AFS_PARTS)+= afs.o
+obj-$(CONFIG_MTD_AR7_PARTS)+= ar7part.o
+obj-$(CONFIG_MTD_BCM63XX_PARTS)+= bcm63xxpart.o
+obj-$(CONFIG_MTD_BCM47XX_PARTS)+= bcm47xxpart.o
diff --git a/drivers/mtd/afs.c b/drivers/mtd/partitions/afs.c
similarity index 100%
rename from drivers/mtd/afs.c
rename to drivers/mtd/partitions/afs.c
diff --git a/drivers/mtd/ar7part.c b/drivers/mtd/partitions/ar7part.c
similarity index 100%
rename from drivers/mtd/ar7part.c
rename to drivers/mtd/partitions/ar7part.c
diff --git a/drivers/mtd/bcm47xxpart.c b/drivers/mtd/partitions/bcm47xxpart.c
similarity index 100%
rename from drivers/mtd/bcm47xxpart.c
rename to drivers/mtd/partitions/bcm47xxpart.c
diff --git a/drivers/mtd/bcm63xxpart.c b/drivers/mtd/partitions/bcm63xxpart.c
similarity index 100%
rename from drivers/mtd/bcm63xxpart.c
rename to drivers/mtd/partitions/bcm63xxpart.c
diff --git a/drivers/mtd/cmdlinepart.c b/drivers/mtd/partitions/cmdlinepart.c
similarity index 100%
rename from drivers/mtd/cmdlinepart.c
rename to drivers/mtd/partitions/cmdlinepart.c
diff --git a/drivers/mtd/ofpart.c b/drivers/mtd/partitions/ofpart.c
similarity index 100%
rename from drivers/mtd/ofpart.c
rename to drivers/mtd/partitions/ofpart.c
diff --git a/drivers/mtd/redboot.c b/drivers/mtd/partitions/redboot.c
similarity index 100%
rename from drivers/mtd/redboot.c
rename to drivers/mtd/partitions/redboot.c
--
2.6.0.rc2.230.g3dd15c0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH 5/7] mtd: partitions: factor out "match by name" handling
This code structure is going to be imitated for a match-by-device-node
implementation, so let's factor out a few functions to make this easier.
Signed-off-by: Brian Norris
---
drivers/mtd/mtdpart.c | 67 +++
1 file changed, 52 insertions(+), 15 deletions(-)
diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index 10bf304027dd..b3100742ddf6 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -687,23 +687,47 @@ int add_mtd_partitions(struct mtd_info *master,
static DEFINE_SPINLOCK(part_parser_lock);
static LIST_HEAD(part_parsers);
-static struct mtd_part_parser *mtd_part_parser_get(const char *name)
+static bool mtd_part_parser_match_name(struct mtd_part_parser *p,
+ const char *name)
+{
+ return !strcmp(p->name, name);
+}
+
+static struct mtd_part_parser *__mtd_part_parser_get_by_name(const char *name)
{
struct mtd_part_parser *p, *ret = NULL;
spin_lock(&part_parser_lock);
- list_for_each_entry(p, &part_parsers, list)
- if (!strcmp(p->name, name) && try_module_get(p->owner)) {
+ list_for_each_entry(p, &part_parsers, list) {
+ if (mtd_part_parser_match_name(p, name) &&
+ try_module_get(p->owner)) {
ret = p;
break;
}
+ }
spin_unlock(&part_parser_lock);
return ret;
}
+static struct mtd_part_parser *mtd_part_parser_get_by_name(const char *name)
+{
+ struct mtd_part_parser *p;
+
+ /* Get parser, if already loaded */
+ p = __mtd_part_parser_get_by_name(name);
+ if (p)
+ return p;
+
+ if (request_module("%s", name))
+ return NULL;
+
+ /* Try again */
+ return __mtd_part_parser_get_by_name(name);
+}
+
static inline void mtd_part_parser_put(const struct mtd_part_parser *p)
{
module_put(p->owner);
@@ -752,6 +776,27 @@ static const char * const default_mtd_part_types[] = {
NULL
};
+static int mtd_part_do_parse(struct mtd_part_parser *parser,
+struct mtd_info *master,
+struct mtd_partitions *pparts,
+struct mtd_part_parser_data *data)
+{
+ int ret;
+
+ ret = (*parser->parse_fn)(master, &pparts->parts, data);
+ pr_debug("%s: parser %s: %i\n", master->name, parser->name, ret);
+ if (ret <= 0)
+ return ret;
+
+ pr_notice("%d %s partitions found on MTD device %s\n",
+ ret, parser->name, master->name);
+
+ pparts->nr_parts = ret;
+ pparts->parser = parser;
+
+ return ret;
+}
+
/**
* parse_mtd_partitions - parse MTD partitions
* @master: the master partition (describes whole MTD device)
@@ -785,23 +830,15 @@ int parse_mtd_partitions(struct mtd_info *master, const
char *const *types,
for ( ; *types; types++) {
pr_debug("%s: parsing partitions %s\n", master->name, *types);
- parser = mtd_part_parser_get(*types);
- if (!parser && !request_module("%s", *types))
- parser = mtd_part_parser_get(*types);
+ parser = mtd_part_parser_get_by_name(*types);
pr_debug("%s: got parser %s\n", master->name,
parser ? parser->name : NULL);
if (!parser)
continue;
- ret = (*parser->parse_fn)(master, &pparts->parts, data);
- pr_debug("%s: parser %s: %i\n",
-master->name, parser->name, ret);
- if (ret > 0) {
- printk(KERN_NOTICE "%d %s partitions found on MTD
device %s\n",
- ret, parser->name, master->name);
- pparts->nr_parts = ret;
- pparts->parser = parser;
+ ret = mtd_part_do_parse(parser, master, pparts, data);
+ /* Found partitions! */
+ if (ret > 0)
return 0;
- }
mtd_part_parser_put(parser);
/*
* Stash the first error we see; only report it if no parser
--
2.6.0.rc2.230.g3dd15c0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH 6/7] RFC: mtd: partitions: enable of_match_table matching
Partition parsers can now provide an of_match_table to enable
flash<-->parser matching via device tree.
TODO: Doesn't yet work when parser is built as module. I can't just use
request_module() and friends, since OF matches don't tell me the name of
the driver/module. Maybe I can report uevents?
Signed-off-by: Brian Norris
---
drivers/mtd/mtdpart.c | 32
1 file changed, 32 insertions(+)
diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index b3100742ddf6..91eb2df0bf1e 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -29,6 +29,7 @@
#include
#include
#include
+#include
#include
#include
@@ -728,6 +729,25 @@ static struct mtd_part_parser
*mtd_part_parser_get_by_name(const char *name)
return __mtd_part_parser_get_by_name(name);
}
+static struct mtd_part_parser *mtd_part_parser_get_by_of(struct mtd_info *mtd)
+{
+ struct mtd_part_parser *p, *ret = NULL;
+
+ spin_lock(&part_parser_lock);
+
+ list_for_each_entry(p, &part_parsers, list) {
+ if (of_mtd_match_mtd_parser(mtd, p) &&
+ try_module_get(p->owner)) {
+ ret = p;
+ break;
+ }
+ }
+
+ spin_unlock(&part_parser_lock);
+
+ return ret;
+}
+
static inline void mtd_part_parser_put(const struct mtd_part_parser *p)
{
module_put(p->owner);
@@ -847,6 +867,18 @@ int parse_mtd_partitions(struct mtd_info *master, const
char *const *types,
if (ret < 0 && !err)
err = ret;
}
+
+ parser = mtd_part_parser_get_by_of(master);
+ if (!parser)
+ return err;
+
+ ret = mtd_part_do_parse(parser, master, pparts, data);
+ if (ret > 0)
+ return 0;
+ mtd_part_parser_put(parser);
+ if (ret < 0 && !err)
+ err = ret;
+
return err;
}
--
2.6.0.rc2.230.g3dd15c0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH 7/7] mtd: partitions: add Google's FMAP partition parser
Cc: David Hendricks
Signed-off-by: Brian Norris
---
drivers/mtd/partitions/Kconfig | 7 ++
drivers/mtd/partitions/Makefile | 1 +
drivers/mtd/partitions/google_fmap.c | 226 +++
3 files changed, 234 insertions(+)
create mode 100644 drivers/mtd/partitions/google_fmap.c
diff --git a/drivers/mtd/partitions/Kconfig b/drivers/mtd/partitions/Kconfig
index 0827d7a8be4e..98783f1d3a36 100644
--- a/drivers/mtd/partitions/Kconfig
+++ b/drivers/mtd/partitions/Kconfig
@@ -129,3 +129,10 @@ config MTD_BCM47XX_PARTS
help
This provides partitions parser for devices based on BCM47xx
boards.
+
+config MTD_GOOGLE_FMAP_PARTS
+ tristate "Google's Flash Map (FMAP) partition support"
+ help
+ This provides partition parsing for Google's flash map layout, used
+ primarily on the boot flash of Chrome OS hardware (e.g., Chromebooks
+ and Chromeboxes).
diff --git a/drivers/mtd/partitions/Makefile b/drivers/mtd/partitions/Makefile
index 89822f2bfa59..ab398c7f4d01 100644
--- a/drivers/mtd/partitions/Makefile
+++ b/drivers/mtd/partitions/Makefile
@@ -5,3 +5,4 @@ obj-$(CONFIG_MTD_AFS_PARTS) += afs.o
obj-$(CONFIG_MTD_AR7_PARTS)+= ar7part.o
obj-$(CONFIG_MTD_BCM63XX_PARTS)+= bcm63xxpart.o
obj-$(CONFIG_MTD_BCM47XX_PARTS)+= bcm47xxpart.o
+obj-$(CONFIG_MTD_GOOGLE_FMAP_PARTS) += google_fmap.o
diff --git a/drivers/mtd/partitions/google_fmap.c
b/drivers/mtd/partitions/google_fmap.c
new file mode 100644
index ..abd10eb65c84
--- /dev/null
+++ b/drivers/mtd/partitions/google_fmap.c
@@ -0,0 +1,226 @@
+/*
+ * Parse Google FMAP partitions
+ *
+ * Author: Brian Norris
+ *
+ * Copyright © 2015 Google, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * See:
+ * https://github.com/dhendrix/flashmap/blob/wiki/FmapSpec.md
+ *
+ * Notes:
+ * - scans only at block boundaries; this is not guaranteed for FMAP (the
+ * Chrome OS tools do a kind of stride search, of decreasing size), but
+ * seems like a decent start
+ * - at worst, scans (beginning of) every block on an unformatted flash
+ * - only validates the "__FMAP__" signature, just like the Chrome OS tools;
+ * however, this seems (theoretically) easy to produce false matches
+ * - major/minor version numbers are currently unused
+ */
+
+#define pr_fmt(fmt)KBUILD_MODNAME ": " fmt
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+
+static const char fmap_signature[] = "__FMAP__";
+
+struct fmap_layout {
+ uint8_t signature[8]; /* "__FMAP__" (0x5F5F50414D465F5F) */
+ uint8_t ver_major; /* major version number of this
structure */
+ uint8_t ver_minor; /* minor version of this structure */
+ __le64 base;/* physical memory-mapped address of
the flash chip */
+ __le32 size;/* size of the flash chip in bytes */
+ uint8_t name[32]; /* descriptive name of this flash
device, 0 terminated */
+ __le16 nareas; /* number of areas described by areas[]
below */
+ struct fmap_area {
+ __le32 offset; /* offset of this area in the flash
device */
+ __le32 size;/* size of this area in bytes */
+ uint8_t name[32]; /* descriptive name of this area, 0
terminated */
+ __le16 flags; /* flags for this area */
+ } __packed areas[0];
+} __packed;
+
+/* mtd_read() helper */
+static int fmap_mtd_read(struct mtd_info *mtd, loff_t offset, size_t len,
+void *buf)
+{
+ size_t retlen;
+ int ret;
+
+ ret = mtd_read(mtd, offset, len, &retlen, buf);
+ if (ret)
+ return ret;
+ if (retlen != len)
+ return -EIO;
+ return 0;
+}
+
+/* Return 0 on no match, non-zero on match */
+static inline int fmap_check_signature(struct fmap_layout *fmap)
+{
+ return !strncmp(fmap->signature, fmap_signature,
+ sizeof(fmap->signature));
+}
+
+static int fmap_parse_block(struct mtd_info *master,
+ const struct mtd_partition **pparts,
+ struct fmap_layout *fmap, size_t maxlen)
+{
+ struct mtd_partition *parts;
+ char *names;
+ int nparts;
+
[RFC PATCH 3/7] doc: dt: mtd: partition: add on-flash format binding
The platform description (such as the type of partition formats used on
a given flash) should be done independently of the flash driver in use.
However, we can't reasonably have *all* partition parsers run on all
flash (until they find a match), so let's overload the 'partitions'
subnode to support specifying which format(s) to try in the device tree.
Start by supporting Google's (Chrome OS) FMAP structure.
There have been others interested in extending devicetree support to
other parsers, like the bcm47xxpart parser:
http://patchwork.ozlabs.org/patch/475986/
and the AFS (ARM Flash Structure?) parser:
http://patchwork.ozlabs.org/patch/537827/
Signed-off-by: Brian Norris
---
.../devicetree/bindings/mtd/partition.txt | 75 --
1 file changed, 69 insertions(+), 6 deletions(-)
diff --git a/Documentation/devicetree/bindings/mtd/partition.txt
b/Documentation/devicetree/bindings/mtd/partition.txt
index 28ae56f5c972..1bf9a7243993 100644
--- a/Documentation/devicetree/bindings/mtd/partition.txt
+++ b/Documentation/devicetree/bindings/mtd/partition.txt
@@ -1,29 +1,56 @@
-Representing flash partitions in devicetree
+Flash partitions in device tree
+===
-Partitions can be represented by sub-nodes of an mtd device. This can be used
+Flash devices can be partitioned into one or more functional ranges (e.g.,
+"boot code", "nvram", and "kernel") in at least two distinct ways:
+
+ (A) a fixed flash layout at production time or
+ (B) with an on-flash partition table, such as RedBoot, that describes the
+ geometry and naming/purpose of each functional region
+
+The former typically requires an operating system to learn about the
+partitioning from some kind of metadata provided by the bootloader/firmware.
+Such partitions can be described using the method in "Section A: Fixed
Partitions".
+
+The latter is somewhat analogous to partition tables used on block devices
+(e.g., MBR or GPT), except that there is less standardization for flash
+devices, and it is not always safe or efficient to attempt to search for all of
+them on every flash device in the system, particularly since many of them allow
+their data structures to be placed anywhere on the flash, and so require
+scanning the entire flash device to find them.
+
+To assist system software in locating these partition tables, we provide a
+binding to describe which partition format(s) may be used on a given flash,
+found below in "Section B: On-Flash Partition Tables".
+
+
+Section A: Fixed Partitions
+---
+
+Partitions can be represented by sub-nodes of a flash device. This can be used
on platforms which have strong conventions about which portions of a flash are
used for what purposes, but which don't use an on-flash partition table such
as RedBoot.
-The partition table should be a subnode of the mtd node and should be named
+The partition table should be a subnode of the flash node and should be named
'partitions'. This node should have the following property:
- compatible : (required) must be "partitions"
Partitions are then defined in subnodes of the partitions node.
-For backwards compatibility partitions as direct subnodes of the mtd device are
+For backwards compatibility partitions as direct subnodes of the flash device
are
supported. This use is discouraged.
NOTE: also for backwards compatibility, direct subnodes that have a compatible
string are not considered partitions, as they may be used for other bindings.
#address-cells & #size-cells must both be present in the partitions subnode of
the
-mtd device. There are two valid values for both:
+flash device. There are two valid values for both:
<1>: for partitions that require a single 32-bit cell to represent their
size/address (aka the value is below 4 GiB)
<2>: for partitions that require two 32-bit cells to represent their
size/address (aka the value is 4 GiB or greater).
Required properties:
-- reg : The partition's offset and size within the mtd bank.
+- reg : The partition's offset and size within the flash
Optional properties:
- label : The label / name for this partition. If omitted, the label is taken
@@ -89,3 +116,39 @@ flash@2 {
};
};
};
+
+
+Section B: On-Flash Partition Tables
+
+
+System designers use a variety of on-flash data structures to describe the
+layout of the flash. Because it's not always optimal for system software to
+scan for every sort of data structure that might be used, one can specify which
+structure(s) might be used on a given flash using the 'partitions' subnode of
+the flash node.
+
+Node name: partitions
+Properties:
+ - compatible: (required) used to define which partition format(s) may be in
+ use on this flash may contain one or mor
[RFC PATCH 0/7] mtd: partitions: add of_match_table support
Hi,
There have been several discussions [1] about adding a device tree binding for
associating flash devices with the partition parser(s) that are used on the
flash. There are a few reasons:
(1) drivers shouldn't have to be encoding platform knowledge by listing what
parsers might be used on a given system (this is the currently all that's
supported)
(2) we can't just scan for all supported parsers (like the block system does),
since
there is a wide diversity of "formats" (no standardization), and it is not
always safe or efficient to attempt to do so, particularly since many of
them allow their data structures to be placed anywhere on the flash, and
so require scanning the entire flash device to find them.
So instead, let's support a new binding so that a device tree can specify what
partition formats might be used. This seems like a reasonable choice (even
though it's not strictly a hardware description) because the flash layout /
partitioning is often very closely tied with the bootloader/firmware, at
production time.
Also, as an example first-use of this mechanism, I support Google's FMAP flash
structure, used on Chrome OS devices.
Note that this is an RFC, mainly for the reason noted in patch 6 ("RFC: mtd:
partitions: enable of_match_table matching"): the of_match_table support won't
yet autoload a partition parser that is built as a module. I'm not quite sure
if there's a lot of value in supporting MTD parsers as modules (block partition
support can't be), but that is supported for "by-name" parser lookups in MTD
already, so I don't feel like dropping that feature yet. Tips or thoughts are
particularly welcome on this aspect!
Also note that there's an existing undocumented binding for a
"linux,part-probe" property, but it is only usable on the physmap_of.c driver
at the moment, and it is IMO not a good binding. I posted my thoughts on that
previously here [2], and since no one else cared to make a better one...I did
it myself.
I'd love it if we could kill the unreviewed binding off in favor of something
more like this...
Currently based on v2 of "mtd: partitions: support cleanup callback for
parsers":
http://lkml.kernel.org/g/1449271518-118900-1-git-send-email-computersforpe...@gmail.com
and this series
("mtd: ofpart: don't complain about missing 'partitions' node too loudly" and
"doc: dt: mtd: partitions: add compatible property to "partitions" node"):
http://lkml.kernel.org/g/1449194529-145705-1-git-send-email-computersforpe...@gmail.com
Both of which should hopefully be merged soon.
The current total of this work is stashed here for now:
git fetch git://git.infradead.org/users/norris/linux-mtd.git
partition-of-match
I may rewrite this branch if I post future revisions of these patch sets, FYI.
I look forward to your reviews.
Regards,
Brian
[1] Trying to extend "linux,part-probe":
http://patchwork.ozlabs.org/patch/475988/
For bcm47xxpart:
http://patchwork.ozlabs.org/patch/475986/
For AFS:
http://patchwork.ozlabs.org/patch/537827/
[2] "mtd: document linux-specific partition parser DT binding"
http://lists.infradead.org/pipermail/linux-mtd/2015-October/062773.html
Brian Norris (7):
mtd: move partition parsers to drivers/mtd/partitions/
mtd: move partition parsers' Kconfig under a sub-menu
doc: dt: mtd: partition: add on-flash format binding
mtd: add of_match_mtd_parser() and of_mtd_match_mtd_parser() helpers
mtd: partitions: factor out "match by name" handling
RFC: mtd: partitions: enable of_match_table matching
mtd: partitions: add Google's FMAP partition parser
.../devicetree/bindings/mtd/partition.txt | 75 ++-
drivers/mtd/Kconfig| 134 +---
drivers/mtd/Makefile | 8 +-
drivers/mtd/mtdpart.c | 99 +++--
drivers/mtd/partitions/Kconfig | 138 +
drivers/mtd/partitions/Makefile| 8 +
drivers/mtd/{ => partitions}/afs.c | 0
drivers/mtd/{ => partitions}/ar7part.c | 0
drivers/mtd/{ => partitions}/bcm47xxpart.c | 0
drivers/mtd/{ => partitions}/bcm63xxpart.c | 0
drivers/mtd/{ => partitions}/cmdlinepart.c | 0
drivers/mtd/partitions/google_fmap.c | 226 +
drivers/mtd/{ => partitions}/ofpart.c | 0
drivers/mtd/{ => partitions}/redboot.c | 0
drivers/of/of_mtd.c| 33 +++
include/linux/mtd/partitions.h | 2 +
include/linux/of_mtd.h | 13 ++
17 files changed, 577 insertions(+), 159 deletions(-)
crea
[PATCH for-4.4 1/2] mtd: ofpart: don't complain about missing 'partitions' node too loudly
The ofpart partition parser might be run on DT-enabled systems that
don't have any "ofpart" partition subnodes at all, since "ofpart" is in
the default parser list. So don't complain loudly on every boot.
Example: using m25p80.c with no intent to use ofpart:
&spi2 {
status = "okay";
flash@0 {
compatible = "jedec,spi-nor";
reg = <0>;
};
};
I see this warning:
[0.588471] m25p80 spi2.0: gd25q32 (4096 Kbytes)
[0.593091] spi2.0: 'partitions' subnode not found on /spi@ff13/flash@0.
Trying to parse direct subnodes as partitions.
Cc: Michal Suchanek
Signed-off-by: Brian Norris
---
This one really seems like we should put it into 4.4. Basically everyone with a
DT-enabled MTD using the default parsers will get this warning, even if they're
not intending to use ofpart at all.
drivers/mtd/ofpart.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/mtd/ofpart.c b/drivers/mtd/ofpart.c
index 669c3452f278..3e9c5857c991 100644
--- a/drivers/mtd/ofpart.c
+++ b/drivers/mtd/ofpart.c
@@ -46,8 +46,13 @@ static int parse_ofpart_partitions(struct mtd_info *master,
ofpart_node = of_get_child_by_name(mtd_node, "partitions");
if (!ofpart_node) {
- pr_warn("%s: 'partitions' subnode not found on %s. Trying to
parse direct subnodes as partitions.\n",
- master->name, mtd_node->full_name);
+ /*
+* We might get here even when ofpart isn't used at all (e.g.,
+* when using another parser), so don't be louder than
+* KERN_DEBUG
+*/
+ pr_debug("%s: 'partitions' subnode not found on %s. Trying to
parse direct subnodes as partitions.\n",
+master->name, mtd_node->full_name);
ofpart_node = mtd_node;
dedicated = false;
}
--
2.6.0.rc2.230.g3dd15c0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH for-4.4 2/2] doc: dt: mtd: partitions: add compatible property to "partitions" node
As noted here [1], there are potentially future conflicts if we try to
use MTD's "partitions" subnode to describe anything besides just the
fixed-in-the-device-tree partitions currently described in this
document. Particularly, there was a proposal to use this node for the
AFS parser too.
It can pose a (small) problem to try to differentiate the following
nodes:
// using binding as currently specified
partitions {
#address-cells = ;
#size-cells = ;
partition@0 {
...;
};
};
and
// proposed future binding
partitions {
compatible = "arm,arm-flash-structure";
};
It's especially difficult if other uses of this node start having
subnodes.
So, since the "partitions" node is new in v4.4, let's fixup the binding
before release so that it requires a compatible property, so it's much
clearer to distinguish. e.g.:
// proposed
partitions {
compatible = "partitions";
#address-cells = ;
#size-cells = ;
partition@0 {
...;
};
};
[1] Subject: "mtd: create a partition type device tree binding"
http://lkml.kernel.org/g/20151113220039.ga74...@google.com
http://lists.infradead.org/pipermail/linux-mtd/2015-November/063355.html
http://lists.infradead.org/pipermail/linux-mtd/2015-November/063364.html
Cc: Michal Suchanek
Signed-off-by: Brian Norris
---
This one is more of a future proofing patch. We should probably take this for
4.4, before the binding "stabilizes" (I don't actually see any users yet), or
else we'll have to find some other (possibly more complicated) way to avoid
this potential collision on future development.
Documentation/devicetree/bindings/mtd/partition.txt | 7 ++-
drivers/mtd/ofpart.c| 3 +++
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/mtd/partition.txt
b/Documentation/devicetree/bindings/mtd/partition.txt
index f1e2a02381a4..047e80575881 100644
--- a/Documentation/devicetree/bindings/mtd/partition.txt
+++ b/Documentation/devicetree/bindings/mtd/partition.txt
@@ -6,7 +6,9 @@ used for what purposes, but which don't use an on-flash
partition table such
as RedBoot.
The partition table should be a subnode of the mtd node and should be named
-'partitions'. Partitions are defined in subnodes of the partitions node.
+'partitions'. This node should have the following property:
+- compatible : (required) must be "partitions"
+Partitions are then defined in subnodes of the partitions node.
For backwards compatibility partitions as direct subnodes of the mtd device are
supported. This use is discouraged.
@@ -36,6 +38,7 @@ Examples:
flash@0 {
partitions {
+ compatible = "partitions";
#address-cells = <1>;
#size-cells = <1>;
@@ -53,6 +56,7 @@ flash@0 {
flash@1 {
partitions {
+ compatible = "partitions";
#address-cells = <1>;
#size-cells = <2>;
@@ -66,6 +70,7 @@ flash@1 {
flash@2 {
partitions {
+ compatible = "partitions";
#address-cells = <2>;
#size-cells = <2>;
diff --git a/drivers/mtd/ofpart.c b/drivers/mtd/ofpart.c
index 3e9c5857c991..23cd809fad4c 100644
--- a/drivers/mtd/ofpart.c
+++ b/drivers/mtd/ofpart.c
@@ -55,6 +55,9 @@ static int parse_ofpart_partitions(struct mtd_info *master,
master->name, mtd_node->full_name);
ofpart_node = mtd_node;
dedicated = false;
+ } else if (!of_device_is_compatible(ofpart_node, "partitions")) {
+ /* The 'partitions' subnode might be used by another parser */
+ return 0;
}
/* First count the subnodes */
--
2.6.0.rc2.230.g3dd15c0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 1/3] dt-bindings: binding for jz4780-{nand,bch}
On Thu, Dec 03, 2015 at 03:38:12PM -0600, Rob Herring wrote: > On Thu, Dec 03, 2015 at 12:02:20PM +, Harvey Hunt wrote: > > v8 -> v9: > > - Document that partitions are represented as a child node of a NAND chip. > > Don't multiple flash chips typically get interleaved in order to get > parallelism needed for performance? Then the view of the partitions > would apply across all chips. Not in MTD so far. We have mtdconcat to do some combination of flash, but it's not too easy to use right now. There are also some "MTD RAID" patches submitted recently that might cover what you're talking about, but that's brand new and unreviewed, and I don't think anyone has considered trying to handle partitions for such a thing yet. Partitioning of this kind isn't even that useful for NAND flash, actually, since fixed assignment of flash ranges restricts the flexibility of UBI's wear-leveling algorithms. It probably makes more sense to deal with UBI volumes instead of MTD partitions when talking about NAND flash. And those aren't specified in DT. > Anyway, it's optional, so: > > Acked-by: Rob Herring Brian -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 00/27] memory: omap-gpmc: mtd: nand: Support GPMC NAND on non-OMAP platforms
Hi,
On Thu, Dec 03, 2015 at 11:38:14AM +0530, Roger Quadros wrote:
> On 03/12/15 10:39, Brian Norris wrote:
> > On Fri, Sep 18, 2015 at 05:53:22PM +0300, Roger Quadros wrote:
> >> We do a couple of things in this series which result in
> >> cleaner device tree implementation, faster perfomance and
> >> multi-platform support. As an added bonus we get new GPI/Interrupt pins
> >> for use in the system.
> >>
> >> - Establish a custom interface between NAND and GPMC driver. This is
> >> needed because all of the NAND registers sit in the GPMC register space.
> >> Some bits like NAND IRQ are even shared with GPMC.
> >>
> >> - Remove NAND IRQ handling from omap-gpmc driver, share the GPMC IRQ
> >> with the omap2-nand driver and handle NAND IRQ events in the NAND driver.
> >> This causes performance increase when using prefetch-irq mode.
> >> 30% increase in read, 17% increase in write in prefetch-irq mode.
> >>
> >> - Clean up device tree support so that omap-gpmc IP and the omap2 NAND
> >> driver can be used on non-OMAP platforms. e.g. Keystone.
> >>
> >> - Implement GPIOCHIP + IRQCHIP for the GPMC WAITPINS. SoCs can contain
> >> 2 to 4 of these and most of them would be unused otherwise. It also
> >> allows a cleaner implementation of NAND Ready pin status for the NAND
> >> driver.
> >>
> >> - Implement GPIOlib based NAND ready pin checking for OMAP NAND driver.
> >>
> >> This series is available at
> >> g...@github.com:rogerq/linux.git
> >> in branch
> >> for-v4.4/gpmc-v3
> >>
> >> cheers,
> >> -roger
> >>
> >> Changelog:
> >> v3:
> >> -Fixed and tested NAND using legacy boot on omap3-beagle.
> >> -Support rising and falling edge interrupts on WAITpins.
> >> -Update DT node of all gpmc users.
> >
> > The MTD stuff looks mostly good to me know. I've made all my comments
> > for now, but I'm not sure how you're going to end up rebasing/splitting
> > and what you're going to do with the irqchip removal, so I'll refrain
> > from ack's for now. Hopefully I can either ack or merge v4.
>
> I'll retain the irqchip model for now and send a v4 with all comments
> addressed and better subsystem wise patch split.
>
> >
> > I brought it up on one other patch, but it's not really clear to me what
> > the split is on board file vs. device tree handling, since you seem to
> > have a combination of both (i.e., platform data that passes along device
> > nodes). What's the plan on that?
>
> Platform data no longer passes device nodes. We're either true device tree
> or plain legacy. The deprecated fields are no longer used once the series is
> applied.
Well, they're still sorta used (you assign info->of_node =
pdata->of_node, for instance). As dicussed in the other thread, I think
we can avoid the deprecation part and just kill the fields though, and
that would make things clearer.
> > And of course, there's the question of how exactly to merge this, given
> > the:
> > (1) conflicts already existing in the MTD dev tree
>
> I'll rebase the series on top of MTD dev tree.
OK. FWIW, we so far only need to base them on commit a61ae81a1907 ("mtd:
nand: drop unnecessary partition parser data"). Maybe when queueing up a
branch, that'd be the best starting point for Tony, so he doesn't need
to have all of MTD's stuff in his tree too? I can set up a signed tag or
something, if that would be helpful.
But for sending patches, the latest l2-mtd.git is fine too.
> > (2) this touches several trees, often in the same patch and
>
> I'll try my best to split the patches but not sure if this could be 100%
> clean split without functional breakage.
>
> > (3) even if the patches were split out a little better into MTD and
> > non-MTD stuff, I think there would still be dependencies such that
> > we'd need at least 1 (probably 2) cross merges to get it all
> > straight
>
> That is correct.
> Is it OK if functionality breaks if for example only MTD changes are
> considered?
I think I may have misunderstood the branch proposal. If Tony queues up:
l2-mtd.git (or just up to commit a61ae81a1907)
+
your patches
and I pull that back into l2-mtd.git as well, then we don't need to
worry about patches that touch multiple "trees". Just do whatever makes
things clearest, including disregarding some of my comments along the
line of (3).
Sorry for any confusion.
Brian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 11/27] mtd: nand: omap: Clean up device tree support
On Thu, Dec 03, 2015 at 11:27:13AM +0530, Roger Quadros wrote:
> On 03/12/15 09:59, Brian Norris wrote:
> > On Tue, Oct 06, 2015 at 01:35:48PM +0300, Roger Quadros wrote:
> >> arch/arm/mach-omap2/gpmc-nand.c | 5 +-
> >> drivers/memory/omap-gpmc.c | 143
> >> +++
> >> drivers/mtd/nand/omap2.c | 136
> >> +
> >> include/linux/platform_data/mtd-nand-omap2.h | 3 +-
> >> 4 files changed, 155 insertions(+), 132 deletions(-)
> >
> > Also, this is going to be hard to manage across trees, as you touch
> > three drivers all at once. Is it not possible to split any of this apart
> > better?
>
> Will need some more effort and I can do it. Butm if we're going to start
> an with immutable branch with everything in, is it worth the effort?
I don't think I noticed the "everything in it" part. I was assuming
you'd have non-MTD changes in the immutable branch, and MTD stuff on
top. But that is indeed more difficult. I'm fine with everything going
in one branch, and pulling that all into MTD.
Then the hangup is that you'll need to have some of l2-mtd.git in that
branch as a prerequisite, if you're going to rebase. Or else I have to
fix it up when merging back into l2-mtd.git...
...
> >> diff --git a/include/linux/platform_data/mtd-nand-omap2.h
> >> b/include/linux/platform_data/mtd-nand-omap2.h
> >> index a067f58..ff27e5a 100644
> >> --- a/include/linux/platform_data/mtd-nand-omap2.h
> >> +++ b/include/linux/platform_data/mtd-nand-omap2.h
> >> @@ -76,11 +76,10 @@ struct omap_nand_platform_data {
> >>int devsize;
> >>enum omap_ecc ecc_opt;
> >>
> >> - /* for passing the partitions */
> >> - struct device_node *of_node;
> >>struct device_node *elm_of_node;
> >>
> >>/* deprecated */
> >>struct gpmc_nand_regs reg;
> >> + struct device_node *of_node;
> >
> > I'm a little confused here. Do you have a mixed platform data / device
> > tree setup here? That's odd. (It also seems if that was really
> > necessary, you could have the board file set pdev->dev.of_node before
> > registering it, then you don't need this field.) But really, if you're
> > partly using device tree, can't you just convert completely? Or is this
> > a two-phase process, and you're planning to convert omap2 to full device
> > tree?
>
> The existing device tree implementation for omap2-nand was like this:
> omap-gpmc.c driver was creating a platform device for the nand device and
> passing the device node via platform data.
>
> After this series we no longer do this and that's why of_node is marked
> deprecated in platform data. I might as well could just get rid of it
> but wasn't sure of how we're going to integrate the changes into the
> subsystem trees so just marked it deprecated.
Whoops, sorry I didn't read the "deprecated" header there this time. I
thought the movement was odd. *facepalm*
But yes, especially if we're putting everything in one branch, it'd be
more clear to simply kill off things instead of marking things
deprecated. It's especially confusing, since you're actually still using
the field.
Brian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 04/27] mtd: nand: omap2: Use gpmc_omap_get_nand_ops() to get NAND registers
On Fri, Sep 18, 2015 at 05:53:26PM +0300, Roger Quadros wrote:
> Deprecate nand register passing via platform data and use
> gpmc_omap_get_nand_ops() instead.
>
> Signed-off-by: Roger Quadros
> ---
> arch/arm/mach-omap2/gpmc-nand.c | 2 --
> drivers/mtd/nand/omap2.c | 9 -
> include/linux/platform_data/mtd-nand-omap2.h | 4 +++-
> 3 files changed, 11 insertions(+), 4 deletions(-)
This one also seems a bit oddly-split, if you're trying to allow
bringing these into different trees. The nand/omap2.c changes seem like
they can be done completely on their own (after the previous patches),
and the arch/arm/mach-omap2/gpmc-nand.c and
include/linux/platform_data/mtd-nand-omap2.h changes can come in their
own patch afterward.
That does still make things a little complicated for applying to
different trees, though, as we have some arch/arm -> drivers/mtd ->
arch/arm dependencies.
If it helps, I can try to provide Tony with a stable v4.4-rc1-based
piece of the MTD tree, and just take everything through there (with
acks). (FWIW, everything in l2-mtd.git can be considered stable,
git-wise. I've been keeping a clean history. But it'd be best to
coordinate what points to cross-merge.)
Then if we do that, we'd have to keep a close eye to make sure I don't
take any more conflicting changes to drivers/mtd/nand/omap2.c, or else
do another cross-merge...
Any better suggestions?
Brian
>
> diff --git a/arch/arm/mach-omap2/gpmc-nand.c b/arch/arm/mach-omap2/gpmc-nand.c
> index 72918c4..04e6998 100644
> --- a/arch/arm/mach-omap2/gpmc-nand.c
> +++ b/arch/arm/mach-omap2/gpmc-nand.c
> @@ -121,8 +121,6 @@ int gpmc_nand_init(struct omap_nand_platform_data
> *gpmc_nand_data,
> if (err < 0)
> goto out_free_cs;
>
> - gpmc_update_nand_reg(&gpmc_nand_data->reg, gpmc_nand_data->cs);
> -
> if (!gpmc_hwecc_bch_capable(gpmc_nand_data->ecc_opt)) {
> pr_err("omap2-nand: Unsupported NAND ECC scheme selected\n");
> err = -EINVAL;
> diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
> index 60fa899..f214fe2 100644
> --- a/drivers/mtd/nand/omap2.c
> +++ b/drivers/mtd/nand/omap2.c
> @@ -28,6 +28,7 @@
> #include
> #include
>
> +#include
> #include
>
> #define DRIVER_NAME "omap2-nand"
> @@ -169,7 +170,9 @@ struct omap_nand_info {
> } iomode;
> u_char *buf;
> int buf_len;
> + /* Interface to GPMC */
> struct gpmc_nand_regs reg;
> + struct gpmc_nand_ops*ops;
> /* generated at runtime depending on ECC algorithm and layout selected
> */
> struct nand_ecclayout oobinfo;
> /* fields specific for BCHx_HW ECC scheme */
> @@ -1677,9 +1680,13 @@ static int omap_nand_probe(struct platform_device
> *pdev)
>
> platform_set_drvdata(pdev, info);
>
> + info->ops = gpmc_omap_get_nand_ops(&info->reg, info->gpmc_cs);
> + if (!info->ops) {
> + dev_err(&pdev->dev, "Failed to get GPMC->NAND interface\n");
> + return -ENODEV;
> + }
> info->pdev = pdev;
> info->gpmc_cs = pdata->cs;
> - info->reg = pdata->reg;
> info->of_node = pdata->of_node;
> info->ecc_opt = pdata->ecc_opt;
> mtd = &info->mtd;
> diff --git a/include/linux/platform_data/mtd-nand-omap2.h
> b/include/linux/platform_data/mtd-nand-omap2.h
> index 090bbab..a067f58 100644
> --- a/include/linux/platform_data/mtd-nand-omap2.h
> +++ b/include/linux/platform_data/mtd-nand-omap2.h
> @@ -75,10 +75,12 @@ struct omap_nand_platform_data {
> enum nand_ioxfer_type;
> int devsize;
> enum omap_ecc ecc_opt;
> - struct gpmc_nand_regs reg;
>
> /* for passing the partitions */
> struct device_node *of_node;
> struct device_node *elm_of_node;
> +
> + /* deprecated */
> + struct gpmc_nand_regs reg;
> };
> #endif
> --
> 2.1.4
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 00/27] memory: omap-gpmc: mtd: nand: Support GPMC NAND on non-OMAP platforms
Hi, On Fri, Sep 18, 2015 at 05:53:22PM +0300, Roger Quadros wrote: > Hi, > > We do a couple of things in this series which result in > cleaner device tree implementation, faster perfomance and > multi-platform support. As an added bonus we get new GPI/Interrupt pins > for use in the system. > > - Establish a custom interface between NAND and GPMC driver. This is > needed because all of the NAND registers sit in the GPMC register space. > Some bits like NAND IRQ are even shared with GPMC. > > - Remove NAND IRQ handling from omap-gpmc driver, share the GPMC IRQ > with the omap2-nand driver and handle NAND IRQ events in the NAND driver. > This causes performance increase when using prefetch-irq mode. > 30% increase in read, 17% increase in write in prefetch-irq mode. > > - Clean up device tree support so that omap-gpmc IP and the omap2 NAND > driver can be used on non-OMAP platforms. e.g. Keystone. > > - Implement GPIOCHIP + IRQCHIP for the GPMC WAITPINS. SoCs can contain > 2 to 4 of these and most of them would be unused otherwise. It also > allows a cleaner implementation of NAND Ready pin status for the NAND driver. > > - Implement GPIOlib based NAND ready pin checking for OMAP NAND driver. > > This series is available at > g...@github.com:rogerq/linux.git > in branch > for-v4.4/gpmc-v3 > > cheers, > -roger > > Changelog: > v3: > -Fixed and tested NAND using legacy boot on omap3-beagle. > -Support rising and falling edge interrupts on WAITpins. > -Update DT node of all gpmc users. The MTD stuff looks mostly good to me know. I've made all my comments for now, but I'm not sure how you're going to end up rebasing/splitting and what you're going to do with the irqchip removal, so I'll refrain from ack's for now. Hopefully I can either ack or merge v4. I brought it up on one other patch, but it's not really clear to me what the split is on board file vs. device tree handling, since you seem to have a combination of both (i.e., platform data that passes along device nodes). What's the plan on that? And of course, there's the question of how exactly to merge this, given the: (1) conflicts already existing in the MTD dev tree (2) this touches several trees, often in the same patch and (3) even if the patches were split out a little better into MTD and non-MTD stuff, I think there would still be dependencies such that we'd need at least 1 (probably 2) cross merges to get it all straight I'd be happy to hear your thoughts. Brian -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 18/27] mtd: nand: omap2: Implement NAND ready using gpiolib
(to be clear, this branch of discussion isn't directly regarding the TI
changes; we can handle any generic handling afterward, as long as we get
the DT binding right now)
On Tue, Oct 27, 2015 at 09:28:32AM +0100, Boris Brezillon wrote:
> On Mon, 26 Oct 2015 13:49:00 -0700
> Brian Norris wrote:
> > On Fri, Sep 18, 2015 at 05:53:40PM +0300, Roger Quadros wrote:
> > > @@ -1782,7 +1780,9 @@ static int omap_nand_probe(struct platform_device
> > > *pdev)
> > > info->reg = pdata->reg;
> > > info->of_node = pdata->of_node;
> > > info->ecc_opt = pdata->ecc_opt;
> > > - info->dev_ready = pdata->dev_ready;
> > > + if (pdata->dev_ready)
> > > + dev_info(&pdev->dev, "pdata->dev_ready is
> > > deprecated\n");
> > > +
> > > info->xfer_type = pdata->xfer_type;
> > > info->devsize = pdata->devsize;
> > > info->elm_of_node = pdata->elm_of_node;
> > > @@ -1815,6 +1815,13 @@ static int omap_nand_probe(struct platform_device
> > > *pdev)
> > > nand_chip->IO_ADDR_W = nand_chip->IO_ADDR_R;
> > > nand_chip->cmd_ctrl = omap_hwcontrol;
> > >
> > > + info->ready_gpiod = devm_gpiod_get_optional(&pdev->dev, "ready",
> > > + GPIOD_IN);
> >
> > Others have been looking at using GPIOs for the ready/busy pin too. At a
> > minimum, we need an updated DT binding doc for this, since I see you're
> > adding this via device tree in a later patch (I don't see any DT binding
> > patch for this; but I could just be overlooking it). It'd also be great
> > if this support was moved to nand_dt_init() so other platforms can
> > benefit, but I won't require that.
>
> Actually I started to work on a generic solution parsing the DT and
> creating CS, WP and RB gpios when they are provided, but I think it's a
> bit more complicated than just moving the rb-gpios parsing into
> nand_dt_init().
> First you'll need something to store your gpio_desc pointers, which
> means you'll have to allocate a table of gpio_desc pointers (or a table
> of struct embedding a gpio_desc pointer).
I'm not sure what you mean by table. It seems like we would just have a
few gpio_desc pointers in struct nand_chip, no?
> The other blocking point is that when nand_scan_ident() is called, the
> caller is supposed to have filled the ->dev_ready() or ->waitfunc()
> fields, and to choose how to implement it he may need to know
> which kind of RB handler should be used (this is the case in the sunxi
> driver, where the user can either use a GPIO or native R/B pin directly
> connected to the controller).
Right, I was thinking about this one.
> All this makes me think that maybe nand_dt_init() should be called
> separately or in a different helper (nand_init() ?) taking care of the
> basic nand_chip initializations/allocations without interacting with
> the NAND itself.
Yeah, I feel like there have been other good reasons for something like
this before, and we just have worked around them so far. Maybe something
more like the alloc/add split in many other subsystems? e.g.,
platform_device_{alloc,add}, spi_{alloc,add}_device. Now we might want
nand_alloc()?
On a slight tangent, it seems silly that nand_scan_tail() doesn't
register the MTD, but nand_release() unregisters it. That shouldn't be.
> Another solution would be to add an ->init() function to nand_chip
> and call it after the generic initialization has been done (but before
> NAND chip detection). This way the NAND controller driver could adapt
> some fields and parse controller specific properties.
That could work too, I guess.
> What do you think?
Brian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 11/27] mtd: nand: omap: Clean up device tree support
Hi Roger,
On Tue, Oct 06, 2015 at 01:35:48PM +0300, Roger Quadros wrote:
> Move NAND specific device tree parsing to NAND driver.
>
> The NAND controller node must have a compatible id, register space
> resource and interrupt resource.
>
> Signed-off-by: Roger Quadros
This one's going to need rebased, as there are some conflicting of_node
changes in l2-mtd.git.
> ---
> v4: Warn if using older incompatible DT i.e. compatible property not present
> in nand node.
>
> arch/arm/mach-omap2/gpmc-nand.c | 5 +-
> drivers/memory/omap-gpmc.c | 143
> +++
> drivers/mtd/nand/omap2.c | 136 +
> include/linux/platform_data/mtd-nand-omap2.h | 3 +-
> 4 files changed, 155 insertions(+), 132 deletions(-)
Also, this is going to be hard to manage across trees, as you touch
three drivers all at once. Is it not possible to split any of this apart
better?
>
> diff --git a/arch/arm/mach-omap2/gpmc-nand.c b/arch/arm/mach-omap2/gpmc-nand.c
> index ffe646a..e07ca27 100644
> --- a/arch/arm/mach-omap2/gpmc-nand.c
> +++ b/arch/arm/mach-omap2/gpmc-nand.c
> @@ -95,10 +95,7 @@ int gpmc_nand_init(struct omap_nand_platform_data
> *gpmc_nand_data,
> gpmc_nand_res[1].start = gpmc_get_irq();
>
> memset(&s, 0, sizeof(struct gpmc_settings));
> - if (gpmc_nand_data->of_node)
> - gpmc_read_settings_dt(gpmc_nand_data->of_node, &s);
> - else
> - gpmc_set_legacy(gpmc_nand_data, &s);
> + gpmc_set_legacy(gpmc_nand_data, &s);
>
> s.device_nand = true;
>
> diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c
> index e75226d..318c187 100644
> --- a/drivers/memory/omap-gpmc.c
> +++ b/drivers/memory/omap-gpmc.c
> @@ -29,7 +29,6 @@
> #include
> #include
> #include
> -#include
> #include
>
> #include
> @@ -1716,105 +1715,6 @@ static void __maybe_unused
> gpmc_read_timings_dt(struct device_node *np,
> of_property_read_bool(np, "gpmc,time-para-granularity");
> }
>
> -#if IS_ENABLED(CONFIG_MTD_NAND)
> -
> -static const char * const nand_xfer_types[] = {
> - [NAND_OMAP_PREFETCH_POLLED] = "prefetch-polled",
> - [NAND_OMAP_POLLED] = "polled",
> - [NAND_OMAP_PREFETCH_DMA]= "prefetch-dma",
> - [NAND_OMAP_PREFETCH_IRQ]= "prefetch-irq",
> -};
> -
> -static int gpmc_probe_nand_child(struct platform_device *pdev,
> - struct device_node *child)
> -{
> - u32 val;
> - const char *s;
> - struct gpmc_timings gpmc_t;
> - struct omap_nand_platform_data *gpmc_nand_data;
> -
> - if (of_property_read_u32(child, "reg", &val) < 0) {
> - dev_err(&pdev->dev, "%s has no 'reg' property\n",
> - child->full_name);
> - return -ENODEV;
> - }
> -
> - gpmc_nand_data = devm_kzalloc(&pdev->dev, sizeof(*gpmc_nand_data),
> - GFP_KERNEL);
> - if (!gpmc_nand_data)
> - return -ENOMEM;
> -
> - gpmc_nand_data->cs = val;
> - gpmc_nand_data->of_node = child;
> -
> - /* Detect availability of ELM module */
> - gpmc_nand_data->elm_of_node = of_parse_phandle(child, "ti,elm-id", 0);
> - if (gpmc_nand_data->elm_of_node == NULL)
> - gpmc_nand_data->elm_of_node =
> - of_parse_phandle(child, "elm_id", 0);
> -
> - /* select ecc-scheme for NAND */
> - if (of_property_read_string(child, "ti,nand-ecc-opt", &s)) {
> - pr_err("%s: ti,nand-ecc-opt not found\n", __func__);
> - return -ENODEV;
> - }
> -
> - if (!strcmp(s, "sw"))
> - gpmc_nand_data->ecc_opt = OMAP_ECC_HAM1_CODE_SW;
> - else if (!strcmp(s, "ham1") ||
> - !strcmp(s, "hw") || !strcmp(s, "hw-romcode"))
> - gpmc_nand_data->ecc_opt =
> - OMAP_ECC_HAM1_CODE_HW;
> - else if (!strcmp(s, "bch4"))
> - if (gpmc_nand_data->elm_of_node)
> - gpmc_nand_data->ecc_opt =
> - OMAP_ECC_BCH4_CODE_HW;
> - else
> - gpmc_nand_data->ecc_opt =
> - OMAP_ECC_BCH4_CODE_HW_DETECTION_SW;
> - else if (!strcmp(s, "bch8"))
> - if (gpmc_nand_data->elm_of_node)
> - gpmc_nand_data->ecc_opt =
> - OMAP_ECC_BCH8_CODE_HW;
> - else
> - gpmc_nand_data->ecc_opt =
> - OMAP_ECC_BCH8_CODE_HW_DETECTION_SW;
> - else if (!strcmp(s, "bch16"))
> - if (gpmc_nand_data->elm_of_node)
> - gpmc_nand_data->ecc_opt =
> - OMAP_ECC_BCH16_CODE_HW;
> - else
> - pr_err("%s: BCH16 requires ELM support\n", __func__);
> - else
> - pr_err("%s: ti,na
Re: [PATCH 2/3] mtd: brcmnand: Request and enable the clock if present
Hi Simon,
On Wed, Dec 02, 2015 at 11:42:44PM +, Simon Arlott wrote:
> Attempt to enable a clock named "nand" as some SoCs have a clock for the
> controller that needs to be enabled.
>
> Signed-off-by: Simon Arlott
> ---
> drivers/mtd/nand/brcmnand/brcmnand.c | 69
>
> 1 file changed, 54 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/mtd/nand/brcmnand/brcmnand.c
> b/drivers/mtd/nand/brcmnand/brcmnand.c
> index 2c8f67f..0a9cccf 100644
> --- a/drivers/mtd/nand/brcmnand/brcmnand.c
> +++ b/drivers/mtd/nand/brcmnand/brcmnand.c
> @@ -11,6 +11,7 @@
> * GNU General Public License for more details.
> */
>
> +#include
> #include
> #include
> #include
> @@ -122,6 +123,9 @@ struct brcmnand_controller {
> /* Some SoCs provide custom interrupt status register(s) */
> struct brcmnand_soc *soc;
>
> + /* Some SoCs have a gateable clock for the controller */
> + struct clk *clk;
> +
> int cmd_pending;
> booldma_pending;
> struct completion done;
> @@ -2136,10 +2140,24 @@ int brcmnand_probe(struct platform_device *pdev,
> struct brcmnand_soc *soc)
> if (IS_ERR(ctrl->nand_base))
> return PTR_ERR(ctrl->nand_base);
>
> + /* Enable clock before using NAND registers */
> + ctrl->clk = devm_clk_get(dev, "nand");
> + if (!IS_ERR(ctrl->clk)) {
> + ret = clk_prepare_enable(ctrl->clk);
> + if (ret)
> + return ret;
> + } else {
> + ret = PTR_ERR(ctrl->clk);
> + if (ret == -EPROBE_DEFER)
> + return ret;
> +
> + ctrl->clk = NULL;
> + }
> +
> /* Initialize NAND revision */
> ret = brcmnand_revision_init(ctrl);
> if (ret)
> - return ret;
> + goto err;
>
> /*
>* Most chips have this cache at a fixed offset within 'nand' block.
> @@ -2148,8 +2166,10 @@ int brcmnand_probe(struct platform_device *pdev,
> struct brcmnand_soc *soc)
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "nand-cache");
> if (res) {
> ctrl->nand_fc = devm_ioremap_resource(dev, res);
> - if (IS_ERR(ctrl->nand_fc))
> - return PTR_ERR(ctrl->nand_fc);
> + if (IS_ERR(ctrl->nand_fc)) {
> + ret = PTR_ERR(ctrl->nand_fc);
> + goto err;
> + }
> } else {
> ctrl->nand_fc = ctrl->nand_base +
> ctrl->reg_offsets[BRCMNAND_FC_BASE];
> @@ -2159,8 +2179,10 @@ int brcmnand_probe(struct platform_device *pdev,
> struct brcmnand_soc *soc)
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "flash-dma");
> if (res) {
> ctrl->flash_dma_base = devm_ioremap_resource(dev, res);
> - if (IS_ERR(ctrl->flash_dma_base))
> - return PTR_ERR(ctrl->flash_dma_base);
> + if (IS_ERR(ctrl->flash_dma_base)) {
> + ret = PTR_ERR(ctrl->flash_dma_base);
> + goto err;
> + }
>
> flash_dma_writel(ctrl, FLASH_DMA_MODE, 1); /* linked-list */
> flash_dma_writel(ctrl, FLASH_DMA_ERROR_STATUS, 0);
> @@ -2169,13 +2191,16 @@ int brcmnand_probe(struct platform_device *pdev,
> struct brcmnand_soc *soc)
> ctrl->dma_desc = dmam_alloc_coherent(dev,
>sizeof(*ctrl->dma_desc),
>&ctrl->dma_pa, GFP_KERNEL);
> - if (!ctrl->dma_desc)
> - return -ENOMEM;
> + if (!ctrl->dma_desc) {
> + ret = -ENOMEM;
> + goto err;
> + }
>
> ctrl->dma_irq = platform_get_irq(pdev, 1);
> if ((int)ctrl->dma_irq < 0) {
> dev_err(dev, "missing FLASH_DMA IRQ\n");
> - return -ENODEV;
> + ret = -ENODEV;
> + goto err;
> }
>
> ret = devm_request_irq(dev, ctrl->dma_irq,
> @@ -2184,7 +2209,7 @@ int brcmnand_probe(struct platform_device *pdev, struct
> brcmnand_soc *soc)
> if (ret < 0) {
> dev_err(dev, "can't allocate IRQ %d: error %d\n",
> ctrl->dma_irq, ret);
> - return ret;
> + goto err;
> }
>
> dev_info(dev, "enabling FLASH_DMA\n");
> @@ -2208,7 +2233,8 @@ int brcmnand_probe(struct platform_device *pdev, struct
> brcmnand_soc *soc)
> ctrl->irq = platform_get_irq(pdev, 0);
> if ((int)ctrl->irq < 0) {
> dev_err(dev, "no IRQ defined\n");
> - return -ENODEV;
> + ret = -ENODEV;
> + goto err;
> }
>
> /
Re: [PATCH (v6) 1/2] mtd: brcmnand: Add brcm,bcm63268-nand device tree binding
On Wed, Dec 02, 2015 at 08:34:38PM +, Simon Arlott wrote:
> On 02/12/15 20:21, Brian Norris wrote:
> > On Wed, Dec 02, 2015 at 08:12:32PM +, Simon Arlott wrote:
> >> On 02/12/15 20:00, Brian Norris wrote:
> >> > On Wed, Dec 02, 2015 at 07:41:07PM +, Simon Arlott wrote:
> >> >> I've created a bcm963268part driver so there won't need to be any
> >> >> partitions in DT for bcm63268.
> >> >
> >> > Just curious, do you plan to submit this driver? We're working on
> >>
> >> Yes, it's just the most recent one I've been working on. I still have
> >> USBH and IUDMA to submit
> >>
> >> > matching up partition parsers to flash devices via device tree
> >> > of_match_table's, so you could do something like this:
> >> >
> >> > nand0: nandcs@0 {
> >> > compatible = "brcm,nandcs";
> >> > ...
> >> >
> >> > partitions {
> >> > compatible = "brcm,bcm963268-partitions";
> >> > ...
> >> > };
> >> > };
> >>
> >> I modified brcmnand to look for a machine matching "brcm,bcm963268", but
> >
> > Like this?
> >
> > http://patchwork.ozlabs.org/patch/473180/
> >
> > I'd like to avoid that (hence the "Rejected" status).
>
> I exported default_mtd_part_types, copied it, and then added to it:
> + for (i = 0; i < nr_types; i++)
> + part_types[i] = default_mtd_part_types[i];
> +
> + /* Add partition type based on machine */
> + if (of_machine_is_compatible("brcm,bcm963268"))
> + part_types[i++] = "bcm963268part";
> + else
> + part_types[i++] = NULL;
> +
> + part_types[i++] = NULL;
OK, so even uglier :)
> >> that way is ok with me. Presumably "ofpart" defers to another matching
> >> partition parser?
> >
> > Yes, "ofpart" is for specifying the entire partition table in the device
> > tree as subnodes of either the flash node or of the flash's "partitions"
> > subnode. It's not the most flexible, but it does work generically.
> >
> >> Is there a patch for that method of parser detection available?
> >
> > I have something working here, but I haven't had time to finish cleaning
> > it up and submitting it. There's an older patch that works similarly,
> > though it has some deficiencies:
> >
> > http://patchwork.ozlabs.org/patch/475988/
> >
> > The main difference between that and my (yet-to-be-submitted) proposal
> > is that I'd like parsers to opt in by adding a proper of_match_table
> > with non-Linux-specific DT bindings, and then we can drop the
> > "linux,..." naming and make it a reasonably generic property.
>
> I'll submit my parser without any means of using it,
Sounds fine.
> let me know if you
> want me to work on a patch for a match table.
If you're interested, I may CC you on my patches for testing/review.
Brian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH (v6) 1/2] mtd: brcmnand: Add brcm,bcm63268-nand device tree binding
On Wed, Dec 02, 2015 at 12:21:27PM -0800, Brian Norris wrote: > On Wed, Dec 02, 2015 at 08:12:32PM +, Simon Arlott wrote: > > Is there a patch for that method of parser detection available? > > I have something working here, but I haven't had time to finish cleaning > it up and submitting it. There's an older patch that works similarly, > though it has some deficiencies: > > http://patchwork.ozlabs.org/patch/475988/ > > The main difference between that and my (yet-to-be-submitted) proposal > is that I'd like parsers to opt in by adding a proper of_match_table > with non-Linux-specific DT bindings, and then we can drop the > "linux,..." naming and make it a reasonably generic property. For other related work: Linus Walleij attempted something similar here: http://lists.infradead.org/pipermail/linux-mtd/2015-October/062899.html -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH (v6) 1/2] mtd: brcmnand: Add brcm,bcm63268-nand device tree binding
Hi Simon,
On Wed, Dec 02, 2015 at 08:12:32PM +, Simon Arlott wrote:
> On 02/12/15 20:00, Brian Norris wrote:
> > On Wed, Dec 02, 2015 at 07:41:07PM +, Simon Arlott wrote:
> >> I've created a bcm963268part driver so there won't need to be any
> >> partitions in DT for bcm63268.
> >
> > Just curious, do you plan to submit this driver? We're working on
>
> Yes, it's just the most recent one I've been working on. I still have
> USBH and IUDMA to submit
>
> > matching up partition parsers to flash devices via device tree
> > of_match_table's, so you could do something like this:
> >
> > nand0: nandcs@0 {
> > compatible = "brcm,nandcs";
> > ...
> >
> > partitions {
> > compatible = "brcm,bcm963268-partitions";
> > ...
> > };
> > };
>
> I modified brcmnand to look for a machine matching "brcm,bcm963268", but
Like this?
http://patchwork.ozlabs.org/patch/473180/
I'd like to avoid that (hence the "Rejected" status).
> that way is ok with me. Presumably "ofpart" defers to another matching
> partition parser?
Yes, "ofpart" is for specifying the entire partition table in the device
tree as subnodes of either the flash node or of the flash's "partitions"
subnode. It's not the most flexible, but it does work generically.
> Is there a patch for that method of parser detection available?
I have something working here, but I haven't had time to finish cleaning
it up and submitting it. There's an older patch that works similarly,
though it has some deficiencies:
http://patchwork.ozlabs.org/patch/475988/
The main difference between that and my (yet-to-be-submitted) proposal
is that I'd like parsers to opt in by adding a proper of_match_table
with non-Linux-specific DT bindings, and then we can drop the
"linux,..." naming and make it a reasonably generic property.
Regards,
Brian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH (v7) 2/2] mtd: brcmnand: Add support for the BCM63268
Hi,
On Wed, Dec 02, 2015 at 07:54:49PM +, Simon Arlott wrote:
> On 02/12/15 19:18, Brian Norris wrote:
> > On Wed, Nov 25, 2015 at 07:49:13PM +, Simon Arlott wrote:
> >> +static int bcm63268_nand_probe(struct platform_device *pdev)
> >> +{
> >> + struct device *dev = &pdev->dev;
> >> + struct bcm63268_nand_soc *priv;
> >> + struct brcmnand_soc *soc;
> >> + struct resource *res;
> >> + int ret;
> >> +
> >> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> >> + if (!priv)
> >> + return -ENOMEM;
> >> + soc = &priv->soc;
> >> +
> >> + res = platform_get_resource_byname(pdev,
> >> + IORESOURCE_MEM, "nand-intr-base");
> >> + if (!res)
> >> + return -EINVAL;
> >> +
> >> + priv->base = devm_ioremap_resource(dev, res);
> >> + if (IS_ERR(priv->base))
> >> + return PTR_ERR(priv->base);
> >> +
> >> + priv->clk = devm_clk_get(&pdev->dev, "nand");
> >> + if (IS_ERR(priv->clk))
> >> + return PTR_ERR(priv->clk);
> >
> > Perhaps we should put this clock handling in brcmnand.c? Just have it
> > treat the clock as optional (i.e., ignore errors except for
> > EPROBE_DEFER?), so we don't fail if no clock was provided? This could
> > help other platforms too, if they gain clock support.
>
> Unless most soc variants have clocks I'd prefer to leave it in this
> module.
I'm quite confident your SoC is not the only one with clocks.
> > If we do this, you'll want to document the clock in the common binding,
> > not the bcm63268-specific part.
> >
> > Also, could it help to disable/enable the clock during suspend/resume?
> > If you move it to brcmnand.c, this would also be pretty simple.
>
> Alternatively, it could proxy the brcmnand_pm_ops functions. I don't
> have any way to test suspend/resume.
OK, no need to add it now then. It can be added if/when it's needed.
> >> +
> >> + ret = clk_prepare_enable(priv->clk);
> >> + if (ret)
> >> + return ret;
> >> +
> >> + soc->ctlrdy_ack = bcm63268_nand_intc_ack;
> >> + soc->ctlrdy_set_enabled = bcm63268_nand_intc_set;
> >> +
> >> + /* Disable and ack all interrupts */
> >> + brcmnand_writel(0, priv->base + BCM63268_NAND_INT);
> >> + brcmnand_writel(BCM63268_NAND_STATUS_MASK,
> >> + priv->base + BCM63268_NAND_INT);
> >> +
> >> + ret = brcmnand_probe(pdev, soc);
> >> + if (ret)
> >> + clk_disable_unprepare(priv->clk);
> >> +
> >> + return ret;
> >> +}
> >> diff --git a/drivers/mtd/nand/brcmnand/brcmnand.c
> >> b/drivers/mtd/nand/brcmnand/brcmnand.c
> >> index 2c8f67f..5f26b8a 100644
> >> --- a/drivers/mtd/nand/brcmnand/brcmnand.c
> >> +++ b/drivers/mtd/nand/brcmnand/brcmnand.c
> >> @@ -2262,6 +2262,14 @@ int brcmnand_probe(struct platform_device *pdev,
> >> struct brcmnand_soc *soc)
> >> }
> >> EXPORT_SYMBOL_GPL(brcmnand_probe);
> >>
> >> +struct brcmnand_soc *brcmnand_get_socdata(struct platform_device *pdev)
> >> +{
> >> + struct brcmnand_controller *ctrl = dev_get_drvdata(&pdev->dev);
> >> +
> >> + return ctrl ? ctrl->soc : NULL;
> >> +}
> >> +EXPORT_SYMBOL_GPL(brcmnand_get_socdata);
> >
> > If you move the clk handling to the core brcmnand.c, then you won't need
> > this still.
>
> Would you prefer a clock name in the soc data structure that is used to
> call devm_clk_get()?
Not really. If we specify a clock name now, we can suggest other SoC's
to use the same name (where possible). So we wouldn't need any new code
or documentation, and we definitely don't need each snowflake sub-driver
to pass a new name.
Brian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH (v6) 1/2] mtd: brcmnand: Add brcm,bcm63268-nand device tree binding
Hi,
On Wed, Dec 02, 2015 at 07:41:07PM +, Simon Arlott wrote:
> >> + nand0: nandcs@0 {
> >> + compatible = "brcm,nandcs";
> >> + reg = <0>;
> >> + nand-on-flash-bbt;
> >> + nand-ecc-strength = <1>;
> >> + nand-ecc-step-size = <512>;
> >> +
> >> + #address-cells = <0>;
> >> + #size-cells = <0>;
> >
> > What are these {address,size}-cells for? If you need them for
> > partitioning, then those are wrong -- they shouldn't be zero. Maybe just
> > drop them? (I can cut them out when applying, if that's the only change
> > to make.)
>
> This is the correct way to do partitioning, there would be a
> "partitions" block with no address/size that contains the partitions as
> child nodes. [Documentation/devicetree/bindings/mtd/partition.txt]
That doc says nothing about {address,size}-cells = 0. When using the new
binding with a 'partitions' subnode, I'm not sure the address
translation specification really matters anyway; the flash space is a
new address space unrelated to the parents, and we don't do any
translation from the 'flash' node to the 'partitions' node. So you don't
need #{address,size}-cells in the flash node, but you do in the
'partitions' node.
> I think they're also implicit so you can just remove those two lines.
>From ePAPR: "The #address-cells and #size-cells properties are not
inherited from ancestors in the device tree. They shall be explicitly
defined."
But we don't want to define them. I'd drop them, at least from the
example, as they're not relevant.
> I've created a bcm963268part driver so there won't need to be any
> partitions in DT for bcm63268.
Just curious, do you plan to submit this driver? We're working on
matching up partition parsers to flash devices via device tree
of_match_table's, so you could do something like this:
nand0: nandcs@0 {
compatible = "brcm,nandcs";
...
partitions {
compatible = "brcm,bcm963268-partitions";
...
};
};
FYI.
> >> + };
> >> +};
Brian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH (v7) 2/2] mtd: brcmnand: Add support for the BCM63268
+ Broadcom list + Kamal
Hi Simon,
On Wed, Nov 25, 2015 at 07:49:13PM +, Simon Arlott wrote:
> The BCM63268 has a NAND interrupt register with combined status and enable
> registers. It also has a clock for the NAND controller that needs to be
> enabled.
>
> Set up the device by enabling the clock, disabling and acking all
> interrupts, then handle the CTRL_READY interrupt.
>
> Add a brcmnand_get_socdata() function so that bcm63268_nand can obtain its
> data and disable the clock when the device is removed.
>
> Signed-off-by: Simon Arlott
> ---
> Added EXPORT_SYMBOL_GPL(brcmnand_get_socdata)
>
> (As the brcmnand module must be loaded first its compatible string will
> apply to any existing devices before the soc-specific module can be
> loaded.)
What's this comment supposed to mean? The brcmnand module will not
directly probe any devices. It doesn't register any driver structs by
itself.
(BTW given that, it probably doesn't need its MODULE_DEVICE_TABLE.)
> drivers/mtd/nand/brcmnand/Makefile| 1 +
> drivers/mtd/nand/brcmnand/bcm63268_nand.c | 179
> ++
> drivers/mtd/nand/brcmnand/brcmnand.c | 8 ++
> drivers/mtd/nand/brcmnand/brcmnand.h | 1 +
> 4 files changed, 189 insertions(+)
> create mode 100644 drivers/mtd/nand/brcmnand/bcm63268_nand.c
>
> diff --git a/drivers/mtd/nand/brcmnand/Makefile
> b/drivers/mtd/nand/brcmnand/Makefile
> index 3b1fbfd..b83a9ae 100644
> --- a/drivers/mtd/nand/brcmnand/Makefile
> +++ b/drivers/mtd/nand/brcmnand/Makefile
> @@ -2,5 +2,6 @@
> # more specific iproc_nand.o, for instance
> obj-$(CONFIG_MTD_NAND_BRCMNAND) += iproc_nand.o
> obj-$(CONFIG_MTD_NAND_BRCMNAND) += bcm63138_nand.o
> +obj-$(CONFIG_MTD_NAND_BRCMNAND) += bcm63268_nand.o
> obj-$(CONFIG_MTD_NAND_BRCMNAND) += brcmstb_nand.o
> obj-$(CONFIG_MTD_NAND_BRCMNAND) += brcmnand.o
> diff --git a/drivers/mtd/nand/brcmnand/bcm63268_nand.c
> b/drivers/mtd/nand/brcmnand/bcm63268_nand.c
> new file mode 100644
> index 000..70ad907
> --- /dev/null
> +++ b/drivers/mtd/nand/brcmnand/bcm63268_nand.c
> @@ -0,0 +1,179 @@
> +/*
> + * Copyright 2015 Simon Arlott
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * Derived from bcm63138_nand.c:
> + * Copyright © 2015 Broadcom Corporation
> + *
> + * Derived from
> bcm963xx_4.12L.06B_consumer/shared/opensource/include/bcm963xx/63268_map_part.h:
> + * Copyright 2000-2010 Broadcom Corporation
> + *
> + * Derived from
> bcm963xx_4.12L.06B_consumer/shared/opensource/flash/nandflash.c:
> + * Copyright 2000-2010 Broadcom Corporation
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "brcmnand.h"
> +
> +struct bcm63268_nand_soc {
> + struct brcmnand_soc soc;
> + void __iomem *base;
> + struct clk *clk;
> +};
> +
> +#define BCM63268_NAND_INT0x00
> +#define BCM63268_NAND_STATUS_SHIFT 0
> +#define BCM63268_NAND_STATUS_MASK (0xfff << BCM63268_NAND_STATUS_SHIFT)
> +#define BCM63268_NAND_ENABLE_SHIFT 16
> +#define BCM63268_NAND_ENABLE_MASK (0x << BCM63268_NAND_ENABLE_SHIFT)
> +#define BCM63268_NAND_BASE_ADDR0 0x04
> +#define BCM63268_NAND_BASE_ADDR1 0x0c
> +
> +enum {
> + BCM63268_NP_READ= BIT(0),
> + BCM63268_BLOCK_ERASE= BIT(1),
> + BCM63268_COPY_BACK = BIT(2),
> + BCM63268_PAGE_PGM = BIT(3),
> + BCM63268_CTRL_READY = BIT(4),
> + BCM63268_DEV_RBPIN = BIT(5),
> + BCM63268_ECC_ERR_UNC= BIT(6),
> + BCM63268_ECC_ERR_CORR = BIT(7),
> +};
> +
> +static bool bcm63268_nand_intc_ack(struct brcmnand_soc *soc)
> +{
> + struct bcm63268_nand_soc *priv =
> + container_of(soc, struct bcm63268_nand_soc, soc);
> + void __iomem *mmio = priv->base + BCM63268_NAND_INT;
> + u32 val = brcmnand_readl(mmio);
> +
> + if (val & (BCM63268_CTRL_READY << BCM63268_NAND_STATUS_SHIFT)) {
> + /* Ack interrupt */
> + val &= ~BCM63268_NAND_STATUS_MASK;
> + val |= BCM63268_CTRL_READY << BCM63268_NAND_STATUS_SHIFT;
> + brcmnand_writel(val, mmio);
> + return true;
> + }
> +
> + return false;
> +}
> +
> +static void bcm63268_nand_intc_set(struct brcmnand_soc *soc, bool en)
> +{
> + struct bcm63268_nand_soc *priv =
> + container_of(soc, struct bcm63268_nand_soc, soc);
> + void __iomem *mmio = priv->base + BCM63268_NAND_INT;
> + u32 val = brcmnan
Re: [PATCH (v6) 1/2] mtd: brcmnand: Add brcm,bcm63268-nand device tree binding
+ Broadcom list + Kamal
On Tue, Nov 24, 2015 at 08:19:37PM -, Simon Arlott wrote:
> Add device tree binding for NAND on the BCM63268.
>
> The BCM63268 has a NAND interrupt register with combined status and enable
> registers.
>
> Signed-off-by: Simon Arlott
> ---
> .../devicetree/bindings/mtd/brcm,brcmnand.txt | 35
> ++
> 1 file changed, 35 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt
> b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt
> index 4ff7128..f2a71c8 100644
> --- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt
> +++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt
> @@ -72,6 +72,14 @@ we define additional 'compatible' properties and
> associated register resources w
> and enable registers
> - reg-names: (required) "nand-int-base"
>
> + * "brcm,nand-bcm63268"
> + - compatible: should contain "brcm,nand-bcm", "brcm,nand-bcm63268"
Looks like you're aiming to support bcm63168? Is bcm63268 the first
chip to include this style of register then? The numbering seems
backwards, but that may just be reality.
> + - reg: (required) the 'NAND_INTR_BASE' register range, with combined
> status
> + and enable registers, and boot address registers
> + - reg-names: (required) "nand-intr-base"
> + - clock: (required) reference to the clock for the NAND controller
> + - clock-names: (required) "nand"
> +
> * "brcm,nand-iproc"
> - reg: (required) the "IDM" register range, for interrupt enable and APB
> bus access endianness configuration, and the "EXT" register range,
> @@ -148,3 +156,30 @@ nand@f0442800 {
> };
> };
> };
> +
> +nand@1200 {
> + compatible = "brcm,nand-bcm63168", "brcm,nand-bcm63268",
> + "brcm,brcmnand-v4.0", "brcm,brcmnand";
> + reg = <0x1200 0x180>,
> + <0x1600 0x200>,
> + <0x10b0 0x10>;
> + reg-names = "nand", "nand-cache", "nand-intr-base";
> + interrupt-parent = <&periph_intc>;
> + interrupts = <50>;
> + clocks = <&periph_clk 20>;
> + clock-names = "nand";
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + nand0: nandcs@0 {
> + compatible = "brcm,nandcs";
> + reg = <0>;
> + nand-on-flash-bbt;
> + nand-ecc-strength = <1>;
> + nand-ecc-step-size = <512>;
> +
> + #address-cells = <0>;
> + #size-cells = <0>;
What are these {address,size}-cells for? If you need them for
partitioning, then those are wrong -- they shouldn't be zero. Maybe just
drop them? (I can cut them out when applying, if that's the only change
to make.)
> + };
> +};
Brian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 00/27] memory: omap-gpmc: mtd: nand: Support GPMC NAND on non-OMAP platforms
Hi Roger, On Wed, Dec 02, 2015 at 10:42:12AM +0530, Roger Quadros wrote: > On 02/12/15 08:56, Brian Norris wrote: > > On Tue, Dec 01, 2015 at 04:41:16PM +0200, Roger Quadros wrote: > >> On 30/11/15 21:54, Brian Norris wrote: > >>> But anyway, I'm not sure that completely answered my question. My > >>> question was whether you were removing the irqchip code solely for > >>> performance reasons, or are there others? > >> > >> Yes. Only for performance reasons. > > > > Hmm, that's not my favorite answer. I'd prefer that more analysis was > > done here before scrapping irqchip... > > I agree. We could retain the irqchip model till we have more satisfying > analysis. I won't insist, though if it's not too ugly/horrible to do so, I think that would make sense. I'll leave it as your call. Brian -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 00/27] memory: omap-gpmc: mtd: nand: Support GPMC NAND on non-OMAP platforms
On Wed, Dec 02, 2015 at 07:03:17AM -0800, Tony Lindgren wrote: > * Roger Quadros [151201 21:13]: > > On 02/12/15 08:56, Brian Norris wrote: > > > > > > I'll take another pass over your patch set, but if things are looking > > > better, how do you expect to merge this? There are significant portions > > > that touch at least 2 or 3 different subsystem trees, AFAICT. > > > > Tony could create an immutable branch with all the dts and memory changes. > > You could base the mtd changes on top of that? > > If we all agree on that it will be immutable Roger can set up the branch > with our acks and we can all merge it in as needed. I believe v4.4-rc1 > should work as a base for us all? There are oustanding comments about the NAND ready/busy GPIO naming in patch 18, which I'd like addressed. I'll re-review the rest before the end of the day (West Coast U.S.A.). I'm not sure my acks are worth much beyond the MTD stuff. Regarding branches, 4.4-rc1 is fine with me. Regards, Brian -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 00/27] memory: omap-gpmc: mtd: nand: Support GPMC NAND on non-OMAP platforms
Hi Roger, On Tue, Dec 01, 2015 at 04:41:16PM +0200, Roger Quadros wrote: > On 30/11/15 21:54, Brian Norris wrote: > > On Tue, Oct 27, 2015 at 11:37:03AM +0200, Roger Quadros wrote: > >> On 26/10/15 23:23, Brian Norris wrote: > >>> On Fri, Sep 18, 2015 at 05:53:22PM +0300, Roger Quadros wrote: > >>>> - Remove NAND IRQ handling from omap-gpmc driver, share the GPMC IRQ > >>>> with the omap2-nand driver and handle NAND IRQ events in the NAND driver. > >>>> This causes performance increase when using prefetch-irq mode. > >>>> 30% increase in read, 17% increase in write in prefetch-irq mode. > >>> > >>> Have you pinpointed the exact causes for the performance increase, or > >>> can you give an educated guess? AIUI, you're reducing the number of > >>> interrupts needed for NAND prefetch mode, but you're also removing a bit > >>> of abstraction and implementing hooks that look awfully like the > >>> existing abstractions: > >>> > >>> + int (*nand_irq_enable)(enum gpmc_nand_irq irq); > >>> + int (*nand_irq_disable)(enum gpmc_nand_irq irq); > >>> + void (*nand_irq_clear)(enum gpmc_nand_irq irq); > >>> + u32 (*nand_irq_status)(void); > >>> > >>> That's not really a problem if there's a good reason for them (brcmnand > >>> implements similar hooks because of quirks in the implementation of > >>> interrupts across various BRCM SoCs, and it's not worth writing irqchip > >>> drivers for those cases). I'm mainly curious for an explanation. > >> > >> I have both implementations with me. My guess is that the 20% performance > >> gain is due to absence of irqchip/irqdomain translation code. > >> I haven't investigated further though. > > > > I don't have much context for whether this makes sense or not. According > > to your tests, you're getting ~800K interrupts over ~15 seconds. So > > should you start noticing performance hits due to abstraction at 53K > > interrupts per second? > > Yes, this was my understanding. Am I computing wrong, or is that a pretty insane rate of interrupts? > > But anyway, I'm not sure that completely answered my question. My > > question was whether you were removing the irqchip code solely for > > performance reasons, or are there others? > > Yes. Only for performance reasons. Hmm, that's not my favorite answer. I'd prefer that more analysis was done here before scrapping irqchip... But maybe that's not too bad. It seems like your patch set overall is a net positive for disentangling some of arch/ and drivers/. I'll take another pass over your patch set, but if things are looking better, how do you expect to merge this? There are significant portions that touch at least 2 or 3 different subsystem trees, AFAICT. Brian -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 00/27] memory: omap-gpmc: mtd: nand: Support GPMC NAND on non-OMAP platforms
Hi Roger, On Tue, Oct 27, 2015 at 11:37:03AM +0200, Roger Quadros wrote: > On 26/10/15 23:23, Brian Norris wrote: > > I'm not too familiar with OMAP platforms, and I might have missed out on > > prior discussions/context, so please forgive if I'm asking silly or old > > questions here. > > No worries at all. > > > > > On Fri, Sep 18, 2015 at 05:53:22PM +0300, Roger Quadros wrote: > >> - Remove NAND IRQ handling from omap-gpmc driver, share the GPMC IRQ > >> with the omap2-nand driver and handle NAND IRQ events in the NAND driver. > >> This causes performance increase when using prefetch-irq mode. > >> 30% increase in read, 17% increase in write in prefetch-irq mode. > > > > Have you pinpointed the exact causes for the performance increase, or > > can you give an educated guess? AIUI, you're reducing the number of > > interrupts needed for NAND prefetch mode, but you're also removing a bit > > of abstraction and implementing hooks that look awfully like the > > existing abstractions: > > > > + int (*nand_irq_enable)(enum gpmc_nand_irq irq); > > + int (*nand_irq_disable)(enum gpmc_nand_irq irq); > > + void (*nand_irq_clear)(enum gpmc_nand_irq irq); > > + u32 (*nand_irq_status)(void); > > > > That's not really a problem if there's a good reason for them (brcmnand > > implements similar hooks because of quirks in the implementation of > > interrupts across various BRCM SoCs, and it's not worth writing irqchip > > drivers for those cases). I'm mainly curious for an explanation. > > I have both implementations with me. My guess is that the 20% performance > gain is due to absence of irqchip/irqdomain translation code. > I haven't investigated further though. I don't have much context for whether this makes sense or not. According to your tests, you're getting ~800K interrupts over ~15 seconds. So should you start noticing performance hits due to abstraction at 53K interrupts per second? But anyway, I'm not sure that completely answered my question. My question was whether you were removing the irqchip code solely for performance reasons, or are there others? > Another concern I have is that I'm not using any locking around > gpmc_nand_irq_enable/disable(). Could this pose problems in multiple NAND > use cases? My understanding is that it should not as the controller access > is serialized between multiple NAND chips. Right, if you're touching just a NAND interrupt, and it's only used by a single instance of this NAND controller, then the NAND controller serialization code will handle this for you. > However I do need to add some locking as the GPMC_IRQENABLE register is shared > between NAND and GPMC driver. > > NOTE: We are not using prefetch-irq mode for any of the OMAP boards because > of lesser performance than prefetch-polled mode. So if the less performance > for an unused mode is a lesser concern compared to cleaner code then > I can resend this with the irqdomain implementation. > > Below are performance logs of irqdomain vs hooks. > > -- > cheers, > -roger > > test logs. [snip] Brian -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 1/2] mtd: mtk-nor: mtk serial flash controller driver
On Wed, Nov 18, 2015 at 11:30:02AM +0800, Bayi Cheng wrote: > add spi nor flash driver for mediatek controller > > Signed-off-by: Bayi Cheng > --- > drivers/mtd/spi-nor/Kconfig | 7 + > drivers/mtd/spi-nor/Makefile | 1 + > drivers/mtd/spi-nor/mtk-quadspi.c | 486 > ++ > 3 files changed, 494 insertions(+) > create mode 100644 drivers/mtd/spi-nor/mtk-quadspi.c Looks good now. Thanks! Applied to l2-mtd.git, with a trivial whitespace fixup. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 3/7] mtd: fsl-quadspi: Support both 24- and 32-bit addressed commands.
Cory and Han,
Did this series get dropped on the floor? I recall Han arguing
previously that this controller is always used with two identical chips.
But apparently that is not the case.
If this request is not truly dead, I'd appreciate it if Han could
review/test.
Brian
On Wed, Jul 08, 2015 at 04:21:17PM -0400, 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
> ---
> drivers/mtd/spi-nor/fsl-quadspi.c | 116
> --
> 1 file changed, 60 insertions(+), 56 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c
> b/drivers/mtd/spi-nor/fsl-quadspi.c
> index 52a872f..4b8038b 100644
> --- a/drivers/mtd/spi-nor/fsl-quadspi.c
> +++ b/drivers/mtd/spi-nor/fsl-quadspi.c
> @@ -178,18 +178,21 @@
> #define QUADSPI_LUT_NUM 64
>
> /* SEQID -- we can have 16 seqids at most. */
> -#define SEQID_QUAD_READ 0
> -#define SEQID_WREN 1
> -#define SEQID_WRDI 2
> -#define SEQID_RDSR 3
> -#define SEQID_SE 4
> -#define SEQID_CHIP_ERASE 5
> -#define SEQID_PP 6
> -#define SEQID_RDID 7
> -#define SEQID_WRSR 8
> -#define SEQID_RDCR 9
> -#define SEQID_EN4B 10
> -#define SEQID_BRWR 11
> +#define SEQID_QUAD_READ_24 0
> +#define SEQID_QUAD_READ_32 1
> +#define SEQID_WREN 2
> +#define SEQID_WRDI 3
> +#define SEQID_RDSR 4
> +#define SEQID_SE_24 5
> +#define SEQID_SE_32 5
> +#define SEQID_CHIP_ERASE 7
> +#define SEQID_PP_24 8
> +#define SEQID_PP_32 8
> +#define SEQID_RDID 9
> +#define SEQID_WRSR 10
> +#define SEQID_RDCR 11
> +#define SEQID_EN4B 12
> +#define SEQID_BRWR 13
>
> enum fsl_qspi_devtype {
> FSL_QUADSPI_VYBRID,
> @@ -287,7 +290,6 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
> void __iomem *base = q->iobase;
> int rxfifo = q->devtype_data->rxfifo;
> u32 lut_base;
> - u8 cmd, addrlen, dummy;
> int i;
>
> fsl_qspi_unlock_lut(q);
> @@ -297,22 +299,16 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
> writel(0, base + QUADSPI_LUT_BASE + i * 4);
>
> /* Quad Read */
> - lut_base = SEQID_QUAD_READ * 4;
> -
> - if (q->nor_size <= SZ_16M) {
> - cmd = SPINOR_OP_READ_1_1_4;
> - addrlen = ADDR24BIT;
> - dummy = 8;
> - } else {
> - /* use the 4-byte address */
> - cmd = SPINOR_OP_READ_1_1_4;
> - addrlen = ADDR32BIT;
> - dummy = 8;
> - }
> + lut_base = SEQID_QUAD_READ_24 * 4;
> + writel(LUT0(CMD, PAD1, SPINOR_OP_READ_1_1_4) | LUT1(ADDR, PAD1,
> ADDR24BIT),
> + base + QUADSPI_LUT(lut_base));
> + writel(LUT0(DUMMY, PAD1, 8) | LUT1(READ, PAD4, rxfifo),
> + base + QUADSPI_LUT(lut_base + 1));
>
> - writel(LUT0(CMD, PAD1, cmd) | LUT1(ADDR, PAD1, addrlen),
> + lut_base = SEQID_QUAD_READ_32 * 4;
> + writel(LUT0(CMD, PAD1, SPINOR_OP_READ_1_1_4) | LUT1(ADDR, PAD1,
> ADDR32BIT),
> base + QUADSPI_LUT(lut_base));
> - writel(LUT0(DUMMY, PAD1, dummy) | LUT1(READ, PAD4, rxfifo),
> + writel(LUT0(DUMMY, PAD1, 8) | LUT1(READ, PAD4, rxfifo),
> base + QUADSPI_LUT(lut_base + 1));
>
> /* Write enable */
> @@ -320,18 +316,13 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
> writel(LUT0(CMD, PAD1, SPINOR_OP_WREN), base + QUADSPI_LUT(lut_base));
>
> /* Page Program */
> - lut_base = SEQID_PP * 4;
> -
> - if (q->nor_size <= SZ_16M) {
> - cmd = SPINOR_OP_PP;
> - addrlen = ADDR24BIT;
> - } else {
> - /* use the 4-byte address */
> - cmd = SPINOR_OP_PP;
> - addrlen = ADDR32BIT;
> - }
> + lut_base = SEQID_PP_24 * 4;
> + writel(LUT0(CMD, PAD1, SPINOR_OP_PP) | LUT1(ADDR, PAD1, ADDR24BIT),
> + base + QUADSPI_LUT(lut_base));
> + writel(LUT0(WRITE, PAD1, 0), base + QUADSPI_LUT(lut_base + 1));
>
> - writel(LUT0(CMD, PAD1, cmd) | LUT1(ADDR, PAD1, addrlen),
> + lut_base = SEQID_PP_32 * 4;
> + writel(LUT0(CMD, PAD1, SPINOR_OP_PP) | LUT1(ADDR, PAD1, ADDR32BIT),
> base + QUADSPI_LUT(lut_base));
> writel(LUT0(WRITE, PAD1, 0), base + QUADSPI_LUT(lut_base + 1));
>
> @@ -341,18 +332,12 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
> base + QUADSPI_LUT(lut_base));
>
> /* Erase a sector */
Re: [PATCH 0/3] mtd: m25p80: fix some module and documentation issues
On Mon, Nov 16, 2015 at 02:34:49PM -0800, Brian Norris wrote: > Hi, > > There were a few mistakes and improvements pointed out at various points in > this thread, subject: > > spi: OF module autoloading is still broken > (was: Re: m25p80: Commit "allow arbitrary OF matching for "jedec,spi-nor"" > breaks module autoloading) > > http://lists.infradead.org/pipermail/linux-mtd/2015-October/062369.html > https://lkml.org/lkml/2015/11/12/574 > > That thread covers some other interesting issues that are still not solved, > but > let's fix the ones we can here in MTD right now. > > Regards, > Brian Pushed all to l2-mtd.git -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] doc: dt: mtd: stop referring to driver code for spi-nor IDs
Hi, On Tue, Nov 17, 2015 at 11:04:55AM -0300, Javier Martinez Canillas wrote: > On 11/16/2015 07:34 PM, Brian Norris wrote: > > diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt > > b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt > > index 2bee68103b01..2c91c03e7eb0 100644 > > --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt > > +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt > > @@ -1,15 +1,61 @@ > > -* MTD SPI driver for ST M25Pxx (and similar) serial flash chips > > +* SPI NOR flash: ST M25Pxx (and similar) serial flash chips > > > > Required properties: > > - #address-cells, #size-cells : Must be present if the device has sub-nodes > >representing partitions. > > - compatible : May include a device-specific string consisting of the > > - manufacturer and name of the chip. Bear in mind the DT > > binding > > - is not Linux-only, but in case of Linux, see the "m25p_ids" > > - table in drivers/mtd/devices/m25p80.c for the list of > > supported > > - chips. > > + manufacturer and name of the chip. A list of supported chip > > + names follows. > > Here says that the compatible string consists of manufacturer and name... > > > Must also include "jedec,spi-nor" for any SPI NOR flash > > that can > > be identified by the JEDEC READ ID opcode (0x9F). > > + > > + Supported chip names: > > + at25df321a > > + at25df641 [...] > + > > ... but the entries in the list don't have a manufacturer. I know this is > due backward compatibility because as we discussed in the thread mentioned > in the cover letter, the SPI core didn't use the manufacturer and that > implementation detail leaked into the DTBs. > > But I think that either only the correct list with vendor should be added > to the DT binding docs (but make sure that backward compatibility in the > driver and SPI core is maintained) or both the wrong and correct list should > be documented and the former be marked as deprecated. First, note that the list says "Supported chip *names*" (not "Supported compatible values"). It does not attempt to specify the full compatible value, and that's intentional. Second, I believe it is hard to determine after-the-fact what all the reasonable pairings of vendors might be. For some of these parts, various companies have produced parts under the same chip ID -- usually because one company bought another. For most chips though, this probably isn't a problem, so I could probably pick reasonable vendor pairings. IOW, there isn't just "a wrong" and "a correct" list; there's a (probably?) correct list and an enormous space of "I don't know what people might have put here" list. It's nearly unbounded, but even a reasonable list might get pretty large. So in practice, we'd essentially be sacrificing completeness for...what reason? > > + The following chip names have been used historically to > > + designate quirky versions of flash chips that do not > > support the > > + JEDEC READ ID opcode (0x9F): > > + m25p05-nonjedec > > + m25p10-nonjedec > > + m25p20-nonjedec > > + m25p40-nonjedec > > + m25p80-nonjedec > > + m25p16-nonjedec > > + m25p32-nonjedec > > + m25p64-nonjedec > > + m25p128-nonjedec > > + > > Same here, I would prefer if the DT binding make it clear that not having > a vendor is wrong and is only documented to maintain backward compatibility. The doc never says anything about not including the vendor. It says "May include a device-specific string consisting of the manufacturer and name of the chip" and it lists the chip names. So if someone is actually following the documentation, they will include a vendor. The vendor names are not listed because they're really not relevant to the implementation. But I could try to include them. > > - reg : Chip-Select number > > - spi-max-frequency : Maximum frequency of the SPI bus the chip can > > operate at > > > > So, what makes sense? I can make a separate list of vendors (my preference), or even try to pair up vendors with chip names (even if it's sometimes an N:1 relationship). But I don't see that really buying us much, since the implementation never has (and probably never will) enforce this. What do you think? Regards, Brian -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
device_node lifetime (was: Re: [PATCH 1/7] phy: brcmstb-sata: add missing of_node_put)
(changing subject, add devicetree@vger.kernel.org) On Tue, Nov 17, 2015 at 11:33:25PM +0100, Julia Lawall wrote: > On Tue, 17 Nov 2015, Brian Norris wrote: > > On Tue, Nov 17, 2015 at 06:48:39PM +0100, Julia Lawall wrote: > > > Is this something that should be checked for elsewhere? > > > > I expect the same sort of problem shows up plenty of other places. I > > don't think many people use CONFIG_OF_DYNAMIC, so the effects of these > > failures probably aren't felt by many. > > I tried the following semantic patch: > > @@ > struct device_node *e; > expression e1; > identifier fld; > @@ > > ... when != of_node_get(...) > *(<+...e1->fld...+>) = e > ... when != of_node_get(...) > return e1; > > basically, this says that a structure field is initilized to a device node > value, the structure is returned by the containing function, and the > containing function contains no of_node_get at all. Certainly this is > quite constrained, but it does produce a number of examples. > > I looked at a few of them: > > drivers/clk/ingenic/cgu.c, ingenic_cgu_new > clk/pistachio/clk.c, pistachio_clk_alloc_provider It looks like the clock core (drivers/clk/clk.c) initially grabs the clk provider node in of_clk_init(), then drops it after it's initialized, but most of these providers use of_clk_add_provider(), which seems to manage the device_node lifetime for the user. So I think these are OK. > drivers/mfd/syscon.c, of_syscon_register This one looks potentially suspect. Syscon nodes aren't usually directly managed by a single driver, and the device_node pointer is used for lookups later...so I think it should keep a kref, and it doesn't. > drivers/of/pdt.c, function of_pdt_create_node Not real sure about this one. > Any idea whether these need of_node_get? In all cases the device node > value comes in as a parameter. I'm really not an expert on this stuff. I just saw a potential problem that I happen to be looking at in other subsystems, and I wanted to know what others thought. I think this discussion should include the DT folks and the subsystems in question. For one, I'm as interested as anyone in getting this todo clarified: Documentation/devicetree/todo.txt - Document node lifecycle for CONFIG_OF_DYNAMIC Regards, Brian -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/5] spi: introduce mmap read support for spi flash devices
Hi, On Tue, Nov 17, 2015 at 12:02:29PM +0530, Vignesh R wrote: > On 11/13/2015 09:35 PM, Cyrille Pitchen wrote: > > > > In September I've sent a series of patches to enhance the support of QSPI > > flash > > memories. Patch 4 was dedicated to the m25p80 driver and set the > > rx_nbits / tx_nbits fields of spi_transfer struct(s) in order to configure > > the > > number of I/O lines independently for the opcode, address and data parts. > > The work was done for m25p80_read() but also for _read_reg(), _write_reg() > > and > > _write(). > > The patched m25p80 driver was then tested with an at25 memory to check non- > > regression. > > > > This series of patches also added 4 enum spi_protocol fields inside struct > > spi_nor so the spi-nor framework can tell the (Q)SPI controller driver what > > SPI > > protocol should be use for erase, read, write and register read/write > > operations, depending on the memory manufacturer and the command opcode. > > This was done to better support Micron, Spansion and Macronix QSPI memories. > > > > I have tested the series with Micron QSPI memories and Atmel QSPI controller > > and I guess Marek also tested it on his side with Spansion QSPI memories and > > another QSPI controller. > > > > So if it can help other developers to develop QSPI controller drivers, the > > series is still available there: > > > > for the whole series: > > http://lists.infradead.org/pipermail/linux-arm-kernel/2015-September/371170.html > > > > for patch 4 (depends on patch 2 for enum spi_protocol): > > http://lists.infradead.org/pipermail/linux-arm-kernel/2015-September/371173.html > > > > Should I rebase my next version on top of above patches by Cyrille or > shall I post on top of 4.4-rc1? I'm sorry to say I really haven't had the time to review that properly. I'm also not sure it is a true dependency for your series, as you're tackling different pieces of the puzzle. So it's mostly just a conflict, not a real direct help. So unless I'm misunderstanding, I'd suggest submitting MTD stuff against the latest l2-mtd.git (or linux-next.git; l2-mtd.git is included there), and I'll let you know if conflicts come up that need fixing. Brian -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [v4 00/10] add support SATA for BMIPS_GENERIC
On Fri, Oct 30, 2015 at 11:01:14PM +0900, Jaedon Shin wrote:
> Hi all,
>
> This patch series add support SATA for BMIPS_GENERIC.
>
> Changes in v4:
> - remove unused properties from bcm{7425,7342,7362}.dtsi
>
> Changes in v3:
> - fix typo quirk instead of quick
> - disable NCQ before initialzing SATA controller endianness
> - fix misnomer controlling phy interface
> - remove brcm,broken-ncq and brcm,broken-phy properties from devicetree
> - use compatible string for quirks
> - use list for compatible strings
> - add "Acked-by:" tags
>
> Changes in v2:
> - adds quirk for ncq
> - adds quirk for phy interface control
> - remove unused definitions in ahci_brcmstb
> - combines compatible string
For the drivers portions (including patch 2, if you fix the error I
pointed out):
Acked-by: Brian Norris
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [v4 02/10] ata: ahci_brcmstb: add quirk for broken ncq
Hi,
On Fri, Oct 30, 2015 at 11:01:16PM +0900, Jaedon Shin wrote:
> Add quirk for broken ncq. Some chipsets (eg. BCM7349A0, BCM7445A0,
> BCM7445B0, and all 40nm chipsets including BCM7425) need a workaround
> disabling NCQ.
>
> Signed-off-by: Jaedon Shin
> ---
> drivers/ata/ahci_brcmstb.c | 46
> ++
> 1 file changed, 46 insertions(+)
>
> diff --git a/drivers/ata/ahci_brcmstb.c b/drivers/ata/ahci_brcmstb.c
> index 73e3b0b2a3c2..194aeda8f14d 100644
> --- a/drivers/ata/ahci_brcmstb.c
> +++ b/drivers/ata/ahci_brcmstb.c
> @@ -69,10 +69,15 @@
> (DATA_ENDIAN << DMADESC_ENDIAN_SHIFT) | \
> (MMIO_ENDIAN << MMIO_ENDIAN_SHIFT))
>
> +enum brcm_ahci_quirks {
> + BRCM_AHCI_QUIRK_NONCQ = BIT(0),
> +};
> +
> struct brcm_ahci_priv {
> struct device *dev;
> void __iomem *top_ctrl;
> u32 port_mask;
> + u32 quirks;
> };
>
> static const struct ata_port_info ahci_brcm_port_info = {
> @@ -202,6 +207,42 @@ static u32 brcm_ahci_get_portmask(struct platform_device
> *pdev,
> return impl;
> }
>
> +static void brcm_sata_quirks(struct platform_device *pdev,
> + struct brcm_ahci_priv *priv)
> +{
> + if (priv->quirks & BRCM_AHCI_QUIRK_NONCQ) {
> + void __iomem *ctrl = priv->top_ctrl + SATA_TOP_CTRL_BUS_CTRL;
> + void __iomem *ahci;
> + struct resource *res;
> + u32 reg;
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> +"ahci");
> + ahci = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(ahci))
> + return;
> +
> + reg = brcm_sata_readreg(ctrl);
> + reg |= OVERRIDE_HWINIT;
> + brcm_sata_writereg(reg, ctrl);
> +
> + /* Clear out the NCQ bit so the AHCI driver will not issue
> + * FPDMA/NCQ commands.
> + */
> + reg = readl(ahci + HOST_CAP);
> + reg &= ~HOST_CAP_NCQ;
> + writel(reg, ahci + HOST_CAP);
You're using readl()/writel() to access the AHCI block, but...
> +
> + reg = brcm_sata_readreg(ctrl);
> + reg &= ~OVERRIDE_HWINIT;
> + brcm_sata_writereg(reg, ctrl);
> +
> + devm_iounmap(&pdev->dev, ahci);
> + devm_release_mem_region(&pdev->dev, res->start,
> + resource_size(res));
> + }
> +}
> +
> static void brcm_sata_init(struct brcm_ahci_priv *priv)
> {
> /* Configure endianness */
> @@ -256,6 +297,11 @@ static int brcm_ahci_probe(struct platform_device *pdev)
> if (IS_ERR(priv->top_ctrl))
> return PTR_ERR(priv->top_ctrl);
>
> + if (of_device_is_compatible(dev->of_node, "brcm,bcm7425-ahci"))
> + priv->quirks |= BRCM_AHCI_QUIRK_NONCQ;
> +
> + brcm_sata_quirks(pdev, priv);
> +
> brcm_sata_init(priv);
...the MMIO endianness is only configured in brcm_sata_init(). You won't
see this problem on ARM LE, but you should on MIPS BE. Maybe
brcm_sata_quirks() should be after brcm_sata_init()?
>
> priv->port_mask = brcm_ahci_get_portmask(pdev, priv);
Brian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] doc: dt: mtd: stop referring to driver code for spi-nor IDs
Pull the supported chip names from drivers/mtd/devices/m25p80.c and stop pointing readers to Linux code. Also (although I see this habit repeated throughout the Documentation/devicetree/bindings/ tree), stop using the title "driver" in this file, when we're trying explicitly to describe hardware, not software. Signed-off-by: Brian Norris Cc: --- .../devicetree/bindings/mtd/jedec,spi-nor.txt | 56 -- 1 file changed, 51 insertions(+), 5 deletions(-) diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt index 2bee68103b01..2c91c03e7eb0 100644 --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt @@ -1,15 +1,61 @@ -* MTD SPI driver for ST M25Pxx (and similar) serial flash chips +* SPI NOR flash: ST M25Pxx (and similar) serial flash chips Required properties: - #address-cells, #size-cells : Must be present if the device has sub-nodes representing partitions. - compatible : May include a device-specific string consisting of the - manufacturer and name of the chip. Bear in mind the DT binding - is not Linux-only, but in case of Linux, see the "m25p_ids" - table in drivers/mtd/devices/m25p80.c for the list of supported - chips. + manufacturer and name of the chip. A list of supported chip + names follows. Must also include "jedec,spi-nor" for any SPI NOR flash that can be identified by the JEDEC READ ID opcode (0x9F). + + Supported chip names: + at25df321a + at25df641 + at26df081a + mr25h256 + mx25l4005a + mx25l1606e + mx25l6405d + mx25l12805d + mx25l25635e + n25q064 + n25q128a11 + n25q128a13 + n25q512a + s25fl256s1 + s25fl512s + s25sl12801 + s25fl008k + s25fl064k + sst25vf040b + m25p40 + m25p80 + m25p16 + m25p32 + m25p64 + m25p128 + w25x80 + w25x32 + w25q32 + w25q32dw + w25q80bl + w25q128 + w25q256 + + The following chip names have been used historically to + designate quirky versions of flash chips that do not support the + JEDEC READ ID opcode (0x9F): + m25p05-nonjedec + m25p10-nonjedec + m25p20-nonjedec + m25p40-nonjedec + m25p80-nonjedec + m25p16-nonjedec + m25p32-nonjedec + m25p64-nonjedec + m25p128-nonjedec + - reg : Chip-Select number - spi-max-frequency : Maximum frequency of the SPI bus the chip can operate at -- 2.6.0.rc2.230.g3dd15c0 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/3] mtd: m25p80: fix some module and documentation issues
Hi, There were a few mistakes and improvements pointed out at various points in this thread, subject: spi: OF module autoloading is still broken (was: Re: m25p80: Commit "allow arbitrary OF matching for "jedec,spi-nor"" breaks module autoloading) http://lists.infradead.org/pipermail/linux-mtd/2015-October/062369.html https://lkml.org/lkml/2015/11/12/574 That thread covers some other interesting issues that are still not solved, but let's fix the ones we can here in MTD right now. Regards, Brian Brian Norris (3): mtd: m25p80: fix module autoloading for "jedec,spi-nor" and "spi-nor" mtd: m25p80: replace leftover "nor-jedec" with "spi-nor" in comments doc: dt: mtd: stop referring to driver code for spi-nor IDs .../devicetree/bindings/mtd/jedec,spi-nor.txt | 56 -- drivers/mtd/devices/m25p80.c | 17 +-- 2 files changed, 64 insertions(+), 9 deletions(-) -- 2.6.0.rc2.230.g3dd15c0 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] mtd: create a partition type device tree binding
On Sat, Nov 14, 2015 at 11:46:59AM +0100, Linus Walleij wrote:
> On Fri, Nov 13, 2015 at 11:00 PM, Brian Norris
> wrote:
>
> (...)
> > (2) we should define a comptible property for the hard-coded
> > partitioning case (e.g., compatible = "partitions")
> (...)
> > If we went with option (2), then ofpart.c could just check only for
> > 'compatible = "partitions"' (or similar), and if not found bail out.
>
> So this "hard-coded partitioning case" the case is where all partitions
> are defined in the device tree as described in
> Documentation/devicetree/bindings/mtd/partition.txt ?
Right.
> Or is it a way to indicate that this array
> static const char * const part_probe_types_def[] = {
> "cmdlinepart", "RedBoot", "ofpart", "ofoldpart", NULL };
> in physmap_of.c should be used?
No. At this point, I would consider that to be a legacy method. We still
have to support this for many cases (including the non-DT case; but
hopefully we can do better than that soon), and that would be an option.
> Sorry if I don't fully follow, I'm a bit stupid when it comes to the MTD
> helicopter view of the situation :(
Yeah...sorry if I wasn't too clear. And I definitely don't blame you for
not understanding the mess that MTD often is :(
> > I think option (2) makes more sense. But it would require an update to
> > the binding and code for 4.4, since [1] was only introduced during this
> > release cycle.
>
> Does that mean all old device trees that specify no compatible
> string all of a sudden stop working? We can't break the DT ABI, so I
> guess not.
No, that's not what I was intending. The binding before commmit
fe2585e9c29a ("doc: dt: mtd: support partitions in a special
'partitions' subnode") should still stay working as-is. That is, we
don't mess with the way things worked for anything that doesn't have a
'partitions' subnode. But now that we have a 'partitions' subnode (in
4.4-rc1), I'm just suggesting that we enforce this always have a
compatible property, so we can be more clear on the difference between:
partitions {
// do I have a
// compatible = "partitons";
// here?
partition@0 {
label = "foo-partition";
reg = <0 0x10>;
};
};
and
partitions {
compatible = "arm,arm-flash-structure";
subnode {
// what if we need something here eventually?
};
};
This would require some modifications to partitions.txt and to
drivers/mtd/ofpart.c.
> A bit confused here, I can't really see what I should do with the patch...
Hopefully that cleared up a bit? The code changes for my suggestion
would just be something like this, I think. (Not tested in any way.)
diff --git a/drivers/mtd/ofpart.c b/drivers/mtd/ofpart.c
index 669c3452f278..6811bc5440a4 100644
--- a/drivers/mtd/ofpart.c
+++ b/drivers/mtd/ofpart.c
@@ -50,6 +50,10 @@ static int parse_ofpart_partitions(struct mtd_info *master,
master->name, mtd_node->full_name);
ofpart_node = mtd_node;
dedicated = false;
+ } else {
+ /* The "partitions" subnode may belong to some other parser */
+ if (!of_device_is_compatible(ofpart_node, "partitions"))
+ return 0;
}
/* First count the subnodes */
I was just bringing this up for discussion, since it's related to
your/Rob's new proposal. I'll send a proper (and tested) patch, along
with a doc update, if that looks reasonable.
Brian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] mtd: create a partition type device tree binding
On Fri, Nov 06, 2015 at 08:13:13AM -0600, Rob Herring wrote:
> Since we now have partitions contained in a sub node, how about using
> compatible for that sub node instead.
I see that Linus and I spoke up in agreement on this one.
I took a little look at adding of_match_table support to the core MTD
partitioning code (not sure if that's duplicating anything Linus was
attempting on his own?), and I'm observing that there's potential for
conflict with the new binding [1]. If we're going to start overloading
the 'partitions' node to support other partitioning types via
'compatible' property, then we either need to:
(1) go back to specifying that full ofpart specifications must have
*no* compatible property or
(2) we should define a comptible property for the hard-coded
partitioning case (e.g., compatible = "partitions")
IOW, I could imagine a new partition parser that needs a DT like this:
flash@xxx {
compatible = "vendor,flash-type";
...
partitions {
compatible = "some-new-partition-parser";
...
subnode {
// "some-new-partition-parser" might
// need to put something here
};
};
};
But currently, the binding would say that 'subnode' must be a partition,
even if it's really something else auxiliary to
"some-new-partition-parser" [2].
If we went with option (1), then we'd just have ofpart.c see that
'partitions' has a compatible property and bail out. That seems kinda
hacky.
If we went with option (2), then ofpart.c could just check only for
'compatible = "partitions"' (or similar), and if not found bail out.
I think option (2) makes more sense. But it would require an update to
the binding and code for 4.4, since [1] was only introduced during this
release cycle.
Brian
[1] fe2585e9c29a ("doc: dt: mtd: support partitions in a special 'partitions'
subnode")
[2] Possibilities: something relevant to partition splitting. See some
previous work, which I haven't gotten around to fully addressing yet,
but can hopefully be rolled into this work:
http://patchwork.ozlabs.org/patch/476373/
http://patchwork.ozlabs.org/patch/473364/
http://patchwork.ozlabs.org/patch/475988/
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 1/3] doc: dt: add documentation for Mediatek spi-nor controller
On Fri, Nov 13, 2015 at 03:20:45PM +0800, Bayi Cheng wrote: > On Wed, 2015-11-11 at 12:38 -0800, Brian Norris wrote: > > Applied to l2-mtd.git/next (for 4.5). This will show up in > > linux-next.git after the merge window. > > > > Also squashed in a small diff (below), to fix up some language issues > > and to refer the reader to the jedec,spi-nor.txt document. > > > > OK, I will fix it in next patch! No, you don't need to send another patch. I already merged just this one (the DT documentation) with my own small fixes, with a plan to go into 4.5. You only need to send another version of patch 2 (the driver). During the merge window, I'm stashing changes in l2-mtd.git's 'next' branch, so we can keep stuff that's going to 4.5 separate from stuff thats getting merged to 4.4 right now. You can see it here for now: http://git.infradead.org/l2-mtd.git/shortlog/refs/heads/next After the merge window closes (probably on Sunday), I'll bring this into +master. Regards, Brian -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mtd: grab a reference to the MTD of_node before registering it
On Thu, Nov 12, 2015 at 09:51:20AM +0100, Boris Brezillon wrote: > On Wed, 11 Nov 2015 16:26:04 -0800 > Brian Norris wrote: > > > We now stick the device node representing the current MTD (if any) into > > sysfs, so let's make sure we have a reference to it before doing that. > > > > Suggested-by: Boris Brezillon > > Signed-off-by: Brian Norris > > Looks good to me, > > Reviewed-by: Boris Brezillon Pushed to l2-mtd.git/next -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] mtd: assign mtd->dev.of_node when creating partition devices
From: Boris BREZILLON
MTD partitions may have been created from a DT definition, and in this case
the ->of_node of the struct device embedded in mtd_info should point to
the DT node that was used to create the partition.
Signed-off-by: Boris Brezillon
Signed-off-by: Brian Norris
---
v2: drop most of the get/put node handling; allow the MTD core to do
this in {add,del}_mtd_device()
depends on: http://patchwork.ozlabs.org/patch/543159/
v1: http://patchwork.ozlabs.org/patch/538838/
drivers/mtd/mtdpart.c | 1 +
drivers/mtd/ofpart.c | 2 +-
include/linux/mtd/partitions.h | 1 +
3 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index f8ba153f63bf..c2b6e967a160 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -391,6 +391,7 @@ static struct mtd_part *allocate_partition(struct mtd_info
*master,
slave->mtd.dev.parent = IS_ENABLED(CONFIG_MTD_PARTITIONED_MASTER) ?
&master->dev :
master->dev.parent;
+ mtd_set_of_node(&slave->mtd, part->of_node);
slave->mtd._read = part_read;
slave->mtd._write = part_write;
diff --git a/drivers/mtd/ofpart.c b/drivers/mtd/ofpart.c
index f78d2aea5545..dacf4c405dec 100644
--- a/drivers/mtd/ofpart.c
+++ b/drivers/mtd/ofpart.c
@@ -111,6 +111,7 @@ static int parse_ofpart_partitions(struct mtd_info *master,
if (of_get_property(pp, "lock", &len))
(*pparts)[i].mask_flags |= MTD_POWERUP_LOCK;
+ (*pparts)[i].of_node = pp;
i++;
}
@@ -124,7 +125,6 @@ ofpart_fail:
master->name, pp->full_name, mtd_node->full_name);
ret = -EINVAL;
ofpart_none:
- of_node_put(pp);
kfree(*pparts);
*pparts = NULL;
return ret;
diff --git a/include/linux/mtd/partitions.h b/include/linux/mtd/partitions.h
index 773975a3c9e6..282644c0c5c0 100644
--- a/include/linux/mtd/partitions.h
+++ b/include/linux/mtd/partitions.h
@@ -42,6 +42,7 @@ struct mtd_partition {
uint64_t offset;/* offset within the master MTD space */
uint32_t mask_flags;/* master MTD flags to mask out for
this partition */
struct nand_ecclayout *ecclayout; /* out of band layout for this
partition (NAND only) */
+ struct device_node *of_node;/* OF node attached to the partition */
};
#define MTDPART_OFS_RETAIN (-3)
--
2.6.0.rc2.230.g3dd15c0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] mtd: grab a reference to the MTD of_node before registering it
We now stick the device node representing the current MTD (if any) into sysfs, so let's make sure we have a reference to it before doing that. Suggested-by: Boris Brezillon Signed-off-by: Brian Norris --- drivers/mtd/mtdcore.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c index a91cee90aef9..c393a1155376 100644 --- a/drivers/mtd/mtdcore.c +++ b/drivers/mtd/mtdcore.c @@ -32,6 +32,7 @@ #include #include #include +#include #include #include #include @@ -454,6 +455,7 @@ int add_mtd_device(struct mtd_info *mtd) mtd->dev.devt = MTD_DEVT(i); dev_set_name(&mtd->dev, "mtd%d", i); dev_set_drvdata(&mtd->dev, mtd); + of_node_get(mtd_get_of_node(mtd)); error = device_register(&mtd->dev); if (error) goto fail_added; @@ -476,6 +478,7 @@ int add_mtd_device(struct mtd_info *mtd) return 0; fail_added: + of_node_put(mtd_get_of_node(mtd)); idr_remove(&mtd_idr, i); fail_locked: mutex_unlock(&mtd_table_mutex); @@ -517,6 +520,7 @@ int del_mtd_device(struct mtd_info *mtd) device_unregister(&mtd->dev); idr_remove(&mtd_idr, mtd->index); + of_node_put(mtd_get_of_node(mtd)); module_put(THIS_MODULE); ret = 0; -- 2.6.0.rc2.230.g3dd15c0 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 2/3] mtd: mtk-nor: mtk serial flash controller driver
One more small comment, since you're respinning this:
On Fri, Nov 06, 2015 at 11:48:08PM +0800, Bayi Cheng wrote:
> diff --git a/drivers/mtd/spi-nor/mtk-quadspi.c
> b/drivers/mtd/spi-nor/mtk-quadspi.c
> new file mode 100644
> index 000..6582811
> --- /dev/null
> +++ b/drivers/mtd/spi-nor/mtk-quadspi.c
> @@ -0,0 +1,475 @@
...
> +static int mtk_nor_drv_probe(struct platform_device *pdev)
> +{
> + struct device_node *flash_np;
> + struct resource *res;
> + int ret;
> + struct mt8173_nor *mt8173_nor;
> +
> + if (!pdev->dev.of_node) {
> + dev_err(&pdev->dev, "No DT found\n");
> + return -EINVAL;
> + }
> +
> + mt8173_nor = devm_kzalloc(&pdev->dev, sizeof(*mt8173_nor), GFP_KERNEL);
> + if (!mt8173_nor)
> + return -ENOMEM;
> + platform_set_drvdata(pdev, mt8173_nor);
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + mt8173_nor->base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(mt8173_nor->base))
> + return PTR_ERR(mt8173_nor->base);
> +
> + mt8173_nor->spi_clk = devm_clk_get(&pdev->dev, "spi");
> + if (IS_ERR(mt8173_nor->spi_clk)) {
> + ret = PTR_ERR(mt8173_nor->spi_clk);
> + goto nor_free;
> + }
> +
> + mt8173_nor->nor_clk = devm_clk_get(&pdev->dev, "sf");
> + if (IS_ERR(mt8173_nor->nor_clk)) {
> + ret = PTR_ERR(mt8173_nor->nor_clk);
> + goto nor_free;
> + }
> +
> + mt8173_nor->dev = &pdev->dev;
> + clk_prepare_enable(mt8173_nor->spi_clk);
> + clk_prepare_enable(mt8173_nor->nor_clk);
You enable the clocks here, but...
> +
> + /* only support one attached flash */
> + flash_np = of_get_next_available_child(pdev->dev.of_node, NULL);
> + if (!flash_np) {
... you might bail out here if there is no available flash node, and you
never disable the clocks. (Same is true if we fail to detect the flash;
you leave the no-longer-needed clocks enabled.) Seems like maybe you
should disable clocks on failure.
> + dev_err(&pdev->dev, "no SPI flash device to configure\n");
> + ret = -ENODEV;
> + goto nor_free;
> + }
> + ret = mtk_nor_init(mt8173_nor, flash_np);
> +
> +nor_free:
> + return ret;
> +}
Brian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 3/3] arm64: dts: mt8173: Add nor flash node
On Fri, Nov 06, 2015 at 11:48:09PM +0800, Bayi Cheng wrote:
> Add Mediatek nor flash node
>
> Signed-off-by: Bayi Cheng
Acked-by: Brian Norris
> ---
> arch/arm64/boot/dts/mediatek/mt8173.dtsi | 16
> 1 file changed, 16 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> index d18ee42..f5f08eb 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> @@ -365,6 +365,22 @@
> status = "disabled";
> };
>
> + nor_flash: spi@1100d000 {
> + compatible = "mediatek,mt8173-nor";
> + reg = <0 0x1100d000 0 0xe0>;
> + clocks = <&pericfg CLK_PERI_SPI>,
> + <&topckgen CLK_TOP_SPINFI_IFR_SEL>;
> + clock-names = "spi", "sf";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + status = "disabled";
> +
> + flash@0 {
> + compatible = "jedec,spi-nor";
> + reg = <0>;
> + };
> + };
> +
> i2c3: i2c3@1101 {
> compatible = "mediatek,mt8173-i2c";
> reg = <0 0x1101 0 0x70>,
> --
> 1.8.1.1.dirty
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 1/3] doc: dt: add documentation for Mediatek spi-nor controller
On Fri, Nov 06, 2015 at 11:48:07PM +0800, Bayi Cheng wrote:
> Add device tree binding documentation for serial flash with
> Mediatek serial flash controller
>
> Signed-off-by: Bayi Cheng
> ---
Applied to l2-mtd.git/next (for 4.5). This will show up in
linux-next.git after the merge window.
Also squashed in a small diff (below), to fix up some language issues
and to refer the reader to the jedec,spi-nor.txt document.
> .../devicetree/bindings/mtd/mtk-quadspi.txt| 41
> ++
> 1 file changed, 41 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mtd/mtk-quadspi.txt
>
> diff --git a/Documentation/devicetree/bindings/mtd/mtk-quadspi.txt
> b/Documentation/devicetree/bindings/mtd/mtk-quadspi.txt
> new file mode 100644
> index 000..866b492
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/mtk-quadspi.txt
> @@ -0,0 +1,41 @@
> +* MTD SPI nor driver for MTK MT81xx (and similar) serial flash controller
The DT binding document shouldn't be talking about software (i.e.,
shouldn't be talking about "drivers").
> +
> +Required properties:
> +- compatible: should be "mediatek,mt8173-nor";
> +- reg: physical base address and length of the controller's
> register
> +- clocks: the phandle of the clock needed by the nor controller
> +- clock-names: the name of the clocks
> + the clocks needed "spi" and "sf". "spi" is used for spi bus,
> + and "sf" is used for controller, these are the clocks witch
> + hardware needs to enabling nor flash and nor flash controller.
> + See
> Documentation/devicetree/bindings/clock/clock-bindings.txt for details.
> +- #address-cells: should be <1>
> +- #size-cells: should be <0>
> +
> +The SPI Flash must be a child of the nor_flash node and must have a
> +compatible property.
> +
> +Required properties:
> +- compatible: May include a device-specific string consisting of
> the manufacturer
> + and name of the chip. Must also include "jedec,spi-nor" for
> any
> + SPI NOR flash that can be identified by the JEDEC READ ID
> opcode (0x9F).
> +- reg :Chip-Select number
> +
> +Example:
> +
> +nor_flash: spi@1100d000 {
> + compatible = "mediatek,mt8173-nor";
> + reg = <0 0x1100d000 0 0xe0>;
> + clocks = <&pericfg CLK_PERI_SPI>,
> + <&topckgen CLK_TOP_SPINFI_IFR_SEL>;
> + clock-names = "spi", "sf";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + status = "disabled";
> +
> + flash@0 {
> + compatible = "jedec,spi-nor";
> + reg = <0>;
> + };
> +};
> +
diff --git a/Documentation/devicetree/bindings/mtd/mtk-quadspi.txt
b/Documentation/devicetree/bindings/mtd/mtk-quadspi.txt
index 866b492c38d2..fb314f09861b 100644
--- a/Documentation/devicetree/bindings/mtd/mtk-quadspi.txt
+++ b/Documentation/devicetree/bindings/mtd/mtk-quadspi.txt
@@ -1,19 +1,19 @@
-* MTD SPI nor driver for MTK MT81xx (and similar) serial flash controller
+* Serial NOR flash controller for MTK MT81xx (and similar)
Required properties:
- compatible:should be "mediatek,mt8173-nor";
- reg: physical base address and length of the controller's
register
-- clocks:the phandle of the clock needed by the nor controller
-- clock-names: the name of the clocks
- the clocks needed "spi" and "sf". "spi" is used for spi bus,
+- clocks:the phandle of the clocks needed by the nor controller
+- clock-names: the names of the clocks
+ the clocks should be named "spi" and "sf". "spi" is used for
spi bus,
and "sf" is used for controller, these are the clocks witch
hardware needs to enabling nor flash and nor flash controller.
See
Documentation/devicetree/bindings/clock/clock-bindings.txt for details.
- #address-cells: should be <1>
- #size-cells: should be <0>
-The SPI Flash must be a child of the nor_flash node and must have a
-compatible property.
+The SPI flash must be a child of the nor_flash node and must have a
+compatible property. Also see jedec,spi-nor.txt.
Required properties:
- compatible:May include a device-specific string consisting of the
manufacturer
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 0/3] Mediatek SPI-NOR flash driver
On Wed, Nov 11, 2015 at 10:04:14PM +0800, Bayi Cheng wrote:
> On Mon, 2015-11-09 at 18:46 -0800, Brian Norris wrote:
> > I believe you didn't completely answer all my questions from v5 though.
> > I'll repeat a bit here. Particularly, refer to [1].
> > I'll summarize; I understand that your common transmit/receive operation
> > works something like this:
> >
> > Quoting from [1]:
> > > (1) total number of bits to send/receive goes in the COUNT register (so
> > > far, as many as 7*8=56?)
> > > (2) opcode is written to PRGDATA5
> > > (3) other "transmit" data (like addresses), if any, are placed on
> > > PRGDATA4..0
> > > (4) command is sent (execute_cmd())
> > > (5) data is read back in SHREG{X..0}, if needed
> >
> > My questions were:
> >
> > (a) Why does mt8173_nor_set_read_mode() use PRGDATA3? That's not
> > mentioned in the SoC manual, and it doesn't really match any of the
> > steps above. Perhaps it's just a quirk of the controller's
> > programming model?
> >
> yes, for this question, I have done some testes, If I change the
> PRGDATA3 to PRGDATA5 for mt8173_nor_set_read_mode() like others
> functions, then the controller will be hanged, and I have asked our
> designer for double confirm.
I wasn't suggesting to change this to PRGDATA5. I just was wondering why
the difference. It's not documented. (I suppose an acceptable answer is
just "because that's how the HW works.")
> > (b) How do you determine X from step (5)?
> >
> > Right now, your code seems to answer that X is "rxlen - 1". Correct?
> >
> yes, I have used "rxlen -1", because the first of nor flash output is
> located at SHREG[0], in the other words, the output data starts at
> SHREG[0] and go up to SHREG[relen -1]
But, we aren't reading from SHREG[0] first; we're reading backwards from
SHREG[rxlen - 1] down to SHREG[0]. It seems that's correct, right?
> > If that's correct and if I put all of my understanding together
> > correctly, this means that you can actually shift out (in PRGDATA) up to
> > 6 bytes (that is, 1 opcode and 5 tx bytes) and shift in (in SHREG) up to
> > 7 bytes, except that the first byte is received during the opcode cycle,
> > and so it is discarded, and we effectively receive only 6 bytes.
> >
> > Is that all correct? If so, then I think you still need to adjust the
> > boundary conditions in your do_tx_rx() function. (I'll comment on the
> > driver to point out the specifics.)
>
> Yes, you are right! and I will adjust the boundary conditions in
> do_tx_rx() function.
OK, good. BTW, can you make sure to rewrite the appropriate MAX macro(s)
to reflect the right values? It seems like maybe you'll want separate
macros for the maximum TX and RX -- and total (?), or is this just the
same as RX? -- since they seem to have different limits.
> By the way, could you tell me whether I need to publish a new patch? or
> you can fix them up directly?
I think there are a few more adjustments to make, so please just post a
new version of the driver only. The DT binding and DTS changes look good
to go now.
Regards,
Brian
> > [1] http://lists.infradead.org/pipermail/linux-mtd/2015-October/062951.html
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/5] spi: introduce mmap read support for spi flash devices
In addition to my other comments:
On Tue, Nov 10, 2015 at 10:59:55AM +0530, Vignesh R wrote:
> In addition to providing direct access to SPI bus, some spi controller
> hardwares (like ti-qspi) provide special memory mapped port
> to accesses SPI flash devices in order to increase read performance.
> This means the controller can automatically send the SPI signals
> required to read data from the SPI flash device.
> For this, spi controller needs to know flash specific information like
> read command to use, dummy bytes and address width. Once these settings
> are populated in hardware registers, any read accesses to flash's memory
> map region(SoC specific) through memcpy (or mem-to mem DMA copy) will be
> handled by controller hardware. The hardware will automatically generate
> SPI signals required to read data from flash and present it to CPU/DMA.
>
> Introduce spi_mtd_mmap_read() interface to support memory mapped read
> over SPI flash devices. SPI master drivers can implement this callback to
> support memory mapped read interfaces. m25p80 flash driver and other
> flash drivers can call this to request memory mapped read. The interface
> should only be used MTD flashes and cannot be used with other SPI devices.
>
> Signed-off-by: Vignesh R
> ---
...
> diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
> index cce80e6dc7d1..2f2c431b8917 100644
> --- a/include/linux/spi/spi.h
> +++ b/include/linux/spi/spi.h
> @@ -361,6 +361,11 @@ static inline void spi_unregister_driver(struct
> spi_driver *sdrv)
> * @handle_err: the subsystem calls the driver to handle an error that occurs
> * in the generic implementation of transfer_one_message().
> * @unprepare_message: undo any work done by prepare_message().
> + * @spi_mtd_mmap_read: some spi-controller hardwares provide memory.
> + * Flash drivers (like m25p80) can request memory
> + * mapped read via this method. This interface
> + * should only be used by mtd flashes and cannot be
> + * used by other spi devices.
> * @cs_gpios: Array of GPIOs to use as chip select lines; one per CS
> * number. Any individual value may be -ENOENT for CS lines that
> * are not GPIOs (driven by the SPI controller itself).
> @@ -507,6 +512,11 @@ struct spi_master {
> struct spi_message *message);
> int (*unprepare_message)(struct spi_master *master,
>struct spi_message *message);
> + int (*spi_mtd_mmap_read)(struct spi_device *spi,
> + loff_t from, size_t len,
> + size_t *retlen, u_char *buf,
> + u8 read_opcode, u8 addr_width,
> + u8 dummy_bytes);
This is seeming to be a longer and longer list of arguments. I know MTD
has a bad habit of long argument lists (which then cause a ton of
unnecessary churn when things need changed in the API), but perhaps we
can limit the damage to the SPI layer. Perhaps this deserves a struct to
encapsulate all the flash read arguments? Like:
struct spi_flash_read_message {
loff_t from;
size_t len;
size_t *retlen;
void *buf;
u8 read_opcode;
u8 addr_width;
u8 dummy_bits;
// additional fields to describe rx_nbits for opcode/addr/data
};
struct spi_master {
...
int (*spi_flash_read)(struct spi_device *spi,
struct spi_flash_message *msg);
};
>
> /*
>* These hooks are for drivers that use a generic implementation
...
Brian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mtd: spi-nor: fsl-quadspi: add big-endian support
+ devicetree
On Wed, Nov 11, 2015 at 11:51:13AM -0600, Han Xu wrote:
> On Fri, Oct 30, 2015 at 04:49:41AM -0500, Yuan Yao-B46683 wrote:
(BTW Yuan, replying on top doesn't make the conversation as easy to
follow)
> > Although it is possible to add the endianness support in the
> > regmap_mmio driver, we don't see too much value in using it
> > especially
> >
> > So we think:
> > static void qspi_writel(struct fsl_qspi *q, u32 val, void __iomem
> > *addr) {
> > if (q->big_endian)
> > iowrite32be(val, addr);
> > else
> > iowrite32(val, addr);
> > }
> > This way is an easier, more effective solution to do the endian issue.
> >
> > How about your think?
>
> I think the implement is fine, but I prefer to use quirk rather than
> read from dts? Please also rebase the patch to latest l2-mtd code.
If it really is just a endianness difference, then I think it makes
sense to use the existing DT bindings for it, rather than relying on a
new compatible string / quirk option. That doesn't mean you can't have a
new SoC-inspired compatible property in addition...
> > Best Regards,
> > Yuan Yao
> >
> > On Sat, Oct 24, 2015 at 11:47 PM, Fabio Estevam wrote:
> > > I suggest you to implement regmap support for this driver instead.
> > >
> > > Take a look at drivers/watchdog/imx2_wdt.c for a reference.
> > >
> > > Then you only need to pass 'big-endian' as a property for the qspi in the
> > > .dtsi
> > > file and regmap core will take care of endianness.
To use the standard binding also means that whether or not you choose to
use regmap right now, it's an easy option in the future, and the core
code will already handle it for you. That's really one of the main
reasons for using standardized bindings in the first place.
Brian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/5] spi: introduce mmap read support for spi flash devices
Hi,
On Wed, Nov 11, 2015 at 12:20:46PM +0530, R, Vignesh wrote:
> On 11/11/2015 4:53 AM, Brian Norris wrote:
> > On Tue, Nov 10, 2015 at 10:59:55AM +0530, Vignesh R wrote:
> >> diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
> >> index cce80e6dc7d1..2f2c431b8917 100644Hi
> >> --- a/include/linux/spi/spi.h
> >> +++ b/include/linux/spi/spi.h
> >> @@ -361,6 +361,11 @@ static inline void spi_unregister_driver(struct
> >> spi_driver *sdrv)
> >> * @handle_err: the subsystem calls the driver to handle an error that
> >> occurs
> >> *in the generic implementation of transfer_one_message().
> >> * @unprepare_message: undo any work done by prepare_message().
> >> + * @spi_mtd_mmap_read: some spi-controller hardwares provide memory.
> >> + * Flash drivers (like m25p80) can request memory
> >> + * mapped read via this method. This interface
> >> + * should only be used by mtd flashes and cannot be
> >> + * used by other spi devices.
> >> * @cs_gpios: Array of GPIOs to use as chip select lines; one per CS
> >> *number. Any individual value may be -ENOENT for CS lines that
> >> *are not GPIOs (driven by the SPI controller itself).
> >> @@ -507,6 +512,11 @@ struct spi_master {
> >> struct spi_message *message);
> >>int (*unprepare_message)(struct spi_master *master,
> >> struct spi_message *message);
> >> + int (*spi_mtd_mmap_read)(struct spi_device *spi,
> >> + loff_t from, size_t len,
> >> + size_t *retlen, u_char *buf,
> >> + u8 read_opcode, u8 addr_width,
> >> + u8 dummy_bytes);
> >
> > Is this API really sufficient? There are actually quite a few other
> > flash-related parameters that might be relevant to a controller. I
> > presume you happen not hit them because of the particular cases you're
> > using this for right now, but:
> >
> > * How many I/O lines are being used? These can vary depending on the
> >type of flash and the number of I/O lines supported by the controller
> >and connected on the board.
> >
>
> This API communicates whatever data is currently communicated via
> spi_message through spi_sync/transfer_one interfaces.
No it doesn't. A spi_message consists of a list of spi_transfer's, and
each spi_transfer has tx_nbits and rx_nbits fields.
> > * The previous point can vary across parts of the message. There are
> >various combinations of 1/2/4 lines used for opcode/address/data. We
> >only support a few of those combinations in m25p80 right now, but
> >you're not specifying any of that in this API. I guess you're just
> >making assumptions? (BTW, I think there are others having problems
> >with the difference between different "quad" modes on Micron flash; I
> >haven't sorted through all the discussion there.)
> >
>
> How is the spi controller currently being made aware of this via
> m25p80_read / spi_sync() interface? AFAIK, mode field of spi_device
> struct tell whether to do normal/dual/quad read but there is no info
> communicated wrt 1/2/4 opcode/address/data combinations.
Yes there is. m25p80 fills out spi_transfer::rx_nbits. Currently, we
only use this for the data portion, but it's possible to support more
lines for the address and opcode portions too, using the rx_nbits for
the opcode and address spi_transfer struct(s) (currently, m25p80_read()
uses 2 spi_transfers per message, where the first one contains opcode +
address + dummy on a single line, and the second transfer receives the
data on 1, 2, or 4 lines).
> And there is no
> info indicating capabilities of spi-master wrt no of IO lines for
> opcode/address/data that it can support.
For a true SPI controller, there is no need to specify something
different for opcode/address/data, since all those are treated the same;
they're just bits on 1, 2, or 4 lines. So the SPI_{TX,RX}_{DUAL,QUAD}
mode flags in struct spi_master tell m25p80 all it needs to know.
> >There are typically both flash device and SPI controller constraints
> >on this question, so there needs to be some kind of negotiation
> >involved, I expect. Or at least, the SPI master needs to expose which
> >modes it can support with this flash-read API.
> >
>
> If spi-master capabilities are known
For generic SPI handling, these are already kn
Re: [PATCH v3 1/5] spi: introduce mmap read support for spi flash devices
Hi Vignesh,
Sorry for the late review. I did not have time to review much back when
you submitted your first RFCs for this.
On Tue, Nov 10, 2015 at 10:59:55AM +0530, Vignesh R wrote:
> diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
> index cce80e6dc7d1..2f2c431b8917 100644
> --- a/include/linux/spi/spi.h
> +++ b/include/linux/spi/spi.h
> @@ -361,6 +361,11 @@ static inline void spi_unregister_driver(struct
> spi_driver *sdrv)
> * @handle_err: the subsystem calls the driver to handle an error that occurs
> * in the generic implementation of transfer_one_message().
> * @unprepare_message: undo any work done by prepare_message().
> + * @spi_mtd_mmap_read: some spi-controller hardwares provide memory.
> + * Flash drivers (like m25p80) can request memory
> + * mapped read via this method. This interface
> + * should only be used by mtd flashes and cannot be
> + * used by other spi devices.
> * @cs_gpios: Array of GPIOs to use as chip select lines; one per CS
> * number. Any individual value may be -ENOENT for CS lines that
> * are not GPIOs (driven by the SPI controller itself).
> @@ -507,6 +512,11 @@ struct spi_master {
> struct spi_message *message);
> int (*unprepare_message)(struct spi_master *master,
>struct spi_message *message);
> + int (*spi_mtd_mmap_read)(struct spi_device *spi,
> + loff_t from, size_t len,
> + size_t *retlen, u_char *buf,
> + u8 read_opcode, u8 addr_width,
> + u8 dummy_bytes);
Is this API really sufficient? There are actually quite a few other
flash-related parameters that might be relevant to a controller. I
presume you happen not hit them because of the particular cases you're
using this for right now, but:
* How many I/O lines are being used? These can vary depending on the
type of flash and the number of I/O lines supported by the controller
and connected on the board.
* The previous point can vary across parts of the message. There are
various combinations of 1/2/4 lines used for opcode/address/data. We
only support a few of those combinations in m25p80 right now, but
you're not specifying any of that in this API. I guess you're just
making assumptions? (BTW, I think there are others having problems
with the difference between different "quad" modes on Micron flash; I
haven't sorted through all the discussion there.)
There are typically both flash device and SPI controller constraints
on this question, so there needs to be some kind of negotiation
involved, I expect. Or at least, the SPI master needs to expose which
modes it can support with this flash-read API.
Also, this API doesn't actually have anything to do with memory mapping.
It has to do with the de facto standard flash protocol. So I don't think
mmap belongs in the name; it should be something about flash. (I know of
at least one other controller that could probably use this API, excpet
it doesn't use memory mapping to accomplish the accelerated flash read.)
>
> /*
>* These hooks are for drivers that use a generic implementation
Brian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] mtd: ofpart: document the lock flag.
On Fri, Oct 30, 2015 at 01:34:00PM -0700, Brian Norris wrote: > From: Michal Suchanek > > The lock flag of ofpart is undocumented. Add to binding doc. > > Signed-off-by: Michal Suchanek > Signed-off-by: Brian Norris > Cc: Sascha Hauer Pushed to l2-mtd.git -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] mtd: create a partition type device tree binding
On Fri, Nov 06, 2015 at 08:13:13AM -0600, Rob Herring wrote: > Since we now have partitions contained in a sub node, how about using > compatible for that sub node instead. That seems like a pretty good idea to me. Brian -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 2/3] mtd: mtk-nor: mtk serial flash controller driver
Hi Bayi,
A few small comments.
On Fri, Nov 06, 2015 at 11:48:08PM +0800, Bayi Cheng wrote:
> add spi nor flash driver for mediatek controller
>
> Signed-off-by: Bayi Cheng
> ---
> drivers/mtd/spi-nor/Kconfig | 7 +
> drivers/mtd/spi-nor/Makefile | 1 +
> drivers/mtd/spi-nor/mtk-quadspi.c | 475
> ++
> 3 files changed, 483 insertions(+)
> create mode 100644 drivers/mtd/spi-nor/mtk-quadspi.c
> diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
> index e5e..0bf3a7f8 100644
> --- a/drivers/mtd/spi-nor/Makefile
> +++ b/drivers/mtd/spi-nor/Makefile
> @@ -1,3 +1,4 @@
> obj-$(CONFIG_MTD_SPI_NOR)+= spi-nor.o
> obj-$(CONFIG_SPI_FSL_QUADSPI)+= fsl-quadspi.o
> +obj-$(CONFIG_MTD_MT81xx_NOR)+= mtk-quadspi.o
> obj-$(CONFIG_SPI_NXP_SPIFI) += nxp-spifi.o
> diff --git a/drivers/mtd/spi-nor/mtk-quadspi.c
> b/drivers/mtd/spi-nor/mtk-quadspi.c
> new file mode 100644
> index 000..6582811
> --- /dev/null
> +++ b/drivers/mtd/spi-nor/mtk-quadspi.c
> @@ -0,0 +1,475 @@
...
> +/* Can shift up to 48 bits (6 bytes) of TX/RX */
> +#define MTK_NOR_MAX_SHIFT6
...
> +static int mt8173_nor_do_tx_rx(struct mt8173_nor *mt8173_nor, u8 op,
> +u8 *tx, int txlen, u8 *rx, int rxlen)
> +{
> + int len = 1 + txlen + rxlen;
> + int i, ret, idx;
> +
> + if (len > MTK_NOR_MAX_SHIFT + 1)
> + return -EINVAL;
So I see you adjusted these bounds to add 1, which inspired one of my
questions on the cover letter.
Why do you allow len=7, which means you'd program 7*8 to the COUNT
register, when the SoC manual says it has a max of 48? Is the manual
wrong?
I notice you added the '+ 1' to your driver, so it allows:
do_tx_rx( txlen = 0 , rxlen = 6) /* e.g., for READ ID */
but that means your driver also allows:
do_tx_rx( txlen = 6, rxlen = 0) /* ERROR: this will allow out of
bounds write on PRGDATA
register -1 */
If I understand correctly, the following constraints are more correct:
/* Can only transmit 1 opcode and 5 other bytes */
if (1 + txlen > MTK_NOR_MAX_SHIFT)
return -EINVAL;
/* Can only receive 6 bytes after the opcode */
if (rxlen > MTK_NOR_MAX_SHIFT)
return -EINVAL;
/* Can only handle XXX bytes total */
/* How many bytes is the max for register MTK_NOR_CNT_REG ? */
if (len > XXX)
return -EINVAL;
> +
> + writeb(len * 8, mt8173_nor->base + MTK_NOR_CNT_REG);
> +
> + /* start at PRGDATA5, go down to PRGDATA0 */
> + idx = MTK_NOR_MAX_SHIFT - 1;
> +
> + /* opcode */
> + writeb(op, mt8173_nor->base + MTK_NOR_PRG_REG(idx));
> + idx--;
> +
> + /* program TX data */
> + for (i = 0; i < txlen; i++, idx--)
> + writeb(tx[i], mt8173_nor->base + MTK_NOR_PRG_REG(idx));
> +
> + /* clear out rest of TX registers */
> + while (idx >= 0) {
> + writeb(0, mt8173_nor->base + MTK_NOR_PRG_REG(idx));
> + idx--;
> + }
> +
> + ret = mt8173_nor_execute_cmd(mt8173_nor, MTK_NOR_PRG_CMD);
> + if (ret)
> + return ret;
> +
> + /* restart at first RX byte */
> + idx = rxlen - 1;
> +
> + /* read out RX data */
> + for (i = 0; i < rxlen; i++, idx--)
> + rx[i] = readb(mt8173_nor->base + MTK_NOR_SHREG(idx));
> +
> + return 0;
> +}
> +
...
> +static int __init mtk_nor_init(struct mt8173_nor *mt8173_nor,
> +struct device_node *flash_node)
> +{
> + struct mtd_part_parser_data ppdata = {
> + .of_node = flash_node,
> + };
> + int ret;
> + struct spi_nor *nor;
Normally we'd have a blank line here.
> + /* initialize controller to accept commands */
> + writel(MTK_NOR_ENABLE_SF_CMD, mt8173_nor->base + MTK_NOR_WRPROT_REG);
> +
> + nor = &mt8173_nor->nor;
> + nor->dev = mt8173_nor->dev;
> + nor->priv = mt8173_nor;
> + nor->flash_node = flash_node;
> +
> + /* fill the hooks to spi nor */
> + nor->read = mt8173_nor_read;
> + nor->read_reg = mt8173_nor_read_reg;
> + nor->write = mt8173_nor_write;
> + nor->write_reg = mt8173_nor_write_reg;
> + nor->mtd.owner = THIS_MODULE;
> + nor->mtd.name = "mtk_nor";
> + /* initialized with NULL */
> + ret = spi_nor_scan(nor, NULL, SPI_NOR_DUAL);
> + if (ret)
> + return ret;
> +
> + return mtd_device_parse_register(&nor->mtd, NULL, &ppdata, NULL, 0);
> +}
...
Brian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 0/3] Mediatek SPI-NOR flash driver
Hi Bayi,
On Fri, Nov 06, 2015 at 11:48:06PM +0800, Bayi Cheng wrote:
> This series is based on v4.3-rc1 and l2-mtd.git [0] and erase_sector
> implementation patch [1]
>
> [0]: git://git.infradead.org/l2-mtd.git
> [1]: http://lists.infradead.org/pipermail/linux-mtd/2015-October//062959.html
>
> Change in v6:
> 1: delete mt8173_nor_do_rx
> 2: delete mt8173_nor_do_rx
> 3: add mt8173_nor_do_tx_rx for general usage
> 4: support nor flash with 6 IDs
> 5: delete mt8173_nor_erase_sector and use "nor->erase_opcode"
> 6: add mt8173_nor_set_addr to programming the address register
> 7: initialize the ppdata in mtk_nor_init
This series is looking a lot better to me. Thanks for incorporating (and
I hope fully reviewing and testing!) my suggested changes. I have a just
a few small comments that I might post to the driver patch, and if
that's all that's outstanding, I can fix them up myself before applying.
I believe you didn't completely answer all my questions from v5 though.
I'll repeat a bit here. Particularly, refer to [1].
I'll summarize; I understand that your common transmit/receive operation
works something like this:
Quoting from [1]:
> (1) total number of bits to send/receive goes in the COUNT register (so
> far, as many as 7*8=56?)
> (2) opcode is written to PRGDATA5
> (3) other "transmit" data (like addresses), if any, are placed on PRGDATA4..0
> (4) command is sent (execute_cmd())
> (5) data is read back in SHREG{X..0}, if needed
My questions were:
(a) Why does mt8173_nor_set_read_mode() use PRGDATA3? That's not
mentioned in the SoC manual, and it doesn't really match any of the
steps above. Perhaps it's just a quirk of the controller's
programming model?
(b) How do you determine X from step (5)?
Right now, your code seems to answer that X is "rxlen - 1". Correct?
If that's correct and if I put all of my understanding together
correctly, this means that you can actually shift out (in PRGDATA) up to
6 bytes (that is, 1 opcode and 5 tx bytes) and shift in (in SHREG) up to
7 bytes, except that the first byte is received during the opcode cycle,
and so it is discarded, and we effectively receive only 6 bytes.
Is that all correct? If so, then I think you still need to adjust the
boundary conditions in your do_tx_rx() function. (I'll comment on the
driver to point out the specifics.)
Regards,
Brian
[1] http://lists.infradead.org/pipermail/linux-mtd/2015-October/062951.html
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 10/17] phy: Add driver for rockchip Display Port PHY
Hi,
A few updates:
On Tue, Nov 03, 2015 at 05:13:48PM -0800, Brian Norris wrote:
> On Wed, Nov 04, 2015 at 08:48:38AM +0800, Yakir Yang wrote:
> > On 11/03/2015 12:38 PM, Brian Norris wrote:
> > >On Thu, Oct 29, 2015 at 09:58:38AM +0800, Yakir Yang wrote:
> > >(FYI, I came across this by inspection when comparing Heiko's
> > >'somewhat-stable' branch [1] with this series. The former brings up eDP
> > >fine on veyron-jaq, whereas this one doesn't yet, even with the above
> > >change. Still debugging the issue.)
Some time after the above comment, I managed to kill the panel on my
Jaq :( I think the wiring around the hinge was a bit flaky, and it
finally went out for good.
> > Hmm... I'm not sure whether your eDP screen have the hotplug signal, so I
>
> I believe hotplug is hooked up but...
>
> > think you can try to add "analogix,force-hpd" flag into
> > rk3288-veyron-jaq.dts
> >
> > &edp {
> > analogix,need-force-hpd;
> > }
>
> ...already tried, just in case. No luck.
However, now when testing a different Jaq device, now this series +
Heiko's DTS updates + the "analogix,force-hpd" (i.e., [1]) works fine,
modulo a few log warnings, some of which are probably expected (for
instance, I believe the EDID is known not-so-helpful). Snippets:
[3.170176] rockchip-dp ff97.dp: AUX CH command reply failed!
[3.178058] rockchip-dp ff97.dp: AUX CH command reply failed!
[3.184166] rockchip-dp ff97.dp: unable to handle edid
and later:
[3.953300] rockchip-dp ff97.dp: EDID data does not include any
extensions.
[3.966731] rockchip-dp ff97.dp: EDID data does not include any
extensions.
[3.979409] rockchip-dp ff97.dp: EDID data does not include any
extensions.
[3.998730] rockchip-dp ff97.dp: Link Training Clock Recovery success
[4.007046] rockchip-dp ff97.dp: Link Training success!
[4.115040] rockchip-dp ff97.dp: Timeout of video streamclk ok
[4.121211] rockchip-dp ff97.dp: unable to config video
[4.127616] rockchip-dp ff97.dp: EDID data does not include any
extensions.
So, I'll chalk that earlier failure up to a hardware failure (or
possibly a still yet-undiagnosed hardware difference; my new Jaq has
some small differences from the previous unit).
Also, it's still not real clear why HPD isn't working upstream (and we
have to use the "force-hpd" property), when it appears to work on our
downstream Chrome OS tree.
Finally, I'll leave you with some small bits I've noticed from exploring
this issue on Jaq:
* The Chrome OS driver for this IP has a much longer timeout in (the
equivalent of) analogix_dp_detect_hpd; it polls in 10-20 ms intervals
(rather than 10-11 us) and takes something around 60 to 120 ms to
notice the panel.
* AFAICT, the Chrome OS driver never actually used the HPD interrupt;
it was only polling the HPD status bit. So I can't claim that the
functionality that Yakir is supporting here has ever been tested on
these platforms. (Now, I'm not sure this is extremely important,
since we still can fall back to polled status checks; see
drm_kms_helper_poll_init().)
That's all I've got for now.
Regards,
Brian
[1] https://github.com/mmind/linux-rockchip/commits/tmp/analogixdp-veyron
plus this diff:
diff --git a/arch/arm/boot/dts/rk3288-veyron-jaq.dts
b/arch/arm/boot/dts/rk3288-veyron-jaq.dts
index 5c97e3153526..e77ae4c5531e 100644
--- a/arch/arm/boot/dts/rk3288-veyron-jaq.dts
+++ b/arch/arm/boot/dts/rk3288-veyron-jaq.dts
@@ -88,6 +88,18 @@
};
};
+&backlight {
+ power-supply = <&backlight_regulator>;
+};
+
+&panel {
+ power-supply = <&panel_regulator>;
+};
+
+&edp {
+ analogix,need-force-hpd;
+};
+
&rk808 {
pinctrl-names = "default";
pinctrl-0 = <&pmic_int_l &dvs_1 &dvs_2>;
diff --git a/drivers/phy/phy-rockchip-dp.c b/drivers/phy/phy-rockchip-dp.c
index c82c22f3d0e1..994189f49db5 100644
--- a/drivers/phy/phy-rockchip-dp.c
+++ b/drivers/phy/phy-rockchip-dp.c
@@ -22,7 +22,7 @@
#define GRF_SOC_CON12 0x0274
-#define GRF_EDP_REF_CLK_SEL_INTER_HIWORD_MASK BIT(4)
+#define GRF_EDP_REF_CLK_SEL_INTER_HIWORD_MASK BIT(20)
#define GRF_EDP_REF_CLK_SEL_INTER BIT(4)
#define GRF_EDP_PHY_SIDDQ_HIWORD_MASK BIT(21)
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 10/17] phy: Add driver for rockchip Display Port PHY
Hi Yakir,
On Wed, Nov 04, 2015 at 08:48:38AM +0800, Yakir Yang wrote:
> On 11/03/2015 12:38 PM, Brian Norris wrote:
> >On Thu, Oct 29, 2015 at 09:58:38AM +0800, Yakir Yang wrote:
> >(FYI, I came across this by inspection when comparing Heiko's
> >'somewhat-stable' branch [1] with this series. The former brings up eDP
> >fine on veyron-jaq, whereas this one doesn't yet, even with the above
> >change. Still debugging the issue.)
>
> Hmm... I'm not sure whether your eDP screen have the hotplug signal, so I
I believe hotplug is hooked up but...
> think you can try to add "analogix,force-hpd" flag into
> rk3288-veyron-jaq.dts
>
> &edp {
> analogix,need-force-hpd;
> }
...already tried, just in case. No luck.
> If that changes couldn't fix the problem, guess you may need max the panel
> power up delay time which pointed by Heiko. Like:
>
> Thanks,
> - Yakir
>
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 4fa5f69..546a506 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -82,6 +82,13 @@ static int analogix_dp_detect_hpd(struct
> analogix_dp_device *dp)
> */
> dev_dbg(dp->dev, "failed to get hpd plug status, try to
> force hpd\n");
>
> + /*
> +* Hotplug signal would indicate the right time to operate
> +* panel after poweron, but if the hotplug is missing, then
> +* panel would need wait hundreds of milliseconds at here.
> +*/
> + mdelay(100);
> +
> analogix_dp_force_hpd(dp);
>
> if (analogix_dp_get_plug_in_status(dp) != 0) {
I'll give that a try. Per Heiko's suggestion, I've already hacked around
with adding delays in the regulator node, but this could give slightly
different behavior. I doubt it'll help, but I'll let you know if it
does.
Regards,
Brian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 10/17] phy: Add driver for rockchip Display Port PHY
Hi Yakir,
On Thu, Oct 29, 2015 at 09:58:38AM +0800, Yakir Yang wrote:
> Add phy driver for the Rockchip DisplayPort PHY module. This
> is required to get DisplayPort working in Rockchip SoCs.
>
> Reviewed-by: Heiko Stuebner
> Signed-off-by: Yakir Yang
> ---
...
> diff --git a/drivers/phy/phy-rockchip-dp.c b/drivers/phy/phy-rockchip-dp.c
> new file mode 100644
> index 000..f3e0058
> --- /dev/null
> +++ b/drivers/phy/phy-rockchip-dp.c
> @@ -0,0 +1,151 @@
> +/*
> + * Rockchip DP PHY driver
> + *
> + * Copyright (C) 2015 FuZhou Rockchip Co., Ltd.
> + * Author: Yakir Yang
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#define GRF_SOC_CON12 0x0274
> +
> +#define GRF_EDP_REF_CLK_SEL_INTER_HIWORD_MASK BIT(4)
> +#define GRF_EDP_REF_CLK_SEL_INTER BIT(4)
Why are the above two macros the same? Judging by the RK3288 manual and
other downstream drivers, it seems like you want the _MASK one to be
shifted left by 16 (i.e., BIT(20)).
> +
> +#define GRF_EDP_PHY_SIDDQ_HIWORD_MASK BIT(21)
> +#define GRF_EDP_PHY_SIDDQ_ON0
> +#define GRF_EDP_PHY_SIDDQ_OFF BIT(5)
> +
...
> + ret = regmap_write(dp->grf, GRF_SOC_CON12, GRF_EDP_REF_CLK_SEL_INTER |
> +GRF_EDP_REF_CLK_SEL_INTER_HIWORD_MASK);
IOW, the above is writing:
BIT(4) | BIT(4)
whereas I think you want:
BIT(4) | BIT(20)
> + if (ret != 0) {
> + dev_err(dp->dev, "Could not config GRF edp ref clk: %d\n", ret);
> + return ret;
> + }
> +
...
(FYI, I came across this by inspection when comparing Heiko's
'somewhat-stable' branch [1] with this series. The former brings up eDP
fine on veyron-jaq, whereas this one doesn't yet, even with the above
change. Still debugging the issue.)
Brian
[1] https://github.com/mmind/linux-rockchip/tree/devel/somewhat-stable
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] doc: dt: mtd: support partitions in a special 'partitions' subnode
On Wed, Oct 28, 2015 at 03:48:06PM -0700, Brian Norris wrote: > From: Michal Suchanek > > To avoid conflict with other drivers using subnodes of the mtd device > create only one ofpart-specific node rather than any number of > arbitrary partition subnodes. > > Signed-off-by: Michal Suchanek > Acked-by: Rob Herring > Signed-off-by: Brian Norris Applied to l2-mtd.git -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 5/5] mtd: ofpart: move ofpart partitions to a dedicated dt node
On Tue, Aug 18, 2015 at 03:34:09PM -, Michal Suchanek wrote: > Parsing direct subnodes of a mtd device as partitions is unreliable > since the mtd device is also part of its bus subsystem and can contain > bus data in subnodes. > > Move ofpart data to a subnode of its own so it is clear which data is > part of the partition layout. > > Signed-off-by: Michal Suchanek Applied this patch to l2-mtd.git. Thanks! -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4] mtd: ofpart: document the lock flag.
From: Michal Suchanek The lock flag of ofpart is undocumented. Add to binding doc. Signed-off-by: Michal Suchanek Signed-off-by: Brian Norris Cc: Sascha Hauer --- It seems my reasoning here [1] was wrong (thanks Sascha!). This is a rewrite of Michal's binding doc from [2]. Note that this property is still broken for the CONFIG_MTD_PARTITIONED_MASTER=y case. [1] http://lists.infradead.org/pipermail/linux-mtd/2015-October/062822.html [2] http://lists.infradead.org/pipermail/linux-mtd/2015-August/061155.html Documentation/devicetree/bindings/mtd/partition.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/mtd/partition.txt b/Documentation/devicetree/bindings/mtd/partition.txt index 8e5557da1955..a7d4d35550e3 100644 --- a/Documentation/devicetree/bindings/mtd/partition.txt +++ b/Documentation/devicetree/bindings/mtd/partition.txt @@ -23,6 +23,8 @@ Optional properties: partition should only be mounted read-only. This is usually used for flash partitions containing early-boot firmware images or data which should not be clobbered. +- lock : Do not unlock the partition at initialization time (not supported on + all devices) Examples: -- 2.6.0.rc2.230.g3dd15c0 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 2/3] mtd: brcmnand: Force 8bit mode before doing nand_scan_ident()
On Fri, Oct 30, 2015 at 12:29:20PM +0530, Anup Patel wrote:
> Just like other NAND controllers,
^^ That part isn't strictly true. While READ ID data only comes out on
the lower 8 bits, that doesn't *actually* mean you can't get valid data
from a 16-bit bus in general; you just have to drop the upper 8 bits. That's
what these two commits did for read ID and parameter page read commands:
commit 3dad2344e92c6e1aeae42df1c4824f307c51bcc7
Author: Brian Norris
Date: Wed Jan 29 14:08:12 2014 -0800
mtd: nand: force NAND_CMD_READID onto 8-bit bus
commit bd9c6e99b58255b9de1982711ac9487c9a2f18be
Author: Brian Norris
Date: Fri Nov 29 22:04:28 2013 -0800
mtd: nand: don't use read_buf for 8-bit ONFI transfers
> the NAND READID command only works
> in 8bit mode for all versions of BRCMNAND controller.
But I presume *this* statement is actually true. This NAND controller doesn't
exactly give us a fully-flexible READID / read_byte / read_word command, as it
works at a higher level than that (although LOW_LEVEL_OP gives us this
flexibility now). I could imagine (though I never tested 16-bit NAND) that
16-bit READID is broken.
BTW, did you ask the HW designer about this? It'd be nice to be 100% sure.
Anyway, as I noted on the cover letter, I've pushed this patch.
Thanks,
Brian
> This patch forces 8bit mode for each NAND CS in brcmnand_init_cs()
> before doing nand_scan_ident() to ensure that BRCMNAND controller
> is in 8bit mode when NAND READID command is issued.
>
> Signed-off-by: Anup Patel
> Reviewed-by: Ray Jui
> Reviewed-by: Scott Branden
> ---
> drivers/mtd/nand/brcmnand/brcmnand.c | 10 ++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/mtd/nand/brcmnand/brcmnand.c
> b/drivers/mtd/nand/brcmnand/brcmnand.c
> index dda96fa..b410527 100644
> --- a/drivers/mtd/nand/brcmnand/brcmnand.c
> +++ b/drivers/mtd/nand/brcmnand/brcmnand.c
> @@ -1912,6 +1912,7 @@ static int brcmnand_init_cs(struct brcmnand_host *host)
> struct mtd_info *mtd;
> struct nand_chip *chip;
> int ret;
> + u16 cfg_offs;
> struct mtd_part_parser_data ppdata = { .of_node = dn };
>
> ret = of_property_read_u32(dn, "reg", &host->cs);
> @@ -1954,6 +1955,15 @@ static int brcmnand_init_cs(struct brcmnand_host *host)
>
> chip->controller = &ctrl->controller;
>
> + /*
> + * The bootloader might have configured 16bit mode but
> + * NAND READID command only works in 8bit mode. We force
> + * 8bit mode here to ensure that NAND READID commands works.
> + */
> + cfg_offs = brcmnand_cs_offset(ctrl, host->cs, BRCMNAND_CS_CFG);
> + nand_writereg(ctrl, cfg_offs,
> + nand_readreg(ctrl, cfg_offs) & ~CFG_BUS_WIDTH);
> +
> if (nand_scan_ident(mtd, 1, NULL))
> return -ENXIO;
>
> --
> 1.9.1
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 0/3] NAND support for Broadcom NS2 SoC
On Fri, Oct 30, 2015 at 12:29:18PM +0530, Anup Patel wrote: > We enable NAND support for Broadcom NS2 SoC by reusing existing > BRCMNAND driver. > > This patchset applies on-top of "arm64: Simple additions to > NS2 DT" v1 patchset and is available in ns2_nand_v5 branch of > https://github.com/Broadcom/arm64-linux.git. > > The patchset is tested on NS2 SVK. Applied patches 1 and 2. My "Reviewed-by" on patch 3 stands. Thanks, Brian -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/2] arm64: dts: Add BRCM IPROC NAND DT node for NS2
On Wed, Oct 28, 2015 at 09:08:02AM -0700, Ray Jui wrote: > On 10/28/2015 2:06 AM, Anup Patel wrote: > > > >I think for a newly created OF devices the Linux device driver framework will > >match the platform drivers in the order in which they are registered by > >module > >init functions. Now the order of module init function calls will be based how > >the .initcall section is formed by linker and order in which objects are > >linked. > > > > Yes, what you said is my understanding as well, but then here is the > mystery. This is the link order in brcmnand/Makefile: > > 1 # link order matters; don't link the more generic brcmstb_nand.o > before the > 2 # more specific iproc_nand.o, for instance > 3 obj-$(CONFIG_MTD_NAND_BRCMNAND) += iproc_nand.o > 4 obj-$(CONFIG_MTD_NAND_BRCMNAND) += bcm63138_nand.o > 5 obj-$(CONFIG_MTD_NAND_BRCMNAND) += brcmstb_nand.o > 6 obj-$(CONFIG_MTD_NAND_BRCMNAND) += brcmnand.o > > Based on the order above, probe from iproc_nand should always be > called first if a matching compatible string is found. If so, then > why having both compatible strings "brcm,brcmnand" and > "brcm,nand-iproc" causes issues for NS2 (I remember it broke > smoketest in the past when you submitted the change)? I'm not saying > we should have "brcm,brcmnand" for iProc devices, but I don't get > why it would cause any issue. FWIW, the above hack doesn't do anything if these are built as modules, AFAICT. So I guess udev's (or similar) module rules would be another point of failure here? Not that any of us probably care too much about this driver as a module, but just throwing it out there... I have a feeling we'll have to solve this locally, by avoiding having "independent" drivers handling similar compatible properties, as I expect (despite our expectation that compatible ordering should matter) this problem will not be solved any time soon in the generic infrastructure. Or we can just use a hack (as Anup is doing) to leave off the "brcm,brcmnand" compatibility property. Unless someone has brilliant ideas, I guess we go with Anup's hack for now. Brian -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 2/3] mtd: mtk-nor: mtk serial flash controller driver
Hi Bayi,
In reviewing your updated instructions on how to read 6 bytes of ID, I
have one more question about how the PRGDATA and SHREG registers are
supposed to work.
On Mon, Oct 26, 2015 at 09:40:38PM +0800, Bayi Cheng wrote:
[...]
> +static void mt8173_nor_set_read_mode(struct mt8173_nor *mt8173_nor)
> +{
> + struct spi_nor *nor = &mt8173_nor->nor;
> +
> + writeb(nor->read_opcode, mt8173_nor->base + MTK_NOR_PRGDATA3_REG);
So far, I've generalized your code to say that except for a few commands
that are treated specially by your controller, all other opcodes are
queued up in the program/shift register this:
(1) total number of bits to send/receive goes in the COUNT register (so
far, as many as 7*8=56?)
(2) opcode is written to PRGDATA5
(3) other "transmit" data (like addresses), if any, are placed on PRGDATA4..0
(4) command is sent (execute_cmd())
(5) data is read back in SHREG{X..0}, if needed
However, I see that
(a) here in mt8173_nor_set_read_mode(), you use only PRGDATA3 (i.e.,
this is different than step (2) above). Why is that different? Is this
just a quirk of the read mode, which is different than the other
arbitrary opcodes (like READ ID)?
(b) it's really unclear how to determine 'X' in step (5). It seems like
it might just be the number of bytes minus 1, since the first byte
aligns with the "opcode" cycle. But I'm not really sure, since in one
case (the 'default' in mt8173_nor_read_reg()) you seemingly randomly
chose to read *only* from SHREG2. That looks wrong to me, but I don't
actually know, because your SoC is very unclear.
My patch at the end of this [1] tries to encapsulate (1)-(5) as best as
I could, but it's really missing out on the answer to (b).
Anyway, I'd really appreciate a good answer to these questions. We
really need to get this stuff right, so your driver can handle NOR flash
opcodes as generically as possible. Right now, your driver mostly looks
like a series of cobbled together switch/case statements to handle the
couple of opcodes you've tested.
Regard,
Brian
[1] http://lists.infradead.org/pipermail/linux-mtd/2015-October/062919.html
> + switch (nor->flash_read) {
> + case SPI_NOR_FAST:
> + writeb(MTK_NOR_FAST_READ, mt8173_nor->base +
> +MTK_NOR_CFG1_REG);
> + break;
> + case SPI_NOR_DUAL:
> + writeb(MTK_NOR_DUAL_READ_EN, mt8173_nor->base +
> +MTK_NOR_DUAL_REG);
> + break;
> + case SPI_NOR_QUAD:
> + writeb(MTK_NOR_QUAD_READ_EN, mt8173_nor->base +
> +MTK_NOR_DUAL_REG);
> + break;
> + default:
> + writeb(MTK_NOR_DUAL_DISABLE, mt8173_nor->base +
> +MTK_NOR_DUAL_REG);
> + break;
> + }
> +}
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] mtd: create a partition type device tree binding
On Fri, Oct 30, 2015 at 03:00:57PM +0100, Linus Walleij wrote: > On Thu, Oct 29, 2015 at 5:29 PM, Brian Norris > wrote: > > >> +Required properties: > >> +- partition-type : the type of partition. Only one type can be specified. > > > > You're supporting this as a list property (for future expansion, > > presumably), so I can only assume the "only one type" is referring to > > the number of different parsers available currently, not the behavior of > > the property itself? > > I was thinking that it would not be a list actually. > > The reason being that if you're anyways going to the trouble of > specifying exactly what partition type is going to be used, you're > not really interested in specifying a few different ones, you know > exactly what type it is going to be. OK, that makes sense. I think it's still *possible* that a board might have the option of more than one partition parser, and so they might just include both in the DTS, but that seems unlikely and so it makes sense not to (over)engineer for it before it's needed. Anyway, your binding can easily be expanded in the future if needed. > But if you think it needs to be a list, I'll specify that, no big > deal. I'll also try to get the Linux code to handle a list then. No, I think your proposal is OK then. It's possible there is some room for clarification in the binding, since I was confused, but that could mostly be attributed to my pre-existing assumptions, not a fault of the text. > > Also, this patch will conflict with [1]. I'll probably take [1] soon, so > > one of us will have to rebase this. > > Sure I'll rebase on whatever you say. OK, I'll let you know. Brian -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 2/3] mtd: mtk-nor: mtk serial flash controller driver
Hi Bayi, On Fri, Oct 30, 2015 at 10:12:39AM +0800, bayi.cheng wrote: > Hi Brian, The current station is as follows. > > 1: put 0x9F to MTK_NOR_PRGDATA5_REG, and five 0x0 to > MTK_NOR_PRGDATA4_REG ~ MTK_NOR_PRGDATA0_REG, then set (1 + 5) * 8 > to MTK_NOR_CNT_REG, for this way, we can read five IDs. > > 2: put 0x9F to MTK_NOR_PRGDATA5_REG, and five 0x0 to > MTK_NOR_PRGDATA4_REG ~ MTK_NOR_PRGDATA0_REG, then set (1 + 5 + 1) * 8 > to MTK_NOR_CNT_REG, for this way, we can read six IDs. > In this case, nor flash IDs can be read from MTK_NOR_SHREG5_REG to > MTK_NOR_SHREG0_REG . Thanks! Thanks for the update. Glad to hear you can read more bytes there; the extra number of Shift Registers *looked* to me like you should be able to read even more than 5... So in my other email, I showed you how I generalized the TX/RX function, so it can handle most of both the write_reg() and read_reg() functions. I'm not 100% sure now that it has all of the RX path correct; it worked for the cases I could test, but it's confusing when reading the manual to figure out which SHREG register I should start from. But anyway, I think it should only take some small adjustments to my patch to make it handle things properly. I'd really appreciate it if you could incorporate my feedback and review/improve the ..._do_tx_rx() function I wrote, to make sure it handles reading any arbitrary number of bytes (at least up to 6). So, you might, for example, run some tests where you have spi-nor.c call nor->read_reg(nor, SPINOR_OP_RDID, id, idlen) with varying values for 'idlen', and make sure they all work properly. Let me know if you have any questions about comments either here, or on v5. Regards, Brian -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 2/3] mtd: mtk-nor: mtk serial flash controller driver
Hi Bayi, (I drafted most of this before you clarified on how to read out 6 bytes of ID. So a bit of this is slightly out-of-date. Still, I think most of the comments and much of the appended patch should be useful to you.) On Mon, Oct 26, 2015 at 09:40:38PM +0800, Bayi Cheng wrote: > add spi nor flash driver for mediatek controller > > Signed-off-by: Bayi Cheng > --- > drivers/mtd/spi-nor/Kconfig | 7 + > drivers/mtd/spi-nor/Makefile | 1 + > drivers/mtd/spi-nor/mtk-quadspi.c | 491 > ++ > 3 files changed, 499 insertions(+) > create mode 100644 drivers/mtd/spi-nor/mtk-quadspi.c > > diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig > index 89bf4c1..387396d 100644 > --- a/drivers/mtd/spi-nor/Kconfig > +++ b/drivers/mtd/spi-nor/Kconfig > @@ -7,6 +7,13 @@ menuconfig MTD_SPI_NOR > > if MTD_SPI_NOR > > +config MTD_MT81xx_NOR > + tristate "Mediatek MT81xx SPI NOR flash controller" > + help > + This enables access to SPI NOR flash, using MT81xx SPI NOR flash > + controller. This controller does not support generic SPI BUS, It only Nit: s/It/it/ > + supports SPI NOR Flash. > + > config MTD_SPI_NOR_USE_4K_SECTORS > bool "Use small 4096 B erase sectors" > default y > diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile > index e5e..0bf3a7f8 100644 > --- a/drivers/mtd/spi-nor/Makefile > +++ b/drivers/mtd/spi-nor/Makefile > @@ -1,3 +1,4 @@ > obj-$(CONFIG_MTD_SPI_NOR)+= spi-nor.o > obj-$(CONFIG_SPI_FSL_QUADSPI)+= fsl-quadspi.o > +obj-$(CONFIG_MTD_MT81xx_NOR)+= mtk-quadspi.o > obj-$(CONFIG_SPI_NXP_SPIFI) += nxp-spifi.o > diff --git a/drivers/mtd/spi-nor/mtk-quadspi.c > b/drivers/mtd/spi-nor/mtk-quadspi.c > new file mode 100644 > index 000..33a8dc5 > --- /dev/null > +++ b/drivers/mtd/spi-nor/mtk-quadspi.c > @@ -0,0 +1,491 @@ > +/* > + * Copyright (c) 2015 MediaTek Inc. > + * Author: Bayi Cheng > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include You're not using GPIOs. You don't need this header. > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define MTK_NOR_CMD_REG 0x00 > +#define MTK_NOR_CNT_REG 0x04 > +#define MTK_NOR_RDSR_REG 0x08 > +#define MTK_NOR_RDATA_REG0x0c > +#define MTK_NOR_RADR0_REG0x10 > +#define MTK_NOR_RADR1_REG0x14 > +#define MTK_NOR_RADR2_REG0x18 > +#define MTK_NOR_WDATA_REG0x1c > +#define MTK_NOR_PRGDATA0_REG 0x20 > +#define MTK_NOR_PRGDATA1_REG 0x24 > +#define MTK_NOR_PRGDATA2_REG 0x28 > +#define MTK_NOR_PRGDATA3_REG 0x2c > +#define MTK_NOR_PRGDATA4_REG 0x30 > +#define MTK_NOR_PRGDATA5_REG 0x34 > +#define MTK_NOR_SHREG0_REG 0x38 > +#define MTK_NOR_SHREG1_REG 0x3c > +#define MTK_NOR_SHREG2_REG 0x40 > +#define MTK_NOR_SHREG3_REG 0x44 > +#define MTK_NOR_SHREG4_REG 0x48 > +#define MTK_NOR_SHREG5_REG 0x4c > +#define MTK_NOR_SHREG6_REG 0x50 > +#define MTK_NOR_SHREG7_REG 0x54 > +#define MTK_NOR_SHREG8_REG 0x58 > +#define MTK_NOR_SHREG9_REG 0x5c > +#define MTK_NOR_CFG1_REG 0x60 > +#define MTK_NOR_CFG2_REG 0x64 > +#define MTK_NOR_CFG3_REG 0x68 > +#define MTK_NOR_STATUS0_REG 0x70 > +#define MTK_NOR_STATUS1_REG 0x74 > +#define MTK_NOR_STATUS2_REG 0x78 > +#define MTK_NOR_STATUS3_REG 0x7c > +#define MTK_NOR_FLHCFG_REG 0x84 > +#define MTK_NOR_TIME_REG 0x94 > +#define MTK_NOR_PP_DATA_REG 0x98 > +#define MTK_NOR_PREBUF_STUS_REG 0x9c > +#define MTK_NOR_DELSEL0_REG 0xa0 > +#define MTK_NOR_DELSEL1_REG 0xa4 > +#define MTK_NOR_INTRSTUS_REG 0xa8 > +#define MTK_NOR_INTREN_REG 0xac > +#define MTK_NOR_CHKSUM_CTL_REG 0xb8 > +#define MTK_NOR_CHKSUM_REG 0xbc > +#define MTK_NOR_CMD2_REG 0xc0 > +#define MTK_NOR_WRPROT_REG 0xc4 > +#define MTK_NOR_RADR3_REG0xc8 > +#define MTK_NOR_DUAL_REG 0xcc > +#define MTK_NOR_DELSEL2_REG 0xd0 > +#define MTK_NOR_DELSEL3_REG 0xd4 > +#define MTK_NOR_DELSEL4_REG 0xd8 > + > +/* commands for mtk
Re: [PATCH 1/3] mtd: create a partition type device tree binding
+ devicetree-spec and others, since other non-driver-specific MTD
partitioning stuff has gotten directed there [1], and this binding
should probably get promoted in short order to the core mtdpart.c
partitioning
On Thu, Oct 29, 2015 at 01:52:30PM +0100, Linus Walleij wrote:
> The Linux code in drivers/mtd/maps/physmap_of.c will optionally
> look for the linux,part-probe binding for hints on a partition type to
> look for in the MTD. It was added in commit 9d5da3a9b849
> "mtd: extend physmap_of to let the device tree specify the parition probe"
>
> This solution is too Linux-specific and depend on some
> Linux kernel-internal naming conventions. We need a proper
> way of describing partition types that follow the pattern set by
> other device tree bindings.
>
> Create a "partition-type" binding for this, and add "my" bindings
> for "arm,arm-flash-structure" as a starter for others to follow.
> A follow-on patch adds support to the Linux kernel to handle this
> binding, with some infrastructure for others to use it too.
Overall, I like this. And thanks for expanding the explanation of DT
partitions vs. parsers. It'd be good if we make parsers like this easier
to integrate, so we see less hard-coded partitions in device tree.
> Cc: devicetree@vger.kernel.org
> Cc: Jason Gunthorpe
> Cc: Liviu Dudau
> Reported-by: Liviu Dudau
> Signed-off-by: Linus Walleij
> ---
> .../devicetree/bindings/mtd/mtd-physmap.txt| 2 ++
> .../devicetree/bindings/mtd/partition.txt | 35
> +++---
> 2 files changed, 33 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mtd/mtd-physmap.txt
> b/Documentation/devicetree/bindings/mtd/mtd-physmap.txt
> index 4a0a48bf4ecb..863560bdbb19 100644
> --- a/Documentation/devicetree/bindings/mtd/mtd-physmap.txt
> +++ b/Documentation/devicetree/bindings/mtd/mtd-physmap.txt
> @@ -23,6 +23,8 @@ file systems on embedded devices.
> unaligned accesses as implemented in the JFFS2 code via memcpy().
> By defining "no-unaligned-direct-access", the flash will not be
> exposed directly to the MTD users (e.g. JFFS2) any more.
> + - partition-type : a flash partition type to expect and probe for
> + on this device. See "partition.txt" for available partition types.
> - linux,mtd-name: allow to specify the mtd name for retro capability with
> physmap-flash drivers as boot loader pass the mtd partition via the old
> device name physmap-flash.
> diff --git a/Documentation/devicetree/bindings/mtd/partition.txt
> b/Documentation/devicetree/bindings/mtd/partition.txt
> index 8e5557da1955..85d45764a4b3 100644
> --- a/Documentation/devicetree/bindings/mtd/partition.txt
> +++ b/Documentation/devicetree/bindings/mtd/partition.txt
> @@ -1,9 +1,36 @@
> Representing flash partitions in devicetree
>
> -Partitions can be represented by sub-nodes of an mtd device. This can be used
> -on platforms which have strong conventions about which portions of a flash
> are
> -used for what purposes, but which don't use an on-flash partition table such
> -as RedBoot.
> +On-device partition types:
> +
> +It is possible for some drivers to indicate an on-device partition type, i.e.
> +partition tables, footers or other binary pattern in the flash used to
> +define how the flash is partitioned. This can be done in the device tree node
> +of an MTD device by specifying partition-type = "foo"; This tells the
> operating
> +system to look for the partition type indicated.
> +
> +Required properties:
> +- partition-type : the type of partition. Only one type can be specified.
You're supporting this as a list property (for future expansion,
presumably), so I can only assume the "only one type" is referring to
the number of different parsers available currently, not the behavior of
the property itself? This should probably be worded differently if
that's the intention. Like, "a list of partition types ... ordered by
priority We currently support the following: (list only the AFS
type)" (fill in the blanks how you'd like).
Also, this patch will conflict with [1]. I'll probably take [1] soon, so
one of us will have to rebase this.
Brian
[1] http://thread.gmane.org/gmane.comp.devicetree.spec/191
> + Valid types are:
> + "arm,arm-flash-structure" (also called AFS)
> +
> +Example:
> +
> +flash0@4000 {
> + /* 2 * 32MiB NOR Flash memory */
> + compatible = "arm,vexpress-flash", "cfi-flash";
> + partition-type = "arm,arm-flash-structure";
> + reg = <0x4000 0x0400>;
> + bank-width = <4>;
> +};
> +
> +
> +Device Tree specified partitions:
> +
> +If there is no specified on-device binary format, partitions can be
> +represented by sub-nodes of an mtd device. This can be used on platforms
> which
> +have strong conventions about which portions of a flash are used for what
> +purposes.
> +
> NOTE: if the sub-node has a compatible string, then it is not a partition.
>
> #address-cells & #size-cells must both
Re: [PATCH v4 2/3] mtd: mtk-nor: mtk serial flash controller driver
On Thu, Oct 29, 2015 at 09:56:31PM +0800, bayi.cheng wrote:
> On Thu, 2015-10-29 at 11:28 +0800, bayi.cheng wrote:
> > On Wed, 2015-10-28 at 18:52 -0700, Brian Norris wrote:
> > > On Sun, Oct 18, 2015 at 10:20:35PM +0800, bayi.cheng wrote:
> > > > On Fri, 2015-10-16 at 00:39 -0700, Brian Norris wrote:
> > > > > On Tue, Oct 13, 2015 at 05:39:19PM +0800, Bayi Cheng wrote:
> > > > > > + /* mtk nor controller doesn't supoort SPINOR_OP_RDCR */
> > > > > > + switch (opcode) {
> > > > > > + case SPINOR_OP_RDID:
> > > > > > + /* read JEDEC ID need 4 bytes commands */
> > > > > > + ret = mt8173_nor_set_cmd(mt8173_nor, 0, 32,
> > > > > > SPINOR_OP_RDID);
> > > > > > + if (ret < 0)
> > > > > > + return ret;
> > > > > > +
> > > > > > + /* mtk nor flash controller only support 3 bytes IDs */
> > > > >
> > > > > Are you absolutely sure of this? That would be highly unfortunate, but
> > > > > I also don't believe it's true.
> > > > >
> > > > Yes, for this issue I have asked our designer of nor flash controller,
> > > > unfortunately, it is true, and I have tried to read more IDs, but our
> > > > controller just accept 3 IDs from nor flash, and our next generation IC
> > > > may solve this problem.
> > >
> > > How exactly did you try? Did you do what I suggested above? Are the
> > > "shift registers" not all actually functional?
> > >
> > Hi Brian, For this problem, I have asked our nor controller designer to
> > double confirm again, and he promise to do some simulation testes, On
> > the other hand, I have got some Spansion nor flash which has 5 IDs, and
> > after our designer make sure our controller can support 5 or 6 IDs, then
> > I will rework a special platform with S25FL256SA. If we have any
> > progress, I will inform you immediately! Thanks!!
> >
> Hi Brian, Thanks very much for your perseverance!!
> Actually, You idea is correct, and our designer has completed some
> simulation test on spansion nor flash witch has more than 3 IDs, and the
> results has prove that we can support to read 5 IDs. I also double
Awesome!
> confirmed with spansion nor flash. The only pity is that our nor
> controller can't support 6 IDs, we just support 5 IDs.
That's not quite as awesome. But that's still much more workable than
only 3 bytes. Perhaps we'll want a way to communicate that to the
spi-nor layer, so it doesn't think it can match a full 6 bytes?
> I will submit my changes to V6 version. Thanks again!!
OK, but let me take another look at v5 and send you comments on that one
first.
Regards,
Brian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 2/3] mtd: mtk-nor: mtk serial flash controller driver
Hi Bayi,
I'm looking over your v5, and I still have a lot of comments. I'll send
those soon. But I still question one of your responses here:
On Sun, Oct 18, 2015 at 10:20:35PM +0800, bayi.cheng wrote:
> On Fri, 2015-10-16 at 00:39 -0700, Brian Norris wrote:
> > On Tue, Oct 13, 2015 at 05:39:19PM +0800, Bayi Cheng wrote:
> > > diff --git a/drivers/mtd/spi-nor/mtk-quadspi.c
> > > b/drivers/mtd/spi-nor/mtk-quadspi.c
> > > new file mode 100644
> > > index 000..c6ac366
> > > --- /dev/null
> > > +++ b/drivers/mtd/spi-nor/mtk-quadspi.c
> > > @@ -0,0 +1,486 @@
[...]
> > > +#define MTK_NOR_CMD_REG 0x00
> > > +#define MTK_NOR_CNT_REG 0x04
> > > +#define MTK_NOR_RDSR_REG 0x08
> > > +#define MTK_NOR_RDATA_REG0x0c
> > > +#define MTK_NOR_RADR0_REG0x10
> > > +#define MTK_NOR_RADR1_REG0x14
> > > +#define MTK_NOR_RADR2_REG0x18
> > > +#define MTK_NOR_WDATA_REG0x1c
> > > +#define MTK_NOR_PRGDATA0_REG 0x20
> > > +#define MTK_NOR_PRGDATA1_REG 0x24
> > > +#define MTK_NOR_PRGDATA2_REG 0x28
> > > +#define MTK_NOR_PRGDATA3_REG 0x2c
> > > +#define MTK_NOR_PRGDATA4_REG 0x30
> > > +#define MTK_NOR_PRGDATA5_REG 0x34
^^ so you have 6 "TX" registers.
> > > +#define MTK_NOR_SHREG0_REG 0x38
> > > +#define MTK_NOR_SHREG1_REG 0x3c
> > > +#define MTK_NOR_SHREG2_REG 0x40
> > > +#define MTK_NOR_SHREG3_REG 0x44
> > > +#define MTK_NOR_SHREG4_REG 0x48
> > > +#define MTK_NOR_SHREG5_REG 0x4c
^^ and you have at least 6 "RX" registers (looking at the mt8173 manual,
I'm not sure what the SHREG{6..9} are for). So I don't see why you can't
program MTK_NOR_CNT_REG to 48 (6 bytes * 8 bits/byte) to get 6 bytes
TX/RX, or IOW, 1 byte of opcode and 5 bytes of either RX or TX data. So
I'm still not convinced about your claim below.
I was able to rewrite your driver to do the above, and I can read out 5
bytes no problem, but unfortunately, the flash device I have has only 3
bytes of ID, so the last two bytes are zero, which doesn't tell me too
much if this is working right.
> > > +#define MTK_NOR_SHREG6_REG 0x50
> > > +#define MTK_NOR_SHREG7_REG 0x54
> > > +#define MTK_NOR_SHREG8_REG 0x58
> > > +#define MTK_NOR_SHREG9_REG 0x5c
> > > +#define MTK_NOR_CFG1_REG 0x60
> > > +#define MTK_NOR_CFG2_REG 0x64
> > > +#define MTK_NOR_CFG3_REG 0x68
> > > +#define MTK_NOR_STATUS0_REG 0x70
> > > +#define MTK_NOR_STATUS1_REG 0x74
> > > +#define MTK_NOR_STATUS2_REG 0x78
> > > +#define MTK_NOR_STATUS3_REG 0x7c
> > > +#define MTK_NOR_FLHCFG_REG 0x84
> > > +#define MTK_NOR_TIME_REG 0x94
> > > +#define MTK_NOR_PP_DATA_REG 0x98
> > > +#define MTK_NOR_PREBUF_STUS_REG 0x9c
> > > +#define MTK_NOR_DELSEL0_REG 0xa0
> > > +#define MTK_NOR_DELSEL1_REG 0xa4
> > > +#define MTK_NOR_INTRSTUS_REG 0xa8
> > > +#define MTK_NOR_INTREN_REG 0xac
> > > +#define MTK_NOR_CHKSUM_CTL_REG 0xb8
> > > +#define MTK_NOR_CHKSUM_REG 0xbc
> > > +#define MTK_NOR_CMD2_REG 0xc0
> > > +#define MTK_NOR_WRPROT_REG 0xc4
> > > +#define MTK_NOR_RADR3_REG0xc8
> > > +#define MTK_NOR_DUAL_REG 0xcc
> > > +#define MTK_NOR_DELSEL2_REG 0xd0
> > > +#define MTK_NOR_DELSEL3_REG 0xd4
> > > +#define MTK_NOR_DELSEL4_REG 0xd8
...
> > > +static int mt8173_nor_read_reg(struct spi_nor *nor, u8 opcode, u8 *buf,
> > > int len)
> > > +{
> > > + int ret;
> > > + struct mt8173_nor *mt8173_nor = nor->priv;
> > > +
> > > + /* mtk nor controller doesn't supoort SPINOR_OP_RDCR */
> > > + switch (opcode) {
> > > + case SPINOR_OP_RDID:
> > > + /* read JEDEC ID need 4 bytes commands */
> > > + ret = mt8173_nor_set_cmd(mt8173_nor, 0, 32, SPINOR_OP_RDID);
> > > + if (ret < 0)
> > > + return ret;
> > > +
> > > + /* mtk nor flash controller only support 3 bytes IDs */
> >
> > Are yo
Re: [PATCH v4] doc: dt: mtd: support partitions in a special 'partitions' subnode
On Wed, Oct 28, 2015 at 03:48:06PM -0700, Brian Norris wrote:
> From: Michal Suchanek
>
> To avoid conflict with other drivers using subnodes of the mtd device
> create only one ofpart-specific node rather than any number of
> arbitrary partition subnodes.
>
> Signed-off-by: Michal Suchanek
> Acked-by: Rob Herring
> Signed-off-by: Brian Norris
> ---
Ugh, I wrote this up to stick below the '---' but I forgot to send it
out with the patch. Here goes:
This is a resend with only slight modifications of Michal's v3 patch, for
clarity:
http://thread.gmane.org/gmane.linux.drivers.mtd/60890/focus=60889
It's been reviewed by me and Rob and I plan to take it for v4.4. But at Rob's
suggestion, I'm sending this to the devicetree-spec list too, as this is a
binding for somewhat generic infrastructure, rather than just drivers.
Regards,
Brian
> .../devicetree/bindings/mtd/partition.txt | 71
> +-
> 1 file changed, 42 insertions(+), 29 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mtd/partition.txt
> b/Documentation/devicetree/bindings/mtd/partition.txt
> index 8e5557da1955..f1e2a02381a4 100644
> --- a/Documentation/devicetree/bindings/mtd/partition.txt
> +++ b/Documentation/devicetree/bindings/mtd/partition.txt
> @@ -4,10 +4,17 @@ Partitions can be represented by sub-nodes of an mtd
> device. This can be used
> on platforms which have strong conventions about which portions of a flash
> are
> used for what purposes, but which don't use an on-flash partition table such
> as RedBoot.
> -NOTE: if the sub-node has a compatible string, then it is not a partition.
>
> -#address-cells & #size-cells must both be present in the mtd device. There
> are
> -two valid values for both:
> +The partition table should be a subnode of the mtd node and should be named
> +'partitions'. Partitions are defined in subnodes of the partitions node.
> +
> +For backwards compatibility partitions as direct subnodes of the mtd device
> are
> +supported. This use is discouraged.
> +NOTE: also for backwards compatibility, direct subnodes that have a
> compatible
> +string are not considered partitions, as they may be used for other bindings.
> +
> +#address-cells & #size-cells must both be present in the partitions subnode
> of the
> +mtd device. There are two valid values for both:
> <1>: for partitions that require a single 32-bit cell to represent their
> size/address (aka the value is below 4 GiB)
> <2>: for partitions that require two 32-bit cells to represent their
> @@ -28,44 +35,50 @@ Examples:
>
>
> flash@0 {
> - #address-cells = <1>;
> - #size-cells = <1>;
> + partitions {
> + #address-cells = <1>;
> + #size-cells = <1>;
>
> - partition@0 {
> - label = "u-boot";
> - reg = <0x000 0x10>;
> - read-only;
> - };
> + partition@0 {
> + label = "u-boot";
> + reg = <0x000 0x10>;
> + read-only;
> + };
>
> - uimage@10 {
> - reg = <0x010 0x20>;
> + uimage@10 {
> + reg = <0x010 0x20>;
> + };
> };
> };
>
> flash@1 {
> - #address-cells = <1>;
> - #size-cells = <2>;
> + partitions {
> + #address-cells = <1>;
> + #size-cells = <2>;
>
> - /* a 4 GiB partition */
> - partition@0 {
> - label = "filesystem";
> - reg = <0x 0x1 0x>;
> + /* a 4 GiB partition */
> + partition@0 {
> + label = "filesystem";
> + reg = <0x 0x1 0x>;
> + };
> };
> };
>
> flash@2 {
> - #address-cells = <2>;
> - #size-cells = <2>;
> + partitions {
> + #address-cells = <2>;
> + #size-cells = <2>;
>
> - /* an 8 GiB partition */
> - partition@0 {
> - label = "filesystem #1";
> - reg = <0x0 0x 0x2 0x>;
> - };
> + /* an 8 GiB partition */
> + partition@0 {
> + label = "filesystem #1";
> + reg = <0x0 0x 0x2 0x>;
> + };
>
> - /* a 4 GiB partition */
> - partition@2 {
> - label = "filesystem #2";
> - reg = <0x2 0x 0x1 0x>;
> + /* a 4 GiB partition */
> + partition@2 {
> + label = "filesystem #2";
> + reg = <0x2 0x 0x1 0x>;
> + };
> };
> };
> --
> 2.6.0.rc2.230.g3dd15c0
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4] doc: dt: mtd: support partitions in a special 'partitions' subnode
From: Michal Suchanek
To avoid conflict with other drivers using subnodes of the mtd device
create only one ofpart-specific node rather than any number of
arbitrary partition subnodes.
Signed-off-by: Michal Suchanek
Acked-by: Rob Herring
Signed-off-by: Brian Norris
---
.../devicetree/bindings/mtd/partition.txt | 71 +-
1 file changed, 42 insertions(+), 29 deletions(-)
diff --git a/Documentation/devicetree/bindings/mtd/partition.txt
b/Documentation/devicetree/bindings/mtd/partition.txt
index 8e5557da1955..f1e2a02381a4 100644
--- a/Documentation/devicetree/bindings/mtd/partition.txt
+++ b/Documentation/devicetree/bindings/mtd/partition.txt
@@ -4,10 +4,17 @@ Partitions can be represented by sub-nodes of an mtd device.
This can be used
on platforms which have strong conventions about which portions of a flash are
used for what purposes, but which don't use an on-flash partition table such
as RedBoot.
-NOTE: if the sub-node has a compatible string, then it is not a partition.
-#address-cells & #size-cells must both be present in the mtd device. There are
-two valid values for both:
+The partition table should be a subnode of the mtd node and should be named
+'partitions'. Partitions are defined in subnodes of the partitions node.
+
+For backwards compatibility partitions as direct subnodes of the mtd device are
+supported. This use is discouraged.
+NOTE: also for backwards compatibility, direct subnodes that have a compatible
+string are not considered partitions, as they may be used for other bindings.
+
+#address-cells & #size-cells must both be present in the partitions subnode of
the
+mtd device. There are two valid values for both:
<1>: for partitions that require a single 32-bit cell to represent their
size/address (aka the value is below 4 GiB)
<2>: for partitions that require two 32-bit cells to represent their
@@ -28,44 +35,50 @@ Examples:
flash@0 {
- #address-cells = <1>;
- #size-cells = <1>;
+ partitions {
+ #address-cells = <1>;
+ #size-cells = <1>;
- partition@0 {
- label = "u-boot";
- reg = <0x000 0x10>;
- read-only;
- };
+ partition@0 {
+ label = "u-boot";
+ reg = <0x000 0x10>;
+ read-only;
+ };
- uimage@10 {
- reg = <0x010 0x20>;
+ uimage@10 {
+ reg = <0x010 0x20>;
+ };
};
};
flash@1 {
- #address-cells = <1>;
- #size-cells = <2>;
+ partitions {
+ #address-cells = <1>;
+ #size-cells = <2>;
- /* a 4 GiB partition */
- partition@0 {
- label = "filesystem";
- reg = <0x 0x1 0x>;
+ /* a 4 GiB partition */
+ partition@0 {
+ label = "filesystem";
+ reg = <0x 0x1 0x>;
+ };
};
};
flash@2 {
- #address-cells = <2>;
- #size-cells = <2>;
+ partitions {
+ #address-cells = <2>;
+ #size-cells = <2>;
- /* an 8 GiB partition */
- partition@0 {
- label = "filesystem #1";
- reg = <0x0 0x 0x2 0x>;
- };
+ /* an 8 GiB partition */
+ partition@0 {
+ label = "filesystem #1";
+ reg = <0x0 0x 0x2 0x>;
+ };
- /* a 4 GiB partition */
- partition@2 {
- label = "filesystem #2";
- reg = <0x2 0x 0x1 0x>;
+ /* a 4 GiB partition */
+ partition@2 {
+ label = "filesystem #2";
+ reg = <0x2 0x 0x1 0x>;
+ };
};
};
--
2.6.0.rc2.230.g3dd15c0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/2] arm64: dts: Add BRCM IPROC NAND DT node for NS2
On Tue, Oct 27, 2015 at 05:25:32PM -0700, Ray Jui wrote:
> On 10/27/2015 5:19 PM, Brian Norris wrote:
> >On Fri, Oct 23, 2015 at 10:46:13AM +0530, Anup Patel wrote:
> >>diff --git a/arch/arm64/boot/dts/broadcom/ns2.dtsi
> >>b/arch/arm64/boot/dts/broadcom/ns2.dtsi
> >>index f603277..9610822 100644
> >>--- a/arch/arm64/boot/dts/broadcom/ns2.dtsi
> >>+++ b/arch/arm64/boot/dts/broadcom/ns2.dtsi
> >>@@ -212,5 +212,19 @@
> >>compatible = "brcm,iproc-rng200";
> >>reg = <0x6622 0x28>;
> >>};
> >>+
> >>+ nand: nand@6646 {
> >>+ compatible = "brcm,nand-iproc", "brcm,brcmnand-v6.1";
> >
> >Technically, the binding says you should also have "brcm,brcmnand" as a
> >last resort. Otherwise (for the NAND parts):
> >
>
> I believe Anup was seeing issues when both "brcm,nand-iproc" and
> "brcm,brcmnand" are present.
>
> Note "brcm,nand-iproc" invokes 'iproc_nand_probe', which calls
> 'brcmnand_probe' in the end.
>
> "brcm,brcmnand" invokes 'brcmstb_nand_probe', which also calls
> 'brcmstb_probe', but without all the prep configuration required for
> "brcm,nand-iproc".
Ah, I forgot about that problem. That seems like an OF infrastructure
issue that could be fixed. We could lump these drivers back together,
and make sure that "brcm,nand-iproc" gets the priority in the
of_device_id list.
Or we could just relax the DT binding.
But wait, wouldn't cygnus already have that problem? You're using the
binding I suggested in arch/arm/boot/dts/bcm-cygnus.dtsi.
Oh, and I see we hacked this one in drivers/mtd/nand/brcmnand/Makefile:
# link order matters; don't link the more generic brcmstb_nand.o before the
# more specific iproc_nand.o, for instance
Brian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
