2014-01-23 10:39:52

by Ard Biesheuvel

[permalink] [raw]
Subject: [PATCH] firmware/google: drop 'select EFI' to avoid recursive dependency

The GOOGLE_SMI Kconfig symbol depends on DMI and selects EFI. This
causes problems on other archs when introducing DMI support that
depends on EFI, as it results in a recursive dependency:

arch/arm/Kconfig:1845:error: recursive dependency detected!
arch/arm/Kconfig:1845: symbol DMI depends on EFI

Fix by changing the 'select EFI' to a 'depends on EFI'.

Signed-off-by: Ard Biesheuvel <[email protected]>
---
drivers/firmware/google/Kconfig | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/firmware/google/Kconfig b/drivers/firmware/google/Kconfig
index 2f21b0bfe653..29c8cdda82a1 100644
--- a/drivers/firmware/google/Kconfig
+++ b/drivers/firmware/google/Kconfig
@@ -12,8 +12,7 @@ menu "Google Firmware Drivers"

config GOOGLE_SMI
tristate "SMI interface for Google platforms"
- depends on ACPI && DMI
- select EFI
+ depends on ACPI && DMI && EFI
select EFI_VARS
help
Say Y here if you want to enable SMI callbacks for Google
--
1.8.3.2


2014-01-23 21:37:04

by David Rientjes

[permalink] [raw]
Subject: Re: [PATCH] firmware/google: drop 'select EFI' to avoid recursive dependency

On Thu, 23 Jan 2014, Ard Biesheuvel wrote:

> The GOOGLE_SMI Kconfig symbol depends on DMI and selects EFI. This
> causes problems on other archs when introducing DMI support that
> depends on EFI, as it results in a recursive dependency:
>
> arch/arm/Kconfig:1845:error: recursive dependency detected!
> arch/arm/Kconfig:1845: symbol DMI depends on EFI
>
> Fix by changing the 'select EFI' to a 'depends on EFI'.
>
> Signed-off-by: Ard Biesheuvel <[email protected]>
> ---
> drivers/firmware/google/Kconfig | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/firmware/google/Kconfig b/drivers/firmware/google/Kconfig
> index 2f21b0bfe653..29c8cdda82a1 100644
> --- a/drivers/firmware/google/Kconfig
> +++ b/drivers/firmware/google/Kconfig
> @@ -12,8 +12,7 @@ menu "Google Firmware Drivers"
>
> config GOOGLE_SMI
> tristate "SMI interface for Google platforms"
> - depends on ACPI && DMI
> - select EFI
> + depends on ACPI && DMI && EFI
> select EFI_VARS
> help
> Say Y here if you want to enable SMI callbacks for Google

Adding Mike Waychison <[email protected]> to the cc since this is his code,
but it looks good to me.

$ ./scripts/get_maintainer.pl -f drivers/firmware/google/Kconfig
[email protected] (open list)

I wonder why he's not listed as a recipient for patches from
get_maintainer.pl since it's clearly obvious he wrote the entire file.

2014-01-23 21:40:58

by Mike Waychison

[permalink] [raw]
Subject: Re: [PATCH] firmware/google: drop 'select EFI' to avoid recursive dependency

On Thu, Jan 23, 2014 at 1:36 PM, David Rientjes <[email protected]> wrote:
> On Thu, 23 Jan 2014, Ard Biesheuvel wrote:
>
>> The GOOGLE_SMI Kconfig symbol depends on DMI and selects EFI. This
>> causes problems on other archs when introducing DMI support that
>> depends on EFI, as it results in a recursive dependency:
>>
>> arch/arm/Kconfig:1845:error: recursive dependency detected!
>> arch/arm/Kconfig:1845: symbol DMI depends on EFI
>>
>> Fix by changing the 'select EFI' to a 'depends on EFI'.
>>
>> Signed-off-by: Ard Biesheuvel <[email protected]>

Acked-by: Mike Waychison <[email protected]>

>> ---
>> drivers/firmware/google/Kconfig | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/firmware/google/Kconfig b/drivers/firmware/google/Kconfig
>> index 2f21b0bfe653..29c8cdda82a1 100644
>> --- a/drivers/firmware/google/Kconfig
>> +++ b/drivers/firmware/google/Kconfig
>> @@ -12,8 +12,7 @@ menu "Google Firmware Drivers"
>>
>> config GOOGLE_SMI
>> tristate "SMI interface for Google platforms"
>> - depends on ACPI && DMI
>> - select EFI
>> + depends on ACPI && DMI && EFI
>> select EFI_VARS
>> help
>> Say Y here if you want to enable SMI callbacks for Google
>
> Adding Mike Waychison <[email protected]> to the cc since this is his code,
> but it looks good to me.
>
> $ ./scripts/get_maintainer.pl -f drivers/firmware/google/Kconfig
> [email protected] (open list)
>
> I wonder why he's not listed as a recipient for patches from
> get_maintainer.pl since it's clearly obvious he wrote the entire file.

2014-01-23 21:48:48

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [PATCH] firmware/google: drop 'select EFI' to avoid recursive dependency

On 23 January 2014 22:40, Mike Waychison <[email protected]> wrote:
> On Thu, Jan 23, 2014 at 1:36 PM, David Rientjes <[email protected]> wrote:
>> On Thu, 23 Jan 2014, Ard Biesheuvel wrote:
>>
>>> The GOOGLE_SMI Kconfig symbol depends on DMI and selects EFI. This
>>> causes problems on other archs when introducing DMI support that
>>> depends on EFI, as it results in a recursive dependency:
>>>
>>> arch/arm/Kconfig:1845:error: recursive dependency detected!
>>> arch/arm/Kconfig:1845: symbol DMI depends on EFI
>>>
>>> Fix by changing the 'select EFI' to a 'depends on EFI'.
>>>
>>> Signed-off-by: Ard Biesheuvel <[email protected]>
>
> Acked-by: Mike Waychison <[email protected]>
>

Thanks.

Indeed, get_maintainer.pl did not produce a name, or I would have
cc'ed you directly.

Regards,
Ard.



>>> ---
>>> drivers/firmware/google/Kconfig | 3 +--
>>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/firmware/google/Kconfig b/drivers/firmware/google/Kconfig
>>> index 2f21b0bfe653..29c8cdda82a1 100644
>>> --- a/drivers/firmware/google/Kconfig
>>> +++ b/drivers/firmware/google/Kconfig
>>> @@ -12,8 +12,7 @@ menu "Google Firmware Drivers"
>>>
>>> config GOOGLE_SMI
>>> tristate "SMI interface for Google platforms"
>>> - depends on ACPI && DMI
>>> - select EFI
>>> + depends on ACPI && DMI && EFI
>>> select EFI_VARS
>>> help
>>> Say Y here if you want to enable SMI callbacks for Google
>>
>> Adding Mike Waychison <[email protected]> to the cc since this is his code,
>> but it looks good to me.
>>
>> $ ./scripts/get_maintainer.pl -f drivers/firmware/google/Kconfig
>> [email protected] (open list)
>>
>> I wonder why he's not listed as a recipient for patches from
>> get_maintainer.pl since it's clearly obvious he wrote the entire file.

2014-01-23 21:49:13

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] firmware/google: drop 'select EFI' to avoid recursive dependency

On Thu, 23 Jan 2014 13:36:55 -0800 (PST) David Rientjes <[email protected]> wrote:

> $ ./scripts/get_maintainer.pl -f drivers/firmware/google/Kconfig
> [email protected] (open list)
>
> I wonder why he's not listed as a recipient for patches from
> get_maintainer.pl since it's clearly obvious he wrote the entire file.

I do wish get_maintainer was better about this. You can apparently
make it dtrt with funky arguments, but --git-since and --git aren't
working for me.

get_maintainer's default output should answer the question "who do I
email about this file", and that ain't working :(

2014-01-23 22:46:09

by David Rientjes

[permalink] [raw]
Subject: Re: [PATCH] firmware/google: drop 'select EFI' to avoid recursive dependency

On Thu, 23 Jan 2014, Andrew Morton wrote:

> > $ ./scripts/get_maintainer.pl -f drivers/firmware/google/Kconfig
> > [email protected] (open list)
> >
> > I wonder why he's not listed as a recipient for patches from
> > get_maintainer.pl since it's clearly obvious he wrote the entire file.
>
> I do wish get_maintainer was better about this. You can apparently
> make it dtrt with funky arguments, but --git-since and --git aren't
> working for me.
>
> get_maintainer's default output should answer the question "who do I
> email about this file", and that ain't working :(
>

I also think it would be great if get_maintainer's output was friendlier
so that I can just copy a comma-separated line of "name <email>, name2
<email2>" into my email client.

2014-01-24 00:02:13

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] firmware/google: drop 'select EFI' to avoid recursive dependency

On Thu, 2014-01-23 at 14:45 -0800, David Rientjes wrote:
> On Thu, 23 Jan 2014, Andrew Morton wrote:
>
> > > $ ./scripts/get_maintainer.pl -f drivers/firmware/google/Kconfig
> > > [email protected] (open list)
> > >
> > > I wonder why he's not listed as a recipient for patches from
> > > get_maintainer.pl since it's clearly obvious he wrote the entire file.
> >
> > I do wish get_maintainer was better about this. You can apparently
> > make it dtrt with funky arguments, but --git-since and --git aren't
> > working for me.

Or use --interactive to figure out what's what when it
doesn't give you an answer you like.

> >
> > get_maintainer's default output should answer the question "who do I
> > email about this file", and that ain't working :(

Complaints cheerfully ignored.
Suggestions gratefully accepted.

Files that haven't had changes in a long time
generally aren't maintained.

Old addresses frequently become stale and bounce.

It'd be better if there was a MAINTAINERS entry
for drivers/firmware/google.

Far be it for me to volunteer someone to be a
maintainer though.

Something like:
---
MAINTAINERS | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 5c5d554..f4271d5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3802,6 +3802,11 @@ F: Documentation/isdn/README.gigaset
F: drivers/isdn/gigaset/
F: include/uapi/linux/gigaset_dev.h

+GOOGLE MEMORY CONSOLE
+M: Mike Waychison <[email protected]>
+S: Supported
+F: drivers/firmware/google/
+
GPIO SUBSYSTEM
M: Linus Walleij <[email protected]>
M: Alexandre Courbot <[email protected]>

> >
>
> I also think it would be great if get_maintainer's output was friendlier
> so that I can just copy a comma-separated line of "name <email>, name2
> <email2>" into my email client.

That's what it was, then people complained about not knowing
what the people did to the particular files being modified
and the default became --rolestats.

You can get that comma separated list by adding
--norolestats --nomultiline

2014-01-24 10:27:42

by David Rientjes

[permalink] [raw]
Subject: Re: [PATCH] firmware/google: drop 'select EFI' to avoid recursive dependency

On Thu, 23 Jan 2014, Joe Perches wrote:

> > > get_maintainer's default output should answer the question "who do I
> > > email about this file", and that ain't working :(
>
> Complaints cheerfully ignored.
> Suggestions gratefully accepted.
>
> Files that haven't had changes in a long time
> generally aren't maintained.
>
> Old addresses frequently become stale and bounce.
>
> It'd be better if there was a MAINTAINERS entry
> for drivers/firmware/google.
>

I think scripts/get_maintainer.pl is only really useful for emailing
patches so I think outputting at least somebody to cc on patches would be
a good idea. It doesn't necessarily need to be someone who maintains the
code and pushes it to Linus.

I'm not sure how much runtime is a factor for people of the script, but
falling back to git-blame behavior to at least get one or two cc's sounds
appropriate. If the email address is outdated, owell, we live and learn.

Otherwise it just seems like people would throw patches at a wall and see
which ones stick.

Now, if Andrew wants an all-encompasing MAINTAINERS entry that really
lists all the directories he maintains, that would be fine as well just as
long as we're ok with the size of MAINTAINERS doubling :)

2014-01-24 22:56:35

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] firmware/google: drop 'select EFI' to avoid recursive dependency

On Fri, 2014-01-24 at 02:27 -0800, David Rientjes wrote:
> On Thu, 23 Jan 2014, Joe Perches wrote:
>
> > > > get_maintainer's default output should answer the question "who do I
> > > > email about this file", and that ain't working :(
> >
> > Complaints cheerfully ignored.
> > Suggestions gratefully accepted.
> >
> > Files that haven't had changes in a long time
> > generally aren't maintained.
> >
> > Old addresses frequently become stale and bounce.
> >
> > It'd be better if there was a MAINTAINERS entry
> > for drivers/firmware/google.
> >
>
> I think scripts/get_maintainer.pl is only really useful for emailing
> patches so I think outputting at least somebody to cc on patches would be
> a good idea. It doesn't necessarily need to be someone who maintains the
> code and pushes it to Linus.

Very very few people listed in MAINTAINERS actual push to Linus.

> I'm not sure how much runtime is a factor for people of the script, but
> falling back to git-blame behavior to at least get one or two cc's sounds
> appropriate. If the email address is outdated, owell, we live and learn.

Maybe something like this would work.
It uses git-blame whenever no maintainers are found.
---
scripts/get_maintainer.pl | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
index 9c3986f..ef05ed6 100755
--- a/scripts/get_maintainer.pl
+++ b/scripts/get_maintainer.pl
@@ -483,6 +483,13 @@ my %deduplicate_address_hash = ();

my @maintainers = get_maintainers();

+if ($email_maintainer && !$interactive && !$email_git_blame &&
+ (!@maintainers || ($email_list && @maintainers == 1))) {
+ warn "$P: No maintainer found, trying harder, addresses may be stale...\n";
+ $email_git_blame = 1;
+ @maintainers = get_maintainers();
+}
+
if (@maintainers) {
@maintainers = merge_email(@maintainers);
output(@maintainers);


2014-01-24 23:17:53

by David Rientjes

[permalink] [raw]
Subject: Re: [PATCH] firmware/google: drop 'select EFI' to avoid recursive dependency

On Fri, 24 Jan 2014, Joe Perches wrote:

> diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
> index 9c3986f..ef05ed6 100755
> --- a/scripts/get_maintainer.pl
> +++ b/scripts/get_maintainer.pl
> @@ -483,6 +483,13 @@ my %deduplicate_address_hash = ();
>
> my @maintainers = get_maintainers();
>
> +if ($email_maintainer && !$interactive && !$email_git_blame &&
> + (!@maintainers || ($email_list && @maintainers == 1))) {
> + warn "$P: No maintainer found, trying harder, addresses may be stale...\n";
> + $email_git_blame = 1;
> + @maintainers = get_maintainers();
> +}
> +
> if (@maintainers) {
> @maintainers = merge_email(@maintainers);
> output(@maintainers);

Works well and has good advice on how emails may be stale, thanks Joe!