Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] regulator: core: Make regulator object reflect configured voltage
From: Mark Brown @ 2014-02-04 11:05 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1391493268-3242-1-git-send-email-bjorn.andersson@sonymobile.com>

On Mon, Feb 03, 2014 at 09:54:28PM -0800, Bjorn Andersson wrote:

> +	/*
> +	 * Make the regulator reflect the configured voltage selected in
> +	 * machine_constraints_voltage()
> +	 */
> +	if (rdev->constraints->apply_uV &&
> +	    rdev->constraints->min_uV == rdev->constraints->max_uV) {
> +		regulator->min_uV = rdev->constraints->min_uV;
> +		regulator->max_uV = rdev->constraints->min_uV;
> +	}
> +

Why not do this at the time we apply the voltage?  That would seem to be
more robust, doing it in a separate place means that we might update one
bit of code and not the other or might change the execution path so that
one gets run and the other doesn't.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140204/539dcbfa/attachment.sig>

^ permalink raw reply

* [PATCH v3 3/5] spi: sunxi: Add Allwinner A31 SPI controller driver
From: Mark Brown @ 2014-02-04 11:02 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140204090926.GI25625@lukather>

On Tue, Feb 04, 2014 at 10:09:26AM +0100, Maxime Ripard wrote:
> On Tue, Feb 04, 2014 at 12:21:10AM +0000, Mark Brown wrote:

> > It isn't awesome, no.  Ideally the runtime PM code would do this but
> > then you couldn't ifdef the operations which as far as I can tell is the
> > main thing people want from disabling it and it gets complicated for
> > devices that genuinely do power up on startup so here we are.

> We discussed it with Kevin on IRC, and he suggested that we move that
> pm_runtime initialization to the SPI core, but I guess that would also
> mean that all drivers shouldn't ifdef the operations, so that the core
> can call the runtime_resume callback directly.

No, that's not going to be robust - it means drivers can't do any power
sequencing of their own.

> However, I don't really get why any driver should be doing so, since
> you still need these functions to at least to the device
> suspend/resume in the probe/remove, and you don't really want to
> duplicate the code.

I don't think it's particularly useful to support disabling runtime PM
in the first place but some drivers do different things when doing
runtime management to those they do on first init - for example there
may be additional steps that only need to be done during first power up.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140204/9428cfc0/attachment.sig>

^ permalink raw reply

* HARDIRQ-safe -> HARDIRQ-unsafe lock order detected
From: Christian Gmeiner @ 2014-02-04 10:46 UTC (permalink / raw)
  To: linux-arm-kernel

I have some filesystem anomalies and and enabled all kind of debug
stuff and got the following:

[    1.265237]
[    1.266761] ======================================================
[    1.272969] [ INFO: HARDIRQ-safe -> HARDIRQ-unsafe lock order detected ]
[    1.279699] 3.12.9 #1 Not tainted
[    1.283032] ------------------------------------------------------
[    1.289240] kworker/u4:2/43 [HC0[0]:SC0[0]:HE0:SE1] is trying to acquire:
[    1.296052]  (prepare_lock){+.+.+.}, at: [<c03bc8f0>]
clk_prepare_lock+0x3c/0xd8
[    1.303583]
[    1.303583] and this task is already holding:
[    1.309441]  (&(&host->lock)->rlock#2){-.-...}, at: [<c03a2e70>]
sdhci_do_set_ios+0x18/0x700
[    1.318035] which would create a new lock dependency:
[    1.323107]  (&(&host->lock)->rlock#2){-.-...} -> (prepare_lock){+.+.+.}
[    1.330019]
[    1.330019] but this new dependency connects a HARDIRQ-irq-safe lock:
[    1.337964]  (&(&host->lock)->rlock#2){-.-...}
... which became HARDIRQ-irq-safe at:
[    1.345915]   [<c0080008>] __lock_acquire+0xac4/0x1df8
[    1.351113]   [<c0081934>] lock_acquire+0xa4/0x158
[    1.355956]   [<c0487480>] _raw_spin_lock+0x40/0x50
[    1.360889]   [<c03a1838>] sdhci_irq+0x18/0xa2c
[    1.365469]   [<c006a190>] handle_irq_event_percpu+0x70/0x2d0
[    1.371276]   [<c006a42c>] handle_irq_event+0x3c/0x5c
[    1.376376]   [<c006d16c>] handle_fasteoi_irq+0xa4/0x138
[    1.381741]   [<c0069b58>] generic_handle_irq+0x20/0x30
[    1.387017]   [<c000eb54>] handle_IRQ+0x38/0x90
[    1.391600]   [<c000850c>] gic_handle_irq+0x28/0x5c
[    1.396526]   [<c0488304>] __irq_svc+0x44/0x78
[    1.401021]   [<c0487c78>] _raw_spin_unlock_irqrestore+0x64/0x68
[    1.407083]   [<c03b0a5c>] of_find_node_by_phandle+0x48/0x58
[    1.412797]   [<c03b108c>] __of_parse_phandle_with_args+0xc0/0x1f8
[    1.419033]   [<c03b1248>] of_parse_phandle_with_args+0x28/0x30
[    1.425008]   [<c027a654>] of_get_named_gpio_flags+0x60/0x88
[    1.430727]   [<c03a652c>] gpio_led_probe+0xa4/0x338
[    1.435747]   [<c02eb1e0>] platform_drv_probe+0x18/0x1c
[    1.441026]   [<c02ea2b0>] driver_probe_device+0xf4/0x208
[    1.446486]   [<c02ea458>] __driver_attach+0x94/0x98
[    1.451501]   [<c02e89ec>] bus_for_each_dev+0x54/0x88
[    1.456605]   [<c02e9984>] bus_add_driver+0xdc/0x264
[    1.461621]   [<c02ea85c>] driver_register+0x78/0xf4
[    1.466638]   [<c0008748>] do_one_initcall+0x34/0x15c
[    1.471741]   [<c0668c24>] kernel_init_freeable+0x158/0x228
[    1.477374]   [<c047d33c>] kernel_init+0x8/0xe4
[    1.481962]   [<c000e328>] ret_from_fork+0x14/0x2c
[    1.486807]
[    1.486807] to a HARDIRQ-irq-unsafe lock:
[    1.492321]  (prepare_lock){+.+.+.}
... which became HARDIRQ-irq-unsafe at:
[    1.499470] ...  [<c007e048>] mark_held_locks+0x64/0x138
[    1.504863]   [<c007e1c4>] trace_hardirqs_on_caller+0xa8/0x230
[    1.510749]   [<c0482de8>] mutex_trylock+0x160/0x1f0
[    1.515764]   [<c03bc8c0>] clk_prepare_lock+0xc/0xd8
[    1.520782]   [<c03bd180>] clk_notifier_register+0x24/0xe8
[    1.526321]   [<c0008748>] do_one_initcall+0x34/0x15c
[    1.531422]   [<c0668c24>] kernel_init_freeable+0x158/0x228
[    1.537050]   [<c047d33c>] kernel_init+0x8/0xe4
[    1.541630]   [<c000e328>] ret_from_fork+0x14/0x2c
[    1.546474]
[    1.546474] other info that might help us debug this:
[    1.546474]
[    1.554515]  Possible interrupt unsafe locking scenario:
[    1.554515]
[    1.561333]        CPU0                    CPU1
[    1.565884]        ----                    ----
[    1.570435]   lock(prepare_lock);
[    1.573816]                                local_irq_disable();
[    1.579759]                                lock(&(&host->lock)->rlock#2);
[    1.586640]                                lock(prepare_lock);
[    1.592542]   <Interrupt>
[    1.595180]     lock(&(&host->lock)->rlock#2);
[    1.599713]
[    1.599713]  *** DEADLOCK ***
[    1.599713]
[    1.605667] 3 locks held by kworker/u4:2/43:
[    1.609961]  #0:  (kmmcd){.+.+..}, at: [<c003cf38>]
process_one_work+0x12c/0x50c
[    1.617507]  #1:  ((&(&host->detect)->work)){+.+...}, at:
[<c003cf38>] process_one_work+0x12c/0x50c
[    1.626704]  #2:  (&(&host->lock)->rlock#2){-.-...}, at:
[<c03a2e70>] sdhci_do_set_ios+0x18/0x700
[    1.635752]
the dependencies between HARDIRQ-irq-safe lock and the holding lock:
[    1.643396] -> (&(&host->lock)->rlock#2){-.-...} ops: 67 {
[    1.649045]    IN-HARDIRQ-W at:
[    1.652229]                     [<c0080008>] __lock_acquire+0xac4/0x1df8
[    1.658987]                     [<c0081934>] lock_acquire+0xa4/0x158
[    1.665394]                     [<c0487480>] _raw_spin_lock+0x40/0x50
[    1.671888]                     [<c03a1838>] sdhci_irq+0x18/0xa2c
[    1.678033]                     [<c006a190>]
handle_irq_event_percpu+0x70/0x2d0
[    1.685403]                     [<c006a42c>] handle_irq_event+0x3c/0x5c
[    1.692072]                     [<c006d16c>] handle_fasteoi_irq+0xa4/0x138
[    1.698999]                     [<c0069b58>] generic_handle_irq+0x20/0x30
[    1.705841]                     [<c000eb54>] handle_IRQ+0x38/0x90
[    1.711988]                     [<c000850c>] gic_handle_irq+0x28/0x5c
[    1.718479]                     [<c0488304>] __irq_svc+0x44/0x78
[    1.724539]                     [<c0487c78>]
_raw_spin_unlock_irqrestore+0x64/0x68
[    1.732166]                     [<c03b0a5c>]
of_find_node_by_phandle+0x48/0x58
[    1.739445]                     [<c03b108c>]
__of_parse_phandle_with_args+0xc0/0x1f8
[    1.747246]                     [<c03b1248>]
of_parse_phandle_with_args+0x28/0x30
[    1.754784]                     [<c027a654>]
of_get_named_gpio_flags+0x60/0x88
[    1.762066]                     [<c03a652c>] gpio_led_probe+0xa4/0x338
[    1.768646]                     [<c02eb1e0>] platform_drv_probe+0x18/0x1c
[    1.775488]                     [<c02ea2b0>] driver_probe_device+0xf4/0x208
[    1.782506]                     [<c02ea458>] __driver_attach+0x94/0x98
[    1.789087]                     [<c02e89ec>] bus_for_each_dev+0x54/0x88
[    1.795754]                     [<c02e9984>] bus_add_driver+0xdc/0x264
[    1.802334]                     [<c02ea85c>] driver_register+0x78/0xf4
[    1.808914]                     [<c0008748>] do_one_initcall+0x34/0x15c
[    1.815581]                     [<c0668c24>] kernel_init_freeable+0x158/0x228
[    1.822772]                     [<c047d33c>] kernel_init+0x8/0xe4
[    1.828920]                     [<c000e328>] ret_from_fork+0x14/0x2c
[    1.835331]    IN-SOFTIRQ-W at:
[    1.838514]                     [<c007fb08>] __lock_acquire+0x5c4/0x1df8
[    1.845270]                     [<c0081934>] lock_acquire+0xa4/0x158
[    1.851676]                     [<c0487580>] _raw_spin_lock_irqsave+0x4c/0x60
[    1.858866]                     [<c03a28b4>] sdhci_tasklet_finish+0x14/0x10c
[    1.865968]                     [<c00293ec>] tasklet_action+0x6c/0x108
[    1.872552]                     [<c0028874>] __do_softirq+0x108/0x2f0
[    1.879046]                     [<c0028b14>] do_softirq+0x68/0x70
[    1.885189]                     [<c0028e14>] irq_exit+0xb0/0xf4
[    1.891160]                     [<c000eb58>] handle_IRQ+0x3c/0x90
[    1.897307]                     [<c000850c>] gic_handle_irq+0x28/0x5c
[    1.903799]                     [<c0488304>] __irq_svc+0x44/0x78
[    1.909857]                     [<c0487c78>]
_raw_spin_unlock_irqrestore+0x64/0x68
[    1.917484]                     [<c03b0a5c>]
of_find_node_by_phandle+0x48/0x58
[    1.924762]                     [<c03b108c>]
__of_parse_phandle_with_args+0xc0/0x1f8
[    1.932562]                     [<c03b1248>]
of_parse_phandle_with_args+0x28/0x30
[    1.940101]                     [<c027a654>]
of_get_named_gpio_flags+0x60/0x88
[    1.947380]                     [<c03a652c>] gpio_led_probe+0xa4/0x338
[    1.953960]                     [<c02eb1e0>] platform_drv_probe+0x18/0x1c
[    1.960799]                     [<c02ea2b0>] driver_probe_device+0xf4/0x208
[    1.967816]                     [<c02ea458>] __driver_attach+0x94/0x98
[    1.974397]                     [<c02e89ec>] bus_for_each_dev+0x54/0x88
[    1.981063]                     [<c02e9984>] bus_add_driver+0xdc/0x264
[    1.987643]                     [<c02ea85c>] driver_register+0x78/0xf4
[    1.994223]                     [<c0008748>] do_one_initcall+0x34/0x15c
[    2.000890]                     [<c0668c24>] kernel_init_freeable+0x158/0x228
[    2.008082]                     [<c047d33c>] kernel_init+0x8/0xe4
[    2.014228]                     [<c000e328>] ret_from_fork+0x14/0x2c
[    2.020636]    INITIAL USE at:
[    2.023731]                    [<c007f844>] __lock_acquire+0x300/0x1df8
[    2.030400]                    [<c0081934>] lock_acquire+0xa4/0x158
[    2.036720]                    [<c0487580>] _raw_spin_lock_irqsave+0x4c/0x60
[    2.043823]                    [<c03a2e70>] sdhci_do_set_ios+0x18/0x700
[    2.050491]                    [<c03907d0>] mmc_power_up+0x78/0xdc
[    2.056726]                    [<c03913d4>] mmc_start_host+0x38/0x50
[    2.063133]                    [<c03920ac>] mmc_add_host+0x58/0x74
[    2.069368]                    [<c03a3d50>] sdhci_add_host+0x7f0/0xba8
[    2.075950]                    [<c03a50c8>] sdhci_esdhc_imx_probe+0x28c/0x4f0
[    2.083139]                    [<c02eb1e0>] platform_drv_probe+0x18/0x1c
[    2.089892]                    [<c02ea2b0>] driver_probe_device+0xf4/0x208
[    2.096823]                    [<c02ea458>] __driver_attach+0x94/0x98
[    2.103317]                    [<c02e89ec>] bus_for_each_dev+0x54/0x88
[    2.109897]                    [<c02e9984>] bus_add_driver+0xdc/0x264
[    2.116390]                    [<c02ea85c>] driver_register+0x78/0xf4
[    2.122883]                    [<c0008748>] do_one_initcall+0x34/0x15c
[    2.129465]                    [<c0668c24>] kernel_init_freeable+0x158/0x228
[    2.136571]                    [<c047d33c>] kernel_init+0x8/0xe4
[    2.142631]                    [<c000e328>] ret_from_fork+0x14/0x2c
[    2.148953]  }
[    2.150636]  ... key      at: [<c0cbebb8>] __key.24056+0x0/0x8
[    2.156525]  ... acquired at:
[    2.159512]    [<c007d58c>] check_irq_usage+0x50/0xb0
[    2.164616]    [<c0080560>] __lock_acquire+0x101c/0x1df8
[    2.169980]    [<c0081934>] lock_acquire+0xa4/0x158
[    2.174908]    [<c04849f8>] mutex_lock_nested+0x58/0x3e0
[    2.180270]    [<c03bc8f0>] clk_prepare_lock+0x3c/0xd8
[    2.185461]    [<c03bd3f0>] clk_get_rate+0xc/0x5c
[    2.190214]    [<c03a4940>] esdhc_pltfm_set_clock+0x14/0x1cc
[    2.195925]    [<c03a24d4>] sdhci_set_clock+0x44/0x410
[    2.201113]    [<c03a3168>] sdhci_do_set_ios+0x310/0x700
[    2.206474]    [<c0390468>] mmc_set_chip_select+0x18/0x1c
[    2.211924]    [<c0394838>] mmc_go_idle+0xdc/0xf8
[    2.216680]    [<c0392b34>] mmc_init_card+0x48/0x1424
[    2.221781]    [<c03940e0>] mmc_attach_mmc+0xc4/0x1cc
[    2.226882]    [<c039134c>] mmc_rescan+0x280/0x2d0
[    2.231720]    [<c003cfb4>] process_one_work+0x1a8/0x50c
[    2.237082]    [<c003d748>] worker_thread+0x144/0x3a4
[    2.242182]    [<c0043de0>] kthread+0xa4/0xb0
[    2.246595]    [<c000e328>] ret_from_fork+0x14/0x2c
[    2.251524]
[    2.253033]
the dependencies between the lock to be acquired and HARDIRQ-irq-unsafe lock:
[    2.261455] -> (prepare_lock){+.+.+.} ops: 312 {
[    2.266213]    HARDIRQ-ON-W at:
[    2.269396]                     [<c007e048>] mark_held_locks+0x64/0x138
[    2.276067]                     [<c007e1c4>]
trace_hardirqs_on_caller+0xa8/0x230
[    2.283521]                     [<c0482de8>] mutex_trylock+0x160/0x1f0
[    2.290101]                     [<c03bc8c0>] clk_prepare_lock+0xc/0xd8
[    2.296682]                     [<c03bd180>] clk_notifier_register+0x24/0xe8
[    2.303785]                     [<c0008748>] do_one_initcall+0x34/0x15c
[    2.310452]                     [<c0668c24>] kernel_init_freeable+0x158/0x228
[    2.317645]                     [<c047d33c>] kernel_init+0x8/0xe4
[    2.323791]                     [<c000e328>] ret_from_fork+0x14/0x2c
[    2.330199]    SOFTIRQ-ON-W at:
[    2.333383]                     [<c007e048>] mark_held_locks+0x64/0x138
[    2.340053]                     [<c007e228>]
trace_hardirqs_on_caller+0x10c/0x230
[    2.347594]                     [<c0482de8>] mutex_trylock+0x160/0x1f0
[    2.354175]                     [<c03bc8c0>] clk_prepare_lock+0xc/0xd8
[    2.360755]                     [<c03bd180>] clk_notifier_register+0x24/0xe8
[    2.367858]                     [<c0008748>] do_one_initcall+0x34/0x15c
[    2.374525]                     [<c0668c24>] kernel_init_freeable+0x158/0x228
[    2.381716]                     [<c047d33c>] kernel_init+0x8/0xe4
[    2.387862]                     [<c000e328>] ret_from_fork+0x14/0x2c
[    2.394270]    RECLAIM_FS-ON-W at:
[    2.397715]                        [<c007e048>] mark_held_locks+0x64/0x138
[    2.404645]                        [<c007e9e8>]
lockdep_trace_alloc+0x90/0x10c
[    2.411926]                        [<c0106494>]
kmem_cache_alloc_trace+0x34/0x1e0
[    2.419470]                        [<c03bd204>]
clk_notifier_register+0xa8/0xe8
[    2.426837]                        [<c0008748>] do_one_initcall+0x34/0x15c
[    2.433765]                        [<c0668c24>]
kernel_init_freeable+0x158/0x228
[    2.441220]                        [<c047d33c>] kernel_init+0x8/0xe4
[    2.447627]                        [<c000e328>] ret_from_fork+0x14/0x2c
[    2.454297]    INITIAL USE at:
[    2.457393]                    [<c007f844>] __lock_acquire+0x300/0x1df8
[    2.464064]                    [<c0081934>] lock_acquire+0xa4/0x158
[    2.470386]                    [<c0482d7c>] mutex_trylock+0xf4/0x1f0
[    2.476792]                    [<c03bc8c0>] clk_prepare_lock+0xc/0xd8
[    2.483285]                    [<c03be6bc>] __clk_init+0x18/0x428
[    2.489435]                    [<c03beb94>] _clk_register+0xc8/0x15c
[    2.495842]                    [<c03beccc>] clk_register+0x38/0x80
[    2.502075]                    [<c03bf5f0>] clk_register_fixed_rate+0x84/0xcc
[    2.509267]                    [<c03bf698>] of_fixed_clk_setup+0x60/0x88
[    2.516022]                    [<c06975d0>] of_clk_init+0x3c/0x64
[    2.522176]                    [<c0671eb8>] imx6q_timer_init+0xc/0x4c
[    2.528674]                    [<c066b2f4>] time_init+0x1c/0x2c
[    2.534652]                    [<c0668954>] start_kernel+0x1c4/0x33c
[    2.541061]                    [<10008074>] 0x10008074
[    2.546249]  }
[    2.547933]  ... key      at: [<c06ee6d8>] prepare_lock+0x38/0x48
[    2.554087]  ... acquired at:
[    2.557075]    [<c007d58c>] check_irq_usage+0x50/0xb0
[    2.562180]    [<c0080560>] __lock_acquire+0x101c/0x1df8
[    2.567542]    [<c0081934>] lock_acquire+0xa4/0x158
[    2.572470]    [<c04849f8>] mutex_lock_nested+0x58/0x3e0
[    2.577831]    [<c03bc8f0>] clk_prepare_lock+0x3c/0xd8
[    2.583021]    [<c03bd3f0>] clk_get_rate+0xc/0x5c
[    2.587774]    [<c03a4940>] esdhc_pltfm_set_clock+0x14/0x1cc
[    2.593486]    [<c03a24d4>] sdhci_set_clock+0x44/0x410
[    2.598673]    [<c03a3168>] sdhci_do_set_ios+0x310/0x700
[    2.604036]    [<c0390468>] mmc_set_chip_select+0x18/0x1c
[    2.609487]    [<c0394838>] mmc_go_idle+0xdc/0xf8
[    2.614241]    [<c0392b34>] mmc_init_card+0x48/0x1424
[    2.619341]    [<c03940e0>] mmc_attach_mmc+0xc4/0x1cc
[    2.624442]    [<c039134c>] mmc_rescan+0x280/0x2d0
[    2.629282]    [<c003cfb4>] process_one_work+0x1a8/0x50c
[    2.634644]    [<c003d748>] worker_thread+0x144/0x3a4
[    2.639745]    [<c0043de0>] kthread+0xa4/0xb0
[    2.644154]    [<c000e328>] ret_from_fork+0x14/0x2c
[    2.649085]
[    2.650594]
[    2.650594] stack backtrace:
[    2.654984] CPU: 1 PID: 43 Comm: kworker/u4:2 Not tainted 3.12.9 #1
[    2.661285] Workqueue: kmmcd mmc_rescan
[    2.665188] [<c00144d4>] (unwind_backtrace+0x0/0xf8) from
[<c0011a3c>] (show_stack+0x10/0x14)
[    2.673766] [<c0011a3c>] (show_stack+0x10/0x14) from [<c0481f58>]
(dump_stack+0x64/0xa4)
[    2.681905] [<c0481f58>] (dump_stack+0x64/0xa4) from [<c007d368>]
(check_usage+0x408/0x5dc)
[    2.690302] [<c007d368>] (check_usage+0x408/0x5dc) from
[<c007d58c>] (check_irq_usage+0x50/0xb0)
[    2.699130] [<c007d58c>] (check_irq_usage+0x50/0xb0) from
[<c0080560>] (__lock_acquire+0x101c/0x1df8)
[    2.708395] [<c0080560>] (__lock_acquire+0x101c/0x1df8) from
[<c0081934>] (lock_acquire+0xa4/0x158)
[    2.717483] [<c0081934>] (lock_acquire+0xa4/0x158) from
[<c04849f8>] (mutex_lock_nested+0x58/0x3e0)
[    2.726576] [<c04849f8>] (mutex_lock_nested+0x58/0x3e0) from
[<c03bc8f0>] (clk_prepare_lock+0x3c/0xd8)
[    2.735929] [<c03bc8f0>] (clk_prepare_lock+0x3c/0xd8) from
[<c03bd3f0>] (clk_get_rate+0xc/0x5c)
[    2.744670] [<c03bd3f0>] (clk_get_rate+0xc/0x5c) from [<c03a4940>]
(esdhc_pltfm_set_clock+0x14/0x1cc)
[    2.753932] [<c03a4940>] (esdhc_pltfm_set_clock+0x14/0x1cc) from
[<c03a24d4>] (sdhci_set_clock+0x44/0x410)
[    2.763631] [<c03a24d4>] (sdhci_set_clock+0x44/0x410) from
[<c03a3168>] (sdhci_do_set_ios+0x310/0x700)
[    2.772982] [<c03a3168>] (sdhci_do_set_ios+0x310/0x700) from
[<c0390468>] (mmc_set_chip_select+0x18/0x1c)
[    2.782595] [<c0390468>] (mmc_set_chip_select+0x18/0x1c) from
[<c0394838>] (mmc_go_idle+0xdc/0xf8)
[    2.791597] [<c0394838>] (mmc_go_idle+0xdc/0xf8) from [<c0392b34>]
(mmc_init_card+0x48/0x1424)
[    2.800249] [<c0392b34>] (mmc_init_card+0x48/0x1424) from
[<c03940e0>] (mmc_attach_mmc+0xc4/0x1cc)
[    2.809251] [<c03940e0>] (mmc_attach_mmc+0xc4/0x1cc) from
[<c039134c>] (mmc_rescan+0x280/0x2d0)
[    2.817988] [<c039134c>] (mmc_rescan+0x280/0x2d0) from [<c003cfb4>]
(process_one_work+0x1a8/0x50c)
[    2.826988] [<c003cfb4>] (process_one_work+0x1a8/0x50c) from
[<c003d748>] (worker_thread+0x144/0x3a4)
[    2.836253] [<c003d748>] (worker_thread+0x144/0x3a4) from
[<c0043de0>] (kthread+0xa4/0xb0)
[    2.844562] [<c0043de0>] (kthread+0xa4/0xb0) from [<c000e328>]
(ret_from_fork+0x14/0x2c)
[    2.854183] [drm] Supports vblank timestamp caching Rev 1 (10.10.2010).
[    2.860894] [drm] No driver support for vblank timestamp query.
[    2.866875] [drm] Initialized imx-drm 1.0.0 20120507 on minor 0
[    2.874731] imx-ipuv3 2400000.ipu: IPUv3H probed
[    2.880040] imx-ipuv3 2800000.ipu: IPUv3H probed
[    2.885687] TCP: cubic registered
[    2.889041] NET: Registered protocol family 17
[    2.893719] Key type dns_resolver registered
[    2.898360] ThumbEE CPU extension supported.
[    2.902728] Registering SWP/SWPB emulation handler
[    2.908105] registered taskstats version 1
[    2.913186] ci_hdrc ci_hdrc.0: doesn't support gadget
[    2.918284] ci_hdrc ci_hdrc.0: EHCI Host Controller
[    2.923253] ci_hdrc ci_hdrc.0: new USB bus registered, assigned bus number 1
[    2.924028] mmc0: BKOPS_EN bit is not set
[    2.930922] mmc0: new high speed MMC card at address 0001
[    2.939922] mmcblk0: mmc0:0001 SEM04G 3.68 GiB
[    2.945589]  mmcblk0: p1 p2
[    2.959204] ci_hdrc ci_hdrc.0: USB 2.0 started, EHCI 1.00

Any ideas?

--
Christian Gmeiner, MSc

https://soundcloud.com/christian-gmeiner

^ permalink raw reply

* [PATCH v2] dma: Add Xilinx AXI Video Direct Memory Access Engine driver support
From: Srikanth Thokala @ 2014-02-04 10:35 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140204052810.GO10628@intel.com>

On Tue, Feb 4, 2014 at 10:58 AM, Vinod Koul <vinod.koul@intel.com> wrote:
> On Fri, Jan 31, 2014 at 12:22:52PM +0530, Srikanth Thokala wrote:
>> >>> >> [...]
>> >>> >>> +/**
>> >>> >>> + * xilinx_vdma_device_control - Configure DMA channel of the device
>> >>> >>> + * @dchan: DMA Channel pointer
>> >>> >>> + * @cmd: DMA control command
>> >>> >>> + * @arg: Channel configuration
>> >>> >>> + *
>> >>> >>> + * Return: '0' on success and failure value on error
>> >>> >>> + */
>> >>> >>> +static int xilinx_vdma_device_control(struct dma_chan *dchan,
>> >>> >>> +                                   enum dma_ctrl_cmd cmd, unsigned long arg)
>> >>> >>> +{
>> >>> >>> +     struct xilinx_vdma_chan *chan = to_xilinx_chan(dchan);
>> >>> >>> +
>> >>> >>> +     switch (cmd) {
>> >>> >>> +     case DMA_TERMINATE_ALL:
>> >>> >>> +             xilinx_vdma_terminate_all(chan);
>> >>> >>> +             return 0;
>> >>> >>> +     case DMA_SLAVE_CONFIG:
>> >>> >>> +             return xilinx_vdma_slave_config(chan,
>> >>> >>> +                                     (struct xilinx_vdma_config *)arg);
>> >>> >>
>> >>> >> You really shouldn't be overloading the generic API with your own semantics.
>> >>> >> DMA_SLAVE_CONFIG should take a dma_slave_config and nothing else.
>> >>> >
>> >>> > Ok.  The driver needs few additional configuration from the slave
>> >>> > device like Vertical
>> >>> > Size, Horizontal Size,  Stride etc., for the DMA transfers, in that case do you
>> >>> > suggest me to define a separate dma_ctrl_cmd like the one FSLDMA_EXTERNAL_START
>> >>> > defined for Freescale drivers?
>> >>>
>> >>> In my opinion it is not a good idea to have driver implement a generic API,
>> >>> but at the same time let the driver have custom semantics for those API
>> >>> calls. It's a bit like having a gpio driver that expects 23 and 42 as the
>> >>> values passed to gpio_set_value instead of 0 and 1. It completely defeats
>> >>> the purpose of a generic API, namely that you are able to write generic code
>> >>> that makes use of the API without having to know about which implementation
>> >>> API it is talking to. The dmaengine framework provides the
>> >>> dmaengine_prep_interleaved_dma() function to setup two dimensional
>> >>> transfers, e.g. take a look at sirf-dma.c or imx-dma.c.
>> >>
>> >> The question here i think would be waht this device supports? Is the hardware
>> >> capable of doing interleaved transfers, then would make sense.
>> >>
>> >> While we do try to get users use dma_slave_config, but there will always be
>> >> someone who have specfic params. If we can generalize then we might want to add
>> >> to the dma_slave_config as well
>> >
>> > There are many configuration parameters which are specific to IP and I
>> > would like to
>> > give an overview of some of parameteres here:
>> >
>> > 1) Park Mode ('cfg->park'): In Park mode, engine will park on frame
>> > referenced by
>> >     'cfg->park_frm', so user will have control on each frame in this mode.
>> >
>> > 2) Interrupt Coalesce ('cfg->coalesce'):  Used for setting interrupt
>> > threshold. This value
>> >    determines the number of frame buffers to process. To use this feature,
>> >    'cfg->frm_cnt_en' should be set.
>> >
>> > 3) Frame Synchronization Source ('cfg->ext_fsync'):  Can be an
>> > external/internal frame
>> >     synchronization source. Used to synchronize one channel (MM2S/S2MM) with
>> >     another (S2MM/MM2S) channel.
>> >
>> > 4) Genlock Synchronization ('cfg->genlock'): Used to avoid mismatch rate between
>> >     master and slave.  In master mode (cfg->master), frames are not dropped and
>> >     slave can drop frames to adjust to master frame rate.
>> >
>> > And in future, this Engine being a soft IP, we could expect some more additional
>> > parameters.  Isn't a good idea to have a private member in dma_slave_config for
>> > sharing additional configuration between slave device and dma engine? Or a new
>> > dma_ctrl_cmd like FSLDMA_EXTERNAL_START?
>
> The idea of a generic API is that we can use it for most of the controllers. Even
> if you are planning to support a family of controllers
>
> ATM, lets not discuss the possiblity of private member and try to exhanust all
> possible options. Worst case you can embed the dma_slave_config in
> xilinx_dma_slave_config and retrieve it in dmac driver

Ok.

Srikanth

>
> --
> ~Vinod
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply

* [PATCH v5 08/14] ahci-platform: "Library-ise" suspend / resume functionality
From: Hans de Goede @ 2014-02-04 10:20 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <201402031553.46083.arnd@arndb.de>

Hi,

On 02/03/2014 03:53 PM, Arnd Bergmann wrote:
> On Wednesday 22 January 2014, Hans de Goede wrote:
>> --- a/include/linux/ahci_platform.h
>> +++ b/include/linux/ahci_platform.h
>> @@ -50,4 +50,11 @@ int ahci_platform_init_host(struct platform_device *pdev,
>>                             unsigned int force_port_map,
>>                             unsigned int mask_port_map);
>>  
>> +#ifdef CONFIG_PM_SLEEP
>> +int ahci_platform_suspend_host(struct device *dev);
>> +int ahci_platform_resume_host(struct device *dev);
>> +int ahci_platform_suspend(struct device *dev);
>> +int ahci_platform_resume(struct device *dev);
>> +#endif
>> +
> 
> Not sure if the #ifdef does any good here. Normally, we don't hide declarations
> so we can do stuff like

Thanks for the review, I'll remove the #ifdef in the next version of this patch-set.

Regards,

Hans

^ permalink raw reply

* [PATCH v2 2/5] clk: sunxi: Add USB clock register defintions
From: Hans de Goede @ 2014-02-04 10:14 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140204094051.GL25625@lukather>

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi,

On 02/04/2014 10:40 AM, Maxime Ripard wrote:
> Hi Hans,
> 
> On Tue, Jan 28, 2014 at 11:00:45AM +0100, Hans de Goede wrote:
>> -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
>> 
>> Hi,
>> 
>> On 01/28/2014 10:44 AM, Maxime Ripard wrote:
>>> On Mon, Jan 27, 2014 at 03:54:14PM +0100, Hans de Goede wrote:
>>>>>> "allwinner,sun5i-a13-usb-gates-clk" - for usb gates + resets on A13
>>>>> 
>>>>> Maybe we can just remove the gates from there? Even though they are gates, they are also (a bit) more than that.
>>>> 
>>>> To be clear you mean s/usb-gates-clk/usb-clk/ right ?
>>> 
>>> Yep, exactly
>>> 
>>>>> I guess that means that we will have the OHCI0 gate declared with <&...-gates-clk 6>, while it's actually the first gate for this clock?
>>>> 
>>>> Correct.
>>>> 
>>>>> Maybe introducing an offset field in the gates_data would be a good idea, so that we always start from indexing the gates from 0 in the DT?
>>>> 
>>>> Well for the other "gates" type clks we also have holes in the range, and we always refer to the clk with the bit number in the reg as the clock-cell value.
>>> 
>>> Yes, we have holes, but I see two majors differences here: - the other gates are just gates, while the usb clocks are a bit more than that.
>> 
>> The usb-clk registers contain more then that, but the bits we are talking about now are gates.
>> 
>>> - the other gates' gating bits thus all start at bit 0, while - here, since it's kind of a "mixed" clock, the gating bits start - at bit 6 (on the A20 at least)
>> 
>> Right, still I believe that the consistent thing to do is keeping the bit-number for the bit in the register controlling the gate as the specifier.  When adding new dts entries / reviewing existing ones I'm used to matching the specifier to the bit-nr in the data-sheet, I think making things different just for this one register is counter productive.
> 
> And if you turn it the other way around, it would be inconsistent that all gates indices start at 0, and we would start at 6 here :)

I think the problem here is that you see the specifier part of the clk
phandle as an index, which it is not. All devicetree docs / code talks
about specifiers or arguments not indexes. Once you stop seeing this as
an index, you will hopefully also stop insisting this needs to
start at 0 :)

Also note that it already is not an index for existing sunxi clks which have
cells != 0, as there are holes in the bits used in the gates registers and
calling the specifier an index suggest we're dealing with an array, and
arrays never have holes.

The clk specifier as currently used in sunxi clks is a 1:1 mapping of the
gate register bit numbers, as is clearly documented in ie:
Documentation/devicetree/bindings/clock/sunxi/sun4i-a10-gates.txt
Where the datasheet is referenced as the source for (most) of the values
to put in the specifier.

My biggest objection is that this would loose 1:1 mapping we currently
have between the specifier and bit-nr in the register, which really is
convenient when writing new dts bindings.

When we add an offset users will need to first lookup which clk they need in
the datasheet and then look at both the dts bindings doc to find how this is
mapped to the specifier. In my experience such an extra level of indirection
in documentation is a PITA, and all that just so that some random number
(it is not an index!) can start at 0 ?

To me adding an offset here and making the clk gates different form all
our other clock gates just feels wrong.

Regards,

Hans
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iEYEARECAAYFAlLwvZMACgkQF3VEtJrzE/sz0gCfQNwhM/RpimtbhumvKKQ4a4V+
Vo4AoLTNKRZXlPC84hi1JInPGYvZIxuR
=dA68
-----END PGP SIGNATURE-----

^ permalink raw reply

* [PATCH 1/2] [media] v4l2: Add settings for Horizontal and Vertical MV Search Range
From: Hans Verkuil @ 2014-02-04 10:13 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1391507999-31437-2-git-send-email-amit.grover@samsung.com>

Acked-by: Hans Verkuil <hans.verkuil@cisco.com>

Regards,

	Hans

On 02/04/14 10:59, Amit Grover wrote:
> Adding V4L2 controls for horizontal and vertical search range in pixels
> for motion estimation module in video encoder.
> 
> Signed-off-by: Swami Nathan <swaminath.p@samsung.com>
> Signed-off-by: Amit Grover <amit.grover@samsung.com>
> ---
>  Documentation/DocBook/media/v4l/controls.xml |   20 ++++++++++++++++++++
>  drivers/media/v4l2-core/v4l2-ctrls.c         |    6 ++++++
>  include/uapi/linux/v4l2-controls.h           |    2 ++
>  3 files changed, 28 insertions(+)
> 
> diff --git a/Documentation/DocBook/media/v4l/controls.xml b/Documentation/DocBook/media/v4l/controls.xml
> index a5a3188..0e1770c 100644
> --- a/Documentation/DocBook/media/v4l/controls.xml
> +++ b/Documentation/DocBook/media/v4l/controls.xml
> @@ -2258,6 +2258,26 @@ Applicable to the MPEG1, MPEG2, MPEG4 encoders.</entry>
>  VBV buffer control.</entry>
>  	      </row>
>  
> +		  <row><entry></entry></row>
> +	      <row id=""v4l2-mpeg-video-hor-search-range">
> +		<entry spanname="id"><constant>V4L2_CID_MPEG_VIDEO_MV_H_SEARCH_RANGE</constant>&nbsp;</entry>
> +		<entry>integer</entry>
> +	      </row>
> +		<row><entry spanname="descr">Horizontal search range defines maximum horizontal search area in pixels
> +to search and match for the present Macroblock (MB) in the reference picture. This V4L2 control macro is used to set
> +horizontal search range for motion estimation module in video encoder.</entry>
> +	      </row>
> +
> +		 <row><entry></entry></row>
> +	      <row id="v4l2-mpeg-video-vert-search-range">
> +		<entry spanname="id"><constant>V4L2_CID_MPEG_VIDEO_MV_V_SEARCH_RANGE</constant>&nbsp;</entry>
> +		<entry>integer</entry>
> +	      </row>
> +		<row><entry spanname="descr">Vertical search range defines maximum vertical search area in pixels
> +to search and match for the present Macroblock (MB) in the reference picture. This V4L2 control macro is used to set
> +vertical search range for motion estimation module in video encoder.</entry>
> +	      </row>
> +
>  	      <row><entry></entry></row>
>  	      <row>
>  		<entry spanname="id"><constant>V4L2_CID_MPEG_VIDEO_H264_CPB_SIZE</constant>&nbsp;</entry>
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
> index 6ff002b..e9e12c4 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -735,6 +735,8 @@ const char *v4l2_ctrl_get_name(u32 id)
>  	case V4L2_CID_MPEG_VIDEO_DEC_PTS:			return "Video Decoder PTS";
>  	case V4L2_CID_MPEG_VIDEO_DEC_FRAME:			return "Video Decoder Frame Count";
>  	case V4L2_CID_MPEG_VIDEO_VBV_DELAY:			return "Initial Delay for VBV Control";
> +	case V4L2_CID_MPEG_VIDEO_MV_H_SEARCH_RANGE:		return "Horizontal MV Search Range";
> +	case V4L2_CID_MPEG_VIDEO_MV_V_SEARCH_RANGE:		return "Vertical MV Search Range";
>  	case V4L2_CID_MPEG_VIDEO_REPEAT_SEQ_HEADER:		return "Repeat Sequence Header";
>  
>  	/* VPX controls */
> @@ -910,6 +912,10 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
>  		*min = 0;
>  		*max = *step = 1;
>  		break;
> +	case V4L2_CID_MPEG_VIDEO_MV_H_SEARCH_RANGE:
> +	case V4L2_CID_MPEG_VIDEO_MV_V_SEARCH_RANGE:
> +		*type = V4L2_CTRL_TYPE_INTEGER;
> +		break;
>  	case V4L2_CID_PAN_RESET:
>  	case V4L2_CID_TILT_RESET:
>  	case V4L2_CID_FLASH_STROBE:
> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> index 2cbe605..cda6fa0 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -376,6 +376,8 @@ enum v4l2_mpeg_video_multi_slice_mode {
>  #define V4L2_CID_MPEG_VIDEO_DEC_FRAME			(V4L2_CID_MPEG_BASE+224)
>  #define V4L2_CID_MPEG_VIDEO_VBV_DELAY			(V4L2_CID_MPEG_BASE+225)
>  #define V4L2_CID_MPEG_VIDEO_REPEAT_SEQ_HEADER		(V4L2_CID_MPEG_BASE+226)
> +#define V4L2_CID_MPEG_VIDEO_MV_H_SEARCH_RANGE		(V4L2_CID_MPEG_BASE+227)
> +#define V4L2_CID_MPEG_VIDEO_MV_V_SEARCH_RANGE		(V4L2_CID_MPEG_BASE+228)
>  
>  #define V4L2_CID_MPEG_VIDEO_H263_I_FRAME_QP		(V4L2_CID_MPEG_BASE+300)
>  #define V4L2_CID_MPEG_VIDEO_H263_P_FRAME_QP		(V4L2_CID_MPEG_BASE+301)
> 

^ permalink raw reply

* [PATCH] pci: Add support for creating a generic host_bridge from device tree
From: Arnd Bergmann @ 2014-02-04 10:09 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140203221743.GB24036@e106497-lin.cambridge.arm.com>

On Monday 03 February 2014 22:17:44 Liviu Dudau wrote:
> On Mon, Feb 03, 2014 at 07:31:31PM +0000, Arnd Bergmann wrote:
> > Let's try to come up with nomenclature so we can talk about this better
> >
> > The ioport_resource is in "logical I/O space", which is a Linux fiction,
> > it goes from 0 to IO_SPACE_LIMIT (2MB on ARM) and is mapped into "virtual
> > I/O space", which start at (void __iomem *)PCI_IO_VIRT_BASE.
> >
> > Each PCI domain can have its own "bus I/O aperture", which is typically
> > between 0x1000 and 0xffff and reflects the address that is used in PCI
> > transactions and in BARs.
> 
> Actually, the bus I/O aperture can start from 0x0000 if you are talking about
> PCI bus addresses.

Right.

> > The aperture here reflects the subset of the
> > 4GB bus I/O space that is actually mapped into a CPU visible "physical
> > I/O aperture" using an inbound mapping of the host bridge. The physical
> > I/O aperture in turn gets mapped to the virtual I/O space using
> > pci_ioremap_io.
> 
> Agree.
> 
> > The difference between a bus I/O address and a logical
> > I/O address is stored in the io_offset.
> 
> Not exactly. If that would be true that means that for an I/O range that
> start at bus I/O address zero but physical I/O apperture starts at
> 0x40000000 the io_offset is zero. For me, the io_offset should be 0x40000000.

That's not how we do it on any of the existing host controllers.
Typically the io_offset is zero for the first one, and may be
either zero for all the others (meaning BARs get > 64KB values
for secondary buses) or between 64KB and 2MB (meaning each bus
starts at I/O port number 0).

> Let me see if I can summarise this correctly, using only CPU addresses:
> 
> 0x0000 - IO_SPACE_LIMIT           <-  logical I/O address
> 0xPPPPPPPP - 0xPPPPPPPP+IO_SIZE   <-  physical address for PCI I/O space
> 0xVVVVVVVV - 0xVVVVVVVV+IO_SPACE_LIMIT <- virtual address for I/O
> 
> The io_offset then is 0xPPPPPPPP - logical I/O address. At least that is
> the intent of the io_offset variable that I introduced in pci_host_bridge.

That is highly confusing then, because we already have something called
io_offset with a different meaning. I would call 0xPPPPPPPP the io_phys_base
if I had to come up with a variable name for it.

> The bus I/O address is generated by the host bridge, I think we can ignore
> it here as it tends to confuse the message.

No, it's important because the PCI core code has to transform between
bus I/O address and logical I/O address when accessing the BARs.

> > So much for basic definitions. When a device driver calls pci_request_region,
> > the port number it sees is the bus I/O port number adjusted using the
> > io_offset to turn it into a logical I/O port number, which should
> > always be within the host bridge window, which in turn is a subset
> > of the ioport_resource.
> 
> My understanding is that device drivers all user port numbers that are logical
> I/O numbers, so no io_offset needs to be applied here. It is only when one
> wants to access the port, that the translation happens. First, inb or outb
> will add the PCI_IO_VIRT_BASE to generate the virtual address, the MMU will
> then convert that address to physical address and the host bridge will
> then translate the physical address into bus address.

This is correct. The bus I/O number is not visible to the device driver,
but it is what you put into the 'ranges' property in DT, and it gets
used during PCI resource scanning.


> > > And that is why the code in probe.c has been added to deal with that. It is
> > > too early to do the adjustments here as all we have is the list of resources
> > > and that might get culled by the architecture fixup code. Remembering the
> > > io_offset will happen once the pci_host_bridge gets created, and the resources
> > > are then adjusted.
> >
> > So you want to register an incorrect I/O resource first and then
> > have it fixed up later, rather than registering the correct
> > one from the start as everyone else?
> 
> The incorrect I/O resource is added to a temporary list of resources, it has not
> been attached yet to the list of windows in the bridge. What gets added is the
> I/O resource as described if it would be an ordinary resource.

I'm not completely sure I'm following here, but let's work out the
other things first, this will probably get clearer then.

> > > > > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> > > > > index 6e34498..16febae 100644
> > > > > --- a/drivers/pci/probe.c
> > > > > +++ b/drivers/pci/probe.c
> > > > > @@ -1787,6 +1787,17 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
> > > > >     list_for_each_entry_safe(window, n, resources, list) {
> > > > >             list_move_tail(&window->list, &bridge->windows);
> > > > >             res = window->res;
> > > > > +           /*
> > > > > +            * IO resources are stored in the kernel with a CPU start
> > > > > +            * address of zero. Adjust the data accordingly and remember
> > > > > +            * the offset
> > > > > +            */
> > > > > +           if (resource_type(res) == IORESOURCE_IO) {
> > > > > +                   bridge->io_offset = res->start;
> > > > > +                   res->end -= res->start;
> > > > > +                   window->offset -= res->start;
> > > > > +                   res->start = 0;
> > > > > +           }
> 
> Here, we correct for the fact that IORESOURCE_IO is not a normal resource, because Linux wants
> a logical I/O as start and end address, not the physical CPU address. We adjust to that and
> remember the offset.

But the offset (phys_base) doesn't actually matter to the PCI core or
the driver. Why save it?

> > > > >             offset = window->offset;
> > > > >             if (res->flags & IORESOURCE_BUS)
> > > >
> > > > Won't this break all existing host bridges?
> > >
> > > I am not sure. I believe not, due to what I've explained earlier, but you might be right.
> > >
> > > The adjustment happens before the resource is added to the host bridge windows and translates
> > > it from MMIO range into IO range.
> >
> > AFAICT, the resource_type of the resource you register above should be
> > IORESOURCE_MEM, so you are not actually matching it here.
> 
> No, all resources are added here. For IORESOURCE_IO we do an adjustment.

But there should never be an IORESOURCE_IO resource structure that is
not in IO space, i.e. within ioport_resource. Doing an "adjustment"
is not an operation defined on this structure. What I meant above is that
the pci range parser gets this right and gives you a resource that looks
like { .flags = IORESOURCE_MEM, .start = phys_base, .end = phys_base +
size - 1}, while the resource we want to register is { .flags = IORESOURCE_IO,
.start = log_base, .end = log_base + size -1}. In the of_pci_range struct for
the I/O space, the "pci_space" is IORESOURCE_IO (for the pci_addr), while the
"flags" are IORESOURCE_MEM, to go along with the cpu_addr.

	Arnd

^ permalink raw reply

* 'unannotated irqs-on' lockdep warning
From: Christian Gmeiner @ 2014-02-04 10:02 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140130155741.GB15937@n2100.arm.linux.org.uk>

Hi

2014-01-30 Russell King - ARM Linux <linux@arm.linux.org.uk>:
> On Thu, Jan 30, 2014 at 03:31:46PM +0100, Christian Gmeiner wrote:
>> [   19.859234] CPU: 0 PID: 1848 Comm: mkdir Not tainted 3.12.4 #44
>> [   19.865190] [<c0013900>] (unwind_backtrace+0x0/0xe0) from
>> [<c00113b8>] (show_stack+0x10/0x14)
>> [   19.873739] [<c00113b8>] (show_stack+0x10/0x14) from [<c044e040>]
>> (dump_stack+0x64/0xa4)
>> [   19.881851] [<c044e040>] (dump_stack+0x64/0xa4) from [<c0022718>]
>> (warn_slowpath_common+0x64/0x84)
>> [   19.890828] [<c0022718>] (warn_slowpath_common+0x64/0x84) from
>> [<c00227b8>] (warn_slowpath_fmt+0x2c/0x3c)
>> [   19.900413] [<c00227b8>] (warn_slowpath_fmt+0x2c/0x3c) from
>> [<c0076c84>] (check_flags.part.26+0xb4/0x1e4)
>> [   19.910001] [<c0076c84>] (check_flags.part.26+0xb4/0x1e4) from
>> [<c0079654>] (lock_release+0x3c/0x100)
>> [   19.919243] [<c0079654>] (lock_release+0x3c/0x100) from
>> [<c00485b4>] (lg_local_unlock+0x18/0x6c)
>> [   19.928055] [<c00485b4>] (lg_local_unlock+0x18/0x6c) from
>> [<c012a2cc>] (free_fs_struct+0x18/0x30)
>> [   19.936947] [<c012a2cc>] (free_fs_struct+0x18/0x30) from
>> [<c0024e24>] (do_exit+0x2ac/0x3f0)
>> [   19.945316] [<c0024e24>] (do_exit+0x2ac/0x3f0) from [<c002501c>]
>> (do_group_exit+0x88/0xb4)
>> [   19.953596] [<c002501c>] (do_group_exit+0x88/0xb4) from
>> [<c0025058>] (__wake_up_parent+0x0/0x18)
>> [   19.962391] ---[ end trace 98a70b5cdc7b49fe ]---
>> [   19.967017] possible reason: unannotated irqs-on.
>> [   19.971729] irq event stamp: 2910
>> [   19.975050] hardirqs last  enabled at (2909): [<c044a160>]
>> __slab_free+0x1c0/0x390
>> [   19.982661] hardirqs last disabled at (2910): [<c0456d14>]
>> __dabt_svc+0x34/0x60
>
> So, I wonder how we got from __dabt_svc to __wake_up_parent.  It looks
> like the unwinder has failed to do a proper job of unwinding, which
> makes this undebuggable.
>
> Can you rebuild in ARM mode with frame pointers enabled please?
>

Maybe i am blind but I can not find that option via make menuconfig. hmmm

--
Christian Gmeiner, MSc

https://soundcloud.com/christian-gmeiner

^ permalink raw reply

* [PATCH 2/2] [media] s5p-mfc: Add Horizontal and Vertical MV Search Range
From: Amit Grover @ 2014-02-04  9:59 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1391507999-31437-1-git-send-email-amit.grover@samsung.com>

This patch adds Controls to set Horizontal and Vertical search range
for Motion Estimation block for Samsung MFC video Encoders.

Signed-off-by: Swami Nathan <swaminath.p@samsung.com>
Signed-off-by: Amit Grover <amit.grover@samsung.com>
---
 drivers/media/platform/s5p-mfc/regs-mfc-v6.h    |    1 +
 drivers/media/platform/s5p-mfc/s5p_mfc_common.h |    2 ++
 drivers/media/platform/s5p-mfc/s5p_mfc_enc.c    |   24 +++++++++++++++++++++++
 drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c |    8 ++------
 4 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/drivers/media/platform/s5p-mfc/regs-mfc-v6.h b/drivers/media/platform/s5p-mfc/regs-mfc-v6.h
index 2398cdf..8d0b686 100644
--- a/drivers/media/platform/s5p-mfc/regs-mfc-v6.h
+++ b/drivers/media/platform/s5p-mfc/regs-mfc-v6.h
@@ -229,6 +229,7 @@
 #define S5P_FIMV_E_PADDING_CTRL_V6		0xf7a4
 #define S5P_FIMV_E_MV_HOR_RANGE_V6		0xf7ac
 #define S5P_FIMV_E_MV_VER_RANGE_V6		0xf7b0
+#define S5P_FIMV_E_MV_RANGE_V6_MASK		0x3fff
 
 #define S5P_FIMV_E_VBV_BUFFER_SIZE_V6		0xf84c
 #define S5P_FIMV_E_VBV_INIT_DELAY_V6		0xf850
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
index f723f1f..5c28cc3 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
@@ -426,6 +426,8 @@ struct s5p_mfc_vp8_enc_params {
 struct s5p_mfc_enc_params {
 	u16 width;
 	u16 height;
+	u32 mv_h_range;
+	u32 mv_v_range;
 
 	u16 gop_size;
 	enum v4l2_mpeg_video_multi_slice_mode slice_mode;
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
index 91b6e02..df83cd1 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
@@ -208,6 +208,24 @@ static struct mfc_control controls[] = {
 		.default_value = 0,
 	},
 	{
+		.id = V4L2_CID_MPEG_VIDEO_MV_H_SEARCH_RANGE,
+		.type = V4L2_CTRL_TYPE_INTEGER,
+		.name = "Horizontal MV Search Range",
+		.minimum = 16,
+		.maximum = 128,
+		.step = 16,
+		.default_value = 32,
+	},
+	{
+		.id = V4L2_CID_MPEG_VIDEO_MV_V_SEARCH_RANGE,
+		.type = V4L2_CTRL_TYPE_INTEGER,
+		.name = "Vertical MV Search Range",
+		.minimum = 16,
+		.maximum = 128,
+		.step = 16,
+		.default_value = 32,
+	},
+	{
 		.id = V4L2_CID_MPEG_VIDEO_H264_CPB_SIZE,
 		.type = V4L2_CTRL_TYPE_INTEGER,
 		.minimum = 0,
@@ -1417,6 +1435,12 @@ static int s5p_mfc_enc_s_ctrl(struct v4l2_ctrl *ctrl)
 	case V4L2_CID_MPEG_VIDEO_VBV_SIZE:
 		p->vbv_size = ctrl->val;
 		break;
+	case V4L2_CID_MPEG_VIDEO_MV_H_SEARCH_RANGE:
+		p->mv_h_range = ctrl->val;
+		break;
+	case V4L2_CID_MPEG_VIDEO_MV_V_SEARCH_RANGE:
+		p->mv_v_range = ctrl->val;
+		break;
 	case V4L2_CID_MPEG_VIDEO_H264_CPB_SIZE:
 		p->codec.h264.cpb_size = ctrl->val;
 		break;
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
index f6ff2db..f64621a 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
@@ -727,14 +727,10 @@ static int s5p_mfc_set_enc_params(struct s5p_mfc_ctx *ctx)
 	WRITEL(reg, S5P_FIMV_E_RC_CONFIG_V6);
 
 	/* setting for MV range [16, 256] */
-	reg = 0;
-	reg &= ~(0x3FFF);
-	reg = 256;
+	reg = (p->mv_h_range & S5P_FIMV_E_MV_RANGE_V6_MASK);
 	WRITEL(reg, S5P_FIMV_E_MV_HOR_RANGE_V6);
 
-	reg = 0;
-	reg &= ~(0x3FFF);
-	reg = 256;
+	reg = (p->mv_v_range & S5P_FIMV_E_MV_RANGE_V6_MASK);
 	WRITEL(reg, S5P_FIMV_E_MV_VER_RANGE_V6);
 
 	WRITEL(0x0, S5P_FIMV_E_FRAME_INSERTION_V6);
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 1/2] [media] v4l2: Add settings for Horizontal and Vertical MV Search Range
From: Amit Grover @ 2014-02-04  9:59 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1391507999-31437-1-git-send-email-amit.grover@samsung.com>

Adding V4L2 controls for horizontal and vertical search range in pixels
for motion estimation module in video encoder.

Signed-off-by: Swami Nathan <swaminath.p@samsung.com>
Signed-off-by: Amit Grover <amit.grover@samsung.com>
---
 Documentation/DocBook/media/v4l/controls.xml |   20 ++++++++++++++++++++
 drivers/media/v4l2-core/v4l2-ctrls.c         |    6 ++++++
 include/uapi/linux/v4l2-controls.h           |    2 ++
 3 files changed, 28 insertions(+)

diff --git a/Documentation/DocBook/media/v4l/controls.xml b/Documentation/DocBook/media/v4l/controls.xml
index a5a3188..0e1770c 100644
--- a/Documentation/DocBook/media/v4l/controls.xml
+++ b/Documentation/DocBook/media/v4l/controls.xml
@@ -2258,6 +2258,26 @@ Applicable to the MPEG1, MPEG2, MPEG4 encoders.</entry>
 VBV buffer control.</entry>
 	      </row>
 
+		  <row><entry></entry></row>
+	      <row id=""v4l2-mpeg-video-hor-search-range">
+		<entry spanname="id"><constant>V4L2_CID_MPEG_VIDEO_MV_H_SEARCH_RANGE</constant>&nbsp;</entry>
+		<entry>integer</entry>
+	      </row>
+		<row><entry spanname="descr">Horizontal search range defines maximum horizontal search area in pixels
+to search and match for the present Macroblock (MB) in the reference picture. This V4L2 control macro is used to set
+horizontal search range for motion estimation module in video encoder.</entry>
+	      </row>
+
+		 <row><entry></entry></row>
+	      <row id="v4l2-mpeg-video-vert-search-range">
+		<entry spanname="id"><constant>V4L2_CID_MPEG_VIDEO_MV_V_SEARCH_RANGE</constant>&nbsp;</entry>
+		<entry>integer</entry>
+	      </row>
+		<row><entry spanname="descr">Vertical search range defines maximum vertical search area in pixels
+to search and match for the present Macroblock (MB) in the reference picture. This V4L2 control macro is used to set
+vertical search range for motion estimation module in video encoder.</entry>
+	      </row>
+
 	      <row><entry></entry></row>
 	      <row>
 		<entry spanname="id"><constant>V4L2_CID_MPEG_VIDEO_H264_CPB_SIZE</constant>&nbsp;</entry>
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
index 6ff002b..e9e12c4 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -735,6 +735,8 @@ const char *v4l2_ctrl_get_name(u32 id)
 	case V4L2_CID_MPEG_VIDEO_DEC_PTS:			return "Video Decoder PTS";
 	case V4L2_CID_MPEG_VIDEO_DEC_FRAME:			return "Video Decoder Frame Count";
 	case V4L2_CID_MPEG_VIDEO_VBV_DELAY:			return "Initial Delay for VBV Control";
+	case V4L2_CID_MPEG_VIDEO_MV_H_SEARCH_RANGE:		return "Horizontal MV Search Range";
+	case V4L2_CID_MPEG_VIDEO_MV_V_SEARCH_RANGE:		return "Vertical MV Search Range";
 	case V4L2_CID_MPEG_VIDEO_REPEAT_SEQ_HEADER:		return "Repeat Sequence Header";
 
 	/* VPX controls */
@@ -910,6 +912,10 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 		*min = 0;
 		*max = *step = 1;
 		break;
+	case V4L2_CID_MPEG_VIDEO_MV_H_SEARCH_RANGE:
+	case V4L2_CID_MPEG_VIDEO_MV_V_SEARCH_RANGE:
+		*type = V4L2_CTRL_TYPE_INTEGER;
+		break;
 	case V4L2_CID_PAN_RESET:
 	case V4L2_CID_TILT_RESET:
 	case V4L2_CID_FLASH_STROBE:
diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
index 2cbe605..cda6fa0 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -376,6 +376,8 @@ enum v4l2_mpeg_video_multi_slice_mode {
 #define V4L2_CID_MPEG_VIDEO_DEC_FRAME			(V4L2_CID_MPEG_BASE+224)
 #define V4L2_CID_MPEG_VIDEO_VBV_DELAY			(V4L2_CID_MPEG_BASE+225)
 #define V4L2_CID_MPEG_VIDEO_REPEAT_SEQ_HEADER		(V4L2_CID_MPEG_BASE+226)
+#define V4L2_CID_MPEG_VIDEO_MV_H_SEARCH_RANGE		(V4L2_CID_MPEG_BASE+227)
+#define V4L2_CID_MPEG_VIDEO_MV_V_SEARCH_RANGE		(V4L2_CID_MPEG_BASE+228)
 
 #define V4L2_CID_MPEG_VIDEO_H263_I_FRAME_QP		(V4L2_CID_MPEG_BASE+300)
 #define V4L2_CID_MPEG_VIDEO_H263_P_FRAME_QP		(V4L2_CID_MPEG_BASE+301)
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v3 0/2] drivers/media: Add controls for Horizontal and Vertical MV Search Range
From: Amit Grover @ 2014-02-04  9:59 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <52E0ED10.2020901@samsung.com>

This is v3 version for the patch:
s5p-mfc: Add Horizontal and Vertical search range for Video Macro Blocks
(https://lkml.org/lkml/2013/12/30/83)

Changes from v1:
1) Splitted the patch into V4L2 and MFC driver patches.
2) Incorporated review comments on v1.

Changes from v2:
1) Removed the range restrictions from drivers/media/v4l2-core/v4l2-ctrls.c.
2) Rebased the patch to git://linuxtv.org/mchehab/media-next.git.
3) Added [media] tag in title.

Amit Grover (2):
  [media] v4l2: Add settings for Horizontal and Vertical MV Search
    Range
  [media] s5p-mfc: Add Horizontal and Vertical MV Search Range

 Documentation/DocBook/media/v4l/controls.xml    |   20 +++++++++++++++++++
 drivers/media/platform/s5p-mfc/regs-mfc-v6.h    |    1 +
 drivers/media/platform/s5p-mfc/s5p_mfc_common.h |    2 ++
 drivers/media/platform/s5p-mfc/s5p_mfc_enc.c    |   24 +++++++++++++++++++++++
 drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c |    8 ++------
 drivers/media/v4l2-core/v4l2-ctrls.c            |    6 ++++++
 include/uapi/linux/v4l2-controls.h              |    2 ++
 7 files changed, 57 insertions(+), 6 deletions(-)

-- 
1.7.9.5

^ permalink raw reply

* [PATCH v3 4/4] ARM/ARM64: KVM: Allow KVM_ARM_VCPU_PSCI_0_2 feature for user space
From: Anup Patel @ 2014-02-04  9:48 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1391507296-2099-1-git-send-email-anup.patel@linaro.org>

Allow user space to set KVM_ARM_VCPU_PSCI_0_2 feature because all
mandatory PSCI v0.2 functions (i.e. complete PSCI v0.2) are available.

Signed-off-by: Anup Patel <anup.patel@linaro.org>
Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
---
 arch/arm/kvm/guest.c   |    3 ---
 arch/arm64/kvm/guest.c |    3 ---
 2 files changed, 6 deletions(-)

diff --git a/arch/arm/kvm/guest.c b/arch/arm/kvm/guest.c
index 89929b6..b23a59c 100644
--- a/arch/arm/kvm/guest.c
+++ b/arch/arm/kvm/guest.c
@@ -307,9 +307,6 @@ int kvm_vcpu_set_target(struct kvm_vcpu *vcpu,
 		if (test_bit(i, (void *)init->features)) {
 			if (i >= KVM_VCPU_MAX_FEATURES)
 				return -ENOENT;
-			/* Don't allow setting experimental features */
-			if (i == KVM_ARM_VCPU_PSCI_0_2)
-				return -ENOENT;
 			set_bit(i, vcpu->arch.features);
 		}
 	}
diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
index b27877c..0874557 100644
--- a/arch/arm64/kvm/guest.c
+++ b/arch/arm64/kvm/guest.c
@@ -246,9 +246,6 @@ int kvm_vcpu_set_target(struct kvm_vcpu *vcpu,
 		if (init->features[i / 32] & (1 << (i % 32))) {
 			if (i >= KVM_VCPU_MAX_FEATURES)
 				return -ENOENT;
-			/* Don't allow setting experimental features */
-			if (i == KVM_ARM_VCPU_PSCI_0_2)
-				return -ENOENT;
 			set_bit(i, vcpu->arch.features);
 		}
 	}
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v3 3/4] KVM: Documentation: Add info regarding KVM_ARM_VCPU_PSCI_0_2 feature
From: Anup Patel @ 2014-02-04  9:48 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1391507296-2099-1-git-send-email-anup.patel@linaro.org>

We have in-kernel emulation of PSCI v0.2 in KVM ARM/ARM64. To provide
PSCI v0.2 interface to VCPUs, we have to enable KVM_ARM_VCPU_PSCI_0_2
feature when doing KVM_ARM_VCPU_INIT ioctl.

The patch updates documentation of KVM_ARM_VCPU_INIT ioctl to provide
info regarding KVM_ARM_VCPU_PSCI_0_2 feature.

Signed-off-by: Anup Patel <anup.patel@linaro.org>
Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
---
 Documentation/virtual/kvm/api.txt |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index 6cd63a9..73cb211 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -2347,6 +2347,8 @@ Possible features:
 	  Depends on KVM_CAP_ARM_PSCI.
 	- KVM_ARM_VCPU_EL1_32BIT: Starts the CPU in a 32bit mode.
 	  Depends on KVM_CAP_ARM_EL1_32BIT (arm64 only).
+	- KVM_ARM_VCPU_PSCI_0_2: Emulate PSCI v0.2 for the CPU.
+	  Depends on KVM_CAP_ARM_PSCI_0_2.
 
 
 4.83 KVM_ARM_PREFERRED_TARGET
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v3 2/4] KVM: Add capability to advertise PSCI v0.2 support
From: Anup Patel @ 2014-02-04  9:48 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1391507296-2099-1-git-send-email-anup.patel@linaro.org>

User space (i.e. QEMU or KVMTOOL) should be able to check whether KVM
ARM/ARM64 supports in-kernel PSCI v0.2 emulation. For this purpose, we
define KVM_CAP_ARM_PSCI_0_2 in KVM user space interface header.

Signed-off-by: Anup Patel <anup.patel@linaro.org>
Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
---
 arch/arm/kvm/arm.c       |    1 +
 include/uapi/linux/kvm.h |    1 +
 2 files changed, 2 insertions(+)

diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index 1d8248e..c8a71df 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -197,6 +197,7 @@ int kvm_dev_ioctl_check_extension(long ext)
 	case KVM_CAP_DESTROY_MEMORY_REGION_WORKS:
 	case KVM_CAP_ONE_REG:
 	case KVM_CAP_ARM_PSCI:
+	case KVM_CAP_ARM_PSCI_0_2:
 		r = 1;
 		break;
 	case KVM_CAP_COALESCED_MMIO:
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 932d7f2..fb3c3f3 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -675,6 +675,7 @@ struct kvm_ppc_smmu_info {
 #define KVM_CAP_SPAPR_MULTITCE 94
 #define KVM_CAP_EXT_EMUL_CPUID 95
 #define KVM_CAP_HYPERV_TIME 96
+#define KVM_CAP_ARM_PSCI_0_2 97
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v3 1/4] ARM/ARM64: KVM: Add support for PSCI v0.2 emulation
From: Anup Patel @ 2014-02-04  9:48 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1391507296-2099-1-git-send-email-anup.patel@linaro.org>

Currently, the in-kernel PSCI emulation provides PSCI v0.1 interface to
VCPUs. This patch extends current in-kernel PSCI emulation to provide
PSCI v0.2 interface to VCPUs.

By default, ARM/ARM64 KVM will always provide PSCI v0.1 interface for
keeping the ABI backward-compatible.

To select PSCI v0.2 interface for VCPUs, the user space (i.e. QEMU or
KVMTOOL) will have to set KVM_ARM_VCPU_PSCI_0_2 feature when doing VCPU
init using KVM_ARM_VCPU_INIT ioctl.

Please note that we do not provide all mandatory functions required
by PSCI v0.2 which makes this implemenation as a base for extension
by subsequent patches. This also means KVM_ARM_VCPU_PSCI_0_2 feature
is experimental and will not be available to user space untill all
PSCI v0.2 mandatory functions are provided by KVM ARM/ARM64.

Signed-off-by: Anup Patel <anup.patel@linaro.org>
Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
---
 arch/arm/include/asm/kvm_host.h   |    2 +-
 arch/arm/include/asm/kvm_psci.h   |    4 ++
 arch/arm/include/uapi/asm/kvm.h   |   35 ++++++++++++++-
 arch/arm/kvm/guest.c              |    3 ++
 arch/arm/kvm/psci.c               |   85 +++++++++++++++++++++++++++++++------
 arch/arm64/include/asm/kvm_host.h |    2 +-
 arch/arm64/include/asm/kvm_psci.h |    4 ++
 arch/arm64/include/uapi/asm/kvm.h |   35 ++++++++++++++-
 arch/arm64/kvm/guest.c            |    3 ++
 9 files changed, 157 insertions(+), 16 deletions(-)

diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index 09af149..193ceaf 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -36,7 +36,7 @@
 #define KVM_COALESCED_MMIO_PAGE_OFFSET 1
 #define KVM_HAVE_ONE_REG
 
-#define KVM_VCPU_MAX_FEATURES 1
+#define KVM_VCPU_MAX_FEATURES 2
 
 #include <kvm/arm_vgic.h>
 
diff --git a/arch/arm/include/asm/kvm_psci.h b/arch/arm/include/asm/kvm_psci.h
index 9a83d98..4c0e3e1 100644
--- a/arch/arm/include/asm/kvm_psci.h
+++ b/arch/arm/include/asm/kvm_psci.h
@@ -18,6 +18,10 @@
 #ifndef __ARM_KVM_PSCI_H__
 #define __ARM_KVM_PSCI_H__
 
+#define KVM_ARM_PSCI_0_1	1
+#define KVM_ARM_PSCI_0_2	2
+
+int kvm_psci_version(struct kvm_vcpu *vcpu);
 bool kvm_psci_call(struct kvm_vcpu *vcpu);
 
 #endif /* __ARM_KVM_PSCI_H__ */
diff --git a/arch/arm/include/uapi/asm/kvm.h b/arch/arm/include/uapi/asm/kvm.h
index ef0c878..9c922d9 100644
--- a/arch/arm/include/uapi/asm/kvm.h
+++ b/arch/arm/include/uapi/asm/kvm.h
@@ -83,6 +83,7 @@ struct kvm_regs {
 #define KVM_VGIC_V2_CPU_SIZE		0x2000
 
 #define KVM_ARM_VCPU_POWER_OFF		0 /* CPU is started in OFF state */
+#define KVM_ARM_VCPU_PSCI_0_2		1 /* CPU uses PSCI v0.2 */
 
 struct kvm_vcpu_init {
 	__u32 target;
@@ -192,7 +193,7 @@ struct kvm_arch_memory_slot {
 /* Highest supported SPI, from VGIC_NR_IRQS */
 #define KVM_ARM_IRQ_GIC_MAX		127
 
-/* PSCI interface */
+/* PSCI v0.1 interface */
 #define KVM_PSCI_FN_BASE		0x95c1ba5e
 #define KVM_PSCI_FN(n)			(KVM_PSCI_FN_BASE + (n))
 
@@ -201,9 +202,41 @@ struct kvm_arch_memory_slot {
 #define KVM_PSCI_FN_CPU_ON		KVM_PSCI_FN(2)
 #define KVM_PSCI_FN_MIGRATE		KVM_PSCI_FN(3)
 
+/* PSCI v0.2 interface */
+#define KVM_PSCI_0_2_FN_BASE		0x84000000
+#define KVM_PSCI_0_2_FN(n)		(KVM_PSCI_0_2_FN_BASE + (n))
+#define KVM_PSCI_0_2_FN64_BASE		0xC4000000
+#define KVM_PSCI_0_2_FN64(n)		(KVM_PSCI_0_2_FN64_BASE + (n))
+
+#define KVM_PSCI_0_2_FN_PSCI_VERSION	KVM_PSCI_0_2_FN(0)
+#define KVM_PSCI_0_2_FN_CPU_SUSPEND	KVM_PSCI_0_2_FN(1)
+#define KVM_PSCI_0_2_FN_CPU_OFF		KVM_PSCI_0_2_FN(2)
+#define KVM_PSCI_0_2_FN_CPU_ON		KVM_PSCI_0_2_FN(3)
+#define KVM_PSCI_0_2_FN_AFFINITY_INFO	KVM_PSCI_0_2_FN(4)
+#define KVM_PSCI_0_2_FN_MIGRATE		KVM_PSCI_0_2_FN(5)
+#define KVM_PSCI_0_2_FN_MIGRATE_INFO_TYPE \
+					KVM_PSCI_0_2_FN(6)
+#define KVM_PSCI_0_2_FN_MIGRATE_INFO_UP_CPU \
+					KVM_PSCI_0_2_FN(7)
+#define KVM_PSCI_0_2_FN_SYSTEM_OFF	KVM_PSCI_0_2_FN(8)
+#define KVM_PSCI_0_2_FN_SYSTEM_RESET	KVM_PSCI_0_2_FN(9)
+
+#define KVM_PSCI_0_2_FN64_CPU_SUSPEND	KVM_PSCI_0_2_FN64(1)
+#define KVM_PSCI_0_2_FN64_CPU_ON	KVM_PSCI_0_2_FN64(3)
+#define KVM_PSCI_0_2_FN64_AFFINITY_INFO	KVM_PSCI_0_2_FN64(4)
+#define KVM_PSCI_0_2_FN64_MIGRATE	KVM_PSCI_0_2_FN64(5)
+#define KVM_PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU \
+					KVM_PSCI_0_2_FN64(7)
+
+/* PSCI return values */
 #define KVM_PSCI_RET_SUCCESS		0
 #define KVM_PSCI_RET_NI			((unsigned long)-1)
 #define KVM_PSCI_RET_INVAL		((unsigned long)-2)
 #define KVM_PSCI_RET_DENIED		((unsigned long)-3)
+#define KVM_PSCI_RET_ALREADY_ON		((unsigned long)-4)
+#define KVM_PSCI_RET_ON_PENDING		((unsigned long)-5)
+#define KVM_PSCI_RET_INTERNAL_FAILURE	((unsigned long)-6)
+#define KVM_PSCI_RET_NOT_PRESENT	((unsigned long)-7)
+#define KVM_PSCI_RET_DISABLED		((unsigned long)-8)
 
 #endif /* __ARM_KVM_H__ */
diff --git a/arch/arm/kvm/guest.c b/arch/arm/kvm/guest.c
index b23a59c..89929b6 100644
--- a/arch/arm/kvm/guest.c
+++ b/arch/arm/kvm/guest.c
@@ -307,6 +307,9 @@ int kvm_vcpu_set_target(struct kvm_vcpu *vcpu,
 		if (test_bit(i, (void *)init->features)) {
 			if (i >= KVM_VCPU_MAX_FEATURES)
 				return -ENOENT;
+			/* Don't allow setting experimental features */
+			if (i == KVM_ARM_VCPU_PSCI_0_2)
+				return -ENOENT;
 			set_bit(i, vcpu->arch.features);
 		}
 	}
diff --git a/arch/arm/kvm/psci.c b/arch/arm/kvm/psci.c
index 448f60e..e4ec4af 100644
--- a/arch/arm/kvm/psci.c
+++ b/arch/arm/kvm/psci.c
@@ -85,17 +85,57 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
 	return KVM_PSCI_RET_SUCCESS;
 }
 
-/**
- * kvm_psci_call - handle PSCI call if r0 value is in range
- * @vcpu: Pointer to the VCPU struct
- *
- * Handle PSCI calls from guests through traps from HVC instructions.
- * The calling convention is similar to SMC calls to the secure world where
- * the function number is placed in r0 and this function returns true if the
- * function number specified in r0 is withing the PSCI range, and false
- * otherwise.
- */
-bool kvm_psci_call(struct kvm_vcpu *vcpu)
+int kvm_psci_version(struct kvm_vcpu *vcpu)
+{
+	if (test_bit(KVM_ARM_VCPU_PSCI_0_2, vcpu->arch.features))
+		return KVM_ARM_PSCI_0_2;
+
+	return KVM_ARM_PSCI_0_1;
+}
+
+static bool kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
+{
+	unsigned long psci_fn = *vcpu_reg(vcpu, 0) & ~((u32) 0);
+	unsigned long val;
+
+	switch (psci_fn) {
+	case KVM_PSCI_0_2_FN_PSCI_VERSION:
+		/*
+		 * Bits[31:16] = Major Version = 0
+		 * Bits[15:0] = Minor Version = 2
+		 */
+		val = 2;
+		break;
+	case KVM_PSCI_0_2_FN_CPU_OFF:
+		kvm_psci_vcpu_off(vcpu);
+		val = KVM_PSCI_RET_SUCCESS;
+		break;
+	case KVM_PSCI_0_2_FN_CPU_ON:
+	case KVM_PSCI_0_2_FN64_CPU_ON:
+		val = kvm_psci_vcpu_on(vcpu);
+		break;
+	case KVM_PSCI_0_2_FN_CPU_SUSPEND:
+	case KVM_PSCI_0_2_FN_AFFINITY_INFO:
+	case KVM_PSCI_0_2_FN_MIGRATE:
+	case KVM_PSCI_0_2_FN_MIGRATE_INFO_TYPE:
+	case KVM_PSCI_0_2_FN_MIGRATE_INFO_UP_CPU:
+	case KVM_PSCI_0_2_FN_SYSTEM_OFF:
+	case KVM_PSCI_0_2_FN_SYSTEM_RESET:
+	case KVM_PSCI_0_2_FN64_CPU_SUSPEND:
+	case KVM_PSCI_0_2_FN64_AFFINITY_INFO:
+	case KVM_PSCI_0_2_FN64_MIGRATE:
+	case KVM_PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU:
+		val = KVM_PSCI_RET_NI;
+		break;
+	default:
+		return false;
+	}
+
+	*vcpu_reg(vcpu, 0) = val;
+	return true;
+}
+
+static bool kvm_psci_0_1_call(struct kvm_vcpu *vcpu)
 {
 	unsigned long psci_fn = *vcpu_reg(vcpu, 0) & ~((u32) 0);
 	unsigned long val;
@@ -112,7 +152,6 @@ bool kvm_psci_call(struct kvm_vcpu *vcpu)
 	case KVM_PSCI_FN_MIGRATE:
 		val = KVM_PSCI_RET_NI;
 		break;
-
 	default:
 		return false;
 	}
@@ -120,3 +159,25 @@ bool kvm_psci_call(struct kvm_vcpu *vcpu)
 	*vcpu_reg(vcpu, 0) = val;
 	return true;
 }
+
+/**
+ * kvm_psci_call - handle PSCI call if r0 value is in range
+ * @vcpu: Pointer to the VCPU struct
+ *
+ * Handle PSCI calls from guests through traps from HVC instructions.
+ * The calling convention is similar to SMC calls to the secure world where
+ * the function number is placed in r0 and this function returns true if the
+ * function number specified in r0 is withing the PSCI range, and false
+ * otherwise.
+ */
+bool kvm_psci_call(struct kvm_vcpu *vcpu)
+{
+	switch (kvm_psci_version(vcpu)) {
+	case KVM_ARM_PSCI_0_2:
+		return kvm_psci_0_2_call(vcpu);
+	case KVM_ARM_PSCI_0_1:
+		return kvm_psci_0_1_call(vcpu);
+	default:
+		return false;
+	};
+}
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 0a1d697..92242ce 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -39,7 +39,7 @@
 #include <kvm/arm_vgic.h>
 #include <kvm/arm_arch_timer.h>
 
-#define KVM_VCPU_MAX_FEATURES 2
+#define KVM_VCPU_MAX_FEATURES 3
 
 struct kvm_vcpu;
 int kvm_target_cpu(void);
diff --git a/arch/arm64/include/asm/kvm_psci.h b/arch/arm64/include/asm/kvm_psci.h
index e301a48..e25c658 100644
--- a/arch/arm64/include/asm/kvm_psci.h
+++ b/arch/arm64/include/asm/kvm_psci.h
@@ -18,6 +18,10 @@
 #ifndef __ARM64_KVM_PSCI_H__
 #define __ARM64_KVM_PSCI_H__
 
+#define KVM_ARM_PSCI_0_1	1
+#define KVM_ARM_PSCI_0_2	2
+
+int kvm_psci_version(struct kvm_vcpu *vcpu);
 bool kvm_psci_call(struct kvm_vcpu *vcpu);
 
 #endif /* __ARM64_KVM_PSCI_H__ */
diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
index 495ab6f..31c2f54 100644
--- a/arch/arm64/include/uapi/asm/kvm.h
+++ b/arch/arm64/include/uapi/asm/kvm.h
@@ -77,6 +77,7 @@ struct kvm_regs {
 
 #define KVM_ARM_VCPU_POWER_OFF		0 /* CPU is started in OFF state */
 #define KVM_ARM_VCPU_EL1_32BIT		1 /* CPU running a 32bit VM */
+#define KVM_ARM_VCPU_PSCI_0_2		2 /* CPU uses PSCI v0.2 */
 
 struct kvm_vcpu_init {
 	__u32 target;
@@ -168,7 +169,7 @@ struct kvm_arch_memory_slot {
 /* Highest supported SPI, from VGIC_NR_IRQS */
 #define KVM_ARM_IRQ_GIC_MAX		127
 
-/* PSCI interface */
+/* PSCI v0.1 interface */
 #define KVM_PSCI_FN_BASE		0x95c1ba5e
 #define KVM_PSCI_FN(n)			(KVM_PSCI_FN_BASE + (n))
 
@@ -177,10 +178,42 @@ struct kvm_arch_memory_slot {
 #define KVM_PSCI_FN_CPU_ON		KVM_PSCI_FN(2)
 #define KVM_PSCI_FN_MIGRATE		KVM_PSCI_FN(3)
 
+/* PSCI v0.2 interface */
+#define KVM_PSCI_0_2_FN_BASE		0x84000000
+#define KVM_PSCI_0_2_FN(n)		(KVM_PSCI_0_2_FN_BASE + (n))
+#define KVM_PSCI_0_2_FN64_BASE		0xC4000000
+#define KVM_PSCI_0_2_FN64(n)		(KVM_PSCI_0_2_FN64_BASE + (n))
+
+#define KVM_PSCI_0_2_FN_PSCI_VERSION	KVM_PSCI_0_2_FN(0)
+#define KVM_PSCI_0_2_FN_CPU_SUSPEND	KVM_PSCI_0_2_FN(1)
+#define KVM_PSCI_0_2_FN_CPU_OFF		KVM_PSCI_0_2_FN(2)
+#define KVM_PSCI_0_2_FN_CPU_ON		KVM_PSCI_0_2_FN(3)
+#define KVM_PSCI_0_2_FN_AFFINITY_INFO	KVM_PSCI_0_2_FN(4)
+#define KVM_PSCI_0_2_FN_MIGRATE		KVM_PSCI_0_2_FN(5)
+#define KVM_PSCI_0_2_FN_MIGRATE_INFO_TYPE \
+					KVM_PSCI_0_2_FN(6)
+#define KVM_PSCI_0_2_FN_MIGRATE_INFO_UP_CPU \
+					KVM_PSCI_0_2_FN(7)
+#define KVM_PSCI_0_2_FN_SYSTEM_OFF	KVM_PSCI_0_2_FN(8)
+#define KVM_PSCI_0_2_FN_SYSTEM_RESET	KVM_PSCI_0_2_FN(9)
+
+#define KVM_PSCI_0_2_FN64_CPU_SUSPEND	KVM_PSCI_0_2_FN64(1)
+#define KVM_PSCI_0_2_FN64_CPU_ON	KVM_PSCI_0_2_FN64(3)
+#define KVM_PSCI_0_2_FN64_AFFINITY_INFO	KVM_PSCI_0_2_FN64(4)
+#define KVM_PSCI_0_2_FN64_MIGRATE	KVM_PSCI_0_2_FN64(5)
+#define KVM_PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU \
+					KVM_PSCI_0_2_FN64(7)
+
+/* PSCI return values */
 #define KVM_PSCI_RET_SUCCESS		0
 #define KVM_PSCI_RET_NI			((unsigned long)-1)
 #define KVM_PSCI_RET_INVAL		((unsigned long)-2)
 #define KVM_PSCI_RET_DENIED		((unsigned long)-3)
+#define KVM_PSCI_RET_ALREADY_ON		((unsigned long)-4)
+#define KVM_PSCI_RET_ON_PENDING		((unsigned long)-5)
+#define KVM_PSCI_RET_INTERNAL_FAILURE	((unsigned long)-6)
+#define KVM_PSCI_RET_NOT_PRESENT	((unsigned long)-7)
+#define KVM_PSCI_RET_DISABLED		((unsigned long)-8)
 
 #endif
 
diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
index 0874557..b27877c 100644
--- a/arch/arm64/kvm/guest.c
+++ b/arch/arm64/kvm/guest.c
@@ -246,6 +246,9 @@ int kvm_vcpu_set_target(struct kvm_vcpu *vcpu,
 		if (init->features[i / 32] & (1 << (i % 32))) {
 			if (i >= KVM_VCPU_MAX_FEATURES)
 				return -ENOENT;
+			/* Don't allow setting experimental features */
+			if (i == KVM_ARM_VCPU_PSCI_0_2)
+				return -ENOENT;
 			set_bit(i, vcpu->arch.features);
 		}
 	}
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v3 0/4] In-kernel PSCI v0.2 emulation for KVM ARM/ARM64
From: Anup Patel @ 2014-02-04  9:48 UTC (permalink / raw)
  To: linux-arm-kernel

Currently, KVM ARM/ARM64 only provides in-kernel emulation of Power State
and Coordination Interface (PSCI) v0.1.

This patchset aims at providing newer PSCI v0.2 for KVM ARM/ARM64 VCPUs
such that it does not break current KVM ARM/ARM64 ABI. Also, the patchset
provides emulation of only few PSCI v0.2 functions such as PSCI_VERSION,
CPU_ON, and CPU_OFF. Emulation of other PSCI v0.2 functions will be added
later.

The user space tools (i.e. QEMU or KVMTOOL) will have to explicitly enable
KVM_ARM_VCPU_PSCI_0_2 feature using KVM_ARM_VCPU_INIT ioctl for providing
PSCI v0.2 to VCPUs.

Changlog:

V3:
 - Make KVM_ARM_VCPU_PSCI_0_2 feature experiementatl for now so that
   it fails for user space till all mandatory PSCI v0.2 functions are
   emulated by KVM ARM/ARM64
 - Have separate patch for making KVM_ARM_VCPU_PSCI_0_2 feature available
   to user space. This patch can be defferred for now.

V2:
 - Don't rename PSCI return values KVM_PSCI_RET_NI and KVM_PSCI_RET_INVAL
 - Added kvm_psci_version() to get PSCI version available to VCPU
 - Fixed grammer in Documentation/virtual/kvm/api.txt

V1:
 - Initial RFC PATCH

Anup Patel (4):
  ARM/ARM64: KVM: Add support for PSCI v0.2 emulation
  KVM: Add capability to advertise PSCI v0.2 support
  KVM: Documentation: Add info regarding KVM_ARM_VCPU_PSCI_0_2 feature
  ARM/ARM64: KVM: Allow KVM_ARM_VCPU_PSCI_0_2 feature for user space

 Documentation/virtual/kvm/api.txt |    2 +
 arch/arm/include/asm/kvm_host.h   |    2 +-
 arch/arm/include/asm/kvm_psci.h   |    4 ++
 arch/arm/include/uapi/asm/kvm.h   |   35 ++++++++++++++-
 arch/arm/kvm/arm.c                |    1 +
 arch/arm/kvm/psci.c               |   85 +++++++++++++++++++++++++++++++------
 arch/arm64/include/asm/kvm_host.h |    2 +-
 arch/arm64/include/asm/kvm_psci.h |    4 ++
 arch/arm64/include/uapi/asm/kvm.h |   35 ++++++++++++++-
 include/uapi/linux/kvm.h          |    1 +
 10 files changed, 155 insertions(+), 16 deletions(-)

-- 
1.7.9.5

^ permalink raw reply

* [PATCH] ARM64: KVM: Fix VGIC compile error for Linux-3.14-rc1
From: Anup Patel @ 2014-02-04  9:47 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <52F0B5BB.3060901@arm.com>

On 4 February 2014 15:11, Marc Zyngier <marc.zyngier@arm.com> wrote:
> On 04/02/14 09:37, Anup Patel wrote:
>> This patch fixes VGIC compilation for Linux-3.14-rc1 ARM64 kernel.
>>
>> Signed-off-by: Anup Patel <anup.patel@linaro.org>
>> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
>
> Already posted by Christoffer, and hopefully on its way to mainline.

Thanks,
Anup

>
>         M.
>
>> ---
>>  arch/arm64/include/uapi/asm/kvm.h |    9 +++++++++
>>  virt/kvm/arm/vgic.c               |    1 +
>>  2 files changed, 10 insertions(+)
>>
>> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
>> index 31c2f54..cadc318 100644
>> --- a/arch/arm64/include/uapi/asm/kvm.h
>> +++ b/arch/arm64/include/uapi/asm/kvm.h
>> @@ -149,6 +149,15 @@ struct kvm_arch_memory_slot {
>>  #define KVM_REG_ARM_TIMER_CNT                ARM64_SYS_REG(3, 3, 14, 3, 2)
>>  #define KVM_REG_ARM_TIMER_CVAL               ARM64_SYS_REG(3, 3, 14, 0, 2)
>>
>> +/* Device Control API: ARM VGIC */
>> +#define KVM_DEV_ARM_VGIC_GRP_ADDR    0
>> +#define KVM_DEV_ARM_VGIC_GRP_DIST_REGS       1
>> +#define KVM_DEV_ARM_VGIC_GRP_CPU_REGS        2
>> +#define   KVM_DEV_ARM_VGIC_CPUID_SHIFT       32
>> +#define   KVM_DEV_ARM_VGIC_CPUID_MASK        (0xffULL << KVM_DEV_ARM_VGIC_CPUID_SHIFT)
>> +#define   KVM_DEV_ARM_VGIC_OFFSET_SHIFT      0
>> +#define   KVM_DEV_ARM_VGIC_OFFSET_MASK       (0xffffffffULL << KVM_DEV_ARM_VGIC_OFFSET_SHIFT)
>> +
>>  /* KVM_IRQ_LINE irq field index values */
>>  #define KVM_ARM_IRQ_TYPE_SHIFT               24
>>  #define KVM_ARM_IRQ_TYPE_MASK                0xff
>> diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
>> index be456ce..55b0609 100644
>> --- a/virt/kvm/arm/vgic.c
>> +++ b/virt/kvm/arm/vgic.c
>> @@ -21,6 +21,7 @@
>>  #include <linux/kvm_host.h>
>>  #include <linux/interrupt.h>
>>  #include <linux/io.h>
>> +#include <linux/uaccess.h>
>>  #include <linux/of.h>
>>  #include <linux/of_address.h>
>>  #include <linux/of_irq.h>
>>
>
>
> --
> Jazz is not dead. It just smells funny...

^ permalink raw reply

* [PATCH] arm64: Add architecture support for PCI
From: Arnd Bergmann @ 2014-02-04  9:44 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140203233137.GH2519@obsidianresearch.com>

On Monday 03 February 2014 16:31:37 Jason Gunthorpe wrote:
> Specifying 'use EHCI, AHCI, etc' - which are all PCI based standards
> without clearly specifying exactly how PCI is suppose to work is
> completely bonkers.
> 
> What is needed is a spec that says:
>  1) Here is how you generate config TLPs. A MMIO region that
>     conforms to the already specified x86 ECAM would
>     be perfect
>  2) Here is a dword by dword break down of the entire config space in
>     a SOC. Here is where a on-board AHCI controller must show up in
>     config space. Here is how an external PCI-E port must show
>     up. Etc. Most of this is already specified, but it clearly needs
>     to be layed out explicitly for ARM SOCs to actually follow it.
>  3) Here is how you specify the aperture(s) associated with PCI BAR's
>     and bridge windows in config space. And yes: The CONFIG SPACE
>     BARS MUST WORK.
>  4) Here is how MSI works, these are the values you put in the
>     address/data and here is how you collect the interrupt.
>  5) Here is how Legacy INTx must be mapped into the GIC.
> 
> This is what x86 does, and they have been doing it well for 10
> years. If you want to play in the server game you have to properly
> implement PCI.

I'm pretty sure the authors of the SBSA actually thought that was
what they wrote, by referring to external specifications (pci-3.0,
ehci, ahci, ...).  However, it seems they were either foolish enough
to believe that hardware designers would follow these specs, or
they were intentionally misled and got talked into putting ambiguous
terminology in because there were already hardware designs that
are not exactly in line with the spirit of the SBSA but can be
argued to be conforming to the text for a lax interpretation.

I think EHCI is a much better example than PCI here, because the
spec has exactly one line to say about it, where it spends a whole
chapter on PCI.

Here is how a sane person would read SBSA to create a compliant
implementation:

  I have to use EHCI version 1.1 to provide USB-2.0 support. EHCI
  is a PCI device, so I'll put it behind a PCIe port that complies
  to the PCIe section of the SBSA. Since EHCI by itself only provides
  high-speed USB, and USB-2.0 mandates I provide low-speed and
  full-speed as well, I have to add a USB hub device. It would have
  been easier to just use OHCI for these, but SBSA says I can't.
  Now I want to integrate the EHCI into my SoC and not waste one
  of my precious PCIe root ports, so I have to create another PCI
  domain with its own ECAM compliant config space to put it into.
  Fortunately SBSA lets me add an arbitrary number of PCI domains,
  as long as they are all strictly compliant. To software it will
  look exactly as if it was on an external card, I just have to
  ensure the boot loader correctly sets up the clocks and the phy
  before an SBSA compliant OS gets loaded, all runtime power
  management will get handled through the EHCI-1.1 energy-efficiency
  extensions.

Here is how a crazy person would read the same sentence in the SBSA:

  I have an IP block that implements the EHCI register set, that
  should be good enough. It's not a fast device, so I can put it
  on a non-coherent bus. Since the SoC will be used for networking,
  I'll put the registers into big-endian configuration to make it
  easier for the OS to access. I'm not allowed to have USB-1.1
  according to SBSA, so I can get away without a hub or an extra
  OHCI. I can't support MSI because it's not a PCI device, and
  the GIC is pretty full, so I'll just connect the IRQ line to
  the GPIO controller. In order to do better power management,
  I'll design a fancy PHY that the device driver will manage
  for implementing autosuspend. I should also give the OS
  fine-grained control over the clocks, but it will have to share
  the clock domain with the other devices on the same edge of the
  chip. The EHCI device is not part of PCI, which measn I don't
  have to use the standard SMMU. However, my EHCI implementation
  can only do 32-bit DMA, and I'll have to design my own IOMMU
  to let it access the entire memory range. USB-OTG is a great
  feature and we already paid for having this in our EHCI
  implementation, better make sure it comes up in endpoint mode
  after waking up from power saving.

	Arnd

^ permalink raw reply

* [PATCH v5 13/14] ARM: sun4i: dts: Add ahci / sata support
From: Maxime Ripard @ 2014-02-04  9:44 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <52EF70E2.6070803@redhat.com>

On Mon, Feb 03, 2014 at 11:35:14AM +0100, Hans de Goede wrote:
> Hi,
> 
> On 01/31/2014 02:45 PM, Maxime Ripard wrote:
> >Hi Hans,
> >
> >On Wed, Jan 22, 2014 at 08:04:48PM +0100, Hans de Goede wrote:
> >>From: Oliver Schinagl <oliver@schinagl.nl>
> >>
> >>This patch adds sunxi sata support to A10 boards that have such a connector.
> >>Some boards also feature a regulator via a GPIO and support for this is also
> >>added.
> >>
> >>Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
> >>Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> >>---
> >>  arch/arm/boot/dts/sun4i-a10-a1000.dts      |  4 ++++
> >>  arch/arm/boot/dts/sun4i-a10-cubieboard.dts |  6 +++++
> >>  arch/arm/boot/dts/sun4i-a10.dtsi           |  8 +++++++
> >>  arch/arm/boot/dts/sunxi-ahci-reg.dtsi      | 38 ++++++++++++++++++++++++++++++
> >
> >I'm still half convinced about this at the moment, given the number of
> >platforms we support, we can always change it back if things become too messy.
> 
> I assume that this == sunxi-ahci-reg.dtsi ?  To be sure I understand
> you correctly, you're ok with going this route for now, right ?

Yep.

> How about the same for the usb ohci/ehci controller dts patches ? Currently they
> are still using the put a regulator node in each dts file model, which leads to
> a lot of boilerplate code. So I would like to move to the same model as I'm
> using here for the sata supply.

That would make sense too.

> >>  4 files changed, 56 insertions(+)
> >>  create mode 100644 arch/arm/boot/dts/sunxi-ahci-reg.dtsi
> >>
> >>diff --git a/arch/arm/boot/dts/sun4i-a10-a1000.dts b/arch/arm/boot/dts/sun4i-a10-a1000.dts
> >>index aef8207..3fb7305 100644
> >>--- a/arch/arm/boot/dts/sun4i-a10-a1000.dts
> >>+++ b/arch/arm/boot/dts/sun4i-a10-a1000.dts
> >>@@ -48,6 +48,10 @@
> >>  			status = "okay";
> >>  		};
> >>
> >>+		ahci: sata at 01c18000 {
> >>+			status = "okay";
> >>+		};
> >>+
> >>  		pinctrl at 01c20800 {
> >>  			mmc0_cd_pin_a1000: mmc0_cd_pin at 0 {
> >>  				allwinner,pins = "PH1";
> >>diff --git a/arch/arm/boot/dts/sun4i-a10-cubieboard.dts b/arch/arm/boot/dts/sun4i-a10-cubieboard.dts
> >>index f50fb2b..6ae1110 100644
> >>--- a/arch/arm/boot/dts/sun4i-a10-cubieboard.dts
> >>+++ b/arch/arm/boot/dts/sun4i-a10-cubieboard.dts
> >>@@ -12,6 +12,7 @@
> >>
> >>  /dts-v1/;
> >>  /include/ "sun4i-a10.dtsi"
> >>+/include/ "sunxi-ahci-reg.dtsi"
> >>
> >>  / {
> >>  	model = "Cubietech Cubieboard";
> >>@@ -51,6 +52,11 @@
> >>  			status = "okay";
> >>  		};
> >>
> >>+		ahci: sata at 01c18000 {
> >>+			target-supply = <&reg_ahci_5v>;
> >>+			status = "okay";
> >>+		};
> >>+
> >>  		pinctrl at 01c20800 {
> >>  			mmc0_cd_pin_cubieboard: mmc0_cd_pin at 0 {
> >>  				allwinner,pins = "PH1";
> >>diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
> >>index 4736dd2..198dcda 100644
> >>--- a/arch/arm/boot/dts/sun4i-a10.dtsi
> >>+++ b/arch/arm/boot/dts/sun4i-a10.dtsi
> >>@@ -331,6 +331,14 @@
> >>  			status = "disabled";
> >>  		};
> >>
> >>+		ahci: sata at 01c18000 {
> >>+			compatible = "allwinner,sun4i-a10-ahci";
> >
> >To be consistent with the rest of the sun4i devices compatible, It
> >should be sun4i-ahci.
> >
> >However, since these devices don't use the same compatible pattern as
> >their own machine compatible, and are consisent with the rest of the
> >compatibles for the other SoCs, we can probably make this a go to
> >transition progressively to this pattern.
> 
> Ack, I think it would be good to be consistent and try to use
> sun?i-aXX-foo everywhere. I noticed that we already use that in various
> places, so I thought it would be good to do that for all new dts bindings.

Yes, that's my plan.

> >I'll cook up some patches for the other devices.
> 
> Thanks.

And I sent them on sunday.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140204/a627150d/attachment-0001.sig>

^ permalink raw reply

* [PATCH 2/2] Documentation: devicetree: Add boost-opp binding to list boost mode OPPs
From: Thomas Abraham @ 2014-02-04  9:41 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1391506890-7335-1-git-send-email-thomas.ab@samsung.com>

From: Thomas Abraham <thomas.ab@samsung.com>

Certain CPUs or devices can support optional boost operating modes. Add a new
binding to list OPPs to be additionally made available in boost operating modes.

Cc: Nishanth Menon <nm@ti.com>
Cc: Lukasz Majewski <l.majewski@samsung.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: Kumar Gala <galak@codeaurora.org>
Signed-off-by: Thomas Abraham <thomas.ab@samsung.com>
---
 Documentation/devicetree/bindings/power/opp.txt |    9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/Documentation/devicetree/bindings/power/opp.txt b/Documentation/devicetree/bindings/power/opp.txt
index 74499e5..4df5cca 100644
--- a/Documentation/devicetree/bindings/power/opp.txt
+++ b/Documentation/devicetree/bindings/power/opp.txt
@@ -10,6 +10,10 @@ Properties:
 	freq: clock frequency in kHz
 	vol: voltage in microvolt
 
+Optional Properties:
+- boost-opp: Similar to "operating-points" property but usable only in
+  optional boost operating modes.
+
 Examples:
 
 cpu at 0 {
@@ -22,4 +26,9 @@ cpu at 0 {
 		396000  950000
 		198000  850000
 	>;
+	boost-opp = <
+		/* kHz     uV */
+		1500000 1350000
+		1400000 1285000
+	>;
 };
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH 1/2] PM / OPP: Add support for 'boost' mode OPP
From: Thomas Abraham @ 2014-02-04  9:41 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1391506890-7335-1-git-send-email-thomas.ab@samsung.com>

From: Thomas Abraham <thomas.ab@samsung.com>

Commit 6f19efc0 ("cpufreq: Add boost frequency support in core") adds
support for CPU boost mode. This patch adds support for finding available
boost OPPs from device tree and marking them as usable in boost mode.

Cc: Nishanth Menon <nm@ti.com>
Cc: Lukasz Majewski <l.majewski@samsung.com>
Signed-off-by: Thomas Abraham <thomas.ab@samsung.com>
---
 drivers/base/power/opp.c |   69 +++++++++++++++++++++++++++++++++++++---------
 1 file changed, 56 insertions(+), 13 deletions(-)

diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
index 2553867..de4d52d 100644
--- a/drivers/base/power/opp.c
+++ b/drivers/base/power/opp.c
@@ -62,6 +62,7 @@ struct dev_pm_opp {
 	struct list_head node;
 
 	bool available;
+	bool boost;
 	unsigned long rate;
 	unsigned long u_volt;
 
@@ -380,10 +381,12 @@ struct dev_pm_opp *dev_pm_opp_find_freq_floor(struct device *dev,
 EXPORT_SYMBOL_GPL(dev_pm_opp_find_freq_floor);
 
 /**
- * dev_pm_opp_add()  - Add an OPP table from a table definitions
+ * dev_pm_opp_add_flags()  - Add an OPP to device OPP list with flags
  * @dev:	device for which we do this operation
  * @freq:	Frequency in Hz for this OPP
  * @u_volt:	Voltage in uVolts for this OPP
+ * @available:	initial availability of the OPP with adding it to the list.
+ * @boost:	availability of this opp in controller's boost operating mode.
  *
  * This function adds an opp definition to the opp list and returns status.
  * The opp is made available by default and it can be controlled using
@@ -395,7 +398,8 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_find_freq_floor);
  * that this function is *NOT* called under RCU protection or in contexts where
  * mutex cannot be locked.
  */
-int dev_pm_opp_add(struct device *dev, unsigned long freq, unsigned long u_volt)
+static int dev_pm_opp_add_flags(struct device *dev, unsigned long freq,
+			unsigned long u_volt, bool available, bool boost)
 {
 	struct device_opp *dev_opp = NULL;
 	struct dev_pm_opp *opp, *new_opp;
@@ -441,7 +445,8 @@ int dev_pm_opp_add(struct device *dev, unsigned long freq, unsigned long u_volt)
 	new_opp->dev_opp = dev_opp;
 	new_opp->rate = freq;
 	new_opp->u_volt = u_volt;
-	new_opp->available = true;
+	new_opp->available = available;
+	new_opp->boost = boost;
 
 	/* Insert new OPP in order of increasing frequency */
 	head = &dev_opp->opp_list;
@@ -462,6 +467,27 @@ int dev_pm_opp_add(struct device *dev, unsigned long freq, unsigned long u_volt)
 	srcu_notifier_call_chain(&dev_opp->head, OPP_EVENT_ADD, new_opp);
 	return 0;
 }
+
+/**
+ * dev_pm_opp_add()  - Add an OPP table from a table definitions
+ * @dev:	device for which we do this operation
+ * @freq:	Frequency in Hz for this OPP
+ * @u_volt:	Voltage in uVolts for this OPP
+ *
+ * This function adds an opp definition to the opp list and returns status.
+ * The opp is made available by default and it can be controlled using
+ * dev_pm_opp_enable/disable functions.
+ *
+ * Locking: The internal device_opp and opp structures are RCU protected.
+ * Hence this function internally uses RCU updater strategy with mutex locks
+ * to keep the integrity of the internal data structures. Callers should ensure
+ * that this function is *NOT* called under RCU protection or in contexts where
+ * mutex cannot be locked.
+ */
+int dev_pm_opp_add(struct device *dev, unsigned long freq, unsigned long u_volt)
+{
+	return dev_pm_opp_add_flags(dev, freq, u_volt, true, false);
+}
 EXPORT_SYMBOL_GPL(dev_pm_opp_add);
 
 /**
@@ -651,7 +677,8 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
 
 	list_for_each_entry(opp, &dev_opp->opp_list, node) {
 		if (opp->available) {
-			freq_table[i].driver_data = i;
+			freq_table[i].driver_data =
+				opp->boost ? CPUFREQ_BOOST_FREQ : i;
 			freq_table[i].frequency = opp->rate / 1000;
 			i++;
 		}
@@ -701,19 +728,14 @@ struct srcu_notifier_head *dev_pm_opp_get_notifier(struct device *dev)
 }
 
 #ifdef CONFIG_OF
-/**
- * of_init_opp_table() - Initialize opp table from device tree
- * @dev:	device pointer used to lookup device OPPs.
- *
- * Register the initial OPP table with the OPP library for given device.
- */
-int of_init_opp_table(struct device *dev)
+static int of_parse_opp_table(struct device *dev, const char *prop_name,
+					bool boost)
 {
 	const struct property *prop;
 	const __be32 *val;
 	int nr;
 
-	prop = of_find_property(dev->of_node, "operating-points", NULL);
+	prop = of_find_property(dev->of_node, prop_name, NULL);
 	if (!prop)
 		return -ENODEV;
 	if (!prop->value)
@@ -734,7 +756,7 @@ int of_init_opp_table(struct device *dev)
 		unsigned long freq = be32_to_cpup(val++) * 1000;
 		unsigned long volt = be32_to_cpup(val++);
 
-		if (dev_pm_opp_add(dev, freq, volt)) {
+		if (dev_pm_opp_add_flags(dev, freq, volt, true, boost)) {
 			dev_warn(dev, "%s: Failed to add OPP %ld\n",
 				 __func__, freq);
 			continue;
@@ -744,5 +766,26 @@ int of_init_opp_table(struct device *dev)
 
 	return 0;
 }
+
+/**
+ * of_init_opp_table() - Initialize opp table from device tree
+ * @dev:	device pointer used to lookup device OPPs.
+ *
+ * Register the initial OPP table with the OPP library for given device.
+ * Additional "boost" operating points of the controller, if any, are
+ * specified with "boost-opp" property.
+ */
+int of_init_opp_table(struct device *dev)
+{
+	int ret;
+
+	ret = of_parse_opp_table(dev, "operating-points", false);
+	if (!ret) {
+		ret = of_parse_opp_table(dev, "boost-opp", true);
+		if (ret == -ENODEV)
+			ret = 0;
+	}
+	return ret;
+}
 EXPORT_SYMBOL_GPL(of_init_opp_table);
 #endif
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH 0/2] Add device tree based lookup of boost mode OPPs
From: Thomas Abraham @ 2014-02-04  9:41 UTC (permalink / raw)
  To: linux-arm-kernel

Commit 6f19efc0 ("cpufreq: Add boost frequency support in core") adds
support for CPU boost mode for CPUfreq drivers. To use the new boost
mode, CPUfreq drivers have to specify the boost mode frequency and
voltage within the CPUfreq driver, which is the case for Exynos4x12
CPUfreq driver.

But for CPUfreq drivers which obtain the OPPs from cpus node, this
patch series adds support to specify boost mode OPPs in dt node. This
requirement came up when Lukasz pointed out the regression caused by
the Exynos CPUfreq driver consolidation patches.

Thomas Abraham (2):
  PM / OPP: Add support for 'boost' mode OPP
  Documentation: devicetree: Add boost-opp binding to list boost mode OPPs

 Documentation/devicetree/bindings/power/opp.txt |    9 +++
 drivers/base/power/opp.c                        |   69 ++++++++++++++++++-----
 2 files changed, 65 insertions(+), 13 deletions(-)

-- 
1.7.10.4

^ permalink raw reply

* [PATCH] ARM64: KVM: Fix VGIC compile error for Linux-3.14-rc1
From: Marc Zyngier @ 2014-02-04  9:41 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1391506640-1901-1-git-send-email-anup.patel@linaro.org>

On 04/02/14 09:37, Anup Patel wrote:
> This patch fixes VGIC compilation for Linux-3.14-rc1 ARM64 kernel.
> 
> Signed-off-by: Anup Patel <anup.patel@linaro.org>
> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>

Already posted by Christoffer, and hopefully on its way to mainline.

	M.

> ---
>  arch/arm64/include/uapi/asm/kvm.h |    9 +++++++++
>  virt/kvm/arm/vgic.c               |    1 +
>  2 files changed, 10 insertions(+)
> 
> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
> index 31c2f54..cadc318 100644
> --- a/arch/arm64/include/uapi/asm/kvm.h
> +++ b/arch/arm64/include/uapi/asm/kvm.h
> @@ -149,6 +149,15 @@ struct kvm_arch_memory_slot {
>  #define KVM_REG_ARM_TIMER_CNT		ARM64_SYS_REG(3, 3, 14, 3, 2)
>  #define KVM_REG_ARM_TIMER_CVAL		ARM64_SYS_REG(3, 3, 14, 0, 2)
>  
> +/* Device Control API: ARM VGIC */
> +#define KVM_DEV_ARM_VGIC_GRP_ADDR	0
> +#define KVM_DEV_ARM_VGIC_GRP_DIST_REGS	1
> +#define KVM_DEV_ARM_VGIC_GRP_CPU_REGS	2
> +#define   KVM_DEV_ARM_VGIC_CPUID_SHIFT	32
> +#define   KVM_DEV_ARM_VGIC_CPUID_MASK	(0xffULL << KVM_DEV_ARM_VGIC_CPUID_SHIFT)
> +#define   KVM_DEV_ARM_VGIC_OFFSET_SHIFT	0
> +#define   KVM_DEV_ARM_VGIC_OFFSET_MASK	(0xffffffffULL << KVM_DEV_ARM_VGIC_OFFSET_SHIFT)
> +
>  /* KVM_IRQ_LINE irq field index values */
>  #define KVM_ARM_IRQ_TYPE_SHIFT		24
>  #define KVM_ARM_IRQ_TYPE_MASK		0xff
> diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
> index be456ce..55b0609 100644
> --- a/virt/kvm/arm/vgic.c
> +++ b/virt/kvm/arm/vgic.c
> @@ -21,6 +21,7 @@
>  #include <linux/kvm_host.h>
>  #include <linux/interrupt.h>
>  #include <linux/io.h>
> +#include <linux/uaccess.h>
>  #include <linux/of.h>
>  #include <linux/of_address.h>
>  #include <linux/of_irq.h>
> 


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

^ permalink raw reply

* [PATCH v2 2/5] clk: sunxi: Add USB clock register defintions
From: Maxime Ripard @ 2014-02-04  9:40 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <52E77FCD.5050701@redhat.com>

Hi Hans,

On Tue, Jan 28, 2014 at 11:00:45AM +0100, Hans de Goede wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Hi,
> 
> On 01/28/2014 10:44 AM, Maxime Ripard wrote:
> > On Mon, Jan 27, 2014 at 03:54:14PM +0100, Hans de Goede wrote:
> >>>> "allwinner,sun5i-a13-usb-gates-clk" - for usb gates + resets on A13
> >>> 
> >>> Maybe we can just remove the gates from there? Even though they
> >>> are gates, they are also (a bit) more than that.
> >> 
> >> To be clear you mean s/usb-gates-clk/usb-clk/ right ?
> > 
> > Yep, exactly
> > 
> >>> I guess that means that we will have the OHCI0 gate declared
> >>> with <&...-gates-clk 6>, while it's actually the first gate for
> >>> this clock?
> >> 
> >> Correct.
> >> 
> >>> Maybe introducing an offset field in the gates_data would be a
> >>> good idea, so that we always start from indexing the gates from
> >>> 0 in the DT?
> >> 
> >> Well for the other "gates" type clks we also have holes in the
> >> range, and we always refer to the clk with the bit number in the
> >> reg as the clock-cell value.
> > 
> > Yes, we have holes, but I see two majors differences here: - the
> > other gates are just gates, while the usb clocks are a bit more
> > than that.
> 
> The usb-clk registers contain more then that, but the bits we are
> talking about now are gates.
> 
> > - the other gates' gating bits thus all start at bit 0, while
> > - here, since it's kind of a "mixed" clock, the gating bits start
> > - at bit 6 (on the A20 at least)
> 
> Right, still I believe that the consistent thing to do is keeping
> the bit-number for the bit in the register controlling the gate as
> the specifier.  When adding new dts entries / reviewing existing
> ones I'm used to matching the specifier to the bit-nr in the
> data-sheet, I think making things different just for this one
> register is counter productive.

And if you turn it the other way around, it would be inconsistent that
all gates indices start at 0, and we would start at 6 here :)

Plus, this clock is already a special case, since it's the only gate
that is more than just a gate so far.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140204/a602e9cd/attachment.sig>

^ 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