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

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

[Ard Biesheuvel]

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 <ard.biesheuvel@linaro.org>
---
 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

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

[David Rientjes]

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 <ard.biesheuvel@linaro.org>
> ---
>  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 <mikew@google.com> to the cc since this is his code, 
but it looks good to me.

$ ./scripts/get_maintainer.pl -f drivers/firmware/google/Kconfig
linux-kernel@vger.kernel.org (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.

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

[Mike Waychison]

On Thu, Jan 23, 2014 at 1:36 PM, David Rientjes <rientjes@google.com> 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 <ard.biesheuvel@linaro.org>

Acked-by: Mike Waychison <mikew@google.com>

>> ---
>>  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 <mikew@google.com> to the cc since this is his code,
> but it looks good to me.
>
> $ ./scripts/get_maintainer.pl -f drivers/firmware/google/Kconfig
> linux-kernel@vger.kernel.org (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.

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

[Ard Biesheuvel]

On 23 January 2014 22:40, Mike Waychison <mikew@google.com> wrote:
> On Thu, Jan 23, 2014 at 1:36 PM, David Rientjes <rientjes@google.com> 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 <ard.biesheuvel@linaro.org>
>
> Acked-by: Mike Waychison <mikew@google.com>
>

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 <mikew@google.com> to the cc since this is his code,
>> but it looks good to me.
>>
>> $ ./scripts/get_maintainer.pl -f drivers/firmware/google/Kconfig
>> linux-kernel@vger.kernel.org (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.

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

[Andrew Morton]

On Thu, 23 Jan 2014 13:36:55 -0800 (PST) David Rientjes <rientjes@google.com> wrote:

> $ ./scripts/get_maintainer.pl -f drivers/firmware/google/Kconfig
> linux-kernel@vger.kernel.org (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 :(

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

[David Rientjes]

On Thu, 23 Jan 2014, Andrew Morton wrote:

> > $ ./scripts/get_maintainer.pl -f drivers/firmware/google/Kconfig
> > linux-kernel@vger.kernel.org (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.

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

[Joe Perches]

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
> > > linux-kernel@vger.kernel.org (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 <mikew@google.com>
+S:     Supported
+F:     drivers/firmware/google/
+
 GPIO SUBSYSTEM
 M:	Linus Walleij <linus.walleij@linaro.org>
 M:	Alexandre Courbot <gnurou@gmail.com>

> > 
> 
> 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

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

[David Rientjes]

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 :)

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

[Joe Perches]

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);

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

[David Rientjes]

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!