* [PATCH 3/3] drivers: iommu: io-pgtable-arm: use const and __initconst for iommu_gather_ops structures
From: Bhumika Goyal @ 2016-10-25 18:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477418772-12184-1-git-send-email-bhumirks@gmail.com>
Check for iommu_gather_ops structures that are only stored in the tlb
field of an io_pgtable_cfg structure. The tlb field is of type
const struct iommu_gather_ops *, so iommu_gather_ops structures
having this property can be declared as const. Also, replace __initdata
with __initconst.
Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>
---
drivers/iommu/io-pgtable-arm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
index f5c90e1..412e21d 100644
--- a/drivers/iommu/io-pgtable-arm.c
+++ b/drivers/iommu/io-pgtable-arm.c
@@ -916,7 +916,7 @@ static void dummy_tlb_sync(void *cookie)
WARN_ON(cookie != cfg_cookie);
}
-static struct iommu_gather_ops dummy_tlb_ops __initdata = {
+static const struct iommu_gather_ops dummy_tlb_ops __initconst = {
.tlb_flush_all = dummy_tlb_flush_all,
.tlb_add_flush = dummy_tlb_add_flush,
.tlb_sync = dummy_tlb_sync,
--
1.9.1
^ permalink raw reply related
* [PATCH 2/3] drivers: iommu: arm-smmu-v3: constify iommu_gather_ops structures
From: Bhumika Goyal @ 2016-10-25 18:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477418772-12184-1-git-send-email-bhumirks@gmail.com>
Check for iommu_gather_ops structures that are only stored in the tlb
field of an io_pgtable_cfg structure. The tlb field is of type
const struct iommu_gather_ops *, so iommu_gather_ops structures
having this property can be declared as const.
Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>
---
drivers/iommu/arm-smmu-v3.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 15c01c3..51d8be4 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -1358,7 +1358,7 @@ static void arm_smmu_tlb_inv_range_nosync(unsigned long iova, size_t size,
} while (size -= granule);
}
-static struct iommu_gather_ops arm_smmu_gather_ops = {
+static const struct iommu_gather_ops arm_smmu_gather_ops = {
.tlb_flush_all = arm_smmu_tlb_inv_context,
.tlb_add_flush = arm_smmu_tlb_inv_range_nosync,
.tlb_sync = arm_smmu_tlb_sync,
--
1.9.1
^ permalink raw reply related
* [PATCH 1/3] drivers: iommu: arm-smmu: constify iommu_gather_ops structures
From: Bhumika Goyal @ 2016-10-25 18:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477418772-12184-1-git-send-email-bhumirks@gmail.com>
Check for iommu_gather_ops structures that are only stored in the tlb
field of an io_pgtable_cfg structure. The tlb field is of type
const struct iommu_gather_ops *, so iommu_gather_ops structures
having this property can be declared as const.
Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>
---
drivers/iommu/arm-smmu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index c841eb7..a5bc8fb 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -640,7 +640,7 @@ static void arm_smmu_tlb_inv_range_nosync(unsigned long iova, size_t size,
}
}
-static struct iommu_gather_ops arm_smmu_gather_ops = {
+static const struct iommu_gather_ops arm_smmu_gather_ops = {
.tlb_flush_all = arm_smmu_tlb_inv_context,
.tlb_add_flush = arm_smmu_tlb_inv_range_nosync,
.tlb_sync = arm_smmu_tlb_sync,
--
1.9.1
^ permalink raw reply related
* [PATCH 0/3] drivers: iommu: declare iommu_gather_ops structures as const
From: Bhumika Goyal @ 2016-10-25 18:06 UTC (permalink / raw)
To: linux-arm-kernel
Constify iommu_gather_ops structures and replace __initdata with
__initconst where needed.
Bhumika Goyal (3):
drivers: iommu: arm-smmu: constify iommu_gather_ops structures
drivers: iommu: arm-smmu-v3: constify iommu_gather_ops structures
drivers: iommu: io-pgtable-arm: use const and __initconst for
iommu_gather_ops structures
drivers/iommu/arm-smmu-v3.c | 2 +-
drivers/iommu/arm-smmu.c | 2 +-
drivers/iommu/io-pgtable-arm.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
--
1.9.1
^ permalink raw reply
* [PATCH] arm64: Neaten show_regs, remove KERN_CONT
From: Joe Perches @ 2016-10-25 18:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CA+55aFw4EP_qxZ8tcRUV=PcLx_XbwXXOU_yp3cJHVjazof_0MA@mail.gmail.com>
On Tue, 2016-10-25 at 10:55 -0700, Linus Torvalds wrote:
> And yes, what we probably *should* do is to do the newline breaking
> when adding things to the log, rather than doing it in the
> "msg_print_text()" phase.
Yeah.
One thing that'd be nice one day is to remove all the
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
lines and have printk do that instead using a singleton
or a lookup for KBUILD_MODNAME as appropriate.
> There's a reason why I actually would have liked to entirely rewrite
> the whole printk mess.But there's also a reason I didn't - I'm not
> quite _that_ much of a glutton for punishment.
You sure?
^ permalink raw reply
* [PATCH] arm64: Neaten show_regs, remove KERN_CONT
From: Mark Rutland @ 2016-10-25 18:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CA+55aFwFQSass91gdGsjo4BkkAYmoFSn-Cpm6yF+SygF_tRp1g@mail.gmail.com>
On Tue, Oct 25, 2016 at 10:38:31AM -0700, Linus Torvalds wrote:
> On Mon, Oct 24, 2016 at 9:42 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> >
> > That does not appear to be the case; as fr as I can tell the core prints a
> > timestamp per line as required. If I run:
> >
> > printk("TEST\nLINE1\nLINE2\nLINE3\nLINE4\n");
>
> Please don't do this.
>
> It has historically not worked well, and it still doesn't actually
> work reliably. In particular, it currently works in the *logs* (ie
> dmesg), but not necessarily on screen (because "msg_print_text()" does
> do the "look for newlines in the middle", but console_cont_flush()
> does not).
Sure; I'll avoid that.
it seems that's a drop in the ocean, though. :/
[mark at leverpostej:~/src/linux]% git grep 'pr\(intk\|_.*\)(.*)' | grep '\\n[^"]' | wc -l
375
> So you can try the attached patch. It likely fixes your issues simply
> because it removes all the crazy code.
That worked for me. I see consistent results over the UART and in dmesg
with that applied atop of v4.9-rc2. Feel free to add:
Tested-by: Mark Rutland <mark.rutland@arm.com>
Thanks,
Mark.
^ permalink raw reply
* [RFC v2] ARM: memory: da8xx-ddrctl: new driver
From: Kevin Hilman @ 2016-10-25 17:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477402876-22472-2-git-send-email-bgolaszewski@baylibre.com>
Bartosz Golaszewski <bgolaszewski@baylibre.com> writes:
> Create a new driver for the da8xx DDR2/mDDR controller and implement
> support for writing to the Peripheral Bus Burst Priority Register.
>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
> .../memory-controllers/ti-da8xx-ddrctl.txt | 20 +++
> drivers/memory/Kconfig | 8 +
> drivers/memory/Makefile | 1 +
> drivers/memory/da8xx-ddrctl.c | 175 +++++++++++++++++++++
> 4 files changed, 204 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/memory-controllers/ti-da8xx-ddrctl.txt
> create mode 100644 drivers/memory/da8xx-ddrctl.c
>
> diff --git a/Documentation/devicetree/bindings/memory-controllers/ti-da8xx-ddrctl.txt b/Documentation/devicetree/bindings/memory-controllers/ti-da8xx-ddrctl.txt
> new file mode 100644
> index 0000000..7e271dd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/memory-controllers/ti-da8xx-ddrctl.txt
> @@ -0,0 +1,20 @@
> +* Device tree bindings for Texas Instruments da8xx DDR2/mDDR memory controller
> +
> +The DDR2/mDDR memory controller present on Texas Instruments da8xx SoCs features
> +a set of registers which allow to tweak the controller's behavior.
> +
> +Documentation:
> +OMAP-L138 (DA850) - http://www.ti.com/lit/ug/spruh82c/spruh82c.pdf
> +
> +Required properties:
> +
> +- compatible: "ti,da850-ddr-controller" - for da850 SoC based boards
> +- reg: a tuple containing the base address of the memory
> + controller and the size of the memory area to map
> +
> +Example for da850 shown below.
> +
> +ddrctl {
> + compatible = "ti,da850-ddr-controller";
> + reg = <0xB0000000 0x100>;
> +};
Axel's series for the USB PHY reminded me that the PHY also has some
config registers in this same area, and his series creates a syscon for
a similar range of registers.
Could you create a syscon for the SYSCFG0 registers, which would then
be used by ths driver and your other drivers/bus driver? Then the
binding would just reference the sysconf via phandle, and your driver
can use syscon_regmap_lookup_by_phandle()
> diff --git a/drivers/memory/Kconfig b/drivers/memory/Kconfig
> index 4b4c0c3..ec80e35 100644
> --- a/drivers/memory/Kconfig
> +++ b/drivers/memory/Kconfig
> @@ -134,6 +134,14 @@ config MTK_SMI
> mainly help enable/disable iommu and control the power domain and
> clocks for each local arbiter.
>
> +config DA8XX_DDRCTL
> + bool "Texas Instruments da8xx DDR2/mDDR driver"
> + depends on ARCH_DAVINCI_DA8XX
> + help
> + This driver is for the DDR2/mDDR Memory Controller present on
> + Texas Instruments da8xx SoCs. It's used to tweak various memory
> + controller configuration options.
> +
> source "drivers/memory/samsung/Kconfig"
> source "drivers/memory/tegra/Kconfig"
>
> diff --git a/drivers/memory/Makefile b/drivers/memory/Makefile
> index b20ae38..e88097fb 100644
> --- a/drivers/memory/Makefile
> +++ b/drivers/memory/Makefile
> @@ -17,6 +17,7 @@ obj-$(CONFIG_MVEBU_DEVBUS) += mvebu-devbus.o
> obj-$(CONFIG_TEGRA20_MC) += tegra20-mc.o
> obj-$(CONFIG_JZ4780_NEMC) += jz4780-nemc.o
> obj-$(CONFIG_MTK_SMI) += mtk-smi.o
> +obj-$(CONFIG_DA8XX_DDRCTL) += da8xx-ddrctl.o
>
> obj-$(CONFIG_SAMSUNG_MC) += samsung/
> obj-$(CONFIG_TEGRA_MC) += tegra/
> diff --git a/drivers/memory/da8xx-ddrctl.c b/drivers/memory/da8xx-ddrctl.c
> new file mode 100644
> index 0000000..66022df
> --- /dev/null
> +++ b/drivers/memory/da8xx-ddrctl.c
> @@ -0,0 +1,175 @@
> +/*
> + * TI da8xx DDR2/mDDR controller driver
> + *
> + * Copyright (C) 2016 BayLibre SAS
> + *
> + * Author:
> + * Bartosz Golaszewski <bgolaszewski@baylibre.com>
> + *
> + * 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.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_fdt.h>
> +#include <linux/platform_device.h>
> +#include <linux/io.h>
> +
> +/*
> + * REVISIT: Linux doesn't have a good framework for the kind of performance
> + * knobs this driver controls. We can't use device tree properties as it deals
> + * with hardware configuration rather than description. We also don't want to
> + * commit to maintaining some random sysfs attributes.
> + *
> + * For now we just hardcode the register values for the boards that need
> + * some changes (as is the case for the LCD controller on da850-lcdk - the
> + * first board we support here). When linux gets an appropriate framework,
> + * we'll easily convert the driver to it.
> + */
> +
> +struct da8xx_ddrctl_config_knob {
> + const char *name;
> + u32 reg;
> + u32 mask;
> + u32 offset;
nit: call this shift instead, which will also map well onto the regmap
accessors (which you'll use when switching to syscon.)
> +};
> +
> +static const struct da8xx_ddrctl_config_knob da8xx_ddrctl_knobs[] = {
> + {
> + .name = "da850-pbbpr",
> + .reg = 0x20,
> + .mask = 0xffffff00,
> + .offset = 0,
> + },
> +};
> +
> +struct da8xx_ddrctl_setting {
> + const char *name;
> + u32 val;
> +};
> +
> +struct da8xx_ddrctl_board_settings {
> + const char *board;
> + const struct da8xx_ddrctl_setting *settings;
> +};
> +
> +static const struct da8xx_ddrctl_setting da850_lcdk_ddrctl_settings[] = {
> + {
> + .name = "da850-pbbpr",
> + .val = 0x20,
> + },
> + { }
> +};
> +
> +static const struct da8xx_ddrctl_board_settings da8xx_ddrctl_board_confs[] = {
> + {
> + .board = "ti,da850-lcdk",
> + .settings = da850_lcdk_ddrctl_settings,
> + },
> +};
> +
> +static const struct da8xx_ddrctl_config_knob *
> +da8xx_ddrctl_match_knob(const struct da8xx_ddrctl_setting *setting)
> +{
> + const struct da8xx_ddrctl_config_knob *knob;
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(da8xx_ddrctl_knobs); i++) {
> + knob = &da8xx_ddrctl_knobs[i];
> +
> + if (strcmp(knob->name, setting->name) == 0)
> + return knob;
> + }
> +
> + return NULL;
> +}
> +
> +static const struct da8xx_ddrctl_setting *da8xx_ddrctl_get_board_settings(void)
> +{
> + const struct da8xx_ddrctl_board_settings *board_settings;
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(da8xx_ddrctl_board_confs); i++) {
> + board_settings = &da8xx_ddrctl_board_confs[0];
Looks like that '0' should be 'i' ?
> + if (of_machine_is_compatible(board_settings->board))
> + return board_settings->settings;
> + }
> +
> + return NULL;
> +}
> +
> +static int da8xx_ddrctl_probe(struct platform_device *pdev)
> +{
> + const struct da8xx_ddrctl_config_knob *knob;
> + const struct da8xx_ddrctl_setting *setting;
> + struct device_node *node;
> + struct resource *res;
> + void __iomem *ddrctl;
> + struct device *dev;
> + u32 reg;
> +
> + dev = &pdev->dev;
> + node = dev->of_node;
> +
> + setting = da8xx_ddrctl_get_board_settings();
> + if (!setting) {
> + dev_err(dev, "no settings for board '%s'\n",
> + of_flat_dt_get_machine_name());
> + return -EINVAL;
> + }
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + ddrctl = devm_ioremap_resource(dev, res);
> + if (IS_ERR(ddrctl)) {
> + dev_err(dev, "unable to map memory controller registers\n");
> + return PTR_ERR(ddrctl);
> + }
> +
> + for (; setting->name; setting++) {
> + knob = da8xx_ddrctl_match_knob(setting);
> + if (!knob) {
> + dev_warn(dev,
> + "no such config option: %s\n", setting->name);
> + continue;
> + }
> +
> + if (knob->reg > (res->end - res->start - sizeof(u32))) {
> + dev_warn(dev,
> + "register offset of '%s' exceeds mapped memory size\n",
> + knob->name);
> + continue;
> + }
> +
> + reg = __raw_readl(ddrctl + knob->reg);
> + reg &= knob->mask;
> + reg |= setting->val << knob->offset;
> +
> + dev_dbg(dev, "writing 0x%08x to %s\n", reg, setting->name);
> +
> + __raw_writel(reg, ddrctl + knob->reg);
nit: I don't think you need the __raw accessors here.
But in any case, moving to syscon you'll be using regmap accessors, and
this can just be converted to a single regmap_update_bits.
Kevin
> + }
> +
> + return 0;
> +}
> +
> +static const struct of_device_id da8xx_ddrctl_of_match[] = {
> + { .compatible = "ti,da850-ddr-controller", },
> + { },
> +};
> +
> +static struct platform_driver da8xx_ddrctl_driver = {
> + .probe = da8xx_ddrctl_probe,
> + .driver = {
> + .name = "da850-ddr-controller",
> + .of_match_table = da8xx_ddrctl_of_match,
> + },
> +};
> +module_platform_driver(da8xx_ddrctl_driver);
> +
> +MODULE_AUTHOR("Bartosz Golaszewski <bgolaszewski@baylibre.com>");
> +MODULE_DESCRIPTION("TI da8xx DDR2/mDDR controller driver");
> +MODULE_LICENSE("GPL v2");
^ permalink raw reply
* [PATCH] exynos-drm: Fix display manager failing to start without IOMMU problem
From: Tobias Jakobi @ 2016-10-25 17:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <fa4114e0-9382-0653-7dc6-a63f1312d616@osg.samsung.com>
Hello Shuah,
Shuah Khan wrote:
> On 10/19/2016 04:27 PM, Shuah Khan wrote:
>> On 10/19/2016 08:16 AM, Inki Dae wrote:
>>> Hi Shuah,
>>>
>>> 2016-10-13 8:11 GMT+09:00 Shuah Khan <shuahkh@osg.samsung.com>:
>>>> Hi Inki,
>>>>
>>>> On 08/15/2016 10:40 PM, Inki Dae wrote:
>>>>
>>>>>>
>>>>>> okay the very first commit that added IOMMU support
>>>>>> introduced the code that rejects non-contig gem memory
>>>>>> type without IOMMU.
>>>>>>
>>>>>> commit 0519f9a12d0113caab78980c48a7902d2bd40c2c
>>>>>> Author: Inki Dae <inki.dae@samsung.com>
>>>>>> Date: Sat Oct 20 07:53:42 2012 -0700
>>>>>>
>>>>>> drm/exynos: add iommu support for exynos drm framework
>>>>>>
>>>>
>>>> I haven't given up on this yet. I am still seeing the following failure:
>>>>
>>>> Additional debug messages I added:
>>>> [ 15.287403] exynos_drm_gem_create_ioctl() 1
>>>> [ 15.287419] exynos_drm_gem_create() flags 1
>>>>
>>>> [ 15.311511] [drm:exynos_drm_framebuffer_init] *ERROR* Non-contiguous GEM memory is not supported.
>>>>
>>>> Additional debug message I added:
>>>> [ 15.318981] [drm:exynos_user_fb_create] *ERROR* failed to initialize framebuffer
>>>>
>>>> This is what happens:
>>>>
>>>> 1. exynos_drm_gem_create_ioctl() gets called with EXYNOS_BO_NONCONTIG request
>>>> 2. exynos_drm_gem_create(0 goes ahead and creates the GEM buffers
>>>> 3. exynos_user_fb_create() tries to associate GEM to fb and fails during
>>>> check_fb_gem_memory_type()
>>>>
>>>> At this point, there is no recovery and lightdm fails
>>>>
>>>> xf86-video-armsoc/src/drmmode_exynos/drmmode_exynos.c assumes contiguous
>>>> allocations are not supported in some exynos drm versions: The following
>>>> commit introduced this change:
>>>>
>>>> https://git.linaro.org/arm/xorg/driver/xf86-video-armsoc.git/commitdiff/3be1f6273441fe95dd442f44064387322e16b7e9
>>>>
>>>> excerpts from the diff:- if (create_gem->buf_type == ARMSOC_BO_SCANOUT)
>>>> - create_exynos.flags = EXYNOS_BO_CONTIG;
>>>> - else
>>>> - create_exynos.flags = EXYNOS_BO_NONCONTIG;
>>>> +
>>>> + /* Contiguous allocations are not supported in some exynos drm versions.
>>>> + * When they are supported all allocations are effectively contiguous
>>>> + * anyway, so for simplicity we always request non contiguous buffers.
>>>> + */
>>>> + create_exynos.flags = EXYNOS_BO_NONCONTIG;
>>>>
>>>
>>> Above comment, "Contiguous allocations are not supported in some
>>> exynos drm versions.", seems wrong assumption.
>>> The root cause, contiguous allocation is not supported, would be that
>>> they used Linux kernel which didn't have CMA region enough - as
>>> default 16MB, or didn't declare CMA region enough for the DMA device.
>>> So I think they should not force to flag EXYNOS_BO_NONCONTIG and they
>>> should manage the error case if the allocation failed.
>>
>> This assumption doesn't sound correct and forcing NONCONTIG isn't right
>> either.
>>
>>>
>>>> There might have been logic on exynos_drm that forced Contig when it coudn't
>>>> support NONCONTIG. At least, that is what this comment suggests. This assumption
>>>> doesn't appear to be a good one and not sure if this change was made to fix a bug.
>>>>
>>>> After the IOMMU support, this assumption is no longer true. Hence, with IOMMU
>>>> support, latest kernels have a mismatch with the installed xf86-video-armsoc
>>>>
>>>> This is what I am running into. This leads to the following question:
>>>>
>>>> 1. How do we ensure exynos_drm kernel changes don't break user-space
>>>> specifically xf86-video-armsoc
>>>> 2. This seems to have gone undetected for a while. I see a change in
>>>> exynos_drm_gem_dumb_create() that is probably addressing this type
>>>> of breakage. Commit 122beea84bb90236b1ae545f08267af58591c21b adds
>>>> handling for IOMMU NONCONTIG case.
>>>
>>> Seems this patch has a problem. This patch forces to flag NONCONTIG if
>>> iommu is enabled. The flag should be depend on the argument from
>>> user-space.
>>> I.e., if user-space requested a gem allocation with CONTIG flag, then
>>> Exynos drm driver should allocate contiguous memory even though iommu
>>> is enabled.
>>>
>>>>
>>>> Anyway, I am interested in getting the exynos_drm kernel side code
>>>> and xf86-video-armsoc in sync to resolve the issue.
>>>>
>>>> Could you recommend a going forward plan?
>>>
>>> My opinion are,
>>>
>>> 1. Do not force to flag EXYNOS_BO_NONCONTIG at xf86-video-armsoc
>
> Okay more on this. I fixed xf86-video-armso to ask for EXYNOS_BO_CONTIG
> for ARMSOC_BO_SCANOUT and EXYNOS_BO_NONCONTIG in all other cases.
>
> Revert of the commit: 3be1f6273441fe95dd442f44064387322e16b7e9
>
> With this change, now display manager starts just fine. However, it turns
> out xf86-video-armsoc is obsoleted in favor of xf86-video-modesetting. The
> last update to xf86-video-armsoc git was 3 years ago.
IIRC xf86-video-armsoc was created to facilitate integration with the
proprietary Mali userspace blob. I don't think that can be done with the
modesetting DDX.
> I am not sure where to send the fix and doesn't look like it is being
> maintained actively. I can pursue it further and try to get this into
> xf86-video-armsoc provided I can find the maintainer for this seemingly
> inactive project.
I was wondering if anyone has actually complained about this issue?
> I brought in the latest xf86-video-modesetting bits from
>
> https://cgit.freedesktop.org/xorg/driver/xf86-video-modesetting
>
> I removed xf86-video-armsoc and installed xf86-video-modesetting and that
> worked just fine. xf86-video-modesetting uses dumb_create interface instead
> of DRM_IOCTL_EXYNOS_GEM_CREATE.
>
> dumb_create interface eliminates the need for DRM_IOCTL_EXYNOS_GEM_CREATE
> in userspace. libdrm-2.4.71 does call DRM_IOCTL_EXYNOS_GEM_CREATE.
>
> The question is do we still need to continue to support the custom gem
> create interface DRM_IOCTL_EXYNOS_GEM_CREATE?
I'd say yes. With just the dumb_create interface it is not possible to
change the type of buffer you get. And if you want to support any kind
of hw acceleration in the DDX, you probably want to control at least the
caching behaviour of the buffer.
> Some drm drivers seem to support it and some don't.
Not sure I understand this. I don't think any other DRM driver except
for exynos supports this ioctl. But all drivers have their own ioctls to
request memory (DRM_IOCTL_ETNAVIV_GEM_NEW, DRM_IOCTL_VC4_CREATE_BO, etc.)
> Unfortunately, if userspace requests noncontig
> for scanout, we will continue to see problems with display manager when
> iommu is disabled. dumb create hides all of that, are there good reasons
> to continue to support it in exynos drm?
In addition to the stuff I said above, you can use it for render nodes.
That doesn't work for dumb_create.
> Exposing CONTIG and NONCONTIG to userspace appears to be causing problems
> when exynos drm determines it can't support non-contig GEM buffers in fb
> init after userspace allocates them.
Might be missing something here, but this whole thing just looks like a
bug in xf86-video-armsoc. No matter from which perspective you look, the
commit that changed all allocations to noncontig was plain wrong.
My guess: This was done to paper over some bug in some vendor kernel for
a product using an Exynos SoC.
With best wishes,
Tobias
>
>>> 2. Do not force to flag NONCONTIG at Exynos drm driver even though
>>> iommu is enabled and flag allocation type with the argument from
>>> user-space.
>>>
>
> exynos_drm_gem_dumb_create() doesn't take any flags, there is no need
> to change the above.
>
> thanks,
> -- Shuah
>
^ permalink raw reply
* [PATCH] arm64: Neaten show_regs, remove KERN_CONT
From: Linus Torvalds @ 2016-10-25 17:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CA+55aFwFQSass91gdGsjo4BkkAYmoFSn-Cpm6yF+SygF_tRp1g@mail.gmail.com>
On Tue, Oct 25, 2016 at 10:38 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Mon, Oct 24, 2016 at 9:42 AM, Mark Rutland <mark.rutland@arm.com> wrote:
>>
>> That does not appear to be the case; as fr as I can tell the core prints a
>> timestamp per line as required. If I run:
>>
>> printk("TEST\nLINE1\nLINE2\nLINE3\nLINE4\n");
>
> Please don't do this.
Side note: even with my patch, the above kind of stuff hits other special cases.
In particular, the "extended header" that is printed to special
consoles (really just the network console) will only print one header
for each record. So you'll end up with the "normal" logs having time
appended to each line, but the extended logs that use
"msg_print_ext_body()" will have just a header for each record, and
then within the record the newlines will be escaped as "\0a", I think.
We could fix that too, but basically newlines in the middle of a
string has never really worked reliably. I think historically (long
long ago) they were just printed as-is, and did not have the loglevel
or the timestamp, for example. Now those should work for normal
logging, but there clearly are still cases where it breaks down.
Of course, you probably could argue that nobody cares too deeply about
the exact format of the extended logging, and you'd probably be right.
But still..
And yes, what we probably *should* do is to do the newline breaking
when adding things to the log, rather than doing it in the
"msg_print_text()" phase.
There's a reason why I actually would have liked to entirely rewrite
the whole printk mess. But there's also a reason I didn't - I'm not
quite _that_ much of a glutton for punishment.
Linus
^ permalink raw reply
* [PATCH] ARM: davinci: da850: Fix pwm name matching
From: David Lechner @ 2016-10-25 17:54 UTC (permalink / raw)
To: linux-arm-kernel
This fixes pwm name matching for DA850 familiy devices. When using device
tree, the da850_auxdata_lookup[] table caused pwm devices to have the exact
same name, which caused errors when trying to register the devices.
The names for clock matching in da850_clks[] also have to be updated to
to exactly match in order for the clock lookup to work correctly.
Signed-off-by: David Lechner <david@lechnology.com>
---
Tested working on LEGO MINDSTORMS EV3.
arch/arm/mach-davinci/da850.c | 10 +++++++---
arch/arm/mach-davinci/da8xx-dt.c | 10 +++++-----
2 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
index ed3d0e9..6b78a8f 100644
--- a/arch/arm/mach-davinci/da850.c
+++ b/arch/arm/mach-davinci/da850.c
@@ -510,9 +510,13 @@ static struct clk_lookup da850_clks[] = {
CLK("vpif", NULL, &vpif_clk),
CLK("ahci_da850", NULL, &sata_clk),
CLK("davinci-rproc.0", NULL, &dsp_clk),
- CLK("ehrpwm", "fck", &ehrpwm_clk),
- CLK("ehrpwm", "tbclk", &ehrpwm_tbclk),
- CLK("ecap", "fck", &ecap_clk),
+ CLK("ehrpwm.0", "fck", &ehrpwm_clk),
+ CLK("ehrpwm.0", "tbclk", &ehrpwm_tbclk),
+ CLK("ehrpwm.1", "fck", &ehrpwm_clk),
+ CLK("ehrpwm.1", "tbclk", &ehrpwm_tbclk),
+ CLK("ecap.0", "fck", &ecap_clk),
+ CLK("ecap.1", "fck", &ecap_clk),
+ CLK("ecap.2", "fck", &ecap_clk),
CLK(NULL, NULL, NULL),
};
diff --git a/arch/arm/mach-davinci/da8xx-dt.c b/arch/arm/mach-davinci/da8xx-dt.c
index 7947267..6ad8ddb 100644
--- a/arch/arm/mach-davinci/da8xx-dt.c
+++ b/arch/arm/mach-davinci/da8xx-dt.c
@@ -23,11 +23,11 @@ static struct of_dev_auxdata da850_auxdata_lookup[] __initdata = {
OF_DEV_AUXDATA("ti,davinci-i2c", 0x01e28000, "i2c_davinci.2", NULL),
OF_DEV_AUXDATA("ti,davinci-wdt", 0x01c21000, "davinci-wdt", NULL),
OF_DEV_AUXDATA("ti,da830-mmc", 0x01c40000, "da830-mmc.0", NULL),
- OF_DEV_AUXDATA("ti,da850-ehrpwm", 0x01f00000, "ehrpwm", NULL),
- OF_DEV_AUXDATA("ti,da850-ehrpwm", 0x01f02000, "ehrpwm", NULL),
- OF_DEV_AUXDATA("ti,da850-ecap", 0x01f06000, "ecap", NULL),
- OF_DEV_AUXDATA("ti,da850-ecap", 0x01f07000, "ecap", NULL),
- OF_DEV_AUXDATA("ti,da850-ecap", 0x01f08000, "ecap", NULL),
+ OF_DEV_AUXDATA("ti,da850-ehrpwm", 0x01f00000, "ehrpwm.0", NULL),
+ OF_DEV_AUXDATA("ti,da850-ehrpwm", 0x01f02000, "ehrpwm.1", NULL),
+ OF_DEV_AUXDATA("ti,da850-ecap", 0x01f06000, "ecap.0", NULL),
+ OF_DEV_AUXDATA("ti,da850-ecap", 0x01f07000, "ecap.1", NULL),
+ OF_DEV_AUXDATA("ti,da850-ecap", 0x01f08000, "ecap.2", NULL),
OF_DEV_AUXDATA("ti,da830-spi", 0x01c41000, "spi_davinci.0", NULL),
OF_DEV_AUXDATA("ti,da830-spi", 0x01f0e000, "spi_davinci.1", NULL),
OF_DEV_AUXDATA("ns16550a", 0x01c42000, "serial8250.0", NULL),
--
2.7.4
^ permalink raw reply related
* arm/boot/dts/at91sam9260.dtsi USB clock divisors and AT91SAM9G20
From: Andrey Yurovsky @ 2016-10-25 17:49 UTC (permalink / raw)
To: linux-arm-kernel
I'm working at an AT91SAM9G20 on which the USB clock divisor was wrong
and traced the issue down to arm/boot/dts/at91sam9260.dtsi which is
included like this:
at91sam9g20ek_common.dtsi
at91sam9g20.dtsi
at91sam9260.dtsi
This has a node:
usb: usbck {
compatible = "atmel,at91rm9200-clk-usb";
#clock-cells = <0>;
atmel,clk-divisors = <1 2 4 0>;
clocks = <&pllb>;
};
However the G20 parts seem to need a different divisor, for me this works:
usb: usbck {
compatible = "atmel,at91rm9200-clk-usb";
#clock-cells = <0>;
atmel,clk-divisors = <4 2 0 0>;
clocks = <&pllb>;
};
Should the USB clock node be moved to a separate .dtsi file to account
for this difference?
^ permalink raw reply
* [PATCH] arm64: Neaten show_regs, remove KERN_CONT
From: Linus Torvalds @ 2016-10-25 17:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161024164255.GN15620@leverpostej>
On Mon, Oct 24, 2016 at 9:42 AM, Mark Rutland <mark.rutland@arm.com> wrote:
>
> That does not appear to be the case; as fr as I can tell the core prints a
> timestamp per line as required. If I run:
>
> printk("TEST\nLINE1\nLINE2\nLINE3\nLINE4\n");
Please don't do this.
It has historically not worked well, and it still doesn't actually
work reliably. In particular, it currently works in the *logs* (ie
dmesg), but not necessarily on screen (because "msg_print_text()" does
do the "look for newlines in the middle", but console_cont_flush()
does not).
It so happens that the patch I've been sending people probably fixes
that odd case too, almost entirely by mistake (if "by mistake" you
mean "it gets rid of the insane special cases that cause problems like
this").
So you can try the attached patch. It likely fixes your issues simply
because it removes all the crazy code.
Linus
-------------- next part --------------
kernel/printk/printk.c | 255 +++++++++++--------------------------------------
1 file changed, 58 insertions(+), 197 deletions(-)
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index de08fc90baaf..e63aa679614e 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -367,7 +367,6 @@ DECLARE_WAIT_QUEUE_HEAD(log_wait);
/* the next printk record to read by syslog(READ) or /proc/kmsg */
static u64 syslog_seq;
static u32 syslog_idx;
-static enum log_flags syslog_prev;
static size_t syslog_partial;
/* index and sequence number of the first record stored in the buffer */
@@ -381,7 +380,6 @@ static u32 log_next_idx;
/* the next printk record to write to the console */
static u64 console_seq;
static u32 console_idx;
-static enum log_flags console_prev;
/* the next printk record to read after the last 'clear' command */
static u64 clear_seq;
@@ -650,27 +648,15 @@ static void append_char(char **pp, char *e, char c)
}
static ssize_t msg_print_ext_header(char *buf, size_t size,
- struct printk_log *msg, u64 seq,
- enum log_flags prev_flags)
+ struct printk_log *msg, u64 seq)
{
u64 ts_usec = msg->ts_nsec;
- char cont = '-';
do_div(ts_usec, 1000);
- /*
- * If we couldn't merge continuation line fragments during the print,
- * export the stored flags to allow an optional external merge of the
- * records. Merging the records isn't always neccessarily correct, like
- * when we hit a race during printing. In most cases though, it produces
- * better readable output. 'c' in the record flags mark the first
- * fragment of a line, '+' the following.
- */
- if (msg->flags & LOG_CONT)
- cont = (prev_flags & LOG_CONT) ? '+' : 'c';
-
return scnprintf(buf, size, "%u,%llu,%llu,%c;",
- (msg->facility << 3) | msg->level, seq, ts_usec, cont);
+ (msg->facility << 3) | msg->level, seq, ts_usec,
+ msg->flags & LOG_CONT ? 'c' : '-');
}
static ssize_t msg_print_ext_body(char *buf, size_t size,
@@ -725,7 +711,6 @@ static ssize_t msg_print_ext_body(char *buf, size_t size,
struct devkmsg_user {
u64 seq;
u32 idx;
- enum log_flags prev;
struct ratelimit_state rs;
struct mutex lock;
char buf[CONSOLE_EXT_LOG_MAX];
@@ -794,7 +779,7 @@ static ssize_t devkmsg_write(struct kiocb *iocb, struct iov_iter *from)
return ret;
}
-static void cont_flush(void);
+static void deferred_cont_flush(void);
static ssize_t devkmsg_read(struct file *file, char __user *buf,
size_t count, loff_t *ppos)
@@ -811,7 +796,6 @@ static ssize_t devkmsg_read(struct file *file, char __user *buf,
if (ret)
return ret;
raw_spin_lock_irq(&logbuf_lock);
- cont_flush();
while (user->seq == log_next_seq) {
if (file->f_flags & O_NONBLOCK) {
ret = -EAGAIN;
@@ -838,12 +822,11 @@ static ssize_t devkmsg_read(struct file *file, char __user *buf,
msg = log_from_idx(user->idx);
len = msg_print_ext_header(user->buf, sizeof(user->buf),
- msg, user->seq, user->prev);
+ msg, user->seq);
len += msg_print_ext_body(user->buf + len, sizeof(user->buf) - len,
log_dict(msg), msg->dict_len,
log_text(msg), msg->text_len);
- user->prev = msg->flags;
user->idx = log_next(user->idx);
user->seq++;
raw_spin_unlock_irq(&logbuf_lock);
@@ -860,6 +843,7 @@ static ssize_t devkmsg_read(struct file *file, char __user *buf,
ret = len;
out:
mutex_unlock(&user->lock);
+ deferred_cont_flush();
return ret;
}
@@ -874,7 +858,6 @@ static loff_t devkmsg_llseek(struct file *file, loff_t offset, int whence)
return -ESPIPE;
raw_spin_lock_irq(&logbuf_lock);
- cont_flush();
switch (whence) {
case SEEK_SET:
/* the first record */
@@ -913,7 +896,6 @@ static unsigned int devkmsg_poll(struct file *file, poll_table *wait)
poll_wait(file, &log_wait, wait);
raw_spin_lock_irq(&logbuf_lock);
- cont_flush();
if (user->seq < log_next_seq) {
/* return error when data has vanished underneath us */
if (user->seq < log_first_seq)
@@ -922,6 +904,7 @@ static unsigned int devkmsg_poll(struct file *file, poll_table *wait)
ret = POLLIN|POLLRDNORM;
}
raw_spin_unlock_irq(&logbuf_lock);
+ deferred_cont_flush();
return ret;
}
@@ -1226,26 +1209,12 @@ static size_t print_prefix(const struct printk_log *msg, bool syslog, char *buf)
return len;
}
-static size_t msg_print_text(const struct printk_log *msg, enum log_flags prev,
- bool syslog, char *buf, size_t size)
+static size_t msg_print_text(const struct printk_log *msg, bool syslog, char *buf, size_t size)
{
const char *text = log_text(msg);
size_t text_size = msg->text_len;
- bool prefix = true;
- bool newline = true;
size_t len = 0;
- if ((prev & LOG_CONT) && !(msg->flags & LOG_PREFIX))
- prefix = false;
-
- if (msg->flags & LOG_CONT) {
- if ((prev & LOG_CONT) && !(prev & LOG_NEWLINE))
- prefix = false;
-
- if (!(msg->flags & LOG_NEWLINE))
- newline = false;
- }
-
do {
const char *next = memchr(text, '\n', text_size);
size_t text_len;
@@ -1263,22 +1232,17 @@ static size_t msg_print_text(const struct printk_log *msg, enum log_flags prev,
text_len + 1 >= size - len)
break;
- if (prefix)
- len += print_prefix(msg, syslog, buf + len);
+ len += print_prefix(msg, syslog, buf + len);
memcpy(buf + len, text, text_len);
len += text_len;
- if (next || newline)
- buf[len++] = '\n';
+ buf[len++] = '\n';
} else {
/* SYSLOG_ACTION_* buffer size only calculation */
- if (prefix)
- len += print_prefix(msg, syslog, NULL);
+ len += print_prefix(msg, syslog, NULL);
len += text_len;
- if (next || newline)
- len++;
+ len++;
}
- prefix = true;
text = next;
} while (text);
@@ -1300,12 +1264,10 @@ static int syslog_print(char __user *buf, int size)
size_t skip;
raw_spin_lock_irq(&logbuf_lock);
- cont_flush();
if (syslog_seq < log_first_seq) {
/* messages are gone, move to first one */
syslog_seq = log_first_seq;
syslog_idx = log_first_idx;
- syslog_prev = 0;
syslog_partial = 0;
}
if (syslog_seq == log_next_seq) {
@@ -1315,13 +1277,11 @@ static int syslog_print(char __user *buf, int size)
skip = syslog_partial;
msg = log_from_idx(syslog_idx);
- n = msg_print_text(msg, syslog_prev, true, text,
- LOG_LINE_MAX + PREFIX_MAX);
+ n = msg_print_text(msg, true, text, LOG_LINE_MAX + PREFIX_MAX);
if (n - syslog_partial <= size) {
/* message fits into buffer, move forward */
syslog_idx = log_next(syslog_idx);
syslog_seq++;
- syslog_prev = msg->flags;
n -= syslog_partial;
syslog_partial = 0;
} else if (!len){
@@ -1360,12 +1320,10 @@ static int syslog_print_all(char __user *buf, int size, bool clear)
return -ENOMEM;
raw_spin_lock_irq(&logbuf_lock);
- cont_flush();
if (buf) {
u64 next_seq;
u64 seq;
u32 idx;
- enum log_flags prev;
/*
* Find first record that fits, including all following records,
@@ -1373,12 +1331,10 @@ static int syslog_print_all(char __user *buf, int size, bool clear)
*/
seq = clear_seq;
idx = clear_idx;
- prev = 0;
while (seq < log_next_seq) {
struct printk_log *msg = log_from_idx(idx);
- len += msg_print_text(msg, prev, true, NULL, 0);
- prev = msg->flags;
+ len += msg_print_text(msg, true, NULL, 0);
idx = log_next(idx);
seq++;
}
@@ -1386,12 +1342,10 @@ static int syslog_print_all(char __user *buf, int size, bool clear)
/* move first record forward until length fits into the buffer */
seq = clear_seq;
idx = clear_idx;
- prev = 0;
while (len > size && seq < log_next_seq) {
struct printk_log *msg = log_from_idx(idx);
- len -= msg_print_text(msg, prev, true, NULL, 0);
- prev = msg->flags;
+ len -= msg_print_text(msg, true, NULL, 0);
idx = log_next(idx);
seq++;
}
@@ -1404,7 +1358,7 @@ static int syslog_print_all(char __user *buf, int size, bool clear)
struct printk_log *msg = log_from_idx(idx);
int textlen;
- textlen = msg_print_text(msg, prev, true, text,
+ textlen = msg_print_text(msg, true, text,
LOG_LINE_MAX + PREFIX_MAX);
if (textlen < 0) {
len = textlen;
@@ -1412,7 +1366,6 @@ static int syslog_print_all(char __user *buf, int size, bool clear)
}
idx = log_next(idx);
seq++;
- prev = msg->flags;
raw_spin_unlock_irq(&logbuf_lock);
if (copy_to_user(buf + len, text, textlen))
@@ -1425,7 +1378,6 @@ static int syslog_print_all(char __user *buf, int size, bool clear)
/* messages are gone, move to next one */
seq = log_first_seq;
idx = log_first_idx;
- prev = 0;
}
}
}
@@ -1522,12 +1474,10 @@ int do_syslog(int type, char __user *buf, int len, int source)
/* Number of chars in the log buffer */
case SYSLOG_ACTION_SIZE_UNREAD:
raw_spin_lock_irq(&logbuf_lock);
- cont_flush();
if (syslog_seq < log_first_seq) {
/* messages are gone, move to first one */
syslog_seq = log_first_seq;
syslog_idx = log_first_idx;
- syslog_prev = 0;
syslog_partial = 0;
}
if (source == SYSLOG_FROM_PROC) {
@@ -1540,16 +1490,14 @@ int do_syslog(int type, char __user *buf, int len, int source)
} else {
u64 seq = syslog_seq;
u32 idx = syslog_idx;
- enum log_flags prev = syslog_prev;
error = 0;
while (seq < log_next_seq) {
struct printk_log *msg = log_from_idx(idx);
- error += msg_print_text(msg, prev, true, NULL, 0);
+ error += msg_print_text(msg, true, NULL, 0);
idx = log_next(idx);
seq++;
- prev = msg->flags;
}
error -= syslog_partial;
}
@@ -1563,6 +1511,7 @@ int do_syslog(int type, char __user *buf, int len, int source)
error = -EINVAL;
break;
}
+ deferred_cont_flush();
out:
return error;
}
@@ -1650,46 +1599,47 @@ static inline void printk_delay(void)
static struct cont {
char buf[LOG_LINE_MAX];
size_t len; /* length == 0 means unused buffer */
- size_t cons; /* bytes written to console */
struct task_struct *owner; /* task of first print*/
u64 ts_nsec; /* time of first print */
u8 level; /* log level of first message */
u8 facility; /* log facility of first message */
enum log_flags flags; /* prefix, newline flags */
- bool flushed:1; /* buffer sealed and committed */
} cont;
-static void cont_flush(void)
+static bool cont_flush(void)
{
- if (cont.flushed)
- return;
- if (cont.len == 0)
+ if (!cont.len)
+ return false;
+
+ log_store(cont.facility, cont.level, cont.flags, cont.ts_nsec,
+ NULL, 0, cont.buf, cont.len);
+ cont.len = 0;
+ return true;
+}
+
+static void flush_timer(unsigned long data)
+{
+ unsigned long flags;
+ bool did_flush;
+
+ raw_spin_lock_irqsave(&logbuf_lock, flags);
+ did_flush = cont_flush();
+ raw_spin_unlock_irqrestore(&logbuf_lock, flags);
+ if (did_flush)
+ wake_up_klogd();
+}
+
+static void deferred_cont_flush(void)
+{
+ static DEFINE_TIMER(timer, flush_timer, 0, 0);
+
+ if (!cont.len)
return;
- if (cont.cons) {
- /*
- * If a fragment of this line was directly flushed to the
- * console; wait for the console to pick up the rest of the
- * line. LOG_NOCONS suppresses a duplicated output.
- */
- log_store(cont.facility, cont.level, cont.flags | LOG_NOCONS,
- cont.ts_nsec, NULL, 0, cont.buf, cont.len);
- cont.flushed = true;
- } else {
- /*
- * If no fragment of this line ever reached the console,
- * just submit it to the store and free the buffer.
- */
- log_store(cont.facility, cont.level, cont.flags, 0,
- NULL, 0, cont.buf, cont.len);
- cont.len = 0;
- }
+ mod_timer(&timer, jiffies + HZ/10);
}
static bool cont_add(int facility, int level, enum log_flags flags, const char *text, size_t len)
{
- if (cont.len && cont.flushed)
- return false;
-
/*
* If ext consoles are present, flush and skip in-kernel
* continuation. See nr_ext_console_drivers definition. Also, if
@@ -1706,8 +1656,6 @@ static bool cont_add(int facility, int level, enum log_flags flags, const char *
cont.owner = current;
cont.ts_nsec = local_clock();
cont.flags = flags;
- cont.cons = 0;
- cont.flushed = false;
}
memcpy(cont.buf + cont.len, text, len);
@@ -1726,34 +1674,6 @@ static bool cont_add(int facility, int level, enum log_flags flags, const char *
return true;
}
-static size_t cont_print_text(char *text, size_t size)
-{
- size_t textlen = 0;
- size_t len;
-
- if (cont.cons == 0 && (console_prev & LOG_NEWLINE)) {
- textlen += print_time(cont.ts_nsec, text);
- size -= textlen;
- }
-
- len = cont.len - cont.cons;
- if (len > 0) {
- if (len+1 > size)
- len = size-1;
- memcpy(text + textlen, cont.buf + cont.cons, len);
- textlen += len;
- cont.cons = cont.len;
- }
-
- if (cont.flushed) {
- if (cont.flags & LOG_NEWLINE)
- text[textlen++] = '\n';
- /* got everything, release buffer */
- cont.len = 0;
- }
- return textlen;
-}
-
static size_t log_output(int facility, int level, enum log_flags lflags, const char *dict, size_t dictlen, char *text, size_t text_len)
{
/*
@@ -1999,11 +1919,9 @@ static u64 syslog_seq;
static u32 syslog_idx;
static u64 console_seq;
static u32 console_idx;
-static enum log_flags syslog_prev;
static u64 log_first_seq;
static u32 log_first_idx;
static u64 log_next_seq;
-static enum log_flags console_prev;
static struct cont {
size_t len;
size_t cons;
@@ -2015,17 +1933,16 @@ static char *log_dict(const struct printk_log *msg) { return NULL; }
static struct printk_log *log_from_idx(u32 idx) { return NULL; }
static u32 log_next(u32 idx) { return 0; }
static ssize_t msg_print_ext_header(char *buf, size_t size,
- struct printk_log *msg, u64 seq,
- enum log_flags prev_flags) { return 0; }
+ struct printk_log *msg,
+ u64 seq) { return 0; }
static ssize_t msg_print_ext_body(char *buf, size_t size,
char *dict, size_t dict_len,
char *text, size_t text_len) { return 0; }
static void call_console_drivers(int level,
const char *ext_text, size_t ext_len,
const char *text, size_t len) {}
-static size_t msg_print_text(const struct printk_log *msg, enum log_flags prev,
+static size_t msg_print_text(const struct printk_log *msg,
bool syslog, char *buf, size_t size) { return 0; }
-static size_t cont_print_text(char *text, size_t size) { return 0; }
static bool suppress_message_printing(int level) { return false; }
/* Still needs to be defined for users */
@@ -2296,42 +2213,6 @@ static inline int can_use_console(void)
return cpu_online(raw_smp_processor_id()) || have_callable_console();
}
-static void console_cont_flush(char *text, size_t size)
-{
- unsigned long flags;
- size_t len;
-
- raw_spin_lock_irqsave(&logbuf_lock, flags);
-
- if (!cont.len)
- goto out;
-
- if (suppress_message_printing(cont.level)) {
- cont.cons = cont.len;
- if (cont.flushed)
- cont.len = 0;
- goto out;
- }
-
- /*
- * We still queue earlier records, likely because the console was
- * busy. The earlier ones need to be printed before this one, we
- * did not flush any fragment so far, so just let it queue up.
- */
- if (console_seq < log_next_seq && !cont.cons)
- goto out;
-
- len = cont_print_text(text, size);
- raw_spin_unlock(&logbuf_lock);
- stop_critical_timings();
- call_console_drivers(cont.level, NULL, 0, text, len);
- start_critical_timings();
- local_irq_restore(flags);
- return;
-out:
- raw_spin_unlock_irqrestore(&logbuf_lock, flags);
-}
-
/**
* console_unlock - unlock the console system
*
@@ -2385,9 +2266,6 @@ void console_unlock(void)
return;
}
- /* flush buffered message fragment immediately to console */
- console_cont_flush(text, sizeof(text));
-
for (;;) {
struct printk_log *msg;
size_t ext_len = 0;
@@ -2407,7 +2285,6 @@ void console_unlock(void)
/* messages are gone, move to first one */
console_seq = log_first_seq;
console_idx = log_first_idx;
- console_prev = 0;
} else {
len = 0;
}
@@ -2417,8 +2294,7 @@ void console_unlock(void)
msg = log_from_idx(console_idx);
level = msg->level;
- if ((msg->flags & LOG_NOCONS) ||
- suppress_message_printing(level)) {
+ if (suppress_message_printing(level)) {
/*
* Skip record we have buffered and already printed
* directly to the console when we received it, and
@@ -2426,22 +2302,14 @@ void console_unlock(void)
*/
console_idx = log_next(console_idx);
console_seq++;
- /*
- * We will get here again when we register a new
- * CON_PRINTBUFFER console. Clear the flag so we
- * will properly dump everything later.
- */
- msg->flags &= ~LOG_NOCONS;
- console_prev = msg->flags;
goto skip;
}
- len += msg_print_text(msg, console_prev, false,
- text + len, sizeof(text) - len);
+ len += msg_print_text(msg, false, text + len, sizeof(text) - len);
if (nr_ext_console_drivers) {
ext_len = msg_print_ext_header(ext_text,
sizeof(ext_text),
- msg, console_seq, console_prev);
+ msg, console_seq);
ext_len += msg_print_ext_body(ext_text + ext_len,
sizeof(ext_text) - ext_len,
log_dict(msg), msg->dict_len,
@@ -2449,7 +2317,6 @@ void console_unlock(void)
}
console_idx = log_next(console_idx);
console_seq++;
- console_prev = msg->flags;
raw_spin_unlock(&logbuf_lock);
stop_critical_timings(); /* don't trace print latency */
@@ -2483,7 +2350,7 @@ void console_unlock(void)
if (retry && console_trylock())
goto again;
- if (wake_klogd)
+ if (wake_klogd || cont.len)
wake_up_klogd();
}
EXPORT_SYMBOL(console_unlock);
@@ -2744,7 +2611,6 @@ void register_console(struct console *newcon)
raw_spin_lock_irqsave(&logbuf_lock, flags);
console_seq = syslog_seq;
console_idx = syslog_idx;
- console_prev = syslog_prev;
raw_spin_unlock_irqrestore(&logbuf_lock, flags);
/*
* We're about to replay the log buffer. Only do this to the
@@ -2883,6 +2749,7 @@ static void wake_up_klogd_work_func(struct irq_work *irq_work)
if (pending & PRINTK_PENDING_WAKEUP)
wake_up_interruptible(&log_wait);
+ deferred_cont_flush();
}
static DEFINE_PER_CPU(struct irq_work, wake_up_klogd_work) = {
@@ -3095,7 +2962,7 @@ bool kmsg_dump_get_line_nolock(struct kmsg_dumper *dumper, bool syslog,
goto out;
msg = log_from_idx(dumper->cur_idx);
- l = msg_print_text(msg, 0, syslog, line, size);
+ l = msg_print_text(msg, syslog, line, size);
dumper->cur_idx = log_next(dumper->cur_idx);
dumper->cur_seq++;
@@ -3165,7 +3032,6 @@ bool kmsg_dump_get_buffer(struct kmsg_dumper *dumper, bool syslog,
u32 idx;
u64 next_seq;
u32 next_idx;
- enum log_flags prev;
size_t l = 0;
bool ret = false;
@@ -3189,27 +3055,23 @@ bool kmsg_dump_get_buffer(struct kmsg_dumper *dumper, bool syslog,
/* calculate length of entire buffer */
seq = dumper->cur_seq;
idx = dumper->cur_idx;
- prev = 0;
while (seq < dumper->next_seq) {
struct printk_log *msg = log_from_idx(idx);
- l += msg_print_text(msg, prev, true, NULL, 0);
+ l += msg_print_text(msg, true, NULL, 0);
idx = log_next(idx);
seq++;
- prev = msg->flags;
}
/* move first record forward until length fits into the buffer */
seq = dumper->cur_seq;
idx = dumper->cur_idx;
- prev = 0;
while (l > size && seq < dumper->next_seq) {
struct printk_log *msg = log_from_idx(idx);
- l -= msg_print_text(msg, prev, true, NULL, 0);
+ l -= msg_print_text(msg, true, NULL, 0);
idx = log_next(idx);
seq++;
- prev = msg->flags;
}
/* last message in next interation */
@@ -3220,10 +3082,9 @@ bool kmsg_dump_get_buffer(struct kmsg_dumper *dumper, bool syslog,
while (seq < dumper->next_seq) {
struct printk_log *msg = log_from_idx(idx);
- l += msg_print_text(msg, prev, syslog, buf + l, size - l);
+ l += msg_print_text(msg, syslog, buf + l, size - l);
idx = log_next(idx);
seq++;
- prev = msg->flags;
}
dumper->next_seq = next_seq;
^ permalink raw reply related
* [PATCH 3/4] usb: musb: da8xx: Add DT support for the DA8xx driver
From: Sergei Shtylyov @ 2016-10-25 17:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477406345-27192-4-git-send-email-abailon@baylibre.com>
Hello.
On 10/25/2016 05:39 PM, Alexandre Bailon wrote:
> From: Petr Kulhavy <petr@barix.com>
>
> This adds DT support for TI DA8xx/OMAP-L1x/AM17xx/AM18xx MUSB driver
>
> Signed-off-by: Petr Kulhavy <petr@barix.com>
> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
> ---
> drivers/usb/musb/da8xx.c | 76 ++++++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 67 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
> index 210b7e4..d465087 100644
> --- a/drivers/usb/musb/da8xx.c
> +++ b/drivers/usb/musb/da8xx.c
> @@ -6,6 +6,9 @@
> * Based on the DaVinci "glue layer" code.
> * Copyright (C) 2005-2006 by Texas Instruments
> *
> + * DT support
> + * Copyright (c) 2016 Petr Kulhavy <petr@barix.com>
> + *
> * This file is part of the Inventra Controller Driver for Linux.
> *
> * The Inventra Controller Driver for Linux is free software; you
[...]
> @@ -499,15 +537,21 @@ static int da8xx_probe(struct platform_device *pdev)
> memset(musb_resources, 0x00, sizeof(*musb_resources) *
> ARRAY_SIZE(musb_resources));
>
> - musb_resources[0].name = pdev->resource[0].name;
> - musb_resources[0].start = pdev->resource[0].start;
> - musb_resources[0].end = pdev->resource[0].end;
> - musb_resources[0].flags = pdev->resource[0].flags;
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res) {
> + dev_err(&pdev->dev, "failed to get memory.\n");
> + ret = -EINVAL;
> + goto err_unregister_usb_phy;
> + }
> + musb_resources[0] = *res;
What does this change have to do with the DT conversion?
>
> - musb_resources[1].name = pdev->resource[1].name;
> - musb_resources[1].start = pdev->resource[1].start;
> - musb_resources[1].end = pdev->resource[1].end;
> - musb_resources[1].flags = pdev->resource[1].flags;
> + res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> + if (!res) {
> + dev_err(&pdev->dev, "failed to get irq.\n");
> + ret = -EINVAL;
> + goto err_unregister_usb_phy;
> + }
> + musb_resources[1] = *res;
And this?
I'm also concerned that you'd copy the resource linkage fields which the
existing code avoids...
[...]
MBR, Sergei
^ permalink raw reply
* [PATCH V4 0/5] firmware: Add support for TI System Control Interface (TI-SCI) protocol driver
From: Tero Kristo @ 2016-10-25 17:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161018230837.6515-1-nm@ti.com>
On 19/10/16 02:08, Nishanth Menon wrote:
> Version 4 of the series is basically a rebase to v4.9-rc1, no functional
> changes.
Hi,
Any final comments on this series, or shall I send a pull-req forward?
Very minimal changes compared to v3 so should be good to go imo.
-Tero
>
> Texas Instruments' Keystone generation System on Chips (SoC)
> starting with 66AK2G02[1], now include a dedicated SoC System Control
> entity called PMMC(Power Management Micro Controller) in line with
> ARM architecture recommendations. The function of this module is
> to integrate all system operations in a centralized location.
> Communication with the SoC System Control entity from various
> processing units like ARM/DSP occurs over Message Manager hardware
> block.
>
> This series adds the base support for TI System Control Interface
> (TI-SCI) protocol[2]. The protocol is built on top of Texas
> Instrument's Message Manager communication mechanism[3].
>
> Overall architecture is very similar to SCPI[4] as follows:
> +-------------+ +---------+ +------^-----+
> | TI SCI GENPD| |TISCI Clk| |TISCI reset |
> +------+------+ +----+----+ +------+-----+
> | | |
> | +----v--------------+ |
> +----------> TISCI Protocol(*) <--+
> +----+--------------+
> |
> +---v-----------+
> | MAILBOX FWK |
> +---+-----------+
> |
> +---v-----------+
> | TI MSGMGR |-> TISCI hardware block
> +---------------+
> (*) This series.
>
>
> V4 of the series is based off v4.9-rc1 and is also available here:
> https://github.com/nmenon/linux-2.6-playground/commits/upstream/v4.10/tisci-base-v4
> Quick boot test: http://pastebin.ubuntu.com/23346183/
>
> Changes since v3:
> - rebase to v4.9-rc1
> - minor checkpatch fixes
>
> V3: https://lkml.org/lkml/2016/9/6/747
> V2: https://lkml.org/lkml/2016/8/30/273
> V1: https://lkml.org/lkml/2016/8/19/768
>
> Nishanth Menon (5):
> Documentation: Add support for TI System Control Interface (TI-SCI)
> protocol
> firmware: Add basic support for TI System Control Interface (TI-SCI)
> protocol
> firmware: ti_sci: Add support for Device control
> firmware: ti_sci: Add support for Clock control
> firmware: ti_sci: Add support for reboot core service
>
> .../devicetree/bindings/arm/keystone/ti,sci.txt | 81 +
> MAINTAINERS | 10 +
> drivers/firmware/Kconfig | 15 +
> drivers/firmware/Makefile | 1 +
> drivers/firmware/ti_sci.c | 1991 ++++++++++++++++++++
> drivers/firmware/ti_sci.h | 492 +++++
> include/linux/soc/ti/ti_sci_protocol.h | 249 +++
> 7 files changed, 2839 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
> create mode 100644 drivers/firmware/ti_sci.c
> create mode 100644 drivers/firmware/ti_sci.h
> create mode 100644 include/linux/soc/ti/ti_sci_protocol.h
>
^ permalink raw reply
* [PATCH] ARM: imx6: Fix GPC probe error path
From: Guenter Roeck @ 2016-10-25 17:34 UTC (permalink / raw)
To: linux-arm-kernel
GPC may fail to instantiate with
imx-gpc: probe of 20dc000.gpc failed with error -22
which is returned from of_genpd_add_provider_onecell(). The error path
does not call pm_genpd_remove(). This results in the following crash
later on.
Unhandled fault: page domain fault (0x01b) at 0x00000040
pgd = c0204000
[00000040] *pgd=00000000
Internal error: : 1b [#1] SMP ARM
Modules linked in:
CPU: 0 PID: 108 Comm: kworker/0:3 Not tainted 4.9.0-rc2 #8
Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
Workqueue: pm genpd_power_off_work_fn
task: c759ea00 task.stack: c766a000
PC is at mutex_lock+0xc/0x4c
LR is at regulator_disable+0x28/0x64
...
[<c0bc2694>] (mutex_lock) from [<c06e4e8c>] (regulator_disable+0x28/0x64)
[<c06e4e8c>] (regulator_disable) from [<c0323b68>] (imx6q_pm_pu_power_off+0x90/0x98)
[<c0323b68>] (imx6q_pm_pu_power_off) from [<c07efb04>] (genpd_poweroff+0x114/0x1d4)
[<c07efb04>] (genpd_poweroff) from [<c07efdc0>] (genpd_power_off_work_fn+0x20/0x2c)
[<c07efdc0>] (genpd_power_off_work_fn) from [<c0358f70>] (process_one_work+0x138/0x34c)
[<c0358f70>] (process_one_work) from [<c03591bc>] (worker_thread+0x38/0x510)
[<c03591bc>] (worker_thread) from [<c035e48c>] (kthread+0xdc/0xf4)
[<c035e48c>] (kthread) from [<c0307eb8>] (ret_from_fork+0x14/0x3c)
This is seen with multi_v7_defconfig and imx6dl-sabrelite.dtb running in
qemu (v2.7 patched to fix a qemu related problem). The error return from
of_genpd_add_provider_onecell() is not seen in v4.8 and may be caused by
a devicetree change (this is a wild guess only), but that is a different
problem.
Fixes: 00eb60a8b4f7 ("ARM: imx6: gpc: Add PU power domain for GPU/VPU")
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
Several bisect attempts trying to track down "imx-gpc: probe ... failed
with error -22" point to commit 00e729c93395 ("Merge tag 'armsoc-dt' of
git://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc"). I have not
been able to track down the real culprit. Part of the problem is that
CONFIG_REGULATOR_ANATOP must be enabled for the problem to be seen, and
CONFIG_ARCH_AT91 causes compile errors for some sequence of commits between
v4.8 and v4.9-rc1. But even after taking this into account, the bisect
results always point to 00e729c93395. If anyone has an idea how to track
down that problem, or what might be causing it, please let me know.
arch/arm/mach-imx/gpc.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-imx/gpc.c b/arch/arm/mach-imx/gpc.c
index 0df062d8b2c9..f3f40045b4c9 100644
--- a/arch/arm/mach-imx/gpc.c
+++ b/arch/arm/mach-imx/gpc.c
@@ -409,6 +409,7 @@ static int imx_gpc_genpd_init(struct device *dev, struct regulator *pu_reg)
{
struct clk *clk;
int i;
+ int ret;
imx6q_pu_domain.reg = pu_reg;
@@ -431,9 +432,14 @@ static int imx_gpc_genpd_init(struct device *dev, struct regulator *pu_reg)
return 0;
pm_genpd_init(&imx6q_pu_domain.base, NULL, false);
- return of_genpd_add_provider_onecell(dev->of_node,
- &imx_gpc_onecell_data);
+ ret = of_genpd_add_provider_onecell(dev->of_node,
+ &imx_gpc_onecell_data);
+ if (ret)
+ goto genpd_remove;
+ return 0;
+genpd_remove:
+ pm_genpd_remove(&imx6q_pu_domain.base);
clk_err:
while (i--)
clk_put(imx6q_pu_domain.clk[i]);
--
2.5.0
^ permalink raw reply related
* [PATCH/RFT v2 12/17] USB: ochi-da8xx: Use a regulator for vbus/overcurrent
From: Axel Haslam @ 2016-10-25 17:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <09a4391e-63b6-1622-f964-4aff0ecae87c@lechnology.com>
On Tue, Oct 25, 2016 at 6:53 PM, David Lechner <david@lechnology.com> wrote:
> On 10/25/2016 03:24 AM, Axel Haslam wrote:
>>
>> On Tue, Oct 25, 2016 at 3:39 AM, David Lechner <david@lechnology.com>
>> wrote:
>>>
>>> On 10/24/2016 11:46 AM, ahaslam at baylibre.com wrote:
>>>>
>>>>
>>>> From: Axel Haslam <ahaslam@baylibre.com>
>>>>
>>>> Currently, the da8xx ohci driver uses a set of gpios and callbacks in
>>>> board files to handle vbus and overcurrent irqs form the power supply.
>>>> However, this does not play nice when moving to a DT based boot were
>>>> we wont have board files.
>>>>
>>>> Instead of requesting and handling the gpio, use the regulator framework
>>>> to take care of enabling and disabling vbus power.
>>>> This has the benefit
>>>> that we dont need to pass any more platform data to the driver:
>>>>
>>>> These will be handled by the regulator framework:
>>>> set_power -> regulator_enable/regulator_disable
>>>> get_power -> regulator_is_enabled
>>>> get_oci -> regulator_get_mode
>>>> ocic_notify -> regulator notification
>>>>
>>>> We can keep the default potpgt and use the regulator start delay
>>>> instead:
>>>> potpgt -> regulator startup delay time
>>>>
>>>> The hawk board does not have a GPIO/OVERCURRENT gpio to control vbus,
>>>> (they should not have been decleared/reserved) so, just remove those
>>>> definitions from the hwk board file.
>>>>
>>>> Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
>>>> ---
>>>
>>>
>>>
>>>
>>> How do you recover after an overcurrent event?
>>>
>>> I have configured a fixed-regulator using device-tree, but similar to the
>>> configuration in the board files here. However, when I shorted out the
>>> VBUS
>>> and caused an overcurrent event, I see nothing in the kernel log saying
>>> that
>>> there was an overcurrent event and after I remove the short, the
>>> regulator
>>> is never turned back on.
>>>
>>>
>>
>> You should have the patch to fix gpiolib, and you should declare the
>> over current gpio on the regulator as such:
>> (if the pin is enabled high you should add oc-active-high);
>>
>> vbus_fixed: fixed-regulator-vbus {
>> compatible = "regulator-fixed";
>> gpio = <&gpio 109 0>;
>> oc-gpio = <&gpio 36 0>;
>> regulator-boot-on;
>> enable-active-high;
>> regulator-name = "vbus";
>> regulator-min-microvolt = <5000000>;
>> regulator-max-microvolt = <5000000>;
>> };
>>
>>
>> Question: Do you see that the over current gpio was requested
>> in debugfs/gpio? and, do you see the interrupt in /proc/interrupts?
>>
>> If you unplug and plug in back the usb device it should work again.
>> also you can unbind and bind it should also start to work:
>> something like:
>>
>> echo usb1 >/sys/bus/usb/drivers/usb/unbind
>> echo usb1 >/sys/bus/usb/drivers/usb/bind
>>
>>
>
> I have added oc-active-high and I get different results, but it is still not
> quite right. When I short the VBUS, I can see that my overcurrent gpio
> changes state. However, the driver does not turn of the VBUS. When I remove
> the short, I get an overcurrent error in the kernel log. I would expect this
> when I create the short, not when I remove it. I also tried adding
> GPIO_ACTIVE_LOW to the oc-gpio, but this did not change the behavior. In
> either case, the oc_gpio shows as high under normal conditions, so perhaps
> there is a problem with the gpio-davinci driver not picking up
> GPIO_ACTIVE_LOW from the device tree.
>
>
> My regulator is basically the same. My device just uses different gpios.
>
> vbus_reg: vbus-reg {
> compatible = "regulator-fixed";
> pinctrl-names = "default";
> pinctrl-0 = <&usb11_pins>;
> gpio = <&gpio 101 GPIO_ACTIVE_LOW>;
> oc-gpio = <&gpio 99 0>;
> enable-active-high;
> oc-active-high;
> regulator-name = "vbus";
> regulator-min-microvolt = <5000000>;
> regulator-max-microvolt = <5000000>;
> }
>
>
> It seems to me though that I should not have oc-active-high since under
> normal conditions, the oc_gpio is high and during an overcurrent event, the
in my board the over current gpio is active low too, i was just mentiontioning
to add that in case yours was not.
> oc_gpio is low. Double-checking the behavior without oc-active-high, I see
> that the vbus gpio is turned off in response to the overcurrent event, but I
> don't get the overcurrent message in the kernel log. Perhaps this is because
> as soon as there is an overcurrent event the vbus turns off and the oc_gpio
> returns to normal before the usb driver has a chance to poll the overcurrent
> state?
Perhaps. i dont have a board that has overcurrent, or that i can
switch vbus off.
what is exactly the log you are refering to?
im wondering, was the behavior different before the patches?
it should be the same without the patches.
>
> Also, unplugging the device and plugging it back in does nothing. Unbinding
> and binding the driver does work, but that does not seem like a very nice
> way to have to recover from an overcurrent event.
im guessing that unplug and plug wont work as vbus is gone and
we cannot detect it anymore (contrary to my setup were i always have vbus)
sorry for suggesting that before.
we could recover vbus automatically form the notification,
(when the over-current condition disappears) but then im afraid we
might oscillate, even if we retry after some timeout. so perhaps
bind/unbind is the right thing to do, as it requires the user to correct
the problem. I dont know if there is a better solution.
>
>
>
^ permalink raw reply
* [PATCH V2] arm64: Neaten show_regs, remove KERN_CONT
From: Joe Perches @ 2016-10-25 17:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161025170745.GE8898@leverpostej>
On Tue, 2016-10-25 at 18:07 +0100, Mark Rutland wrote:
> On Tue, Oct 25, 2016 at 09:40:26AM -0700, Joe Perches wrote:
> > commit db4b0710fae9 ("arm64: fix show_regs fallout from KERN_CONT changes")
> > corrected the KERN_CONT fallout from commit 4bcc595ccd80
> > ("printk: reinstate KERN_CONT for printing continuation lines"), but
> > the code still has unnecessary KERN_CONT uses.
> >
> > Remove the KERN_CONT uses to avoid possible message interleaving.
Hey Mark.
Please fix it as you think appropriate.
No credit to me is necessary.
cheers, Joe
^ permalink raw reply
* [PATCH] tty/serial: at91: fix hardware handshake on Atmel platforms
From: Uwe Kleine-König @ 2016-10-25 17:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161025161135.7316-1-richard.genoud@gmail.com>
Hello,
On Tue, Oct 25, 2016 at 06:11:35PM +0200, Richard Genoud wrote:
> commit 1cf6e8fc8341 ("tty/serial: at91: fix RTS line management when
> hardware handshake is enabled"), despite its title, broke hardware
> handshake on *every* Atmel platforms.
s/platforms/platform/
> The only one partially working is the SAMA5D2.
>
> To understand why, one has to understand the flag ATMEL_US_USMODE_HWHS
> first:
> Before commit 1cf6e8fc8341 ("tty/serial: at91: fix RTS line management
> when hardware handshake is enabled"), this flag was never set.
> Thus, the CTS/RTS where only handled by serial_core (and everything
> worked just fine).
>
> This commit introduced the use of the ATMEL_US_USMODE_HWHS flag,
> enabling it for all boards when the user space enables flow control.
>
> When the ATMEL_US_USMODE_HWHS is set, the Atmel USART controller
> handles a part of the flow control job:
> - disable the transmitter when the CTS pin gets high.
> - drive the RTS pin high when the DMA buffer transfer is completed or
> PDC RX buffer full or RX FIFO is beyond threshold. (depending on the
> controller version).
I don't understand the DMA buffer part.
> NB: This feature is *not* mandatory for the flow control to work.
>
> Now, the specifics of the ATMEL_US_USMODE_HWHS flag:
>
> - For platforms with DMAC and no FIFOs (sam9x25, sam9x35, sama5D3,
> sama5D4, sam9g15, sam9g25, sam9g35)* this feature simply doesn't work.
> ( source: https://lkml.org/lkml/2016/9/7/598 )
What does "doesn't work" mean? Is ATMEL_US_USMODE_HWHS a noop, or does
it break something?
> Tested it on sam9g35, the RTS pins always stays up, even when RXEN=1
> or a new DMA transfer descriptor is set.
> => ATMEL_US_USMODE_HWHS should not be used for those platforms
Depending on the answer to the above question it might not matter if it
is set or not.
> - For platforms with a PDC (sam926{0,1,3}, sam9g10, sam9g20, sam9g45,
> sam9g46)*, there's another kind of problem. Once the flag
> ATMEL_US_USMODE_HWHS is set, the RTS pin can't be driven anymore via
> RTSEN/RTSDIS in USART Control Register. The RTS pin can only be driven
> by enabling/disabling the receiver or setting RCR=RNCR=0 in the PDC
> (Receive (Next) Counter Register).
> => Doing this is beyond the scope of this patch and could add other
> bugs, so the original (and working) behaviour should be set for those
> platforms (meaning ATMEL_US_USMODE_HWHS flag should be unset).
Then maybe just revert the faulty patch for now and do it better later
on top of this?
> - For platforms with a FIFO (sama5d2)*, the RTS pin is driven according
> to the RX FIFO thresholds, and can be also driven by RTSEN/RTSDIS in
> USART Control Register. No problem here.
> (This was the use case of commit 1cf6e8fc8341 ("tty/serial: at91: fix
> RTS line management when hardware handshake is enabled"))
> NB: If the CTS pin declared as a GPIO in the DTS, (for instance
> cts-gpios = <&pioA PIN_PB31 GPIO_ACTIVE_LOW>), the transmitter will be
> disabled.
> => ATMEL_US_USMODE_HWHS flag can be set for this platform ONLY IF the
> CTS pin is not a GPIO.
How did you test this? What I consider interesting here is if the
hardware CTS function was muxed on a pin and in which state this pin (if
any) is. If it is not muxed anywhere and disables the transmitter
because of an internal pull up that is IMHO a hw bug and should be
mentioned more explicitly in the comment.
> So, the only case when ATMEL_US_USMODE_HWHS can be enabled is when
> (atmel_use_fifo(port) &&
> !mctrl_gpio_to_gpiod(atmel_port->gpios, UART_GPIO_CTS))
>
> Tested on all Atmel USART controller flavours:
> AT91SAM9G35-CM, AT91SAM9G20-EK and SAMA5D2xplained
> ^^^^ ^^^^ ^^^^
> (DMAC flavour), (PDC flavour) and (FIFO flavour)
I'd write that as: Tested on all Atmel USART controller flavours:
AT91SAM9G35-CM (DMAC flavour), AT91SAM9G20-EK (PDC flavour),
SAMA5D2xplained (FIFO flavour).
> Changes since v4:
> - the mctrl_gpio_use_rtscts() is gone since it was atmel_serial
> specific. (so patch 1 is gone)
> - patches 2 and 3 have been merged together since it didn't make
> a lot of sense to correct the GPIO case in one separate patch.
> - ATMEL_US_USMODE_HWHS is now unset for platform with PDC
>
> Changes since v3:
> - remove superfluous #include <linux/err.h> (thanks to Uwe)
> - rebase on next-20160930
>
> Changes since v2:
> - remove IS_ERR_OR_NULL() test in patch 1/3 as Uwe suggested.
> - fix typos in patch 2/3
> - rebase on next-20160927
> - simplify the logic in patch 3/3.
>
> Changes since v1:
> - Correct patch 1 with the error found by kbuild.
> - Add Alexandre's Acked-by on patch 2
> - Rewrite patch 3 logic in the light of the on-going discussion
> with Cyrille and Alexandre.
>
> * the list may not be exhaustive
Add a Fixes: line please.
> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
> ---
> drivers/tty/serial/atmel_serial.c | 25 +++++++++++++++++++++----
> 1 file changed, 21 insertions(+), 4 deletions(-)
>
> I think this should go in the stable tree since it fixes the flow
> control broken since v4.0.
> But It won't compile on versions before 4.9rc1 because:
> function atmel_use_fifo was introduced in 4.4.12 / 4.7
> variable atmel_port was introduced in 4.9rc1
>
> That's why I didn't add the Cc stable in the email body.
>
>
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index fd8aa1f4ba78..2c7c45904ba7 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -2132,11 +2132,28 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
> mode |= ATMEL_US_USMODE_RS485;
> } else if (termios->c_cflag & CRTSCTS) {
> /* RS232 with hardware handshake (RTS/CTS) */
> - if (atmel_use_dma_rx(port) && !atmel_use_fifo(port)) {
> - dev_info(port->dev, "not enabling hardware flow control because DMA is used");
> - termios->c_cflag &= ~CRTSCTS;
This if was not introduced in commit 1cf6e8fc8341. Is it still right to
remove this here?
> - } else {
> + if (atmel_use_fifo(port) &&
> + !mctrl_gpio_to_gpiod(atmel_port->gpios, UART_GPIO_CTS)) {
> + /*
> + * with ATMEL_US_USMODE_HWHS set, the controller will
> + * be able to drive the RTS pin high/low when the RX
> + * FIFO is above RXFTHRES/below RXFTHRES2.
> + * It will also disable the transmitter when the CTS
> + * pin is high.
> + * This mode is not activated if CTS pin is a GPIO
> + * because in this case, the transmitter is always
> + * disabled.
> + * If the RTS pin is a GPIO, the controller won't be
> + * able to drive it according to the FIFO thresholds,
> + * but it will be handled by the driver.
> + */
> mode |= ATMEL_US_USMODE_HWHS;
> + } else {
> + /*
> + * For platforms without FIFO, the flow control is
> + * handled by the driver.
> + */
> + mode |= ATMEL_US_USMODE_NORMAL;
> }
> } else {
> /* RS232 without hadware handshake */
(unrelated to this patch) s/hadware/hardware/
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* [PATCH 2/3] ARM: convert to generated system call tables
From: Geert Uytterhoeven @ 2016-10-25 17:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4146248.jXuviLlvH5@wuerfel>
On Tue, Oct 25, 2016 at 12:28 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tuesday, October 25, 2016 10:12:10 PM CEST Michael Cree wrote:
>> On Fri, Oct 21, 2016 at 03:06:45PM +0200, Arnd Bergmann wrote:
>> > I see your point, but I think there are serious issues with the current
>> > approach as well:
>> >
>> > - a lot of the less common architectures just don't get updated
>> > in time, out of 22 architectures that don't use asm-generic/unistd.h,
>> > only 12 have pwritev2 in linux-next, and only three have pkey_mprotect
>> >
>> > - some architectures that add all syscalls sometimes make a mistake
>> > and forget one, e.g. alpha apparently never added __NR_bpf, but it
>> > did add the later __NR_execveat.
>>
>> __NR_bpf was not forgotten on Alpha. It was not wired up because
>> extra architecture support is needed which has not been implemented.
>>
>> But maybe we should just wire it up to sys_ni_syscall in the meantime
>> so a syscall number is reserved for it, and user space can call it to
>> get -ENOSYS returned.
>
> Ah, I must have misinterpreted the code then. I assumed that the
> bpf syscall always works on all architectures, but that only the
> jit compiler for it required architecture specific code to make it
> more efficient.
>
> The implementation of sys_bfp is compile-time selectable at the moment
> and falls back to sys_no_syscall if CONFIG_BPF_SYSCALL is disabled.
> If it doesn't work on Alpha, maybe that symbol could have a "depends
> on !ALPHA" or "depends on BPF_SUPPORT"?
Yes, BPF should just work (m68k has it).
> sys_seccomp is another one that falls into a similar category, but
> it already depends on HAVE_ARCH_SECCOMP_FILTER, and most other
> architectures have assigned a syscall number but not set this symbol.
> This one will actually allow you to set strict seccomp mode even
> without the Kconfig symbol, just not allow to set a filter.
Seccomp needs architecture support (m68k doesn't have it).
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* [PATCH V2] arm64: Neaten show_regs, remove KERN_CONT
From: Mark Rutland @ 2016-10-25 17:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <289fb30081aba03fec0c960e7334f707bf36c881.1477413522.git.joe@perches.com>
On Tue, Oct 25, 2016 at 09:40:26AM -0700, Joe Perches wrote:
> commit db4b0710fae9 ("arm64: fix show_regs fallout from KERN_CONT changes")
> corrected the KERN_CONT fallout from commit 4bcc595ccd80
> ("printk: reinstate KERN_CONT for printing continuation lines"), but
> the code still has unnecessary KERN_CONT uses.
>
> Remove the KERN_CONT uses to avoid possible message interleaving.
>
> Miscellanea:
>
> o Remove unnecessary trailing blank from the output too.
> o Convert i and top_reg to unsigned int
> o Move the extra blank line after __show_reg to the caller for symmetry
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
> arch/arm64/kernel/process.c | 21 ++++++++++-----------
> 1 file changed, 10 insertions(+), 11 deletions(-)
>
> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> index 01753cd7d3f0..5ba12f019bf7 100644
> --- a/arch/arm64/kernel/process.c
> +++ b/arch/arm64/kernel/process.c
> @@ -168,7 +168,7 @@ void machine_restart(char *cmd)
>
> void __show_regs(struct pt_regs *regs)
> {
> - int i, top_reg;
> + unsigned int i, top_reg;
This change is not necessary. These will work perfectly fine as ints;
please leave them as-is.
> u64 lr, sp;
>
> if (compat_user_mode(regs)) {
> @@ -190,24 +190,23 @@ void __show_regs(struct pt_regs *regs)
>
> i = top_reg;
>
> - while (i >= 0) {
> - printk("x%-2d: %016llx ", i, regs->regs[i]);
> + if (i % 2) {
This should be:
if (i % 2 == 0) {
Otherwise we'll lose x0 in the native case (and x29 will be given a line
of its own).
> + printk("x%-2d: %016llx\n", i, regs->regs[i]);
> i--;
> -
> - if (i % 2 == 0) {
> - pr_cont("x%-2d: %016llx ", i, regs->regs[i]);
> - i--;
> - }
> -
> - pr_cont("\n");
> }
> - printk("\n");
> + while (i > 0) {
> + printk("x%-2d: %016llx x%-2d: %016llx\n",
> + i, regs->regs[i],
> + i - 1, regs->regs[i - 1]);
> + i -= 2;
> + }
> }
>
> void show_regs(struct pt_regs * regs)
> {
> printk("\n");
> __show_regs(regs);
> + printk("\n");
> }
Other functions call __show_regs() directly and the trailing newline
there is expected. Please leave it in __show_regs().
Otherwise, this looks fine to me.
Thanks,
Mark.
^ permalink raw reply
* [PATCH 2/3] ARM: convert to generated system call tables
From: Richard Henderson @ 2016-10-25 17:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4146248.jXuviLlvH5@wuerfel>
On 10/25/2016 03:28 AM, Arnd Bergmann wrote:
> On Tuesday, October 25, 2016 10:12:10 PM CEST Michael Cree wrote:
>> On Fri, Oct 21, 2016 at 03:06:45PM +0200, Arnd Bergmann wrote:
>>> I see your point, but I think there are serious issues with the current
>>> approach as well:
>>>
>>> - a lot of the less common architectures just don't get updated
>>> in time, out of 22 architectures that don't use asm-generic/unistd.h,
>>> only 12 have pwritev2 in linux-next, and only three have pkey_mprotect
>>>
>>> - some architectures that add all syscalls sometimes make a mistake
>>> and forget one, e.g. alpha apparently never added __NR_bpf, but it
>>> did add the later __NR_execveat.
>>
>> __NR_bpf was not forgotten on Alpha. It was not wired up because
>> extra architecture support is needed which has not been implemented.
>>
>> But maybe we should just wire it up to sys_ni_syscall in the meantime
>> so a syscall number is reserved for it, and user space can call it to
>> get -ENOSYS returned.
>
> Ah, I must have misinterpreted the code then. I assumed that the
> bpf syscall always works on all architectures, but that only the
> jit compiler for it required architecture specific code to make it
> more efficient.
That was my interpretation as well. What's the problem, Michael?
r~
^ permalink raw reply
* [PATCH v2 0/4] ARM: K2G: Add support for TI-SCI Generic PM Domains
From: Kevin Hilman @ 2016-10-25 17:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161019203347.17893-1-d-gerlach@ti.com>
Dave Gerlach <d-gerlach@ti.com> writes:
> Hi,
> This is v2 of the series to add support for TI SCI PM Domains. v1 of
> the series can be found here [1]. Several things have changed since v1:
>
> - New patch to add a void *data to struct generic_pm_domain_data to
> allow to store per device data associated with a genpd
> - From v1, squash patch 1 and 2 to introduce docs and dt-bindings in
> one patch based on comment from Ulf
> - Fix some grammar errors in Documentation
> - Based on comments from Ulf, rework actual genpd implementation to
> avoid creating one genpd per device and instead use device start/stop
> hooks provided as part of genpd to control device state based on pm_runtime
> implementation. Also make use of new of_genpd_add_provider_simple API
> introduced by Jon Hunter and do not provide custom of_xlate to genpd core,
> instead registering devices as they probe through attach_dev hook provided
> by genpd framework.
>
> Most of the changes were motivated by the comments from Ulf Hannson on v1 that we
> should not use a 1-to-1 genpd to device mapping. The new approach allows us to
> create a single genpd and store information about each device as they attach to
> the genpd. Then the device start/stop hooks for that genpd leverage the
> per-device data to control power states over the TI SCI protocol.
>
> This driver makes use of the ti_sci driver sent here [2] by Nishanth Menon and
> applies on top of his series on v4.9-rc1.
Reviewed-by: Kevin Hilman <khilman@baylibre.com>
^ permalink raw reply
* [PATCH/RFT v2 12/17] USB: ochi-da8xx: Use a regulator for vbus/overcurrent
From: David Lechner @ 2016-10-25 16:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAKXjFTOxQ58rThpPw9t73k-BBZ8YKGvMhWfAG1r+3kC=GJVxWg@mail.gmail.com>
On 10/25/2016 03:24 AM, Axel Haslam wrote:
> On Tue, Oct 25, 2016 at 3:39 AM, David Lechner <david@lechnology.com> wrote:
>> On 10/24/2016 11:46 AM, ahaslam at baylibre.com wrote:
>>>
>>> From: Axel Haslam <ahaslam@baylibre.com>
>>>
>>> Currently, the da8xx ohci driver uses a set of gpios and callbacks in
>>> board files to handle vbus and overcurrent irqs form the power supply.
>>> However, this does not play nice when moving to a DT based boot were
>>> we wont have board files.
>>>
>>> Instead of requesting and handling the gpio, use the regulator framework
>>> to take care of enabling and disabling vbus power.
>>> This has the benefit
>>> that we dont need to pass any more platform data to the driver:
>>>
>>> These will be handled by the regulator framework:
>>> set_power -> regulator_enable/regulator_disable
>>> get_power -> regulator_is_enabled
>>> get_oci -> regulator_get_mode
>>> ocic_notify -> regulator notification
>>>
>>> We can keep the default potpgt and use the regulator start delay instead:
>>> potpgt -> regulator startup delay time
>>>
>>> The hawk board does not have a GPIO/OVERCURRENT gpio to control vbus,
>>> (they should not have been decleared/reserved) so, just remove those
>>> definitions from the hwk board file.
>>>
>>> Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
>>> ---
>>
>>
>>
>> How do you recover after an overcurrent event?
>>
>> I have configured a fixed-regulator using device-tree, but similar to the
>> configuration in the board files here. However, when I shorted out the VBUS
>> and caused an overcurrent event, I see nothing in the kernel log saying that
>> there was an overcurrent event and after I remove the short, the regulator
>> is never turned back on.
>>
>>
>
> You should have the patch to fix gpiolib, and you should declare the
> over current gpio on the regulator as such:
> (if the pin is enabled high you should add oc-active-high);
>
> vbus_fixed: fixed-regulator-vbus {
> compatible = "regulator-fixed";
> gpio = <&gpio 109 0>;
> oc-gpio = <&gpio 36 0>;
> regulator-boot-on;
> enable-active-high;
> regulator-name = "vbus";
> regulator-min-microvolt = <5000000>;
> regulator-max-microvolt = <5000000>;
> };
>
>
> Question: Do you see that the over current gpio was requested
> in debugfs/gpio? and, do you see the interrupt in /proc/interrupts?
>
> If you unplug and plug in back the usb device it should work again.
> also you can unbind and bind it should also start to work:
> something like:
>
> echo usb1 >/sys/bus/usb/drivers/usb/unbind
> echo usb1 >/sys/bus/usb/drivers/usb/bind
>
>
I have added oc-active-high and I get different results, but it is still
not quite right. When I short the VBUS, I can see that my overcurrent
gpio changes state. However, the driver does not turn of the VBUS. When
I remove the short, I get an overcurrent error in the kernel log. I
would expect this when I create the short, not when I remove it. I also
tried adding GPIO_ACTIVE_LOW to the oc-gpio, but this did not change
the behavior. In either case, the oc_gpio shows as high under normal
conditions, so perhaps there is a problem with the gpio-davinci driver
not picking up GPIO_ACTIVE_LOW from the device tree.
My regulator is basically the same. My device just uses different gpios.
vbus_reg: vbus-reg {
compatible = "regulator-fixed";
pinctrl-names = "default";
pinctrl-0 = <&usb11_pins>;
gpio = <&gpio 101 GPIO_ACTIVE_LOW>;
oc-gpio = <&gpio 99 0>;
enable-active-high;
oc-active-high;
regulator-name = "vbus";
regulator-min-microvolt = <5000000>;
regulator-max-microvolt = <5000000>;
}
It seems to me though that I should not have oc-active-high since under
normal conditions, the oc_gpio is high and during an overcurrent event,
the oc_gpio is low. Double-checking the behavior without oc-active-high,
I see that the vbus gpio is turned off in response to the overcurrent
event, but I don't get the overcurrent message in the kernel log.
Perhaps this is because as soon as there is an overcurrent event the
vbus turns off and the oc_gpio returns to normal before the usb driver
has a chance to poll the overcurrent state?
Also, unplugging the device and plugging it back in does nothing.
Unbinding and binding the driver does work, but that does not seem like
a very nice way to have to recover from an overcurrent event.
^ permalink raw reply
* [PATCH 3/4] dt-bindings: Update domain-idle-state binding to use correct compatibles
From: Sudeep Holla @ 2016-10-25 16:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161025162440.GA48977@linaro.org>
On 25/10/16 17:24, Lina Iyer wrote:
> On Tue, Oct 25 2016 at 09:59 -0600, Sudeep Holla wrote:
>>
>>
>> On 25/10/16 16:26, Lina Iyer wrote:
>>> Update domain-idle-state binding to use "domain-idle-state" compatible
>>> from Documentation/devicetree/bindings/arm/idle-states.txt.
>>>
>>> Cc: <devicetree@vger.kernel.org>
>>> Cc: Rob Herring <robh@kernel.org>
>>> Suggested-by: Sudeep Holla <sudeep.holla@arm.com>
>>> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
>>> ---
>>> Documentation/devicetree/bindings/power/power_domain.txt | 9 +++++----
>>> 1 file changed, 5 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/power/power_domain.txt
>>> b/Documentation/devicetree/bindings/power/power_domain.txt
>>> index e165036..6fb53a3 100644
>>> --- a/Documentation/devicetree/bindings/power/power_domain.txt
>>> +++ b/Documentation/devicetree/bindings/power/power_domain.txt
>>> @@ -30,8 +30,9 @@ Optional properties:
>>> available in the next section.
>>>
>>> - domain-idle-states : A phandle of an idle-state that shall be
>>> soaked into a
>>> - generic domain power state. The idle state
>>> definitions are
>>> - compatible with arm,idle-state specified in [1].
>>> + generic domain power state. The idle state
>>> definitions must be
>>> + compatible with "domain-idle-state"
>>
>> I would reword the below a bit different so that it's flexible to be
>> reused without "arm,idle-state".
>>
>>> as well as
>>> + "arm,idle-state" as defined in [1].
>>
>> 'Idle states that are "arm,idle-state" compatible are generally
>> "domain-idle-state" compatible as well if it's a PM domain.'
>>
> I believe we should have both compatible strings. Per [1], any CPU that
> follows the idle state compatible *must* have "arm,idle-state" as a
> compatible.
Yes that's implicit for a CPU device. But generic power domain bindings
should not have that explicitly as it *can be* used for non CPU device.
> Since we are re-using the same compatible, its only correct
> that we retain what is already spec'd up in [1] and in addition provide
> this new compatible.
>
Yes [1] applies for *CPUs only* while this applies for *any device* and
*any power domain*, so I would drop *must have* "arm,idle-state" here
to keep this generic based on my understanding on how compatibles work.
--
Regards,
Sudeep
^ permalink raw reply
* [PATCH 2/2] arm64: Support systems without FP/ASIMD
From: Suzuki K Poulose @ 2016-10-25 16:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAKv+Gu9mzX5dUgkJzDYEwxP+u9RVL=RWdsPepQ-UeCCxpx1j_g@mail.gmail.com>
On 25/10/16 15:00, Ard Biesheuvel wrote:
> Hi Suzuki,
>
> On 25 October 2016 at 14:50, Suzuki K Poulose <suzuki.poulose@arm.com> wrote:
>> The arm64 kernel assumes that FP/ASIMD units are always present
>> and accesses the FP/ASIMD specific registers unconditionally. This
>> could cause problems when they are absent. This patch adds the
>> support for kernel handling systems without FP/ASIMD by skipping the
>> register access within the kernel. For kvm, we trap the accesses
>> to FP/ASIMD and inject an undefined instruction exception to the VM.
>>
>> The callers of the exported kernel_neon_begin_parital() should
>> make sure that the FP/ASIMD is supported.
>>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Will Deacon <will.deacon@arm.com>
>> Cc: Christoffer Dall <christoffer.dall@linaro.org>
>> Cc: Marc Zyngier <marc.zyngier@arm.com>
>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>> ---
>> ---
>> arch/arm64/crypto/aes-ce-ccm-glue.c | 2 +-
>> arch/arm64/crypto/aes-ce-cipher.c | 2 ++
>> arch/arm64/crypto/ghash-ce-glue.c | 2 ++
>> arch/arm64/crypto/sha1-ce-glue.c | 2 ++
>> arch/arm64/include/asm/cpufeature.h | 8 +++++++-
>> arch/arm64/include/asm/neon.h | 3 ++-
>> arch/arm64/kernel/cpufeature.c | 15 +++++++++++++++
>> arch/arm64/kernel/fpsimd.c | 14 ++++++++++++++
>> arch/arm64/kvm/handle_exit.c | 11 +++++++++++
>> arch/arm64/kvm/hyp/hyp-entry.S | 9 ++++++++-
>> arch/arm64/kvm/hyp/switch.c | 5 ++++-
>> 11 files changed, 68 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/arm64/crypto/aes-ce-ccm-glue.c b/arch/arm64/crypto/aes-ce-ccm-glue.c
>> index f4bf2f2..d001b4e 100644
>> --- a/arch/arm64/crypto/aes-ce-ccm-glue.c
>> +++ b/arch/arm64/crypto/aes-ce-ccm-glue.c
>> @@ -296,7 +296,7 @@ static struct aead_alg ccm_aes_alg = {
>>
>> static int __init aes_mod_init(void)
>> {
>> - if (!(elf_hwcap & HWCAP_AES))
>> + if (!(elf_hwcap & HWCAP_AES) || !system_supports_fpsimd())
>
> This looks weird to me. All crypto extensionsinstructions except CRC
> operate strictly on FP/ASIMD registers, and so support for FP/ASIMD is
> implied by having HWCAP_AES. In other words, I think it makes more
> sense to sanity check that the info registers are consistent with each
> other in core code than modifying each user (which for HWCAP_xxx
> includes userland) to double check that the world is sane.
You're right. I will respin it.
Btw, I think we need the following feature check for the above. I will send
that in and remove the explicit HWCAP_AES check.
module_cpu_feature_match(AES, aes_mod_init());
Cheers
Suzuki
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
^ 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