[Buildroot] [PATCH v1] busybox: Fix rtcwake to use /dev/rtc0 properly

[Buildroot] [PATCH v1] busybox: Fix rtcwake to use /dev/rtc0 properly

[Andy Shevchenko]

rtcwake from busybox has failed in case the /dev/rtc is a symlink
(which is default case for udev enabled systems) due to wrong pathname
used for a sysfs wakeup attribute.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 ...bb-Try-dev-rtc0-first-followed-by-dev-rtc.patch | 45 ++++++++++++++++++++++
 1 file changed, 45 insertions(+)
 create mode 100644 package/busybox/0006-libbb-Try-dev-rtc0-first-followed-by-dev-rtc.patch

diff --git a/package/busybox/0006-libbb-Try-dev-rtc0-first-followed-by-dev-rtc.patch b/package/busybox/0006-libbb-Try-dev-rtc0-first-followed-by-dev-rtc.patch
new file mode 100644
index 0000000000..0702c3c065
--- /dev/null
+++ b/package/busybox/0006-libbb-Try-dev-rtc0-first-followed-by-dev-rtc.patch
@@ -0,0 +1,45 @@
+From b59088ab14fb9c336414e2c9b27a9f5b7447ce5b Mon Sep 17 00:00:00 2001
+From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
+Date: Thu, 23 Nov 2017 19:33:08 +0200
+Subject: [PATCH v1] libbb: Try /dev/rtc0 first followed by /dev/rtc
+
+In case we run
+
+% rtcwake -s5 -mmem
+
+in udev environment we will get an error:
+
+rtcwake: /dev/rtc not enabled for wakeup events
+
+because /dev/rtc is a symlink to /dev/rtc0 in Buildroot and other Linux
+distributions (like Fedora) and make_wakeup() obviously fails.
+
+As a quick fix just reorder probe of device nodes to try /dev/rtc0
+first.
+
+The proper solution, of course much expensive by code footprint,
+is to resolve /dev/rtc to a proper device node and then
+to follow sysfs attributes.
+
+Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
+---
+ libbb/rtc.c | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/libbb/rtc.c b/libbb/rtc.c
+index c4117ba34..e087a8fd6 100644
+--- a/libbb/rtc.c
++++ b/libbb/rtc.c
+@@ -64,8 +64,8 @@ int FAST_FUNC rtc_xopen(const char **default_rtc, int flags)
+ {
+ 	int rtc;
+ 	const char *name =
+-		"/dev/rtc""\0"
+ 		"/dev/rtc0""\0"
++		"/dev/rtc""\0"
+ 		"/dev/misc/rtc""\0";
+ 
+ 	if (!*default_rtc)
+-- 
+2.15.0
+
-- 
2.15.0

[Buildroot] [PATCH v1] busybox: Fix rtcwake to use /dev/rtc0 properly

[Arnout Vandecappelle]

 Hi Andy,

On 23-11-17 19:39, Andy Shevchenko wrote:
> rtcwake from busybox has failed in case the /dev/rtc is a symlink
> (which is default case for udev enabled systems) due to wrong pathname
> used for a sysfs wakeup attribute.

 In Buildroot, we don't accept "feature patches" for packages. We try to limit
to patches that fix the build or complete breakage, sometimes also to make it
work together with other packages. I think this patch doesn't fall in that category.

 For sure, you should first send the patch upstream. Particularly in the case of
busybox, Denys often proposes improved patches. If it gets accepted upstream,
then we can consider including it in Buildroot as well.

 Regards,
 Arnout

> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  ...bb-Try-dev-rtc0-first-followed-by-dev-rtc.patch | 45 ++++++++++++++++++++++
>  1 file changed, 45 insertions(+)
>  create mode 100644 package/busybox/0006-libbb-Try-dev-rtc0-first-followed-by-dev-rtc.patch
> 
> diff --git a/package/busybox/0006-libbb-Try-dev-rtc0-first-followed-by-dev-rtc.patch b/package/busybox/0006-libbb-Try-dev-rtc0-first-followed-by-dev-rtc.patch
> new file mode 100644
> index 0000000000..0702c3c065
> --- /dev/null
> +++ b/package/busybox/0006-libbb-Try-dev-rtc0-first-followed-by-dev-rtc.patch
> @@ -0,0 +1,45 @@
> +From b59088ab14fb9c336414e2c9b27a9f5b7447ce5b Mon Sep 17 00:00:00 2001
> +From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> +Date: Thu, 23 Nov 2017 19:33:08 +0200
> +Subject: [PATCH v1] libbb: Try /dev/rtc0 first followed by /dev/rtc
> +
> +In case we run
> +
> +% rtcwake -s5 -mmem
> +
> +in udev environment we will get an error:
> +
> +rtcwake: /dev/rtc not enabled for wakeup events
> +
> +because /dev/rtc is a symlink to /dev/rtc0 in Buildroot and other Linux
> +distributions (like Fedora) and make_wakeup() obviously fails.
> +
> +As a quick fix just reorder probe of device nodes to try /dev/rtc0
> +first.
> +
> +The proper solution, of course much expensive by code footprint,
> +is to resolve /dev/rtc to a proper device node and then
> +to follow sysfs attributes.
> +
> +Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> +---
> + libbb/rtc.c | 2 +-
> + 1 file changed, 1 insertion(+), 1 deletion(-)
> +
> +diff --git a/libbb/rtc.c b/libbb/rtc.c
> +index c4117ba34..e087a8fd6 100644
> +--- a/libbb/rtc.c
> ++++ b/libbb/rtc.c
> +@@ -64,8 +64,8 @@ int FAST_FUNC rtc_xopen(const char **default_rtc, int flags)
> + {
> + 	int rtc;
> + 	const char *name =
> +-		"/dev/rtc""\0"
> + 		"/dev/rtc0""\0"
> ++		"/dev/rtc""\0"
> + 		"/dev/misc/rtc""\0";
> + 
> + 	if (!*default_rtc)
> +-- 
> +2.15.0
> +
> 

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH v1] busybox: Fix rtcwake to use /dev/rtc0 properly

[Andy Shevchenko]

On Thu, 2017-11-23 at 23:46 +0100, Arnout Vandecappelle wrote:
>  Hi Andy,
> 
> On 23-11-17 19:39, Andy Shevchenko wrote:
> > rtcwake from busybox has failed in case the /dev/rtc is a symlink
> > (which is default case for udev enabled systems) due to wrong
> > pathname
> > used for a sysfs wakeup attribute.
> 
>  In Buildroot, we don't accept "feature patches" for packages.

To be honest it's not a feature patch at all. It fixes (okay,
workarounds) obvious bug in rtcwake logic. Easy to reproduce. 100%
reproducible.

>  We try to limit
> to patches that fix the build or complete breakage, sometimes also to
> make it
> work together with other packages. I think this patch doesn't fall in
> that category.

Whatever, your choice at the end.

> 
>  For sure, you should first send the patch upstream.

Are you sure I didn't?
The policy of Busybox mailing list is to reject (I'm not subscriber and
after a such policy would not like to be one). Happy contribution!

>  Particularly in the case of
> busybox, Denys often proposes improved patches. If it gets accepted
> upstream,
> then we can consider including it in Buildroot as well.

Good luck!

I'm done with it. If Denys is caring about project he will take the
series (there are more patches than just one) from his private mailbox
(Cc was there as well).

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

[Buildroot] [PATCH v1] busybox: Fix rtcwake to use /dev/rtc0 properly

[Arnout Vandecappelle]

 Hi Andy,

 Sorry, I should have started my mail with:

 Thank you very much for your contributation. I'm afraid, however, we will not
accept it as is in Buildroot.


On 24-11-17 14:27, Andy Shevchenko wrote:
> On Thu, 2017-11-23 at 23:46 +0100, Arnout Vandecappelle wrote:
>>  Hi Andy,
>>
>> On 23-11-17 19:39, Andy Shevchenko wrote:
>>> rtcwake from busybox has failed in case the /dev/rtc is a symlink
>>> (which is default case for udev enabled systems) due to wrong
>>> pathname
>>> used for a sysfs wakeup attribute.
>>
>>  In Buildroot, we don't accept "feature patches" for packages.
> 
> To be honest it's not a feature patch at all. It fixes (okay,
> workarounds) obvious bug in rtcwake logic. Easy to reproduce. 100%
> reproducible

 Yes, that's why I continued with:

>>  We try to limit
>> to patches that fix the build or complete breakage, sometimes also to
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

 For other bugs, we will happily include upstream patches. But patches that are
not yet upstream are dangerous: how can we be sure that it doesn't break things?
In this particular case, you patch *will* actually break things for a who does
have a working /dev/rtc but it is something different than /dev/rtc0. We want
upstream to take the responsibility for that.

>> make it
>> work together with other packages. I think this patch doesn't fall in
>> that category.
> 
> Whatever, your choice at the end.
> 
>>
>>  For sure, you should first send the patch upstream.
> 
> Are you sure I didn't?

 I checked the mailing list, it wasn't there. I checked google, and found nothing.

> The policy of Busybox mailing list is to reject (I'm not subscriber and

 Buildroot mailing list has the same policy (well, it goes into a moderation
queue but that is filled up with so much spam that it's unlikely it ever makes
it to the list). If you have a solution to bar spam from mailing lists, please
share it!

> after a such policy would not like to be one). Happy contribution!
> 
>>  Particularly in the case of
>> busybox, Denys often proposes improved patches. If it gets accepted
>> upstream,
>> then we can consider including it in Buildroot as well.
> 
> Good luck!
> 
> I'm done with it. If Denys is caring about project he will take the
> series (there are more patches than just one) from his private mailbox
> (Cc was there as well).

 If/when he does, feel free to resubmit with a reference to the upstream commit.

 Thanks!

 Regards,
 Arnout


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH v1] busybox: Fix rtcwake to use /dev/rtc0 properly

[Andy Shevchenko]

On Sat, 2017-11-25 at 18:40 +0100, Arnout Vandecappelle wrote:
>  Hi Andy,
> 
>  Sorry, I should have started my mail with:
> 
>  Thank you very much for your contributation. I'm afraid, however, we
> will not
> accept it as is in Buildroot.

Much better :-)

> > >  In Buildroot, we don't accept "feature patches" for packages.
> > 
> > To be honest it's not a feature patch at all. It fixes (okay,
> > workarounds) obvious bug in rtcwake logic. Easy to reproduce. 100%
> > reproducible
> 
>  Yes, that's why I continued with:
> 
> > >  We try to limit
> > > to patches that fix the build or complete breakage, sometimes also
> > > to
> 
>               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
>  For other bugs, we will happily include upstream patches. But patches
> that are
> not yet upstream are dangerous: how can we be sure that it doesn't
> break things?
> In this particular case, you patch *will* actually break things for a
> who does
> have a working /dev/rtc but it is something different than /dev/rtc0.
> We want
> upstream to take the responsibility for that.

Of course.

Btw, how many use cases where people use rtcwake from busybox vs. util-
linux?

I doubt there are many.

> > >  For sure, you should first send the patch upstream.
> > 
> > Are you sure I didn't?
> 
>  I checked the mailing list, it wasn't there. I checked google, and
> found nothing.
> 
> > The policy of Busybox mailing list is to reject (I'm not subscriber
> > and
> 
>  Buildroot mailing list has the same policy (well, it goes into a
> moderation
> queue but that is filled up with so much spam that it's unlikely it
> ever makes
> it to the list).

Exactly! Not reject, but put in to moderation queue. Feel the
difference!

>  If you have a solution to bar spam from mailing lists, please
> share it!

See above.

> > after a such policy would not like to be one). Happy contribution!
> > 
> > >  Particularly in the case of
> > > busybox, Denys often proposes improved patches. If it gets
> > > accepted
> > > upstream,
> > > then we can consider including it in Buildroot as well.
> > 
> > Good luck!
> > 
> > I'm done with it. If Denys is caring about project he will take the
> > series (there are more patches than just one) from his private
> > mailbox
> > (Cc was there as well).
> 
>  If/when he does, feel free to resubmit with a reference to the
> upstream commit.

OK.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy