Bug 221227 - Raspberry Pi 2: kernel panic introduced by r321712 (i2c RTC drivers added to GENERIC)
Summary: Raspberry Pi 2: kernel panic introduced by r321712 (i2c RTC drivers added to ...
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: arm (show other bugs)
Version: CURRENT
Hardware: arm Any
: --- Affects Only Me
Assignee: Ian Lepore
URL:
Keywords: regression
Depends on:
Blocks:
 
Reported: 2017-08-04 18:41 UTC by Sylvain Garrigues
Modified: 2017-09-11 22:21 UTC (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Sylvain Garrigues 2017-08-04 18:41:33 UTC
I build my kernel every week:
* r321484 boots ;
* r321899 panics (sorry, no log yet). It panics before any console output.
Comment 1 Sylvain Garrigues 2017-08-04 18:47:48 UTC
I also weekly update my amd64 build machine to same commits and noticed clang 5 was used to build the failing armv6 kernel, while clang 4 was used for the working armv6 one.

Can it be the cause of this bug? Is clang 5 robust enough to cross-compile for arm?
Comment 2 Dimitry Andric freebsd_committer freebsd_triage 2017-08-04 19:11:10 UTC
I haven't heard any complaints yet about clang 5.0.0 from arm users, you are the first one.  Without any information about the crash itself, it is impossible to say what is going on, though.

You could try building the r321899 kernel with clang 4.0.0, and see what happens then.  This seems to be the easiest way of figuring out whether it is due to the clang upgrade.
Comment 3 Sylvain Garrigues 2017-08-05 07:41:51 UTC
@dim, I'm sorry, I compiled r321484 with clang5 and it boots, so it's not a compiler-related issue, it must be a regression introduced in some other commits.

I don't have any serial cable yet to display the early console output. I know the regression happens in the early arm initialization because I have not output on the vt framebuffer output shown on my TV.

The raspberry pi just keeps rebooting with r321899
Comment 4 Sylvain Garrigues 2017-08-06 11:45:20 UTC
Just identified r321712 as the culprit, therefore adding Ian.
Comment 5 Ian Lepore freebsd_committer freebsd_triage 2017-08-07 14:55:30 UTC
(In reply to Sylvain Garrigues from comment #4)
There is no way to debug this problem with the (lack of) information provided.  About all I can say at this point is "My rpi-b does not panic".
Comment 6 Ian Lepore freebsd_committer freebsd_triage 2017-08-09 00:19:07 UTC
I committed a workaround in r322282, in which I simply removed 2 misbehaving rtc drivers from the generic config.  The real fix will be to update the drivers to behave properly; I've ordered samples of both types of chip and will fix the drivers when they arrive and I can do some testing.
Comment 7 commit-hook freebsd_committer freebsd_triage 2017-08-14 00:12:33 UTC
A commit references this bug:

Author: ian
Date: Mon Aug 14 00:12:15 UTC 2017
New revision: 322478
URL: https://svnweb.freebsd.org/changeset/base/322478

Log:
  Add back the drivers for Dallas/Maxim ds13xx and Seiko S35390x now that
  they've been rewritten/fixed to not cause panics by doing i2c transfers
  before interrupts are available.

  PR:		221227

Changes:
  head/sys/arm/conf/GENERIC
Comment 8 Ian Lepore freebsd_committer freebsd_triage 2017-08-14 00:16:08 UTC
I believe this problem should be fixed now (but I can't confirm the panic is gone since it never happened on my systems).  Please re-open if the problem still occurs on other types of systems.
Comment 9 commit-hook freebsd_committer freebsd_triage 2017-09-11 22:21:57 UTC
A commit references this bug:

Author: ian
Date: Mon Sep 11 22:21:16 UTC 2017
New revision: 323467
URL: https://svnweb.freebsd.org/changeset/base/323467

Log:
  MFC r321708-r321712, r321721, r321726-r321727, r321746, r321751,
      r321791-r321792, r321795, r321798, r321821, r321823, r321826,
      r321828, r321841, r321934, r322025-r322026, r322282, r322431,
      r322473, r322475-r322479

  Lots of i2c RTC driver stuff...

  r321708:
  Replace the pcf8563 i2c RTC driver with a new nxprtc driver which handles
  all the chips in the NXP PCA212x and PCA/PCF85xx series.  In addition to
  supporting more chips, this driver uses the countdown timer on the chips as
  a fractional seconds counter, giving it a resolution of about 15 milliseconds.

  r321709:
  Fix building this driver on non-FDT platforms.

  r321710:
  Add a few missing i2c devices that build fine on all arches.

  r321711:
  Move the device descriptions onto the device lines, so they cut and paste
  nicely into other config files.

  r321712:
  Add the i2c RTC drivers found on various arm systems.

  r321721:
  Switch from using iic_transfer() to iicdev_readfrom/writeto(), mostly so
  that transfers will be done with proper ownership of the bus. No
  behavioral changes.

  r321726:
  Bugfixes and enhancements...

  Don't enable the oscillator when it is found to be stopped at init time,
  just let the first setting of valid time start it.  But still report a dead
  battery if it's stopped at init time.

  Don't force the chip into 24hr mode, just cope with whatever mode it is
  already in.

  Align the RTC clock to top of second when setting it.

  r321727:
  Fix AM/PM mode handling.  The bits to mask off in the hours register changes
  between 12/24 hour mode.  Also fix conversion between 12 and 24 hour mode.
  It's not as easy as adding/subtracting 12, because the clock doesn't roll
  over 11->0, it rolls over 12->1; 0 isn't a valid hour in AM/PM mode.

  r321746:
  Use the new clock_schedule() to arrange for clock_settime() to be called
  at the right time to keep the RTC hardware time in sync, instead of using
  pause_sbt() to sleep until the right time.

  r321751:
  Remove now-unused variable.

  r321791:
  Switch from using iic_transfer() to iicdev_readfrom/writeto(), mostly so
  that transfers will be done with proper ownership of the bus. No
  behavioral changes.  Also add a detach() method.

  r321792:
  Add a detach() method.

  r321795:
  Check the clock-halted flag every time the clock is read, not just once
  at startup.  The flag stays set until the clock is loaded with good time,
  so we need to keep saying the time is invalid until that happens.

  r321798:
  Restore a few rather important lines of code that got fumbled in r321746.

  r321821:
  No need to call getnanotime() now that the waiting is done by the central
  subr_rtc code, switch from CLOCKF_SETTIME_NO_TS to CLOCKF_SETTIME_NO_ADJ
  so that we get fed a timestamp, but it's not adjusted to compensate for
  inaccuracy in setting time.

  r321823:
  Bugfixes and enhancements...

  Don't enable the oscillator when it is found to be stopped at init time,
  just let the first setting of valid time start it.  But still report a dead
  battery if it's stopped at init time.

  Don't force the chip into 24hr mode, just cope with whatever mode it is
  already in.

  Schedule the clock_settime() callbacks to align the RTC clock to top of
  second when setting it.

  r321826:
  Restructure the SUBDIR list as 1-per-line and alphabetize, so it will be
  easier to add new things (and see what changed) in the future.

  r321828:
  Build iicbus/{ds1307,ds3231,nxprtc} as modules.

  r321841:
  Add a driver for the Intersil ISL12xx family of i2c RTC chips.

  Supports ISL1209, ISL1218, ISL1219, ISL1220, ISL1221 (just basic RTC
  functionality, not all the other fancy stuff the chips can do).

  r321934:
  Add missing ofw_bus_if.h src file.

  r322025:
  Switch to iicdev_readfrom/writeto() to do xfers with proper bus ownership.

  Tested by:	manu@

  r322026:
  Add missing header file to SRCS.

  Reported by:	manu@

  r322282:
  Remove the ds133x and s35390a i2c RTC drivers for now.  They both do i2c
  transfers in their probe() or attach() routines, and that doesn't work
  when the low-level controller requires interrupts to be functional.

  The DS133x family of chips is nearly identical to the DS1307 and support
  for them should be added to that driver, then the ds133x driver can be
  deleted.  The s35390a driver just needs a non-trivial workover.  In both
  cases that work will be done and committed separately.

  r322431:
  Bid for the device with BUS_PROBE_GENERIC, because this is very much a
  generic driver with minimal feature support for a large number of chips.
  More featureful per-chip drivers might exist (especially out-of-tree) and
  those should win the bidding even if they use BUS_PROBE_DEFAULT.

  r322473:
  Add a new driver, ds13rtc, that handles all DS13xx series i2c RTC chips.

  This driver supports only basic timekeeping functionality.  It completely
  replaces the ds133x driver.  It can also replace the ds1374 driver, but that
  will take a few other changes in MIPS code and config, and will be committed
  separately.  It does NOT replace the existing ds1307 driver, which provides
  access to some of the extended features on the 1307 chip, such as controlling
  the square wave output signal.  If both ds1307 and ds13rtc drivers are
  present, the ds1307 driver will outbid and win control of the device.

  This driver can be configured with FDT data, or by using hints on non-FDT
  systems.  In addition to the standard hints for i2c devices, it requires
  a "chiptype" string of the form "dallas,ds13xx" where 'xx' is the chip id
  (i.e., the same format as FDT compat strings).

  r322475:
  Change "chiptype" to "compatible".  Making the hint name the same as the FDT
  property name should make it easier to document the list of names accepted
  by both configuration mechanisms.

  r322476:
  Remove the old ds1374 driver and use the ds13rtc driver instead.  Adjust
  several mips config files accordingly.

  r322477:
  Minor fixes and enhancements for the s35390a i2c RTC driver...

  - Add FDT probe code.
  - Do i2c transfers with exclusive bus ownership.
  - Use config_intrhook_oneshot() to defer chip setup because some i2c
    busses can't do transfers without interrupts.
  - Add a detach() routine.
  - Add to module build.

  r322478:
  Add back the drivers for Dallas/Maxim ds13xx and Seiko S35390x now that
  they've been rewritten/fixed to not cause panics by doing i2c transfers
  before interrupts are available.

  PR:		221227

  r322479:
  Add hinted attachment for non-FDT systems.  Also, print a message if
  setting up the timer fails, because on some types of chips that's the
  first attempt to access the device.  If the chip is missing/non-responsive
  then you'd get a driver that attached and didn't register the rtc, with
  no clue about why.  On other chip types there are inits that come before
  timer setup, and they already print messages about errors.

Changes:
_U  stable/11/
  stable/11/sys/arm/allwinner/axp209.c
  stable/11/sys/conf/NOTES
  stable/11/sys/conf/files
  stable/11/sys/dev/iicbus/ds1307.c
  stable/11/sys/dev/iicbus/ds1307reg.h
  stable/11/sys/dev/iicbus/ds133x.c
  stable/11/sys/dev/iicbus/ds1374.c
  stable/11/sys/dev/iicbus/ds13rtc.c
  stable/11/sys/dev/iicbus/ds3231.c
  stable/11/sys/dev/iicbus/ds3231reg.h
  stable/11/sys/dev/iicbus/isl12xx.c
  stable/11/sys/dev/iicbus/nxprtc.c
  stable/11/sys/dev/iicbus/pcf8563.c
  stable/11/sys/dev/iicbus/pcf8563reg.h
  stable/11/sys/dev/iicbus/s35390a.c
  stable/11/sys/mips/conf/XLP.hints
  stable/11/sys/mips/conf/XLR
  stable/11/sys/mips/conf/XLR64
  stable/11/sys/mips/conf/XLRN32
  stable/11/sys/mips/conf/std.XLP
  stable/11/sys/mips/rmi/xlr_i2c.c
  stable/11/sys/modules/i2c/Makefile
  stable/11/sys/modules/i2c/ds1307/
  stable/11/sys/modules/i2c/ds1307/Makefile
  stable/11/sys/modules/i2c/ds13rtc/
  stable/11/sys/modules/i2c/ds3231/
  stable/11/sys/modules/i2c/ds3231/Makefile
  stable/11/sys/modules/i2c/isl12xx/
  stable/11/sys/modules/i2c/isl12xx/Makefile
  stable/11/sys/modules/i2c/nxprtc/
  stable/11/sys/modules/i2c/nxprtc/Makefile
  stable/11/sys/modules/i2c/s35390a/