Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ARM: ftrace: fix syscall name matching
From: Russell King - ARM Linux @ 2016-11-14 13:40 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1479128625-20948-1-git-send-email-rabin.vincent@axis.com>

On Mon, Nov 14, 2016 at 02:03:45PM +0100, Rabin Vincent wrote:
> From: Rabin Vincent <rabinv@axis.com>
> 
> ARM has a few system calls (most notably mmap) for which the names of
> the functions which are referenced in the syscall table do not match the
> names of the syscall tracepoints.  As a consequence of this, these
> tracepoints are not made available.  Implement
> arch_syscall_match_sym_name to fix this and allow tracing even these
> system calls.
> 
> Signed-off-by: Rabin Vincent <rabinv@axis.com>
> ---
>  arch/arm/include/asm/ftrace.h | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/arch/arm/include/asm/ftrace.h b/arch/arm/include/asm/ftrace.h
> index bfe2a2f..8467909 100644
> --- a/arch/arm/include/asm/ftrace.h
> +++ b/arch/arm/include/asm/ftrace.h
> @@ -54,6 +54,27 @@ static inline void *return_address(unsigned int level)
>  
>  #define ftrace_return_address(n) return_address(n)
>  
> +#define ARCH_HAS_SYSCALL_MATCH_SYM_NAME
> +
> +static inline bool arch_syscall_match_sym_name(const char *sym,
> +					       const char *name)
> +{
> +	/* Skip sys_ */
> +	sym += 4;
> +	name += 4;

Is this really safe?  What guarantees that we can wind forward four
bytes here?  If it's always safe, it needs a better comment than just
two words.

> +
> +	if (!strcmp(sym, "mmap2"))
> +		sym = "mmap_pgoff";
> +	else if (!strcmp(sym, "statfs64_wrapper"))
> +		sym = "statfs64";
> +	else if (!strcmp(sym, "fstatfs64_wrapper"))
> +		sym = "fstatfs64";
> +	else if (!strcmp(sym, "arm_fadvise64_64"))
> +		sym = "fadvise64_64";
> +
> +	return !strcmp(sym, name);
> +}
> +
>  #endif /* ifndef __ASSEMBLY__ */
>  
>  #endif /* _ASM_ARM_FTRACE */
> -- 
> 2.1.4
> 

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply

* [PATCH v18 1/2] Documentation: bridge: Add documentation for ps8640 DT properties
From: Jitao Shi @ 2016-11-14 13:41 UTC (permalink / raw)
  To: linux-arm-kernel

Add documentation for DT properties supported by
ps8640 DSI-eDP converter.

Signed-off-by: Jitao Shi <jitao.shi@mediatek.com>
Acked-by: Rob Herring <robh@kernel.org>
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
---
Changes since v17:
 - No change.

Changes since v16:
 - No change.

Changes since v15:
 - No change.

Changes since v14:
 - change mode-sel-gpios as optional.
---
 .../devicetree/bindings/display/bridge/ps8640.txt  |   44 ++++++++++++++++++++
 1 file changed, 44 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/ps8640.txt

diff --git a/Documentation/devicetree/bindings/display/bridge/ps8640.txt b/Documentation/devicetree/bindings/display/bridge/ps8640.txt
new file mode 100644
index 0000000..7b13f92
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/ps8640.txt
@@ -0,0 +1,44 @@
+ps8640-bridge bindings
+
+Required properties:
+	- compatible: "parade,ps8640"
+	- reg: first page address of the bridge.
+	- sleep-gpios: OF device-tree gpio specification for PD pin.
+	- reset-gpios: OF device-tree gpio specification for reset pin.
+	- vdd12-supply: OF device-tree regulator specification for 1.2V power.
+	- vdd33-supply: OF device-tree regulator specification for 3.3V power.
+	- ports: The device node can contain video interface port nodes per
+		 the video-interfaces bind[1]. For port at 0,set the reg = <0> as
+		 ps8640 dsi in and port at 1,set the reg = <1> as ps8640 eDP out.
+
+Optional properties:
+	- mode-sel-gpios: OF device-tree gpio specification for mode-sel pin.
+[1]: Documentation/devicetree/bindings/media/video-interfaces.txt
+
+Example:
+	edp-bridge at 18 {
+		compatible = "parade,ps8640";
+		reg = <0x18>;
+		sleep-gpios = <&pio 116 GPIO_ACTIVE_LOW>;
+		reset-gpios = <&pio 115 GPIO_ACTIVE_LOW>;
+		mode-sel-gpios = <&pio 92 GPIO_ACTIVE_HIGH>;
+		vdd12-supply = <&ps8640_fixed_1v2>;
+		vdd33-supply = <&mt6397_vgp2_reg>;
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			port at 0 {
+				reg = <0>;
+				ps8640_in: endpoint {
+					remote-endpoint = <&dsi0_out>;
+				};
+			};
+			port at 1 {
+				reg = <1>;
+				ps8640_out: endpoint {
+					remote-endpoint = <&panel_in>;
+				};
+			};
+		};
+	};
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v18 2/2] drm/bridge: Add I2C based driver for ps8640 bridge
From: Jitao Shi @ 2016-11-14 13:41 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1479130908-17593-1-git-send-email-jitao.shi@mediatek.com>

This patch adds drm_bridge driver for parade DSI to eDP bridge chip.

Signed-off-by: Jitao Shi <jitao.shi@mediatek.com>
Reviewed-by: Daniel Kurtz <djkurtz@chromium.org>
Reviewed-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
Changes since v17:
 - remove some unused head files.
 - add macros for ps8640 pages.
 - remove ddc_i2c client
 - add mipi_dsi_device_register_full
 - remove the manufacturer from the name and i2c_device_id
 
Changes since v16:
 - Disable ps8640 DSI MCS Function.
 - Rename gpios name more clearly.
 - Tune the ps8640 power on sequence.

Changes since v15:
 - Drop drm_connector_(un)register calls from parade ps8640.
   The main DRM driver mtk_drm_drv now calls
   drm_connector_register_all() after drm_dev_register() in the
   mtk_drm_bind() function. That function should iterate over all
   connectors and call drm_connector_register() for each of them.
   So, remove drm_connector_(un)register calls from parade ps8640.

Changes since v14:
 - update copyright info.
 - change bridge_to_ps8640 and connector_to_ps8640 to inline function.
 - fix some coding style.
 - use sizeof as array counter.
 - use drm_get_edid when read edid.
 - add mutex when firmware updating. 

Changes since v13:
 - add const on data, ps8640_write_bytes(struct i2c_client *client, const u8 *data, u16 data_len)
 - fix PAGE2_SW_REST tyro.
 - move the buf[3] init to entrance of the function.

Changes since v12:
 - fix hw_chip_id build warning

Changes since v11:
 - Remove depends on I2C, add DRM depends
 - Reuse ps8640_write_bytes() in ps8640_write_byte()
 - Use timer check for polling like the routines in <linux/iopoll.h>
 - Fix no drm_connector_unregister/drm_connector_cleanup when ps8640_bridge_attach fail
 - Check the ps8640 hardware id in ps8640_validate_firmware
 - Remove fw_version check
 - Move ps8640_validate_firmware before ps8640_enter_bl
 - Add ddc_i2c unregister when probe fail and ps8640_remove
---
 drivers/gpu/drm/bridge/Kconfig         |   12 +
 drivers/gpu/drm/bridge/Makefile        |    1 +
 drivers/gpu/drm/bridge/parade-ps8640.c | 1079 ++++++++++++++++++++++++++++++++
 3 files changed, 1092 insertions(+)
 create mode 100644 drivers/gpu/drm/bridge/parade-ps8640.c

diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index 10e12e7..7f41bbc 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -57,6 +57,18 @@ config DRM_PARADE_PS8622
 	---help---
 	  Parade eDP-LVDS bridge chip driver.
 
+config DRM_PARADE_PS8640
+	tristate "Parade PS8640 MIPI DSI to eDP Converter"
+	depends on DRM
+	depends on OF
+	select DRM_KMS_HELPER
+	select DRM_MIPI_DSI
+	select DRM_PANEL
+	---help---
+	  Choose this option if you have PS8640 for display
+	  The PS8640 is a high-performance and low-power
+	  MIPI DSI to eDP converter
+
 config DRM_SII902X
 	tristate "Silicon Image sii902x RGB/HDMI bridge"
 	depends on OF
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index cdf3a3c..7d93d40 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_DRM_DW_HDMI) += dw-hdmi.o
 obj-$(CONFIG_DRM_DW_HDMI_AHB_AUDIO) += dw-hdmi-ahb-audio.o
 obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
 obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o
+obj-$(CONFIG_DRM_PARADE_PS8640) += parade-ps8640.o
 obj-$(CONFIG_DRM_SII902X) += sii902x.o
 obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o
 obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c b/drivers/gpu/drm/bridge/parade-ps8640.c
new file mode 100644
index 0000000..2d9c337
--- /dev/null
+++ b/drivers/gpu/drm/bridge/parade-ps8640.c
@@ -0,0 +1,1079 @@
+/*
+ * Copyright (c) 2016 MediaTek Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/firmware.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/of_graph.h>
+#include <linux/regulator/consumer.h>
+#include <asm/unaligned.h>
+#include <drm/drm_panel.h>
+
+#include <drmP.h>
+#include <drm_atomic_helper.h>
+#include <drm_crtc_helper.h>
+#include <drm_edid.h>
+#include <drm_mipi_dsi.h>
+
+#define PAGE1_VSTART		0x6b
+#define PAGE2_SPI_CFG3		0x82
+#define I2C_TO_SPI_RESET	0x20
+#define PAGE2_ROMADD_BYTE1	0x8e
+#define PAGE2_ROMADD_BYTE2	0x8f
+#define PAGE2_SWSPI_WDATA	0x90
+#define PAGE2_SWSPI_RDATA	0x91
+#define PAGE2_SWSPI_LEN		0x92
+#define PAGE2_SWSPI_CTL		0x93
+#define TRIGGER_NO_READBACK	0x05
+#define TRIGGER_READBACK	0x01
+#define PAGE2_SPI_STATUS	0x9e
+#define SPI_READY		0x0c
+#define PAGE2_GPIO_L		0xa6
+#define PAGE2_GPIO_H		0xa7
+#define PS_GPIO9		BIT(1)
+#define PAGE2_IROM_CTRL		0xb0
+#define IROM_ENABLE		0xc0
+#define IROM_DISABLE		0x80
+#define PAGE2_SW_RESET		0xbc
+#define SPI_SW_RESET		BIT(7)
+#define MPU_SW_RESET		BIT(6)
+#define PAGE2_ENCTLSPI_WR	0xda
+#define PAGE2_I2C_BYPASS	0xea
+#define I2C_BYPASS_EN		0xd0
+#define PAGE2_MCS_EN		0xf3
+#define MCS_EN			BIT(0)
+#define PAGE3_SET_ADD		0xfe
+#define PAGE3_SET_VAL		0xff
+#define VDO_CTL_ADD		0x13
+#define VDO_DIS			0x18
+#define VDO_EN			0x1c
+#define PAGE4_REV_L		0xf0
+#define PAGE4_REV_H		0xf1
+#define PAGE4_CHIP_L		0xf2
+#define PAGE4_CHIP_H		0xf3
+
+#define PAGE0_DP_CNTL	0
+#define PAGE1_VDO_BDG	1
+#define PAGE2_TOP_CNTL	2
+#define PAGE3_DSI_CNTL1	3
+#define PAGE4_MIPI_PHY	4
+#define PAGE5_VPLL	5
+#define PAGE6_DSI_CNTL2	6
+#define PAGE7_SPI_CNTL	7
+#define MAX_DEVS		0x8
+
+/* Firmware */
+#define PS_FW_NAME		"ps864x_fw.bin"
+
+#define FW_CHIP_ID_OFFSET	0
+#define FW_VERSION_OFFSET	2
+#define EDID_I2C_ADDR		0x50
+
+#define WRITE_STATUS_REG_CMD	0x01
+#define READ_STATUS_REG_CMD	0x05
+#define BUSY			BIT(0)
+#define CLEAR_ALL_PROTECT	0x00
+#define BLK_PROTECT_BITS	0x0c
+#define STATUS_REG_PROTECT	BIT(7)
+#define WRITE_ENABLE_CMD	0x06
+#define CHIP_ERASE_CMD		0xc7
+
+struct ps8640_info {
+	u8 family_id;
+	u8 variant_id;
+	u16 version;
+};
+
+struct ps8640 {
+	struct drm_connector connector;
+	struct drm_bridge bridge;
+	struct edid *edid;
+	struct mipi_dsi_device *dsi;
+	struct i2c_client *page[MAX_DEVS];
+	struct regulator_bulk_data supplies[2];
+	struct drm_panel *panel;
+	struct gpio_desc *gpio_reset;
+	struct gpio_desc *gpio_power_down;
+	struct gpio_desc *gpio_mode_sel;
+	bool enabled;
+
+	/* firmware file info */
+	struct ps8640_info info;
+	bool in_fw_update;
+	/* for firmware update protect */
+	struct mutex fw_mutex;
+};
+
+static const u8 enc_ctrl_code[6] = { 0xaa, 0x55, 0x50, 0x41, 0x52, 0x44 };
+static const u8 hw_chip_id[4] = { 0x00, 0x0a, 0x00, 0x30 };
+
+static inline struct ps8640 *bridge_to_ps8640(struct drm_bridge *e)
+{
+	return container_of(e, struct ps8640, bridge);
+}
+
+static inline struct ps8640 *connector_to_ps8640(struct drm_connector *e)
+{
+	return container_of(e, struct ps8640, connector);
+}
+
+static int ps8640_read(struct i2c_client *client, u8 reg, u8 *data,
+		       u16 data_len)
+{
+	int ret;
+	struct i2c_msg msgs[] = {
+		{
+		 .addr = client->addr,
+		 .flags = 0,
+		 .len = 1,
+		 .buf = &reg,
+		},
+		{
+		 .addr = client->addr,
+		 .flags = I2C_M_RD,
+		 .len = data_len,
+		 .buf = data,
+		}
+	};
+
+	ret = i2c_transfer(client->adapter, msgs, 2);
+
+	if (ret == 2)
+		return 0;
+	if (ret < 0)
+		return ret;
+	else
+		return -EIO;
+}
+
+static int ps8640_write_bytes(struct i2c_client *client, const u8 *data,
+			      u16 data_len)
+{
+	int ret;
+	struct i2c_msg msg;
+
+	msg.addr = client->addr;
+	msg.flags = 0;
+	msg.len = data_len;
+	msg.buf = (u8 *)data;
+
+	ret = i2c_transfer(client->adapter, &msg, 1);
+	if (ret == 1)
+		return 0;
+	if (ret < 0)
+		return ret;
+	else
+		return -EIO;
+}
+
+static int ps8640_write_byte(struct i2c_client *client, u8 reg,  u8 data)
+{
+	u8 buf[] = { reg, data };
+
+	return ps8640_write_bytes(client, buf, sizeof(buf));
+}
+
+static void ps8640_get_mcu_fw_version(struct ps8640 *ps_bridge)
+{
+	struct i2c_client *client = ps_bridge->page[PAGE5_VPLL];
+	u8 fw_ver[2];
+
+	ps8640_read(client, 0x4, fw_ver, sizeof(fw_ver));
+	ps_bridge->info.version = (fw_ver[0] << 8) | fw_ver[1];
+
+	DRM_INFO_ONCE("ps8640 rom fw version %d.%d\n", fw_ver[0], fw_ver[1]);
+}
+
+static int ps8640_bridge_unmute(struct ps8640 *ps_bridge)
+{
+	struct i2c_client *client = ps_bridge->page[PAGE3_DSI_CNTL1];
+	u8 vdo_ctrl_buf[3] = { PAGE3_SET_ADD, VDO_CTL_ADD, VDO_EN };
+
+	return ps8640_write_bytes(client, vdo_ctrl_buf, sizeof(vdo_ctrl_buf));
+}
+
+static int ps8640_bridge_mute(struct ps8640 *ps_bridge)
+{
+	struct i2c_client *client = ps_bridge->page[PAGE3_DSI_CNTL1];
+	u8 vdo_ctrl_buf[3] = { PAGE3_SET_ADD, VDO_CTL_ADD, VDO_DIS };
+
+	return ps8640_write_bytes(client, vdo_ctrl_buf, sizeof(vdo_ctrl_buf));
+}
+
+static void ps8640_pre_enable(struct drm_bridge *bridge)
+{
+	struct ps8640 *ps_bridge = bridge_to_ps8640(bridge);
+	struct i2c_client *client = ps_bridge->page[PAGE2_TOP_CNTL];
+	struct i2c_client *page1 = ps_bridge->page[PAGE1_VDO_BDG];
+	int err;
+	u8 set_vdo_done, mcs_en, vstart;
+	ktime_t timeout;
+
+	if (ps_bridge->in_fw_update)
+		return;
+
+	if (ps_bridge->enabled)
+		return;
+
+	err = drm_panel_prepare(ps_bridge->panel);
+	if (err < 0) {
+		DRM_ERROR("failed to prepare panel: %d\n", err);
+		return;
+	}
+
+	err = regulator_bulk_enable(ARRAY_SIZE(ps_bridge->supplies),
+				    ps_bridge->supplies);
+	if (err < 0) {
+		DRM_ERROR("cannot enable regulators %d\n", err);
+		goto err_panel_unprepare;
+	}
+
+	gpiod_set_value(ps_bridge->gpio_power_down, 1);
+	gpiod_set_value(ps_bridge->gpio_reset, 0);
+	usleep_range(2000, 2500);
+	gpiod_set_value(ps_bridge->gpio_reset, 1);
+
+	/*
+	 * Wait for the ps8640 embed mcu ready
+	 * First wait 200ms and then check the mcu ready flag every 20ms
+	 */
+	msleep(200);
+
+	timeout = ktime_add_ms(ktime_get(), 200);
+	for (;;) {
+		err = ps8640_read(client, PAGE2_GPIO_H, &set_vdo_done, 1);
+		if (err < 0) {
+			DRM_ERROR("failed read PAGE2_GPIO_H: %d\n", err);
+			goto err_regulators_disable;
+		}
+		if ((set_vdo_done & PS_GPIO9) == PS_GPIO9)
+			break;
+		if (ktime_compare(ktime_get(), timeout) > 0)
+			break;
+		msleep(20);
+	}
+
+	msleep(50);
+
+	ps8640_read(page1, PAGE1_VSTART, &vstart, 1);
+	DRM_INFO("PS8640 PAGE1.0x6B = 0x%x\n", vstart);
+
+	/**
+	 * The Manufacturer Command Set (MCS) is a device dependent interface
+	 * intended for factory programming of the display module default
+	 * parameters. Once the display module is configured, the MCS shall be
+	 * disabled by the manufacturer. Once disabled, all MCS commands are
+	 * ignored by the display interface.
+	 */
+	ps8640_read(client, PAGE2_MCS_EN, &mcs_en, 1);
+	ps8640_write_byte(client, PAGE2_MCS_EN, mcs_en & ~MCS_EN);
+
+	if (ps_bridge->info.version == 0)
+		ps8640_get_mcu_fw_version(ps_bridge);
+
+	err = ps8640_bridge_unmute(ps_bridge);
+	if (err)
+		DRM_ERROR("failed to enable unmutevideo: %d\n", err);
+	/* Switch access edp panel's edid through i2c */
+	ps8640_write_byte(client, PAGE2_I2C_BYPASS, I2C_BYPASS_EN);
+	ps_bridge->enabled = true;
+
+	return;
+
+err_regulators_disable:
+	regulator_bulk_disable(ARRAY_SIZE(ps_bridge->supplies),
+			       ps_bridge->supplies);
+err_panel_unprepare:
+	drm_panel_unprepare(ps_bridge->panel);
+}
+
+static void ps8640_enable(struct drm_bridge *bridge)
+{
+	struct ps8640 *ps_bridge = bridge_to_ps8640(bridge);
+	int err;
+
+	err = drm_panel_enable(ps_bridge->panel);
+	if (err < 0)
+		DRM_ERROR("failed to enable panel: %d\n", err);
+}
+
+static void ps8640_disable(struct drm_bridge *bridge)
+{
+	struct ps8640 *ps_bridge = bridge_to_ps8640(bridge);
+	int err;
+
+	err = drm_panel_disable(ps_bridge->panel);
+	if (err < 0)
+		DRM_ERROR("failed to disable panel: %d\n", err);
+}
+
+static void ps8640_post_disable(struct drm_bridge *bridge)
+{
+	struct ps8640 *ps_bridge = bridge_to_ps8640(bridge);
+	int err;
+
+	if (ps_bridge->in_fw_update)
+		return;
+
+	if (!ps_bridge->enabled)
+		return;
+
+	ps_bridge->enabled = false;
+
+	err = ps8640_bridge_mute(ps_bridge);
+	if (err < 0)
+		DRM_ERROR("failed to unmutevideo: %d\n", err);
+
+	gpiod_set_value(ps_bridge->gpio_reset, 0);
+	gpiod_set_value(ps_bridge->gpio_power_down, 0);
+	err = regulator_bulk_disable(ARRAY_SIZE(ps_bridge->supplies),
+				     ps_bridge->supplies);
+	if (err < 0)
+		DRM_ERROR("cannot disable regulators %d\n", err);
+
+	err = drm_panel_unprepare(ps_bridge->panel);
+	if (err)
+		DRM_ERROR("failed to unprepare panel: %d\n", err);
+}
+
+static int ps8640_get_modes(struct drm_connector *connector)
+{
+	struct ps8640 *ps_bridge = connector_to_ps8640(connector);
+	struct edid *edid;
+	int num_modes = 0;
+	bool power_off;
+
+	if (ps_bridge->edid)
+		return drm_add_edid_modes(connector, ps_bridge->edid);
+
+	power_off = !ps_bridge->enabled;
+	ps8640_pre_enable(&ps_bridge->bridge);
+
+	edid = drm_get_edid(connector, ps_bridge->page[0]->adapter);
+	if (!edid)
+		goto out;
+
+	ps_bridge->edid = edid;
+	drm_mode_connector_update_edid_property(connector, ps_bridge->edid);
+	num_modes = drm_add_edid_modes(connector, ps_bridge->edid);
+
+out:
+	if (power_off)
+		ps8640_post_disable(&ps_bridge->bridge);
+
+	return num_modes;
+}
+
+static const struct drm_connector_helper_funcs ps8640_connector_helper_funcs = {
+	.get_modes = ps8640_get_modes,
+};
+
+static enum drm_connector_status ps8640_detect(struct drm_connector *connector,
+					       bool force)
+{
+	return connector_status_connected;
+}
+
+static const struct drm_connector_funcs ps8640_connector_funcs = {
+	.dpms = drm_atomic_helper_connector_dpms,
+	.fill_modes = drm_helper_probe_single_connector_modes,
+	.detect = ps8640_detect,
+	.reset = drm_atomic_helper_connector_reset,
+	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+};
+
+int ps8640_bridge_attach(struct drm_bridge *bridge)
+{
+	struct ps8640 *ps_bridge = bridge_to_ps8640(bridge);
+	struct device *dev = &ps_bridge->page[0]->dev;
+	struct device_node *in_ep, *dsi_node = NULL;
+	struct mipi_dsi_device *dsi;
+	struct mipi_dsi_host *host = NULL;
+	int ret;
+	const struct mipi_dsi_device_info info = { .type = "ps8640",
+						   .channel = 0,
+						   .node = NULL,
+						 };
+
+	ret = drm_connector_init(bridge->dev, &ps_bridge->connector,
+				 &ps8640_connector_funcs,
+				 DRM_MODE_CONNECTOR_eDP);
+
+	if (ret) {
+		DRM_ERROR("Failed to initialize connector with drm: %d\n", ret);
+		return ret;
+	}
+
+	drm_connector_helper_add(&ps_bridge->connector,
+				 &ps8640_connector_helper_funcs);
+
+	ps_bridge->connector.dpms = DRM_MODE_DPMS_ON;
+	drm_mode_connector_attach_encoder(&ps_bridge->connector,
+					  bridge->encoder);
+
+	if (ps_bridge->panel)
+		drm_panel_attach(ps_bridge->panel, &ps_bridge->connector);
+
+	/* port at 0 is ps8640 dsi input port */
+	in_ep = of_graph_get_endpoint_by_regs(dev->of_node, 0, -1);
+	if (in_ep) {
+		dsi_node = of_graph_get_remote_port_parent(in_ep);
+		of_node_put(in_ep);
+	}
+
+	if (dsi_node) {
+		host = of_find_mipi_dsi_host_by_node(dsi_node);
+		of_node_put(dsi_node);
+		if (!host) {
+			ret = -ENODEV;
+			goto err;
+		}
+	}
+
+	dsi = mipi_dsi_device_register_full(host, &info);
+	if (IS_ERR(dsi)) {
+		dev_err(dev, "failed to create dsi device\n");
+		ret = PTR_ERR(dsi);
+		goto err;
+	}
+
+	ps_bridge->dsi = dsi;
+
+	dsi->host = host;
+	dsi->mode_flags = MIPI_DSI_MODE_VIDEO |
+				     MIPI_DSI_MODE_VIDEO_SYNC_PULSE;
+	dsi->format = MIPI_DSI_FMT_RGB888;
+	dsi->lanes = 4;
+	ret = mipi_dsi_attach(dsi);
+	if (ret)
+		goto err_dsi_attach;
+
+	return 0;
+
+err_dsi_attach:
+	mipi_dsi_device_unregister(dsi);
+err:
+	if (ps_bridge->panel)
+		drm_panel_detach(ps_bridge->panel);
+	drm_connector_cleanup(&ps_bridge->connector);
+	return ret;
+}
+
+static const struct drm_bridge_funcs ps8640_bridge_funcs = {
+	.attach = ps8640_bridge_attach,
+	.disable = ps8640_disable,
+	.post_disable = ps8640_post_disable,
+	.pre_enable = ps8640_pre_enable,
+	.enable = ps8640_enable,
+};
+
+/* Firmware Version is returned as Major.Minor */
+static ssize_t ps8640_fw_version_show(struct device *dev,
+				      struct device_attribute *attr, char *buf)
+{
+	struct ps8640 *ps_bridge = dev_get_drvdata(dev);
+	struct ps8640_info *info = &ps_bridge->info;
+
+	return scnprintf(buf, PAGE_SIZE, "%u.%u\n", info->version >> 8,
+			 info->version & 0xff);
+}
+
+/* Hardware Version is returned as FamilyID.VariantID */
+static ssize_t ps8640_hw_version_show(struct device *dev,
+				      struct device_attribute *attr, char *buf)
+{
+	struct ps8640 *ps_bridge = dev_get_drvdata(dev);
+	struct ps8640_info *info = &ps_bridge->info;
+
+	return scnprintf(buf, PAGE_SIZE, "ps%u.%u\n", info->family_id,
+			 info->variant_id);
+}
+
+static int ps8640_spi_send_cmd(struct ps8640 *ps_bridge, u8 *cmd, u8 cmd_len)
+{
+	struct i2c_client *client = ps_bridge->page[PAGE2_TOP_CNTL];
+	u8 i, buf[3] = { PAGE2_SWSPI_LEN, cmd_len - 1, TRIGGER_NO_READBACK };
+	int ret;
+
+	ret = ps8640_write_byte(client, PAGE2_IROM_CTRL, IROM_ENABLE);
+	if (ret)
+		goto err;
+
+	/* write command in write port */
+	for (i = 0; i < cmd_len; i++) {
+		ret = ps8640_write_byte(client, PAGE2_SWSPI_WDATA, cmd[i]);
+		if (ret)
+			goto err_irom_disable;
+	}
+
+	ret = ps8640_write_bytes(client, buf, sizeof(buf));
+	if (ret)
+		goto err_irom_disable;
+
+	ret = ps8640_write_byte(client, PAGE2_IROM_CTRL, IROM_DISABLE);
+	if (ret)
+		goto err;
+
+	return 0;
+err_irom_disable:
+	ps8640_write_byte(client, PAGE2_IROM_CTRL, IROM_DISABLE);
+err:
+	dev_err(&client->dev, "send command err: %d\n", ret);
+	return ret;
+}
+
+static int ps8640_wait_spi_ready(struct ps8640 *ps_bridge)
+{
+	struct i2c_client *client = ps_bridge->page[PAGE2_TOP_CNTL];
+	u8 spi_rdy_st;
+	ktime_t timeout;
+
+	timeout = ktime_add_ms(ktime_get(), 200);
+	for (;;) {
+		ps8640_read(client, PAGE2_SPI_STATUS, &spi_rdy_st, 1);
+		if ((spi_rdy_st & SPI_READY) != SPI_READY)
+			break;
+
+		if (ktime_compare(ktime_get(), timeout) > 0) {
+			dev_err(&client->dev, "wait spi ready timeout\n");
+			return -EBUSY;
+		}
+
+		msleep(20);
+	}
+
+	return 0;
+}
+
+static int ps8640_wait_spi_nobusy(struct ps8640 *ps_bridge)
+{
+	struct i2c_client *client = ps_bridge->page[PAGE2_TOP_CNTL];
+	u8 spi_status, buf[3] = { PAGE2_SWSPI_LEN, 0, TRIGGER_READBACK };
+	int ret;
+	ktime_t timeout;
+
+	timeout = ktime_add_ms(ktime_get(), 500);
+	for (;;) {
+		/* 0x05 RDSR; Read-Status-Register */
+		ret = ps8640_write_byte(client, PAGE2_SWSPI_WDATA,
+					READ_STATUS_REG_CMD);
+		if (ret)
+			goto err_send_cmd_exit;
+
+		ret = ps8640_write_bytes(client, buf, 3);
+		if (ret)
+			goto err_send_cmd_exit;
+
+		/* delay for cmd send */
+		usleep_range(300, 500);
+		/* wait for SPI ROM until not busy */
+		ret = ps8640_read(client, PAGE2_SWSPI_RDATA, &spi_status, 1);
+		if (ret)
+			goto err_send_cmd_exit;
+
+		if (!(spi_status & BUSY))
+			break;
+
+		if (ktime_compare(ktime_get(), timeout) > 0) {
+			dev_err(&client->dev, "wait spi no busy timeout: %d\n",
+				ret);
+			return -EBUSY;
+		}
+	}
+
+	return 0;
+
+err_send_cmd_exit:
+	dev_err(&client->dev, "send command err: %d\n", ret);
+	return ret;
+}
+
+static int ps8640_wait_rom_idle(struct ps8640 *ps_bridge)
+{
+	struct i2c_client *client = ps_bridge->page[PAGE0_DP_CNTL];
+	int ret;
+
+	ret = ps8640_write_byte(client, PAGE2_IROM_CTRL, IROM_ENABLE);
+	if (ret)
+		goto exit;
+
+	ret = ps8640_wait_spi_ready(ps_bridge);
+	if (ret)
+		goto err_spi;
+
+	ret = ps8640_wait_spi_nobusy(ps_bridge);
+	if (ret)
+		goto err_spi;
+
+	ret = ps8640_write_byte(client, PAGE2_IROM_CTRL, IROM_DISABLE);
+	if (ret)
+		goto exit;
+
+	return 0;
+
+err_spi:
+	ps8640_write_byte(client, PAGE2_IROM_CTRL, IROM_DISABLE);
+exit:
+	dev_err(&client->dev, "wait ps8640 rom idle fail: %d\n", ret);
+
+	return ret;
+}
+
+static int ps8640_spi_dl_mode(struct ps8640 *ps_bridge)
+{
+	struct i2c_client *client = ps_bridge->page[PAGE2_TOP_CNTL];
+	int ret;
+
+	/* switch ps8640 mode to spi dl mode */
+	if (ps_bridge->gpio_mode_sel)
+		gpiod_set_value(ps_bridge->gpio_mode_sel, 0);
+
+	/* reset spi interface */
+	ret = ps8640_write_byte(client, PAGE2_SW_RESET,
+				SPI_SW_RESET | MPU_SW_RESET);
+	if (ret)
+		goto exit;
+
+	ret = ps8640_write_byte(client, PAGE2_SW_RESET, MPU_SW_RESET);
+	if (ret)
+		goto exit;
+
+	return 0;
+
+exit:
+	dev_err(&client->dev, "fail reset spi interface: %d\n", ret);
+
+	return ret;
+}
+
+static int ps8640_rom_prepare(struct ps8640 *ps_bridge)
+{
+	struct i2c_client *client = ps_bridge->page[PAGE2_TOP_CNTL];
+	struct device *dev = &client->dev;
+	u8 i, cmd[2];
+	int ret;
+
+	cmd[0] = WRITE_ENABLE_CMD;
+	ret = ps8640_spi_send_cmd(ps_bridge, cmd, 1);
+	if (ret) {
+		dev_err(dev, "failed enable-write-status-register: %d\n", ret);
+		return ret;
+	}
+
+	cmd[0] = WRITE_STATUS_REG_CMD;
+	cmd[1] = CLEAR_ALL_PROTECT;
+	ret = ps8640_spi_send_cmd(ps_bridge, cmd, 2);
+	if (ret) {
+		dev_err(dev, "fail disable all protection: %d\n", ret);
+		return ret;
+	}
+
+	/* wait for SPI module ready */
+	ret = ps8640_wait_rom_idle(ps_bridge);
+	if (ret) {
+		dev_err(dev, "fail wait rom idle: %d\n", ret);
+		return ret;
+	}
+
+	ps8640_write_byte(client, PAGE2_IROM_CTRL, IROM_ENABLE);
+	for (i = 0; i < ARRAY_SIZE(enc_ctrl_code); i++)
+		ps8640_write_byte(client, PAGE2_ENCTLSPI_WR, enc_ctrl_code[i]);
+	ps8640_write_byte(client, PAGE2_IROM_CTRL, IROM_DISABLE);
+
+	/* Enable-Write-Status-Register */
+	cmd[0] = WRITE_ENABLE_CMD;
+	ret = ps8640_spi_send_cmd(ps_bridge, cmd, 1);
+	if (ret) {
+		dev_err(dev, "fail enable-write-status-register: %d\n", ret);
+		return ret;
+	}
+
+	/* chip erase command */
+	cmd[0] = CHIP_ERASE_CMD;
+	ret = ps8640_spi_send_cmd(ps_bridge, cmd, 1);
+	if (ret) {
+		dev_err(dev, "fail disable all protection: %d\n", ret);
+		return ret;
+	}
+
+	ret = ps8640_wait_rom_idle(ps_bridge);
+	if (ret) {
+		dev_err(dev, "fail wait rom idle: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int ps8640_check_chip_id(struct ps8640 *ps_bridge)
+{
+	struct i2c_client *client = ps_bridge->page[PAGE4_MIPI_PHY];
+	u8 buf[4];
+
+	ps8640_read(client, PAGE4_REV_L, buf, 4);
+	return memcmp(buf, hw_chip_id, sizeof(buf));
+}
+
+static int ps8640_validate_firmware(struct ps8640 *ps_bridge,
+				    const struct firmware *fw)
+{
+	struct i2c_client *client = ps_bridge->page[0];
+	u16 fw_chip_id;
+
+	/*
+	 * Get the chip_id from the firmware. Make sure that it is the
+	 * right controller to do the firmware and config update.
+	 */
+	fw_chip_id = get_unaligned_le16(fw->data + FW_CHIP_ID_OFFSET);
+
+	if (fw_chip_id != 0x8640 && ps8640_check_chip_id(ps_bridge) == 0) {
+		dev_err(&client->dev,
+			"chip id mismatch: fw 0x%x vs. chip 0x8640\n",
+			fw_chip_id);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int ps8640_write_rom(struct ps8640 *ps_bridge, const struct firmware *fw)
+{
+	struct i2c_client *client = ps_bridge->page[PAGE0_DP_CNTL];
+	struct device *dev = &client->dev;
+	struct i2c_client *client2 = ps_bridge->page[PAGE2_TOP_CNTL];
+	struct i2c_client *client7 = ps_bridge->page[PAGE7_SPI_CNTL];
+	size_t pos, cpy_len;
+	u8 buf[257];
+	int ret;
+
+	ps8640_write_byte(client2, PAGE2_SPI_CFG3, I2C_TO_SPI_RESET);
+	msleep(100);
+	ps8640_write_byte(client2, PAGE2_SPI_CFG3, 0x00);
+
+	for (pos = 0; pos < fw->size; pos += cpy_len) {
+		buf[0] = PAGE2_ROMADD_BYTE1;
+		buf[1] = pos >> 8;
+		buf[2] = pos >> 16;
+		ret = ps8640_write_bytes(client2, buf, 3);
+		if (ret)
+			goto error;
+		cpy_len = fw->size >= 256 + pos ? 256 : fw->size - pos;
+		buf[0] = 0;
+		memcpy(buf + 1, fw->data + pos, cpy_len);
+		ret = ps8640_write_bytes(client7, buf, cpy_len + 1);
+		if (ret)
+			goto error;
+
+		dev_dbg(dev, "fw update completed %zu / %zu bytes\n", pos,
+			fw->size);
+	}
+	return 0;
+
+error:
+	dev_err(dev, "failed write external flash, %d\n", ret);
+	return ret;
+}
+
+static int ps8640_spi_normal_mode(struct ps8640 *ps_bridge)
+{
+	u8 cmd[2];
+	struct i2c_client *client = ps_bridge->page[PAGE2_TOP_CNTL];
+
+	/* Enable-Write-Status-Register */
+	cmd[0] = WRITE_ENABLE_CMD;
+	ps8640_spi_send_cmd(ps_bridge, cmd, 1);
+
+	/* protect BPL/BP0/BP1 */
+	cmd[0] = WRITE_STATUS_REG_CMD;
+	cmd[1] = BLK_PROTECT_BITS | STATUS_REG_PROTECT;
+	ps8640_spi_send_cmd(ps_bridge, cmd, 2);
+
+	/* wait for SPI rom ready */
+	ps8640_wait_rom_idle(ps_bridge);
+
+	/* disable PS8640 mapping function */
+	ps8640_write_byte(client, PAGE2_ENCTLSPI_WR, 0x00);
+
+	if (ps_bridge->gpio_mode_sel)
+		gpiod_set_value(ps_bridge->gpio_mode_sel, 1);
+	return 0;
+}
+
+static int ps8640_enter_bl(struct ps8640 *ps_bridge)
+{
+	ps_bridge->in_fw_update = true;
+	return ps8640_spi_dl_mode(ps_bridge);
+}
+
+static void ps8640_exit_bl(struct ps8640 *ps_bridge, const struct firmware *fw)
+{
+	ps8640_spi_normal_mode(ps_bridge);
+	ps_bridge->in_fw_update = false;
+}
+
+static int ps8640_load_fw(struct ps8640 *ps_bridge, const struct firmware *fw)
+{
+	struct i2c_client *client = ps_bridge->page[PAGE0_DP_CNTL];
+	struct device *dev = &client->dev;
+	int ret;
+	bool ps8640_status_backup = ps_bridge->enabled;
+
+	ret = ps8640_validate_firmware(ps_bridge, fw);
+	if (ret)
+		return ret;
+
+	mutex_lock(&ps_bridge->fw_mutex);
+	if (!ps_bridge->in_fw_update) {
+		if (!ps8640_status_backup)
+			ps8640_pre_enable(&ps_bridge->bridge);
+
+		ret = ps8640_enter_bl(ps_bridge);
+		if (ret)
+			goto exit;
+	}
+
+	ret = ps8640_rom_prepare(ps_bridge);
+	if (ret)
+		goto exit;
+
+	ret = ps8640_write_rom(ps_bridge, fw);
+
+exit:
+	if (ret)
+		dev_err(dev, "Failed to load firmware, %d\n", ret);
+
+	ps8640_exit_bl(ps_bridge, fw);
+	if (!ps8640_status_backup)
+		ps8640_post_disable(&ps_bridge->bridge);
+	mutex_unlock(&ps_bridge->fw_mutex);
+	return ret;
+}
+
+static ssize_t ps8640_update_fw_store(struct device *dev,
+				      struct device_attribute *attr,
+				      const char *buf, size_t count)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct ps8640 *ps_bridge = i2c_get_clientdata(client);
+	const struct firmware *fw;
+	int error;
+
+	error = request_firmware(&fw, PS_FW_NAME, dev);
+	if (error) {
+		dev_err(dev, "Unable to open firmware %s: %d\n",
+			PS_FW_NAME, error);
+		return error;
+	}
+
+	error = ps8640_load_fw(ps_bridge, fw);
+	if (error)
+		dev_err(dev, "The firmware update failed(%d)\n", error);
+	else
+		dev_info(dev, "The firmware update succeeded\n");
+
+	release_firmware(fw);
+	return error ? error : count;
+}
+
+static DEVICE_ATTR(fw_version, S_IRUGO, ps8640_fw_version_show, NULL);
+static DEVICE_ATTR(hw_version, S_IRUGO, ps8640_hw_version_show, NULL);
+static DEVICE_ATTR(update_fw, S_IWUSR, NULL, ps8640_update_fw_store);
+
+static struct attribute *ps8640_attrs[] = {
+	&dev_attr_fw_version.attr,
+	&dev_attr_hw_version.attr,
+	&dev_attr_update_fw.attr,
+	NULL
+};
+
+static const struct attribute_group ps8640_attr_group = {
+	.attrs = ps8640_attrs,
+};
+
+static void ps8640_remove_sysfs_group(void *data)
+{
+	struct ps8640 *ps_bridge = data;
+
+	sysfs_remove_group(&ps_bridge->page[0]->dev.kobj, &ps8640_attr_group);
+}
+
+static int ps8640_probe(struct i2c_client *client,
+			const struct i2c_device_id *id)
+{
+	struct device *dev = &client->dev;
+	struct ps8640 *ps_bridge;
+	struct device_node *np = dev->of_node;
+	struct device_node *port, *out_ep;
+	struct device_node *panel_node = NULL;
+	int ret;
+	u32 i;
+
+	ps_bridge = devm_kzalloc(dev, sizeof(*ps_bridge), GFP_KERNEL);
+	if (!ps_bridge)
+		return -ENOMEM;
+
+	/* port at 1 is ps8640 output port */
+	port = of_graph_get_port_by_id(np, 1);
+	if (port) {
+		out_ep = of_get_child_by_name(port, "endpoint");
+		of_node_put(port);
+		if (out_ep) {
+			panel_node = of_graph_get_remote_port_parent(out_ep);
+			of_node_put(out_ep);
+		}
+	}
+	if (panel_node) {
+		ps_bridge->panel = of_drm_find_panel(panel_node);
+		of_node_put(panel_node);
+		if (!ps_bridge->panel)
+			return -EPROBE_DEFER;
+	}
+
+	mutex_init(&ps_bridge->fw_mutex);
+	ps_bridge->supplies[0].supply = "vdd33";
+	ps_bridge->supplies[1].supply = "vdd12";
+	ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(ps_bridge->supplies),
+				      ps_bridge->supplies);
+	if (ret) {
+		dev_info(dev, "failed to get regulators: %d\n", ret);
+		return ret;
+	}
+
+	ps_bridge->gpio_mode_sel = devm_gpiod_get_optional(&client->dev,
+							     "mode-sel",
+							     GPIOD_OUT_HIGH);
+	if (IS_ERR(ps_bridge->gpio_mode_sel)) {
+		ret = PTR_ERR(ps_bridge->gpio_mode_sel);
+		dev_err(dev, "cannot get mode-sel %d\n", ret);
+		return ret;
+	}
+
+	ps_bridge->gpio_power_down = devm_gpiod_get(&client->dev, "sleep",
+					       GPIOD_OUT_LOW);
+	if (IS_ERR(ps_bridge->gpio_power_down)) {
+		ret = PTR_ERR(ps_bridge->gpio_power_down);
+		dev_err(dev, "cannot get sleep: %d\n", ret);
+		return ret;
+	}
+
+	/*
+	 * Request the reset pin low to avoid the bridge being
+	 * initialized prematurely
+	 */
+	ps_bridge->gpio_reset = devm_gpiod_get(&client->dev, "reset",
+					       GPIOD_OUT_LOW);
+	if (IS_ERR(ps_bridge->gpio_reset)) {
+		ret = PTR_ERR(ps_bridge->gpio_reset);
+		dev_err(dev, "cannot get reset: %d\n", ret);
+		return ret;
+	}
+
+	ps_bridge->bridge.funcs = &ps8640_bridge_funcs;
+	ps_bridge->bridge.of_node = dev->of_node;
+
+	ps_bridge->page[0] = client;
+
+	/*
+	 * ps8640 uses multiple addresses, use dummy devices for them
+	 * page[0]: for DP control
+	 * page[1]: for VIDEO Bridge
+	 * page[2]: for control top
+	 * page[3]: for DSI Link Control1
+	 * page[4]: for MIPI Phy
+	 * page[5]: for VPLL
+	 * page[6]: for DSI Link Control2
+	 * page[7]: for spi rom mapping
+	 */
+	for (i = 1; i < MAX_DEVS; i++) {
+		ps_bridge->page[i] = i2c_new_dummy(client->adapter,
+						   client->addr + i);
+		if (!ps_bridge->page[i]) {
+			dev_err(dev, "failed i2c dummy device, address%02x\n",
+				client->addr + i);
+			ret = -EBUSY;
+			goto exit_dummy;
+		}
+	}
+	i2c_set_clientdata(client, ps_bridge);
+
+	ret = sysfs_create_group(&client->dev.kobj, &ps8640_attr_group);
+	if (ret) {
+		dev_err(dev, "failed to create sysfs entries: %d\n", ret);
+		goto exit_dummy;
+	}
+
+	ret = devm_add_action(dev, ps8640_remove_sysfs_group, ps_bridge);
+	if (ret) {
+		dev_err(dev, "failed to add sysfs cleanup action: %d\n", ret);
+		goto exit_remove_sysfs;
+	}
+
+	ret = drm_bridge_add(&ps_bridge->bridge);
+	if (ret) {
+		dev_err(dev, "Failed to add bridge: %d\n", ret);
+		goto exit_remove_sysfs;
+	}
+	return 0;
+
+exit_remove_sysfs:
+	sysfs_remove_group(&ps_bridge->page[0]->dev.kobj, &ps8640_attr_group);
+exit_dummy:
+	while (--i)
+		i2c_unregister_device(ps_bridge->page[i]);
+	return ret;
+}
+
+static int ps8640_remove(struct i2c_client *client)
+{
+	struct ps8640 *ps_bridge = i2c_get_clientdata(client);
+	int i = MAX_DEVS;
+
+	drm_bridge_remove(&ps_bridge->bridge);
+	sysfs_remove_group(&ps_bridge->page[0]->dev.kobj, &ps8640_attr_group);
+	while (--i)
+		i2c_unregister_device(ps_bridge->page[i]);
+
+	return 0;
+}
+
+static const struct i2c_device_id ps8640_i2c_table[] = {
+	{ "ps8640", 0 },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(i2c, ps8640_i2c_table);
+
+static const struct of_device_id ps8640_match[] = {
+	{ .compatible = "parade,ps8640" },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, ps8640_match);
+
+static struct i2c_driver ps8640_driver = {
+	.id_table = ps8640_i2c_table,
+	.probe = ps8640_probe,
+	.remove = ps8640_remove,
+	.driver = {
+		.name = "ps8640",
+		.of_match_table = ps8640_match,
+	},
+};
+module_i2c_driver(ps8640_driver);
+
+MODULE_AUTHOR("Jitao Shi <jitao.shi@mediatek.com>");
+MODULE_AUTHOR("CK Hu <ck.hu@mediatek.com>");
+MODULE_DESCRIPTION("PARADE ps8640 DSI-eDP converter driver");
+MODULE_LICENSE("GPL v2");
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 01/16] ARM: scu: Provide support for parsing SCU device node to enable SCU
From: Russell King - ARM Linux @ 2016-11-14 13:48 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1479099731-28108-2-git-send-email-pankaj.dubey@samsung.com>

This should be sent _to_ me because it's touching generic ARM code.
Thanks.

On Mon, Nov 14, 2016 at 10:31:56AM +0530, Pankaj Dubey wrote:
> Many platforms are duplicating code for enabling SCU, lets add
> common code to enable SCU by parsing SCU device node so the duplication
> in each platform can be avoided.
> 
> CC: Krzysztof Kozlowski <krzk@kernel.org>
> CC: Jisheng Zhang <jszhang@marvell.com>
> CC: Russell King <linux@armlinux.org.uk>
> CC: Dinh Nguyen <dinguyen@opensource.altera.com>
> CC: Patrice Chotard <patrice.chotard@st.com>
> CC: Linus Walleij <linus.walleij@linaro.org>
> CC: Liviu Dudau <liviu.dudau@arm.com>
> CC: Ray Jui <rjui@broadcom.com>
> CC: Stephen Warren <swarren@wwwdotorg.org>
> CC: Heiko Stuebner <heiko@sntech.de>
> CC: Shawn Guo <shawnguo@kernel.org>
> CC: Michal Simek <michal.simek@xilinx.com>
> CC: Wei Xu <xuwei5@hisilicon.com>
> CC: Andrew Lunn <andrew@lunn.ch>
> CC: Jun Nie <jun.nie@linaro.org> 
> Suggested-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> ---
>  arch/arm/include/asm/smp_scu.h |  4 +++
>  arch/arm/kernel/smp_scu.c      | 56 ++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 60 insertions(+)
> 
> diff --git a/arch/arm/include/asm/smp_scu.h b/arch/arm/include/asm/smp_scu.h
> index bfe163c..fdeec07 100644
> --- a/arch/arm/include/asm/smp_scu.h
> +++ b/arch/arm/include/asm/smp_scu.h
> @@ -39,8 +39,12 @@ static inline int scu_power_mode(void __iomem *scu_base, unsigned int mode)
>  
>  #if defined(CONFIG_SMP) && defined(CONFIG_HAVE_ARM_SCU)
>  void scu_enable(void __iomem *scu_base);
> +void __iomem *of_scu_get_base(void);
> +int of_scu_enable(void);
>  #else
>  static inline void scu_enable(void __iomem *scu_base) {}
> +static inline void __iomem *of_scu_get_base(void) {return NULL; }
> +static inline int of_scu_enable(void) {return 0; }
>  #endif
>  
>  #endif
> diff --git a/arch/arm/kernel/smp_scu.c b/arch/arm/kernel/smp_scu.c
> index 72f9241..d0ac3ed 100644
> --- a/arch/arm/kernel/smp_scu.c
> +++ b/arch/arm/kernel/smp_scu.c
> @@ -10,6 +10,7 @@
>   */
>  #include <linux/init.h>
>  #include <linux/io.h>
> +#include <linux/of_address.h>
>  
>  #include <asm/smp_plat.h>
>  #include <asm/smp_scu.h>
> @@ -70,6 +71,61 @@ void scu_enable(void __iomem *scu_base)
>  	 */
>  	flush_cache_all();
>  }
> +
> +static const struct of_device_id scu_match[] = {
> +	{ .compatible = "arm,cortex-a9-scu", },
> +	{ .compatible = "arm,cortex-a5-scu", },
> +	{ }
> +};
> +
> +/*
> + * Helper API to get SCU base address
> + * In case platform DT do not have SCU node, or iomap fails
> + * this call will fallback and will try to map via call to
> + * scu_a9_get_base.
> + * This will return ownership of scu_base to the caller
> + */
> +void __iomem *of_scu_get_base(void)
> +{
> +	unsigned long base = 0;
> +	struct device_node *np;
> +	void __iomem *scu_base;
> +
> +	np = of_find_matching_node(NULL, scu_match);
> +	scu_base = of_iomap(np, 0);
> +	of_node_put(np);
> +	if (!scu_base) {
> +		pr_err("%s failed to map scu_base via DT\n", __func__);
> +		if (scu_a9_has_base()) {
> +			base = scu_a9_get_base();
> +			scu_base = ioremap(base, SZ_4K);
> +		}
> +		if (!scu_base) {
> +			pr_err("%s failed to map scu_base\n", __func__);
> +			return IOMEM_ERR_PTR(-ENOMEM);

I can't see the point of using error-pointers here - it's not like we
really know why we're failing, so just return NULL.

> +		}
> +	}
> +	return scu_base;
> +}
> +
> +/*
> + * Enable SCU via mapping scu_base DT
> + * If scu_base mapped successfully scu will be enabled and in case of
> + * failure if will return non-zero error code
> + */
> +int of_scu_enable(void)
> +{
> +	void __iomem *scu_base;
> +
> +	scu_base = of_scu_get_base();
> +	if (!IS_ERR(scu_base)) {
> +		scu_enable(scu_base);
> +		iounmap(scu_base);
> +		return 0;
> +	}
> +	return PTR_ERR(scu_base);

and just return your -ENOMEM here.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply

* [PATCH 01/16] ARM: scu: Provide support for parsing SCU device node to enable SCU
From: Russell King - ARM Linux @ 2016-11-14 13:50 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <2479429.Js1FDSdQzs@wuerfel>

On Mon, Nov 14, 2016 at 01:03:09PM +0100, Arnd Bergmann wrote:
> On Monday, November 14, 2016 2:10:16 PM CET pankaj.dubey wrote:
> > >> +    scu_base = of_iomap(np, 0);
> > >> +    of_node_put(np);
> > >> +    if (!scu_base) {
> > >> +            pr_err("%s failed to map scu_base via DT\n", __func__);
> > > 
> > > For non-ca5, non-ca9 based SoCs, we'll see this error msg. We understand
> > > what does it mean, but it may confuse normal users. In current version,
> > > berlin doesn't complain like this for non-ca9 SoCs
> > > 
> > 
> > OK, let me see other reviewer's comment on this. Then we will decide if
> > this error message is required or can be omitted.
> 
> We need to look at all callers here, to see if the function ever gets
> called for a CPU that doesn't have an SCU. I'd say we should warn if
> we know there is an SCU but we cannot map it, but never warn on
> any of the CPU cores that don't support an SCU.

Maybe there should be two helpers:

of_scu_enable() which _only_ looks up the SCU address in DT and enables
it if it finds it, otherwise returning failure.

a9_scu_enable() which tries to use the A9 provided SCU address and
enables it if it finds it, otherwise returning failure.

Then callers can decide which of these to call, and what error messages
to print on their failures.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply

* [v17 2/2] drm/bridge: Add I2C based driver for ps8640 bridge
From: Jitao Shi @ 2016-11-14 13:50 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <30b2f209-d957-b0ed-2805-7038e4be6cf1@codeaurora.org>

Dear Archit,

  Thanks a lot for your reviewing. 
  I have sent a new patchset for those review items.

On Fri, 2016-11-11 at 11:32 +0530, Archit Taneja wrote:
> Hi Jitao,
> 
> I couldn't locate the original mail, so posting on this thread instead.
> Some comments below.
> 
> On 11/10/2016 10:09 PM, Enric Balletbo Serra wrote:
> > Hi Jitao,
> >
> > 2016-08-27 8:44 GMT+02:00 Jitao Shi <jitao.shi@mediatek.com>:
> >> This patch adds drm_bridge driver for parade DSI to eDP bridge chip.
> >>
> >> Signed-off-by: Jitao Shi <jitao.shi@mediatek.com>
> >> Reviewed-by: Daniel Kurtz <djkurtz@chromium.org>
> >> ---
> >> Changes since v16:
> >>  - Disable ps8640 DSI MCS Function.
> >>  - Rename gpios name more clearly.
> >>  - Tune the ps8640 power on sequence.
> >>
> >> Changes since v15:
> >>  - Drop drm_connector_(un)register calls from parade ps8640.
> >>    The main DRM driver mtk_drm_drv now calls
> >>    drm_connector_register_all() after drm_dev_register() in the
> >>    mtk_drm_bind() function. That function should iterate over all
> >>    connectors and call drm_connector_register() for each of them.
> >>    So, remove drm_connector_(un)register calls from parade ps8640.
> >>
> >> Changes since v14:
> >>  - update copyright info.
> >>  - change bridge_to_ps8640 and connector_to_ps8640 to inline function.
> >>  - fix some coding style.
> >>  - use sizeof as array counter.
> >>  - use drm_get_edid when read edid.
> >>  - add mutex when firmware updating.
> >>
> >> Changes since v13:
> >>  - add const on data, ps8640_write_bytes(struct i2c_client *client, const u8 *data, u16 data_len)
> >>  - fix PAGE2_SW_REST tyro.
> >>  - move the buf[3] init to entrance of the function.
> >>
> >> Changes since v12:
> >>  - fix hw_chip_id build warning
> >>
> >> Changes since v11:
> >>  - Remove depends on I2C, add DRM depends
> >>  - Reuse ps8640_write_bytes() in ps8640_write_byte()
> >>  - Use timer check for polling like the routines in <linux/iopoll.h>
> >>  - Fix no drm_connector_unregister/drm_connector_cleanup when ps8640_bridge_attach fail
> >>  - Check the ps8640 hardware id in ps8640_validate_firmware
> >>  - Remove fw_version check
> >>  - Move ps8640_validate_firmware before ps8640_enter_bl
> >>  - Add ddc_i2c unregister when probe fail and ps8640_remove
> >> ---
> >>  drivers/gpu/drm/bridge/Kconfig         |   12 +
> >>  drivers/gpu/drm/bridge/Makefile        |    1 +
> >>  drivers/gpu/drm/bridge/parade-ps8640.c | 1077 ++++++++++++++++++++++++++++++++
> >>  3 files changed, 1090 insertions(+)
> >>  create mode 100644 drivers/gpu/drm/bridge/parade-ps8640.c
> >>
> >> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> >> index b590e67..c59d043 100644
> >> --- a/drivers/gpu/drm/bridge/Kconfig
> >> +++ b/drivers/gpu/drm/bridge/Kconfig
> >> @@ -50,6 +50,18 @@ config DRM_PARADE_PS8622
> >>         ---help---
> >>           Parade eDP-LVDS bridge chip driver.
> >>
> >> +config DRM_PARADE_PS8640
> >> +       tristate "Parade PS8640 MIPI DSI to eDP Converter"
> >> +       depends on DRM
> >> +       depends on OF
> >> +       select DRM_KMS_HELPER
> >> +       select DRM_MIPI_DSI
> >> +       select DRM_PANEL
> >> +       ---help---
> >> +         Choose this option if you have PS8640 for display
> >> +         The PS8640 is a high-performance and low-power
> >> +         MIPI DSI to eDP converter
> >> +
> >>  config DRM_SII902X
> >>         tristate "Silicon Image sii902x RGB/HDMI bridge"
> >>         depends on OF
> >> diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
> >> index efdb07e..3360537 100644
> >> --- a/drivers/gpu/drm/bridge/Makefile
> >> +++ b/drivers/gpu/drm/bridge/Makefile
> >> @@ -5,6 +5,7 @@ obj-$(CONFIG_DRM_DW_HDMI) += dw-hdmi.o
> >>  obj-$(CONFIG_DRM_DW_HDMI_AHB_AUDIO) += dw-hdmi-ahb-audio.o
> >>  obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
> >>  obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o
> >> +obj-$(CONFIG_DRM_PARADE_PS8640) += parade-ps8640.o
> >>  obj-$(CONFIG_DRM_SII902X) += sii902x.o
> >>  obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o
> >>  obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
> >> diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c b/drivers/gpu/drm/bridge/parade-ps8640.c
> >> new file mode 100644
> >> index 0000000..7d67431
> >> --- /dev/null
> >> +++ b/drivers/gpu/drm/bridge/parade-ps8640.c
> >> @@ -0,0 +1,1077 @@
> >> +/*
> >> + * Copyright (c) 2016 MediaTek Inc.
> >> + *
> >> + * This program is free software; you can redistribute it and/or modify
> >> + * it under the terms of the GNU General Public License version 2 as
> >> + * published by the Free Software Foundation.
> >> + *
> >> + * This program is distributed in the hope that it will be useful,
> >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> >> + * GNU General Public License for more details.
> >> + */
> >> +
> >> +#include <linux/delay.h>
> >> +#include <linux/err.h>
> >> +#include <linux/firmware.h>
> >> +#include <linux/gpio.h>
> 
> Not needed.
> 
> >> +#include <linux/gpio/consumer.h>
> >> +#include <linux/i2c.h>
> >> +#include <linux/module.h>
> >> +#include <linux/of.h>
> >> +#include <linux/of_gpio.h>
> 
> The above 2 aren't needed.
> 
> >> +#include <linux/of_graph.h>
> >> +#include <linux/regulator/consumer.h>
> >> +#include <asm/unaligned.h>
> >> +#include <drm/drm_panel.h>
> >> +
> >> +#include <drmP.h>
> >> +#include <drm_atomic_helper.h>
> >> +#include <drm_crtc_helper.h>
> >> +#include <drm_crtc.h>
> 
> Not needed.
> 
> >> +#include <drm_edid.h>
> >> +#include <drm_mipi_dsi.h>
> >> +
> >> +#define PAGE1_VSTART           0x6b
> >> +#define PAGE2_SPI_CFG3         0x82
> >> +#define I2C_TO_SPI_RESET       0x20
> >> +#define PAGE2_ROMADD_BYTE1     0x8e
> >> +#define PAGE2_ROMADD_BYTE2     0x8f
> >> +#define PAGE2_SWSPI_WDATA      0x90
> >> +#define PAGE2_SWSPI_RDATA      0x91
> >> +#define PAGE2_SWSPI_LEN                0x92
> >> +#define PAGE2_SWSPI_CTL                0x93
> >> +#define TRIGGER_NO_READBACK    0x05
> >> +#define TRIGGER_READBACK       0x01
> >> +#define PAGE2_SPI_STATUS       0x9e
> >> +#define SPI_READY              0x0c
> >> +#define PAGE2_GPIO_L           0xa6
> >> +#define PAGE2_GPIO_H           0xa7
> >> +#define PS_GPIO9               BIT(1)
> >> +#define PAGE2_IROM_CTRL                0xb0
> >> +#define IROM_ENABLE            0xc0
> >> +#define IROM_DISABLE           0x80
> >> +#define PAGE2_SW_RESET         0xbc
> >> +#define SPI_SW_RESET           BIT(7)
> >> +#define MPU_SW_RESET           BIT(6)
> >> +#define PAGE2_ENCTLSPI_WR      0xda
> >> +#define PAGE2_I2C_BYPASS       0xea
> >> +#define I2C_BYPASS_EN          0xd0
> >> +#define PAGE2_MCS_EN           0xf3
> >> +#define MCS_EN                 BIT(0)
> >> +#define PAGE3_SET_ADD          0xfe
> >> +#define PAGE3_SET_VAL          0xff
> >> +#define VDO_CTL_ADD            0x13
> >> +#define VDO_DIS                        0x18
> >> +#define VDO_EN                 0x1c
> >> +#define PAGE4_REV_L            0xf0
> >> +#define PAGE4_REV_H            0xf1
> >> +#define PAGE4_CHIP_L           0xf2
> >> +#define PAGE4_CHIP_H           0xf3
> >> +
> >> +/* Firmware */
> >> +#define PS_FW_NAME             "ps864x_fw.bin"
> >> +
> >
> > About the firmware discussion I think that if you want to maintain the
> > upgrade firmware thing you should also include this patch in the
> > series.
> >
> >  https://chromium-review.googlesource.com/#/c/317221/
> >
> > Otherwise, if this is not really needed I think that remove this from
> > the driver is the best. Just an opinion, this is something the
> > maintainer should decide.
> 
> Was there a conclusion on this? As Daniel Kurtz suggested, can we drop
> the update firmware stuff for now and try to get the functional part
> for 4.10?
> 
> >
> >> +#define FW_CHIP_ID_OFFSET      0
> >> +#define FW_VERSION_OFFSET      2
> >> +#define EDID_I2C_ADDR          0x50
> >> +
> >> +#define WRITE_STATUS_REG_CMD   0x01
> >> +#define READ_STATUS_REG_CMD    0x05
> >> +#define BUSY                   BIT(0)
> >> +#define CLEAR_ALL_PROTECT      0x00
> >> +#define BLK_PROTECT_BITS       0x0c
> >> +#define STATUS_REG_PROTECT     BIT(7)
> >> +#define WRITE_ENABLE_CMD       0x06
> >> +#define CHIP_ERASE_CMD         0xc7
> >> +#define MAX_DEVS               0x8
> >> +
> >> +struct ps8640_info {
> >> +       u8 family_id;
> >> +       u8 variant_id;
> >> +       u16 version;
> >> +};
> >> +
> >> +struct ps8640 {
> >> +       struct drm_connector connector;
> >> +       struct drm_bridge bridge;
> >> +       struct edid *edid;
> >> +       struct mipi_dsi_device dsi;
> >> +       struct i2c_client *page[MAX_DEVS];
> >> +       struct i2c_client *ddc_i2c;
> >> +       struct regulator_bulk_data supplies[2];
> >> +       struct drm_panel *panel;
> >> +       struct gpio_desc *gpio_reset;
> >> +       struct gpio_desc *gpio_power_down;
> >> +       struct gpio_desc *gpio_mode_sel;
> >> +       bool enabled;
> >> +
> >> +       /* firmware file info */
> >> +       struct ps8640_info info;
> >> +       bool in_fw_update;
> >> +       /* for firmware update protect */
> >> +       struct mutex fw_mutex;
> >> +};
> >> +
> >> +static const u8 enc_ctrl_code[6] = { 0xaa, 0x55, 0x50, 0x41, 0x52, 0x44 };
> >> +static const u8 hw_chip_id[4] = { 0x00, 0x0a, 0x00, 0x30 };
> >> +
> >> +static inline struct ps8640 *bridge_to_ps8640(struct drm_bridge *e)
> >> +{
> >> +       return container_of(e, struct ps8640, bridge);
> >> +}
> >> +
> >> +static inline struct ps8640 *connector_to_ps8640(struct drm_connector *e)
> >> +{
> >> +       return container_of(e, struct ps8640, connector);
> >> +}
> >> +
> >> +static int ps8640_read(struct i2c_client *client, u8 reg, u8 *data,
> >> +                      u16 data_len)
> >> +{
> >> +       int ret;
> >> +       struct i2c_msg msgs[] = {
> >> +               {
> >> +                .addr = client->addr,
> >> +                .flags = 0,
> >> +                .len = 1,
> >> +                .buf = &reg,
> >> +               },
> >> +               {
> >> +                .addr = client->addr,
> >> +                .flags = I2C_M_RD,
> >> +                .len = data_len,
> >> +                .buf = data,
> >> +               }
> >> +       };
> >> +
> >> +       ret = i2c_transfer(client->adapter, msgs, 2);
> >> +
> >> +       if (ret == 2)
> >> +               return 0;
> >> +       if (ret < 0)
> >> +               return ret;
> >> +       else
> >> +               return -EIO;
> >> +}
> >> +
> >> +static int ps8640_write_bytes(struct i2c_client *client, const u8 *data,
> >> +                             u16 data_len)
> >> +{
> >> +       int ret;
> >> +       struct i2c_msg msg;
> >> +
> >> +       msg.addr = client->addr;
> >> +       msg.flags = 0;
> >> +       msg.len = data_len;
> >> +       msg.buf = (u8 *)data;
> >> +
> >> +       ret = i2c_transfer(client->adapter, &msg, 1);
> >> +       if (ret == 1)
> >> +               return 0;
> >> +       if (ret < 0)
> >> +               return ret;
> >> +       else
> >> +               return -EIO;
> >> +}
> >> +
> >> +static int ps8640_write_byte(struct i2c_client *client, u8 reg,  u8 data)
> >> +{
> >> +       u8 buf[] = { reg, data };
> >> +
> >> +       return ps8640_write_bytes(client, buf, sizeof(buf));
> >> +}
> >> +
> >> +static void ps8640_get_mcu_fw_version(struct ps8640 *ps_bridge)
> >> +{
> >> +       struct i2c_client *client = ps_bridge->page[5];
> >> +       u8 fw_ver[2];
> >> +
> >> +       ps8640_read(client, 0x4, fw_ver, sizeof(fw_ver));
> >> +       ps_bridge->info.version = (fw_ver[0] << 8) | fw_ver[1];
> >> +
> >> +       DRM_INFO_ONCE("ps8640 rom fw version %d.%d\n", fw_ver[0], fw_ver[1]);
> >> +}
> >> +
> >> +static int ps8640_bridge_unmute(struct ps8640 *ps_bridge)
> >> +{
> >> +       struct i2c_client *client = ps_bridge->page[3];
> >> +       u8 vdo_ctrl_buf[3] = { PAGE3_SET_ADD, VDO_CTL_ADD, VDO_EN };
> >> +
> >> +       return ps8640_write_bytes(client, vdo_ctrl_buf, sizeof(vdo_ctrl_buf));
> >> +}
> >> +
> >> +static int ps8640_bridge_mute(struct ps8640 *ps_bridge)
> >> +{
> >> +       struct i2c_client *client = ps_bridge->page[3];
> >> +       u8 vdo_ctrl_buf[3] = { PAGE3_SET_ADD, VDO_CTL_ADD, VDO_DIS };
> >> +
> >> +       return ps8640_write_bytes(client, vdo_ctrl_buf, sizeof(vdo_ctrl_buf));
> >> +}
> >> +
> >> +static void ps8640_pre_enable(struct drm_bridge *bridge)
> >> +{
> >> +       struct ps8640 *ps_bridge = bridge_to_ps8640(bridge);
> >> +       struct i2c_client *client = ps_bridge->page[2];
> >> +       struct i2c_client *page1 = ps_bridge->page[1];
> 
> It's a bit hard to follow what page[3] or page[5] means without going to the
> bottom and reading the dummy devices comment. It would be nice to have some
> macros here.
> 
> >> +       int err;
> >> +       u8 set_vdo_done, mcs_en, vstart;
> >> +       ktime_t timeout;
> >> +
> >> +       if (ps_bridge->in_fw_update)
> >> +               return;
> >> +
> >> +       if (ps_bridge->enabled)
> >> +               return;
> >> +
> >> +       err = drm_panel_prepare(ps_bridge->panel);
> >> +       if (err < 0) {
> >> +               DRM_ERROR("failed to prepare panel: %d\n", err);
> >> +               return;
> >> +       }
> >> +
> >> +       err = regulator_bulk_enable(ARRAY_SIZE(ps_bridge->supplies),
> >> +                                   ps_bridge->supplies);
> >> +       if (err < 0) {
> >> +               DRM_ERROR("cannot enable regulators %d\n", err);
> >> +               goto err_panel_unprepare;
> >> +       }
> >> +
> >> +       gpiod_set_value(ps_bridge->gpio_power_down, 1);
> >> +       gpiod_set_value(ps_bridge->gpio_reset, 0);
> >> +       usleep_range(2000, 2500);
> >> +       gpiod_set_value(ps_bridge->gpio_reset, 1);
> >> +
> >> +       /*
> >> +        * Wait for the ps8640 embed mcu ready
> >> +        * First wait 200ms and then check the mcu ready flag every 20ms
> >> +        */
> >> +       msleep(200);
> >> +
> >> +       timeout = ktime_add_ms(ktime_get(), 200);
> >> +       for (;;) {
> >> +               err = ps8640_read(client, PAGE2_GPIO_H, &set_vdo_done, 1);
> >> +               if (err < 0) {
> >> +                       DRM_ERROR("failed read PAGE2_GPIO_H: %d\n", err);
> >> +                       goto err_regulators_disable;
> >> +               }
> >> +               if ((set_vdo_done & PS_GPIO9) == PS_GPIO9)
> >> +                       break;
> >> +               if (ktime_compare(ktime_get(), timeout) > 0)
> >> +                       break;
> >> +               msleep(20);
> >> +       }
> >> +
> >> +       msleep(50);
> >> +
> >> +       ps8640_read(page1, PAGE1_VSTART, &vstart, 1);
> >> +       DRM_INFO("PS8640 PAGE1.0x6B = 0x%x\n", vstart);
> >> +
> >> +       /**
> >> +        * The Manufacturer Command Set (MCS) is a device dependent interface
> >> +        * intended for factory programming of the display module default
> >> +        * parameters. Once the display module is configured, the MCS shall be
> >> +        * disabled by the manufacturer. Once disabled, all MCS commands are
> >> +        * ignored by the display interface.
> >> +        */
> >> +       ps8640_read(client, PAGE2_MCS_EN, &mcs_en, 1);
> >> +       ps8640_write_byte(client, PAGE2_MCS_EN, mcs_en & ~MCS_EN);
> >> +
> >> +       if (ps_bridge->info.version == 0)
> >> +               ps8640_get_mcu_fw_version(ps_bridge);
> >> +
> >> +       err = ps8640_bridge_unmute(ps_bridge);
> >> +       if (err)
> >> +               DRM_ERROR("failed to enable unmutevideo: %d\n", err);
> >> +       /* Switch access edp panel's edid through i2c */
> >> +       ps8640_write_byte(client, PAGE2_I2C_BYPASS, I2C_BYPASS_EN);
> >> +       ps_bridge->enabled = true;
> >> +
> >> +       return;
> >> +
> >> +err_regulators_disable:
> >> +       regulator_bulk_disable(ARRAY_SIZE(ps_bridge->supplies),
> >> +                              ps_bridge->supplies);
> >> +err_panel_unprepare:
> >> +       drm_panel_unprepare(ps_bridge->panel);
> >> +}
> >> +
> >> +static void ps8640_enable(struct drm_bridge *bridge)
> >> +{
> >> +       struct ps8640 *ps_bridge = bridge_to_ps8640(bridge);
> >> +       int err;
> >> +
> >> +       err = drm_panel_enable(ps_bridge->panel);
> >> +       if (err < 0)
> >> +               DRM_ERROR("failed to enable panel: %d\n", err);
> >> +}
> >> +
> >> +static void ps8640_disable(struct drm_bridge *bridge)
> >> +{
> >> +       struct ps8640 *ps_bridge = bridge_to_ps8640(bridge);
> >> +       int err;
> >> +
> >> +       err = drm_panel_disable(ps_bridge->panel);
> >> +       if (err < 0)
> >> +               DRM_ERROR("failed to disable panel: %d\n", err);
> >> +}
> >> +
> >> +static void ps8640_post_disable(struct drm_bridge *bridge)
> >> +{
> >> +       struct ps8640 *ps_bridge = bridge_to_ps8640(bridge);
> >> +       int err;
> >> +
> >> +       if (ps_bridge->in_fw_update)
> >> +               return;
> >> +
> >> +       if (!ps_bridge->enabled)
> >> +               return;
> >> +
> >> +       ps_bridge->enabled = false;
> >> +
> >> +       err = ps8640_bridge_mute(ps_bridge);
> >> +       if (err < 0)
> >> +               DRM_ERROR("failed to unmutevideo: %d\n", err);
> >> +
> >> +       gpiod_set_value(ps_bridge->gpio_reset, 0);
> >> +       gpiod_set_value(ps_bridge->gpio_power_down, 0);
> >> +       err = regulator_bulk_disable(ARRAY_SIZE(ps_bridge->supplies),
> >> +                                    ps_bridge->supplies);
> >> +       if (err < 0)
> >> +               DRM_ERROR("cannot disable regulators %d\n", err);
> >> +
> >> +       err = drm_panel_unprepare(ps_bridge->panel);
> >> +       if (err)
> >> +               DRM_ERROR("failed to unprepare panel: %d\n", err);
> >> +}
> >> +
> >> +static int ps8640_get_modes(struct drm_connector *connector)
> >> +{
> >> +       struct ps8640 *ps_bridge = connector_to_ps8640(connector);
> >> +       struct edid *edid;
> >> +       int num_modes = 0;
> >> +       bool power_off;
> >> +
> >> +       if (ps_bridge->edid)
> >> +               return drm_add_edid_modes(connector, ps_bridge->edid);
> >> +
> >> +       power_off = !ps_bridge->enabled;
> >> +       ps8640_pre_enable(&ps_bridge->bridge);
> >> +
> >> +       edid = drm_get_edid(connector, ps_bridge->ddc_i2c->adapter);
> 
> See comments related to this in ps8640_probe.
> 
> >> +       if (!edid)
> >> +               goto out;
> >> +
> >> +       ps_bridge->edid = edid;
> >> +       drm_mode_connector_update_edid_property(connector, ps_bridge->edid);
> >> +       num_modes = drm_add_edid_modes(connector, ps_bridge->edid);
> >> +
> >> +out:
> >> +       if (power_off)
> >> +               ps8640_post_disable(&ps_bridge->bridge);
> >> +
> >> +       return num_modes;
> >> +}
> >> +
> >> +static struct drm_encoder *ps8640_best_encoder(struct drm_connector *connector)
> >> +{
> >> +       struct ps8640 *ps_bridge = connector_to_ps8640(connector);
> >> +
> >> +       return ps_bridge->bridge.encoder;
> >> +}
> 
> We can drop the above func.
> 
> >> +
> >> +static const struct drm_connector_helper_funcs ps8640_connector_helper_funcs = {
> >> +       .get_modes = ps8640_get_modes,
> >> +       .best_encoder = ps8640_best_encoder,
> >> +};
> >> +
> >> +static enum drm_connector_status ps8640_detect(struct drm_connector *connector,
> >> +                                              bool force)
> >> +{
> >> +       return connector_status_connected;
> >> +}
> >> +
> >> +static const struct drm_connector_funcs ps8640_connector_funcs = {
> >> +       .dpms = drm_atomic_helper_connector_dpms,
> >> +       .fill_modes = drm_helper_probe_single_connector_modes,
> >> +       .detect = ps8640_detect,
> >> +       .reset = drm_atomic_helper_connector_reset,
> >> +       .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> >> +       .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> >> +};
> >> +
> >> +int ps8640_bridge_attach(struct drm_bridge *bridge)
> >> +{
> >> +       struct ps8640 *ps_bridge = bridge_to_ps8640(bridge);
> >> +       struct device *dev = &ps_bridge->page[0]->dev;
> >> +       struct device_node *port, *in_ep;
> >> +       struct device_node *dsi_node = NULL;
> >> +       struct mipi_dsi_host *host = NULL;
> >> +       int ret;
> >> +
> >> +       ret = drm_connector_init(bridge->dev, &ps_bridge->connector,
> >> +                                &ps8640_connector_funcs,
> >> +                                DRM_MODE_CONNECTOR_eDP);
> >> +
> >> +       if (ret) {
> >> +               DRM_ERROR("Failed to initialize connector with drm: %d\n", ret);
> >> +               return ret;
> >> +       }
> >> +
> >> +       drm_connector_helper_add(&ps_bridge->connector,
> >> +                                &ps8640_connector_helper_funcs);
> >> +
> >> +       ps_bridge->connector.dpms = DRM_MODE_DPMS_ON;
> >> +       drm_mode_connector_attach_encoder(&ps_bridge->connector,
> >> +                                         bridge->encoder);
> >> +
> >> +       if (ps_bridge->panel)
> >> +               drm_panel_attach(ps_bridge->panel, &ps_bridge->connector);
> >> +
> >> +       /* port at 0 is ps8640 dsi input port */
> >> +       port = of_graph_get_port_by_id(dev->of_node, 0);
> >> +       if (port) {
> >> +               in_ep = of_get_child_by_name(port, "endpoint");
> >> +               of_node_put(port);
> 
> The above 2 funcs can be done by a single func: of_graph_get_endpoint_by_regs().
> 
> >> +               if (in_ep) {
> >> +                       dsi_node = of_graph_get_remote_port_parent(in_ep);
> >> +                       of_node_put(in_ep);
> >> +               }
> >> +       }
> >> +       if (dsi_node) {
> >> +               host = of_find_mipi_dsi_host_by_node(dsi_node);
> >> +               of_node_put(dsi_node);
> >> +               if (!host) {
> >> +                       ret = -ENODEV;
> >> +                       goto err;
> >> +               }
> >> +       }
> >> +
> 
> We haven't created a DSI device for this yet. Don't we need to call
> mipi_dsi_device_register_full() here?
> 
> >> +       ps_bridge->dsi.host = host;
> 
> The code above proceeds even if we don't find a dsi host. In that
> case, the host would be a NULL pointer. We shouldn't call
> mipi_dsi_attach() with a NULL host. We should have returned earlier with
> an error.
> 
> >> +       ps_bridge->dsi.mode_flags = MIPI_DSI_MODE_VIDEO |
> >> +                                    MIPI_DSI_MODE_VIDEO_SYNC_PULSE;
> >> +       ps_bridge->dsi.format = MIPI_DSI_FMT_RGB888;
> >> +       ps_bridge->dsi.lanes = 4;
> >> +       ret = mipi_dsi_attach(&ps_bridge->dsi);
> >> +       if (ret)
> >> +               goto err;
> >> +
> >> +       return 0;
> >> +err:
> >> +       if (ps_bridge->panel)
> >> +               drm_panel_detach(ps_bridge->panel);
> >> +       drm_connector_cleanup(&ps_bridge->connector);
> >> +       return ret;
> >> +}
> >> +
> >> +static const struct drm_bridge_funcs ps8640_bridge_funcs = {
> >> +       .attach = ps8640_bridge_attach,
> >> +       .disable = ps8640_disable,
> >> +       .post_disable = ps8640_post_disable,
> >> +       .pre_enable = ps8640_pre_enable,
> >> +       .enable = ps8640_enable,
> >> +};
> >> +
> >> +/* Firmware Version is returned as Major.Minor */
> >> +static ssize_t ps8640_fw_version_show(struct device *dev,
> >> +                                     struct device_attribute *attr, char *buf)
> >> +{
> >> +       struct ps8640 *ps_bridge = dev_get_drvdata(dev);
> >> +       struct ps8640_info *info = &ps_bridge->info;
> >> +
> >> +       return scnprintf(buf, PAGE_SIZE, "%u.%u\n", info->version >> 8,
> >> +                        info->version & 0xff);
> >> +}
> >> +
> >> +/* Hardware Version is returned as FamilyID.VariantID */
> >> +static ssize_t ps8640_hw_version_show(struct device *dev,
> >> +                                     struct device_attribute *attr, char *buf)
> >> +{
> >> +       struct ps8640 *ps_bridge = dev_get_drvdata(dev);
> >> +       struct ps8640_info *info = &ps_bridge->info;
> >> +
> >> +       return scnprintf(buf, PAGE_SIZE, "ps%u.%u\n", info->family_id,
> >> +                        info->variant_id);
> >> +}
> >> +
> >> +static int ps8640_spi_send_cmd(struct ps8640 *ps_bridge, u8 *cmd, u8 cmd_len)
> >> +{
> >> +       struct i2c_client *client = ps_bridge->page[2];
> >> +       u8 i, buf[3] = { PAGE2_SWSPI_LEN, cmd_len - 1, TRIGGER_NO_READBACK };
> >> +       int ret;
> >> +
> >> +       ret = ps8640_write_byte(client, PAGE2_IROM_CTRL, IROM_ENABLE);
> >> +       if (ret)
> >> +               goto err;
> >> +
> >> +       /* write command in write port */
> >> +       for (i = 0; i < cmd_len; i++) {
> >> +               ret = ps8640_write_byte(client, PAGE2_SWSPI_WDATA, cmd[i]);
> >> +               if (ret)
> >> +                       goto err_irom_disable;
> >> +       }
> >> +
> >> +       ret = ps8640_write_bytes(client, buf, sizeof(buf));
> >> +       if (ret)
> >> +               goto err_irom_disable;
> >> +
> >> +       ret = ps8640_write_byte(client, PAGE2_IROM_CTRL, IROM_DISABLE);
> >> +       if (ret)
> >> +               goto err;
> >> +
> >> +       return 0;
> >> +err_irom_disable:
> >> +       ps8640_write_byte(client, PAGE2_IROM_CTRL, IROM_DISABLE);
> >> +err:
> >> +       dev_err(&client->dev, "send command err: %d\n", ret);
> >> +       return ret;
> >> +}
> >> +
> >> +static int ps8640_wait_spi_ready(struct ps8640 *ps_bridge)
> >> +{
> >> +       struct i2c_client *client = ps_bridge->page[2];
> >> +       u8 spi_rdy_st;
> >> +       ktime_t timeout;
> >> +
> >> +       timeout = ktime_add_ms(ktime_get(), 200);
> >> +       for (;;) {
> >> +               ps8640_read(client, PAGE2_SPI_STATUS, &spi_rdy_st, 1);
> >> +               if ((spi_rdy_st & SPI_READY) != SPI_READY)
> >> +                       break;
> >> +
> >> +               if (ktime_compare(ktime_get(), timeout) > 0) {
> >> +                       dev_err(&client->dev, "wait spi ready timeout\n");
> >> +                       return -EBUSY;
> >> +               }
> >> +
> >> +               msleep(20);
> >> +       }
> >> +
> >> +       return 0;
> >> +}
> >> +
> >> +static int ps8640_wait_spi_nobusy(struct ps8640 *ps_bridge)
> >> +{
> >> +       struct i2c_client *client = ps_bridge->page[2];
> >> +       u8 spi_status, buf[3] = { PAGE2_SWSPI_LEN, 0, TRIGGER_READBACK };
> >> +       int ret;
> >> +       ktime_t timeout;
> >> +
> >> +       timeout = ktime_add_ms(ktime_get(), 500);
> >> +       for (;;) {
> >> +               /* 0x05 RDSR; Read-Status-Register */
> >> +               ret = ps8640_write_byte(client, PAGE2_SWSPI_WDATA,
> >> +                                       READ_STATUS_REG_CMD);
> >> +               if (ret)
> >> +                       goto err_send_cmd_exit;
> >> +
> >> +               ret = ps8640_write_bytes(client, buf, 3);
> >> +               if (ret)
> >> +                       goto err_send_cmd_exit;
> >> +
> >> +               /* delay for cmd send */
> >> +               usleep_range(300, 500);
> >> +               /* wait for SPI ROM until not busy */
> >> +               ret = ps8640_read(client, PAGE2_SWSPI_RDATA, &spi_status, 1);
> >> +               if (ret)
> >> +                       goto err_send_cmd_exit;
> >> +
> >> +               if (!(spi_status & BUSY))
> >> +                       break;
> >> +
> >> +               if (ktime_compare(ktime_get(), timeout) > 0) {
> >> +                       dev_err(&client->dev, "wait spi no busy timeout: %d\n",
> >> +                               ret);
> >> +                       return -EBUSY;
> >> +               }
> >> +       }
> >> +
> >> +       return 0;
> >> +
> >> +err_send_cmd_exit:
> >> +       dev_err(&client->dev, "send command err: %d\n", ret);
> >> +       return ret;
> >> +}
> >> +
> >> +static int ps8640_wait_rom_idle(struct ps8640 *ps_bridge)
> >> +{
> >> +       struct i2c_client *client = ps_bridge->page[0];
> >> +       int ret;
> >> +
> >> +       ret = ps8640_write_byte(client, PAGE2_IROM_CTRL, IROM_ENABLE);
> >> +       if (ret)
> >> +               goto exit;
> >> +
> >> +       ret = ps8640_wait_spi_ready(ps_bridge);
> >> +       if (ret)
> >> +               goto err_spi;
> >> +
> >> +       ret = ps8640_wait_spi_nobusy(ps_bridge);
> >> +       if (ret)
> >> +               goto err_spi;
> >> +
> >> +       ret = ps8640_write_byte(client, PAGE2_IROM_CTRL, IROM_DISABLE);
> >> +       if (ret)
> >> +               goto exit;
> >> +
> >> +       return 0;
> >> +
> >> +err_spi:
> >> +       ps8640_write_byte(client, PAGE2_IROM_CTRL, IROM_DISABLE);
> >> +exit:
> >> +       dev_err(&client->dev, "wait ps8640 rom idle fail: %d\n", ret);
> >> +
> >> +       return ret;
> >> +}
> >> +
> >> +static int ps8640_spi_dl_mode(struct ps8640 *ps_bridge)
> >> +{
> >> +       struct i2c_client *client = ps_bridge->page[2];
> >> +       int ret;
> >> +
> >> +       /* switch ps8640 mode to spi dl mode */
> >> +       if (ps_bridge->gpio_mode_sel)
> >> +               gpiod_set_value(ps_bridge->gpio_mode_sel, 0);
> >> +
> >> +       /* reset spi interface */
> >> +       ret = ps8640_write_byte(client, PAGE2_SW_RESET,
> >> +                               SPI_SW_RESET | MPU_SW_RESET);
> >> +       if (ret)
> >> +               goto exit;
> >> +
> >> +       ret = ps8640_write_byte(client, PAGE2_SW_RESET, MPU_SW_RESET);
> >> +       if (ret)
> >> +               goto exit;
> >> +
> >> +       return 0;
> >> +
> >> +exit:
> >> +       dev_err(&client->dev, "fail reset spi interface: %d\n", ret);
> >> +
> >> +       return ret;
> >> +}
> >> +
> >> +static int ps8640_rom_prepare(struct ps8640 *ps_bridge)
> >> +{
> >> +       struct i2c_client *client = ps_bridge->page[2];
> >> +       struct device *dev = &client->dev;
> >> +       u8 i, cmd[2];
> >> +       int ret;
> >> +
> >> +       cmd[0] = WRITE_ENABLE_CMD;
> >> +       ret = ps8640_spi_send_cmd(ps_bridge, cmd, 1);
> >> +       if (ret) {
> >> +               dev_err(dev, "failed enable-write-status-register: %d\n", ret);
> >> +               return ret;
> >> +       }
> >> +
> >> +       cmd[0] = WRITE_STATUS_REG_CMD;
> >> +       cmd[1] = CLEAR_ALL_PROTECT;
> >> +       ret = ps8640_spi_send_cmd(ps_bridge, cmd, 2);
> >> +       if (ret) {
> >> +               dev_err(dev, "fail disable all protection: %d\n", ret);
> >> +               return ret;
> >> +       }
> >> +
> >> +       /* wait for SPI module ready */
> >> +       ret = ps8640_wait_rom_idle(ps_bridge);
> >> +       if (ret) {
> >> +               dev_err(dev, "fail wait rom idle: %d\n", ret);
> >> +               return ret;
> >> +       }
> >> +
> >> +       ps8640_write_byte(client, PAGE2_IROM_CTRL, IROM_ENABLE);
> >> +       for (i = 0; i < ARRAY_SIZE(enc_ctrl_code); i++)
> >> +               ps8640_write_byte(client, PAGE2_ENCTLSPI_WR, enc_ctrl_code[i]);
> >> +       ps8640_write_byte(client, PAGE2_IROM_CTRL, IROM_DISABLE);
> >> +
> >> +       /* Enable-Write-Status-Register */
> >> +       cmd[0] = WRITE_ENABLE_CMD;
> >> +       ret = ps8640_spi_send_cmd(ps_bridge, cmd, 1);
> >> +       if (ret) {
> >> +               dev_err(dev, "fail enable-write-status-register: %d\n", ret);
> >> +               return ret;
> >> +       }
> >> +
> >> +       /* chip erase command */
> >> +       cmd[0] = CHIP_ERASE_CMD;
> >> +       ret = ps8640_spi_send_cmd(ps_bridge, cmd, 1);
> >> +       if (ret) {
> >> +               dev_err(dev, "fail disable all protection: %d\n", ret);
> >> +               return ret;
> >> +       }
> >> +
> >> +       ret = ps8640_wait_rom_idle(ps_bridge);
> >> +       if (ret) {
> >> +               dev_err(dev, "fail wait rom idle: %d\n", ret);
> >> +               return ret;
> >> +       }
> >> +
> >> +       return 0;
> >> +}
> >> +
> >> +static int ps8640_check_chip_id(struct ps8640 *ps_bridge)
> >> +{
> >> +       struct i2c_client *client = ps_bridge->page[4];
> >> +       u8 buf[4];
> >> +
> >> +       ps8640_read(client, PAGE4_REV_L, buf, 4);
> >> +       return memcmp(buf, hw_chip_id, sizeof(buf));
> >> +}
> >> +
> >> +static int ps8640_validate_firmware(struct ps8640 *ps_bridge,
> >> +                                   const struct firmware *fw)
> >> +{
> >> +       struct i2c_client *client = ps_bridge->page[0];
> >> +       u16 fw_chip_id;
> >> +
> >> +       /*
> >> +        * Get the chip_id from the firmware. Make sure that it is the
> >> +        * right controller to do the firmware and config update.
> >> +        */
> >> +       fw_chip_id = get_unaligned_le16(fw->data + FW_CHIP_ID_OFFSET);
> >> +
> >> +       if (fw_chip_id != 0x8640 && ps8640_check_chip_id(ps_bridge) == 0) {
> >> +               dev_err(&client->dev,
> >> +                       "chip id mismatch: fw 0x%x vs. chip 0x8640\n",
> >> +                       fw_chip_id);
> >> +               return -EINVAL;
> >> +       }
> >> +
> >> +       return 0;
> >> +}
> >> +
> >> +static int ps8640_write_rom(struct ps8640 *ps_bridge, const struct firmware *fw)
> >> +{
> >> +       struct i2c_client *client = ps_bridge->page[0];
> >> +       struct device *dev = &client->dev;
> >> +       struct i2c_client *client2 = ps_bridge->page[2];
> >> +       struct i2c_client *client7 = ps_bridge->page[7];
> >> +       size_t pos, cpy_len;
> >> +       u8 buf[257];
> >> +       int ret;
> >> +
> >> +       ps8640_write_byte(client2, PAGE2_SPI_CFG3, I2C_TO_SPI_RESET);
> >> +       msleep(100);
> >> +       ps8640_write_byte(client2, PAGE2_SPI_CFG3, 0x00);
> >> +
> >> +       for (pos = 0; pos < fw->size; pos += cpy_len) {
> >> +               buf[0] = PAGE2_ROMADD_BYTE1;
> >> +               buf[1] = pos >> 8;
> >> +               buf[2] = pos >> 16;
> >> +               ret = ps8640_write_bytes(client2, buf, 3);
> >> +               if (ret)
> >> +                       goto error;
> >> +               cpy_len = fw->size >= 256 + pos ? 256 : fw->size - pos;
> >> +               buf[0] = 0;
> >> +               memcpy(buf + 1, fw->data + pos, cpy_len);
> >> +               ret = ps8640_write_bytes(client7, buf, cpy_len + 1);
> >> +               if (ret)
> >> +                       goto error;
> >> +
> >> +               dev_dbg(dev, "fw update completed %zu / %zu bytes\n", pos,
> >> +                       fw->size);
> >> +       }
> >> +       return 0;
> >> +
> >> +error:
> >> +       dev_err(dev, "failed write external flash, %d\n", ret);
> >> +       return ret;
> >> +}
> >> +
> >> +static int ps8640_spi_normal_mode(struct ps8640 *ps_bridge)
> >> +{
> >> +       u8 cmd[2];
> >> +       struct i2c_client *client = ps_bridge->page[2];
> >> +
> >> +       /* Enable-Write-Status-Register */
> >> +       cmd[0] = WRITE_ENABLE_CMD;
> >> +       ps8640_spi_send_cmd(ps_bridge, cmd, 1);
> >> +
> >> +       /* protect BPL/BP0/BP1 */
> >> +       cmd[0] = WRITE_STATUS_REG_CMD;
> >> +       cmd[1] = BLK_PROTECT_BITS | STATUS_REG_PROTECT;
> >> +       ps8640_spi_send_cmd(ps_bridge, cmd, 2);
> >> +
> >> +       /* wait for SPI rom ready */
> >> +       ps8640_wait_rom_idle(ps_bridge);
> >> +
> >> +       /* disable PS8640 mapping function */
> >> +       ps8640_write_byte(client, PAGE2_ENCTLSPI_WR, 0x00);
> >> +
> >> +       if (ps_bridge->gpio_mode_sel)
> >> +               gpiod_set_value(ps_bridge->gpio_mode_sel, 1);
> >> +       return 0;
> >> +}
> >> +
> >> +static int ps8640_enter_bl(struct ps8640 *ps_bridge)
> >> +{
> >> +       ps_bridge->in_fw_update = true;
> >> +       return ps8640_spi_dl_mode(ps_bridge);
> >> +}
> >> +
> >> +static void ps8640_exit_bl(struct ps8640 *ps_bridge, const struct firmware *fw)
> >> +{
> >> +       ps8640_spi_normal_mode(ps_bridge);
> >> +       ps_bridge->in_fw_update = false;
> >> +}
> >> +
> >> +static int ps8640_load_fw(struct ps8640 *ps_bridge, const struct firmware *fw)
> >> +{
> >> +       struct i2c_client *client = ps_bridge->page[0];
> >> +       struct device *dev = &client->dev;
> >> +       int ret;
> >> +       bool ps8640_status_backup = ps_bridge->enabled;
> >> +
> >> +       ret = ps8640_validate_firmware(ps_bridge, fw);
> >> +       if (ret)
> >> +               return ret;
> >> +
> >> +       mutex_lock(&ps_bridge->fw_mutex);
> >> +       if (!ps_bridge->in_fw_update) {
> >> +               if (!ps8640_status_backup)
> >> +                       ps8640_pre_enable(&ps_bridge->bridge);
> >> +
> >> +               ret = ps8640_enter_bl(ps_bridge);
> >> +               if (ret)
> >> +                       goto exit;
> >> +       }
> >> +
> >> +       ret = ps8640_rom_prepare(ps_bridge);
> >> +       if (ret)
> >> +               goto exit;
> >> +
> >> +       ret = ps8640_write_rom(ps_bridge, fw);
> >> +
> >> +exit:
> >> +       if (ret)
> >> +               dev_err(dev, "Failed to load firmware, %d\n", ret);
> >> +
> >> +       ps8640_exit_bl(ps_bridge, fw);
> >> +       if (!ps8640_status_backup)
> >> +               ps8640_post_disable(&ps_bridge->bridge);
> >> +       mutex_unlock(&ps_bridge->fw_mutex);
> >> +       return ret;
> >> +}
> >> +
> >> +static ssize_t ps8640_update_fw_store(struct device *dev,
> >> +                                     struct device_attribute *attr,
> >> +                                     const char *buf, size_t count)
> >> +{
> >> +       struct i2c_client *client = to_i2c_client(dev);
> >> +       struct ps8640 *ps_bridge = i2c_get_clientdata(client);
> >> +       const struct firmware *fw;
> >> +       int error;
> >> +
> >> +       error = request_firmware(&fw, PS_FW_NAME, dev);
> >> +       if (error) {
> >> +               dev_err(dev, "Unable to open firmware %s: %d\n",
> >> +                       PS_FW_NAME, error);
> >> +               return error;
> >> +       }
> >> +
> >> +       error = ps8640_load_fw(ps_bridge, fw);
> >> +       if (error)
> >> +               dev_err(dev, "The firmware update failed(%d)\n", error);
> >> +       else
> >> +               dev_info(dev, "The firmware update succeeded\n");
> >> +
> >> +       release_firmware(fw);
> >> +       return error ? error : count;
> >> +}
> >> +
> >> +static DEVICE_ATTR(fw_version, S_IRUGO, ps8640_fw_version_show, NULL);
> >> +static DEVICE_ATTR(hw_version, S_IRUGO, ps8640_hw_version_show, NULL);
> >> +static DEVICE_ATTR(update_fw, S_IWUSR, NULL, ps8640_update_fw_store);
> >> +
> >> +static struct attribute *ps8640_attrs[] = {
> >> +       &dev_attr_fw_version.attr,
> >> +       &dev_attr_hw_version.attr,
> >> +       &dev_attr_update_fw.attr,
> >> +       NULL
> >> +};
> >> +
> >> +static const struct attribute_group ps8640_attr_group = {
> >> +       .attrs = ps8640_attrs,
> >> +};
> >> +
> >> +static void ps8640_remove_sysfs_group(void *data)
> >> +{
> >> +       struct ps8640 *ps_bridge = data;
> >> +
> >> +       sysfs_remove_group(&ps_bridge->page[0]->dev.kobj, &ps8640_attr_group);
> >> +}
> >> +
> >> +static int ps8640_probe(struct i2c_client *client,
> >> +                       const struct i2c_device_id *id)
> >> +{
> >> +       struct device *dev = &client->dev;
> >> +       struct ps8640 *ps_bridge;
> >> +       struct device_node *np = dev->of_node;
> >> +       struct device_node *port, *out_ep;
> >> +       struct device_node *panel_node = NULL;
> >> +       int ret;
> >> +       u32 i;
> >> +
> >> +       ps_bridge = devm_kzalloc(dev, sizeof(*ps_bridge), GFP_KERNEL);
> >> +       if (!ps_bridge)
> >> +               return -ENOMEM;
> >> +
> >> +       /* port at 1 is ps8640 output port */
> >> +       port = of_graph_get_port_by_id(np, 1);
> >> +       if (port) {
> >> +               out_ep = of_get_child_by_name(port, "endpoint");
> >> +               of_node_put(port);
> >> +               if (out_ep) {
> >> +                       panel_node = of_graph_get_remote_port_parent(out_ep);
> >> +                       of_node_put(out_ep);
> >> +               }
> >> +       }
> >> +       if (panel_node) {
> >> +               ps_bridge->panel = of_drm_find_panel(panel_node);
> >> +               of_node_put(panel_node);
> >> +               if (!ps_bridge->panel)
> >> +                       return -EPROBE_DEFER;
> >> +       }
> >> +
> >> +       mutex_init(&ps_bridge->fw_mutex);
> >> +       ps_bridge->supplies[0].supply = "vdd33";
> >> +       ps_bridge->supplies[1].supply = "vdd12";
> >> +       ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(ps_bridge->supplies),
> >> +                                     ps_bridge->supplies);
> >> +       if (ret) {
> >> +               dev_info(dev, "failed to get regulators: %d\n", ret);
> >> +               return ret;
> >> +       }
> >> +
> >> +       ps_bridge->gpio_mode_sel = devm_gpiod_get_optional(&client->dev,
> >> +                                                            "mode-sel",
> >> +                                                            GPIOD_OUT_HIGH);
> >> +       if (IS_ERR(ps_bridge->gpio_mode_sel)) {
> >> +               ret = PTR_ERR(ps_bridge->gpio_mode_sel);
> >> +               dev_err(dev, "cannot get mode-sel %d\n", ret);
> >> +               return ret;
> >> +       }
> >> +
> >> +       ps_bridge->gpio_power_down = devm_gpiod_get(&client->dev, "sleep",
> >> +                                              GPIOD_OUT_LOW);
> >> +       if (IS_ERR(ps_bridge->gpio_power_down)) {
> >> +               ret = PTR_ERR(ps_bridge->gpio_power_down);
> >> +               dev_err(dev, "cannot get sleep: %d\n", ret);
> >> +               return ret;
> >> +       }
> >> +
> >> +       /*
> >> +        * Request the reset pin low to avoid the bridge being
> >> +        * initialized prematurely
> >> +        */
> >> +       ps_bridge->gpio_reset = devm_gpiod_get(&client->dev, "reset",
> >> +                                              GPIOD_OUT_LOW);
> >> +       if (IS_ERR(ps_bridge->gpio_reset)) {
> >> +               ret = PTR_ERR(ps_bridge->gpio_reset);
> >> +               dev_err(dev, "cannot get reset: %d\n", ret);
> >> +               return ret;
> >> +       }
> >> +
> >> +       ps_bridge->bridge.funcs = &ps8640_bridge_funcs;
> >> +       ps_bridge->bridge.of_node = dev->of_node;
> >> +
> >> +       ps_bridge->page[0] = client;
> >> +       ps_bridge->ddc_i2c = i2c_new_dummy(client->adapter, EDID_I2C_ADDR);
> 
> I don't see why we need to create this dummy client. The drm edid helper
> drm_get_edid() just needs the i2c adapter to which the client is connected.
> It will internally initiate a read form the address EDID_I2C_ADDR.
> 
> I guess "drm_get_edid(connector, ps_bridge->page[0]->adapter)" should work.
> 
> >> +       if (!ps_bridge->ddc_i2c) {
> >> +               dev_err(dev, "failed ddc_i2c dummy device, address%02x\n",
> >> +                       EDID_I2C_ADDR);
> >> +               return -EBUSY;
> >> +       }
> >> +       /*
> >> +        * ps8640 uses multiple addresses, use dummy devices for them
> >> +        * page[0]: for DP control
> >> +        * page[1]: for VIDEO Bridge
> >> +        * page[2]: for control top
> >> +        * page[3]: for DSI Link Control1
> >> +        * page[4]: for MIPI Phy
> >> +        * page[5]: for VPLL
> >> +        * page[6]: for DSI Link Control2
> 
> Does this chip support 2 DSI inputs, and we're just exposing one for now?
> If so, we should probably revisit the DT bindings, so that port at 2 doesn't
> need to represent the 2nd DSI link.
> 

PS8640 has only one dsi input.
DSI Link Control1 and DSI Link Control2 just two parts of the DSI
controller.


> Thanks,
> Archit
> 

^ permalink raw reply

* [PATCH 1/4] fpga mgr: Introduce FPGA capabilities
From: atull @ 2016-11-14 14:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161107001326.7395-2-moritz.fischer@ettus.com>

On Mon, 7 Nov 2016, Moritz Fischer wrote:

> Add FPGA capabilities as a way to express the capabilities
> of a given FPGA manager.
> 
> Removes code duplication by comparing the low-level driver's
> capabilities at the framework level rather than having each driver
> check for supported operations in the write_init() callback.
> 
> This allows for extending with additional capabilities, similar
> to the the dmaengine framework's implementation.
> 
> Signed-off-by: Moritz Fischer <moritz.fischer@ettus.com>
> Cc: Alan Tull <atull@opensource.altera.com>
> Cc: Michal Simek <michal.simek@xilinx.com>
> Cc: S?ren Brinkmann <soren.brinkmann@xilinx.com>
> Cc: linux-kernel at vger.kernel.org
> Cc: linux-arm-kernel at lists.infradead.org
> ---
> 
> Changes from RFC:
> * in the RFC the caps weren't actually stored into the struct fpga_mgr
> 
> Note:
> 
> If people disagree on the typedef being a 'false positive' I can fix
> that in a future rev of the patchset.
> 
> Thanks,
> 
>     Moritz

Hi Moritz,

As I said at the Plumbers, I wasn't so sure about replacing
7 lines of code with 70 to reduce code duplication.  But it
looks useful to me and I guess I'm ok with it.  This will need
to be rebased onto the current linux-next master since my
device tree overlays stuff went in last week.

Alan

> 
> ---
>  drivers/fpga/fpga-mgr.c       | 15 ++++++++++++++
>  drivers/fpga/socfpga.c        | 10 +++++-----
>  drivers/fpga/zynq-fpga.c      |  7 ++++++-
>  include/linux/fpga/fpga-mgr.h | 46 ++++++++++++++++++++++++++++++++++++++++++-
>  4 files changed, 71 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
> index 953dc91..ed57c17 100644
> --- a/drivers/fpga/fpga-mgr.c
> +++ b/drivers/fpga/fpga-mgr.c
> @@ -49,6 +49,18 @@ int fpga_mgr_buf_load(struct fpga_manager *mgr, u32 flags, const char *buf,
>  	struct device *dev = &mgr->dev;
>  	int ret;
>  
> +	if (flags & FPGA_MGR_PARTIAL_RECONFIG &&
> +	    !fpga_mgr_has_cap(FPGA_MGR_CAP_PARTIAL_RECONF, mgr->caps)) {
> +		dev_err(dev, "Partial reconfiguration not supported\n");
> +		return -ENOTSUPP;
> +	}
> +
> +	if (flags & FPGA_MGR_FULL_RECONFIG &&
> +	    !fpga_mgr_has_cap(FPGA_MGR_CAP_FULL_RECONF, mgr->caps)) {
> +		dev_err(dev, "Full reconfiguration not supported\n");
> +		return -ENOTSUPP;
> +	}
> +
>  	/*
>  	 * Call the low level driver's write_init function.  This will do the
>  	 * device-specific things to get the FPGA into the state where it is
> @@ -245,12 +257,14 @@ EXPORT_SYMBOL_GPL(fpga_mgr_put);
>   * @dev:	fpga manager device from pdev
>   * @name:	fpga manager name
>   * @mops:	pointer to structure of fpga manager ops
> + * @caps:	fpga manager capabilities
>   * @priv:	fpga manager private data
>   *
>   * Return: 0 on success, negative error code otherwise.
>   */
>  int fpga_mgr_register(struct device *dev, const char *name,
>  		      const struct fpga_manager_ops *mops,
> +		      fpga_mgr_cap_mask_t caps,
>  		      void *priv)
>  {
>  	struct fpga_manager *mgr;
> @@ -282,6 +296,7 @@ int fpga_mgr_register(struct device *dev, const char *name,
>  	mgr->name = name;
>  	mgr->mops = mops;
>  	mgr->priv = priv;
> +	mgr->caps = caps;
>  
>  	/*
>  	 * Initialize framework state by requesting low level driver read state
> diff --git a/drivers/fpga/socfpga.c b/drivers/fpga/socfpga.c
> index 27d2ff2..fd9760c 100644
> --- a/drivers/fpga/socfpga.c
> +++ b/drivers/fpga/socfpga.c
> @@ -413,10 +413,6 @@ static int socfpga_fpga_ops_configure_init(struct fpga_manager *mgr, u32 flags,
>  	struct socfpga_fpga_priv *priv = mgr->priv;
>  	int ret;
>  
> -	if (flags & FPGA_MGR_PARTIAL_RECONFIG) {
> -		dev_err(&mgr->dev, "Partial reconfiguration not supported.\n");
> -		return -EINVAL;
> -	}
>  	/* Steps 1 - 5: Reset the FPGA */
>  	ret = socfpga_fpga_reset(mgr);
>  	if (ret)
> @@ -555,6 +551,7 @@ static int socfpga_fpga_probe(struct platform_device *pdev)
>  	struct device *dev = &pdev->dev;
>  	struct socfpga_fpga_priv *priv;
>  	struct resource *res;
> +	fpga_mgr_cap_mask_t caps;
>  	int ret;
>  
>  	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> @@ -580,8 +577,11 @@ static int socfpga_fpga_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> +	fpga_mgr_cap_zero(&caps);
> +	fpga_mgr_cap_set(FPGA_MGR_CAP_FULL_RECONF, caps);
> +
>  	return fpga_mgr_register(dev, "Altera SOCFPGA FPGA Manager",
> -				 &socfpga_fpga_ops, priv);
> +				 &socfpga_fpga_ops, caps, priv);
>  }
>  
>  static int socfpga_fpga_remove(struct platform_device *pdev)
> diff --git a/drivers/fpga/zynq-fpga.c b/drivers/fpga/zynq-fpga.c
> index c2fb412..1d37ff0 100644
> --- a/drivers/fpga/zynq-fpga.c
> +++ b/drivers/fpga/zynq-fpga.c
> @@ -410,6 +410,7 @@ static int zynq_fpga_probe(struct platform_device *pdev)
>  	struct device *dev = &pdev->dev;
>  	struct zynq_fpga_priv *priv;
>  	struct resource *res;
> +	fpga_mgr_cap_mask_t caps;
>  	int err;
>  
>  	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> @@ -461,9 +462,13 @@ static int zynq_fpga_probe(struct platform_device *pdev)
>  	zynq_fpga_write(priv, UNLOCK_OFFSET, UNLOCK_MASK);
>  
>  	clk_disable(priv->clk);
> +	fpga_mgr_cap_zero(&caps);
> +	fpga_mgr_cap_set(FPGA_MGR_CAP_FULL_RECONF, caps);
> +	fpga_mgr_cap_set(FPGA_MGR_CAP_PARTIAL_RECONF, caps);
> +
>  
>  	err = fpga_mgr_register(dev, "Xilinx Zynq FPGA Manager",
> -				&zynq_fpga_ops, priv);
> +				&zynq_fpga_ops, caps, priv);
>  	if (err) {
>  		dev_err(dev, "unable to register FPGA manager");
>  		clk_unprepare(priv->clk);
> diff --git a/include/linux/fpga/fpga-mgr.h b/include/linux/fpga/fpga-mgr.h
> index 0940bf4..e73429c 100644
> --- a/include/linux/fpga/fpga-mgr.h
> +++ b/include/linux/fpga/fpga-mgr.h
> @@ -67,6 +67,47 @@ enum fpga_mgr_states {
>   * FPGA_MGR_PARTIAL_RECONFIG: do partial reconfiguration if supported
>   */
>  #define FPGA_MGR_PARTIAL_RECONFIG	BIT(0)
> +#define FPGA_MGR_FULL_RECONFIG		BIT(1)
> +
> +enum fpga_mgr_capability {
> +	FPGA_MGR_CAP_PARTIAL_RECONF,
> +	FPGA_MGR_CAP_FULL_RECONF,
> +
> +/* last capability type for creation of the capabilities mask */
> +	FPGA_MGR_CAP_END,
> +};
> +
> +typedef struct { DECLARE_BITMAP(bits, FPGA_MGR_CAP_END); } fpga_mgr_cap_mask_t;
> +
> +#define fpga_mgr_has_cap(cap, mask) __fpga_mgr_has_cap((cap), &(mask))
> +static inline int __fpga_mgr_has_cap(enum fpga_mgr_capability cap,
> +				     fpga_mgr_cap_mask_t *mask)
> +{
> +	return test_bit(cap, mask->bits);
> +}
> +
> +#define fpga_mgr_cap_zero(mask) __fpga_mgr_cap_zero(mask)
> +static inline void __fpga_mgr_cap_zero(fpga_mgr_cap_mask_t *mask)
> +{
> +	bitmap_zero(mask->bits, FPGA_MGR_CAP_END);
> +}
> +
> +#define fpga_mgr_cap_clear(cap, mask) __fpga_mgr_cap_clear((cap), &(mask))
> +static inline void __fpga_mgr_cap_clear(enum fpga_mgr_capability cap,
> +				       fpga_mgr_cap_mask_t *mask)
> +
> +{
> +	clear_bit(cap, mask->bits);
> +}
> +
> +#define fpga_mgr_cap_set(cap, mask) __fpga_mgr_cap_set((cap), &(mask))
> +static inline void __fpga_mgr_cap_set(enum fpga_mgr_capability cap,
> +				      fpga_mgr_cap_mask_t *mask)
> +
> +{
> +	set_bit(cap, mask->bits);
> +}
> +
>  
>  /**
>   * struct fpga_manager_ops - ops for low level fpga manager drivers
> @@ -105,6 +146,7 @@ struct fpga_manager {
>  	enum fpga_mgr_states state;
>  	const struct fpga_manager_ops *mops;
>  	void *priv;
> +	fpga_mgr_cap_mask_t caps;
>  };
>  
>  #define to_fpga_manager(d) container_of(d, struct fpga_manager, dev)
> @@ -120,7 +162,9 @@ struct fpga_manager *of_fpga_mgr_get(struct device_node *node);
>  void fpga_mgr_put(struct fpga_manager *mgr);
>  
>  int fpga_mgr_register(struct device *dev, const char *name,
> -		      const struct fpga_manager_ops *mops, void *priv);
> +		      const struct fpga_manager_ops *mops,
> +		      fpga_mgr_cap_mask_t caps,
> +		      void *priv);
>  
>  void fpga_mgr_unregister(struct device *dev);
>  
> -- 
> 2.10.0
> 
> 

^ permalink raw reply

* [PATCH v2] ARM: dts: vfxxx: Enable DMA for DSPI2 and DSPI3
From: Shawn Guo @ 2016-11-14 14:02 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161114123701.4355-1-maitysanchayan@gmail.com>

On Mon, Nov 14, 2016 at 06:07:01PM +0530, Sanchayan Maity wrote:
> Enable DMA for DSPI2 and DSPI3 on Vybrid.
> 
> Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>

Applied, thanks.

^ permalink raw reply

* [PATCH V3 0/5] ARM64: More flexible HW watchpoint
From: Pratyush Anand @ 2016-11-14 14:02 UTC (permalink / raw)
  To: linux-arm-kernel

Currently, we do not support all the byte select option provided by ARM64
specs for a HW watchpoint.

This patch set will help user to instrument a watchpoint with all possible
byte select options.

Changes since v2:
- used __ffs() instead of ffs() - 1. Similarly for fls().
- fixed ptrace_hbp_get_addr() to report correct address to user space
- handling stepping for inexact watchpoint as well now.

Changes since v1:
- Introduced a new patch 3/5 where it takes care of the situation when HW
does not report a watchpoint hit with the address that matches one of the
watchpoints set.
- Added corresponding test case to test that functionality.

Pavel Labath (1):
  arm64: hw_breakpoint: Handle inexact watchpoint addresses

Pratyush Anand (4):
  hw_breakpoint: Allow watchpoint of length 3,5,6 and 7
  arm64: Allow hw watchpoint at varied offset from base address
  arm64: Allow hw watchpoint of length 3,5,6 and 7
  selftests: arm64: add test for unaligned/inexact watchpoint handling

 arch/arm64/include/asm/hw_breakpoint.h             |   6 +-
 arch/arm64/kernel/hw_breakpoint.c                  | 153 +++++++++----
 arch/arm64/kernel/ptrace.c                         |   7 +-
 include/uapi/linux/hw_breakpoint.h                 |   4 +
 tools/include/uapi/linux/hw_breakpoint.h           |   4 +
 tools/testing/selftests/breakpoints/Makefile       |   5 +-
 .../selftests/breakpoints/breakpoint_test_arm64.c  | 236 +++++++++++++++++++++
 7 files changed, 372 insertions(+), 43 deletions(-)
 create mode 100644 tools/testing/selftests/breakpoints/breakpoint_test_arm64.c

-- 
2.7.4

^ permalink raw reply

* [PATCH V3 1/5] hw_breakpoint: Allow watchpoint of length 3,5,6 and 7
From: Pratyush Anand @ 2016-11-14 14:02 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <cover.1479130617.git.panand@redhat.com>

We only support breakpoint/watchpoint of length 1, 2, 4 and 8. If we can
support other length as well, then user may watch more data with less
number of watchpoints (provided hardware supports it). For example: if we
have to watch only 4th, 5th and 6th byte from a 64 bit aligned address, we
will have to use two slots to implement it currently. One slot will watch a
half word at offset 4 and other a byte at offset 6. If we can have a
watchpoint of length 3 then we can watch it with single slot as well.

ARM64 hardware does support such functionality, therefore adding these new
definitions in generic layer.

Signed-off-by: Pratyush Anand <panand@redhat.com>
---
 include/uapi/linux/hw_breakpoint.h       | 4 ++++
 tools/include/uapi/linux/hw_breakpoint.h | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/include/uapi/linux/hw_breakpoint.h b/include/uapi/linux/hw_breakpoint.h
index b04000a2296a..2b65efd19a46 100644
--- a/include/uapi/linux/hw_breakpoint.h
+++ b/include/uapi/linux/hw_breakpoint.h
@@ -4,7 +4,11 @@
 enum {
 	HW_BREAKPOINT_LEN_1 = 1,
 	HW_BREAKPOINT_LEN_2 = 2,
+	HW_BREAKPOINT_LEN_3 = 3,
 	HW_BREAKPOINT_LEN_4 = 4,
+	HW_BREAKPOINT_LEN_5 = 5,
+	HW_BREAKPOINT_LEN_6 = 6,
+	HW_BREAKPOINT_LEN_7 = 7,
 	HW_BREAKPOINT_LEN_8 = 8,
 };
 
diff --git a/tools/include/uapi/linux/hw_breakpoint.h b/tools/include/uapi/linux/hw_breakpoint.h
index b04000a2296a..2b65efd19a46 100644
--- a/tools/include/uapi/linux/hw_breakpoint.h
+++ b/tools/include/uapi/linux/hw_breakpoint.h
@@ -4,7 +4,11 @@
 enum {
 	HW_BREAKPOINT_LEN_1 = 1,
 	HW_BREAKPOINT_LEN_2 = 2,
+	HW_BREAKPOINT_LEN_3 = 3,
 	HW_BREAKPOINT_LEN_4 = 4,
+	HW_BREAKPOINT_LEN_5 = 5,
+	HW_BREAKPOINT_LEN_6 = 6,
+	HW_BREAKPOINT_LEN_7 = 7,
 	HW_BREAKPOINT_LEN_8 = 8,
 };
 
-- 
2.7.4

^ permalink raw reply related

* [PATCH V3 2/5] arm64: Allow hw watchpoint at varied offset from base address
From: Pratyush Anand @ 2016-11-14 14:02 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <cover.1479130617.git.panand@redhat.com>

ARM64 hardware supports watchpoint at any double word aligned address.
However, it can select any consecutive bytes from offset 0 to 7 from that
base address. For example, if base address is programmed as 0x420030 and
byte select is 0x1C, then access of 0x420032,0x420033 and 0x420034 will
generate a watchpoint exception.

Currently, we do not have such modularity. We can only program byte,
halfword, word and double word access exception from any base address.

This patch adds support to overcome above limitations.

Signed-off-by: Pratyush Anand <panand@redhat.com>
---
 arch/arm64/include/asm/hw_breakpoint.h |  2 +-
 arch/arm64/kernel/hw_breakpoint.c      | 47 +++++++++++++++++-----------------
 arch/arm64/kernel/ptrace.c             |  7 ++---
 3 files changed, 28 insertions(+), 28 deletions(-)

diff --git a/arch/arm64/include/asm/hw_breakpoint.h b/arch/arm64/include/asm/hw_breakpoint.h
index 9510ace570e2..d1c3b06ad307 100644
--- a/arch/arm64/include/asm/hw_breakpoint.h
+++ b/arch/arm64/include/asm/hw_breakpoint.h
@@ -119,7 +119,7 @@ struct perf_event;
 struct pmu;
 
 extern int arch_bp_generic_fields(struct arch_hw_breakpoint_ctrl ctrl,
-				  int *gen_len, int *gen_type);
+				  int *gen_len, int *gen_type, int *offset);
 extern int arch_check_bp_in_kernelspace(struct perf_event *bp);
 extern int arch_validate_hwbkpt_settings(struct perf_event *bp);
 extern int hw_breakpoint_exceptions_notify(struct notifier_block *unused,
diff --git a/arch/arm64/kernel/hw_breakpoint.c b/arch/arm64/kernel/hw_breakpoint.c
index 948b73148d56..3f7bc65e7ef6 100644
--- a/arch/arm64/kernel/hw_breakpoint.c
+++ b/arch/arm64/kernel/hw_breakpoint.c
@@ -349,7 +349,7 @@ int arch_check_bp_in_kernelspace(struct perf_event *bp)
  * to generic breakpoint descriptions.
  */
 int arch_bp_generic_fields(struct arch_hw_breakpoint_ctrl ctrl,
-			   int *gen_len, int *gen_type)
+			   int *gen_len, int *gen_type, int *offset)
 {
 	/* Type */
 	switch (ctrl.type) {
@@ -369,8 +369,12 @@ int arch_bp_generic_fields(struct arch_hw_breakpoint_ctrl ctrl,
 		return -EINVAL;
 	}
 
+	if (!ctrl.len)
+		return -EINVAL;
+	*offset = __ffs(ctrl.len);
+
 	/* Len */
-	switch (ctrl.len) {
+	switch (ctrl.len >> *offset) {
 	case ARM_BREAKPOINT_LEN_1:
 		*gen_len = HW_BREAKPOINT_LEN_1;
 		break;
@@ -517,18 +521,17 @@ int arch_validate_hwbkpt_settings(struct perf_event *bp)
 		default:
 			return -EINVAL;
 		}
-
-		info->address &= ~alignment_mask;
-		info->ctrl.len <<= offset;
 	} else {
 		if (info->ctrl.type == ARM_BREAKPOINT_EXECUTE)
 			alignment_mask = 0x3;
 		else
 			alignment_mask = 0x7;
-		if (info->address & alignment_mask)
-			return -EINVAL;
+		offset = info->address & alignment_mask;
 	}
 
+	info->address &= ~alignment_mask;
+	info->ctrl.len <<= offset;
+
 	/*
 	 * Disallow per-task kernel breakpoints since these would
 	 * complicate the stepping code.
@@ -665,8 +668,8 @@ static int watchpoint_handler(unsigned long addr, unsigned int esr,
 			      struct pt_regs *regs)
 {
 	int i, step = 0, *kernel_step, access;
-	u32 ctrl_reg;
-	u64 val, alignment_mask;
+	u32 ctrl_reg, lens, lene;
+	u64 val;
 	struct perf_event *wp, **slots;
 	struct debug_info *debug_info;
 	struct arch_hw_breakpoint *info;
@@ -684,25 +687,21 @@ static int watchpoint_handler(unsigned long addr, unsigned int esr,
 			goto unlock;
 
 		info = counter_arch_bp(wp);
-		/* AArch32 watchpoints are either 4 or 8 bytes aligned. */
-		if (is_compat_task()) {
-			if (info->ctrl.len == ARM_BREAKPOINT_LEN_8)
-				alignment_mask = 0x7;
-			else
-				alignment_mask = 0x3;
-		} else {
-			alignment_mask = 0x7;
-		}
 
-		/* Check if the watchpoint value matches. */
+		/* Check if the watchpoint value and byte select match. */
 		val = read_wb_reg(AARCH64_DBG_REG_WVR, i);
-		if (val != (addr & ~alignment_mask))
-			goto unlock;
-
-		/* Possible match, check the byte address select to confirm. */
 		ctrl_reg = read_wb_reg(AARCH64_DBG_REG_WCR, i);
 		decode_ctrl_reg(ctrl_reg, &ctrl);
-		if (!((1 << (addr & alignment_mask)) & ctrl.len))
+		lens = ffs(ctrl.len) - 1;
+		lene = fls(ctrl.len) - 1;
+		/*
+		 * FIXME: reported address can be anywhere between "the
+		 * lowest address accessed by the memory access that
+		 * triggered the watchpoint" and "the highest watchpointed
+		 * address accessed by the memory access". So, it may not
+		 * lie in the interval of watchpoint address range.
+		 */
+		if (addr < val + lens || addr > val + lene)
 			goto unlock;
 
 		/*
diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index e0c81da60f76..fc35e06ccaac 100644
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c
@@ -327,13 +327,13 @@ static int ptrace_hbp_fill_attr_ctrl(unsigned int note_type,
 				     struct arch_hw_breakpoint_ctrl ctrl,
 				     struct perf_event_attr *attr)
 {
-	int err, len, type, disabled = !ctrl.enabled;
+	int err, len, type, offset, disabled = !ctrl.enabled;
 
 	attr->disabled = disabled;
 	if (disabled)
 		return 0;
 
-	err = arch_bp_generic_fields(ctrl, &len, &type);
+	err = arch_bp_generic_fields(ctrl, &len, &type, &offset);
 	if (err)
 		return err;
 
@@ -352,6 +352,7 @@ static int ptrace_hbp_fill_attr_ctrl(unsigned int note_type,
 
 	attr->bp_len	= len;
 	attr->bp_type	= type;
+	attr->bp_addr	+= offset;
 
 	return 0;
 }
@@ -404,7 +405,7 @@ static int ptrace_hbp_get_addr(unsigned int note_type,
 	if (IS_ERR(bp))
 		return PTR_ERR(bp);
 
-	*addr = bp ? bp->attr.bp_addr : 0;
+	*addr = bp ? counter_arch_bp(bp)->address : 0;
 	return 0;
 }
 
-- 
2.7.4

^ permalink raw reply related

* [PATCH V3 3/5] arm64: hw_breakpoint: Handle inexact watchpoint addresses
From: Pratyush Anand @ 2016-11-14 14:02 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <cover.1479130617.git.panand@redhat.com>

From: Pavel Labath <test.tberghammer@gmail.com>

Arm64 hardware does not always report a watchpoint hit address that
matches one of the watchpoints set. It can also report an address
"near" the watchpoint if a single instruction access both watched and
unwatched addresses. There is no straight-forward way, short of
disassembling the offending instruction, to map that address back to
the watchpoint.

Previously, when the hardware reported a watchpoint hit on an address
that did not match our watchpoint (this happens in case of instructions
which access large chunks of memory such as "stp") the process would
enter a loop where we would be continually resuming it (because we did
not recognise that watchpoint hit) and it would keep hitting the
watchpoint again and again. The tracing process would never get
notified of the watchpoint hit.

This commit fixes the problem by looking at the watchpoints near the
address reported by the hardware. If the address does not exactly match
one of the watchpoints we have set, it attributes the hit to the
nearest watchpoint we have.  This heuristic is a bit dodgy, but I don't
think we can do much more, given the hardware limitations.

[panand: reworked to rebase on his patches]

Signed-off-by: Pavel Labath <labath@google.com>
Signed-off-by: Pratyush Anand <panand@redhat.com>
---
 arch/arm64/kernel/hw_breakpoint.c | 96 ++++++++++++++++++++++++++++-----------
 1 file changed, 69 insertions(+), 27 deletions(-)

diff --git a/arch/arm64/kernel/hw_breakpoint.c b/arch/arm64/kernel/hw_breakpoint.c
index 3f7bc65e7ef6..f69bf368d916 100644
--- a/arch/arm64/kernel/hw_breakpoint.c
+++ b/arch/arm64/kernel/hw_breakpoint.c
@@ -664,11 +664,46 @@ static int breakpoint_handler(unsigned long unused, unsigned int esr,
 }
 NOKPROBE_SYMBOL(breakpoint_handler);
 
+/*
+ * Arm64 hardware does not always report a watchpoint hit address that matches
+ * one of the watchpoints set. It can also report an address "near" the
+ * watchpoint if a single instruction access both watched and unwatched
+ * addresses. There is no straight-forward way, short of disassembling the
+ * offending instruction, to map that address back to the watchpoint. This
+ * function computes the distance of the memory access from the watchpoint as a
+ * heuristic for the likelyhood that a given access triggered the watchpoint.
+ *
+ * See Section D2.10.5 "Determining the memory location that caused a Watchpoint
+ * exception" of ARMv8 Architecture Reference Manual for details.
+ *
+ * The function returns the distance of the address from the bytes watched by
+ * the watchpoint. In case of an exact match, it returns 0.
+ */
+static u64 get_distance_from_watchpoint(unsigned long addr, u64 val,
+					struct arch_hw_breakpoint_ctrl *ctrl)
+{
+	u64 wp_low, wp_high;
+	u32 lens, lene;
+
+	lens = ffs(ctrl->len) - 1;
+	lene = fls(ctrl->len) - 1;
+
+	wp_low = val + lens;
+	wp_high = val + lene;
+	if (addr < wp_low)
+		return wp_low - addr;
+	else if (addr > wp_high)
+		return addr - wp_high;
+	else
+		return 0;
+}
+
 static int watchpoint_handler(unsigned long addr, unsigned int esr,
 			      struct pt_regs *regs)
 {
-	int i, step = 0, *kernel_step, access;
-	u32 ctrl_reg, lens, lene;
+	int i, step = 0, *kernel_step, access, closest_match = 0;
+	u64 min_dist = -1, dist;
+	u32 ctrl_reg;
 	u64 val;
 	struct perf_event *wp, **slots;
 	struct debug_info *debug_info;
@@ -678,31 +713,15 @@ static int watchpoint_handler(unsigned long addr, unsigned int esr,
 	slots = this_cpu_ptr(wp_on_reg);
 	debug_info = &current->thread.debug;
 
+	/*
+	 * Find all watchpoints that match the reported address. If no exact
+	 * match is found. Attribute the hit to the closest watchpoint.
+	 */
+	rcu_read_lock();
 	for (i = 0; i < core_num_wrps; ++i) {
-		rcu_read_lock();
-
 		wp = slots[i];
-
 		if (wp == NULL)
-			goto unlock;
-
-		info = counter_arch_bp(wp);
-
-		/* Check if the watchpoint value and byte select match. */
-		val = read_wb_reg(AARCH64_DBG_REG_WVR, i);
-		ctrl_reg = read_wb_reg(AARCH64_DBG_REG_WCR, i);
-		decode_ctrl_reg(ctrl_reg, &ctrl);
-		lens = ffs(ctrl.len) - 1;
-		lene = fls(ctrl.len) - 1;
-		/*
-		 * FIXME: reported address can be anywhere between "the
-		 * lowest address accessed by the memory access that
-		 * triggered the watchpoint" and "the highest watchpointed
-		 * address accessed by the memory access". So, it may not
-		 * lie in the interval of watchpoint address range.
-		 */
-		if (addr < val + lens || addr > val + lene)
-			goto unlock;
+			continue;
 
 		/*
 		 * Check that the access type matches.
@@ -711,18 +730,41 @@ static int watchpoint_handler(unsigned long addr, unsigned int esr,
 		access = (esr & AARCH64_ESR_ACCESS_MASK) ? HW_BREAKPOINT_W :
 			 HW_BREAKPOINT_R;
 		if (!(access & hw_breakpoint_type(wp)))
-			goto unlock;
+			continue;
 
+		/* Check if the watchpoint value and byte select match. */
+		val = read_wb_reg(AARCH64_DBG_REG_WVR, i);
+		ctrl_reg = read_wb_reg(AARCH64_DBG_REG_WCR, i);
+		decode_ctrl_reg(ctrl_reg, &ctrl);
+		dist = get_distance_from_watchpoint(addr, val, &ctrl);
+		if (dist < min_dist) {
+			min_dist = dist;
+			closest_match = i;
+		}
+		/* Is this an exact match? */
+		if (dist != 0)
+			continue;
+
+		info = counter_arch_bp(wp);
 		info->trigger = addr;
 		perf_bp_event(wp, regs);
 
 		/* Do we need to handle the stepping? */
 		if (is_default_overflow_handler(wp))
 			step = 1;
+	}
+	if (min_dist > 0 && min_dist != -1) {
+		/* No exact match found. */
+		wp = slots[closest_match];
+		info = counter_arch_bp(wp);
+		info->trigger = addr;
+		perf_bp_event(wp, regs);
 
-unlock:
-		rcu_read_unlock();
+		/* Do we need to handle the stepping? */
+		if (is_default_overflow_handler(wp))
+			step = 1;
 	}
+	rcu_read_unlock();
 
 	if (!step)
 		return 0;
-- 
2.7.4

^ permalink raw reply related

* [PATCH V3 4/5] arm64: Allow hw watchpoint of length 3,5,6 and 7
From: Pratyush Anand @ 2016-11-14 14:02 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <cover.1479130617.git.panand@redhat.com>

Since, arm64 can support all offset within a double word limit. Therefore,
now support other lengths within that range as well.

Signed-off-by: Pratyush Anand <panand@redhat.com>
---
 arch/arm64/include/asm/hw_breakpoint.h |  4 ++++
 arch/arm64/kernel/hw_breakpoint.c      | 36 ++++++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+)

diff --git a/arch/arm64/include/asm/hw_breakpoint.h b/arch/arm64/include/asm/hw_breakpoint.h
index d1c3b06ad307..b6b167ac082b 100644
--- a/arch/arm64/include/asm/hw_breakpoint.h
+++ b/arch/arm64/include/asm/hw_breakpoint.h
@@ -77,7 +77,11 @@ static inline void decode_ctrl_reg(u32 reg,
 /* Lengths */
 #define ARM_BREAKPOINT_LEN_1	0x1
 #define ARM_BREAKPOINT_LEN_2	0x3
+#define ARM_BREAKPOINT_LEN_3	0x7
 #define ARM_BREAKPOINT_LEN_4	0xf
+#define ARM_BREAKPOINT_LEN_5	0x1f
+#define ARM_BREAKPOINT_LEN_6	0x3f
+#define ARM_BREAKPOINT_LEN_7	0x7f
 #define ARM_BREAKPOINT_LEN_8	0xff
 
 /* Kernel stepping */
diff --git a/arch/arm64/kernel/hw_breakpoint.c b/arch/arm64/kernel/hw_breakpoint.c
index f69bf368d916..504d075a1351 100644
--- a/arch/arm64/kernel/hw_breakpoint.c
+++ b/arch/arm64/kernel/hw_breakpoint.c
@@ -317,9 +317,21 @@ static int get_hbp_len(u8 hbp_len)
 	case ARM_BREAKPOINT_LEN_2:
 		len_in_bytes = 2;
 		break;
+	case ARM_BREAKPOINT_LEN_3:
+		len_in_bytes = 3;
+		break;
 	case ARM_BREAKPOINT_LEN_4:
 		len_in_bytes = 4;
 		break;
+	case ARM_BREAKPOINT_LEN_5:
+		len_in_bytes = 5;
+		break;
+	case ARM_BREAKPOINT_LEN_6:
+		len_in_bytes = 6;
+		break;
+	case ARM_BREAKPOINT_LEN_7:
+		len_in_bytes = 7;
+		break;
 	case ARM_BREAKPOINT_LEN_8:
 		len_in_bytes = 8;
 		break;
@@ -381,9 +393,21 @@ int arch_bp_generic_fields(struct arch_hw_breakpoint_ctrl ctrl,
 	case ARM_BREAKPOINT_LEN_2:
 		*gen_len = HW_BREAKPOINT_LEN_2;
 		break;
+	case ARM_BREAKPOINT_LEN_3:
+		*gen_len = HW_BREAKPOINT_LEN_3;
+		break;
 	case ARM_BREAKPOINT_LEN_4:
 		*gen_len = HW_BREAKPOINT_LEN_4;
 		break;
+	case ARM_BREAKPOINT_LEN_5:
+		*gen_len = HW_BREAKPOINT_LEN_5;
+		break;
+	case ARM_BREAKPOINT_LEN_6:
+		*gen_len = HW_BREAKPOINT_LEN_6;
+		break;
+	case ARM_BREAKPOINT_LEN_7:
+		*gen_len = HW_BREAKPOINT_LEN_7;
+		break;
 	case ARM_BREAKPOINT_LEN_8:
 		*gen_len = HW_BREAKPOINT_LEN_8;
 		break;
@@ -427,9 +451,21 @@ static int arch_build_bp_info(struct perf_event *bp)
 	case HW_BREAKPOINT_LEN_2:
 		info->ctrl.len = ARM_BREAKPOINT_LEN_2;
 		break;
+	case HW_BREAKPOINT_LEN_3:
+		info->ctrl.len = ARM_BREAKPOINT_LEN_3;
+		break;
 	case HW_BREAKPOINT_LEN_4:
 		info->ctrl.len = ARM_BREAKPOINT_LEN_4;
 		break;
+	case HW_BREAKPOINT_LEN_5:
+		info->ctrl.len = ARM_BREAKPOINT_LEN_5;
+		break;
+	case HW_BREAKPOINT_LEN_6:
+		info->ctrl.len = ARM_BREAKPOINT_LEN_6;
+		break;
+	case HW_BREAKPOINT_LEN_7:
+		info->ctrl.len = ARM_BREAKPOINT_LEN_7;
+		break;
 	case HW_BREAKPOINT_LEN_8:
 		info->ctrl.len = ARM_BREAKPOINT_LEN_8;
 		break;
-- 
2.7.4

^ permalink raw reply related

* [PATCH V3 5/5] selftests: arm64: add test for unaligned/inexact watchpoint handling
From: Pratyush Anand @ 2016-11-14 14:02 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <cover.1479130617.git.panand@redhat.com>

ARM64 hardware expects 64bit aligned address for watchpoint invocation.
However, it provides byte selection method to select any number of
consecutive byte set within the range of 1-8.

This patch adds support to test all such byte selection option for
different memory write sizes.

Patch also adds a test for handling the case when the cpu does not
report an address which exactly matches one of the regions we have
been watching (which is a situation permitted by the spec if an
instruction accesses both watched and unwatched regions). The test
was failing on a MSM8996pro before this patch series and is
passing now.

Signed-off-by: Pavel Labath <labath@google.com>
Signed-off-by: Pratyush Anand <panand@redhat.com>
---
 tools/testing/selftests/breakpoints/Makefile       |   5 +-
 .../selftests/breakpoints/breakpoint_test_arm64.c  | 236 +++++++++++++++++++++
 2 files changed, 240 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/breakpoints/breakpoint_test_arm64.c

diff --git a/tools/testing/selftests/breakpoints/Makefile b/tools/testing/selftests/breakpoints/Makefile
index 74e533fd4bc5..61b79e8df1f4 100644
--- a/tools/testing/selftests/breakpoints/Makefile
+++ b/tools/testing/selftests/breakpoints/Makefile
@@ -5,6 +5,9 @@ ARCH ?= $(shell echo $(uname_M) | sed -e s/i.86/x86/ -e s/x86_64/x86/)
 ifeq ($(ARCH),x86)
 TEST_PROGS := breakpoint_test
 endif
+ifeq ($(ARCH),aarch64)
+TEST_PROGS := breakpoint_test_arm64
+endif
 
 TEST_PROGS += step_after_suspend_test
 
@@ -13,4 +16,4 @@ all: $(TEST_PROGS)
 include ../lib.mk
 
 clean:
-	rm -fr breakpoint_test step_after_suspend_test
+	rm -fr breakpoint_test breakpoint_test_arm64 step_after_suspend_test
diff --git a/tools/testing/selftests/breakpoints/breakpoint_test_arm64.c b/tools/testing/selftests/breakpoints/breakpoint_test_arm64.c
new file mode 100644
index 000000000000..3897e996541e
--- /dev/null
+++ b/tools/testing/selftests/breakpoints/breakpoint_test_arm64.c
@@ -0,0 +1,236 @@
+/*
+ * Copyright (C) 2016 Google, Inc.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * Original Code by Pavel Labath <labath@google.com>
+ *
+ * Code modified by Pratyush Anand <panand@redhat.com>
+ * for testing different byte select for each access size.
+ *
+ */
+
+#define _GNU_SOURCE
+
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <sys/ptrace.h>
+#include <sys/param.h>
+#include <sys/uio.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <stddef.h>
+#include <string.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <elf.h>
+#include <errno.h>
+#include <signal.h>
+
+#include "../kselftest.h"
+
+static volatile uint8_t var[96] __attribute__((__aligned__(32)));
+
+static void child(int size, int wr)
+{
+	volatile uint8_t *addr = &var[32 + wr];
+
+	if (ptrace(PTRACE_TRACEME, 0, NULL, NULL) != 0) {
+		perror("ptrace(PTRACE_TRACEME) failed");
+		_exit(1);
+	}
+
+	if (raise(SIGSTOP) != 0) {
+		perror("raise(SIGSTOP) failed");
+		_exit(1);
+	}
+
+	if ((uintptr_t) addr % size) {
+		perror("Wrong address write for the given size\n");
+		_exit(1);
+	}
+	switch (size) {
+	case 1:
+		*addr = 47;
+		break;
+	case 2:
+		*(uint16_t *)addr = 47;
+		break;
+	case 4:
+		*(uint32_t *)addr = 47;
+		break;
+	case 8:
+		*(uint64_t *)addr = 47;
+		break;
+	case 16:
+		__asm__ volatile ("stp x29, x30, %0" : "=m" (addr[0]));
+		break;
+	case 32:
+		__asm__ volatile ("stp q29, q30, %0" : "=m" (addr[0]));
+		break;
+	}
+
+	_exit(0);
+}
+
+static bool set_watchpoint(pid_t pid, int size, int wp)
+{
+	const volatile uint8_t *addr = &var[32 + wp];
+	const int offset = (uintptr_t)addr % 8;
+	const unsigned int byte_mask = ((1 << size) - 1) << offset;
+	const unsigned int type = 2; /* Write */
+	const unsigned int enable = 1;
+	const unsigned int control = byte_mask << 5 | type << 3 | enable;
+	struct user_hwdebug_state dreg_state;
+	struct iovec iov;
+
+	memset(&dreg_state, 0, sizeof(dreg_state));
+	dreg_state.dbg_regs[0].addr = (uintptr_t)(addr - offset);
+	dreg_state.dbg_regs[0].ctrl = control;
+	iov.iov_base = &dreg_state;
+	iov.iov_len = offsetof(struct user_hwdebug_state, dbg_regs) +
+				sizeof(dreg_state.dbg_regs[0]);
+	if (ptrace(PTRACE_SETREGSET, pid, NT_ARM_HW_WATCH, &iov) == 0)
+		return true;
+
+	if (errno == EIO) {
+		printf("ptrace(PTRACE_SETREGSET, NT_ARM_HW_WATCH) "
+			"not supported on this hardware\n");
+		ksft_exit_skip();
+	}
+	perror("ptrace(PTRACE_SETREGSET, NT_ARM_HW_WATCH) failed");
+	return false;
+}
+
+static bool run_test(int wr_size, int wp_size, int wr, int wp)
+{
+	int status;
+	siginfo_t siginfo;
+	pid_t pid = fork();
+	pid_t wpid;
+
+	if (pid < 0) {
+		perror("fork() failed");
+		return false;
+	}
+	if (pid == 0)
+		child(wr_size, wr);
+
+	wpid = waitpid(pid, &status, __WALL);
+	if (wpid != pid) {
+		perror("waitpid() failed");
+		return false;
+	}
+	if (!WIFSTOPPED(status)) {
+		printf("child did not stop\n");
+		return false;
+	}
+	if (WSTOPSIG(status) != SIGSTOP) {
+		printf("child did not stop with SIGSTOP\n");
+		return false;
+	}
+
+	if (!set_watchpoint(pid, wp_size, wp))
+		return false;
+
+	if (ptrace(PTRACE_CONT, pid, NULL, NULL) < 0) {
+		perror("ptrace(PTRACE_SINGLESTEP) failed");
+		return false;
+	}
+
+	alarm(3);
+	wpid = waitpid(pid, &status, __WALL);
+	if (wpid != pid) {
+		perror("waitpid() failed");
+		return false;
+	}
+	alarm(0);
+	if (WIFEXITED(status)) {
+		printf("child did not single-step\t");
+		return false;
+	}
+	if (!WIFSTOPPED(status)) {
+		printf("child did not stop\n");
+		return false;
+	}
+	if (WSTOPSIG(status) != SIGTRAP) {
+		printf("child did not stop with SIGTRAP\n");
+		return false;
+	}
+	if (ptrace(PTRACE_GETSIGINFO, pid, NULL, &siginfo) != 0) {
+		perror("ptrace(PTRACE_GETSIGINFO)");
+		return false;
+	}
+	if (siginfo.si_code != TRAP_HWBKPT) {
+		printf("Unexpected si_code %d\n", siginfo.si_code);
+		return false;
+	}
+
+	kill(pid, SIGKILL);
+	wpid = waitpid(pid, &status, 0);
+	if (wpid != pid) {
+		perror("waitpid() failed");
+		return false;
+	}
+	return true;
+}
+
+static void sigalrm(int sig)
+{
+}
+
+int main(int argc, char **argv)
+{
+	int opt;
+	bool succeeded = true;
+	struct sigaction act;
+	int wr, wp, size;
+	bool result;
+
+	act.sa_handler = sigalrm;
+	sigemptyset(&act.sa_mask);
+	act.sa_flags = 0;
+	sigaction(SIGALRM, &act, NULL);
+	for (size = 1; size <= 32; size = size*2) {
+		for (wr = 0; wr <= 32; wr = wr + size) {
+			for (wp = wr - size; wp <= wr + size; wp = wp + size) {
+				printf("Test size = %d write offset = %d watchpoint offset = %d\t", size, wr, wp);
+				result = run_test(size, MIN(size, 8), wr, wp);
+				if ((result && wr == wp) || (!result && wr != wp)) {
+					printf("[OK]\n");
+					ksft_inc_pass_cnt();
+				} else {
+					printf("[FAILED]\n");
+					ksft_inc_fail_cnt();
+					succeeded = false;
+				}
+			}
+		}
+	}
+
+	for (size = 1; size <= 32; size = size*2) {
+		printf("Test size = %d write offset = %d watchpoint offset = -8\t", size, -size);
+
+		if (run_test(size, 8, -size, -8)) {
+			printf("[OK]\n");
+			ksft_inc_pass_cnt();
+		} else {
+			printf("[FAILED]\n");
+			ksft_inc_fail_cnt();
+			succeeded = false;
+		}
+	}
+
+	ksft_print_cnts();
+	if (succeeded)
+		ksft_exit_pass();
+	else
+		ksft_exit_fail();
+}
-- 
2.7.4

^ permalink raw reply related

* [PATCH 1/4] fpga mgr: Introduce FPGA capabilities
From: atull @ 2016-11-14 14:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161107001326.7395-2-moritz.fischer@ettus.com>

On Mon, 7 Nov 2016, Moritz Fischer wrote:

> Add FPGA capabilities as a way to express the capabilities
> of a given FPGA manager.
> 
> Removes code duplication by comparing the low-level driver's
> capabilities at the framework level rather than having each driver
> check for supported operations in the write_init() callback.
> 
> This allows for extending with additional capabilities, similar
> to the the dmaengine framework's implementation.
> 
> Signed-off-by: Moritz Fischer <moritz.fischer@ettus.com>
> Cc: Alan Tull <atull@opensource.altera.com>
> Cc: Michal Simek <michal.simek@xilinx.com>
> Cc: S?ren Brinkmann <soren.brinkmann@xilinx.com>
> Cc: linux-kernel at vger.kernel.org
> Cc: linux-arm-kernel at lists.infradead.org
> ---
> 
> Changes from RFC:
> * in the RFC the caps weren't actually stored into the struct fpga_mgr
> 
> Note:
> 
> If people disagree on the typedef being a 'false positive' I can fix
> that in a future rev of the patchset.
> 
> Thanks,
> 
>     Moritz
> 
> ---
>  drivers/fpga/fpga-mgr.c       | 15 ++++++++++++++
>  drivers/fpga/socfpga.c        | 10 +++++-----
>  drivers/fpga/zynq-fpga.c      |  7 ++++++-
>  include/linux/fpga/fpga-mgr.h | 46 ++++++++++++++++++++++++++++++++++++++++++-
>  4 files changed, 71 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
> index 953dc91..ed57c17 100644
> --- a/drivers/fpga/fpga-mgr.c
> +++ b/drivers/fpga/fpga-mgr.c
> @@ -49,6 +49,18 @@ int fpga_mgr_buf_load(struct fpga_manager *mgr, u32 flags, const char *buf,
>  	struct device *dev = &mgr->dev;
>  	int ret;
>  
> +	if (flags & FPGA_MGR_PARTIAL_RECONFIG &&
> +	    !fpga_mgr_has_cap(FPGA_MGR_CAP_PARTIAL_RECONF, mgr->caps)) {
> +		dev_err(dev, "Partial reconfiguration not supported\n");
> +		return -ENOTSUPP;
> +	}
> +
> +	if (flags & FPGA_MGR_FULL_RECONFIG &&
> +	    !fpga_mgr_has_cap(FPGA_MGR_CAP_FULL_RECONF, mgr->caps)) {
> +		dev_err(dev, "Full reconfiguration not supported\n");
> +		return -ENOTSUPP;
> +	}
> +

Could you move the checks to their own function like
'fpga_mgr_check_caps()' or something?  I really like it if we can keep
the functions short, like a screen or so where it's practicle to do
so and I could see the number of caps growing here.  The only
counter argument I could think of is if a cap affects the sequence
in this function.  Hmmm...

Alan

^ permalink raw reply

* [PATCH v6 7/9] drm/hisilicon/hibmc: Add connector for VDAC
From: Rongrong Zou @ 2016-11-14 14:07 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAOw6vbJA3Oe3GmRZBZjF24wtu_5hyTdVL2HK-1p3BLbiY1G7Tw@mail.gmail.com>

? 2016/11/11 9:45, Sean Paul ??:
> On Fri, Oct 28, 2016 at 3:28 AM, Rongrong Zou <zourongrong@gmail.com> wrote:
>> Add connector funcs and helper funcs for VDAC.
>>
>> Signed-off-by: Rongrong Zou <zourongrong@gmail.com>
>> ---
>>   drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c  |  8 +++
>>   drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h  |  2 +
>>   drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c | 76 ++++++++++++++++++++++++
>>   3 files changed, 86 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
>> index ba191e1..4253603 100644
>> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
>> @@ -131,6 +131,14 @@ static int hibmc_kms_init(struct hibmc_drm_device *hidev)
>>                  return ret;
>>          }
>>
>> +       ret = hibmc_connector_init(hidev);
>> +       if (ret) {
>> +               DRM_ERROR("failed to init connector\n");
>> +               return ret;
>> +       }
>> +
>> +       drm_mode_connector_attach_encoder(&hidev->connector,
>> +                                         &hidev->encoder);
>
> The connector should be initialized in the vdac driver with the
> encoder, not in the drv file (same as plane/crtc)

applied, thanks.

>
>>          return 0;
>>   }
>>
>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
>> index 401cea4..450247d 100644
>> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
>> @@ -48,6 +48,7 @@ struct hibmc_drm_device {
>>          struct drm_plane plane;
>>          struct drm_crtc crtc;
>>          struct drm_encoder encoder;
>> +       struct drm_connector connector;
>
> No need to keep track here

will allocate with devm_kzalloc(), thanks.

>
>>          bool mode_config_initialized;
>>
>>          /* ttm */
>> @@ -89,6 +90,7 @@ static inline struct hibmc_bo *gem_to_hibmc_bo(struct drm_gem_object *gem)
>>   int hibmc_plane_init(struct hibmc_drm_device *hidev);
>>   int hibmc_crtc_init(struct hibmc_drm_device *hidev);
>>   int hibmc_encoder_init(struct hibmc_drm_device *hidev);
>> +int hibmc_connector_init(struct hibmc_drm_device *hidev);
>>   int hibmc_fbdev_init(struct hibmc_drm_device *hidev);
>>   void hibmc_fbdev_fini(struct hibmc_drm_device *hidev);
>>
>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
>> index 953f659..ebefcd1 100644
>> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
>> @@ -87,3 +87,79 @@ int hibmc_encoder_init(struct hibmc_drm_device *hidev)
>>          drm_encoder_helper_add(encoder, &hibmc_encoder_helper_funcs);
>>          return 0;
>>   }
>> +
>> +static int hibmc_connector_get_modes(struct drm_connector *connector)
>> +{
>> +       int count;
>> +
>> +       count = drm_add_modes_noedid(connector, 800, 600);
>> +       drm_set_preferred_mode(connector, defx, defy);
>
> So you have defx/defy as module parameters, but then hardcode the
> 800x600 mode. If defx/defy is anything other than 800/600, this won't
> work. I think you should just remove the defx/defy module params and
> rely on userspace adding modes as appropriate.

will remove them, thanks.

>
>> +       return count;
>> +}
>> +
>> +static int hibmc_connector_mode_valid(struct drm_connector *connector,
>> +                                     struct drm_display_mode *mode)
>> +{
>> +       struct hibmc_drm_device *hiprivate =
>> +        container_of(connector, struct hibmc_drm_device, connector);
>> +       unsigned long size = mode->hdisplay * mode->vdisplay * 4;
>
> Why * 4 here and why * 2 below? You support formats less than 32 bpp,
> so the * 4 isn't necessarily correct for all formats. Is the * 2 to
> account for front & back buffer?
>

it is from bochs, the original comments below:
	/*
	 * Make sure we can fit two framebuffers into video memory.
	 * This allows up to 1600x1200 with 16 MB (default size).
	 * If you want more try this:
	 *     'qemu -vga std -global VGA.vgamem_mb=32 $otherargs'
	 */
i think it is not necessary for hibmc, so will remove it and return
MODE_OK, thanks.

>
>> +
>> +       if (size * 2 > hiprivate->fb_size)
>> +               return MODE_BAD;
>> +
>> +       return MODE_OK;
>> +}
>> +
>> +static struct drm_encoder *
>> +hibmc_connector_best_encoder(struct drm_connector *connector)
>> +{
>> +       int enc_id = connector->encoder_ids[0];
>> +
>> +       /* pick the encoder ids */
>> +       if (enc_id)
>> +               return drm_encoder_find(connector->dev, enc_id);
>
> Can't you just do return drm_encoder_find(connector->dev,
> connector->encoder_ids[0]); ?
>
> ie: won't drm_encoder_find do the right thing if you pass in id == 0?

hibmc_connector_best_encoder(struct drm_connector *connector)
{
	return drm_encoder_find(connector->dev, connector->encoder_ids[0]);
}
looks more simple, i like it, thanks:)

>
>> +
>> +       return NULL;
>> +}
>> +
>> +static enum drm_connector_status hibmc_connector_detect(struct drm_connector
>> +                                                *connector, bool force)
>> +{
>> +       return connector_status_connected;
>
> Perhaps this should be connector_status_unknown, since you don't
> necessarily know it's connected.

it is always connected in hibmc's scenario, usually it is used as a
management chip on server, we use KVM(keyboard, mouse, video) rather than
a directly connected monitor. I will modify if a phisical monitor
is connected later.

>
>> +}
>> +
>> +static const struct drm_connector_helper_funcs
>> +       hibmc_connector_connector_helper_funcs = {
>> +       .get_modes = hibmc_connector_get_modes,
>> +       .mode_valid = hibmc_connector_mode_valid,
>> +       .best_encoder = hibmc_connector_best_encoder,
>> +};
>> +
>> +static const struct drm_connector_funcs hibmc_connector_connector_funcs = {
>> +       .dpms = drm_atomic_helper_connector_dpms,
>> +       .detect = hibmc_connector_detect,
>> +       .fill_modes = drm_helper_probe_single_connector_modes,
>> +       .destroy = drm_connector_cleanup,
>> +       .reset = drm_atomic_helper_connector_reset,
>> +       .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
>> +       .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>> +};
>> +
>> +int hibmc_connector_init(struct hibmc_drm_device *hidev)
>> +{
>> +       struct drm_device *dev = hidev->dev;
>> +       struct drm_connector *connector = &hidev->connector;
>> +       int ret;
>> +
>> +       ret = drm_connector_init(dev, connector,
>> +                                &hibmc_connector_connector_funcs,
>> +                                DRM_MODE_CONNECTOR_VGA);
>> +       if (ret) {
>> +               DRM_ERROR("failed to init connector\n");
>> +               return ret;
>> +       }
>> +       drm_connector_helper_add(connector,
>> +                                &hibmc_connector_connector_helper_funcs);
>> +
>> +       return 0;
>> +}
>> --
>> 1.9.1
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> _______________________________________________
> linuxarm mailing list
> linuxarm at huawei.com
> http://rnd-openeuler.huawei.com/mailman/listinfo/linuxarm
>
> .
>


-- 
Regards, Rongrong

^ permalink raw reply

* [PATCH v6 8/9] drm/hisilicon/hibmc: Add vblank interruput
From: Rongrong Zou @ 2016-11-14 14:10 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAOw6vbKx88Bz4RcD3a0K7m-uomHx9wOGN6y7eOuq+yeUdRjZ5Q@mail.gmail.com>

? 2016/11/11 9:49, Sean Paul ??:
> On Fri, Oct 28, 2016 at 3:28 AM, Rongrong Zou <zourongrong@gmail.com> wrote:
>> Add vblank interrupt.
>>
>> Signed-off-by: Rongrong Zou <zourongrong@gmail.com>
>> ---
>>   drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 56 ++++++++++++++++++++++++-
>>   drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h |  1 +
>>   2 files changed, 56 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
>> index 4253603..b668e3e 100644
>> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
>> @@ -40,16 +40,46 @@
>>
>>   static int hibmc_enable_vblank(struct drm_device *dev, unsigned int pipe)
>>   {
>> +       struct hibmc_drm_device *hidev =
>> +               (struct hibmc_drm_device *)dev->dev_private;
>> +
>> +       writel(HIBMC_RAW_INTERRUPT_EN_VBLANK(1),
>> +              hidev->mmio + HIBMC_RAW_INTERRUPT_EN);
>> +
>>          return 0;
>>   }
>>
>>   static void hibmc_disable_vblank(struct drm_device *dev, unsigned int pipe)
>>   {
>> +       struct hibmc_drm_device *hidev =
>> +               (struct hibmc_drm_device *)dev->dev_private;
>> +
>> +       writel(HIBMC_RAW_INTERRUPT_EN_VBLANK(0),
>> +              hidev->mmio + HIBMC_RAW_INTERRUPT_EN);
>> +}
>> +
>> +irqreturn_t hibmc_drm_interrupt(int irq, void *arg)
>> +{
>> +       struct drm_device *dev = (struct drm_device *)arg;
>> +       struct hibmc_drm_device *hidev =
>> +               (struct hibmc_drm_device *)dev->dev_private;
>> +       struct drm_crtc *crtc = &hidev->crtc;
>> +       u32 status;
>> +
>> +       status = readl(hidev->mmio + HIBMC_RAW_INTERRUPT);
>> +
>> +       if (status & HIBMC_RAW_INTERRUPT_VBLANK(1)) {
>> +               writel(HIBMC_RAW_INTERRUPT_VBLANK(1),
>> +                      hidev->mmio + HIBMC_RAW_INTERRUPT);
>> +               drm_crtc_handle_vblank(crtc);
>> +       }
>> +
>> +       return IRQ_HANDLED;
>>   }
>>
>>   static struct drm_driver hibmc_driver = {
>>          .driver_features        = DRIVER_GEM | DRIVER_MODESET |
>> -                                 DRIVER_ATOMIC,
>> +                                 DRIVER_ATOMIC | DRIVER_HAVE_IRQ,
>>          .fops                   = &hibmc_fops,
>>          .name                   = "hibmc",
>>          .date                   = "20160828",
>> @@ -63,6 +93,7 @@ static void hibmc_disable_vblank(struct drm_device *dev, unsigned int pipe)
>>          .dumb_create            = hibmc_dumb_create,
>>          .dumb_map_offset        = hibmc_dumb_mmap_offset,
>>          .dumb_destroy           = drm_gem_dumb_destroy,
>> +       .irq_handler            = hibmc_drm_interrupt,
>>   };
>>
>>   static int hibmc_pm_suspend(struct device *dev)
>> @@ -242,6 +273,13 @@ static int hibmc_unload(struct drm_device *dev)
>>          struct hibmc_drm_device *hidev = dev->dev_private;
>>
>>          hibmc_fbdev_fini(hidev);
>> +
>> +       if (dev->irq_enabled)
>> +               drm_irq_uninstall(dev);
>> +       if (hidev->msi_enabled)
>> +               pci_disable_msi(dev->pdev);
>> +       drm_vblank_cleanup(dev);
>> +
>>          hibmc_kms_fini(hidev);
>>          hibmc_mm_fini(hidev);
>>          hibmc_hw_fini(hidev);
>> @@ -272,6 +310,22 @@ static int hibmc_load(struct drm_device *dev, unsigned long flags)
>>          if (ret)
>>                  goto err;
>>
>> +       ret = drm_vblank_init(dev, dev->mode_config.num_crtc);
>> +       if (ret) {
>> +               DRM_ERROR("failed to initialize vblank.\n");
>> +               goto err;
>> +       }
>> +
>> +       hidev->msi_enabled = 0;
>> +       if (pci_enable_msi(dev->pdev)) {
>
> It would be useful to check and print the return value of this.

agreed, thanks.

>
>> +               DRM_ERROR("Enabling MSI failed!\n");
>> +       } else {
>> +               hidev->msi_enabled = 1;
>> +               ret = drm_irq_install(dev, dev->pdev->irq);
>> +               if (ret)
>> +                       DRM_ERROR("install irq failed , ret = %d\n", ret);
>
> DRM_WARN might be more appropriate, given that this isn't considered fatal.

agreed, thanks.

>
>> +       }
>> +
>>          /* reset all the states of crtc/plane/encoder/connector */
>>          drm_mode_config_reset(dev);
>>
>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
>> index 450247d..f1706fb 100644
>> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
>> @@ -42,6 +42,7 @@ struct hibmc_drm_device {
>>          void __iomem   *fb_map;
>>          unsigned long  fb_base;
>>          unsigned long  fb_size;
>> +       int msi_enabled;
>
> Why not bool?

agreed, thanks.

Regards,
Rongrong.

>
>>
>>          /* drm */
>>          struct drm_device  *dev;
>> --
>> 1.9.1
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> _______________________________________________
> linuxarm mailing list
> linuxarm at huawei.com
> http://rnd-openeuler.huawei.com/mailman/listinfo/linuxarm
>
> .
>

^ permalink raw reply

* [PATCH v4 1/2] iommu/dma: Implement dma_{map,unmap}_resource()
From: Catalin Marinas @ 2016-11-14 14:12 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <d1cbd5ce714ce3c978aa8bfcb31def87d7919198.1479125555.git.robin.murphy@arm.com>

On Mon, Nov 14, 2016 at 12:16:26PM +0000, Robin Murphy wrote:
> With the new dma_{map,unmap}_resource() functions added to the DMA API
> for the benefit of cases like slave DMA, add suitable implementations to
> the arsenal of our generic layer. Since cache maintenance should not be
> a concern, these can both be standalone callback implementations without
> the need for arch code wrappers.
> 
> CC: Joerg Roedel <joro@8bytes.org>
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>

FWIW:

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

^ permalink raw reply

* [PATCH 12/16] ARM: imx: use generic API for enabling SCU
From: Shawn Guo @ 2016-11-14 14:26 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1479099731-28108-13-git-send-email-pankaj.dubey@samsung.com>

On Mon, Nov 14, 2016 at 10:32:07AM +0530, Pankaj Dubey wrote:
> Now as we have of_scu_enable which takes care of mapping
> scu base from DT, lets use it.
> 
> At the same time this patch cleans up mach-imx platform files by
> removing static mapping of SCU and dropping imx_scu_map_io function.

I remember that the static mapping of SCU is necessary because SCU is
being accessed at very early boot stage where dynamic mapping hasn't
been set up.

> CC: Shawn Guo <shawnguo@kernel.org>
> CC: Sascha Hauer <kernel@pengutronix.de>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> ---
>  arch/arm/mach-imx/common.h     |  5 -----
>  arch/arm/mach-imx/mach-imx6q.c |  8 +-------
>  arch/arm/mach-imx/platsmp.c    | 32 +++++---------------------------
>  arch/arm/mach-imx/pm-imx6.c    |  3 ++-
>  4 files changed, 8 insertions(+), 40 deletions(-)

I tested it and saw that the booting of imx6q is broken like below.

[    0.000000] Booting Linux on physical CPU 0x0
[    0.000000] Linux version 4.9.0-rc5-00002-g3e5aac418b91 (r65073 at dragon) (gcc version 4.9.3 20150413 (prerelease) (Linaro GCC 4.9-2015.04-1) ) #5 SMP Mon Nov 14 22:17:36 CST 2016
[    0.000000] CPU: ARMv7 Processor [412fc09a] revision 10 (ARMv7), cr=10c5387d
[    0.000000] CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing instruction cache
[    0.000000] OF: fdt:Machine model: Freescale i.MX6 Quad SABRE Smart Device Board
[    0.000000] earlycon: ec_imx21 at MMIO 0x02020000 (options '')
[    0.000000] bootconsole [ec_imx21] enabled
[    0.000000] cma: Reserved 16 MiB at 0x4f000000
[    0.000000] Memory policy: Data cache writealloc
[    0.000000] Unable to handle kernel NULL pointer dereference at virtual address 00000004
[    0.000000] pgd = c0004000
[    0.000000] [00000004] *pgd=00000000
[    0.000000] Internal error: Oops: 5 [#1] SMP ARM
[    0.000000] Modules linked in:
[    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 4.9.0-rc5-00002-g3e5aac418b91 #5
[    0.000000] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
[    0.000000] task: c0e086c0 task.stack: c0e00000
[    0.000000] PC is at scu_get_core_count+0xc/0x1c
[    0.000000] LR is at imx_smp_init_cpus+0x20/0x4c
[    0.000000] pc : [<c0d05438>]    lr : [<c0d0df2c>]    psr: 000000d3
[    0.000000] sp : c0e01f10  ip : c0e01f20  fp : c0e01f1c
[    0.000000] r10: c0bed850  r9 : c0e051c0  r8 : c0e75140
[    0.000000] r7 : c164c5f0  r6 : c0e09f08  r5 : c0d5d6fc  r4 : c0e08340
[    0.000000] r3 : c0e755f8  r2 : 00000000  r1 : c0120d3c  r0 : 00000000
[    0.000000] Flags: nzcv  IRQs off  FIQs off  Mode SVC_32  ISA ARM  Segment none
[    0.000000] Control: 10c5387d  Table: 1000404a  DAC: 00000051
[    0.000000] Process swapper (pid: 0, stack limit = 0xc0e00210)
[    0.000000] Stack: (0xc0e01f10 to 0xc0e02000)
[    0.000000] 1f00:                                     c0e01f34 c0e01f20 c0d0df2c c0d05438
[    0.000000] 1f20: c0e08340 c0d5d6fc c0e01f44 c0e01f38 c0d052b8 c0d0df18 c0e01fac c0e01f48
[    0.000000] 1f40: c0d0457c c0d052a4 ffffffff 10c5387d c0e050c0 1000406a 4fffffff efffeec0
[    0.000000] 1f60: c0e01f84 c0e01f70 c017b614 c017af04 c0bebc44 c0e01fa4 c0e01f9c c0e01f88
[    0.000000] 1f80: c01cf6dc c0e75294 c0e050d8 c0d5fa44 c0e050c0 1000406a 412fc09a 00000000
[    0.000000] 1fa0: c0e01ff4 c0e01fb0 c0d009b0 c0d03d24 00000000 00000000 00000000 00000000
[    0.000000] 1fc0: 00000000 c0d5fa48 00000000 c0e75294 c0e050d8 c0d5fa44 c0e0a070 1000406a
[    0.000000] 1fe0: 412fc09a 00000000 00000000 c0e01ff8 1000807c c0d0096c 00000000 00000000
[    0.000000] Backtrace: 
[    0.000000] [<c0d0542c>] (scu_get_core_count) from [<c0d0df2c>] (imx_smp_init_cpus+0x20/0x4c)
[    0.000000] [<c0d0df0c>] (imx_smp_init_cpus) from [<c0d052b8>] (smp_init_cpus+0x20/0x28)
[    0.000000]  r5:c0d5d6fc[    0.000000]  r4:c0e08340
[    0.000000] 
[    0.000000] [<c0d05298>] (smp_init_cpus) from [<c0d0457c>] (setup_arch+0x864/0xc50)
[    0.000000] [<c0d03d18>] (setup_arch) from [<c0d009b0>] (start_kernel+0x50/0x398)
[    0.000000]  r10:00000000[    0.000000]  r9:412fc09a
 r8:1000406a[    0.000000]  r7:c0e050c0
 r6:c0d5fa44[    0.000000]  r5:c0e050d8
[    0.000000]  r4:c0e75294[    0.000000] 
[    0.000000] [<c0d00960>] (start_kernel) from [<1000807c>] (0x1000807c)
[    0.000000]  r10:00000000[    0.000000]  r9:412fc09a
 r8:1000406a[    0.000000]  r7:c0e0a070
 r6:c0d5fa44[    0.000000]  r5:c0e050d8
[    0.000000]  r4:c0e75294[    0.000000] 
[    0.000000] Code: c0e75338 e1a0c00d e92dd800 e24cb004 (e5900004) 
[    0.000000] ---[ end trace 0000000000000000 ]---
[    0.000000] Kernel panic - not syncing: Attempted to kill the idle task!
[    0.000000] ---[ end Kernel panic - not syncing: Attempted to kill the idle task!

^ permalink raw reply

* [PATCH 04/16] ARM: realview: use generic API for enabling SCU
From: Arnd Bergmann @ 2016-11-14 14:28 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <95b10b29-2d51-9e96-c628-232f081e09f4@samsung.com>

On Monday, November 14, 2016 5:36:43 PM CET pankaj.dubey wrote:
> On Monday 14 November 2016 05:26 PM, Arnd Bergmann wrote:
> > On Monday, November 14, 2016 10:31:59 AM CET Pankaj Dubey wrote:
> > 
> > The only difference here seems to be that realview also needs to handle
> > "arm,arm11mp-scu". Why not move that into the generic implementation?
> > 
> 
> Do you mean "arm,arm11mp-scu" this is generic SCU specific to ARM?

Yes.

> If it's generic then it can be moved on the same line I moved a5-scu and
> a9-scu.
> 
> I left it here in same file, as I understood it might be related with
> very specific to realview platform.

The reason why only realview has it is that this is currently the
only platform we support that uses SMP on an ARM11MPcore.

cns3xxx in is also SMP hardware, but we only support the first core.

	ARnd

^ permalink raw reply

* [PATCH 2/4] fpga mgr: Expose FPGA capabilities to userland via sysfs
From: atull @ 2016-11-14 14:33 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161107001326.7395-3-moritz.fischer@ettus.com>

On Mon, 7 Nov 2016, Moritz Fischer wrote:

Hi Moritz,

One nit below.  Otherwise,

    Acked-by: Alan Tull <atull@opensource.altera.com>

Alan


> Expose FPGA capabilities to userland via sysfs.
> 
> Add Documentation for currently supported capabilities
> that get exported via sysfs.
> 
> Signed-off-by: Moritz Fischer <moritz.fischer@ettus.com>
> Cc: Alan Tull <atull@opensource.altera.com>
> Cc: Michal Simek <michal.simek@xilinx.com>
> Cc: S?ren Brinkmann <soren.brinkmann@xilinx.com>
> Cc: linux-kernel at vger.kernel.org
> Cc: linux-arm-kernel at lists.infradead.org
> ---
>  Documentation/ABI/testing/sysfs-class-fpga-manager | 16 ++++++++++++++++
>  drivers/fpga/fpga-mgr.c                            | 20 ++++++++++++++++++++
>  include/linux/fpga/fpga-mgr.h                      |  2 ++
>  3 files changed, 38 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-class-fpga-manager b/Documentation/ABI/testing/sysfs-class-fpga-manager
> index 23056c5..d9aee21 100644
> --- a/Documentation/ABI/testing/sysfs-class-fpga-manager
> +++ b/Documentation/ABI/testing/sysfs-class-fpga-manager
> @@ -35,3 +35,19 @@ Description:	Read fpga manager state as a string.
>  		* write complete	= Doing post programming steps
>  		* write complete error	= Error while doing post programming
>  		* operating		= FPGA is programmed and operating
> +
> +What: 		/sys/class/fpga_manager/fpga/capabilities
> +Date:		November 2016
> +KernelVersion:	4.9
> +Contact:	Moritz Fischer <moritz.fischer@ettus.com>
> +Description:	Read fpga manager capabilities as a string.
> +		The intent is to provide userspace with information on what
> +		operations the particular instance can execute.
> +
> +		Each line expresses a capability that is available on the
> +		particular instance of an fpga manager.
> +		Supported so far:
> +
> +		* Full reconfiguration
> +		* Partial reconfiguration
> +		* Decrypt bitstream on the fly

Decrypt gets added in a later patch.

> diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
> index ed57c17..98230b7 100644
> --- a/drivers/fpga/fpga-mgr.c
> +++ b/drivers/fpga/fpga-mgr.c
> @@ -167,6 +167,11 @@ static const char * const state_str[] = {
>  	[FPGA_MGR_STATE_OPERATING] =		"operating",
>  };
>  
> +static const char * const cap_str[] = {
> +	[FPGA_MGR_CAP_FULL_RECONF] = "Full reconfiguration",
> +	[FPGA_MGR_CAP_PARTIAL_RECONF] = "Partial reconfiguration",
> +};
> +
>  static ssize_t name_show(struct device *dev,
>  			 struct device_attribute *attr, char *buf)
>  {
> @@ -183,10 +188,25 @@ static ssize_t state_show(struct device *dev,
>  	return sprintf(buf, "%s\n", state_str[mgr->state]);
>  }
>  
> +static ssize_t capabilities_show(struct device *dev,
> +				 struct device_attribute *attr, char *buf)
> +{
> +	struct fpga_manager *mgr = to_fpga_manager(dev);
> +	char *start = buf;
> +	enum fpga_mgr_capability cap;
> +
> +	for_each_fpga_mgr_cap_mask(cap, mgr->caps)
> +		buf += sprintf(buf, "%s\n", cap_str[cap]);
> +
> +	return buf - start;
> +}
> +
> +static DEVICE_ATTR_RO(capabilities);
>  static DEVICE_ATTR_RO(name);
>  static DEVICE_ATTR_RO(state);
>  
>  static struct attribute *fpga_mgr_attrs[] = {
> +	&dev_attr_capabilities.attr,
>  	&dev_attr_name.attr,
>  	&dev_attr_state.attr,
>  	NULL,
> diff --git a/include/linux/fpga/fpga-mgr.h b/include/linux/fpga/fpga-mgr.h
> index e73429c..9bb96a5 100644
> --- a/include/linux/fpga/fpga-mgr.h
> +++ b/include/linux/fpga/fpga-mgr.h
> @@ -108,6 +108,8 @@ static inline void __fpga_mgr_cap_set(enum fpga_mgr_capability cap,
>  	set_bit(cap, mask->bits);
>  }
>  
> +#define for_each_fpga_mgr_cap_mask(cap, mask) \
> +	for_each_set_bit(cap, mask.bits, FPGA_MGR_CAP_END)
>  
>  /**
>   * struct fpga_manager_ops - ops for low level fpga manager drivers
> -- 
> 2.10.0
> 
> 

^ permalink raw reply

* [PATCH 01/16] ARM: scu: Provide support for parsing SCU device node to enable SCU
From: Arnd Bergmann @ 2016-11-14 14:37 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161114135018.GL1041@n2100.armlinux.org.uk>

On Monday, November 14, 2016 1:50:18 PM CET Russell King - ARM Linux wrote:
> On Mon, Nov 14, 2016 at 01:03:09PM +0100, Arnd Bergmann wrote:
> > On Monday, November 14, 2016 2:10:16 PM CET pankaj.dubey wrote:
> > > >> +    scu_base = of_iomap(np, 0);
> > > >> +    of_node_put(np);
> > > >> +    if (!scu_base) {
> > > >> +            pr_err("%s failed to map scu_base via DT\n", __func__);
> > > > 
> > > > For non-ca5, non-ca9 based SoCs, we'll see this error msg. We understand
> > > > what does it mean, but it may confuse normal users. In current version,
> > > > berlin doesn't complain like this for non-ca9 SoCs
> > > > 
> > > 
> > > OK, let me see other reviewer's comment on this. Then we will decide if
> > > this error message is required or can be omitted.
> > 
> > We need to look at all callers here, to see if the function ever gets
> > called for a CPU that doesn't have an SCU. I'd say we should warn if
> > we know there is an SCU but we cannot map it, but never warn on
> > any of the CPU cores that don't support an SCU.
> 
> Maybe there should be two helpers:
> 
> of_scu_enable() which _only_ looks up the SCU address in DT and enables
> it if it finds it, otherwise returning failure.
> 
> a9_scu_enable() which tries to use the A9 provided SCU address and
> enables it if it finds it, otherwise returning failure.
> 
> Then callers can decide which of these to call, and what error messages
> to print on their failures.

Splitting the function in two is probably simpler overall, but
we may still have to look at all the callers: Any platform that
currently tries to map it on any CPU and doesn't warn about the
absence of the device node (or about scu_a9_has_base() == false)
should really continue not to warn about that.

If all platforms only call these on SMP machines with an
ARM11MPcore, Cortex-A5 or Cortex-A9, everything should be
fine here, otherwise we can leave the warning in the caller
after checking the return code of the new APIs.

	Arnd

^ permalink raw reply

* [PATCH] staging: vc04_services: Remove explicit NULL pointer
From: Greg Kroah-Hartman @ 2016-11-14 14:42 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1478866605-1561-1-git-send-email-maninder.s2@samsung.com>

On Fri, Nov 11, 2016 at 05:46:45PM +0530, Maninder Singh wrote:
> Replace direct comparisons to NULL i.e.
> 'x == NULL' with '!x'
> 'x != NULL' with 'x'
> 
> Signed-off-by: Maninder Singh <maninder.s2@samsung.com>

A patch that was sent right before yours causes this patch to now have
conflicts.  Can you refresh it against my staging-testing branch and
resend?

thanks,

greg k-h

^ permalink raw reply

* [PATCH] arm64: dts: rockchip: add gmac needed clk for rk3399 pd
From: Heiko Stuebner @ 2016-11-14 14:42 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1479098199-20170-1-git-send-email-wxt@rock-chips.com>

Am Montag, 14. November 2016, 12:36:39 CET schrieb Caesar Wang:
> From: Jeffy Chen <jeffy.chen@rock-chips.com>
> 
> This patch fixes that sometimes hang at start-up time of the system.
> As the below log:
> ...
> [   11.136543] calling  pm_genpd_debug_init+0x0/0x60 @ 1
> [   11.141602] initcall pm_genpd_debug_init+0x0/0x60 returned 0 after 11
> usecs [   11.148558] calling  genpd_poweroff_unused+0x0/0x84 @ 1
> <hang>
> 
> In some cases, the rk3399 should turn off the gmac power domain to save
> power if some boards didn't register the gmac device node for rk3399.
> Then, rk3399 need to make sure the gmac's pclk enabled if we need
> operate the gmac power domain. (Due to the NOC had enabled always)
> 
> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
> Signed-off-by: Caesar Wang <wxt@rock-chips.com>

applied to my dts64 branch


Thanks
Heiko

^ permalink raw reply

* [PATCH resend 0/1] input: touchscreen: silead: Add regulator support
From: Hans de Goede @ 2016-11-14 14:45 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Dmitri,

This one seems to have fallen through the cracks, can you pick it up
please ?

Regards,

Hans

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox