| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20150206011857.GC30781@ld-irv-0074>
Date: Thu, 5 Feb 2015 17:18:57 -0800
From: Brian Norris <computersforpeace@...il.com>
To: Jim Kuo <jimtingkuo@...il.com>
Cc: David Woodhouse <dwmw2@...radead.org>, Marek Vasut <marex@...x.de>,
Huang Shijie <b32955@...escale.com>,
Geert Uytterhoeven <geert+renesas@...der.be>,
Rafał Miłecki <zajec5@...il.com>,
Ben Hutchings <ben@...adent.org.uk>,
Kuninori Morimoto <kuninori.morimoto.gx@...esas.com>,
linux-mtd@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] m25p80, spi-nor: Update id list of Macronix chips
On Fri, Feb 06, 2015 at 02:44:04AM +0800, Jim Kuo wrote:
> Update the Macronix chip IDs. And add two functions to m25p80.c to
> support some undefined read/write actions.
>
> Signed-off-by: Jim Kuo <jimtingkuo@...il.com>
> ---
> drivers/mtd/devices/m25p80.c | 83 ++++++++++++++++++++++++++++++++++++++++---
> drivers/mtd/spi-nor/spi-nor.c | 44 ++++++++++++++++-------
> 2 files changed, 110 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
> index 85e35467..731f568 100644
> --- a/drivers/mtd/devices/m25p80.c
> +++ b/drivers/mtd/devices/m25p80.c
> @@ -170,6 +170,74 @@ static int m25p80_erase(struct spi_nor *nor, loff_t offset)
> return 0;
> }
>
> +static int m25p80_write_xfer(struct spi_nor *nor,
> + struct spi_nor_xfer_cfg *cfg, u8 *buf, size_t len)
> +{
> + struct m25p *flash = nor->priv;
> + struct spi_device *spi = flash->spi;
> + struct spi_transfer t[2] = {};
> + struct spi_message m;
> + unsigned int dummy = cfg->dummy_cycles;
> + int ret;
> +
> + dummy /= 8;
> +
> + if (cfg->wren) {
> + ret = nor->write_reg(nor, SPINOR_OP_WREN, NULL, 0, 0);
> + if (ret)
> + return ret;
> + }
> +
> + spi_message_init(&m);
> +
> + flash->command[0] = cfg->cmd;
> + m25p_addr2cmd(nor, cfg->addr, flash->command);
> +
> + t[0].tx_buf = flash->command;
> + t[0].len = cfg->cmd_pins + cfg->addr_pins + dummy;
> + spi_message_add_tail(&t[0], &m);
> +
> + t[1].tx_buf = buf;
> + t[1].tx_nbits = cfg->mode_pins;
> + t[1].len = len;
> + spi_message_add_tail(&t[1], &m);
> +
> + spi_sync(spi, &m);
> +
> + return 0;
> +}
> +
> +static int m25p80_read_xfer(struct spi_nor *nor,
> + struct spi_nor_xfer_cfg *cfg, u8 *buf, size_t len)
> +{
> + struct m25p *flash = nor->priv;
> + struct spi_device *spi = flash->spi;
> + struct spi_transfer t[2];
> + struct spi_message m;
> + unsigned int dummy = cfg->dummy_cycles;
> +
> + dummy /= 8;
> +
> + spi_message_init(&m);
> + memset(t, 0, sizeof(t));
> +
> + flash->command[0] = cfg->cmd;
> + m25p_addr2cmd(nor, cfg->addr, flash->command);
> +
> + t[0].tx_buf = flash->command;
> + t[0].len = cfg->cmd_pins + cfg->addr_pins + dummy;
> + spi_message_add_tail(&t[0], &m);
> +
> + t[1].rx_buf = buf;
> + t[1].rx_nbits = cfg->mode_pins;
> + t[1].len = len;
> + spi_message_add_tail(&t[1], &m);
> +
> + spi_sync(spi, &m);
> +
> + return 0;
> +}
> +
All of the above is pointless. The write_xfer/read_xfer functions are
not even used. (They should probably just be dropped, since they're
doing nothing as-is.)
> /*
> * board specific setup should have ensured the SPI clock used here
> * matches what the READ command supports, at least until this driver
> @@ -199,6 +267,8 @@ static int m25p_probe(struct spi_device *spi)
> nor->erase = m25p80_erase;
> nor->write_reg = m25p80_write_reg;
> nor->read_reg = m25p80_read_reg;
> + nor->write_xfer = m25p80_write_xfer;
> + nor->read_xfer = m25p80_read_xfer;
>
> nor->dev = &spi->dev;
> nor->mtd = &flash->mtd;
> @@ -261,10 +331,15 @@ static const struct spi_device_id m25p_ids[] = {
> {"mr25h256"}, {"mr25h10"},
> {"gd25q32"}, {"gd25q64"},
> {"160s33b"}, {"320s33b"}, {"640s33b"},
> - {"mx25l2005a"}, {"mx25l4005a"}, {"mx25l8005"}, {"mx25l1606e"},
> - {"mx25l3205d"}, {"mx25l3255e"}, {"mx25l6405d"}, {"mx25l12805d"},
> - {"mx25l12855e"},{"mx25l25635e"},{"mx25l25655e"},{"mx66l51235l"},
> - {"mx66l1g55g"},
> + {"mx25l512e"}, {"mx25l5121e"}, {"mx25l1006e"}, {"mx25l1021e"},
> + {"mx25l2006e"}, {"mx25l4006e"}, {"mx25u4035"}, {"mx25v4035"},
> + {"mx25l8006e"}, {"mx25u8035"}, {"mx25v8035"}, {"mx25l1606e"},
> + {"mx25l1633e"}, {"mx25l1635e"}, {"mx25u1635e"}, {"mx25l3206e"},
> + {"mx25l3239e"}, {"mx25l3225d"}, {"mx25l3255e"}, {"mx25l6406e"},
> + {"mx25l6439e"}, {"mx25l12839f"}, {"mx25l12855e"},
> + {"mx25u12835f"}, {"mx25l25635e"}, {"mx25l25655e"},
> + {"mx25u25635f"}, {"mx66l51235f"}, {"mx66u51235f"},
> + {"mx66l1g45g"}, {"mx66l1g55g"},
> {"n25q064"}, {"n25q128a11"}, {"n25q128a13"}, {"n25q256a"},
> {"n25q512a"}, {"n25q512ax3"}, {"n25q00"},
> {"pm25lv512"}, {"pm25lv010"}, {"pm25lq032"},
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 0f8ec3c..a6c7337 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -545,19 +545,37 @@ static const struct spi_device_id spi_nor_ids[] = {
> { "640s33b", INFO(0x898913, 0, 64 * 1024, 128, 0) },
>
> /* Macronix */
> - { "mx25l2005a", INFO(0xc22012, 0, 64 * 1024, 4, SECT_4K) },
Nak.
> - { "mx25l4005a", INFO(0xc22013, 0, 64 * 1024, 8, SECT_4K) },
Nak.
> - { "mx25l8005", INFO(0xc22014, 0, 64 * 1024, 16, 0) },
Nak.
(you get the picture)
> - { "mx25l1606e", INFO(0xc22015, 0, 64 * 1024, 32, SECT_4K) },
> - { "mx25l3205d", INFO(0xc22016, 0, 64 * 1024, 64, 0) },
> - { "mx25l3255e", INFO(0xc29e16, 0, 64 * 1024, 64, SECT_4K) },
> - { "mx25l6405d", INFO(0xc22017, 0, 64 * 1024, 128, 0) },
> - { "mx25l12805d", INFO(0xc22018, 0, 64 * 1024, 256, 0) },
> - { "mx25l12855e", INFO(0xc22618, 0, 64 * 1024, 256, 0) },
> - { "mx25l25635e", INFO(0xc22019, 0, 64 * 1024, 512, 0) },
> - { "mx25l25655e", INFO(0xc22619, 0, 64 * 1024, 512, 0) },
> - { "mx66l51235l", INFO(0xc2201a, 0, 64 * 1024, 1024, SPI_NOR_QUAD_READ) },
> - { "mx66l1g55g", INFO(0xc2261b, 0, 64 * 1024, 2048, SPI_NOR_QUAD_READ) },
This is a load of junk. You can't just remove table entries that already
have a name entry. It's not my fault that flash vendors continuously
output new flash generations that reuse IDs from the previous. We need
name stability, for anyone who might be using these name strings to bind
to the m25p80 driver.
> + { "mx25l512e", INFO(0xc22010, 0, 64 * 1024, 1, SECT_4K) },
> + { "mx25l5121e", INFO(0xc22210, 0, 64 * 1024, 1, SECT_4K) },
> + { "mx25l1006e", INFO(0xc22011, 0, 64 * 1024, 2, SECT_4K) },
> + { "mx25l1021e", INFO(0xc22211, 0, 64 * 1024, 2, SECT_4K) },
> + { "mx25l2006e", INFO(0xc22012, 0, 64 * 1024, 4, SECT_4K) },
> + { "mx25l4006e", INFO(0xc22013, 0, 64 * 1024, 8, SECT_4K) },
> + { "mx25u4035", INFO(0xc22533, 0, 64 * 1024, 8, SECT_4K) },
> + { "mx25v4035", INFO(0xc22553, 0, 64 * 1024, 8, SECT_4K) },
> + { "mx25l8006e", INFO(0xc22014, 0, 64 * 1024, 16, 0) },
> + { "mx25u8035", INFO(0xc22534, 0, 64 * 1024, 16, 0) },
> + { "mx25v8035", INFO(0xc22554, 0, 64 * 1024, 16, 0) },
> + { "mx25l1606e", INFO(0xc22015, 0, 64 * 1024, 32, SECT_4K) },
> + { "mx25l1633e", INFO(0xc22415, 0, 64 * 1024, 32, 0) },
> + { "mx25l1635e", INFO(0xc22515, 0, 64 * 1024, 32, 0) },
> + { "mx25u1635e", INFO(0xc22535, 0, 64 * 1024, 32, 0) },
> + { "mx25l3206e", INFO(0xc22016, 0, 64 * 1024, 64, 0) },
> + { "mx25l3239e", INFO(0xc22536, 0, 64 * 1024, 64, SPI_NOR_QUAD_READ) },
> + { "mx25l3225d", INFO(0xc25e16, 0, 64 * 1024, 64, 0) },
> + { "mx25l3255e", INFO(0xc29e16, 0, 64 * 1024, 64, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> + { "mx25l6406e", INFO(0xc22017, 0, 64 * 1024, 128, 0) },
> + { "mx25l6439e", INFO(0xc22537, 0, 64 * 1024, 128, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> + { "mx25l12839f", INFO(0xc22018, 0, 64 * 1024, 256, SPI_NOR_QUAD_READ) },
> + { "mx25l12855e", INFO(0xc22618, 0, 64 * 1024, 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> + { "mx25u12835f", INFO(0xc22538, 0, 64 * 1024, 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> + { "mx25l25635e", INFO(0xc22019, 0, 64 * 1024, 512, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> + { "mx25l25655e", INFO(0xc22619, 0, 64 * 1024, 512, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> + { "mx25u25635f", INFO(0xc22539, 0, 64 * 1024, 512, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> + { "mx66l51235f", INFO(0xc2201a, 0, 64 * 1024, 1024, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> + { "mx66u51235f", INFO(0xc2253a, 0, 64 * 1024, 1024, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> + { "mx66l1g45g", INFO(0xc2201b, 0, 64 * 1024, 2048, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> + { "mx66l1g55g", INFO(0xc2261b, 0, 64 * 1024, 2048, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
You need to do a lot better job of documenting your patches (i.e.,
describe why you're doing these things) if you want me to take them.
What's more, the SFDP support you're trying to add should be done in a
way where you *don't* need to muck with the ID table every time. With
SFDP you can get most/all this information anyway, and devices should
just be able to bind to this driver using a generic name like
"spi-nor,sfdp" or something like that, instead of having to expand this
table.
>
> /* Micron */
> { "n25q032", INFO(0x20ba16, 0, 64 * 1024, 64, 0) },
Brian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists