[Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons

[Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons

[John Stultz]

Hey Jon,
  So after rebasing my nexus7 patch stack onto pre-4.8-rc1 tree, I
noticed the power/volume buttons stopped working.

I did a manual rebased bisection and chased it down to your commit
1e2a7d78499e ("irqdomain: Don't set type when mapping an IRQ").

Reverting that patch makes things work again, so I wanted to see if
there was any debugging info I could provide to try to help narrow
down the problem here. (Sorry, I'd tinker myself with it some and try
to debug the issue, but after burning my friday night on this, I'm
eager to get away from the keyboard for the weekend).

thanks
-john

Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons

[Bjorn Andersson]

On Fri 29 Jul 21:39 PDT 2016, John Stultz wrote:

> Hey Jon,
>   So after rebasing my nexus7 patch stack onto pre-4.8-rc1 tree, I
> noticed the power/volume buttons stopped working.
> 

+Linus, as that's gpio-keys on top of two fairly standard gpio/pinctrl
drivers (8064 TLMM and SSBI).

Regards,
Bjorn

> I did a manual rebased bisection and chased it down to your commit
> 1e2a7d78499e ("irqdomain: Don't set type when mapping an IRQ").
> 
> Reverting that patch makes things work again, so I wanted to see if
> there was any debugging info I could provide to try to help narrow
> down the problem here. (Sorry, I'd tinker myself with it some and try
> to debug the issue, but after burning my friday night on this, I'm
> eager to get away from the keyboard for the weekend).
> 
> thanks
> -john

Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons

[Marc Zyngier]

On Fri, 29 Jul 2016 21:52:01 -0700
Bjorn Andersson <bjorn.andersson@linaro.org> wrote:

> On Fri 29 Jul 21:39 PDT 2016, John Stultz wrote:
> 
> > Hey Jon,
> >   So after rebasing my nexus7 patch stack onto pre-4.8-rc1 tree, I
> > noticed the power/volume buttons stopped working.
> >   
> 
> +Linus, as that's gpio-keys on top of two fairly standard gpio/pinctrl
> drivers (8064 TLMM and SSBI).
> 
> Regards,
> Bjorn
> 
> > I did a manual rebased bisection and chased it down to your commit
> > 1e2a7d78499e ("irqdomain: Don't set type when mapping an IRQ").
> > 
> > Reverting that patch makes things work again, so I wanted to see if
> > there was any debugging info I could provide to try to help narrow
> > down the problem here. (Sorry, I'd tinker myself with it some and try
> > to debug the issue, but after burning my friday night on this, I'm
> > eager to get away from the keyboard for the weekend).

It feels either like a case of trigger information not being provided
through DT *and* not passed as arguments to request_irq (and friends),
or like a conflicting trigger.

Thanks,

	M.
-- 
Without deviation from the norm, progress is not possible.

Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons

[Thomas Gleixner]

On Fri, 29 Jul 2016, John Stultz wrote:
> Hey Jon,
>   So after rebasing my nexus7 patch stack onto pre-4.8-rc1 tree, I
> noticed the power/volume buttons stopped working.
> 
> I did a manual rebased bisection and chased it down to your commit
> 1e2a7d78499e ("irqdomain: Don't set type when mapping an IRQ").
> 
> Reverting that patch makes things work again, so I wanted to see if
> there was any debugging info I could provide to try to help narrow
> down the problem here. (Sorry, I'd tinker myself with it some and try
> to debug the issue, but after burning my friday night on this, I'm
> eager to get away from the keyboard for the weekend).

dmesg should contain debug output which yells about non matching types. Can
you provide that?

Thanks,

	tglx

Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons

[John Stultz]

[-- Attachment #1: Type: text/plain, Size: 1011 bytes --]

On Sat, Jul 30, 2016 at 1:07 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Fri, 29 Jul 2016, John Stultz wrote:
>> Hey Jon,
>>   So after rebasing my nexus7 patch stack onto pre-4.8-rc1 tree, I
>> noticed the power/volume buttons stopped working.
>>
>> I did a manual rebased bisection and chased it down to your commit
>> 1e2a7d78499e ("irqdomain: Don't set type when mapping an IRQ").
>>
>> Reverting that patch makes things work again, so I wanted to see if
>> there was any debugging info I could provide to try to help narrow
>> down the problem here. (Sorry, I'd tinker myself with it some and try
>> to debug the issue, but after burning my friday night on this, I'm
>> eager to get away from the keyboard for the weekend).
>
> dmesg should contain debug output which yells about non matching types. Can
> you provide that?

I'm not seeing anything about problematic type matching in the dmesg.

I've also added LinusW's patches but that didn't seem to help either.

dmesg attached.

thanks
-john

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: flo-noirq.dmesg --]
[-- Type: text/plain; charset=utf-8, Size: 23569 bytes --]

[    0.000000] Booting Linux on physical CPU 0x0
[    0.000000] Linux version 4.7.0-11903-g0ab3b2f (jstultz@buildbox) (gcc version 4.7.3 (Ubuntu/Linaro 4.7.3-12ubuntu1) ) #1413 SMP PREEMPT Fri Aug 5 11:04:12 PDT 2016
[    0.000000] CPU: ARMv7 Processor [511f06f0] revision 0 (ARMv7), cr=10c5787d
[    0.000000] CPU: div instructions available: patching division code
[    0.000000] CPU: PIPT / VIPT nonaliasing data cache, PIPT instruction cache
[    0.000000] OF: fdt:Machine model: Asus Nexus7(flo)
[    0.000000] Memory policy: Data cache writealloc
[    0.000000] On node 0 totalpages: 512255
[    0.000000] free_area_init_node: node 0, pgdat c0d75d80, node_mem_map e93fe000
[    0.000000]   Normal zone: 1348 pages used for memmap
[    0.000000]   Normal zone: 0 pages reserved
[    0.000000]   Normal zone: 161024 pages, LIFO batch:31
[    0.000000]   HighMem zone: 351231 pages, LIFO batch:31
[    0.000000] percpu: Embedded 13 pages/cpu @e93a6000 s30464 r0 d22784 u53248
[    0.000000] pcpu-alloc: s30464 r0 d22784 u53248 alloc=13*4096
[    0.000000] pcpu-alloc: [0] 0 [0] 1 [0] 2 [0] 3 
[    0.000000] Built 1 zonelists in Zone order, mobility grouping on.  Total pages: 510907
[    0.000000] Kernel command line: androidboot.hardware=flo user_debug=31 msm_rtb.filter=0x3F ehci-hcd.park=3 console=ttyMSM0,115200,n8 androidboot.selinux=permissive vmalloc=340MÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿPAEH€Ú ‰|\x01 androidboot.emmc=true androidboot.serialno=0ac0e39c bootreason=Reboot fuse_info=Y ddr_vendor=hynix androidboot.baseband=apq asustek.hw_rev=rev_e androidboot.bootloader=FLO-04.05
[    0.000000] PID hash table entries: 4096 (order: 2, 16384 bytes)
[    0.000000] Dentry cache hash table entries: 131072 (order: 7, 524288 bytes)
[    0.000000] Inode-cache hash table entries: 65536 (order: 6, 262144 bytes)
[    0.000000] Memory: 2019688K/2049020K available (7168K kernel code, 546K rwdata, 1800K rodata, 1024K init, 356K bss, 29332K reserved, 0K cma-reserved, 1404924K highmem)
[    0.000000] Virtual kernel memory layout:
[    0.000000]     vector  : 0xffff0000 - 0xffff1000   (   4 kB)
[    0.000000]     fixmap  : 0xffc00000 - 0xfff00000   (3072 kB)
[    0.000000]     vmalloc : 0xea800000 - 0xff800000   ( 336 MB)
[    0.000000]     lowmem  : 0xc0000000 - 0xea400000   ( 676 MB)
[    0.000000]     pkmap   : 0xbfe00000 - 0xc0000000   (   2 MB)
[    0.000000]       .text : 0xc0208000 - 0xc0a00000   (8160 kB)
[    0.000000]       .init : 0xc0c00000 - 0xc0d00000   (1024 kB)
[    0.000000]       .data : 0xc0d00000 - 0xc0d88828   ( 547 kB)
[    0.000000]        .bss : 0xc0d8a000 - 0xc0de32b4   ( 357 kB)
[    0.000000] SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=4, Nodes=1
[    0.000000] Preemptible hierarchical RCU implementation.
[    0.000000] 	Build-time adjustment of leaf fanout to 32.
[    0.000000] NR_IRQS:16 nr_irqs:16 16
[    0.000000] clocksource: dg_timer: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 283149695806 ns
[    0.000008] sched_clock: 32 bits at 6MHz, resolution 148ns, wraps every 318145725365ns
[    0.000026] Switching to timer-based delay loop, resolution 148ns
[    0.000459] Console: colour dummy device 80x30
[    0.000496] Calibrating delay loop (skipped), value calculated using timer frequency.. 13.50 BogoMIPS (lpj=67500)
[    0.000523] pid_max: default: 32768 minimum: 301
[    0.000613] Security Framework initialized
[    0.000636] SELinux:  Initializing.
[    0.000712] SELinux:  Starting in permissive mode
[    0.000769] Mount-cache hash table entries: 2048 (order: 1, 8192 bytes)
[    0.000785] Mountpoint-cache hash table entries: 2048 (order: 1, 8192 bytes)
[    0.001767] CPU: Testing write buffer coherency: ok
[    0.002227] CPU0: thread -1, cpu 0, socket 0, mpidr 80000000
[    0.002288] Setting up static identity map for 0x80300000 - 0x80300058
[    0.123347] CPU1: thread -1, cpu 1, socket 0, mpidr 80000001
[    0.163780] CPU2: thread -1, cpu 2, socket 0, mpidr 80000002
[    0.204307] CPU3: thread -1, cpu 3, socket 0, mpidr 80000003
[    0.204646] Brought up 4 CPUs
[    0.204679] SMP: Total of 4 processors activated (54.00 BogoMIPS).
[    0.204696] CPU: All CPU(s) started in SVC mode.
[    0.207763] devtmpfs: initialized
[    0.220432] VFP support v0.3: implementor 51 architecture 64 part 6f variant 1 rev 0
[    0.220947] clocksource: jiffies: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 19112604462750000 ns
[    0.221199] pinctrl core: initialized pinctrl subsystem
[    0.222655] NET: Registered protocol family 16
[    0.223179] DMA: preallocated 256 KiB pool for atomic coherent allocations
[    0.264168] cpuidle: using governor menu
[    0.264535] hw-breakpoint: Failed to enable monitor mode on CPU 3.
[    0.264548] hw-breakpoint: Failed to enable monitor mode on CPU 1.
[    0.264560] hw-breakpoint: Failed to enable monitor mode on CPU 2.
[    0.278269] qcom_rpm 108000.rpm: RPM firmware 3.0.16842936
[    0.327983] s1: Bringing 0uV into 1225000-1225000uV
[    0.328725] qcom_rpm_reg 108000.rpm:regulators: regulator requires qcom,switch-mode-frequency property
[    0.328744] qcom_rpm_reg 108000.rpm:regulators: driver callback failed to parse DT for regulator s2
[    0.329093] s3: Bringing 0uV into 500000-500000uV
[    0.329471] s4: Bringing 0uV into 1800000-1800000uV
[    0.330413] s7: Bringing 0uV into 1300000-1300000uV
[    0.330789] qcom_rpm_reg 108000.rpm:regulators: regulator requires qcom,switch-mode-frequency property
[    0.330807] qcom_rpm_reg 108000.rpm:regulators: driver callback failed to parse DT for regulator s8
[    0.331270] l1: supplied by s4
[    0.331793] l2: supplied by s4
[    0.331904] l2: Bringing 0uV into 1200000-1200000uV
[    0.332355] l3: Bringing 0uV into 3075000-3075000uV
[    0.332838] l4: Bringing 0uV into 1800000-1800000uV
[    0.333351] l5: Bringing 0uV into 2950000-2950000uV
[    0.334409] l6: Bringing 0uV into 2950000-2950000uV
[    0.337719] l11: Bringing 0uV into 3000000-3000000uV
[    0.338530] l12: supplied by s4
[    0.342047] l17: Bringing 0uV into 3000000-3000000uV
[    0.343084] l18: supplied by s4
[    0.346576] l23: Bringing 0uV into 1800000-1800000uV
[    0.347839] l24: supplied by s1
[    0.349235] l25: supplied by s1
[    0.349333] l25: Bringing 0uV into 1250000-1250000uV
[    0.350665] l26: supplied by s7
[    0.352114] l27: supplied by s7
[    0.353570] l28: supplied by s7
[    0.356649] lvs1: supplied by s4
[    0.358226] lvs2: supplied by s1
[    0.359832] lvs3: supplied by s4
[    0.361519] lvs4: supplied by s4
[    0.363206] lvs5: supplied by s4
[    0.364933] lvs6: supplied by s4
[    0.366772] lvs7: supplied by s4
[    0.372123] qcom_rpm_reg 108000.rpm:regulators: regulator requires qcom,switch-mode-frequency property
[    0.372141] qcom_rpm_reg 108000.rpm:regulators: driver callback failed to parse DT for regulator ncp
[    0.372279] ncp: supplied by l6
[    0.374869] SCSI subsystem initialized
[    0.375223] usbcore: registered new interface driver usbfs
[    0.375305] usbcore: registered new interface driver hub
[    0.375583] usbcore: registered new device driver usb
[    0.376583] Advanced Linux Sound Architecture Driver Initialized.
[    0.379194] clocksource: Switched to clocksource dg_timer
[    0.475512] NET: Registered protocol family 2
[    0.476304] TCP established hash table entries: 8192 (order: 3, 32768 bytes)
[    0.476375] TCP bind hash table entries: 8192 (order: 4, 65536 bytes)
[    0.476470] TCP: Hash tables configured (established 8192 bind 8192)
[    0.476555] UDP hash table entries: 512 (order: 2, 16384 bytes)
[    0.476590] UDP-Lite hash table entries: 512 (order: 2, 16384 bytes)
[    0.476822] NET: Registered protocol family 1
[    0.477375] Trying to unpack rootfs image as initramfs...
[    0.542772] Freeing initrd memory: 900K (c2200000 - c22e1000)
[    0.543450] hw perfevents: enabled with armv7_krait PMU driver, 5 counters available
[    0.545665] futex hash table entries: 1024 (order: 4, 65536 bytes)
[    0.545734] audit: initializing netlink subsys (disabled)
[    0.545833] audit: type=2000 audit(0.539:1): initialized
[    0.547209] workingset: timestamp_bits=30 max_order=19 bucket_order=0
[    0.557442] fuse init (API version 7.25)
[    0.558310] SELinux:  Registering netfilter hooks
[    0.566548] bounce: pool size: 64 pages
[    0.566639] Block layer SCSI generic (bsg) driver version 0.4 loaded (major 252)
[    0.566662] io scheduler noop registered
[    0.566677] io scheduler deadline registered
[    0.566731] io scheduler cfq registered (default)
[    0.571896] gsbi 12440000.gsbi: GSBI port protocol: 2 crci: 0
[    0.572621] gsbi 16200000.gsbi: GSBI port protocol: 2 crci: 0
[    0.573146] gsbi 16500000.gsbi: GSBI port protocol: 6 crci: 0
[    0.574138] gsbi 16600000.gsbi: GSBI port protocol: 6 crci: 0
[    0.575667] msm_serial 16540000.serial: msm_serial: detected port #1
[    0.575760] msm_serial 16540000.serial: uartclk = 27000000
[    0.575838] 16540000.serial: ttyMSM1 at MMIO 0x16540000 (irq = 121, base_baud = 1687500) is a MSM
[    0.576515] msm_serial 16640000.serial: msm_serial: detected port #0
[    0.576600] msm_serial 16640000.serial: uartclk = 1843200
[    0.576667] 16640000.serial: ttyMSM0 at MMIO 0x16640000 (irq = 123, base_baud = 115200) is a MSM
[    0.576716] msm_serial: console setup on port #0
[    1.339702] console [ttyMSM0] enabled
[    1.344883] msm_serial: driver initialized
[    1.372207] brd: module loaded
[    1.383806] loop: module loaded
[    1.384166] ssbi 500000.qcom,ssbi: SSBI controller type: 'pmic-arbiter'
[    1.386317] pm8921_probe: PMIC revision 1: F4
[    1.392516] pm8921_probe: PMIC revision 2: 06
[    1.406860] SCSI Media Changer driver v0.25 
[    1.409002] libphy: Fixed MDIO Bus: probed
[    1.410360] tun: Universal TUN/TAP device driver, 1.6
[    1.414104] tun: (C) 1999-2004 Max Krasnyansky <maxk@qualcomm.com>
[    1.419983] PPP generic driver version 2.4.2
[    1.425769] PPP BSD Compression module registered
[    1.429811] PPP Deflate Compression module registered
[    1.434351] PPP MPPE Compression module registered
[    1.439455] SLIP: version 0.8.4-NET3.019-NEWTTY (dynamic channels, max=256) (6 bit encapsulation enabled).
[    1.444058] CSLIP: code copyright 1989 Regents of the University of California.
[    1.453858] usbcore: registered new interface driver ax88179_178a
[    1.461039] usbcore: registered new interface driver cdc_ether
[    1.467203] usbcore: registered new interface driver net1080
[    1.473020] usbcore: registered new interface driver cdc_subset
[    1.478763] usbcore: registered new interface driver cdc_ncm
[    1.484801] msm_otg 12500000.phy: OTG regs = ea92c000
[    1.610285] ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver
[    1.610348] ehci-msm: Qualcomm On-Chip EHCI Host Controller
[    1.616407] usbcore: registered new interface driver cdc_acm
[    1.621276] cdc_acm: USB Abstract Control Model driver for USB modems and ISDN adapters
[    1.627153] usbcore: registered new interface driver usb-storage
[    1.635020] usbcore: registered new interface driver usbserial
[    1.648234] mousedev: PS/2 mouse device common for all mice
[    1.649570] input: pmic8xxx_pwrkey as /devices/platform/soc/500000.qcom,ssbi/500000.qcom,ssbi:pmic@0/500000.qcom,ssbi:pmic@0:pwrkey@1c/input/input0
[    1.654673] rtc-pm8xxx 500000.qcom,ssbi:pmic@0:rtc@11d: rtc core: registered pm8xxx_rtc as rtc0
[    1.666036] i2c /dev entries driver
[    1.678160] apq8064-pinctrl 800000.pinctrl: pin GPIO_16 already requested by 16540000.serial; cannot claim for 16580000.i2c
[    1.678207] apq8064-pinctrl 800000.pinctrl: pin-16 (16580000.i2c) status -22
[    1.678362] 1-0010 supply vcc33 not found, using dummy regulator
[    1.678779] 1-0010 supply vccio not found, using dummy regulator
[    1.702427] apq8064-pinctrl 800000.pinctrl: could not request pin 16 (GPIO_16) from group gpio16  on device 800000.pinctrl
[    1.708353] i2c_qup 16580000.i2c: Error applying setting, reverse things back
[    1.721193] bq27xxx-battery 0-0055: support ver. 1.2.0 enabled
[    1.744176] device-mapper: uevent: version 1.0.3
[    1.745346] device-mapper: ioctl: 4.35.0-ioctl (2016-06-23) initialised: dm-devel@redhat.com
[    1.750077] mmci-pl18x 12400000.sdcc: mmc0: PL180 manf 51 rev0 at 0x12400000 irq 118,0 (pio)
[    1.756470] mmci-pl18x 12400000.sdcc: DMA channels RX dma0chan1, TX dma0chan2
[    1.765137] s4: voltage operation not allowed
[    1.771830] mmci-pl18x 12400000.sdcc: Voltage switch failed
[    1.779388] msm_otg 12500000.phy: Avail curr from USB = 100
[    1.823366] input: Elan Touchscreen as /devices/platform/soc/16200000.gsbi/16280000.i2c/i2c-1/1-0010/input/input1
[    1.829545] sdhci: Secure Digital Host Controller Interface driver
[    1.832695] sdhci: Copyright(c) Pierre Ossman
[    1.839743] sdhci-pltfm: SDHCI platform and OF driver helper
[    1.845554] hidraw: raw HID events driver (C) Jiri Kosina
[    1.849544] usbcore: registered new interface driver usbhid
[    1.854200] usbhid: USB HID core driver
[    1.860207] ashmem: initialized
[    1.882659] oprofile: using timer interrupt.
[    1.882829] u32 classifier
[    1.885992]     Actions configured
[    1.888521] Netfilter messages via NETLINK v0.30.
[    1.892458] nf_conntrack version 0.5.0 (16384 buckets, 65536 max)
[    1.897323] ctnetlink v0.93: registering with nfnetlink.
[    1.903039] nf_tables: (c) 2007-2009 Patrick McHardy <kaber@trash.net>
[    1.908589] xt_time: kernel timezone is -0000
[    1.914959] ip_tables: (C) 2000-2006 Netfilter Core Team
[    1.919457] arp_tables: arp_tables: (C) 2002 David S. Miller
[    1.924395] Initializing XFRM netlink socket
[    1.931085] NET: Registered protocol family 10
[    1.936032] mip6: Mobile IPv6
[    1.938458] ip6_tables: (C) 2000-2006 Netfilter Core Team
[    1.941842] sit: IPv6, IPv4 and MPLS over IPv4 tunneling driver
[    1.947793] NET: Registered protocol family 17
[    1.952730] NET: Registered protocol family 15
[    1.957476] Registering SWP/SWPB emulation handler
[    1.965391] input: gpio-keys as /devices/platform/gpio-keys/input/input2
[    1.967226] rtc-pm8xxx 500000.qcom,ssbi:pmic@0:rtc@11d: setting system clock to 2016-08-05 18:04:52 UTC (1470420292)
[    1.996762] ext_3p3v: disabling
[    1.997670] ALSA device list:
[    1.998718]   No soundcards found.
[    2.003445] mmc0: new high speed MMC card at address 0001
[    2.006240] mmcblk0: mmc0:0001 016GE2 14.7 GiB 
[    2.011830] mmcblk0boot0: mmc0:0001 016GE2 partition 1 4.00 MiB
[    2.012502] mmcblk0boot1: mmc0:0001 016GE2 partition 2 4.00 MiB
[    2.013667] Freeing unused kernel memory: 1024K (c0c00000 - c0d00000)
[    2.023550] mmcblk0rpmb: mmc0:0001 016GE2 partition 3 4.00 MiB
[    2.034144]  mmcblk0: p1 p2 p3 p4 p5 p6 p7 p8 p9 p10 p11 p12 p13 p14 p15 p16 p17 p18 p19 p20 p21 p22 p23 p24 p25 p26 p27 p28 p29 p30
[    2.050694] init: init started!
[    2.075616] SELinux: 2048 avtab hash slots, 5414 rules.
[    2.079089] SELinux:  Android master kernel running Android M policy in compatibility mode.
[    2.093626] SELinux: 2048 avtab hash slots, 5414 rules.
[    2.093735] SELinux:  1 users, 2 roles, 581 types, 0 bools, 1 sens, 1024 cats
[    2.093787] SELinux:  87 classes, 5414 rules
[    2.098908] SELinux:  Permission validate_trans in class security not defined in policy.
[    2.099060] SELinux:  Permission module_load in class system not defined in policy.
[    2.107326] SELinux:  Class netlink_iscsi_socket not defined in policy.
[    2.113662] SELinux:  Class netlink_fib_lookup_socket not defined in policy.
[    2.120187] SELinux:  Class netlink_connector_socket not defined in policy.
[    2.127367] SELinux:  Class netlink_netfilter_socket not defined in policy.
[    2.134255] SELinux:  Class netlink_generic_socket not defined in policy.
[    2.141091] SELinux:  Class netlink_scsitransport_socket not defined in policy.
[    2.147939] SELinux:  Class netlink_rdma_socket not defined in policy.
[    2.155158] SELinux:  Class netlink_crypto_socket not defined in policy.
[    2.161919] SELinux:  Permission audit_read in class capability2 not defined in policy.
[    2.168575] SELinux:  Class cap_userns not defined in policy.
[    2.176337] SELinux:  Class cap2_userns not defined in policy.
[    2.182258] SELinux: the above unknown classes and permissions will be denied
[    2.187944] SELinux:  Completing initialization.
[    2.187960] SELinux:  Setting up existing superblocks.
[    2.319904] audit: type=1403 audit(1470420292.849:2): policy loaded auid=4294967295 ses=4294967295
[    2.321917] init: (Initializing SELinux non-enforcing took 0.27s.)
[    2.378846] init: init second stage started!
[    2.568874] init: waitpid failed: No child processes
[    2.570383] init: (Loading properties from /default.prop took 0.00s.)
[    2.583174] init: (Parsing /init.environ.rc took 0.00s.)
[    2.584847] init: (Parsing /init.usb.rc took 0.00s.)
[    2.593192] init: (Parsing init.flo.usb.rc took 0.00s.)
[    2.593563] init: (Parsing init.flo.diag.rc took 0.00s.)
[    2.597559] init: (Parsing /init.flo.rc took 0.01s.)
[    2.605660] init: (Parsing /init.usb.configfs.rc took 0.00s.)
[    2.608448] init: (Parsing /init.zygote32.rc took 0.00s.)
[    2.618378] audit: type=1400 audit(1470420293.139:3): avc:  denied  { mounton } for  pid=1 comm="init" path="/sys/kernel/debug" dev="debugfs" ino=1 scontext=u:r:init:s0 tcontext=u:object_r:debugfs:s0 tclass=dir permissive=1
[    2.622672] ueventd: ueventd started!
[    3.180479] ueventd: Coldboot took 0.53s.
[    3.353090] audit: type=1400 audit(1470420293.879:4): avc:  denied  { write } for  pid=1 comm="init" name="cpu" dev="proc" ino=4026531912 scontext=u:r:init:s0 tcontext=u:object_r:proc:s0 tclass=dir permissive=1
[    3.353317] audit: type=1400 audit(1470420293.879:5): avc:  denied  { add_name } for  pid=1 comm="init" name="alignment" scontext=u:r:init:s0 tcontext=u:object_r:proc:s0 tclass=dir permissive=1
[    3.370990] audit: type=1400 audit(1470420293.899:6): avc:  denied  { create } for  pid=1 comm="init" name="alignment" scontext=u:r:init:s0 tcontext=u:object_r:proc:s0 tclass=file permissive=1
[    3.397979] audit: type=1400 audit(1470420293.919:7): avc:  denied  { create } for  pid=1 comm="init" name="cpu.shares" scontext=u:r:init:s0 tcontext=u:object_r:cgroup:s0 tclass=file permissive=1
[    3.440034] EXT4-fs (mmcblk0p22): mounted filesystem without journal. Opts: barrier=1
[    3.443034] audit: type=1400 audit(1470420293.969:8): avc:  denied  { mounton } for  pid=113 comm="init" path="/cache" dev="rootfs" ino=9243 scontext=u:r:init:s0 tcontext=u:object_r:cache_file:s0 tclass=dir permissive=1
[    3.447674] EXT4-fs (mmcblk0p23): Ignoring removed nomblk_io_submit option
[    3.474315] EXT4-fs (mmcblk0p23): mounted filesystem with ordered data mode. Opts: nomblk_io_submit,errors=remount-ro
[    3.515264] random: fast init done
[    3.682524] audit: type=1400 audit(1470420294.209:9): avc:  denied  { getattr } for  pid=118 comm="e2fsck" path="/dev/block/mmcblk0p23" dev="tmpfs" ino=8416 scontext=u:r:fsck:s0 tcontext=u:object_r:block_device:s0 tclass=blk_file permissive=1
[    3.683207] audit: type=1400 audit(1470420294.209:10): avc:  denied  { read } for  pid=118 comm="e2fsck" name="mmcblk0p23" dev="tmpfs" ino=8416 scontext=u:r:fsck:s0 tcontext=u:object_r:block_device:s0 tclass=blk_file permissive=1
[    3.718336] EXT4-fs (mmcblk0p23): Ignoring removed nomblk_io_submit option
[    3.732767] EXT4-fs (mmcblk0p23): mounted filesystem with ordered data mode. Opts: barrier=1,data=ordered,nomblk_io_submit,errors=panic
[    3.736337] EXT4-fs (mmcblk0p30): Ignoring removed nomblk_io_submit option
[    3.754923] EXT4-fs (mmcblk0p30): mounted filesystem with ordered data mode. Opts: nomblk_io_submit,errors=remount-ro
[    4.021422] EXT4-fs (mmcblk0p30): Ignoring removed nomblk_io_submit option
[    4.032191] EXT4-fs (mmcblk0p30): mounted filesystem with ordered data mode. Opts: barrier=1,data=ordered,nomblk_io_submit,errors=panic
[    4.043903] EXT4-fs (mmcblk0p4): mounted filesystem with ordered data mode. Opts: barrier=1,data=ordered,nodelalloc
[    4.245507] logd.auditd: start
[    4.245733] logd.klogd: 4246495553
[    4.484262] binder: 143:143 transaction failed 29189, size 0-0
[    4.521095] file system registered
[    4.522644] type=1400 audit(1470420295.039:16): avc: denied { associate } for pid=1 comm="init" name="g1" scontext=u:object_r:unlabeled:s0 tcontext=u:object_r:unlabeled:s0 tclass=filesystem permissive=1
[    4.549652] type=1400 audit(1470420295.059:17): avc: denied { write } for pid=32 comm="kdevtmpfs" name="/" dev="devtmpfs" ino=1025 scontext=u:r:kernel:s0 tcontext=u:object_r:device:s0 tclass=dir permissive=1
[    4.552119] type=1400 audit(1470420295.059:18): avc: denied { mknod } for pid=32 comm="kdevtmpfs" capability=27 scontext=u:r:kernel:s0 tcontext=u:r:kernel:s0 tclass=capability permissive=1
[    4.577782] logd.daemon: reinit
[    4.624654] type=1400 audit(1470420295.059:19): avc: denied { add_name } for pid=32 comm="kdevtmpfs" name="mtp_usb" scontext=u:r:kernel:s0 tcontext=u:object_r:device:s0 tclass=dir permissive=1
[    4.780015] read descriptors
[    4.780100] read strings
[    4.873189] type=1400 audit(1470420295.059:20): avc: denied { create } for pid=32 comm="kdevtmpfs" name="mtp_usb" scontext=u:r:kernel:s0 tcontext=u:object_r:device:s0 tclass=chr_file permissive=1
[    4.891134] type=1400 audit(1470420295.059:21): avc: denied { setattr } for pid=32 comm="kdevtmpfs" name="mtp_usb" dev="devtmpfs" ino=1227 scontext=u:r:kernel:s0 tcontext=u:object_r:device:s0 tclass=chr_file permissive=1
[    4.891615] type=1400 audit(1470420295.069:22): avc: denied { mount } for pid=1 comm="init" name="/" dev="tracefs" ino=1 scontext=u:r:kernel:s0 tcontext=u:object_r:unlabeled:s0 tclass=filesystem permissive=1
[    5.185119] android_work: sent uevent USB_STATE=CONNECTED
[    5.188892] android_work: sent uevent USB_STATE=DISCONNECTED
[    5.293100] android_work: sent uevent USB_STATE=CONNECTED
[    5.295648] configfs-gadget gadget: high-speed config #1: b
[    5.297556] msm_otg 12500000.phy: Avail curr from USB = 500
[    5.303372] android_work: sent uevent USB_STATE=CONFIGURED
[    5.491990] healthd: No charger supplies found
[    9.527933] init: Starting service 'netd'...
[    9.531934] init: Starting service 'surfaceflinger'...
[    9.535424] init: Starting service 'media'...
[    9.540183] init: Starting service 'zygote'...
[   10.564509] init: Service 'surfaceflinger' (pid 223) killed by signal 6
[   10.564602] init: Service 'surfaceflinger' (pid 223) killing any children in process group
[   10.570165] init: Service 'zygote' is being killed...
[   10.596251] init: Service 'zygote' (pid 225) killed by signal 9
[   10.596340] init: Service 'zygote' (pid 225) killing any children in process group
[   10.601333] init: write_file: Unable to open '/sys/android_power/request_state': No such file or directory
[   14.985486] init: Starting service 'media'...

Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons

[Jon Hunter]

On 05/08/16 19:12, John Stultz wrote:
> On Sat, Jul 30, 2016 at 1:07 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
>> On Fri, 29 Jul 2016, John Stultz wrote:
>>> Hey Jon,
>>>   So after rebasing my nexus7 patch stack onto pre-4.8-rc1 tree, I
>>> noticed the power/volume buttons stopped working.
>>>
>>> I did a manual rebased bisection and chased it down to your commit
>>> 1e2a7d78499e ("irqdomain: Don't set type when mapping an IRQ").
>>>
>>> Reverting that patch makes things work again, so I wanted to see if
>>> there was any debugging info I could provide to try to help narrow
>>> down the problem here. (Sorry, I'd tinker myself with it some and try
>>> to debug the issue, but after burning my friday night on this, I'm
>>> eager to get away from the keyboard for the weekend).
>>
>> dmesg should contain debug output which yells about non matching types. Can
>> you provide that?
> 
> I'm not seeing anything about problematic type matching in the dmesg.
> 
> I've also added LinusW's patches but that didn't seem to help either.
> 
> dmesg attached.

I am wondering if it is related to ...

[    1.678160] apq8064-pinctrl 800000.pinctrl: pin GPIO_16 already requested by 16540000.serial; cannot claim for 16580000.i2c
[    1.678207] apq8064-pinctrl 800000.pinctrl: pin-16 (16580000.i2c) status -22
[    1.678362] 1-0010 supply vcc33 not found, using dummy regulator
[    1.678779] 1-0010 supply vccio not found, using dummy regulator
[    1.702427] apq8064-pinctrl 800000.pinctrl: could not request pin 16 (GPIO_16) from group gpio16  on device 800000.pinctrl

Cheers
Jon

-- 
nvpublic

Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons

[Linus Walleij]

On Mon, Aug 8, 2016 at 11:04 AM, Jon Hunter <jonathanh@nvidia.com> wrote:

> I am wondering if it is related to ...
>
> [    1.678160] apq8064-pinctrl 800000.pinctrl: pin GPIO_16 already requested by 16540000.serial; cannot claim for 16580000.i2c
> [    1.678207] apq8064-pinctrl 800000.pinctrl: pin-16 (16580000.i2c) status -22
> [    1.678362] 1-0010 supply vcc33 not found, using dummy regulator
> [    1.678779] 1-0010 supply vccio not found, using dummy regulator
> [    1.702427] apq8064-pinctrl 800000.pinctrl: could not request pin 16 (GPIO_16) from group gpio16  on device 800000.pinctrl

I don't think so. That looks like a straight-formward pin conflict where the
i2c is tossed out because the pins it tries to take are already taken by
the serial.

Has nothing to do with any IRQs.

It's not neat but a different issue.

Yours,
Linus Walleij

Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons

[Linus Walleij]

On Sat, Jul 30, 2016 at 10:07 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Fri, 29 Jul 2016, John Stultz wrote:
>> Hey Jon,
>>   So after rebasing my nexus7 patch stack onto pre-4.8-rc1 tree, I
>> noticed the power/volume buttons stopped working.
>>
>> I did a manual rebased bisection and chased it down to your commit
>> 1e2a7d78499e ("irqdomain: Don't set type when mapping an IRQ").
>>
>> Reverting that patch makes things work again, so I wanted to see if
>> there was any debugging info I could provide to try to help narrow
>> down the problem here. (Sorry, I'd tinker myself with it some and try
>> to debug the issue, but after burning my friday night on this, I'm
>> eager to get away from the keyboard for the weekend).
>
> dmesg should contain debug output which yells about non matching types. Can
> you provide that?

I had that problem *too* on the Qualcomm platforms (APQ8060, MSM8660
and the APQ8064 variants, but fixed it up with these two patches:
http://marc.info/?l=linux-arm-kernel&m=147038646226142&w=2

In addition to that there is some other problem, discussed in this
thread... I understand that less.

Yours,
Linus Walleij

Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons

[Jon Hunter]

Hi John,

On 30/07/16 05:39, John Stultz wrote:
> Hey Jon,
>   So after rebasing my nexus7 patch stack onto pre-4.8-rc1 tree, I
> noticed the power/volume buttons stopped working.
> 
> I did a manual rebased bisection and chased it down to your commit
> 1e2a7d78499e ("irqdomain: Don't set type when mapping an IRQ").
> 
> Reverting that patch makes things work again, so I wanted to see if
> there was any debugging info I could provide to try to help narrow
> down the problem here. (Sorry, I'd tinker myself with it some and try
> to debug the issue, but after burning my friday night on this, I'm
> eager to get away from the keyboard for the weekend).

Before this commit bad IRQ type settings in device-tree were not getting
reported and so failures to set the IRQ type were going unnoticed. It's
most likely a bad IRQ type settings somewhere.

As Thomas mentioned hopefully dmesg will shed a bit more light.

Otherwise it can be worth looking at the ->irq_set_type() function for
the irqchips in the path of the interrupt requested to see if any are
failing. Looking at the nexus7 (assuming qcom variant), it looks like
there are 3 irqchips in the path (pm8921 --> apq8064-pinctrl --> gic).
The pm8xxx_irq_set_type() could return a failure when setting up the IRQ
type and could be worth checking. It does not look like the set_type for
the apq8064-pinctrl should ever fail (apart from calling BUG() which
would be obvious). The gic can also return a failure for setting the
type, but I did not see anything at first glance that looks incorrect in
the dts.

If we can narrow down irqchip, then hopefully it will be clearer.

Cheers
Jon

-- 
nvpublic

Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons

[John Stultz]

On Mon, Aug 1, 2016 at 3:26 AM, Jon Hunter <jonathanh@nvidia.com> wrote:
> Hi John,
>
> On 30/07/16 05:39, John Stultz wrote:
>> Hey Jon,
>>   So after rebasing my nexus7 patch stack onto pre-4.8-rc1 tree, I
>> noticed the power/volume buttons stopped working.
>>
>> I did a manual rebased bisection and chased it down to your commit
>> 1e2a7d78499e ("irqdomain: Don't set type when mapping an IRQ").
>>
>> Reverting that patch makes things work again, so I wanted to see if
>> there was any debugging info I could provide to try to help narrow
>> down the problem here. (Sorry, I'd tinker myself with it some and try
>> to debug the issue, but after burning my friday night on this, I'm
>> eager to get away from the keyboard for the weekend).
>
> Before this commit bad IRQ type settings in device-tree were not getting
> reported and so failures to set the IRQ type were going unnoticed. It's
> most likely a bad IRQ type settings somewhere.
>
> As Thomas mentioned hopefully dmesg will shed a bit more light.
>
> Otherwise it can be worth looking at the ->irq_set_type() function for
> the irqchips in the path of the interrupt requested to see if any are
> failing. Looking at the nexus7 (assuming qcom variant), it looks like
> there are 3 irqchips in the path (pm8921 --> apq8064-pinctrl --> gic).
> The pm8xxx_irq_set_type() could return a failure when setting up the IRQ
> type and could be worth checking. It does not look like the set_type for
> the apq8064-pinctrl should ever fail (apart from calling BUG() which
> would be obvious). The gic can also return a failure for setting the
> type, but I did not see anything at first glance that looks incorrect in
> the dts.
>
> If we can narrow down irqchip, then hopefully it will be clearer.

The pm_8xxx_irq_set_type doesn't seem to be failing as far as I can see..

Looking at the patch that seems to cause the trouble, I narrowed it
down to just the following chunk:

@@ -614,7 +615,11 @@ unsigned int irq_create_fwspec_mapping(struct
irq_fwspec *fwspec)
                 * it now and return the interrupt number.
                 */
                if (irq_get_trigger_type(virq) == IRQ_TYPE_NONE) {
-                       irq_set_irq_type(virq, type);
+                       irq_data = irq_get_irq_data(virq);
+                       if (!irq_data)
+                               return 0;
+
+                       irqd_set_trigger_type(irq_data, type);
                        return virq;
                }

If I revert just that, it works again.

I was worried we were hitting an early failure from !irq_data, but it
seems there's some subtle difference between irqd_set_trigger_type and
irq_set_type that makes the former break for me.

Still digging though.

thanks
-john

Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons

[Jon Hunter]

On 06/08/16 00:45, John Stultz wrote:
> On Mon, Aug 1, 2016 at 3:26 AM, Jon Hunter <jonathanh@nvidia.com> wrote:
>> Hi John,
>>
>> On 30/07/16 05:39, John Stultz wrote:
>>> Hey Jon,
>>>   So after rebasing my nexus7 patch stack onto pre-4.8-rc1 tree, I
>>> noticed the power/volume buttons stopped working.
>>>
>>> I did a manual rebased bisection and chased it down to your commit
>>> 1e2a7d78499e ("irqdomain: Don't set type when mapping an IRQ").
>>>
>>> Reverting that patch makes things work again, so I wanted to see if
>>> there was any debugging info I could provide to try to help narrow
>>> down the problem here. (Sorry, I'd tinker myself with it some and try
>>> to debug the issue, but after burning my friday night on this, I'm
>>> eager to get away from the keyboard for the weekend).
>>
>> Before this commit bad IRQ type settings in device-tree were not getting
>> reported and so failures to set the IRQ type were going unnoticed. It's
>> most likely a bad IRQ type settings somewhere.
>>
>> As Thomas mentioned hopefully dmesg will shed a bit more light.
>>
>> Otherwise it can be worth looking at the ->irq_set_type() function for
>> the irqchips in the path of the interrupt requested to see if any are
>> failing. Looking at the nexus7 (assuming qcom variant), it looks like
>> there are 3 irqchips in the path (pm8921 --> apq8064-pinctrl --> gic).
>> The pm8xxx_irq_set_type() could return a failure when setting up the IRQ
>> type and could be worth checking. It does not look like the set_type for
>> the apq8064-pinctrl should ever fail (apart from calling BUG() which
>> would be obvious). The gic can also return a failure for setting the
>> type, but I did not see anything at first glance that looks incorrect in
>> the dts.
>>
>> If we can narrow down irqchip, then hopefully it will be clearer.
> 
> The pm_8xxx_irq_set_type doesn't seem to be failing as far as I can see..
> 
> Looking at the patch that seems to cause the trouble, I narrowed it
> down to just the following chunk:
> 
> @@ -614,7 +615,11 @@ unsigned int irq_create_fwspec_mapping(struct
> irq_fwspec *fwspec)
>                  * it now and return the interrupt number.
>                  */
>                 if (irq_get_trigger_type(virq) == IRQ_TYPE_NONE) {
> -                       irq_set_irq_type(virq, type);
> +                       irq_data = irq_get_irq_data(virq);
> +                       if (!irq_data)
> +                               return 0;
> +
> +                       irqd_set_trigger_type(irq_data, type);
>                         return virq;
>                 }
> 
> If I revert just that, it works again.
> 
> I was worried we were hitting an early failure from !irq_data, but it
> seems there's some subtle difference between irqd_set_trigger_type and
> irq_set_type that makes the former break for me.

Thanks this is good info and at the same time odd.

I am guessing that it is failing above because the irq_data is not found
for the irq?

What is odd, is that the above sequence is only executed if a irq
mapping exists and so really, AFAICT this should not happen. Ie. the irq
descriptor should have been allocated for the mapping to exist. We
should probably warn if this happens.

Without reverting the above, can you add a print to show the
domain->name, hwirq and virq information if !irq_data? That will confirm
the domain for us.

Also it could be worth enabling all the debug prints in
kernel/irq/irqdomain.c to see what is happening on boot.

Cheers
Jon

-- 
nvpublic

Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons

[John Stultz]

On Mon, Aug 8, 2016 at 2:31 AM, Jon Hunter <jonathanh@nvidia.com> wrote:
>
> On 06/08/16 00:45, John Stultz wrote:
>> On Mon, Aug 1, 2016 at 3:26 AM, Jon Hunter <jonathanh@nvidia.com> wrote:
>>> Hi John,
>>>
>>> On 30/07/16 05:39, John Stultz wrote:
>>>> Hey Jon,
>>>>   So after rebasing my nexus7 patch stack onto pre-4.8-rc1 tree, I
>>>> noticed the power/volume buttons stopped working.
>>>>
>>>> I did a manual rebased bisection and chased it down to your commit
>>>> 1e2a7d78499e ("irqdomain: Don't set type when mapping an IRQ").
>>>>
>>>> Reverting that patch makes things work again, so I wanted to see if
>>>> there was any debugging info I could provide to try to help narrow
>>>> down the problem here. (Sorry, I'd tinker myself with it some and try
>>>> to debug the issue, but after burning my friday night on this, I'm
>>>> eager to get away from the keyboard for the weekend).
>>>
>>> Before this commit bad IRQ type settings in device-tree were not getting
>>> reported and so failures to set the IRQ type were going unnoticed. It's
>>> most likely a bad IRQ type settings somewhere.
>>>
>>> As Thomas mentioned hopefully dmesg will shed a bit more light.
>>>
>>> Otherwise it can be worth looking at the ->irq_set_type() function for
>>> the irqchips in the path of the interrupt requested to see if any are
>>> failing. Looking at the nexus7 (assuming qcom variant), it looks like
>>> there are 3 irqchips in the path (pm8921 --> apq8064-pinctrl --> gic).
>>> The pm8xxx_irq_set_type() could return a failure when setting up the IRQ
>>> type and could be worth checking. It does not look like the set_type for
>>> the apq8064-pinctrl should ever fail (apart from calling BUG() which
>>> would be obvious). The gic can also return a failure for setting the
>>> type, but I did not see anything at first glance that looks incorrect in
>>> the dts.
>>>
>>> If we can narrow down irqchip, then hopefully it will be clearer.
>>
>> The pm_8xxx_irq_set_type doesn't seem to be failing as far as I can see..
>>
>> Looking at the patch that seems to cause the trouble, I narrowed it
>> down to just the following chunk:
>>
>> @@ -614,7 +615,11 @@ unsigned int irq_create_fwspec_mapping(struct
>> irq_fwspec *fwspec)
>>                  * it now and return the interrupt number.
>>                  */
>>                 if (irq_get_trigger_type(virq) == IRQ_TYPE_NONE) {
>> -                       irq_set_irq_type(virq, type);
>> +                       irq_data = irq_get_irq_data(virq);
>> +                       if (!irq_data)
>> +                               return 0;
>> +
>> +                       irqd_set_trigger_type(irq_data, type);
>>                         return virq;
>>                 }
>>
>> If I revert just that, it works again.
>>
>> I was worried we were hitting an early failure from !irq_data, but it
>> seems there's some subtle difference between irqd_set_trigger_type and
>> irq_set_type that makes the former break for me.
>
> Thanks this is good info and at the same time odd.
>
> I am guessing that it is failing above because the irq_data is not found
> for the irq?

So actually no. We usually call irqd_set_trigger_type() but something
still doesn't work.

Interestingly, just adding irq_set_irq_type(virq, type); to the top of
that block (leaving the rest of the code) also works.

> What is odd, is that the above sequence is only executed if a irq
> mapping exists and so really, AFAICT this should not happen. Ie. the irq
> descriptor should have been allocated for the mapping to exist. We
> should probably warn if this happens.
>
> Without reverting the above, can you add a print to show the
> domain->name, hwirq and virq information if !irq_data? That will confirm
> the domain for us.

So I put some printk info in (in either case since I'm never seeing
the !irq_data case happen):

[    1.514217] JDB: virq: 93  hwirq: 74 domain name: msmgpio
[    1.838342] JDB: virq: 25  hwirq: 6 domain name: msmgpio

Which is odd, looking at:

shell@flo:/ $ cat /proc/interrupts
           CPU0       CPU1       CPU2       CPU3
 16:       1159       1138       1332       1574     GIC-0  18 Edge
  gp_timer
 25:          0          0          0          0   msmgpio   6 Edge
  ekth3500
111:          6          0          0          0     GIC-0  51 Edge
  qcom_rpm_ack
112:          0          0          0          0     GIC-0  53 Edge
  qcom_rpm_err
113:          0          0          0          0     GIC-0  54 Edge
  qcom_rpm_wakeup
114:         48          0          0          0     GIC-0 132 Edge
  msm_otg, ci_hdrc_msm
115:        796          0          0          0     GIC-0 130 Level     bam_dma
116:          0          0          0          0     GIC-0 128 Level     bam_dma
117:          0          0          0          0     GIC-0 127 Level     bam_dma
118:       2627          0          0          0     GIC-0 136 Level
  mmci-pl18x (cmd)
119:         54          0          0          0     GIC-0 226 Level     i2c_qup
120:         21          0          0          0     GIC-0 183 Level     i2c_qup
122:          0          0          0          0     GIC-0 189 Level     i2c_qup
123:        202          0          0          0     GIC-0 190 Level
  msm_serial0
124:          0          0          0          0     GIC-0  70 Edge      smsm
125:          0          0          0          0     GIC-0 121 Edge      smsm
126:          0          0          0          0     GIC-0 236 Edge      smsm
127:          0          0          0          0     GIC-0 169 Edge      smsm
131:          0          0          0          0    pm8xxx 195 Edge
  Volume Up
165:          0          0          0          0    pm8xxx 229 Edge
  Volume Down
184:          0          0          0          0    pm8xxx  39 Edge
  pm8xxx_rtc_alarm
185:          0          0          0          0    pm8xxx  50 Edge
  pmic8xxx_pwrkey_release
186:          0          0          0          0    pm8xxx  51 Edge
  pmic8xxx_pwrkey_press
IPI0:          0          1          1          1  CPU wakeup interrupts
IPI1:          0          0          0          0  Timer broadcast interrupts
IPI2:        944        539       1015        529  Rescheduling interrupts
IPI3:          1          4          6          4  Function call interrupts
IPI4:          0          0          0          0  CPU stop interrupts
IPI5:          0          0          0          0  IRQ work interrupts
IPI6:          0          0          0          0  completion interrupts
Err:          0

Since 25 maps to the ekth3500 (touch panel, which is still working
fine), but 93/74 doesn't seem to map to anything, and the problematic
irqs are the volume keys 195/229 and power keys 50/51.

thanks
-john

Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons

[Jon Hunter]

On 09/08/16 05:25, John Stultz wrote:

...

> So actually no. We usually call irqd_set_trigger_type() but something
> still doesn't work.
> 
> Interestingly, just adding irq_set_irq_type(virq, type); to the top of
> that block (leaving the rest of the code) also works.

Interesting. By saving the trigger type during the mapping, we defer
setting the interrupt type to when the interrupt is requested. So this
would imply that the interrupt is not being setup as expected when its
requested.
 
>> What is odd, is that the above sequence is only executed if a irq
>> mapping exists and so really, AFAICT this should not happen. Ie. the irq
>> descriptor should have been allocated for the mapping to exist. We
>> should probably warn if this happens.
>>
>> Without reverting the above, can you add a print to show the
>> domain->name, hwirq and virq information if !irq_data? That will confirm
>> the domain for us.
> 
> So I put some printk info in (in either case since I'm never seeing
> the !irq_data case happen):
> 
> [    1.514217] JDB: virq: 93  hwirq: 74 domain name: msmgpio
> [    1.838342] JDB: virq: 25  hwirq: 6 domain name: msmgpio
> 
> Which is odd, looking at:
> 
> shell@flo:/ $ cat /proc/interrupts
>            CPU0       CPU1       CPU2       CPU3
>  16:       1159       1138       1332       1574     GIC-0  18 Edge
>   gp_timer
>  25:          0          0          0          0   msmgpio   6 Edge
>   ekth3500
> 111:          6          0          0          0     GIC-0  51 Edge
>   qcom_rpm_ack
> 112:          0          0          0          0     GIC-0  53 Edge
>   qcom_rpm_err
> 113:          0          0          0          0     GIC-0  54 Edge
>   qcom_rpm_wakeup
> 114:         48          0          0          0     GIC-0 132 Edge
>   msm_otg, ci_hdrc_msm
> 115:        796          0          0          0     GIC-0 130 Level     bam_dma
> 116:          0          0          0          0     GIC-0 128 Level     bam_dma
> 117:          0          0          0          0     GIC-0 127 Level     bam_dma
> 118:       2627          0          0          0     GIC-0 136 Level
>   mmci-pl18x (cmd)
> 119:         54          0          0          0     GIC-0 226 Level     i2c_qup
> 120:         21          0          0          0     GIC-0 183 Level     i2c_qup
> 122:          0          0          0          0     GIC-0 189 Level     i2c_qup
> 123:        202          0          0          0     GIC-0 190 Level
>   msm_serial0
> 124:          0          0          0          0     GIC-0  70 Edge      smsm
> 125:          0          0          0          0     GIC-0 121 Edge      smsm
> 126:          0          0          0          0     GIC-0 236 Edge      smsm
> 127:          0          0          0          0     GIC-0 169 Edge      smsm
> 131:          0          0          0          0    pm8xxx 195 Edge
>   Volume Up
> 165:          0          0          0          0    pm8xxx 229 Edge
>   Volume Down
> 184:          0          0          0          0    pm8xxx  39 Edge
>   pm8xxx_rtc_alarm
> 185:          0          0          0          0    pm8xxx  50 Edge
>   pmic8xxx_pwrkey_release
> 186:          0          0          0          0    pm8xxx  51 Edge
>   pmic8xxx_pwrkey_press
> IPI0:          0          1          1          1  CPU wakeup interrupts
> IPI1:          0          0          0          0  Timer broadcast interrupts
> IPI2:        944        539       1015        529  Rescheduling interrupts
> IPI3:          1          4          6          4  Function call interrupts
> IPI4:          0          0          0          0  CPU stop interrupts
> IPI5:          0          0          0          0  IRQ work interrupts
> IPI6:          0          0          0          0  completion interrupts
> Err:          0
> 
> Since 25 maps to the ekth3500 (touch panel, which is still working
> fine), but 93/74 doesn't seem to map to anything, and the problematic
> irqs are the volume keys 195/229 and power keys 50/51.

So looking at the DT source, I believe that hwirq 74 (virq 93) is the
problem. This is the parent interrupt from the pm8xxx to the apq8064
if it is not requested then the type is not set. It seems that for
parent interrupts these are not typically requested, but enabled when
an irqchip is chained.

To confirm and for testing purposes I am curious if this works ...

	if (irq_get_trigger_type(virq) == IRQ_TYPE_NONE) {
		irq_data = irq_get_irq_data(virq);
		if (!irq_data)
			return 0;

-		irqd_set_trigger_type(irq_data, type);
+ 		if (hwirq == 74)
+			irq_set_irq_type(virq, type);
+		else
+			irqd_set_trigger_type(irq_data, type);
		return virq;
	}

If that works, then does the following also work (without the above) ...

diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index b4c1bc7c9ca2..e111b72e3162 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -824,6 +824,7 @@ __irq_do_set_handler(struct irq_desc *desc, irq_flow_handler_t handle,
                irq_settings_set_norequest(desc);
                irq_settings_set_nothread(desc);
                desc->action = &chained_action;
+               __irq_set_trigger(desc, irqd_get_trigger_type(&desc->irq_data));
                irq_startup(desc, true);
        }
 }

It looks like there is a path for parent interrupts where the type
is not getting set. If the above works then we can discuss with Thomas
and Marc on the correct fix.

Cheers
Jon

-- 
nvpublic

Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons

[Marc Zyngier]

On 09/08/16 14:20, Jon Hunter wrote:
> 
> On 09/08/16 05:25, John Stultz wrote:
> 
> ...
> 
>> So actually no. We usually call irqd_set_trigger_type() but something
>> still doesn't work.
>>
>> Interestingly, just adding irq_set_irq_type(virq, type); to the top of
>> that block (leaving the rest of the code) also works.
> 
> Interesting. By saving the trigger type during the mapping, we defer
> setting the interrupt type to when the interrupt is requested. So this
> would imply that the interrupt is not being setup as expected when its
> requested.
>  
>>> What is odd, is that the above sequence is only executed if a irq
>>> mapping exists and so really, AFAICT this should not happen. Ie. the irq
>>> descriptor should have been allocated for the mapping to exist. We
>>> should probably warn if this happens.
>>>
>>> Without reverting the above, can you add a print to show the
>>> domain->name, hwirq and virq information if !irq_data? That will confirm
>>> the domain for us.
>>
>> So I put some printk info in (in either case since I'm never seeing
>> the !irq_data case happen):
>>
>> [    1.514217] JDB: virq: 93  hwirq: 74 domain name: msmgpio
>> [    1.838342] JDB: virq: 25  hwirq: 6 domain name: msmgpio
>>
>> Which is odd, looking at:
>>
>> shell@flo:/ $ cat /proc/interrupts
>>            CPU0       CPU1       CPU2       CPU3
>>  16:       1159       1138       1332       1574     GIC-0  18 Edge
>>   gp_timer
>>  25:          0          0          0          0   msmgpio   6 Edge
>>   ekth3500
>> 111:          6          0          0          0     GIC-0  51 Edge
>>   qcom_rpm_ack
>> 112:          0          0          0          0     GIC-0  53 Edge
>>   qcom_rpm_err
>> 113:          0          0          0          0     GIC-0  54 Edge
>>   qcom_rpm_wakeup
>> 114:         48          0          0          0     GIC-0 132 Edge
>>   msm_otg, ci_hdrc_msm
>> 115:        796          0          0          0     GIC-0 130 Level     bam_dma
>> 116:          0          0          0          0     GIC-0 128 Level     bam_dma
>> 117:          0          0          0          0     GIC-0 127 Level     bam_dma
>> 118:       2627          0          0          0     GIC-0 136 Level
>>   mmci-pl18x (cmd)
>> 119:         54          0          0          0     GIC-0 226 Level     i2c_qup
>> 120:         21          0          0          0     GIC-0 183 Level     i2c_qup
>> 122:          0          0          0          0     GIC-0 189 Level     i2c_qup
>> 123:        202          0          0          0     GIC-0 190 Level
>>   msm_serial0
>> 124:          0          0          0          0     GIC-0  70 Edge      smsm
>> 125:          0          0          0          0     GIC-0 121 Edge      smsm
>> 126:          0          0          0          0     GIC-0 236 Edge      smsm
>> 127:          0          0          0          0     GIC-0 169 Edge      smsm
>> 131:          0          0          0          0    pm8xxx 195 Edge
>>   Volume Up
>> 165:          0          0          0          0    pm8xxx 229 Edge
>>   Volume Down
>> 184:          0          0          0          0    pm8xxx  39 Edge
>>   pm8xxx_rtc_alarm
>> 185:          0          0          0          0    pm8xxx  50 Edge
>>   pmic8xxx_pwrkey_release
>> 186:          0          0          0          0    pm8xxx  51 Edge
>>   pmic8xxx_pwrkey_press
>> IPI0:          0          1          1          1  CPU wakeup interrupts
>> IPI1:          0          0          0          0  Timer broadcast interrupts
>> IPI2:        944        539       1015        529  Rescheduling interrupts
>> IPI3:          1          4          6          4  Function call interrupts
>> IPI4:          0          0          0          0  CPU stop interrupts
>> IPI5:          0          0          0          0  IRQ work interrupts
>> IPI6:          0          0          0          0  completion interrupts
>> Err:          0
>>
>> Since 25 maps to the ekth3500 (touch panel, which is still working
>> fine), but 93/74 doesn't seem to map to anything, and the problematic
>> irqs are the volume keys 195/229 and power keys 50/51.
> 
> So looking at the DT source, I believe that hwirq 74 (virq 93) is the
> problem. This is the parent interrupt from the pm8xxx to the apq8064
> if it is not requested then the type is not set. It seems that for
> parent interrupts these are not typically requested, but enabled when
> an irqchip is chained.
> 
> To confirm and for testing purposes I am curious if this works ...
> 
> 	if (irq_get_trigger_type(virq) == IRQ_TYPE_NONE) {
> 		irq_data = irq_get_irq_data(virq);
> 		if (!irq_data)
> 			return 0;
> 
> -		irqd_set_trigger_type(irq_data, type);
> + 		if (hwirq == 74)
> +			irq_set_irq_type(virq, type);
> +		else
> +			irqd_set_trigger_type(irq_data, type);
> 		return virq;
> 	}
> 
> If that works, then does the following also work (without the above) ...
> 
> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> index b4c1bc7c9ca2..e111b72e3162 100644
> --- a/kernel/irq/chip.c
> +++ b/kernel/irq/chip.c
> @@ -824,6 +824,7 @@ __irq_do_set_handler(struct irq_desc *desc, irq_flow_handler_t handle,
>                 irq_settings_set_norequest(desc);
>                 irq_settings_set_nothread(desc);
>                 desc->action = &chained_action;
> +               __irq_set_trigger(desc, irqd_get_trigger_type(&desc->irq_data));
>                 irq_startup(desc, true);
>         }
>  }
> 
> It looks like there is a path for parent interrupts where the type
> is not getting set. If the above works then we can discuss with Thomas
> and Marc on the correct fix.

This definitely looks like an something that is worth a patch anyway, as
I otherwise don't see how we configure cascaded interrupts with the new
deferred scheme.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons

[Linus Walleij]

On Tue, Aug 9, 2016 at 3:20 PM, Jon Hunter <jonathanh@nvidia.com> wrote:

> If that works, then does the following also work (without the above) ...
>
> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> index b4c1bc7c9ca2..e111b72e3162 100644
> --- a/kernel/irq/chip.c
> +++ b/kernel/irq/chip.c
> @@ -824,6 +824,7 @@ __irq_do_set_handler(struct irq_desc *desc, irq_flow_handler_t handle,
>                 irq_settings_set_norequest(desc);
>                 irq_settings_set_nothread(desc);
>                 desc->action = &chained_action;
> +               __irq_set_trigger(desc, irqd_get_trigger_type(&desc->irq_data));
>                 irq_startup(desc, true);
>         }
>  }
>
> It looks like there is a path for parent interrupts where the type
> is not getting set. If the above works then we can discuss with Thomas
> and Marc on the correct fix.

I tried this on my problematic platform and then this happens:

preparing networking...
[    2.628246] ------------[ cut here ]------------
[    2.628303] WARNING: CPU: 0 PID: 92 at ../kernel/irq/chip.c:26
bad_chained_irq+0x44/0x4c
[    2.631939] Chained irq 109 should not call an action
[    2.640008] Modules linked in:
[    2.647909] CPU: 0 PID: 92 Comm: ip Not tainted
4.8.0-rc1-00011-ga21e27b4cb66 #194
[    2.647996] Hardware name: Generic DT based system
[    2.655486] [<c030f8c8>] (unwind_backtrace) from [<c030c714>]
(show_stack+0x10/0x14)
[    2.660254] [<c030c714>] (show_stack) from [<c05df420>]
(dump_stack+0x78/0x8c)
[    2.668147] [<c05df420>] (dump_stack) from [<c031cef4>] (__warn+0xec/0x104)
[    2.675173] [<c031cef4>] (__warn) from [<c031cf44>]
(warn_slowpath_fmt+0x38/0x48)
[    2.682033] [<c031cf44>] (warn_slowpath_fmt) from [<c0369160>]
(bad_chained_irq+0x44/0x4c)
[    2.689687] [<c0369160>] (bad_chained_irq) from [<c0365e28>]
(__handle_irq_event_percpu+0x5c/0x290)
[    2.697836] [<c0365e28>] (__handle_irq_event_percpu) from
[<c0366078>] (handle_irq_event_percpu+0x1c/0x58)
[    2.706778] [<c0366078>] (handle_irq_event_percpu) from
[<c03660ec>] (handle_irq_event+0x38/0x5c)
[    2.716498] [<c03660ec>] (handle_irq_event) from [<c03693f0>]
(handle_level_irq+0xc4/0x150)
[    2.725438] [<c03693f0>] (handle_level_irq) from [<c036542c>]
(generic_handle_irq+0x24/0x34)
[    2.733602] [<c036542c>] (generic_handle_irq) from [<c06127d4>]
(msm_gpio_irq_handler+0xc8/0x150)
[    2.742280] [<c06127d4>] (msm_gpio_irq_handler) from [<c036542c>]
(generic_handle_irq+0x24/0x34)
[    2.751048] [<c036542c>] (generic_handle_irq) from [<c0365720>]
(__handle_domain_irq+0x7c/0xec)
[    2.759901] [<c0365720>] (__handle_domain_irq) from [<c0301464>]
(gic_handle_irq+0x48/0x8c)
[    2.768323] [<c0301464>] (gic_handle_irq) from [<c08b8e4c>]
(__irq_svc+0x6c/0xa8)
[    2.776644] Exception stack(0xdeca1d48 to 0xdeca1d90)
[    2.784284] 1d40:                   deca1dc0 00000000 00000000
deca0018 ffff8bd6 deca1dc0
[    2.789324] 1d60: 00000000 c0378ad0 60070013 00000000 00000000
001f3df8 c108fa04 deca1d98
[    2.797481] 1d80: c08b77fc c0377930 60070013 ffffffff
[    2.805651] [<c08b8e4c>] (__irq_svc) from [<c0377930>]
(init_timer_key+0x28/0x104)
[    2.810679] [<c0377930>] (init_timer_key) from [<c08b77fc>]
(schedule_timeout+0x48/0x410)
[    2.818145] [<c08b77fc>] (schedule_timeout) from [<c0378ad0>]
(msleep+0x2c/0x38)
[    2.826399] [<c0378ad0>] (msleep) from [<c06c2980>]
(smsc911x_open+0x268/0x50c)
[    2.833859] [<c06c2980>] (smsc911x_open) from [<c07c1994>]
(__dev_open+0xa8/0x10c)
[    2.840887] [<c07c1994>] (__dev_open) from [<c07c1c1c>]
(__dev_change_flags+0x94/0x144)
[    2.848525] [<c07c1c1c>] (__dev_change_flags) from [<c07c1ce4>]
(dev_change_flags+0x18/0x48)
[    2.856428] [<c07c1ce4>] (dev_change_flags) from [<c0823048>]
(devinet_ioctl+0x6b0/0x768)
[    2.865120] [<c0823048>] (devinet_ioctl) from [<c07a4ac4>]
(sock_ioctl+0x1f4/0x2c8)
[    2.873186] [<c07a4ac4>] (sock_ioctl) from [<c0432678>]
(do_vfs_ioctl+0x9c/0x910)
[    2.880645] [<c0432678>] (do_vfs_ioctl) from [<c0432f20>]
(SyS_ioctl+0x34/0x5c)
[    2.888290] [<c0432f20>] (SyS_ioctl) from [<c0308480>]
(ret_fast_syscall+0x0/0x3c)
[    2.895395] ---[ end trace a53e1e63b7bdfc4a ]---
[    2.903917] random: fast init done
[    3.036378] random: crng init done
[    3.883906] irq 109: nobody cared (try booting with the "irqpoll" option)
[    3.883940] CPU: 0 PID: 92 Comm: ip Tainted: G        W
4.8.0-rc1-00011-ga21e27b4cb66 #194
[    3.889673] Hardware name: Generic DT based system
[    3.898538] [<c030f8c8>] (unwind_backtrace) from [<c030c714>]
(show_stack+0x10/0x14)
[    3.903137] [<c030c714>] (show_stack) from [<c05df420>]
(dump_stack+0x78/0x8c)
[    3.911034] [<c05df420>] (dump_stack) from [<c0368804>]
(__report_bad_irq+0x28/0xcc)
[    3.918065] [<c0368804>] (__report_bad_irq) from [<c0368c18>]
(note_interrupt+0x298/0x2e8)
[    3.925971] [<c0368c18>] (note_interrupt) from [<c03660a8>]
(handle_irq_event_percpu+0x4c/0x58)
[    3.934040] [<c03660a8>] (handle_irq_event_percpu) from
[<c03660ec>] (handle_irq_event+0x38/0x5c)
[    3.942634] [<c03660ec>] (handle_irq_event) from [<c03693f0>]
(handle_level_irq+0xc4/0x150)
[    3.951660] [<c03693f0>] (handle_level_irq) from [<c036542c>]
(generic_handle_irq+0x24/0x34)
[    3.959821] [<c036542c>] (generic_handle_irq) from [<c06127d4>]
(msm_gpio_irq_handler+0xc8/0x150)
[    3.968502] [<c06127d4>] (msm_gpio_irq_handler) from [<c036542c>]
(generic_handle_irq+0x24/0x34)
[    3.977269] [<c036542c>] (generic_handle_irq) from [<c0365720>]
(__handle_domain_irq+0x7c/0xec)
[    3.986122] [<c0365720>] (__handle_domain_irq) from [<c0301464>]
(gic_handle_irq+0x48/0x8c)
[    3.994541] [<c0301464>] (gic_handle_irq) from [<c08b8e4c>]
(__irq_svc+0x6c/0xa8)
[    4.002865] Exception stack(0xdeca1c60 to 0xdeca1ca8)
[    4.010507] 1c60: 00000000 c0abce68 c109f9c0 00000000 c109f9c0
00000000 deca0000 00000000
[    4.015546] 1c80: 00000282 deca1d48 c0210800 001f3df8 e080400c
deca1cb0 c0322330 c0322340
[    4.023701] 1ca0: 20070113 ffffffff
[    4.031868] [<c08b8e4c>] (__irq_svc) from [<c0322340>]
(__do_softirq+0x9c/0x388)
[    4.035166] [<c0322340>] (__do_softirq) from [<c03228f0>]
(irq_exit+0xc0/0xfc)
[    4.042805] [<c03228f0>] (irq_exit) from [<c0365724>]
(__handle_domain_irq+0x80/0xec)
[    4.049835] [<c0365724>] (__handle_domain_irq) from [<c0301464>]
(gic_handle_irq+0x48/0x8c)
[    4.057735] [<c0301464>] (gic_handle_irq) from [<c08b8e4c>]
(__irq_svc+0x6c/0xa8)
[    4.065887] Exception stack(0xdeca1d48 to 0xdeca1d90)
[    4.073528] 1d40:                   deca1dc0 00000000 00000000
deca0018 ffff8bd6 deca1dc0
[    4.078566] 1d60: 00000000 c0378ad0 60070013 00000000 00000000
001f3df8 c108fa04 deca1d98
[    4.086723] 1d80: c08b77fc c0377930 60070013 ffffffff
[    4.094889] [<c08b8e4c>] (__irq_svc) from [<c0377930>]
(init_timer_key+0x28/0x104)
[    4.099921] [<c0377930>] (init_timer_key) from [<c08b77fc>]
(schedule_timeout+0x48/0x410)
[    4.107389] [<c08b77fc>] (schedule_timeout) from [<c0378ad0>]
(msleep+0x2c/0x38)
[    4.115640] [<c0378ad0>] (msleep) from [<c06c2980>]
(smsc911x_open+0x268/0x50c)
[    4.123100] [<c06c2980>] (smsc911x_open) from [<c07c1994>]
(__dev_open+0xa8/0x10c)
[    4.130128] [<c07c1994>] (__dev_open) from [<c07c1c1c>]
(__dev_change_flags+0x94/0x144)
[    4.137769] [<c07c1c1c>] (__dev_change_flags) from [<c07c1ce4>]
(dev_change_flags+0x18/0x48)
[    4.145670] [<c07c1ce4>] (dev_change_flags) from [<c0823048>]
(devinet_ioctl+0x6b0/0x768)
[    4.154357] [<c0823048>] (devinet_ioctl) from [<c07a4ac4>]
(sock_ioctl+0x1f4/0x2c8)
[    4.162425] [<c07a4ac4>] (sock_ioctl) from [<c0432678>]
(do_vfs_ioctl+0x9c/0x910)
[    4.169887] [<c0432678>] (do_vfs_ioctl) from [<c0432f20>]
(SyS_ioctl+0x34/0x5c)
[    4.177529] [<c0432f20>] (SyS_ioctl) from [<c0308480>]
(ret_fast_syscall+0x0/0x3c)
[    4.184635] handlers:
[    4.192273] [<c036911c>] bad_chained_irq
[    4.198255] Disabling IRQ #109
(...)
[   34.170316] smsc911x 1b800000.ethernet-ebi2 eth0: ISR failed
signaling test (IRQ 208)


Yours,
Linus Walleij

Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons

[Marc Zyngier]

Hi Linus,

On 10/08/16 00:03, Linus Walleij wrote:
> On Tue, Aug 9, 2016 at 3:20 PM, Jon Hunter <jonathanh@nvidia.com> wrote:
> 
>> If that works, then does the following also work (without the above) ...
>>
>> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
>> index b4c1bc7c9ca2..e111b72e3162 100644
>> --- a/kernel/irq/chip.c
>> +++ b/kernel/irq/chip.c
>> @@ -824,6 +824,7 @@ __irq_do_set_handler(struct irq_desc *desc, irq_flow_handler_t handle,
>>                 irq_settings_set_norequest(desc);
>>                 irq_settings_set_nothread(desc);
>>                 desc->action = &chained_action;
>> +               __irq_set_trigger(desc, irqd_get_trigger_type(&desc->irq_data));
>>                 irq_startup(desc, true);
>>         }
>>  }
>>
>> It looks like there is a path for parent interrupts where the type
>> is not getting set. If the above works then we can discuss with Thomas
>> and Marc on the correct fix.
> 
> I tried this on my problematic platform and then this happens:
> 
> preparing networking...
> [    2.628246] ------------[ cut here ]------------
> [    2.628303] WARNING: CPU: 0 PID: 92 at ../kernel/irq/chip.c:26
> bad_chained_irq+0x44/0x4c
> [    2.631939] Chained irq 109 should not call an action
> [    2.640008] Modules linked in:
> [    2.647909] CPU: 0 PID: 92 Comm: ip Not tainted
> 4.8.0-rc1-00011-ga21e27b4cb66 #194
> [    2.647996] Hardware name: Generic DT based system
> [    2.655486] [<c030f8c8>] (unwind_backtrace) from [<c030c714>]
> (show_stack+0x10/0x14)
> [    2.660254] [<c030c714>] (show_stack) from [<c05df420>]
> (dump_stack+0x78/0x8c)
> [    2.668147] [<c05df420>] (dump_stack) from [<c031cef4>] (__warn+0xec/0x104)
> [    2.675173] [<c031cef4>] (__warn) from [<c031cf44>]
> (warn_slowpath_fmt+0x38/0x48)
> [    2.682033] [<c031cf44>] (warn_slowpath_fmt) from [<c0369160>]
> (bad_chained_irq+0x44/0x4c)
> [    2.689687] [<c0369160>] (bad_chained_irq) from [<c0365e28>]
> (__handle_irq_event_percpu+0x5c/0x290)
> [    2.697836] [<c0365e28>] (__handle_irq_event_percpu) from
> [<c0366078>] (handle_irq_event_percpu+0x1c/0x58)
> [    2.706778] [<c0366078>] (handle_irq_event_percpu) from
> [<c03660ec>] (handle_irq_event+0x38/0x5c)
> [    2.716498] [<c03660ec>] (handle_irq_event) from [<c03693f0>]
> (handle_level_irq+0xc4/0x150)
> [    2.725438] [<c03693f0>] (handle_level_irq) from [<c036542c>]
> (generic_handle_irq+0x24/0x34)
> [    2.733602] [<c036542c>] (generic_handle_irq) from [<c06127d4>]
> (msm_gpio_irq_handler+0xc8/0x150)
> [    2.742280] [<c06127d4>] (msm_gpio_irq_handler) from [<c036542c>]
> (generic_handle_irq+0x24/0x34)
> [    2.751048] [<c036542c>] (generic_handle_irq) from [<c0365720>]
> (__handle_domain_irq+0x7c/0xec)
> [    2.759901] [<c0365720>] (__handle_domain_irq) from [<c0301464>]
> (gic_handle_irq+0x48/0x8c)
> [    2.768323] [<c0301464>] (gic_handle_irq) from [<c08b8e4c>]
> (__irq_svc+0x6c/0xa8)
> [    2.776644] Exception stack(0xdeca1d48 to 0xdeca1d90)
> [    2.784284] 1d40:                   deca1dc0 00000000 00000000
> deca0018 ffff8bd6 deca1dc0
> [    2.789324] 1d60: 00000000 c0378ad0 60070013 00000000 00000000
> 001f3df8 c108fa04 deca1d98
> [    2.797481] 1d80: c08b77fc c0377930 60070013 ffffffff
> [    2.805651] [<c08b8e4c>] (__irq_svc) from [<c0377930>]
> (init_timer_key+0x28/0x104)
> [    2.810679] [<c0377930>] (init_timer_key) from [<c08b77fc>]
> (schedule_timeout+0x48/0x410)
> [    2.818145] [<c08b77fc>] (schedule_timeout) from [<c0378ad0>]
> (msleep+0x2c/0x38)
> [    2.826399] [<c0378ad0>] (msleep) from [<c06c2980>]
> (smsc911x_open+0x268/0x50c)
> [    2.833859] [<c06c2980>] (smsc911x_open) from [<c07c1994>]
> (__dev_open+0xa8/0x10c)
> [    2.840887] [<c07c1994>] (__dev_open) from [<c07c1c1c>]
> (__dev_change_flags+0x94/0x144)
> [    2.848525] [<c07c1c1c>] (__dev_change_flags) from [<c07c1ce4>]
> (dev_change_flags+0x18/0x48)
> [    2.856428] [<c07c1ce4>] (dev_change_flags) from [<c0823048>]
> (devinet_ioctl+0x6b0/0x768)
> [    2.865120] [<c0823048>] (devinet_ioctl) from [<c07a4ac4>]
> (sock_ioctl+0x1f4/0x2c8)
> [    2.873186] [<c07a4ac4>] (sock_ioctl) from [<c0432678>]
> (do_vfs_ioctl+0x9c/0x910)
> [    2.880645] [<c0432678>] (do_vfs_ioctl) from [<c0432f20>]
> (SyS_ioctl+0x34/0x5c)
> [    2.888290] [<c0432f20>] (SyS_ioctl) from [<c0308480>]
> (ret_fast_syscall+0x0/0x3c)
> [    2.895395] ---[ end trace a53e1e63b7bdfc4a ]---
> [    2.903917] random: fast init done
> [    3.036378] random: crng init done
> [    3.883906] irq 109: nobody cared (try booting with the "irqpoll" option)
> [    3.883940] CPU: 0 PID: 92 Comm: ip Tainted: G        W
> 4.8.0-rc1-00011-ga21e27b4cb66 #194
> [    3.889673] Hardware name: Generic DT based system
> [    3.898538] [<c030f8c8>] (unwind_backtrace) from [<c030c714>]
> (show_stack+0x10/0x14)
> [    3.903137] [<c030c714>] (show_stack) from [<c05df420>]
> (dump_stack+0x78/0x8c)
> [    3.911034] [<c05df420>] (dump_stack) from [<c0368804>]
> (__report_bad_irq+0x28/0xcc)
> [    3.918065] [<c0368804>] (__report_bad_irq) from [<c0368c18>]
> (note_interrupt+0x298/0x2e8)
> [    3.925971] [<c0368c18>] (note_interrupt) from [<c03660a8>]
> (handle_irq_event_percpu+0x4c/0x58)
> [    3.934040] [<c03660a8>] (handle_irq_event_percpu) from
> [<c03660ec>] (handle_irq_event+0x38/0x5c)
> [    3.942634] [<c03660ec>] (handle_irq_event) from [<c03693f0>]
> (handle_level_irq+0xc4/0x150)
> [    3.951660] [<c03693f0>] (handle_level_irq) from [<c036542c>]
> (generic_handle_irq+0x24/0x34)
> [    3.959821] [<c036542c>] (generic_handle_irq) from [<c06127d4>]
> (msm_gpio_irq_handler+0xc8/0x150)
> [    3.968502] [<c06127d4>] (msm_gpio_irq_handler) from [<c036542c>]
> (generic_handle_irq+0x24/0x34)
> [    3.977269] [<c036542c>] (generic_handle_irq) from [<c0365720>]
> (__handle_domain_irq+0x7c/0xec)
> [    3.986122] [<c0365720>] (__handle_domain_irq) from [<c0301464>]
> (gic_handle_irq+0x48/0x8c)
> [    3.994541] [<c0301464>] (gic_handle_irq) from [<c08b8e4c>]
> (__irq_svc+0x6c/0xa8)
> [    4.002865] Exception stack(0xdeca1c60 to 0xdeca1ca8)
> [    4.010507] 1c60: 00000000 c0abce68 c109f9c0 00000000 c109f9c0
> 00000000 deca0000 00000000
> [    4.015546] 1c80: 00000282 deca1d48 c0210800 001f3df8 e080400c
> deca1cb0 c0322330 c0322340
> [    4.023701] 1ca0: 20070113 ffffffff
> [    4.031868] [<c08b8e4c>] (__irq_svc) from [<c0322340>]
> (__do_softirq+0x9c/0x388)
> [    4.035166] [<c0322340>] (__do_softirq) from [<c03228f0>]
> (irq_exit+0xc0/0xfc)
> [    4.042805] [<c03228f0>] (irq_exit) from [<c0365724>]
> (__handle_domain_irq+0x80/0xec)
> [    4.049835] [<c0365724>] (__handle_domain_irq) from [<c0301464>]
> (gic_handle_irq+0x48/0x8c)
> [    4.057735] [<c0301464>] (gic_handle_irq) from [<c08b8e4c>]
> (__irq_svc+0x6c/0xa8)
> [    4.065887] Exception stack(0xdeca1d48 to 0xdeca1d90)
> [    4.073528] 1d40:                   deca1dc0 00000000 00000000
> deca0018 ffff8bd6 deca1dc0
> [    4.078566] 1d60: 00000000 c0378ad0 60070013 00000000 00000000
> 001f3df8 c108fa04 deca1d98
> [    4.086723] 1d80: c08b77fc c0377930 60070013 ffffffff
> [    4.094889] [<c08b8e4c>] (__irq_svc) from [<c0377930>]
> (init_timer_key+0x28/0x104)
> [    4.099921] [<c0377930>] (init_timer_key) from [<c08b77fc>]
> (schedule_timeout+0x48/0x410)
> [    4.107389] [<c08b77fc>] (schedule_timeout) from [<c0378ad0>]
> (msleep+0x2c/0x38)
> [    4.115640] [<c0378ad0>] (msleep) from [<c06c2980>]
> (smsc911x_open+0x268/0x50c)
> [    4.123100] [<c06c2980>] (smsc911x_open) from [<c07c1994>]
> (__dev_open+0xa8/0x10c)
> [    4.130128] [<c07c1994>] (__dev_open) from [<c07c1c1c>]
> (__dev_change_flags+0x94/0x144)
> [    4.137769] [<c07c1c1c>] (__dev_change_flags) from [<c07c1ce4>]
> (dev_change_flags+0x18/0x48)
> [    4.145670] [<c07c1ce4>] (dev_change_flags) from [<c0823048>]
> (devinet_ioctl+0x6b0/0x768)
> [    4.154357] [<c0823048>] (devinet_ioctl) from [<c07a4ac4>]
> (sock_ioctl+0x1f4/0x2c8)
> [    4.162425] [<c07a4ac4>] (sock_ioctl) from [<c0432678>]
> (do_vfs_ioctl+0x9c/0x910)
> [    4.169887] [<c0432678>] (do_vfs_ioctl) from [<c0432f20>]
> (SyS_ioctl+0x34/0x5c)
> [    4.177529] [<c0432f20>] (SyS_ioctl) from [<c0308480>]
> (ret_fast_syscall+0x0/0x3c)
> [    4.184635] handlers:
> [    4.192273] [<c036911c>] bad_chained_irq
> [    4.198255] Disabling IRQ #109
> (...)
> [   34.170316] smsc911x 1b800000.ethernet-ebi2 eth0: ISR failed
> signaling test (IRQ 208)

Is this platform related to the Dragonboard 410C? I've got one from
Sudeep, and it seems to work fine (though I've spotted a couple of
gotchas in the DT).

Do you see this symptom on all chained interrupts? Or just this
particular one?

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons

[Jon Hunter]

Hi Marc, Linus,

On 10/08/16 10:41, Marc Zyngier wrote:
> Hi Linus,
> 
> On 10/08/16 00:03, Linus Walleij wrote:
>> On Tue, Aug 9, 2016 at 3:20 PM, Jon Hunter <jonathanh@nvidia.com> wrote:
>>
>>> If that works, then does the following also work (without the above) ...
>>>
>>> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
>>> index b4c1bc7c9ca2..e111b72e3162 100644
>>> --- a/kernel/irq/chip.c
>>> +++ b/kernel/irq/chip.c
>>> @@ -824,6 +824,7 @@ __irq_do_set_handler(struct irq_desc *desc, irq_flow_handler_t handle,
>>>                 irq_settings_set_norequest(desc);
>>>                 irq_settings_set_nothread(desc);
>>>                 desc->action = &chained_action;
>>> +               __irq_set_trigger(desc, irqd_get_trigger_type(&desc->irq_data));
>>>                 irq_startup(desc, true);
>>>         }
>>>  }
>>>
>>> It looks like there is a path for parent interrupts where the type
>>> is not getting set. If the above works then we can discuss with Thomas
>>> and Marc on the correct fix.
>>
>> I tried this on my problematic platform and then this happens:
>>
>> preparing networking...
>> [    2.628246] ------------[ cut here ]------------
>> [    2.628303] WARNING: CPU: 0 PID: 92 at ../kernel/irq/chip.c:26
>> bad_chained_irq+0x44/0x4c
>> [    2.631939] Chained irq 109 should not call an action
>> [    2.640008] Modules linked in:
>> [    2.647909] CPU: 0 PID: 92 Comm: ip Not tainted
>> 4.8.0-rc1-00011-ga21e27b4cb66 #194
>> [    2.647996] Hardware name: Generic DT based system
>> [    2.655486] [<c030f8c8>] (unwind_backtrace) from [<c030c714>]
>> (show_stack+0x10/0x14)
>> [    2.660254] [<c030c714>] (show_stack) from [<c05df420>]
>> (dump_stack+0x78/0x8c)
>> [    2.668147] [<c05df420>] (dump_stack) from [<c031cef4>] (__warn+0xec/0x104)
>> [    2.675173] [<c031cef4>] (__warn) from [<c031cf44>]
>> (warn_slowpath_fmt+0x38/0x48)
>> [    2.682033] [<c031cf44>] (warn_slowpath_fmt) from [<c0369160>]
>> (bad_chained_irq+0x44/0x4c)
>> [    2.689687] [<c0369160>] (bad_chained_irq) from [<c0365e28>]
>> (__handle_irq_event_percpu+0x5c/0x290)
>> [    2.697836] [<c0365e28>] (__handle_irq_event_percpu) from
>> [<c0366078>] (handle_irq_event_percpu+0x1c/0x58)
>> [    2.706778] [<c0366078>] (handle_irq_event_percpu) from
>> [<c03660ec>] (handle_irq_event+0x38/0x5c)
>> [    2.716498] [<c03660ec>] (handle_irq_event) from [<c03693f0>]
>> (handle_level_irq+0xc4/0x150)
>> [    2.725438] [<c03693f0>] (handle_level_irq) from [<c036542c>]
>> (generic_handle_irq+0x24/0x34)
>> [    2.733602] [<c036542c>] (generic_handle_irq) from [<c06127d4>]
>> (msm_gpio_irq_handler+0xc8/0x150)
>> [    2.742280] [<c06127d4>] (msm_gpio_irq_handler) from [<c036542c>]
>> (generic_handle_irq+0x24/0x34)
>> [    2.751048] [<c036542c>] (generic_handle_irq) from [<c0365720>]
>> (__handle_domain_irq+0x7c/0xec)
>> [    2.759901] [<c0365720>] (__handle_domain_irq) from [<c0301464>]
>> (gic_handle_irq+0x48/0x8c)
>> [    2.768323] [<c0301464>] (gic_handle_irq) from [<c08b8e4c>]
>> (__irq_svc+0x6c/0xa8)
>> [    2.776644] Exception stack(0xdeca1d48 to 0xdeca1d90)
>> [    2.784284] 1d40:                   deca1dc0 00000000 00000000
>> deca0018 ffff8bd6 deca1dc0
>> [    2.789324] 1d60: 00000000 c0378ad0 60070013 00000000 00000000
>> 001f3df8 c108fa04 deca1d98
>> [    2.797481] 1d80: c08b77fc c0377930 60070013 ffffffff
>> [    2.805651] [<c08b8e4c>] (__irq_svc) from [<c0377930>]
>> (init_timer_key+0x28/0x104)
>> [    2.810679] [<c0377930>] (init_timer_key) from [<c08b77fc>]
>> (schedule_timeout+0x48/0x410)
>> [    2.818145] [<c08b77fc>] (schedule_timeout) from [<c0378ad0>]
>> (msleep+0x2c/0x38)
>> [    2.826399] [<c0378ad0>] (msleep) from [<c06c2980>]
>> (smsc911x_open+0x268/0x50c)
>> [    2.833859] [<c06c2980>] (smsc911x_open) from [<c07c1994>]
>> (__dev_open+0xa8/0x10c)
>> [    2.840887] [<c07c1994>] (__dev_open) from [<c07c1c1c>]
>> (__dev_change_flags+0x94/0x144)
>> [    2.848525] [<c07c1c1c>] (__dev_change_flags) from [<c07c1ce4>]
>> (dev_change_flags+0x18/0x48)
>> [    2.856428] [<c07c1ce4>] (dev_change_flags) from [<c0823048>]
>> (devinet_ioctl+0x6b0/0x768)
>> [    2.865120] [<c0823048>] (devinet_ioctl) from [<c07a4ac4>]
>> (sock_ioctl+0x1f4/0x2c8)
>> [    2.873186] [<c07a4ac4>] (sock_ioctl) from [<c0432678>]
>> (do_vfs_ioctl+0x9c/0x910)
>> [    2.880645] [<c0432678>] (do_vfs_ioctl) from [<c0432f20>]
>> (SyS_ioctl+0x34/0x5c)
>> [    2.888290] [<c0432f20>] (SyS_ioctl) from [<c0308480>]
>> (ret_fast_syscall+0x0/0x3c)
>> [    2.895395] ---[ end trace a53e1e63b7bdfc4a ]---
>> [    2.903917] random: fast init done
>> [    3.036378] random: crng init done
>> [    3.883906] irq 109: nobody cared (try booting with the "irqpoll" option)
>> [    3.883940] CPU: 0 PID: 92 Comm: ip Tainted: G        W
>> 4.8.0-rc1-00011-ga21e27b4cb66 #194
>> [    3.889673] Hardware name: Generic DT based system
>> [    3.898538] [<c030f8c8>] (unwind_backtrace) from [<c030c714>]
>> (show_stack+0x10/0x14)
>> [    3.903137] [<c030c714>] (show_stack) from [<c05df420>]
>> (dump_stack+0x78/0x8c)
>> [    3.911034] [<c05df420>] (dump_stack) from [<c0368804>]
>> (__report_bad_irq+0x28/0xcc)
>> [    3.918065] [<c0368804>] (__report_bad_irq) from [<c0368c18>]
>> (note_interrupt+0x298/0x2e8)
>> [    3.925971] [<c0368c18>] (note_interrupt) from [<c03660a8>]
>> (handle_irq_event_percpu+0x4c/0x58)
>> [    3.934040] [<c03660a8>] (handle_irq_event_percpu) from
>> [<c03660ec>] (handle_irq_event+0x38/0x5c)
>> [    3.942634] [<c03660ec>] (handle_irq_event) from [<c03693f0>]
>> (handle_level_irq+0xc4/0x150)
>> [    3.951660] [<c03693f0>] (handle_level_irq) from [<c036542c>]
>> (generic_handle_irq+0x24/0x34)
>> [    3.959821] [<c036542c>] (generic_handle_irq) from [<c06127d4>]
>> (msm_gpio_irq_handler+0xc8/0x150)
>> [    3.968502] [<c06127d4>] (msm_gpio_irq_handler) from [<c036542c>]
>> (generic_handle_irq+0x24/0x34)
>> [    3.977269] [<c036542c>] (generic_handle_irq) from [<c0365720>]
>> (__handle_domain_irq+0x7c/0xec)
>> [    3.986122] [<c0365720>] (__handle_domain_irq) from [<c0301464>]
>> (gic_handle_irq+0x48/0x8c)
>> [    3.994541] [<c0301464>] (gic_handle_irq) from [<c08b8e4c>]
>> (__irq_svc+0x6c/0xa8)
>> [    4.002865] Exception stack(0xdeca1c60 to 0xdeca1ca8)
>> [    4.010507] 1c60: 00000000 c0abce68 c109f9c0 00000000 c109f9c0
>> 00000000 deca0000 00000000
>> [    4.015546] 1c80: 00000282 deca1d48 c0210800 001f3df8 e080400c
>> deca1cb0 c0322330 c0322340
>> [    4.023701] 1ca0: 20070113 ffffffff
>> [    4.031868] [<c08b8e4c>] (__irq_svc) from [<c0322340>]
>> (__do_softirq+0x9c/0x388)
>> [    4.035166] [<c0322340>] (__do_softirq) from [<c03228f0>]
>> (irq_exit+0xc0/0xfc)
>> [    4.042805] [<c03228f0>] (irq_exit) from [<c0365724>]
>> (__handle_domain_irq+0x80/0xec)
>> [    4.049835] [<c0365724>] (__handle_domain_irq) from [<c0301464>]
>> (gic_handle_irq+0x48/0x8c)
>> [    4.057735] [<c0301464>] (gic_handle_irq) from [<c08b8e4c>]
>> (__irq_svc+0x6c/0xa8)
>> [    4.065887] Exception stack(0xdeca1d48 to 0xdeca1d90)
>> [    4.073528] 1d40:                   deca1dc0 00000000 00000000
>> deca0018 ffff8bd6 deca1dc0
>> [    4.078566] 1d60: 00000000 c0378ad0 60070013 00000000 00000000
>> 001f3df8 c108fa04 deca1d98
>> [    4.086723] 1d80: c08b77fc c0377930 60070013 ffffffff
>> [    4.094889] [<c08b8e4c>] (__irq_svc) from [<c0377930>]
>> (init_timer_key+0x28/0x104)
>> [    4.099921] [<c0377930>] (init_timer_key) from [<c08b77fc>]
>> (schedule_timeout+0x48/0x410)
>> [    4.107389] [<c08b77fc>] (schedule_timeout) from [<c0378ad0>]
>> (msleep+0x2c/0x38)
>> [    4.115640] [<c0378ad0>] (msleep) from [<c06c2980>]
>> (smsc911x_open+0x268/0x50c)
>> [    4.123100] [<c06c2980>] (smsc911x_open) from [<c07c1994>]
>> (__dev_open+0xa8/0x10c)
>> [    4.130128] [<c07c1994>] (__dev_open) from [<c07c1c1c>]
>> (__dev_change_flags+0x94/0x144)
>> [    4.137769] [<c07c1c1c>] (__dev_change_flags) from [<c07c1ce4>]
>> (dev_change_flags+0x18/0x48)
>> [    4.145670] [<c07c1ce4>] (dev_change_flags) from [<c0823048>]
>> (devinet_ioctl+0x6b0/0x768)
>> [    4.154357] [<c0823048>] (devinet_ioctl) from [<c07a4ac4>]
>> (sock_ioctl+0x1f4/0x2c8)
>> [    4.162425] [<c07a4ac4>] (sock_ioctl) from [<c0432678>]
>> (do_vfs_ioctl+0x9c/0x910)
>> [    4.169887] [<c0432678>] (do_vfs_ioctl) from [<c0432f20>]
>> (SyS_ioctl+0x34/0x5c)
>> [    4.177529] [<c0432f20>] (SyS_ioctl) from [<c0308480>]
>> (ret_fast_syscall+0x0/0x3c)
>> [    4.184635] handlers:
>> [    4.192273] [<c036911c>] bad_chained_irq
>> [    4.198255] Disabling IRQ #109
>> (...)
>> [   34.170316] smsc911x 1b800000.ethernet-ebi2 eth0: ISR failed
>> signaling test (IRQ 208)
> 
> Is this platform related to the Dragonboard 410C? I've got one from
> Sudeep, and it seems to work fine (though I've spotted a couple of
> gotchas in the DT).

Just to confirm is that with or without the proposed change?

Linus, I have been trying to find out which qcom board has this smsc911x
but I was unable to find any, so more info on the platform would be great!

I have been testing this on various Tegra boards that use gpio irqchips
for various external functions and have not seen any such problems so far.

Jon

-- 
nvpublic

Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons

[Marc Zyngier]

On 10/08/16 10:56, Jon Hunter wrote:
> Hi Marc, Linus,
> 
> On 10/08/16 10:41, Marc Zyngier wrote:
>> Hi Linus,
>>
>> On 10/08/16 00:03, Linus Walleij wrote:
>>> On Tue, Aug 9, 2016 at 3:20 PM, Jon Hunter <jonathanh@nvidia.com> wrote:
>>>
>>>> If that works, then does the following also work (without the above) ...
>>>>
>>>> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
>>>> index b4c1bc7c9ca2..e111b72e3162 100644
>>>> --- a/kernel/irq/chip.c
>>>> +++ b/kernel/irq/chip.c
>>>> @@ -824,6 +824,7 @@ __irq_do_set_handler(struct irq_desc *desc, irq_flow_handler_t handle,
>>>>                 irq_settings_set_norequest(desc);
>>>>                 irq_settings_set_nothread(desc);
>>>>                 desc->action = &chained_action;
>>>> +               __irq_set_trigger(desc, irqd_get_trigger_type(&desc->irq_data));
>>>>                 irq_startup(desc, true);
>>>>         }
>>>>  }
>>>>
>>>> It looks like there is a path for parent interrupts where the type
>>>> is not getting set. If the above works then we can discuss with Thomas
>>>> and Marc on the correct fix.
>>>
>>> I tried this on my problematic platform and then this happens:
>>>
>>> preparing networking...
>>> [    2.628246] ------------[ cut here ]------------
>>> [    2.628303] WARNING: CPU: 0 PID: 92 at ../kernel/irq/chip.c:26
>>> bad_chained_irq+0x44/0x4c
>>> [    2.631939] Chained irq 109 should not call an action
>>> [    2.640008] Modules linked in:
>>> [    2.647909] CPU: 0 PID: 92 Comm: ip Not tainted
>>> 4.8.0-rc1-00011-ga21e27b4cb66 #194
>>> [    2.647996] Hardware name: Generic DT based system
>>> [    2.655486] [<c030f8c8>] (unwind_backtrace) from [<c030c714>]
>>> (show_stack+0x10/0x14)
>>> [    2.660254] [<c030c714>] (show_stack) from [<c05df420>]
>>> (dump_stack+0x78/0x8c)
>>> [    2.668147] [<c05df420>] (dump_stack) from [<c031cef4>] (__warn+0xec/0x104)
>>> [    2.675173] [<c031cef4>] (__warn) from [<c031cf44>]
>>> (warn_slowpath_fmt+0x38/0x48)
>>> [    2.682033] [<c031cf44>] (warn_slowpath_fmt) from [<c0369160>]
>>> (bad_chained_irq+0x44/0x4c)
>>> [    2.689687] [<c0369160>] (bad_chained_irq) from [<c0365e28>]
>>> (__handle_irq_event_percpu+0x5c/0x290)
>>> [    2.697836] [<c0365e28>] (__handle_irq_event_percpu) from
>>> [<c0366078>] (handle_irq_event_percpu+0x1c/0x58)
>>> [    2.706778] [<c0366078>] (handle_irq_event_percpu) from
>>> [<c03660ec>] (handle_irq_event+0x38/0x5c)
>>> [    2.716498] [<c03660ec>] (handle_irq_event) from [<c03693f0>]
>>> (handle_level_irq+0xc4/0x150)
>>> [    2.725438] [<c03693f0>] (handle_level_irq) from [<c036542c>]
>>> (generic_handle_irq+0x24/0x34)
>>> [    2.733602] [<c036542c>] (generic_handle_irq) from [<c06127d4>]
>>> (msm_gpio_irq_handler+0xc8/0x150)
>>> [    2.742280] [<c06127d4>] (msm_gpio_irq_handler) from [<c036542c>]
>>> (generic_handle_irq+0x24/0x34)
>>> [    2.751048] [<c036542c>] (generic_handle_irq) from [<c0365720>]
>>> (__handle_domain_irq+0x7c/0xec)
>>> [    2.759901] [<c0365720>] (__handle_domain_irq) from [<c0301464>]
>>> (gic_handle_irq+0x48/0x8c)
>>> [    2.768323] [<c0301464>] (gic_handle_irq) from [<c08b8e4c>]
>>> (__irq_svc+0x6c/0xa8)
>>> [    2.776644] Exception stack(0xdeca1d48 to 0xdeca1d90)
>>> [    2.784284] 1d40:                   deca1dc0 00000000 00000000
>>> deca0018 ffff8bd6 deca1dc0
>>> [    2.789324] 1d60: 00000000 c0378ad0 60070013 00000000 00000000
>>> 001f3df8 c108fa04 deca1d98
>>> [    2.797481] 1d80: c08b77fc c0377930 60070013 ffffffff
>>> [    2.805651] [<c08b8e4c>] (__irq_svc) from [<c0377930>]
>>> (init_timer_key+0x28/0x104)
>>> [    2.810679] [<c0377930>] (init_timer_key) from [<c08b77fc>]
>>> (schedule_timeout+0x48/0x410)
>>> [    2.818145] [<c08b77fc>] (schedule_timeout) from [<c0378ad0>]
>>> (msleep+0x2c/0x38)
>>> [    2.826399] [<c0378ad0>] (msleep) from [<c06c2980>]
>>> (smsc911x_open+0x268/0x50c)
>>> [    2.833859] [<c06c2980>] (smsc911x_open) from [<c07c1994>]
>>> (__dev_open+0xa8/0x10c)
>>> [    2.840887] [<c07c1994>] (__dev_open) from [<c07c1c1c>]
>>> (__dev_change_flags+0x94/0x144)
>>> [    2.848525] [<c07c1c1c>] (__dev_change_flags) from [<c07c1ce4>]
>>> (dev_change_flags+0x18/0x48)
>>> [    2.856428] [<c07c1ce4>] (dev_change_flags) from [<c0823048>]
>>> (devinet_ioctl+0x6b0/0x768)
>>> [    2.865120] [<c0823048>] (devinet_ioctl) from [<c07a4ac4>]
>>> (sock_ioctl+0x1f4/0x2c8)
>>> [    2.873186] [<c07a4ac4>] (sock_ioctl) from [<c0432678>]
>>> (do_vfs_ioctl+0x9c/0x910)
>>> [    2.880645] [<c0432678>] (do_vfs_ioctl) from [<c0432f20>]
>>> (SyS_ioctl+0x34/0x5c)
>>> [    2.888290] [<c0432f20>] (SyS_ioctl) from [<c0308480>]
>>> (ret_fast_syscall+0x0/0x3c)
>>> [    2.895395] ---[ end trace a53e1e63b7bdfc4a ]---
>>> [    2.903917] random: fast init done
>>> [    3.036378] random: crng init done
>>> [    3.883906] irq 109: nobody cared (try booting with the "irqpoll" option)
>>> [    3.883940] CPU: 0 PID: 92 Comm: ip Tainted: G        W
>>> 4.8.0-rc1-00011-ga21e27b4cb66 #194
>>> [    3.889673] Hardware name: Generic DT based system
>>> [    3.898538] [<c030f8c8>] (unwind_backtrace) from [<c030c714>]
>>> (show_stack+0x10/0x14)
>>> [    3.903137] [<c030c714>] (show_stack) from [<c05df420>]
>>> (dump_stack+0x78/0x8c)
>>> [    3.911034] [<c05df420>] (dump_stack) from [<c0368804>]
>>> (__report_bad_irq+0x28/0xcc)
>>> [    3.918065] [<c0368804>] (__report_bad_irq) from [<c0368c18>]
>>> (note_interrupt+0x298/0x2e8)
>>> [    3.925971] [<c0368c18>] (note_interrupt) from [<c03660a8>]
>>> (handle_irq_event_percpu+0x4c/0x58)
>>> [    3.934040] [<c03660a8>] (handle_irq_event_percpu) from
>>> [<c03660ec>] (handle_irq_event+0x38/0x5c)
>>> [    3.942634] [<c03660ec>] (handle_irq_event) from [<c03693f0>]
>>> (handle_level_irq+0xc4/0x150)
>>> [    3.951660] [<c03693f0>] (handle_level_irq) from [<c036542c>]
>>> (generic_handle_irq+0x24/0x34)
>>> [    3.959821] [<c036542c>] (generic_handle_irq) from [<c06127d4>]
>>> (msm_gpio_irq_handler+0xc8/0x150)
>>> [    3.968502] [<c06127d4>] (msm_gpio_irq_handler) from [<c036542c>]
>>> (generic_handle_irq+0x24/0x34)
>>> [    3.977269] [<c036542c>] (generic_handle_irq) from [<c0365720>]
>>> (__handle_domain_irq+0x7c/0xec)
>>> [    3.986122] [<c0365720>] (__handle_domain_irq) from [<c0301464>]
>>> (gic_handle_irq+0x48/0x8c)
>>> [    3.994541] [<c0301464>] (gic_handle_irq) from [<c08b8e4c>]
>>> (__irq_svc+0x6c/0xa8)
>>> [    4.002865] Exception stack(0xdeca1c60 to 0xdeca1ca8)
>>> [    4.010507] 1c60: 00000000 c0abce68 c109f9c0 00000000 c109f9c0
>>> 00000000 deca0000 00000000
>>> [    4.015546] 1c80: 00000282 deca1d48 c0210800 001f3df8 e080400c
>>> deca1cb0 c0322330 c0322340
>>> [    4.023701] 1ca0: 20070113 ffffffff
>>> [    4.031868] [<c08b8e4c>] (__irq_svc) from [<c0322340>]
>>> (__do_softirq+0x9c/0x388)
>>> [    4.035166] [<c0322340>] (__do_softirq) from [<c03228f0>]
>>> (irq_exit+0xc0/0xfc)
>>> [    4.042805] [<c03228f0>] (irq_exit) from [<c0365724>]
>>> (__handle_domain_irq+0x80/0xec)
>>> [    4.049835] [<c0365724>] (__handle_domain_irq) from [<c0301464>]
>>> (gic_handle_irq+0x48/0x8c)
>>> [    4.057735] [<c0301464>] (gic_handle_irq) from [<c08b8e4c>]
>>> (__irq_svc+0x6c/0xa8)
>>> [    4.065887] Exception stack(0xdeca1d48 to 0xdeca1d90)
>>> [    4.073528] 1d40:                   deca1dc0 00000000 00000000
>>> deca0018 ffff8bd6 deca1dc0
>>> [    4.078566] 1d60: 00000000 c0378ad0 60070013 00000000 00000000
>>> 001f3df8 c108fa04 deca1d98
>>> [    4.086723] 1d80: c08b77fc c0377930 60070013 ffffffff
>>> [    4.094889] [<c08b8e4c>] (__irq_svc) from [<c0377930>]
>>> (init_timer_key+0x28/0x104)
>>> [    4.099921] [<c0377930>] (init_timer_key) from [<c08b77fc>]
>>> (schedule_timeout+0x48/0x410)
>>> [    4.107389] [<c08b77fc>] (schedule_timeout) from [<c0378ad0>]
>>> (msleep+0x2c/0x38)
>>> [    4.115640] [<c0378ad0>] (msleep) from [<c06c2980>]
>>> (smsc911x_open+0x268/0x50c)
>>> [    4.123100] [<c06c2980>] (smsc911x_open) from [<c07c1994>]
>>> (__dev_open+0xa8/0x10c)
>>> [    4.130128] [<c07c1994>] (__dev_open) from [<c07c1c1c>]
>>> (__dev_change_flags+0x94/0x144)
>>> [    4.137769] [<c07c1c1c>] (__dev_change_flags) from [<c07c1ce4>]
>>> (dev_change_flags+0x18/0x48)
>>> [    4.145670] [<c07c1ce4>] (dev_change_flags) from [<c0823048>]
>>> (devinet_ioctl+0x6b0/0x768)
>>> [    4.154357] [<c0823048>] (devinet_ioctl) from [<c07a4ac4>]
>>> (sock_ioctl+0x1f4/0x2c8)
>>> [    4.162425] [<c07a4ac4>] (sock_ioctl) from [<c0432678>]
>>> (do_vfs_ioctl+0x9c/0x910)
>>> [    4.169887] [<c0432678>] (do_vfs_ioctl) from [<c0432f20>]
>>> (SyS_ioctl+0x34/0x5c)
>>> [    4.177529] [<c0432f20>] (SyS_ioctl) from [<c0308480>]
>>> (ret_fast_syscall+0x0/0x3c)
>>> [    4.184635] handlers:
>>> [    4.192273] [<c036911c>] bad_chained_irq
>>> [    4.198255] Disabling IRQ #109
>>> (...)
>>> [   34.170316] smsc911x 1b800000.ethernet-ebi2 eth0: ISR failed
>>> signaling test (IRQ 208)
>>
>> Is this platform related to the Dragonboard 410C? I've got one from
>> Sudeep, and it seems to work fine (though I've spotted a couple of
>> gotchas in the DT).
> 
> Just to confirm is that with or without the proposed change?

The proposed change didn't have any impact on this board, but it may
just be that the firmware had done the right thing...

> Linus, I have been trying to find out which qcom board has this smsc911x
> but I was unable to find any, so more info on the platform would be great!
> 
> I have been testing this on various Tegra boards that use gpio irqchips
> for various external functions and have not seen any such problems so far.

I'm also struggling to reproduce this issue, so any pointer to an easy
to source board would be appreciated (I can trade in Versatile/Realview
HW...).

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons

[Linus Walleij]

On Wed, Aug 10, 2016 at 11:56 AM, Jon Hunter <jonathanh@nvidia.com> wrote:
> Hi Marc, Linus,
> On 10/08/16 10:41, Marc Zyngier wrote:
>> On 10/08/16 00:03, Linus Walleij wrote:
>>> On Tue, Aug 9, 2016 at 3:20 PM, Jon Hunter <jonathanh@nvidia.com> wrote:
>>>
>>>> If that works, then does the following also work (without the above) ...
>>>>
>>>> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
>>>> index b4c1bc7c9ca2..e111b72e3162 100644
>>>> --- a/kernel/irq/chip.c
>>>> +++ b/kernel/irq/chip.c
>>>> @@ -824,6 +824,7 @@ __irq_do_set_handler(struct irq_desc *desc, irq_flow_handler_t handle,
>>>>                 irq_settings_set_norequest(desc);
>>>>                 irq_settings_set_nothread(desc);
>>>>                 desc->action = &chained_action;
>>>> +               __irq_set_trigger(desc, irqd_get_trigger_type(&desc->irq_data));
>>>>                 irq_startup(desc, true);
>>>>         }
>>>>  }
(...)
>>> I tried this on my problematic platform and then this happens:
>>>
>>> preparing networking...
>>> [    2.628246] ------------[ cut here ]------------
>>> [    2.628303] WARNING: CPU: 0 PID: 92 at ../kernel/irq/chip.c:26
>>> bad_chained_irq+0x44/0x4c
(...)
> Just to confirm is that with or without the proposed change?

This is with:
+               __irq_set_trigger(desc, irqd_get_trigger_type(&desc->irq_data));

No other changes.

Before the change it boots, but the IRQs don't work.

After the change it boots and crashes like that.

> Linus, I have been trying to find out which qcom board has this smsc911x
> but I was unable to find any, so more info on the platform would be great!

It has nothing to do with the SMSC911x per se. The problem pertains
to *all* IRQs from the PMIC.

This is the original APQ8060 Dragonboard from BSquare.
https://dflund.se/~triad/krad/dragonboard/

Qualcomm also have a development board called "Surf" which
likely have this problem.

This is the MFD irqchip driver that has the problem:
drivers/mfd/pm8921-core.c

No IRQs from this works. Not even the power button or anything.

> I have been testing this on various Tegra boards that use gpio irqchips
> for various external functions and have not seen any such problems so far.

John seems to be seeing it on other Qualcomm gpiochip drivers with
APQ8064 on the Nexus 7. I would be surprised if any of the APQ8064
boards doesn't have the issue.

Yours,
Linus Walleij

Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons

[Jon Hunter]

On 10/08/16 14:58, Linus Walleij wrote:
> On Wed, Aug 10, 2016 at 11:56 AM, Jon Hunter <jonathanh@nvidia.com> wrote:
>> Hi Marc, Linus,
>> On 10/08/16 10:41, Marc Zyngier wrote:
>>> On 10/08/16 00:03, Linus Walleij wrote:
>>>> On Tue, Aug 9, 2016 at 3:20 PM, Jon Hunter <jonathanh@nvidia.com> wrote:
>>>>
>>>>> If that works, then does the following also work (without the above) ...
>>>>>
>>>>> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
>>>>> index b4c1bc7c9ca2..e111b72e3162 100644
>>>>> --- a/kernel/irq/chip.c
>>>>> +++ b/kernel/irq/chip.c
>>>>> @@ -824,6 +824,7 @@ __irq_do_set_handler(struct irq_desc *desc, irq_flow_handler_t handle,
>>>>>                 irq_settings_set_norequest(desc);
>>>>>                 irq_settings_set_nothread(desc);
>>>>>                 desc->action = &chained_action;
>>>>> +               __irq_set_trigger(desc, irqd_get_trigger_type(&desc->irq_data));
>>>>>                 irq_startup(desc, true);
>>>>>         }
>>>>>  }
> (...)
>>>> I tried this on my problematic platform and then this happens:
>>>>
>>>> preparing networking...
>>>> [    2.628246] ------------[ cut here ]------------
>>>> [    2.628303] WARNING: CPU: 0 PID: 92 at ../kernel/irq/chip.c:26
>>>> bad_chained_irq+0x44/0x4c
> (...)
>> Just to confirm is that with or without the proposed change?
> 
> This is with:
> +               __irq_set_trigger(desc, irqd_get_trigger_type(&desc->irq_data));
> 
> No other changes.
> 
> Before the change it boots, but the IRQs don't work.
> 
> After the change it boots and crashes like that.

OK.

>> Linus, I have been trying to find out which qcom board has this smsc911x
>> but I was unable to find any, so more info on the platform would be great!
> 
> It has nothing to do with the SMSC911x per se. The problem pertains
> to *all* IRQs from the PMIC.
> 
> This is the original APQ8060 Dragonboard from BSquare.
> https://dflund.se/~triad/krad/dragonboard/

Where can I see the DT source for this board? There is a
arch/arm/boot/dts/qcom-apq8060-dragonboard.dts but this does not appear
to be the same (does not have smsc911x).

Cheers
Jon

-- 
nvpublic

Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons

[Linus Walleij]

On Wed, Aug 10, 2016 at 4:12 PM, Jon Hunter <jonathanh@nvidia.com> wrote:

> Where can I see the DT source for this board? There is a
> arch/arm/boot/dts/qcom-apq8060-dragonboard.dts but this does not appear
> to be the same (does not have smsc911x).

There:
https://git.kernel.org/cgit/linux/kernel/git/linusw/linux-integrator.git/tree/arch/arm/boot/dts/qcom-apq8060-dragonboard.dts?h=apq8060-dragonboard-ethernet

Branch:
https://git.kernel.org/cgit/linux/kernel/git/linusw/linux-integrator.git/log/?h=apq8060-dragonboard-ethernet

Linus Walleij

Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons

[Linus Walleij]

On Wed, Aug 10, 2016 at 11:41 AM, Marc Zyngier <marc.zyngier@arm.com> wrote:

> Is this platform related to the Dragonboard 410C? I've got one from
> Sudeep, and it seems to work fine (though I've spotted a couple of
> gotchas in the DT).

Nopes this is the ARMv7 APQ8060, the original (first!) dragonboard.
https://dflund.se/~triad/krad/dragonboard/

(You know me, I always use the odd hardware nobody else use...)

> Do you see this symptom on all chained interrupts? Or just this
> particular one?

This appears on all IRQs on the PM (MFD) ASIC.

Yours,
Linus Walleij

Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons

[Marc Zyngier]

On 10/08/16 14:50, Linus Walleij wrote:
> On Wed, Aug 10, 2016 at 11:41 AM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> 
>> Is this platform related to the Dragonboard 410C? I've got one from
>> Sudeep, and it seems to work fine (though I've spotted a couple of
>> gotchas in the DT).
> 
> Nopes this is the ARMv7 APQ8060, the original (first!) dragonboard.
> https://dflund.se/~triad/krad/dragonboard/
> 
> (You know me, I always use the odd hardware nobody else use...)

Guess what, I just found this exact sucker in the pile of
"junk we won't ever use because it can't run mainline".
I even booted one of your test images on it.

Do you have a tree I can clone directly, with all the ugly patches
applied?

> 
>> Do you see this symptom on all chained interrupts? Or just this
>> particular one?
> 
> This appears on all IRQs on the PM (MFD) ASIC.

These interrupts?

197:          1          0    pm8xxx  50 Edge      pmic8xxx_pwrkey_release
198:          1          0    pm8xxx  51 Edge      pmic8xxx_pwrkey_press
199:          8          0    pm8xxx  74 Edge      pmic-keypad
200:          0          0    pm8xxx  75 Edge      pmic-keypad-stuck
201:          0          0    pm8xxx  39 Edge      pm8xxx_rtc_alarm

How is the interrupt topology built? pm8085 -> tlmm -> GIC?

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons

[Linus Walleij]

On Wed, Aug 10, 2016 at 5:17 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:

> Guess what, I just found this exact sucker in the pile of
> "junk we won't ever use because it can't run mainline".
> I even booted one of your test images on it.
>
> Do you have a tree I can clone directly, with all the ugly patches
> applied?

Yeah:
https://git.kernel.org/cgit/linux/kernel/git/linusw/linux-integrator.git/log/?h=apq8060-dragonboard-ethernet

The apq8060-dragonboard branch is clean (just necessary boot fixes,
the flag fixes and the revert), gives the right IRQs:
https://git.kernel.org/cgit/linux/kernel/git/linusw/linux-integrator.git/log/?h=apq8060-dragonboard

The webpage gives the details of how I build, my Makefile and
initramfs.cpio etc if you want:
https://dflund.se/~triad/krad/dragonboard/

>>> Do you see this symptom on all chained interrupts? Or just this
>>> particular one?
>>
>> This appears on all IRQs on the PM (MFD) ASIC.
>
> These interrupts?
>
> 197:          1          0    pm8xxx  50 Edge      pmic8xxx_pwrkey_release
> 198:          1          0    pm8xxx  51 Edge      pmic8xxx_pwrkey_press
> 199:          8          0    pm8xxx  74 Edge      pmic-keypad
> 200:          0          0    pm8xxx  75 Edge      pmic-keypad-stuck
> 201:          0          0    pm8xxx  39 Edge      pm8xxx_rtc_alarm

Yes. Doesn' appear anymore in v4.8-rc1

> How is the interrupt topology built? pm8085 -> tlmm -> GIC?

Yes AFAICT.

pm8058 -> tlmm hwirq 88 -> GIC hwirq 16

Yours,
Linus Walleij

Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons

[Linus Walleij]

On Sat, Aug 6, 2016 at 1:45 AM, John Stultz <john.stultz@linaro.org> wrote:

> @@ -614,7 +615,11 @@ unsigned int irq_create_fwspec_mapping(struct
> irq_fwspec *fwspec)
>                  * it now and return the interrupt number.
>                  */
>                 if (irq_get_trigger_type(virq) == IRQ_TYPE_NONE) {
> -                       irq_set_irq_type(virq, type);
> +                       irq_data = irq_get_irq_data(virq);
> +                       if (!irq_data)
> +                               return 0;
> +
> +                       irqd_set_trigger_type(irq_data, type);
>                         return virq;
>                 }
>
> If I revert just that, it works again.

This makes my platform work too.
Tested-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons

[Marc Zyngier]

On 08/08/16 22:48, Linus Walleij wrote:
> On Sat, Aug 6, 2016 at 1:45 AM, John Stultz <john.stultz@linaro.org> wrote:
> 
>> @@ -614,7 +615,11 @@ unsigned int irq_create_fwspec_mapping(struct
>> irq_fwspec *fwspec)
>>                  * it now and return the interrupt number.
>>                  */
>>                 if (irq_get_trigger_type(virq) == IRQ_TYPE_NONE) {
>> -                       irq_set_irq_type(virq, type);
>> +                       irq_data = irq_get_irq_data(virq);
>> +                       if (!irq_data)
>> +                               return 0;
>> +
>> +                       irqd_set_trigger_type(irq_data, type);
>>                         return virq;
>>                 }
>>
>> If I revert just that, it works again.
> 
> This makes my platform work too.
> Tested-by: Linus Walleij <linus.walleij@linaro.org>

Hmmm. I'm now booting your kernel on the APQ8060, and reverting this
hunk doesn't fix it for me. I'm confused...

The interesting part is this:
109:     100000          0   msmgpio  88 Level     (null)

which shows that the cascade interrupt has been disabled after 100000
unhandled interrupts. Somehow, this screams "misconfiguration"...

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons

[Jon Hunter]

On 11/08/16 09:37, Marc Zyngier wrote:
> On 08/08/16 22:48, Linus Walleij wrote:
>> On Sat, Aug 6, 2016 at 1:45 AM, John Stultz <john.stultz@linaro.org> wrote:
>>
>>> @@ -614,7 +615,11 @@ unsigned int irq_create_fwspec_mapping(struct
>>> irq_fwspec *fwspec)
>>>                  * it now and return the interrupt number.
>>>                  */
>>>                 if (irq_get_trigger_type(virq) == IRQ_TYPE_NONE) {
>>> -                       irq_set_irq_type(virq, type);
>>> +                       irq_data = irq_get_irq_data(virq);
>>> +                       if (!irq_data)
>>> +                               return 0;
>>> +
>>> +                       irqd_set_trigger_type(irq_data, type);
>>>                         return virq;
>>>                 }
>>>
>>> If I revert just that, it works again.
>>
>> This makes my platform work too.
>> Tested-by: Linus Walleij <linus.walleij@linaro.org>
> 
> Hmmm. I'm now booting your kernel on the APQ8060, and reverting this
> hunk doesn't fix it for me. I'm confused...
> 
> The interesting part is this:
> 109:     100000          0   msmgpio  88 Level     (null)

88 is the pm8058 parent interrupt and so I am surprised you would even
see this in /proc/interrupts as it should be a chained interrupt, right?

Are you seeing this with all the ethernet updates for the APQ8060 in
Linus' branch? I am curious what you see with stock v4.8-rc1 and if
interrupts work ok with the change I had proposed. Hard to tell if there
is more than one issue here.

Cheers
Jon

-- 
nvpublic

Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons

[Marc Zyngier]

On 11/08/16 10:47, Jon Hunter wrote:
> 
> On 11/08/16 09:37, Marc Zyngier wrote:
>> On 08/08/16 22:48, Linus Walleij wrote:
>>> On Sat, Aug 6, 2016 at 1:45 AM, John Stultz <john.stultz@linaro.org> wrote:
>>>
>>>> @@ -614,7 +615,11 @@ unsigned int irq_create_fwspec_mapping(struct
>>>> irq_fwspec *fwspec)
>>>>                  * it now and return the interrupt number.
>>>>                  */
>>>>                 if (irq_get_trigger_type(virq) == IRQ_TYPE_NONE) {
>>>> -                       irq_set_irq_type(virq, type);
>>>> +                       irq_data = irq_get_irq_data(virq);
>>>> +                       if (!irq_data)
>>>> +                               return 0;
>>>> +
>>>> +                       irqd_set_trigger_type(irq_data, type);
>>>>                         return virq;
>>>>                 }
>>>>
>>>> If I revert just that, it works again.
>>>
>>> This makes my platform work too.
>>> Tested-by: Linus Walleij <linus.walleij@linaro.org>
>>
>> Hmmm. I'm now booting your kernel on the APQ8060, and reverting this
>> hunk doesn't fix it for me. I'm confused...
>>
>> The interesting part is this:
>> 109:     100000          0   msmgpio  88 Level     (null)
> 
> 88 is the pm8058 parent interrupt and so I am surprised you would even
> see this in /proc/interrupts as it should be a chained interrupt, right?

That's because it repeatedly fires without a proper handler, and only
appears then.

> Are you seeing this with all the ethernet updates for the APQ8060 in
> Linus' branch? I am curious what you see with stock v4.8-rc1 and if
> interrupts work ok with the change I had proposed. Hard to tell if there
> is more than one issue here.

(mostly) stock v4.8-rc1 exhibits the same issue, and your fix doesn't
help this particular issue. Reverting your fix *and* applying the above
revert makes it work again. Which is just papering over the issue, as it
only does something when the interrupt is seen for a second time.

I must be missing something obvious...

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons

[Marc Zyngier]

On 11/08/16 12:45, Marc Zyngier wrote:
> On 11/08/16 10:47, Jon Hunter wrote:
>>
>> On 11/08/16 09:37, Marc Zyngier wrote:
>>> On 08/08/16 22:48, Linus Walleij wrote:
>>>> On Sat, Aug 6, 2016 at 1:45 AM, John Stultz <john.stultz@linaro.org> wrote:
>>>>
>>>>> @@ -614,7 +615,11 @@ unsigned int irq_create_fwspec_mapping(struct
>>>>> irq_fwspec *fwspec)
>>>>>                  * it now and return the interrupt number.
>>>>>                  */
>>>>>                 if (irq_get_trigger_type(virq) == IRQ_TYPE_NONE) {
>>>>> -                       irq_set_irq_type(virq, type);
>>>>> +                       irq_data = irq_get_irq_data(virq);
>>>>> +                       if (!irq_data)
>>>>> +                               return 0;
>>>>> +
>>>>> +                       irqd_set_trigger_type(irq_data, type);
>>>>>                         return virq;
>>>>>                 }
>>>>>
>>>>> If I revert just that, it works again.
>>>>
>>>> This makes my platform work too.
>>>> Tested-by: Linus Walleij <linus.walleij@linaro.org>
>>>
>>> Hmmm. I'm now booting your kernel on the APQ8060, and reverting this
>>> hunk doesn't fix it for me. I'm confused...
>>>
>>> The interesting part is this:
>>> 109:     100000          0   msmgpio  88 Level     (null)
>>
>> 88 is the pm8058 parent interrupt and so I am surprised you would even
>> see this in /proc/interrupts as it should be a chained interrupt, right?
> 
> That's because it repeatedly fires without a proper handler, and only
> appears then.
> 
>> Are you seeing this with all the ethernet updates for the APQ8060 in
>> Linus' branch? I am curious what you see with stock v4.8-rc1 and if
>> interrupts work ok with the change I had proposed. Hard to tell if there
>> is more than one issue here.
> 
> (mostly) stock v4.8-rc1 exhibits the same issue, and your fix doesn't
> help this particular issue. Reverting your fix *and* applying the above
> revert makes it work again. Which is just papering over the issue, as it
> only does something when the interrupt is seen for a second time.

Also: this behaviour only affects the second cascade (from the pm8058 to
the tlmm). I can perfectly configure the first one (from the tlmm to the
GIC) with your fix.

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons

[Marc Zyngier]

On 11/08/16 10:47, Jon Hunter wrote:
> 
> On 11/08/16 09:37, Marc Zyngier wrote:
>> On 08/08/16 22:48, Linus Walleij wrote:
>>> On Sat, Aug 6, 2016 at 1:45 AM, John Stultz <john.stultz@linaro.org> wrote:
>>>
>>>> @@ -614,7 +615,11 @@ unsigned int irq_create_fwspec_mapping(struct
>>>> irq_fwspec *fwspec)
>>>>                  * it now and return the interrupt number.
>>>>                  */
>>>>                 if (irq_get_trigger_type(virq) == IRQ_TYPE_NONE) {
>>>> -                       irq_set_irq_type(virq, type);
>>>> +                       irq_data = irq_get_irq_data(virq);
>>>> +                       if (!irq_data)
>>>> +                               return 0;
>>>> +
>>>> +                       irqd_set_trigger_type(irq_data, type);
>>>>                         return virq;
>>>>                 }
>>>>
>>>> If I revert just that, it works again.
>>>
>>> This makes my platform work too.
>>> Tested-by: Linus Walleij <linus.walleij@linaro.org>
>>
>> Hmmm. I'm now booting your kernel on the APQ8060, and reverting this
>> hunk doesn't fix it for me. I'm confused...
>>
>> The interesting part is this:
>> 109:     100000          0   msmgpio  88 Level     (null)
> 
> 88 is the pm8058 parent interrupt and so I am surprised you would even
> see this in /proc/interrupts as it should be a chained interrupt, right?
> 
> Are you seeing this with all the ethernet updates for the APQ8060 in
> Linus' branch? I am curious what you see with stock v4.8-rc1 and if
> interrupts work ok with the change I had proposed. Hard to tell if there
> is more than one issue here.

Nailed the sucker:

diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index b4c1bc7..9d7284a 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -820,6 +820,18 @@ __irq_do_set_handler(struct irq_desc *desc, irq_flow_handler_t handle,
 	desc->name = name;
 
 	if (handle != handle_bad_irq && is_chained) {
+		int ret;
+
+		ret = __irq_set_trigger(desc,
+					irqd_get_trigger_type(&desc->irq_data));
+		WARN_ON(ret);
+		/*
+		 * This is beyond ugly: .set_type may have overridden
+		 * the flow, not not knowing that we're dealing with a
+		 * chained handler. Reset it here because we know
+		 * better.
+		 */
+		desc->handle_irq = handle;
 		irq_settings_set_noprobe(desc);
 		irq_settings_set_norequest(desc);
 		irq_settings_set_nothread(desc);

Linus, Jon: Can you please confirm this fixes your respective issues?

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons

[Jon Hunter]

On 11/08/16 13:46, Marc Zyngier wrote:
> On 11/08/16 10:47, Jon Hunter wrote:
>>
>> On 11/08/16 09:37, Marc Zyngier wrote:
>>> On 08/08/16 22:48, Linus Walleij wrote:
>>>> On Sat, Aug 6, 2016 at 1:45 AM, John Stultz <john.stultz@linaro.org> wrote:
>>>>
>>>>> @@ -614,7 +615,11 @@ unsigned int irq_create_fwspec_mapping(struct
>>>>> irq_fwspec *fwspec)
>>>>>                  * it now and return the interrupt number.
>>>>>                  */
>>>>>                 if (irq_get_trigger_type(virq) == IRQ_TYPE_NONE) {
>>>>> -                       irq_set_irq_type(virq, type);
>>>>> +                       irq_data = irq_get_irq_data(virq);
>>>>> +                       if (!irq_data)
>>>>> +                               return 0;
>>>>> +
>>>>> +                       irqd_set_trigger_type(irq_data, type);
>>>>>                         return virq;
>>>>>                 }
>>>>>
>>>>> If I revert just that, it works again.
>>>>
>>>> This makes my platform work too.
>>>> Tested-by: Linus Walleij <linus.walleij@linaro.org>
>>>
>>> Hmmm. I'm now booting your kernel on the APQ8060, and reverting this
>>> hunk doesn't fix it for me. I'm confused...
>>>
>>> The interesting part is this:
>>> 109:     100000          0   msmgpio  88 Level     (null)
>>
>> 88 is the pm8058 parent interrupt and so I am surprised you would even
>> see this in /proc/interrupts as it should be a chained interrupt, right?
>>
>> Are you seeing this with all the ethernet updates for the APQ8060 in
>> Linus' branch? I am curious what you see with stock v4.8-rc1 and if
>> interrupts work ok with the change I had proposed. Hard to tell if there
>> is more than one issue here.
> 
> Nailed the sucker:

Great!

> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> index b4c1bc7..9d7284a 100644
> --- a/kernel/irq/chip.c
> +++ b/kernel/irq/chip.c
> @@ -820,6 +820,18 @@ __irq_do_set_handler(struct irq_desc *desc, irq_flow_handler_t handle,
>  	desc->name = name;
>  
>  	if (handle != handle_bad_irq && is_chained) {
> +		int ret;
> +
> +		ret = __irq_set_trigger(desc,
> +					irqd_get_trigger_type(&desc->irq_data));
> +		WARN_ON(ret);

You could wrap the entire call in the WARN_ON(). I was not sure if there
was a better way to handle that.

> +		/*
> +		 * This is beyond ugly: .set_type may have overridden
> +		 * the flow, not not knowing that we're dealing with a
> +		 * chained handler. Reset it here because we know
> +		 * better.
> +		 */
> +		desc->handle_irq = handle;

Yes I see the call to irq_set_handler in the pinctrl-msm.c set_type.
Good catch!

Apart from the above ...

Acked-by: Jon Hunter <jonathanh@nvidia.com>

Cheers
Jon

-- 
nvpublic

Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons

[Marc Zyngier]

On 11/08/16 14:29, Jon Hunter wrote:
> 
> On 11/08/16 13:46, Marc Zyngier wrote:
>> On 11/08/16 10:47, Jon Hunter wrote:
>>>
>>> On 11/08/16 09:37, Marc Zyngier wrote:
>>>> On 08/08/16 22:48, Linus Walleij wrote:
>>>>> On Sat, Aug 6, 2016 at 1:45 AM, John Stultz <john.stultz@linaro.org> wrote:
>>>>>
>>>>>> @@ -614,7 +615,11 @@ unsigned int irq_create_fwspec_mapping(struct
>>>>>> irq_fwspec *fwspec)
>>>>>>                  * it now and return the interrupt number.
>>>>>>                  */
>>>>>>                 if (irq_get_trigger_type(virq) == IRQ_TYPE_NONE) {
>>>>>> -                       irq_set_irq_type(virq, type);
>>>>>> +                       irq_data = irq_get_irq_data(virq);
>>>>>> +                       if (!irq_data)
>>>>>> +                               return 0;
>>>>>> +
>>>>>> +                       irqd_set_trigger_type(irq_data, type);
>>>>>>                         return virq;
>>>>>>                 }
>>>>>>
>>>>>> If I revert just that, it works again.
>>>>>
>>>>> This makes my platform work too.
>>>>> Tested-by: Linus Walleij <linus.walleij@linaro.org>
>>>>
>>>> Hmmm. I'm now booting your kernel on the APQ8060, and reverting this
>>>> hunk doesn't fix it for me. I'm confused...
>>>>
>>>> The interesting part is this:
>>>> 109:     100000          0   msmgpio  88 Level     (null)
>>>
>>> 88 is the pm8058 parent interrupt and so I am surprised you would even
>>> see this in /proc/interrupts as it should be a chained interrupt, right?
>>>
>>> Are you seeing this with all the ethernet updates for the APQ8060 in
>>> Linus' branch? I am curious what you see with stock v4.8-rc1 and if
>>> interrupts work ok with the change I had proposed. Hard to tell if there
>>> is more than one issue here.
>>
>> Nailed the sucker:
> 
> Great!
> 
>> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
>> index b4c1bc7..9d7284a 100644
>> --- a/kernel/irq/chip.c
>> +++ b/kernel/irq/chip.c
>> @@ -820,6 +820,18 @@ __irq_do_set_handler(struct irq_desc *desc, irq_flow_handler_t handle,
>>  	desc->name = name;
>>  
>>  	if (handle != handle_bad_irq && is_chained) {
>> +		int ret;
>> +
>> +		ret = __irq_set_trigger(desc,
>> +					irqd_get_trigger_type(&desc->irq_data));
>> +		WARN_ON(ret);
> 
> You could wrap the entire call in the WARN_ON(). I was not sure if there
> was a better way to handle that.

Actually, I've decided to drop it. We already have a message in
__irq_set_trigger(), and if we really want to scream, that's the one we
should consider upgrading to a WARN_ON().

> 
>> +		/*
>> +		 * This is beyond ugly: .set_type may have overridden
>> +		 * the flow, not not knowing that we're dealing with a
>> +		 * chained handler. Reset it here because we know
>> +		 * better.
>> +		 */
>> +		desc->handle_irq = handle;
> 
> Yes I see the call to irq_set_handler in the pinctrl-msm.c set_type.
> Good catch!

I can't believe it took me that much time to realize that. Guess I need
an extended weekend! ;-)

> 
> Apart from the above ...
> 
> Acked-by: Jon Hunter <jonathanh@nvidia.com>

Thanks a lot Jon,

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons

[John Stultz]

On Thu, Aug 11, 2016 at 5:46 AM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> On 11/08/16 10:47, Jon Hunter wrote:
>>
>> On 11/08/16 09:37, Marc Zyngier wrote:
>>> On 08/08/16 22:48, Linus Walleij wrote:
>>>> On Sat, Aug 6, 2016 at 1:45 AM, John Stultz <john.stultz@linaro.org> wrote:
>>>>
>>>>> @@ -614,7 +615,11 @@ unsigned int irq_create_fwspec_mapping(struct
>>>>> irq_fwspec *fwspec)
>>>>>                  * it now and return the interrupt number.
>>>>>                  */
>>>>>                 if (irq_get_trigger_type(virq) == IRQ_TYPE_NONE) {
>>>>> -                       irq_set_irq_type(virq, type);
>>>>> +                       irq_data = irq_get_irq_data(virq);
>>>>> +                       if (!irq_data)
>>>>> +                               return 0;
>>>>> +
>>>>> +                       irqd_set_trigger_type(irq_data, type);
>>>>>                         return virq;
>>>>>                 }
>>>>>
>>>>> If I revert just that, it works again.
>>>>
>>>> This makes my platform work too.
>>>> Tested-by: Linus Walleij <linus.walleij@linaro.org>
>>>
>>> Hmmm. I'm now booting your kernel on the APQ8060, and reverting this
>>> hunk doesn't fix it for me. I'm confused...
>>>
>>> The interesting part is this:
>>> 109:     100000          0   msmgpio  88 Level     (null)
>>
>> 88 is the pm8058 parent interrupt and so I am surprised you would even
>> see this in /proc/interrupts as it should be a chained interrupt, right?
>>
>> Are you seeing this with all the ethernet updates for the APQ8060 in
>> Linus' branch? I am curious what you see with stock v4.8-rc1 and if
>> interrupts work ok with the change I had proposed. Hard to tell if there
>> is more than one issue here.
>
> Nailed the sucker:
>
> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> index b4c1bc7..9d7284a 100644
> --- a/kernel/irq/chip.c
> +++ b/kernel/irq/chip.c
> @@ -820,6 +820,18 @@ __irq_do_set_handler(struct irq_desc *desc, irq_flow_handler_t handle,
>         desc->name = name;
>
>         if (handle != handle_bad_irq && is_chained) {
> +               int ret;
> +
> +               ret = __irq_set_trigger(desc,
> +                                       irqd_get_trigger_type(&desc->irq_data));
> +               WARN_ON(ret);
> +               /*
> +                * This is beyond ugly: .set_type may have overridden
> +                * the flow, not not knowing that we're dealing with a
> +                * chained handler. Reset it here because we know
> +                * better.
> +                */
> +               desc->handle_irq = handle;
>                 irq_settings_set_noprobe(desc);
>                 irq_settings_set_norequest(desc);
>                 irq_settings_set_nothread(desc);
>
> Linus, Jon: Can you please confirm this fixes your respective issues?

Yep. That works for me!

Tested-by: John Stultz <john.stultz@linaro.org>

Thanks so much for hunting this down!
-john

Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons

[Marc Zyngier]

On 11/08/16 16:32, John Stultz wrote:
> On Thu, Aug 11, 2016 at 5:46 AM, Marc Zyngier <marc.zyngier@arm.com> wrote:
>
>> Nailed the sucker:
>>
>> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
>> index b4c1bc7..9d7284a 100644
>> --- a/kernel/irq/chip.c
>> +++ b/kernel/irq/chip.c
>> @@ -820,6 +820,18 @@ __irq_do_set_handler(struct irq_desc *desc, irq_flow_handler_t handle,
>>         desc->name = name;
>>
>>         if (handle != handle_bad_irq && is_chained) {
>> +               int ret;
>> +
>> +               ret = __irq_set_trigger(desc,
>> +                                       irqd_get_trigger_type(&desc->irq_data));
>> +               WARN_ON(ret);
>> +               /*
>> +                * This is beyond ugly: .set_type may have overridden
>> +                * the flow, not not knowing that we're dealing with a
>> +                * chained handler. Reset it here because we know
>> +                * better.
>> +                */
>> +               desc->handle_irq = handle;
>>                 irq_settings_set_noprobe(desc);
>>                 irq_settings_set_norequest(desc);
>>                 irq_settings_set_nothread(desc);
>>
>> Linus, Jon: Can you please confirm this fixes your respective issues?
> 
> Yep. That works for me!
> 
> Tested-by: John Stultz <john.stultz@linaro.org>
> 
> Thanks so much for hunting this down!

Thanks for the report and the testing! I'll post the final patch
shortly. tglx being away for a couple of weeks, it may take some time
before this hits mainline though.

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons

[Linus Walleij]

On Thu, Aug 11, 2016 at 2:46 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:

> Nailed the sucker:
>
> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> index b4c1bc7..9d7284a 100644
> --- a/kernel/irq/chip.c
> +++ b/kernel/irq/chip.c
> @@ -820,6 +820,18 @@ __irq_do_set_handler(struct irq_desc *desc, irq_flow_handler_t handle,
>         desc->name = name;
>
>         if (handle != handle_bad_irq && is_chained) {
> +               int ret;
> +
> +               ret = __irq_set_trigger(desc,
> +                                       irqd_get_trigger_type(&desc->irq_data));
> +               WARN_ON(ret);
> +               /*
> +                * This is beyond ugly: .set_type may have overridden
> +                * the flow, not not knowing that we're dealing with a
> +                * chained handler. Reset it here because we know
> +                * better.
> +                */
> +               desc->handle_irq = handle;
>                 irq_settings_set_noprobe(desc);
>                 irq_settings_set_norequest(desc);
>                 irq_settings_set_nothread(desc);
>
> Linus, Jon: Can you please confirm this fixes your respective issues?

Yes, bullseye.

Tested-by: Linus Walleij <linus.walleij@linaro.org>

Thanks for your efforts!

Yours,
Linus Walleij

Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons

[Bjorn Andersson]

On Thu 11 Aug 05:46 PDT 2016, Marc Zyngier wrote:

> On 11/08/16 10:47, Jon Hunter wrote:
> > 
> > On 11/08/16 09:37, Marc Zyngier wrote:
> >> On 08/08/16 22:48, Linus Walleij wrote:
> >>> On Sat, Aug 6, 2016 at 1:45 AM, John Stultz <john.stultz@linaro.org> wrote:
> >>>
> >>>> @@ -614,7 +615,11 @@ unsigned int irq_create_fwspec_mapping(struct
> >>>> irq_fwspec *fwspec)
> >>>>                  * it now and return the interrupt number.
> >>>>                  */
> >>>>                 if (irq_get_trigger_type(virq) == IRQ_TYPE_NONE) {
> >>>> -                       irq_set_irq_type(virq, type);
> >>>> +                       irq_data = irq_get_irq_data(virq);
> >>>> +                       if (!irq_data)
> >>>> +                               return 0;
> >>>> +
> >>>> +                       irqd_set_trigger_type(irq_data, type);
> >>>>                         return virq;
> >>>>                 }
> >>>>
> >>>> If I revert just that, it works again.
> >>>
> >>> This makes my platform work too.
> >>> Tested-by: Linus Walleij <linus.walleij@linaro.org>
> >>
> >> Hmmm. I'm now booting your kernel on the APQ8060, and reverting this
> >> hunk doesn't fix it for me. I'm confused...
> >>
> >> The interesting part is this:
> >> 109:     100000          0   msmgpio  88 Level     (null)
> > 
> > 88 is the pm8058 parent interrupt and so I am surprised you would even
> > see this in /proc/interrupts as it should be a chained interrupt, right?
> > 
> > Are you seeing this with all the ethernet updates for the APQ8060 in
> > Linus' branch? I am curious what you see with stock v4.8-rc1 and if
> > interrupts work ok with the change I had proposed. Hard to tell if there
> > is more than one issue here.
> 
> Nailed the sucker:
> 
> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> index b4c1bc7..9d7284a 100644
> --- a/kernel/irq/chip.c
> +++ b/kernel/irq/chip.c
> @@ -820,6 +820,18 @@ __irq_do_set_handler(struct irq_desc *desc, irq_flow_handler_t handle,
>  	desc->name = name;
>  
>  	if (handle != handle_bad_irq && is_chained) {
> +		int ret;
> +
> +		ret = __irq_set_trigger(desc,
> +					irqd_get_trigger_type(&desc->irq_data));
> +		WARN_ON(ret);
> +		/*
> +		 * This is beyond ugly: .set_type may have overridden
> +		 * the flow, not not knowing that we're dealing with a
> +		 * chained handler. Reset it here because we know
> +		 * better.
> +		 */

Thanks for this Marc!

But it makes me (author of pinctrl-msm) wonder, am I supposed to not
implement .set_type like this for handling the transition between edge
and level handlers?

Regards,
Bjorn

Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons

[Marc Zyngier]

On Thu, 11 Aug 2016 14:23:53 -0700
Bjorn Andersson <bjorn.andersson@linaro.org> wrote:

Hi Bjorn,

> On Thu 11 Aug 05:46 PDT 2016, Marc Zyngier wrote:
> 
> > On 11/08/16 10:47, Jon Hunter wrote:  
> > > 
> > > On 11/08/16 09:37, Marc Zyngier wrote:  
> > >> On 08/08/16 22:48, Linus Walleij wrote:  
> > >>> On Sat, Aug 6, 2016 at 1:45 AM, John Stultz <john.stultz@linaro.org> wrote:
> > >>>  
> > >>>> @@ -614,7 +615,11 @@ unsigned int irq_create_fwspec_mapping(struct
> > >>>> irq_fwspec *fwspec)
> > >>>>                  * it now and return the interrupt number.
> > >>>>                  */
> > >>>>                 if (irq_get_trigger_type(virq) == IRQ_TYPE_NONE) {
> > >>>> -                       irq_set_irq_type(virq, type);
> > >>>> +                       irq_data = irq_get_irq_data(virq);
> > >>>> +                       if (!irq_data)
> > >>>> +                               return 0;
> > >>>> +
> > >>>> +                       irqd_set_trigger_type(irq_data, type);
> > >>>>                         return virq;
> > >>>>                 }
> > >>>>
> > >>>> If I revert just that, it works again.  
> > >>>
> > >>> This makes my platform work too.
> > >>> Tested-by: Linus Walleij <linus.walleij@linaro.org>  
> > >>
> > >> Hmmm. I'm now booting your kernel on the APQ8060, and reverting this
> > >> hunk doesn't fix it for me. I'm confused...
> > >>
> > >> The interesting part is this:
> > >> 109:     100000          0   msmgpio  88 Level     (null)  
> > > 
> > > 88 is the pm8058 parent interrupt and so I am surprised you would even
> > > see this in /proc/interrupts as it should be a chained interrupt, right?
> > > 
> > > Are you seeing this with all the ethernet updates for the APQ8060 in
> > > Linus' branch? I am curious what you see with stock v4.8-rc1 and if
> > > interrupts work ok with the change I had proposed. Hard to tell if there
> > > is more than one issue here.  
> > 
> > Nailed the sucker:
> > 
> > diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> > index b4c1bc7..9d7284a 100644
> > --- a/kernel/irq/chip.c
> > +++ b/kernel/irq/chip.c
> > @@ -820,6 +820,18 @@ __irq_do_set_handler(struct irq_desc *desc, irq_flow_handler_t handle,
> >  	desc->name = name;
> >  
> >  	if (handle != handle_bad_irq && is_chained) {
> > +		int ret;
> > +
> > +		ret = __irq_set_trigger(desc,
> > +					irqd_get_trigger_type(&desc->irq_data));
> > +		WARN_ON(ret);
> > +		/*
> > +		 * This is beyond ugly: .set_type may have overridden
> > +		 * the flow, not not knowing that we're dealing with a
> > +		 * chained handler. Reset it here because we know
> > +		 * better.
> > +		 */  
> 
> Thanks for this Marc!
> 
> But it makes me (author of pinctrl-msm) wonder, am I supposed to not
> implement .set_type like this for handling the transition between edge
> and level handlers?

You definitely need to implement .set_type and set the flow handler
the way you do it, and there is hardly anything an irqchip driver can do
to detect that case.

The main issue is that as far as the core code is concerned, the
chained handler is just another flow handler. It just happen to be
provided by another irqchip.

We used to call .set_type early, and a driver like yours would set the
flow corresponding to the trigger of the interrupt. Later, the
secondary irqchip would then call irq_set_chained_handler_and_data(),
which would override the flow with the custom one. Now, we call it much
later, after the custom flow handler has been assigned. Kaboom, we
end-up calling the chained_action handler through one of the normal
flows, instead of going into our special flow.

We *could* make irq_set_handler_locked() check for this condition
though (testing for the chained_action pointer), probably at the cost of
un-inlining it.

I'll try to put something together next week, and see what sticks.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny.

Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons

[Linus Walleij]

On Thu, Aug 11, 2016 at 10:37 AM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> On 08/08/16 22:48, Linus Walleij wrote:
>> On Sat, Aug 6, 2016 at 1:45 AM, John Stultz <john.stultz@linaro.org> wrote:
>>
>>> @@ -614,7 +615,11 @@ unsigned int irq_create_fwspec_mapping(struct
>>> irq_fwspec *fwspec)
>>>                  * it now and return the interrupt number.
>>>                  */
>>>                 if (irq_get_trigger_type(virq) == IRQ_TYPE_NONE) {
>>> -                       irq_set_irq_type(virq, type);
>>> +                       irq_data = irq_get_irq_data(virq);
>>> +                       if (!irq_data)
>>> +                               return 0;
>>> +
>>> +                       irqd_set_trigger_type(irq_data, type);
>>>                         return virq;
>>>                 }
>>>
>>> If I revert just that, it works again.
>>
>> This makes my platform work too.
>> Tested-by: Linus Walleij <linus.walleij@linaro.org>
>
> Hmmm. I'm now booting your kernel on the APQ8060, and reverting this
> hunk doesn't fix it for me. I'm confused...

Not quite following ... If you build the branch apq8060-dragonboard
which has these patches:

ARM: dts: MSM8660 remove flags from SPMI/MPP IRQs
Revert "irqdomain: Don't set type when mapping an IRQ
iio: pressure: bmp280: fix runtime suspend/resume crash
DO NOT MERGE: ARM: qcom: uglyfix

(I'm sorry about the two patches in the bottom needed to boot
this platform...)

You should be seeing e.g. the power button IRQs appear if you press
the "power" button close to the DC connector.

If you remove the revert, the interrupts are gone. For me, atleaset,
no reaction to the power button.

The issue is the same with any PMIC IRQ I try to use: sensors,
ethernet, SD card slot ... the SD card slot can also be tested right
off just like the power button. Just insert/eject an SD card in the
primary card slot.

Yours,
Linus Walleij