2021-09-28 07:36:22

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: [PATCH v12 00/11] Add support for Hikey 970 PCIe

The pcie-kirin PCIe driver contains internally a PHY interface for
Kirin 960, but it misses support for Kirin 970.

Patch1 contains a PHY for Kirin 970 PCIe.

The remaining patches add support for Kirin 970 at the pcie-kirin driver, and
add the needed logic to compile it as module and to allow to dynamically
remove the driver in runtime.

Tested on HiKey970:

# lspci -D -PP
0000:00:00.0 PCI bridge: Huawei Technologies Co., Ltd. Device 3670 (rev 01)
0000:00:00.0/01:00.0 PCI bridge: PLX Technology, Inc. PEX 8606 6 Lane, 6 Port PCI Express Gen 2 (5.0 GT/s) Switch (rev ba)
0000:00:00.0/01:00.0/02:01.0 PCI bridge: PLX Technology, Inc. PEX 8606 6 Lane, 6 Port PCI Express Gen 2 (5.0 GT/s) Switch (rev ba)
0000:00:00.0/01:00.0/02:04.0 PCI bridge: PLX Technology, Inc. PEX 8606 6 Lane, 6 Port PCI Express Gen 2 (5.0 GT/s) Switch (rev ba)
0000:00:00.0/01:00.0/02:05.0 PCI bridge: PLX Technology, Inc. PEX 8606 6 Lane, 6 Port PCI Express Gen 2 (5.0 GT/s) Switch (rev ba)
0000:00:00.0/01:00.0/02:07.0 PCI bridge: PLX Technology, Inc. PEX 8606 6 Lane, 6 Port PCI Express Gen 2 (5.0 GT/s) Switch (rev ba)
0000:00:00.0/01:00.0/02:09.0 PCI bridge: PLX Technology, Inc. PEX 8606 6 Lane, 6 Port PCI Express Gen 2 (5.0 GT/s) Switch (rev ba)
0000:00:00.0/01:00.0/02:01.0/03:00.0 Non-Volatile memory controller: Samsung Electronics Co Ltd Device a809
0000:00:00.0/01:00.0/02:07.0/06:00.0 Ethernet controller: Realtek Semiconductor Co., Ltd. RTL8111/8168/8411 PCI Express Gigabit Ethernet Controller (rev 07)

Tested on HiKey960:

# lspci -D
0000:00:00.0 PCI bridge: Huawei Technologies Co., Ltd. Device 3660 (rev 01)

---

v12:
- Change a comment at patch 1 to not use c99 style.

v11:
- patch 5 changed to use the right PCIe topology
- all other patches are identical to v10.

v10:
- patch 1: dropped magic numbers from PHY driver
- patch 5: allow pcie child nodes without reset-gpios
- all other patches are identical to v9.

v9:
- Did some cleanups at patches 1 and 5



Mauro Carvalho Chehab (11):
phy: HiSilicon: Add driver for Kirin 970 PCIe PHY
PCI: kirin: Reorganize the PHY logic inside the driver
PCI: kirin: Add support for a PHY layer
PCI: kirin: Use regmap for APB registers
PCI: kirin: Add support for bridge slot DT schema
PCI: kirin: Add Kirin 970 compatible
PCI: kirin: Add MODULE_* macros
PCI: kirin: Allow building it as a module
PCI: kirin: Add power_off support for Kirin 960 PHY
PCI: kirin: fix poweroff sequence
PCI: kirin: Allow removing the driver

drivers/pci/controller/dwc/Kconfig | 2 +-
drivers/pci/controller/dwc/pcie-kirin.c | 644 +++++++++++++-----
drivers/phy/hisilicon/Kconfig | 10 +
drivers/phy/hisilicon/Makefile | 1 +
drivers/phy/hisilicon/phy-hi3670-pcie.c | 845 ++++++++++++++++++++++++
5 files changed, 1354 insertions(+), 148 deletions(-)
create mode 100644 drivers/phy/hisilicon/phy-hi3670-pcie.c

--
2.31.1



2021-09-28 07:36:29

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: [PATCH v12 03/11] PCI: kirin: Add support for a PHY layer

The pcie-kirin driver contains both PHY and generic PCI driver
on it.

The best would be, instead, to support a PCI PHY driver, making
the driver more generic.

However, it is too late to remove the Kirin 960 PHY, as a change
like that would make the DT schema incompatible with past versions.

So, add support for an external PHY driver without removing the
existing Kirin 960 PHY from it.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---

To mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH v12 00/11] at: https://lore.kernel.org/all/[email protected]/

drivers/pci/controller/dwc/pcie-kirin.c | 95 +++++++++++++++++++++----
1 file changed, 80 insertions(+), 15 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-kirin.c b/drivers/pci/controller/dwc/pcie-kirin.c
index b4063a3434df..31514a5d4bb4 100644
--- a/drivers/pci/controller/dwc/pcie-kirin.c
+++ b/drivers/pci/controller/dwc/pcie-kirin.c
@@ -8,16 +8,18 @@
* Author: Xiaowei Song <[email protected]>
*/

-#include <linux/compiler.h>
#include <linux/clk.h>
+#include <linux/compiler.h>
#include <linux/delay.h>
#include <linux/err.h>
#include <linux/gpio.h>
#include <linux/interrupt.h>
#include <linux/mfd/syscon.h>
#include <linux/of_address.h>
+#include <linux/of_device.h>
#include <linux/of_gpio.h>
#include <linux/of_pci.h>
+#include <linux/phy/phy.h>
#include <linux/pci.h>
#include <linux/pci_regs.h>
#include <linux/platform_device.h>
@@ -50,11 +52,18 @@
#define PCIE_DEBOUNCE_PARAM 0xF0F400
#define PCIE_OE_BYPASS (0x3 << 28)

+enum pcie_kirin_phy_type {
+ PCIE_KIRIN_INTERNAL_PHY,
+ PCIE_KIRIN_EXTERNAL_PHY
+};
+
struct kirin_pcie {
+ enum pcie_kirin_phy_type type;
+
struct dw_pcie *pci;
struct phy *phy;
void __iomem *apb_base;
- void *phy_priv; /* Needed for Kirin 960 PHY */
+ void *phy_priv; /* only for PCIE_KIRIN_INTERNAL_PHY */
};

/*
@@ -476,8 +485,63 @@ static const struct dw_pcie_host_ops kirin_pcie_host_ops = {
.host_init = kirin_pcie_host_init,
};

+static int kirin_pcie_power_on(struct platform_device *pdev,
+ struct kirin_pcie *kirin_pcie)
+{
+ struct device *dev = &pdev->dev;
+ int ret;
+
+ if (kirin_pcie->type == PCIE_KIRIN_INTERNAL_PHY) {
+ ret = hi3660_pcie_phy_init(pdev, kirin_pcie);
+ if (ret)
+ return ret;
+
+ return hi3660_pcie_phy_power_on(kirin_pcie);
+ }
+
+ kirin_pcie->phy = devm_of_phy_get(dev, dev->of_node, NULL);
+ if (IS_ERR(kirin_pcie->phy))
+ return PTR_ERR(kirin_pcie->phy);
+
+ ret = phy_init(kirin_pcie->phy);
+ if (ret)
+ goto err;
+
+ ret = phy_power_on(kirin_pcie->phy);
+ if (ret)
+ goto err;
+
+ return 0;
+err:
+ phy_exit(kirin_pcie->phy);
+ return ret;
+}
+
+static int __exit kirin_pcie_remove(struct platform_device *pdev)
+{
+ struct kirin_pcie *kirin_pcie = platform_get_drvdata(pdev);
+
+ if (kirin_pcie->type == PCIE_KIRIN_INTERNAL_PHY)
+ return 0;
+
+ phy_power_off(kirin_pcie->phy);
+ phy_exit(kirin_pcie->phy);
+
+ return 0;
+}
+
+static const struct of_device_id kirin_pcie_match[] = {
+ {
+ .compatible = "hisilicon,kirin960-pcie",
+ .data = (void *)PCIE_KIRIN_INTERNAL_PHY
+ },
+ {},
+};
+
static int kirin_pcie_probe(struct platform_device *pdev)
{
+ enum pcie_kirin_phy_type phy_type;
+ const struct of_device_id *of_id;
struct device *dev = &pdev->dev;
struct kirin_pcie *kirin_pcie;
struct dw_pcie *pci;
@@ -488,6 +552,14 @@ static int kirin_pcie_probe(struct platform_device *pdev)
return -EINVAL;
}

+ of_id = of_match_device(kirin_pcie_match, dev);
+ if (!of_id) {
+ dev_err(dev, "OF data missing\n");
+ return -EINVAL;
+ }
+
+ phy_type = (enum pcie_kirin_phy_type)of_id->data;
+
kirin_pcie = devm_kzalloc(dev, sizeof(struct kirin_pcie), GFP_KERNEL);
if (!kirin_pcie)
return -ENOMEM;
@@ -500,31 +572,24 @@ static int kirin_pcie_probe(struct platform_device *pdev)
pci->ops = &kirin_dw_pcie_ops;
pci->pp.ops = &kirin_pcie_host_ops;
kirin_pcie->pci = pci;
-
- ret = hi3660_pcie_phy_init(pdev, kirin_pcie);
- if (ret)
- return ret;
+ kirin_pcie->type = phy_type;

ret = kirin_pcie_get_resource(kirin_pcie, pdev);
if (ret)
return ret;

- ret = hi3660_pcie_phy_power_on(kirin_pcie);
- if (ret)
- return ret;
-
platform_set_drvdata(pdev, kirin_pcie);

+ ret = kirin_pcie_power_on(pdev, kirin_pcie);
+ if (ret)
+ return ret;
+
return dw_pcie_host_init(&pci->pp);
}

-static const struct of_device_id kirin_pcie_match[] = {
- { .compatible = "hisilicon,kirin960-pcie" },
- {},
-};
-
static struct platform_driver kirin_pcie_driver = {
.probe = kirin_pcie_probe,
+ .remove = __exit_p(kirin_pcie_remove),
.driver = {
.name = "kirin-pcie",
.of_match_table = kirin_pcie_match,
--
2.31.1

2021-09-28 07:36:35

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: [PATCH v12 06/11] PCI: kirin: Add Kirin 970 compatible

Now that everything is in place, add a compatible for Kirin 970.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---

To mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH v12 00/11] at: https://lore.kernel.org/all/[email protected]/

drivers/pci/controller/dwc/pcie-kirin.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-kirin.c b/drivers/pci/controller/dwc/pcie-kirin.c
index 5c97e91adbb0..4f4c86b92353 100644
--- a/drivers/pci/controller/dwc/pcie-kirin.c
+++ b/drivers/pci/controller/dwc/pcie-kirin.c
@@ -738,6 +738,10 @@ static const struct of_device_id kirin_pcie_match[] = {
.compatible = "hisilicon,kirin960-pcie",
.data = (void *)PCIE_KIRIN_INTERNAL_PHY
},
+ {
+ .compatible = "hisilicon,kirin970-pcie",
+ .data = (void *)PCIE_KIRIN_EXTERNAL_PHY
+ },
{},
};

--
2.31.1

2021-09-28 07:36:45

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: [PATCH v12 07/11] PCI: kirin: Add MODULE_* macros

This driver misses the MODULE_* macros. Add them.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---

To mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH v12 00/11] at: https://lore.kernel.org/all/[email protected]/

drivers/pci/controller/dwc/pcie-kirin.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-kirin.c b/drivers/pci/controller/dwc/pcie-kirin.c
index 4f4c86b92353..d5c29a266756 100644
--- a/drivers/pci/controller/dwc/pcie-kirin.c
+++ b/drivers/pci/controller/dwc/pcie-kirin.c
@@ -804,3 +804,8 @@ static struct platform_driver kirin_pcie_driver = {
},
};
builtin_platform_driver(kirin_pcie_driver);
+
+MODULE_DEVICE_TABLE(of, kirin_pcie_match);
+MODULE_DESCRIPTION("PCIe host controller driver for Kirin Phone SoCs");
+MODULE_AUTHOR("Xiaowei Song <[email protected]>");
+MODULE_LICENSE("GPL v2");
--
2.31.1

2021-09-28 07:37:19

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: [PATCH v12 05/11] PCI: kirin: Add support for bridge slot DT schema

On HiKey970, there's a PEX 8606 PCI bridge on its PHY with
6 lanes. Only 4 lanes are connected:

lane 0 - connected to Kirin 970;
lane 4 - M.2 slot;
lane 5 - mini PCIe slot;
lane 6 - in-board Ethernet controller.

Each lane has its own PERST# gpio pin, and needs a clock
request.

Add support to parse a DT schema containing the above data.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---

To mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH v12 00/11] at: https://lore.kernel.org/all/[email protected]/

drivers/pci/controller/dwc/pcie-kirin.c | 264 +++++++++++++++++++++---
1 file changed, 232 insertions(+), 32 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-kirin.c b/drivers/pci/controller/dwc/pcie-kirin.c
index 0ea92a521e1c..5c97e91adbb0 100644
--- a/drivers/pci/controller/dwc/pcie-kirin.c
+++ b/drivers/pci/controller/dwc/pcie-kirin.c
@@ -52,6 +52,19 @@
#define PCIE_DEBOUNCE_PARAM 0xF0F400
#define PCIE_OE_BYPASS (0x3 << 28)

+/*
+ * Max number of connected PCI slots at an external PCI bridge
+ *
+ * This is used on HiKey 970, which has a PEX 8606 bridge with has
+ * 4 connected lanes (lane 0 upstream, and the other tree lanes,
+ * one connected to an in-board Ethernet adapter and the other two
+ * connected to M.2 and mini PCI slots.
+ *
+ * Each slot has a different clock source and uses a separate PERST#
+ * pin.
+ */
+#define MAX_PCI_SLOTS 3
+
enum pcie_kirin_phy_type {
PCIE_KIRIN_INTERNAL_PHY,
PCIE_KIRIN_EXTERNAL_PHY
@@ -64,6 +77,19 @@ struct kirin_pcie {
struct regmap *apb;
struct phy *phy;
void *phy_priv; /* only for PCIE_KIRIN_INTERNAL_PHY */
+
+ /* DWC PERST# */
+ int gpio_id_dwc_perst;
+
+ /* Per-slot PERST# */
+ int num_slots;
+ int gpio_id_reset[MAX_PCI_SLOTS];
+ const char *reset_names[MAX_PCI_SLOTS];
+
+ /* Per-slot clkreq */
+ int n_gpio_clkreq;
+ int gpio_id_clkreq[MAX_PCI_SLOTS];
+ const char *clkreq_names[MAX_PCI_SLOTS];
};

/*
@@ -87,7 +113,7 @@ struct kirin_pcie {
#define CRGCTRL_PCIE_ASSERT_BIT 0x8c000000

/* Time for delay */
-#define REF_2_PERST_MIN 20000
+#define REF_2_PERST_MIN 21000
#define REF_2_PERST_MAX 25000
#define PERST_2_ACCESS_MIN 10000
#define PERST_2_ACCESS_MAX 12000
@@ -108,7 +134,6 @@ struct hi3660_pcie_phy {
struct clk *phy_ref_clk;
struct clk *aclk;
struct clk *aux_clk;
- int gpio_id_reset;
};

/* Registers in PCIePHY */
@@ -171,16 +196,6 @@ static int hi3660_pcie_phy_get_resource(struct hi3660_pcie_phy *phy)
if (IS_ERR(phy->sysctrl))
return PTR_ERR(phy->sysctrl);

- /* gpios */
- phy->gpio_id_reset = of_get_named_gpio(dev->of_node,
- "reset-gpios", 0);
- if (phy->gpio_id_reset == -EPROBE_DEFER) {
- return -EPROBE_DEFER;
- } else if (!gpio_is_valid(phy->gpio_id_reset)) {
- dev_err(phy->dev, "unable to get a valid gpio pin\n");
- return -ENODEV;
- }
-
return 0;
}

@@ -297,15 +312,7 @@ static int hi3660_pcie_phy_power_on(struct kirin_pcie *pcie)
if (ret)
goto disable_clks;

- /* perst assert Endpoint */
- if (!gpio_request(phy->gpio_id_reset, "pcie_perst")) {
- usleep_range(REF_2_PERST_MIN, REF_2_PERST_MAX);
- ret = gpio_direction_output(phy->gpio_id_reset, 1);
- if (ret)
- goto disable_clks;
- usleep_range(PERST_2_ACCESS_MIN, PERST_2_ACCESS_MAX);
- return 0;
- }
+ return 0;

disable_clks:
hi3660_pcie_phy_clk_ctrl(phy, false);
@@ -347,11 +354,98 @@ static const struct regmap_config pcie_kirin_regmap_conf = {
.reg_stride = 4,
};

+static int kirin_pcie_get_gpio_enable(struct kirin_pcie *pcie,
+ struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct device_node *np = dev->of_node;
+ char name[32];
+ int ret, i;
+
+ /* This is an optional property */
+ ret = of_gpio_named_count(np, "hisilicon,clken-gpios");
+ if (ret < 0)
+ return 0;
+
+ if (ret > MAX_PCI_SLOTS) {
+ dev_err(dev, "Too many GPIO clock requests!\n");
+ return -EINVAL;
+ }
+
+ pcie->n_gpio_clkreq = ret;
+
+ for (i = 0; i < pcie->n_gpio_clkreq; i++) {
+ pcie->gpio_id_clkreq[i] = of_get_named_gpio(dev->of_node,
+ "hisilicon,clken-gpios", i);
+ if (pcie->gpio_id_clkreq[i] < 0)
+ return pcie->gpio_id_clkreq[i];
+
+ sprintf(name, "pcie_clkreq_%d", i);
+ pcie->clkreq_names[i] = devm_kstrdup_const(dev, name,
+ GFP_KERNEL);
+ if (!pcie->clkreq_names[i])
+ return -ENOMEM;
+ }
+
+ return 0;
+}
+
+static int kirin_pcie_parse_port(struct kirin_pcie *pcie,
+ struct platform_device *pdev,
+ struct device_node *node)
+{
+ struct device *dev = &pdev->dev;
+ struct device_node *parent, *child;
+ int ret, slot, i;
+ char name[32];
+
+ for_each_available_child_of_node(node, parent) {
+ for_each_available_child_of_node(parent, child) {
+ i = pcie->num_slots;
+
+ pcie->gpio_id_reset[i] = of_get_named_gpio(child,
+ "reset-gpios", 0);
+ if (pcie->gpio_id_reset[i] < 0)
+ continue;
+
+ pcie->num_slots++;
+ if (pcie->num_slots > MAX_PCI_SLOTS) {
+ dev_err(dev, "Too many PCI slots!\n");
+ return -EINVAL;
+ }
+
+ ret = of_pci_get_devfn(child);
+ if (ret < 0) {
+ dev_err(dev, "failed to parse devfn: %d\n", ret);
+ goto put_node;
+ }
+
+ slot = PCI_SLOT(ret);
+
+ sprintf(name, "pcie_perst_%d", slot);
+ pcie->reset_names[i] = devm_kstrdup_const(dev, name,
+ GFP_KERNEL);
+ if (!pcie->reset_names[i]) {
+ ret = -ENOMEM;
+ goto put_node;
+ }
+ }
+ }
+
+ return 0;
+
+put_node:
+ of_node_put(child);
+ return ret;
+}
+
static long kirin_pcie_get_resource(struct kirin_pcie *kirin_pcie,
struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
+ struct device_node *node = dev->of_node, *child;
void __iomem *apb_base;
+ int ret;

apb_base = devm_platform_ioremap_resource_byname(pdev, "apb");
if (IS_ERR(apb_base))
@@ -362,7 +456,32 @@ static long kirin_pcie_get_resource(struct kirin_pcie *kirin_pcie,
if (IS_ERR(kirin_pcie->apb))
return PTR_ERR(kirin_pcie->apb);

+ /* pcie internal PERST# gpio */
+ kirin_pcie->gpio_id_dwc_perst = of_get_named_gpio(dev->of_node,
+ "reset-gpios", 0);
+ if (kirin_pcie->gpio_id_dwc_perst == -EPROBE_DEFER) {
+ return -EPROBE_DEFER;
+ } else if (!gpio_is_valid(kirin_pcie->gpio_id_dwc_perst)) {
+ dev_err(dev, "unable to get a valid gpio pin\n");
+ return -ENODEV;
+ }
+
+ ret = kirin_pcie_get_gpio_enable(kirin_pcie, pdev);
+ if (ret)
+ return ret;
+
+ /* Parse OF children */
+ for_each_available_child_of_node(node, child) {
+ ret = kirin_pcie_parse_port(kirin_pcie, pdev, child);
+ if (ret)
+ goto put_node;
+ }
+
return 0;
+
+put_node:
+ of_node_put(child);
+ return ret;
}

static void kirin_pcie_sideband_dbi_w_mode(struct kirin_pcie *kirin_pcie,
@@ -419,9 +538,33 @@ static int kirin_pcie_wr_own_conf(struct pci_bus *bus, unsigned int devfn,
return PCIBIOS_SUCCESSFUL;
}

+static int kirin_pcie_add_bus(struct pci_bus *bus)
+{
+ struct dw_pcie *pci = to_dw_pcie_from_pp(bus->sysdata);
+ struct kirin_pcie *kirin_pcie = to_kirin_pcie(pci);
+ int i, ret;
+
+ if (!kirin_pcie->num_slots)
+ return 0;
+
+ /* Send PERST# to each slot */
+ for (i = 0; i < kirin_pcie->num_slots; i++) {
+ ret = gpio_direction_output(kirin_pcie->gpio_id_reset[i], 1);
+ if (ret) {
+ dev_err(pci->dev, "PERST# %s error: %d\n",
+ kirin_pcie->reset_names[i], ret);
+ }
+ }
+ usleep_range(PERST_2_ACCESS_MIN, PERST_2_ACCESS_MAX);
+
+ return 0;
+}
+
+
static struct pci_ops kirin_pci_ops = {
.read = kirin_pcie_rd_own_conf,
.write = kirin_pcie_wr_own_conf,
+ .add_bus = kirin_pcie_add_bus,
};

static u32 kirin_pcie_read_dbi(struct dw_pcie *pci, void __iomem *base,
@@ -477,6 +620,44 @@ static int kirin_pcie_host_init(struct pcie_port *pp)
return 0;
}

+static int kirin_pcie_gpio_request(struct kirin_pcie *kirin_pcie,
+ struct device *dev)
+{
+ int ret, i;
+
+ for (i = 0; i < kirin_pcie->num_slots; i++) {
+ if (!gpio_is_valid(kirin_pcie->gpio_id_reset[i])) {
+ dev_err(dev, "unable to get a valid %s gpio\n",
+ kirin_pcie->reset_names[i]);
+ return -ENODEV;
+ }
+
+ ret = devm_gpio_request(dev, kirin_pcie->gpio_id_reset[i],
+ kirin_pcie->reset_names[i]);
+ if (ret)
+ return ret;
+ }
+
+ for (i = 0; i < kirin_pcie->n_gpio_clkreq; i++) {
+ if (!gpio_is_valid(kirin_pcie->gpio_id_clkreq[i])) {
+ dev_err(dev, "unable to get a valid %s gpio\n",
+ kirin_pcie->clkreq_names[i]);
+ return -ENODEV;
+ }
+
+ ret = devm_gpio_request(dev, kirin_pcie->gpio_id_clkreq[i],
+ kirin_pcie->clkreq_names[i]);
+ if (ret)
+ return ret;
+
+ ret = gpio_direction_output(kirin_pcie->gpio_id_clkreq[i], 0);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
static const struct dw_pcie_ops kirin_dw_pcie_ops = {
.read_dbi = kirin_pcie_read_dbi,
.write_dbi = kirin_pcie_write_dbi,
@@ -499,24 +680,43 @@ static int kirin_pcie_power_on(struct platform_device *pdev,
if (ret)
return ret;

- return hi3660_pcie_phy_power_on(kirin_pcie);
+ ret = hi3660_pcie_phy_power_on(kirin_pcie);
+ if (ret)
+ return ret;
+ } else {
+ kirin_pcie->phy = devm_of_phy_get(dev, dev->of_node, NULL);
+ if (IS_ERR(kirin_pcie->phy))
+ return PTR_ERR(kirin_pcie->phy);
+
+ ret = kirin_pcie_gpio_request(kirin_pcie, dev);
+ if (ret)
+ return ret;
+
+ ret = phy_init(kirin_pcie->phy);
+ if (ret)
+ goto err;
+
+ ret = phy_power_on(kirin_pcie->phy);
+ if (ret)
+ goto err;
}

- kirin_pcie->phy = devm_of_phy_get(dev, dev->of_node, NULL);
- if (IS_ERR(kirin_pcie->phy))
- return PTR_ERR(kirin_pcie->phy);
+ /* perst assert Endpoint */
+ usleep_range(REF_2_PERST_MIN, REF_2_PERST_MAX);

- ret = phy_init(kirin_pcie->phy);
- if (ret)
- goto err;
+ if (!gpio_request(kirin_pcie->gpio_id_dwc_perst, "pcie_perst_bridge")) {
+ ret = gpio_direction_output(kirin_pcie->gpio_id_dwc_perst, 1);
+ if (ret)
+ goto err;
+ }

- ret = phy_power_on(kirin_pcie->phy);
- if (ret)
- goto err;
+ usleep_range(PERST_2_ACCESS_MIN, PERST_2_ACCESS_MAX);

return 0;
err:
- phy_exit(kirin_pcie->phy);
+ if (kirin_pcie->type != PCIE_KIRIN_INTERNAL_PHY)
+ phy_exit(kirin_pcie->phy);
+
return ret;
}

--
2.31.1

2021-09-28 07:37:25

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: [PATCH v12 08/11] PCI: kirin: Allow building it as a module

There's nothing preventing this driver to be loaded as a
module. So, change its config from bool to tristate.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---

To mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH v12 00/11] at: https://lore.kernel.org/all/[email protected]/

drivers/pci/controller/dwc/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
index 76c0a63a3f64..a07c08d714c9 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -266,7 +266,7 @@ config PCIE_KEEMBAY_EP

config PCIE_KIRIN
depends on OF && (ARM64 || COMPILE_TEST)
- bool "HiSilicon Kirin series SoCs PCIe controllers"
+ tristate "HiSilicon Kirin series SoCs PCIe controllers"
depends on PCI_MSI_IRQ_DOMAIN
select PCIE_DW_HOST
help
--
2.31.1

2021-09-28 07:37:45

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: [PATCH v12 01/11] phy: HiSilicon: Add driver for Kirin 970 PCIe PHY

The Kirin 970 PHY is somewhat similar to the Kirin 960, but it
does a lot more. Add the needed bits for PCIe to start working on
HiKey 970 boards.

Co-developed-by: Manivannan Sadhasivam <[email protected]>
Signed-off-by: Manivannan Sadhasivam <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---

To mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH v12 00/11] at: https://lore.kernel.org/all/[email protected]/

drivers/phy/hisilicon/Kconfig | 10 +
drivers/phy/hisilicon/Makefile | 1 +
drivers/phy/hisilicon/phy-hi3670-pcie.c | 845 ++++++++++++++++++++++++
3 files changed, 856 insertions(+)
create mode 100644 drivers/phy/hisilicon/phy-hi3670-pcie.c

diff --git a/drivers/phy/hisilicon/Kconfig b/drivers/phy/hisilicon/Kconfig
index 4d008cfc279c..d3b92c288554 100644
--- a/drivers/phy/hisilicon/Kconfig
+++ b/drivers/phy/hisilicon/Kconfig
@@ -33,6 +33,16 @@ config PHY_HI3670_USB

To compile this driver as a module, choose M here.

+config PHY_HI3670_PCIE
+ tristate "hi3670 PCIe PHY support"
+ depends on (ARCH_HISI && ARM64) || COMPILE_TEST
+ select GENERIC_PHY
+ select MFD_SYSCON
+ help
+ Enable this to support the HiSilicon hi3670 PCIe PHY.
+
+ To compile this driver as a module, choose M here.
+
config PHY_HISTB_COMBPHY
tristate "HiSilicon STB SoCs COMBPHY support"
depends on (ARCH_HISI && ARM64) || COMPILE_TEST
diff --git a/drivers/phy/hisilicon/Makefile b/drivers/phy/hisilicon/Makefile
index 51729868145b..4029d3813b1e 100644
--- a/drivers/phy/hisilicon/Makefile
+++ b/drivers/phy/hisilicon/Makefile
@@ -2,6 +2,7 @@
obj-$(CONFIG_PHY_HI6220_USB) += phy-hi6220-usb.o
obj-$(CONFIG_PHY_HI3660_USB) += phy-hi3660-usb3.o
obj-$(CONFIG_PHY_HI3670_USB) += phy-hi3670-usb3.o
+obj-$(CONFIG_PHY_HI3670_PCIE) += phy-hi3670-pcie.o
obj-$(CONFIG_PHY_HISTB_COMBPHY) += phy-histb-combphy.o
obj-$(CONFIG_PHY_HISI_INNO_USB2) += phy-hisi-inno-usb2.o
obj-$(CONFIG_PHY_HIX5HD2_SATA) += phy-hix5hd2-sata.o
diff --git a/drivers/phy/hisilicon/phy-hi3670-pcie.c b/drivers/phy/hisilicon/phy-hi3670-pcie.c
new file mode 100644
index 000000000000..c64c6679b1b9
--- /dev/null
+++ b/drivers/phy/hisilicon/phy-hi3670-pcie.c
@@ -0,0 +1,845 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * PCIe phy driver for Kirin 970
+ *
+ * Copyright (C) 2017 HiSilicon Electronics Co., Ltd.
+ * https://www.huawei.com
+ * Copyright (C) 2021 Huawei Technologies Co., Ltd.
+ * https://www.huawei.com
+ *
+ * Authors:
+ * Mauro Carvalho Chehab <[email protected]>
+ * Manivannan Sadhasivam <[email protected]>
+ *
+ * Based on:
+ * https://lore.kernel.org/lkml/4c9d6581478aa966698758c0420933f5defab4dd.1612335031.git.mchehab+huawei@kernel.org/
+ */
+
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/gpio.h>
+#include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of_gpio.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#define AXI_CLK_FREQ 207500000
+#define REF_CLK_FREQ 100000000
+
+/* PCIe CTRL registers */
+#define SOC_PCIECTRL_CTRL7_ADDR 0x01c
+#define SOC_PCIECTRL_CTRL12_ADDR 0x030
+#define SOC_PCIECTRL_CTRL20_ADDR 0x050
+#define SOC_PCIECTRL_CTRL21_ADDR 0x054
+
+#define PCIE_OUTPUT_PULL_BITS GENMASK(3, 0)
+#define SOC_PCIECTRL_CTRL20_2P_MEM_CTRL 0x02605550
+#define SOC_PCIECTRL_CTRL21_DEFAULT 0x20000070
+#define PCIE_PULL_UP_SYS_AUX_PWR_DET BIT(10)
+#define PCIE_OUTPUT_PULL_DOWN BIT(1)
+
+/* PCIe PHY registers */
+#define SOC_PCIEPHY_CTRL0_ADDR 0x000
+#define SOC_PCIEPHY_CTRL1_ADDR 0x004
+#define SOC_PCIEPHY_CTRL38_ADDR 0x0098
+#define SOC_PCIEPHY_STATE0_ADDR 0x400
+
+#define RAWLANEN_DIG_PCS_XF_TX_OVRD_IN_1 0xc004
+#define SUP_DIG_LVL_OVRD_IN 0x003c
+#define LANEN_DIG_ASIC_TX_OVRD_IN_1 0x4008
+#define LANEN_DIG_ASIC_TX_OVRD_IN_2 0x400c
+
+#define PCIEPHY_RESET_BIT BIT(17)
+#define PCIEPHY_PIPE_LINE0_RESET_BIT BIT(19)
+#define PCIE_TXDETECT_RX_FAIL BIT(2)
+#define PCIE_CLK_SOURCE BIT(8)
+#define PCIE_IS_CLOCK_STABLE BIT(19)
+#define PCIE_PULL_DOWN_PHY_TEST_POWERDOWN BIT(22)
+#define PCIE_DEASSERT_CONTROLLER_PERST BIT(2)
+
+#define EYEPARAM_NOCFG 0xffffffff
+#define EYE_PARM0_MASK GENMASK(8, 6)
+#define EYE_PARM1_MASK GENMASK(11, 8)
+#define EYE_PARM2_MASK GENMASK(5, 0)
+#define EYE_PARM3_MASK GENMASK(12, 7)
+#define EYE_PARM4_MASK GENMASK(14, 9)
+#define EYE_PARM0_EN BIT(9)
+#define EYE_PARM1_EN BIT(12)
+#define EYE_PARM2_EN BIT(6)
+#define EYE_PARM3_EN BIT(13)
+#define EYE_PARM4_EN BIT(15)
+
+/* hi3670 pciephy register */
+#define APB_PHY_START_ADDR 0x40000
+#define SOC_PCIEPHY_MMC1PLL_CTRL1 0xc04
+#define SOC_PCIEPHY_MMC1PLL_CTRL16 0xC40
+#define SOC_PCIEPHY_MMC1PLL_CTRL17 0xC44
+#define SOC_PCIEPHY_MMC1PLL_CTRL20 0xC50
+#define SOC_PCIEPHY_MMC1PLL_CTRL21 0xC54
+#define SOC_PCIEPHY_MMC1PLL_STAT0 0xE00
+
+#define CRGPERIPH_PEREN12 0x470
+#define CRGPERIPH_PERDIS12 0x474
+#define CRGPERIPH_PCIECTRL0 0x800
+
+#define PCIE_FNPLL_FBDIV_MASK GENMASK(27, 16)
+#define PCIE_FNPLL_FRACDIV_MASK GENMASK(23, 0)
+#define PCIE_FNPLL_POSTDIV1_MASK GENMASK(10, 8)
+#define PCIE_FNPLL_POSTDIV2_MASK GENMASK(14, 12)
+#define PCIE_FNPLL_PLL_MODE_MASK BIT(25)
+
+#define PCIE_FNPLL_DLL_EN BIT(27)
+#define PCIE_FNPLL_FBDIV 0xd0
+#define PCIE_FNPLL_FRACDIV 0x555555
+#define PCIE_FNPLL_POSTDIV1 0x5
+#define PCIE_FNPLL_POSTDIV2 0x4
+#define PCIE_FNPLL_PLL_MODE 0x0
+
+#define PCIE_PHY_MMC1PLL 0x20
+#define PCIE_PHY_CHOOSE_FNPLL BIT(27)
+#define PCIE_PHY_MMC1PLL_DISABLE BIT(0)
+#define PCIE_PHY_PCIEPL_BP BIT(16)
+
+/* define ie,oe cfg */
+#define IO_OE_HARD_GT_MODE BIT(1)
+#define IO_IE_EN_HARD_BYPASS BIT(27)
+#define IO_OE_EN_HARD_BYPASS BIT(11)
+#define IO_HARD_CTRL_DEBOUNCE_BYPASS BIT(10)
+#define IO_OE_GT_MODE BIT(8)
+#define DEBOUNCE_WAITCFG_IN GENMASK(23, 20)
+#define DEBOUNCE_WAITCFG_OUT GENMASK(16, 13)
+
+#define IO_HP_DEBOUNCE_GT (BIT(12) | BIT(15))
+#define IO_PHYREF_SOFT_GT_MODE BIT(14)
+#define IO_REF_SOFT_GT_MODE BIT(13)
+#define IO_REF_HARD_GT_MODE BIT(0)
+
+/* noc power domain */
+#define NOC_POWER_IDLEREQ_1 0x38c
+#define NOC_POWER_IDLE_1 0x394
+#define NOC_PW_MASK 0x10000
+#define NOC_PW_SET_BIT 0x1
+
+#define NUM_EYEPARAM 5
+
+/* info located in sysctrl */
+#define SCTRL_PCIE_CMOS_OFFSET 0x60
+#define SCTRL_PCIE_CMOS_BIT 0x10
+#define SCTRL_PCIE_ISO_OFFSET 0x44
+#define SCTRL_PCIE_ISO_BIT 0x30
+#define SCTRL_PCIE_HPCLK_OFFSET 0x190
+#define SCTRL_PCIE_HPCLK_BIT 0x184000
+#define SCTRL_PCIE_OE_OFFSET 0x14a
+#define PCIE_DEBOUNCE_PARAM 0xf0f400
+#define PCIE_OE_BYPASS GENMASK(29, 28)
+
+/* peri_crg ctrl */
+#define CRGCTRL_PCIE_ASSERT_OFFSET 0x88
+#define CRGCTRL_PCIE_ASSERT_BIT 0x8c000000
+
+#define FNPLL_HAS_LOCKED BIT(4)
+
+/* Time for delay */
+#define TIME_CMOS_MIN 100
+#define TIME_CMOS_MAX 105
+#define PIPE_CLK_STABLE_TIME 100
+#define PLL_CTRL_WAIT_TIME 200
+#define NOC_POWER_TIME 100
+
+struct hi3670_pcie_phy {
+ struct device *dev;
+ void __iomem *base;
+ struct regmap *apb;
+ struct regmap *crgctrl;
+ struct regmap *sysctrl;
+ struct regmap *pmctrl;
+ struct clk *apb_sys_clk;
+ struct clk *apb_phy_clk;
+ struct clk *phy_ref_clk;
+ struct clk *aclk;
+ struct clk *aux_clk;
+ u32 eye_param[NUM_EYEPARAM];
+};
+
+/* Registers in PCIePHY */
+static inline void hi3670_apb_phy_writel(struct hi3670_pcie_phy *phy, u32 val,
+ u32 reg)
+{
+ writel(val, phy->base + APB_PHY_START_ADDR + reg);
+}
+
+static inline u32 hi3670_apb_phy_readl(struct hi3670_pcie_phy *phy, u32 reg)
+{
+ return readl(phy->base + APB_PHY_START_ADDR + reg);
+}
+
+static inline void hi3670_apb_phy_updatel(struct hi3670_pcie_phy *phy,
+ u32 val, u32 mask, u32 reg)
+{
+ u32 regval;
+
+ regval = hi3670_apb_phy_readl(phy, reg);
+ regval &= ~mask;
+ regval |= val;
+ hi3670_apb_phy_writel(phy, regval, reg);
+}
+
+static inline void kirin_apb_natural_phy_writel(struct hi3670_pcie_phy *phy,
+ u32 val, u32 reg)
+{
+ writel(val, phy->base + reg);
+}
+
+static inline u32 kirin_apb_natural_phy_readl(struct hi3670_pcie_phy *phy,
+ u32 reg)
+{
+ return readl(phy->base + reg);
+}
+
+static void hi3670_pcie_phy_oe_enable(struct hi3670_pcie_phy *phy, bool enable)
+{
+ u32 val;
+
+ regmap_read(phy->sysctrl, SCTRL_PCIE_OE_OFFSET, &val);
+ val |= PCIE_DEBOUNCE_PARAM;
+ if (enable)
+ val &= ~PCIE_OE_BYPASS;
+ else
+ val |= PCIE_OE_BYPASS;
+ regmap_write(phy->sysctrl, SCTRL_PCIE_OE_OFFSET, val);
+}
+
+static void hi3670_pcie_get_eyeparam(struct hi3670_pcie_phy *phy)
+{
+ struct device *dev = phy->dev;
+ struct device_node *np;
+ int ret, i;
+
+ np = dev->of_node;
+
+ ret = of_property_read_u32_array(np, "hisilicon,eye-diagram-param",
+ phy->eye_param, NUM_EYEPARAM);
+ if (!ret)
+ return;
+
+ /* There's no optional eye_param property. Set array to default */
+ for (i = 0; i < NUM_EYEPARAM; i++)
+ phy->eye_param[i] = EYEPARAM_NOCFG;
+}
+
+static void hi3670_pcie_set_eyeparam(struct hi3670_pcie_phy *phy)
+{
+ u32 val;
+
+ val = kirin_apb_natural_phy_readl(phy, RAWLANEN_DIG_PCS_XF_TX_OVRD_IN_1);
+
+ if (phy->eye_param[1] != EYEPARAM_NOCFG) {
+ val &= ~EYE_PARM1_MASK;
+ val |= FIELD_PREP(EYE_PARM1_MASK, phy->eye_param[1]);
+ val |= EYE_PARM1_EN;
+ }
+ kirin_apb_natural_phy_writel(phy, val,
+ RAWLANEN_DIG_PCS_XF_TX_OVRD_IN_1);
+
+ val = kirin_apb_natural_phy_readl(phy, LANEN_DIG_ASIC_TX_OVRD_IN_2);
+ val &= ~(EYE_PARM2_MASK | EYE_PARM3_MASK);
+ if (phy->eye_param[2] != EYEPARAM_NOCFG) {
+ val |= FIELD_PREP(EYE_PARM2_MASK, phy->eye_param[2]);
+ val |= EYE_PARM2_EN;
+ }
+
+ if (phy->eye_param[3] != EYEPARAM_NOCFG) {
+ val |= FIELD_PREP(EYE_PARM3_MASK, phy->eye_param[3]);
+ val |= EYE_PARM3_EN;
+ }
+
+ kirin_apb_natural_phy_writel(phy, val, LANEN_DIG_ASIC_TX_OVRD_IN_2);
+
+ val = kirin_apb_natural_phy_readl(phy, SUP_DIG_LVL_OVRD_IN);
+ if (phy->eye_param[0] != EYEPARAM_NOCFG) {
+ val &= ~EYE_PARM0_MASK;
+ val |= FIELD_PREP(EYE_PARM0_MASK, phy->eye_param[0]);
+ val |= EYE_PARM0_EN;
+ }
+ kirin_apb_natural_phy_writel(phy, val, SUP_DIG_LVL_OVRD_IN);
+
+ val = kirin_apb_natural_phy_readl(phy, LANEN_DIG_ASIC_TX_OVRD_IN_1);
+ if (phy->eye_param[4] != EYEPARAM_NOCFG) {
+ val &= ~EYE_PARM4_MASK;
+ val |= FIELD_PREP(EYE_PARM4_MASK, phy->eye_param[4]);
+ val |= EYE_PARM4_EN;
+ }
+ kirin_apb_natural_phy_writel(phy, val, LANEN_DIG_ASIC_TX_OVRD_IN_1);
+}
+
+static void hi3670_pcie_natural_cfg(struct hi3670_pcie_phy *phy)
+{
+ u32 val;
+
+ /* change 2p mem_ctrl */
+ regmap_write(phy->apb, SOC_PCIECTRL_CTRL20_ADDR,
+ SOC_PCIECTRL_CTRL20_2P_MEM_CTRL);
+
+ regmap_read(phy->apb, SOC_PCIECTRL_CTRL7_ADDR, &val);
+ val |= PCIE_PULL_UP_SYS_AUX_PWR_DET;
+ regmap_write(phy->apb, SOC_PCIECTRL_CTRL7_ADDR, val);
+
+ /* output, pull down */
+ regmap_read(phy->apb, SOC_PCIECTRL_CTRL12_ADDR, &val);
+ val &= ~PCIE_OUTPUT_PULL_BITS;
+ val |= PCIE_OUTPUT_PULL_DOWN;
+ regmap_write(phy->apb, SOC_PCIECTRL_CTRL12_ADDR, val);
+
+ /* Handle phy_reset and lane0_reset to HW */
+ hi3670_apb_phy_updatel(phy, PCIEPHY_RESET_BIT,
+ PCIEPHY_PIPE_LINE0_RESET_BIT | PCIEPHY_RESET_BIT,
+ SOC_PCIEPHY_CTRL1_ADDR);
+
+ /* fix chip bug: TxDetectRx fail */
+ hi3670_apb_phy_updatel(phy, PCIE_TXDETECT_RX_FAIL, PCIE_TXDETECT_RX_FAIL,
+ SOC_PCIEPHY_CTRL38_ADDR);
+}
+
+static void hi3670_pcie_pll_init(struct hi3670_pcie_phy *phy)
+{
+ hi3670_apb_phy_updatel(phy, PCIE_PHY_CHOOSE_FNPLL, PCIE_PHY_CHOOSE_FNPLL,
+ SOC_PCIEPHY_MMC1PLL_CTRL1);
+
+ hi3670_apb_phy_updatel(phy,
+ FIELD_PREP(PCIE_FNPLL_FBDIV_MASK, PCIE_FNPLL_FBDIV),
+ PCIE_FNPLL_FBDIV_MASK,
+ SOC_PCIEPHY_MMC1PLL_CTRL16);
+
+ hi3670_apb_phy_updatel(phy,
+ FIELD_PREP(PCIE_FNPLL_FRACDIV_MASK, PCIE_FNPLL_FRACDIV),
+ PCIE_FNPLL_FRACDIV_MASK, SOC_PCIEPHY_MMC1PLL_CTRL17);
+
+ hi3670_apb_phy_updatel(phy,
+ PCIE_FNPLL_DLL_EN |
+ FIELD_PREP(PCIE_FNPLL_POSTDIV1_MASK, PCIE_FNPLL_POSTDIV1) |
+ FIELD_PREP(PCIE_FNPLL_POSTDIV2_MASK, PCIE_FNPLL_POSTDIV2) |
+ FIELD_PREP(PCIE_FNPLL_PLL_MODE_MASK, PCIE_FNPLL_PLL_MODE),
+ PCIE_FNPLL_POSTDIV1_MASK |
+ PCIE_FNPLL_POSTDIV2_MASK |
+ PCIE_FNPLL_PLL_MODE_MASK | PCIE_FNPLL_DLL_EN,
+ SOC_PCIEPHY_MMC1PLL_CTRL20);
+
+ hi3670_apb_phy_writel(phy, PCIE_PHY_MMC1PLL,
+ SOC_PCIEPHY_MMC1PLL_CTRL21);
+}
+
+static int hi3670_pcie_pll_ctrl(struct hi3670_pcie_phy *phy, bool enable)
+{
+ struct device *dev = phy->dev;
+ u32 val;
+ int time = PLL_CTRL_WAIT_TIME;
+
+ if (enable) {
+ /* pd = 0 */
+ hi3670_apb_phy_updatel(phy, 0, PCIE_PHY_MMC1PLL_DISABLE,
+ SOC_PCIEPHY_MMC1PLL_CTRL16);
+
+ /* choose FNPLL */
+ val = hi3670_apb_phy_readl(phy, SOC_PCIEPHY_MMC1PLL_STAT0);
+ while (!(val & FNPLL_HAS_LOCKED)) {
+ if (!time) {
+ dev_err(dev, "wait for pll_lock timeout\n");
+ return -EINVAL;
+ }
+ time--;
+ udelay(1);
+ val = hi3670_apb_phy_readl(phy, SOC_PCIEPHY_MMC1PLL_STAT0);
+ }
+
+ hi3670_apb_phy_updatel(phy, 0, PCIE_PHY_PCIEPL_BP,
+ SOC_PCIEPHY_MMC1PLL_CTRL20);
+
+ } else {
+ hi3670_apb_phy_updatel(phy,
+ PCIE_PHY_MMC1PLL_DISABLE,
+ PCIE_PHY_MMC1PLL_DISABLE,
+ SOC_PCIEPHY_MMC1PLL_CTRL16);
+
+ hi3670_apb_phy_updatel(phy, PCIE_PHY_PCIEPL_BP,
+ PCIE_PHY_PCIEPL_BP,
+ SOC_PCIEPHY_MMC1PLL_CTRL20);
+ }
+
+ return 0;
+}
+
+static void hi3670_pcie_hp_debounce_gt(struct hi3670_pcie_phy *phy, bool open)
+{
+ if (open)
+ /* gt_clk_pcie_hp/gt_clk_pcie_debounce open */
+ regmap_write(phy->crgctrl, CRGPERIPH_PEREN12,
+ IO_HP_DEBOUNCE_GT);
+ else
+ /* gt_clk_pcie_hp/gt_clk_pcie_debounce close */
+ regmap_write(phy->crgctrl, CRGPERIPH_PERDIS12,
+ IO_HP_DEBOUNCE_GT);
+}
+
+static void hi3670_pcie_phyref_gt(struct hi3670_pcie_phy *phy, bool open)
+{
+ unsigned int val;
+
+ regmap_read(phy->crgctrl, CRGPERIPH_PCIECTRL0, &val);
+
+ if (open)
+ val &= ~IO_OE_HARD_GT_MODE; /* enable hard gt mode */
+ else
+ val |= IO_OE_HARD_GT_MODE; /* disable hard gt mode */
+
+ regmap_write(phy->crgctrl, CRGPERIPH_PCIECTRL0, val);
+
+ /* disable soft gt mode */
+ regmap_write(phy->crgctrl, CRGPERIPH_PERDIS12, IO_PHYREF_SOFT_GT_MODE);
+}
+
+static void hi3670_pcie_oe_ctrl(struct hi3670_pcie_phy *phy, bool en_flag)
+{
+ unsigned int val;
+
+ regmap_read(phy->crgctrl, CRGPERIPH_PCIECTRL0, &val);
+
+ /* set ie cfg */
+ val |= IO_IE_EN_HARD_BYPASS;
+
+ /* set oe cfg */
+ val &= ~IO_HARD_CTRL_DEBOUNCE_BYPASS;
+
+ /* set phy_debounce in&out time */
+ val |= (DEBOUNCE_WAITCFG_IN | DEBOUNCE_WAITCFG_OUT);
+
+ /* select oe_gt_mode */
+ val |= IO_OE_GT_MODE;
+
+ if (en_flag)
+ val &= ~IO_OE_EN_HARD_BYPASS;
+ else
+ val |= IO_OE_EN_HARD_BYPASS;
+
+ regmap_write(phy->crgctrl, CRGPERIPH_PCIECTRL0, val);
+}
+
+static void hi3670_pcie_ioref_gt(struct hi3670_pcie_phy *phy, bool open)
+{
+ unsigned int val;
+
+ if (open) {
+ regmap_write(phy->apb, SOC_PCIECTRL_CTRL21_ADDR,
+ SOC_PCIECTRL_CTRL21_DEFAULT);
+
+ hi3670_pcie_oe_ctrl(phy, true);
+
+ /* en hard gt mode */
+ regmap_read(phy->crgctrl, CRGPERIPH_PCIECTRL0, &val);
+ val &= ~IO_REF_HARD_GT_MODE;
+ regmap_write(phy->crgctrl, CRGPERIPH_PCIECTRL0, val);
+
+ /* disable soft gt mode */
+ regmap_write(phy->crgctrl, CRGPERIPH_PERDIS12,
+ IO_REF_SOFT_GT_MODE);
+
+ } else {
+ /* disable hard gt mode */
+ regmap_read(phy->crgctrl, CRGPERIPH_PCIECTRL0, &val);
+ val |= IO_REF_HARD_GT_MODE;
+ regmap_write(phy->crgctrl, CRGPERIPH_PCIECTRL0, val);
+
+ /* disable soft gt mode */
+ regmap_write(phy->crgctrl, CRGPERIPH_PERDIS12,
+ IO_REF_SOFT_GT_MODE);
+
+ hi3670_pcie_oe_ctrl(phy, false);
+ }
+}
+
+static int hi3670_pcie_allclk_ctrl(struct hi3670_pcie_phy *phy, bool clk_on)
+{
+ struct device *dev = phy->dev;
+ int ret = 0;
+
+ if (!clk_on)
+ goto close_clocks;
+
+ /* choose 100MHz clk src: Bit[8]==1 pad, Bit[8]==0 pll */
+ hi3670_apb_phy_updatel(phy, 0, PCIE_CLK_SOURCE,
+ SOC_PCIEPHY_CTRL1_ADDR);
+
+ hi3670_pcie_pll_init(phy);
+
+ ret = hi3670_pcie_pll_ctrl(phy, true);
+ if (ret) {
+ dev_err(dev, "Failed to enable pll\n");
+ return -EINVAL;
+ }
+ hi3670_pcie_hp_debounce_gt(phy, true);
+ hi3670_pcie_phyref_gt(phy, true);
+ hi3670_pcie_ioref_gt(phy, true);
+
+ ret = clk_set_rate(phy->aclk, AXI_CLK_FREQ);
+ if (ret) {
+ dev_err(dev, "Failed to set rate\n");
+ goto close_clocks;
+ }
+
+ return 0;
+
+close_clocks:
+ hi3670_pcie_ioref_gt(phy, false);
+ hi3670_pcie_phyref_gt(phy, false);
+ hi3670_pcie_hp_debounce_gt(phy, false);
+
+ hi3670_pcie_pll_ctrl(phy, false);
+
+ return ret;
+}
+
+static bool is_pipe_clk_stable(struct hi3670_pcie_phy *phy)
+{
+ struct device *dev = phy->dev;
+ u32 val;
+ u32 time = PIPE_CLK_STABLE_TIME;
+ u32 pipe_clk_stable = PCIE_IS_CLOCK_STABLE;
+
+ val = hi3670_apb_phy_readl(phy, SOC_PCIEPHY_STATE0_ADDR);
+ while (val & pipe_clk_stable) {
+ mdelay(1);
+ if (!time) {
+ dev_err(dev, "PIPE clk is not stable\n");
+ return false;
+ }
+ time--;
+ val = hi3670_apb_phy_readl(phy, SOC_PCIEPHY_STATE0_ADDR);
+ }
+
+ return true;
+}
+
+static int hi3670_pcie_noc_power(struct hi3670_pcie_phy *phy, bool enable)
+{
+ struct device *dev = phy->dev;
+ u32 time = NOC_POWER_TIME;
+ unsigned int val = NOC_PW_MASK;
+ int rst;
+
+ if (enable)
+ val = NOC_PW_MASK | NOC_PW_SET_BIT;
+ else
+ val = NOC_PW_MASK;
+ rst = enable ? 1 : 0;
+
+ regmap_write(phy->pmctrl, NOC_POWER_IDLEREQ_1, val);
+
+ time = NOC_POWER_TIME;
+ regmap_read(phy->pmctrl, NOC_POWER_IDLE_1, &val);
+ while ((val & NOC_PW_SET_BIT) != rst) {
+ udelay(10);
+ if (!time) {
+ dev_err(dev, "Failed to reverse noc power-status\n");
+ return -EINVAL;
+ }
+ time--;
+ regmap_read(phy->pmctrl, NOC_POWER_IDLE_1, &val);
+ }
+
+ return 0;
+}
+
+static int hi3670_pcie_get_resources_from_pcie(struct hi3670_pcie_phy *phy)
+{
+ struct device_node *pcie_port;
+ struct device *dev = phy->dev;
+ struct device *pcie_dev;
+
+ pcie_port = of_get_child_by_name(dev->parent->of_node, "pcie");
+ if (!pcie_port) {
+ dev_err(dev, "no pcie node found in %s\n",
+ dev->parent->of_node->full_name);
+ return -ENODEV;
+ }
+
+ pcie_dev = bus_find_device_by_of_node(&platform_bus_type, pcie_port);
+ if (!pcie_dev) {
+ dev_err(dev, "Didn't find pcie device\n");
+ return -ENODEV;
+ }
+
+ /*
+ * We might just use NULL instead of the APB name, as the
+ * pcie-kirin currently registers directly just one regmap (although
+ * the DWC driver register other regmaps).
+ *
+ * Yet, it sounds safer to warrant that it will be accessing the
+ * right regmap. So, let's use the named version.
+ */
+ phy->apb = dev_get_regmap(pcie_dev, "kirin_pcie_apb");
+ if (!phy->apb) {
+ dev_err(dev, "Failed to get APB regmap\n");
+ return -ENODEV;
+ }
+
+ return 0;
+}
+
+static int kirin_pcie_clk_ctrl(struct hi3670_pcie_phy *phy, bool enable)
+{
+ int ret = 0;
+
+ if (!enable)
+ goto close_clk;
+
+ ret = clk_set_rate(phy->phy_ref_clk, REF_CLK_FREQ);
+ if (ret)
+ return ret;
+
+ ret = clk_prepare_enable(phy->phy_ref_clk);
+ if (ret)
+ return ret;
+
+ ret = clk_prepare_enable(phy->apb_sys_clk);
+ if (ret)
+ goto apb_sys_fail;
+
+ ret = clk_prepare_enable(phy->apb_phy_clk);
+ if (ret)
+ goto apb_phy_fail;
+
+ ret = clk_prepare_enable(phy->aclk);
+ if (ret)
+ goto aclk_fail;
+
+ ret = clk_prepare_enable(phy->aux_clk);
+ if (ret)
+ goto aux_clk_fail;
+
+ return 0;
+
+close_clk:
+ clk_disable_unprepare(phy->aux_clk);
+aux_clk_fail:
+ clk_disable_unprepare(phy->aclk);
+aclk_fail:
+ clk_disable_unprepare(phy->apb_phy_clk);
+apb_phy_fail:
+ clk_disable_unprepare(phy->apb_sys_clk);
+apb_sys_fail:
+ clk_disable_unprepare(phy->phy_ref_clk);
+
+ return ret;
+}
+
+static int hi3670_pcie_phy_init(struct phy *generic_phy)
+{
+ struct hi3670_pcie_phy *phy = phy_get_drvdata(generic_phy);
+ int ret;
+
+ /*
+ * The code under hi3670_pcie_get_resources_from_pcie() need to
+ * access the reset-gpios and the APB registers, both from the
+ * pcie-kirin driver.
+ *
+ * The APB is obtained via the pcie driver's regmap
+ * Such kind of resource can only be obtained during the PCIe
+ * power_on sequence, as the code inside pcie-kirin needs to
+ * be already probed, as it needs to register the APB regmap.
+ */
+
+ ret = hi3670_pcie_get_resources_from_pcie(phy);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static int hi3670_pcie_phy_power_on(struct phy *generic_phy)
+{
+ struct hi3670_pcie_phy *phy = phy_get_drvdata(generic_phy);
+ int val, ret;
+
+ /* Power supply for Host */
+ regmap_write(phy->sysctrl, SCTRL_PCIE_CMOS_OFFSET, SCTRL_PCIE_CMOS_BIT);
+ usleep_range(TIME_CMOS_MIN, TIME_CMOS_MAX);
+
+ hi3670_pcie_phy_oe_enable(phy, true);
+
+ ret = kirin_pcie_clk_ctrl(phy, true);
+ if (ret)
+ return ret;
+
+ /* ISO disable, PCIeCtrl, PHY assert and clk gate clear */
+ regmap_write(phy->sysctrl, SCTRL_PCIE_ISO_OFFSET, SCTRL_PCIE_ISO_BIT);
+ regmap_write(phy->crgctrl, CRGCTRL_PCIE_ASSERT_OFFSET,
+ CRGCTRL_PCIE_ASSERT_BIT);
+ regmap_write(phy->sysctrl, SCTRL_PCIE_HPCLK_OFFSET,
+ SCTRL_PCIE_HPCLK_BIT);
+
+ hi3670_pcie_natural_cfg(phy);
+
+ ret = hi3670_pcie_allclk_ctrl(phy, true);
+ if (ret)
+ goto disable_clks;
+
+ /* pull down phy_test_powerdown signal */
+ hi3670_apb_phy_updatel(phy, 0, PCIE_PULL_DOWN_PHY_TEST_POWERDOWN,
+ SOC_PCIEPHY_CTRL0_ADDR);
+
+ /* deassert controller perst_n */
+ regmap_read(phy->apb, SOC_PCIECTRL_CTRL12_ADDR, &val);
+ val |= PCIE_DEASSERT_CONTROLLER_PERST;
+ regmap_write(phy->apb, SOC_PCIECTRL_CTRL12_ADDR, val);
+ udelay(10);
+
+ ret = is_pipe_clk_stable(phy);
+ if (!ret)
+ goto disable_clks;
+
+ hi3670_pcie_set_eyeparam(phy);
+
+ ret = hi3670_pcie_noc_power(phy, false);
+ if (ret)
+ goto disable_clks;
+
+ return 0;
+
+disable_clks:
+ kirin_pcie_clk_ctrl(phy, false);
+ return ret;
+}
+
+static int hi3670_pcie_phy_power_off(struct phy *generic_phy)
+{
+ struct hi3670_pcie_phy *phy = phy_get_drvdata(generic_phy);
+
+ hi3670_pcie_phy_oe_enable(phy, false);
+
+ hi3670_pcie_allclk_ctrl(phy, false);
+
+ /* Drop power supply for Host */
+ regmap_write(phy->sysctrl, SCTRL_PCIE_CMOS_OFFSET, 0);
+
+ /*
+ * FIXME: The enabled clocks should be disabled here by calling
+ * kirin_pcie_clk_ctrl(phy, false);
+ * However, some clocks used at Kirin 970 should be marked as
+ * CLK_IS_CRITICAL at clk-hi3670 driver, as powering such clocks off
+ * cause an Asynchronous SError interrupt, which produces panic().
+ * While clk-hi3670 is not fixed, we cannot risk disabling clocks here.
+ */
+
+ return 0;
+}
+
+static const struct phy_ops hi3670_phy_ops = {
+ .init = hi3670_pcie_phy_init,
+ .power_on = hi3670_pcie_phy_power_on,
+ .power_off = hi3670_pcie_phy_power_off,
+ .owner = THIS_MODULE,
+};
+
+static int hi3670_pcie_phy_get_resources(struct hi3670_pcie_phy *phy,
+ struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+
+ /* syscon */
+ phy->crgctrl = syscon_regmap_lookup_by_compatible("hisilicon,hi3670-crgctrl");
+ if (IS_ERR(phy->crgctrl))
+ return PTR_ERR(phy->crgctrl);
+
+ phy->sysctrl = syscon_regmap_lookup_by_compatible("hisilicon,hi3670-sctrl");
+ if (IS_ERR(phy->sysctrl))
+ return PTR_ERR(phy->sysctrl);
+
+ phy->pmctrl = syscon_regmap_lookup_by_compatible("hisilicon,hi3670-pmctrl");
+ if (IS_ERR(phy->sysctrl))
+ return PTR_ERR(phy->sysctrl);
+
+ /* clocks */
+ phy->phy_ref_clk = devm_clk_get(dev, "phy_ref");
+ if (IS_ERR(phy->phy_ref_clk))
+ return PTR_ERR(phy->phy_ref_clk);
+
+ phy->aux_clk = devm_clk_get(dev, "aux");
+ if (IS_ERR(phy->aux_clk))
+ return PTR_ERR(phy->aux_clk);
+
+ phy->apb_phy_clk = devm_clk_get(dev, "apb_phy");
+ if (IS_ERR(phy->apb_phy_clk))
+ return PTR_ERR(phy->apb_phy_clk);
+
+ phy->apb_sys_clk = devm_clk_get(dev, "apb_sys");
+ if (IS_ERR(phy->apb_sys_clk))
+ return PTR_ERR(phy->apb_sys_clk);
+
+ phy->aclk = devm_clk_get(dev, "aclk");
+ if (IS_ERR(phy->aclk))
+ return PTR_ERR(phy->aclk);
+
+ /* registers */
+ phy->base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(phy->base))
+ return PTR_ERR(phy->base);
+
+ hi3670_pcie_get_eyeparam(phy);
+
+ return 0;
+}
+
+static int hi3670_pcie_phy_probe(struct platform_device *pdev)
+{
+ struct phy_provider *phy_provider;
+ struct device *dev = &pdev->dev;
+ struct hi3670_pcie_phy *phy;
+ struct phy *generic_phy;
+ int ret;
+
+ phy = devm_kzalloc(dev, sizeof(*phy), GFP_KERNEL);
+ if (!phy)
+ return -ENOMEM;
+
+ phy->dev = dev;
+
+ ret = hi3670_pcie_phy_get_resources(phy, pdev);
+ if (ret)
+ return ret;
+
+ generic_phy = devm_phy_create(dev, dev->of_node, &hi3670_phy_ops);
+ if (IS_ERR(generic_phy)) {
+ dev_err(dev, "failed to create PHY\n");
+ return PTR_ERR(generic_phy);
+ }
+
+ phy_set_drvdata(generic_phy, phy);
+ phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+
+ return PTR_ERR_OR_ZERO(phy_provider);
+}
+
+static const struct of_device_id hi3670_pcie_phy_match[] = {
+ {
+ .compatible = "hisilicon,hi970-pcie-phy",
+ },
+ {},
+};
+
+static struct platform_driver hi3670_pcie_phy_driver = {
+ .probe = hi3670_pcie_phy_probe,
+ .driver = {
+ .of_match_table = hi3670_pcie_phy_match,
+ .name = "hi3670_pcie_phy",
+ .suppress_bind_attrs = true,
+ }
+};
+builtin_platform_driver(hi3670_pcie_phy_driver);
+
+MODULE_DEVICE_TABLE(of, hi3670_pcie_phy_match);
+MODULE_DESCRIPTION("PCIe phy driver for Kirin 970");
+MODULE_AUTHOR("Mauro Carvalho Chehab <[email protected]>");
+MODULE_AUTHOR("Manivannan Sadhasivam <[email protected]>");
+MODULE_LICENSE("GPL v2");
--
2.31.1

2021-09-28 07:38:02

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: [PATCH v12 04/11] PCI: kirin: Use regmap for APB registers

The PHY layer need to access APB registers too, for Kirin 970.
So, place them into a named regmap.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---

To mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH v12 00/11] at: https://lore.kernel.org/all/[email protected]/

drivers/pci/controller/dwc/pcie-kirin.c | 49 +++++++++++++------------
1 file changed, 26 insertions(+), 23 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-kirin.c b/drivers/pci/controller/dwc/pcie-kirin.c
index 31514a5d4bb4..0ea92a521e1c 100644
--- a/drivers/pci/controller/dwc/pcie-kirin.c
+++ b/drivers/pci/controller/dwc/pcie-kirin.c
@@ -61,8 +61,8 @@ struct kirin_pcie {
enum pcie_kirin_phy_type type;

struct dw_pcie *pci;
+ struct regmap *apb;
struct phy *phy;
- void __iomem *apb_base;
void *phy_priv; /* only for PCIE_KIRIN_INTERNAL_PHY */
};

@@ -340,25 +340,27 @@ static int hi3660_pcie_phy_init(struct platform_device *pdev,
* The non-PHY part starts here
*/

-/* Registers in PCIeCTRL */
-static inline void kirin_apb_ctrl_writel(struct kirin_pcie *kirin_pcie,
- u32 val, u32 reg)
-{
- writel(val, kirin_pcie->apb_base + reg);
-}
-
-static inline u32 kirin_apb_ctrl_readl(struct kirin_pcie *kirin_pcie, u32 reg)
-{
- return readl(kirin_pcie->apb_base + reg);
-}
+static const struct regmap_config pcie_kirin_regmap_conf = {
+ .name = "kirin_pcie_apb",
+ .reg_bits = 32,
+ .val_bits = 32,
+ .reg_stride = 4,
+};

static long kirin_pcie_get_resource(struct kirin_pcie *kirin_pcie,
struct platform_device *pdev)
{
- kirin_pcie->apb_base =
- devm_platform_ioremap_resource_byname(pdev, "apb");
- if (IS_ERR(kirin_pcie->apb_base))
- return PTR_ERR(kirin_pcie->apb_base);
+ struct device *dev = &pdev->dev;
+ void __iomem *apb_base;
+
+ apb_base = devm_platform_ioremap_resource_byname(pdev, "apb");
+ if (IS_ERR(apb_base))
+ return PTR_ERR(apb_base);
+
+ kirin_pcie->apb = devm_regmap_init_mmio(dev, apb_base,
+ &pcie_kirin_regmap_conf);
+ if (IS_ERR(kirin_pcie->apb))
+ return PTR_ERR(kirin_pcie->apb);

return 0;
}
@@ -368,13 +370,13 @@ static void kirin_pcie_sideband_dbi_w_mode(struct kirin_pcie *kirin_pcie,
{
u32 val;

- val = kirin_apb_ctrl_readl(kirin_pcie, SOC_PCIECTRL_CTRL0_ADDR);
+ regmap_read(kirin_pcie->apb, SOC_PCIECTRL_CTRL0_ADDR, &val);
if (on)
val = val | PCIE_ELBI_SLV_DBI_ENABLE;
else
val = val & ~PCIE_ELBI_SLV_DBI_ENABLE;

- kirin_apb_ctrl_writel(kirin_pcie, val, SOC_PCIECTRL_CTRL0_ADDR);
+ regmap_write(kirin_pcie->apb, SOC_PCIECTRL_CTRL0_ADDR, val);
}

static void kirin_pcie_sideband_dbi_r_mode(struct kirin_pcie *kirin_pcie,
@@ -382,13 +384,13 @@ static void kirin_pcie_sideband_dbi_r_mode(struct kirin_pcie *kirin_pcie,
{
u32 val;

- val = kirin_apb_ctrl_readl(kirin_pcie, SOC_PCIECTRL_CTRL1_ADDR);
+ regmap_read(kirin_pcie->apb, SOC_PCIECTRL_CTRL1_ADDR, &val);
if (on)
val = val | PCIE_ELBI_SLV_DBI_ENABLE;
else
val = val & ~PCIE_ELBI_SLV_DBI_ENABLE;

- kirin_apb_ctrl_writel(kirin_pcie, val, SOC_PCIECTRL_CTRL1_ADDR);
+ regmap_write(kirin_pcie->apb, SOC_PCIECTRL_CTRL1_ADDR, val);
}

static int kirin_pcie_rd_own_conf(struct pci_bus *bus, unsigned int devfn,
@@ -448,8 +450,9 @@ static void kirin_pcie_write_dbi(struct dw_pcie *pci, void __iomem *base,
static int kirin_pcie_link_up(struct dw_pcie *pci)
{
struct kirin_pcie *kirin_pcie = to_kirin_pcie(pci);
- u32 val = kirin_apb_ctrl_readl(kirin_pcie, PCIE_APB_PHY_STATUS0);
+ u32 val;

+ regmap_read(kirin_pcie->apb, PCIE_APB_PHY_STATUS0, &val);
if ((val & PCIE_LINKUP_ENABLE) == PCIE_LINKUP_ENABLE)
return 1;

@@ -461,8 +464,8 @@ static int kirin_pcie_start_link(struct dw_pcie *pci)
struct kirin_pcie *kirin_pcie = to_kirin_pcie(pci);

/* assert LTSSM enable */
- kirin_apb_ctrl_writel(kirin_pcie, PCIE_LTSSM_ENABLE_BIT,
- PCIE_APP_LTSSM_ENABLE);
+ regmap_write(kirin_pcie->apb, PCIE_APP_LTSSM_ENABLE,
+ PCIE_LTSSM_ENABLE_BIT);

return 0;
}
--
2.31.1

2021-09-28 07:39:09

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: [PATCH v12 09/11] PCI: kirin: Add power_off support for Kirin 960 PHY

In order to prepare for module unload, add a power_off method
for HiKey 960.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---

To mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH v12 00/11] at: https://lore.kernel.org/all/[email protected]/

drivers/pci/controller/dwc/pcie-kirin.c | 21 ++++++++++++++++++---
1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-kirin.c b/drivers/pci/controller/dwc/pcie-kirin.c
index d5c29a266756..b17a194cf78d 100644
--- a/drivers/pci/controller/dwc/pcie-kirin.c
+++ b/drivers/pci/controller/dwc/pcie-kirin.c
@@ -343,6 +343,18 @@ static int hi3660_pcie_phy_init(struct platform_device *pdev,
return hi3660_pcie_phy_get_resource(phy);
}

+static int hi3660_pcie_phy_power_off(struct kirin_pcie *pcie)
+{
+ struct hi3660_pcie_phy *phy = pcie->phy_priv;
+
+ /* Drop power supply for Host */
+ regmap_write(phy->sysctrl, SCTRL_PCIE_CMOS_OFFSET, 0x00);
+
+ hi3660_pcie_phy_clk_ctrl(phy, false);
+
+ return 0;
+}
+
/*
* The non-PHY part starts here
*/
@@ -560,7 +572,6 @@ static int kirin_pcie_add_bus(struct pci_bus *bus)
return 0;
}

-
static struct pci_ops kirin_pci_ops = {
.read = kirin_pcie_rd_own_conf,
.write = kirin_pcie_wr_own_conf,
@@ -714,8 +725,12 @@ static int kirin_pcie_power_on(struct platform_device *pdev,

return 0;
err:
- if (kirin_pcie->type != PCIE_KIRIN_INTERNAL_PHY)
+ if (kirin_pcie->type == PCIE_KIRIN_INTERNAL_PHY) {
+ hi3660_pcie_phy_power_off(kirin_pcie);
+ } else {
+ phy_power_off(kirin_pcie->phy);
phy_exit(kirin_pcie->phy);
+ }

return ret;
}
@@ -725,7 +740,7 @@ static int __exit kirin_pcie_remove(struct platform_device *pdev)
struct kirin_pcie *kirin_pcie = platform_get_drvdata(pdev);

if (kirin_pcie->type == PCIE_KIRIN_INTERNAL_PHY)
- return 0;
+ return hi3660_pcie_phy_power_off(kirin_pcie);

phy_power_off(kirin_pcie->phy);
phy_exit(kirin_pcie->phy);
--
2.31.1

2021-09-28 07:39:10

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: [PATCH v12 02/11] PCI: kirin: Reorganize the PHY logic inside the driver

The pcie-kirin PCIe driver contains internally a PHY interface for
Kirin 960.

As the next patches will add support for using an external PHY
driver, reorganize the driver in a way that the PHY part
will be self-contained.

This could be moved to a separate PHY driver, but a change
like that would mean a non-backward-compatible DT schema
change.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---

To mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH v12 00/11] at: https://lore.kernel.org/all/[email protected]/

drivers/pci/controller/dwc/pcie-kirin.c | 298 ++++++++++++++----------
1 file changed, 173 insertions(+), 125 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-kirin.c b/drivers/pci/controller/dwc/pcie-kirin.c
index 026fd1e42a55..b4063a3434df 100644
--- a/drivers/pci/controller/dwc/pcie-kirin.c
+++ b/drivers/pci/controller/dwc/pcie-kirin.c
@@ -28,26 +28,16 @@

#define to_kirin_pcie(x) dev_get_drvdata((x)->dev)

-#define REF_CLK_FREQ 100000000
-
/* PCIe ELBI registers */
#define SOC_PCIECTRL_CTRL0_ADDR 0x000
#define SOC_PCIECTRL_CTRL1_ADDR 0x004
-#define SOC_PCIEPHY_CTRL2_ADDR 0x008
-#define SOC_PCIEPHY_CTRL3_ADDR 0x00c
#define PCIE_ELBI_SLV_DBI_ENABLE (0x1 << 21)

/* info located in APB */
#define PCIE_APP_LTSSM_ENABLE 0x01c
-#define PCIE_APB_PHY_CTRL0 0x0
-#define PCIE_APB_PHY_CTRL1 0x4
#define PCIE_APB_PHY_STATUS0 0x400
#define PCIE_LINKUP_ENABLE (0x8020)
#define PCIE_LTSSM_ENABLE_BIT (0x1 << 11)
-#define PIPE_CLK_STABLE (0x1 << 19)
-#define PHY_REF_PAD_BIT (0x1 << 8)
-#define PHY_PWR_DOWN_BIT (0x1 << 22)
-#define PHY_RST_ACK_BIT (0x1 << 16)

/* info located in sysctrl */
#define SCTRL_PCIE_CMOS_OFFSET 0x60
@@ -60,6 +50,29 @@
#define PCIE_DEBOUNCE_PARAM 0xF0F400
#define PCIE_OE_BYPASS (0x3 << 28)

+struct kirin_pcie {
+ struct dw_pcie *pci;
+ struct phy *phy;
+ void __iomem *apb_base;
+ void *phy_priv; /* Needed for Kirin 960 PHY */
+};
+
+/*
+ * Kirin 960 PHY. Can't be split into a PHY driver without changing the
+ * DT schema.
+ */
+
+#define REF_CLK_FREQ 100000000
+
+/* PHY info located in APB */
+#define PCIE_APB_PHY_CTRL0 0x0
+#define PCIE_APB_PHY_CTRL1 0x4
+#define PCIE_APB_PHY_STATUS0 0x400
+#define PIPE_CLK_STABLE BIT(19)
+#define PHY_REF_PAD_BIT BIT(8)
+#define PHY_PWR_DOWN_BIT BIT(22)
+#define PHY_RST_ACK_BIT BIT(16)
+
/* peri_crg ctrl */
#define CRGCTRL_PCIE_ASSERT_OFFSET 0x88
#define CRGCTRL_PCIE_ASSERT_BIT 0x8c000000
@@ -69,8 +82,6 @@
#define REF_2_PERST_MAX 25000
#define PERST_2_ACCESS_MIN 10000
#define PERST_2_ACCESS_MAX 12000
-#define LINK_WAIT_MIN 900
-#define LINK_WAIT_MAX 1000
#define PIPE_CLK_WAIT_MIN 550
#define PIPE_CLK_WAIT_MAX 600
#define TIME_CMOS_MIN 100
@@ -78,118 +89,112 @@
#define TIME_PHY_PD_MIN 10
#define TIME_PHY_PD_MAX 11

-struct kirin_pcie {
- struct dw_pcie *pci;
- void __iomem *apb_base;
- void __iomem *phy_base;
+struct hi3660_pcie_phy {
+ struct device *dev;
+ void __iomem *base;
struct regmap *crgctrl;
struct regmap *sysctrl;
struct clk *apb_sys_clk;
struct clk *apb_phy_clk;
struct clk *phy_ref_clk;
- struct clk *pcie_aclk;
- struct clk *pcie_aux_clk;
+ struct clk *aclk;
+ struct clk *aux_clk;
int gpio_id_reset;
};

-/* Registers in PCIeCTRL */
-static inline void kirin_apb_ctrl_writel(struct kirin_pcie *kirin_pcie,
- u32 val, u32 reg)
-{
- writel(val, kirin_pcie->apb_base + reg);
-}
-
-static inline u32 kirin_apb_ctrl_readl(struct kirin_pcie *kirin_pcie, u32 reg)
-{
- return readl(kirin_pcie->apb_base + reg);
-}
-
/* Registers in PCIePHY */
-static inline void kirin_apb_phy_writel(struct kirin_pcie *kirin_pcie,
+static inline void kirin_apb_phy_writel(struct hi3660_pcie_phy *hi3660_pcie_phy,
u32 val, u32 reg)
{
- writel(val, kirin_pcie->phy_base + reg);
+ writel(val, hi3660_pcie_phy->base + reg);
}

-static inline u32 kirin_apb_phy_readl(struct kirin_pcie *kirin_pcie, u32 reg)
+static inline u32 kirin_apb_phy_readl(struct hi3660_pcie_phy *hi3660_pcie_phy,
+ u32 reg)
{
- return readl(kirin_pcie->phy_base + reg);
+ return readl(hi3660_pcie_phy->base + reg);
}

-static long kirin_pcie_get_clk(struct kirin_pcie *kirin_pcie,
- struct platform_device *pdev)
+static int hi3660_pcie_phy_get_clk(struct hi3660_pcie_phy *phy)
{
- struct device *dev = &pdev->dev;
+ struct device *dev = phy->dev;

- kirin_pcie->phy_ref_clk = devm_clk_get(dev, "pcie_phy_ref");
- if (IS_ERR(kirin_pcie->phy_ref_clk))
- return PTR_ERR(kirin_pcie->phy_ref_clk);
+ phy->phy_ref_clk = devm_clk_get(dev, "pcie_phy_ref");
+ if (IS_ERR(phy->phy_ref_clk))
+ return PTR_ERR(phy->phy_ref_clk);

- kirin_pcie->pcie_aux_clk = devm_clk_get(dev, "pcie_aux");
- if (IS_ERR(kirin_pcie->pcie_aux_clk))
- return PTR_ERR(kirin_pcie->pcie_aux_clk);
+ phy->aux_clk = devm_clk_get(dev, "pcie_aux");
+ if (IS_ERR(phy->aux_clk))
+ return PTR_ERR(phy->aux_clk);

- kirin_pcie->apb_phy_clk = devm_clk_get(dev, "pcie_apb_phy");
- if (IS_ERR(kirin_pcie->apb_phy_clk))
- return PTR_ERR(kirin_pcie->apb_phy_clk);
+ phy->apb_phy_clk = devm_clk_get(dev, "pcie_apb_phy");
+ if (IS_ERR(phy->apb_phy_clk))
+ return PTR_ERR(phy->apb_phy_clk);

- kirin_pcie->apb_sys_clk = devm_clk_get(dev, "pcie_apb_sys");
- if (IS_ERR(kirin_pcie->apb_sys_clk))
- return PTR_ERR(kirin_pcie->apb_sys_clk);
+ phy->apb_sys_clk = devm_clk_get(dev, "pcie_apb_sys");
+ if (IS_ERR(phy->apb_sys_clk))
+ return PTR_ERR(phy->apb_sys_clk);

- kirin_pcie->pcie_aclk = devm_clk_get(dev, "pcie_aclk");
- if (IS_ERR(kirin_pcie->pcie_aclk))
- return PTR_ERR(kirin_pcie->pcie_aclk);
+ phy->aclk = devm_clk_get(dev, "pcie_aclk");
+ if (IS_ERR(phy->aclk))
+ return PTR_ERR(phy->aclk);

return 0;
}

-static long kirin_pcie_get_resource(struct kirin_pcie *kirin_pcie,
- struct platform_device *pdev)
+static int hi3660_pcie_phy_get_resource(struct hi3660_pcie_phy *phy)
{
- kirin_pcie->apb_base =
- devm_platform_ioremap_resource_byname(pdev, "apb");
- if (IS_ERR(kirin_pcie->apb_base))
- return PTR_ERR(kirin_pcie->apb_base);
-
- kirin_pcie->phy_base =
- devm_platform_ioremap_resource_byname(pdev, "phy");
- if (IS_ERR(kirin_pcie->phy_base))
- return PTR_ERR(kirin_pcie->phy_base);
-
- kirin_pcie->crgctrl =
- syscon_regmap_lookup_by_compatible("hisilicon,hi3660-crgctrl");
- if (IS_ERR(kirin_pcie->crgctrl))
- return PTR_ERR(kirin_pcie->crgctrl);
-
- kirin_pcie->sysctrl =
- syscon_regmap_lookup_by_compatible("hisilicon,hi3660-sctrl");
- if (IS_ERR(kirin_pcie->sysctrl))
- return PTR_ERR(kirin_pcie->sysctrl);
+ struct device *dev = phy->dev;
+ struct platform_device *pdev;
+
+ /* registers */
+ pdev = container_of(dev, struct platform_device, dev);
+
+ phy->base = devm_platform_ioremap_resource_byname(pdev, "phy");
+ if (IS_ERR(phy->base))
+ return PTR_ERR(phy->base);
+
+ phy->crgctrl = syscon_regmap_lookup_by_compatible("hisilicon,hi3660-crgctrl");
+ if (IS_ERR(phy->crgctrl))
+ return PTR_ERR(phy->crgctrl);
+
+ phy->sysctrl = syscon_regmap_lookup_by_compatible("hisilicon,hi3660-sctrl");
+ if (IS_ERR(phy->sysctrl))
+ return PTR_ERR(phy->sysctrl);
+
+ /* gpios */
+ phy->gpio_id_reset = of_get_named_gpio(dev->of_node,
+ "reset-gpios", 0);
+ if (phy->gpio_id_reset == -EPROBE_DEFER) {
+ return -EPROBE_DEFER;
+ } else if (!gpio_is_valid(phy->gpio_id_reset)) {
+ dev_err(phy->dev, "unable to get a valid gpio pin\n");
+ return -ENODEV;
+ }

return 0;
}

-static int kirin_pcie_phy_init(struct kirin_pcie *kirin_pcie)
+static int hi3660_pcie_phy_start(struct hi3660_pcie_phy *phy)
{
- struct device *dev = kirin_pcie->pci->dev;
+ struct device *dev = phy->dev;
u32 reg_val;

- reg_val = kirin_apb_phy_readl(kirin_pcie, PCIE_APB_PHY_CTRL1);
+ reg_val = kirin_apb_phy_readl(phy, PCIE_APB_PHY_CTRL1);
reg_val &= ~PHY_REF_PAD_BIT;
- kirin_apb_phy_writel(kirin_pcie, reg_val, PCIE_APB_PHY_CTRL1);
+ kirin_apb_phy_writel(phy, reg_val, PCIE_APB_PHY_CTRL1);

- reg_val = kirin_apb_phy_readl(kirin_pcie, PCIE_APB_PHY_CTRL0);
+ reg_val = kirin_apb_phy_readl(phy, PCIE_APB_PHY_CTRL0);
reg_val &= ~PHY_PWR_DOWN_BIT;
- kirin_apb_phy_writel(kirin_pcie, reg_val, PCIE_APB_PHY_CTRL0);
+ kirin_apb_phy_writel(phy, reg_val, PCIE_APB_PHY_CTRL0);
usleep_range(TIME_PHY_PD_MIN, TIME_PHY_PD_MAX);

- reg_val = kirin_apb_phy_readl(kirin_pcie, PCIE_APB_PHY_CTRL1);
+ reg_val = kirin_apb_phy_readl(phy, PCIE_APB_PHY_CTRL1);
reg_val &= ~PHY_RST_ACK_BIT;
- kirin_apb_phy_writel(kirin_pcie, reg_val, PCIE_APB_PHY_CTRL1);
+ kirin_apb_phy_writel(phy, reg_val, PCIE_APB_PHY_CTRL1);

usleep_range(PIPE_CLK_WAIT_MIN, PIPE_CLK_WAIT_MAX);
- reg_val = kirin_apb_phy_readl(kirin_pcie, PCIE_APB_PHY_STATUS0);
+ reg_val = kirin_apb_phy_readl(phy, PCIE_APB_PHY_STATUS0);
if (reg_val & PIPE_CLK_STABLE) {
dev_err(dev, "PIPE clk is not stable\n");
return -EINVAL;
@@ -198,105 +203,157 @@ static int kirin_pcie_phy_init(struct kirin_pcie *kirin_pcie)
return 0;
}

-static void kirin_pcie_oe_enable(struct kirin_pcie *kirin_pcie)
+static void hi3660_pcie_phy_oe_enable(struct hi3660_pcie_phy *phy)
{
u32 val;

- regmap_read(kirin_pcie->sysctrl, SCTRL_PCIE_OE_OFFSET, &val);
+ regmap_read(phy->sysctrl, SCTRL_PCIE_OE_OFFSET, &val);
val |= PCIE_DEBOUNCE_PARAM;
val &= ~PCIE_OE_BYPASS;
- regmap_write(kirin_pcie->sysctrl, SCTRL_PCIE_OE_OFFSET, val);
+ regmap_write(phy->sysctrl, SCTRL_PCIE_OE_OFFSET, val);
}

-static int kirin_pcie_clk_ctrl(struct kirin_pcie *kirin_pcie, bool enable)
+static int hi3660_pcie_phy_clk_ctrl(struct hi3660_pcie_phy *phy, bool enable)
{
int ret = 0;

if (!enable)
goto close_clk;

- ret = clk_set_rate(kirin_pcie->phy_ref_clk, REF_CLK_FREQ);
+ ret = clk_set_rate(phy->phy_ref_clk, REF_CLK_FREQ);
if (ret)
return ret;

- ret = clk_prepare_enable(kirin_pcie->phy_ref_clk);
+ ret = clk_prepare_enable(phy->phy_ref_clk);
if (ret)
return ret;

- ret = clk_prepare_enable(kirin_pcie->apb_sys_clk);
+ ret = clk_prepare_enable(phy->apb_sys_clk);
if (ret)
goto apb_sys_fail;

- ret = clk_prepare_enable(kirin_pcie->apb_phy_clk);
+ ret = clk_prepare_enable(phy->apb_phy_clk);
if (ret)
goto apb_phy_fail;

- ret = clk_prepare_enable(kirin_pcie->pcie_aclk);
+ ret = clk_prepare_enable(phy->aclk);
if (ret)
goto aclk_fail;

- ret = clk_prepare_enable(kirin_pcie->pcie_aux_clk);
+ ret = clk_prepare_enable(phy->aux_clk);
if (ret)
goto aux_clk_fail;

return 0;

close_clk:
- clk_disable_unprepare(kirin_pcie->pcie_aux_clk);
+ clk_disable_unprepare(phy->aux_clk);
aux_clk_fail:
- clk_disable_unprepare(kirin_pcie->pcie_aclk);
+ clk_disable_unprepare(phy->aclk);
aclk_fail:
- clk_disable_unprepare(kirin_pcie->apb_phy_clk);
+ clk_disable_unprepare(phy->apb_phy_clk);
apb_phy_fail:
- clk_disable_unprepare(kirin_pcie->apb_sys_clk);
+ clk_disable_unprepare(phy->apb_sys_clk);
apb_sys_fail:
- clk_disable_unprepare(kirin_pcie->phy_ref_clk);
+ clk_disable_unprepare(phy->phy_ref_clk);

return ret;
}

-static int kirin_pcie_power_on(struct kirin_pcie *kirin_pcie)
+static int hi3660_pcie_phy_power_on(struct kirin_pcie *pcie)
{
+ struct hi3660_pcie_phy *phy = pcie->phy_priv;
int ret;

/* Power supply for Host */
- regmap_write(kirin_pcie->sysctrl,
+ regmap_write(phy->sysctrl,
SCTRL_PCIE_CMOS_OFFSET, SCTRL_PCIE_CMOS_BIT);
usleep_range(TIME_CMOS_MIN, TIME_CMOS_MAX);
- kirin_pcie_oe_enable(kirin_pcie);

- ret = kirin_pcie_clk_ctrl(kirin_pcie, true);
+ hi3660_pcie_phy_oe_enable(phy);
+
+ ret = hi3660_pcie_phy_clk_ctrl(phy, true);
if (ret)
return ret;

/* ISO disable, PCIeCtrl, PHY assert and clk gate clear */
- regmap_write(kirin_pcie->sysctrl,
+ regmap_write(phy->sysctrl,
SCTRL_PCIE_ISO_OFFSET, SCTRL_PCIE_ISO_BIT);
- regmap_write(kirin_pcie->crgctrl,
+ regmap_write(phy->crgctrl,
CRGCTRL_PCIE_ASSERT_OFFSET, CRGCTRL_PCIE_ASSERT_BIT);
- regmap_write(kirin_pcie->sysctrl,
+ regmap_write(phy->sysctrl,
SCTRL_PCIE_HPCLK_OFFSET, SCTRL_PCIE_HPCLK_BIT);

- ret = kirin_pcie_phy_init(kirin_pcie);
+ ret = hi3660_pcie_phy_start(phy);
if (ret)
- goto close_clk;
+ goto disable_clks;

/* perst assert Endpoint */
- if (!gpio_request(kirin_pcie->gpio_id_reset, "pcie_perst")) {
+ if (!gpio_request(phy->gpio_id_reset, "pcie_perst")) {
usleep_range(REF_2_PERST_MIN, REF_2_PERST_MAX);
- ret = gpio_direction_output(kirin_pcie->gpio_id_reset, 1);
+ ret = gpio_direction_output(phy->gpio_id_reset, 1);
if (ret)
- goto close_clk;
+ goto disable_clks;
usleep_range(PERST_2_ACCESS_MIN, PERST_2_ACCESS_MAX);
-
return 0;
}

-close_clk:
- kirin_pcie_clk_ctrl(kirin_pcie, false);
+disable_clks:
+ hi3660_pcie_phy_clk_ctrl(phy, false);
return ret;
}

+static int hi3660_pcie_phy_init(struct platform_device *pdev,
+ struct kirin_pcie *pcie)
+{
+ struct device *dev = &pdev->dev;
+ struct hi3660_pcie_phy *phy;
+ int ret;
+
+ phy = devm_kzalloc(dev, sizeof(*phy), GFP_KERNEL);
+ if (!phy)
+ return -ENOMEM;
+
+ pcie->phy_priv = phy;
+ phy->dev = dev;
+
+ /* registers */
+ pdev = container_of(dev, struct platform_device, dev);
+
+ ret = hi3660_pcie_phy_get_clk(phy);
+ if (ret)
+ return ret;
+
+ return hi3660_pcie_phy_get_resource(phy);
+}
+
+/*
+ * The non-PHY part starts here
+ */
+
+/* Registers in PCIeCTRL */
+static inline void kirin_apb_ctrl_writel(struct kirin_pcie *kirin_pcie,
+ u32 val, u32 reg)
+{
+ writel(val, kirin_pcie->apb_base + reg);
+}
+
+static inline u32 kirin_apb_ctrl_readl(struct kirin_pcie *kirin_pcie, u32 reg)
+{
+ return readl(kirin_pcie->apb_base + reg);
+}
+
+static long kirin_pcie_get_resource(struct kirin_pcie *kirin_pcie,
+ struct platform_device *pdev)
+{
+ kirin_pcie->apb_base =
+ devm_platform_ioremap_resource_byname(pdev, "apb");
+ if (IS_ERR(kirin_pcie->apb_base))
+ return PTR_ERR(kirin_pcie->apb_base);
+
+ return 0;
+}
+
static void kirin_pcie_sideband_dbi_w_mode(struct kirin_pcie *kirin_pcie,
bool on)
{
@@ -444,7 +501,7 @@ static int kirin_pcie_probe(struct platform_device *pdev)
pci->pp.ops = &kirin_pcie_host_ops;
kirin_pcie->pci = pci;

- ret = kirin_pcie_get_clk(kirin_pcie, pdev);
+ ret = hi3660_pcie_phy_init(pdev, kirin_pcie);
if (ret)
return ret;

@@ -452,16 +509,7 @@ static int kirin_pcie_probe(struct platform_device *pdev)
if (ret)
return ret;

- kirin_pcie->gpio_id_reset = of_get_named_gpio(dev->of_node,
- "reset-gpios", 0);
- if (kirin_pcie->gpio_id_reset == -EPROBE_DEFER) {
- return -EPROBE_DEFER;
- } else if (!gpio_is_valid(kirin_pcie->gpio_id_reset)) {
- dev_err(dev, "unable to get a valid gpio pin\n");
- return -ENODEV;
- }
-
- ret = kirin_pcie_power_on(kirin_pcie);
+ ret = hi3660_pcie_phy_power_on(kirin_pcie);
if (ret)
return ret;

@@ -479,8 +527,8 @@ static struct platform_driver kirin_pcie_driver = {
.probe = kirin_pcie_probe,
.driver = {
.name = "kirin-pcie",
- .of_match_table = kirin_pcie_match,
- .suppress_bind_attrs = true,
+ .of_match_table = kirin_pcie_match,
+ .suppress_bind_attrs = true,
},
};
builtin_platform_driver(kirin_pcie_driver);
--
2.31.1

2021-09-28 07:39:25

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: [PATCH v12 10/11] PCI: kirin: fix poweroff sequence

This driver currently doesn't call dw_pcie_host_deinit()
at the .remove() callback. This can cause an OOPS if the driver
is unbound.

While here, add a poweroff function, in order to abstract
between the internal and external PHY logic.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---

To mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH v12 00/11] at: https://lore.kernel.org/all/[email protected]/

drivers/pci/controller/dwc/pcie-kirin.c | 30 ++++++++++++++++---------
1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-kirin.c b/drivers/pci/controller/dwc/pcie-kirin.c
index b17a194cf78d..ffc63d12f8ed 100644
--- a/drivers/pci/controller/dwc/pcie-kirin.c
+++ b/drivers/pci/controller/dwc/pcie-kirin.c
@@ -680,6 +680,23 @@ static const struct dw_pcie_host_ops kirin_pcie_host_ops = {
.host_init = kirin_pcie_host_init,
};

+static int kirin_pcie_power_off(struct kirin_pcie *kirin_pcie)
+{
+ int i;
+
+ if (kirin_pcie->type == PCIE_KIRIN_INTERNAL_PHY)
+ return hi3660_pcie_phy_power_off(kirin_pcie);
+
+ for (i = 0; i < kirin_pcie->n_gpio_clkreq; i++) {
+ gpio_direction_output(kirin_pcie->gpio_id_clkreq[i], 1);
+ }
+
+ phy_power_off(kirin_pcie->phy);
+ phy_exit(kirin_pcie->phy);
+
+ return 0;
+}
+
static int kirin_pcie_power_on(struct platform_device *pdev,
struct kirin_pcie *kirin_pcie)
{
@@ -725,12 +742,7 @@ static int kirin_pcie_power_on(struct platform_device *pdev,

return 0;
err:
- if (kirin_pcie->type == PCIE_KIRIN_INTERNAL_PHY) {
- hi3660_pcie_phy_power_off(kirin_pcie);
- } else {
- phy_power_off(kirin_pcie->phy);
- phy_exit(kirin_pcie->phy);
- }
+ kirin_pcie_power_off(kirin_pcie);

return ret;
}
@@ -739,11 +751,9 @@ static int __exit kirin_pcie_remove(struct platform_device *pdev)
{
struct kirin_pcie *kirin_pcie = platform_get_drvdata(pdev);

- if (kirin_pcie->type == PCIE_KIRIN_INTERNAL_PHY)
- return hi3660_pcie_phy_power_off(kirin_pcie);
+ dw_pcie_host_deinit(&kirin_pcie->pci->pp);

- phy_power_off(kirin_pcie->phy);
- phy_exit(kirin_pcie->phy);
+ kirin_pcie_power_off(kirin_pcie);

return 0;
}
--
2.31.1

2021-09-28 07:39:29

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: [PATCH v12 11/11] PCI: kirin: Allow removing the driver

Now that everything is in place at the poweroff sequence,
this driver can use module_platform_driver(), which allows
it to be removed.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---

To mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH v12 00/11] at: https://lore.kernel.org/all/[email protected]/

drivers/pci/controller/dwc/pcie-kirin.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pcie-kirin.c b/drivers/pci/controller/dwc/pcie-kirin.c
index ffc63d12f8ed..efab2af093b6 100644
--- a/drivers/pci/controller/dwc/pcie-kirin.c
+++ b/drivers/pci/controller/dwc/pcie-kirin.c
@@ -828,7 +828,7 @@ static struct platform_driver kirin_pcie_driver = {
.suppress_bind_attrs = true,
},
};
-builtin_platform_driver(kirin_pcie_driver);
+module_platform_driver(kirin_pcie_driver);

MODULE_DEVICE_TABLE(of, kirin_pcie_match);
MODULE_DESCRIPTION("PCIe host controller driver for Kirin Phone SoCs");
--
2.31.1

2021-10-01 08:15:59

by Vinod Koul

[permalink] [raw]
Subject: Re: [PATCH v12 01/11] phy: HiSilicon: Add driver for Kirin 970 PCIe PHY

On 28-09-21, 09:34, Mauro Carvalho Chehab wrote:
> The Kirin 970 PHY is somewhat similar to the Kirin 960, but it
> does a lot more. Add the needed bits for PCIe to start working on
> HiKey 970 boards.

Applied, thanks

--
~Vinod

2021-10-05 09:27:44

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: Re: [PATCH v12 00/11] Add support for Hikey 970 PCIe

Hi Bjorn,

Em Tue, 28 Sep 2021 09:34:10 +0200
Mauro Carvalho Chehab <[email protected]> escreveu:

> The pcie-kirin PCIe driver contains internally a PHY interface for
> Kirin 960, but it misses support for Kirin 970.
>
> Patch1 contains a PHY for Kirin 970 PCIe.
>
> The remaining patches add support for Kirin 970 at the pcie-kirin driver, and
> add the needed logic to compile it as module and to allow to dynamically
> remove the driver in runtime.
>
> Tested on HiKey970:
>
> # lspci -D -PP
> 0000:00:00.0 PCI bridge: Huawei Technologies Co., Ltd. Device 3670 (rev 01)
> 0000:00:00.0/01:00.0 PCI bridge: PLX Technology, Inc. PEX 8606 6 Lane, 6 Port PCI Express Gen 2 (5.0 GT/s) Switch (rev ba)
> 0000:00:00.0/01:00.0/02:01.0 PCI bridge: PLX Technology, Inc. PEX 8606 6 Lane, 6 Port PCI Express Gen 2 (5.0 GT/s) Switch (rev ba)
> 0000:00:00.0/01:00.0/02:04.0 PCI bridge: PLX Technology, Inc. PEX 8606 6 Lane, 6 Port PCI Express Gen 2 (5.0 GT/s) Switch (rev ba)
> 0000:00:00.0/01:00.0/02:05.0 PCI bridge: PLX Technology, Inc. PEX 8606 6 Lane, 6 Port PCI Express Gen 2 (5.0 GT/s) Switch (rev ba)
> 0000:00:00.0/01:00.0/02:07.0 PCI bridge: PLX Technology, Inc. PEX 8606 6 Lane, 6 Port PCI Express Gen 2 (5.0 GT/s) Switch (rev ba)
> 0000:00:00.0/01:00.0/02:09.0 PCI bridge: PLX Technology, Inc. PEX 8606 6 Lane, 6 Port PCI Express Gen 2 (5.0 GT/s) Switch (rev ba)
> 0000:00:00.0/01:00.0/02:01.0/03:00.0 Non-Volatile memory controller: Samsung Electronics Co Ltd Device a809
> 0000:00:00.0/01:00.0/02:07.0/06:00.0 Ethernet controller: Realtek Semiconductor Co., Ltd. RTL8111/8168/8411 PCI Express Gigabit Ethernet Controller (rev 07)
>
> Tested on HiKey960:
>
> # lspci -D
> 0000:00:00.0 PCI bridge: Huawei Technologies Co., Ltd. Device 3660 (rev 01)
>
> ---
>
> v12:
> - Change a comment at patch 1 to not use c99 style.
>
> v11:
> - patch 5 changed to use the right PCIe topology
> - all other patches are identical to v10.
>
> v10:
> - patch 1: dropped magic numbers from PHY driver
> - patch 5: allow pcie child nodes without reset-gpios
> - all other patches are identical to v9.
>
> v9:
> - Did some cleanups at patches 1 and 5
>

As the DT changes needed by HiKey 970 PCIe support are already upstream:

commit cfcf126fc6795e843d090d98754391ece55e8b0c
Author: Mauro Carvalho Chehab <[email protected]>
AuthorDate: Wed Aug 4 09:18:56 2021 +0200
Commit: Rob Herring <[email protected]>
CommitDate: Mon Aug 16 16:00:52 2021 -0500

dt-bindings: PCI: kirin: Add support for Kirin970

Add a new compatible, plus the new bindings needed by
HiKey970 board.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
Link: https://lore.kernel.org/r/875a4571e253040d3885ee1f37467b0bade7361b.1628061310.git.mchehab+huawei@kernel.org
Signed-off-by: Rob Herring <[email protected]>

>
> Mauro Carvalho Chehab (11):
> phy: HiSilicon: Add driver for Kirin 970 PCIe PHY

And the PHY patch was already accepted and merged at today's
linux-next:

commit 73075011ffff876de8516a1e583dc41869293da9
Author: Mauro Carvalho Chehab <[email protected]>
AuthorDate: Tue Sep 28 09:34:11 2021 +0200
Commit: Vinod Koul <[email protected]>
CommitDate: Fri Oct 1 13:42:18 2021 +0530

phy: HiSilicon: Add driver for Kirin 970 PCIe PHY

The Kirin 970 PHY is somewhat similar to the Kirin 960, but it
does a lot more. Add the needed bits for PCIe to start working on
HiKey 970 boards.

Co-developed-by: Manivannan Sadhasivam <[email protected]>
Signed-off-by: Manivannan Sadhasivam <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
Link: https://lore.kernel.org/r/b7a4ff41b57d861b003f1a00cae81f3d226fbe18.1632814194.git.mchehab+huawei@kernel.org
Signed-off-by: Vinod Koul <[email protected]>

> PCI: kirin: Reorganize the PHY logic inside the driver
> PCI: kirin: Add support for a PHY layer
> PCI: kirin: Use regmap for APB registers
> PCI: kirin: Add support for bridge slot DT schema
> PCI: kirin: Add Kirin 970 compatible
> PCI: kirin: Add MODULE_* macros
> PCI: kirin: Allow building it as a module
> PCI: kirin: Add power_off support for Kirin 960 PHY
> PCI: kirin: fix poweroff sequence
> PCI: kirin: Allow removing the driver

I guess everything is already satisfying the review feedbacks.
If so, could you please merge the PCI ones?

>
> drivers/pci/controller/dwc/Kconfig | 2 +-
> drivers/pci/controller/dwc/pcie-kirin.c | 644 +++++++++++++-----
> drivers/phy/hisilicon/Kconfig | 10 +
> drivers/phy/hisilicon/Makefile | 1 +
> drivers/phy/hisilicon/phy-hi3670-pcie.c | 845 ++++++++++++++++++++++++
> 5 files changed, 1354 insertions(+), 148 deletions(-)
> create mode 100644 drivers/phy/hisilicon/phy-hi3670-pcie.c

Thanks,
Mauro

2021-10-05 18:25:10

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v12 00/11] Add support for Hikey 970 PCIe

[+cc Lorenzo]

On Tue, Oct 05, 2021 at 11:24:48AM +0200, Mauro Carvalho Chehab wrote:
> Hi Bjorn,
>
> Em Tue, 28 Sep 2021 09:34:10 +0200
> Mauro Carvalho Chehab <[email protected]> escreveu:

> > PCI: kirin: Reorganize the PHY logic inside the driver
> > PCI: kirin: Add support for a PHY layer
> > PCI: kirin: Use regmap for APB registers
> > PCI: kirin: Add support for bridge slot DT schema
> > PCI: kirin: Add Kirin 970 compatible
> > PCI: kirin: Add MODULE_* macros
> > PCI: kirin: Allow building it as a module
> > PCI: kirin: Add power_off support for Kirin 960 PHY
> > PCI: kirin: fix poweroff sequence
> > PCI: kirin: Allow removing the driver
>
> I guess everything is already satisfying the review feedbacks.
> If so, could you please merge the PCI ones?

Lorenzo takes care of the native host bridge drivers, so I'm sure this
is on his list. I added him to cc: in case not.

2021-10-05 20:33:48

by Pali Rohár

[permalink] [raw]
Subject: Re: [PATCH v12 03/11] PCI: kirin: Add support for a PHY layer

Hello!

On Tuesday 28 September 2021 09:34:13 Mauro Carvalho Chehab wrote:
> The pcie-kirin driver contains both PHY and generic PCI driver
> on it.
>
> The best would be, instead, to support a PCI PHY driver, making
> the driver more generic.
>
> However, it is too late to remove the Kirin 960 PHY, as a change
> like that would make the DT schema incompatible with past versions.

I have not looked deeply at it. But is not it really possible to declare
PHY node in DTS file with backward compatible manner? Or cannot Rob help
with it (maybe there was similar issue in past with other driver)?

I was fixing something similar, address space defined in DTS was used by
two HW blocks: clock and UART. And I was able to make both DTS file and
driver backward compatible.

2021-10-05 22:37:16

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: Re: [PATCH v12 03/11] PCI: kirin: Add support for a PHY layer

Hi Pali,

Em Tue, 5 Oct 2021 22:31:48 +0200
Pali Rohár <[email protected]> escreveu:

> Hello!
>
> On Tuesday 28 September 2021 09:34:13 Mauro Carvalho Chehab wrote:
> > The pcie-kirin driver contains both PHY and generic PCI driver
> > on it.
> >
> > The best would be, instead, to support a PCI PHY driver, making
> > the driver more generic.
> >
> > However, it is too late to remove the Kirin 960 PHY, as a change
> > like that would make the DT schema incompatible with past versions.
>
> I have not looked deeply at it. But is not it really possible to declare
> PHY node in DTS file with backward compatible manner? Or cannot Rob help
> with it (maybe there was similar issue in past with other driver)?

It would be possible to split the Kirin 960 PHY into drivers/phy. It is
also possible to hack the phy driver to search for the PHY-specific data
inside the PCI compatible string (I did wrote some patches doing that
and sent as a RFC several months ago), but the problem is that the
PHY driver won't be probed without adding a new compatible inside the
DT schema. By doing that, the schema will be incompatible.

In any case, the patches on this series split all PHY-specific code
inside the driver on a separate part of the source code. Moving it
to a new driver would be easy once someone comes with a solution
to add some new method at the PHY layer that would allow to load a
new module without having a compatible for it.

> I was fixing something similar, address space defined in DTS was used by
> two HW blocks: clock and UART. And I was able to make both DTS file and
> driver backward compatible.

Thanks,
Mauro

2021-10-07 14:43:29

by Lorenzo Pieralisi

[permalink] [raw]
Subject: Re: [PATCH v12 00/11] Add support for Hikey 970 PCIe

On Tue, Oct 05, 2021 at 01:23:21PM -0500, Bjorn Helgaas wrote:
> [+cc Lorenzo]
>
> On Tue, Oct 05, 2021 at 11:24:48AM +0200, Mauro Carvalho Chehab wrote:
> > Hi Bjorn,
> >
> > Em Tue, 28 Sep 2021 09:34:10 +0200
> > Mauro Carvalho Chehab <[email protected]> escreveu:
>
> > > PCI: kirin: Reorganize the PHY logic inside the driver
> > > PCI: kirin: Add support for a PHY layer
> > > PCI: kirin: Use regmap for APB registers
> > > PCI: kirin: Add support for bridge slot DT schema
> > > PCI: kirin: Add Kirin 970 compatible
> > > PCI: kirin: Add MODULE_* macros
> > > PCI: kirin: Allow building it as a module
> > > PCI: kirin: Add power_off support for Kirin 960 PHY
> > > PCI: kirin: fix poweroff sequence
> > > PCI: kirin: Allow removing the driver
> >
> > I guess everything is already satisfying the review feedbacks.
> > If so, could you please merge the PCI ones?
>
> Lorenzo takes care of the native host bridge drivers, so I'm sure this
> is on his list. I added him to cc: in case not.

Ideally I'd like to see these patches ACKed/Review-ed by the kirin
maintainers - that's what I was waiting for and that's what they
are there for.

Having said that, I will keep an eye on this series so that we
can hopefully queue it for v5.16.

Lorenzo

2021-10-08 10:56:29

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: Re: [PATCH v12 00/11] Add support for Hikey 970 PCIe

Hi Lorenzo,

Em Thu, 7 Oct 2021 15:41:03 +0100
Lorenzo Pieralisi <[email protected]> escreveu:

> On Tue, Oct 05, 2021 at 01:23:21PM -0500, Bjorn Helgaas wrote:
> > [+cc Lorenzo]
> >
> > On Tue, Oct 05, 2021 at 11:24:48AM +0200, Mauro Carvalho Chehab wrote:
> > > Hi Bjorn,
> > >
> > > Em Tue, 28 Sep 2021 09:34:10 +0200
> > > Mauro Carvalho Chehab <[email protected]> escreveu:
> >
> > > > PCI: kirin: Reorganize the PHY logic inside the driver
> > > > PCI: kirin: Add support for a PHY layer
> > > > PCI: kirin: Use regmap for APB registers
> > > > PCI: kirin: Add support for bridge slot DT schema
> > > > PCI: kirin: Add Kirin 970 compatible
> > > > PCI: kirin: Add MODULE_* macros
> > > > PCI: kirin: Allow building it as a module
> > > > PCI: kirin: Add power_off support for Kirin 960 PHY
> > > > PCI: kirin: fix poweroff sequence
> > > > PCI: kirin: Allow removing the driver
> > >
> > > I guess everything is already satisfying the review feedbacks.
> > > If so, could you please merge the PCI ones?
> >
> > Lorenzo takes care of the native host bridge drivers, so I'm sure this
> > is on his list. I added him to cc: in case not.
>
> Ideally I'd like to see these patches ACKed/Review-ed by the kirin
> maintainers - that's what I was waiting for and that's what they
> are there for.
>
> Having said that, I will keep an eye on this series so that we
> can hopefully queue it for v5.16.

Not sure if you received the e-mail from Xiaowei with his ack.

At least here, I only received on my internal e-mail (perhaps because
the original e-mail was base64-encoded with gb2312 charset).

So, let me forward his answer to you, c/c the mailing lists.

Thanks,
Mauro

-------- Forwarded Message --------
From: Songxiaowei (Kirin_DRV) <[email protected]>
To: Lorenzo Pieralisi <[email protected]>, Bjorn Helgaas <[email protected]>
CC: Mauro Carvalho Chehab <[email protected]>, Bjorn Helgaas <[email protected]>, Linuxarm <[email protected]>, Mauro Carvalho Chehab <[email protected]>, Krzysztof Wilczyński <[email protected]>, Wangbinghui (Biggio, Kirin_DRV) <[email protected]>, Rob Herring <[email protected]>, [email protected] <[email protected]>, [email protected] <[email protected]>, [email protected] <[email protected]>, Kongfei <[email protected]>
Subject: Re: [PATCH v12 00/11] Add support for Hikey 970 PCIe
Date: Fri, 8 Oct 2021 11:45:06 +0100
Message-ID: <[email protected]>

Hi Bjorn,

ACKed, it seems ok to me and Binghui.

Thanks a lot.

B. R.

-----邮件原件-----
发件人: Lorenzo Pieralisi [mailto:[email protected]]
发送时间: 2021年10月7日 22:41
收件人: Bjorn Helgaas <[email protected]>
抄送: Mauro Carvalho Chehab <[email protected]>; Bjorn Helgaas <[email protected]>; Linuxarm <[email protected]>; Mauro Carvalho Chehab <[email protected]>; Krzysztof Wilczyński <[email protected]>; Wangbinghui (Biggio, Kirin_DRV) <[email protected]>; Rob Herring <[email protected]>; Songxiaowei (Kirin_DRV) <[email protected]>; [email protected]; [email protected]; [email protected]
主题: Re: [PATCH v12 00/11] Add support for Hikey 970 PCIe

On Tue, Oct 05, 2021 at 01:23:21PM -0500, Bjorn Helgaas wrote:
> [+cc Lorenzo]
>
> On Tue, Oct 05, 2021 at 11:24:48AM +0200, Mauro Carvalho Chehab wrote:
> > Hi Bjorn,
> >
> > Em Tue, 28 Sep 2021 09:34:10 +0200
> > Mauro Carvalho Chehab <[email protected]> escreveu:
>
> > > PCI: kirin: Reorganize the PHY logic inside the driver
> > > PCI: kirin: Add support for a PHY layer
> > > PCI: kirin: Use regmap for APB registers
> > > PCI: kirin: Add support for bridge slot DT schema
> > > PCI: kirin: Add Kirin 970 compatible
> > > PCI: kirin: Add MODULE_* macros
> > > PCI: kirin: Allow building it as a module
> > > PCI: kirin: Add power_off support for Kirin 960 PHY
> > > PCI: kirin: fix poweroff sequence
> > > PCI: kirin: Allow removing the driver
> >
> > I guess everything is already satisfying the review feedbacks.
> > If so, could you please merge the PCI ones?
>
> Lorenzo takes care of the native host bridge drivers, so I'm sure this
> is on his list. I added him to cc: in case not.

Ideally I'd like to see these patches ACKed/Review-ed by the kirin maintainers - that's what I was waiting for and that's what they are there for.

Having said that, I will keep an eye on this series so that we can hopefully queue it for v5.16.

Lorenzo


2021-10-08 17:37:13

by Lorenzo Pieralisi

[permalink] [raw]
Subject: Re: [PATCH v12 00/11] Add support for Hikey 970 PCIe

On Fri, Oct 08, 2021 at 12:55:21PM +0200, Mauro Carvalho Chehab wrote:
> Hi Lorenzo,
>
> Em Thu, 7 Oct 2021 15:41:03 +0100
> Lorenzo Pieralisi <[email protected]> escreveu:
>
> > On Tue, Oct 05, 2021 at 01:23:21PM -0500, Bjorn Helgaas wrote:
> > > [+cc Lorenzo]
> > >
> > > On Tue, Oct 05, 2021 at 11:24:48AM +0200, Mauro Carvalho Chehab wrote:
> > > > Hi Bjorn,
> > > >
> > > > Em Tue, 28 Sep 2021 09:34:10 +0200
> > > > Mauro Carvalho Chehab <[email protected]> escreveu:
> > >
> > > > > PCI: kirin: Reorganize the PHY logic inside the driver
> > > > > PCI: kirin: Add support for a PHY layer
> > > > > PCI: kirin: Use regmap for APB registers
> > > > > PCI: kirin: Add support for bridge slot DT schema
> > > > > PCI: kirin: Add Kirin 970 compatible
> > > > > PCI: kirin: Add MODULE_* macros
> > > > > PCI: kirin: Allow building it as a module
> > > > > PCI: kirin: Add power_off support for Kirin 960 PHY
> > > > > PCI: kirin: fix poweroff sequence
> > > > > PCI: kirin: Allow removing the driver
> > > >
> > > > I guess everything is already satisfying the review feedbacks.
> > > > If so, could you please merge the PCI ones?
> > >
> > > Lorenzo takes care of the native host bridge drivers, so I'm sure this
> > > is on his list. I added him to cc: in case not.
> >
> > Ideally I'd like to see these patches ACKed/Review-ed by the kirin
> > maintainers - that's what I was waiting for and that's what they
> > are there for.
> >
> > Having said that, I will keep an eye on this series so that we
> > can hopefully queue it for v5.16.
>
> Not sure if you received the e-mail from Xiaowei with his ack.

I have not (and it did not make it to linux-pci either).

> At least here, I only received on my internal e-mail (perhaps because
> the original e-mail was base64-encoded with gb2312 charset).
>
> So, let me forward his answer to you, c/c the mailing lists.

Patches should be acked with tags that tooling recognize, this
would help me.

> Thanks,
> Mauro
>
> -------- Forwarded Message --------
> From: Songxiaowei (Kirin_DRV) <[email protected]>
> To: Lorenzo Pieralisi <[email protected]>, Bjorn Helgaas <[email protected]>
> CC: Mauro Carvalho Chehab <[email protected]>, Bjorn Helgaas <[email protected]>, Linuxarm <[email protected]>, Mauro Carvalho Chehab <[email protected]>, Krzysztof Wilczyński <[email protected]>, Wangbinghui (Biggio, Kirin_DRV) <[email protected]>, Rob Herring <[email protected]>, [email protected] <[email protected]>, [email protected] <[email protected]>, [email protected] <[email protected]>, Kongfei <[email protected]>
> Subject: Re: [PATCH v12 00/11] Add support for Hikey 970 PCIe
> Date: Fri, 8 Oct 2021 11:45:06 +0100
> Message-ID: <[email protected]>
>
> Hi Bjorn,
>
> ACKed, it seems ok to me and Binghui.

For Xiaowei:

https://www.kernel.org/doc/html/latest/process/submitting-patches.html

and your email must make it to the mailing list; if it does not
it does not exist as far as I am concerned. I will apply the
ACK manually for this time but let's keep this in mind please.

Thanks,
Lorenzo
>
> Thanks a lot.
>
> B. R.
>
> -----邮件原件-----
> 发件人: Lorenzo Pieralisi [mailto:[email protected]]
> 发送时间: 2021年10月7日 22:41
> 收件人: Bjorn Helgaas <[email protected]>
> 抄送: Mauro Carvalho Chehab <[email protected]>; Bjorn Helgaas <[email protected]>; Linuxarm <[email protected]>; Mauro Carvalho Chehab <[email protected]>; Krzysztof Wilczyński <[email protected]>; Wangbinghui (Biggio, Kirin_DRV) <[email protected]>; Rob Herring <[email protected]>; Songxiaowei (Kirin_DRV) <[email protected]>; [email protected]; [email protected]; [email protected]
> 主题: Re: [PATCH v12 00/11] Add support for Hikey 970 PCIe
>
> On Tue, Oct 05, 2021 at 01:23:21PM -0500, Bjorn Helgaas wrote:
> > [+cc Lorenzo]
> >
> > On Tue, Oct 05, 2021 at 11:24:48AM +0200, Mauro Carvalho Chehab wrote:
> > > Hi Bjorn,
> > >
> > > Em Tue, 28 Sep 2021 09:34:10 +0200
> > > Mauro Carvalho Chehab <[email protected]> escreveu:
> >
> > > > PCI: kirin: Reorganize the PHY logic inside the driver
> > > > PCI: kirin: Add support for a PHY layer
> > > > PCI: kirin: Use regmap for APB registers
> > > > PCI: kirin: Add support for bridge slot DT schema
> > > > PCI: kirin: Add Kirin 970 compatible
> > > > PCI: kirin: Add MODULE_* macros
> > > > PCI: kirin: Allow building it as a module
> > > > PCI: kirin: Add power_off support for Kirin 960 PHY
> > > > PCI: kirin: fix poweroff sequence
> > > > PCI: kirin: Allow removing the driver
> > >
> > > I guess everything is already satisfying the review feedbacks.
> > > If so, could you please merge the PCI ones?
> >
> > Lorenzo takes care of the native host bridge drivers, so I'm sure this
> > is on his list. I added him to cc: in case not.
>
> Ideally I'd like to see these patches ACKed/Review-ed by the kirin maintainers - that's what I was waiting for and that's what they are there for.
>
> Having said that, I will keep an eye on this series so that we can hopefully queue it for v5.16.
>
> Lorenzo
>
>

2021-10-09 01:48:08

by Songxiaowei (Kirin_DRV)

[permalink] [raw]
Subject: Re: [PATCH v12 00/11] Add support for Hikey 970 PCIe

Ack-by Xiaowei Song.

> Hi Lorenzo,
>
> Em Thu, 7 Oct 2021 15:41:03 +0100
> Lorenzo Pieralisi <[email protected]> escreveu:
>
> > On Tue, Oct 05, 2021 at 01:23:21PM -0500, Bjorn Helgaas wrote:
> > > [+cc Lorenzo]
> > >
> > > On Tue, Oct 05, 2021 at 11:24:48AM +0200, Mauro Carvalho Chehab wrote:
> > > > Hi Bjorn,
> > > >
> > > > Em Tue, 28 Sep 2021 09:34:10 +0200
> > > > Mauro Carvalho Chehab <[email protected]> escreveu:
> > >
> > > > > PCI: kirin: Reorganize the PHY logic inside the driver
> > > > > PCI: kirin: Add support for a PHY layer
> > > > > PCI: kirin: Use regmap for APB registers
> > > > > PCI: kirin: Add support for bridge slot DT schema
> > > > > PCI: kirin: Add Kirin 970 compatible
> > > > > PCI: kirin: Add MODULE_* macros
> > > > > PCI: kirin: Allow building it as a module
> > > > > PCI: kirin: Add power_off support for Kirin 960 PHY
> > > > > PCI: kirin: fix poweroff sequence
> > > > > PCI: kirin: Allow removing the driver
> > > >
> > > > I guess everything is already satisfying the review feedbacks.
> > > > If so, could you please merge the PCI ones?
> > >
> > > Lorenzo takes care of the native host bridge drivers, so I'm sure this
> > > is on his list. I added him to cc: in case not.
> >
> > Ideally I'd like to see these patches ACKed/Review-ed by the kirin
> > maintainers - that's what I was waiting for and that's what they
> > are there for.
> >
> > Having said that, I will keep an eye on this series so that we
> > can hopefully queue it for v5.16.
>
> Not sure if you received the e-mail from Xiaowei with his ack.

I have not (and it did not make it to linux-pci either).

> At least here, I only received on my internal e-mail (perhaps because
> the original e-mail was base64-encoded with gb2312 charset).
>
> So, let me forward his answer to you, c/c the mailing lists.

Patches should be acked with tags that tooling recognize, this
would help me.

> Thanks,
> Mauro
>

Thanks.

2021-10-12 13:08:48

by Lorenzo Pieralisi

[permalink] [raw]
Subject: Re: [PATCH v12 00/11] Add support for Hikey 970 PCIe

On Sat, Oct 09, 2021 at 01:44:24AM +0000, Songxiaowei (Kirin_DRV) wrote:
> Ack-by Xiaowei Song.

First of all, you should not top-post:

https://kernelnewbies.org/PatchCulture

Second the format does not match the guidelines:

https://www.kernel.org/doc/html/latest/process/submitting-patches.html

So let's retry to send your ACK a third time please.

Thanks,
Lorenzo

>
> > Hi Lorenzo,
> >
> > Em Thu, 7 Oct 2021 15:41:03 +0100
> > Lorenzo Pieralisi <[email protected]> escreveu:
> >
> > > On Tue, Oct 05, 2021 at 01:23:21PM -0500, Bjorn Helgaas wrote:
> > > > [+cc Lorenzo]
> > > >
> > > > On Tue, Oct 05, 2021 at 11:24:48AM +0200, Mauro Carvalho Chehab wrote:
> > > > > Hi Bjorn,
> > > > >
> > > > > Em Tue, 28 Sep 2021 09:34:10 +0200
> > > > > Mauro Carvalho Chehab <[email protected]> escreveu:
> > > >
> > > > > > PCI: kirin: Reorganize the PHY logic inside the driver
> > > > > > PCI: kirin: Add support for a PHY layer
> > > > > > PCI: kirin: Use regmap for APB registers
> > > > > > PCI: kirin: Add support for bridge slot DT schema
> > > > > > PCI: kirin: Add Kirin 970 compatible
> > > > > > PCI: kirin: Add MODULE_* macros
> > > > > > PCI: kirin: Allow building it as a module
> > > > > > PCI: kirin: Add power_off support for Kirin 960 PHY
> > > > > > PCI: kirin: fix poweroff sequence
> > > > > > PCI: kirin: Allow removing the driver
> > > > >
> > > > > I guess everything is already satisfying the review feedbacks.
> > > > > If so, could you please merge the PCI ones?
> > > >
> > > > Lorenzo takes care of the native host bridge drivers, so I'm sure this
> > > > is on his list. I added him to cc: in case not.
> > >
> > > Ideally I'd like to see these patches ACKed/Review-ed by the kirin
> > > maintainers - that's what I was waiting for and that's what they
> > > are there for.
> > >
> > > Having said that, I will keep an eye on this series so that we
> > > can hopefully queue it for v5.16.
> >
> > Not sure if you received the e-mail from Xiaowei with his ack.
>
> I have not (and it did not make it to linux-pci either).
>
> > At least here, I only received on my internal e-mail (perhaps because
> > the original e-mail was base64-encoded with gb2312 charset).
> >
> > So, let me forward his answer to you, c/c the mailing lists.
>
> Patches should be acked with tags that tooling recognize, this
> would help me.
>
> > Thanks,
> > Mauro
> >
>
> Thanks.