Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/46140 )
Change subject: s25f.c: implement probing and block erasers for Spansion ......................................................................
Patch Set 14:
(2 comments)
https://review.coreboot.org/c/flashrom/+/46140/14/s25f.c File s25f.c:
https://review.coreboot.org/c/flashrom/+/46140/14/s25f.c@1 PS14, Line 1: /* : * This file is part of the flashrom project. : * : * Copyright (C) 2014 Google Inc. : * : * Redistribution and use in source and binary forms, with or without : * modification, are permitted provided that the following conditions : * are met: : * : * Redistributions of source code must retain the above copyright : * notice, this list of conditions and the following disclaimer. : * : * Redistributions in binary form must reproduce the above copyright : * notice, this list of conditions and the following disclaimer in the : * documentation and/or other materials provided with the distribution. : * : * Neither the name of Google or the names of contributors or : * licensors may be used to endorse or promote products derived from this : * software without specific prior written permission. : * : * This software is provided "AS IS," without a warranty of any kind. : * ALL EXPRESS OR IMPLIED CONDITIONS, REPRESENTATIONS AND WARRANTIES, : * INCLUDING ANY IMPLIED WARRANTY OF MERCHANTABILITY, FITNESS FOR A : * PARTICULAR PURPOSE OR NON-INFRINGEMENT, ARE HEREBY EXCLUDED. : * GOOGLE INC AND ITS LICENSORS SHALL NOT BE LIABLE : * FOR ANY DAMAGES SUFFERED BY LICENSEE AS A RESULT OF USING, MODIFYING : * OR DISTRIBUTING THIS SOFTWARE OR ITS DERIVATIVES. IN NO EVENT WILL : * GOOGLE OR ITS LICENSORS BE LIABLE FOR ANY LOST REVENUE, PROFIT OR DATA, : * OR FOR DIRECT, INDIRECT, SPECIAL, CONSEQUENTIAL, INCIDENTAL OR : * PUNITIVE DAMAGES, HOWEVER CAUSED AND REGARDLESS OF THE THEORY OF : * LIABILITY, ARISING OUT OF THE USE OF OR INABILITY TO USE THIS SOFTWARE, : * EVEN IF GOOGLE HAS BEEN ADVISED OF THE POSSIBILITY OF SUCH DAMAGES. : */ Use,
``` /* * This file is part of the flashrom project. * * Copyright (C) 2020 The Chromium OS Authors * * 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. */ ```
and send a patch to our tree to fix it up for us.
https://review.coreboot.org/c/flashrom/+/46140/14/s25f.c@370 PS14, Line 370: int probe_spi_big_spansion(struct flashctx *flash) : { : uint8_t cmd = JEDEC_RDID; : uint8_t dev_id[6]; /* We care only about 6 first bytes */ : : if (spi_send_command(flash, sizeof(cmd), sizeof(dev_id), &cmd, dev_id)) : return 0; : : msg_gdbg("Read id bytes: "); : for (size_t i = 0; i < sizeof(dev_id); i++) : msg_gdbg(" 0x%02x", dev_id[i]); : msg_gdbg(".\n"); : : /* : * The structure of the RDID output is as follows: : * : * offset value meaning : * 00h 01h Manufacturer ID for Spansion : * 01h 20h 128 Mb capacity : * 01h 02h 256 Mb capacity : * 02h 18h 128 Mb capacity : * 02h 19h 256 Mb capacity : * 03h 4Dh Full size of the RDID output (ignored) : * 04h 00h FS: 256-kB physical sectors : * 04h 01h FS: 64-kB physical sectors : * 04h 00h FL: 256-kB physical sectors : * 04h 01h FL: Mix of 64-kB and 4KB overlayed sectors : * 05h 80h FL family : * 05h 81h FS family : * : * Need to use bytes 1, 2, 4, and 5 to properly identify one of eight : * possible chips: : * : * 2 types * 2 possible sizes * 2 possible sector layouts : * : */ : : uint32_t model_id = : dev_id[1] << 24 | : dev_id[2] << 16 | : dev_id[4] << 8 | : dev_id[5] << 0; : : if (dev_id[0] == flash->chip->manufacture_id && model_id == flash->chip->model_id) : return 1; : : return 0; : } : I think this duplicates logic that is in spi25.c ``` static void rdid_get_ids(unsigned char *readarr, int bytes, uint32_t *id1, uint32_t *id2) ``
I'll allow it for now but I would like to see a follow up if possible to consolidate it into the canonical rdid paths from spi25.c as I cannot see anything too bespoke going on here.