* [PATCH 2/2] Add support for the Nexbox A1 board based on the Amlogic S912 SoC.
From: Kevin Hilman @ 2016-11-28 20:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161123225205.re77xff5vcccaltl@rob-hp-laptop>
Rob Herring <robh@kernel.org> writes:
> On Mon, Nov 21, 2016 at 05:29:05PM +0100, Neil Armstrong wrote:
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>> Documentation/devicetree/bindings/arm/amlogic.txt | 1 +
>> arch/arm64/boot/dts/amlogic/Makefile | 1 +
>> .../arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts | 169 +++++++++++++++++++++
>> 3 files changed, 171 insertions(+)
>> create mode 100644 arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts
>
> A few nits below, otherwise:
>
> Acked-by: Rob Herring <robh@kernel.org>
>
Fixed up the changes locally, and applied for v4.10.
Kevin
^ permalink raw reply
* [PATCH V8 2/6] thermal: bcm2835: add thermal driver for bcm2835 soc
From: Eric Anholt @ 2016-11-28 20:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161125052008.GA8342@localhost.localdomain>
Eduardo Valentin <edubezval@gmail.com> writes:
> Hello,
>
> On Tue, Nov 22, 2016 at 03:28:04PM +0100, Martin Sperl wrote:
>> Hi Eduardo!
>>
>> On 19.11.2016 05:22, Eduardo Valentin wrote:
>> > Hello Martin,
>
> <cut>
>
>>
>> I was asked to implement the "initialize" case just in case FW ever
>> stopped setting up the device itself, so that is why this code is
>> included.
>
> OK. Looks like we (like, we in the Linux side) do not understand the
> conditions that firmware fails to initialize the thermal device, but we
> still want to force an initialization, hoping to get the device in a sane
> state, even if we do not know if the firmware is correctly booted or
> not. And that is done silently, with no notification to user. I see.
The firmware today always initializes thermal. I suggested adding the
init code because we (myself and the Pi Foundation) would like to reduce
how much closed firmware code is required in the platform, and the Linux
driver doing this would help make that possible in the future.
>> > Who has the ownership of this device?
>>
>> Joined ownership I suppose...
>>
>
> with no synchronization mechanism?
Correct, because none is necessary.
>> >> The above mentioned ?configuration if not running? reflect the values that
>> >> the FW is currently setting. We should not change those values as long as the
>> >> Firmware is also reading the temperature on its own.
>> >
>> > hmm.. that looks like racy to me. Again, How do you synchronize accesses to
>> > this device? What if you configure the device and right after the
>> > firmware updates the configs? How do you make sure the configs you are
>> > writing here are the same used by the firmware? What if the firmware
>> > version changes? What versions of the firmware does this driver support?
>> >
>> > Would it make sense to simply always initialize the device? Do you have
>> > a way to tell the firmware that it should not use the device?
>> >
>> > Or, if you want to keep the device driver simply being a dummy reader,
>> > would it make sense to simply avoid writing configurations to the
>> > device, and simply retry to check if the firmware gets the device
>> > initialized?
>>
>> Again: the device registers are only ever written if the device is not started
>> already. Otherwise the driver only reads for the ADC register, so there
>> is no real race here.
>>
>
> and no race?
>
> To me, there is a race when you write to the config of this device,
> given that there is no sync between the two. We do not know if the
> firmware would be still attempting to initialize the device or not, do
> we?
Either the device was initialized by the firmware before handing off to
ARM (today's firmware) or it never will be (potential future firmware).
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161128/af046805/attachment.sig>
^ permalink raw reply
* Adding a .platform_init callback to sdhci_arasan_ops
From: Doug Anderson @ 2016-11-28 20:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <9c783843-d0e4-92ef-d156-6826e38d9fa8@free.fr>
Hi,
On Mon, Nov 28, 2016 at 11:32 AM, Mason <slash.tmp@free.fr> wrote:
> On 28/11/2016 18:46, Doug Anderson wrote:
>
>> As argued in my original patch the field "corecfg_baseclkfreq" is
>> documented in the generic Arasan document
>> <https://arasan.com/wp-content/media/eMMC-5-1-Total-Solution_Rev-1-3.pdf>
>> and thus is unlikely to be Rockchip specific.
>
> I downloaded the data sheet, which doesn't mention registers,
> but "pins" and "signals". Does that mean it is up to every
> platform to decide how to group these wires into individual
> registers?
As I understand it: yes. ...but remember that I'm not arasan nor even
Rockchip and I don't have any access to that they provide their IP
licensees.
That's why I structured my config the way it did. It means that no
matter how a licensee stuffs these configs into their register map we
can support it. Every configuration is individually described by a
register offset, width, and mask. Technically the width _ought_ to be
the same for all users, so I guess you could try to optimize that out.
...but it doesn't hurt the way it is.
> corecfg_baseclkfreq[7:0]
> Base Clock Frequency for SD Clock.
> This is the frequency of the xin_clk.
>
> How can 8 bits encode a frequency?
> Is there an implicit LUT? Is it a MHz count?
Use the source. :)
* The corecfg_baseclkfreq is supposed to contain the MHz of clk_xin.
It also turns out to match the SDHCI spec "Base Clock Frequency for SD
Clock". See below.
---
It actually turns out that both of the "corecfg" bits we're setting
right now for Rockchip are a bit on the silly side. All they do is
get mirrored out the other end in the "caps" register. I found that
out from folks at Rockchip much later after I wrote my patch.
So, for instance, setting corecfg_clockmultiplier (I'm told) does
nothing more than control the bits read here:
(caps[1] & SDHCI_CLOCK_MUL_MASK) >> SDHCI_CLOCK_MUL_SHIFT;
I can certainly see that they are mirrored, but I have to just take it
on faith that it does nothing else.
>>> w(grf + 0xf02c, 0x00ff0011)
>>> hex((r(sdhci + 0x44) & 0x00ff0000) >> 16)
'0x11L'
...and similar for the speed:
>>> w(grf + 0xf000, 0xff009700)
>>> hex((r(sdhci + 0x40) & 0x0000ff00) >> 8)
'0x97L'
-Doug
^ permalink raw reply
* [PATCH 5/7] add bindings for stm32 IIO timer drivers
From: Rob Herring @ 2016-11-28 21:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CA+M3ks7MgnUJoDoNG157+MbdhgbqwzRmhPcD-uQKC7_DJVAK6A@mail.gmail.com>
On Wed, Nov 23, 2016 at 09:17:58AM +0100, Benjamin Gaignard wrote:
> If it is ok for you I will add "id" parameter in mfd driver and
> forward it to the sub-devices drivers
> to be able to distinguish the hardware blocks
Please don't top post.
No, it's not okay. If the counter sizes are different, then have a
property for the counter size. Describe how they are different without
numbering them. If you can't describe the differences, then it shouldn't
matter which ones the OS picks to use.
> 2016-11-22 18:18 GMT+01:00 Lee Jones <lee.jones@linaro.org>:
> > On Tue, 22 Nov 2016, Benjamin Gaignard wrote:
> >
> >> [snip]
> >> >> + "st,stm32-iio-timer5"
> >> >> + "st,stm32-iio-timer6"
> >> >> + "st,stm32-iio-timer7"
> >> >> + "st,stm32-iio-timer8"
> >> >> + "st,stm32-iio-timer9"
> >> >> + "st,stm32-iio-timer10"
> >> >> + "st,stm32-iio-timer11"
> >> >> + "st,stm32-iio-timer12"
> >> >> + "st,stm32-iio-timer13"
> >> >> + "st,stm32-iio-timer14"
I doubt the h/w manual calls these "IIO timers".
> >> >
> >> > We can't do this. This is a binding for a driver, not for the hardware.
> >> >
> >>
> >> Unfortunately each instance for the hardware IP have little
> >> differences like which triggers they could accept or size of the
> >> counter register,
> >> and I doesn't have value inside the hardware to distinguish them so
> >> the only way I found is to use compatible.
> >
> > Can't you represent these as properties?
> >
> > --
> > Lee Jones
> > Linaro STMicroelectronics Landing Team Lead
> > Linaro.org ? Open source software for ARM SoCs
> > Follow Linaro: Facebook | Twitter | Blog
>
>
>
> --
> Benjamin Gaignard
>
> Graphic Study Group
>
> Linaro.org ? Open source software for ARM SoCs
>
> Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* [PATCH 1/3] devicetree: bindings: pinctrl: Add binding for ti,da850-pupd
From: Rob Herring @ 2016-11-28 21:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479871767-20160-2-git-send-email-david@lechnology.com>
On Tue, Nov 22, 2016 at 09:29:25PM -0600, David Lechner wrote:
> Device-tree bindings for TI DA8XX/OMAP-L138/AM18XX pullup/pulldown
> pinconf controller.
>
> Signed-off-by: David Lechner <david@lechnology.com>
> ---
> .../devicetree/bindings/pinctrl/ti,da850-pupd.txt | 55 ++++++++++++++++++++++
> 1 file changed, 55 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/pinctrl/ti,da850-pupd.txt
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* [PATCH V7 2/3] ACPI: Add support for ResourceSource/IRQ domain mapping
From: Agustin Vega-Frias @ 2016-11-28 22:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161125114035.GA13739@red-moon>
Hi Rafael,
Can you chime in on Lorenzo's feedback and the discussion below?
It would be great if you can comment on the reason ACPI does things
in a certain way.
Hi Lorenzo,
On 2016-11-25 06:40, Lorenzo Pieralisi wrote:
> Hi Agustin,
>
> On Thu, Nov 24, 2016 at 04:15:48PM +0000, Lorenzo Pieralisi wrote:
>
> [...]
>
>> > @@ -448,6 +449,7 @@ bool acpi_dev_resource_interrupt(struct acpi_resource *ares, int index,
>> > {
>> > struct acpi_resource_irq *irq;
>> > struct acpi_resource_extended_irq *ext_irq;
>> > + struct fwnode_handle *src;
>> >
>> > switch (ares->type) {
>> > case ACPI_RESOURCE_TYPE_IRQ:
>> > @@ -460,7 +462,7 @@ bool acpi_dev_resource_interrupt(struct acpi_resource *ares, int index,
>> > acpi_dev_irqresource_disabled(res, 0);
>> > return false;
>> > }
>> > - acpi_dev_get_irqresource(res, irq->interrupts[index],
>> > + acpi_dev_get_irqresource(res, irq->interrupts[index], NULL,
>> > irq->triggering, irq->polarity,
>> > irq->sharable, true);
>> > break;
>> > @@ -470,7 +472,8 @@ bool acpi_dev_resource_interrupt(struct acpi_resource *ares, int index,
>> > acpi_dev_irqresource_disabled(res, 0);
>> > return false;
>> > }
>> > - acpi_dev_get_irqresource(res, ext_irq->interrupts[index],
>> > + src = acpi_get_irq_source_fwhandle(&ext_irq->resource_source);
>>
>> Is there a reason why we need to do the domain look-up here ?
Because we need to pass the resource down to acpi_dev_get_irqresource
which does the mapping through acpi_register_irq/acpi_register_gsi.
>>
>> I would like to understand if, by reshuffling the code (and by
>> returning
>> the resource_source to the calling code - somehow), it would be
>> possible
>> to just mirror what the OF code does in of_irq_get(), namely:
>>
>> (1) parse the irq entry -> of_irq_parse_one()
>> (2) look the domain up -> irq_find_host()
>> (3) create the mapping -> irq_create_of_mapping()
>>
>> You wrote the code already, I think it is just a matter of shuffling
>> it around (well, minus returning the resource_source to the caller
>> which is phandle equivalent in DT).
This is one area in which DT and ACPI are fundamentally different. In DT
once the flattened blob is expanded the data is fixed. In ACPI the data
returned by a method can change. In reality most methods like CRS return
constants, but given that per-spec they are methods the interpreter has
to be involved, which makes it an expensive operation. I believe that is
the reason the resource parsing code in ACPI attempts all mappings
during
the bus scan. Rafael can you comment on this?
One way to do what you suggest would be to defer IRQ mapping by, e.g.,
populating res->start with the HW IRQ number and res->end with the
fwnode.
That way we can avoid having to walk the resource buffer when a mapping
is needed. I don't think that approach would deviate much more from
the spec from what the current ahead-of-time mapping does, but it would
require more changes in the core code. An alternative would be to do
that only for resources that fail to map.
>>
>> You abstracted away (2) and (3) behind acpi_register_irq(), that
>> on anything than does not use ACPI_GENERIC_GSI is just glue code
>> to acpi_register_gsi().
>>
>> Also, it is not a question on this patch but I ask it here because it
>> is related. On ACPI you are doing the reverse of what is done in
>> DT in platform_get_irq():
>>
>> - get the resources already parsed -> platform_get_resource()
>> - if they are disabled -> acpi_irq_get()
>>
>> and I think the ordering is tied to my question above because
>> you carry out the domain look up in acpi_dev_resource_interrupt()
>> so that if for any reason it fails the corresponding resource
>> is disabled so that we try to get it again through acpi_irq_get().
>>
>> I suspect you did it this way to make sure:
>>
>> a) keep the current ACPI IRQ parsing interface changes to a mininum
>> b) avoid changing the behaviour on x86/ia64; in particular, calling
>> acpi_register_gsi() for the _same_ mapping (an IRQ that was already
>> registered at device creation resource parsing) multiple times can
>> trigger issues on x86/ia64
You are correct about my reasons. I wanted to keep ACPI core code
changes
to a minimum, and I also needed to work within the current
implementation
which uses the pre-converted IRQ resources.
>>
>> I think that's a reasonable approach but I wanted to get these
>> clarifications, I do not think you are far from getting this
>> done but since it is a significant change I think it is worth
>> discussing the points I raised above because I think the DT code
>> sequence in of_irq_get() (1-2-3 above) is cleaner from an IRQ
>> layer perspective (instead of having the domain look-up buried
>> inside the ACPI IRQ resource parsing API).
>
> I had another look and to achieve the above one way of doing that is to
> implement acpi_irq_get() only for ACPI_GENERIC_GSI and stub it out for
> !ACPI_GENERIC_GSI (ie return an error code so that on !ACPI_GENERIC_GSI
> we would fall back to current solution for ACPI). Within acpi_irq_get()
> you can easily carry out the same steps (1->2->3) above in ACPI you
> have
> the code already there I think it is easy to change the
> acpi_irq_get_cb() interface to return a filled in struct irq_fwspec and
> the interface would become identical to of_irq_get() that is an
> advantage to maintain it from an IRQ maintainership perspective I
> think,
> that's my opinion.
I think I get what you mean. I'll take a stab at implementing
acpi_irq_get()
in the way you suggest.
>
> There is still a nagging snag though. When platform devices are
> created, core ACPI code parse the resources through:
>
> acpi_dev_get_resources()
>
> and we _have_ to have way to avoid initializing IRQ resources that
> have a dependency (ie there is a resource_source pointer that is valid
> in their descriptors) that's easy to do if we think that's the right
> thing to do and can hardly break current code (which ignores the
> resource_source altogether).
I'd rather keep the core code as-is with regard to the ahead-of-time
conversion. Whether a resource source is available at the time of the
bus
scan should be transparent to the code in drivers/acpi/resource.c, and
we need the initialization as a disabled resource to signal the need
to retry anyway.
Rafael, do you have any other suggestions/feedback on how to go about
doing this?
Thanks,
Agustin
>
> It is an important difference with DT probing, where the IRQ
> resources are only created if the domain reference (ie interrupt
> controller phandle) is satisfied at of_device_alloc() time
> (see of_device_alloc()).
>
> Thoughts ? Please let me know, the code to implement what I say
> is already in these patches, it is just a matter of reshuffling it.
>
> Thanks !
> Lorenzo
--
Qualcomm Datacenter Technologies, Inc. on behalf of the Qualcomm
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a
Linux Foundation Collaborative Project.
^ permalink raw reply
* [PATCH] arm64: dts: Add symlinks for cros-ec-keyboard and cros-ec-sbs
From: Brian Norris @ 2016-11-28 23:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1463090530-21864-1-git-send-email-dianders@chromium.org>
Hi Olof, Arnd,
On Thu, May 12, 2016 at 03:02:10PM -0700, Doug Anderson wrote:
> We'd like to be able to use the cros-ec-keyboard.dtsi and
> cros-ec-sbs.dtsi snippets for arm64 devices. Currently those files live
> in the arm/boot/dts directory.
>
> Let's follow the convention set by commit 8ee57b8182c4 ("ARM64: dts:
> vexpress: Use a symlink to vexpress-v2m-rs1.dtsi from arch=arm") and use
> a symlink. Note that in this case we put the files in a new
> "include/common" directory since these snippets may need to be
> referenced by dts files in many different subdirectories.
>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
> Note that, as of right now, there are no users of this. However, given
> development happening it is almost 100% certain that users will arrive
> soon. If we need to wait for the first user before landing this we can
> leave this on the back burner. If it's OK to have no users (yet), let's
> land.
This may not fit your definition of "soon", but I'm looking to start
using these files in arch/arm64/. I'll probably carry along this patch
and resend when I'm ready, but it'd be just as well if you'd merge it
now (or express a preference for a different directory structure).
> arch/arm64/boot/dts/include/common/cros-ec-keyboard.dtsi | 1 +
> arch/arm64/boot/dts/include/common/cros-ec-sbs.dtsi | 1 +
> 2 files changed, 2 insertions(+)
> create mode 120000 arch/arm64/boot/dts/include/common/cros-ec-keyboard.dtsi
> create mode 120000 arch/arm64/boot/dts/include/common/cros-ec-sbs.dtsi
>
> \ No newline at end of file
>
> diff --git a/arch/arm64/boot/dts/include/common/cros-ec-keyboard.dtsi b/arch/arm64/boot/dts/include/common/cros-ec-keyboard.dtsi
> new file mode 120000
> index 000000000000..1c1889f0a791
> --- /dev/null
> +++ b/arch/arm64/boot/dts/include/common/cros-ec-keyboard.dtsi
> @@ -0,0 +1 @@
> +../../../../../arm/boot/dts/cros-ec-keyboard.dtsi
> \ No newline at end of file
> diff --git a/arch/arm64/boot/dts/include/common/cros-ec-sbs.dtsi b/arch/arm64/boot/dts/include/common/cros-ec-sbs.dtsi
> new file mode 120000
> index 000000000000..3d7ae9c88bcd
> --- /dev/null
> +++ b/arch/arm64/boot/dts/include/common/cros-ec-sbs.dtsi
> @@ -0,0 +1 @@
> +../../../../../arm/boot/dts/cros-ec-sbs.dtsi
FWIW:
Reviewed-by: Brian Norris <briannorris@chromium.org>
^ permalink raw reply
* [PATCH v5 8/8] iommu/rockchip: Enable Rockchip IOMMU on ARM64
From: Brian Norris @ 2016-11-29 0:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1466734413-7453-9-git-send-email-zhengsq@rock-chips.com>
Hi,
On Fri, Jun 24, 2016 at 10:13:33AM +0800, Shunqian Zheng wrote:
> From: Simon Xue <xxm@rock-chips.com>
>
> This patch makes it possible to compile the rockchip-iommu driver on
> ARM64, so that it can be used with 64-bit SoCs equipped with this type
> of IOMMU.
>
> Signed-off-by: Simon Xue <xxm@rock-chips.com>
> Signed-off-by: Shunqian Zheng <zhengsq@rock-chips.com>
> Signed-off-by: Tomasz Figa <tfiga@chromium.org>
> ---
> drivers/iommu/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> index ad08603..5572621 100644
> --- a/drivers/iommu/Kconfig
> +++ b/drivers/iommu/Kconfig
> @@ -218,7 +218,7 @@ config OMAP_IOMMU_DEBUG
>
> config ROCKCHIP_IOMMU
> bool "Rockchip IOMMU Support"
> - depends on ARM
> + depends on ARM || ARM64
> depends on ARCH_ROCKCHIP || COMPILE_TEST
> select IOMMU_API
> select ARM_DMA_USE_IOMMU
Whatever happened with the rest of this series? Some of the IOMMU bits
made it, but the DRM fixes never did, and so this didn't get applied.
This leaves the whole DRM stack unusable on ARM64 Rockchip systems.
The patch context has changed a bit on patch 7 (and maybe 6?), so
somebody will need to refresh those on the latest upstream.
Brian
^ permalink raw reply
* [bug report v4.8] fs/locks.c: kernel oops during posix lock stress test
From: Ming Lei @ 2016-11-29 1:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480340400.2606.10.camel@poochiereds.net>
Hi Jeff,
On Mon, Nov 28, 2016 at 9:40 PM, Jeff Layton <jlayton@poochiereds.net> wrote:
> On Mon, 2016-11-28 at 11:10 +0800, Ming Lei wrote:
>> Hi Guys,
>>
>> When I run stress-ng via the following steps on one ARM64 dual
>> socket system(Cavium Thunder), the kernel oops[1] can often be
>> triggered after running the stress test for several hours(sometimes
>> it may take longer):
>>
>> - git clone git://kernel.ubuntu.com/cking/stress-ng.git
>> - apply the attachment patch which just makes the posix file
>> lock stress test more aggressive
>> - run the test via '~/git/stress-ng$./stress-ng --lockf 128 --aggressive'
>>
>>
>> From the oops log, looks one garbage file_lock node is got
>> from the linked list of 'ctx->flc_posix' when the issue happens.
>>
>> BTW, the issue isn't observed on single socket Cavium Thunder yet,
>> and the same issue can be seen on Ubuntu Xenial(v4.4 based kernel)
>> too.
>>
>> Thanks,
>> Ming
>>
>
> Some questions just for clarification:
>
> - I assume this is being run on a local fs of some sort? ext4 or xfs or
> something?
Yes, I just tested it on local ext4, and not test it on other filesystems yet.
>
> - have you seen this on any other arch, besides ARM?
I run the same tests on x86 before, and not see the issue.
>
> The file locking code does do some lockless checking to see whether the
> i_flctx is even present and whether the list is empty in
> locks_remove_posix. It's possible we have some barrier problems there,
> but I don't quite see how that would cause us to have a corrupt lock on
> the flc_posix list.
Yeah, I looked at the function of posix_lock_inode(), seems both add and
remove are protected by the lock.
Thanks,
Ming
^ permalink raw reply
* [PATCH v4] clkdev: add devm_of_clk_get()
From: Kuninori Morimoto @ 2016-11-29 1:21 UTC (permalink / raw)
To: linux-arm-kernel
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Current Linux has of_clk_get(), but doesn't have devm_of_clk_get().
This patch adds it. It is implemeted in clk-devres.c to share
devm_clk_release().
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
v3 -> v4
- git log explain why it is implemeted in clk-devres
- it is related to CONFIG_HAVE_CLK
drivers/clk/clk-devres.c | 21 +++++++++++++++++++++
include/linux/clk.h | 27 +++++++++++++++++++++++----
2 files changed, 44 insertions(+), 4 deletions(-)
diff --git a/drivers/clk/clk-devres.c b/drivers/clk/clk-devres.c
index 8f57154..2449b25 100644
--- a/drivers/clk/clk-devres.c
+++ b/drivers/clk/clk-devres.c
@@ -53,3 +53,24 @@ void devm_clk_put(struct device *dev, struct clk *clk)
WARN_ON(ret);
}
EXPORT_SYMBOL(devm_clk_put);
+
+struct clk *devm_of_clk_get(struct device *dev,
+ struct device_node *np, int index)
+{
+ struct clk **ptr, *clk;
+
+ ptr = devres_alloc(devm_clk_release, sizeof(*ptr), GFP_KERNEL);
+ if (!ptr)
+ return ERR_PTR(-ENOMEM);
+
+ clk = of_clk_get(np, index);
+ if (!IS_ERR(clk)) {
+ *ptr = clk;
+ devres_add(dev, ptr);
+ } else {
+ devres_free(ptr);
+ }
+
+ return clk;
+}
+EXPORT_SYMBOL(devm_of_clk_get);
diff --git a/include/linux/clk.h b/include/linux/clk.h
index 123c027..7f50c5f 100644
--- a/include/linux/clk.h
+++ b/include/linux/clk.h
@@ -17,8 +17,9 @@
#include <linux/notifier.h>
struct device;
-
struct clk;
+struct device_node;
+struct of_phandle_args;
/**
* DOC: clk notifier callback types
@@ -249,6 +250,21 @@ static inline void clk_unprepare(struct clk *clk)
struct clk *devm_clk_get(struct device *dev, const char *id);
/**
+ * devm_clk_get - lookup and obtain a managed reference to a clock producer.
+ * @dev: device for clock "consumer"
+ * @np: pointer to clock consumer node
+ * @index: clock index
+ *
+ * This function parses the clocks, and uses them to look up the
+ * struct clk from the registered list of clock providers by using
+ * @np and @index.
+ *
+ * The clock will automatically be freed when the device is unbound
+ * from the bus.
+ */
+struct clk *devm_of_clk_get(struct device *dev, struct device_node *np, int index);
+
+/**
* clk_enable - inform the system when the clock source should be running.
* @clk: clock source
*
@@ -432,6 +448,12 @@ static inline struct clk *devm_clk_get(struct device *dev, const char *id)
return NULL;
}
+static inline struct clk *devm_of_clk_get(struct device *dev,
+ struct device_node *np, int index)
+{
+ return NULL;
+}
+
static inline void clk_put(struct clk *clk) {}
static inline void devm_clk_put(struct device *dev, struct clk *clk) {}
@@ -501,9 +523,6 @@ static inline void clk_disable_unprepare(struct clk *clk)
clk_unprepare(clk);
}
-struct device_node;
-struct of_phandle_args;
-
#if defined(CONFIG_OF) && defined(CONFIG_COMMON_CLK)
struct clk *of_clk_get(struct device_node *np, int index);
struct clk *of_clk_get_by_name(struct device_node *np, const char *name);
--
1.9.1
^ permalink raw reply related
* [PATCH V8 2/6] thermal: bcm2835: add thermal driver for bcm2835 soc
From: Eduardo Valentin @ 2016-11-29 1:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <87twarz6s1.fsf@eliezer.anholt.net>
Hello Eric, Martin,
On Mon, Nov 28, 2016 at 12:30:38PM -0800, Eric Anholt wrote:
> Eduardo Valentin <edubezval@gmail.com> writes:
>
<cut>
> The firmware today always initializes thermal. I suggested adding the
> init code because we (myself and the Pi Foundation) would like to reduce
> how much closed firmware code is required in the platform, and the Linux
> driver doing this would help make that possible in the future.
Oh I see. Backup code for future chips/firmware.
>
> >> > Who has the ownership of this device?
> >>
> >> Joined ownership I suppose...
> >>
> >
> > with no synchronization mechanism?
>
> Correct, because none is necessary.
>
<cut>
>
> Either the device was initialized by the firmware before handing off to
> ARM (today's firmware) or it never will be (potential future firmware).
And do you have a way to check if the firmware has the initialization
code or not? By firmware version, for example. Or even, chip version,
maybe?
If the current firmware will always initialize the chip, I would say,
ARM should simply read the registers, no initialization, unless it is
known that the firmware intentionally left the device uninitialized.
Again, just trying to avoid obscure misbehavior, when running into a
faulty state, and running silently broken.
^ permalink raw reply
* [PATCH] arm64: head.S: Fix CNTHCTL_EL2 access on VHE system
From: Jintack Lim @ 2016-11-29 2:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <871868f8-563c-98a1-e3f7-f3ac9457265e@arm.com>
On Mon, Nov 28, 2016 at 11:56 AM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> On 28/11/16 16:43, Jintack Lim wrote:
>> From: Jintack <jintack@cs.columbia.edu>
>>
>> Bit positions of CNTHCTL_EL2 are changing depending on HCR_EL2.E2H bit.
>> EL1PCEN and EL1PCTEN are 1st and 0th bits when E2H is not set, but they
>> are 11th and 10th bits respectively when E2H is set. Current code is
>> unintentionally setting wrong bits to CNTHCTL_EL2 with E2H set.
>>
>> In fact, we don't need to set those two bits, which allow EL1 and EL0 to
>> access physical timer and counter respectively, if E2H and TGE are set
>> for the host kernel. They will be configured later as necessary. First,
>> we don't need to configure those bits for EL1, since the host kernel
>> runs in EL2. It is a hypervisor's responsibility to configure them
>> before entering a VM, which runs in EL0 and EL1. Second, EL0 accesses
>> are configured in the later stage of boot process.
>>
>> Signed-off-by: Jintack Lim <jintack@cs.columbia.edu>
>> ---
>> arch/arm64/kernel/head.S | 8 +++++++-
>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
>> index 332e331..bc3d2db 100644
>> --- a/arch/arm64/kernel/head.S
>> +++ b/arch/arm64/kernel/head.S
>> @@ -524,10 +524,16 @@ set_hcr:
>> msr hcr_el2, x0
>> isb
>>
>> - /* Generic timers. */
>> + /*
>> + * Allow Non-secure EL1 and EL0 to access physical timer and counter.
>> + * This is not necessary for VHE, since the host kernel runs in EL2,
>> + * and EL0 accesses are configured in the later stage of boot process.
>> + */
>> + cbnz x2, 1f
>> mrs x0, cnthctl_el2
>> orr x0, x0, #3 // Enable EL1 physical timers
>> msr cnthctl_el2, x0
>> +1:
>> msr cntvoff_el2, xzr // Clear virtual offset
>>
>> #ifdef CONFIG_ARM_GIC_V3
>>
>
> Nice catch. It may be worth documenting that when HCR_EL2.E2H == 1,
> CNTHCTL_EL2 has the same bit layout as CNTKCTL_EL1, allowing the kernel
> designed to run at EL1 to transparently mess with the EL0 bits (as
> CNTHCTL_EL2 and CNTKCTL_EL1 are the same register in this configuration).
Hi Marc, thanks for the review. I'll add this comment and send out v2.
>
> Acked-by: Marc Zyngier <marc.zyngier@arm.com>
>
> Thanks,
>
> M.
> --
> Jazz is not dead. It just smells funny...
>
^ permalink raw reply
* [PATCH v2] arm64: head.S: Fix CNTHCTL_EL2 access on VHE system
From: Jintack Lim @ 2016-11-29 2:13 UTC (permalink / raw)
To: linux-arm-kernel
From: Jintack <jintack@cs.columbia.edu>
Bit positions of CNTHCTL_EL2 are changing depending on HCR_EL2.E2H bit.
EL1PCEN and EL1PCTEN are 1st and 0th bits when E2H is not set, but they
are 11th and 10th bits respectively when E2H is set. Current code is
unintentionally setting wrong bits to CNTHCTL_EL2 with E2H set.
In fact, we don't need to set those two bits, which allow EL1 and EL0 to
access physical timer and counter respectively, if E2H and TGE are set
for the host kernel. They will be configured later as necessary. First,
we don't need to configure those bits for EL1, since the host kernel
runs in EL2. It is a hypervisor's responsibility to configure them
before entering a VM, which runs in EL0 and EL1. Second, EL0 accesses
are configured in the later stage of boot process.
Signed-off-by: Jintack Lim <jintack@cs.columbia.edu>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
---
v2: Add comments about accessing CNTHCTL_EL2 when HCR_EL2.ECH == 1
---
arch/arm64/kernel/head.S | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index 332e331..136cb0d 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -524,10 +524,21 @@ set_hcr:
msr hcr_el2, x0
isb
- /* Generic timers. */
+ /*
+ * Allow Non-secure EL1 and EL0 to access physical timer and counter.
+ * This is not necessary for VHE, since the host kernel runs in EL2,
+ * and EL0 accesses are configured in the later stage of boot process.
+ * Note that when HCR_EL2.E2H == 1, CNTHCTL_EL2 has the same bit layout
+ * as CNTKCTL_EL1, and CNTKCTL_EL1 accessing instructions are redefined
+ * to access CNTHCTL_EL2. This allows the kernel designed to run at EL1
+ * to transparently mess with the EL0 bits via CNTKCTL_EL1 access in
+ * EL2.
+ */
+ cbnz x2, 1f
mrs x0, cnthctl_el2
orr x0, x0, #3 // Enable EL1 physical timers
msr cnthctl_el2, x0
+1:
msr cntvoff_el2, xzr // Clear virtual offset
#ifdef CONFIG_ARM_GIC_V3
--
1.9.1
^ permalink raw reply related
* [PATCH v2] irqchip/gicv3-its: Enable cacheable attribute Read-allocate hints
From: Shanker Donthineni @ 2016-11-29 2:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478589498-17696-1-git-send-email-shankerd@codeaurora.org>
Hi Marc,
Please comment on this patch. I'm seeing ~1000 cpu cycles interrupt
latency improvement on Qualcomm server platforms if the ITS tables are
not in L1/L2/L3 cache. You can find the test code at
https://patchwork.kernel.org/patch/9416793 which I used to validate my
code changes.
Thanks,
Shanker
On 11/08/2016 01:18 AM, Shanker Donthineni wrote:
> Read-allocation hints are not enabled for both the GIC-ITS and GICR
> tables. This forces the hardware to always read the table contents
> from an external memory (DDR) which is slow compared to cache memory.
> Most of the tables are often read by hardware. So, it's better to
> enable Read-allocate hints in addition to Write-allocate hints in
> order to improve the GICR_PEND, GICR_PROP, Collection, Device, and
> vCPU tables lookup time.
>
> Signed-off-by: Shanker Donthineni <shankerd@codeaurora.org>
> ---
> Implemented a test case to prove that enabling Read Allocation hints
> improves ITS lookup time ~15% while delivering a LPI event. Used the
> ITS command INV to analyze time spent in device, collection, prop and
> pending table lookups.
>
> Pseudo code:
> Create a fake ITS device.
> Record PMU cycle counter before sending INV command.
> Build and send ITS INT command.
> ITS hardware triggers device table lookup.
> ITTE table & collection table lookup.
> ITS property table lookup.
> ITS pending table lookup.
> Deliver interrupt to CPU interface.
> do_IRQ() called.
> Measure the total CPU cycle spent to reach this point.
>
> Without ReadAllocation hints:
> /sys/kernel/debug # echo 100 > lpitest
> [ 94.693968] CPU[1] niter=100 cycles=0x8dfc0 avg=0x16b7 min=0x1652
>
> With ReadAllocation hints:
> /sys/kernel/debug # echo 100 > lpitest
> [ 98.617873] CPU[1] niter=100 cycles=0x7df49 avg=0x1427 min=0x1388
>
> drivers/irqchip/irq-gic-v3-its.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/irqchip/irq-gic-v3-its.c
> b/drivers/irqchip/irq-gic-v3-its.c
> index c5dee30..227a1eb 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -961,7 +961,7 @@ static bool its_parse_baser_device(struct its_node
> *its, struct its_baser *baser
> u32 psz, u32 *order)
> {
> u64 esz = GITS_BASER_ENTRY_SIZE(its_read_baser(its, baser));
> - u64 val = GITS_BASER_InnerShareable | GITS_BASER_WaWb;
> + u64 val = GITS_BASER_InnerShareable | GITS_BASER_RaWaWb;
> u32 ids = its->device_ids;
> u32 new_order = *order;
> bool indirect = false;
> @@ -1026,7 +1026,7 @@ static int its_alloc_tables(struct its_node *its)
> u64 typer = gic_read_typer(its->base + GITS_TYPER);
> u32 ids = GITS_TYPER_DEVBITS(typer);
> u64 shr = GITS_BASER_InnerShareable;
> - u64 cache = GITS_BASER_WaWb;
> + u64 cache = GITS_BASER_RaWaWb;
> u32 psz = SZ_64K;
> int err, i;
>
> @@ -1123,7 +1123,7 @@ static void its_cpu_init_lpis(void)
> /* set PROPBASE */
> val = (page_to_phys(gic_rdists->prop_page) |
> GICR_PROPBASER_InnerShareable |
> - GICR_PROPBASER_WaWb |
> + GICR_PROPBASER_RaWaWb |
> ((LPI_NRBITS - 1) & GICR_PROPBASER_IDBITS_MASK));
>
> writeq_relaxed(val, rbase + GICR_PROPBASER);
> @@ -1148,7 +1148,7 @@ static void its_cpu_init_lpis(void)
> /* set PENDBASE */
> val = (page_to_phys(pend_page) |
> GICR_PENDBASER_InnerShareable |
> - GICR_PENDBASER_WaWb);
> + GICR_PENDBASER_RaWaWb);
>
> writeq_relaxed(val, rbase + GICR_PENDBASER);
> tmp = readq_relaxed(rbase + GICR_PENDBASER);
> @@ -1712,7 +1712,7 @@ static int __init its_probe_one(struct resource
> *res,
> goto out_free_tables;
>
> baser = (virt_to_phys(its->cmd_base) |
> - GITS_CBASER_WaWb |
> + GITS_CBASER_RaWaWb |
> GITS_CBASER_InnerShareable |
> (ITS_CMD_QUEUE_SZ / SZ_4K - 1) |
> GITS_CBASER_VALID);
--
Shanker Donthineni
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply
* [PATCH 7/10] mmc: sdhci-xenon: Add support to PHYs of Marvell Xenon SDHC
From: Ziji Hu @ 2016-11-29 2:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAPDyKFr8rX04iY92OeQpSkS+3HN2-FxijCCiDb2sSKvP+TZPog@mail.gmail.com>
Hi Ulf,
On 2016/11/28 23:16, Ulf Hansson wrote:
> On 28 November 2016 at 12:38, Ziji Hu <huziji@marvell.com> wrote:
>> Hi Ulf,
>>
>> On 2016/11/28 19:13, Ulf Hansson wrote:
>>>>
>>>> As you suggest, I replace mmc_wait_for_cmd() with mmc_send_tuning(), to
>>>> send commands for testing current sampling point set in our host PHY.
>>>>
>>>> According to my test result, it shows that mmc_send_tuning() can only support
>>>> tuning command (CMD21/CMD19).
>>>> As a result, we cannot use mmc_send_tuning() when card is in the speed modes
>>>> which doesn't support tuning, such as eMMC HS SDR, eMMC HS DRR and
>>>> SD SDR 12/SDR25/DDR50. Card will not response to tuning commands in those
>>>> speed modes.
>>>>
>>>> Could you please provide suggestions for the speed mode in which tuning is
>>>> not available?
>>>>
>>>
>>> Normally the mmc host driver shouldn't have to care about what the
>>> card supports, as that is the responsibility of the mmc core to
>>> manage.
>>>
>>> The host should only need to implement the ->execute_tuning() ops,
>>> which gets called when the card supports tuning (CMD19/21). Does it
>>> make sense?
>>>
>> I think it is irrelevant to tuning procedure.
>>
>> Our host requires to adjust PHY setting after each time ios setting
>> (SDCLK/bus width/speed mode) is changed.
>> The simplified sequence is:
>> mmc change ios --> mmc_set_ios() --> ->set_ios() --> after sdhci_set_ios(),
>> adjust PHY setting.
>> During PHY setting adjustment, out host driver has to send commands to
>> test current sampling point. Tuning is another independent step.
>
> For those speed modes (or other ios changes) that *don't* requires
> tuning, then what will you do when you send the command to confirm the
> change of PHY setting and it fails?
>
> My assumption is that you will fail anyway, by propagating the error
> to the mmc core. At least that what was my understanding from your
> earlier replies, right!?
>
> Then, I think there are no point having the host driver sending a
> command to confirm the PHY settings, as the mmc core will anyway
> discover if something goes wrong when the next command is sent.
>
> Please correct me if I am wrong!
>
Sorry that I didn't make myself clear.
Our host PHY delay line consists of hundreds of sampling points.
Each sampling point represents a different phase shift.
In lower speed mode, our host driver will scan the delay line.
It will select and test multiple sampling points, other than testing
only single sampling point.
If a sampling point fails to transfer cmd/data, our host driver will
move to test next sampling point, until we find out a group of successful
sampling points which can transfer cmd/data. At last we will select
a perfect one from them.
Thank you.
Best regards,
Hu Ziji
>>
>> Thus our host needs a valid command in PHY setting adjustment. Tuning command
>> can be borrowed to complete this task in SD SDR50. But for other speed mode,
>> we have to find out a valid command.
>
> I thought we agreed on this wasn't necessary? Please see my upper response.
>
> Kind regards
> Uffe
>
^ permalink raw reply
* [PATCH] KVM: arm/arm64: Access CNTHCTL_EL2 bit fields correctly
From: Jintack Lim @ 2016-11-29 3:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <799d03f5-a929-9547-1ae7-94026b76f116@arm.com>
On Mon, Nov 28, 2016 at 1:39 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> On 28/11/16 17:43, Marc Zyngier wrote:
>> Hi Jintack,
Hi Marc,
>>
>> On 28/11/16 16:46, Jintack Lim wrote:
>>> Bit positions of CNTHCTL_EL2 are changing depending on HCR_EL2.E2H bit.
>>> EL1PCEN and EL1PCTEN are 1st and 0th bits when E2H is not set, but they
>>> are 11th and 10th bits respectively when E2H is set. Current code is
>>> unintentionally setting wrong bits to CNTHCTL_EL2 with E2H set, which
>>> may allow guest OS to access physical timer. So, fix it.
>>>
>>> Signed-off-by: Jintack Lim <jintack@cs.columbia.edu>
>>> ---
>>> arch/arm/include/asm/kvm_timer.h | 33 +++++++++++++++++++
>>> arch/arm64/include/asm/kvm_timer.h | 62 ++++++++++++++++++++++++++++++++++++
>>> include/clocksource/arm_arch_timer.h | 6 ++--
>>> virt/kvm/arm/hyp/timer-sr.c | 8 ++---
>>> 4 files changed, 103 insertions(+), 6 deletions(-)
>>> create mode 100644 arch/arm/include/asm/kvm_timer.h
>>> create mode 100644 arch/arm64/include/asm/kvm_timer.h
>>>
>
> [...]
>
>> We could make it nicer (read "faster") by introducing a
>> hyp_alternate_select construct that only returns a value instead
>> of calling a function. I remember writing something like that
>> at some point, and dropping it...
>
> So here's what this could look like (warning, wacky code ahead,
> though I fixed a stupid bug that was present in the previous patch).
> The generated code is quite nice (no branch, only an extra mov
> instruction on the default path)... Of course, completely untested!
This looks much cleaner than my patch.
While we are at it, is it worth to consider that we just need to set
those bits once for VHE case, not for every world switch as an
optimization?
>
> diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h
> index b18e852..d173902 100644
> --- a/arch/arm64/include/asm/kvm_hyp.h
> +++ b/arch/arm64/include/asm/kvm_hyp.h
> @@ -121,6 +121,17 @@ typeof(orig) * __hyp_text fname(void) \
> return val; \
> }
>
> +#define hyp_alternate_select_const(fname, orig, alt, cond) \
> +inline const typeof(orig) __hyp_text fname(void) \
> +{ \
> + typeof(alt) val = orig; \
> + asm volatile(ALTERNATIVE("nop \n", \
> + "mov %0, %1 \n", \
> + cond) \
> + : "+r" (val) : "r" (alt)); \
> + return val; \
> +}
> +
> void __vgic_v2_save_state(struct kvm_vcpu *vcpu);
> void __vgic_v2_restore_state(struct kvm_vcpu *vcpu);
> int __vgic_v2_perform_cpuif_access(struct kvm_vcpu *vcpu);
> diff --git a/virt/kvm/arm/hyp/timer-sr.c b/virt/kvm/arm/hyp/timer-sr.c
> index 798866a..7a783af 100644
> --- a/virt/kvm/arm/hyp/timer-sr.c
> +++ b/virt/kvm/arm/hyp/timer-sr.c
> @@ -21,11 +21,29 @@
>
> #include <asm/kvm_hyp.h>
>
> +#ifdef CONFIG_ARM64
> +static hyp_alternate_select_const(cnthclt_shift, 0, 10,
> + ARM64_HAS_VIRT_HOST_EXTN)
> +
> +#else
> +#define cnthclt_shift() (0)
> +#endif
> +
> +static inline void __hyp_text cnthctl_rmw(u32 clr, u32 set)
> +{
> + u32 val;
> + int shift = cnthclt_shift();
> +
> + val = read_sysreg(cnthctl_el2);
> + val &= ~(clr << shift);
> + val |= set << shift;
> + write_sysreg(val, cnthctl_el2);
> +}
> +
> /* vcpu is already in the HYP VA space */
> void __hyp_text __timer_save_state(struct kvm_vcpu *vcpu)
> {
> struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;
> - u64 val;
>
> if (timer->enabled) {
> timer->cntv_ctl = read_sysreg_el0(cntv_ctl);
> @@ -36,9 +54,7 @@ void __hyp_text __timer_save_state(struct kvm_vcpu *vcpu)
> write_sysreg_el0(0, cntv_ctl);
>
> /* Allow physical timer/counter access for the host */
> - val = read_sysreg(cnthctl_el2);
> - val |= CNTHCTL_EL1PCTEN | CNTHCTL_EL1PCEN;
> - write_sysreg(val, cnthctl_el2);
> + cnthctl_rmw(0, CNTHCTL_EL1PCTEN | CNTHCTL_EL1PCEN);
>
> /* Clear cntvoff for the host */
> write_sysreg(0, cntvoff_el2);
> @@ -48,16 +64,12 @@ void __hyp_text __timer_restore_state(struct kvm_vcpu *vcpu)
> {
> struct kvm *kvm = kern_hyp_va(vcpu->kvm);
> struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;
> - u64 val;
>
> /*
> * Disallow physical timer access for the guest
> * Physical counter access is allowed
> */
> - val = read_sysreg(cnthctl_el2);
> - val &= ~CNTHCTL_EL1PCEN;
> - val |= CNTHCTL_EL1PCTEN;
> - write_sysreg(val, cnthctl_el2);
> + cnthctl_rmw(CNTHCTL_EL1PCEN, CNTHCTL_EL1PCTEN);
>
> if (timer->enabled) {
> write_sysreg(kvm->arch.timer.cntvoff, cntvoff_el2);
>
> Thanks,
>
> M.
> --
> Jazz is not dead. It just smells funny...
>
^ permalink raw reply
* [PATCH v28 4/9] arm64: kdump: implement machine_crash_shutdown()
From: AKASHI Takahiro @ 2016-11-29 5:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161128150433.GC17125@e104818-lin.cambridge.arm.com>
On Mon, Nov 28, 2016 at 03:04:33PM +0000, Catalin Marinas wrote:
> On Thu, Nov 24, 2016 at 06:58:05PM +0900, AKASHI Takahiro wrote:
> > diff --git a/arch/arm64/include/asm/kexec.h b/arch/arm64/include/asm/kexec.h
> > index 04744dc..b5168e8 100644
> > --- a/arch/arm64/include/asm/kexec.h
> > +++ b/arch/arm64/include/asm/kexec.h
> > @@ -40,7 +40,47 @@
> > static inline void crash_setup_regs(struct pt_regs *newregs,
> > struct pt_regs *oldregs)
> > {
> > - /* Empty routine needed to avoid build errors. */
> > + if (oldregs) {
> > + memcpy(newregs, oldregs, sizeof(*newregs));
> > + } else {
> > + u64 tmp1, tmp2;
> > +
> > + __asm__ __volatile__ (
> > + "stp x0, x1, [%2, #16 * 0]\n"
> > + "stp x2, x3, [%2, #16 * 1]\n"
> > + "stp x4, x5, [%2, #16 * 2]\n"
> > + "stp x6, x7, [%2, #16 * 3]\n"
> > + "stp x8, x9, [%2, #16 * 4]\n"
> > + "stp x10, x11, [%2, #16 * 5]\n"
> > + "stp x12, x13, [%2, #16 * 6]\n"
> > + "stp x14, x15, [%2, #16 * 7]\n"
> > + "stp x16, x17, [%2, #16 * 8]\n"
> > + "stp x18, x19, [%2, #16 * 9]\n"
> > + "stp x20, x21, [%2, #16 * 10]\n"
> > + "stp x22, x23, [%2, #16 * 11]\n"
> > + "stp x24, x25, [%2, #16 * 12]\n"
> > + "stp x26, x27, [%2, #16 * 13]\n"
> > + "stp x28, x29, [%2, #16 * 14]\n"
> > + "mov %0, sp\n"
> > + "stp x30, %0, [%2, #16 * 15]\n"
> > +
> > + "/* faked current PSTATE */\n"
> > + "mrs %0, CurrentEL\n"
> > + "mrs %1, SPSEL\n"
> > + "orr %0, %0, %1\n"
> > + "mrs %1, DAIF\n"
> > + "orr %0, %0, %1\n"
> > + "mrs %1, NZCV\n"
> > + "orr %0, %0, %1\n"
> > + /* pc */
> > + "adr %1, 1f\n"
> > + "1:\n"
> > + "stp %1, %0, [%2, #16 * 16]\n"
> > + : "+r" (tmp1), "+r" (tmp2)
> > + : "r" (newregs)
> > + : "memory"
>
> tmp1 and tmp2 are not input arguments here, so you should use the "=&"
> modifier. With my compiler, I get warnings of these variables being used
> uninitialised.
Thank you for this heads-up.
(I misunderstood "+" a bit.)
With this change applied, I confirmed that kdump still works.
-Takahiro AKASHI
> --
> Catalin
^ permalink raw reply
* arasan,sdhci.txt "compatibility" DT binding
From: Rameshwar Sahu @ 2016-11-29 7:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <583C60CC.2020408@free.fr>
Hi Mason,
Nowhere in the documentation do they specify an "IP version".
Some documents do provide a revision number, but that's just
a *documentation* revision number, e.g.
changes in version 3.6 : fix typos
changes in version 9.1a : update company logo
That's why Xilinx used "arasan,sdhci-8.9a" and APM used
"arasan,sdhci-4.9a". These are documentation revisions.
In my opinion, that information is mostly worthless.
Arasan SD/SDIO/eMMC IP has a register which tells about the SD
specification version and Vendor version number
Reg Name: Host controller version register (offset 0FEh)
bit [15:8] is for vendor version number,
But, I have seen that Arasaan vendor version number is same as
document revision number.
On Mon, Nov 28, 2016 at 10:22 PM, Mason <slash.tmp@free.fr> wrote:
> On 28/11/2016 17:15, Arnd Bergmann wrote:
>
>> On Monday, November 28, 2016 4:44:39 PM CET Mason wrote:
>>
>>> Hello,
>>>
>>> @Shawn Lin, could you take a look below and tell me exactly
>>> which IP core(s) Rockchip is using in its SoCs?
>>>
>>> Based on the feedback I received, here is an updated list of
>>> compatible strings and controller versions dealt with by the
>>> drivers/mmc/host/sdhci-of-arasan.c code.
>>>
>>>
>>> Xilinx Zynq:
>>> "SD2.0 / SDIO2.0 / MMC3.31 AHB Host Controller"
>>> "arasan,sdhci-8.9a"
>>> NB: 8.9a is the documentation revision (dated 2011-10-19)
>>> subsequent tweaks labeled 9.0a, 9.1a, 9.2a
>>>
>>> Xilinx ZynqMP:
>>> "SD3.0 / SDIO3.0 / eMMC4.51 AHB Host Controller"
>>> "arasan,sdhci-8.9a"
>>> NB: using the same compatible string as Zynq
>>>
>>> Sigma SMP87xx
>>> "SD3.0 / SDIO3.0 / eMMC4.4 AHB Host Controller"
>>> no compatible string yet, platform-specific init required
>>>
>>> APM:
>>> "SD3.0 / SDIO3.0 / eMMC4.41 AHB Host Controller"
>>> "arasan,sdhci-4.9a"
>>> NB: 4.9a appears to be the documentation revision
>>> no functional diff with "arasan,sdhci-8.9a"
>>>
>>> Rockchip
>>> Exact IP unknown, waiting for Shawn's answer
>>> "arasan,sdhci-5.1"
>>> NB: 5.1 appears to refer to the eMMC standard supported
>>>
>>>
>>> On a final note, there are many variations of the Arasan IP.
>>> I've tracked down at least the following:
>>>
>>> SD_2.0_SDIO_2.0__MMC_3.31_AHB_Host_Controller.pdf
>>> SD_3.0_SDIO_3.0_eMMC_4.41_OCP_Host_Controller.pdf
>>> SD_3.0_SDIO_3.0_eMMC_4.4__AHB_Host_Controller.pdf
>>> SD_3.0_SDIO_3.0_eMMC_4.51_Host_Controller.pdf
>>> SD_3.0_SDIO_3.0_eMMC_4.5__Host_Controller.pdf
>>> SD_4.1_SDIO_4.1_eMMC_4.51_Host_Controller.pdf
>>> SD_4.1_SDIO_4.1_eMMC_5.1__Host_Controller.pdf
>>>
>>> It seems to me the compatible string should specify
>>> the SD/SDIO version AND the eMMC version, since it
>>> seems many combinations are allowed, e.g. eMMC 4.51
>>> has two possible SD versions.
>>>
>>> What do you think?
>>
>> It seems wrong to have the eMMC or SD version in the compatible
>> string. Is that the only difference between the documents you
>> found? Normally there should be a version of IP block itself,
>> besides the supported protocol.
>
> But that is exactly the problem :-)
>
> Nowhere in the documentation do they specify an "IP version".
> Some documents do provide a revision number, but that's just
> a *documentation* revision number, e.g.
>
> changes in version 3.6 : fix typos
> changes in version 9.1a : update company logo
>
> That's why Xilinx used "arasan,sdhci-8.9a" and APM used
> "arasan,sdhci-4.9a". These are documentation revisions.
> In my opinion, that information is mostly worthless.
>
>
> Looking more closely at SD_3.0_SDIO_3.0_eMMC_4.4__AHB_Host_Controller.pdf
> (User Guide, which has more info than Datasheet) I see this:
>
> Changed Host Controller Version Register value from 16'h0002 to 16'h7501
> Changed Host Controller Version Register value from 16'h8301 to 16'h8401
> Changed Host Controller Version Register value from 16'h8401 to 16'h8501
> Changed Host Controller Version Register to 16'h9502
> Changed Host Controller Version Register to 16'h9602
> Changed Host Controller Version Register to 16'h9902
>
> Host controller version register (offset 0FEh)
>
> Vendor Version Number 15:8
> HwInit=0x99
> This status is reserved for the vendor version number.
> The HD should not use this status.
>
> Specification Version Number 7:0
> HwInit=0x02
> This status indicates the Host Controller Spec. Version.
> The upper and lower 4-bits indicate the version.
> Description
> 00 - SD Host Specification version 1.0
> 01 - SD Host Specification version 2.00
> including only the feature of the Test Register
> 02 - SD Host Specification Version 3.00
> others - Reserved
>
> I'm not sure what this "Vendor Version Number" specifies, nor if is
> guaranteed to be unique across controllers.
>
> In SD_3.0_SDIO_3.0_eMMC_4.5__Host_Controller_UserGuide.pdf,
> they write "The Vendor Version Number is set to 0x10 (1.0)"
>
> I don't have a UserGuide for "arasan,sdhci-5.1".
>
> Regards.
>
^ permalink raw reply
* [PATCH v9 06/11] arm/arm64: vgic: Implement VGICv3 CPU interface access
From: Vijay Kilari @ 2016-11-29 7:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161128193938.GF18170@cbox>
On Tue, Nov 29, 2016 at 1:09 AM, Christoffer Dall
<christoffer.dall@linaro.org> wrote:
> On Wed, Nov 23, 2016 at 06:31:53PM +0530, vijay.kilari at gmail.com wrote:
>> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>>
>> VGICv3 CPU interface registers are accessed using
>> KVM_DEV_ARM_VGIC_CPU_SYSREGS ioctl. These registers are accessed
>> as 64-bit. The cpu MPIDR value is passed along with register id.
>> is used to identify the cpu for registers access.
>>
>> The VM that supports SEIs expect it on destination machine to handle
>> guest aborts and hence checked for ICC_CTLR_EL1.SEIS compatibility.
>> Similarly, VM that supports Affinity Level 3 that is required for AArch64
>> mode, is required to be supported on destination machine. Hence checked
>> for ICC_CTLR_EL1.A3V compatibility.
>>
>> The CPU system register handling is spitted into two files
>
> spitted? Did you mean 'split into' ?
>
>> vgic-sys-reg-common.c and vgic-sys-reg-v3.c.
>> The vgic-sys-reg-common.c handles read and write of VGIC CPU registers
>
> So this is weird because everything in virt/kvm/arm/ is exactly supposed
> to be common between arm and arm64 already.
>
> I would rather that you had a copy of vgic-sys-reg-v3.c in arch/arm/kvm/
> and in arch/arm64/kvm/ each taking care of its own architecture.
>
> But note that I didn't actually require that you implemented support for
> GICv3 migration on AArch32 hosts for these patches, I just didn't want
> thigns to silently break.
>
> If we cannot test the AArch32 implementation, we should potentially just
> make sure that is not supported yet, return a proper error to userspace
> and get the AArch64 host implementation correct.
>
> I suggest you move your:
> virt/kvm/arm/vgic/vgic-sys-reg-v3.c to
> arch/arm64/kvm/vgic-sys-reg-v3.c
>
> and rename
> virt/kvm/arm/vgic/vgic-sys-reg-common.c to
> virt/kvm/arm/vgic/vgic-sys-reg-v3.c
>
> And then wait with the AArch32 host side for now, but just make sure it
> compiles and returns an error as opposed to crashing the system if
> someone tries to excercise this interface on an AArch32 host.
I will add arch/arm/kvm/vgic-coproc-v3.c (pls check if file name is ok or not?)
and return -ENXIO as shown below and update document accordingly.
int vgic_v3_has_cpu_sysregs_attr(struct kvm_vcpu *vcpu, bool is_write, u64 id,
u64 *reg)
{
/*
* TODO: Implement for AArch32
*/
return -ENXIO;
}
int vgic_v3_cpu_sysregs_uaccess(struct kvm_vcpu *vcpu, bool is_write, u64 id,
u64 *reg)
{
/*
* TODO: Implement for AArch32
*/
return -ENXIO;
}
>
>> for both AArch64 and AArch32 mode. The vgic-sys-reg-v3.c handles AArch64
>> mode and is compiled only for AArch64 mode.
>>
>> Updated arch/arm/include/uapi/asm/kvm.h with new definitions
>> required to compile for AArch32.
>>
>> The version of VGIC v3 specification is define here
>> Documentation/virtual/kvm/devices/arm-vgic-v3.txt
>>
>> Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
>> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>> ---
[...]
>> +static bool access_gic_aprn(struct kvm_vcpu *vcpu, bool is_write, u8 apr,
>> + u8 idx, unsigned long *reg)
>> +{
>> + struct vgic_cpu *vgic_v3_cpu = &vcpu->arch.vgic_cpu;
>> +
>> + /* num_pri_bits are initialized with HW supported values.
>> + * We can rely safely on num_pri_bits even if VM has not
>> + * restored ICC_CTLR_EL1 before restoring APnR registers.
>> + */
>
> nit: commenting style
ok
>
>> + switch (vgic_v3_cpu->num_pri_bits) {
>> + case 7:
>> + vgic_v3_access_apr_reg(vcpu, is_write, apr, idx, reg);
>> + break;
>> + case 6:
>> + if (idx > 1)
>> + goto err;
>> + vgic_v3_access_apr_reg(vcpu, is_write, apr, idx, reg);
>> + break;
>> + default:
>> + if (idx > 0)
>> + goto err;
>> + vgic_v3_access_apr_reg(vcpu, is_write, apr, idx, reg);
>> + }
>
> It looks to me like userspace can then program active priorities with
> higher numbers than what it will program num_pri_bits to later. Is that
> not weird, or am I missing something?
As long as it is within HW supported priorities it is safe.
>
>> +
>> + return true;
>> +err:
>> + if (!is_write)
>> + *reg = 0;
>> +
>> + return false;
>> +}
>> +
>> +bool access_gic_ap0r_reg(struct kvm_vcpu *vcpu, bool is_write, u8 idx,
>> + unsigned long *reg)
>> +{
>> + return access_gic_aprn(vcpu, is_write, 0, idx, reg);
>> +}
>> +
>> +bool access_gic_ap1r_reg(struct kvm_vcpu *vcpu, bool is_write, u8 idx,
>> + unsigned long *reg)
>> +{
>> + return access_gic_aprn(vcpu, is_write, 1, idx, reg);
>> +}
>> +
>> +bool access_gic_sre_reg(struct kvm_vcpu *vcpu, bool is_write,
>> + unsigned long *reg)
>> +{
>> + struct vgic_v3_cpu_if *vgicv3 = &vcpu->arch.vgic_cpu.vgic_v3;
>> +
>> + /* Validate SRE bit */
>> + if (is_write) {
>> + if (!(*reg & ICC_SRE_EL1_SRE))
>> + return false;
>> + } else {
>> + *reg = vgicv3->vgic_sre;
>> + }
>> +
>> + return true;
>> +}
>> diff --git a/virt/kvm/arm/vgic/vgic-sys-reg-v3.c b/virt/kvm/arm/vgic/vgic-sys-reg-v3.c
>> new file mode 100644
>> index 0000000..82c2f02
>> --- /dev/null
>> +++ b/virt/kvm/arm/vgic/vgic-sys-reg-v3.c
>> @@ -0,0 +1,142 @@
>> +/*
>> + * VGIC system registers handling functions
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#include <linux/kvm.h>
>> +#include <linux/kvm_host.h>
>> +#include <asm/kvm_emulate.h>
>> +#include "vgic.h"
>> +#include "sys_regs.h"
>> +
>> +#define ACCESS_SYS_REG(REG) \
>> +static bool access_gic_##REG##_sys_reg(struct kvm_vcpu *vcpu, \
>> + struct sys_reg_params *p, \
>> + const struct sys_reg_desc *r) \
>> +{ \
>> + unsigned long tmp; \
>> + bool ret; \
>> + \
>> + if (p->is_write) \
>> + tmp = p->regval; \
>> + ret = access_gic_##REG##_reg(vcpu, p->is_write, &tmp); \
>> + if (!p->is_write) \
>> + p->regval = tmp; \
>> + \
>> + return ret; \
>> +}
>> +
>> +ACCESS_SYS_REG(ctlr)
>> +ACCESS_SYS_REG(pmr)
>> +ACCESS_SYS_REG(bpr0)
>> +ACCESS_SYS_REG(bpr1)
>> +ACCESS_SYS_REG(sre)
>> +ACCESS_SYS_REG(grpen0)
>> +ACCESS_SYS_REG(grpen1)
>> +
>> +#define ACCESS_APNR_SYS_REG(REG) \
>> +static bool access_gic_##REG##_sys_reg(struct kvm_vcpu *vcpu, \
>> + struct sys_reg_params *p, \
>> + const struct sys_reg_desc *r) \
>> +{ \
>> + unsigned long tmp; \
>> + u8 idx = p->Op2 & 3; \
>> + bool ret; \
>> + \
>> + if (p->is_write) \
>> + tmp = p->regval; \
>> + ret = access_gic_##REG##_reg(vcpu, p->is_write, idx, &tmp); \
>> + if (!p->is_write) \
>> + p->regval = tmp; \
>> + \
>> + return ret; \
>> +}
>> +
>> +ACCESS_APNR_SYS_REG(ap0r)
>> +ACCESS_APNR_SYS_REG(ap1r)
>
> I don't get these indirections. Why can't you call the functions
> directly?
The code is same for accessing the registers hence added this indirection.
>
>> +
>> +static const struct sys_reg_desc gic_v3_icc_reg_descs[] = {
>> + /* ICC_PMR_EL1 */
>> + { Op0(3), Op1(0), CRn(4), CRm(6), Op2(0), access_gic_pmr_sys_reg },
>> + /* ICC_BPR0_EL1 */
>> + { Op0(3), Op1(0), CRn(12), CRm(8), Op2(3), access_gic_bpr0_sys_reg },
>> + /* ICC_AP0R0_EL1 */
>> + { Op0(3), Op1(0), CRn(12), CRm(8), Op2(4), access_gic_ap0r_sys_reg },
>> + /* ICC_AP0R1_EL1 */
>> + { Op0(3), Op1(0), CRn(12), CRm(8), Op2(5), access_gic_ap0r_sys_reg },
>> + /* ICC_AP0R2_EL1 */
>> + { Op0(3), Op1(0), CRn(12), CRm(8), Op2(6), access_gic_ap0r_sys_reg },
>> + /* ICC_AP0R3_EL1 */
>> + { Op0(3), Op1(0), CRn(12), CRm(8), Op2(7), access_gic_ap0r_sys_reg },
>> + /* ICC_AP1R0_EL1 */
>> + { Op0(3), Op1(0), CRn(12), CRm(9), Op2(0), access_gic_ap1r_sys_reg },
>> + /* ICC_AP1R1_EL1 */
>> + { Op0(3), Op1(0), CRn(12), CRm(9), Op2(1), access_gic_ap1r_sys_reg },
>> + /* ICC_AP1R2_EL1 */
>> + { Op0(3), Op1(0), CRn(12), CRm(9), Op2(2), access_gic_ap1r_sys_reg },
>> + /* ICC_AP1R3_EL1 */
>> + { Op0(3), Op1(0), CRn(12), CRm(9), Op2(3), access_gic_ap1r_sys_reg },
>> + /* ICC_BPR1_EL1 */
>> + { Op0(3), Op1(0), CRn(12), CRm(12), Op2(3), access_gic_bpr1_sys_reg },
>> + /* ICC_CTLR_EL1 */
>> + { Op0(3), Op1(0), CRn(12), CRm(12), Op2(4), access_gic_ctlr_sys_reg },
>> + /* ICC_SRE_EL1 */
>> + { Op0(3), Op1(0), CRn(12), CRm(12), Op2(5), access_gic_sre_sys_reg },
>> + /* ICC_IGRPEN0_EL1 */
>> + { Op0(3), Op1(0), CRn(12), CRm(12), Op2(6), access_gic_grpen0_sys_reg },
>> + /* ICC_GRPEN1_EL1 */
>> + { Op0(3), Op1(0), CRn(12), CRm(12), Op2(7), access_gic_grpen1_sys_reg },
>> +};
>> +
>> +int vgic_v3_has_cpu_sysregs_attr(struct kvm_vcpu *vcpu, bool is_write, u64 id,
>> + u64 *reg)
>> +{
>> + struct sys_reg_params params;
>> + u64 sysreg = (id & KVM_DEV_ARM_VGIC_SYSREG_MASK) | KVM_REG_SIZE_U64;
>> +
>> + params.regval = *reg;
>> + params.is_write = is_write;
>> + params.is_aarch32 = false;
>> + params.is_32bit = false;
>> +
>> + if (find_reg_by_id(sysreg, ¶ms, gic_v3_icc_reg_descs,
>> + ARRAY_SIZE(gic_v3_icc_reg_descs)))
>> + return 0;
>> +
>> + return -ENXIO;
>> +}
>> +
>> +int vgic_v3_cpu_sysregs_uaccess(struct kvm_vcpu *vcpu, bool is_write, u64 id,
>> + u64 *reg)
>> +{
>> + struct sys_reg_params params;
>> + const struct sys_reg_desc *r;
>> + u64 sysreg = (id & KVM_DEV_ARM_VGIC_SYSREG_MASK) | KVM_REG_SIZE_U64;
>> +
>> + if (is_write)
>> + params.regval = *reg;
>> + params.is_write = is_write;
>> + params.is_aarch32 = false;
>> + params.is_32bit = false;
>> +
>> + r = find_reg_by_id(sysreg, ¶ms, gic_v3_icc_reg_descs,
>> + ARRAY_SIZE(gic_v3_icc_reg_descs));
>> + if (!r)
>> + return -ENXIO;
>> +
>> + if (!r->access(vcpu, ¶ms, r))
>> + return -EINVAL;
>> +
>> + if (!is_write)
>> + *reg = params.regval;
>> +
>> + return 0;
>> +}
>> diff --git a/virt/kvm/arm/vgic/vgic-v3.c b/virt/kvm/arm/vgic/vgic-v3.c
>> index a3ff04b..6e7400e 100644
>> --- a/virt/kvm/arm/vgic/vgic-v3.c
>> +++ b/virt/kvm/arm/vgic/vgic-v3.c
>> @@ -240,6 +240,13 @@ void vgic_v3_enable(struct kvm_vcpu *vcpu)
>> vgic_v3->vgic_sre = 0;
>> }
>>
>> + vcpu->arch.vgic_cpu.num_id_bits = (kvm_vgic_global_state.ich_vtr_el2 &
>> + ICH_VTR_ID_BITS_MASK) >>
>> + ICH_VTR_ID_BITS_SHIFT;
>> + vcpu->arch.vgic_cpu.num_pri_bits = ((kvm_vgic_global_state.ich_vtr_el2 &
>> + ICH_VTR_PRI_BITS_MASK) >>
>> + ICH_VTR_PRI_BITS_SHIFT) + 1;
>> +
>> /* Get the show on the road... */
>> vgic_v3->vgic_hcr = ICH_HCR_EN;
>> }
>> @@ -340,6 +347,7 @@ int vgic_v3_probe(const struct gic_kvm_info *info)
>> */
>> kvm_vgic_global_state.nr_lr = (ich_vtr_el2 & 0xf) + 1;
>> kvm_vgic_global_state.can_emulate_gicv2 = false;
>> + kvm_vgic_global_state.ich_vtr_el2 = ich_vtr_el2;
>>
>> if (!info->vcpu.start) {
>> kvm_info("GICv3: no GICV resource entry\n");
>> diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h
>> index 9232791..af23399 100644
>> --- a/virt/kvm/arm/vgic/vgic.h
>> +++ b/virt/kvm/arm/vgic/vgic.h
>> @@ -140,6 +140,28 @@ int vgic_v3_dist_uaccess(struct kvm_vcpu *vcpu, bool is_write,
>> int offset, u32 *val);
>> int vgic_v3_redist_uaccess(struct kvm_vcpu *vcpu, bool is_write,
>> int offset, u32 *val);
>> +int vgic_v3_cpu_sysregs_uaccess(struct kvm_vcpu *vcpu, bool is_write,
>> + u64 id, u64 *val);
>> +int vgic_v3_has_cpu_sysregs_attr(struct kvm_vcpu *vcpu, bool is_write, u64 id,
>> + u64 *reg);
>> +bool access_gic_ctlr_reg(struct kvm_vcpu *vcpu, bool is_write,
>> + unsigned long *reg);
>> +bool access_gic_pmr_reg(struct kvm_vcpu *vcpu, bool is_write,
>> + unsigned long *reg);
>> +bool access_gic_bpr0_reg(struct kvm_vcpu *vcpu, bool is_write,
>> + unsigned long *reg);
>> +bool access_gic_bpr1_reg(struct kvm_vcpu *vcpu, bool is_write,
>> + unsigned long *reg);
>> +bool access_gic_grpen0_reg(struct kvm_vcpu *vcpu, bool is_write,
>> + unsigned long *reg);
>> +bool access_gic_grpen1_reg(struct kvm_vcpu *vcpu, bool is_write,
>> + unsigned long *reg);
>> +bool access_gic_ap0r_reg(struct kvm_vcpu *vcpu, bool is_write,
>> + u8 idx, unsigned long *reg);
>> +bool access_gic_ap1r_reg(struct kvm_vcpu *vcpu, bool is_write,
>> + u8 idx, unsigned long *reg);
>> +bool access_gic_sre_reg(struct kvm_vcpu *vcpu, bool is_write,
>> + unsigned long *reg);
>
> nit: since all of this is exported, I would name them vgic_access_ctlr()
> and so on. The _reg postfix is probably also unnecessary for all of
> these.
OK
>
>> int kvm_register_vgic_device(unsigned long type);
>> void vgic_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr);
>> void vgic_get_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr);
>> --
>> 1.9.1
>>
>
> Thanks,
> -Christoffer
^ permalink raw reply
* [PATCH 0/2] Support ARM SMCC SoC vendor quirks
From: Andy Gross @ 2016-11-29 7:44 UTC (permalink / raw)
To: linux-arm-kernel
At least one SoC vendor (Qualcomm) requires additional processing done
during ARM SMCCC calls. As such, an additional parameter to the
arm_smccc_smc is required to be able to handle SoC specific quirks.
The Qualcomm quirk is necessary due to the fact that the scm call can
be interrupted on Qualcomm ARM64 platforms. When this occurs, the
call must be restarted using information that was passed back during
the original smc call.
The first patch in this series adds a quirk structure and also adds a
quirk paramter to arm_smccc_smc calls. All of the current users of
the call are modified to accomodate the new API parameter.
The second patch adds the Qualcomm quirk and also implements the
Qualcomm firmware changes required to handle the restarting of the
interrupted SMC call.
The original patch set for the SMCCC session ID is located at:
https://lkml.org/lkml/2016/8/20/7
Andy Gross (2):
arm: kernel: Add SMC structure parameter
firmware: qcom: scm: Fix interrupted SCM calls
arch/arm/kernel/smccc-call.S | 3 ++-
arch/arm/mach-artpec/board-artpec6.c | 2 +-
arch/arm64/kernel/asm-offsets.c | 7 +++++--
arch/arm64/kernel/smccc-call.S | 12 ++++++++++--
drivers/clk/rockchip/clk-ddr.c | 6 +++---
drivers/devfreq/rk3399_dmc.c | 6 +++---
drivers/firmware/meson/meson_sm.c | 2 +-
drivers/firmware/psci.c | 2 +-
drivers/firmware/qcom_scm-64.c | 13 ++++++++++---
drivers/gpu/drm/mediatek/mtk_hdmi.c | 2 +-
include/linux/arm-smccc.h | 29 ++++++++++++++++++++++++-----
11 files changed, 61 insertions(+), 23 deletions(-)
--
1.9.1
^ permalink raw reply
* [PATCH 1/2] arm: kernel: Add SMC structure parameter
From: Andy Gross @ 2016-11-29 7:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480405463-23165-1-git-send-email-andy.gross@linaro.org>
This patch adds a quirk parameter to the arm_smccc_smc call. The quirk
structure allows for specialized SMC operations due to SoC specific
requirements.
This patch also fixes up all the current users of the arm_smccc_smc API.
This patch and partial implementation was suggested by Will Deacon.
Signed-off-by: Andy Gross <andy.gross@linaro.org>
---
arch/arm/kernel/smccc-call.S | 3 ++-
arch/arm/mach-artpec/board-artpec6.c | 2 +-
arch/arm64/kernel/asm-offsets.c | 7 +++++--
arch/arm64/kernel/smccc-call.S | 3 ++-
drivers/clk/rockchip/clk-ddr.c | 6 +++---
drivers/devfreq/rk3399_dmc.c | 6 +++---
drivers/firmware/meson/meson_sm.c | 2 +-
drivers/firmware/psci.c | 2 +-
drivers/firmware/qcom_scm-64.c | 4 ++--
drivers/gpu/drm/mediatek/mtk_hdmi.c | 2 +-
include/linux/arm-smccc.h | 18 ++++++++++++++++--
11 files changed, 37 insertions(+), 18 deletions(-)
diff --git a/arch/arm/kernel/smccc-call.S b/arch/arm/kernel/smccc-call.S
index 37669e7..e77950a 100644
--- a/arch/arm/kernel/smccc-call.S
+++ b/arch/arm/kernel/smccc-call.S
@@ -47,7 +47,8 @@ UNWIND( .fnend)
/*
* void smccc_smc(unsigned long a0, unsigned long a1, unsigned long a2,
* unsigned long a3, unsigned long a4, unsigned long a5,
- * unsigned long a6, unsigned long a7, struct arm_smccc_res *res)
+ * unsigned long a6, unsigned long a7, struct arm_smccc_res *res,
+ * struct arm_smccc_quirk *quirk)
*/
ENTRY(arm_smccc_smc)
SMCCC SMCCC_SMC
diff --git a/arch/arm/mach-artpec/board-artpec6.c b/arch/arm/mach-artpec/board-artpec6.c
index a0b1979..3a4d330 100644
--- a/arch/arm/mach-artpec/board-artpec6.c
+++ b/arch/arm/mach-artpec/board-artpec6.c
@@ -50,7 +50,7 @@ static void artpec6_l2c310_write_sec(unsigned long val, unsigned reg)
struct arm_smccc_res res;
arm_smccc_smc(SECURE_OP_L2C_WRITEREG, reg, val, 0,
- 0, 0, 0, 0, &res);
+ 0, 0, 0, 0, &res, NULL);
WARN_ON(res.a0);
}
diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
index 4a2f0f0..c58ddf8 100644
--- a/arch/arm64/kernel/asm-offsets.c
+++ b/arch/arm64/kernel/asm-offsets.c
@@ -140,8 +140,11 @@ int main(void)
DEFINE(SLEEP_STACK_DATA_SYSTEM_REGS, offsetof(struct sleep_stack_data, system_regs));
DEFINE(SLEEP_STACK_DATA_CALLEE_REGS, offsetof(struct sleep_stack_data, callee_saved_regs));
#endif
- DEFINE(ARM_SMCCC_RES_X0_OFFS, offsetof(struct arm_smccc_res, a0));
- DEFINE(ARM_SMCCC_RES_X2_OFFS, offsetof(struct arm_smccc_res, a2));
+ DEFINE(ARM_SMCCC_RES_X0_OFFS, offsetof(struct arm_smccc_res, a0));
+ DEFINE(ARM_SMCCC_RES_X2_OFFS, offsetof(struct arm_smccc_res, a2));
+ DEFINE(ARM_SMCCC_QUIRK_ID_OFFS, offsetof(struct arm_smccc_quirk, id));
+ DEFINE(ARM_SMCCC_QUIRK_STATE_OFFS, offsetof(struct arm_smccc_quirk, state));
+
BLANK();
DEFINE(HIBERN_PBE_ORIG, offsetof(struct pbe, orig_address));
DEFINE(HIBERN_PBE_ADDR, offsetof(struct pbe, address));
diff --git a/arch/arm64/kernel/smccc-call.S b/arch/arm64/kernel/smccc-call.S
index ae0496f..7b0b3f6 100644
--- a/arch/arm64/kernel/smccc-call.S
+++ b/arch/arm64/kernel/smccc-call.S
@@ -27,7 +27,8 @@
/*
* void arm_smccc_smc(unsigned long a0, unsigned long a1, unsigned long a2,
* unsigned long a3, unsigned long a4, unsigned long a5,
- * unsigned long a6, unsigned long a7, struct arm_smccc_res *res)
+ * unsigned long a6, unsigned long a7, struct arm_smccc_res *res,
+ * struct arm_smccc_quirk *quirk)
*/
ENTRY(arm_smccc_smc)
SMCCC smc
diff --git a/drivers/clk/rockchip/clk-ddr.c b/drivers/clk/rockchip/clk-ddr.c
index 8feba93..cbdc0f8 100644
--- a/drivers/clk/rockchip/clk-ddr.c
+++ b/drivers/clk/rockchip/clk-ddr.c
@@ -45,7 +45,7 @@ static int rockchip_ddrclk_sip_set_rate(struct clk_hw *hw, unsigned long drate,
spin_lock_irqsave(ddrclk->lock, flags);
arm_smccc_smc(ROCKCHIP_SIP_DRAM_FREQ, drate, 0,
ROCKCHIP_SIP_CONFIG_DRAM_SET_RATE,
- 0, 0, 0, 0, &res);
+ 0, 0, 0, 0, &res, NULL);
spin_unlock_irqrestore(ddrclk->lock, flags);
return res.a0;
@@ -59,7 +59,7 @@ static int rockchip_ddrclk_sip_set_rate(struct clk_hw *hw, unsigned long drate,
arm_smccc_smc(ROCKCHIP_SIP_DRAM_FREQ, 0, 0,
ROCKCHIP_SIP_CONFIG_DRAM_GET_RATE,
- 0, 0, 0, 0, &res);
+ 0, 0, 0, 0, &res, NULL);
return res.a0;
}
@@ -72,7 +72,7 @@ static long rockchip_ddrclk_sip_round_rate(struct clk_hw *hw,
arm_smccc_smc(ROCKCHIP_SIP_DRAM_FREQ, rate, 0,
ROCKCHIP_SIP_CONFIG_DRAM_ROUND_RATE,
- 0, 0, 0, 0, &res);
+ 0, 0, 0, 0, &res, NULL);
return res.a0;
}
diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c
index e24b73d..a2e1f4c 100644
--- a/drivers/devfreq/rk3399_dmc.c
+++ b/drivers/devfreq/rk3399_dmc.c
@@ -258,7 +258,7 @@ static irqreturn_t rk3399_dmc_irq(int irq, void *dev_id)
/* Clear the DCF interrupt */
arm_smccc_smc(ROCKCHIP_SIP_DRAM_FREQ, 0, 0,
ROCKCHIP_SIP_CONFIG_DRAM_CLR_IRQ,
- 0, 0, 0, 0, &res);
+ 0, 0, 0, 0, &res, NULL);
return IRQ_HANDLED;
}
@@ -395,7 +395,7 @@ static int rk3399_dmcfreq_probe(struct platform_device *pdev)
for (index = 0; index < size; index++) {
arm_smccc_smc(ROCKCHIP_SIP_DRAM_FREQ, *timing++, index,
ROCKCHIP_SIP_CONFIG_DRAM_SET_PARAM,
- 0, 0, 0, 0, &res);
+ 0, 0, 0, 0, &res, NULL);
if (res.a0) {
dev_err(dev, "Failed to set dram param: %ld\n",
res.a0);
@@ -406,7 +406,7 @@ static int rk3399_dmcfreq_probe(struct platform_device *pdev)
arm_smccc_smc(ROCKCHIP_SIP_DRAM_FREQ, 0, 0,
ROCKCHIP_SIP_CONFIG_DRAM_INIT,
- 0, 0, 0, 0, &res);
+ 0, 0, 0, 0, &res, NULL);
/*
* We add a devfreq driver to our parent since it has a device tree node
diff --git a/drivers/firmware/meson/meson_sm.c b/drivers/firmware/meson/meson_sm.c
index b0d2549..8863637 100644
--- a/drivers/firmware/meson/meson_sm.c
+++ b/drivers/firmware/meson/meson_sm.c
@@ -74,7 +74,7 @@ static u32 __meson_sm_call(u32 cmd, u32 arg0, u32 arg1, u32 arg2,
{
struct arm_smccc_res res;
- arm_smccc_smc(cmd, arg0, arg1, arg2, arg3, arg4, 0, 0, &res);
+ arm_smccc_smc(cmd, arg0, arg1, arg2, arg3, arg4, 0, 0, &res, NULL);
return res.a0;
}
diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
index 8263429..130c50f 100644
--- a/drivers/firmware/psci.c
+++ b/drivers/firmware/psci.c
@@ -126,7 +126,7 @@ static unsigned long __invoke_psci_fn_smc(unsigned long function_id,
{
struct arm_smccc_res res;
- arm_smccc_smc(function_id, arg0, arg1, arg2, 0, 0, 0, 0, &res);
+ arm_smccc_smc(function_id, arg0, arg1, arg2, 0, 0, 0, 0, &res, NULL);
return res.a0;
}
diff --git a/drivers/firmware/qcom_scm-64.c b/drivers/firmware/qcom_scm-64.c
index 4a0f5ea..d164a9b 100644
--- a/drivers/firmware/qcom_scm-64.c
+++ b/drivers/firmware/qcom_scm-64.c
@@ -134,7 +134,7 @@ static int qcom_scm_call(struct device *dev, u32 svc_id, u32 cmd_id,
do {
arm_smccc_smc(cmd, desc->arginfo, desc->args[0],
desc->args[1], desc->args[2], x5, 0, 0,
- res);
+ res, NULL);
} while (res->a0 == QCOM_SCM_INTERRUPTED);
mutex_unlock(&qcom_scm_lock);
@@ -253,7 +253,7 @@ void __qcom_scm_init(void)
ARM_SMCCC_OWNER_SIP, function);
arm_smccc_smc(cmd, QCOM_SCM_ARGS(1), cmd & (~BIT(ARM_SMCCC_TYPE_SHIFT)),
- 0, 0, 0, 0, 0, &res);
+ 0, 0, 0, 0, 0, &res, NULL);
if (!res.a0 && res.a1)
qcom_smccc_convention = ARM_SMCCC_SMC_64;
diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c b/drivers/gpu/drm/mediatek/mtk_hdmi.c
index 71227de..073f3845 100644
--- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
+++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
@@ -240,7 +240,7 @@ static void mtk_hdmi_hw_make_reg_writable(struct mtk_hdmi *hdmi, bool enable)
* this control bit to enable HDMI output in supervisor mode.
*/
arm_smccc_smc(MTK_SIP_SET_AUTHORIZED_SECURE_REG, 0x14000904, 0x80000000,
- 0, 0, 0, 0, 0, &res);
+ 0, 0, 0, 0, 0, &res, NULL);
regmap_update_bits(hdmi->sys_regmap, hdmi->sys_offset + HDMI_SYS_CFG20,
HDMI_PCLK_FREE_RUN, enable ? HDMI_PCLK_FREE_RUN : 0);
diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
index b5abfda..74231b4c 100644
--- a/include/linux/arm-smccc.h
+++ b/include/linux/arm-smccc.h
@@ -72,19 +72,33 @@ struct arm_smccc_res {
};
/**
+ * struct arm_smccc_quirk - Contains quirk information
+ * id contains quirk identification
+ * state contains the quirk specific information
+ */
+struct arm_smccc_quirk {
+ int id;
+ union {
+ unsigned long a6;
+ } state;
+};
+
+/**
* arm_smccc_smc() - make SMC calls
* @a0-a7: arguments passed in registers 0 to 7
* @res: result values from registers 0 to 3
+ * @quirk: optional quirk structure
*
* This function is used to make SMC calls following SMC Calling Convention.
* The content of the supplied param are copied to registers 0 to 7 prior
* to the SMC instruction. The return values are updated with the content
- * from register 0 to 3 on return from the SMC instruction.
+ * from register 0 to 3 on return from the SMC instruction. An optional
+ * quirk structure provides vendor specific behavior.
*/
asmlinkage void arm_smccc_smc(unsigned long a0, unsigned long a1,
unsigned long a2, unsigned long a3, unsigned long a4,
unsigned long a5, unsigned long a6, unsigned long a7,
- struct arm_smccc_res *res);
+ struct arm_smccc_res *res, struct arm_smccc_quirk *quirk);
/**
* arm_smccc_hvc() - make HVC calls
--
1.9.1
^ permalink raw reply related
* [PATCH 2/2] firmware: qcom: scm: Fix interrupted SCM calls
From: Andy Gross @ 2016-11-29 7:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480405463-23165-1-git-send-email-andy.gross@linaro.org>
This patch adds a Qualcomm specific quirk to the arm_smccc_smc call.
On Qualcomm ARM64 platforms, the SMC call can return before it has
completed. If this occurs, the call can be restarted, but it requires
using the returned session ID value from the interrupted SMC call.
The quirk stores off the session ID from the interrupted call in the
quirk structure so that it can be used by the caller.
This patch folds in a fix given by Sricharan R:
https://lkml.org/lkml/2016/9/28/272
Signed-off-by: Andy Gross <andy.gross@linaro.org>
---
arch/arm64/kernel/smccc-call.S | 9 ++++++++-
drivers/firmware/qcom_scm-64.c | 11 +++++++++--
include/linux/arm-smccc.h | 11 ++++++++---
3 files changed, 25 insertions(+), 6 deletions(-)
diff --git a/arch/arm64/kernel/smccc-call.S b/arch/arm64/kernel/smccc-call.S
index 7b0b3f6..8191fde 100644
--- a/arch/arm64/kernel/smccc-call.S
+++ b/arch/arm64/kernel/smccc-call.S
@@ -12,6 +12,7 @@
*
*/
#include <linux/linkage.h>
+#include <linux/arm-smccc.h>
#include <asm/asm-offsets.h>
.macro SMCCC instr
@@ -20,7 +21,13 @@
ldr x4, [sp]
stp x0, x1, [x4, #ARM_SMCCC_RES_X0_OFFS]
stp x2, x3, [x4, #ARM_SMCCC_RES_X2_OFFS]
- ret
+ ldr x4, [sp, #8]
+ cbz x4, 1f /* no quirk structure */
+ ldr x9, [x4, #ARM_SMCCC_QUIRK_ID_OFFS]
+ cmp x9, #ARM_SMCCC_QUIRK_QCOM_A6
+ b.ne 1f
+ str x6, [x4, ARM_SMCCC_QUIRK_STATE_OFFS]
+1: ret
.cfi_endproc
.endm
diff --git a/drivers/firmware/qcom_scm-64.c b/drivers/firmware/qcom_scm-64.c
index d164a9b..6e997a6 100644
--- a/drivers/firmware/qcom_scm-64.c
+++ b/drivers/firmware/qcom_scm-64.c
@@ -91,6 +91,7 @@ static int qcom_scm_call(struct device *dev, u32 svc_id, u32 cmd_id,
dma_addr_t args_phys = 0;
void *args_virt = NULL;
size_t alloc_len;
+ struct arm_smccc_quirk quirk = {.id = ARM_SMCCC_QUIRK_QCOM_A6};
if (unlikely(arglen > N_REGISTER_ARGS)) {
alloc_len = N_EXT_QCOM_SCM_ARGS * sizeof(u64);
@@ -131,10 +132,16 @@ static int qcom_scm_call(struct device *dev, u32 svc_id, u32 cmd_id,
qcom_smccc_convention,
ARM_SMCCC_OWNER_SIP, fn_id);
+ quirk.state.a6 = 0;
+
do {
arm_smccc_smc(cmd, desc->arginfo, desc->args[0],
- desc->args[1], desc->args[2], x5, 0, 0,
- res, NULL);
+ desc->args[1], desc->args[2], x5,
+ quirk.state.a6, 0, res, &quirk);
+
+ if (res->a0 == QCOM_SCM_INTERRUPTED)
+ cmd = res->a0;
+
} while (res->a0 == QCOM_SCM_INTERRUPTED);
mutex_unlock(&qcom_scm_lock);
diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
index 74231b4c..0a239a0 100644
--- a/include/linux/arm-smccc.h
+++ b/include/linux/arm-smccc.h
@@ -14,9 +14,6 @@
#ifndef __LINUX_ARM_SMCCC_H
#define __LINUX_ARM_SMCCC_H
-#include <linux/linkage.h>
-#include <linux/types.h>
-
/*
* This file provides common defines for ARM SMC Calling Convention as
* specified in
@@ -60,6 +57,13 @@
#define ARM_SMCCC_OWNER_TRUSTED_OS 50
#define ARM_SMCCC_OWNER_TRUSTED_OS_END 63
+#define ARM_SMCCC_QUIRK_NONE 0
+#define ARM_SMCCC_QUIRK_QCOM_A6 1 /* Save/restore register a6 */
+
+#ifndef __ASSEMBLY__
+
+#include <linux/linkage.h>
+#include <linux/types.h>
/**
* struct arm_smccc_res - Result from SMC/HVC call
* @a0-a3 result values from registers 0 to 3
@@ -115,4 +119,5 @@ asmlinkage void arm_smccc_hvc(unsigned long a0, unsigned long a1,
unsigned long a5, unsigned long a6, unsigned long a7,
struct arm_smccc_res *res);
+#endif /*__ASSEMBLY__*/
#endif /*__LINUX_ARM_SMCCC_H*/
--
1.9.1
^ permalink raw reply related
* [PATCH 7/10] mmc: sdhci-xenon: Add support to PHYs of Marvell Xenon SDHC
From: Ulf Hansson @ 2016-11-29 7:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <c30cead8-17b6-48b0-7355-cd82268842e1@marvell.com>
On 29 November 2016 at 03:53, Ziji Hu <huziji@marvell.com> wrote:
> Hi Ulf,
>
> On 2016/11/28 23:16, Ulf Hansson wrote:
>> On 28 November 2016 at 12:38, Ziji Hu <huziji@marvell.com> wrote:
>>> Hi Ulf,
>>>
>>> On 2016/11/28 19:13, Ulf Hansson wrote:
>>>>>
>>>>> As you suggest, I replace mmc_wait_for_cmd() with mmc_send_tuning(), to
>>>>> send commands for testing current sampling point set in our host PHY.
>>>>>
>>>>> According to my test result, it shows that mmc_send_tuning() can only support
>>>>> tuning command (CMD21/CMD19).
>>>>> As a result, we cannot use mmc_send_tuning() when card is in the speed modes
>>>>> which doesn't support tuning, such as eMMC HS SDR, eMMC HS DRR and
>>>>> SD SDR 12/SDR25/DDR50. Card will not response to tuning commands in those
>>>>> speed modes.
>>>>>
>>>>> Could you please provide suggestions for the speed mode in which tuning is
>>>>> not available?
>>>>>
>>>>
>>>> Normally the mmc host driver shouldn't have to care about what the
>>>> card supports, as that is the responsibility of the mmc core to
>>>> manage.
>>>>
>>>> The host should only need to implement the ->execute_tuning() ops,
>>>> which gets called when the card supports tuning (CMD19/21). Does it
>>>> make sense?
>>>>
>>> I think it is irrelevant to tuning procedure.
>>>
>>> Our host requires to adjust PHY setting after each time ios setting
>>> (SDCLK/bus width/speed mode) is changed.
>>> The simplified sequence is:
>>> mmc change ios --> mmc_set_ios() --> ->set_ios() --> after sdhci_set_ios(),
>>> adjust PHY setting.
>>> During PHY setting adjustment, out host driver has to send commands to
>>> test current sampling point. Tuning is another independent step.
>>
>> For those speed modes (or other ios changes) that *don't* requires
>> tuning, then what will you do when you send the command to confirm the
>> change of PHY setting and it fails?
>>
>> My assumption is that you will fail anyway, by propagating the error
>> to the mmc core. At least that what was my understanding from your
>> earlier replies, right!?
>>
>> Then, I think there are no point having the host driver sending a
>> command to confirm the PHY settings, as the mmc core will anyway
>> discover if something goes wrong when the next command is sent.
>>
>> Please correct me if I am wrong!
>>
>
> Sorry that I didn't make myself clear.
>
> Our host PHY delay line consists of hundreds of sampling points.
> Each sampling point represents a different phase shift.
>
> In lower speed mode, our host driver will scan the delay line.
> It will select and test multiple sampling points, other than testing
> only single sampling point.
>
> If a sampling point fails to transfer cmd/data, our host driver will
> move to test next sampling point, until we find out a group of successful
> sampling points which can transfer cmd/data. At last we will select
> a perfect one from them.
Ahh, I see. Unfortunate, this is going to be very hard to implement properly.
The main problem is that the host driver has *no* knowledge about the
internal state of the card, as that is the responsibility of the mmc
core to keep track of.
If the host driver would send a command during every update of the
"ios" setting, from ->set_ios(), for sure it would lead to commands
being sent that are "forbidden" in the current internal state of the
card.
This would lead to that the card initialization sequence fails,
because the card may move to an unknown internal state and the mmc
core would have no knowledge about what happened.
Hmm..
Can you specify, *exactly*, under which "ios updates" you need to
verify updated PHY setting changes by sending a cmd/data? Also, please
specify if it's enough to only test the CMD line or also DATA lines.
Kind regards
Uffe
^ permalink raw reply
* [PATCH] crypto: arm64/sha2: add generated .S files to .gitignore
From: Herbert Xu @ 2016-11-29 8:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480340269-30418-1-git-send-email-ard.biesheuvel@linaro.org>
On Mon, Nov 28, 2016 at 02:37:49PM +0100, Ard Biesheuvel wrote:
> Add the files that are generated by the recently merged OpenSSL
> SHA-256/512 implementation to .gitignore so Git disregards them
> when showing untracked files.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Patch applied. Thanks.
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* [PATCH v9 06/11] arm/arm64: vgic: Implement VGICv3 CPU interface access
From: Christoffer Dall @ 2016-11-29 8:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CALicx6s4E0-VJMm6Ff5qu=sZgXu-mUnJdDCmaiZO0i3yzF8wxA@mail.gmail.com>
On Tue, Nov 29, 2016 at 01:08:26PM +0530, Vijay Kilari wrote:
> On Tue, Nov 29, 2016 at 1:09 AM, Christoffer Dall
> <christoffer.dall@linaro.org> wrote:
> > On Wed, Nov 23, 2016 at 06:31:53PM +0530, vijay.kilari at gmail.com wrote:
> >> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
> >>
> >> VGICv3 CPU interface registers are accessed using
> >> KVM_DEV_ARM_VGIC_CPU_SYSREGS ioctl. These registers are accessed
> >> as 64-bit. The cpu MPIDR value is passed along with register id.
> >> is used to identify the cpu for registers access.
> >>
> >> The VM that supports SEIs expect it on destination machine to handle
> >> guest aborts and hence checked for ICC_CTLR_EL1.SEIS compatibility.
> >> Similarly, VM that supports Affinity Level 3 that is required for AArch64
> >> mode, is required to be supported on destination machine. Hence checked
> >> for ICC_CTLR_EL1.A3V compatibility.
> >>
> >> The CPU system register handling is spitted into two files
> >
> > spitted? Did you mean 'split into' ?
> >
> >> vgic-sys-reg-common.c and vgic-sys-reg-v3.c.
> >> The vgic-sys-reg-common.c handles read and write of VGIC CPU registers
> >
> > So this is weird because everything in virt/kvm/arm/ is exactly supposed
> > to be common between arm and arm64 already.
> >
> > I would rather that you had a copy of vgic-sys-reg-v3.c in arch/arm/kvm/
> > and in arch/arm64/kvm/ each taking care of its own architecture.
> >
> > But note that I didn't actually require that you implemented support for
> > GICv3 migration on AArch32 hosts for these patches, I just didn't want
> > thigns to silently break.
> >
> > If we cannot test the AArch32 implementation, we should potentially just
> > make sure that is not supported yet, return a proper error to userspace
> > and get the AArch64 host implementation correct.
> >
> > I suggest you move your:
> > virt/kvm/arm/vgic/vgic-sys-reg-v3.c to
> > arch/arm64/kvm/vgic-sys-reg-v3.c
> >
> > and rename
> > virt/kvm/arm/vgic/vgic-sys-reg-common.c to
> > virt/kvm/arm/vgic/vgic-sys-reg-v3.c
> >
> > And then wait with the AArch32 host side for now, but just make sure it
> > compiles and returns an error as opposed to crashing the system if
> > someone tries to excercise this interface on an AArch32 host.
>
> I will add arch/arm/kvm/vgic-coproc-v3.c (pls check if file name is ok or not?)
I would call it vgic-v3-coproc.c
> and return -ENXIO as shown below and update document accordingly.
>
> int vgic_v3_has_cpu_sysregs_attr(struct kvm_vcpu *vcpu, bool is_write, u64 id,
> u64 *reg)
> {
> /*
> * TODO: Implement for AArch32
> */
> return -ENXIO;
> }
>
> int vgic_v3_cpu_sysregs_uaccess(struct kvm_vcpu *vcpu, bool is_write, u64 id,
> u64 *reg)
> {
> /*
> * TODO: Implement for AArch32
> */
> return -ENXIO;
> }
>
> >
> >> for both AArch64 and AArch32 mode. The vgic-sys-reg-v3.c handles AArch64
> >> mode and is compiled only for AArch64 mode.
> >>
> >> Updated arch/arm/include/uapi/asm/kvm.h with new definitions
> >> required to compile for AArch32.
> >>
> >> The version of VGIC v3 specification is define here
> >> Documentation/virtual/kvm/devices/arm-vgic-v3.txt
> >>
> >> Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
> >> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
> >> ---
> [...]
> >> +static bool access_gic_aprn(struct kvm_vcpu *vcpu, bool is_write, u8 apr,
> >> + u8 idx, unsigned long *reg)
> >> +{
> >> + struct vgic_cpu *vgic_v3_cpu = &vcpu->arch.vgic_cpu;
> >> +
> >> + /* num_pri_bits are initialized with HW supported values.
> >> + * We can rely safely on num_pri_bits even if VM has not
> >> + * restored ICC_CTLR_EL1 before restoring APnR registers.
> >> + */
> >
> > nit: commenting style
> ok
> >
> >> + switch (vgic_v3_cpu->num_pri_bits) {
> >> + case 7:
> >> + vgic_v3_access_apr_reg(vcpu, is_write, apr, idx, reg);
> >> + break;
> >> + case 6:
> >> + if (idx > 1)
> >> + goto err;
> >> + vgic_v3_access_apr_reg(vcpu, is_write, apr, idx, reg);
> >> + break;
> >> + default:
> >> + if (idx > 0)
> >> + goto err;
> >> + vgic_v3_access_apr_reg(vcpu, is_write, apr, idx, reg);
> >> + }
> >
> > It looks to me like userspace can then program active priorities with
> > higher numbers than what it will program num_pri_bits to later. Is that
> > not weird, or am I missing something?
>
> As long as it is within HW supported priorities it is safe.
I know that it is safe on the hardware, but it is weird to define a VM
with some max priority and still be able to set a higher active priority
is it not?
On the other hand, if we cannot enforce this at runtime, it may not
matter?
Hint: I'd like for you to actually think about these constraints and
make sure the sematics of the emulated VM environment remain intact
across migrations.
> >
> >> +
> >> + return true;
> >> +err:
> >> + if (!is_write)
> >> + *reg = 0;
> >> +
> >> + return false;
> >> +}
> >> +
> >> +bool access_gic_ap0r_reg(struct kvm_vcpu *vcpu, bool is_write, u8 idx,
> >> + unsigned long *reg)
> >> +{
> >> + return access_gic_aprn(vcpu, is_write, 0, idx, reg);
> >> +}
> >> +
> >> +bool access_gic_ap1r_reg(struct kvm_vcpu *vcpu, bool is_write, u8 idx,
> >> + unsigned long *reg)
> >> +{
> >> + return access_gic_aprn(vcpu, is_write, 1, idx, reg);
> >> +}
> >> +
> >> +bool access_gic_sre_reg(struct kvm_vcpu *vcpu, bool is_write,
> >> + unsigned long *reg)
> >> +{
> >> + struct vgic_v3_cpu_if *vgicv3 = &vcpu->arch.vgic_cpu.vgic_v3;
> >> +
> >> + /* Validate SRE bit */
> >> + if (is_write) {
> >> + if (!(*reg & ICC_SRE_EL1_SRE))
> >> + return false;
> >> + } else {
> >> + *reg = vgicv3->vgic_sre;
> >> + }
> >> +
> >> + return true;
> >> +}
> >> diff --git a/virt/kvm/arm/vgic/vgic-sys-reg-v3.c b/virt/kvm/arm/vgic/vgic-sys-reg-v3.c
> >> new file mode 100644
> >> index 0000000..82c2f02
> >> --- /dev/null
> >> +++ b/virt/kvm/arm/vgic/vgic-sys-reg-v3.c
> >> @@ -0,0 +1,142 @@
> >> +/*
> >> + * VGIC system registers handling functions
> >> + *
> >> + * This program is free software; you can redistribute it and/or modify
> >> + * it under the terms of the GNU General Public License version 2 as
> >> + * published by the Free Software Foundation.
> >> + *
> >> + * This program is distributed in the hope that it will be useful,
> >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> >> + * GNU General Public License for more details.
> >> + */
> >> +
> >> +#include <linux/kvm.h>
> >> +#include <linux/kvm_host.h>
> >> +#include <asm/kvm_emulate.h>
> >> +#include "vgic.h"
> >> +#include "sys_regs.h"
> >> +
> >> +#define ACCESS_SYS_REG(REG) \
> >> +static bool access_gic_##REG##_sys_reg(struct kvm_vcpu *vcpu, \
> >> + struct sys_reg_params *p, \
> >> + const struct sys_reg_desc *r) \
> >> +{ \
> >> + unsigned long tmp; \
> >> + bool ret; \
> >> + \
> >> + if (p->is_write) \
> >> + tmp = p->regval; \
> >> + ret = access_gic_##REG##_reg(vcpu, p->is_write, &tmp); \
> >> + if (!p->is_write) \
> >> + p->regval = tmp; \
> >> + \
> >> + return ret; \
> >> +}
> >> +
> >> +ACCESS_SYS_REG(ctlr)
> >> +ACCESS_SYS_REG(pmr)
> >> +ACCESS_SYS_REG(bpr0)
> >> +ACCESS_SYS_REG(bpr1)
> >> +ACCESS_SYS_REG(sre)
> >> +ACCESS_SYS_REG(grpen0)
> >> +ACCESS_SYS_REG(grpen1)
> >> +
> >> +#define ACCESS_APNR_SYS_REG(REG) \
> >> +static bool access_gic_##REG##_sys_reg(struct kvm_vcpu *vcpu, \
> >> + struct sys_reg_params *p, \
> >> + const struct sys_reg_desc *r) \
> >> +{ \
> >> + unsigned long tmp; \
> >> + u8 idx = p->Op2 & 3; \
> >> + bool ret; \
> >> + \
> >> + if (p->is_write) \
> >> + tmp = p->regval; \
> >> + ret = access_gic_##REG##_reg(vcpu, p->is_write, idx, &tmp); \
> >> + if (!p->is_write) \
> >> + p->regval = tmp; \
> >> + \
> >> + return ret; \
> >> +}
> >> +
> >> +ACCESS_APNR_SYS_REG(ap0r)
> >> +ACCESS_APNR_SYS_REG(ap1r)
> >
> > I don't get these indirections. Why can't you call the functions
> > directly?
>
> The code is same for accessing the registers hence added this indirection.
>
That's not answering my question.
What is the benefit of adding this indirection as opposed to having the
functions called directly?
To make my point clear: I hate this kind of preprocessor macro fun, and
I think it should only ever be used when there's a huge benefit in terms
of code reuse or simplicity of some sort. I don't see anything like
that in this case.
Thanks,
-Christoffer
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox