Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ramips: add support for Creality WB-01 #3802

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

ihrapsa
Copy link

@ihrapsa ihrapsa commented Jan 20, 2021

I guess this could be merged with master

@ihrapsa ihrapsa changed the title wb01 - master merge Creality wi-fi box support - merge Jan 20, 2021
Subject: [PATCH] mtd: spi-nor: add support for Boya BY25Q128AS

Adds support for Boya Microelectronics (BoyaMicro) BY25Q128AS 128 MiB flash.
Tested on Creality WB-01 embedded device which uses this chip.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this submitted upstream, i.e. correctly placed in the pending-* directory?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not currently, apologies. Would you rather I move it to the correct folder or submit it to upstream? (also working on the other changes, just need to do some testing for those so it'll be a bit)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Submitting upstream is obviously the desired option, as this will have the proper authority care about the change.

@adschm adschm added kernel pull request/issue with Linux kernel related changes target/ramips pull request/issue for ramips target labels Jan 20, 2021
led-failsafe = &led_status;
led-upgrade = &led_status;
led-running = &led_status;
label-mac-device = &wmac;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only works in combination with mtd-mac-address. You will need to set the label MAC address in the lower section of 02_network; the correct lines are already there for vocore,vocore2 ...

flash@0 {
compatible = "jedec,spi-nor";
reg = <0>;
spi-max-frequency = <10000000>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this go faster?

&state_default {
gpio {
groups = "i2s", "refclk", "wdt", "p4led_an",
"p3led_an", "p2led_an", "p1led_an", "wled_an";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I end up with just "wdt", "p3led_an", "p1led_an", "p0led_an", "wled_an" ?

@@ -13,6 +13,27 @@ define Build/ravpower-wd009-factory
@mv $@.new $@
endef

define Build/creality_wb-01-factory
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move this up so Build/* recipes are sorting alphabetically.

@@ -13,6 +13,27 @@ define Build/ravpower-wd009-factory
@mv $@.new $@
endef

define Build/creality_wb-01-factory
echo $@
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's that supposed to do?

echo '#!/bin/sh' > $@.install.sh
echo 'kernel_size=851968' >> $@.install.sh
echo 'file=$$2/factory.bin' >> $@.install.sh
echo 'file_size=`wc -c < $$file`' >> $@.install.sh
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deprecated backticks might be replaced by $() here ...

echo 'kernel_size=851968' >> $@.install.sh
echo 'file=$$2/factory.bin' >> $@.install.sh
echo 'file_size=`wc -c < $$file`' >> $@.install.sh
echo 'fs_size=`expr $$file_size - $$kernel_size`' >> $@.install.sh
Copy link
Member

@adschm adschm Jan 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not simply fs_size=$$((file_size - kernel_size)) ?

@@ -63,6 +84,17 @@ define Device/buffalo_wcr-1166ds
endef
TARGET_DEVICES += buffalo_wcr-1166ds

define Device/creality_wb-01
IMAGE_SIZE := 16064k
IMAGES += cxsw_update.tar.bz2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd still call this factory.bin, or at least factory.tar.bz2.

The name of the images should indicate the use from an OpenWrt perspective, i.e. factory means it's for initial flashing.
Since renaming the image will be necessary anyway, one can also rename from factory.* to the proper name then.

DEVICE_VENDOR := Creality
DEVICE_MODEL := WB-01
DEVICE_PACKAGES := kmod-usb2 kmod-usb-ohci kmod-sdhci-mt7620
SUPPORTED_DEVICES += creality_wb-01
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is default and can be dropped.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now it's not the default anymore, but just wrong. Please drop SUPPORTED_DEVICES.

@@ -20,6 +20,10 @@ netgear,r6120)
ucidef_set_led_switch "lan" "lan" "green:lan" "switch0" "0xf"
ucidef_set_led_switch "wan" "wan" "green:wan" "switch0" "0x10"
;;
creality,wb-01)
ucidef_set_led_switch "lan" "lan" "blue:lan" "switch0" "0x04"
ucidef_set_led_netdev "wlan2g" "wlan2g" "yellow:wlan2g" "phy0tpt"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WiFi trigger is already in the DTS and can be dropped here.

@adschm
Copy link
Member

adschm commented Jan 20, 2021

Consider renaming your PR to something like "ramips: add support for Creality WB-01" as well, so it's found more easily by other people.

@adschm
Copy link
Member

adschm commented Feb 5, 2021

Ping.

@figgyc
Copy link

figgyc commented Feb 8, 2021

Hi - I think I've made all the changes that you requested now.
I've gotten started on submitting my kernel patch to upstream, however it turns out that there might be some issues with some nonstandard manufacturer IDs (http://lists.infradead.org/pipermail/linux-mtd/2021-February/085068.html)
Am going to start debugging that now - in the meantime not sure if it would be the best idea to merge this or not.

@@ -228,6 +229,7 @@ ramips_setup_macs()
tplink,archer-c50-v4)
wan_mac=$(macaddr_add "$(mtd_get_mac_binary rom 0xf100)" 1)
;;
creality,wb-01|\
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move that block based on alphabetic sorting.

@ihrapsa ihrapsa changed the title Creality wi-fi box support - merge ramips: add support for Creality WB-01 Feb 18, 2021
@figgyc
Copy link

figgyc commented Mar 1, 2021

Hi sorry for the long inactivity. Should have fixed all the problems flagged up.
See https://lkml.org/lkml/2021/2/13/188 I think is the last thing I said about the kernel patch and I havent gotten any replies about it, but from my point of view I think it should be fine to merge? Let me know what you think.

@ihrapsa
Copy link
Author

ihrapsa commented Apr 4, 2021

Ping.

@musztarda
Copy link

ping

@wireddown
Copy link

Any help reviewing the PR would be much appreciated 🙏🏼

It looks like this PR is stuck at the upstream kernel changes because there's no confident way to correctly identify the SPI memory chip on the WB-01. The first SPI standard only allocated 126 manufacturer IDs (often called the "first bank" of IDs), but there are now many more than that, and it's possible for one ID to match multiple different chips 😱

I haven't done any embedded work with SPI devices, but my understanding from the LKML thread is that the kernel is missing a single mechanism to identify and distinguish SPI devices between the first bank of 126 vendors and the many others who have joined since.

Further, the problem has other factors contributing to its difficulty:

  • The device manufacturers themselves are incorrectly implementing the standards for device identification.
      Boya BY25Q128AS responses to SPI command 0x90: "manufacturer and device ID"
    
    + expected: 0x7f 0x7f 0x7f 0x7f 0x7f 0x7f 0x7f 0x7f 0x68
    - actual:                       0x00 0x00 0x00 0x00 0x68 0x17
  • The other standard for identifying SPI devices -- Serial Flash Discoverable Parameter (SFDP) -- is optional and only implemented by some chip makers. Unsurprisingly, the Boya chip on the WB-01 does not implement this.

We need a SPI device expert who can find a method (or knows of another that we don't) to identify any SPI chip using the mixed and partial support we've observed so far.

@ihrapsa
Copy link
Author

ihrapsa commented May 30, 2021

Thanks @wireddown for the insight!
Apparently the guys from flashrom have issues with it as well: this flashrom/flashrom#161 and this flashrom/flashrom#210
We need someone brave and knowledgeable enough to troubleshoot this. 😣

gpios = <&gpio 38 GPIO_ACTIVE_LOW>;
linux,code = <KEY_RESTART>;
};

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unneeded empty line.

echo 'mtd_write -o 0 -l $$kernel_size write $$file Kernel' \
>> $@.install.sh
echo 'mtd_write -r -o $$kernel_size -l $$fs_size write $$file RootFS' \
>> $@.install.sh
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder whether this can be written at once with some <-EOF syntax ...

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so? It seems as though you have to use .ONESHELL: to do heredoc syntax in make but that caused errors like /usr/bin/env bash: No such file or directory for me when I tried, and it might also cause problems somewhere else because of how that directive works. There might be a better way but I can't think of one.

@adschm
Copy link
Member

adschm commented Jun 6, 2021

Kernel 5.10 has arrived, so the patch should be applied there as well.

@arrowcircle
Copy link

Hey! Any news on that?

@figgyc
Copy link

figgyc commented Nov 11, 2021

Hi - sorry for the inactivity lately, I've been quite busy with my not-software development life stuff.

I've mostly been waiting on the outcome of the aforementioned patch series discussion upstream. v3 of the flash collision handling series was posted recently, although I reckon it will still be a while until it is finished.

I'll run a rebase on this soon - I did notice though, it seems like a patch for the BoHong chip of the same ID ended up in the openwrt tree. Not quite sure if/how to deal with that yet, At the very least having two flash patches doing essentially the same thing would probably cause issues, so I'll need to change something before my next rebase. But backporting the flash collisions patch (and presumably the large set of refactoring patches on which it depends, and probably a lot of other changes in the mtd module since 5.10) would also be quite the endeavour, and I'm not sure how necessary it is in this case.

In this particular scenario I'm actually fairly sure that the collisions module doesn't make much difference, since the BY and BH chips behave seemingly identically, their datasheets are clearly copy pastes of one another, and there is no way to distinguish them at runtime (that we're aware of), so realistically there isn't much change in functionality of porting those patches as opposed to just using the simple patch used currently. (That's not to say I don't support the patchset, there are other flash collisions covered there which can be distinguished eg some Micron flashes of the same ID with and without SFDP, so it is a good idea to do it properly in the long term.)

I am a bit conflicted on what to do as I am of course enthusiastic about adding support to this device in OpenWrt (it's been my 3D printers' webcam server running stable for several months - wow I started working on this in January? Time flies.) but I don't want to make any mistakes, is all.

Has anyone else got any thoughts? As a last point, have there been any significant refactors in device tree files that I might want to fix? I think I heard stuff about mac address fetching on the mailing list but haven't particularly been keeping track.

@gretel
Copy link

gretel commented Nov 17, 2021

@figgyc thanks, how about forking-off and provide binaries to allow users to test - and eventually progress in regards to the development? not sure if the openwrt context - this PR - will give that.

@figgyc
Copy link

figgyc commented Feb 25, 2022

Hi, got bored and decided to rebase today. Tested it and it still seems to work.

For now I just removed the Boya flash patch to avoid conflict with the BoHong one, as I've mentioned it shouldn't make a difference. If you want it back I've created a wb01-old branch on my fork which is just the old commits pre-rebase. As for upstream, the ID collisions patchset hasn't seen activity since I last posted. There have been a lot of other things going on the linux-mtd mailing list though so I'm not surprised by that. It would probably need a rebase at the very least. (see edit below)

So yeah that's the thing. Sorry for not working on this for a while, I've been busy with other projects. I think this would mostly be mergeable with a bit more testing from others, but if I've missed anything do let me know.

There is only one issue I noticed which is this menu in LuCI:
image
I assume this has something to do with missing DSA configuration? This is with ethernet plugged in by the way. Couldn't find a DTS with only one port in my brief search but I didn't look for that long.

edit: v4 was posted a few days ago, things do seem to get closer to merging but there is still quite a few things to fix and decide, there will be a v5 probably. Also, I've been watching the linux-mtd mailing list since the start of this, and a lot of refactoring has happened in the mtd module, so this patch depends on a lot of other changes as it is, a backport would be quite difficult and as I have mentioned above not strictly necessary; I think that the way this PR is now it does work and I don't expect it to be a problem, it's just not elegant, but not the worst patch in the tree I imagine ;) Or if we decide to wait until it goes into an upstream kernel I wouldn't mind that, it's easy enough to build this PR yourself if need be, plenty of people already are which is cool.

@ihrapsa
Copy link
Author

ihrapsa commented Jun 25, 2022

Hey @figgyc,
I've tried building this with the BoHong patch and it works. However, I've noticed that the wifi mac address changes randomly on every reboot (I think it's not able to access the mac written on the flash). Are you experiencing the same issue? Switching back to an old build with your BoyaMicro patch restors the original mac address.

@fanoush
Copy link

fanoush commented Jun 27, 2022

Thanks for continuing effort!
BTW just noticed there is 2.0 version https://store.creality.com/products/creality-wifi-cloud-box-2-0-with-tf-card , looks mostly the same with bluetooth added for twice the price , no firmware mentioning 2.0 device here https://www.crealitycloud.com/software-firmware/box?type=1 anyone knows what is inside?

The Creality WB-01 (unhelpfully referred to interchangably as the
"Creality Wifi Box" or simply the "Creality Box") is a device intended
to interface Creality brand 3D printers to a cloud service.

Specifications:

- SoC: MediaTek MT7688AN @ 580 MHz
- Flash: BoyaMicro BY25Q128AS (16 MiB, SPI NOR)
  Seems to be a clone/rebrand of the BoHong BH25Q128AS chip (or vice
  versa), support for which was added in commit 51b61fd
- RAM: 128 MiB DDR2 (Winbond W971GG6SB-25)
- Peripheral: Genesys Logic GL850G 2 port USB 2.0 hub
- I/O: 1x 10/100 Ethernet port, microSD SD-XC Class 10 slot, 4x LEDs,
  2x USB 2.0 ports, micro USB input (for power only), reset button
- FCC ID: 2AXH6CREALITY-BOX
- UART: test pads: (square on silkscreen) 3V3, TX, RX, GND

LEDs:
40 red    - microSD presence (OpenWrt has no function for this I think)
42 green  - "Cloud service or network configuration indicator"
43 blue   - Ethernet
44 yellow - Wi-Fi

Reset button = 38
SD presence switch = 23

MAC addresses:
factory+0x04 wlan and label MAC
factory+0x28 eth0
factory+0x2e has fc:ee:e6:xx:xx:xx here, but it appears to be unused

Stock firmware /proc/mtd
dev:    size   erasesize  name
mtd0: 01000000 00010000 "ALL"
mtd1: 00030000 00010000 "Bootloader"
mtd2: 00010000 00010000 "Config"
mtd3: 00010000 00010000 "Factory"
mtd4: 000d0000 00010000 "Kernel"
mtd5: 00ee0000 00010000 "RootFS"

Flashing:
1) Rename factory.bin to cxsw_update.tar.bz2
2) Copy it to the root of a FAT32 formatted microSD card.
3) Turn on the device, wait for it to start, then insert the card.
The stock firmware reads the install.sh script from this archive, the
build script I added creates one that works in a similar way.

Stock firmware can be downloaded from here:
http://file2-cdn.creality.com/model/cfg/box/latest/cxsw_update.tar.bz2
(source: https://forum.creality.com/topic/1701/ )
Stock firmware supports telnet root@10.10.10.254, password cxswprin

Recovery:
1) Add a file called root_uImage to the root of a FAT32 formatted
   USB drive (tip: a microSD to USB adapter works, as included with
   many Creality printers)
   You can find root_uImage in the stock firmware or you can rename
   sysupgrade.bin.
2) Hold the reset pinhole switch and plug in the device.
3) Keep holding reset for 5 seconds, then release.
4) Wait for the lights to stop flashing.

Signed-off-by: George Brooke <figgyc@figgyc.uk>
@figgyc
Copy link

figgyc commented Jun 27, 2022

Hello again.

I've investigated the issue @ihrapsa reported about the MAC addresses and I think I've fixed it with this new commit and I also did a rebase. (This commit changed how they're defined and I basically just copied that.) I've tested it and it works for me, but let me know if it works now for you.

Also thanks @fanoush for telling me about the v2. Obviously if someone was to work on the v2 it should be in a seperate patch but I had a little look into it anyway so someone can use this as a starting point. I haven't got one to test but I found the FCC photos here.
Inside it's got the same MT7688AN processor, so it'll be possible to port in theory.

The Bluetooth chipset, however, appears to be a YC1021 chip. I'm pretty sure this isn't usually used as a Linux Bluetooth chip, they might be using it as a serial interface? Anyway a very brief look at the "V2.00b28" firmware and I couldn't actually find anything about it, maybe its only mentioned in the binaries. I'm not sure if there's a later firmware or if the v2 uses a different firmware which I can't find, it might be unreleased because it seems like the device hasn't been shipped out yet, at least not to anyone on YouTube. I should note I'm not going to buy a v2 (especially not for that price!) because I don't need one. Of course if someone else would like to port it to OpenWrt then that'd be great. (There's a non-zero chance that the v1 custom firmware might "just work" (minus Bluetooth of course) on the v2, assuming they didn't change any of the pinouts or bootloader configuration, but try at your own risk, I have no idea.)

Lastly @adschm or anyone else, once this new commit is tested is there a chance this can be merged? It would be nice for this device to be upstreamed and get any future updates automatically (like the nvmem-cells thing!). The only thing I'm still a little uncertain about is the flash chip stuff but it works fine and functionally the only difference in the code really is the name. The upstream patch was last updated in February, not sure if it will get revived at some point.
I know @ihrapsa has been providing some binaries here (along with some admittedly hacky extra packages for 3D printer stuff) and they've been tested by a couple people although of course those have been on the outdated version.

@ihrapsa
Copy link
Author

ihrapsa commented Jun 28, 2022

Thank you @figgyc! That commit fixed the mac address issue. Hopefully, this will get merged soon 🙂

Copy link

@drone777 drone777 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this request only approvals missing?

Pull3D added a commit to Pull3D/openwrt that referenced this pull request Feb 21, 2023
ramips: add support for Creality WB-01 openwrt#3802
@TheRealGramdalf
Copy link

TheRealGramdalf commented Apr 15, 2023

Heya @figgyc, @drone777 , @adschm (and @ihrapsa if you're still active)

I've been working on https://github.com/TheRealGramdalf/KlipperWrt-EX recently, the next evolution of KlipperWRT (by ihrapsa) - as it seems to be mostly unmaintained at the moment.

It would be incredibly helpful to have this available in the image builder, particularly in docker if possible - it would make the eventual install guide simpler and easier to follow.
I have a creality wifi box, generously donated by Bronius on discord, and I'd be happy to do some testing if needed.

If this won't be getting merged upstream anytime soon, instructions from figgyc on how to build would be great - i.e. what distro did you use to compile, what packages, etc. I've had some weird issues when attempting to build in a debian 11 VM, mostly related to clang and llvm

Consider this a +1 for the merge :)

@chepo92
Copy link

chepo92 commented Jul 23, 2023

Why this is still not merged?

@fanoush
Copy link

fanoush commented Jul 25, 2023

Why this is still not merged?

It is shown right under last post

Review required
At least 6 approving reviews are required by reviewers with write access. 

From 2 year history of this PR it looks like quite unrealistic requirement :-(
I checked other PRs that got merged into this project recently and did not see so strict requirements anywhere.
However when I sorted PRs by age from oldest https://github.com/openwrt/openwrt/pulls?q=is%3Apr+is%3Aopen+sort%3Acreated-asc I see many PRs adding hardware support dragging for years like this :-(

I think @figgyc did excellent work fixing code review issues and rebasing it over time and the PR is not really so big and the one controversial issue with SPI flash was resolved long time ago so it is frustrating to see such contributions to be treated like this.

@adschm you made the most competent and thorough review, can you comment what can be realistically done to finally merge this?

@motolav
Copy link
Contributor

motolav commented Jul 25, 2023

@fanoush The reviewer requirement is just to block people from accidentally merging using github. OpenWrt's main repo hosted by the project elsewhere, this is just a mirror for other people to contribute through github. Maintainers are all volunteers and PRs that appear stalled tend to get hidden here. It could've been made a bit more clear that the PR was complete and ready for review.

@chepo92
Copy link

chepo92 commented Jul 26, 2023

How do we get this and others PRs reviewed and aproved for merge? maybe update labels

@hauke
Copy link
Member

hauke commented Aug 26, 2023

Is this device still working with the changes from this pull request?
The change adding support for the SPI NOR flash was removed. Does Linux detect a BoHong bh25q128as SPI NOR chip and is this working fine?

Maybe both companies are selling the same chip and did not change the ID, but just what is printed on the chip.

@andytimofeev
Copy link

it is pity this PR was buried.
Could it be reviewed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kernel pull request/issue with Linux kernel related changes needs changes target/ramips pull request/issue for ramips target
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet