| Message ID | 1eac0db3-17ce-8ebd-4997-8b1c282126e4@wizzup.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | N900: Remove mmc1 "safety feature"? (was: Re: mmc0 on Nokia N900 on Linux 5.4.18) | expand |
Hi! > >>> When booting Linux 5.4.18 with omap2plus_defconfig, I no longer get a > >>> /dev/mmcblk0 device - the one corresponding to my microSD card, where > >>> u-boot also loads the kernel from. > >>> > >>> This also likely seems to be a regression, but I haven't tried to hunt > >>> it down yet. > >>> > >>> Kernel log attached, below. > >> > >> Do you have back cover attached? That's common trap... > > > > Right. > > > > No, I did not, since I have the serial pins connected to my PC, and the > > whole device is mounted on a piece of hardware for that. I thought that > > we fixed the problem where we require the cover to be on... > > Indeed, this was the problem. > > Is there really a reason to have this in the DTS? I have a hard time > imagining a use case for not showing SD card to users or developers > simply because the casing is open. If anything, this sounds like it > should be a userspace thing. It is very bad for debugging, agreed. It makes sense for regular usage: when user removes back cover, system unmounts the u-SD card, so that it is ready for user to remove it. Note that we do _not_ have "remove the card safely" button in the UI; back cover serves that purpose. That said... for Leste just keep the patch. And maybe apply that one to shutdown system when battery is low :-). Best regards, Pavel
Hi, On 08/02/2020 23:06, Pavel Machek wrote: > > It is very bad for debugging, agreed. > > It makes sense for regular usage: when user removes back cover, system > unmounts the u-SD card, so that it is ready for user to remove > it. Note that we do _not_ have "remove the card safely" button in the > UI; back cover serves that purpose. > > That said... for Leste just keep the patch. And maybe apply that one > to shutdown system when battery is low :-). So how does this currently happen, the unmounting? Does the mmc1 card just disappear from /dev/ without any safe unmount? I don't understand how the current setup can work from a userspace point of view. Userspace could react on kernel events that tell it the cover is open, but I assume the kernel doesn't just decide to nuke the node from /dev/, so in that case the current DTS setup still doesn't make sense, right? What am I missing? Could you describe how this would work in a 'real life' scenario? Cheers, Merlijn
I suppose the real life scenario would be: 0. The OS runs on eMMC. 1. The user opens his phone back cover and inserts MicroSD card. 2. Kernel doesn't try to access the card until the cover is closed. It seems wise to me: we don't want to damage user's data or hardware. Accessing data on unstable medium can't be considered safe. While the cover is open and user just inserted the microsd card everything can happen - the phone can be dropped, may be unstable contact to microsd while the user interacting it, etc. It seems to be inconvenient for us, while we run OS mostly from microsd. But removing such behaviour from the kernel completely doesn't seem to be a good idea generally. Vanilla kernel shouldn't be dedicated for debugging purposes or edge cases (like running only from external MMC). Running from embedded MMC should be common for most users in perspective. If I remember correctly, if the microsd is already mounted and the back cover is open, nothing bad happens. It continues to work. You can unmount the card and remove it safely. But you are not going to see the new card insertion detected until you close the cover. I'm not sure about the last paragraph. Please correct me if I'm wrong about how it actually works. >> It is very bad for debugging, agreed. >> >> It makes sense for regular usage: when user removes back cover, system >> unmounts the u-SD card, so that it is ready for user to remove >> it. Note that we do _not_ have "remove the card safely" button in the >> UI; back cover serves that purpose. >> >> That said... for Leste just keep the patch. And maybe apply that one >> to shutdown system when battery is low :-). > > So how does this currently happen, the unmounting? Does the mmc1 card > just disappear from /dev/ without any safe unmount? I don't understand > how the current setup can work from a userspace point of view. > > Userspace could react on kernel events that tell it the cover is open, > but I assume the kernel doesn't just decide to nuke the node from /dev/, > so in that case the current DTS setup still doesn't make sense, right? > > What am I missing? Could you describe how this would work in a 'real > life' scenario?
Hi, On 09/02/2020 04:48, Arthur D. wrote: > I suppose the real life scenario would be: > > 0. The OS runs on eMMC. > 1. The user opens his phone back cover and inserts MicroSD card. > 2. Kernel doesn't try to access the card until the cover is closed. This kinda makes sense - but assume the card is already in there, and the user wants to swap them? Then it doesn't help. > It seems wise to me: we don't want to damage user's data or hardware. > Accessing data on unstable medium can't be considered safe. > While the cover is open and user just inserted the microsd card everything > can happen - the phone can be dropped, may be unstable contact to microsd > while the user interacting it, etc. Maybe I am just used to the microsd port being exposed, but I have no other ARM device that doesn't have the microsd slot easily exposed. Many phones, cables, development boards, and they all just allow me to insert a microSD card, and it'll work. Clearly the manufacturers didn't have a similar worry there. > It seems to be inconvenient for us, while we run OS mostly from microsd. > But removing such behaviour from the kernel completely doesn't seem to > be a good idea generally. Vanilla kernel shouldn't be dedicated for > debugging purposes or edge cases (like running only from external MMC). > Running from embedded MMC should be common for most users in perspective. > > If I remember correctly, if the microsd is already mounted and the back > cover is open, nothing bad happens. It continues to work. You can unmount > the card and remove it safely. But you are not going to see the new card > insertion detected until you close the cover. I think the last paragraph might actually be key, because when you said that, my initial reaction was: "aha! that makes some sense". Still not sure if it warrants the hassle it currently is. And as it stands, we also disable this feature in Maemo Leste, so we clearly decided we don't want it either. ;-) > I'm not sure about the last paragraph. Please correct me if I'm wrong about > how it actually works. OK. If we don't want to remove this, can we at least somehow have a runtime warning? I spent quite some time searching my dmesg, trying to figure out why I couldn't even see the mmc interface that I needed. Cheers, Merlijn
Hi, On Sat, Feb 08, 2020 at 11:19:21PM +0100, Merlijn Wajer wrote: > On 08/02/2020 23:06, Pavel Machek wrote: > > It is very bad for debugging, agreed. > > > > It makes sense for regular usage: when user removes back cover, system > > unmounts the u-SD card, so that it is ready for user to remove > > it. Note that we do _not_ have "remove the card safely" button in the > > UI; back cover serves that purpose. > > > > That said... for Leste just keep the patch. And maybe apply that one > > to shutdown system when battery is low :-). > > So how does this currently happen, the unmounting? Does the mmc1 card > just disappear from /dev/ without any safe unmount? I don't understand > how the current setup can work from a userspace point of view. > > Userspace could react on kernel events that tell it the cover is open, > but I assume the kernel doesn't just decide to nuke the node from /dev/, > so in that case the current DTS setup still doesn't make sense, right? > > What am I missing? Could you describe how this would work in a 'real > life' scenario? I don't think it can work with the current mainline kernel. I recall the original Nokia kernel used the GPIO for "cover switch" instead of card detect, and it was visible in sysfs, and this allowed userspace to react on cover removal.. In the mainline kernel we have this for older Nokia devices (770, N8x0), but not for N900. Still it wouldn't help much for "safe unmount" as the unmount can take quite a while, and user may remove the card too early. A.
Hi, On Sun, Feb 09, 2020 at 06:48:47AM +0300, Arthur D. wrote: > I suppose the real life scenario would be: > > 0. The OS runs on eMMC. > 1. The user opens his phone back cover and inserts MicroSD card. > 2. Kernel doesn't try to access the card until the cover is closed. > > It seems wise to me: we don't want to damage user's data or hardware. > Accessing data on unstable medium can't be considered safe. > While the cover is open and user just inserted the microsd card everything > can happen - the phone can be dropped, may be unstable contact to microsd > while the user interacting it, etc. If the cover is open, you should also avoid writing to flash or eMMC, as the battery may get removed easily, and the end result can be bad. A.
Hi, On 10/02/2020 20:27, Aaro Koskinen wrote: >> >> So how does this currently happen, the unmounting? Does the mmc1 card >> just disappear from /dev/ without any safe unmount? I don't understand >> how the current setup can work from a userspace point of view. >> >> Userspace could react on kernel events that tell it the cover is open, >> but I assume the kernel doesn't just decide to nuke the node from /dev/, >> so in that case the current DTS setup still doesn't make sense, right? >> >> What am I missing? Could you describe how this would work in a 'real >> life' scenario? > > I don't think it can work with the current mainline kernel. > > I recall the original Nokia kernel used the GPIO for "cover switch" > instead of card detect, and it was visible in sysfs, and this allowed > userspace to react on cover removal.. In the mainline kernel we have > this for older Nokia devices (770, N8x0), but not for N900. Still it > wouldn't help much for "safe unmount" as the unmount can take quite a > while, and user may remove the card too early. Shall I send in a patch removing this check from the device tree, then? Cheers, Merlijn
Hi, On Wed, Feb 12, 2020 at 02:02:53PM +0100, Merlijn Wajer wrote: > On 10/02/2020 20:27, Aaro Koskinen wrote: > >> So how does this currently happen, the unmounting? Does the mmc1 card > >> just disappear from /dev/ without any safe unmount? I don't understand > >> how the current setup can work from a userspace point of view. > >> > >> Userspace could react on kernel events that tell it the cover is open, > >> but I assume the kernel doesn't just decide to nuke the node from /dev/, > >> so in that case the current DTS setup still doesn't make sense, right? > >> > >> What am I missing? Could you describe how this would work in a 'real > >> life' scenario? > > > > I don't think it can work with the current mainline kernel. > > > > I recall the original Nokia kernel used the GPIO for "cover switch" > > instead of card detect, and it was visible in sysfs, and this allowed > > userspace to react on cover removal.. In the mainline kernel we have > > this for older Nokia devices (770, N8x0), but not for N900. Still it > > wouldn't help much for "safe unmount" as the unmount can take quite a > > while, and user may remove the card too early. > > Shall I send in a patch removing this check from the device tree, then? I agree it's mostly annoying and suggest to convert the GPIO into a gpio-key using a newly defined SW_MACHINE_COVER /* set = cover closed */ Similar to the camera lens cover. It means userspace has a chance to show a warning, but system works generally. I think it's ok to assume that people running mainline on their N900 nowadays know what could happen when they hot-remove SD cards. -- Sebastian
Hi, On 14.02.20 г. 3:39 ч., Sebastian Reichel wrote: > Hi, > > On Wed, Feb 12, 2020 at 02:02:53PM +0100, Merlijn Wajer wrote: >> On 10/02/2020 20:27, Aaro Koskinen wrote: >>>> So how does this currently happen, the unmounting? Does the mmc1 card >>>> just disappear from /dev/ without any safe unmount? I don't understand >>>> how the current setup can work from a userspace point of view. >>>> >>>> Userspace could react on kernel events that tell it the cover is open, >>>> but I assume the kernel doesn't just decide to nuke the node from /dev/, >>>> so in that case the current DTS setup still doesn't make sense, right? >>>> >>>> What am I missing? Could you describe how this would work in a 'real >>>> life' scenario? >>> >>> I don't think it can work with the current mainline kernel. >>> >>> I recall the original Nokia kernel used the GPIO for "cover switch" >>> instead of card detect, and it was visible in sysfs, and this allowed >>> userspace to react on cover removal.. In the mainline kernel we have >>> this for older Nokia devices (770, N8x0), but not for N900. Still it >>> wouldn't help much for "safe unmount" as the unmount can take quite a >>> while, and user may remove the card too early. >> >> Shall I send in a patch removing this check from the device tree, then? > > I agree it's mostly annoying and suggest to convert the GPIO > into a gpio-key using a newly defined > > SW_MACHINE_COVER /* set = cover closed */ > > Similar to the camera lens cover. It means userspace has a chance to > show a warning, but system works generally. I think it's ok to > assume that people running mainline on their N900 nowadays know what > could happen when they hot-remove SD cards. > That sounds way better to me than just removing the check. Regards, Ivo
Hi, On 14/02/2020 02:39, Sebastian Reichel wrote: > Hi, > >> Shall I send in a patch removing this check from the device tree, then? > > I agree it's mostly annoying and suggest to convert the GPIO > into a gpio-key using a newly defined > > SW_MACHINE_COVER /* set = cover closed */ > > Similar to the camera lens cover. It means userspace has a chance to > show a warning, but system works generally. I think it's ok to > assume that people running mainline on their N900 nowadays know what > could happen when they hot-remove SD cards. OK, I will make a patch. Thanks! Cheers, Merlijn
* Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> [200214 05:52]: > Hi, > > On 14.02.20 г. 3:39 ч., Sebastian Reichel wrote: > > Hi, > > > > On Wed, Feb 12, 2020 at 02:02:53PM +0100, Merlijn Wajer wrote: > > > On 10/02/2020 20:27, Aaro Koskinen wrote: > > > > > So how does this currently happen, the unmounting? Does the mmc1 card > > > > > just disappear from /dev/ without any safe unmount? I don't understand > > > > > how the current setup can work from a userspace point of view. > > > > > > > > > > Userspace could react on kernel events that tell it the cover is open, > > > > > but I assume the kernel doesn't just decide to nuke the node from /dev/, > > > > > so in that case the current DTS setup still doesn't make sense, right? > > > > > > > > > > What am I missing? Could you describe how this would work in a 'real > > > > > life' scenario? > > > > > > > > I don't think it can work with the current mainline kernel. > > > > > > > > I recall the original Nokia kernel used the GPIO for "cover switch" > > > > instead of card detect, and it was visible in sysfs, and this allowed > > > > userspace to react on cover removal.. In the mainline kernel we have > > > > this for older Nokia devices (770, N8x0), but not for N900. Still it > > > > wouldn't help much for "safe unmount" as the unmount can take quite a > > > > while, and user may remove the card too early. > > > > > > Shall I send in a patch removing this check from the device tree, then? > > > > I agree it's mostly annoying and suggest to convert the GPIO > > into a gpio-key using a newly defined > > > > SW_MACHINE_COVER /* set = cover closed */ > > > > Similar to the camera lens cover. It means userspace has a chance to > > show a warning, but system works generally. I think it's ok to > > assume that people running mainline on their N900 nowadays know what > > could happen when they hot-remove SD cards. > > > > That sounds way better to me than just removing the check. Sounds good to me too. The MMC card is there and usable when cover is off, so the cover GPIO is not the same as MMC detect GPIO. Regards, Tony
diff --git a/arch/arm/boot/dts/omap3-n900.dts b/arch/arm/boot/dts/omap3-n900.dts index 7028a7cb2849e..03481302704de 100644 --- a/arch/arm/boot/dts/omap3-n900.dts +++ b/arch/arm/boot/dts/omap3-n900.dts @@ -805,10 +805,6 @@ pinctrl-0 = <&mmc1_pins>; vmmc-supply = <&vmmc1>; bus-width = <4>; - /* For debugging, it is often good idea to remove this GPIO. - It means you can remove back cover (to reboot by removing - battery) and still use the MMC card. */ - cd-gpios = <&gpio6 0 GPIO_ACTIVE_LOW>; /* 160 */ }; /* most boards use vaux3, only some old versions use vmmc2 instead */