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

[Michael Walle <michael@walle.cc>]

Am 3. Juli 2021 17:58:57 MESZ schrieb George Brooke <figgyc@figgyc.uk>:
>Hi Tudor,
>
>Tudor.Ambarus@microchip.com writes:
>
>> On 6/28/21 8:48 AM, Tudor.Ambarus@microchip.com wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you
>know the content is safe
>>>
>>> On 5/18/21 10:39 PM, David Bauer wrote:
>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you
>know the content is safe
>>>>
>>>> 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?
>>>>
>>>
>>> So no SFDP data?
>>> Have you tried to read more of ID bytes, maybe there's an extended
>ID? Please
>>> dump 15 bytes of ID.
>>
>> what's the difference between by25q128as and bh25q128as? I see they
>share the
>> same flash ID.
>>
>
>I've got the by25q128as, so I compiled the SFDP and sysfs patch kernel
>to read it out.
>
>figgyc@figgyc-pi:~ $ ls /sys/class/spi_master/spi0/spi0.0/spi-nor/
>jedec_id  manufacturer  partname
>$ cat /sys/class/spi_master/spi0/spi0.0/spi-nor/jedec_id
>684018
>$ cat /sys/class/spi_master/spi0/spi0.0/spi-nor/manufacturer
>boya
>$ cat /sys/class/spi_master/spi0/spi0.0/spi-nor/partname
>by25q128as
>(this is using my patch for the chip support)
>
>There was no sfdp file for me either, failed the version check like
>David's chip (I added a dev_dbg to check).

Then it seems it doesn't have SFDP. 


>One thing I noticed reading the datasheet[1] again was this line:
>"Security Register 0 can be used to store the Flash Discoverable
>Parameters, The feature is upon special order, please contact Boya
>Microelectronics for details."
>The same line is also present in the BoHong datasheet but it says
>HuaHong instead of Boya. That makes me wonder if the meaning of
>"Discoverable Parameters (SFDP) register" in the datasheet does not
>actually mean that it has SFDP data programmed in by default, which
>would be quite strange, but if true then that would be quite annoying
>because then I don't think there are any differences between Boya and
>BoHong. Very strange design decision in my opinion but it is what it
>is.

I'd say it is exactly this. There is no SFDP. only on "special request", I guess that means "we were too lazy and if there is a client big enough we'll do it".

>The only other explanation I could think of is that erasing the chip
>might erase security register 0? Unfortunately I only have one chip so
>I can't test that. Even if that were the case it would still be
>unhelpful.

There should be a special command to erase security registers, aka OTP. Winbond does the same, just return the first security register content if you send a RDSFDP. Just that it's programmed by default and the data sheet doesn't mention security register 0 (and treat it as reserved). 

-michael

>I dumped extra ID in a previous email thread, IIRC it just loops, no
>extra 7f bytes like there should be.
>
>In conclusion it seems to me as though the two chips behave
>identically, there's probably no way to know for certain though without
>asking the manufacturer.
>
>[1] http://www.bmsemi.com/upload/file/20180425/15246261557309416.pdf
>
>> ______________________________________________________
>> Linux MTD discussion mailing list
>> http://lists.infradead.org/mailman/listinfo/linux-mtd/


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