Re: [Regression] Changes to i2c-piix4.c initialisation prevent loading of sp5100_tco watchdog driver on AMD SB800 chipset

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Paul,

On 03/31/2017 12:17 AM, Paul Menzel wrote:
Dear Wolfram,


Thank you for the reply, which we talked about briefly at the
Chemnitzer LinuxTage.


Am Freitag, den 03.03.2017, 11:17 +0100 schrieb Wolfram Sang:
Unfortunately, commit 2fee61d22e (i2c: piix4: Add support for
multiplexed main adapter in SB800) [1] caused a regression. Tim
reported that to the Linux Kernel Bugtracker as bug #170741 last
September [2], but it looks like the affected subsystems don’t use it.

Jean Delvare pointed out this issue amongst others[1] last year already.
Let me quote:

===

5* The I/O ports used for SMBus configuration and port switching are
also needed by a watchdog driver, sp5100_tco. Both drivers request the
region, so the first one wins, and the other driver can't be loaded.
sp5100_tco was there first, so the changes done to the i2c-piix4 driver
recently will cause a regression for some users by preventing them
from using the sp5100_tco and i2c-piix4 drivers at the same time. In
the long run I guess we will need a helper module to handle this shared
resource. Unless IORESOURCE_MUXED can be used for that. Either way,
that's more work than I can put into this before kernel v4.5 is
released. For the time being, I think we should simply make it
non-fatal if the I/O ports can't be requested, and continue without
multiplexing (as before.)

===

Seems nobody had the resources, so far.

I still don’t understand, why Jean then not immediately reverted the
commit to adhere to the Linux Kernel’s no-regression-policy.

I don't have the HW and not much experience with non-embedded
platforms. I wonder, though, if we really need to convert the drivers
to MFD ones, or if we could use the simpler MFD_SYSCON mechanism
which helps in exactly such cases for embedded platforms. But I am
really lacking details here and am afraid this is probably all the
input I can give currently.

Zoltan stepped up, and uploaded a patch for review to the Kernel.org
Bugzilla [2], also attached to this message.


Please don't send patches as attachments.

request_muxed_region() can fail, and literally every other driver
using it checks for that failure. Please do the same.

The sp5100_tco_dev_name change in the watchdog driver is unnecessary.
There are some unnecessary { } in the watchdog driver after the patch
is applied.

Please split the patch into two patches so they can be reviewed and
applied separately.

Thanks,
Guenter

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



[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux