Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [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 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 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 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

* [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 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

* [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 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 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] 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 2/2] ARM: rename is_smp()
From: Russell King - ARM Linux @ 2016-11-14 13:38 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <878tsmchyc.fsf@free-electrons.com>

On Mon, Nov 14, 2016 at 02:24:43PM +0100, Gregory CLEMENT wrote:
> Hi Russell,
> 
> Unless I am wrong I don't see any modification of the code behavior:
> just renaming the function and adding more comments. So it won't affect
> our platform and I am OK with the new name which also match our
> comments.

That's correct - it's an exercise in renaming to a clearer name.

Thanks for the ack.

-- 
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 1/1] ARM: dts: enable GPIO-b for Broadcom NSP
From: Yendapally Reddy Dhananjaya Reddy @ 2016-11-14 13:30 UTC (permalink / raw)
  To: linux-arm-kernel

This enables the GPIO-b support for Broadcom NSP SoC

Signed-off-by: Yendapally Reddy Dhananjaya Reddy <yendapally.reddy@broadcom.com>
---
 arch/arm/boot/dts/bcm-nsp.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/bcm-nsp.dtsi b/arch/arm/boot/dts/bcm-nsp.dtsi
index 7c9e0fa..2f699a0 100644
--- a/arch/arm/boot/dts/bcm-nsp.dtsi
+++ b/arch/arm/boot/dts/bcm-nsp.dtsi
@@ -241,6 +241,16 @@
 			brcm,nand-has-wp;
 		};
 
+		gpiob: gpio at 30000 {
+			compatible = "brcm,iproc-nsp-gpio", "brcm,iproc-gpio";
+			reg = <0x30000 0x50>;
+			#gpio-cells = <2>;
+			gpio-controller;
+			ngpios = <4>;
+			interrupt-controller;
+			interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>;
+		};
+
 		pwm: pwm at 31000 {
 			compatible = "brcm,iproc-pwm";
 			reg = <0x31000 0x28>;
-- 
2.1.0

^ permalink raw reply related

* [PATCH 2/2] ARM: rename is_smp()
From: Gregory CLEMENT @ 2016-11-14 13:24 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <E1c6GpW-0008DO-BD@rmk-PC.armlinux.org.uk>

Hi Russell,
 
 On lun., nov. 14 2016, Russell King <rmk+kernel@armlinux.org.uk> wrote:

> is_smp() is causing some confusion - rename it to indicate that it's a
> property of the CPU that we're running on, which is not the same as the
> system.  Document what and why it is being used at most sites.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
> Some people are reporting that "is_smp()" is broken wrt Cortex-A15 CPUs
> when they integrate a single Cortex-A15 SMP capable CPU into a
> uniprocessor system.  This is most likely because of a misunderstanding
> about what is_smp() is really detected from: it's detected from the CPU
> capabilities, not from the system capabilities.  If the CPU says that it
> is SMP capable (and it's not a broken Cortex-A9 core) we will make use
> of various instructions which appear on SMP cores, and we set is_smp()
> to follow that.  So, is_smp() is more of a CPU capability rather than a
> system capability.
>
> Trying to use it as a system capability will lead to problems.
> Arguably, the use of it in arch_irq_work_has_interrupt() is wrong,
> because we don't know whether the GIC is SMP capable or not, but it's
> currently the best we can do.
>
> I felt the other two sites I left undocumented (which read the MPIDR)
> were rather obvious - a uniprocessor only capable CPU doesn't have a
> MPIDR.
>
> Really, !cpu_smp() is an indication that the CPU is UP-only, not that
> it is _S_MP capable, so even this is lightly misleading.  I suppose
> we could replace it with cpu_up_only() but I think that makes the code
> harder to understand (due to double-negatives appearing in places.)
>
>  arch/arm/include/asm/cputype.h  |  2 +-
>  arch/arm/include/asm/irq_work.h |  2 +-
>  arch/arm/include/asm/smp_plat.h | 11 +++++++----
>  arch/arm/kernel/devtree.c       |  2 +-
>  arch/arm/kernel/module.c        | 10 +++++++++-
>  arch/arm/kernel/setup.c         |  7 ++++---
>  arch/arm/mach-mvebu/coherency.c |  4 ++--
>  arch/arm/mm/mmu.c               | 13 ++++++++++++-
>  8 files changed, 37 insertions(+), 14 deletions(-)

[...]

> diff --git a/arch/arm/mach-mvebu/coherency.c b/arch/arm/mach-mvebu/coherency.c
> index ae2a018b9305..fe4b1e15ebb8 100644
> --- a/arch/arm/mach-mvebu/coherency.c
> +++ b/arch/arm/mach-mvebu/coherency.c
> @@ -216,7 +216,7 @@ static int coherency_type(void)
>  	 *
>  	 * Note that this means that on Armada 370, there is currently
>  	 * no way to use hardware I/O coherency, because even when
> -	 * CONFIG_SMP is enabled, is_smp() returns false due to the
> +	 * CONFIG_SMP is enabled, cpu_smp() returns false due to the
>  	 * Armada 370 being a single-core processor. To lift this
>  	 * limitation, we would have to find a way to make the cache
>  	 * policy set to write-allocate (on all Armada SoCs), and to
> @@ -226,7 +226,7 @@ static int coherency_type(void)
>  	 * where we don't know yet on which SoC we are running.
>  
>  	 */
> -	if (!is_smp())
> +	if (!cpu_smp())
>  		return COHERENCY_FABRIC_TYPE_NONE;
>  
>  	np = of_find_matching_node_and_match(NULL, of_coherency_table, &match);

Unless I am wrong I don't see any modification of the code behavior:
just renaming the function and adding more comments. So it won't affect
our platform and I am OK with the new name which also match our
comments.

So, for this chunk:

Acked-by: Gregory CLEMENT <gregory.clement@free-electrons.com>

Thanks,

Gregory


> diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
> index 4001dd15818d..d3dc758d5391 100644
> --- a/arch/arm/mm/mmu.c
> +++ b/arch/arm/mm/mmu.c
> @@ -449,11 +449,22 @@ static void __init build_mem_type_table(void)
>  		ecc_mask = 0;
>  	}
>  
> -	if (is_smp()) {
> +	if (cpu_smp()) {
> +		/*
> +		 * SMP requires a write-allocate cache policy for proper
> +		 * functioning of the coherency protocol.  If the CPU supports
> +		 * MP extensions, assume we are part of a SMP system.
> +		 */
>  		if (cachepolicy != CPOLICY_WRITEALLOC) {
>  			pr_warn("Forcing write-allocate cache policy for SMP\n");
>  			cachepolicy = CPOLICY_WRITEALLOC;
>  		}
> +
> +		/*
> +		 * SMP systems depend on the S bit being shared for coherency
> +		 * between the CPUs.  However, setting this for non-SMP CPUs
> +		 * may result in the mappings being treated as uncached.
> +		 */
>  		if (!(initial_pmd_value & PMD_SECT_S)) {
>  			pr_warn("Forcing shared mappings for SMP\n");
>  			initial_pmd_value |= PMD_SECT_S;
> -- 
> 2.7.4
>

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply

* [PATCH 04/16] ARM: realview: use generic API for enabling SCU
From: Pankaj Dubey @ 2016-11-14 13:19 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <3587281.4kOCcW9Ryd@wuerfel>

Hi Arnd,

On 14 November 2016 at 17:26, Arnd Bergmann <arnd@arndb.de> wrote:
> On Monday, November 14, 2016 10:31:59 AM CET Pankaj Dubey wrote:
>>  static const struct of_device_id realview_scu_match[] = {
>>         { .compatible = "arm,arm11mp-scu", },
>> -       { .compatible = "arm,cortex-a9-scu", },
>> -       { .compatible = "arm,cortex-a5-scu", },
>>         { }
>>  };
>>
>> @@ -41,27 +39,18 @@ static void __init realview_smp_prepare_cpus(unsigned int max_cpus)
>>         struct device_node *np;
>>         void __iomem *scu_base;
>>         struct regmap *map;
>> -       unsigned int ncores;
>>         int i;
>>
>> -       np = of_find_matching_node(NULL, realview_scu_match);
>> -       if (!np) {
>> -               pr_err("PLATSMP: No SCU base address\n");
>> -               return;
>> +       if (of_scu_enable()) {
>> +               np = of_find_matching_node(NULL, realview_scu_match);
>> +               scu_base = of_iomap(np, 0);
>> +               of_node_put(np);
>> +               if (!scu_base) {
>> +                       pr_err("PLATSMP: No SCU remap\n");
>> +                       return;
>> +               }
>> +               scu_enable(scu_base);
>>         }
>>
>
> The only difference here seems to be that realview also needs to handle
> "arm,arm11mp-scu". Why not move that into the generic implementation?
>

Just checked scu binding documentation for "arm,arm11mp-scu" and came
to know its for ARM11 MPCore based SoC's SCU. So as you said, it can
surely be moved to genenric imeplemenation. Will do this change and
resubmit again.

Thanks,
Pankaj Dubey

>         Arnd
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Tegra baseline test results for v4.9-rc5
From: Jon Hunter @ 2016-11-14 13:19 UTC (permalink / raw)
  To: linux-arm-kernel

Here are some basic Tegra test results for Linux v4.9-rc5.
Logs and other details at:

    https://nvtb.github.io//linux/test_v4.9-rc5/20161114020102/


Test summary
------------

Build: zImage:
    Pass: ( 2/ 2): multi_v7_defconfig, tegra_defconfig

Build: Image:
    Pass: ( 1/ 1): defconfig

Boot to userspace: defconfig:
    Pass: ( 4/ 4): qemu-vexpress64, tegra132-norrin,
		   tegra210-p2371-0000, tegra210-smaug

Boot to userspace: multi_v7_defconfig:
    Pass: ( 5/ 5): tegra114-dalmore-a04, tegra124-jetson-tk1,
		   tegra124-nyan-big, tegra20-trimslice, tegra30-beaver

Boot to userspace: tegra_defconfig:
    Pass: ( 5/ 5): tegra114-dalmore-a04, tegra124-jetson-tk1,
		   tegra124-nyan-big, tegra20-trimslice, tegra30-beaver

PM: System suspend: multi_v7_defconfig:
    Pass: ( 5/ 5): tegra114-dalmore-a04, tegra124-jetson-tk1,
		   tegra124-nyan-big, tegra20-trimslice, tegra30-beaver

PM: System suspend: tegra_defconfig:
    Pass: ( 5/ 5): tegra114-dalmore-a04, tegra124-jetson-tk1,
		   tegra124-nyan-big, tegra20-trimslice, tegra30-beaver


vmlinux object size
(delta in bytes from test_v4.9-rc4 (bc33b0ca11e3df467777a4fa7639ba488c9d4911)):
   text     data      bss    total  kernel
  -1032    -2736        0    -3768  defconfig
  +1056       -8        0    +1048  multi_v7_defconfig
   +688    -4104        0    -3416  tegra_defconfig


Boot-time memory difference
(delta in bytes from test_v4.9-rc4 (bc33b0ca11e3df467777a4fa7639ba488c9d4911))
    avail    rsrvd     high    freed                board              kconfig                  dtb
        .        .        .        .      qemu-vexpress64            defconfig           __internal
        .        .        .        . tegra114-dalmore-a04   multi_v7_defconfig     tegra114-dalmore
        .        .        .        . tegra114-dalmore-a04      tegra_defconfig     tegra114-dalmore
        .        .        .        .  tegra124-jetson-tk1   multi_v7_defconfig  tegra124-jetson-tk1
        .        .        .        .  tegra124-jetson-tk1      tegra_defconfig  tegra124-jetson-tk1
        .        .        .        .    tegra124-nyan-big   multi_v7_defconfig    tegra124-nyan-big
        .        .        .        .    tegra124-nyan-big      tegra_defconfig    tegra124-nyan-big
        .        .        .        .      tegra132-norrin            defconfig      tegra132-norrin
        .        .        .        .    tegra20-trimslice   multi_v7_defconfig    tegra20-trimslice
        .        .        .        .    tegra20-trimslice      tegra_defconfig    tegra20-trimslice
        .        .        .        .  tegra210-p2371-0000            defconfig  tegra210-p2371-0000
        .        .        .        .       tegra210-smaug            defconfig       tegra210-smaug
        .        .        .        .       tegra30-beaver   multi_v7_defconfig       tegra30-beaver
        .        .        .        .       tegra30-beaver      tegra_defconfig       tegra30-beaver


--
nvpublic

^ permalink raw reply

* [PATCH v2] input: bma150: Only claim to support the bma180 if the separate iio bma180 driver is not build
From: Hans de Goede @ 2016-11-14 13:11 UTC (permalink / raw)
  To: linux-arm-kernel

commit ef3714fdbc8d ("Input: bma150 - extend chip detection for bma180"),
adds bma180 chip-ids to the input bma150 driver, assuming that they are
100% compatible, but the bma180 is not compatible with the bma150 at all,
it has 14 bits resolution instead of 10, and it has quite different
control registers too.

Treating the bma180 as a bma150 wrt its data registers will just result
in throwing away the lowest 4 bits, which is not too bad. But the ctrl
registers are a different story. Things happen to just work but supporting
that certainly does not make treating the bma180 the same as the bma150
right.

Since some setups depend on the evdev interface the bma150 driver offers
on top of the bma180, we cannot simply remove the bma180 ids.

So this commit only removes the bma180 id when the bma180 iio driver,
which does treat the bma180 properly, is enabled.

Cc: Dr. H. Nikolaus Schaller <hns@goldelico.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Use IS_ENABLED(CONFIG_BMA180) instead of #ifdef CONFIG_BMA180
---
 drivers/input/misc/bma150.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/input/misc/bma150.c b/drivers/input/misc/bma150.c
index b0d4453..2124390 100644
--- a/drivers/input/misc/bma150.c
+++ b/drivers/input/misc/bma150.c
@@ -538,8 +538,13 @@ static int bma150_probe(struct i2c_client *client,
 		return -EIO;
 	}
 
+	/*
+	 * Note if the IIO CONFIG_BMA180 driver is enabled we want to fail
+	 * the probe for the bma180 as the iio driver is preferred.
+	 */
 	chip_id = i2c_smbus_read_byte_data(client, BMA150_CHIP_ID_REG);
-	if (chip_id != BMA150_CHIP_ID && chip_id != BMA180_CHIP_ID) {
+	if (chip_id != BMA150_CHIP_ID &&
+	    (IS_ENABLED(CONFIG_BMA180) || chip_id != BMA180_CHIP_ID)) {
 		dev_err(&client->dev, "BMA150 chip id error: %d\n", chip_id);
 		return -EINVAL;
 	}
@@ -643,7 +648,9 @@ static UNIVERSAL_DEV_PM_OPS(bma150_pm, bma150_suspend, bma150_resume, NULL);
 
 static const struct i2c_device_id bma150_id[] = {
 	{ "bma150", 0 },
+#if !IS_ENABLED(CONFIG_BMA180)
 	{ "bma180", 0 },
+#endif
 	{ "smb380", 0 },
 	{ "bma023", 0 },
 	{ }
-- 
2.9.3

^ permalink raw reply related

* [linux-sunxi] Re: [PATCH 3/3] ARM: dts: sunxi: add support for Orange Pi Zero board
From: Maxime Ripard @ 2016-11-14 13:10 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <cbbf8ae1-ff29-2a83-265b-a9ff5bacf97b@redhat.com>

On Mon, Nov 14, 2016 at 11:00:32AM +0100, Hans de Goede wrote:
> > > > +&usbphy {
> > > > +     /* USB VBUS is always on */
> > > 
> > > You can put the always on regulators (I'm guessing reg_vcc5v0 ?) here.
> > 
> > AFAIK the regulator properties are optional the the USB PHY.
> > So we probably don't need to add it. Hans (CC-ed) could explain
> > his original intent?
> 
> I've made the regulators optional exactly for boards like these,
> where there is no regulator. Likely the Vbus is simply wired
> directly to the 5V DC-in jack. So IMHO adding something like
> the fixed reg_vcc5v0 a supply here just makes the dt
> harder to read.

It also makes the regulator tree more complete and accurate because
you'd list all the devices that are needing those regulators. That
would also make it easier to deal with in the future.

But fair enough.
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161114/4cfb2c48/attachment.sig>

^ permalink raw reply

* [PATCH RFC] ARM: dts: add support for Turris Omnia
From: Andrew Lunn @ 2016-11-14 13:10 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1479126185.15557.5@smtp.gmail.com>

> Actually SFP is connected to SGMII interface of eth1, which is
> routed through SERDES 5.

You say eth1 here. Yet lower down you say got eth0 and eth1 are
connected to the switch?

> We have our proprietary support hacked onto mvneta driver for
> disconnecting PHY on the fly. It is a bit nasty, so I suggest to
> ignore SFP in this DTS altogether and let's wait till "phylink based
> SFP module support" or something alike hits upstream, so we can base
> the SFP support on solid code;

It would be great if you could work on getting the phylink patches
into mainline. It is something i have wanted to do for a long time,
but it is too low down on my priority list to get to. The code is high
quality, so i don't think there will be too many issues. It probably
just needs splitting up into smaller batches, submitting, and working
on any comments.

> Actually eth0 and eth1 (both are RGMII) are connected to the 88E6176
> switch. The problem is that from what I have read so far the switch
> can not operate in DSA mode with two CPU ports.

Again, this is something i wanted to do, and i did have a prototype at
one point. But again, not enough time. If you have resources to work
on this, i can find my code, explain my ideas, and let you complete
it.

	Andrew

^ permalink raw reply

* [PATCH] spi: atmel: Fix scheduling while atomic
From: Alexandre Belloni @ 2016-11-14 13:08 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1478792090-6376-1-git-send-email-ben.whitten@gmail.com>

Hi,

On 10/11/2016 at 15:34:50 +0000, ben.whitten at gmail.com wrote :
> From: Ben Whitten <ben.whitten@lairdtech.com>
> 
> A call to clk_get_rate appears to be called in the context of an interrupt,
> cache the bus clock for the frequency calculations in transmission.
> 
> This fixes a 'BUG: scheduling while atomic' and
> 'WARNING: CPU: 0 PID: 777 at kernel/sched/core.c:2960 atmel_spi_unlock'
> 
> Signed-off-by: Ben Whitten <ben.whitten@lairdtech.com>
> Signed-off-by: Steve deRosier <steve.derosier@lairdtech.com>

The spi subsystem maintainer is not copied so this fix will not be
applied.

Else, it seems ok to me.

> ---
>  drivers/spi/spi-atmel.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
> index 8feac59..c281d1a 100644
> --- a/drivers/spi/spi-atmel.c
> +++ b/drivers/spi/spi-atmel.c
> @@ -295,6 +295,7 @@ struct atmel_spi {
>  	int			irq;
>  	struct clk		*clk;
>  	struct platform_device	*pdev;
> +	unsigned long		spi_clk;
>  
>  	struct spi_transfer	*current_transfer;
>  	int			current_remaining_bytes;
> @@ -864,7 +865,7 @@ static int atmel_spi_set_xfer_speed(struct atmel_spi *as,
>  	unsigned long		bus_hz;
>  
>  	/* v1 chips start out at half the peripheral bus speed. */
> -	bus_hz = clk_get_rate(as->clk);
> +	bus_hz = as->spi_clk;
>  	if (!atmel_spi_is_v2(as))
>  		bus_hz /= 2;
>  
> @@ -1606,6 +1607,9 @@ static int atmel_spi_probe(struct platform_device *pdev)
>  	ret = clk_prepare_enable(clk);
>  	if (ret)
>  		goto out_free_irq;
> +
> +	as->spi_clk = clk_get_rate(clk);
> +
>  	spi_writel(as, CR, SPI_BIT(SWRST));
>  	spi_writel(as, CR, SPI_BIT(SWRST)); /* AT91SAM9263 Rev B workaround */
>  	if (as->caps.has_wdrbt) {
> -- 
> 2.7.4
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply

* [PATCH] input: bma150: Only claim to support the bma180 if the separate iio bma180 driver is not build
From: Hans de Goede @ 2016-11-14 13:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161114053523.GA21471@dtor-ws>

Hi,

On 14-11-16 06:35, Dmitry Torokhov wrote:
> Hi Hans,
>
> On Sun, Nov 13, 2016 at 07:34:07PM +0100, Hans de Goede wrote:
>> commit ef3714fdbc8d ("Input: bma150 - extend chip detection for bma180"),
>> adds bma180 chip-ids to the input bma150 driver, assuming that they are
>> 100% compatible, but the bma180 is not compatible with the bma150 at all,
>> it has 14 bits resolution instead of 10, and it has quite different
>> control registers too.
>>
>> Treating the bma180 as a bma150 wrt its data registers will just result
>> in throwing away the lowest 4 bits, which is not too bad. But the ctrl
>> registers are a different story. Things happen to just work but supporting
>> that certainly does not make treating the bma180 the same as the bma150
>> right.
>>
>> Since some setups depend on the evdev interface the bma150 driver offers
>> on top of the bma180, we cannot simply remove the bma180 ids.
>>
>> So this commit only removes the bma180 id when the bma180 iio driver,
>> which does treat the bma180 properly, is enabled.
>>
>> Cc: Dr. H. Nikolaus Schaller <hns@goldelico.com>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>  drivers/input/misc/bma150.c | 8 +++++++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/input/misc/bma150.c b/drivers/input/misc/bma150.c
>> index b0d4453..9fa1c9a 100644
>> --- a/drivers/input/misc/bma150.c
>> +++ b/drivers/input/misc/bma150.c
>> @@ -539,7 +539,11 @@ static int bma150_probe(struct i2c_client *client,
>>  	}
>>
>>  	chip_id = i2c_smbus_read_byte_data(client, BMA150_CHIP_ID_REG);
>> -	if (chip_id != BMA150_CHIP_ID && chip_id != BMA180_CHIP_ID) {
>> +	if (chip_id != BMA150_CHIP_ID
>> +#ifndef CONFIG_BMA180
>> +	    && chip_id != BMA180_CHIP_ID
>> +#endif
>
> Does not this break if bma180 is compiled as module? I'd rather we did
>
> 	if (chip_id != BMA150_CHIP_ID &&
>             (IS_ENABLED(CONFIG_BMA180) || chip_id != BMA180_CHIP_ID)) {
> 		...

Yes using IS_ENABLED() is a good idea, both for readability and for
the building as module reason. I'll send a v2.

Regards,

Hans


>
>
>> +	    ) {
>>  		dev_err(&client->dev, "BMA150 chip id error: %d\n", chip_id);
>>  		return -EINVAL;
>>  	}
>> @@ -643,7 +647,9 @@ static UNIVERSAL_DEV_PM_OPS(bma150_pm, bma150_suspend, bma150_resume, NULL);
>>
>>  static const struct i2c_device_id bma150_id[] = {
>>  	{ "bma150", 0 },
>> +#ifndef CONFIG_BMA180
> #if !IS_ENABLED(CONFIG_BMA180)
>
>>  	{ "bma180", 0 },
>> +#endif
>>  	{ "smb380", 0 },
>>  	{ "bma023", 0 },
>>  	{ }
>> --
>> 2.9.3
>>
>
> Thanks.
>

^ permalink raw reply

* [PATCH] ARM: ftrace: fix syscall name matching
From: Rabin Vincent @ 2016-11-14 13:03 UTC (permalink / raw)
  To: linux-arm-kernel

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;
+
+	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

^ permalink raw reply related

* [GIT PULL] DaVinci additional driver support for v4.10
From: Sekhar Nori @ 2016-11-14 12:58 UTC (permalink / raw)
  To: linux-arm-kernel

The following changes since commit 1001354ca34179f3db924eb66672442a173147dc:

  Linux 4.9-rc1 (2016-10-15 12:17:50 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/nsekhar/linux-davinci.git tags/davinci-for-v4.10/drivers

for you to fetch changes up to 8e7223fc8626db7c302136747bb68213100d290c:

  bus: davinci: add support for da8xx bus master priority control (2016-11-14 17:20:29 +0530)

----------------------------------------------------------------
This pull request adds two new drivers for better
support for LCD found on DaVinci DA8xx devices.

They allow configuration of memory interface and
bus priorities on the SoC to allow sufficient
bandwidth for the LCD and prevent underruns.

The DT bindings have been reviewed by Rob and
patches have been reviewed by Kevin.

----------------------------------------------------------------
Bartosz Golaszewski (2):
      memory: davinci: add support for da8xx DDR2/mDDR controller
      bus: davinci: add support for da8xx bus master priority control

 .../devicetree/bindings/bus/ti,da850-mstpri.txt    |  20 ++
 .../memory-controllers/ti-da8xx-ddrctl.txt         |  20 ++
 drivers/bus/Kconfig                                |   9 +
 drivers/bus/Makefile                               |   2 +
 drivers/bus/da8xx-mstpri.c                         | 269 +++++++++++++++++++++
 drivers/memory/Kconfig                             |   8 +
 drivers/memory/Makefile                            |   1 +
 drivers/memory/da8xx-ddrctl.c                      | 175 ++++++++++++++
 8 files changed, 504 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/bus/ti,da850-mstpri.txt
 create mode 100644 Documentation/devicetree/bindings/memory-controllers/ti-da8xx-ddrctl.txt
 create mode 100644 drivers/bus/da8xx-mstpri.c
 create mode 100644 drivers/memory/da8xx-ddrctl.c

^ permalink raw reply

* [PATCH 2/2] ARM: rename is_smp()
From: Russell King @ 2016-11-14 12:57 UTC (permalink / raw)
  To: linux-arm-kernel

is_smp() is causing some confusion - rename it to indicate that it's a
property of the CPU that we're running on, which is not the same as the
system.  Document what and why it is being used at most sites.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
Some people are reporting that "is_smp()" is broken wrt Cortex-A15 CPUs
when they integrate a single Cortex-A15 SMP capable CPU into a
uniprocessor system.  This is most likely because of a misunderstanding
about what is_smp() is really detected from: it's detected from the CPU
capabilities, not from the system capabilities.  If the CPU says that it
is SMP capable (and it's not a broken Cortex-A9 core) we will make use
of various instructions which appear on SMP cores, and we set is_smp()
to follow that.  So, is_smp() is more of a CPU capability rather than a
system capability.

Trying to use it as a system capability will lead to problems.
Arguably, the use of it in arch_irq_work_has_interrupt() is wrong,
because we don't know whether the GIC is SMP capable or not, but it's
currently the best we can do.

I felt the other two sites I left undocumented (which read the MPIDR)
were rather obvious - a uniprocessor only capable CPU doesn't have a
MPIDR.

Really, !cpu_smp() is an indication that the CPU is UP-only, not that
it is _S_MP capable, so even this is lightly misleading.  I suppose
we could replace it with cpu_up_only() but I think that makes the code
harder to understand (due to double-negatives appearing in places.)

 arch/arm/include/asm/cputype.h  |  2 +-
 arch/arm/include/asm/irq_work.h |  2 +-
 arch/arm/include/asm/smp_plat.h | 11 +++++++----
 arch/arm/kernel/devtree.c       |  2 +-
 arch/arm/kernel/module.c        | 10 +++++++++-
 arch/arm/kernel/setup.c         |  7 ++++---
 arch/arm/mach-mvebu/coherency.c |  4 ++--
 arch/arm/mm/mmu.c               | 13 ++++++++++++-
 8 files changed, 37 insertions(+), 14 deletions(-)

diff --git a/arch/arm/include/asm/cputype.h b/arch/arm/include/asm/cputype.h
index 522b5feb4eaa..8c82e6b4961d 100644
--- a/arch/arm/include/asm/cputype.h
+++ b/arch/arm/include/asm/cputype.h
@@ -109,7 +109,7 @@ extern unsigned int processor_id;
 
 /*
  * The memory clobber prevents gcc 4.5 from reordering the mrc before
- * any is_smp() tests, which can cause undefined instruction aborts on
+ * any cpu_smp() tests, which can cause undefined instruction aborts on
  * ARM1136 r0 due to the missing extended CP15 registers.
  */
 #define read_cpuid_ext(ext_reg)						\
diff --git a/arch/arm/include/asm/irq_work.h b/arch/arm/include/asm/irq_work.h
index 712d03e5973a..2dc8d7995b48 100644
--- a/arch/arm/include/asm/irq_work.h
+++ b/arch/arm/include/asm/irq_work.h
@@ -5,7 +5,7 @@
 
 static inline bool arch_irq_work_has_interrupt(void)
 {
-	return is_smp();
+	return cpu_smp();
 }
 
 #endif /* _ASM_ARM_IRQ_WORK_H */
diff --git a/arch/arm/include/asm/smp_plat.h b/arch/arm/include/asm/smp_plat.h
index 43f246b73ce7..bdba301d01e4 100644
--- a/arch/arm/include/asm/smp_plat.h
+++ b/arch/arm/include/asm/smp_plat.h
@@ -12,9 +12,10 @@
 #include <asm/cputype.h>
 
 /*
- * Return true if we are running on a SMP platform
+ * Return true if we are running on a CPU which supports SMP, and the
+ * kernel supports SMP.
  */
-static inline bool is_smp(void)
+static inline bool cpu_smp(void)
 {
 #ifndef CONFIG_SMP
 	return false;
@@ -43,7 +44,8 @@ static inline unsigned int smp_cpuid_part(int cpu)
 #else
 static inline int tlb_ops_need_broadcast(void)
 {
-	if (!is_smp())
+	/* Non-SMP CPUs don't need to check for broadcast */
+	if (!cpu_smp())
 		return 0;
 
 	return ((read_cpuid_ext(CPUID_EXT_MMFR3) >> 12) & 0xf) < 2;
@@ -55,7 +57,8 @@ static inline int tlb_ops_need_broadcast(void)
 #else
 static inline int cache_ops_need_broadcast(void)
 {
-	if (!is_smp())
+	/* Non-SMP CPUs don't need to check for broadcast */
+	if (!cpu_smp())
 		return 0;
 
 	return ((read_cpuid_ext(CPUID_EXT_MMFR3) >> 12) & 0xf) < 1;
diff --git a/arch/arm/kernel/devtree.c b/arch/arm/kernel/devtree.c
index f676febbb270..19a9653df6d2 100644
--- a/arch/arm/kernel/devtree.c
+++ b/arch/arm/kernel/devtree.c
@@ -78,7 +78,7 @@ void __init arm_dt_init_cpu_maps(void)
 	struct device_node *cpu, *cpus;
 	int found_method = 0;
 	u32 i, j, cpuidx = 1;
-	u32 mpidr = is_smp() ? read_cpuid_mpidr() & MPIDR_HWID_BITMASK : 0;
+	u32 mpidr = cpu_smp() ? read_cpuid_mpidr() & MPIDR_HWID_BITMASK : 0;
 
 	u32 tmp_map[NR_CPUS] = { [0 ... NR_CPUS-1] = MPIDR_INVALID };
 	bool bootcpu_valid = false;
diff --git a/arch/arm/kernel/module.c b/arch/arm/kernel/module.c
index 4f14b5ce6535..0b49aa426180 100644
--- a/arch/arm/kernel/module.c
+++ b/arch/arm/kernel/module.c
@@ -374,12 +374,20 @@ int module_finalize(const Elf32_Ehdr *hdr, const Elf_Shdr *sechdrs,
 		fixup_pv_table((void *)s->sh_addr, s->sh_size);
 #endif
 	s = find_mod_section(hdr, sechdrs, ".alt.smp.init");
-	if (s && !is_smp())
+	if (s && !cpu_smp()) {
+		/*
+		 * Modules running on non-SMP capable CPUs must not use SMP
+		 * instructions, as they may cause undefined instruction
+		 * exceptions.  This means we have to fix up the SMP
+		 * alternatives on SMP-on-UP modules, and are unable to load
+		 * modules built for SMP-only configurations.
+		 */
 #ifdef CONFIG_SMP_ON_UP
 		fixup_smp((void *)s->sh_addr, s->sh_size);
 #else
 		return -EINVAL;
 #endif
+	}
 	return 0;
 }
 
diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index 80f45b01fbaa..cdf71941c129 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -584,7 +584,7 @@ u32 __cpu_logical_map[NR_CPUS] = { [0 ... NR_CPUS-1] = MPIDR_INVALID };
 void __init smp_setup_processor_id(void)
 {
 	int i;
-	u32 mpidr = is_smp() ? read_cpuid_mpidr() & MPIDR_HWID_BITMASK : 0;
+	u32 mpidr = cpu_smp() ? read_cpuid_mpidr() & MPIDR_HWID_BITMASK : 0;
 	u32 cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
 
 	cpu_logical_map(0) = cpu;
@@ -1112,7 +1112,8 @@ void __init setup_arch(char **cmdline_p)
 	arm_dt_init_cpu_maps();
 	psci_dt_init();
 #ifdef CONFIG_SMP
-	if (is_smp()) {
+	if (cpu_smp()) {
+		/* Ignore SMP on non-SMP capable CPUs */
 		if (!mdesc->smp_init || !mdesc->smp_init()) {
 			if (psci_smp_available())
 				smp_set_ops(&psci_smp_ops);
@@ -1124,7 +1125,7 @@ void __init setup_arch(char **cmdline_p)
 	}
 #endif
 
-	if (!is_smp())
+	if (!cpu_smp())
 		hyp_mode_check();
 
 	reserve_crashkernel();
diff --git a/arch/arm/mach-mvebu/coherency.c b/arch/arm/mach-mvebu/coherency.c
index ae2a018b9305..fe4b1e15ebb8 100644
--- a/arch/arm/mach-mvebu/coherency.c
+++ b/arch/arm/mach-mvebu/coherency.c
@@ -216,7 +216,7 @@ static int coherency_type(void)
 	 *
 	 * Note that this means that on Armada 370, there is currently
 	 * no way to use hardware I/O coherency, because even when
-	 * CONFIG_SMP is enabled, is_smp() returns false due to the
+	 * CONFIG_SMP is enabled, cpu_smp() returns false due to the
 	 * Armada 370 being a single-core processor. To lift this
 	 * limitation, we would have to find a way to make the cache
 	 * policy set to write-allocate (on all Armada SoCs), and to
@@ -226,7 +226,7 @@ static int coherency_type(void)
 	 * where we don't know yet on which SoC we are running.
 
 	 */
-	if (!is_smp())
+	if (!cpu_smp())
 		return COHERENCY_FABRIC_TYPE_NONE;
 
 	np = of_find_matching_node_and_match(NULL, of_coherency_table, &match);
diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index 4001dd15818d..d3dc758d5391 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -449,11 +449,22 @@ static void __init build_mem_type_table(void)
 		ecc_mask = 0;
 	}
 
-	if (is_smp()) {
+	if (cpu_smp()) {
+		/*
+		 * SMP requires a write-allocate cache policy for proper
+		 * functioning of the coherency protocol.  If the CPU supports
+		 * MP extensions, assume we are part of a SMP system.
+		 */
 		if (cachepolicy != CPOLICY_WRITEALLOC) {
 			pr_warn("Forcing write-allocate cache policy for SMP\n");
 			cachepolicy = CPOLICY_WRITEALLOC;
 		}
+
+		/*
+		 * SMP systems depend on the S bit being shared for coherency
+		 * between the CPUs.  However, setting this for non-SMP CPUs
+		 * may result in the mappings being treated as uncached.
+		 */
 		if (!(initial_pmd_value & PMD_SECT_S)) {
 			pr_warn("Forcing shared mappings for SMP\n");
 			initial_pmd_value |= PMD_SECT_S;
-- 
2.7.4

^ permalink raw reply related

* [PATCH 1/2] ARM: always store CPU ID early into percpu data
From: Russell King @ 2016-11-14 12:57 UTC (permalink / raw)
  To: linux-arm-kernel

Always store the CPU ID into the percpu data so that it is available
early on.  This allows us to eliminate various is_smp() tests that
select between reading the hardware CPU ID directly and reading from
the percpu data.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 arch/arm/include/asm/smp_plat.h | 5 +----
 arch/arm/kernel/setup.c         | 5 ++++-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arm/include/asm/smp_plat.h b/arch/arm/include/asm/smp_plat.h
index f9080717fc88..43f246b73ce7 100644
--- a/arch/arm/include/asm/smp_plat.h
+++ b/arch/arm/include/asm/smp_plat.h
@@ -34,10 +34,7 @@ static inline bool is_smp(void)
  */
 static inline unsigned int smp_cpuid_part(int cpu)
 {
-	struct cpuinfo_arm *cpu_info = &per_cpu(cpu_data, cpu);
-
-	return is_smp() ? cpu_info->cpuid & ARM_CPU_PART_MASK :
-			  read_cpuid_part();
+	return per_cpu(cpu_data, cpu).cpuid;
 }
 
 /* all SMP configurations have the extended CPUID registers */
diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index 34e3f3c45634..80f45b01fbaa 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -724,6 +724,9 @@ static void __init setup_processor(void)
 
 	cacheid_init();
 	cpu_init();
+
+	/* Always setup the boot CPU ID in the per-cpu data */
+	per_cpu(cpu_data, smp_processor_id()).cpuid = read_cpuid_id();
 }
 
 void __init dump_machine_table(void)
@@ -1217,7 +1220,7 @@ static int c_show(struct seq_file *m, void *v)
 		 * "processor".  Give glibc what it expects.
 		 */
 		seq_printf(m, "processor\t: %d\n", i);
-		cpuid = is_smp() ? per_cpu(cpu_data, i).cpuid : read_cpuid_id();
+		cpuid = per_cpu(cpu_data, i).cpuid;
 		seq_printf(m, "model name\t: %s rev %d (%s)\n",
 			   cpu_name, cpuid & 15, elf_platform);
 
-- 
2.7.4

^ permalink raw reply related

* PM regression with LED changes in next-20161109
From: Hans de Goede @ 2016-11-14 12:51 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <127cdd42-6fd8-c671-60b7-3826b351577f@samsung.com>

Hi,

On 14-11-16 10:12, Jacek Anaszewski wrote:
> Hi,
>
> On 11/13/2016 02:52 PM, Hans de Goede wrote:
>> Hi,
>>
>> On 13-11-16 12:44, Jacek Anaszewski wrote:
>>> Hi,
>>>
>>> On 11/12/2016 10:14 PM, Hans de Goede wrote:
>>
>> <snip>
>>
>>>>>> So I would like to propose creating a new read-write
>>>>>> user_brightness file.
>>>>>>
>>>>>> The write behavior would be 100% identical to the brightness
>>>>>> file (in code terms it will call the same store function).
>>>>>>
>>>>>> The the read behavior otoh will be different: it will shows
>>>>>> the last brightness as set by the user, this would show the
>>>>>> read behavior we really want of brightness: show the real
>>>>>> brightness when not blinking / triggers are active and show
>>>>>> the brightness used when on when blinking / triggers are active.
>>>>>>
>>>>>> We could then add poll support on this new user_brightness
>>>>>> file, thus avoiding the problem with the extra cpu-load on
>>>>>> notifications on blinking / triggers.
>>>>>
>>>>> I agree that user_brightness allows to solve the issues you raised
>>>>> about inconsistent write and read brightness' semantics
>>>>> (which is not that painful IMHO).
>>>>>
>>>>> Reporting non-user brightness changes on user_brightness file
>>>>> doesn't sound reasonable though.
>>>>
>>>> The changes I'm interested in are user brightness changes they
>>>> are just not done through sysfs, but through a hardwired hotkey,
>>>> they are however very much done by the user.
>>>
>>> Ah, so this file name would be misleading especially taking into account
>>> the context in which "user" is used in kernel, which predominantly
>>> means "userspace", e.g. copy_to_user(), copy_from_user().
>>>
>>>>> Also, how would we read the
>>>>> brightness set by the firmware? We'd have to read brightness
>>>>> file, so still two files would have to be opened which is
>>>>> a second drawback of this approach.
>>>>
>>>> No, look carefully at the definition of the read behavior
>>>> I plan to put in the ABI doc:
>>>
>>> OK, "user" was what confused me. So in this case changes made
>>> by the firmware even if in a result of user activity
>>> (pressing hardware key) obviously cannot be treated similarly
>>> to the changes made from the userspace context.
>>
>> In the end both result on the brightness of the device
>> changing, so any userspace process interested in monitoring
>> the brightness will want to know about both type of changes.
>>
>>> Unless you're able to give references to the kernel code which
>>> contradict my judgement.
>>
>> AFAIK the audio code will signal volume changes done by
>> hardwired buttons the same way as audio changes done
>> by userspace calling into the kernel. This also makes
>> sense because in the end, what is interesting for a
>> mixer app, is that the volume changed, and what the
>> new volume is.
>
> OK, so it is indeed similar to your LED use case. Nonetheless
> in case of LED controllers it is also possible that hardware
> adjusts LED brightness in case of low battery voltage.
>
> If a device is able e.g. to generate an interrupt to notify this
> kind of event, then we would like also to be able to notify the client
> about that. It wouldn't be user generated brightness change though.
>
>>>> "Reading this file will return the actual led brightness
>>>> when not blinking and no triggers are active; reading this
>>>> file will return the brightness used when the led is on
>>>> when blinking or triggers are active."
>>>
>>> This is unnecessarily entangled. Blinking means timer trigger
>>> is active.
>>
>> Ok.
>>
>>>> So for e.g. the backlit keyboard case reading this single
>>>> file will return the actual brightness of the backlight,
>>>> since this does not involve blinking or triggers.
>>>>
>>>> Basically the idea is that the user_brightness file
>>>> will have the semantics which IMHO the brightness file
>>>> itself should have had from the beginning, but which
>>>> we can't change now due to ABI reasons.
>>>
>>> And in fact introducing user_brightness file would indeed
>>> fix that shortcoming. However without providing notifications
>>> of hw brightness changes on it.
>>
>> See above, I believe such a file should report any
>> changes in brightness, except those caused by triggers,
>> so it would report hw brightness changes.
>>
>> Anyways if you're not interested in fixing the
>> shortcomings of the current read behavior on the
>> brightness file (I'm fine with that, I can live
>> with the shortcomings) I suggest that we simply go
>> with v2 of my poll() patch.
>
> v2 entails power consumption related issues.
>
> Generally I think that we could add the file you proposed,
> however it would be good to devise a name which will cover
> also the cases when brightness is changed by firmware without
> user interaction.
>
>>>>> Having no difference in this area between the two approaches
>>>>> I'm still in favour of the read-only file for notifying
>>>>> brightness changes procured by hardware.
>>>>
>>>> That brings back the needing 2 fds problem; and does
>>>> not solve userspace not being able to reliably read
>>>> the led on brightness when blinking or using triggers.
>>>>
>>>> And this also has the issue that one is doing poll() on
>>>> one fd to detect changes on another fd,
>>>
>>> It is not necessarily true. We can treat the polling on
>>> hw_brightness_change file as a means to detect brightness
>>> changes procured by hardware and we can read that brightness
>>> by executing read on this same fd. It could return -ENODATA
>>> if no such an event has occurred so far.
>>
>> That would still require 2 fds as userspace also wants to
>> be able to set the keyboard backlight, but allowing read()
>> on the hw_brightness_change file at least fixes the weirdness
>> where userspace gets woken from poll() without being able to
>> read. So if you insist on going the hw_brightness_change file
>> route, then I can live with that (and upower will simply
>> need to open 2 fds, that is doable).
>>
>> But, BUT, I would greatly prefer to just go for v4 of my
>> patch, which fixes the only real problem we've seen with
>> my patch as original merged without adding a new, somewhat
>> convoluted sysfs attribute.
>
> Hmm, v4 still calls led_notify_brightness_change(led_cdev)
> from both __led_set_brightness() and __led_set_brightness_blocking().

Ugh, I see I accidentally send a v4 twice, instead of
calling the version which dropped those called v5 as
I should have, sorry.

The v4 which I would like to see merged, the one with
those calls dropped, is here:

https://patchwork.kernel.org/patch/9423093/

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