Re: [PATCH] mtd: spi-nor: Add support for BoHong bh25q128as

David Bauer
2021-05-10T11:43:18Z

Hi Michael,

On 5/10/21 12:56 PM, Michael Walle wrote:
> Am 2021-05-10 12:27, schrieb David Bauer:
>> On 5/10/21 11:35 AM, Michael Walle wrote:
>>> Am 2021-05-10 11:28, schrieb David Bauer:
>>>> On 5/10/21 10:00 AM, Michael Walle wrote
>>>>
>>>> [...]
>>>>
>>>>>> +static const struct flash_info bohong_parts[] = {
>>>>>> +    /* BoHong Microelectronics */
>>>>>> +    { "bh25q128as", INFO(0x684018, 0, 64 * 1024, 256,
>>>>>
>>>>> I couldn't find "BoHong" in JEP106BC. 0x68 (without continuation codes)
>>>>> is "Convex Computer". So this is wrong. OTOH I'm not sure, how many
>>>>> SPI flashes "convex computer" have, if any ;) This company was brought
>>>>> by HP in the end.
>>>>>
>>>>> In any case, this patch depends on how we handle continuation codes or
>>>>> if we can handle them at all. Or if this flash just lie about its
>>>>> manufacturer id and don't and CC.
>>>>
>>>> First of all, BoHong and Boya microelectronics seems to be the same
>>>> company, as their datasheets seem to copy each other. There's not much
>>>> information about either of both, so I'd say that's a fair assumption.
>>>>
>>>> Regarding the continuation codes, Boya is listed in bank nine, however
>>>> in this case I should currently read an all 0x7f ID shouldn't I?
>>>
>>> I'd guess so, yes.
>>>
>>>> The datasheet also only specifies 3 bytes as a return value for
>>>> register 0x9fh :(
>>>
>>> Yeah. So, this flash falls into the same category "simply hijacks
>>> a manuf id" as all the other flashes.
>>
>> From a quick check, this is also be the case for GigaDevices and XMC.
>>
>> My spontaneous idea would be to extend support for JEDEC IDs to read
>> the up to 9 banks of the vendor ID and fix up the existing offenders.
>
> you mean gigadevices and xmc? I'd presume they are also lacking the
> continuation bytes.

Correct, same story with them.

>
>> To not break existing boards, we could either skip the continuation
>> bytes of the kernel ID definitions for all flash chips or flag the
>> already existing ones and only perform this on such flagged chips.
>>
>> Personally, I'd say that only performing this on existing chips would
>> be better, as new vendors with this violation scheme might probably
>> appear and cause conflicts.
>>
>> As we still lack auto detection for new chips with that, configuring
>> the flash chip used with the chip name via DT would allow to set the
>> exact chip used and also validate if the manufacturer / product after
>> the continuation bits matches the one read from the chip.
>>
>> What do you think?
>
> If you'd ask me, unless there is a real world conflict, I'd just go
> ahead and add them as is. If there is a conflict we'd need to find
> a per device resolution for it.

Okay, I'll resend a v2 with the removed copyright then.

>
> There is another problem: shared device ids per vendor. Some (most?)
> flash vendor share device ids on "similar" flashes, which we still
> need tell apart in the kernel. So technically, this is the same problem
> as with non-existing continuation bytes. Two different flashes sharing
> the same id. For now, we look for differences in the SFDP.
>
> Right now, the flash is probed first by its id and the the SFDP is
> read and parsed. There are ideas to first read the SFDP. Having this
> might come in handy here, too. Eg. we could fingerprint the flash
> by its SFDP.

I was also thinking about that and we're also already bitten by identical
JEDEC IDs for different models. It's really a pity that there is no real
unique model identifier for us to use, which is not hacked to support
legacy implementations like with this chip :(

Best
David

>
> I know Vingesh is working on the continuation bytes stuff. So I might
> be late to the party ;)
>
> Regarding the device tree compatibles, the maintainers doesn't like
> that very much. Maybe as a last resort method.
>
> -michael