All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4] clk: renesas: cpg-mssr: add R8A7745 support
From: Sergei Shtylyov @ 2016-11-08 21:25 UTC (permalink / raw)
  To: mturquette, linux-clk, sboyd, robh+dt, mark.rutland, devicetree
  Cc: linux-renesas-soc, horms+renesas
In-Reply-To: <3539878.XFtRJDB9iq@wasted.cogentembedded.com>

Add RZ/G1E (R8A7745) Clock  Pulse Generator / Module Standby and Software
Reset support, using the CPG/MSSR driver core and the common R-Car Gen2
(and RZ/G) code.

Based on the proof-of-concept R8A7791 CPG/MSSR patch by Geert Uytterhoeven
<geert+renesas@glider.be>.

Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

---
This patch  is against the 'clk-next' branch of CLK group's 'linux.git' repo
plus the R8A7743 clock driver patch. It depends on the common R-Car gen2 (and
RZ/G) support just posted.

Changes in version 4:
- changed the Z2 clock's divisor to 1;
- passed  the PLL0 divisor to rcar_gen2_cpg_init();
- renamed the ACP clock to CPEX;
- removed the thermal module clock.

Changes in version 3:
- removed the FDP1-1 module clock;
- added Geert's tag.

Changes in version 2:
- changed the Z2 clock's divisor to 3.

 Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt |    5 
 drivers/clk/renesas/Kconfig                                  |    1 
 drivers/clk/renesas/Makefile                                 |    1 
 drivers/clk/renesas/r8a7745-cpg-mssr.c                       |  259 +++++++++++
 drivers/clk/renesas/renesas-cpg-mssr.c                       |    6 
 drivers/clk/renesas/renesas-cpg-mssr.h                       |    1 
 6 files changed, 271 insertions(+), 2 deletions(-)

Index: linux/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt
===================================================================
--- linux.orig/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt
+++ linux/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt
@@ -14,6 +14,7 @@ They provide the following functionaliti
 Required Properties:
   - compatible: Must be one of:
       - "renesas,r8a7743-cpg-mssr" for the r8a7743 SoC (RZ/G1M)
+      - "renesas,r8a7745-cpg-mssr" for the r8a7745 SoC (RZ/G1E)
       - "renesas,r8a7795-cpg-mssr" for the r8a7795 SoC (R-Car H3)
       - "renesas,r8a7796-cpg-mssr" for the r8a7796 SoC (R-Car M3-W)
 
@@ -23,8 +24,8 @@ Required Properties:
   - clocks: References to external parent clocks, one entry for each entry in
     clock-names
   - clock-names: List of external parent clock names. Valid names are:
-      - "extal" (r8a7743, r8a7795, r8a7796)
-      - "usb_extal" (r8a7743)
+      - "extal" (r8a7743, r8a7745, r8a7795, r8a7796)
+      - "usb_extal" (r8a7743, r8a7745)
       - "extalr" (r8a7795, r8a7796)
 
   - #clock-cells: Must be 2
Index: linux/drivers/clk/renesas/Kconfig
===================================================================
--- linux.orig/drivers/clk/renesas/Kconfig
+++ linux/drivers/clk/renesas/Kconfig
@@ -1,6 +1,7 @@
 config CLK_RENESAS_CPG_MSSR
 	bool
 	default y if ARCH_R8A7743
+	default y if ARCH_R8A7745
 	default y if ARCH_R8A7795
 	default y if ARCH_R8A7796
 
Index: linux/drivers/clk/renesas/Makefile
===================================================================
--- linux.orig/drivers/clk/renesas/Makefile
+++ linux/drivers/clk/renesas/Makefile
@@ -3,6 +3,7 @@ obj-$(CONFIG_ARCH_R7S72100)		+= clk-rz.o
 obj-$(CONFIG_ARCH_R8A73A4)		+= clk-r8a73a4.o clk-div6.o
 obj-$(CONFIG_ARCH_R8A7740)		+= clk-r8a7740.o clk-div6.o
 obj-$(CONFIG_ARCH_R8A7743)		+= r8a7743-cpg-mssr.o rcar-gen2-cpg.o
+obj-$(CONFIG_ARCH_R8A7745)		+= r8a7745-cpg-mssr.o rcar-gen2-cpg.o
 obj-$(CONFIG_ARCH_R8A7778)		+= clk-r8a7778.o
 obj-$(CONFIG_ARCH_R8A7779)		+= clk-r8a7779.o
 obj-$(CONFIG_ARCH_R8A7790)		+= clk-rcar-gen2.o clk-div6.o
Index: linux/drivers/clk/renesas/r8a7745-cpg-mssr.c
===================================================================
--- /dev/null
+++ linux/drivers/clk/renesas/r8a7745-cpg-mssr.c
@@ -0,0 +1,259 @@
+/*
+ * r8a7745 Clock Pulse Generator / Module Standby and Software Reset
+ *
+ * Copyright (C) 2016 Cogent Embedded Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published
+ * by the Free Software Foundation; of the License.
+ */
+
+#include <linux/device.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/soc/renesas/rcar-rst.h>
+
+#include <dt-bindings/clock/r8a7745-cpg-mssr.h>
+
+#include "renesas-cpg-mssr.h"
+#include "rcar-gen2-cpg.h"
+
+enum clk_ids {
+	/* Core Clock Outputs exported to DT */
+	LAST_DT_CORE_CLK = R8A7745_CLK_OSC,
+
+	/* External Input Clocks */
+	CLK_EXTAL,
+	CLK_USB_EXTAL,
+
+	/* Internal Core Clocks */
+	CLK_MAIN,
+	CLK_PLL0,
+	CLK_PLL1,
+	CLK_PLL3,
+	CLK_PLL1_DIV2,
+
+	/* Module Clocks */
+	MOD_CLK_BASE
+};
+
+static const struct cpg_core_clk r8a7745_core_clks[] __initconst = {
+	/* External Clock Inputs */
+	DEF_INPUT("extal",	CLK_EXTAL),
+	DEF_INPUT("usb_extal",	CLK_USB_EXTAL),
+
+	/* Internal Core Clocks */
+	DEF_BASE(".main",	CLK_MAIN, CLK_TYPE_GEN2_MAIN, CLK_EXTAL),
+	DEF_BASE(".pll0",	CLK_PLL0, CLK_TYPE_GEN2_PLL0, CLK_MAIN),
+	DEF_BASE(".pll1",	CLK_PLL1, CLK_TYPE_GEN2_PLL1, CLK_MAIN),
+	DEF_BASE(".pll3",	CLK_PLL3, CLK_TYPE_GEN2_PLL3, CLK_MAIN),
+
+	DEF_FIXED(".pll1_div2", CLK_PLL1_DIV2, CLK_PLL1, 2, 1),
+
+	/* Core Clock Outputs */
+	DEF_BASE("lb",	 R8A7745_CLK_LB,   CLK_TYPE_GEN2_LB,   CLK_PLL1),
+	DEF_BASE("sdh",  R8A7745_CLK_SDH,  CLK_TYPE_GEN2_SDH,  CLK_PLL1),
+	DEF_BASE("sd0",  R8A7745_CLK_SD0,  CLK_TYPE_GEN2_SD0,  CLK_PLL1),
+	DEF_BASE("qspi", R8A7745_CLK_QSPI, CLK_TYPE_GEN2_QSPI, CLK_PLL1_DIV2),
+	DEF_BASE("rcan", R8A7745_CLK_RCAN, CLK_TYPE_GEN2_RCAN, CLK_USB_EXTAL),
+
+	DEF_FIXED("z2",     R8A7745_CLK_Z2,    CLK_PLL0,	  1, 1),
+	DEF_FIXED("zg",     R8A7745_CLK_ZG,    CLK_PLL1,          6, 1),
+	DEF_FIXED("zx",     R8A7745_CLK_ZX,    CLK_PLL1,          3, 1),
+	DEF_FIXED("zs",     R8A7745_CLK_ZS,    CLK_PLL1,          6, 1),
+	DEF_FIXED("hp",     R8A7745_CLK_HP,    CLK_PLL1,         12, 1),
+	DEF_FIXED("b",      R8A7745_CLK_B,     CLK_PLL1,         12, 1),
+	DEF_FIXED("p",      R8A7745_CLK_P,     CLK_PLL1,         24, 1),
+	DEF_FIXED("cl",     R8A7745_CLK_CL,    CLK_PLL1,         48, 1),
+	DEF_FIXED("cp",     R8A7745_CLK_CP,    CLK_PLL1,         48, 1),
+	DEF_FIXED("m2",     R8A7745_CLK_M2,    CLK_PLL1,          8, 1),
+	DEF_FIXED("zb3",    R8A7745_CLK_ZB3,   CLK_PLL3,          4, 1),
+	DEF_FIXED("zb3d2",  R8A7745_CLK_ZB3D2, CLK_PLL3,          8, 1),
+	DEF_FIXED("ddr",    R8A7745_CLK_DDR,   CLK_PLL3,          8, 1),
+	DEF_FIXED("mp",     R8A7745_CLK_MP,    CLK_PLL1_DIV2,    15, 1),
+	DEF_FIXED("cpex",   R8A7745_CLK_CPEX,  CLK_EXTAL,         2, 1),
+	DEF_FIXED("r",      R8A7745_CLK_R,     CLK_PLL1,      49152, 1),
+	DEF_FIXED("osc",    R8A7745_CLK_OSC,   CLK_PLL1,      12288, 1),
+
+	DEF_DIV6P1("sd2",   R8A7745_CLK_SD2,   CLK_PLL1_DIV2, 0x078),
+	DEF_DIV6P1("sd3",   R8A7745_CLK_SD3,   CLK_PLL1_DIV2, 0x26c),
+	DEF_DIV6P1("mmc0",  R8A7745_CLK_MMC0,  CLK_PLL1_DIV2, 0x240),
+};
+
+static const struct mssr_mod_clk r8a7745_mod_clks[] __initconst = {
+	DEF_MOD("msiof0",	    0,	R8A7745_CLK_MP),
+	DEF_MOD("vcp0",		  101,	R8A7745_CLK_ZS),
+	DEF_MOD("vpc0",		  103,	R8A7745_CLK_ZS),
+	DEF_MOD("tmu1",		  111,	R8A7745_CLK_P),
+	DEF_MOD("3dg",		  112,	R8A7745_CLK_ZG),
+	DEF_MOD("2d-dmac",	  115,	R8A7745_CLK_ZS),
+	DEF_MOD("fdp1-0",	  119,	R8A7745_CLK_ZS),
+	DEF_MOD("tmu3",		  121,	R8A7745_CLK_P),
+	DEF_MOD("tmu2",		  122,	R8A7745_CLK_P),
+	DEF_MOD("cmt0",		  124,	R8A7745_CLK_R),
+	DEF_MOD("tmu0",		  125,	R8A7745_CLK_CP),
+	DEF_MOD("vsp1du0",	  128,	R8A7745_CLK_ZS),
+	DEF_MOD("vsp1-sy",	  131,	R8A7745_CLK_ZS),
+	DEF_MOD("scifa2",	  202,	R8A7745_CLK_MP),
+	DEF_MOD("scifa1",	  203,	R8A7745_CLK_MP),
+	DEF_MOD("scifa0",	  204,	R8A7745_CLK_MP),
+	DEF_MOD("msiof2",	  205,	R8A7745_CLK_MP),
+	DEF_MOD("scifb0",	  206,	R8A7745_CLK_MP),
+	DEF_MOD("scifb1",	  207,	R8A7745_CLK_MP),
+	DEF_MOD("msiof1",	  208,	R8A7745_CLK_MP),
+	DEF_MOD("scifb2",	  216,	R8A7745_CLK_MP),
+	DEF_MOD("sys-dmac1",	  218,	R8A7745_CLK_ZS),
+	DEF_MOD("sys-dmac0",	  219,	R8A7745_CLK_ZS),
+	DEF_MOD("tpu0",		  304,	R8A7745_CLK_CP),
+	DEF_MOD("sdhi3",	  311,	R8A7745_CLK_SD3),
+	DEF_MOD("sdhi2",	  312,	R8A7745_CLK_SD2),
+	DEF_MOD("sdhi0",	  314,	R8A7745_CLK_SD0),
+	DEF_MOD("mmcif0",	  315,	R8A7745_CLK_MMC0),
+	DEF_MOD("iic0",		  318,	R8A7745_CLK_HP),
+	DEF_MOD("iic1",		  323,	R8A7745_CLK_HP),
+	DEF_MOD("cmt1",		  329,	R8A7745_CLK_R),
+	DEF_MOD("usbhs-dmac0",	  330,	R8A7745_CLK_HP),
+	DEF_MOD("usbhs-dmac1",	  331,	R8A7745_CLK_HP),
+	DEF_MOD("irqc",		  407,	R8A7745_CLK_CP),
+	DEF_MOD("intc-sys",	  408,	R8A7745_CLK_ZS),
+	DEF_MOD("audio-dmac0",	  502,	R8A7745_CLK_HP),
+	DEF_MOD("pwm",		  523,	R8A7745_CLK_P),
+	DEF_MOD("usb-ehci",	  703,	R8A7745_CLK_MP),
+	DEF_MOD("usbhs",	  704,	R8A7745_CLK_HP),
+	DEF_MOD("hscif2",	  713,	R8A7745_CLK_ZS),
+	DEF_MOD("scif5",	  714,	R8A7745_CLK_P),
+	DEF_MOD("scif4",	  715,	R8A7745_CLK_P),
+	DEF_MOD("hscif1",	  716,	R8A7745_CLK_ZS),
+	DEF_MOD("hscif0",	  717,	R8A7745_CLK_ZS),
+	DEF_MOD("scif3",	  718,	R8A7745_CLK_P),
+	DEF_MOD("scif2",	  719,	R8A7745_CLK_P),
+	DEF_MOD("scif1",	  720,	R8A7745_CLK_P),
+	DEF_MOD("scif0",	  721,	R8A7745_CLK_P),
+	DEF_MOD("du0",		  724,	R8A7745_CLK_ZX),
+	DEF_MOD("ipmmu-sgx",	  800,	R8A7745_CLK_ZX),
+	DEF_MOD("vin1",		  810,	R8A7745_CLK_ZG),
+	DEF_MOD("vin0",		  811,	R8A7745_CLK_ZG),
+	DEF_MOD("etheravb",	  812,	R8A7745_CLK_HP),
+	DEF_MOD("ether",	  813,	R8A7745_CLK_P),
+	DEF_MOD("gpio6",	  905,	R8A7745_CLK_CP),
+	DEF_MOD("gpio5",	  907,	R8A7745_CLK_CP),
+	DEF_MOD("gpio4",	  908,	R8A7745_CLK_CP),
+	DEF_MOD("gpio3",	  909,	R8A7745_CLK_CP),
+	DEF_MOD("gpio2",	  910,	R8A7745_CLK_CP),
+	DEF_MOD("gpio1",	  911,	R8A7745_CLK_CP),
+	DEF_MOD("gpio0",	  912,	R8A7745_CLK_CP),
+	DEF_MOD("can1",		  915,	R8A7745_CLK_P),
+	DEF_MOD("can0",		  916,	R8A7745_CLK_P),
+	DEF_MOD("qspi_mod",	  917,	R8A7745_CLK_QSPI),
+	DEF_MOD("i2c5",		  925,	R8A7745_CLK_HP),
+	DEF_MOD("i2c4",		  927,	R8A7745_CLK_HP),
+	DEF_MOD("i2c3",		  928,	R8A7745_CLK_HP),
+	DEF_MOD("i2c2",		  929,	R8A7745_CLK_HP),
+	DEF_MOD("i2c1",		  930,	R8A7745_CLK_HP),
+	DEF_MOD("i2c0",		  931,	R8A7745_CLK_HP),
+	DEF_MOD("ssi-all",	 1005,	R8A7745_CLK_P),
+	DEF_MOD("ssi9",		 1006,	MOD_CLK_ID(1005)),
+	DEF_MOD("ssi8",		 1007,	MOD_CLK_ID(1005)),
+	DEF_MOD("ssi7",		 1008,	MOD_CLK_ID(1005)),
+	DEF_MOD("ssi6",		 1009,	MOD_CLK_ID(1005)),
+	DEF_MOD("ssi5",		 1010,	MOD_CLK_ID(1005)),
+	DEF_MOD("ssi4",		 1011,	MOD_CLK_ID(1005)),
+	DEF_MOD("ssi3",		 1012,	MOD_CLK_ID(1005)),
+	DEF_MOD("ssi2",		 1013,	MOD_CLK_ID(1005)),
+	DEF_MOD("ssi1",		 1014,	MOD_CLK_ID(1005)),
+	DEF_MOD("ssi0",		 1015,	MOD_CLK_ID(1005)),
+	DEF_MOD("scu-all",	 1017,	R8A7745_CLK_P),
+	DEF_MOD("scu-dvc1",	 1018,	MOD_CLK_ID(1017)),
+	DEF_MOD("scu-dvc0",	 1019,	MOD_CLK_ID(1017)),
+	DEF_MOD("scu-ctu1-mix1", 1020,	MOD_CLK_ID(1017)),
+	DEF_MOD("scu-ctu0-mix0", 1021,	MOD_CLK_ID(1017)),
+	DEF_MOD("scu-src9",	 1022,	MOD_CLK_ID(1017)),
+	DEF_MOD("scu-src8",	 1023,	MOD_CLK_ID(1017)),
+	DEF_MOD("scu-src7",	 1024,	MOD_CLK_ID(1017)),
+	DEF_MOD("scu-src6",	 1025,	MOD_CLK_ID(1017)),
+	DEF_MOD("scu-src5",	 1026,	MOD_CLK_ID(1017)),
+	DEF_MOD("scu-src4",	 1027,	MOD_CLK_ID(1017)),
+	DEF_MOD("scu-src3",	 1028,	MOD_CLK_ID(1017)),
+	DEF_MOD("scu-src2",	 1029,	MOD_CLK_ID(1017)),
+	DEF_MOD("scu-src1",	 1030,	MOD_CLK_ID(1017)),
+	DEF_MOD("scu-src0",	 1031,	MOD_CLK_ID(1017)),
+	DEF_MOD("scifa3",	 1106,	R8A7745_CLK_MP),
+	DEF_MOD("scifa4",	 1107,	R8A7745_CLK_MP),
+	DEF_MOD("scifa5",	 1108,	R8A7745_CLK_MP),
+};
+
+static const unsigned int r8a7745_crit_mod_clks[] __initconst = {
+	MOD_CLK_ID(408),	/* INTC-SYS (GIC) */
+};
+
+/*
+ * CPG Clock Data
+ */
+
+/*
+ *    MD	EXTAL		PLL0	PLL1	PLL3
+ * 14 13 19	(MHz)		*1	*2
+ *---------------------------------------------------
+ * 0  0  0	15		x200/3	x208/2	x106
+ * 0  0  1	15		x200/3	x208/2	x88
+ * 0  1  0	20		x150/3	x156/2	x80
+ * 0  1  1	20		x150/3	x156/2	x66
+ * 1  0  0	26 / 2		x230/3	x240/2	x122
+ * 1  0  1	26 / 2		x230/3	x240/2	x102
+ * 1  1  0	30 / 2		x200/3	x208/2	x106
+ * 1  1  1	30 / 2		x200/3	x208/2	x88
+ *
+ * *1 :	Table 7.5b indicates VCO output (PLL0 = VCO/3)
+ * *2 :	Table 7.5b indicates VCO output (PLL1 = VCO/2)
+ */
+#define CPG_PLL_CONFIG_INDEX(md)	((((md) & BIT(14)) >> 12) | \
+					 (((md) & BIT(13)) >> 12) | \
+					 (((md) & BIT(19)) >> 19))
+
+static const struct rcar_gen2_cpg_pll_config cpg_pll_configs[8] __initconst = {
+	/* EXTAL div	PLL1 mult	PLL3 mult	PLL0 mult */
+	{ 1,		208,		106,		200	},
+	{ 1,		208,		88,		200	},
+	{ 1,		156,		80,		150	},
+	{ 1,		156,		66,		150	},
+	{ 2,		240,		122,		230	},
+	{ 2,		240,		102,		230	},
+	{ 2,		208,		106,		200	},
+	{ 2,		208,		88,		200	},
+};
+
+static int __init r8a7745_cpg_mssr_init(struct device *dev)
+{
+	const struct rcar_gen2_cpg_pll_config *cpg_pll_config;
+	u32 cpg_mode;
+	int error;
+
+	error = rcar_rst_read_mode_pins(&cpg_mode);
+	if (error)
+		return error;
+
+	cpg_pll_config = &cpg_pll_configs[CPG_PLL_CONFIG_INDEX(cpg_mode)];
+
+	return rcar_gen2_cpg_init(cpg_pll_config, 3);
+}
+
+const struct cpg_mssr_info r8a7745_cpg_mssr_info __initconst = {
+	/* Core Clocks */
+	.core_clks = r8a7745_core_clks,
+	.num_core_clks = ARRAY_SIZE(r8a7745_core_clks),
+	.last_dt_core_clk = LAST_DT_CORE_CLK,
+	.num_total_core_clks = MOD_CLK_BASE,
+
+	/* Module Clocks */
+	.mod_clks = r8a7745_mod_clks,
+	.num_mod_clks = ARRAY_SIZE(r8a7745_mod_clks),
+	.num_hw_mod_clks = 12 * 32,
+
+	/* Critical Module Clocks */
+	.crit_mod_clks = r8a7745_crit_mod_clks,
+	.num_crit_mod_clks = ARRAY_SIZE(r8a7745_crit_mod_clks),
+
+	/* Callbacks */
+	.init = r8a7745_cpg_mssr_init,
+	.cpg_clk_register = rcar_gen2_cpg_clk_register,
+};
Index: linux/drivers/clk/renesas/renesas-cpg-mssr.c
===================================================================
--- linux.orig/drivers/clk/renesas/renesas-cpg-mssr.c
+++ linux/drivers/clk/renesas/renesas-cpg-mssr.c
@@ -509,6 +509,12 @@ static const struct of_device_id cpg_mss
 		.data = &r8a7743_cpg_mssr_info,
 	},
 #endif
+#ifdef CONFIG_ARCH_R8A7745
+	{
+		.compatible = "renesas,r8a7745-cpg-mssr",
+		.data = &r8a7745_cpg_mssr_info,
+	},
+#endif
 #ifdef CONFIG_ARCH_R8A7795
 	{
 		.compatible = "renesas,r8a7795-cpg-mssr",
Index: linux/drivers/clk/renesas/renesas-cpg-mssr.h
===================================================================
--- linux.orig/drivers/clk/renesas/renesas-cpg-mssr.h
+++ linux/drivers/clk/renesas/renesas-cpg-mssr.h
@@ -131,6 +131,7 @@ struct cpg_mssr_info {
 };
 
 extern const struct cpg_mssr_info r8a7743_cpg_mssr_info;
+extern const struct cpg_mssr_info r8a7745_cpg_mssr_info;
 extern const struct cpg_mssr_info r8a7795_cpg_mssr_info;
 extern const struct cpg_mssr_info r8a7796_cpg_mssr_info;
 #endif

^ permalink raw reply

* Re: [PATCH net-next 1/2] bnxt_en: do not call napi_hash_add()
From: Michael Chan @ 2016-11-08 21:25 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, netdev
In-Reply-To: <1478632013.17367.23.camel@edumazet-glaptop3.roam.corp.google.com>

On Tue, Nov 8, 2016 at 11:06 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> From: Eric Dumazet <edumazet@google.com>
>
> This is automatically done from netif_napi_add(), and we want to not
> export napi_hash_add() anymore in the following patch.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Michael Chan <michael.chan@broadcom.com>

Acked-by: Michael Chan <michael.chan@broadcom.com>

^ permalink raw reply

* Re: [v2] drm/rockchip: Use an enum to identify Rockchip VOPs
From: Guenter Roeck @ 2016-11-08 21:06 UTC (permalink / raw)
  To: Kristian Høgsberg; +Cc: Kristian H. Kristensen, dri-devel
In-Reply-To: <1478202408-11635-1-git-send-email-hoegsberg@chromium.org>

On Thu, Nov 03, 2016 at 12:46:48PM -0700, Kristian Høgsberg wrote:
> We used to call drm_of_encoder_active_endpoint_id() from
> rockchip_dp_drm_encoder_atomic_check() to determine the endpoint for the
> active encoder. However, the encoder isn't necessarily active at this
> point or it may be connected to different crtc than what we're switching
> to. Instead, look at the crtc from the drm_crtc_state.  This fixes wrong
> colors when driving the eDP with the big VOP.  Further, we can identify
> the type of VOP we're dealing with by just putting a VOP id enum in the
> vop_data.
> 
> On way to test this is to use the modetest tool from libdrm:
> 
>   $ modetest -M rockchip -s 32@28:2400x1600
> 
> which displays dark or black colors because we fail to look up the
> endpoint and use default 0 (which is ROCKCHIP_OUT_MODE_P888) for big
> VOP instead of RGB10 as required.
> 
> For reference,
> 
>   $ modetest -M rockchip -s 32@25:2400x1600
> 
> drives the eDP from little VOP and displays correctly.
> 
> Signed-off-by: Kristian H. Kristensen <hoegsberg@chromium.org>
> ---
> v2: Stripped chromeos annotations, fix compile errors for drivers I didn't
> compile when I first wrote the patch.
> 

[ ... ]

>  
> +enum vop_id vop_get_crtc_vop_id(struct drm_crtc *crtc)
> +{
> +	struct vop *vop = to_vop(crtc);
> +
> +	return vop->data->id;
> +}

Missing EXPORT() causes build errors if calling code is built as module.

Example arm64:allmodconfig:

ERROR: "vop_get_crtc_vop_id" [drivers/gpu/drm/rockchip/dw-mipi-dsi.ko] undefined!
ERROR: "vop_get_crtc_vop_id" [drivers/gpu/drm/rockchip/cdn-dp.ko] undefined!
ERROR: "vop_get_crtc_vop_id" [drivers/gpu/drm/rockchip/analogix_dp-rockchip.ko] undefined!

Guenter
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* Re: Configure RAM size on iMX53 board
From: Sascha Hauer @ 2016-11-08 21:24 UTC (permalink / raw)
  To: Jose Luis Zabalza; +Cc: barebox
In-Reply-To: <CAKZffXG5CG9R8e4xoMdP6DFJq42ng5EQqwmhafikNn6HNzuedQ@mail.gmail.com>

On Tue, Nov 08, 2016 at 09:51:36PM +0100, Jose Luis Zabalza wrote:
> > So you have 512MiB on each chip select, so I assume that on the 512MiB
> > board variants CS1 is not equipped.
> 
> Yes, it is.
> 
> >In that case you can in lowlevel.c
> > test if you find SDRAM on CS1 and if not, disable the chip select
> > completely in the SDRAM controller.
> 
> OK. But how ? I enable CS0 and CS1 on DCD table. Is there any way to
> tell barebox not to use CS1 ?
> 
> > I am not sure how you can detect if there's SDRAM on CS1. I've seen
> > situations in which the board just hangs if you access non existent RAM
> > areas.
> 
> I have tried it, but I have not be able to implement a code for
> autodetect. If the code write or read a value on a position without
> physical  chip, the microcontroller hangs. ????
> 
> But it's not a problem. A solution is configure both CS and MMU. If
> bootloader don't access to high positions, there is not problem. After
> I set a environment variable with memory size and the mem kernel
> parameter (or ATAG) does the rest.

You said that both board variants work with the same U-Boot binary, how
does it work there? Is there some detection mechanism or is it only some
environment variable that you have to set manually?

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply

* Re: [PATCH] x86/MCE: Remove MCP_TIMESTAMP
From: Borislav Petkov @ 2016-11-08 21:24 UTC (permalink / raw)
  To: Thomas Gleixner; +Cc: Luck, Tony, linux-edac, X86 ML, LKML
In-Reply-To: <alpine.DEB.2.20.1611082212140.3501@nanos>

On Tue, Nov 08, 2016 at 10:14:04PM +0100, Thomas Gleixner wrote:
> And yes, you should spend the extra cycles. Adding a flags argument to
> mce_setup() and propagate it through the various callsites shouldn't be
> that hard and would make the stuff obvious instead of obfuscated.

Sure, that's already on my TODO. I want to take a look at it when I have
a quiet moment.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

^ permalink raw reply

* Re: [Qemu-devel] Crashing in tcp_close
From: Brian Candler @ 2016-11-08 21:22 UTC (permalink / raw)
  To: Stefan Hajnoczi; +Cc: Samuel Thibault, qemu-devel, Jan Kiszka
In-Reply-To: <20161107104245.GC5036@stefanha-x1.localdomain>

On 07/11/2016 10:42, Stefan Hajnoczi wrote:
> On Mon, Nov 07, 2016 at 08:42:17AM +0000, Brian Candler wrote:
>> >On 06/11/2016 18:04, Samuel Thibault wrote:
>>> > >Brian, could you run it with
>>> > >
>>> > >export MALLOC_CHECK_=2
>>> > >
>>> > >and also this could be useful:
>>> > >
>>> > >export MALLOC_PERTURB_=1234
>>> > >
>>> > >Also, to rule out the double-free scenario, and try to catch a buffer
>>> > >overflow coming from the socket structure itself, I have attached a
>>> > >patch which adds some debugging.
>> >
>> >Thanks. I've added the patch, and re-run the stress test.

Back to the original setup, I can still get dumps. I notice I'm now 
getting "malloc_printerr" in the backtrace, but unfortunately I don't 
get to see the actual error message. It would seem that the malloc_check 
is being done and finding an issue.  I haven't been able to get one in 
tcp_close again though :-(

Regards,

Brian.

[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
Core was generated by `/usr/local/bin/qemu-system-x86_64 -m 4G -machine 
type=pc,accel=kvm -device virt'.
Program terminated with signal SIGABRT, Aborted.
#0  0x00007eff4f3df428 in __GI_raise (sig=sig@entry=6) at 
../sysdeps/unix/sysv/linux/raise.c:54
54    ../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
[Current thread is 1 (Thread 0x7eff50dffa80 (LWP 13616))]
(gdb) bt
#0  0x00007eff4f3df428 in __GI_raise (sig=sig@entry=6) at 
../sysdeps/unix/sysv/linux/raise.c:54
#1  0x00007eff4f3e102a in __GI_abort () at abort.c:89
#2  0x00007eff4f42bc1f in malloc_printerr (ar_ptr=<optimised out>, 
ptr=<optimised out>, str=<optimised out>,
     action=<optimised out>) at malloc.c:5008
#3  _int_malloc (av=av@entry=0x7eff4f76db20 <main_arena>, 
bytes=bytes@entry=89) at malloc.c:3384
#4  0x00007eff4f42c409 in malloc_check (sz=88, caller=<optimised out>) 
at hooks.c:295
#5  0x00007eff50106729 in g_malloc () from 
/lib/x86_64-linux-gnu/libglib-2.0.so.0
#6  0x0000563ca16930cf in qemu_aio_get 
(aiocb_info=aiocb_info@entry=0x563ca1841190 <blk_aio_em_aiocb_info>,
     bs=0x563ca4132f40, cb=cb@entry=0x563ca14d85e0 <dma_blk_cb>, 
opaque=opaque@entry=0x563ca5b78910)
     at /home/nsrc/qemu-2.7.0/block/io.c:2231
#7  0x0000563ca1687aa8 in blk_aio_get (opaque=0x563ca5b78910, 
cb=0x563ca14d85e0 <dma_blk_cb>, blk=0x563ca4132d70,
     aiocb_info=0x563ca1841190 <blk_aio_em_aiocb_info>) at 
/home/nsrc/qemu-2.7.0/block/block-backend.c:1477
#8  blk_aio_prwv (blk=0x563ca4132d70, offset=5278244864, bytes=4096, 
qiov=0x563ca5b78968,
     co_entry=co_entry@entry=0x563ca16872b0 <blk_aio_write_entry>, 
flags=0, cb=0x563ca14d85e0 <dma_blk_cb>,
     opaque=0x563ca5b78910) at 
/home/nsrc/qemu-2.7.0/block/block-backend.c:941
#9  0x0000563ca1687bc0 in blk_aio_pwritev (blk=<optimised out>, 
offset=<optimised out>, qiov=<optimised out>,
     flags=<optimised out>, cb=<optimised out>, opaque=<optimised out>)
     at /home/nsrc/qemu-2.7.0/block/block-backend.c:1054
#10 0x0000563ca14d8718 in dma_blk_cb (opaque=0x563ca5b78910, 
ret=<optimised out>)
     at /home/nsrc/qemu-2.7.0/dma-helpers.c:167
#11 0x0000563ca14d8bf8 in dma_blk_io (ctx=0x563ca41184a0, 
sg=sg@entry=0x563ca59a08f0,
     offset=offset@entry=5278244864, 
io_func=io_func@entry=0x563ca15a58e0 <scsi_dma_writev>,
     io_func_opaque=io_func_opaque@entry=0x563ca5c8a350, 
cb=cb@entry=0x563ca15a7250 <scsi_dma_complete>,
     opaque=0x563ca5c8a350, dir=DMA_DIRECTION_TO_DEVICE) at 
/home/nsrc/qemu-2.7.0/dma-helpers.c:222
#12 0x0000563ca15a764e in scsi_write_data (req=0x563ca5c8a350) at 
/home/nsrc/qemu-2.7.0/hw/scsi/scsi-disk.c:540
#13 0x0000563ca15ac743 in scsi_req_continue (req=req@entry=0x563ca5c8a350)
     at /home/nsrc/qemu-2.7.0/hw/scsi/scsi-bus.c:1680
#14 0x0000563ca14381a2 in virtio_scsi_handle_cmd_req_submit 
(s=0x563ca5abc1d0, req=<optimised out>)
     at /home/nsrc/qemu-2.7.0/hw/scsi/virtio-scsi.c:565
#15 virtio_scsi_handle_cmd_vq (s=0x563ca5abc1d0, vq=0x7eff4963f110)
     at /home/nsrc/qemu-2.7.0/hw/scsi/virtio-scsi.c:583
#16 0x0000563ca144a0d6 in virtio_queue_notify_vq (vq=0x7eff4963f110)
---Type <return> to continue, or q <return> to quit---
     at /home/nsrc/qemu-2.7.0/hw/virtio/virtio.c:1113
#17 0x0000563ca1654965 in aio_dispatch (ctx=0x563ca41184a0) at 
/home/nsrc/qemu-2.7.0/aio-posix.c:330
#18 0x0000563ca164a3ae in aio_ctx_dispatch (source=<optimised out>, 
callback=<optimised out>,
     user_data=<optimised out>) at /home/nsrc/qemu-2.7.0/async.c:234
#19 0x00007eff501011a7 in g_main_context_dispatch () from 
/lib/x86_64-linux-gnu/libglib-2.0.so.0
#20 0x0000563ca16531db in glib_pollfds_poll () at 
/home/nsrc/qemu-2.7.0/main-loop.c:213
#21 os_host_main_loop_wait (timeout=<optimised out>) at 
/home/nsrc/qemu-2.7.0/main-loop.c:258
#22 main_loop_wait (nonblocking=<optimised out>) at 
/home/nsrc/qemu-2.7.0/main-loop.c:506
#23 0x0000563ca13be431 in main_loop () at /home/nsrc/qemu-2.7.0/vl.c:1908
#24 main (argc=<optimised out>, argv=<optimised out>, envp=<optimised 
out>) at /home/nsrc/qemu-2.7.0/vl.c:4604
(gdb)

^ permalink raw reply

* [PATCH v6] clk: renesas: cpg-mssr: add R8A7743 support
From: Sergei Shtylyov @ 2016-11-08 21:21 UTC (permalink / raw)
  To: mturquette, linux-clk, sboyd, robh+dt, mark.rutland, devicetree
  Cc: linux-renesas-soc, horms+renesas
In-Reply-To: <1737840.BeMyoUTlpl@wasted.cogentembedded.com>

Add RZ/G1M (R8A7743) Clock  Pulse Generator / Module Standby and Software
Reset support, using the CPG/MSSR driver core and the common R-Car Gen2
(and RZ/G) code.

Based on the proof-of-concept R8A7791 CPG/MSSR patch by Geert Uytterhoeven
<geert+renesas@glider.be>.

Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Acked-by: Rob Herring <robh@kernel.org>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

---
This patch is against the 'clk-next' branch of CLK group's 'linux.git' repo.
It depends on the common R-Car gen2 (and RZ/G) support just posted.

Changes in version 6:
- passed the PLL0 divisor to rcar_gen2_cpg_init();
- undid  the version 5 changes.

Changes in version 5:
- described the Z clock as PLL0/VCO divided by 2.

Changes in version 4:
- made use of the new RST  driver's API to read the mode pins;
- cleaned  up the table copied from the manual, fixed the manual reference.

Changes in version 3:
- exchanged  Audio-DMAC0/1  entries in r8a7743_mod_clks[];
- renamed the I2C6 entry to IICDVFS;
- fixed the comment to the INTC-SYS entry in r8a7743_crit_mod_clks[];
- removed the useless EXTAL divisor check from r8a7743_cpg_mssr_init();
- added Rob's ACK and Geert's tag.

Changes in version 2:
- documented the R8A7743 device tree binding;
- fixed whitespace errors/warnings reported by checkpatch.pl;
- used  tabs instead of spaces for indentation;
- fixed the PLL3 column name in the PLL table.

 Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt |    4 
 drivers/clk/renesas/Kconfig                                  |    1 
 drivers/clk/renesas/Makefile                                 |    1 
 drivers/clk/renesas/r8a7743-cpg-mssr.c                       |  270 +++++++++++
 drivers/clk/renesas/renesas-cpg-mssr.c                       |    6 
 drivers/clk/renesas/renesas-cpg-mssr.h                       |    1 
 6 files changed, 282 insertions(+), 1 deletion(-)

Index: linux/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt
===================================================================
--- linux.orig/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt
+++ linux/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt
@@ -13,6 +13,7 @@ They provide the following functionaliti
 
 Required Properties:
   - compatible: Must be one of:
+      - "renesas,r8a7743-cpg-mssr" for the r8a7743 SoC (RZ/G1M)
       - "renesas,r8a7795-cpg-mssr" for the r8a7795 SoC (R-Car H3)
       - "renesas,r8a7796-cpg-mssr" for the r8a7796 SoC (R-Car M3-W)
 
@@ -22,7 +23,8 @@ Required Properties:
   - clocks: References to external parent clocks, one entry for each entry in
     clock-names
   - clock-names: List of external parent clock names. Valid names are:
-      - "extal" (r8a7795, r8a7796)
+      - "extal" (r8a7743, r8a7795, r8a7796)
+      - "usb_extal" (r8a7743)
       - "extalr" (r8a7795, r8a7796)
 
   - #clock-cells: Must be 2
Index: linux/drivers/clk/renesas/Kconfig
===================================================================
--- linux.orig/drivers/clk/renesas/Kconfig
+++ linux/drivers/clk/renesas/Kconfig
@@ -1,5 +1,6 @@
 config CLK_RENESAS_CPG_MSSR
 	bool
+	default y if ARCH_R8A7743
 	default y if ARCH_R8A7795
 	default y if ARCH_R8A7796
 
Index: linux/drivers/clk/renesas/Makefile
===================================================================
--- linux.orig/drivers/clk/renesas/Makefile
+++ linux/drivers/clk/renesas/Makefile
@@ -2,6 +2,7 @@ obj-$(CONFIG_ARCH_EMEV2)		+= clk-emev2.o
 obj-$(CONFIG_ARCH_R7S72100)		+= clk-rz.o
 obj-$(CONFIG_ARCH_R8A73A4)		+= clk-r8a73a4.o clk-div6.o
 obj-$(CONFIG_ARCH_R8A7740)		+= clk-r8a7740.o clk-div6.o
+obj-$(CONFIG_ARCH_R8A7743)		+= r8a7743-cpg-mssr.o rcar-gen2-cpg.o
 obj-$(CONFIG_ARCH_R8A7778)		+= clk-r8a7778.o
 obj-$(CONFIG_ARCH_R8A7779)		+= clk-r8a7779.o
 obj-$(CONFIG_ARCH_R8A7790)		+= clk-rcar-gen2.o clk-div6.o
Index: linux/drivers/clk/renesas/r8a7743-cpg-mssr.c
===================================================================
--- /dev/null
+++ linux/drivers/clk/renesas/r8a7743-cpg-mssr.c
@@ -0,0 +1,270 @@
+/*
+ * r8a7743 Clock Pulse Generator / Module Standby and Software Reset
+ *
+ * Copyright (C) 2016 Cogent Embedded Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published
+ * by the Free Software Foundation; of the License.
+ */
+
+#include <linux/device.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/soc/renesas/rcar-rst.h>
+
+#include <dt-bindings/clock/r8a7743-cpg-mssr.h>
+
+#include "renesas-cpg-mssr.h"
+#include "rcar-gen2-cpg.h"
+
+enum clk_ids {
+	/* Core Clock Outputs exported to DT */
+	LAST_DT_CORE_CLK = R8A7743_CLK_OSC,
+
+	/* External Input Clocks */
+	CLK_EXTAL,
+	CLK_USB_EXTAL,
+
+	/* Internal Core Clocks */
+	CLK_MAIN,
+	CLK_PLL0,
+	CLK_PLL1,
+	CLK_PLL3,
+	CLK_PLL1_DIV2,
+
+	/* Module Clocks */
+	MOD_CLK_BASE
+};
+
+static const struct cpg_core_clk r8a7743_core_clks[] __initconst = {
+	/* External Clock Inputs */
+	DEF_INPUT("extal",	CLK_EXTAL),
+	DEF_INPUT("usb_extal",	CLK_USB_EXTAL),
+
+	/* Internal Core Clocks */
+	DEF_BASE(".main",	CLK_MAIN, CLK_TYPE_GEN2_MAIN, CLK_EXTAL),
+	DEF_BASE(".pll0",	CLK_PLL0, CLK_TYPE_GEN2_PLL0, CLK_MAIN),
+	DEF_BASE(".pll1",	CLK_PLL1, CLK_TYPE_GEN2_PLL1, CLK_MAIN),
+	DEF_BASE(".pll3",	CLK_PLL3, CLK_TYPE_GEN2_PLL3, CLK_MAIN),
+
+	DEF_FIXED(".pll1_div2", CLK_PLL1_DIV2, CLK_PLL1, 2, 1),
+
+	/* Core Clock Outputs */
+	DEF_BASE("z",    R8A7743_CLK_Z,    CLK_TYPE_GEN2_Z,     CLK_PLL0),
+	DEF_BASE("lb",	 R8A7743_CLK_LB,   CLK_TYPE_GEN2_LB,	CLK_PLL1),
+	DEF_BASE("sdh",  R8A7743_CLK_SDH,  CLK_TYPE_GEN2_SDH,	CLK_PLL1),
+	DEF_BASE("sd0",  R8A7743_CLK_SD0,  CLK_TYPE_GEN2_SD0,	CLK_PLL1),
+	DEF_BASE("qspi", R8A7743_CLK_QSPI, CLK_TYPE_GEN2_QSPI,	CLK_PLL1_DIV2),
+	DEF_BASE("rcan", R8A7743_CLK_RCAN, CLK_TYPE_GEN2_RCAN,	CLK_USB_EXTAL),
+
+	DEF_FIXED("zg",    R8A7743_CLK_ZG,	CLK_PLL1,	    3, 1),
+	DEF_FIXED("zx",    R8A7743_CLK_ZX,	CLK_PLL1,	    3, 1),
+	DEF_FIXED("zs",    R8A7743_CLK_ZS,	CLK_PLL1,	    6, 1),
+	DEF_FIXED("hp",    R8A7743_CLK_HP,	CLK_PLL1,	   12, 1),
+	DEF_FIXED("b",     R8A7743_CLK_B,	CLK_PLL1,	   12, 1),
+	DEF_FIXED("p",     R8A7743_CLK_P,	CLK_PLL1,	   24, 1),
+	DEF_FIXED("cl",    R8A7743_CLK_CL,	CLK_PLL1,	   48, 1),
+	DEF_FIXED("m2",    R8A7743_CLK_M2,	CLK_PLL1,	    8, 1),
+	DEF_FIXED("zb3",   R8A7743_CLK_ZB3,	CLK_PLL3,	    4, 1),
+	DEF_FIXED("zb3d2", R8A7743_CLK_ZB3D2,	CLK_PLL3,	    8, 1),
+	DEF_FIXED("ddr",   R8A7743_CLK_DDR,	CLK_PLL3,	    8, 1),
+	DEF_FIXED("mp",    R8A7743_CLK_MP,	CLK_PLL1_DIV2,     15, 1),
+	DEF_FIXED("cp",    R8A7743_CLK_CP,	CLK_EXTAL,	    2, 1),
+	DEF_FIXED("r",     R8A7743_CLK_R,	CLK_PLL1,	49152, 1),
+	DEF_FIXED("osc",   R8A7743_CLK_OSC,	CLK_PLL1,	12288, 1),
+
+	DEF_DIV6P1("sd2",  R8A7743_CLK_SD2,	CLK_PLL1_DIV2,	0x078),
+	DEF_DIV6P1("sd3",  R8A7743_CLK_SD3,	CLK_PLL1_DIV2,	0x26c),
+	DEF_DIV6P1("mmc0", R8A7743_CLK_MMC0,	CLK_PLL1_DIV2,	0x240),
+};
+
+static const struct mssr_mod_clk r8a7743_mod_clks[] __initconst = {
+	DEF_MOD("msiof0",	    0,	R8A7743_CLK_MP),
+	DEF_MOD("vcp0",		  101,	R8A7743_CLK_ZS),
+	DEF_MOD("vpc0",		  103,	R8A7743_CLK_ZS),
+	DEF_MOD("tmu1",		  111,	R8A7743_CLK_P),
+	DEF_MOD("3dg",		  112,	R8A7743_CLK_ZG),
+	DEF_MOD("2d-dmac",	  115,	R8A7743_CLK_ZS),
+	DEF_MOD("fdp1-1",	  118,	R8A7743_CLK_ZS),
+	DEF_MOD("fdp1-0",	  119,	R8A7743_CLK_ZS),
+	DEF_MOD("tmu3",		  121,	R8A7743_CLK_P),
+	DEF_MOD("tmu2",		  122,	R8A7743_CLK_P),
+	DEF_MOD("cmt0",		  124,	R8A7743_CLK_R),
+	DEF_MOD("tmu0",		  125,	R8A7743_CLK_CP),
+	DEF_MOD("vsp1du1",	  127,	R8A7743_CLK_ZS),
+	DEF_MOD("vsp1du0",	  128,	R8A7743_CLK_ZS),
+	DEF_MOD("vsp1-sy",	  131,	R8A7743_CLK_ZS),
+	DEF_MOD("scifa2",	  202,	R8A7743_CLK_MP),
+	DEF_MOD("scifa1",	  203,	R8A7743_CLK_MP),
+	DEF_MOD("scifa0",	  204,	R8A7743_CLK_MP),
+	DEF_MOD("msiof2",	  205,	R8A7743_CLK_MP),
+	DEF_MOD("scifb0",	  206,	R8A7743_CLK_MP),
+	DEF_MOD("scifb1",	  207,	R8A7743_CLK_MP),
+	DEF_MOD("msiof1",	  208,	R8A7743_CLK_MP),
+	DEF_MOD("scifb2",	  216,	R8A7743_CLK_MP),
+	DEF_MOD("sys-dmac1",	  218,	R8A7743_CLK_ZS),
+	DEF_MOD("sys-dmac0",	  219,	R8A7743_CLK_ZS),
+	DEF_MOD("tpu0",		  304,	R8A7743_CLK_CP),
+	DEF_MOD("sdhi3",	  311,	R8A7743_CLK_SD3),
+	DEF_MOD("sdhi2",	  312,	R8A7743_CLK_SD2),
+	DEF_MOD("sdhi0",	  314,	R8A7743_CLK_SD0),
+	DEF_MOD("mmcif0",	  315,	R8A7743_CLK_MMC0),
+	DEF_MOD("iic0",		  318,	R8A7743_CLK_HP),
+	DEF_MOD("pciec",	  319,	R8A7743_CLK_MP),
+	DEF_MOD("iic1",		  323,	R8A7743_CLK_HP),
+	DEF_MOD("usb3.0",	  328,	R8A7743_CLK_MP),
+	DEF_MOD("cmt1",		  329,	R8A7743_CLK_R),
+	DEF_MOD("usbhs-dmac0",	  330,	R8A7743_CLK_HP),
+	DEF_MOD("usbhs-dmac1",	  331,	R8A7743_CLK_HP),
+	DEF_MOD("irqc",		  407,	R8A7743_CLK_CP),
+	DEF_MOD("intc-sys",	  408,	R8A7743_CLK_ZS),
+	DEF_MOD("audio-dmac1",	  501,	R8A7743_CLK_HP),
+	DEF_MOD("audio-dmac0",	  502,	R8A7743_CLK_HP),
+	DEF_MOD("thermal",	  522,	CLK_EXTAL),
+	DEF_MOD("pwm",		  523,	R8A7743_CLK_P),
+	DEF_MOD("usb-ehci",	  703,	R8A7743_CLK_MP),
+	DEF_MOD("usbhs",	  704,	R8A7743_CLK_HP),
+	DEF_MOD("hscif2",	  713,	R8A7743_CLK_ZS),
+	DEF_MOD("scif5",	  714,	R8A7743_CLK_P),
+	DEF_MOD("scif4",	  715,	R8A7743_CLK_P),
+	DEF_MOD("hscif1",	  716,	R8A7743_CLK_ZS),
+	DEF_MOD("hscif0",	  717,	R8A7743_CLK_ZS),
+	DEF_MOD("scif3",	  718,	R8A7743_CLK_P),
+	DEF_MOD("scif2",	  719,	R8A7743_CLK_P),
+	DEF_MOD("scif1",	  720,	R8A7743_CLK_P),
+	DEF_MOD("scif0",	  721,	R8A7743_CLK_P),
+	DEF_MOD("du1",		  723,	R8A7743_CLK_ZX),
+	DEF_MOD("du0",		  724,	R8A7743_CLK_ZX),
+	DEF_MOD("lvds0",	  726,	R8A7743_CLK_ZX),
+	DEF_MOD("ipmmu-sgx",	  800,	R8A7743_CLK_ZX),
+	DEF_MOD("vin2",		  809,	R8A7743_CLK_ZG),
+	DEF_MOD("vin1",		  810,	R8A7743_CLK_ZG),
+	DEF_MOD("vin0",		  811,	R8A7743_CLK_ZG),
+	DEF_MOD("etheravb",	  812,	R8A7743_CLK_HP),
+	DEF_MOD("ether",	  813,	R8A7743_CLK_P),
+	DEF_MOD("sata1",	  814,	R8A7743_CLK_ZS),
+	DEF_MOD("sata0",	  815,	R8A7743_CLK_ZS),
+	DEF_MOD("gpio7",	  904,	R8A7743_CLK_CP),
+	DEF_MOD("gpio6",	  905,	R8A7743_CLK_CP),
+	DEF_MOD("gpio5",	  907,	R8A7743_CLK_CP),
+	DEF_MOD("gpio4",	  908,	R8A7743_CLK_CP),
+	DEF_MOD("gpio3",	  909,	R8A7743_CLK_CP),
+	DEF_MOD("gpio2",	  910,	R8A7743_CLK_CP),
+	DEF_MOD("gpio1",	  911,	R8A7743_CLK_CP),
+	DEF_MOD("gpio0",	  912,	R8A7743_CLK_CP),
+	DEF_MOD("can1",		  915,	R8A7743_CLK_P),
+	DEF_MOD("can0",		  916,	R8A7743_CLK_P),
+	DEF_MOD("qspi_mod",	  917,	R8A7743_CLK_QSPI),
+	DEF_MOD("i2c5",		  925,	R8A7743_CLK_HP),
+	DEF_MOD("iicdvfs",	  926,	R8A7743_CLK_CP),
+	DEF_MOD("i2c4",		  927,	R8A7743_CLK_HP),
+	DEF_MOD("i2c3",		  928,	R8A7743_CLK_HP),
+	DEF_MOD("i2c2",		  929,	R8A7743_CLK_HP),
+	DEF_MOD("i2c1",		  930,	R8A7743_CLK_HP),
+	DEF_MOD("i2c0",		  931,	R8A7743_CLK_HP),
+	DEF_MOD("ssi-all",	 1005,	R8A7743_CLK_P),
+	DEF_MOD("ssi9",		 1006,	MOD_CLK_ID(1005)),
+	DEF_MOD("ssi8",		 1007,	MOD_CLK_ID(1005)),
+	DEF_MOD("ssi7",		 1008,	MOD_CLK_ID(1005)),
+	DEF_MOD("ssi6",		 1009,	MOD_CLK_ID(1005)),
+	DEF_MOD("ssi5",		 1010,	MOD_CLK_ID(1005)),
+	DEF_MOD("ssi4",		 1011,	MOD_CLK_ID(1005)),
+	DEF_MOD("ssi3",		 1012,	MOD_CLK_ID(1005)),
+	DEF_MOD("ssi2",		 1013,	MOD_CLK_ID(1005)),
+	DEF_MOD("ssi1",		 1014,	MOD_CLK_ID(1005)),
+	DEF_MOD("ssi0",		 1015,	MOD_CLK_ID(1005)),
+	DEF_MOD("scu-all",	 1017,	R8A7743_CLK_P),
+	DEF_MOD("scu-dvc1",	 1018,	MOD_CLK_ID(1017)),
+	DEF_MOD("scu-dvc0",	 1019,	MOD_CLK_ID(1017)),
+	DEF_MOD("scu-ctu1-mix1", 1020,	MOD_CLK_ID(1017)),
+	DEF_MOD("scu-ctu0-mix0", 1021,	MOD_CLK_ID(1017)),
+	DEF_MOD("scu-src9",	 1022,	MOD_CLK_ID(1017)),
+	DEF_MOD("scu-src8",	 1023,	MOD_CLK_ID(1017)),
+	DEF_MOD("scu-src7",	 1024,	MOD_CLK_ID(1017)),
+	DEF_MOD("scu-src6",	 1025,	MOD_CLK_ID(1017)),
+	DEF_MOD("scu-src5",	 1026,	MOD_CLK_ID(1017)),
+	DEF_MOD("scu-src4",	 1027,	MOD_CLK_ID(1017)),
+	DEF_MOD("scu-src3",	 1028,	MOD_CLK_ID(1017)),
+	DEF_MOD("scu-src2",	 1029,	MOD_CLK_ID(1017)),
+	DEF_MOD("scu-src1",	 1030,	MOD_CLK_ID(1017)),
+	DEF_MOD("scu-src0",	 1031,	MOD_CLK_ID(1017)),
+	DEF_MOD("scifa3",	 1106,	R8A7743_CLK_MP),
+	DEF_MOD("scifa4",	 1107,	R8A7743_CLK_MP),
+	DEF_MOD("scifa5",	 1108,	R8A7743_CLK_MP),
+};
+
+static const unsigned int r8a7743_crit_mod_clks[] __initconst = {
+	MOD_CLK_ID(408),	/* INTC-SYS (GIC) */
+};
+
+/*
+ * CPG Clock Data
+ */
+
+/*
+ *    MD	EXTAL		PLL0	PLL1	PLL3
+ * 14 13 19	(MHz)		*1	*1
+ *---------------------------------------------------
+ * 0  0  0	15		x172/2	x208/2	x106
+ * 0  0  1	15		x172/2	x208/2	x88
+ * 0  1  0	20		x130/2	x156/2	x80
+ * 0  1  1	20		x130/2	x156/2	x66
+ * 1  0  0	26 / 2		x200/2	x240/2	x122
+ * 1  0  1	26 / 2		x200/2	x240/2	x102
+ * 1  1  0	30 / 2		x172/2	x208/2	x106
+ * 1  1  1	30 / 2		x172/2	x208/2	x88
+ *
+ * *1 :	Table 7.5a indicates VCO output (PLLx = VCO/2)
+ */
+#define CPG_PLL_CONFIG_INDEX(md)	((((md) & BIT(14)) >> 12) | \
+					 (((md) & BIT(13)) >> 12) | \
+					 (((md) & BIT(19)) >> 19))
+
+static const struct rcar_gen2_cpg_pll_config cpg_pll_configs[8] __initconst = {
+	/* EXTAL div	PLL1 mult	PLL3 mult */
+	{ 1,		208,		106,	},
+	{ 1,		208,		88,	},
+	{ 1,		156,		80,	},
+	{ 1,		156,		66,	},
+	{ 2,		240,		122,	},
+	{ 2,		240,		102,	},
+	{ 2,		208,		106,	},
+	{ 2,		208,		88,	},
+};
+
+static int __init r8a7743_cpg_mssr_init(struct device *dev)
+{
+	const struct rcar_gen2_cpg_pll_config *cpg_pll_config;
+	u32 cpg_mode;
+	int error;
+
+	error = rcar_rst_read_mode_pins(&cpg_mode);
+	if (error)
+		return error;
+
+	cpg_pll_config = &cpg_pll_configs[CPG_PLL_CONFIG_INDEX(cpg_mode)];
+
+	return rcar_gen2_cpg_init(cpg_pll_config, 2);
+}
+
+const struct cpg_mssr_info r8a7743_cpg_mssr_info __initconst = {
+	/* Core Clocks */
+	.core_clks = r8a7743_core_clks,
+	.num_core_clks = ARRAY_SIZE(r8a7743_core_clks),
+	.last_dt_core_clk = LAST_DT_CORE_CLK,
+	.num_total_core_clks = MOD_CLK_BASE,
+
+	/* Module Clocks */
+	.mod_clks = r8a7743_mod_clks,
+	.num_mod_clks = ARRAY_SIZE(r8a7743_mod_clks),
+	.num_hw_mod_clks = 12 * 32,
+
+	/* Critical Module Clocks */
+	.crit_mod_clks = r8a7743_crit_mod_clks,
+	.num_crit_mod_clks = ARRAY_SIZE(r8a7743_crit_mod_clks),
+
+	/* Callbacks */
+	.init = r8a7743_cpg_mssr_init,
+	.cpg_clk_register = rcar_gen2_cpg_clk_register,
+};
Index: linux/drivers/clk/renesas/renesas-cpg-mssr.c
===================================================================
--- linux.orig/drivers/clk/renesas/renesas-cpg-mssr.c
+++ linux/drivers/clk/renesas/renesas-cpg-mssr.c
@@ -503,6 +503,12 @@ static int __init cpg_mssr_add_clk_domai
 }
 
 static const struct of_device_id cpg_mssr_match[] = {
+#ifdef CONFIG_ARCH_R8A7743
+	{
+		.compatible = "renesas,r8a7743-cpg-mssr",
+		.data = &r8a7743_cpg_mssr_info,
+	},
+#endif
 #ifdef CONFIG_ARCH_R8A7795
 	{
 		.compatible = "renesas,r8a7795-cpg-mssr",
Index: linux/drivers/clk/renesas/renesas-cpg-mssr.h
===================================================================
--- linux.orig/drivers/clk/renesas/renesas-cpg-mssr.h
+++ linux/drivers/clk/renesas/renesas-cpg-mssr.h
@@ -130,6 +130,7 @@ struct cpg_mssr_info {
 					struct clk **clks, void __iomem *base);
 };
 
+extern const struct cpg_mssr_info r8a7743_cpg_mssr_info;
 extern const struct cpg_mssr_info r8a7795_cpg_mssr_info;
 extern const struct cpg_mssr_info r8a7796_cpg_mssr_info;
 #endif

^ permalink raw reply

* Re: [PATCH 3/7] genirq/affinity: Handle pre/post vectors in irq_create_affinity_masks()
From: Bjorn Helgaas @ 2016-11-08 21:20 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: tglx, axboe, linux-block, linux-pci, linux-kernel
In-Reply-To: <1478544462-9549-4-git-send-email-hch@lst.de>

On Mon, Nov 07, 2016 at 10:47:38AM -0800, Christoph Hellwig wrote:
> From: Christogh Hellwig <hch@lst.de>
> 
> Only calculate the affinity for the main I/O vectors, and skip the
> pre or post vectors specified by struct irq_affinity.
> 
> Also remove the irq_affinity cpumask argument that has never been used.
> If we ever need it in the future we can pass it through struct
> irq_affinity.
> 
> Signed-off-by: Christogh Hellwig <hch@lst.de>

s/Christogh/Christoph/ (also above, and maybe other patches too?)

Acked-by: Bjorn Helgaas <bhelgaas@google.com>

> ---
>  drivers/pci/msi.c         |  4 ++--
>  include/linux/interrupt.h |  4 ++--
>  kernel/irq/affinity.c     | 46 +++++++++++++++++++++++++---------------------
>  3 files changed, 29 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
> index c58d3c2..1761b8a 100644
> --- a/drivers/pci/msi.c
> +++ b/drivers/pci/msi.c
> @@ -558,7 +558,7 @@ msi_setup_entry(struct pci_dev *dev, int nvec, bool affinity)
>  	u16 control;
>  
>  	if (affinity) {
> -		masks = irq_create_affinity_masks(dev->irq_affinity, nvec);
> +		masks = irq_create_affinity_masks(nvec, NULL);
>  		if (!masks)
>  			pr_err("Unable to allocate affinity masks, ignoring\n");
>  	}
> @@ -697,7 +697,7 @@ static int msix_setup_entries(struct pci_dev *dev, void __iomem *base,
>  	int ret, i;
>  
>  	if (affinity) {
> -		masks = irq_create_affinity_masks(dev->irq_affinity, nvec);
> +		masks = irq_create_affinity_masks(nvec, NULL);
>  		if (!masks)
>  			pr_err("Unable to allocate affinity masks, ignoring\n");

Not caused by this patch, but can we use dev_err() here and above?

^ permalink raw reply

* Re: [PATCH 5/7] pci/msi: Provide pci_alloc_irq_vectors_affinity()
From: Christoph Hellwig @ 2016-11-08 21:20 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Christoph Hellwig, tglx, axboe, linux-block, linux-pci,
	linux-kernel
In-Reply-To: <20161108211732.GC14322@bhelgaas-glaptop.roam.corp.google.com>

On Tue, Nov 08, 2016 at 03:17:32PM -0600, Bjorn Helgaas wrote:
> On Mon, Nov 07, 2016 at 10:47:40AM -0800, Christoph Hellwig wrote:
> > From: Christogh Hellwig <hch@lst.de>
> 
> s/Christogh/Christoph/ (also below)

Haha, so much for taking Thomas' split of my patches and not proof-reading
them..

^ permalink raw reply

* Re: [PATCH 4/7] pci/msi: Propagate irq affinity description through the MSI code
From: Bjorn Helgaas @ 2016-11-08 21:18 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: tglx, axboe, linux-block, linux-pci, linux-kernel
In-Reply-To: <1478544462-9549-5-git-send-email-hch@lst.de>

s|pci/msi|PCI/MSI| (subject)
s/irq/IRQ/ (subject)

On Mon, Nov 07, 2016 at 10:47:39AM -0800, Christoph Hellwig wrote:
> From: Christogh Hellwig <hch@lst.de>
> 
> No API change yet, just pass it down all the way from
> pci_alloc_irq_vectors to the core MSI code.

pci_alloc_irq_vectors()

> Signed-off-by: Christogh Hellwig <hch@lst.de>

Acked-by: Bjorn Helgaas <bhelgaas@google.com>

> ---
>  drivers/pci/msi.c | 62 +++++++++++++++++++++++++++++--------------------------
>  1 file changed, 33 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
> index 1761b8a..512f388 100644
> --- a/drivers/pci/msi.c
> +++ b/drivers/pci/msi.c
> @@ -551,14 +551,14 @@ static int populate_msi_sysfs(struct pci_dev *pdev)
>  }
>  
>  static struct msi_desc *
> -msi_setup_entry(struct pci_dev *dev, int nvec, bool affinity)
> +msi_setup_entry(struct pci_dev *dev, int nvec, const struct irq_affinity *affd)
>  {
>  	struct cpumask *masks = NULL;
>  	struct msi_desc *entry;
>  	u16 control;
>  
> -	if (affinity) {
> -		masks = irq_create_affinity_masks(nvec, NULL);
> +	if (affd) {
> +		masks = irq_create_affinity_masks(nvec, affd);
>  		if (!masks)
>  			pr_err("Unable to allocate affinity masks, ignoring\n");
>  	}
> @@ -618,7 +618,8 @@ static int msi_verify_entries(struct pci_dev *dev)
>   * an error, and a positive return value indicates the number of interrupts
>   * which could have been allocated.
>   */
> -static int msi_capability_init(struct pci_dev *dev, int nvec, bool affinity)
> +static int msi_capability_init(struct pci_dev *dev, int nvec,
> +			       const struct irq_affinity *affd)
>  {
>  	struct msi_desc *entry;
>  	int ret;
> @@ -626,7 +627,7 @@ static int msi_capability_init(struct pci_dev *dev, int nvec, bool affinity)
>  
>  	pci_msi_set_enable(dev, 0);	/* Disable MSI during set up */
>  
> -	entry = msi_setup_entry(dev, nvec, affinity);
> +	entry = msi_setup_entry(dev, nvec, affd);
>  	if (!entry)
>  		return -ENOMEM;
>  
> @@ -690,14 +691,14 @@ static void __iomem *msix_map_region(struct pci_dev *dev, unsigned nr_entries)
>  
>  static int msix_setup_entries(struct pci_dev *dev, void __iomem *base,
>  			      struct msix_entry *entries, int nvec,
> -			      bool affinity)
> +			      const struct irq_affinity *affd)
>  {
>  	struct cpumask *curmsk, *masks = NULL;
>  	struct msi_desc *entry;
>  	int ret, i;
>  
> -	if (affinity) {
> -		masks = irq_create_affinity_masks(nvec, NULL);
> +	if (affd) {
> +		masks = irq_create_affinity_masks(nvec, affd);
>  		if (!masks)
>  			pr_err("Unable to allocate affinity masks, ignoring\n");
>  	}
> @@ -753,14 +754,14 @@ static void msix_program_entries(struct pci_dev *dev,
>   * @dev: pointer to the pci_dev data structure of MSI-X device function
>   * @entries: pointer to an array of struct msix_entry entries
>   * @nvec: number of @entries
> - * @affinity: flag to indicate cpu irq affinity mask should be set
> + * @affd: Optional pointer to enable automatic affinity assignement
>   *
>   * Setup the MSI-X capability structure of device function with a
>   * single MSI-X irq. A return of zero indicates the successful setup of
>   * requested MSI-X entries with allocated irqs or non-zero for otherwise.
>   **/
>  static int msix_capability_init(struct pci_dev *dev, struct msix_entry *entries,
> -				int nvec, bool affinity)
> +				int nvec, const struct irq_affinity *affd)
>  {
>  	int ret;
>  	u16 control;
> @@ -775,7 +776,7 @@ static int msix_capability_init(struct pci_dev *dev, struct msix_entry *entries,
>  	if (!base)
>  		return -ENOMEM;
>  
> -	ret = msix_setup_entries(dev, base, entries, nvec, affinity);
> +	ret = msix_setup_entries(dev, base, entries, nvec, affd);
>  	if (ret)
>  		return ret;
>  
> @@ -956,7 +957,7 @@ int pci_msix_vec_count(struct pci_dev *dev)
>  EXPORT_SYMBOL(pci_msix_vec_count);
>  
>  static int __pci_enable_msix(struct pci_dev *dev, struct msix_entry *entries,
> -			     int nvec, bool affinity)
> +			     int nvec, const struct irq_affinity *affd)
>  {
>  	int nr_entries;
>  	int i, j;
> @@ -988,7 +989,7 @@ static int __pci_enable_msix(struct pci_dev *dev, struct msix_entry *entries,
>  		dev_info(&dev->dev, "can't enable MSI-X (MSI IRQ already assigned)\n");
>  		return -EINVAL;
>  	}
> -	return msix_capability_init(dev, entries, nvec, affinity);
> +	return msix_capability_init(dev, entries, nvec, affd);
>  }
>  
>  /**
> @@ -1008,7 +1009,7 @@ static int __pci_enable_msix(struct pci_dev *dev, struct msix_entry *entries,
>   **/
>  int pci_enable_msix(struct pci_dev *dev, struct msix_entry *entries, int nvec)
>  {
> -	return __pci_enable_msix(dev, entries, nvec, false);
> +	return __pci_enable_msix(dev, entries, nvec, NULL);
>  }
>  EXPORT_SYMBOL(pci_enable_msix);
>  
> @@ -1059,9 +1060,8 @@ int pci_msi_enabled(void)
>  EXPORT_SYMBOL(pci_msi_enabled);
>  
>  static int __pci_enable_msi_range(struct pci_dev *dev, int minvec, int maxvec,
> -		unsigned int flags)
> +				  const struct irq_affinity *affd)
>  {
> -	bool affinity = flags & PCI_IRQ_AFFINITY;
>  	int nvec;
>  	int rc;
>  
> @@ -1090,13 +1090,13 @@ static int __pci_enable_msi_range(struct pci_dev *dev, int minvec, int maxvec,
>  		nvec = maxvec;
>  
>  	for (;;) {
> -		if (affinity) {
> -			nvec = irq_calc_affinity_vectors(nvec, NULL);
> +		if (affd) {
> +			nvec = irq_calc_affinity_vectors(nvec, affd);
>  			if (nvec < minvec)
>  				return -ENOSPC;
>  		}
>  
> -		rc = msi_capability_init(dev, nvec, affinity);
> +		rc = msi_capability_init(dev, nvec, affd);
>  		if (rc == 0)
>  			return nvec;
>  
> @@ -1123,28 +1123,27 @@ static int __pci_enable_msi_range(struct pci_dev *dev, int minvec, int maxvec,
>   **/
>  int pci_enable_msi_range(struct pci_dev *dev, int minvec, int maxvec)
>  {
> -	return __pci_enable_msi_range(dev, minvec, maxvec, 0);
> +	return __pci_enable_msi_range(dev, minvec, maxvec, NULL);
>  }
>  EXPORT_SYMBOL(pci_enable_msi_range);
>  
>  static int __pci_enable_msix_range(struct pci_dev *dev,
> -		struct msix_entry *entries, int minvec, int maxvec,
> -		unsigned int flags)
> +				   struct msix_entry *entries, int minvec,
> +				   int maxvec, const struct irq_affinity *affd)
>  {
> -	bool affinity = flags & PCI_IRQ_AFFINITY;
>  	int rc, nvec = maxvec;
>  
>  	if (maxvec < minvec)
>  		return -ERANGE;
>  
>  	for (;;) {
> -		if (affinity) {
> -			nvec = irq_calc_affinity_vectors(nvec, NULL);
> +		if (affd) {
> +			nvec = irq_calc_affinity_vectors(nvec, affd);
>  			if (nvec < minvec)
>  				return -ENOSPC;
>  		}
>  
> -		rc = __pci_enable_msix(dev, entries, nvec, affinity);
> +		rc = __pci_enable_msix(dev, entries, nvec, affd);
>  		if (rc == 0)
>  			return nvec;
>  
> @@ -1175,7 +1174,7 @@ static int __pci_enable_msix_range(struct pci_dev *dev,
>  int pci_enable_msix_range(struct pci_dev *dev, struct msix_entry *entries,
>  		int minvec, int maxvec)
>  {
> -	return __pci_enable_msix_range(dev, entries, minvec, maxvec, 0);
> +	return __pci_enable_msix_range(dev, entries, minvec, maxvec, NULL);
>  }
>  EXPORT_SYMBOL(pci_enable_msix_range);
>  
> @@ -1199,17 +1198,22 @@ EXPORT_SYMBOL(pci_enable_msix_range);
>  int pci_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs,
>  		unsigned int max_vecs, unsigned int flags)
>  {
> +	static const struct irq_affinity msi_default_affd;
> +	const struct irq_affinity *affd = NULL;
>  	int vecs = -ENOSPC;
>  
> +	if (flags & PCI_IRQ_AFFINITY)
> +		affd = &msi_default_affd;
> +
>  	if (flags & PCI_IRQ_MSIX) {
>  		vecs = __pci_enable_msix_range(dev, NULL, min_vecs, max_vecs,
> -				flags);
> +				affd);
>  		if (vecs > 0)
>  			return vecs;
>  	}
>  
>  	if (flags & PCI_IRQ_MSI) {
> -		vecs = __pci_enable_msi_range(dev, min_vecs, max_vecs, flags);
> +		vecs = __pci_enable_msi_range(dev, min_vecs, max_vecs, affd);
>  		if (vecs > 0)
>  			return vecs;
>  	}
> -- 
> 2.1.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [alsa-devel] [PATCH 2/9] ALSA: ac97: add an ac97 bus
From: Robert Jarzmik @ 2016-11-08 21:18 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <d57cdfba-da22-652d-3d75-6d1b754cf1d5@metafoo.de>

Lars-Peter Clausen <lars@metafoo.de> writes:

> On 10/26/2016 09:41 PM, Robert Jarzmik wrote:
>> +#define to_ac97_device(d) container_of(d, struct ac97_codec_device, dev)
>> +#define to_ac97_driver(d) container_of(d, struct ac97_codec_driver, driver)
>
> In my opinion these should be inline functions rather than macros as that
> generates much more legible compiler errors e.g. in case there is a type
> mismatch.
Sure, why not.

>> +struct ac97_codec_driver {
>> +	struct device_driver	driver;
>> +	int			(*probe)(struct ac97_codec_device *);
>> +	int			(*remove)(struct ac97_codec_device *);
>> +	int			(*suspend)(struct ac97_codec_device *);
>> +	int			(*resume)(struct ac97_codec_device *);
>> +	void			(*shutdown)(struct ac97_codec_device *);
>
> The suspend, resume and shutdown callbacks are never used. Which is good,
> since all new frameworks should use dev_pm_ops. Just drop the from the struct.
Ok.

>> +/**
>> + * struct ac97_controller - The AC97 controller of the AC-Link
>> + * @ops:		the AC97 operations.
>> + * @controllers:	linked list of all existing controllers.
>> + * @dev:		the device providing the AC97 controller.
>> + * @slots_available:	the mask of accessible/scanable codecs.
>> + * @codecs:		the 4 possible AC97 codecs (NULL if none found).
>> + * @codecs_pdata:	platform_data for each codec (NULL if no pdata).
>> + *
>> + * This structure is internal to AC97 bus, and should not be used by the
>> + * controllers themselves, excepting for using @dev.
>> + */
>> +struct ac97_controller {
>> +	const struct ac97_controller_ops *ops;
>> +	struct list_head controllers;
>> +	struct device *dev;
>
> I'd make the controller itself a struct dev, rather than just having the
> pointer to the parent. This is more idiomatic and matches what other
> subsystems do. It has several advantages, you get proper refcounting of your
> controller structure, the controller gets its own sysfs directory where the
> CODECs appear as children, you don't need the manual sysfs attribute
> creation and removal in ac97_controler_{un,}register().

If you mean having "struct device dev" instead of "struct device *dev", it has
also a drawback : all the legacy platforms have already a probing method, be
that platform data, device-tree or something else.

I'm a bit converned about the conversion toll. Maybe I've not understood your
point fully, so please feel free to explain, with an actual example even better.

>> +	void (*wait)(struct ac97_controller *adrv, int slot);
>> +	void (*init)(struct ac97_controller *adrv, int slot);
>
> Neither wait nor init are ever used.
This is because I've not begun to porting sound/pci/ac97_codec.c into
sound/ac97.

>> +	if ((codec_num < 0) || (codec_num >= AC97_BUS_MAX_CODECS))
>> +		return ERR_PTR(-ERANGE);
>
> I'd make this EINVAL.
Ok.

>> +	adev = container_of(dev, struct ac97_codec_device, dev);
>
> to_ac97_device()
Sure.

>> +	codec_name = kasprintf(GFP_KERNEL, "%s:%d", dev_name(ac97_ctrl->dev),
>> +			       idx);
>> +	codec->dev.init_name = codec_name;
>
> init_name is only for statically allocated devices. Use dev_set_name(dev,
> ...). No need for kasprintf() either as dev_set_name() takes a format string.
I'll try again, I seem to remember having tried that and it failed, but I can't
remember why.

> For this you need to split device_register into device_initialize() and
> device_add(). But usually that is what you want anyway.
Let me try again.

>> +	ret = sysfs_create_link(&codec->dev.kobj, &ac97_ctrl->dev->kobj,
>> +				"ac97_controller");
>
> Since the CODEC is a child of the controller this should not be necessary as
> this just points one directory up. It's like `ln -s .. parent`
This creates :
/sys/bus/ac97/devices/pxa2xx-ac97\:0/ac97_controller

Of course as you pointed out, it's a 'ln -s .. parent' like link, but it seems
to me very unfriendly to have :
 - /sys/bus/ac97/devices/pxa2xx-ac97\:0/../ac97_operations
 - while /sys/bus/ac97/devices/ac97_operations doesn't exist (obviously)

Mmm, I don't know for this one, my mind is not set, it's a bit hard to tell if
it's only an unecessary link bringing "comfort", or something usefull.

>
>> +	if (ret)
>> +		goto err_unregister_device;
>> +
>> +	return 0;
>> +err_unregister_device:
>> +	put_device(&codec->dev);
>> +err_free_codec:
>> +	kfree(codec);
>
> Since the struct is reference counted, the freeing is done in the release
> callback and this leads to a double free.
A yes in the "goto err_unregister_device" case right, while it's necessary in
the "goto err_free_codec" case ... I need to rework that a bit.

>> +int snd_ac97_codec_driver_register(struct ac97_codec_driver *drv)
>> +{
>> +	int ret;
>> +
>> +	drv->driver.bus = &ac97_bus_type;
>> +
>> +	ret = driver_register(&drv->driver);
>> +	if (!ret)
>> +		ac97_rescan_all_controllers();
>
> Rescanning the bus when a new codec driver is registered should not be
> neccessary. The bus is scanned once when the controller is registered, this
> creates the device. The device driver core will take care of binding the
> device to the driver, if the driver is registered after thed evice.
That's because you suppose the initial scanning found all the ac97 codecs.
But that's an incomplete vision as a codec might be powered off at that time and
not seen by the first scanning, while the new scanning will discover it.

>> +static int ac97_ctrl_codecs_unregister(struct ac97_controller *ac97_ctrl)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < AC97_BUS_MAX_CODECS; i++)
>> +		if (ac97_ctrl->codecs[i])
>> +			put_device(&ac97_ctrl->codecs[i]->dev);
>
> This should be device_unregister() to match the device_register() in
> ac97_codec_add().
Right.

>> +
>> +	return 0;
>> +}
>> +
>> +static ssize_t cold_reset_store(struct device *dev,
>> +				struct device_attribute *attr, const char *buf,
>> +				size_t len)
>> +{
>> +	struct ac97_controller *ac97_ctrl = ac97_ctrl_find(dev);
>> +
>> +	if (!dev)
>> +		return -ENODEV;
>
> dev is never NULL here.
Ok.

> And for the ac97_ctrl there is a race condition. It could be unregistered and
> freed after ac97_ctrl_find() returned sucessfully, but before ac97_ctrl->ops
> is used.
A good catch, the ac97_controllers_mutex is missing.

> Same here.
Indeed.

>> +
>> +	ac97_ctrl->ops->warm_reset(ac97_ctrl);
>> +	return len;
>> +}
>> +static DEVICE_ATTR_WO(warm_reset);
>> +
>> +static struct attribute *ac97_controller_device_attrs[] = {
>> +	&dev_attr_cold_reset.attr,
>> +	&dev_attr_warm_reset.attr,
>> +	NULL
>> +};
>
> This adds new userspace ABI that is not documented at the moment.
Very true. And as all userspace interface, it needs to be discussed. If nobody
complains, I'll add the documentation for next patch round.
>
>> +int snd_ac97_controller_register(const struct ac97_controller_ops *ops,
>> +				 struct device *dev,
>> +				 unsigned short slots_available,
>> +				 void **codecs_pdata)
>
> In my opinion this should return a handle to a ac97 controller which can
> then be passed to snd_ac97_controller_unregister(). This is in my opinion
> the better approach rather than looking up the controller by parent device.
This is another "legacy drivers" issue.

The legacy driver (the one probed by platform_data or devicetree) doesn't
necessarily have a private structure to store this ac97_controller pointer. This
enables an "easier" porting of existing drivers.

>> +/**
>> + * snd_ac97_controller_unregister - unregister an ac97 controller
>> + * @dev: the device previously provided to ac97_controller_register()
>> + *
>> + * Returns 0 on success, negative upon error
>
> Unregister must not be able to fail. Hotunplug is one of the core concepts
> of the device driver model and there is really nothing that can be done to
> prevent a device from disappearing, so there is no sensible way of handling
> the error (and your pxa driver modifications simply ignore it as well).
>
> This also means the framework needs to cope with the case where the
> controller is removed and the CODEC devices are still present. All future
> operations should return -ENODEV in that case.
Ah ... that will require a serious modification, and I see your point, so I'll
prepare this for next patchset.
>> +static struct bus_type ac97_bus_type = {
>> +	.name		= "ac97",
>> +	.dev_attrs	= ac97_dev_attrs,
>
> dev_attrs is deprecated in favor of dev_groups (See commit 880ffb5c6).
Ok.

>> index 000000000000..a835f03744bf
>> --- /dev/null
>> +++ b/sound/ac97/codec.c
>> @@ -0,0 +1,15 @@
>> +/*
>> + *  Copyright (C) 2016 Robert Jarzmik <robert.jarzmik@free.fr>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include <sound/ac97_codec.h>
>> +#include <sound/ac97/codec.h>
>> +#include <sound/ac97/controller.h>
>> +#include <linux/device.h>
>> +#include <linux/slab.h>
>> +#include <sound/soc.h>	/* For compat_ac97_* */
>> +
>
> I'm not sure I understand what this file does.
Ah yes, I'll remove it.
It's the future conversion of sound/pci/ac97_codec.c, but it's ... empty so far.

Thanks very much for the very detailed review.

-- 
Robert

^ permalink raw reply

* Re: [alsa-devel] [PATCH 2/9] ALSA: ac97: add an ac97 bus
From: Robert Jarzmik @ 2016-11-08 21:18 UTC (permalink / raw)
  To: Lars-Peter Clausen
  Cc: alsa-devel, patches, Liam Girdwood, linux-kernel, linux-pm,
	Dmitry Torokhov, Takashi Iwai, Haojian Zhuang, Sebastian Reichel,
	Mark Brown, linux-arm-kernel, linux-input, Jaroslav Kysela,
	Lee Jones, Daniel Mack
In-Reply-To: <d57cdfba-da22-652d-3d75-6d1b754cf1d5@metafoo.de>

Lars-Peter Clausen <lars@metafoo.de> writes:

> On 10/26/2016 09:41 PM, Robert Jarzmik wrote:
>> +#define to_ac97_device(d) container_of(d, struct ac97_codec_device, dev)
>> +#define to_ac97_driver(d) container_of(d, struct ac97_codec_driver, driver)
>
> In my opinion these should be inline functions rather than macros as that
> generates much more legible compiler errors e.g. in case there is a type
> mismatch.
Sure, why not.

>> +struct ac97_codec_driver {
>> +	struct device_driver	driver;
>> +	int			(*probe)(struct ac97_codec_device *);
>> +	int			(*remove)(struct ac97_codec_device *);
>> +	int			(*suspend)(struct ac97_codec_device *);
>> +	int			(*resume)(struct ac97_codec_device *);
>> +	void			(*shutdown)(struct ac97_codec_device *);
>
> The suspend, resume and shutdown callbacks are never used. Which is good,
> since all new frameworks should use dev_pm_ops. Just drop the from the struct.
Ok.

>> +/**
>> + * struct ac97_controller - The AC97 controller of the AC-Link
>> + * @ops:		the AC97 operations.
>> + * @controllers:	linked list of all existing controllers.
>> + * @dev:		the device providing the AC97 controller.
>> + * @slots_available:	the mask of accessible/scanable codecs.
>> + * @codecs:		the 4 possible AC97 codecs (NULL if none found).
>> + * @codecs_pdata:	platform_data for each codec (NULL if no pdata).
>> + *
>> + * This structure is internal to AC97 bus, and should not be used by the
>> + * controllers themselves, excepting for using @dev.
>> + */
>> +struct ac97_controller {
>> +	const struct ac97_controller_ops *ops;
>> +	struct list_head controllers;
>> +	struct device *dev;
>
> I'd make the controller itself a struct dev, rather than just having the
> pointer to the parent. This is more idiomatic and matches what other
> subsystems do. It has several advantages, you get proper refcounting of your
> controller structure, the controller gets its own sysfs directory where the
> CODECs appear as children, you don't need the manual sysfs attribute
> creation and removal in ac97_controler_{un,}register().

If you mean having "struct device dev" instead of "struct device *dev", it has
also a drawback : all the legacy platforms have already a probing method, be
that platform data, device-tree or something else.

I'm a bit converned about the conversion toll. Maybe I've not understood your
point fully, so please feel free to explain, with an actual example even better.

>> +	void (*wait)(struct ac97_controller *adrv, int slot);
>> +	void (*init)(struct ac97_controller *adrv, int slot);
>
> Neither wait nor init are ever used.
This is because I've not begun to porting sound/pci/ac97_codec.c into
sound/ac97.

>> +	if ((codec_num < 0) || (codec_num >= AC97_BUS_MAX_CODECS))
>> +		return ERR_PTR(-ERANGE);
>
> I'd make this EINVAL.
Ok.

>> +	adev = container_of(dev, struct ac97_codec_device, dev);
>
> to_ac97_device()
Sure.

>> +	codec_name = kasprintf(GFP_KERNEL, "%s:%d", dev_name(ac97_ctrl->dev),
>> +			       idx);
>> +	codec->dev.init_name = codec_name;
>
> init_name is only for statically allocated devices. Use dev_set_name(dev,
> ...). No need for kasprintf() either as dev_set_name() takes a format string.
I'll try again, I seem to remember having tried that and it failed, but I can't
remember why.

> For this you need to split device_register into device_initialize() and
> device_add(). But usually that is what you want anyway.
Let me try again.

>> +	ret = sysfs_create_link(&codec->dev.kobj, &ac97_ctrl->dev->kobj,
>> +				"ac97_controller");
>
> Since the CODEC is a child of the controller this should not be necessary as
> this just points one directory up. It's like `ln -s .. parent`
This creates :
/sys/bus/ac97/devices/pxa2xx-ac97\:0/ac97_controller

Of course as you pointed out, it's a 'ln -s .. parent' like link, but it seems
to me very unfriendly to have :
 - /sys/bus/ac97/devices/pxa2xx-ac97\:0/../ac97_operations
 - while /sys/bus/ac97/devices/ac97_operations doesn't exist (obviously)

Mmm, I don't know for this one, my mind is not set, it's a bit hard to tell if
it's only an unecessary link bringing "comfort", or something usefull.

>
>> +	if (ret)
>> +		goto err_unregister_device;
>> +
>> +	return 0;
>> +err_unregister_device:
>> +	put_device(&codec->dev);
>> +err_free_codec:
>> +	kfree(codec);
>
> Since the struct is reference counted, the freeing is done in the release
> callback and this leads to a double free.
A yes in the "goto err_unregister_device" case right, while it's necessary in
the "goto err_free_codec" case ... I need to rework that a bit.

>> +int snd_ac97_codec_driver_register(struct ac97_codec_driver *drv)
>> +{
>> +	int ret;
>> +
>> +	drv->driver.bus = &ac97_bus_type;
>> +
>> +	ret = driver_register(&drv->driver);
>> +	if (!ret)
>> +		ac97_rescan_all_controllers();
>
> Rescanning the bus when a new codec driver is registered should not be
> neccessary. The bus is scanned once when the controller is registered, this
> creates the device. The device driver core will take care of binding the
> device to the driver, if the driver is registered after thed evice.
That's because you suppose the initial scanning found all the ac97 codecs.
But that's an incomplete vision as a codec might be powered off at that time and
not seen by the first scanning, while the new scanning will discover it.

>> +static int ac97_ctrl_codecs_unregister(struct ac97_controller *ac97_ctrl)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < AC97_BUS_MAX_CODECS; i++)
>> +		if (ac97_ctrl->codecs[i])
>> +			put_device(&ac97_ctrl->codecs[i]->dev);
>
> This should be device_unregister() to match the device_register() in
> ac97_codec_add().
Right.

>> +
>> +	return 0;
>> +}
>> +
>> +static ssize_t cold_reset_store(struct device *dev,
>> +				struct device_attribute *attr, const char *buf,
>> +				size_t len)
>> +{
>> +	struct ac97_controller *ac97_ctrl = ac97_ctrl_find(dev);
>> +
>> +	if (!dev)
>> +		return -ENODEV;
>
> dev is never NULL here.
Ok.

> And for the ac97_ctrl there is a race condition. It could be unregistered and
> freed after ac97_ctrl_find() returned sucessfully, but before ac97_ctrl->ops
> is used.
A good catch, the ac97_controllers_mutex is missing.

> Same here.
Indeed.

>> +
>> +	ac97_ctrl->ops->warm_reset(ac97_ctrl);
>> +	return len;
>> +}
>> +static DEVICE_ATTR_WO(warm_reset);
>> +
>> +static struct attribute *ac97_controller_device_attrs[] = {
>> +	&dev_attr_cold_reset.attr,
>> +	&dev_attr_warm_reset.attr,
>> +	NULL
>> +};
>
> This adds new userspace ABI that is not documented at the moment.
Very true. And as all userspace interface, it needs to be discussed. If nobody
complains, I'll add the documentation for next patch round.
>
>> +int snd_ac97_controller_register(const struct ac97_controller_ops *ops,
>> +				 struct device *dev,
>> +				 unsigned short slots_available,
>> +				 void **codecs_pdata)
>
> In my opinion this should return a handle to a ac97 controller which can
> then be passed to snd_ac97_controller_unregister(). This is in my opinion
> the better approach rather than looking up the controller by parent device.
This is another "legacy drivers" issue.

The legacy driver (the one probed by platform_data or devicetree) doesn't
necessarily have a private structure to store this ac97_controller pointer. This
enables an "easier" porting of existing drivers.

>> +/**
>> + * snd_ac97_controller_unregister - unregister an ac97 controller
>> + * @dev: the device previously provided to ac97_controller_register()
>> + *
>> + * Returns 0 on success, negative upon error
>
> Unregister must not be able to fail. Hotunplug is one of the core concepts
> of the device driver model and there is really nothing that can be done to
> prevent a device from disappearing, so there is no sensible way of handling
> the error (and your pxa driver modifications simply ignore it as well).
>
> This also means the framework needs to cope with the case where the
> controller is removed and the CODEC devices are still present. All future
> operations should return -ENODEV in that case.
Ah ... that will require a serious modification, and I see your point, so I'll
prepare this for next patchset.
>> +static struct bus_type ac97_bus_type = {
>> +	.name		= "ac97",
>> +	.dev_attrs	= ac97_dev_attrs,
>
> dev_attrs is deprecated in favor of dev_groups (See commit 880ffb5c6).
Ok.

>> index 000000000000..a835f03744bf
>> --- /dev/null
>> +++ b/sound/ac97/codec.c
>> @@ -0,0 +1,15 @@
>> +/*
>> + *  Copyright (C) 2016 Robert Jarzmik <robert.jarzmik@free.fr>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include <sound/ac97_codec.h>
>> +#include <sound/ac97/codec.h>
>> +#include <sound/ac97/controller.h>
>> +#include <linux/device.h>
>> +#include <linux/slab.h>
>> +#include <sound/soc.h>	/* For compat_ac97_* */
>> +
>
> I'm not sure I understand what this file does.
Ah yes, I'll remove it.
It's the future conversion of sound/pci/ac97_codec.c, but it's ... empty so far.

Thanks very much for the very detailed review.

-- 
Robert

^ permalink raw reply

* Re: [PATCH 5/7] pci/msi: Provide pci_alloc_irq_vectors_affinity()
From: Bjorn Helgaas @ 2016-11-08 21:17 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: tglx, axboe, linux-block, linux-pci, linux-kernel
In-Reply-To: <1478544462-9549-6-git-send-email-hch@lst.de>

On Mon, Nov 07, 2016 at 10:47:40AM -0800, Christoph Hellwig wrote:
> From: Christogh Hellwig <hch@lst.de>

s/Christogh/Christoph/ (also below)

> This is a variant of pci_alloc_irq_vectors() that allows passing a
> struct irq_affinity to provide fine-grainded IRQ affinity control.

s/grainded/grained/

> For now this means being able to exclude vectors at the beginning or
> end of the MSI vector space, but it could also be used for any other
> quirks needed in the future (e.g. more vectors than CPUs, or exluding

s/exluding/excluding/

> CPUs from the spreading).
> 
> Signed-off-by: Christogh Hellwig <hch@lst.de>

Acked-by: Bjorn Helgaas <bhelgaas@google.com>

> +int pci_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
> +				   unsigned int max_vecs, unsigned int flags,
> +				   const struct irq_affinity *affd)

> +int pci_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
> +				   unsigned int max_vecs, unsigned int flags,
> +				   const struct irq_affinity *affd);

> +static inline int
> +pci_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
> +			       unsigned int max_vecs, unsigned int flags,
> +			       const struct irq_affinity *aff_desc)

Maybe use the same formal parameter name as in the definition and
declaration above?

^ permalink raw reply

* [PATCH v4] clk: renesas: cpg-mssr: add common R-Car Gen2 support
From: Sergei Shtylyov @ 2016-11-08 21:17 UTC (permalink / raw)
  To: mturquette, linux-clk, sboyd; +Cc: linux-renesas-soc
In-Reply-To: <19700058.YBgRO3SXUI@wasted.cogentembedded.com>

Add the common R-Car  Gen2 (and RZ/G) Clock Pulse Generator / Module Standby
and Software Reset support code, using the CPG/MSSR driver core. 

Based on the proof-of-concept R8A7791 CPG/MSSR patch by Geert Uytterhoeven
<geert+renesas@glider.be>.

Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

---
This patch is against the 'clk-next' branch of CLK group's 'linux.git' repo.

Changes in version 4:
- added a static variable for the PLL0 divisor and a parameter of the same kind
  to rcar_gen2_cpg_init(), used that  variable in rcar_gen2_cpg_clk_register().

Changes in version 3:
- removed  the divisor logic from the PLL0 related code;
- also mentioned R-Car V2H as not having the PLL0CR register.

Changes in version 2:
- added support  for non-existing PLL0CR;
- removed the function reading the mode pins;
- added/used the #define's for PLL0CR.STC;
- used CPG_FRQCRC_ZFC_SHIFT to #define CPG_FRQCRC_ZFC_MASK;
- removed rcar_gen2_read_modemr();
- added Geert's tag.

 drivers/clk/renesas/rcar-gen2-cpg.c |  370 ++++++++++++++++++++++++++++++++++++
 drivers/clk/renesas/rcar-gen2-cpg.h |   43 ++++
 2 files changed, 413 insertions(+)

Index: linux/drivers/clk/renesas/rcar-gen2-cpg.c
===================================================================
--- /dev/null
+++ linux/drivers/clk/renesas/rcar-gen2-cpg.c
@@ -0,0 +1,370 @@
+/*
+ * R-Car Gen2 Clock Pulse Generator
+ *
+ * Copyright (C) 2016 Cogent Embedded Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published
+ * by the Free Software Foundation.
+ */
+
+#include <linux/bug.h>
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/slab.h>
+
+#include "renesas-cpg-mssr.h"
+#include "rcar-gen2-cpg.h"
+
+#define CPG_FRQCRB		0x0004
+#define CPG_FRQCRB_KICK		BIT(31)
+#define CPG_SDCKCR		0x0074
+#define CPG_PLL0CR		0x00d8
+#define CPG_PLL0CR_STC_SHIFT	24
+#define CPG_PLL0CR_STC_MASK	(0x7f << CPG_PLL0CR_STC_SHIFT)
+#define CPG_FRQCRC		0x00e0
+#define CPG_FRQCRC_ZFC_SHIFT	8
+#define CPG_FRQCRC_ZFC_MASK	(0x1f << CPG_FRQCRC_ZFC_SHIFT)
+#define CPG_ADSPCKCR		0x025c
+#define CPG_RCANCKCR		0x0270
+
+static spinlock_t cpg_lock;
+
+/*
+ * Z Clock
+ *
+ * Traits of this clock:
+ * prepare - clk_prepare only ensures that parents are prepared
+ * enable - clk_enable only ensures that parents are enabled
+ * rate - rate is adjustable.  clk->rate = parent->rate * mult / 32
+ * parent - fixed parent.  No clk_set_parent support
+ */
+
+struct cpg_z_clk {
+	struct clk_hw hw;
+	void __iomem *reg;
+	void __iomem *kick_reg;
+};
+
+#define to_z_clk(_hw)	container_of(_hw, struct cpg_z_clk, hw)
+
+static unsigned long cpg_z_clk_recalc_rate(struct clk_hw *hw,
+					   unsigned long parent_rate)
+{
+	struct cpg_z_clk *zclk = to_z_clk(hw);
+	unsigned int mult;
+	unsigned int val;
+
+	val = (readl(zclk->reg) & CPG_FRQCRC_ZFC_MASK) >> CPG_FRQCRC_ZFC_SHIFT;
+	mult = 32 - val;
+
+	return div_u64((u64)parent_rate * mult, 32);
+}
+
+static long cpg_z_clk_round_rate(struct clk_hw *hw, unsigned long rate,
+				 unsigned long *parent_rate)
+{
+	unsigned long prate  = *parent_rate;
+	unsigned int mult;
+
+	if (!prate)
+		prate = 1;
+
+	mult = div_u64((u64)rate * 32, prate);
+	mult = clamp(mult, 1U, 32U);
+
+	return *parent_rate / 32 * mult;
+}
+
+static int cpg_z_clk_set_rate(struct clk_hw *hw, unsigned long rate,
+			      unsigned long parent_rate)
+{
+	struct cpg_z_clk *zclk = to_z_clk(hw);
+	unsigned int mult;
+	u32 val, kick;
+	unsigned int i;
+
+	mult = div_u64((u64)rate * 32, parent_rate);
+	mult = clamp(mult, 1U, 32U);
+
+	if (readl(zclk->kick_reg) & CPG_FRQCRB_KICK)
+		return -EBUSY;
+
+	val = readl(zclk->reg);
+	val &= ~CPG_FRQCRC_ZFC_MASK;
+	val |= (32 - mult) << CPG_FRQCRC_ZFC_SHIFT;
+	writel(val, zclk->reg);
+
+	/*
+	 * Set KICK bit in FRQCRB to update hardware setting and wait for
+	 * clock change completion.
+	 */
+	kick = readl(zclk->kick_reg);
+	kick |= CPG_FRQCRB_KICK;
+	writel(kick, zclk->kick_reg);
+
+	/*
+	 * Note: There is no HW information about the worst case latency.
+	 *
+	 * Using experimental measurements, it seems that no more than
+	 * ~10 iterations are needed, independently of the CPU rate.
+	 * Since this value might be dependent on external xtal rate, pll1
+	 * rate or even the other emulation clocks rate, use 1000 as a
+	 * "super" safe value.
+	 */
+	for (i = 1000; i; i--) {
+		if (!(readl(zclk->kick_reg) & CPG_FRQCRB_KICK))
+			return 0;
+
+		cpu_relax();
+	}
+
+	return -ETIMEDOUT;
+}
+
+static const struct clk_ops cpg_z_clk_ops = {
+	.recalc_rate = cpg_z_clk_recalc_rate,
+	.round_rate = cpg_z_clk_round_rate,
+	.set_rate = cpg_z_clk_set_rate,
+};
+
+static struct clk * __init cpg_z_clk_register(const char *name,
+					      const char *parent_name,
+					      void __iomem *base)
+{
+	struct clk_init_data init;
+	struct cpg_z_clk *zclk;
+	struct clk *clk;
+
+	zclk = kzalloc(sizeof(*zclk), GFP_KERNEL);
+	if (!zclk)
+		return ERR_PTR(-ENOMEM);
+
+	init.name = name;
+	init.ops = &cpg_z_clk_ops;
+	init.flags = 0;
+	init.parent_names = &parent_name;
+	init.num_parents = 1;
+
+	zclk->reg = base + CPG_FRQCRC;
+	zclk->kick_reg = base + CPG_FRQCRB;
+	zclk->hw.init = &init;
+
+	clk = clk_register(NULL, &zclk->hw);
+	if (IS_ERR(clk))
+		kfree(zclk);
+
+	return clk;
+}
+
+static struct clk * __init cpg_rcan_clk_register(const char *name,
+						 const char *parent_name,
+						 void __iomem *base)
+{
+	struct clk_fixed_factor *fixed;
+	struct clk_gate *gate;
+	struct clk *clk;
+
+	fixed = kzalloc(sizeof(*fixed), GFP_KERNEL);
+	if (!fixed)
+		return ERR_PTR(-ENOMEM);
+
+	fixed->mult = 1;
+	fixed->div = 6;
+
+	gate = kzalloc(sizeof(*gate), GFP_KERNEL);
+	if (!gate) {
+		kfree(fixed);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	gate->reg = base + CPG_RCANCKCR;
+	gate->bit_idx = 8;
+	gate->flags = CLK_GATE_SET_TO_DISABLE;
+	gate->lock = &cpg_lock;
+
+	clk = clk_register_composite(NULL, name, &parent_name, 1, NULL, NULL,
+				     &fixed->hw, &clk_fixed_factor_ops,
+				     &gate->hw, &clk_gate_ops, 0);
+	if (IS_ERR(clk)) {
+		kfree(gate);
+		kfree(fixed);
+	}
+
+	return clk;
+}
+
+/* ADSP divisors */
+static const struct clk_div_table cpg_adsp_div_table[] = {
+	{  1,  3 }, {  2,  4 }, {  3,  6 }, {  4,  8 },
+	{  5, 12 }, {  6, 16 }, {  7, 18 }, {  8, 24 },
+	{ 10, 36 }, { 11, 48 }, {  0,  0 },
+};
+
+static struct clk * __init cpg_adsp_clk_register(const char *name,
+						 const char *parent_name,
+						 void __iomem *base)
+{
+	struct clk_divider *div;
+	struct clk_gate *gate;
+	struct clk *clk;
+
+	div = kzalloc(sizeof(*div), GFP_KERNEL);
+	if (!div)
+		return ERR_PTR(-ENOMEM);
+
+	div->reg = base + CPG_ADSPCKCR;
+	div->width = 4;
+	div->table = cpg_adsp_div_table;
+	div->lock = &cpg_lock;
+
+	gate = kzalloc(sizeof(*gate), GFP_KERNEL);
+	if (!gate) {
+		kfree(div);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	gate->reg = base + CPG_ADSPCKCR;
+	gate->bit_idx = 8;
+	gate->flags = CLK_GATE_SET_TO_DISABLE;
+	gate->lock = &cpg_lock;
+
+	clk = clk_register_composite(NULL, name, &parent_name, 1, NULL, NULL,
+				     &div->hw, &clk_divider_ops,
+				     &gate->hw, &clk_gate_ops, 0);
+	if (IS_ERR(clk)) {
+		kfree(gate);
+		kfree(div);
+	}
+
+	return clk;
+}
+
+/* SDHI divisors */
+static const struct clk_div_table cpg_sdh_div_table[] = {
+	{  0,  2 }, {  1,  3 }, {  2,  4 }, {  3,  6 },
+	{  4,  8 }, {  5, 12 }, {  6, 16 }, {  7, 18 },
+	{  8, 24 }, { 10, 36 }, { 11, 48 }, {  0,  0 },
+};
+
+static const struct clk_div_table cpg_sd01_div_table[] = {
+	{  4,  8 }, {  5, 12 }, {  6, 16 }, {  7, 18 },
+	{  8, 24 }, { 10, 36 }, { 11, 48 }, { 12, 10 },
+	{  0,  0 },
+};
+
+static const struct rcar_gen2_cpg_pll_config *cpg_pll_config __initdata;
+static unsigned int cpg_pll0_div __initdata;
+static u32 cpg_mode __initdata;
+
+struct clk * __init rcar_gen2_cpg_clk_register(struct device *dev,
+					       const struct cpg_core_clk *core,
+					       const struct cpg_mssr_info *info,
+					       struct clk **clks,
+					       void __iomem *base)
+{
+	const struct clk_div_table *table = NULL;
+	const struct clk *parent;
+	const char *parent_name;
+	unsigned int mult = 1;
+	unsigned int div = 1;
+	unsigned int shift;
+
+	parent = clks[core->parent];
+	if (IS_ERR(parent))
+		return ERR_CAST(parent);
+
+	parent_name = __clk_get_name(parent);
+
+	switch (core->type) {
+	/* R-Car Gen2 */
+	case CLK_TYPE_GEN2_MAIN:
+		div = cpg_pll_config->extal_div;
+		break;
+
+	case CLK_TYPE_GEN2_PLL0:
+		/*
+		 * PLL0 is a  configurable multiplier clock except on R-Car
+		 * V2H/E2. Register the PLL0 clock as a fixed factor clock for
+		 * now as there's no generic multiplier clock implementation and
+		 * we  currently  have no need to change  the multiplier value.
+		 */
+		mult = cpg_pll_config->pll0_mult;
+		div  = cpg_pll0_div;
+		if (!mult) {
+			u32 pll0cr = readl(base + CPG_PLL0CR);
+
+			mult = (((pll0cr & CPG_PLL0CR_STC_MASK) >>
+				 CPG_PLL0CR_STC_SHIFT) + 1) * 2;
+		}
+		break;
+
+	case CLK_TYPE_GEN2_PLL1:
+		mult = cpg_pll_config->pll1_mult / 2;
+		break;
+
+	case CLK_TYPE_GEN2_PLL3:
+		mult = cpg_pll_config->pll3_mult;
+		break;
+
+	case CLK_TYPE_GEN2_Z:
+		return cpg_z_clk_register(core->name, parent_name, base);
+
+	case CLK_TYPE_GEN2_LB:
+		div = cpg_mode & BIT(18) ? 36 : 24;
+		break;
+
+	case CLK_TYPE_GEN2_ADSP:
+		return cpg_adsp_clk_register(core->name, parent_name, base);
+
+	case CLK_TYPE_GEN2_SDH:
+		table = cpg_sdh_div_table;
+		shift = 8;
+		break;
+
+	case CLK_TYPE_GEN2_SD0:
+		table = cpg_sd01_div_table;
+		shift = 4;
+		break;
+
+	case CLK_TYPE_GEN2_SD1:
+		table = cpg_sd01_div_table;
+		shift = 0;
+		break;
+
+	case CLK_TYPE_GEN2_QSPI:
+		div = (cpg_mode & (BIT(3) | BIT(2) | BIT(1))) == BIT(2) ?
+		      8 : 10;
+		break;
+
+	case CLK_TYPE_GEN2_RCAN:
+		return cpg_rcan_clk_register(core->name, parent_name, base);
+
+	default:
+		return ERR_PTR(-EINVAL);
+	}
+
+	if (!table)
+		return clk_register_fixed_factor(NULL, core->name, parent_name,
+						 0, mult, div);
+	else
+		return clk_register_divider_table(NULL, core->name,
+						  parent_name, 0,
+						  base + CPG_SDCKCR, shift, 4,
+						  0, table, &cpg_lock);
+}
+
+int __init rcar_gen2_cpg_init(const struct rcar_gen2_cpg_pll_config *config,
+			      unsigned int pll0_div)
+{
+	cpg_pll_config = config;
+	cpg_pll0_div = pll0_div;
+
+	spin_lock_init(&cpg_lock);
+
+	return 0;
+}
Index: linux/drivers/clk/renesas/rcar-gen2-cpg.h
===================================================================
--- /dev/null
+++ linux/drivers/clk/renesas/rcar-gen2-cpg.h
@@ -0,0 +1,43 @@
+/*
+ * R-Car Gen2 Clock Pulse Generator
+ *
+ * Copyright (C) 2016 Cogent Embedded Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published
+ * by the Free Software Foundation; version 2 of the License.
+ */
+
+#ifndef __CLK_RENESAS_RCAR_GEN2_CPG_H__
+#define __CLK_RENESAS_RCAR_GEN2_CPG_H__
+
+enum rcar_gen2_clk_types {
+	CLK_TYPE_GEN2_MAIN = CLK_TYPE_CUSTOM,
+	CLK_TYPE_GEN2_PLL0,
+	CLK_TYPE_GEN2_PLL1,
+	CLK_TYPE_GEN2_PLL3,
+	CLK_TYPE_GEN2_Z,
+	CLK_TYPE_GEN2_LB,
+	CLK_TYPE_GEN2_ADSP,
+	CLK_TYPE_GEN2_SDH,
+	CLK_TYPE_GEN2_SD0,
+	CLK_TYPE_GEN2_SD1,
+	CLK_TYPE_GEN2_QSPI,
+	CLK_TYPE_GEN2_RCAN,
+};
+
+struct rcar_gen2_cpg_pll_config {
+	unsigned int extal_div;
+	unsigned int pll1_mult;
+	unsigned int pll3_mult;
+	unsigned int pll0_mult;		/* leave as zero if PLL0CR exists */
+};
+
+struct clk *rcar_gen2_cpg_clk_register(struct device *dev,
+				       const struct cpg_core_clk *core,
+				       const struct cpg_mssr_info *info,
+				       struct clk **clks, void __iomem *base);
+int rcar_gen2_cpg_init(const struct rcar_gen2_cpg_pll_config *config,
+		       unsigned int pll0_div);
+
+#endif

^ permalink raw reply

* Re: [PATCH] x86/MCE: Remove MCP_TIMESTAMP
From: Thomas Gleixner @ 2016-11-08 21:14 UTC (permalink / raw)
  To: Borislav Petkov; +Cc: Luck, Tony, linux-edac, X86 ML, LKML
In-Reply-To: <20161108210819.gx7bfw7pfx4e6tf4@pd.tnic>

On Tue, 8 Nov 2016, Borislav Petkov wrote:
> On Tue, Nov 08, 2016 at 09:39:02PM +0100, Thomas Gleixner wrote:
> > That does not make any sense. Where is m.tsc initialized? I couldn't find
> > any place which does, except this and the conditional clear farther down in
> > that function.
> 
> mce_gather_info->mce_setup does
> 
> m->tsc = rdtsc();
> 
> And we do that *everytime* but then we go and clear the damn thing. I
> know, I know, I wanted to flip that logic too and read the TSC *only*
> when we want a precise timestamp but that would require more changes as
> mce_setup() is used at a bunch of places.

And yes, you should spend the extra cycles. Adding a flags argument to
mce_setup() and propagate it through the various callsites shouldn't be
that hard and would make the stuff obvious instead of obfuscated.

Thanks,

	tglx

^ permalink raw reply

* Re: [v4.9-rc4] dvb-usb/cinergyT2 NULL pointer dereference
From: Benjamin Larsson @ 2016-11-08 21:15 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Linus Torvalds
  Cc: Jörg Otte, Patrick Boettcher, Mauro Carvalho Chehab,
	Linux Kernel Mailing List, Linux Media Mailing List
In-Reply-To: <20161108182215.41f1f3d2@vento.lan>

On 11/08/2016 09:22 PM, Mauro Carvalho Chehab wrote:
> Em Tue, 8 Nov 2016 10:42:03 -0800
> Linus Torvalds <torvalds@linux-foundation.org> escreveu:
>
>> On Sun, Nov 6, 2016 at 7:40 AM, Jörg Otte <jrg.otte@gmail.com> wrote:
>>> Since v4.9-rc4 I get following crash in dvb-usb-cinergyT2 module.
>>
>> Looks like it's commit 5ef8ed0e5608f ("[media] cinergyT2-core: don't
>> do DMA on stack"), which movced the DMA data array from the stack to
>> the "private" pointer. In the process it also added serialization in
>> the form of "data_mutex", but and now it oopses on that mutex because
>> the private pointer is NULL.
>>
>> It looks like the "->private" pointer is allocated in dvb_usb_adapter_init()
>>
>> cinergyt2_usb_probe ->
>>   dvb_usb_device_init ->
>>     dvb_usb_init() ->
>>       dvb_usb_adapter_init()
>>
>> but the dvb_usb_init() function calls dvb_usb_device_power_ctrl()
>> (which calls the "power_ctrl" function, which is
>> cinergyt2_power_ctrl() for that drive) *before* it initializes the
>> private field.
>>
>> Mauro, Patrick, could dvb_usb_adapter_init() be called earlier, perhaps?
>
> Calling it earlier won't work, as we need to load the firmware before
> sending the power control commands on some devices.
>
> Probably the best here is to pass an extra optional function parameter
> that will initialize the mutex before calling any functions.
>
> Btw, if it broke here, the DMA fixes will likely break on other drivers.
> So, after Jörg tests this patch, I'll work on a patch series addressing
> this issue on the other drivers I touched.
>
> Regards,
> Mauro

Just for reference I got the following call trace a week ago. I looks 
like this confirms that other drivers are affected also.

MvH
Benjamin Larsson


dvb-usb: found a 'Mygica T230 DVB-T/T2/C' in warm state.
BUG: unable to handle kernel NULL pointer dereference at           (null)
IP: [<ffffffff8182f688>] __mutex_lock_slowpath+0x98/0x130
PGD 0
Oops: 0002 [#1] SMP
Modules linked in: dvb_usb_cxusb(OE+) dib0070(OE) dvb_usb_rtl28xxu(OE+) 
dvb_usb_dw2102(OE+) dvb_usb(OE) dvb_usb_v2(OE) dvb_core(OE) rc_core(OE) 
ipmi_ssif media(OE) lpc_sch joydev input_leds i2c_ismt ipmi_si 
8250_fintek ipmi_msghandler shpchp mac_hid kvm_intel kvm irqbypass 
ib_iser rdma_cm iw_cm ib_cm ib_sa ib_mad ib_core ib_addr iscsi_tcp 
libiscsi_tcp libiscsi scsi_transport_iscsi autofs4 btrfs raid10 
multipath linear raid456 async_raid6_recov async_memcpy async_pq 
async_xor async_tx xor raid6_pq libcrc32c raid0 raid1 hid_generic igb 
dca usbhid ptp pps_core i2c_algo_bit hid ahci libahci fjes
CPU: 3 PID: 571 Comm: systemd-udevd Tainted: G           OE 
4.4.0-45-generic #66-Ubuntu
Hardware name: Supermicro X9SBAA/X9SBAA, BIOS 1.00 04/29/2014
task: ffff880231678000 ti: ffff880232048000 task.ti: ffff880232048000
RIP: 0010:[<ffffffff8182f688>]  [<ffffffff8182f688>] 
__mutex_lock_slowpath+0x98/0x130
RSP: 0018:ffff88023204b920  EFLAGS: 00010282
RAX: 0000000000000000 RBX: ffff88023152fea8 RCX: 0000000000000000
RDX: 0000000000000001 RSI: ffff880231678000 RDI: ffff88023152feac
RBP: ffff88023204b970 R08: ffff880232048000 R09: 0000000000000000
R10: 0000000000000325 R11: 0000000000000000 R12: ffff88023152feac
R13: ffff880231678000 R14: 00000000ffffffff R15: ffff88023152feb0
FS:  00007fc5390d68c0(0000) GS:ffff88023fd80000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000000 CR3: 000000023262f000 CR4: 00000000000006e0
Stack:
ffff88023152feb0 0000000000000000 ffff88023204b978 0000000027226a0c
01ff880200000000 ffff88023152fea8 ffff88023152fe40 ffff88023152fea8
ffff8800ba9e4000 0000000000000000 ffff88023204b988 ffffffff8182f73f
Call Trace:
[<ffffffff8182f73f>] mutex_lock+0x1f/0x30
[<ffffffffc04793da>] cxusb_ctrl_msg+0x5a/0xe0 [dvb_usb_cxusb]
[<ffffffffc0479ac8>] cxusb_power_ctrl+0x58/0x60 [dvb_usb_cxusb]
[<ffffffffc047b0db>] cxusb_d680_dmb_power_ctrl+0x2b/0x90 [dvb_usb_cxusb]
[<ffffffffc050e5a1>] dvb_usb_device_power_ctrl.part.4+0x31/0x50 [dvb_usb]
[<ffffffffc050e5ea>] dvb_usb_device_power_ctrl+0x2a/0x40 [dvb_usb]
[<ffffffffc050e844>] dvb_usb_device_init+0x244/0x750 [dvb_usb]
[<ffffffff8128baca>] ? kernfs_activate+0x7a/0xe0
[<ffffffffc0479360>] cxusb_probe+0x260/0x280 [dvb_usb_cxusb]
[<ffffffff8161e858>] usb_probe_interface+0x118/0x2f0
[<ffffffff815550d2>] driver_probe_device+0x222/0x4a0
[<ffffffff815553d4>] __driver_attach+0x84/0x90
[<ffffffff81555350>] ? driver_probe_device+0x4a0/0x4a0
[<ffffffff81552cfc>] bus_for_each_dev+0x6c/0xc0
[<ffffffff8155488e>] driver_attach+0x1e/0x20
[<ffffffff815543cb>] bus_add_driver+0x1eb/0x280
[<ffffffff81555ce0>] driver_register+0x60/0xe0
[<ffffffff8161d224>] usb_register_driver+0x84/0x140
[<ffffffffc03d8000>] ? 0xffffffffc03d8000
[<ffffffffc03d801e>] cxusb_driver_init+0x1e/0x1000 [dvb_usb_cxusb]
[<ffffffff81002123>] do_one_initcall+0xb3/0x200
[<ffffffff811cfa51>] ? __vunmap+0x91/0xe0
[<ffffffff811ebc03>] ? kmem_cache_alloc_trace+0x183/0x1f0
[<ffffffff811ec9fa>] ? kfree+0x13a/0x150
[<ffffffff8118caa3>] do_init_module+0x5f/0x1cf
[<ffffffff8110a2ff>] load_module+0x166f/0x1c10
[<ffffffff811068a0>] ? __symbol_put+0x60/0x60
[<ffffffff81213760>] ? kernel_read+0x50/0x80
[<ffffffff8110aae4>] SYSC_finit_module+0xb4/0xe0
[<ffffffff8110ab2e>] SyS_finit_module+0xe/0x10
[<ffffffff818318b2>] entry_SYSCALL_64_fastpath+0x16/0x71
Code: e8 ae 1f 00 00 8b 03 83 f8 01 0f 84 94 00 00 00 48 8b 43 10 4c 8d 
7b 08 48 89 63 10 41 be ff ff ff ff 4c 89 3c 24 48 89 44 24 08 <48> 89 
20 4c 89 6c 24 10 eb 1f 49 c7 45 00 02 00 00 00 4c 89 e7
RIP  [<ffffffff8182f688>] __mutex_lock_slowpath+0x98/0x130
  RSP <ffff88023204b920>
CR2: 0000000000000000
---[ end trace b28b9190ee8e4917 ]---


^ permalink raw reply

* Re: How to handle GPIO differences between P8 and P9
From: Mine @ 2016-11-08 21:13 UTC (permalink / raw)
  To: Xo Wang; +Cc: Patrick Williams, OpenBMC Maillist
In-Reply-To: <CAPH-5w6jxPPBYPSh1NQF-vg2w2HJ7ZU_r50ttSsUsXDMPS3_5A@mail.gmail.com>

On Mon, Nov 7, 2016 at 3:53 PM, Xo Wang <xow@google.com> wrote:
> On Mon, Nov 7, 2016 at 9:01 AM, Mine <mine260309@gmail.com> wrote:
>> On Mon, Nov 7, 2016 at 10:19 AM, Patrick Williams <patrick@stwcx.xyz> wrote:
>>> On Mon, Nov 07, 2016 at 05:10:59PM +0800, Yi TZ Li wrote:
>>>> I committed a patch: https://gerrit.openbmc-project.xyz/#/c/1019/
>>>> to add a getSystemName() dbus method in org.obmc.managers.System interface.
>>>> There are several places in skeleton requires fix for a specific system.
>>>>
>>>> Other way be add a compile flag in yocto when building Skeleton.
>>>
>>> We are really trying to avoid "if (machine_type == ...)" type code.  This
>>> becomes a gigantic problem when we are talking about supporting 3 dozen
>>> machines.
>>>
>>> --
>>> Patrick Williams
>>
>> Yup, I prefer not to write `if (machine_type == ...)` type code, nor
>> to have a compile flag (it just becomes `#if xxx` or `#ifdef xxx`).
>>
>> The temp fix by not defining `IDBTN` will not only cause error log,
>> but also prevent the code in `op-hostctl/control_host_obj.c` to work,
>> because it will get error during opening the GPIO, and jump out,
>> resulting in error: "GPIO sequence failed".
>>
>> If we are targeting for booting P9 up, I would suggest a temp fix by
>> defining `IDBTN` for both Witherspoon and Romulus.
>> It will not block power on, but may break a function related to LED.
>>
>> --
>> BRs,
>> Lei YU
>> _______________________________________________
>> openbmc mailing list
>> openbmc@lists.ozlabs.org
>> https://lists.ozlabs.org/listinfo/openbmc
>
> We had a similar issue in the op-pwrctl where the source hard-coded
> the system net names. There I put in a short-term solution of adding a
> function-specific abstraction over the net names into the System.py
> config, removing the per-machine patches for name and polarity:
> https://gerrit.openbmc-project.xyz/#/c/606/
> https://gerrit.openbmc-project.xyz/#/c/689/
>
> It's not difficult to extend POWER_CONFIG for GPIO functions that only
> exist on one machine, like so:
> https://gerrit.openbmc-project.xyz/#/c/608/3
>
> With the abstraction taking place in the Python machine config, we
> don't need per-machine dispatch in the user interface (op-pwrctl,
> op-hostctl, op-pwr-button, etc).
>
> Does that work for you in the short term?
>
> cheers
> xo

Yup, that could be a solution. It's just the GPIOs are not for power control,
so following the solution would probably introduce similar code to
hostctl_gpio.h/c.

Maybe I can combine the power_gpio and hostctl_gpio, so they share the same
code?

Something like:

GPIO_CONFIG = {
  'power_gpios': {
    'latch_out' : 'BMC_UCD_LATCH_LE',
    'power_good_in' : 'SYS_PWROK_BUFF',
    'power_up_outs' : [
        ('SOFTWARE_PGOOD', True),
        ('BMC_POWER_UP', True),
    ],
    'reset_outs' : [
    ],
  },
  'hostctl_gpios': {
    'fsi_data' : 'FSI_DATA',
    ...
    'optionals' : [
      ('BMC_THROTTLE', True),
      ('IDBTN', False),
    ],
  }
}

What do you think?

--
BRs,
Lei YU

^ permalink raw reply

* Re: [RFC][ATCH 1/3] ptrace: Remove maxargs from task_current_syscall()
From: Steven Rostedt @ 2016-11-08 21:13 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Andrew Morton, linux-arch, Oleg Nesterov, Ingo Molnar,
	Linux Kernel Mailing List, Peter Zijlstra, Roland McGrath,
	Linus Torvalds
In-Reply-To: <CALCETrXOTUOfq3m1fXVUv1bxmF2YKHrkEX2cuHGCAESb5Fyhpg@mail.gmail.com>

On Tue, 8 Nov 2016 13:06:35 -0800
Andy Lutomirski <luto@amacapital.net> wrote:
 
>   Or you could
> embed seccomp_data in a bigger structure.

I was thinking about doing just that.

But for now, I may just use seccomp_data and when we want to add more,
we can do the embedding.

-- Steve

^ permalink raw reply

* [PATCH v4 3/9] selinux lsm IB/core: Implement LSM notification system
From: Dan Jurgens @ 2016-11-08 21:06 UTC (permalink / raw)
  To: chrisw, paul, sds, eparis, dledford, sean.hefty, hal.rosenstock
  Cc: selinux, linux-security-module, linux-rdma, yevgenyp, liranl,
	leonro, Daniel Jurgens
In-Reply-To: <1478639185-47521-1-git-send-email-danielj@mellanox.com>

From: Daniel Jurgens <danielj@mellanox.com>

Add a generic notificaiton mechanism in the LSM. Interested consumers
can register a callback with the LSM and security modules can produce
events.

Because access to Infiniband QPs are enforced in the setup phase of a
connection security should be enforced again if the policy changes.
Register infiniband devices for policy change notification and check all
QPs on that device when the notification is received.

Add a call to the notification mechanism from SELinux when the AVC
cache changes or setenforce is cleared.

issue: 736423
Change-Id: I9f5a13026c9219105ee5e0cd99edf8eabdafe946
Signed-off-by: Daniel Jurgens <danielj@mellanox.com>

---
v2:
- new patch that has the generic notification, replaces selinux and
  IB/core patches related to the ib_flush callback. Yuval Shaia and Paul
  Moore

v3:
- use notifier chains. Paul Moore

v4:
- Seperate avc callback for LSM notifier. Paul Moore

Signed-off-by: Daniel Jurgens <danielj@mellanox.com>
---
 drivers/infiniband/core/device.c | 53 ++++++++++++++++++++++++++++++++++++++++
 include/linux/security.h         |  8 ++++++
 security/security.c              | 20 +++++++++++++++
 security/selinux/hooks.c         | 11 +++++++++
 security/selinux/selinuxfs.c     |  2 ++
 5 files changed, 94 insertions(+)

diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
index 5b42e83..7b6fd06 100644
--- a/drivers/infiniband/core/device.c
+++ b/drivers/infiniband/core/device.c
@@ -39,6 +39,8 @@
 #include <linux/init.h>
 #include <linux/mutex.h>
 #include <linux/netdevice.h>
+#include <linux/security.h>
+#include <linux/notifier.h>
 #include <rdma/rdma_netlink.h>
 #include <rdma/ib_addr.h>
 #include <rdma/ib_cache.h>
@@ -82,6 +84,14 @@ struct ib_client_data {
 static DEFINE_MUTEX(device_mutex);
 static DECLARE_RWSEM(lists_rwsem);
 
+static int ib_security_change(struct notifier_block *nb, unsigned long event,
+			      void *lsm_data);
+static void ib_policy_change_task(struct work_struct *work);
+static DECLARE_WORK(ib_policy_change_work, ib_policy_change_task);
+
+static struct notifier_block ibdev_lsm_nb = {
+	.notifier_call = ib_security_change,
+};
 
 static int ib_device_check_mandatory(struct ib_device *device)
 {
@@ -344,6 +354,40 @@ static int setup_port_pkey_list(struct ib_device *device)
 	return 0;
 }
 
+static void ib_policy_change_task(struct work_struct *work)
+{
+	struct ib_device *dev;
+
+	down_read(&lists_rwsem);
+	list_for_each_entry(dev, &device_list, core_list) {
+		int i;
+
+		for (i = rdma_start_port(dev); i <= rdma_end_port(dev); i++) {
+			u64 sp;
+			int ret = ib_get_cached_subnet_prefix(dev,
+							      i,
+							      &sp);
+
+			WARN_ONCE(ret,
+				  "ib_get_cached_subnet_prefix err: %d, this should never happen here\n",
+				  ret);
+			ib_security_cache_change(dev, i, sp);
+		}
+	}
+	up_read(&lists_rwsem);
+}
+
+static int ib_security_change(struct notifier_block *nb, unsigned long event,
+			      void *lsm_data)
+{
+	if (event != LSM_POLICY_CHANGE)
+		return NOTIFY_DONE;
+
+	schedule_work(&ib_policy_change_work);
+
+	return NOTIFY_OK;
+}
+
 /**
  * ib_register_device - Register an IB device with IB core
  * @device:Device to register
@@ -1075,10 +1119,18 @@ static int __init ib_core_init(void)
 		goto err_sa;
 	}
 
+	ret = register_lsm_notifier(&ibdev_lsm_nb);
+	if (ret) {
+		pr_warn("Couldn't register LSM notifier. ret %d\n", ret);
+		goto err_ibnl_clients;
+	}
+
 	ib_cache_setup();
 
 	return 0;
 
+err_ibnl_clients:
+	ib_remove_ibnl_clients();
 err_sa:
 	ib_sa_cleanup();
 err_mad:
@@ -1098,6 +1150,7 @@ static int __init ib_core_init(void)
 
 static void __exit ib_core_cleanup(void)
 {
+	unregister_lsm_notifier(&ibdev_lsm_nb);
 	ib_cache_cleanup();
 	ib_remove_ibnl_clients();
 	ib_sa_cleanup();
diff --git a/include/linux/security.h b/include/linux/security.h
index 342ca4c..d8c29cd 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -69,6 +69,14 @@
 struct user_namespace;
 struct timezone;
 
+enum lsm_event {
+	LSM_POLICY_CHANGE,
+};
+
+int call_lsm_notifier(enum lsm_event event, void *data);
+int register_lsm_notifier(struct notifier_block *nb);
+int unregister_lsm_notifier(struct notifier_block *nb);
+
 /* These functions are in security/commoncap.c */
 extern int cap_capable(const struct cred *cred, struct user_namespace *ns,
 		       int cap, int audit);
diff --git a/security/security.c b/security/security.c
index 7d3bf2f..40326d4 100644
--- a/security/security.c
+++ b/security/security.c
@@ -33,6 +33,8 @@
 /* Maximum number of letters for an LSM name string */
 #define SECURITY_NAME_MAX	10
 
+static ATOMIC_NOTIFIER_HEAD(lsm_notifier_chain);
+
 /* Boot-time LSM user choice */
 static __initdata char chosen_lsm[SECURITY_NAME_MAX + 1] =
 	CONFIG_DEFAULT_SECURITY;
@@ -98,6 +100,24 @@ int __init security_module_enable(const char *module)
 	return !strcmp(module, chosen_lsm);
 }
 
+int call_lsm_notifier(enum lsm_event event, void *data)
+{
+	return atomic_notifier_call_chain(&lsm_notifier_chain, event, data);
+}
+EXPORT_SYMBOL(call_lsm_notifier);
+
+int register_lsm_notifier(struct notifier_block *nb)
+{
+	return atomic_notifier_chain_register(&lsm_notifier_chain, nb);
+}
+EXPORT_SYMBOL(register_lsm_notifier);
+
+int unregister_lsm_notifier(struct notifier_block *nb)
+{
+	return atomic_notifier_chain_unregister(&lsm_notifier_chain, nb);
+}
+EXPORT_SYMBOL(unregister_lsm_notifier);
+
 /*
  * Hook list operation macros.
  *
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 09fd610..7578198 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -170,6 +170,14 @@ static int selinux_netcache_avc_callback(u32 event)
 	return 0;
 }
 
+static int selinux_lsm_notifier_avc_callback(u32 event)
+{
+	if (event == AVC_CALLBACK_RESET) {
+		call_lsm_notifier(LSM_POLICY_CHANGE, NULL);
+	}
+	return 0;
+}
+
 /*
  * initialise the security for the init task
  */
@@ -6325,6 +6333,9 @@ static __init int selinux_init(void)
 	if (avc_add_callback(selinux_netcache_avc_callback, AVC_CALLBACK_RESET))
 		panic("SELinux: Unable to register AVC netcache callback\n");
 
+	if (avc_add_callback(selinux_lsm_notifier_avc_callback, AVC_CALLBACK_RESET))
+		panic("SELinux: Unable to register AVC LSM notifier callback\n");
+
 	if (selinux_enforcing)
 		printk(KERN_DEBUG "SELinux:  Starting in enforcing mode\n");
 	else
diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
index 72c145d..d3f9192 100644
--- a/security/selinux/selinuxfs.c
+++ b/security/selinux/selinuxfs.c
@@ -177,6 +177,8 @@ static ssize_t sel_write_enforce(struct file *file, const char __user *buf,
 			avc_ss_reset(0);
 		selnl_notify_setenforce(selinux_enforcing);
 		selinux_status_update_setenforce(selinux_enforcing);
+		if (!selinux_enforcing)
+			call_lsm_notifier(LSM_POLICY_CHANGE, NULL);
 	}
 	length = count;
 out:
-- 
1.8.3.1

^ permalink raw reply related

* Re: [Qemu-devel] [PATCH v11 01/22] vfio: Mediated device Core driver
From: Kirti Wankhede @ 2016-11-08 21:06 UTC (permalink / raw)
  To: alex.williamson, pbonzini, kraxel, cjia, qemu-devel, kvm,
	kevin.tian, jike.song, bjsdjshi, linux-kernel
In-Reply-To: <20161108092552.GA2090@bjsdjshi@linux.vnet.ibm.com>



On 11/8/2016 2:55 PM, Dong Jia Shi wrote:
> * Kirti Wankhede <kwankhede@nvidia.com> [2016-11-05 02:40:35 +0530]:
> 
> Hi Kirti,
> 
> [...]
>> diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig
>> index da6e2ce77495..23eced02aaf6 100644
>> --- a/drivers/vfio/Kconfig
>> +++ b/drivers/vfio/Kconfig
>> @@ -48,4 +48,5 @@ menuconfig VFIO_NOIOMMU
>>
>>  source "drivers/vfio/pci/Kconfig"
>>  source "drivers/vfio/platform/Kconfig"
>> +source "drivers/vfio/mdev/Kconfig"
>>  source "virt/lib/Kconfig"
>> diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile
>> index 7b8a31f63fea..4a23c13b6be4 100644
>> --- a/drivers/vfio/Makefile
>> +++ b/drivers/vfio/Makefile
>> @@ -7,3 +7,4 @@ obj-$(CONFIG_VFIO_IOMMU_SPAPR_TCE) += vfio_iommu_spapr_tce.o
>>  obj-$(CONFIG_VFIO_SPAPR_EEH) += vfio_spapr_eeh.o
>>  obj-$(CONFIG_VFIO_PCI) += pci/
>>  obj-$(CONFIG_VFIO_PLATFORM) += platform/
>> +obj-$(CONFIG_VFIO_MDEV) += mdev/
>> diff --git a/drivers/vfio/mdev/Kconfig b/drivers/vfio/mdev/Kconfig
>> new file mode 100644
>> index 000000000000..303c14ce2847
>> --- /dev/null
>> +++ b/drivers/vfio/mdev/Kconfig
>> @@ -0,0 +1,10 @@
>> +
>> +config VFIO_MDEV
>> +    tristate "Mediated device driver framework"
>> +    depends on VFIO
>> +    default n
>> +    help
>> +        Provides a framework to virtualize devices.
>> +	See Documentation/vfio-mdev/vfio-mediated-device.txt for more details.
> We don't have this doc at this point of time.
> 

Yes, but I have this doc in this patch series.

>> +
>> +        If you don't know what do here, say N.
>> diff --git a/drivers/vfio/mdev/Makefile b/drivers/vfio/mdev/Makefile
>> new file mode 100644
>> index 000000000000..31bc04801d94
>> --- /dev/null
>> +++ b/drivers/vfio/mdev/Makefile
>> @@ -0,0 +1,4 @@
>> +
>> +mdev-y := mdev_core.o mdev_sysfs.o mdev_driver.o
>> +
>> +obj-$(CONFIG_VFIO_MDEV) += mdev.o
>> diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
>> new file mode 100644
>> index 000000000000..54c59f325336
>> --- /dev/null
>> +++ b/drivers/vfio/mdev/mdev_core.c
> [...]
> 
>> +
>> +/*
>> + * mdev_device_remove_ops gets called from sysfs's 'remove' and when parent
>> + * device is being unregistered from mdev device framework.
>> + * - 'force_remove' is set to 'false' when called from sysfs's 'remove' which
>> + *   indicates that if the mdev device is active, used by VMM or userspace
>> + *   application, vendor driver could return error then don't remove the device.
>> + * - 'force_remove' is set to 'true' when called from mdev_unregister_device()
>> + *   which indicate that parent device is being removed from mdev device
>> + *   framework so remove mdev device forcefully.
>> + */
>> +static int mdev_device_remove_ops(struct mdev_device *mdev, bool force_remove)
> ?
> s/force_remove/force/
> 
>> +{
>> +	struct parent_device *parent = mdev->parent;
>> +	int ret;
>> +
>> +	/*
>> +	 * Vendor driver can return error if VMM or userspace application is
>> +	 * using this mdev device.
>> +	 */
>> +	ret = parent->ops->remove(mdev);
>> +	if (ret && !force_remove)
>> +		return -EBUSY;
>> +
>> +	sysfs_remove_groups(&mdev->dev.kobj, parent->ops->mdev_attr_groups);
>> +	return 0;
>> +}
>> +
>> +static int mdev_device_remove_cb(struct device *dev, void *data)
>> +{
>> +	if (!dev_is_mdev(dev))
>> +		return 0;
>> +
>> +	return mdev_device_remove(dev, data ? *(bool *)data : true);
>> +}
>> +
>> +/*
>> + * mdev_register_device : Register a device
>> + * @dev: device structure representing parent device.
>> + * @ops: Parent device operation structure to be registered.
>> + *
>> + * Add device to list of registered parent devices.
>> + * Returns a negative value on error, otherwise 0.
>> + */
>> +int mdev_register_device(struct device *dev, const struct parent_ops *ops)
>> +{
>> +	int ret;
>> +	struct parent_device *parent;
>> +
>> +	/* check for mandatory ops */
>> +	if (!ops || !ops->create || !ops->remove || !ops->supported_type_groups)
>> +		return -EINVAL;
>> +
>> +	dev = get_device(dev);
>> +	if (!dev)
>> +		return -EINVAL;
>> +
>> +	mutex_lock(&parent_list_lock);
>> +
>> +	/* Check for duplicate */
>> +	parent = __find_parent_device(dev);
>> +	if (parent) {
>> +		ret = -EEXIST;
>> +		goto add_dev_err;
>> +	}
>> +
>> +	parent = kzalloc(sizeof(*parent), GFP_KERNEL);
>> +	if (!parent) {
>> +		ret = -ENOMEM;
>> +		goto add_dev_err;
>> +	}
>> +
>> +	kref_init(&parent->ref);
>> +	mutex_init(&parent->lock);
>> +
>> +	parent->dev = dev;
>> +	parent->ops = ops;
>> +
>> +	if (!mdev_bus_compat_class) {
>> +		mdev_bus_compat_class = class_compat_register("mdev_bus");
>> +		if (!mdev_bus_compat_class) {
>> +			ret = -ENOMEM;
>> +			goto add_dev_err;
>> +		}
>> +	}
>> +
>> +	ret = parent_create_sysfs_files(parent);
>> +	if (ret)
>> +		goto add_dev_err;
>> +
>> +	ret = class_compat_create_link(mdev_bus_compat_class, dev, NULL);
>> +	if (ret)
>> +		dev_warn(dev, "Failed to create compatibility class link\n");
>> +
>> +	list_add(&parent->next, &parent_list);
>> +	mutex_unlock(&parent_list_lock);
>> +
>> +	dev_info(dev, "MDEV: Registered\n");
>> +	return 0;
>> +
>> +add_dev_err:
>> +	mutex_unlock(&parent_list_lock);
>> +	if (parent)
>> +		mdev_put_parent(parent);
> Why do this? I don't find the place that you call mdev_get_parent above.
> 

	kref_init(&parent->ref);
Above increments the ref_count, so mdev_put_parent() should be called if
anything fails.

>> +	else
>> +		put_device(dev);
> Shouldn't we always do this?
> 

When mdev_put_parent() is called, its release function do this. So if
mdev_put_parent() is called, we don't need this.

>> +	return ret;
>> +}
>> +EXPORT_SYMBOL(mdev_register_device);
>> +
>> +/*
>> + * mdev_unregister_device : Unregister a parent device
>> + * @dev: device structure representing parent device.
>> + *
>> + * Remove device from list of registered parent devices. Give a chance to free
>> + * existing mediated devices for given device.
>> + */
>> +
>> +void mdev_unregister_device(struct device *dev)
>> +{
>> +	struct parent_device *parent;
>> +	bool force_remove = true;
>> +
>> +	mutex_lock(&parent_list_lock);
>> +	parent = __find_parent_device(dev);
>> +
>> +	if (!parent) {
>> +		mutex_unlock(&parent_list_lock);
>> +		return;
>> +	}
>> +	dev_info(dev, "MDEV: Unregistering\n");
>> +
>> +	list_del(&parent->next);
>> +	class_compat_remove_link(mdev_bus_compat_class, dev, NULL);
>> +
>> +	device_for_each_child(dev, (void *)&force_remove,
>> +			      mdev_device_remove_cb);
>> +
>> +	parent_remove_sysfs_files(parent);
>> +
>> +	mutex_unlock(&parent_list_lock);
>> +	mdev_put_parent(parent);
> We also need to call put_device(dev) here since we have a get_device
> during registration, no?
> Or I must miss something...
> 

As explained above.

>> +}
>> +EXPORT_SYMBOL(mdev_unregister_device);
>> +
> [...]
> 
>> +static int __init mdev_init(void)
>> +{
>> +	int ret;
>> +
>> +	ret = mdev_bus_register();
>> +	if (ret) {
>> +		pr_err("Failed to register mdev bus\n");
>> +		return ret;
>> +	}
>> +
>> +	/*
>> +	 * Attempt to load known vfio_mdev.  This gives us a working environment
>> +	 * without the user needing to explicitly load vfio_mdev driver.
>> +	 */
>> +	request_module_nowait("vfio_mdev");
> We don't have this module yet.
> 

Yes, this module is added in 02/22 patch. I'll move only this call to
02/22 patch in my next update.

But ideally it should not fails anything if this vfio_mdev module is not
found.

>> +
>> +	return ret;
>> +}
>> +
>> +static void __exit mdev_exit(void)
>> +{
>> +	if (mdev_bus_compat_class)
>> +		class_compat_unregister(mdev_bus_compat_class);
>> +
>> +	mdev_bus_unregister();
>> +}
>> +
>> +module_init(mdev_init)
>> +module_exit(mdev_exit)
>> +
>> +MODULE_VERSION(DRIVER_VERSION);
>> +MODULE_LICENSE("GPL");
>> +MODULE_AUTHOR(DRIVER_AUTHOR);
>> +MODULE_DESCRIPTION(DRIVER_DESC);
>> diff --git a/drivers/vfio/mdev/mdev_driver.c b/drivers/vfio/mdev/mdev_driver.c
>> new file mode 100644
>> index 000000000000..0b3250044a80
>> --- /dev/null
>> +++ b/drivers/vfio/mdev/mdev_driver.c
>> @@ -0,0 +1,122 @@
>> +/*
>> + * MDEV driver
>> + *
>> + * Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved.
>> + *     Author: Neo Jia <cjia@nvidia.com>
>> + *	       Kirti Wankhede <kwankhede@nvidia.com>
> Don't know if you care much for this:
> There is a TAB before your name. :>
> 

Oh ok, Thanks for pointing that, I'll remove TAB.
I'll also take care of all the nits you suggested below.

Thanks,
Kirti

>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include <linux/device.h>
>> +#include <linux/iommu.h>
>> +#include <linux/mdev.h>
>> +
>> +#include "mdev_private.h"
>> +
>> +static int mdev_attach_iommu(struct mdev_device *mdev)
>> +{
>> +	int ret;
>> +	struct iommu_group *group;
>> +
>> +	group = iommu_group_alloc();
>> +	if (IS_ERR(group))
>> +		return PTR_ERR(group);
>> +
>> +	ret = iommu_group_add_device(group, &mdev->dev);
>> +	if (ret)
>> +		goto attach_fail;
>> +
>> +	dev_info(&mdev->dev, "MDEV: group_id = %d\n",
>> +				 iommu_group_id(group));
> nit: strange indentation.
> The above two lines could be safely merge into one line.
> 
>> +attach_fail:
>> +	iommu_group_put(group);
>> +	return ret;
>> +}
>> +
>> +static void mdev_detach_iommu(struct mdev_device *mdev)
>> +{
>> +	iommu_group_remove_device(&mdev->dev);
>> +	dev_info(&mdev->dev, "MDEV: detaching iommu\n");
>> +}
>> +
>> +static int mdev_probe(struct device *dev)
>> +{
>> +	struct mdev_driver *drv = to_mdev_driver(dev->driver);
>> +	struct mdev_device *mdev = to_mdev_device(dev);
>> +	int ret;
>> +
>> +	ret = mdev_attach_iommu(mdev);
>> +	if (ret)
>> +		return ret;
>> +
>> +	if (drv && drv->probe)
>> +		ret = drv->probe(dev);
>> +
>> +	if (ret)
>> +		mdev_detach_iommu(mdev);
> ?
> 	if (drv && drv->probe) {
> 		ret = drv->probe(dev);
> 		if (ret)
> 			mdev_detach_iommu(mdev);
> 	}
> 
>> +
>> +	return ret;
>> +}
>> +
>> +static int mdev_remove(struct device *dev)
>> +{
>> +	struct mdev_driver *drv = to_mdev_driver(dev->driver);
>> +	struct mdev_device *mdev = to_mdev_device(dev);
>> +
>> +	if (drv && drv->remove)
>> +		drv->remove(dev);
>> +
>> +	mdev_detach_iommu(mdev);
>> +
>> +	return 0;
>> +}
>> +
>> +struct bus_type mdev_bus_type = {
>> +	.name		= "mdev",
>> +	.probe		= mdev_probe,
>> +	.remove		= mdev_remove,
>> +};
>> +EXPORT_SYMBOL_GPL(mdev_bus_type);
>> +
>> +/*
> Is this a kernel-doc comment?
> It should be started with:
> /**
> 
>> + * mdev_register_driver - register a new MDEV driver
>> + * @drv: the driver to register
>> + * @owner: module owner of driver to be registered
>> + *
>> + * Returns a negative value on error, otherwise 0.
>> + */
>> +int mdev_register_driver(struct mdev_driver *drv, struct module *owner)
>> +{
>> +	/* initialize common driver fields */
>> +	drv->driver.name = drv->name;
>> +	drv->driver.bus = &mdev_bus_type;
>> +	drv->driver.owner = owner;
>> +
>> +	/* register with core */
>> +	return driver_register(&drv->driver);
>> +}
>> +EXPORT_SYMBOL(mdev_register_driver);
>> +
>> +/*
>> + * mdev_unregister_driver - unregister MDEV driver
>> + * @drv: the driver to unregister
>> + *
> Empty line.
> 
>> + */
>> +void mdev_unregister_driver(struct mdev_driver *drv)
>> +{
>> +	driver_unregister(&drv->driver);
>> +}
>> +EXPORT_SYMBOL(mdev_unregister_driver);
>> +
>> +int mdev_bus_register(void)
>> +{
>> +	return bus_register(&mdev_bus_type);
>> +}
>> +
>> +void mdev_bus_unregister(void)
>> +{
>> +	bus_unregister(&mdev_bus_type);
>> +}
> [...]
> 
>> diff --git a/include/linux/mdev.h b/include/linux/mdev.h
>> +/* Mediated device */
>> +struct mdev_device {
>> +	struct device		dev;
>> +	struct parent_device	*parent;
>> +	uuid_le			uuid;
>> +	void			*driver_data;
>> +
>> +	/* internal */
>> +	struct kref		ref;
>> +	struct list_head	next;
>> +	struct kobject		*type_kobj;
>> +};
>> +
>> +
> Empty line.
> 
>> +/**
>> + * struct parent_ops - Structure to be registered for each parent device to
>> + * register the device to mdev module.
>> + *
> [...]
> 
>> + * @mmap:		mmap callback
> No need a piece of description for arguments of the mmap callback?
> 
>> + * Parent device that support mediated device should be registered with mdev
>> + * module with parent_ops structure.
>> + **/
>> +
>> +struct parent_ops {
>> +	struct module   *owner;
>> +	const struct attribute_group **dev_attr_groups;
>> +	const struct attribute_group **mdev_attr_groups;
>> +	struct attribute_group **supported_type_groups;
>> +
>> +	int     (*create)(struct kobject *kobj, struct mdev_device *mdev);
>> +	int     (*remove)(struct mdev_device *mdev);
>> +	int     (*open)(struct mdev_device *mdev);
>> +	void    (*release)(struct mdev_device *mdev);
>> +	ssize_t (*read)(struct mdev_device *mdev, char __user *buf,
>> +			size_t count, loff_t *ppos);
>> +	ssize_t (*write)(struct mdev_device *mdev, const char __user *buf,
>> +			 size_t count, loff_t *ppos);
>> +	ssize_t (*ioctl)(struct mdev_device *mdev, unsigned int cmd,
>> +			 unsigned long arg);
>> +	int	(*mmap)(struct mdev_device *mdev, struct vm_area_struct *vma);
>> +};
>> +
> [...]
> 

^ permalink raw reply

* Re: [PATCH v11 01/22] vfio: Mediated device Core driver
From: Kirti Wankhede @ 2016-11-08 21:06 UTC (permalink / raw)
  To: alex.williamson, pbonzini, kraxel, cjia, qemu-devel, kvm,
	kevin.tian, jike.song, bjsdjshi, linux-kernel
In-Reply-To: <20161108092552.GA2090@bjsdjshi@linux.vnet.ibm.com>



On 11/8/2016 2:55 PM, Dong Jia Shi wrote:
> * Kirti Wankhede <kwankhede@nvidia.com> [2016-11-05 02:40:35 +0530]:
> 
> Hi Kirti,
> 
> [...]
>> diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig
>> index da6e2ce77495..23eced02aaf6 100644
>> --- a/drivers/vfio/Kconfig
>> +++ b/drivers/vfio/Kconfig
>> @@ -48,4 +48,5 @@ menuconfig VFIO_NOIOMMU
>>
>>  source "drivers/vfio/pci/Kconfig"
>>  source "drivers/vfio/platform/Kconfig"
>> +source "drivers/vfio/mdev/Kconfig"
>>  source "virt/lib/Kconfig"
>> diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile
>> index 7b8a31f63fea..4a23c13b6be4 100644
>> --- a/drivers/vfio/Makefile
>> +++ b/drivers/vfio/Makefile
>> @@ -7,3 +7,4 @@ obj-$(CONFIG_VFIO_IOMMU_SPAPR_TCE) += vfio_iommu_spapr_tce.o
>>  obj-$(CONFIG_VFIO_SPAPR_EEH) += vfio_spapr_eeh.o
>>  obj-$(CONFIG_VFIO_PCI) += pci/
>>  obj-$(CONFIG_VFIO_PLATFORM) += platform/
>> +obj-$(CONFIG_VFIO_MDEV) += mdev/
>> diff --git a/drivers/vfio/mdev/Kconfig b/drivers/vfio/mdev/Kconfig
>> new file mode 100644
>> index 000000000000..303c14ce2847
>> --- /dev/null
>> +++ b/drivers/vfio/mdev/Kconfig
>> @@ -0,0 +1,10 @@
>> +
>> +config VFIO_MDEV
>> +    tristate "Mediated device driver framework"
>> +    depends on VFIO
>> +    default n
>> +    help
>> +        Provides a framework to virtualize devices.
>> +	See Documentation/vfio-mdev/vfio-mediated-device.txt for more details.
> We don't have this doc at this point of time.
> 

Yes, but I have this doc in this patch series.

>> +
>> +        If you don't know what do here, say N.
>> diff --git a/drivers/vfio/mdev/Makefile b/drivers/vfio/mdev/Makefile
>> new file mode 100644
>> index 000000000000..31bc04801d94
>> --- /dev/null
>> +++ b/drivers/vfio/mdev/Makefile
>> @@ -0,0 +1,4 @@
>> +
>> +mdev-y := mdev_core.o mdev_sysfs.o mdev_driver.o
>> +
>> +obj-$(CONFIG_VFIO_MDEV) += mdev.o
>> diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
>> new file mode 100644
>> index 000000000000..54c59f325336
>> --- /dev/null
>> +++ b/drivers/vfio/mdev/mdev_core.c
> [...]
> 
>> +
>> +/*
>> + * mdev_device_remove_ops gets called from sysfs's 'remove' and when parent
>> + * device is being unregistered from mdev device framework.
>> + * - 'force_remove' is set to 'false' when called from sysfs's 'remove' which
>> + *   indicates that if the mdev device is active, used by VMM or userspace
>> + *   application, vendor driver could return error then don't remove the device.
>> + * - 'force_remove' is set to 'true' when called from mdev_unregister_device()
>> + *   which indicate that parent device is being removed from mdev device
>> + *   framework so remove mdev device forcefully.
>> + */
>> +static int mdev_device_remove_ops(struct mdev_device *mdev, bool force_remove)
> ?
> s/force_remove/force/
> 
>> +{
>> +	struct parent_device *parent = mdev->parent;
>> +	int ret;
>> +
>> +	/*
>> +	 * Vendor driver can return error if VMM or userspace application is
>> +	 * using this mdev device.
>> +	 */
>> +	ret = parent->ops->remove(mdev);
>> +	if (ret && !force_remove)
>> +		return -EBUSY;
>> +
>> +	sysfs_remove_groups(&mdev->dev.kobj, parent->ops->mdev_attr_groups);
>> +	return 0;
>> +}
>> +
>> +static int mdev_device_remove_cb(struct device *dev, void *data)
>> +{
>> +	if (!dev_is_mdev(dev))
>> +		return 0;
>> +
>> +	return mdev_device_remove(dev, data ? *(bool *)data : true);
>> +}
>> +
>> +/*
>> + * mdev_register_device : Register a device
>> + * @dev: device structure representing parent device.
>> + * @ops: Parent device operation structure to be registered.
>> + *
>> + * Add device to list of registered parent devices.
>> + * Returns a negative value on error, otherwise 0.
>> + */
>> +int mdev_register_device(struct device *dev, const struct parent_ops *ops)
>> +{
>> +	int ret;
>> +	struct parent_device *parent;
>> +
>> +	/* check for mandatory ops */
>> +	if (!ops || !ops->create || !ops->remove || !ops->supported_type_groups)
>> +		return -EINVAL;
>> +
>> +	dev = get_device(dev);
>> +	if (!dev)
>> +		return -EINVAL;
>> +
>> +	mutex_lock(&parent_list_lock);
>> +
>> +	/* Check for duplicate */
>> +	parent = __find_parent_device(dev);
>> +	if (parent) {
>> +		ret = -EEXIST;
>> +		goto add_dev_err;
>> +	}
>> +
>> +	parent = kzalloc(sizeof(*parent), GFP_KERNEL);
>> +	if (!parent) {
>> +		ret = -ENOMEM;
>> +		goto add_dev_err;
>> +	}
>> +
>> +	kref_init(&parent->ref);
>> +	mutex_init(&parent->lock);
>> +
>> +	parent->dev = dev;
>> +	parent->ops = ops;
>> +
>> +	if (!mdev_bus_compat_class) {
>> +		mdev_bus_compat_class = class_compat_register("mdev_bus");
>> +		if (!mdev_bus_compat_class) {
>> +			ret = -ENOMEM;
>> +			goto add_dev_err;
>> +		}
>> +	}
>> +
>> +	ret = parent_create_sysfs_files(parent);
>> +	if (ret)
>> +		goto add_dev_err;
>> +
>> +	ret = class_compat_create_link(mdev_bus_compat_class, dev, NULL);
>> +	if (ret)
>> +		dev_warn(dev, "Failed to create compatibility class link\n");
>> +
>> +	list_add(&parent->next, &parent_list);
>> +	mutex_unlock(&parent_list_lock);
>> +
>> +	dev_info(dev, "MDEV: Registered\n");
>> +	return 0;
>> +
>> +add_dev_err:
>> +	mutex_unlock(&parent_list_lock);
>> +	if (parent)
>> +		mdev_put_parent(parent);
> Why do this? I don't find the place that you call mdev_get_parent above.
> 

	kref_init(&parent->ref);
Above increments the ref_count, so mdev_put_parent() should be called if
anything fails.

>> +	else
>> +		put_device(dev);
> Shouldn't we always do this?
> 

When mdev_put_parent() is called, its release function do this. So if
mdev_put_parent() is called, we don't need this.

>> +	return ret;
>> +}
>> +EXPORT_SYMBOL(mdev_register_device);
>> +
>> +/*
>> + * mdev_unregister_device : Unregister a parent device
>> + * @dev: device structure representing parent device.
>> + *
>> + * Remove device from list of registered parent devices. Give a chance to free
>> + * existing mediated devices for given device.
>> + */
>> +
>> +void mdev_unregister_device(struct device *dev)
>> +{
>> +	struct parent_device *parent;
>> +	bool force_remove = true;
>> +
>> +	mutex_lock(&parent_list_lock);
>> +	parent = __find_parent_device(dev);
>> +
>> +	if (!parent) {
>> +		mutex_unlock(&parent_list_lock);
>> +		return;
>> +	}
>> +	dev_info(dev, "MDEV: Unregistering\n");
>> +
>> +	list_del(&parent->next);
>> +	class_compat_remove_link(mdev_bus_compat_class, dev, NULL);
>> +
>> +	device_for_each_child(dev, (void *)&force_remove,
>> +			      mdev_device_remove_cb);
>> +
>> +	parent_remove_sysfs_files(parent);
>> +
>> +	mutex_unlock(&parent_list_lock);
>> +	mdev_put_parent(parent);
> We also need to call put_device(dev) here since we have a get_device
> during registration, no?
> Or I must miss something...
> 

As explained above.

>> +}
>> +EXPORT_SYMBOL(mdev_unregister_device);
>> +
> [...]
> 
>> +static int __init mdev_init(void)
>> +{
>> +	int ret;
>> +
>> +	ret = mdev_bus_register();
>> +	if (ret) {
>> +		pr_err("Failed to register mdev bus\n");
>> +		return ret;
>> +	}
>> +
>> +	/*
>> +	 * Attempt to load known vfio_mdev.  This gives us a working environment
>> +	 * without the user needing to explicitly load vfio_mdev driver.
>> +	 */
>> +	request_module_nowait("vfio_mdev");
> We don't have this module yet.
> 

Yes, this module is added in 02/22 patch. I'll move only this call to
02/22 patch in my next update.

But ideally it should not fails anything if this vfio_mdev module is not
found.

>> +
>> +	return ret;
>> +}
>> +
>> +static void __exit mdev_exit(void)
>> +{
>> +	if (mdev_bus_compat_class)
>> +		class_compat_unregister(mdev_bus_compat_class);
>> +
>> +	mdev_bus_unregister();
>> +}
>> +
>> +module_init(mdev_init)
>> +module_exit(mdev_exit)
>> +
>> +MODULE_VERSION(DRIVER_VERSION);
>> +MODULE_LICENSE("GPL");
>> +MODULE_AUTHOR(DRIVER_AUTHOR);
>> +MODULE_DESCRIPTION(DRIVER_DESC);
>> diff --git a/drivers/vfio/mdev/mdev_driver.c b/drivers/vfio/mdev/mdev_driver.c
>> new file mode 100644
>> index 000000000000..0b3250044a80
>> --- /dev/null
>> +++ b/drivers/vfio/mdev/mdev_driver.c
>> @@ -0,0 +1,122 @@
>> +/*
>> + * MDEV driver
>> + *
>> + * Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved.
>> + *     Author: Neo Jia <cjia@nvidia.com>
>> + *	       Kirti Wankhede <kwankhede@nvidia.com>
> Don't know if you care much for this:
> There is a TAB before your name. :>
> 

Oh ok, Thanks for pointing that, I'll remove TAB.
I'll also take care of all the nits you suggested below.

Thanks,
Kirti

>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include <linux/device.h>
>> +#include <linux/iommu.h>
>> +#include <linux/mdev.h>
>> +
>> +#include "mdev_private.h"
>> +
>> +static int mdev_attach_iommu(struct mdev_device *mdev)
>> +{
>> +	int ret;
>> +	struct iommu_group *group;
>> +
>> +	group = iommu_group_alloc();
>> +	if (IS_ERR(group))
>> +		return PTR_ERR(group);
>> +
>> +	ret = iommu_group_add_device(group, &mdev->dev);
>> +	if (ret)
>> +		goto attach_fail;
>> +
>> +	dev_info(&mdev->dev, "MDEV: group_id = %d\n",
>> +				 iommu_group_id(group));
> nit: strange indentation.
> The above two lines could be safely merge into one line.
> 
>> +attach_fail:
>> +	iommu_group_put(group);
>> +	return ret;
>> +}
>> +
>> +static void mdev_detach_iommu(struct mdev_device *mdev)
>> +{
>> +	iommu_group_remove_device(&mdev->dev);
>> +	dev_info(&mdev->dev, "MDEV: detaching iommu\n");
>> +}
>> +
>> +static int mdev_probe(struct device *dev)
>> +{
>> +	struct mdev_driver *drv = to_mdev_driver(dev->driver);
>> +	struct mdev_device *mdev = to_mdev_device(dev);
>> +	int ret;
>> +
>> +	ret = mdev_attach_iommu(mdev);
>> +	if (ret)
>> +		return ret;
>> +
>> +	if (drv && drv->probe)
>> +		ret = drv->probe(dev);
>> +
>> +	if (ret)
>> +		mdev_detach_iommu(mdev);
> ?
> 	if (drv && drv->probe) {
> 		ret = drv->probe(dev);
> 		if (ret)
> 			mdev_detach_iommu(mdev);
> 	}
> 
>> +
>> +	return ret;
>> +}
>> +
>> +static int mdev_remove(struct device *dev)
>> +{
>> +	struct mdev_driver *drv = to_mdev_driver(dev->driver);
>> +	struct mdev_device *mdev = to_mdev_device(dev);
>> +
>> +	if (drv && drv->remove)
>> +		drv->remove(dev);
>> +
>> +	mdev_detach_iommu(mdev);
>> +
>> +	return 0;
>> +}
>> +
>> +struct bus_type mdev_bus_type = {
>> +	.name		= "mdev",
>> +	.probe		= mdev_probe,
>> +	.remove		= mdev_remove,
>> +};
>> +EXPORT_SYMBOL_GPL(mdev_bus_type);
>> +
>> +/*
> Is this a kernel-doc comment?
> It should be started with:
> /**
> 
>> + * mdev_register_driver - register a new MDEV driver
>> + * @drv: the driver to register
>> + * @owner: module owner of driver to be registered
>> + *
>> + * Returns a negative value on error, otherwise 0.
>> + */
>> +int mdev_register_driver(struct mdev_driver *drv, struct module *owner)
>> +{
>> +	/* initialize common driver fields */
>> +	drv->driver.name = drv->name;
>> +	drv->driver.bus = &mdev_bus_type;
>> +	drv->driver.owner = owner;
>> +
>> +	/* register with core */
>> +	return driver_register(&drv->driver);
>> +}
>> +EXPORT_SYMBOL(mdev_register_driver);
>> +
>> +/*
>> + * mdev_unregister_driver - unregister MDEV driver
>> + * @drv: the driver to unregister
>> + *
> Empty line.
> 
>> + */
>> +void mdev_unregister_driver(struct mdev_driver *drv)
>> +{
>> +	driver_unregister(&drv->driver);
>> +}
>> +EXPORT_SYMBOL(mdev_unregister_driver);
>> +
>> +int mdev_bus_register(void)
>> +{
>> +	return bus_register(&mdev_bus_type);
>> +}
>> +
>> +void mdev_bus_unregister(void)
>> +{
>> +	bus_unregister(&mdev_bus_type);
>> +}
> [...]
> 
>> diff --git a/include/linux/mdev.h b/include/linux/mdev.h
>> +/* Mediated device */
>> +struct mdev_device {
>> +	struct device		dev;
>> +	struct parent_device	*parent;
>> +	uuid_le			uuid;
>> +	void			*driver_data;
>> +
>> +	/* internal */
>> +	struct kref		ref;
>> +	struct list_head	next;
>> +	struct kobject		*type_kobj;
>> +};
>> +
>> +
> Empty line.
> 
>> +/**
>> + * struct parent_ops - Structure to be registered for each parent device to
>> + * register the device to mdev module.
>> + *
> [...]
> 
>> + * @mmap:		mmap callback
> No need a piece of description for arguments of the mmap callback?
> 
>> + * Parent device that support mediated device should be registered with mdev
>> + * module with parent_ops structure.
>> + **/
>> +
>> +struct parent_ops {
>> +	struct module   *owner;
>> +	const struct attribute_group **dev_attr_groups;
>> +	const struct attribute_group **mdev_attr_groups;
>> +	struct attribute_group **supported_type_groups;
>> +
>> +	int     (*create)(struct kobject *kobj, struct mdev_device *mdev);
>> +	int     (*remove)(struct mdev_device *mdev);
>> +	int     (*open)(struct mdev_device *mdev);
>> +	void    (*release)(struct mdev_device *mdev);
>> +	ssize_t (*read)(struct mdev_device *mdev, char __user *buf,
>> +			size_t count, loff_t *ppos);
>> +	ssize_t (*write)(struct mdev_device *mdev, const char __user *buf,
>> +			 size_t count, loff_t *ppos);
>> +	ssize_t (*ioctl)(struct mdev_device *mdev, unsigned int cmd,
>> +			 unsigned long arg);
>> +	int	(*mmap)(struct mdev_device *mdev, struct vm_area_struct *vma);
>> +};
>> +
> [...]
> 

^ permalink raw reply

* Re: [PATCH v2] block: disallow changing max_sectors_kb on a request stacking device
From: Martin K. Petersen @ 2016-11-08 21:10 UTC (permalink / raw)
  To: Mike Snitzer; +Cc: Martin K. Petersen, Jens Axboe, linux-block, dm-devel
In-Reply-To: <20161108033457.GA30325@redhat.com>

>>>>> "Mike" == Mike Snitzer <snitzer@redhat.com> writes:

Mike,

>> However, doesn't it make more sense to tweak limits on DM device
>> instead of the underlying ones? It seems a bit counter-intuitive to
>> me to change max_sectors_kb on a different device than the one where
>> the filesystem whose max I/O size you want to change is located.

Mike> As you know, the limits are stacked from the bottom-up.

Yep, but since max_sectors_kb is a performance tunable and not something
enforced by the hardware, maybe we should consider treating it
differently?

Mike> As the commit header from this thread mentioned, what I've arrived
Mike> at is to have multipathd establish a desired max_sectors_kb (if
Mike> configured to do so in multipath.conf) on the underlying paths
Mike> _before_ the multipath device is created.  Yet to check with Ben
Mike> Marzinski or others but it seems like a reasonable feature (and
Mike> really the cleaniset way to deal with this issue IMHO.. no need
Mike> for lots of kernel code changes for something so niche).

That's fine. Another option would be to use the max_dev_sectors queue
limit to contain the minimum max_sectors from below. It was really only
envisioned as a LLD limit but it may be useful in this case.
queue_max_sectors_store() already enforces it.

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply

* Re: BUG: Hung task timeouts in for-4.10/dio
From: Mike Snitzer @ 2016-11-08 21:08 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Logan Gunthorpe, Christoph Hellwig, linux-block
In-Reply-To: <1e0e590b-fde3-a86d-bc60-8e5465928533@fb.com>

On Tue, Nov 08 2016 at  3:21pm -0500,
Jens Axboe <axboe@fb.com> wrote:

> On 11/08/2016 12:55 PM, Logan Gunthorpe wrote:
> >Hey,
> >
> >I've attached the output of dmesg from a working boot and the output of
> >mount.
> >
> >Pretty much all the file systems are ext4. We have some experimental
> >nvme devices in this system which I did try removing to eliminate that
> >possibility.
> >
> >Let me know if you need anything else.

I'm not seeing Logan's above message (with dmesg) on linux-block
archives.  So I cannot yet appreciate which DM targets are in play.

> You're using dm, that might be related. Mike, have you tried booting
> for-4.10/block and checking if dm works fine?

I haven't tried for-4.10/block yet.  But I should be able to try it this
week.

^ permalink raw reply

* Re: [PATCH] x86/MCE: Remove MCP_TIMESTAMP
From: Borislav Petkov @ 2016-11-08 21:08 UTC (permalink / raw)
  To: Thomas Gleixner; +Cc: Luck, Tony, linux-edac, X86 ML, LKML
In-Reply-To: <alpine.DEB.2.20.1611082136380.3501@nanos>

On Tue, Nov 08, 2016 at 09:39:02PM +0100, Thomas Gleixner wrote:
> That does not make any sense. Where is m.tsc initialized? I couldn't find
> any place which does, except this and the conditional clear farther down in
> that function.

mce_gather_info->mce_setup does

m->tsc = rdtsc();

And we do that *everytime* but then we go and clear the damn thing. I
know, I know, I wanted to flip that logic too and read the TSC *only*
when we want a precise timestamp but that would require more changes as
mce_setup() is used at a bunch of places.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

^ permalink raw reply

* Re: [Qemu-devel] [PATCH kernel v4 7/7] virtio-balloon: tell host vm's unused page info
From: Michael S. Tsirkin @ 2016-11-08 21:07 UTC (permalink / raw)
  To: Dave Hansen
  Cc: Li, Liang Z, pbonzini@redhat.com, amit.shah@redhat.com,
	quintela@redhat.com, dgilbert@redhat.com, qemu-devel@nongnu.org,
	kvm@vger.kernel.org, virtio-dev@lists.oasis-open.org,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	virtualization@lists.linux-foundation.org,
	mgorman@techsingularity.net, cornelia.huck@de.ibm.com
In-Reply-To: <281acd8d-fd94-6318-35e5-9eb130303dc6@intel.com>

On Mon, Nov 07, 2016 at 09:23:38AM -0800, Dave Hansen wrote:
> On 11/06/2016 07:37 PM, Li, Liang Z wrote:
> >> Let's say we do a 32k bitmap that can hold ~1M pages.  That's 4GB of RAM.
> >> On a 1TB system, that's 256 passes through the top-level loop.
> >> The bottom-level lists have tens of thousands of pages in them, even on my
> >> laptop.  Only 1/256 of these pages will get consumed in a given pass.
> >>
> > Your description is not exactly.
> > A 32k bitmap is used only when there is few free memory left in the system and when 
> > the extend_page_bitmap() failed to allocate more memory for the bitmap. Or dozens of 
> > 32k split bitmap will be used, this version limit the bitmap count to 32, it means we can use
> > at most 32*32 kB for the bitmap, which can cover 128GB for RAM. We can increase the bitmap
> > count limit to a larger value if 32 is not big enough.
> 
> OK, so it tries to allocate a large bitmap.  But, if it fails, it will
> try to work with a smaller bitmap.  Correct?
> 
> So, what's the _worst_ case?  It sounds like it is even worse than I was
> positing.
> 
> >> That's an awfully inefficient way of doing it.  This patch essentially changed
> >> the data structure without changing the algorithm to populate it.
> >>
> >> Please change the *algorithm* to use the new data structure efficiently.
> >>  Such a change would only do a single pass through each freelist, and would
> >> choose whether to use the extent-based (pfn -> range) or bitmap-based
> >> approach based on the contents of the free lists.
> > 
> > Save the free page info to a raw bitmap first and then process the raw bitmap to
> > get the proper ' extent-based ' and  'bitmap-based' is the most efficient way I can 
> > come up with to save the virtio data transmission.  Do you have some better idea?
> 
> That's kinda my point.  This patch *does* processing to try to pack the
> bitmaps full of pages from the various pfn ranges.  It's a form of
> processing that gets *REALLY*, *REALLY* bad in some (admittedly obscure)
> cases.
> 
> Let's not pretend that making an essentially unlimited number of passes
> over the free lists is not processing.
> 
> 1. Allocate as large of a bitmap as you can. (what you already do)
> 2. Iterate from the largest freelist order.  Store those pages in the
>    bitmap.
> 3. If you can no longer fit pages in the bitmap, return the list that
>    you have.
> 4. Make an approximation about where the bitmap does not make any more,
>    and fall back to listing individual PFNs.  This would make sens, for
>    instance in a large zone with very few free order-0 pages left.

In practice, a single PFN using the bitmap format
only takes up twice the size: I think it's 128 instead of 64 bit
per entry.

So it's not a a given that point 4 is worth it at any point,
just packing multiple bitmaps might be good enough.

> 
> > It seems the benefit we get for this feature is not as big as that in fast balloon inflating/deflating.
> >>
> >> You should not be using get_max_pfn().  Any patch set that continues to use
> >> it is not likely to be using a proper algorithm.
> > 
> > Do you have any suggestion about how to avoid it?
> 
> Yes: get the pfns from the page free lists alone.  Don't derive them
> from the pfn limits of the system or zones.

^ permalink raw reply


This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.