Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 1/4] dt-bindings: arm: Add bindings for Mediatek MT8183 SoC Platform
From: Rob Herring @ 2018-05-22 17:14 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1526538126-51497-2-git-send-email-erin.lo@mediatek.com>

On Thu, May 17, 2018 at 02:22:03PM +0800, Erin Lo wrote:
> This adds dt-binding documentation of cpu for Mediatek MT8183.
> 
> Signed-off-by: Erin Lo <erin.lo@mediatek.com>
> ---
>  Documentation/devicetree/bindings/arm/mediatek.txt | 4 ++++
>  1 file changed, 4 insertions(+)

Reviewed-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* [PATCH v3 6/6] drm/rockchip: dw_hdmi: add dw-hdmi support for the rk3328
From: Rob Herring @ 2018-05-22 17:13 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180515134736.5824-7-heiko@sntech.de>

On Tue, May 15, 2018 at 03:47:36PM +0200, Heiko Stuebner wrote:
> The rk3328 uses a dw-hdmi controller with an external hdmi phy from
> Innosilicon which uses the generic phy framework for access.
> Add the necessary data and the compatible for the rk3328 to the
> rockchip dw-hdmi driver.
> 
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> Tested-by: Robin Murphy <robin.murphy@arm.com>
> ---
> changes in v3:
> - reword as suggested by Rob to show that it's a dw-hdmi + Inno phy
> 
>  .../display/rockchip/dw_hdmi-rockchip.txt     |   1 +

Acked-by: Rob Herring <robh@kernel.org>

>  drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c   | 106 ++++++++++++++++++
>  2 files changed, 107 insertions(+)

^ permalink raw reply

* [PATCH RT] arm64: fpsimd: use a local_lock() in addition to local_bh_disable()
From: Steven Rostedt @ 2018-05-22 17:10 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180517124006.ohygrrpg7z2moqqt@linutronix.de>

On Thu, 17 May 2018 14:40:06 +0200
Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:

> +static DEFINE_LOCAL_IRQ_LOCK(fpsimd_lock);
>  /*
>   * Update current's FPSIMD/SVE registers from thread_struct.
>   *
> @@ -594,6 +595,7 @@ int sve_set_vector_length(struct task_struct *task,
>  	 * non-SVE thread.
>  	 */
>  	if (task == current) {
> +		local_lock(fpsimd_lock);
>  		local_bh_disable();

I'm surprised that we don't have a "local_lock_bh()"?

-- Steve

>  
>  		task_fpsimd_save();

^ permalink raw reply

* [PATCH v6 3/9] docs: Add Generic Counter interface documentation
From: Jonathan Cameron @ 2018-05-22 17:08 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180521134729.GB5723@sophia>

...
> >> +
> >> +COUNT
> >> +-----
> >> +A Count represents the count data for a set of Signals. The Generic
> >> +Counter interface provides the following available count data types:
> >> +
> >> +* COUNT_POSITION_UNSIGNED:
> >> +  Unsigned integer value representing position.
> >> +
> >> +* COUNT_POSITION_SIGNED:
> >> +  Signed integer value representing position.  
> >
> >Just a thought: As the '0' position is effectively arbitrary is there any
> >actual difference between signed and unsigned? If we defined all counters
> >to be unsigned and just offset any signed ones so the range still fitted
> >would we end up with a simpler interface - there would be no real loss
> >of meaning that I can see..  I suppose it might not be what people expect
> >though when they see their counters start at a large positive value...  
> 
> This is something I've been on the fence about for a while. I would
> actually prefer the interface to have simply a COUNT_POSITION data type
> to represent position and leave it as unsigned; distinguishing between
> signed and unsigned position seems ultimately arbitrary given that it's
> mathematically just an offset as you have pointed out.
> 
> If we were to go down this route, then we'd have a count value that may
> always be represented using an unsigned data type, with an offset value
> that may always be represented using a signed data type -- the
> relationship being such: position = count + offset. Is that correct?

Given the positions are all arbitrary (such as limits etc) there is no
real need to have 0 as in anyway special.  So you could just apply an
offset to everything then don't make them visible to userspace at all.

> 
> My reason for giving the option for either signed or unsigned position
> was to help minimize the work userspace would have to do in order to get
> the value in which they're actually interested. I suppose it was a
> question of how abstract I want to make the interface -- although,
> making it simpler for userspace put more of a burden on the kernel side.
> 
> However, the "offset" value route may actually be more robust in the end
> because userspace would have to know whether they want a signed or
> unsigned position regardless in order to parse, so with count and offet
> defined we know they will always be unsigned and signed respectively.

Hmm. It's not obvious to me which is the best option.

> 
> >
> >
> >
> >  
> >> +
> >> +A Count has a count function mode which represents the update behavior
> >> +for the count data. The Generic Counter interface provides the following
> >> +available count function modes:
> >> +
> >> +* Increase:
> >> +  Accumulated count is incremented.
> >> +
> >> +* Decrease:
> >> +  Accumulated count is decremented.
> >> +
> >> +* Pulse-Direction:
> >> +  Rising edges on quadrature pair signal A updates the respective count.
> >> +  The input level of quadrature pair signal B determines direction.
> >> +  
> >Perhaps group the quadrature modes for the point of view of this document?
> >Might be slightly easier to read?  
> >
> >* Quadrature Modes
> >
> >  - x1 A: etc.
> >  
> >> +* Quadrature x1 A:
> >> +  If direction is forward, rising edges on quadrature pair signal A
> >> +  updates the respective count; if the direction is backward, falling
> >> +  edges on quadrature pair signal A updates the respective count.
> >> +  Quadrature encoding determines the direction.
> >> +
> >> +* Quadrature x1 B:
> >> +  If direction is forward, rising edges on quadrature pair signal B
> >> +  updates the respective count; if the direction is backward, falling
> >> +  edges on quadrature pair signal B updates the respective count.
> >> +  Quadrature encoding determines the direction.
> >> +
> >> +* Quadrature x2 A:
> >> +  Any state transition on quadrature pair signal A updates the
> >> +  respective count. Quadrature encoding determines the direction.
> >> +
> >> +* Quadrature x2 B:
> >> +  Any state transition on quadrature pair signal B updates the
> >> +  respective count. Quadrature encoding determines the direction.
> >> +
> >> +* Quadrature x2 Rising:
> >> +  Rising edges on either quadrature pair signals updates the respective
> >> +  count. Quadrature encoding determines the direction.  
> >
> >This one I've never met.  Really? There are devices who do this form
> >of crazy? It gives really uneven counting and I'm failing to see when
> >it would ever make sense...  References for these odd corner cases
> >would be good.
> >
> >
> >__|---|____|-----|____
> >____|----|____|-----|____
> >
> >001122222223334444444  
> 
> That's the same reaction I had when I discovered this -- in fact the
> STM32 LP Timer is the first time I've come across such a quadrature
> mode. I'm not sure of the use case for this mode, because positioning
> wouldn't be precise as you've pointed out. Perhaps Fabrice or Benjamin
> can probe the ST guys responsible for this design choice to figure out
> the rationale.

Hmm.  My inclination would be to not support it unless someone can up
with a meaningful use.  We are adding ABI (be it not much) for a case
that to us makes no sense.

Looks rather like the sort of thing that is a side effect of the
implementation rather than deliberate.

> 
> I'm leaving in these modes for now, as they do exist in the STM32 LP
> Timer, but it does make me curious what the intentions for them were
> (perhaps use cases outside of traditional quadrature encoder
> positioning).
> 

Sure if there is a usecase then fair enough.

Jonathan

^ permalink raw reply

* [PATCH v6 5/5] drm/rockchip: support dp training outside dp firmware
From: Enric Balletbo Serra @ 2018-05-22 17:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAFqH_50Gn7SuzJo2mDjGhsKC6_H=xMVKCbr2BM82geAyZaDu+A@mail.gmail.com>

Lin,

2018-05-22 9:41 GMT+02:00 Enric Balletbo Serra <eballetbo@gmail.com>:
> Hi Lin
>
> 2018-05-22 3:08 GMT+02:00 hl <hl@rock-chips.com>:
>> Hi Enric,
>>
>>
>>
>>
>> On Monday, May 21, 2018 11:22 PM, Enric Balletbo Serra wrote:
>>>
>>> Hi Lin,
>>>
>>> 2018-05-21 11:37 GMT+02:00 Lin Huang <hl@rock-chips.com>:
>>>>
>>>> DP firmware uses fixed phy config values to do training, but some
>>>> boards need to adjust these values to fit for their unique hardware
>>>> design. So get phy config values from dts and use software link training
>>>> instead of relying on firmware, if software training fail, keep firmware
>>>> training as a fallback if sw training fails.
>>>>
>>>> Signed-off-by: Chris Zhong <zyw@rock-chips.com>
>>>> Signed-off-by: Lin Huang <hl@rock-chips.com>
>>>> Reviewed-by: Sean Paul <seanpaul@chromium.org>
>>>> ---
>>>> Changes in v2:
>>>> - update patch following Enric suggest
>>>> Changes in v3:
>>>> - use variable fw_training instead sw_training_success
>>>> - base on DP SPCE, if training fail use lower link rate to retry training
>>>> Changes in v4:
>>>> - improve cdn_dp_get_lower_link_rate() and cdn_dp_software_train_link()
>>>> follow Sean suggest
>>>> Changes in v5:
>>>> - fix some whitespcae issue
>>>> Changes in v6:
>>>> - None
>>>>
>>>>   drivers/gpu/drm/rockchip/Makefile               |   3 +-
>>>>   drivers/gpu/drm/rockchip/cdn-dp-core.c          |  24 +-
>>>>   drivers/gpu/drm/rockchip/cdn-dp-core.h          |   2 +
>>>>   drivers/gpu/drm/rockchip/cdn-dp-link-training.c | 420
>>>> ++++++++++++++++++++++++
>>>>   drivers/gpu/drm/rockchip/cdn-dp-reg.c           |  31 +-
>>>>   drivers/gpu/drm/rockchip/cdn-dp-reg.h           |  38 ++-
>>>>   6 files changed, 505 insertions(+), 13 deletions(-)
>>>>   create mode 100644 drivers/gpu/drm/rockchip/cdn-dp-link-training.c
>>>>
>>>> diff --git a/drivers/gpu/drm/rockchip/Makefile
>>>> b/drivers/gpu/drm/rockchip/Makefile
>>>> index a314e21..b932f62 100644
>>>> --- a/drivers/gpu/drm/rockchip/Makefile
>>>> +++ b/drivers/gpu/drm/rockchip/Makefile
>>>> @@ -9,7 +9,8 @@ rockchipdrm-y := rockchip_drm_drv.o rockchip_drm_fb.o \
>>>>   rockchipdrm-$(CONFIG_DRM_FBDEV_EMULATION) += rockchip_drm_fbdev.o
>>>>
>>>>   rockchipdrm-$(CONFIG_ROCKCHIP_ANALOGIX_DP) += analogix_dp-rockchip.o
>>>> -rockchipdrm-$(CONFIG_ROCKCHIP_CDN_DP) += cdn-dp-core.o cdn-dp-reg.o
>>>> +rockchipdrm-$(CONFIG_ROCKCHIP_CDN_DP) += cdn-dp-core.o cdn-dp-reg.o \
>>>> +                                       cdn-dp-link-training.o
>>>>   rockchipdrm-$(CONFIG_ROCKCHIP_DW_HDMI) += dw_hdmi-rockchip.o
>>>>   rockchipdrm-$(CONFIG_ROCKCHIP_DW_MIPI_DSI) += dw-mipi-dsi.o
>>>>   rockchipdrm-$(CONFIG_ROCKCHIP_INNO_HDMI) += inno_hdmi.o
>>>> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c
>>>> b/drivers/gpu/drm/rockchip/cdn-dp-core.c
>>>> index cce64c1..d9d0d4d 100644
>>>> --- a/drivers/gpu/drm/rockchip/cdn-dp-core.c
>>>> +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c
>>>> @@ -629,11 +629,13 @@ static void cdn_dp_encoder_enable(struct
>>>> drm_encoder *encoder)
>>>>                          goto out;
>>>>                  }
>>>>          }
>>>> -
>>>> -       ret = cdn_dp_set_video_status(dp, CONTROL_VIDEO_IDLE);
>>>> -       if (ret) {
>>>> -               DRM_DEV_ERROR(dp->dev, "Failed to idle video %d\n", ret);
>>>> -               goto out;
>>>> +       if (dp->use_fw_training == true) {
>>>
>>> You don't need to compare to true. Simply do:
>>>
>>> if (dp->use_fw_training)
>>>
>>>> +               ret = cdn_dp_set_video_status(dp, CONTROL_VIDEO_IDLE);
>>>> +               if (ret) {
>>>> +                       DRM_DEV_ERROR(dp->dev,
>>>> +                                     "Failed to idle video %d\n", ret);
>>>> +                       goto out;
>>>> +               }
>>>>          }
>>>>
>>>>          ret = cdn_dp_config_video(dp);
>>>> @@ -642,11 +644,15 @@ static void cdn_dp_encoder_enable(struct
>>>> drm_encoder *encoder)
>>>>                  goto out;
>>>>          }
>>>>
>>>> -       ret = cdn_dp_set_video_status(dp, CONTROL_VIDEO_VALID);
>>>> -       if (ret) {
>>>> -               DRM_DEV_ERROR(dp->dev, "Failed to valid video %d\n",
>>>> ret);
>>>> -               goto out;
>>>> +       if (dp->use_fw_training == true) {
>>>
>>> if (dp->use_fw_training)
>>>
>>>> +               ret = cdn_dp_set_video_status(dp, CONTROL_VIDEO_VALID);
>>>> +               if (ret) {
>>>> +                       DRM_DEV_ERROR(dp->dev,
>>>> +                               "Failed to valid video %d\n", ret);
>>>> +                       goto out;
>>>> +               }
>>>>          }
>>>> +
>>>>   out:
>>>>          mutex_unlock(&dp->lock);
>>>>   }
>>>> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.h
>>>> b/drivers/gpu/drm/rockchip/cdn-dp-core.h
>>>> index 46159b2..77a9793 100644
>>>> --- a/drivers/gpu/drm/rockchip/cdn-dp-core.h
>>>> +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.h
>>>> @@ -84,6 +84,7 @@ struct cdn_dp_device {
>>>>          bool connected;
>>>>          bool active;
>>>>          bool suspended;
>>>> +       bool use_fw_training;
>>>>
>>>>          const struct firmware *fw;      /* cdn dp firmware */
>>>>          unsigned int fw_version;        /* cdn fw version */
>>>> @@ -106,6 +107,7 @@ struct cdn_dp_device {
>>>>          u8 ports;
>>>>          u8 lanes;
>>>>          int active_port;
>>>> +       u8 train_set[4];
>>>>
>>>>          u8 dpcd[DP_RECEIVER_CAP_SIZE];
>>>>          bool sink_has_audio;
>>>> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-link-training.c
>>>> b/drivers/gpu/drm/rockchip/cdn-dp-link-training.c
>>>> new file mode 100644
>>>> index 0000000..73c3290
>>>> --- /dev/null
>>>> +++ b/drivers/gpu/drm/rockchip/cdn-dp-link-training.c
>>>> @@ -0,0 +1,420 @@
>>>> +// SPDX-License-Identifier: GPL-2.0
>>>> +/*
>>>> + * Copyright (C) Fuzhou Rockchip Electronics Co.Ltd
>>>> + * Author: Chris Zhong <zyw@rock-chips.com>
>>>> + */

Oh, I just noticed that this is not the preferred format for .c files,
see the discussion here

https://lkml.org/lkml/2017/11/25/133

>>>> +
>>>> +#include <linux/device.h>
>>>> +#include <linux/delay.h>
>>>> +#include <linux/phy/phy.h>
>>>> +#include <soc/rockchip/rockchip_phy_typec.h>
>>>> +
>>>> +#include "cdn-dp-core.h"
>>>> +#include "cdn-dp-reg.h"
>>>> +
>>>> +static void cdn_dp_set_signal_levels(struct cdn_dp_device *dp)
>>>> +{
>>>> +       struct cdn_dp_port *port = dp->port[dp->active_port];
>>>> +       struct rockchip_typec_phy *tcphy = phy_get_drvdata(port->phy);
>>>> +
>>>> +       int rate = drm_dp_bw_code_to_link_rate(dp->link.rate);
>>>> +       u8 swing = (dp->train_set[0] & DP_TRAIN_VOLTAGE_SWING_MASK) >>
>>>> +                  DP_TRAIN_VOLTAGE_SWING_SHIFT;
>>>> +       u8 pre_emphasis = (dp->train_set[0] & DP_TRAIN_PRE_EMPHASIS_MASK)
>>>> +                         >> DP_TRAIN_PRE_EMPHASIS_SHIFT;
>>>> +
>>>> +       tcphy->typec_phy_config(port->phy, rate, dp->link.num_lanes,
>>>> +                               swing, pre_emphasis);
>>>> +}
>>>> +
>>>> +static int cdn_dp_set_pattern(struct cdn_dp_device *dp, uint8_t
>>>> dp_train_pat)
>>>> +{
>>>> +       u32 phy_config, global_config;
>>>> +       int ret;
>>>> +       uint8_t pattern = dp_train_pat & DP_TRAINING_PATTERN_MASK;
>>>> +
>>>> +       global_config = NUM_LANES(dp->link.num_lanes - 1) | SST_MODE |
>>>> +                       GLOBAL_EN | RG_EN | ENC_RST_DIS | WR_VHSYNC_FALL;
>>>> +
>>>> +       phy_config = DP_TX_PHY_ENCODER_BYPASS(0) |
>>>> +                    DP_TX_PHY_SKEW_BYPASS(0) |
>>>> +                    DP_TX_PHY_DISPARITY_RST(0) |
>>>> +                    DP_TX_PHY_LANE0_SKEW(0) |
>>>> +                    DP_TX_PHY_LANE1_SKEW(1) |
>>>> +                    DP_TX_PHY_LANE2_SKEW(2) |
>>>> +                    DP_TX_PHY_LANE3_SKEW(3) |
>>>> +                    DP_TX_PHY_10BIT_ENABLE(0);
>>>> +
>>>> +       if (pattern != DP_TRAINING_PATTERN_DISABLE) {
>>>> +               global_config |= NO_VIDEO;
>>>> +               phy_config |= DP_TX_PHY_TRAINING_ENABLE(1) |
>>>> +                             DP_TX_PHY_SCRAMBLER_BYPASS(1) |
>>>> +                             DP_TX_PHY_TRAINING_PATTERN(pattern);
>>>> +       }
>>>> +
>>>> +       ret = cdn_dp_reg_write(dp, DP_FRAMER_GLOBAL_CONFIG,
>>>> global_config);
>>>> +       if (ret) {
>>>> +               DRM_ERROR("fail to set DP_FRAMER_GLOBAL_CONFIG, error:
>>>> %d\n",
>>>> +                         ret);
>>>> +               return ret;
>>>> +       }
>>>> +
>>>> +       ret = cdn_dp_reg_write(dp, DP_TX_PHY_CONFIG_REG, phy_config);
>>>> +       if (ret) {
>>>> +               DRM_ERROR("fail to set DP_TX_PHY_CONFIG_REG, error:
>>>> %d\n",
>>>> +                         ret);
>>>> +               return ret;
>>>> +       }
>>>> +
>>>> +       ret = cdn_dp_reg_write(dp, DPTX_LANE_EN, BIT(dp->link.num_lanes)
>>>> - 1);
>>>> +       if (ret) {
>>>> +               DRM_ERROR("fail to set DPTX_LANE_EN, error: %d\n", ret);
>>>> +               return ret;
>>>> +       }
>>>> +
>>>> +       if (drm_dp_enhanced_frame_cap(dp->dpcd))
>>>> +               ret = cdn_dp_reg_write(dp, DPTX_ENHNCD, 1);
>>>> +       else
>>>> +               ret = cdn_dp_reg_write(dp, DPTX_ENHNCD, 0);
>>>> +       if (ret)
>>>> +               DRM_ERROR("failed to set DPTX_ENHNCD, error: %x\n", ret);
>>>> +
>>>> +       return ret;
>>>> +}
>>>> +
>>>> +static u8 cdn_dp_pre_emphasis_max(u8 voltage_swing)
>>>> +{
>>>> +       switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
>>>> +       case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
>>>> +               return DP_TRAIN_PRE_EMPH_LEVEL_3;
>>>> +       case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
>>>> +               return DP_TRAIN_PRE_EMPH_LEVEL_2;
>>>> +       case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
>>>> +               return DP_TRAIN_PRE_EMPH_LEVEL_1;
>>>> +       default:
>>>> +               return DP_TRAIN_PRE_EMPH_LEVEL_0;
>>>> +       }
>>>> +}
>>>> +
>>>> +static void cdn_dp_get_adjust_train(struct cdn_dp_device *dp,
>>>> +                                   uint8_t
>>>> link_status[DP_LINK_STATUS_SIZE])
>>>> +{
>>>> +       int i;
>>>> +       uint8_t v = 0, p = 0;
>>>> +       uint8_t preemph_max;
>>>> +
>>>> +       for (i = 0; i < dp->link.num_lanes; i++) {
>>>> +               v = max(v, drm_dp_get_adjust_request_voltage(link_status,
>>>> i));
>>>> +               p = max(p,
>>>> drm_dp_get_adjust_request_pre_emphasis(link_status,
>>>> +                                                                 i));
>>>> +       }
>>>> +
>>>> +       if (v >= VOLTAGE_LEVEL_2)
>>>> +               v = VOLTAGE_LEVEL_2 | DP_TRAIN_MAX_SWING_REACHED;
>>>> +
>>>> +       preemph_max = cdn_dp_pre_emphasis_max(v);
>>>> +       if (p >= preemph_max)
>>>> +               p = preemph_max | DP_TRAIN_MAX_PRE_EMPHASIS_REACHED;
>>>> +
>>>> +       for (i = 0; i < dp->link.num_lanes; i++)
>>>> +               dp->train_set[i] = v | p;
>>>> +}
>>>> +
>>>> +/*
>>>> + * Pick training pattern for channel equalization. Training Pattern 3
>>>> for HBR2
>>>> + * or 1.2 devices that support it, Training Pattern 2 otherwise.
>>>> + */
>>>> +static u32 cdn_dp_select_chaneq_pattern(struct cdn_dp_device *dp)
>>>> +{
>>>> +       u32 training_pattern = DP_TRAINING_PATTERN_2;
>>>> +
>>>> +       /*
>>>> +        * cdn dp support HBR2 also support TPS3. TPS3 support is also
>>>> mandatory
>>>> +        * for downstream devices that support HBR2. However, not all
>>>> sinks
>>>> +        * follow the spec.
>>>> +        */
>>>> +       if (drm_dp_tps3_supported(dp->dpcd))
>>>> +               training_pattern = DP_TRAINING_PATTERN_3;
>>>> +       else
>>>> +               DRM_DEBUG_KMS("5.4 Gbps link rate without sink TPS3
>>>> support\n");
>>>> +
>>>> +       return training_pattern;
>>>> +}
>>>> +
>>>> +
>>>> +static bool cdn_dp_link_max_vswing_reached(struct cdn_dp_device *dp)
>>>> +{
>>>> +       int lane;
>>>> +
>>>> +       for (lane = 0; lane < dp->link.num_lanes; lane++)
>>>> +               if ((dp->train_set[lane] & DP_TRAIN_MAX_SWING_REACHED) ==
>>>> 0)
>>>> +                       return false;
>>>> +
>>>> +       return true;
>>>> +}
>>>> +
>>>> +static int cdn_dp_update_link_train(struct cdn_dp_device *dp)
>>>> +{
>>>> +       int ret;
>>>> +
>>>> +       cdn_dp_set_signal_levels(dp);
>>>> +
>>>> +       ret = drm_dp_dpcd_write(&dp->aux, DP_TRAINING_LANE0_SET,
>>>> +                               dp->train_set, dp->link.num_lanes);
>>>> +       if (ret != dp->link.num_lanes)
>>>> +               return -EINVAL;
>>>> +
>>>> +       return 0;
>>>> +}
>>>> +
>>>> +static int cdn_dp_set_link_train(struct cdn_dp_device *dp,
>>>> +                                 uint8_t dp_train_pat)
>>>> +{
>>>> +       uint8_t buf[sizeof(dp->train_set) + 1];
>>>> +       int ret, len;
>>>> +
>>>> +       buf[0] = dp_train_pat;
>>>> +       if ((dp_train_pat & DP_TRAINING_PATTERN_MASK) ==
>>>> +           DP_TRAINING_PATTERN_DISABLE) {
>>>> +               /* don't write DP_TRAINING_LANEx_SET on disable */
>>>> +               len = 1;
>>>> +       } else {
>>>> +               /* DP_TRAINING_LANEx_SET follow DP_TRAINING_PATTERN_SET
>>>> */
>>>> +               memcpy(buf + 1, dp->train_set, dp->link.num_lanes);
>>>> +               len = dp->link.num_lanes + 1;
>>>> +       }
>>>> +
>>>> +       ret = drm_dp_dpcd_write(&dp->aux, DP_TRAINING_PATTERN_SET,
>>>> +                               buf, len);
>>>> +       if (ret != len)
>>>> +               return -EINVAL;
>>>> +
>>>> +       return 0;
>>>> +}
>>>> +
>>>> +static int cdn_dp_reset_link_train(struct cdn_dp_device *dp,
>>>> +                                   uint8_t dp_train_pat)
>>>> +{
>>>> +       int ret;
>>>> +
>>>> +       memset(dp->train_set, 0, sizeof(dp->train_set));
>>>> +
>>>> +       cdn_dp_set_signal_levels(dp);
>>>> +
>>>> +       ret = cdn_dp_set_pattern(dp, dp_train_pat);
>>>> +       if (ret)
>>>> +               return ret;
>>>> +
>>>> +       return cdn_dp_set_link_train(dp, dp_train_pat);
>>>> +}
>>>> +
>>>> +/* Enable corresponding port and start training pattern 1 */
>>>> +static int cdn_dp_link_training_clock_recovery(struct cdn_dp_device *dp)
>>>> +{
>>>> +       u8 voltage;
>>>> +       u8 link_status[DP_LINK_STATUS_SIZE];
>>>> +       u32 voltage_tries, max_vswing_tries;
>>>> +       int ret;
>>>> +
>>>> +       /* clock recovery */
>>>> +       ret = cdn_dp_reset_link_train(dp, DP_TRAINING_PATTERN_1 |
>>>> +                                         DP_LINK_SCRAMBLING_DISABLE);
>>>> +       if (ret) {
>>>> +               DRM_ERROR("failed to start link train\n");
>>>> +               return ret;
>>>> +       }
>>>> +
>>>> +       voltage_tries = 1;
>>>> +       max_vswing_tries = 0;
>>>> +       for (;;) {
>>>> +               drm_dp_link_train_clock_recovery_delay(dp->dpcd);
>>>> +               if (drm_dp_dpcd_read_link_status(&dp->aux, link_status)
>>>> !=
>>>> +                   DP_LINK_STATUS_SIZE) {
>>>> +                       DRM_ERROR("failed to get link status\n");
>>>> +                       return -EINVAL;
>>>> +               }
>>>> +
>>>> +               if (drm_dp_clock_recovery_ok(link_status,
>>>> dp->link.num_lanes)) {
>>>> +                       DRM_DEBUG_KMS("clock recovery OK\n");
>>>> +                       return 0;
>>>> +               }
>>>> +
>>>> +               if (voltage_tries >= 5) {
>>>> +                       DRM_DEBUG_KMS("Same voltage tried 5 times\n");
>>>> +                       return -EINVAL;
>>>> +               }
>>>> +
>>>> +               if (max_vswing_tries >= 1) {
>>>> +                       DRM_DEBUG_KMS("Max Voltage Swing reached\n");
>>>> +                       return -EINVAL;
>>>> +               }
>>>> +
>>>> +               voltage = dp->train_set[0] & DP_TRAIN_VOLTAGE_SWING_MASK;
>>>> +
>>>> +               /* Update training set as requested by target */
>>>> +               cdn_dp_get_adjust_train(dp, link_status);
>>>> +               if (cdn_dp_update_link_train(dp)) {
>>>> +                       DRM_ERROR("failed to update link training\n");
>>>> +                       return -EINVAL;
>>>> +               }
>>>> +
>>>> +               if ((dp->train_set[0] & DP_TRAIN_VOLTAGE_SWING_MASK) ==
>>>> +                   voltage)
>>>> +                       ++voltage_tries;
>>>> +               else
>>>> +                       voltage_tries = 1;
>>>> +
>>>> +               if (cdn_dp_link_max_vswing_reached(dp))
>>>> +                       ++max_vswing_tries;
>>>> +       }
>>>> +}
>>>> +
>>>> +static int cdn_dp_link_training_channel_equalization(struct
>>>> cdn_dp_device *dp)
>>>> +{
>>>> +       int tries, ret;
>>>> +       u32 training_pattern;
>>>> +       uint8_t link_status[DP_LINK_STATUS_SIZE];
>>>> +
>>>> +       training_pattern = cdn_dp_select_chaneq_pattern(dp);
>>>> +       training_pattern |= DP_LINK_SCRAMBLING_DISABLE;
>>>> +
>>>> +       ret = cdn_dp_set_pattern(dp, training_pattern);
>>>> +       if (ret)
>>>> +               return ret;
>>>> +
>>>> +       ret = cdn_dp_set_link_train(dp, training_pattern);
>>>> +       if (ret) {
>>>> +               DRM_ERROR("failed to start channel equalization\n");
>>>> +               return ret;
>>>> +       }
>>>> +
>>>> +       for (tries = 0; tries < 5; tries++) {
>>>> +               drm_dp_link_train_channel_eq_delay(dp->dpcd);
>>>> +               if (drm_dp_dpcd_read_link_status(&dp->aux, link_status)
>>>> !=
>>>> +                   DP_LINK_STATUS_SIZE) {
>>>> +                       DRM_ERROR("failed to get link status\n");
>>>> +                       break;
>>>> +               }
>>>> +
>>>> +               /* Make sure clock is still ok */
>>>> +               if (!drm_dp_clock_recovery_ok(link_status,
>>>> +                                             dp->link.num_lanes)) {
>>>> +                       DRM_DEBUG_KMS("Clock recovery check failed\n");
>>>> +                       break;
>>>> +               }
>>>> +
>>>> +               if (drm_dp_channel_eq_ok(link_status,
>>>> dp->link.num_lanes)) {
>>>> +                       DRM_DEBUG_KMS("Channel EQ done\n");
>>>> +                       return 0;
>>>> +               }
>>>> +
>>>> +               /* Update training set as requested by target */
>>>> +               cdn_dp_get_adjust_train(dp, link_status);
>>>> +               if (cdn_dp_update_link_train(dp)) {
>>>> +                       DRM_ERROR("failed to update link training\n");
>>>> +                       break;
>>>> +               }
>>>> +       }
>>>> +
>>>> +       /* Try 5 times, else fail and try at lower BW */
>>>> +       if (tries == 5)
>>>> +               DRM_DEBUG_KMS("Channel equalization failed 5 times\n");
>>>> +
>>>> +       return -EINVAL;
>>>> +}
>>>> +
>>>> +static int cdn_dp_stop_link_train(struct cdn_dp_device *dp)
>>>> +{
>>>> +       int ret = cdn_dp_set_pattern(dp, DP_TRAINING_PATTERN_DISABLE);
>>>> +
>>>> +       if (ret)
>>>> +               return ret;
>>>> +
>>>> +       return cdn_dp_set_link_train(dp, DP_TRAINING_PATTERN_DISABLE);
>>>> +}
>>>> +
>>>> +static int cdn_dp_get_lower_link_rate(struct cdn_dp_device *dp)
>>>> +{
>>>> +       switch (dp->link.rate) {
>>>> +       case DP_LINK_BW_1_62:
>>>> +               return -EINVAL;
>>>> +       case DP_LINK_BW_2_7:
>>>> +               dp->link.rate = DP_LINK_BW_1_62;
>>>> +               break;
>>>> +       case DP_LINK_BW_5_4:
>>>> +               dp->link.rate = DP_LINK_BW_2_7;
>>>> +               break;
>>>> +       default:
>>>> +               dp->link.rate = DP_LINK_BW_5_4;
>>>> +               break;
>>>> +       }
>>>> +
>>>> +       return 0;
>>>> +}
>>>> +
>>>> +int cdn_dp_software_train_link(struct cdn_dp_device *dp)
>>>> +{
>>>> +       int ret, stop_err;
>>>> +       u8 link_config[2];
>>>> +       u32 rate, sink_max, source_max;
>>>> +
>>>> +       ret = drm_dp_dpcd_read(&dp->aux, DP_DPCD_REV, dp->dpcd,
>>>> +                              sizeof(dp->dpcd));
>>>> +       if (ret < 0) {
>>>> +               DRM_DEV_ERROR(dp->dev, "Failed to get caps %d\n", ret);
>>>> +               return ret;
>>>> +       }
>>>> +
>>>> +       source_max = dp->lanes;
>>>> +       sink_max = drm_dp_max_lane_count(dp->dpcd);
>>>> +       dp->link.num_lanes = min(source_max, sink_max);
>>>> +
>>>> +       source_max = drm_dp_bw_code_to_link_rate(CDN_DP_MAX_LINK_RATE);
>>>> +       sink_max = drm_dp_max_link_rate(dp->dpcd);
>>>> +       rate = min(source_max, sink_max);
>>>> +       dp->link.rate = drm_dp_link_rate_to_bw_code(rate);
>>>> +
>>>> +       link_config[0] = 0;
>>>> +       link_config[1] = 0;
>>>> +       if (dp->dpcd[DP_MAIN_LINK_CHANNEL_CODING] & 0x01)
>>>> +               link_config[1] = DP_SET_ANSI_8B10B;
>>>> +       drm_dp_dpcd_write(&dp->aux, DP_DOWNSPREAD_CTRL, link_config, 2);
>>>> +
>>>> +       while (true) {
>>>> +
>>>> +               /* Write the link configuration data */
>>>> +               link_config[0] = dp->link.rate;
>>>> +               link_config[1] = dp->link.num_lanes;
>>>> +               if (drm_dp_enhanced_frame_cap(dp->dpcd))
>>>> +                       link_config[1] |=
>>>> DP_LANE_COUNT_ENHANCED_FRAME_EN;
>>>> +               drm_dp_dpcd_write(&dp->aux, DP_LINK_BW_SET, link_config,
>>>> 2);
>>>> +
>>>> +               ret = cdn_dp_link_training_clock_recovery(dp);
>>>> +               if (ret) {
>>>> +                       if (!cdn_dp_get_lower_link_rate(dp))
>>>> +                               continue;
>>>> +
>>>> +                       DRM_ERROR("training clock recovery failed: %d\n",
>>>> ret);
>>>> +                       break;
>>>> +               }
>>>> +
>>>> +               ret = cdn_dp_link_training_channel_equalization(dp);
>>>> +               if (ret) {
>>>> +                       if (!cdn_dp_get_lower_link_rate(dp))
>>>> +                               continue;
>>>> +
>>>> +                       DRM_ERROR("training channel eq failed: %d\n",
>>>> ret);
>>>> +                       break;
>>>> +               }
>>>> +
>>>> +               break;
>>>> +       }
>>>> +
>>>> +       stop_err = cdn_dp_stop_link_train(dp);
>>>> +       if (stop_err) {
>>>> +               DRM_ERROR("stop training fail, error: %d\n", stop_err);
>>>> +               return stop_err;
>>>> +       }
>>>> +
>>>> +       return ret;
>>>> +}
>>>> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-reg.c
>>>> b/drivers/gpu/drm/rockchip/cdn-dp-reg.c
>>>> index 979355d..e1273e6 100644
>>>> --- a/drivers/gpu/drm/rockchip/cdn-dp-reg.c
>>>> +++ b/drivers/gpu/drm/rockchip/cdn-dp-reg.c
>>>> @@ -17,7 +17,9 @@
>>>>   #include <linux/delay.h>
>>>>   #include <linux/io.h>
>>>>   #include <linux/iopoll.h>
>>>> +#include <linux/phy/phy.h>
>>>>   #include <linux/reset.h>
>>>> +#include <soc/rockchip/rockchip_phy_typec.h>
>>>>
>>>>   #include "cdn-dp-core.h"
>>>>   #include "cdn-dp-reg.h"
>>>> @@ -189,7 +191,7 @@ static int cdn_dp_mailbox_send(struct cdn_dp_device
>>>> *dp, u8 module_id,
>>>>          return 0;
>>>>   }
>>>>
>>>> -static int cdn_dp_reg_write(struct cdn_dp_device *dp, u16 addr, u32 val)
>>>> +int cdn_dp_reg_write(struct cdn_dp_device *dp, u16 addr, u32 val)
>>>>   {
>>>>          u8 msg[6];
>>>>
>>>> @@ -609,6 +611,31 @@ int cdn_dp_train_link(struct cdn_dp_device *dp)
>>>>   {
>>>>          int ret;
>>>>
>>>> +       /*
>>>> +        * DP firmware uses fixed phy config values to do training, but
>>>> some
>>>> +        * boards need to adjust these values to fit for their unique
>>>> hardware
>>>> +        * design. So if the phy is using custom config values, do
>>>> software
>>>> +        * link training instead of relying on firmware, if software
>>>> training
>>>> +        * fail, keep firmware training as a fallback if sw training
>>>> fails.
>>>> +        */
>>>> +       ret = cdn_dp_software_train_link(dp);
>>>> +       if (ret) {
>>>> +               DRM_DEV_ERROR(dp->dev,
>>>> +                       "Failed to do software training %d\n", ret);
>>>> +               goto do_fw_training;
>>>> +       }
>>>
>>> If I understand correctly you are changing current behavior. Before
>>> this patch, we use always firmware link training, and after this
>>> patch, we always use software link training. If fails we use the
>>> firmware link training.
>>>
>>> AFAIK my Samsung Chromebook Plus works well with firmware link
>>> training, so there are any benefits of use software link training
>>> instead of firmware link training?
>>>
>>> Looks to me that we should only do software link training on these
>>> platforms that need it, so on those that define the phy-config
>>> property in their DT and use by default firmware link training.
>>
>> Sean and me have discussed about that, and we all agree to use sw training
>> instead the
>> fw training to keep training process consistent, and we do not need add a
>> varialbe in
>> struct rockchip_typec_phy(like use_sw_training before) to distinguish sw and
>> fw training.
>> If training process implement correctly, sw training not different with fw
>> training, and as my
>> test so far, sw training work well on Kevin, ofcourse, we need keep testing
>> it.
>>
>
> Ok, I can also confirm that software training works well on kevin.
> Could you mention this change of behavior in the commit message? I
> think that after these patches the firmware training is unlikely to
> happen.
>
> Thanks,
>  Enric
>
>
>>>> +       ret = cdn_dp_reg_write(dp, SOURCE_HDTX_CAR, 0xf);
>>>> +       if (ret) {
>>>> +               DRM_DEV_ERROR(dp->dev,
>>>> +               "Failed to write SOURCE_HDTX_CAR register %d\n", ret);
>>>> +               goto do_fw_training;
>>>> +       }
>>>> +       dp->use_fw_training = false;
>>>> +       return 0;
>>>> +
>>>> +do_fw_training:
>>>> +       dp->use_fw_training = true;
>>>> +       DRM_DEV_DEBUG_KMS(dp->dev, "use fw training\n");
>>>>          ret = cdn_dp_training_start(dp);
>>>>          if (ret) {
>>>>                  DRM_DEV_ERROR(dp->dev, "Failed to start training %d\n",
>>>> ret);
>>>> @@ -623,7 +650,7 @@ int cdn_dp_train_link(struct cdn_dp_device *dp)
>>>>
>>>>          DRM_DEV_DEBUG_KMS(dp->dev, "rate:0x%x, lanes:%d\n",
>>>> dp->link.rate,
>>>>                            dp->link.num_lanes);
>>>> -       return ret;
>>>> +       return 0;
>>>>   }
>>>>
>>>>   int cdn_dp_set_video_status(struct cdn_dp_device *dp, int active)
>>>> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-reg.h
>>>> b/drivers/gpu/drm/rockchip/cdn-dp-reg.h
>>>> index 6580b11..3420771 100644
>>>> --- a/drivers/gpu/drm/rockchip/cdn-dp-reg.h
>>>> +++ b/drivers/gpu/drm/rockchip/cdn-dp-reg.h
>>>> @@ -137,7 +137,7 @@
>>>>   #define HPD_EVENT_MASK                 0x211c
>>>>   #define HPD_EVENT_DET                  0x2120
>>>>
>>>> -/* dpyx framer addr */
>>>> +/* dptx framer addr */
>>>>   #define DP_FRAMER_GLOBAL_CONFIG                0x2200
>>>>   #define DP_SW_RESET                    0x2204
>>>>   #define DP_FRAMER_TU                   0x2208
>>>> @@ -431,6 +431,40 @@
>>>>   /* Reference cycles when using lane clock as reference */
>>>>   #define LANE_REF_CYC                           0x8000
>>>>
>>>> +/* register CM_VID_CTRL */
>>>> +#define LANE_VID_REF_CYC(x)                    (((x) & (BIT(24) - 1)) <<
>>>> 0)
>>>> +#define NMVID_MEAS_TOLERANCE(x)                        (((x) & 0xf) <<
>>>> 24)
>>>> +
>>>> +/* register DP_TX_PHY_CONFIG_REG */
>>>> +#define DP_TX_PHY_TRAINING_ENABLE(x)           ((x) & 1)
>>>> +#define DP_TX_PHY_TRAINING_TYPE_PRBS7          (0 << 1)
>>>> +#define DP_TX_PHY_TRAINING_TYPE_TPS1           (1 << 1)
>>>> +#define DP_TX_PHY_TRAINING_TYPE_TPS2           (2 << 1)
>>>> +#define DP_TX_PHY_TRAINING_TYPE_TPS3           (3 << 1)
>>>> +#define DP_TX_PHY_TRAINING_TYPE_TPS4           (4 << 1)
>>>> +#define DP_TX_PHY_TRAINING_TYPE_PLTPAT         (5 << 1)
>>>> +#define DP_TX_PHY_TRAINING_TYPE_D10_2          (6 << 1)
>>>> +#define DP_TX_PHY_TRAINING_TYPE_HBR2CPAT       (8 << 1)
>>>> +#define DP_TX_PHY_TRAINING_PATTERN(x)          ((x) << 1)
>>>> +#define DP_TX_PHY_SCRAMBLER_BYPASS(x)          (((x) & 1) << 5)
>>>> +#define DP_TX_PHY_ENCODER_BYPASS(x)            (((x) & 1) << 6)
>>>> +#define DP_TX_PHY_SKEW_BYPASS(x)               (((x) & 1) << 7)
>>>> +#define DP_TX_PHY_DISPARITY_RST(x)             (((x) & 1) << 8)
>>>> +#define DP_TX_PHY_LANE0_SKEW(x)                (((x) & 7) << 9)
>>>> +#define DP_TX_PHY_LANE1_SKEW(x)                (((x) & 7) << 12)
>>>> +#define DP_TX_PHY_LANE2_SKEW(x)                (((x) & 7) << 15)
>>>> +#define DP_TX_PHY_LANE3_SKEW(x)                (((x) & 7) << 18)
>>>> +#define DP_TX_PHY_10BIT_ENABLE(x)              (((x) & 1) << 21)
>>>> +
>>>> +/* register DP_FRAMER_GLOBAL_CONFIG */
>>>> +#define NUM_LANES(x)           ((x) & 3)
>>>> +#define SST_MODE               (0 << 2)
>>>> +#define RG_EN                  (0 << 4)
>>>> +#define GLOBAL_EN              BIT(3)
>>>> +#define NO_VIDEO               BIT(5)
>>>> +#define ENC_RST_DIS            BIT(6)
>>>> +#define WR_VHSYNC_FALL         BIT(7)
>>>> +
>>>>   enum voltage_swing_level {
>>>>          VOLTAGE_LEVEL_0,
>>>>          VOLTAGE_LEVEL_1,
>>>> @@ -476,6 +510,7 @@ int cdn_dp_set_host_cap(struct cdn_dp_device *dp, u8
>>>> lanes, bool flip);
>>>>   int cdn_dp_event_config(struct cdn_dp_device *dp);
>>>>   u32 cdn_dp_get_event(struct cdn_dp_device *dp);
>>>>   int cdn_dp_get_hpd_status(struct cdn_dp_device *dp);
>>>> +int cdn_dp_reg_write(struct cdn_dp_device *dp, u16 addr, u32 val);
>>>>   ssize_t cdn_dp_dpcd_write(struct cdn_dp_device *dp, u32 addr,
>>>>                            u8 *data, u16 len);
>>>>   ssize_t cdn_dp_dpcd_read(struct cdn_dp_device *dp, u32 addr,
>>>> @@ -489,4 +524,5 @@ int cdn_dp_config_video(struct cdn_dp_device *dp);
>>>>   int cdn_dp_audio_stop(struct cdn_dp_device *dp, struct audio_info
>>>> *audio);
>>>>   int cdn_dp_audio_mute(struct cdn_dp_device *dp, bool enable);
>>>>   int cdn_dp_audio_config(struct cdn_dp_device *dp, struct audio_info
>>>> *audio);
>>>> +int cdn_dp_software_train_link(struct cdn_dp_device *dp);
>>>>   #endif /* _CDN_DP_REG_H */
>>>> --
>>>> 2.7.4
>>>>
>>>
>>>
>>
>>

^ permalink raw reply

* [PATCH 1/2] arm64: make is_permission_fault() name clearer
From: Will Deacon @ 2018-05-22 17:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180521131451.41040-2-mark.rutland@arm.com>

On Mon, May 21, 2018 at 02:14:50PM +0100, Mark Rutland wrote:
> The naming of is_permission_fault() makes it sound like it should return
> true for permission faults from EL0, but by design, it only does so for
> faults from EL1.
> 
> Let's make this clear by dropping el1 in the name, as we do for
> is_el1_instruction_abort().
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> ---
>  arch/arm64/mm/fault.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)

Acked-by: Will Deacon <will.deacon@arm.com>

Will

^ permalink raw reply

* [PATCH] arm64: dts: specify 1.8V EMMC capabilities for bcm958742t
From: Scott Branden @ 2018-05-22 17:01 UTC (permalink / raw)
  To: linux-arm-kernel

Specify 1.8V EMMC capabilities for bcm958742t board to indicate support
for UHS mode.

Fixes: d4b4aba6be8a ("arm64: dts: Initial DTS files for Broadcom Stingray SOC")
Signed-off-by: Scott Branden <scott.branden@broadcom.com>
---
 arch/arm64/boot/dts/broadcom/stingray/bcm958742t.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/boot/dts/broadcom/stingray/bcm958742t.dts b/arch/arm64/boot/dts/broadcom/stingray/bcm958742t.dts
index 6a4d19e..dd9de6b 100644
--- a/arch/arm64/boot/dts/broadcom/stingray/bcm958742t.dts
+++ b/arch/arm64/boot/dts/broadcom/stingray/bcm958742t.dts
@@ -44,3 +44,7 @@
 &gphy0 {
 	enet-phy-lane-swap;
 };
+
+&sdio0 {
+	mmc-ddr-1_8v;
+};
-- 
2.5.0

^ permalink raw reply related

* [PATCH v3 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings
From: Mark Brown @ 2018-05-22 16:55 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAD=FV=XARtQNo5Cyg78XuT26JUFgn=7BjWRnXPjP566=a-sF1w@mail.gmail.com>

On Tue, May 22, 2018 at 09:43:02AM -0700, Doug Anderson wrote:
> On Mon, May 21, 2018 at 5:00 PM, David Collins <collinsd@codeaurora.org> wrote:

> > Returning the cached (but not sent) initial voltage equal to the min
> > constraint voltage in get_voltage() calls should not cause any problems.
> > This represents the highest voltage that is guaranteed to be output by the
> > regulator.  Consumer's should call regulator_set_voltage() to specify
> > their voltage needs.  If they simply call regulator_enable(), then the
> > cached voltage will be sent along with the enable request.

> I'm still not seeing the argument for initial-voltage here.  If we
> added a feature like you describe where we don't send the voltage
> until the first enable couldn't we use the minimum voltage here?  If a
> consumer calls regulator_enable() without setting a voltage (which
> seems like a terrible idea for something where the voltage could vary)
> then it would end up at the minimum voltage.

Or if something else has already set a voltage...  though shared voltage
varying regulators aren't a superb idea they do occasionally happen.

> >> I was asking for specific examples.  Do you have any?

> > I was able to track down an example that requires initialization at a
> > non-minimum voltage: PM8998 LDO 13 on SDM845 MTP.  This regulator supplies
> > the microSD card with a voltage in the range 1.8 V to 2.96 V.  The boot
> > loader leaves this regulator enabled at 2.96 V.  It is only guaranteed to
> > be safe to reduce the voltage to 1.8 V on UHS type cards and only after
> > following the proper SD identification and command sequence.

> Ironically, this is also a perfect example of why we _shouldn't_ have
> an "initial-voltage" specified in the device tree.  It is certainly
> plausible to imagine a bootloader that might enable UHS speeds on an
> SD card and leave the rail on at 1.8V.  Having an initial-voltage
> specified in the device tree would be a bad idea here because it's
> even worse to transition to 2.96V when the card was expecting 1.8V.

Right, this sort of situation is why the regulator API has a policy of
leaving things untouched unless it was specifically told to do
something.

> I suppose this is a theoretical example since (as far as I know) no
> bootloaders run the micro SD card at UHS speeds, but it is still

kexec is the most obvious example I can think of here.  You could
probably arrange for something to patch the device tree that's provided
to the kexeced kernel to tell it about the current state but we don't
currently do anything there.

> OK, so how's this for a proposal then:

> 1. For RPMh-regulator whenever we see a "set voltage" but Linux hasn't
> specified that the regulator is enabled then we don't send the
> voltage, we just cache it.

> 2. When we see the first enable then we first send the cached voltage
> and then do the enable.

> 3. We don't need an "initial voltage" because any rails that are
> expected to be variable voltage the client should be choosing a
> voltage.

That seems relatively safe.

> You can even come up with a less contrived use case here.  One could
> argue that the SD card framework really ought to be ensuring VMMC and
> VQMMC are off before it starts tweaking with them just in case the
> bootloader left them on.  Thus, it should do:

> A) Turn off VMMC and VQMMC
> B) Program VMMC and VQMMC to defaults
> C) Turn on VMMC and VQMMC

> ...right now we bootup and pretend to Linux that VMMC and VQMMC start
> off, so step A) will be no-op.  Sigh.

> Do we need to have ".is_enabled" return -ENOTRECOVERABLE to help the
> regulator framework understand that we don't know the state?  I think

Yes, we should be doing that.  Then subsystems where it's an issue can
explicitly turn off the supply.

> that might require a pile of patches to the regulator framework, but
> it can't be helped unless we can somehow get RPMh to give us back the
> status of how the bootloader left us (if we had that, we could return
> 0 for anything the bootloader didn't touch and that would be correct
> from the point of view of the AP).

I think it's fine from a framework point of view, just provide an
is_enabled() operation which returns the error.  The required fiddling
in the core should be fairly minor.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180522/c6e35bbf/attachment-0001.sig>

^ permalink raw reply

* [PATCH v2 03/40] iommu/sva: Manage process address spaces
From: Jacob Pan @ 2018-05-22 16:43 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <de478769-9f7a-d40b-a55e-e2c63ad883e8@arm.com>

On Thu, 17 May 2018 11:02:42 +0100
Jean-Philippe Brucker <jean-philippe.brucker@arm.com> wrote:

> On 17/05/18 00:31, Jacob Pan wrote:
> > On Fri, 11 May 2018 20:06:04 +0100
> > I am a little confused about domain vs. pasid relationship. If
> > each domain represents a address space, should there be a domain for
> > each pasid?  
> 
> I don't think there is a formal definition, but from previous
> discussion the consensus seems to be: domains are a collection of
> devices that have the same virtual address spaces (one or many).
> 
> Keeping that definition makes things easier, in my opinion. Some time
> ago, I did try to represent PASIDs using "subdomains" (introducing a
> hierarchy of struct iommu_domain), but it required invasive changes in
> the IOMMU subsystem and probably all over the tree.
> 
> You do need some kind of "root domain" for each device, so that
> "iommu_get_domain_for_dev()" still makes sense. That root domain
> doesn't have a single address space but a collection of subdomains.
> If you need this anyway, representing a PASID with an iommu_domain
> doesn't seem preferable than using a different structure (io_mm),
> because they don't have anything in common.
> 
My main concern is the PASID table storage. If PASID table storage
is tied to domain, it is ok to scale up, i.e. multiple devices in a
domain share a single PASID table. But to scale down, e.g. further
partition device with VFIO mdev for assignment, each mdev may get its
own domain via vfio. But there is no IOMMU storage for PASID table at
mdev device level. Perhaps we do need root domain or some parent-child
relationship to locate PASID table.

> Thanks,
> Jean

[Jacob Pan]

^ permalink raw reply

* [PATCH v3 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings
From: Doug Anderson @ 2018-05-22 16:43 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <fb390ccb-927a-8449-05c3-8dcac964bfae@codeaurora.org>

Hi,

On Mon, May 21, 2018 at 5:00 PM, David Collins <collinsd@codeaurora.org> wrote:
> On 05/21/2018 11:01 AM, Doug Anderson wrote:
>> On Fri, May 18, 2018 at 5:46 PM, David Collins <collinsd@codeaurora.org> wrote:
> ...
>>> Something to keep in mind about the downstream rpmh-regulator driver is
>>> that it caches the initial voltages specified in device tree and only
>>> sends them after a consumer driver makes a regulator framework call. This
>>> saves time during boot and ensures that requests are not made for
>>> regulators that no Linux consumer cares about.
>>
>> You're saying that in the downstream driver you'd specify
>> "initial-voltage" in the device tree and:
>>
>> * This voltage would be reported by any subsequent get_voltage() calls
>>
>> * This voltage would _not_ be communicated to RPMh.
>>
>> That seems really strange because you're essentially going to be
>> returning something from get_voltage() that could be a lie.  You don't
>> know if the BIOS actually set the value or not but you'll claim that
>> it did.  It also doesn't seem to match what I see in the downstream
>> driver.  There I see it read "qcom,init-voltage" and then do a
>> "rpmh_regulator_set_reg()".  Thus my reading of the downstream driver
>> is that it should do the same requests that you're doing.
>
> In the downstream rpmh-regulator driver [1], the value specified via the
> "qcom,init-voltage" DT property is only sent to RPMh at probe time if the
> "qcom,send-defaults" property is also specified.  "qcom,send-defaults" is
> currently specified only for PMI8998 BOB.  The rpmh_regulator_set_reg()
> function only updates the cached RPMh request value to be sent in the next
> request.  The rpmh_regulator_send_aggregate_requests() function must be
> called to actually issue the request.

Got it.  This wasn't in the "snapshot" of the downstream driver that I saw.


> Returning the cached (but not sent) initial voltage equal to the min
> constraint voltage in get_voltage() calls should not cause any problems.
> This represents the highest voltage that is guaranteed to be output by the
> regulator.  Consumer's should call regulator_set_voltage() to specify
> their voltage needs.  If they simply call regulator_enable(), then the
> cached voltage will be sent along with the enable request.

I'm still not seeing the argument for initial-voltage here.  If we
added a feature like you describe where we don't send the voltage
until the first enable couldn't we use the minimum voltage here?  If a
consumer calls regulator_enable() without setting a voltage (which
seems like a terrible idea for something where the voltage could vary)
then it would end up at the minimum voltage.


>>> It is generally not safe to request all regulators to be set to the
>>> minimum allowed voltage.  Special care will be needed with the upstream
>>> qcom-rpmh-regulator driver to avoid disrupting the boot up state of
>>> regulators that are needed by other subsystems.  Therefore, I would like
>>> to keep the initial voltage feature supported.
>>
>> I was asking for specific examples.  Do you have any?
>
> I was able to track down an example that requires initialization at a
> non-minimum voltage: PM8998 LDO 13 on SDM845 MTP.  This regulator supplies
> the microSD card with a voltage in the range 1.8 V to 2.96 V.  The boot
> loader leaves this regulator enabled at 2.96 V.  It is only guaranteed to
> be safe to reduce the voltage to 1.8 V on UHS type cards and only after
> following the proper SD identification and command sequence.

Ironically, this is also a perfect example of why we _shouldn't_ have
an "initial-voltage" specified in the device tree.  It is certainly
plausible to imagine a bootloader that might enable UHS speeds on an
SD card and leave the rail on at 1.8V.  Having an initial-voltage
specified in the device tree would be a bad idea here because it's
even worse to transition to 2.96V when the card was expecting 1.8V.

I suppose this is a theoretical example since (as far as I know) no
bootloaders run the micro SD card at UHS speeds, but it is still
worrying that the kernel needs to have a hardcoded initial voltage in
it.  ...basically whenever we're playing with the voltage at bootup
before a regulator has been claimed it's not amazingly safe.
Generally Linux doesn't need to worry about this because it will only
play with voltages if they are out of the constrained range, but in
Qualcomm's case where we can't read the existing voltages then we get
badness.


>> BTW: have I already said how terrible of a design it is that you can't
>> read back the voltages that the BIOS set?  If we could just read back
>> what the BIOS set then everything would work great and we could stop
>> talking about this.
>
> Even if such reading were feasible, it would not help the situation
> substantially.  Very few requests are made via the AP RSC before Linux
> kernel boot, so 0 V values would still be read back for most regulators.

Sure, but all the regulators we're talking about are ones where this
would help.  Said another way: are there any rails that are not
touched by the bootloader where it's bad to set the minimum voltage?


OK, so how's this for a proposal then:

1. For RPMh-regulator whenever we see a "set voltage" but Linux hasn't
specified that the regulator is enabled then we don't send the
voltage, we just cache it.

2. When we see the first enable then we first send the cached voltage
and then do the enable.

3. We don't need an "initial voltage" because any rails that are
expected to be variable voltage the client should be choosing a
voltage.


...taking the SD card case as an example: if the regulator framework
says "set to the minimum: 1.8V" we'll cache this but not apply it yet
because the rail is off as far as Linux is concerned.  Then when the
SD card framework starts up it will set the voltage to 3.3V which will
overwrite the cache.  Then the SD card framework will enable the
regulator and RPMh will set the voltage to 3.3V and enable.


This whole thing makes me worry about another problem, though.  You
say that the bootloader left the SD card rail on, right?  ...but as
far as Linux is concerned the SD card rail is off (argh, it can't read
the state that the bootloader left the rail in!)

...now imagine any of the following:

* The user boots up a kernel where the SD card driver is disabled.

* The user ejects the SD card right after the bootloader used it but
before the kernel driver started.

When the kernel comes up it will believe that the SD card rail is
disabled so it won't try to disable it.  So the voltage will be left
on.


You can even come up with a less contrived use case here.  One could
argue that the SD card framework really ought to be ensuring VMMC and
VQMMC are off before it starts tweaking with them just in case the
bootloader left them on.  Thus, it should do:

A) Turn off VMMC and VQMMC
B) Program VMMC and VQMMC to defaults
C) Turn on VMMC and VQMMC

...right now we bootup and pretend to Linux that VMMC and VQMMC start
off, so step A) will be no-op.  Sigh.


Do we need to have ".is_enabled" return -ENOTRECOVERABLE to help the
regulator framework understand that we don't know the state?  I think
that might require a pile of patches to the regulator framework, but
it can't be helped unless we can somehow get RPMh to give us back the
status of how the bootloader left us (if we had that, we could return
0 for anything the bootloader didn't touch and that would be correct
from the point of view of the AP).


-Doug

^ permalink raw reply

* [PATCH 1/2] regulator: add QCOM RPMh regulator driver
From: Mark Brown @ 2018-05-22 16:36 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <6cb2ac52-258c-332b-e912-809d16e14114@codeaurora.org>

On Thu, May 17, 2018 at 01:48:41PM -0700, David Collins wrote:

> The RPMh hardware is configured by the boot loader.  The configuration
> does reflect reality; however, it cannot handle all configurations at
> initialization time.  Specific headroom management typically comes up in
> modem usecases for RF supplies that are sensitive to noise.  This feature

...

> If you really don't like having this feature present in the Linux RPMh
> regulator driver, then I'd be ok removing it.  It is not required for
> SDM845 which the driver is initially targeting.

It's certainly going to be easier to review it separately.

> >> In the case of XOB managed LDO regulators, the LDOs physically can be
> >> configured to different voltages by the bootloader.  However, the RPMh
> >> interface provides no mechanism for the application processor to read or
> >> change that voltage.  Therefore, we need a way to specify such voltages in
> >> a board specific (as opposed to driver specific) manner (i.e. device tree).

> > Is the kernel somehow prevented from varying these voltages?

> Yes.  Physically, there exists no RPMh register to read or write the
> voltage of LDOs managed via XOB.  Additionally, the kernel running on the
> application processor is blocked from configuring the voltage via a direct
> SPMI writes by access permissions that crash the system when violated.

*sigh*  Please provide feedback on the problems this (and everything
else) is causing to the team working on the firmware.  The number of
issues with this interface compared to anything else we've seen is
really noticable, I see what it's trying to do with providing something
like the regulator API is doing but there's quite a lot of missing steps
in it which cause no end of problems for general purpose software
written on top of it.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180522/56c284ba/attachment-0001.sig>

^ permalink raw reply

* [PATCH v4 1/3] dt-bindings: media: rcar-vin: Add R8A77995 support
From: Rob Herring @ 2018-05-22 16:34 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1526913942-15426-2-git-send-email-jacopo+renesas@jmondi.org>

On Mon, May 21, 2018 at 04:45:40PM +0200, Jacopo Mondi wrote:
> Add compatible string for R-Car D3 R8A7795 to list of SoCs supported by
> rcar-vin driver.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> Acked-by: Niklas S?derlund <niklas.soderlund+renesas@ragnatech.se>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
> ---
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 1 +
>  1 file changed, 1 insertion(+)

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* Clang arm64 build is broken
From: Andrey Konovalov @ 2018-05-22 16:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAKwvOdkedxmstRytbWeYU8Pj9ahgWOe7rODJJ8BDtRzUroQeXw@mail.gmail.com>

On Mon, May 14, 2018 at 6:24 PM, Nick Desaulniers
<ndesaulniers@google.com> wrote:
> On Fri, Apr 20, 2018 at 7:59 AM Andrey Konovalov <andreyknvl@google.com>
> wrote:
>> On Fri, Apr 20, 2018 at 10:13 AM, Marc Zyngier <marc.zyngier@arm.com>
> wrote:
>> >> The issue is that
>> >> clang doesn't know about the "S" asm constraint. I reported this to
>> >> clang [2], and hopefully this will get fixed. In the meantime, would
>> >> it possible to work around using the "S" constraint in the kernel?
>> >
>> > I have no idea, I've never used clang to build the kernel. Clang isn't
>> > really supported to build the arm64 kernel anyway (as you mention
>> > below), and working around clang deficiencies would mean that we leave
>> > with the workaround forever. I'd rather enable clang once it is at
>> > feature parity with GCC.
>
>> The fact that there are some existing issues with building arm64
>> kernel with clang doesn't sound like a good justification for adding
>> new issues :)
>
>> However in this case I do believe that this is more of a bug in clang
>> that should be fixed.
>
> Just to follow up with this thread;
>
> Support for "S" constraints is being (re-)added to Clang in:
> https://reviews.llvm.org/D46745

Hi Nick!

I can confirm that the latest clang (which includes this patch) is
able to build the kernel with CONFIG_KVM enabled.

Thanks!

^ permalink raw reply

* [PATCH 10/14] arm64: ssbd: Add prctl interface for per-thread mitigation
From: Marc Zyngier @ 2018-05-22 16:30 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180522154842.pjml5lagtpyvwwqo@isilmar-4.linta.de>

On Tue, 22 May 2018 16:48:42 +0100,
Dominik Brodowski wrote:
> 
> 
> On Tue, May 22, 2018 at 04:06:44PM +0100, Marc Zyngier wrote:
> > If running on a system that performs dynamic SSBD mitigation, allow
> > userspace to request the mitigation for itself. This is implemented
> > as a prctl call, allowing the mitigation to be enabled or disabled at
> > will for this particular thread.
> > 
> > Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> > ---
> >  arch/arm64/kernel/Makefile |   1 +
> >  arch/arm64/kernel/ssbd.c   | 107 +++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 108 insertions(+)
> >  create mode 100644 arch/arm64/kernel/ssbd.c
> > 
> > diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
> > index bf825f38d206..0025f8691046 100644
> > --- a/arch/arm64/kernel/Makefile
> > +++ b/arch/arm64/kernel/Makefile
> > @@ -54,6 +54,7 @@ arm64-obj-$(CONFIG_ARM64_RELOC_TEST)	+= arm64-reloc-test.o
> >  arm64-reloc-test-y := reloc_test_core.o reloc_test_syms.o
> >  arm64-obj-$(CONFIG_CRASH_DUMP)		+= crash_dump.o
> >  arm64-obj-$(CONFIG_ARM_SDE_INTERFACE)	+= sdei.o
> > +arm64-obj-$(CONFIG_ARM64_SSBD)		+= ssbd.o
> >  
> >  obj-y					+= $(arm64-obj-y) vdso/ probes/
> >  obj-m					+= $(arm64-obj-m)
> > diff --git a/arch/arm64/kernel/ssbd.c b/arch/arm64/kernel/ssbd.c
> > new file mode 100644
> > index 000000000000..34e3c430176b
> > --- /dev/null
> > +++ b/arch/arm64/kernel/ssbd.c
> > @@ -0,0 +1,107 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2018 ARM Ltd, All Rights Reserved.
> > + */
> > +
> > +#include <linux/sched.h>
> > +#include <linux/thread_info.h>
> > +
> > +#include <asm/cpufeature.h>
> > +
> > +/*
> > + * prctl interface for SSBD
> > + * FIXME: Drop the below ifdefery once the common interface has been merged.
> > + */
> > +#ifdef PR_SPEC_STORE_BYPASS
> 
> That FIXME wants to be looked at.

It is what allowed the series to compile with mainline until last
night. Once I rebase on top of -rc7, I'll remove it.

Thanks,

	M.

-- 
Jazz is not dead, it just smell funny.

^ permalink raw reply

* [PATCH v5 1/4] dt-bindings: pinctrl: qcom: add gpio-ranges, gpio-reserved-ranges
From: Rob Herring @ 2018-05-22 16:29 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <910e5a85a6a8020069996e2ff397c93e9c5fe18c.1526935804.git.chunkeey@gmail.com>

On Mon, May 21, 2018 at 10:57:36PM +0200, Christian Lamparter wrote:
> This patch adds the gpio-ranges and gpio-reserved-ranges property
> definitions to the binding text files supported by the pinctrl-msm
> driver framework.
> 
> gpio-ranges:
> For DT-based platforms the pinctrl-msm framework currently relies
> on the deprecated-for-DT gpiochip_add_pin_range() function to add
> the range of GPIOs to be handled by the pin controller. Due to
> interactions within gpiolib code, this causes the pinctrl-msm
> driver to bail out (-517) during boot when a gpio-hog is declared.
> This can be fatal and cause the system to not boot or reset
> (for a detailed explanation and call-trace, refer to patch:
> "pinctrl: msm: fix gpio-hog related boot issues" in this series).
> 
> gpio-reserved-ranges:
> The binding has been added as a precaution since the TrustZone
> firmware (aka QSEE), which is running as the hypervisor, might
> have reserved certain, but undisclosed pins. Hence reading or
> writing to the registers for those pins will cause an
> XPU violation and this subsequently crashes the kernel.
> 
> Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
> ---
>  .../bindings/pinctrl/qcom,apq8064-pinctrl.txt         |  6 ++++++
>  .../bindings/pinctrl/qcom,apq8084-pinctrl.txt         | 11 +++++++++++
>  .../bindings/pinctrl/qcom,ipq4019-pinctrl.txt         |  6 ++++++
>  .../bindings/pinctrl/qcom,ipq8064-pinctrl.txt         |  6 ++++++
>  .../bindings/pinctrl/qcom,ipq8074-pinctrl.txt         | 10 ++++++++++
>  .../bindings/pinctrl/qcom,mdm9615-pinctrl.txt         | 11 +++++++++++
>  .../bindings/pinctrl/qcom,msm8660-pinctrl.txt         |  6 ++++++
>  .../bindings/pinctrl/qcom,msm8916-pinctrl.txt         | 11 +++++++++++
>  .../bindings/pinctrl/qcom,msm8960-pinctrl.txt         | 11 +++++++++++
>  .../bindings/pinctrl/qcom,msm8974-pinctrl.txt         |  6 ++++++
>  .../bindings/pinctrl/qcom,msm8994-pinctrl.txt         | 11 +++++++++++
>  .../bindings/pinctrl/qcom,msm8996-pinctrl.txt         | 11 +++++++++++
>  12 files changed, 106 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,apq8064-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/qcom,apq8064-pinctrl.txt
> index a752a4716486..7f78c6bb4e35 100644
> --- a/Documentation/devicetree/bindings/pinctrl/qcom,apq8064-pinctrl.txt
> +++ b/Documentation/devicetree/bindings/pinctrl/qcom,apq8064-pinctrl.txt
> @@ -10,6 +10,11 @@ Required properties:
>  - #gpio-cells : Should be two.
>                  The first cell is the gpio pin number and the
>                  second cell is used for optional parameters.
> +- gpio-ranges: Range of pins managed by the GPIO controller.

Just 'see gpio.txt' is sufficient unless you can say how many entries.

> +
> +Optional properties:
> +
> +- gpio-reserved-ranges: Range of pins reserved by the TrustZone TEE.

ditto.

>  
>  Please refer to ../gpio/gpio.txt and ../interrupt-controller/interrupts.txt for
>  a general description of GPIO and interrupt bindings.
> @@ -67,6 +72,7 @@ Example:
>  
>  		pinctrl-names = "default";
>  		pinctrl-0 = <&gsbi5_uart_default>;
> +		gpio-ranges = <&msmgpio 0 0 90>;
>  
>  		gsbi5_uart_default: gsbi5_uart_default {
>  			mux {
> diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,apq8084-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/qcom,apq8084-pinctrl.txt
> index c4ea61ac56f2..362f32b945af 100644
> --- a/Documentation/devicetree/bindings/pinctrl/qcom,apq8084-pinctrl.txt
> +++ b/Documentation/devicetree/bindings/pinctrl/qcom,apq8084-pinctrl.txt
> @@ -40,6 +40,16 @@ MSM8960 platform.
>  	Definition: must be 2. Specifying the pin number and flags, as defined
>  		    in <dt-bindings/gpio/gpio.h>
>  
> +- gpio-ranges:
> +	Usage: required
> +	Value type: <prop-encoded-array>

But this is phandle with 3 cells.

Still, you don't need to redefine standard properties here. Just need to 
know optional vs. required and any constraints you can define (e.g. 
allowed values, number of values, etc.)


Same comments apply to the rest.

Rob

^ permalink raw reply

* [PATCH 8/8] arm64: tegra: Mark tcu as primary serial port on Tegra194 P2888
From: Jon Hunter @ 2018-05-22 16:29 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180508114403.14499-9-mperttunen@nvidia.com>


On 08/05/18 12:44, Mikko Perttunen wrote:
> The Tegra Combined UART is the proper primary serial port on P2888,
> so use it.
> 
> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
> ---
>   arch/arm64/boot/dts/nvidia/tegra194-p2888.dtsi | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/nvidia/tegra194-p2888.dtsi b/arch/arm64/boot/dts/nvidia/tegra194-p2888.dtsi
> index 859ab5af17c1..95e2433984f7 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra194-p2888.dtsi
> +++ b/arch/arm64/boot/dts/nvidia/tegra194-p2888.dtsi
> @@ -10,7 +10,7 @@
>   	aliases {
>   		sdhci0 = "/cbb/sdhci at 3460000";
>   		sdhci1 = "/cbb/sdhci at 3400000";
> -		serial0 = &uartb;
> +		serial0 = &tcu;
>   		i2c0 = "/bpmp/i2c";
>   		i2c1 = "/cbb/i2c at 3160000";
>   		i2c2 = "/cbb/i2c at c240000";
> 

Acked-by: Jon Hunter <jonathanh@nvidia.com>

Cheers
Jon

-- 
nvpublic

^ permalink raw reply

* [PATCH 7/8] arm64: tegra: Add nodes for tcu on Tegra194
From: Jon Hunter @ 2018-05-22 16:28 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180508114403.14499-8-mperttunen@nvidia.com>


On 08/05/18 12:44, Mikko Perttunen wrote:
> Add nodes required for communication through the Tegra Combined UART.
> This includes the AON HSP instance, addition of shared interrupts
> for the TOP0 HSP instance, and finally the TCU node itself. Also
> mark the HSP instances as compatible to tegra194-hsp, as the hardware
> is not identical but is compatible to tegra186-hsp.
> 
> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
> ---
>   arch/arm64/boot/dts/nvidia/tegra194.dtsi | 34 +++++++++++++++++++++++++++++---
>   1 file changed, 31 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/nvidia/tegra194.dtsi b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
> index 6d699815a84f..d7f780b06fe2 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra194.dtsi
> +++ b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
> @@ -217,10 +217,31 @@
>   		};
>   
>   		hsp_top0: hsp at 3c00000 {
> -			compatible = "nvidia,tegra186-hsp";
> +			compatible = "nvidia,tegra194-hsp", "nvidia,tegra186-hsp";

I might be wrong, but I think we may need to update the binding doc 
compatibility property for hsp. For example see 
'Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-flowctrl.txt'.

Cheers
Jon

-- 
nvpublic

^ permalink raw reply

* [PATCH 5/8] mailbox: tegra-hsp: Add support for shared mailboxes
From: Jon Hunter @ 2018-05-22 16:20 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180508114403.14499-6-mperttunen@nvidia.com>


On 08/05/18 12:44, Mikko Perttunen wrote:
> The Tegra HSP block supports 'shared mailboxes' that are simple 32-bit
> registers consisting of a FULL bit in MSB position and 31 bits of data.
> The hardware can be configured to trigger interrupts when a mailbox
> is empty or full. Add support for these shared mailboxes to the HSP
> driver.
> 
> The initial use for the mailboxes is the Tegra Combined UART. For this
> purpose, we use interrupts to receive data, and spinning to wait for
> the transmit mailbox to be emptied to minimize unnecessary overhead.
> 
> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
> ---
>   drivers/mailbox/tegra-hsp.c | 216 +++++++++++++++++++++++++++++++++++++++-----
>   1 file changed, 193 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/mailbox/tegra-hsp.c b/drivers/mailbox/tegra-hsp.c
> index 16eb970f2c9f..77bc8ed7ef15 100644
> --- a/drivers/mailbox/tegra-hsp.c
> +++ b/drivers/mailbox/tegra-hsp.c
> @@ -21,6 +21,11 @@
>   
>   #include <dt-bindings/mailbox/tegra186-hsp.h>
>   
> +#include "mailbox.h"
> +
> +#define HSP_INT0_IE		0x100
> +#define HSP_INT_IR		0x304
> +
>   #define HSP_INT_DIMENSIONING	0x380
>   #define HSP_nSM_SHIFT		0
>   #define HSP_nSS_SHIFT		4
> @@ -34,6 +39,8 @@
>   #define HSP_DB_RAW	0x8
>   #define HSP_DB_PENDING	0xc
>   
> +#define HSP_SM_SHRD_MBOX	0x0
> +
>   #define HSP_DB_CCPLEX		1
>   #define HSP_DB_BPMP		3
>   #define HSP_DB_MAX		7
> @@ -68,6 +75,18 @@ struct tegra_hsp_db_map {
>   	unsigned int index;
>   };
>   
> +struct tegra_hsp_mailbox {
> +	struct tegra_hsp_channel channel;
> +	unsigned int index;
> +	bool sending;
> +};
> +
> +static inline struct tegra_hsp_mailbox *
> +channel_to_mailbox(struct tegra_hsp_channel *channel)
> +{
> +	return container_of(channel, struct tegra_hsp_mailbox, channel);
> +}
> +
>   struct tegra_hsp_soc {
>   	const struct tegra_hsp_db_map *map;
>   };
> @@ -77,6 +96,7 @@ struct tegra_hsp {
>   	struct mbox_controller mbox;
>   	void __iomem *regs;
>   	unsigned int doorbell_irq;
> +	unsigned int shared_irq;
>   	unsigned int num_sm;
>   	unsigned int num_as;
>   	unsigned int num_ss;
> @@ -85,6 +105,7 @@ struct tegra_hsp {
>   	spinlock_t lock;
>   
>   	struct list_head doorbells;
> +	struct tegra_hsp_mailbox *mailboxes;
>   };
>   
>   static inline struct tegra_hsp *
> @@ -189,6 +210,35 @@ static irqreturn_t tegra_hsp_doorbell_irq(int irq, void *data)
>   	return IRQ_HANDLED;
>   }
>   
> +static irqreturn_t tegra_hsp_shared_irq(int irq, void *data)
> +{
> +	struct tegra_hsp_mailbox *mb;
> +	struct tegra_hsp *hsp = data;
> +	unsigned long bit, mask;
> +	u32 value;
> +
> +	mask = tegra_hsp_readl(hsp, HSP_INT_IR);
> +	/* Only interested in FULL interrupts */
> +	mask &= 0xff << 8;

Maybe add some definitions for the above.

Should we qualify 'mask' against the HSP_INT_IE register as well?

> +
> +	for_each_set_bit(bit, &mask, 16) {
> +		unsigned int mb_i = bit % 8;

If you right-shifted the mask above, you could avoid this modulo.

> +
> +		mb = &hsp->mailboxes[mb_i];
> +
> +		if (!mb->sending) {
> +			value = tegra_hsp_channel_readl(&mb->channel,
> +							HSP_SM_SHRD_MBOX);
> +			value &= ~BIT(31);

Similarly a definition for bit 31 may add some clarity.

> +			mbox_chan_received_data(mb->channel.chan, &value);
> +			tegra_hsp_channel_writel(&mb->channel, value,
> +						 HSP_SM_SHRD_MBOX);
> +		}
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
>   static struct tegra_hsp_channel *
>   tegra_hsp_doorbell_create(struct tegra_hsp *hsp, const char *name,
>   			  unsigned int master, unsigned int index)
> @@ -277,15 +327,58 @@ static void tegra_hsp_doorbell_shutdown(struct tegra_hsp_doorbell *db)
>   	spin_unlock_irqrestore(&hsp->lock, flags);
>   }
>   
> +static int tegra_hsp_mailbox_startup(struct tegra_hsp_mailbox *mb)
> +{
> +	struct tegra_hsp *hsp = mb->channel.hsp;
> +	u32 value;
> +
> +	mb->channel.chan->txdone_method = TXDONE_BY_BLOCK;
> +
> +	/* Route FULL interrupt to external IRQ 0 */
> +	value = tegra_hsp_readl(hsp, HSP_INT0_IE);
> +	value |= BIT(mb->index + 8);
> +	tegra_hsp_writel(hsp, value, HSP_INT0_IE);
> +
> +	return 0;
> +}
> +
> +static int tegra_hsp_mailbox_shutdown(struct tegra_hsp_mailbox *mb)
> +{
> +	struct tegra_hsp *hsp = mb->channel.hsp;
> +	u32 value;
> +
> +	value = tegra_hsp_readl(hsp, HSP_INT0_IE);
> +	value &= ~BIT(mb->index + 8);
> +	tegra_hsp_writel(hsp, value, HSP_INT0_IE);
> +
> +	return 0;
> +}
> +
>   static int tegra_hsp_send_data(struct mbox_chan *chan, void *data)
>   {
>   	struct tegra_hsp_channel *channel = chan->con_priv;
> -	struct tegra_hsp_doorbell *db;
> +	struct tegra_hsp_mailbox *mailbox;
> +	uint32_t value;
>   
>   	switch (channel->type) {
>   	case TEGRA_HSP_MBOX_TYPE_DB:
> -		db = channel_to_doorbell(channel);
> -		tegra_hsp_channel_writel(&db->channel, 1, HSP_DB_TRIGGER);
> +		tegra_hsp_channel_writel(channel, 1, HSP_DB_TRIGGER);
> +		return 0;
> +	case TEGRA_HSP_MBOX_TYPE_SM:
> +		mailbox = channel_to_mailbox(channel);
> +		mailbox->sending = true;
> +
> +		value = *(uint32_t *)data;
> +		/* Mark mailbox full */
> +		value |= BIT(31);
> +
> +		tegra_hsp_channel_writel(channel, value, HSP_SM_SHRD_MBOX);
> +
> +		while (tegra_hsp_channel_readl(channel, HSP_SM_SHRD_MBOX)
> +		               & BIT(31))
> +			cpu_relax();

Could be nice to use readx_poll_timeout() here.

> +
> +		return 0;
>   	}
>   
>   	return -EINVAL;
> @@ -298,6 +391,8 @@ static int tegra_hsp_startup(struct mbox_chan *chan)
>   	switch (channel->type) {
>   	case TEGRA_HSP_MBOX_TYPE_DB:
>   		return tegra_hsp_doorbell_startup(channel_to_doorbell(channel));
> +	case TEGRA_HSP_MBOX_TYPE_SM:
> +		return tegra_hsp_mailbox_startup(channel_to_mailbox(channel));
>   	}
>   
>   	return -EINVAL;
> @@ -311,6 +406,9 @@ static void tegra_hsp_shutdown(struct mbox_chan *chan)
>   	case TEGRA_HSP_MBOX_TYPE_DB:
>   		tegra_hsp_doorbell_shutdown(channel_to_doorbell(channel));
>   		break;
> +	case TEGRA_HSP_MBOX_TYPE_SM:
> +		tegra_hsp_mailbox_shutdown(channel_to_mailbox(channel));
> +		break;
>   	}
>   }
>   
> @@ -364,7 +462,16 @@ static struct mbox_chan *of_tegra_hsp_xlate(struct mbox_controller *mbox,
>   
>   	switch (type) {
>   	case TEGRA_HSP_MBOX_TYPE_DB:
> -		return tegra_hsp_doorbell_xlate(hsp, param);
> +		if (hsp->doorbell_irq)
> +			return tegra_hsp_doorbell_xlate(hsp, param);
> +		else
> +			return ERR_PTR(-EINVAL);
> +
> +	case TEGRA_HSP_MBOX_TYPE_SM:
> +		if (hsp->shared_irq && param < hsp->num_sm)
> +			return hsp->mailboxes[param].channel.chan;
> +		else
> +			return ERR_PTR(-EINVAL);
>   
>   	default:
>   		return ERR_PTR(-EINVAL);
> @@ -403,6 +510,31 @@ static int tegra_hsp_add_doorbells(struct tegra_hsp *hsp)
>   	return 0;
>   }
>   
> +static int tegra_hsp_add_mailboxes(struct tegra_hsp *hsp, struct device *dev)
> +{
> +	int i;
> +
> +	hsp->mailboxes = devm_kcalloc(dev, hsp->num_sm, sizeof(*hsp->mailboxes),
> +				      GFP_KERNEL);
> +	if (!hsp->mailboxes)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < hsp->num_sm; i++) {
> +		struct tegra_hsp_mailbox *mb = &hsp->mailboxes[i];
> +
> +		mb->index = i;
> +		mb->sending = false;
> +
> +		mb->channel.hsp = hsp;
> +		mb->channel.type = TEGRA_HSP_MBOX_TYPE_SM;
> +		mb->channel.regs = hsp->regs + SZ_64K + i * SZ_32K;
> +		mb->channel.chan = &hsp->mbox.chans[i];
> +		mb->channel.chan->con_priv = &mb->channel;
> +	}
> +
> +	return 0;
> +}
> +
>   static int tegra_hsp_probe(struct platform_device *pdev)
>   {
>   	struct tegra_hsp *hsp;
> @@ -431,14 +563,19 @@ static int tegra_hsp_probe(struct platform_device *pdev)
>   	hsp->num_si = (value >> HSP_nSI_SHIFT) & HSP_nINT_MASK;
>   
>   	err = platform_get_irq_byname(pdev, "doorbell");
> -	if (err < 0) {
> -		dev_err(&pdev->dev, "failed to get doorbell IRQ: %d\n", err);
> -		return err;
> -	}
> +	if (err < 0)
> +		hsp->doorbell_irq = 0;

It should not be necessary to set to zero as it should already be zero.

> +	else
> +		hsp->doorbell_irq = err;
>   
> -	hsp->doorbell_irq = err;
> +	err = platform_get_irq_byname(pdev, "shared0");
> +	if (err < 0)
> +		hsp->shared_irq = 0;

It should not be necessary to set to zero as it should already be zero.

> +	else
> +		hsp->shared_irq = err;
>   
>   	hsp->mbox.of_xlate = of_tegra_hsp_xlate;
> +	/* First nSM are reserved for mailboxes */
>   	hsp->mbox.num_chans = 32;
>   	hsp->mbox.dev = &pdev->dev;
>   	hsp->mbox.txdone_irq = false;
> @@ -451,10 +588,22 @@ static int tegra_hsp_probe(struct platform_device *pdev)
>   	if (!hsp->mbox.chans)
>   		return -ENOMEM;
>   
> -	err = tegra_hsp_add_doorbells(hsp);
> -	if (err < 0) {
> -		dev_err(&pdev->dev, "failed to add doorbells: %d\n", err);
> -		return err;
> +	if (hsp->doorbell_irq) {
> +		err = tegra_hsp_add_doorbells(hsp);
> +		if (err < 0) {
> +			dev_err(&pdev->dev, "failed to add doorbells: %d\n",
> +			        err);
> +			return err;
> +		}
> +	}
> +
> +	if (hsp->shared_irq) {
> +		err = tegra_hsp_add_mailboxes(hsp, &pdev->dev);
> +		if (err < 0) {
> +			dev_err(&pdev->dev, "failed to add mailboxes: %d\n",
> +			        err);
> +			goto remove_doorbells;
> +		}
>   	}
>   
>   	platform_set_drvdata(pdev, hsp);
> @@ -462,20 +611,40 @@ static int tegra_hsp_probe(struct platform_device *pdev)
>   	err = mbox_controller_register(&hsp->mbox);
>   	if (err) {
>   		dev_err(&pdev->dev, "failed to register mailbox: %d\n", err);
> -		tegra_hsp_remove_doorbells(hsp);
> -		return err;
> +		goto remove_doorbells;
>   	}
>   
> -	err = devm_request_irq(&pdev->dev, hsp->doorbell_irq,
> -			       tegra_hsp_doorbell_irq, IRQF_NO_SUSPEND,
> -			       dev_name(&pdev->dev), hsp);
> -	if (err < 0) {
> -		dev_err(&pdev->dev, "failed to request doorbell IRQ#%u: %d\n",
> -			hsp->doorbell_irq, err);
> -		return err;
> +	if (hsp->doorbell_irq) {
> +		err = devm_request_irq(&pdev->dev, hsp->doorbell_irq,
> +				       tegra_hsp_doorbell_irq, IRQF_NO_SUSPEND,
> +				       dev_name(&pdev->dev), hsp);
> +		if (err < 0) {
> +			dev_err(&pdev->dev,
> +			        "failed to request doorbell IRQ#%u: %d\n",
> +				hsp->doorbell_irq, err);
> +			return err;

Clean-up?

> +		}
> +	}
> +
> +	if (hsp->shared_irq) {
> +		err = devm_request_irq(&pdev->dev, hsp->shared_irq,
> +				       tegra_hsp_shared_irq, 0,
> +				       dev_name(&pdev->dev), hsp);
> +		if (err < 0) {
> +			dev_err(&pdev->dev,
> +				"failed to request shared0 IRQ%u: %d\n",
> +				hsp->shared_irq, err);
> +			return err;

Clean-up?

> +		}
>   	}
>   
>   	return 0;
> +
> +remove_doorbells:
> +	if (hsp->doorbell_irq)
> +		tegra_hsp_remove_doorbells(hsp);
> +
> +	return err;
>   }
>   
>   static int tegra_hsp_remove(struct platform_device *pdev)
> @@ -483,7 +652,8 @@ static int tegra_hsp_remove(struct platform_device *pdev)
>   	struct tegra_hsp *hsp = platform_get_drvdata(pdev);
>   
>   	mbox_controller_unregister(&hsp->mbox);
> -	tegra_hsp_remove_doorbells(hsp);
> +	if (hsp->doorbell_irq)
> +		tegra_hsp_remove_doorbells(hsp);
>   
	>   	return 0;
>   }
> 

Cheers
Jon

-- 
nvpublic

^ permalink raw reply

* [rjarzmik:work/dma_slave_map 12/15] drivers/mtd/nand/marvell_nand.c:2621:17: sparse: call with no type!
From: kbuild test robot @ 2018-05-22 16:15 UTC (permalink / raw)
  To: linux-arm-kernel

tree:   https://github.com/rjarzmik/linux work/dma_slave_map
head:   51b9077eb5c5f055bbbc8138b08830ce23ad340e
commit: 308214fe387eaf6b5547d01e573c41bb309a59fb [12/15] dmaengine: pxa: make the filter function internal
reproduce:
        # apt-get install sparse
        git checkout 308214fe387eaf6b5547d01e573c41bb309a59fb
        make ARCH=x86_64 allmodconfig
        make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

   drivers/mtd/nand/marvell_nand.c:2621:17: sparse: undefined identifier 'pxad_filter_fn'
>> drivers/mtd/nand/marvell_nand.c:2621:17: sparse: call with no type!
   In file included from drivers/mtd/nand/marvell_nand.c:21:0:
   drivers/mtd/nand/marvell_nand.c: In function 'marvell_nfc_init_dma':
   drivers/mtd/nand/marvell_nand.c:2621:42: error: 'pxad_filter_fn' undeclared (first use in this function); did you mean 'dma_filter_fn'?
      dma_request_slave_channel_compat(mask, pxad_filter_fn,
                                             ^
   include/linux/dmaengine.h:1408:46: note: in definition of macro 'dma_request_slave_channel_compat'
     __dma_request_slave_channel_compat(&(mask), x, y, dev, name)
                                                 ^
   drivers/mtd/nand/marvell_nand.c:2621:42: note: each undeclared identifier is reported only once for each function it appears in
      dma_request_slave_channel_compat(mask, pxad_filter_fn,
                                             ^
   include/linux/dmaengine.h:1408:46: note: in definition of macro 'dma_request_slave_channel_compat'
     __dma_request_slave_channel_compat(&(mask), x, y, dev, name)
                                                 ^

vim +2621 drivers/mtd/nand/marvell_nand.c

02f26ecf Miquel Raynal 2018-01-09  2588  
02f26ecf Miquel Raynal 2018-01-09  2589  static int marvell_nfc_init_dma(struct marvell_nfc *nfc)
02f26ecf Miquel Raynal 2018-01-09  2590  {
02f26ecf Miquel Raynal 2018-01-09  2591  	struct platform_device *pdev = container_of(nfc->dev,
02f26ecf Miquel Raynal 2018-01-09  2592  						    struct platform_device,
02f26ecf Miquel Raynal 2018-01-09  2593  						    dev);
02f26ecf Miquel Raynal 2018-01-09  2594  	struct dma_slave_config config = {};
02f26ecf Miquel Raynal 2018-01-09  2595  	struct resource *r;
02f26ecf Miquel Raynal 2018-01-09  2596  	dma_cap_mask_t mask;
02f26ecf Miquel Raynal 2018-01-09  2597  	struct pxad_param param;
02f26ecf Miquel Raynal 2018-01-09  2598  	int ret;
02f26ecf Miquel Raynal 2018-01-09  2599  
02f26ecf Miquel Raynal 2018-01-09  2600  	if (!IS_ENABLED(CONFIG_PXA_DMA)) {
02f26ecf Miquel Raynal 2018-01-09  2601  		dev_warn(nfc->dev,
02f26ecf Miquel Raynal 2018-01-09  2602  			 "DMA not enabled in configuration\n");
02f26ecf Miquel Raynal 2018-01-09  2603  		return -ENOTSUPP;
02f26ecf Miquel Raynal 2018-01-09  2604  	}
02f26ecf Miquel Raynal 2018-01-09  2605  
02f26ecf Miquel Raynal 2018-01-09  2606  	ret = dma_set_mask_and_coherent(nfc->dev, DMA_BIT_MASK(32));
02f26ecf Miquel Raynal 2018-01-09  2607  	if (ret)
02f26ecf Miquel Raynal 2018-01-09  2608  		return ret;
02f26ecf Miquel Raynal 2018-01-09  2609  
02f26ecf Miquel Raynal 2018-01-09  2610  	r = platform_get_resource(pdev, IORESOURCE_DMA, 0);
02f26ecf Miquel Raynal 2018-01-09  2611  	if (!r) {
02f26ecf Miquel Raynal 2018-01-09  2612  		dev_err(nfc->dev, "No resource defined for data DMA\n");
02f26ecf Miquel Raynal 2018-01-09  2613  		return -ENXIO;
02f26ecf Miquel Raynal 2018-01-09  2614  	}
02f26ecf Miquel Raynal 2018-01-09  2615  
02f26ecf Miquel Raynal 2018-01-09  2616  	param.drcmr = r->start;
02f26ecf Miquel Raynal 2018-01-09  2617  	param.prio = PXAD_PRIO_LOWEST;
02f26ecf Miquel Raynal 2018-01-09  2618  	dma_cap_zero(mask);
02f26ecf Miquel Raynal 2018-01-09  2619  	dma_cap_set(DMA_SLAVE, mask);
02f26ecf Miquel Raynal 2018-01-09  2620  	nfc->dma_chan =
02f26ecf Miquel Raynal 2018-01-09 @2621  		dma_request_slave_channel_compat(mask, pxad_filter_fn,
02f26ecf Miquel Raynal 2018-01-09  2622  						 &param, nfc->dev,
02f26ecf Miquel Raynal 2018-01-09  2623  						 "data");
02f26ecf Miquel Raynal 2018-01-09  2624  	if (!nfc->dma_chan) {
02f26ecf Miquel Raynal 2018-01-09  2625  		dev_err(nfc->dev,
02f26ecf Miquel Raynal 2018-01-09  2626  			"Unable to request data DMA channel\n");
02f26ecf Miquel Raynal 2018-01-09  2627  		return -ENODEV;
02f26ecf Miquel Raynal 2018-01-09  2628  	}
02f26ecf Miquel Raynal 2018-01-09  2629  
02f26ecf Miquel Raynal 2018-01-09  2630  	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
02f26ecf Miquel Raynal 2018-01-09  2631  	if (!r)
02f26ecf Miquel Raynal 2018-01-09  2632  		return -ENXIO;
02f26ecf Miquel Raynal 2018-01-09  2633  
02f26ecf Miquel Raynal 2018-01-09  2634  	config.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
02f26ecf Miquel Raynal 2018-01-09  2635  	config.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
02f26ecf Miquel Raynal 2018-01-09  2636  	config.src_addr = r->start + NDDB;
02f26ecf Miquel Raynal 2018-01-09  2637  	config.dst_addr = r->start + NDDB;
02f26ecf Miquel Raynal 2018-01-09  2638  	config.src_maxburst = 32;
02f26ecf Miquel Raynal 2018-01-09  2639  	config.dst_maxburst = 32;
02f26ecf Miquel Raynal 2018-01-09  2640  	ret = dmaengine_slave_config(nfc->dma_chan, &config);
02f26ecf Miquel Raynal 2018-01-09  2641  	if (ret < 0) {
02f26ecf Miquel Raynal 2018-01-09  2642  		dev_err(nfc->dev, "Failed to configure DMA channel\n");
02f26ecf Miquel Raynal 2018-01-09  2643  		return ret;
02f26ecf Miquel Raynal 2018-01-09  2644  	}
02f26ecf Miquel Raynal 2018-01-09  2645  
02f26ecf Miquel Raynal 2018-01-09  2646  	/*
02f26ecf Miquel Raynal 2018-01-09  2647  	 * DMA must act on length multiple of 32 and this length may be
02f26ecf Miquel Raynal 2018-01-09  2648  	 * bigger than the destination buffer. Use this buffer instead
02f26ecf Miquel Raynal 2018-01-09  2649  	 * for DMA transfers and then copy the desired amount of data to
02f26ecf Miquel Raynal 2018-01-09  2650  	 * the provided buffer.
02f26ecf Miquel Raynal 2018-01-09  2651  	 */
c495a927 Miquel Raynal 2018-01-19  2652  	nfc->dma_buf = kmalloc(MAX_CHUNK_SIZE, GFP_KERNEL | GFP_DMA);
02f26ecf Miquel Raynal 2018-01-09  2653  	if (!nfc->dma_buf)
02f26ecf Miquel Raynal 2018-01-09  2654  		return -ENOMEM;
02f26ecf Miquel Raynal 2018-01-09  2655  
02f26ecf Miquel Raynal 2018-01-09  2656  	nfc->use_dma = true;
02f26ecf Miquel Raynal 2018-01-09  2657  
02f26ecf Miquel Raynal 2018-01-09  2658  	return 0;
02f26ecf Miquel Raynal 2018-01-09  2659  }
02f26ecf Miquel Raynal 2018-01-09  2660  

:::::: The code at line 2621 was first introduced by commit
:::::: 02f26ecf8c772751d4b24744d487f6b1b20e75d4 mtd: nand: add reworked Marvell NAND controller driver

:::::: TO: Miquel Raynal <miquel.raynal@free-electrons.com>
:::::: CC: Boris Brezillon <boris.brezillon@free-electrons.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

^ permalink raw reply

* [PATCH v3] arm64: fault: Don't leak data in ESR context for user fault on kernel VA
From: Peter Maydell @ 2018-05-22 16:11 UTC (permalink / raw)
  To: linux-arm-kernel

If userspace faults on a kernel address, handing them the raw ESR
value on the sigframe as part of the delivered signal can leak data
useful to attackers who are using information about the underlying hardware
fault type (e.g. translation vs permission) as a mechanism to defeat KASLR.

However there are also legitimate uses for the information provided
in the ESR -- notably the GCC and LLVM sanitizers use this to report
whether wild pointer accesses by the application are reads or writes
(since a wild write is a more serious bug than a wild read), so we
don't want to drop the ESR information entirely.

For faulting addresses in the kernel, sanitize the ESR. We choose
to present userspace with the illusion that there is nothing mapped
in the kernel's part of the address space at all, by reporting all
faults as level 0 translation faults taken to EL1.

These fields are safe to pass through to userspace as they depend
only on the instruction that userspace used to provoke the fault:
 EC IL (always)
 ISV CM WNR (for all data aborts)
All the other fields in ESR except DFSC are architecturally RES0
for an L0 translation fault taken to EL1, so can be zeroed out
without confusing userspace.

The illusion is not entirely perfect, as there is a tiny wrinkle
where we will report an alignment fault that was not due to the memory
type (for instance a LDREX to an unaligned address) as a translation
fault, whereas if you do this on real unmapped memory the alignment
fault takes precedence. This is not likely to trip anybody up in
practice, as the only users we know of for the ESR information who
care about the behaviour for kernel addresses only really want to
know about the WnR bit.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
This patch is an alternative proposal to Will's patch
https://patchwork.kernel.org/patch/10258781/
which simply removed the ESR record entirely for kernel addresses.

Changes v1->v2:
 * rebased on master
 * commit message tweak
 * DABT_CUR and IABT_CUR moved to "can't happen" default case
 * explicitly clear the bits which are RES0 if ISV == 0
 * comment text tweaks
Changes v2->v3:
 * remove the support for reporting ESRs with ISV == 1 (this
   can't happen, and we probably don't want to tell userspace
   that the exception was taken to EL2 if in some hypothetical
   future that becomes possible)
 * rebased on 4.17-rc6
---
 arch/arm64/mm/fault.c | 51 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index 4165485e8b6e..2af3dd89bcdb 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -293,6 +293,57 @@ static void __do_kernel_fault(unsigned long addr, unsigned int esr,
 static void __do_user_fault(struct siginfo *info, unsigned int esr)
 {
 	current->thread.fault_address = (unsigned long)info->si_addr;
+
+	/*
+	 * If the faulting address is in the kernel, we must sanitize the ESR.
+	 * From userspace's point of view, kernel-only mappings don't exist
+	 * at all, so we report them as level 0 translation faults.
+	 * (This is not quite the way that "no mapping there at all" behaves:
+	 * an alignment fault not caused by the memory type would take
+	 * precedence over translation fault for a real access to empty
+	 * space. Unfortunately we can't easily distinguish "alignment fault
+	 * not caused by memory type" from "alignment fault caused by memory
+	 * type", so we ignore this wrinkle and just return the translation
+	 * fault.)
+	 */
+	if (current->thread.fault_address >= TASK_SIZE) {
+		switch (ESR_ELx_EC(esr)) {
+		case ESR_ELx_EC_DABT_LOW:
+			/*
+			 * These bits provide only information about the
+			 * faulting instruction, which userspace knows already.
+			 * We explicitly clear bits which are architecturally
+			 * RES0 in case they are given meanings in future.
+			 * We always report the ESR as if the fault was taken
+			 * to EL1 and so ISV and the bits in ISS[23:14] are
+			 * clear. (In fact it always will be a fault to EL1.)
+			 */
+			esr &= ESR_ELx_EC_MASK | ESR_ELx_IL |
+				ESR_ELx_CM | ESR_ELx_WNR;
+			esr |= ESR_ELx_FSC_FAULT;
+			break;
+		case ESR_ELx_EC_IABT_LOW:
+			/*
+			 * Claim a level 0 translation fault.
+			 * All other bits are architecturally RES0 for faults
+			 * reported with that DFSC value, so we clear them.
+			 */
+			esr &= ESR_ELx_EC_MASK | ESR_ELx_IL;
+			esr |= ESR_ELx_FSC_FAULT;
+			break;
+		default:
+			/*
+			 * This should never happen (entry.S only brings us
+			 * into this code for insn and data aborts from a lower
+			 * exception level). Fail safe by not providing an ESR
+			 * context record at all.
+			 */
+			WARN(1, "ESR 0x%x is not DABT or IABT from EL0\n", esr);
+			esr = 0;
+			break;
+		}
+	}
+
 	current->thread.fault_code = esr;
 	arm64_force_sig_info(info, esr_to_fault_info(esr)->name, current);
 }
-- 
2.17.0

^ permalink raw reply related

* [PATCH] drivers/perf: Remove ARM_SPE_PMU explicit PERF_EVENTS dependency
From: Mark Rutland @ 2018-05-22 16:08 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1527004444-239462-1-git-send-email-john.garry@huawei.com>

On Tue, May 22, 2018 at 11:54:04PM +0800, John Garry wrote:
> Since commit bddb9b68d3fb ("drivers/perf: commonise PERF_EVENTS
> dependency"), all perf drivers depend on PERF_EVENTS config under a
> common menu.
> 
> Config ARM_SPE_PMU still declares explicitly a dependency on
> PERF_EVENTS, which is unneeded, so remove it.
> 
> Signed-off-by: John Garry <john.garry@huawei.com>

Acked-by: Mark Rutland <mark.rutland@arm.com>

Will, I assume that you will queue this.

Mark.

> 
> diff --git a/drivers/perf/Kconfig b/drivers/perf/Kconfig
> index 28bb5a0..90ce4da 100644
> --- a/drivers/perf/Kconfig
> +++ b/drivers/perf/Kconfig
> @@ -94,7 +94,7 @@ config XGENE_PMU
>  
>  config ARM_SPE_PMU
>  	tristate "Enable support for the ARMv8.2 Statistical Profiling Extension"
> -	depends on PERF_EVENTS && ARM64
> +	depends on ARM64
>  	help
>  	  Enable perf support for the ARMv8.2 Statistical Profiling
>  	  Extension, which provides periodic sampling of operations in
> -- 
> 1.9.1
> 

^ permalink raw reply

* [PATCH v10 18/18] KVM: arm64: Invoke FPSIMD context switch trap from C
From: Dave Martin @ 2018-05-22 16:05 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1527005119-6842-1-git-send-email-Dave.Martin@arm.com>

The conversion of the FPSIMD context switch trap code to C has added
some overhead to calling it, due to the need to save registers that
the procedure call standard defines as caller-saved.

So, perhaps it is no longer worth invoking this trap handler quite
so early.

Instead, we can invoke it from fixup_guest_exit(), with little
likelihood of increasing the overhead much further.

As a convenience, this patch gives __hyp_switch_fpsimd() the same
return semantics fixup_guest_exit().  For now there is no
possibility of a spurious FPSIMD trap, so the function always
returns true, but this allows it to be tail-called with a single
return statement.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
Reviewed-by: Christoffer Dall <christoffer.dall@arm.com>
---
 arch/arm64/kvm/hyp/entry.S     | 30 ------------------------------
 arch/arm64/kvm/hyp/hyp-entry.S | 19 -------------------
 arch/arm64/kvm/hyp/switch.c    | 15 +++++++++++++--
 3 files changed, 13 insertions(+), 51 deletions(-)

diff --git a/arch/arm64/kvm/hyp/entry.S b/arch/arm64/kvm/hyp/entry.S
index 40f349b..fad1e16 100644
--- a/arch/arm64/kvm/hyp/entry.S
+++ b/arch/arm64/kvm/hyp/entry.S
@@ -166,33 +166,3 @@ abort_guest_exit_end:
 	orr	x0, x0, x5
 1:	ret
 ENDPROC(__guest_exit)
-
-ENTRY(__fpsimd_guest_restore)
-	// x0: esr
-	// x1: vcpu
-	// x2-x29,lr: vcpu regs
-	// vcpu x0-x1 on the stack
-	stp	x2, x3, [sp, #-144]!
-	stp	x4, x5, [sp, #16]
-	stp	x6, x7, [sp, #32]
-	stp	x8, x9, [sp, #48]
-	stp	x10, x11, [sp, #64]
-	stp	x12, x13, [sp, #80]
-	stp	x14, x15, [sp, #96]
-	stp	x16, x17, [sp, #112]
-	stp	x18, lr, [sp, #128]
-
-	bl	__hyp_switch_fpsimd
-
-	ldp	x4, x5, [sp, #16]
-	ldp	x6, x7, [sp, #32]
-	ldp	x8, x9, [sp, #48]
-	ldp	x10, x11, [sp, #64]
-	ldp	x12, x13, [sp, #80]
-	ldp	x14, x15, [sp, #96]
-	ldp	x16, x17, [sp, #112]
-	ldp	x18, lr, [sp, #128]
-	ldp	x0, x1, [sp, #144]
-	ldp	x2, x3, [sp], #160
-	eret
-ENDPROC(__fpsimd_guest_restore)
diff --git a/arch/arm64/kvm/hyp/hyp-entry.S b/arch/arm64/kvm/hyp/hyp-entry.S
index bffece2..753b9d2 100644
--- a/arch/arm64/kvm/hyp/hyp-entry.S
+++ b/arch/arm64/kvm/hyp/hyp-entry.S
@@ -113,25 +113,6 @@ el1_hvc_guest:
 
 el1_trap:
 	get_vcpu_ptr	x1, x0
-
-	mrs		x0, esr_el2
-	lsr		x0, x0, #ESR_ELx_EC_SHIFT
-	/*
-	 * x0: ESR_EC
-	 * x1: vcpu pointer
-	 */
-
-	/*
-	 * We trap the first access to the FP/SIMD to save the host context
-	 * and restore the guest context lazily.
-	 * If FP/SIMD is not implemented, handle the trap and inject an
-	 * undefined instruction exception to the guest.
-	 */
-alternative_if_not ARM64_HAS_NO_FPSIMD
-	cmp	x0, #ESR_ELx_EC_FP_ASIMD
-	b.eq	__fpsimd_guest_restore
-alternative_else_nop_endif
-
 	mov	x0, #ARM_EXCEPTION_TRAP
 	b	__guest_exit
 
diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index 4fbee95..2d45bd7 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -328,8 +328,7 @@ static bool __hyp_text __skip_instr(struct kvm_vcpu *vcpu)
 	}
 }
 
-void __hyp_text __hyp_switch_fpsimd(u64 esr __always_unused,
-				    struct kvm_vcpu *vcpu)
+static bool __hyp_text __hyp_switch_fpsimd(struct kvm_vcpu *vcpu)
 {
 	struct user_fpsimd_state *host_fpsimd = vcpu->arch.host_fpsimd_state;
 
@@ -369,6 +368,8 @@ void __hyp_text __hyp_switch_fpsimd(u64 esr __always_unused,
 			     fpexc32_el2);
 
 	vcpu->arch.flags |= KVM_ARM64_FP_ENABLED;
+
+	return true;
 }
 
 /*
@@ -390,6 +391,16 @@ static bool __hyp_text fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
 	if (*exit_code != ARM_EXCEPTION_TRAP)
 		goto exit;
 
+	/*
+	 * We trap the first access to the FP/SIMD to save the host context
+	 * and restore the guest context lazily.
+	 * If FP/SIMD is not implemented, handle the trap and inject an
+	 * undefined instruction exception to the guest.
+	 */
+	if (system_supports_fpsimd() &&
+	    kvm_vcpu_trap_get_class(vcpu) == ESR_ELx_EC_FP_ASIMD)
+		return __hyp_switch_fpsimd(vcpu);
+
 	if (!__populate_fault_info(vcpu))
 		return true;
 
-- 
2.1.4

^ permalink raw reply related

* [PATCH v10 17/18] KVM: arm64: Fold redundant exit code checks out of fixup_guest_exit()
From: Dave Martin @ 2018-05-22 16:05 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1527005119-6842-1-git-send-email-Dave.Martin@arm.com>

The entire tail of fixup_guest_exit() is contained in if statements
of the form if (x && *exit_code == ARM_EXCEPTION_TRAP).  As a result,
we can check just once and bail out of the function early, allowing
the remaining if conditions to be simplified.

The only awkward case is where *exit_code is changed to
ARM_EXCEPTION_EL1_SERROR in the case of an illegal GICv2 CPU
interface access: in that case, the GICv3 trap handling code is
skipped using a goto.  This avoids pointlessly evaluating the
static branch check for the GICv3 case, even though we can't have
vgic_v2_cpuif_trap and vgic_v3_cpuif_trap true simultaneously
unless we have a GICv3 and GICv2 on the host: that sounds stupid,
but I haven't satisfied myself that it can't happen.

No functional change.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/kvm/hyp/switch.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index 18d0faa..4fbee95 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -387,11 +387,13 @@ static bool __hyp_text fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
 	 * same PC once the SError has been injected, and replay the
 	 * trapping instruction.
 	 */
-	if (*exit_code == ARM_EXCEPTION_TRAP && !__populate_fault_info(vcpu))
+	if (*exit_code != ARM_EXCEPTION_TRAP)
+		goto exit;
+
+	if (!__populate_fault_info(vcpu))
 		return true;
 
-	if (static_branch_unlikely(&vgic_v2_cpuif_trap) &&
-	    *exit_code == ARM_EXCEPTION_TRAP) {
+	if (static_branch_unlikely(&vgic_v2_cpuif_trap)) {
 		bool valid;
 
 		valid = kvm_vcpu_trap_get_class(vcpu) == ESR_ELx_EC_DABT_LOW &&
@@ -417,11 +419,12 @@ static bool __hyp_text fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
 					*vcpu_cpsr(vcpu) &= ~DBG_SPSR_SS;
 				*exit_code = ARM_EXCEPTION_EL1_SERROR;
 			}
+
+			goto exit;
 		}
 	}
 
 	if (static_branch_unlikely(&vgic_v3_cpuif_trap) &&
-	    *exit_code == ARM_EXCEPTION_TRAP &&
 	    (kvm_vcpu_trap_get_class(vcpu) == ESR_ELx_EC_SYS64 ||
 	     kvm_vcpu_trap_get_class(vcpu) == ESR_ELx_EC_CP15_32)) {
 		int ret = __vgic_v3_perform_cpuif_access(vcpu);
@@ -430,6 +433,7 @@ static bool __hyp_text fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
 			return true;
 	}
 
+exit:
 	/* Return to the host kernel and handle the exit */
 	return false;
 }
-- 
2.1.4

^ permalink raw reply related

* [PATCH v10 16/18] KVM: arm64: Remove redundant *exit_code changes in fpsimd_guest_exit()
From: Dave Martin @ 2018-05-22 16:05 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1527005119-6842-1-git-send-email-Dave.Martin@arm.com>

In fixup_guest_exit(), there are a couple of cases where after
checking what the exit code was, we assign it explicitly with the
value it already had.

Assuming this is not indicative of a bug, these assignments are not
needed.

This patch removes the redundant assignments, and simplifies some
if-nesting that becomes trivial as a result.

No functional change.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/kvm/hyp/switch.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index a6a8c7d..18d0faa 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -403,12 +403,8 @@ static bool __hyp_text fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
 		if (valid) {
 			int ret = __vgic_v2_perform_cpuif_access(vcpu);
 
-			if (ret == 1) {
-				if (__skip_instr(vcpu))
-					return true;
-				else
-					*exit_code = ARM_EXCEPTION_TRAP;
-			}
+			if (ret ==  1 && __skip_instr(vcpu))
+				return true;
 
 			if (ret == -1) {
 				/* Promote an illegal access to an
@@ -430,12 +426,8 @@ static bool __hyp_text fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
 	     kvm_vcpu_trap_get_class(vcpu) == ESR_ELx_EC_CP15_32)) {
 		int ret = __vgic_v3_perform_cpuif_access(vcpu);
 
-		if (ret == 1) {
-			if (__skip_instr(vcpu))
-				return true;
-			else
-				*exit_code = ARM_EXCEPTION_TRAP;
-		}
+		if (ret == 1 && __skip_instr(vcpu))
+			return true;
 	}
 
 	/* Return to the host kernel and handle the exit */
-- 
2.1.4

^ permalink raw reply related

* [PATCH v10 15/18] KVM: arm64: Remove eager host SVE state saving
From: Dave Martin @ 2018-05-22 16:05 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1527005119-6842-1-git-send-email-Dave.Martin@arm.com>

Now that the host SVE context can be saved on demand from Hyp,
there is no longer any need to save this state in advance before
entering the guest.

This patch removes the relevant call to
kvm_fpsimd_flush_cpu_state().

Since the problem that function was intended to solve now no longer
exists, the function and its dependencies are also deleted.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
Acked-by: Christoffer Dall <christoffer.dall@arm.com>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm/include/asm/kvm_host.h   |  3 ---
 arch/arm64/include/asm/kvm_host.h | 10 ----------
 arch/arm64/kernel/fpsimd.c        | 21 ---------------------
 virt/kvm/arm/arm.c                |  3 ---
 4 files changed, 37 deletions(-)

diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index 3b85bbb..f079a20 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -312,9 +312,6 @@ static inline void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu) {}
 static inline void kvm_arch_vcpu_ctxsync_fp(struct kvm_vcpu *vcpu) {}
 static inline void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu) {}
 
-/* All host FP/SIMD state is restored on guest exit, so nothing to save: */
-static inline void kvm_fpsimd_flush_cpu_state(void) {}
-
 static inline void kvm_arm_vhe_guest_enter(void) {}
 static inline void kvm_arm_vhe_guest_exit(void) {}
 
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 06d5a61..ce7ed92 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -457,16 +457,6 @@ static inline int kvm_arch_vcpu_run_pid_change(struct kvm_vcpu *vcpu)
 }
 #endif
 
-/*
- * All host FP/SIMD state is restored on guest exit, so nothing needs
- * doing here except in the SVE case:
-*/
-static inline void kvm_fpsimd_flush_cpu_state(void)
-{
-	if (system_supports_sve())
-		sve_flush_cpu_state();
-}
-
 static inline void kvm_arm_vhe_guest_enter(void)
 {
 	local_daif_mask();
diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index f39d3b0..ea5d780 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -120,7 +120,6 @@
  */
 struct fpsimd_last_state_struct {
 	struct user_fpsimd_state *st;
-	bool sve_in_use;
 };
 
 static DEFINE_PER_CPU(struct fpsimd_last_state_struct, fpsimd_last_state);
@@ -1003,7 +1002,6 @@ void fpsimd_bind_task_to_cpu(void)
 		this_cpu_ptr(&fpsimd_last_state);
 
 	last->st = &current->thread.uw.fpsimd_state;
-	last->sve_in_use = test_thread_flag(TIF_SVE);
 	current->thread.fpsimd_cpu = smp_processor_id();
 
 	if (system_supports_sve()) {
@@ -1025,7 +1023,6 @@ void fpsimd_bind_state_to_cpu(struct user_fpsimd_state *st)
 	WARN_ON(!in_softirq() && !irqs_disabled());
 
 	last->st = st;
-	last->sve_in_use = false;
 }
 
 /*
@@ -1086,24 +1083,6 @@ void fpsimd_flush_cpu_state(void)
 	set_thread_flag(TIF_FOREIGN_FPSTATE);
 }
 
-/*
- * Invalidate any task SVE state currently held in this CPU's regs.
- *
- * This is used to prevent the kernel from trying to reuse SVE register data
- * that is detroyed by KVM guest enter/exit.  This function should go away when
- * KVM SVE support is implemented.  Don't use it for anything else.
- */
-#ifdef CONFIG_ARM64_SVE
-void sve_flush_cpu_state(void)
-{
-	struct fpsimd_last_state_struct const *last =
-		this_cpu_ptr(&fpsimd_last_state);
-
-	if (last->st && last->sve_in_use)
-		fpsimd_flush_cpu_state();
-}
-#endif /* CONFIG_ARM64_SVE */
-
 #ifdef CONFIG_KERNEL_MODE_NEON
 
 DEFINE_PER_CPU(bool, kernel_neon_busy);
diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
index ce7c6f3..39e7771 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -682,9 +682,6 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
 		 */
 		preempt_disable();
 
-		/* Flush FP/SIMD state that can't survive guest entry/exit */
-		kvm_fpsimd_flush_cpu_state();
-
 		kvm_pmu_flush_hwstate(vcpu);
 
 		local_irq_disable();
-- 
2.1.4

^ permalink raw reply related


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