| Message ID | 610761cf-5a19-c182-07d8-8d118ca20035@cogentembedded.com |
|---|---|
| Headers | show |
| Series | Untangle Spansion S25F{L|S}512S chip IDs | expand |
Hi, Sergei, On 01/16/2019 07:49 PM, Sergei Shtylyov wrote: > Hello! > > Here's the set of 2 patches against the 'spi-nor/next' branch of 'linux-mtd.git' > repo. We're untangling the S25FS512S being taken for S25FL512S by the SPI NOR code > due to omitting the family ID byte in the latter chip's ID. I'm proposing to use > the full 6-byte JEDEC ID for both these chips. The patch set is looking good. If these patches are based on Cyrille's suggestion from [1], we can give him credit by adding: Suggested-by: Cyrille Pitchen <cyrille.pitchen@microchip.com> We'll need his approval if that's the case. If you're going to resubmit, it's a good time to get rid of the 80 char limit checkpatch warning. I would recommend this way of formatting entries, because it has fewer lines: { "mx25u12835f", INFO(0xc22538, 0, 64 * 1024, 256, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, Of course, if this is not the case, and you came with this proposal without knowing about Cyrille's suggestion, I'm not going to ask you to resubmit just for this little cosmetic change, just say. Thanks, ta [1] https://patchwork.kernel.org/patch/10595835/
Hello! On 01/21/2019 11:28 AM, Tudor.Ambarus@microchip.com wrote: >> Here's the set of 2 patches against the 'spi-nor/next' branch of 'linux-mtd.git' >> repo. We're untangling the S25FS512S being taken for S25FL512S by the SPI NOR code >> due to omitting the family ID byte in the latter chip's ID. I'm proposing to use >> the full 6-byte JEDEC ID for both these chips. > > The patch set is looking good. If these patches are based on Cyrille's > suggestion from [1], we can give him credit by adding: No, I even had trouble finding where Cyrille was suggesting that. :-) BTW, looking at his message, I feel somewhat perplexed: "Maybe you can do the magic by placing a new INFO6() entry for the S25FL512S, just before the legacy INFO() entry for S25FL512S. Indeed entries are tested in the order they appear inside the spi_nor_ids[] array from spi_nor_read_id(), so the INFO6() entry would be tested 1st, trying to match 6 bytes, then the INFO() entry only trying to match 3 bytes." Did he actually mean adding a new INFO6() entry for S25FS512S? Else that sentence doesn't make a lot of sense to me. :-) Also INFO() entry matches 5 bytes, not 3. Anyway, I'm suggesting to get rid of the INFO() entry for S25FL512S altogether -- and that's not a part of his suggestion... perhaps it's too risky; I'm not sure why 5-byte matching was used for S25FL512S in the 1st place. > Suggested-by: Cyrille Pitchen <cyrille.pitchen@microchip.com> > We'll need his approval if that's the case. [...] > Of course, if this is not the case, and you came with this proposal without > knowing about Cyrille's suggestion, Well, I remember you (or somebody else) pointing to that patchwork entry... however, the idea I got was from the Renesas BSP patch. > I'm not going to ask you to resubmit just > for this little cosmetic change, just say. Thank you. :-) > Thanks, > ta > > [1] https://patchwork.kernel.org/patch/10595835/ MBR, Sergei