* Aw: [PATCH net-next v2 0/3] net: dsa: mt7530: Convert to PHYLINK and add support for port 5
From: Frank Wunderlich @ 2019-08-22 15:44 UTC (permalink / raw)
To: "René van Dorst"
Cc: Andrew Lunn, Florian Fainelli, netdev, Sean Wang, linux-mips,
David S . Miller, "René van Dorst", linux-mediatek,
John Crispin, Matthias Brugger, Vivien Didelot, linux-arm-kernel
In-Reply-To: <20190821144547.15113-1-opensource@vdorst.com>
Hi,
tested on BPI-R2 (mt7623) with 2 Problems (already reported to Rene, just to inform everyone)...maybe anybody has an idea
- linux-next (i know it's not part of the series, but a pitfall on testing other devices) seems to break power-regulator somewhere here:
priv->core_pwr = devm_regulator_get(&mdiodev->dev, "core"); returns 517
#define EPROBE_DEFER517/* Driver requests probe retry */
https://elixir.bootlin.com/linux/latest/source/drivers/regulator/core.c#L1726
without linux-next switch came up including dsa-ports
- RX-traffic (run iperf3 -c x.x.x.x -R) is only 780 Mbits/sec (TX=940 Mbits/sec), same measure with 5.3-rc4 gives 940 MBit/s with same devices,
maybe caused by changes for mt76x8?
regards Frank
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 04/10] KVM: Implement kvm_put_guest()
From: Steven Price @ 2019-08-22 15:46 UTC (permalink / raw)
To: Sean Christopherson
Cc: Mark Rutland, Radim Krčmář, kvm, Suzuki K Pouloze,
Marc Zyngier, linux-doc, Russell King, linux-kernel, James Morse,
linux-arm-kernel, Catalin Marinas, Paolo Bonzini, Will Deacon,
kvmarm, Julien Thierry
In-Reply-To: <20190822152854.GE25467@linux.intel.com>
On 22/08/2019 16:28, Sean Christopherson wrote:
> On Wed, Aug 21, 2019 at 04:36:50PM +0100, Steven Price wrote:
>> kvm_put_guest() is analogous to put_user() - it writes a single value to
>> the guest physical address. The implementation is built upon put_user()
>> and so it has the same single copy atomic properties.
>
> What you mean by "single copy atomic"? I.e. what guarantees does
> put_user() provide that __copy_to_user() does not?
Single-copy atomicity is defined by the Arm architecture[1] and I'm not
going to try to go into the full details here, so this is a summary.
For the sake of this feature what we care about is that the value
written/read cannot be "torn". In other words if there is a read (in
this case from another VCPU) that is racing with the write then the read
will either get the old value or the new value. It cannot return a
mixture. (This is of course assuming that the read is using a
single-copy atomic safe method).
__copy_to_user() is implemented as a memcpy() and as such cannot provide
single-copy atomicity in the general case (the buffer could easily be
bigger than the architecture can guarantee).
put_user() on the other hand is implemented (on arm64) as an explicit
store instruction and therefore is guaranteed by the architecture to be
single-copy atomic (i.e. another CPU cannot see a half-written value).
Steve
[1] https://static.docs.arm.com/ddi0487/ea/DDI0487E_a_armv8_arm.pdf#page=110
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2] drm: meson: use match data to detect vpu compatibility
From: Neil Armstrong @ 2019-08-22 15:52 UTC (permalink / raw)
To: Julien Masson, Kevin Hilman
Cc: linux-amlogic, dri-devel, linux-arm-kernel, linux-kernel
In-Reply-To: <87imqpz21w.fsf@masson.i-did-not-set--mail-host-address--so-tickle-me>
Hi Julien,
On 22/08/2019 16:43, Julien Masson wrote:
> This patch introduce new enum which contains all VPU family (GXBB,
> GXL, GXM and G12A).
> This enum is used to detect the VPU compatible with the device.
>
> We only need to set .data to the corresponding enum in the device
> table, no need to check .compatible string anymore.
>
> Signed-off-by: Julien Masson <jmasson@baylibre.com>
> ---
>
> Changes since v1 at [1]:
> - Set .data field in struct dt_match instead of connectors_match
> - Add compat in struct meson_drm and set it the corresponding family at probe
> - Check directly compat in meson_vpu_is_compatible
>
> [1] https://patchwork.kernel.org/patch/11108855/
>
> drivers/gpu/drm/meson/meson_crtc.c | 2 +-
> drivers/gpu/drm/meson/meson_drv.c | 14 ++++--
> drivers/gpu/drm/meson/meson_drv.h | 13 ++++-
> drivers/gpu/drm/meson/meson_dw_hdmi.c | 2 +-
> drivers/gpu/drm/meson/meson_overlay.c | 2 +-
> drivers/gpu/drm/meson/meson_plane.c | 10 ++--
> drivers/gpu/drm/meson/meson_vclk.c | 64 ++++++++++++-------------
> drivers/gpu/drm/meson/meson_venc.c | 2 +-
> drivers/gpu/drm/meson/meson_venc_cvbs.c | 10 ++--
> drivers/gpu/drm/meson/meson_viu.c | 10 ++--
> drivers/gpu/drm/meson/meson_vpp.c | 10 ++--
> 11 files changed, 77 insertions(+), 62 deletions(-)
Tested-by: Neil Armstrong <narmstrong@baylibre.com>
on SEI510 (G12A), VIM2 (GXM) and Libretech-AC (GXL)
and
Acked-by: Neil Armstrong <narmstrong@baylibre.com>
Applying to drm-misc-next
Neil
>
> diff --git a/drivers/gpu/drm/meson/meson_crtc.c b/drivers/gpu/drm/meson/meson_crtc.c
> index bba25325aa9c..57ae1c13d1e6 100644
> --- a/drivers/gpu/drm/meson/meson_crtc.c
> +++ b/drivers/gpu/drm/meson/meson_crtc.c
> @@ -575,7 +575,7 @@ int meson_crtc_create(struct meson_drm *priv)
> return ret;
> }
>
> - if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) {
> + if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) {
> meson_crtc->enable_osd1 = meson_g12a_crtc_enable_osd1;
> meson_crtc->enable_vd1 = meson_g12a_crtc_enable_vd1;
> meson_crtc->viu_offset = MESON_G12A_VIU_OFFSET;
> diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
> index ae0166181606..a24f8dec5adc 100644
> --- a/drivers/gpu/drm/meson/meson_drv.c
> +++ b/drivers/gpu/drm/meson/meson_drv.c
> @@ -209,6 +209,8 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
> priv->drm = drm;
> priv->dev = dev;
>
> + priv->compat = (enum vpu_compatible)of_device_get_match_data(priv->dev);
> +
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "vpu");
> regs = devm_ioremap_resource(dev, res);
> if (IS_ERR(regs)) {
> @@ -453,10 +455,14 @@ static int meson_drv_probe(struct platform_device *pdev)
> };
>
> static const struct of_device_id dt_match[] = {
> - { .compatible = "amlogic,meson-gxbb-vpu" },
> - { .compatible = "amlogic,meson-gxl-vpu" },
> - { .compatible = "amlogic,meson-gxm-vpu" },
> - { .compatible = "amlogic,meson-g12a-vpu" },
> + { .compatible = "amlogic,meson-gxbb-vpu",
> + .data = (void *)VPU_COMPATIBLE_GXBB },
> + { .compatible = "amlogic,meson-gxl-vpu",
> + .data = (void *)VPU_COMPATIBLE_GXL },
> + { .compatible = "amlogic,meson-gxm-vpu",
> + .data = (void *)VPU_COMPATIBLE_GXM },
> + { .compatible = "amlogic,meson-g12a-vpu",
> + .data = (void *)VPU_COMPATIBLE_G12A },
> {}
> };
> MODULE_DEVICE_TABLE(of, dt_match);
> diff --git a/drivers/gpu/drm/meson/meson_drv.h b/drivers/gpu/drm/meson/meson_drv.h
> index c9aaec1a846e..820d07bdd42a 100644
> --- a/drivers/gpu/drm/meson/meson_drv.h
> +++ b/drivers/gpu/drm/meson/meson_drv.h
> @@ -9,6 +9,7 @@
>
> #include <linux/device.h>
> #include <linux/of.h>
> +#include <linux/of_device.h>
> #include <linux/regmap.h>
>
> struct drm_crtc;
> @@ -16,8 +17,16 @@ struct drm_device;
> struct drm_plane;
> struct meson_drm;
>
> +enum vpu_compatible {
> + VPU_COMPATIBLE_GXBB = 0,
> + VPU_COMPATIBLE_GXL = 1,
> + VPU_COMPATIBLE_GXM = 2,
> + VPU_COMPATIBLE_G12A = 3,
> +};
> +
> struct meson_drm {
> struct device *dev;
> + enum vpu_compatible compat;
> void __iomem *io_base;
> struct regmap *hhi;
> int vsync_irq;
> @@ -116,9 +125,9 @@ struct meson_drm {
> };
>
> static inline int meson_vpu_is_compatible(struct meson_drm *priv,
> - const char *compat)
> + enum vpu_compatible family)
> {
> - return of_device_is_compatible(priv->dev->of_node, compat);
> + return priv->compat == family;
> }
>
> #endif /* __MESON_DRV_H */
> diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> index f893ebd0b799..68bbd987147b 100644
> --- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
> +++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> @@ -937,7 +937,7 @@ static int meson_dw_hdmi_bind(struct device *dev, struct device *master,
> reset_control_reset(meson_dw_hdmi->hdmitx_phy);
>
> /* Enable APB3 fail on error */
> - if (!meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) {
> + if (!meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) {
> writel_bits_relaxed(BIT(15), BIT(15),
> meson_dw_hdmi->hdmitx + HDMITX_TOP_CTRL_REG);
> writel_bits_relaxed(BIT(15), BIT(15),
> diff --git a/drivers/gpu/drm/meson/meson_overlay.c b/drivers/gpu/drm/meson/meson_overlay.c
> index 5aa9dcb4b35e..2468b0212d52 100644
> --- a/drivers/gpu/drm/meson/meson_overlay.c
> +++ b/drivers/gpu/drm/meson/meson_overlay.c
> @@ -513,7 +513,7 @@ static void meson_overlay_atomic_disable(struct drm_plane *plane,
> priv->viu.vd1_enabled = false;
>
> /* Disable VD1 */
> - if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) {
> + if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) {
> writel_relaxed(0, priv->io_base + _REG(VD1_BLEND_SRC_CTRL));
> writel_relaxed(0, priv->io_base + _REG(VD2_BLEND_SRC_CTRL));
> writel_relaxed(0, priv->io_base + _REG(VD1_IF0_GEN_REG + 0x17b0));
> diff --git a/drivers/gpu/drm/meson/meson_plane.c b/drivers/gpu/drm/meson/meson_plane.c
> index b9e1e117fb85..ed543227b00d 100644
> --- a/drivers/gpu/drm/meson/meson_plane.c
> +++ b/drivers/gpu/drm/meson/meson_plane.c
> @@ -138,7 +138,7 @@ static void meson_plane_atomic_update(struct drm_plane *plane,
> OSD_ENDIANNESS_LE);
>
> /* On GXBB, Use the old non-HDR RGB2YUV converter */
> - if (meson_vpu_is_compatible(priv, "amlogic,meson-gxbb-vpu"))
> + if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXBB))
> priv->viu.osd1_blk0_cfg[0] |= OSD_OUTPUT_COLOR_RGB;
>
> switch (fb->format->format) {
> @@ -292,7 +292,7 @@ static void meson_plane_atomic_update(struct drm_plane *plane,
> priv->viu.osd1_blk0_cfg[3] = ((dest.x2 - 1) << 16) | dest.x1;
> priv->viu.osd1_blk0_cfg[4] = ((dest.y2 - 1) << 16) | dest.y1;
>
> - if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) {
> + if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) {
> priv->viu.osd_blend_din0_scope_h = ((dest.x2 - 1) << 16) | dest.x1;
> priv->viu.osd_blend_din0_scope_v = ((dest.y2 - 1) << 16) | dest.y1;
> priv->viu.osb_blend0_size = dst_h << 16 | dst_w;
> @@ -308,8 +308,8 @@ static void meson_plane_atomic_update(struct drm_plane *plane,
>
> if (!meson_plane->enabled) {
> /* Reset OSD1 before enabling it on GXL+ SoCs */
> - if (meson_vpu_is_compatible(priv, "amlogic,meson-gxm-vpu") ||
> - meson_vpu_is_compatible(priv, "amlogic,meson-gxl-vpu"))
> + if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXM) ||
> + meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXL))
> meson_viu_osd1_reset(priv);
>
> meson_plane->enabled = true;
> @@ -327,7 +327,7 @@ static void meson_plane_atomic_disable(struct drm_plane *plane,
> struct meson_drm *priv = meson_plane->priv;
>
> /* Disable OSD1 */
> - if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu"))
> + if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A))
> writel_bits_relaxed(VIU_OSD1_POSTBLD_SRC_OSD1, 0,
> priv->io_base + _REG(OSD1_BLEND_SRC_CTRL));
> else
> diff --git a/drivers/gpu/drm/meson/meson_vclk.c b/drivers/gpu/drm/meson/meson_vclk.c
> index 869231c93617..ac491a781952 100644
> --- a/drivers/gpu/drm/meson/meson_vclk.c
> +++ b/drivers/gpu/drm/meson/meson_vclk.c
> @@ -242,7 +242,7 @@ static void meson_venci_cvbs_clock_config(struct meson_drm *priv)
> unsigned int val;
>
> /* Setup PLL to output 1.485GHz */
> - if (meson_vpu_is_compatible(priv, "amlogic,meson-gxbb-vpu")) {
> + if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXBB)) {
> regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL, 0x5800023d);
> regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL2, 0x00404e00);
> regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL3, 0x0d5c5091);
> @@ -254,8 +254,8 @@ static void meson_venci_cvbs_clock_config(struct meson_drm *priv)
> /* Poll for lock bit */
> regmap_read_poll_timeout(priv->hhi, HHI_HDMI_PLL_CNTL, val,
> (val & HDMI_PLL_LOCK), 10, 0);
> - } else if (meson_vpu_is_compatible(priv, "amlogic,meson-gxm-vpu") ||
> - meson_vpu_is_compatible(priv, "amlogic,meson-gxl-vpu")) {
> + } else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXM) ||
> + meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXL)) {
> regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL, 0x4000027b);
> regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL2, 0x800cb300);
> regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL3, 0xa6212844);
> @@ -272,7 +272,7 @@ static void meson_venci_cvbs_clock_config(struct meson_drm *priv)
> /* Poll for lock bit */
> regmap_read_poll_timeout(priv->hhi, HHI_HDMI_PLL_CNTL, val,
> (val & HDMI_PLL_LOCK), 10, 0);
> - } else if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) {
> + } else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) {
> regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL, 0x1a0504f7);
> regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL2, 0x00010000);
> regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL3, 0x00000000);
> @@ -300,7 +300,7 @@ static void meson_venci_cvbs_clock_config(struct meson_drm *priv)
> VCLK2_DIV_MASK, (55 - 1));
>
> /* select vid_pll for vclk2 */
> - if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu"))
> + if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A))
> regmap_update_bits(priv->hhi, HHI_VIID_CLK_CNTL,
> VCLK2_SEL_MASK, (0 << VCLK2_SEL_SHIFT));
> else
> @@ -455,7 +455,7 @@ void meson_hdmi_pll_set_params(struct meson_drm *priv, unsigned int m,
> {
> unsigned int val;
>
> - if (meson_vpu_is_compatible(priv, "amlogic,meson-gxbb-vpu")) {
> + if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXBB)) {
> regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL, 0x58000200 | m);
> if (frac)
> regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL2,
> @@ -475,8 +475,8 @@ void meson_hdmi_pll_set_params(struct meson_drm *priv, unsigned int m,
> /* Poll for lock bit */
> regmap_read_poll_timeout(priv->hhi, HHI_HDMI_PLL_CNTL,
> val, (val & HDMI_PLL_LOCK), 10, 0);
> - } else if (meson_vpu_is_compatible(priv, "amlogic,meson-gxm-vpu") ||
> - meson_vpu_is_compatible(priv, "amlogic,meson-gxl-vpu")) {
> + } else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXM) ||
> + meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXL)) {
> regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL, 0x40000200 | m);
> regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL2, 0x800cb000 | frac);
> regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL3, 0x860f30c4);
> @@ -493,7 +493,7 @@ void meson_hdmi_pll_set_params(struct meson_drm *priv, unsigned int m,
> /* Poll for lock bit */
> regmap_read_poll_timeout(priv->hhi, HHI_HDMI_PLL_CNTL, val,
> (val & HDMI_PLL_LOCK), 10, 0);
> - } else if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) {
> + } else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) {
> regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL, 0x0b3a0400 | m);
>
> /* Enable and reset */
> @@ -545,36 +545,36 @@ void meson_hdmi_pll_set_params(struct meson_drm *priv, unsigned int m,
> } while(1);
> }
>
> - if (meson_vpu_is_compatible(priv, "amlogic,meson-gxbb-vpu"))
> + if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXBB))
> regmap_update_bits(priv->hhi, HHI_HDMI_PLL_CNTL2,
> 3 << 16, pll_od_to_reg(od1) << 16);
> - else if (meson_vpu_is_compatible(priv, "amlogic,meson-gxm-vpu") ||
> - meson_vpu_is_compatible(priv, "amlogic,meson-gxl-vpu"))
> + else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXM) ||
> + meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXL))
> regmap_update_bits(priv->hhi, HHI_HDMI_PLL_CNTL3,
> 3 << 21, pll_od_to_reg(od1) << 21);
> - else if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu"))
> + else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A))
> regmap_update_bits(priv->hhi, HHI_HDMI_PLL_CNTL,
> 3 << 16, pll_od_to_reg(od1) << 16);
>
> - if (meson_vpu_is_compatible(priv, "amlogic,meson-gxbb-vpu"))
> + if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXBB))
> regmap_update_bits(priv->hhi, HHI_HDMI_PLL_CNTL2,
> 3 << 22, pll_od_to_reg(od2) << 22);
> - else if (meson_vpu_is_compatible(priv, "amlogic,meson-gxm-vpu") ||
> - meson_vpu_is_compatible(priv, "amlogic,meson-gxl-vpu"))
> + else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXM) ||
> + meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXL))
> regmap_update_bits(priv->hhi, HHI_HDMI_PLL_CNTL3,
> 3 << 23, pll_od_to_reg(od2) << 23);
> - else if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu"))
> + else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A))
> regmap_update_bits(priv->hhi, HHI_HDMI_PLL_CNTL,
> 3 << 18, pll_od_to_reg(od2) << 18);
>
> - if (meson_vpu_is_compatible(priv, "amlogic,meson-gxbb-vpu"))
> + if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXBB))
> regmap_update_bits(priv->hhi, HHI_HDMI_PLL_CNTL2,
> 3 << 18, pll_od_to_reg(od3) << 18);
> - else if (meson_vpu_is_compatible(priv, "amlogic,meson-gxm-vpu") ||
> - meson_vpu_is_compatible(priv, "amlogic,meson-gxl-vpu"))
> + else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXM) ||
> + meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXL))
> regmap_update_bits(priv->hhi, HHI_HDMI_PLL_CNTL3,
> 3 << 19, pll_od_to_reg(od3) << 19);
> - else if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu"))
> + else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A))
> regmap_update_bits(priv->hhi, HHI_HDMI_PLL_CNTL,
> 3 << 20, pll_od_to_reg(od3) << 20);
> }
> @@ -585,7 +585,7 @@ static unsigned int meson_hdmi_pll_get_m(struct meson_drm *priv,
> unsigned int pll_freq)
> {
> /* The GXBB PLL has a /2 pre-multiplier */
> - if (meson_vpu_is_compatible(priv, "amlogic,meson-gxbb-vpu"))
> + if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXBB))
> pll_freq /= 2;
>
> return pll_freq / XTAL_FREQ;
> @@ -605,12 +605,12 @@ static unsigned int meson_hdmi_pll_get_frac(struct meson_drm *priv,
> unsigned int frac;
>
> /* The GXBB PLL has a /2 pre-multiplier and a larger FRAC width */
> - if (meson_vpu_is_compatible(priv, "amlogic,meson-gxbb-vpu")) {
> + if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXBB)) {
> frac_max = HDMI_FRAC_MAX_GXBB;
> parent_freq *= 2;
> }
>
> - if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu"))
> + if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A))
> frac_max = HDMI_FRAC_MAX_G12A;
>
> /* We can have a perfect match !*/
> @@ -631,15 +631,15 @@ static bool meson_hdmi_pll_validate_params(struct meson_drm *priv,
> unsigned int m,
> unsigned int frac)
> {
> - if (meson_vpu_is_compatible(priv, "amlogic,meson-gxbb-vpu")) {
> + if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXBB)) {
> /* Empiric supported min/max dividers */
> if (m < 53 || m > 123)
> return false;
> if (frac >= HDMI_FRAC_MAX_GXBB)
> return false;
> - } else if (meson_vpu_is_compatible(priv, "amlogic,meson-gxm-vpu") ||
> - meson_vpu_is_compatible(priv, "amlogic,meson-gxl-vpu") ||
> - meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) {
> + } else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXM) ||
> + meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXL) ||
> + meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) {
> /* Empiric supported min/max dividers */
> if (m < 106 || m > 247)
> return false;
> @@ -759,7 +759,7 @@ static void meson_vclk_set(struct meson_drm *priv, unsigned int pll_base_freq,
> /* Set HDMI PLL rate */
> if (!od1 && !od2 && !od3) {
> meson_hdmi_pll_generic_set(priv, pll_base_freq);
> - } else if (meson_vpu_is_compatible(priv, "amlogic,meson-gxbb-vpu")) {
> + } else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXBB)) {
> switch (pll_base_freq) {
> case 2970000:
> m = 0x3d;
> @@ -776,8 +776,8 @@ static void meson_vclk_set(struct meson_drm *priv, unsigned int pll_base_freq,
> }
>
> meson_hdmi_pll_set_params(priv, m, frac, od1, od2, od3);
> - } else if (meson_vpu_is_compatible(priv, "amlogic,meson-gxm-vpu") ||
> - meson_vpu_is_compatible(priv, "amlogic,meson-gxl-vpu")) {
> + } else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXM) ||
> + meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXL)) {
> switch (pll_base_freq) {
> case 2970000:
> m = 0x7b;
> @@ -794,7 +794,7 @@ static void meson_vclk_set(struct meson_drm *priv, unsigned int pll_base_freq,
> }
>
> meson_hdmi_pll_set_params(priv, m, frac, od1, od2, od3);
> - } else if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) {
> + } else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) {
> switch (pll_base_freq) {
> case 2970000:
> m = 0x7b;
> diff --git a/drivers/gpu/drm/meson/meson_venc.c b/drivers/gpu/drm/meson/meson_venc.c
> index 679d2274531c..4efd7864d5bf 100644
> --- a/drivers/gpu/drm/meson/meson_venc.c
> +++ b/drivers/gpu/drm/meson/meson_venc.c
> @@ -1759,7 +1759,7 @@ void meson_venc_disable_vsync(struct meson_drm *priv)
> void meson_venc_init(struct meson_drm *priv)
> {
> /* Disable CVBS VDAC */
> - if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) {
> + if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) {
> regmap_write(priv->hhi, HHI_VDAC_CNTL0_G12A, 0);
> regmap_write(priv->hhi, HHI_VDAC_CNTL1_G12A, 8);
> } else {
> diff --git a/drivers/gpu/drm/meson/meson_venc_cvbs.c b/drivers/gpu/drm/meson/meson_venc_cvbs.c
> index 6dc130a24070..9ab27aecfcf3 100644
> --- a/drivers/gpu/drm/meson/meson_venc_cvbs.c
> +++ b/drivers/gpu/drm/meson/meson_venc_cvbs.c
> @@ -155,7 +155,7 @@ static void meson_venc_cvbs_encoder_disable(struct drm_encoder *encoder)
> struct meson_drm *priv = meson_venc_cvbs->priv;
>
> /* Disable CVBS VDAC */
> - if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) {
> + if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) {
> regmap_write(priv->hhi, HHI_VDAC_CNTL0_G12A, 0);
> regmap_write(priv->hhi, HHI_VDAC_CNTL1_G12A, 0);
> } else {
> @@ -174,14 +174,14 @@ static void meson_venc_cvbs_encoder_enable(struct drm_encoder *encoder)
> writel_bits_relaxed(VENC_VDAC_SEL_ATV_DMD, 0,
> priv->io_base + _REG(VENC_VDAC_DACSEL0));
>
> - if (meson_vpu_is_compatible(priv, "amlogic,meson-gxbb-vpu")) {
> + if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXBB)) {
> regmap_write(priv->hhi, HHI_VDAC_CNTL0, 1);
> regmap_write(priv->hhi, HHI_VDAC_CNTL1, 0);
> - } else if (meson_vpu_is_compatible(priv, "amlogic,meson-gxm-vpu") ||
> - meson_vpu_is_compatible(priv, "amlogic,meson-gxl-vpu")) {
> + } else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXM) ||
> + meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXL)) {
> regmap_write(priv->hhi, HHI_VDAC_CNTL0, 0xf0001);
> regmap_write(priv->hhi, HHI_VDAC_CNTL1, 0);
> - } else if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) {
> + } else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) {
> regmap_write(priv->hhi, HHI_VDAC_CNTL0_G12A, 0x906001);
> regmap_write(priv->hhi, HHI_VDAC_CNTL1_G12A, 0);
> }
> diff --git a/drivers/gpu/drm/meson/meson_viu.c b/drivers/gpu/drm/meson/meson_viu.c
> index e70cd55d56c9..68cf2c2eca5f 100644
> --- a/drivers/gpu/drm/meson/meson_viu.c
> +++ b/drivers/gpu/drm/meson/meson_viu.c
> @@ -353,10 +353,10 @@ void meson_viu_init(struct meson_drm *priv)
> priv->io_base + _REG(VIU_OSD2_CTRL_STAT));
>
> /* On GXL/GXM, Use the 10bit HDR conversion matrix */
> - if (meson_vpu_is_compatible(priv, "amlogic,meson-gxm-vpu") ||
> - meson_vpu_is_compatible(priv, "amlogic,meson-gxl-vpu"))
> + if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXM) ||
> + meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXL))
> meson_viu_load_matrix(priv);
> - else if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu"))
> + else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A))
> meson_viu_set_g12a_osd1_matrix(priv, RGB709_to_YUV709l_coeff,
> true);
>
> @@ -367,7 +367,7 @@ void meson_viu_init(struct meson_drm *priv)
> VIU_OSD_WORDS_PER_BURST(4) | /* 4 words in 1 burst */
> VIU_OSD_FIFO_LIMITS(2); /* fifo_lim: 2*16=32 */
>
> - if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu"))
> + if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A))
> reg |= meson_viu_osd_burst_length_reg(32);
> else
> reg |= meson_viu_osd_burst_length_reg(64);
> @@ -394,7 +394,7 @@ void meson_viu_init(struct meson_drm *priv)
> writel_relaxed(0x00FF00C0,
> priv->io_base + _REG(VD2_IF0_LUMA_FIFO_SIZE));
>
> - if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) {
> + if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) {
> writel_relaxed(VIU_OSD_BLEND_REORDER(0, 1) |
> VIU_OSD_BLEND_REORDER(1, 0) |
> VIU_OSD_BLEND_REORDER(2, 0) |
> diff --git a/drivers/gpu/drm/meson/meson_vpp.c b/drivers/gpu/drm/meson/meson_vpp.c
> index 1429f3be6028..154837688ab0 100644
> --- a/drivers/gpu/drm/meson/meson_vpp.c
> +++ b/drivers/gpu/drm/meson/meson_vpp.c
> @@ -91,20 +91,20 @@ static void meson_vpp_write_vd_scaling_filter_coefs(struct meson_drm *priv,
> void meson_vpp_init(struct meson_drm *priv)
> {
> /* set dummy data default YUV black */
> - if (meson_vpu_is_compatible(priv, "amlogic,meson-gxl-vpu"))
> + if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXL))
> writel_relaxed(0x108080, priv->io_base + _REG(VPP_DUMMY_DATA1));
> - else if (meson_vpu_is_compatible(priv, "amlogic,meson-gxm-vpu")) {
> + else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXM)) {
> writel_bits_relaxed(0xff << 16, 0xff << 16,
> priv->io_base + _REG(VIU_MISC_CTRL1));
> writel_relaxed(VPP_PPS_DUMMY_DATA_MODE,
> priv->io_base + _REG(VPP_DOLBY_CTRL));
> writel_relaxed(0x1020080,
> priv->io_base + _REG(VPP_DUMMY_DATA1));
> - } else if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu"))
> + } else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A))
> writel_relaxed(0xf, priv->io_base + _REG(DOLBY_PATH_CTRL));
>
> /* Initialize vpu fifo control registers */
> - if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu"))
> + if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A))
> writel_relaxed(VPP_OFIFO_SIZE_DEFAULT,
> priv->io_base + _REG(VPP_OFIFO_SIZE));
> else
> @@ -113,7 +113,7 @@ void meson_vpp_init(struct meson_drm *priv)
> writel_relaxed(VPP_POSTBLEND_HOLD_LINES(4) | VPP_PREBLEND_HOLD_LINES(4),
> priv->io_base + _REG(VPP_HOLD_LINES));
>
> - if (!meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) {
> + if (!meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) {
> /* Turn off preblend */
> writel_bits_relaxed(VPP_PREBLEND_ENABLE, 0,
> priv->io_base + _REG(VPP_MISC));
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v9 3/3] arm64: Relax Documentation/arm64/tagged-pointers.rst
From: Catalin Marinas @ 2019-08-22 15:55 UTC (permalink / raw)
To: Dave Martin
Cc: linux-arch, linux-doc, Szabolcs Nagy, Andrey Konovalov,
Kevin Brodsky, Will Deacon, linux-mm, Dave Hansen, Andrew Morton,
Vincenzo Frascino, Will Deacon, linux-arm-kernel
In-Reply-To: <20190821184649.GD27757@arm.com>
On Wed, Aug 21, 2019 at 07:46:51PM +0100, Dave P Martin wrote:
> On Wed, Aug 21, 2019 at 06:33:53PM +0100, Will Deacon wrote:
> > On Wed, Aug 21, 2019 at 05:47:30PM +0100, Catalin Marinas wrote:
> > > @@ -59,6 +63,11 @@ be preserved.
> > > The architecture prevents the use of a tagged PC, so the upper byte will
> > > be set to a sign-extension of bit 55 on exception return.
> > >
> > > +This behaviour is maintained when the AArch64 Tagged Address ABI is
> > > +enabled. In addition, with the exceptions above, the kernel will
> > > +preserve any non-zero tags passed by the user via syscalls and stored in
> > > +kernel data structures (e.g. ``set_robust_list()``, ``sigaltstack()``).
>
> sigaltstack() is interesting, since we don't support tagged stacks.
We should support tagged SP with the new ABI as they'll be required for
MTE. sigaltstack() and clone() are the two syscalls that come to mind
here.
> Do we keep the ss_sp tag in the kernel, but squash it when delivering
> a signal to the alternate stack?
We don't seem to be doing any untagging, so we just just use whatever
the caller asked for. We may need a small test to confirm.
That said, on_sig_stack() probably needs some untagging as it does user
pointer arithmetics with potentially different tags.
> > Hmm. I can see the need to provide this guarantee for things like
> > set_robust_list(), but the problem is that the statement above is too broad
> > and isn't strictly true: for example, mmap() doesn't propagate the tag of
> > its address parameter into the VMA.
> >
> > So I think we need to nail this down a bit more, but I'm having a really
> > hard time coming up with some wording :(
>
> Time for some creative vagueness?
>
> We can write a statement of our overall intent, along with examples of
> a few cases where the tag should and should not be expected to emerge
> intact.
>
> There is no foolproof rule, unless we can rewrite history...
I would expect the norm to be the preservation of tags with a few
exceptions. The only ones I think where we won't preserve the tags are
mmap, mremap, brk (apart from the signal stuff already mentioned in the
current tagged-pointers.rst doc).
So I can remove this paragraph altogether and add a note in part 3 of
the tagged-address-abi.rst document that mmap/mremap/brk do not preserve
the tag information.
--
Catalin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 3/3] watchdog/aspeed: add support for dual boot
From: Guenter Roeck @ 2019-08-22 16:01 UTC (permalink / raw)
To: Alexander Amelkin
Cc: linux-watchdog, linux-aspeed, Andrew Jeffery, linux-kernel,
Joel Stanley, Ivan Mikhaylov, Wim Van Sebroeck, linux-arm-kernel
In-Reply-To: <5cb20f52-884a-b921-c904-ebf244092318@yadro.com>
On Thu, Aug 22, 2019 at 05:36:21PM +0300, Alexander Amelkin wrote:
> 21.08.2019 21:10, Guenter Roeck wrote:
> > On Wed, Aug 21, 2019 at 08:42:24PM +0300, Alexander Amelkin wrote:
> >> 21.08.2019 19:32, Guenter Roeck wrote:
> >>> On Wed, Aug 21, 2019 at 06:57:43PM +0300, Ivan Mikhaylov wrote:
> >>>> Set WDT_CLEAR_TIMEOUT_AND_BOOT_CODE_SELECTION into WDT_CLEAR_TIMEOUT_STATUS
> >>>> to clear out boot code source and re-enable access to the primary SPI flash
> >>>> chip while booted via wdt2 from the alternate chip.
> >>>>
> >>>> AST2400 datasheet says:
> >>>> "In the 2nd flash booting mode, all the address mapping to CS0# would be
> >>>> re-directed to CS1#. And CS0# is not accessable under this mode. To access
> >>>> CS0#, firmware should clear the 2nd boot mode register in the WDT2 status
> >>>> register WDT30.bit[1]."
> >>> Is there reason to not do this automatically when loading the module
> >>> in alt-boot mode ? What means does userspace have to determine if CS0
> >>> or CS1 is active at any given time ? If there is reason to ever have CS1
> >>> active instead of CS0, what means would userspace have to enable it ?
> >> Yes, there is. The driver is loaded long before the filesystems are mounted.
> >> The filesystems, in the event of alternate/recovery boot, need to be mounted
> >> from the same chip that the kernel was booted. For one reason because the main
> >> chip at CS0 is most probably corrupt. If you clear that bit when driver is
> >> loaded, your software will not know that and will try to mount the wrong
> >> filesystems. The whole idea of ASPEED's switching chipselects is to have
> >> identical firmware in both chips, without the need to process the alternate
> >> boot state in any way except for indicating a successful boot and restoring
> >> access to CS0 when needed.
> >>
> >> The userspace can read bootstatus sysfs node to determine if an alternate
> >> boot has occured.
> >>
> >> With ASPEED, CS1 is activated automatically by wdt2 when system fails to boot
> >> from the primary flash chip (at CS0) and disable the watchdog to indicate a
> >> successful boot. When that happens, both CS0 and CS1 controls get routed in
> >> hardware to CS1 line, making the primary flash chip inaccessible. Depending
> >> on the architecture of the user-space software, it may choose to re-enable
> >> access to the primary chip via CS0 at different times. There must be a way to do so.
> >>
> > So by activating cs0, userspace would essentially pull its own root file system
> > from underneath itself ?
>
> Exactly. That's why for alternate boot the firmware would usually copy
> all filesystems to memory and mount from there. Some embedded systems
> do that always, regardless of which chip they boot from.
>
That is different, though, to what you said earlier. Linux would then start
with a clean file system, and not need access to the file system in cs1 at all.
Clearing the flag when starting the driver would then be ok.
> However, to be able to recover the main flash chip, the system needs CS0
> to function as such (not as CS1). That's why this control is needed.
>
If what you said is correct, not really. It should be fine and create more
predictive behavior if the probe function selects cs0 automatically.
Guenter
> As Ivan mentioned, for AST2500 and the upcoming AST2600 the behavior
> is slightly different. They don't just connect both CS controls to CS1 but instead
> swap them so the primary chip becomes secondary from the software point
> of view. The means to restore the normal wiring may still be needed.
>
> >
> >> This code most probably adds nothing at the assembly level.
> >>
> > That seems quite unlikely. Please demonstrate.
>
> Yes, you were right. It adds 7 instructions. We'll drop the check.
> It's just my DO-178 background, I add 'robustness' checks everywhere.
>
> >>>> + writel(WDT_CLEAR_TIMEOUT_AND_BOOT_CODE_SELECTION,
> >>>> + wdt->base + WDT_CLEAR_TIMEOUT_STATUS);
> >>>> + wdt->wdd.bootstatus |= WDIOF_EXTERN1;
> >>> The variable reflects the _boot status_. It should not change after booting.
> >> Is there any documentation that dictates that? All I could find is
> >>
> >> "bootstatus: status of the device after booting". That doesn't look to me like it absolutely can not change to reflect the updated status (that is, to reflect that the originally set up alternate CS routing has been reset to normal).
> >>
> > You choose to interpret "after booting" in a kind of novel way,
> > which I find a bit disturbing. I am not really sure how else to
> > describe "boot status" in a way that does not permit such
> > reinterpratation of the term.
>
> How about "Reflects reasons that caused a reboot, remains constant until the next boot" ?
>
> > On top of that, how specifically would "WDIOF_EXTERN1" reflect
> > what you claim it does ? Not only you are hijacking bootstatus9
> > (which is supposed to describe the reason for a reboot), you
> > are also hijacking WDIOF_EXTERN1. That seems highly arbitrary
> > to me, and is not really how an API/ABI should be used.
>
> We used WDIOF_EXTERN1 because:
>
> 1. We thought that bootstatus _can_ change
>
> 2. We thought that adding extra bits wouldn't be appreciated
>
> Now as you clarified that assumption 1 was wrong we are going to implement status as I proposed earlier:
>
> >
> >> I think we could make 'access_cs0' readable instead, so it could report the
> >> current state of the boot code selection bit. Reverted, I suppose. That
> >> way 'access_cs0' would report 1 after 1 has been written to it (it wouldn't
> >> be possible to write a zero).
>
> With best regards,
> Alexander Amelkin,
> BIOS/BMC Team Lead, YADRO
> https://yadro.com
>
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v9 2/3] fdt: add support for rng-seed
From: Theodore Y. Ts'o @ 2019-08-22 16:03 UTC (permalink / raw)
To: Hsin-Yi Wang
Cc: Kate Stewart, Peter Zijlstra, Catalin Marinas, Mukesh Ojha,
Grzegorz Halat, H . Peter Anvin, Guenter Roeck, Will Deacon,
Marek Szyprowski, Rob Herring, Daniel Thompson, Anders Roxell,
Yury Norov, Marc Zyngier, Russell King, Aaro Koskinen,
Ingo Molnar, Viresh Kumar, Waiman Long, Paul E . McKenney, Wei Li,
Alexey Dobriyan, Julien Thierry, Len Brown, Kees Cook,
Arnd Bergmann, Rik van Riel, Stephen Boyd, Shaokun Zhang,
Mike Rapoport, Borislav Petkov, Josh Poimboeuf, Thomas Gleixner,
linux-arm-kernel, Greg Kroah-Hartman, Marcelo Tosatti,
linux-kernel, Armijn Hemel, Jiri Kosina, Mathieu Desnoyers,
Andrew Morton, Tim Chen, David S . Miller
In-Reply-To: <20190822071522.143986-3-hsinyi@chromium.org>
On Thu, Aug 22, 2019 at 03:15:22PM +0800, Hsin-Yi Wang wrote:
> Introducing a chosen node, rng-seed, which is an entropy that can be
> passed to kernel called very early to increase initial device
> randomness. Bootloader should provide this entropy and the value is
> read from /chosen/rng-seed in DT.
>
> Obtain of_fdt_crc32 for CRC check after early_init_dt_scan_nodes(),
> since early_init_dt_scan_chosen() would modify fdt to erase rng-seed.
>
> Add a new interface add_bootloader_randomness() for rng-seed use case.
> Depends on whether the seed is trustworthy, rng seed would be passed to
> add_hwgenerator_randomness(). Otherwise it would be passed to
> add_device_randomness(). Decision is controlled by kernel config
> RANDOM_TRUST_BOOTLOADER.
>
> Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> Reviewed-by: Stephen Boyd <swboyd@chromium.org>
> Reviewed-by: Rob Herring <robh@kernel.org>
For the changes to drivers/char/random.c:
Reviewed-by: Theodore Ts'o <tytso@mit.edu>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 05/12] irqchip/gic: Prepare for more than 16 PPIs
From: Julien @ 2019-08-22 16:11 UTC (permalink / raw)
To: Marc Zyngier, Thomas Gleixner, Jason Cooper, Rob Herring
Cc: Lokesh Vutla, John Garry, linux-kernel, Shameerali Kolothum Thodi,
linux-arm-kernel
In-Reply-To: <20190806100121.240767-6-maz@kernel.org>
Hi Marc,
On 06/08/19 11:01, Marc Zyngier wrote:
> GICv3.1 allows up to 80 PPIs (16 legaci PPIs and 64 Extended PPIs),
> meaning we can't just leave the old 16 hardcoded everywhere.
>
> We also need to add the infrastructure to discover the number of PPIs
> on a per redistributor basis, although we still pretend there is only
> 16 of them for now.
>
> No functional change.
>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
> drivers/irqchip/irq-gic-common.c | 19 ++++++++++++-------
> drivers/irqchip/irq-gic-common.h | 2 +-
> drivers/irqchip/irq-gic-v3.c | 22 +++++++++++++++-------
> drivers/irqchip/irq-gic.c | 2 +-
> drivers/irqchip/irq-hip04.c | 2 +-
> 5 files changed, 30 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/irqchip/irq-gic-common.c b/drivers/irqchip/irq-gic-common.c
> index 6900b6f0921c..14110db01c05 100644
> --- a/drivers/irqchip/irq-gic-common.c
> +++ b/drivers/irqchip/irq-gic-common.c
> @@ -128,26 +128,31 @@ void gic_dist_config(void __iomem *base, int gic_irqs,
> sync_access();
> }
>
> -void gic_cpu_config(void __iomem *base, void (*sync_access)(void))
> +void gic_cpu_config(void __iomem *base, int nr, void (*sync_access)(void))
> {
> int i;
>
> /*
> * Deal with the banked PPI and SGI interrupts - disable all
> - * PPI interrupts, ensure all SGI interrupts are enabled.
> - * Make sure everything is deactivated.
> + * private interrupts. Make sure everything is deactivated.
> */
> - writel_relaxed(GICD_INT_EN_CLR_X32, base + GIC_DIST_ACTIVE_CLEAR);
> - writel_relaxed(GICD_INT_EN_CLR_PPI, base + GIC_DIST_ENABLE_CLEAR);
> - writel_relaxed(GICD_INT_EN_SET_SGI, base + GIC_DIST_ENABLE_SET);
> + for (i = 0; i < nr; i += 32) {
You added "nr" as argument but if "nr" isn't a multiple of 32 weird
things might happen, no?
It would be worth specifying that somewhere, and checking it with a WARN().
Maybe it might be worth reducing the granularity to manipulating 16 irqs
since there are 16 SGI + 16 PPI + 64 EPPI, but that might not be very
useful right now.
Cheers,
Julien
> + writel_relaxed(GICD_INT_EN_CLR_X32,
> + base + GIC_DIST_ACTIVE_CLEAR + i / 8);
> + writel_relaxed(GICD_INT_EN_CLR_X32,
> + base + GIC_DIST_ENABLE_CLEAR + i / 8);
> + }
>
> /*
> * Set priority on PPI and SGI interrupts
> */
> - for (i = 0; i < 32; i += 4)
> + for (i = 0; i < nr; i += 4)
> writel_relaxed(GICD_INT_DEF_PRI_X4,
> base + GIC_DIST_PRI + i * 4 / 4);
>
> + /* Ensure all SGI interrupts are now enabled */
> + writel_relaxed(GICD_INT_EN_SET_SGI, base + GIC_DIST_ENABLE_SET);
> +
> if (sync_access)
> sync_access();
> }
> diff --git a/drivers/irqchip/irq-gic-common.h b/drivers/irqchip/irq-gic-common.h
> index 5a46b6b57750..ccba8b0fe0f5 100644
> --- a/drivers/irqchip/irq-gic-common.h
> +++ b/drivers/irqchip/irq-gic-common.h
> @@ -22,7 +22,7 @@ int gic_configure_irq(unsigned int irq, unsigned int type,
> void __iomem *base, void (*sync_access)(void));
> void gic_dist_config(void __iomem *base, int gic_irqs,
> void (*sync_access)(void));
> -void gic_cpu_config(void __iomem *base, void (*sync_access)(void));
> +void gic_cpu_config(void __iomem *base, int nr, void (*sync_access)(void));
> void gic_enable_quirks(u32 iidr, const struct gic_quirk *quirks,
> void *data);
> void gic_enable_of_quirks(const struct device_node *np,
> diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
> index 1ca4dde32034..e03fb6d7c2ce 100644
> --- a/drivers/irqchip/irq-gic-v3.c
> +++ b/drivers/irqchip/irq-gic-v3.c
> @@ -51,6 +51,7 @@ struct gic_chip_data {
> u32 nr_redist_regions;
> u64 flags;
> bool has_rss;
> + unsigned int ppi_nr;
> struct partition_desc *ppi_descs[16];
> };
>
> @@ -812,19 +813,24 @@ static int gic_populate_rdist(void)
> return -ENODEV;
> }
>
> -static int __gic_update_vlpi_properties(struct redist_region *region,
> - void __iomem *ptr)
> +static int __gic_update_rdist_properties(struct redist_region *region,
> + void __iomem *ptr)
> {
> u64 typer = gic_read_typer(ptr + GICR_TYPER);
> gic_data.rdists.has_vlpis &= !!(typer & GICR_TYPER_VLPIS);
> gic_data.rdists.has_direct_lpi &= !!(typer & GICR_TYPER_DirectLPIS);
> + gic_data.ppi_nr = 16;
>
> return 1;
> }
>
> -static void gic_update_vlpi_properties(void)
> +static void gic_update_rdist_properties(void)
> {
> - gic_iterate_rdists(__gic_update_vlpi_properties);
> + gic_data.ppi_nr = UINT_MAX;
> + gic_iterate_rdists(__gic_update_rdist_properties);
> + if (WARN_ON(gic_data.ppi_nr == UINT_MAX))
> + gic_data.ppi_nr = 0;
> + pr_info("%d PPIs implemented\n", gic_data.ppi_nr);
> pr_info("%sVLPI support, %sdirect LPI support\n",
> !gic_data.rdists.has_vlpis ? "no " : "",
> !gic_data.rdists.has_direct_lpi ? "no " : "");
> @@ -968,6 +974,7 @@ static int gic_dist_supports_lpis(void)
> static void gic_cpu_init(void)
> {
> void __iomem *rbase;
> + int i;
>
> /* Register ourselves with the rest of the world */
> if (gic_populate_rdist())
> @@ -978,9 +985,10 @@ static void gic_cpu_init(void)
> rbase = gic_data_rdist_sgi_base();
>
> /* Configure SGIs/PPIs as non-secure Group-1 */
> - writel_relaxed(~0, rbase + GICR_IGROUPR0);
> + for (i = 0; i < gic_data.ppi_nr + 16; i += 32)
> + writel_relaxed(~0, rbase + GICR_IGROUPR0 + i / 8);
>
> - gic_cpu_config(rbase, gic_redist_wait_for_rwp);
> + gic_cpu_config(rbase, gic_data.ppi_nr + 16, gic_redist_wait_for_rwp);
>
> /* initialise system registers */
> gic_cpu_sys_reg_init();
> @@ -1449,7 +1457,7 @@ static int __init gic_init_bases(void __iomem *dist_base,
>
> set_handle_irq(gic_handle_irq);
>
> - gic_update_vlpi_properties();
> + gic_update_rdist_properties();
>
> gic_smp_init();
> gic_dist_init();
> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> index ab48760acabb..25c1ae69db30 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -543,7 +543,7 @@ static int gic_cpu_init(struct gic_chip_data *gic)
> gic_cpu_map[i] &= ~cpu_mask;
> }
>
> - gic_cpu_config(dist_base, NULL);
> + gic_cpu_config(dist_base, 32, NULL);
>
> writel_relaxed(GICC_INT_PRI_THRESHOLD, base + GIC_CPU_PRIMASK);
> gic_cpu_if_up(gic);
> diff --git a/drivers/irqchip/irq-hip04.c b/drivers/irqchip/irq-hip04.c
> index 1626131834a6..130caa1c9d93 100644
> --- a/drivers/irqchip/irq-hip04.c
> +++ b/drivers/irqchip/irq-hip04.c
> @@ -273,7 +273,7 @@ static void hip04_irq_cpu_init(struct hip04_irq_data *intc)
> if (i != cpu)
> hip04_cpu_map[i] &= ~cpu_mask;
>
> - gic_cpu_config(dist_base, NULL);
> + gic_cpu_config(dist_base, 32, NULL);
>
> writel_relaxed(0xf0, base + GIC_CPU_PRIMASK);
> writel_relaxed(1, base + GIC_CPU_CTRL);
>
--
Julien Thierry
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 00/20] Initial support for Marvell MMP3 SoC
From: Olof Johansson @ 2019-08-22 16:23 UTC (permalink / raw)
To: Marc Zyngier
Cc: Mark Rutland, DTML, Jason Cooper, Stephen Boyd,
Linux Kernel Mailing List, Michael Turquette, Russell King,
Kishon Vijay Abraham I, Lubomir Rintel, Rob Herring,
Thomas Gleixner, linux-clk, Linux ARM Mailing List
In-Reply-To: <244fdc87-0fe5-be79-d9cd-2395d0ac3f57@kernel.org>
On Thu, Aug 22, 2019 at 3:32 AM Marc Zyngier <maz@kernel.org> wrote:
>
> On 22/08/2019 10:26, Lubomir Rintel wrote:
> > Hi,
> >
> > this is a second spin of a patch set that adds support for the Marvell
> > MMP3 processor. MMP3 is used in OLPC XO-4 laptops, Panasonic Toughpad
> > FZ-A1 tablet and Dell Wyse 3020 Tx0D thin clients.
> >
> > Compared to v1, there's a handful of fixes in response to reviews. Patch
> > 02/20 is new. Details in individual patches.
> >
> > Apart from the adjustments in mach-mmp/, the patch makes necessary
> > changes to the irqchip driver and adds an USB2 PHY driver. The latter
> > has a dependency on the mach-mmp/ changes, so it can't be submitted
> > separately.
> >
> > The patch set has been tested to work on Wyse Tx0D and not ruin MMP2
> > support on XO-1.75.
>
> How do you want this series to be merged? I'm happy to take the irqchip
> related patches as well as the corresponding DT change (once reviewed)
> through my tree.
DT changes, unless there's lack of backwards compatibility, are best
merged through the platform trees. Especially for new platforms like
these where there's likely going to be nearby changes (and thus
conflicts).
I.e. driver changes I'm all for bringing through driver trees
(including binding patches), but please leave dts/dtsi changes to the
platform.
-Olof
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 04/10] KVM: Implement kvm_put_guest()
From: Sean Christopherson @ 2019-08-22 16:24 UTC (permalink / raw)
To: Steven Price
Cc: Mark Rutland, Radim Krčmář, kvm, Suzuki K Pouloze,
Marc Zyngier, linux-doc, Russell King, linux-kernel, James Morse,
linux-arm-kernel, Catalin Marinas, Paolo Bonzini, Will Deacon,
kvmarm, Julien Thierry
In-Reply-To: <e2abc69b-74c2-64ef-e270-43d93513eaae@arm.com>
On Thu, Aug 22, 2019 at 04:46:10PM +0100, Steven Price wrote:
> On 22/08/2019 16:28, Sean Christopherson wrote:
> > On Wed, Aug 21, 2019 at 04:36:50PM +0100, Steven Price wrote:
> >> kvm_put_guest() is analogous to put_user() - it writes a single value to
> >> the guest physical address. The implementation is built upon put_user()
> >> and so it has the same single copy atomic properties.
> >
> > What you mean by "single copy atomic"? I.e. what guarantees does
> > put_user() provide that __copy_to_user() does not?
>
> Single-copy atomicity is defined by the Arm architecture[1] and I'm not
> going to try to go into the full details here, so this is a summary.
>
> For the sake of this feature what we care about is that the value
> written/read cannot be "torn". In other words if there is a read (in
> this case from another VCPU) that is racing with the write then the read
> will either get the old value or the new value. It cannot return a
> mixture. (This is of course assuming that the read is using a
> single-copy atomic safe method).
Thanks for the explanation. I assumed that's what you were referring to,
but wanted to double check.
> __copy_to_user() is implemented as a memcpy() and as such cannot provide
> single-copy atomicity in the general case (the buffer could easily be
> bigger than the architecture can guarantee).
>
> put_user() on the other hand is implemented (on arm64) as an explicit
> store instruction and therefore is guaranteed by the architecture to be
> single-copy atomic (i.e. another CPU cannot see a half-written value).
I don't think kvm_put_guest() belongs in generic code, at least not with
the current changelog explanation about it providing single-copy atomic
semantics. AFAICT, the single-copy thing is very much an arm64
implementation detail, e.g. the vast majority of 32-bit architectures,
including x86, do not provide any guarantees, and x86-64 generates more
or less the same code for put_user() and __copy_to_user() for 8-byte and
smaller accesses.
As an alternative to kvm_put_guest() entirely, is it an option to change
arm64's raw_copy_to_user() to redirect to __put_user() for sizes that are
constant at compile time and can be handled by __put_user()? That would
allow using kvm_write_guest() to update stolen time, albeit with
arguably an even bigger dependency on the uaccess implementation details.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Aw: [PATCH net-next v2 0/3] net: dsa: mt7530: Convert to PHYLINK and add support for port 5
From: Frank Wunderlich @ 2019-08-22 16:26 UTC (permalink / raw)
To: Frank Wunderlich
Cc: Andrew Lunn, Florian Fainelli, netdev, Sean Wang, linux-mips,
Vivien Didelot, "René van Dorst", linux-mediatek,
John Crispin, Matthias Brugger, David S . Miller,
linux-arm-kernel
In-Reply-To: <trinity-b1f48e51-af73-466d-9ecf-d560a7d7c1ee-1566488653737@3c-app-gmx-bap07>
tested now also on bpi-r64 (mt7622) v0.1 (rtl8367 switch), without linux-next to avoid power-regulator-problems like on bpi-r2
dmesg without warnings/errors caused by this patches
link came up as desired
iperf3 looks good: 943 Mbits/sec in both directions and no other issues
so it is currently only the rx-throughput-problem on mt7623/bpi-r2
regards Frank
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC v4 01/18] objtool: Add abstraction for computation of symbols offsets
From: Julien @ 2019-08-22 16:30 UTC (permalink / raw)
To: Raphael Gault, linux-arm-kernel, linux-kernel, jpoimboe
Cc: peterz, catalin.marinas, will.deacon, raph.gault+kdev
In-Reply-To: <20190816122403.14994-2-raphael.gault@arm.com>
Hi Raphaël,
On 16/08/19 13:23, Raphael Gault wrote:
> The jump destination and relocation offset used previously are only
> reliable on x86_64 architecture. We abstract these computations by calling
> arch-dependent implementations.
>
> Signed-off-by: Raphael Gault <raphael.gault@arm.com>
> ---
> tools/objtool/arch.h | 6 ++++++
> tools/objtool/arch/x86/decode.c | 11 +++++++++++
> tools/objtool/check.c | 15 ++++++++++-----
> 3 files changed, 27 insertions(+), 5 deletions(-)
>
> diff --git a/tools/objtool/arch.h b/tools/objtool/arch.h
> index ced3765c4f44..a9a50a25ca66 100644
> --- a/tools/objtool/arch.h
> +++ b/tools/objtool/arch.h
> @@ -66,6 +66,8 @@ struct stack_op {
> struct op_src src;
> };
>
> +struct instruction;
> +
> void arch_initial_func_cfi_state(struct cfi_state *state);
>
> int arch_decode_instruction(struct elf *elf, struct section *sec,
> @@ -75,4 +77,8 @@ int arch_decode_instruction(struct elf *elf, struct section *sec,
>
> bool arch_callee_saved_reg(unsigned char reg);
>
> +unsigned long arch_jump_destination(struct instruction *insn);
> +
> +unsigned long arch_dest_rela_offset(int addend);
> +
> #endif /* _ARCH_H */
> diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c
> index 0567c47a91b1..fa33b3465722 100644
> --- a/tools/objtool/arch/x86/decode.c
> +++ b/tools/objtool/arch/x86/decode.c
> @@ -11,6 +11,7 @@
> #include "lib/inat.c"
> #include "lib/insn.c"
>
> +#include "../../check.h"
> #include "../../elf.h"
> #include "../../arch.h"
> #include "../../warn.h"
> @@ -66,6 +67,11 @@ bool arch_callee_saved_reg(unsigned char reg)
> }
> }
>
> +unsigned long arch_dest_rela_offset(int addend)
> +{
> + return addend + 4;
> +}
> +
> int arch_decode_instruction(struct elf *elf, struct section *sec,
> unsigned long offset, unsigned int maxlen,
> unsigned int *len, enum insn_type *type,
> @@ -497,3 +503,8 @@ void arch_initial_func_cfi_state(struct cfi_state *state)
> state->regs[16].base = CFI_CFA;
> state->regs[16].offset = -8;
> }
> +
> +unsigned long arch_jump_destination(struct instruction *insn)
> +{
> + return insn->offset + insn->len + insn->immediate;
> +}
> diff --git a/tools/objtool/check.c b/tools/objtool/check.c
> index 176f2f084060..479fab46b656 100644
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -563,7 +563,7 @@ static int add_jump_destinations(struct objtool_file *file)
> insn->len);
> if (!rela) {
> dest_sec = insn->sec;
> - dest_off = insn->offset + insn->len + insn->immediate;
> + dest_off = arch_jump_destination(insn);
> } else if (rela->sym->type == STT_SECTION) {
> dest_sec = rela->sym->sec;
> dest_off = rela->addend + 4;
> @@ -659,7 +659,7 @@ static int add_call_destinations(struct objtool_file *file)
> rela = find_rela_by_dest_range(insn->sec, insn->offset,
> insn->len);
> if (!rela) {
> - dest_off = insn->offset + insn->len + insn->immediate;
> + dest_off = arch_jump_destination(insn);
> insn->call_dest = find_symbol_by_offset(insn->sec,
> dest_off);
>
> @@ -672,14 +672,19 @@ static int add_call_destinations(struct objtool_file *file)
> }
>
> } else if (rela->sym->type == STT_SECTION) {
> + /*
> + * the original x86_64 code adds 4 to the rela->addend
> + * which is not needed on arm64 architecture.
> + */
I'm not sure this is worth mentioning in generic code. You might include
it in the commit message to justify the change.
> + dest_off = arch_dest_rela_offset(rela->addend);
> insn->call_dest = find_symbol_by_offset(rela->sym->sec,
> - rela->addend+4);
> + dest_off);
> if (!insn->call_dest ||
> insn->call_dest->type != STT_FUNC) {
> - WARN_FUNC("can't find call dest symbol at %s+0x%x",
> + WARN_FUNC("can't find call dest symbol at %s+0x%lx",
> insn->sec, insn->offset,
> rela->sym->sec->name,
> - rela->addend + 4);
> + dest_off);
> return -1;
> }
> } else
>
Otherwise, the change looks good to me.
Thanks,
--
Julien Thierry
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 05/12] irqchip/gic: Prepare for more than 16 PPIs
From: Marc Zyngier @ 2019-08-22 16:32 UTC (permalink / raw)
To: Julien, Thomas Gleixner, Jason Cooper, Rob Herring
Cc: Lokesh Vutla, John Garry, linux-kernel, Shameerali Kolothum Thodi,
linux-arm-kernel
In-Reply-To: <1b2675f6-f839-80f8-b7d8-a7d402085745@gmail.com>
Hi Julien,
On 22/08/2019 17:11, Julien wrote:
> Hi Marc,
>
> On 06/08/19 11:01, Marc Zyngier wrote:
>> GICv3.1 allows up to 80 PPIs (16 legaci PPIs and 64 Extended PPIs),
>> meaning we can't just leave the old 16 hardcoded everywhere.
>>
>> We also need to add the infrastructure to discover the number of PPIs
>> on a per redistributor basis, although we still pretend there is only
>> 16 of them for now.
>>
>> No functional change.
>>
>> Signed-off-by: Marc Zyngier <maz@kernel.org>
>> ---
>> drivers/irqchip/irq-gic-common.c | 19 ++++++++++++-------
>> drivers/irqchip/irq-gic-common.h | 2 +-
>> drivers/irqchip/irq-gic-v3.c | 22 +++++++++++++++-------
>> drivers/irqchip/irq-gic.c | 2 +-
>> drivers/irqchip/irq-hip04.c | 2 +-
>> 5 files changed, 30 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/irqchip/irq-gic-common.c b/drivers/irqchip/irq-gic-common.c
>> index 6900b6f0921c..14110db01c05 100644
>> --- a/drivers/irqchip/irq-gic-common.c
>> +++ b/drivers/irqchip/irq-gic-common.c
>> @@ -128,26 +128,31 @@ void gic_dist_config(void __iomem *base, int gic_irqs,
>> sync_access();
>> }
>>
>> -void gic_cpu_config(void __iomem *base, void (*sync_access)(void))
>> +void gic_cpu_config(void __iomem *base, int nr, void (*sync_access)(void))
>> {
>> int i;
>>
>> /*
>> * Deal with the banked PPI and SGI interrupts - disable all
>> - * PPI interrupts, ensure all SGI interrupts are enabled.
>> - * Make sure everything is deactivated.
>> + * private interrupts. Make sure everything is deactivated.
>> */
>> - writel_relaxed(GICD_INT_EN_CLR_X32, base + GIC_DIST_ACTIVE_CLEAR);
>> - writel_relaxed(GICD_INT_EN_CLR_PPI, base + GIC_DIST_ENABLE_CLEAR);
>> - writel_relaxed(GICD_INT_EN_SET_SGI, base + GIC_DIST_ENABLE_SET);
>> + for (i = 0; i < nr; i += 32) {
>
> You added "nr" as argument but if "nr" isn't a multiple of 32 weird
> things might happen, no?
>
> It would be worth specifying that somewhere, and checking it with a WARN().
TBH, I'm unsure whether that's worth it. The architecture is completely
built around having the private interrupts in blocks of 32, and you can
only get something wrong if you misdecode the number of interrupts from
the registers.
> Maybe it might be worth reducing the granularity to manipulating 16 irqs
> since there are 16 SGI + 16 PPI + 64 EPPI, but that might not be very
> useful right now.
I don't see what this brings us at this point. The architecture doesn't
seem to go in the direction of adding more SGIs, so we're pretty safe on
that front...
Thanks,
M.
--
Jazz is not dead, it just smells funny...
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 16/20] ARM: mmp: add SMP support
From: Florian Fainelli @ 2019-08-22 16:36 UTC (permalink / raw)
To: Lubomir Rintel, Olof Johansson
Cc: Mark Rutland, devicetree, linux-kernel, Jason Cooper,
Stephen Boyd, Marc Zyngier, Michael Turquette, Russell King,
Kishon Vijay Abraham I, Rob Herring, Thomas Gleixner, linux-clk,
linux-arm-kernel
In-Reply-To: <20190822092643.593488-17-lkundrak@v3.sk>
On 8/22/19 2:26 AM, Lubomir Rintel wrote:
> Used to bring up the second core on MMP3.
>
> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
>
> ---
> Changes since v1:
> - Wrap SW_BRANCH_VIRT_ADDR with __pa_symbol()
>
> arch/arm/mach-mmp/Makefile | 3 +++
> arch/arm/mach-mmp/platsmp.c | 33 +++++++++++++++++++++++++++++++++
> 2 files changed, 36 insertions(+)
> create mode 100644 arch/arm/mach-mmp/platsmp.c
>
> diff --git a/arch/arm/mach-mmp/Makefile b/arch/arm/mach-mmp/Makefile
> index 322c1c97dc900..7b3a7f979eece 100644
> --- a/arch/arm/mach-mmp/Makefile
> +++ b/arch/arm/mach-mmp/Makefile
> @@ -22,6 +22,9 @@ ifeq ($(CONFIG_PM),y)
> obj-$(CONFIG_CPU_PXA910) += pm-pxa910.o
> obj-$(CONFIG_CPU_MMP2) += pm-mmp2.o
> endif
> +ifeq ($(CONFIG_SMP),y)
> +obj-$(CONFIG_MACH_MMP3_DT) += platsmp.o
> +endif
>
> # board support
> obj-$(CONFIG_MACH_ASPENITE) += aspenite.o
> diff --git a/arch/arm/mach-mmp/platsmp.c b/arch/arm/mach-mmp/platsmp.c
> new file mode 100644
> index 0000000000000..98d5ef23623cb
> --- /dev/null
> +++ b/arch/arm/mach-mmp/platsmp.c
> @@ -0,0 +1,33 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2019 Lubomir Rintel <lkundrak@v3.sk>
> + */
> +#include <linux/io.h>
> +#include <asm/smp_scu.h>
> +#include <asm/smp.h>
> +#include "addr-map.h"
> +
> +#define SW_BRANCH_VIRT_ADDR CIU_REG(0x24)
> +
> +static int mmp3_boot_secondary(unsigned int cpu, struct task_struct *idle)
> +{
> + /*
> + * Apparently, the boot ROM on the second core spins on this
> + * register becoming non-zero and then jumps to the address written
> + * there. No IPIs involved.
> + */
> + __raw_writel(virt_to_phys(secondary_startup),
> + __pa_symbol(SW_BRANCH_VIRT_ADDR));
That looks wrong, the __pa_symbol() is applicable to secondary_startup,
while SW_BRANCH_VIRT_ADDR does not need that.
> + return 0;
> +}
> +
> +static void mmp3_smp_prepare_cpus(unsigned int max_cpus)
> +{
> + scu_enable(SCU_VIRT_BASE);
> +}
> +
> +static const struct smp_operations mmp3_smp_ops __initconst = {
> + .smp_prepare_cpus = mmp3_smp_prepare_cpus,
> + .smp_boot_secondary = mmp3_boot_secondary,
> +};
> +CPU_METHOD_OF_DECLARE(mmp3_smp, "marvell,mmp3-smp", &mmp3_smp_ops);
>
--
Florian
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v9 3/3] arm64: Relax Documentation/arm64/tagged-pointers.rst
From: Dave Martin @ 2019-08-22 16:37 UTC (permalink / raw)
To: Catalin Marinas
Cc: linux-arch, linux-doc, Szabolcs Nagy, Andrey Konovalov,
Kevin Brodsky, Will Deacon, linux-mm, Andrew Morton,
Vincenzo Frascino, Will Deacon, Dave Hansen, linux-arm-kernel
In-Reply-To: <20190822155531.GB55798@arrakis.emea.arm.com>
On Thu, Aug 22, 2019 at 04:55:32PM +0100, Catalin Marinas wrote:
> On Wed, Aug 21, 2019 at 07:46:51PM +0100, Dave P Martin wrote:
> > On Wed, Aug 21, 2019 at 06:33:53PM +0100, Will Deacon wrote:
> > > On Wed, Aug 21, 2019 at 05:47:30PM +0100, Catalin Marinas wrote:
> > > > @@ -59,6 +63,11 @@ be preserved.
> > > > The architecture prevents the use of a tagged PC, so the upper byte will
> > > > be set to a sign-extension of bit 55 on exception return.
> > > >
> > > > +This behaviour is maintained when the AArch64 Tagged Address ABI is
> > > > +enabled. In addition, with the exceptions above, the kernel will
> > > > +preserve any non-zero tags passed by the user via syscalls and stored in
> > > > +kernel data structures (e.g. ``set_robust_list()``, ``sigaltstack()``).
> >
> > sigaltstack() is interesting, since we don't support tagged stacks.
>
> We should support tagged SP with the new ABI as they'll be required for
> MTE. sigaltstack() and clone() are the two syscalls that come to mind
> here.
>
> > Do we keep the ss_sp tag in the kernel, but squash it when delivering
> > a signal to the alternate stack?
>
> We don't seem to be doing any untagging, so we just just use whatever
> the caller asked for. We may need a small test to confirm.
If we want to support tagged SP, then I guess we shouldn't be squashing
the tag anywhere. A test for that would be sensible to have.
> That said, on_sig_stack() probably needs some untagging as it does user
> pointer arithmetics with potentially different tags.
Good point.
> > > Hmm. I can see the need to provide this guarantee for things like
> > > set_robust_list(), but the problem is that the statement above is too broad
> > > and isn't strictly true: for example, mmap() doesn't propagate the tag of
> > > its address parameter into the VMA.
> > >
> > > So I think we need to nail this down a bit more, but I'm having a really
> > > hard time coming up with some wording :(
> >
> > Time for some creative vagueness?
> >
> > We can write a statement of our overall intent, along with examples of
> > a few cases where the tag should and should not be expected to emerge
> > intact.
> >
> > There is no foolproof rule, unless we can rewrite history...
>
> I would expect the norm to be the preservation of tags with a few
> exceptions. The only ones I think where we won't preserve the tags are
> mmap, mremap, brk (apart from the signal stuff already mentioned in the
> current tagged-pointers.rst doc).
>
> So I can remove this paragraph altogether and add a note in part 3 of
> the tagged-address-abi.rst document that mmap/mremap/brk do not preserve
> the tag information.
Deleting text is always a good idea ;)
There are other cases like (non-)propagation of the tag to si_addr
when a fault is reported via a signal, but I think we already have
appropriate wording to cover that.
Cheers
---Dave
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v4 6/7] arm64: perf: Enable pmu counter direct access for perf event on armv8
From: Jonathan Cameron @ 2019-08-22 16:39 UTC (permalink / raw)
To: Raphael Gault
Cc: mark.rutland, raph.gault+kdev, peterz, catalin.marinas,
will.deacon, linux-kernel, acme, mingo, linux-arm-kernel
In-Reply-To: <20190822144220.27860-7-raphael.gault@arm.com>
On Thu, 22 Aug 2019 15:42:19 +0100
Raphael Gault <raphael.gault@arm.com> wrote:
> Keep track of event opened with direct access to the hardware counters
> and modify permissions while they are open.
>
> The strategy used here is the same which x86 uses: everytime an event
> is mapped, the permissions are set if required. The atomic field added
> in the mm_context helps keep track of the different event opened and
> de-activate the permissions when all are unmapped.
> We also need to update the permissions in the context switch code so
> that tasks keep the right permissions.
>
> Signed-off-by: Raphael Gault <raphael.gault@arm.com>
Hi Raphael,
One trivial comment inline.
Thanks,
Jonathan
> ---
> arch/arm64/include/asm/mmu.h | 6 ++++
> arch/arm64/include/asm/mmu_context.h | 2 ++
> arch/arm64/include/asm/perf_event.h | 14 ++++++++
> arch/arm64/kernel/perf_event.c | 1 +
> drivers/perf/arm_pmu.c | 54 ++++++++++++++++++++++++++++
> 5 files changed, 77 insertions(+)
>
> diff --git a/arch/arm64/include/asm/mmu.h b/arch/arm64/include/asm/mmu.h
> index fd6161336653..88ed4466bd06 100644
> --- a/arch/arm64/include/asm/mmu.h
> +++ b/arch/arm64/include/asm/mmu.h
> @@ -18,6 +18,12 @@
>
> typedef struct {
> atomic64_t id;
> +
> + /*
> + * non-zero if userspace have access to hardware
> + * counters directly.
> + */
> + atomic_t pmu_direct_access;
> void *vdso;
> unsigned long flags;
> } mm_context_t;
> diff --git a/arch/arm64/include/asm/mmu_context.h b/arch/arm64/include/asm/mmu_context.h
> index 7ed0adb187a8..6e66ff940494 100644
> --- a/arch/arm64/include/asm/mmu_context.h
> +++ b/arch/arm64/include/asm/mmu_context.h
> @@ -21,6 +21,7 @@
> #include <asm-generic/mm_hooks.h>
> #include <asm/cputype.h>
> #include <asm/pgtable.h>
> +#include <asm/perf_event.h>
> #include <asm/sysreg.h>
> #include <asm/tlbflush.h>
>
> @@ -224,6 +225,7 @@ static inline void __switch_mm(struct mm_struct *next)
> }
>
> check_and_switch_context(next, cpu);
> + perf_switch_user_access(next);
> }
>
> static inline void
> diff --git a/arch/arm64/include/asm/perf_event.h b/arch/arm64/include/asm/perf_event.h
> index 2bdbc79bbd01..ba58fa726631 100644
> --- a/arch/arm64/include/asm/perf_event.h
> +++ b/arch/arm64/include/asm/perf_event.h
> @@ -8,6 +8,7 @@
>
> #include <asm/stack_pointer.h>
> #include <asm/ptrace.h>
> +#include <linux/mm_types.h>
>
> #define ARMV8_PMU_MAX_COUNTERS 32
> #define ARMV8_PMU_COUNTER_MASK (ARMV8_PMU_MAX_COUNTERS - 1)
> @@ -223,4 +224,17 @@ extern unsigned long perf_misc_flags(struct pt_regs *regs);
> (regs)->pstate = PSR_MODE_EL1h; \
> }
>
> +static inline void perf_switch_user_access(struct mm_struct *mm)
> +{
> + if (!IS_ENABLED(CONFIG_PERF_EVENTS))
> + return;
> +
> + if (atomic_read(&mm->context.pmu_direct_access)) {
> + write_sysreg(ARMV8_PMU_USERENR_ER|ARMV8_PMU_USERENR_CR,
> + pmuserenr_el0);
> + } else {
> + write_sysreg(0, pmuserenr_el0);
> + }
> +}
> +
> #endif
> diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
> index de9b001e8b7c..7de56f22d038 100644
> --- a/arch/arm64/kernel/perf_event.c
> +++ b/arch/arm64/kernel/perf_event.c
> @@ -1285,6 +1285,7 @@ void arch_perf_update_userpage(struct perf_event *event,
> */
> freq = arch_timer_get_rate();
> userpg->cap_user_time = 1;
> + userpg->cap_user_rdpmc = !!(event->hw.flags & ARMPMU_EL0_RD_CNTR);
>
> clocks_calc_mult_shift(&userpg->time_mult, &shift, freq,
> NSEC_PER_SEC, 0);
> diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
> index 2d06b8095a19..d0d3e523a4c4 100644
> --- a/drivers/perf/arm_pmu.c
> +++ b/drivers/perf/arm_pmu.c
> @@ -25,6 +25,7 @@
> #include <linux/irqdesc.h>
>
> #include <asm/irq_regs.h>
> +#include <asm/mmu_context.h>
>
> static DEFINE_PER_CPU(struct arm_pmu *, cpu_armpmu);
> static DEFINE_PER_CPU(int, cpu_irq);
> @@ -778,6 +779,57 @@ static void cpu_pmu_destroy(struct arm_pmu *cpu_pmu)
> &cpu_pmu->node);
> }
>
> +static void refresh_pmuserenr(void *mm)
> +{
> + perf_switch_user_access(mm);
> +}
> +
> +static int check_homogeneous_cap(struct perf_event *event, struct mm_struct *mm)
> +{
> + pr_info("checking HAS_HOMOGENEOUS_PMU");
Can we drop this spam from the good path. Makes a bit of a mess of my
terminal when running the test ;)
> + if (!cpus_have_cap(ARM64_HAS_HOMOGENEOUS_PMU)) {
> + pr_info("Disable direct access (!HAS_HOMOGENEOUS_PMU)");
> + atomic_set(&mm->context.pmu_direct_access, 0);
> + on_each_cpu(refresh_pmuserenr, mm, 1);
> + event->hw.flags &= ~ARMPMU_EL0_RD_CNTR;
> + return 0;
> + }
> +
> + return 1;
> +}
> +
> +static void armpmu_event_mapped(struct perf_event *event, struct mm_struct *mm)
> +{
> + if (!(event->hw.flags & ARMPMU_EL0_RD_CNTR))
> + return;
> +
> + /*
> + * This function relies on not being called concurrently in two
> + * tasks in the same mm. Otherwise one task could observe
> + * pmu_direct_access > 1 and return all the way back to
> + * userspace with user access disabled while another task is still
> + * doing on_each_cpu_mask() to enable user access.
> + *
> + * For now, this can't happen because all callers hold mmap_sem
> + * for write. If this changes, we'll need a different solution.
> + */
> + lockdep_assert_held_write(&mm->mmap_sem);
> +
> + if (check_homogeneous_cap(event, mm) &&
> + atomic_inc_return(&mm->context.pmu_direct_access) == 1)
> + on_each_cpu(refresh_pmuserenr, mm, 1);
> +}
> +
> +static void armpmu_event_unmapped(struct perf_event *event, struct mm_struct *mm)
> +{
> + if (!(event->hw.flags & ARMPMU_EL0_RD_CNTR))
> + return;
> +
> + if (check_homogeneous_cap(event, mm) &&
> + atomic_dec_and_test(&mm->context.pmu_direct_access))
> + on_each_cpu_mask(mm_cpumask(mm), refresh_pmuserenr, NULL, 1);
> +}
> +
> static struct arm_pmu *__armpmu_alloc(gfp_t flags)
> {
> struct arm_pmu *pmu;
> @@ -799,6 +851,8 @@ static struct arm_pmu *__armpmu_alloc(gfp_t flags)
> .pmu_enable = armpmu_enable,
> .pmu_disable = armpmu_disable,
> .event_init = armpmu_event_init,
> + .event_mapped = armpmu_event_mapped,
> + .event_unmapped = armpmu_event_unmapped,
> .add = armpmu_add,
> .del = armpmu_del,
> .start = armpmu_start,
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 3/3] watchdog/aspeed: add support for dual boot
From: Alexander Amelkin @ 2019-08-22 16:45 UTC (permalink / raw)
To: Guenter Roeck
Cc: linux-watchdog, linux-aspeed, Andrew Jeffery, linux-kernel,
Joel Stanley, Ivan Mikhaylov, Wim Van Sebroeck, linux-arm-kernel
In-Reply-To: <20190822160127.GA6992@roeck-us.net>
[-- Attachment #1.1.1: Type: text/plain, Size: 4196 bytes --]
22.08.2019 19:01, Guenter Roeck wrote:
> On Thu, Aug 22, 2019 at 05:36:21PM +0300, Alexander Amelkin wrote:
>> 21.08.2019 21:10, Guenter Roeck wrote:
>>> On Wed, Aug 21, 2019 at 08:42:24PM +0300, Alexander Amelkin wrote:
>>>> 21.08.2019 19:32, Guenter Roeck wrote:
>>>>> On Wed, Aug 21, 2019 at 06:57:43PM +0300, Ivan Mikhaylov wrote:
>>>>>> Set WDT_CLEAR_TIMEOUT_AND_BOOT_CODE_SELECTION into WDT_CLEAR_TIMEOUT_STATUS
>>>>>> to clear out boot code source and re-enable access to the primary SPI flash
>>>>>> chip while booted via wdt2 from the alternate chip.
>>>>>>
>>>>>> AST2400 datasheet says:
>>>>>> "In the 2nd flash booting mode, all the address mapping to CS0# would be
>>>>>> re-directed to CS1#. And CS0# is not accessable under this mode. To access
>>>>>> CS0#, firmware should clear the 2nd boot mode register in the WDT2 status
>>>>>> register WDT30.bit[1]."
>>>>> Is there reason to not do this automatically when loading the module
>>>>> in alt-boot mode ? What means does userspace have to determine if CS0
>>>>> or CS1 is active at any given time ? If there is reason to ever have CS1
>>>>> active instead of CS0, what means would userspace have to enable it ?
>>>> Yes, there is. The driver is loaded long before the filesystems are mounted.
>>>> The filesystems, in the event of alternate/recovery boot, need to be mounted
>>>> from the same chip that the kernel was booted. For one reason because the main
>>>> chip at CS0 is most probably corrupt. If you clear that bit when driver is
>>>> loaded, your software will not know that and will try to mount the wrong
>>>> filesystems. The whole idea of ASPEED's switching chipselects is to have
>>>> identical firmware in both chips, without the need to process the alternate
>>>> boot state in any way except for indicating a successful boot and restoring
>>>> access to CS0 when needed.
>>>>
>>>> The userspace can read bootstatus sysfs node to determine if an alternate
>>>> boot has occured.
>>>>
>>>> With ASPEED, CS1 is activated automatically by wdt2 when system fails to boot
>>>> from the primary flash chip (at CS0) and disable the watchdog to indicate a
>>>> successful boot. When that happens, both CS0 and CS1 controls get routed in
>>>> hardware to CS1 line, making the primary flash chip inaccessible. Depending
>>>> on the architecture of the user-space software, it may choose to re-enable
>>>> access to the primary chip via CS0 at different times. There must be a way to do so.
>>>>
>>> So by activating cs0, userspace would essentially pull its own root file system
>>> from underneath itself ?
>> Exactly. That's why for alternate boot the firmware would usually copy
>> all filesystems to memory and mount from there. Some embedded systems
>> do that always, regardless of which chip they boot from.
>>
> That is different, though, to what you said earlier. Linux would then start
> with a clean file system, and not need access to the file system in cs1 at all.
> Clearing the flag when starting the driver would then be ok.
I don't see how that is different. Copying to memory may be done by startup
scripts that run after the driver is loaded, so they need to read the data from
the chip they are booted from. That is how it is done in OpenBMC, for instance.
Other flavors of firmware may choose a different approach.
Having the control available via sysfs gives more flexibility.
>> However, to be able to recover the main flash chip, the system needs CS0
>> to function as such (not as CS1). That's why this control is needed.
>>
> If what you said is correct, not really. It should be fine and create more
> predictive behavior if the probe function selects cs0 automatically.
Well, this is not a function for home users. This is for servers. You won't
even find an ASPEED BMC chip in a home PC. Aspeed's dual-boot is quite
an advanced feature and people willing to use it are expected to be able
to predict the behavior. To me, as an embedded systems developer,
automatic selection of cs0 by probe is a limitation. I prefer flexibility.
With best regards,
Alexander Amelkin,
BIOS/BMC Team Lead, YADRO
https://yadro.com
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 3/5] arm64: atomics: avoid out-of-line ll/sc atomics
From: Mark Rutland @ 2019-08-22 17:01 UTC (permalink / raw)
To: Andrew Murray
Cc: Peter Zijlstra, Catalin Marinas, Boqun Feng, Will Deacon,
Ard.Biesheuvel, linux-arm-kernel
In-Reply-To: <20190812143625.42745-4-andrew.murray@arm.com>
On Mon, Aug 12, 2019 at 03:36:23PM +0100, Andrew Murray wrote:
> When building for LSE atomics (CONFIG_ARM64_LSE_ATOMICS), if the hardware
> or toolchain doesn't support it the existing code will fallback to ll/sc
> atomics. It achieves this by branching from inline assembly to a function
> that is built with specical compile flags. Further this results in the
> clobbering of registers even when the fallback isn't used increasing
> register pressure.
>
> Let's improve this by providing inline implementations of both LSE and
> ll/sc and use a static key to select between them. This allows for the
> compiler to generate better atomics code.
>
> To improve icache performance for the LL/SC fallback atomics, we put them
> in their own subsection.
>
> Please note that as atomic_arch.h is included indirectly by kernel.h
> (via bitops.h), we cannot depend on features provided later in the kernel.h
> file. This prevents us from placing the system_uses_lse_atomics function
> in cpu_feature.h due to its dependencies.
>
> Signed-off-by: Andrew Murray <andrew.murray@arm.com>
[...]
> diff --git a/arch/arm64/include/asm/atomic_arch.h b/arch/arm64/include/asm/atomic_arch.h
> new file mode 100644
> index 000000000000..255a284321c6
> --- /dev/null
> +++ b/arch/arm64/include/asm/atomic_arch.h
> @@ -0,0 +1,154 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Selection between LSE and LL/SC atomics.
> + *
> + * Copyright (C) 2018 ARM Ltd.
> + * Author: Andrew Murray <andrew.murray@arm.com>
> + */
> +
> +#ifndef __ASM_ATOMIC_ARCH_H
> +#define __ASM_ATOMIC_ARCH_H
> +
> +#include <asm/atomic_lse.h>
> +#include <asm/atomic_ll_sc.h>
> +
> +#include <linux/jump_label.h>
> +#include <asm/cpucaps.h>
I'm guessing that we have to include the <asm/atomic_*> headers first
due to the include dependencies. If that's the case, could we please
have a comment here to that effect?
Minor nit, but could we also order those two alphabetically, please?
The general style is to have headers alphabetically, with (for reasons
unknown) the <linux/*> headers before the <asm/*> headers.
[...]
> +#if IS_ENABLED(CONFIG_ARM64_LSE_ATOMICS) && IS_ENABLED(CONFIG_AS_LSE)
> +#define __LL_SC_FALLBACK(asm_ops) \
> +" b 3f\n" \
> +" .subsection 1\n" \
> +"3:\n" \
> +asm_ops "\n" \
> +" b 4f\n" \
> +" .previous\n" \
> +"4:\n"
> +#else
> +#define __LL_SC_FALLBACK(asm_ops) asm_ops
> #endif
Can we instead make the ll/sc functions with the cold attribute (wrapped
by <linux/compiler.h> as __cold)?
IIUC that should have a similar effect, and might allow GCC to do better
(e.g. merging compatible instances of the ll/sc code in the same cold
subsection).
https://gcc.gnu.org/onlinedocs/gcc-9.2.0/gcc/Common-Function-Attributes.html#index-cold-function-attribute
Otherwise, this is looking much nicer!
Thanks,
Mark.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [EXT] [PATCH v3 2/2] drm/bridge: Add NWL MIPI DSI host controller support
From: Guido Günther @ 2019-08-22 17:03 UTC (permalink / raw)
To: Robert Chiras
Cc: mark.rutland@arm.com, devicetree@vger.kernel.org,
jernej.skrabec@siol.net, kernel@pengutronix.de, sam@ravnborg.org,
narmstrong@baylibre.com, airlied@linux.ie, festevam@gmail.com,
s.hauer@pengutronix.de, jonas@kwiboo.se,
linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
a.hajda@samsung.com, robh+dt@kernel.org, arnd@arndb.de,
dl-linux-imx, daniel@ffwll.ch, shawnguo@kernel.org,
lee.jones@linaro.org, linux-arm-kernel@lists.infradead.org,
Laurent.pinchart@ideasonboard.com
In-Reply-To: <1566479899.3209.101.camel@nxp.com>
Hi Robert,
thanks for your comments! Most of this make sense, i have some comments
inline below (mostly since I only have access to the imx8mq reference
manual but not to the any NWL IP docs):
On Thu, Aug 22, 2019 at 01:18:21PM +0000, Robert Chiras wrote:
> Hi Guido,
>
> I added my signed-off, plus some comments inline.
>
> On Jo, 2019-08-22 at 12:44 +0200, Guido Günther wrote:
> > This adds initial support for the NWL MIPI DSI Host controller found
> > on
> > i.MX8 SoCs.
> >
> > It adds support for the i.MX8MQ but the same IP can be found on
> > e.g. the i.MX8QXP.
> >
> > It has been tested on the Librem 5 devkit using mxsfb.
> >
> > Signed-off-by: Guido Günther <agx@sigxcpu.org>
> Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
Thanks!
> > Co-developed-by: Robert Chiras <robert.chiras@nxp.com>
> > ---
> > drivers/gpu/drm/bridge/Kconfig | 2 +
> > drivers/gpu/drm/bridge/Makefile | 1 +
> > drivers/gpu/drm/bridge/nwl-dsi/Kconfig | 16 +
> > drivers/gpu/drm/bridge/nwl-dsi/Makefile | 4 +
> > drivers/gpu/drm/bridge/nwl-dsi/nwl-drv.c | 501 ++++++++++++++++
> > drivers/gpu/drm/bridge/nwl-dsi/nwl-drv.h | 65 +++
> > drivers/gpu/drm/bridge/nwl-dsi/nwl-dsi.c | 700
> > +++++++++++++++++++++++
> > drivers/gpu/drm/bridge/nwl-dsi/nwl-dsi.h | 112 ++++
> > 8 files changed, 1401 insertions(+)
> > create mode 100644 drivers/gpu/drm/bridge/nwl-dsi/Kconfig
> > create mode 100644 drivers/gpu/drm/bridge/nwl-dsi/Makefile
> > create mode 100644 drivers/gpu/drm/bridge/nwl-dsi/nwl-drv.c
> > create mode 100644 drivers/gpu/drm/bridge/nwl-dsi/nwl-drv.h
> > create mode 100644 drivers/gpu/drm/bridge/nwl-dsi/nwl-dsi.c
> > create mode 100644 drivers/gpu/drm/bridge/nwl-dsi/nwl-dsi.h
> >
> > diff --git a/drivers/gpu/drm/bridge/Kconfig
> > b/drivers/gpu/drm/bridge/Kconfig
> > index 1cc9f502c1f2..7980b5c2156f 100644
> > --- a/drivers/gpu/drm/bridge/Kconfig
> > +++ b/drivers/gpu/drm/bridge/Kconfig
> > @@ -154,6 +154,8 @@ source "drivers/gpu/drm/bridge/analogix/Kconfig"
> >
> > source "drivers/gpu/drm/bridge/adv7511/Kconfig"
> >
> > +source "drivers/gpu/drm/bridge/nwl-dsi/Kconfig"
> > +
> > source "drivers/gpu/drm/bridge/synopsys/Kconfig"
> >
> > endmenu
> > diff --git a/drivers/gpu/drm/bridge/Makefile
> > b/drivers/gpu/drm/bridge/Makefile
> > index 4934fcf5a6f8..d9f6c0f77592 100644
> > --- a/drivers/gpu/drm/bridge/Makefile
> > +++ b/drivers/gpu/drm/bridge/Makefile
> > @@ -16,4 +16,5 @@ obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
> > obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/
> > obj-$(CONFIG_DRM_TI_SN65DSI86) += ti-sn65dsi86.o
> > obj-$(CONFIG_DRM_TI_TFP410) += ti-tfp410.o
> > +obj-$(CONFIG_DRM_NWL_MIPI_DSI) += nwl-dsi/
> > obj-y += synopsys/
> > diff --git a/drivers/gpu/drm/bridge/nwl-dsi/Kconfig
> > b/drivers/gpu/drm/bridge/nwl-dsi/Kconfig
> > new file mode 100644
> > index 000000000000..3b157a9f2229
> > --- /dev/null
> > +++ b/drivers/gpu/drm/bridge/nwl-dsi/Kconfig
> > @@ -0,0 +1,16 @@
> > +config DRM_NWL_MIPI_DSI
> > + tristate "Support for Northwest Logic MIPI DSI Host
> > controller"
> > + depends on DRM
> > + depends on COMMON_CLK
> > + depends on OF && HAS_IOMEM
> > + select DRM_KMS_HELPER
> > + select DRM_MIPI_DSI
> > + select DRM_PANEL_BRIDGE
> > + select GENERIC_PHY_MIPI_DPHY
> > + select MFD_SYSCON
> > + select MULTIPLEXER
> > + select REGMAP_MMIO
> > + help
> > + This enables the Northwest Logic MIPI DSI Host controller
> > as
> > + for example found on NXP's i.MX8 Processors.
> > +
> > diff --git a/drivers/gpu/drm/bridge/nwl-dsi/Makefile
> > b/drivers/gpu/drm/bridge/nwl-dsi/Makefile
> > new file mode 100644
> > index 000000000000..804baf2f1916
> > --- /dev/null
> > +++ b/drivers/gpu/drm/bridge/nwl-dsi/Makefile
> > @@ -0,0 +1,4 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +nwl-mipi-dsi-y := nwl-drv.o nwl-dsi.o
> > +obj-$(CONFIG_DRM_NWL_MIPI_DSI) += nwl-mipi-dsi.o
> > +header-test-y += nwl-drv.h nwl-dsi.h
> > diff --git a/drivers/gpu/drm/bridge/nwl-dsi/nwl-drv.c
> > b/drivers/gpu/drm/bridge/nwl-dsi/nwl-drv.c
> > new file mode 100644
> > index 000000000000..e457438738c0
> > --- /dev/null
> > +++ b/drivers/gpu/drm/bridge/nwl-dsi/nwl-drv.c
> > @@ -0,0 +1,501 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * i.MX8 NWL MIPI DSI host driver
> > + *
> > + * Copyright (C) 2017 NXP
> > + * Copyright (C) 2019 Purism SPC
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/irq.h>
> > +#include <linux/mfd/syscon.h>
> > +#include <linux/module.h>
> > +#include <linux/mux/consumer.h>
> > +#include <linux/of.h>
> > +#include <linux/of_platform.h>
> > +#include <linux/phy/phy.h>
> > +#include <linux/reset.h>
> > +#include <linux/regmap.h>
> > +#include <linux/sys_soc.h>
> > +
> > +#include <drm/drm_atomic_helper.h>
> > +#include <drm/drm_of.h>
> > +#include <drm/drm_print.h>
> > +#include <drm/drm_probe_helper.h>
> > +
> > +#include "nwl-drv.h"
> > +#include "nwl-dsi.h"
> > +
> > +#define DRV_NAME "nwl-dsi"
> > +
> > +/* Possible platform specific clocks */
> > +#define NWL_DSI_CLK_CORE "core"
> > +
> > +static const struct regmap_config nwl_dsi_regmap_config = {
> > + .reg_bits = 16,
> > + .val_bits = 32,
> > + .reg_stride = 4,
> > + .max_register = NWL_DSI_IRQ_MASK2,
> > + .name = DRV_NAME,
> > +};
> > +
> > +struct nwl_dsi_platform_data {
> > + int (*poweron)(struct nwl_dsi *dsi);
> > + int (*poweroff)(struct nwl_dsi *dsi);
> > + int (*select_input)(struct nwl_dsi *dsi);
> > + int (*deselect_input)(struct nwl_dsi *dsi);
> > + struct nwl_dsi_plat_clk_config
> > clk_config[NWL_DSI_MAX_PLATFORM_CLOCKS];
> > +};
> > +
> > +static inline struct nwl_dsi *bridge_to_dsi(struct drm_bridge
> > *bridge)
> > +{
> > + return container_of(bridge, struct nwl_dsi, bridge);
> > +}
> > +
> > +static int nwl_dsi_set_platform_clocks(struct nwl_dsi *dsi, bool
> > enable)
> > +{
> > + struct device *dev = dsi->dev;
> > + const char *id;
> > + struct clk *clk;
> > + size_t i;
> > + unsigned long rate;
> > + int ret, result = 0;
> > +
> > + DRM_DEV_DEBUG_DRIVER(dev, "%s platform clocks\n",
> > + enable ? "enabling" : "disabling");
> > + for (i = 0; i < ARRAY_SIZE(dsi->pdata->clk_config); i++) {
> > + if (!dsi->clk_config[i].present)
> > + continue;
> > + id = dsi->clk_config[i].id;
> > + clk = dsi->clk_config[i].clk;
> > +
> > + if (enable) {
> > + ret = clk_prepare_enable(clk);
> > + if (ret < 0) {
> > + DRM_DEV_ERROR(dev,
> > + "Failed to enable %s
> > clk: %d\n",
> > + id, ret);
> > + result = result ?: ret;
> > + }
> > + rate = clk_get_rate(clk);
> > + DRM_DEV_DEBUG_DRIVER(dev, "Enabled %s clk
> > @%lu Hz\n",
> > + id, rate);
> > + } else {
> > + clk_disable_unprepare(clk);
> > + DRM_DEV_DEBUG_DRIVER(dev, "Disabled %s
> > clk\n", id);
> > + }
> > + }
> > +
> > + return result;
> > +}
> > +
> > +static int nwl_dsi_plat_enable(struct nwl_dsi *dsi)
> > +{
> > + struct device *dev = dsi->dev;
> > + int ret;
> > +
> > + if (dsi->pdata->select_input)
> > + dsi->pdata->select_input(dsi);
> > +
> > + ret = nwl_dsi_set_platform_clocks(dsi, true);
> > + if (ret < 0)
> > + return ret;
> > +
> > + ret = dsi->pdata->poweron(dsi);
> > + if (ret < 0)
> > + DRM_DEV_ERROR(dev, "Failed to power on DSI: %d\n",
> > ret);
> > + return ret;
> > +}
> > +
> > +static void nwl_dsi_plat_disable(struct nwl_dsi *dsi)
> > +{
> > + dsi->pdata->poweroff(dsi);
> > + nwl_dsi_set_platform_clocks(dsi, false);
> > + if (dsi->pdata->deselect_input)
> > + dsi->pdata->deselect_input(dsi);
> > +}
> > +
> > +static void nwl_dsi_bridge_disable(struct drm_bridge *bridge)
> > +{
> > + struct nwl_dsi *dsi = bridge_to_dsi(bridge);
> > +
> > + nwl_dsi_disable(dsi);
> > + nwl_dsi_plat_disable(dsi);
> > + pm_runtime_put(dsi->dev);
> > +}
> > +
> > +static int nwl_dsi_get_dphy_params(struct nwl_dsi *dsi,
> > + const struct drm_display_mode
> > *mode,
> > + union phy_configure_opts
> > *phy_opts)
> > +{
> > + unsigned long rate;
> > + int ret;
> > +
> > + if (dsi->lanes < 1 || dsi->lanes > 4)
> > + return -EINVAL;
> > +
> > + /*
> > + * So far the DPHY spec minimal timings work for both mixel
> > + * dphy and nwl dsi host
> > + */
> > + ret = phy_mipi_dphy_get_default_config(
> > + mode->crtc_clock * 1000,
> > + mipi_dsi_pixel_format_to_bpp(dsi->format), dsi-
> > >lanes,
> > + &phy_opts->mipi_dphy);
> > + if (ret < 0)
> > + return ret;
> > +
> > + rate = clk_get_rate(dsi->tx_esc_clk);
> > + DRM_DEV_DEBUG_DRIVER(dsi->dev, "LP clk is @%lu Hz\n", rate);
> > + phy_opts->mipi_dphy.lp_clk_rate = rate;
> > +
> > + return 0;
> > +}
> > +
> > +static bool nwl_dsi_bridge_mode_fixup(struct drm_bridge *bridge,
> > + const struct drm_display_mode
> > *mode,
> > + struct drm_display_mode
> > *adjusted_mode)
> > +{
> > + /* At least LCDIF + NWL needs active high sync */
> > + adjusted_mode->flags |= (DRM_MODE_FLAG_PHSYNC |
> > DRM_MODE_FLAG_PVSYNC);
> > + adjusted_mode->flags &= ~(DRM_MODE_FLAG_NHSYNC |
> > DRM_MODE_FLAG_NVSYNC);
> > +
> > + return true;
> > +}
> > +
> > +static enum drm_mode_status
> > +nwl_dsi_bridge_mode_valid(struct drm_bridge *bridge,
> > + const struct drm_display_mode *mode)
> > +{
> > + struct nwl_dsi *dsi = bridge_to_dsi(bridge);
> > + int bpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
> > +
> > + if (mode->clock * bpp > 15000000)
> > + return MODE_CLOCK_HIGH;
> > +
> > + if (mode->clock * bpp < 80000)
> > + return MODE_CLOCK_LOW;
> These limits (80MBPS min and 1500MBPS max) are per lane, so you should
> involve the numbers of lanes here, too. According to this formula, you
> limit the maximum DSI throughput to 1.5Gbps, while the maximum is 6Gbps
> (1.5 * 4 lanes)
You're right. I thought i had the number of lanes in but seems i
forgot. thanks.
> > +
> > + return MODE_OK;
> > +}
> > +
> > +static void
> > +nwl_dsi_bridge_mode_set(struct drm_bridge *bridge,
> > + const struct drm_display_mode *mode,
> > + const struct drm_display_mode *adjusted_mode)
> > +{
> > + struct nwl_dsi *dsi = bridge_to_dsi(bridge);
> > + struct device *dev = dsi->dev;
> > + union phy_configure_opts new_cfg;
> > + unsigned long phy_ref_rate;
> > + int ret;
> > +
> > + ret = nwl_dsi_get_dphy_params(dsi, adjusted_mode, &new_cfg);
> > + if (ret < 0)
> > + return;
> > +
> > + /*
> > + * If hs clock is unchanged, we're all good - all parameters
> > are
> > + * derived from it atm.
> > + */
> > + if (new_cfg.mipi_dphy.hs_clk_rate == dsi-
> > >phy_cfg.mipi_dphy.hs_clk_rate)
> > + return;
> > +
> > + phy_ref_rate = clk_get_rate(dsi->phy_ref_clk);
> > + DRM_DEV_DEBUG_DRIVER(dev, "PHY at ref rate: %lu\n",
> > phy_ref_rate);
> > + /* Save the new desired phy config */
> > + memcpy(&dsi->phy_cfg, &new_cfg, sizeof(new_cfg));
> > +
> > + memcpy(&dsi->mode, adjusted_mode, sizeof(dsi->mode));
> > + drm_mode_debug_printmodeline(adjusted_mode);
> > +}
> > +
> > +static void nwl_dsi_bridge_pre_enable(struct drm_bridge *bridge)
> > +{
> > + struct nwl_dsi *dsi = bridge_to_dsi(bridge);
> > +
> > + pm_runtime_get_sync(dsi->dev);
> > + nwl_dsi_plat_enable(dsi);
> > + nwl_dsi_enable(dsi);
> > +}
> > +
> > +static int nwl_dsi_bridge_attach(struct drm_bridge *bridge)
> > +{
> > + struct nwl_dsi *dsi = bridge->driver_private;
> > +
> > + return drm_bridge_attach(bridge->encoder, dsi->panel_bridge,
> > bridge);
> > +}
> > +
> > +static const struct drm_bridge_funcs nwl_dsi_bridge_funcs = {
> > + .pre_enable = nwl_dsi_bridge_pre_enable,
> > + .disable = nwl_dsi_bridge_disable,
> > + .mode_fixup = nwl_dsi_bridge_mode_fixup,
> > + .mode_set = nwl_dsi_bridge_mode_set,
> > + .mode_valid = nwl_dsi_bridge_mode_valid,
> > + .attach = nwl_dsi_bridge_attach,
> > +};
> > +
> > +static int nwl_dsi_parse_dt(struct nwl_dsi *dsi)
> > +{
> > + struct platform_device *pdev = to_platform_device(dsi->dev);
> > + struct clk *clk;
> > + const char *clk_id;
> > + void __iomem *base;
> > + int i, ret;
> > +
> > + dsi->phy = devm_phy_get(dsi->dev, "dphy");
> > + if (IS_ERR(dsi->phy)) {
> > + ret = PTR_ERR(dsi->phy);
> > + if (ret != -EPROBE_DEFER)
> > + DRM_DEV_ERROR(dsi->dev, "Could not get PHY:
> > %d\n", ret);
> > + return ret;
> > + }
> > +
> > + /* Platform dependent clocks */
> > + memcpy(dsi->clk_config, dsi->pdata->clk_config,
> > + sizeof(dsi->pdata->clk_config));
> > +
> > + for (i = 0; i < ARRAY_SIZE(dsi->pdata->clk_config); i++) {
> > + if (!dsi->clk_config[i].present)
> > + continue;
> > +
> > + clk_id = dsi->clk_config[i].id;
> > + clk = devm_clk_get(dsi->dev, clk_id);
> > + if (IS_ERR(clk)) {
> > + ret = PTR_ERR(clk);
> > + DRM_DEV_ERROR(dsi->dev, "Failed to get %s
> > clock: %d\n",
> > + clk_id, ret);
> > + return ret;
> > + }
> > + DRM_DEV_DEBUG_DRIVER(dsi->dev, "Setup clk %s (rate:
> > %lu)\n",
> > + clk_id, clk_get_rate(clk));
> > + dsi->clk_config[i].clk = clk;
> > + }
> > +
> > + /* DSI clocks */
> > + clk = devm_clk_get(dsi->dev, "phy_ref");
> > + if (IS_ERR(clk)) {
> > + ret = PTR_ERR(clk);
> > + DRM_DEV_ERROR(dsi->dev, "Failed to get phy_ref clock:
> > %d\n",
> > + ret);
> > + return ret;
> > + }
> > + dsi->phy_ref_clk = clk;
> > +
> > + clk = devm_clk_get(dsi->dev, "rx_esc");
> > + if (IS_ERR(clk)) {
> > + ret = PTR_ERR(clk);
> > + DRM_DEV_ERROR(dsi->dev, "Failed to get rx_esc clock:
> > %d\n",
> > + ret);
> > + return ret;
> > + }
> > + dsi->rx_esc_clk = clk;
> > +
> > + clk = devm_clk_get(dsi->dev, "tx_esc");
> > + if (IS_ERR(clk)) {
> > + ret = PTR_ERR(clk);
> > + DRM_DEV_ERROR(dsi->dev, "Failed to get tx_esc clock:
> > %d\n",
> > + ret);
> > + return ret;
> > + }
> > + dsi->tx_esc_clk = clk;
> > +
> > + dsi->mux = devm_mux_control_get(dsi->dev, NULL);
> > + if (IS_ERR(dsi->mux)) {
> > + ret = PTR_ERR(dsi->mux);
> > + if (ret != -EPROBE_DEFER)
> > + DRM_DEV_ERROR(dsi->dev, "Failed to get mux:
> > %d\n", ret);
> > + return ret;
> > + }
> > +
> > + base = devm_platform_ioremap_resource(pdev, 0);
> > + if (IS_ERR(base))
> > + return PTR_ERR(base);
> > +
> > + dsi->regmap =
> > + devm_regmap_init_mmio(dsi->dev, base,
> > &nwl_dsi_regmap_config);
> > + if (IS_ERR(dsi->regmap)) {
> > + ret = PTR_ERR(dsi->regmap);
> > + DRM_DEV_ERROR(dsi->dev, "Failed to create NWL DSI
> > regmap: %d\n",
> > + ret);
> > + return ret;
> > + }
> > +
> > + dsi->irq = platform_get_irq(pdev, 0);
> > + if (dsi->irq < 0) {
> > + DRM_DEV_ERROR(dsi->dev, "Failed to get device IRQ:
> > %d\n",
> > + dsi->irq);
> > + return dsi->irq;
> > + }
> > +
> > + dsi->rstc = devm_reset_control_array_get(dsi->dev, false,
> > true);
> > + if (IS_ERR(dsi->rstc)) {
> > + DRM_DEV_ERROR(dsi->dev, "Failed to get resets:
> > %ld\n",
> > + PTR_ERR(dsi->rstc));
> > + return PTR_ERR(dsi->rstc);
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int imx8mq_dsi_select_input(struct nwl_dsi *dsi)
> > +{
> > + struct device_node *remote;
> > + u32 use_dcss = 1;
> > + int ret;
> > +
> > + remote = of_graph_get_remote_node(dsi->dev->of_node, 0, 0);
> > + if (strcmp(remote->name, "lcdif") == 0)
> > + use_dcss = 0;
> > +
> > + DRM_DEV_INFO(dsi->dev, "Using %s as input source\n",
> > + (use_dcss) ? "DCSS" : "LCDIF");
> > +
> > + ret = mux_control_try_select(dsi->mux, use_dcss);
> > + if (ret < 0)
> > + DRM_DEV_ERROR(dsi->dev, "Failed to select input:
> > %d\n", ret);
> > +
> > + of_node_put(remote);
> > + return ret;
> > +}
> > +
> > +
> > +static int imx8mq_dsi_deselect_input(struct nwl_dsi *dsi)
> > +{
> > + int ret;
> > +
> > + ret = mux_control_deselect(dsi->mux);
> > + if (ret < 0)
> > + DRM_DEV_ERROR(dsi->dev, "Failed to deselect input:
> > %d\n", ret);
> > +
> > + return ret;
> > +}
> > +
> > +
> > +static int imx8mq_dsi_poweron(struct nwl_dsi *dsi)
> > +{
> > + int ret = 0;
> > +
> > + /* otherwise the display stays blank */
> > + usleep_range(200, 300);
> > +
> > + if (dsi->rstc)
> > + ret = reset_control_deassert(dsi->rstc);
> > +
> > + return ret;
> > +}
> > +
> > +static int imx8mq_dsi_poweroff(struct nwl_dsi *dsi)
> > +{
> > + int ret = 0;
> > +
> > + if (dsi->quirks & SRC_RESET_QUIRK)
> > + return 0;
> > +
> > + if (dsi->rstc)
> > + ret = reset_control_assert(dsi->rstc);
> > + return ret;
> > +}
> > +
> > +static const struct drm_bridge_timings nwl_dsi_timings = {
> > + .input_bus_flags = DRM_BUS_FLAG_DE_LOW,
> > +};
> > +
> > +static const struct nwl_dsi_platform_data imx8mq_dev = {
> > + .poweron = &imx8mq_dsi_poweron,
> > + .poweroff = &imx8mq_dsi_poweroff,
> > + .select_input = &imx8mq_dsi_select_input,
> > + .deselect_input = &imx8mq_dsi_deselect_input,
> > + .clk_config = {
> > + { .id = NWL_DSI_CLK_CORE, .present = true },
> > + },
> > +};
> > +
> > +static const struct of_device_id nwl_dsi_dt_ids[] = {
> > + { .compatible = "fsl,imx8mq-nwl-dsi", .data = &imx8mq_dev, },
> > + { /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(of, nwl_dsi_dt_ids);
> > +
> > +static const struct soc_device_attribute nwl_dsi_quirks_match[] = {
> > + { .soc_id = "i.MX8MQ", .revision = "2.0",
> > + .data = (void *)(E11418_HS_MODE_QUIRK | SRC_RESET_QUIRK) },
> > + { /* sentinel. */ },
> > +};
> > +
> > +static int nwl_dsi_probe(struct platform_device *pdev)
> > +{
> > + struct device *dev = &pdev->dev;
> > + const struct of_device_id *of_id =
> > of_match_device(nwl_dsi_dt_ids, dev);
> > + const struct nwl_dsi_platform_data *pdata = of_id->data;
> > + const struct soc_device_attribute *attr;
> > + struct nwl_dsi *dsi;
> > + int ret;
> > +
> > + dsi = devm_kzalloc(dev, sizeof(*dsi), GFP_KERNEL);
> > + if (!dsi)
> > + return -ENOMEM;
> > +
> > + dsi->dev = dev;
> > + dsi->pdata = pdata;
> > +
> > + ret = nwl_dsi_parse_dt(dsi);
> > + if (ret)
> > + return ret;
> > +
> > + ret = devm_request_irq(dev, dsi->irq, nwl_dsi_irq_handler, 0,
> > + dev_name(dev), dsi);
> > + if (ret < 0) {
> > + DRM_DEV_ERROR(dev, "Failed to request IRQ %d: %d\n",
> > dsi->irq,
> > + ret);
> > + return ret;
> > + }
> > +
> > + dsi->dsi_host.ops = &nwl_dsi_host_ops;
> > + dsi->dsi_host.dev = dev;
> > + ret = mipi_dsi_host_register(&dsi->dsi_host);
> > + if (ret) {
> > + DRM_DEV_ERROR(dev, "Failed to register MIPI host:
> > %d\n", ret);
> > + return ret;
> > + }
> > +
> > + attr = soc_device_match(nwl_dsi_quirks_match);
> > + if (attr)
> > + dsi->quirks = (uintptr_t)attr->data;
> > +
> > + dsi->bridge.driver_private = dsi;
> > + dsi->bridge.funcs = &nwl_dsi_bridge_funcs;
> > + dsi->bridge.of_node = dev->of_node;
> > + dsi->bridge.timings = &nwl_dsi_timings;
> > +
> > + drm_bridge_add(&dsi->bridge);
> > +
> > + dev_set_drvdata(dev, dsi);
> > + pm_runtime_enable(dev);
> > + return 0;
> > +}
> > +
> > +static int nwl_dsi_remove(struct platform_device *pdev)
> > +{
> > + struct nwl_dsi *dsi = platform_get_drvdata(pdev);
> > +
> > + mipi_dsi_host_unregister(&dsi->dsi_host);
> > + pm_runtime_disable(&pdev->dev);
> > + return 0;
> > +}
> > +
> > +static struct platform_driver nwl_dsi_driver = {
> > + .probe = nwl_dsi_probe,
> > + .remove = nwl_dsi_remove,
> > + .driver = {
> > + .of_match_table = nwl_dsi_dt_ids,
> > + .name = DRV_NAME,
> > + },
> > +};
> > +
> > +module_platform_driver(nwl_dsi_driver);
> > +
> > +MODULE_AUTHOR("NXP Semiconductor");
> > +MODULE_AUTHOR("Purism SPC");
> > +MODULE_DESCRIPTION("Northwest Logic MIPI-DSI driver");
> > +MODULE_LICENSE("GPL"); /* GPLv2 or later */
> > diff --git a/drivers/gpu/drm/bridge/nwl-dsi/nwl-drv.h
> > b/drivers/gpu/drm/bridge/nwl-dsi/nwl-drv.h
> > new file mode 100644
> > index 000000000000..1e72a9221401
> > --- /dev/null
> > +++ b/drivers/gpu/drm/bridge/nwl-dsi/nwl-drv.h
> > @@ -0,0 +1,65 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ */
> > +/*
> > + * NWL MIPI DSI host driver
> > + *
> > + * Copyright (C) 2017 NXP
> > + * Copyright (C) 2019 Purism SPC
> > + */
> > +
> > +#ifndef __NWL_DRV_H__
> > +#define __NWL_DRV_H__
> > +
> > +#include <linux/mux/consumer.h>
> > +#include <linux/phy/phy.h>
> > +
> > +#include <drm/drm_bridge.h>
> > +#include <drm/drm_mipi_dsi.h>
> > +
> > +struct nwl_dsi_platform_data;
> > +
> > +/* i.MX8 NWL quirks */
> > +/* i.MX8MQ errata E11418 */
> > +#define E11418_HS_MODE_QUIRK BIT(0)
> > +/* Skip DSI bits in SRC on disable to avoid blank display on enable
> > */
> > +#define SRC_RESET_QUIRK BIT(1)
> > +
> > +#define NWL_DSI_MAX_PLATFORM_CLOCKS 1
> > +struct nwl_dsi_plat_clk_config {
> > + const char *id;
> > + struct clk *clk;
> > + bool present;
> > +};
> > +
> > +struct nwl_dsi {
> > + struct drm_bridge bridge;
> > + struct mipi_dsi_host dsi_host;
> > + struct drm_bridge *panel_bridge;
> > + struct device *dev;
> > + struct phy *phy;
> > + union phy_configure_opts phy_cfg;
> > + unsigned int quirks;
> > +
> > + struct regmap *regmap;
> > + int irq;
> > + struct reset_control *rstc;
> > + struct mux_control *mux;
> > +
> > + /* DSI clocks */
> > + struct clk *phy_ref_clk;
> > + struct clk *rx_esc_clk;
> > + struct clk *tx_esc_clk;
> > + /* Platform dependent clocks */
> > + struct nwl_dsi_plat_clk_config
> > clk_config[NWL_DSI_MAX_PLATFORM_CLOCKS];
> > +
> > + /* dsi lanes */
> > + u32 lanes;
> > + enum mipi_dsi_pixel_format format;
> > + struct drm_display_mode mode;
> > + unsigned long dsi_mode_flags;
> > +
> > + struct nwl_dsi_transfer *xfer;
> > +
> > + const struct nwl_dsi_platform_data *pdata;
> > +};
> > +
> > +#endif /* __NWL_DRV_H__ */
> > diff --git a/drivers/gpu/drm/bridge/nwl-dsi/nwl-dsi.c
> > b/drivers/gpu/drm/bridge/nwl-dsi/nwl-dsi.c
> > new file mode 100644
> > index 000000000000..fd030af55bb4
> > --- /dev/null
> > +++ b/drivers/gpu/drm/bridge/nwl-dsi/nwl-dsi.c
> > @@ -0,0 +1,700 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * NWL MIPI DSI host driver
> > + *
> > + * Copyright (C) 2017 NXP
> > + * Copyright (C) 2019 Purism SPC
> > + */
> > +
> > +#include <linux/bitfield.h>
> > +#include <linux/clk.h>
> > +#include <linux/irq.h>
> > +#include <linux/regmap.h>
> > +#include <linux/time64.h>
> > +
> > +#include <video/mipi_display.h>
> > +#include <video/videomode.h>
> > +
> > +#include <drm/drm_atomic_helper.h>
> > +#include <drm/drm_crtc_helper.h>
> > +#include <drm/drm_of.h>
> > +#include <drm/drm_panel.h>
> > +#include <drm/drm_print.h>
> > +
> > +#include "nwl-drv.h"
> > +#include "nwl-dsi.h"
> > +
> > +#define NWL_DSI_MIPI_FIFO_TIMEOUT msecs_to_jiffies(500)
> > +
> > +/*
> > + * PKT_CONTROL format:
> > + * [15: 0] - word count
> > + * [17:16] - virtual channel
> > + * [23:18] - data type
> > + * [24] - LP or HS select (0 - LP, 1 - HS)
> > + * [25] - perform BTA after packet is sent
> > + * [26] - perform BTA only, no packet tx
> > + */
> > +#define NWL_DSI_WC(x) FIELD_PREP(GENMASK(15, 0), (x))
> > +#define NWL_DSI_TX_VC(x) FIELD_PREP(GENMASK(17, 16), (x))
> > +#define NWL_DSI_TX_DT(x) FIELD_PREP(GENMASK(23, 18), (x))
> > +#define NWL_DSI_HS_SEL(x) FIELD_PREP(GENMASK(24, 24), (x))
> > +#define NWL_DSI_BTA_TX(x) FIELD_PREP(GENMASK(25, 25), (x))
> > +#define NWL_DSI_BTA_NO_TX(x) FIELD_PREP(GENMASK(26, 26), (x))
> > +
> > +/*
> > + * RX_PKT_HEADER format:
> > + * [15: 0] - word count
> > + * [21:16] - data type
> > + * [23:22] - virtual channel
> > + */
> > +#define NWL_DSI_RX_DT(x) FIELD_GET(GENMASK(21, 16), (x))
> > +#define NWL_DSI_RX_VC(x) FIELD_GET(GENMASK(23, 22), (x))
> > +
> > +/* DSI Video mode */
> > +#define NWL_DSI_VM_BURST_MODE_WITH_SYNC_PULSES 0
> > +#define NWL_DSI_VM_NON_BURST_MODE_WITH_SYNC_EVENTS BIT(0)
> > +#define NWL_DSI_VM_BURST_MODE BIT(1)
> > +
> > +/* * DPI color coding */
> > +#define NWL_DSI_DPI_16_BIT_565_PACKED 0
> > +#define NWL_DSI_DPI_16_BIT_565_ALIGNED 1
> > +#define NWL_DSI_DPI_16_BIT_565_SHIFTED 2
> > +#define NWL_DSI_DPI_18_BIT_PACKED 3
> > +#define NWL_DSI_DPI_18_BIT_ALIGNED 4
> > +#define NWL_DSI_DPI_24_BIT 5
> > +
> > +/* * DPI Pixel format */
> > +#define NWL_DSI_PIXEL_FORMAT_16 0
> > +#define NWL_DSI_PIXEL_FORMAT_18 BIT(0)
> > +#define NWL_DSI_PIXEL_FORMAT_18L BIT(1)
> > +#define NWL_DSI_PIXEL_FORMAT_24 (BIT(0) | BIT(1))
> > +
> > +enum transfer_direction {
> > + DSI_PACKET_SEND,
> > + DSI_PACKET_RECEIVE,
> > +};
> > +
> > +struct nwl_dsi_transfer {
> > + const struct mipi_dsi_msg *msg;
> > + struct mipi_dsi_packet packet;
> > + struct completion completed;
> > +
> > + int status; /* status of transmission */
> > + enum transfer_direction direction;
> > + bool need_bta;
> > + u8 cmd;
> > + u16 rx_word_count;
> > + size_t tx_len; /* in bytes */
> > + size_t rx_len; /* in bytes */
> > +};
> > +
> > +static int nwl_dsi_write(struct nwl_dsi *dsi, unsigned int reg, u32
> > val)
> > +{
> > + int ret;
> > +
> > + ret = regmap_write(dsi->regmap, reg, val);
> > + if (ret < 0)
> > + DRM_DEV_ERROR(dsi->dev,
> > + "Failed to write NWL DSI reg 0x%x:
> > %d\n", reg,
> > + ret);
> > + return ret;
> > +}
> > +
> > +static u32 nwl_dsi_read(struct nwl_dsi *dsi, u32 reg)
> > +{
> > + unsigned int val;
> > + int ret;
> > +
> > + ret = regmap_read(dsi->regmap, reg, &val);
> > + if (ret < 0)
> > + DRM_DEV_ERROR(dsi->dev, "Failed to read NWL DSI reg
> > 0x%x: %d\n",
> > + reg, ret);
> > +
> > + return val;
> > +}
> > +
> > +static u32 nwl_dsi_get_dpi_pixel_format(enum mipi_dsi_pixel_format
> > format)
> > +{
> > + switch (format) {
> > + case MIPI_DSI_FMT_RGB565:
> > + return NWL_DSI_PIXEL_FORMAT_16;
> > + case MIPI_DSI_FMT_RGB666:
> > + return NWL_DSI_PIXEL_FORMAT_18L;
> > + case MIPI_DSI_FMT_RGB666_PACKED:
> > + return NWL_DSI_PIXEL_FORMAT_18;
> > + case MIPI_DSI_FMT_RGB888:
> > + return NWL_DSI_PIXEL_FORMAT_24;
> > + default:
> > + return -EINVAL;
> > + }
> > +}
> > +
> > +#define PSEC_PER_SEC 1000000000000LL
> > +/*
> > + * ps2bc - Picoseconds to byte clock cycles
> > + */
> > +static u32 ps2bc(struct nwl_dsi *dsi, unsigned long long ps)
> > +{
> > + int bpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
> > +
> > + return DIV_ROUND_UP(ps * dsi->mode.clock * 1000 * bpp,
> > + dsi->lanes * 8 * PSEC_PER_SEC);
> > +}
> > +
> > +/*
> > + * ui2bc - UI time periods to byte clock cycles
> > + */
> > +static u32 ui2bc(struct nwl_dsi *dsi, unsigned long long ui)
> > +{
> > + int bpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
> > +
> > + return DIV_ROUND_UP(ui * dsi->lanes, dsi->mode.clock * 1000 *
> > bpp);
> > +}
> > +
> > +/*
> > + * us2bc - micro seconds to lp clock cycles
> > + */
> > +static u32 us2lp(u32 lp_clk_rate, unsigned long us)
> > +{
> > + return DIV_ROUND_UP(us * lp_clk_rate, USEC_PER_SEC);
> > +}
> > +
> > +static int nwl_dsi_config_host(struct nwl_dsi *dsi)
> > +{
> > + u32 cycles;
> > + struct phy_configure_opts_mipi_dphy *cfg = &dsi-
> > >phy_cfg.mipi_dphy;
> > +
> > + if (dsi->lanes < 1 || dsi->lanes > 4)
> > + return -EINVAL;
> > +
> > + DRM_DEV_DEBUG_DRIVER(dsi->dev, "DSI Lanes %d\n", dsi->lanes);
> > + nwl_dsi_write(dsi, NWL_DSI_CFG_NUM_LANES, dsi->lanes - 1);
> > +
> > + if (dsi->dsi_mode_flags & MIPI_DSI_CLOCK_NON_CONTINUOUS) {
> > + nwl_dsi_write(dsi, NWL_DSI_CFG_NONCONTINUOUS_CLK,
> > 0x01);
> > + nwl_dsi_write(dsi, NWL_DSI_CFG_AUTOINSERT_EOTP,
> > 0x01);
> > + } else {
> > + nwl_dsi_write(dsi, NWL_DSI_CFG_NONCONTINUOUS_CLK,
> > 0x00);
> > + nwl_dsi_write(dsi, NWL_DSI_CFG_AUTOINSERT_EOTP,
> > 0x00);
> > + }
> > +
> > + /* values in byte clock cycles */
> > + cycles = ui2bc(dsi, cfg->clk_pre);
> > + DRM_DEV_DEBUG_DRIVER(dsi->dev, "cfg_t_pre: 0x%x\n", cycles);
> > + nwl_dsi_write(dsi, NWL_DSI_CFG_T_PRE, cycles);
> > + cycles = ps2bc(dsi, cfg->lpx + cfg->clk_prepare + cfg-
> > >clk_zero);
> > + DRM_DEV_DEBUG_DRIVER(dsi->dev, "cfg_tx_gap (pre): 0x%x\n",
> > cycles);
> > + cycles += ui2bc(dsi, cfg->clk_pre);
> > + DRM_DEV_DEBUG_DRIVER(dsi->dev, "cfg_tx_gap: 0x%x\n", cycles);
> > + nwl_dsi_write(dsi, NWL_DSI_CFG_T_POST, cycles);
> > + cycles = ps2bc(dsi, cfg->hs_exit);
> > + DRM_DEV_DEBUG_DRIVER(dsi->dev, "cfg_tx_gap: 0x%x\n", cycles);
> > + nwl_dsi_write(dsi, NWL_DSI_CFG_TX_GAP, cycles);
> > +
> > + nwl_dsi_write(dsi, NWL_DSI_CFG_EXTRA_CMDS_AFTER_EOTP, 0x01);
> > + nwl_dsi_write(dsi, NWL_DSI_CFG_HTX_TO_COUNT, 0x00);
> > + nwl_dsi_write(dsi, NWL_DSI_CFG_LRX_H_TO_COUNT, 0x00);
> > + nwl_dsi_write(dsi, NWL_DSI_CFG_BTA_H_TO_COUNT, 0x00);
> > + /* In LP clock cycles */
> > + cycles = us2lp(cfg->lp_clk_rate, cfg->wakeup);
> > + DRM_DEV_DEBUG_DRIVER(dsi->dev, "cfg_twakeup: 0x%x\n",
> > cycles);
> > + nwl_dsi_write(dsi, NWL_DSI_CFG_TWAKEUP, cycles);
> > +
> > + return 0;
> > +}
> > +
> > +static int nwl_dsi_config_dpi(struct nwl_dsi *dsi)
> > +{
> > + u32 color_format, mode;
> > + bool burst_mode;
> > + int hfront_porch, hback_porch, vfront_porch, vback_porch;
> > + int hsync_len, vsync_len;
> > +
> > + hfront_porch = dsi->mode.hsync_start - dsi->mode.hdisplay;
> > + hsync_len = dsi->mode.hsync_end - dsi->mode.hsync_start;
> > + hback_porch = dsi->mode.htotal - dsi->mode.hsync_end;
> > +
> > + vfront_porch = dsi->mode.vsync_start - dsi->mode.vdisplay;
> > + vsync_len = dsi->mode.vsync_end - dsi->mode.vsync_start;
> > + vback_porch = dsi->mode.vtotal - dsi->mode.vsync_end;
> > +
> > + DRM_DEV_DEBUG_DRIVER(dsi->dev, "hfront_porch = %d\n",
> > hfront_porch);
> > + DRM_DEV_DEBUG_DRIVER(dsi->dev, "hback_porch = %d\n",
> > hback_porch);
> > + DRM_DEV_DEBUG_DRIVER(dsi->dev, "hsync_len = %d\n",
> > hsync_len);
> > + DRM_DEV_DEBUG_DRIVER(dsi->dev, "hdisplay = %d\n", dsi-
> > >mode.hdisplay);
> > + DRM_DEV_DEBUG_DRIVER(dsi->dev, "vfront_porch = %d\n",
> > vfront_porch);
> > + DRM_DEV_DEBUG_DRIVER(dsi->dev, "vback_porch = %d\n",
> > vback_porch);
> > + DRM_DEV_DEBUG_DRIVER(dsi->dev, "vsync_len = %d\n",
> > vsync_len);
> > + DRM_DEV_DEBUG_DRIVER(dsi->dev, "vactive = %d\n", dsi-
> > >mode.vdisplay);
> > + DRM_DEV_DEBUG_DRIVER(dsi->dev, "clock = %d kHz\n", dsi-
> > >mode.clock);
> > +
> > + color_format = nwl_dsi_get_dpi_pixel_format(dsi->format);
> > + if (color_format < 0) {
> > + DRM_DEV_ERROR(dsi->dev, "Invalid color format
> > 0x%x\n",
> > + dsi->format);
> > + return color_format;
> > + }
> > + DRM_DEV_DEBUG_DRIVER(dsi->dev, "pixel fmt = %d\n", dsi-
> > >format);
> > +
> > + nwl_dsi_write(dsi, NWL_DSI_INTERFACE_COLOR_CODING,
> > NWL_DSI_DPI_24_BIT);
> > + nwl_dsi_write(dsi, NWL_DSI_PIXEL_FORMAT, color_format);
> > + /*
> > + * Adjusting input polarity based on the video mode results
> > in
> > + * a black screen so always pick active low:
> > + */
> > + nwl_dsi_write(dsi, NWL_DSI_VSYNC_POLARITY,
> > + NWL_DSI_VSYNC_POLARITY_ACTIVE_LOW);
> > + nwl_dsi_write(dsi, NWL_DSI_HSYNC_POLARITY,
> > + NWL_DSI_HSYNC_POLARITY_ACTIVE_LOW);
> > +
> > + burst_mode = (dsi->dsi_mode_flags &
> > MIPI_DSI_MODE_VIDEO_BURST) &&
> > + !(dsi->dsi_mode_flags &
> > MIPI_DSI_MODE_VIDEO_SYNC_PULSE);
> > +
> > + if (burst_mode) {
> > + nwl_dsi_write(dsi, NWL_DSI_VIDEO_MODE,
> > NWL_DSI_VM_BURST_MODE);
> > + nwl_dsi_write(dsi, NWL_DSI_PIXEL_FIFO_SEND_LEVEL,
> > 256);
> > + } else {
> > + mode = ((dsi->dsi_mode_flags &
> > MIPI_DSI_MODE_VIDEO_SYNC_PULSE) ?
> > + NWL_DSI_VM_BURST_MODE_WITH_SYNC_PULSE
> > S :
> > + NWL_DSI_VM_NON_BURST_MODE_WITH_SYNC_E
> > VENTS);
> > + nwl_dsi_write(dsi, NWL_DSI_VIDEO_MODE, mode);
> > + nwl_dsi_write(dsi, NWL_DSI_PIXEL_FIFO_SEND_LEVEL,
> > + dsi->mode.hdisplay);
> > + }
> > +
> > + nwl_dsi_write(dsi, NWL_DSI_HFP, hfront_porch);
> > + nwl_dsi_write(dsi, NWL_DSI_HBP, hback_porch);
> > + nwl_dsi_write(dsi, NWL_DSI_HSA, hsync_len);
> > +
> > + nwl_dsi_write(dsi, NWL_DSI_ENABLE_MULT_PKTS, 0x0);
> > + nwl_dsi_write(dsi, NWL_DSI_BLLP_MODE, 0x1);
> > + nwl_dsi_write(dsi, NWL_DSI_ENABLE_MULT_PKTS, 0x0);
> NWL_DSI_ENABLE_MULT_PKTS is written twice, by mistake, so remove this
> line. I did the same mistake on NXP tree, I think you got it from there
Dropped.
> :)
> > + nwl_dsi_write(dsi, NWL_DSI_USE_NULL_PKT_BLLP, 0x0);
> > + nwl_dsi_write(dsi, NWL_DSI_VC, 0x0);
> > +
> > + nwl_dsi_write(dsi, NWL_DSI_PIXEL_PAYLOAD_SIZE, dsi-
> > >mode.hdisplay);
> > + nwl_dsi_write(dsi, NWL_DSI_VACTIVE, dsi->mode.vdisplay - 1);
> VACTIVE shold contain the number of lines in the vertical area. "-1"
> subtraction was actually wrong.
So is that an error in the imx8MQ reference manual which says in
MIPI_DSI_HOST_DPI_INTFC_DSI_HOST_CFG_DPI_VACTIVE:
Sets the number of lines in the vertical active area. This field is
equivalent to (real vertical size) - 1. For example, for an image of
size 640x480, the bit field should be set as 479.
Is so I'll fix that but it'd be great if you could confirm. I don't see
any visual difference with the panel i have here.
> > + nwl_dsi_write(dsi, NWL_DSI_VBP, vback_porch);
> > + nwl_dsi_write(dsi, NWL_DSI_VFP, vfront_porch);
> > +
> > + return 0;
> > +}
> > +
> > +static void nwl_dsi_init_interrupts(struct nwl_dsi *dsi)
> > +{
> > + u32 irq_enable;
> > +
> > + nwl_dsi_write(dsi, NWL_DSI_IRQ_MASK, 0xffffffff);
> > + nwl_dsi_write(dsi, NWL_DSI_IRQ_MASK2, 0x7);
> > +
> > + irq_enable = ~(u32)(NWL_DSI_TX_PKT_DONE_MASK |
> > + NWL_DSI_RX_PKT_HDR_RCVD_MASK |
> > + NWL_DSI_TX_FIFO_OVFLW_MASK |
> > + NWL_DSI_HS_TX_TIMEOUT_MASK);
> > +
> > + nwl_dsi_write(dsi, NWL_DSI_IRQ_MASK, irq_enable);
> > +}
> > +
> > +static int nwl_dsi_host_attach(struct mipi_dsi_host *dsi_host,
> > + struct mipi_dsi_device *device)
> > +{
> > + struct nwl_dsi *dsi = container_of(dsi_host, struct nwl_dsi,
> > dsi_host);
> > + struct device *dev = dsi->dev;
> > + struct drm_bridge *bridge;
> > + struct drm_panel *panel;
> > + int ret;
> > +
> > + DRM_DEV_INFO(dev, "lanes=%u, format=0x%x flags=0x%lx\n",
> > device->lanes,
> > + device->format, device->mode_flags);
> > +
> > + if (device->lanes < 1 || device->lanes > 4)
> > + return -EINVAL;
> > +
> > + dsi->lanes = device->lanes;
> > + dsi->format = device->format;
> > + dsi->dsi_mode_flags = device->mode_flags;
> > +
> > + ret = drm_of_find_panel_or_bridge(dsi->dev->of_node, 1, 0,
> > &panel,
> > + &bridge);
> > + if (ret)
> > + return ret;
> > +
> > + if (panel) {
> > + bridge = drm_panel_bridge_add(panel,
> > DRM_MODE_CONNECTOR_DSI);
> > + if (IS_ERR(bridge))
> > + return PTR_ERR(bridge);
> > + }
> > +
> > + dsi->panel_bridge = bridge;
> > + drm_bridge_add(&dsi->bridge);
> > +
> > + return 0;
> > +}
> > +
> > +static int nwl_dsi_host_detach(struct mipi_dsi_host *dsi_host,
> > + struct mipi_dsi_device *device)
> > +{
> > + struct nwl_dsi *dsi = container_of(dsi_host, struct nwl_dsi,
> > dsi_host);
> > +
> > + drm_of_panel_bridge_remove(dsi->dev->of_node, 1, 0);
> > + drm_bridge_remove(&dsi->bridge);
> > +
> > + return 0;
> > +}
> > +
> > +static bool nwl_dsi_read_packet(struct nwl_dsi *dsi, u32 status)
> > +{
> > + struct device *dev = dsi->dev;
> > + struct nwl_dsi_transfer *xfer = dsi->xfer;
> > + u8 *payload = xfer->msg->rx_buf;
> > + u32 val;
> > + u16 word_count;
> > + u8 channel;
> > + u8 data_type;
> > +
> > + xfer->status = 0;
> > +
> > + if (xfer->rx_word_count == 0) {
> > + if (!(status & NWL_DSI_RX_PKT_HDR_RCVD))
> > + return false;
> > + /* Get the RX header and parse it */
> > + val = nwl_dsi_read(dsi, NWL_DSI_RX_PKT_HEADER);
> > + word_count = NWL_DSI_WC(val);
> > + channel = NWL_DSI_RX_VC(val);
> > + data_type = NWL_DSI_RX_DT(val);
> > +
> > + if (channel != xfer->msg->channel) {
> > + DRM_DEV_ERROR(dev,
> > + "[%02X] Channel mismatch (%u !=
> > %u)\n",
> > + xfer->cmd, channel, xfer->msg-
> > >channel);
> > + return true;
> > + }
> > +
> > + switch (data_type) {
> > + case MIPI_DSI_RX_GENERIC_SHORT_READ_RESPONSE_2BYTE:
> > + /* Fall through */
> > + case MIPI_DSI_RX_DCS_SHORT_READ_RESPONSE_2BYTE:
> > + if (xfer->msg->rx_len > 1) {
> > + /* read second byte */
> > + payload[1] = word_count >> 8;
> > + ++xfer->rx_len;
> > + }
> > + /* Fall through */
> > + case MIPI_DSI_RX_GENERIC_SHORT_READ_RESPONSE_1BYTE:
> > + /* Fall through */
> > + case MIPI_DSI_RX_DCS_SHORT_READ_RESPONSE_1BYTE:
> > + if (xfer->msg->rx_len > 0) {
> > + /* read first byte */
> > + payload[0] = word_count & 0xff;
> > + ++xfer->rx_len;
> > + }
> > + xfer->status = xfer->rx_len;
> > + return true;
> > + case MIPI_DSI_RX_ACKNOWLEDGE_AND_ERROR_REPORT:
> > + word_count &= 0xff;
> > + DRM_DEV_ERROR(dev, "[%02X] DSI error report:
> > 0x%02x\n",
> > + xfer->cmd, word_count);
> > + xfer->status = -EPROTO;
> > + return true;
> > + }
> > +
> > + if (word_count > xfer->msg->rx_len) {
> > + DRM_DEV_ERROR(
> > + dev,
> > + "[%02X] Receive buffer too small: %lu
> > (< %u)\n",
> > + xfer->cmd, xfer->msg->rx_len,
> > word_count);
> > + return true;
> > + }
> > +
> > + xfer->rx_word_count = word_count;
> > + } else {
> > + /* Set word_count from previous header read */
> > + word_count = xfer->rx_word_count;
> > + }
> > +
> > + /* If RX payload is not yet received, wait for it */
> > + if (!(status & NWL_DSI_RX_PKT_PAYLOAD_DATA_RCVD))
> > + return false;
> > +
> > + /* Read the RX payload */
> > + while (word_count >= 4) {
> > + val = nwl_dsi_read(dsi, NWL_DSI_RX_PAYLOAD);
> > + payload[0] = (val >> 0) & 0xff;
> > + payload[1] = (val >> 8) & 0xff;
> > + payload[2] = (val >> 16) & 0xff;
> > + payload[3] = (val >> 24) & 0xff;
> > + payload += 4;
> > + xfer->rx_len += 4;
> > + word_count -= 4;
> > + }
> > +
> > + if (word_count > 0) {
> > + val = nwl_dsi_read(dsi, NWL_DSI_RX_PAYLOAD);
> > + switch (word_count) {
> > + case 3:
> > + payload[2] = (val >> 16) & 0xff;
> > + ++xfer->rx_len;
> > + /* Fall through */
> > + case 2:
> > + payload[1] = (val >> 8) & 0xff;
> > + ++xfer->rx_len;
> > + /* Fall through */
> > + case 1:
> > + payload[0] = (val >> 0) & 0xff;
> > + ++xfer->rx_len;
> > + break;
> > + }
> > + }
> > +
> > + xfer->status = xfer->rx_len;
> > +
> > + return true;
> > +}
> > +
> > +static void nwl_dsi_finish_transmission(struct nwl_dsi *dsi, u32
> > status)
> > +{
> > + struct nwl_dsi_transfer *xfer = dsi->xfer;
> > + bool end_packet = false;
> > +
> > + if (!xfer)
> > + return;
> > +
> > + if (status & NWL_DSI_TX_FIFO_OVFLW) {
> > + DRM_DEV_ERROR_RATELIMITED(dsi->dev, "tx fifo
> > overflow\n");
> > + return;
> > + }
> > +
> > + if (status & NWL_DSI_HS_TX_TIMEOUT) {
> > + DRM_DEV_ERROR_RATELIMITED(dsi->dev, "HS tx
> > timeout\n");
> > + return;
> > + }
> > +
> > + if (xfer->direction == DSI_PACKET_SEND &&
> > + status & NWL_DSI_TX_PKT_DONE) {
> > + xfer->status = xfer->tx_len;
> > + end_packet = true;
> > + } else if (status & NWL_DSI_DPHY_DIRECTION &&
> > + ((status & (NWL_DSI_RX_PKT_HDR_RCVD |
> > + NWL_DSI_RX_PKT_PAYLOAD_DATA_RCVD)))) {
> > + end_packet = nwl_dsi_read_packet(dsi, status);
> > + }
> > +
> > + if (end_packet)
> > + complete(&xfer->completed);
> > +}
> > +
> > +static void nwl_dsi_begin_transmission(struct nwl_dsi *dsi)
> > +{
> > + struct nwl_dsi_transfer *xfer = dsi->xfer;
> > + struct mipi_dsi_packet *pkt = &xfer->packet;
> > + const u8 *payload;
> > + size_t length;
> > + u16 word_count;
> > + u8 hs_mode;
> > + u32 val;
> > + u32 hs_workaround = 0;
> > +
> > + /* Send the payload, if any */
> > + length = pkt->payload_length;
> > + payload = pkt->payload;
> > +
> > + while (length >= 4) {
> > + val = *(u32 *)payload;
> > + hs_workaround |= !(val & 0xFFFF00);
> > + nwl_dsi_write(dsi, NWL_DSI_TX_PAYLOAD, val);
> > + payload += 4;
> > + length -= 4;
> > + }
> > + /* Send the rest of the payload */
> > + val = 0;
> > + switch (length) {
> > + case 3:
> > + val |= payload[2] << 16;
> > + /* Fall through */
> > + case 2:
> > + val |= payload[1] << 8;
> > + hs_workaround |= !(val & 0xFFFF00);
> > + /* Fall through */
> > + case 1:
> > + val |= payload[0];
> > + nwl_dsi_write(dsi, NWL_DSI_TX_PAYLOAD, val);
> > + break;
> > + }
> > + xfer->tx_len = pkt->payload_length;
> > +
> > + /*
> > + * Send the header
> > + * header[0] = Virtual Channel + Data Type
> > + * header[1] = Word Count LSB (LP) or first param (SP)
> > + * header[2] = Word Count MSB (LP) or second param (SP)
> > + */
> > + word_count = pkt->header[1] | (pkt->header[2] << 8);
> > + if (hs_workaround && (dsi->quirks & E11418_HS_MODE_QUIRK)) {
> > + DRM_DEV_DEBUG_DRIVER(dsi->dev,
> > + "Using hs mode workaround for
> > cmd 0x%x\n",
> > + xfer->cmd);
> > + hs_mode = 1;
> > + } else {
> > + hs_mode = (xfer->msg->flags & MIPI_DSI_MSG_USE_LPM) ?
> > 0 : 1;
> > + }
> > + val = NWL_DSI_WC(word_count) | NWL_DSI_TX_VC(xfer->msg-
> > >channel) |
> > + NWL_DSI_TX_DT(xfer->msg->type) |
> > NWL_DSI_HS_SEL(hs_mode) |
> > + NWL_DSI_BTA_TX(xfer->need_bta);
> > + nwl_dsi_write(dsi, NWL_DSI_PKT_CONTROL, val);
> > +
> > + /* Send packet command */
> > + nwl_dsi_write(dsi, NWL_DSI_SEND_PACKET, 0x1);
> > +}
> > +
> > +static ssize_t nwl_dsi_host_transfer(struct mipi_dsi_host *dsi_host,
> > + const struct mipi_dsi_msg *msg)
> > +{
> > + struct nwl_dsi *dsi = container_of(dsi_host, struct nwl_dsi,
> > dsi_host);
> > + struct nwl_dsi_transfer xfer;
> > + ssize_t ret = 0;
> > +
> > + /* Create packet to be sent */
> > + dsi->xfer = &xfer;
> > + ret = mipi_dsi_create_packet(&xfer.packet, msg);
> > + if (ret < 0) {
> > + dsi->xfer = NULL;
> > + return ret;
> > + }
> > +
> > + if ((msg->type & MIPI_DSI_GENERIC_READ_REQUEST_0_PARAM ||
> > + msg->type & MIPI_DSI_GENERIC_READ_REQUEST_1_PARAM ||
> > + msg->type & MIPI_DSI_GENERIC_READ_REQUEST_2_PARAM ||
> > + msg->type & MIPI_DSI_DCS_READ) &&
> > + msg->rx_len > 0 && msg->rx_buf != NULL)
> > + xfer.direction = DSI_PACKET_RECEIVE;
> > + else
> > + xfer.direction = DSI_PACKET_SEND;
> > +
> > + xfer.need_bta = (xfer.direction == DSI_PACKET_RECEIVE);
> > + xfer.need_bta |= (msg->flags & MIPI_DSI_MSG_REQ_ACK) ? 1 : 0;
> > + xfer.msg = msg;
> > + xfer.status = -ETIMEDOUT;
> > + xfer.rx_word_count = 0;
> > + xfer.rx_len = 0;
> > + xfer.cmd = 0x00;
> > + if (msg->tx_len > 0)
> > + xfer.cmd = ((u8 *)(msg->tx_buf))[0];
> > + init_completion(&xfer.completed);
> > +
> > + ret = clk_prepare_enable(dsi->rx_esc_clk);
> > + if (ret < 0) {
> > + DRM_DEV_ERROR(dsi->dev, "Failed to enable rx_esc clk:
> > %zd\n",
> > + ret);
> > + return ret;
> > + }
> > + DRM_DEV_DEBUG_DRIVER(dsi->dev, "Enabled rx_esc clk @%lu
> > Hz\n",
> > + clk_get_rate(dsi->rx_esc_clk));
> This will be printed every time a DSI command is sent, of course, when
> debugging only, but still: are you sure you need this info?
i'd like to keep it for the moment, it's only on during init and only in
debug but helps to see clocks are toggled correctly.
> > +
> > + /* Initiate the DSI packet transmision */
> > + nwl_dsi_begin_transmission(dsi);
> > +
> > + if (!wait_for_completion_timeout(&xfer.completed,
> > + NWL_DSI_MIPI_FIFO_TIMEOUT))
> > {
> > + DRM_DEV_ERROR(dsi_host->dev, "[%02X] DSI transfer
> > timed out\n",
> > + xfer.cmd);
> > + ret = -ETIMEDOUT;
> > + } else {
> > + ret = xfer.status;
> > + }
> > +
> > + clk_disable_unprepare(dsi->rx_esc_clk);
> > +
> > + return ret;
> > +}
> > +
> > +const struct mipi_dsi_host_ops nwl_dsi_host_ops = {
> > + .attach = nwl_dsi_host_attach,
> > + .detach = nwl_dsi_host_detach,
> > + .transfer = nwl_dsi_host_transfer,
> > +};
> > +
> > +irqreturn_t nwl_dsi_irq_handler(int irq, void *data)
> > +{
> > + u32 irq_status;
> > + struct nwl_dsi *dsi = data;
> > +
> > + irq_status = nwl_dsi_read(dsi, NWL_DSI_IRQ_STATUS);
> > +
> > + if (irq_status & NWL_DSI_TX_PKT_DONE ||
> > + irq_status & NWL_DSI_RX_PKT_HDR_RCVD ||
> > + irq_status & NWL_DSI_RX_PKT_PAYLOAD_DATA_RCVD)
> > + nwl_dsi_finish_transmission(dsi, irq_status);
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
> > +int nwl_dsi_enable(struct nwl_dsi *dsi)
> > +{
> > + struct device *dev = dsi->dev;
> > + union phy_configure_opts *phy_cfg = &dsi->phy_cfg;
> > + int ret;
> > +
> > + if (!dsi->lanes) {
> > + DRM_DEV_ERROR(dev, "Need DSI lanes: %d\n", dsi-
> > >lanes);
> > + return -EINVAL;
> > + }
> > +
> > + ret = phy_init(dsi->phy);
> > + if (ret < 0) {
> > + DRM_DEV_ERROR(dev, "Failed to init DSI phy: %d\n",
> > ret);
> > + return ret;
> > + }
> > +
> > + ret = phy_configure(dsi->phy, phy_cfg);
> > + if (ret < 0) {
> > + DRM_DEV_ERROR(dev, "Failed to configure DSI phy:
> > %d\n", ret);
> > + return ret;
> > + }
> > +
> > + ret = clk_prepare_enable(dsi->tx_esc_clk);
> > + if (ret < 0) {
> > + DRM_DEV_ERROR(dsi->dev, "Failed to enable tx_esc clk:
> > %d\n",
> > + ret);
> > + return ret;
> > + }
> > + DRM_DEV_DEBUG_DRIVER(dsi->dev, "Enabled tx_esc clk @%lu
> > Hz\n",
> > + clk_get_rate(dsi->tx_esc_clk));
> > +
> > + ret = nwl_dsi_config_host(dsi);
> > + if (ret < 0) {
> > + DRM_DEV_ERROR(dev, "Failed to set up DSI: %d", ret);
> > + return ret;
> > + }
> > +
> > + ret = nwl_dsi_config_dpi(dsi);
> > + if (ret < 0) {
> > + DRM_DEV_ERROR(dev, "Failed to set up DPI: %d", ret);
> > + return ret;
> > + }
> > +
> > + ret = phy_power_on(dsi->phy);
> > + if (ret < 0) {
> > + DRM_DEV_ERROR(dev, "Failed to power on DPHY (%d)\n",
> > ret);
> > + return ret;
> > + }
> > +
> > + nwl_dsi_init_interrupts(dsi);
> > +
> > + return 0;
> > +}
> I see that you do all the initialization from the pre_enable stage. If
> you power on the DPHY, the DSI host block will start transmitting pixel
> data on the data lanes. We had some customers that complained about
> this, so I think is better to hold on the pixel flow until the whole
> pixel pipe is completely initialized.
I had the dphy power on past the dsi host setup to make sure the host
sends data matching the dphy setup. I'm happy to change that but...
> What I recommend here, is to also use the enable stage in the bridge
> and move nwl_dsi_config_host in there. Thus, the call flow will be:
> 1. bridge->pre_enable (DSI is configured, but not streaming since the
> host is not yet configured)
...what parameter exactly is gating the pixel stream in
nwl_dsi_config_host() ? because that is what would need to go into
enable at least, right? The function is mostly concerned with HS mode
setup and from the docs nothing stands out that would enable/disable the
pixel stream per se. Or do you mean nwl_dsi_config_dpi() ?
Thanks a lot for your comments!
-- Guido
> 2. panel->prepare (panel will use the DSI APB block to send DSI
> commands)
> 3. bridge->enable (DSI host block is configured, DSI starts streamming
> pixels)
> 3. panel->enable (panel is ready to display the pixel flow)
> > +
> > +int nwl_dsi_disable(struct nwl_dsi *dsi)
> > +{
> > + struct device *dev = dsi->dev;
> > +
> > + DRM_DEV_DEBUG_DRIVER(dev, "Disabling clocks and phy\n");
> > +
> > + phy_power_off(dsi->phy);
> > + phy_exit(dsi->phy);
> > +
> > + /* Disabling the clock before the phy breaks enabling dsi
> > again */
> > + clk_disable_unprepare(dsi->tx_esc_clk);
> > +
> > + return 0;
> > +}
> > diff --git a/drivers/gpu/drm/bridge/nwl-dsi/nwl-dsi.h
> > b/drivers/gpu/drm/bridge/nwl-dsi/nwl-dsi.h
> > new file mode 100644
> > index 000000000000..579b366de652
> > --- /dev/null
> > +++ b/drivers/gpu/drm/bridge/nwl-dsi/nwl-dsi.h
> > @@ -0,0 +1,112 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ */
> > +/*
> > + * NWL MIPI DSI host driver
> > + *
> > + * Copyright (C) 2017 NXP
> > + * Copyright (C) 2019 Purism SPC
> > + */
> > +#ifndef __NWL_DSI_H__
> > +#define __NWL_DSI_H__
> > +
> > +#include <linux/irqreturn.h>
> > +
> > +#include <drm/drm_mipi_dsi.h>
> > +
> > +#include "nwl-drv.h"
> > +
> > +/* DSI HOST registers */
> > +#define NWL_DSI_CFG_NUM_LANES 0x0
> > +#define NWL_DSI_CFG_NONCONTINUOUS_CLK 0x4
> > +#define NWL_DSI_CFG_T_PRE 0x8
> > +#define NWL_DSI_CFG_T_POST 0xc
> > +#define NWL_DSI_CFG_TX_GAP 0x10
> > +#define NWL_DSI_CFG_AUTOINSERT_EOTP 0x14
> > +#define NWL_DSI_CFG_EXTRA_CMDS_AFTER_EOTP 0x18
> > +#define NWL_DSI_CFG_HTX_TO_COUNT 0x1c
> > +#define NWL_DSI_CFG_LRX_H_TO_COUNT 0x20
> > +#define NWL_DSI_CFG_BTA_H_TO_COUNT 0x24
> > +#define NWL_DSI_CFG_TWAKEUP 0x28
> > +#define NWL_DSI_CFG_STATUS_OUT 0x2c
> > +#define NWL_DSI_RX_ERROR_STATUS 0x30
> > +
> > +/* DSI DPI registers */
> > +#define NWL_DSI_PIXEL_PAYLOAD_SIZE 0x200
> > +#define NWL_DSI_PIXEL_FIFO_SEND_LEVEL 0x204
> > +#define NWL_DSI_INTERFACE_COLOR_CODING 0x208
> > +#define NWL_DSI_PIXEL_FORMAT 0x20c
> > +#define NWL_DSI_VSYNC_POLARITY 0x210
> > +#define NWL_DSI_VSYNC_POLARITY_ACTIVE_LOW 0
> > +#define NWL_DSI_VSYNC_POLARITY_ACTIVE_HIGH BIT(1)
> > +
> > +#define NWL_DSI_HSYNC_POLARITY 0x214
> > +#define NWL_DSI_HSYNC_POLARITY_ACTIVE_LOW 0
> > +#define NWL_DSI_HSYNC_POLARITY_ACTIVE_HIGH BIT(1)
> > +
> > +#define NWL_DSI_VIDEO_MODE 0x218
> > +#define NWL_DSI_HFP 0x21c
> > +#define NWL_DSI_HBP 0x220
> > +#define NWL_DSI_HSA 0x224
> > +#define NWL_DSI_ENABLE_MULT_PKTS 0x228
> > +#define NWL_DSI_VBP 0x22c
> > +#define NWL_DSI_VFP 0x230
> > +#define NWL_DSI_BLLP_MODE 0x234
> > +#define NWL_DSI_USE_NULL_PKT_BLLP 0x238
> > +#define NWL_DSI_VACTIVE 0x23c
> > +#define NWL_DSI_VC 0x240
> > +
> > +/* DSI APB PKT control */
> > +#define NWL_DSI_TX_PAYLOAD 0x280
> > +#define NWL_DSI_PKT_CONTROL 0x284
> > +#define NWL_DSI_SEND_PACKET 0x288
> > +#define NWL_DSI_PKT_STATUS 0x28c
> > +#define NWL_DSI_PKT_FIFO_WR_LEVEL 0x290
> > +#define NWL_DSI_PKT_FIFO_RD_LEVEL 0x294
> > +#define NWL_DSI_RX_PAYLOAD 0x298
> > +#define NWL_DSI_RX_PKT_HEADER 0x29c
> > +
> > +/* DSI IRQ handling */
> > +#define NWL_DSI_IRQ_STATUS 0x2a0
> > +#define NWL_DSI_SM_NOT_IDLE BIT(0)
> > +#define NWL_DSI_TX_PKT_DONE BIT(1)
> > +#define NWL_DSI_DPHY_DIRECTION BIT(2)
> > +#define NWL_DSI_TX_FIFO_OVFLW BIT(3)
> > +#define NWL_DSI_TX_FIFO_UDFLW BIT(4)
> > +#define NWL_DSI_RX_FIFO_OVFLW BIT(5)
> > +#define NWL_DSI_RX_FIFO_UDFLW BIT(6)
> > +#define NWL_DSI_RX_PKT_HDR_RCVD BIT(7)
> > +#define NWL_DSI_RX_PKT_PAYLOAD_DATA_RCVD BIT(8)
> > +#define NWL_DSI_BTA_TIMEOUT BIT(29)
> > +#define NWL_DSI_LP_RX_TIMEOUT BIT(30)
> > +#define NWL_DSI_HS_TX_TIMEOUT BIT(31)
> > +
> > +#define NWL_DSI_IRQ_STATUS2 0x2a4
> > +#define NWL_DSI_SINGLE_BIT_ECC_ERR BIT(0)
> > +#define NWL_DSI_MULTI_BIT_ECC_ERR BIT(1)
> > +#define NWL_DSI_CRC_ERR BIT(2)
> > +
> > +#define NWL_DSI_IRQ_MASK 0x2a8
> > +#define NWL_DSI_SM_NOT_IDLE_MASK BIT(0)
> > +#define NWL_DSI_TX_PKT_DONE_MASK BIT(1)
> > +#define NWL_DSI_DPHY_DIRECTION_MASK BIT(2)
> > +#define NWL_DSI_TX_FIFO_OVFLW_MASK BIT(3)
> > +#define NWL_DSI_TX_FIFO_UDFLW_MASK BIT(4)
> > +#define NWL_DSI_RX_FIFO_OVFLW_MASK BIT(5)
> > +#define NWL_DSI_RX_FIFO_UDFLW_MASK BIT(6)
> > +#define NWL_DSI_RX_PKT_HDR_RCVD_MASK BIT(7)
> > +#define NWL_DSI_RX_PKT_PAYLOAD_DATA_RCVD_MASK BIT(8)
> > +#define NWL_DSI_BTA_TIMEOUT_MASK BIT(29)
> > +#define NWL_DSI_LP_RX_TIMEOUT_MASK BIT(30)
> > +#define NWL_DSI_HS_TX_TIMEOUT_MASK BIT(31)
> > +
> > +#define NWL_DSI_IRQ_MASK2 0x2ac
> > +#define NWL_DSI_SINGLE_BIT_ECC_ERR_MASK BIT(0)
> > +#define NWL_DSI_MULTI_BIT_ECC_ERR_MASK BIT(1)
> > +#define NWL_DSI_CRC_ERR_MASK BIT(2)
> > +
> > +extern const struct mipi_dsi_host_ops nwl_dsi_host_ops;
> > +
> > +irqreturn_t nwl_dsi_irq_handler(int irq, void *data);
> > +int nwl_dsi_enable(struct nwl_dsi *dsi);
> > +int nwl_dsi_disable(struct nwl_dsi *dsi);
> > +
> > +#endif /* __NWL_DSI_H__ */
> > --
> > 2.20.1
> >
> >
> Best regards,
> Robert
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v2] KVM: arm: VGIC: properly initialise private IRQ affinity
From: Andre Przywara @ 2019-08-22 17:05 UTC (permalink / raw)
To: Marc Zyngier, Christoffer Dall
Cc: Zenghui Yu, Julien Grall, Dave Martin, linux-arm-kernel, kvmarm
At the moment we initialise the target *mask* of a virtual IRQ to the
VCPU it belongs to, even though this mask is only defined for GICv2 and
quickly runs out of bits for many GICv3 guests.
This behaviour triggers an UBSAN complaint for more than 32 VCPUs:
------
[ 5659.462377] UBSAN: Undefined behaviour in virt/kvm/arm/vgic/vgic-init.c:223:21
[ 5659.471689] shift exponent 32 is too large for 32-bit type 'unsigned int'
------
Also for GICv3 guests the reporting of TARGET in the "vgic-state" debugfs
dump is wrong, due to this very same problem.
Because there is no requirement to create the VGIC device before the
VCPUs (and QEMU actually does it the other way round), we can't safely
initialise mpidr or targets in kvm_vgic_vcpu_init(). But since we touch
every private IRQ for each VCPU anyway later (in vgic_init()), we can
just move the initialisation of those fields into there, where we
definitely know the VGIC type.
On the way make sure we really have either a VGICv2 or a VGICv3 device,
since the former checks was just checking for "VGICv3 or not", silently
ignoring the uninitialised case.
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
Reported-by: Dave Martin <dave.martin@arm.com>
---
Hi,
tested with 4, 8 and 33 VCPUs with kvmtool and QEMU, on a GICv2 and a
GICv3 machine.
Also briefly tested localhost migration on the GICv3 machine w/ 33
VCPUs, although I think all IRQs are group 1.
Cheers,
Andre
virt/kvm/arm/vgic/vgic-init.c | 28 ++++++++++++++++++----------
1 file changed, 18 insertions(+), 10 deletions(-)
diff --git a/virt/kvm/arm/vgic/vgic-init.c b/virt/kvm/arm/vgic/vgic-init.c
index 80127ca9269f..413fb6a5525c 100644
--- a/virt/kvm/arm/vgic/vgic-init.c
+++ b/virt/kvm/arm/vgic/vgic-init.c
@@ -8,6 +8,7 @@
#include <linux/cpu.h>
#include <linux/kvm_host.h>
#include <kvm/arm_vgic.h>
+#include <asm/kvm_emulate.h>
#include <asm/kvm_mmu.h>
#include "vgic.h"
@@ -165,12 +166,17 @@ static int kvm_vgic_dist_init(struct kvm *kvm, unsigned int nr_spis)
irq->vcpu = NULL;
irq->target_vcpu = vcpu0;
kref_init(&irq->refcount);
- if (dist->vgic_model == KVM_DEV_TYPE_ARM_VGIC_V2) {
+ switch (dist->vgic_model) {
+ case KVM_DEV_TYPE_ARM_VGIC_V2:
irq->targets = 0;
irq->group = 0;
- } else {
+ break;
+ case KVM_DEV_TYPE_ARM_VGIC_V3:
irq->mpidr = 0;
irq->group = 1;
+ break;
+ default:
+ BUG_ON(1);
}
}
return 0;
@@ -210,7 +216,6 @@ int kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu)
irq->intid = i;
irq->vcpu = NULL;
irq->target_vcpu = vcpu;
- irq->targets = 1U << vcpu->vcpu_id;
kref_init(&irq->refcount);
if (vgic_irq_is_sgi(i)) {
/* SGIs */
@@ -220,11 +225,6 @@ int kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu)
/* PPIs */
irq->config = VGIC_CONFIG_LEVEL;
}
-
- if (dist->vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3)
- irq->group = 1;
- else
- irq->group = 0;
}
if (!irqchip_in_kernel(vcpu->kvm))
@@ -287,10 +287,18 @@ int vgic_init(struct kvm *kvm)
for (i = 0; i < VGIC_NR_PRIVATE_IRQS; i++) {
struct vgic_irq *irq = &vgic_cpu->private_irqs[i];
- if (dist->vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3)
+ switch (dist->vgic_model) {
+ case KVM_DEV_TYPE_ARM_VGIC_V3:
irq->group = 1;
- else
+ irq->mpidr = kvm_vcpu_get_mpidr_aff(vcpu);
+ break;
+ case KVM_DEV_TYPE_ARM_VGIC_V2:
irq->group = 0;
+ irq->targets = 1U << idx;
+ break;
+ default:
+ BUG_ON(1);
+ }
}
}
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: next take at setting up a dma mask by default for platform devices v2
From: Greg Kroah-Hartman @ 2019-08-22 17:11 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-arch, Gavin Li, linuxppc-dev, linux-kernel, Mathias Nyman,
Geoff Levand, Fabio Estevam, Sascha Hauer, linux-usb,
Michal Simek, iommu, Maxime Chevallier, linux-m68k, Alan Stern,
NXP Linux Team, Pengutronix Kernel Team, Minas Harutyunyan,
Shawn Guo, Bin Liu, linux-arm-kernel, Laurentiu Tudor
In-Reply-To: <20190816062435.881-1-hch@lst.de>
On Fri, Aug 16, 2019 at 08:24:29AM +0200, Christoph Hellwig wrote:
> Hi all,
>
> this is another attempt to make sure the dma_mask pointer is always
> initialized for platform devices. Not doing so lead to lots of
> boilerplate code, and makes platform devices different from all our
> major busses like PCI where we always set up a dma_mask. In the long
> run this should also help to eventually make dma_mask a scalar value
> instead of a pointer and remove even more cruft.
>
> The bigger blocker for this last time was the fact that the usb
> subsystem uses the presence or lack of a dma_mask to check if the core
> should do dma mapping for the driver, which is highly unusual. So we
> fix this first. Note that this has some overlap with the pending
> desire to use the proper dma_mmap_coherent helper for mapping usb
> buffers. The first two patches have already been queued up by Greg
> and are only included for completeness.
Note to everyone. The first two patches in this series is already in
5.3-rc5.
I've applied the rest of the series to my usb-next branch (with the 6th
patch landing there later today.) They are scheduled to be merge to
Linus in 5.4-rc1.
Christoph, thanks so much for these cleanups.
greg k-h
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v10 09/23] iommu/io-pgtable-arm-v7s: Extend to support PA[33:32] for MediaTek
From: Will Deacon @ 2019-08-22 17:14 UTC (permalink / raw)
To: Yong Wu
Cc: youlin.pei, devicetree, Nicolas Boichat, cui.zhang,
srv_heupstream, Tomasz Figa, Joerg Roedel, linux-kernel,
Evan Green, chao.hao, iommu, Rob Herring, linux-mediatek,
Matthias Brugger, ming-fan.chen, anan.sun, Robin Murphy,
Matthias Kaehlcke, linux-arm-kernel
In-Reply-To: <1566475533.11621.18.camel@mhfsdcap03>
On Thu, Aug 22, 2019 at 08:05:33PM +0800, Yong Wu wrote:
> On Thu, 2019-08-22 at 12:28 +0100, Will Deacon wrote:
> > Ok, great. Yong Wu -- are you ok respinning with the above + missing
> > brackets?
>
> Of course I can.
>
> NearlyAll the interface in this file is prefixed with "arm_v7s_", so
> does the new interface also need it?, like arm_v7s_is_mtk_enabled. And
> keep the iopte_to_paddr and paddr_to_iopte symmetrical.
>
>
> Then the final patch would looks like below, is it ok?
Looks good to me:
Acked-by: Will Deacon <will@kernel.org>
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v4 0/8] Add Bitmain BM1880 clock driver
From: Manivannan Sadhasivam @ 2019-08-22 17:24 UTC (permalink / raw)
To: sboyd, mturquette, robh+dt
Cc: devicetree, Manivannan Sadhasivam, darren.tsao, linux-kernel,
linux-arm-kernel, fisher.cheng, alec.lin, linux-clk, haitao.suo
Hello,
This patchset adds common clock driver for Bitmain BM1880 SoC clock
controller. The clock controller consists of gate, divider, mux
and pll clocks with different compositions. Hence, the driver uses
composite clock structure in place where multiple clocking units are
combined together.
This patchset also removes UART fixed clock and sources clocks from clock
controller for Sophon Edge board where the driver has been validated.
Thanks,
Mani
Changes in v4:
* Fixed devicetree binding issue
* Added ARCH_BITMAIN as the default for the clk driver
Changes in v3:
* Switched to clk_hw_{register/unregister} APIs
* Returned clk_hw from the in-driver registration helpers
Changes in v2:
* Converted the dt binding to YAML
* Incorporated review comments from Stephen (majority of change is switching
to new way of specifying clk parents)
Manivannan Sadhasivam (8):
clk: Zero init clk_init_data in helpers
clk: Warn if clk_init_data is not zero initialized
clk: Add clk_hw_unregister_composite helper function definition
dt-bindings: clock: Add devicetree binding for BM1880 SoC
arm64: dts: bitmain: Add clock controller support for BM1880 SoC
arm64: dts: bitmain: Source common clock for UART controllers
clk: Add common clock driver for BM1880 SoC
MAINTAINERS: Add entry for BM1880 SoC clock driver
.../bindings/clock/bitmain,bm1880-clk.yaml | 74 ++
MAINTAINERS | 2 +
.../boot/dts/bitmain/bm1880-sophon-edge.dts | 9 -
arch/arm64/boot/dts/bitmain/bm1880.dtsi | 28 +
drivers/clk/Kconfig | 7 +
drivers/clk/Makefile | 1 +
drivers/clk/clk-bm1880.c | 966 ++++++++++++++++++
drivers/clk/clk-composite.c | 13 +-
drivers/clk/clk-divider.c | 2 +-
drivers/clk/clk-fixed-rate.c | 2 +-
drivers/clk/clk-gate.c | 2 +-
drivers/clk/clk-mux.c | 2 +-
drivers/clk/clk.c | 8 +
include/dt-bindings/clock/bm1880-clock.h | 82 ++
14 files changed, 1184 insertions(+), 14 deletions(-)
create mode 100644 Documentation/devicetree/bindings/clock/bitmain,bm1880-clk.yaml
create mode 100644 drivers/clk/clk-bm1880.c
create mode 100644 include/dt-bindings/clock/bm1880-clock.h
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v4 1/8] clk: Zero init clk_init_data in helpers
From: Manivannan Sadhasivam @ 2019-08-22 17:24 UTC (permalink / raw)
To: sboyd, mturquette, robh+dt
Cc: devicetree, Manivannan Sadhasivam, darren.tsao, linux-kernel,
linux-arm-kernel, fisher.cheng, alec.lin, linux-clk, haitao.suo
In-Reply-To: <20190822172426.25879-1-manivannan.sadhasivam@linaro.org>
The clk_init_data struct needs to be initialized to zero for the new
parent_map implementation to work correctly. Otherwise, the member which
is available first will get processed.
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
drivers/clk/clk-composite.c | 2 +-
drivers/clk/clk-divider.c | 2 +-
drivers/clk/clk-fixed-rate.c | 2 +-
drivers/clk/clk-gate.c | 2 +-
drivers/clk/clk-mux.c | 2 +-
5 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/clk/clk-composite.c b/drivers/clk/clk-composite.c
index b06038b8f658..4d579f9d20f6 100644
--- a/drivers/clk/clk-composite.c
+++ b/drivers/clk/clk-composite.c
@@ -208,7 +208,7 @@ struct clk_hw *clk_hw_register_composite(struct device *dev, const char *name,
unsigned long flags)
{
struct clk_hw *hw;
- struct clk_init_data init;
+ struct clk_init_data init = { NULL };
struct clk_composite *composite;
struct clk_ops *clk_composite_ops;
int ret;
diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
index 3f9ff78c4a2a..65dd8137f9ec 100644
--- a/drivers/clk/clk-divider.c
+++ b/drivers/clk/clk-divider.c
@@ -471,7 +471,7 @@ static struct clk_hw *_register_divider(struct device *dev, const char *name,
{
struct clk_divider *div;
struct clk_hw *hw;
- struct clk_init_data init;
+ struct clk_init_data init = { NULL };
int ret;
if (clk_divider_flags & CLK_DIVIDER_HIWORD_MASK) {
diff --git a/drivers/clk/clk-fixed-rate.c b/drivers/clk/clk-fixed-rate.c
index a7e4aef7a376..746c3ecdc5b3 100644
--- a/drivers/clk/clk-fixed-rate.c
+++ b/drivers/clk/clk-fixed-rate.c
@@ -58,7 +58,7 @@ struct clk_hw *clk_hw_register_fixed_rate_with_accuracy(struct device *dev,
{
struct clk_fixed_rate *fixed;
struct clk_hw *hw;
- struct clk_init_data init;
+ struct clk_init_data init = { NULL };
int ret;
/* allocate fixed-rate clock */
diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c
index 1b99fc962745..8ed83ec730cb 100644
--- a/drivers/clk/clk-gate.c
+++ b/drivers/clk/clk-gate.c
@@ -141,7 +141,7 @@ struct clk_hw *clk_hw_register_gate(struct device *dev, const char *name,
{
struct clk_gate *gate;
struct clk_hw *hw;
- struct clk_init_data init;
+ struct clk_init_data init = { NULL };
int ret;
if (clk_gate_flags & CLK_GATE_HIWORD_MASK) {
diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c
index 66e91f740508..2caa6b2a9ee5 100644
--- a/drivers/clk/clk-mux.c
+++ b/drivers/clk/clk-mux.c
@@ -153,7 +153,7 @@ struct clk_hw *clk_hw_register_mux_table(struct device *dev, const char *name,
{
struct clk_mux *mux;
struct clk_hw *hw;
- struct clk_init_data init;
+ struct clk_init_data init = { NULL };
u8 width = 0;
int ret;
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v4 2/8] clk: Warn if clk_init_data is not zero initialized
From: Manivannan Sadhasivam @ 2019-08-22 17:24 UTC (permalink / raw)
To: sboyd, mturquette, robh+dt
Cc: devicetree, Manivannan Sadhasivam, darren.tsao, linux-kernel,
linux-arm-kernel, fisher.cheng, alec.lin, linux-clk, haitao.suo
In-Reply-To: <20190822172426.25879-1-manivannan.sadhasivam@linaro.org>
The new implementation for determining parent map uses multiple ways
to pass parent info. The order in which it gets processed depends on
the first available member. Hence, it is necessary to zero init the
clk_init_data struct so that the expected member gets processed correctly.
So, add a warning if multiple clk_init_data members are available during
clk registration.
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
drivers/clk/clk.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index c0990703ce54..7d6d6984c979 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -3497,6 +3497,14 @@ static int clk_core_populate_parent_map(struct clk_core *core)
if (!num_parents)
return 0;
+ /*
+ * Check for non-zero initialized clk_init_data struct. This is
+ * required because, we only require one of the (parent_names/
+ * parent_data/parent_hws) to be set at a time. Otherwise, the
+ * current code would use first available member.
+ */
+ WARN_ON((parent_names && parent_data) || (parent_names && parent_hws));
+
/*
* Avoid unnecessary string look-ups of clk_core's possible parents by
* having a cache of names/clk_hw pointers to clk_core pointers.
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v4 3/8] clk: Add clk_hw_unregister_composite helper function definition
From: Manivannan Sadhasivam @ 2019-08-22 17:24 UTC (permalink / raw)
To: sboyd, mturquette, robh+dt
Cc: devicetree, Manivannan Sadhasivam, darren.tsao, linux-kernel,
linux-arm-kernel, fisher.cheng, alec.lin, linux-clk, haitao.suo
In-Reply-To: <20190822172426.25879-1-manivannan.sadhasivam@linaro.org>
This function has been delcared but not defined anywhere. Hence, this
commit adds definition for it.
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
drivers/clk/clk-composite.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/clk/clk-composite.c b/drivers/clk/clk-composite.c
index 4d579f9d20f6..ccca58a6d271 100644
--- a/drivers/clk/clk-composite.c
+++ b/drivers/clk/clk-composite.c
@@ -344,3 +344,14 @@ void clk_unregister_composite(struct clk *clk)
clk_unregister(clk);
kfree(composite);
}
+
+void clk_hw_unregister_composite(struct clk_hw *hw)
+{
+ struct clk_composite *composite;
+
+ composite = to_clk_composite(hw);
+
+ clk_hw_unregister(hw);
+ kfree(composite);
+}
+EXPORT_SYMBOL_GPL(clk_hw_unregister_composite);
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox