* [PATCH v3 00/17] ASoC: fsl_ssi: Clean up - program flow level
From: Nicolin Chen @ 2018-01-15 4:21 UTC (permalink / raw)
To: timur, broonie, mail
Cc: linux-kernel, linuxppc-dev, alsa-devel, lgirdwood, fabio.estevam,
caleb, arnaud.mouiche, lukma, kernel
==Change log==
v3
* Reworked the series by taking suggestions from Maciej
+ Added PATCH-01 to make RX and TX more clearly defined
+ Replaced "bool dir" with "int dir" in PATCH-04
+ Replaced "!dir" with "int adir" in PATCH-05
+ Put CBM_CFS behind the baudclk check to keep the same
program flow in PATCH-14
+ Removed all cpu_dai_drv changes in PATCH-15
v2
* Reworked the series by taking suggestions from Maciej
+ Added PATCH-01 to keep all ssi->i2s_net updated
+ Replaced bool tx with bool dir in PATCH-03 and PATCH-06
+ Moved all initial register configurations from dai probe() to
platform probe() so as to let AC97 CODEC successfully probe.
* Added Tested-by from Caleb for TDM test cases.
==Background==
The fsl_ssi driver was designed for PPC originally and then it has
been updated to support different modes for i.MX Series, including
SDMA, I2S Master mode, AC97 and older i.MXs with FIQ, by different
contributors for different use cases in different coding styles.
Additionally, in order to fix/work-around hardware bugs and design
flaws, the driver made a lot of compromise so now its program flow
looks very complicated and it's getting hard to maintain or update.
So I am going to clean up the driver on both coding style level and
program flow level.
==Introduction==
This series of patches is the second set to clean up fsl_ssi driver
in the program flow level. Any patch here may impact a fundamental
test case like playback or record.
==Verification==
This series of patches require fully tested. I have done such tests
on i.MX6SoloX with WM8962 using imx_v6_v7_defconfig as:
- Playback via I2S Master and Slave mode
- Record via I2S Master and Slave mode
- Simultaneous playback and record via I2S Master and Slave mode
- Background playback with foreground record (starting at different
time) via I2S Master and Slave mode
- Background record with foreground playback (starting at different
time) via I2S Master and Slave mode
* All tests above by hacking offline_config to true in imx51.
Caleb has tested v1 with TDM lookback tests on i.MX6.
Example of uncovered tests: AC97, PowerPC and FIQ.
Nicolin Chen (17):
ASoC: fsl_ssi: Redefine RX and TX macros
ASoC: fsl_ssi: Keep ssi->i2s_net updated
ASoC: fsl_ssi: Clean up set_dai_tdm_slot()
ASoC: fsl_ssi: Maintain a mask of active streams
ASoC: fsl_ssi: Rename fsl_ssi_disable_val macro
ASoC: fsl_ssi: Clear FIFO directly in fsl_ssi_config()
ASoC: fsl_ssi: Clean up helper functions of trigger()
ASoC: fsl_ssi: Add DAIFMT define for AC97
ASoC: fsl_ssi: Clean up fsl_ssi_setup_regvals()
ASoC: fsl_ssi: Set xFEN0 and xFEN1 together
ASoC: fsl_ssi: Use snd_soc_init_dma_data instead
ASoC: fsl_ssi: Move one-time configurations to probe()
ASoC: fsl_ssi: Setup AC97 in fsl_ssi_hw_init()
ASoC: fsl_ssi: Clean up _fsl_ssi_set_dai_fmt()
ASoC: fsl_ssi: Add bool synchronous to mark synchronous mode
ASoC: fsl_ssi: Move DT related code to a separate probe()
ASoC: fsl_ssi: Use ssi->streams instead of reading register
sound/soc/fsl/fsl_ssi.c | 733 ++++++++++++++++++++++++------------------------
sound/soc/fsl/fsl_ssi.h | 3 -
2 files changed, 370 insertions(+), 366 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH v3 01/17] ASoC: fsl_ssi: Redefine RX and TX macros
From: Nicolin Chen @ 2018-01-15 4:21 UTC (permalink / raw)
To: timur, broonie, mail
Cc: linux-kernel, linuxppc-dev, alsa-devel, lgirdwood, fabio.estevam,
caleb, arnaud.mouiche, lukma, kernel
In-Reply-To: <1515990087-11598-1-git-send-email-nicoleotsuka@gmail.com>
The RX and TX macros were defined implicitly and there was
a potential risk if someone changes their values.
Since they were defined to index the array ssi->regvals[2],
this patch moves these two macros to fsl_ssi.c, closer to
its owner ssi->regvals. And it also puts some comments here
to limit their value within [0, 1].
Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com>
---
sound/soc/fsl/fsl_ssi.c | 4 ++++
sound/soc/fsl/fsl_ssi.h | 3 ---
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index aecd00f..001e453 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -56,6 +56,10 @@
#include "fsl_ssi.h"
#include "imx-pcm.h"
+/* Define RX and TX to index ssi->regvals array; Can be 0 or 1 only */
+#define RX 0
+#define TX 1
+
/**
* FSLSSI_I2S_FORMATS: audio formats supported by the SSI
*
diff --git a/sound/soc/fsl/fsl_ssi.h b/sound/soc/fsl/fsl_ssi.h
index de2fdc5..18f8dd5 100644
--- a/sound/soc/fsl/fsl_ssi.h
+++ b/sound/soc/fsl/fsl_ssi.h
@@ -12,9 +12,6 @@
#ifndef _MPC8610_I2S_H
#define _MPC8610_I2S_H
-#define RX 0
-#define TX 1
-
/* -- SSI Register Map -- */
/* SSI Transmit Data Register 0 */
--
2.7.4
^ permalink raw reply related
* [PATCH 1/6] resource: Extend the PPC32 reserved memory hack
From: Jonathan Neuschäfer @ 2018-01-15 3:13 UTC (permalink / raw)
To: linux-kernel
Cc: linuxppc-dev, linux-gpio, devicetree, Jonathan Neuschäfer,
Brijesh Singh, Tom Lendacky, Thomas Gleixner, Borislav Petkov,
Kees Cook
In-Reply-To: <20180115031401.19577-1-j.neuschaefer@gmx.net>
On the Nintendo Wii, there are two ranges of physical memory, and MMIO
in between, but Linux on ppc32 doesn't support discontiguous memory.
Therefore a hack was introduced in commit c5df7f775148 ("powerpc: allow
ioremap within reserved memory regions") and commit de32400dd26e ("wii:
use both mem1 and mem2 as ram"):
- Treat the area from the start of the first memory area (MEM1) to the
end of the second (MEM2) as one big memory area, but mark the part
that doesn't belong to MEM1 or MEM2 as reserved.
- Only on the Wii, allow ioremap to be used on reserved memory.
This hack, however, doesn't account for the "resource"-based API in
kernel/resource.c, because __request_region performs its own checks.
Extend the hack to kernel/resource.c, to allow more drivers to allocate
their MMIO regions on the Wii.
Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
---
kernel/resource.c | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/kernel/resource.c b/kernel/resource.c
index 54ba6de3757c..bb3d329329da 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -1134,6 +1134,24 @@ resource_size_t resource_alignment(struct resource *res)
static DECLARE_WAIT_QUEUE_HEAD(muxed_resource_wait);
+/*
+ * On some ppc32 platforms (Nintendo Wii), reserved memory is used to work
+ * around the fact that Linux doesn't support discontiguous memory (all memory
+ * is treated as one large area with holes punched in it), and reserved memory
+ * is allowed to be allocated.
+ */
+#ifdef CONFIG_PPC32
+static bool conflict_ignored(struct resource *conflict)
+{
+ extern int __allow_ioremap_reserved;
+
+ return __allow_ioremap_reserved &&
+ (conflict->flags & IORESOURCE_SYSRAM);
+}
+#else
+static bool conflict_ignored(struct resource *conflict) { return false; }
+#endif
+
/**
* __request_region - create a new busy resource region
* @parent: parent resource descriptor
@@ -1166,8 +1184,9 @@ struct resource * __request_region(struct resource *parent,
res->desc = parent->desc;
conflict = __request_resource(parent, res);
- if (!conflict)
+ if (!conflict || conflict_ignored(conflict))
break;
+
if (conflict != parent) {
if (!(conflict->flags & IORESOURCE_BUSY)) {
parent = conflict;
--
2.15.1
^ permalink raw reply related
* [PATCH 0/6] Nintendo Wii GPIO driver
From: Jonathan Neuschäfer @ 2018-01-15 3:13 UTC (permalink / raw)
To: linux-kernel
Cc: linuxppc-dev, linux-gpio, devicetree, Jonathan Neuschäfer
This series adds a driver for the GPIO controller used in the Nintendo
Wii game console.
The driver itself, and the related devicetree work should be pretty
uncontroversial, but due to the system architecture of the Wii, I also
had to extend an old resource allocation hack to kernel/resource.c: On
the Wii, there are two separate RAM ranges, with MMIO right in the
middle, but AFAIK, Linux on PPC32 doesn't support discontiguous memory
properly. So the hack is to allocate one big RAM range with a hole
(marked as reserved memory) for MMIO in the middle.
Because this series touches different subsystems (GPIO, DT, core
resource management), I guess it should be picked up patch-by-patch by
the different maintainers.
Jonathan Neuschäfer (6):
resource: Extend the PPC32 reserved memory hack
powerpc: wii: Explicitly configure GPIO owner for poweroff pin
gpio: Add GPIO driver for Nintendo Wii
dt-bindings: gpio: Add binding for Wii GPIO controller
powerpc: wii.dts: Add ngpios property
powerpc: wii.dts: Add GPIO line names
.../bindings/gpio/nintendo,hollywood-gpio.txt | 27 +++
.../devicetree/bindings/powerpc/nintendo/wii.txt | 9 +-
arch/powerpc/boot/dts/wii.dts | 9 +
arch/powerpc/platforms/embedded6xx/wii.c | 7 +
drivers/gpio/Kconfig | 8 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-hlwd.c | 183 +++++++++++++++++++++
kernel/resource.c | 21 ++-
8 files changed, 256 insertions(+), 9 deletions(-)
create mode 100644 Documentation/devicetree/bindings/gpio/nintendo,hollywood-gpio.txt
create mode 100644 drivers/gpio/gpio-hlwd.c
--
2.15.1
^ permalink raw reply
* [PATCH 4/6] dt-bindings: gpio: Add binding for Wii GPIO controller
From: Jonathan Neuschäfer @ 2018-01-15 3:13 UTC (permalink / raw)
To: linux-kernel
Cc: linuxppc-dev, linux-gpio, devicetree, Jonathan Neuschäfer,
Linus Walleij, Rob Herring, Mark Rutland, Benjamin Herrenschmidt,
Paul Mackerras, Michael Ellerman
In-Reply-To: <20180115031401.19577-1-j.neuschaefer@gmx.net>
Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
---
.../bindings/gpio/nintendo,hollywood-gpio.txt | 27 ++++++++++++++++++++++
.../devicetree/bindings/powerpc/nintendo/wii.txt | 9 +-------
2 files changed, 28 insertions(+), 8 deletions(-)
create mode 100644 Documentation/devicetree/bindings/gpio/nintendo,hollywood-gpio.txt
diff --git a/Documentation/devicetree/bindings/gpio/nintendo,hollywood-gpio.txt b/Documentation/devicetree/bindings/gpio/nintendo,hollywood-gpio.txt
new file mode 100644
index 000000000000..a97ce6b5b724
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/nintendo,hollywood-gpio.txt
@@ -0,0 +1,27 @@
+Nintendo Wii (Hollywood) GPIO controller
+
+Required properties:
+- compatible: "nintendo,hollywood-gpio
+- reg: Physical base address and length of the controller's registers.
+- gpio-controller: Marks the device node as a GPIO controller.
+- #gpio-cells: Should be <2>. The first cell is the pin number and the
+ second cell is used to specify optional parameters:
+ - bit 0 specifies polarity (0 for normal, 1 for inverted).
+
+Optional properties:
+- ngpios: see Documentation/devicetree/bindings/gpio/gpio.txt
+- interrupt-controller: Marks the device node as an interrupt controller.
+- #interrupt-cells: Should be two.
+- interrupts: Interrupt specifier for the controller's Broadway (PowerPC)
+ interrupt.
+- interrupt-parent: phandle of the parent interrupt controller.
+
+Example:
+
+ GPIO: gpio@0d8000c0 {
+ #gpio-cells = <2>;
+ compatible = "nintendo,hollywood-gpio";
+ reg = <0x0d8000c0 0x40>;
+ gpio-controller;
+ ngpios = <24>;
+ }
diff --git a/Documentation/devicetree/bindings/powerpc/nintendo/wii.txt b/Documentation/devicetree/bindings/powerpc/nintendo/wii.txt
index 36afa322b04b..a3dc4b9fa11a 100644
--- a/Documentation/devicetree/bindings/powerpc/nintendo/wii.txt
+++ b/Documentation/devicetree/bindings/powerpc/nintendo/wii.txt
@@ -152,14 +152,7 @@ Nintendo Wii device tree
1.l) The General Purpose I/O (GPIO) controller node
- Represents the dual access 32 GPIO controller interface.
-
- Required properties:
-
- - #gpio-cells : <2>
- - compatible : should be "nintendo,hollywood-gpio"
- - reg : should contain the IPC registers location and length
- - gpio-controller
+ see Documentation/devicetree/bindings/gpio/nintendo,hollywood-gpio.txt
1.m) The control node
--
2.15.1
^ permalink raw reply related
* [PATCH 2/6] powerpc: wii: Explicitly configure GPIO owner for poweroff pin
From: Jonathan Neuschäfer @ 2018-01-15 3:13 UTC (permalink / raw)
To: linux-kernel
Cc: linuxppc-dev, linux-gpio, devicetree, Jonathan Neuschäfer,
Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
In-Reply-To: <20180115031401.19577-1-j.neuschaefer@gmx.net>
The Hollywood chipset's GPIO controller has two sets of registers: One
for access by the PowerPC CPU, and one for access by the ARM coprocessor
(but both are accessible from the PPC because the memory firewall
(AHBPROT) is usually disabled when booting Linux, today).
The wii_power_off function currently assumes that the poweroff GPIO pin
is configured for use via the ARM side, but the upcoming GPIO driver
configures all pins for use via the PPC side, breaking poweroff.
Configure the owner register explicitly in wii_power_off to make
wii_power_off work with and without the new GPIO driver.
I think the Wii can be switched to the generic gpio-poweroff driver,
after the GPIO driver is merged.
Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
---
arch/powerpc/platforms/embedded6xx/wii.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/powerpc/platforms/embedded6xx/wii.c b/arch/powerpc/platforms/embedded6xx/wii.c
index 79a1fe54ebc9..6e6db1e16d71 100644
--- a/arch/powerpc/platforms/embedded6xx/wii.c
+++ b/arch/powerpc/platforms/embedded6xx/wii.c
@@ -45,6 +45,7 @@
#define HW_GPIO_BASE(idx) (idx * 0x20)
#define HW_GPIO_OUT(idx) (HW_GPIO_BASE(idx) + 0)
#define HW_GPIO_DIR(idx) (HW_GPIO_BASE(idx) + 4)
+#define HW_GPIO_OWNER (HW_GPIO_BASE(1) + 0x1c)
#define HW_GPIO_SHUTDOWN (1<<1)
#define HW_GPIO_SLOT_LED (1<<5)
@@ -177,6 +178,12 @@ static void wii_power_off(void)
local_irq_disable();
if (hw_gpio) {
+ /*
+ * set the owner of the shutdown pin to ARM, because it is
+ * accessed through the registers for the ARM, below
+ */
+ clrbits32(hw_gpio + HW_GPIO_OWNER, HW_GPIO_SHUTDOWN);
+
/* make sure that the poweroff GPIO is configured as output */
setbits32(hw_gpio + HW_GPIO_DIR(1), HW_GPIO_SHUTDOWN);
--
2.15.1
^ permalink raw reply related
* [PATCH 5/6] powerpc: wii.dts: Add ngpios property
From: Jonathan Neuschäfer @ 2018-01-15 3:14 UTC (permalink / raw)
To: linux-kernel
Cc: linuxppc-dev, linux-gpio, devicetree, Jonathan Neuschäfer,
Rob Herring, Mark Rutland, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman
In-Reply-To: <20180115031401.19577-1-j.neuschaefer@gmx.net>
The Hollywood GPIO controller supports 32 GPIOs, but on the Wii, only 24
are used.
Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
---
arch/powerpc/boot/dts/wii.dts | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/boot/dts/wii.dts b/arch/powerpc/boot/dts/wii.dts
index 40b324b6391e..7235e375919c 100644
--- a/arch/powerpc/boot/dts/wii.dts
+++ b/arch/powerpc/boot/dts/wii.dts
@@ -176,6 +176,7 @@
compatible = "nintendo,hollywood-gpio";
reg = <0x0d8000c0 0x40>;
gpio-controller;
+ ngpios = <24>;
/*
* This is commented out while a standard binding
--
2.15.1
^ permalink raw reply related
* [PATCH 6/6] powerpc: wii.dts: Add GPIO line names
From: Jonathan Neuschäfer @ 2018-01-15 3:14 UTC (permalink / raw)
To: linux-kernel
Cc: linuxppc-dev, linux-gpio, devicetree, Jonathan Neuschäfer,
Rob Herring, Mark Rutland, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman
In-Reply-To: <20180115031401.19577-1-j.neuschaefer@gmx.net>
These are the GPIO line names on a Nintendo Wii, as documented in:
https://wiibrew.org/wiki/Hardware/Hollywood_GPIOs
Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
---
arch/powerpc/boot/dts/wii.dts | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/arch/powerpc/boot/dts/wii.dts b/arch/powerpc/boot/dts/wii.dts
index 7235e375919c..07d5e84e98b1 100644
--- a/arch/powerpc/boot/dts/wii.dts
+++ b/arch/powerpc/boot/dts/wii.dts
@@ -178,6 +178,14 @@
gpio-controller;
ngpios = <24>;
+ gpio-line-names =
+ "POWER", "SHUTDOWN", "FAN", "DC_DC",
+ "DI_SPIN", "SLOT_LED", "EJECT_BTN", "SLOT_IN",
+ "SENSOR_BAR", "DO_EJECT", "EEP_CS", "EEP_CLK",
+ "EEP_MOSI", "EEP_MISO", "AVE_SCL", "AVE_SDA",
+ "DEBUG0", "DEBUG1", "DEBUG2", "DEBUG3",
+ "DEBUG4", "DEBUG5", "DEBUG6", "DEBUG7";
+
/*
* This is commented out while a standard binding
* for i2c over gpio is defined.
--
2.15.1
^ permalink raw reply related
* [PATCH 3/6] gpio: Add GPIO driver for Nintendo Wii
From: Jonathan Neuschäfer @ 2018-01-15 3:13 UTC (permalink / raw)
To: linux-kernel
Cc: linuxppc-dev, linux-gpio, devicetree, Jonathan Neuschäfer,
Albert Herranz, Segher Boessenkool, Linus Walleij
In-Reply-To: <20180115031401.19577-1-j.neuschaefer@gmx.net>
The Nintendo Wii's chipset (called "Hollywood") has a GPIO controller
that supports a configurable number of pins (up to 32), interrupts, and
some special mechanisms to share the controller between the system's
security processor (an ARM926) and the PowerPC CPU. Pin multiplexing is
not supported.
This patch adds a basic driver for this GPIO controller. Interrupt
support will come in a later patch.
This patch is based on code developed by Albert Herranz and the GameCube
Linux Team, file arch/powerpc/platforms/embedded6xx/hlwd-gpio.c,
available at https://github.com/DeltaResero/GC-Wii-Linux-Kernels, but
has grown quite dissimilar.
To compare this version of the driver against the original code:
$ git fetch https://github.com/DeltaResero/GC-Wii-Linux-Kernels
$ git co FETCH_HEAD -- arch/powerpc/platforms/embedded6xx/hlwd-gpio.c
$ diff -u arch/powerpc/platforms/embedded6xx/hlwd-gpio.c \
drivers/gpio/gpio-hlwd.c
Cc: Albert Herranz <albert_herranz@yahoo.es>
Cc: Segher Boessenkool <segher@kernel.crashing.org>
Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
---
This driver currently uses __raw_readl and __raw_writel to access the
GPIO controller's MMIO registers. I wonder if readl/writel plus explicit
byte-swapping would be more correct, because it could be independent of
the CPU's endianness. That said, this hardware only exists in two
big-endian machines (Wii and Wii U).
---
drivers/gpio/Kconfig | 8 +++
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-hlwd.c | 183 +++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 192 insertions(+)
create mode 100644 drivers/gpio/gpio-hlwd.c
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index d6a8e851ad13..4f85c2053f7d 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -229,6 +229,14 @@ config GPIO_GRGPIO
Select this to support Aeroflex Gaisler GRGPIO cores from the GRLIB
VHDL IP core library.
+config GPIO_HLWD
+ tristate "Nintendo Wii (Hollywood) GPIO"
+ depends on OF_GPIO
+ help
+ Select this to support the GPIO controller of the Nintendo Wii.
+
+ If unsure, say N.
+
config GPIO_ICH
tristate "Intel ICH GPIO"
depends on PCI && X86
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 4bc24febb889..492f62d0eb59 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -54,6 +54,7 @@ obj-$(CONFIG_GPIO_FTGPIO010) += gpio-ftgpio010.o
obj-$(CONFIG_GPIO_GE_FPGA) += gpio-ge.o
obj-$(CONFIG_GPIO_GPIO_MM) += gpio-gpio-mm.o
obj-$(CONFIG_GPIO_GRGPIO) += gpio-grgpio.o
+obj-$(CONFIG_GPIO_HLWD) += gpio-hlwd.o
obj-$(CONFIG_HTC_EGPIO) += gpio-htc-egpio.o
obj-$(CONFIG_GPIO_ICH) += gpio-ich.o
obj-$(CONFIG_GPIO_INGENIC) += gpio-ingenic.o
diff --git a/drivers/gpio/gpio-hlwd.c b/drivers/gpio/gpio-hlwd.c
new file mode 100644
index 000000000000..0f8942ea6ed6
--- /dev/null
+++ b/drivers/gpio/gpio-hlwd.c
@@ -0,0 +1,183 @@
+// SPDX-License-Identifier: GPL-2.0+
+// Copyright (C) 2008-2009 The GameCube Linux Team
+// Copyright (C) 2008,2009 Albert Herranz
+// Copyright (C) 2017-2018 Jonathan Neuschäfer
+//
+// Nintendo Wii (Hollywood) GPIO driver
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
+#include <linux/of_platform.h>
+#include <linux/slab.h>
+#include <linux/module.h>
+#include <linux/gpio/driver.h>
+#include <linux/spinlock.h>
+
+/*
+ * Register names and offsets courtesy of WiiBrew:
+ * https://wiibrew.org/wiki/Hardware/Hollywood_GPIOs
+ *
+ * Note that for most registers, there are two versions:
+ * - HW_GPIOB_* Is always accessible by the Broadway PowerPC core, but does
+ * always give access to all GPIO lines
+ * - HW_GPIO_* Is only accessible by the Broadway PowerPC code if the memory
+ * firewall (AHBPROT) in the Hollywood chipset has been configured to allow
+ * such access.
+ *
+ * The ownership of each GPIO line can be configured in the HW_GPIO_OWNER
+ * register: A one bit configures the line for access via the HW_GPIOB_*
+ * registers, a zero bit indicates access via HW_GPIO_*. This driver uses
+ * HW_GPIOB_*.
+ */
+#define HW_GPIOB_OUT 0x00
+#define HW_GPIOB_DIR 0x04
+#define HW_GPIOB_IN 0x08
+#define HW_GPIOB_INTLVL 0x0c
+#define HW_GPIOB_INTFLAG 0x10
+#define HW_GPIOB_INTMASK 0x14
+#define HW_GPIOB_INMIR 0x18
+#define HW_GPIO_ENABLE 0x1c
+#define HW_GPIO_OUT 0x20
+#define HW_GPIO_DIR 0x24
+#define HW_GPIO_IN 0x28
+#define HW_GPIO_INTLVL 0x2c
+#define HW_GPIO_INTFLAG 0x30
+#define HW_GPIO_INTMASK 0x34
+#define HW_GPIO_INMIR 0x38
+#define HW_GPIO_OWNER 0x3c
+
+
+struct hlwd_gpio {
+ struct gpio_chip gpioc;
+ void __iomem *regs;
+ spinlock_t lock;
+};
+
+/*
+ * Update the bit with the given bit offset in the given register to a given
+ * value
+ */
+static void hlwd_gpio_update_bit(struct gpio_chip *gc, unsigned int reg,
+ int offset, int value)
+{
+ struct hlwd_gpio *hlwd = gpiochip_get_data(gc);
+ unsigned long flags;
+ u32 bit = 1UL << offset;
+ u32 tmp;
+
+ spin_lock_irqsave(&hlwd->lock, flags);
+ tmp = __raw_readl(hlwd->regs + reg);
+ if (value)
+ __raw_writel(tmp | bit, hlwd->regs + reg);
+ else
+ __raw_writel(tmp & ~bit, hlwd->regs + reg);
+ spin_unlock_irqrestore(&hlwd->lock, flags);
+}
+
+/* Read the bit with the given bit offset in the given register */
+static int hlwd_gpio_read_bit(struct gpio_chip *gc, unsigned int reg,
+ unsigned int offset)
+{
+ struct hlwd_gpio *hlwd = gpiochip_get_data(gc);
+ unsigned long flags;
+ u32 bit = 1UL << offset;
+ u32 tmp;
+
+ spin_lock_irqsave(&hlwd->lock, flags);
+ tmp = __raw_readl(hlwd->regs + reg);
+ spin_unlock_irqrestore(&hlwd->lock, flags);
+
+ return !!(tmp & bit);
+}
+
+static int hlwd_gpio_get(struct gpio_chip *gc, unsigned int offset)
+{
+ return hlwd_gpio_read_bit(gc, HW_GPIOB_IN, offset);
+}
+
+static void hlwd_gpio_set(struct gpio_chip *gc, unsigned int offset, int val)
+{
+ hlwd_gpio_update_bit(gc, HW_GPIOB_OUT, offset, val);
+}
+
+static int hlwd_gpio_dir_in(struct gpio_chip *gc, unsigned int offset)
+{
+ hlwd_gpio_update_bit(gc, HW_GPIOB_DIR, offset, 0);
+
+ return 0;
+}
+
+static int hlwd_gpio_dir_out(struct gpio_chip *gc,
+ unsigned int offset, int val)
+{
+ /* Set the GPIO value, and then set the direction */
+ hlwd_gpio_set(gc, offset, val);
+ hlwd_gpio_update_bit(gc, HW_GPIOB_DIR, offset, 1);
+
+ return 0;
+}
+
+static int hlwd_gpio_probe(struct platform_device *pdev)
+{
+ struct hlwd_gpio *hlwd;
+ struct resource *regs_resource;
+ u32 ngpios;
+
+ hlwd = devm_kzalloc(&pdev->dev, sizeof(*hlwd), GFP_KERNEL);
+ if (!hlwd)
+ return -ENOMEM;
+
+ regs_resource = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (IS_ERR(regs_resource))
+ return PTR_ERR(regs_resource);
+
+ hlwd->regs = devm_ioremap_resource(&pdev->dev, regs_resource);
+ if (IS_ERR(hlwd->regs))
+ return PTR_ERR(hlwd->regs);
+
+ /*
+ * Claim all GPIOs using the OWNER register. This will not work on
+ * systems where the AHBPROT memory firewall hasn't been configured to
+ * permit PPC access to HW_GPIO_*.
+ */
+ __raw_writel(0xffffffff, hlwd->regs + HW_GPIO_OWNER);
+
+ spin_lock_init(&hlwd->lock);
+
+ hlwd->gpioc.label = dev_name(&pdev->dev);
+ hlwd->gpioc.parent = &pdev->dev;
+ hlwd->gpioc.owner = THIS_MODULE;
+ hlwd->gpioc.direction_input = hlwd_gpio_dir_in;
+ hlwd->gpioc.direction_output = hlwd_gpio_dir_out;
+ hlwd->gpioc.get = hlwd_gpio_get;
+ hlwd->gpioc.set = hlwd_gpio_set;
+
+ if (of_property_read_u32(pdev->dev.of_node, "ngpios", &ngpios))
+ ngpios = 32;
+ hlwd->gpioc.ngpio = ngpios;
+
+ return devm_gpiochip_add_data(&pdev->dev, &hlwd->gpioc, hlwd);
+}
+
+static const struct of_device_id hlwd_gpio_match[] = {
+ { .compatible = "nintendo,hollywood-gpio", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, hlwd_gpio_match);
+
+static struct platform_driver hlwd_gpio_driver = {
+ .driver = {
+ .name = "hlwd_gpio",
+ .of_match_table = hlwd_gpio_match,
+ },
+ .probe = hlwd_gpio_probe,
+};
+module_platform_driver(hlwd_gpio_driver);
+
+MODULE_AUTHOR("Jonathan Neuschäfer <j.neuschaefer@gmx.net>");
+MODULE_DESCRIPTION("Nintendo Wii GPIO driver");
+MODULE_LICENSE("GPL");
--
2.15.1
^ permalink raw reply related
* Re: [cryptodev:master 130/134] aes_generic.c:undefined reference to `_restgpr_31_x'
From: Segher Boessenkool @ 2018-01-15 0:21 UTC (permalink / raw)
To: Arnd Bergmann
Cc: open list:HARDWARE RANDOM NUMBER GENERATOR CORE,
kbuild test robot, linuxppc-dev, Herbert Xu
In-Reply-To: <CAK8P3a30k-JjMJ0M9KWXqUMT3eHmuuFvAe1KuKdqNoAiSZrgog@mail.gmail.com>
On Sun, Jan 14, 2018 at 10:40:36PM +0100, Arnd Bergmann wrote:
> Right. I've done some more investigation anyway, starting over with the
> analysis of the gcc options that change it. I've found now that turning
> off '-fcode-hoisting' but leaving on the other options I had suspected
> earlier (-O2 instead of -Os, -ftree-sra, -ftree-pre) also fixes the
> stack problem, and appears to result in the best performance so
> far.
Oh nice!
> I need to rerun the whole test matrix, but that seems rather
> promising, and the result may also help debug what's really happening.
-fcode-hoisting moves all expression evaluation to as early as possible;
for this AES code that means it will increase register pressure a lot,
causing a lot of spilling (well, that is my guess). If that is so, then
we need to dial down -fcode-hoisting a bit, maybe make it aware of
register pressure.
Glad you found a smoking gun,
Segher
^ permalink raw reply
* Re: [PATCH v2 14/16] ASoC: fsl_ssi: Remove cpu_dai_drv from fsl_ssi structure
From: Nicolin Chen @ 2018-01-15 0:13 UTC (permalink / raw)
To: Maciej S. Szmigiero
Cc: timur, broonie, linux-kernel, linuxppc-dev, alsa-devel, lgirdwood,
fabio.estevam, caleb, arnaud.mouiche, lukma, kernel
In-Reply-To: <4626cd37-c84c-6187-51ae-73fa419b801e@maciej.szmigiero.name>
On Sun, Jan 14, 2018 at 11:42:59PM +0100, Maciej S. Szmigiero wrote:
> On 11.01.2018 07:43, Nicolin Chen wrote:
> > The cpu_dai_drv is only used for symmetric_rates. So this patch replaces
> > it with a synchronous boolean flag.
>
> You make cpu_dai_drv common to all SSI instances instead of per-instance.
>
> What if you have multiple SSIs in the system with different
> symmetric_{rates,samplebits,channels} settings?
Good catch...it should maintain each cpu_dai_drv separately. Thanks!
^ permalink raw reply
* Re: [PATCH v2 13/16] ASoC: fsl_ssi: Clean up _fsl_ssi_set_dai_fmt()
From: Nicolin Chen @ 2018-01-15 0:11 UTC (permalink / raw)
To: Maciej S. Szmigiero
Cc: timur, broonie, linux-kernel, linuxppc-dev, alsa-devel, lgirdwood,
fabio.estevam, caleb, arnaud.mouiche, lukma, kernel
In-Reply-To: <2e285df9-834b-41bf-1a09-a8de3f2a1f7c@maciej.szmigiero.name>
On Sun, Jan 14, 2018 at 11:40:31PM +0100, Maciej S. Szmigiero wrote:
> > case SND_SOC_DAIFMT_I2S:
> > - regmap_update_bits(regs, REG_SSI_STCCR,
> > - SSI_SxCCR_DC_MASK, SSI_SxCCR_DC(2));
> > - regmap_update_bits(regs, REG_SSI_SRCCR,
> > - SSI_SxCCR_DC_MASK, SSI_SxCCR_DC(2));
> > switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
> > case SND_SOC_DAIFMT_CBM_CFS:
> > case SND_SOC_DAIFMT_CBS_CFS:
> > + if (IS_ERR(ssi->baudclk)) {
> > + dev_err(ssi->dev,
> > + "missing baudclk for master mode\n");
> > + return -EINVAL;
> > + }
>
> The original code did this check only for fsl_ssi_is_i2s_master(ssi),
> that is, only for SND_SOC_DAIFMT_CBS_CFS while here you also do it for
> SND_SOC_DAIFMT_CBM_CFS.
You are right. This patch isn't supposed to change that. I mixed an
intention from another patch. Will revise this part in the v3.
Thanks a lot
^ permalink raw reply
* Re: [PATCH v2 04/16] ASoC: fsl_ssi: Rename fsl_ssi_disable_val macro
From: Nicolin Chen @ 2018-01-15 0:03 UTC (permalink / raw)
To: Maciej S. Szmigiero
Cc: timur, broonie, linux-kernel, linuxppc-dev, alsa-devel, lgirdwood,
fabio.estevam, caleb, arnaud.mouiche, lukma, kernel
In-Reply-To: <6dd7d3e8-4673-c56d-c0ce-80df1f537f9b@maciej.szmigiero.name>
On Sun, Jan 14, 2018 at 11:34:37PM +0100, Maciej S. Szmigiero wrote:
> > + /* Check if the opposite stream is active */
> > + aactive = ssi->streams & BIT(!dir);
> ^
> Here an implicit assumption that either RX == 0, TX == 1 or
> RX == 1, TX == 0 still remains.
I would try to get rid of this !dir. However the regvals is
defined as regvals[2] so this assumption (either RX == 0 or
TX == 0) doesn't look wrong to me. I would prefer to add a
comment to limit a potential modification to RX and TX.
^ permalink raw reply
* Re: [PATCH v2 03/16] ASoC: fsl_ssi: Maintain a mask of active streams
From: Nicolin Chen @ 2018-01-14 23:57 UTC (permalink / raw)
To: Maciej S. Szmigiero
Cc: timur, broonie, linux-kernel, linuxppc-dev, alsa-devel, lgirdwood,
fabio.estevam, caleb, arnaud.mouiche, lukma, kernel
In-Reply-To: <1488354c-99b0-fa64-a88c-0fa065df4b7d@maciej.szmigiero.name>
On Sun, Jan 14, 2018 at 11:34:01PM +0100, Maciej S. Szmigiero wrote:
> > + bool dir = (&ssi->regvals[TX] == vals) ? TX : RX;
> Using a bool variable for a bit index (and array index in other parts
> of code) looks just wrong.
>
> Even a simple int would look better IMHO here (and in patch 5 that
> rewrites this line a bit).
Will change to int. Thanks
^ permalink raw reply
* Re: [PATCH] EDAC, mv64x60: Remove some code duplication
From: Chris Packham @ 2018-01-14 22:48 UTC (permalink / raw)
To: Christophe JAILLET, Borislav Petkov
Cc: mchehab@kernel.org, linux-edac@vger.kernel.org,
linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org, mpe@ellerman.id.au
In-Reply-To: <49db37d7-3d54-3ae5-9cb9-702ef14f1f43@wanadoo.fr>
Hi Christophe,=0A=
=0A=
On 14/01/18 06:17, Christophe JAILLET wrote:=0A=
> Le 13/01/2018 =E0 15:22, Borislav Petkov a =E9crit=A0:=0A=
>> + Chris Packham who's been fixing some stuff in here too.=0A=
>>=0A=
>> On Sat, Jan 13, 2018 at 08:28:21AM +0100, Christophe JAILLET wrote:=0A=
>>> Reorder the error handling code in order to release the resources in=0A=
>>> reverse order than allocation.=0A=
>>>=0A=
>>> Introduce a new 'release_group' label in the error handling path and us=
e=0A=
>>> it to void some code duplication.=0A=
>>>=0A=
>>> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>=0A=
>>> ---=0A=
>>> drivers/edac/mv64x60_edac.c | 7 ++++---=0A=
>>> 1 file changed, 4 insertions(+), 3 deletions(-)=0A=
>>>=0A=
>>> diff --git a/drivers/edac/mv64x60_edac.c b/drivers/edac/mv64x60_edac.c=
=0A=
>>> index 3c68bb525d5d..aa5bc1d8f424 100644=0A=
>>> --- a/drivers/edac/mv64x60_edac.c=0A=
>>> +++ b/drivers/edac/mv64x60_edac.c=0A=
>>> @@ -450,8 +450,8 @@ static int mv64x60_cpu_err_probe(struct platform_de=
vice *pdev)=0A=
>>> "cpu", 1, NULL, 0, 0, NULL, 0,=0A=
>>> edac_dev_idx);=0A=
>>> if (!edac_dev) {=0A=
>>> - devres_release_group(&pdev->dev, mv64x60_cpu_err_probe);=0A=
>>> - return -ENOMEM;=0A=
>>> + res =3D -ENOMEM;=0A=
>>> + goto release_group;=0A=
>>> }=0A=
>>> =0A=
>>> pdata =3D edac_dev->pvt_info;=0A=
>>> @@ -561,8 +561,9 @@ static int mv64x60_cpu_err_probe(struct platform_de=
vice *pdev)=0A=
>>> err2:=0A=
>>> edac_device_del_device(&pdev->dev);=0A=
>>> err:=0A=
>>> - devres_release_group(&pdev->dev, mv64x60_cpu_err_probe);=0A=
>>> edac_device_free_ctl_info(edac_dev);=0A=
>>> +release_group:=0A=
>>> + devres_release_group(&pdev->dev, mv64x60_cpu_err_probe);=0A=
>>> return res;=0A=
>>> }=0A=
>>> =0A=
>>> -- =0A=
>> Thanks, looks good. But looking at this driver, mv64x60_mc_err_probe()=
=0A=
>> and mv64x60_sram_err_probe() have the same problem too. Can you address =
them=0A=
>> with your patch too pls?=0A=
> Will do. mv64x60_pci_err_probe() also needs some tweaks.=0A=
> =0A=
>> Also, if you feel like fixing more stuff in this driver, it doesn't use=
=0A=
>> the edac_printk() infrastructure but naked printk() calls. It could be=
=0A=
>> converted to it.=0A=
> I will only propose to remove a useless message and improve another one,=
=0A=
> but won't convert the whole driver, sorry.=0A=
> =0A=
=0A=
I take this you mean you have a system with a mv64x60 SoC? You might =0A=
want to make yourself known to the linuxppc-dev list. A while back the =0A=
prospects of dropping CONFIG_MV64X60 was raised[1]. I don't see anyone =0A=
actually following through on this yet but I'm not really following =0A=
linuxppc that closely.=0A=
=0A=
[1] - https://marc.info/?l=3Dlinux-edac&m=3D149518763115206&w=3D2=0A=
^ permalink raw reply
* Re: [PATCH v2 14/16] ASoC: fsl_ssi: Remove cpu_dai_drv from fsl_ssi structure
From: Maciej S. Szmigiero @ 2018-01-14 22:42 UTC (permalink / raw)
To: Nicolin Chen
Cc: timur, broonie, linux-kernel, linuxppc-dev, alsa-devel, lgirdwood,
fabio.estevam, caleb, arnaud.mouiche, lukma, kernel
In-Reply-To: <1515652995-15996-15-git-send-email-nicoleotsuka@gmail.com>
On 11.01.2018 07:43, Nicolin Chen wrote:
> The cpu_dai_drv is only used for symmetric_rates. So this patch replaces
> it with a synchronous boolean flag.
You make cpu_dai_drv common to all SSI instances instead of per-instance.
What if you have multiple SSIs in the system with different
symmetric_{rates,samplebits,channels} settings?
Maciej
> Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com>
> Tested-by: Caleb Crome <caleb@crome.org>
> ---
> sound/soc/fsl/fsl_ssi.c | 32 ++++++++++++++++----------------
> 1 file changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
> index 213962a..716603c 100644
> --- a/sound/soc/fsl/fsl_ssi.c
> +++ b/sound/soc/fsl/fsl_ssi.c
> @@ -208,11 +208,11 @@ struct fsl_ssi_soc_data {
> *
> * @regs: Pointer to the regmap registers
> * @irq: IRQ of this SSI
> - * @cpu_dai_drv: CPU DAI driver for this device
> *
> * @dai_fmt: DAI configuration this device is currently used with
> * @streams: Mask of current active streams: BIT(TX) and BIT(RX)
> * @i2s_net: I2S and Network mode configurations of SCR register
> + * @synchronous: Use synchronous mode - both of TX and RX use STCK and SFCK
> * @use_dma: DMA is used or FIQ with stream filter
> * @use_dual_fifo: DMA with support for dual FIFO mode
> * @has_ipg_clk_name: If "ipg" is in the clock name list of device tree
> @@ -253,11 +253,11 @@ struct fsl_ssi_soc_data {
> struct fsl_ssi {
> struct regmap *regs;
> int irq;
> - struct snd_soc_dai_driver cpu_dai_drv;
>
> unsigned int dai_fmt;
> u8 streams;
> u8 i2s_net;
> + bool synchronous;
> bool use_dma;
> bool use_dual_fifo;
> bool has_ipg_clk_name;
> @@ -668,7 +668,6 @@ static int fsl_ssi_set_bclk(struct snd_pcm_substream *substream,
> bool tx2, tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
> struct fsl_ssi *ssi = snd_soc_dai_get_drvdata(dai);
> struct regmap *regs = ssi->regs;
> - int synchronous = ssi->cpu_dai_drv.symmetric_rates, ret;
> u32 pm = 999, div2, psr, stccr, mask, afreq, factor, i;
> unsigned long clkrate, baudrate, tmprate;
> unsigned int slots = params_channels(hw_params);
> @@ -676,6 +675,7 @@ static int fsl_ssi_set_bclk(struct snd_pcm_substream *substream,
> u64 sub, savesub = 100000;
> unsigned int freq;
> bool baudclk_is_used;
> + int ret;
>
> /* Override slots and slot_width if being specifically set... */
> if (ssi->slots)
> @@ -754,7 +754,7 @@ static int fsl_ssi_set_bclk(struct snd_pcm_substream *substream,
> mask = SSI_SxCCR_PM_MASK | SSI_SxCCR_DIV2 | SSI_SxCCR_PSR;
>
> /* STCCR is used for RX in synchronous mode */
> - tx2 = tx || synchronous;
> + tx2 = tx || ssi->synchronous;
> regmap_update_bits(regs, REG_SSI_SxCCR(tx2), mask, stccr);
>
> if (!baudclk_is_used) {
> @@ -802,7 +802,7 @@ static int fsl_ssi_hw_params(struct snd_pcm_substream *substream,
> * that should set separate configurations for STCCR and SRCCR
> * despite running in the synchronous mode.
> */
> - if (enabled && ssi->cpu_dai_drv.symmetric_rates)
> + if (enabled && ssi->synchronous)
> return 0;
>
> if (fsl_ssi_is_i2s_master(ssi)) {
> @@ -834,7 +834,7 @@ static int fsl_ssi_hw_params(struct snd_pcm_substream *substream,
> }
>
> /* In synchronous mode, the SSI uses STCCR for capture */
> - tx2 = tx || ssi->cpu_dai_drv.symmetric_rates;
> + tx2 = tx || ssi->synchronous;
> regmap_update_bits(regs, REG_SSI_SxCCR(tx2), SSI_SxCCR_WL_MASK, wl);
>
> return 0;
> @@ -959,7 +959,7 @@ static int _fsl_ssi_set_dai_fmt(struct fsl_ssi *ssi, unsigned int fmt)
> srcr = strcr;
>
> /* Set SYN mode and clear RXDIR bit when using SYN or AC97 mode */
> - if (ssi->cpu_dai_drv.symmetric_rates || fsl_ssi_is_ac97(ssi)) {
> + if (ssi->synchronous || fsl_ssi_is_ac97(ssi)) {
> srcr &= ~SSI_SRCR_RXDIR;
> scr |= SSI_SCR_SYN;
> }
> @@ -1360,6 +1360,7 @@ static void fsl_ssi_imx_clean(struct platform_device *pdev, struct fsl_ssi *ssi)
>
> static int fsl_ssi_probe(struct platform_device *pdev)
> {
> + struct snd_soc_dai_driver *cpu_dai_drv;
> struct fsl_ssi *ssi;
> int ret = 0;
> struct device_node *np = pdev->dev.of_node;
> @@ -1394,14 +1395,12 @@ static int fsl_ssi_probe(struct platform_device *pdev)
> ssi->use_dma = !of_property_read_bool(np, "fsl,fiq-stream-filter");
>
> if (fsl_ssi_is_ac97(ssi)) {
> - memcpy(&ssi->cpu_dai_drv, &fsl_ssi_ac97_dai,
> - sizeof(fsl_ssi_ac97_dai));
> + cpu_dai_drv = &fsl_ssi_ac97_dai;
> fsl_ac97_data = ssi;
> } else {
> - memcpy(&ssi->cpu_dai_drv, &fsl_ssi_dai_template,
> - sizeof(fsl_ssi_dai_template));
> + cpu_dai_drv = &fsl_ssi_dai_template;
> }
> - ssi->cpu_dai_drv.name = dev_name(dev);
> + cpu_dai_drv->name = dev_name(dev);
>
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> iomem = devm_ioremap_resource(dev, res);
> @@ -1439,11 +1438,12 @@ static int fsl_ssi_probe(struct platform_device *pdev)
> /* Set software limitations for synchronous mode */
> if (!of_find_property(np, "fsl,ssi-asynchronous", NULL)) {
> if (!fsl_ssi_is_ac97(ssi)) {
> - ssi->cpu_dai_drv.symmetric_rates = 1;
> - ssi->cpu_dai_drv.symmetric_samplebits = 1;
> + cpu_dai_drv->symmetric_rates = 1;
> + cpu_dai_drv->symmetric_samplebits = 1;
> + ssi->synchronous = true;
> }
>
> - ssi->cpu_dai_drv.symmetric_channels = 1;
> + cpu_dai_drv->symmetric_channels = 1;
> }
>
> /* Fetch FIFO depth; Set to 8 for older DT without this property */
> @@ -1498,7 +1498,7 @@ static int fsl_ssi_probe(struct platform_device *pdev)
> }
>
> ret = devm_snd_soc_register_component(dev, &fsl_ssi_component,
> - &ssi->cpu_dai_drv, 1);
> + cpu_dai_drv, 1);
> if (ret) {
> dev_err(dev, "failed to register DAI: %d\n", ret);
> goto error_asoc_register;
>
^ permalink raw reply
* Re: [PATCH v2 13/16] ASoC: fsl_ssi: Clean up _fsl_ssi_set_dai_fmt()
From: Maciej S. Szmigiero @ 2018-01-14 22:40 UTC (permalink / raw)
To: Nicolin Chen
Cc: timur, broonie, linux-kernel, linuxppc-dev, alsa-devel, lgirdwood,
fabio.estevam, caleb, arnaud.mouiche, lukma, kernel
In-Reply-To: <1515652995-15996-14-git-send-email-nicoleotsuka@gmail.com>
On 11.01.2018 07:43, Nicolin Chen wrote:
> The _fsl_ssi_set_dai_fmt() is a helper function being called from
> fsl_ssi_set_dai_fmt() as an ASoC operation and fsl_ssi_hw_init()
> mainly for AC97 format initialization.
>
> This patch cleans the _fsl_ssi_set_dai_fmt() in following ways:
> * Removing *dev pointer in the parameters as it's included in the
> *ssi pointer of struct fsl_ssi.
> * Using regmap_update_bits() instead of regmap_read() with masking
> the value manually.
> * Removing TXBIT0 configurations since this bit is set to 1 as its
> reset value and there is no use case so far to unset it. And it
> is safe to remove since regmap_update_bits() won't touch it.
The old code set this bit in any mode other than AC'97 (where the
hardware always treats this bit as set regardless of the actual value).
I would play safe here and not rely on this bit being set by a SSI
reset on all SSI models.
> * Moving baudclk check to the switch-case routine to skip the I2S
> master check. And moving SxCCR.DC settings after baudclk check.
> * Adding format settings for SND_SOC_DAIFMT_AC97 like others.
>
> Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com>
> Tested-by: Caleb Crome <caleb@crome.org>
> ---
> sound/soc/fsl/fsl_ssi.c | 70 ++++++++++++++++++++++---------------------------
> 1 file changed, 31 insertions(+), 39 deletions(-)
>
> diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
> index 178c192..213962a 100644
> --- a/sound/soc/fsl/fsl_ssi.c
> +++ b/sound/soc/fsl/fsl_ssi.c
> @@ -855,42 +855,27 @@ static int fsl_ssi_hw_free(struct snd_pcm_substream *substream,
> return 0;
> }
>
> -static int _fsl_ssi_set_dai_fmt(struct device *dev,
> - struct fsl_ssi *ssi, unsigned int fmt)
> +static int _fsl_ssi_set_dai_fmt(struct fsl_ssi *ssi, unsigned int fmt)
> {
> - struct regmap *regs = ssi->regs;
> - u32 strcr = 0, stcr, srcr, scr, mask;
> + u32 strcr = 0, scr = 0, stcr, srcr, mask;
>
> ssi->dai_fmt = fmt;
>
> - if (fsl_ssi_is_i2s_master(ssi) && IS_ERR(ssi->baudclk)) {
> - dev_err(dev, "missing baudclk for master mode\n");
> - return -EINVAL;
> - }
> -
> - regmap_read(regs, REG_SSI_SCR, &scr);
> - scr &= ~(SSI_SCR_SYN | SSI_SCR_I2S_MODE_MASK);
> /* Synchronize frame sync clock for TE to avoid data slipping */
> scr |= SSI_SCR_SYNC_TX_FS;
>
> - mask = SSI_STCR_TXBIT0 | SSI_STCR_TFDIR | SSI_STCR_TXDIR |
> - SSI_STCR_TSCKP | SSI_STCR_TFSI | SSI_STCR_TFSL | SSI_STCR_TEFS;
> - regmap_read(regs, REG_SSI_STCR, &stcr);
> - regmap_read(regs, REG_SSI_SRCR, &srcr);
> - stcr &= ~mask;
> - srcr &= ~mask;
> -
> /* Use Network mode as default */
> ssi->i2s_net = SSI_SCR_NET;
> switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
> case SND_SOC_DAIFMT_I2S:
> - regmap_update_bits(regs, REG_SSI_STCCR,
> - SSI_SxCCR_DC_MASK, SSI_SxCCR_DC(2));
> - regmap_update_bits(regs, REG_SSI_SRCCR,
> - SSI_SxCCR_DC_MASK, SSI_SxCCR_DC(2));
> switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
> case SND_SOC_DAIFMT_CBM_CFS:
> case SND_SOC_DAIFMT_CBS_CFS:
> + if (IS_ERR(ssi->baudclk)) {
> + dev_err(ssi->dev,
> + "missing baudclk for master mode\n");
> + return -EINVAL;
> + }
The original code did this check only for fsl_ssi_is_i2s_master(ssi),
that is, only for SND_SOC_DAIFMT_CBS_CFS while here you also do it for
SND_SOC_DAIFMT_CBM_CFS.
Was this changed on purpose?
Maciej
^ permalink raw reply
* Re: [PATCH v2 06/16] ASoC: fsl_ssi: Clean up helper functions of trigger()
From: Maciej S. Szmigiero @ 2018-01-14 22:37 UTC (permalink / raw)
To: Nicolin Chen
Cc: timur, broonie, linux-kernel, linuxppc-dev, alsa-devel, lgirdwood,
fabio.estevam, caleb, arnaud.mouiche, lukma, kernel
In-Reply-To: <1515652995-15996-7-git-send-email-nicoleotsuka@gmail.com>
On 11.01.2018 07:43, Nicolin Chen wrote:
> The trigger() calls fsl_ssi_tx_config() and fsl_ssi_rx_config(),
> and both of them jump to fsl_ssi_config(). And fsl_ssi_config()
> later calls another fsl_ssi_rxtx_config().
>
> However, the whole routine, especially fsl_ssi_config() function,
> is too complicated because of the folowing reasons:
> 1) It has to handle the concern of the opposite stream.
> 2) It has to handle cases of offline configurations support.
> 3) It has to handle enable and disable operations while they're
> mostly different.
>
> Since the enable and disable routines have more differences than
> TX and RX rountines, this patch simplifies these helper functions
> with the following changes:
> - Changing to two helper functions of enable and disable instead
> of TX and RX.
> - Removing fsl_ssi_rxtx_config() by separately integrating it to
> two newly introduced enable & disable functions.
>
> Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com>
> Tested-by: Caleb Crome <caleb@crome.org>
> ---
> sound/soc/fsl/fsl_ssi.c | 256 +++++++++++++++++++++++-------------------------
> 1 file changed, 122 insertions(+), 134 deletions(-)
>
> diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
> index 263c067..09a571a 100644
> --- a/sound/soc/fsl/fsl_ssi.c
> +++ b/sound/soc/fsl/fsl_ssi.c
> @@ -378,31 +378,83 @@ static irqreturn_t fsl_ssi_isr(int irq, void *dev_id)
> }
>
> /**
> - * Enable or disable all rx/tx config flags at once
> + * Set SCR, SIER, STCR and SRCR registers with cached values in regvals
> + *
> + * Notes:
> + * 1) For offline_config SoCs, enable all necessary bits of both streams
> + * when 1st stream starts, even if the opposite stream will not start
> + * 2) It also clears FIFO before setting regvals; SOR is safe to set online
> */
> -static void fsl_ssi_rxtx_config(struct fsl_ssi *ssi, bool enable)
> +static void fsl_ssi_config_enable(struct fsl_ssi *ssi, bool tx)
> {
> - struct regmap *regs = ssi->regs;
> struct fsl_ssi_regvals *vals = ssi->regvals;
> + bool dir = tx ? TX : RX;
A similar case as in patch 3 of a bool variable used for a bit index.
> + u32 sier, srcr, stcr;
>
> - if (enable) {
> - regmap_update_bits(regs, REG_SSI_SIER,
> - vals[RX].sier | vals[TX].sier,
> - vals[RX].sier | vals[TX].sier);
> - regmap_update_bits(regs, REG_SSI_SRCR,
> - vals[RX].srcr | vals[TX].srcr,
> - vals[RX].srcr | vals[TX].srcr);
> - regmap_update_bits(regs, REG_SSI_STCR,
> - vals[RX].stcr | vals[TX].stcr,
> - vals[RX].stcr | vals[TX].stcr);
> + /* Clear dirty data in the FIFO; It also prevents channel slipping */
> + regmap_update_bits(ssi->regs, REG_SSI_SOR,
> + SSI_SOR_xX_CLR(tx), SSI_SOR_xX_CLR(tx));
> +
> + /*
> + * On offline_config SoCs, SxCR and SIER are already configured when
> + * the previous stream started. So skip all SxCR and SIER settings
> + * to prevent online reconfigurations, then jump to set SCR directly
> + */
> + if (ssi->soc->offline_config && ssi->streams)
> + goto enable_scr;
> +
> + if (ssi->soc->offline_config) {
> + /*
> + * Online reconfiguration not supported, so enable all bits for
> + * both streams at once to avoid necessity of reconfigurations
> + */
> + srcr = vals[RX].srcr | vals[TX].srcr;
> + stcr = vals[RX].stcr | vals[TX].stcr;
> + sier = vals[RX].sier | vals[TX].sier;
> } else {
> - regmap_update_bits(regs, REG_SSI_SRCR,
> - vals[RX].srcr | vals[TX].srcr, 0);
> - regmap_update_bits(regs, REG_SSI_STCR,
> - vals[RX].stcr | vals[TX].stcr, 0);
> - regmap_update_bits(regs, REG_SSI_SIER,
> - vals[RX].sier | vals[TX].sier, 0);
> + /* Otherwise, only set bits for the current stream */
> + srcr = vals[dir].srcr;
> + stcr = vals[dir].stcr;
> + sier = vals[dir].sier;
> }
> +
> + /* Configure SRCR, STCR and SIER at once */
> + regmap_update_bits(ssi->regs, REG_SSI_SRCR, srcr, srcr);
> + regmap_update_bits(ssi->regs, REG_SSI_STCR, stcr, stcr);
> + regmap_update_bits(ssi->regs, REG_SSI_SIER, sier, sier);
> +
> +enable_scr:
> + /*
> + * Start DMA before setting TE to avoid FIFO underrun
> + * which may cause a channel slip or a channel swap
> + *
> + * TODO: FIQ cases might also need this upon testing
> + */
> + if (ssi->use_dma && tx) {
> + int try = 100;
> + u32 sfcsr;
> +
> + /* Enable SSI first to send TX DMA request */
> + regmap_update_bits(ssi->regs, REG_SSI_SCR,
> + SSI_SCR_SSIEN, SSI_SCR_SSIEN);
> +
> + /* Busy wait until TX FIFO not empty -- DMA working */
> + do {
> + regmap_read(ssi->regs, REG_SSI_SFCSR, &sfcsr);
> + if (SSI_SFCSR_TFCNT0(sfcsr))
> + break;
> + } while (--try);
> +
> + /* FIFO still empty -- something might be wrong */
> + if (!SSI_SFCSR_TFCNT0(sfcsr))
> + dev_warn(ssi->dev, "Timeout waiting TX FIFO filling\n");
> + }
> + /* Enable all remaining bits in SCR */
> + regmap_update_bits(ssi->regs, REG_SSI_SCR,
> + vals[dir].scr, vals[dir].scr);
> +
> + /* Log the enabled stream to the mask */
> + ssi->streams |= BIT(dir);
> }
>
> /**
> @@ -426,66 +478,53 @@ static void fsl_ssi_rxtx_config(struct fsl_ssi *ssi, bool enable)
> ((vals) & _ssi_xor_shared_bits(vals, avals, aactive))
>
> /**
> - * Enable or disable SSI configuration.
> + * Unset SCR, SIER, STCR and SRCR registers with cached values in regvals
> + *
> + * Notes:
> + * 1) For offline_config SoCs, to avoid online reconfigurations, disable all
> + * bits of both streams at once when the last stream is abort to end
> + * 2) It also clears FIFO after unsetting regvals; SOR is safe to set online
> */
> -static void fsl_ssi_config(struct fsl_ssi *ssi, bool enable,
> - struct fsl_ssi_regvals *vals)
> +static void fsl_ssi_config_disable(struct fsl_ssi *ssi, bool tx)
> {
> - bool tx = &ssi->regvals[TX] == vals;
> + struct fsl_ssi_regvals *vals, *avals;
> + u32 sier, srcr, stcr, scr;
> bool dir = tx ? TX : RX;
> - struct regmap *regs = ssi->regs;
> - struct fsl_ssi_regvals *avals;
> bool aactive;
>
> /* Check if the opposite stream is active */
> aactive = ssi->streams & BIT(!dir);
>
> - /* Get the opposite direction to keep its values untouched */
> - if (&ssi->regvals[RX] == vals)
> - avals = &ssi->regvals[TX];
> - else
> - avals = &ssi->regvals[RX];
> + vals = &ssi->regvals[dir];>
> - if (!enable) {
> - /*
> - * To keep the other stream safe, exclude shared bits between
> - * both streams, and get safe bits to disable current stream
> - */
> - u32 scr = ssi_excl_shared_bits(vals->scr, avals->scr, aactive);
> - /* Safely disable SCR register for the stream */
> - regmap_update_bits(regs, REG_SSI_SCR, scr, 0);
> -
> - /* Log the disabled stream to the mask */
> - ssi->streams &= ~BIT(dir);
> - }
> + /* Get regvals of the opposite stream to keep opposite stream safe */
> + avals = &ssi->regvals[!dir];
^
The same implicit assumption here as in patch 4.
Maciej
^ permalink raw reply
* Re: [PATCH v2 04/16] ASoC: fsl_ssi: Rename fsl_ssi_disable_val macro
From: Maciej S. Szmigiero @ 2018-01-14 22:34 UTC (permalink / raw)
To: Nicolin Chen
Cc: timur, broonie, linux-kernel, linuxppc-dev, alsa-devel, lgirdwood,
fabio.estevam, caleb, arnaud.mouiche, lukma, kernel
In-Reply-To: <1515652995-15996-5-git-send-email-nicoleotsuka@gmail.com>
On 11.01.2018 07:43, Nicolin Chen wrote:
> The define of fsl_ssi_disable_val is not so clear as it mixes two
> steps of calculations together. And those parameter names are also
> a bit long to read.
>
> Since it just tries to exclude the shared bits from the regvals of
> current stream while the opposite stream is active, it's better to
> use something like ssi_excl_shared_bits.
>
> This patch also bisects fsl_ssi_disable_val into two macros of two
> corresponding steps and then shortens its parameter names. It also
> updates callers in the fsl_ssi_config() accordingly.
>
> Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com>
> Tested-by: Caleb Crome <caleb@crome.org>
> ---
> sound/soc/fsl/fsl_ssi.c | 54 ++++++++++++++++++++-----------------------------
> 1 file changed, 22 insertions(+), 32 deletions(-)
>
> diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
> index aa14a5d..f026386 100644
> --- a/sound/soc/fsl/fsl_ssi.c
> +++ b/sound/soc/fsl/fsl_ssi.c
> @@ -445,16 +445,10 @@ static void fsl_ssi_config(struct fsl_ssi *ssi, bool enable,
> bool dir = (&ssi->regvals[TX] == vals) ? TX : RX;
> struct regmap *regs = ssi->regs;
> struct fsl_ssi_regvals *avals;
> - int nr_active_streams;
> - int keep_active;
> -
> - nr_active_streams = !!(ssi->streams & BIT(TX)) +
> - !!(ssi->streams & BIT(RX));
> + bool aactive;
>
> - if (nr_active_streams - 1 > 0)
> - keep_active = 1;
> - else
> - keep_active = 0;
> + /* Check if the opposite stream is active */
> + aactive = ssi->streams & BIT(!dir);
^
Here an implicit assumption that either RX == 0, TX == 1 or
RX == 1, TX == 0 still remains.
Maciej
^ permalink raw reply
* Re: [PATCH v2 03/16] ASoC: fsl_ssi: Maintain a mask of active streams
From: Maciej S. Szmigiero @ 2018-01-14 22:34 UTC (permalink / raw)
To: Nicolin Chen
Cc: timur, broonie, linux-kernel, linuxppc-dev, alsa-devel, lgirdwood,
fabio.estevam, caleb, arnaud.mouiche, lukma, kernel
In-Reply-To: <1515652995-15996-4-git-send-email-nicoleotsuka@gmail.com>
On 11.01.2018 07:43, Nicolin Chen wrote:
> Checking TE and RE bits in SCR register doesn't work for AC97 mode
> which enables SSIEN, TE and RE in the fsl_ssi_setup_ac97() that's
> called during probe().
>
> So when running into the trigger(), it will always get the result
> of both TE and RE being enabled already, even if actually there is
> no active stream.
>
> This patch fixes this issue by adding a variable to log the active
> streams manually.
>
> Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com>
> Tested-by: Caleb Crome <caleb@crome.org>
> ---
> sound/soc/fsl/fsl_ssi.c | 15 +++++++++++----
> 1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
> index 491b660..aa14a5d 100644
> --- a/sound/soc/fsl/fsl_ssi.c
> +++ b/sound/soc/fsl/fsl_ssi.c
> @@ -201,6 +201,7 @@ struct fsl_ssi_soc_data {
> * @cpu_dai_drv: CPU DAI driver for this device
> *
> * @dai_fmt: DAI configuration this device is currently used with
> + * @streams: Mask of current active streams: BIT(TX) and BIT(RX)
> * @i2s_net: I2S and Network mode configurations of SCR register
> * @use_dma: DMA is used or FIQ with stream filter
> * @use_dual_fifo: DMA with support for dual FIFO mode
> @@ -245,6 +246,7 @@ struct fsl_ssi {
> struct snd_soc_dai_driver cpu_dai_drv;
>
> unsigned int dai_fmt;
> + u8 streams;
> u8 i2s_net;
> bool use_dma;
> bool use_dual_fifo;
> @@ -440,15 +442,14 @@ static void fsl_ssi_fifo_clear(struct fsl_ssi *ssi, bool is_rx)
> static void fsl_ssi_config(struct fsl_ssi *ssi, bool enable,
> struct fsl_ssi_regvals *vals)
> {
> + bool dir = (&ssi->regvals[TX] == vals) ? TX : RX;
Using a bool variable for a bit index (and array index in other parts
of code) looks just wrong.
Even a simple int would look better IMHO here (and in patch 5 that
rewrites this line a bit).
Maciej
^ permalink raw reply
* [GIT PULL] Please pull powerpc/linux.git powerpc-4.15-7 tag
From: Michael Ellerman @ 2018-01-14 22:27 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-kernel, linuxppc-dev, mikey, npiggin, oohall
[-- Attachment #1: Type: text/plain, Size: 3129 bytes --]
Hi Linus,
Please pull some more powerpc fixes for 4.15, sorry these are so late.
The following changes since commit ecb101aed86156ec7cd71e5dca668e09146e6994:
powerpc/mm: Fix SEGV on mapped region to return SEGV_ACCERR (2018-01-02 21:12:33 +1100)
are available in the Git repository at:
https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git tags/powerpc-4.15-7
for you to fetch changes up to 6e032b350cd1fdb830f18f8320ef0e13b4e24094:
powerpc/powernv: Check device-tree for RFI flush settings (2018-01-10 21:27:16 +1100)
----------------------------------------------------------------
powerpc fixes for 4.15 #7
One fix for an oops at boot if we take a hotplug interrupt before we are ready
to handle it.
The bulk is patches to implement mitigation for Meltdown, see the change logs
for more details.
Thanks to:
Nicholas Piggin, Michael Neuling, Oliver O'Halloran, Jon Masters, Jose Ricardo
Ziviani, David Gibson.
----------------------------------------------------------------
Michael Ellerman (4):
powerpc/pseries: Make RAS IRQ explicitly dependent on DLPAR WQ
Merge branch 'topic/ppc-kvm' into fixes
powerpc/64s: Add support for RFI flush of L1-D cache
powerpc/64s: Support disabling RFI flush with no_rfi_flush and nopti
Michael Neuling (2):
powerpc/pseries: Add H_GET_CPU_CHARACTERISTICS flags & wrapper
powerpc/pseries: Query hypervisor for RFI flush settings
Nicholas Piggin (5):
powerpc/64: Add macros for annotating the destination of rfid/hrfid
powerpc/64s: Simple RFI macro conversions
powerpc/64: Convert the syscall exit path to use RFI_TO_USER/KERNEL
powerpc/64: Convert fast_exception_return to use RFI_TO_USER/KERNEL
powerpc/64s: Convert slb_miss_common to use RFI_TO_USER/KERNEL
Oliver O'Halloran (1):
powerpc/powernv: Check device-tree for RFI flush settings
arch/powerpc/include/asm/exception-64e.h | 6 ++
arch/powerpc/include/asm/exception-64s.h | 57 ++++++++++++-
arch/powerpc/include/asm/feature-fixups.h | 13 +++
arch/powerpc/include/asm/hvcall.h | 17 ++++
arch/powerpc/include/asm/paca.h | 10 +++
arch/powerpc/include/asm/plpar_wrappers.h | 14 +++
arch/powerpc/include/asm/setup.h | 13 +++
arch/powerpc/kernel/asm-offsets.c | 5 ++
arch/powerpc/kernel/entry_64.S | 44 ++++++++--
arch/powerpc/kernel/exceptions-64s.S | 137 +++++++++++++++++++++++++++---
arch/powerpc/kernel/setup_64.c | 101 ++++++++++++++++++++++
arch/powerpc/kernel/vmlinux.lds.S | 9 ++
arch/powerpc/kvm/book3s_hv_rmhandlers.S | 9 +-
arch/powerpc/kvm/book3s_rmhandlers.S | 7 +-
arch/powerpc/kvm/book3s_segment.S | 4 +-
arch/powerpc/lib/feature-fixups.c | 41 +++++++++
arch/powerpc/platforms/powernv/setup.c | 49 +++++++++++
arch/powerpc/platforms/pseries/dlpar.c | 21 ++++-
arch/powerpc/platforms/pseries/pseries.h | 2 +
arch/powerpc/platforms/pseries/ras.c | 3 +-
arch/powerpc/platforms/pseries/setup.c | 35 ++++++++
21 files changed, 561 insertions(+), 36 deletions(-)
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 800 bytes --]
^ permalink raw reply
* Re: [cryptodev:master 130/134] aes_generic.c:undefined reference to `_restgpr_31_x'
From: Arnd Bergmann @ 2018-01-14 21:40 UTC (permalink / raw)
To: Segher Boessenkool
Cc: open list:HARDWARE RANDOM NUMBER GENERATOR CORE,
kbuild test robot, linuxppc-dev, Herbert Xu
In-Reply-To: <20180112221050.GR21977@gate.crashing.org>
On Fri, Jan 12, 2018 at 11:10 PM, Segher Boessenkool
<segher@kernel.crashing.org> wrote:
> On Fri, Jan 12, 2018 at 10:45:31PM +0100, Arnd Bergmann wrote:
>> > I guess you could enable the _x routines whenever you use ubsan? Ubsan
>> > will cause much bigger code growth than the handful of insns in those
>> > routines?
>>
>> Right, that could work, too. My patch that Herbert merged intentionally
>> used -Os also for non-UBSAN builds because it turned out to
>> be much faster (see gcc PR83651),
>
> "Much"?
>
> -Os is *slower* with 8.0, 5% faster with 7.2, 4% faster with 7.1,
> slower with 7.0 and 6.3. Your numbers, #c1.
>
> Anf this is the generic code of course, which is slow anyway (not to
> mention insecure).
Right. I've done some more investigation anyway, starting over with the
analysis of the gcc options that change it. I've found now that turning
off '-fcode-hoisting' but leaving on the other options I had suspected
earlier (-O2 instead of -Os, -ftree-sra, -ftree-pre) also fixes the
stack problem, and appears to result in the best performance so
far. I need to rerun the whole test matrix, but that seems rather
promising, and the result may also help debug what's really happening.
Arnd
^ permalink raw reply
* Re: [PATCH v6 22/24] mm: Speculative page fault handler return VMA
From: Matthew Wilcox @ 2018-01-13 4:23 UTC (permalink / raw)
To: Laurent Dufour
Cc: paulmck, peterz, akpm, kirill, ak, mhocko, dave, jack, benh, mpe,
paulus, Thomas Gleixner, Ingo Molnar, hpa, Will Deacon,
Sergey Senozhatsky, Andrea Arcangeli, Alexei Starovoitov,
kemi.wang, sergey.senozhatsky.work, linux-kernel, linux-mm, haren,
khandual, npiggin, bsingharora, Tim Chen, linuxppc-dev, x86
In-Reply-To: <20180112190251.GC7590@bombadil.infradead.org>
On Fri, Jan 12, 2018 at 11:02:51AM -0800, Matthew Wilcox wrote:
> On Fri, Jan 12, 2018 at 06:26:06PM +0100, Laurent Dufour wrote:
> > @@ -1354,7 +1354,10 @@ extern int handle_mm_fault(struct vm_area_struct *vma, unsigned long address,
> > unsigned int flags);
> > #ifdef CONFIG_SPF
> > extern int handle_speculative_fault(struct mm_struct *mm,
> > + unsigned long address, unsigned int flags,
> > + struct vm_area_struct **vma);
>
> I think this shows that we need to create 'struct vm_fault' on the stack
> in the arch code and then pass it to handle_speculative_fault(), followed
> by handle_mm_fault(). That should be quite a nice cleanup actually.
> I know that's only 30+ architectures to change ;-)
Of course, we don't need to change them all. Try this:
Subject: [PATCH] Add vm_handle_fault
For the speculative fault handler, we want to create the struct vm_fault
on the stack in the arch code and pass it into the generic mm code.
To avoid changing 30+ architectures, leave handle_mm_fault with its
current function signature and move its guts into the new vm_handle_fault
function. Even this saves a nice 172 bytes on the random x86-64 .config
I happen to have around.
Signed-off-by: Matthew Wilcox <mawilcox@microsoft.com>
diff --git a/mm/memory.c b/mm/memory.c
index 5eb3d2524bdc..403934297a3d 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3977,36 +3977,28 @@ static int handle_pte_fault(struct vm_fault *vmf)
* The mmap_sem may have been released depending on flags and our
* return value. See filemap_fault() and __lock_page_or_retry().
*/
-static int __handle_mm_fault(struct vm_area_struct *vma, unsigned long address,
- unsigned int flags)
+static int __handle_mm_fault(struct vm_fault *vmf)
{
- struct vm_fault vmf = {
- .vma = vma,
- .address = address & PAGE_MASK,
- .flags = flags,
- .pgoff = linear_page_index(vma, address),
- .gfp_mask = __get_fault_gfp_mask(vma),
- };
- unsigned int dirty = flags & FAULT_FLAG_WRITE;
- struct mm_struct *mm = vma->vm_mm;
+ unsigned int dirty = vmf->flags & FAULT_FLAG_WRITE;
+ struct mm_struct *mm = vmf->vma->vm_mm;
pgd_t *pgd;
p4d_t *p4d;
int ret;
- pgd = pgd_offset(mm, address);
- p4d = p4d_alloc(mm, pgd, address);
+ pgd = pgd_offset(mm, vmf->address);
+ p4d = p4d_alloc(mm, pgd, vmf->address);
if (!p4d)
return VM_FAULT_OOM;
- vmf.pud = pud_alloc(mm, p4d, address);
- if (!vmf.pud)
+ vmf->pud = pud_alloc(mm, p4d, vmf->address);
+ if (!vmf->pud)
return VM_FAULT_OOM;
- if (pud_none(*vmf.pud) && transparent_hugepage_enabled(vma)) {
- ret = create_huge_pud(&vmf);
+ if (pud_none(*vmf->pud) && transparent_hugepage_enabled(vmf->vma)) {
+ ret = create_huge_pud(vmf);
if (!(ret & VM_FAULT_FALLBACK))
return ret;
} else {
- pud_t orig_pud = *vmf.pud;
+ pud_t orig_pud = *vmf->pud;
barrier();
if (pud_trans_huge(orig_pud) || pud_devmap(orig_pud)) {
@@ -4014,50 +4006,51 @@ static int __handle_mm_fault(struct vm_area_struct *vma, unsigned long address,
/* NUMA case for anonymous PUDs would go here */
if (dirty && !pud_access_permitted(orig_pud, WRITE)) {
- ret = wp_huge_pud(&vmf, orig_pud);
+ ret = wp_huge_pud(vmf, orig_pud);
if (!(ret & VM_FAULT_FALLBACK))
return ret;
} else {
- huge_pud_set_accessed(&vmf, orig_pud);
+ huge_pud_set_accessed(vmf, orig_pud);
return 0;
}
}
}
- vmf.pmd = pmd_alloc(mm, vmf.pud, address);
- if (!vmf.pmd)
+ vmf->pmd = pmd_alloc(mm, vmf->pud, vmf->address);
+ if (!vmf->pmd)
return VM_FAULT_OOM;
- if (pmd_none(*vmf.pmd) && transparent_hugepage_enabled(vma)) {
- ret = create_huge_pmd(&vmf);
+ if (pmd_none(*vmf->pmd) && transparent_hugepage_enabled(vmf->vma)) {
+ ret = create_huge_pmd(vmf);
if (!(ret & VM_FAULT_FALLBACK))
return ret;
} else {
- pmd_t orig_pmd = *vmf.pmd;
+ pmd_t orig_pmd = *vmf->pmd;
barrier();
if (unlikely(is_swap_pmd(orig_pmd))) {
VM_BUG_ON(thp_migration_supported() &&
!is_pmd_migration_entry(orig_pmd));
if (is_pmd_migration_entry(orig_pmd))
- pmd_migration_entry_wait(mm, vmf.pmd);
+ pmd_migration_entry_wait(mm, vmf->pmd);
return 0;
}
if (pmd_trans_huge(orig_pmd) || pmd_devmap(orig_pmd)) {
- if (pmd_protnone(orig_pmd) && vma_is_accessible(vma))
- return do_huge_pmd_numa_page(&vmf, orig_pmd);
+ if (pmd_protnone(orig_pmd) &&
+ vma_is_accessible(vmf->vma))
+ return do_huge_pmd_numa_page(vmf, orig_pmd);
if (dirty && !pmd_access_permitted(orig_pmd, WRITE)) {
- ret = wp_huge_pmd(&vmf, orig_pmd);
+ ret = wp_huge_pmd(vmf, orig_pmd);
if (!(ret & VM_FAULT_FALLBACK))
return ret;
} else {
- huge_pmd_set_accessed(&vmf, orig_pmd);
+ huge_pmd_set_accessed(vmf, orig_pmd);
return 0;
}
}
}
- return handle_pte_fault(&vmf);
+ return handle_pte_fault(vmf);
}
/*
@@ -4066,9 +4059,10 @@ static int __handle_mm_fault(struct vm_area_struct *vma, unsigned long address,
* The mmap_sem may have been released depending on flags and our
* return value. See filemap_fault() and __lock_page_or_retry().
*/
-int handle_mm_fault(struct vm_area_struct *vma, unsigned long address,
- unsigned int flags)
+int vm_handle_fault(struct vm_fault *vmf)
{
+ unsigned int flags = vmf->flags;
+ struct vm_area_struct *vma = vmf->vma;
int ret;
__set_current_state(TASK_RUNNING);
@@ -4092,9 +4086,9 @@ int handle_mm_fault(struct vm_area_struct *vma, unsigned long address,
mem_cgroup_oom_enable();
if (unlikely(is_vm_hugetlb_page(vma)))
- ret = hugetlb_fault(vma->vm_mm, vma, address, flags);
+ ret = hugetlb_fault(vma->vm_mm, vma, vmf->address, flags);
else
- ret = __handle_mm_fault(vma, address, flags);
+ ret = __handle_mm_fault(vmf);
if (flags & FAULT_FLAG_USER) {
mem_cgroup_oom_disable();
@@ -4110,6 +4104,26 @@ int handle_mm_fault(struct vm_area_struct *vma, unsigned long address,
return ret;
}
+
+/*
+ * By the time we get here, we already hold the mm semaphore
+ *
+ * The mmap_sem may have been released depending on flags and our
+ * return value. See filemap_fault() and __lock_page_or_retry().
+ */
+int handle_mm_fault(struct vm_area_struct *vma, unsigned long address,
+ unsigned int flags)
+{
+ struct vm_fault vmf = {
+ .vma = vma,
+ .address = address & PAGE_MASK,
+ .flags = flags,
+ .pgoff = linear_page_index(vma, address),
+ .gfp_mask = __get_fault_gfp_mask(vma),
+ };
+
+ return vm_handle_fault(&vmf);
+}
EXPORT_SYMBOL_GPL(handle_mm_fault);
#ifndef __PAGETABLE_P4D_FOLDED
^ permalink raw reply related
* Re: [PATCH V2] powerpc/kernel: Add 'ibm, thread-groups' property for CPU allocation
From: Michael Ellerman @ 2018-01-13 2:33 UTC (permalink / raw)
To: Nathan Fontenot, Michael Bringmann, linuxppc-dev
In-Reply-To: <d7825e1c-ca1f-3bbe-be41-897b3e98e928@linux.vnet.ibm.com>
Nathan Fontenot <nfont@linux.vnet.ibm.com> writes:
> On 01/08/2018 11:19 AM, Michael Bringmann wrote:
>> Add code to parse the new property 'ibm,thread-groups" when it is
>> present. The content of this property explicitly defines the number
>> of threads per core as well as the PowerPC 'threads_core_mask'.
>> The design provides a common device-tree for both P9 normal core and
>> P9 fused core systems. The new property has been observed to be
>> available on P9 pHyp systems, but it is not always present on
>> OpenPower BMC systems.
>>
>> The property updates the kernel to know which CPUs/threads of each
>> core are actually present, and then use the map when adding cores
>> to the system at boot, or during hotplug operations.
>>
>> * Previously, the information about the number of threads per core
>> was inferred solely from the "ibm,ppc-interrupt-server#s" property
>> in the system device tree.
>> * Also previous to this property, The mask of threads per CPU was
>> inferred to be a strict linear series from 0..(nthreads-1).
>> * After reading the "ibm,thread-group" property, we can determine
>> the number of threads per core to be the 'bitmask weight' of the
>> CPU thread mask.
>> * Also after reading the property, we can determine which of the
>> possible threads we are allowed to online for each CPU. It is no
>> longer a simple linear sequence, but may be discontinuous e.g.
>> activate threads 1,2,3,5,6,7 on a core instead of 0-5 sequentially.
>>
>> Implementation of the "ibm,thread-groups" property is spread across
>> a few files in the powerpc specific code:
>>
>> * prom.c: Parse the property and create 'ppc_thread_group_mask'.
>> Use the mask in operation of early_init_dt_scan_cpus().
>> * setup-common.c: Import 'ppc_thread_group_mask' and use the value
>> in the operation of cpu_init_thread_core_maps(), and
>> smp_setup_cpu_maps.
>> * hotplug-cpu.c: Use 'ppc_thread_group_mask' in several locations
>> where the code previously expected to iterate over a
>> linear series of active threads (0..nthreads-1).
>>
>> Note that the "ibm,thread-groups" property also includes semantics
>> of 'thread-group' i.e. define one or more subgroups of the available
>> threads, each group of threads to be used for a specific class of
>> task. Translating thread group semantics into Linux kernel features
>> is TBD.
>
> One thing I don't see addressed in the comments or in the code is
> migration support. I think we need to update the thread group mask
> post-migration to reflect the threads per core on the new system.
Normally I'd agree with you, but I don't see any prospect of the kernel
surviving if the threads per core changes across a migration. We'll have
data structures allocated based on the old value and things will
definitely crash if the value increases. If it shrinks maybe we'd get
away with it, but either way is dicey.
If there's an expectation that we'll be able to migrate between systems
with different settings then we have a much bigger problem.
cheers
^ permalink raw reply
* Re: [cryptodev:master 130/134] aes_generic.c:undefined reference to `_restgpr_31_x'
From: Segher Boessenkool @ 2018-01-12 22:10 UTC (permalink / raw)
To: Arnd Bergmann
Cc: open list:HARDWARE RANDOM NUMBER GENERATOR CORE,
kbuild test robot, linuxppc-dev, Herbert Xu
In-Reply-To: <CAK8P3a3XKwrVpXH9=8n1=qAKtqHE+1sh6OhaJ_39GZcWSP-j=Q@mail.gmail.com>
On Fri, Jan 12, 2018 at 10:45:31PM +0100, Arnd Bergmann wrote:
> > I guess you could enable the _x routines whenever you use ubsan? Ubsan
> > will cause much bigger code growth than the handful of insns in those
> > routines?
>
> Right, that could work, too. My patch that Herbert merged intentionally
> used -Os also for non-UBSAN builds because it turned out to
> be much faster (see gcc PR83651),
"Much"?
-Os is *slower* with 8.0, 5% faster with 7.2, 4% faster with 7.1,
slower with 7.0 and 6.3. Your numbers, #c1.
Anf this is the generic code of course, which is slow anyway (not to
mention insecure).
> but we could revert that back
> to the default and only use the -Os for UBSAN, essentially
> addressing only PR83356 but not PR83651.
Segher
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox