* Re: [PATCH v3 4/9] ARM: EXYNOS: enable exynos_chipid for ARCH_EXYNOS
From: Krzysztof Kozlowski @ 2019-08-15 18:29 UTC (permalink / raw)
To: Sylwester Nawrocki
Cc: devicetree, linux-samsung-soc, linux-pm, pankaj.dubey,
b.zolnierkie, linux-kernel, robh+dt, kgene, vireshk,
linux-arm-kernel, m.szyprowski
In-Reply-To: <20190813150827.31972-5-s.nawrocki@samsung.com>
On Tue, Aug 13, 2019 at 05:08:22PM +0200, Sylwester Nawrocki wrote:
> From: Pankaj Dubey <pankaj.dubey@samsung.com>
>
> As now we have chipid driver to initialize SoC related information
> let's include it in build by default.
>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> ---
> Changes since v1 (RFC):
Thanks, applied.
Best regards,
Krzysztof
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 0/6] drm+dma: cache support for arm, etc
From: Christoph Hellwig @ 2019-08-15 18:27 UTC (permalink / raw)
To: Koenig, Christian
Cc: Kate Stewart, Masayoshi Mizuma, Maciej W. Rozycki, Eric Biggers,
Catalin Marinas, Imre Deak, dri-devel, Chris Wilson,
Masahiro Yamada, Benjamin Gaignard, Mauro Carvalho Chehab,
Will Deacon, Christoph Hellwig, Emil Velikov, Rob Clark,
Michael Ellerman, Paul Burton, Mike Rapoport, Geert Uytterhoeven,
moderated list:ARM64 PORT (AARCH64 ARCHITECTURE), Daniel Vetter,
open list:MIPS, Linus Walleij, Robin Murphy,
open list:DRM DRIVER FOR MSM ADRENO GPU, Sharma, Deepak,
Joerg Roedel, Arnd Bergmann, Anshuman Khandual, Hauke Mehrtens,
Jesper Dangaard Brouer, Wolfram Sang (Renesas),
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT), Alexios Zavras,
Russell King, Doug Anderson, Thomas Gleixner, Sean Paul,
Allison Randal, Christophe Leroy, Ard Biesheuvel, Enrico Weigelt,
open list, Rob Clark, Souptick Joarder, Greg Kroah-Hartman,
Andrew Morton, open list:DRM DRIVER FOR MSM ADRENO GPU
In-Reply-To: <e3f73b3c-49d5-3c19-dfff-0a24b4617e50@amd.com>
On Thu, Aug 15, 2019 at 06:21:00PM +0000, Koenig, Christian wrote:
> > (2) Add support for DMA_ATTR_NO_KERNEL_MAPPING to this new API instead
> > of dma_alloc_attrs. The initial difference with that flag is just
> > that we allow highmem, but in the future we could also unmap this
> > memory from the kernel linear mapping entirely on architectures
> > where we can easily do that.
>
> Mhm, why would we want to do this?
To avoid the CPU misspeculating into this memory. For example NVMe SSDs
have a feature called host memory buffer that is a lot like your stolen
main ram for the GPU case. We currently hand the SSD a
DMA_ATTR_NO_KERNEL_MAPPING allocation if it requests such a buffer. If
possible we'd really like to make sure no speculative execution bug
(or intentional attacker with a kernel exploit for that matter) can easily
access that memory.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v8 04/14] media: rkisp1: add Rockchip MIPI Synopsys DPHY driver
From: Heiko Stübner @ 2019-08-15 18:26 UTC (permalink / raw)
To: Laurent Pinchart
Cc: devicetree, eddie.cai.linux, kernel, jacob2.chen, jeffy.chen, zyc,
linux-kernel, tfiga, linux-rockchip, Helen Koike, Sakari Ailus,
sakari.ailus, zhengsq, hans.verkuil, mchehab, ezequiel,
linux-arm-kernel, linux-media
In-Reply-To: <20190815175440.GW5011@pendragon.ideasonboard.com>
Hi Laurent,
Am Donnerstag, 15. August 2019, 19:54:40 CEST schrieb Laurent Pinchart:
> On Wed, Aug 07, 2019 at 10:37:55AM -0300, Helen Koike wrote:
> > On 8/7/19 10:05 AM, Sakari Ailus wrote:
> > > On Tue, Jul 30, 2019 at 03:42:46PM -0300, Helen Koike wrote:
> > >> From: Jacob Chen <jacob2.chen@rock-chips.com>
> > >>
> > >> This commit adds a subdev driver for Rockchip MIPI Synopsys DPHY driver
> > >>
> > >> Signed-off-by: Jacob Chen <jacob2.chen@rock-chips.com>
> > >> Signed-off-by: Shunqian Zheng <zhengsq@rock-chips.com>
> > >> Signed-off-by: Tomasz Figa <tfiga@chromium.org>
> > >> [migrate to phy framework]
> > >> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> > >> [update for upstream]
> > >> Signed-off-by: Helen Koike <helen.koike@collabora.com>
> > >>
> > >> ---
> > >>
> > >> Changes in v8:
> > >> - Remove boiler plate license text
> > >>
> > >> Changes in v7:
> > >> - Migrate dphy specific code from
> > >> drivers/media/platform/rockchip/isp1/mipi_dphy_sy.c
> > >> to drivers/phy/rockchip/phy-rockchip-dphy.c
> > >> - Drop support for rk3288
> > >> - Drop support for dphy txrx
> > >> - code styling and checkpatch fixes
> > >>
> > >> drivers/phy/rockchip/Kconfig | 8 +
> > >> drivers/phy/rockchip/Makefile | 1 +
> > >> drivers/phy/rockchip/phy-rockchip-dphy.c | 408 +++++++++++++++++++++++
> > >> 3 files changed, 417 insertions(+)
> > >> create mode 100644 drivers/phy/rockchip/phy-rockchip-dphy.c
> > >>
> > >> diff --git a/drivers/phy/rockchip/Kconfig b/drivers/phy/rockchip/Kconfig
> > >> index c454c90cd99e..afd072f135e6 100644
> > >> --- a/drivers/phy/rockchip/Kconfig
> > >> +++ b/drivers/phy/rockchip/Kconfig
> > >> @@ -9,6 +9,14 @@ config PHY_ROCKCHIP_DP
> > >> help
> > >> Enable this to support the Rockchip Display Port PHY.
> > >>
> > >> +config PHY_ROCKCHIP_DPHY
> > >> + tristate "Rockchip MIPI Synopsys DPHY driver"
>
> How much of this PHY is Rockchip-specific ? Would it make sense to turn
> it into a Synopsys DPHY driver, with some Rockchip glue ? I suppose this
> could always be done later, if needed (and I also suppose there's no
> existing driver in drivers/phy/ that support the same Synopsys IP).
I'm not so versed in the video-stuff itself but I should be able to shed
some light on where the this thingy sits on the soc :-) .
SoCs like the rk3399 have 3 mipi-dphy instances:
- TX0 - output-only for dsi0
with its access living _inside_ the mipi-dsi0 controller
the registers to control it are part of the dsi-registers]
- TX1RX1 - output for dsi1 / input for isp1 (shared between them)
its access lives inside the dsi1 controller's registers as well
[not part of Helen's initial series, but I do have a wip-patch
that exposes a phy-device from the dsi-driver for a later addon]
- RX1 - input for isp0 - this driver
it's access is exposed through the GRF (General Register Files) of the soc
The GRF is "chip designer's playground", registers change between each
soc ... and the "randomness" can be seen by the RK3399_GRF_SOC_CONx
register-naming, which is what they're called in all docs as well.
So I do think this is highly Rockchip-specific and it doesn't really make
a lot of sense trying to squeeze commonality out of it _before_ a second
actual instance of this rather strange system appears from some other
vendor :-) ... and even then I'd think keeping them separate would
actually make things a lot easier long term.
Heiko
>
> > >> + depends on ARCH_ROCKCHIP && OF
> > >
> > > How about (...) || COMPILE_TEST ?
> > >
> > >> + select GENERIC_PHY_MIPI_DPHY
> > >> + select GENERIC_PHY
> > >> + help
> > >> + Enable this to support the Rockchip MIPI Synopsys DPHY.
> > >> +
> > >> config PHY_ROCKCHIP_EMMC
> > >> tristate "Rockchip EMMC PHY Driver"
> > >> depends on ARCH_ROCKCHIP && OF
> > >> diff --git a/drivers/phy/rockchip/Makefile b/drivers/phy/rockchip/Makefile
> > >> index fd21cbaf40dd..f62e9010bcaf 100644
> > >> --- a/drivers/phy/rockchip/Makefile
> > >> +++ b/drivers/phy/rockchip/Makefile
> > >> @@ -1,5 +1,6 @@
> > >> # SPDX-License-Identifier: GPL-2.0
> > >> obj-$(CONFIG_PHY_ROCKCHIP_DP) += phy-rockchip-dp.o
> > >> +obj-$(CONFIG_PHY_ROCKCHIP_DPHY) += phy-rockchip-dphy.o
> > >> obj-$(CONFIG_PHY_ROCKCHIP_EMMC) += phy-rockchip-emmc.o
> > >> obj-$(CONFIG_PHY_ROCKCHIP_INNO_HDMI) += phy-rockchip-inno-hdmi.o
> > >> obj-$(CONFIG_PHY_ROCKCHIP_INNO_USB2) += phy-rockchip-inno-usb2.o
> > >> diff --git a/drivers/phy/rockchip/phy-rockchip-dphy.c b/drivers/phy/rockchip/phy-rockchip-dphy.c
> > >> new file mode 100644
> > >> index 000000000000..3a29976c2dff
> > >> --- /dev/null
> > >> +++ b/drivers/phy/rockchip/phy-rockchip-dphy.c
> > >> @@ -0,0 +1,408 @@
> > >> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > >> +/*
> > >> + * Rockchip MIPI Synopsys DPHY driver
> > >> + *
> > >> + * Based on:
> > >> + *
> > >> + * Copyright (C) 2016 FuZhou Rockchip Co., Ltd.
> > >> + * Author: Yakir Yang <ykk@@rock-chips.com>
> > >> + */
> > >> +
> > >> +#include <linux/clk.h>
> > >> +#include <linux/io.h>
> > >> +#include <linux/mfd/syscon.h>
> > >> +#include <linux/module.h>
> > >> +#include <linux/of.h>
> > >> +#include <linux/of_device.h>
> > >> +#include <linux/phy/phy.h>
> > >> +#include <linux/phy/phy-mipi-dphy.h>
> > >> +#include <linux/platform_device.h>
> > >> +#include <linux/regmap.h>
> > >> +
> > >> +#define RK3399_GRF_SOC_CON9 0x6224
> > >> +#define RK3399_GRF_SOC_CON21 0x6254
> > >> +#define RK3399_GRF_SOC_CON22 0x6258
> > >> +#define RK3399_GRF_SOC_CON23 0x625c
> > >> +#define RK3399_GRF_SOC_CON24 0x6260
> > >> +#define RK3399_GRF_SOC_CON25 0x6264
> > >> +#define RK3399_GRF_SOC_STATUS1 0xe2a4
> > >> +
> > >> +#define CLOCK_LANE_HS_RX_CONTROL 0x34
> > >> +#define LANE0_HS_RX_CONTROL 0x44
> > >> +#define LANE1_HS_RX_CONTROL 0x54
> > >> +#define LANE2_HS_RX_CONTROL 0x84
> > >> +#define LANE3_HS_RX_CONTROL 0x94
> > >> +#define HS_RX_DATA_LANES_THS_SETTLE_CONTROL 0x75
> > >> +
> > >> +#define MAX_DPHY_CLK 8
> > >> +
> > >> +#define PHY_TESTEN_ADDR (0x1 << 16)
> > >> +#define PHY_TESTEN_DATA (0x0 << 16)
> > >> +#define PHY_TESTCLK (0x1 << 1)
> > >> +#define PHY_TESTCLR (0x1 << 0)
>
> Maybe s/0x// for the previous four lines ?
>
> > >> +#define THS_SETTLE_COUNTER_THRESHOLD 0x04
> > >> +
> > >> +#define HIWORD_UPDATE(val, mask, shift) \
> > >> + ((val) << (shift) | (mask) << ((shift) + 16))
>
> As you use this in a single place, I would inline it, possibly with a
> small comment that explains what's happening.
>
> > >> +
> > >> +#define GRF_SOC_CON12 0x0274
> > >> +
> > >> +#define GRF_EDP_REF_CLK_SEL_INTER_HIWORD_MASK BIT(20)
> > >> +#define GRF_EDP_REF_CLK_SEL_INTER BIT(4)
> > >> +
> > >> +#define GRF_EDP_PHY_SIDDQ_HIWORD_MASK BIT(21)
> > >> +#define GRF_EDP_PHY_SIDDQ_ON 0
> > >> +#define GRF_EDP_PHY_SIDDQ_OFF BIT(5)
>
> I would recommend aligning the value of of all macros in the same way.
>
> > >> +
> > >> +struct hsfreq_range {
> > >> + u32 range_h;
>
> The structure would be more compact if you turned this into a u16.
>
> > >> + u8 cfg_bit;
> > >> +};
> > >> +
> > >> +static const struct hsfreq_range rk3399_mipidphy_hsfreq_ranges[] = {
> > >> + { 89, 0x00}, { 99, 0x10}, { 109, 0x20}, { 129, 0x01},
> > >> + { 139, 0x11}, { 149, 0x21}, { 169, 0x02}, { 179, 0x12},
> > >> + { 199, 0x22}, { 219, 0x03}, { 239, 0x13}, { 249, 0x23},
> > >> + { 269, 0x04}, { 299, 0x14}, { 329, 0x05}, { 359, 0x15},
> > >> + { 399, 0x25}, { 449, 0x06}, { 499, 0x16}, { 549, 0x07},
> > >> + { 599, 0x17}, { 649, 0x08}, { 699, 0x18}, { 749, 0x09},
> > >> + { 799, 0x19}, { 849, 0x29}, { 899, 0x39}, { 949, 0x0a},
> > >> + { 999, 0x1a}, {1049, 0x2a}, {1099, 0x3a}, {1149, 0x0b},
> > >> + {1199, 0x1b}, {1249, 0x2b}, {1299, 0x3b}, {1349, 0x0c},
> > >> + {1399, 0x1c}, {1449, 0x2c}, {1500, 0x3c}
>
> Maybe s/{/{ / and s/}/ }/ to give it a bit more air ? :-)
>
> > >> +};
> > >> +
> > >> +static const char * const rk3399_mipidphy_clks[] = {
> > >> + "dphy-ref",
> > >> + "dphy-cfg",
> > >> + "grf",
> > >> +};
> > >> +
> > >> +enum dphy_reg_id {
> > >> + GRF_DPHY_RX0_TURNDISABLE = 0,
> > >> + GRF_DPHY_RX0_FORCERXMODE,
> > >> + GRF_DPHY_RX0_FORCETXSTOPMODE,
> > >> + GRF_DPHY_RX0_ENABLE,
> > >> + GRF_DPHY_RX0_TESTCLR,
> > >> + GRF_DPHY_RX0_TESTCLK,
> > >> + GRF_DPHY_RX0_TESTEN,
> > >> + GRF_DPHY_RX0_TESTDIN,
> > >> + GRF_DPHY_RX0_TURNREQUEST,
> > >> + GRF_DPHY_RX0_TESTDOUT,
> > >> + GRF_DPHY_TX0_TURNDISABLE,
> > >> + GRF_DPHY_TX0_FORCERXMODE,
> > >> + GRF_DPHY_TX0_FORCETXSTOPMODE,
> > >> + GRF_DPHY_TX0_TURNREQUEST,
> > >> + GRF_DPHY_TX1RX1_TURNDISABLE,
> > >> + GRF_DPHY_TX1RX1_FORCERXMODE,
> > >> + GRF_DPHY_TX1RX1_FORCETXSTOPMODE,
> > >> + GRF_DPHY_TX1RX1_ENABLE,
> > >> + GRF_DPHY_TX1RX1_MASTERSLAVEZ,
> > >> + GRF_DPHY_TX1RX1_BASEDIR,
> > >> + GRF_DPHY_TX1RX1_ENABLECLK,
> > >> + GRF_DPHY_TX1RX1_TURNREQUEST,
> > >> + GRF_DPHY_RX1_SRC_SEL,
> > >> + /* rk3288 only */
> > >> + GRF_CON_DISABLE_ISP,
> > >> + GRF_CON_ISP_DPHY_SEL,
> > >> + GRF_DSI_CSI_TESTBUS_SEL,
> > >> + GRF_DVP_V18SEL,
> > >> + /* below is for rk3399 only */
> > >> + GRF_DPHY_RX0_CLK_INV_SEL,
> > >> + GRF_DPHY_RX1_CLK_INV_SEL,
> > >> +};
> > >> +
> > >> +struct dphy_reg {
> > >> + u32 offset;
> > >> + u32 mask;
> > >> + u32 shift;
>
> The offset should hold in 16 bits and the mask and shift in 8 bits. That
> would save space in the table below.
>
> > >> +};
> > >> +
> > >> +#define PHY_REG(_offset, _width, _shift) \
> > >> + { .offset = _offset, .mask = BIT(_width) - 1, .shift = _shift, }
> > >> +
> > >> +static const struct dphy_reg rk3399_grf_dphy_regs[] = {
> > >> + [GRF_DPHY_RX0_TURNREQUEST] = PHY_REG(RK3399_GRF_SOC_CON9, 4, 0),
> > >> + [GRF_DPHY_RX0_CLK_INV_SEL] = PHY_REG(RK3399_GRF_SOC_CON9, 1, 10),
> > >> + [GRF_DPHY_RX1_CLK_INV_SEL] = PHY_REG(RK3399_GRF_SOC_CON9, 1, 11),
> > >> + [GRF_DPHY_RX0_ENABLE] = PHY_REG(RK3399_GRF_SOC_CON21, 4, 0),
> > >> + [GRF_DPHY_RX0_FORCERXMODE] = PHY_REG(RK3399_GRF_SOC_CON21, 4, 4),
> > >> + [GRF_DPHY_RX0_FORCETXSTOPMODE] = PHY_REG(RK3399_GRF_SOC_CON21, 4, 8),
> > >> + [GRF_DPHY_RX0_TURNDISABLE] = PHY_REG(RK3399_GRF_SOC_CON21, 4, 12),
> > >> + [GRF_DPHY_TX0_FORCERXMODE] = PHY_REG(RK3399_GRF_SOC_CON22, 4, 0),
> > >> + [GRF_DPHY_TX0_FORCETXSTOPMODE] = PHY_REG(RK3399_GRF_SOC_CON22, 4, 4),
> > >> + [GRF_DPHY_TX0_TURNDISABLE] = PHY_REG(RK3399_GRF_SOC_CON22, 4, 8),
> > >> + [GRF_DPHY_TX0_TURNREQUEST] = PHY_REG(RK3399_GRF_SOC_CON22, 4, 12),
> > >> + [GRF_DPHY_TX1RX1_ENABLE] = PHY_REG(RK3399_GRF_SOC_CON23, 4, 0),
> > >> + [GRF_DPHY_TX1RX1_FORCERXMODE] = PHY_REG(RK3399_GRF_SOC_CON23, 4, 4),
> > >> + [GRF_DPHY_TX1RX1_FORCETXSTOPMODE] = PHY_REG(RK3399_GRF_SOC_CON23, 4, 8),
> > >> + [GRF_DPHY_TX1RX1_TURNDISABLE] = PHY_REG(RK3399_GRF_SOC_CON23, 4, 12),
> > >> + [GRF_DPHY_TX1RX1_TURNREQUEST] = PHY_REG(RK3399_GRF_SOC_CON24, 4, 0),
> > >> + [GRF_DPHY_RX1_SRC_SEL] = PHY_REG(RK3399_GRF_SOC_CON24, 1, 4),
> > >> + [GRF_DPHY_TX1RX1_BASEDIR] = PHY_REG(RK3399_GRF_SOC_CON24, 1, 5),
> > >> + [GRF_DPHY_TX1RX1_ENABLECLK] = PHY_REG(RK3399_GRF_SOC_CON24, 1, 6),
> > >> + [GRF_DPHY_TX1RX1_MASTERSLAVEZ] = PHY_REG(RK3399_GRF_SOC_CON24, 1, 7),
> > >> + [GRF_DPHY_RX0_TESTDIN] = PHY_REG(RK3399_GRF_SOC_CON25, 8, 0),
> > >> + [GRF_DPHY_RX0_TESTEN] = PHY_REG(RK3399_GRF_SOC_CON25, 1, 8),
> > >> + [GRF_DPHY_RX0_TESTCLK] = PHY_REG(RK3399_GRF_SOC_CON25, 1, 9),
> > >> + [GRF_DPHY_RX0_TESTCLR] = PHY_REG(RK3399_GRF_SOC_CON25, 1, 10),
> > >> + [GRF_DPHY_RX0_TESTDOUT] = PHY_REG(RK3399_GRF_SOC_STATUS1, 8, 0),
>
> The annoying part with such an indirection is that you can't really
> write multiple fields in a single register with a single operation. Is
> the register mapping completely different between the rk3288 and the
> rk3399, or are the fields grouped in registers in a similar way ? In the
> latter case we could possibly optimise it.
>
> > >> +};
> > >> +
> > >> +struct dphy_drv_data {
> > >> + const char * const *clks;
> > >> + int num_clks;
>
> This is never negative, you can make it an unsigned int.
>
> > >> + const struct hsfreq_range *hsfreq_ranges;
> > >> + int num_hsfreq_ranges;
>
> Same here.
>
> > >> + const struct dphy_reg *regs;
> > >> +};
> > >> +
> > >> +struct rockchip_dphy {
> > >> + struct device *dev;
> > >> + struct regmap *grf;
> > >> + const struct dphy_reg *grf_regs;
> > >> + struct clk_bulk_data clks[MAX_DPHY_CLK];
> > >> +
> > >> + const struct dphy_drv_data *drv_data;
> > >> + struct phy_configure_opts_mipi_dphy config;
> > >> +};
> > >> +
> > >> +static inline void write_grf_reg(struct rockchip_dphy *priv,
> > >> + int index, u8 value)
>
> Maybe unsigned int index ?
>
> > >> +{
> > >> + const struct dphy_reg *reg = &priv->grf_regs[index];
> > >> + unsigned int val = HIWORD_UPDATE(value, reg->mask, reg->shift);
> > >> +
> > >> + WARN_ON(!reg->offset);
> > >> + regmap_write(priv->grf, reg->offset, val);
> > >> +}
> > >> +
> > >> +static void mipidphy0_wr_reg(struct rockchip_dphy *priv,
> > >> + u8 test_code, u8 test_data)
>
> Function (and structure) names have different prefixes, would it make
> sense to standardise them ? Maybe rockchip_dphy_ ? Or rk_dphy_ for a
> shorter version ? This could become rk_dphy_write_dphy(), and the
> previous function rk_dphy_write_grf().
>
> > >> +{
> > >> + /*
> > >> + * With the falling edge on TESTCLK, the TESTDIN[7:0] signal content
> > >> + * is latched internally as the current test code. Test data is
> > >> + * programmed internally by rising edge on TESTCLK.
> > >> + */
>
> I've never understood why PHYs tend to have a register named TEST that
> contains way more than test data :-)
>
> > >> + write_grf_reg(priv, GRF_DPHY_RX0_TESTCLK, 1);
> > >> + write_grf_reg(priv, GRF_DPHY_RX0_TESTDIN, test_code);
> > >> + write_grf_reg(priv, GRF_DPHY_RX0_TESTEN, 1);
> > >> + write_grf_reg(priv, GRF_DPHY_RX0_TESTCLK, 0);
> > >> + write_grf_reg(priv, GRF_DPHY_RX0_TESTEN, 0);
> > >> + write_grf_reg(priv, GRF_DPHY_RX0_TESTDIN, test_data);
> > >> + write_grf_reg(priv, GRF_DPHY_RX0_TESTCLK, 1);
> > >> +}
> > >> +
> > >> +/* should be move to power_on */
>
> s/move/moved/
>
> Do you mean merging the two functions together ? What prevents from
> doing so ?
>
> > >> +static int mipidphy_rx_stream_on(struct rockchip_dphy *priv)
> > >> +{
> > >> + const struct dphy_drv_data *drv_data = priv->drv_data;
> > >> + const struct hsfreq_range *hsfreq_ranges = drv_data->hsfreq_ranges;
> > >> + struct phy_configure_opts_mipi_dphy *config = &priv->config;
> > >> + unsigned int i, hsfreq = 0, data_rate_mbps = config->hs_clk_rate;
> > >> + int num_hsfreq_ranges = drv_data->num_hsfreq_ranges;
> > >> +
> > >> + do_div(data_rate_mbps, 1000 * 1000);
> > >> +
> > >> + dev_dbg(priv->dev, "%s: lanes %d - data_rate_mbps %u\n",
> > >> + __func__, config->lanes, data_rate_mbps);
> > >> +
> > >> + for (i = 0; i < num_hsfreq_ranges; i++) {
> > >> + if (hsfreq_ranges[i].range_h >= data_rate_mbps) {
> > >> + hsfreq = hsfreq_ranges[i].cfg_bit;
> > >> + break;
> > >> + }
> > >> + }
>
> As num_hsfreq_ranges and hsfreq_ranges are only used in this loop, I
> would remove the local variables.
>
> > >> +
> > >> + write_grf_reg(priv, GRF_DPHY_RX0_FORCERXMODE, 0);
> > >> + write_grf_reg(priv, GRF_DPHY_RX0_FORCETXSTOPMODE, 0);
> > >> +
> > >> + /* Disable lan turn around, which is ignored in receive mode */
>
> Is it "lan turn around", or "lane turn around" ?
>
> > >> + write_grf_reg(priv, GRF_DPHY_RX0_TURNREQUEST, 0);
> > >> + write_grf_reg(priv, GRF_DPHY_RX0_TURNDISABLE, 0xf);
> > >> +
> > >> + write_grf_reg(priv, GRF_DPHY_RX0_ENABLE, GENMASK(config->lanes - 1, 0));
> > >> +
> > >> + /* dphy start */
> > >> + write_grf_reg(priv, GRF_DPHY_RX0_TESTCLK, 1);
> > >> + write_grf_reg(priv, GRF_DPHY_RX0_TESTCLR, 1);
> > >> + usleep_range(100, 150);
> > >> + write_grf_reg(priv, GRF_DPHY_RX0_TESTCLR, 0);
> > >> + usleep_range(100, 150);
> > >> +
> > >> + /* set clock lane */
> > >> + /* HS hsfreq_range & lane 0 settle bypass */
> > >> + mipidphy0_wr_reg(priv, CLOCK_LANE_HS_RX_CONTROL, 0);
> > >> + /* HS RX Control of lane0 */
> > >> + mipidphy0_wr_reg(priv, LANE0_HS_RX_CONTROL, hsfreq << 1);
> > >> + /* HS RX Control of lane1 */
> > >> + mipidphy0_wr_reg(priv, LANE1_HS_RX_CONTROL, 0);
> > >> + /* HS RX Control of lane2 */
> > >> + mipidphy0_wr_reg(priv, LANE2_HS_RX_CONTROL, 0);
> > >> + /* HS RX Control of lane3 */
> > >> + mipidphy0_wr_reg(priv, LANE3_HS_RX_CONTROL, 0);
>
> Does this hardcode usage of a single lane ?
>
> > >> + /* HS RX Data Lanes Settle State Time Control */
> > >> + mipidphy0_wr_reg(priv, HS_RX_DATA_LANES_THS_SETTLE_CONTROL,
> > >> + THS_SETTLE_COUNTER_THRESHOLD);
> > >> +
> > >> + /* Normal operation */
> > >> + mipidphy0_wr_reg(priv, 0x0, 0);
> > >> +
> > >> + return 0;
> > >> +}
> > >> +
> > >> +static int rockchip_dphy_configure(struct phy *phy, union phy_configure_opts *opts)
> > >> +{
> > >> + struct rockchip_dphy *priv = phy_get_drvdata(phy);
> > >> + int ret;
> > >> +
> > >> + /* pass with phy_mipi_dphy_get_default_config (with pixel rate?) */
>
> I'm not sure to understand what this means.
>
> > >> + ret = phy_mipi_dphy_config_validate(&opts->mipi_dphy);
> > >> + if (ret)
> > >> + return ret;
> > >> +
> > >> + memcpy(&priv->config, opts, sizeof(priv->config));
> > >
> > > You could to:
> > >
> > > priv->config = *opts;
> > >
> > > Up to you. Some people like memcpy(). :-)
> >
> > your way is better thanks!
> >
> > >> +
> > >> + return 0;
> > >> +}
> > >> +
> > >> +static int rockchip_dphy_power_on(struct phy *phy)
> > >> +{
> > >> + struct rockchip_dphy *priv = phy_get_drvdata(phy);
> > >> + int ret;
> > >> +
> > >> + ret = clk_bulk_enable(priv->drv_data->num_clks, priv->clks);
> > >> + if (ret)
> > >> + return ret;
> > >> +
> > >> + return mipidphy_rx_stream_on(priv);
>
> Should you call clk_bulk_disable() if mipidphy_rx_stream_on() fails ?
> Actually that function never fails, so I'd make it a void function, and
> return 0 here.
>
> What happens if the clock rate is higher than the maximum supported by
> the PHY ? Shouldn't rockchip_dphy_configure() fail in that case ?
>
> > >> +}
> > >> +
> > >> +static int rockchip_dphy_power_off(struct phy *phy)
> > >> +{
> > >> + struct rockchip_dphy *priv = phy_get_drvdata(phy);
> > >> +
>
> No need to write any register ? That's scary, what will happen on the
> next power on, when the clocks gets enabled ?
>
> > >> + clk_bulk_disable(priv->drv_data->num_clks, priv->clks);
> > >> + return 0;
> > >> +}
> > >> +
> > >> +static int rockchip_dphy_init(struct phy *phy)
> > >> +{
> > >> + struct rockchip_dphy *priv = phy_get_drvdata(phy);
> > >> + int ret;
> > >> +
> > >> + ret = clk_bulk_prepare(priv->drv_data->num_clks, priv->clks);
> > >
> > > return ...;
> > >
> > >> + if (ret)
> > >> + return ret;
> > >> + return 0;
> > >> +}
> > >> +
> > >> +static int rockchip_dphy_exit(struct phy *phy)
> > >> +{
> > >> + struct rockchip_dphy *priv = phy_get_drvdata(phy);
> > >> +
> > >> + clk_bulk_unprepare(priv->drv_data->num_clks, priv->clks);
> > >> + return 0;
> > >> +}
> > >> +
> > >> +static const struct phy_ops rockchip_dphy_ops = {
> > >> + .power_on = rockchip_dphy_power_on,
> > >> + .power_off = rockchip_dphy_power_off,
> > >> + .init = rockchip_dphy_init,
> > >> + .exit = rockchip_dphy_exit,
> > >> + .configure = rockchip_dphy_configure,
> > >> + .owner = THIS_MODULE,
> > >> +};
> > >> +
> > >> +static const struct dphy_drv_data rk3399_mipidphy_drv_data = {
> > >> + .clks = rk3399_mipidphy_clks,
> > >> + .num_clks = ARRAY_SIZE(rk3399_mipidphy_clks),
> > >> + .hsfreq_ranges = rk3399_mipidphy_hsfreq_ranges,
> > >> + .num_hsfreq_ranges = ARRAY_SIZE(rk3399_mipidphy_hsfreq_ranges),
> > >> + .regs = rk3399_grf_dphy_regs,
> > >
> > > Do you expect to support more of the similar PHY(s) --- are there such? If
> > > not, you could put these in the code that uses them.
> >
> > Yes, for rk3288 in the future.
> >
> > >> +};
> > >> +
> > >> +static const struct of_device_id rockchip_dphy_dt_ids[] = {
> > >> + {
> > >> + .compatible = "rockchip,rk3399-mipi-dphy",
> > >> + .data = &rk3399_mipidphy_drv_data,
> > >> + },
> > >> + {}
> > >> +};
> > >> +MODULE_DEVICE_TABLE(of, rockchip_dphy_dt_ids);
> > >> +
> > >> +static int rockchip_dphy_probe(struct platform_device *pdev)
> > >> +{
> > >> + struct device *dev = &pdev->dev;
> > >> + struct device_node *np = dev->of_node;
> > >> + const struct dphy_drv_data *drv_data;
> > >> + struct phy_provider *phy_provider;
> > >> + const struct of_device_id *of_id;
> > >> + struct rockchip_dphy *priv;
> > >> + struct regmap *grf;
> > >> + struct phy *phy;
> > >> + unsigned int i;
> > >> + int ret;
> > >> +
> > >> + if (!dev->parent || !dev->parent->of_node)
> > >> + return -ENODEV;
> > >> +
> > >> + if (platform_get_resource(pdev, IORESOURCE_MEM, 0)) {
> > >> + dev_err(&pdev->dev, "Rockchip DPHY driver only suports rx\n");
>
> You can replace pdev->dev with dev here and below.
>
> s/rx/RX mode/ ?
>
> > >> + return -EINVAL;
> > >> + }
> > >> +
> > >> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> > >> + if (!priv)
> > >> + return -ENOMEM;
> > >> + priv->dev = dev;
> > >> +
> > >> + grf = syscon_node_to_regmap(dev->parent->of_node);
> > >> + if (IS_ERR(grf)) {
> > >> + grf = syscon_regmap_lookup_by_phandle(dev->of_node,
> > >> + "rockchip,grf");
> > >> + if (IS_ERR(grf)) {
> > >> + dev_err(dev, "Can't find GRF syscon\n");
> > >> + return -ENODEV;
> > >> + }
> > >> + }
> > >> + priv->grf = grf;
> > >> +
> > >> + of_id = of_match_device(rockchip_dphy_dt_ids, dev);
> > >> + if (!of_id)
> > >> + return -EINVAL;
> > >> +
> > >> + drv_data = of_id->data;
> > >> + priv->grf_regs = drv_data->regs;
>
> Do you have to store grf_regs in priv, or could it be accessed through
> priv->drv_data->regs ?
>
> > >> + priv->drv_data = drv_data;
> > >> + for (i = 0; i < drv_data->num_clks; i++)
> > >> + priv->clks[i].id = drv_data->clks[i];
> > >> + ret = devm_clk_bulk_get(&pdev->dev, drv_data->num_clks, priv->clks);
> > >> + if (ret)
> > >> + return ret;
> > >> +
> > >> + phy = devm_phy_create(dev, np, &rockchip_dphy_ops);
> > >> + if (IS_ERR(phy)) {
> > >> + dev_err(dev, "failed to create phy\n");
> > >> + return PTR_ERR(phy);
> > >> + }
> > >> + phy_set_drvdata(phy, priv);
> > >> +
> > >> + phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> > >> +
> > >> + return PTR_ERR_OR_ZERO(phy_provider);
> > >> +}
> > >> +
> > >> +static struct platform_driver rockchip_dphy_driver = {
> > >> + .probe = rockchip_dphy_probe,
> > >> + .driver = {
> > >> + .name = "rockchip-mipi-dphy",
> > >> + .of_match_table = rockchip_dphy_dt_ids,
> > >> + },
> > >> +};
> > >> +module_platform_driver(rockchip_dphy_driver);
> > >> +
> > >> +MODULE_AUTHOR("Ezequiel Garcia <ezequiel@collabora.com>");
> > >> +MODULE_DESCRIPTION("Rockchip MIPI Synopsys DPHY driver");
> > >> +MODULE_LICENSE("Dual MIT/GPL");
>
> Overall this is quite good, there are only small issues.
>
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 7/9] soc: samsung: Update the CHIP ID DT binding documentation
From: Krzysztof Kozlowski @ 2019-08-15 18:24 UTC (permalink / raw)
To: Sylwester Nawrocki
Cc: devicetree, linux-samsung-soc, linux-pm, pankaj.dubey,
b.zolnierkie, linux-kernel, robh+dt, kgene, vireshk,
linux-arm-kernel, m.szyprowski
In-Reply-To: <20190813150827.31972-8-s.nawrocki@samsung.com>
On Tue, Aug 13, 2019 at 05:08:25PM +0200, Sylwester Nawrocki wrote:
> This patch adds documentation of a new optional "samsung,asv-bin"
> property in the chipid device node and documents requirement of
> "syscon" compatible string. These additions are needed to support
> Exynos ASV (Adaptive Supply Voltage) feature.
>
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> ---
> Changes since v2:
> - none
>
> Changes since v1 (RFC):
Title should match bindings, so:
dt-bindings: samsung:
Also please put it before the driver change using these bindings.
Best regards,
Krzysztof
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 2/9] soc: samsung: Convert exynos-chipid driver to use the regmap API
From: Krzysztof Kozlowski @ 2019-08-15 18:22 UTC (permalink / raw)
To: Sylwester Nawrocki
Cc: devicetree, linux-samsung-soc, linux-pm, pankaj.dubey,
b.zolnierkie, linux-kernel, robh+dt, kgene, vireshk,
linux-arm-kernel, m.szyprowski
In-Reply-To: <20190813150827.31972-3-s.nawrocki@samsung.com>
On Tue, Aug 13, 2019 at 05:08:20PM +0200, Sylwester Nawrocki wrote:
> Convert the driver to use regmap API in order to allow other
> drivers, like ASV, to access the CHIPID registers.
>
> This patch adds definition of selected CHIPID register offsets
> and register bit fields for Exynos5422 SoC.
>
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> ---
> Changes since v2:
> - s/_EXYNOS_ASV_H/__LINU_SOC_EXYNOS_ASV_H,
> - removed __func__ from error log,
> - removed unneeded <linux/of_address.h> header inclusion.
>
> Changes since v1 (RFC):
> - new patch
> ---
> drivers/soc/samsung/exynos-chipid.c | 34 ++++++---------
> include/linux/soc/samsung/exynos-chipid.h | 52 +++++++++++++++++++++++
> 2 files changed, 65 insertions(+), 21 deletions(-)
Thanks, applied.
Best regards,
Krzysztof
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 0/6] drm+dma: cache support for arm, etc
From: Koenig, Christian @ 2019-08-15 18:21 UTC (permalink / raw)
To: Christoph Hellwig, Rob Clark
Cc: Kate Stewart, Masayoshi Mizuma, Maciej W. Rozycki, Eric Biggers,
Catalin Marinas, Imre Deak, dri-devel, Chris Wilson,
Masahiro Yamada, Benjamin Gaignard, Mauro Carvalho Chehab,
Will Deacon, Emil Velikov, Sharma, Deepak, Michael Ellerman,
Paul Burton, Mike Rapoport, Geert Uytterhoeven,
moderated list:ARM64 PORT (AARCH64 ARCHITECTURE), Daniel Vetter,
open list:MIPS, Linus Walleij, Robin Murphy,
open list:DRM DRIVER FOR MSM ADRENO GPU, Joerg Roedel,
Arnd Bergmann, Anshuman Khandual, Hauke Mehrtens,
Jesper Dangaard Brouer, Wolfram Sang (Renesas),
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT), Alexios Zavras,
Russell King, Doug Anderson, Thomas Gleixner, Sean Paul,
Allison Randal, Christophe Leroy, Enrico Weigelt, Ard Biesheuvel,
Greg Kroah-Hartman, open list, Rob Clark, Souptick Joarder,
Andrew Morton, open list:DRM DRIVER FOR MSM ADRENO GPU
In-Reply-To: <20190815175346.GA19839@lst.de>
Am 15.08.19 um 19:53 schrieb Christoph Hellwig:
> On Thu, Aug 15, 2019 at 06:54:39AM -0700, Rob Clark wrote:
>> On Wed, Aug 14, 2019 at 11:51 PM Christoph Hellwig <hch@lst.de> wrote:
>>> As said before I don't think these low-level helpers are the
>>> right API to export, but even if they did you'd just cover a tiny
>>> subset of the architectures.
>> Are you thinking instead something like:
>>
>> void dma_sync_sg_for_{cpu,device}(struct device *dev, struct scatterlist *sgl,
>> int nents, enum dma_data_direction dir)
>> {
>> for_each_sg(sgl, sg, nents, i) {
>> arch_sync_dma_for_..(dev, sg_phys(sg), sg->length, dir);
>> }
>> }
>> EXPORT_SYMBOL_GPL(dma_sync_sg_for_..)
>>
>> or did you have something else in mind?
> No. We really need an interface thay says please give me uncached
> memory (for some definition of uncached that includes that grapics
> drivers call write combine), and then let the architecture do the right
> thing. Basically dma_alloc_coherent with DMA_ATTR_NO_KERNEL_MAPPING
> is superficially close to what you want, except that the way the drm
> drivers work you can't actually use it.
The terminology graphics driver use is USWC (Uncached Speculative Write
Combine).
Essentially it is a leftover from the AGP days where the graphics
devices couldn't snoop the CPU caches.
Nowadays they either don't want to snoop the CPU caches for performance
reasons.
Or because of special requirements that certain areas of system memory
are not cached for certain functionality.
For example the "VRAM" on AMD GPUs which are integrated into the CPU is
just stolen system memory. Then you can scanout your picture to the
display from this memory or "normal" system memory, but only if the
system memory is mapped as USWC.
> The reason for that is if we can we really need to not create another
> uncachable alias, but instead change the page attributes in place.
> On x86 we can and must do that for example, and based on the
> conversation with Will arm64 could do that fairly easily. arm32 can
> right now only do that for CMA, though.
>
> The big question is what API do we want. I had a pretty similar
> discussion with Christian on doing such an allocation for amdgpu,
> where the device normally is cache coherent, but they actually want
> to turn it into non-coherent by using PCIe unsnooped transactions.
>
> Here is my high level plan, which still has a few lose end:
>
> (1) provide a new API:
>
> struct page *dma_alloc_pages(struct device *dev, unsigned nr_pages,
> gfp_t gfp, unsigned long flags);
> void dma_free_pages(struct device *dev, unsigned nr_pages,
> unsigned long flags);
>
> These give you back page backed memory that is guaranteed to be
> addressable by the device (no swiotlb or similar). The memory can
> then be mapped using dma_map*, including unmap and dma_sync to
> bounce ownership around. This is the replacement for the current
> dma_alloc_attrs with DMA_ATTR_NON_CONSISTENT API, that is rather
> badly defined.
>
> (2) Add support for DMA_ATTR_NO_KERNEL_MAPPING to this new API instead
> of dma_alloc_attrs. The initial difference with that flag is just
> that we allow highmem, but in the future we could also unmap this
> memory from the kernel linear mapping entirely on architectures
> where we can easily do that.
Mhm, why would we want to do this?
>
> (3) Add a dma_pages_map/dma_pages_unmap or similar API that allows you
> to get a kernel mapping for parts or all of a
> DMA_ATTR_NO_KERNEL_MAPPING allocation. This is to replace things
> like your open-coded vmap in msm (or similarly elsewhere in dma-buf
> providers).
>
> (4) Add support for a DMA_ATTR_UNCACHABLE flags (or similar) to the new
> API, that maps the pages as uncachable iff they have a kernel
> mapping, including invalidating the caches at time of this page
> attribute change (or creation of a new mapping). This API will fail
> if the architecture does not allow in-place remapping. Note that for
> arm32 we could always dip into the CMA pool if one is present to not
> fail. We'll also need some helper to map from the DMA_ATTR_* flags
> to a pgprot for mapping the page to userspace. There is also a few
> other weird bits here, e.g. on architectures like mips that use an
> uncached segment we'll have to fail use with the plain
> DMA_ATTR_UNCACHABLE flag, but it could be supported with
> DMA_ATTR_UNCACHABLE | DMA_ATTR_NO_KERNEL_MAPPING.
>
> I was hoping to get most of this done for this merge window, but I'm
> probably lucky if I get at least parts done. Too much distraction.
Thanks again for taking care of this,
Christian.
>
>> Hmm, not entirely sure why.. you should be on the cc list for each
>> individual patch.
> They finally made it, although even with the delay they only ended up
> in the spam mailbox. I still can't see them on the various mailing
> lists.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 1/9] soc: samsung: Add exynos chipid driver support
From: Krzysztof Kozlowski @ 2019-08-15 18:18 UTC (permalink / raw)
To: Sylwester Nawrocki
Cc: devicetree, linux-samsung-soc, linux-pm, pankaj.dubey,
b.zolnierkie, linux-kernel, robh+dt, kgene, vireshk,
linux-arm-kernel, m.szyprowski
In-Reply-To: <20190813150827.31972-2-s.nawrocki@samsung.com>
On Tue, Aug 13, 2019 at 05:08:19PM +0200, Sylwester Nawrocki wrote:
> From: Pankaj Dubey <pankaj.dubey@samsung.com>
>
> Exynos SoCs have Chipid, for identification of product IDs and SoC
> revisions. This patch intends to provide initialization code for all
> these functionalities, at the same time it provides some sysfs entries
> for accessing these information to user-space.
>
Thanks, applied.
Best regards,
Krzysztof
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v8 11/14] dt-bindings: Document the Rockchip MIPI RX D-PHY bindings
From: Laurent Pinchart @ 2019-08-15 18:14 UTC (permalink / raw)
To: Helen Koike
Cc: devicetree, eddie.cai.linux, kernel, heiko, Rob Herring,
jacob2.chen, jeffy.chen, zyc, linux-kernel, tfiga, linux-rockchip,
hans.verkuil, sakari.ailus, zhengsq, mchehab, ezequiel,
linux-arm-kernel, linux-media
In-Reply-To: <20190730184256.30338-12-helen.koike@collabora.com>
Hi Helen,
Thank you for the patch.
On Tue, Jul 30, 2019 at 03:42:53PM -0300, Helen Koike wrote:
> From: Jacob Chen <jacob2.chen@rock-chips.com>
>
> Add DT bindings documentation for Rockchip MIPI D-PHY RX
>
> Signed-off-by: Jacob Chen <jacob2.chen@rock-chips.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
> [update for upstream]
> Signed-off-by: Helen Koike <helen.koike@collabora.com>
>
> ---
>
> Changes in v8: None
> Changes in v7:
> - updated doc with new design and tested example
>
> .../bindings/media/rockchip-mipi-dphy.txt | 38 +++++++++++++++++++
Shouldn't this go to bindings/phy/ ?
> 1 file changed, 38 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/rockchip-mipi-dphy.txt
>
> diff --git a/Documentation/devicetree/bindings/media/rockchip-mipi-dphy.txt b/Documentation/devicetree/bindings/media/rockchip-mipi-dphy.txt
> new file mode 100644
> index 000000000000..2305d44d92db
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/rockchip-mipi-dphy.txt
> @@ -0,0 +1,38 @@
> +Rockchip SoC MIPI RX D-PHY
> +-------------------------------------------------------------
How about already converting the bindings to yaml ? There's one example
in bindings/phy/ that looks quite similar to what you need here. Make
sure to have a look at Documentation/devicetree/writing-schema.md, and
in particular to run make dt_binding_check.
> +
> +Required properties:
> +- compatible: value should be one of the following
> + "rockchip,rk3288-mipi-dphy"
> + "rockchip,rk3399-mipi-dphy"
> +- clocks : list of clock specifiers, corresponding to entries in
> + clock-names property;
> +- clock-names: required clock name.
> +- #phy-cells: Number of cells in a PHY specifier; Should be 0.
> +
> +MIPI RX D-PHY use registers in "general register files", it
> +should be a child of the GRF.
> +
> +Optional properties:
> +- reg: offset and length of the register set for the device.
> +- rockchip,grf: MIPI TX1RX1 D-PHY not only has its own register but also
> + the GRF, so it is only necessary for MIPI TX1RX1 D-PHY.
> +
> +Device node example
> +-------------------
> +
> +grf: syscon@ff770000 {
> + compatible = "rockchip,rk3399-grf", "syscon", "simple-mfd";
> +
> +...
> +
> + dphy: mipi-dphy {
> + compatible = "rockchip,rk3399-mipi-dphy";
> + clocks = <&cru SCLK_MIPIDPHY_REF>,
> + <&cru SCLK_DPHY_RX0_CFG>,
> + <&cru PCLK_VIO_GRF>;
> + clock-names = "dphy-ref", "dphy-cfg", "grf";
> + power-domains = <&power RK3399_PD_VIO>;
> + #phy-cells = <0>;
> + };
> +};
--
Regards,
Laurent Pinchart
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v1 2/8] arm64, mm: transitional tables
From: James Morse @ 2019-08-15 18:11 UTC (permalink / raw)
To: Pavel Tatashin
Cc: sashal, vladimir.murzin, corbet, marc.zyngier, catalin.marinas,
bhsharma, kexec, linux-kernel, jmorris, linux-mm, ebiederm,
matthias.bgg, will, linux-arm-kernel
In-Reply-To: <20190801152439.11363-3-pasha.tatashin@soleen.com>
Hi Pavel,
On 01/08/2019 16:24, Pavel Tatashin wrote:
> There are cases where normal kernel pages tables, i.e. idmap_pg_dir
> and swapper_pg_dir are not sufficient because they may be overwritten.
>
> This happens when we transition from one world to another: for example
> during kexec kernel relocation transition, and also during hibernate
> kernel restore transition.
>
> In these cases, if MMU is needed, the page table memory must be allocated
> from a safe place. Transitional tables is intended to allow just that.
> diff --git a/arch/arm64/include/asm/pgtable-hwdef.h b/arch/arm64/include/asm/pgtable-hwdef.h
> index db92950bb1a0..dcb4f13c7888 100644
> --- a/arch/arm64/include/asm/pgtable-hwdef.h
> +++ b/arch/arm64/include/asm/pgtable-hwdef.h
> @@ -110,6 +110,7 @@
> #define PUD_TABLE_BIT (_AT(pudval_t, 1) << 1)
> #define PUD_TYPE_MASK (_AT(pudval_t, 3) << 0)
> #define PUD_TYPE_SECT (_AT(pudval_t, 1) << 0)
> +#define PUD_SECT_RDONLY (_AT(pudval_t, 1) << 7) /* AP[2] */
This shouldn't be needed. As far as I'm aware, we only get read-only pages in the linear
map from debug-pagealloc, and the module aliases. Both of which require the linear map to
be made of page-size mappings.
Where are you seeing these?
> diff --git a/arch/arm64/include/asm/trans_table.h b/arch/arm64/include/asm/trans_table.h
> new file mode 100644
> index 000000000000..c7aef70587a1
> --- /dev/null
> +++ b/arch/arm64/include/asm/trans_table.h
> @@ -0,0 +1,68 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +/*
> + * Copyright (c) 2019, Microsoft Corporation.
> + * Pavel Tatashin <patatash@linux.microsoft.com>
> + */
> +
> +#ifndef _ASM_TRANS_TABLE_H
> +#define _ASM_TRANS_TABLE_H
> +
> +#include <linux/bits.h>
> +#include <asm/pgtable-types.h>
> +
> +/*
> + * trans_alloc_page
> + * - Allocator that should return exactly one uninitilaized page, if this
> + * allocator fails, trans_table returns -ENOMEM error.
> + *
> + * trans_alloc_arg
> + * - Passed to trans_alloc_page as an argument
> + *
> + * trans_flags
> + * - bitmap with flags that control how page table is filled.
> + * TRANS_MKWRITE: during page table copy make PTE, PME, and PUD page
> + * writeable by removing RDONLY flag from PTE.
> + * TRANS_MKVALID: during page table copy, if PTE present, but not valid,
> + * make it valid.
> + * TRANS_CHECKPFN: During page table copy, for every PTE entry check that
> + * PFN that this PTE points to is valid. Otherwise return
> + * -ENXIO
Adding top-level global knobs to manipulate the copied linear map is going to lead to
bugs. The existing code will only change the PTE in specific circumstances, that it tests
for, that only happen at the PTE level.
> + * TRANS_FORCEMAP: During page map, if translation exists, force
> + * overwrite it. Otherwise -ENXIO may be returned by
> + * trans_table_map_* functions if conflict is detected.
This one, sounds like a very bad idea.
Thanks,
James
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v1 0/8] arm64: MMU enabled kexec relocation
From: James Morse @ 2019-08-15 18:11 UTC (permalink / raw)
To: Pavel Tatashin
Cc: Sasha Levin, Vladimir Murzin, Jonathan Corbet, Marc Zyngier,
Catalin Marinas, Bhupesh Sharma, kexec mailing list, LKML,
James Morris, linux-mm, Eric W. Biederman, Matthias Brugger, will,
Linux ARM
In-Reply-To: <CA+CK2bADiBMEx9cJuXT5fQkBYFZAtxUtc7ZzjrNfEjijPZkPtw@mail.gmail.com>
Hi Pavel,
On 08/08/2019 19:44, Pavel Tatashin wrote:
> Just a friendly reminder, please send your comments on this series.
(Please don't top-post)
> It's been a week since I sent out these patches, and no feedback yet.
A week is not a lot of time, people are busy, go to conferences, some even dare to take
holiday!
> Also, I'd appreciate if anyone could test this series on vhe hardware
> with vhe kernel, it does not look like QEMU can emulate it yet
This locks up during resume from hibernate on my AMD Seattle, a regular v8.0 machine.
Please try and build the series to reduce review time. What you have here is an all-new
page-table generation API, which you switch hibernate and kexec too. This is effectively a
new implementation of hibernate and kexec. There are three things here that need review.
You have a regression in your all-new implementation of hibernate. It took six months (and
lots of review) to get the existing code right, please don't rip it out if there is
nothing wrong with it.
Instead, please just move the hibernate copy_page_tables() code, and then wire kexec up.
You shouldn't need to change anything in the copy_page_tables() code as the linear map is
the same in both cases.
It looks like you are creating the page tables just after the kexec:segments have been
loaded. This will go horribly wrong if anything changes between then and kexec time. (e.g.
memory you've got mapped gets hot-removed).
This needs to be done as late as possible, so we don't waste memory, and the world can't
change around us. Reboot notifiers run before kexec, can't we do the memory-allocation there?
> On Thu, Aug 1, 2019 at 11:24 AM Pavel Tatashin
> <pasha.tatashin@soleen.com> wrote:
>>
>> Enable MMU during kexec relocation in order to improve reboot performance.
>>
>> If kexec functionality is used for a fast system update, with a minimal
>> downtime, the relocation of kernel + initramfs takes a significant portion
>> of reboot.
>>
>> The reason for slow relocation is because it is done without MMU, and thus
>> not benefiting from D-Cache.
>>
>> Performance data
>> ----------------
>> For this experiment, the size of kernel plus initramfs is small, only 25M.
>> If initramfs was larger, than the improvements would be greater, as time
>> spent in relocation is proportional to the size of relocation.
>>
>> Previously:
>> kernel shutdown 0.022131328s
>> relocation 0.440510736s
>> kernel startup 0.294706768s
>>
>> Relocation was taking: 58.2% of reboot time
>>
>> Now:
>> kernel shutdown 0.032066576s
>> relocation 0.022158152s
>> kernel startup 0.296055880s
>>
>> Now: Relocation takes 6.3% of reboot time
>>
>> Total reboot is x2.16 times faster.
When I first saw these numbers they were ~'0.29s', which I wrongly assumed was 29 seconds.
Savings in milliseconds, for _reboot_ is a hard sell. I'm hoping that on the machines that
take minutes to kexec we'll get numbers that make this change more convincing.
Thanks,
James
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v5 06/18] compat_ioctl: move WDIOC handling into wdt drivers
From: Guenter Roeck @ 2019-08-15 18:10 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-hwmon, linux-rtc, Alexandre Belloni, Jean Delvare,
Anatolij Gustschin, linux-um, linux-kernel, Ludovic Desroches,
Florian Fainelli, bcm-kernel-feedback-list, viro, linux-fsdevel,
openipmi-developer, Wim Van Sebroeck, linuxppc-dev,
linux-arm-kernel, linux-watchdog
In-Reply-To: <20190814205245.121691-1-arnd@arndb.de>
On Wed, Aug 14, 2019 at 10:49:18PM +0200, Arnd Bergmann wrote:
> All watchdog drivers implement the same set of ioctl commands, and
> fortunately all of them are compatible between 32-bit and 64-bit
> architectures.
>
> Modern drivers always go through drivers/watchdog/wdt.c as an abstraction
> layer, but older ones implement their own file_operations on a character
> device for this.
>
> Move the handling from fs/compat_ioctl.c into the individual drivers.
>
> Note that most of the legacy drivers will never be used on 64-bit
> hardware, because they are for an old 32-bit SoC implementation, but
> doing them all at once is safer than trying to guess which ones do
> or do not need the compat_ioctl handling.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
This patch doesn't seem to have a useful base (or at least git says so).
It does not apply to mainline nor to my own watchdog-next branch.
I assume you plan to apply the entire series together. Please not
that there will be conflicts against watchdog-next when you do so.
Guenter
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 2/2] arm64: Don't use KPTI where we have E0PD
From: Mark Brown @ 2019-08-15 18:00 UTC (permalink / raw)
To: Will Deacon; +Cc: Catalin Marinas, linux-arm-kernel, Suzuki K Poulose
In-Reply-To: <20190815163541.yngqvjmehpuf74ye@willie-the-truck>
[-- Attachment #1.1: Type: text/plain, Size: 1351 bytes --]
On Thu, Aug 15, 2019 at 05:35:42PM +0100, Will Deacon wrote:
> I'm still unsure as to how this works with the kaslr check in
> kpti_install_ng_mappings(). Imagine you have a big.LITTLE system using
> kaslr where the boot CPU has E0PD but the secondary CPU doesn't, and
> requires kpti.
Yes, in fact that is my default big.LITTLE test case.
> In this case, I think we'll:
> 1. Start off with global mappings installed by the boot CPU
> 2. Detect KPTI as being required on the secondary CPU
> 3. Avoid rewriting the page tables because kaslr_offset > 0
> At this point, we've got exposed global mappings on the secondary CPU.
Right, yes. It'd be enormously helpful if KASLR were a bit more visible
in the boot logs or something since I yet again managed to do that bit
of my testing without KASLR actually taking effect :/
> Thinking about this further, I think we can simply move all of the
> 'kaslr_offset() > 0' checks used by the kpti code (i.e. in
> arm64_kernel_unmapped_at_el0(), kpti_install_ng_mappings() and
> unmap_kernel_at_el0()) into a helper function which does the check for
> E0PD as well. Perhaps 'kaslr_requires_kpti()' ?
> I think that should simplify your patch as well. What do you think?
Dunno about simplifying the patch particularly, looks very similar but
in any case it does appear to solve the problem - thanks.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v8 04/14] media: rkisp1: add Rockchip MIPI Synopsys DPHY driver
From: Laurent Pinchart @ 2019-08-15 17:54 UTC (permalink / raw)
To: Helen Koike
Cc: devicetree, eddie.cai.linux, kernel, heiko, jacob2.chen,
jeffy.chen, zyc, linux-kernel, tfiga, linux-rockchip,
Sakari Ailus, sakari.ailus, zhengsq, hans.verkuil, mchehab,
ezequiel, linux-arm-kernel, linux-media
In-Reply-To: <c61498b0-dd4c-53af-db82-169f8dfdc6bd@collabora.com>
Hi Helen,
Thank you for the patch.
On Wed, Aug 07, 2019 at 10:37:55AM -0300, Helen Koike wrote:
> On 8/7/19 10:05 AM, Sakari Ailus wrote:
> > On Tue, Jul 30, 2019 at 03:42:46PM -0300, Helen Koike wrote:
> >> From: Jacob Chen <jacob2.chen@rock-chips.com>
> >>
> >> This commit adds a subdev driver for Rockchip MIPI Synopsys DPHY driver
> >>
> >> Signed-off-by: Jacob Chen <jacob2.chen@rock-chips.com>
> >> Signed-off-by: Shunqian Zheng <zhengsq@rock-chips.com>
> >> Signed-off-by: Tomasz Figa <tfiga@chromium.org>
> >> [migrate to phy framework]
> >> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> >> [update for upstream]
> >> Signed-off-by: Helen Koike <helen.koike@collabora.com>
> >>
> >> ---
> >>
> >> Changes in v8:
> >> - Remove boiler plate license text
> >>
> >> Changes in v7:
> >> - Migrate dphy specific code from
> >> drivers/media/platform/rockchip/isp1/mipi_dphy_sy.c
> >> to drivers/phy/rockchip/phy-rockchip-dphy.c
> >> - Drop support for rk3288
> >> - Drop support for dphy txrx
> >> - code styling and checkpatch fixes
> >>
> >> drivers/phy/rockchip/Kconfig | 8 +
> >> drivers/phy/rockchip/Makefile | 1 +
> >> drivers/phy/rockchip/phy-rockchip-dphy.c | 408 +++++++++++++++++++++++
> >> 3 files changed, 417 insertions(+)
> >> create mode 100644 drivers/phy/rockchip/phy-rockchip-dphy.c
> >>
> >> diff --git a/drivers/phy/rockchip/Kconfig b/drivers/phy/rockchip/Kconfig
> >> index c454c90cd99e..afd072f135e6 100644
> >> --- a/drivers/phy/rockchip/Kconfig
> >> +++ b/drivers/phy/rockchip/Kconfig
> >> @@ -9,6 +9,14 @@ config PHY_ROCKCHIP_DP
> >> help
> >> Enable this to support the Rockchip Display Port PHY.
> >>
> >> +config PHY_ROCKCHIP_DPHY
> >> + tristate "Rockchip MIPI Synopsys DPHY driver"
How much of this PHY is Rockchip-specific ? Would it make sense to turn
it into a Synopsys DPHY driver, with some Rockchip glue ? I suppose this
could always be done later, if needed (and I also suppose there's no
existing driver in drivers/phy/ that support the same Synopsys IP).
> >> + depends on ARCH_ROCKCHIP && OF
> >
> > How about (...) || COMPILE_TEST ?
> >
> >> + select GENERIC_PHY_MIPI_DPHY
> >> + select GENERIC_PHY
> >> + help
> >> + Enable this to support the Rockchip MIPI Synopsys DPHY.
> >> +
> >> config PHY_ROCKCHIP_EMMC
> >> tristate "Rockchip EMMC PHY Driver"
> >> depends on ARCH_ROCKCHIP && OF
> >> diff --git a/drivers/phy/rockchip/Makefile b/drivers/phy/rockchip/Makefile
> >> index fd21cbaf40dd..f62e9010bcaf 100644
> >> --- a/drivers/phy/rockchip/Makefile
> >> +++ b/drivers/phy/rockchip/Makefile
> >> @@ -1,5 +1,6 @@
> >> # SPDX-License-Identifier: GPL-2.0
> >> obj-$(CONFIG_PHY_ROCKCHIP_DP) += phy-rockchip-dp.o
> >> +obj-$(CONFIG_PHY_ROCKCHIP_DPHY) += phy-rockchip-dphy.o
> >> obj-$(CONFIG_PHY_ROCKCHIP_EMMC) += phy-rockchip-emmc.o
> >> obj-$(CONFIG_PHY_ROCKCHIP_INNO_HDMI) += phy-rockchip-inno-hdmi.o
> >> obj-$(CONFIG_PHY_ROCKCHIP_INNO_USB2) += phy-rockchip-inno-usb2.o
> >> diff --git a/drivers/phy/rockchip/phy-rockchip-dphy.c b/drivers/phy/rockchip/phy-rockchip-dphy.c
> >> new file mode 100644
> >> index 000000000000..3a29976c2dff
> >> --- /dev/null
> >> +++ b/drivers/phy/rockchip/phy-rockchip-dphy.c
> >> @@ -0,0 +1,408 @@
> >> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> >> +/*
> >> + * Rockchip MIPI Synopsys DPHY driver
> >> + *
> >> + * Based on:
> >> + *
> >> + * Copyright (C) 2016 FuZhou Rockchip Co., Ltd.
> >> + * Author: Yakir Yang <ykk@@rock-chips.com>
> >> + */
> >> +
> >> +#include <linux/clk.h>
> >> +#include <linux/io.h>
> >> +#include <linux/mfd/syscon.h>
> >> +#include <linux/module.h>
> >> +#include <linux/of.h>
> >> +#include <linux/of_device.h>
> >> +#include <linux/phy/phy.h>
> >> +#include <linux/phy/phy-mipi-dphy.h>
> >> +#include <linux/platform_device.h>
> >> +#include <linux/regmap.h>
> >> +
> >> +#define RK3399_GRF_SOC_CON9 0x6224
> >> +#define RK3399_GRF_SOC_CON21 0x6254
> >> +#define RK3399_GRF_SOC_CON22 0x6258
> >> +#define RK3399_GRF_SOC_CON23 0x625c
> >> +#define RK3399_GRF_SOC_CON24 0x6260
> >> +#define RK3399_GRF_SOC_CON25 0x6264
> >> +#define RK3399_GRF_SOC_STATUS1 0xe2a4
> >> +
> >> +#define CLOCK_LANE_HS_RX_CONTROL 0x34
> >> +#define LANE0_HS_RX_CONTROL 0x44
> >> +#define LANE1_HS_RX_CONTROL 0x54
> >> +#define LANE2_HS_RX_CONTROL 0x84
> >> +#define LANE3_HS_RX_CONTROL 0x94
> >> +#define HS_RX_DATA_LANES_THS_SETTLE_CONTROL 0x75
> >> +
> >> +#define MAX_DPHY_CLK 8
> >> +
> >> +#define PHY_TESTEN_ADDR (0x1 << 16)
> >> +#define PHY_TESTEN_DATA (0x0 << 16)
> >> +#define PHY_TESTCLK (0x1 << 1)
> >> +#define PHY_TESTCLR (0x1 << 0)
Maybe s/0x// for the previous four lines ?
> >> +#define THS_SETTLE_COUNTER_THRESHOLD 0x04
> >> +
> >> +#define HIWORD_UPDATE(val, mask, shift) \
> >> + ((val) << (shift) | (mask) << ((shift) + 16))
As you use this in a single place, I would inline it, possibly with a
small comment that explains what's happening.
> >> +
> >> +#define GRF_SOC_CON12 0x0274
> >> +
> >> +#define GRF_EDP_REF_CLK_SEL_INTER_HIWORD_MASK BIT(20)
> >> +#define GRF_EDP_REF_CLK_SEL_INTER BIT(4)
> >> +
> >> +#define GRF_EDP_PHY_SIDDQ_HIWORD_MASK BIT(21)
> >> +#define GRF_EDP_PHY_SIDDQ_ON 0
> >> +#define GRF_EDP_PHY_SIDDQ_OFF BIT(5)
I would recommend aligning the value of of all macros in the same way.
> >> +
> >> +struct hsfreq_range {
> >> + u32 range_h;
The structure would be more compact if you turned this into a u16.
> >> + u8 cfg_bit;
> >> +};
> >> +
> >> +static const struct hsfreq_range rk3399_mipidphy_hsfreq_ranges[] = {
> >> + { 89, 0x00}, { 99, 0x10}, { 109, 0x20}, { 129, 0x01},
> >> + { 139, 0x11}, { 149, 0x21}, { 169, 0x02}, { 179, 0x12},
> >> + { 199, 0x22}, { 219, 0x03}, { 239, 0x13}, { 249, 0x23},
> >> + { 269, 0x04}, { 299, 0x14}, { 329, 0x05}, { 359, 0x15},
> >> + { 399, 0x25}, { 449, 0x06}, { 499, 0x16}, { 549, 0x07},
> >> + { 599, 0x17}, { 649, 0x08}, { 699, 0x18}, { 749, 0x09},
> >> + { 799, 0x19}, { 849, 0x29}, { 899, 0x39}, { 949, 0x0a},
> >> + { 999, 0x1a}, {1049, 0x2a}, {1099, 0x3a}, {1149, 0x0b},
> >> + {1199, 0x1b}, {1249, 0x2b}, {1299, 0x3b}, {1349, 0x0c},
> >> + {1399, 0x1c}, {1449, 0x2c}, {1500, 0x3c}
Maybe s/{/{ / and s/}/ }/ to give it a bit more air ? :-)
> >> +};
> >> +
> >> +static const char * const rk3399_mipidphy_clks[] = {
> >> + "dphy-ref",
> >> + "dphy-cfg",
> >> + "grf",
> >> +};
> >> +
> >> +enum dphy_reg_id {
> >> + GRF_DPHY_RX0_TURNDISABLE = 0,
> >> + GRF_DPHY_RX0_FORCERXMODE,
> >> + GRF_DPHY_RX0_FORCETXSTOPMODE,
> >> + GRF_DPHY_RX0_ENABLE,
> >> + GRF_DPHY_RX0_TESTCLR,
> >> + GRF_DPHY_RX0_TESTCLK,
> >> + GRF_DPHY_RX0_TESTEN,
> >> + GRF_DPHY_RX0_TESTDIN,
> >> + GRF_DPHY_RX0_TURNREQUEST,
> >> + GRF_DPHY_RX0_TESTDOUT,
> >> + GRF_DPHY_TX0_TURNDISABLE,
> >> + GRF_DPHY_TX0_FORCERXMODE,
> >> + GRF_DPHY_TX0_FORCETXSTOPMODE,
> >> + GRF_DPHY_TX0_TURNREQUEST,
> >> + GRF_DPHY_TX1RX1_TURNDISABLE,
> >> + GRF_DPHY_TX1RX1_FORCERXMODE,
> >> + GRF_DPHY_TX1RX1_FORCETXSTOPMODE,
> >> + GRF_DPHY_TX1RX1_ENABLE,
> >> + GRF_DPHY_TX1RX1_MASTERSLAVEZ,
> >> + GRF_DPHY_TX1RX1_BASEDIR,
> >> + GRF_DPHY_TX1RX1_ENABLECLK,
> >> + GRF_DPHY_TX1RX1_TURNREQUEST,
> >> + GRF_DPHY_RX1_SRC_SEL,
> >> + /* rk3288 only */
> >> + GRF_CON_DISABLE_ISP,
> >> + GRF_CON_ISP_DPHY_SEL,
> >> + GRF_DSI_CSI_TESTBUS_SEL,
> >> + GRF_DVP_V18SEL,
> >> + /* below is for rk3399 only */
> >> + GRF_DPHY_RX0_CLK_INV_SEL,
> >> + GRF_DPHY_RX1_CLK_INV_SEL,
> >> +};
> >> +
> >> +struct dphy_reg {
> >> + u32 offset;
> >> + u32 mask;
> >> + u32 shift;
The offset should hold in 16 bits and the mask and shift in 8 bits. That
would save space in the table below.
> >> +};
> >> +
> >> +#define PHY_REG(_offset, _width, _shift) \
> >> + { .offset = _offset, .mask = BIT(_width) - 1, .shift = _shift, }
> >> +
> >> +static const struct dphy_reg rk3399_grf_dphy_regs[] = {
> >> + [GRF_DPHY_RX0_TURNREQUEST] = PHY_REG(RK3399_GRF_SOC_CON9, 4, 0),
> >> + [GRF_DPHY_RX0_CLK_INV_SEL] = PHY_REG(RK3399_GRF_SOC_CON9, 1, 10),
> >> + [GRF_DPHY_RX1_CLK_INV_SEL] = PHY_REG(RK3399_GRF_SOC_CON9, 1, 11),
> >> + [GRF_DPHY_RX0_ENABLE] = PHY_REG(RK3399_GRF_SOC_CON21, 4, 0),
> >> + [GRF_DPHY_RX0_FORCERXMODE] = PHY_REG(RK3399_GRF_SOC_CON21, 4, 4),
> >> + [GRF_DPHY_RX0_FORCETXSTOPMODE] = PHY_REG(RK3399_GRF_SOC_CON21, 4, 8),
> >> + [GRF_DPHY_RX0_TURNDISABLE] = PHY_REG(RK3399_GRF_SOC_CON21, 4, 12),
> >> + [GRF_DPHY_TX0_FORCERXMODE] = PHY_REG(RK3399_GRF_SOC_CON22, 4, 0),
> >> + [GRF_DPHY_TX0_FORCETXSTOPMODE] = PHY_REG(RK3399_GRF_SOC_CON22, 4, 4),
> >> + [GRF_DPHY_TX0_TURNDISABLE] = PHY_REG(RK3399_GRF_SOC_CON22, 4, 8),
> >> + [GRF_DPHY_TX0_TURNREQUEST] = PHY_REG(RK3399_GRF_SOC_CON22, 4, 12),
> >> + [GRF_DPHY_TX1RX1_ENABLE] = PHY_REG(RK3399_GRF_SOC_CON23, 4, 0),
> >> + [GRF_DPHY_TX1RX1_FORCERXMODE] = PHY_REG(RK3399_GRF_SOC_CON23, 4, 4),
> >> + [GRF_DPHY_TX1RX1_FORCETXSTOPMODE] = PHY_REG(RK3399_GRF_SOC_CON23, 4, 8),
> >> + [GRF_DPHY_TX1RX1_TURNDISABLE] = PHY_REG(RK3399_GRF_SOC_CON23, 4, 12),
> >> + [GRF_DPHY_TX1RX1_TURNREQUEST] = PHY_REG(RK3399_GRF_SOC_CON24, 4, 0),
> >> + [GRF_DPHY_RX1_SRC_SEL] = PHY_REG(RK3399_GRF_SOC_CON24, 1, 4),
> >> + [GRF_DPHY_TX1RX1_BASEDIR] = PHY_REG(RK3399_GRF_SOC_CON24, 1, 5),
> >> + [GRF_DPHY_TX1RX1_ENABLECLK] = PHY_REG(RK3399_GRF_SOC_CON24, 1, 6),
> >> + [GRF_DPHY_TX1RX1_MASTERSLAVEZ] = PHY_REG(RK3399_GRF_SOC_CON24, 1, 7),
> >> + [GRF_DPHY_RX0_TESTDIN] = PHY_REG(RK3399_GRF_SOC_CON25, 8, 0),
> >> + [GRF_DPHY_RX0_TESTEN] = PHY_REG(RK3399_GRF_SOC_CON25, 1, 8),
> >> + [GRF_DPHY_RX0_TESTCLK] = PHY_REG(RK3399_GRF_SOC_CON25, 1, 9),
> >> + [GRF_DPHY_RX0_TESTCLR] = PHY_REG(RK3399_GRF_SOC_CON25, 1, 10),
> >> + [GRF_DPHY_RX0_TESTDOUT] = PHY_REG(RK3399_GRF_SOC_STATUS1, 8, 0),
The annoying part with such an indirection is that you can't really
write multiple fields in a single register with a single operation. Is
the register mapping completely different between the rk3288 and the
rk3399, or are the fields grouped in registers in a similar way ? In the
latter case we could possibly optimise it.
> >> +};
> >> +
> >> +struct dphy_drv_data {
> >> + const char * const *clks;
> >> + int num_clks;
This is never negative, you can make it an unsigned int.
> >> + const struct hsfreq_range *hsfreq_ranges;
> >> + int num_hsfreq_ranges;
Same here.
> >> + const struct dphy_reg *regs;
> >> +};
> >> +
> >> +struct rockchip_dphy {
> >> + struct device *dev;
> >> + struct regmap *grf;
> >> + const struct dphy_reg *grf_regs;
> >> + struct clk_bulk_data clks[MAX_DPHY_CLK];
> >> +
> >> + const struct dphy_drv_data *drv_data;
> >> + struct phy_configure_opts_mipi_dphy config;
> >> +};
> >> +
> >> +static inline void write_grf_reg(struct rockchip_dphy *priv,
> >> + int index, u8 value)
Maybe unsigned int index ?
> >> +{
> >> + const struct dphy_reg *reg = &priv->grf_regs[index];
> >> + unsigned int val = HIWORD_UPDATE(value, reg->mask, reg->shift);
> >> +
> >> + WARN_ON(!reg->offset);
> >> + regmap_write(priv->grf, reg->offset, val);
> >> +}
> >> +
> >> +static void mipidphy0_wr_reg(struct rockchip_dphy *priv,
> >> + u8 test_code, u8 test_data)
Function (and structure) names have different prefixes, would it make
sense to standardise them ? Maybe rockchip_dphy_ ? Or rk_dphy_ for a
shorter version ? This could become rk_dphy_write_dphy(), and the
previous function rk_dphy_write_grf().
> >> +{
> >> + /*
> >> + * With the falling edge on TESTCLK, the TESTDIN[7:0] signal content
> >> + * is latched internally as the current test code. Test data is
> >> + * programmed internally by rising edge on TESTCLK.
> >> + */
I've never understood why PHYs tend to have a register named TEST that
contains way more than test data :-)
> >> + write_grf_reg(priv, GRF_DPHY_RX0_TESTCLK, 1);
> >> + write_grf_reg(priv, GRF_DPHY_RX0_TESTDIN, test_code);
> >> + write_grf_reg(priv, GRF_DPHY_RX0_TESTEN, 1);
> >> + write_grf_reg(priv, GRF_DPHY_RX0_TESTCLK, 0);
> >> + write_grf_reg(priv, GRF_DPHY_RX0_TESTEN, 0);
> >> + write_grf_reg(priv, GRF_DPHY_RX0_TESTDIN, test_data);
> >> + write_grf_reg(priv, GRF_DPHY_RX0_TESTCLK, 1);
> >> +}
> >> +
> >> +/* should be move to power_on */
s/move/moved/
Do you mean merging the two functions together ? What prevents from
doing so ?
> >> +static int mipidphy_rx_stream_on(struct rockchip_dphy *priv)
> >> +{
> >> + const struct dphy_drv_data *drv_data = priv->drv_data;
> >> + const struct hsfreq_range *hsfreq_ranges = drv_data->hsfreq_ranges;
> >> + struct phy_configure_opts_mipi_dphy *config = &priv->config;
> >> + unsigned int i, hsfreq = 0, data_rate_mbps = config->hs_clk_rate;
> >> + int num_hsfreq_ranges = drv_data->num_hsfreq_ranges;
> >> +
> >> + do_div(data_rate_mbps, 1000 * 1000);
> >> +
> >> + dev_dbg(priv->dev, "%s: lanes %d - data_rate_mbps %u\n",
> >> + __func__, config->lanes, data_rate_mbps);
> >> +
> >> + for (i = 0; i < num_hsfreq_ranges; i++) {
> >> + if (hsfreq_ranges[i].range_h >= data_rate_mbps) {
> >> + hsfreq = hsfreq_ranges[i].cfg_bit;
> >> + break;
> >> + }
> >> + }
As num_hsfreq_ranges and hsfreq_ranges are only used in this loop, I
would remove the local variables.
> >> +
> >> + write_grf_reg(priv, GRF_DPHY_RX0_FORCERXMODE, 0);
> >> + write_grf_reg(priv, GRF_DPHY_RX0_FORCETXSTOPMODE, 0);
> >> +
> >> + /* Disable lan turn around, which is ignored in receive mode */
Is it "lan turn around", or "lane turn around" ?
> >> + write_grf_reg(priv, GRF_DPHY_RX0_TURNREQUEST, 0);
> >> + write_grf_reg(priv, GRF_DPHY_RX0_TURNDISABLE, 0xf);
> >> +
> >> + write_grf_reg(priv, GRF_DPHY_RX0_ENABLE, GENMASK(config->lanes - 1, 0));
> >> +
> >> + /* dphy start */
> >> + write_grf_reg(priv, GRF_DPHY_RX0_TESTCLK, 1);
> >> + write_grf_reg(priv, GRF_DPHY_RX0_TESTCLR, 1);
> >> + usleep_range(100, 150);
> >> + write_grf_reg(priv, GRF_DPHY_RX0_TESTCLR, 0);
> >> + usleep_range(100, 150);
> >> +
> >> + /* set clock lane */
> >> + /* HS hsfreq_range & lane 0 settle bypass */
> >> + mipidphy0_wr_reg(priv, CLOCK_LANE_HS_RX_CONTROL, 0);
> >> + /* HS RX Control of lane0 */
> >> + mipidphy0_wr_reg(priv, LANE0_HS_RX_CONTROL, hsfreq << 1);
> >> + /* HS RX Control of lane1 */
> >> + mipidphy0_wr_reg(priv, LANE1_HS_RX_CONTROL, 0);
> >> + /* HS RX Control of lane2 */
> >> + mipidphy0_wr_reg(priv, LANE2_HS_RX_CONTROL, 0);
> >> + /* HS RX Control of lane3 */
> >> + mipidphy0_wr_reg(priv, LANE3_HS_RX_CONTROL, 0);
Does this hardcode usage of a single lane ?
> >> + /* HS RX Data Lanes Settle State Time Control */
> >> + mipidphy0_wr_reg(priv, HS_RX_DATA_LANES_THS_SETTLE_CONTROL,
> >> + THS_SETTLE_COUNTER_THRESHOLD);
> >> +
> >> + /* Normal operation */
> >> + mipidphy0_wr_reg(priv, 0x0, 0);
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static int rockchip_dphy_configure(struct phy *phy, union phy_configure_opts *opts)
> >> +{
> >> + struct rockchip_dphy *priv = phy_get_drvdata(phy);
> >> + int ret;
> >> +
> >> + /* pass with phy_mipi_dphy_get_default_config (with pixel rate?) */
I'm not sure to understand what this means.
> >> + ret = phy_mipi_dphy_config_validate(&opts->mipi_dphy);
> >> + if (ret)
> >> + return ret;
> >> +
> >> + memcpy(&priv->config, opts, sizeof(priv->config));
> >
> > You could to:
> >
> > priv->config = *opts;
> >
> > Up to you. Some people like memcpy(). :-)
>
> your way is better thanks!
>
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static int rockchip_dphy_power_on(struct phy *phy)
> >> +{
> >> + struct rockchip_dphy *priv = phy_get_drvdata(phy);
> >> + int ret;
> >> +
> >> + ret = clk_bulk_enable(priv->drv_data->num_clks, priv->clks);
> >> + if (ret)
> >> + return ret;
> >> +
> >> + return mipidphy_rx_stream_on(priv);
Should you call clk_bulk_disable() if mipidphy_rx_stream_on() fails ?
Actually that function never fails, so I'd make it a void function, and
return 0 here.
What happens if the clock rate is higher than the maximum supported by
the PHY ? Shouldn't rockchip_dphy_configure() fail in that case ?
> >> +}
> >> +
> >> +static int rockchip_dphy_power_off(struct phy *phy)
> >> +{
> >> + struct rockchip_dphy *priv = phy_get_drvdata(phy);
> >> +
No need to write any register ? That's scary, what will happen on the
next power on, when the clocks gets enabled ?
> >> + clk_bulk_disable(priv->drv_data->num_clks, priv->clks);
> >> + return 0;
> >> +}
> >> +
> >> +static int rockchip_dphy_init(struct phy *phy)
> >> +{
> >> + struct rockchip_dphy *priv = phy_get_drvdata(phy);
> >> + int ret;
> >> +
> >> + ret = clk_bulk_prepare(priv->drv_data->num_clks, priv->clks);
> >
> > return ...;
> >
> >> + if (ret)
> >> + return ret;
> >> + return 0;
> >> +}
> >> +
> >> +static int rockchip_dphy_exit(struct phy *phy)
> >> +{
> >> + struct rockchip_dphy *priv = phy_get_drvdata(phy);
> >> +
> >> + clk_bulk_unprepare(priv->drv_data->num_clks, priv->clks);
> >> + return 0;
> >> +}
> >> +
> >> +static const struct phy_ops rockchip_dphy_ops = {
> >> + .power_on = rockchip_dphy_power_on,
> >> + .power_off = rockchip_dphy_power_off,
> >> + .init = rockchip_dphy_init,
> >> + .exit = rockchip_dphy_exit,
> >> + .configure = rockchip_dphy_configure,
> >> + .owner = THIS_MODULE,
> >> +};
> >> +
> >> +static const struct dphy_drv_data rk3399_mipidphy_drv_data = {
> >> + .clks = rk3399_mipidphy_clks,
> >> + .num_clks = ARRAY_SIZE(rk3399_mipidphy_clks),
> >> + .hsfreq_ranges = rk3399_mipidphy_hsfreq_ranges,
> >> + .num_hsfreq_ranges = ARRAY_SIZE(rk3399_mipidphy_hsfreq_ranges),
> >> + .regs = rk3399_grf_dphy_regs,
> >
> > Do you expect to support more of the similar PHY(s) --- are there such? If
> > not, you could put these in the code that uses them.
>
> Yes, for rk3288 in the future.
>
> >> +};
> >> +
> >> +static const struct of_device_id rockchip_dphy_dt_ids[] = {
> >> + {
> >> + .compatible = "rockchip,rk3399-mipi-dphy",
> >> + .data = &rk3399_mipidphy_drv_data,
> >> + },
> >> + {}
> >> +};
> >> +MODULE_DEVICE_TABLE(of, rockchip_dphy_dt_ids);
> >> +
> >> +static int rockchip_dphy_probe(struct platform_device *pdev)
> >> +{
> >> + struct device *dev = &pdev->dev;
> >> + struct device_node *np = dev->of_node;
> >> + const struct dphy_drv_data *drv_data;
> >> + struct phy_provider *phy_provider;
> >> + const struct of_device_id *of_id;
> >> + struct rockchip_dphy *priv;
> >> + struct regmap *grf;
> >> + struct phy *phy;
> >> + unsigned int i;
> >> + int ret;
> >> +
> >> + if (!dev->parent || !dev->parent->of_node)
> >> + return -ENODEV;
> >> +
> >> + if (platform_get_resource(pdev, IORESOURCE_MEM, 0)) {
> >> + dev_err(&pdev->dev, "Rockchip DPHY driver only suports rx\n");
You can replace pdev->dev with dev here and below.
s/rx/RX mode/ ?
> >> + return -EINVAL;
> >> + }
> >> +
> >> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> >> + if (!priv)
> >> + return -ENOMEM;
> >> + priv->dev = dev;
> >> +
> >> + grf = syscon_node_to_regmap(dev->parent->of_node);
> >> + if (IS_ERR(grf)) {
> >> + grf = syscon_regmap_lookup_by_phandle(dev->of_node,
> >> + "rockchip,grf");
> >> + if (IS_ERR(grf)) {
> >> + dev_err(dev, "Can't find GRF syscon\n");
> >> + return -ENODEV;
> >> + }
> >> + }
> >> + priv->grf = grf;
> >> +
> >> + of_id = of_match_device(rockchip_dphy_dt_ids, dev);
> >> + if (!of_id)
> >> + return -EINVAL;
> >> +
> >> + drv_data = of_id->data;
> >> + priv->grf_regs = drv_data->regs;
Do you have to store grf_regs in priv, or could it be accessed through
priv->drv_data->regs ?
> >> + priv->drv_data = drv_data;
> >> + for (i = 0; i < drv_data->num_clks; i++)
> >> + priv->clks[i].id = drv_data->clks[i];
> >> + ret = devm_clk_bulk_get(&pdev->dev, drv_data->num_clks, priv->clks);
> >> + if (ret)
> >> + return ret;
> >> +
> >> + phy = devm_phy_create(dev, np, &rockchip_dphy_ops);
> >> + if (IS_ERR(phy)) {
> >> + dev_err(dev, "failed to create phy\n");
> >> + return PTR_ERR(phy);
> >> + }
> >> + phy_set_drvdata(phy, priv);
> >> +
> >> + phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> >> +
> >> + return PTR_ERR_OR_ZERO(phy_provider);
> >> +}
> >> +
> >> +static struct platform_driver rockchip_dphy_driver = {
> >> + .probe = rockchip_dphy_probe,
> >> + .driver = {
> >> + .name = "rockchip-mipi-dphy",
> >> + .of_match_table = rockchip_dphy_dt_ids,
> >> + },
> >> +};
> >> +module_platform_driver(rockchip_dphy_driver);
> >> +
> >> +MODULE_AUTHOR("Ezequiel Garcia <ezequiel@collabora.com>");
> >> +MODULE_DESCRIPTION("Rockchip MIPI Synopsys DPHY driver");
> >> +MODULE_LICENSE("Dual MIT/GPL");
Overall this is quite good, there are only small issues.
--
Regards,
Laurent Pinchart
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 0/6] drm+dma: cache support for arm, etc
From: Christoph Hellwig @ 2019-08-15 17:53 UTC (permalink / raw)
To: Rob Clark
Cc: Kate Stewart, Masayoshi Mizuma, Maciej W. Rozycki, Eric Biggers,
Catalin Marinas, Imre Deak, dri-devel, Chris Wilson,
Masahiro Yamada, Benjamin Gaignard, Mauro Carvalho Chehab,
Will Deacon, Christoph Hellwig, Emil Velikov, Deepak Sharma,
Michael Ellerman, Paul Burton, Mike Rapoport, Geert Uytterhoeven,
moderated list:ARM64 PORT (AARCH64 ARCHITECTURE), Daniel Vetter,
open list:MIPS, Linus Walleij, Robin Murphy,
open list:DRM DRIVER FOR MSM ADRENO GPU, Joerg Roedel,
Arnd Bergmann, Anshuman Khandual, Hauke Mehrtens,
Jesper Dangaard Brouer, Wolfram Sang (Renesas),
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT), Alexios Zavras,
Russell King, Doug Anderson, Thomas Gleixner, Sean Paul,
Allison Randal, Christophe Leroy, Enrico Weigelt, Ard Biesheuvel,
Greg Kroah-Hartman, open list, Rob Clark, Souptick Joarder,
Andrew Morton, open list:DRM DRIVER FOR MSM ADRENO GPU,
christian.koenig
In-Reply-To: <CAJs_Fx4bS64s7+xQqsead3N80ZQpofqegFQu+tT=b3wcGd_2pA@mail.gmail.com>
On Thu, Aug 15, 2019 at 06:54:39AM -0700, Rob Clark wrote:
> On Wed, Aug 14, 2019 at 11:51 PM Christoph Hellwig <hch@lst.de> wrote:
> >
> > As said before I don't think these low-level helpers are the
> > right API to export, but even if they did you'd just cover a tiny
> > subset of the architectures.
>
> Are you thinking instead something like:
>
> void dma_sync_sg_for_{cpu,device}(struct device *dev, struct scatterlist *sgl,
> int nents, enum dma_data_direction dir)
> {
> for_each_sg(sgl, sg, nents, i) {
> arch_sync_dma_for_..(dev, sg_phys(sg), sg->length, dir);
> }
> }
> EXPORT_SYMBOL_GPL(dma_sync_sg_for_..)
>
> or did you have something else in mind?
No. We really need an interface thay says please give me uncached
memory (for some definition of uncached that includes that grapics
drivers call write combine), and then let the architecture do the right
thing. Basically dma_alloc_coherent with DMA_ATTR_NO_KERNEL_MAPPING
is superficially close to what you want, except that the way the drm
drivers work you can't actually use it.
The reason for that is if we can we really need to not create another
uncachable alias, but instead change the page attributes in place.
On x86 we can and must do that for example, and based on the
conversation with Will arm64 could do that fairly easily. arm32 can
right now only do that for CMA, though.
The big question is what API do we want. I had a pretty similar
discussion with Christian on doing such an allocation for amdgpu,
where the device normally is cache coherent, but they actually want
to turn it into non-coherent by using PCIe unsnooped transactions.
Here is my high level plan, which still has a few lose end:
(1) provide a new API:
struct page *dma_alloc_pages(struct device *dev, unsigned nr_pages,
gfp_t gfp, unsigned long flags);
void dma_free_pages(struct device *dev, unsigned nr_pages,
unsigned long flags);
These give you back page backed memory that is guaranteed to be
addressable by the device (no swiotlb or similar). The memory can
then be mapped using dma_map*, including unmap and dma_sync to
bounce ownership around. This is the replacement for the current
dma_alloc_attrs with DMA_ATTR_NON_CONSISTENT API, that is rather
badly defined.
(2) Add support for DMA_ATTR_NO_KERNEL_MAPPING to this new API instead
of dma_alloc_attrs. The initial difference with that flag is just
that we allow highmem, but in the future we could also unmap this
memory from the kernel linear mapping entirely on architectures
where we can easily do that.
(3) Add a dma_pages_map/dma_pages_unmap or similar API that allows you
to get a kernel mapping for parts or all of a
DMA_ATTR_NO_KERNEL_MAPPING allocation. This is to replace things
like your open-coded vmap in msm (or similarly elsewhere in dma-buf
providers).
(4) Add support for a DMA_ATTR_UNCACHABLE flags (or similar) to the new
API, that maps the pages as uncachable iff they have a kernel
mapping, including invalidating the caches at time of this page
attribute change (or creation of a new mapping). This API will fail
if the architecture does not allow in-place remapping. Note that for
arm32 we could always dip into the CMA pool if one is present to not
fail. We'll also need some helper to map from the DMA_ATTR_* flags
to a pgprot for mapping the page to userspace. There is also a few
other weird bits here, e.g. on architectures like mips that use an
uncached segment we'll have to fail use with the plain
DMA_ATTR_UNCACHABLE flag, but it could be supported with
DMA_ATTR_UNCACHABLE | DMA_ATTR_NO_KERNEL_MAPPING.
I was hoping to get most of this done for this merge window, but I'm
probably lucky if I get at least parts done. Too much distraction.
> Hmm, not entirely sure why.. you should be on the cc list for each
> individual patch.
They finally made it, although even with the delay they only ended up
in the spam mailbox. I still can't see them on the various mailing
lists.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2] coresight: tmc-etr: Fix perf_data check.
From: Mathieu Poirier @ 2019-08-15 17:37 UTC (permalink / raw)
To: Yabin Cui
Cc: Alexander Shishkin, Linux Kernel Mailing List, linux-arm-kernel,
Suzuki K Poulose
In-Reply-To: <20190812190320.209988-1-yabinc@google.com>
On Mon, 12 Aug 2019 at 13:03, Yabin Cui <yabinc@google.com> wrote:
>
> When tracing etm data of multiple threads on multiple cpus through
> perf interface, each cpu has a unique etr_perf_buffer while sharing
> the same etr device. There is no guarantee that the last cpu starts
> etm tracing also stops last. This makes perf_data check fail.
>
> Fix it by checking etr_buf instead of etr_perf_buffer.
>
> Fixes: 3147da92a8a8 ("coresight: tmc-etr: Allocate and free ETR memory buffers for CPU-wide scenarios")
> Signed-off-by: Yabin Cui <yabinc@google.com>
> ---
>
> v1 -> v2: rename perf_data to perf_buf. Add fixes tag.
>
> ---
> drivers/hwtracing/coresight/coresight-tmc-etr.c | 6 +++---
> drivers/hwtracing/coresight/coresight-tmc.h | 6 +++---
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> index 17006705287a..90d1548ad268 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> @@ -1484,7 +1484,7 @@ tmc_update_etr_buffer(struct coresight_device *csdev,
> goto out;
> }
>
> - if (WARN_ON(drvdata->perf_data != etr_perf)) {
> + if (WARN_ON(drvdata->perf_buf != etr_buf)) {
> lost = true;
> spin_unlock_irqrestore(&drvdata->spinlock, flags);
> goto out;
> @@ -1497,7 +1497,7 @@ tmc_update_etr_buffer(struct coresight_device *csdev,
>
> CS_LOCK(drvdata->base);
> /* Reset perf specific data */
> - drvdata->perf_data = NULL;
> + drvdata->perf_buf = NULL;
> spin_unlock_irqrestore(&drvdata->spinlock, flags);
>
> size = etr_buf->len;
> @@ -1556,7 +1556,7 @@ static int tmc_enable_etr_sink_perf(struct coresight_device *csdev, void *data)
> }
>
> etr_perf->head = PERF_IDX2OFF(handle->head, etr_perf);
> - drvdata->perf_data = etr_perf;
> + drvdata->perf_buf = etr_perf->etr_buf;
Ok for the fix. Looking a things again I don't see a need to do the
assignment for each event - this needs to be done only when the device
is assocated with a monitored process. Please move it here [1].
Thanks,
Mathieu
[1]. https://elixir.bootlin.com/linux/v5.3-rc4/source/drivers/hwtracing/coresight/coresight-tmc-etr.c#L1572
>
> /*
> * No HW configuration is needed if the sink is already in
> diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
> index 1ed50411cc3c..f9a0c95e9ba2 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc.h
> +++ b/drivers/hwtracing/coresight/coresight-tmc.h
> @@ -178,8 +178,8 @@ struct etr_buf {
> * device configuration register (DEVID)
> * @idr: Holds etr_bufs allocated for this ETR.
> * @idr_mutex: Access serialisation for idr.
> - * @perf_data: PERF buffer for ETR.
> - * @sysfs_data: SYSFS buffer for ETR.
> + * @sysfs_buf: SYSFS buffer for ETR.
> + * @perf_buf: PERF buffer for ETR.
> */
> struct tmc_drvdata {
> void __iomem *base;
> @@ -202,7 +202,7 @@ struct tmc_drvdata {
> struct idr idr;
> struct mutex idr_mutex;
> struct etr_buf *sysfs_buf;
> - void *perf_data;
> + struct etr_buf *perf_buf;
> };
>
> struct etr_buf_operations {
> --
> 2.23.0.rc1.153.gdeed80330f-goog
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2] coresight: tmc-etr: Fix perf_data check.
From: Mathieu Poirier @ 2019-08-15 17:22 UTC (permalink / raw)
To: Yabin Cui
Cc: Alexander Shishkin, Linux Kernel Mailing List, linux-arm-kernel,
Suzuki K Poulose
In-Reply-To: <20190814235058.184204-1-yabinc@google.com>
On Wed, 14 Aug 2019 at 17:51, Yabin Cui <yabinc@google.com> wrote:
>
> > Did you actually see the check fail or is this a theoretical thing?
> > I'm really perplex here has I have tested this scenario many times
> > without issues.
> >
> I have seen this warning in dmesg output, that's how I find the problem.
>
> > In CPU wide scenarios each perf event (one per CPU) is associated with
> > an event_data during the setup process. The event_data is the
> > etr_perf holding a reference to the perf ring buffer for that specific
> > event along with the etr_buf, regardless of who created the latter.
>
> Agree.
>
> > From there, when the event is installed on a CPU, the csdev for that
> > CPU is given a reference to the event_data of that event[1]. Before
> > going further notice how there is a per CPU csdev and event handle to
> > keep track of event specifics[2]. As such both (per CPU) csdev and
> > event handle carry the exact same reference to the etr_perf.
> >
> On my test device (Pixel 3), there is an ETM device on each cpu, but only
> one ETR device for the whole device. So there is only one instance of etr
> csdev in the kernel. If multiple cpus are scheduling on etm perf events at
> the same time, all of them are trying to set their event_data to the same
> etr csdev. And different perf events have different event_data. A warning
> situation is as below:
>
> cpu 0
> schedule on event A (set etr csdev->perf_data to event_a.etr_perf)
>
> cpu 1
> schedule on event B (set etr csdev->perf_data to event_b.etr_perf)
>
You are 100% right and looking at it again this morning it just jumped
at me. I simply can't understand how it did not manifest itself
during all the hammering I did on it.
Please see details in my other (and upcoming) email.
Thanks,
Mathieu
> cpu 1
> schedule off event B (update buffer, does nothing since csdev->refcnt != 1)
>
> cpu 0
> schedule off event A (update buffer, but etr csdev->perf_data check fail)
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v1 0/8] arm64: MMU enabled kexec relocation
From: Pavel Tatashin @ 2019-08-15 17:16 UTC (permalink / raw)
To: Pavel Tatashin, James Morris, Sasha Levin, Eric W. Biederman,
kexec mailing list, LKML, Jonathan Corbet, Catalin Marinas, will,
Linux ARM, Marc Zyngier, James Morse, Vladimir Murzin,
Matthias Brugger, Bhupesh Sharma, linux-mm
In-Reply-To: <CA+CK2bADiBMEx9cJuXT5fQkBYFZAtxUtc7ZzjrNfEjijPZkPtw@mail.gmail.com>
Hi,
It is been two weeks, and no review activity yet. Please help with
reviewing this work.
Thank you,
Pasha
On Thu, Aug 8, 2019 at 2:44 PM Pavel Tatashin <pasha.tatashin@soleen.com> wrote:
>
> Just a friendly reminder, please send your comments on this series.
> It's been a week since I sent out these patches, and no feedback yet.
> Also, I'd appreciate if anyone could test this series on vhe hardware
> with vhe kernel, it does not look like QEMU can emulate it yet
>
> Thank you,
> Pasha
>
> On Thu, Aug 1, 2019 at 11:24 AM Pavel Tatashin
> <pasha.tatashin@soleen.com> wrote:
> >
> > Enable MMU during kexec relocation in order to improve reboot performance.
> >
> > If kexec functionality is used for a fast system update, with a minimal
> > downtime, the relocation of kernel + initramfs takes a significant portion
> > of reboot.
> >
> > The reason for slow relocation is because it is done without MMU, and thus
> > not benefiting from D-Cache.
> >
> > Performance data
> > ----------------
> > For this experiment, the size of kernel plus initramfs is small, only 25M.
> > If initramfs was larger, than the improvements would be greater, as time
> > spent in relocation is proportional to the size of relocation.
> >
> > Previously:
> > kernel shutdown 0.022131328s
> > relocation 0.440510736s
> > kernel startup 0.294706768s
> >
> > Relocation was taking: 58.2% of reboot time
> >
> > Now:
> > kernel shutdown 0.032066576s
> > relocation 0.022158152s
> > kernel startup 0.296055880s
> >
> > Now: Relocation takes 6.3% of reboot time
> >
> > Total reboot is x2.16 times faster.
> >
> > Previous approaches and discussions
> > -----------------------------------
> > https://lore.kernel.org/lkml/20190709182014.16052-1-pasha.tatashin@soleen.com
> > reserve space for kexec to avoid relocation, involves changes to generic code
> > to optimize a problem that exists on arm64 only:
> >
> > https://lore.kernel.org/lkml/20190716165641.6990-1-pasha.tatashin@soleen.com
> > The first attempt to enable MMU, some bugs that prevented performance
> > improvement. The page tables unnecessary configured idmap for the whole
> > physical space.
> >
> > https://lore.kernel.org/lkml/20190731153857.4045-1-pasha.tatashin@soleen.com
> > No linear copy, bug with EL2 reboots.
> >
> > Pavel Tatashin (8):
> > kexec: quiet down kexec reboot
> > arm64, mm: transitional tables
> > arm64: hibernate: switch to transtional page tables.
> > kexec: add machine_kexec_post_load()
> > arm64, kexec: move relocation function setup and clean up
> > arm64, kexec: add expandable argument to relocation function
> > arm64, kexec: configure transitional page table for kexec
> > arm64, kexec: enable MMU during kexec relocation
> >
> > arch/arm64/Kconfig | 4 +
> > arch/arm64/include/asm/kexec.h | 51 ++++-
> > arch/arm64/include/asm/pgtable-hwdef.h | 1 +
> > arch/arm64/include/asm/trans_table.h | 68 ++++++
> > arch/arm64/kernel/asm-offsets.c | 14 ++
> > arch/arm64/kernel/cpu-reset.S | 4 +-
> > arch/arm64/kernel/cpu-reset.h | 8 +-
> > arch/arm64/kernel/hibernate.c | 261 ++++++-----------------
> > arch/arm64/kernel/machine_kexec.c | 199 ++++++++++++++----
> > arch/arm64/kernel/relocate_kernel.S | 196 +++++++++---------
> > arch/arm64/mm/Makefile | 1 +
> > arch/arm64/mm/trans_table.c | 273 +++++++++++++++++++++++++
> > kernel/kexec.c | 4 +
> > kernel/kexec_core.c | 8 +-
> > kernel/kexec_file.c | 4 +
> > kernel/kexec_internal.h | 2 +
> > 16 files changed, 758 insertions(+), 340 deletions(-)
> > create mode 100644 arch/arm64/include/asm/trans_table.h
> > create mode 100644 arch/arm64/mm/trans_table.c
> >
> > --
> > 2.22.0
> >
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Applied "ASoC: mt2701: remove unused variables" to the asoc tree
From: Mark Brown @ 2019-08-15 17:14 UTC (permalink / raw)
To: YueHaibing
Cc: alsa-devel, perex, tiwai, lgirdwood, linux-kernel, Hulk Robot,
Mark Brown, linux-mediatek, matthias.bgg, linux-arm-kernel
In-Reply-To: <20190813143811.31456-1-yuehaibing@huawei.com>
The patch
ASoC: mt2701: remove unused variables
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.4
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
From a9e792d006edbd33724f2eb858887d3b591d82c5 Mon Sep 17 00:00:00 2001
From: YueHaibing <yuehaibing@huawei.com>
Date: Tue, 13 Aug 2019 22:38:11 +0800
Subject: [PATCH] ASoC: mt2701: remove unused variables
sound/soc/mediatek/mt2701/mt2701-afe-pcm.c:799:38: warning:
mt2701_afe_o23_mix defined but not used [-Wunused-const-variable=]
sound/soc/mediatek/mt2701/mt2701-afe-pcm.c:803:38: warning:
mt2701_afe_o24_mix defined but not used [-Wunused-const-variable=]
sound/soc/mediatek/mt2701/mt2701-afe-pcm.c:835:38: warning:
mt2701_afe_multi_ch_out_i2s4 defined but not used [-Wunused-const-variable=]
They are never used, so can be removed.
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Link: https://lore.kernel.org/r/20190813143811.31456-1-yuehaibing@huawei.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
sound/soc/mediatek/mt2701/mt2701-afe-pcm.c | 13 -------------
1 file changed, 13 deletions(-)
diff --git a/sound/soc/mediatek/mt2701/mt2701-afe-pcm.c b/sound/soc/mediatek/mt2701/mt2701-afe-pcm.c
index 9af76ae315a5..d7f5defa50c2 100644
--- a/sound/soc/mediatek/mt2701/mt2701-afe-pcm.c
+++ b/sound/soc/mediatek/mt2701/mt2701-afe-pcm.c
@@ -796,14 +796,6 @@ static const struct snd_kcontrol_new mt2701_afe_o22_mix[] = {
SOC_DAPM_SINGLE_AUTODISABLE("I19 Switch", AFE_CONN22, 19, 1, 0),
};
-static const struct snd_kcontrol_new mt2701_afe_o23_mix[] = {
- SOC_DAPM_SINGLE_AUTODISABLE("I20 Switch", AFE_CONN23, 20, 1, 0),
-};
-
-static const struct snd_kcontrol_new mt2701_afe_o24_mix[] = {
- SOC_DAPM_SINGLE_AUTODISABLE("I21 Switch", AFE_CONN24, 21, 1, 0),
-};
-
static const struct snd_kcontrol_new mt2701_afe_o31_mix[] = {
SOC_DAPM_SINGLE_AUTODISABLE("I35 Switch", AFE_CONN41, 9, 1, 0),
};
@@ -832,11 +824,6 @@ static const struct snd_kcontrol_new mt2701_afe_multi_ch_out_i2s3[] = {
PWR2_TOP_CON, 18, 1, 0),
};
-static const struct snd_kcontrol_new mt2701_afe_multi_ch_out_i2s4[] = {
- SOC_DAPM_SINGLE_AUTODISABLE("Multich I2S4 Out Switch",
- PWR2_TOP_CON, 19, 1, 0),
-};
-
static const struct snd_soc_dapm_widget mt2701_afe_pcm_widgets[] = {
/* inter-connections */
SND_SOC_DAPM_MIXER("I00", SND_SOC_NOPM, 0, 0, NULL, 0),
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Applied "ASoC: rockchip: rockchip_max98090: Set period size to 240" to the asoc tree
From: Mark Brown @ 2019-08-15 17:14 UTC (permalink / raw)
To: Cheng-Yi Chiang
Cc: alsa-devel, tzungbi, Heiko Stuebner, zhengxing, Liam Girdwood,
linux-kernel, jeffy.chen, cain.cai, dianders, linux-rockchip,
Mark Brown, eddie.cai, Takashi Iwai, enric.balletbo, dgreid,
Jaroslav Kysela, linux-arm-kernel
In-Reply-To: <20190813074430.191791-1-cychiang@chromium.org>
The patch
ASoC: rockchip: rockchip_max98090: Set period size to 240
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.3
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
From 7188f656cdf762d4ea8ce16b6aaf4c6b06e119ec Mon Sep 17 00:00:00 2001
From: Cheng-Yi Chiang <cychiang@chromium.org>
Date: Tue, 13 Aug 2019 15:44:30 +0800
Subject: [PATCH] ASoC: rockchip: rockchip_max98090: Set period size to 240
From stress testing of arecord, we found that period size
greater than ~900 will bring pl330 to DYING state and
can not recover within 100 iterations.
The result is that arecord will stuck and get I/O error,
and issue can not be recovered until reboot.
This issue does not happen when period size is small.
Set constraint of period size to 240 to prevent such issue.
With the constraint, there will be no issue after 2000 iterations.
We can revert this patch once the root cause is found
in rockchip's pl330 implementation.
Signed-off-by: Cheng-Yi Chiang <cychiang@chromium.org>
Link: https://lore.kernel.org/r/20190813074430.191791-1-cychiang@chromium.org
Signed-off-by: Mark Brown <broonie@kernel.org>
---
sound/soc/rockchip/rockchip_max98090.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/sound/soc/rockchip/rockchip_max98090.c b/sound/soc/rockchip/rockchip_max98090.c
index 782e534d4c0d..d54f672d38d8 100644
--- a/sound/soc/rockchip/rockchip_max98090.c
+++ b/sound/soc/rockchip/rockchip_max98090.c
@@ -138,8 +138,19 @@ static int rk_aif1_hw_params(struct snd_pcm_substream *substream,
return ret;
}
+static int rk_aif1_startup(struct snd_pcm_substream *substream)
+{
+ /*
+ * Set period size to 240 because pl330 has issue
+ * dealing with larger period in stress testing.
+ */
+ return snd_pcm_hw_constraint_minmax(substream->runtime,
+ SNDRV_PCM_HW_PARAM_PERIOD_SIZE, 240, 240);
+}
+
static const struct snd_soc_ops rk_aif1_ops = {
.hw_params = rk_aif1_hw_params,
+ .startup = rk_aif1_startup,
};
SND_SOC_DAILINK_DEFS(hifi,
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Applied "ASoC: mediatek: mt8183-mt6358-ts3a227-max98357: remove unused variables" to the asoc tree
From: Mark Brown @ 2019-08-15 17:14 UTC (permalink / raw)
To: YueHaibing
Cc: alsa-devel, perex, tiwai, lgirdwood, linux-kernel, Hulk Robot,
Mark Brown, linux-mediatek, matthias.bgg, linux-arm-kernel
In-Reply-To: <20190813144122.67676-1-yuehaibing@huawei.com>
The patch
ASoC: mediatek: mt8183-mt6358-ts3a227-max98357: remove unused variables
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.4
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
From d59170b42610c7cbc6e96431ca8357a8bdbf592b Mon Sep 17 00:00:00 2001
From: YueHaibing <yuehaibing@huawei.com>
Date: Tue, 13 Aug 2019 22:41:22 +0800
Subject: [PATCH] ASoC: mediatek: mt8183-mt6358-ts3a227-max98357: remove unused
variables
sound/soc/mediatek/mt8183/mt8183-mt6358-ts3a227-max98357.c:50:1: warning:
mt8183_mt6358_ts3a227_max98357_dapm_widgets defined but not used [-Wunused-const-variable=]
sound/soc/mediatek/mt8183/mt8183-mt6358-ts3a227-max98357.c:55:1: warning:
mt8183_mt6358_ts3a227_max98357_dapm_routes defined but not used [-Wunused-const-variable=]
They are never used, so can be removed.
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Link: https://lore.kernel.org/r/20190813144122.67676-1-yuehaibing@huawei.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
.../mediatek/mt8183/mt8183-mt6358-ts3a227-max98357.c | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/sound/soc/mediatek/mt8183/mt8183-mt6358-ts3a227-max98357.c b/sound/soc/mediatek/mt8183/mt8183-mt6358-ts3a227-max98357.c
index 53f54078f78c..272766c1b859 100644
--- a/sound/soc/mediatek/mt8183/mt8183-mt6358-ts3a227-max98357.c
+++ b/sound/soc/mediatek/mt8183/mt8183-mt6358-ts3a227-max98357.c
@@ -46,16 +46,6 @@ static int mt8183_i2s_hw_params_fixup(struct snd_soc_pcm_runtime *rtd,
return 0;
}
-static const struct snd_soc_dapm_widget
-mt8183_mt6358_ts3a227_max98357_dapm_widgets[] = {
- SND_SOC_DAPM_OUTPUT("IT6505_8CH"),
-};
-
-static const struct snd_soc_dapm_route
-mt8183_mt6358_ts3a227_max98357_dapm_routes[] = {
- {"IT6505_8CH", NULL, "TDM"},
-};
-
static int
mt8183_mt6358_ts3a227_max98357_bt_sco_startup(
struct snd_pcm_substream *substream)
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Applied "ASoC: mediatek: mt8183-da7219-max98357: remove unused variable" to the asoc tree
From: Mark Brown @ 2019-08-15 17:14 UTC (permalink / raw)
To: YueHaibing
Cc: alsa-devel, perex, tiwai, lgirdwood, linux-kernel, Hulk Robot,
Mark Brown, linux-mediatek, matthias.bgg, linux-arm-kernel
In-Reply-To: <20190813143952.29232-1-yuehaibing@huawei.com>
The patch
ASoC: mediatek: mt8183-da7219-max98357: remove unused variable
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.4
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
From 57c3ed42f52cdc51f416c93b19708ef6ceb4a00b Mon Sep 17 00:00:00 2001
From: YueHaibing <yuehaibing@huawei.com>
Date: Tue, 13 Aug 2019 22:39:52 +0800
Subject: [PATCH] ASoC: mediatek: mt8183-da7219-max98357: remove unused
variable
sound/soc/mediatek/mt8183/mt8183-da7219-max98357.c:120:1: warning:
mt8183_da7219_max98357_dapm_widgets defined but not used [-Wunused-const-variable=]
sound/soc/mediatek/mt8183/mt8183-da7219-max98357.c:124:40: warning:
mt8183_da7219_max98357_dapm_routes defined but not used [-Wunused-const-variable=]
They are never used, so can be removed.
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Link: https://lore.kernel.org/r/20190813143952.29232-1-yuehaibing@huawei.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
sound/soc/mediatek/mt8183/mt8183-da7219-max98357.c | 9 ---------
1 file changed, 9 deletions(-)
diff --git a/sound/soc/mediatek/mt8183/mt8183-da7219-max98357.c b/sound/soc/mediatek/mt8183/mt8183-da7219-max98357.c
index 2a6097174614..43f99e59a078 100644
--- a/sound/soc/mediatek/mt8183/mt8183-da7219-max98357.c
+++ b/sound/soc/mediatek/mt8183/mt8183-da7219-max98357.c
@@ -116,15 +116,6 @@ static int mt8183_i2s_hw_params_fixup(struct snd_soc_pcm_runtime *rtd,
return 0;
}
-static const struct snd_soc_dapm_widget
-mt8183_da7219_max98357_dapm_widgets[] = {
- SND_SOC_DAPM_OUTPUT("IT6505_8CH"),
-};
-
-static const struct snd_soc_dapm_route mt8183_da7219_max98357_dapm_routes[] = {
- {"IT6505_8CH", NULL, "TDM"},
-};
-
/* FE */
SND_SOC_DAILINK_DEFS(playback1,
DAILINK_COMP_ARRAY(COMP_CPU("DL1")),
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH v8 4/5] arm64: Define Documentation/arm64/tagged-address-abi.rst
From: Kevin Brodsky @ 2019-08-15 16:54 UTC (permalink / raw)
To: Catalin Marinas, linux-arm-kernel, linux-mm
Cc: linux-arch, linux-doc, Szabolcs Nagy, Andrey Konovalov,
Will Deacon, Dave Hansen, Andrew Morton, Vincenzo Frascino,
Dave P Martin
In-Reply-To: <20190815154403.16473-5-catalin.marinas@arm.com>
On 15/08/2019 16:44, Catalin Marinas wrote:
> From: Vincenzo Frascino <vincenzo.frascino@arm.com>
>
> On AArch64 the TCR_EL1.TBI0 bit is set by default, allowing userspace
> (EL0) to perform memory accesses through 64-bit pointers with a non-zero
> top byte. Introduce the document describing the relaxation of the
> syscall ABI that allows userspace to pass certain tagged pointers to
> kernel syscalls.
>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Andrey Konovalov <andreyknvl@google.com>
> Cc: Szabolcs Nagy <szabolcs.nagy@arm.com>
> Cc: Kevin Brodsky <kevin.brodsky@arm.com>
> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
> Co-developed-by: Catalin Marinas <catalin.marinas@arm.com>
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> ---
> Documentation/arm64/tagged-address-abi.rst | 155 +++++++++++++++++++++
> 1 file changed, 155 insertions(+)
> create mode 100644 Documentation/arm64/tagged-address-abi.rst
>
> diff --git a/Documentation/arm64/tagged-address-abi.rst b/Documentation/arm64/tagged-address-abi.rst
> new file mode 100644
> index 000000000000..8808337775d6
> --- /dev/null
> +++ b/Documentation/arm64/tagged-address-abi.rst
> @@ -0,0 +1,155 @@
> +==========================
> +AArch64 TAGGED ADDRESS ABI
> +==========================
> +
> +Authors: Vincenzo Frascino <vincenzo.frascino@arm.com>
> + Catalin Marinas <catalin.marinas@arm.com>
> +
> +Date: 15 August 2019
> +
> +This document describes the usage and semantics of the Tagged Address
> +ABI on AArch64 Linux.
> +
> +1. Introduction
> +---------------
> +
> +On AArch64 the TCR_EL1.TBI0 bit is set by default, allowing userspace
> +(EL0) to perform memory accesses through 64-bit pointers with a non-zero
> +top byte. This document describes the relaxation of the syscall ABI that
> +allows userspace to pass certain tagged pointers to kernel syscalls.
> +
> +2. AArch64 Tagged Address ABI
> +-----------------------------
> +
> +From the kernel syscall interface perspective and for the purposes of
> +this document, a "valid tagged pointer" is a pointer with a potentially
> +non-zero top-byte that references an address in the user process address
> +space obtained in one of the following ways:
> +
> +- mmap() done by the process itself (or its parent), where either:
The "parent" aspect is a useful addition, but technically, the mapping may have been
established by any process indirectly forked from the current process, not just its
immediate parent. I wonder if there is a better way to formulate this, to avoid this
complication. Maybe simply "mmap() syscall" (syscalls are always made from userspace,
and any mapping requested by userspace is eligible here)?
> +
> + - flags have the **MAP_ANONYMOUS** bit set
> + - the file descriptor refers to a regular file (including those
> + returned by memfd_create()) or **/dev/zero**
> +
> +- brk() system call done by the process itself (i.e. the heap area
Same idea.
> + between the initial location of the program break at process creation
> + and its current location).
> +
> +- any memory mapped by the kernel in the address space of the process
> + during creation and with the same restrictions as for mmap() above
> + (e.g. data, bss, stack).
> +
> +The AArch64 Tagged Address ABI has two stages of relaxation depending
> +how the user addresses are used by the kernel:
> +
> +1. User addresses not accessed by the kernel but used for address space
> + management (e.g. mmap(), mprotect(), madvise()). The use of valid
> + tagged pointers in this context is always allowed.
> +
> +2. User addresses accessed by the kernel (e.g. write()). This ABI
> + relaxation is disabled by default and the application thread needs to
> + explicitly enable it via **prctl()** as follows:
> +
> + - **PR_SET_TAGGED_ADDR_CTRL**: enable or disable the AArch64 Tagged
> + Address ABI for the calling thread.
> +
> + The (unsigned int) arg2 argument is a bit mask describing the
> + control mode used:
> +
> + - **PR_TAGGED_ADDR_ENABLE**: enable AArch64 Tagged Address ABI.
> + Default status is disabled.
> +
> + Arguments arg3, arg4, and arg5 must be 0.
> +
> + - **PR_GET_TAGGED_ADDR_CTRL**: get the status of the AArch64 Tagged
> + Address ABI for the calling thread.
> +
> + Arguments arg2, arg3, arg4, and arg5 must be 0.
> +
> + The ABI properties described above are thread-scoped, inherited on
> + clone() and fork() and cleared on exec().
> +
> + Calling prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0)
> + returns -EINVAL if the AArch64 Tagged Address ABI is globally disabled
> + by sysctl abi.tagged_addr_disabled=1. The default sysctl
> + abi.tagged_addr_disabled configuration is 0.
> +
> +When the AArch64 Tagged Address ABI is enabled for a thread, the
> +following behaviours are guaranteed:
> +
> +- All syscalls except the cases mentioned in section 3 can accept any
> + valid tagged pointer.
> +
> +- The syscall behaviour is undefined for invalid tagged pointers: it may
> + result in an error code being returned, a (fatal) signal being raised,
> + or other modes of failure.
> +
> +- A valid tagged pointer has the same semantics as the corresponding
> + untagged pointer.
> +
> +A definition of the meaning of tagged pointers on AArch64 can be found
> +in Documentation/arm64/tagged-pointers.rst.
> +
> +3. AArch64 Tagged Address ABI Exceptions
> +-----------------------------------------
> +
> +The following system call parameters must be untagged regardless of the
> +ABI relaxation:
> +
> +- prctl() other than arguments pointing to user structures to be
> + accessed by the kernel.
> +
> +- ioctl() other than arguments pointing to user structures to be
> + accessed by the kernel.
Isn't "user structures" too restrictive? For instance, PR_SET_NAME takes a char *,
and there's no reason not allow it to be tagged. Maybe a more generic "user data"?
There is the additional issue of user struct's containing pointers, I guess the
restriction should apply recursively...
Otherwise, the ABI looks pretty good to me, especially the new address space
management / user data distinction.
Kevin
> +
> +- shmat() and shmdt().
> +
> +Any attempt to use non-zero tagged pointers may result in an error code
> +being returned, a (fatal) signal being raised, or other modes of
> +failure.
> +
> +4. Example of correct usage
> +---------------------------
> +.. code-block:: c
> +
> + #include <stdlib.h>
> + #include <string.h>
> + #include <unistd.h>
> + #include <sys/mman.h>
> + #include <sys/prctl.h>
> +
> + #define PR_SET_TAGGED_ADDR_CTRL 55
> + #define PR_TAGGED_ADDR_ENABLE (1UL << 0)
> +
> + #define TAG_SHIFT 56
> +
> + int main(void)
> + {
> + int tbi_enabled = 0;
> + unsigned long tag = 0;
> + char *ptr;
> +
> + /* check/enable the tagged address ABI */
> + if (!prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0))
> + tbi_enabled = 1;
> +
> + /* memory allocation */
> + ptr = mmap(NULL, sysconf(_SC_PAGE_SIZE), PROT_READ | PROT_WRITE,
> + MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> + if (ptr == MAP_FAILED)
> + return 1;
> +
> + /* set a non-zero tag if the ABI is available */
> + if (tbi_enabled)
> + tag = rand() & 0xff;
> + ptr = (char *)((unsigned long)ptr | (tag << TAG_SHIFT));
> +
> + /* memory access to a tagged address */
> + strcpy(ptr, "tagged pointer\n");
> +
> + /* syscall with a tagged pointer */
> + write(1, ptr, strlen(ptr));
> +
> + return 0;
> + }
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] arm64: do_csum: implement accelerated scalar version
From: Will Deacon @ 2019-08-15 16:46 UTC (permalink / raw)
To: Zhangshaokun
Cc: Ard Biesheuvel, netdev, ilias.apalodimas, huanglingyan (A),
Robin Murphy, linux-arm-kernel, steve.capper
In-Reply-To: <440eb674-0e59-a97e-4a90-0026e2327069@hisilicon.com>
On Thu, May 16, 2019 at 11:14:35AM +0800, Zhangshaokun wrote:
> On 2019/5/15 17:47, Will Deacon wrote:
> > On Mon, Apr 15, 2019 at 07:18:22PM +0100, Robin Murphy wrote:
> >> On 12/04/2019 10:52, Will Deacon wrote:
> >>> I'm waiting for Robin to come back with numbers for a C implementation.
> >>>
> >>> Robin -- did you get anywhere with that?
> >>
> >> Still not what I would call finished, but where I've got so far (besides an
> >> increasingly elaborate test rig) is as below - it still wants some unrolling
> >> in the middle to really fly (and actual testing on BE), but the worst-case
> >> performance already equals or just beats this asm version on Cortex-A53 with
> >> GCC 7 (by virtue of being alignment-insensitive and branchless except for
> >> the loop). Unfortunately, the advantage of C code being instrumentable does
> >> also come around to bite me...
> >
> > Is there any interest from anybody in spinning a proper patch out of this?
> > Shaokun?
>
> HiSilicon's Kunpeng920(Hi1620) benefits from do_csum optimization, if Ard and
> Robin are ok, Lingyan or I can try to do it.
> Of course, if any guy posts the patch, we are happy to test it.
> Any will be ok.
I don't mind who posts it, but Robin is super busy with SMMU stuff at the
moment so it probably makes more sense for you or Lingyan to do it.
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] i2c: iproc: Make bcm_iproc_i2c_quirks constant
From: Ray Jui @ 2019-08-15 16:43 UTC (permalink / raw)
To: Nishka Dasgupta, rjui, sbranden, bcm-kernel-feedback-list,
linux-i2c, linux-arm-kernel
In-Reply-To: <20190815055550.1588-1-nishkadg.linux@gmail.com>
Hi Nishka,
On 8/14/19 10:55 PM, Nishka Dasgupta wrote:
> Static structure bcm_iproc_i2c_quirks, of type i2c_adapter_quirks, is
> only used when being assigned to constant field quirks of a variable
> having type i2c_adapter. Hence make bcm_iproc_i2c_quirks constant as
> well to prevent it from unintended modification.
> Issue found with Coccinelle.
>
> Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
> ---
> drivers/i2c/busses/i2c-bcm-iproc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c b/drivers/i2c/busses/i2c-bcm-iproc.c
> index d7fd76baec92..e9f0e5b6eadc 100644
> --- a/drivers/i2c/busses/i2c-bcm-iproc.c
> +++ b/drivers/i2c/busses/i2c-bcm-iproc.c
> @@ -805,7 +805,7 @@ static struct i2c_algorithm bcm_iproc_algo = {
> .unreg_slave = bcm_iproc_i2c_unreg_slave,
> };
>
> -static struct i2c_adapter_quirks bcm_iproc_i2c_quirks = {
> +static const struct i2c_adapter_quirks bcm_iproc_i2c_quirks = {
> .max_read_len = M_RX_MAX_READ_LEN,
> };
>
>
This looks good to me. Thanks!
Reviewed-by: Ray Jui <ray.jui@broadcom.com>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCHv3 6/6] smccc: make 1.1 macros value-returning
From: Will Deacon @ 2019-08-15 16:42 UTC (permalink / raw)
To: Mark Rutland
Cc: lorenzo.pieralisi, suzuki.poulose, marc.zyngier, catalin.marinas,
will.deacon, linux, james.morse, robin.murphy, linux-arm-kernel
In-Reply-To: <20190809132245.43505-7-mark.rutland@arm.com>
On Fri, Aug 09, 2019 at 02:22:45PM +0100, Mark Rutland wrote:
> The arm_smccc_1_1_{smc,hvc}() macros for inline invocation take a res
> pointer as their final argument, matching the out-of-line SMCCC
> invocation functions.
>
> However, the inline invocation macros are variadic, so it's easy to mess
> up passsing the correct parameters.
passing
> Instead, let's make them value-returning, which is less confusing.
I'm not completely sure I agree with you here because, as far as I can
tell, it means that we have a different calling convention for 1.0 (i.e.
arm_smccc_smc()) and 1.1 (i.e. arm_smccc_1_1_smc).
Can we do the same for 1.0 as well or am I missing something?
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ 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