* [PATCH v4 2/3] regulator: add support for SY8106A regulator
From: Icenowy Zheng @ 2018-05-07 12:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180507122942.25758-1-icenowy@aosc.io>
From: Ondrej Jirman <megous@megous.com>
SY8106A is an I2C attached single output regulator made by Silergy Corp,
which is used on several Allwinner H3/H5 SBCs to control the power
supply of the ARM cores.
Add a driver for it.
Signed-off-by: Ondrej Jirman <megous@megous.com>
[Icenowy: Change commit message, remove enable/disable code, add default
ramp_delay, add comment for go bit, add code for fixed mode voltage]
Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
Reviewed-by: Chen-Yu Tsai <wens@csie.org>
---
Changes in v4:
- Drop custom {get,set}_voltage_sel function, and force GO_BIT to be set
when probing.
Changes in v3:
- Return the fixed voltage defined in device tree when I2C regulating
not enabled (in this situation the register may contain no valid
voltage data).
Changes in v2:
- Dropped the enable/disable code.
- Added default ramp_delay value.
- Added comment for the "go bit".
MAINTAINERS | 6 +
drivers/regulator/Kconfig | 7 ++
drivers/regulator/Makefile | 2 +-
drivers/regulator/sy8106a-regulator.c | 168 ++++++++++++++++++++++++++
4 files changed, 182 insertions(+), 1 deletion(-)
create mode 100644 drivers/regulator/sy8106a-regulator.c
diff --git a/MAINTAINERS b/MAINTAINERS
index e03887faca4f..a27b8e02c7d8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13512,6 +13512,12 @@ S: Supported
F: net/switchdev/
F: include/net/switchdev.h
+SY8106A REGULATOR DRIVER
+M: Icenowy Zheng <icenowy@aosc.io>
+S: Maintained
+F: drivers/regulator/sy8106a-regulator.c
+F: Documentation/devicetree/bindings/regulator/sy8106a-regulator.txt
+
SYNC FILE FRAMEWORK
M: Sumit Semwal <sumit.semwal@linaro.org>
R: Gustavo Padovan <gustavo@padovan.org>
diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 097f61784a7d..4efae3b7e746 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -801,6 +801,13 @@ config REGULATOR_STW481X_VMMC
This driver supports the internal VMMC regulator in the STw481x
PMIC chips.
+config REGULATOR_SY8106A
+ tristate "Silergy SY8106A regulator"
+ depends on I2C && (OF || COMPILE_TEST)
+ select REGMAP_I2C
+ help
+ This driver supports SY8106A single output regulator.
+
config REGULATOR_TPS51632
tristate "TI TPS51632 Power Regulator"
depends on I2C
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 590674fbecd7..d81fb02bd6e9 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -100,6 +100,7 @@ obj-$(CONFIG_REGULATOR_SC2731) += sc2731-regulator.o
obj-$(CONFIG_REGULATOR_SKY81452) += sky81452-regulator.o
obj-$(CONFIG_REGULATOR_STM32_VREFBUF) += stm32-vrefbuf.o
obj-$(CONFIG_REGULATOR_STW481X_VMMC) += stw481x-vmmc.o
+obj-$(CONFIG_REGULATOR_SY8106A) += sy8106a-regulator.o
obj-$(CONFIG_REGULATOR_TI_ABB) += ti-abb-regulator.o
obj-$(CONFIG_REGULATOR_TPS6105X) += tps6105x-regulator.o
obj-$(CONFIG_REGULATOR_TPS62360) += tps62360-regulator.o
@@ -125,5 +126,4 @@ obj-$(CONFIG_REGULATOR_WM8350) += wm8350-regulator.o
obj-$(CONFIG_REGULATOR_WM8400) += wm8400-regulator.o
obj-$(CONFIG_REGULATOR_WM8994) += wm8994-regulator.o
-
ccflags-$(CONFIG_REGULATOR_DEBUG) += -DDEBUG
diff --git a/drivers/regulator/sy8106a-regulator.c b/drivers/regulator/sy8106a-regulator.c
new file mode 100644
index 000000000000..0a398be3f023
--- /dev/null
+++ b/drivers/regulator/sy8106a-regulator.c
@@ -0,0 +1,167 @@
+// SPDX-License-Identifier: GPL-2.0+
+//
+// sy8106a-regulator.c - Regulator device driver for SY8106A
+//
+// Copyright (C) 2016 Ond?ej Jirman <megous@megous.com>
+// Copyright (c) 2017-2018 Icenowy Zheng <icenowy@aosc.io>
+
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/of_regulator.h>
+
+#define SY8106A_REG_VOUT1_SEL 0x01
+#define SY8106A_REG_VOUT_COM 0x02
+#define SY8106A_REG_VOUT1_SEL_MASK 0x7f
+#define SY8106A_DISABLE_REG BIT(0)
+/*
+ * The I2C controlled voltage will only work when this bit is set; otherwise
+ * it will behave like a fixed regulator.
+ */
+#define SY8106A_GO_BIT BIT(7)
+
+struct sy8106a {
+ struct regulator_dev *rdev;
+ struct regmap *regmap;
+ u32 fixed_voltage;
+};
+
+static const struct regmap_config sy8106a_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+};
+
+static const struct regulator_ops sy8106a_ops = {
+ .set_voltage_sel = regulator_set_voltage_sel_regmap,
+ .set_voltage_time_sel = regulator_set_voltage_time_sel,
+ .get_voltage_sel = regulator_get_voltage_sel_regmap,
+ .list_voltage = regulator_list_voltage_linear,
+ /* Enabling/disabling the regulator is not yet implemented */
+};
+
+/* Default limits measured in millivolts */
+#define SY8106A_MIN_MV 680
+#define SY8106A_MAX_MV 1950
+#define SY8106A_STEP_MV 10
+
+static const struct regulator_desc sy8106a_reg = {
+ .name = "SY8106A",
+ .id = 0,
+ .ops = &sy8106a_ops,
+ .type = REGULATOR_VOLTAGE,
+ .n_voltages = ((SY8106A_MAX_MV - SY8106A_MIN_MV) / SY8106A_STEP_MV) + 1,
+ .min_uV = (SY8106A_MIN_MV * 1000),
+ .uV_step = (SY8106A_STEP_MV * 1000),
+ .vsel_reg = SY8106A_REG_VOUT1_SEL,
+ .vsel_mask = SY8106A_REG_VOUT1_SEL_MASK,
+ /*
+ * This ramp_delay is a conservative default value which works on
+ * H3/H5 boards VDD-CPUX situations.
+ */
+ .ramp_delay = 200,
+ .owner = THIS_MODULE,
+};
+
+/*
+ * I2C driver interface functions
+ */
+static int sy8106a_i2c_probe(struct i2c_client *i2c,
+ const struct i2c_device_id *id)
+{
+ struct sy8106a *chip;
+ struct device *dev = &i2c->dev;
+ struct regulator_dev *rdev = NULL;
+ struct regulator_config config = { };
+ unsigned int reg, vsel;
+ int error;
+
+ chip = devm_kzalloc(&i2c->dev, sizeof(struct sy8106a), GFP_KERNEL);
+ if (!chip)
+ return -ENOMEM;
+
+ error = of_property_read_u32(dev->of_node, "silergy,fixed-microvolt",
+ &chip->fixed_voltage);
+ if (error)
+ return error;
+
+ if (chip->fixed_voltage < SY8106A_MIN_MV * 1000 ||
+ chip->fixed_voltage > SY8106A_MAX_MV * 1000)
+ return -EINVAL;
+
+ chip->regmap = devm_regmap_init_i2c(i2c, &sy8106a_regmap_config);
+ if (IS_ERR(chip->regmap)) {
+ error = PTR_ERR(chip->regmap);
+ dev_err(dev, "Failed to allocate register map: %d\n", error);
+ return error;
+ }
+
+ config.dev = &i2c->dev;
+ config.regmap = chip->regmap;
+ config.driver_data = chip;
+
+ config.of_node = dev->of_node;
+ config.init_data = of_get_regulator_init_data(dev, dev->of_node,
+ &sy8106a_reg);
+
+ if (!config.init_data)
+ return -ENOMEM;
+
+ /* Ensure GO_BIT is enabled when probing */
+ error = regmap_read(chip->regmap, SY8106A_REG_VOUT1_SEL, ®);
+ if (error)
+ return error;
+
+ if (!(reg & SY8106A_GO_BIT)) {
+ vsel = (chip->fixed_voltage / 1000 - SY8106A_MIN_MV) /
+ SY8106A_STEP_MV;
+
+ error = regmap_write(chip->regmap, SY8106A_REG_VOUT1_SEL,
+ vsel | SY8106A_GO_BIT);
+ if (error)
+ return error;
+ }
+
+ /* Probe regulator */
+ rdev = devm_regulator_register(&i2c->dev, &sy8106a_reg, &config);
+ if (IS_ERR(rdev)) {
+ error = PTR_ERR(rdev);
+ dev_err(&i2c->dev, "Failed to register SY8106A regulator: %d\n", error);
+ return error;
+ }
+
+ chip->rdev = rdev;
+
+ i2c_set_clientdata(i2c, chip);
+
+ return 0;
+}
+
+static const struct of_device_id sy8106a_i2c_of_match[] = {
+ { .compatible = "silergy,sy8106a" },
+ { },
+};
+MODULE_DEVICE_TABLE(of, sy8106a_i2c_of_match);
+
+static const struct i2c_device_id sy8106a_i2c_id[] = {
+ { "sy8106a", 0 },
+ { },
+};
+MODULE_DEVICE_TABLE(i2c, sy8106a_i2c_id);
+
+static struct i2c_driver sy8106a_regulator_driver = {
+ .driver = {
+ .name = "sy8106a",
+ .of_match_table = of_match_ptr(sy8106a_i2c_of_match),
+ },
+ .probe = sy8106a_i2c_probe,
+ .id_table = sy8106a_i2c_id,
+};
+
+module_i2c_driver(sy8106a_regulator_driver);
+
+MODULE_AUTHOR("Ond?ej Jirman <megous@megous.com>");
+MODULE_AUTHOR("Icenowy Zheng <icenowy@aosc.io>");
+MODULE_DESCRIPTION("Regulator device driver for Silergy SY8106A");
+MODULE_LICENSE("GPL");
--
2.17.0
^ permalink raw reply related
* [PATCH v4 1/3] dt-bindings: add binding for the SY8106A voltage regulator
From: Icenowy Zheng @ 2018-05-07 12:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180507122942.25758-1-icenowy@aosc.io>
From: Ondrej Jirman <megous@megous.com>
SY8106A is an I2C-controlled adjustable voltage regulator made by
Silergy Corp.
Add its device tree binding.
Signed-off-by: Ondrej Jirman <megous@megous.com>
[Icenowy: Change commit message and slight fixes]
Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
Reviewed-by: Chen-Yu Tsai <wens@csie.org>
Acked-by: Rob Herring <robh@kernel.org>
---
No changes in v4.
Changes in v3:
- Added silergy,fixed-microvolt property.
Changes in v2:
- Added Chen-Yu's Reviewed tag and Rob's ACK tag.
- Specify regulator.txt's directory.
.../bindings/regulator/sy8106a-regulator.txt | 23 +++++++++++++++++++
1 file changed, 23 insertions(+)
create mode 100644 Documentation/devicetree/bindings/regulator/sy8106a-regulator.txt
diff --git a/Documentation/devicetree/bindings/regulator/sy8106a-regulator.txt b/Documentation/devicetree/bindings/regulator/sy8106a-regulator.txt
new file mode 100644
index 000000000000..39a8ca73f572
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/sy8106a-regulator.txt
@@ -0,0 +1,23 @@
+SY8106A Voltage regulator
+
+Required properties:
+- compatible: Must be "silergy,sy8106a"
+- reg: I2C slave address - must be <0x65>
+- silergy,fixed-microvolt - the voltage when I2C regulating is disabled (set
+ by external resistor like a fixed voltage)
+
+Any property defined as part of the core regulator binding, defined in
+./regulator.txt, can also be used.
+
+Example:
+
+ sy8106a {
+ compatible = "silergy,sy8106a";
+ reg = <0x65>;
+ regulator-name = "sy8106a-vdd";
+ silergy,fixed-microvolt = <1200000>;
+ regulator-min-microvolt = <1000000>;
+ regulator-max-microvolt = <1400000>;
+ regulator-boot-on;
+ regulator-always-on;
+ };
--
2.17.0
^ permalink raw reply related
* [PATCH v4 0/3] SY8106A regulator support and enable it on Orange Pi PC
From: Icenowy Zheng @ 2018-05-07 12:29 UTC (permalink / raw)
To: linux-arm-kernel
This patchset adds dt-bindings and driver for Silergy SY8106A, and then
utilize it on the Orange Pi PC board, which uses SY8016A as its CPUX
(main ARM CPU cluster in an Allwinner SoC) power supply.
The driver's functionality is restricted now, mainly {en,dis}able function
is not yet implemented, as it cannot be debugged on Orange Pi PC
(disable it will kill Linux).
Ondrej Jirman (3):
dt-bindings: add binding for the SY8106A voltage regulator
regulator: add support for SY8106A regulator
ARM: dts: sun8i: h3: Add SY8106A regulator to Orange Pi PC
.../bindings/regulator/sy8106a-regulator.txt | 23 +++
MAINTAINERS | 6 +
arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts | 28 +++
drivers/regulator/Kconfig | 7 +
drivers/regulator/Makefile | 2 +-
drivers/regulator/sy8106a-regulator.c | 168 ++++++++++++++++++
6 files changed, 233 insertions(+), 1 deletion(-)
create mode 100644 Documentation/devicetree/bindings/regulator/sy8106a-regulator.txt
create mode 100644 drivers/regulator/sy8106a-regulator.c
--
2.17.0
^ permalink raw reply
* [PATCH] arm64: dts: fsl-ls1012a: Fix DTC aliases warnings
From: Fabio Estevam @ 2018-05-07 12:02 UTC (permalink / raw)
To: linux-arm-kernel
From: Fabio Estevam <fabio.estevam@nxp.com>
Use '-' instead of '_' to fix the following DTC warnings with W=1:
arch/arm64/boot/dts/freescale/fsl-ls1012a-frdm.dtb: Warning (alias_paths): /aliases: aliases property name must include only lowercase and '-'
Cc: Harninder Rai <harninder.rai@nxp.com>
Cc: Bhaskar Upadhaya <Bhaskar.Upadhaya@nxp.com>
Cc: Li Yang <leoyang.li@nxp.com>
Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
---
arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi
index bb788ed..205f0f4 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi
@@ -53,11 +53,11 @@
aliases {
crypto = &crypto;
- rtic_a = &rtic_a;
- rtic_b = &rtic_b;
- rtic_c = &rtic_c;
- rtic_d = &rtic_d;
- sec_mon = &sec_mon;
+ rtic-a = &rtic_a;
+ rtic-b = &rtic_b;
+ rtic-c = &rtic_c;
+ rtic-d = &rtic_d;
+ sec-mon = &sec_mon;
};
cpus {
--
2.7.4
^ permalink raw reply related
* [PATCH] ARM: dts: imx28-tx28: Fix alias DTC warnings
From: Fabio Estevam @ 2018-05-07 11:39 UTC (permalink / raw)
To: linux-arm-kernel
From: Fabio Estevam <fabio.estevam@nxp.com>
Remove underscore from alias entries to fix the following DTC warnings
with W=1:
arch/arm/boot/dts/imx28-tx28.dtb: Warning (alias_paths): /aliases: aliases property name must include only lowercase and '-'
Cc: Lothar Wa?mann <LW@KARO-electronics.de>
Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
---
arch/arm/boot/dts/imx28-tx28.dts | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/arm/boot/dts/imx28-tx28.dts b/arch/arm/boot/dts/imx28-tx28.dts
index e4f19f9b..a5d6c97 100644
--- a/arch/arm/boot/dts/imx28-tx28.dts
+++ b/arch/arm/boot/dts/imx28-tx28.dts
@@ -56,11 +56,11 @@
ds1339 = &ds1339;
gpio5 = &gpio5;
lcdif = &lcdif;
- lcdif_23bit_pins = &tx28_lcdif_23bit_pins;
- lcdif_24bit_pins = &lcdif_24bit_pins_a;
- reg_can_xcvr = ®_can_xcvr;
- spi_gpio = &spi_gpio;
- spi_mxs = &ssp3;
+ lcdif-23bit-pins = &tx28_lcdif_23bit_pins;
+ lcdif-24bit-pins = &lcdif_24bit_pins_a;
+ reg-can-xcvr = ®_can_xcvr;
+ spigpio = &spi_gpio;
+ spi-mxs = &ssp3;
stk5led = &user_led;
usbotg = &usb0;
};
@@ -220,7 +220,7 @@
linux,no-autorepeat;
};
- spi_gpio: spi-gpio {
+ spi_gpio: spi {
compatible = "spi-gpio";
#address-cells = <1>;
#size-cells = <0>;
--
2.7.4
^ permalink raw reply related
* [PATCH] clk: davinci: psc-da830: fix USB0 48MHz PHY clock registration
From: Sekhar Nori @ 2018-05-07 11:34 UTC (permalink / raw)
To: linux-arm-kernel
USB0 48MHz PHY clock registration fails on DA830 because the
da8xx-cfgchip clock driver cannot get a reference to USB0
LPSC clock.
The USB0 LPSC needs to be enabled during PHY clock enable. Setup
the clock lookup correctly to fix this.
Signed-off-by: Sekhar Nori <nsekhar@ti.com>
---
drivers/clk/davinci/psc-da830.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/davinci/psc-da830.c b/drivers/clk/davinci/psc-da830.c
index f61abf5632ff..081b039fcb02 100644
--- a/drivers/clk/davinci/psc-da830.c
+++ b/drivers/clk/davinci/psc-da830.c
@@ -55,7 +55,8 @@ const struct davinci_psc_init_data da830_psc0_init_data = {
.psc_init = &da830_psc0_init,
};
-LPSC_CLKDEV2(usb0_clkdev, NULL, "musb-da8xx",
+LPSC_CLKDEV3(usb0_clkdev, "fck", "da830-usb-phy-clks",
+ NULL, "musb-da8xx",
NULL, "cppi41-dmaengine");
LPSC_CLKDEV1(usb1_clkdev, NULL, "ohci-da8xx");
/* REVISIT: gpio-davinci.c should be modified to drop con_id */
--
2.16.2
^ permalink raw reply related
* [PATCH 4/4] drm/rockchip: support dp training outside dp firmware
From: Enric Balletbo Serra @ 2018-05-07 11:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1525421338-1021-4-git-send-email-hl@rock-chips.com>
Hi Lin,
Thanks for the patch.
2018-05-04 10:08 GMT+02:00 Lin Huang <hl@rock-chips.com>:
> DP firware use fix phy config value to do training, but some
s/fiware/firmware/
> board need to adjust these value to fit for their hardware design,
> so we use new phy config to do training outside firmware to meet
> this situation, if there have new phy config pass from dts, it will
> use training outside firmware.
>
maybe you can rewrite all this in a better way.
ooi, which boards needs this?
> Signed-off-by: Chris Zhong <zyw@rock-chips.com>
> Signed-off-by: Lin Huang <hl@rock-chips.com>
> ---
> drivers/gpu/drm/rockchip/Makefile | 3 +-
> drivers/gpu/drm/rockchip/cdn-dp-core.c | 23 +-
> drivers/gpu/drm/rockchip/cdn-dp-core.h | 2 +
> drivers/gpu/drm/rockchip/cdn-dp-link-training.c | 398 ++++++++++++++++++++++++
> drivers/gpu/drm/rockchip/cdn-dp-reg.c | 33 +-
> drivers/gpu/drm/rockchip/cdn-dp-reg.h | 38 ++-
> 6 files changed, 480 insertions(+), 17 deletions(-)
> create mode 100644 drivers/gpu/drm/rockchip/cdn-dp-link-training.c
>
> diff --git a/drivers/gpu/drm/rockchip/Makefile b/drivers/gpu/drm/rockchip/Makefile
> index a314e21..b932f62 100644
> --- a/drivers/gpu/drm/rockchip/Makefile
> +++ b/drivers/gpu/drm/rockchip/Makefile
> @@ -9,7 +9,8 @@ rockchipdrm-y := rockchip_drm_drv.o rockchip_drm_fb.o \
> rockchipdrm-$(CONFIG_DRM_FBDEV_EMULATION) += rockchip_drm_fbdev.o
>
> rockchipdrm-$(CONFIG_ROCKCHIP_ANALOGIX_DP) += analogix_dp-rockchip.o
> -rockchipdrm-$(CONFIG_ROCKCHIP_CDN_DP) += cdn-dp-core.o cdn-dp-reg.o
> +rockchipdrm-$(CONFIG_ROCKCHIP_CDN_DP) += cdn-dp-core.o cdn-dp-reg.o \
> + cdn-dp-link-training.o
> rockchipdrm-$(CONFIG_ROCKCHIP_DW_HDMI) += dw_hdmi-rockchip.o
> rockchipdrm-$(CONFIG_ROCKCHIP_DW_MIPI_DSI) += dw-mipi-dsi.o
> rockchipdrm-$(CONFIG_ROCKCHIP_INNO_HDMI) += inno_hdmi.o
> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c b/drivers/gpu/drm/rockchip/cdn-dp-core.c
> index 268c190..a2a4208 100644
> --- a/drivers/gpu/drm/rockchip/cdn-dp-core.c
> +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c
> @@ -629,11 +629,13 @@ static void cdn_dp_encoder_enable(struct drm_encoder *encoder)
> goto out;
> }
> }
> -
> - ret = cdn_dp_set_video_status(dp, CONTROL_VIDEO_IDLE);
> - if (ret) {
> - DRM_DEV_ERROR(dp->dev, "Failed to idle video %d\n", ret);
> - goto out;
> + if (dp->sw_training_success == false) {
> + ret = cdn_dp_set_video_status(dp, CONTROL_VIDEO_IDLE);
> + if (ret) {
> + DRM_DEV_ERROR(dp->dev,
> + "Failed to idle video %d\n", ret);
> + goto out;
> + }
> }
>
> ret = cdn_dp_config_video(dp);
> @@ -642,11 +644,14 @@ static void cdn_dp_encoder_enable(struct drm_encoder *encoder)
> goto out;
> }
>
> - ret = cdn_dp_set_video_status(dp, CONTROL_VIDEO_VALID);
> - if (ret) {
> - DRM_DEV_ERROR(dp->dev, "Failed to valid video %d\n", ret);
> - goto out;
> + if (dp->sw_training_success == false) {
> + ret = cdn_dp_set_video_status(dp, CONTROL_VIDEO_VALID);
> + if (ret) {
> + DRM_DEV_ERROR(dp->dev, "Failed to valid video %d\n", ret);
> + goto out;
> + }
> }
> +
> out:
> mutex_unlock(&dp->lock);
> }
> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.h b/drivers/gpu/drm/rockchip/cdn-dp-core.h
> index 46159b2..c6050ab 100644
> --- a/drivers/gpu/drm/rockchip/cdn-dp-core.h
> +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.h
> @@ -84,6 +84,7 @@ struct cdn_dp_device {
> bool connected;
> bool active;
> bool suspended;
> + bool sw_training_success;
>
> const struct firmware *fw; /* cdn dp firmware */
> unsigned int fw_version; /* cdn fw version */
> @@ -106,6 +107,7 @@ struct cdn_dp_device {
> u8 ports;
> u8 lanes;
> int active_port;
> + u8 train_set[4];
>
> u8 dpcd[DP_RECEIVER_CAP_SIZE];
> bool sink_has_audio;
> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-link-training.c b/drivers/gpu/drm/rockchip/cdn-dp-link-training.c
> new file mode 100644
> index 0000000..558c945
> --- /dev/null
> +++ b/drivers/gpu/drm/rockchip/cdn-dp-link-training.c
> @@ -0,0 +1,398 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
For a C source file the format is:
(https://www.kernel.org/doc/html/latest/process/license-rules.html)
// SPDX-License-Identifier: <SPDX License Expression>
> +/*
> + * Copyright (C) Fuzhou Rockchip Electronics Co.Ltd
> + * Author: Chris Zhong <zyw@rock-chips.com>
> + */
> +
> +#include <linux/arm-smccc.h>
Why you need this include?
> +#include <linux/clk.h>
> +#include <linux/device.h>
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/phy/phy.h>
> +#include <linux/reset.h>
> +#include <soc/rockchip/rockchip_phy_typec.h>
> +
In fact, I think that there are other includes that can be removed,
please review.
> +#include "cdn-dp-core.h"
> +#include "cdn-dp-reg.h"
> +
> +static void cdn_dp_set_signal_levels(struct cdn_dp_device *dp)
> +{
> + struct cdn_dp_port *port = dp->port[dp->active_port];
> + struct rockchip_typec_phy *tcphy = phy_get_drvdata(port->phy);
> +
> + int rate = drm_dp_bw_code_to_link_rate(dp->link.rate);
> + u8 swing = (dp->train_set[0] & DP_TRAIN_VOLTAGE_SWING_MASK) >>
> + DP_TRAIN_VOLTAGE_SWING_SHIFT;
> + u8 pre_emphasis = (dp->train_set[0] & DP_TRAIN_PRE_EMPHASIS_MASK)
> + >> DP_TRAIN_PRE_EMPHASIS_SHIFT;
> +
> + tcphy->typec_phy_config(port->phy, rate, dp->link.num_lanes,
> + swing, pre_emphasis);
> +}
> +
> +static int cdn_dp_set_pattern(struct cdn_dp_device *dp, uint8_t dp_train_pat)
> +{
> + u32 phy_config, global_config;
> + int ret;
> + uint8_t pattern = dp_train_pat & DP_TRAINING_PATTERN_MASK;
> +
> + global_config = NUM_LANES(dp->link.num_lanes - 1) | SST_MODE |
> + GLOBAL_EN | RG_EN | ENC_RST_DIS | WR_VHSYNC_FALL;
> +
> + phy_config = DP_TX_PHY_ENCODER_BYPASS(0) |
> + DP_TX_PHY_SKEW_BYPASS(0) |
> + DP_TX_PHY_DISPARITY_RST(0) |
> + DP_TX_PHY_LANE0_SKEW(0) |
> + DP_TX_PHY_LANE1_SKEW(1) |
> + DP_TX_PHY_LANE2_SKEW(2) |
> + DP_TX_PHY_LANE3_SKEW(3) |
> + DP_TX_PHY_10BIT_ENABLE(0);
> +
> + if (pattern != DP_TRAINING_PATTERN_DISABLE) {
> + global_config |= NO_VIDEO;
> + phy_config |= DP_TX_PHY_TRAINING_ENABLE(1) |
> + DP_TX_PHY_SCRAMBLER_BYPASS(1) |
> + DP_TX_PHY_TRAINING_PATTERN(pattern);
> + }
> +
> + ret = cdn_dp_reg_write(dp, DP_FRAMER_GLOBAL_CONFIG, global_config);
> + if (ret) {
> + DRM_ERROR("fail to set DP_FRAMER_GLOBAL_CONFIG, error: %d\n",
> + ret);
> + return ret;
> + }
> +
> + ret = cdn_dp_reg_write(dp, DP_TX_PHY_CONFIG_REG, phy_config);
> + if (ret) {
> + DRM_ERROR("fail to set DP_TX_PHY_CONFIG_REG, error: %d\n",
> + ret);
> + return ret;
> + }
> +
> + ret = cdn_dp_reg_write(dp, DPTX_LANE_EN, BIT(dp->link.num_lanes) - 1);
> + if (ret) {
> + DRM_ERROR("fail to set DPTX_LANE_EN, error: %d\n", ret);
> + return ret;
> + }
> +
> + if (drm_dp_enhanced_frame_cap(dp->dpcd))
> + ret = cdn_dp_reg_write(dp, DPTX_ENHNCD, 1);
> + else
> + ret = cdn_dp_reg_write(dp, DPTX_ENHNCD, 0);
> + if (ret)
> + DRM_ERROR("failed to set DPTX_ENHNCD, error: %x\n", ret);
> +
> + return ret;
> +}
> +
> +static u8 cdn_dp_pre_emphasis_max(u8 voltage_swing)
> +{
> + switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
> + case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
> + return DP_TRAIN_PRE_EMPH_LEVEL_3;
> + case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
> + return DP_TRAIN_PRE_EMPH_LEVEL_2;
> + case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
> + return DP_TRAIN_PRE_EMPH_LEVEL_1;
> + default:
> + return DP_TRAIN_PRE_EMPH_LEVEL_0;
> + }
> +}
> +
> +static void cdn_dp_get_adjust_train(struct cdn_dp_device *dp,
> + uint8_t link_status[DP_LINK_STATUS_SIZE])
> +{
> + int i;
> + uint8_t v = 0, p = 0;
> + uint8_t preemph_max;
> +
> + for (i = 0; i < dp->link.num_lanes; i++) {
> + v = max(v, drm_dp_get_adjust_request_voltage(link_status, i));
> + p = max(p, drm_dp_get_adjust_request_pre_emphasis(link_status,
> + i));
> + }
> +
> + if (v >= VOLTAGE_LEVEL_2)
> + v = VOLTAGE_LEVEL_2 | DP_TRAIN_MAX_SWING_REACHED;
> +
> + preemph_max = cdn_dp_pre_emphasis_max(v);
> + if (p >= preemph_max)
> + p = preemph_max | DP_TRAIN_MAX_PRE_EMPHASIS_REACHED;
> +
> + for (i = 0; i < dp->link.num_lanes; i++)
> + dp->train_set[i] = v | p;
> +}
> +
> +/*
> + * Pick training pattern for channel equalization. Training Pattern 3 for HBR2
> + * or 1.2 devices that support it, Training Pattern 2 otherwise.
> + */
> +static u32 cdn_dp_select_chaneq_pattern(struct cdn_dp_device *dp)
> +{
> + u32 training_pattern = DP_TRAINING_PATTERN_2;
> +
> + /*
> + * cdn dp support HBR2 also support TPS3. TPS3 support is also mandatory
> + * for downstream devices that support HBR2. However, not all sinks
> + * follow the spec.
> + */
> + if (drm_dp_tps3_supported(dp->dpcd))
> + training_pattern = DP_TRAINING_PATTERN_3;
> + else
> + DRM_DEBUG_KMS("5.4 Gbps link rate without sink TPS3 support\n");
> +
> + return training_pattern;
> +}
> +
> +
> +static bool cdn_dp_link_max_vswing_reached(struct cdn_dp_device *dp)
> +{
> + int lane;
> +
> + for (lane = 0; lane < dp->link.num_lanes; lane++)
> + if ((dp->train_set[lane] & DP_TRAIN_MAX_SWING_REACHED) == 0)
> + return false;
> +
> + return true;
> +}
> +
> +static int cdn_dp_update_link_train(struct cdn_dp_device *dp)
> +{
> + int ret;
> +
> + cdn_dp_set_signal_levels(dp);
> +
> + ret = drm_dp_dpcd_write(&dp->aux, DP_TRAINING_LANE0_SET,
> + dp->train_set, dp->link.num_lanes);
> + if (ret != dp->link.num_lanes)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +static int cdn_dp_set_link_train(struct cdn_dp_device *dp,
> + uint8_t dp_train_pat)
> +{
> + uint8_t buf[sizeof(dp->train_set) + 1];
> + int ret, len;
> +
> + buf[0] = dp_train_pat;
> + if ((dp_train_pat & DP_TRAINING_PATTERN_MASK) ==
> + DP_TRAINING_PATTERN_DISABLE) {
> + /* don't write DP_TRAINING_LANEx_SET on disable */
> + len = 1;
> + } else {
> + /* DP_TRAINING_LANEx_SET follow DP_TRAINING_PATTERN_SET */
> + memcpy(buf + 1, dp->train_set, dp->link.num_lanes);
> + len = dp->link.num_lanes + 1;
> + }
> +
> + ret = drm_dp_dpcd_write(&dp->aux, DP_TRAINING_PATTERN_SET,
> + buf, len);
> + if (ret != len)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +static int cdn_dp_reset_link_train(struct cdn_dp_device *dp,
> + uint8_t dp_train_pat)
> +{
> + int ret;
> +
> + memset(dp->train_set, 0, sizeof(dp->train_set));
> +
> + cdn_dp_set_signal_levels(dp);
> +
> + ret = cdn_dp_set_pattern(dp, dp_train_pat);
> + if (ret)
> + return ret;
> +
> + return cdn_dp_set_link_train(dp, dp_train_pat);
> +}
> +
> +/* Enable corresponding port and start training pattern 1 */
> +static int cdn_dp_link_training_clock_recovery(struct cdn_dp_device *dp)
> +{
> + u8 voltage, link_config[2];
> + u8 link_status[DP_LINK_STATUS_SIZE];
> + u32 voltage_tries, max_vswing_tries;
> + u32 rate, sink_max, source_max;
> + int ret;
> +
> + source_max = dp->lanes;
> + sink_max = drm_dp_max_lane_count(dp->dpcd);
> + dp->link.num_lanes = min(source_max, sink_max);
> +
> + source_max = drm_dp_bw_code_to_link_rate(CDN_DP_MAX_LINK_RATE);
> + sink_max = drm_dp_max_link_rate(dp->dpcd);
> + rate = min(source_max, sink_max);
> + dp->link.rate = drm_dp_link_rate_to_bw_code(rate);
> +
> + /* Write the link configuration data */
> + link_config[0] = dp->link.rate;
> + link_config[1] = dp->link.num_lanes;
> + if (drm_dp_enhanced_frame_cap(dp->dpcd))
> + link_config[1] |= DP_LANE_COUNT_ENHANCED_FRAME_EN;
> + drm_dp_dpcd_write(&dp->aux, DP_LINK_BW_SET, link_config, 2);
> +
> + link_config[0] = 0;
> + link_config[1] = 0;
> + if (dp->dpcd[DP_MAIN_LINK_CHANNEL_CODING] & 0x01)
> + link_config[1] = DP_SET_ANSI_8B10B;
> +
> + drm_dp_dpcd_write(&dp->aux, DP_DOWNSPREAD_CTRL, link_config, 2);
> +
> + /* clock recovery */
> + ret = cdn_dp_reset_link_train(dp, DP_TRAINING_PATTERN_1 |
> + DP_LINK_SCRAMBLING_DISABLE);
> + if (ret) {
> + DRM_ERROR("failed to start link train\n");
> + return ret;
> + }
> +
> + voltage_tries = 1;
> + max_vswing_tries = 0;
> + for (;;) {
> + drm_dp_link_train_clock_recovery_delay(dp->dpcd);
> + if (drm_dp_dpcd_read_link_status(&dp->aux, link_status) !=
> + DP_LINK_STATUS_SIZE) {
> + DRM_ERROR("failed to get link status\n");
> + return -EINVAL;
> + }
> +
> + if (drm_dp_clock_recovery_ok(link_status, dp->link.num_lanes)) {
> + DRM_DEBUG_KMS("clock recovery OK\n");
> + return 0;
> + }
> +
> + if (voltage_tries >= 5) {
> + DRM_DEBUG_KMS("Same voltage tried 5 times\n");
> + return -EINVAL;
> + }
> +
> + if (max_vswing_tries >= 1) {
> + DRM_DEBUG_KMS("Max Voltage Swing reached\n");
> + return -EINVAL;
> + }
> +
> + voltage = dp->train_set[0] & DP_TRAIN_VOLTAGE_SWING_MASK;
> +
> + /* Update training set as requested by target */
> + cdn_dp_get_adjust_train(dp, link_status);
> + if (cdn_dp_update_link_train(dp)) {
> + DRM_ERROR("failed to update link training\n");
> + return -EINVAL;
> + }
> +
> + if ((dp->train_set[0] & DP_TRAIN_VOLTAGE_SWING_MASK) ==
> + voltage)
> + ++voltage_tries;
> + else
> + voltage_tries = 1;
> +
> + if (cdn_dp_link_max_vswing_reached(dp))
> + ++max_vswing_tries;
> + }
> +}
> +
> +static int cdn_dp_link_training_channel_equalization(struct cdn_dp_device *dp)
> +{
> + int tries, ret;
> + u32 training_pattern;
> + uint8_t link_status[DP_LINK_STATUS_SIZE];
> +
> + training_pattern = cdn_dp_select_chaneq_pattern(dp);
> + training_pattern |= DP_LINK_SCRAMBLING_DISABLE;
> +
> + ret = cdn_dp_set_pattern(dp, training_pattern);
> + if (ret)
> + return ret;
> +
> + ret = cdn_dp_set_link_train(dp, training_pattern);
> + if (ret) {
> + DRM_ERROR("failed to start channel equalization\n");
> + return ret;
> + }
> +
> + for (tries = 0; tries < 5; tries++) {
> + drm_dp_link_train_channel_eq_delay(dp->dpcd);
> + if (drm_dp_dpcd_read_link_status(&dp->aux, link_status) !=
> + DP_LINK_STATUS_SIZE) {
> + DRM_ERROR("failed to get link status\n");
> + break;
> + }
> +
> + /* Make sure clock is still ok */
> + if (!drm_dp_clock_recovery_ok(link_status, dp->link.num_lanes)) {
> + DRM_DEBUG_KMS("Clock recovery check failed, cannot "
> + "continue channel equalization\n");
> + break;
> + }
> +
> + if (drm_dp_channel_eq_ok(link_status, dp->link.num_lanes)) {
> + DRM_DEBUG_KMS("Channel EQ done. DP Training "
> + "successful\n");
> + return 0;
> + }
> +
> + /* Update training set as requested by target */
> + cdn_dp_get_adjust_train(dp, link_status);
> + if (cdn_dp_update_link_train(dp)) {
> + DRM_ERROR("failed to update link training\n");
> + break;
> + }
> + }
> +
> + /* Try 5 times, else fail and try at lower BW */
> + if (tries == 5)
> + DRM_DEBUG_KMS("Channel equalization failed 5 times\n");
> +
> + return -EINVAL;
> +
> +}
> +
> +static int cdn_dp_stop_link_train(struct cdn_dp_device *dp)
> +{
> + int ret = cdn_dp_set_pattern(dp, DP_TRAINING_PATTERN_DISABLE);
> +
> + if (ret)
> + return ret;
> +
> + return cdn_dp_set_link_train(dp, DP_TRAINING_PATTERN_DISABLE);
> +}
> +
> +int cdn_dp_software_train_link(struct cdn_dp_device *dp)
> +{
> + int ret, stop_err;
> +
> + ret = drm_dp_dpcd_read(&dp->aux, DP_DPCD_REV, dp->dpcd,
> + sizeof(dp->dpcd));
> + if (ret < 0) {
> + DRM_DEV_ERROR(dp->dev, "Failed to get caps %d\n", ret);
> + return ret;
> + }
> +
> + ret = cdn_dp_link_training_clock_recovery(dp);
> + if (ret) {
> + DRM_ERROR("training clock recovery fail, error: %d\n", ret);
> + goto out;
> + }
> +
> + ret = cdn_dp_link_training_channel_equalization(dp);
> + if (ret) {
> + DRM_ERROR("training channel equalization fail, error: %d\n",
> + ret);
> + goto out;
> + }
> +out:
> + stop_err = cdn_dp_stop_link_train(dp);
> + if (stop_err) {
> + DRM_ERROR("stop training fail, error: %d\n", stop_err);
> + return stop_err;
> + }
> +
> + return ret;
> +}
> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-reg.c b/drivers/gpu/drm/rockchip/cdn-dp-reg.c
> index b2f532a..72780f1 100644
> --- a/drivers/gpu/drm/rockchip/cdn-dp-reg.c
> +++ b/drivers/gpu/drm/rockchip/cdn-dp-reg.c
> @@ -17,7 +17,9 @@
> #include <linux/delay.h>
> #include <linux/io.h>
> #include <linux/iopoll.h>
> +#include <linux/phy/phy.h>
> #include <linux/reset.h>
> +#include <soc/rockchip/rockchip_phy_typec.h>
>
> #include "cdn-dp-core.h"
> #include "cdn-dp-reg.h"
> @@ -189,7 +191,7 @@ static int cdn_dp_mailbox_send(struct cdn_dp_device *dp, u8 module_id,
> return 0;
> }
>
> -static int cdn_dp_reg_write(struct cdn_dp_device *dp, u16 addr, u32 val)
> +int cdn_dp_reg_write(struct cdn_dp_device *dp, u16 addr, u32 val)
> {
> u8 msg[6];
>
> @@ -605,22 +607,41 @@ static int cdn_dp_get_training_status(struct cdn_dp_device *dp)
> int cdn_dp_train_link(struct cdn_dp_device *dp)
> {
> int ret;
> + struct cdn_dp_port *port = dp->port[dp->active_port];
> + struct rockchip_typec_phy *tcphy = phy_get_drvdata(port->phy);
>
> + /*
> + * DP firmware uses fixed phy config values to do training, but some
> + * boards need to adjust these values to fit for their unique hardware
> + * design. So if the phy is using custom config values, do software
> + * link training instead of relying on firmware, if software training
> + * fail, keep firmware training as a fallback if sw training fails.
> + */
> + if (tcphy->need_software_training) {
> + ret = cdn_dp_software_train_link(dp);
> + if (ret) {
> + DRM_DEV_ERROR(dp->dev,
> + "Failed to do software training %d\n", ret);
> + goto do_fw_training;
> + }
> + cdn_dp_reg_write(dp, SOURCE_HDTX_CAR, 0xf);
Check for an error here and act accordingly? Or doesn't matter?
> + dp->sw_training_success = true;
> + return 0;
> + }
> +
nit: Personally I don't like this goto. Maybe you can refactor this.
if (tcphy->need_software_training) {
ret = cdn_dp_software_train_link(dp);
if (!ret) {
cdn_dp_reg_write(dp, SOURCE_HDTX_CAR, 0xf);
dp->sw_training_success = true;
return 0;
}
DRM_DEV_ERROR(dp->dev, "Failed to do software training
%d\n", ret);
}
> +do_fw_training:
Then remove the goto label.
> + dp->sw_training_success = false;
> ret = cdn_dp_training_start(dp);
> if (ret) {
> DRM_DEV_ERROR(dp->dev, "Failed to start training %d\n", ret);
> return ret;
> }
> -
Do not remove the new line.
> ret = cdn_dp_get_training_status(dp);
> if (ret) {
> DRM_DEV_ERROR(dp->dev, "Failed to get training stat %d\n", ret);
> return ret;
> }
> -
> - DRM_DEV_DEBUG_KMS(dp->dev, "rate:0x%x, lanes:%d\n", dp->link.rate,
> - dp->link.num_lanes);
Why you remove this debug message?
> - return ret;
> + return 0;
> }
>
> int cdn_dp_set_video_status(struct cdn_dp_device *dp, int active)
> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-reg.h b/drivers/gpu/drm/rockchip/cdn-dp-reg.h
> index aedf2dc..b60a6b2 100644
> --- a/drivers/gpu/drm/rockchip/cdn-dp-reg.h
> +++ b/drivers/gpu/drm/rockchip/cdn-dp-reg.h
> @@ -137,7 +137,7 @@
> #define HPD_EVENT_MASK 0x211c
> #define HPD_EVENT_DET 0x2120
>
> -/* dpyx framer addr */
> +/* dptx framer addr */
> #define DP_FRAMER_GLOBAL_CONFIG 0x2200
> #define DP_SW_RESET 0x2204
> #define DP_FRAMER_TU 0x2208
> @@ -431,6 +431,40 @@
> /* Reference cycles when using lane clock as reference */
> #define LANE_REF_CYC 0x8000
>
> +/* register CM_VID_CTRL */
> +#define LANE_VID_REF_CYC(x) (((x) & (BIT(24) - 1)) << 0)
> +#define NMVID_MEAS_TOLERANCE(x) (((x) & 0xf) << 24)
> +
> +/* register DP_TX_PHY_CONFIG_REG */
> +#define DP_TX_PHY_TRAINING_ENABLE(x) ((x) & 1)
> +#define DP_TX_PHY_TRAINING_TYPE_PRBS7 (0 << 1)
> +#define DP_TX_PHY_TRAINING_TYPE_TPS1 (1 << 1)
> +#define DP_TX_PHY_TRAINING_TYPE_TPS2 (2 << 1)
> +#define DP_TX_PHY_TRAINING_TYPE_TPS3 (3 << 1)
> +#define DP_TX_PHY_TRAINING_TYPE_TPS4 (4 << 1)
> +#define DP_TX_PHY_TRAINING_TYPE_PLTPAT (5 << 1)
> +#define DP_TX_PHY_TRAINING_TYPE_D10_2 (6 << 1)
> +#define DP_TX_PHY_TRAINING_TYPE_HBR2CPAT (8 << 1)
> +#define DP_TX_PHY_TRAINING_PATTERN(x) ((x) << 1)
> +#define DP_TX_PHY_SCRAMBLER_BYPASS(x) (((x) & 1) << 5)
> +#define DP_TX_PHY_ENCODER_BYPASS(x) (((x) & 1) << 6)
> +#define DP_TX_PHY_SKEW_BYPASS(x) (((x) & 1) << 7)
> +#define DP_TX_PHY_DISPARITY_RST(x) (((x) & 1) << 8)
> +#define DP_TX_PHY_LANE0_SKEW(x) (((x) & 7) << 9)
> +#define DP_TX_PHY_LANE1_SKEW(x) (((x) & 7) << 12)
> +#define DP_TX_PHY_LANE2_SKEW(x) (((x) & 7) << 15)
> +#define DP_TX_PHY_LANE3_SKEW(x) (((x) & 7) << 18)
> +#define DP_TX_PHY_10BIT_ENABLE(x) (((x) & 1) << 21)
> +
> +/* register DP_FRAMER_GLOBAL_CONFIG */
> +#define NUM_LANES(x) ((x) & 3)
> +#define SST_MODE (0 << 2)
> +#define GLOBAL_EN (1 << 3)
> +#define RG_EN (0 << 4)
> +#define NO_VIDEO (1 << 5)
> +#define ENC_RST_DIS (1 << 6)
> +#define WR_VHSYNC_FALL (1 << 7)
> +
Use the BIT() macros for these.
> enum voltage_swing_level {
> VOLTAGE_LEVEL_0,
> VOLTAGE_LEVEL_1,
> @@ -476,6 +510,7 @@ int cdn_dp_set_host_cap(struct cdn_dp_device *dp, u8 lanes, bool flip);
> int cdn_dp_event_config(struct cdn_dp_device *dp);
> u32 cdn_dp_get_event(struct cdn_dp_device *dp);
> int cdn_dp_get_hpd_status(struct cdn_dp_device *dp);
> +int cdn_dp_reg_write(struct cdn_dp_device *dp, u16 addr, u32 val);
> ssize_t cdn_dp_dpcd_write(struct cdn_dp_device *dp, u32 addr,
> u8 *data, u16 len);
> ssize_t cdn_dp_dpcd_read(struct cdn_dp_device *dp, u32 addr,
> @@ -489,4 +524,5 @@ int cdn_dp_config_video(struct cdn_dp_device *dp);
> int cdn_dp_audio_stop(struct cdn_dp_device *dp, struct audio_info *audio);
> int cdn_dp_audio_mute(struct cdn_dp_device *dp, bool enable);
> int cdn_dp_audio_config(struct cdn_dp_device *dp, struct audio_info *audio);
> +int cdn_dp_software_train_link(struct cdn_dp_device *dp);
> #endif /* _CDN_DP_REG_H */
> --
> 2.7.4
>
^ permalink raw reply
* [PATCH 1/4] drm/rockchip: add transfer function for cdn-dp
From: Enric Balletbo Serra @ 2018-05-07 11:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1525421338-1021-1-git-send-email-hl@rock-chips.com>
Hi Lin,
I am interested in these patches, could you cc me on newer versions? Thanks.
Some comments below.
2018-05-04 10:08 GMT+02:00 Lin Huang <hl@rock-chips.com>:
> From: Chris Zhong <zyw@rock-chips.com>
>
> We may support training outside firmware, so we need support
> dpcd read/write to get the message or do some setting with
> display.
>
> Signed-off-by: Chris Zhong <zyw@rock-chips.com>
> Signed-off-by: Lin Huang <hl@rock-chips.com>
> ---
> drivers/gpu/drm/rockchip/cdn-dp-core.c | 55 ++++++++++++++++++++++++----
> drivers/gpu/drm/rockchip/cdn-dp-core.h | 1 +
> drivers/gpu/drm/rockchip/cdn-dp-reg.c | 66 +++++++++++++++++++++++++++++-----
> drivers/gpu/drm/rockchip/cdn-dp-reg.h | 14 ++++++--
> 4 files changed, 119 insertions(+), 17 deletions(-)
>
In general, for this patch and all the other patches in the series I
saw that checkpatch spits out some warnings, could you fix these and
ideally run checkpatch with the --strict --subjective option?
> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c b/drivers/gpu/drm/rockchip/cdn-dp-core.c
> index c6fbdcd..268c190 100644
> --- a/drivers/gpu/drm/rockchip/cdn-dp-core.c
> +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c
> @@ -176,8 +176,8 @@ static int cdn_dp_get_sink_count(struct cdn_dp_device *dp, u8 *sink_count)
> u8 value;
>
> *sink_count = 0;
> - ret = cdn_dp_dpcd_read(dp, DP_SINK_COUNT, &value, 1);
> - if (ret)
> + ret = drm_dp_dpcd_read(&dp->aux, DP_SINK_COUNT, &value, 1);
> + if (ret < 0)
> return ret;
>
> *sink_count = DP_GET_SINK_COUNT(value);
> @@ -374,9 +374,9 @@ static int cdn_dp_get_sink_capability(struct cdn_dp_device *dp)
> if (!cdn_dp_check_sink_connection(dp))
> return -ENODEV;
>
> - ret = cdn_dp_dpcd_read(dp, DP_DPCD_REV, dp->dpcd,
> - DP_RECEIVER_CAP_SIZE);
> - if (ret) {
> + ret = drm_dp_dpcd_read(&dp->aux, DP_DPCD_REV, dp->dpcd,
> + sizeof(dp->dpcd));
> + if (ret < 0) {
> DRM_DEV_ERROR(dp->dev, "Failed to get caps %d\n", ret);
> return ret;
> }
> @@ -582,8 +582,8 @@ static bool cdn_dp_check_link_status(struct cdn_dp_device *dp)
> if (!port || !dp->link.rate || !dp->link.num_lanes)
> return false;
>
> - if (cdn_dp_dpcd_read(dp, DP_LANE0_1_STATUS, link_status,
> - DP_LINK_STATUS_SIZE)) {
> + if (drm_dp_dpcd_read_link_status(&dp->aux, link_status) !=
> + DP_LINK_STATUS_SIZE) {
> DRM_ERROR("Failed to get link status\n");
> return false;
> }
> @@ -1012,6 +1012,40 @@ static int cdn_dp_pd_event(struct notifier_block *nb,
> return NOTIFY_DONE;
> }
>
> +static ssize_t cdn_dp_aux_transfer(struct drm_dp_aux *aux,
> + struct drm_dp_aux_msg *msg)
> +{
> + struct cdn_dp_device *dp = container_of(aux, struct cdn_dp_device, aux);
> + int ret;
> + u8 status;
> +
> + switch (msg->request & ~DP_AUX_I2C_MOT) {
> + case DP_AUX_NATIVE_WRITE:
> + case DP_AUX_I2C_WRITE:
> + case DP_AUX_I2C_WRITE_STATUS_UPDATE:
> + ret = cdn_dp_dpcd_write(dp, msg->address, msg->buffer,
> + msg->size);
> + break;
> + case DP_AUX_NATIVE_READ:
> + case DP_AUX_I2C_READ:
> + ret = cdn_dp_dpcd_read(dp, msg->address, msg->buffer,
> + msg->size);
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + status = cdn_dp_get_aux_status(dp);
> + if (status == AUX_STAUS_ACK)
> + msg->reply = DP_AUX_NATIVE_REPLY_ACK;
> + else if (status == AUX_STAUS_NACK)
> + msg->reply = DP_AUX_NATIVE_REPLY_NACK;
> + else if (status == AUX_STAUS_DEFER)
> + msg->reply = DP_AUX_NATIVE_REPLY_DEFER;
> +
I think that you would mean STATUS instead of STAUS on these defines.
What happens if the status is AUX_STATUS_SINK_ERROR or AUX_STATUS_BUS_ERROR?
> + return ret;
> +}
> +
> static int cdn_dp_bind(struct device *dev, struct device *master, void *data)
> {
> struct cdn_dp_device *dp = dev_get_drvdata(dev);
> @@ -1030,6 +1064,13 @@ static int cdn_dp_bind(struct device *dev, struct device *master, void *data)
> dp->active = false;
> dp->active_port = -1;
> dp->fw_loaded = false;
> + dp->aux.name = "DP-AUX";
> + dp->aux.transfer = cdn_dp_aux_transfer;
> + dp->aux.dev = dev;
> +
> + ret = drm_dp_aux_register(&dp->aux);
> + if (ret)
> + return ret;
>
> INIT_WORK(&dp->event_work, cdn_dp_pd_event_work);
>
> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.h b/drivers/gpu/drm/rockchip/cdn-dp-core.h
> index f57e296..46159b2 100644
> --- a/drivers/gpu/drm/rockchip/cdn-dp-core.h
> +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.h
> @@ -78,6 +78,7 @@ struct cdn_dp_device {
> struct platform_device *audio_pdev;
> struct work_struct event_work;
> struct edid *edid;
> + struct drm_dp_aux aux;
>
> struct mutex lock;
> bool connected;
> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-reg.c b/drivers/gpu/drm/rockchip/cdn-dp-reg.c
> index eb3042c..b2f532a 100644
> --- a/drivers/gpu/drm/rockchip/cdn-dp-reg.c
> +++ b/drivers/gpu/drm/rockchip/cdn-dp-reg.c
> @@ -221,7 +221,11 @@ static int cdn_dp_reg_write_bit(struct cdn_dp_device *dp, u16 addr,
> sizeof(field), field);
> }
>
> -int cdn_dp_dpcd_read(struct cdn_dp_device *dp, u32 addr, u8 *data, u16 len)
> +/*
> + * Returns the number of bytes transferred on success, or a negative error
> + * code on failure. -ETIMEDOUT is returned if mailbox message not send success;
> + */
Returns the number of bytes or -ETIMEDOUT, no other negative errors
can be returned, right?
I am not English native but the last phrase sounds incorrect to me,
I'd rephrase it: (open to suggestions)
-ETIMEDOUT if fails to receive the mailbox message.
> +ssize_t cdn_dp_dpcd_read(struct cdn_dp_device *dp, u32 addr, u8 *data, u16 len)
> {
> u8 msg[5], reg[5];
> int ret;
> @@ -247,24 +251,40 @@ int cdn_dp_dpcd_read(struct cdn_dp_device *dp, u32 addr, u8 *data, u16 len)
> goto err_dpcd_read;
>
> ret = cdn_dp_mailbox_read_receive(dp, data, len);
> + if (!ret)
> + return len;
>
> err_dpcd_read:
> + DRM_DEV_ERROR(dp->dev, "dpcd read failed: %d\n", ret);
> return ret;
> }
>
> -int cdn_dp_dpcd_write(struct cdn_dp_device *dp, u32 addr, u8 value)
> +#define CDN_AUX_HEADER_SIZE 5
> +#define CDN_AUX_MSG_SIZE 20
> +/*
> + * Returns the number of bytes transferred on success, or a negative error
> + * code on failure. -ETIMEDOUT is returned if mailbox message not send success;
Same as above. Sounds incorrect to me.
> + * -EINVAL is return if get the wrong data size after message send.
Same here.
> + */
> +ssize_t cdn_dp_dpcd_write(struct cdn_dp_device *dp, u32 addr, u8 *data, u16 len)
> {
> - u8 msg[6], reg[5];
> + u8 msg[CDN_AUX_MSG_SIZE + CDN_AUX_HEADER_SIZE];
> + u8 reg[CDN_AUX_HEADER_SIZE];
> int ret;
>
> - msg[0] = 0;
> - msg[1] = 1;
> + if (WARN_ON(len > CDN_AUX_MSG_SIZE) || WARN_ON(len <= 0))
> + return -EINVAL;
> +
> + msg[0] = (len >> 8) & 0xff;
> + msg[1] = len & 0xff;
> msg[2] = (addr >> 16) & 0xff;
> msg[3] = (addr >> 8) & 0xff;
> msg[4] = addr & 0xff;
> - msg[5] = value;
> +
> + memcpy(msg + CDN_AUX_HEADER_SIZE, data, len);
> +
> ret = cdn_dp_mailbox_send(dp, MB_MODULE_ID_DP_TX, DPTX_WRITE_DPCD,
> - sizeof(msg), msg);
> + CDN_AUX_HEADER_SIZE + len, msg);
> if (ret)
> goto err_dpcd_write;
>
> @@ -277,8 +297,12 @@ int cdn_dp_dpcd_write(struct cdn_dp_device *dp, u32 addr, u8 value)
> if (ret)
> goto err_dpcd_write;
>
> - if (addr != (reg[2] << 16 | reg[3] << 8 | reg[4]))
> + if ((len != (reg[0] << 8 | reg[1])) ||
> + (addr != (reg[2] << 16 | reg[3] << 8 | reg[4]))) {
> ret = -EINVAL;
> + } else {
> + return len;
> + }
>
> err_dpcd_write:
> if (ret)
> @@ -286,6 +310,32 @@ int cdn_dp_dpcd_write(struct cdn_dp_device *dp, u32 addr, u8 value)
> return ret;
> }
>
> +int cdn_dp_get_aux_status(struct cdn_dp_device *dp)
> +{
> + u8 status;
> + int ret;
> +
> + ret = cdn_dp_mailbox_send(dp, MB_MODULE_ID_DP_TX, DPTX_GET_LAST_AUX_STAUS,
> + 0, NULL);
> + if (ret)
> + goto err_get_hpd;
> +
> + ret = cdn_dp_mailbox_validate_receive(dp, MB_MODULE_ID_DP_TX,
> + DPTX_GET_LAST_AUX_STAUS, sizeof(status));
> + if (ret)
> + goto err_get_hpd;
> +
> + ret = cdn_dp_mailbox_read_receive(dp, &status, sizeof(status));
> + if (ret)
> + goto err_get_hpd;
> +
> + return status;
> +
> +err_get_hpd:
> + DRM_DEV_ERROR(dp->dev, "get aux status failed: %d\n", ret);
> + return ret;
> +}
> +
> int cdn_dp_load_firmware(struct cdn_dp_device *dp, const u32 *i_mem,
> u32 i_size, const u32 *d_mem, u32 d_size)
> {
> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-reg.h b/drivers/gpu/drm/rockchip/cdn-dp-reg.h
> index c4bbb4a83..aedf2dc 100644
> --- a/drivers/gpu/drm/rockchip/cdn-dp-reg.h
> +++ b/drivers/gpu/drm/rockchip/cdn-dp-reg.h
> @@ -328,6 +328,13 @@
> #define GENERAL_BUS_SETTINGS 0x03
> #define GENERAL_TEST_ACCESS 0x04
>
> +/* AUX status*/
> +#define AUX_STAUS_ACK 0
> +#define AUX_STAUS_NACK 1
> +#define AUX_STAUS_DEFER 2
> +#define AUX_STAUS_SINK_ERROR 3
> +#define AUX_STAUS_BUS_ERROR 4
> +
For the five defines, s/STAUS/STATUS/
> #define DPTX_SET_POWER_MNG 0x00
> #define DPTX_SET_HOST_CAPABILITIES 0x01
> #define DPTX_GET_EDID 0x02
> @@ -469,8 +476,11 @@ int cdn_dp_set_host_cap(struct cdn_dp_device *dp, u8 lanes, bool flip);
> int cdn_dp_event_config(struct cdn_dp_device *dp);
> u32 cdn_dp_get_event(struct cdn_dp_device *dp);
> int cdn_dp_get_hpd_status(struct cdn_dp_device *dp);
> -int cdn_dp_dpcd_write(struct cdn_dp_device *dp, u32 addr, u8 value);
> -int cdn_dp_dpcd_read(struct cdn_dp_device *dp, u32 addr, u8 *data, u16 len);
> +ssize_t cdn_dp_dpcd_write(struct cdn_dp_device *dp, u32 addr,
> + u8 *data, u16 len);
> +ssize_t cdn_dp_dpcd_read(struct cdn_dp_device *dp, u32 addr,
> + u8 *data, u16 len);
> +int cdn_dp_get_aux_status(struct cdn_dp_device *dp);
> int cdn_dp_get_edid_block(void *dp, u8 *edid,
> unsigned int block, size_t length);
> int cdn_dp_train_link(struct cdn_dp_device *dp);
> --
> 2.7.4
>
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply
* [PATCH v4 5/9] iio: adc: at91-sama5d2_adc: add support for position and pressure channels
From: Eugen Hristev @ 2018-05-07 10:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180507102810.GO10960@piout.net>
On 07.05.2018 13:28, Alexandre Belloni wrote:
> On 07/05/2018 09:18:39+0300, Eugen Hristev wrote:
>> On 06.05.2018 20:59, Alexandre Belloni wrote:
>>> Hi,
>>>
>>> On 06/05/2018 18:29:53+0100, Jonathan Cameron wrote:
>>>> On Mon, 30 Apr 2018 13:32:11 +0300
>>>> Eugen Hristev <eugen.hristev@microchip.com> wrote:
>>>>
>>>>> This implements the support for position and pressure for the included
>>>>> touchscreen support in the SAMA5D2 SOC ADC block.
>>>>> Two position channels are added and one for pressure.
>>>>> They can be read in raw format, or through a buffer.
>>>>> A normal use case is for a consumer driver to register a callback buffer
>>>>> for these channels.
>>>>> When the touchscreen channels are in the active scan mask,
>>>>> the driver will start the touchscreen sampling and push the data to the
>>>>> buffer.
>>>>>
>>>>> Some parts of this patch are based on initial original work by
>>>>> Mohamed Jamsheeth Hajanajubudeen and Bandaru Venkateswara Swamy
>>>>>
>>>>> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
>>>> Looks good to me now.
>>>>
>>>> I'm assuming that once Dmitry and others are happy, I'll take the
>>>> series through the IIO tree. Will reply to the cover letter if the
>>>> rest of the patches look good to me to let everyone know that without
>>>> having to catch this comment down in here!
>>>>
>>>
>>> I'm planning to take both DT patches through the at91 tree once you take
>>> the DT bindings patches.
>>
>> Please take into consideration that those DT patches do not build
>> stand-alone, they depend on
>> [PATCH v4 7/9] dt-bindings: iio: adc: at91-sama5d2_adc: add channel specific
>> consumer info
>>
>> (the DT patches add an include statement of a file which is created in this
>> patch).
>>
>
> So the proper way is to actually have the values in the dt instead of
> the define and then patch it on the next version of the kernel.
>
> Or we take the dts patches on the next version.
Taking the DTS patches on the next version is fine for me.
Thanks
>
^ permalink raw reply
* dma-debug cleanups, including removing the arch hook
From: Christoph Hellwig @ 2018-05-07 10:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180424140235.9125-1-hch@lst.de>
Any comments? I'd like to move forward with this rather sooner
than later, so any reviews welcome!
On Tue, Apr 24, 2018 at 04:02:32PM +0200, Christoph Hellwig wrote:
> Hi all,
>
> this series has a few dma-debug cleanups, most notably removing the need
> for architectures to explicitly initialize dma-debug.
> _______________________________________________
> iommu mailing list
> iommu at lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
---end quoted text---
^ permalink raw reply
* [PATCH v4 5/9] iio: adc: at91-sama5d2_adc: add support for position and pressure channels
From: Alexandre Belloni @ 2018-05-07 10:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <eaeced9e-390f-0eea-0466-66c789bacdd6@microchip.com>
On 07/05/2018 09:18:39+0300, Eugen Hristev wrote:
> On 06.05.2018 20:59, Alexandre Belloni wrote:
> > Hi,
> >
> > On 06/05/2018 18:29:53+0100, Jonathan Cameron wrote:
> > > On Mon, 30 Apr 2018 13:32:11 +0300
> > > Eugen Hristev <eugen.hristev@microchip.com> wrote:
> > >
> > > > This implements the support for position and pressure for the included
> > > > touchscreen support in the SAMA5D2 SOC ADC block.
> > > > Two position channels are added and one for pressure.
> > > > They can be read in raw format, or through a buffer.
> > > > A normal use case is for a consumer driver to register a callback buffer
> > > > for these channels.
> > > > When the touchscreen channels are in the active scan mask,
> > > > the driver will start the touchscreen sampling and push the data to the
> > > > buffer.
> > > >
> > > > Some parts of this patch are based on initial original work by
> > > > Mohamed Jamsheeth Hajanajubudeen and Bandaru Venkateswara Swamy
> > > >
> > > > Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
> > > Looks good to me now.
> > >
> > > I'm assuming that once Dmitry and others are happy, I'll take the
> > > series through the IIO tree. Will reply to the cover letter if the
> > > rest of the patches look good to me to let everyone know that without
> > > having to catch this comment down in here!
> > >
> >
> > I'm planning to take both DT patches through the at91 tree once you take
> > the DT bindings patches.
>
> Please take into consideration that those DT patches do not build
> stand-alone, they depend on
> [PATCH v4 7/9] dt-bindings: iio: adc: at91-sama5d2_adc: add channel specific
> consumer info
>
> (the DT patches add an include statement of a file which is created in this
> patch).
>
So the proper way is to actually have the values in the dt instead of
the define and then patch it on the next version of the kernel.
Or we take the dts patches on the next version.
--
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* [PATCH] tee: shm: fix use-after-free via temporarily dropped reference
From: Jens Wiklander @ 2018-05-07 9:58 UTC (permalink / raw)
To: linux-arm-kernel
From: Jann Horn <jannh@google.com>
Bump the file's refcount before moving the reference into the fd table,
not afterwards. The old code could drop the file's refcount to zero for a
short moment before calling get_file() via get_dma_buf().
This code can only be triggered on ARM systems that use Linaro's OP-TEE.
Fixes: 967c9cca2cc5 ("tee: generic TEE subsystem")
Signed-off-by: Jann Horn <jannh@google.com>
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
---
drivers/tee/tee_shm.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c
index 556960a1bab3..07d3be6f0780 100644
--- a/drivers/tee/tee_shm.c
+++ b/drivers/tee/tee_shm.c
@@ -360,9 +360,10 @@ int tee_shm_get_fd(struct tee_shm *shm)
if (!(shm->flags & TEE_SHM_DMA_BUF))
return -EINVAL;
+ get_dma_buf(shm->dmabuf);
fd = dma_buf_fd(shm->dmabuf, O_CLOEXEC);
- if (fd >= 0)
- get_dma_buf(shm->dmabuf);
+ if (fd < 0)
+ dma_buf_put(shm->dmabuf);
return fd;
}
--
2.17.0
^ permalink raw reply related
* [PATCH] locking/atomics: Simplify the op definitions in atomic.h some more
From: Andrea Parri @ 2018-05-07 9:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180506145726.y4jxhvfolzvbuft5@gmail.com>
On Sun, May 06, 2018 at 04:57:27PM +0200, Ingo Molnar wrote:
>
> * Andrea Parri <andrea.parri@amarulasolutions.com> wrote:
>
> > Hi Ingo,
> >
> > > From 5affbf7e91901143f84f1b2ca64f4afe70e210fd Mon Sep 17 00:00:00 2001
> > > From: Ingo Molnar <mingo@kernel.org>
> > > Date: Sat, 5 May 2018 10:23:23 +0200
> > > Subject: [PATCH] locking/atomics: Simplify the op definitions in atomic.h some more
> > >
> > > Before:
> > >
> > > #ifndef atomic_fetch_dec_relaxed
> > > # ifndef atomic_fetch_dec
> > > # define atomic_fetch_dec(v) atomic_fetch_sub(1, (v))
> > > # define atomic_fetch_dec_relaxed(v) atomic_fetch_sub_relaxed(1, (v))
> > > # define atomic_fetch_dec_acquire(v) atomic_fetch_sub_acquire(1, (v))
> > > # define atomic_fetch_dec_release(v) atomic_fetch_sub_release(1, (v))
> > > # else
> > > # define atomic_fetch_dec_relaxed atomic_fetch_dec
> > > # define atomic_fetch_dec_acquire atomic_fetch_dec
> > > # define atomic_fetch_dec_release atomic_fetch_dec
> > > # endif
> > > #else
> > > # ifndef atomic_fetch_dec_acquire
> > > # define atomic_fetch_dec_acquire(...) __atomic_op_acquire(atomic_fetch_dec, __VA_ARGS__)
> > > # endif
> > > # ifndef atomic_fetch_dec_release
> > > # define atomic_fetch_dec_release(...) __atomic_op_release(atomic_fetch_dec, __VA_ARGS__)
> > > # endif
> > > # ifndef atomic_fetch_dec
> > > # define atomic_fetch_dec(...) __atomic_op_fence(atomic_fetch_dec, __VA_ARGS__)
> > > # endif
> > > #endif
> > >
> > > After:
> > >
> > > #ifndef atomic_fetch_dec_relaxed
> > > # ifndef atomic_fetch_dec
> > > # define atomic_fetch_dec(v) atomic_fetch_sub(1, (v))
> > > # define atomic_fetch_dec_relaxed(v) atomic_fetch_sub_relaxed(1, (v))
> > > # define atomic_fetch_dec_acquire(v) atomic_fetch_sub_acquire(1, (v))
> > > # define atomic_fetch_dec_release(v) atomic_fetch_sub_release(1, (v))
> > > # else
> > > # define atomic_fetch_dec_relaxed atomic_fetch_dec
> > > # define atomic_fetch_dec_acquire atomic_fetch_dec
> > > # define atomic_fetch_dec_release atomic_fetch_dec
> > > # endif
> > > #else
> > > # ifndef atomic_fetch_dec
> > > # define atomic_fetch_dec(...) __atomic_op_fence(atomic_fetch_dec, __VA_ARGS__)
> > > # define atomic_fetch_dec_acquire(...) __atomic_op_acquire(atomic_fetch_dec, __VA_ARGS__)
> > > # define atomic_fetch_dec_release(...) __atomic_op_release(atomic_fetch_dec, __VA_ARGS__)
> > > # endif
> > > #endif
> > >
> > > The idea is that because we already group these APIs by certain defines
> > > such as atomic_fetch_dec_relaxed and atomic_fetch_dec in the primary
> > > branches - we can do the same in the secondary branch as well.
> > >
> > > ( Also remove some unnecessarily duplicate comments, as the API
> > > group defines are now pretty much self-documenting. )
> > >
> > > No change in functionality.
> > >
> > > Cc: Peter Zijlstra <peterz@infradead.org>
> > > Cc: Linus Torvalds <torvalds@linux-foundation.org>
> > > Cc: Andrew Morton <akpm@linux-foundation.org>
> > > Cc: Thomas Gleixner <tglx@linutronix.de>
> > > Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > > Cc: Will Deacon <will.deacon@arm.com>
> > > Cc: linux-kernel at vger.kernel.org
> > > Signed-off-by: Ingo Molnar <mingo@kernel.org>
> >
> > This breaks compilation on RISC-V. (For some of its atomics, the arch
> > currently defines the _relaxed and the full variants and it relies on
> > the generic definitions for the _acquire and the _release variants.)
>
> I don't have cross-compilation for RISC-V, which is a relatively new arch.
> (Is there any RISC-V set of cross-compilation tools on kernel.org somewhere?)
I'm using the toolchain from:
https://riscv.org/software-tools/
(adding Palmer and Albert in Cc:)
>
> Could you please send a patch that defines those variants against Linus's tree,
> like the PowerPC patch that does something similar:
>
> 0476a632cb3a: locking/atomics/powerpc: Move cmpxchg helpers to asm/cmpxchg.h and define the full set of cmpxchg APIs
>
> ?
Yes, please see below for a first RFC.
(BTW, get_maintainer.pl says that that patch missed Benjamin, Paul, Michael
and linuxppc-dev at lists.ozlabs.org: FWIW, I'm Cc-ing the maintainers here.)
Andrea
>From 411f05a44e0b53a435331b977ff864fba7501a95 Mon Sep 17 00:00:00 2001
From: Andrea Parri <andrea.parri@amarulasolutions.com>
Date: Mon, 7 May 2018 10:59:20 +0200
Subject: [RFC PATCH] riscv/atomic: Defines _acquire/_release variants
In preparation for Ingo's renovation of the generic atomic.h header [1],
define the _acquire/_release variants in the arch's header.
No change in code generation.
[1] http://lkml.kernel.org/r/20180505081100.nsyrqrpzq2vd27bk at gmail.com
http://lkml.kernel.org/r/20180505083635.622xmcvb42dw5xxh at gmail.com
Suggested-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Andrea Parri <andrea.parri@amarulasolutions.com>
Cc: Palmer Dabbelt <palmer@sifive.com>
Cc: Albert Ou <albert@sifive.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Boqun Feng <boqun.feng@gmail.com>
Cc: linux-riscv at lists.infradead.org
---
arch/riscv/include/asm/atomic.h | 88 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 88 insertions(+)
diff --git a/arch/riscv/include/asm/atomic.h b/arch/riscv/include/asm/atomic.h
index 855115ace98c8..7cbd8033dfb5d 100644
--- a/arch/riscv/include/asm/atomic.h
+++ b/arch/riscv/include/asm/atomic.h
@@ -153,22 +153,54 @@ ATOMIC_OPS(sub, add, +, -i)
#define atomic_add_return_relaxed atomic_add_return_relaxed
#define atomic_sub_return_relaxed atomic_sub_return_relaxed
+#define atomic_add_return_acquire(...) \
+ __atomic_op_acquire(atomic_add_return, __VA_ARGS__)
+#define atomic_sub_return_acquire(...) \
+ __atomic_op_acquire(atomic_sub_return, __VA_ARGS__)
+#define atomic_add_return_release(...) \
+ __atomic_op_release(atomic_add_return, __VA_ARGS__)
+#define atomic_sub_return_release(...) \
+ __atomic_op_release(atomic_sub_return, __VA_ARGS__)
#define atomic_add_return atomic_add_return
#define atomic_sub_return atomic_sub_return
#define atomic_fetch_add_relaxed atomic_fetch_add_relaxed
#define atomic_fetch_sub_relaxed atomic_fetch_sub_relaxed
+#define atomic_fetch_add_acquire(...) \
+ __atomic_op_acquire(atomic_fetch_add, __VA_ARGS__)
+#define atomic_fetch_sub_acquire(...) \
+ __atomic_op_acquire(atomic_fetch_sub, __VA_ARGS__)
+#define atomic_fetch_add_release(...) \
+ __atomic_op_release(atomic_fetch_add, __VA_ARGS__)
+#define atomic_fetch_sub_release(...) \
+ __atomic_op_release(atomic_fetch_sub, __VA_ARGS__)
#define atomic_fetch_add atomic_fetch_add
#define atomic_fetch_sub atomic_fetch_sub
#ifndef CONFIG_GENERIC_ATOMIC64
#define atomic64_add_return_relaxed atomic64_add_return_relaxed
#define atomic64_sub_return_relaxed atomic64_sub_return_relaxed
+#define atomic64_add_return_acquire(...) \
+ __atomic_op_acquire(atomic64_add_return, __VA_ARGS__)
+#define atomic64_sub_return_acquire(...) \
+ __atomic_op_acquire(atomic64_sub_return, __VA_ARGS__)
+#define atomic64_add_return_release(...) \
+ __atomic_op_release(atomic64_add_return, __VA_ARGS__)
+#define atomic64_sub_return_release(...) \
+ __atomic_op_release(atomic64_sub_return, __VA_ARGS__)
#define atomic64_add_return atomic64_add_return
#define atomic64_sub_return atomic64_sub_return
#define atomic64_fetch_add_relaxed atomic64_fetch_add_relaxed
#define atomic64_fetch_sub_relaxed atomic64_fetch_sub_relaxed
+#define atomic64_fetch_add_acquire(...) \
+ __atomic_op_acquire(atomic64_fetch_add, __VA_ARGS__)
+#define atomic64_fetch_sub_acquire(...) \
+ __atomic_op_acquire(atomic64_fetch_sub, __VA_ARGS__)
+#define atomic64_fetch_add_release(...) \
+ __atomic_op_release(atomic64_fetch_add, __VA_ARGS__)
+#define atomic64_fetch_sub_release(...) \
+ __atomic_op_release(atomic64_fetch_sub, __VA_ARGS__)
#define atomic64_fetch_add atomic64_fetch_add
#define atomic64_fetch_sub atomic64_fetch_sub
#endif
@@ -191,6 +223,18 @@ ATOMIC_OPS(xor, xor, i)
#define atomic_fetch_and_relaxed atomic_fetch_and_relaxed
#define atomic_fetch_or_relaxed atomic_fetch_or_relaxed
#define atomic_fetch_xor_relaxed atomic_fetch_xor_relaxed
+#define atomic_fetch_and_acquire(...) \
+ __atomic_op_acquire(atomic_fetch_and, __VA_ARGS__)
+#define atomic_fetch_or_acquire(...) \
+ __atomic_op_acquire(atomic_fetch_or, __VA_ARGS__)
+#define atomic_fetch_xor_acquire(...) \
+ __atomic_op_acquire(atomic_fetch_xor, __VA_ARGS__)
+#define atomic_fetch_and_release(...) \
+ __atomic_op_release(atomic_fetch_and, __VA_ARGS__)
+#define atomic_fetch_or_release(...) \
+ __atomic_op_release(atomic_fetch_or, __VA_ARGS__)
+#define atomic_fetch_xor_release(...) \
+ __atomic_op_release(atomic_fetch_xor, __VA_ARGS__)
#define atomic_fetch_and atomic_fetch_and
#define atomic_fetch_or atomic_fetch_or
#define atomic_fetch_xor atomic_fetch_xor
@@ -199,6 +243,18 @@ ATOMIC_OPS(xor, xor, i)
#define atomic64_fetch_and_relaxed atomic64_fetch_and_relaxed
#define atomic64_fetch_or_relaxed atomic64_fetch_or_relaxed
#define atomic64_fetch_xor_relaxed atomic64_fetch_xor_relaxed
+#define atomic64_fetch_and_acquire(...) \
+ __atomic_op_acquire(atomic64_fetch_and, __VA_ARGS__)
+#define atomic64_fetch_or_acquire(...) \
+ __atomic_op_acquire(atomic64_fetch_or, __VA_ARGS__)
+#define atomic64_fetch_xor_acquire(...) \
+ __atomic_op_acquire(atomic64_fetch_xor, __VA_ARGS__)
+#define atomic64_fetch_and_release(...) \
+ __atomic_op_release(atomic64_fetch_and, __VA_ARGS__)
+#define atomic64_fetch_or_release(...) \
+ __atomic_op_release(atomic64_fetch_or, __VA_ARGS__)
+#define atomic64_fetch_xor_release(...) \
+ __atomic_op_release(atomic64_fetch_xor, __VA_ARGS__)
#define atomic64_fetch_and atomic64_fetch_and
#define atomic64_fetch_or atomic64_fetch_or
#define atomic64_fetch_xor atomic64_fetch_xor
@@ -290,22 +346,54 @@ ATOMIC_OPS(dec, add, +, -1)
#define atomic_inc_return_relaxed atomic_inc_return_relaxed
#define atomic_dec_return_relaxed atomic_dec_return_relaxed
+#define atomic_inc_return_acquire(...) \
+ __atomic_op_acquire(atomic_inc_return, __VA_ARGS__)
+#define atomic_dec_return_acquire(...) \
+ __atomic_op_acquire(atomic_dec_return, __VA_ARGS__)
+#define atomic_inc_return_release(...) \
+ __atomic_op_release(atomic_inc_return, __VA_ARGS__)
+#define atomic_dec_return_release(...) \
+ __atomic_op_release(atomic_dec_return, __VA_ARGS__)
#define atomic_inc_return atomic_inc_return
#define atomic_dec_return atomic_dec_return
#define atomic_fetch_inc_relaxed atomic_fetch_inc_relaxed
#define atomic_fetch_dec_relaxed atomic_fetch_dec_relaxed
+#define atomic_fetch_inc_acquire(...) \
+ __atomic_op_acquire(atomic_fetch_inc, __VA_ARGS__)
+#define atomic_fetch_dec_acquire(...) \
+ __atomic_op_acquire(atomic_fetch_dec, __VA_ARGS__)
+#define atomic_fetch_inc_release(...) \
+ __atomic_op_release(atomic_fetch_inc, __VA_ARGS__)
+#define atomic_fetch_dec_release(...) \
+ __atomic_op_release(atomic_fetch_dec, __VA_ARGS__)
#define atomic_fetch_inc atomic_fetch_inc
#define atomic_fetch_dec atomic_fetch_dec
#ifndef CONFIG_GENERIC_ATOMIC64
#define atomic64_inc_return_relaxed atomic64_inc_return_relaxed
#define atomic64_dec_return_relaxed atomic64_dec_return_relaxed
+#define atomic64_inc_return_acquire(...) \
+ __atomic_op_acquire(atomic64_inc_return, __VA_ARGS__)
+#define atomic64_dec_return_acquire(...) \
+ __atomic_op_acquire(atomic64_dec_return, __VA_ARGS__)
+#define atomic64_inc_return_release(...) \
+ __atomic_op_release(atomic64_inc_return, __VA_ARGS__)
+#define atomic64_dec_return_release(...) \
+ __atomic_op_release(atomic64_dec_return, __VA_ARGS__)
#define atomic64_inc_return atomic64_inc_return
#define atomic64_dec_return atomic64_dec_return
#define atomic64_fetch_inc_relaxed atomic64_fetch_inc_relaxed
#define atomic64_fetch_dec_relaxed atomic64_fetch_dec_relaxed
+#define atomic64_fetch_inc_acquire(...) \
+ __atomic_op_acquire(atomic64_fetch_inc, __VA_ARGS__)
+#define atomic64_fetch_dec_acquire(...) \
+ __atomic_op_acquire(atomic64_fetch_dec, __VA_ARGS__)
+#define atomic64_fetch_inc_release(...) \
+ __atomic_op_release(atomic64_fetch_inc, __VA_ARGS__)
+#define atomic64_fetch_dec_release(...) \
+ __atomic_op_release(atomic64_fetch_dec, __VA_ARGS__)
#define atomic64_fetch_inc atomic64_fetch_inc
#define atomic64_fetch_dec atomic64_fetch_dec
#endif
--
2.7.4
>
> ... and I'll integrate it into the proper place to make it all bisectable, etc.
>
> Thanks,
>
> Ingo
^ permalink raw reply related
* [PATCH v2 03/13] drm/kms/mode/exynos-dsi: using helper func drm_display_mode_to_videomode for calculating timing parameters
From: Andrzej Hajda @ 2018-05-07 9:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1525663938-4172-1-git-send-email-satendra.t@samsung.com>
Hi Satendra Singh Thakur,
On 07.05.2018 05:32, Satendra Singh Thakur wrote:
> To avoid duplicate logic for the same
>
> Signed-off-by: Satendra Singh Thakur <satendra.t@samsung.com>
> Acked-by: Madhur Verma <madhur.verma@samsung.com>
> Cc: Hemanshu Srivastava <hemanshu.s@samsung.com>
Whole exynos_dsi_mode_set callback is redundant, so I have posted patch
removing it [1], so this patch can be dropped.
[1]: https://marc.info/?l=dri-devel&m=152568538400712
Regards
Andrzej
> ---
>
> v2: Removed Mr Robin from reviewed-by field
>
> drivers/gpu/drm/exynos/exynos_drm_dsi.c | 13 ++-----------
> 1 file changed, 2 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> index 7904ffa..7fe84fd 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> @@ -1490,17 +1490,8 @@ static void exynos_dsi_mode_set(struct drm_encoder *encoder,
> struct drm_display_mode *adjusted_mode)
> {
> struct exynos_dsi *dsi = encoder_to_dsi(encoder);
> - struct videomode *vm = &dsi->vm;
> - struct drm_display_mode *m = adjusted_mode;
> -
> - vm->hactive = m->hdisplay;
> - vm->vactive = m->vdisplay;
> - vm->vfront_porch = m->vsync_start - m->vdisplay;
> - vm->vback_porch = m->vtotal - m->vsync_end;
> - vm->vsync_len = m->vsync_end - m->vsync_start;
> - vm->hfront_porch = m->hsync_start - m->hdisplay;
> - vm->hback_porch = m->htotal - m->hsync_end;
> - vm->hsync_len = m->hsync_end - m->hsync_start;
> +
> + drm_display_mode_to_videomode(adjusted_mode, &dsi->vm);
> }
>
> static const struct drm_encoder_helper_funcs exynos_dsi_encoder_helper_funcs = {
^ permalink raw reply
* [PATCH v2] tty: implement led triggers
From: Johan Hovold @ 2018-05-07 9:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180507084127.ekpd3ze2itkzo7fd@pengutronix.de>
On Mon, May 07, 2018 at 10:41:27AM +0200, Uwe Kleine-K?nig wrote:
> Hello Johan,
>
> thanks for your feedback.
>
> On Mon, May 07, 2018 at 10:02:52AM +0200, Johan Hovold wrote:
> > On Thu, May 03, 2018 at 10:19:52PM +0200, Uwe Kleine-K?nig wrote:
> > > The rx trigger fires when data is pushed to the ldisc. This is a bit later
> > > than the actual receiving of data but has the nice benefit that it
> > > doesn't need adaption for each driver and isn't in the hot path.
> > >
> > > Similarily the tx trigger fires when data taken from the ldisc.
> >
> > You meant copied from user space, or written to the ldisc, here.
>
> ack.
>
> > Note that the rx-path is shared with serdev, but the write path is not.
> > So with this series, serdev devices would only trigger the rx-led.
>
> Where would be the right place to put the tx trigger to catch serdev,
> too?
I haven't given this much thought, but do we really want this for serdev
at all? I'm thinking whatever driver or subsystem is using serdev should
have their own triggers (e.g. bluetooth or net).
So it might be better to move the rx-blinking to the default tty-port
client receive_buf callback instead (i.e.
tty_port_default_receive_buf()).
And then the resource allocations would need to go after the serdev
registration in tty_port_register_device_attr_serdev().
> > > Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
> > > ---
> > > Changes since v1, sent with Message-Id:
> > > 20180503100448.1350-1-u.kleine-koenig at pengutronix.de:
> > >
> > > - implement tx trigger;
> > > - introduce Kconfig symbol for conditional compilation;
> > > - set trigger to NULL if allocating the name failed to not free random
> > > pointers in case the port struct wasn't zeroed;
> > > - use if/else instead of goto
> >
> > > @@ -499,6 +500,7 @@ static void flush_to_ldisc(struct work_struct *work)
> > > struct tty_buffer *head = buf->head;
> > > struct tty_buffer *next;
> > > int count;
> > > + unsigned long delay = 50 /* ms */;
> >
> > Comment after the semicolon?
>
> Given that this comment is about the 50 and not the delay member, I
> prefer it before the ;.
Hmm. I personally find it hard to read and can only find about 30
instances of this comment style (for assignments) in the kernel. And
arguably the comment applies equally well to the delay variable in this
case too.
> > Besides the ugly ifdefs here, you're leaking the above LED resources in
> > the error paths below (e.g. on probe deferrals).
> ack, the ifdevs are ugly. I'm working on an idea to get rid of them.
Sounds good.
Thanks,
Johan
^ permalink raw reply
* [PATCH v2] tty: implement led triggers
From: Uwe Kleine-König @ 2018-05-07 8:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180507080252.GO2285@localhost>
Hello Johan,
thanks for your feedback.
On Mon, May 07, 2018 at 10:02:52AM +0200, Johan Hovold wrote:
> On Thu, May 03, 2018 at 10:19:52PM +0200, Uwe Kleine-K?nig wrote:
> > The rx trigger fires when data is pushed to the ldisc. This is a bit later
> > than the actual receiving of data but has the nice benefit that it
> > doesn't need adaption for each driver and isn't in the hot path.
> >
> > Similarily the tx trigger fires when data taken from the ldisc.
>
> You meant copied from user space, or written to the ldisc, here.
ack.
> Note that the rx-path is shared with serdev, but the write path is not.
> So with this series, serdev devices would only trigger the rx-led.
Where would be the right place to put the tx trigger to catch serdev,
too?
> > Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
> > ---
> > Changes since v1, sent with Message-Id:
> > 20180503100448.1350-1-u.kleine-koenig at pengutronix.de:
> >
> > - implement tx trigger;
> > - introduce Kconfig symbol for conditional compilation;
> > - set trigger to NULL if allocating the name failed to not free random
> > pointers in case the port struct wasn't zeroed;
> > - use if/else instead of goto
>
> > @@ -499,6 +500,7 @@ static void flush_to_ldisc(struct work_struct *work)
> > struct tty_buffer *head = buf->head;
> > struct tty_buffer *next;
> > int count;
> > + unsigned long delay = 50 /* ms */;
>
> Comment after the semicolon?
Given that this comment is about the 50 and not the delay member, I
prefer it before the ;.
> >
> > /* Ldisc or user is trying to gain exclusive access */
> > if (atomic_read(&buf->priority))
>
> > diff --git a/drivers/tty/tty_port.c b/drivers/tty/tty_port.c
> > index 25d736880013..f042879a597c 100644
> > --- a/drivers/tty/tty_port.c
> > +++ b/drivers/tty/tty_port.c
> > @@ -16,6 +16,7 @@
> > #include <linux/wait.h>
> > #include <linux/bitops.h>
> > #include <linux/delay.h>
> > +#include <linux/leds.h>
> > #include <linux/module.h>
> > #include <linux/serdev.h>
> >
> > @@ -157,6 +158,30 @@ struct device *tty_port_register_device_attr_serdev(struct tty_port *port,
> >
> > tty_port_link_device(port, driver, index);
> >
> > +#ifdef CONFIG_TTY_LEDS_TRIGGER
> > + port->led_trigger_rx_name = kasprintf(GFP_KERNEL, "%s%d-rx",
> > + driver->name, index);
> > + if (port->led_trigger_rx_name) {
> > + led_trigger_register_simple(port->led_trigger_rx_name,
> > + &port->led_trigger_rx);
> > + } else {
> > + port->led_trigger_rx = NULL;
> > + pr_err("Failed to allocate trigger name for %s%d\n",
> > + driver->name, index);
> > + }
> > +
> > + port->led_trigger_tx_name = kasprintf(GFP_KERNEL, "%s%d-tx",
> > + driver->name, index);
> > + if (port->led_trigger_tx_name) {
> > + led_trigger_register_simple(port->led_trigger_tx_name,
> > + &port->led_trigger_tx);
> > + } else {
> > + port->led_trigger_tx = NULL;
> > + pr_err("Failed to allocate trigger name for %s%d\n",
> > + driver->name, index);
> > + }
> > +#endif
>
> Besides the ugly ifdefs here, you're leaking the above LED resources in
> the error paths below (e.g. on probe deferrals).
ack, the ifdevs are ugly. I'm working on an idea to get rid of them.
Will think about how to plug the leak.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* [PATCH 2/2] ARM: dts: imx28-duckbill-2-enocean: Remove unnecessary #address/#size-cells
From: Stefan Wahren @ 2018-05-07 8:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1525655020-31260-2-git-send-email-festevam@gmail.com>
Am 07.05.2018 um 03:03 schrieb Fabio Estevam:
> From: Fabio Estevam <fabio.estevam@nxp.com>
>
> Remove unnecessary #address-cells/#size-cells to fix the following
> DTC warnings:
>
> arch/arm/boot/dts/imx28-duckbill-2-enocean.dtb: Warning (avoid_unnecessary_addr_size): /gpio-keys: unnecessary #address-cells/#size-cells without "ranges" or child "reg" propert
>
> Cc: Stefan Wahren <stefan.wahren@i2se.com>
> Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
Acked-by: Stefan Wahren <stefan.wahren@i2se.com>
Thanks
Stefan
^ permalink raw reply
* [PATCH v2] tty: implement led triggers
From: Johan Hovold @ 2018-05-07 8:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180503201952.16592-1-u.kleine-koenig@pengutronix.de>
On Thu, May 03, 2018 at 10:19:52PM +0200, Uwe Kleine-K?nig wrote:
> The rx trigger fires when data is pushed to the ldisc. This is a bit later
> than the actual receiving of data but has the nice benefit that it
> doesn't need adaption for each driver and isn't in the hot path.
>
> Similarily the tx trigger fires when data taken from the ldisc.
You meant copied from user space, or written to the ldisc, here.
Note that the rx-path is shared with serdev, but the write path is not.
So with this series, serdev devices would only trigger the rx-led.
> Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
> ---
> Changes since v1, sent with Message-Id:
> 20180503100448.1350-1-u.kleine-koenig at pengutronix.de:
>
> - implement tx trigger;
> - introduce Kconfig symbol for conditional compilation;
> - set trigger to NULL if allocating the name failed to not free random
> pointers in case the port struct wasn't zeroed;
> - use if/else instead of goto
> @@ -499,6 +500,7 @@ static void flush_to_ldisc(struct work_struct *work)
> struct tty_buffer *head = buf->head;
> struct tty_buffer *next;
> int count;
> + unsigned long delay = 50 /* ms */;
Comment after the semicolon?
>
> /* Ldisc or user is trying to gain exclusive access */
> if (atomic_read(&buf->priority))
> diff --git a/drivers/tty/tty_port.c b/drivers/tty/tty_port.c
> index 25d736880013..f042879a597c 100644
> --- a/drivers/tty/tty_port.c
> +++ b/drivers/tty/tty_port.c
> @@ -16,6 +16,7 @@
> #include <linux/wait.h>
> #include <linux/bitops.h>
> #include <linux/delay.h>
> +#include <linux/leds.h>
> #include <linux/module.h>
> #include <linux/serdev.h>
>
> @@ -157,6 +158,30 @@ struct device *tty_port_register_device_attr_serdev(struct tty_port *port,
>
> tty_port_link_device(port, driver, index);
>
> +#ifdef CONFIG_TTY_LEDS_TRIGGER
> + port->led_trigger_rx_name = kasprintf(GFP_KERNEL, "%s%d-rx",
> + driver->name, index);
> + if (port->led_trigger_rx_name) {
> + led_trigger_register_simple(port->led_trigger_rx_name,
> + &port->led_trigger_rx);
> + } else {
> + port->led_trigger_rx = NULL;
> + pr_err("Failed to allocate trigger name for %s%d\n",
> + driver->name, index);
> + }
> +
> + port->led_trigger_tx_name = kasprintf(GFP_KERNEL, "%s%d-tx",
> + driver->name, index);
> + if (port->led_trigger_tx_name) {
> + led_trigger_register_simple(port->led_trigger_tx_name,
> + &port->led_trigger_tx);
> + } else {
> + port->led_trigger_tx = NULL;
> + pr_err("Failed to allocate trigger name for %s%d\n",
> + driver->name, index);
> + }
> +#endif
Besides the ugly ifdefs here, you're leaking the above LED resources in
the error paths below (e.g. on probe deferrals).
> dev = serdev_tty_port_register(port, device, driver, index);
> if (PTR_ERR(dev) != -ENODEV) {
> /* Skip creating cdev if we registered a serdev device */
> @@ -249,6 +249,13 @@ struct tty_port {
> set to size of fifo */
> struct kref kref; /* Ref counter */
> void *client_data;
> +
> +#ifdef CONFIG_TTY_LEDS_TRIGGER
> + struct led_trigger *led_trigger_rx;
> + char *led_trigger_rx_name;
const?
> + struct led_trigger *led_trigger_tx;
> + char *led_trigger_tx_name;
ditto.
> +#endif
Johan
^ permalink raw reply
* [PATCH] arm: berlin: remove non-necessary flush_cache_all()
From: Jisheng Zhang @ 2018-05-07 7:44 UTC (permalink / raw)
To: linux-arm-kernel
I believe the flush_cache_all() after scu_enable() is to "Ensure that
the data accessed by CPU0 before the SCU was initialised is visible
to the other CPUs." as commented in scu_enable(). So flush_cache_all()
here is a duplication of the one in scu_enable(), remove it.
Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
---
arch/arm/mach-berlin/platsmp.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/arm/mach-berlin/platsmp.c b/arch/arm/mach-berlin/platsmp.c
index 7586b7aec272..a8ae4a566d99 100644
--- a/arch/arm/mach-berlin/platsmp.c
+++ b/arch/arm/mach-berlin/platsmp.c
@@ -81,7 +81,6 @@ static void __init berlin_smp_prepare_cpus(unsigned int max_cpus)
goto unmap_scu;
scu_enable(scu_base);
- flush_cache_all();
/*
* Write the first instruction the CPU will execute after being reset
--
2.17.0
^ permalink raw reply related
* [PATCH] arm: oxnas: remove non-necessary flush_cache_all()
From: Jisheng Zhang @ 2018-05-07 7:42 UTC (permalink / raw)
To: linux-arm-kernel
I believe the flush_cache_all() after scu_enable() is to "Ensure that
the data accessed by CPU0 before the SCU was initialised is visible
to the other CPUs." as commented in scu_enable(). So flush_cache_all()
here is a duplication of the one in scu_enable(), remove it.
Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
---
arch/arm/mach-oxnas/platsmp.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/arm/mach-oxnas/platsmp.c b/arch/arm/mach-oxnas/platsmp.c
index 442cc8a2f7dc..1a8e1e9fc222 100644
--- a/arch/arm/mach-oxnas/platsmp.c
+++ b/arch/arm/mach-oxnas/platsmp.c
@@ -85,7 +85,6 @@ static void __init ox820_smp_prepare_cpus(unsigned int max_cpus)
goto unmap_scu;
scu_enable(scu_base);
- flush_cache_all();
unmap_scu:
iounmap(scu_base);
--
2.17.0
^ permalink raw reply related
* [PATCH] arm: Convert arm boot_lock to raw
From: Sebastian Andrzej Siewior @ 2018-05-07 7:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180506182325.w7yvrpp3hcii54pj@kozik-lap>
On 2018-05-06 20:23:25 [+0200], Krzysztof Kozlowski wrote:
> > arch/arm/mach-actions/platsmp.c | 6 +++---
> > arch/arm/mach-exynos/platsmp.c | 12 ++++++------
>
> I assume this is needed for -rt patches?
correct.
> For Exynos:
> Acked-by: Krzysztof Kozlowski <krzk@kernel.org>
> Tested on Exynos5422 (Odroid HC1) with Linaro PM-QA:
> Tested-by: Krzysztof Kozlowski <krzk@kernel.org>
thank you.
> Best regards,
> Krzysztof
Sebastian
^ permalink raw reply
* [PATCH 7/7] ARM: dts: sun7i: Add dts file for the A20-linova1-7 HMI
From: Maxime Ripard @ 2018-05-07 7:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <afc78821-2d08-3f4f-d008-5b2d216e96c0@micronovasrl.com>
On Fri, May 04, 2018 at 11:52:59PM +0200, Giulio Benetti wrote:
> Hi Maxime!
>
> Il 04/05/2018 10:06, Maxime Ripard ha scritto:
> > Hi,
> >
> > On Wed, May 02, 2018 at 06:41:34PM +0200, Giulio Benetti wrote:
> > > > > You don't have to handcode the fragments anymore with the new syntax,
> > > > > and U-Boot makes it really trivial to use if you use the FIT image
> > > > > format to have multiple overlays bundled in the same image. You can
> > > > > choose to apply them dynamically, for example based on an EEPROM or
> > > > > some other metric to see which combination you have.
> > > >
> > > > Ah, this is interesting. I'm going to experiment with that.
> > > >
> > >
> > > I'm struggling against this, I don't really know how to proceed,
> > > except keeping monolithic dts files including other dtsi files.
> > >
> > > About dt-overlays I've tried to look around lot of time,
> > > but the only thing I've found is this:
> > > https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git/tree/arch/arm/boot/dts?h=topic/renesas-overlays
> > >
> > > where they use .dtso tagging them as "/plugin/;"
> > > and compile all .dtso found in dts folder.
> > > Then they obtain .dtbo files that should be the dt-overlays we have spoken
> > > about right?
> >
> > Yes. You don't have to do that though, you can just rely on dtc to
> > compile them, outside of the linux build system.
> >
> > > What I can't understand is if there's a real standard at this time to
> > > follow, because on renesas-driver they use their way to handle all .dtso
> > > files, but on mainline there seems to be nothing about that.
> >
> > I'm not sure what you mean here. It's just fragments of device tree,
> > that have to be compiled using dtc, that's it. You can use the Linux
> > build system infrastructure to do that, or you can build your own
> > simpler one. That's really up to you. See for example
> > https://github.com/NextThingCo/CHIP-dt-overlays/blob/master/Makefile
> >
> > (even though the overlays themselves use the legacy syntax and
> > shouldn't really be used an examples)
>
> Everything works now!
> Thank you very much!
> I've setted up a Repo on Github to give an example on how make it work with
> no pain:
> https://github.com/micronovasrl/linova-dtoverlays
>
> At the moment it's a mess all around, but it's working and give an idea on
> how to make it work. Though I'm going to clean it up well as a base for
> linova dtoverlays.
>
> Ah, btw, can you confirm me that base dts file must be compiled outside
> kernel with:
> dtc -@ ....
> Otherwise as in-tree dts with make dtbs "-@" argument is not passed.
> Right?
You should use DTC_FLAGS='-@'
Maxime
--
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180507/73b6662a/attachment.sig>
^ permalink raw reply
* [PATCH v9 06/11] arm64: kexec_file: allow for loading Image-format kernel
From: AKASHI Takahiro @ 2018-05-07 7:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <c604b578-6667-3532-eac9-55c428001025@arm.com>
James,
On Tue, May 01, 2018 at 06:46:11PM +0100, James Morse wrote:
> Hi Akashi,
>
> On 25/04/18 07:26, AKASHI Takahiro wrote:
> > This patch provides kexec_file_ops for "Image"-format kernel. In this
> > implementation, a binary is always loaded with a fixed offset identified
> > in text_offset field of its header.
>
>
> > diff --git a/arch/arm64/include/asm/kexec.h b/arch/arm64/include/asm/kexec.h
> > index e4de1223715f..3cba4161818a 100644
> > --- a/arch/arm64/include/asm/kexec.h
> > +++ b/arch/arm64/include/asm/kexec.h
> > @@ -102,6 +102,56 @@ struct kimage_arch {
> > void *dtb_buf;
> > };
> >
> > +/**
> > + * struct arm64_image_header - arm64 kernel image header
> > + *
> > + * @pe_sig: Optional PE format 'MZ' signature
> > + * @branch_code: Instruction to branch to stext
> > + * @text_offset: Image load offset, little endian
> > + * @image_size: Effective image size, little endian
> > + * @flags:
> > + * Bit 0: Kernel endianness. 0=little endian, 1=big endian
>
> Page size? What about 'phys_base'?, (whatever that is...)
> Probably best to refer to Documentation/arm64/booting.txt here, its the
> authoritative source of what these fields mean.
While we don't care other bit fields for now, I will add the reference
to the Documentation file.
>
> > + * @reserved: Reserved
> > + * @magic: Magic number, "ARM\x64"
> > + * @pe_header: Optional offset to a PE format header
> > + **/
> > +
> > +struct arm64_image_header {
> > + u8 pe_sig[2];
> > + u8 pad[2];
> > + u32 branch_code;
> > + u64 text_offset;
> > + u64 image_size;
> > + u64 flags;
>
> __le64 as appropriate here would let tools like sparse catch any missing endian
> conversion bugs.
OK.
>
> > + u64 reserved[3];
> > + u8 magic[4];
> > + u32 pe_header;
> > +};
>
> I'm surprised we don't have a definition for this already, I guess its always
> done in asm. We have kernel/image.h that holds some of this stuff, if we are
> going to validate the flags, is it worth adding the code there, (and moving it
> to include/asm)?
A comment at the beginning of this file says,
#ifndef LINKER_SCRIPT
#error This file should only be included in vmlinux.lds.S
#endif
Let me think about.
>
> > +static const u8 arm64_image_magic[4] = {'A', 'R', 'M', 0x64U};
>
> Any chance this magic could be a pre-processor symbol shared with head.S?
OK.
>
> > +
> > +/**
> > + * arm64_header_check_magic - Helper to check the arm64 image header.
> > + *
> > + * Returns non-zero if header is OK.
> > + */
> > +
> > +static inline int arm64_header_check_magic(const struct arm64_image_header *h)
> > +{
> > + if (!h)
> > + return 0;
> > +
> > + if (!h->text_offset)
> > + return 0;
> > +
> > + return (h->magic[0] == arm64_image_magic[0]
> > + && h->magic[1] == arm64_image_magic[1]
> > + && h->magic[2] == arm64_image_magic[2]
> > + && h->magic[3] == arm64_image_magic[3]);
>
> memcmp()? Or just define it as a 32bit value?
OK. As you know, I always tried to keep the code not diverted
from kexec-tools for maintainability reason.
> I guess you skip the MZ prefix as its not present for !EFI?
CONFIG_KEXEC_IMAGE_VERIFY_SIG depends on the fact that the file
format is PE (that is, EFI is enabled).
> Could we check branch_code is non-zero, and text-offset points within image-size?
We could do it, but I don't think this check is very useful.
>
> We could check that this platform supports the page-size/endian config that this
> Image was built with... We get a message from the EFI stub if the page-size
> can't be supported, it would be nice to do the same here (as we can).
There is no restriction on page-size or endianness for kexec.
What will be the purpose of this check?
> (no idea if kexec-tool checks this stuff, it probably can't get at the id
> registers to know)
>
>
> > diff --git a/arch/arm64/kernel/kexec_image.c b/arch/arm64/kernel/kexec_image.c
> > new file mode 100644
> > index 000000000000..4dd524ad6611
> > --- /dev/null
> > +++ b/arch/arm64/kernel/kexec_image.c
> > @@ -0,0 +1,79 @@
>
> > +static void *image_load(struct kimage *image,
> > + char *kernel, unsigned long kernel_len,
> > + char *initrd, unsigned long initrd_len,
> > + char *cmdline, unsigned long cmdline_len)
> > +{
> > + struct kexec_buf kbuf;
> > + struct arm64_image_header *h = (struct arm64_image_header *)kernel;
> > + unsigned long text_offset;
> > + int ret;
> > +
> > + /* Load the kernel */
> > + kbuf.image = image;
> > + kbuf.buf_min = 0;
> > + kbuf.buf_max = ULONG_MAX;
> > + kbuf.top_down = false;
> > +
> > + kbuf.buffer = kernel;
> > + kbuf.bufsz = kernel_len;
> > + kbuf.memsz = le64_to_cpu(h->image_size);
> > + text_offset = le64_to_cpu(h->text_offset);
> > + kbuf.buf_align = SZ_2M;
>
> > + /* Adjust kernel segment with TEXT_OFFSET */
> > + kbuf.memsz += text_offset;
> > +
> > + ret = kexec_add_buffer(&kbuf);
> > + if (ret)
> > + goto out;
> > +
> > + image->arch.kern_segment = image->nr_segments - 1;
>
> You only seem to use kern_segment here, and in load_other_segments() called
> below. Could it not be a local variable passed in? Instead of arch-specific data
> we keep forever?
No, kern_segment is also used in load_other_segments() in machine_kexec_file.c.
To optimize memory hole allocation logic in locate_mem_hole_callback(),
we need to know the exact range of kernel image (start and end).
(Known drawback in this code is that Image only occupies one segment, but
once vmlinux might be supported, it would occupy two segments for text and
data.)
>
> > + image->segment[image->arch.kern_segment].mem += text_offset;
> > + image->segment[image->arch.kern_segment].memsz -= text_offset;
> > + image->start = image->segment[image->arch.kern_segment].mem;
> > +
> > + pr_debug("Loaded kernel at 0x%lx bufsz=0x%lx memsz=0x%lx\n",
> > + image->segment[image->arch.kern_segment].mem,
> > + kbuf.bufsz, kbuf.memsz);
> > +
> > + /* Load additional data */
> > + ret = load_other_segments(image, initrd, initrd_len,
> > + cmdline, cmdline_len);
> > +
> > +out:
> > + return ERR_PTR(ret);
> > +}
> Looks good,
Thank you for thorough review.
-Takahiro AKASHI
> Thanks,
>
> James
^ permalink raw reply
* [PATCH v3 3/3] ARM: dts: sun7i: Add support for the Ainol AW1 tablet
From: Maxime Ripard @ 2018-05-07 7:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180506214901.23429-3-contact@paulk.fr>
Hi,
On Sun, May 06, 2018 at 11:49:01PM +0200, Paul Kocialkowski wrote:
> This adds support for the Ainol AW1, an A20-based 7" tablet from Ainol.
>
> The following board-specific features are supported:
> * LCD panel
> * Backlight
> * USB OTG
> * Buttons
> * Touchscreen (doesn't work without non-free firmware)
> * Accelerometer
> * Battery
>
> The following are untested:
> * Audio output
> * Audio speakers
> * USB via SPCI connector
>
> The following are not supported:
> * Wi-Fi
> * Bluetooth
> * NAND
> * Audio via SPCI connector
> * Audio via Bluetooth I2S
>
> Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> ---
> arch/arm/boot/dts/Makefile | 1 +
> arch/arm/boot/dts/sun7i-a20-ainol-aw1.dts | 275 ++++++++++++++++++++++
> 2 files changed, 276 insertions(+)
> create mode 100644 arch/arm/boot/dts/sun7i-a20-ainol-aw1.dts
>
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index 7e2424957809..4a80971f2bc9 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -946,6 +946,7 @@ dtb-$(CONFIG_MACH_SUN6I) += \
> sun6i-a31s-sinovoip-bpi-m2.dtb \
> sun6i-a31s-yones-toptech-bs1078-v2.dtb
> dtb-$(CONFIG_MACH_SUN7I) += \
> + sun7i-a20-ainol-aw1.dtb \
> sun7i-a20-bananapi.dtb \
> sun7i-a20-bananapi-m1-plus.dtb \
> sun7i-a20-bananapro.dtb \
> diff --git a/arch/arm/boot/dts/sun7i-a20-ainol-aw1.dts b/arch/arm/boot/dts/sun7i-a20-ainol-aw1.dts
> new file mode 100644
> index 000000000000..9a1d54a9f9a0
> --- /dev/null
> +++ b/arch/arm/boot/dts/sun7i-a20-ainol-aw1.dts
> @@ -0,0 +1,275 @@
> +/*
> + * Copyright (C) 2018 Paul Kocialkowski <contact@paulk.fr>
> + *
> + * SPDX-License-Identifier: GPL-2.0+
This should be your first line. Also, we usually license our DT under
a dual license (GPL and MIT) so that other projects (like FreeBSD) can
use them as well, instead of duplicating them. It would be great if
you'd consider it.
> + */
> +
> +/dts-v1/;
> +#include "sun7i-a20.dtsi"
> +#include "sunxi-common-regulators.dtsi"
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/input/input.h>
> +#include <dt-bindings/interrupt-controller/irq.h>
> +#include <dt-bindings/pwm/pwm.h>
> +
> +/ {
> + model = "Ainol AW1";
> + compatible = "ainol,ainol-aw1", "allwinner,sun7i-a20";
> +
> + aliases {
> + serial0 = &uart0;
> + };
> +
> + chosen {
> + stdout-path = "serial0:115200n8";
> + };
> +
> + backlight: backlight {
> + compatible = "pwm-backlight";
> + pwms = <&pwm 0 50000 PWM_POLARITY_INVERTED>;
> + brightness-levels = <0 10 20 30 40 50 60 70 80 90 100>;
The increase in perceived brightness should be linear. Usually, for
PWMs backed backlight, an exponential list is a much better
approximation.
> + default-brightness-level = <5>;
> + enable-gpios = <&pio 7 7 GPIO_ACTIVE_HIGH>; /* PH7 */
> + };
> +
> + panel: panel {
> + compatible = "innolux,at070tn90";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + power-supply = <&panel_power>;
> + backlight = <&backlight>;
> +
> + port at 0 {
> + reg = <0>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + panel_input: endpoint at 0 {
> + reg = <0>;
> + remote-endpoint = <&tcon0_out_panel>;
> + };
> + };
> + };
> +
> + panel_power: panel_power {
> + compatible = "regulator-fixed";
> + pinctrl-names = "default";
> + pinctrl-0 = <&panel_power_pin>;
> + regulator-name = "panel-power";
> + regulator-min-microvolt = <10400000>;
> + regulator-max-microvolt = <10400000>;
> + gpio = <&pio 7 8 GPIO_ACTIVE_HIGH>; /* PH8 */
> + enable-active-high;
> + regulator-boot-on;
> + };
> +};
> +
> +&codec {
> + allwinner,pa-gpios = <&pio 7 15 GPIO_ACTIVE_HIGH>; /* PH15 */
> + status = "okay";
> +};
> +
> +&cpu0 {
> + cpu-supply = <®_dcdc2>;
> +};
> +
> +&de {
> + status = "okay";
> +};
> +
> +&ehci0 {
> + status = "okay";
> +};
> +
> +&ehci1 {
> + status = "okay";
> +};
> +
> +&i2c0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&i2c0_pins_a>;
> + status = "okay";
> +
> + axp209: pmic at 34 {
> + reg = <0x34>;
> + interrupt-parent = <&nmi_intc>;
> + interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
> + };
> +};
> +
> +&i2c1 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&i2c1_pins_a>;
> + status = "okay";
> +
> + lis3dh: accelerometer at 18 {
> + compatible = "st,lis3dh-accel";
> + reg = <0x18>;
> + vdd-supply = <®_vcc3v3>;
> + vddio-supply = <®_vcc3v3>;
> + st,drdy-int-pin = <1>;
> + };
> +};
> +
> +&i2c2 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&i2c2_pins_a>;
> + status = "okay";
> + clock-frequency = <400000>;
> +
> + gsl1680: touchscreen at 40 {
> + compatible = "silead,gsl1680";
> + reg = <0x40>;
> + interrupt-parent = <&pio>;
> + interrupts = <7 21 IRQ_TYPE_EDGE_FALLING>; /* EINT21 (PH21) */
> + power-gpios = <&pio 7 20 GPIO_ACTIVE_HIGH>; /* PH20 */
> + firmware-name = "gsl1680-ainol-aw1.fw";
> + touchscreen-size-x = <480>;
> + touchscreen-size-y = <800>;
> + touchscreen-swapped-x-y;
> + touchscreen-inverted-y;
> + silead,max-fingers = <5>;
> + };
> +};
> +
> +&lradc {
> + vref-supply = <®_ldo2>;
> + status = "okay";
> +
> + button at 571 {
> + label = "Volume Up";
> + linux,code = <KEY_VOLUMEUP>;
> + channel = <0>;
> + voltage = <571428>;
> + };
> +
> + button at 761 {
> + label = "Volume Down";
> + linux,code = <KEY_VOLUMEDOWN>;
> + channel = <0>;
> + voltage = <761904>;
> + };
> +
> + button at 952 {
> + label = "Home";
> + linux,code = <KEY_HOME>;
> + channel = <0>;
> + voltage = <952380>;
> + };
> +};
> +
> +&mmc0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&mmc0_pins_a>;
> + vmmc-supply = <®_vcc3v3>;
You have the regulators described in your DT, you'd better use them
instead of the one coming from sunxi-common-regulators.dtsi.
> + bus-width = <4>;
> + cd-gpios = <&pio 7 1 GPIO_ACTIVE_HIGH>; /* PH1 */
> + cd-inverted;
> + status = "okay";
> +};
> +
> +&ohci0 {
> + status = "okay";
> +};
> +
> +&ohci1 {
> + status = "okay";
> +};
> +
> +&otg_sram {
> + status = "okay";
> +};
> +
> +&pio {
> + panel_power_pin: panel_power_pin at 0 {
> + pins = "PH8";
> + function = "gpio_out";
> + };
> +};
You don't need that pinctrl node.
Maxime
--
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180507/19ecef0e/attachment.sig>
^ permalink raw reply
* [PATCH v3 1/3] drm/panel: Add RGB666 variant of Innolux AT070TN90
From: Maxime Ripard @ 2018-05-07 7:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180506214901.23429-1-contact@paulk.fr>
Hi,
On Sun, May 06, 2018 at 11:48:59PM +0200, Paul Kocialkowski wrote:
> This adds timings for the RGB666 variant of the Innolux AT070TN90 panel,
> as found on the Ainol AW1 tablet.
>
> The panel also supports RGB888 output. When RGB666 mode is used instead,
> the two extra lanes per component are grounded.
>
> In the future, it might become necessary to introduce a dedicated
> device-tree property to specify the bus format and maybe specify it in
> the mode description instead of panel description so that the
> appropriate mode can be selected for each bus format.
>
> Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
A change log would be nice. Also, you mentionned in your first version
that the screen was an AT070TN92, and now you mention that it is an
AT070TN90, which one is it?
Maxime
> ---
> drivers/gpu/drm/panel/panel-simple.c | 26 ++++++++++++++++++++++++++
> 1 file changed, 26 insertions(+)
>
> diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
> index cbf1ab404ee7..351742df8ee1 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -1086,6 +1086,29 @@ static const struct panel_desc innolux_at070tn92 = {
> .bus_format = MEDIA_BUS_FMT_RGB888_1X24,
> };
>
> +static const struct drm_display_mode innolux_at070tn90_mode = {
> + .clock = 40000,
> + .hdisplay = 800,
> + .hsync_start = 800 + 112,
> + .hsync_end = 800 + 112 + 1,
> + .htotal = 800 + 112 + 1 + 87,
> + .vdisplay = 480,
> + .vsync_start = 480 + 141,
> + .vsync_end = 480 + 141 + 1,
> + .vtotal = 480 + 141 + 1 + 38,
> + .vrefresh = 60,
> +};
> +
> +static const struct panel_desc innolux_at070tn90 = {
> + .modes = &innolux_at070tn90_mode,
> + .num_modes = 1,
> + .size = {
> + .width = 154,
> + .height = 86,
> + },
> + .bus_format = MEDIA_BUS_FMT_RGB666_1X18,
> +};
> +
> static const struct display_timing innolux_g101ice_l01_timing = {
> .pixelclock = { 60400000, 71100000, 74700000 },
> .hactive = { 1280, 1280, 1280 },
> @@ -2154,6 +2177,9 @@ static const struct of_device_id platform_of_match[] = {
> }, {
> .compatible = "innolux,at070tn92",
> .data = &innolux_at070tn92,
> + }, {
> + .compatible = "innolux,at070tn90",
> + .data = &innolux_at070tn90,
This should be ordered alphabetically.
Maxime
--
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180507/c2eb471e/attachment.sig>
^ 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