* [PATCH v1 2/3] nvmem: Add the Broadcom OTP controller driver
From: Jonathan Richardson @ 2016-10-24 19:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477336324-10543-1-git-send-email-jonathan.richardson@broadcom.com>
From: Jonathan Richardson <jonathar@broadcom.com>
Add support for 32 and 64-bit versions of Broadcom's On-Chip OTP
controller. These controllers are used on SoC's such as Cygnus and
Stingray.
Reviewed-by: Ray Jui <ray.jui@broadcom.com>
Tested-by: Jonathan Richardson <jonathan.richardson@broadcom.com>
Signed-off-by: Scott Branden <scott.branden@broadcom.com>
Signed-off-by: Oza Pawandeep <oza@broadcom.com>
Signed-off-by: Jonathan Richardson <jonathan.richardson@broadcom.com>
---
drivers/nvmem/Kconfig | 12 ++
drivers/nvmem/Makefile | 2 +
drivers/nvmem/bcm-ocotp.c | 335 ++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 349 insertions(+)
create mode 100644 drivers/nvmem/bcm-ocotp.c
diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
index ba140ea..06935a7 100644
--- a/drivers/nvmem/Kconfig
+++ b/drivers/nvmem/Kconfig
@@ -80,6 +80,18 @@ config ROCKCHIP_EFUSE
This driver can also be built as a module. If so, the module
will be called nvmem_rockchip_efuse.
+config NVMEM_BCM_OCOTP
+ tristate "Broadcom On-Chip OTP Controller support"
+ depends on ARCH_BCM_IPROC || COMPILE_TEST
+ depends on HAS_IOMEM
+ default ARCH_BCM_IPROC
+ help
+ Say y here to enable read/write access to the Broadcom OTP
+ controller.
+
+ This driver can also be built as a module. If so, the module
+ will be called nvmem-bcm-ocotp.
+
config NVMEM_SUNXI_SID
tristate "Allwinner SoCs SID support"
depends on ARCH_SUNXI
diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile
index 8f942a0..71781ca 100644
--- a/drivers/nvmem/Makefile
+++ b/drivers/nvmem/Makefile
@@ -6,6 +6,8 @@ obj-$(CONFIG_NVMEM) += nvmem_core.o
nvmem_core-y := core.o
# Devices
+obj-$(CONFIG_NVMEM_BCM_OCOTP) += nvmem-bcm-ocotp.o
+nvmem-bcm-ocotp-y := bcm-ocotp.o
obj-$(CONFIG_NVMEM_IMX_OCOTP) += nvmem-imx-ocotp.o
nvmem-imx-ocotp-y := imx-ocotp.o
obj-$(CONFIG_NVMEM_LPC18XX_EEPROM) += nvmem_lpc18xx_eeprom.o
diff --git a/drivers/nvmem/bcm-ocotp.c b/drivers/nvmem/bcm-ocotp.c
new file mode 100644
index 0000000..646cadb
--- /dev/null
+++ b/drivers/nvmem/bcm-ocotp.c
@@ -0,0 +1,335 @@
+/*
+ * Copyright (C) 2016 Broadcom
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/nvmem-provider.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+
+/*
+ * # of tries for OTP Status. The time to execute a command varies. The slowest
+ * commands are writes which also vary based on the # of bits turned on. Writing
+ * 0xffffffff takes ~3800 us.
+ */
+#define OTPC_RETRIES 5000
+
+/* Sequence to enable OTP program */
+#define OTPC_PROG_EN_SEQ { 0xf, 0x4, 0x8, 0xd }
+
+/* OTPC Commands */
+#define OTPC_CMD_READ 0x0
+#define OTPC_CMD_OTP_PROG_ENABLE 0x2
+#define OTPC_CMD_OTP_PROG_DISABLE 0x3
+#define OTPC_CMD_PROGRAM 0xA
+
+/* OTPC Status Bits */
+#define OTPC_STAT_CMD_DONE BIT(1)
+#define OTPC_STAT_PROG_OK BIT(2)
+
+/* OTPC register definition */
+#define OTPC_MODE_REG_OFFSET 0x0
+#define OTPC_MODE_REG_OTPC_MODE 0
+#define OTPC_COMMAND_OFFSET 0x4
+#define OTPC_COMMAND_COMMAND_WIDTH 6
+#define OTPC_CMD_START_OFFSET 0x8
+#define OTPC_CMD_START_START 0
+#define OTPC_CPU_STATUS_OFFSET 0xc
+#define OTPC_CPUADDR_REG_OFFSET 0x28
+#define OTPC_CPUADDR_REG_OTPC_CPU_ADDRESS_WIDTH 16
+#define OTPC_CPU_WRITE_REG_OFFSET 0x2c
+
+#define OTPC_CMD_MASK (BIT(OTPC_COMMAND_COMMAND_WIDTH) - 1)
+#define OTPC_ADDR_MASK (BIT(OTPC_CPUADDR_REG_OTPC_CPU_ADDRESS_WIDTH) - 1)
+
+
+struct otpc_map {
+ /* in words. */
+ u32 otpc_row_size;
+ /* 128 bit row / 4 words support. */
+ u16 data_r_offset[4];
+ /* 128 bit row / 4 words support. */
+ u16 data_w_offset[4];
+};
+
+static struct otpc_map otp_map = {
+ .otpc_row_size = 1,
+ .data_r_offset = {0x10},
+ .data_w_offset = {0x2c},
+};
+
+static struct otpc_map otp_map_v2 = {
+ .otpc_row_size = 2,
+ .data_r_offset = {0x10, 0x5c},
+ .data_w_offset = {0x2c, 0x64},
+};
+
+struct otpc_priv {
+ struct device *dev;
+ void __iomem *base;
+ struct otpc_map *map;
+ struct nvmem_config *config;
+};
+
+static inline void set_command(void __iomem *base, u32 command)
+{
+ writel(command & OTPC_CMD_MASK, base + OTPC_COMMAND_OFFSET);
+}
+
+static inline void set_cpu_address(void __iomem *base, u32 addr)
+{
+ writel(addr & OTPC_ADDR_MASK, base + OTPC_CPUADDR_REG_OFFSET);
+}
+
+static inline void set_start_bit(void __iomem *base)
+{
+ writel(1 << OTPC_CMD_START_START, base + OTPC_CMD_START_OFFSET);
+}
+
+static inline void reset_start_bit(void __iomem *base)
+{
+ writel(0, base + OTPC_CMD_START_OFFSET);
+}
+
+static inline void write_cpu_data(void __iomem *base, u32 value)
+{
+ writel(value, base + OTPC_CPU_WRITE_REG_OFFSET);
+}
+
+static int poll_cpu_status(void __iomem *base, u32 value)
+{
+ u32 status;
+ u32 retries;
+
+ for (retries = 0; retries < OTPC_RETRIES; retries++) {
+ status = readl(base + OTPC_CPU_STATUS_OFFSET);
+ if (status & value)
+ break;
+ udelay(1);
+ }
+ if (retries == OTPC_RETRIES)
+ return -EAGAIN;
+
+ return 0;
+}
+
+static int enable_ocotp_program(void __iomem *base)
+{
+ static const u32 vals[] = OTPC_PROG_EN_SEQ;
+ int i;
+ int ret;
+
+ /* Write the magic sequence to enable programming */
+ set_command(base, OTPC_CMD_OTP_PROG_ENABLE);
+ for (i = 0; i < ARRAY_SIZE(vals); i++) {
+ write_cpu_data(base, vals[i]);
+ set_start_bit(base);
+ ret = poll_cpu_status(base, OTPC_STAT_CMD_DONE);
+ reset_start_bit(base);
+ if (ret)
+ return ret;
+ }
+
+ return poll_cpu_status(base, OTPC_STAT_PROG_OK);
+}
+
+static int disable_ocotp_program(void __iomem *base)
+{
+ int ret;
+
+ set_command(base, OTPC_CMD_OTP_PROG_DISABLE);
+ set_start_bit(base);
+ ret = poll_cpu_status(base, OTPC_STAT_PROG_OK);
+ reset_start_bit(base);
+
+ return ret;
+}
+
+static int bcm_otpc_read(void *context, unsigned int offset, void *val,
+ size_t bytes)
+{
+ struct otpc_priv *priv = context;
+ u32 *buf = val;
+ u32 bytes_read;
+ u32 address = offset / priv->config->word_size;
+ int i, ret;
+
+ for (bytes_read = 0; bytes_read < bytes;) {
+ set_command(priv->base, OTPC_CMD_READ);
+ set_cpu_address(priv->base, address++);
+ set_start_bit(priv->base);
+ ret = poll_cpu_status(priv->base, OTPC_STAT_CMD_DONE);
+ if (ret) {
+ dev_err(priv->dev, "otp read error: 0x%x", ret);
+ return -EIO;
+ }
+
+ for (i = 0; i < priv->map->otpc_row_size; i++) {
+ *buf++ = readl(priv->base +
+ priv->map->data_r_offset[i]);
+ bytes_read += sizeof(*buf);
+ }
+
+ reset_start_bit(priv->base);
+ }
+
+ return 0;
+}
+
+static int bcm_otpc_write(void *context, unsigned int offset, void *val,
+ size_t bytes)
+{
+ struct otpc_priv *priv = context;
+ u32 *buf = val;
+ u32 bytes_written;
+ u32 address = offset / priv->config->word_size;
+ int i, ret;
+
+ if (offset % priv->config->word_size)
+ return -EINVAL;
+
+ ret = enable_ocotp_program(priv->base);
+ if (ret)
+ return -EIO;
+
+ for (bytes_written = 0; bytes_written < bytes;) {
+ set_command(priv->base, OTPC_CMD_PROGRAM);
+ set_cpu_address(priv->base, address++);
+ for (i = 0; i < priv->map->otpc_row_size; i++) {
+ writel(*buf, priv->base + priv->map->data_r_offset[i]);
+ buf++;
+ bytes_written += sizeof(*buf);
+ }
+ set_start_bit(priv->base);
+ ret = poll_cpu_status(priv->base, OTPC_STAT_CMD_DONE);
+ reset_start_bit(priv->base);
+ if (ret) {
+ dev_err(priv->dev, "otp write error: 0x%x", ret);
+ return -EIO;
+ }
+ }
+
+ disable_ocotp_program(priv->base);
+
+ return 0;
+}
+
+static struct nvmem_config bcm_otpc_nvmem_config = {
+ .name = "bcm-ocotp",
+ .read_only = false,
+ .word_size = 4,
+ .stride = 4,
+ .owner = THIS_MODULE,
+ .reg_read = bcm_otpc_read,
+ .reg_write = bcm_otpc_write,
+};
+
+static const struct of_device_id bcm_otpc_dt_ids[] = {
+ { .compatible = "brcm,ocotp" },
+ { .compatible = "brcm,ocotp-v2" },
+ { },
+};
+MODULE_DEVICE_TABLE(of, bcm_otpc_dt_ids);
+
+static int bcm_otpc_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct device_node *dn = dev->of_node;
+ struct resource *res;
+ struct otpc_priv *priv;
+ struct nvmem_device *nvmem;
+ int err;
+ u32 num_words;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ if (of_device_is_compatible(dev->of_node, "brcm,ocotp"))
+ priv->map = &otp_map;
+ else if (of_device_is_compatible(dev->of_node, "brcm,ocotp-v2"))
+ priv->map = &otp_map_v2;
+ else {
+ dev_err(&pdev->dev,
+ "%s otpc config map not defined\n", __func__);
+ return -EINVAL;
+ }
+
+ /* Get OTP base address register. */
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ priv->base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(priv->base)) {
+ dev_err(dev, "unable to map I/O memory\n");
+ return PTR_ERR(priv->base);
+ }
+
+ /* Enable CPU access to OTPC. */
+ writel(readl(priv->base + OTPC_MODE_REG_OFFSET) |
+ BIT(OTPC_MODE_REG_OTPC_MODE),
+ priv->base + OTPC_MODE_REG_OFFSET);
+ reset_start_bit(priv->base);
+
+ /* Read size of memory in words. */
+ err = of_property_read_u32(dn, "brcm,ocotp-size", &num_words);
+ if (err) {
+ dev_err(dev, "size parameter not specified\n");
+ return -EINVAL;
+ } else if (num_words == 0) {
+ dev_err(dev, "size must be > 0\n");
+ return -EINVAL;
+ }
+
+ bcm_otpc_nvmem_config.size = 4 * num_words;
+ bcm_otpc_nvmem_config.dev = dev;
+ bcm_otpc_nvmem_config.priv = priv;
+
+ if (of_device_is_compatible(dev->of_node, "brcm,ocotp-v2")) {
+ bcm_otpc_nvmem_config.word_size = 8;
+ bcm_otpc_nvmem_config.stride = 8;
+ }
+
+ priv->config = &bcm_otpc_nvmem_config;
+
+ nvmem = nvmem_register(&bcm_otpc_nvmem_config);
+ if (IS_ERR(nvmem)) {
+ dev_err(dev, "error registering nvmem config\n");
+ return PTR_ERR(nvmem);
+ }
+
+ platform_set_drvdata(pdev, nvmem);
+
+ return 0;
+}
+
+static int bcm_otpc_remove(struct platform_device *pdev)
+{
+ struct nvmem_device *nvmem = platform_get_drvdata(pdev);
+
+ return nvmem_unregister(nvmem);
+}
+
+static struct platform_driver bcm_otpc_driver = {
+ .probe = bcm_otpc_probe,
+ .remove = bcm_otpc_remove,
+ .driver = {
+ .name = "brcm-otpc",
+ .of_match_table = bcm_otpc_dt_ids,
+ },
+};
+module_platform_driver(bcm_otpc_driver);
+
+MODULE_DESCRIPTION("Broadcom OTPC driver");
+MODULE_LICENSE("GPL v2");
--
1.9.1
^ permalink raw reply related
* [PATCH v1 3/3] ARM: dts: Add node for Broadcom OTP controller driver
From: Jonathan Richardson @ 2016-10-24 19:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477336324-10543-1-git-send-email-jonathan.richardson@broadcom.com>
From: Jonathan Richardson <jonathar@broadcom.com>
Reviewed-by: Ray Jui <ray.jui@broadcom.com>
Tested-by: Jonathan Richardson <jonathan.richardson@broadcom.com>
Signed-off-by: Scott Branden <scott.branden@broadcom.com>
Signed-off-by: Oza Pawandeep <oza@broadcom.com>
Signed-off-by: Jonathan Richardson <jonathan.richardson@broadcom.com>
---
arch/arm/boot/dts/bcm-cygnus.dtsi | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/arm/boot/dts/bcm-cygnus.dtsi b/arch/arm/boot/dts/bcm-cygnus.dtsi
index fabc9f3..a74a430 100644
--- a/arch/arm/boot/dts/bcm-cygnus.dtsi
+++ b/arch/arm/boot/dts/bcm-cygnus.dtsi
@@ -91,6 +91,13 @@
#address-cells = <1>;
#size-cells = <1>;
+ otp: otp at 0301c800 {
+ compatible = "brcm,ocotp";
+ reg = <0x0301c800 0x2c>;
+ brcm,ocotp-size = <2048>;
+ status = "disabled";
+ };
+
pcie_phy: phy at 0301d0a0 {
compatible = "brcm,cygnus-pcie-phy";
reg = <0x0301d0a0 0x14>;
--
1.9.1
^ permalink raw reply related
* [PATCH] ahci: use pci_alloc_irq_vectors
From: Christoph Hellwig @ 2016-10-24 19:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <580E59AA.60107@gmail.com>
Hi David,
On Mon, Oct 24, 2016 at 11:57:46AM -0700, David Daney wrote:
>> If so can you try the patch below?
>
> I applied it to v4.9-rc1 (really commit
> 6f33d6458e35d6ba53c2635ee4b8a3177cbd912d), and this didn't seem to make it
> work.
Please try on top of the libata for-4.9-fixes branch:
https://git.kernel.org/cgit/linux/kernel/git/tj/libata.git/log/?h=for-4.9-fixes
^ permalink raw reply
* [PATCH/RESEND] thermal: qcom-spmi: Treat reg property as a single cell
From: Andy Gross @ 2016-10-24 19:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161018234019.5489-1-sboyd@codeaurora.org>
On Tue, Oct 18, 2016 at 04:40:19PM -0700, Stephen Boyd wrote:
> We only read the first element of the reg property to figure out
> the offset of the temperature sensor inside the PMIC.
> Furthermore, we want to remove the second element in DT, so just
> don't read the second element so that probe keeps working if we
> change the DT in the future.
>
> Cc: Ivan T. Ivanov <iivanov.xz@gmail.com>
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
Reviewed-by: Andy Gross <andy.gross@linaro.org>
^ permalink raw reply
* [PATCH 0/5] Switch to the DT cpufreq policy on the Integrator
From: Linus Walleij @ 2016-10-24 19:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <6720864.e2sjvVVhdQ@wuerfel>
On Wed, Oct 19, 2016 at 12:14 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> I don't see any hard dependency here, so I'd suggest to merge the
> two cpufreq patches through the subsystem tree, and the ARM patches
> through arm-soc.
That's right, Viresh can you merge patches 1 and 5 to the
cpufreq tree please?
I will funnel patches 2, 3 and 4 to ARM SoC.
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH v1 0/3] Add support for Broadcom OTP controller
From: Linus Torvalds @ 2016-10-24 19:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477336324-10543-1-git-send-email-jonathan.richardson@broadcom.com>
On Mon, Oct 24, 2016 at 12:12 PM, Jonathan Richardson
<jonathan.richardson@broadcom.com> wrote:
> This patch set adds support for Broadcom's OTP controller found on chips such
> as Cygnus and Stingray. A node has been added to the Cygnus dts.
These patches fail DKIM and will thus be marked as spam for a lot of people.
The usual reason tends to be that you use the wrong smtp server that
doesn't add the right signature. That's happened before with
broadcom.com addresses.
Linus
^ permalink raw reply
* [PATCH v14 00/16] KVM PCIe/MSI passthrough on ARM/ARM64
From: Robin Murphy @ 2016-10-24 19:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <8132b350-59ff-0bfb-9539-e98e63d1adb6@redhat.com>
On 21/10/16 10:26, Auger Eric wrote:
> Hi Will,
>
> On 20/10/2016 19:32, Will Deacon wrote:
>> Hi Eric,
>>
>> Thanks for posting this.
>>
>> On Wed, Oct 12, 2016 at 01:22:08PM +0000, Eric Auger wrote:
>>> This is the second respin on top of Robin's series [1], addressing Alex' comments.
>>>
>>> Major changes are:
>>> - MSI-doorbell API now is moved to DMA IOMMU API following Alex suggestion
>>> to put all API pieces at the same place (so eventually in the IOMMU
>>> subsystem)
>>> - new iommu_domain_msi_resv struct and accessor through DOMAIN_ATTR_MSI_RESV
>>> domain with mirror VFIO capability
>>> - more robustness I think in the VFIO layer
>>> - added "iommu/iova: fix __alloc_and_insert_iova_range" since with the current
>>> code I failed allocating an IOVA page in a single page domain with upper part
>>> reserved
>>>
>>> IOVA range exclusion will be handled in a separate series
>>>
>>> The priority really is to discuss and freeze the API and especially the MSI
>>> doorbell's handling. Do we agree to put that in DMA IOMMU?
>>>
>>> Note: the size computation does not take into account possible page overlaps
>>> between doorbells but it would add quite a lot of complexity i think.
>>>
>>> Tested on AMD Overdrive (single GICv2m frame) with I350 VF assignment.
>>
>> Marc, Robin and I sat down and had a look at the series and, whilst it's
>> certainly addressing a problem that we desperately want to see fixed, we
>> think that it's slightly over-engineering in places and could probably
>> be simplified in the interest of getting something upstream that can be
>> used as a base, on which the ABI can be extended as concrete use-cases
>> become clear.
>>
>> Stepping back a minute, we're trying to reserve some of the VFIO virtual
>> address space so that it can be used by devices to map their MSI doorbells
>> using the SMMU. With your patches, this requires that (a) the kernel
>> tells userspace about the size and alignment of the doorbell region
>> (MSI_RESV) and (b) userspace tells the kernel the VA-range that can be
>> used (RESERVED_MSI_IOVA).
>>
>> However, this is all special-cased for MSI doorbells and there are
>> potentially other regions of the VFIO address space that are reserved
>> and need to be communicated to userspace as well. We already know of
>> hardware where the PCI RC intercepts p2p accesses before they make it
>> to the SMMU, and other hardware where the MSI doorbell is at a fixed
>> address. This means that we need a mechanism to communicate *fixed*
>> regions of virtual address space that are reserved by VFIO. I don't
>> even particularly care if VFIO_MAP_DMA enforces that, but we do need
>> a way to tell userspace "hey, you don't want to put memory here because
>> it won't work well with devices".
>
> I think we all agree on this. Exposing an API to the user space
> reporting *fixed* reserved IOVA ranges is a requirement anyway. The
> problem was quite clearly stated by Alex in
> http://lkml.iu.edu/hypermail/linux/kernel/1610.0/03308.html
> (VFIO_IOMMU_TYPE1_INFO_CAP_IOVA_RANGE)
>
> I started working on this VFIO_IOMMU_TYPE1_INFO_CAP_IOVA_RANGE
> capability but to me and I think according to Alex, it was a different
> API from MSI_RESV.
>
>>
>> In that case, we end up with something like your MSI_RESV capability,
>> but actually specifying a virtual address range that is simply not to
>> be used by MAP_DMA -- we don't say anything about MSIs. Now, taking this
>> to its logical conclusion, we no longer need to distinguish between
>> remappable reserved regions and fixed reserved regions in the ABI.
>> Instead, we can have the kernel allocate the virtual address space for
>> the remappable reserved regions (probably somewhere in the bottom 4GB)
>> and expose them via the capability.
>
>
> If I understand correctly you want the host to arbitrarily choose where
> it puts the iovas reserved for MSI and not ask the userspace.
>
> Well so we are back to the discussions we had in Dec 2015 (see Marc's
> answer in http://thread.gmane.org/gmane.comp.emulators.kvm.arm.devel/3858).
To an extent, yes, however the difference is that we now know we
definitely have to deal with situations in which userspace *cannot* be
in total control of the memory map, and that changes the game:
_________
/ \
/ Fixed \
/ things (A) \
( _________ )
\ / MSI \ /
X doorbells X
/ \___(B)___/ \
( )
\ Remappable /
\ things (C)/
\_________/
In the absence of A, then B == C so it was very hard to not want to
implement C. As soon as A *has* to be implemented for other reasons,
then that is also sufficient to encompass B. C can still be implemented
later as a nice-to-have, but is not necessary to get B off the ground.
> - So I guess you will init an iova_domain seomewhere below the 4GB to
> allocate the MSIs. what size are you going to choose. Don't you have the
> same need to dimension the iova range.
> - we still need to assess the MSI assignment safety. How will we compute
> safety for VFIO?
Absolutely. We're talking in general terms of the userspace ABI here,
although that can't help but colour the underlying implementation
decisions. Of course the VFIO internals still have to handle the
specific case of MSIs, but that's basically no more than this:
static bool msi_isolation = true; /* until proven otherwise */
static unsigned long msi_remap_virt_base = 0x08000000; /* fits QEMU */
static size_t msi_remap_size;
vfio_msi_thing_callback(thing) {
msi_remap_size += thing->info.size;
msi_isolation &= thing->info.flags & PROVIDES_ISOLATION;
}
vfio_msi_init(...) {
...
#ifdef CONFIG_X86
msi_remap_virt_base = 0xfee00000;
msi_remap_size = 0x100000;
msi_isolation = irq_remapping_enabled;
#else
irq_for_each_msi_thing(vfio_msi_thing_callback);
#endif
...
}
vfio_attach_group(...) {
...
if (!msi_isolation && !allow_unsafe_interrupts)
return -ENOWAY;
...
get_msi_region_cookie(domain, msi_remap_base, msi_remap_size);
...
}
And when a well-behaved userspace queries the reserved regions, that
base address and size is just one of potentially several that it should
get back. It's that "querying the reserved regions" bit that needs to be
gotten right first time.
Note that at this point I'm no longer even overly bothered about the
details of irq_for_each_msi_thing(), as it's an internal kernel
interface and thus malleable, although obviously the simpler the better.
I have to say Punit's idea of iterating irq_domains does actually look
really neat and tidy as a proof-of-concept, and also makes me think off
on a tangent that it would be sweet to be able to retrieve base+size
from dev->msi_domain to pre-allocate MSI pages in default domains, and
obviate the compose 'failure' case.
> This simplifies things in the
>> following ways:
>>
>> * You don't need to keep track of MSI vs DMA addresses in the VFIO rbtree
> right: I guess you rely on iommu_map to return an error in case the iova
> is already mapped somewhere else.
>> * You don't need to try collapsing doorbells into a single region
> why? at host level I guess you will init a single iova domain?
Yeah, right now this one goes either way - as things stand, it does make
life easier on the host side to make a single region to hang off the
back of the current iova_cookie magic, and as illustrated above it's
possibly the most trivial part of the whole thing, but the point is we
still don't *need* to. Since a userspace ABI for generic reservations
has to be able handle more than one region for the sake of non-MSI
things, we'd be free to change the kernel-side implementation in future
to just report multiple doorbells as individual regions - for starters,
if and when we add dynamic reservations and userspace gets to pick its
own IOVAs for those, it'll be a damn sight easier *not* to coalesce
everything.
>> * You don't need a special MAP flavour to map MSI doorbells
> right
>> * The ABI is reusable for PCI p2p and fixed doorbells
> right
>
> Aren't we moving the issue at user-space? Currently QEMU mach-virt
> address space is fully static. Adapting mach-virt to adjust to host
> constraints is not straightforward. It is simple to reject the
> assignment in case of collision but more difficult to react positively.
The point is that we *have* to move at least some of the issue to
userspace, and by then I'm struggling to see any real difference between
these situations:
a) QEMU asks VFIO to map some pages for DMA, gets an error back because
VFIO detects it conflicts with a reserved region, and gives up.
b) QEMU starts by asking VFIO what regions are reserved, realises they
will overlap with its hard-coded RAM address, and gives up.
where (a) requires a bunch of kernel machinery to second-guess
userspace, while (b) simply relies on userspace not being broken. And if
userspace fails at not being broken, then we simply retain the behaviour
which actually happens right now:
c) QEMU maps some pages for DMA at the same address as PCI config space
on the underlying hardware. Hilarity ensues.
Of course, userspace could be anything other than QEMU as well, so it's
not necessarily second-guessable at all; maybe we make the arbitrary
msi_remap_virt_base a VFIO module parameter to be more accommodating.
Who knows, maybe it turns out that's enough to keep users happy and we
never need to implement fully dynamic reservations.
Robin.
>> I really think it would make your patch series both generally useful and
>> an awful lot smaller, whilst leaving the door open to ABI extension on
>> a case-by-case basis when we determine that it's really needed.
>
> I would like to have a better understanding of how you assess the
> security and dimension the iova domain. This is the purpose of msi
> doorbell registration, which is not neat at all I acknowledge but well I
> did not find any other solution and did not get any other suggestion.
> Besides I think the per-cpu thing is over-engineered and this can
> definitively be simplified.
>
> VFIO part was reviewed by Alex and I don't have the impression that this
> is the blocking part. besides there is on iova.c fix,
> IOMMU_CAP_INTR_REMAP removal; so is it really over-complicated?
>
> Thanks
>
> Eric
>
>>
>> Thoughts?
>>
>> Will
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>
>
^ permalink raw reply
* [PATCH 0/7] ARM: dts: support I2SE Duckbill device
From: Michael Heimpold @ 2016-10-24 19:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161024131209.GK30578@tiger>
Am Montag, 24. Oktober 2016, 21:12:09 CEST schrieb Shawn Guo:
> On Sat, Oct 22, 2016 at 09:22:23PM +0200, Michael Heimpold wrote:
> > This series updates/adds Device Tree support for I2SE's Duckbill
> > device family.
> >
> > The Duckbill devices are small, pen-drive sized boards based on
> > NXP's i.MX28 SoC. While the initial variants (Duckbill series) were
> > equipped with a micro SD card slot only, the latest generation
> > (Duckbill 2 series) have an additional internal eMMC onboard.
> >
> > Both device generations consists of four "family members":
> >
> > - Duckbill/Duckbill 2: generic board, intented to be used as
> >
> > baseboard for custom designs and/or as development board
> >
> > - Duckbill EnOcean/Duckbill 2 EnOcean: come with an EnOcean
> >
> > daugther board equipped with the popular TCM310 module
> >
> > - Duckbill 485/Duckbill 2 485: as the name implies, these
> >
> > devices are intended to be used as Ethernet - RS485 converters
> >
> > - Duckbill SPI/Duckbill 2 SPI: not sold separately, but used
> >
> > in I2SE's development kits for Green PHY HomePlug Powerline
> > communication
>
> So we basically need to maintain 8 dts/dtb files for imx28-duckbill
> board. This is not pleasant. Can you please investigate if Device Tree
> overlay is going to help here?
First, thank you very much for your quick feedback.
Actually it would even be 11 files for 8 boards, thats right.
There is no mechanism (no EEPROM, GPIO...) to automatically probe for the
device variants, and the "variant-forming daugther boards" are
not switchable like the expansion boards e.g. for Raspberry Pi.
Another point is, that this would really require to use U-Boot to apply
the overlay (or another DT capable bootloader with this functionality).
For booting very fast, the good old Freescale bootlets are still
an option, which could not handle the overlays - at least at the moment...
However, it still an option to have a second look...
To reduce the number of files, I see the following other approaches:
- don't use the three "common" files which are included: -3 files (8 total)
- don't mainline Duckbill EnOcean, Duckbill 485 and Duckbill SPI:
they are EOL, only Duckbill 2 variants are still sold;
however, since Duckbill (without 2) is/was already mainlined, we
should keep this, leaving 4 new variants + 1 old variant: -3 files (5 total)
- only mainline Duckbill 2 (generic board) and maintain all other variants
in a private Github repo (this is what we are doing now): -3 files (2 total)
So, I'll discuss this again internally. Do you have any preference?
I was also told off-list, that there should be an additional file in
Documentation/devicetree/bindings/arm to document the imx28-duckbill
binding.
And DT maintainers don't like the simple-bus stuff around the regulators.
So, I've already material to fix for v2 :-)
Michael
>
> Shawn
>
> [1] http://free-electrons.com/blog/dt-overlay-uboot-libfdt/
>
> > Since all devices are very similar and only differ in few
> > aspects, the following patch series introduces first common
> > device tree snippets which are then included by the real
> > devices. For better understanding, I tried to illustrate the
> > hierarchy:
> >
> > ...
> > [removed brocken ASCII art]
> > ...
> >
> > Michael Heimpold (7):
> > ARM: dts: imx28: add alternative pinmuxing for mmc2
> > ARM: dts: imx28: rename mmc2_sck_cfg to prepare for an alternative
> >
> > muxing setup
> >
> > ARM: dts: imx28: add alternative muxing for mmc2_sck_cfg
> > ARM: dts: add I2SE Duckbill common definitions
> > ARM: dts: duckbill: simplify DT and use common definitions
> > ARM: dts: add support for remaining members of Duckbill series
> > ARM: dts: add support for Duckbill 2 series devices
> >
> > arch/arm/boot/dts/Makefile | 7 ++
> > arch/arm/boot/dts/imx28-duckbill-2-485.dts | 70 ++++++++++++++++
> > arch/arm/boot/dts/imx28-duckbill-2-common.dtsi | 110
> > +++++++++++++++++++++++++ arch/arm/boot/dts/imx28-duckbill-2-enocean.dts
> > | 100 ++++++++++++++++++++++ arch/arm/boot/dts/imx28-duckbill-2-spi.dts
> > | 63 ++++++++++++++ arch/arm/boot/dts/imx28-duckbill-2.dts |
> > 46 +++++++++++
> > arch/arm/boot/dts/imx28-duckbill-485.dts | 60 ++++++++++++++
> > arch/arm/boot/dts/imx28-duckbill-base.dtsi | 88 ++++++++++++++++++++
> > arch/arm/boot/dts/imx28-duckbill-common.dtsi | 80 ++++++++++++++++++
> > arch/arm/boot/dts/imx28-duckbill-enocean.dts | 90 ++++++++++++++++++++
> > arch/arm/boot/dts/imx28-duckbill-spi.dts | 64 ++++++++++++++
> > arch/arm/boot/dts/imx28-duckbill.dts | 99
> > +++-------------------
> > arch/arm/boot/dts/imx28-m28cu3.dts | 2 +-
> > arch/arm/boot/dts/imx28.dtsi | 28 ++++++-
> > 14 files changed, 817 insertions(+), 90 deletions(-)
> > create mode 100644 arch/arm/boot/dts/imx28-duckbill-2-485.dts
> > create mode 100644 arch/arm/boot/dts/imx28-duckbill-2-common.dtsi
> > create mode 100644 arch/arm/boot/dts/imx28-duckbill-2-enocean.dts
> > create mode 100644 arch/arm/boot/dts/imx28-duckbill-2-spi.dts
> > create mode 100644 arch/arm/boot/dts/imx28-duckbill-2.dts
> > create mode 100644 arch/arm/boot/dts/imx28-duckbill-485.dts
> > create mode 100644 arch/arm/boot/dts/imx28-duckbill-base.dtsi
> > create mode 100644 arch/arm/boot/dts/imx28-duckbill-common.dtsi
> > create mode 100644 arch/arm/boot/dts/imx28-duckbill-enocean.dts
> > create mode 100644 arch/arm/boot/dts/imx28-duckbill-spi.dts
^ permalink raw reply
* [PATCH v5 02/23] of: device: Export of_device_{get_modalias, uvent_modalias} to modules
From: Stephen Boyd @ 2016-10-24 19:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAGb2v66t8t3o0PxzaaBKnRvNP=CWG5nYud7Wjp27Q1x2baM4hg@mail.gmail.com>
Quoting Chen-Yu Tsai (2016-10-24 05:19:05)
> Hi,
>
> On Tue, Oct 18, 2016 at 9:56 AM, Stephen Boyd <stephen.boyd@linaro.org> wrote:
> > The ULPI bus can be built as a module, and it will soon be
> > calling these functions when it supports probing devices from DT.
> > Export them so they can be used by the ULPI module.
> >
> > Acked-by: Rob Herring <robh@kernel.org>
> > Cc: <devicetree@vger.kernel.org>
> > Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
> > ---
> > drivers/of/device.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/of/device.c b/drivers/of/device.c
> > index 8a22a253a830..6719ab35b62e 100644
> > --- a/drivers/of/device.c
> > +++ b/drivers/of/device.c
> > @@ -225,6 +225,7 @@ ssize_t of_device_get_modalias(struct device *dev, char *str, ssize_t len)
> >
> > return tsize;
> > }
> > +EXPORT_SYMBOL_GPL(of_device_get_modalias);
> >
> > int of_device_request_module(struct device *dev)
> > {
> > @@ -290,6 +291,7 @@ void of_device_uevent(struct device *dev, struct kobj_uevent_env *env)
> > }
> > mutex_unlock(&of_mutex);
> > }
> > +EXPORT_SYMBOL_GPL(of_device_uevent_modalias);
>
> This is trailing the wrong function.
>
Good catch. Must have been some bad rebase.
Peter, can you fix it while applying or should I resend this patch?
^ permalink raw reply
* [PATCH v3 3/6] dt-bindings: pinctrl: Deprecate sunxi pinctrl bindings
From: Maxime Ripard @ 2016-10-24 19:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACRpkdb7zs7Nhs3i1o=WUt787vJCuePre3Tj3wLu++qwpO0atQ@mail.gmail.com>
Hi Linus,
On Mon, Oct 24, 2016 at 02:03:59AM +0200, Linus Walleij wrote:
> On Thu, Oct 20, 2016 at 3:49 PM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
>
> > The generic pin configuration and multiplexing should be preferred now,
> > even though we still support the old one.
> >
> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> > Acked-by: Chen-Yu Tsai <wens@csie.org>
> > Acked-by: Rob Herring <robh@kernel.org>
>
> Patch applied.
Thanks a lot.
However, it looks like the first patch from this serie is missing from
your tree, is there a reason for that?
Also, in order to preserve bisectability, could you create an
immutable branch for those sunxi patches so that I can merge the DT
bits?
Thanks,
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161024/b378beb9/attachment.sig>
^ permalink raw reply
* [PATCH 5/5] ARM: dts: Add LEGO MINDSTORTMS EV3 dts
From: David Lechner @ 2016-10-24 19:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1593441f-d147-4c91-8aab-8622dd8ced19@lechnology.com>
On 10/24/2016 10:50 AM, David Lechner wrote:
> On 10/24/2016 06:58 AM, Sekhar Nori wrote:
>> On Saturday 22 October 2016 12:06 AM, David Lechner wrote:
>>
>>> +&ehrpwm1 {
>>> + status = "disabled";
>>
>> Hmm, disabled? Can you add this node when you actually use it?
>
> Not sure why I have this disabled. Like the gpios, the pwms can be used
> via sysfs, so I would like to leave them.
>
Now I remember why these are disabled. The clock matching is broken.
Only the first ehrpwm and the first ecap get clocks. The others fail.
I can change these to "okay". It will just result in a kernel error
message until the clocks are fixed.
^ permalink raw reply
* [PATCH v1 0/3] Add support for Broadcom OTP controller
From: Florian Fainelli @ 2016-10-24 19:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAADWXX-43-k2K2T1V5MQMd0z0V502wf1syNHERPfvNstOJw-3A@mail.gmail.com>
On 10/24/2016 12:39 PM, Linus Torvalds wrote:
> On Mon, Oct 24, 2016 at 12:12 PM, Jonathan Richardson
> <jonathan.richardson@broadcom.com> wrote:
>> This patch set adds support for Broadcom's OTP controller found on chips such
>> as Cygnus and Stingray. A node has been added to the Cygnus dts.
>
> These patches fail DKIM and will thus be marked as spam for a lot of people.
>
> The usual reason tends to be that you use the wrong smtp server that
> doesn't add the right signature. That's happened before with
> broadcom.com addresses.
The older setup was using smtphost.broadcom.com which we have now
documented as being invalid, here Jonathan used gmail directly (since
that's our mail provider now):
Received: from lbrmn-lnxub108.corp.ad.broadcom.com ([216.31.219.19])
by smtp.gmail.com with ESMTPSA id
s89sm8325746qkl.44.2016.10.24.12.12.00
(version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128);
Mon, 24 Oct 2016 12:12:03 -0700 (PDT)
Is there something else we need to check? Here is what I read for the
cover-letter:
Authentication-Results: mx.google.com;
dkim=pass header.i=@broadcom.com;
spf=pass (google.com: domain of
bcm-kernel-feedback-list.pdl+bncbdh5xfvr4ydrbbn2xhaakgqed7hz4rq at broadcom.com
designates 2607:f8b0:400c:c08::247 as permitted sender)
smtp.mailfrom=bcm-kernel-feedback-list.pdl+bncBDH5XFVR4YDRBBN2XHAAKGQED7HZ4RQ at broadcom.com;
dmarc=pass (p=QUARANTINE dis=NONE) header.from=broadcom.com
--
Florian
^ permalink raw reply
* [PATCH] nvmem: sunxi-sid: SID content is not a valid source of randomness
From: Maxime Ripard @ 2016-10-24 20:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477144408-15896-1-git-send-email-clabbe.montjoie@gmail.com>
On Sat, Oct 22, 2016 at 03:53:28PM +0200, Corentin Labbe wrote:
> Since SID's content is constant over reboot,
That's not true, at least not across all the Allwinner SoCs, and
especially not on the A10 and A20 that this driver supports.
> it must not be used as source of randomness.
And I don't think that's true either. A constant entropy provider will
not add any entropy, but will not remove any, would it?
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161024/1739c38f/attachment.sig>
^ permalink raw reply
* [PATCH v2 2/2] power: bq27xxx_battery: add poll interval property query
From: Pavel Machek @ 2016-10-24 20:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAKzfze9uvMvrF=gWph_HKFk_cH0qzRz6Wa2VJafK+H+wdnP0rg@mail.gmail.com>
On Mon 2016-10-24 12:58:25, Matt Ranostay wrote:
> Pavel + Sebastian this is the patchset that need I some input on :)
Better then previous one.
But my version of bq27xxx_battery.c already contains this:
static const struct kernel_param_ops param_ops_poll_interval = {
.get = param_get_uint,
.set = poll_interval_param_set,
};
...so it should be possible to set poll interval already.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161024/31406715/attachment.sig>
^ permalink raw reply
* [PATCH v1 0/3] Add support for Broadcom OTP controller
From: Linus Torvalds @ 2016-10-24 20:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <e34f01b6-a059-9988-0f93-e5adb82a5436@gmail.com>
On Mon, Oct 24, 2016 at 12:54 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
>
> The older setup was using smtphost.broadcom.com which we have now
> documented as being invalid, here Jonathan used gmail directly (since
> that's our mail provider now):
>
> Received: from lbrmn-lnxub108.corp.ad.broadcom.com ([216.31.219.19])
> by smtp.gmail.com with ESMTPSA id
> s89sm8325746qkl.44.2016.10.24.12.12.00
> (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128);
> Mon, 24 Oct 2016 12:12:03 -0700 (PDT)
Hmm. I get that too, so if that's the right thing for a broadcom.com
address, it's not the smtp server issue.
We had a few cases of the kernel mailing list itself messing up emails
sufficiently to fail dkim, but that shouldn't be an issue for the
relaxed/relaxed model that broadcom uses (the vger mailing list
software screws up whitespace, which "relaxed" ignores).
> Is there something else we need to check? Here is what I read for the
> cover-letter:
>
> Authentication-Results: mx.google.com;
> dkim=pass header.i=@broadcom.com;
> spf=pass (google.com: domain of ...
Hmm. I get:
Authentication-Results: mx.google.com;
dkim=fail header.i=@broadcom.com;
with the actual dkim signature looking like this:
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=broadcom.com; s=google;
h=from:to:cc:subject:date:message-id;
bh=9zStGnsZQDQqP6cm1CHPk7EYVtLvDsm2wN5qy5Mgx7M=;
b=Z/1QD+FwJogJY9D8Qd197Q+VJt7Tr9+WoHFeKYRL00yhvxrMg0P8jKj1FbucJTluvM
agC2eq9qCpZcNAfridjExDRDCuUPAIJIXTr9Npkpqlk6gEMq2FysrGer2D9Z4HQ/atTX
67VirFsQK0gK7impYMn9kW5Q9BIIw5bOg7OdI=
and those fields that it protects look like this:
From: Jonathan Richardson <jonathan.richardson@broadcom.com>
To: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>, Maxime
Ripard <maxime.ripard@free-electrons.com>
Cc: linux-arm-kernel at lists.infradead.org,
linux-kernel at vger.kernel.org, devicetree at vger.kernel.org, Mark Rutland
<mark.rutland@arm.com>, Rob Herring <robh+dt@kernel.org>, Scott
Branden <sbranden@broadcom.com>, Ray Jui <rjui@broadcom.com>,
bcm-kernel-feedback-list at broadcom.com, Jonathan Richardson
<jonathan.richardson@broadcom.com>
Subject: [PATCH v1 0/3] Add support for Broadcom OTP controller
Date: Mon, 24 Oct 2016 12:12:01 -0700
Message-Id: <1477336324-10543-1-git-send-email-jonathan.richardson@broadcom.com>
and I don't see anything obviously wrong anywhere - except for that
"dkim=fail" thing, and the email being in my spam folder.
Linus
^ permalink raw reply
* [PATCH 0/5] Switch to the DT cpufreq policy on the Integrator
From: Rafael J. Wysocki @ 2016-10-24 20:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACRpkdZvUh0Z0kxu+WmvwqH3kuaOxaP1Cgxv0WUE2PT-eLs62w@mail.gmail.com>
On Mon, Oct 24, 2016 at 9:28 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Wed, Oct 19, 2016 at 12:14 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>
>> I don't see any hard dependency here, so I'd suggest to merge the
>> two cpufreq patches through the subsystem tree, and the ARM patches
>> through arm-soc.
>
> That's right, Viresh can you merge patches 1 and 5 to the
> cpufreq tree please?
Well, that would be me.
Can you please resend the two as a separate series, with all tags
collected so far if any?
Thanks,
Rafael
^ permalink raw reply
* [PATCH V4 01/10] acpi: apei: read ack upon ghes record consumption
From: Baicar, Tyler @ 2016-10-24 20:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <b4b69d66-ae37-a528-d64c-ee9b3fe7b02c@arm.com>
On 10/24/2016 2:51 AM, Suzuki K Poulose wrote:
> On 21/10/16 18:30, Tyler Baicar wrote:
>> A RAS (Reliability, Availability, Serviceability) controller
>> may be a separate processor running in parallel with OS
>> execution, and may generate error records for consumption by
>> the OS. If the RAS controller produces multiple error records,
>> then they may be overwritten before the OS has consumed them.
>>
>> The Generic Hardware Error Source (GHES) v2 structure
>> introduces the capability for the OS to acknowledge the
>> consumption of the error record generated by the RAS
>> controller. A RAS controller supporting GHESv2 shall wait for
>> the acknowledgment before writing a new error record, thus
>> eliminating the race condition.
>>
>> Signed-off-by: Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
>> Signed-off-by: Richard Ruigrok <rruigrok@codeaurora.org>
>> Signed-off-by: Tyler Baicar <tbaicar@codeaurora.org>
>> Signed-off-by: Naveen Kaje <nkaje@codeaurora.org>
>> ---
>> drivers/acpi/apei/ghes.c | 42
>> ++++++++++++++++++++++++++++++++++++++++++
>> drivers/acpi/apei/hest.c | 7 +++++--
>> include/acpi/ghes.h | 5 ++++-
>> 3 files changed, 51 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
>> index 60746ef..7d020b0 100644
>> --- a/drivers/acpi/apei/ghes.c
>> +++ b/drivers/acpi/apei/ghes.c
>> @@ -45,6 +45,7 @@
>> #include <linux/aer.h>
>> #include <linux/nmi.h>
>>
>> +#include <acpi/actbl1.h>
>> #include <acpi/ghes.h>
>> #include <acpi/apei.h>
>> #include <asm/tlbflush.h>
>> @@ -79,6 +80,10 @@
>> ((struct acpi_hest_generic_status *) \
>> ((struct ghes_estatus_node *)(estatus_node) + 1))
>>
>> +#define HEST_TYPE_GENERIC_V2(ghes) \
>> + ((struct acpi_hest_header *)ghes->generic)->type == \
>> + ACPI_HEST_TYPE_GENERIC_ERROR_V2
>> +
>> /*
>> * This driver isn't really modular, however for the time being,
>> * continuing to use module_param is the easiest way to remain
>> @@ -248,7 +253,15 @@ static struct ghes *ghes_new(struct
>> acpi_hest_generic *generic)
>> ghes = kzalloc(sizeof(*ghes), GFP_KERNEL);
>> if (!ghes)
>> return ERR_PTR(-ENOMEM);
>> +
>> ghes->generic = generic;
>> + if (HEST_TYPE_GENERIC_V2(ghes)) {
>> + rc = apei_map_generic_address(
>> + &ghes->generic_v2->read_ack_register);
>> + if (rc)
>> + goto err_unmap;
>
> I think should be goto err_free, see more below.
>
>> + }
>> +
>> rc = apei_map_generic_address(&generic->error_status_address);
>> if (rc)
>> goto err_free;
>> @@ -270,6 +283,9 @@ static struct ghes *ghes_new(struct
>> acpi_hest_generic *generic)
>>
>> err_unmap:
>> apei_unmap_generic_address(&generic->error_status_address);
>> + if (HEST_TYPE_GENERIC_V2(ghes))
>> + apei_unmap_generic_address(
>> + &ghes->generic_v2->read_ack_register);
>
> We might end up trying to unmap (error_status_address) which is not
> mapped
> if we hit the error in mapping read_ack_register. The
> read_ack_register unmap
> hunk should be moved below to err_free.
>
This needs to be changed, I'll add a separate label for unmapping
read_ack_register and error_status_address for the case that the
read_ack_register map succeeds but the error_status_address map fails.
err_unmap_status_addr:
apei_unmap_generic_address(&generic->error_status_address);
err_unmap_read_ack_addr:
if (HEST_TYPE_GENERIC_V2(ghes))
apei_unmap_generic_address(
&ghes->generic_v2->read_ack_register);
err_free:
kfree(ghes);
return ERR_PTR(rc);
If mapping read_ack_register fails, goto err_free.
If mapping read_ack_register is successful but mapping
error_status_address fails, goto err_unmap_read_ack_addr.
And if both mappings succeed but the kmalloc fails, then goto
err_unmap_status_addr.
>
>> err_free:
>> kfree(ghes);
>> return ERR_PTR(rc);
>> @@ -279,6 +295,9 @@ static void ghes_fini(struct ghes *ghes)
>> {
>> kfree(ghes->estatus);
>> apei_unmap_generic_address(&ghes->generic->error_status_address);
>> + if (HEST_TYPE_GENERIC_V2(ghes))
>> + apei_unmap_generic_address(
>> + &ghes->generic_v2->read_ack_register);
>> }
>>
>> static inline int ghes_severity(int severity)
>> @@ -648,6 +667,23 @@ static void ghes_estatus_cache_add(
>> rcu_read_unlock();
>> }
>>
>
>> +static int ghes_do_read_ack(struct acpi_hest_generic_v2 *generic_v2)
>
> nit: We are actually writing something to the read_ack_register. The
> names
> read_ack_register (which may be as per standard) and more importantly the
> function name (ghes_do_read_ack) sounds a bit misleading.
It is called "Read Ack Register" in the spec (ACPI 6.1 table 18-344),
but I agree the function name can be improved.
Maybe ghes_acknowledge_error or ghes_ack_error.
Thanks,
Tyler
>
> Rest looks fine to me.
>
> Suzuki
>
--
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] ahci: use pci_alloc_irq_vectors
From: David Daney @ 2016-10-24 20:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161022141123.GA25500@lst.de>
On 10/22/2016 07:11 AM, Christoph Hellwig wrote:
> Hi Robert,
>
> is this a controller that's using MSI-X?
>
> If so can you try the patch below?
>
After better understanding your request, I applied this against:
http://git.kernel.org/cgit/linux/kernel/git/tj/libata.git/log/?h=for-4.9-fixes
(commit a478b097474cd9f2268ab1beaca74ff09e652b9b)
This is now working on my ThunderX CRB-2S system.
You can add:
Tested-by: David Daney <david.daney@cavium.com>
Thanks for taking the time to work through this.
> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
> index ba5f11c..5fe852d 100644
> --- a/drivers/ata/ahci.c
> +++ b/drivers/ata/ahci.c
> @@ -1617,7 +1617,7 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
> /* legacy intx interrupts */
> pci_intx(pdev, 1);
> }
> - hpriv->irq = pdev->irq;
> + hpriv->irq = pci_irq_vector(pdev, 0);
>
> if (!(hpriv->cap & HOST_CAP_SSS) || ahci_ignore_sss)
> host->flags |= ATA_HOST_PARALLEL_SCAN;
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply
* [PATCH V4 02/10] ras: acpi/apei: cper: generic error data entry v3 per ACPI 6.1
From: Baicar, Tyler @ 2016-10-24 20:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <da257bb4-48f3-b187-1d43-c68669427cef@arm.com>
On 10/24/2016 3:50 AM, Suzuki K Poulose wrote:
> On 21/10/16 18:30, Tyler Baicar wrote:
>> Currently when a RAS error is reported it is not timestamped.
>> The ACPI 6.1 spec adds the timestamp field to the generic error
>> data entry v3 structure. The timestamp of when the firmware
>> generated the error is now being reported.
>>
>> Signed-off-by: Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
>> Signed-off-by: Richard Ruigrok <rruigrok@codeaurora.org>
>> Signed-off-by: Tyler Baicar <tbaicar@codeaurora.org>
>> Signed-off-by: Naveen Kaje <nkaje@codeaurora.org>
>> ---
>> drivers/acpi/apei/ghes.c | 14 +++++++---
>> drivers/firmware/efi/cper.c | 67
>> +++++++++++++++++++++++++++++++++++++--------
>> include/acpi/ghes.h | 10 +++++++
>> include/linux/cper.h | 12 ++++++++
>> 4 files changed, 88 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
>> index 7d020b0..7610f08 100644
>> --- a/drivers/acpi/apei/ghes.c
>> +++ b/drivers/acpi/apei/ghes.c
>> @@ -419,7 +419,8 @@ static void ghes_handle_memory_failure(struct
>> acpi_hest_generic_data *gdata, int
>> int flags = -1;
>> int sec_sev = ghes_severity(gdata->error_severity);
>> struct cper_sec_mem_err *mem_err;
>> - mem_err = (struct cper_sec_mem_err *)(gdata + 1);
>> +
>> + mem_err = acpi_hest_generic_data_payload(gdata);
>>
>> if (!(mem_err->validation_bits & CPER_MEM_VALID_PA))
>> return;
>> @@ -449,14 +450,18 @@ static void ghes_do_proc(struct ghes *ghes,
>> {
>> int sev, sec_sev;
>> struct acpi_hest_generic_data *gdata;
>> + uuid_le sec_type;
>>
>> sev = ghes_severity(estatus->error_severity);
>> apei_estatus_for_each_section(estatus, gdata) {
>> sec_sev = ghes_severity(gdata->error_severity);
>> - if (!uuid_le_cmp(*(uuid_le *)gdata->section_type,
>> + sec_type = *(uuid_le *)gdata->section_type;
>> +
>> + if (!uuid_le_cmp(sec_type,
>> CPER_SEC_PLATFORM_MEM)) {
>> struct cper_sec_mem_err *mem_err;
>> - mem_err = (struct cper_sec_mem_err *)(gdata+1);
>> +
>> + mem_err = acpi_hest_generic_data_payload(gdata);
>> ghes_edac_report_mem_error(ghes, sev, mem_err);
>>
>> arch_apei_report_mem_error(sev, mem_err);
>> @@ -466,7 +471,8 @@ static void ghes_do_proc(struct ghes *ghes,
>> else if (!uuid_le_cmp(*(uuid_le *)gdata->section_type,
>> CPER_SEC_PCIE)) {
>> struct cper_sec_pcie *pcie_err;
>> - pcie_err = (struct cper_sec_pcie *)(gdata+1);
>> +
>> + pcie_err = acpi_hest_generic_data_payload(gdata);
>> if (sev == GHES_SEV_RECOVERABLE &&
>> sec_sev == GHES_SEV_RECOVERABLE &&
>> pcie_err->validation_bits &
>> CPER_PCIE_VALID_DEVICE_ID &&
>> diff --git a/drivers/firmware/efi/cper.c b/drivers/firmware/efi/cper.c
>> index d425374..af7e1e9 100644
>> --- a/drivers/firmware/efi/cper.c
>> +++ b/drivers/firmware/efi/cper.c
>> @@ -32,6 +32,9 @@
>> #include <linux/acpi.h>
>> #include <linux/pci.h>
>> #include <linux/aer.h>
>> +#include <linux/printk.h>
>> +#include <linux/bcd.h>
>> +#include <acpi/ghes.h>
>>
>> #define INDENT_SP " "
>>
>> @@ -386,13 +389,37 @@ static void cper_print_pcie(const char *pfx,
>> const struct cper_sec_pcie *pcie,
>> pfx, pcie->bridge.secondary_status, pcie->bridge.control);
>> }
>>
>> +static void cper_estatus_print_section_v300(const char *pfx,
>> + const struct acpi_hest_generic_data_v300 *gdata)
>> +{
>> + __u8 hour, min, sec, day, mon, year, century, *timestamp;
>> +
>> + if (gdata->validation_bits & ACPI_HEST_GEN_VALID_TIMESTAMP) {
>> + timestamp = (__u8 *)&(gdata->time_stamp);
>> + sec = bcd2bin(timestamp[0]);
>> + min = bcd2bin(timestamp[1]);
>> + hour = bcd2bin(timestamp[2]);
>> + day = bcd2bin(timestamp[4]);
>> + mon = bcd2bin(timestamp[5]);
>> + year = bcd2bin(timestamp[6]);
>> + century = bcd2bin(timestamp[7]);
>> + printk("%stime: %7s %02d%02d-%02d-%02d %02d:%02d:%02d\n", pfx,
>> + 0x01 & *(timestamp + 3) ? "precise" : "", century,
>> + year, mon, day, hour, min, sec);
>> + }
>> +}
>> +
>> static void cper_estatus_print_section(
>> - const char *pfx, const struct acpi_hest_generic_data *gdata, int
>> sec_no)
>> + const char *pfx, struct acpi_hest_generic_data *gdata, int sec_no)
>> {
>> uuid_le *sec_type = (uuid_le *)gdata->section_type;
>> __u16 severity;
>> char newpfx[64];
>>
>> + if (acpi_hest_generic_data_version(gdata))
>> + cper_estatus_print_section_v300(pfx,
>> + (const struct acpi_hest_generic_data_v300 *)gdata);
>> +
>> severity = gdata->error_severity;
>> printk("%s""Error %d, type: %s\n", pfx, sec_no,
>> cper_severity_str(severity));
>> @@ -403,14 +430,18 @@ static void cper_estatus_print_section(
>>
>> snprintf(newpfx, sizeof(newpfx), "%s%s", pfx, INDENT_SP);
>> if (!uuid_le_cmp(*sec_type, CPER_SEC_PROC_GENERIC)) {
>> - struct cper_sec_proc_generic *proc_err = (void *)(gdata + 1);
>> + struct cper_sec_proc_generic *proc_err;
>> +
>> + proc_err = acpi_hest_generic_data_payload(gdata);
>> printk("%s""section_type: general processor error\n", newpfx);
>> if (gdata->error_data_length >= sizeof(*proc_err))
>> cper_print_proc_generic(newpfx, proc_err);
>> else
>> goto err_section_too_small;
>> } else if (!uuid_le_cmp(*sec_type, CPER_SEC_PLATFORM_MEM)) {
>> - struct cper_sec_mem_err *mem_err = (void *)(gdata + 1);
>> + struct cper_sec_mem_err *mem_err;
>> +
>> + mem_err = acpi_hest_generic_data_payload(gdata);
>> printk("%s""section_type: memory error\n", newpfx);
>> if (gdata->error_data_length >=
>> sizeof(struct cper_sec_mem_err_old))
>> @@ -419,7 +450,9 @@ static void cper_estatus_print_section(
>> else
>> goto err_section_too_small;
>> } else if (!uuid_le_cmp(*sec_type, CPER_SEC_PCIE)) {
>> - struct cper_sec_pcie *pcie = (void *)(gdata + 1);
>> + struct cper_sec_pcie *pcie;
>> +
>> + pcie = acpi_hest_generic_data_payload(gdata);
>> printk("%s""section_type: PCIe error\n", newpfx);
>> if (gdata->error_data_length >= sizeof(*pcie))
>> cper_print_pcie(newpfx, pcie, gdata);
>> @@ -438,6 +471,7 @@ void cper_estatus_print(const char *pfx,
>> const struct acpi_hest_generic_status *estatus)
>> {
>> struct acpi_hest_generic_data *gdata;
>> + struct acpi_hest_generic_data_v300 *gdata_v3 = NULL;
>> unsigned int data_len, gedata_len;
>> int sec_no = 0;
>> char newpfx[64];
>> @@ -451,12 +485,22 @@ void cper_estatus_print(const char *pfx,
>> printk("%s""event severity: %s\n", pfx,
>> cper_severity_str(severity));
>> data_len = estatus->data_length;
>> gdata = (struct acpi_hest_generic_data *)(estatus + 1);
>> + if (acpi_hest_generic_data_version(gdata))
>> + gdata_v3 = (struct acpi_hest_generic_data_v300 *)gdata;
>
> I think the acpi_hest_generic_data_version() doesn't check if the
> version is
> V3 or higher ?
Oops :) I need to make sure that returns 3.
>
>> +
>> snprintf(newpfx, sizeof(newpfx), "%s%s", pfx, INDENT_SP);
>> +
>> while (data_len >= sizeof(*gdata)) {
>> gedata_len = gdata->error_data_length;
>> cper_estatus_print_section(newpfx, gdata, sec_no);
>> - data_len -= gedata_len + sizeof(*gdata);
>> - gdata = (void *)(gdata + 1) + gedata_len;
>> + if(gdata_v3) {
>> + data_len -= gedata_len + sizeof(*gdata_v3);
>> + gdata_v3 = (void *)(gdata_v3 + 1) + gedata_len;
>> + gdata = (struct acpi_hest_generic_data *)gdata_v3;
>> + } else {
>> + data_len -= gedata_len + sizeof(*gdata);
>> + gdata = (void *)(gdata + 1) + gedata_len;
>> + }
>
> Could we not use the helpers we define below to unify the code here
> and avoid the
> switch ?
>
Yes, those helpers should be able to reduce the code here, I'll update this.
>
>> sec_no++;
>> }
>> }
>> @@ -486,12 +530,13 @@ int cper_estatus_check(const struct
>> acpi_hest_generic_status *estatus)
>> return rc;
>> data_len = estatus->data_length;
>> gdata = (struct acpi_hest_generic_data *)(estatus + 1);
>> - while (data_len >= sizeof(*gdata)) {
>> - gedata_len = gdata->error_data_length;
>> - if (gedata_len > data_len - sizeof(*gdata))
>> +
>> + while (data_len >= acpi_hest_generic_data_size(gdata)) {
>> + gedata_len = acpi_hest_generic_data_error_length(gdata);
>> + if (gedata_len > data_len - acpi_hest_generic_data_size(gdata))
>> return -EINVAL;
>> - data_len -= gedata_len + sizeof(*gdata);
>> - gdata = (void *)(gdata + 1) + gedata_len;
>> + data_len -= gedata_len + acpi_hest_generic_data_size(gdata);
>> + gdata = acpi_hest_generic_data_next(gdata);
>> }
>> if (data_len)
>> return -EINVAL;
>> diff --git a/include/acpi/ghes.h b/include/acpi/ghes.h
>> index 68f088a..56b9679 100644
>> --- a/include/acpi/ghes.h
>> +++ b/include/acpi/ghes.h
>> @@ -73,3 +73,13 @@ static inline void ghes_edac_unregister(struct
>> ghes *ghes)
>> {
>> }
>> #endif
>> +
>> +#define acpi_hest_generic_data_version(gdata) \
>> + (gdata->revision >> 8)
>> +
>
>
>>
>> +#define acpi_hest_generic_data_error_length(gdata) \
>> + (((struct acpi_hest_generic_data *)(gdata))->error_data_length)
>> +#define acpi_hest_generic_data_size(gdata) \
>> + ((acpi_hest_generic_data_version(gdata) >= 3) ? \
>> + sizeof(struct acpi_hest_generic_data_v300) : \
>> + sizeof(struct acpi_hest_generic_data))
>> +#define acpi_hest_generic_data_record_size(gdata) \
>> + (acpi_hest_generic_data_size(gdata) + \
>> + acpi_hest_generic_data_error_length(gdata))
>> +#define acpi_hest_generic_data_next(gdata) \
>> + ((void *)(gdata) + acpi_hest_generic_data_record_size(gdata))
>> +
>
> Rest looks good
>
> Cheers
> Suzuki
Thanks,
Tyler
--
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] arm64: SMMU-v2: Workaround for Cavium ThunderX erratum 28168
From: Thomas Gleixner @ 2016-10-24 20:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <fbc51a32-aaba-0ee6-c0bd-07a02fb2a6b4@arm.com>
On Mon, 24 Oct 2016, Marc Zyngier wrote:
> On 22/10/16 05:54, Geetha sowjanya wrote:
> > diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> > index be3c34e..6add8da 100644
> > --- a/kernel/irq/chip.c
> > +++ b/kernel/irq/chip.c
> > @@ -585,6 +585,10 @@ void handle_fasteoi_irq(struct irq_desc *desc)
> > goto out;
> > }
> >
> > +#ifdef CONFIG_CAVIUM_ERRATUM_28168
> > + if (chip->irq_ack)
> > + chip->irq_ack(&desc->irq_data);
> > +#endif
> > kstat_incr_irqs_this_cpu(desc);
> > if (desc->istate & IRQS_ONESHOT)
> > mask_irq(desc);
> >
>
> Overall, this workaround is not acceptable as it is.
Aside of being not acceptable this thing is completely broken.
If that erratum is enabled then a interrupt chip which implements both EOI
and ACK callbacks will issue irq_ack when using the fasteoi handler. While
this might work on that cavium trainwreck, it will just make other
platforms pretty unhappy.
Platform specific hacks have no place in the core code at all. We have
enough options to handle oddball hardware, you just have to use them.
Thanks,
tglx
^ permalink raw reply
* [PATCH] drivers: mfd: ti_am335x_tscadc: increase ADC ref clock to 24MHz
From: John Syne @ 2016-10-24 20:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161024060226.4170-1-mugunthanvnm@ti.com>
> On Oct 23, 2016, at 11:02 PM, Mugunthan V N <mugunthanvnm@ti.com> wrote:
>
> Increase ADC reference clock from 3MHz to 24MHz so that the
> sampling rates goes up from 100K samples per second to 800K
> samples per second on AM335x and AM437x SoC.
>
> Also increase opendelay for touchscreen configuration to
> equalize the increase in ADC reference clock frequency,
> which results in the same amount touch events reported via
> evtest on AM335x GP EVM.
>
> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
> ---
>
> This patch depends on ADC DMA patch series [1]
>
> Without DMA support, when ADC ref clock is set at 24MHz, I am
> seeing fifo overflow as CPU is not able to pull the ADC samples.
> This answers that DMA support is must for ADC to consume the
> samples generated at 24MHz with no open, step delay or
> averaging with patch [2].
>
> Measured the performance with the iio_generic_buffer with the
> patch [3] applied
>
> [1] - http://www.spinics.net/lists/devicetree/msg145045.html
> [2] - http://pastebin.ubuntu.com/23357935/
> [3] - http://pastebin.ubuntu.com/23357939/
>
> ---
> include/linux/mfd/ti_am335x_tscadc.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
> index b9a53e0..96c4207 100644
> --- a/include/linux/mfd/ti_am335x_tscadc.h
> +++ b/include/linux/mfd/ti_am335x_tscadc.h
> @@ -90,7 +90,7 @@
> /* Delay register */
> #define STEPDELAY_OPEN_MASK (0x3FFFF << 0)
> #define STEPDELAY_OPEN(val) ((val) << 0)
> -#define STEPCONFIG_OPENDLY STEPDELAY_OPEN(0x098)
Wouldn?t this be better to add this to the devicetree?
ti,chan-step-avg = <0x16 0x16 0x16 0x16 0x16 0x16 0x16>;
ti,chan-step-opendelay = <0x500 0x500 0x500 0x500 0x500 0x500 0x500>;
ti,chan-step-sampledelay = <0x0 0x0 0x0 0x0 0x0 0x0 0x0>;
Regards,
John
> +#define STEPCONFIG_OPENDLY STEPDELAY_OPEN(0x500)
> #define STEPDELAY_SAMPLE_MASK (0xFF << 24)
> #define STEPDELAY_SAMPLE(val) ((val) << 24)
> #define STEPCONFIG_SAMPLEDLY STEPDELAY_SAMPLE(0)
> @@ -137,7 +137,7 @@
> #define SEQ_STATUS BIT(5)
> #define CHARGE_STEP 0x11
>
> -#define ADC_CLK 3000000
> +#define ADC_CLK 24000000
> #define TOTAL_STEPS 16
> #define TOTAL_CHANNELS 8
> #define FIFO1_THRESHOLD 19
> --
> 2.10.1.502.g6598894
>
^ permalink raw reply
* [PATCH v3 3/8] PM / Domains: Allow domain power states to be read from DT
From: Lina Iyer @ 2016-10-24 21:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <12435f67-bcb6-33b5-4399-f4f628f70be8@arm.com>
On Mon, Oct 24 2016 at 11:27 -0600, Sudeep Holla wrote:
>
>
>On 24/10/16 17:48, Lina Iyer wrote:
>>Hi Sudeep,
>>
>>On Mon, Oct 24 2016 at 07:39 -0600, Sudeep Holla wrote:
>>>
>>>
>>>On 14/10/16 18:47, Lina Iyer wrote:
>>>>This patch allows domains to define idle states in the DT. SoC's can
>>>>define domain idle states in DT using the "domain-idle-states" property
>>>>of the domain provider. Add API to read the idle states from DT that can
>>>>be set in the genpd object.
>>>>
>>>>This patch is based on the original patch by Marc Titinger.
>>>>
>>>>Signed-off-by: Marc Titinger <mtitinger+renesas@baylibre.com>
>>>>Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>>>>Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
>>>>---
>>>>drivers/base/power/domain.c | 94
>>>>+++++++++++++++++++++++++++++++++++++++++++++
>>>>include/linux/pm_domain.h | 8 ++++
>>>>2 files changed, 102 insertions(+)
>>>>
>>>>diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
>>>>index 37ab7f1..9af75ba 100644
>>>>--- a/drivers/base/power/domain.c
>>>>+++ b/drivers/base/power/domain.c
>>>>@@ -1916,6 +1916,100 @@ out:
>>>> return ret ? -EPROBE_DEFER : 0;
>>>>}
>>>>EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
>>>>+
>>>>+static const struct of_device_id idle_state_match[] = {
>>>>+ { .compatible = "arm,idle-state", },
>>>>+ { }
>>>>+};
>>>>+
>>>
>>>I still think it's better to have another compatible to serve this
>>>purpose. We don't want to end up creating genpd domains just because
>>>they are "arm,idle-state" compatible IMO ?
>>>
>>>I agree you can prevent it checking for OSC mode support in the
>>>firmware. But I want to understand if you have any strong reasons for
>>>avoiding that approach.
>>>
>>Why are you still held up with OSI/PC PSCI modes?
>
>I am just pointing that out to make sure you are not defining these
>DT bindings with just QCOM platform and OSC in consideration.
>
>I am thinking about how can it be used/extended in other use-cases.
>
>>I repeat again this
>>series is not about any of that, it is just about PM domains.
>
>I completely understand that, no argument on that. What I worry is that
>if a system(in future) represents this power domains and domain idles
>states and doesn't want to create a genpd, do we have to handle it
>differently or even the way your CPUidle series would do or can we say
>those states need to specify that they are compatible with the new
>feature(the one being added in this series) with a new compatible.
>I prefer the latter than the former to avoid all possible workarounds
>in future.
>
>>PM domains
>>have idle states and the idle-state description is similar in definition
>>to arm,idle-state and therefore uses the same compatible. There is no
>>point re-defining something that already exists in the kernel.
>>
>
>Yes I understand that but "arm,idle-states" were not defined with this
>feature in mind and hence I am bit worried if it could cause any issue
>especially if deprecate cpu-idle-states and move to this model
>completely. I really don't like this mix and hence I am raising the
>concern here. I am trying to ease that migration.
>
>>I was able to find the original thread, where we discussed this [1].
>>
>>I suggest, you read about PM domains and its idle states and understand
>>this series in the context of PM domains.
>>
>
>Sure, will do that. Thanks for pointing that out. But the concern I am
>raising is entirely different. I am asking if this re-use will cause any
>issue in future as pointed out above.
>
>I re-iterate that I understand this series is independent of the CPUIdle
>and hence asking why not make it completely independent by just adding
>the new compatible.
>
>I am *not asking to redefine something completely*. What I am saying is
>*just* to add new compatible that may(for cpu devices)/may not(for
>other/non-CPU devices) be used along with "arm,idle-state".
>
I am fine with that, as long as the compatible string is an alternate
for "arm,idle-state" it shouldn't be a problem.
Any recommendations?
Thanks,
Lina
>I may be too paranoid here but I think that's safer. It helps to skip
>creating of genpd if required for some domains as I had explained before.
>
>--
>Regards,
>Sudeep
^ permalink raw reply
* [PATCH v3 [fix]] PM / doc: Update device documentation for devices in IRQ safe PM domains
From: Lina Iyer @ 2016-10-24 21:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <3700155.5AlzjeyN5H@vostro.rjw.lan>
On Mon, Oct 24 2016 at 15:15 -0600, Rafael J. Wysocki wrote:
>On Monday, October 24, 2016 10:16:05 AM Lina Iyer wrote:
>> On Sat, Oct 22 2016 at 18:19 -0600, Rafael J. Wysocki wrote:
>> >On Friday, October 21, 2016 03:52:55 PM Lina Iyer wrote:
>> >> Update documentation to reflect the changes made to support IRQ safe PM
>> >> domains.
>> >>
>> >> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
>> >> Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
>> >> ---
>> >> Changes since v3:
>> >> - Moved para to the end of the section
>> >> - Added clause for all IRQ safe devices in a domain
>> >> - Cleanup explanation of nested domains
>> >> ---
>> >> Documentation/power/devices.txt | 11 ++++++++++-
>> >> 1 file changed, 10 insertions(+), 1 deletion(-)
>> >>
>> >> diff --git a/Documentation/power/devices.txt b/Documentation/power/devices.txt
>> >> index 8ba6625..9218ce6 100644
>> >> --- a/Documentation/power/devices.txt
>> >> +++ b/Documentation/power/devices.txt
>> >> @@ -607,7 +607,9 @@ individually. Instead, a set of devices sharing a power resource can be put
>> >> into a low-power state together at the same time by turning off the shared
>> >> power resource. Of course, they also need to be put into the full-power state
>> >> together, by turning the shared power resource on. A set of devices with this
>> >> -property is often referred to as a power domain.
>> >> +property is often referred to as a power domain. A power domain may also be
>> >> +nested inside another power domain. The nested domain is referred to as the
>> >> +sub-domain of the parent domain.
>> >>
>> >> Support for power domains is provided through the pm_domain field of struct
>> >> device. This field is a pointer to an object of type struct dev_pm_domain,
>> >> @@ -629,6 +631,13 @@ support for power domains into subsystem-level callbacks, for example by
>> >> modifying the platform bus type. Other platforms need not implement it or take
>> >> it into account in any way.
>> >>
>> >> +Devices and PM domains may be defined as IRQ-safe, if they can be powered
>> >> +on/off even when the IRQs are disabled.
>> >
>> >What IRQ-safe means for devices is that their runtime PM callbacks may be
>> >invoked with interrupts disabled on the local CPU. I guess the meaning of
>> >IRQ-safe for PM domains is analogous, but the above isn't precise enough to me.
>> >
>> >> An IRQ-safe device in a domain will
>> >> +disallow power management on the domain, unless the domain is also defined as
>> >> +IRQ-safe. In other words, a domain containing all IRQ-safe devices must also
>> >> +be defined as IRQ-safe. Another restriction this framework imposes on the
>> >> +parent domain of an IRQ-safe domain is that the parent domain must also be
>> >> +defined as IRQ-safe.
>> >
>> >What about this:
>> >
>> >"Devices may be defined as IRQ-safe which indicates to the PM core that their
>> >runtime PM callbacks may be invoked with disabled interrupts (see
>> >Documentation/power/runtime_pm.txt for more information). If an IRQ-safe
>> >device belongs to a PM domain, the runtime PM of the domain will be disallowed,
>> >unless the domain itself is defined as IRQ-safe. However, a PM domain can only
>> >be defined as IRQ-safe if all of the devices in it are IRQ-safe.
>> >
>> This is correct. But the last line may need a bit of modification. If
>> all devices in a PM domain are IRQ-safe and the domain is NOT, then it
>> it is a valid combination just that the domain would never do runtime
>> PM.
>
>That doesn't contradict the last sentence of mine above. I guess what you mean
>is that having a non-IRQ-safe device in an IRQ-safe domain is a valid
>configuration. I wonder how it works then. :-)
>
>In any case, what about changing that sentence to something like:
>
>"However, it only makes sense to define a PM domain as IRQ-safe if all devices
>in it are IRQ-safe."
>
That's precise. I will add your para instead of mine to the
documentation.
Thanks,
Lina
>
^ permalink raw reply
* [PATCH 5/5] ARM: dts: Add LEGO MINDSTORTMS EV3 dts
From: David Lechner @ 2016-10-24 21:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <b6cd3c47-0df8-266a-2b8d-13c82062b647@lechnology.com>
On 10/24/2016 02:50 PM, David Lechner wrote:
> On 10/24/2016 10:50 AM, David Lechner wrote:
>> On 10/24/2016 06:58 AM, Sekhar Nori wrote:
>>> On Saturday 22 October 2016 12:06 AM, David Lechner wrote:
>>>
>>>> +&ehrpwm1 {
>>>> + status = "disabled";
>>>
>>> Hmm, disabled? Can you add this node when you actually use it?
>>
>> Not sure why I have this disabled. Like the gpios, the pwms can be used
>> via sysfs, so I would like to leave them.
>>
>
> Now I remember why these are disabled. The clock matching is broken.
> Only the first ehrpwm and the first ecap get clocks. The others fail.
>
> I can change these to "okay". It will just result in a kernel error
> message until the clocks are fixed.
>
correction: it is not the clocks that are broken. it is the device names.
In arch/arm/mach-davinci/da8xx-dt.c, we have...
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),
Which causes each device to have the same device node name. This causes
sysfs errors because it is trying to register a second device at the
same sysfs path.
If you change the names here, then the device do not work because the
clock lookup fails.
^ permalink raw reply
* [PATCH v3 [fix]] PM / doc: Update device documentation for devices in IRQ safe PM domains
From: Rafael J. Wysocki @ 2016-10-24 21:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161024161605.GD72940@linaro.org>
On Monday, October 24, 2016 10:16:05 AM Lina Iyer wrote:
> On Sat, Oct 22 2016 at 18:19 -0600, Rafael J. Wysocki wrote:
> >On Friday, October 21, 2016 03:52:55 PM Lina Iyer wrote:
> >> Update documentation to reflect the changes made to support IRQ safe PM
> >> domains.
> >>
> >> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
> >> Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
> >> ---
> >> Changes since v3:
> >> - Moved para to the end of the section
> >> - Added clause for all IRQ safe devices in a domain
> >> - Cleanup explanation of nested domains
> >> ---
> >> Documentation/power/devices.txt | 11 ++++++++++-
> >> 1 file changed, 10 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/Documentation/power/devices.txt b/Documentation/power/devices.txt
> >> index 8ba6625..9218ce6 100644
> >> --- a/Documentation/power/devices.txt
> >> +++ b/Documentation/power/devices.txt
> >> @@ -607,7 +607,9 @@ individually. Instead, a set of devices sharing a power resource can be put
> >> into a low-power state together at the same time by turning off the shared
> >> power resource. Of course, they also need to be put into the full-power state
> >> together, by turning the shared power resource on. A set of devices with this
> >> -property is often referred to as a power domain.
> >> +property is often referred to as a power domain. A power domain may also be
> >> +nested inside another power domain. The nested domain is referred to as the
> >> +sub-domain of the parent domain.
> >>
> >> Support for power domains is provided through the pm_domain field of struct
> >> device. This field is a pointer to an object of type struct dev_pm_domain,
> >> @@ -629,6 +631,13 @@ support for power domains into subsystem-level callbacks, for example by
> >> modifying the platform bus type. Other platforms need not implement it or take
> >> it into account in any way.
> >>
> >> +Devices and PM domains may be defined as IRQ-safe, if they can be powered
> >> +on/off even when the IRQs are disabled.
> >
> >What IRQ-safe means for devices is that their runtime PM callbacks may be
> >invoked with interrupts disabled on the local CPU. I guess the meaning of
> >IRQ-safe for PM domains is analogous, but the above isn't precise enough to me.
> >
> >> An IRQ-safe device in a domain will
> >> +disallow power management on the domain, unless the domain is also defined as
> >> +IRQ-safe. In other words, a domain containing all IRQ-safe devices must also
> >> +be defined as IRQ-safe. Another restriction this framework imposes on the
> >> +parent domain of an IRQ-safe domain is that the parent domain must also be
> >> +defined as IRQ-safe.
> >
> >What about this:
> >
> >"Devices may be defined as IRQ-safe which indicates to the PM core that their
> >runtime PM callbacks may be invoked with disabled interrupts (see
> >Documentation/power/runtime_pm.txt for more information). If an IRQ-safe
> >device belongs to a PM domain, the runtime PM of the domain will be disallowed,
> >unless the domain itself is defined as IRQ-safe. However, a PM domain can only
> >be defined as IRQ-safe if all of the devices in it are IRQ-safe.
> >
> This is correct. But the last line may need a bit of modification. If
> all devices in a PM domain are IRQ-safe and the domain is NOT, then it
> it is a valid combination just that the domain would never do runtime
> PM.
That doesn't contradict the last sentence of mine above. I guess what you mean
is that having a non-IRQ-safe device in an IRQ-safe domain is a valid
configuration. I wonder how it works then. :-)
In any case, what about changing that sentence to something like:
"However, it only makes sense to define a PM domain as IRQ-safe if all devices
in it are IRQ-safe."
Thanks,
Rafael
^ 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