Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V3 0/8] IOMMU probe deferral support
From: Sricharan @ 2016-10-26 14:44 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <60ee8066-f167-e9df-ae3e-4138f1133bad@arm.com>

Hi Robin,

>On 04/10/16 18:03, Sricharan R wrote:
>> Initial post from Laurent Pinchart[1]. This is
>> series calls the dma ops configuration for the devices
>> at a generic place so that it works for all busses.
>> The dma_configure_ops for a device is now called during
>> the device_attach callback just before the probe of the
>> bus/driver is called. Similarly dma_deconfigure is called during
>> device/driver_detach path.
>>
>>
>> pci_bus_add_devices    (platform/amba)(_device_create/driver_register)
>>        |                         |
>> pci_bus_add_device     (device_add/driver_register)
>>        |                         |
>> device_attach           device_initial_probe
>>        |                         |
>> __device_attach_driver    __device_attach_driver
>>        |
>> driver_probe_device
>>        |
>> really_probe
>>        |
>> dma_configure
>>
>>  Similarly on the device/driver_unregister path __device_release_driver is
>>  called which inturn calls dma_deconfigure.
>>
>>  If the ACPI bus code follows the same, we can add acpi_dma_configure
>>  at the same place as of_dma_configure.
>>
>>  This series is based on the recently merged Generic DT bindings for
>>  PCI IOMMUs and ARM SMMU from Robin Murphy robin.murphy at arm.com [2]
>>
>>  This time tested this with platform and pci device for probe deferral
>>  and reprobe on arm64 based platform. There is an issue on the cleanup
>>  path for arm64 though, where there is WARN_ON if the dma_ops is reset while
>>  device is attached to an domain in arch_teardown_dma_ops.
>>  But with iommu_groups created from the iommu driver, the device is always
>>  attached to a domain/default_domain. So so the WARN has to be removed/handled
>>  probably.
>
>I've finally got the chance to take a proper look at this series (sorry
>for the delay). First up, with these patches on top of 4.9-rc2, my
>little script for unbinding some PCI devices and rebinding them to VFIO
>now goes horribly, horribly wrong.
>
>Robin.
>

   Thanks for looking in to this.
    I was trying to reproduce the below with a command like this in my setup.

echo 0002\:00\:00.0 >  /sys/bus/pci/devices/0002\:00\:000.0/driver/unbind0.0/driver/unbind
echo 0x17cb 0x0104 > /sys/bus/pci/drivers/vfio-pci/new_id

But for me the unbind and reconfiguring/adding the iommus_ops to vfio-pci did
succeed, although the WARN_ON in arch_teardown_dma_ops was there, that
could be suppresed.  The vfio_pci_probe was not going through because
 the pci hdr_type was not PCI_HEADER_TYPE_NORMAL.
 But anyways iommu unbind/rebind seemed to be going through.

If i can get what your script is doing, i can try that and see what happens.

Regards,
  Sricharan



>[   39.901592] iommu: Removing device 0000:08:00.0 from group 0
>[   39.907383] ------------[ cut here ]------------
>[   39.911969] WARNING: CPU: 0 PID: 174 at
>arch/arm64/mm/dma-mapping.c:856 arch_teardown_dma_ops+0x48/0x68
>[   39.921266] Modules linked in:
>[   39.924290]
>[   39.925766] CPU: 0 PID: 174 Comm: vfio Not tainted 4.9.0-rc2+ #1249
>[   39.931967] Hardware name: ARM Juno development board (r1) (DT)
>[   39.937826] task: ffffffc975ee9900 task.stack: ffffffc974d60000
>[   39.943687] PC is at arch_teardown_dma_ops+0x48/0x68
>[   39.948603] LR is at arch_teardown_dma_ops+0x34/0x68
>[   39.953516] pc : [<ffffff80080948f8>] lr : [<ffffff80080948e4>]
>pstate: 60000145
>[   39.960834] sp : ffffffc974d63ca0
>[   39.964112] x29: ffffffc974d63ca0 x28: ffffffc974d60000
>[   39.969377] x27: ffffff80088a2000 x26: 0000000000000040
>[   39.974642] x25: 0000000000000123 x24: ffffffc976a48918
>[   39.979907] x23: ffffffc974d63eb8 x22: ffffff8008db7550
>[   39.985171] x21: 000000000000000d x20: ffffffc9763e9b50
>[   39.990435] x19: ffffffc9763f20a0 x18: 0000000000000010
>[   39.995699] x17: 0000007f99c18018 x16: ffffff80080c0580
>[   40.000964] x15: ffffff8008bb7000 x14: 000137c100013798
>[   40.006228] x13: ffffffffff000000 x12: ffffffffffffffff
>[   40.011492] x11: 0000000000000018 x10: 000000000000000d
>[   40.016757] x9 : 0000000040000000 x8 : 0000000000210d00
>[   40.022021] x7 : 00000049771bc000 x6 : 00000000001f17ed
>[   40.027286] x5 : ffffff80084c4208 x4 : 0000000000000080
>[   40.032551] x3 : ffffffc975ea9800 x2 : ffffffbf25d7aa50
>[   40.037815] x1 : 0000000000000000 x0 : 0000000000000080
>[   40.043078]
>[   40.044549] ---[ end trace 35c1e743d6e6c035 ]---
>[   40.049117] Call trace:
>[   40.051537] Exception stack(0xffffffc974d63ad0 to 0xffffffc974d63c00)
>[   40.057914] 3ac0:                                   ffffffc9763f20a0
>0000008000000000
>[   40.065668] 3ae0: ffffffc974d63ca0 ffffff80080948f8 ffffffbf25d7aa40
>ffffffc975ea9800
>[   40.073421] 3b00: ffffffc974d60000 000000000002fc80 ffffffc976801e00
>ffffffc974d60000
>[   40.081175] 3b20: ffffff80084c4208 0000000000000040 ffffff80088a2000
>ffffffc974d60000
>[   40.088928] 3b40: ffffff80084caf78 ffffffc974d60000 ffffffc974d60000
>ffffffc974d60000
>[   40.096682] 3b60: ffffffc975ea9800 ffffffc974d60000 0000000000000080
>0000000000000000
>[   40.104435] 3b80: ffffffbf25d7aa50 ffffffc975ea9800 0000000000000080
>ffffff80084c4208
>[   40.112188] 3ba0: 00000000001f17ed 00000049771bc000 0000000000210d00
>0000000040000000
>[   40.119941] 3bc0: 000000000000000d 0000000000000018 ffffffffffffffff
>ffffffffff000000
>[   40.127695] 3be0: 000137c100013798 ffffff8008bb7000 ffffff80080c0580
>0000007f99c18018
>[   40.135450] [<ffffff80080948f8>] arch_teardown_dma_ops+0x48/0x68
>[   40.141400] [<ffffff8008764a14>] of_dma_deconfigure+0xc/0x18
>[   40.147005] [<ffffff8008552804>] dma_deconfigure+0xc/0x18
>[   40.152353] [<ffffff800853ba10>] __device_release_driver+0x88/0x120
>[   40.158560] [<ffffff800853bacc>] device_release_driver+0x24/0x38
>[   40.164507] [<ffffff800853a868>] unbind_store+0xe8/0x110
>[   40.169767] [<ffffff8008539c70>] drv_attr_store+0x20/0x30
>[   40.175113] [<ffffff800823ab18>] sysfs_kf_write+0x48/0x58
>[   40.180458] [<ffffff8008239ea8>] kernfs_fop_write+0xb0/0x1d8
>[   40.186063] [<ffffff80081c507c>] __vfs_write+0x1c/0x100
>[   40.191237] [<ffffff80081c5e80>] vfs_write+0xa0/0x1b8
>[   40.196239] [<ffffff80081c7274>] SyS_write+0x44/0xa0
>[   40.201155] [<ffffff8008082ef0>] el0_svc_naked+0x24/0x28
>[   40.206703] vfio-pci 0000:08:00.0: Failed to setup iommu ops
>[   40.212382] vfio-pci: probe of 0000:08:00.0 failed with error -22
>[   40.228075] ------------[ cut here ]------------
>[   40.235263] WARNING: CPU: 1 PID: 174 at ./include/linux/kref.h:46
>kobject_get+0x64/0x88
>[   40.243181] Modules linked in:
>[   40.246201]
>[   40.247673] CPU: 1 PID: 174 Comm: vfio Tainted: G        W
>4.9.0-rc2+ #1249
>[   40.255076] Hardware name: ARM Juno development board (r1) (DT)
>[   40.260932] task: ffffffc975ee9900 task.stack: ffffffc974d60000
>[   40.266787] PC is at kobject_get+0x64/0x88
>[   40.270840] LR is at iommu_bus_notifier+0x40/0x110
>[   40.275577] pc : [<ffffff800834d20c>] lr : [<ffffff80084c3fd0>]
>pstate: 80000145
>[   40.282894] sp : ffffffc974d63c00
>[   40.286169] x29: ffffffc974d63c00 x28: ffffffc974d60000
>[   40.291431] x27: ffffff80088a2000 x26: 0000000000000040
>[   40.296692] x25: 0000000000000123 x24: ffffffc974c8f418
>[   40.301953] x23: 0000000000000006 x22: ffffffc9763f10a0
>[   40.307214] x21: ffffffc9763e9a00 x20: ffffffc9763f10a0
>[   40.312474] x19: ffffffc9763ebc80 x18: 0000007fd65069e0
>[   40.317734] x17: 0000007f8d0ae3c0 x16: ffffff80081c7230
>[   40.322995] x15: 0000007f8d136588 x14: ffffffffffffffff
>[   40.328255] x13: 0000000000000004 x12: 0000000000000030
>[   40.333515] x11: 0000000000000030 x10: 0101010101010101
>[   40.338775] x9 : feff716475687163 x8 : 7f7f7f7f7f7f7f7f
>[   40.344035] x7 : feff716475687163 x6 : ffffffc976abf400
>[   40.349295] x5 : ffffffc976abf400 x4 : 0000000000000000
>[   40.354555] x3 : ffffff80084c3f90 x2 : ffffffc9763ebcb8
>[   40.359814] x1 : 0000000000000001 x0 : ffffff8008d4f000
>[   40.365074]
>[   40.366542] ---[ end trace 35c1e743d6e6c036 ]---
>[   40.371107] Call trace:
>[   40.373523] Exception stack(0xffffffc974d63a30 to 0xffffffc974d63b60)
>[   40.379895] 3a20:                                   ffffffc9763ebc80
>0000008000000000
>[   40.387643] 3a40: ffffffc974d63c00 ffffff800834d20c ffffffc976812400
>ffffff8008237d94
>[   40.395391] 3a60: ffffffbf25d78940 ffffffc974d60000 ffffffc975e259d8
>0000000000005b81
>[   40.403139] 3a80: ffffffc974d60000 ffffff8008d4b31f ffffff8008b0f000
>ffffffc976811c80
>[   40.410887] 3aa0: ffffffc974d60000 ffffffc974d60000 ffffffc974d60000
>ffffff8008237000
>[   40.418634] 3ac0: ffffffc975e259d8 ffffff8008b1b9a8 ffffff8008d4f000
>0000000000000001
>[   40.426382] 3ae0: ffffffc9763ebcb8 ffffff80084c3f90 0000000000000000
>ffffffc976abf400
>[   40.434130] 3b00: ffffffc976abf400 feff716475687163 7f7f7f7f7f7f7f7f
>feff716475687163
>[   40.441877] 3b20: 0101010101010101 0000000000000030 0000000000000030
>0000000000000004
>[   40.449625] 3b40: ffffffffffffffff 0000007f8d136588 ffffff80081c7230
>0000007f8d0ae3c0
>[   40.457372] [<ffffff800834d20c>] kobject_get+0x64/0x88
>[   40.462455] [<ffffff80084c3fd0>] iommu_bus_notifier+0x40/0x110
>[   40.468227] [<ffffff80080da288>] notifier_call_chain+0x50/0x90
>[   40.473997] [<ffffff80080da694>] __blocking_notifier_call_chain+0x4c/0x90
>[   40.480713] [<ffffff80080da6ec>] blocking_notifier_call_chain+0x14/0x20
>[   40.487259] [<ffffff800853b9e4>] __device_release_driver+0x5c/0x120
>[   40.493460] [<ffffff800853bacc>] device_release_driver+0x24/0x38
>[   40.499402] [<ffffff800853a868>] unbind_store+0xe8/0x110
>[   40.504656] [<ffffff8008539c70>] drv_attr_store+0x20/0x30
>[   40.509997] [<ffffff800823ab18>] sysfs_kf_write+0x48/0x58
>[   40.515337] [<ffffff8008239ea8>] kernfs_fop_write+0xb0/0x1d8
>[   40.520936] [<ffffff80081c507c>] __vfs_write+0x1c/0x100
>[   40.526104] [<ffffff80081c5e80>] vfs_write+0xa0/0x1b8
>[   40.531100] [<ffffff80081c7274>] SyS_write+0x44/0xa0
>[   40.536011] [<ffffff8008082ef0>] el0_svc_naked+0x24/0x28
>[   40.541324] ata1.00: disabled
>[   40.544878] sd 0:0:0:0: [sda] Synchronizing SCSI cache
>[   40.550062] sd 0:0:0:0: [sda] Synchronize Cache(10) failed: Result:
>hostbyte=0x04 driverbyte=0x00
>[   40.558871] sd 0:0:0:0: [sda] Stopping disk
>[   40.563037] sd 0:0:0:0: [sda] Start/Stop Unit failed: Result:
>hostbyte=0x04 driverbyte=0x00
>[   40.586990] Unable to handle kernel paging request at virtual address
>0002003e
>[   40.594702] pgd = ffffffc974c80000
>[   40.598165] [0002003e] *pgd=00000009f5102003[   40.602241] ,
>*pud=00000009f5102003
>, *pmd=0000000000000000[   40.607694]
>[   40.609171] Internal error: Oops: 96000006 [#1] PREEMPT SMP
>[   40.614684] Modules linked in:
>[   40.617712] CPU: 3 PID: 174 Comm: vfio Tainted: G        W
>4.9.0-rc2+ #1249
>[   40.625118] Hardware name: ARM Juno development board (r1) (DT)
>[   40.630977] task: ffffffc975ee9900 task.stack: ffffffc974d60000
>[   40.636841] PC is at kobject_get+0x14/0x88
>[   40.640897] LR is at iommu_get_domain_for_dev+0x1c/0x48
>[   40.646068] pc : [<ffffff800834d1bc>] lr : [<ffffff80084c3dec>]
>pstate: 60000145
>[   40.653387] sp : ffffffc974d63c60
>[   40.656664] x29: ffffffc974d63c60 x28: ffffffc974d60000
>[   40.661928] x27: ffffff80088a2000 x26: 0000000000000040
>[   40.667193] x25: 0000000000000123 x24: ffffffc974c8f418
>[   40.672457] x23: ffffffc974d63eb8 x22: ffffff8008dab568
>[   40.677720] x21: 000000000000000d x20: ffffff8008dab568
>[   40.682984] x19: 0000000000020002 x18: 0000000000000000
>[   40.688246] x17: 0000000000000007 x16: 0000000000000001
>[   40.693509] x15: ffffffc974cd091c x14: ffffffffffffffff
>[   40.698773] x13: ffffffc974cd01cd x12: 0000000000000030
>[   40.704036] x11: 0101010101010101 x10: 7f7f7f7f7f7f7f7f
>[   40.709300] x9 : 0000000040000000 x8 : 0000000000210d00
>[   40.714563] x7 : ffffffc975f95018 x6 : 0000000000000000
>[   40.719826] x5 : 0000000000000000 x4 : ffffffc9763f1210
>[   40.725088] x3 : 0000000000000000 x2 : ffffffc9763f10e8
>[   40.730351] x1 : 0000000000000000 x0 : 0000000000020002
>[   40.735613]
>[   40.737085] Process vfio (pid: 174, stack limit = 0xffffffc974d60020)
>[   40.743460] Stack: (0xffffffc974d63c60 to 0xffffffc974d64000)
>[   40.749150] 3c60: ffffffc974d63c80 ffffff80084c3dec ffffffc9763e9a00
>ffffff80085377a4
>[   40.756904] 3c80: ffffffc974d63ca0 ffffff80080948c4 ffffffc9763f10a0
>ffffff8008dab568
>[   40.764658] 3ca0: ffffffc974d63cc0 ffffff8008764a14 ffffffc9763f10a0
>ffffff8008dab568
>[   40.772411] 3cc0: ffffffc974d63cd0 ffffff8008552804 ffffffc974d63ce0
>ffffff800853ba10
>[   40.780165] 3ce0: ffffffc974d63d00 ffffff800853bacc ffffffc9763f1100
>ffffffc9763f10a0
>[   40.787918] 3d00: ffffffc974d63d20 ffffff800853a868 ffffff8008d68f18
>ffffffc9763f10a0
>[   40.795672] 3d20: ffffffc974d63d50 ffffff8008539c70 000000000000000d
>ffffffc974c8f400
>[   40.803425] 3d40: ffffffc9757d5880 0000000000000000 ffffffc974d63d60
>ffffff800823ab18
>[   40.811178] 3d60: ffffffc974d63d70 ffffff8008239ea8 ffffffc974d63dc0
>ffffff80081c507c
>[   40.818931] 3d80: 000000000000000d 0000000000000000 ffffffc974c8f100
>ffffffc974d63eb8
>[   40.826684] 3da0: 000000001285f6a0 0000000000000015 0000000000000123
>ffffff80080bf6ac
>[   40.834437] 3dc0: ffffffc974d63e40 ffffff80081c5e80 000000000000000d
>0000000000000000
>[   40.842190] 3de0: ffffffc974d63e30 ffffff80080c087c ffffffc974d63e20
>ffffff80081c5c0c
>[   40.849943] 3e00: ffffffc974c8f100 0000000000000001 ffffffc974c8f100
>ffffffc974d63eb8
>[   40.857696] 3e20: ffffffc974d63e40 ffffff80081c5f48 000000000000000d
>ffffffc974c8f100
>[   40.865450] 3e40: ffffffc974d63e80 ffffff80081c7274 ffffffc974c8f100
>ffffffc974c8f100
>[   40.873203] 3e60: 000000001285f6a0 000000000000000d 0000000060000000
>0000000000000000
>[   40.880956] 3e80: 0000000000000000 ffffff8008082ef0 0000000000000000
>0000000000000001
>[   40.888709] 3ea0: ffffffffffffffff 0000007f8d0ae3dc 0000000000000000
>0000000000000000
>[   40.896461] 3ec0: 0000000000000001 000000001285f6a0 000000000000000d
>0000000000000000
>[   40.904215] 3ee0: ae2e2e2e3f464b49 0000000000000000 000000001285f6b0
>39322f392f2f2f2f
>[   40.911968] 3f00: 0000000000000040 fefefeff2f2d2f2f 7f7f7f7f7f7f7f7f
>0101010101010101
>[   40.919721] 3f20: 0000000000000002 0000000000000004 ffffffffffffffff
>0000007f8d136588
>[   40.927474] 3f40: 0000000000000000 0000007f8d0ae3c0 0000007fd65069e0
>00000000004ee000
>[   40.935226] 3f60: 0000000000000001 000000001285f6a0 000000000000000d
>0000000000000001
>[   40.942980] 3f80: 0000000000000020 000000001285eed8 00000000004ba158
>0000000000000000
>[   40.950732] 3fa0: 0000000000000000 0000007fd6507f30 000000000040e74c
>0000007fd6507130
>[   40.958485] 3fc0: 0000007f8d0ae3dc 0000000060000000 0000000000000001
>0000000000000040
>[   40.966238] 3fe0: 0000000000000000 0000000000000000 0000002000103a00
>4000000010000000
>[   40.973986] Call trace:
>[   40.976405] Exception stack(0xffffffc974d63a90 to 0xffffffc974d63bc0)
>[   40.982780] 3a80:                                   0000000000020002
>0000008000000000
>[   40.990533] 3aa0: ffffffc974d63c60 ffffff800834d1bc ffffffc974d63ae0
>ffffff80085377a4
>[   40.998287] 3ac0: ffffffc974d63b10 ffffff8008537424 ffffffc975e3ac28
>ffffffc975e3ac38
>[   41.006041] 3ae0: ffffffc974d63b30 ffffff80081737cc ffffffc975e3ac38
>ffffff8008da62c0
>[   41.013794] 3b00: ffffffc975e98100 ffffff80085401b0 0000000000000001
>ffffff8008540a08
>[   41.021547] 3b20: 00000000000036b8 0000000000000040 0000000000020002
>0000000000000000
>[   41.029300] 3b40: ffffffc9763f10e8 0000000000000000 ffffffc9763f1210
>0000000000000000
>[   41.037053] 3b60: 0000000000000000 ffffffc975f95018 0000000000210d00
>0000000040000000
>[   41.044805] 3b80: 7f7f7f7f7f7f7f7f 0101010101010101 0000000000000030
>ffffffc974cd01cd
>[   41.052558] 3ba0: ffffffffffffffff ffffffc974cd091c 0000000000000001
>0000000000000007
>[   41.060311] [<ffffff800834d1bc>] kobject_get+0x14/0x88
>[   41.065398] [<ffffff80084c3dec>] iommu_get_domain_for_dev+0x1c/0x48
>[   41.071607] [<ffffff80080948c4>] arch_teardown_dma_ops+0x14/0x68
>[   41.077556] [<ffffff8008764a14>] of_dma_deconfigure+0xc/0x18
>[   41.083161] [<ffffff8008552804>] dma_deconfigure+0xc/0x18
>[   41.088509] [<ffffff800853ba10>] __device_release_driver+0x88/0x120
>[   41.094715] [<ffffff800853bacc>] device_release_driver+0x24/0x38
>[   41.100663] [<ffffff800853a868>] unbind_store+0xe8/0x110
>[   41.105922] [<ffffff8008539c70>] drv_attr_store+0x20/0x30
>[   41.111268] [<ffffff800823ab18>] sysfs_kf_write+0x48/0x58
>[   41.116612] [<ffffff8008239ea8>] kernfs_fop_write+0xb0/0x1d8
>[   41.122216] [<ffffff80081c507c>] __vfs_write+0x1c/0x100
>[   41.127390] [<ffffff80081c5e80>] vfs_write+0xa0/0x1b8
>[   41.132391] [<ffffff80081c7274>] SyS_write+0x44/0xa0
>[   41.137307] [<ffffff8008082ef0>] el0_svc_naked+0x24/0x28
>[   41.142567] Code: 910003fd f9000bf3 aa0003f3 b4000180 (3940f000)
>[   41.148667] ---[ end trace 35c1e743d6e6c037 ]---
>Segmentation fault
>/ #
>
>
>_______________________________________________
>linux-arm-kernel mailing list
>linux-arm-kernel at lists.infradead.org
>http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH 4/9] pinctrl: meson: allow gpio to request irq
From: Linus Walleij @ 2016-10-26 14:44 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1477491816.2482.160.camel@baylibre.com>

On Wed, Oct 26, 2016 at 4:23 PM, Jerome Brunet <jbrunet@baylibre.com> wrote:
> [Me]
>> We usually refer to the local numberspace on the GPIO controller
>> as "offsets", so line offsets 0...31 on a gpiochip with 31 lines.
>>
>> The ngpio in struct gpio_chip is the number of lines on that
>> controller,
>> and should nominally map 1:1 to hwirq sources.
>
> Indeed it should be the the case, and for meson, it is pretty close.
> The irqchip controller provide a number of hwirq. Each hwirq maps to
> one, and only one, pin. But since not every pins are connected to the
> irqchip controller, the opposite is not true.
>
> Taking an example with 16 gpios, here is what it could look like with
> the exception we have on meson :
>
> gpio offset [ 0  1  2  3  4  5  6  7  8  9  10 11 12 13 14 15 ]
> hwirq num   [ 0  1  2  3] NC NC[4  5  6  7  8  9  10]NC NC NC
>
> Like gpio offset are used (internally) in the driver to find
> appropriate gpio registers and bit, the hwirq has a meaning too.
> It is the setting you put in the channel multiplexer of the controller
> to select the proper pin to spy on.
>
> In the end, these gpio offset and hwirq number are different. I would
> prefer to have hwirq == gpio and go your way, it would make my life
> easier, but I don't see how it would work.
>
> The irqchip controller cares only about the hwirq number. You can
> actually request an interrupt directly to the controller by asking the
> proper hwirq number (in DT for example), without involving the gpio
> driver (tested).
>
> The relation between the pins and the interrupt number is provided by
> the manufacturer in the Datasheet [1], in the section GPIO Interrupt.

I think I kind of get it.

This reminds me of recent patches to the generic GPIOLIB_IRQCHIP
where we made it possible to "mask" set of IRQ lines, saying
"these are in the irqdomain, but you cannot map them".

See
commit 79b804cb6af4f128b2c53f0887c02537a7eb5824
"gpiolib: Make it possible to exclude GPIOs from IRQ domain"
commit 96b2cca64fa3e1a31b573bb308b2944c802aacf3
"gpio: stmpe: forbid unused lines to be mapped as IRQs"

So what we do in the generic case is put a linear map over all
the lines/offsets, then punch holes in that map and say
"this and this and this can not be mapped as IRQ".

As you can see in _gpiochip_irqchip_add() we pre-map all
except those with irq_create_mapping().

Does this scheme fit with your usecase? I would guess so,
just that your domain is hierarchic, not simple/linear.

Maybe the takeaway is that your map does not need to
be "dense", i.e. every hwirq is in use. It can be sparse.
It is stored in a radix tree anyways.

> Looking at other gpio drivers, it is not uncommon to have some simple
> calculation to get from gpio offset to the hwirq number. I don't get
> what is the specific problem here ?

It's OK to use the offset vs hwirq.

I just misunderstand it as the global GPIO number, that is
not OK.

Yours,
Linus Walleij

^ permalink raw reply

* [PATCH 4/9] pinctrl: meson: allow gpio to request irq
From: Linus Walleij @ 2016-10-26 14:32 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1477491766.2482.159.camel@baylibre.com>

On Wed, Oct 26, 2016 at 4:22 PM, Jerome Brunet <jbrunet@baylibre.com> wrote:
> On Tue, 2016-10-25 at 20:20 +0200, Linus Walleij wrote:

>> However the semantic is such, that it is not necessary to call
>> to_irq()
>> before using an IRQ: the irqchip and gpiochip abstractions should be
>> orthogonal.
>
> Linus,
>
> They are orthogonal. You can request an irq from the irqchip controller
> without the gpiochip, like any other irq controller.

OK good, sorry if I'm stating the obvious.

> irq_create_mapping (and irq_create_fwspec_mapping) internally calls
> irq_find_mapping. So if the mapping already exist (the irq is already
> used before calling to_irq), the existing mapping will be returned. The
> mapping will be actually created only if needed. It seems to be in line
> with your explanation, no ?

Yes, but you want to call irq_create_mapping() in slowpath (irq setup)
and irq_find_mapping() in fastpath (irq handler). Else the first IRQ
may result in unwelcomed surprises.

> There is really a *lot* of gpio drivers which use irq_create_mapping in
> the to_irq callback, are these all wrong ?

Yes they are all wrong. They should all be using irq_find_mapping().

> If this should not be used, what should we all do instead ?

Call irq_create_mapping() in some other place.

Yours,
Linus Walleij

^ permalink raw reply

* [PATCH 4/9] pinctrl: meson: allow gpio to request irq
From: Jerome Brunet @ 2016-10-26 14:23 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CACRpkdba_iaOK61KiQGvJZYuz7-PiBgtN4vxv__tJLsZ8ZVL=w@mail.gmail.com>

On Tue, 2016-10-25 at 20:10 +0200, Linus Walleij wrote:
> On Tue, Oct 25, 2016 at 4:47 PM, Marc Zyngier <marc.zyngier@arm.com>
> wrote:
> > 
> > On 25/10/16 15:22, Jerome Brunet wrote:
> 
> > 
> > > 
> > > There is a few problems to guarantee that gpio == hwirq.
> > > 1. We have 2 instances of pinctrl, to guarantee that the linux
> > > gpio
> > > number == hwirq, we would have to guarantee the order in which
> > > they are
> > > probed. At least this my understanding
> > 
> > Maybe I wasn't clear enough, and my use of gpio is probably wrong.
> > So
> > Linux has a gpio number, which is obviously an abstract number
> > (just
> > like the Linux irq number). But the pad number, in the context of
> > given
> > SoC, is constant. So we have:
> > 
> > ????????pad->gpio
> > ????????hwirq->irq
> > 
> > Why can't you have pad == hwirq, always? This is already what you
> > have
> > in the irqchip driver. This would simplify a lot of things.
> 
> My thought as well.
> 
> We usually refer to the local numberspace on the GPIO controller
> as "offsets", so line offsets 0...31 on a gpiochip with 31 lines.
> 
> The ngpio in struct gpio_chip is the number of lines on that
> controller,
> and should nominally map 1:1 to hwirq sources.

Indeed it should be the the case, and for meson, it is pretty close.
The irqchip controller provide a number of hwirq. Each hwirq maps to
one, and only one, pin. But since not every pins are connected to the
irqchip controller, the opposite is not true.

Taking an example with 16 gpios, here is what it could look like with
the exception we have on meson :

gpio offset [ 0 ?1 ?2 ?3 ?4 ?5 ?6 ?7 ?8 ?9 ?10 11 12 13 14 15 ]
hwirq num ? [ 0 ?1 ?2 ?3] NC NC[4 ?5 ?6 ?7 ?8 ?9 ?10]NC NC NC

Like gpio offset are used (internally) in the driver to find
appropriate gpio registers and bit, the hwirq has a meaning too.
It is the setting you put in the channel multiplexer of the controller
to select the proper pin to spy on.

In the end, these gpio offset and hwirq number are different. I would
prefer to have hwirq == gpio and go your way, it would make my life
easier, but I don't see how it would work.

The irqchip controller cares only about the hwirq number. You can
actually request an interrupt directly to the controller by asking the
proper hwirq number (in DT for example), without involving the gpio
driver (tested).

The relation between the pins and the interrupt number is provided by
the manufacturer in the Datasheet [1], in the section GPIO Interrupt.

Looking at other gpio drivers, it is not uncommon to have some simple
calculation to get from gpio offset to the hwirq number. I don't get
what is the specific problem here ?

If I missed something, feel free to point it out.

[1]?http://dn.odroid.com/S805/Datasheet/S805_Datasheet%20V0.8%202015012
6.pdf
> 
> Yours,
> Linus Walleij

^ permalink raw reply

* [PATCH 4/9] pinctrl: meson: allow gpio to request irq
From: Jerome Brunet @ 2016-10-26 14:22 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CACRpkdbGo4BJOdzkgBrE9jT-rKodd4zssCnOtOuGS+OqV-Uc6Q@mail.gmail.com>

On Tue, 2016-10-25 at 20:20 +0200, Linus Walleij wrote:
> On Tue, Oct 25, 2016 at 5:31 PM, Jerome Brunet <jbrunet@baylibre.com>
> wrote:
> > 
> > On Tue, 2016-10-25 at 15:47 +0100, Marc Zyngier wrote:
> 
> > 
> > > 
> > > Is gpio_to_irq() supposed to allocate an interrupt? Or merely to
> > > report the existence of a mapping?
> 
> It should provide an IRQ corresponding to the gpio line, if possible.
> 
> However the semantic is such, that it is not necessary to call
> to_irq()
> before using an IRQ: the irqchip and gpiochip abstractions should be
> orthogonal.

Linus,

They are?orthogonal. You can request an irq from the irqchip controller
without the gpiochip, like any other irq controller.

> 
> This goes especially when using device tree or ACPI, where you
> may reference an IRQ from something modeled as irqchip, which
> is simultaneously a gpiochip.
> 
> > 
> > Linus, please correct me if I'm wrong,
> > .to_irq gets the linux gpio number and returns the linux virtual
> > irq
> > numbers, 0 if there is no interrupt.
> 
> Yes. But it may *or may not* be called before using the IRQ.
> 
> So it should look up or try to create a mapping on request, but not
> assume to have been called before using some line as IRQ.
> 
> The only thing you should assume to be called before an interrupt
> is put to use is the stuff in irqchip. So you have to do your
> dynamic irqdomain mapping elsewhere than .to_irq().

irq_create_mapping (and irq_create_fwspec_mapping) internally calls
irq_find_mapping. So if the mapping already exist (the irq is already
used before calling to_irq), the existing mapping will be returned. The
mapping will be actually created only if needed. It seems to be in line
with your explanation, no ?

There is really a *lot* of gpio drivers which use irq_create_mapping in
the to_irq callback, are these all wrong ?
If this should not be used, what should we all do instead ??

> 
> Yours,
> Linus Walleij

^ permalink raw reply

* [PATCH V3 4/8] drivers: platform: Configure dma operations at probe time
From: Robin Murphy @ 2016-10-26 14:07 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1475600632-21289-5-git-send-email-sricharan@codeaurora.org>

+Lorenzo

