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

[David Bauer <mail@david-bauer.net>]

Hi Michael,

Sorry for the late reply, was not feeling well past week.

On 5/10/21 1:22 PM, Michael Walle wrote:
> Hi David,
> 
> Am 2021-05-10 13:04, schrieb David Bauer:
>> 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.
> 
> Could you also apply my SFDP patch [1] and send the dump (if there
> is any)? Unfortunately, I can't think of a good way to do that along
> with the patch and if this in some way regarded as copyrighted material.
> So feel free to send it to me privately. I'm starting to build a
> database.

Bad news, I'm not able to get a SFDP with your patches, as the SFDP extraction
fails at the version check.

Is there anything else I can try?

Best
David

> 
>>> 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 :(
> 
> Unfortunately, they aren't legacy and new chips still have this
> behavior..
> 
> -michael
> 
> [1] https://lore.kernel.org/linux-mtd/20210503155651.30889-1-michael@walle.cc/

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/