Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 04/15] dt-bindings: display: sunxi-drm: Add TCON TOP description
From: Maxime Ripard @ 2018-05-21  8:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180519183127.2718-5-jernej.skrabec@siol.net>

Hi,

On Sat, May 19, 2018 at 08:31:16PM +0200, Jernej Skrabec wrote:
> TCON TOP main purpose is to configure whole display pipeline. It
> determines relationships between mixers and TCONs, selects source TCON
> for HDMI, muxes LCD and TV encoder GPIO output,

I'm not sure you mean GPIO here, but rather pin?

> selects TV encoder clock source and contains additional TV TCON and
> DSI gates.
> 
> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> ---
>  .../bindings/display/sunxi/sun4i-drm.txt      | 20 +++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
> index 3346c1e2a7a0..a099957ab62a 100644
> --- a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
> +++ b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
> @@ -187,6 +187,26 @@ And on the A23, A31, A31s and A33, you need one more clock line:
>     - 'lvds-alt': An alternative clock source, separate from the TCON channel 0
>                   clock, that can be used to drive the LVDS clock
>  
> +TCON TOP
> +--------
> +
> +TCON TOPs main purpose is to configure whole display pipeline. It determines
> +relationships between mixers and TCONs, selects source TCON for HDMI, muxes
> +LCD and TV encoder GPIO output, selects TV encoder clock source and contains
> +additional TV TCON and DSI gates.
> +
> +Required properties:
> +  - compatible: value must be one of:
> +    * allwinner,sun8i-r40-tcon-top
> +  - reg: base address and size of the memory-mapped region.
> +  - clocks: phandle to the clocks feeding the TCON TOP
> +    * bus: TCON TOP interface clock
> +  - clock-names: clock name mentioned above
> +  - resets: phandle to the reset line driving the DRC
> +    * rst: TCON TOP reset line
> +  - reset-names: reset name mentioned above
> +  - #clock-cells : must contain 1
> +

I guess you should better describe the OF-graph endpoints, and the
clocks output. Just using the binding additions here doesn't allow to
get a clear idea of how the DT should look like.

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180521/0904bec1/attachment.sig>

^ permalink raw reply

* [PATCH 12/15] drm/sun4i: Add support for second clock parent to DW HDMI PHY clk driver
From: kbuild test robot @ 2018-05-21  7:47 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180519183127.2718-13-jernej.skrabec@siol.net>

Hi Jernej,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on drm/drm-next]
[also build test WARNING on v4.17-rc6 next-20180517]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Jernej-Skrabec/Add-support-for-R40-HDMI-pipeline/20180521-131839
base:   git://people.freedesktop.org/~airlied/linux.git drm-next
config: arm64-allmodconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm64 

All warnings (new ones prefixed by >>):

   In file included from drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h:12:0,
                    from drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c:9:
   drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c: In function 'sun8i_hdmi_phy_config_h3':
>> drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c:191:7: warning: large integer implicitly truncated to unsigned type [-Woverflow]
          ~SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_MSK,
          ^
   include/linux/regmap.h:76:36: note: in definition of macro 'regmap_update_bits'
     regmap_update_bits_base(map, reg, mask, val, NULL, false, false)
                                       ^~~~

vim +191 drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c

     8	
   > 9	#include "sun8i_dw_hdmi.h"
    10	
    11	/*
    12	 * Address can be actually any value. Here is set to same value as
    13	 * it is set in BSP driver.
    14	 */
    15	#define I2C_ADDR	0x69
    16	
    17	static int sun8i_hdmi_phy_config_a83t(struct dw_hdmi *hdmi,
    18					      struct sun8i_hdmi_phy *phy,
    19					      unsigned int clk_rate)
    20	{
    21		regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_REXT_CTRL_REG,
    22				   SUN8I_HDMI_PHY_REXT_CTRL_REXT_EN,
    23				   SUN8I_HDMI_PHY_REXT_CTRL_REXT_EN);
    24	
    25		/* power down */
    26		dw_hdmi_phy_gen2_txpwron(hdmi, 0);
    27		dw_hdmi_phy_gen2_pddq(hdmi, 1);
    28	
    29		dw_hdmi_phy_reset(hdmi);
    30	
    31		dw_hdmi_phy_gen2_pddq(hdmi, 0);
    32	
    33		dw_hdmi_phy_i2c_set_addr(hdmi, I2C_ADDR);
    34	
    35		/*
    36		 * Values are taken from BSP HDMI driver. Although AW didn't
    37		 * release any documentation, explanation of this values can
    38		 * be found in i.MX 6Dual/6Quad Reference Manual.
    39		 */
    40		if (clk_rate <= 27000000) {
    41			dw_hdmi_phy_i2c_write(hdmi, 0x01e0, 0x06);
    42			dw_hdmi_phy_i2c_write(hdmi, 0x0000, 0x15);
    43			dw_hdmi_phy_i2c_write(hdmi, 0x08da, 0x10);
    44			dw_hdmi_phy_i2c_write(hdmi, 0x0007, 0x19);
    45			dw_hdmi_phy_i2c_write(hdmi, 0x0318, 0x0e);
    46			dw_hdmi_phy_i2c_write(hdmi, 0x8009, 0x09);
    47		} else if (clk_rate <= 74250000) {
    48			dw_hdmi_phy_i2c_write(hdmi, 0x0540, 0x06);
    49			dw_hdmi_phy_i2c_write(hdmi, 0x0005, 0x15);
    50			dw_hdmi_phy_i2c_write(hdmi, 0x0000, 0x10);
    51			dw_hdmi_phy_i2c_write(hdmi, 0x0007, 0x19);
    52			dw_hdmi_phy_i2c_write(hdmi, 0x02b5, 0x0e);
    53			dw_hdmi_phy_i2c_write(hdmi, 0x8009, 0x09);
    54		} else if (clk_rate <= 148500000) {
    55			dw_hdmi_phy_i2c_write(hdmi, 0x04a0, 0x06);
    56			dw_hdmi_phy_i2c_write(hdmi, 0x000a, 0x15);
    57			dw_hdmi_phy_i2c_write(hdmi, 0x0000, 0x10);
    58			dw_hdmi_phy_i2c_write(hdmi, 0x0002, 0x19);
    59			dw_hdmi_phy_i2c_write(hdmi, 0x0021, 0x0e);
    60			dw_hdmi_phy_i2c_write(hdmi, 0x8029, 0x09);
    61		} else {
    62			dw_hdmi_phy_i2c_write(hdmi, 0x0000, 0x06);
    63			dw_hdmi_phy_i2c_write(hdmi, 0x000f, 0x15);
    64			dw_hdmi_phy_i2c_write(hdmi, 0x0000, 0x10);
    65			dw_hdmi_phy_i2c_write(hdmi, 0x0002, 0x19);
    66			dw_hdmi_phy_i2c_write(hdmi, 0x0000, 0x0e);
    67			dw_hdmi_phy_i2c_write(hdmi, 0x802b, 0x09);
    68		}
    69	
    70		dw_hdmi_phy_i2c_write(hdmi, 0x0000, 0x1e);
    71		dw_hdmi_phy_i2c_write(hdmi, 0x0000, 0x13);
    72		dw_hdmi_phy_i2c_write(hdmi, 0x0000, 0x17);
    73	
    74		dw_hdmi_phy_gen2_txpwron(hdmi, 1);
    75	
    76		return 0;
    77	}
    78	
    79	static int sun8i_hdmi_phy_config_h3(struct dw_hdmi *hdmi,
    80					    struct sun8i_hdmi_phy *phy,
    81					    unsigned int clk_rate)
    82	{
    83		u32 pll_cfg1_init;
    84		u32 pll_cfg2_init;
    85		u32 ana_cfg1_end;
    86		u32 ana_cfg2_init;
    87		u32 ana_cfg3_init;
    88		u32 b_offset = 0;
    89		u32 val;
    90	
    91		/* bandwidth / frequency independent settings */
    92	
    93		pll_cfg1_init = SUN8I_HDMI_PHY_PLL_CFG1_LDO2_EN |
    94				SUN8I_HDMI_PHY_PLL_CFG1_LDO1_EN |
    95				SUN8I_HDMI_PHY_PLL_CFG1_LDO_VSET(7) |
    96				SUN8I_HDMI_PHY_PLL_CFG1_UNKNOWN(1) |
    97				SUN8I_HDMI_PHY_PLL_CFG1_PLLDBEN |
    98				SUN8I_HDMI_PHY_PLL_CFG1_CS |
    99				SUN8I_HDMI_PHY_PLL_CFG1_CP_S(2) |
   100				SUN8I_HDMI_PHY_PLL_CFG1_CNT_INT(63) |
   101				SUN8I_HDMI_PHY_PLL_CFG1_BWS;
   102	
   103		pll_cfg2_init = SUN8I_HDMI_PHY_PLL_CFG2_SV_H |
   104				SUN8I_HDMI_PHY_PLL_CFG2_VCOGAIN_EN |
   105				SUN8I_HDMI_PHY_PLL_CFG2_SDIV2;
   106	
   107		ana_cfg1_end = SUN8I_HDMI_PHY_ANA_CFG1_REG_SVBH(1) |
   108			       SUN8I_HDMI_PHY_ANA_CFG1_AMP_OPT |
   109			       SUN8I_HDMI_PHY_ANA_CFG1_EMP_OPT |
   110			       SUN8I_HDMI_PHY_ANA_CFG1_AMPCK_OPT |
   111			       SUN8I_HDMI_PHY_ANA_CFG1_EMPCK_OPT |
   112			       SUN8I_HDMI_PHY_ANA_CFG1_ENRCAL |
   113			       SUN8I_HDMI_PHY_ANA_CFG1_ENCALOG |
   114			       SUN8I_HDMI_PHY_ANA_CFG1_REG_SCKTMDS |
   115			       SUN8I_HDMI_PHY_ANA_CFG1_TMDSCLK_EN |
   116			       SUN8I_HDMI_PHY_ANA_CFG1_TXEN_MASK |
   117			       SUN8I_HDMI_PHY_ANA_CFG1_TXEN_ALL |
   118			       SUN8I_HDMI_PHY_ANA_CFG1_BIASEN_TMDSCLK |
   119			       SUN8I_HDMI_PHY_ANA_CFG1_BIASEN_TMDS2 |
   120			       SUN8I_HDMI_PHY_ANA_CFG1_BIASEN_TMDS1 |
   121			       SUN8I_HDMI_PHY_ANA_CFG1_BIASEN_TMDS0 |
   122			       SUN8I_HDMI_PHY_ANA_CFG1_ENP2S_TMDS2 |
   123			       SUN8I_HDMI_PHY_ANA_CFG1_ENP2S_TMDS1 |
   124			       SUN8I_HDMI_PHY_ANA_CFG1_ENP2S_TMDS0 |
   125			       SUN8I_HDMI_PHY_ANA_CFG1_CKEN |
   126			       SUN8I_HDMI_PHY_ANA_CFG1_LDOEN |
   127			       SUN8I_HDMI_PHY_ANA_CFG1_ENVBS |
   128			       SUN8I_HDMI_PHY_ANA_CFG1_ENBI;
   129	
   130		ana_cfg2_init = SUN8I_HDMI_PHY_ANA_CFG2_M_EN |
   131				SUN8I_HDMI_PHY_ANA_CFG2_REG_DENCK |
   132				SUN8I_HDMI_PHY_ANA_CFG2_REG_DEN |
   133				SUN8I_HDMI_PHY_ANA_CFG2_REG_CKSS(1) |
   134				SUN8I_HDMI_PHY_ANA_CFG2_REG_CSMPS(1);
   135	
   136		ana_cfg3_init = SUN8I_HDMI_PHY_ANA_CFG3_REG_WIRE(0x3e0) |
   137				SUN8I_HDMI_PHY_ANA_CFG3_SDAEN |
   138				SUN8I_HDMI_PHY_ANA_CFG3_SCLEN;
   139	
   140		/* bandwidth / frequency dependent settings */
   141		if (clk_rate <= 27000000) {
   142			pll_cfg1_init |= SUN8I_HDMI_PHY_PLL_CFG1_HV_IS_33 |
   143					 SUN8I_HDMI_PHY_PLL_CFG1_CNT_INT(32);
   144			pll_cfg2_init |= SUN8I_HDMI_PHY_PLL_CFG2_VCO_S(4) |
   145					 SUN8I_HDMI_PHY_PLL_CFG2_S(4);
   146			ana_cfg1_end |= SUN8I_HDMI_PHY_ANA_CFG1_REG_CALSW;
   147			ana_cfg2_init |= SUN8I_HDMI_PHY_ANA_CFG2_REG_SLV(4) |
   148					 SUN8I_HDMI_PHY_ANA_CFG2_REG_RESDI(phy->rcal);
   149			ana_cfg3_init |= SUN8I_HDMI_PHY_ANA_CFG3_REG_AMPCK(3) |
   150					 SUN8I_HDMI_PHY_ANA_CFG3_REG_AMP(5);
   151		} else if (clk_rate <= 74250000) {
   152			pll_cfg1_init |= SUN8I_HDMI_PHY_PLL_CFG1_HV_IS_33 |
   153					 SUN8I_HDMI_PHY_PLL_CFG1_CNT_INT(32);
   154			pll_cfg2_init |= SUN8I_HDMI_PHY_PLL_CFG2_VCO_S(4) |
   155					 SUN8I_HDMI_PHY_PLL_CFG2_S(5);
   156			ana_cfg1_end |= SUN8I_HDMI_PHY_ANA_CFG1_REG_CALSW;
   157			ana_cfg2_init |= SUN8I_HDMI_PHY_ANA_CFG2_REG_SLV(4) |
   158					 SUN8I_HDMI_PHY_ANA_CFG2_REG_RESDI(phy->rcal);
   159			ana_cfg3_init |= SUN8I_HDMI_PHY_ANA_CFG3_REG_AMPCK(5) |
   160					 SUN8I_HDMI_PHY_ANA_CFG3_REG_AMP(7);
   161		} else if (clk_rate <= 148500000) {
   162			pll_cfg1_init |= SUN8I_HDMI_PHY_PLL_CFG1_HV_IS_33 |
   163					 SUN8I_HDMI_PHY_PLL_CFG1_CNT_INT(32);
   164			pll_cfg2_init |= SUN8I_HDMI_PHY_PLL_CFG2_VCO_S(4) |
   165					 SUN8I_HDMI_PHY_PLL_CFG2_S(6);
   166			ana_cfg2_init |= SUN8I_HDMI_PHY_ANA_CFG2_REG_BIGSWCK |
   167					 SUN8I_HDMI_PHY_ANA_CFG2_REG_BIGSW |
   168					 SUN8I_HDMI_PHY_ANA_CFG2_REG_SLV(2);
   169			ana_cfg3_init |= SUN8I_HDMI_PHY_ANA_CFG3_REG_AMPCK(7) |
   170					 SUN8I_HDMI_PHY_ANA_CFG3_REG_AMP(9);
   171		} else {
   172			b_offset = 2;
   173			pll_cfg1_init |= SUN8I_HDMI_PHY_PLL_CFG1_CNT_INT(63);
   174			pll_cfg2_init |= SUN8I_HDMI_PHY_PLL_CFG2_VCO_S(6) |
   175					 SUN8I_HDMI_PHY_PLL_CFG2_S(7);
   176			ana_cfg2_init |= SUN8I_HDMI_PHY_ANA_CFG2_REG_BIGSWCK |
   177					 SUN8I_HDMI_PHY_ANA_CFG2_REG_BIGSW |
   178					 SUN8I_HDMI_PHY_ANA_CFG2_REG_SLV(4);
   179			ana_cfg3_init |= SUN8I_HDMI_PHY_ANA_CFG3_REG_AMPCK(9) |
   180					 SUN8I_HDMI_PHY_ANA_CFG3_REG_AMP(13);
   181		}
   182	
   183		regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_ANA_CFG1_REG,
   184				   SUN8I_HDMI_PHY_ANA_CFG1_TXEN_MASK, 0);
   185	
   186		/*
   187		 * NOTE: We have to be careful not to overwrite PHY parent
   188		 * clock selection bit and clock divider.
   189		 */
   190		regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_PLL_CFG1_REG,
 > 191				   ~SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_MSK,
   192				   pll_cfg1_init);
   193		regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_PLL_CFG2_REG,
   194				   (u32)~SUN8I_HDMI_PHY_PLL_CFG2_PREDIV_MSK,
   195				   pll_cfg2_init);
   196		usleep_range(10000, 15000);
   197		regmap_write(phy->regs, SUN8I_HDMI_PHY_PLL_CFG3_REG,
   198			     SUN8I_HDMI_PHY_PLL_CFG3_SOUT_DIV2);
   199		regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_PLL_CFG1_REG,
   200				   SUN8I_HDMI_PHY_PLL_CFG1_PLLEN,
   201				   SUN8I_HDMI_PHY_PLL_CFG1_PLLEN);
   202		msleep(100);
   203	
   204		/* get B value */
   205		regmap_read(phy->regs, SUN8I_HDMI_PHY_ANA_STS_REG, &val);
   206		val = (val & SUN8I_HDMI_PHY_ANA_STS_B_OUT_MSK) >>
   207			SUN8I_HDMI_PHY_ANA_STS_B_OUT_SHIFT;
   208		val = min(val + b_offset, (u32)0x3f);
   209	
   210		regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_PLL_CFG1_REG,
   211				   SUN8I_HDMI_PHY_PLL_CFG1_REG_OD1 |
   212				   SUN8I_HDMI_PHY_PLL_CFG1_REG_OD,
   213				   SUN8I_HDMI_PHY_PLL_CFG1_REG_OD1 |
   214				   SUN8I_HDMI_PHY_PLL_CFG1_REG_OD);
   215		regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_PLL_CFG1_REG,
   216				   SUN8I_HDMI_PHY_PLL_CFG1_B_IN_MSK,
   217				   val << SUN8I_HDMI_PHY_PLL_CFG1_B_IN_SHIFT);
   218		msleep(100);
   219		regmap_write(phy->regs, SUN8I_HDMI_PHY_ANA_CFG1_REG, ana_cfg1_end);
   220		regmap_write(phy->regs, SUN8I_HDMI_PHY_ANA_CFG2_REG, ana_cfg2_init);
   221		regmap_write(phy->regs, SUN8I_HDMI_PHY_ANA_CFG3_REG, ana_cfg3_init);
   222	
   223		return 0;
   224	}
   225	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/gzip
Size: 59031 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180521/e25d95d9/attachment-0001.gz>

^ permalink raw reply

* [PATCH v1 4/5] ARM: tegra: Don't apply CPU erratas in insecure mode
From: Dmitry Osipenko @ 2018-05-21  7:36 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180520101542.12206-5-digetx@gmail.com>

On 20.05.2018 13:15, Dmitry Osipenko wrote:
> CPU isn't allowed to touch secure registers while running under secure
> monitor. Hence skip applying CPU erratas in the reset handler if Trusted
> Foundations firmware presents.
> 
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  arch/arm/mach-tegra/reset-handler.S | 27 +++++++++++++++++++--------
>  arch/arm/mach-tegra/reset.c         |  3 +++
>  arch/arm/mach-tegra/reset.h         |  4 +++-
>  3 files changed, 25 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm/mach-tegra/reset-handler.S b/arch/arm/mach-tegra/reset-handler.S
> index 805f306fa6f7..d84c74a95806 100644
> --- a/arch/arm/mach-tegra/reset-handler.S
> +++ b/arch/arm/mach-tegra/reset-handler.S
> @@ -121,6 +121,12 @@ ENTRY(__tegra_cpu_reset_handler)
>  	cpsid	aif, 0x13			@ SVC mode, interrupts disabled
>  
>  	tegra_get_soc_id TEGRA_APB_MISC_BASE, r6
> +
> +	adr	r12, __tegra_cpu_reset_handler_data
> +	ldr	r0, [r12, #RESET_DATA(TF_PRESENT)]
> +	cmp	r0, #0
> +	bne	after_errata
> +
>  #ifdef CONFIG_ARCH_TEGRA_2x_SOC
>  t20_check:
>  	cmp	r6, #TEGRA20
> @@ -155,7 +161,6 @@ after_errata:
>  	and	r10, r10, #0x3			@ R10 = CPU number
>  	mov	r11, #1
>  	mov	r11, r11, lsl r10  		@ R11 = CPU mask
> -	adr	r12, __tegra_cpu_reset_handler_data
>  
>  #ifdef CONFIG_SMP
>  	/* Does the OS know about this CPU? */
> @@ -169,10 +174,9 @@ after_errata:
>  	cmp	r6, #TEGRA20
>  	bne	1f
>  	/* If not CPU0, don't let CPU0 reset CPU1 now that CPU1 is coming up. */
> -	mov32	r5, TEGRA_IRAM_BASE + TEGRA_IRAM_RESET_HANDLER_OFFSET
>  	mov	r0, #CPU_NOT_RESETTABLE
>  	cmp	r10, #0
> -	strneb	r0, [r5, #__tegra20_cpu1_resettable_status_offset]
> +	strneb	r0, [r12, #RESET_DATA(RESETTABLE_STATUS)]
>  1:
>  #endif
>  
> @@ -278,13 +282,20 @@ ENDPROC(__tegra_cpu_reset_handler)
>  	.type	__tegra_cpu_reset_handler_data, %object
>  	.globl	__tegra_cpu_reset_handler_data
>  __tegra_cpu_reset_handler_data:
> -	.rept	TEGRA_RESET_DATA_SIZE
> -	.long	0
> -	.endr
> +	.long	0	/* TEGRA_RESET_MASK_PRESENT */
> +	.long	0	/* TEGRA_RESET_MASK_LP1 */
> +	.long	0	/* TEGRA_RESET_MASK_LP2 */
> +	.long	0	/* TEGRA_RESET_STARTUP_SECONDARY */
> +	.long	0	/* TEGRA_RESET_STARTUP_LP2 */
> +	.long	0	/* TEGRA_RESET_STARTUP_LP1 */
> +
>  	.globl	__tegra20_cpu1_resettable_status_offset
>  	.equ	__tegra20_cpu1_resettable_status_offset, \
>  					. - __tegra_cpu_reset_handler_start
> -	.byte	0
> -	.align L1_CACHE_SHIFT
> +	.long	0	/* TEGRA_RESET_RESETTABLE_STATUS */
>  
> +	.globl	__tegra_tf_present
> +	.equ	__tegra_tf_present, . - __tegra_cpu_reset_handler_start

I've noticed that __tegra_tf_present shouldn't belong to this patch, I've missed
to remove it while was rebasing.

Also, it occurred to me that it will be much better to remove the whole array
__tegra_cpu_reset_handler_data definition in the asm and get back to the
original ".rept   TEGRA_RESET_DATA_SIZE" instead. That will make this part of
code much nicer, I'll change that in v2.

Russell / Thierry, please give you acks-reviews where appropriate and let me
know if I should change anything else in v2, thanks.

> +	.long	0	/* TEGRA_RESET_TF_PRESENT */
> +	.align L1_CACHE_SHIFT
>  ENTRY(__tegra_cpu_reset_handler_end)
> diff --git a/arch/arm/mach-tegra/reset.c b/arch/arm/mach-tegra/reset.c
> index dc558892753c..b02ae7699842 100644
> --- a/arch/arm/mach-tegra/reset.c
> +++ b/arch/arm/mach-tegra/reset.c
> @@ -24,6 +24,7 @@
>  #include <asm/cacheflush.h>
>  #include <asm/firmware.h>
>  #include <asm/hardware/cache-l2x0.h>
> +#include <asm/trusted_foundations.h>
>  
>  #include "iomap.h"
>  #include "irammap.h"
> @@ -89,6 +90,8 @@ static void __init tegra_cpu_reset_handler_enable(void)
>  
>  void __init tegra_cpu_reset_handler_init(void)
>  {
> +	__tegra_cpu_reset_handler_data[TEGRA_RESET_TF_PRESENT] =
> +		trusted_foundations_registered();
>  
>  #ifdef CONFIG_SMP
>  	__tegra_cpu_reset_handler_data[TEGRA_RESET_MASK_PRESENT] =
> diff --git a/arch/arm/mach-tegra/reset.h b/arch/arm/mach-tegra/reset.h
> index 9c479c7925b8..0d9ddc022ece 100644
> --- a/arch/arm/mach-tegra/reset.h
> +++ b/arch/arm/mach-tegra/reset.h
> @@ -25,7 +25,9 @@
>  #define TEGRA_RESET_STARTUP_SECONDARY	3
>  #define TEGRA_RESET_STARTUP_LP2		4
>  #define TEGRA_RESET_STARTUP_LP1		5
> -#define TEGRA_RESET_DATA_SIZE		6
> +#define TEGRA_RESET_RESETTABLE_STATUS	6
> +#define TEGRA_RESET_TF_PRESENT		7
> +#define TEGRA_RESET_DATA_SIZE		8
>  
>  #ifndef __ASSEMBLY__
>  
> 

^ permalink raw reply

* [RFC 12/13] ARM: dts: ti: add dra71-evm FIT description file
From: Tero Kristo @ 2018-05-21  6:57 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180417144913.GD5669@atomide.com>

On 17/04/18 17:49, Tony Lindgren wrote:
> * Tero Kristo <t-kristo@ti.com> [180417 09:36]:
>> In typical setup, you can boot a large number of different configs via:
>>
>> bootm 0x82000000#dra71-evm#nand#lcd-auo-g101evn01.0
>>
>> ... assuming the configs were named like that, and assuming they would be
>> compatible with each other. The am57xx-evm example provided is better, as
>> you can chain the different cameras to the available evm configs.
> 
> Why not just do it in the bootloader to put together the dtb?
> 
> Then for external devices, you could just pass info on the
> kernel cmdline with lcd=foo camera=bar if they cannot be
> detected over I2C.

(Added Linux ARM list to CC, this was not part of the original delivery.)

Ok trying to resurrect this thread a bit. Is there any kind of consensus 
how things like this should be handled? Should we add the DT overlay 
files to kernel tree or not?

Should we add any kind of build infra to kernel tree, and at what level 
would this be? Just DT overlay file building support, and drop the FIT 
build support as was proposed in this RFC series or...?

U-boot can obviously parse the base DTB + overlay DTB:s into a single 
DTB, but this is somewhat clumsy approach and is relatively error prone 
to get it right.

Building the FIT image post kernel build would also be possible, but who 
would be doing this, is there any need to get this done in generic 
manner or shall we just add SoC vendor specific tools for this?

-Tero
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

^ permalink raw reply

* [PATCH 5/6] mtd: rawnand: ams-delta: use GPIO lookup table
From: Andy Shevchenko @ 2018-05-21  6:44 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180520192523.32dde632@xps13>

On Sun, May 20, 2018 at 8:25 PM, Miquel Raynal
<miquel.raynal@bootlin.com> wrote:
> On Sun, 20 May 2018 19:17:04 +0300, Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:

> Do we actually have the possibility to rename this gpio? I guess no
> since it would break DT backward compatibility.

No we don't.

> Otherwise it would have
> been more descriptive to call it something like 'gpio-rb'.

"gpio" prefix, actually "gpios" suffix is a mandatory part of the
name. For sake of convenience it's not used in API calls.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* [PATCH]irqchip/irq-gic-v3:Avoid a waste of LPI resource
From: Zhang, Lei @ 2018-05-21  6:12 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <8898674D84E3B24BA3A2D289B872026A69F0E26D@G01JPEXMBKW03>

My patch was based old kernel version, So I wrote a new patch based linux-4.17-rc6.
> -----Original Message-----
> From: linux-arm-kernel
> [mailto:linux-arm-kernel-bounces at lists.infradead.org] On Behalf Of Zhang,
> Lei
> Sent: Friday, May 18, 2018 6:49 PM
> To: 'Marc Zyngier'; linux-arm-kernel at lists.infradead.org
> Subject: RE: [PATCH]irqchip/irq-gic-v3:Avoid a waste of LPI resource
> 
> I rewrote the mechanism of lpis's management by using free list.
> 
> Below is my patch for core ITS driver.
> Would you give me comments?

-------------------------------- 
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 5416f2b..a42df4a 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -1405,82 +1405,122 @@ static int its_irq_set_vcpu_affinity(struct irq_data *d, void *vcpu_info)
        .irq_set_vcpu_affinity  = its_irq_set_vcpu_affinity,
 };

-/*
- * How we allocate LPIs:
- *
- * The GIC has id_bits bits for interrupt identifiers. From there, we
- * must subtract 8192 which are reserved for SGIs/PPIs/SPIs. Then, as
- * we allocate LPIs by chunks of 32, we can shift the whole thing by 5
- * bits to the right.
- *
- * This gives us (((1UL << id_bits) - 8192) >> 5) possible allocations.
- */
-#define IRQS_PER_CHUNK_SHIFT   5
-#define IRQS_PER_CHUNK         (1UL << IRQS_PER_CHUNK_SHIFT)
-#define ITS_MAX_LPI_NRBITS     16 /* 64K LPIs */
+static struct list_head lpi_free_list;
+static struct list_head lpi_alloc_list;
+struct lpi_mng {
+       struct list_head lpi_list;
+       int base;
+       int len;
+};

-static unsigned long *lpi_bitmap;
-static u32 lpi_chunks;
+#define ITS_MAX_LPI_NRBITS     16 /* 64K LPIs */
 static DEFINE_SPINLOCK(lpi_lock);

-static int its_lpi_to_chunk(int lpi)
-{
-       return (lpi - 8192) >> IRQS_PER_CHUNK_SHIFT;
-}
-
-static int its_chunk_to_lpi(int chunk)
-{
-       return (chunk << IRQS_PER_CHUNK_SHIFT) + 8192;
-}

 static int __init its_lpi_init(u32 id_bits)
 {
-       lpi_chunks = its_lpi_to_chunk(1UL << id_bits);
+       u32 nr_irq = 1UL << id_bits;
+       struct lpi_mng *lpi_free_mng = NULL;
+       struct lpi_mng *lpi_new = NULL;
+
+       INIT_LIST_HEAD(&lpi_free_list);
+       INIT_LIST_HEAD(&lpi_alloc_list);

-       lpi_bitmap = kzalloc(BITS_TO_LONGS(lpi_chunks) * sizeof(long),
-                            GFP_KERNEL);
-       if (!lpi_bitmap) {
-               lpi_chunks = 0;
+       lpi_free_mng = kzalloc(sizeof(struct lpi_mng), GFP_KERNEL);
+       if (!lpi_free_mng)
                return -ENOMEM;
-       }

-       pr_info("ITS: Allocated %d chunks for LPIs\n", (int)lpi_chunks);
+       lpi_free_mng->base = 0;
+       lpi_free_mng->len = nr_irq;
+       list_add(&lpi_free_mng->lpi_list, &lpi_free_list);
+
+       do {
+               lpi_free_mng = list_first_entry(&lpi_free_list, struct lpi_mng,
+                       lpi_list);
+               if (lpi_free_mng->len == 8192) {
+                       /*It is not lpi, so we delete */
+                       if (lpi_free_mng->base == 0) {
+                               list_del_init(&lpi_free_mng->lpi_list);
+                               kfree(lpi_free_mng);
+                               continue;
+                       }
+                       if (lpi_free_mng->base == 8192)
+                               goto out;
+               }
+               if (lpi_free_mng->len > 8192) {
+                       lpi_new  = kzalloc(sizeof(struct lpi_mng),
+                                        GFP_ATOMIC);
+                       if (!lpi_new)
+                               return -ENOMEM;
+                       lpi_free_mng->len /= 2;
+                       lpi_new->base = lpi_free_mng->base + lpi_free_mng->len;
+                       lpi_new->len = lpi_free_mng->len;
+                       list_add(&lpi_new->lpi_list, &lpi_free_mng->lpi_list);
+               }
+       } while (1);
+
+out:
+       pr_info("ITS: Allocated %d  LPIs\n", nr_irq - 8192);
        return 0;
 }

+static struct lpi_mng *its_alloc_lpi(int nr_irqs)
+{
+       struct lpi_mng *lpi_alloc_mng = NULL;
+       struct lpi_mng *lpi_split = NULL;
+       struct lpi_mng *lpi_new = NULL;
+       int base;
+
+       base = 0x7fffffff;
+       do {
+               list_for_each_entry(lpi_alloc_mng, &lpi_free_list, lpi_list) {
+                       if (nr_irqs > lpi_alloc_mng->len)
+                               continue;
+                       if (nr_irqs == lpi_alloc_mng->len) {
+                               list_del_init(&lpi_alloc_mng->lpi_list);
+                               list_add(&lpi_alloc_mng->lpi_list,
+                                       &lpi_alloc_list);
+                               return lpi_alloc_mng;
+                       }
+                       if ((nr_irqs < lpi_alloc_mng->len)
+                               && (lpi_alloc_mng->base < base)) {
+                               base = lpi_alloc_mng->base;
+                               lpi_split = lpi_alloc_mng;
+                       }
+               }
+               lpi_new  = kzalloc(sizeof(struct lpi_mng),
+                                GFP_ATOMIC);
+               if (!lpi_new || !lpi_split)
+                       return NULL;
+
+               lpi_split->len /= 2;
+               lpi_new->base = lpi_split->base + lpi_split->len;
+               lpi_new->len = lpi_split->len;
+               list_add(&lpi_new->lpi_list, &lpi_split->lpi_list);
+
+       } while (1);
+}
+
 static unsigned long *its_lpi_alloc_chunks(int nr_irqs, int *base, int *nr_ids)
 {
        unsigned long *bitmap = NULL;
-       int chunk_id;
-       int nr_chunks;
-       int i;
-
-       nr_chunks = DIV_ROUND_UP(nr_irqs, IRQS_PER_CHUNK);
+       struct lpi_mng *lpi_alloc_mng = NULL;

        spin_lock(&lpi_lock);

-       do {
-               chunk_id = bitmap_find_next_zero_area(lpi_bitmap, lpi_chunks,
-                                                     0, nr_chunks, 0);
-               if (chunk_id < lpi_chunks)
-                       break;
-
-               nr_chunks--;
-       } while (nr_chunks > 0);
+       lpi_alloc_mng = its_alloc_lpi(nr_irqs);

-       if (!nr_chunks)
+       if (!lpi_alloc_mng)
                goto out;

-       bitmap = kzalloc(BITS_TO_LONGS(nr_chunks * IRQS_PER_CHUNK) * sizeof (long),
+       bitmap = kzalloc(BITS_TO_LONGS(nr_irqs) * sizeof(long),
                         GFP_ATOMIC);
        if (!bitmap)
                goto out;

-       for (i = 0; i < nr_chunks; i++)
-               set_bit(chunk_id + i, lpi_bitmap);

-       *base = its_chunk_to_lpi(chunk_id);
-       *nr_ids = nr_chunks * IRQS_PER_CHUNK;
+       *base = lpi_alloc_mng->base;
+       *nr_ids = lpi_alloc_mng->len;

 out:
        spin_unlock(&lpi_lock);
@@ -1491,23 +1531,53 @@ static unsigned long *its_lpi_alloc_chunks(int nr_irqs, int *base, int *nr_ids)
        return bitmap;
 }

+static void its_joint_free_list(struct lpi_mng *free, struct lpi_mng *alloc)
+{
+       free->len = free->len * 2;
+       if (free->base > alloc->base)
+               free->base = alloc->base;
+}
+
 static void its_lpi_free_chunks(unsigned long *bitmap, int base, int nr_ids)
 {
-       int lpi;
+       struct lpi_mng *lpi_alloc_mng = NULL;
+       struct lpi_mng *lpi_free_mng = NULL;
+       bool first_half;
+       int pair_base;

        spin_lock(&lpi_lock);

-       for (lpi = base; lpi < (base + nr_ids); lpi += IRQS_PER_CHUNK) {
-               int chunk = its_lpi_to_chunk(lpi);
-
-               BUG_ON(chunk > lpi_chunks);
-               if (test_bit(chunk, lpi_bitmap)) {
-                       clear_bit(chunk, lpi_bitmap);
-               } else {
-                       pr_err("Bad LPI chunk %d\n", chunk);
+       list_for_each_entry(lpi_alloc_mng, &lpi_alloc_list, lpi_list) {
+               if (lpi_alloc_mng->base == base) {
+                       list_del_init(&lpi_alloc_mng->lpi_list);
+                       break;
                }
        }

+       first_half = (lpi_alloc_mng->base % (lpi_alloc_mng->len * 2))
+                        ? false : true;
+       if (first_half)
+               pair_base = lpi_alloc_mng->base + lpi_alloc_mng->len;
+       else
+               pair_base = lpi_alloc_mng->base - lpi_alloc_mng->len;
+
+       // found the other half
+       list_for_each_entry(lpi_free_mng, &lpi_free_list, lpi_list) {
+               if (lpi_free_mng->base == pair_base) {
+                       its_joint_free_list(lpi_free_mng, lpi_alloc_mng);
+                       kfree(lpi_alloc_mng);
+                       goto out;
+               }
+       }
+       // Not found the other half
+       list_for_each_entry(lpi_free_mng, &lpi_free_list, lpi_list) {
+               if (lpi_alloc_mng->base  < lpi_free_mng->base) {
+                       list_add_tail(&lpi_alloc_mng->lpi_list,
+                               &lpi_free_mng->lpi_list);
+                       break;
+               }
+       }
+out:
        spin_unlock(&lpi_lock);

        kfree(bitmap);
@@ -2117,7 +2187,7 @@ static struct its_device *its_create_device(struct its_node *its, u32 dev_id,
         * We allocate at least one chunk worth of LPIs bet device,
         * and thus that many ITEs. The device may require less though.
         */
-       nr_ites = max(IRQS_PER_CHUNK, roundup_pow_of_two(nvecs));
+       nr_ites = max(2UL, roundup_pow_of_two(nvecs));
        sz = nr_ites * its->ite_size;
        sz = max(sz, ITS_ITT_ALIGN) + ITS_ITT_ALIGN - 1;
        itt = kzalloc(sz, GFP_KERNEL);
--------------------------------
Best Regards,
Lei Zhang
--
Lei Zhang  e-mail: zhang.lei at jp.fujitsu.com FUJITSU LIMITED

^ permalink raw reply related

* [PATCH] cpufreq: Add Kryo CPU scaling driver
From: Viresh Kumar @ 2018-05-21  5:04 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1526729701-8589-1-git-send-email-ilialin@codeaurora.org>

More comments after Russell's reply.

On 19-05-18, 14:35, Ilia Lin wrote:
> +static int __init qcom_cpufreq_kryo_driver_init(void)
> +{
> +	struct device *cpu_dev_silver, *cpu_dev_gold;
> +	struct opp_table *opp_silver, *opp_gold;
> +	enum _msm8996_version msm8996_version;
> +	struct nvmem_cell *speedbin_nvmem;
> +	struct platform_device *pdev;
> +	struct device_node *np;
> +	u8 *speedbin;
> +	u32 versions;
> +	size_t len;
> +	int ret;
> +
> +	cpu_dev_silver = get_cpu_device(SILVER_LEAD);
> +	if (IS_ERR_OR_NULL(cpu_dev_silver))

get_cpu_device() returns only NULL on error.

> +		return PTR_ERR(cpu_dev_silver);
> +
> +	cpu_dev_gold = get_cpu_device(SILVER_LEAD);
> +	if (IS_ERR_OR_NULL(cpu_dev_gold))
> +		return PTR_ERR(cpu_dev_gold);
> +
> +	msm8996_version = qcom_cpufreq_kryo_get_msm_id();
> +	if (NUM_OF_MSM8996_VERSIONS == msm8996_version) {
> +		dev_err(cpu_dev_silver, "Not Snapdragon 820/821!");
> +		return -ENODEV;
> +	}
> +
> +	np = dev_pm_opp_of_get_opp_desc_node(cpu_dev_silver);
> +	if (IS_ERR_OR_NULL(np))

same here.

> +		return PTR_ERR(np);
> +
> +	if (!of_device_is_compatible(np, "operating-points-v2-kryo-cpu")) {
> +		ret = -ENOENT;
> +		goto free_np;
> +	}
> +
> +	speedbin_nvmem = of_nvmem_cell_get(np, NULL);
> +	if (IS_ERR(speedbin_nvmem)) {
> +		ret = PTR_ERR(speedbin_nvmem);
> +		dev_err(cpu_dev_silver, "Could not get nvmem cell: %d\n", ret);
> +		goto free_np;
> +	}
> +
> +	speedbin = nvmem_cell_read(speedbin_nvmem, &len);
> +	nvmem_cell_put(speedbin_nvmem);
> +
> +	switch (msm8996_version) {
> +	case MSM8996_V3:
> +		versions = 1 << (unsigned int)(*speedbin);
> +		break;
> +	case MSM8996_SG:
> +		versions = 1 << ((unsigned int)(*speedbin) + 4);
> +		break;
> +	default:
> +		BUG();
> +		break;
> +	}
> +
> +	opp_silver = dev_pm_opp_set_supported_hw(cpu_dev_silver,&versions,1);
> +	if (IS_ERR(opp_silver)) {
> +		dev_err(cpu_dev_silver, "Failed to set supported hardware\n");
> +		ret = PTR_ERR(opp_silver);
> +		goto free_np;
> +	}
> +
> +	opp_gold = dev_pm_opp_set_supported_hw(cpu_dev_gold,&versions,1);
> +	if (IS_ERR(opp_gold)) {
> +		dev_err(cpu_dev_gold, "Failed to set supported hardware\n");
> +		ret = PTR_ERR(opp_gold);
> +		goto free_opp_silver;
> +	}
> +
> +	pdev = platform_device_register_simple("cpufreq-dt", -1, NULL, 0);

and this only returns ERR_PTR() on error.

> +	if (!IS_ERR_OR_NULL(pdev))
> +		return 0;
> +
> +	ret = PTR_ERR(pdev);
> +	dev_err(cpu_dev_silver, "Failed to register platform device\n");
> +	dev_pm_opp_put_supported_hw(opp_gold);
> +
> +free_opp_silver:
> +	dev_pm_opp_put_supported_hw(opp_silver);
> +
> +free_np:
> +	of_node_put(np);
> +
> +	return ret;
> +}
> +late_initcall(qcom_cpufreq_kryo_driver_init);
> +
> +MODULE_DESCRIPTION("Qualcomm Technologies, Inc. Kryo CPUfreq driver");
> +MODULE_LICENSE("GPL v2");
> -- 
> 1.9.1

-- 
viresh

^ permalink raw reply

* [PATCH v8 10/15] cpufreq: Add Kryo CPU scaling driver
From: Viresh Kumar @ 2018-05-21  4:49 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <019201d3ef66$db97aed0$92c70c70$@codeaurora.org>

On 19-05-18, 14:45, ilialin at codeaurora.org wrote:
> Hi Viresh,
> 
> If I send patches in reply, it will produce new patches, instead of answers
> in the thread. Please find below the file dump.

There is one email from you which appears to be just fine and appears
to be in reply to this thread only. Maybe its your email client that
screwed it up for you ? Things look good in mutt.

-- 
viresh

^ permalink raw reply

* [PATCH] clk: aspeed: Add 24MHz fixed clock
From: Joel Stanley @ 2018-05-21  4:33 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1526633822-17138-1-git-send-email-mine260309@gmail.com>

On 18 May 2018 at 18:27, Lei YU <mine260309@gmail.com> wrote:
> Add a 24MHz fixed clock.
> This clock will be used for certain devices, e.g. pwm.
>
> Signed-off-by: Lei YU <mine260309@gmail.com>

Reviewed-by: Joel Stanley <joel@jms.id.au>

Cheers,

Joel

> ---
>  drivers/clk/clk-aspeed.c                 | 9 ++++++++-
>  include/dt-bindings/clock/aspeed-clock.h | 1 +
>  2 files changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clk/clk-aspeed.c b/drivers/clk/clk-aspeed.c
> index 5eb50c3..4664088 100644
> --- a/drivers/clk/clk-aspeed.c
> +++ b/drivers/clk/clk-aspeed.c
> @@ -14,7 +14,7 @@
>
>  #include <dt-bindings/clock/aspeed-clock.h>
>
> -#define ASPEED_NUM_CLKS                35
> +#define ASPEED_NUM_CLKS                36
>
>  #define ASPEED_RESET_CTRL      0x04
>  #define ASPEED_CLK_SELECTION   0x08
> @@ -474,6 +474,13 @@ static int aspeed_clk_probe(struct platform_device *pdev)
>                 return PTR_ERR(hw);
>         aspeed_clk_data->hws[ASPEED_CLK_BCLK] = hw;
>
> +       /* Fixed 24MHz clock */
> +       hw = clk_hw_register_fixed_rate(NULL, "fixed-24m", "clkin",
> +                                       0, 24000000);
> +       if (IS_ERR(hw))
> +               return PTR_ERR(hw);
> +       aspeed_clk_data->hws[ASPEED_CLK_24M] = hw;
> +
>         /*
>          * TODO: There are a number of clocks that not included in this driver
>          * as more information is required:
> diff --git a/include/dt-bindings/clock/aspeed-clock.h b/include/dt-bindings/clock/aspeed-clock.h
> index d3558d8..ff29d8e 100644
> --- a/include/dt-bindings/clock/aspeed-clock.h
> +++ b/include/dt-bindings/clock/aspeed-clock.h
> @@ -38,6 +38,7 @@
>  #define ASPEED_CLK_MAC                 32
>  #define ASPEED_CLK_BCLK                        33
>  #define ASPEED_CLK_MPLL                        34
> +#define ASPEED_CLK_24M                 35
>
>  #define ASPEED_RESET_XDMA              0
>  #define ASPEED_RESET_MCTP              1
> --
> 2.7.4
>

^ permalink raw reply

* [linux-next PATCH 0/4] Enable network driver on K2G ICE and GP EVMs
From: santosh.shilimkar at oracle.com @ 2018-05-21  3:17 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1526066952-5230-1-git-send-email-m-karicheri2@ti.com>

On 5/11/18 12:29 PM, Murali Karicheri wrote:
> Now that NetCP driver patches for K2G SoC is merged to linux-next master
> this series add patches to enable network driver on K2G ICE and GP EVMs.
> 
> Thanks
> 
> Applied the patches on top of latest linux-next master, built kernel and
> booted up on both EVMs. The logs are below
> 
> K2G GP EVM: https://pastebin.ubuntu.com/p/ycZDnZXYPx/
> K2G ICE EVM: https://pastebin.ubuntu.com/p/bdCpzgdrXr/
> 
> Murali Karicheri (4):
>    ARM: dts: k2g: add dt bindings to support network driver
>    ARM: dts: keystone-k2g-evm: Enable netcp network driver
>    ARM: dts: keystone-k2g-ice: Enable netcp network driver
>    ARM: keystone: k2g: enable micrel and dp83867 phys
> 
>   arch/arm/boot/dts/keystone-k2g-evm.dts    |  53 +++++++++++
>   arch/arm/boot/dts/keystone-k2g-ice.dts    |  59 ++++++++++++
>   arch/arm/boot/dts/keystone-k2g-netcp.dtsi | 147 ++++++++++++++++++++++++++++++
>   arch/arm/boot/dts/keystone-k2g.dtsi       |  13 +++
>   arch/arm/configs/keystone_defconfig       |   2 +
>   5 files changed, 274 insertions(+)
>   create mode 100644 arch/arm/boot/dts/keystone-k2g-netcp.dtsi
> 
Looks good. Will add this to the queue for 4.19. Thanks !!

Regards,
Santosh

^ permalink raw reply

* [PATCH v3] KVM: arm/arm64: add WARN_ON if size is not PAGE_SIZE aligned in unmap_stage2_range
From: Jia He @ 2018-05-21  3:05 UTC (permalink / raw)
  To: linux-arm-kernel

There is a panic in armv8a server(QDF2400) under memory pressure tests
(start 20 guests and run memhog in the host).

---------------------------------begin--------------------------------
[35380.800950] BUG: Bad page state in process qemu-kvm  pfn:dd0b6
[35380.805825] page:ffff7fe003742d80 count:-4871 mapcount:-2126053375
mapping:          (null) index:0x0
[35380.815024] flags: 0x1fffc00000000000()
[35380.818845] raw: 1fffc00000000000 0000000000000000 0000000000000000
ffffecf981470000
[35380.826569] raw: dead000000000100 dead000000000200 ffff8017c001c000
0000000000000000
[35380.805825] page:ffff7fe003742d80 count:-4871 mapcount:-2126053375
mapping:          (null) index:0x0
[35380.815024] flags: 0x1fffc00000000000()
[35380.818845] raw: 1fffc00000000000 0000000000000000 0000000000000000
ffffecf981470000
[35380.826569] raw: dead000000000100 dead000000000200 ffff8017c001c000
0000000000000000
[35380.834294] page dumped because: nonzero _refcount
[35380.839069] Modules linked in: vhost_net vhost tap ebtable_filter
ebtables ip6table_filter ip6_tables iptable_filter fcoe libfcoe libfc
8021q garp mrp stp llc scsi_transport_fc openvswitch nf_conntrack_ipv6
nf_nat_ipv6 nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_defrag_ipv6
nf_nat nf_conntrack vfat fat rpcrdma ib_isert iscsi_target_mod ib_iser
libiscsi scsi_transport_iscsi ib_srpt target_core_mod ib_srp
scsi_transport_srp ib_ipoib rdma_ucm ib_ucm ib_uverbs ib_umad rdma_cm
ib_cm iw_cm mlx5_ib ib_core crc32_ce ipmi_ssif tpm_tis tpm_tis_core sg
nfsd auth_rpcgss nfs_acl lockd grace sunrpc dm_multipath ip_tables xfs
libcrc32c mlx5_core mlxfw devlink ahci_platform libahci_platform libahci
qcom_emac sdhci_acpi sdhci hdma mmc_core hdma_mgmt i2c_qup dm_mirror
dm_region_hash dm_log dm_mod
[35380.908341] CPU: 29 PID: 18323 Comm: qemu-kvm Tainted: G W
4.14.15-5.hxt.aarch64 #1
[35380.917107] Hardware name: <snip for confidential issues>
[35380.930909] Call trace:
[35380.933345] [<ffff000008088f00>] dump_backtrace+0x0/0x22c
[35380.938723] [<ffff000008089150>] show_stack+0x24/0x2c
[35380.943759] [<ffff00000893c078>] dump_stack+0x8c/0xb0
[35380.948794] [<ffff00000820ab50>] bad_page+0xf4/0x154
[35380.953740] [<ffff000008211ce8>] free_pages_check_bad+0x90/0x9c
[35380.959642] [<ffff00000820c430>] free_pcppages_bulk+0x464/0x518
[35380.965545] [<ffff00000820db98>] free_hot_cold_page+0x22c/0x300
[35380.971448] [<ffff0000082176fc>] __put_page+0x54/0x60
[35380.976484] [<ffff0000080b1164>] unmap_stage2_range+0x170/0x2b4
[35380.982385] [<ffff0000080b12d8>] kvm_unmap_hva_handler+0x30/0x40
[35380.988375] [<ffff0000080b0104>] handle_hva_to_gpa+0xb0/0xec
[35380.994016] [<ffff0000080b2644>] kvm_unmap_hva_range+0x5c/0xd0
[35380.999833] [<ffff0000080a8054>]
kvm_mmu_notifier_invalidate_range_start+0x60/0xb0
[35381.007387] [<ffff000008271f44>]
__mmu_notifier_invalidate_range_start+0x64/0x8c
[35381.014765] [<ffff0000082547c8>] try_to_unmap_one+0x78c/0x7a4
[35381.020493] [<ffff000008276d04>] rmap_walk_ksm+0x124/0x1a0
[35381.025961] [<ffff0000082551b4>] rmap_walk+0x94/0x98
[35381.030909] [<ffff0000082555e4>] try_to_unmap+0x100/0x124
[35381.036293] [<ffff00000828243c>] unmap_and_move+0x480/0x6fc
[35381.041847] [<ffff000008282b6c>] migrate_pages+0x10c/0x288
[35381.047318] [<ffff00000823c164>] compact_zone+0x238/0x954
[35381.052697] [<ffff00000823c944>] compact_zone_order+0xc4/0xe8
[35381.058427] [<ffff00000823d25c>] try_to_compact_pages+0x160/0x294
[35381.064503] [<ffff00000820f074>]
__alloc_pages_direct_compact+0x68/0x194
[35381.071187] [<ffff000008210138>] __alloc_pages_nodemask+0xc20/0xf7c
[35381.077437] [<ffff0000082709e4>] alloc_pages_vma+0x1a4/0x1c0
[35381.083080] [<ffff000008285b68>]
do_huge_pmd_anonymous_page+0x128/0x324
[35381.089677] [<ffff000008248a24>] __handle_mm_fault+0x71c/0x7e8
[35381.095492] [<ffff000008248be8>] handle_mm_fault+0xf8/0x194
[35381.101049] [<ffff000008240dcc>] __get_user_pages+0x124/0x34c
[35381.106777] [<ffff000008241870>] populate_vma_page_range+0x90/0x9c
[35381.112941] [<ffff000008241940>] __mm_populate+0xc4/0x15c
[35381.118322] [<ffff00000824b294>] SyS_mlockall+0x100/0x164
[35381.123705] Exception stack(0xffff800dce5f3ec0 to 0xffff800dce5f4000)
[35381.130128] 3ec0: 0000000000000003 d6e6024cc9b87e00 0000aaaabe94f000
0000000000000000
[35381.137940] 3ee0: 0000000000000002 0000000000000000 0000000000000000
0000aaaacf6fc3c0
[35381.145753] 3f00: 00000000000000e6 0000aaaacf6fc490 0000ffffeeeab0f0
d6e6024cc9b87e00
[35381.153565] 3f20: 0000000000000000 0000aaaabe81b3c0 0000000000000020
00009e53eff806b5
[35381.161379] 3f40: 0000aaaabe94de48 0000ffffa7c269b0 0000000000000011
0000ffffeeeabf68
[35381.169190] 3f60: 0000aaaaceacfe60 0000aaaabe94f000 0000aaaabe9ba358
0000aaaabe7ffb80
[35381.177003] 3f80: 0000aaaabe9ba000 0000aaaabe959f64 0000000000000000
0000aaaabe94f000
[35381.184815] 3fa0: 0000000000000000 0000ffffeeeabdb0 0000aaaabe5f3bf8
0000ffffeeeabdb0
[35381.192628] 3fc0: 0000ffffa7c269b8 0000000060000000 0000000000000003
00000000000000e6
[35381.200440] 3fe0: 0000000000000000 0000000000000000 0000000000000000
0000000000000000
[35381.208254] [<ffff00000808339c>] __sys_trace_return+0x0/0x4
[35381.213809] Disabling lock debugging due to kernel taint
--------------------------------end--------------------------------------

The root cause might be what I fixed at [1]. But from arm kvm points of
view, it would be better we caught the exception earlier and clearer.

If the size is not PAGE_SIZE aligned, unmap_stage2_range might unmap the
wrong(more or less) page range. Hence it caused the "BUG: Bad page
state"

[1] https://lkml.org/lkml/2018/5/3/1042

Signed-off-by: jia.he at hxt-semitech.com
Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
v3: resend after dropping patch 2/2 of the series

 virt/kvm/arm/mmu.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
index 7f6a944..8dac311 100644
--- a/virt/kvm/arm/mmu.c
+++ b/virt/kvm/arm/mmu.c
@@ -297,6 +297,8 @@ static void unmap_stage2_range(struct kvm *kvm, phys_addr_t start, u64 size)
 	phys_addr_t next;
 
 	assert_spin_locked(&kvm->mmu_lock);
+	WARN_ON(size & ~PAGE_MASK);
+
 	pgd = kvm->arch.pgd + stage2_pgd_index(addr);
 	do {
 		/*
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH] arm64: KVM: reduce guest fpsimd trap
From: Tangnianyao (ICT) @ 2018-05-21  2:56 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180520120350.212b4b4d@why.wild-wind.fr.eu.org>

On Sun, May 20, 2018 at 19:04 PM GST+8, Marc Zyngier wrote:

> On Sat, 19 May 2018 02:38:33 +0000
> "Tangnianyao (ICT)" <tangnianyao@huawei.com> wrote:
> 
> > On Wed, May 16, 2018 at 20:46 PM GST+8, Christoffer Dall wrote:
> > > On Wed, May 16, 2018 at 11:32:17AM +0100, Dave Martin wrote:  
> > > > On Wed, May 16, 2018 at 10:25:40AM +0100, Marc Zyngier wrote:  
> > > > > [+Dave]
> > > > > 
> > > > > Hi Nianyao,
> > > > > 
> > > > > On 16/05/18 10:08, Tangnianyao (ICT) wrote:  
> > > > > > Add last_fpsimd_trap to notify if guest last exit reason is handling fpsimd. If guest is using fpsimd frequently, save host's fpsimd state and restore guest's fpsimd state and deactive fpsimd trap before returning to guest. It can avoid guest fpsimd trap soon to improve performance.  
> > > > 
> > > > So, the purpose of this patch is to context switch the FPSIMD 
> > > > state on initial entry to the guest, instead of enabling the trap 
> > > > and context switching the FPSIMD state lazily?
> > > > 
> > > > And you decide whether to do this or not, based on whether the guest   
> > > triggered a lazy FPSIMD switch previously?  
> > 
> > I try to detect guest using fpsimd , but not overtraining which may 
> > include more complexity and not suitable for common cases. Therefore I 
> > just decide whether guest have triggerd a lazy fpsimd switch last time.
> > 
> > > >   
> > > > > > 
> > > > > > Signed-off-by: Nianyao Tang <tangnianyao@huawei.com>
> > > > > > ---
> > > > > >  arch/arm64/kernel/asm-offsets.c |  1 +
> > > > > >  arch/arm64/kvm/hyp/entry.S      |  5 +++++
> > > > > >  arch/arm64/kvm/hyp/switch.c     | 38 ++++++++++++++++++++++++++++++++++++--
> > > > > >  include/linux/kvm_host.h        |  1 +
> > > > > >  4 files changed, 43 insertions(+), 2 deletions(-)
> > > > > > 
> > > > > > diff --git a/arch/arm64/kernel/asm-offsets.c 
> > > > > > b/arch/arm64/kernel/asm-offsets.c index 5bdda65..35a9c5c 
> > > > > > 100644
> > > > > > --- a/arch/arm64/kernel/asm-offsets.c
> > > > > > +++ b/arch/arm64/kernel/asm-offsets.c
> > > > > > @@ -136,6 +136,7 @@ int main(void)  #ifdef CONFIG_KVM_ARM_HOST
> > > > > >    DEFINE(VCPU_CONTEXT,		offsetof(struct kvm_vcpu, arch.ctxt));
> > > > > >    DEFINE(VCPU_FAULT_DISR,	offsetof(struct kvm_vcpu, arch.fault.disr_el1));
> > > > > > +  DEFINE(VCPU_LAST_FPSIMD_TRAP, offsetof(struct kvm_vcpu, 
> > > > > > + last_fpsimd_trap));
> > > > > >    DEFINE(CPU_GP_REGS,		offsetof(struct kvm_cpu_context, gp_regs));
> > > > > >    DEFINE(CPU_USER_PT_REGS,	offsetof(struct kvm_regs, regs));
> > > > > >    DEFINE(CPU_FP_REGS,		offsetof(struct kvm_regs, fp_regs));
> > > > > > diff --git a/arch/arm64/kvm/hyp/entry.S 
> > > > > > b/arch/arm64/kvm/hyp/entry.S index e41a161..956e042 100644
> > > > > > --- a/arch/arm64/kvm/hyp/entry.S
> > > > > > +++ b/arch/arm64/kvm/hyp/entry.S
> > > > > > @@ -197,6 +197,11 @@ alternative_endif
> > > > > >  	add	x0, x2, #CPU_GP_REG_OFFSET(CPU_FP_REGS)
> > > > > >  	bl	__fpsimd_restore_state
> > > > > >  
> > > > > > +	// Mark guest using fpsimd now
> > > > > > +	ldr x0, [x3, #VCPU_LAST_FPSIMD_TRAP]
> > > > > > +	add x0, x0, #1
> > > > 
> > > > Can this overflow?  
> > 
> > It won't overflow. It only trigger one fpsimd trap and restore guest's 
> > fpsimd state and deactivate fpsimd trap. When next non-fpsimd trap comes, it will be clear unconditionally.
> > 
> > > >   
> > > > > > +	str x0, [x3, #VCPU_LAST_FPSIMD_TRAP]
> > > > > > +
> > > > > >  	// Skip restoring fpexc32 for AArch64 guests
> > > > > >  	mrs	x1, hcr_el2
> > > > > >  	tbnz	x1, #HCR_RW_SHIFT, 1f
> > > > > > diff --git a/arch/arm64/kvm/hyp/switch.c 
> > > > > > b/arch/arm64/kvm/hyp/switch.c index d964523..86eea1b 100644
> > > > > > --- a/arch/arm64/kvm/hyp/switch.c
> > > > > > +++ b/arch/arm64/kvm/hyp/switch.c
> > > > > > @@ -92,7 +92,13 @@ static void activate_traps_vhe(struct 
> > > > > > kvm_vcpu
> > > > > > *vcpu)
> > > > > >  
> > > > > >  	val = read_sysreg(cpacr_el1);
> > > > > >  	val |= CPACR_EL1_TTA;
> > > > > > -	val &= ~(CPACR_EL1_FPEN | CPACR_EL1_ZEN);
> > > > > > +	val &= ~CPACR_EL1_ZEN;
> > > > > > +
> > > > > > +	if (vcpu->last_fpsimd_trap)
> > > > > > +		val |= CPACR_EL1_FPEN;
> > > > > > +	else
> > > > > > +		val &= ~CPACR_EL1_FPEN;
> > > > > > +
> > > > > >  	write_sysreg(val, cpacr_el1);
> > > > > >  
> > > > > >  	write_sysreg(kvm_get_hyp_vector(), vbar_el1); @@ -105,7 +111,13 @@ static void __hyp_text __activate_traps_nvhe(struct kvm_vcpu *vcpu)
> > > > > >  	__activate_traps_common(vcpu);
> > > > > >  
> > > > > >  	val = CPTR_EL2_DEFAULT;
> > > > > > -	val |= CPTR_EL2_TTA | CPTR_EL2_TFP | CPTR_EL2_TZ;
> > > > > > +	val |= CPTR_EL2_TTA | CPTR_EL2_TZ;
> > > > > > +
> > > > > > +	if (vcpu->last_fpsimd_trap)
> > > > > > +		val &= ~CPTR_EL2_TFP;
> > > > > > +	else
> > > > > > +		val |= CPTR_EL2_TFP;
> > > > > > +
> > > > > >  	write_sysreg(val, cptr_el2);  }
> > > > > >  
> > > > > > @@ -406,6 +418,17 @@ int kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
> > > > > >  	__activate_traps(vcpu);
> > > > > >  	__activate_vm(vcpu->kvm);
> > > > > >  
> > > > > > +	/*
> > > > > > +	 * If guest last trap to host for handling fpsimd, last_fpsimd_trap
> > > > > > +	 * is set. Restore guest's fpsimd state and deactivate fpsimd trap
> > > > > > +	 * to avoid guest traping soon.
> > > > > > +	 */
> > > > > > +	if (vcpu->last_fpsimd_trap) {
> > > > > > +		__fpsimd_save_state(&host_ctxt->gp_regs.fp_regs);
> > > > > > +		__fpsimd_restore_state(&guest_ctxt->gp_regs.fp_regs);
> > > > > > +		vcpu->last_fpsimd_trap = 0;
> > > > > > +	}
> > > > > > +
> > > > > >  	sysreg_restore_guest_state_vhe(guest_ctxt);
> > > > > >  	__debug_switch_to_guest(vcpu);
> > > > > >  
> > > > > > @@ -454,6 +477,17 @@ int __hyp_text __kvm_vcpu_run_nvhe(struct kvm_vcpu *vcpu)
> > > > > >  	__activate_traps(vcpu);
> > > > > >  	__activate_vm(kern_hyp_va(vcpu->kvm));
> > > > > >  
> > > > > > +	/*
> > > > > > +	 * If guest last trap to host for handling fpsimd, last_fpsimd_trap
> > > > > > +	 * is set. Restore guest's fpsimd state and deactivate fpsimd trap
> > > > > > +	 * to avoid guest traping soon.
> > > > > > +	 */
> > > > > > +	if (vcpu->last_fpsimd_trap) {
> > > > > > +		__fpsimd_save_state(&host_ctxt->gp_regs.fp_regs);
> > > > > > +		__fpsimd_restore_state(&guest_ctxt->gp_regs.fp_regs);
> > > > > > +		vcpu->last_fpsimd_trap = 0;
> > > > > > +	}
> > > > > > +
> > > > > >  	__hyp_vgic_restore_state(vcpu);
> > > > > >  	__timer_enable_traps(vcpu);
> > > > > >  
> > > > > > diff --git a/include/linux/kvm_host.h 
> > > > > > b/include/linux/kvm_host.h index 6930c63..46bdf0d 100644
> > > > > > --- a/include/linux/kvm_host.h
> > > > > > +++ b/include/linux/kvm_host.h
> > > > > > @@ -274,6 +274,7 @@ struct kvm_vcpu {
> > > > > >  	bool preempted;
> > > > > >  	struct kvm_vcpu_arch arch;
> > > > > >  	struct dentry *debugfs_dentry;
> > > > > > +	unsigned int last_fpsimd_trap;
> > > > > >  };
> > > > > >  
> > > > > >  static inline int kvm_vcpu_exiting_guest_mode(struct kvm_vcpu
> > > > > > *vcpu)
> > > > > > --
> > > > > > 2.7.4
> > > > > >   
> > > > > 
> > > > > This doesn't seem to be the 100% correct. I can't see how this 
> > > > > works when being preempted, for example... I suggest you look at 
> > > > > Dave Martin's series[1], which does this correctly.
> > > > 
> > > > If I've understood correctly, this chooses between eager and lazy 
> > > > switching, which is addressing a different issue from [1].
> > > > 
> > > > In effect, this code is attempting to predict whether the guest 
> > > > will use FPSIMD before the next exit.  If the prediction is 
> > > > correct and the guest does use FPSIMD, then the overhead of the 
> > > > lazy FPSIMD trap disappears.  This is probably a good thing, 
> > > > though it may increase interrupt latency a little.  However, if 
> > > > the prediction is wrong and the guest doesn't use FPSIMD before 
> > > > the next exit, then the overhead of guest entry increases for no 
> > > > benefit, because FPSIMD was switched unnecessarily.
> > > > 
> > > > Do you have any benchmarks or metrics on the accuracy of the 
> > > > prediction and the overall impact on performance?
> > > > 
> > > > The changes in [1] should reduce the number of FPSIMD context 
> > > > switches overall, so may reduce the benefit of this patch.
> > > >   
> > 
> > This idea may be different. It tries to detect guest using fpsimd, do 
> > eager switching ,and reduce guest trap. [1] reduces fpsimd context 
> > switches in host when it has already trapped.
> > 
> > > 
> > > 
> > > Based on previous measurements [2], I found that only in 29% of the times when we return to userspace or are preempted, has the VCPU touched the FPSIMD registers, and therefore a significant majority of the times we enter the guest, the guest doesn't touch the FPSIMD state, even if it has touched it before.
> > > 
> > > As Dave suggests, if there is data that shows that an immediately following entry from an exit that had VFP enabled implies with a very high probablity that the guest will touch VFP again, then something like this might make sense, but I'd like to see some data to back up this hypothesis before adding additional complexity to the code.
> > > 
> > > [2]: 
> > > https://lists.cs.columbia.edu/pipermail/kvmarm/2018-February/029835.
> > > html
> > 
> > I try to detect guest using fpsimd, and choose eager switching to avoid trapping to hyp soon.
> > The reason is, in some micro architecture, vmid switch will flush all 
> > L1 TLB entry which doesn't contain vmid. It will result in extra 
> > 2-stage table walk when returning to guest, which could be avoid if not trap to el2. As I know, Maia has this issue.
> 
> This looks extremely microarchitectural. Have you tested how this behaves on other implementations?

I have only tested on Maia. As I know in research, both Maia and A72 behave in this way, so
it may be a common problem.

> 
> > I have done some benchmark test on guest and find that, frequent 
> > context switch , which save and restore fpsimd frequently, resulted in 
> > extra cost compared to host context switch because of fpsimd trap. In 
> > some benchmarks, like lat_ctx and lat_udp, one asid usually got stuck and scheduled to another, where context switch and handling fpsimd happened very frequently.
> > In my performance test, lat_ctx with 4 asid switch. In comparison 
> > between guest and host, without this, guest is 10% worse than host. With this, guest is only 8% worse than host.
> > 
> > Detect guest using fpsimd based on lazy fpsimd switch may be not 100% correct. 
> > I think accurate detection may be overtraining and is not suitable for common cases.
> > Therefore I just do it in simply way to reduce some fpsimd traps.
> 
> Can you please with Dave's series (which I have now merged for 4.18) and let us know if you're still seeing this overhead? If you're still seeing it, we can then look into it.

Ok? I will seeing this based on 4.18.

Thanks,
-Nianyao Tang

^ permalink raw reply

* [PATCH V2 2/3] clk: imx7d: correct enet clock CCGR registers
From: Anson Huang @ 2018-05-21  2:35 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <c42c4274cfd06297d1699c96d3253125@agner.ch>

Hi, Stefan

Anson Huang
Best Regards!


> -----Original Message-----
> From: Stefan Agner [mailto:stefan at agner.ch]
> Sent: Friday, May 18, 2018 9:02 PM
> To: Anson Huang <anson.huang@nxp.com>
> Cc: shawnguo at kernel.org; kernel at pengutronix.de; Fabio Estevam
> <fabio.estevam@nxp.com>; robh+dt at kernel.org; mark.rutland at arm.com;
> mturquette at baylibre.com; sboyd at kernel.org; Adriana Reus
> <adriana.reus@nxp.com>; rui.silva at linaro.org; dl-linux-imx
> <linux-imx@nxp.com>; linux-arm-kernel at lists.infradead.org;
> devicetree at vger.kernel.org; linux-kernel at vger.kernel.org;
> linux-clk at vger.kernel.org
> Subject: Re: [PATCH V2 2/3] clk: imx7d: correct enet clock CCGR registers
> 
> On 18.05.2018 03:01, Anson Huang wrote:
> > Correct enet clock gates as below:
> >
> > CCGR6: IMX7D_ENET_AXI_ROOT_CLK (enet1 and enet2 bus clocks)
> > CCGR112: IMX7D_ENET1_TIME_ROOT_CLK, IMX7D_ENET1_IPG_ROOT_CLK
> > CCGR113: IMX7D_ENET2_TIME_ROOT_CLK, IMX7D_ENET2_IPG_ROOT_CLK
> >
> > Just rename unused IMX7D_ENETx_REF_ROOT_CLK for
> > IMX7D_ENETx_IPG_ROOT_CLK instead of adding new clocks.
> 
> Are you sure that IMX7D_ENETx_REF_ROOT_CLK are not used?
> 
> I understand that the reference manual does not a gate at 0x44e0...
> 
> But in a earlier revision of our Colibri iMX7 we actually used clock out, and
> referenced this clock to enable the reference clock (see also:
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatch
> work.kernel.org%2Fpatch%2F9211371%2F&data=02%7C01%7CAnson.Huang%
> 40nxp.com%7Cea0856a68d8e4b921ba608d5bcbf9c02%7C686ea1d3bc2b4c6fa
> 92cd99c5c301635%7C0%7C1%7C636622453508888330&sdata=rEhwj0innLDc
> AEgxJyqd5vtG3SNVS05r2hEFvSc%2BQQs%3D&reserved=0).
> 
> I guess if the gate really does not exist, then we should/would have to set
> IMX7D_ENET1_REF_ROOT_DIV to use the SoC provided ref clock.
> 
> --
> Stefan
 
I looked into the RTL and also checked with our design team, they confirm that
there is no CCGR78(0x44e0) and CCGR80(0x4500) on i.MX7D, the register offset
are there, but no hardware wire connection for them. That is why they did NOT
list them in Reference Manual. So I think we can remove them. 

For your case of using them as clock input, maybe clock tree auto use its parent
IMX7D_ENETx_REF_ROOT_DIV which is existing, so it works.

Anson.

> 
> >
> > Based on Andy Duan's patch from the NXP kernel tree.
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > ---
> >  drivers/clk/imx/clk-imx7d.c             | 10 ++++++----
> >  include/dt-bindings/clock/imx7d-clock.h |  4 ++--
> >  2 files changed, 8 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/clk/imx/clk-imx7d.c b/drivers/clk/imx/clk-imx7d.c
> > index 23d5090a..d4936b9 100644
> > --- a/drivers/clk/imx/clk-imx7d.c
> > +++ b/drivers/clk/imx/clk-imx7d.c
> > @@ -26,6 +26,8 @@ static u32 share_count_sai1;  static u32
> > share_count_sai2;  static u32 share_count_sai3;  static u32
> > share_count_nand;
> > +static u32 share_count_enet1;
> > +static u32 share_count_enet2;
> >
> >  static const struct clk_div_table test_div_table[] = {
> >  	{ .val = 3, .div = 1, },
> > @@ -805,6 +807,10 @@ static void __init imx7d_clocks_init(struct
> > device_node *ccm_node)
> >  	clks[IMX7D_MIPI_DSI_ROOT_CLK] = imx_clk_gate4("mipi_dsi_root_clk",
> > "mipi_dsi_post_div", base + 0x4650, 0);
> >  	clks[IMX7D_MIPI_CSI_ROOT_CLK] = imx_clk_gate4("mipi_csi_root_clk",
> > "mipi_csi_post_div", base + 0x4640, 0);
> >  	clks[IMX7D_MIPI_DPHY_ROOT_CLK] =
> imx_clk_gate4("mipi_dphy_root_clk",
> > "mipi_dphy_post_div", base + 0x4660, 0);
> > +	clks[IMX7D_ENET1_IPG_ROOT_CLK] =
> > imx_clk_gate2_shared2("enet1_ipg_root_clk", "enet_axi_post_div", base
> > + 0x4700, 0, &share_count_enet1);
> > +	clks[IMX7D_ENET1_TIME_ROOT_CLK] =
> > imx_clk_gate2_shared2("enet1_time_root_clk", "enet1_time_post_div",
> > base + 0x4700, 0, &share_count_enet1);
> > +	clks[IMX7D_ENET2_IPG_ROOT_CLK] =
> > imx_clk_gate2_shared2("enet2_ipg_root_clk", "enet_axi_post_div", base
> > + 0x4710, 0, &share_count_enet2);
> > +	clks[IMX7D_ENET2_TIME_ROOT_CLK] =
> > imx_clk_gate2_shared2("enet2_time_root_clk", "enet2_time_post_div",
> > base + 0x4710, 0, &share_count_enet2);
> >  	clks[IMX7D_SAI1_ROOT_CLK] = imx_clk_gate2_shared2("sai1_root_clk",
> > "sai1_post_div", base + 0x48c0, 0, &share_count_sai1);
> >  	clks[IMX7D_SAI1_IPG_CLK]  = imx_clk_gate2_shared2("sai1_ipg_clk",
> > "ipg_root_clk",  base + 0x48c0, 0, &share_count_sai1);
> >  	clks[IMX7D_SAI2_ROOT_CLK] = imx_clk_gate2_shared2("sai2_root_clk",
> > "sai2_post_div", base + 0x48d0, 0, &share_count_sai2); @@ -812,10
> > +818,6 @@ static void __init imx7d_clocks_init(struct device_node
> > *ccm_node)
> >  	clks[IMX7D_SAI3_ROOT_CLK] = imx_clk_gate2_shared2("sai3_root_clk",
> > "sai3_post_div", base + 0x48e0, 0, &share_count_sai3);
> >  	clks[IMX7D_SAI3_IPG_CLK]  = imx_clk_gate2_shared2("sai3_ipg_clk",
> > "ipg_root_clk",  base + 0x48e0, 0, &share_count_sai3);
> >  	clks[IMX7D_SPDIF_ROOT_CLK] = imx_clk_gate4("spdif_root_clk",
> > "spdif_post_div", base + 0x44d0, 0);
> > -	clks[IMX7D_ENET1_REF_ROOT_CLK] =
> imx_clk_gate4("enet1_ref_root_clk",
> > "enet1_ref_post_div", base + 0x44e0, 0);
> > -	clks[IMX7D_ENET1_TIME_ROOT_CLK] =
> > imx_clk_gate4("enet1_time_root_clk", "enet1_time_post_div", base +
> > 0x44f0, 0);
> > -	clks[IMX7D_ENET2_REF_ROOT_CLK] =
> imx_clk_gate4("enet2_ref_root_clk",
> > "enet2_ref_post_div", base + 0x4500, 0);
> > -	clks[IMX7D_ENET2_TIME_ROOT_CLK] =
> > imx_clk_gate4("enet2_time_root_clk", "enet2_time_post_div", base +
> > 0x4510, 0);
> >  	clks[IMX7D_EIM_ROOT_CLK] = imx_clk_gate4("eim_root_clk",
> > "eim_post_div", base + 0x4160, 0);
> >  	clks[IMX7D_NAND_RAWNAND_CLK] =
> > imx_clk_gate2_shared2("nand_rawnand_clk", "nand_root_clk", base +
> > 0x4140, 0, &share_count_nand);
> >  	clks[IMX7D_NAND_USDHC_BUS_RAWNAND_CLK] =
> > imx_clk_gate2_shared2("nand_usdhc_rawnand_clk",
> "nand_usdhc_root_clk",
> > base + 0x4140, 0, &share_count_nand); diff --git
> > a/include/dt-bindings/clock/imx7d-clock.h
> > b/include/dt-bindings/clock/imx7d-clock.h
> > index b2325d3e2..0d67f53 100644
> > --- a/include/dt-bindings/clock/imx7d-clock.h
> > +++ b/include/dt-bindings/clock/imx7d-clock.h
> > @@ -168,7 +168,7 @@
> >  #define IMX7D_SPDIF_ROOT_SRC		155
> >  #define IMX7D_SPDIF_ROOT_CG		156
> >  #define IMX7D_SPDIF_ROOT_DIV		157
> > -#define IMX7D_ENET1_REF_ROOT_CLK	158
> > +#define IMX7D_ENET1_IPG_ROOT_CLK        158
> >  #define IMX7D_ENET1_REF_ROOT_SRC	159
> >  #define IMX7D_ENET1_REF_ROOT_CG		160
> >  #define IMX7D_ENET1_REF_ROOT_DIV	161
> > @@ -176,7 +176,7 @@
> >  #define IMX7D_ENET1_TIME_ROOT_SRC	163
> >  #define IMX7D_ENET1_TIME_ROOT_CG	164
> >  #define IMX7D_ENET1_TIME_ROOT_DIV	165
> > -#define IMX7D_ENET2_REF_ROOT_CLK	166
> > +#define IMX7D_ENET2_IPG_ROOT_CLK        166
> >  #define IMX7D_ENET2_REF_ROOT_SRC	167
> >  #define IMX7D_ENET2_REF_ROOT_CG		168
> >  #define IMX7D_ENET2_REF_ROOT_DIV	169

^ permalink raw reply

* [PATCH] ARM: dts: imx7d: use operating-points-v2 for cpu
From: Anson Huang @ 2018-05-21  1:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180520125048.GX26863@dragon>



Anson Huang
Best Regards!


> -----Original Message-----
> From: Shawn Guo [mailto:shawnguo at kernel.org]
> Sent: Sunday, May 20, 2018 8:51 PM
> To: Anson Huang <anson.huang@nxp.com>
> Cc: kernel at pengutronix.de; Fabio Estevam <fabio.estevam@nxp.com>;
> robh+dt at kernel.org; mark.rutland at arm.com; dl-linux-imx
> <linux-imx@nxp.com>; linux-arm-kernel at lists.infradead.org;
> devicetree at vger.kernel.org; linux-kernel at vger.kernel.org
> Subject: Re: [PATCH] ARM: dts: imx7d: use operating-points-v2 for cpu
> 
> On Wed, May 16, 2018 at 12:48:17PM +0800, Anson Huang wrote:
> > This patch uses "operating-points-v2" instead of "operating-points" to
> > be more fit with cpufreq-dt driver.
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > ---
> >  arch/arm/boot/dts/imx7d.dtsi | 24 +++++++++++++++++++-----
> >  1 file changed, 19 insertions(+), 5 deletions(-)
> >
> > diff --git a/arch/arm/boot/dts/imx7d.dtsi
> > b/arch/arm/boot/dts/imx7d.dtsi index 4c9877e..28980c8 100644
> > --- a/arch/arm/boot/dts/imx7d.dtsi
> > +++ b/arch/arm/boot/dts/imx7d.dtsi
> > @@ -9,12 +9,8 @@
> >  / {
> >  	cpus {
> >  		cpu0: cpu at 0 {
> > -			operating-points = <
> > -				/* KHz	uV */
> > -				996000	1075000
> > -				792000	975000
> > -			>;
> >  			clock-frequency = <996000000>;
> > +			operating-points-v2 = <&cpu0_opp_table>;
> >  		};
> >
> >  		cpu1: cpu at 1 {
> > @@ -22,6 +18,24 @@
> >  			device_type = "cpu";
> >  			reg = <1>;
> >  			clock-frequency = <996000000>;
> > +			operating-points-v2 = <&cpu0_opp_table>;
> > +		};
> > +	};
> > +
> > +	cpu0_opp_table: opp_table0 {
> 
> Hyphen is recommended in node name.  Also the suffix 0 doesn't mean too
> much here.  That said, a better node name would be 'opp-table'.
> 
> > +		compatible = "operating-points-v2";
> > +		opp-shared;
> > +
> > +		opp-792000000 {
> > +			opp-hz = /bits/ 64 <792000000>;
> > +			opp-microvolt = <975000>;
> > +			clock-latency-ns = <150000>;
> > +		};
> 
> We recommend to have a newline between nodes.
> 
> I fixed them all and applied the patch.
> 
> Shawn
 
Thanks Shawn.

Anson.



> 
> > +		opp-996000000 {
> > +			opp-hz = /bits/ 64 <996000000>;
> > +			opp-microvolt = <1075000>;
> > +			clock-latency-ns = <150000>;
> > +			opp-suspend;
> >  		};
> >  	};
> >
> > --
> > 2.7.4
> >

^ permalink raw reply

* [reset-control] How to initialize hardware state with the shared reset line?
From: Masahiro Yamada @ 2018-05-21  1:27 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAFBinCADQsXoc6CUKabH6VX3ZBYb5apo-xBbe1w0BiO8wBAsPw@mail.gmail.com>

Hi.


2018-05-20 19:57 GMT+09:00 Martin Blumenstingl
<martin.blumenstingl@googlemail.com>:
> Hi,
>
> On Thu, May 10, 2018 at 11:16 AM, Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
> [snip]
>> I may be missing something, but
>> one solution might be reset hogging on the
>> reset provider side.  This allows us to describe
>> the initial state of reset lines in the reset controller.
>>
>> The idea for "reset-hog" is similar to:
>>  - "gpio-hog" defined in
>>    Documentation/devicetree/bindings/gpio/gpio.txt
>>  - "assigned-clocks" defined in
>>    Documetation/devicetree/bindings/clock/clock-bindings.txt
>>
>>
>>
>> For example,
>>
>>    reset-controller {
>>             ....
>>
>>             line_a {
>>                   reset-hog;
>>                   resets = <1>;
>>                   reset-assert;
>>             };
>>    }
>>
>>
>> When the reset controller is registered,
>> the reset ID '1' is asserted.
>>
>>
>> So, all reset consumers that share the reset line '1'
>> will start from the asserted state
>> (i.e. defined state machine state).
> I wonder if a "reset hog" can be board specific:
> - GPIO hogs are definitely board specific (meson-gxbb-odroidc2.dts for
> example uses it to take the USB hub out of reset)
> - assigned-clock-parents (and the like) can also be board specific (I
> made up a use-case since I don't know of any actual examples: board A
> uses an external XTAL while board B uses some other internal
> clock-source because it doesn't have an external XTAL)
>
> however, can reset lines be board specific? or in other words: do we
> need to describe them in device-tree?

Indeed.

I did not come up with board-specific cases.

The problem we are discussing is SoC-specific,
and reset-controller drivers are definitely SoC-specific.

So, I think the initial state can be coded in drivers instead of DT.


> we could extend struct reset_controller_dev (= reset controller
> driver) if they are not board specific:
> - either assert all reset lines by default except if they are listed
> in a new field (may break backwards compatibility, requires testing of
> all reset controller drivers)

This is quite simple, but I am afraid there are some cases where the forcible
reset-assert is not preferred.

For example, the earlycon.  When we use earlycon, we would expect it has been
initialized by a boot-loader, or something.
If it is reset-asserted on the while, the console output
will not be good.



> - specify a list of reset lines and their desired state (or to keep it
> easy: specify a list of reset lines that should be asserted)
> (I must admit that this is basically your idea but the definition is
> moved from device-tree to the reset controller driver)

Yes, I think the list of "reset line ID" and "init state" pairs
would be nicer.


> any "chip" specific differences could be expressed by using a
> different of_device_id
>
> one the other hand: your "reset hog" solution looks fine to me if
> reset lines can be board specific
>
>> From the discussion with Martin Blumenstingl
>> (https://lkml.org/lkml/2018/4/28/115),
>> the problem for Amlogic is that
>> the reset line is "de-asserted" by default.
>> If so, the 'reset-hog' would fix the problem,
>> and DWC3 driver would be able to use
>> shared, level reset, I think.
> I think you are right: if we could control the initial state then we
> should be able to use level resets


Even further, can we drop the shared reset_control_reset() support, maybe?
(in other words, revert commit 7da33a37b48f11)


Thanks for your comment!


>
> Regards
> Martin
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Best Regards
Masahiro Yamada

^ permalink raw reply

* [PATCH v3 2/3] arm64: dts: renesas: draak: Describe CVBS input
From: kbuild test robot @ 2018-05-20 23:28 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1526654878-11143-3-git-send-email-jacopo+renesas@jmondi.org>

Hi Jacopo,

I love your patch! Yet something to improve:

[auto build test ERROR on linuxtv-media/master]
[cannot apply to renesas/next v4.17-rc6]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Jacopo-Mondi/arm64-dts-Draak-Enable-video-inputs-and-VIN4/20180521-052159
base:   git://linuxtv.org/media_tree.git master
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm64 

All errors (new ones prefixed by >>):

>> Error: arch/arm64/boot/dts/renesas/r8a77995-draak.dts:272.1-6 Label or path vin4 not found
>> FATAL ERROR: Syntax error parsing input tree

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/gzip
Size: 37440 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180521/60417e5b/attachment-0001.gz>

^ permalink raw reply

* [PATCH v3 1/2] soc: imx: gpcv2: Do not pass static memory as platform data
From: Andrey Smirnov @ 2018-05-20 22:24 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180520062631.GS26863@dragon>

On Sat, May 19, 2018 at 11:26 PM, Shawn Guo <shawnguo@kernel.org> wrote:
> On Sat, May 19, 2018 at 03:35:55PM -0700, Andrey Smirnov wrote:
>> On Tue, Apr 10, 2018 at 11:32 AM, Andrey Smirnov
>> <andrew.smirnov@gmail.com> wrote:
>> > Platform device core assumes the ownership of dev.platform_data as
>> > well as that it is dynamically allocated and it will try to kfree it
>> > as a part of platform_device_release(). Change the code to use
>> > platform_device_add_data() n instead of a pointer to a static memory
>> > to avoid causing a BUG() when calling platform_device_put().
>> >
>> > The problem can be reproduced by artificially enabling the error path
>> > of platform_device_add() call (around line 357).
>> >
>> > Note that this change also allows us to constify imx7_pgc_domains,
>> > since we no longer need to be able to modify it.
>> >
>>
>> Shawn,
>>
>> What's the status of these two patches? Do I need to change anything
>> or are they good to go?
>
> The patches were queued on imx/drivers branch for a while.  I forgot to
> let you know.  Sorry.
>

No worries and good to know!

Thanks,
Andrey Smirnov

^ permalink raw reply

* [kvmarm:queue 9/29] arch/arm/kvm/../../../virt/kvm/arm/arm.c:783:3: error: implicit declaration of function 'kvm_arch_vcpu_ctxsync_fp'; did you mean 'kvm_arch_vcpu_put_fp'?
From: kbuild test robot @ 2018-05-20 22:20 UTC (permalink / raw)
  To: linux-arm-kernel

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/kvmarm/kvmarm.git queue
head:   f2d1aab22d57235b58db391d318727d3e5ef1e89
commit: 9d346205940be002dc43a3274f8c6c47beddd8cc [9/29] KVM: arm64: Optimise FPSIMD handling to reduce guest/host thrashing
config: arm-axm55xx_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        git checkout 9d346205940be002dc43a3274f8c6c47beddd8cc
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All errors (new ones prefixed by >>):

   arch/arm/kvm/../../../virt/kvm/arm/arm.c: In function 'kvm_arch_vcpu_ioctl_run':
>> arch/arm/kvm/../../../virt/kvm/arm/arm.c:783:3: error: implicit declaration of function 'kvm_arch_vcpu_ctxsync_fp'; did you mean 'kvm_arch_vcpu_put_fp'? [-Werror=implicit-function-declaration]
      kvm_arch_vcpu_ctxsync_fp(vcpu);
      ^~~~~~~~~~~~~~~~~~~~~~~~
      kvm_arch_vcpu_put_fp
   cc1: some warnings being treated as errors

vim +783 arch/arm/kvm/../../../virt/kvm/arm/arm.c

   626	
   627	/**
   628	 * kvm_arch_vcpu_ioctl_run - the main VCPU run function to execute guest code
   629	 * @vcpu:	The VCPU pointer
   630	 * @run:	The kvm_run structure pointer used for userspace state exchange
   631	 *
   632	 * This function is called through the VCPU_RUN ioctl called from user space. It
   633	 * will execute VM code in a loop until the time slice for the process is used
   634	 * or some emulation is needed from user space in which case the function will
   635	 * return with return value 0 and with the kvm_run structure filled in with the
   636	 * required data for the requested emulation.
   637	 */
   638	int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
   639	{
   640		int ret;
   641	
   642		if (unlikely(!kvm_vcpu_initialized(vcpu)))
   643			return -ENOEXEC;
   644	
   645		ret = kvm_vcpu_first_run_init(vcpu);
   646		if (ret)
   647			return ret;
   648	
   649		if (run->exit_reason == KVM_EXIT_MMIO) {
   650			ret = kvm_handle_mmio_return(vcpu, vcpu->run);
   651			if (ret)
   652				return ret;
   653			if (kvm_arm_handle_step_debug(vcpu, vcpu->run))
   654				return 0;
   655		}
   656	
   657		if (run->immediate_exit)
   658			return -EINTR;
   659	
   660		vcpu_load(vcpu);
   661	
   662		kvm_sigset_activate(vcpu);
   663	
   664		ret = 1;
   665		run->exit_reason = KVM_EXIT_UNKNOWN;
   666		while (ret > 0) {
   667			/*
   668			 * Check conditions before entering the guest
   669			 */
   670			cond_resched();
   671	
   672			update_vttbr(vcpu->kvm);
   673	
   674			check_vcpu_requests(vcpu);
   675	
   676			/*
   677			 * Preparing the interrupts to be injected also
   678			 * involves poking the GIC, which must be done in a
   679			 * non-preemptible context.
   680			 */
   681			preempt_disable();
   682	
   683			/* Flush FP/SIMD state that can't survive guest entry/exit */
   684			kvm_fpsimd_flush_cpu_state();
   685	
   686			kvm_pmu_flush_hwstate(vcpu);
   687	
   688			local_irq_disable();
   689	
   690			kvm_vgic_flush_hwstate(vcpu);
   691	
   692			/*
   693			 * Exit if we have a signal pending so that we can deliver the
   694			 * signal to user space.
   695			 */
   696			if (signal_pending(current)) {
   697				ret = -EINTR;
   698				run->exit_reason = KVM_EXIT_INTR;
   699			}
   700	
   701			/*
   702			 * If we're using a userspace irqchip, then check if we need
   703			 * to tell a userspace irqchip about timer or PMU level
   704			 * changes and if so, exit to userspace (the actual level
   705			 * state gets updated in kvm_timer_update_run and
   706			 * kvm_pmu_update_run below).
   707			 */
   708			if (static_branch_unlikely(&userspace_irqchip_in_use)) {
   709				if (kvm_timer_should_notify_user(vcpu) ||
   710				    kvm_pmu_should_notify_user(vcpu)) {
   711					ret = -EINTR;
   712					run->exit_reason = KVM_EXIT_INTR;
   713				}
   714			}
   715	
   716			/*
   717			 * Ensure we set mode to IN_GUEST_MODE after we disable
   718			 * interrupts and before the final VCPU requests check.
   719			 * See the comment in kvm_vcpu_exiting_guest_mode() and
   720			 * Documentation/virtual/kvm/vcpu-requests.rst
   721			 */
   722			smp_store_mb(vcpu->mode, IN_GUEST_MODE);
   723	
   724			if (ret <= 0 || need_new_vmid_gen(vcpu->kvm) ||
   725			    kvm_request_pending(vcpu)) {
   726				vcpu->mode = OUTSIDE_GUEST_MODE;
   727				isb(); /* Ensure work in x_flush_hwstate is committed */
   728				kvm_pmu_sync_hwstate(vcpu);
   729				if (static_branch_unlikely(&userspace_irqchip_in_use))
   730					kvm_timer_sync_hwstate(vcpu);
   731				kvm_vgic_sync_hwstate(vcpu);
   732				local_irq_enable();
   733				preempt_enable();
   734				continue;
   735			}
   736	
   737			kvm_arm_setup_debug(vcpu);
   738	
   739			/**************************************************************
   740			 * Enter the guest
   741			 */
   742			trace_kvm_entry(*vcpu_pc(vcpu));
   743			guest_enter_irqoff();
   744	
   745			if (has_vhe()) {
   746				kvm_arm_vhe_guest_enter();
   747				ret = kvm_vcpu_run_vhe(vcpu);
   748				kvm_arm_vhe_guest_exit();
   749			} else {
   750				ret = kvm_call_hyp(__kvm_vcpu_run_nvhe, vcpu);
   751			}
   752	
   753			vcpu->mode = OUTSIDE_GUEST_MODE;
   754			vcpu->stat.exits++;
   755			/*
   756			 * Back from guest
   757			 *************************************************************/
   758	
   759			kvm_arm_clear_debug(vcpu);
   760	
   761			/*
   762			 * We must sync the PMU state before the vgic state so
   763			 * that the vgic can properly sample the updated state of the
   764			 * interrupt line.
   765			 */
   766			kvm_pmu_sync_hwstate(vcpu);
   767	
   768			/*
   769			 * Sync the vgic state before syncing the timer state because
   770			 * the timer code needs to know if the virtual timer
   771			 * interrupts are active.
   772			 */
   773			kvm_vgic_sync_hwstate(vcpu);
   774	
   775			/*
   776			 * Sync the timer hardware state before enabling interrupts as
   777			 * we don't want vtimer interrupts to race with syncing the
   778			 * timer virtual interrupt state.
   779			 */
   780			if (static_branch_unlikely(&userspace_irqchip_in_use))
   781				kvm_timer_sync_hwstate(vcpu);
   782	
 > 783			kvm_arch_vcpu_ctxsync_fp(vcpu);
   784	
   785			/*
   786			 * We may have taken a host interrupt in HYP mode (ie
   787			 * while executing the guest). This interrupt is still
   788			 * pending, as we haven't serviced it yet!
   789			 *
   790			 * We're now back in SVC mode, with interrupts
   791			 * disabled.  Enabling the interrupts now will have
   792			 * the effect of taking the interrupt again, in SVC
   793			 * mode this time.
   794			 */
   795			local_irq_enable();
   796	
   797			/*
   798			 * We do local_irq_enable() before calling guest_exit() so
   799			 * that if a timer interrupt hits while running the guest we
   800			 * account that tick as being spent in the guest.  We enable
   801			 * preemption after calling guest_exit() so that if we get
   802			 * preempted we make sure ticks after that is not counted as
   803			 * guest time.
   804			 */
   805			guest_exit();
   806			trace_kvm_exit(ret, kvm_vcpu_trap_get_class(vcpu), *vcpu_pc(vcpu));
   807	
   808			/* Exit types that need handling before we can be preempted */
   809			handle_exit_early(vcpu, run, ret);
   810	
   811			preempt_enable();
   812	
   813			ret = handle_exit(vcpu, run, ret);
   814		}
   815	
   816		/* Tell userspace about in-kernel device output levels */
   817		if (unlikely(!irqchip_in_kernel(vcpu->kvm))) {
   818			kvm_timer_update_run(vcpu);
   819			kvm_pmu_update_run(vcpu);
   820		}
   821	
   822		kvm_sigset_deactivate(vcpu);
   823	
   824		vcpu_put(vcpu);
   825		return ret;
   826	}
   827	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/gzip
Size: 19846 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180521/3cee3bbc/attachment-0001.gz>

^ permalink raw reply

* [kvmarm:queue 13/29] arch/arm/kvm/../../../virt/kvm/arm/arm.c:1598:6: error: implicit declaration of function 'system_supports_sve'
From: kbuild test robot @ 2018-05-20 22:12 UTC (permalink / raw)
  To: linux-arm-kernel

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/kvmarm/kvmarm.git queue
head:   f2d1aab22d57235b58db391d318727d3e5ef1e89
commit: 61d47b5d95db9a4ce12c50ffaa6918a40619984f [13/29] KVM: arm64: Save host SVE context as appropriate
config: arm-axm55xx_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        git checkout 61d47b5d95db9a4ce12c50ffaa6918a40619984f
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All errors (new ones prefixed by >>):

   arch/arm/kvm/../../../virt/kvm/arm/arm.c: In function 'kvm_arch_vcpu_ioctl_run':
   arch/arm/kvm/../../../virt/kvm/arm/arm.c:785:3: error: implicit declaration of function 'kvm_arch_vcpu_ctxsync_fp'; did you mean 'kvm_arch_vcpu_put_fp'? [-Werror=implicit-function-declaration]
      kvm_arch_vcpu_ctxsync_fp(vcpu);
      ^~~~~~~~~~~~~~~~~~~~~~~~
      kvm_arch_vcpu_put_fp
   arch/arm/kvm/../../../virt/kvm/arm/arm.c: In function 'kvm_arch_init':
>> arch/arm/kvm/../../../virt/kvm/arm/arm.c:1598:6: error: implicit declaration of function 'system_supports_sve' [-Werror=implicit-function-declaration]
     if (system_supports_sve() && !has_vhe()) {
         ^~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors

vim +/system_supports_sve +1598 arch/arm/kvm/../../../virt/kvm/arm/arm.c

  1564	
  1565	/**
  1566	 * Initialize Hyp-mode and memory mappings on all CPUs.
  1567	 */
  1568	int kvm_arch_init(void *opaque)
  1569	{
  1570		int err;
  1571		int ret, cpu;
  1572		bool in_hyp_mode;
  1573	
  1574		if (!is_hyp_mode_available()) {
  1575			kvm_info("HYP mode not available\n");
  1576			return -ENODEV;
  1577		}
  1578	
  1579		for_each_online_cpu(cpu) {
  1580			smp_call_function_single(cpu, check_kvm_target_cpu, &ret, 1);
  1581			if (ret < 0) {
  1582				kvm_err("Error, CPU %d not supported!\n", cpu);
  1583				return -ENODEV;
  1584			}
  1585		}
  1586	
  1587		/*
  1588		 * VHE is a prerequisite for SVE in the Arm architecture, and
  1589		 * Kconfig ensures that if system_supports_sve() here then
  1590		 * CONFIG_ARM64_VHE is enabled, so if VHE support wasn't already
  1591		 * detected and enabled, the CPU is architecturally
  1592		 * noncompliant.
  1593		 *
  1594		 * Just in case this mismatch is seen, detect it, warn and give
  1595		 * up.  Supporting this forbidden configuration in Hyp would be
  1596		 * pointless.
  1597		 */
> 1598		if (system_supports_sve() && !has_vhe()) {
  1599			kvm_pr_unimpl("SVE system without VHE unsupported.  Broken cpu?");
  1600			return -ENODEV;
  1601		}
  1602	
  1603		err = init_common_resources();
  1604		if (err)
  1605			return err;
  1606	
  1607		in_hyp_mode = is_kernel_in_hyp_mode();
  1608	
  1609		if (!in_hyp_mode) {
  1610			err = init_hyp_mode();
  1611			if (err)
  1612				goto out_err;
  1613		}
  1614	
  1615		err = init_subsystems();
  1616		if (err)
  1617			goto out_hyp;
  1618	
  1619		if (in_hyp_mode)
  1620			kvm_info("VHE mode initialized successfully\n");
  1621		else
  1622			kvm_info("Hyp mode initialized successfully\n");
  1623	
  1624		return 0;
  1625	
  1626	out_hyp:
  1627		if (!in_hyp_mode)
  1628			teardown_hyp_mode();
  1629	out_err:
  1630		return err;
  1631	}
  1632	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/gzip
Size: 19846 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180521/90c5c740/attachment-0001.gz>

^ permalink raw reply

* [PATCH 2/2] rtc: brcmstb-waketimer: add range
From: Alexandre Belloni @ 2018-05-20 20:27 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180520202730.28172-1-alexandre.belloni@bootlin.com>

Let the core handle the range.

Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
 drivers/rtc/rtc-brcmstb-waketimer.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/rtc/rtc-brcmstb-waketimer.c b/drivers/rtc/rtc-brcmstb-waketimer.c
index ba49d9bcff12..f4010a75f2be 100644
--- a/drivers/rtc/rtc-brcmstb-waketimer.c
+++ b/drivers/rtc/rtc-brcmstb-waketimer.c
@@ -145,9 +145,6 @@ static int brcmstb_waketmr_settime(struct device *dev,
 
 	sec = rtc_tm_to_time64(tm);
 
-	if (sec > U32_MAX || sec < 0)
-		return -EINVAL;
-
 	writel_relaxed(sec, timer->base + BRCMSTB_WKTMR_COUNTER);
 
 	return 0;
@@ -184,9 +181,6 @@ static int brcmstb_waketmr_setalarm(struct device *dev,
 	else
 		sec = 0;
 
-	if (sec > U32_MAX || sec < 0)
-		return -EINVAL;
-
 	brcmstb_waketmr_set_alarm(timer, sec);
 
 	return 0;
@@ -266,6 +260,7 @@ static int brcmstb_waketmr_probe(struct platform_device *pdev)
 	register_reboot_notifier(&timer->reboot_notifier);
 
 	timer->rtc->ops = &brcmstb_waketmr_ops;
+	timer->rtc->range_max = U32_MAX;
 
 	ret = rtc_register_device(timer->rtc);
 	if (ret) {
-- 
2.17.0

^ permalink raw reply related

* [PATCH 1/2] rtc: brcmstb-waketimer: switch to rtc_register_device
From: Alexandre Belloni @ 2018-05-20 20:27 UTC (permalink / raw)
  To: linux-arm-kernel

Switch to devm_rtc_allocate_device/rtc_register_device.

Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
 drivers/rtc/rtc-brcmstb-waketimer.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/rtc/rtc-brcmstb-waketimer.c b/drivers/rtc/rtc-brcmstb-waketimer.c
index bdd6674a1054..ba49d9bcff12 100644
--- a/drivers/rtc/rtc-brcmstb-waketimer.c
+++ b/drivers/rtc/rtc-brcmstb-waketimer.c
@@ -229,6 +229,10 @@ static int brcmstb_waketmr_probe(struct platform_device *pdev)
 	if (IS_ERR(timer->base))
 		return PTR_ERR(timer->base);
 
+	timer->rtc = devm_rtc_allocate_device(dev);
+	if (IS_ERR(timer->rtc))
+		return PTR_ERR(timer->rtc);
+
 	/*
 	 * Set wakeup capability before requesting wakeup interrupt, so we can
 	 * process boot-time "wakeups" (e.g., from S5 soft-off)
@@ -261,11 +265,11 @@ static int brcmstb_waketmr_probe(struct platform_device *pdev)
 	timer->reboot_notifier.notifier_call = brcmstb_waketmr_reboot;
 	register_reboot_notifier(&timer->reboot_notifier);
 
-	timer->rtc = rtc_device_register("brcmstb-waketmr", dev,
-					 &brcmstb_waketmr_ops, THIS_MODULE);
-	if (IS_ERR(timer->rtc)) {
+	timer->rtc->ops = &brcmstb_waketmr_ops;
+
+	ret = rtc_register_device(timer->rtc);
+	if (ret) {
 		dev_err(dev, "unable to register device\n");
-		ret = PTR_ERR(timer->rtc);
 		goto err_notifier;
 	}
 
@@ -288,7 +292,6 @@ static int brcmstb_waketmr_remove(struct platform_device *pdev)
 	struct brcmstb_waketmr *timer = dev_get_drvdata(&pdev->dev);
 
 	unregister_reboot_notifier(&timer->reboot_notifier);
-	rtc_device_unregister(timer->rtc);
 
 	return 0;
 }
-- 
2.17.0

^ permalink raw reply related

* [PATCH 2/6] Input: ams_delta_serio: use GPIO lookup table
From: Dmitry Torokhov @ 2018-05-20 20:17 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180518210954.29044-2-jmkrzyszt@gmail.com>

Hi Janusz,

On Fri, May 18, 2018 at 11:09:50PM +0200, Janusz Krzysztofik wrote:
> Now as the Amstrad Delta board provides GPIO lookup tables, switch from
> GPIO numbers to GPIO descriptors and use the table to locate required
> GPIO pins.
> 
> Declare static variables for storing GPIO descriptors and replace
> gpio_ functions with their gpiod_ equivalents.
> 
> Pin naming used by the driver should be followed while respective GPIO
> lookup table is initialized by a board init code.
> 
> Created and tested against linux-4.17-rc3, on top of patch 1/6 "ARM:
> OMAP1: ams-delta: add GPIO lookup tables"
> 
> Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
> ---
>  drivers/input/serio/ams_delta_serio.c | 98 +++++++++++++++++++----------------
>  1 file changed, 53 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/input/serio/ams_delta_serio.c b/drivers/input/serio/ams_delta_serio.c
> index 3df501c3421b..dd1f8f118c08 100644
> --- a/drivers/input/serio/ams_delta_serio.c
> +++ b/drivers/input/serio/ams_delta_serio.c
> @@ -20,14 +20,13 @@
>   * However, when used with the E3 mailboard that producecs non-standard
>   * scancodes, a custom key table must be prepared and loaded from userspace.
>   */
> -#include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>
>  #include <linux/irq.h>
>  #include <linux/serio.h>
>  #include <linux/slab.h>
>  #include <linux/module.h>
>  
>  #include <asm/mach-types.h>
> -#include <mach/board-ams-delta.h>
>  
>  #include <mach/ams-delta-fiq.h>
>  
> @@ -36,6 +35,10 @@ MODULE_DESCRIPTION("AMS Delta (E3) keyboard port driver");
>  MODULE_LICENSE("GPL");
>  
>  static struct serio *ams_delta_serio;
> +static struct gpio_desc *gpiod_data;
> +static struct gpio_desc *gpiod_clock;
> +static struct gpio_desc *gpiod_power;
> +static struct gpio_desc *gpiod_dataout;

Since you are doing the conversion: it does not appear that all these
are necessarily GPIOs; for example should not power be gpio-regulator
and data be simply expressed as IRQ resource? And the driver to be
converted into a platform driver?

I think this needs to be done first, because otherwise you are
committing to a certain binding and will have hard time changing it
later.

Thanks.

>  
>  static int check_data(int data)
>  {
> @@ -92,7 +95,7 @@ static irqreturn_t ams_delta_serio_interrupt(int irq, void *dev_id)
>  static int ams_delta_serio_open(struct serio *serio)
>  {
>  	/* enable keyboard */
> -	gpio_set_value(AMS_DELTA_GPIO_PIN_KEYBRD_PWR, 1);
> +	gpiod_set_value(gpiod_power, 1);
>  
>  	return 0;
>  }
> @@ -100,32 +103,9 @@ static int ams_delta_serio_open(struct serio *serio)
>  static void ams_delta_serio_close(struct serio *serio)
>  {
>  	/* disable keyboard */
> -	gpio_set_value(AMS_DELTA_GPIO_PIN_KEYBRD_PWR, 0);
> +	gpiod_set_value(gpiod_power, 0);
>  }
>  
> -static const struct gpio ams_delta_gpios[] __initconst_or_module = {
> -	{
> -		.gpio	= AMS_DELTA_GPIO_PIN_KEYBRD_DATA,
> -		.flags	= GPIOF_DIR_IN,
> -		.label	= "serio-data",
> -	},
> -	{
> -		.gpio	= AMS_DELTA_GPIO_PIN_KEYBRD_CLK,
> -		.flags	= GPIOF_DIR_IN,
> -		.label	= "serio-clock",
> -	},
> -	{
> -		.gpio	= AMS_DELTA_GPIO_PIN_KEYBRD_PWR,
> -		.flags	= GPIOF_OUT_INIT_LOW,
> -		.label	= "serio-power",
> -	},
> -	{
> -		.gpio	= AMS_DELTA_GPIO_PIN_KEYBRD_DATAOUT,
> -		.flags	= GPIOF_OUT_INIT_LOW,
> -		.label	= "serio-dataout",
> -	},
> -};
> -
>  static int __init ams_delta_serio_init(void)
>  {
>  	int err;
> @@ -145,36 +125,62 @@ static int __init ams_delta_serio_init(void)
>  	strlcpy(ams_delta_serio->phys, "GPIO/serio0",
>  			sizeof(ams_delta_serio->phys));
>  
> -	err = gpio_request_array(ams_delta_gpios,
> -				ARRAY_SIZE(ams_delta_gpios));
> -	if (err) {
> -		pr_err("ams_delta_serio: Couldn't request gpio pins\n");
> +	gpiod_data = gpiod_get(NULL, "data", GPIOD_IN);
> +	if (IS_ERR(gpiod_data)) {
> +		err = PTR_ERR(gpiod_data);
> +		pr_err("%s: 'data' GPIO request failed (%d)\n", __func__,
> +		       err);
>  		goto serio;
>  	}
> +	gpiod_clock = gpiod_get(NULL, "clock", GPIOD_IN);
> +	if (IS_ERR(gpiod_clock)) {
> +		err = PTR_ERR(gpiod_clock);
> +		pr_err("%s: 'clock' GPIO request failed (%d)\n", __func__,
> +		       err);
> +		goto gpio_data;
> +	}
> +	gpiod_power = gpiod_get(NULL, "power", GPIOD_OUT_LOW);
> +	if (IS_ERR(gpiod_power)) {
> +		err = PTR_ERR(gpiod_power);
> +		pr_err("%s: 'power' GPIO request failed (%d)\n", __func__,
> +		       err);
> +		goto gpio_clock;
> +	}
> +	gpiod_dataout = gpiod_get(NULL, "dataout", GPIOD_OUT_LOW);
> +	if (IS_ERR(gpiod_dataout)) {
> +		err = PTR_ERR(gpiod_dataout);
> +		pr_err("%s: 'dataout' GPIO request failed (%d)\n",
> +		       __func__, err);
> +		goto gpio_power;
> +	}
>  
> -	err = request_irq(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK),
> -			ams_delta_serio_interrupt, IRQ_TYPE_EDGE_RISING,
> -			"ams-delta-serio", 0);
> +	err = request_irq(gpiod_to_irq(gpiod_clock),
> +			  ams_delta_serio_interrupt, IRQ_TYPE_EDGE_RISING,
> +			  "ams-delta-serio", 0);
>  	if (err < 0) {
> -		pr_err("ams_delta_serio: couldn't request gpio interrupt %d\n",
> -				gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK));
> -		goto gpio;
> +		pr_err("%s: 'clock' GPIO interrupt request failed (%d)\n",
> +		       __func__, err);
> +		goto gpio_dataout;
>  	}
>  	/*
>  	 * Since GPIO register handling for keyboard clock pin is performed
>  	 * at FIQ level, switch back from edge to simple interrupt handler
>  	 * to avoid bad interaction.
>  	 */
> -	irq_set_handler(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK),
> -			handle_simple_irq);
> +	irq_set_handler(gpiod_to_irq(gpiod_clock), handle_simple_irq);
>  
>  	serio_register_port(ams_delta_serio);
>  	dev_info(&ams_delta_serio->dev, "%s\n", ams_delta_serio->name);
>  
>  	return 0;
> -gpio:
> -	gpio_free_array(ams_delta_gpios,
> -			ARRAY_SIZE(ams_delta_gpios));
> +gpio_dataout:
> +	gpiod_put(gpiod_dataout);
> +gpio_power:
> +	gpiod_put(gpiod_power);
> +gpio_clock:
> +	gpiod_put(gpiod_clock);
> +gpio_data:
> +	gpiod_put(gpiod_data);
>  serio:
>  	kfree(ams_delta_serio);
>  	return err;
> @@ -184,8 +190,10 @@ module_init(ams_delta_serio_init);
>  static void __exit ams_delta_serio_exit(void)
>  {
>  	serio_unregister_port(ams_delta_serio);
> -	free_irq(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK), 0);
> -	gpio_free_array(ams_delta_gpios,
> -			ARRAY_SIZE(ams_delta_gpios));
> +	free_irq(gpiod_to_irq(gpiod_clock), 0);
> +	gpiod_put(gpiod_dataout);
> +	gpiod_put(gpiod_power);
> +	gpiod_put(gpiod_clock);
> +	gpiod_put(gpiod_data);
>  }
>  module_exit(ams_delta_serio_exit);
> -- 
> 2.16.1
> 

-- 
Dmitry

^ permalink raw reply

* [alsa-devel] [PATCH 5/6] mtd: rawnand: ams-delta: use GPIO lookup table
From: Dmitry Torokhov @ 2018-05-20 20:08 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180520192705.GA12883@lenoch>

On Sun, May 20, 2018 at 09:27:05PM +0200, Ladislav Michl wrote:
> On Sat, May 19, 2018 at 11:55:51PM +0200, Janusz Krzysztofik wrote:
> > On Saturday, May 19, 2018 8:00:38 PM CEST Andy Shevchenko wrote:
> > > On Sat, May 19, 2018 at 2:15 AM, Janusz Krzysztofik <jmkrzyszt@gmail.com> 
> > wrote:
> > > > On Friday, May 18, 2018 11:21:14 PM CEST Andy Shevchenko wrote:
> > > >> On Sat, May 19, 2018 at 12:09 AM, Janusz Krzysztofik
> > > >> 
> > > >> <jmkrzyszt@gmail.com> wrote:
> > > >> > +       gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy",
> > > >> > GPIOD_IN);
> > > >> > +       if (!IS_ERR_OR_NULL(gpiod_rdy)) {
> > > >> 
> > > >> So, is it optional or not at the end?
> > > >> If it is, why do we check for NULL?
> > > > 
> > > > As far as I can understand, nand_chip->dev_ready() callback is optional.
> > > > That's why I decided to use the _optional variant of devm_gpiod_get(). In
> > > > case of ams-delta, the dev_ready() callback depends on availability of
> > > > the 'rdy' GPIO pin. As a consequence, I'm checking for both NULL and ERR
> > > > in order to decide if dev_ready() will be supported.
> > > > 
> > > > I can pretty well replace it with the standard form and check for ERR only
> > > > if the purpose of the _optional form is different.
> > > 
> > > NULL check in practice discards the _optional part of gpiod_get(). So,
> > > either you use non-optional variant and decide how to handle an
> > > errors, or user _optional w/o NULL check.
> > 
> > OK, I'm going to use something like the below while submitting v2:
> > 
> > -	gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy", GPIOD_IN);
> > -	if (!IS_ERR_OR_NULL(gpiod_rdy)) {
> > -		this->dev_ready = ams_delta_nand_ready;
> > -	} else {
> > -		this->dev_ready = NULL;
> > -		pr_notice("Couldn't request gpio for Delta NAND ready.\n");
> > +	priv->gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy",
> > +						  GPIOD_IN);
> > +	if (IS_ERR(priv->gpiod_rdy)) {
> > +		err = PTR_ERR(priv->gpiod_nwp);
> ??? --------------------------------^^^^^^^^^
> > +		dev_warn(&pdev->dev, "RDY GPIO request failed (%d)\n", err);
> > +		goto err_gpiod;
> 
> Driver will just use worst case delay instead of RDY signal, so this
> is perhaps too strict. I will work with degraded performance.

If RDY signal is not available then the board should not define it.
Degrading performance and having users wondering because RDY is
sometimes not available is not great. Especially if we get -EPROBE_DEFER
here.

Thanks.

-- 
Dmitry

^ permalink raw reply

* [alsa-devel] [PATCH 5/6] mtd: rawnand: ams-delta: use GPIO lookup table
From: Ladislav Michl @ 2018-05-20 19:27 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <5456625.lDWjtgZygK@z50>

On Sat, May 19, 2018 at 11:55:51PM +0200, Janusz Krzysztofik wrote:
> On Saturday, May 19, 2018 8:00:38 PM CEST Andy Shevchenko wrote:
> > On Sat, May 19, 2018 at 2:15 AM, Janusz Krzysztofik <jmkrzyszt@gmail.com> 
> wrote:
> > > On Friday, May 18, 2018 11:21:14 PM CEST Andy Shevchenko wrote:
> > >> On Sat, May 19, 2018 at 12:09 AM, Janusz Krzysztofik
> > >> 
> > >> <jmkrzyszt@gmail.com> wrote:
> > >> > +       gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy",
> > >> > GPIOD_IN);
> > >> > +       if (!IS_ERR_OR_NULL(gpiod_rdy)) {
> > >> 
> > >> So, is it optional or not at the end?
> > >> If it is, why do we check for NULL?
> > > 
> > > As far as I can understand, nand_chip->dev_ready() callback is optional.
> > > That's why I decided to use the _optional variant of devm_gpiod_get(). In
> > > case of ams-delta, the dev_ready() callback depends on availability of
> > > the 'rdy' GPIO pin. As a consequence, I'm checking for both NULL and ERR
> > > in order to decide if dev_ready() will be supported.
> > > 
> > > I can pretty well replace it with the standard form and check for ERR only
> > > if the purpose of the _optional form is different.
> > 
> > NULL check in practice discards the _optional part of gpiod_get(). So,
> > either you use non-optional variant and decide how to handle an
> > errors, or user _optional w/o NULL check.
> 
> OK, I'm going to use something like the below while submitting v2:
> 
> -	gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy", GPIOD_IN);
> -	if (!IS_ERR_OR_NULL(gpiod_rdy)) {
> -		this->dev_ready = ams_delta_nand_ready;
> -	} else {
> -		this->dev_ready = NULL;
> -		pr_notice("Couldn't request gpio for Delta NAND ready.\n");
> +	priv->gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy",
> +						  GPIOD_IN);
> +	if (IS_ERR(priv->gpiod_rdy)) {
> +		err = PTR_ERR(priv->gpiod_nwp);
??? --------------------------------^^^^^^^^^
> +		dev_warn(&pdev->dev, "RDY GPIO request failed (%d)\n", err);
> +		goto err_gpiod;

Driver will just use worst case delay instead of RDY signal, so this
is perhaps too strict. I will work with degraded performance.

	ladis

>  	}
>  
> +	if (priv->gpiod_rdy)
> +		this->dev_ready = ams_delta_nand_ready;
> 
> > 
> > >> > +err_gpiod:
> > >> > +       if (err == -ENODEV || err == -ENOENT)
> > >> > +               err = -EPROBE_DEFER;
> > >> 
> > >> Hmm...
> > > 
> > > Amstrad Delta uses gpio-mmio driver. Unfortunatelty that driver is not
> > > availble before device init phase, unlike other crucial GPIO drivers which
> > > are initialized earlier, e.g. during the postcore or at latetst the
> > > subsys phase. Hence, devices which depend on GPIO pins provided by
> > > gpio-mmio must either be declared late or fail softly so they get another
> > > chance of being probed succesfully.
> > > 
> > > I thought of replacing the gpio-mmio platform driver with bgpio functions
> > > it exports but for now I haven't implemented it, not even shared the
> > > idea.
> > > 
> > > Does it really hurt to return -EPROBE_DEFER if a GPIO pin can't be
> > > obtained?
> > I'm only concerned if it would be an infinite defer in the case when
> > driver will never appear.
> > But I don't remember the details.
> 
> Deferred probes are handled effectively during late_initcall, no risk of 
> infinite defer, see drivers/base/dd.c for details.
> 
> Thanks,
> Janusz
> 
> 
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel at alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply

* [PATCH 5/6] mtd: rawnand: ams-delta: use GPIO lookup table
From: Miquel Raynal @ 2018-05-20 17:25 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAHp75Vdp5A58JMTf8vaET84QEzzZ_T7eKLUYfWV9qpWt5ptYCA@mail.gmail.com>

Hello,

On Sun, 20 May 2018 19:17:04 +0300, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:

> >> Though, I completely dislike "rdy" name of GPIO. Where is it documented?  
> >
> > No documentation files for Amstrad Delta nor for its NAND driver specifically
> > exist under Documentation/. However, there exist some for generic GPIO NAND
> > driver where the pin name "rdy" is used explicitly:
> > Documentation/driver-api/gpio/drivers-on-gpio.rst
> > Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
> > You can find that mnemonic used across drivers/mtd/nand/, standalone or as a
> > suffix, including the Amstrad Delta NAND driver before the change discussed.  
> 
> > To be honest, I don't like it much either, but I'm just using it instead of
> > inventing something new.  
> 
> OK, that's what I was looking for. Since it's already in use and
> documented, then it's fine for me.

Do we actually have the possibility to rename this gpio? I guess no
since it would break DT backward compatibility. Otherwise it would have
been more descriptive to call it something like 'gpio-rb'.

Anyway, if you find the time, documentation for this controller would be
welcome!

Thanks,
Miqu?l

^ 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