diff mbox

Input: synaptics-rmi4 - make F03 a tristate symbol

Message ID 3051252.9A92ba0o10@wuerfel (mailing list archive)
State New, archived
Headers show

Commit Message

Arnd Bergmann Jan. 13, 2017, 9:06 p.m. UTC
On Thursday, January 12, 2017 10:22:03 PM CET Dmitry Torokhov wrote:
> As it was explained townthread we can't [currently] make functions
> modules, in the meantime I have d7ddad0acc4add42567f7879b116a0b9eea31860
> that should fix this issue (and I just sent pull request for it).

On today's linux-next (which includes d7ddad0acc4ad), I was still
getting this warning :

warning: (HID_RMI) selects RMI4_F03 which has unmet direct dependencies (!UML && INPUT && RMI4_CORE && (SERIO=y || RMI4_CORE=SERIO))

This is my fixup, though I'm not too happy with that version.
    
Signed-off-by: Arnd Bergmann <arnd@arndb.de>


--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Dmitry Torokhov Jan. 13, 2017, 9:15 p.m. UTC | #1
On January 13, 2017 1:06:12 PM PST, Arnd Bergmann <arnd@arndb.de> wrote:
>On Thursday, January 12, 2017 10:22:03 PM CET Dmitry Torokhov wrote:
>> As it was explained townthread we can't [currently] make functions
>> modules, in the meantime I have
>d7ddad0acc4add42567f7879b116a0b9eea31860
>> that should fix this issue (and I just sent pull request for it).
>
>On today's linux-next (which includes d7ddad0acc4ad), I was still
>getting this warning :
>
>warning: (HID_RMI) selects RMI4_F03 which has unmet direct dependencies
>(!UML && INPUT && RMI4_CORE && (SERIO=y || RMI4_CORE=SERIO))

Ah, yes, that's new hid RMI code..


>
>This is my fixup, though I'm not too happy with that version.
>    
>Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
>diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
>index 1aeb80e52424..3927259a5d5d 100644
>--- a/drivers/hid/Kconfig
>+++ b/drivers/hid/Kconfig
>@@ -785,7 +785,8 @@ config HID_SUNPLUS
> config HID_RMI
> 	tristate "Synaptics RMI4 device support"
> 	depends on HID
>-	select RMI4_CORE
>+	depends on SERIO && RMI4_CORE
>+	depends on SERIO=y || RMI4_CORE=SERIO

Shouldn't this be simply

select SERIO # needed for F03

?

> 	select RMI4_F03
> 	select RMI4_F11
> 	select RMI4_F12


Thanks.
Arnd Bergmann Jan. 13, 2017, 9:34 p.m. UTC | #2
On Fri, Jan 13, 2017 at 10:15 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
>>This is my fixup, though I'm not too happy with that version.
>>
>>Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>>
>>diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
>>index 1aeb80e52424..3927259a5d5d 100644
>>--- a/drivers/hid/Kconfig
>>+++ b/drivers/hid/Kconfig
>>@@ -785,7 +785,8 @@ config HID_SUNPLUS
>> config HID_RMI
>>       tristate "Synaptics RMI4 device support"
>>       depends on HID
>>-      select RMI4_CORE
>>+      depends on SERIO && RMI4_CORE
>>+      depends on SERIO=y || RMI4_CORE=SERIO
>
> Shouldn't this be simply
>
> select SERIO # needed for F03

Ah, I guess this would work too. I didn't consider it because SERIO is
a user-visible symbol and we generally try not to 'select' them but instead
use 'depends on.

However, SERIO is already used with 'select' all over the place, so adding
another select is actually safer than adding a dependency (which could
cause dependency loops here).

Actually the best solution is probably to have 'select SERIO' in RMI4, like

config RMI4_F03_SERIO
       tristate
       depends on RMI4_CORE
       depends on RMI4_F03
       default RMI4_CORE
       select SERIO

As that avoids the 'depends on SERIO=y || RMI4_CORE=SERIO' statement that
is different from the other SERIO users, it keeps it all in one place,
and it doesn't
prevent you from seeing the RMI4_F03 symbol when SERIO=m.

       Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dmitry Torokhov Jan. 13, 2017, 9:42 p.m. UTC | #3
On Fri, Jan 13, 2017 at 10:34:32PM +0100, Arnd Bergmann wrote:
> On Fri, Jan 13, 2017 at 10:15 PM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> >
> >>This is my fixup, though I'm not too happy with that version.
> >>
> >>Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> >>
> >>diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> >>index 1aeb80e52424..3927259a5d5d 100644
> >>--- a/drivers/hid/Kconfig
> >>+++ b/drivers/hid/Kconfig
> >>@@ -785,7 +785,8 @@ config HID_SUNPLUS
> >> config HID_RMI
> >>       tristate "Synaptics RMI4 device support"
> >>       depends on HID
> >>-      select RMI4_CORE
> >>+      depends on SERIO && RMI4_CORE
> >>+      depends on SERIO=y || RMI4_CORE=SERIO
> >
> > Shouldn't this be simply
> >
> > select SERIO # needed for F03
> 
> Ah, I guess this would work too. I didn't consider it because SERIO is
> a user-visible symbol and we generally try not to 'select' them but instead
> use 'depends on.
> 
> However, SERIO is already used with 'select' all over the place, so adding
> another select is actually safer than adding a dependency (which could
> cause dependency loops here).
> 
> Actually the best solution is probably to have 'select SERIO' in RMI4, like
> 
> config RMI4_F03_SERIO
>        tristate
>        depends on RMI4_CORE
>        depends on RMI4_F03
>        default RMI4_CORE
>        select SERIO
> 
> As that avoids the 'depends on SERIO=y || RMI4_CORE=SERIO' statement that
> is different from the other SERIO users, it keeps it all in one place,
> and it doesn't
> prevent you from seeing the RMI4_F03 symbol when SERIO=m.

Hmm, if this works and resilient with user changing symbols after
they've been auto-selected then I like it. How can we run it through
multitude of randconfigs?

Thanks.
Arnd Bergmann Jan. 14, 2017, 12:09 p.m. UTC | #4
On Jan 13, 2017 10:42 PM, "Dmitry Torokhov" <dmitry.torokhov@gmail.com> wrote:
> On Fri, Jan 13, 2017 at 10:34:32PM +0100, Arnd Bergmann wrote:
> > config RMI4_F03_SERIO
> >        tristate
> >        depends on RMI4_CORE
> >        depends on RMI4_F03
> >        default RMI4_CORE
> >        select SERIO
> >
> > As that avoids the 'depends on SERIO=y || RMI4_CORE=SERIO' statement that
> > is different from the other SERIO users, it keeps it all in one place,
> > and it doesn't
> > prevent you from seeing the RMI4_F03 symbol when SERIO=m.
> Hmm, if this works and resilient with user changing symbols after
> they've been auto-selected then I like it. How can we run it through
> multitude of randconfigs?

I've successfully run it over night on a few hundred randconfig builds without
problems now, so I'm pretty confident it works.

The hidden option will ensure the configuration is always valid even when
the user changes it, the only thing that can be unexpected is the same as
every 'select': when you enable this option, SERIO will get turned on, and
when you disable it again after leaving 'menuconfig', it stays on.

    Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dmitry Torokhov Jan. 15, 2017, 11:39 p.m. UTC | #5
On Sat, Jan 14, 2017 at 01:09:57PM +0100, Arnd Bergmann wrote:
> On Jan 13, 2017 10:42 PM, "Dmitry Torokhov" <dmitry.torokhov@gmail.com> wrote:
> > On Fri, Jan 13, 2017 at 10:34:32PM +0100, Arnd Bergmann wrote:
> > > config RMI4_F03_SERIO
> > >        tristate
> > >        depends on RMI4_CORE
> > >        depends on RMI4_F03
> > >        default RMI4_CORE
> > >        select SERIO
> > >
> > > As that avoids the 'depends on SERIO=y || RMI4_CORE=SERIO' statement that
> > > is different from the other SERIO users, it keeps it all in one place,
> > > and it doesn't
> > > prevent you from seeing the RMI4_F03 symbol when SERIO=m.
> > Hmm, if this works and resilient with user changing symbols after
> > they've been auto-selected then I like it. How can we run it through
> > multitude of randconfigs?
> 
> I've successfully run it over night on a few hundred randconfig builds without
> problems now, so I'm pretty confident it works.
> 
> The hidden option will ensure the configuration is always valid even when
> the user changes it, the only thing that can be unexpected is the same as
> every 'select': when you enable this option, SERIO will get turned on, and
> when you disable it again after leaving 'menuconfig', it stays on.

Great. Could you please send me real patch and I'll get it to Linus?

Thanks.
diff mbox

Patch

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 1aeb80e52424..3927259a5d5d 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -785,7 +785,8 @@  config HID_SUNPLUS
 config HID_RMI
 	tristate "Synaptics RMI4 device support"
 	depends on HID
-	select RMI4_CORE
+	depends on SERIO && RMI4_CORE
+	depends on SERIO=y || RMI4_CORE=SERIO
 	select RMI4_F03
 	select RMI4_F11
 	select RMI4_F12