diff mbox

[LEDE-DEV,v2,1/3] base-files: add/convert generic board detection scripts

Message ID 1494321407-7422-1-git-send-email-roman@advem.lv
State Not Applicable
Delegated to: Mathias Kresin
Headers show

Commit Message

Roman Yeryomin May 9, 2017, 9:16 a.m. UTC 
Signed-off-by: Roman Yeryomin <roman@advem.lv>
---
 package/base-files/files/lib/board_detect.sh                 |  9 +++++++++
 package/base-files/files/lib/preinit/03_preinit_board_detect | 11 +++++++++++
 package/base-files/files/lib/preinit/10_sysinfo              | 10 ----------
 3 files changed, 20 insertions(+), 10 deletions(-)
 create mode 100644 package/base-files/files/lib/board_detect.sh
 create mode 100644 package/base-files/files/lib/preinit/03_preinit_board_detect
 delete mode 100644 package/base-files/files/lib/preinit/10_sysinfo

Comments

Piotr Dymacz May 9, 2017, 9:51 a.m. UTC | #1 
Hello Roman,

Your mail addresses in from header and in SoB line don't match.
Also, some questions inline, below.

On 09.05.2017 11:16, Roman Yeryomin wrote:
> Signed-off-by: Roman Yeryomin <roman@advem.lv>
> ---
>  package/base-files/files/lib/board_detect.sh                 |  9 +++++++++
>  package/base-files/files/lib/preinit/03_preinit_board_detect | 11 +++++++++++
>  package/base-files/files/lib/preinit/10_sysinfo              | 10 ----------
>  3 files changed, 20 insertions(+), 10 deletions(-)
>  create mode 100644 package/base-files/files/lib/board_detect.sh
>  create mode 100644 package/base-files/files/lib/preinit/03_preinit_board_detect
>  delete mode 100644 package/base-files/files/lib/preinit/10_sysinfo
>
> diff --git a/package/base-files/files/lib/board_detect.sh b/package/base-files/files/lib/board_detect.sh
> new file mode 100644
> index 0000000..e2f0f89
> --- /dev/null
> +++ b/package/base-files/files/lib/board_detect.sh
> @@ -0,0 +1,9 @@
> +board_detect()
> +{
> +	[ -d /proc/device-tree ] || return
> +	mkdir -p /tmp/sysinfo
> +	[ -e /tmp/sysinfo/board_name ] || \
> +		echo "$(strings /proc/device-tree/compatible | head -1)" > /tmp/sysinfo/board_name
> +	[ ! -e /tmp/sysinfo/model -a -e /proc/device-tree/model ] && \
> +		echo "$(cat /proc/device-tree/model)" > /tmp/sysinfo/model
> +}
> diff --git a/package/base-files/files/lib/preinit/03_preinit_board_detect b/package/base-files/files/lib/preinit/03_preinit_board_detect
> new file mode 100644
> index 0000000..739ab02
> --- /dev/null
> +++ b/package/base-files/files/lib/preinit/03_preinit_board_detect
> @@ -0,0 +1,11 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2017 The Linux Foundation. All rights reserved.

Why LF? Shouldn't you use here your own copyright or none at all?

What's more, does it really make sense for you to copyright shell code 
which length is almost the same as the copyright line?

--
Cheers,
Piotr

> +#
> +
> +do_board_detect()
> +{
> +	. /lib/board_detect.sh && board_detect
> +}
> +
> +boot_hook_add preinit_main do_board_detect
> diff --git a/package/base-files/files/lib/preinit/10_sysinfo b/package/base-files/files/lib/preinit/10_sysinfo
> deleted file mode 100644
> index 65b5096..0000000
> --- a/package/base-files/files/lib/preinit/10_sysinfo
> +++ /dev/null
> @@ -1,10 +0,0 @@
> -do_sysinfo_generic() {
> -	[ -d /proc/device-tree ] || return
> -	mkdir -p /tmp/sysinfo
> -	[ -e /tmp/sysinfo/board_name ] || \
> -		echo "$(strings /proc/device-tree/compatible | head -1)" > /tmp/sysinfo/board_name
> -	[ ! -e /tmp/sysinfo/model -a -e /proc/device-tree/model ] && \
> -		echo "$(cat /proc/device-tree/model)" > /tmp/sysinfo/model
> -}
> -
> -boot_hook_add preinit_main do_sysinfo_generic
>
Roman Yeryomin May 9, 2017, 8:24 p.m. UTC | #2 
On 9 May 2017 at 12:51, Piotr Dymacz <pepe2k@gmail.com> wrote:
> Hello Roman,

Hi

> Your mail addresses in from header and in SoB line don't match.

is that a problem?

> Also, some questions inline, below.
>
>
> On 09.05.2017 11:16, Roman Yeryomin wrote:
>>
>> Signed-off-by: Roman Yeryomin <roman@advem.lv>
>> ---
>>  package/base-files/files/lib/board_detect.sh                 |  9
>> +++++++++
>>  package/base-files/files/lib/preinit/03_preinit_board_detect | 11
>> +++++++++++
>>  package/base-files/files/lib/preinit/10_sysinfo              | 10
>> ----------
>>  3 files changed, 20 insertions(+), 10 deletions(-)
>>  create mode 100644 package/base-files/files/lib/board_detect.sh
>>  create mode 100644
>> package/base-files/files/lib/preinit/03_preinit_board_detect
>>  delete mode 100644 package/base-files/files/lib/preinit/10_sysinfo
>>
>> diff --git a/package/base-files/files/lib/board_detect.sh
>> b/package/base-files/files/lib/board_detect.sh
>> new file mode 100644
>> index 0000000..e2f0f89
>> --- /dev/null
>> +++ b/package/base-files/files/lib/board_detect.sh
>> @@ -0,0 +1,9 @@
>> +board_detect()
>> +{
>> +       [ -d /proc/device-tree ] || return
>> +       mkdir -p /tmp/sysinfo
>> +       [ -e /tmp/sysinfo/board_name ] || \
>> +               echo "$(strings /proc/device-tree/compatible | head -1)" >
>> /tmp/sysinfo/board_name
>> +       [ ! -e /tmp/sysinfo/model -a -e /proc/device-tree/model ] && \
>> +               echo "$(cat /proc/device-tree/model)" > /tmp/sysinfo/model
>> +}
>> diff --git a/package/base-files/files/lib/preinit/03_preinit_board_detect
>> b/package/base-files/files/lib/preinit/03_preinit_board_detect
>> new file mode 100644
>> index 0000000..739ab02
>> --- /dev/null
>> +++ b/package/base-files/files/lib/preinit/03_preinit_board_detect
>> @@ -0,0 +1,11 @@
>> +#!/bin/sh
>> +#
>> +# Copyright (c) 2017 The Linux Foundation. All rights reserved.
>
>
> Why LF? Shouldn't you use here your own copyright or none at all?

Probably because that script is actually just a copy from ipq target.
Also I would ask the same question -- why LF? Maybe there is an answer
buried somewhere in the mail list...

> What's more, does it really make sense for you to copyright shell code which
> length is almost the same as the copyright line?

I don't care much about that but technically you are right.
And still, I would make it consistent. So, you can propose a patch
which removes all copyrights from scripts. That could even increase
performance by some us.

> --
> Cheers,
> Piotr
>
>
>> +#
>> +
>> +do_board_detect()
>> +{
>> +       . /lib/board_detect.sh && board_detect
>> +}
>> +
>> +boot_hook_add preinit_main do_board_detect
>> diff --git a/package/base-files/files/lib/preinit/10_sysinfo
>> b/package/base-files/files/lib/preinit/10_sysinfo
>> deleted file mode 100644
>> index 65b5096..0000000
>> --- a/package/base-files/files/lib/preinit/10_sysinfo
>> +++ /dev/null
>> @@ -1,10 +0,0 @@
>> -do_sysinfo_generic() {
>> -       [ -d /proc/device-tree ] || return
>> -       mkdir -p /tmp/sysinfo
>> -       [ -e /tmp/sysinfo/board_name ] || \
>> -               echo "$(strings /proc/device-tree/compatible | head -1)" >
>> /tmp/sysinfo/board_name
>> -       [ ! -e /tmp/sysinfo/model -a -e /proc/device-tree/model ] && \
>> -               echo "$(cat /proc/device-tree/model)" > /tmp/sysinfo/model
>> -}
>> -
>> -boot_hook_add preinit_main do_sysinfo_generic
>>
>
Piotr Dymacz May 10, 2017, 8:25 p.m. UTC | #3 
Hello Roman,

On 09.05.2017 22:24, Roman Yeryomin wrote:
> On 9 May 2017 at 12:51, Piotr Dymacz <pepe2k@gmail.com> wrote:
>> Hello Roman,
>
> Hi
>
>> Your mail addresses in from header and in SoB line don't match.
>
> is that a problem?

AFAIK, not a big one but in general, yes. Someone would need to fix that 
before merging.

[snip]

>>> diff --git a/package/base-files/files/lib/preinit/03_preinit_board_detect
>>> b/package/base-files/files/lib/preinit/03_preinit_board_detect
>>> new file mode 100644
>>> index 0000000..739ab02
>>> --- /dev/null
>>> +++ b/package/base-files/files/lib/preinit/03_preinit_board_detect
>>> @@ -0,0 +1,11 @@
>>> +#!/bin/sh
>>> +#
>>> +# Copyright (c) 2017 The Linux Foundation. All rights reserved.
>>
>>
>> Why LF? Shouldn't you use here your own copyright or none at all?
>
> Probably because that script is actually just a copy from ipq target.

As far as I can see, it's not a 1:1 copy.
Please, use your own copyrights or none at all.

> Also I would ask the same question -- why LF? Maybe there is an answer
> buried somewhere in the mail list...

It could be just a copy&paste from vendor SDK (QLSDK?).

>> What's more, does it really make sense for you to copyright shell code which
>> length is almost the same as the copyright line?
>
> I don't care much about that but technically you are right.
> And still, I would make it consistent. So, you can propose a patch
> which removes all copyrights from scripts. That could even increase
> performance by some us.

I really don't think that you can just drop all copyright lines from 
code if they are already there.

--
Cheers,
Piotr

>
>> --
>> Cheers,
>> Piotr
>>
>>
>>> +#
>>> +
>>> +do_board_detect()
>>> +{
>>> +       . /lib/board_detect.sh && board_detect
>>> +}
>>> +
>>> +boot_hook_add preinit_main do_board_detect
>>> diff --git a/package/base-files/files/lib/preinit/10_sysinfo
>>> b/package/base-files/files/lib/preinit/10_sysinfo
>>> deleted file mode 100644
>>> index 65b5096..0000000
>>> --- a/package/base-files/files/lib/preinit/10_sysinfo
>>> +++ /dev/null
>>> @@ -1,10 +0,0 @@
>>> -do_sysinfo_generic() {
>>> -       [ -d /proc/device-tree ] || return
>>> -       mkdir -p /tmp/sysinfo
>>> -       [ -e /tmp/sysinfo/board_name ] || \
>>> -               echo "$(strings /proc/device-tree/compatible | head -1)" >
>>> /tmp/sysinfo/board_name
>>> -       [ ! -e /tmp/sysinfo/model -a -e /proc/device-tree/model ] && \
>>> -               echo "$(cat /proc/device-tree/model)" > /tmp/sysinfo/model
>>> -}
>>> -
>>> -boot_hook_add preinit_main do_sysinfo_generic
>>>
>>
>
diff mbox

Patch

diff --git a/package/base-files/files/lib/board_detect.sh b/package/base-files/files/lib/board_detect.sh
new file mode 100644
index 0000000..e2f0f89
--- /dev/null
+++ b/package/base-files/files/lib/board_detect.sh
@@ -0,0 +1,9 @@ 
+board_detect()
+{
+	[ -d /proc/device-tree ] || return
+	mkdir -p /tmp/sysinfo
+	[ -e /tmp/sysinfo/board_name ] || \
+		echo "$(strings /proc/device-tree/compatible | head -1)" > /tmp/sysinfo/board_name
+	[ ! -e /tmp/sysinfo/model -a -e /proc/device-tree/model ] && \
+		echo "$(cat /proc/device-tree/model)" > /tmp/sysinfo/model
+}
diff --git a/package/base-files/files/lib/preinit/03_preinit_board_detect b/package/base-files/files/lib/preinit/03_preinit_board_detect
new file mode 100644
index 0000000..739ab02
--- /dev/null
+++ b/package/base-files/files/lib/preinit/03_preinit_board_detect
@@ -0,0 +1,11 @@ 
+#!/bin/sh
+#
+# Copyright (c) 2017 The Linux Foundation. All rights reserved.
+#
+
+do_board_detect()
+{
+	. /lib/board_detect.sh && board_detect
+}
+
+boot_hook_add preinit_main do_board_detect
diff --git a/package/base-files/files/lib/preinit/10_sysinfo b/package/base-files/files/lib/preinit/10_sysinfo
deleted file mode 100644
index 65b5096..0000000
--- a/package/base-files/files/lib/preinit/10_sysinfo
+++ /dev/null
@@ -1,10 +0,0 @@ 
-do_sysinfo_generic() {
-	[ -d /proc/device-tree ] || return
-	mkdir -p /tmp/sysinfo
-	[ -e /tmp/sysinfo/board_name ] || \
-		echo "$(strings /proc/device-tree/compatible | head -1)" > /tmp/sysinfo/board_name
-	[ ! -e /tmp/sysinfo/model -a -e /proc/device-tree/model ] && \
-		echo "$(cat /proc/device-tree/model)" > /tmp/sysinfo/model
-}
-
-boot_hook_add preinit_main do_sysinfo_generic