* [PATCH v5 09/11] PCI: qcom: Add ipq8064 rev2 variant
From: Ansuel Smith @ 2020-06-02 11:53 UTC (permalink / raw)
To: Rob Herring
Cc: Ansuel Smith, Andy Gross, Bjorn Andersson, Bjorn Helgaas,
Mark Rutland, Stanimir Varbanov, Lorenzo Pieralisi, Andrew Murray,
Philipp Zabel, linux-arm-msm, linux-pci, devicetree, linux-kernel
In-Reply-To: <20200602115353.20143-1-ansuelsmth@gmail.com>
Ipq8064-v2 have tx term offset set to 0. Introduce this variant to permit
different offset based on the revision.
Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
drivers/pci/controller/dwc/pcie-qcom.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 2cd6d1456210..259b627bf890 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -366,7 +366,8 @@ static int qcom_pcie_init_2_1_0(struct qcom_pcie *pcie)
val &= ~BIT(0);
writel(val, pcie->parf + PCIE20_PARF_PHY_CTRL);
- if (of_device_is_compatible(node, "qcom,pcie-ipq8064")) {
+ if (of_device_is_compatible(node, "qcom,pcie-ipq8064") ||
+ of_device_is_compatible(node, "qcom,pcie-ipq8064-v2")) {
writel(PCS_DEEMPH_TX_DEEMPH_GEN1(24) |
PCS_DEEMPH_TX_DEEMPH_GEN2_3_5DB(24) |
PCS_DEEMPH_TX_DEEMPH_GEN2_6DB(34),
@@ -1464,6 +1465,7 @@ static int qcom_pcie_probe(struct platform_device *pdev)
static const struct of_device_id qcom_pcie_match[] = {
{ .compatible = "qcom,pcie-apq8084", .data = &ops_1_0_0 },
{ .compatible = "qcom,pcie-ipq8064", .data = &ops_2_1_0 },
+ { .compatible = "qcom,pcie-ipq8064-v2", .data = &ops_2_1_0 },
{ .compatible = "qcom,pcie-apq8064", .data = &ops_2_1_0 },
{ .compatible = "qcom,pcie-msm8996", .data = &ops_2_3_2 },
{ .compatible = "qcom,pcie-ipq8074", .data = &ops_2_3_3 },
--
2.25.1
^ permalink raw reply related
* [PATCH v5 10/11] dt-bindings: PCI: qcom: Add ipq8064 rev 2 variant
From: Ansuel Smith @ 2020-06-02 11:53 UTC (permalink / raw)
To: Rob Herring
Cc: Ansuel Smith, Rob Herring, Andy Gross, Bjorn Andersson,
Bjorn Helgaas, Mark Rutland, Stanimir Varbanov, Lorenzo Pieralisi,
Andrew Murray, Philipp Zabel, linux-arm-msm, linux-pci,
devicetree, linux-kernel
In-Reply-To: <20200602115353.20143-1-ansuelsmth@gmail.com>
Document qcom,pcie-ipq8064-v2 needed to use different phy_tx0_term_offset.
In ipq8064 phy_tx0_term_offset is 7. In ipq8064 v2 other SoC it's set to 0
by default.
Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
Acked-by: Rob Herring <robh@kernel.org>
---
Documentation/devicetree/bindings/pci/qcom,pcie.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.txt b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
index 6efcef040741..02bc81bb8b2d 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie.txt
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
@@ -5,6 +5,7 @@
Value type: <stringlist>
Definition: Value should contain
- "qcom,pcie-ipq8064" for ipq8064
+ - "qcom,pcie-ipq8064-v2" for ipq8064 rev 2 or ipq8065
- "qcom,pcie-apq8064" for apq8064
- "qcom,pcie-apq8084" for apq8084
- "qcom,pcie-msm8996" for msm8996 or apq8096
--
2.25.1
^ permalink raw reply related
* [PATCH v5 11/11] PCI: qcom: Add Force GEN1 support
From: Ansuel Smith @ 2020-06-02 11:53 UTC (permalink / raw)
To: Rob Herring
Cc: Sham Muthayyan, Ansuel Smith, Rob Herring, Andy Gross,
Bjorn Andersson, Bjorn Helgaas, Mark Rutland, Stanimir Varbanov,
Lorenzo Pieralisi, Andrew Murray, Philipp Zabel, linux-arm-msm,
linux-pci, devicetree, linux-kernel
In-Reply-To: <20200602115353.20143-1-ansuelsmth@gmail.com>
From: Sham Muthayyan <smuthayy@codeaurora.org>
Add Force GEN1 support needed in some ipq8064 board that needs to limit
some PCIe line to gen1 for some hardware limitation. This is set by the
max-link-speed binding and needed by some soc based on ipq8064. (for
example Netgear R7800 router)
Signed-off-by: Sham Muthayyan <smuthayy@codeaurora.org>
Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
drivers/pci/controller/dwc/pcie-qcom.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 259b627bf890..0ce15d53c46e 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -27,6 +27,7 @@
#include <linux/slab.h>
#include <linux/types.h>
+#include "../../pci.h"
#include "pcie-designware.h"
#define PCIE20_PARF_SYS_CTRL 0x00
@@ -99,6 +100,8 @@
#define PCIE20_v3_PARF_SLV_ADDR_SPACE_SIZE 0x358
#define SLV_ADDR_SPACE_SZ 0x10000000
+#define PCIE20_LNK_CONTROL2_LINK_STATUS2 0xa0
+
#define DEVICE_TYPE_RC 0x4
#define QCOM_PCIE_2_1_0_MAX_SUPPLY 3
@@ -195,6 +198,7 @@ struct qcom_pcie {
struct phy *phy;
struct gpio_desc *reset;
const struct qcom_pcie_ops *ops;
+ int gen;
};
#define to_qcom_pcie(x) dev_get_drvdata((x)->dev)
@@ -395,6 +399,11 @@ static int qcom_pcie_init_2_1_0(struct qcom_pcie *pcie)
/* wait for clock acquisition */
usleep_range(1000, 1500);
+ if (pcie->gen == 1) {
+ val = readl(pci->dbi_base + PCIE20_LNK_CONTROL2_LINK_STATUS2);
+ val |= 1;
+ writel(val, pci->dbi_base + PCIE20_LNK_CONTROL2_LINK_STATUS2);
+ }
/* Set the Max TLP size to 2K, instead of using default of 4K */
writel(CFG_REMOTE_RD_REQ_BRIDGE_SIZE_2K,
@@ -1397,6 +1406,10 @@ static int qcom_pcie_probe(struct platform_device *pdev)
goto err_pm_runtime_put;
}
+ pcie->gen = of_pci_get_max_link_speed(pdev->dev.of_node);
+ if (pcie->gen < 0)
+ pcie->gen = 2;
+
res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "parf");
pcie->parf = devm_ioremap_resource(dev, res);
if (IS_ERR(pcie->parf)) {
--
2.25.1
^ permalink raw reply related
* [PATCH v5 06/11] PCI: qcom: Use bulk clk api and assert on error
From: Ansuel Smith @ 2020-06-02 11:53 UTC (permalink / raw)
To: Rob Herring
Cc: Ansuel Smith, Rob Herring, Andy Gross, Bjorn Andersson,
Bjorn Helgaas, Mark Rutland, Stanimir Varbanov, Lorenzo Pieralisi,
Andrew Murray, Philipp Zabel, linux-arm-msm, linux-pci,
devicetree, linux-kernel
In-Reply-To: <20200602115353.20143-1-ansuelsmth@gmail.com>
Rework 2.1.0 revision to use bulk clk api and fix missing assert on
reset_control_deassert error.
Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
drivers/pci/controller/dwc/pcie-qcom.c | 131 +++++++++----------------
1 file changed, 46 insertions(+), 85 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 4dab5ef630cc..f2ea1ab6f584 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -84,12 +84,9 @@
#define DEVICE_TYPE_RC 0x4
#define QCOM_PCIE_2_1_0_MAX_SUPPLY 3
+#define QCOM_PCIE_2_1_0_MAX_CLOCKS 5
struct qcom_pcie_resources_2_1_0 {
- struct clk *iface_clk;
- struct clk *core_clk;
- struct clk *phy_clk;
- struct clk *aux_clk;
- struct clk *ref_clk;
+ struct clk_bulk_data clks[QCOM_PCIE_2_1_0_MAX_CLOCKS];
struct reset_control *pci_reset;
struct reset_control *axi_reset;
struct reset_control *ahb_reset;
@@ -237,25 +234,21 @@ static int qcom_pcie_get_resources_2_1_0(struct qcom_pcie *pcie)
if (ret)
return ret;
- res->iface_clk = devm_clk_get(dev, "iface");
- if (IS_ERR(res->iface_clk))
- return PTR_ERR(res->iface_clk);
-
- res->core_clk = devm_clk_get(dev, "core");
- if (IS_ERR(res->core_clk))
- return PTR_ERR(res->core_clk);
-
- res->phy_clk = devm_clk_get(dev, "phy");
- if (IS_ERR(res->phy_clk))
- return PTR_ERR(res->phy_clk);
+ res->clks[0].id = "iface";
+ res->clks[1].id = "core";
+ res->clks[2].id = "phy";
+ res->clks[3].id = "aux";
+ res->clks[4].id = "ref";
- res->aux_clk = devm_clk_get_optional(dev, "aux");
- if (IS_ERR(res->aux_clk))
- return PTR_ERR(res->aux_clk);
+ /* iface, core, phy are required */
+ ret = devm_clk_bulk_get(dev, 3, res->clks);
+ if (ret < 0)
+ return ret;
- res->ref_clk = devm_clk_get_optional(dev, "ref");
- if (IS_ERR(res->ref_clk))
- return PTR_ERR(res->ref_clk);
+ /* aux, ref are optional */
+ ret = devm_clk_bulk_get_optional(dev, 2, res->clks + 3);
+ if (ret < 0)
+ return ret;
res->pci_reset = devm_reset_control_get_exclusive(dev, "pci");
if (IS_ERR(res->pci_reset))
@@ -285,17 +278,13 @@ static void qcom_pcie_deinit_2_1_0(struct qcom_pcie *pcie)
{
struct qcom_pcie_resources_2_1_0 *res = &pcie->res.v2_1_0;
- clk_disable_unprepare(res->phy_clk);
+ clk_bulk_disable_unprepare(ARRAY_SIZE(res->clks), res->clks);
reset_control_assert(res->pci_reset);
reset_control_assert(res->axi_reset);
reset_control_assert(res->ahb_reset);
reset_control_assert(res->por_reset);
reset_control_assert(res->ext_reset);
reset_control_assert(res->phy_reset);
- clk_disable_unprepare(res->iface_clk);
- clk_disable_unprepare(res->core_clk);
- clk_disable_unprepare(res->aux_clk);
- clk_disable_unprepare(res->ref_clk);
regulator_bulk_disable(ARRAY_SIZE(res->supplies), res->supplies);
}
@@ -313,36 +302,6 @@ static int qcom_pcie_init_2_1_0(struct qcom_pcie *pcie)
return ret;
}
- ret = reset_control_assert(res->ahb_reset);
- if (ret) {
- dev_err(dev, "cannot assert ahb reset\n");
- goto err_assert_ahb;
- }
-
- ret = clk_prepare_enable(res->iface_clk);
- if (ret) {
- dev_err(dev, "cannot prepare/enable iface clock\n");
- goto err_assert_ahb;
- }
-
- ret = clk_prepare_enable(res->core_clk);
- if (ret) {
- dev_err(dev, "cannot prepare/enable core clock\n");
- goto err_clk_core;
- }
-
- ret = clk_prepare_enable(res->aux_clk);
- if (ret) {
- dev_err(dev, "cannot prepare/enable aux clock\n");
- goto err_clk_aux;
- }
-
- ret = clk_prepare_enable(res->ref_clk);
- if (ret) {
- dev_err(dev, "cannot prepare/enable ref clock\n");
- goto err_clk_ref;
- }
-
ret = reset_control_deassert(res->ahb_reset);
if (ret) {
dev_err(dev, "cannot deassert ahb reset\n");
@@ -352,48 +311,46 @@ static int qcom_pcie_init_2_1_0(struct qcom_pcie *pcie)
ret = reset_control_deassert(res->ext_reset);
if (ret) {
dev_err(dev, "cannot deassert ext reset\n");
- goto err_deassert_ahb;
+ goto err_deassert_ext;
}
- /* enable PCIe clocks and resets */
- val = readl(pcie->parf + PCIE20_PARF_PHY_CTRL);
- val &= ~BIT(0);
- writel(val, pcie->parf + PCIE20_PARF_PHY_CTRL);
-
- /* enable external reference clock */
- val = readl(pcie->parf + PCIE20_PARF_PHY_REFCLK);
- val |= BIT(16);
- writel(val, pcie->parf + PCIE20_PARF_PHY_REFCLK);
-
ret = reset_control_deassert(res->phy_reset);
if (ret) {
dev_err(dev, "cannot deassert phy reset\n");
- return ret;
+ goto err_deassert_phy;
}
ret = reset_control_deassert(res->pci_reset);
if (ret) {
dev_err(dev, "cannot deassert pci reset\n");
- return ret;
+ goto err_deassert_pci;
}
ret = reset_control_deassert(res->por_reset);
if (ret) {
dev_err(dev, "cannot deassert por reset\n");
- return ret;
+ goto err_deassert_por;
}
ret = reset_control_deassert(res->axi_reset);
if (ret) {
dev_err(dev, "cannot deassert axi reset\n");
- return ret;
+ goto err_deassert_axi;
}
- ret = clk_prepare_enable(res->phy_clk);
- if (ret) {
- dev_err(dev, "cannot prepare/enable phy clock\n");
- goto err_deassert_ahb;
- }
+ ret = clk_bulk_prepare_enable(ARRAY_SIZE(res->clks), res->clks);
+ if (ret)
+ goto err_clks;
+
+ /* enable PCIe clocks and resets */
+ val = readl(pcie->parf + PCIE20_PARF_PHY_CTRL);
+ val &= ~BIT(0);
+ writel(val, pcie->parf + PCIE20_PARF_PHY_CTRL);
+
+ /* enable external reference clock */
+ val = readl(pcie->parf + PCIE20_PARF_PHY_REFCLK);
+ val |= BIT(16);
+ writel(val, pcie->parf + PCIE20_PARF_PHY_REFCLK);
/* wait for clock acquisition */
usleep_range(1000, 1500);
@@ -407,15 +364,19 @@ static int qcom_pcie_init_2_1_0(struct qcom_pcie *pcie)
return 0;
+err_clks:
+ reset_control_assert(res->axi_reset);
+err_deassert_axi:
+ reset_control_assert(res->por_reset);
+err_deassert_por:
+ reset_control_assert(res->pci_reset);
+err_deassert_pci:
+ reset_control_assert(res->phy_reset);
+err_deassert_phy:
+ reset_control_assert(res->ext_reset);
+err_deassert_ext:
+ reset_control_assert(res->ahb_reset);
err_deassert_ahb:
- clk_disable_unprepare(res->ref_clk);
-err_clk_ref:
- clk_disable_unprepare(res->aux_clk);
-err_clk_aux:
- clk_disable_unprepare(res->core_clk);
-err_clk_core:
- clk_disable_unprepare(res->iface_clk);
-err_assert_ahb:
regulator_bulk_disable(ARRAY_SIZE(res->supplies), res->supplies);
return ret;
--
2.25.1
^ permalink raw reply related
* [PATCH v5 03/11] PCI: qcom: Change duplicate PCI reset to phy reset
From: Ansuel Smith @ 2020-06-02 11:53 UTC (permalink / raw)
To: Rob Herring
Cc: Abhishek Sahu, Ansuel Smith, Rob Herring, Andy Gross,
Bjorn Andersson, Bjorn Helgaas, Mark Rutland, Stanimir Varbanov,
Lorenzo Pieralisi, Andrew Murray, Philipp Zabel, linux-arm-msm,
linux-pci, devicetree, linux-kernel
In-Reply-To: <20200602115353.20143-1-ansuelsmth@gmail.com>
From: Abhishek Sahu <absahu@codeaurora.org>
The deinit issues reset_control_assert for PCI twice and does not contain
phy reset.
Signed-off-by: Abhishek Sahu <absahu@codeaurora.org>
Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
drivers/pci/controller/dwc/pcie-qcom.c | 18 ++++++++----------
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 4bf93ab8c7a7..4512c2c5f61c 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -280,14 +280,14 @@ static void qcom_pcie_deinit_2_1_0(struct qcom_pcie *pcie)
{
struct qcom_pcie_resources_2_1_0 *res = &pcie->res.v2_1_0;
+ clk_disable_unprepare(res->phy_clk);
reset_control_assert(res->pci_reset);
reset_control_assert(res->axi_reset);
reset_control_assert(res->ahb_reset);
reset_control_assert(res->por_reset);
- reset_control_assert(res->pci_reset);
+ reset_control_assert(res->phy_reset);
clk_disable_unprepare(res->iface_clk);
clk_disable_unprepare(res->core_clk);
- clk_disable_unprepare(res->phy_clk);
clk_disable_unprepare(res->aux_clk);
clk_disable_unprepare(res->ref_clk);
regulator_bulk_disable(ARRAY_SIZE(res->supplies), res->supplies);
@@ -325,12 +325,6 @@ static int qcom_pcie_init_2_1_0(struct qcom_pcie *pcie)
goto err_clk_core;
}
- ret = clk_prepare_enable(res->phy_clk);
- if (ret) {
- dev_err(dev, "cannot prepare/enable phy clock\n");
- goto err_clk_phy;
- }
-
ret = clk_prepare_enable(res->aux_clk);
if (ret) {
dev_err(dev, "cannot prepare/enable aux clock\n");
@@ -383,6 +377,12 @@ static int qcom_pcie_init_2_1_0(struct qcom_pcie *pcie)
return ret;
}
+ ret = clk_prepare_enable(res->phy_clk);
+ if (ret) {
+ dev_err(dev, "cannot prepare/enable phy clock\n");
+ goto err_deassert_ahb;
+ }
+
/* wait for clock acquisition */
usleep_range(1000, 1500);
@@ -400,8 +400,6 @@ static int qcom_pcie_init_2_1_0(struct qcom_pcie *pcie)
err_clk_ref:
clk_disable_unprepare(res->aux_clk);
err_clk_aux:
- clk_disable_unprepare(res->phy_clk);
-err_clk_phy:
clk_disable_unprepare(res->core_clk);
err_clk_core:
clk_disable_unprepare(res->iface_clk);
--
2.25.1
^ permalink raw reply related
* [PATCH v5 00/11] Multiple fixes in PCIe qcom driver
From: Ansuel Smith @ 2020-06-02 11:53 UTC (permalink / raw)
To: Rob Herring
Cc: Ansuel Smith, Andy Gross, Bjorn Andersson, Bjorn Helgaas,
Mark Rutland, Stanimir Varbanov, Lorenzo Pieralisi, Andrew Murray,
Philipp Zabel, linux-arm-msm, linux-pci, devicetree, linux-kernel
This contains multiple fix for PCIe qcom driver.
Some optional reset and clocks were missing.
Fix a problem with no PARF programming that cause kernel lock on load.
Add support to force gen 1 speed if needed. (due to hardware limitation)
Add ipq8064 rev 2 support that use a different tx termination offset.
v5:
* Split PCI: qcom: Add ipq8064 rev2 variant and set tx term offset
v4:
* Fix grammar error across all patch subject
* Use bulk api for clks
* Program PARF only in ipq8064 SoC
* Program tx term only in ipq8064 SoC
* Drop configurable tx-dempth rx-eq
* Make added clk optional
v3:
* Fix check reported by checkpatch --strict
* Rename force_gen1 to gen
v2:
* Drop iATU programming (already done in pcie init)
* Use max-link-speed instead of force-gen1 custom definition
* Drop MRRS to 256B (Can't find a realy reason why this was suggested)
* Introduce a new variant for different revision of ipq8064
Abhishek Sahu (1):
PCI: qcom: Change duplicate PCI reset to phy reset
Ansuel Smith (9):
PCI: qcom: Add missing ipq806x clocks in PCIe driver
dt-bindings: PCI: qcom: Add missing clks
PCI: qcom: Add missing reset for ipq806x
dt-bindings: PCI: qcom: Add ext reset
PCI: qcom: Use bulk clk api and assert on error
PCI: qcom: Define some PARF params needed for ipq8064 SoC
PCI: qcom: Add support for tx term offset for rev 2.1.0
PCI: qcom: Add ipq8064 rev2 variant
dt-bindings: PCI: qcom: Add ipq8064 rev 2 variant
Sham Muthayyan (1):
PCI: qcom: Add Force GEN1 support
.../devicetree/bindings/pci/qcom,pcie.txt | 15 +-
drivers/pci/controller/dwc/pcie-qcom.c | 171 ++++++++++++------
2 files changed, 123 insertions(+), 63 deletions(-)
--
2.25.1
^ permalink raw reply
* Re: [PATCH v7 21/24] iommu/arm-smmu-v3: Add stall support for platform devices
From: Jean-Philippe Brucker @ 2020-06-02 11:46 UTC (permalink / raw)
To: Shameerali Kolothum Thodi
Cc: devicetree@vger.kernel.org, kevin.tian@intel.com, will@kernel.org,
fenghua.yu@intel.com, jgg@ziepe.ca, linux-pci@vger.kernel.org,
felix.kuehling@amd.com, hch@infradead.org, linux-mm@kvack.org,
iommu@lists.linux-foundation.org, catalin.marinas@arm.com,
zhangfei.gao@linaro.org, robin.murphy@arm.com,
christian.koenig@amd.com, linux-arm-kernel@lists.infradead.org
In-Reply-To: <1517c4d97b5849e6b6d32e7d7ed35289@huawei.com>
[-- Attachment #1: Type: text/plain, Size: 5830 bytes --]
On Tue, Jun 02, 2020 at 10:31:29AM +0000, Shameerali Kolothum Thodi wrote:
> Hi Jean,
>
> > -----Original Message-----
> > From: linux-arm-kernel [mailto:linux-arm-kernel-bounces@lists.infradead.org]
> > On Behalf Of Jean-Philippe Brucker
> > Sent: 02 June 2020 10:39
> > To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
> > Cc: devicetree@vger.kernel.org; kevin.tian@intel.com; will@kernel.org;
> > fenghua.yu@intel.com; jgg@ziepe.ca; linux-pci@vger.kernel.org;
> > felix.kuehling@amd.com; hch@infradead.org; linux-mm@kvack.org;
> > iommu@lists.linux-foundation.org; catalin.marinas@arm.com;
> > zhangfei.gao@linaro.org; robin.murphy@arm.com;
> > christian.koenig@amd.com; linux-arm-kernel@lists.infradead.org
> > Subject: Re: [PATCH v7 21/24] iommu/arm-smmu-v3: Add stall support for
> > platform devices
> >
> > Hi Shameer,
> >
> > On Mon, Jun 01, 2020 at 12:42:15PM +0000, Shameerali Kolothum Thodi
> > wrote:
> > > > /* IRQ and event handlers */
> > > > +static int arm_smmu_handle_evt(struct arm_smmu_device *smmu, u64
> > > > +*evt) {
> > > > + int ret;
> > > > + u32 perm = 0;
> > > > + struct arm_smmu_master *master;
> > > > + bool ssid_valid = evt[0] & EVTQ_0_SSV;
> > > > + u8 type = FIELD_GET(EVTQ_0_ID, evt[0]);
> > > > + u32 sid = FIELD_GET(EVTQ_0_SID, evt[0]);
> > > > + struct iommu_fault_event fault_evt = { };
> > > > + struct iommu_fault *flt = &fault_evt.fault;
> > > > +
> > > > + /* Stage-2 is always pinned at the moment */
> > > > + if (evt[1] & EVTQ_1_S2)
> > > > + return -EFAULT;
> > > > +
> > > > + master = arm_smmu_find_master(smmu, sid);
> > > > + if (!master)
> > > > + return -EINVAL;
> > > > +
> > > > + if (evt[1] & EVTQ_1_READ)
> > > > + perm |= IOMMU_FAULT_PERM_READ;
> > > > + else
> > > > + perm |= IOMMU_FAULT_PERM_WRITE;
> > > > +
> > > > + if (evt[1] & EVTQ_1_EXEC)
> > > > + perm |= IOMMU_FAULT_PERM_EXEC;
> > > > +
> > > > + if (evt[1] & EVTQ_1_PRIV)
> > > > + perm |= IOMMU_FAULT_PERM_PRIV;
> > > > +
> > > > + if (evt[1] & EVTQ_1_STALL) {
> > > > + flt->type = IOMMU_FAULT_PAGE_REQ;
> > > > + flt->prm = (struct iommu_fault_page_request) {
> > > > + .flags = IOMMU_FAULT_PAGE_REQUEST_LAST_PAGE,
> > > > + .pasid = FIELD_GET(EVTQ_0_SSID, evt[0]),
> > > > + .grpid = FIELD_GET(EVTQ_1_STAG, evt[1]),
> > > > + .perm = perm,
> > > > + .addr = FIELD_GET(EVTQ_2_ADDR, evt[2]),
> > > > + };
> > > > +
> > >
> > > > + if (ssid_valid)
> > > > + flt->prm.flags |=
> > IOMMU_FAULT_PAGE_REQUEST_PASID_VALID;
> > >
> > > Do we need to set this for STALL mode only support? I had an issue
> > > with this being set on a vSVA POC based on our D06 zip device(which is
> > > a "fake " pci dev that supports STALL mode but no PRI). The issue is,
> > > CMDQ_OP_RESUME doesn't have any ssid or SSV params and works on sid
> > and stag only.
> >
> > I don't understand the problem, arm_smmu_page_response() doesn't set SSID
> > or SSV when sending a CMDQ_OP_RESUME. Could you detail the flow of a stall
> > event and RESUME command in your prototype? Are you getting issues with
> > the host driver or the guest driver?
>
> The issue is on the host side iommu_page_response(). The flow is something like
> below.
>
> Stall: Host:-
>
> arm_smmu_handle_evt()
> iommu_report_device_fault()
> vfio_pci_iommu_dev_fault_handler()
>
> Stall: Qemu:-
>
> vfio_dma_fault_notifier_handler()
> inject_faults()
> smmuv3_inject_faults()
>
> Stall: Guest:-
>
> arm_smmu_handle_evt()
> iommu_report_device_fault()
> iommu_queue_iopf
> ...
> iopf_handle_group()
> iopf_handle_single()
> handle_mm_fault()
> iopf_complete()
> iommu_page_response()
> arm_smmu_page_response()
> arm_smmu_cmdq_issue_cmd(CMDQ_OP_RESUME)
>
> Resume: Qemu:-
>
> smmuv3_cmdq_consume(SMMU_CMD_RESUME)
> smmuv3_notify_page_resp()
> vfio:ioctl(page_response) --> struct iommu_page_response is filled
> with only version, grpid and code.
>
> Resume: Host:-
> ioctl(page_response)
> iommu_page_response() --> fails as the pending req has PASID_VALID flag
> set and it checks for a match.
I believe the fix needs to be here. It's also wrong for PRI since not all
PCIe endpoint require a PASID in the page response. Could you try the
attached patch?
Thanks,
Jean
> arm_smmu_page_response()
>
> Hope the above is clear.
>
> > We do need to forward the SSV flag all the way to the guest driver, so the guest
> > can find the faulting address space using the SSID. Once the guest handled the
> > fault, then we don't send the SSID back to the host as part of the RESUME
> > command.
>
> True, the guest requires SSV flag to handle the page fault. But, as shown in the
> flow above, the issue is on the host side iommu_page_response() where it
> searches for a matching pending req based on pasid. Not sure we can bypass
> that and call arm_smmu_page_response() directly but then have to delete the
> pending req from the list as well.
>
> Please let me know if there is a better way to handle the host side page
> response.
>
> Thanks,
> Shameer
>
> > Thanks,
> > Jean
> >
> > > Hence, it is difficult for
> > > Qemu SMMUv3 to populate this fields while preparing a page response. I
> > > can see that this flag is being checked in iopf_handle_single() (patch
> > > 04/24) as well. For POC, I used a temp fix[1] to work around this. Please let
> > me know your thoughts.
> > >
> > > Thanks,
> > > Shameer
> > >
> > > 1.
> > > https://github.com/hisilicon/kernel-dev/commit/99ff96146e924055f38d97a
> > > 5897e4becfa378d15
> > >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
[-- Attachment #2: 0001-iommu-Allow-page-responses-without-PASID.patch --]
[-- Type: text/plain, Size: 1682 bytes --]
From 9baf5b9894d4e4be05e665d80fd0ebac8b621aa4 Mon Sep 17 00:00:00 2001
From: Jean-Philippe Brucker <jean-philippe@linaro.org>
Date: Tue, 2 Jun 2020 13:13:27 +0200
Subject: [PATCH] iommu: Allow page responses without PASID
Some PCIe devices do not expect a PASID value in PRI Page Responses. If
the "PRG Response PASID Required" bit in the PRI capability is zero,
then the OS should not set the PASID field. Similarly on Arm SMMU,
responses to stall events do not have a PASID.
Currently iommu_page_response() checks that the PASID in the page
response corresponds to the one in the page request without first
checking the "PASID valid" bit. A page response coming from a guest OS
does not necessarily have a PASID, if the passed-through device does not
require one.
Allow page responses without PASID. The page request corresponding to a
page response is identified by device and by Page Response Group Index
(or stall tag).
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
drivers/iommu/iommu.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index e61a9fc65b7e4..e481fdfafb77c 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1296,7 +1296,8 @@ int iommu_page_response(struct device *dev,
*/
list_for_each_entry(evt, ¶m->fault_param->faults, list) {
prm = &evt->fault.prm;
- pasid_valid = prm->flags & IOMMU_FAULT_PAGE_REQUEST_PASID_VALID;
+ pasid_valid = prm->flags & IOMMU_FAULT_PAGE_REQUEST_PASID_VALID
+ && msg->flags & IOMMU_PAGE_RESP_PASID_VALID;
if ((pasid_valid && prm->pasid != msg->pasid) ||
prm->grpid != msg->grpid)
--
2.26.2
^ permalink raw reply related
* [PATCHv2 2/3] dt-binding: phy: ti,omap-usb2: Add quirk to disable charger detection
From: Roger Quadros @ 2020-06-02 11:46 UTC (permalink / raw)
To: kishon, robh+dt; +Cc: linux-kernel, devicetree, Roger Quadros
In-Reply-To: <20200602114606.32045-1-rogerq@ti.com>
Add "ti,dis-chg-det-quirk" property to disable the USB2_PHY Charger Detect
logic.
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
Documentation/devicetree/bindings/phy/ti,omap-usb2.yaml | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/Documentation/devicetree/bindings/phy/ti,omap-usb2.yaml b/Documentation/devicetree/bindings/phy/ti,omap-usb2.yaml
index 2bbea8d2bcb1..5e8c7a98de1e 100644
--- a/Documentation/devicetree/bindings/phy/ti,omap-usb2.yaml
+++ b/Documentation/devicetree/bindings/phy/ti,omap-usb2.yaml
@@ -50,6 +50,11 @@ properties:
(deprecated) phandle of the control module used by PHY driver
to power on the PHY. Use syscon-phy-power instead.
+ ti,dis-chg-det-quirk:
+ description:
+ if present, driver will disable charger detection logic.
+ type: boolean
+
required:
- compatible
- reg
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply related
* [PATCHv2 1/3] dt-binding: phy: convert ti,omap-usb2 to YAML
From: Roger Quadros @ 2020-06-02 11:46 UTC (permalink / raw)
To: kishon, robh+dt; +Cc: linux-kernel, devicetree, Roger Quadros
In-Reply-To: <20200602114606.32045-1-rogerq@ti.com>
Move ti,omap-usb2 to its own YAML schema.
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
.../devicetree/bindings/phy/ti,omap-usb2.yaml | 69 +++++++++++++++++++
.../devicetree/bindings/phy/ti-phy.txt | 37 ----------
2 files changed, 69 insertions(+), 37 deletions(-)
create mode 100644 Documentation/devicetree/bindings/phy/ti,omap-usb2.yaml
diff --git a/Documentation/devicetree/bindings/phy/ti,omap-usb2.yaml b/Documentation/devicetree/bindings/phy/ti,omap-usb2.yaml
new file mode 100644
index 000000000000..2bbea8d2bcb1
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/ti,omap-usb2.yaml
@@ -0,0 +1,69 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/ti,omap-usb2.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: OMAP USB2 PHY
+
+maintainers:
+ - Kishon Vijay Abraham I <kishon@ti.com>
+ - Roger Quadros <rogerq@ti.com>
+
+properties:
+ compatible:
+ items:
+ - enum:
+ - "ti,dra7x-usb2"
+ - "ti,dra7x-usb2-phy2"
+ - "ti,am654-usb2"
+ - enum:
+ - "ti,omap-usb2"
+
+ reg:
+ maxItems: 1
+
+ '#phy-cells':
+ const: 0
+
+ clocks:
+ minItems: 1
+ items:
+ - description: wakeup clock
+ - description: reference clock
+
+ clock-names:
+ minItems: 1
+ items:
+ - const: wkupclk
+ - const: refclk
+
+ syscon-phy-power:
+ $ref: /schemas/types.yaml#definitions/phandle-array
+ description:
+ phandle/offset pair. Phandle to the system control module and
+ register offset to power on/off the PHY.
+
+ ctrl-module:
+ $ref: /schemas/types.yaml#definitions/phandle
+ description:
+ (deprecated) phandle of the control module used by PHY driver
+ to power on the PHY. Use syscon-phy-power instead.
+
+required:
+ - compatible
+ - reg
+ - '#phy-cells'
+ - clocks
+ - clock-names
+
+examples:
+ - |
+ usb0_phy: phy@4100000 {
+ compatible = "ti,am654-usb2", "ti,omap-usb2";
+ reg = <0x0 0x4100000 0x0 0x54>;
+ syscon-phy-power = <&scm_conf 0x4000>;
+ clocks = <&k3_clks 151 0>, <&k3_clks 151 1>;
+ clock-names = "wkupclk", "refclk";
+ #phy-cells = <0>;
+ };
diff --git a/Documentation/devicetree/bindings/phy/ti-phy.txt b/Documentation/devicetree/bindings/phy/ti-phy.txt
index 8f93c3b694a7..60c9d0ac75e6 100644
--- a/Documentation/devicetree/bindings/phy/ti-phy.txt
+++ b/Documentation/devicetree/bindings/phy/ti-phy.txt
@@ -27,43 +27,6 @@ omap_control_usb: omap-control-usb@4a002300 {
reg-names = "otghs_control";
};
-OMAP USB2 PHY
-
-Required properties:
- - compatible: Should be "ti,omap-usb2"
- Should be "ti,dra7x-usb2" for the 1st instance of USB2 PHY on
- DRA7x
- Should be "ti,dra7x-usb2-phy2" for the 2nd instance of USB2 PHY
- in DRA7x
- Should be "ti,am654-usb2" for the USB2 PHYs on AM654.
- - reg : Address and length of the register set for the device.
- - #phy-cells: determine the number of cells that should be given in the
- phandle while referencing this phy.
- - clocks: a list of phandles and clock-specifier pairs, one for each entry in
- clock-names.
- - clock-names: should include:
- * "wkupclk" - wakeup clock.
- * "refclk" - reference clock (optional).
-
-Deprecated properties:
- - ctrl-module : phandle of the control module used by PHY driver to power on
- the PHY.
-
-Recommended properies:
-- syscon-phy-power : phandle/offset pair. Phandle to the system control
- module and the register offset to power on/off the PHY.
-
-This is usually a subnode of ocp2scp to which it is connected.
-
-usb2phy@4a0ad080 {
- compatible = "ti,omap-usb2";
- reg = <0x4a0ad080 0x58>;
- ctrl-module = <&omap_control_usb>;
- #phy-cells = <0>;
- clocks = <&usb_phy_cm_clk32k>, <&usb_otg_ss_refclk960m>;
- clock-names = "wkupclk", "refclk";
-};
-
TI PIPE3 PHY
Required properties:
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply related
* [PATCHv2 3/3] phy: omap-usb2-phy: disable PHY charger detect
From: Roger Quadros @ 2020-06-02 11:46 UTC (permalink / raw)
To: kishon, robh+dt
Cc: linux-kernel, devicetree, Roger Quadros, Bin Liu, Sekhar Nori
In-Reply-To: <20200602114606.32045-1-rogerq@ti.com>
AM654x PG1.0 has a silicon bug that D+ is pulled high after POR, which
could cause enumeration failure with some USB hubs. Disabling the
USB2_PHY Charger Detect function will put D+ into the normal state.
Using property "ti,dis-chg-det-quirk" in the DT usb2-phy node to
enable this workaround for AM654x PG1.0.
This addresses Silicon Errata:
i2075 - "USB2PHY: USB2PHY Charger Detect is Enabled by Default Without VBUS
Presence"
Signed-off-by: Bin Liu <b-liu@ti.com>
Signed-off-by: Sekhar Nori <nsekhar@ti.com>
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
drivers/phy/ti/phy-omap-usb2.c | 35 +++++++++++++++++++++++++++-------
1 file changed, 28 insertions(+), 7 deletions(-)
diff --git a/drivers/phy/ti/phy-omap-usb2.c b/drivers/phy/ti/phy-omap-usb2.c
index cb2dd3230fa7..8ab8b94511d4 100644
--- a/drivers/phy/ti/phy-omap-usb2.c
+++ b/drivers/phy/ti/phy-omap-usb2.c
@@ -26,6 +26,10 @@
#define USB2PHY_ANA_CONFIG1 0x4c
#define USB2PHY_DISCON_BYP_LATCH BIT(31)
+#define USB2PHY_CHRG_DET 0x14
+#define USB2PHY_CHRG_DET_USE_CHG_DET_REG BIT(29)
+#define USB2PHY_CHRG_DET_DIS_CHG_DET BIT(28)
+
/* SoC Specific USB2_OTG register definitions */
#define AM654_USB2_OTG_PD BIT(8)
#define AM654_USB2_VBUS_DET_EN BIT(5)
@@ -43,6 +47,7 @@
#define OMAP_USB2_HAS_START_SRP BIT(0)
#define OMAP_USB2_HAS_SET_VBUS BIT(1)
#define OMAP_USB2_CALIBRATE_FALSE_DISCONNECT BIT(2)
+#define OMAP_USB2_DISABLE_CHRG_DET BIT(3)
struct omap_usb {
struct usb_phy phy;
@@ -236,6 +241,13 @@ static int omap_usb_init(struct phy *x)
omap_usb_writel(phy->phy_base, USB2PHY_ANA_CONFIG1, val);
}
+ if (phy->flags & OMAP_USB2_DISABLE_CHRG_DET) {
+ val = omap_usb_readl(phy->phy_base, USB2PHY_CHRG_DET);
+ val |= USB2PHY_CHRG_DET_USE_CHG_DET_REG |
+ USB2PHY_CHRG_DET_DIS_CHG_DET;
+ omap_usb_writel(phy->phy_base, USB2PHY_CHRG_DET, val);
+ }
+
return 0;
}
@@ -366,14 +378,12 @@ static int omap_usb2_probe(struct platform_device *pdev)
phy->mask = phy_data->mask;
phy->power_on = phy_data->power_on;
phy->power_off = phy_data->power_off;
+ phy->flags = phy_data->flags;
- if (phy_data->flags & OMAP_USB2_CALIBRATE_FALSE_DISCONNECT) {
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- phy->phy_base = devm_ioremap_resource(&pdev->dev, res);
- if (IS_ERR(phy->phy_base))
- return PTR_ERR(phy->phy_base);
- phy->flags |= OMAP_USB2_CALIBRATE_FALSE_DISCONNECT;
- }
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ phy->phy_base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(phy->phy_base))
+ return PTR_ERR(phy->phy_base);
phy->syscon_phy_power = syscon_regmap_lookup_by_phandle(node,
"syscon-phy-power");
@@ -405,6 +415,17 @@ static int omap_usb2_probe(struct platform_device *pdev)
}
}
+ /*
+ * Errata i2075: USB2PHY: USB2PHY Charger Detect is Enabled by
+ * Default Without VBUS Presence.
+ *
+ * AM654x SR1.0 has a silicon bug due to which D+ is pulled high after
+ * POR, which could cause enumeration failure with some USB hubs.
+ * Disabling the USB2_PHY Charger Detect function will put D+
+ * into the normal state.
+ */
+ if (of_property_read_bool(node, "ti,dis-chg-det-quirk"))
+ phy->flags |= OMAP_USB2_DISABLE_CHRG_DET;
phy->wkupclk = devm_clk_get(phy->dev, "wkupclk");
if (IS_ERR(phy->wkupclk)) {
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply related
* [PATCHv2 0/3] phy: omap-usb2: add quirk to disable charger detection
From: Roger Quadros @ 2020-06-02 11:46 UTC (permalink / raw)
To: kishon, robh+dt; +Cc: linux-kernel, devicetree, Roger Quadros
Hi,
- convert DT binding to YAML
- add DT property to disable charger detection
(Errata i2075 for AM65 SR1.0)
Changelog:
v2
- Address Rob's comments on YAML schema.
cheers,
-roger
Bin Liu (1):
dts: am65: add ti,dis-chg-det-quirk flag to usb phy nodes
Roger Quadros (3):
dt-binding: phy: convert ti,omap-usb2 to YAML
dt-binding: phy: ti,omap-usb2: Add quirk to disable charger detection
phy: omap-usb2-phy: disable PHY charger detect
.../devicetree/bindings/phy/ti,omap-usb2.yaml | 74 +++++++++++++++++++
.../devicetree/bindings/phy/ti-phy.txt | 37 ----------
drivers/phy/ti/phy-omap-usb2.c | 35 +++++++--
3 files changed, 102 insertions(+), 44 deletions(-)
create mode 100644 Documentation/devicetree/bindings/phy/ti,omap-usb2.yaml
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply
* Re: [PATCH v4 4/5] regulator: qcom: Add labibb driver
From: Mark Brown @ 2020-06-02 11:32 UTC (permalink / raw)
To: Sumit Semwal
Cc: agross, bjorn.andersson, lgirdwood, robh+dt, nishakumari,
linux-arm-msm, linux-kernel, devicetree, kgunda, rnayak
In-Reply-To: <20200602100924.26256-5-sumit.semwal@linaro.org>
[-- Attachment #1: Type: text/plain, Size: 1497 bytes --]
On Tue, Jun 02, 2020 at 03:39:23PM +0530, Sumit Semwal wrote:
> +static int qcom_labibb_regulator_is_enabled(struct regulator_dev *rdev)
> +{
> + int ret;
> + unsigned int val;
> + struct labibb_regulator *reg = rdev_get_drvdata(rdev);
> +
> + ret = regmap_read(reg->regmap, reg->base + REG_LABIBB_STATUS1, &val);
> + if (ret < 0) {
> + dev_err(reg->dev, "Read register failed ret = %d\n", ret);
> + return ret;
> + }
> + return !!(val & LABIBB_STATUS1_VREG_OK_BIT);
> +}
This should be a get_status() callback...
> +static int qcom_labibb_regulator_enable(struct regulator_dev *rdev)
> +{
> + return regulator_enable_regmap(rdev);
> +}
> +
> +static int qcom_labibb_regulator_disable(struct regulator_dev *rdev)
> +{
> + return regulator_disable_regmap(rdev);
> +}
...is_enabled() should just be regulator_is_enabled_regmap() and these
functions should just be removed entirely, you can use the regmap
operations directly as the ops without the wrapper.
> + match = of_match_device(qcom_labibb_match, &pdev->dev);
> + if (!match)
> + return -ENODEV;
> +
> + for (reg_data = match->data; reg_data->name; reg_data++) {
> + child = of_get_child_by_name(pdev->dev.of_node, reg_data->name);
> +
> + if (WARN_ON(child == NULL))
> + return -EINVAL;
This feels like the DT bindings are confused - why do we need to search
like this?
> + dev_info(dev, "Registering %s regulator\n", child->full_name);
This is noise, remove it. The regulator framework will announce new
regulators anyway.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v4 1/5] regulator: Allow regulators to verify enabled during enable()
From: Mark Brown @ 2020-06-02 11:24 UTC (permalink / raw)
To: Sumit Semwal
Cc: agross, bjorn.andersson, lgirdwood, robh+dt, nishakumari,
linux-arm-msm, linux-kernel, devicetree, kgunda, rnayak
In-Reply-To: <20200602100924.26256-2-sumit.semwal@linaro.org>
[-- Attachment #1: Type: text/plain, Size: 409 bytes --]
On Tue, Jun 02, 2020 at 03:39:20PM +0530, Sumit Semwal wrote:
> +
> + if (time_remaining <= 0) {
> + rdev_err(rdev, "Enabled check failed.\n");
> + return -ETIMEDOUT;
s/failed/timed out/
> + * @poll_enabled_time: Maximum time (in uS) to poll if the regulator is
> + * actually enabled, after enable() call
> + *
This comment needs updating to reflect the new implementation.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v2 00/91] drm/vc4: Support BCM2711 Display Pipelin
From: Maxime Ripard @ 2020-06-02 11:04 UTC (permalink / raw)
To: Jian-Hong Pan
Cc: Daniel Drake, Nicolas Saenz Julienne, Eric Anholt, dri-devel,
linux-rpi-kernel, bcm-kernel-feedback-list, linux-arm-kernel,
Linux Kernel, devicetree, linux-clk, linux-i2c,
Linux Upstreaming Team
In-Reply-To: <CAPpJ_ec1KRwUrHGVVZrReaDPz4iga-Nvj5H652-tTKmkXL=Xmg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1807 bytes --]
Hi,
On Mon, Jun 01, 2020 at 03:58:26PM +0800, Jian-Hong Pan wrote:
> Maxime Ripard <maxime@cerno.tech> 於 2020年5月28日 週四 下午3:30寫道：
> >
> > Hi Daniel,
> >
> > On Wed, May 27, 2020 at 05:15:12PM +0800, Daniel Drake wrote:
> > > On Wed, May 27, 2020 at 5:13 PM Maxime Ripard <maxime@cerno.tech> wrote:
> > > > I'm about to send a v3 today or tomorrow, I can Cc you (and Jian-Hong) if you
> > > > want.
> > >
> > > That would be great, although given the potentially inconsistent
> > > results we've been seeing so far it would be great if you could
> > > additionally push a git branch somewhere.
> > > That way we can have higher confidence that we are applying exactly
> > > the same patches to the same base etc.
> >
> > So I sent a new iteration yesterday, and of course forgot to cc you... Sorry for
> > that.
> >
> > I've pushed my current branch here:
> > https://git.kernel.org/pub/scm/linux/kernel/git/mripard/linux.git/log/?h=rpi4-kms
>
> Thanks to Maxime!
>
> I have tried your repository on branch rpi4-kms. The DRM VC4 is used!
> But got some issues:
> 1. Some weird error message in dmesg. Not sure it is related, or not
> [ 5.219321] [drm:vc5_hdmi_init_resources] *ERROR* Failed to get
> HDMI state machine clock
> https://gist.github.com/starnight/3f317dca121065a361cf08e91225e389
That's a deferred probing. The first time the HDMI driver is being
probed, the firmware clock driver has not been probed yet. It's making
another attempt later on, which succeeds.
> 2. The screen flashes suddenly sometimes.
>
> 3. The higher resolutions, like 1920x1080 ... are lost after hot
> re-plug HDMI cable (HDMI0)
I'm not sure on how to exactly reproduce those issues (or what they are)
though, can you expand on this?
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [RFC v1 2/3] drivers: nvmem: Add driver for QTI qfprom-efuse support
From: Srinivas Kandagatla @ 2020-06-02 10:56 UTC (permalink / raw)
To: Doug Anderson
Cc: Ravi Kumar Bokka (Temp), Rob Herring, LKML,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Rajendra Nayak, Sai Prakash Ranjan, dhavalp, mturney, sparate,
c_rbokka, mkurumel
In-Reply-To: <CAD=FV=VcfYVQMmWvSkBG0VNBcqt5a3Y_b4eNBgDenPB5wUNGaw@mail.gmail.com>
On 01/06/2020 19:08, Doug Anderson wrote:
>> Am not 100% sure if "qcom,fuse-blow-frequency" is something integration
>> specific or SoC Specific, My idea was that this will give more
>> flexibility in future. As adding new SoC Support does not need driver
>> changes.
>>
>> Having said that, Am okay either way!
> Yeah, it's always a balance. I guess the question is: why do we think
> driver changes are worse than dts changes? The value still needs to
> be somewhere and having it in the driver isn't a terrible place.
>
TBH, its an overkill if we are using same IP version across multiple SoCs.
>
>> Incase we go compatible way, I would like to see compatible strings
>> having proper IP versions to have ip version rather than SoC names.
>>
>> Having SoC names in compatible string means both driver and bindings
>> need update for every new SoC which can be overhead very soon!
> Almost certainly the compatible strings should have SoC names in them.
> Yes it means a binding update every time a new SoC comes up but that
> is just how device tree works. Presumably there's enough chatter on
> this that Rob H has totally tuned it out at this point in time, but
> there are many other instances of this.
>
> NOTE: just because we have the SoC name in the compatible string
> _doesn't_ mean that the driver has to change. You already said that
> the IP version can be detected earlier in this thread, right? You
> said:
>
> I found out that there is a version register at offset of 0x6000 which
> can give MAJOR, MINOR and STEP numbers.
>
> So how about this:
>
> a) Compatible contains "SoC" version and the generic "qcom,qfrom", so:
>
> compatible = "qcom,sdm845-qfprom", "qcom,qfrom"
>
> b) Bindings will need to be updated for every new SoC, but that's
> normal and should be a trivial patch to just add a new SoC to the
> list.
>
> c) If the driver can be made to make its decisions about frequencies /
> timings completely by MAJOR/MINOR/STEP numbers then it can use those
> in its decision and it will never need to use the SoC-specific
> compatible string. The SoC-specific compatible string will only be
> present as a fallback "oops we have to workaround a bug that we didn't
> know about".
This makes more sense to me, I would still stay with MAJOR/MINOR/STEP
numbers mostly unless we are dealing with some corner cases.
thanks,
srini
>
>
>> Rob can help review once we have v2 bindings out!
> Sounds good. If you're still not convinced by my arguments we can see
> if we can get Rob to clarify once we have a v2.:-)
>
>
^ permalink raw reply
* [PATCH] ARM: dts: imx6sll: Make ssi node name same as other platforms
From: Shengjiu Wang @ 2020-06-02 10:44 UTC (permalink / raw)
To: robh+dt, shawnguo, s.hauer, kernel, festevam, linux-imx,
devicetree, linux-arm-kernel, linux-kernel
In imx6sll.dtsi, the ssi node name is different with other
platforms (imx6qdl, imx6sl, imx6sx), but the
sound/soc/fsl/fsl-asoc-card.c machine driver needs to check
ssi node name for audmux configuration, then different ssi
node name causes issue on imx6sll platform.
So we change ssi node name to make all platforms have same
name.
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
arch/arm/boot/dts/imx6sll.dtsi | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm/boot/dts/imx6sll.dtsi b/arch/arm/boot/dts/imx6sll.dtsi
index edd3abb9a9f1..51e59e16f6c9 100644
--- a/arch/arm/boot/dts/imx6sll.dtsi
+++ b/arch/arm/boot/dts/imx6sll.dtsi
@@ -271,7 +271,7 @@
status = "disabled";
};
- ssi1: ssi-controller@2028000 {
+ ssi1: ssi@2028000 {
compatible = "fsl,imx6sl-ssi", "fsl,imx51-ssi";
reg = <0x02028000 0x4000>;
interrupts = <GIC_SPI 46 IRQ_TYPE_LEVEL_HIGH>;
@@ -284,7 +284,7 @@
status = "disabled";
};
- ssi2: ssi-controller@202c000 {
+ ssi2: ssi@202c000 {
compatible = "fsl,imx6sl-ssi", "fsl,imx51-ssi";
reg = <0x0202c000 0x4000>;
interrupts = <GIC_SPI 47 IRQ_TYPE_LEVEL_HIGH>;
@@ -297,7 +297,7 @@
status = "disabled";
};
- ssi3: ssi-controller@2030000 {
+ ssi3: ssi@2030000 {
compatible = "fsl,imx6sl-ssi", "fsl,imx51-ssi";
reg = <0x02030000 0x4000>;
interrupts = <GIC_SPI 48 IRQ_TYPE_LEVEL_HIGH>;
--
2.21.0
^ permalink raw reply related
* [PATCH V3 2/3] mmc: sdhci: Allow platform controlled voltage switching
From: Veerabhadrarao Badiganti @ 2020-06-02 10:47 UTC (permalink / raw)
To: adrian.hunter, ulf.hansson, bjorn.andersson, robh+dt
Cc: linux-mmc, linux-kernel, linux-arm-msm, devicetree,
Vijay Viswanath, Veerabhadrarao Badiganti
In-Reply-To: <1591094883-11674-1-git-send-email-vbadigan@codeaurora.org>
From: Vijay Viswanath <vviswana@codeaurora.org>
If vendor platform drivers are controlling whole logic of voltage
switching, then sdhci driver no need control vqmmc regulator.
So skip enabling/disable vqmmc from SDHC driver.
Signed-off-by: Vijay Viswanath <vviswana@codeaurora.org>
Signed-off-by: Veerabhadrarao Badiganti <vbadigan@codeaurora.org>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
---
drivers/mmc/host/sdhci.c | 32 +++++++++++++++++++-------------
drivers/mmc/host/sdhci.h | 1 +
2 files changed, 20 insertions(+), 13 deletions(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 37b1158c1c0c..e6275c2202b0 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -4105,6 +4105,7 @@ int sdhci_setup_host(struct sdhci_host *host)
unsigned int override_timeout_clk;
u32 max_clk;
int ret;
+ bool enable_vqmmc = false;
WARN_ON(host == NULL);
if (host == NULL)
@@ -4118,9 +4119,12 @@ int sdhci_setup_host(struct sdhci_host *host)
* the host can take the appropriate action if regulators are not
* available.
*/
- ret = mmc_regulator_get_supply(mmc);
- if (ret)
- return ret;
+ if (!mmc->supply.vqmmc) {
+ ret = mmc_regulator_get_supply(mmc);
+ if (ret)
+ return ret;
+ enable_vqmmc = true;
+ }
DBG("Version: 0x%08x | Present: 0x%08x\n",
sdhci_readw(host, SDHCI_HOST_VERSION),
@@ -4377,7 +4381,15 @@ int sdhci_setup_host(struct sdhci_host *host)
mmc->caps |= MMC_CAP_NEEDS_POLL;
if (!IS_ERR(mmc->supply.vqmmc)) {
- ret = regulator_enable(mmc->supply.vqmmc);
+ if (enable_vqmmc) {
+ ret = regulator_enable(mmc->supply.vqmmc);
+ if (ret) {
+ pr_warn("%s: Failed to enable vqmmc regulator: %d\n",
+ mmc_hostname(mmc), ret);
+ mmc->supply.vqmmc = ERR_PTR(-EINVAL);
+ }
+ host->sdhci_core_to_disable_vqmmc = !ret;
+ }
/* If vqmmc provides no 1.8V signalling, then there's no UHS */
if (!regulator_is_supported_voltage(mmc->supply.vqmmc, 1700000,
@@ -4390,12 +4402,6 @@ int sdhci_setup_host(struct sdhci_host *host)
if (!regulator_is_supported_voltage(mmc->supply.vqmmc, 2700000,
3600000))
host->flags &= ~SDHCI_SIGNALING_330;
-
- if (ret) {
- pr_warn("%s: Failed to enable vqmmc regulator: %d\n",
- mmc_hostname(mmc), ret);
- mmc->supply.vqmmc = ERR_PTR(-EINVAL);
- }
}
if (host->quirks2 & SDHCI_QUIRK2_NO_1_8_V) {
@@ -4626,7 +4632,7 @@ int sdhci_setup_host(struct sdhci_host *host)
return 0;
unreg:
- if (!IS_ERR(mmc->supply.vqmmc))
+ if (host->sdhci_core_to_disable_vqmmc)
regulator_disable(mmc->supply.vqmmc);
undma:
if (host->align_buffer)
@@ -4644,7 +4650,7 @@ void sdhci_cleanup_host(struct sdhci_host *host)
{
struct mmc_host *mmc = host->mmc;
- if (!IS_ERR(mmc->supply.vqmmc))
+ if (host->sdhci_core_to_disable_vqmmc)
regulator_disable(mmc->supply.vqmmc);
if (host->align_buffer)
@@ -4787,7 +4793,7 @@ void sdhci_remove_host(struct sdhci_host *host, int dead)
destroy_workqueue(host->complete_wq);
- if (!IS_ERR(mmc->supply.vqmmc))
+ if (host->sdhci_core_to_disable_vqmmc)
regulator_disable(mmc->supply.vqmmc);
if (host->align_buffer)
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 0008bbd27127..0770c036e2ff 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -567,6 +567,7 @@ struct sdhci_host {
u32 caps1; /* CAPABILITY_1 */
bool read_caps; /* Capability flags have been read */
+ bool sdhci_core_to_disable_vqmmc; /* sdhci core can disable vqmmc */
unsigned int ocr_avail_sdio; /* OCR bit masks */
unsigned int ocr_avail_sd;
unsigned int ocr_avail_mmc;
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
^ permalink raw reply related
* [PATCH V3 3/3] mmc: sdhci-msm: Use internal voltage control
From: Veerabhadrarao Badiganti @ 2020-06-02 10:47 UTC (permalink / raw)
To: adrian.hunter, ulf.hansson, bjorn.andersson, robh+dt
Cc: linux-mmc, linux-kernel, linux-arm-msm, devicetree,
Veerabhadrarao Badiganti, Asutosh Das, Vijay Viswanath,
Andy Gross
In-Reply-To: <1591094883-11674-1-git-send-email-vbadigan@codeaurora.org>
On qcom SD host controllers voltage switching be done after the HW
is ready for it. The HW informs its readiness through power irq.
The voltage switching should happen only then.
Use the internal voltage switching and then control the voltage
switching using power irq.
Set the regulator load as well so that regulator can be configured
in LPM mode when in is not being used.
Co-developed-by: Asutosh Das <asutoshd@codeaurora.org>
Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
Co-developed-by: Vijay Viswanath <vviswana@codeaurora.org>
Signed-off-by: Vijay Viswanath <vviswana@codeaurora.org>
Co-developed-by: Veerabhadrarao Badiganti <vbadigan@codeaurora.org>
Signed-off-by: Veerabhadrarao Badiganti <vbadigan@codeaurora.org>
---
drivers/mmc/host/sdhci-msm.c | 235 +++++++++++++++++++++++++++++++++++++++++--
1 file changed, 226 insertions(+), 9 deletions(-)
diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 95cd9735e9a3..20ef90fc7dd7 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -36,7 +36,9 @@
#define CORE_PWRCTL_IO_LOW BIT(2)
#define CORE_PWRCTL_IO_HIGH BIT(3)
#define CORE_PWRCTL_BUS_SUCCESS BIT(0)
+#define CORE_PWRCTL_BUS_FAIL BIT(1)
#define CORE_PWRCTL_IO_SUCCESS BIT(2)
+#define CORE_PWRCTL_IO_FAIL BIT(3)
#define REQ_BUS_OFF BIT(0)
#define REQ_BUS_ON BIT(1)
#define REQ_IO_LOW BIT(2)
@@ -277,6 +279,8 @@ struct sdhci_msm_host {
bool uses_tassadar_dll;
u32 dll_config;
u32 ddr_config;
+ u32 vqmmc_load;
+ bool vqmmc_enabled;
};
static const struct sdhci_msm_offset *sdhci_priv_msm_offset(struct sdhci_host *host)
@@ -1339,6 +1343,91 @@ static void sdhci_msm_set_uhs_signaling(struct sdhci_host *host,
sdhci_msm_hs400(host, &mmc->ios);
}
+static int sdhci_msm_set_vmmc(struct mmc_host *mmc)
+{
+ if (IS_ERR(mmc->supply.vmmc))
+ return 0;
+
+ return mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, mmc->ios.vdd);
+}
+
+static int msm_toggle_vqmmc(struct sdhci_msm_host *msm_host,
+ struct mmc_host *mmc, bool level)
+{
+ int ret;
+ struct mmc_ios ios;
+
+ if (msm_host->vqmmc_enabled == level)
+ return 0;
+
+ if (level) {
+ /* Set the IO voltage regulator to default voltage level */
+ if (msm_host->caps_0 & CORE_3_0V_SUPPORT)
+ ios.signal_voltage = MMC_SIGNAL_VOLTAGE_330;
+ else if (msm_host->caps_0 & CORE_1_8V_SUPPORT)
+ ios.signal_voltage = MMC_SIGNAL_VOLTAGE_180;
+
+ if (msm_host->caps_0 & CORE_VOLT_SUPPORT) {
+ ret = mmc_regulator_set_vqmmc(mmc, &ios);
+ if (ret < 0) {
+ dev_err(mmc_dev(mmc), "%s: vqmmc set volgate failed: %d\n",
+ mmc_hostname(mmc), ret);
+ goto out;
+ }
+ }
+ ret = regulator_enable(mmc->supply.vqmmc);
+ } else {
+ ret = regulator_disable(mmc->supply.vqmmc);
+ }
+
+ if (ret)
+ dev_err(mmc_dev(mmc), "%s: vqmm %sable failed: %d\n",
+ mmc_hostname(mmc), level ? "en":"dis", ret);
+ else
+ msm_host->vqmmc_enabled = level;
+out:
+ return ret;
+}
+
+static int msm_config_vqmmc_mode(struct sdhci_msm_host *msm_host,
+ struct mmc_host *mmc, bool hpm)
+{
+ int load, ret;
+
+ if (!msm_host->vqmmc_load)
+ return 0;
+
+ load = hpm ? msm_host->vqmmc_load : 0;
+ ret = regulator_set_load(mmc->supply.vqmmc, load);
+ if (ret)
+ dev_err(mmc_dev(mmc), "%s: vqmmc set load failed: %d\n",
+ mmc_hostname(mmc), ret);
+ return ret;
+}
+
+static int sdhci_msm_set_vqmmc(struct sdhci_msm_host *msm_host,
+ struct mmc_host *mmc, bool level)
+{
+ int ret;
+ bool always_on;
+
+ if (IS_ERR(mmc->supply.vqmmc) ||
+ (mmc->ios.power_mode == MMC_POWER_UNDEFINED))
+ return 0;
+ /*
+ * For eMMC don't turn off Vqmmc, Instead just configure it in LPM
+ * and HPM modes by setting the right amonut of load.
+ */
+ always_on = mmc->card && mmc_card_mmc(mmc->card);
+
+ if (always_on)
+ ret = msm_config_vqmmc_mode(msm_host, mmc, level);
+ else
+ ret = msm_toggle_vqmmc(msm_host, mmc, level);
+
+ return ret;
+}
+
static inline void sdhci_msm_init_pwr_irq_wait(struct sdhci_msm_host *msm_host)
{
init_waitqueue_head(&msm_host->pwr_irq_wait);
@@ -1442,8 +1531,9 @@ static void sdhci_msm_handle_pwr_irq(struct sdhci_host *host, int irq)
{
struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
+ struct mmc_host *mmc = host->mmc;
u32 irq_status, irq_ack = 0;
- int retry = 10;
+ int retry = 10, ret;
u32 pwr_state = 0, io_level = 0;
u32 config;
const struct sdhci_msm_offset *msm_offset = msm_host->offset;
@@ -1481,21 +1571,42 @@ static void sdhci_msm_handle_pwr_irq(struct sdhci_host *host, int irq)
if (irq_status & CORE_PWRCTL_BUS_ON) {
pwr_state = REQ_BUS_ON;
io_level = REQ_IO_HIGH;
- irq_ack |= CORE_PWRCTL_BUS_SUCCESS;
}
if (irq_status & CORE_PWRCTL_BUS_OFF) {
pwr_state = REQ_BUS_OFF;
io_level = REQ_IO_LOW;
- irq_ack |= CORE_PWRCTL_BUS_SUCCESS;
}
+
+ if (pwr_state) {
+ ret = sdhci_msm_set_vmmc(mmc);
+ if (!ret)
+ ret = sdhci_msm_set_vqmmc(msm_host, mmc,
+ pwr_state & REQ_BUS_ON);
+ if (!ret)
+ irq_ack |= CORE_PWRCTL_BUS_SUCCESS;
+ else
+ irq_ack |= CORE_PWRCTL_BUS_FAIL;
+ }
+
/* Handle IO LOW/HIGH */
- if (irq_status & CORE_PWRCTL_IO_LOW) {
+ if (irq_status & CORE_PWRCTL_IO_LOW)
io_level = REQ_IO_LOW;
- irq_ack |= CORE_PWRCTL_IO_SUCCESS;
- }
- if (irq_status & CORE_PWRCTL_IO_HIGH) {
+
+ if (irq_status & CORE_PWRCTL_IO_HIGH)
io_level = REQ_IO_HIGH;
+
+ if (io_level)
irq_ack |= CORE_PWRCTL_IO_SUCCESS;
+
+ if (io_level && !IS_ERR(mmc->supply.vqmmc) && !pwr_state) {
+ ret = mmc_regulator_set_vqmmc(mmc, &mmc->ios);
+ if (ret < 0) {
+ dev_err(mmc_dev(mmc), "%s: IO_level setting failed(%d). signal_voltage: %d, vdd: %d irq_status: 0x%08x\n",
+ mmc_hostname(mmc), ret,
+ mmc->ios.signal_voltage, mmc->ios.vdd,
+ irq_status);
+ irq_ack |= CORE_PWRCTL_IO_FAIL;
+ }
}
/*
@@ -1544,7 +1655,7 @@ static void sdhci_msm_handle_pwr_irq(struct sdhci_host *host, int irq)
if (io_level)
msm_host->curr_io_level = io_level;
- pr_debug("%s: %s: Handled IRQ(%d), irq_status=0x%x, ack=0x%x\n",
+ dev_dbg(mmc_dev(mmc), "%s: %s: Handled IRQ(%d), irq_status=0x%x, ack=0x%x\n",
mmc_hostname(msm_host->mmc), __func__, irq, irq_status,
irq_ack);
}
@@ -1874,6 +1985,106 @@ static void sdhci_msm_reset(struct sdhci_host *host, u8 mask)
sdhci_reset(host, mask);
}
+static int sdhci_msm_register_vreg(struct sdhci_msm_host *msm_host)
+{
+ int ret;
+ u32 vmmc_load;
+ struct mmc_host *mmc = msm_host->mmc;
+
+ ret = mmc_regulator_get_supply(msm_host->mmc);
+ if (ret)
+ return ret;
+ device_property_read_u32(&msm_host->pdev->dev,
+ "vmmc-supply-max-microamp",
+ &vmmc_load);
+ device_property_read_u32(&msm_host->pdev->dev,
+ "vqmmc-supply-max-microamp",
+ &msm_host->vqmmc_load);
+
+ /* Set active load */
+ if (!IS_ERR(mmc->supply.vmmc) && vmmc_load) {
+ ret = regulator_set_load(mmc->supply.vmmc, vmmc_load);
+ if (ret) {
+ dev_err(mmc_dev(mmc), "%s: vmmc set active load failed: %d\n",
+ mmc_hostname(mmc), ret);
+ return ret;
+ }
+ }
+ if (!IS_ERR(mmc->supply.vqmmc) && msm_host->vqmmc_load) {
+ ret = regulator_set_load(mmc->supply.vmmc,
+ msm_host->vqmmc_load);
+ if (ret) {
+ dev_err(mmc_dev(mmc), "%s: vqmmc set active load failed: %d\n",
+ mmc_hostname(mmc), ret);
+ return ret;
+ }
+ }
+
+ sdhci_msm_set_regulator_caps(msm_host);
+ mmc->ios.power_mode = MMC_POWER_UNDEFINED;
+
+ return 0;
+}
+
+static int sdhci_msm_start_signal_voltage_switch(struct mmc_host *mmc,
+ struct mmc_ios *ios)
+{
+ struct sdhci_host *host = mmc_priv(mmc);
+ u16 ctrl, status;
+
+ /*
+ * Signal Voltage Switching is only applicable for Host Controllers
+ * v3.00 and above.
+ */
+ if (host->version < SDHCI_SPEC_300)
+ return 0;
+
+ ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
+
+ switch (ios->signal_voltage) {
+ case MMC_SIGNAL_VOLTAGE_330:
+ if (!(host->flags & SDHCI_SIGNALING_330))
+ return -EINVAL;
+
+ /* Set 1.8V Signal Enable in the Host Control2 register to 0 */
+ ctrl &= ~SDHCI_CTRL_VDD_180;
+ break;
+ case MMC_SIGNAL_VOLTAGE_180:
+ if (!(host->flags & SDHCI_SIGNALING_180))
+ return -EINVAL;
+
+ /*
+ * Enable 1.8V Signal Enable in the Host Control2
+ * register
+ */
+ ctrl |= SDHCI_CTRL_VDD_180;
+ break;
+ case MMC_SIGNAL_VOLTAGE_120:
+ if (!(host->flags & SDHCI_SIGNALING_120))
+ return -EINVAL;
+ return 0;
+ default:
+ /* No signal voltage switch required */
+ return 0;
+ }
+
+ sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
+
+ /* Wait for 5ms */
+ usleep_range(5000, 5500);
+
+ /* regulator output should be stable within 5 ms */
+ status = ctrl & SDHCI_CTRL_VDD_180;
+ ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
+ if ((ctrl & SDHCI_CTRL_VDD_180) == status)
+ return 0;
+
+ dev_warn(mmc_dev(mmc), "%s: Regulator output did not became stable\n",
+ mmc_hostname(mmc));
+
+ return -EAGAIN;
+}
+
#define DRIVER_NAME "sdhci_msm"
#define SDHCI_MSM_DUMP(f, x...) \
pr_err("%s: " DRIVER_NAME ": " f, mmc_hostname(host->mmc), ## x)
@@ -1960,6 +2171,7 @@ void sdhci_msm_dump_vendor_regs(struct sdhci_host *host)
.write_b = sdhci_msm_writeb,
.irq = sdhci_msm_cqe_irq,
.dump_vendor_regs = sdhci_msm_dump_vendor_regs,
+ .set_power = sdhci_set_power_noreg,
};
static const struct sdhci_pltfm_data sdhci_msm_pdata = {
@@ -2169,6 +2381,10 @@ static int sdhci_msm_probe(struct platform_device *pdev)
if (core_major == 1 && core_minor >= 0x49)
msm_host->updated_ddr_cfg = true;
+ ret = sdhci_msm_register_vreg(msm_host);
+ if (ret)
+ goto clk_disable;
+
/*
* Power on reset state may trigger power irq if previous status of
* PWRCTL was either BUS_ON or IO_HIGH_V. So before enabling pwr irq
@@ -2213,6 +2429,8 @@ static int sdhci_msm_probe(struct platform_device *pdev)
MSM_MMC_AUTOSUSPEND_DELAY_MS);
pm_runtime_use_autosuspend(&pdev->dev);
+ host->mmc_host_ops.start_signal_voltage_switch =
+ sdhci_msm_start_signal_voltage_switch;
host->mmc_host_ops.execute_tuning = sdhci_msm_execute_tuning;
if (of_property_read_bool(node, "supports-cqe"))
ret = sdhci_msm_cqe_add_host(host, pdev);
@@ -2220,7 +2438,6 @@ static int sdhci_msm_probe(struct platform_device *pdev)
ret = sdhci_add_host(host);
if (ret)
goto pm_runtime_disable;
- sdhci_msm_set_regulator_caps(msm_host);
pm_runtime_mark_last_busy(&pdev->dev);
pm_runtime_put_autosuspend(&pdev->dev);
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
^ permalink raw reply related
* [PATCH V3 0/3] Internal voltage control for qcom SDHC
From: Veerabhadrarao Badiganti @ 2020-06-02 10:47 UTC (permalink / raw)
To: adrian.hunter, ulf.hansson, bjorn.andersson, robh+dt
Cc: linux-mmc, linux-kernel, linux-arm-msm, devicetree,
Veerabhadrarao Badiganti
In-Reply-To: <1589541535-8523-1-git-send-email-vbadigan@codeaurora.org>
On qcom SD host controllers voltage switching be done after the HW
is ready for it. The HW informs its readiness through power irq.
The voltage switching should happen only then.
So added support to register voltage regulators from the msm driver
and use them.
This patchset was posted long back but not actively pursued
https://lore.kernel.org/linux-arm-msm/1539004739-32060-1-git-send-email-vbadigan@codeaurora.org/
So posting it as fresh patchset.
Changes since V2:
- Removed redundant log from sdhci_msm_set_vmmc.
- Added the condition for skiping disabling of vqmmc for eMMC.
- Updated logic such that, setting load for vqmmc only if
it is kept ON.
- Retained ack by Adrian for second patch.
- Updated dt properties names as per Robs comments.
Changes since V1:
- Removed setting load for Vmmc regulator while turning it on/off.
Instead setting the active load once during probe.
- Simplified handlng of supplies for BUS_ON/OFF cases in shci_msm_handle_pwr_irq().
- Moved common code out of switch case in sdhci_msm_start_signal_voltage_switch().
- Updated variable name to sdhci_core_to_disable_vqmmc.
- Updated pr_err logs to dev_err logs.
Veerabhadrarao Badiganti (2):
dt-bindings: mmc: Supply max load for mmc supplies
mmc: sdhci-msm: Use internal voltage control
Vijay Viswanath (1):
mmc: sdhci: Allow platform controlled voltage switching
.../devicetree/bindings/mmc/mmc-controller.yaml | 6 +
drivers/mmc/host/sdhci-msm.c | 235 ++++++++++++++++++++-
drivers/mmc/host/sdhci.c | 32 +--
drivers/mmc/host/sdhci.h | 1 +
4 files changed, 252 insertions(+), 22 deletions(-)
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
^ permalink raw reply
* [PATCH V3 1/3] dt-bindings: mmc: Supply max load for mmc supplies
From: Veerabhadrarao Badiganti @ 2020-06-02 10:47 UTC (permalink / raw)
To: adrian.hunter, ulf.hansson, bjorn.andersson, robh+dt
Cc: linux-mmc, linux-kernel, linux-arm-msm, devicetree,
Veerabhadrarao Badiganti
In-Reply-To: <1591094883-11674-1-git-send-email-vbadigan@codeaurora.org>
Supply the max load needed for driving the mmc supplies.
Signed-off-by: Veerabhadrarao Badiganti <vbadigan@codeaurora.org>
---
Documentation/devicetree/bindings/mmc/mmc-controller.yaml | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/mmc/mmc-controller.yaml b/Documentation/devicetree/bindings/mmc/mmc-controller.yaml
index acc9f10871d4..d95219721fa1 100644
--- a/Documentation/devicetree/bindings/mmc/mmc-controller.yaml
+++ b/Documentation/devicetree/bindings/mmc/mmc-controller.yaml
@@ -290,6 +290,12 @@ properties:
description:
Supply for the bus IO line power
+ vmmc-supply-max-microamp:
+ description: Maximum load for the card power.
+
+ vqmmc-supply-max-microamp:
+ description: Maximum load for the bus IO line power.
+
mmc-pwrseq:
$ref: /schemas/types.yaml#/definitions/phandle
description:
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
^ permalink raw reply related
* Re: [PATCH V6 4/5] clk: qcom: Add ipq6018 apss clock controller
From: Sivaprakash Murugesan @ 2020-06-02 10:47 UTC (permalink / raw)
To: Stephen Boyd, agross, bjorn.andersson, devicetree, linux-arm-msm,
linux-clk, linux-kernel, mturquette, robh+dt
In-Reply-To: <159104019638.69627.9161269856470136421@swboyd.mtv.corp.google.com>
On 6/2/2020 1:06 AM, Stephen Boyd wrote:
> Quoting Sivaprakash Murugesan (2020-06-01 05:41:15)
>> On 5/28/2020 7:29 AM, Stephen Boyd wrote:
>>> Quoting Sivaprakash Murugesan (2020-05-27 05:24:51)
>>>> diff --git a/drivers/clk/qcom/apss-ipq6018.c b/drivers/clk/qcom/apss-ipq6018.c
>>>> new file mode 100644
>>>> index 0000000..004f7e1
>>>> --- /dev/null
>>>> +++ b/drivers/clk/qcom/apss-ipq6018.c
>>>> @@ -0,0 +1,106 @@
>>>> + P_XO,
>>>> + P_APSS_PLL_EARLY,
>>>> +};
>>>> +
>>>> +static const struct clk_parent_data parents_apcs_alias0_clk_src[] = {
>>>> + { .fw_name = "xo" },
>>>> + { .fw_name = "pll" },
>>> This pll clk is not described in the binding. Please add it there.
>> Sorry I did not get this, this PLL is not directly defined in this
>> driver and it comes
>>
>> from dts. do you still want to describe it in binding?
>>
> Yes, there should be a clock-names property for "pll" and a clocks
> property in the binding document. I didn't see that.
These are defined in
https://lkml.org/lkml/2020/5/27/658and
https://lkml.org/lkml/2020/5/27/659
it has been defined as part of mailbox binding, since this driver does
not have a dts node and it is child of apcs mailbox driver.
^ permalink raw reply
* Re: [PATCH v2 2/2] media: v4l: xilinx: Add Xilinx UHD-SDI Rx Subsystem driver
From: Hans Verkuil @ 2020-06-02 10:44 UTC (permalink / raw)
To: Vishal Sagar, Hyun Kwon, laurent.pinchart@ideasonboard.com,
mchehab@kernel.org, robh+dt@kernel.org, mark.rutland@arm.com,
Michal Simek, linux-media@vger.kernel.org,
devicetree@vger.kernel.org, hans.verkuil@cisco.com,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, Dinesh Kumar, Sandip Kothari,
Joe Perches
In-Reply-To: <DM6PR02MB6876F989682935D38AA9BF19A78A0@DM6PR02MB6876.namprd02.prod.outlook.com>
On 01/06/2020 16:59, Vishal Sagar wrote:
> Hi Hans,
>
> Thanks for reviewing!
>
>>> + case V4L2_CID_XILINX_SDIRX_TS_IS_INTERLACED:
>>> + if (!xsdirxss->vidlocked) {
>>> + dev_err(dev, "Can't get values when video not
>> locked!\n");
>>> + return -EINVAL;
>>> + }
>>> + ctrl->val = xsdirxss->ts_is_interlaced;
>>
>> This control makes no sense: the v4l2_dv_timings struct will already tell you
>> if it is an interlaced format or not. Same for v4l2_mbus_framefmt.
>>
>
> The SDI has a concept of supporting progressive, interlaced (both as we know normally) and a progressive segmented frames(psf).
> The progressive segmented frames have their video content in progressive format but the transport stream is interlaced.
> This is distinguished using the bit 6 and 7 of Byte 2 in the 4 byte ST352 payload.
> Refer to sec 5.3 in SMPTE ST 352:2010.
>
> This control can be used by the application to distinguish normal interlaced and progressive segmented frames.
Ah, interesting. So this relies on the receiver to reconstruct the progressive
frame by combining the top and bottom field, right?
I think this deserves a new v4l2_field value:
V4L2_FIELD_ALTERNATE_PROG
Basically this is identical to V4L2_FIELD_ALTERNATE, except that the two fields
combine to a single progressive frame.
Regards,
Hans
PS: I'll look at your other comments separately
^ permalink raw reply
* Re: [PATCH v6 2/2] hwrng: add sec-rng driver
From: Greg Kroah-Hartman @ 2020-06-02 10:38 UTC (permalink / raw)
To: Neal Liu
Cc: Matt Mackall, Herbert Xu, Rob Herring, Matthias Brugger,
Sean Wang, Arnd Bergmann, linux-crypto, devicetree,
linux-arm-kernel, linux-mediatek, lkml, wsd_upstream, Crystal Guo
In-Reply-To: <1591085678-22764-3-git-send-email-neal.liu@mediatek.com>
On Tue, Jun 02, 2020 at 04:14:38PM +0800, Neal Liu wrote:
> For security awareness SoCs on ARMv8 with TrustZone enabled,
> peripherals like entropy sources is not accessible from normal world
> (linux) and rather accessible from secure world (HYP/ATF/TEE) only.
> This driver aims to provide a generic interface to Arm Trusted
> Firmware or Hypervisor rng service.
>
> Signed-off-by: Neal Liu <neal.liu@mediatek.com>
> ---
> drivers/char/hw_random/Kconfig | 13 ++++
> drivers/char/hw_random/Makefile | 1 +
> drivers/char/hw_random/sec-rng.c | 155 ++++++++++++++++++++++++++++++++++++++
> 3 files changed, 169 insertions(+)
> create mode 100644 drivers/char/hw_random/sec-rng.c
>
> diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
> index 9bc46da..cb9c8a9 100644
> --- a/drivers/char/hw_random/Kconfig
> +++ b/drivers/char/hw_random/Kconfig
> @@ -474,6 +474,19 @@ config HW_RANDOM_KEYSTONE
> help
> This option enables Keystone's hardware random generator.
>
> +config HW_RANDOM_SECURE
> + tristate "Arm Security Random Number Generator support"
> + depends on HAVE_ARM_SMCCC || COMPILE_TEST
> + default HW_RANDOM
> + help
> + This driver provides kernel-side support for the Arm Security
> + Random Number Generator.
> +
> + To compile this driver as a module, choose M here. the
> + module will be called sec-rng.
> +
> + If unsure, say Y.
Why Y?
> +
> endif # HW_RANDOM
>
> config UML_RANDOM
> diff --git a/drivers/char/hw_random/Makefile b/drivers/char/hw_random/Makefile
> index a7801b4..04533d1 100644
> --- a/drivers/char/hw_random/Makefile
> +++ b/drivers/char/hw_random/Makefile
> @@ -41,3 +41,4 @@ obj-$(CONFIG_HW_RANDOM_S390) += s390-trng.o
> obj-$(CONFIG_HW_RANDOM_KEYSTONE) += ks-sa-rng.o
> obj-$(CONFIG_HW_RANDOM_OPTEE) += optee-rng.o
> obj-$(CONFIG_HW_RANDOM_NPCM) += npcm-rng.o
> +obj-$(CONFIG_HW_RANDOM_SECURE) += sec-rng.o
> diff --git a/drivers/char/hw_random/sec-rng.c b/drivers/char/hw_random/sec-rng.c
> new file mode 100644
> index 0000000..c6d3872
> --- /dev/null
> +++ b/drivers/char/hw_random/sec-rng.c
> @@ -0,0 +1,155 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2020 MediaTek Inc.
> + */
> +
> +#include <linux/arm-smccc.h>
> +#include <linux/hw_random.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +
> +#define SMC_RET_NUM 4
> +#define SEC_RND_SIZE (sizeof(u32) * SMC_RET_NUM)
> +
> +#define HWRNG_SMC_FAST_CALL_VAL(func_num) \
> + ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, ARM_SMCCC_SMC_32, \
> + ARM_SMCCC_OWNER_SIP, (func_num))
> +
> +#define to_sec_rng(p) container_of(p, struct sec_rng_priv, rng)
> +
> +typedef void (sec_rng_fn)(unsigned long, unsigned long, unsigned long,
> + unsigned long, unsigned long, unsigned long,
> + unsigned long, unsigned long,
> + struct arm_smccc_res *);
Why not throw some more unsigned longs in there? :)
Seriously, no variable names for these? Why not?
And given that you only use the first parameter, why have 7 of them that
are not used at all? That feels pointless and needlessly complex.
> +
> +struct sec_rng_priv {
> + u16 func_num;
> + sec_rng_fn *rng_fn;
> + struct hwrng rng;
> +};
Nit, if you put 'struct hwrng' at the top of the structure, your
"to_sec_rng()" macro resolves to a simple cast, no math at all.
> +
> +/* Simple wrapper functions to be able to use a function pointer */
> +static void sec_rng_smc(unsigned long a0, unsigned long a1,
> + unsigned long a2, unsigned long a3,
> + unsigned long a4, unsigned long a5,
> + unsigned long a6, unsigned long a7,
> + struct arm_smccc_res *res)
> +{
> + arm_smccc_smc(a0, a1, a2, a3, a4, a5, a6, a7, res);
> +}
> +
> +static void sec_rng_hvc(unsigned long a0, unsigned long a1,
> + unsigned long a2, unsigned long a3,
> + unsigned long a4, unsigned long a5,
> + unsigned long a6, unsigned long a7,
> + struct arm_smccc_res *res)
> +{
> + arm_smccc_hvc(a0, a1, a2, a3, a4, a5, a6, a7, res);
> +}
> +
> +static bool __sec_get_rnd(struct sec_rng_priv *priv, uint32_t *val)
> +{
> + struct arm_smccc_res res;
> +
> + priv->rng_fn(HWRNG_SMC_FAST_CALL_VAL(priv->func_num),
> + 0, 0, 0, 0, 0, 0, 0, &res);
See, all 0's :(
You could hard-code them in the functions above instead.
But, all of this pointer indirection is really odd, why is it needed at
all? Why not just call one or the other depending on the "type" at
runtime? Wouldn't that actually be faster (hint, it is...), if you
cared about speed here (hint, I doubt it matters).
> +
> + if (!res.a0 && !res.a1 && !res.a2 && !res.a3)
> + return false;
> +
> + val[0] = res.a0;
> + val[1] = res.a1;
> + val[2] = res.a2;
> + val[3] = res.a3;
So no values out of the random number generator can be 0? Feels like an
odd thing for a random number not to be allowed to do, why this
restriction?
> +
> + return true;
> +}
> +
> +static int sec_rng_read(struct hwrng *rng, void *buf, size_t max, bool wait)
> +{
> + struct sec_rng_priv *priv = to_sec_rng(rng);
> + u32 val[4] = {0};
> + int retval = 0;
> + int i;
> +
> + while (max >= SEC_RND_SIZE) {
> + if (!__sec_get_rnd(priv, val))
> + return retval;
> +
> + for (i = 0; i < SMC_RET_NUM; i++) {
> + *(u32 *)buf = val[i];
> + buf += sizeof(u32);
Wait, what happens if buf is not a multiple of 4? Didn't you just
overwrite some memory above with the previous line?
> + }
> +
> + retval += SEC_RND_SIZE;
> + max -= SEC_RND_SIZE;
> + }
> +
> + return retval;
> +}
> +
> +static int sec_rng_probe(struct platform_device *pdev)
> +{
> + struct sec_rng_priv *priv;
> + const char *method;
> + int ret;
> +
> + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + if (of_property_read_string(pdev->dev.of_node, "method", &method))
> + return -ENXIO;
> +
> + if (!strncmp("smc", method, strlen("smc")))
> + priv->rng_fn = sec_rng_smc;
> + else if (!strncmp("hvc", method, strlen("hvc")))
> + priv->rng_fn = sec_rng_hvc;
> +
> + if (IS_ERR(priv->rng_fn)) {
How can this ever be true?
Just put another else on the above list and you should be fine.
> + dev_err(&pdev->dev, "method %s is not supported\n", method);
> + return -EINVAL;
> + }
> +
> + if (of_property_read_u16(pdev->dev.of_node, "method-fid",
> + &priv->func_num))
> + return -ENXIO;
> +
> + if (of_property_read_u16(pdev->dev.of_node, "quality",
> + &priv->rng.quality))
> + return -ENXIO;
> +
> + priv->rng.name = pdev->name;
> + priv->rng.read = sec_rng_read;
> + priv->rng.priv = (unsigned long)&pdev->dev;
> +
> + ret = devm_hwrng_register(&pdev->dev, &priv->rng);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to register rng device: %d\n", ret);
Doesn't the caller print out something if this fails?
thanks,
greg k-h
^ permalink raw reply
* RE: [PATCH v7 21/24] iommu/arm-smmu-v3: Add stall support for platform devices
From: Shameerali Kolothum Thodi @ 2020-06-02 10:31 UTC (permalink / raw)
To: Jean-Philippe Brucker
Cc: devicetree@vger.kernel.org, kevin.tian@intel.com, will@kernel.org,
fenghua.yu@intel.com, jgg@ziepe.ca, linux-pci@vger.kernel.org,
felix.kuehling@amd.com, hch@infradead.org, linux-mm@kvack.org,
iommu@lists.linux-foundation.org, catalin.marinas@arm.com,
zhangfei.gao@linaro.org, robin.murphy@arm.com,
christian.koenig@amd.com, linux-arm-kernel@lists.infradead.org
In-Reply-To: <20200602093836.GA1029680@myrica>
Hi Jean,
> -----Original Message-----
> From: linux-arm-kernel [mailto:linux-arm-kernel-bounces@lists.infradead.org]
> On Behalf Of Jean-Philippe Brucker
> Sent: 02 June 2020 10:39
> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
> Cc: devicetree@vger.kernel.org; kevin.tian@intel.com; will@kernel.org;
> fenghua.yu@intel.com; jgg@ziepe.ca; linux-pci@vger.kernel.org;
> felix.kuehling@amd.com; hch@infradead.org; linux-mm@kvack.org;
> iommu@lists.linux-foundation.org; catalin.marinas@arm.com;
> zhangfei.gao@linaro.org; robin.murphy@arm.com;
> christian.koenig@amd.com; linux-arm-kernel@lists.infradead.org
> Subject: Re: [PATCH v7 21/24] iommu/arm-smmu-v3: Add stall support for
> platform devices
>
> Hi Shameer,
>
> On Mon, Jun 01, 2020 at 12:42:15PM +0000, Shameerali Kolothum Thodi
> wrote:
> > > /* IRQ and event handlers */
> > > +static int arm_smmu_handle_evt(struct arm_smmu_device *smmu, u64
> > > +*evt) {
> > > + int ret;
> > > + u32 perm = 0;
> > > + struct arm_smmu_master *master;
> > > + bool ssid_valid = evt[0] & EVTQ_0_SSV;
> > > + u8 type = FIELD_GET(EVTQ_0_ID, evt[0]);
> > > + u32 sid = FIELD_GET(EVTQ_0_SID, evt[0]);
> > > + struct iommu_fault_event fault_evt = { };
> > > + struct iommu_fault *flt = &fault_evt.fault;
> > > +
> > > + /* Stage-2 is always pinned at the moment */
> > > + if (evt[1] & EVTQ_1_S2)
> > > + return -EFAULT;
> > > +
> > > + master = arm_smmu_find_master(smmu, sid);
> > > + if (!master)
> > > + return -EINVAL;
> > > +
> > > + if (evt[1] & EVTQ_1_READ)
> > > + perm |= IOMMU_FAULT_PERM_READ;
> > > + else
> > > + perm |= IOMMU_FAULT_PERM_WRITE;
> > > +
> > > + if (evt[1] & EVTQ_1_EXEC)
> > > + perm |= IOMMU_FAULT_PERM_EXEC;
> > > +
> > > + if (evt[1] & EVTQ_1_PRIV)
> > > + perm |= IOMMU_FAULT_PERM_PRIV;
> > > +
> > > + if (evt[1] & EVTQ_1_STALL) {
> > > + flt->type = IOMMU_FAULT_PAGE_REQ;
> > > + flt->prm = (struct iommu_fault_page_request) {
> > > + .flags = IOMMU_FAULT_PAGE_REQUEST_LAST_PAGE,
> > > + .pasid = FIELD_GET(EVTQ_0_SSID, evt[0]),
> > > + .grpid = FIELD_GET(EVTQ_1_STAG, evt[1]),
> > > + .perm = perm,
> > > + .addr = FIELD_GET(EVTQ_2_ADDR, evt[2]),
> > > + };
> > > +
> >
> > > + if (ssid_valid)
> > > + flt->prm.flags |=
> IOMMU_FAULT_PAGE_REQUEST_PASID_VALID;
> >
> > Do we need to set this for STALL mode only support? I had an issue
> > with this being set on a vSVA POC based on our D06 zip device(which is
> > a "fake " pci dev that supports STALL mode but no PRI). The issue is,
> > CMDQ_OP_RESUME doesn't have any ssid or SSV params and works on sid
> and stag only.
>
> I don't understand the problem, arm_smmu_page_response() doesn't set SSID
> or SSV when sending a CMDQ_OP_RESUME. Could you detail the flow of a stall
> event and RESUME command in your prototype? Are you getting issues with
> the host driver or the guest driver?
The issue is on the host side iommu_page_response(). The flow is something like
below.
Stall: Host:-
arm_smmu_handle_evt()
iommu_report_device_fault()
vfio_pci_iommu_dev_fault_handler()
Stall: Qemu:-
vfio_dma_fault_notifier_handler()
inject_faults()
smmuv3_inject_faults()
Stall: Guest:-
arm_smmu_handle_evt()
iommu_report_device_fault()
iommu_queue_iopf
...
iopf_handle_group()
iopf_handle_single()
handle_mm_fault()
iopf_complete()
iommu_page_response()
arm_smmu_page_response()
arm_smmu_cmdq_issue_cmd(CMDQ_OP_RESUME)
Resume: Qemu:-
smmuv3_cmdq_consume(SMMU_CMD_RESUME)
smmuv3_notify_page_resp()
vfio:ioctl(page_response) --> struct iommu_page_response is filled
with only version, grpid and code.
Resume: Host:-
ioctl(page_response)
iommu_page_response() --> fails as the pending req has PASID_VALID flag
set and it checks for a match.
arm_smmu_page_response()
Hope the above is clear.
> We do need to forward the SSV flag all the way to the guest driver, so the guest
> can find the faulting address space using the SSID. Once the guest handled the
> fault, then we don't send the SSID back to the host as part of the RESUME
> command.
True, the guest requires SSV flag to handle the page fault. But, as shown in the
flow above, the issue is on the host side iommu_page_response() where it
searches for a matching pending req based on pasid. Not sure we can bypass
that and call arm_smmu_page_response() directly but then have to delete the
pending req from the list as well.
Please let me know if there is a better way to handle the host side page
response.
Thanks,
Shameer
> Thanks,
> Jean
>
> > Hence, it is difficult for
> > Qemu SMMUv3 to populate this fields while preparing a page response. I
> > can see that this flag is being checked in iopf_handle_single() (patch
> > 04/24) as well. For POC, I used a temp fix[1] to work around this. Please let
> me know your thoughts.
> >
> > Thanks,
> > Shameer
> >
> > 1.
> > https://github.com/hisilicon/kernel-dev/commit/99ff96146e924055f38d97a
> > 5897e4becfa378d15
> >
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 6/6] MAINTAINERS: Add maintainers for MIPS core drivers
From: Marc Zyngier @ 2020-06-02 10:12 UTC (permalink / raw)
To: Serge Semin
Cc: Thomas Bogendoerfer, Thomas Gleixner, Greg Kroah-Hartman,
Serge Semin, Alexey Malahov, Paul Burton, Rob Herring,
Arnd Bergmann, Jason Cooper, Rafael J. Wysocki, Daniel Lezcano,
James Hogan, linux-mips, devicetree, linux-kernel
In-Reply-To: <20200602100921.1155-7-Sergey.Semin@baikalelectronics.ru>
On 2020-06-02 11:09, Serge Semin wrote:
> Add Thomas and myself as maintainers of the MIPS CPU and GIC IRQchip,
> MIPS
> GIC timer and MIPS CPS CPUidle drivers.
>
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
>
> ---
>
> Changelog v3:
> - Keep the files list alphabetically ordered.
> - Add Thomas as the co-maintainer of the designated drivers.
> ---
> MAINTAINERS | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 2926327e4976..20532e0287d7 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11278,6 +11278,17 @@
> F: arch/mips/configs/generic/board-boston.config
> F: drivers/clk/imgtec/clk-boston.c
> F: include/dt-bindings/clock/boston-clock.h
>
> +MIPS CORE DRIVERS
> +M: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> +M: Serge Semin <fancer.lancer@gmail.com>
> +L: linux-mips@vger.kernel.org
> +S: Supported
> +F: drivers/bus/mips_cdmm.c
> +F: drivers/clocksource/mips-gic-timer.c
> +F: drivers/cpuidle/cpuidle-cps.c
> +F: drivers/irqchip/irq-mips-cpu.c
> +F: drivers/irqchip/irq-mips-gic.c
> +
> MIPS GENERIC PLATFORM
> M: Paul Burton <paulburton@kernel.org>
> L: linux-mips@vger.kernel.org
Acked-by: Marc Zyngier <maz@kernel.org>
I assume this will go via the MIPS tree.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ 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