* [PATCH 0/2] phy: mediatek: Add PCIe PHY driver
@ 2022-03-11 13:35 Jianjun Wang
2022-03-11 13:35 ` [PATCH 1/2] " Jianjun Wang
2022-03-11 13:35 ` [PATCH 2/2] dt-bindings: phy: mediatek: Add YAML schema for PCIe PHY Jianjun Wang
0 siblings, 2 replies; 8+ messages in thread
From: Jianjun Wang @ 2022-03-11 13:35 UTC (permalink / raw)
To: Chunfeng Yun, Kishon Vijay Abraham I, Vinod Koul, Rob Herring,
Matthias Brugger
Cc: Jianjun Wang, linux-arm-kernel, linux-mediatek, linux-phy,
devicetree, linux-kernel, rex-bc.chen, randy.wu, jieyy.yang,
chuanjia.liu, qizhong.cheng, jian.yang
These series patches add support for PCIe PHY driver on MediaTek chipsets.
Jianjun Wang (2):
phy: mediatek: Add PCIe PHY driver
dt-bindings: phy: mediatek: Add YAML schema for PCIe PHY
.../bindings/phy/mediatek,pcie-phy.yaml | 71 +++++++
drivers/phy/mediatek/Kconfig | 11 +
drivers/phy/mediatek/Makefile | 1 +
drivers/phy/mediatek/phy-mtk-pcie.c | 198 ++++++++++++++++++
4 files changed, 281 insertions(+)
create mode 100644 Documentation/devicetree/bindings/phy/mediatek,pcie-phy.yaml
create mode 100644 drivers/phy/mediatek/phy-mtk-pcie.c
--
2.18.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] phy: mediatek: Add PCIe PHY driver
2022-03-11 13:35 [PATCH 0/2] phy: mediatek: Add PCIe PHY driver Jianjun Wang
@ 2022-03-11 13:35 ` Jianjun Wang
2022-03-17 7:49 ` Chen-Yu Tsai
2022-03-11 13:35 ` [PATCH 2/2] dt-bindings: phy: mediatek: Add YAML schema for PCIe PHY Jianjun Wang
1 sibling, 1 reply; 8+ messages in thread
From: Jianjun Wang @ 2022-03-11 13:35 UTC (permalink / raw)
To: Chunfeng Yun, Kishon Vijay Abraham I, Vinod Koul, Rob Herring,
Matthias Brugger
Cc: Jianjun Wang, linux-arm-kernel, linux-mediatek, linux-phy,
devicetree, linux-kernel, rex-bc.chen, randy.wu, jieyy.yang,
chuanjia.liu, qizhong.cheng, jian.yang
Add PCIe GEN3 PHY driver support on MediaTek chipsets.
Signed-off-by: Jianjun Wang <jianjun.wang@mediatek.com>
---
drivers/phy/mediatek/Kconfig | 11 ++
drivers/phy/mediatek/Makefile | 1 +
drivers/phy/mediatek/phy-mtk-pcie.c | 198 ++++++++++++++++++++++++++++
3 files changed, 210 insertions(+)
create mode 100644 drivers/phy/mediatek/phy-mtk-pcie.c
diff --git a/drivers/phy/mediatek/Kconfig b/drivers/phy/mediatek/Kconfig
index 55f8e6c048ab..387ed1b3f2cc 100644
--- a/drivers/phy/mediatek/Kconfig
+++ b/drivers/phy/mediatek/Kconfig
@@ -55,3 +55,14 @@ config PHY_MTK_MIPI_DSI
select GENERIC_PHY
help
Support MIPI DSI for Mediatek SoCs.
+
+config PHY_MTK_PCIE
+ tristate "MediaTek PCIe-PHY Driver"
+ depends on ARCH_MEDIATEK || COMPILE_TEST
+ depends on OF
+ select GENERIC_PHY
+ help
+ Say 'Y' here to add support for MediaTek PCIe PHY driver.
+ This driver create the basic PHY instance and provides initialize
+ callback for PCIe GEN3 port, it supports software efuse
+ initialization.
diff --git a/drivers/phy/mediatek/Makefile b/drivers/phy/mediatek/Makefile
index ace660fbed3a..788c13147f63 100644
--- a/drivers/phy/mediatek/Makefile
+++ b/drivers/phy/mediatek/Makefile
@@ -6,6 +6,7 @@
obj-$(CONFIG_PHY_MTK_TPHY) += phy-mtk-tphy.o
obj-$(CONFIG_PHY_MTK_UFS) += phy-mtk-ufs.o
obj-$(CONFIG_PHY_MTK_XSPHY) += phy-mtk-xsphy.o
+obj-$(CONFIG_PHY_MTK_PCIE) += phy-mtk-pcie.o
phy-mtk-hdmi-drv-y := phy-mtk-hdmi.o
phy-mtk-hdmi-drv-y += phy-mtk-hdmi-mt2701.o
diff --git a/drivers/phy/mediatek/phy-mtk-pcie.c b/drivers/phy/mediatek/phy-mtk-pcie.c
new file mode 100644
index 000000000000..45a67d9171f6
--- /dev/null
+++ b/drivers/phy/mediatek/phy-mtk-pcie.c
@@ -0,0 +1,198 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2022 MediaTek Inc.
+ * Author: Jianjun Wang <jianjun.wang@mediatek.com>
+ */
+
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/module.h>
+#include <linux/nvmem-consumer.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+
+#include "phy-mtk-io.h"
+
+#define PEXTP_ANA_GLB_00_REG 0x9000
+#define PEXTP_ANA_LN0_TX_REG 0xA004
+#define PEXTP_ANA_LN0_RX_REG 0xA03C
+#define PEXTP_ANA_LN1_TX_REG 0xA104
+#define PEXTP_ANA_LN1_RX_REG 0xA13c
+
+/* PEXTP_GLB_00_RG[28:24] Internal Resistor Selection of TX Bias Current */
+#define EFUSE_GLB_INTR_SEL GENMASK(28, 24)
+#define EFUSE_GLB_INTR_VAL(x) ((0x1f & (x)) << 24)
+
+/* PEXTP_ANA_LN_RX_RG[3:0] LN0 RX impedance selection */
+#define EFUSE_LN_RX_SEL GENMASK(3, 0)
+#define EFUSE_LN_RX_VAL(x) (0xf & (x))
+
+/* PEXTP_ANA_LN_TX_RG[5:2] LN0 TX PMOS impedance selection */
+#define EFUSE_LN_TX_PMOS_SEL GENMASK(5, 2)
+#define EFUSE_LN_TX_PMOS_VAL(x) ((0xf & (x)) << 2)
+
+/* PEXTP_ANA_LN_TX_RG[11:8] LN0 TX NMOS impedance selection */
+#define EFUSE_LN_TX_NMOS_SEL GENMASK(11, 8)
+#define EFUSE_LN_TX_NMOS_VAL(x) ((0xf & (x)) << 8)
+
+struct mtk_pcie_phy {
+ struct device *dev;
+ struct phy *phy;
+ void __iomem *sif_base;
+};
+
+static int mtk_pcie_phy_init(struct phy *phy)
+{
+ struct mtk_pcie_phy *pcie_phy = phy_get_drvdata(phy);
+ struct device *dev = pcie_phy->dev;
+ bool nvmem_enabled;
+ u32 glb_intr, tx_pmos, tx_nmos, rx_data;
+ int ret;
+
+ nvmem_enabled = device_property_read_bool(dev, "nvmem-cells");
+ if (!nvmem_enabled)
+ return 0;
+
+ /* Set efuse value for lane0 */
+ ret = nvmem_cell_read_variable_le_u32(dev, "tx_ln0_pmos", &tx_pmos);
+ if (ret) {
+ dev_err(dev, "%s: Failed to read tx_ln0_pmos\n", __func__);
+ return ret;
+ }
+
+ ret = nvmem_cell_read_variable_le_u32(dev, "tx_ln0_nmos", &tx_nmos);
+ if (ret) {
+ dev_err(dev, "%s: Failed to read tx_ln0_nmos\n", __func__);
+ return ret;
+ }
+
+ ret = nvmem_cell_read_variable_le_u32(dev, "rx_ln0", &rx_data);
+ if (ret) {
+ dev_err(dev, "%s: Failed to read rx_ln0\n", __func__);
+ return ret;
+ }
+
+ /* Don't wipe the old data if there is no data in efuse cell */
+ if (!(tx_pmos || tx_nmos || rx_data)) {
+ dev_warn(dev, "%s: No efuse data found, but dts enable it\n",
+ __func__);
+ return 0;
+ }
+
+ mtk_phy_update_bits(pcie_phy->sif_base + PEXTP_ANA_LN0_TX_REG,
+ EFUSE_LN_TX_PMOS_SEL,
+ EFUSE_LN_TX_PMOS_VAL(tx_pmos));
+
+ mtk_phy_update_bits(pcie_phy->sif_base + PEXTP_ANA_LN0_TX_REG,
+ EFUSE_LN_TX_NMOS_SEL,
+ EFUSE_LN_TX_NMOS_VAL(tx_nmos));
+
+ mtk_phy_update_bits(pcie_phy->sif_base + PEXTP_ANA_LN0_RX_REG,
+ EFUSE_LN_RX_SEL, EFUSE_LN_RX_VAL(rx_data));
+
+ /* Set global data */
+ ret = nvmem_cell_read_variable_le_u32(dev, "glb_intr", &glb_intr);
+ if (ret) {
+ dev_err(dev, "%s: Failed to read glb_intr\n", __func__);
+ return ret;
+ }
+
+ mtk_phy_update_bits(pcie_phy->sif_base + PEXTP_ANA_GLB_00_REG,
+ EFUSE_GLB_INTR_SEL, EFUSE_GLB_INTR_VAL(glb_intr));
+
+ /*
+ * Set efuse value for lane1, only available for the platform which
+ * supports two lane.
+ */
+ ret = nvmem_cell_read_variable_le_u32(dev, "tx_ln1_pmos", &tx_pmos);
+ if (ret) {
+ dev_err(dev, "%s: Failed to read tx_ln1_pmos, efuse value not support for lane 1\n",
+ __func__);
+ return 0;
+ }
+
+ ret = nvmem_cell_read_variable_le_u32(dev, "tx_ln1_nmos", &tx_nmos);
+ if (ret) {
+ dev_err(dev, "%s: Failed to read tx_ln1_pmos\n", __func__);
+ return ret;
+ }
+
+ ret = nvmem_cell_read_variable_le_u32(dev, "rx_ln1", &rx_data);
+ if (ret) {
+ dev_err(dev, "%s: Failed to read rx_ln1\n", __func__);
+ return ret;
+ }
+
+ if (!(tx_pmos || tx_nmos || rx_data))
+ return 0;
+
+ mtk_phy_update_bits(pcie_phy->sif_base + PEXTP_ANA_LN1_TX_REG,
+ EFUSE_LN_TX_PMOS_SEL,
+ EFUSE_LN_TX_PMOS_VAL(tx_pmos));
+
+ mtk_phy_update_bits(pcie_phy->sif_base + PEXTP_ANA_LN1_TX_REG,
+ EFUSE_LN_TX_NMOS_SEL,
+ EFUSE_LN_TX_NMOS_VAL(tx_nmos));
+
+ mtk_phy_update_bits(pcie_phy->sif_base + PEXTP_ANA_LN1_RX_REG,
+ EFUSE_LN_RX_SEL, EFUSE_LN_RX_VAL(rx_data));
+
+ return 0;
+}
+
+static const struct phy_ops mtk_pcie_phy_ops = {
+ .init = mtk_pcie_phy_init,
+ .owner = THIS_MODULE,
+};
+
+static int mtk_pcie_phy_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct phy_provider *provider;
+ struct mtk_pcie_phy *pcie_phy;
+
+ pcie_phy = devm_kzalloc(dev, sizeof(*pcie_phy), GFP_KERNEL);
+ if (!pcie_phy)
+ return -ENOMEM;
+
+ pcie_phy->dev = dev;
+
+ pcie_phy->sif_base = devm_platform_ioremap_resource_byname(pdev, "sif");
+ if (IS_ERR(pcie_phy->sif_base)) {
+ dev_err(dev, "%s: Failed to map phy-sif base\n", __func__);
+ return PTR_ERR(pcie_phy->sif_base);
+ }
+
+ pcie_phy->phy = devm_phy_create(dev, dev->of_node, &mtk_pcie_phy_ops);
+ if (IS_ERR(pcie_phy->phy)) {
+ dev_err(dev, "%s: Failed to create PCIe phy\n", __func__);
+ return PTR_ERR(pcie_phy->phy);
+ }
+
+ phy_set_drvdata(pcie_phy->phy, pcie_phy);
+
+ provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+
+ return PTR_ERR_OR_ZERO(provider);
+}
+
+static const struct of_device_id mtk_pcie_phy_of_match[] = {
+ { .compatible = "mediatek,pcie-phy" },
+ { },
+};
+MODULE_DEVICE_TABLE(of, mtk_pcie_phy_of_match);
+
+static struct platform_driver mtk_pcie_phy_driver = {
+ .probe = mtk_pcie_phy_probe,
+ .driver = {
+ .name = "mtk-pcie-phy",
+ .of_match_table = mtk_pcie_phy_of_match,
+ },
+};
+module_platform_driver(mtk_pcie_phy_driver);
+
+MODULE_DESCRIPTION("MediaTek PCIe PHY driver");
+MODULE_AUTHOR("Jianjun Wang <jianjun.wang@mediatek.com>");
+MODULE_LICENSE("GPL v2");
--
2.18.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] dt-bindings: phy: mediatek: Add YAML schema for PCIe PHY
2022-03-11 13:35 [PATCH 0/2] phy: mediatek: Add PCIe PHY driver Jianjun Wang
2022-03-11 13:35 ` [PATCH 1/2] " Jianjun Wang
@ 2022-03-11 13:35 ` Jianjun Wang
2022-03-11 14:28 ` Krzysztof Kozlowski
1 sibling, 1 reply; 8+ messages in thread
From: Jianjun Wang @ 2022-03-11 13:35 UTC (permalink / raw)
To: Chunfeng Yun, Kishon Vijay Abraham I, Vinod Koul, Rob Herring,
Matthias Brugger
Cc: Jianjun Wang, linux-arm-kernel, linux-mediatek, linux-phy,
devicetree, linux-kernel, rex-bc.chen, randy.wu, jieyy.yang,
chuanjia.liu, qizhong.cheng, jian.yang
Add YAML schema documentation for PCIe PHY on MediaTek chipsets.
Signed-off-by: Jianjun Wang <jianjun.wang@mediatek.com>
---
.../bindings/phy/mediatek,pcie-phy.yaml | 71 +++++++++++++++++++
1 file changed, 71 insertions(+)
create mode 100644 Documentation/devicetree/bindings/phy/mediatek,pcie-phy.yaml
diff --git a/Documentation/devicetree/bindings/phy/mediatek,pcie-phy.yaml b/Documentation/devicetree/bindings/phy/mediatek,pcie-phy.yaml
new file mode 100644
index 000000000000..da15b4bf3117
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/mediatek,pcie-phy.yaml
@@ -0,0 +1,71 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/mediatek,pcie-phy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MediaTek PCIe PHY Device Tree Binding
+
+maintainers:
+ - Jianjun Wang <jianjun.wang@mediatek.com>
+
+description: |
+ The PCIe PHY supports physical layer functionality for PCIe Gen3 port.
+
+properties:
+ compatible:
+ const: mediatek,pcie-phy
+
+ reg:
+ maxItems: 1
+
+ reg-names:
+ items:
+ - const: sif
+
+ "#phy-cells":
+ const: 0
+
+ nvmem-cells:
+ maxItems: 7
+ description:
+ Phandles to nvmem cell that contains the efuse data, if unspecified,
+ default value is used.
+
+ nvmem-cell-names:
+ items:
+ - const: glb_intr
+ - const: tx_ln0_pmos
+ - const: tx_ln0_nmos
+ - const: rx_ln0
+ - const: tx_ln1_pmos
+ - const: tx_ln1_nmos
+ - const: rx_ln1
+
+required:
+ - compatible
+ - reg
+ - reg-names
+ - "#phy-cells"
+
+additionalProperties: false
+
+examples:
+ - |
+ pciephy: phy@11e80000 {
+ compatible = "mediatek,pcie-phy";
+ #phy-cells = <0>;
+ reg = <0x11e80000 0x10000>;
+ reg-names = "sif";
+ nvmem-cells = <&pciephy_glb_intr>,
+ <&pciephy_tx_ln0_pmos>,
+ <&pciephy_tx_ln0_nmos>,
+ <&pciephy_rx_ln0>,
+ <&pciephy_tx_ln1_pmos>,
+ <&pciephy_tx_ln1_nmos>,
+ <&pciephy_rx_ln1>;
+ nvmem-cell-names = "glb_intr", "tx_ln0_pmos",
+ "tx_ln0_nmos", "rx_ln0",
+ "tx_ln1_pmos", "tx_ln1_nmos",
+ "rx_ln1";
+ };
--
2.18.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] dt-bindings: phy: mediatek: Add YAML schema for PCIe PHY
2022-03-11 13:35 ` [PATCH 2/2] dt-bindings: phy: mediatek: Add YAML schema for PCIe PHY Jianjun Wang
@ 2022-03-11 14:28 ` Krzysztof Kozlowski
2022-03-14 1:42 ` Jianjun Wang
0 siblings, 1 reply; 8+ messages in thread
From: Krzysztof Kozlowski @ 2022-03-11 14:28 UTC (permalink / raw)
To: Jianjun Wang, Chunfeng Yun, Kishon Vijay Abraham I, Vinod Koul,
Rob Herring, Matthias Brugger
Cc: linux-arm-kernel, linux-mediatek, linux-phy, devicetree,
linux-kernel, rex-bc.chen, randy.wu, jieyy.yang, chuanjia.liu,
qizhong.cheng, jian.yang
On 11/03/2022 14:35, Jianjun Wang wrote:
> Add YAML schema documentation for PCIe PHY on MediaTek chipsets.
>
> Signed-off-by: Jianjun Wang <jianjun.wang@mediatek.com>
> ---
> .../bindings/phy/mediatek,pcie-phy.yaml | 71 +++++++++++++++++++
> 1 file changed, 71 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/phy/mediatek,pcie-phy.yaml
>
> diff --git a/Documentation/devicetree/bindings/phy/mediatek,pcie-phy.yaml b/Documentation/devicetree/bindings/phy/mediatek,pcie-phy.yaml
> new file mode 100644
> index 000000000000..da15b4bf3117
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/mediatek,pcie-phy.yaml
> @@ -0,0 +1,71 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/phy/mediatek,pcie-phy.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MediaTek PCIe PHY Device Tree Binding
Title is for hardware, so s/Device Tree Binding//
> +
> +maintainers:
> + - Jianjun Wang <jianjun.wang@mediatek.com>
> +
> +description: |
> + The PCIe PHY supports physical layer functionality for PCIe Gen3 port.
> +
> +properties:
> + compatible:
> + const: mediatek,pcie-phy
Is it going to be exactly one pcie-phy for all Mediatek chipsets for
next years? Are you sure about that? It sounds highly unlikely....
> +
> + reg:
> + maxItems: 1
> +
> + reg-names:
> + items:
> + - const: sif
> +
> + "#phy-cells":
> + const: 0
> +
> + nvmem-cells:
> + maxItems: 7
> + description:
> + Phandles to nvmem cell that contains the efuse data, if unspecified,
> + default value is used.
> +
> + nvmem-cell-names:
> + items:
> + - const: glb_intr
> + - const: tx_ln0_pmos
> + - const: tx_ln0_nmos
> + - const: rx_ln0
> + - const: tx_ln1_pmos
> + - const: tx_ln1_nmos
> + - const: rx_ln1
> +
> +required:
> + - compatible
> + - reg
> + - reg-names
> + - "#phy-cells"
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + pciephy: phy@11e80000 {
> + compatible = "mediatek,pcie-phy";
> + #phy-cells = <0>;
> + reg = <0x11e80000 0x10000>;
> + reg-names = "sif";
> + nvmem-cells = <&pciephy_glb_intr>,
> + <&pciephy_tx_ln0_pmos>,
> + <&pciephy_tx_ln0_nmos>,
> + <&pciephy_rx_ln0>,
> + <&pciephy_tx_ln1_pmos>,
> + <&pciephy_tx_ln1_nmos>,
> + <&pciephy_rx_ln1>;
> + nvmem-cell-names = "glb_intr", "tx_ln0_pmos",
> + "tx_ln0_nmos", "rx_ln0",
> + "tx_ln1_pmos", "tx_ln1_nmos",
> + "rx_ln1";
> + };
Best regards,
Krzysztof
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] dt-bindings: phy: mediatek: Add YAML schema for PCIe PHY
2022-03-11 14:28 ` Krzysztof Kozlowski
@ 2022-03-14 1:42 ` Jianjun Wang
2022-03-15 3:15 ` Chunfeng Yun
0 siblings, 1 reply; 8+ messages in thread
From: Jianjun Wang @ 2022-03-14 1:42 UTC (permalink / raw)
To: Krzysztof Kozlowski, Chunfeng Yun, Kishon Vijay Abraham I,
Vinod Koul, Rob Herring, Matthias Brugger
Cc: linux-arm-kernel, linux-mediatek, linux-phy, devicetree,
linux-kernel, rex-bc.chen, randy.wu, jieyy.yang, chuanjia.liu,
qizhong.cheng, jian.yang
Hi Krzysztof,
On Fri, 2022-03-11 at 15:28 +0100, Krzysztof Kozlowski wrote:
> On 11/03/2022 14:35, Jianjun Wang wrote:
> > Add YAML schema documentation for PCIe PHY on MediaTek chipsets.
> >
> > Signed-off-by: Jianjun Wang <jianjun.wang@mediatek.com>
> > ---
> > .../bindings/phy/mediatek,pcie-phy.yaml | 71
> > +++++++++++++++++++
> > 1 file changed, 71 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/phy/mediatek,pcie-phy.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/phy/mediatek,pcie-
> > phy.yaml b/Documentation/devicetree/bindings/phy/mediatek,pcie-
> > phy.yaml
> > new file mode 100644
> > index 000000000000..da15b4bf3117
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/phy/mediatek,pcie-phy.yaml
> > @@ -0,0 +1,71 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/phy/mediatek,pcie-phy.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: MediaTek PCIe PHY Device Tree Binding
>
> Title is for hardware, so s/Device Tree Binding//
>
> > +
> > +maintainers:
> > + - Jianjun Wang <jianjun.wang@mediatek.com>
> > +
> > +description: |
> > + The PCIe PHY supports physical layer functionality for PCIe Gen3
> > port.
> > +
> > +properties:
> > + compatible:
> > + const: mediatek,pcie-phy
>
> Is it going to be exactly one pcie-phy for all Mediatek chipsets for
> next years? Are you sure about that? It sounds highly unlikely....
We have only one pcie-phy for now, but if this is not recommended, I
will replace it with a specific name in the next version, thanks for
your review.
Thanks.
>
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + reg-names:
> > + items:
> > + - const: sif
> > +
> > + "#phy-cells":
> > + const: 0
> > +
> > + nvmem-cells:
> > + maxItems: 7
> > + description:
> > + Phandles to nvmem cell that contains the efuse data, if
> > unspecified,
> > + default value is used.
> > +
> > + nvmem-cell-names:
> > + items:
> > + - const: glb_intr
> > + - const: tx_ln0_pmos
> > + - const: tx_ln0_nmos
> > + - const: rx_ln0
> > + - const: tx_ln1_pmos
> > + - const: tx_ln1_nmos
> > + - const: rx_ln1
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - reg-names
> > + - "#phy-cells"
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + pciephy: phy@11e80000 {
> > + compatible = "mediatek,pcie-phy";
> > + #phy-cells = <0>;
> > + reg = <0x11e80000 0x10000>;
> > + reg-names = "sif";
> > + nvmem-cells = <&pciephy_glb_intr>,
> > + <&pciephy_tx_ln0_pmos>,
> > + <&pciephy_tx_ln0_nmos>,
> > + <&pciephy_rx_ln0>,
> > + <&pciephy_tx_ln1_pmos>,
> > + <&pciephy_tx_ln1_nmos>,
> > + <&pciephy_rx_ln1>;
> > + nvmem-cell-names = "glb_intr", "tx_ln0_pmos",
> > + "tx_ln0_nmos", "rx_ln0",
> > + "tx_ln1_pmos", "tx_ln1_nmos",
> > + "rx_ln1";
> > + };
>
>
> Best regards,
> Krzysztof
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] dt-bindings: phy: mediatek: Add YAML schema for PCIe PHY
2022-03-14 1:42 ` Jianjun Wang
@ 2022-03-15 3:15 ` Chunfeng Yun
0 siblings, 0 replies; 8+ messages in thread
From: Chunfeng Yun @ 2022-03-15 3:15 UTC (permalink / raw)
To: Jianjun Wang, Krzysztof Kozlowski, Kishon Vijay Abraham I,
Vinod Koul, Rob Herring, Matthias Brugger
Cc: linux-arm-kernel, linux-mediatek, linux-phy, devicetree,
linux-kernel, rex-bc.chen, randy.wu, jieyy.yang, chuanjia.liu,
qizhong.cheng, jian.yang
On Mon, 2022-03-14 at 09:42 +0800, Jianjun Wang wrote:
> Hi Krzysztof,
>
> On Fri, 2022-03-11 at 15:28 +0100, Krzysztof Kozlowski wrote:
> > On 11/03/2022 14:35, Jianjun Wang wrote:
> > > Add YAML schema documentation for PCIe PHY on MediaTek chipsets.
> > >
> > > Signed-off-by: Jianjun Wang <jianjun.wang@mediatek.com>
> > > ---
> > > .../bindings/phy/mediatek,pcie-phy.yaml | 71
> > > +++++++++++++++++++
> > > 1 file changed, 71 insertions(+)
> > > create mode 100644
> > > Documentation/devicetree/bindings/phy/mediatek,pcie-phy.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/phy/mediatek,pcie-
> > > phy.yaml b/Documentation/devicetree/bindings/phy/mediatek,pcie-
> > > phy.yaml
> > > new file mode 100644
> > > index 000000000000..da15b4bf3117
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/phy/mediatek,pcie-
> > > phy.yaml
> > > @@ -0,0 +1,71 @@
> > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/phy/mediatek,pcie-phy.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: MediaTek PCIe PHY Device Tree Binding
> >
> > Title is for hardware, so s/Device Tree Binding//
> >
> > > +
> > > +maintainers:
> > > + - Jianjun Wang <jianjun.wang@mediatek.com>
> > > +
> > > +description: |
> > > + The PCIe PHY supports physical layer functionality for PCIe
> > > Gen3
> > > port.
> > > +
> > > +properties:
> > > + compatible:
> > > + const: mediatek,pcie-phy
> >
> > Is it going to be exactly one pcie-phy for all Mediatek chipsets
> > for
> > next years? Are you sure about that? It sounds highly unlikely....
>
> We have only one pcie-phy for now, but if this is not recommended, I
> will replace it with a specific name in the next version, thanks for
> your review.
Prefer to add specific ones
>
> Thanks.
>
> >
> > > +
> > > + reg:
> > > + maxItems: 1
> > > +
> > > + reg-names:
> > > + items:
> > > + - const: sif
> > > +
> > > + "#phy-cells":
> > > + const: 0
> > > +
> > > + nvmem-cells:
> > > + maxItems: 7
> > > + description:
> > > + Phandles to nvmem cell that contains the efuse data, if
> > > unspecified,
> > > + default value is used.
> > > +
> > > + nvmem-cell-names:
> > > + items:
> > > + - const: glb_intr
> > > + - const: tx_ln0_pmos
> > > + - const: tx_ln0_nmos
> > > + - const: rx_ln0
> > > + - const: tx_ln1_pmos
> > > + - const: tx_ln1_nmos
> > > + - const: rx_ln1
> > > +
> > > +required:
> > > + - compatible
> > > + - reg
> > > + - reg-names
> > > + - "#phy-cells"
> > > +
> > > +additionalProperties: false
> > > +
> > > +examples:
> > > + - |
> > > + pciephy: phy@11e80000 {
> > > + compatible = "mediatek,pcie-phy";
> > > + #phy-cells = <0>;
> > > + reg = <0x11e80000 0x10000>;
> > > + reg-names = "sif";
> > > + nvmem-cells = <&pciephy_glb_intr>,
> > > + <&pciephy_tx_ln0_pmos>,
> > > + <&pciephy_tx_ln0_nmos>,
> > > + <&pciephy_rx_ln0>,
> > > + <&pciephy_tx_ln1_pmos>,
> > > + <&pciephy_tx_ln1_nmos>,
> > > + <&pciephy_rx_ln1>;
> > > + nvmem-cell-names = "glb_intr", "tx_ln0_pmos",
> > > + "tx_ln0_nmos", "rx_ln0",
> > > + "tx_ln1_pmos", "tx_ln1_nmos",
> > > + "rx_ln1";
> > > + };
> >
> >
> > Best regards,
> > Krzysztof
>
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] phy: mediatek: Add PCIe PHY driver
2022-03-11 13:35 ` [PATCH 1/2] " Jianjun Wang
@ 2022-03-17 7:49 ` Chen-Yu Tsai
2022-03-17 9:34 ` Jianjun Wang
0 siblings, 1 reply; 8+ messages in thread
From: Chen-Yu Tsai @ 2022-03-17 7:49 UTC (permalink / raw)
To: Jianjun Wang
Cc: Chunfeng Yun, Kishon Vijay Abraham I, Vinod Koul, Rob Herring,
Matthias Brugger, linux-arm-kernel, linux-mediatek, linux-phy,
devicetree, linux-kernel, rex-bc.chen, Randy.Wu, jieyy.yang,
chuanjia.liu, qizhong.cheng, jian.yang
On Fri, Mar 11, 2022 at 9:46 PM Jianjun Wang <jianjun.wang@mediatek.com> wrote:
>
> Add PCIe GEN3 PHY driver support on MediaTek chipsets.
>
> Signed-off-by: Jianjun Wang <jianjun.wang@mediatek.com>
> ---
> drivers/phy/mediatek/Kconfig | 11 ++
> drivers/phy/mediatek/Makefile | 1 +
> drivers/phy/mediatek/phy-mtk-pcie.c | 198 ++++++++++++++++++++++++++++
> 3 files changed, 210 insertions(+)
> create mode 100644 drivers/phy/mediatek/phy-mtk-pcie.c
>
> diff --git a/drivers/phy/mediatek/Kconfig b/drivers/phy/mediatek/Kconfig
> index 55f8e6c048ab..387ed1b3f2cc 100644
> --- a/drivers/phy/mediatek/Kconfig
> +++ b/drivers/phy/mediatek/Kconfig
> @@ -55,3 +55,14 @@ config PHY_MTK_MIPI_DSI
> select GENERIC_PHY
> help
> Support MIPI DSI for Mediatek SoCs.
> +
> +config PHY_MTK_PCIE
> + tristate "MediaTek PCIe-PHY Driver"
> + depends on ARCH_MEDIATEK || COMPILE_TEST
> + depends on OF
> + select GENERIC_PHY
> + help
> + Say 'Y' here to add support for MediaTek PCIe PHY driver.
> + This driver create the basic PHY instance and provides initialize
> + callback for PCIe GEN3 port, it supports software efuse
> + initialization.
> diff --git a/drivers/phy/mediatek/Makefile b/drivers/phy/mediatek/Makefile
> index ace660fbed3a..788c13147f63 100644
> --- a/drivers/phy/mediatek/Makefile
> +++ b/drivers/phy/mediatek/Makefile
> @@ -6,6 +6,7 @@
> obj-$(CONFIG_PHY_MTK_TPHY) += phy-mtk-tphy.o
> obj-$(CONFIG_PHY_MTK_UFS) += phy-mtk-ufs.o
> obj-$(CONFIG_PHY_MTK_XSPHY) += phy-mtk-xsphy.o
> +obj-$(CONFIG_PHY_MTK_PCIE) += phy-mtk-pcie.o
>
> phy-mtk-hdmi-drv-y := phy-mtk-hdmi.o
> phy-mtk-hdmi-drv-y += phy-mtk-hdmi-mt2701.o
> diff --git a/drivers/phy/mediatek/phy-mtk-pcie.c b/drivers/phy/mediatek/phy-mtk-pcie.c
> new file mode 100644
> index 000000000000..45a67d9171f6
> --- /dev/null
> +++ b/drivers/phy/mediatek/phy-mtk-pcie.c
> @@ -0,0 +1,198 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2022 MediaTek Inc.
> + * Author: Jianjun Wang <jianjun.wang@mediatek.com>
> + */
> +
Please include linux/bits.h for GENMASK().
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
You don't need these two headers.
You could include linux/compiler_types.h for definition of __iomem.
> +#include <linux/module.h>
> +#include <linux/nvmem-consumer.h>
> +#include <linux/of_address.h>
> +#include <linux/of_device.h>
Nor do you need these two.
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
Please include linux/slab.h for kmalloc() and friends.
> +
> +#include "phy-mtk-io.h"
> +
> +#define PEXTP_ANA_GLB_00_REG 0x9000
> +#define PEXTP_ANA_LN0_TX_REG 0xA004
> +#define PEXTP_ANA_LN0_RX_REG 0xA03C
> +#define PEXTP_ANA_LN1_TX_REG 0xA104
> +#define PEXTP_ANA_LN1_RX_REG 0xA13c
> +
> +/* PEXTP_GLB_00_RG[28:24] Internal Resistor Selection of TX Bias Current */
> +#define EFUSE_GLB_INTR_SEL GENMASK(28, 24)
> +#define EFUSE_GLB_INTR_VAL(x) ((0x1f & (x)) << 24)
> +
> +/* PEXTP_ANA_LN_RX_RG[3:0] LN0 RX impedance selection */
> +#define EFUSE_LN_RX_SEL GENMASK(3, 0)
> +#define EFUSE_LN_RX_VAL(x) (0xf & (x))
> +
> +/* PEXTP_ANA_LN_TX_RG[5:2] LN0 TX PMOS impedance selection */
> +#define EFUSE_LN_TX_PMOS_SEL GENMASK(5, 2)
> +#define EFUSE_LN_TX_PMOS_VAL(x) ((0xf & (x)) << 2)
> +
> +/* PEXTP_ANA_LN_TX_RG[11:8] LN0 TX NMOS impedance selection */
> +#define EFUSE_LN_TX_NMOS_SEL GENMASK(11, 8)
> +#define EFUSE_LN_TX_NMOS_VAL(x) ((0xf & (x)) << 8)
Register definitions look good. I matched them to the datasheet.
> +struct mtk_pcie_phy {
> + struct device *dev;
> + struct phy *phy;
> + void __iomem *sif_base;
> +};
> +
> +static int mtk_pcie_phy_init(struct phy *phy)
> +{
> + struct mtk_pcie_phy *pcie_phy = phy_get_drvdata(phy);
> + struct device *dev = pcie_phy->dev;
> + bool nvmem_enabled;
> + u32 glb_intr, tx_pmos, tx_nmos, rx_data;
> + int ret;
> +
> + nvmem_enabled = device_property_read_bool(dev, "nvmem-cells");
> + if (!nvmem_enabled)
> + return 0;
> +
> + /* Set efuse value for lane0 */
> + ret = nvmem_cell_read_variable_le_u32(dev, "tx_ln0_pmos", &tx_pmos);
> + if (ret) {
> + dev_err(dev, "%s: Failed to read tx_ln0_pmos\n", __func__);
> + return ret;
> + }
> +
> + ret = nvmem_cell_read_variable_le_u32(dev, "tx_ln0_nmos", &tx_nmos);
> + if (ret) {
> + dev_err(dev, "%s: Failed to read tx_ln0_nmos\n", __func__);
> + return ret;
> + }
> +
> + ret = nvmem_cell_read_variable_le_u32(dev, "rx_ln0", &rx_data);
> + if (ret) {
> + dev_err(dev, "%s: Failed to read rx_ln0\n", __func__);
> + return ret;
> + }
> +
> + /* Don't wipe the old data if there is no data in efuse cell */
> + if (!(tx_pmos || tx_nmos || rx_data)) {
> + dev_warn(dev, "%s: No efuse data found, but dts enable it\n",
> + __func__);
> + return 0;
> + }
> +
> + mtk_phy_update_bits(pcie_phy->sif_base + PEXTP_ANA_LN0_TX_REG,
> + EFUSE_LN_TX_PMOS_SEL,
> + EFUSE_LN_TX_PMOS_VAL(tx_pmos));
> +
> + mtk_phy_update_bits(pcie_phy->sif_base + PEXTP_ANA_LN0_TX_REG,
> + EFUSE_LN_TX_NMOS_SEL,
> + EFUSE_LN_TX_NMOS_VAL(tx_nmos));
> +
> + mtk_phy_update_bits(pcie_phy->sif_base + PEXTP_ANA_LN0_RX_REG,
> + EFUSE_LN_RX_SEL, EFUSE_LN_RX_VAL(rx_data));
> +
> + /* Set global data */
> + ret = nvmem_cell_read_variable_le_u32(dev, "glb_intr", &glb_intr);
> + if (ret) {
> + dev_err(dev, "%s: Failed to read glb_intr\n", __func__);
> + return ret;
> + }
> +
> + mtk_phy_update_bits(pcie_phy->sif_base + PEXTP_ANA_GLB_00_REG,
> + EFUSE_GLB_INTR_SEL, EFUSE_GLB_INTR_VAL(glb_intr));
> +
> + /*
> + * Set efuse value for lane1, only available for the platform which
> + * supports two lane.
> + */
> + ret = nvmem_cell_read_variable_le_u32(dev, "tx_ln1_pmos", &tx_pmos);
> + if (ret) {
> + dev_err(dev, "%s: Failed to read tx_ln1_pmos, efuse value not support for lane 1\n",
> + __func__);
> + return 0;
> + }
> +
> + ret = nvmem_cell_read_variable_le_u32(dev, "tx_ln1_nmos", &tx_nmos);
> + if (ret) {
> + dev_err(dev, "%s: Failed to read tx_ln1_pmos\n", __func__);
> + return ret;
> + }
> +
> + ret = nvmem_cell_read_variable_le_u32(dev, "rx_ln1", &rx_data);
> + if (ret) {
> + dev_err(dev, "%s: Failed to read rx_ln1\n", __func__);
> + return ret;
> + }
> +
> + if (!(tx_pmos || tx_nmos || rx_data))
> + return 0;
> +
> + mtk_phy_update_bits(pcie_phy->sif_base + PEXTP_ANA_LN1_TX_REG,
> + EFUSE_LN_TX_PMOS_SEL,
> + EFUSE_LN_TX_PMOS_VAL(tx_pmos));
> +
> + mtk_phy_update_bits(pcie_phy->sif_base + PEXTP_ANA_LN1_TX_REG,
> + EFUSE_LN_TX_NMOS_SEL,
> + EFUSE_LN_TX_NMOS_VAL(tx_nmos));
> +
> + mtk_phy_update_bits(pcie_phy->sif_base + PEXTP_ANA_LN1_RX_REG,
> + EFUSE_LN_RX_SEL, EFUSE_LN_RX_VAL(rx_data));
> +
> + return 0;
> +}
> +
> +static const struct phy_ops mtk_pcie_phy_ops = {
> + .init = mtk_pcie_phy_init,
This function doesn't look like it is enabling any resources, and there
is no .exit counterpart. You could simply call this in the probe function.
Or maybe the hardware settings get reset during suspend, and you want the
settings reinitialized when the consumer calls phy_init() again? This
design limitation and decision should be noted in a comment.
Also, it is better to do the NVMEM readout at probe time, so if
nvmem_cell_read_* returns -EPROBE_DEFER, you get proper probe
sequencing.
Consumers likely won't be expecting -EPROBE_DEFER from phy_init().
> + .owner = THIS_MODULE,
> +};
> +
> +static int mtk_pcie_phy_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct phy_provider *provider;
> + struct mtk_pcie_phy *pcie_phy;
> +
> + pcie_phy = devm_kzalloc(dev, sizeof(*pcie_phy), GFP_KERNEL);
> + if (!pcie_phy)
> + return -ENOMEM;
> +
> + pcie_phy->dev = dev;
> +
> + pcie_phy->sif_base = devm_platform_ioremap_resource_byname(pdev, "sif");
> + if (IS_ERR(pcie_phy->sif_base)) {
> + dev_err(dev, "%s: Failed to map phy-sif base\n", __func__);
> + return PTR_ERR(pcie_phy->sif_base);
Use "return dev_err_probe(...)".
> + }
> +
> + pcie_phy->phy = devm_phy_create(dev, dev->of_node, &mtk_pcie_phy_ops);
> + if (IS_ERR(pcie_phy->phy)) {
> + dev_err(dev, "%s: Failed to create PCIe phy\n", __func__);
> + return PTR_ERR(pcie_phy->phy);
Same here.
> + }
> +
> + phy_set_drvdata(pcie_phy->phy, pcie_phy);
> +
> + provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> +
> + return PTR_ERR_OR_ZERO(provider);
No error message if it fails?
Also, small nit: many prefer not to use PTR_ERR_OR_ZERO, as it adds a line
that needs to be changed if the core needs to be extended.
> +}
> +
> +static const struct of_device_id mtk_pcie_phy_of_match[] = {
> + { .compatible = "mediatek,pcie-phy" },
As mentioned for the binding patch, use a SoC specific compatible string.
Regards
ChenYu
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, mtk_pcie_phy_of_match);
> +
> +static struct platform_driver mtk_pcie_phy_driver = {
> + .probe = mtk_pcie_phy_probe,
> + .driver = {
> + .name = "mtk-pcie-phy",
> + .of_match_table = mtk_pcie_phy_of_match,
> + },
> +};
> +module_platform_driver(mtk_pcie_phy_driver);
> +
> +MODULE_DESCRIPTION("MediaTek PCIe PHY driver");
> +MODULE_AUTHOR("Jianjun Wang <jianjun.wang@mediatek.com>");
> +MODULE_LICENSE("GPL v2");
> --
> 2.18.0
>
>
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] phy: mediatek: Add PCIe PHY driver
2022-03-17 7:49 ` Chen-Yu Tsai
@ 2022-03-17 9:34 ` Jianjun Wang
0 siblings, 0 replies; 8+ messages in thread
From: Jianjun Wang @ 2022-03-17 9:34 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Chunfeng Yun, Kishon Vijay Abraham I, Vinod Koul, Rob Herring,
Matthias Brugger, linux-arm-kernel, linux-mediatek, linux-phy,
devicetree, linux-kernel, rex-bc.chen, Randy.Wu, jieyy.yang,
chuanjia.liu, qizhong.cheng, jian.yang
Hi Chen-Yu,
Thanks for your review, I'll fix them in the next version based on your
comments.
Thanks.
On Thu, 2022-03-17 at 15:49 +0800, Chen-Yu Tsai wrote:
> On Fri, Mar 11, 2022 at 9:46 PM Jianjun Wang <
> jianjun.wang@mediatek.com> wrote:
> >
> > Add PCIe GEN3 PHY driver support on MediaTek chipsets.
> >
> > Signed-off-by: Jianjun Wang <jianjun.wang@mediatek.com>
> > ---
> > drivers/phy/mediatek/Kconfig | 11 ++
> > drivers/phy/mediatek/Makefile | 1 +
> > drivers/phy/mediatek/phy-mtk-pcie.c | 198
> > ++++++++++++++++++++++++++++
> > 3 files changed, 210 insertions(+)
> > create mode 100644 drivers/phy/mediatek/phy-mtk-pcie.c
> >
> > diff --git a/drivers/phy/mediatek/Kconfig
> > b/drivers/phy/mediatek/Kconfig
> > index 55f8e6c048ab..387ed1b3f2cc 100644
> > --- a/drivers/phy/mediatek/Kconfig
> > +++ b/drivers/phy/mediatek/Kconfig
> > @@ -55,3 +55,14 @@ config PHY_MTK_MIPI_DSI
> > select GENERIC_PHY
> > help
> > Support MIPI DSI for Mediatek SoCs.
> > +
> > +config PHY_MTK_PCIE
> > + tristate "MediaTek PCIe-PHY Driver"
> > + depends on ARCH_MEDIATEK || COMPILE_TEST
> > + depends on OF
> > + select GENERIC_PHY
> > + help
> > + Say 'Y' here to add support for MediaTek PCIe PHY driver.
> > + This driver create the basic PHY instance and provides
> > initialize
> > + callback for PCIe GEN3 port, it supports software efuse
> > + initialization.
> > diff --git a/drivers/phy/mediatek/Makefile
> > b/drivers/phy/mediatek/Makefile
> > index ace660fbed3a..788c13147f63 100644
> > --- a/drivers/phy/mediatek/Makefile
> > +++ b/drivers/phy/mediatek/Makefile
> > @@ -6,6 +6,7 @@
> > obj-$(CONFIG_PHY_MTK_TPHY) += phy-mtk-tphy.o
> > obj-$(CONFIG_PHY_MTK_UFS) += phy-mtk-ufs.o
> > obj-$(CONFIG_PHY_MTK_XSPHY) += phy-mtk-xsphy.o
> > +obj-$(CONFIG_PHY_MTK_PCIE) += phy-mtk-pcie.o
> >
> > phy-mtk-hdmi-drv-y := phy-mtk-hdmi.o
> > phy-mtk-hdmi-drv-y += phy-mtk-hdmi-mt2701.o
> > diff --git a/drivers/phy/mediatek/phy-mtk-pcie.c
> > b/drivers/phy/mediatek/phy-mtk-pcie.c
> > new file mode 100644
> > index 000000000000..45a67d9171f6
> > --- /dev/null
> > +++ b/drivers/phy/mediatek/phy-mtk-pcie.c
> > @@ -0,0 +1,198 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) 2022 MediaTek Inc.
> > + * Author: Jianjun Wang <jianjun.wang@mediatek.com>
> > + */
> > +
>
> Please include linux/bits.h for GENMASK().
>
> > +#include <linux/io.h>
> > +#include <linux/iopoll.h>
>
> You don't need these two headers.
>
> You could include linux/compiler_types.h for definition of __iomem.
>
> > +#include <linux/module.h>
> > +#include <linux/nvmem-consumer.h>
> > +#include <linux/of_address.h>
> > +#include <linux/of_device.h>
>
> Nor do you need these two.
>
> > +#include <linux/phy/phy.h>
> > +#include <linux/platform_device.h>
>
> Please include linux/slab.h for kmalloc() and friends.
>
> > +
> > +#include "phy-mtk-io.h"
> > +
> > +#define PEXTP_ANA_GLB_00_REG 0x9000
> > +#define PEXTP_ANA_LN0_TX_REG 0xA004
> > +#define PEXTP_ANA_LN0_RX_REG 0xA03C
> > +#define PEXTP_ANA_LN1_TX_REG 0xA104
> > +#define PEXTP_ANA_LN1_RX_REG 0xA13c
> > +
> > +/* PEXTP_GLB_00_RG[28:24] Internal Resistor Selection of TX Bias
> > Current */
> > +#define EFUSE_GLB_INTR_SEL GENMASK(28, 24)
> > +#define EFUSE_GLB_INTR_VAL(x) ((0x1f & (x)) << 24)
> > +
> > +/* PEXTP_ANA_LN_RX_RG[3:0] LN0 RX impedance selection */
> > +#define EFUSE_LN_RX_SEL GENMASK(3, 0)
> > +#define EFUSE_LN_RX_VAL(x) (0xf & (x))
> > +
> > +/* PEXTP_ANA_LN_TX_RG[5:2] LN0 TX PMOS impedance selection */
> > +#define EFUSE_LN_TX_PMOS_SEL GENMASK(5, 2)
> > +#define EFUSE_LN_TX_PMOS_VAL(x) ((0xf & (x)) << 2)
> > +
> > +/* PEXTP_ANA_LN_TX_RG[11:8] LN0 TX NMOS impedance selection */
> > +#define EFUSE_LN_TX_NMOS_SEL GENMASK(11, 8)
> > +#define EFUSE_LN_TX_NMOS_VAL(x) ((0xf & (x)) << 8)
>
> Register definitions look good. I matched them to the datasheet.
>
> > +struct mtk_pcie_phy {
> > + struct device *dev;
> > + struct phy *phy;
> > + void __iomem *sif_base;
> > +};
> > +
> > +static int mtk_pcie_phy_init(struct phy *phy)
> > +{
> > + struct mtk_pcie_phy *pcie_phy = phy_get_drvdata(phy);
> > + struct device *dev = pcie_phy->dev;
> > + bool nvmem_enabled;
> > + u32 glb_intr, tx_pmos, tx_nmos, rx_data;
> > + int ret;
> > +
> > + nvmem_enabled = device_property_read_bool(dev, "nvmem-
> > cells");
> > + if (!nvmem_enabled)
> > + return 0;
> > +
> > + /* Set efuse value for lane0 */
> > + ret = nvmem_cell_read_variable_le_u32(dev, "tx_ln0_pmos",
> > &tx_pmos);
> > + if (ret) {
> > + dev_err(dev, "%s: Failed to read tx_ln0_pmos\n",
> > __func__);
> > + return ret;
> > + }
> > +
> > + ret = nvmem_cell_read_variable_le_u32(dev, "tx_ln0_nmos",
> > &tx_nmos);
> > + if (ret) {
> > + dev_err(dev, "%s: Failed to read tx_ln0_nmos\n",
> > __func__);
> > + return ret;
> > + }
> > +
> > + ret = nvmem_cell_read_variable_le_u32(dev, "rx_ln0",
> > &rx_data);
> > + if (ret) {
> > + dev_err(dev, "%s: Failed to read rx_ln0\n",
> > __func__);
> > + return ret;
> > + }
> > +
> > + /* Don't wipe the old data if there is no data in efuse
> > cell */
> > + if (!(tx_pmos || tx_nmos || rx_data)) {
> > + dev_warn(dev, "%s: No efuse data found, but dts
> > enable it\n",
> > + __func__);
> > + return 0;
> > + }
> > +
> > + mtk_phy_update_bits(pcie_phy->sif_base +
> > PEXTP_ANA_LN0_TX_REG,
> > + EFUSE_LN_TX_PMOS_SEL,
> > + EFUSE_LN_TX_PMOS_VAL(tx_pmos));
> > +
> > + mtk_phy_update_bits(pcie_phy->sif_base +
> > PEXTP_ANA_LN0_TX_REG,
> > + EFUSE_LN_TX_NMOS_SEL,
> > + EFUSE_LN_TX_NMOS_VAL(tx_nmos));
> > +
> > + mtk_phy_update_bits(pcie_phy->sif_base +
> > PEXTP_ANA_LN0_RX_REG,
> > + EFUSE_LN_RX_SEL,
> > EFUSE_LN_RX_VAL(rx_data));
> > +
> > + /* Set global data */
> > + ret = nvmem_cell_read_variable_le_u32(dev, "glb_intr",
> > &glb_intr);
> > + if (ret) {
> > + dev_err(dev, "%s: Failed to read glb_intr\n",
> > __func__);
> > + return ret;
> > + }
> > +
> > + mtk_phy_update_bits(pcie_phy->sif_base +
> > PEXTP_ANA_GLB_00_REG,
> > + EFUSE_GLB_INTR_SEL,
> > EFUSE_GLB_INTR_VAL(glb_intr));
> > +
> > + /*
> > + * Set efuse value for lane1, only available for the
> > platform which
> > + * supports two lane.
> > + */
> > + ret = nvmem_cell_read_variable_le_u32(dev, "tx_ln1_pmos",
> > &tx_pmos);
> > + if (ret) {
> > + dev_err(dev, "%s: Failed to read tx_ln1_pmos, efuse
> > value not support for lane 1\n",
> > + __func__);
> > + return 0;
> > + }
> > +
> > + ret = nvmem_cell_read_variable_le_u32(dev, "tx_ln1_nmos",
> > &tx_nmos);
> > + if (ret) {
> > + dev_err(dev, "%s: Failed to read tx_ln1_pmos\n",
> > __func__);
> > + return ret;
> > + }
> > +
> > + ret = nvmem_cell_read_variable_le_u32(dev, "rx_ln1",
> > &rx_data);
> > + if (ret) {
> > + dev_err(dev, "%s: Failed to read rx_ln1\n",
> > __func__);
> > + return ret;
> > + }
> > +
> > + if (!(tx_pmos || tx_nmos || rx_data))
> > + return 0;
> > +
> > + mtk_phy_update_bits(pcie_phy->sif_base +
> > PEXTP_ANA_LN1_TX_REG,
> > + EFUSE_LN_TX_PMOS_SEL,
> > + EFUSE_LN_TX_PMOS_VAL(tx_pmos));
> > +
> > + mtk_phy_update_bits(pcie_phy->sif_base +
> > PEXTP_ANA_LN1_TX_REG,
> > + EFUSE_LN_TX_NMOS_SEL,
> > + EFUSE_LN_TX_NMOS_VAL(tx_nmos));
> > +
> > + mtk_phy_update_bits(pcie_phy->sif_base +
> > PEXTP_ANA_LN1_RX_REG,
> > + EFUSE_LN_RX_SEL,
> > EFUSE_LN_RX_VAL(rx_data));
> > +
> > + return 0;
> > +}
> > +
> > +static const struct phy_ops mtk_pcie_phy_ops = {
> > + .init = mtk_pcie_phy_init,
>
> This function doesn't look like it is enabling any resources, and
> there
> is no .exit counterpart. You could simply call this in the probe
> function.
>
> Or maybe the hardware settings get reset during suspend, and you want
> the
> settings reinitialized when the consumer calls phy_init() again? This
> design limitation and decision should be noted in a comment.
>
>
> Also, it is better to do the NVMEM readout at probe time, so if
> nvmem_cell_read_* returns -EPROBE_DEFER, you get proper probe
> sequencing.
>
> Consumers likely won't be expecting -EPROBE_DEFER from phy_init().
>
> > + .owner = THIS_MODULE,
> > +};
> > +
> > +static int mtk_pcie_phy_probe(struct platform_device *pdev)
> > +{
> > + struct device *dev = &pdev->dev;
> > + struct phy_provider *provider;
> > + struct mtk_pcie_phy *pcie_phy;
> > +
> > + pcie_phy = devm_kzalloc(dev, sizeof(*pcie_phy),
> > GFP_KERNEL);
> > + if (!pcie_phy)
> > + return -ENOMEM;
> > +
> > + pcie_phy->dev = dev;
> > +
> > + pcie_phy->sif_base =
> > devm_platform_ioremap_resource_byname(pdev, "sif");
> > + if (IS_ERR(pcie_phy->sif_base)) {
> > + dev_err(dev, "%s: Failed to map phy-sif base\n",
> > __func__);
> > + return PTR_ERR(pcie_phy->sif_base);
>
> Use "return dev_err_probe(...)".
>
> > + }
> > +
> > + pcie_phy->phy = devm_phy_create(dev, dev->of_node,
> > &mtk_pcie_phy_ops);
> > + if (IS_ERR(pcie_phy->phy)) {
> > + dev_err(dev, "%s: Failed to create PCIe phy\n",
> > __func__);
> > + return PTR_ERR(pcie_phy->phy);
>
> Same here.
>
> > + }
> > +
> > + phy_set_drvdata(pcie_phy->phy, pcie_phy);
> > +
> > + provider = devm_of_phy_provider_register(dev,
> > of_phy_simple_xlate);
> > +
> > + return PTR_ERR_OR_ZERO(provider);
>
> No error message if it fails?
>
> Also, small nit: many prefer not to use PTR_ERR_OR_ZERO, as it adds a
> line
> that needs to be changed if the core needs to be extended.
>
> > +}
> > +
> > +static const struct of_device_id mtk_pcie_phy_of_match[] = {
> > + { .compatible = "mediatek,pcie-phy" },
>
> As mentioned for the binding patch, use a SoC specific compatible
> string.
>
>
> Regards
> ChenYu
>
>
> > + { },
> > +};
> > +MODULE_DEVICE_TABLE(of, mtk_pcie_phy_of_match);
> > +
> > +static struct platform_driver mtk_pcie_phy_driver = {
> > + .probe = mtk_pcie_phy_probe,
> > + .driver = {
> > + .name = "mtk-pcie-phy",
> > + .of_match_table = mtk_pcie_phy_of_match,
> > + },
> > +};
> > +module_platform_driver(mtk_pcie_phy_driver);
> > +
> > +MODULE_DESCRIPTION("MediaTek PCIe PHY driver");
> > +MODULE_AUTHOR("Jianjun Wang <jianjun.wang@mediatek.com>");
> > +MODULE_LICENSE("GPL v2");
> > --
> > 2.18.0
> >
> >
> > _______________________________________________
> > Linux-mediatek mailing list
> > Linux-mediatek@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-mediatek
>
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-03-17 9:35 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-11 13:35 [PATCH 0/2] phy: mediatek: Add PCIe PHY driver Jianjun Wang
2022-03-11 13:35 ` [PATCH 1/2] " Jianjun Wang
2022-03-17 7:49 ` Chen-Yu Tsai
2022-03-17 9:34 ` Jianjun Wang
2022-03-11 13:35 ` [PATCH 2/2] dt-bindings: phy: mediatek: Add YAML schema for PCIe PHY Jianjun Wang
2022-03-11 14:28 ` Krzysztof Kozlowski
2022-03-14 1:42 ` Jianjun Wang
2022-03-15 3:15 ` Chunfeng Yun
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).