* [PATCH 5/8] mailbox: tegra-hsp: Add support for shared mailboxes
From: Jon Hunter @ 2018-05-22 16:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180508114403.14499-6-mperttunen@nvidia.com>
On 08/05/18 12:44, Mikko Perttunen wrote:
> The Tegra HSP block supports 'shared mailboxes' that are simple 32-bit
> registers consisting of a FULL bit in MSB position and 31 bits of data.
> The hardware can be configured to trigger interrupts when a mailbox
> is empty or full. Add support for these shared mailboxes to the HSP
> driver.
>
> The initial use for the mailboxes is the Tegra Combined UART. For this
> purpose, we use interrupts to receive data, and spinning to wait for
> the transmit mailbox to be emptied to minimize unnecessary overhead.
>
> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
> ---
> drivers/mailbox/tegra-hsp.c | 216 +++++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 193 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/mailbox/tegra-hsp.c b/drivers/mailbox/tegra-hsp.c
> index 16eb970f2c9f..77bc8ed7ef15 100644
> --- a/drivers/mailbox/tegra-hsp.c
> +++ b/drivers/mailbox/tegra-hsp.c
> @@ -21,6 +21,11 @@
>
> #include <dt-bindings/mailbox/tegra186-hsp.h>
>
> +#include "mailbox.h"
> +
> +#define HSP_INT0_IE 0x100
> +#define HSP_INT_IR 0x304
> +
> #define HSP_INT_DIMENSIONING 0x380
> #define HSP_nSM_SHIFT 0
> #define HSP_nSS_SHIFT 4
> @@ -34,6 +39,8 @@
> #define HSP_DB_RAW 0x8
> #define HSP_DB_PENDING 0xc
>
> +#define HSP_SM_SHRD_MBOX 0x0
> +
> #define HSP_DB_CCPLEX 1
> #define HSP_DB_BPMP 3
> #define HSP_DB_MAX 7
> @@ -68,6 +75,18 @@ struct tegra_hsp_db_map {
> unsigned int index;
> };
>
> +struct tegra_hsp_mailbox {
> + struct tegra_hsp_channel channel;
> + unsigned int index;
> + bool sending;
> +};
> +
> +static inline struct tegra_hsp_mailbox *
> +channel_to_mailbox(struct tegra_hsp_channel *channel)
> +{
> + return container_of(channel, struct tegra_hsp_mailbox, channel);
> +}
> +
> struct tegra_hsp_soc {
> const struct tegra_hsp_db_map *map;
> };
> @@ -77,6 +96,7 @@ struct tegra_hsp {
> struct mbox_controller mbox;
> void __iomem *regs;
> unsigned int doorbell_irq;
> + unsigned int shared_irq;
> unsigned int num_sm;
> unsigned int num_as;
> unsigned int num_ss;
> @@ -85,6 +105,7 @@ struct tegra_hsp {
> spinlock_t lock;
>
> struct list_head doorbells;
> + struct tegra_hsp_mailbox *mailboxes;
> };
>
> static inline struct tegra_hsp *
> @@ -189,6 +210,35 @@ static irqreturn_t tegra_hsp_doorbell_irq(int irq, void *data)
> return IRQ_HANDLED;
> }
>
> +static irqreturn_t tegra_hsp_shared_irq(int irq, void *data)
> +{
> + struct tegra_hsp_mailbox *mb;
> + struct tegra_hsp *hsp = data;
> + unsigned long bit, mask;
> + u32 value;
> +
> + mask = tegra_hsp_readl(hsp, HSP_INT_IR);
> + /* Only interested in FULL interrupts */
> + mask &= 0xff << 8;
Maybe add some definitions for the above.
Should we qualify 'mask' against the HSP_INT_IE register as well?
> +
> + for_each_set_bit(bit, &mask, 16) {
> + unsigned int mb_i = bit % 8;
If you right-shifted the mask above, you could avoid this modulo.
> +
> + mb = &hsp->mailboxes[mb_i];
> +
> + if (!mb->sending) {
> + value = tegra_hsp_channel_readl(&mb->channel,
> + HSP_SM_SHRD_MBOX);
> + value &= ~BIT(31);
Similarly a definition for bit 31 may add some clarity.
> + mbox_chan_received_data(mb->channel.chan, &value);
> + tegra_hsp_channel_writel(&mb->channel, value,
> + HSP_SM_SHRD_MBOX);
> + }
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
> static struct tegra_hsp_channel *
> tegra_hsp_doorbell_create(struct tegra_hsp *hsp, const char *name,
> unsigned int master, unsigned int index)
> @@ -277,15 +327,58 @@ static void tegra_hsp_doorbell_shutdown(struct tegra_hsp_doorbell *db)
> spin_unlock_irqrestore(&hsp->lock, flags);
> }
>
> +static int tegra_hsp_mailbox_startup(struct tegra_hsp_mailbox *mb)
> +{
> + struct tegra_hsp *hsp = mb->channel.hsp;
> + u32 value;
> +
> + mb->channel.chan->txdone_method = TXDONE_BY_BLOCK;
> +
> + /* Route FULL interrupt to external IRQ 0 */
> + value = tegra_hsp_readl(hsp, HSP_INT0_IE);
> + value |= BIT(mb->index + 8);
> + tegra_hsp_writel(hsp, value, HSP_INT0_IE);
> +
> + return 0;
> +}
> +
> +static int tegra_hsp_mailbox_shutdown(struct tegra_hsp_mailbox *mb)
> +{
> + struct tegra_hsp *hsp = mb->channel.hsp;
> + u32 value;
> +
> + value = tegra_hsp_readl(hsp, HSP_INT0_IE);
> + value &= ~BIT(mb->index + 8);
> + tegra_hsp_writel(hsp, value, HSP_INT0_IE);
> +
> + return 0;
> +}
> +
> static int tegra_hsp_send_data(struct mbox_chan *chan, void *data)
> {
> struct tegra_hsp_channel *channel = chan->con_priv;
> - struct tegra_hsp_doorbell *db;
> + struct tegra_hsp_mailbox *mailbox;
> + uint32_t value;
>
> switch (channel->type) {
> case TEGRA_HSP_MBOX_TYPE_DB:
> - db = channel_to_doorbell(channel);
> - tegra_hsp_channel_writel(&db->channel, 1, HSP_DB_TRIGGER);
> + tegra_hsp_channel_writel(channel, 1, HSP_DB_TRIGGER);
> + return 0;
> + case TEGRA_HSP_MBOX_TYPE_SM:
> + mailbox = channel_to_mailbox(channel);
> + mailbox->sending = true;
> +
> + value = *(uint32_t *)data;
> + /* Mark mailbox full */
> + value |= BIT(31);
> +
> + tegra_hsp_channel_writel(channel, value, HSP_SM_SHRD_MBOX);
> +
> + while (tegra_hsp_channel_readl(channel, HSP_SM_SHRD_MBOX)
> + & BIT(31))
> + cpu_relax();
Could be nice to use readx_poll_timeout() here.
> +
> + return 0;
> }
>
> return -EINVAL;
> @@ -298,6 +391,8 @@ static int tegra_hsp_startup(struct mbox_chan *chan)
> switch (channel->type) {
> case TEGRA_HSP_MBOX_TYPE_DB:
> return tegra_hsp_doorbell_startup(channel_to_doorbell(channel));
> + case TEGRA_HSP_MBOX_TYPE_SM:
> + return tegra_hsp_mailbox_startup(channel_to_mailbox(channel));
> }
>
> return -EINVAL;
> @@ -311,6 +406,9 @@ static void tegra_hsp_shutdown(struct mbox_chan *chan)
> case TEGRA_HSP_MBOX_TYPE_DB:
> tegra_hsp_doorbell_shutdown(channel_to_doorbell(channel));
> break;
> + case TEGRA_HSP_MBOX_TYPE_SM:
> + tegra_hsp_mailbox_shutdown(channel_to_mailbox(channel));
> + break;
> }
> }
>
> @@ -364,7 +462,16 @@ static struct mbox_chan *of_tegra_hsp_xlate(struct mbox_controller *mbox,
>
> switch (type) {
> case TEGRA_HSP_MBOX_TYPE_DB:
> - return tegra_hsp_doorbell_xlate(hsp, param);
> + if (hsp->doorbell_irq)
> + return tegra_hsp_doorbell_xlate(hsp, param);
> + else
> + return ERR_PTR(-EINVAL);
> +
> + case TEGRA_HSP_MBOX_TYPE_SM:
> + if (hsp->shared_irq && param < hsp->num_sm)
> + return hsp->mailboxes[param].channel.chan;
> + else
> + return ERR_PTR(-EINVAL);
>
> default:
> return ERR_PTR(-EINVAL);
> @@ -403,6 +510,31 @@ static int tegra_hsp_add_doorbells(struct tegra_hsp *hsp)
> return 0;
> }
>
> +static int tegra_hsp_add_mailboxes(struct tegra_hsp *hsp, struct device *dev)
> +{
> + int i;
> +
> + hsp->mailboxes = devm_kcalloc(dev, hsp->num_sm, sizeof(*hsp->mailboxes),
> + GFP_KERNEL);
> + if (!hsp->mailboxes)
> + return -ENOMEM;
> +
> + for (i = 0; i < hsp->num_sm; i++) {
> + struct tegra_hsp_mailbox *mb = &hsp->mailboxes[i];
> +
> + mb->index = i;
> + mb->sending = false;
> +
> + mb->channel.hsp = hsp;
> + mb->channel.type = TEGRA_HSP_MBOX_TYPE_SM;
> + mb->channel.regs = hsp->regs + SZ_64K + i * SZ_32K;
> + mb->channel.chan = &hsp->mbox.chans[i];
> + mb->channel.chan->con_priv = &mb->channel;
> + }
> +
> + return 0;
> +}
> +
> static int tegra_hsp_probe(struct platform_device *pdev)
> {
> struct tegra_hsp *hsp;
> @@ -431,14 +563,19 @@ static int tegra_hsp_probe(struct platform_device *pdev)
> hsp->num_si = (value >> HSP_nSI_SHIFT) & HSP_nINT_MASK;
>
> err = platform_get_irq_byname(pdev, "doorbell");
> - if (err < 0) {
> - dev_err(&pdev->dev, "failed to get doorbell IRQ: %d\n", err);
> - return err;
> - }
> + if (err < 0)
> + hsp->doorbell_irq = 0;
It should not be necessary to set to zero as it should already be zero.
> + else
> + hsp->doorbell_irq = err;
>
> - hsp->doorbell_irq = err;
> + err = platform_get_irq_byname(pdev, "shared0");
> + if (err < 0)
> + hsp->shared_irq = 0;
It should not be necessary to set to zero as it should already be zero.
> + else
> + hsp->shared_irq = err;
>
> hsp->mbox.of_xlate = of_tegra_hsp_xlate;
> + /* First nSM are reserved for mailboxes */
> hsp->mbox.num_chans = 32;
> hsp->mbox.dev = &pdev->dev;
> hsp->mbox.txdone_irq = false;
> @@ -451,10 +588,22 @@ static int tegra_hsp_probe(struct platform_device *pdev)
> if (!hsp->mbox.chans)
> return -ENOMEM;
>
> - err = tegra_hsp_add_doorbells(hsp);
> - if (err < 0) {
> - dev_err(&pdev->dev, "failed to add doorbells: %d\n", err);
> - return err;
> + if (hsp->doorbell_irq) {
> + err = tegra_hsp_add_doorbells(hsp);
> + if (err < 0) {
> + dev_err(&pdev->dev, "failed to add doorbells: %d\n",
> + err);
> + return err;
> + }
> + }
> +
> + if (hsp->shared_irq) {
> + err = tegra_hsp_add_mailboxes(hsp, &pdev->dev);
> + if (err < 0) {
> + dev_err(&pdev->dev, "failed to add mailboxes: %d\n",
> + err);
> + goto remove_doorbells;
> + }
> }
>
> platform_set_drvdata(pdev, hsp);
> @@ -462,20 +611,40 @@ static int tegra_hsp_probe(struct platform_device *pdev)
> err = mbox_controller_register(&hsp->mbox);
> if (err) {
> dev_err(&pdev->dev, "failed to register mailbox: %d\n", err);
> - tegra_hsp_remove_doorbells(hsp);
> - return err;
> + goto remove_doorbells;
> }
>
> - err = devm_request_irq(&pdev->dev, hsp->doorbell_irq,
> - tegra_hsp_doorbell_irq, IRQF_NO_SUSPEND,
> - dev_name(&pdev->dev), hsp);
> - if (err < 0) {
> - dev_err(&pdev->dev, "failed to request doorbell IRQ#%u: %d\n",
> - hsp->doorbell_irq, err);
> - return err;
> + if (hsp->doorbell_irq) {
> + err = devm_request_irq(&pdev->dev, hsp->doorbell_irq,
> + tegra_hsp_doorbell_irq, IRQF_NO_SUSPEND,
> + dev_name(&pdev->dev), hsp);
> + if (err < 0) {
> + dev_err(&pdev->dev,
> + "failed to request doorbell IRQ#%u: %d\n",
> + hsp->doorbell_irq, err);
> + return err;
Clean-up?
> + }
> + }
> +
> + if (hsp->shared_irq) {
> + err = devm_request_irq(&pdev->dev, hsp->shared_irq,
> + tegra_hsp_shared_irq, 0,
> + dev_name(&pdev->dev), hsp);
> + if (err < 0) {
> + dev_err(&pdev->dev,
> + "failed to request shared0 IRQ%u: %d\n",
> + hsp->shared_irq, err);
> + return err;
Clean-up?
> + }
> }
>
> return 0;
> +
> +remove_doorbells:
> + if (hsp->doorbell_irq)
> + tegra_hsp_remove_doorbells(hsp);
> +
> + return err;
> }
>
> static int tegra_hsp_remove(struct platform_device *pdev)
> @@ -483,7 +652,8 @@ static int tegra_hsp_remove(struct platform_device *pdev)
> struct tegra_hsp *hsp = platform_get_drvdata(pdev);
>
> mbox_controller_unregister(&hsp->mbox);
> - tegra_hsp_remove_doorbells(hsp);
> + if (hsp->doorbell_irq)
> + tegra_hsp_remove_doorbells(hsp);
>
> return 0;
> }
>
Cheers
Jon
--
nvpublic
^ permalink raw reply
* [PATCH 7/8] arm64: tegra: Add nodes for tcu on Tegra194
From: Jon Hunter @ 2018-05-22 16:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180508114403.14499-8-mperttunen@nvidia.com>
On 08/05/18 12:44, Mikko Perttunen wrote:
> Add nodes required for communication through the Tegra Combined UART.
> This includes the AON HSP instance, addition of shared interrupts
> for the TOP0 HSP instance, and finally the TCU node itself. Also
> mark the HSP instances as compatible to tegra194-hsp, as the hardware
> is not identical but is compatible to tegra186-hsp.
>
> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
> ---
> arch/arm64/boot/dts/nvidia/tegra194.dtsi | 34 +++++++++++++++++++++++++++++---
> 1 file changed, 31 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/nvidia/tegra194.dtsi b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
> index 6d699815a84f..d7f780b06fe2 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra194.dtsi
> +++ b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
> @@ -217,10 +217,31 @@
> };
>
> hsp_top0: hsp at 3c00000 {
> - compatible = "nvidia,tegra186-hsp";
> + compatible = "nvidia,tegra194-hsp", "nvidia,tegra186-hsp";
I might be wrong, but I think we may need to update the binding doc
compatibility property for hsp. For example see
'Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-flowctrl.txt'.
Cheers
Jon
--
nvpublic
^ permalink raw reply
* [PATCH 8/8] arm64: tegra: Mark tcu as primary serial port on Tegra194 P2888
From: Jon Hunter @ 2018-05-22 16:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180508114403.14499-9-mperttunen@nvidia.com>
On 08/05/18 12:44, Mikko Perttunen wrote:
> The Tegra Combined UART is the proper primary serial port on P2888,
> so use it.
>
> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
> ---
> arch/arm64/boot/dts/nvidia/tegra194-p2888.dtsi | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/nvidia/tegra194-p2888.dtsi b/arch/arm64/boot/dts/nvidia/tegra194-p2888.dtsi
> index 859ab5af17c1..95e2433984f7 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra194-p2888.dtsi
> +++ b/arch/arm64/boot/dts/nvidia/tegra194-p2888.dtsi
> @@ -10,7 +10,7 @@
> aliases {
> sdhci0 = "/cbb/sdhci at 3460000";
> sdhci1 = "/cbb/sdhci at 3400000";
> - serial0 = &uartb;
> + serial0 = &tcu;
> i2c0 = "/bpmp/i2c";
> i2c1 = "/cbb/i2c at 3160000";
> i2c2 = "/cbb/i2c at c240000";
>
Acked-by: Jon Hunter <jonathanh@nvidia.com>
Cheers
Jon
--
nvpublic
^ permalink raw reply
* [PATCH v5 1/4] dt-bindings: pinctrl: qcom: add gpio-ranges, gpio-reserved-ranges
From: Rob Herring @ 2018-05-22 16:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <910e5a85a6a8020069996e2ff397c93e9c5fe18c.1526935804.git.chunkeey@gmail.com>
On Mon, May 21, 2018 at 10:57:36PM +0200, Christian Lamparter wrote:
> This patch adds the gpio-ranges and gpio-reserved-ranges property
> definitions to the binding text files supported by the pinctrl-msm
> driver framework.
>
> gpio-ranges:
> For DT-based platforms the pinctrl-msm framework currently relies
> on the deprecated-for-DT gpiochip_add_pin_range() function to add
> the range of GPIOs to be handled by the pin controller. Due to
> interactions within gpiolib code, this causes the pinctrl-msm
> driver to bail out (-517) during boot when a gpio-hog is declared.
> This can be fatal and cause the system to not boot or reset
> (for a detailed explanation and call-trace, refer to patch:
> "pinctrl: msm: fix gpio-hog related boot issues" in this series).
>
> gpio-reserved-ranges:
> The binding has been added as a precaution since the TrustZone
> firmware (aka QSEE), which is running as the hypervisor, might
> have reserved certain, but undisclosed pins. Hence reading or
> writing to the registers for those pins will cause an
> XPU violation and this subsequently crashes the kernel.
>
> Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
> ---
> .../bindings/pinctrl/qcom,apq8064-pinctrl.txt | 6 ++++++
> .../bindings/pinctrl/qcom,apq8084-pinctrl.txt | 11 +++++++++++
> .../bindings/pinctrl/qcom,ipq4019-pinctrl.txt | 6 ++++++
> .../bindings/pinctrl/qcom,ipq8064-pinctrl.txt | 6 ++++++
> .../bindings/pinctrl/qcom,ipq8074-pinctrl.txt | 10 ++++++++++
> .../bindings/pinctrl/qcom,mdm9615-pinctrl.txt | 11 +++++++++++
> .../bindings/pinctrl/qcom,msm8660-pinctrl.txt | 6 ++++++
> .../bindings/pinctrl/qcom,msm8916-pinctrl.txt | 11 +++++++++++
> .../bindings/pinctrl/qcom,msm8960-pinctrl.txt | 11 +++++++++++
> .../bindings/pinctrl/qcom,msm8974-pinctrl.txt | 6 ++++++
> .../bindings/pinctrl/qcom,msm8994-pinctrl.txt | 11 +++++++++++
> .../bindings/pinctrl/qcom,msm8996-pinctrl.txt | 11 +++++++++++
> 12 files changed, 106 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,apq8064-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/qcom,apq8064-pinctrl.txt
> index a752a4716486..7f78c6bb4e35 100644
> --- a/Documentation/devicetree/bindings/pinctrl/qcom,apq8064-pinctrl.txt
> +++ b/Documentation/devicetree/bindings/pinctrl/qcom,apq8064-pinctrl.txt
> @@ -10,6 +10,11 @@ Required properties:
> - #gpio-cells : Should be two.
> The first cell is the gpio pin number and the
> second cell is used for optional parameters.
> +- gpio-ranges: Range of pins managed by the GPIO controller.
Just 'see gpio.txt' is sufficient unless you can say how many entries.
> +
> +Optional properties:
> +
> +- gpio-reserved-ranges: Range of pins reserved by the TrustZone TEE.
ditto.
>
> Please refer to ../gpio/gpio.txt and ../interrupt-controller/interrupts.txt for
> a general description of GPIO and interrupt bindings.
> @@ -67,6 +72,7 @@ Example:
>
> pinctrl-names = "default";
> pinctrl-0 = <&gsbi5_uart_default>;
> + gpio-ranges = <&msmgpio 0 0 90>;
>
> gsbi5_uart_default: gsbi5_uart_default {
> mux {
> diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,apq8084-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/qcom,apq8084-pinctrl.txt
> index c4ea61ac56f2..362f32b945af 100644
> --- a/Documentation/devicetree/bindings/pinctrl/qcom,apq8084-pinctrl.txt
> +++ b/Documentation/devicetree/bindings/pinctrl/qcom,apq8084-pinctrl.txt
> @@ -40,6 +40,16 @@ MSM8960 platform.
> Definition: must be 2. Specifying the pin number and flags, as defined
> in <dt-bindings/gpio/gpio.h>
>
> +- gpio-ranges:
> + Usage: required
> + Value type: <prop-encoded-array>
But this is phandle with 3 cells.
Still, you don't need to redefine standard properties here. Just need to
know optional vs. required and any constraints you can define (e.g.
allowed values, number of values, etc.)
Same comments apply to the rest.
Rob
^ permalink raw reply
* [PATCH 10/14] arm64: ssbd: Add prctl interface for per-thread mitigation
From: Marc Zyngier @ 2018-05-22 16:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180522154842.pjml5lagtpyvwwqo@isilmar-4.linta.de>
On Tue, 22 May 2018 16:48:42 +0100,
Dominik Brodowski wrote:
>
>
> On Tue, May 22, 2018 at 04:06:44PM +0100, Marc Zyngier wrote:
> > If running on a system that performs dynamic SSBD mitigation, allow
> > userspace to request the mitigation for itself. This is implemented
> > as a prctl call, allowing the mitigation to be enabled or disabled at
> > will for this particular thread.
> >
> > Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> > ---
> > arch/arm64/kernel/Makefile | 1 +
> > arch/arm64/kernel/ssbd.c | 107 +++++++++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 108 insertions(+)
> > create mode 100644 arch/arm64/kernel/ssbd.c
> >
> > diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
> > index bf825f38d206..0025f8691046 100644
> > --- a/arch/arm64/kernel/Makefile
> > +++ b/arch/arm64/kernel/Makefile
> > @@ -54,6 +54,7 @@ arm64-obj-$(CONFIG_ARM64_RELOC_TEST) += arm64-reloc-test.o
> > arm64-reloc-test-y := reloc_test_core.o reloc_test_syms.o
> > arm64-obj-$(CONFIG_CRASH_DUMP) += crash_dump.o
> > arm64-obj-$(CONFIG_ARM_SDE_INTERFACE) += sdei.o
> > +arm64-obj-$(CONFIG_ARM64_SSBD) += ssbd.o
> >
> > obj-y += $(arm64-obj-y) vdso/ probes/
> > obj-m += $(arm64-obj-m)
> > diff --git a/arch/arm64/kernel/ssbd.c b/arch/arm64/kernel/ssbd.c
> > new file mode 100644
> > index 000000000000..34e3c430176b
> > --- /dev/null
> > +++ b/arch/arm64/kernel/ssbd.c
> > @@ -0,0 +1,107 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2018 ARM Ltd, All Rights Reserved.
> > + */
> > +
> > +#include <linux/sched.h>
> > +#include <linux/thread_info.h>
> > +
> > +#include <asm/cpufeature.h>
> > +
> > +/*
> > + * prctl interface for SSBD
> > + * FIXME: Drop the below ifdefery once the common interface has been merged.
> > + */
> > +#ifdef PR_SPEC_STORE_BYPASS
>
> That FIXME wants to be looked at.
It is what allowed the series to compile with mainline until last
night. Once I rebase on top of -rc7, I'll remove it.
Thanks,
M.
--
Jazz is not dead, it just smell funny.
^ permalink raw reply
* Clang arm64 build is broken
From: Andrey Konovalov @ 2018-05-22 16:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAKwvOdkedxmstRytbWeYU8Pj9ahgWOe7rODJJ8BDtRzUroQeXw@mail.gmail.com>
On Mon, May 14, 2018 at 6:24 PM, Nick Desaulniers
<ndesaulniers@google.com> wrote:
> On Fri, Apr 20, 2018 at 7:59 AM Andrey Konovalov <andreyknvl@google.com>
> wrote:
>> On Fri, Apr 20, 2018 at 10:13 AM, Marc Zyngier <marc.zyngier@arm.com>
> wrote:
>> >> The issue is that
>> >> clang doesn't know about the "S" asm constraint. I reported this to
>> >> clang [2], and hopefully this will get fixed. In the meantime, would
>> >> it possible to work around using the "S" constraint in the kernel?
>> >
>> > I have no idea, I've never used clang to build the kernel. Clang isn't
>> > really supported to build the arm64 kernel anyway (as you mention
>> > below), and working around clang deficiencies would mean that we leave
>> > with the workaround forever. I'd rather enable clang once it is at
>> > feature parity with GCC.
>
>> The fact that there are some existing issues with building arm64
>> kernel with clang doesn't sound like a good justification for adding
>> new issues :)
>
>> However in this case I do believe that this is more of a bug in clang
>> that should be fixed.
>
> Just to follow up with this thread;
>
> Support for "S" constraints is being (re-)added to Clang in:
> https://reviews.llvm.org/D46745
Hi Nick!
I can confirm that the latest clang (which includes this patch) is
able to build the kernel with CONFIG_KVM enabled.
Thanks!
^ permalink raw reply
* [PATCH v4 1/3] dt-bindings: media: rcar-vin: Add R8A77995 support
From: Rob Herring @ 2018-05-22 16:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526913942-15426-2-git-send-email-jacopo+renesas@jmondi.org>
On Mon, May 21, 2018 at 04:45:40PM +0200, Jacopo Mondi wrote:
> Add compatible string for R-Car D3 R8A7795 to list of SoCs supported by
> rcar-vin driver.
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> Acked-by: Niklas S?derlund <niklas.soderlund+renesas@ragnatech.se>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
> ---
> Documentation/devicetree/bindings/media/rcar_vin.txt | 1 +
> 1 file changed, 1 insertion(+)
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* [PATCH 1/2] regulator: add QCOM RPMh regulator driver
From: Mark Brown @ 2018-05-22 16:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <6cb2ac52-258c-332b-e912-809d16e14114@codeaurora.org>
On Thu, May 17, 2018 at 01:48:41PM -0700, David Collins wrote:
> The RPMh hardware is configured by the boot loader. The configuration
> does reflect reality; however, it cannot handle all configurations at
> initialization time. Specific headroom management typically comes up in
> modem usecases for RF supplies that are sensitive to noise. This feature
...
> If you really don't like having this feature present in the Linux RPMh
> regulator driver, then I'd be ok removing it. It is not required for
> SDM845 which the driver is initially targeting.
It's certainly going to be easier to review it separately.
> >> In the case of XOB managed LDO regulators, the LDOs physically can be
> >> configured to different voltages by the bootloader. However, the RPMh
> >> interface provides no mechanism for the application processor to read or
> >> change that voltage. Therefore, we need a way to specify such voltages in
> >> a board specific (as opposed to driver specific) manner (i.e. device tree).
> > Is the kernel somehow prevented from varying these voltages?
> Yes. Physically, there exists no RPMh register to read or write the
> voltage of LDOs managed via XOB. Additionally, the kernel running on the
> application processor is blocked from configuring the voltage via a direct
> SPMI writes by access permissions that crash the system when violated.
*sigh* Please provide feedback on the problems this (and everything
else) is causing to the team working on the firmware. The number of
issues with this interface compared to anything else we've seen is
really noticable, I see what it's trying to do with providing something
like the regulator API is doing but there's quite a lot of missing steps
in it which cause no end of problems for general purpose software
written on top of it.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180522/56c284ba/attachment-0001.sig>
^ permalink raw reply
* [PATCH v3 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings
From: Doug Anderson @ 2018-05-22 16:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <fb390ccb-927a-8449-05c3-8dcac964bfae@codeaurora.org>
Hi,
On Mon, May 21, 2018 at 5:00 PM, David Collins <collinsd@codeaurora.org> wrote:
> On 05/21/2018 11:01 AM, Doug Anderson wrote:
>> On Fri, May 18, 2018 at 5:46 PM, David Collins <collinsd@codeaurora.org> wrote:
> ...
>>> Something to keep in mind about the downstream rpmh-regulator driver is
>>> that it caches the initial voltages specified in device tree and only
>>> sends them after a consumer driver makes a regulator framework call. This
>>> saves time during boot and ensures that requests are not made for
>>> regulators that no Linux consumer cares about.
>>
>> You're saying that in the downstream driver you'd specify
>> "initial-voltage" in the device tree and:
>>
>> * This voltage would be reported by any subsequent get_voltage() calls
>>
>> * This voltage would _not_ be communicated to RPMh.
>>
>> That seems really strange because you're essentially going to be
>> returning something from get_voltage() that could be a lie. You don't
>> know if the BIOS actually set the value or not but you'll claim that
>> it did. It also doesn't seem to match what I see in the downstream
>> driver. There I see it read "qcom,init-voltage" and then do a
>> "rpmh_regulator_set_reg()". Thus my reading of the downstream driver
>> is that it should do the same requests that you're doing.
>
> In the downstream rpmh-regulator driver [1], the value specified via the
> "qcom,init-voltage" DT property is only sent to RPMh at probe time if the
> "qcom,send-defaults" property is also specified. "qcom,send-defaults" is
> currently specified only for PMI8998 BOB. The rpmh_regulator_set_reg()
> function only updates the cached RPMh request value to be sent in the next
> request. The rpmh_regulator_send_aggregate_requests() function must be
> called to actually issue the request.
Got it. This wasn't in the "snapshot" of the downstream driver that I saw.
> Returning the cached (but not sent) initial voltage equal to the min
> constraint voltage in get_voltage() calls should not cause any problems.
> This represents the highest voltage that is guaranteed to be output by the
> regulator. Consumer's should call regulator_set_voltage() to specify
> their voltage needs. If they simply call regulator_enable(), then the
> cached voltage will be sent along with the enable request.
I'm still not seeing the argument for initial-voltage here. If we
added a feature like you describe where we don't send the voltage
until the first enable couldn't we use the minimum voltage here? If a
consumer calls regulator_enable() without setting a voltage (which
seems like a terrible idea for something where the voltage could vary)
then it would end up at the minimum voltage.
>>> It is generally not safe to request all regulators to be set to the
>>> minimum allowed voltage. Special care will be needed with the upstream
>>> qcom-rpmh-regulator driver to avoid disrupting the boot up state of
>>> regulators that are needed by other subsystems. Therefore, I would like
>>> to keep the initial voltage feature supported.
>>
>> I was asking for specific examples. Do you have any?
>
> I was able to track down an example that requires initialization at a
> non-minimum voltage: PM8998 LDO 13 on SDM845 MTP. This regulator supplies
> the microSD card with a voltage in the range 1.8 V to 2.96 V. The boot
> loader leaves this regulator enabled at 2.96 V. It is only guaranteed to
> be safe to reduce the voltage to 1.8 V on UHS type cards and only after
> following the proper SD identification and command sequence.
Ironically, this is also a perfect example of why we _shouldn't_ have
an "initial-voltage" specified in the device tree. It is certainly
plausible to imagine a bootloader that might enable UHS speeds on an
SD card and leave the rail on at 1.8V. Having an initial-voltage
specified in the device tree would be a bad idea here because it's
even worse to transition to 2.96V when the card was expecting 1.8V.
I suppose this is a theoretical example since (as far as I know) no
bootloaders run the micro SD card at UHS speeds, but it is still
worrying that the kernel needs to have a hardcoded initial voltage in
it. ...basically whenever we're playing with the voltage at bootup
before a regulator has been claimed it's not amazingly safe.
Generally Linux doesn't need to worry about this because it will only
play with voltages if they are out of the constrained range, but in
Qualcomm's case where we can't read the existing voltages then we get
badness.
>> BTW: have I already said how terrible of a design it is that you can't
>> read back the voltages that the BIOS set? If we could just read back
>> what the BIOS set then everything would work great and we could stop
>> talking about this.
>
> Even if such reading were feasible, it would not help the situation
> substantially. Very few requests are made via the AP RSC before Linux
> kernel boot, so 0 V values would still be read back for most regulators.
Sure, but all the regulators we're talking about are ones where this
would help. Said another way: are there any rails that are not
touched by the bootloader where it's bad to set the minimum voltage?
OK, so how's this for a proposal then:
1. For RPMh-regulator whenever we see a "set voltage" but Linux hasn't
specified that the regulator is enabled then we don't send the
voltage, we just cache it.
2. When we see the first enable then we first send the cached voltage
and then do the enable.
3. We don't need an "initial voltage" because any rails that are
expected to be variable voltage the client should be choosing a
voltage.
...taking the SD card case as an example: if the regulator framework
says "set to the minimum: 1.8V" we'll cache this but not apply it yet
because the rail is off as far as Linux is concerned. Then when the
SD card framework starts up it will set the voltage to 3.3V which will
overwrite the cache. Then the SD card framework will enable the
regulator and RPMh will set the voltage to 3.3V and enable.
This whole thing makes me worry about another problem, though. You
say that the bootloader left the SD card rail on, right? ...but as
far as Linux is concerned the SD card rail is off (argh, it can't read
the state that the bootloader left the rail in!)
...now imagine any of the following:
* The user boots up a kernel where the SD card driver is disabled.
* The user ejects the SD card right after the bootloader used it but
before the kernel driver started.
When the kernel comes up it will believe that the SD card rail is
disabled so it won't try to disable it. So the voltage will be left
on.
You can even come up with a less contrived use case here. One could
argue that the SD card framework really ought to be ensuring VMMC and
VQMMC are off before it starts tweaking with them just in case the
bootloader left them on. Thus, it should do:
A) Turn off VMMC and VQMMC
B) Program VMMC and VQMMC to defaults
C) Turn on VMMC and VQMMC
...right now we bootup and pretend to Linux that VMMC and VQMMC start
off, so step A) will be no-op. Sigh.
Do we need to have ".is_enabled" return -ENOTRECOVERABLE to help the
regulator framework understand that we don't know the state? I think
that might require a pile of patches to the regulator framework, but
it can't be helped unless we can somehow get RPMh to give us back the
status of how the bootloader left us (if we had that, we could return
0 for anything the bootloader didn't touch and that would be correct
from the point of view of the AP).
-Doug
^ permalink raw reply
* [PATCH v2 03/40] iommu/sva: Manage process address spaces
From: Jacob Pan @ 2018-05-22 16:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <de478769-9f7a-d40b-a55e-e2c63ad883e8@arm.com>
On Thu, 17 May 2018 11:02:42 +0100
Jean-Philippe Brucker <jean-philippe.brucker@arm.com> wrote:
> On 17/05/18 00:31, Jacob Pan wrote:
> > On Fri, 11 May 2018 20:06:04 +0100
> > I am a little confused about domain vs. pasid relationship. If
> > each domain represents a address space, should there be a domain for
> > each pasid?
>
> I don't think there is a formal definition, but from previous
> discussion the consensus seems to be: domains are a collection of
> devices that have the same virtual address spaces (one or many).
>
> Keeping that definition makes things easier, in my opinion. Some time
> ago, I did try to represent PASIDs using "subdomains" (introducing a
> hierarchy of struct iommu_domain), but it required invasive changes in
> the IOMMU subsystem and probably all over the tree.
>
> You do need some kind of "root domain" for each device, so that
> "iommu_get_domain_for_dev()" still makes sense. That root domain
> doesn't have a single address space but a collection of subdomains.
> If you need this anyway, representing a PASID with an iommu_domain
> doesn't seem preferable than using a different structure (io_mm),
> because they don't have anything in common.
>
My main concern is the PASID table storage. If PASID table storage
is tied to domain, it is ok to scale up, i.e. multiple devices in a
domain share a single PASID table. But to scale down, e.g. further
partition device with VFIO mdev for assignment, each mdev may get its
own domain via vfio. But there is no IOMMU storage for PASID table at
mdev device level. Perhaps we do need root domain or some parent-child
relationship to locate PASID table.
> Thanks,
> Jean
[Jacob Pan]
^ permalink raw reply
* [PATCH v3 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings
From: Mark Brown @ 2018-05-22 16:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAD=FV=XARtQNo5Cyg78XuT26JUFgn=7BjWRnXPjP566=a-sF1w@mail.gmail.com>
On Tue, May 22, 2018 at 09:43:02AM -0700, Doug Anderson wrote:
> On Mon, May 21, 2018 at 5:00 PM, David Collins <collinsd@codeaurora.org> wrote:
> > Returning the cached (but not sent) initial voltage equal to the min
> > constraint voltage in get_voltage() calls should not cause any problems.
> > This represents the highest voltage that is guaranteed to be output by the
> > regulator. Consumer's should call regulator_set_voltage() to specify
> > their voltage needs. If they simply call regulator_enable(), then the
> > cached voltage will be sent along with the enable request.
> I'm still not seeing the argument for initial-voltage here. If we
> added a feature like you describe where we don't send the voltage
> until the first enable couldn't we use the minimum voltage here? If a
> consumer calls regulator_enable() without setting a voltage (which
> seems like a terrible idea for something where the voltage could vary)
> then it would end up at the minimum voltage.
Or if something else has already set a voltage... though shared voltage
varying regulators aren't a superb idea they do occasionally happen.
> >> I was asking for specific examples. Do you have any?
> > I was able to track down an example that requires initialization at a
> > non-minimum voltage: PM8998 LDO 13 on SDM845 MTP. This regulator supplies
> > the microSD card with a voltage in the range 1.8 V to 2.96 V. The boot
> > loader leaves this regulator enabled at 2.96 V. It is only guaranteed to
> > be safe to reduce the voltage to 1.8 V on UHS type cards and only after
> > following the proper SD identification and command sequence.
> Ironically, this is also a perfect example of why we _shouldn't_ have
> an "initial-voltage" specified in the device tree. It is certainly
> plausible to imagine a bootloader that might enable UHS speeds on an
> SD card and leave the rail on at 1.8V. Having an initial-voltage
> specified in the device tree would be a bad idea here because it's
> even worse to transition to 2.96V when the card was expecting 1.8V.
Right, this sort of situation is why the regulator API has a policy of
leaving things untouched unless it was specifically told to do
something.
> I suppose this is a theoretical example since (as far as I know) no
> bootloaders run the micro SD card at UHS speeds, but it is still
kexec is the most obvious example I can think of here. You could
probably arrange for something to patch the device tree that's provided
to the kexeced kernel to tell it about the current state but we don't
currently do anything there.
> OK, so how's this for a proposal then:
> 1. For RPMh-regulator whenever we see a "set voltage" but Linux hasn't
> specified that the regulator is enabled then we don't send the
> voltage, we just cache it.
> 2. When we see the first enable then we first send the cached voltage
> and then do the enable.
> 3. We don't need an "initial voltage" because any rails that are
> expected to be variable voltage the client should be choosing a
> voltage.
That seems relatively safe.
> You can even come up with a less contrived use case here. One could
> argue that the SD card framework really ought to be ensuring VMMC and
> VQMMC are off before it starts tweaking with them just in case the
> bootloader left them on. Thus, it should do:
> A) Turn off VMMC and VQMMC
> B) Program VMMC and VQMMC to defaults
> C) Turn on VMMC and VQMMC
> ...right now we bootup and pretend to Linux that VMMC and VQMMC start
> off, so step A) will be no-op. Sigh.
> Do we need to have ".is_enabled" return -ENOTRECOVERABLE to help the
> regulator framework understand that we don't know the state? I think
Yes, we should be doing that. Then subsystems where it's an issue can
explicitly turn off the supply.
> that might require a pile of patches to the regulator framework, but
> it can't be helped unless we can somehow get RPMh to give us back the
> status of how the bootloader left us (if we had that, we could return
> 0 for anything the bootloader didn't touch and that would be correct
> from the point of view of the AP).
I think it's fine from a framework point of view, just provide an
is_enabled() operation which returns the error. The required fiddling
in the core should be fairly minor.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180522/c6e35bbf/attachment-0001.sig>
^ permalink raw reply
* [PATCH] arm64: dts: specify 1.8V EMMC capabilities for bcm958742t
From: Scott Branden @ 2018-05-22 17:01 UTC (permalink / raw)
To: linux-arm-kernel
Specify 1.8V EMMC capabilities for bcm958742t board to indicate support
for UHS mode.
Fixes: d4b4aba6be8a ("arm64: dts: Initial DTS files for Broadcom Stingray SOC")
Signed-off-by: Scott Branden <scott.branden@broadcom.com>
---
arch/arm64/boot/dts/broadcom/stingray/bcm958742t.dts | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/arm64/boot/dts/broadcom/stingray/bcm958742t.dts b/arch/arm64/boot/dts/broadcom/stingray/bcm958742t.dts
index 6a4d19e..dd9de6b 100644
--- a/arch/arm64/boot/dts/broadcom/stingray/bcm958742t.dts
+++ b/arch/arm64/boot/dts/broadcom/stingray/bcm958742t.dts
@@ -44,3 +44,7 @@
&gphy0 {
enet-phy-lane-swap;
};
+
+&sdio0 {
+ mmc-ddr-1_8v;
+};
--
2.5.0
^ permalink raw reply related
* [PATCH 1/2] arm64: make is_permission_fault() name clearer
From: Will Deacon @ 2018-05-22 17:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180521131451.41040-2-mark.rutland@arm.com>
On Mon, May 21, 2018 at 02:14:50PM +0100, Mark Rutland wrote:
> The naming of is_permission_fault() makes it sound like it should return
> true for permission faults from EL0, but by design, it only does so for
> faults from EL1.
>
> Let's make this clear by dropping el1 in the name, as we do for
> is_el1_instruction_abort().
>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> ---
> arch/arm64/mm/fault.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
Acked-by: Will Deacon <will.deacon@arm.com>
Will
^ permalink raw reply
* [PATCH v6 5/5] drm/rockchip: support dp training outside dp firmware
From: Enric Balletbo Serra @ 2018-05-22 17:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAFqH_50Gn7SuzJo2mDjGhsKC6_H=xMVKCbr2BM82geAyZaDu+A@mail.gmail.com>
Lin,
2018-05-22 9:41 GMT+02:00 Enric Balletbo Serra <eballetbo@gmail.com>:
> Hi Lin
>
> 2018-05-22 3:08 GMT+02:00 hl <hl@rock-chips.com>:
>> Hi Enric,
>>
>>
>>
>>
>> On Monday, May 21, 2018 11:22 PM, Enric Balletbo Serra wrote:
>>>
>>> Hi Lin,
>>>
>>> 2018-05-21 11:37 GMT+02:00 Lin Huang <hl@rock-chips.com>:
>>>>
>>>> DP firmware uses fixed phy config values to do training, but some
>>>> boards need to adjust these values to fit for their unique hardware
>>>> design. So get phy config values from dts and use software link training
>>>> instead of relying on firmware, if software training fail, keep firmware
>>>> training as a fallback if sw training fails.
>>>>
>>>> Signed-off-by: Chris Zhong <zyw@rock-chips.com>
>>>> Signed-off-by: Lin Huang <hl@rock-chips.com>
>>>> Reviewed-by: Sean Paul <seanpaul@chromium.org>
>>>> ---
>>>> Changes in v2:
>>>> - update patch following Enric suggest
>>>> Changes in v3:
>>>> - use variable fw_training instead sw_training_success
>>>> - base on DP SPCE, if training fail use lower link rate to retry training
>>>> Changes in v4:
>>>> - improve cdn_dp_get_lower_link_rate() and cdn_dp_software_train_link()
>>>> follow Sean suggest
>>>> Changes in v5:
>>>> - fix some whitespcae issue
>>>> Changes in v6:
>>>> - None
>>>>
>>>> drivers/gpu/drm/rockchip/Makefile | 3 +-
>>>> drivers/gpu/drm/rockchip/cdn-dp-core.c | 24 +-
>>>> drivers/gpu/drm/rockchip/cdn-dp-core.h | 2 +
>>>> drivers/gpu/drm/rockchip/cdn-dp-link-training.c | 420
>>>> ++++++++++++++++++++++++
>>>> drivers/gpu/drm/rockchip/cdn-dp-reg.c | 31 +-
>>>> drivers/gpu/drm/rockchip/cdn-dp-reg.h | 38 ++-
>>>> 6 files changed, 505 insertions(+), 13 deletions(-)
>>>> create mode 100644 drivers/gpu/drm/rockchip/cdn-dp-link-training.c
>>>>
>>>> diff --git a/drivers/gpu/drm/rockchip/Makefile
>>>> b/drivers/gpu/drm/rockchip/Makefile
>>>> index a314e21..b932f62 100644
>>>> --- a/drivers/gpu/drm/rockchip/Makefile
>>>> +++ b/drivers/gpu/drm/rockchip/Makefile
>>>> @@ -9,7 +9,8 @@ rockchipdrm-y := rockchip_drm_drv.o rockchip_drm_fb.o \
>>>> rockchipdrm-$(CONFIG_DRM_FBDEV_EMULATION) += rockchip_drm_fbdev.o
>>>>
>>>> rockchipdrm-$(CONFIG_ROCKCHIP_ANALOGIX_DP) += analogix_dp-rockchip.o
>>>> -rockchipdrm-$(CONFIG_ROCKCHIP_CDN_DP) += cdn-dp-core.o cdn-dp-reg.o
>>>> +rockchipdrm-$(CONFIG_ROCKCHIP_CDN_DP) += cdn-dp-core.o cdn-dp-reg.o \
>>>> + cdn-dp-link-training.o
>>>> rockchipdrm-$(CONFIG_ROCKCHIP_DW_HDMI) += dw_hdmi-rockchip.o
>>>> rockchipdrm-$(CONFIG_ROCKCHIP_DW_MIPI_DSI) += dw-mipi-dsi.o
>>>> rockchipdrm-$(CONFIG_ROCKCHIP_INNO_HDMI) += inno_hdmi.o
>>>> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c
>>>> b/drivers/gpu/drm/rockchip/cdn-dp-core.c
>>>> index cce64c1..d9d0d4d 100644
>>>> --- a/drivers/gpu/drm/rockchip/cdn-dp-core.c
>>>> +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c
>>>> @@ -629,11 +629,13 @@ static void cdn_dp_encoder_enable(struct
>>>> drm_encoder *encoder)
>>>> goto out;
>>>> }
>>>> }
>>>> -
>>>> - ret = cdn_dp_set_video_status(dp, CONTROL_VIDEO_IDLE);
>>>> - if (ret) {
>>>> - DRM_DEV_ERROR(dp->dev, "Failed to idle video %d\n", ret);
>>>> - goto out;
>>>> + if (dp->use_fw_training == true) {
>>>
>>> You don't need to compare to true. Simply do:
>>>
>>> if (dp->use_fw_training)
>>>
>>>> + ret = cdn_dp_set_video_status(dp, CONTROL_VIDEO_IDLE);
>>>> + if (ret) {
>>>> + DRM_DEV_ERROR(dp->dev,
>>>> + "Failed to idle video %d\n", ret);
>>>> + goto out;
>>>> + }
>>>> }
>>>>
>>>> ret = cdn_dp_config_video(dp);
>>>> @@ -642,11 +644,15 @@ static void cdn_dp_encoder_enable(struct
>>>> drm_encoder *encoder)
>>>> goto out;
>>>> }
>>>>
>>>> - ret = cdn_dp_set_video_status(dp, CONTROL_VIDEO_VALID);
>>>> - if (ret) {
>>>> - DRM_DEV_ERROR(dp->dev, "Failed to valid video %d\n",
>>>> ret);
>>>> - goto out;
>>>> + if (dp->use_fw_training == true) {
>>>
>>> if (dp->use_fw_training)
>>>
>>>> + ret = cdn_dp_set_video_status(dp, CONTROL_VIDEO_VALID);
>>>> + if (ret) {
>>>> + DRM_DEV_ERROR(dp->dev,
>>>> + "Failed to valid video %d\n", ret);
>>>> + goto out;
>>>> + }
>>>> }
>>>> +
>>>> out:
>>>> mutex_unlock(&dp->lock);
>>>> }
>>>> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.h
>>>> b/drivers/gpu/drm/rockchip/cdn-dp-core.h
>>>> index 46159b2..77a9793 100644
>>>> --- a/drivers/gpu/drm/rockchip/cdn-dp-core.h
>>>> +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.h
>>>> @@ -84,6 +84,7 @@ struct cdn_dp_device {
>>>> bool connected;
>>>> bool active;
>>>> bool suspended;
>>>> + bool use_fw_training;
>>>>
>>>> const struct firmware *fw; /* cdn dp firmware */
>>>> unsigned int fw_version; /* cdn fw version */
>>>> @@ -106,6 +107,7 @@ struct cdn_dp_device {
>>>> u8 ports;
>>>> u8 lanes;
>>>> int active_port;
>>>> + u8 train_set[4];
>>>>
>>>> u8 dpcd[DP_RECEIVER_CAP_SIZE];
>>>> bool sink_has_audio;
>>>> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-link-training.c
>>>> b/drivers/gpu/drm/rockchip/cdn-dp-link-training.c
>>>> new file mode 100644
>>>> index 0000000..73c3290
>>>> --- /dev/null
>>>> +++ b/drivers/gpu/drm/rockchip/cdn-dp-link-training.c
>>>> @@ -0,0 +1,420 @@
>>>> +// SPDX-License-Identifier: GPL-2.0
>>>> +/*
>>>> + * Copyright (C) Fuzhou Rockchip Electronics Co.Ltd
>>>> + * Author: Chris Zhong <zyw@rock-chips.com>
>>>> + */
Oh, I just noticed that this is not the preferred format for .c files,
see the discussion here
https://lkml.org/lkml/2017/11/25/133
>>>> +
>>>> +#include <linux/device.h>
>>>> +#include <linux/delay.h>
>>>> +#include <linux/phy/phy.h>
>>>> +#include <soc/rockchip/rockchip_phy_typec.h>
>>>> +
>>>> +#include "cdn-dp-core.h"
>>>> +#include "cdn-dp-reg.h"
>>>> +
>>>> +static void cdn_dp_set_signal_levels(struct cdn_dp_device *dp)
>>>> +{
>>>> + struct cdn_dp_port *port = dp->port[dp->active_port];
>>>> + struct rockchip_typec_phy *tcphy = phy_get_drvdata(port->phy);
>>>> +
>>>> + int rate = drm_dp_bw_code_to_link_rate(dp->link.rate);
>>>> + u8 swing = (dp->train_set[0] & DP_TRAIN_VOLTAGE_SWING_MASK) >>
>>>> + DP_TRAIN_VOLTAGE_SWING_SHIFT;
>>>> + u8 pre_emphasis = (dp->train_set[0] & DP_TRAIN_PRE_EMPHASIS_MASK)
>>>> + >> DP_TRAIN_PRE_EMPHASIS_SHIFT;
>>>> +
>>>> + tcphy->typec_phy_config(port->phy, rate, dp->link.num_lanes,
>>>> + swing, pre_emphasis);
>>>> +}
>>>> +
>>>> +static int cdn_dp_set_pattern(struct cdn_dp_device *dp, uint8_t
>>>> dp_train_pat)
>>>> +{
>>>> + u32 phy_config, global_config;
>>>> + int ret;
>>>> + uint8_t pattern = dp_train_pat & DP_TRAINING_PATTERN_MASK;
>>>> +
>>>> + global_config = NUM_LANES(dp->link.num_lanes - 1) | SST_MODE |
>>>> + GLOBAL_EN | RG_EN | ENC_RST_DIS | WR_VHSYNC_FALL;
>>>> +
>>>> + phy_config = DP_TX_PHY_ENCODER_BYPASS(0) |
>>>> + DP_TX_PHY_SKEW_BYPASS(0) |
>>>> + DP_TX_PHY_DISPARITY_RST(0) |
>>>> + DP_TX_PHY_LANE0_SKEW(0) |
>>>> + DP_TX_PHY_LANE1_SKEW(1) |
>>>> + DP_TX_PHY_LANE2_SKEW(2) |
>>>> + DP_TX_PHY_LANE3_SKEW(3) |
>>>> + DP_TX_PHY_10BIT_ENABLE(0);
>>>> +
>>>> + if (pattern != DP_TRAINING_PATTERN_DISABLE) {
>>>> + global_config |= NO_VIDEO;
>>>> + phy_config |= DP_TX_PHY_TRAINING_ENABLE(1) |
>>>> + DP_TX_PHY_SCRAMBLER_BYPASS(1) |
>>>> + DP_TX_PHY_TRAINING_PATTERN(pattern);
>>>> + }
>>>> +
>>>> + ret = cdn_dp_reg_write(dp, DP_FRAMER_GLOBAL_CONFIG,
>>>> global_config);
>>>> + if (ret) {
>>>> + DRM_ERROR("fail to set DP_FRAMER_GLOBAL_CONFIG, error:
>>>> %d\n",
>>>> + ret);
>>>> + return ret;
>>>> + }
>>>> +
>>>> + ret = cdn_dp_reg_write(dp, DP_TX_PHY_CONFIG_REG, phy_config);
>>>> + if (ret) {
>>>> + DRM_ERROR("fail to set DP_TX_PHY_CONFIG_REG, error:
>>>> %d\n",
>>>> + ret);
>>>> + return ret;
>>>> + }
>>>> +
>>>> + ret = cdn_dp_reg_write(dp, DPTX_LANE_EN, BIT(dp->link.num_lanes)
>>>> - 1);
>>>> + if (ret) {
>>>> + DRM_ERROR("fail to set DPTX_LANE_EN, error: %d\n", ret);
>>>> + return ret;
>>>> + }
>>>> +
>>>> + if (drm_dp_enhanced_frame_cap(dp->dpcd))
>>>> + ret = cdn_dp_reg_write(dp, DPTX_ENHNCD, 1);
>>>> + else
>>>> + ret = cdn_dp_reg_write(dp, DPTX_ENHNCD, 0);
>>>> + if (ret)
>>>> + DRM_ERROR("failed to set DPTX_ENHNCD, error: %x\n", ret);
>>>> +
>>>> + return ret;
>>>> +}
>>>> +
>>>> +static u8 cdn_dp_pre_emphasis_max(u8 voltage_swing)
>>>> +{
>>>> + switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
>>>> + case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
>>>> + return DP_TRAIN_PRE_EMPH_LEVEL_3;
>>>> + case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
>>>> + return DP_TRAIN_PRE_EMPH_LEVEL_2;
>>>> + case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
>>>> + return DP_TRAIN_PRE_EMPH_LEVEL_1;
>>>> + default:
>>>> + return DP_TRAIN_PRE_EMPH_LEVEL_0;
>>>> + }
>>>> +}
>>>> +
>>>> +static void cdn_dp_get_adjust_train(struct cdn_dp_device *dp,
>>>> + uint8_t
>>>> link_status[DP_LINK_STATUS_SIZE])
>>>> +{
>>>> + int i;
>>>> + uint8_t v = 0, p = 0;
>>>> + uint8_t preemph_max;
>>>> +
>>>> + for (i = 0; i < dp->link.num_lanes; i++) {
>>>> + v = max(v, drm_dp_get_adjust_request_voltage(link_status,
>>>> i));
>>>> + p = max(p,
>>>> drm_dp_get_adjust_request_pre_emphasis(link_status,
>>>> + i));
>>>> + }
>>>> +
>>>> + if (v >= VOLTAGE_LEVEL_2)
>>>> + v = VOLTAGE_LEVEL_2 | DP_TRAIN_MAX_SWING_REACHED;
>>>> +
>>>> + preemph_max = cdn_dp_pre_emphasis_max(v);
>>>> + if (p >= preemph_max)
>>>> + p = preemph_max | DP_TRAIN_MAX_PRE_EMPHASIS_REACHED;
>>>> +
>>>> + for (i = 0; i < dp->link.num_lanes; i++)
>>>> + dp->train_set[i] = v | p;
>>>> +}
>>>> +
>>>> +/*
>>>> + * Pick training pattern for channel equalization. Training Pattern 3
>>>> for HBR2
>>>> + * or 1.2 devices that support it, Training Pattern 2 otherwise.
>>>> + */
>>>> +static u32 cdn_dp_select_chaneq_pattern(struct cdn_dp_device *dp)
>>>> +{
>>>> + u32 training_pattern = DP_TRAINING_PATTERN_2;
>>>> +
>>>> + /*
>>>> + * cdn dp support HBR2 also support TPS3. TPS3 support is also
>>>> mandatory
>>>> + * for downstream devices that support HBR2. However, not all
>>>> sinks
>>>> + * follow the spec.
>>>> + */
>>>> + if (drm_dp_tps3_supported(dp->dpcd))
>>>> + training_pattern = DP_TRAINING_PATTERN_3;
>>>> + else
>>>> + DRM_DEBUG_KMS("5.4 Gbps link rate without sink TPS3
>>>> support\n");
>>>> +
>>>> + return training_pattern;
>>>> +}
>>>> +
>>>> +
>>>> +static bool cdn_dp_link_max_vswing_reached(struct cdn_dp_device *dp)
>>>> +{
>>>> + int lane;
>>>> +
>>>> + for (lane = 0; lane < dp->link.num_lanes; lane++)
>>>> + if ((dp->train_set[lane] & DP_TRAIN_MAX_SWING_REACHED) ==
>>>> 0)
>>>> + return false;
>>>> +
>>>> + return true;
>>>> +}
>>>> +
>>>> +static int cdn_dp_update_link_train(struct cdn_dp_device *dp)
>>>> +{
>>>> + int ret;
>>>> +
>>>> + cdn_dp_set_signal_levels(dp);
>>>> +
>>>> + ret = drm_dp_dpcd_write(&dp->aux, DP_TRAINING_LANE0_SET,
>>>> + dp->train_set, dp->link.num_lanes);
>>>> + if (ret != dp->link.num_lanes)
>>>> + return -EINVAL;
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static int cdn_dp_set_link_train(struct cdn_dp_device *dp,
>>>> + uint8_t dp_train_pat)
>>>> +{
>>>> + uint8_t buf[sizeof(dp->train_set) + 1];
>>>> + int ret, len;
>>>> +
>>>> + buf[0] = dp_train_pat;
>>>> + if ((dp_train_pat & DP_TRAINING_PATTERN_MASK) ==
>>>> + DP_TRAINING_PATTERN_DISABLE) {
>>>> + /* don't write DP_TRAINING_LANEx_SET on disable */
>>>> + len = 1;
>>>> + } else {
>>>> + /* DP_TRAINING_LANEx_SET follow DP_TRAINING_PATTERN_SET
>>>> */
>>>> + memcpy(buf + 1, dp->train_set, dp->link.num_lanes);
>>>> + len = dp->link.num_lanes + 1;
>>>> + }
>>>> +
>>>> + ret = drm_dp_dpcd_write(&dp->aux, DP_TRAINING_PATTERN_SET,
>>>> + buf, len);
>>>> + if (ret != len)
>>>> + return -EINVAL;
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static int cdn_dp_reset_link_train(struct cdn_dp_device *dp,
>>>> + uint8_t dp_train_pat)
>>>> +{
>>>> + int ret;
>>>> +
>>>> + memset(dp->train_set, 0, sizeof(dp->train_set));
>>>> +
>>>> + cdn_dp_set_signal_levels(dp);
>>>> +
>>>> + ret = cdn_dp_set_pattern(dp, dp_train_pat);
>>>> + if (ret)
>>>> + return ret;
>>>> +
>>>> + return cdn_dp_set_link_train(dp, dp_train_pat);
>>>> +}
>>>> +
>>>> +/* Enable corresponding port and start training pattern 1 */
>>>> +static int cdn_dp_link_training_clock_recovery(struct cdn_dp_device *dp)
>>>> +{
>>>> + u8 voltage;
>>>> + u8 link_status[DP_LINK_STATUS_SIZE];
>>>> + u32 voltage_tries, max_vswing_tries;
>>>> + int ret;
>>>> +
>>>> + /* clock recovery */
>>>> + ret = cdn_dp_reset_link_train(dp, DP_TRAINING_PATTERN_1 |
>>>> + DP_LINK_SCRAMBLING_DISABLE);
>>>> + if (ret) {
>>>> + DRM_ERROR("failed to start link train\n");
>>>> + return ret;
>>>> + }
>>>> +
>>>> + voltage_tries = 1;
>>>> + max_vswing_tries = 0;
>>>> + for (;;) {
>>>> + drm_dp_link_train_clock_recovery_delay(dp->dpcd);
>>>> + if (drm_dp_dpcd_read_link_status(&dp->aux, link_status)
>>>> !=
>>>> + DP_LINK_STATUS_SIZE) {
>>>> + DRM_ERROR("failed to get link status\n");
>>>> + return -EINVAL;
>>>> + }
>>>> +
>>>> + if (drm_dp_clock_recovery_ok(link_status,
>>>> dp->link.num_lanes)) {
>>>> + DRM_DEBUG_KMS("clock recovery OK\n");
>>>> + return 0;
>>>> + }
>>>> +
>>>> + if (voltage_tries >= 5) {
>>>> + DRM_DEBUG_KMS("Same voltage tried 5 times\n");
>>>> + return -EINVAL;
>>>> + }
>>>> +
>>>> + if (max_vswing_tries >= 1) {
>>>> + DRM_DEBUG_KMS("Max Voltage Swing reached\n");
>>>> + return -EINVAL;
>>>> + }
>>>> +
>>>> + voltage = dp->train_set[0] & DP_TRAIN_VOLTAGE_SWING_MASK;
>>>> +
>>>> + /* Update training set as requested by target */
>>>> + cdn_dp_get_adjust_train(dp, link_status);
>>>> + if (cdn_dp_update_link_train(dp)) {
>>>> + DRM_ERROR("failed to update link training\n");
>>>> + return -EINVAL;
>>>> + }
>>>> +
>>>> + if ((dp->train_set[0] & DP_TRAIN_VOLTAGE_SWING_MASK) ==
>>>> + voltage)
>>>> + ++voltage_tries;
>>>> + else
>>>> + voltage_tries = 1;
>>>> +
>>>> + if (cdn_dp_link_max_vswing_reached(dp))
>>>> + ++max_vswing_tries;
>>>> + }
>>>> +}
>>>> +
>>>> +static int cdn_dp_link_training_channel_equalization(struct
>>>> cdn_dp_device *dp)
>>>> +{
>>>> + int tries, ret;
>>>> + u32 training_pattern;
>>>> + uint8_t link_status[DP_LINK_STATUS_SIZE];
>>>> +
>>>> + training_pattern = cdn_dp_select_chaneq_pattern(dp);
>>>> + training_pattern |= DP_LINK_SCRAMBLING_DISABLE;
>>>> +
>>>> + ret = cdn_dp_set_pattern(dp, training_pattern);
>>>> + if (ret)
>>>> + return ret;
>>>> +
>>>> + ret = cdn_dp_set_link_train(dp, training_pattern);
>>>> + if (ret) {
>>>> + DRM_ERROR("failed to start channel equalization\n");
>>>> + return ret;
>>>> + }
>>>> +
>>>> + for (tries = 0; tries < 5; tries++) {
>>>> + drm_dp_link_train_channel_eq_delay(dp->dpcd);
>>>> + if (drm_dp_dpcd_read_link_status(&dp->aux, link_status)
>>>> !=
>>>> + DP_LINK_STATUS_SIZE) {
>>>> + DRM_ERROR("failed to get link status\n");
>>>> + break;
>>>> + }
>>>> +
>>>> + /* Make sure clock is still ok */
>>>> + if (!drm_dp_clock_recovery_ok(link_status,
>>>> + dp->link.num_lanes)) {
>>>> + DRM_DEBUG_KMS("Clock recovery check failed\n");
>>>> + break;
>>>> + }
>>>> +
>>>> + if (drm_dp_channel_eq_ok(link_status,
>>>> dp->link.num_lanes)) {
>>>> + DRM_DEBUG_KMS("Channel EQ done\n");
>>>> + return 0;
>>>> + }
>>>> +
>>>> + /* Update training set as requested by target */
>>>> + cdn_dp_get_adjust_train(dp, link_status);
>>>> + if (cdn_dp_update_link_train(dp)) {
>>>> + DRM_ERROR("failed to update link training\n");
>>>> + break;
>>>> + }
>>>> + }
>>>> +
>>>> + /* Try 5 times, else fail and try at lower BW */
>>>> + if (tries == 5)
>>>> + DRM_DEBUG_KMS("Channel equalization failed 5 times\n");
>>>> +
>>>> + return -EINVAL;
>>>> +}
>>>> +
>>>> +static int cdn_dp_stop_link_train(struct cdn_dp_device *dp)
>>>> +{
>>>> + int ret = cdn_dp_set_pattern(dp, DP_TRAINING_PATTERN_DISABLE);
>>>> +
>>>> + if (ret)
>>>> + return ret;
>>>> +
>>>> + return cdn_dp_set_link_train(dp, DP_TRAINING_PATTERN_DISABLE);
>>>> +}
>>>> +
>>>> +static int cdn_dp_get_lower_link_rate(struct cdn_dp_device *dp)
>>>> +{
>>>> + switch (dp->link.rate) {
>>>> + case DP_LINK_BW_1_62:
>>>> + return -EINVAL;
>>>> + case DP_LINK_BW_2_7:
>>>> + dp->link.rate = DP_LINK_BW_1_62;
>>>> + break;
>>>> + case DP_LINK_BW_5_4:
>>>> + dp->link.rate = DP_LINK_BW_2_7;
>>>> + break;
>>>> + default:
>>>> + dp->link.rate = DP_LINK_BW_5_4;
>>>> + break;
>>>> + }
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +int cdn_dp_software_train_link(struct cdn_dp_device *dp)
>>>> +{
>>>> + int ret, stop_err;
>>>> + u8 link_config[2];
>>>> + u32 rate, sink_max, source_max;
>>>> +
>>>> + ret = drm_dp_dpcd_read(&dp->aux, DP_DPCD_REV, dp->dpcd,
>>>> + sizeof(dp->dpcd));
>>>> + if (ret < 0) {
>>>> + DRM_DEV_ERROR(dp->dev, "Failed to get caps %d\n", ret);
>>>> + return ret;
>>>> + }
>>>> +
>>>> + source_max = dp->lanes;
>>>> + sink_max = drm_dp_max_lane_count(dp->dpcd);
>>>> + dp->link.num_lanes = min(source_max, sink_max);
>>>> +
>>>> + source_max = drm_dp_bw_code_to_link_rate(CDN_DP_MAX_LINK_RATE);
>>>> + sink_max = drm_dp_max_link_rate(dp->dpcd);
>>>> + rate = min(source_max, sink_max);
>>>> + dp->link.rate = drm_dp_link_rate_to_bw_code(rate);
>>>> +
>>>> + link_config[0] = 0;
>>>> + link_config[1] = 0;
>>>> + if (dp->dpcd[DP_MAIN_LINK_CHANNEL_CODING] & 0x01)
>>>> + link_config[1] = DP_SET_ANSI_8B10B;
>>>> + drm_dp_dpcd_write(&dp->aux, DP_DOWNSPREAD_CTRL, link_config, 2);
>>>> +
>>>> + while (true) {
>>>> +
>>>> + /* Write the link configuration data */
>>>> + link_config[0] = dp->link.rate;
>>>> + link_config[1] = dp->link.num_lanes;
>>>> + if (drm_dp_enhanced_frame_cap(dp->dpcd))
>>>> + link_config[1] |=
>>>> DP_LANE_COUNT_ENHANCED_FRAME_EN;
>>>> + drm_dp_dpcd_write(&dp->aux, DP_LINK_BW_SET, link_config,
>>>> 2);
>>>> +
>>>> + ret = cdn_dp_link_training_clock_recovery(dp);
>>>> + if (ret) {
>>>> + if (!cdn_dp_get_lower_link_rate(dp))
>>>> + continue;
>>>> +
>>>> + DRM_ERROR("training clock recovery failed: %d\n",
>>>> ret);
>>>> + break;
>>>> + }
>>>> +
>>>> + ret = cdn_dp_link_training_channel_equalization(dp);
>>>> + if (ret) {
>>>> + if (!cdn_dp_get_lower_link_rate(dp))
>>>> + continue;
>>>> +
>>>> + DRM_ERROR("training channel eq failed: %d\n",
>>>> ret);
>>>> + break;
>>>> + }
>>>> +
>>>> + break;
>>>> + }
>>>> +
>>>> + stop_err = cdn_dp_stop_link_train(dp);
>>>> + if (stop_err) {
>>>> + DRM_ERROR("stop training fail, error: %d\n", stop_err);
>>>> + return stop_err;
>>>> + }
>>>> +
>>>> + return ret;
>>>> +}
>>>> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-reg.c
>>>> b/drivers/gpu/drm/rockchip/cdn-dp-reg.c
>>>> index 979355d..e1273e6 100644
>>>> --- a/drivers/gpu/drm/rockchip/cdn-dp-reg.c
>>>> +++ b/drivers/gpu/drm/rockchip/cdn-dp-reg.c
>>>> @@ -17,7 +17,9 @@
>>>> #include <linux/delay.h>
>>>> #include <linux/io.h>
>>>> #include <linux/iopoll.h>
>>>> +#include <linux/phy/phy.h>
>>>> #include <linux/reset.h>
>>>> +#include <soc/rockchip/rockchip_phy_typec.h>
>>>>
>>>> #include "cdn-dp-core.h"
>>>> #include "cdn-dp-reg.h"
>>>> @@ -189,7 +191,7 @@ static int cdn_dp_mailbox_send(struct cdn_dp_device
>>>> *dp, u8 module_id,
>>>> return 0;
>>>> }
>>>>
>>>> -static int cdn_dp_reg_write(struct cdn_dp_device *dp, u16 addr, u32 val)
>>>> +int cdn_dp_reg_write(struct cdn_dp_device *dp, u16 addr, u32 val)
>>>> {
>>>> u8 msg[6];
>>>>
>>>> @@ -609,6 +611,31 @@ int cdn_dp_train_link(struct cdn_dp_device *dp)
>>>> {
>>>> int ret;
>>>>
>>>> + /*
>>>> + * DP firmware uses fixed phy config values to do training, but
>>>> some
>>>> + * boards need to adjust these values to fit for their unique
>>>> hardware
>>>> + * design. So if the phy is using custom config values, do
>>>> software
>>>> + * link training instead of relying on firmware, if software
>>>> training
>>>> + * fail, keep firmware training as a fallback if sw training
>>>> fails.
>>>> + */
>>>> + ret = cdn_dp_software_train_link(dp);
>>>> + if (ret) {
>>>> + DRM_DEV_ERROR(dp->dev,
>>>> + "Failed to do software training %d\n", ret);
>>>> + goto do_fw_training;
>>>> + }
>>>
>>> If I understand correctly you are changing current behavior. Before
>>> this patch, we use always firmware link training, and after this
>>> patch, we always use software link training. If fails we use the
>>> firmware link training.
>>>
>>> AFAIK my Samsung Chromebook Plus works well with firmware link
>>> training, so there are any benefits of use software link training
>>> instead of firmware link training?
>>>
>>> Looks to me that we should only do software link training on these
>>> platforms that need it, so on those that define the phy-config
>>> property in their DT and use by default firmware link training.
>>
>> Sean and me have discussed about that, and we all agree to use sw training
>> instead the
>> fw training to keep training process consistent, and we do not need add a
>> varialbe in
>> struct rockchip_typec_phy(like use_sw_training before) to distinguish sw and
>> fw training.
>> If training process implement correctly, sw training not different with fw
>> training, and as my
>> test so far, sw training work well on Kevin, ofcourse, we need keep testing
>> it.
>>
>
> Ok, I can also confirm that software training works well on kevin.
> Could you mention this change of behavior in the commit message? I
> think that after these patches the firmware training is unlikely to
> happen.
>
> Thanks,
> Enric
>
>
>>>> + ret = cdn_dp_reg_write(dp, SOURCE_HDTX_CAR, 0xf);
>>>> + if (ret) {
>>>> + DRM_DEV_ERROR(dp->dev,
>>>> + "Failed to write SOURCE_HDTX_CAR register %d\n", ret);
>>>> + goto do_fw_training;
>>>> + }
>>>> + dp->use_fw_training = false;
>>>> + return 0;
>>>> +
>>>> +do_fw_training:
>>>> + dp->use_fw_training = true;
>>>> + DRM_DEV_DEBUG_KMS(dp->dev, "use fw training\n");
>>>> ret = cdn_dp_training_start(dp);
>>>> if (ret) {
>>>> DRM_DEV_ERROR(dp->dev, "Failed to start training %d\n",
>>>> ret);
>>>> @@ -623,7 +650,7 @@ int cdn_dp_train_link(struct cdn_dp_device *dp)
>>>>
>>>> DRM_DEV_DEBUG_KMS(dp->dev, "rate:0x%x, lanes:%d\n",
>>>> dp->link.rate,
>>>> dp->link.num_lanes);
>>>> - return ret;
>>>> + return 0;
>>>> }
>>>>
>>>> int cdn_dp_set_video_status(struct cdn_dp_device *dp, int active)
>>>> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-reg.h
>>>> b/drivers/gpu/drm/rockchip/cdn-dp-reg.h
>>>> index 6580b11..3420771 100644
>>>> --- a/drivers/gpu/drm/rockchip/cdn-dp-reg.h
>>>> +++ b/drivers/gpu/drm/rockchip/cdn-dp-reg.h
>>>> @@ -137,7 +137,7 @@
>>>> #define HPD_EVENT_MASK 0x211c
>>>> #define HPD_EVENT_DET 0x2120
>>>>
>>>> -/* dpyx framer addr */
>>>> +/* dptx framer addr */
>>>> #define DP_FRAMER_GLOBAL_CONFIG 0x2200
>>>> #define DP_SW_RESET 0x2204
>>>> #define DP_FRAMER_TU 0x2208
>>>> @@ -431,6 +431,40 @@
>>>> /* Reference cycles when using lane clock as reference */
>>>> #define LANE_REF_CYC 0x8000
>>>>
>>>> +/* register CM_VID_CTRL */
>>>> +#define LANE_VID_REF_CYC(x) (((x) & (BIT(24) - 1)) <<
>>>> 0)
>>>> +#define NMVID_MEAS_TOLERANCE(x) (((x) & 0xf) <<
>>>> 24)
>>>> +
>>>> +/* register DP_TX_PHY_CONFIG_REG */
>>>> +#define DP_TX_PHY_TRAINING_ENABLE(x) ((x) & 1)
>>>> +#define DP_TX_PHY_TRAINING_TYPE_PRBS7 (0 << 1)
>>>> +#define DP_TX_PHY_TRAINING_TYPE_TPS1 (1 << 1)
>>>> +#define DP_TX_PHY_TRAINING_TYPE_TPS2 (2 << 1)
>>>> +#define DP_TX_PHY_TRAINING_TYPE_TPS3 (3 << 1)
>>>> +#define DP_TX_PHY_TRAINING_TYPE_TPS4 (4 << 1)
>>>> +#define DP_TX_PHY_TRAINING_TYPE_PLTPAT (5 << 1)
>>>> +#define DP_TX_PHY_TRAINING_TYPE_D10_2 (6 << 1)
>>>> +#define DP_TX_PHY_TRAINING_TYPE_HBR2CPAT (8 << 1)
>>>> +#define DP_TX_PHY_TRAINING_PATTERN(x) ((x) << 1)
>>>> +#define DP_TX_PHY_SCRAMBLER_BYPASS(x) (((x) & 1) << 5)
>>>> +#define DP_TX_PHY_ENCODER_BYPASS(x) (((x) & 1) << 6)
>>>> +#define DP_TX_PHY_SKEW_BYPASS(x) (((x) & 1) << 7)
>>>> +#define DP_TX_PHY_DISPARITY_RST(x) (((x) & 1) << 8)
>>>> +#define DP_TX_PHY_LANE0_SKEW(x) (((x) & 7) << 9)
>>>> +#define DP_TX_PHY_LANE1_SKEW(x) (((x) & 7) << 12)
>>>> +#define DP_TX_PHY_LANE2_SKEW(x) (((x) & 7) << 15)
>>>> +#define DP_TX_PHY_LANE3_SKEW(x) (((x) & 7) << 18)
>>>> +#define DP_TX_PHY_10BIT_ENABLE(x) (((x) & 1) << 21)
>>>> +
>>>> +/* register DP_FRAMER_GLOBAL_CONFIG */
>>>> +#define NUM_LANES(x) ((x) & 3)
>>>> +#define SST_MODE (0 << 2)
>>>> +#define RG_EN (0 << 4)
>>>> +#define GLOBAL_EN BIT(3)
>>>> +#define NO_VIDEO BIT(5)
>>>> +#define ENC_RST_DIS BIT(6)
>>>> +#define WR_VHSYNC_FALL BIT(7)
>>>> +
>>>> enum voltage_swing_level {
>>>> VOLTAGE_LEVEL_0,
>>>> VOLTAGE_LEVEL_1,
>>>> @@ -476,6 +510,7 @@ int cdn_dp_set_host_cap(struct cdn_dp_device *dp, u8
>>>> lanes, bool flip);
>>>> int cdn_dp_event_config(struct cdn_dp_device *dp);
>>>> u32 cdn_dp_get_event(struct cdn_dp_device *dp);
>>>> int cdn_dp_get_hpd_status(struct cdn_dp_device *dp);
>>>> +int cdn_dp_reg_write(struct cdn_dp_device *dp, u16 addr, u32 val);
>>>> ssize_t cdn_dp_dpcd_write(struct cdn_dp_device *dp, u32 addr,
>>>> u8 *data, u16 len);
>>>> ssize_t cdn_dp_dpcd_read(struct cdn_dp_device *dp, u32 addr,
>>>> @@ -489,4 +524,5 @@ int cdn_dp_config_video(struct cdn_dp_device *dp);
>>>> int cdn_dp_audio_stop(struct cdn_dp_device *dp, struct audio_info
>>>> *audio);
>>>> int cdn_dp_audio_mute(struct cdn_dp_device *dp, bool enable);
>>>> int cdn_dp_audio_config(struct cdn_dp_device *dp, struct audio_info
>>>> *audio);
>>>> +int cdn_dp_software_train_link(struct cdn_dp_device *dp);
>>>> #endif /* _CDN_DP_REG_H */
>>>> --
>>>> 2.7.4
>>>>
>>>
>>>
>>
>>
^ permalink raw reply
* [PATCH v6 3/9] docs: Add Generic Counter interface documentation
From: Jonathan Cameron @ 2018-05-22 17:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180521134729.GB5723@sophia>
...
> >> +
> >> +COUNT
> >> +-----
> >> +A Count represents the count data for a set of Signals. The Generic
> >> +Counter interface provides the following available count data types:
> >> +
> >> +* COUNT_POSITION_UNSIGNED:
> >> + Unsigned integer value representing position.
> >> +
> >> +* COUNT_POSITION_SIGNED:
> >> + Signed integer value representing position.
> >
> >Just a thought: As the '0' position is effectively arbitrary is there any
> >actual difference between signed and unsigned? If we defined all counters
> >to be unsigned and just offset any signed ones so the range still fitted
> >would we end up with a simpler interface - there would be no real loss
> >of meaning that I can see.. I suppose it might not be what people expect
> >though when they see their counters start at a large positive value...
>
> This is something I've been on the fence about for a while. I would
> actually prefer the interface to have simply a COUNT_POSITION data type
> to represent position and leave it as unsigned; distinguishing between
> signed and unsigned position seems ultimately arbitrary given that it's
> mathematically just an offset as you have pointed out.
>
> If we were to go down this route, then we'd have a count value that may
> always be represented using an unsigned data type, with an offset value
> that may always be represented using a signed data type -- the
> relationship being such: position = count + offset. Is that correct?
Given the positions are all arbitrary (such as limits etc) there is no
real need to have 0 as in anyway special. So you could just apply an
offset to everything then don't make them visible to userspace at all.
>
> My reason for giving the option for either signed or unsigned position
> was to help minimize the work userspace would have to do in order to get
> the value in which they're actually interested. I suppose it was a
> question of how abstract I want to make the interface -- although,
> making it simpler for userspace put more of a burden on the kernel side.
>
> However, the "offset" value route may actually be more robust in the end
> because userspace would have to know whether they want a signed or
> unsigned position regardless in order to parse, so with count and offet
> defined we know they will always be unsigned and signed respectively.
Hmm. It's not obvious to me which is the best option.
>
> >
> >
> >
> >
> >> +
> >> +A Count has a count function mode which represents the update behavior
> >> +for the count data. The Generic Counter interface provides the following
> >> +available count function modes:
> >> +
> >> +* Increase:
> >> + Accumulated count is incremented.
> >> +
> >> +* Decrease:
> >> + Accumulated count is decremented.
> >> +
> >> +* Pulse-Direction:
> >> + Rising edges on quadrature pair signal A updates the respective count.
> >> + The input level of quadrature pair signal B determines direction.
> >> +
> >Perhaps group the quadrature modes for the point of view of this document?
> >Might be slightly easier to read?
> >
> >* Quadrature Modes
> >
> > - x1 A: etc.
> >
> >> +* Quadrature x1 A:
> >> + If direction is forward, rising edges on quadrature pair signal A
> >> + updates the respective count; if the direction is backward, falling
> >> + edges on quadrature pair signal A updates the respective count.
> >> + Quadrature encoding determines the direction.
> >> +
> >> +* Quadrature x1 B:
> >> + If direction is forward, rising edges on quadrature pair signal B
> >> + updates the respective count; if the direction is backward, falling
> >> + edges on quadrature pair signal B updates the respective count.
> >> + Quadrature encoding determines the direction.
> >> +
> >> +* Quadrature x2 A:
> >> + Any state transition on quadrature pair signal A updates the
> >> + respective count. Quadrature encoding determines the direction.
> >> +
> >> +* Quadrature x2 B:
> >> + Any state transition on quadrature pair signal B updates the
> >> + respective count. Quadrature encoding determines the direction.
> >> +
> >> +* Quadrature x2 Rising:
> >> + Rising edges on either quadrature pair signals updates the respective
> >> + count. Quadrature encoding determines the direction.
> >
> >This one I've never met. Really? There are devices who do this form
> >of crazy? It gives really uneven counting and I'm failing to see when
> >it would ever make sense... References for these odd corner cases
> >would be good.
> >
> >
> >__|---|____|-----|____
> >____|----|____|-----|____
> >
> >001122222223334444444
>
> That's the same reaction I had when I discovered this -- in fact the
> STM32 LP Timer is the first time I've come across such a quadrature
> mode. I'm not sure of the use case for this mode, because positioning
> wouldn't be precise as you've pointed out. Perhaps Fabrice or Benjamin
> can probe the ST guys responsible for this design choice to figure out
> the rationale.
Hmm. My inclination would be to not support it unless someone can up
with a meaningful use. We are adding ABI (be it not much) for a case
that to us makes no sense.
Looks rather like the sort of thing that is a side effect of the
implementation rather than deliberate.
>
> I'm leaving in these modes for now, as they do exist in the STM32 LP
> Timer, but it does make me curious what the intentions for them were
> (perhaps use cases outside of traditional quadrature encoder
> positioning).
>
Sure if there is a usecase then fair enough.
Jonathan
^ permalink raw reply
* [PATCH RT] arm64: fpsimd: use a local_lock() in addition to local_bh_disable()
From: Steven Rostedt @ 2018-05-22 17:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180517124006.ohygrrpg7z2moqqt@linutronix.de>
On Thu, 17 May 2018 14:40:06 +0200
Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:
> +static DEFINE_LOCAL_IRQ_LOCK(fpsimd_lock);
> /*
> * Update current's FPSIMD/SVE registers from thread_struct.
> *
> @@ -594,6 +595,7 @@ int sve_set_vector_length(struct task_struct *task,
> * non-SVE thread.
> */
> if (task == current) {
> + local_lock(fpsimd_lock);
> local_bh_disable();
I'm surprised that we don't have a "local_lock_bh()"?
-- Steve
>
> task_fpsimd_save();
^ permalink raw reply
* [PATCH v3 6/6] drm/rockchip: dw_hdmi: add dw-hdmi support for the rk3328
From: Rob Herring @ 2018-05-22 17:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180515134736.5824-7-heiko@sntech.de>
On Tue, May 15, 2018 at 03:47:36PM +0200, Heiko Stuebner wrote:
> The rk3328 uses a dw-hdmi controller with an external hdmi phy from
> Innosilicon which uses the generic phy framework for access.
> Add the necessary data and the compatible for the rk3328 to the
> rockchip dw-hdmi driver.
>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> Tested-by: Robin Murphy <robin.murphy@arm.com>
> ---
> changes in v3:
> - reword as suggested by Rob to show that it's a dw-hdmi + Inno phy
>
> .../display/rockchip/dw_hdmi-rockchip.txt | 1 +
Acked-by: Rob Herring <robh@kernel.org>
> drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 106 ++++++++++++++++++
> 2 files changed, 107 insertions(+)
^ permalink raw reply
* [PATCH v3 1/4] dt-bindings: arm: Add bindings for Mediatek MT8183 SoC Platform
From: Rob Herring @ 2018-05-22 17:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526538126-51497-2-git-send-email-erin.lo@mediatek.com>
On Thu, May 17, 2018 at 02:22:03PM +0800, Erin Lo wrote:
> This adds dt-binding documentation of cpu for Mediatek MT8183.
>
> Signed-off-by: Erin Lo <erin.lo@mediatek.com>
> ---
> Documentation/devicetree/bindings/arm/mediatek.txt | 4 ++++
> 1 file changed, 4 insertions(+)
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* [PATCH v3 2/4] dt-bindings: mtk-sysirq: Add compatible for Mediatek MT8183
From: Rob Herring @ 2018-05-22 17:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526538126-51497-3-git-send-email-erin.lo@mediatek.com>
On Thu, May 17, 2018 at 02:22:04PM +0800, Erin Lo wrote:
> This adds dt-binding documentation of SYSIRQ for Mediatek MT8183 SoC
> Platform.
>
> Signed-off-by: Erin Lo <erin.lo@mediatek.com>
> ---
> .../devicetree/bindings/interrupt-controller/mediatek,sysirq.txt | 1 +
> 1 file changed, 1 insertion(+)
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* [PATCH 08/14] ARM: spectre-v2: harden user aborts in kernel space
From: Marc Zyngier @ 2018-05-22 17:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <E1fKjFO-000648-8d@rmk-PC.armlinux.org.uk>
On 21/05/18 12:45, Russell King wrote:
> In order to prevent aliasing attacks on the branch predictor,
> invalidate the BTB or instruction cache on CPUs that are known to be
> affected when taking an abort on a address that is outside of a user
> task limit:
>
> Cortex A8, A9, A12, A17, A73, A75: flush BTB.
> Cortex A15, Brahma B15: invalidate icache.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> arch/arm/include/asm/cp15.h | 3 +++
> arch/arm/include/asm/system_misc.h | 8 ++++++
> arch/arm/mm/fault.c | 3 +++
> arch/arm/mm/proc-v7-bugs.c | 51 ++++++++++++++++++++++++++++++++++++++
> arch/arm/mm/proc-v7.S | 8 +++---
> 5 files changed, 70 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/include/asm/cp15.h b/arch/arm/include/asm/cp15.h
> index 4c9fa72b59f5..07e27f212dc7 100644
> --- a/arch/arm/include/asm/cp15.h
> +++ b/arch/arm/include/asm/cp15.h
> @@ -65,6 +65,9 @@
> #define __write_sysreg(v, r, w, c, t) asm volatile(w " " c : : "r" ((t)(v)))
> #define write_sysreg(v, ...) __write_sysreg(v, __VA_ARGS__)
>
> +#define BPIALL __ACCESS_CP15(c7, 0, c5, 6)
> +#define ICIALLU __ACCESS_CP15(c7, 0, c5, 0)
> +
> extern unsigned long cr_alignment; /* defined in entry-armv.S */
>
> static inline unsigned long get_cr(void)
> diff --git a/arch/arm/include/asm/system_misc.h b/arch/arm/include/asm/system_misc.h
> index 78f6db114faf..3cfe010c5734 100644
> --- a/arch/arm/include/asm/system_misc.h
> +++ b/arch/arm/include/asm/system_misc.h
> @@ -15,6 +15,14 @@ void soft_restart(unsigned long);
> extern void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd);
> extern void (*arm_pm_idle)(void);
>
> +#ifdef CONFIG_HARDEN_BRANCH_PREDICTOR
> +extern void (*harden_branch_predictor)(void);
> +#define harden_branch_predictor() \
> + do { if (harden_branch_predictor) harden_branch_predictor(); } while (0)
> +#else
> +#define harden_branch_predictor() do { } while (0)
> +#endif
> +
> #define UDBG_UNDEFINED (1 << 0)
> #define UDBG_SYSCALL (1 << 1)
> #define UDBG_BADABORT (1 << 2)
> diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
> index b75eada23d0a..3b1ba003c4f9 100644
> --- a/arch/arm/mm/fault.c
> +++ b/arch/arm/mm/fault.c
> @@ -163,6 +163,9 @@ __do_user_fault(struct task_struct *tsk, unsigned long addr,
> {
> struct siginfo si;
>
> + if (addr > TASK_SIZE)
> + harden_branch_predictor();
> +
> #ifdef CONFIG_DEBUG_USER
> if (((user_debug & UDBG_SEGV) && (sig == SIGSEGV)) ||
> ((user_debug & UDBG_BUS) && (sig == SIGBUS))) {
> diff --git a/arch/arm/mm/proc-v7-bugs.c b/arch/arm/mm/proc-v7-bugs.c
> index a32ce13479d9..65a9b8141f86 100644
> --- a/arch/arm/mm/proc-v7-bugs.c
> +++ b/arch/arm/mm/proc-v7-bugs.c
> @@ -2,6 +2,12 @@
> #include <linux/kernel.h>
> #include <linux/smp.h>
>
> +#include <asm/cp15.h>
> +#include <asm/cputype.h>
> +#include <asm/system_misc.h>
> +
> +void cpu_v7_bugs_init(void);
> +
> static __maybe_unused void cpu_v7_check_auxcr_set(u32 mask, const char *msg)
> {
> u32 aux_cr;
> @@ -21,9 +27,54 @@ static void check_spectre_auxcr(u32 bit)
> void cpu_v7_ca8_ibe(void)
> {
> check_spectre_auxcr(BIT(6));
> + cpu_v7_bugs_init();
> }
>
> void cpu_v7_ca15_ibe(void)
> {
> check_spectre_auxcr(BIT(0));
> + cpu_v7_bugs_init();
> +}
> +
> +#ifdef CONFIG_HARDEN_BRANCH_PREDICTOR
> +void (*harden_branch_predictor)(void);
> +
> +static void harden_branch_predictor_bpiall(void)
> +{
> + write_sysreg(0, BPIALL);
> +}
> +
> +static void harden_branch_predictor_iciallu(void)
> +{
> + write_sysreg(0, ICIALLU);
> +}
> +
> +void cpu_v7_bugs_init(void)
> +{
> + const char *spectre_v2_method = NULL;
> +
> + if (harden_branch_predictor)
> + return;
How does it work on a big-little systems where two CPUs have diverging
mitigation methods? Let's say an hypothetical A15/A17 system? Or even a
more common A15/A7 system, where the small core doesn't require the
mitigation?
> +
> + switch (read_cpuid_part()) {
> + case ARM_CPU_PART_CORTEX_A8:
> + case ARM_CPU_PART_CORTEX_A9:
> + case ARM_CPU_PART_CORTEX_A12:
> + case ARM_CPU_PART_CORTEX_A17:
> + case ARM_CPU_PART_CORTEX_A73:
> + case ARM_CPU_PART_CORTEX_A75:
> + harden_branch_predictor = harden_branch_predictor_bpiall;
> + spectre_v2_method = "BPIALL";
> + break;
You don't seem to take into account the PFR0.CSV2 field which indicates
that the CPU has a branch predictor that is immune to Spectre-v2.
See for example the Cortex-A75 r3p0 TRM[1].
> +
> + case ARM_CPU_PART_CORTEX_A15:
> + case ARM_CPU_PART_BRAHMA_B15:
> + harden_branch_predictor = harden_branch_predictor_iciallu;
> + spectre_v2_method = "ICIALLU";
> + break;
> + }
> + if (spectre_v2_method)
> + pr_info("CPU: Spectre v2: using %s workaround\n",
> + spectre_v2_method);
> }
> +#endif
> diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
> index fa9214036fb3..79510011e7eb 100644
> --- a/arch/arm/mm/proc-v7.S
> +++ b/arch/arm/mm/proc-v7.S
> @@ -532,8 +532,10 @@ ENDPROC(__v7_setup)
>
> __INITDATA
>
> + .weak cpu_v7_bugs_init
> +
> @ define struct processor (see <asm/proc-fns.h> and proc-macros.S)
> - define_processor_functions v7, dabort=v7_early_abort, pabort=v7_pabort, suspend=1
> + define_processor_functions v7, dabort=v7_early_abort, pabort=v7_pabort, suspend=1, bugs=cpu_v7_bugs_init
>
> #ifdef CONFIG_HARDEN_BRANCH_PREDICTOR
> @ generic v7 bpiall on context switch
> @@ -548,7 +550,7 @@ ENDPROC(__v7_setup)
> globl_equ cpu_v7_bpiall_do_suspend, cpu_v7_do_suspend
> globl_equ cpu_v7_bpiall_do_resume, cpu_v7_do_resume
> #endif
> - define_processor_functions v7_bpiall, dabort=v7_early_abort, pabort=v7_pabort, suspend=1
> + define_processor_functions v7_bpiall, dabort=v7_early_abort, pabort=v7_pabort, suspend=1, bugs=cpu_v7_bugs_init
>
> #define HARDENED_BPIALL_PROCESSOR_FUNCTIONS v7_bpiall_processor_functions
> #else
> @@ -584,7 +586,7 @@ ENDPROC(__v7_setup)
> globl_equ cpu_ca9mp_switch_mm, cpu_v7_switch_mm
> #endif
> globl_equ cpu_ca9mp_set_pte_ext, cpu_v7_set_pte_ext
> - define_processor_functions ca9mp, dabort=v7_early_abort, pabort=v7_pabort, suspend=1
> + define_processor_functions ca9mp, dabort=v7_early_abort, pabort=v7_pabort, suspend=1, bugs=cpu_v7_bugs_init
> #endif
>
> @ Cortex-A15 - needs iciallu switch_mm for hardening
>
Thanks,
M.
[1]
http://infocenter.arm.com/help/topic/com.arm.doc.100403_0300_00_en/axa1518783469631.html
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* [PATCH v3 3/4] dt-bindings: serial: Add compatible for Mediatek MT8183
From: Rob Herring @ 2018-05-22 17:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526538126-51497-4-git-send-email-erin.lo@mediatek.com>
On Thu, May 17, 2018 at 02:22:05PM +0800, Erin Lo wrote:
> This adds dt-binding documentation of uart for Mediatek MT8183 SoC
> Platform.
>
> Signed-off-by: Erin Lo <erin.lo@mediatek.com>
> ---
> Documentation/devicetree/bindings/serial/mtk-uart.txt | 1 +
> 1 file changed, 1 insertion(+)
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* [PATCH v3 3/4] dt-bindings: rtc: update stm32-rtc documentation for stm32mp1 rtc
From: Rob Herring @ 2018-05-22 17:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526558666-24243-4-git-send-email-amelie.delaunay@st.com>
On Thu, May 17, 2018 at 02:04:25PM +0200, Amelie Delaunay wrote:
> RTC embedded in stm32mp1 SoC is slightly different from stm32h7 one, it
> doesn't require to disable backup domain write protection, and rtc_ck
> parent clock assignment isn't allowed.
> To sum up, stm32mp1 RTC requires 2 clocks, pclk and rtc_ck, and an RTC
> alarm interrupt.
>
> Signed-off-by: Amelie Delaunay <amelie.delaunay@st.com>
> ---
> .../devicetree/bindings/rtc/st,stm32-rtc.txt | 27 ++++++++++++++++------
> 1 file changed, 20 insertions(+), 7 deletions(-)
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* [PATCH 2/2] arm64: Unify kernel fault reporting
From: Will Deacon @ 2018-05-22 17:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180521131451.41040-3-mark.rutland@arm.com>
On Mon, May 21, 2018 at 02:14:51PM +0100, Mark Rutland wrote:
> In do_page_fault(), we handle some kernel faults early, and simply
> die() with a message. For faults handled later, we dump the faulting
> address, decode the ESR, walk the page tables, and perform a number of
> steps to ensure that this data is reported.
>
> Let's unify the handling of fatal kernel faults with a new
> die_kernel_fault() helper, handling all of these details. This is
> largely the same as the existing logic in __do_kernel_fault(), except
> that addresses are consistently padded to 16 hex characters, as would be
> expected for a 64-bit address.
>
> The messages currently logged in do_page_fault are adjusted to fit into
> the die_kernel_fault() message template.
>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> ---
> arch/arm64/mm/fault.c | 37 +++++++++++++++++++++++--------------
> 1 file changed, 23 insertions(+), 14 deletions(-)
Acked-by: Will Deacon <will.deacon@arm.com>
Will
^ permalink raw reply
* [PATCH v2] iommu/io-pgtable-arm: Make allocations NUMA-aware
From: Will Deacon @ 2018-05-22 17:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <b420b4733b5bc939931c11ee573bbb598f72e7b0.1526989635.git.robin.murphy@arm.com>
On Tue, May 22, 2018 at 12:50:09PM +0100, Robin Murphy wrote:
> We would generally expect pagetables to be read by the IOMMU more than
> written by the CPU, so in NUMA systems it makes sense to locate them
> close to the former and avoid cross-node pagetable walks if at all
> possible. As it turns out, we already have a handle on the IOMMU device
> for the sake of coherency management, so it's trivial to grab the
> appropriate NUMA node when allocating new pagetable pages.
>
> Note that we drop the semantics of alloc_pages_exact(), but that's fine
> since they have never been necessary: the only time we're allocating
> more than one page is for stage 2 top-level concatenation, but since
> that is based on the number of IPA bits, the size is always some exact
> power of two anyway.
>
> Acked-by: Will Deacon <will.deacon@arm.com>
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> ---
>
> v2: Retain equivalent highmem check
>
> drivers/iommu/io-pgtable-arm.c | 13 +++++++++----
> 1 file changed, 9 insertions(+), 4 deletions(-)
Thanks, Robin.
Joerg -- please you can pick this up for 4.18? I don't have any other SMMU
patches pending, so it doesn't seem worth putting together a pull request
just for this.
Cheers,
Will
> diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
> index 39c2a056da21..5194755bba29 100644
> --- a/drivers/iommu/io-pgtable-arm.c
> +++ b/drivers/iommu/io-pgtable-arm.c
> @@ -231,12 +231,17 @@ static void *__arm_lpae_alloc_pages(size_t size, gfp_t gfp,
> struct io_pgtable_cfg *cfg)
> {
> struct device *dev = cfg->iommu_dev;
> + int order = get_order(size);
> + struct page *p;
> dma_addr_t dma;
> - void *pages = alloc_pages_exact(size, gfp | __GFP_ZERO);
> + void *pages;
>
> - if (!pages)
> + VM_BUG_ON((gfp & __GFP_HIGHMEM));
> + p = alloc_pages_node(dev_to_node(dev), gfp | __GFP_ZERO, order);
> + if (!p)
> return NULL;
>
> + pages = page_address(p);
> if (!(cfg->quirks & IO_PGTABLE_QUIRK_NO_DMA)) {
> dma = dma_map_single(dev, pages, size, DMA_TO_DEVICE);
> if (dma_mapping_error(dev, dma))
> @@ -256,7 +261,7 @@ static void *__arm_lpae_alloc_pages(size_t size, gfp_t gfp,
> dev_err(dev, "Cannot accommodate DMA translation for IOMMU page tables\n");
> dma_unmap_single(dev, dma, size, DMA_TO_DEVICE);
> out_free:
> - free_pages_exact(pages, size);
> + __free_pages(p, order);
> return NULL;
> }
>
> @@ -266,7 +271,7 @@ static void __arm_lpae_free_pages(void *pages, size_t size,
> if (!(cfg->quirks & IO_PGTABLE_QUIRK_NO_DMA))
> dma_unmap_single(cfg->iommu_dev, __arm_lpae_dma_addr(pages),
> size, DMA_TO_DEVICE);
> - free_pages_exact(pages, size);
> + free_pages((unsigned long)pages, get_order(size));
> }
>
> static void __arm_lpae_sync_pte(arm_lpae_iopte *ptep,
> --
> 2.17.0.dirty
>
^ permalink raw reply
* [PATCH v3] arm64: signal: Report signal frame size to userspace via auxv
From: Will Deacon @ 2018-05-22 17:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526571941-9816-1-git-send-email-Dave.Martin@arm.com>
Hi Dave,
On Thu, May 17, 2018 at 04:45:41PM +0100, Dave Martin wrote:
> Stateful CPU architecture extensions may require the signal frame
> to grow to a size that exceeds the arch's MINSIGSTKSZ #define.
> However, changing this #define is an ABI break.
[...]
> For arm64 SVE:
>
> The SVE context block in the signal frame needs to be considered
> too when computing the maximum possible signal frame size.
>
> Because the size of this block depends on the vector length, this
> patch computes the size based not on the thread's current vector
> length but instead on the maximum possible vector length: this
> determines the maximum size of SVE context block that can be
> observed in any signal frame for the lifetime of the process.
>
> Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Alex Benn?e <alex.bennee@linaro.org>
>
> ---
>
> Changes since v2:
>
> * Redefine AT_MINSIGSTKSZ as 51 to avoid clash with values defined by
> other architectures.
>
> This turns out to be a problem for glibc; also random userspace
> software does not necessary check the architecture before using
> getauxval() or otherwise parsing the aux vector, which can make
> aliased tags problematic.
>
> Really, the headers need cleaning up tree-wide in such away that the
> AT_* definitions do not appear to be arch-private. To be addressed
> separately.
> ---
> arch/arm64/include/asm/elf.h | 11 ++++++++
> arch/arm64/include/asm/processor.h | 5 ++++
> arch/arm64/include/uapi/asm/auxvec.h | 3 ++-
> arch/arm64/kernel/cpufeature.c | 1 +
> arch/arm64/kernel/signal.c | 51 +++++++++++++++++++++++++++++++-----
> 5 files changed, 63 insertions(+), 8 deletions(-)
>
> diff --git a/arch/arm64/include/asm/elf.h b/arch/arm64/include/asm/elf.h
> index fac1c4d..dc32adb 100644
> --- a/arch/arm64/include/asm/elf.h
> +++ b/arch/arm64/include/asm/elf.h
> @@ -24,6 +24,11 @@
> #include <asm/ptrace.h>
> #include <asm/user.h>
>
> +#ifndef __ASSEMBLY__
> +#include <linux/bug.h>
> +#include <asm/processor.h> /* for signal_minsigstksz, used by ARCH_DLINFO */
> +#endif
Maybe move these inside the pre-existing #ifndef __ASSEMBLY__ block.
> /*
> * AArch64 static relocation types.
> */
> @@ -146,8 +151,14 @@ typedef struct user_fpsimd_state elf_fpregset_t;
> /* update AT_VECTOR_SIZE_ARCH if the number of NEW_AUX_ENT entries changes */
> #define ARCH_DLINFO \
> do { \
> + int minsigstksz = signal_minsigstksz; \
> + \
> + if (WARN_ON(minsigstksz <= 0)) \
> + minsigstksz = MINSIGSTKSZ; \
> + \
How can this happen?
> NEW_AUX_ENT(AT_SYSINFO_EHDR, \
> (elf_addr_t)current->mm->context.vdso); \
> + NEW_AUX_ENT(AT_MINSIGSTKSZ, minsigstksz); \
> } while (0)
>
> #define ARCH_HAS_SETUP_ADDITIONAL_PAGES
> diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
> index 7675989..6f60e92 100644
> --- a/arch/arm64/include/asm/processor.h
> +++ b/arch/arm64/include/asm/processor.h
> @@ -35,6 +35,8 @@
> #ifdef __KERNEL__
>
> #include <linux/build_bug.h>
> +#include <linux/cache.h>
> +#include <linux/init.h>
> #include <linux/stddef.h>
> #include <linux/string.h>
>
> @@ -244,6 +246,9 @@ void cpu_enable_pan(const struct arm64_cpu_capabilities *__unused);
> void cpu_enable_cache_maint_trap(const struct arm64_cpu_capabilities *__unused);
> void cpu_clear_disr(const struct arm64_cpu_capabilities *__unused);
>
> +extern int __ro_after_init signal_minsigstksz; /* user signal frame size */
Probably better as unsigned long, to be consistent with the size field
of the sigframe user layout structure.
> +extern void __init minsigstksz_setup(void);
> +
> /* Userspace interface for PR_SVE_{SET,GET}_VL prctl()s: */
> #define SVE_SET_VL(arg) sve_set_current_vl(arg)
> #define SVE_GET_VL() sve_get_current_vl()
> diff --git a/arch/arm64/include/uapi/asm/auxvec.h b/arch/arm64/include/uapi/asm/auxvec.h
> index ec0a86d..743c0b8 100644
> --- a/arch/arm64/include/uapi/asm/auxvec.h
> +++ b/arch/arm64/include/uapi/asm/auxvec.h
> @@ -19,7 +19,8 @@
>
> /* vDSO location */
> #define AT_SYSINFO_EHDR 33
> +#define AT_MINSIGSTKSZ 51 /* stack needed for signal delivery */
>
> -#define AT_VECTOR_SIZE_ARCH 1 /* entries in ARCH_DLINFO */
> +#define AT_VECTOR_SIZE_ARCH 2 /* entries in ARCH_DLINFO */
>
> #endif
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index 9d1b06d..0e0b53d 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -1619,6 +1619,7 @@ void __init setup_cpu_features(void)
> pr_info("emulated: Privileged Access Never (PAN) using TTBR0_EL1 switching\n");
>
> sve_setup();
> + minsigstksz_setup();
>
> /* Advertise that we have computed the system capabilities */
> set_sys_caps_initialised();
> diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
> index 154b7d3..ae8d4ea 100644
> --- a/arch/arm64/kernel/signal.c
> +++ b/arch/arm64/kernel/signal.c
> @@ -17,6 +17,7 @@
> * along with this program. If not, see <http://www.gnu.org/licenses/>.
> */
>
> +#include <linux/cache.h>
> #include <linux/compat.h>
> #include <linux/errno.h>
> #include <linux/kernel.h>
> @@ -570,8 +571,15 @@ asmlinkage long sys_rt_sigreturn(struct pt_regs *regs)
> return 0;
> }
>
> -/* Determine the layout of optional records in the signal frame */
> -static int setup_sigframe_layout(struct rt_sigframe_user_layout *user)
> +/*
> + * Determine the layout of optional records in the signal frame
> + *
> + * add_all: if true, lays out the biggest possible signal frame for
> + * this task; otherwise, generates a layout for the current state
> + * of the task.
> + */
> +static int setup_sigframe_layout(struct rt_sigframe_user_layout *user,
> + bool add_all)
> {
> int err;
>
> @@ -581,7 +589,7 @@ static int setup_sigframe_layout(struct rt_sigframe_user_layout *user)
> return err;
>
> /* fault information, if valid */
> - if (current->thread.fault_code) {
> + if (add_all || current->thread.fault_code) {
> err = sigframe_alloc(user, &user->esr_offset,
> sizeof(struct esr_context));
> if (err)
> @@ -591,8 +599,18 @@ static int setup_sigframe_layout(struct rt_sigframe_user_layout *user)
> if (system_supports_sve()) {
> unsigned int vq = 0;
>
> - if (test_thread_flag(TIF_SVE))
> - vq = sve_vq_from_vl(current->thread.sve_vl);
> + if (add_all || test_thread_flag(TIF_SVE)) {
> + int vl = sve_max_vl;
> +
> + if (!add_all)
> + vl = current->thread.sve_vl;
> +
> + /* Fail safe if something wasn't initialised */
> + if (WARN_ON(!sve_vl_valid(vl)))
> + vl = SVE_VL_MIN;
How can this happen?
> +
> + vq = sve_vq_from_vl(vl);
> + }
>
> err = sigframe_alloc(user, &user->sve_offset,
> SVE_SIG_CONTEXT_SIZE(vq));
> @@ -603,7 +621,6 @@ static int setup_sigframe_layout(struct rt_sigframe_user_layout *user)
> return sigframe_alloc_end(user);
> }
>
> -
> static int setup_sigframe(struct rt_sigframe_user_layout *user,
> struct pt_regs *regs, sigset_t *set)
> {
> @@ -701,7 +718,7 @@ static int get_sigframe(struct rt_sigframe_user_layout *user,
> int err;
>
> init_user_layout(user);
> - err = setup_sigframe_layout(user);
> + err = setup_sigframe_layout(user, false);
> if (err)
> return err;
>
> @@ -936,3 +953,23 @@ asmlinkage void do_notify_resume(struct pt_regs *regs,
> thread_flags = READ_ONCE(current_thread_info()->flags);
> } while (thread_flags & _TIF_WORK_MASK);
> }
> +
> +int __ro_after_init signal_minsigstksz;
> +
> +/*
> + * Determine the stack space required for guaranteed signal devliery.
> + * This function is used to populate AT_MINSIGSTKSZ at process startup.
> + */
> +void __init minsigstksz_setup(void)
> +{
> + struct rt_sigframe_user_layout user;
> +
> + init_user_layout(&user);
> +
> + if (WARN_ON(setup_sigframe_layout(&user, true)))
> + signal_minsigstksz = SIGSTKSZ;
Why not just omit the aux record in this case? Something has gone badly
wrong, so it's unlikely we're going to get much further anyway.
Will
^ permalink raw reply
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