Basic support for Kobo Nia - WiFi and USB storage export. #634
Conversation
| @@ -45,6 +45,13 @@ if [ -f /mnt/onboard/.kobo/Kobo.tgz -o -f /mnt/onboard/.kobo/KoboRoot.tgz \ | |||
| exec /etc/init.d/rcS | |||
| fi | |||
|
|
|||
| # Figure out our platform. | |||
| PLATFORM=freescale | |||
| if [ `dd if=/dev/mmcblk0 bs=512 skip=1024 count=1 | grep -c "HW CONFIG"` == 1 ]; then | |||
There was a problem hiding this comment.
How does this test work? Why use grep?
The string "HW CONFIG" looks rather generic. Are you sure this really identifies this platform?
There was a problem hiding this comment.
Yup, all kobos from the original Touch have this blob at the same offset on unpartitioned internal storage.
| # Figure out our platform. | ||
| PLATFORM=freescale | ||
| if [ `dd if=/dev/mmcblk0 bs=512 skip=1024 count=1 | grep -c "HW CONFIG"` == 1 ]; then | ||
| CPU=`ntx_hwconfig -s -p /dev/mmcblk0 CPU` |
There was a problem hiding this comment.
ntx_hwconfig is a tool used to read blob values from linux userspace.
This will return a cpu identifier, such as 507,508, etc.
Some legacy versions might ship a blob older than the ntx_hwconfig tool and that will cause the command to throw some errors to stderr, so it is a good idea to redirect errors with:
CPU=$(ntx_hwconfig -s -p /dev/mmcblk0 CPU 2>/dev/null)
kobo/rcS
Outdated
| @@ -62,8 +69,19 @@ fi | |||
| /bin/mknod /dev/null c 1 3 | |||
| /bin/chmod 666 /dev/null | |||
|
|
|||
| /sbin/udevd --daemon | |||
| /sbin/udevadm trigger | |||
| # udev doesn't create all /dev/ entries on some platforms (e.g. Nia), | |||
There was a problem hiding this comment.
Which ones are missing? Are there other ways to create them?
This looks like a really really ugly kludge. And your code does it on all platforms. I'd like to avoid this on all platforms where the kludge is not strictly necessary.
And I don't understand the kludge. How does it work?
There was a problem hiding this comment.
Thanks for a quick review! This is what the supplied init script on the Nia is doing, so I suspect we won't fully understand it without knowing the hardware details. I'll pull out C++ changes into a separate commit, but for here our options are either to use this code, change it to uncompress the .tgz unconditionally if it's available, or use mknod to create the device tree if some required /dev/ entries are missing. My worry with unconditionally using .tgz is that it's different from what Kobo's init is doing, which is known to work on all platforms.
Basically what happened is that sometime between 6.8 and 7 rcS switched to calling udev instead of manually creating /dev/ entires. On the Nia this does not work, and installing XCSoar 7 bricks it.
There was a problem hiding this comment.
I poked around a bit more and and added documentation to what this is doing. Note that the same code is present in the original Kobo boot script, and in TopHat, so it should allow XCSoar to boot on more platforms without impacting others.
Let me know if you prefer to use one of the other approaches I outlined above, though personally I'd prefer to follow what is proven to work in existing boot scripts.
There was a problem hiding this comment.
I'd prefer not to apply this ugly kludge to devices which don't need it. Keep the scope of bad code as small as possible.
There was a problem hiding this comment.
There was a problem hiding this comment.
It already works this way
No, it doesn't. You changed the behavior of all devices, not just the Kobo Nia.
There was a problem hiding this comment.
The behavior is conditional on the platform. We're going in circles here, if you don't like this approach either pick one of the alternatives, as I keep asking, or make a concrete suggestion. This has stopped being productive a few iterations ago.
There was a problem hiding this comment.
The behavior is conditional on the platform.
Yes, but that only appears to contradict with my statement "You changed the behavior of all devices, not just the Kobo Nia".
We're going in circles here
True, because I see code changes where I don't want any changes, but you don't seem to see those changes in your own code. If you want to support the Kobo Nia and take responsibility for that platform, do that, but don't change behavior for all the other Kobos. (Unless you want to be the responsible maintainer for all of them, which I'd very much welcome.)
There was a problem hiding this comment.
True, because I see code changes where I don't want any changes,
Then pick one of the options I've repeatedly suggested instead of ignoring them entirely, just like you did here. This entire discussion is you saying "I don't like it" to the approach taken by the device manufacturer, with no suggestions of your own nor any acknowledgement of multiple options I keep offering.
There was a problem hiding this comment.
Maybe this wasn't clear enough:
I'd prefer not to apply this ugly kludge to devices which don't need it. Keep the scope of bad code as small as possible.
This means: go ahead with bad code, but limit it to devices that really really need it; i.e. only Kobo Nia. Do not change anything for all other devices.
|
I still don't understand the rcS changes which you did for all Kobo devices, not just the Nia.
Did you attempt to debug it? What does the serial console say? |
I added comments to the changes, happy to clarify what you think is missing. The /dev/ setup logic now matches what Kobo itself is doing, if you don't like it we can use one of the alternate approaches I proposed above, or I can special case it to just Nia. There would be a decent chance of bricking another Kobo as there must be a reason why they don't rely on udev for all platforms.
I did debug it, /dev/ entries are missing, that's why it doesn't boot and that's why I needed to change the boot script. I don't have a serial console on my Kobo, this took repeated iterations of redirecting boot script output and pulling the SD card out to see what the problem was. |
|
This is the commit that prevents booting on the Nia, FWIW -2de78d4 . Adding /bin/mknod calls back in would also get it to boot. |
|
I realize this probably doesn't help much, but my current build of XCSoar 7.8 or 7.9 HEAD boots fine on my Kobo Clara HD and hasn't brick. Are both the Clara HD and Nia are the Mark7 devices family so should be similar? I wonder if something changed in a newer kernel / firmware update from Kobo? My current Kobo firmware and kernel are from around June 2020 so are not the latest, and I haven't updated it recently, but I believe there may be newer kernels / firmware available (https://github.com/kobolabs/Kobo-Reader). I may have to try update my Kobo firmware on my Clara and see what happens if I have time. |
|
Clara HD and Nia are actually different platforms per Wikipedia, looks like Nia is the only one using the ULL chipset so far, but could be a firmware issue as well of course (I'm on the latest one for the Nia from February 2021). LK8000, which I think does run on the Nia, does not rely on udev for everything - they create some /dev/ entries via mknod. I suspect that would be enough for the Nia as well. |
|
@MaxKellermann so what would you like to do here? |
|
Did you address all of my comments yet? |
What's missing? There's an updated request with additional comments, and proposed alternatives. |
| /sbin/udevadm settle --timeout=2 | ||
| /sbin/udevadm control --env=STARTUP= | ||
| # Create a /dev/ tree backup to restore next time around. | ||
| [ $PLATFORM != freescale ] && tar cpzf /etc/udev.tgz /dev & |
There was a problem hiding this comment.
This will leave a Zombie process around.
There was a problem hiding this comment.
This won't affect any supported model. The freescale platform only covers the original Kobo and the Kobo Wifi, both models without touchscreen.
Anyways backgrounded jobs should be calling waitpid() on completion. No problem if you want to run this in the foreground either.
There was a problem hiding this comment.
Now that I read this again it doesn't make much sense. The tar cpzf ... will never execute as the condition isFreescale and isNotFreescale cannot be true at the same time.
There was a problem hiding this comment.
Anyways backgrounded jobs should be calling
waitpid()on completion.
Not quite - the parent process should be calling waitpid(), not the backgrounded job.
So, who's going to do that?
There was a problem hiding this comment.
Not familiarized with this repo to tell you. If rcS runs the entire time because XCSoar doesn't daemonize then either insert a wait or just don't background. Otherwise the child will be reaped when the script finish its execution.
But as it's currently wroten that chunk of code won't execute, as the precondition doesn't match the specific condition to untar device nodes.
There was a problem hiding this comment.
If rcS runs the entire time
It does not. It execs XCSoar's KoboMenu, replacing itself.
because XCSoar doesn't daemonize
It does not, but it does not need to. The KoboMenu process just keeps running until you power the Kobo off. There's no point in daemonizing it.
MaxKellermann
added
the
needs-work
lordfolken
force-pushed
the
master
branch
2 times, most recently
from
bb2289c to
65aacd6
Compare
|
@vnks Thanks for the change and @MaxKellermann @pazos thanks for helping review it. I recently ran into issue with Kobo Nia and would love to see this change merged, fixing the issues with Kobo Nia. Just wanted to bring this PR up as I’m curious what is remaining to get this in? |
|
@zabirauf I posted the answer to your question already more than half a year ago. |
Brief summary of the changes
Updated boot script to work on Nia (without this XCSoar 7 bricks the device and requires a full factory reset), and required kernel modules for WiFi and USB Storage export to work.
Related issues and discussions