On 04/10/16 18:03, Sricharan R wrote:
> Configuring DMA ops at probe time will allow deferring device probe when
> the IOMMU isn't available yet. The dma_configure for the device is now called
> from the generic device_attach callback just before the bus/driver probe
> is called. This way, configuring the dma ops for the device would be called
> at the same place for all bus_types, hence the deferred probing mechanism
> should work for all buses as well.
> 
> pci_bus_add_devices    (platform/amba)(_device_create/driver_register)
>        |                         |
> pci_bus_add_device     (device_add/driver_register)
>        |                         |
> device_attach           device_initial_probe
>        |                         |
> __device_attach_driver    __device_attach_driver
>        |
> driver_probe_device
>        |
> really_probe
>        |
> dma_configure
> 
>  Similarly on the device/driver_unregister path __device_release_driver is
>  called which inturn calls dma_deconfigure.
> 
> Signed-off-by: Sricharan R <sricharan@codeaurora.org>
> ---
>  drivers/base/dd.c           | 10 ++++++++++
>  drivers/base/dma-mapping.c  | 11 +++++++++++
>  drivers/of/platform.c       |  4 ----
>  drivers/pci/probe.c         |  5 +----
>  include/linux/dma-mapping.h |  3 +++
>  5 files changed, 25 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index 16688f5..cfebd48 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -19,6 +19,7 @@
>  
>  #include <linux/device.h>
>  #include <linux/delay.h>
> +#include <linux/dma-mapping.h>
>  #include <linux/module.h>
>  #include <linux/kthread.h>
>  #include <linux/wait.h>
> @@ -353,6 +354,10 @@ static int really_probe(struct device *dev, struct device_driver *drv)
>  	if (ret)
>  		goto pinctrl_bind_failed;
>  
> +	ret = dma_configure(dev);
> +	if (ret)
> +		goto dma_failed;
> +
>  	if (driver_sysfs_add(dev)) {
>  		printk(KERN_ERR "%s: driver_sysfs_add(%s) failed\n",
>  			__func__, dev_name(dev));
> @@ -395,6 +400,8 @@ static int really_probe(struct device *dev, struct device_driver *drv)
>  	goto done;
>  
>  probe_failed:
> +	dma_deconfigure(dev);
> +dma_failed:
>  	if (dev->bus)
>  		blocking_notifier_call_chain(&dev->bus->p->bus_notifier,
>  					     BUS_NOTIFY_DRIVER_NOT_BOUND, dev);
> @@ -780,6 +787,9 @@ static void __device_release_driver(struct device *dev)
>  			dev->bus->remove(dev);
>  		else if (drv->remove)
>  			drv->remove(dev);
> +
> +		dma_deconfigure(dev);
> +
>  		devres_release_all(dev);
>  		dev->driver = NULL;
>  		dev_set_drvdata(dev, NULL);
> diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c
> index d799662..54e87f5 100644
> --- a/drivers/base/dma-mapping.c
> +++ b/drivers/base/dma-mapping.c
> @@ -10,6 +10,7 @@
>  #include <linux/dma-mapping.h>
>  #include <linux/export.h>
>  #include <linux/gfp.h>
> +#include <linux/of_device.h>
>  #include <linux/slab.h>
>  #include <linux/vmalloc.h>
>  
> @@ -166,6 +167,16 @@ void dmam_free_noncoherent(struct device *dev, size_t size, void *vaddr,
>  }
>  EXPORT_SYMBOL(dmam_free_noncoherent);
>  
> +int dma_configure(struct device *dev)
> +{
> +	return of_dma_configure(dev, dev->of_node);
> +}
> +
> +void dma_deconfigure(struct device *dev)
> +{
> +	of_dma_deconfigure(dev);
> +}
> +
>  #ifdef CONFIG_HAVE_GENERIC_DMA_COHERENT
>  
>  static void dmam_coherent_decl_release(struct device *dev, void *res)
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index 9cb7090..adbd77c 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -181,11 +181,9 @@ static struct platform_device *of_platform_device_create_pdata(
>  
>  	dev->dev.bus = &platform_bus_type;
>  	dev->dev.platform_data = platform_data;
> -	of_dma_configure(&dev->dev, dev->dev.of_node);
>  	of_msi_configure(&dev->dev, dev->dev.of_node);
>  
>  	if (of_device_add(dev) != 0) {
> -		of_dma_deconfigure(&dev->dev);
>  		platform_device_put(dev);
>  		goto err_clear_flag;
>  	}
> @@ -242,7 +240,6 @@ static struct amba_device *of_amba_device_create(struct device_node *node,
>  		dev_set_name(&dev->dev, "%s", bus_id);
>  	else
>  		of_device_make_bus_id(&dev->dev);
> -	of_dma_configure(&dev->dev, dev->dev.of_node);
>  
>  	/* Allow the HW Peripheral ID to be overridden */
>  	prop = of_get_property(node, "arm,primecell-periphid", NULL);
> @@ -536,7 +533,6 @@ static int of_platform_device_destroy(struct device *dev, void *data)
>  		amba_device_unregister(to_amba_device(dev));
>  #endif
>  
> -	of_dma_deconfigure(dev);
>  	of_node_clear_flag(dev->of_node, OF_POPULATED);
>  	of_node_clear_flag(dev->of_node, OF_POPULATED_BUS);
>  	return 0;
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 93f280d..85c9553 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1724,10 +1724,7 @@ static void pci_dma_configure(struct pci_dev *dev)
>  {
>  	struct device *bridge = pci_get_host_bridge_device(dev);
>  
> -	if (IS_ENABLED(CONFIG_OF) &&
> -		bridge->parent && bridge->parent->of_node) {
> -			of_dma_configure(&dev->dev, bridge->parent->of_node);
> -	} else if (has_acpi_companion(bridge)) {
> +	if (has_acpi_companion(bridge)) {
>  		struct acpi_device *adev = to_acpi_device_node(bridge->fwnode);
>  		enum dev_dma_attr attr = acpi_get_dma_attr(adev);

It seems a bit awkward leaving pci_dma_configure here, doing DMA
configuration at device add, after we've allegedly moved DMA
configuration to driver probe. Lorenzo, do you foresee any issues if
this probe-time of_dma_configure() path were to also multiplex
acpi_dma_configure() in future, such that everything would be back in
the same place eventually?

Conversely, is there actually any issue with leaving pci_dma_configure()
unchanged, and simply moving the call from pci_device_add() into
dma_configure()?

Robin.

> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> index 66533e1..2766dbe 100644
> --- a/include/linux/dma-mapping.h
> +++ b/include/linux/dma-mapping.h
> @@ -656,6 +656,9 @@ dma_mark_declared_memory_occupied(struct device *dev,
>  }
>  #endif /* CONFIG_HAVE_GENERIC_DMA_COHERENT */
>  
> +int dma_configure(struct device *dev);
> +void dma_deconfigure(struct device *dev);
> +
>  /*
>   * Managed DMA API
>   */
> 

^ permalink raw reply

* [PATCH 05/14] mfd: sun6i-prcm: Add sun8i analog codec as subnode
From: Lee Jones @ 2016-10-26 14:05 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <c74aea452bd1831f439dd67884cb2879e0849f03.1475571575.git.mylene.josserand@free-electrons.com>

On Tue, 04 Oct 2016, Myl?ne Josserand wrote:

> The sun8i audio codec is using PRCM registers to configure all the
> analog part of the audio codec. It is added as a subnode of the PRCM
> with his resource (offset of 0x1c0).
> 
> Signed-off-by: Myl?ne Josserand <mylene.josserand@free-electrons.com>
> ---
>  drivers/mfd/sun6i-prcm.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/mfd/sun6i-prcm.c b/drivers/mfd/sun6i-prcm.c
> index 011fcc5..e0c6bf5 100644
> --- a/drivers/mfd/sun6i-prcm.c
> +++ b/drivers/mfd/sun6i-prcm.c
> @@ -12,6 +12,8 @@
>  #include <linux/init.h>
>  #include <linux/of.h>
>  
> +#define SUN6I_AUDIO_CODEC_ANALOG	0x1c0
> +
>  struct prcm_data {
>  	int nsubdevs;
>  	const struct mfd_cell *subdevs;
> @@ -57,6 +59,14 @@ static const struct resource sun6i_a31_apb0_rstc_res[] = {
>  	},
>  };
>  
> +static const struct resource sun8i_adda_res[] = {
> +	{
> +		.start = SUN6I_AUDIO_CODEC_ANALOG,
> +		.end = 0x1c3,

This also needs defining.  No magic numbers please.

> +		.flags = IORESOURCE_MEM,
> +	},
> +};
> +
>  static const struct mfd_cell sun6i_a31_prcm_subdevs[] = {
>  	{
>  		.name = "sun6i-a31-ar100-clk",
> @@ -109,6 +119,12 @@ static const struct mfd_cell sun8i_a23_prcm_subdevs[] = {
>  		.num_resources = ARRAY_SIZE(sun6i_a31_apb0_rstc_res),
>  		.resources = sun6i_a31_apb0_rstc_res,
>  	},
> +	{
> +		.name = "sun8i-codec-analog",
> +		.of_compatible = "allwinner,sun8i-codec-analog",
> +		.num_resources = ARRAY_SIZE(sun8i_adda_res),
> +		.resources = sun8i_adda_res,
> +	},
>  };
>  
>  static const struct prcm_data sun6i_a31_prcm_data = {

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* [PATCH] mfd: axp20x-i2c: Add i2c-ids to fix module auto-loading
From: Lee Jones @ 2016-10-26 14:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161005155112.13774-2-hdegoede@redhat.com>

On Wed, 05 Oct 2016, Hans de Goede wrote:

> The i2c subsys does not load modules by compatible, only by
> i2c-id, with e.g. a modalias of: "i2c:axp209".
> 
> Populate the axp20x_i2c_id[] table with supported ids, so that
> module auto-loading will work.
> 
> Reported-by: Dennis Gilmore <dennis@ausil.us>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/mfd/axp20x-i2c.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)

Applied (for now), thanks.

> diff --git a/drivers/mfd/axp20x-i2c.c b/drivers/mfd/axp20x-i2c.c
> index b1b8658..d35a5fe 100644
> --- a/drivers/mfd/axp20x-i2c.c
> +++ b/drivers/mfd/axp20x-i2c.c
> @@ -69,10 +69,11 @@ static const struct of_device_id axp20x_i2c_of_match[] = {
>  };
>  MODULE_DEVICE_TABLE(of, axp20x_i2c_of_match);
>  
> -/*
> - * This is useless for OF-enabled devices, but it is needed by I2C subsystem
> - */
>  static const struct i2c_device_id axp20x_i2c_id[] = {
> +	{ "axp152", 0 },
> +	{ "axp202", 0 },
> +	{ "axp209", 0 },
> +	{ "axp221", 0 },
>  	{ },
>  };
>  MODULE_DEVICE_TABLE(i2c, axp20x_i2c_id);

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* [PATCH] mfd: axp20x-i2c: Add i2c-ids to fix module auto-loading
From: Lee Jones @ 2016-10-26 14:00 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161024104747.GC1440@katana>

On Mon, 24 Oct 2016, Wolfram Sang wrote:

> 
> > I was under the impression it was all but ready.
> 
> Then, I would have applied it.
> 
> > What are you waiting on?
> 
> Lee, I don't want to explain it *again*. Please re-read Kieran's last
> attempt.

I met with Kieran.  He's submitted a new version.

Here's hoping! ;)

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* [PATCH 7/7] mfd: tps65217: Fix mismatched interrupt number
From: Lee Jones @ 2016-10-26 13:56 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161021140106.21531-8-woogyom.kim@gmail.com>

On Fri, 21 Oct 2016, Milo Kim wrote:

> Enum value of 'tps65217_irq_type' is not matched with DT parsed hwirq
> number[*].
> 
> The MFD driver gets the IRQ data by referencing hwirq, but the value is
> different. So, irq_to_tps65217_irq() returns mismatched IRQ data.
> Eventually, the power button driver enables not PB but USB interrupt
> when it is probed.
> 
> According to the TPS65217 register map[**], USB interrupt is the LSB.
> This patch synchronizes TPS65217 IRQ index.
> 
> [*]  include/dt-bindings/mfd/tps65217.h
> [**] http://www.ti.com/lit/ds/symlink/tps65217.pdf
> 
> Signed-off-by: Milo Kim <woogyom.kim@gmail.com>
> ---
>  include/linux/mfd/tps65217.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/mfd/tps65217.h b/include/linux/mfd/tps65217.h
> index 4ccda89..75a3a5f 100644
> --- a/include/linux/mfd/tps65217.h
> +++ b/include/linux/mfd/tps65217.h
> @@ -235,9 +235,9 @@ struct tps65217_bl_pdata {
>  };
>  
>  enum tps65217_irq_type {
> -	TPS65217_IRQ_PB,
> -	TPS65217_IRQ_AC,
>  	TPS65217_IRQ_USB,
> +	TPS65217_IRQ_AC,
> +	TPS65217_IRQ_PB,
>  	TPS65217_NUM_IRQ
>  };

This is why using enum for these types of assignments is sometimes
dangerous.  It's probably best to be explicit.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* [PATCH 0/7] ARM: dts: support I2SE Duckbill device
From: Shawn Guo @ 2016-10-26 13:55 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <2694949.M3JJFNG6Xz@kerker>

On Mon, Oct 24, 2016 at 09:45:56PM +0200, Michael Heimpold wrote:
> Actually it would even be 11 files for 8 boards, thats right.
> There is no mechanism (no EEPROM, GPIO...) to automatically probe for the 
> device variants, and the "variant-forming daugther boards" are
> not switchable like the expansion boards e.g. for Raspberry Pi.
> Another point is, that this would really require to use U-Boot to apply
> the overlay (or another DT capable bootloader with this functionality).
> For booting very fast, the good old Freescale bootlets are still
> an option, which could not handle the overlays - at least at the moment...
> However, it still an option to have a second look...
> 
> To reduce the number of files, I see the following other approaches:
> - don't use the three "common" files which are included: -3 files (8 total)
> - don't mainline Duckbill EnOcean, Duckbill 485 and Duckbill SPI:
>   they are EOL, only Duckbill 2 variants are still sold;
>   however, since Duckbill (without 2) is/was already mainlined, we
>   should keep this, leaving 4 new variants + 1 old variant: -3 files (5 total)
> - only mainline Duckbill 2 (generic board) and maintain all other variants
>   in a private Github repo (this is what we are doing now): -3 files (2 total)
> 
> So, I'll discuss this again internally. Do you have any preference?

I'm fine with the second one.

Shawn

^ permalink raw reply

* [PATCH 4/7] ARM: dts: add I2SE Duckbill common definitions
From: Shawn Guo @ 2016-10-26 13:53 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1477164150-13553-5-git-send-email-mhei@heimpold.de>

On Sat, Oct 22, 2016 at 09:22:27PM +0200, Michael Heimpold wrote:
> +			pinctrl at 80018000 {
> +				pinctrl-names = "default";
> +				pinctrl-0 = <&hog_pins_a>;
> +
> +				mac0_phy_reset_pin: mac0_phy_reset_pin at 0 {

Please use hyphen instead of underscore in node name.

Shawn

> +					reg = <0>;
> +					fsl,pinmux-ids = <
> +						MX28_PAD_GPMI_ALE__GPIO_0_26    /* PHY Reset */
> +					>;
> +					fsl,drive-strength = <MXS_DRIVE_4mA>;
> +					fsl,voltage = <MXS_VOLTAGE_HIGH>;
> +					fsl,pull-up = <MXS_PULL_DISABLE>;
> +				};

^ permalink raw reply

* [PATCH v2] clocksource/arm_arch_timer: Map frame with of_io_request_and_map()
From: Marc Zyngier @ 2016-10-26 13:45 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161026073550.22918-1-sboyd@codeaurora.org>

On 26/10/16 08:35, Stephen Boyd wrote:
> Let's use the of_io_request_and_map() API so that the frame
> region is protected and shows up in /proc/iomem.
> 
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
> 
> Changes from v1:
>  * Check for IS_ERR instead
> 
>  drivers/clocksource/arm_arch_timer.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index 73c487da6d2a..723cc924d8d1 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -964,8 +964,9 @@ static int __init arch_timer_mem_init(struct device_node *np)
>  	}
>  
>  	ret= -ENXIO;
> -	base = arch_counter_base = of_iomap(best_frame, 0);
> -	if (!base) {
> +	base = arch_counter_base = of_io_request_and_map(best_frame, 0,
> +							 "arch_mem_timer");
> +	if (IS_ERR(base)) {
>  		pr_err("arch_timer: Can't map frame's registers\n");
>  		goto out;
>  	}
> 

Acked-by: Marc Zyngier <marc.zyngier@arm.com>

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

^ permalink raw reply

* [PATCH 1/2] of, numa: Add function to disable of_node_to_nid().
From: Robert Richter @ 2016-10-26 13:43 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1477431061-7258-2-git-send-email-ddaney.cavm@gmail.com>

On 25.10.16 14:31:00, David Daney wrote:
> From: David Daney <david.daney@cavium.com>
> 
> On arm64 NUMA kernels we can pass "numa=off" on the command line to
> disable NUMA.  A side effect of this is that kmalloc_node() calls to
> non-zero nodes will crash the system with an OOPS:
> 
> [    0.000000] [<fffffc00081bba84>] __alloc_pages_nodemask+0xa4/0xe68
> [    0.000000] [<fffffc00082163a8>] new_slab+0xd0/0x57c
> [    0.000000] [<fffffc000821879c>] ___slab_alloc+0x2e4/0x514
> [    0.000000] [<fffffc000823882c>] __slab_alloc+0x48/0x58
> [    0.000000] [<fffffc00082195a0>] __kmalloc_node+0xd0/0x2e0
> [    0.000000] [<fffffc00081119b8>] __irq_domain_add+0x7c/0x164
> [    0.000000] [<fffffc0008b75d30>] its_probe+0x784/0x81c
> [    0.000000] [<fffffc0008b75e10>] its_init+0x48/0x1b0
> .
> .
> .
> 
> This is caused by code like this in kernel/irq/irqdomain.c
> 
>     domain = kzalloc_node(sizeof(*domain) + (sizeof(unsigned int) * size),
>                   GFP_KERNEL, of_node_to_nid(of_node));
> 
> When NUMA is disabled, the concept of a node is really undefined, so
> of_node_to_nid() should unconditionally return NUMA_NO_NODE.
> 
> Add __of_force_no_numa() to allow of_node_to_nid() to be forced to
> return NUMA_NO_NODE.
> 
> The follow on patch will call this new function from the arm64 numa
> code.

Didn't that work before? numa=off just maps all mem to node 0. If mem
allocation is requested for another node it should just fall back to a
node with mem (node 0 then). I suspect there is something wrong with
the page initialization, see:

 http://www.spinics.net/lists/arm-kernel/msg535191.html
 https://bugzilla.redhat.com/show_bug.cgi?id=1387793

What is the complete oops?

So I think k*alloc_node() must be able to handle requests to
non-existing nodes. Otherwise your fix is incomplete, assume a failed
of_numa_init() causing a dummy init but still some devices reporting a
node.

-Robert

^ permalink raw reply

* [PATCH v14 4/9] acpi/arm64: Add GTDT table parse driver
From: Fu Wei @ 2016-10-26 13:41 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <57010864-b2f0-d230-3138-8ace484acb04@arm.com>

Hi Marc,

On 26 October 2016 at 20:11, Marc Zyngier <marc.zyngier@arm.com> wrote:
> On 26/10/16 12:10, Fu Wei wrote:
>> Hi Mark,
>>
>> On 21 October 2016 at 00:37, Mark Rutland <mark.rutland@arm.com> wrote:
>>> Hi,
>>>
>>> As a heads-up, on v4.9-rc1 I see conflicts at least against
>>> arch/arm64/Kconfig. Luckily git am -3 seems to be able to fix that up
>>> automatically, but this will need to be rebased before the next posting
>>> and/or merging.
>>>
>>> On Thu, Sep 29, 2016 at 02:17:12AM +0800, fu.wei at linaro.org wrote:
>>>> +static int __init map_gt_gsi(u32 interrupt, u32 flags)
>>>> +{
>>>> +     int trigger, polarity;
>>>> +
>>>> +     if (!interrupt)
>>>> +             return 0;
>>>
>>> Urgh.
>>>
>>> Only the secure interrupt (which we do not need) is optional in this
>>> manner, and (hilariously), zero appears to also be a valid GSIV, per
>>> figure 5-24 in the ACPI 6.1 spec.
>>>
>>> So, I think that:
>>>
>>> (a) we should not bother parsing the secure interrupt
>>
>> If I understand correctly, from this point of view, kernel don't
>> handle the secure interrupt.
>> But the current arm_arch_timer driver still enable/disable/request
>> PHYS_SECURE_PPI
>> with PHYS_NONSECURE_PPI.
>> That means we still need to parse the secure interrupt.
>> Please correct me, if I misunderstand something? :-)
>
> That's because we can use the per-cpu timer when 32bit Linux is running
> on the secure side (and we cannot distinguish between secure and
> non-secure at runtime). ACPI is 64bit only, and Linux on 64bit isn't
> supported on the secure side, so only registering the non-secure timer
> is perfectly acceptable.

Great thanks for your explanation :-)
So we just don't need to fill arch_timer_ppi[PHYS_SECURE_PPI] , just skip it.

>
> Thanks,
>
>         M.
> --
> Jazz is not dead. It just smells funny...



-- 
Best regards,

Fu Wei
Software Engineer
Red Hat

^ permalink raw reply

* [PATCH v4 0/7] Add R8A7743/SK-RZG1M board support
From: Geert Uytterhoeven @ 2016-10-26 13:27 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1a85f851-29ec-1c14-8905-8b0645f5bb1c@cogentembedded.com>

On Wed, Oct 26, 2016 at 3:23 PM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> On 10/26/2016 03:08 PM, Geert Uytterhoeven wrote:
>>>    Here's the set of 8 patches against Simon Horman's 'renesas.git'
>>> repo's
>>> 'renesas-devel-20161021-v4.9-rc1' tag. I'm adding the device tree support
>>> for
>>> the R8A7743-based SK-RZG1M board. The SoC is close to R8A7791 and the
>>> board
>>> seems identical to the R8A7791/Porter board. The device tree patches
>>> depend on
>>> the R8A7743 CPG/MSSR driver series just posted in order to compile and
>>> work.
>>
>>
>> They depend only on "[PATCH v3 1/2] ARM: shmobile: r8a7743: add CPG clock
>> index macros" of that series, right?
>>
>> "[PATCH v3 2/2] clk: renesas: cpg-mssr: add R8A7743 support" is not
>> needed,
>
>    How would "clocks" props _work_ without this patch?

Sorry, I was focusing too much on "compile"...

Got my coke, switching brain to overdrive mode...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* [PATCH v4 0/7] Add R8A7743/SK-RZG1M board support
From: Sergei Shtylyov @ 2016-10-26 13:23 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAMuHMdUNgnYc1O7DATNH1r9SQ72eqtxwFvqcen08Tq+Er3LuvQ@mail.gmail.com>

On 10/26/2016 03:08 PM, Geert Uytterhoeven wrote:

>>    Here's the set of 8 patches against Simon Horman's 'renesas.git' repo's
>> 'renesas-devel-20161021-v4.9-rc1' tag. I'm adding the device tree support for
>> the R8A7743-based SK-RZG1M board. The SoC is close to R8A7791 and the board
>> seems identical to the R8A7791/Porter board. The device tree patches depend on
>> the R8A7743 CPG/MSSR driver series just posted in order to compile and work.
>
> They depend only on "[PATCH v3 1/2] ARM: shmobile: r8a7743: add CPG clock
> index macros" of that series, right?
>
> "[PATCH v3 2/2] clk: renesas: cpg-mssr: add R8A7743 support" is not needed,

    How would "clocks" props _work_ without this patch?

> and introduces an additional dependency on "[PATCH 1/3] clk: renesas:
> cpg-mssr: add common R-Car Gen2 support".

    Yes, of course...

> Gr{oetje,eeting}s,
>
>                         Geert

MBR, Sergei

^ permalink raw reply

* [PATCH v3] drivers: psci: PSCI checker module
From: Lorenzo Pieralisi @ 2016-10-26 13:17 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161025183436.GF3716@linux.vnet.ibm.com>

On Tue, Oct 25, 2016 at 11:34:36AM -0700, Paul E. McKenney wrote:

[...]

> > > +static int __init psci_checker(void)
> > > +{
> > > +	int ret;
> > > +
> > > +	/*
> > > +	 * Since we're in an initcall, we assume that all the CPUs that all
> > > +	 * CPUs that can be onlined have been onlined.
> > > +	 *
> > > +	 * The tests assume that hotplug is enabled but nobody else is using it,
> > > +	 * otherwise the results will be unpredictable. However, since there
> > > +	 * is no userspace yet in initcalls, that should be fine.
> > 
> > I do not think it is. If you run a kernel with, say,
> > CONFIG_LOCK_TORTURE_TEST, cpus may disappear from the radar while
> > running the PSCI checker test itself; that at least would confuse the
> > checker, and that's just an example.
> > 
> > I added Paul to check what are the assumptions behind the torture test
> > hotplug tests, in particular if there are any implicit assumptions for
> > it to work (ie it is the only kernel test hotplugging cpus in and out
> > (?)), what I know is that the PSCI checker assumptions are not correct.
> 
> Both CONFIG_LOCK_TORTURE_TEST and CONFIG_RCU_TORTURE_TEST can and will
> hotplug CPUs.  The locktorture.onoff_holdoff and rcutorture.onoff_holdoff
> kernel parameters can delay the start of CPU-hotplug testing, and in
> my testing I set this delay to 30 seconds after boot.
> 
> One approach would be to make your test refuse to run if either of
> the lock/RCU torture tests was running.  Or do what Lorenzo suggests
> below.  The torture tests aren't crazy enough to offline the last CPU.
> Though they do try, just for effect, in cases where the last CPU is
> marked cpu_is_hotpluggable().  ;-)

Thank you Paul. I have an additional question though. Is there any
implicit assumption in LOCK/RCU torture tests whereby nothing else
in the kernel is hotplugging cpus in/out (through cpu_down()/up())
while they are running ?

I am asking because that's the main reason behind my query. Those tests
hotplug cpus in and out through cpu_down/up() but AFAICS nothing
prevents another piece of code in the kernel to call those functions and
the tests may just fail in that case (ie trying to cpu_down() a cpu
that is not online).

Are false negatives contemplated (or I am missing something) ?

I just would like to understand if what this patch currently does
is safe and sound. I think that calling cpu_down() and cpu_up()
is always safe, but the test can result in false negatives if
other kernel subsystems (eg LOCK torture test) are calling those
APIs in parallel (because cpu_down()/cpu_up() calls can fail - eg
trying to cpu_down() a cpu that is not online any longer, since it
was taken down by another kernel control path), that's the question
I have.

Thanks !
Lorenzo

> 
> 						Thanx, Paul
> 
> > There is something simple you can do to get this "fixed".
> > 
> > You can use the new API James implemented for hibernate,
> > that allows you to disable (ie PSCI CPU OFF) all "secondary" cpus
> > other than the primary one passed in as parameter:
> > 
> > freeze_secondary_cpus(int primary);
> > 
> > that function will _cpu_down() all online cpus other than "primary"
> > in one go, without any interference allowed from other bits of the
> > kernel. It requires an enable_nonboot_cpus() counterpart, and you
> > can do that for every online cpus you detect (actually you can even
> > avoid using the online cpu mask and use the present mask to carry
> > out the test). If there is a resident trusted OS you can just
> > trigger the test with primary == tos_resident_cpu, since all
> > others are bound to fail (well, you can run them and check they
> > do fail, it is a checker after all).
> > 
> > You would lose the capability of hotplugging a "cluster" at a time, but
> > I do not think it is a big problem, the test above would cover it
> > and more importantly, it is safe to execute.
> > 
> > Or we can augment the torture test API to restrict the cpumask
> > it actually uses to offline/online cpus (I am referring to
> > torture_onoff(), kernel/torture.c).
> > 
> > Further comments appreciated since I am not sure I grokked the
> > assumption the torture tests make about hotplugging cpus in and out,
> > I will go through the commits logs again to find more info.
> > 
> > Thanks !
> > Lorenzo
> > 
> > > +	 */
> > > +	nb_available_cpus = num_online_cpus();
> > > +
> > > +	/* Check PSCI operations are set up and working. */
> > > +	ret = psci_ops_check();
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	pr_info("PSCI checker started using %u CPUs\n", nb_available_cpus);
> > > +
> > > +	pr_info("Starting hotplug tests\n");
> > > +	ret = hotplug_tests();
> > > +	if (ret == 0)
> > > +		pr_info("Hotplug tests passed OK\n");
> > > +	else if (ret > 0)
> > > +		pr_err("%d error(s) encountered in hotplug tests\n", ret);
> > > +	else {
> > > +		pr_err("Out of memory\n");
> > > +		return ret;
> > > +	}
> > > +
> > > +	pr_info("Starting suspend tests (%d cycles per state)\n",
> > > +		NUM_SUSPEND_CYCLE);
> > > +	ret = suspend_tests();
> > > +	if (ret == 0)
> > > +		pr_info("Suspend tests passed OK\n");
> > > +	else if (ret > 0)
> > > +		pr_err("%d error(s) encountered in suspend tests\n", ret);
> > > +	else {
> > > +		switch (ret) {
> > > +		case -ENOMEM:
> > > +			pr_err("Out of memory\n");
> > > +			break;
> > > +		case -ENODEV:
> > > +			pr_warn("Could not start suspend tests on any CPU\n");
> > > +			break;
> > > +		}
> > > +	}
> > > +
> > > +	pr_info("PSCI checker completed\n");
> > > +	return ret < 0 ? ret : 0;
> > > +}
> > > +late_initcall(psci_checker);
> > > -- 
> > > 2.10.0
> > > 
> > 
> 

^ permalink raw reply

* [PATCH] mfd: qcom-pm8xxx: Clean up PM8XXX namespace
From: Linus Walleij @ 2016-10-26 13:10 UTC (permalink / raw)
  To: linux-arm-kernel

The Kconfig and file naming for the PM8xxx driver is totally
confusing:

- Kconfig options MFD_PM8XXX and MFD_PM8921_CORE, some in-kernel
  users depending on or selecting either at random.
- A driver file named pm8921-core.c even if it is indeed
  used by the whole PM8xxx family of chips.
- An irqchip named pm8xxx since it was (I guess) realized that
  the driver was generic for all pm8xxx PMICs.

As I may want to add support for PM8901 this is starting to get
really messy. Fix this situation by:

- Remove the MFD_PM8921_CORE symbol and rely solely on MFD_PM8XXX
  and convert all users, including LEDs Kconfig and ARM defconfigs
  for qcom and multi_v7 to use that single symbol.
- Renaming the driver to qcom-pm8xxx.c to fit along the two
  other qcom* prefixed drivers.
- Rename functions withing the driver from 8921 to 8xxx to
  indicate it is generic.
- Just drop the =m config from the pxa_defconfig, I have no clue
  why it is even there, it is not a Qualcomm platform. (Possibly
  older Kconfig noise from saveconfig.)

Cc: Stephen Boyd <sboyd@codeaurora.org>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: Neil Armstrong <narmstrong@baylibre.com>
Cc: Abhijeet Dharmapurikar <adharmap@codeaurora.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
I do NOT think it is a good idea to try to split this commit up,
I rather prefer that Lee simply merge it into MFD.

The reason is that files like qcom_defconfig already contain both
the right symbols, but the MFD_PM8921_CORE symbol cannot be removed
until this rename has happened, whereas multi_v7_defconfig needs
it added etc, and this is just a clean nice cut.

Jacek, ARM SoC person: please ACK this patch to get merged into
MFD.
---
 arch/arm/configs/multi_v7_defconfig          |  2 +-
 arch/arm/configs/pxa_defconfig               |  1 -
 arch/arm/configs/qcom_defconfig              |  1 -
 drivers/leds/Kconfig                         |  2 +-
 drivers/mfd/Kconfig                          | 14 ++++------
 drivers/mfd/Makefile                         |  2 +-
 drivers/mfd/{pm8921-core.c => qcom-pm8xxx.c} | 42 ++++++++++++++--------------
 7 files changed, 29 insertions(+), 35 deletions(-)
 rename drivers/mfd/{pm8921-core.c => qcom-pm8xxx.c} (92%)

diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
index 437d0740dec6..4280de7b9b4e 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -489,7 +489,7 @@ CONFIG_MFD_MAX8907=y
 CONFIG_MFD_MAX8997=y
 CONFIG_MFD_MAX8998=y
 CONFIG_MFD_RK808=y
-CONFIG_MFD_PM8921_CORE=y
+CONFIG_MFD_PM8XXX=y
 CONFIG_MFD_QCOM_RPM=y
 CONFIG_MFD_SPMI_PMIC=y
 CONFIG_MFD_SEC_CORE=y
diff --git a/arch/arm/configs/pxa_defconfig b/arch/arm/configs/pxa_defconfig
index a016ecc0084b..e4314b1227a3 100644
--- a/arch/arm/configs/pxa_defconfig
+++ b/arch/arm/configs/pxa_defconfig
@@ -411,7 +411,6 @@ CONFIG_MFD_MAX77693=y
 CONFIG_MFD_MAX8907=m
 CONFIG_EZX_PCAP=y
 CONFIG_UCB1400_CORE=m
-CONFIG_MFD_PM8921_CORE=m
 CONFIG_MFD_SEC_CORE=y
 CONFIG_MFD_PALMAS=y
 CONFIG_MFD_TPS65090=y
diff --git a/arch/arm/configs/qcom_defconfig b/arch/arm/configs/qcom_defconfig
index c2dff4fd5fc4..74e9cd759b99 100644
--- a/arch/arm/configs/qcom_defconfig
+++ b/arch/arm/configs/qcom_defconfig
@@ -119,7 +119,6 @@ CONFIG_POWER_RESET=y
 CONFIG_POWER_RESET_MSM=y
 CONFIG_THERMAL=y
 CONFIG_MFD_PM8XXX=y
-CONFIG_MFD_PM8921_CORE=y
 CONFIG_MFD_QCOM_RPM=y
 CONFIG_MFD_SPMI_PMIC=y
 CONFIG_REGULATOR=y
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 7a628c6516f6..86bb1515a00e 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -645,7 +645,7 @@ config LEDS_VERSATILE
 
 config LEDS_PM8058
 	tristate "LED Support for the Qualcomm PM8058 PMIC"
-	depends on MFD_PM8921_CORE
+	depends on MFD_PM8XXX
 	depends on LEDS_CLASS
 	help
 	  Choose this option if you want to use the LED drivers in
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index c6df6442ba2b..1ed0584f494e 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -756,24 +756,20 @@ config UCB1400_CORE
 	  module will be called ucb1400_core.
 
 config MFD_PM8XXX
-	tristate
-
-config MFD_PM8921_CORE
-	tristate "Qualcomm PM8921 PMIC chip"
+	tristate "Qualcomm PM8xxx PMIC chips driver"
 	depends on (ARM || HEXAGON)
 	select IRQ_DOMAIN
 	select MFD_CORE
-	select MFD_PM8XXX
 	select REGMAP
 	help
 	  If you say yes to this option, support will be included for the
-	  built-in PM8921 PMIC chip.
+	  built-in PM8xxx PMIC chips.
 
-	  This is required if your board has a PM8921 and uses its features,
+	  This is required if your board has a PM8xxx and uses its features,
 	  such as: MPPs, GPIOs, regulators, interrupts, and PWM.
 
-	  Say M here if you want to include support for PM8921 chip as a module.
-	  This will build a module called "pm8921-core".
+	  Say M here if you want to include support for PM8xxx chips as a
+	  module. This will build a module called "pm8xxx-core".
 
 config MFD_QCOM_RPM
 	tristate "Qualcomm Resource Power Manager (RPM)"
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 9834e669d985..7bb5a50127cb 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -172,7 +172,7 @@ obj-$(CONFIG_MFD_SI476X_CORE)	+= si476x-core.o
 
 obj-$(CONFIG_MFD_CS5535)	+= cs5535-mfd.o
 obj-$(CONFIG_MFD_OMAP_USB_HOST)	+= omap-usb-host.o omap-usb-tll.o
-obj-$(CONFIG_MFD_PM8921_CORE) 	+= pm8921-core.o ssbi.o
+obj-$(CONFIG_MFD_PM8XXX) 	+= qcom-pm8xxx.o ssbi.o
 obj-$(CONFIG_MFD_QCOM_RPM)	+= qcom_rpm.o
 obj-$(CONFIG_MFD_SPMI_PMIC)	+= qcom-spmi-pmic.o
 obj-$(CONFIG_TPS65911_COMPARATOR)	+= tps65911-comparator.o
diff --git a/drivers/mfd/pm8921-core.c b/drivers/mfd/qcom-pm8xxx.c
similarity index 92%
rename from drivers/mfd/pm8921-core.c
rename to drivers/mfd/qcom-pm8xxx.c
index 0e3a2ea25942..7f9620ec61e8 100644
--- a/drivers/mfd/pm8921-core.c
+++ b/drivers/mfd/qcom-pm8xxx.c
@@ -53,7 +53,7 @@
 #define REG_HWREV		0x002  /* PMIC4 revision */
 #define REG_HWREV_2		0x0E8  /* PMIC4 revision 2 */
 
-#define PM8921_NR_IRQS		256
+#define PM8XXX_NR_IRQS		256
 
 struct pm_irq_chip {
 	struct regmap		*regmap;
@@ -308,22 +308,22 @@ static const struct regmap_config ssbi_regmap_config = {
 	.reg_write = ssbi_reg_write
 };
 
-static const struct of_device_id pm8921_id_table[] = {
+static const struct of_device_id pm8xxx_id_table[] = {
 	{ .compatible = "qcom,pm8018", },
 	{ .compatible = "qcom,pm8058", },
 	{ .compatible = "qcom,pm8921", },
 	{ }
 };
-MODULE_DEVICE_TABLE(of, pm8921_id_table);
+MODULE_DEVICE_TABLE(of, pm8xxx_id_table);
 
-static int pm8921_probe(struct platform_device *pdev)
+static int pm8xxx_probe(struct platform_device *pdev)
 {
 	struct regmap *regmap;
 	int irq, rc;
 	unsigned int val;
 	u32 rev;
 	struct pm_irq_chip *chip;
-	unsigned int nirqs = PM8921_NR_IRQS;
+	unsigned int nirqs = PM8XXX_NR_IRQS;
 
 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0)
@@ -384,46 +384,46 @@ static int pm8921_probe(struct platform_device *pdev)
 	return rc;
 }
 
-static int pm8921_remove_child(struct device *dev, void *unused)
+static int pm8xxx_remove_child(struct device *dev, void *unused)
 {
 	platform_device_unregister(to_platform_device(dev));
 	return 0;
 }
 
-static int pm8921_remove(struct platform_device *pdev)
+static int pm8xxx_remove(struct platform_device *pdev)
 {
 	int irq = platform_get_irq(pdev, 0);
 	struct pm_irq_chip *chip = platform_get_drvdata(pdev);
 
-	device_for_each_child(&pdev->dev, NULL, pm8921_remove_child);
+	device_for_each_child(&pdev->dev, NULL, pm8xxx_remove_child);
 	irq_set_chained_handler_and_data(irq, NULL, NULL);
 	irq_domain_remove(chip->irqdomain);
 
 	return 0;
 }
 
-static struct platform_driver pm8921_driver = {
-	.probe		= pm8921_probe,
-	.remove		= pm8921_remove,
+static struct platform_driver pm8xxx_driver = {
+	.probe		= pm8xxx_probe,
+	.remove		= pm8xxx_remove,
 	.driver		= {
-		.name	= "pm8921-core",
-		.of_match_table = pm8921_id_table,
+		.name	= "pm8xxx-core",
+		.of_match_table = pm8xxx_id_table,
 	},
 };
 
-static int __init pm8921_init(void)
+static int __init pm8xxx_init(void)
 {
-	return platform_driver_register(&pm8921_driver);
+	return platform_driver_register(&pm8xxx_driver);
 }
-subsys_initcall(pm8921_init);
+subsys_initcall(pm8xxx_init);
 
-static void __exit pm8921_exit(void)
+static void __exit pm8xxx_exit(void)
 {
-	platform_driver_unregister(&pm8921_driver);
+	platform_driver_unregister(&pm8xxx_driver);
 }
-module_exit(pm8921_exit);
+module_exit(pm8xxx_exit);
 
 MODULE_LICENSE("GPL v2");
-MODULE_DESCRIPTION("PMIC 8921 core driver");
+MODULE_DESCRIPTION("PMIC 8xxx core driver");
 MODULE_VERSION("1.0");
-MODULE_ALIAS("platform:pm8921-core");
+MODULE_ALIAS("platform:pm8xxx-core");
-- 
2.7.4

^ permalink raw reply related

* [PATCH v2] modversions: treat symbol CRCs as 32 bit quantities on 64 bit archs
From: Ard Biesheuvel @ 2016-10-26 13:04 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <871sz3xw1s.fsf@concordia.ellerman.id.au>

On 26 October 2016 at 11:07, Michael Ellerman <mpe@ellerman.id.au> wrote:
> Hi Ard,
>
> I like the concept, but ...
>
> Ard Biesheuvel <ard.biesheuvel@linaro.org> writes:
>> The symbol CRCs are emitted as ELF symbols, which allows us to easily
>> populate the kcrctab sections by relying on the linker to associate
>> each kcrctab slot with the correct value.
>>
>> This has two downsides:
>> - given that the CRCs are treated as pointers, we waste 4 bytes for
>>   each CRC on 64 bit architectures,
>> - on architectures that support runtime relocation, a relocation entry is
>>   emitted for each CRC value, which may take up 24 bytes of __init space
>>   (on ELF64 systems)
>>
>> This comes down to a x8 overhead in [uncompressed] kernel size. In addition,
>> each relocation has to be reverted before the CRC value can be used.
>>
>> Switching to explicit 32 bit values on 64 bit architectures fixes both
>> issues, since 32 bit values are not treated as relocatable quantities on
>> ELF64 systems, even if the value ultimately resolves to a linker supplied
>> value.
>
> Are we sure that part is true? ("not treated as relocatable")
>

Thanks for testing this.

> A quick test build on powerpc gives me:
>
>   WARNING: 6829 bad relocations
>   c000000000ca3748 R_PPC64_ADDR16    *ABS*+0x0000000013f53da6
>   c000000000ca374a R_PPC64_ADDR16    *ABS*+0x00000000f7272059
>   c000000000ca374c R_PPC64_ADDR16    *ABS*+0x0000000002013d36
>   c000000000ca374e R_PPC64_ADDR16    *ABS*+0x00000000a59dffc8
>   ...
>
> Which is coming from our relocs_check.sh script, which checks that the
> generated relocations are ones we know how to handle.
>

OK, first of all, it appears the ppc64 assembler interprets .word
differently than the arm64 one, so I will need to fold this in

"""
diff --git a/include/linux/export.h b/include/linux/export.h
index fa51ab2ad190..a000d421526d 100644
--- a/include/linux/export.h
+++ b/include/linux/export.h
@@ -54,7 +54,7 @@ extern struct module __this_module;
 #define __CRC_SYMBOL(sym, sec)                                         \
        asm("   .section \"___kcrctab" sec "+" #sym "\", \"a\"  \n"     \
            "   .weak   " VMLINUX_SYMBOL_STR(__crc_##sym) "     \n"     \
-           "   .word   " VMLINUX_SYMBOL_STR(__crc_##sym) "     \n"     \
+           "   .long   " VMLINUX_SYMBOL_STR(__crc_##sym) "     \n"     \
            "   .previous                                       \n");
 #endif
 #else
"""

With that change, the CRCs are actually emitted as

WARNING: 7525 bad relocations
c000000000ce7f50 R_PPC64_ADDR32    *ABS*+0x0000000013f53da6
c000000000ce7f54 R_PPC64_ADDR32    *ABS*+0x0000000004f7c64e
c000000000ce7f58 R_PPC64_ADDR32    *ABS*+0x0000000092be8a3e

which is still a bit disappointing, given that we still have 7525 RELA
entries to process.
I tried several thing, i.e., adding -Bsymbolic to the linker command
line, emitting the reference above as .hidden or emit  the definition
from the linker script as HIDDEN(), but nothing seems to make any
difference. (On arm64, -Bsymbolic eliminates *all* runtime relocations
except R_<arch>_RELATIVE ones)

> And when I try to boot it I get:
>
>   virtio: disagrees about version of symbol module_layout
>   virtio: disagrees about version of symbol module_layout
>   scsi_mod: disagrees about version of symbol module_layout
>
> And it can't find my root file system (unsurprisingly as it's on scsi).
>

Something like the below should fix it, I hope.

"""
diff --git a/arch/powerpc/kernel/reloc_64.S b/arch/powerpc/kernel/reloc_64.S
index d88736fbece6..99cdf2311ab5 100644
--- a/arch/powerpc/kernel/reloc_64.S
+++ b/arch/powerpc/kernel/reloc_64.S
@@ -14,6 +14,7 @@
 RELA = 7
 RELACOUNT = 0x6ffffff9
 R_PPC64_RELATIVE = 22
+R_PPC64_ADDR32 = 1

 /*
  * r3 = desired final address of kernel
@@ -77,9 +78,22 @@ _GLOBAL(relocate)
        add     r0,r0,r3
        stdx    r0,r7,r6
        addi    r9,r9,24
-       bdnz    5b
+       b       7f
+
+       /*
+        * CRCs of exported symbols are emitted as 32-bit relocations against
+        * the *ABS* section with the CRC value recorded in the addend.
+        */
+6:     cmpdi   r0,R_PPC64_ADDR32
+       bne     7f
+       ld      r6,0(r9)        /* reloc->r_offset */
+       ld      r0,16(r9)       /* reloc->r_addend */
+       stwx    r0,r7,r6
+       addi    r9,r9,24
+
+7:     bdnz    5b
+       blr

-6:     blr

 .balign 8
 p_dyn: .llong  __dynamic_start - 0b
"""

Note that the loop no longer terminates at the first
non-R_PPC64_RELATIVE relocation, but that seems safer to me in any
case. It simply stores the value of r_addend at r_offset, which is the
correct thing to do for R_PPC64_ADDR32 relocations against the *ABS*
section, regardless of whether we are dealing with CRCs or something
else. Note that the comparison above will fail for R_PPC64_ADDR32
relocations against named symbols, since we compare the entire r_info
field and not just the type (as the comment a few lines higher up
suggests)

Also a fix for relocs_check.sh:

"""
diff --git a/arch/powerpc/relocs_check.sh b/arch/powerpc/relocs_check.sh
index ec2d5c835170..2f510fbc87da 100755
--- a/arch/powerpc/relocs_check.sh
+++ b/arch/powerpc/relocs_check.sh
@@ -43,7 +43,8 @@ R_PPC_ADDR16_HA
 R_PPC_RELATIVE
 R_PPC_NONE' |
        grep -E -v '\<R_PPC64_ADDR64[[:space:]]+mach_' |
-       grep -E -v '\<R_PPC64_ADDR64[[:space:]]+__crc_'
+       grep -E -v '\<R_PPC64_ADDR64[[:space:]]+__crc_' |
+       grep -E -v '\<R_PPC64_ADDR32[[:space:]]+\*ABS\*'
 )

 if [ -z "$bad_relocs" ]; then
"""

If these changes work for PPC, I think we should fold them in.
Hopefully, GNU ld for PPC will gain that ability to resolve absolute
relocations@build time (like other architectures), and then the
R_PPC64_ADDR32 handling will simply become dead code. In any case, it
is an improvement over the mangling of CRC values to undo the runtime
relocation, imo.

Regards,
Ard.

^ permalink raw reply related

* [PATCH] ARM: sti: stih410-clocks: Add PROC_STFE as a critical clock
From: Lee Jones @ 2016-10-26 13:00 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161026085620.GA5476@griffinp-ThinkPad-X1-Carbon-2nd>

> > > > > If the clock is enabled when Linux boots, the Linux clock framework *needs*
> > > > > to assume the hardware may have been used in previous boot stages, and it should
> > > > > not attempt to disable the clock.
> > > > 
> > > > None of the boot loaders we use do this.
> > > 
> > > But the Linux kernel isn't just used by us. It is not uncommon for STB
> > > bootloaders to get information from the frontend as part of the boot process. 
> > 
> > Okay, this is the clincher.  Since we need to support non-standard
> > bootloaders, it's difficult to guarantee that the clock will be
> > disabled at boot.  For this reason, I believe that we can call this a
> > critical clock.

> That's good news as the STi maintainer already acked and applied the patch.

Matters not.  That's why we have `git rebase` and `git revert`. ;)

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* [PATCH v5 3/9] drm/hisilicon/hibmc: Add support for frame buffer
From: Daniel Vetter @ 2016-10-26 12:59 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <58107523.20409@huawei.com>

On Wed, Oct 26, 2016 at 05:19:31PM +0800, Rongrong Zou wrote:
> Hi Daniel,
> 
> Thansk for reviewing!
> 
> ? 2016/10/26 13:56, Daniel Vetter ??:
> > On Wed, Oct 26, 2016 at 10:37:00AM +0800, Rongrong Zou wrote:
> > > Add support for fbdev and kms fb management.
> > > 
> > > Signed-off-by: Rongrong Zou <zourongrong@gmail.com>
> > 
> > Small drive-by comment below.
> > 
> > > diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> > > index db8d80e..d41138a 100644
> > > --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> > > +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> > > @@ -20,9 +20,23 @@
> > >   #define HIBMC_DRM_DRV_H
> > > 
> > >   #include <drm/drmP.h>
> > > +#include <drm/drm_fb_helper.h>
> > >   #include <drm/ttm/ttm_bo_driver.h>
> > >   #include <drm/drm_gem.h>
> > > 
> > > +struct hibmc_framebuffer {
> > > +	struct drm_framebuffer fb;
> > > +	struct drm_gem_object *obj;
> > > +	bool is_fbdev_fb;
> > > +};
> > > +
> > > +struct hibmc_fbdev {
> > > +	struct drm_fb_helper helper;
> > > +	struct hibmc_framebuffer fb;
> > 
> > I wouldn't embed the single framebuffer here, but instead have a pointer
> > and just refcount it. This here is a pattern that predates framebuffer
> > refcounting, and it leads to plenty of surprises.
> 
> will allocate fbdev in next patch version, thanks.

Not the fbdev, the hibmc_framebuffer.

> > Maybe we should update the documentation of
> > drm_framebuffer_unregister_private() to mention that it is deprecated? The
> > overview doc in drm_framebuffer.c already explains that, but I guess
> > that's not obvious enough.
> 
> Just replace drm_framebuffer_unregister_private() with
> drm_framebuffer_remove()?
> 
> I found many other drivers use the following two functions in their
> own xxx_fbdev_destroy():
> 	drm_framebuffer_unregister_private(fbdev->fb);
> 	drm_framebuffer_remove(fbdev->fb);
> so the former is redundant and can be removed directly?

They all embed the fb instead of having a pointer, because those drivers
are all older than the fb refcounting support. In general good practice is
to look at the most recently merged driver, not at all of them. Only the
most recently one has a good chance to be up-to-date with latest best
practices. The function to call would be drm_framebuffer_unreference(),
and your struct above should be

struct hibmc_fbdev {
	struct drm_fb_helper helper;
	struct hibmc_framebuffer *fb;
	...
};

Note how fb is a pointer here, not an embedded struct.
-Daniel

> 
> > 
> > Can you pls do that patch? And pls make sure it all looks pretty when
> > building the docs with
> 
> No problem, i'll send another patch later.
> 
> Regards,
> Rongrong
> 
> > 
> > $ make htmldocs
> > 
> > Thanks, Daniel
> > 
> 
> 
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

^ permalink raw reply

* [PATCH 3/4] mfd: altr-a10sr: Add Arria10 SR Monitor
From: Lee Jones @ 2016-10-26 12:52 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1476394329-31696-4-git-send-email-tthayer@opensource.altera.com>

On Thu, 13 Oct 2016, tthayer at opensource.altera.com wrote:

> From: Thor Thayer <tthayer@opensource.altera.com>
> 
> Add the Altera Arria10 DevKit System Resource Monitor functionality
> to the MFD device.
> 
> Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
> ---
>  drivers/mfd/altera-a10sr.c |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/mfd/altera-a10sr.c b/drivers/mfd/altera-a10sr.c
> index 06e1f7f..0942d7d 100644
> --- a/drivers/mfd/altera-a10sr.c
> +++ b/drivers/mfd/altera-a10sr.c
> @@ -33,6 +33,10 @@
>  		.name = "altr_a10sr_gpio",
>  		.of_compatible = "altr,a10sr-gpio",
>  	},
> +	{
> +		.name = "altr_a10sr_mon",
> +		.of_compatible = "altr,a10sr-mon",

"-monitor" would be better in my opinion.

> +	},
>  };
>  
>  static bool altr_a10sr_reg_readable(struct device *dev, unsigned int reg)

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* [PATCH 1/4] dt-bindings: mfd: Add Altera Arria10 SR Monitor
From: Lee Jones @ 2016-10-26 12:51 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1476394329-31696-2-git-send-email-tthayer@opensource.altera.com>

On Thu, 13 Oct 2016, tthayer at opensource.altera.com wrote:

> From: Thor Thayer <tthayer@opensource.altera.com>
> 
> Add the Arria10 DevKit System Resource Chip register and state
> monitoring module to the MFD.
> 
> Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
> ---
> Note: This needs to be applied to the bindings document that
> was Acked & Applied but didn't reach the for-next branch.
> See https://patchwork.ozlabs.org/patch/629397/
> ---
> ---
>  .../devicetree/bindings/mfd/altera-a10sr.txt       |    9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/altera-a10sr.txt b/Documentation/devicetree/bindings/mfd/altera-a10sr.txt
> index ea151f2..0787ec6 100644
> --- a/Documentation/devicetree/bindings/mfd/altera-a10sr.txt
> +++ b/Documentation/devicetree/bindings/mfd/altera-a10sr.txt
> @@ -18,6 +18,7 @@ The A10SR consists of these sub-devices:
>  Device                   Description
>  ------                   ----------
>  a10sr_gpio               GPIO Controller
> +a10sr_monitor            Register and State Monitoring
>  
>  Arria10 GPIO
>  Required Properties:
> @@ -27,6 +28,10 @@ Required Properties:
>                        the second cell is used to specify flags.
>                        See ../gpio/gpio.txt for more information.
>  
> +Arria10 Register and State Monitor
> +Required Properties:
> +- compatible        : Should be "altr,a10sr-mon"

Why not "-monitor"?

>  Example:
>  
>          resource-manager at 0 {
> @@ -43,4 +48,8 @@ Example:
>  			gpio-controller;
>  			#gpio-cells = <2>;
>  		};
> +
> +		a10sr_monitor {
> +			compatible = "altr,a10sr-mon";
> +		};
>  	};

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* [PATCH v2] ARM: davinci: da8xx: Fix some redefined symbol warnings
From: Alexandre Bailon @ 2016-10-26 12:39 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <3dc1c2c2-c4c5-07d0-3d3e-b8d917adda81@ti.com>

On 10/26/2016 01:57 PM, Sekhar Nori wrote:
> On Tuesday 25 October 2016 05:41 PM, Alexandre Bailon wrote:
>> Some macro for DA8xx CFGCHIP are defined in usb-davinci.h,
>> but da8xx-cfgchip.h intend to replace them.
>> The usb-da8xx.c is using both headers, causing redefined symbol warnings.
>> Remove the macro and update the da830-evm board file to use da8xx-cfgchip.h
>>
>> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
> 
> This leads to build error on v4.9-rc2:
Oops! Sorry for that. I will  fit it.
> 
> arch/arm/mach-davinci/board-da830-evm.c: In function 'da830_evm_usb_init':
> arch/arm/mach-davinci/board-da830-evm.c:120:15: error: 'CFGCHIP2_REFFREQ' undeclared (first use in this function)
>   cfgchip2 &= ~CFGCHIP2_REFFREQ;
> 
> Thanks,
> Sekhar
> 

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox