Re: [PATCHi v2] dt-bindings: add device tree binding for Arm TrustZone CryptoCell crypto engine

[Herbert Xu]

On Thu, Nov 16, 2017 at 06:15:28AM +, Gilad Ben-Yossef wrote:
> The Arm TrustZone CryptoCell is a hardware security engine. This patch
> adds DT bindings for its Rich Execution Environment crypto engine.
> 
> A driver supporting this device is already present in the staging tree.
> 
> Signed-off-by: Gilad Ben-Yossef 
> Acked-by: Rob Herring 

Patch applied.  Thanks.
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v4 1/8] dt-bindings: Add Cavium Octeon Common Ethernet Interface.

[David Daney]

On 11/28/2017 06:01 PM, Andrew Lunn wrote:

On Tue, Nov 28, 2017 at 04:55:33PM -0800, David Daney wrote:

From: Carlos Munoz 

Add bindings for Common Ethernet Interface (BGX) block.

Acked-by: Rob Herring 
Signed-off-by: Carlos Munoz 
Signed-off-by: Steven J. Hill 
Signed-off-by: David Daney 
---
  .../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 ..830c5f08
--- /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@11800e000 {
+   compatible = "cavium,octeon-7890-bgx";
+   reg = <0x00011800 0xe000 0x 0x0100>;


Hi David

In the probe function we have:

+   reg = of_get_property(pdev->dev.of_node, "reg", NULL);
+   addr = of_translate_address(pdev->dev.of_node, reg);
+   interface = (addr >> 24) & 0xf;
+   numa_node = (addr >> 36) & 0x7;

Is this documented somewhere?


In the Hardware Reference Manual for the chips.



The numa_node is particularly
interesting. MMIO changes depends on what node this is in the cluster?


Yes.  Bits 38..36 of MMIO addresses indicate which NUMA node the 
corresponding hardware device resides on.  Some operations can only be 
performed by a CPU on the same NUMA node as the device (RX or 
TX-complete interrupts for example)  Other operations can cross CPU 
nodes (TX packet submission for example)





Andrew



___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v4 1/8] dt-bindings: Add Cavium Octeon Common Ethernet Interface.

[Andrew Lunn]

On Tue, Nov 28, 2017 at 04:55:33PM -0800, David Daney wrote:
> From: Carlos Munoz 
> 
> Add bindings for Common Ethernet Interface (BGX) block.
> 
> Acked-by: Rob Herring 
> Signed-off-by: Carlos Munoz 
> Signed-off-by: Steven J. Hill 
> Signed-off-by: David Daney 
> ---
>  .../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 ..830c5f08
> --- /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@11800e000 {
> + compatible = "cavium,octeon-7890-bgx";
> + reg = <0x00011800 0xe000 0x 0x0100>;

Hi David

In the probe function we have:

+   reg = of_get_property(pdev->dev.of_node, "reg", NULL);
+   addr = of_translate_address(pdev->dev.of_node, reg);
+   interface = (addr >> 24) & 0xf;
+   numa_node = (addr >> 36) & 0x7;

Is this documented somewhere? The numa_node is particularly
interesting. MMIO changes depends on what node this is in the cluster?

Andrew
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v4 8/8] MAINTAINERS: Add entry for drivers/net/ethernet/cavium/octeon/octeon3-*

[David Daney]

Signed-off-by: David Daney 
---
 MAINTAINERS | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index aa71ab52fd76..e9239ff3cf05 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3249,6 +3249,12 @@ W:   http://www.cavium.com
 S: Supported
 F: drivers/mmc/host/cavium*
 
+CAVIUM OCTEON-III NETWORK DRIVER
+M: David Daney 
+L: net...@vger.kernel.org
+S: Supported
+F: drivers/net/ethernet/cavium/octeon/octeon3-*
+
 CAVIUM OCTEON-TX CRYPTO DRIVER
 M: George Cherian 
 L: linux-cry...@vger.kernel.org
-- 
2.14.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v4 1/8] dt-bindings: Add Cavium Octeon Common Ethernet Interface.

[David Daney]

From: Carlos Munoz 

Add bindings for Common Ethernet Interface (BGX) block.

Acked-by: Rob Herring 
Signed-off-by: Carlos Munoz 
Signed-off-by: Steven J. Hill 
Signed-off-by: David Daney 
---
 .../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 ..830c5f08
--- /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@11800e000 {
+   compatible = "cavium,octeon-7890-bgx";
+   reg = <0x00011800 0xe000 0x 0x0100>;
+   #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 = <>;
+   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 = <>;
+   phy-mode = "sgmii"
+   };
+   };
-- 
2.14.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v4 4/8] MIPS: Octeon: Add Free Pointer Unit (FPA) support.

[David Daney]

From: Carlos Munoz 

>From the hardware user manual: "The FPA is a unit that maintains
pools of pointers to free L2/DRAM memory. To provide QoS, the pools
are referenced indirectly through 1024 auras. Both core software
and hardware units allocate and free pointers."

Signed-off-by: Carlos Munoz 
Signed-off-by: Steven J. Hill 
Signed-off-by: David Daney 
---
 arch/mips/cavium-octeon/Kconfig   |   8 +
 arch/mips/cavium-octeon/Makefile  |   1 +
 arch/mips/cavium-octeon/octeon-fpa3.c | 364 ++
 arch/mips/include/asm/octeon/octeon.h |  15 ++
 4 files changed, 388 insertions(+)
 create mode 100644 arch/mips/cavium-octeon/octeon-fpa3.c

diff --git a/arch/mips/cavium-octeon/Kconfig b/arch/mips/cavium-octeon/Kconfig
index 204a1670fd9b..ce469f982134 100644
--- a/arch/mips/cavium-octeon/Kconfig
+++ b/arch/mips/cavium-octeon/Kconfig
@@ -87,4 +87,12 @@ config OCTEON_ILM
  To compile this driver as a module, choose M here.  The module
  will be called octeon-ilm
 
+config OCTEON_FPA3
+   tristate "Octeon III fpa driver"
+   help
+ This option enables a Octeon III driver for the Free Pool Unit (FPA).
+ The FPA is a hardware unit that manages pools of pointers to free
+ L2/DRAM memory. This driver provides an interface to reserve,
+ initialize, and fill fpa pools.
+
 endif # CAVIUM_OCTEON_SOC
diff --git a/arch/mips/cavium-octeon/Makefile b/arch/mips/cavium-octeon/Makefile
index 0a299ab8719f..0ef967399702 100644
--- a/arch/mips/cavium-octeon/Makefile
+++ b/arch/mips/cavium-octeon/Makefile
@@ -20,3 +20,4 @@ obj-$(CONFIG_MTD)   += flash_setup.o
 obj-$(CONFIG_SMP)+= smp.o
 obj-$(CONFIG_OCTEON_ILM) += oct_ilm.o
 obj-$(CONFIG_USB)+= octeon-usb.o
+obj-$(CONFIG_OCTEON_FPA3)+= octeon-fpa3.o
diff --git a/arch/mips/cavium-octeon/octeon-fpa3.c 
b/arch/mips/cavium-octeon/octeon-fpa3.c
new file mode 100644
index ..a7c7decdbc9a
--- /dev/null
+++ b/arch/mips/cavium-octeon/octeon-fpa3.c
@@ -0,0 +1,364 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for the Octeon III Free Pool Unit (fpa).
+ *
+ * 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) 2015-2017 Cavium, Inc.
+ */
+
+#include 
+
+#include 
+
+
+/* Registers are accessed via xkphys */
+#define SET_XKPHYS (1ull << 63)
+#define NODE_OFFSET0x10ull
+#define SET_NODE(node) ((node) * NODE_OFFSET)
+
+#define FPA_BASE   0x12800ull
+#define SET_FPA_BASE(node) (SET_XKPHYS + SET_NODE(node) + FPA_BASE)
+
+#define FPA_GEN_CFG(n) (SET_FPA_BASE(n)   + 0x0050)
+
+#define FPA_POOLX_CFG(n, p)(SET_FPA_BASE(n) + (p<<3)  + 0x1000)
+#define FPA_POOLX_START_ADDR(n, p) (SET_FPA_BASE(n) + (p<<3)  + 0x1050)
+#define FPA_POOLX_END_ADDR(n, p)   (SET_FPA_BASE(n) + (p<<3)  + 0x1060)
+#define FPA_POOLX_STACK_BASE(n, p) (SET_FPA_BASE(n) + (p<<3)  + 0x1070)
+#define FPA_POOLX_STACK_END(n, p)  (SET_FPA_BASE(n) + (p<<3)  + 0x1080)
+#define FPA_POOLX_STACK_ADDR(n, p) (SET_FPA_BASE(n) + (p<<3)  + 0x1090)
+
+#define FPA_AURAX_POOL(n, a)   (SET_FPA_BASE(n) + (a<<3)  + 0x2000)
+#define FPA_AURAX_CFG(n, a)(SET_FPA_BASE(n) + (a<<3)  + 0x2010)
+#define FPA_AURAX_CNT(n, a)(SET_FPA_BASE(n) + (a<<3)  + 0x2020)
+#define FPA_AURAX_CNT_LIMIT(n, a)  (SET_FPA_BASE(n) + (a<<3)  + 0x2040)
+#define FPA_AURAX_CNT_THRESHOLD(n, a)  (SET_FPA_BASE(n) + (a<<3)  + 0x2050)
+#define FPA_AURAX_POOL_LEVELS(n, a)(SET_FPA_BASE(n) + (a<<3)  + 0x2070)
+#define FPA_AURAX_CNT_LEVELS(n, a) (SET_FPA_BASE(n) + (a<<3)  + 0x2080)
+
+static inline u64 oct_csr_read(u64 addr)
+{
+   return __raw_readq((void __iomem *)addr);
+}
+
+static inline void oct_csr_write(u64 data, u64 addr)
+{
+   __raw_writeq(data, (void __iomem *)addr);
+}
+
+static DEFINE_MUTEX(octeon_fpa3_lock);
+
+static int get_num_pools(void)
+{
+   if (OCTEON_IS_MODEL(OCTEON_CN78XX))
+   return 64;
+   if (OCTEON_IS_MODEL(OCTEON_CNF75XX) || OCTEON_IS_MODEL(OCTEON_CN73XX))
+   return 32;
+   return 0;
+}
+
+static int get_num_auras(void)
+{
+   if (OCTEON_IS_MODEL(OCTEON_CN78XX))
+   return 1024;
+   if (OCTEON_IS_MODEL(OCTEON_CNF75XX) || OCTEON_IS_MODEL(OCTEON_CN73XX))
+   return 512;
+   return 0;
+}
+
+/**
+ * octeon_fpa3_init - Initialize the fpa to default values.
+ * @node: Node of fpa to initialize.
+ *
+ * Returns 0 if successful.
+ * Returns <0 for error codes.
+ */
+int octeon_fpa3_init(int node)
+{
+   static bool init_done[2];

[PATCH v4 2/8] MIPS: Octeon: Enable LMTDMA/LMTST operations.

[David Daney]

From: Carlos Munoz 

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 
Signed-off-by: Steven J. Hill 
Signed-off-by: David Daney 
---
 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

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v4 3/8] MIPS: Octeon: Add a global resource manager.

[David Daney]

From: Carlos Munoz 

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 
Signed-off-by: Steven J. Hill 
Signed-off-by: David Daney 
---
 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 ..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 
+
+#include 
+#include 
+
+#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)_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] "="(tmp) :
+   [addr] "r"(lock) :
+   "memory");
+}
+
+static void res_mgr_unlock(void)
+{
+   u64 lock = (u64)_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 = _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.
+*/
+ 

[PATCH v4 5/8] MIPS: Octeon: Automatically provision CVMSEG space.

[David Daney]

Remove CONFIG_CAVIUM_OCTEON_CVMSEG_SIZE and automatically calculate
the amount of CVMSEG space needed.

1st 128-bytes: Use by IOBDMA
2nd 128-bytes: Reserved by kernel for scratch/TLS emulation.
3rd 128-bytes: OCTEON-III LMTLINE

New config variable CONFIG_CAVIUM_OCTEON_EXTRA_CVMSEG provisions
additional lines, defaults to zero.

Signed-off-by: David Daney 
Signed-off-by: Carlos Munoz 
---
 arch/mips/cavium-octeon/Kconfig| 27 
 arch/mips/cavium-octeon/setup.c| 16 ++--
 .../asm/mach-cavium-octeon/kernel-entry-init.h | 20 +--
 arch/mips/include/asm/mipsregs.h   |  2 ++
 arch/mips/include/asm/octeon/octeon.h  |  2 ++
 arch/mips/include/asm/processor.h  |  2 +-
 arch/mips/kernel/octeon_switch.S   |  2 --
 arch/mips/mm/tlbex.c   | 29 ++
 drivers/staging/octeon/ethernet-defines.h  |  2 +-
 9 files changed, 50 insertions(+), 52 deletions(-)

diff --git a/arch/mips/cavium-octeon/Kconfig b/arch/mips/cavium-octeon/Kconfig
index ce469f982134..29c4d81364a6 100644
--- a/arch/mips/cavium-octeon/Kconfig
+++ b/arch/mips/cavium-octeon/Kconfig
@@ -11,21 +11,26 @@ config CAVIUM_CN63XXP1
  non-CN63XXP1 hardware, so it is recommended to select "n"
  unless it is known the workarounds are needed.
 
-config CAVIUM_OCTEON_CVMSEG_SIZE
-   int "Number of L1 cache lines reserved for CVMSEG memory"
-   range 0 54
-   default 1
-   help
- CVMSEG LM is a segment that accesses portions of the dcache as a
- local memory; the larger CVMSEG is, the smaller the cache is.
- This selects the size of CVMSEG LM, which is in cache blocks. The
- legally range is from zero to 54 cache blocks (i.e. CVMSEG LM is
- between zero and 6192 bytes).
-
 endif # CPU_CAVIUM_OCTEON
 
 if CAVIUM_OCTEON_SOC
 
+config CAVIUM_OCTEON_EXTRA_CVMSEG
+   int "Number of extra L1 cache lines reserved for CVMSEG memory"
+   range 0 50
+   default 0
+   help
+ CVMSEG LM is a segment that accesses portions of the dcache
+ as a local memory; the larger CVMSEG is, the smaller the
+ cache is.  The kernel uses two or three blocks (one for TLB
+ exception handlers, one for driver IOBDMA operations, and on
+ models that need it, one for LMTDMA operations). This
+ selects an optional extra number of CVMSEG lines for use by
+ other software.
+
+ Normally no extra lines are required, and this parameter
+ should be set to zero.
+
 config CAVIUM_OCTEON_LOCK_L2
bool "Lock often used kernel code in the L2"
default "y"
diff --git a/arch/mips/cavium-octeon/setup.c b/arch/mips/cavium-octeon/setup.c
index 99e6a68bc652..51c4d3c3cada 100644
--- a/arch/mips/cavium-octeon/setup.c
+++ b/arch/mips/cavium-octeon/setup.c
@@ -68,6 +68,12 @@ extern void pci_console_init(const char *arg);
 static unsigned long long max_memory = ULLONG_MAX;
 static unsigned long long reserve_low_mem;
 
+/*
+ * modified in hernel-entry-init.h, must have an initial value to keep
+ * it from being clobbered when bss is zeroed.
+ */
+u32 octeon_cvmseg_lines = 2;
+
 DEFINE_SEMAPHORE(octeon_bootbus_sem);
 EXPORT_SYMBOL(octeon_bootbus_sem);
 
@@ -604,11 +610,7 @@ void octeon_user_io_init(void)
 
/* R/W If set, CVMSEG is available for loads/stores in
 * kernel/debug mode. */
-#if CONFIG_CAVIUM_OCTEON_CVMSEG_SIZE > 0
cvmmemctl.s.cvmsegenak = 1;
-#else
-   cvmmemctl.s.cvmsegenak = 0;
-#endif
if (OCTEON_IS_OCTEON3()) {
/* Enable LMTDMA */
cvmmemctl.s.lmtena = 1;
@@ -626,9 +628,9 @@ void octeon_user_io_init(void)
 
/* Setup of CVMSEG is done in kernel-entry-init.h */
if (smp_processor_id() == 0)
-   pr_notice("CVMSEG size: %d cache lines (%d bytes)\n",
- CONFIG_CAVIUM_OCTEON_CVMSEG_SIZE,
- CONFIG_CAVIUM_OCTEON_CVMSEG_SIZE * 128);
+   pr_notice("CVMSEG size: %u cache lines (%u bytes)\n",
+ octeon_cvmseg_lines,
+ octeon_cvmseg_lines * 128);
 
if (octeon_has_feature(OCTEON_FEATURE_FAU)) {
union cvmx_iob_fau_timeout fau_timeout;
diff --git a/arch/mips/include/asm/mach-cavium-octeon/kernel-entry-init.h 
b/arch/mips/include/asm/mach-cavium-octeon/kernel-entry-init.h
index c38b38ce5a3d..cdcca60978a2 100644
--- a/arch/mips/include/asm/mach-cavium-octeon/kernel-entry-init.h
+++ b/arch/mips/include/asm/mach-cavium-octeon/kernel-entry-init.h
@@ -26,11 +26,18 @@
# a3 = address of boot descriptor block
.set push
.set arch=octeon
+   mfc0v1, CP0_PRID_REG
+   andiv1, 0xff00
+   li  v0, 0x9500  # cn78XX or later
+   subuv1, v1, v0
+ 

[PATCH v4 6/8] staging: octeon: Remove USE_ASYNC_IOBDMA macro.

[David Daney]

Previous patch sets USE_ASYNC_IOBDMA to 1 unconditionally.  Remove
USE_ASYNC_IOBDMA from all if statements.  Remove dead code caused by
the change.

Signed-off-by: David Daney 
---
 drivers/staging/octeon/ethernet-defines.h |  6 ---
 drivers/staging/octeon/ethernet-rx.c  | 25 -
 drivers/staging/octeon/ethernet-tx.c  | 85 ++-
 3 files changed, 37 insertions(+), 79 deletions(-)

diff --git a/drivers/staging/octeon/ethernet-defines.h 
b/drivers/staging/octeon/ethernet-defines.h
index e898df25b87f..21438c804a43 100644
--- a/drivers/staging/octeon/ethernet-defines.h
+++ b/drivers/staging/octeon/ethernet-defines.h
@@ -10,10 +10,6 @@
 
 /*
  * A few defines are used to control the operation of this driver:
- *  USE_ASYNC_IOBDMA
- *  Use asynchronous IO access to hardware. This uses Octeon's asynchronous
- *  IOBDMAs to issue IO accesses without stalling. Set this to zero
- *  to disable this. Note that IOBDMAs require CVMSEG.
  *  REUSE_SKBUFFS_WITHOUT_FREE
  *  Allows the TX path to free an skbuff into the FPA hardware pool. This
  *  can significantly improve performance for forwarding and bridging, but
@@ -32,8 +28,6 @@
 #define REUSE_SKBUFFS_WITHOUT_FREE  1
 #endif
 
-#define USE_ASYNC_IOBDMA   1
-
 /* Maximum number of SKBs to try to free per xmit packet. */
 #define MAX_OUT_QUEUE_DEPTH 1000
 
diff --git a/drivers/staging/octeon/ethernet-rx.c 
b/drivers/staging/octeon/ethernet-rx.c
index 1a44291318ee..dd76c99d5ae0 100644
--- a/drivers/staging/octeon/ethernet-rx.c
+++ b/drivers/staging/octeon/ethernet-rx.c
@@ -201,11 +201,9 @@ static int cvm_oct_poll(struct oct_rx_group *rx_group, int 
budget)
/* Prefetch cvm_oct_device since we know we need it soon */
prefetch(cvm_oct_device);
 
-   if (USE_ASYNC_IOBDMA) {
-   /* Save scratch in case userspace is using it */
-   CVMX_SYNCIOBDMA;
-   old_scratch = cvmx_scratch_read64(CVMX_SCR_SCRATCH);
-   }
+   /* Save scratch in case userspace is using it */
+   CVMX_SYNCIOBDMA;
+   old_scratch = cvmx_scratch_read64(CVMX_SCR_SCRATCH);
 
/* Only allow work for our group (and preserve priorities) */
if (OCTEON_IS_MODEL(OCTEON_CN68XX)) {
@@ -220,10 +218,8 @@ static int cvm_oct_poll(struct oct_rx_group *rx_group, int 
budget)
   BIT(rx_group->group));
}
 
-   if (USE_ASYNC_IOBDMA) {
-   cvmx_pow_work_request_async(CVMX_SCR_SCRATCH, CVMX_POW_NO_WAIT);
-   did_work_request = 1;
-   }
+   cvmx_pow_work_request_async(CVMX_SCR_SCRATCH, CVMX_POW_NO_WAIT);
+   did_work_request = 1;
 
while (rx_count < budget) {
struct sk_buff *skb = NULL;
@@ -232,7 +228,7 @@ static int cvm_oct_poll(struct oct_rx_group *rx_group, int 
budget)
cvmx_wqe_t *work;
int port;
 
-   if (USE_ASYNC_IOBDMA && did_work_request)
+   if (did_work_request)
work = cvmx_pow_work_response_async(CVMX_SCR_SCRATCH);
else
work = cvmx_pow_work_request_sync(CVMX_POW_NO_WAIT);
@@ -260,7 +256,7 @@ static int cvm_oct_poll(struct oct_rx_group *rx_group, int 
budget)
sizeof(void *));
prefetch(pskb);
 
-   if (USE_ASYNC_IOBDMA && rx_count < (budget - 1)) {
+   if (rx_count < (budget - 1)) {
cvmx_pow_work_request_async_nocheck(CVMX_SCR_SCRATCH,
CVMX_POW_NO_WAIT);
did_work_request = 1;
@@ -403,10 +399,9 @@ static int cvm_oct_poll(struct oct_rx_group *rx_group, int 
budget)
cvmx_write_csr(CVMX_POW_PP_GRP_MSKX(coreid), old_group_mask);
}
 
-   if (USE_ASYNC_IOBDMA) {
-   /* Restore the scratch area */
-   cvmx_scratch_write64(CVMX_SCR_SCRATCH, old_scratch);
-   }
+   /* Restore the scratch area */
+   cvmx_scratch_write64(CVMX_SCR_SCRATCH, old_scratch);
+
cvm_oct_rx_refill_pool(0);
 
return rx_count;
diff --git a/drivers/staging/octeon/ethernet-tx.c 
b/drivers/staging/octeon/ethernet-tx.c
index 31f35025d19e..2eede0907924 100644
--- a/drivers/staging/octeon/ethernet-tx.c
+++ b/drivers/staging/octeon/ethernet-tx.c
@@ -179,23 +179,18 @@ int cvm_oct_xmit(struct sk_buff *skb, struct net_device 
*dev)
qos = 0;
}
 
-   if (USE_ASYNC_IOBDMA) {
-   /* Save scratch in case userspace is using it */
-   CVMX_SYNCIOBDMA;
-   old_scratch = cvmx_scratch_read64(CVMX_SCR_SCRATCH);
-   old_scratch2 = cvmx_scratch_read64(CVMX_SCR_SCRATCH + 8);
-
-   /*
-* Fetch and increment the number of packets to be
-* freed.
-*/
-   cvmx_fau_async_fetch_and_add32(CVMX_SCR_SCRATCH 

[PATCH v4 0/8] Cavium OCTEON-III network driver.

[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

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 1/3] media: staging: atomisp: fix for sparse "using plain integer as NULL pointer" warnings.

[Dan Carpenter]

On Tue, Nov 28, 2017 at 11:33:37PM +, Jeremy Sowden wrote:
> On 2017-11-28, at 17:15:24 +0300, Dan Carpenter wrote:
> > On Mon, Nov 27, 2017 at 12:44:48PM +, Jeremy Sowden wrote:
> > > The "address" member of struct ia_css_host_data is a
> > > pointer-to-char, so define default as NULL.
> > >
> > > --- 
> > > a/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/isp_param/interface/ia_css_isp_param_types.h
> > > +++ 
> > > b/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/isp_param/interface/ia_css_isp_param_types.h
> > > @@ -95,7 +95,7 @@ union ia_css_all_memory_offsets {
> > >  };
> > >
> > >  #define IA_CSS_DEFAULT_ISP_MEM_PARAMS \
> > > - { { { { 0, 0 } } } }
> > > + { { { { NULL, 0 } } } }
> >
> > This define is way ugly and instead of making superficial changes, you
> > should try to eliminate it.
> >
> > People look at warnings as a bad thing but they are actually a
> > valuable resource which call attention to bad code.  By making this
> > change you're kind of wasting the warning.  The bad code is still
> > there, it's just swept under the rug but like a dead mouse carcass
> > it's still stinking up the living room.  We should leave the warning
> > there until it irritates someone enough to fix it properly.
> 
> Tracking down the offending initializer was definitely a pain.
> 
> Compound literals with designated initializers would make this macro
> (and a number of others) easier to understand and more type-safe:
> 
>#define IA_CSS_DEFAULT_ISP_MEM_PARAMS \
>   -   { { { { 0, 0 } } } }
>   +   (struct ia_css_isp_param_host_segments) { \
>   +   .params = { { \
>   +   (struct ia_css_host_data) { \
>   +   .address = NULL, \
>   +   .size = 0 \
>   +   } \
>   +   } } \
>   +   }

Using designated initializers is good, yes.  Can't we just use an
empty initializer since this is all zeroed memory anyway?

(struct ia_css_isp_param_host_segments) { }

I haven't tried it.

> 
> Unfortunately this default value is one end of a chain of default values


Yeah.  A really long chain...


> used to initialize members of default values of enclosing structs where
> the outermost values are used to initialize some static variables:
> 
>   static enum ia_css_err
>   init_pipe_defaults(enum ia_css_pipe_mode mode,
>struct ia_css_pipe *pipe,
>bool copy_pipe)
>   {
> static struct ia_css_pipe default_pipe = IA_CSS_DEFAULT_PIPE;
> static struct ia_css_preview_settings prev  = 
> IA_CSS_DEFAULT_PREVIEW_SETTINGS;
> static struct ia_css_capture_settings capt  = 
> IA_CSS_DEFAULT_CAPTURE_SETTINGS;
> static struct ia_css_video_settings   video = 
> IA_CSS_DEFAULT_VIDEO_SETTINGS;
> static struct ia_css_yuvpp_settings   yuvpp = 
> IA_CSS_DEFAULT_YUVPP_SETTINGS;
> 
> if (pipe == NULL) {
>   IA_CSS_ERROR("NULL pipe parameter");
>   return IA_CSS_ERR_INVALID_ARGUMENTS;
> }
> 
> /* Initialize pipe to pre-defined defaults */
> *pipe = default_pipe;
> 
> /* TODO: JB should not be needed, but temporary backward reference */
> switch (mode) {
> case IA_CSS_PIPE_MODE_PREVIEW:
>   pipe->mode = IA_CSS_PIPE_ID_PREVIEW;
>   pipe->pipe_settings.preview = prev;
>   break;
> case IA_CSS_PIPE_MODE_CAPTURE:
>   if (copy_pipe) {
>   pipe->mode = IA_CSS_PIPE_ID_COPY;
>   } else {
>   pipe->mode = IA_CSS_PIPE_ID_CAPTURE;
>   }
>   pipe->pipe_settings.capture = capt;
>   break;
> case IA_CSS_PIPE_MODE_VIDEO:
>   pipe->mode = IA_CSS_PIPE_ID_VIDEO;
>   pipe->pipe_settings.video = video;
>   break;
> case IA_CSS_PIPE_MODE_ACC:
>   pipe->mode = IA_CSS_PIPE_ID_ACC;
>   break;
> case IA_CSS_PIPE_MODE_COPY:
>   pipe->mode = IA_CSS_PIPE_ID_CAPTURE;
>   break;
> case IA_CSS_PIPE_MODE_YUVPP:
>   pipe->mode = IA_CSS_PIPE_ID_YUVPP;
>   pipe->pipe_settings.yuvpp = yuvpp;
>   break;
> default:
>   return IA_CSS_ERR_INVALID_ARGUMENTS;
> }
> 
> return IA_CSS_SUCCESS;
>   }
> 
> and GCC's limited support for using compound literals to initialize
> static variables doesn't stretch this far.
> 
> I'm not convinced, however, that those variables actually achieve very
> much.  If I change the code to assign the defaults directly, the problem
> goes away:
> 
>   diff --git a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c 
> b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c
>   index f92b6a9f77eb..671b2c732a46 100644
>   --- a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c
>   +++ b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c
>   @@ -2291,25 +2291,19 @@ init_pipe_defaults(enum ia_css_pipe_mode mode,
>struct ia_css_pipe *pipe,
>bool copy_pipe)
>{
>   -   static struct ia_css_pipe default_pipe = IA_CSS_DEFAULT_PIPE;

Re: [PATCH v2 1/3] media: staging: atomisp: fix for sparse "using plain integer as NULL pointer" warnings.

[Jeremy Sowden]

On 2017-11-28, at 17:15:24 +0300, Dan Carpenter wrote:
> On Mon, Nov 27, 2017 at 12:44:48PM +, Jeremy Sowden wrote:
> > The "address" member of struct ia_css_host_data is a
> > pointer-to-char, so define default as NULL.
> >
> > --- 
> > a/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/isp_param/interface/ia_css_isp_param_types.h
> > +++ 
> > b/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/isp_param/interface/ia_css_isp_param_types.h
> > @@ -95,7 +95,7 @@ union ia_css_all_memory_offsets {
> >  };
> >
> >  #define IA_CSS_DEFAULT_ISP_MEM_PARAMS \
> > -   { { { { 0, 0 } } } }
> > +   { { { { NULL, 0 } } } }
>
> This define is way ugly and instead of making superficial changes, you
> should try to eliminate it.
>
> People look at warnings as a bad thing but they are actually a
> valuable resource which call attention to bad code.  By making this
> change you're kind of wasting the warning.  The bad code is still
> there, it's just swept under the rug but like a dead mouse carcass
> it's still stinking up the living room.  We should leave the warning
> there until it irritates someone enough to fix it properly.

Tracking down the offending initializer was definitely a pain.

Compound literals with designated initializers would make this macro
(and a number of others) easier to understand and more type-safe:

   #define IA_CSS_DEFAULT_ISP_MEM_PARAMS \
  - { { { { 0, 0 } } } }
  + (struct ia_css_isp_param_host_segments) { \
  + .params = { { \
  + (struct ia_css_host_data) { \
  + .address = NULL, \
  + .size = 0 \
  + } \
  + } } \
  + }

Unfortunately this default value is one end of a chain of default values
used to initialize members of default values of enclosing structs where
the outermost values are used to initialize some static variables:

  static enum ia_css_err
  init_pipe_defaults(enum ia_css_pipe_mode mode,
 struct ia_css_pipe *pipe,
 bool copy_pipe)
  {
static struct ia_css_pipe default_pipe = IA_CSS_DEFAULT_PIPE;
static struct ia_css_preview_settings prev  = 
IA_CSS_DEFAULT_PREVIEW_SETTINGS;
static struct ia_css_capture_settings capt  = 
IA_CSS_DEFAULT_CAPTURE_SETTINGS;
static struct ia_css_video_settings   video = IA_CSS_DEFAULT_VIDEO_SETTINGS;
static struct ia_css_yuvpp_settings   yuvpp = IA_CSS_DEFAULT_YUVPP_SETTINGS;

if (pipe == NULL) {
  IA_CSS_ERROR("NULL pipe parameter");
  return IA_CSS_ERR_INVALID_ARGUMENTS;
}

/* Initialize pipe to pre-defined defaults */
*pipe = default_pipe;

/* TODO: JB should not be needed, but temporary backward reference */
switch (mode) {
case IA_CSS_PIPE_MODE_PREVIEW:
  pipe->mode = IA_CSS_PIPE_ID_PREVIEW;
  pipe->pipe_settings.preview = prev;
  break;
case IA_CSS_PIPE_MODE_CAPTURE:
  if (copy_pipe) {
pipe->mode = IA_CSS_PIPE_ID_COPY;
  } else {
pipe->mode = IA_CSS_PIPE_ID_CAPTURE;
  }
  pipe->pipe_settings.capture = capt;
  break;
case IA_CSS_PIPE_MODE_VIDEO:
  pipe->mode = IA_CSS_PIPE_ID_VIDEO;
  pipe->pipe_settings.video = video;
  break;
case IA_CSS_PIPE_MODE_ACC:
  pipe->mode = IA_CSS_PIPE_ID_ACC;
  break;
case IA_CSS_PIPE_MODE_COPY:
  pipe->mode = IA_CSS_PIPE_ID_CAPTURE;
  break;
case IA_CSS_PIPE_MODE_YUVPP:
  pipe->mode = IA_CSS_PIPE_ID_YUVPP;
  pipe->pipe_settings.yuvpp = yuvpp;
  break;
default:
  return IA_CSS_ERR_INVALID_ARGUMENTS;
}

return IA_CSS_SUCCESS;
  }

and GCC's limited support for using compound literals to initialize
static variables doesn't stretch this far.

I'm not convinced, however, that those variables actually achieve very
much.  If I change the code to assign the defaults directly, the problem
goes away:

  diff --git a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c
  index f92b6a9f77eb..671b2c732a46 100644
  --- a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c
  +++ b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c
  @@ -2291,25 +2291,19 @@ init_pipe_defaults(enum ia_css_pipe_mode mode,
 struct ia_css_pipe *pipe,
 bool copy_pipe)
   {
  -   static struct ia_css_pipe default_pipe = IA_CSS_DEFAULT_PIPE;
  -   static struct ia_css_preview_settings prev  = 
IA_CSS_DEFAULT_PREVIEW_SETTINGS;
  -   static struct ia_css_capture_settings capt  = 
IA_CSS_DEFAULT_CAPTURE_SETTINGS;
  -   static struct ia_css_video_settings   video = 
IA_CSS_DEFAULT_VIDEO_SETTINGS;
  -   static struct ia_css_yuvpp_settings   yuvpp = 
IA_CSS_DEFAULT_YUVPP_SETTINGS;
  -
  if (pipe == NULL) {
  IA_CSS_ERROR("NULL pipe parameter");
  return IA_CSS_ERR_INVALID_ARGUMENTS;
   

Re: [PATCH v2 00/10] posix_clocks: Prepare syscalls for 64 bit time_t conversion

[Deepa Dinamani]

On Tue, Nov 28, 2017 at 6:17 AM, Arnd Bergmann  wrote:
> On Mon, Nov 27, 2017 at 11:29 PM, Deepa Dinamani  
> wrote:
 I decided against using LEGACY_TIME_SYSCALLS to conditionally compile
 legacy time syscalls such as sys_nanosleep because this will need to
 enclose compat_sys_nanosleep as well. So, defining it as

 config LEGACY_TIME_SYSCALLS
  def_bool 64BIT || !64BIT_TIME

 will not include compat_sys_nanosleep. We will instead need a new config to
 exclusively mark legacy syscalls.
>>>
>>> Do you mean we would need to do this separately for native and compat
>>> syscalls, and have yet another option, like LEGACY_TIME_SYSCALLS
>>> and LEGACY_TIME_COMPAT_SYSCALLS, to cover all cases? I would
>>> think that CONFIG_COMPAT_32BIT_TIME handles all the compat versions,
>>> while CONFIG_LEGACY_TIME_SYSCALLS handles all the native ones.
>>
>> I meant sys_nanosleep would be covered by LEGACY_TIME_SYSCALLS, but
>> compat_sys_nanosleep would be covered by CONFIG_COMPAT_32BIT_TIME
>> along with other compat syscalls.
>> So, if we define the LEGACY_TIME_SYSCALLS as
>>
>>
>> "This controls the compilation of the following system calls:
>> time, stime, gettimeofday, settimeofday, adjtimex, nanosleep,
>> alarm, getitimer,
>> setitimer, select, utime, utimes, futimesat, and
>> {old,new}{l,f,}stat{,64}.
>> These all pass 32-bit time_t arguments on 32-bit architectures and
>> are replaced by other interfaces (e.g. posix timers and clocks, 
>> statx).
>> C libraries implementing 64-bit time_t in 32-bit architectures have 
>> to
>> implement the handles by wrapping around the newer interfaces.
>> New architectures should not explicitly enable this."
>>
>> This would not be really true as compat interfaces have nothing to do
>> with this config.
>>
>> I was proposing that we could have LEGACY_TIME_SYSCALLS config, but
>> then have all these "deprecated" syscalls be enclosed within this,
>> compat or not.
>> This will also mean that we will have to come up representing these
>> syscalls in the syscall header files.
>> This can be a separate patch and this series can be merged as is if
>> everyone agrees.
>
> I think doing this separately  would be good, I don't see any interdependency
> with the other patches, we just need to decide what we want in the long
> run.

Right. There are three options:

1. Use two configs to identify which syscalls need not be supported by
new architectures.
In this case it makes sense to say LEGACY_TIME_SYSCALLS and
COMPAT_32BIT_TIME both need to be disabled for new architectures. And,
I can reword the config to what you mention below.

2. Make the LEGACY_TIME_SYSCALLS eliminate non y2038 safe syscalls
mentioned below only.
In this case only the native and compat functions of the below
mentioned syscalls need to be identified by the config. I like this
option as this clearly identifies which syscalls are deprecated and do
not have a 64 bit counterpart. Not all architectures need to support
turning this off.

3. If we don't need either 1 or 2, then we could stick with what we
have today in the series as CONFIG_64BIT_TIME will be deleted and they
only need #ifdef CONFIG_64BIT.

Let me know if anyone prefers something else.

> I agree my text that you cited doesn't capture the situation correctly,
> as this is really about the obsolete system calls that take 64-bit time_t
> arguments on architectures that are converted to allow 64-bit time_t
> for non-obsolete system calls.
>
> Maybe it's better to just reword this to
>
>   "This controls the compilation of the following system calls:
>   time, stime, gettimeofday, settimeofday, adjtimex, nanosleep,
> alarm, getitimer,
>   setitimer, select, utime, utimes, futimesat, and 
> {old,new}{l,f,}stat{,64}.
>   These are all replaced by other interfaces (e.g. posix timers and 
> clocks,
>   statx) on architectures that got converted from 32-bit time_t to
> 64-bit time_t.
>   C libraries implementing 64-bit time_t in 32-bit architectures have to
>   implement the handles by wrapping around the newer interfaces.
>   New architectures should not explicitly enable this."
>
> That would clarify that it's not about the compat system calls, while
> also allowing the two options to be set independently.

-Deepa
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v6 2/2] staging: ion: create one device entry per heap

[Greg KH]

On Tue, Nov 28, 2017 at 05:37:53PM +, Mark Brown wrote:
> On Tue, Nov 28, 2017 at 06:28:38PM +0100, Greg KH wrote:
> > On Tue, Nov 28, 2017 at 05:12:23PM +, Mark Brown wrote:
> 
> > > I think it's reasonable to ask for userspace, I'm querying why it needs
> > > to specifically be Android.
> 
> > Does anyone other than Android use this interface?
> 
> There's plenty of other userspaces that have the same requirements for
> allocation for applications like video capture, I believe some of them
> have actually moved to ION already and they're certainly where some of
> the requirements here are coming from.

Then the Kconfig option should start to describe some of this :)
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH Resend] staging: media: lirc: style fix - replace hard-coded function names

[Martin Homuth]

This patch fixes the remaining coding style warnings in the lirc module.
Instead of hard coding the function name the __func__ variable
should be used.

It fixes the following checkpatch.pl warning:

WARNING: Prefer using '"%s...", __func__' to using 'read', this
function's name, in a string

Signed-off-by: Martin Homuth 
---
 drivers/staging/media/lirc/lirc_zilog.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/media/lirc/lirc_zilog.c
b/drivers/staging/media/lirc/lirc_zilog.c
index 6bd0717bf76e..be68ee652071 100644
--- a/drivers/staging/media/lirc/lirc_zilog.c
+++ b/drivers/staging/media/lirc/lirc_zilog.c
@@ -888,9 +888,9 @@ static ssize_t read(struct file *filep, char __user
*outbuf, size_t n,
unsigned int m;
DECLARE_WAITQUEUE(wait, current);

-   dev_dbg(ir->dev, "read called\n");
+   dev_dbg(ir->dev, "%s called\n", __func__);
if (n % rbuf->chunk_size) {
-   dev_dbg(ir->dev, "read result = -EINVAL\n");
+   dev_dbg(ir->dev, "%s result = -EINVAL\n", __func__);
return -EINVAL;
}

@@ -949,7 +949,7 @@ static ssize_t read(struct file *filep, char __user
*outbuf, size_t n,
retries++;
}
if (retries >= 5) {
-   dev_err(ir->dev, "Buffer read failed!\n");
+   dev_err(ir->dev, "%s failed!\n", __func__);
ret = -EIO;
}
}
@@ -959,7 +959,7 @@ static ssize_t read(struct file *filep, char __user
*outbuf, size_t n,
put_ir_rx(rx, false);
set_current_state(TASK_RUNNING);

-   dev_dbg(ir->dev, "read result = %d (%s)\n", ret,
+   dev_dbg(ir->dev, "%s result = %d (%s)\n", __func__, ret,
ret ? "Error" : "OK");

return ret ? ret : written;
-- 
2.13.6

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v6 2/2] staging: ion: create one device entry per heap

[Mark Brown]

On Tue, Nov 28, 2017 at 06:28:38PM +0100, Greg KH wrote:
> On Tue, Nov 28, 2017 at 05:12:23PM +, Mark Brown wrote:

> > I think it's reasonable to ask for userspace, I'm querying why it needs
> > to specifically be Android.

> Does anyone other than Android use this interface?

There's plenty of other userspaces that have the same requirements for
allocation for applications like video capture, I believe some of them
have actually moved to ION already and they're certainly where some of
the requirements here are coming from.


signature.asc
Description: PGP signature
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: pi433: pi433_if.c codestyle space prohibited

[Oliver Graute]

This patch fixes the following checkpatch.pl error:

ERROR: space prohibited after that open parenthesis '('
#973: FILE: pi433_if.c:973:
+   if ( IS_ERR(device->gpiod[i]) )


ERROR: space prohibited after that open parenthesis '('
#19: FILE: drivers/staging/pi433/pi433_if.c:954:
+   if ( IS_ERR(device->gpiod[i]))

Signed-off-by: Oliver Graute 
---
 drivers/staging/pi433/pi433_if.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
index 8d51f055..4b8980c 100644
--- a/drivers/staging/pi433/pi433_if.c
+++ b/drivers/staging/pi433/pi433_if.c
@@ -969,7 +969,7 @@ static void free_GPIOs(struct pi433_device *device)
for (i = 0; i < NUM_DIO; i++)
{
/* check if gpiod is valid */
-   if ( IS_ERR(device->gpiod[i]) )
+   if (IS_ERR(device->gpiod[i]))
continue;
 
free_irq(device->irq_num[i], device);
-- 
1.9.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/2] vmbus: unregister device_obj->channels_kset

[Stephen Hemminger]

On Tue, 28 Nov 2017 16:56:05 +0100
Greg KH  wrote:

> On Tue, Nov 14, 2017 at 06:53:32AM -0700, k...@exchange.microsoft.com wrote:
> > From: Dexuan Cui 
> > 
> > Fixes: c2e5df616e1a ("vmbus: add per-channel sysfs info")
> > 
> > Without the patch, a device can't be thoroughly destroyed, because
> > vmbus_device_register() -> kset_create_and_add() still holds a reference
> > to the hv_device's device.kobj.
> > 
> > Signed-off-by: Dexuan Cui 
> > Cc: Stephen Hemminger 
> > Cc: sta...@vger.kernel.org  
> 
> Why is this marked for stable when the patch it "fixes" is in 4.15-rc1?

It doesn't need to go to stable.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v6 2/2] staging: ion: create one device entry per heap

[Greg KH]

On Tue, Nov 28, 2017 at 05:12:23PM +, Mark Brown wrote:
> On Tue, Nov 28, 2017 at 06:08:22PM +0100, Greg KH wrote:
> > On Tue, Nov 28, 2017 at 04:26:20PM +, Mark Brown wrote:
> > > On Tue, Nov 28, 2017 at 02:32:17PM +0100, Greg KH wrote:
> 
> > > > call you added?  What did you do to test this out?  Where are the AOSP
> > > > patches to use this?  Happen to have a VTS test for it?
> 
> > > Do we need to convert Android for this to be accepted?  The single
> > > device is being kept around for it and the use case was from non-Android
> > > users wasn't it?
> 
> > So we are just going to add kernel features with no userspace users of
> > it at all?  Why would we do that?  How was it even tested?
> 
> I think it's reasonable to ask for userspace, I'm querying why it needs
> to specifically be Android.

Does anyone other than Android use this interface?
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v6 2/2] staging: ion: create one device entry per heap

[Mark Brown]

On Tue, Nov 28, 2017 at 06:08:22PM +0100, Greg KH wrote:
> On Tue, Nov 28, 2017 at 04:26:20PM +, Mark Brown wrote:
> > On Tue, Nov 28, 2017 at 02:32:17PM +0100, Greg KH wrote:

> > > call you added?  What did you do to test this out?  Where are the AOSP
> > > patches to use this?  Happen to have a VTS test for it?

> > Do we need to convert Android for this to be accepted?  The single
> > device is being kept around for it and the use case was from non-Android
> > users wasn't it?

> So we are just going to add kernel features with no userspace users of
> it at all?  Why would we do that?  How was it even tested?

I think it's reasonable to ask for userspace, I'm querying why it needs
to specifically be Android.


signature.asc
Description: PGP signature
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v6 2/2] staging: ion: create one device entry per heap

[Greg KH]

On Tue, Nov 28, 2017 at 04:26:20PM +, Mark Brown wrote:
> On Tue, Nov 28, 2017 at 02:32:17PM +0100, Greg KH wrote:
> 
> > Where is the documentation for the new sysfs files and the new ioctl
> 
> Didn't see any sysfs files in there?

New struct devices were created and registered.  Why would that happen
if there was not a need for sysfs files? :)

> > call you added?  What did you do to test this out?  Where are the AOSP
> > patches to use this?  Happen to have a VTS test for it?
> 
> Do we need to convert Android for this to be accepted?  The single
> device is being kept around for it and the use case was from non-Android
> users wasn't it?

So we are just going to add kernel features with no userspace users of
it at all?  Why would we do that?  How was it even tested?

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH 2/2] vmbus: make hv_get_ringbuffer_availbytes local

[Stephen Hemminger]

This patch required a patch that is still going through net-next.


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v6 2/2] staging: ion: create one device entry per heap

[Mark Brown]

On Tue, Nov 28, 2017 at 02:32:17PM +0100, Greg KH wrote:

> Where is the documentation for the new sysfs files and the new ioctl

Didn't see any sysfs files in there?

> call you added?  What did you do to test this out?  Where are the AOSP
> patches to use this?  Happen to have a VTS test for it?

Do we need to convert Android for this to be accepted?  The single
device is being kept around for it and the use case was from non-Android
users wasn't it?


signature.asc
Description: PGP signature
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3] staging: fsl-mc: move bus driver out of staging

[Andrew Lunn]

On Tue, Nov 28, 2017 at 05:27:57PM +0200, laurentiu.tu...@nxp.com wrote:
> diff --git a/drivers/staging/fsl-mc/bus/dpmcp.h b/drivers/bus/fsl-mc/dpmcp.h
> similarity index 100%
> rename from drivers/staging/fsl-mc/bus/dpmcp.h
> rename to drivers/bus/fsl-mc/dpmcp.h
> diff --git a/drivers/staging/fsl-mc/bus/dpmng-cmd.h 
> b/drivers/bus/fsl-mc/dpmng-cmd.h
> similarity index 100%
> rename from drivers/staging/fsl-mc/bus/dpmng-cmd.h
> rename to drivers/bus/fsl-mc/dpmng-cmd.h
> diff --git a/drivers/staging/fsl-mc/bus/dprc-cmd.h 
> b/drivers/bus/fsl-mc/dprc-cmd.h
> similarity index 100%
> rename from drivers/staging/fsl-mc/bus/dprc-cmd.h
> rename to drivers/bus/fsl-mc/dprc-cmd.h
> diff --git a/drivers/staging/fsl-mc/bus/dprc-driver.c 
> b/drivers/bus/fsl-mc/dprc-driver.c
> similarity index 99%
> rename from drivers/staging/fsl-mc/bus/dprc-driver.c
> rename to drivers/bus/fsl-mc/dprc-driver.c

Hi Laurentiu

It is hard to review code when we don't see it. 

   Andrew
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH 2/2] vmbus: make hv_get_ringbuffer_availbytes local

[KY Srinivasan]

> -Original Message-
> From: Greg KH [mailto:gre...@linuxfoundation.org]
> Sent: Tuesday, November 28, 2017 7:35 AM
> To: KY Srinivasan 
> Cc: linux-ker...@vger.kernel.org; de...@linuxdriverproject.org;
> o...@aepfle.de; a...@canonical.com; vkuzn...@redhat.com;
> jasow...@redhat.com; leann.ogasaw...@canonical.com;
> marcelo.ce...@canonical.com; Stephen Hemminger
> ; Stephen Hemminger
> 
> Subject: Re: [PATCH 2/2] vmbus: make hv_get_ringbuffer_availbytes local
> 
> On Tue, Nov 14, 2017 at 10:52:08AM -0700, k...@exchange.microsoft.com
> wrote:
> > From: Stephen Hemminger 
> >
> > hv_get_ringbuffer_availbytes is only used by the debug info
> > routine so make it static. Also, add READ_ONCE() to avoid any
> > possible issues with potentially volatile index values.
> >
> > Signed-off-by: Stephen Hemminger 
> > Signed-off-by: K. Y. Srinivasan 
> > ---
> >  drivers/hv/ring_buffer.c |   23 +++
> >  include/linux/hyperv.h   |   22 --
> >  2 files changed, 23 insertions(+), 22 deletions(-)
> >
> > diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c
> > index 12eb8ca..50e0714 100644
> > --- a/drivers/hv/ring_buffer.c
> > +++ b/drivers/hv/ring_buffer.c
> > @@ -140,6 +140,29 @@ static u32 hv_copyto_ringbuffer(
> > return start_write_offset;
> >  }
> >
> > +/*
> > + *
> > + * hv_get_ringbuffer_availbytes()
> > + *
> > + * Get number of bytes available to read and to write to
> > + * for the specified ring buffer
> > + */
> > +static void
> > +hv_get_ringbuffer_availbytes(const struct hv_ring_buffer_info *rbi,
> > +u32 *read, u32 *write)
> > +{
> > +   u32 read_loc, write_loc, dsize;
> > +
> > +   /* Capture the read/write indices before they changed */
> > +   read_loc = READ_ONCE(rbi->ring_buffer->read_index);
> > +   write_loc = READ_ONCE(rbi->ring_buffer->write_index);
> > +   dsize = rbi->ring_datasize;
> > +
> > +   *write = write_loc >= read_loc ? dsize - (write_loc - read_loc) :
> > +   read_loc - write_loc;
> > +   *read = dsize - *write;
> > +}
> > +
> >  /* Get various debug metrics for the specified ring buffer. */
> >  void hv_ringbuffer_get_debuginfo(const struct hv_ring_buffer_info
> *ring_info,
> >  struct hv_ring_buffer_debug_info
> *debug_info)
> > diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> > index 6c93366..93bd6fc 100644
> > --- a/include/linux/hyperv.h
> > +++ b/include/linux/hyperv.h
> > @@ -127,28 +127,6 @@ struct hv_ring_buffer_info {
> > u32 priv_read_index;
> >  };
> >
> > -/*
> > - *
> > - * hv_get_ringbuffer_availbytes()
> > - *
> > - * Get number of bytes available to read and to write to
> > - * for the specified ring buffer
> > - */
> > -static inline void
> > -hv_get_ringbuffer_availbytes(const struct hv_ring_buffer_info *rbi,
> > -u32 *read, u32 *write)
> > -{
> > -   u32 read_loc, write_loc, dsize;
> > -
> > -   /* Capture the read/write indices before they changed */
> > -   read_loc = rbi->ring_buffer->read_index;
> > -   write_loc = rbi->ring_buffer->write_index;
> > -   dsize = rbi->ring_datasize;
> > -
> > -   *write = write_loc >= read_loc ? dsize - (write_loc - read_loc) :
> > -   read_loc - write_loc;
> > -   *read = dsize - *write;
> > -}
> >
> >  static inline u32 hv_get_bytes_to_read(const struct hv_ring_buffer_info
> *rbi)
> >  {
> > --
> > 1.7.1
> 
> This breaks the build.
> 
> Please be more careful...

Sorry about that. I did not have a build issue on the tree I tested. I will 
rebase and send.

K. Y
> 
> greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/2] vmbus: unregister device_obj->channels_kset

[Greg KH]

On Tue, Nov 14, 2017 at 06:53:32AM -0700, k...@exchange.microsoft.com wrote:
> From: Dexuan Cui 
> 
> Fixes: c2e5df616e1a ("vmbus: add per-channel sysfs info")
> 
> Without the patch, a device can't be thoroughly destroyed, because
> vmbus_device_register() -> kset_create_and_add() still holds a reference
> to the hv_device's device.kobj.
> 
> Signed-off-by: Dexuan Cui 
> Cc: Stephen Hemminger 
> Cc: sta...@vger.kernel.org

Why is this marked for stable when the patch it "fixes" is in 4.15-rc1?

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/2] vmbus: make hv_get_ringbuffer_availbytes local

[Greg KH]

On Tue, Nov 14, 2017 at 10:52:08AM -0700, k...@exchange.microsoft.com wrote:
> From: Stephen Hemminger 
> 
> hv_get_ringbuffer_availbytes is only used by the debug info
> routine so make it static. Also, add READ_ONCE() to avoid any
> possible issues with potentially volatile index values.
> 
> Signed-off-by: Stephen Hemminger 
> Signed-off-by: K. Y. Srinivasan 
> ---
>  drivers/hv/ring_buffer.c |   23 +++
>  include/linux/hyperv.h   |   22 --
>  2 files changed, 23 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c
> index 12eb8ca..50e0714 100644
> --- a/drivers/hv/ring_buffer.c
> +++ b/drivers/hv/ring_buffer.c
> @@ -140,6 +140,29 @@ static u32 hv_copyto_ringbuffer(
>   return start_write_offset;
>  }
>  
> +/*
> + *
> + * hv_get_ringbuffer_availbytes()
> + *
> + * Get number of bytes available to read and to write to
> + * for the specified ring buffer
> + */
> +static void
> +hv_get_ringbuffer_availbytes(const struct hv_ring_buffer_info *rbi,
> +  u32 *read, u32 *write)
> +{
> + u32 read_loc, write_loc, dsize;
> +
> + /* Capture the read/write indices before they changed */
> + read_loc = READ_ONCE(rbi->ring_buffer->read_index);
> + write_loc = READ_ONCE(rbi->ring_buffer->write_index);
> + dsize = rbi->ring_datasize;
> +
> + *write = write_loc >= read_loc ? dsize - (write_loc - read_loc) :
> + read_loc - write_loc;
> + *read = dsize - *write;
> +}
> +
>  /* Get various debug metrics for the specified ring buffer. */
>  void hv_ringbuffer_get_debuginfo(const struct hv_ring_buffer_info *ring_info,
>struct hv_ring_buffer_debug_info *debug_info)
> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> index 6c93366..93bd6fc 100644
> --- a/include/linux/hyperv.h
> +++ b/include/linux/hyperv.h
> @@ -127,28 +127,6 @@ struct hv_ring_buffer_info {
>   u32 priv_read_index;
>  };
>  
> -/*
> - *
> - * hv_get_ringbuffer_availbytes()
> - *
> - * Get number of bytes available to read and to write to
> - * for the specified ring buffer
> - */
> -static inline void
> -hv_get_ringbuffer_availbytes(const struct hv_ring_buffer_info *rbi,
> -  u32 *read, u32 *write)
> -{
> - u32 read_loc, write_loc, dsize;
> -
> - /* Capture the read/write indices before they changed */
> - read_loc = rbi->ring_buffer->read_index;
> - write_loc = rbi->ring_buffer->write_index;
> - dsize = rbi->ring_datasize;
> -
> - *write = write_loc >= read_loc ? dsize - (write_loc - read_loc) :
> - read_loc - write_loc;
> - *read = dsize - *write;
> -}
>  
>  static inline u32 hv_get_bytes_to_read(const struct hv_ring_buffer_info *rbi)
>  {
> -- 
> 1.7.1

This breaks the build.

Please be more careful...

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/2] Drivers: hv: vmbus: Remove x86-isms from arch independent drivers

[Greg KH]

On Tue, Nov 14, 2017 at 10:52:07AM -0700, k...@exchange.microsoft.com wrote:
> From: Michael Kelley 
> 
> hv_is_hypercall_page_setup() is used to check if Hyper-V is
> initialized, but a 'hypercall page' is an x86 implementation detail
> that isn't necessarily present on other architectures. Rename to the
> architecture independent hv_is_hyperv_initialized() and add check
> that x86_hyper is pointing to Hyper-V.  Use this function instead of
> direct references to x86-specific data structures in vmbus_drv.c,
> and remove now redundant call in hv_init(). Also remove 'x86' from
> the string name passed to cpuhp_setup_state().
> 
> Signed-off-by: Michael Kelley 
> Signed-off-by: K. Y. Srinivasan 
> ---
>  arch/x86/hyperv/hv_init.c   |   21 ++---
>  arch/x86/include/asm/mshyperv.h |4 ++--
>  drivers/hv/hv.c |3 ---
>  drivers/hv/vmbus_drv.c  |5 ++---
>  4 files changed, 18 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> index aeb8edf..e5372c9 100644
> --- a/arch/x86/hyperv/hv_init.c
> +++ b/arch/x86/hyperv/hv_init.c
> @@ -239,17 +239,24 @@ void hyperv_report_panic(struct pt_regs *regs, long err)
>  }
>  EXPORT_SYMBOL_GPL(hyperv_report_panic);
>  
> -bool hv_is_hypercall_page_setup(void)
> +bool hv_is_hyperv_initialized(void)
>  {
>   union hv_x64_msr_hypercall_contents hypercall_msr;
>  
> - /* Check if the hypercall page is setup */
> + /*
> +  * Ensure that we're really on Hyper-V, and not a KVM or Xen
> +  * emulation of Hyper-V
> +  */
> + if (x86_hyper != _hyper_ms_hyperv)
> + return false;
> +
> + /*
> +  * Verify that earlier initialization succeeded by checking
> +  * that the hypercall page is setup
> +  */
>   hypercall_msr.as_uint64 = 0;
>   rdmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
>  
> - if (!hypercall_msr.enable)
> - return false;
> -
> - return true;
> + return hypercall_msr.enable;
>  }
> -EXPORT_SYMBOL_GPL(hv_is_hypercall_page_setup);
> +EXPORT_SYMBOL_GPL(hv_is_hyperv_initialized);
> diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
> index bd89104..740dc97 100644
> --- a/arch/x86/include/asm/mshyperv.h
> +++ b/arch/x86/include/asm/mshyperv.h
> @@ -311,11 +311,11 @@ static inline int hv_cpu_number_to_vp_number(int 
> cpu_number)
>  void hyperv_setup_mmu_ops(void);
>  void hyper_alloc_mmu(void);
>  void hyperv_report_panic(struct pt_regs *regs, long err);
> -bool hv_is_hypercall_page_setup(void);
> +bool hv_is_hyperv_initialized(void);
>  void hyperv_cleanup(void);
>  #else /* CONFIG_HYPERV */
>  static inline void hyperv_init(void) {}
> -static inline bool hv_is_hypercall_page_setup(void) { return false; }
> +static inline bool hv_is_hyperv_initialized(void) { return false; }
>  static inline void hyperv_cleanup(void) {}
>  static inline void hyperv_setup_mmu_ops(void) {}
>  #endif /* CONFIG_HYPERV */
> diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
> index 8267439..fe96aab 100644
> --- a/drivers/hv/hv.c
> +++ b/drivers/hv/hv.c
> @@ -49,9 +49,6 @@ struct hv_context hv_context = {
>   */
>  int hv_init(void)
>  {
> - if (!hv_is_hypercall_page_setup())
> - return -ENOTSUPP;
> -
>   hv_context.cpu_context = alloc_percpu(struct hv_per_cpu_context);
>   if (!hv_context.cpu_context)
>   return -ENOMEM;
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index 4f3faf5..398643b 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -37,7 +37,6 @@
>  #include 
>  
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
> @@ -1053,7 +1052,7 @@ static int vmbus_bus_init(void)
>* Initialize the per-cpu interrupt state and
>* connect to the host.
>*/
> - ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "x86/hyperv:online",
> + ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "hyperv/vmbus:online",
>   hv_synic_init, hv_synic_cleanup);
>   if (ret < 0)
>   goto err_alloc;
> @@ -1717,7 +1716,7 @@ static int __init hv_acpi_init(void)
>  {
>   int ret, t;
>  
> - if (x86_hyper != _hyper_ms_hyperv)
> + if (!hv_is_hyperv_initialized())
>   return -ENODEV;
>  
>   init_completion(_event);
> -- 
> 1.7.1


Does not apply to 4.15-rc1 :(

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3] staging: fsl-mc: move bus driver out of staging

[laurentiu.tudor]

From: Stuart Yoder 

Move the source files out of staging into their final locations:
  -include files in drivers/staging/fsl-mc/include go to include/linux/fsl
  -irq-gic-v3-its-fsl-mc-msi.c goes to drivers/irqchip
  -source in drivers/staging/fsl-mc/bus goes to drivers/bus/fsl-mc
  -README.txt, providing and overview of DPAA goes to
   Documentation/dpaa2/overview.txt

Update or delete other remaining staging files-- Makefile, Kconfig, TODO.
Update dpaa2_eth and dpio staging drivers.

Signed-off-by: Stuart Yoder 
Signed-off-by: Laurentiu Tudor 
[Laurentiu: rebased, add dpaa2_eth and dpio #include updates]
Cc: Thomas Gleixner 
Cc: Jason Cooper 
Cc: Marc Zyngier 
---
Notes:
-v3:
  - rebased
-v2:
  - group irqchip gic its glue code together with the rest (Marc Zyngier)

 .../README.txt => Documentation/dpaa2/overview.txt |  0
 MAINTAINERS|  2 +-
 drivers/bus/Kconfig|  2 ++
 drivers/bus/Makefile   |  3 +++
 drivers/bus/fsl-mc/Kconfig | 17 +
 drivers/bus/fsl-mc/Makefile| 17 +
 drivers/{staging/fsl-mc/bus => bus/fsl-mc}/dpmcp-cmd.h |  0
 drivers/{staging/fsl-mc/bus => bus/fsl-mc}/dpmcp.c |  2 +-
 drivers/{staging/fsl-mc/bus => bus/fsl-mc}/dpmcp.h |  0
 drivers/{staging/fsl-mc/bus => bus/fsl-mc}/dpmng-cmd.h |  0
 drivers/{staging/fsl-mc/bus => bus/fsl-mc}/dprc-cmd.h  |  0
 .../{staging/fsl-mc/bus => bus/fsl-mc}/dprc-driver.c   |  1 -
 drivers/{staging/fsl-mc/bus => bus/fsl-mc}/dprc.c  |  2 +-
 drivers/{staging/fsl-mc/bus => bus/fsl-mc}/dprc.h  |  0
 .../fsl-mc/bus => bus/fsl-mc}/fsl-mc-allocator.c   |  1 -
 .../{staging/fsl-mc/bus => bus/fsl-mc}/fsl-mc-bus.c|  0
 .../{staging/fsl-mc/bus => bus/fsl-mc}/fsl-mc-msi.c|  0
 .../fsl-mc/bus => bus/fsl-mc}/fsl-mc-private.h |  2 +-
 drivers/{staging/fsl-mc/bus => bus/fsl-mc}/mc-io.c |  1 -
 drivers/{staging/fsl-mc/bus => bus/fsl-mc}/mc-sys.c|  2 +-
 drivers/irqchip/Makefile   |  1 +
 .../fsl-mc/bus => irqchip}/irq-gic-v3-its-fsl-mc-msi.c |  2 +-
 drivers/staging/fsl-dpaa2/ethernet/README  |  2 +-
 drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c |  2 +-
 drivers/staging/fsl-dpaa2/ethernet/dpni.c  |  2 +-
 drivers/staging/fsl-mc/TODO| 18 --
 drivers/staging/fsl-mc/bus/Kconfig | 10 --
 drivers/staging/fsl-mc/bus/Makefile| 15 ++-
 drivers/staging/fsl-mc/bus/dpbp.c  |  2 +-
 drivers/staging/fsl-mc/bus/dpcon.c |  2 +-
 drivers/staging/fsl-mc/bus/dpio/dpio-driver.c  |  2 +-
 drivers/staging/fsl-mc/bus/dpio/dpio-service.c |  2 +-
 drivers/staging/fsl-mc/bus/dpio/dpio.c |  2 +-
 .../staging/fsl-mc/include => include/linux/fsl}/mc.h  |  0
 34 files changed, 56 insertions(+), 58 deletions(-)
 rename drivers/staging/fsl-mc/README.txt => Documentation/dpaa2/overview.txt 
(100%)
 create mode 100644 drivers/bus/fsl-mc/Kconfig
 create mode 100644 drivers/bus/fsl-mc/Makefile
 rename drivers/{staging/fsl-mc/bus => bus/fsl-mc}/dpmcp-cmd.h (100%)
 rename drivers/{staging/fsl-mc/bus => bus/fsl-mc}/dpmcp.c (99%)
 rename drivers/{staging/fsl-mc/bus => bus/fsl-mc}/dpmcp.h (100%)
 rename drivers/{staging/fsl-mc/bus => bus/fsl-mc}/dpmng-cmd.h (100%)
 rename drivers/{staging/fsl-mc/bus => bus/fsl-mc}/dprc-cmd.h (100%)
 rename drivers/{staging/fsl-mc/bus => bus/fsl-mc}/dprc-driver.c (99%)
 rename drivers/{staging/fsl-mc/bus => bus/fsl-mc}/dprc.c (99%)
 rename drivers/{staging/fsl-mc/bus => bus/fsl-mc}/dprc.h (100%)
 rename drivers/{staging/fsl-mc/bus => bus/fsl-mc}/fsl-mc-allocator.c (99%)
 rename drivers/{staging/fsl-mc/bus => bus/fsl-mc}/fsl-mc-bus.c (100%)
 rename drivers/{staging/fsl-mc/bus => bus/fsl-mc}/fsl-mc-msi.c (100%)
 rename drivers/{staging/fsl-mc/bus => bus/fsl-mc}/fsl-mc-private.h (99%)
 rename drivers/{staging/fsl-mc/bus => bus/fsl-mc}/mc-io.c (99%)
 rename drivers/{staging/fsl-mc/bus => bus/fsl-mc}/mc-sys.c (99%)
 rename drivers/{staging/fsl-mc/bus => irqchip}/irq-gic-v3-its-fsl-mc-msi.c 
(99%)
 delete mode 100644 drivers/staging/fsl-mc/TODO
 rename {drivers/staging/fsl-mc/include => include/linux/fsl}/mc.h (100%)

diff --git a/drivers/staging/fsl-mc/README.txt 
b/Documentation/dpaa2/overview.txt
similarity index 100%
rename from drivers/staging/fsl-mc/README.txt
rename to Documentation/dpaa2/overview.txt
diff --git a/MAINTAINERS b/MAINTAINERS
index aa71ab52f..f2d9fe7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11276,7 +11276,7 @@ M:  Stuart Yoder 
 M: Laurentiu Tudor 
 L: linux-ker...@vger.kernel.org
 

Re: [PATCH] staging: lustre: Fix line over 80 characters

[Dan Carpenter]

On Tue, Nov 28, 2017 at 03:18:59PM +0100, Greg KH wrote:
> On Tue, Nov 28, 2017 at 05:13:55PM +0300, Dan Carpenter wrote:
> > On Mon, Nov 27, 2017 at 03:36:58PM +0200, Talat Batheesh wrote:
> > > This patch fix the line over 80 characters warning that was detected
> > > using checkpatch.pl script.
> > > 
> > > Signed-off-by: Talat Batheesh 
> > > ---
> > >  drivers/staging/lustre/lustre/obdclass/class_obd.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/staging/lustre/lustre/obdclass/class_obd.c 
> > > b/drivers/staging/lustre/lustre/obdclass/class_obd.c
> > > index 2985bca..3e24b76 100644
> > > --- a/drivers/staging/lustre/lustre/obdclass/class_obd.c
> > > +++ b/drivers/staging/lustre/lustre/obdclass/class_obd.c
> > > @@ -377,7 +377,8 @@ static int obd_init_checks(void)
> > >   char buf[64];
> > >   int len, ret = 0;
> > >  
> > > - CDEBUG(D_INFO, "LPU64=%s, LPD64=%s, LPX64=%s\n", "%llu", "%lld", 
> > > "%#llx");
> > > + CDEBUG(D_INFO, "LPU64=%s, LPD64=%s, LPX64=%s\n", "%llu", "%lld",
> > > +"%#llx");
> > 
> > No, that will break things.  Ignore string literals when dealing with
> > the 80 character limit.
> 
> What will that break?  Yeah, it's a horrid line, but it should still
> work...
> 

Oh...  Yeah.  You're right.  It's fine.

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: lustre: Fix line over 80 characters

[Greg KH]

On Tue, Nov 28, 2017 at 05:13:55PM +0300, Dan Carpenter wrote:
> On Mon, Nov 27, 2017 at 03:36:58PM +0200, Talat Batheesh wrote:
> > This patch fix the line over 80 characters warning that was detected
> > using checkpatch.pl script.
> > 
> > Signed-off-by: Talat Batheesh 
> > ---
> >  drivers/staging/lustre/lustre/obdclass/class_obd.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/staging/lustre/lustre/obdclass/class_obd.c 
> > b/drivers/staging/lustre/lustre/obdclass/class_obd.c
> > index 2985bca..3e24b76 100644
> > --- a/drivers/staging/lustre/lustre/obdclass/class_obd.c
> > +++ b/drivers/staging/lustre/lustre/obdclass/class_obd.c
> > @@ -377,7 +377,8 @@ static int obd_init_checks(void)
> > char buf[64];
> > int len, ret = 0;
> >  
> > -   CDEBUG(D_INFO, "LPU64=%s, LPD64=%s, LPX64=%s\n", "%llu", "%lld", 
> > "%#llx");
> > +   CDEBUG(D_INFO, "LPU64=%s, LPD64=%s, LPX64=%s\n", "%llu", "%lld",
> > +  "%#llx");
> 
> No, that will break things.  Ignore string literals when dealing with
> the 80 character limit.

What will that break?  Yeah, it's a horrid line, but it should still
work...

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 00/10] posix_clocks: Prepare syscalls for 64 bit time_t conversion

[Arnd Bergmann]

On Mon, Nov 27, 2017 at 11:29 PM, Deepa Dinamani  wrote:
>>> I decided against using LEGACY_TIME_SYSCALLS to conditionally compile
>>> legacy time syscalls such as sys_nanosleep because this will need to
>>> enclose compat_sys_nanosleep as well. So, defining it as
>>>
>>> config LEGACY_TIME_SYSCALLS
>>>  def_bool 64BIT || !64BIT_TIME
>>>
>>> will not include compat_sys_nanosleep. We will instead need a new config to
>>> exclusively mark legacy syscalls.
>>
>> Do you mean we would need to do this separately for native and compat
>> syscalls, and have yet another option, like LEGACY_TIME_SYSCALLS
>> and LEGACY_TIME_COMPAT_SYSCALLS, to cover all cases? I would
>> think that CONFIG_COMPAT_32BIT_TIME handles all the compat versions,
>> while CONFIG_LEGACY_TIME_SYSCALLS handles all the native ones.
>
> I meant sys_nanosleep would be covered by LEGACY_TIME_SYSCALLS, but
> compat_sys_nanosleep would be covered by CONFIG_COMPAT_32BIT_TIME
> along with other compat syscalls.
> So, if we define the LEGACY_TIME_SYSCALLS as
>
>
> "This controls the compilation of the following system calls:
> time, stime, gettimeofday, settimeofday, adjtimex, nanosleep,
> alarm, getitimer,
> setitimer, select, utime, utimes, futimesat, and
> {old,new}{l,f,}stat{,64}.
> These all pass 32-bit time_t arguments on 32-bit architectures and
> are replaced by other interfaces (e.g. posix timers and clocks, 
> statx).
> C libraries implementing 64-bit time_t in 32-bit architectures have to
> implement the handles by wrapping around the newer interfaces.
> New architectures should not explicitly enable this."
>
> This would not be really true as compat interfaces have nothing to do
> with this config.
>
> I was proposing that we could have LEGACY_TIME_SYSCALLS config, but
> then have all these "deprecated" syscalls be enclosed within this,
> compat or not.
> This will also mean that we will have to come up representing these
> syscalls in the syscall header files.
> This can be a separate patch and this series can be merged as is if
> everyone agrees.

I think doing this separately  would be good, I don't see any interdependency
with the other patches, we just need to decide what we want in the long
run.

I agree my text that you cited doesn't capture the situation correctly,
as this is really about the obsolete system calls that take 64-bit time_t
arguments on architectures that are converted to allow 64-bit time_t
for non-obsolete system calls.

Maybe it's better to just reword this to

  "This controls the compilation of the following system calls:
  time, stime, gettimeofday, settimeofday, adjtimex, nanosleep,
alarm, getitimer,
  setitimer, select, utime, utimes, futimesat, and {old,new}{l,f,}stat{,64}.
  These are all replaced by other interfaces (e.g. posix timers and clocks,
  statx) on architectures that got converted from 32-bit time_t to
64-bit time_t.
  C libraries implementing 64-bit time_t in 32-bit architectures have to
  implement the handles by wrapping around the newer interfaces.
  New architectures should not explicitly enable this."

That would clarify that it's not about the compat system calls, while
also allowing the two options to be set independently.

Arnd
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 1/3] media: staging: atomisp: fix for sparse "using plain integer as NULL pointer" warnings.

[Dan Carpenter]

On Mon, Nov 27, 2017 at 12:44:48PM +, Jeremy Sowden wrote:
> The "address" member of struct ia_css_host_data is a pointer-to-char, so 
> define default as NULL.
> 
> Signed-off-by: Jeremy Sowden 
> ---
>  .../css2400/runtime/isp_param/interface/ia_css_isp_param_types.h| 2 
> +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git 
> a/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/isp_param/interface/ia_css_isp_param_types.h
>  
> b/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/isp_param/interface/ia_css_isp_param_types.h
> index 8e651b80345a..6fee3f7fd184 100644
> --- 
> a/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/isp_param/interface/ia_css_isp_param_types.h
> +++ 
> b/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/isp_param/interface/ia_css_isp_param_types.h
> @@ -95,7 +95,7 @@ union ia_css_all_memory_offsets {
>  };
>  
>  #define IA_CSS_DEFAULT_ISP_MEM_PARAMS \
> - { { { { 0, 0 } } } }
> + { { { { NULL, 0 } } } }

This define is way ugly and instead of making superficial changes, you
should try to eliminate it.

People look at warnings as a bad thing but they are actually a valuable
resource which call attention to bad code.  By making this change you're
kind of wasting the warning.  The bad code is still there, it's just
swept under the rug but like a dead mouse carcass it's still stinking up
the living room.  We should leave the warning there until it irritates
someone enough to fix it properly.

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: lustre: Fix line over 80 characters

[Dan Carpenter]

On Mon, Nov 27, 2017 at 03:36:58PM +0200, Talat Batheesh wrote:
> This patch fix the line over 80 characters warning that was detected
> using checkpatch.pl script.
> 
> Signed-off-by: Talat Batheesh 
> ---
>  drivers/staging/lustre/lustre/obdclass/class_obd.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/lustre/lustre/obdclass/class_obd.c 
> b/drivers/staging/lustre/lustre/obdclass/class_obd.c
> index 2985bca..3e24b76 100644
> --- a/drivers/staging/lustre/lustre/obdclass/class_obd.c
> +++ b/drivers/staging/lustre/lustre/obdclass/class_obd.c
> @@ -377,7 +377,8 @@ static int obd_init_checks(void)
>   char buf[64];
>   int len, ret = 0;
>  
> - CDEBUG(D_INFO, "LPU64=%s, LPD64=%s, LPX64=%s\n", "%llu", "%lld", 
> "%#llx");
> + CDEBUG(D_INFO, "LPU64=%s, LPD64=%s, LPX64=%s\n", "%llu", "%lld",
> +"%#llx");

No, that will break things.  Ignore string literals when dealing with
the 80 character limit.

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: fsl-mc: fix uninitialized variable use

[Dan Carpenter]

On Mon, Nov 27, 2017 at 09:31:21AM +, Laurentiu Tudor wrote:
> I did but it didn't show up on my version of powerpc toolchain (gcc 
> 4.8.1). Maybe it's too old ...
> 

Smatch has new uninitialized variable warnings these days.  In theory,
it should be smarter and more strict than GCC.

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [bug report] staging: lustre: remove unnecessary NULL checks in kernel_comm.c

[Dan Carpenter]

Sorry for the bad subject line.

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/2] staging: unisys: visorbus: address theoretical int overflows

[Dan Carpenter]

> --- a/drivers/staging/unisys/visorbus/visorchipset.c
> +++ b/drivers/staging/unisys/visorbus/visorchipset.c
> @@ -581,7 +581,8 @@ static void *parser_name_get(struct parser_context *ctx)
>   struct visor_controlvm_parameters_header *phdr;
>  
>   phdr = >data;
> - if (phdr->name_offset + phdr->name_length > ctx->param_bytes)
> + if ((unsigned long)phdr->name_offset +
> + (unsigned long)phdr->name_length > ctx->param_bytes)
>   return NULL;
>   ctx->curr = (char *) + phdr->name_offset;
>   ctx->bytes_remaining = phdr->name_length;


I haven't reviewed this code very thouroughly.  This should fix the
issue on 64 bit systems, but it's a no-op on 32 bit systems.  Which
might be fine?  I would be more comfortable if we just checked for
integer overflow explicitly.  There are bunch of ways to do that:

if (phdr->name_offset > ctx->param_bytes ||
phdr->name_length > ctx->param_bytes ||
phdr->name_offset + phdr->name_length > ctx->param_bytes)

Or:

if (phdr->name_offset + phdr->name_length < phdr->name_offset ||
phdr->name_offset + phdr->name_length > ctx->param_bytes)
return NULL;

regards,
dan carpenter


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2] staging: fsl-mc: move bus driver out of staging

[Laurentiu Tudor]

On 11/28/2017 02:59 PM, Greg KH wrote:
> On Mon, Nov 27, 2017 at 03:32:28PM +, Laurentiu Tudor wrote:
>>
>>
>> On 11/03/2017 05:17 PM, Greg KH wrote:
>>> On Thu, Aug 31, 2017 at 06:04:30PM +0200, Greg KH wrote:
 On Mon, Aug 28, 2017 at 01:54:05PM +0300, laurentiu.tu...@nxp.com wrote:
> From: Stuart Yoder 
>
> Move the source files out of staging into their final locations:
> -include files in drivers/staging/fsl-mc/include go to 
> include/linux/fsl
> -irq-gic-v3-its-fsl-mc-msi.c goes to drivers/irqchip
> -source in drivers/staging/fsl-mc/bus goes to drivers/bus/fsl-mc
> -README.txt, providing and overview of DPAA goes to
>  Documentation/dpaa2/overview.txt
>
> Update or delete other remaining staging files-- Makefile, Kconfig, TODO.
> Update dpaa2_eth and dpio staging drivers.
>
> Signed-off-by: Stuart Yoder 
> Signed-off-by: Laurentiu Tudor 
> [Laurentiu: rebased, add dpaa2_eth and dpio #include updates]
> Cc: Thomas Gleixner 
> Cc: Jason Cooper 
> Cc: Marc Zyngier 

 This is going to have to wait until I get a chunk of time to do the
 review.  Probably after 4.13-final is out.
>>>
>>> Ok, review comments as I go through the code:
>>> mc-sys.c 323 EXPORT_SYMBOL(mc_send_command);
>>>
>>> should be EXPORT_SYMBOL_GPL(fsl_mc_send_command); to match up with your
>>> other exports and global namespace, right?
>>>
>>> You export a lot of dpcon_* symbols that no one uses, please do not do
>>> that.  Verify that all of them are actually used right now, if not,
>>> remove them.  If you think you are going to use them in the future,
>>> wonderful, add them in then.
>>>
>>> Same for your dpaa2_* exported symbols, most are not used from what I
>>> can see.
>>>
>>> struct dpaa2_io {
>>>   atomic_t refs;
>>>
>>> That's a kref, please use it instead of trying to roll your own.
>>>
>>> And even for this, your locking is not correct (i.e. you do not have
>>> any), that needs to be fixed so that teardown works correctly.
>>>
>>> You have a lot of WARN_ON() calls, that's going to be ignored and should
>>> all not be needed now that the code is debugged and working properly.
>>> Please remove them, or turn them into dev_err() calls with a real if ()
>>> check instead.
>>>
>>> You are checking "strings" for the type of device in a lot of places,
>>> like this:
>>> if (strcmp(obj_desc->type, "dprc") == 0) {
>>> why are you not just using the built-in driver model .type field and
>>> comparing that to the different type structures?  It's much easier, and
>>> you don't have to again, "roll your own".  See the USB or Greybus code
>>> for examples of busses that have different "types" of devices on them at
>>> the same time.
>>>
>>> Ok, that's enough for now, please work on this, and we can go from
>>> there...
>>>
>>
>> What would the next steps be, now that the patches are in staging-next?
>> Are there plans for a new round of review?
>
> Send a patch that moves the files you think should be moved at this
> point in time, as I'm not quite sure which ones exactly you feel are
> ready to go.

The initial plan was to strictly move the things needed by this bus 
driver, thus leaving some source files at a later time. To the point, 
these files:
   "drivers/staging/fsl-mc/bus/dpbp*.*"
   "drivers/staging/fsl-mc/bus/dpcon*.*"
and this whole dir:
   "drivers/staging/fsl-mc/bus/dpio/*".

I'll prepare the patch so that it's more visible what files are to be moved.

---
Thanks & Best Regards, Laurentiu
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2] Staging: sm750fb: Fix coding style issue in ddk750_sii164.c

[Jeremy Lacomis]

On Tue, Nov 28, 2017 at 02:14:11PM +0100, Greg Kroah-Hartman wrote:
> On Sat, Nov 25, 2017 at 08:04:52PM -0500, Jeremy Lacomis wrote:
> > This is a patch to the ddk750_sii164.c file that fixes line length warnings
> > found by the checkpatch.pl script
> 
> That's not all this patch did :(
>

Sorry again, I'll be sure to be much more careful in the future.

Thanks,
- Jeremy
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2] staging: sm750b: Fix coding style issues in sm750_accel.c

[Jeremy Lacomis]

On Tue, Nov 28, 2017 at 02:13:31PM +0100, Greg Kroah-Hartman wrote:
> On Sat, Nov 25, 2017 at 05:48:11PM -0500, Jeremy Lacomis wrote:
> > This is a patch to sm750_accel.c that fixes 80-character line length
> > warnings found by checkpatch.pl. It also fixes some grammatical errors
> > in comments and moves parameter-specific comments from inline to before
> > the function.
> > 
> > Signed-off-by: Jeremy Lacomis 
> > ---
> > Changes in v2:
> > - Change function comments to the kernel-doc format
> > 
> >  drivers/staging/sm750fb/sm750_accel.c | 189 
> > ++
> >  1 file changed, 103 insertions(+), 86 deletions(-)
> > 
> > diff --git a/drivers/staging/sm750fb/sm750_accel.c 
> > b/drivers/staging/sm750fb/sm750_accel.c
> > index 1035e91e7cd3..42cd920111bf 100644
> > --- a/drivers/staging/sm750fb/sm750_accel.c
> > +++ b/drivers/staging/sm750fb/sm750_accel.c
> > @@ -1,4 +1,4 @@
> > -// SPDX-License-Identifier: GPL-2.0
> > +/* SPDX-License-Identifier: GPL-2.0 */
> 
> Why did you change this line?  That is not correct.

Sorry about this Greg, I wasn't familiar with the format of the license
identifier line, and it seemed inconsistent with the rest of the comments at the
time.

> Only ever do one "logical" thing per patch.  If you have to say "also"
> in a changelog text, that's a huge hint you are doing something wrong.

I'll keep this in mind. I think my natural inclination is to be afraid that the
patches I'm sending in are _too_ trivial and breaking up these things into
several different patches would create more work for the maintainers.

Thanks,
- Jeremy
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[bug report] staging: lustre: remove unnecessary NULL checks in kernel_comm.c

[Dan Carpenter]

Hi Lustre devs,

Smatch generates this warning:

drivers/staging/lustre/lustre/obdclass/kernelcomm.c:146 
libcfs_kkuc_group_rem()
error: buffer overflow 'kkuc_groups' 3 <= u32max

drivers/staging/lustre/lustre/obdclass/kernelcomm.c
   142  int libcfs_kkuc_group_rem(int uid, unsigned int group)
   143  {
   144  struct kkuc_reg *reg, *next;
   145  
   146  if (!kkuc_groups[group].next)
 ^
Basically the complaint is that Smatch thinks "group" comes from the
user and hasn't been checked at all.

   147  return 0;
   148  
   149  if (!uid) {
   150  /* Broadcast a shutdown message */
   151  struct kuc_hdr lh;
   152  

Where "group" comes from is lmv_iocontrol() in lmv_obd.c:

drivers/staging/lustre/lustre/lmv/lmv_obd.c
  1078  case LL_IOC_HSM_CT_START: {
  1079  struct lustre_kernelcomm *lk = karg;
  ^
  1080  
  1081  if (lk->lk_flags & LK_FLG_STOP)
  1082  rc = lmv_hsm_ct_unregister(lmv, cmd, len, lk, 
uarg);
  ^^
We take "karg" from somewhere get lk->lk_group here.

  1083  else
  1084  rc = lmv_hsm_ct_register(lmv, cmd, len, lk, 
uarg);
  1085  break;
  1086  }

The problem is that, in Lustre, instead of functions we just have
ioctls.  Sometimes lmv_iocontrol() is called with validated data and
sometimes it's called with untrusted data.  Often with these kinds of
things I could just print the call tree to see where it's set to user
data and audit those places, but in Lustre the ioctls just call
themselves recursively so the call tree doesn't help.

And what I hate the most is obd_iocontrol().  Everything goes through
obd_iocontrol()...  It would be hard to design something which makes the
code more opaque than obd_iocontrol().

The bottom line, is I have no idea if this warning is correct or not.

regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v6 1/2] staging: ion: reorder include

[Greg KH]

On Tue, Nov 28, 2017 at 02:34:00PM +0100, Benjamin Gaignard wrote:
> 2017-11-28 14:20 GMT+01:00 Greg KH :
> > On Mon, Nov 06, 2017 at 04:59:44PM +0100, Benjamin Gaignard wrote:
> >> Put include in alphabetic order
> >
> > Why???
> 
> Mainly because the next patch in the series adds new includes and I have
> decide to split clean-up and new feature patches

That's fine, but this really isn't needed for any type of "clean-up" at
all.  But oh well, if you all like it that way, I'm not going to
complain that much :)

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v6 1/2] staging: ion: reorder include

[Benjamin Gaignard]

2017-11-28 14:20 GMT+01:00 Greg KH :
> On Mon, Nov 06, 2017 at 04:59:44PM +0100, Benjamin Gaignard wrote:
>> Put include in alphabetic order
>
> Why???

Mainly because the next patch in the series adds new includes and I have
decide to split clean-up and new feature patches

>
> That should not matter at all.
>
> I'll take this, but really, ick ick ick ick ick.
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v6 2/2] staging: ion: create one device entry per heap

[Greg KH]

On Mon, Nov 06, 2017 at 04:59:45PM +0100, Benjamin Gaignard wrote:
> Instead a getting only one common device "/dev/ion" for
> all the heaps this patch allow to create one device
> entry ("/dev/ionX") per heap.
> Getting an entry per heap could allow to set security rules
> per heap and global ones for all heaps.
> 
> Allocation requests will be only allowed if the mask_id
> match with device minor.
> Query request could be done on any of the devices.
> 
> Signed-off-by: Benjamin Gaignard 
> ---
>  drivers/staging/android/TODO|  1 -
>  drivers/staging/android/ion/Kconfig |  7 
>  drivers/staging/android/ion/ion-ioctl.c | 18 --
>  drivers/staging/android/ion/ion.c   | 62 
> +
>  drivers/staging/android/ion/ion.h   | 15 ++--
>  5 files changed, 91 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/staging/android/TODO b/drivers/staging/android/TODO
> index 687e0ea..8a11931 100644
> --- a/drivers/staging/android/TODO
> +++ b/drivers/staging/android/TODO
> @@ -8,7 +8,6 @@ TODO:
>  ion/
>   - Add dt-bindings for remaining heaps (chunk and carveout heaps). This would
> involve putting appropriate bindings in a memory node for Ion to find.
> - - Split /dev/ion up into multiple nodes (e.g. /dev/ion/heap0)
>   - Better test framework (integration with VGEM was suggested)
>  
>  Please send patches to Greg Kroah-Hartman  and Cc:
> diff --git a/drivers/staging/android/ion/Kconfig 
> b/drivers/staging/android/ion/Kconfig
> index a517b2d..cb4666e 100644
> --- a/drivers/staging/android/ion/Kconfig
> +++ b/drivers/staging/android/ion/Kconfig
> @@ -10,6 +10,13 @@ menuconfig ION
> If you're not using Android its probably safe to
> say N here.
>  
> +config ION_LEGACY_DEVICE_API
> + bool "Keep using Ion legacy misc device API"
> + depends on ION

You want to default to Y here, so when you do 'make oldconfig' nothing
breaks, right?

> + help
> +   Choose this option to keep using Ion legacy misc device API
> +   i.e. /dev/ion

You need more text here to describe the trade offs.  Why would I not
want to keep doing this?  What does turning this off get me?  What does
keeping it on keep me from doing?

> +
>  config ION_SYSTEM_HEAP
>   bool "Ion system heap"
>   depends on ION
> diff --git a/drivers/staging/android/ion/ion-ioctl.c 
> b/drivers/staging/android/ion/ion-ioctl.c
> index e26b786..bb5c77b 100644
> --- a/drivers/staging/android/ion/ion-ioctl.c
> +++ b/drivers/staging/android/ion/ion-ioctl.c
> @@ -25,7 +25,8 @@ union ion_ioctl_arg {
>   struct ion_heap_query query;
>  };
>  
> -static int validate_ioctl_arg(unsigned int cmd, union ion_ioctl_arg *arg)
> +static int validate_ioctl_arg(struct file *filp,
> +   unsigned int cmd, union ion_ioctl_arg *arg)
>  {
>   switch (cmd) {
>   case ION_IOC_HEAP_QUERY:
> @@ -34,6 +35,19 @@ static int validate_ioctl_arg(unsigned int cmd, union 
> ion_ioctl_arg *arg)
>   arg->query.reserved2 )
>   return -EINVAL;
>   break;
> +
> + case ION_IOC_ALLOC:
> + {
> + int mask = 1 << iminor(filp->f_inode);
> +
> +#ifdef CONFIG_ION_LEGACY_DEVICE_API
> + if (imajor(filp->f_inode) == MISC_MAJOR)
> + return 0;

Why return 0?  Because it is already allocated?

Some comments here as to exactly what you are doing would be nice.

> +#endif
> + if (!(arg->allocation.heap_id_mask & mask))

This doesn't allocate anthing, just check to see if it is?

> + return -EINVAL;
> + break;
> + }
>   default:
>   break;
>   }
> @@ -69,7 +83,7 @@ long ion_ioctl(struct file *filp, unsigned int cmd, 
> unsigned long arg)
>   if (copy_from_user(, (void __user *)arg, _IOC_SIZE(cmd)))
>   return -EFAULT;
>  
> - ret = validate_ioctl_arg(cmd, );
> + ret = validate_ioctl_arg(filp, cmd, );
>   if (WARN_ON_ONCE(ret))
>   return ret;
>  
> diff --git a/drivers/staging/android/ion/ion.c 
> b/drivers/staging/android/ion/ion.c
> index fda9756..2c2568b 100644
> --- a/drivers/staging/android/ion/ion.c
> +++ b/drivers/staging/android/ion/ion.c
> @@ -40,6 +40,9 @@
>  
>  #include "ion.h"
>  
> +#define ION_DEV_MAX 32

Why 32?  Where did that number come from?

> +#define ION_NAME "ion"
> +
>  static struct ion_device *internal_dev;
>  static int heap_id;
>  
> @@ -535,15 +538,38 @@ static int debug_shrink_get(void *data, u64 *val)
>  DEFINE_SIMPLE_ATTRIBUTE(debug_shrink_fops, debug_shrink_get,
>   debug_shrink_set, "%llu\n");
>  
> -void ion_device_add_heap(struct ion_heap *heap)
> +static struct device ion_bus = {
> + .init_name = ION_NAME,
> +};

Oh look, a struct device on the stack.  Watch bad things happen :(

This is a dynamic device, make it dynamic, or else you had better know
exactly what you 

Re: [PATCH] staging: irda: annotate irlan_seq_start() and irlan_seq_stop() for sparse

[Andrii]

On 11/28/2017 3:05 PM, Greg KH wrote:

On Tue, Nov 28, 2017 at 02:45:25PM +0200, Andrii Vladyka wrote:

Annotate rcu_read_lock in irlan_seq_start() and rcu_read_unlock in
irlan_seq_stop() for sparse

Signed-off-by: Andrii Vladyka >

Something went really wrong here with your email address :(


diff --git a/drivers/staging/irda/net/irlan/irlan_common.c 
b/drivers/staging/irda/net/irlan/irlan_common.c
index fdcd714..832df11 100644
--- a/drivers/staging/irda/net/irlan/irlan_common.c
+++ b/drivers/staging/irda/net/irlan/irlan_common.c

Did you read drivers/staging/irda/TODO?

What a shame... Thanks.

thanks,

greg k-h


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 0/4] hv_balloon: fixes for num_pages_onlined accounting and misc improvements

[Vitaly Kuznetsov]

Vitaly Kuznetsov  writes:

> While doing routing code review I noticed that commit 6df8d9aaf3af
> ("Drivers: hv: balloon: Correctly update onlined page count") introduced
> an issue with num_pages_onlined accounting on memory offlining. Deeper look
> showed that the accounting was always buggy. This is fixed in PATCH3.
> PATCHes 1 and 2 are preparatory cleanups, PATCH4 adds a tracepoint to
> post_status so it's now possible to see what's being sent to the host and
> where the data comes from.
>

K. Y., Alex,

did you have a chance to take a look?

Thanks,

-- 
  Vitaly
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 4/4] staging: most: Remove a attribute group from a kobject

[Greg KH]

On Sat, Nov 25, 2017 at 12:09:31PM +0530, Arvind Yadav wrote:
> All attribute group created during dim2_sysfs_probe() should be removed
> in dim2_sysfs_destroy().
> 
> Signed-off-by: Arvind Yadav 
> ---
>  drivers/staging/most/hdm-dim2/dim2_sysfs.c | 1 +

This file is not in the tree anymore.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v6 1/2] staging: ion: reorder include

[Greg KH]

On Mon, Nov 06, 2017 at 04:59:44PM +0100, Benjamin Gaignard wrote:
> Put include in alphabetic order

Why???

That should not matter at all.

I'll take this, but really, ick ick ick ick ick.

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging/media: lirc: style fix - replace hard-coded function names

[Greg KH]

On Sun, Nov 26, 2017 at 08:49:42PM +0100, Martin Homuth wrote:
> This patch fixes the remaining coding style warnings in the lirc module.
> 
> It fixes the following checkpatch.pl warning:
> 
> WARNING: Prefer using '"%s...", __func__' to using 'read', this
> function's name, in a string

> >From f11f24667ba6696cb71ac33a67fc0c7d3b4cd542 Mon Sep 17 00:00:00 2001
> From: Martin Homuth 
> Date: Sun, 26 Nov 2017 20:14:33 +0100
> Subject: [PATCH] lirc: style fix - replace hard-coded function names
> 
> Instead of hard coding the function name the __func__ variable
> should be used.
> 
> Signed-off-by: Martin Homuth 
> ---
>  drivers/staging/media/lirc/lirc_zilog.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/media/lirc/lirc_zilog.c 
> b/drivers/staging/media/lirc/lirc_zilog.c
> index 6bd0717bf76e..be68ee652071 100644
> --- a/drivers/staging/media/lirc/lirc_zilog.c
> +++ b/drivers/staging/media/lirc/lirc_zilog.c
> @@ -888,9 +888,9 @@ static ssize_t read(struct file *filep, char __user 
> *outbuf, size_t n,
>   unsigned int m;
>   DECLARE_WAITQUEUE(wait, current);
>  
> - dev_dbg(ir->dev, "read called\n");
> + dev_dbg(ir->dev, "%s called\n", __func__);
>   if (n % rbuf->chunk_size) {
> - dev_dbg(ir->dev, "read result = -EINVAL\n");
> + dev_dbg(ir->dev, "%s result = -EINVAL\n", __func__);
>   return -EINVAL;
>   }
>  
> @@ -949,7 +949,7 @@ static ssize_t read(struct file *filep, char __user 
> *outbuf, size_t n,
>   retries++;
>   }
>   if (retries >= 5) {
> - dev_err(ir->dev, "Buffer read failed!\n");
> + dev_err(ir->dev, "%s failed!\n", __func__);
>   ret = -EIO;
>   }
>   }
> @@ -959,7 +959,7 @@ static ssize_t read(struct file *filep, char __user 
> *outbuf, size_t n,
>   put_ir_rx(rx, false);
>   set_current_state(TASK_RUNNING);
>  
> - dev_dbg(ir->dev, "read result = %d (%s)\n", ret,
> + dev_dbg(ir->dev, "%s result = %d (%s)\n", __func__, ret,
>   ret ? "Error" : "OK");
>  
>   return ret ? ret : written;
> -- 
> 2.13.6
> 

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- Your patch was attached, please place it inline so that it can be
  applied directly from the email message itself.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH V2] staging: rtl8188eu: Fix incorrect response to SIOCGIWESSID

[Greg KH]

On Sat, Nov 25, 2017 at 01:32:38PM -0600, Larry Finger wrote:
> When not associated with an AP, wifi device drivers should respond to the
> SIOCGIWESSID ioctl with a zero-length string for the SSID, which is the
> behavior expected by dhcpcd.
> 
> Currently, this driver returns an error code (-1) from the ioctl call,
> which causes dhcpcd to assume that the device is not a wireless interface
> and therefore it fails to work correctly with it thereafter.
> 
> This problem was reported and tested at
> https://github.com/lwfinger/rtl8188eu/issues/234.
> 
> Signed-off-by: Larry Finger 
> ---
> 
> v2 - completed missing subject

Ah, nevermind, you already did...
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: rtl8188eu:

[Greg KH]

On Sat, Nov 25, 2017 at 11:59:51AM -0600, Larry Finger wrote:
> When not associated with an AP, wifi device drivers should respond to the
> SIOCGIWESSID ioctl with a zero-length string for the SSID, which is the
> behavior expected by dhcpcd.
> 
> Currently, this driver returns an error code (-1) from the ioctl call,
> which causes dhcpcd to assume that the device is not a wireless interface
> and therefore it fails to work correctly with it thereafter.
> 
> This problem was reported and tested at
> https://github.com/lwfinger/rtl8188eu/issues/234.
> 
> Signed-off-by: Larry Finger 
> ---
>  drivers/staging/rtl8188eu/os_dep/ioctl_linux.c | 14 --
>  1 file changed, 4 insertions(+), 10 deletions(-)

Your subject line looks broken :(

Please fix and resend.

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2] Staging: sm750fb: Fix coding style issue in ddk750_sii164.c

[Greg Kroah-Hartman]

On Sat, Nov 25, 2017 at 08:04:52PM -0500, Jeremy Lacomis wrote:
> This is a patch to the ddk750_sii164.c file that fixes line length warnings
> found by the checkpatch.pl script

That's not all this patch did :(

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2] staging: sm750b: Fix coding style issues in sm750_accel.c

[Greg Kroah-Hartman]

On Sat, Nov 25, 2017 at 05:48:11PM -0500, Jeremy Lacomis wrote:
> This is a patch to sm750_accel.c that fixes 80-character line length
> warnings found by checkpatch.pl. It also fixes some grammatical errors
> in comments and moves parameter-specific comments from inline to before
> the function.
> 
> Signed-off-by: Jeremy Lacomis 
> ---
> Changes in v2:
> - Change function comments to the kernel-doc format
> 
>  drivers/staging/sm750fb/sm750_accel.c | 189 
> ++
>  1 file changed, 103 insertions(+), 86 deletions(-)
> 
> diff --git a/drivers/staging/sm750fb/sm750_accel.c 
> b/drivers/staging/sm750fb/sm750_accel.c
> index 1035e91e7cd3..42cd920111bf 100644
> --- a/drivers/staging/sm750fb/sm750_accel.c
> +++ b/drivers/staging/sm750fb/sm750_accel.c
> @@ -1,4 +1,4 @@
> -// SPDX-License-Identifier: GPL-2.0
> +/* SPDX-License-Identifier: GPL-2.0 */

Why did you change this line?  That is not correct.

Only ever do one "logical" thing per patch.  If you have to say "also"
in a changelog text, that's a huge hint you are doing something wrong.

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 0/6] staging: pi433: pi433_if.c: coding style fixes

[Greg KH]

On Wed, Nov 15, 2017 at 04:35:47PM -0800, Chase Metzger wrote:
> Fixes some of the coding style violations to comply with the kernel
> coding style.
> 
> Chase Metzger (6):
>   staging: pi433: pi433_if.c: fix opening curly brace coding style
> issues
>   staging: pi433: pi433_if.c: fix else if/else statements coding style
> issues
>   staging: pi433: pi433_if.c: fix trailing statement coding style issues
>   staging: pi433: pi433_if.c: fix space prohibited coding style issues
>   staging: pi433: pi433_if.c: fix space(s) required coding style issues
>   staging: pi433: pi433_if.c: use tabs instead of spaces
> 
>  drivers/staging/pi433/pi433_if.c | 203 
> +--
>  1 file changed, 87 insertions(+), 116 deletions(-)

Please rebase and resend this series based on the review comments.

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 4/6] staging: pi433: pi433_if.c: fix space prohibited coding style issues

[Greg KH]

On Wed, Nov 15, 2017 at 04:35:51PM -0800, Chase Metzger wrote:
> Remove unnecessary spaces before and after parenthesis' to comply
> with the kernel coding style.
> 
> Signed-off-by: Chase Metzger 
> ---
>  drivers/staging/pi433/pi433_if.c | 34 +-
>  1 file changed, 17 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/staging/pi433/pi433_if.c 
> b/drivers/staging/pi433/pi433_if.c
> index 3c5d8a05e976..ad89504655af 100644
> --- a/drivers/staging/pi433/pi433_if.c
> +++ b/drivers/staging/pi433/pi433_if.c
> @@ -342,7 +342,7 @@ pi433_receive(void *data)
>   return retval;
>  
>   /* now check RSSI, if low wait for getting high (RSSI interrupt) */
> - while ( !rf69_get_flag(dev->spi, rssiExceededThreshold) ) {
> + while (!rf69_get_flag(dev->spi, rssiExceededThreshold)) {
>   /* allow tx to interrupt us while waiting for high RSSI */
>   dev->interrupt_rx_allowed = true;
>   wake_up_interruptible(>tx_wait_queue);
> @@ -413,7 +413,7 @@ pi433_receive(void *data)
>  
>   /* get payload */
>   while (dev->rx_position < bytes_total) {
> - if ( !rf69_get_flag(dev->spi, payloadReady) ) {
> + if (!rf69_get_flag(dev->spi, payloadReady)) {
>   retval = wait_event_interruptible(dev->fifo_wait_queue,
> dev->free_in_fifo < 
> FIFO_SIZE);
>   if (retval) /* wait was interrupted */
> @@ -473,9 +473,9 @@ pi433_tx_thread(void *data)
>   /* wait for fifo to be populated or for request to terminate*/
>   dev_dbg(device->dev, "thread: going to wait for new messages");
>   wait_event_interruptible(device->tx_wait_queue,
> -  ( !kfifo_is_empty(>tx_fifo) ||
> - kthread_should_stop() ));
> - if ( kthread_should_stop() )
> +  (!kfifo_is_empty(>tx_fifo) ||
> + kthread_should_stop()));

Indent this properly too if you are touching it.

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/6] staging: pi433: pi433_if.c: fix opening curly brace coding style issues

[Greg KH]

On Wed, Nov 15, 2017 at 04:35:48PM -0800, Chase Metzger wrote:
> Put opening curly braces for if/else/else if statements and for/while loops
> on the same line as the statements or loops to comply with the kernel
> coding style.
> 
> Signed-off-by: Chase Metzger 
> ---
>  drivers/staging/pi433/pi433_if.c | 110 
> ++-
>  1 file changed, 38 insertions(+), 72 deletions(-)
> 
> diff --git a/drivers/staging/pi433/pi433_if.c 
> b/drivers/staging/pi433/pi433_if.c
> index ff86db1c6cbd..76d70345229a 100644
> --- a/drivers/staging/pi433/pi433_if.c
> +++ b/drivers/staging/pi433/pi433_if.c
> @@ -153,12 +153,10 @@ static irqreturn_t DIO1_irq_handler(int irq, void 
> *dev_id)
>  {
>   struct pi433_device *device = dev_id;
>  
> - if  (device->irq_state[DIO1] == DIO_FifoNotEmpty_DIO1)
> - {
> + if  (device->irq_state[DIO1] == DIO_FifoNotEmpty_DIO1) {

Again, fix up the space issue here at the same time please.

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: irda: annotate irlan_seq_start() and irlan_seq_stop() for sparse

[Greg KH]

On Tue, Nov 28, 2017 at 02:45:25PM +0200, Andrii Vladyka wrote:
> Annotate rcu_read_lock in irlan_seq_start() and rcu_read_unlock in
> irlan_seq_stop() for sparse
> 
> Signed-off-by: Andrii Vladyka  >

Something went really wrong here with your email address :(

> diff --git a/drivers/staging/irda/net/irlan/irlan_common.c 
> b/drivers/staging/irda/net/irlan/irlan_common.c
> index fdcd714..832df11 100644
> --- a/drivers/staging/irda/net/irlan/irlan_common.c
> +++ b/drivers/staging/irda/net/irlan/irlan_common.c

Did you read drivers/staging/irda/TODO?

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: pi433: pi433_if.c: fix if/else if and brace coding style issues

[Greg KH]

On Tue, Nov 14, 2017 at 12:03:51PM -0800, Chase Metzger wrote:
> Fix if/else if and braces in function DIO0_irq_handler to comply with
> the kernel coding style.
> 
> Signed-off-by: Chase Metzger 
> ---
>  drivers/staging/pi433/pi433_if.c | 11 +++
>  1 file changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/staging/pi433/pi433_if.c 
> b/drivers/staging/pi433/pi433_if.c
> index d946838450d4..dee70c47629c 100644
> --- a/drivers/staging/pi433/pi433_if.c
> +++ b/drivers/staging/pi433/pi433_if.c
> @@ -133,19 +133,14 @@ static irqreturn_t DIO0_irq_handler(int irq, void 
> *dev_id)
>  {
>   struct pi433_device *device = dev_id;
>  
> - if  (device->irq_state[DIO0] == DIO_PacketSent)
> - {
> + if  (device->irq_state[DIO0] == DIO_PacketSent) {

When you are touching the line, be sure to fix the obvious space issue
here as well.

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2] staging: fsl-mc: move bus driver out of staging

[Greg KH]

On Mon, Nov 27, 2017 at 03:32:28PM +, Laurentiu Tudor wrote:
> 
> 
> On 11/03/2017 05:17 PM, Greg KH wrote:
> > On Thu, Aug 31, 2017 at 06:04:30PM +0200, Greg KH wrote:
> >> On Mon, Aug 28, 2017 at 01:54:05PM +0300, laurentiu.tu...@nxp.com wrote:
> >>> From: Stuart Yoder 
> >>>
> >>> Move the source files out of staging into their final locations:
> >>>-include files in drivers/staging/fsl-mc/include go to 
> >>> include/linux/fsl
> >>>-irq-gic-v3-its-fsl-mc-msi.c goes to drivers/irqchip
> >>>-source in drivers/staging/fsl-mc/bus goes to drivers/bus/fsl-mc
> >>>-README.txt, providing and overview of DPAA goes to
> >>> Documentation/dpaa2/overview.txt
> >>>
> >>> Update or delete other remaining staging files-- Makefile, Kconfig, TODO.
> >>> Update dpaa2_eth and dpio staging drivers.
> >>>
> >>> Signed-off-by: Stuart Yoder 
> >>> Signed-off-by: Laurentiu Tudor 
> >>> [Laurentiu: rebased, add dpaa2_eth and dpio #include updates]
> >>> Cc: Thomas Gleixner 
> >>> Cc: Jason Cooper 
> >>> Cc: Marc Zyngier 
> >>
> >> This is going to have to wait until I get a chunk of time to do the
> >> review.  Probably after 4.13-final is out.
> >
> > Ok, review comments as I go through the code:
> > mc-sys.c 323 EXPORT_SYMBOL(mc_send_command);
> >
> > should be EXPORT_SYMBOL_GPL(fsl_mc_send_command); to match up with your
> > other exports and global namespace, right?
> >
> > You export a lot of dpcon_* symbols that no one uses, please do not do
> > that.  Verify that all of them are actually used right now, if not,
> > remove them.  If you think you are going to use them in the future,
> > wonderful, add them in then.
> >
> > Same for your dpaa2_* exported symbols, most are not used from what I
> > can see.
> >
> > struct dpaa2_io {
> >  atomic_t refs;
> >
> > That's a kref, please use it instead of trying to roll your own.
> >
> > And even for this, your locking is not correct (i.e. you do not have
> > any), that needs to be fixed so that teardown works correctly.
> >
> > You have a lot of WARN_ON() calls, that's going to be ignored and should
> > all not be needed now that the code is debugged and working properly.
> > Please remove them, or turn them into dev_err() calls with a real if ()
> > check instead.
> >
> > You are checking "strings" for the type of device in a lot of places,
> > like this:
> > if (strcmp(obj_desc->type, "dprc") == 0) {
> > why are you not just using the built-in driver model .type field and
> > comparing that to the different type structures?  It's much easier, and
> > you don't have to again, "roll your own".  See the USB or Greybus code
> > for examples of busses that have different "types" of devices on them at
> > the same time.
> >
> > Ok, that's enough for now, please work on this, and we can go from
> > there...
> >
> 
> What would the next steps be, now that the patches are in staging-next?
> Are there plans for a new round of review?

Send a patch that moves the files you think should be moved at this
point in time, as I'm not quite sure which ones exactly you feel are
ready to go.

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/2] staging: sm750fb: Fix coding style in ddk750_sii164.h

[Greg KH]

On Mon, Nov 27, 2017 at 09:37:27AM -0500, Jeremy Lacomis wrote:
> On Mon, Nov 27, 2017 at 11:45:00AM +0100, Geert Uytterhoeven wrote:
> > On Mon, Nov 27, 2017 at 10:46 AM, Dan Carpenter
> >  wrote:
> > > On Sat, Nov 25, 2017 at 01:26:35PM -0500, Jeremy Lacomis wrote:
> > >> This patch to ddk750_sii164.h fixes line length warnings found by the
> > >> checkpatch.pl script and reformats comments uniformly.
> > >>
> > >> Signed-off-by: Jeremy Lacomis 
> > >> ---
> > >>  drivers/staging/sm750fb/ddk750_sii164.h | 57 
> > >> +
> > >>  1 file changed, 22 insertions(+), 35 deletions(-)
> > >>
> > >> diff --git a/drivers/staging/sm750fb/ddk750_sii164.h 
> > >> b/drivers/staging/sm750fb/ddk750_sii164.h
> > >> index 2e9a88cd6af3..393a3c5be3ae 100644
> > >> --- a/drivers/staging/sm750fb/ddk750_sii164.h
> > >> +++ b/drivers/staging/sm750fb/ddk750_sii164.h
> > >> @@ -4,15 +4,20 @@
> > >>
> > >>  #define USE_DVICHIP
> > >>
> > >> -/* Hot Plug detection mode structure */
> > >> +/*
> > >> + * Hot Plug detection mode structure:
> > >> + *  Disable Hot Plug output bit (always high).
> > >> + *  Use Monitor Detect Interrupt bit.
> > >> + *  Use Receiver Sense detect bit.
> > >> + *  Use Hot Plug detect bit.
> > >> + */
> > >>  enum sii164_hot_plug_mode {
> > >> - SII164_HOTPLUG_DISABLE = 0, /* Disable Hot Plug output bit 
> > >> (always high). */
> > >> - SII164_HOTPLUG_USE_MDI, /* Use Monitor Detect 
> > >> Interrupt bit. */
> > >> - SII164_HOTPLUG_USE_RSEN,/* Use Receiver Sense detect 
> > >> bit. */
> > >> - SII164_HOTPLUG_USE_HTPLG/* Use Hot Plug detect bit. */
> > >> + SII164_HOTPLUG_DISABLE = 0,
> > >> + SII164_HOTPLUG_USE_MDI,
> > >> + SII164_HOTPLUG_USE_RSEN,
> > >> + SII164_HOTPLUG_USE_HTPLG
> > >>  };
> > >
> > > I feel like this makes it less readable.  The original was better.
> > 
> > And more error prone.
> 
> Thanks for taking the time to review my patch, I'm just getting into kernel
> development and am trying to do as little disservice as possible while 
> learning
> the process.
> 
> For something like this, would using the kernel-doc format make more sense, or
> should the original inline comments be kept? I found the original difficult to
> read because my terminal was just small enough that this didn't fit.

kernel-doc might be good, but it also might just be worth it to leave it
alone as it might be too much work.

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: irda: annotate irlan_seq_start() and irlan_seq_stop() for sparse

[Andrii Vladyka]

Annotate rcu_read_lock in irlan_seq_start() and rcu_read_unlock in 
irlan_seq_stop() for sparse


Signed-off-by: Andrii Vladyka >


diff --git a/drivers/staging/irda/net/irlan/irlan_common.c 
b/drivers/staging/irda/net/irlan/irlan_common.c
index fdcd714..832df11 100644
--- a/drivers/staging/irda/net/irlan/irlan_common.c
+++ b/drivers/staging/irda/net/irlan/irlan_common.c
@@ -1090,6 +1090,7 @@ int irlan_extract_param(__u8 *buf, char *name, char 
*value, __u16 *len)
  * or NULL if end of file
  */
 static void *irlan_seq_start(struct seq_file *seq, loff_t *pos)
+   __acquires(RCU)
 {
rcu_read_lock();
return seq_list_start_head(, *pos);
@@ -1103,6 +1104,7 @@ static void *irlan_seq_next(struct seq_file *seq, void 
*v, loff_t *pos)
 
 /* End of reading /proc file */
 static void irlan_seq_stop(struct seq_file *seq, void *v)
+   __releases(RCU)
 {
rcu_read_unlock();
 }
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: most: Fix build errors

[Greg Kroah-Hartman]

On Tue, Nov 28, 2017 at 12:12:47PM +, Chris Coffey wrote:
> On Tue, Nov 28, 2017 at 08:26:07AM +0100, Greg Kroah-Hartman wrote:
> > On Mon, Nov 27, 2017 at 09:12:39PM +, Chris Coffey wrote:
> > > This patch fixes build errors that occur when trying to build the
> > > MOST modules. For example, the cdev module build fails with:
> > > 
> > > make[3]: *** No rule to make target 
> > > 'drivers/staging/most/aim-cdev/cdev.o', needed by 
> > > 'drivers/staging/most/aim-cdev/aim_cdev.o'.  Stop.
> > > scripts/Makefile.build:569: recipe for target 
> > > 'drivers/staging/most/aim-cdev' failed
> > > make[2]: *** [drivers/staging/most/aim-cdev] Error 2
> > > scripts/Makefile.build:569: recipe for target 'drivers/staging/most' 
> > > failed
> > > make[1]: *** [drivers/staging/most] Error 2
> > > Makefile:1502: recipe for target '_module_drivers/staging' failed
> > > make: *** [_module_drivers/staging] Error 2
> > 
> > How are you seeing this error?  What is the build command line that you
> > provided to do this?  It should just "not build" right now at all.
> > 
> 
> I did a git pull to get the latest staging updates, enabled a bunch of
> staging drivers, and then ran a build on the staging modules with 'make
> modules M=drivers/staging'. Unfortunately now I'm unable to reproduce
> the error (of course).

If you run 'make' it would have fixed up the dependancies and not caused
a build error.  That makes sense how you saw it, but it's not there for
anyone else so all is fine.

> > > Update the Makefile variables and paths with the new directory
> > > structure to fix these errors.
> > > 
> > > Signed-off-by: Chris Coffey 
> > > ---
> > >  drivers/staging/most/Makefile | 14 +++---
> > >  1 file changed, 7 insertions(+), 7 deletions(-)
> > 
> > I think a patch was sent yesterday to fix these up, did you see it in
> > the archives?
> 
> I checked the archives before making the patch, and didn't see anything
> MOST-related in there for the past few days. The patch below made the 
> build succeed and generate all of the MOST modules.

Subject: [PATCH 1/3] staging: most: fix Makefile
is the patch sent yesterday that should resolve this.  I'm going to go
apply it right now...

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: most: Fix build errors

[Chris Coffey]

On Tue, Nov 28, 2017 at 08:26:07AM +0100, Greg Kroah-Hartman wrote:
> On Mon, Nov 27, 2017 at 09:12:39PM +, Chris Coffey wrote:
> > This patch fixes build errors that occur when trying to build the
> > MOST modules. For example, the cdev module build fails with:
> > 
> > make[3]: *** No rule to make target 'drivers/staging/most/aim-cdev/cdev.o', 
> > needed by 'drivers/staging/most/aim-cdev/aim_cdev.o'.  Stop.
> > scripts/Makefile.build:569: recipe for target 
> > 'drivers/staging/most/aim-cdev' failed
> > make[2]: *** [drivers/staging/most/aim-cdev] Error 2
> > scripts/Makefile.build:569: recipe for target 'drivers/staging/most' failed
> > make[1]: *** [drivers/staging/most] Error 2
> > Makefile:1502: recipe for target '_module_drivers/staging' failed
> > make: *** [_module_drivers/staging] Error 2
> 
> How are you seeing this error?  What is the build command line that you
> provided to do this?  It should just "not build" right now at all.
> 

I did a git pull to get the latest staging updates, enabled a bunch of
staging drivers, and then ran a build on the staging modules with 'make
modules M=drivers/staging'. Unfortunately now I'm unable to reproduce
the error (of course).

> > Update the Makefile variables and paths with the new directory
> > structure to fix these errors.
> > 
> > Signed-off-by: Chris Coffey 
> > ---
> >  drivers/staging/most/Makefile | 14 +++---
> >  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> I think a patch was sent yesterday to fix these up, did you see it in
> the archives?

I checked the archives before making the patch, and didn't see anything
MOST-related in there for the past few days. The patch below made the 
build succeed and generate all of the MOST modules.

> 
> thanks,
> 
> greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3 2/2] media: staging: atomisp: fixes for "symbol was not declared. Should it be static?" sparse warnings.

[Jeremy Sowden]

Defined some const arrays as static since they don't need external linkage.

Signed-off-by: Jeremy Sowden 
---
 .../isp/kernels/eed1_8/ia_css_eed1_8.host.c| 24 +++---
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/isp/kernels/eed1_8/ia_css_eed1_8.host.c
 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/isp/kernels/eed1_8/ia_css_eed1_8.host.c
index 682f8b709ff9..47bb5042381b 100644
--- 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/isp/kernels/eed1_8/ia_css_eed1_8.host.c
+++ 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/isp/kernels/eed1_8/ia_css_eed1_8.host.c
@@ -32,44 +32,44 @@
 #define NUMBER_OF_TCINV_POINTS 9
 #define NUMBER_OF_FCINV_POINTS 9
 
-const int16_t chgrinv_x[NUMBER_OF_CHGRINV_POINTS] = {
+static const int16_t chgrinv_x[NUMBER_OF_CHGRINV_POINTS] = {
 0, 16, 64, 144, 272, 448, 672, 976,
 1376, 1888, 2528, 3312, 4256, 5376, 6688};
 
-const int16_t chgrinv_a[NUMBER_OF_CHGRINV_POINTS] = {
+static const int16_t chgrinv_a[NUMBER_OF_CHGRINV_POINTS] = {
 -7171, -256, -29, -3456, -1071, -475, -189, -102,
 -48, -38, -10, -9, -7, -6, 0};
 
-const int16_t chgrinv_b[NUMBER_OF_CHGRINV_POINTS] = {
+static const int16_t chgrinv_b[NUMBER_OF_CHGRINV_POINTS] = {
 8191, 1021, 256, 114, 60, 37, 24, 17,
 12, 9, 6, 5, 4, 3, 2};
 
-const int16_t chgrinv_c[NUMBER_OF_CHGRINV_POINTS] = {
+static const int16_t chgrinv_c[NUMBER_OF_CHGRINV_POINTS] = {
 1, 1, 1, 0, 0, 0, 0, 0,
 0, 0, 0, 0, 0, 0, 0};
 
-const int16_t tcinv_x[NUMBER_OF_TCINV_POINTS] = {
+static const int16_t tcinv_x[NUMBER_OF_TCINV_POINTS] = {
 0, 4, 11, 23, 42, 68, 102, 148, 205};
 
-const int16_t tcinv_a[NUMBER_OF_TCINV_POINTS] = {
+static const int16_t tcinv_a[NUMBER_OF_TCINV_POINTS] = {
 -6364, -631, -126, -34, -13, -6, -4452, -2156, 0};
 
-const int16_t tcinv_b[NUMBER_OF_TCINV_POINTS] = {
+static const int16_t tcinv_b[NUMBER_OF_TCINV_POINTS] = {
 8191, 1828, 726, 352, 197, 121, 80, 55, 40};
 
-const int16_t tcinv_c[NUMBER_OF_TCINV_POINTS] = {
+static const int16_t tcinv_c[NUMBER_OF_TCINV_POINTS] = {
 1, 1, 1, 1, 1, 1, 0, 0, 0};
 
-const int16_t fcinv_x[NUMBER_OF_FCINV_POINTS] = {
+static const int16_t fcinv_x[NUMBER_OF_FCINV_POINTS] = {
 0, 80, 216, 456, 824, 1344, 2040, 2952, 4096};
 
-const int16_t fcinv_a[NUMBER_OF_FCINV_POINTS] = {
+static const int16_t fcinv_a[NUMBER_OF_FCINV_POINTS] = {
 -5244, -486, -86, -2849, -961, -400, -180, -86, 0};
 
-const int16_t fcinv_b[NUMBER_OF_FCINV_POINTS] = {
+static const int16_t fcinv_b[NUMBER_OF_FCINV_POINTS] = {
 8191, 1637, 607, 287, 159, 98, 64, 44, 32};
 
-const int16_t fcinv_c[NUMBER_OF_FCINV_POINTS] = {
+static const int16_t fcinv_c[NUMBER_OF_FCINV_POINTS] = {
 1, 1, 1, 0, 0, 0, 0, 0, 0};
 
 
-- 
2.15.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3 1/2] media: staging: atomisp: fix for sparse "using plain integer as NULL pointer" warnings.

[Jeremy Sowden]

The "address" member of struct ia_css_host_data is a pointer-to-char, so define 
default as NULL.

Signed-off-by: Jeremy Sowden 
---
 .../css2400/runtime/isp_param/interface/ia_css_isp_param_types.h| 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/isp_param/interface/ia_css_isp_param_types.h
 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/isp_param/interface/ia_css_isp_param_types.h
index 8e651b80345a..6fee3f7fd184 100644
--- 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/isp_param/interface/ia_css_isp_param_types.h
+++ 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/isp_param/interface/ia_css_isp_param_types.h
@@ -95,7 +95,7 @@ union ia_css_all_memory_offsets {
 };
 
 #define IA_CSS_DEFAULT_ISP_MEM_PARAMS \
-   { { { { 0, 0 } } } }
+   { { { { NULL, 0 } } } }
 
 #define IA_CSS_DEFAULT_ISP_CSS_PARAMS \
{ { { { 0, 0 } } } }
-- 
2.15.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 0/3] Sparse fixes for the Atom ISP Staging Driver

[Jeremy Sowden]

Fixed some sparse warnings in the Atom ISP staging driver.

This time with longer commit messages. :)

I've chosen to ignore checkpatch.pl's suggestion to change the types of
the arrays in the second patch from int16_t to s16.

Jeremy Sowden (2):
  media: staging: atomisp: fix for sparse "using plain integer as NULL
pointer" warnings.
  media: staging: atomisp: fixes for "symbol was not declared. Should it
be static?" sparse warnings.

 .../isp/kernels/eed1_8/ia_css_eed1_8.host.c| 24 +++---
 .../isp_param/interface/ia_css_isp_param_types.h   |  2 +-
 2 files changed, 13 insertions(+), 13 deletions(-)


base-commit: 844056fd74ebdd826bd23a7d989597e15f478acb
-- 
2.15.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3] staging: rtl8188eu: Fix private WEXT IOCTL calls

[kbuild test robot]

Hi Ishraq,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on staging/staging-testing]
[also build test WARNING on v4.15-rc1 next-20171127]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/ishraq-i-ashraf-gmail-com/staging-rtl8188eu-Fix-private-WEXT-IOCTL-calls/20171128-130403


coccinelle warnings: (new ones prefixed by >>)

>> drivers/staging/rtl8188eu/os_dep/ioctl_linux.c:3379:36-43: WARNING: kzalloc 
>> should be used for pwep, instead of kmalloc/memset

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: rtl8188eu: fix kzalloc-simple.cocci warnings

[kbuild test robot]

drivers/staging/rtl8188eu/os_dep/ioctl_linux.c:3379:36-43: WARNING: kzalloc 
should be used for pwep, instead of kmalloc/memset


 Use kzalloc rather than kmalloc followed by memset with 0

 This considers some simple cases that are common and easy to validate
 Note in particular that there are no ...s in the rule, so all of the
 matched code has to be contiguous

Generated by: scripts/coccinelle/api/alloc/kzalloc-simple.cocci

Fixes: f1ac2c75e0c6 ("staging: rtl8188eu: Fix private WEXT IOCTL calls")
CC: Ishraq Ibne Ashraf 
Signed-off-by: Fengguang Wu 
---

 ioctl_linux.c |3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

--- a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
+++ b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
@@ -3376,14 +3376,13 @@ static int rtw_set_encryption_pvt(struct
wep_key_len = wep_key_len <= 5 ? 5 : 13;
wep_total_len = wep_key_len + offsetof(struct 
ndis_802_11_wep, KeyMaterial);
 
-   pwep = (struct ndis_802_11_wep *)kmalloc(wep_total_len, 
GFP_KERNEL);
+   pwep = kzalloc(wep_total_len, GFP_KERNEL);
if (!pwep) {
DBG_88E(" r871x_set_encryption: pwep allocate 
fail !!!\n");
ret = -ENOMEM;
goto err_free_param;
}
 
-   memset(pwep, 0, wep_total_len);
pwep->KeyLength = wep_key_len;
pwep->Length = wep_total_len;
}
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel