by Luca Coelho

[permalink] [raw]

Subject: Re: After upgrading to 4.11.1, wifi driver refuses to load after being unloaded once.

On Wed, 2017-10-18 at 07:59 +0300, Kalle Valo wrote:
> Luca Coelho <[email protected]> writes:
>
> > On Tue, 2017-10-17 at 14:23 -0700, Marc MERLIN wrote:
> > > On Tue, Oct 17, 2017 at 05:05:57PM +0300, Luca Coelho wrote:
> > > > Hi,
> > > >
> > > > On Tue, 2017-10-17 at 02:44 -0700, Marc MERLIN wrote:
> > > > > Was broken in 4.11, still broken in 4.12. This is crippling,
> > > > > I'm
> > > > > not
> > > > > running linux so that I have to reboot it to reload an intel
> > > > > wireless
> > > > > driver :-/
> > > >
> > > > Can you report a bug in https://bugzilla.kernel.org so it's
> > > > easier
> > > > to
> > > > track this?
> > > >
> > > > The problem seems to be that the rootport is not leaving D3 for
> > > > some
> > > > reason when the driver is loaded again. Do you have
> > > > CONFIG_IWLWIFI_PCIE_RTPM enabled in your .config? (you
> > > > shouldn't)
> > >
> > >
> > > I don't know how or why, but I seem to:
> > > saruman:~# grep IWLWIFI /boot/config-4.12.10-amd64-preempt-sysrq-
> > > 20170406
> > > CONFIG_IWLWIFI=m
> > > CONFIG_IWLWIFI_LEDS=y
> > > CONFIG_IWLWIFI_OPMODE_MODULAR=y
> > > # CONFIG_IWLWIFI_BCAST_FILTERING is not set
> > > CONFIG_IWLWIFI_PCIE_RTPM=y
> > > CONFIG_IWLWIFI_DEBUG=y
> > > CONFIG_IWLWIFI_DEVICE_TRACING=y
> > >
> > > I'll remove that, thanks.
> >
> > Cool, I think that might help. If it doesn't, please report a bug
> > in
> > buzilla. ;)
>
> But a Kconfig option should never break functionality, so IMHO this
> still sounds like a bug in iwlwifi.

The problem is that to get this to work, some changes need to be made
in the platform side. In this case, the rootport is not configured
properly so it won't work.

We discussed this before and that's why this option now depends on
EXPERT.

--
Luca.

2017-10-20 06:35:41

by Kalle Valo

[permalink] [raw]

Subject: Re: After upgrading to 4.11.1, wifi driver refuses to load after being unloaded once.

Luca Coelho <[email protected]> writes:

> On Wed, 2017-10-18 at 12:50 +0300, Kalle Valo wrote:
>> Luca Coelho <[email protected]> writes:
>>
>> > On Wed, 2017-10-18 at 07:59 +0300, Kalle Valo wrote:
>> > > Luca Coelho <[email protected]> writes:
>> > >
>> > > > On Tue, 2017-10-17 at 14:23 -0700, Marc MERLIN wrote:
>> > > >
>> > > > > I don't know how or why, but I seem to:
>> > > > > saruman:~# grep IWLWIFI /boot/config-4.12.10-amd64-preempt-
>> > > > > sysrq-
>> > > > > 20170406
>> > > > > CONFIG_IWLWIFI=m
>> > > > > CONFIG_IWLWIFI_LEDS=y
>> > > > > CONFIG_IWLWIFI_OPMODE_MODULAR=y
>> > > > > # CONFIG_IWLWIFI_BCAST_FILTERING is not set
>> > > > > CONFIG_IWLWIFI_PCIE_RTPM=y
>> > > > > CONFIG_IWLWIFI_DEBUG=y
>> > > > > CONFIG_IWLWIFI_DEVICE_TRACING=y
>> > > > >
>> > > > > I'll remove that, thanks.
>> > > >
>> > > > Cool, I think that might help. If it doesn't, please report a
>> > > > bug
>> > > > in
>> > > > buzilla. ;)
>> > >
>> > > But a Kconfig option should never break functionality, so IMHO
>> > > this
>> > > still sounds like a bug in iwlwifi.
>> >
>> > The problem is that to get this to work, some changes need to be
>> > made
>> > in the platform side. In this case, the rootport is not configured
>> > properly so it won't work.
>>
>> Yeah, but users or distros might accidentally enable this Kconfig
>> option and break the driver unintentionally. And subtle bugs like
>> this
>> are even worse as the user will not realise that it's because of a
>> new
>> Kconfig option.
>>
>> So I guess you can't automatically detect it the platform supports
>> RTPM,
>> right? Maybe there should be a module parameter which has to be set
>> to
>> enable this? And at least a big fat warning to the user that RTPM is
>> enabled, bugs are likely and the user has to know what she's doing.
>
> I thought this was what EXPERT was used for:
>
> menuconfig EXPERT
> bool "Configure standard kernel features (expert users)"
> # Unhide debug options, to make the on-by-default options visible
> select DEBUG_KERNEL
> help
> This option allows certain base kernel options and settings
> to be disabled or tweaked. This is for specialized
> environments which can tolerate a "non-standard" kernel.
> Only use this if you really know what you are doing.
>
>
> But it seems that it's widely used even by people/distros who don't
> know what they are doing. :(

Yeah, people seem to just blindly enable kconfig options. I don't blame
them, there are so many options and difficult to know what to choose.

> Would it be okay if we just add a printk(KERN_ERR, ...)?

I think we at least we should do that, preferably even something else to
prevent accidental use if the feature is so fragile.

--
Kalle Valo