* [PATCH v4 3/8] MIPS: Octeon: Add a global resource manager.
From: David Daney @ 2017-11-29 0:55 UTC (permalink / raw)
To: linux-mips, ralf, James Hogan, netdev, David S. Miller,
Rob Herring, Mark Rutland, devel, Greg Kroah-Hartman
Cc: linux-kernel, Steven J. Hill, devicetree, Andrew Lunn,
Florian Fainelli, Carlos Munoz, Steven J . Hill, David Daney
In-Reply-To: <20171129005540.28829-1-david.daney@cavium.com>
From: Carlos Munoz <cmunoz@cavium.com>
Add a global resource manager to manage tagged pointers within
bootmem allocated memory. This is used by various functional
blocks in the Octeon core like the FPA, Ethernet nexus, etc.
Signed-off-by: Carlos Munoz <cmunoz@cavium.com>
Signed-off-by: Steven J. Hill <Steven.Hill@cavium.com>
Signed-off-by: David Daney <david.daney@cavium.com>
---
arch/mips/cavium-octeon/Makefile | 3 +-
arch/mips/cavium-octeon/resource-mgr.c | 371 +++++++++++++++++++++++++++++++++
arch/mips/include/asm/octeon/octeon.h | 18 ++
3 files changed, 391 insertions(+), 1 deletion(-)
create mode 100644 arch/mips/cavium-octeon/resource-mgr.c
diff --git a/arch/mips/cavium-octeon/Makefile b/arch/mips/cavium-octeon/Makefile
index 7c02e542959a..0a299ab8719f 100644
--- a/arch/mips/cavium-octeon/Makefile
+++ b/arch/mips/cavium-octeon/Makefile
@@ -9,7 +9,8 @@
# Copyright (C) 2005-2009 Cavium Networks
#
-obj-y := cpu.o setup.o octeon-platform.o octeon-irq.o csrc-octeon.o
+obj-y := cpu.o setup.o octeon-platform.o octeon-irq.o csrc-octeon.o \
+ resource-mgr.o
obj-y += dma-octeon.o
obj-y += octeon-memcpy.o
obj-y += executive/
diff --git a/arch/mips/cavium-octeon/resource-mgr.c b/arch/mips/cavium-octeon/resource-mgr.c
new file mode 100644
index 000000000000..ca25fa953402
--- /dev/null
+++ b/arch/mips/cavium-octeon/resource-mgr.c
@@ -0,0 +1,371 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Resource manager for Octeon.
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License. See the file "COPYING" in the main directory of this archive
+ * for more details.
+ *
+ * Copyright (C) 2017 Cavium, Inc.
+ */
+#include <linux/module.h>
+
+#include <asm/octeon/octeon.h>
+#include <asm/octeon/cvmx-bootmem.h>
+
+#define RESOURCE_MGR_BLOCK_NAME "cvmx-global-resources"
+#define MAX_RESOURCES 128
+#define INST_AVAILABLE -88
+#define OWNER 0xbadc0de
+
+struct global_resource_entry {
+ struct global_resource_tag tag;
+ u64 phys_addr;
+ u64 size;
+};
+
+struct global_resources {
+#ifdef __LITTLE_ENDIAN_BITFIELD
+ u32 rlock;
+ u32 pad;
+#else
+ u32 pad;
+ u32 rlock;
+#endif
+ u64 entry_cnt;
+ struct global_resource_entry resource_entry[];
+};
+
+static struct global_resources *res_mgr_info;
+
+
+/*
+ * The resource manager interacts with software running outside of the
+ * Linux kernel, which necessitates locking to maintain data structure
+ * consistency. These custom locking functions implement the locking
+ * protocol, and cannot be replaced by kernel locking functions that
+ * may use different in-memory structures.
+ */
+
+static void res_mgr_lock(void)
+{
+ unsigned int tmp;
+ u64 lock = (u64)&res_mgr_info->rlock;
+
+ __asm__ __volatile__(
+ ".set noreorder\n"
+ "1: ll %[tmp], 0(%[addr])\n"
+ " bnez %[tmp], 1b\n"
+ " li %[tmp], 1\n"
+ " sc %[tmp], 0(%[addr])\n"
+ " beqz %[tmp], 1b\n"
+ " nop\n"
+ ".set reorder\n" :
+ [tmp] "=&r"(tmp) :
+ [addr] "r"(lock) :
+ "memory");
+}
+
+static void res_mgr_unlock(void)
+{
+ u64 lock = (u64)&res_mgr_info->rlock;
+
+ /* Wait until all resource operations finish before unlocking. */
+ mb();
+ __asm__ __volatile__(
+ "sw $0, 0(%[addr])\n" : :
+ [addr] "r"(lock) :
+ "memory");
+
+ /* Force a write buffer flush. */
+ mb();
+}
+
+static int res_mgr_find_resource(struct global_resource_tag tag)
+{
+ struct global_resource_entry *res_entry;
+ int i;
+
+ for (i = 0; i < res_mgr_info->entry_cnt; i++) {
+ res_entry = &res_mgr_info->resource_entry[i];
+ if (res_entry->tag.lo == tag.lo && res_entry->tag.hi == tag.hi)
+ return i;
+ }
+ return -1;
+}
+
+/**
+ * res_mgr_create_resource - Create a resource.
+ * @tag: Identifies the resource.
+ * @inst_cnt: Number of resource instances to create.
+ *
+ * Returns 0 if the source was created successfully.
+ * Returns <0 for error codes.
+ */
+int res_mgr_create_resource(struct global_resource_tag tag, int inst_cnt)
+{
+ struct global_resource_entry *res_entry;
+ u64 size;
+ u64 *res_addr;
+ int res_index, i, rc = 0;
+
+ res_mgr_lock();
+
+ /* Make sure resource doesn't already exist. */
+ res_index = res_mgr_find_resource(tag);
+ if (res_index >= 0) {
+ rc = -1;
+ goto err;
+ }
+
+ if (res_mgr_info->entry_cnt >= MAX_RESOURCES) {
+ pr_err("Resource max limit reached, not created\n");
+ rc = -1;
+ goto err;
+ }
+
+ /*
+ * Each instance is kept in an array of u64s. The first array element
+ * holds the number of allocated instances.
+ */
+ size = sizeof(u64) * (inst_cnt + 1);
+ res_addr = cvmx_bootmem_alloc_range(size, CVMX_CACHE_LINE_SIZE, 0, 0);
+ if (!res_addr) {
+ pr_err("Failed to allocate resource. not created\n");
+ rc = -1;
+ goto err;
+ }
+
+ /* Initialize the newly created resource. */
+ *res_addr = inst_cnt;
+ for (i = 1; i < inst_cnt + 1; i++)
+ *(res_addr + i) = INST_AVAILABLE;
+
+ res_index = res_mgr_info->entry_cnt;
+ res_entry = &res_mgr_info->resource_entry[res_index];
+ res_entry->tag.lo = tag.lo;
+ res_entry->tag.hi = tag.hi;
+ res_entry->phys_addr = virt_to_phys(res_addr);
+ res_entry->size = size;
+ res_mgr_info->entry_cnt++;
+
+err:
+ res_mgr_unlock();
+
+ return rc;
+}
+EXPORT_SYMBOL(res_mgr_create_resource);
+
+/**
+ * res_mgr_alloc_range - Allocate a range of resource instances.
+ * @tag: Identifies the resource.
+ * @req_inst: Requested start of instance range to allocate.
+ * Range instances are guaranteed to be sequential
+ * (-1 for don't care).
+ * @req_cnt: Number of instances to allocate.
+ * @use_last_avail: Set to request the last available instance.
+ * @inst: Updated with the allocated instances.
+ *
+ * Returns 0 if the source was created successfully.
+ * Returns <0 for error codes.
+ */
+int res_mgr_alloc_range(struct global_resource_tag tag, int req_inst,
+ int req_cnt, bool use_last_avail, int *inst)
+{
+ struct global_resource_entry *res_entry;
+ int res_index;
+ u64 *res_addr;
+ u64 inst_cnt;
+ int alloc_cnt, i, rc = -1;
+
+ /* Start with no instances allocated. */
+ for (i = 0; i < req_cnt; i++)
+ inst[i] = INST_AVAILABLE;
+
+ res_mgr_lock();
+
+ /* Find the resource. */
+ res_index = res_mgr_find_resource(tag);
+ if (res_index < 0) {
+ pr_err("Resource not found, can't allocate instance\n");
+ goto err;
+ }
+
+ /* Get resource data. */
+ res_entry = &res_mgr_info->resource_entry[res_index];
+ res_addr = phys_to_virt(res_entry->phys_addr);
+ inst_cnt = *res_addr;
+
+ /* Allocate the requested instances. */
+ if (req_inst >= 0) {
+ /* Specific instance range requested. */
+ if (req_inst + req_cnt >= inst_cnt) {
+ pr_err("Requested instance out of range\n");
+ goto err;
+ }
+
+ for (i = 0; i < req_cnt; i++) {
+ if (*(res_addr + req_inst + 1 + i) == INST_AVAILABLE)
+ inst[i] = req_inst + i;
+ else {
+ inst[0] = INST_AVAILABLE;
+ break;
+ }
+ }
+ } else if (use_last_avail) {
+ /* Last available instance requested. */
+ alloc_cnt = 0;
+ for (i = inst_cnt; i > 0; i--) {
+ if (*(res_addr + i) == INST_AVAILABLE) {
+ /*
+ * Instance off by 1 (first element holds the
+ * count).
+ */
+ inst[alloc_cnt] = i - 1;
+
+ alloc_cnt++;
+ if (alloc_cnt == req_cnt)
+ break;
+ }
+ }
+
+ if (i == 0)
+ inst[0] = INST_AVAILABLE;
+ } else {
+ /* Next available instance requested. */
+ alloc_cnt = 0;
+ for (i = 1; i <= inst_cnt; i++) {
+ if (*(res_addr + i) == INST_AVAILABLE) {
+ /*
+ * Instance off by 1 (first element holds the
+ * count).
+ */
+ inst[alloc_cnt] = i - 1;
+
+ alloc_cnt++;
+ if (alloc_cnt == req_cnt)
+ break;
+ }
+ }
+
+ if (i > inst_cnt)
+ inst[0] = INST_AVAILABLE;
+ }
+
+ if (inst[0] != INST_AVAILABLE) {
+ for (i = 0; i < req_cnt; i++)
+ *(res_addr + inst[i] + 1) = OWNER;
+ rc = 0;
+ }
+
+err:
+ res_mgr_unlock();
+
+ return rc;
+}
+EXPORT_SYMBOL(res_mgr_alloc_range);
+
+/**
+ * res_mgr_alloc - Allocate a resource instance.
+ * @tag: Identifies the resource.
+ * @req_inst: Requested instance to allocate (-1 for don't care).
+ * @use_last_avail: Set to request the last available instance.
+ *
+ * Returns: Allocated resource instance if successful.
+ * Returns <0 for error codes.
+ */
+int res_mgr_alloc(struct global_resource_tag tag, int req_inst, bool use_last_avail)
+{
+ int inst, rc;
+
+ rc = res_mgr_alloc_range(tag, req_inst, 1, use_last_avail, &inst);
+ if (!rc)
+ return inst;
+ return rc;
+}
+EXPORT_SYMBOL(res_mgr_alloc);
+
+/**
+ * res_mgr_free_range - Free a resource instance range.
+ * @tag: Identifies the resource.
+ * @req_inst: Requested instance to free.
+ * @req_cnt: Number of instances to free.
+ */
+void res_mgr_free_range(struct global_resource_tag tag, const int *inst, int req_cnt)
+{
+ struct global_resource_entry *res_entry;
+ int res_index, i;
+ u64 *res_addr;
+
+ res_mgr_lock();
+
+ /* Find the resource. */
+ res_index = res_mgr_find_resource(tag);
+ if (res_index < 0) {
+ pr_err("Resource not found, can't free instance\n");
+ goto err;
+ }
+
+ /* Get the resource data. */
+ res_entry = &res_mgr_info->resource_entry[res_index];
+ res_addr = phys_to_virt(res_entry->phys_addr);
+
+ /* Free the resource instances. */
+ for (i = 0; i < req_cnt; i++) {
+ /* Instance off by 1 (first element holds the count). */
+ *(res_addr + inst[i] + 1) = INST_AVAILABLE;
+ }
+
+err:
+ res_mgr_unlock();
+}
+EXPORT_SYMBOL(res_mgr_free_range);
+
+/**
+ * res_mgr_free - Free a resource instance.
+ * @tag: Identifies the resource.
+ * @req_inst: Requested instance to free.
+ */
+void res_mgr_free(struct global_resource_tag tag, int inst)
+{
+ res_mgr_free_range(tag, &inst, 1);
+}
+EXPORT_SYMBOL(res_mgr_free);
+
+static int __init res_mgr_init(void)
+{
+ struct cvmx_bootmem_named_block_desc *block;
+ int block_size;
+ u64 addr;
+
+ cvmx_bootmem_lock();
+
+ /* Search for the resource manager data in boot memory. */
+ block = cvmx_bootmem_phy_named_block_find(RESOURCE_MGR_BLOCK_NAME, CVMX_BOOTMEM_FLAG_NO_LOCKING);
+ if (block) {
+ /* Found. */
+ res_mgr_info = phys_to_virt(block->base_addr);
+ } else {
+ /* Create it. */
+ block_size = sizeof(struct global_resources) +
+ sizeof(struct global_resource_entry) * MAX_RESOURCES;
+ addr = cvmx_bootmem_phy_named_block_alloc(block_size, 0, 0,
+ CVMX_CACHE_LINE_SIZE, RESOURCE_MGR_BLOCK_NAME,
+ CVMX_BOOTMEM_FLAG_NO_LOCKING);
+ if (!addr) {
+ pr_err("Failed to allocate name block %s\n",
+ RESOURCE_MGR_BLOCK_NAME);
+ } else {
+ res_mgr_info = phys_to_virt(addr);
+ memset(res_mgr_info, 0, block_size);
+ }
+ }
+
+ cvmx_bootmem_unlock();
+
+ return 0;
+}
+device_initcall(res_mgr_init);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Cavium, Inc. Octeon resource manager");
diff --git a/arch/mips/include/asm/octeon/octeon.h b/arch/mips/include/asm/octeon/octeon.h
index 92a17d67c1fa..0411efdb465c 100644
--- a/arch/mips/include/asm/octeon/octeon.h
+++ b/arch/mips/include/asm/octeon/octeon.h
@@ -346,6 +346,24 @@ void octeon_mult_restore3_end(void);
void octeon_mult_restore2(void);
void octeon_mult_restore2_end(void);
+/*
+ * This definition must be kept in sync with the one in
+ * cvmx-global-resources.c
+ */
+struct global_resource_tag {
+ uint64_t lo;
+ uint64_t hi;
+};
+
+void res_mgr_free(struct global_resource_tag tag, int inst);
+void res_mgr_free_range(struct global_resource_tag tag, const int *inst,
+ int req_cnt);
+int res_mgr_alloc(struct global_resource_tag tag, int req_inst,
+ bool use_last_avail);
+int res_mgr_alloc_range(struct global_resource_tag tag, int req_inst,
+ int req_cnt, bool use_last_avail, int *inst);
+int res_mgr_create_resource(struct global_resource_tag tag, int inst_cnt);
+
/**
* Read a 32bit value from the Octeon NPI register space
*
--
2.14.3
^ permalink raw reply related
* [PATCH v4 2/8] MIPS: Octeon: Enable LMTDMA/LMTST operations.
From: David Daney @ 2017-11-29 0:55 UTC (permalink / raw)
To: linux-mips, ralf, James Hogan, netdev, David S. Miller,
Rob Herring, Mark Rutland, devel, Greg Kroah-Hartman
Cc: devicetree, Florian Fainelli, Andrew Lunn, David Daney,
linux-kernel, Carlos Munoz, Steven J . Hill
In-Reply-To: <20171129005540.28829-1-david.daney@cavium.com>
From: Carlos Munoz <cmunoz@cavium.com>
LMTDMA/LMTST operations move data between cores and I/O devices:
* LMTST operations can send an address and a variable length
(up to 128 bytes) of data to an I/O device.
* LMTDMA operations can send an address and a variable length
(up to 128) of data to the I/O device and then return a
variable length (up to 128 bytes) response from the IOI device.
Signed-off-by: Carlos Munoz <cmunoz@cavium.com>
Signed-off-by: Steven J. Hill <Steven.Hill@cavium.com>
Signed-off-by: David Daney <david.daney@cavium.com>
---
arch/mips/cavium-octeon/setup.c | 6 ++++++
arch/mips/include/asm/octeon/octeon.h | 12 ++++++++++--
2 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/arch/mips/cavium-octeon/setup.c b/arch/mips/cavium-octeon/setup.c
index a8034d0dcade..99e6a68bc652 100644
--- a/arch/mips/cavium-octeon/setup.c
+++ b/arch/mips/cavium-octeon/setup.c
@@ -609,6 +609,12 @@ void octeon_user_io_init(void)
#else
cvmmemctl.s.cvmsegenak = 0;
#endif
+ if (OCTEON_IS_OCTEON3()) {
+ /* Enable LMTDMA */
+ cvmmemctl.s.lmtena = 1;
+ /* Scratch line to use for LMT operation */
+ cvmmemctl.s.lmtline = 2;
+ }
/* R/W If set, CVMSEG is available for loads/stores in
* supervisor mode. */
cvmmemctl.s.cvmsegenas = 0;
diff --git a/arch/mips/include/asm/octeon/octeon.h b/arch/mips/include/asm/octeon/octeon.h
index c99c4b6a79f4..92a17d67c1fa 100644
--- a/arch/mips/include/asm/octeon/octeon.h
+++ b/arch/mips/include/asm/octeon/octeon.h
@@ -179,7 +179,15 @@ union octeon_cvmemctl {
/* RO 1 = BIST fail, 0 = BIST pass */
__BITFIELD_FIELD(uint64_t wbfbist:1,
/* Reserved */
- __BITFIELD_FIELD(uint64_t reserved:17,
+ __BITFIELD_FIELD(uint64_t reserved_52_57:6,
+ /* When set, LMTDMA/LMTST operations are permitted */
+ __BITFIELD_FIELD(uint64_t lmtena:1,
+ /* Selects the CVMSEG LM cacheline used by LMTDMA
+ * LMTST and wide atomic store operations.
+ */
+ __BITFIELD_FIELD(uint64_t lmtline:6,
+ /* Reserved */
+ __BITFIELD_FIELD(uint64_t reserved_41_44:4,
/* OCTEON II - TLB replacement policy: 0 = bitmask LRU; 1 = NLU.
* This field selects between the TLB replacement policies:
* bitmask LRU or NLU. Bitmask LRU maintains a mask of
@@ -275,7 +283,7 @@ union octeon_cvmemctl {
/* R/W Size of local memory in cache blocks, 54 (6912
* bytes) is max legal value. */
__BITFIELD_FIELD(uint64_t lmemsz:6,
- ;)))))))))))))))))))))))))))))))))
+ ;))))))))))))))))))))))))))))))))))))
} s;
};
--
2.14.3
^ permalink raw reply related
* [PATCH v4 1/8] dt-bindings: Add Cavium Octeon Common Ethernet Interface.
From: David Daney @ 2017-11-29 0:55 UTC (permalink / raw)
To: linux-mips, ralf, James Hogan, netdev, David S. Miller,
Rob Herring, Mark Rutland, devel, Greg Kroah-Hartman
Cc: devicetree, Florian Fainelli, Andrew Lunn, David Daney,
linux-kernel, Carlos Munoz, Steven J . Hill
In-Reply-To: <20171129005540.28829-1-david.daney@cavium.com>
From: Carlos Munoz <cmunoz@cavium.com>
Add bindings for Common Ethernet Interface (BGX) block.
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Carlos Munoz <cmunoz@cavium.com>
Signed-off-by: Steven J. Hill <Steven.Hill@cavium.com>
Signed-off-by: David Daney <david.daney@cavium.com>
---
.../devicetree/bindings/net/cavium-bgx.txt | 61 ++++++++++++++++++++++
1 file changed, 61 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/cavium-bgx.txt
diff --git a/Documentation/devicetree/bindings/net/cavium-bgx.txt b/Documentation/devicetree/bindings/net/cavium-bgx.txt
new file mode 100644
index 000000000000..830c5f08dddd
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/cavium-bgx.txt
@@ -0,0 +1,61 @@
+* Common Ethernet Interface (BGX) block
+
+Properties:
+
+- compatible: "cavium,octeon-7890-bgx": Compatibility with all cn7xxx SOCs.
+
+- reg: The base address of the BGX block.
+
+- #address-cells: Must be <1>.
+
+- #size-cells: Must be <0>. BGX addresses have no size component.
+
+A BGX block has several children, each representing an Ethernet
+interface.
+
+
+* Ethernet Interface (BGX port) connects to PKI/PKO
+
+Properties:
+
+- compatible: "cavium,octeon-7890-bgx-port": Compatibility with all
+ cn7xxx SOCs.
+
+ "cavium,octeon-7360-xcv": Compatibility with cn73xx SOCs
+ for RGMII.
+
+- reg: The index of the interface within the BGX block.
+
+Optional properties:
+
+- local-mac-address: Mac address for the interface.
+
+- phy-handle: phandle to the phy node connected to the interface.
+
+- phy-mode: described in ethernet.txt.
+
+- fixed-link: described in fixed-link.txt.
+
+Example:
+
+ ethernet-mac-nexus@11800e0000000 {
+ compatible = "cavium,octeon-7890-bgx";
+ reg = <0x00011800 0xe0000000 0x00000000 0x01000000>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ ethernet@0 {
+ compatible = "cavium,octeon-7360-xcv";
+ reg = <0>;
+ local-mac-address = [ 00 01 23 45 67 89 ];
+ phy-handle = <&phy3>;
+ phy-mode = "rgmii-rxid"
+ };
+ ethernet@1 {
+ compatible = "cavium,octeon-7890-bgx-port";
+ reg = <1>;
+ local-mac-address = [ 00 01 23 45 67 8a ];
+ phy-handle = <&phy4>;
+ phy-mode = "sgmii"
+ };
+ };
--
2.14.3
^ permalink raw reply related
* [PATCH v4 0/8] Cavium OCTEON-III network driver.
From: David Daney @ 2017-11-29 0:55 UTC (permalink / raw)
To: linux-mips-6z/3iImG2C8G8FEW9MqTrA, ralf-6z/3iImG2C8G8FEW9MqTrA,
James Hogan, netdev-u79uwXL29TY76Z2rM5mHXA, David S. Miller,
Rob Herring, Mark Rutland, devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b,
Greg Kroah-Hartman
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Steven J. Hill,
devicetree-u79uwXL29TY76Z2rM5mHXA, Andrew Lunn, Florian Fainelli,
David Daney
We are adding the Cavium OCTEON-III network driver. But since
interacting with the input and output queues is done via special CPU
local memory, we also need to add support to the MIPS/Octeon
architecture code. Aren't SoCs nice in this way?
The first six patches add the SoC support needed by the driver, the
last two add the driver and an entry in MAINTAINERS.
Since these touch several subsystems (mips, staging, netdev), I would
propose merging via netdev, but defer to the maintainers if they think
something else would work better.
A separate pull request was recently done by Steven Hill for the
firmware required by the driver.
Changes from v3:
o Use phy_print_status() instead of open coding the equivalent.
o Print warning on phy mode mismatch.
o Improve dt-bindings and add Acked-by.
Changes from v2:
o Fix PKI (RX path) initialization to work with little endian kernel.
Changes from v1:
o Cleanup and use of standard bindings in the device tree bindings
document.
o Added (hopefully) clarifying comments about several OCTEON
architectural peculiarities.
o Removed unused testing code from the driver.
o Removed some module parameters that already default to the proper
values.
o KConfig cleanup, including testing on x86_64, arm64 and mips.
o Fixed breakage to the driver for previous generation of OCTEON SoCs (in
the staging directory still).
o Verified bisectability of the patch set.
Carlos Munoz (5):
dt-bindings: Add Cavium Octeon Common Ethernet Interface.
MIPS: Octeon: Enable LMTDMA/LMTST operations.
MIPS: Octeon: Add a global resource manager.
MIPS: Octeon: Add Free Pointer Unit (FPA) support.
netdev: octeon-ethernet: Add Cavium Octeon III support.
David Daney (3):
MIPS: Octeon: Automatically provision CVMSEG space.
staging: octeon: Remove USE_ASYNC_IOBDMA macro.
MAINTAINERS: Add entry for
drivers/net/ethernet/cavium/octeon/octeon3-*
.../devicetree/bindings/net/cavium-bgx.txt | 61 +
MAINTAINERS | 6 +
arch/mips/cavium-octeon/Kconfig | 35 +-
arch/mips/cavium-octeon/Makefile | 4 +-
arch/mips/cavium-octeon/octeon-fpa3.c | 364 ++++
arch/mips/cavium-octeon/resource-mgr.c | 371 ++++
arch/mips/cavium-octeon/setup.c | 22 +-
.../asm/mach-cavium-octeon/kernel-entry-init.h | 20 +-
arch/mips/include/asm/mipsregs.h | 2 +
arch/mips/include/asm/octeon/octeon.h | 47 +-
arch/mips/include/asm/processor.h | 2 +-
arch/mips/kernel/octeon_switch.S | 2 -
arch/mips/mm/tlbex.c | 29 +-
drivers/net/ethernet/cavium/Kconfig | 55 +-
drivers/net/ethernet/cavium/octeon/Makefile | 6 +
.../net/ethernet/cavium/octeon/octeon3-bgx-nexus.c | 698 +++++++
.../net/ethernet/cavium/octeon/octeon3-bgx-port.c | 2033 +++++++++++++++++++
drivers/net/ethernet/cavium/octeon/octeon3-core.c | 2068 ++++++++++++++++++++
drivers/net/ethernet/cavium/octeon/octeon3-pki.c | 832 ++++++++
drivers/net/ethernet/cavium/octeon/octeon3-pko.c | 1719 ++++++++++++++++
drivers/net/ethernet/cavium/octeon/octeon3-sso.c | 309 +++
drivers/net/ethernet/cavium/octeon/octeon3.h | 411 ++++
drivers/staging/octeon/ethernet-defines.h | 6 -
drivers/staging/octeon/ethernet-rx.c | 25 +-
drivers/staging/octeon/ethernet-tx.c | 85 +-
25 files changed, 9069 insertions(+), 143 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/cavium-bgx.txt
create mode 100644 arch/mips/cavium-octeon/octeon-fpa3.c
create mode 100644 arch/mips/cavium-octeon/resource-mgr.c
create mode 100644 drivers/net/ethernet/cavium/octeon/octeon3-bgx-nexus.c
create mode 100644 drivers/net/ethernet/cavium/octeon/octeon3-bgx-port.c
create mode 100644 drivers/net/ethernet/cavium/octeon/octeon3-core.c
create mode 100644 drivers/net/ethernet/cavium/octeon/octeon3-pki.c
create mode 100644 drivers/net/ethernet/cavium/octeon/octeon3-pko.c
create mode 100644 drivers/net/ethernet/cavium/octeon/octeon3-sso.c
create mode 100644 drivers/net/ethernet/cavium/octeon/octeon3.h
--
2.14.3
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 0/4] Move DP phy switch to PHY driver
From: Doug Anderson @ 2017-11-28 23:32 UTC (permalink / raw)
To: Chris Zhong
Cc: Mark Rutland, David Airlie, Catalin Marinas, Shawn Lin,
Will Deacon, Kever Yang, dri-devel, Guenter Roeck, Brian Norris,
Kishon Vijay Abraham I, open list:ARM/Rockchip SoC..., Jianqun Xu,
Caesar Wang, devicetree, Elaine Zhang, Rob Herring, William wu,
Linux ARM, Mark yao, LKML, Tomasz Figa, David Wu,
Enric Balletbo i Serra <enric.ballet>
In-Reply-To: <1486712654-15431-1-git-send-email-zyw@rock-chips.com>
Hi,
On Thu, Feb 9, 2017 at 11:44 PM, Chris Zhong <zyw@rock-chips.com> wrote:
>
> There are 2 Type-c PHYs in RK3399, but only one DP controller. Hence
> only one PHY can connect to DP controller at one time, the other should
> be disconnected. The GRF_SOC_CON26 register has a switch bit to do it,
> set this bit means enable PHY 1, clear this bit means enable PHY 0.
>
> If the board has 2 Type-C ports, the DP driver get the phy id from
> devm_of_phy_get_by_index, and then control this switch according to
> this id. But some others board only has one Type-C port, it may be PHY 0
> or PHY 1. The dts node id can not tell us the correct PHY id. Hence move
> this switch to PHY driver, the PHY driver can distinguish between PHY 0
> and PHY 1, and then write the correct register bit.
>
>
>
> Chris Zhong (4):
> Documentation: bindings: add uphy-dp-sel for Rockchip USB Type-C PHY
> arm64: dts: rockchip: add rockchip,uphy-dp-sel for Type-C phy
> phy: rockchip-typec: support DP phy switch
> drm/rockchip: cdn-dp: remove the DP phy switch
>
> Documentation/devicetree/bindings/phy/phy-rockchip-typec.txt | 5 +++++
> arch/arm64/boot/dts/rockchip/rk3399.dtsi | 2 ++
> drivers/gpu/drm/rockchip/cdn-dp-core.c | 7 -------
> drivers/phy/phy-rockchip-typec.c | 9 +++++++++
> 4 files changed, 16 insertions(+), 7 deletions(-)
What ever happened to this series? It seemed like it just dropped on
the floor...
There was a bit of contention on patch #3
<https://patchwork.kernel.org/patch/9566095/> about the fact that we
were specifying addresses in the device tree vs. hardcoding them in
the driver. Any way we can just make a decision and go with it?
-Doug
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH v3 2/3] clk: meson-axg: add clock controller drivers
From: Yixun Lan @ 2017-11-28 23:16 UTC (permalink / raw)
To: Rob Herring
Cc: yixun.lan, Neil Armstrong, Jerome Brunet, Kevin Hilman,
Mark Rutland, Michael Turquette, Stephen Boyd, Carlo Caione,
Qiufang Dai, linux-amlogic, devicetree, linux-clk,
linux-arm-kernel, linux-kernel
In-Reply-To: <20171128163022.7b7wvfqedkhhnaj6@rob-hp-laptop>
HI Rob
On 11/29/17 00:30, Rob Herring wrote:
> On Tue, Nov 28, 2017 at 08:53:29PM +0800, Yixun Lan wrote:
>> From: Qiufang Dai <qiufang.dai@amlogic.com>
>>
>> Add clock controller drivers for Amlogic Meson-AXG SoC.
>>
>> Signed-off-by: Qiufang Dai <qiufang.dai@amlogic.com>
>> Signed-off-by: Yixun Lan <yixun.lan@amlogic.com>
>> ---
>> arch/arm64/Kconfig.platforms | 1 +
>> drivers/clk/meson/Kconfig | 8 +
>> drivers/clk/meson/Makefile | 1 +
>> drivers/clk/meson/axg.c | 948 +++++++++++++++++++++++++++++++++++
>> drivers/clk/meson/axg.h | 126 +++++
>
>> include/dt-bindings/clock/axg-clkc.h | 72 +++
>
> This belongs in the binding patch.
>
is it Ok to keep it in this patch?
the initial idea to do this is to keep the commit bisectable..
> Otherwise,
>
> Acked-by: Rob Herring <robh@kernel.org>
>
>
>> 6 files changed, 1156 insertions(+)
>> create mode 100644 drivers/clk/meson/axg.c
>> create mode 100644 drivers/clk/meson/axg.h
>> create mode 100644 include/dt-bindings/clock/axg-clkc.h
>
> .
>
^ permalink raw reply
* Re: [PATCH v4 3/5] misc serdev: Add w2sg0004 (gps receiver) power control driver
From: H. Nikolaus Schaller @ 2017-11-28 21:42 UTC (permalink / raw)
To: Andrew F. Davis
Cc: Rob Herring, Mark Rutland, Benoît Cousson, Tony Lindgren,
Russell King, Arnd Bergmann, Greg Kroah-Hartman, Kevin Hilman,
Andreas Färber, Thierry Reding, Jonathan Cameron, DTML,
Linux Kernel Mailing List, linux-omap,
Discussions about the Letux Kernel,
kernel-Jl6IXVxNIMRxAtABVqVhTwC/G2K4zDHf, Linux ARM, Marek Belisko
In-Reply-To: <8dcd4cbb-02eb-b1d4-186e-63823199d5bb-l0cyMroinI0@public.gmane.org>
Hi Andrew,
now as 4.15-rc1 is out I find time to continue this work and prepare [PATCH v5].
> Am 23.11.2017 um 17:06 schrieb Andrew F. Davis <afd-l0cyMroinI0@public.gmane.org>:
>
> On 11/15/2017 03:37 PM, H. Nikolaus Schaller wrote:
>> Add driver for Wi2Wi W2SG0004/84 GPS module connected through uart.
>>
>> Use serdev API hooks to monitor and forward the UART traffic to /dev/ttyGPSn
>> and turn on/off the module. It also detects if the module is turned on (sends data)
>> but should be off, e.g. if it was already turned on during boot or power-on-reset.
>>
>> Additionally, rfkill block/unblock can be used to control an external LNA
>> (and power down the module if not needed).
>>
>> The driver concept is based on code developed by NeilBrown <neilb-l3A5Bk7waGM@public.gmane.org>
>> but simplified and adapted to use the new serdev API introduced in 4.11.
>>
>> Signed-off-by: H. Nikolaus Schaller <hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org>
>> ---
>> drivers/misc/Kconfig | 10 +
>> drivers/misc/Makefile | 1 +
>> drivers/misc/w2sg0004.c | 545 ++++++++++++++++++++++++++++++++++++++++++++++++
I wonder if this shouldn't better go to
drivers/gps
But this directory does not yet exist and has no overall maintainer.
So it could be left in drivers/misc and moved as soon as drivers/gps
is created elsewhere.
>> 3 files changed, 556 insertions(+)
>> create mode 100644 drivers/misc/w2sg0004.c
>>
>> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
>> index 8136dc7e863d..09d171d68408 100644
>> --- a/drivers/misc/Kconfig
>> +++ b/drivers/misc/Kconfig
>> @@ -518,4 +518,14 @@ source "drivers/misc/mic/Kconfig"
>> source "drivers/misc/genwqe/Kconfig"
>> source "drivers/misc/echo/Kconfig"
>> source "drivers/misc/cxl/Kconfig"
>> +
>> +config W2SG0004
>> + tristate "W2SG00x4 on/off control"
>> + depends on GPIOLIB && SERIAL_DEV_BUS
>> + help
>> + Enable on/off control of W2SG00x4 GPS moduled connected
>> + to some SoC UART to allow powering up/down if the /dev/ttyGPSn
>> + is opened/closed.
>> + It also provides a rfkill gps name to control the LNA power.
>> +
>> endmenu
>> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
>> index ad0e64fdba34..abcb667e0ff0 100644
>> --- a/drivers/misc/Makefile
>> +++ b/drivers/misc/Makefile
>> @@ -51,6 +51,7 @@ obj-$(CONFIG_SRAM_EXEC) += sram-exec.o
>> obj-y += mic/
>> obj-$(CONFIG_GENWQE) += genwqe/
>> obj-$(CONFIG_ECHO) += echo/
>> +obj-$(CONFIG_W2SG0004) += w2sg0004.o
>> obj-$(CONFIG_VEXPRESS_SYSCFG) += vexpress-syscfg.o
>> obj-$(CONFIG_CXL_BASE) += cxl/
>> obj-$(CONFIG_ASPEED_LPC_CTRL) += aspeed-lpc-ctrl.o
>> diff --git a/drivers/misc/w2sg0004.c b/drivers/misc/w2sg0004.c
>> new file mode 100644
>> index 000000000000..12e14b5e0a99
>> --- /dev/null
>> +++ b/drivers/misc/w2sg0004.c
>> @@ -0,0 +1,545 @@
>> +// SPDX-License-Identifier: GPL-2.0
>
> Damn this looks ugly, oh well :/
I could remove it for [PATCH v5] ... :\
>
>> +/*
>> + * Driver for power controlling the w2sg0004/w2sg0084 GPS receiver.
>> + *
>
>
> Think you still need copyright tag here somewhere.
At the bottom there is:
>> +MODULE_AUTHOR("NeilBrown <neilb-l3A5Bk7waGM@public.gmane.org>");
>> +MODULE_DESCRIPTION("w2sg0004 GPS power management driver");
>> +MODULE_LICENSE("GPL v2");
Isn't that enough any more?
>
>> + * This receiver has an ON/OFF pin which must be toggled to
>> + * turn the device 'on' of 'off'. A high->low->high toggle
>> + * will switch the device on if it is off, and off if it is on.
>> + *
>> + * To enable receiving on/off requests we register with the
>> + * UART power management notifications.
>> + *
>> + * It is not possible to directly detect the state of the device.
>> + * However when it is on it will send characters on a UART line
>> + * regularly.
>> + *
>> + * To detect that the power state is out of sync (e.g. if GPS
>> + * was enabled before a reboot), we register for UART data received
>> + * notifications.
>> + *
>> + * In addition we register as a rfkill client so that we can
>> + * control the LNA power.
>> + *
>> + */
>> +
>> +#include <linux/delay.h>
>> +#include <linux/err.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/irq.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_irq.h>
>> +#include <linux/of_gpio.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regulator/consumer.h>
>> +#include <linux/rfkill.h>
>> +#include <linux/serdev.h>
>> +#include <linux/sched.h>
>> +#include <linux/slab.h>
>> +#include <linux/tty.h>
>> +#include <linux/tty_flip.h>
>> +#include <linux/workqueue.h>
>> +
>> +/*
>> + * There seems to be restrictions on how quickly we can toggle the
>> + * on/off line. data sheets says "two rtc ticks", whatever that means.
>> + * If we do it too soon it doesn't work.
>> + * So we have a state machine which uses the common work queue to ensure
>> + * clean transitions.
>> + * When a change is requested we record that request and only act on it
>> + * once the previous change has completed.
>> + * A change involves a 10ms low pulse, and a 990ms raised level, so only
>> + * one change per second.
>> + */
>> +
>> +enum w2sg_state {
>> + W2SG_IDLE, /* is not changing state */
>> + W2SG_PULSE, /* activate on/off impulse */
>> + W2SG_NOPULSE /* deactivate on/off impulse */
>> +};
>> +
>> +struct w2sg_data {
>> + struct rfkill *rf_kill;
>> + struct regulator *lna_regulator;
>> + int lna_blocked; /* rfkill block gps active */
>> + int lna_is_off; /* LNA is currently off */
>> + int is_on; /* current state (0/1) */
>> + unsigned long last_toggle;
>> + unsigned long backoff; /* time to wait since last_toggle */
>> + int on_off_gpio; /* the on-off gpio number */
>> + struct serdev_device *uart; /* uart connected to the chip */
>> + struct tty_driver *tty_drv; /* this is the user space tty */
>> + struct device *dev; /* from tty_port_register_device() */
>> + struct tty_port port;
>> + int open_count; /* how often we were opened */
>> + enum w2sg_state state;
>> + int requested; /* requested state (0/1) */
>> + int suspended;
>> + struct delayed_work work;
>> + int discard_count;
>> +};
>> +
>
> Kernel doc style.
Is this really needed here?
For pure driver internal structs (they are not kernel infrastructure API) I usually
consult the source code of a driver and never well formatted kernel doc. Hence I think
readability by programmers looking into the source file is more important than serving
kernel doc tools.
Yes, there are examples like:
https://elixir.free-electrons.com/linux/v4.15-rc1/source/drivers/iio/accel/mma8452.c#L113
But I find them more confusing than helpful because I have to jump between code lines
to match the comment with the data type.
>
>> +static struct w2sg_data *w2sg_by_minor[1];
>> +
>
> If you can only have one right now then just drop the array.
w2sg_get_by_minor() is prepared to access more than one record.
And there are plans to have more than one (but I don't know exactly
how to provide it).
Making it a non-array seems to be an unnecessary hurdle for such
improvements. And regarding memory footprint, a single-element
array is equivalent to a non-array.
>
>> +static int w2sg_set_lna_power(struct w2sg_data *data)
>> +{
>> + int ret = 0;
>> + int off = data->suspended || !data->requested || data->lna_blocked;
>> +
>> + pr_debug("%s: %s\n", __func__, off ? "off" : "on");
>> +
>> + if (off != data->lna_is_off) {
>> + data->lna_is_off = off;
>> + if (!IS_ERR_OR_NULL(data->lna_regulator)) {
>> + if (off)
>> + regulator_disable(data->lna_regulator);
>> + else
>> + ret = regulator_enable(data->lna_regulator);
>> + }
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +static void w2sg_set_power(void *pdata, int val)
>> +{
>> + struct w2sg_data *data = (struct w2sg_data *) pdata;
>> +
>> + pr_debug("%s to state=%d (requested=%d)\n", __func__, val, data->requested);
>> +
>> + if (val && !data->requested) {
>> + data->requested = true;
>> + } else if (!val && data->requested) {
>> + data->backoff = HZ;
>> + data->requested = false;
>> + } else
>> + return;
>> +
>> + pr_debug("w2sg00x4 scheduled for %d\n", data->requested);
>> +
>> + if (!data->suspended)
>> + schedule_delayed_work(&data->work, 0);
>> +}
>> +
>> +/* called each time data is received by the UART (i.e. sent by the w2sg0004) */
>> +
>> +static int w2sg_uart_receive_buf(struct serdev_device *serdev,
>> + const unsigned char *rxdata,
>> + size_t count)
>> +{
>> + struct w2sg_data *data =
>> + (struct w2sg_data *) serdev_device_get_drvdata(serdev);
>> +
>> + if (!data->requested && !data->is_on) {
>> + /*
>> + * we have received characters while the w2sg
>> + * should have been be turned off
>> + */
>> + data->discard_count += count;
>> + if ((data->state == W2SG_IDLE) &&
>> + time_after(jiffies,
>> + data->last_toggle + data->backoff)) {
>> + /* Should be off by now, time to toggle again */
>> + pr_debug("w2sg00x4 has sent %d characters data although it should be off!\n",
>> + data->discard_count);
>> +
>> + data->discard_count = 0;
>> +
>> + data->is_on = true;
>> + data->backoff *= 2;
>> + if (!data->suspended)
>> + schedule_delayed_work(&data->work, 0);
>> + }
>> + } else if (data->open_count > 0) {
>> + int n;
>> +
>> + pr_debug("w2sg00x4: push %lu chars to tty port\n",
>> + (unsigned long) count);
>> +
>> + /* pass to user-space */
>> + n = tty_insert_flip_string(&data->port, rxdata, count);
>> + if (n != count)
>> + pr_err("w2sg00x4: did loose %lu characters\n",
>> + (unsigned long) (count - n));
>> + tty_flip_buffer_push(&data->port);
>> + return n;
>> + }
>> +
>> + /* assume we have processed everything */
>> + return count;
>> +}
>> +
>> +/* try to toggle the power state by sending a pulse to the on-off GPIO */
>> +
>
> drop extra line, same everywhere
Ok! Was just this and one more location.
>
>> +static void toggle_work(struct work_struct *work)
>> +{
>> + struct w2sg_data *data = container_of(work, struct w2sg_data,
>> + work.work);
>> +
>> + switch (data->state) {
>> + case W2SG_IDLE:
>> + if (data->requested == data->is_on)
>> + return;
>> +
>> + w2sg_set_lna_power(data); /* update LNA power state */
>> + gpio_set_value_cansleep(data->on_off_gpio, 0);
>> + data->state = W2SG_PULSE;
>> +
>> + pr_debug("w2sg: power gpio ON\n");
>> +
>> + schedule_delayed_work(&data->work,
>> + msecs_to_jiffies(10));
>> + break;
>> +
>> + case W2SG_PULSE:
>> + gpio_set_value_cansleep(data->on_off_gpio, 1);
>> + data->last_toggle = jiffies;
>> + data->state = W2SG_NOPULSE;
>> + data->is_on = !data->is_on;
>> +
>> + pr_debug("w2sg: power gpio OFF\n");
>> +
>> + schedule_delayed_work(&data->work,
>> + msecs_to_jiffies(10));
>> + break;
>> +
>> + case W2SG_NOPULSE:
>> + data->state = W2SG_IDLE;
>> +
>> + pr_debug("w2sg: idle\n");
>> +
>> + break;
>> +
>> + }
>> +}
>> +
>> +static int w2sg_rfkill_set_block(void *pdata, bool blocked)
>> +{
>> + struct w2sg_data *data = pdata;
>> +
>> + pr_debug("%s: blocked: %d\n", __func__, blocked);
>> +
>> + data->lna_blocked = blocked;
>> +
>> + return w2sg_set_lna_power(data);
>> +}
>> +
>> +static struct rfkill_ops w2sg0004_rfkill_ops = {
>> + .set_block = w2sg_rfkill_set_block,
>> +};
>> +
>> +static struct serdev_device_ops serdev_ops = {
>> + .receive_buf = w2sg_uart_receive_buf,
>> +};
>> +
>> +/*
>> + * we are a man-in the middle between the user-space visible tty port
>> + * and the serdev tty where the chip is connected.
>> + * This allows us to recognise when the device should be powered on
>> + * or off and handle the "false" state that data arrives while no
>> + * users-space tty client exists.
>> + */
>> +
>> +static struct w2sg_data *w2sg_get_by_minor(unsigned int minor)
>> +{
>> + return w2sg_by_minor[minor];
>> +}
>> +
>> +static int w2sg_tty_install(struct tty_driver *driver, struct tty_struct *tty)
>> +{
>> + struct w2sg_data *data;
>> + int retval;
>> +
>> + pr_debug("%s() tty = %p\n", __func__, tty);
>> +
>> + data = w2sg_get_by_minor(tty->index);
>> +
>> + if (!data)
>> + return -ENODEV;
>> +
>> + retval = tty_standard_install(driver, tty);
>> + if (retval)
>> + goto error_init_termios;
>> +
>> + tty->driver_data = data;
>> +
>> + return 0;
>> +
>> +error_init_termios:
>> + tty_port_put(&data->port);
>> + return retval;
>> +}
>> +
>> +static int w2sg_tty_open(struct tty_struct *tty, struct file *file)
>> +{
>> + struct w2sg_data *data = tty->driver_data;
>> +
>> + pr_debug("%s() data = %p open_count = ++%d\n", __func__, data, data->open_count);
>> +
>> + w2sg_set_power(data, ++data->open_count > 0);
>> +
>> + return tty_port_open(&data->port, tty, file);
>> +}
>> +
>> +static void w2sg_tty_close(struct tty_struct *tty, struct file *file)
>> +{
>> + struct w2sg_data *data = tty->driver_data;
>> +
>> + pr_debug("%s()\n", __func__);
>> +
>> + w2sg_set_power(data, --data->open_count > 0);
>> +
>> + tty_port_close(&data->port, tty, file);
>> +}
>> +
>> +static int w2sg_tty_write(struct tty_struct *tty,
>> + const unsigned char *buffer, int count)
>> +{
>> + struct w2sg_data *data = tty->driver_data;
>> +
>> + /* simply pass down to UART */
>> + return serdev_device_write_buf(data->uart, buffer, count);
>> +}
>> +
>> +static const struct tty_operations w2sg_serial_ops = {
>> + .install = w2sg_tty_install,
>> + .open = w2sg_tty_open,
>> + .close = w2sg_tty_close,
>> + .write = w2sg_tty_write,
>> +};
>> +
>> +static const struct tty_port_operations w2sg_port_ops = {
>> +};
>
> drop the brackets? or use NULL below, not sure if this needs memory.
This allocates a struct tty_port_operations initialized with NULL for
all function pointers.
I am not sure if this is the same as providing no w2sg_port_ops at all.
Rather I think the serial core is only port == NULL safe but not
port->ops == NULL e.g.:
http://elixir.free-electrons.com/linux/v4.15-rc1/source/drivers/tty/tty_port.c#L422
A test confirms (see comment below):
>
>> +
>> +static int w2sg_probe(struct serdev_device *serdev)
>> +{
>> + struct w2sg_data *data;
>> + struct rfkill *rf_kill;
>> + int err;
>> + int minor;
>> + enum of_gpio_flags flags;
>> +
>> + pr_debug("%s()\n", __func__);
>> +
>> + minor = 0;
>> + if (w2sg_by_minor[minor]) {
>> + pr_err("w2sg minor is already in use!\n");
>> + return -ENODEV;
>> + }
>> +
>> + data = devm_kzalloc(&serdev->dev, sizeof(*data), GFP_KERNEL);
>> + if (data == NULL)
>> + return -ENOMEM;
>> +
>> + w2sg_by_minor[minor] = data;
While looking through this, I found a potential issue if allocating the
gpio fails with -EPROBE_DEFER.
Then, we have set w2sg_by_minor[minor] != NULL and the above test will already
find it allocated on next deferred probe attempt.
So I'll move that to a position further down.
>> +
>> + serdev_device_set_drvdata(serdev, data);
>> +
>> + data->on_off_gpio = of_get_named_gpio_flags(serdev->dev.of_node,
>> + "enable-gpios", 0,
>> + &flags);
>> + /* defer until we have all gpios */
>> + if (data->on_off_gpio == -EPROBE_DEFER)
>> + return -EPROBE_DEFER;
>> +
>> + data->lna_regulator = devm_regulator_get_optional(&serdev->dev,
>> + "lna");
>> + if (IS_ERR(data->lna_regulator)) {
>> + /* defer until we can get the regulator */
>> + if (PTR_ERR(data->lna_regulator) == -EPROBE_DEFER)
>> + return -EPROBE_DEFER;
>> +
>> + data->lna_regulator = NULL;
>> + }
>> + pr_debug("%s() lna_regulator = %p\n", __func__, data->lna_regulator);
>> +
>> + data->lna_blocked = true;
>> + data->lna_is_off = true;
>> +
>> + data->is_on = false;
>> + data->requested = false;
>> + data->state = W2SG_IDLE;
>> + data->last_toggle = jiffies;
>> + data->backoff = HZ;
>> +
>> + data->uart = serdev;
>> +
>> + INIT_DELAYED_WORK(&data->work, toggle_work);
>> +
>> + err = devm_gpio_request(&serdev->dev, data->on_off_gpio,
>> + "w2sg0004-on-off");
>> + if (err < 0)
>> + goto out;
>> +
>> + gpio_direction_output(data->on_off_gpio, false);
>> +
>> + serdev_device_set_client_ops(data->uart, &serdev_ops);
>> + serdev_device_open(data->uart);
>> +
>> + serdev_device_set_baudrate(data->uart, 9600);
>> + serdev_device_set_flow_control(data->uart, false);
>> +
>> + rf_kill = rfkill_alloc("GPS", &serdev->dev, RFKILL_TYPE_GPS,
>> + &w2sg0004_rfkill_ops, data);
>> + if (rf_kill == NULL) {
>> + err = -ENOMEM;
>> + goto err_rfkill;
>> + }
>> +
>> + err = rfkill_register(rf_kill);
>> + if (err) {
>> + dev_err(&serdev->dev, "Cannot register rfkill device\n");
>> + goto err_rfkill;
>> + }
>> +
>> + data->rf_kill = rf_kill;
>> +
>> + /* allocate the tty driver */
>> + data->tty_drv = alloc_tty_driver(1);
>> + if (!data->tty_drv)
>> + return -ENOMEM;
>> +
>> + /* initialize the tty driver */
>> + data->tty_drv->owner = THIS_MODULE;
>> + data->tty_drv->driver_name = "w2sg0004";
>> + data->tty_drv->name = "ttyGPS";
>> + data->tty_drv->major = 0;
>> + data->tty_drv->minor_start = 0;
>> + data->tty_drv->type = TTY_DRIVER_TYPE_SERIAL;
>> + data->tty_drv->subtype = SERIAL_TYPE_NORMAL;
>> + data->tty_drv->flags = TTY_DRIVER_REAL_RAW | TTY_DRIVER_DYNAMIC_DEV;
>> + data->tty_drv->init_termios = tty_std_termios;
>> + data->tty_drv->init_termios.c_cflag = B9600 | CS8 | CREAD |
>> + HUPCL | CLOCAL;
>> + /*
>> + * optional:
>> + * tty_termios_encode_baud_rate(&data->tty_drv->init_termios,
>> + 115200, 115200);
>> + * w2sg_tty_termios(&data->tty_drv->init_termios);
>> + */
>> + tty_set_operations(data->tty_drv, &w2sg_serial_ops);
>> +
>> + /* register the tty driver */
>> + err = tty_register_driver(data->tty_drv);
>> + if (err) {
>> + pr_err("%s - tty_register_driver failed(%d)\n",
>> + __func__, err);
>> + put_tty_driver(data->tty_drv);
>> + goto err_rfkill;
>> + }
>> +
>> + tty_port_init(&data->port);
>> + data->port.ops = &w2sg_port_ops;
A test setting this to NULL leads to a kernel oops in:
[ 3048.821258] [<c046c598>] (tty_port_open) from [<c0466328>] (tty_open+0x1e8/0x2d8)
So we must define this completely empty struct w2sg_port_ops
and pass a reference.
>> +
>> + pr_debug("w2sg call tty_port_register_device\n");
>> +
>> + data->dev = tty_port_register_device(&data->port,
>> + data->tty_drv, minor, &serdev->dev);
>> +
>> + pr_debug("w2sg probed\n");
>> +
>> + /* keep off until user space requests the device */
>> + w2sg_set_power(data, false);
>> +
>> + return 0;
>> +
>> +err_rfkill:
>> + rfkill_destroy(rf_kill);
>> + serdev_device_close(data->uart);
>> +out:
>> + return err;
>> +}
>> +
>> +static void w2sg_remove(struct serdev_device *serdev)
>> +{
>> + struct w2sg_data *data = serdev_device_get_drvdata(serdev);
>> + int minor;
>> +
>> + cancel_delayed_work_sync(&data->work);
>> +
>> + /* what is the right sequence to avoid problems? */
>> + serdev_device_close(data->uart);
>> +
>> + minor = 0;
>> + tty_unregister_device(data->tty_drv, minor);
>> +
>> + tty_unregister_driver(data->tty_drv);
>> +}
>> +
>> +static int w2sg_suspend(struct device *dev)
>
> __maybe_unused, or if PM is disabled you will get a warning.
Ok.
>
>> +{
>> + struct w2sg_data *data = dev_get_drvdata(dev);
>> +
>> + data->suspended = true;
>> +
>> + cancel_delayed_work_sync(&data->work);
>> +
>> + w2sg_set_lna_power(data); /* shuts down if needed */
>> +
>> + if (data->state == W2SG_PULSE) {
>> + msleep(10);
>> + gpio_set_value_cansleep(data->on_off_gpio, 1);
>> + data->last_toggle = jiffies;
>> + data->is_on = !data->is_on;
>> + data->state = W2SG_NOPULSE;
>> + }
>> +
>> + if (data->state == W2SG_NOPULSE) {
>> + msleep(10);
>> + data->state = W2SG_IDLE;
>> + }
>> +
>> + if (data->is_on) {
>> + pr_info("GPS off for suspend %d %d %d\n", data->requested,
>> + data->is_on, data->lna_is_off);
>> +
>> + gpio_set_value_cansleep(data->on_off_gpio, 0);
>> + msleep(10);
>> + gpio_set_value_cansleep(data->on_off_gpio, 1);
>> + data->is_on = 0;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int w2sg_resume(struct device *dev)
>> +{
>> + struct w2sg_data *data = dev_get_drvdata(dev);
>> +
>> + data->suspended = false;
>> +
>> + pr_info("GPS resuming %d %d %d\n", data->requested,
>> + data->is_on, data->lna_is_off);
>> +
>> + schedule_delayed_work(&data->work, 0); /* enables LNA if needed */
>> +
>> + return 0;
>> +}
>> +
>> +static const struct of_device_id w2sg0004_of_match[] = {
>> + { .compatible = "wi2wi,w2sg0004" },
>> + { .compatible = "wi2wi,w2sg0084" },
>> + {},
>> +};
>> +MODULE_DEVICE_TABLE(of, w2sg0004_of_match);
>> +
>> +SIMPLE_DEV_PM_OPS(w2sg_pm_ops, w2sg_suspend, w2sg_resume);
>> +
>> +static struct serdev_device_driver w2sg_driver = {
>> + .probe = w2sg_probe,
>> + .remove = w2sg_remove,
>> + .driver = {
>> + .name = "w2sg0004",
>> + .owner = THIS_MODULE,
>> + .pm = &w2sg_pm_ops,
>> + .of_match_table = of_match_ptr(w2sg0004_of_match)
>> + },
>> +};
>> +
>> +module_serdev_device_driver(w2sg_driver);
>> +
>> +MODULE_ALIAS("w2sg0004");
>
> Is this needed?
Seems to be redundant.
After removal the driver is still loaded automatically after boot:
root@letux:~# modprobe -c | fgrep w2sg
alias of:N*T*Cwi2wi,w2sg0004 w2sg0004
alias of:N*T*Cwi2wi,w2sg0004C* w2sg0004
alias of:N*T*Cwi2wi,w2sg0084 w2sg0004
alias of:N*T*Cwi2wi,w2sg0084C* w2sg0004
root@letux:~# lsmod | fgrep w2sg
w2sg0004 16384 2
root@letux:~#
>
>> +
>> +MODULE_AUTHOR("NeilBrown <neilb-l3A5Bk7waGM@public.gmane.org>");
>
> Who really wrote this?
Good question.
The problem is that with cleaning up the code and rewriting history over such
a long time, it is no more visible.
But I have searched through our older branches:
Neil Brown had developed the first version for v3.7 in 2013:
http://git.goldelico.com/?p=gta04-kernel.git;a=commit;h=5b6fad7c7f15db8bb8e2c98ae9a50da52bda8b69
This is where the MODULE_AUTHOR line comes from.
Later, Marek Belisko worked on the UART driver, pinmux and interrupts.
And I have
* added lna-regulator and rfkill ca. v3.12
* added device tree support ca. v3.15
* moved to drivers/misc
* ported the code to serdev API ca. v4.11
* submitted to LKML and worked in comments by reviewers ca. 4.15
So this is the first version that is close to be acceptable.
Neil had also submitted different versions to LKML but I am not
sure if he still is active anywhere.
I have also checked a diff between the v3.7 version and the
current one and there are ca. 30-40% of lines original code by
Neil.
So I'd say:
* Neil is the original author and a significant amount of untouched code lines and comments are still there
* he designed the overall driver architecture
* Hence he is the copyright holder, did license under GPL v2 and we have just made a derivative work out of it
* Neil did submit his version of serdev ca. 2015 (quite different from this) but resigned after review feedback
* I did add some important features but not the core code and driver architecture
* I feel my role as maintainer and massaging everything for DT, serdev and get it upstream, but not "the author"
How should we best reflect this in the AUTHOR macro?
>
>> +MODULE_DESCRIPTION("w2sg0004 GPS power management driver");
>> +MODULE_LICENSE("GPL v2");
>>
BR and thanks,
Nikolaus Schaller
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 1/1] ARM: dts: sunxi: Update dts for A20-OLinuXino boards
From: Maxime Ripard @ 2017-11-28 21:32 UTC (permalink / raw)
To: Stefan Mavrodiev
Cc: Rob Herring, Mark Rutland, Russell King, Chen-Yu Tsai,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <1511784231-6485-1-git-send-email-stefan-kyXcfZUBQGPQT0dZR+AlfA@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1900 bytes --]
Hi Stefan,
On Mon, Nov 27, 2017 at 02:03:51PM +0200, Stefan Mavrodiev wrote:
> There will be option with 16MB flash for the following boards:
>
> * A20-OLinuXino-MICRO Rev.K
> * A20-OLinuXino-LIME Rev.J
> * A20-OLinuXino-LIME2 Rev.J
> * A20-SOM-EVB Rev.E
>
> The used flash chip is Winbond W25Q128FV, which is connected to
> SPI0 port. Since this is optional feature, spi0 node is dissabled
> by default. Also this option is incompatible with NAND flash, so
> they shouldn't be enabled at the same time.
>
> Signed-off-by: Stefan Mavrodiev <stefan-kyXcfZUBQGPQT0dZR+AlfA@public.gmane.org>
> ---
> arch/arm/boot/dts/sun7i-a20-olimex-som-evb.dts | 14 ++++++++++++++
> arch/arm/boot/dts/sun7i-a20-olinuxino-lime.dts | 14 ++++++++++++++
> arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts | 14 ++++++++++++++
> arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts | 14 ++++++++++++++
> 4 files changed, 56 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sun7i-a20-olimex-som-evb.dts b/arch/arm/boot/dts/sun7i-a20-olimex-som-evb.dts
> index 64c8ef9..98b7697 100644
> --- a/arch/arm/boot/dts/sun7i-a20-olimex-som-evb.dts
> +++ b/arch/arm/boot/dts/sun7i-a20-olimex-som-evb.dts
> @@ -291,6 +291,20 @@
> status = "okay";
> };
>
> +&spi0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&spi0_pins_b>,<&spi0_cs0_pins_b>;
> + status = "disabled";
> +
> + flash@0 {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + compatible = "winbond,w25q128","jedec,spi-nor";
> + reg = <0>;
> + spi-max-frequency = <40000000>;
> + };
> +};
> +
So those kind of variations are usually best handled using overlays.
This as become quite convenient using the FIT image rework that
Pantelis did here:
https://lists.denx.de/pipermail/u-boot/2017-June/296879.html
and got merged since 2017.11.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* [PATCH] of: enable unittests on UML
From: Rob Herring @ 2017-11-28 21:19 UTC (permalink / raw)
To: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Frank Rowand, Thomas Meyer
The unittests can run on UML, but OF_IRQ and OF_ADDRESS need to be
enabled. Rework the kconfig dependencies to enable the unittests. The
unittests cannot build on Sparc, so we need to add an explicit
dependency for !SPARC.
There's one failure in overlay tests because the base DT is not
unflattened early.
Cc: Thomas Meyer <thomas-VsYtu1Qij5c@public.gmane.org>
Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
drivers/of/Kconfig | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index ad9a9578f9c4..5020d7ef7494 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -13,7 +13,8 @@ if OF
config OF_UNITTEST
bool "Device Tree runtime unit tests"
- depends on OF_IRQ
+ depends on !SPARC
+ select IRQ_DOMAIN
select OF_EARLY_FLATTREE
select OF_RESOLVE
help
@@ -61,7 +62,7 @@ config OF_DYNAMIC
config OF_ADDRESS
def_bool y
- depends on !SPARC && HAS_IOMEM
+ depends on !SPARC && (HAS_IOMEM || UML)
select OF_ADDRESS_PCI if PCI
config OF_ADDRESS_PCI
--
2.14.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH 3/5] PCI: cadence: Add host driver for Cadence PCIe controller
From: Bjorn Helgaas @ 2017-11-28 20:46 UTC (permalink / raw)
To: Cyrille Pitchen
Cc: bhelgaas, kishon, lorenzo.pieralisi, linux-pci, adouglas,
stelford, dgary, kgopi, eandrews, thomas.petazzoni, sureshp,
nsekhar, linux-kernel, robh, devicetree
In-Reply-To: <20171128204114.GE11228@bhelgaas-glaptop.roam.corp.google.com>
On Tue, Nov 28, 2017 at 02:41:14PM -0600, Bjorn Helgaas wrote:
> On Thu, Nov 23, 2017 at 04:01:48PM +0100, Cyrille Pitchen wrote:
> > diff --git a/drivers/Makefile b/drivers/Makefile
> > index 1d034b680431..27bdd98784d9 100644
> > --- a/drivers/Makefile
> > +++ b/drivers/Makefile
> > @@ -18,6 +18,7 @@ obj-y += pwm/
> >
> > obj-$(CONFIG_PCI) += pci/
> > obj-$(CONFIG_PCI_ENDPOINT) += pci/endpoint/
> > +obj-$(CONFIG_PCI_CADENCE) += pci/cadence/
>
> I can't remember why we added CONFIG_PCI_ENDPOINT here instead of in
> drivers/pci/Makefile. Is there any way to move both CONFIG_PCI_ENDPOINT
> and CONFIG_PCI_CADENCE into drivers/pci/Makefile so this is better
> encapsulated?
I now see the note in your cover letter. If there is a reason for
this, it should go in the changelog to forestall questions like this.
But what's in the cover letter isn't quite enough because it doesn't
answer the question of why drivers/pci/Makefile isn't sufficient to
control the ordering of pci/dwc and pci/endpoint.
And sorry, I now see Lorenzo's similar query so he's already jumped on
this :)
> > # PCI dwc controller drivers
> > obj-y += pci/dwc/
^ permalink raw reply
* Re: [PATCH 3/5] PCI: cadence: Add host driver for Cadence PCIe controller
From: Bjorn Helgaas @ 2017-11-28 20:41 UTC (permalink / raw)
To: Cyrille Pitchen
Cc: bhelgaas, kishon, lorenzo.pieralisi, linux-pci, adouglas,
stelford, dgary, kgopi, eandrews, thomas.petazzoni, sureshp,
nsekhar, linux-kernel, robh, devicetree
In-Reply-To: <2670f7ddf59e708beb9d32bef1353e15bd4e1ecf.1511439189.git.cyrille.pitchen@free-electrons.com>
On Thu, Nov 23, 2017 at 04:01:48PM +0100, Cyrille Pitchen wrote:
> This patch adds support to the Cadence PCIe controller in host mode.
>
> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@free-electrons.com>
> ---
> drivers/Makefile | 1 +
> drivers/pci/Kconfig | 1 +
> drivers/pci/cadence/Kconfig | 24 ++
> drivers/pci/cadence/Makefile | 2 +
> drivers/pci/cadence/pcie-cadence-host.c | 425 ++++++++++++++++++++++++++++++++
> drivers/pci/cadence/pcie-cadence.c | 110 +++++++++
> drivers/pci/cadence/pcie-cadence.h | 325 ++++++++++++++++++++++++
> 7 files changed, 888 insertions(+)
> create mode 100644 drivers/pci/cadence/Kconfig
> create mode 100644 drivers/pci/cadence/Makefile
> create mode 100644 drivers/pci/cadence/pcie-cadence-host.c
> create mode 100644 drivers/pci/cadence/pcie-cadence.c
> create mode 100644 drivers/pci/cadence/pcie-cadence.h
I prefer a single file per driver. I assume you're anticipating
something like dwc, where the DesignWare core is incorporated into
several devices in slightly different ways. But it doesn't look like
that's here yet, and personally, I'd rather split out the required
things when they actually become required, not ahead of time.
> diff --git a/drivers/Makefile b/drivers/Makefile
> index 1d034b680431..27bdd98784d9 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -18,6 +18,7 @@ obj-y += pwm/
>
> obj-$(CONFIG_PCI) += pci/
> obj-$(CONFIG_PCI_ENDPOINT) += pci/endpoint/
> +obj-$(CONFIG_PCI_CADENCE) += pci/cadence/
I can't remember why we added CONFIG_PCI_ENDPOINT here instead of in
drivers/pci/Makefile. Is there any way to move both CONFIG_PCI_ENDPOINT
and CONFIG_PCI_CADENCE into drivers/pci/Makefile so this is better
encapsulated?
> # PCI dwc controller drivers
> obj-y += pci/dwc/
>...
> + * struct cdns_pcie_rc_data - hardware specific data
"cdns" is a weird abbreviation for "Cadence", since "Cadence" doesn't
contain an "s".
>...
> +static int cdns_pcie_parse_request_of_pci_ranges(struct device *dev,
> + struct list_head *resources,
> + struct resource **bus_range)
> +{
> + int err, res_valid = 0;
> + struct device_node *np = dev->of_node;
> + resource_size_t iobase;
> + struct resource_entry *win, *tmp;
> +
> + err = of_pci_get_host_bridge_resources(np, 0, 0xff, resources, &iobase);
> + if (err)
> + return err;
> +
> + err = devm_request_pci_bus_resources(dev, resources);
> + if (err)
> + return err;
> +
> + resource_list_for_each_entry_safe(win, tmp, resources) {
> + struct resource *res = win->res;
> +
> + switch (resource_type(res)) {
> + case IORESOURCE_IO:
> + err = pci_remap_iospace(res, iobase);
> + if (err) {
> + dev_warn(dev, "error %d: failed to map resource %pR\n",
> + err, res);
> + resource_list_destroy_entry(win);
> + }
> + break;
> + case IORESOURCE_MEM:
> + res_valid |= !(res->flags & IORESOURCE_PREFETCH);
> + break;
> + case IORESOURCE_BUS:
> + *bus_range = res;
> + break;
> + }
> + }
> +
> + if (res_valid)
> + return 0;
> +
> + dev_err(dev, "non-prefetchable memory resource required\n");
> + return -EINVAL;
> +}
The code above is starting to look awfully familiar. I wonder if it's
time to think about some PCI-internal interface that can encapsulate
this. In this case, there's really nothing Cadence-specific here.
There are other callers where there *is* vendor-specific code, but
possibly that could be handled by returning pointers to bus number,
I/O port, and MMIO resources so the caller could do the
vendor-specific stuff?
Bjorn
^ permalink raw reply
* Re: [PATCH v1] usb: xhci: allow imod-interval to be configurable
From: Adam Wallis @ 2017-11-28 20:32 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA, Mathias Nyman,
timur-sgV2jX0FEOL9JmXXK+q4OQ, linux-usb-u79uwXL29TY76Z2rM5mHXA,
chunfeng.yun-NuS5LvNUpcJWk0Htik3J/w, Rob Herring,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Matthias Brugger,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20171128193500.GA30609-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>
On 11/28/2017 2:35 PM, Greg Kroah-Hartman wrote:
> On Tue, Nov 28, 2017 at 12:11:46PM -0500, Adam Wallis wrote:
>> The xHCI driver currently has the IMOD set to 160, which
>> translates to an IMOD interval of 40,000ns (160 * 250)ns
>>
[..]
>> --- a/drivers/usb/host/xhci-plat.c
>> +++ b/drivers/usb/host/xhci-plat.c
>> @@ -23,6 +23,7 @@
>> #include "xhci-plat.h"
>> #include "xhci-mvebu.h"
>> #include "xhci-rcar.h"
>> +#include "xhci-mtk.h"
>>
>> static struct hc_driver __read_mostly xhci_plat_hc_driver;
>>
>> @@ -269,6 +270,20 @@ static int xhci_plat_probe(struct platform_device *pdev)
>> if (device_property_read_bool(&pdev->dev, "quirk-broken-port-ped"))
>> xhci->quirks |= XHCI_BROKEN_PORT_PED;
>>
>> + /* imod interval in nanoseconds */
>> + if (device_property_read_u32(sysdev,
>> + "imod-interval", &xhci->imod_interval))
>> + xhci->imod_interval = 40000;
>
> So no matter what value you read, you set it to 40000? Or am I reading
> this code incorrectly?
I think you may be reading the code incorrectly. device_property_read_u32()
returns 0 when the property is found and valid...and stored into
xhci->imod_interval. When 0 is returned in this case, the default value of
40,000 is skipped over.
If the property is not found, a number of different errors could be returned,
but any of them will cause the default value to be used.
> There's a good reason putting function calls inside if() is considered a
> bad coding style :)
I do not disagree with you, however, I was trying to maintain style consistency
with the device property reads with the xhci_plat_probe function.
If I break that consistency, a couple of ways I might write this cleaner
1) set xhci->imod_interval to 40,000 before the call to
device_property_read_u32. If the property exists in a firmware node, it will
update the imod_interval value...if it does not exist, it will not update this
value and the default will be used. In this case, I would not even check the
return value. This method is used quite a bit in the kernel.
2) use a if (device_property_present()) and check to see if that property is
even present. If so, call device_property_read_u32() without check return value.
This has the downside of still using a function call within the if() which you
have already indicated is not your preference.
3) add a retval and test off of this instead of using the function directly in
the if()
> thanks,
>
> greg k-h
Thanks for taking time to review my patch. I really have no strong preference on
how call device_property_read_u32() is tested and I am open to any suggestions.
Adam
--
Adam Wallis
Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Applied "spi: meson-axg: update compatible string for the Meson-AXG" to the spi tree
From: Mark Brown @ 2017-11-28 20:22 UTC (permalink / raw)
To: Sunny Luo; +Cc: Yixun Lan, Rob Herring, Mark Brown, Kevin Hilman, devicetree
In-Reply-To: <20171128132926.19051-2-yixun.lan@amlogic.com>
The patch
spi: meson-axg: update compatible string for the Meson-AXG
has been applied to the spi tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
>From a78841479696efa557a2251c86462c936a208f90 Mon Sep 17 00:00:00 2001
From: Sunny Luo <sunny.luo@amlogic.com>
Date: Tue, 28 Nov 2017 21:29:24 +0800
Subject: [PATCH] spi: meson-axg: update compatible string for the Meson-AXG
Update the compatbile string to support Meson-AXG SoCs.
Signed-off-by: Sunny Luo <sunny.luo@amlogic.com>
Signed-off-by: Yixun Lan <yixun.lan@amlogic.com>
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
Documentation/devicetree/bindings/spi/spi-meson.txt | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/spi/spi-meson.txt b/Documentation/devicetree/bindings/spi/spi-meson.txt
index 825c39cae74a..b7f5e86fed22 100644
--- a/Documentation/devicetree/bindings/spi/spi-meson.txt
+++ b/Documentation/devicetree/bindings/spi/spi-meson.txt
@@ -27,7 +27,9 @@ The Meson SPICC is generic SPI controller for general purpose Full-Duplex
communications with dedicated 16 words RX/TX PIO FIFOs.
Required properties:
- - compatible: should be "amlogic,meson-gx-spicc" on Amlogic GX SoCs.
+ - compatible: should be:
+ "amlogic,meson-gx-spicc" on Amlogic GX and compatible SoCs.
+ "amlogic,meson-axg-spicc" on Amlogic AXG and compatible SoCs
- reg: physical base address and length of the controller registers
- interrupts: The interrupt specifier
- clock-names: Must contain "core"
--
2.15.0
^ permalink raw reply related
* Applied "spi: meson-axg: add SPICC driver support" to the spi tree
From: Mark Brown @ 2017-11-28 20:22 UTC (permalink / raw)
To: Sunny Luo
Cc: Yixun Lan, Mark Brown, Kevin Hilman,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20171128132926.19051-3-yixun.lan-LpR1jeaWuhtBDgjK7y7TUQ@public.gmane.org>
The patch
spi: meson-axg: add SPICC driver support
has been applied to the spi tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
>From a5db27c00da37654ba518b814925d4e9cd05259c Mon Sep 17 00:00:00 2001
From: Sunny Luo <sunny.luo-LpR1jeaWuhtBDgjK7y7TUQ@public.gmane.org>
Date: Tue, 28 Nov 2017 21:29:25 +0800
Subject: [PATCH] spi: meson-axg: add SPICC driver support
Add new compatible string to support SPICC controller which
found at Amlogic Meson-AXG SoC. This is aiming at adding
a couple of enhanced feature patches.
Signed-off-by: Sunny Luo <sunny.luo-LpR1jeaWuhtBDgjK7y7TUQ@public.gmane.org>
Signed-off-by: Yixun Lan <yixun.lan-LpR1jeaWuhtBDgjK7y7TUQ@public.gmane.org>
Signed-off-by: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
drivers/spi/spi-meson-spicc.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/spi/spi-meson-spicc.c b/drivers/spi/spi-meson-spicc.c
index 7f8429635502..5c82910e3480 100644
--- a/drivers/spi/spi-meson-spicc.c
+++ b/drivers/spi/spi-meson-spicc.c
@@ -599,6 +599,7 @@ static int meson_spicc_remove(struct platform_device *pdev)
static const struct of_device_id meson_spicc_of_match[] = {
{ .compatible = "amlogic,meson-gx-spicc", },
+ { .compatible = "amlogic,meson-axg-spicc", },
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, meson_spicc_of_match);
--
2.15.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH 1/3] dt-bindings: spicc: update compatible string for the Meson-AXG
From: Mark Brown @ 2017-11-28 20:20 UTC (permalink / raw)
To: Yixun Lan
Cc: Kevin Hilman, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-spi-u79uwXL29TY76Z2rM5mHXA, Neil Armstrong, Jerome Brunet,
Rob Herring, Mark Rutland, Carlo Caione, Sunny Luo,
linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20171128132926.19051-2-yixun.lan-LpR1jeaWuhtBDgjK7y7TUQ@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 486 bytes --]
On Tue, Nov 28, 2017 at 09:29:24PM +0800, Yixun Lan wrote:
> From: Sunny Luo <sunny.luo-LpR1jeaWuhtBDgjK7y7TUQ@public.gmane.org>
>
> Update the compatbile string to support Meson-AXG SoCs.
Please submit patches using subject lines reflecting the style for the
subsystem. This makes it easier for people to identify relevant
patches. Look at what existing commits in the area you're changing are
doing and make sure your subject lines visually resemble what they're
doing.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 484 bytes --]
^ permalink raw reply
* [PATCH 2/2] arm64: dts: renesas: r8a77970: use SYSC power domain macros
From: Sergei Shtylyov @ 2017-11-28 20:15 UTC (permalink / raw)
To: Simon Horman, Rob Herring, Catalin Marinas, Will Deacon,
linux-renesas-soc, devicetree
Cc: Magnus Damm, Mark Rutland, linux-arm-kernel, Sergei Shtylyov
[-- Attachment #1: arm64-dts-renesas-r8a77970-use-SYSC-power-domain-macros.patch --]
[-- Type: text/plain, Size: 4948 bytes --]
Now that the commit 833bdb47c826 ("dt-bindings: power: add R8A77970 SYSC
power domain definitions") has hit Linus' tree, we can replace the bare
numbers (we had to use to avoid a cross tree dependency) with these macro
definitions...
Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
---
arch/arm64/boot/dts/renesas/r8a77970.dtsi | 32 +++++++++++++++---------------
1 file changed, 16 insertions(+), 16 deletions(-)
Index: renesas/arch/arm64/boot/dts/renesas/r8a77970.dtsi
===================================================================
--- renesas.orig/arch/arm64/boot/dts/renesas/r8a77970.dtsi
+++ renesas/arch/arm64/boot/dts/renesas/r8a77970.dtsi
@@ -33,14 +33,14 @@
compatible = "arm,cortex-a53", "arm,armv8";
reg = <0>;
clocks = <&cpg CPG_CORE R8A77970_CLK_Z2>;
- power-domains = <&sysc 5>;
+ power-domains = <&sysc R8A77970_PD_CA53_CPU0>;
next-level-cache = <&L2_CA53>;
enable-method = "psci";
};
L2_CA53: cache-controller {
compatible = "cache";
- power-domains = <&sysc 21>;
+ power-domains = <&sysc R8A77970_PD_CA53_SCU>;
cache-unified;
cache-level = <2>;
};
@@ -88,7 +88,7 @@
IRQ_TYPE_LEVEL_HIGH)>;
clocks = <&cpg CPG_MOD 408>;
clock-names = "clk";
- power-domains = <&sysc 32>;
+ power-domains = <&sysc R8A77970_PD_ALWAYS_ON>;
resets = <&cpg 408>;
};
@@ -109,7 +109,7 @@
"renesas,rcar-gen3-wdt";
reg = <0 0xe6020000 0 0x0c>;
clocks = <&cpg CPG_MOD 402>;
- power-domains = <&sysc 32>;
+ power-domains = <&sysc R8A77970_PD_ALWAYS_ON>;
resets = <&cpg 402>;
status = "disabled";
};
@@ -190,7 +190,7 @@
GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH
GIC_SPI 161 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&cpg CPG_MOD 407>;
- power-domains = <&sysc 32>;
+ power-domains = <&sysc R8A77970_PD_ALWAYS_ON>;
resets = <&cpg 407>;
};
@@ -217,7 +217,7 @@
"ch4", "ch5", "ch6", "ch7";
clocks = <&cpg CPG_MOD 218>;
clock-names = "fck";
- power-domains = <&sysc 32>;
+ power-domains = <&sysc R8A77970_PD_ALWAYS_ON>;
resets = <&cpg 218>;
#dma-cells = <1>;
dma-channels = <8>;
@@ -245,7 +245,7 @@
"ch4", "ch5", "ch6", "ch7";
clocks = <&cpg CPG_MOD 217>;
clock-names = "fck";
- power-domains = <&sysc 32>;
+ power-domains = <&sysc R8A77970_PD_ALWAYS_ON>;
resets = <&cpg 217>;
#dma-cells = <1>;
dma-channels = <8>;
@@ -268,7 +268,7 @@
dmas = <&dmac1 0x31>, <&dmac1 0x30>,
<&dmac2 0x31>, <&dmac2 0x30>;
dma-names = "tx", "rx", "tx", "rx";
- power-domains = <&sysc 32>;
+ power-domains = <&sysc R8A77970_PD_ALWAYS_ON>;
resets = <&cpg 520>;
status = "disabled";
};
@@ -286,7 +286,7 @@
dmas = <&dmac1 0x33>, <&dmac1 0x32>,
<&dmac2 0x33>, <&dmac2 0x32>;
dma-names = "tx", "rx", "tx", "rx";
- power-domains = <&sysc 32>;
+ power-domains = <&sysc R8A77970_PD_ALWAYS_ON>;
resets = <&cpg 519>;
status = "disabled";
};
@@ -304,7 +304,7 @@
dmas = <&dmac1 0x35>, <&dmac1 0x34>,
<&dmac2 0x35>, <&dmac2 0x34>;
dma-names = "tx", "rx", "tx", "rx";
- power-domains = <&sysc 32>;
+ power-domains = <&sysc R8A77970_PD_ALWAYS_ON>;
resets = <&cpg 518>;
status = "disabled";
};
@@ -321,7 +321,7 @@
dmas = <&dmac1 0x37>, <&dmac1 0x36>,
<&dmac2 0x37>, <&dmac2 0x36>;
dma-names = "tx", "rx", "tx", "rx";
- power-domains = <&sysc 32>;
+ power-domains = <&sysc R8A77970_PD_ALWAYS_ON>;
resets = <&cpg 517>;
status = "disabled";
};
@@ -339,7 +339,7 @@
dmas = <&dmac1 0x51>, <&dmac1 0x50>,
<&dmac2 0x51>, <&dmac2 0x50>;
dma-names = "tx", "rx", "tx", "rx";
- power-domains = <&sysc 32>;
+ power-domains = <&sysc R8A77970_PD_ALWAYS_ON>;
resets = <&cpg 207>;
status = "disabled";
};
@@ -357,7 +357,7 @@
dmas = <&dmac1 0x53>, <&dmac1 0x52>,
<&dmac2 0x53>, <&dmac2 0x52>;
dma-names = "tx", "rx", "tx", "rx";
- power-domains = <&sysc 32>;
+ power-domains = <&sysc R8A77970_PD_ALWAYS_ON>;
resets = <&cpg 206>;
status = "disabled";
};
@@ -375,7 +375,7 @@
dmas = <&dmac1 0x57>, <&dmac1 0x56>,
<&dmac2 0x57>, <&dmac2 0x56>;
dma-names = "tx", "rx", "tx", "rx";
- power-domains = <&sysc 32>;
+ power-domains = <&sysc R8A77970_PD_ALWAYS_ON>;
resets = <&cpg 204>;
status = "disabled";
};
@@ -392,7 +392,7 @@
dmas = <&dmac1 0x59>, <&dmac1 0x58>,
<&dmac2 0x59>, <&dmac2 0x58>;
dma-names = "tx", "rx", "tx", "rx";
- power-domains = <&sysc 32>;
+ power-domains = <&sysc R8A77970_PD_ALWAYS_ON>;
resets = <&cpg 203>;
status = "disabled";
};
@@ -434,7 +434,7 @@
"ch20", "ch21", "ch22", "ch23",
"ch24";
clocks = <&cpg CPG_MOD 812>;
- power-domains = <&sysc 32>;
+ power-domains = <&sysc R8A77970_PD_ALWAYS_ON>;
resets = <&cpg 812>;
phy-mode = "rgmii-id";
iommus = <&ipmmu_rt 3>;
^ permalink raw reply
* [PATCH 1/2] arm64: dts: renesas: r8a77970: use CPG core clock macros
From: Sergei Shtylyov @ 2017-11-28 20:15 UTC (permalink / raw)
To: Simon Horman, Rob Herring, Catalin Marinas, Will Deacon,
linux-renesas-soc, devicetree
Cc: Magnus Damm, Mark Rutland, linux-arm-kernel, Sergei Shtylyov
[-- Attachment #1: arm64-dts-renesas-r8a77970-use-CPG-core-clock-macros.patch --]
[-- Type: text/plain, Size: 3735 bytes --]
Now that the commit ecadea00f588 ("dt-bindings: clock: Add R8A77970 CPG
core clock definitions") has hit Linus' tree, we can replace the bare
numbers (we had to use to avoid a cross tree dependency) with these macro
definitions...
Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
---
arch/arm64/boot/dts/renesas/r8a77970.dtsi | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
Index: renesas/arch/arm64/boot/dts/renesas/r8a77970.dtsi
===================================================================
--- renesas.orig/arch/arm64/boot/dts/renesas/r8a77970.dtsi
+++ renesas/arch/arm64/boot/dts/renesas/r8a77970.dtsi
@@ -9,7 +9,7 @@
* kind, whether express or implied.
*/
-#include <dt-bindings/clock/renesas-cpg-mssr.h>
+#include <dt-bindings/clock/r8a77970-cpg-mssr.h>
#include <dt-bindings/interrupt-controller/arm-gic.h>
#include <dt-bindings/interrupt-controller/irq.h>
#include <dt-bindings/power/r8a77970-sysc.h>
@@ -32,7 +32,7 @@
device_type = "cpu";
compatible = "arm,cortex-a53", "arm,armv8";
reg = <0>;
- clocks = <&cpg CPG_CORE 0>;
+ clocks = <&cpg CPG_CORE R8A77970_CLK_Z2>;
power-domains = <&sysc 5>;
next-level-cache = <&L2_CA53>;
enable-method = "psci";
@@ -262,7 +262,7 @@
reg = <0 0xe6540000 0 96>;
interrupts = <GIC_SPI 154 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&cpg CPG_MOD 520>,
- <&cpg CPG_CORE 9>,
+ <&cpg CPG_CORE R8A77970_CLK_S2D1>,
<&scif_clk>;
clock-names = "fck", "brg_int", "scif_clk";
dmas = <&dmac1 0x31>, <&dmac1 0x30>,
@@ -280,7 +280,7 @@
reg = <0 0xe6550000 0 96>;
interrupts = <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&cpg CPG_MOD 519>,
- <&cpg CPG_CORE 9>,
+ <&cpg CPG_CORE R8A77970_CLK_S2D1>,
<&scif_clk>;
clock-names = "fck", "brg_int", "scif_clk";
dmas = <&dmac1 0x33>, <&dmac1 0x32>,
@@ -298,7 +298,7 @@
reg = <0 0xe6560000 0 96>;
interrupts = <GIC_SPI 144 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&cpg CPG_MOD 518>,
- <&cpg CPG_CORE 9>,
+ <&cpg CPG_CORE R8A77970_CLK_S2D1>,
<&scif_clk>;
clock-names = "fck", "brg_int", "scif_clk";
dmas = <&dmac1 0x35>, <&dmac1 0x34>,
@@ -315,7 +315,7 @@
reg = <0 0xe66a0000 0 96>;
interrupts = <GIC_SPI 145 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&cpg CPG_MOD 517>,
- <&cpg CPG_CORE 9>,
+ <&cpg CPG_CORE R8A77970_CLK_S2D1>,
<&scif_clk>;
clock-names = "fck", "brg_int", "scif_clk";
dmas = <&dmac1 0x37>, <&dmac1 0x36>,
@@ -333,7 +333,7 @@
reg = <0 0xe6e60000 0 64>;
interrupts = <GIC_SPI 152 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&cpg CPG_MOD 207>,
- <&cpg CPG_CORE 9>,
+ <&cpg CPG_CORE R8A77970_CLK_S2D1>,
<&scif_clk>;
clock-names = "fck", "brg_int", "scif_clk";
dmas = <&dmac1 0x51>, <&dmac1 0x50>,
@@ -351,7 +351,7 @@
reg = <0 0xe6e68000 0 64>;
interrupts = <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&cpg CPG_MOD 206>,
- <&cpg CPG_CORE 9>,
+ <&cpg CPG_CORE R8A77970_CLK_S2D1>,
<&scif_clk>;
clock-names = "fck", "brg_int", "scif_clk";
dmas = <&dmac1 0x53>, <&dmac1 0x52>,
@@ -369,7 +369,7 @@
reg = <0 0xe6c50000 0 64>;
interrupts = <GIC_SPI 23 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&cpg CPG_MOD 204>,
- <&cpg CPG_CORE 9>,
+ <&cpg CPG_CORE R8A77970_CLK_S2D1>,
<&scif_clk>;
clock-names = "fck", "brg_int", "scif_clk";
dmas = <&dmac1 0x57>, <&dmac1 0x56>,
@@ -386,7 +386,7 @@
reg = <0 0xe6c40000 0 64>;
interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&cpg CPG_MOD 203>,
- <&cpg CPG_CORE 9>,
+ <&cpg CPG_CORE R8A77970_CLK_S2D1>,
<&scif_clk>;
clock-names = "fck", "brg_int", "scif_clk";
dmas = <&dmac1 0x59>, <&dmac1 0x58>,
^ permalink raw reply
* [PATCH 0/2] Use R8A77970 CPG core clock and SYSC pwoer domain macros
From: Sergei Shtylyov @ 2017-11-28 20:15 UTC (permalink / raw)
To: Simon Horman, Rob Herring, Catalin Marinas, Will Deacon,
linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: Magnus Damm, Mark Rutland,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Sergei Shtylyov
Hello!
Here's the set of 2 patches against Simon Horman's 'renesas.git' repo's
'renesas-devel-20171128-v4.15-rc1' tag. Now that the R8A77970 CPG core clock
and SYSC power domain #define's have hit Linus' tree, we can replace the bare
numbers (we had to use to avoid a cross tree dependencies) with these #define's,
at last...
[1/2] arm64: dts: renesas: r8a77970: use CPG core clock macros
[2/2] arm64: dts: renesas: r8a77970: use SYSC power domain macros
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH V4 3/6] iommu/arm-smmu: Invoke pm_runtime during probe, add/remove device
From: Rob Clark @ 2017-11-28 20:05 UTC (permalink / raw)
To: Vivek Gautam
Cc: Stephen Boyd, Robin Murphy, Will Deacon, Rafael J. Wysocki,
Sricharan R, Joerg Roedel, Rob Herring, Mark Rutland,
Marek Szyprowski,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Linux Kernel Mailing List, linux-clk, linux-arm-msm,
Stanimir Varbanov, Archit Taneja
In-Reply-To: <3a2f74e9-90cf-d843-d801-15eb614d7abe-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
On Tue, Nov 28, 2017 at 8:43 AM, Vivek Gautam
<vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
>
>
> On 11/28/2017 05:13 AM, Rob Clark wrote:
>>
>> On Mon, Nov 27, 2017 at 5:22 PM, Stephen Boyd<sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>> wrote:
>>>
>>> On 11/15, Vivek Gautam wrote:
>>>>
>>>> Hi,
>>>>
>>>>
>>>> On Mon, Aug 7, 2017 at 5:59 PM, Rob Clark<robdclark-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>>>
>>>>> On Mon, Aug 7, 2017 at 4:27 AM, Vivek Gautam
>>>>> <vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
>>>>>>
>>>>>> On Thu, Jul 13, 2017 at 5:20 PM, Rob Clark<robdclark-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>>>> wrote:
>>>>>>>
>>>>>>> On Thu, Jul 13, 2017 at 1:35 AM, Sricharan
>>>>>>> R<sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
>>>>>>>>
>>>>>>>> Hi Vivek,
>>>>>>>>
>>>>>>>> On 7/13/2017 10:43 AM, Vivek Gautam wrote:
>>>>>>>>>
>>>>>>>>> Hi Stephen,
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 07/13/2017 04:24 AM, Stephen Boyd wrote:
>>>>>>>>>>
>>>>>>>>>> On 07/06, Vivek Gautam wrote:
>>>>>>>>>>>
>>>>>>>>>>> @@ -1231,12 +1237,18 @@ static int arm_smmu_map(struct
>>>>>>>>>>> iommu_domain *domain, unsigned long iova,
>>>>>>>>>>> static size_t arm_smmu_unmap(struct iommu_domain *domain,
>>>>>>>>>>> unsigned long iova,
>>>>>>>>>>> size_t size)
>>>>>>>>>>> {
>>>>>>>>>>> - struct io_pgtable_ops *ops =
>>>>>>>>>>> to_smmu_domain(domain)->pgtbl_ops;
>>>>>>>>>>> + struct arm_smmu_domain *smmu_domain =
>>>>>>>>>>> to_smmu_domain(domain);
>>>>>>>>>>> + struct io_pgtable_ops *ops = smmu_domain->pgtbl_ops;
>>>>>>>>>>> + size_t ret;
>>>>>>>>>>> if (!ops)
>>>>>>>>>>> return 0;
>>>>>>>>>>> - return ops->unmap(ops, iova, size);
>>>>>>>>>>> + pm_runtime_get_sync(smmu_domain->smmu->dev);
>>>>>>>>>>
>>>>>>>>>> Can these map/unmap ops be called from an atomic context? I seem
>>>>>>>>>> to recall that being a problem before.
>>>>>>>>>
>>>>>>>>> That's something which was dropped in the following patch merged in
>>>>>>>>> master:
>>>>>>>>> 523d7423e21b iommu/arm-smmu: Remove io-pgtable spinlock
>>>>>>>>>
>>>>>>>>> Looks like we don't need locks here anymore?
>>>>>>>>
>>>>>>>> Apart from the locking, wonder why a explicit pm_runtime is needed
>>>>>>>> from unmap. Somehow looks like some path in the master using that
>>>>>>>> should have enabled the pm ?
>>>>>>>>
>>>>>>> Yes, there are a bunch of scenarios where unmap can happen with
>>>>>>> disabled master (but not in atomic context).
>>>>>>
>>>>>> I would like to understand whether there is a situation where an unmap
>>>>>> is
>>>>>> called in atomic context without an enabled master?
>>>>>>
>>>>>> Let's say we have the case where all the unmap calls in atomic context
>>>>>> happen
>>>>>> only from the master's context (in which case the device link should
>>>>>> take care of
>>>>>> the pm state of smmu), and the only unmap that happen in non-atomic
>>>>>> context
>>>>>> is the one with master disabled. In such a case doesn it make sense to
>>>>>> distinguish
>>>>>> the atomic/non-atomic context and add pm_runtime_get_sync()/put_sync()
>>>>>> only
>>>>>> for the non-atomic context since that would be the one with master
>>>>>> disabled.
>>>>>>
>>>>> At least drm/msm needs to hold obj->lock (a mutex) in unmap, so it
>>>>> won't unmap anything in atomic ctx (but it can unmap w/ master
>>>>> disabled). I can't really comment about other non-gpu drivers. It
>>>>> seems like a reasonable constraint that either master is enabled or
>>>>> not in atomic ctx.
>>>>>
>>>>> Currently we actually wrap unmap w/ pm_runtime_get/put_sync(), but I'd
>>>>> like to drop that to avoid powering up the gpu.
>>>>
>>>> Since the deferring the TLB maintenance doesn't look like the best
>>>> approach [1],
>>>> how about if we try to power-up only the smmu from different client
>>>> devices such as,
>>>> GPU in the unmap path. Then we won't need to add pm_runtime_get/put()
>>>> calls in
>>>> arm_smmu_unmap().
>>>>
>>>> The client device can use something like - pm_runtime_get_supplier()
>>>> since
>>>> we already have the device link in place with this patch series. This
>>>> should
>>>> power-on the supplier (which is smmu) without turning on the consumer
>>>> (such as GPU).
>>>>
>>>> pm_runtime_get_supplier() however is not exported at this moment.
>>>> Will it be useful to export this API and use it in the drivers.
>>>>
>>> I'm not sure pm_runtime_get_supplier() is correct either. That
>>> feels like we're relying on the GPU driver knowing the internal
>>> details of how the device links are configured.
>>>
>> what does pm_runtime_get_supplier() do if IOMMU driver hasn't setup
>> device-link?
>
>
> It will be a no-op.
>
>> If it is a no-op, then I guess the GPU driver calling
>> pm_runtime_get_supplier() seems reasonable, and less annoying than
>> having special cases in pm_resume path.. I don't feel too bad about
>> having "just in case" get/put_supplier() calls in the unmap path.
>>
>> Also, presumably we still want to avoid powering up GPU even if we
>> short circuit the firmware loading and rest of "booting up the GPU"..
>> since presumably the GPU draws somewhat more power than the IOMMU..
>> having the pm_resume/suspend path know about the diff between waking
>> up / suspending the iommu and itself doesn't really feel less-bad than
>> just doing "just in case" get/put_supplier() calls.
>
>
> If it sounds okay, then i can send a patch that exports the
> pm_runtime_get/put_suppliers() APIs.
>
sounds good to me
BR,
-R
>
> Best regards
> Vivek
>
>> BR,
>> -R
>>
>>> Is there some way to have the GPU driver know in its runtime PM
>>> resume hook that it doesn't need to be powered on because it
>>> isn't actively drawing anything or processing commands? I'm
>>> thinking of the code calling pm_runtime_get() as proposed around
>>> the IOMMU unmap path in the GPU driver and then having the
>>> runtime PM resume hook in the GPU driver return some special
>>> value to indicate that it didn't really resume because it didn't
>>> need to and to treat the device as runtime suspended but not
>>> return an error. Then the runtime PM core can keep track of that
>>> and try to power the GPU on again when another pm_runtime_get()
>>> is called on the GPU device.
>>>
>>> This keeps the consumer API the same, always pm_runtime_get(),
>>> but leaves the device driver logic of what to do when the GPU
>>> doesn't need to power on to the runtime PM hook where the driver
>>> has all the information.
>>>
>>> --
>>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
>>> a Linux Foundation Collaborative Project
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm"
>> in
>> the body of a message tomajordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> More majordomo info athttp://vger.kernel.org/majordomo-info.html
>
>
> --
> The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
>
> a Linux Foundation Collaborative Project
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [patches] Re: [PATCH] dt-bindings: Add a RISC-V SBI firmware node
From: Rob Herring @ 2017-11-28 19:43 UTC (permalink / raw)
To: Jonathan Neuschäfer
Cc: Palmer Dabbelt, Mark Rutland, devicetree@vger.kernel.org, patches,
linux-kernel@vger.kernel.org
In-Reply-To: <20171121200832.d55khzcufo2owcff@latitude>
On Tue, Nov 21, 2017 at 2:08 PM, Jonathan Neuschäfer <j.neuschaefer@gmx.net> wrote:
> On Tue, Nov 21, 2017 at 09:37:02AM -0800, Palmer Dabbelt wrote:
> [...]
>> This isn't really a big deal to me, as I'm only interested in RISC-V
>> systems, but there's been some pushback on the concept of an SBI so it
>> seemed like a simple way to allow people to build non-SBI (and there for not
>> really RISC-V) systems.
>
> For those reading along: I suggested the /firmware/sbi node to Palmer,
> because I'm interested in such "not really RISC-V" systems, (because it
> makes the firmware's job easier to not implement the SBI — speaking with
> my coreboot hat, here.)
Seems some property to signify "not really RISC-V" would be better than adding a node for all conformant systems. If this is just one
>
>> One option that wouldn't require a device tree node
>> would be to have Linux boot in machine mode [...] and then provide its
>> own SBI implementation.
>
> I think this can work.
>
>
> Thanks,
> Jonathan Neuschäfer
^ permalink raw reply
* Re: [patches] Re: [PATCH] dt-bindings: Add an enable method to RISC-V
From: Rob Herring @ 2017-11-28 19:41 UTC (permalink / raw)
To: Palmer Dabbelt
Cc: Mark Rutland, devicetree@vger.kernel.org, patches,
linux-kernel@vger.kernel.org
In-Reply-To: <mhng-53fb904e-d47e-4887-b661-560a8dd25313@palmer-si-x1c4>
On Wed, Nov 22, 2017 at 11:11 AM, Palmer Dabbelt <palmer@sifive.com> wrote:
> On Tue, 21 Nov 2017 03:04:52 PST (-0800), mark.rutland@arm.com wrote:
>>
>> Hi Palmer,
>>
>> On Mon, Nov 20, 2017 at 11:50:22AM -0800, Palmer Dabbelt wrote:
>>>
>>> RISC-V doesn't currently specify a mechanism for enabling or disabling
>>> CPUs. Instead, we assume that all CPUs are enabled on boot, and if
>>> someone wants to save power we instead put a CPU to sleep via a WFI
>>> loop.
>>>
>>> This patch adds "enable-method" to the RISC-V CPU binding, which
>>> currently only has the value "none". This allows us to change the
>>> enable method in the future.
>>
>>
>> I think you might want to be a bit more explicit about what this means,
>> and this could do with a better name, as "none" sounds like the CPU is
>> unusable, rather than it having been placed within the kernel already by
>> the FW/bootloader (which IIUC is what happens currently).
>
>
> It was proposed to make "enable-method" optional, and have the lack of an
> enable method signify the current scheme. The current scheme is that the
> bootloader starts every hart at the kernel's entry point.
>
> Calling this "always-enabled" was also suggested, which seems fine to me.
>
>> As previosuly commented, I also really think you'll want to define a
>> simple boot protocol (like PPC spin-table) whereby the kernel can bring
>> each CPU into the kernel independently. That will save you a lot of pain
>> in future with things like kexec, suspend/resume, etc.
>>
>> For arm64 we had a spin-table clone (implemented in our boot-wrapper
>> firmware) that allowed us to bring CPUs into the kernel explicitly.
>> However, we made the mistake of allowing CPUs to share a mailbox, and we
>> couldn't tell how many CPUs were stuck in the kernel at any point in
>> time (rendering kexec, suspend, etc impossible).
>
>
> This is actually why I'm kind of pushing back on this: because we don't know
> how we're actually going to handle this, I don't want to go build an
> interface to the firmware that might be broken. Essentially what we're
> doing now is just keeping the spin table entirely within Linux, so we can
> change this interface whenever we want. The start of our kernel looks like
>
> _start(char *dtb_pointer, long hartid)
> if (atomic_increment_return(hart_lottery) == 0)
> start_kernel()
> else
> while (READ_ONCE(__cpu_up_has_turned_on_hart[hartid]) == 0)
> wait_for_interrupt()
> smp_callin()
>
> If I understand correctly, this is essentially what the spin tables are
> doing in arm64. Our mechanism is a bit different because we can expose a
> much more complicated interface here, but since the interface can change
> (it's a kernel-internal interface, not a firmware->kernel interface) that's
> the natural thing to do.
>
> While I haven't actually gone through and looked at any of this (and I admit
> I have only a vague idea of how it works), I think this should work fine for
> kexec, CPU hotplug, and suspend. kexec is easy: the fresh kernel's image
> will boot exactly like a regular one, as all the harts can just jump to the
> entry point at the same time. Since "hart_lottery" is initialized to 0 by
> the ELF there isn't anything special required to make it work.
>
> Actually turning off harts will require us to add an interface that does so,
> which will probably happen via an SBI call. We haven't actually designed
> the interface yet, but I'm assuming it'll just reset the hart. In general,
> we like to make any interface that sleeps also work as a NOP, so for now
> let's just pretend that this interface does nothing and go straight
> to_start. This should map pretty well, our __cpu_down could just be the
> mirror of __cpu_up
>
> __cpu_down(int hartid)
> __cpu_up_has_turned_on_hart[hartid] = false;
> atomic_decrement(hart_lottery);
> __sbi_suspend_hart();
> jump _start
>
> That should cover hotplug, and then suspend is just a matter of hotplugging
> out the last CPU. I assume that lots of our stuff will blow up when we
> start removing harts at runtime, but that'll all happen regardless of how we
> wake them up. There's also a bit of a race here (bringing up a hart while
> the last one is suspending), and that counter overflows, but those seem
> solvable.
>
> Does that sound sane? If not, I'd be happy to go and design a spin table
> firmware interface. We just like to avoid inventing external interfaces
> until we really know what we're doing :).
>
>
>> Thanks,
>> Mark.
>>
>>> CC: Mark Rutland <mark.rutland@arm.com>
>>> Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
>>> ---
>>> Documentation/devicetree/bindings/riscv/cpus.txt | 7 +++++++
>>> 1 file changed, 7 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/riscv/cpus.txt
>>> b/Documentation/devicetree/bindings/riscv/cpus.txt
>>> index adf7b7af5dc3..dd9e1ae197e2 100644
>>> --- a/Documentation/devicetree/bindings/riscv/cpus.txt
>>> +++ b/Documentation/devicetree/bindings/riscv/cpus.txt
>>> @@ -82,6 +82,11 @@ described below.
>>> Value type: <string>
>>> Definition: Contains the RISC-V ISA string of this hart.
>>> These
>>> ISA strings are defined by the RISC-V ISA
>>> manual.
>>> + - cpu-enable-method:
>>> + Usage: required
>>> + Value type: <stringlist>
>>> + Definition: Must be one of
>>> + "none": This CPU's state cannot be changed.
>>>
>>> Example: SiFive Freedom U540G Development Kit
>>> ---------------------------------------------
>>> @@ -105,6 +110,7 @@ Linux is allowed to run on.
>>> reg = <0>;
>>> riscv,isa = "rv64imac";
>>> status = "disabled";
>>> + enable-method = "none";
>>> L10: interrupt-controller {
>>> #interrupt-cells = <1>;
>>> compatible = "riscv,cpu-intc";
>>> @@ -130,6 +136,7 @@ Linux is allowed to run on.
>>> reg = <1>;
>>> riscv,isa = "rv64imafdc";
>>> status = "okay";
>>> + enable-method = "none";
>>> tlb-split;
>>> L13: interrupt-controller {
>>> #interrupt-cells = <1>;
>>> --
>>> 2.13.6
>>>
>
^ permalink raw reply
* Re: [PATCH 3/6] arm: dts: marvell: Add missing #phy-cells to usb-nop-xceiv
From: Rob Herring @ 2017-11-28 19:38 UTC (permalink / raw)
To: Gregory CLEMENT
Cc: Arnd Bergmann, Andrew Lunn,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
arm-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, Jason Cooper,
Sebastian Hesselbarth
In-Reply-To: <87po8az1uc.fsf-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
On Wed, Nov 22, 2017 at 10:59 AM, Gregory CLEMENT
<gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> Hi Arnd,
>
> On mar., nov. 21 2017, Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote:
>
>> On Tue, Nov 21, 2017 at 9:09 PM, Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org> wrote:
>>> On Tue, Nov 21, 2017 at 12:29:48PM -0600, Rob Herring wrote:
>>>> On Thu, Nov 9, 2017 at 4:26 PM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>>>> > "usb-nop-xceiv" is using the phy binding, but is missing #phy-cells
>>>> > property. This is probably because the binding was the precursor to the phy
>>>> > binding.
>>>> >
>>>> > Fixes the following warning in Marvell dts files:
>>>> >
>>>> > Warning (phys_property): Missing property '#phy-cells' in node ...
>>>> >
>>>> > Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>>>> > Cc: Jason Cooper <jason-NLaQJdtUoK4Be96aLqz0jA@public.gmane.org>
>>>> > Cc: Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org>
>>>> > Cc: Gregory Clement <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>>> > Cc: Sebastian Hesselbarth <sebastian.hesselbarth-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>> > ---
>>>> > Please apply to Marvell tree.
>>>>
>>>> Ping.
>>>>
>>>> Arnd, can you apply if you'd like the warnings fixed and Marvell
>>>> maintainers don't respond.
>>>
>>> Hi Rob
>>>
>>> Patches submitted a week before the merge window opens are generally
>>> deferred to the next cycle. I expect that once -rc1 is out, Gregory
>>> will pick up this patch.
>>
>> These are real bugfixes, I want them in 4.15, since we now get a
>> loud warning for them. I'd rather not shut up that warning because
>> it's not a false-positive.
>
> As pointed by Andrew, I planned to take them once the 4.15-rc1 would be
> released. It was not obvious for me that it was real fixes. Actually I
> would have expected to have first the dts fixed in a release and then
> the warning enabled.
I didn't plan to fix everyone's warnings for them. Unfortunately, the
combination of linux-next being infrequent in Oct and kernelci.org
failing to report dtc warnings caused all the warnings started to get
noticed late, so I fixed the noisier cases. The only way anyone pays
attention is when dtc is updated and warnings appear. I've emailed out
warnings in advance before. I've turned off the more subjective ones
by default. Neither seems to get widely noticed (now in reviews
instead of saying "don't do X", I say "don't do X and building with
W=2 will tell you this"). More dtc checks are coming, so I'm open to
suggestions on the process.
Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v1] usb: xhci: allow imod-interval to be configurable
From: Greg Kroah-Hartman @ 2017-11-28 19:35 UTC (permalink / raw)
To: Adam Wallis
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Rob Herring,
Mathias Nyman, linux-usb-u79uwXL29TY76Z2rM5mHXA, Mark Rutland,
Matthias Brugger, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
chunfeng.yun-NuS5LvNUpcJWk0Htik3J/w, timur-sgV2jX0FEOL9JmXXK+q4OQ
In-Reply-To: <1511889106-9239-1-git-send-email-awallis-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
On Tue, Nov 28, 2017 at 12:11:46PM -0500, Adam Wallis wrote:
> The xHCI driver currently has the IMOD set to 160, which
> translates to an IMOD interval of 40,000ns (160 * 250)ns
>
> Commit 0cbd4b34cda9 ("xhci: mediatek: support MTK xHCI host controller")
> introduced a QUIRK for the MTK platform to adjust this interval to 20,
> which translates to an IMOD interval of 5,000ns (20 * 250)ns. This is
> due to the fact that the MTK controller IMOD interval is 8 times
> as much as defined in xHCI spec.
>
> Instead of adding more quirk bits for additional platforms, this patch
> introduces the ability for vendors to set the IMOD_INTERVAL as is
> optimal for their platform. By using device_property_read_u32() on
> "imod-interval", the IMOD INTERVAL can be specified in nano seconds. If
> no interval is specified, the default of 40,000ns (IMOD=160) will be
> used.
>
> No bounds checking has been implemented due to the fact that a vendor
> may have violated the spec and would need to specify a value outside of
> the max 8,000 IRQs/second limit specified in the xHCI spec.
>
> Backwards compatibility is maintained for MTK_HOSTS through the quirk
> bit, however, imod_interval should be pushed into device tree at a
> future point and this quirk should be removed from xhci_plat_probe
>
> Signed-off-by: Adam Wallis <awallis-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> ---
> Documentation/devicetree/bindings/usb/usb-xhci.txt | 1 +
> drivers/usb/host/xhci-plat.c | 15 +++++++++++++++
> drivers/usb/host/xhci.c | 7 ++-----
> drivers/usb/host/xhci.h | 2 ++
> 4 files changed, 20 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt b/Documentation/devicetree/bindings/usb/usb-xhci.txt
> index ae6e484..3998459 100644
> --- a/Documentation/devicetree/bindings/usb/usb-xhci.txt
> +++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt
> @@ -29,6 +29,7 @@ Optional properties:
> - usb2-lpm-disable: indicate if we don't want to enable USB2 HW LPM
> - usb3-lpm-capable: determines if platform is USB3 LPM capable
> - quirk-broken-port-ped: set if the controller has broken port disable mechanism
> + - imod-interval: IMOD_INTERVAL in nano-seconds. Default is 40000
>
> Example:
> usb@f0931000 {
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index 09f164f..f7730c8 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -23,6 +23,7 @@
> #include "xhci-plat.h"
> #include "xhci-mvebu.h"
> #include "xhci-rcar.h"
> +#include "xhci-mtk.h"
>
> static struct hc_driver __read_mostly xhci_plat_hc_driver;
>
> @@ -269,6 +270,20 @@ static int xhci_plat_probe(struct platform_device *pdev)
> if (device_property_read_bool(&pdev->dev, "quirk-broken-port-ped"))
> xhci->quirks |= XHCI_BROKEN_PORT_PED;
>
> + /* imod interval in nanoseconds */
> + if (device_property_read_u32(sysdev,
> + "imod-interval", &xhci->imod_interval))
> + xhci->imod_interval = 40000;
So no matter what value you read, you set it to 40000? Or am I reading
this code incorrectly?
There's a good reason putting function calls inside if() is considered a
bad coding style :)
thanks,
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RFC 1/2] of: overlay: add whitelist
From: Alan Tull @ 2017-11-28 19:26 UTC (permalink / raw)
To: Rob Herring
Cc: Frank Rowand, Pantelis Antoniou, Moritz Fischer,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel,
linux-fpga-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20171128151538.2w6fjcib6my6wt5n@rob-hp-laptop>
On Tue, Nov 28, 2017 at 9:15 AM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Mon, Nov 27, 2017 at 02:58:03PM -0600, Alan Tull wrote:
>> Add simple whitelist. When an overlay is submitted, if any target in
>> the overlay is not in the whitelist, the overlay is rejected. Drivers
>> that support dynamic configuration can register their device node with:
>>
>> int of_add_whitelist_node(struct device_node *np)
>>
>> and remove themselves with:
>>
>> void of_remove_whitelist_node(struct device_node *np)
>
> I think these should be named for what they do, not how it is
> implemented.
Sure, such as of_node_overlay_enable and of_node_overlay_disable?
>
>>
>> Signed-off-by: Alan Tull <atull-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>> ---
>> drivers/of/overlay.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>> include/linux/of.h | 12 +++++++++
>> 2 files changed, 85 insertions(+)
>>
>> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
>> index c150abb..5f952a1 100644
>> --- a/drivers/of/overlay.c
>> +++ b/drivers/of/overlay.c
>> @@ -21,6 +21,7 @@
>> #include <linux/slab.h>
>> #include <linux/err.h>
>> #include <linux/idr.h>
>> +#include <linux/spinlock.h>
>>
>> #include "of_private.h"
>>
>> @@ -646,6 +647,74 @@ static void free_overlay_changeset(struct overlay_changeset *ovcs)
>> kfree(ovcs);
>> }
>>
>> +/* lock for adding/removing device nodes to the whitelist */
>> +static spinlock_t whitelist_lock;
>> +
>> +static struct list_head whitelist_list = LIST_HEAD_INIT(whitelist_list);
>> +
>> +struct dt_overlay_whitelist {
>> + struct device_node *np;
>> + struct list_head node;
>> +};
>
> Can't we just add a flags bit in device_node.flags? That would be much
> simpler.
Yes, much simpler. Such as:
#define OF_OVERLAY_ENABLED 5 /* allow DT overlay targeting this node */
>
>> +
>> +int of_add_whitelist_node(struct device_node *np)
>> +{
>> + unsigned long flags;
>> + struct dt_overlay_whitelist *wln;
>> +
>> + wln = kzalloc(sizeof(*wln), GFP_KERNEL);
>> + if (!wln)
>> + return -ENOMEM;
>> +
>> + wln->np = np;
>> +
>> + spin_lock_irqsave(&whitelist_lock, flags);
>> + list_add(&wln->node, &whitelist_list);
>> + spin_unlock_irqrestore(&whitelist_lock, flags);
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(of_add_whitelist_node);
>> +
>> +void of_remove_whitelist_node(struct device_node *np)
>> +{
>> + struct dt_overlay_whitelist *wln;
>> + unsigned long flags;
>> +
>> + list_for_each_entry(wln, &whitelist_list, node) {
>> + if (np == wln->np) {
>> + spin_lock_irqsave(&whitelist_lock, flags);
>> + list_del(&wln->node);
>> + spin_unlock_irqrestore(&whitelist_lock, flags);
>> + kfree(wln);
>> + return;
>> + }
>> + }
>> +}
>> +EXPORT_SYMBOL_GPL(of_remove_whitelist_node);
>> +
>> +static int of_check_whitelist(struct overlay_changeset *ovcs)
>> +{
>> + struct dt_overlay_whitelist *wln;
>> + struct device_node *target;
>> + int i;
>> +
>> + for (i = 0; i < ovcs->count; i++) {
>> + target = ovcs->fragments[i].target;
>> + if (!of_node_cmp(target->name, "__symbols__"))
>> + continue;
>> +
>> + list_for_each_entry(wln, &whitelist_list, node)
>> + if (target == wln->np)
>> + break;
>> +
>> + if (target != wln->np)
>> + return -ENODEV;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> /**
>> * of_overlay_apply() - Create and apply an overlay changeset
>> * @tree: Expanded overlay device tree
>> @@ -717,6 +786,10 @@ int of_overlay_apply(struct device_node *tree, int *ovcs_id)
>> if (ret)
>> goto err_free_overlay_changeset;
>>
>> + ret = of_check_whitelist(ovcs);
>> + if (ret)
>> + goto err_free_overlay_changeset;
>
> This will break you until the next patch and breaks any other users. I
> think this is now just the unittest as tilcdc overlay is getting
> removed.
>
> You have to make this chunk the last patch in the series.
I'd rather squash the two patches. In either case, the contents of
second patch are dependent on stuff in char-misc-testing today, so it
won't be able to apply yet on linux-next or anywhere else.
Thanks
Alan
>
> Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 08/13] dt-bindings: mips: Add bindings for Microsemi SoCs
From: Florian Fainelli @ 2017-11-28 19:14 UTC (permalink / raw)
To: Alexandre Belloni, Ralf Baechle
Cc: linux-mips-6z/3iImG2C8G8FEW9MqTrA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20171128152643.20463-9-alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
On 11/28/2017 07:26 AM, Alexandre Belloni wrote:
> Add bindings for Microsemi SoCs. Currently only Ocelot is supported.
>
> Signed-off-by: Alexandre Belloni <alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> ---
> Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>
> Documentation/devicetree/bindings/mips/mscc.txt | 6 ++++++
> 1 file changed, 6 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mips/mscc.txt
>
> diff --git a/Documentation/devicetree/bindings/mips/mscc.txt b/Documentation/devicetree/bindings/mips/mscc.txt
> new file mode 100644
> index 000000000000..2c52e76b7142
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mips/mscc.txt
> @@ -0,0 +1,6 @@
> +* Microsemi MIPS CPUs
> +
> +Required properties:
> +- compatible: "brcm,ocelot"
You probably intended to use mscc,ocelot here, right?
--
Florian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ 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