linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 1/3] usb typec: mt6360: Add support for mt6360 Type-C driver
@ 2020-08-27 11:18 cy_huang
  2020-08-27 11:18 ` [PATCH v3 2/3] usb typec: mt6360: Rename driver/Kconfig/Makefile from mt6360 to mt636x cy_huang
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: cy_huang @ 2020-08-27 11:18 UTC (permalink / raw)
  To: gregkh, robh+dt, matthias.bgg, linux, heikki.krogerus
  Cc: cy_huang, gene_chen, linux-usb, devicetree, linux-arm-kernel,
	linux-mediatek, linux-kernel

From: ChiYuan Huang <cy_huang@richtek.com>

Mediatek MT6360 is a multi-functional IC that includes USB Type-C.
It works with Type-C Port Controller Manager to provide USB PD
and USB Type-C functionalities.

v1 to v2
1. Add fix to Prevent the race condition from interrupt and tcpci port
unregister during module remove.

v2 to v3
1. Change comment style for the head of source code.
2. No need to print error for platform_get_irq_byname.
3. Fix tcpci_register_port check from IS_ERR_OR_NULL to IS_ERR.
4. Rename driver/Kconfig/Makefile form mt6360 to mt636x.
5. Rename DT binding documents from mt6360 to mt636x.

Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
---
 drivers/usb/typec/tcpm/Kconfig        |   8 ++
 drivers/usb/typec/tcpm/Makefile       |   1 +
 drivers/usb/typec/tcpm/tcpci_mt6360.c | 212 ++++++++++++++++++++++++++++++++++
 3 files changed, 221 insertions(+)
 create mode 100644 drivers/usb/typec/tcpm/tcpci_mt6360.c

diff --git a/drivers/usb/typec/tcpm/Kconfig b/drivers/usb/typec/tcpm/Kconfig
index fa3f393..58a64e1 100644
--- a/drivers/usb/typec/tcpm/Kconfig
+++ b/drivers/usb/typec/tcpm/Kconfig
@@ -27,6 +27,14 @@ config TYPEC_RT1711H
 	  Type-C Port Controller Manager to provide USB PD and USB
 	  Type-C functionalities.
 
+config TYPEC_MT6360
+	tristate "Mediatek MT6360 Type-C driver"
+	depends on MFD_MT6360
+	help
+	  Mediatek MT6360 is a multi-functional IC that includes
+	  USB Type-C. It works with Type-C Port Controller Manager
+	  to provide USB PD and USB Type-C functionalities.
+
 endif # TYPEC_TCPCI
 
 config TYPEC_FUSB302
diff --git a/drivers/usb/typec/tcpm/Makefile b/drivers/usb/typec/tcpm/Makefile
index a5ff6c8..7592ccb 100644
--- a/drivers/usb/typec/tcpm/Makefile
+++ b/drivers/usb/typec/tcpm/Makefile
@@ -5,3 +5,4 @@ obj-$(CONFIG_TYPEC_WCOVE)	+= typec_wcove.o
 typec_wcove-y			:= wcove.o
 obj-$(CONFIG_TYPEC_TCPCI)	+= tcpci.o
 obj-$(CONFIG_TYPEC_RT1711H)	+= tcpci_rt1711h.o
+obj-$(CONFIG_TYPEC_MT6360)	+= tcpci_mt6360.o
diff --git a/drivers/usb/typec/tcpm/tcpci_mt6360.c b/drivers/usb/typec/tcpm/tcpci_mt6360.c
new file mode 100644
index 00000000..f1bd9e0
--- /dev/null
+++ b/drivers/usb/typec/tcpm/tcpci_mt6360.c
@@ -0,0 +1,212 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2020 MediaTek Inc.
+ *
+ * Author: ChiYuan Huang <cy_huang@richtek.com>
+ */
+
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/usb/tcpm.h>
+
+#include "tcpci.h"
+
+#define MT6360_REG_VCONNCTRL1	0x8C
+#define MT6360_REG_MODECTRL2	0x8F
+#define MT6360_REG_SWRESET	0xA0
+#define MT6360_REG_DEBCTRL1	0xA1
+#define MT6360_REG_DRPCTRL1	0xA2
+#define MT6360_REG_DRPCTRL2	0xA3
+#define MT6360_REG_I2CTORST	0xBF
+#define MT6360_REG_RXCTRL2	0xCF
+#define MT6360_REG_CTDCTRL2	0xEC
+
+/* MT6360_REG_VCONNCTRL1 */
+#define MT6360_VCONNCL_ENABLE	BIT(0)
+/* MT6360_REG_RXCTRL2 */
+#define MT6360_OPEN40M_ENABLE	BIT(7)
+/* MT6360_REG_CTDCTRL2 */
+#define MT6360_RPONESHOT_ENABLE	BIT(6)
+
+struct mt6360_tcpc_info {
+	struct tcpci_data tdata;
+	struct tcpci *tcpci;
+	struct device *dev;
+	int irq;
+};
+
+static inline int mt6360_tcpc_read16(struct regmap *regmap,
+				     unsigned int reg, u16 *val)
+{
+	return regmap_raw_read(regmap, reg, val, sizeof(u16));
+}
+
+static inline int mt6360_tcpc_write16(struct regmap *regmap,
+				      unsigned int reg, u16 val)
+{
+	return regmap_raw_write(regmap, reg, &val, sizeof(u16));
+}
+
+static int mt6360_tcpc_init(struct tcpci *tcpci, struct tcpci_data *tdata)
+{
+	struct regmap *regmap = tdata->regmap;
+	int ret;
+
+	ret = regmap_write(regmap, MT6360_REG_SWRESET, 0x01);
+	if (ret)
+		return ret;
+
+	/* after reset command, wait 1~2ms to wait IC action */
+	usleep_range(1000, 2000);
+
+	/* write all alert to masked */
+	ret = mt6360_tcpc_write16(regmap, TCPC_ALERT_MASK, 0);
+	if (ret)
+		return ret;
+
+	/* config I2C timeout reset enable , and timeout to 200ms */
+	ret = regmap_write(regmap, MT6360_REG_I2CTORST, 0x8F);
+	if (ret)
+		return ret;
+
+	/* config CC Detect Debounce : 26.7*val us */
+	ret = regmap_write(regmap, MT6360_REG_DEBCTRL1, 0x10);
+	if (ret)
+		return ret;
+
+	/* DRP Toggle Cycle : 51.2 + 6.4*val ms */
+	ret = regmap_write(regmap, MT6360_REG_DRPCTRL1, 4);
+	if (ret)
+		return ret;
+
+	/* DRP Duyt Ctrl : dcSRC: /1024 */
+	ret = mt6360_tcpc_write16(regmap, MT6360_REG_DRPCTRL2, 330);
+	if (ret)
+		return ret;
+
+	/* Enable VCONN Current Limit function */
+	ret = regmap_update_bits(regmap, MT6360_REG_VCONNCTRL1, MT6360_VCONNCL_ENABLE,
+				 MT6360_VCONNCL_ENABLE);
+	if (ret)
+		return ret;
+
+	/* Enable cc open 40ms when pmic send vsysuv signal */
+	ret = regmap_update_bits(regmap, MT6360_REG_RXCTRL2, MT6360_OPEN40M_ENABLE,
+				 MT6360_OPEN40M_ENABLE);
+	if (ret)
+		return ret;
+
+	/* Enable Rpdet oneshot detection */
+	ret = regmap_update_bits(regmap, MT6360_REG_CTDCTRL2, MT6360_RPONESHOT_ENABLE,
+				 MT6360_RPONESHOT_ENABLE);
+	if (ret)
+		return ret;
+
+	/* Set shipping mode off, AUTOIDLE on */
+	return regmap_write(regmap, MT6360_REG_MODECTRL2, 0x7A);
+}
+
+static irqreturn_t mt6360_irq(int irq, void *dev_id)
+{
+	struct mt6360_tcpc_info *mti = dev_id;
+
+	return tcpci_irq(mti->tcpci);
+}
+
+static int mt6360_tcpc_probe(struct platform_device *pdev)
+{
+	struct mt6360_tcpc_info *mti;
+	int ret;
+
+	mti = devm_kzalloc(&pdev->dev, sizeof(*mti), GFP_KERNEL);
+	if (!mti)
+		return -ENOMEM;
+
+	mti->dev = &pdev->dev;
+
+	mti->tdata.regmap = dev_get_regmap(pdev->dev.parent, NULL);
+	if (!mti->tdata.regmap) {
+		dev_err(&pdev->dev, "Failed to get parent regmap\n");
+		return -ENODEV;
+	}
+
+	mti->irq = platform_get_irq_byname(pdev, "PD_IRQB");
+	if (mti->irq < 0)
+		return mti->irq;
+
+	mti->tdata.init = mt6360_tcpc_init;
+	mti->tcpci = tcpci_register_port(&pdev->dev, &mti->tdata);
+	if (IS_ERR(mti->tcpci)) {
+		dev_err(&pdev->dev, "Failed to register tcpci port\n");
+		return PTR_ERR(mti->tcpci);
+	}
+
+	ret = devm_request_threaded_irq(mti->dev, mti->irq, NULL, mt6360_irq, IRQF_ONESHOT,
+					dev_name(&pdev->dev), mti);
+	if (ret) {
+		dev_err(mti->dev, "Failed to register irq\n");
+		tcpci_unregister_port(mti->tcpci);
+		return ret;
+	}
+
+	device_init_wakeup(&pdev->dev, true);
+	platform_set_drvdata(pdev, mti);
+
+	return 0;
+}
+
+static int mt6360_tcpc_remove(struct platform_device *pdev)
+{
+	struct mt6360_tcpc_info *mti = platform_get_drvdata(pdev);
+
+	disable_irq(mti->irq);
+	tcpci_unregister_port(mti->tcpci);
+	return 0;
+}
+
+static int __maybe_unused mt6360_tcpc_suspend(struct device *dev)
+{
+	struct mt6360_tcpc_info *mti = dev_get_drvdata(dev);
+
+	if (device_may_wakeup(dev))
+		enable_irq_wake(mti->irq);
+
+	return 0;
+}
+
+static int __maybe_unused mt6360_tcpc_resume(struct device *dev)
+{
+	struct mt6360_tcpc_info *mti = dev_get_drvdata(dev);
+
+	if (device_may_wakeup(dev))
+		disable_irq_wake(mti->irq);
+
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(mt6360_tcpc_pm_ops, mt6360_tcpc_suspend, mt6360_tcpc_resume);
+
+static const struct of_device_id __maybe_unused mt6360_tcpc_of_id[] = {
+	{ .compatible = "mediatek,mt6360-tcpc", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, mt6360_tcpc_of_id);
+
+static struct platform_driver mt6360_tcpc_driver = {
+	.driver = {
+		.name = "mt6360-tcpc",
+		.pm = &mt6360_tcpc_pm_ops,
+		.of_match_table = mt6360_tcpc_of_id,
+	},
+	.probe = mt6360_tcpc_probe,
+	.remove = mt6360_tcpc_remove,
+};
+module_platform_driver(mt6360_tcpc_driver);
+
+MODULE_AUTHOR("ChiYuan Huang <cy_huang@richtek.com>");
+MODULE_DESCRIPTION("MT6360 USB Type-C Port Controller Interface Driver");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v3 2/3] usb typec: mt6360: Rename driver/Kconfig/Makefile from mt6360 to mt636x
  2020-08-27 11:18 [PATCH v3 1/3] usb typec: mt6360: Add support for mt6360 Type-C driver cy_huang
@ 2020-08-27 11:18 ` cy_huang
  2020-08-27 16:41   ` Guenter Roeck
  2020-08-27 11:18 ` [PATCH v3 3/3] usb typec: mt6360: Add MT6360 Type-C DT binding documentation cy_huang
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 13+ messages in thread
From: cy_huang @ 2020-08-27 11:18 UTC (permalink / raw)
  To: gregkh, robh+dt, matthias.bgg, linux, heikki.krogerus
  Cc: cy_huang, gene_chen, linux-usb, devicetree, linux-arm-kernel,
	linux-mediatek, linux-kernel

From: ChiYuan Huang <cy_huang@richtek.com>

1. Rename file form tcpci_mt6360.c to tcpci_mt636x.c
2. Rename internal function from mt6360 to mt636x, except the register
definition.
3. Change Kconfig/Makefile from MT6360 to MT636X.

Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
---
 drivers/usb/typec/tcpm/Kconfig        |   6 +-
 drivers/usb/typec/tcpm/Makefile       |   2 +-
 drivers/usb/typec/tcpm/tcpci_mt6360.c | 212 ----------------------------------
 drivers/usb/typec/tcpm/tcpci_mt636x.c | 212 ++++++++++++++++++++++++++++++++++
 4 files changed, 216 insertions(+), 216 deletions(-)
 delete mode 100644 drivers/usb/typec/tcpm/tcpci_mt6360.c
 create mode 100644 drivers/usb/typec/tcpm/tcpci_mt636x.c

diff --git a/drivers/usb/typec/tcpm/Kconfig b/drivers/usb/typec/tcpm/Kconfig
index 58a64e1..c96141f 100644
--- a/drivers/usb/typec/tcpm/Kconfig
+++ b/drivers/usb/typec/tcpm/Kconfig
@@ -27,11 +27,11 @@ config TYPEC_RT1711H
 	  Type-C Port Controller Manager to provide USB PD and USB
 	  Type-C functionalities.
 
-config TYPEC_MT6360
-	tristate "Mediatek MT6360 Type-C driver"
+config TYPEC_MT636X
+	tristate "Mediatek MT636x Type-C driver"
 	depends on MFD_MT6360
 	help
-	  Mediatek MT6360 is a multi-functional IC that includes
+	  Mediatek MT636x is a multi-functional IC that includes
 	  USB Type-C. It works with Type-C Port Controller Manager
 	  to provide USB PD and USB Type-C functionalities.
 
diff --git a/drivers/usb/typec/tcpm/Makefile b/drivers/usb/typec/tcpm/Makefile
index 7592ccb..ccd7923 100644
--- a/drivers/usb/typec/tcpm/Makefile
+++ b/drivers/usb/typec/tcpm/Makefile
@@ -5,4 +5,4 @@ obj-$(CONFIG_TYPEC_WCOVE)	+= typec_wcove.o
 typec_wcove-y			:= wcove.o
 obj-$(CONFIG_TYPEC_TCPCI)	+= tcpci.o
 obj-$(CONFIG_TYPEC_RT1711H)	+= tcpci_rt1711h.o
-obj-$(CONFIG_TYPEC_MT6360)	+= tcpci_mt6360.o
+obj-$(CONFIG_TYPEC_MT636X)	+= tcpci_mt636x.o
diff --git a/drivers/usb/typec/tcpm/tcpci_mt6360.c b/drivers/usb/typec/tcpm/tcpci_mt6360.c
deleted file mode 100644
index f1bd9e0..00000000
--- a/drivers/usb/typec/tcpm/tcpci_mt6360.c
+++ /dev/null
@@ -1,212 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-only
-/*
- * Copyright (C) 2020 MediaTek Inc.
- *
- * Author: ChiYuan Huang <cy_huang@richtek.com>
- */
-
-#include <linux/interrupt.h>
-#include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/of.h>
-#include <linux/platform_device.h>
-#include <linux/regmap.h>
-#include <linux/usb/tcpm.h>
-
-#include "tcpci.h"
-
-#define MT6360_REG_VCONNCTRL1	0x8C
-#define MT6360_REG_MODECTRL2	0x8F
-#define MT6360_REG_SWRESET	0xA0
-#define MT6360_REG_DEBCTRL1	0xA1
-#define MT6360_REG_DRPCTRL1	0xA2
-#define MT6360_REG_DRPCTRL2	0xA3
-#define MT6360_REG_I2CTORST	0xBF
-#define MT6360_REG_RXCTRL2	0xCF
-#define MT6360_REG_CTDCTRL2	0xEC
-
-/* MT6360_REG_VCONNCTRL1 */
-#define MT6360_VCONNCL_ENABLE	BIT(0)
-/* MT6360_REG_RXCTRL2 */
-#define MT6360_OPEN40M_ENABLE	BIT(7)
-/* MT6360_REG_CTDCTRL2 */
-#define MT6360_RPONESHOT_ENABLE	BIT(6)
-
-struct mt6360_tcpc_info {
-	struct tcpci_data tdata;
-	struct tcpci *tcpci;
-	struct device *dev;
-	int irq;
-};
-
-static inline int mt6360_tcpc_read16(struct regmap *regmap,
-				     unsigned int reg, u16 *val)
-{
-	return regmap_raw_read(regmap, reg, val, sizeof(u16));
-}
-
-static inline int mt6360_tcpc_write16(struct regmap *regmap,
-				      unsigned int reg, u16 val)
-{
-	return regmap_raw_write(regmap, reg, &val, sizeof(u16));
-}
-
-static int mt6360_tcpc_init(struct tcpci *tcpci, struct tcpci_data *tdata)
-{
-	struct regmap *regmap = tdata->regmap;
-	int ret;
-
-	ret = regmap_write(regmap, MT6360_REG_SWRESET, 0x01);
-	if (ret)
-		return ret;
-
-	/* after reset command, wait 1~2ms to wait IC action */
-	usleep_range(1000, 2000);
-
-	/* write all alert to masked */
-	ret = mt6360_tcpc_write16(regmap, TCPC_ALERT_MASK, 0);
-	if (ret)
-		return ret;
-
-	/* config I2C timeout reset enable , and timeout to 200ms */
-	ret = regmap_write(regmap, MT6360_REG_I2CTORST, 0x8F);
-	if (ret)
-		return ret;
-
-	/* config CC Detect Debounce : 26.7*val us */
-	ret = regmap_write(regmap, MT6360_REG_DEBCTRL1, 0x10);
-	if (ret)
-		return ret;
-
-	/* DRP Toggle Cycle : 51.2 + 6.4*val ms */
-	ret = regmap_write(regmap, MT6360_REG_DRPCTRL1, 4);
-	if (ret)
-		return ret;
-
-	/* DRP Duyt Ctrl : dcSRC: /1024 */
-	ret = mt6360_tcpc_write16(regmap, MT6360_REG_DRPCTRL2, 330);
-	if (ret)
-		return ret;
-
-	/* Enable VCONN Current Limit function */
-	ret = regmap_update_bits(regmap, MT6360_REG_VCONNCTRL1, MT6360_VCONNCL_ENABLE,
-				 MT6360_VCONNCL_ENABLE);
-	if (ret)
-		return ret;
-
-	/* Enable cc open 40ms when pmic send vsysuv signal */
-	ret = regmap_update_bits(regmap, MT6360_REG_RXCTRL2, MT6360_OPEN40M_ENABLE,
-				 MT6360_OPEN40M_ENABLE);
-	if (ret)
-		return ret;
-
-	/* Enable Rpdet oneshot detection */
-	ret = regmap_update_bits(regmap, MT6360_REG_CTDCTRL2, MT6360_RPONESHOT_ENABLE,
-				 MT6360_RPONESHOT_ENABLE);
-	if (ret)
-		return ret;
-
-	/* Set shipping mode off, AUTOIDLE on */
-	return regmap_write(regmap, MT6360_REG_MODECTRL2, 0x7A);
-}
-
-static irqreturn_t mt6360_irq(int irq, void *dev_id)
-{
-	struct mt6360_tcpc_info *mti = dev_id;
-
-	return tcpci_irq(mti->tcpci);
-}
-
-static int mt6360_tcpc_probe(struct platform_device *pdev)
-{
-	struct mt6360_tcpc_info *mti;
-	int ret;
-
-	mti = devm_kzalloc(&pdev->dev, sizeof(*mti), GFP_KERNEL);
-	if (!mti)
-		return -ENOMEM;
-
-	mti->dev = &pdev->dev;
-
-	mti->tdata.regmap = dev_get_regmap(pdev->dev.parent, NULL);
-	if (!mti->tdata.regmap) {
-		dev_err(&pdev->dev, "Failed to get parent regmap\n");
-		return -ENODEV;
-	}
-
-	mti->irq = platform_get_irq_byname(pdev, "PD_IRQB");
-	if (mti->irq < 0)
-		return mti->irq;
-
-	mti->tdata.init = mt6360_tcpc_init;
-	mti->tcpci = tcpci_register_port(&pdev->dev, &mti->tdata);
-	if (IS_ERR(mti->tcpci)) {
-		dev_err(&pdev->dev, "Failed to register tcpci port\n");
-		return PTR_ERR(mti->tcpci);
-	}
-
-	ret = devm_request_threaded_irq(mti->dev, mti->irq, NULL, mt6360_irq, IRQF_ONESHOT,
-					dev_name(&pdev->dev), mti);
-	if (ret) {
-		dev_err(mti->dev, "Failed to register irq\n");
-		tcpci_unregister_port(mti->tcpci);
-		return ret;
-	}
-
-	device_init_wakeup(&pdev->dev, true);
-	platform_set_drvdata(pdev, mti);
-
-	return 0;
-}
-
-static int mt6360_tcpc_remove(struct platform_device *pdev)
-{
-	struct mt6360_tcpc_info *mti = platform_get_drvdata(pdev);
-
-	disable_irq(mti->irq);
-	tcpci_unregister_port(mti->tcpci);
-	return 0;
-}
-
-static int __maybe_unused mt6360_tcpc_suspend(struct device *dev)
-{
-	struct mt6360_tcpc_info *mti = dev_get_drvdata(dev);
-
-	if (device_may_wakeup(dev))
-		enable_irq_wake(mti->irq);
-
-	return 0;
-}
-
-static int __maybe_unused mt6360_tcpc_resume(struct device *dev)
-{
-	struct mt6360_tcpc_info *mti = dev_get_drvdata(dev);
-
-	if (device_may_wakeup(dev))
-		disable_irq_wake(mti->irq);
-
-	return 0;
-}
-
-static SIMPLE_DEV_PM_OPS(mt6360_tcpc_pm_ops, mt6360_tcpc_suspend, mt6360_tcpc_resume);
-
-static const struct of_device_id __maybe_unused mt6360_tcpc_of_id[] = {
-	{ .compatible = "mediatek,mt6360-tcpc", },
-	{},
-};
-MODULE_DEVICE_TABLE(of, mt6360_tcpc_of_id);
-
-static struct platform_driver mt6360_tcpc_driver = {
-	.driver = {
-		.name = "mt6360-tcpc",
-		.pm = &mt6360_tcpc_pm_ops,
-		.of_match_table = mt6360_tcpc_of_id,
-	},
-	.probe = mt6360_tcpc_probe,
-	.remove = mt6360_tcpc_remove,
-};
-module_platform_driver(mt6360_tcpc_driver);
-
-MODULE_AUTHOR("ChiYuan Huang <cy_huang@richtek.com>");
-MODULE_DESCRIPTION("MT6360 USB Type-C Port Controller Interface Driver");
-MODULE_LICENSE("GPL v2");
diff --git a/drivers/usb/typec/tcpm/tcpci_mt636x.c b/drivers/usb/typec/tcpm/tcpci_mt636x.c
new file mode 100644
index 00000000..ca0f743
--- /dev/null
+++ b/drivers/usb/typec/tcpm/tcpci_mt636x.c
@@ -0,0 +1,212 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2020 MediaTek Inc.
+ *
+ * Author: ChiYuan Huang <cy_huang@richtek.com>
+ */
+
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/usb/tcpm.h>
+
+#include "tcpci.h"
+
+#define MT6360_REG_VCONNCTRL1	0x8C
+#define MT6360_REG_MODECTRL2	0x8F
+#define MT6360_REG_SWRESET	0xA0
+#define MT6360_REG_DEBCTRL1	0xA1
+#define MT6360_REG_DRPCTRL1	0xA2
+#define MT6360_REG_DRPCTRL2	0xA3
+#define MT6360_REG_I2CTORST	0xBF
+#define MT6360_REG_RXCTRL2	0xCF
+#define MT6360_REG_CTDCTRL2	0xEC
+
+/* MT6360_REG_VCONNCTRL1 */
+#define MT6360_VCONNCL_ENABLE	BIT(0)
+/* MT6360_REG_RXCTRL2 */
+#define MT6360_OPEN40M_ENABLE	BIT(7)
+/* MT6360_REG_CTDCTRL2 */
+#define MT6360_RPONESHOT_ENABLE	BIT(6)
+
+struct mt636x_tcpc_info {
+	struct tcpci_data tdata;
+	struct tcpci *tcpci;
+	struct device *dev;
+	int irq;
+};
+
+static inline int mt636x_tcpc_read16(struct regmap *regmap,
+				     unsigned int reg, u16 *val)
+{
+	return regmap_raw_read(regmap, reg, val, sizeof(u16));
+}
+
+static inline int mt636x_tcpc_write16(struct regmap *regmap,
+				      unsigned int reg, u16 val)
+{
+	return regmap_raw_write(regmap, reg, &val, sizeof(u16));
+}
+
+static int mt636x_tcpc_init(struct tcpci *tcpci, struct tcpci_data *tdata)
+{
+	struct regmap *regmap = tdata->regmap;
+	int ret;
+
+	ret = regmap_write(regmap, MT6360_REG_SWRESET, 0x01);
+	if (ret)
+		return ret;
+
+	/* after reset command, wait 1~2ms to wait IC action */
+	usleep_range(1000, 2000);
+
+	/* write all alert to masked */
+	ret = mt636x_tcpc_write16(regmap, TCPC_ALERT_MASK, 0);
+	if (ret)
+		return ret;
+
+	/* config I2C timeout reset enable , and timeout to 200ms */
+	ret = regmap_write(regmap, MT6360_REG_I2CTORST, 0x8F);
+	if (ret)
+		return ret;
+
+	/* config CC Detect Debounce : 26.7*val us */
+	ret = regmap_write(regmap, MT6360_REG_DEBCTRL1, 0x10);
+	if (ret)
+		return ret;
+
+	/* DRP Toggle Cycle : 51.2 + 6.4*val ms */
+	ret = regmap_write(regmap, MT6360_REG_DRPCTRL1, 4);
+	if (ret)
+		return ret;
+
+	/* DRP Duyt Ctrl : dcSRC: /1024 */
+	ret = mt636x_tcpc_write16(regmap, MT6360_REG_DRPCTRL2, 330);
+	if (ret)
+		return ret;
+
+	/* Enable VCONN Current Limit function */
+	ret = regmap_update_bits(regmap, MT6360_REG_VCONNCTRL1, MT6360_VCONNCL_ENABLE,
+				 MT6360_VCONNCL_ENABLE);
+	if (ret)
+		return ret;
+
+	/* Enable cc open 40ms when pmic send vsysuv signal */
+	ret = regmap_update_bits(regmap, MT6360_REG_RXCTRL2, MT6360_OPEN40M_ENABLE,
+				 MT6360_OPEN40M_ENABLE);
+	if (ret)
+		return ret;
+
+	/* Enable Rpdet oneshot detection */
+	ret = regmap_update_bits(regmap, MT6360_REG_CTDCTRL2, MT6360_RPONESHOT_ENABLE,
+				 MT6360_RPONESHOT_ENABLE);
+	if (ret)
+		return ret;
+
+	/* Set shipping mode off, AUTOIDLE on */
+	return regmap_write(regmap, MT6360_REG_MODECTRL2, 0x7A);
+}
+
+static irqreturn_t mt636x_irq(int irq, void *dev_id)
+{
+	struct mt636x_tcpc_info *mti = dev_id;
+
+	return tcpci_irq(mti->tcpci);
+}
+
+static int mt636x_tcpc_probe(struct platform_device *pdev)
+{
+	struct mt636x_tcpc_info *mti;
+	int ret;
+
+	mti = devm_kzalloc(&pdev->dev, sizeof(*mti), GFP_KERNEL);
+	if (!mti)
+		return -ENOMEM;
+
+	mti->dev = &pdev->dev;
+
+	mti->tdata.regmap = dev_get_regmap(pdev->dev.parent, NULL);
+	if (!mti->tdata.regmap) {
+		dev_err(&pdev->dev, "Failed to get parent regmap\n");
+		return -ENODEV;
+	}
+
+	mti->irq = platform_get_irq_byname(pdev, "PD_IRQB");
+	if (mti->irq < 0)
+		return mti->irq;
+
+	mti->tdata.init = mt636x_tcpc_init;
+	mti->tcpci = tcpci_register_port(&pdev->dev, &mti->tdata);
+	if (IS_ERR(mti->tcpci)) {
+		dev_err(&pdev->dev, "Failed to register tcpci port\n");
+		return PTR_ERR(mti->tcpci);
+	}
+
+	ret = devm_request_threaded_irq(mti->dev, mti->irq, NULL, mt636x_irq, IRQF_ONESHOT,
+					dev_name(&pdev->dev), mti);
+	if (ret) {
+		dev_err(mti->dev, "Failed to register irq\n");
+		tcpci_unregister_port(mti->tcpci);
+		return ret;
+	}
+
+	device_init_wakeup(&pdev->dev, true);
+	platform_set_drvdata(pdev, mti);
+
+	return 0;
+}
+
+static int mt636x_tcpc_remove(struct platform_device *pdev)
+{
+	struct mt636x_tcpc_info *mti = platform_get_drvdata(pdev);
+
+	disable_irq(mti->irq);
+	tcpci_unregister_port(mti->tcpci);
+	return 0;
+}
+
+static int __maybe_unused mt636x_tcpc_suspend(struct device *dev)
+{
+	struct mt636x_tcpc_info *mti = dev_get_drvdata(dev);
+
+	if (device_may_wakeup(dev))
+		enable_irq_wake(mti->irq);
+
+	return 0;
+}
+
+static int __maybe_unused mt636x_tcpc_resume(struct device *dev)
+{
+	struct mt636x_tcpc_info *mti = dev_get_drvdata(dev);
+
+	if (device_may_wakeup(dev))
+		disable_irq_wake(mti->irq);
+
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(mt636x_tcpc_pm_ops, mt636x_tcpc_suspend, mt636x_tcpc_resume);
+
+static const struct of_device_id __maybe_unused mt636x_tcpc_of_id[] = {
+	{ .compatible = "mediatek,mt6360-tcpc", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, mt636x_tcpc_of_id);
+
+static struct platform_driver mt636x_tcpc_driver = {
+	.driver = {
+		.name = "mt636x-tcpc",
+		.pm = &mt636x_tcpc_pm_ops,
+		.of_match_table = mt636x_tcpc_of_id,
+	},
+	.probe = mt636x_tcpc_probe,
+	.remove = mt636x_tcpc_remove,
+};
+module_platform_driver(mt636x_tcpc_driver);
+
+MODULE_AUTHOR("ChiYuan Huang <cy_huang@richtek.com>");
+MODULE_DESCRIPTION("MT636x USB Type-C Port Controller Interface Driver");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v3 3/3] usb typec: mt6360: Add MT6360 Type-C DT binding documentation
  2020-08-27 11:18 [PATCH v3 1/3] usb typec: mt6360: Add support for mt6360 Type-C driver cy_huang
  2020-08-27 11:18 ` [PATCH v3 2/3] usb typec: mt6360: Rename driver/Kconfig/Makefile from mt6360 to mt636x cy_huang
@ 2020-08-27 11:18 ` cy_huang
  2020-08-27 14:00 ` [PATCH v3 1/3] usb typec: mt6360: Add support for mt6360 Type-C driver Heikki Krogerus
  2020-08-28 12:10 ` Chunfeng Yun
  3 siblings, 0 replies; 13+ messages in thread
From: cy_huang @ 2020-08-27 11:18 UTC (permalink / raw)
  To: gregkh, robh+dt, matthias.bgg, linux, heikki.krogerus
  Cc: cy_huang, gene_chen, linux-usb, devicetree, linux-arm-kernel,
	linux-mediatek, linux-kernel

From: ChiYuan Huang <cy_huang@richtek.com>

Add a devicetree binding documentation for the MT6360 Type-C driver.

usb typec: mt6360: Rename DT binding doument from mt6360 to mt636x

Change binding document file name from mt6360 to mt636xfor the
future compatible.
Also change internal description from mt6360 to mt636x.

Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
---
 .../bindings/usb/mediatek,mt636x-tcpc.yaml         | 73 ++++++++++++++++++++++
 1 file changed, 73 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/mediatek,mt636x-tcpc.yaml

diff --git a/Documentation/devicetree/bindings/usb/mediatek,mt636x-tcpc.yaml b/Documentation/devicetree/bindings/usb/mediatek,mt636x-tcpc.yaml
new file mode 100644
index 00000000..e6d2b3d
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/mediatek,mt636x-tcpc.yaml
@@ -0,0 +1,73 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/usb/mediatek,mt636x-tcpc.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Mediatek MT636x Type-C Port Switch and Power Delivery controller DT bindings
+
+maintainers:
+  - ChiYuan Huang <cy_huang@richtek.com>
+
+description: |
+  Mediatek MT636x is a multi-functional device. It integrates charger, ADC, flash, RGB indicators,
+  regulators (BUCKs/LDOs), and TypeC Port Switch with Power Delivery controller.
+  This document only describes MT636x Type-C Port Switch and Power Delivery controller.
+
+properties:
+  compatible:
+    enum:
+      - mediatek,mt6360-tcpc
+
+  interrupts-extended:
+    maxItems: 1
+
+  interrupt-names:
+    items:
+      - const: PD_IRQB
+
+patternProperties:
+  "connector":
+    type: object
+    $ref: ../connector/usb-connector.yaml#
+    description:
+      Properties for usb c connector.
+
+additionalProperties: false
+
+required:
+  - compatible
+  - interrupts-extended
+  - interrupt-names
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/usb/pd.h>
+    i2c0 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        mt6360@34 {
+            compatible = "mediatek,mt6360";
+            reg = <0x34>;
+
+            tcpc {
+                compatible = "mediatek,mt6360-tcpc";
+                interrupts-extended = <&gpio26 3 IRQ_TYPE_LEVEL_LOW>;
+                interrupt-names = "PD_IRQB";
+
+                connector {
+                        compatible = "usb-c-connector";
+                        label = "USB-C";
+                        data-role = "dual";
+                        power-role = "dual";
+                        try-power-role = "sink";
+                        source-pdos = <PDO_FIXED(5000, 1000, PDO_FIXED_DUAL_ROLE | PDO_FIXED_DATA_SWAP)>;
+                        sink-pdos = <PDO_FIXED(5000, 2000, PDO_FIXED_DUAL_ROLE | PDO_FIXED_DATA_SWAP)>;
+                        op-sink-microwatt = <10000000>;
+                };
+            };
+        };
+    };
+...
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 1/3] usb typec: mt6360: Add support for mt6360 Type-C driver
  2020-08-27 11:18 [PATCH v3 1/3] usb typec: mt6360: Add support for mt6360 Type-C driver cy_huang
  2020-08-27 11:18 ` [PATCH v3 2/3] usb typec: mt6360: Rename driver/Kconfig/Makefile from mt6360 to mt636x cy_huang
  2020-08-27 11:18 ` [PATCH v3 3/3] usb typec: mt6360: Add MT6360 Type-C DT binding documentation cy_huang
@ 2020-08-27 14:00 ` Heikki Krogerus
  2020-08-27 14:51   ` ChiYuan Huang
  2020-08-28 12:10 ` Chunfeng Yun
  3 siblings, 1 reply; 13+ messages in thread
From: Heikki Krogerus @ 2020-08-27 14:00 UTC (permalink / raw)
  To: cy_huang
  Cc: gregkh, robh+dt, matthias.bgg, linux, cy_huang, gene_chen,
	linux-usb, devicetree, linux-arm-kernel, linux-mediatek,
	linux-kernel

On Thu, Aug 27, 2020 at 07:18:55PM +0800, cy_huang wrote:
> From: ChiYuan Huang <cy_huang@richtek.com>
> 
> Mediatek MT6360 is a multi-functional IC that includes USB Type-C.
> It works with Type-C Port Controller Manager to provide USB PD
> and USB Type-C functionalities.
> 
> v1 to v2
> 1. Add fix to Prevent the race condition from interrupt and tcpci port
> unregister during module remove.
> 
> v2 to v3
> 1. Change comment style for the head of source code.
> 2. No need to print error for platform_get_irq_byname.
> 3. Fix tcpci_register_port check from IS_ERR_OR_NULL to IS_ERR.
> 4. Rename driver/Kconfig/Makefile form mt6360 to mt636x.
> 5. Rename DT binding documents from mt6360 to mt636x.

You don't place additional changelog here...

> Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
> ---

You put it here, after that '---' marker:
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#the-canonical-patch-format


>  drivers/usb/typec/tcpm/Kconfig        |   8 ++
>  drivers/usb/typec/tcpm/Makefile       |   1 +
>  drivers/usb/typec/tcpm/tcpci_mt6360.c | 212 ++++++++++++++++++++++++++++++++++
>  3 files changed, 221 insertions(+)
>  create mode 100644 drivers/usb/typec/tcpm/tcpci_mt6360.c

thanks,

-- 
heikki

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 1/3] usb typec: mt6360: Add support for mt6360 Type-C driver
  2020-08-27 14:00 ` [PATCH v3 1/3] usb typec: mt6360: Add support for mt6360 Type-C driver Heikki Krogerus
@ 2020-08-27 14:51   ` ChiYuan Huang
  2020-08-28  9:38     ` Heikki Krogerus
  0 siblings, 1 reply; 13+ messages in thread
From: ChiYuan Huang @ 2020-08-27 14:51 UTC (permalink / raw)
  To: Heikki Krogerus
  Cc: Greg KH, robh+dt, matthias.bgg, Guenter Roeck, cy_huang,
	gene_chen, linux-usb, devicetree, linux-arm-kernel,
	linux-mediatek, linux-kernel

Heikki Krogerus <heikki.krogerus@linux.intel.com> 於 2020年8月27日 週四 下午10:00寫道:
>
> On Thu, Aug 27, 2020 at 07:18:55PM +0800, cy_huang wrote:
> > From: ChiYuan Huang <cy_huang@richtek.com>
> >
> > Mediatek MT6360 is a multi-functional IC that includes USB Type-C.
> > It works with Type-C Port Controller Manager to provide USB PD
> > and USB Type-C functionalities.
> >
> > v1 to v2
> > 1. Add fix to Prevent the race condition from interrupt and tcpci port
> > unregister during module remove.
> >
> > v2 to v3
> > 1. Change comment style for the head of source code.
> > 2. No need to print error for platform_get_irq_byname.
> > 3. Fix tcpci_register_port check from IS_ERR_OR_NULL to IS_ERR.
> > 4. Rename driver/Kconfig/Makefile form mt6360 to mt636x.
> > 5. Rename DT binding documents from mt6360 to mt636x.
>
> You don't place additional changelog here...
>
> > Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
> > ---
>
> You put it here, after that '---' marker:
> https://www.kernel.org/doc/html/latest/process/submitting-patches.html#the-canonical-patch-format
>
HI Hekki:
     after reading the document, I have a little bit confused how to
use diffstat for the changelog.
      Is there any example that make me know to write a clear
description for the changelog?
>
> >  drivers/usb/typec/tcpm/Kconfig        |   8 ++
> >  drivers/usb/typec/tcpm/Makefile       |   1 +
> >  drivers/usb/typec/tcpm/tcpci_mt6360.c | 212 ++++++++++++++++++++++++++++++++++
> >  3 files changed, 221 insertions(+)
> >  create mode 100644 drivers/usb/typec/tcpm/tcpci_mt6360.c
>
> thanks,
>
> --
> heikki

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 2/3] usb typec: mt6360: Rename driver/Kconfig/Makefile from mt6360 to mt636x
  2020-08-27 11:18 ` [PATCH v3 2/3] usb typec: mt6360: Rename driver/Kconfig/Makefile from mt6360 to mt636x cy_huang
@ 2020-08-27 16:41   ` Guenter Roeck
  2020-08-28  5:54     ` ChiYuan Huang
  0 siblings, 1 reply; 13+ messages in thread
From: Guenter Roeck @ 2020-08-27 16:41 UTC (permalink / raw)
  To: cy_huang
  Cc: gregkh, robh+dt, matthias.bgg, heikki.krogerus, cy_huang,
	gene_chen, linux-usb, devicetree, linux-arm-kernel,
	linux-mediatek, linux-kernel

On Thu, Aug 27, 2020 at 07:18:56PM +0800, cy_huang wrote:
> From: ChiYuan Huang <cy_huang@richtek.com>
> 
> 1. Rename file form tcpci_mt6360.c to tcpci_mt636x.c
> 2. Rename internal function from mt6360 to mt636x, except the register
> definition.
> 3. Change Kconfig/Makefile from MT6360 to MT636X.
> 
> Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
> ---
>  drivers/usb/typec/tcpm/Kconfig        |   6 +-
>  drivers/usb/typec/tcpm/Makefile       |   2 +-
>  drivers/usb/typec/tcpm/tcpci_mt6360.c | 212 ----------------------------------
>  drivers/usb/typec/tcpm/tcpci_mt636x.c | 212 ++++++++++++++++++++++++++++++++++
>  4 files changed, 216 insertions(+), 216 deletions(-)
>  delete mode 100644 drivers/usb/typec/tcpm/tcpci_mt6360.c
>  create mode 100644 drivers/usb/typec/tcpm/tcpci_mt636x.c

Maybe Heikki is ok with this change, but I am not, for the reasons
mentioned before. So I won't approve this patch. Note that, either
case, it should be merged with the first patch.

Guenter

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 2/3] usb typec: mt6360: Rename driver/Kconfig/Makefile from mt6360 to mt636x
  2020-08-27 16:41   ` Guenter Roeck
@ 2020-08-28  5:54     ` ChiYuan Huang
  0 siblings, 0 replies; 13+ messages in thread
From: ChiYuan Huang @ 2020-08-28  5:54 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Greg KH, robh+dt, matthias.bgg, Heikki Krogerus, cy_huang,
	gene_chen, linux-usb, devicetree, linux-arm-kernel,
	linux-mediatek, linux-kernel

Guenter Roeck <linux@roeck-us.net> 於 2020年8月28日 週五 上午12:41寫道:
>
> On Thu, Aug 27, 2020 at 07:18:56PM +0800, cy_huang wrote:
> > From: ChiYuan Huang <cy_huang@richtek.com>
> >
> > 1. Rename file form tcpci_mt6360.c to tcpci_mt636x.c
> > 2. Rename internal function from mt6360 to mt636x, except the register
> > definition.
> > 3. Change Kconfig/Makefile from MT6360 to MT636X.
> >
> > Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
> > ---
> >  drivers/usb/typec/tcpm/Kconfig        |   6 +-
> >  drivers/usb/typec/tcpm/Makefile       |   2 +-
> >  drivers/usb/typec/tcpm/tcpci_mt6360.c | 212 ----------------------------------
> >  drivers/usb/typec/tcpm/tcpci_mt636x.c | 212 ++++++++++++++++++++++++++++++++++
> >  4 files changed, 216 insertions(+), 216 deletions(-)
> >  delete mode 100644 drivers/usb/typec/tcpm/tcpci_mt6360.c
> >  create mode 100644 drivers/usb/typec/tcpm/tcpci_mt636x.c
>
> Maybe Heikki is ok with this change, but I am not, for the reasons
> mentioned before. So I won't approve this patch. Note that, either
> case, it should be merged with the first patch.

Yes, I agree with you opinion. use 636x,  the range is too large from
0 to 9, it may not all be compatible.
Even though it's also possible that the part number don't have the
same function.
So I'm going to remove the rename patch.
Do I need to add a patch named "revert"? Or just remove it. I'm not
sure which way is better.

It seems you all want the code change to be squashed into the first
code. And the second one is the DT binding. Right?


>
> Guenter

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 1/3] usb typec: mt6360: Add support for mt6360 Type-C driver
  2020-08-27 14:51   ` ChiYuan Huang
@ 2020-08-28  9:38     ` Heikki Krogerus
  2020-08-28 10:27       ` ChiYuan Huang
  0 siblings, 1 reply; 13+ messages in thread
From: Heikki Krogerus @ 2020-08-28  9:38 UTC (permalink / raw)
  To: ChiYuan Huang
  Cc: Greg KH, robh+dt, matthias.bgg, Guenter Roeck, cy_huang,
	gene_chen, linux-usb, devicetree, linux-arm-kernel,
	linux-mediatek, linux-kernel

On Thu, Aug 27, 2020 at 10:51:43PM +0800, ChiYuan Huang wrote:
> Heikki Krogerus <heikki.krogerus@linux.intel.com> 於 2020年8月27日 週四 下午10:00寫道:
> >
> > On Thu, Aug 27, 2020 at 07:18:55PM +0800, cy_huang wrote:
> > > From: ChiYuan Huang <cy_huang@richtek.com>
> > >
> > > Mediatek MT6360 is a multi-functional IC that includes USB Type-C.
> > > It works with Type-C Port Controller Manager to provide USB PD
> > > and USB Type-C functionalities.
> > >
> > > v1 to v2
> > > 1. Add fix to Prevent the race condition from interrupt and tcpci port
> > > unregister during module remove.
> > >
> > > v2 to v3
> > > 1. Change comment style for the head of source code.
> > > 2. No need to print error for platform_get_irq_byname.
> > > 3. Fix tcpci_register_port check from IS_ERR_OR_NULL to IS_ERR.
> > > 4. Rename driver/Kconfig/Makefile form mt6360 to mt636x.
> > > 5. Rename DT binding documents from mt6360 to mt636x.
> >
> > You don't place additional changelog here...
> >
> > > Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
> > > ---
> >
> > You put it here, after that '---' marker:
> > https://www.kernel.org/doc/html/latest/process/submitting-patches.html#the-canonical-patch-format
> >
> HI Hekki:
>      after reading the document, I have a little bit confused how to
> use diffstat for the changelog.
>       Is there any example that make me know to write a clear
> description for the changelog?

Picking the latest patch from linux-usb ml. with version history:
https://lore.kernel.org/linux-usb/1598083553-31896-11-git-send-email-chunfeng.yun@mediatek.com/

See how the last tag line "Signed-off-by: Chunfeng Yun..." is followed
by marker "---", which then is followed by the version history (the
version history is then also ended with the marker "---", a step that
I don't think is mandatory, but commonly used and often recommended).

That way that patch version history does not contaminate the actual
commit message.


thanks,

-- 
heikki

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 1/3] usb typec: mt6360: Add support for mt6360 Type-C driver
  2020-08-28  9:38     ` Heikki Krogerus
@ 2020-08-28 10:27       ` ChiYuan Huang
  2020-08-28 11:57         ` Heikki Krogerus
  0 siblings, 1 reply; 13+ messages in thread
From: ChiYuan Huang @ 2020-08-28 10:27 UTC (permalink / raw)
  To: Heikki Krogerus
  Cc: Greg KH, robh+dt, matthias.bgg, Guenter Roeck, cy_huang,
	gene_chen, linux-usb, devicetree, linux-arm-kernel,
	linux-mediatek, linux-kernel

Heikki Krogerus <heikki.krogerus@linux.intel.com> 於 2020年8月28日 週五 下午5:39寫道:
>
> On Thu, Aug 27, 2020 at 10:51:43PM +0800, ChiYuan Huang wrote:
> > Heikki Krogerus <heikki.krogerus@linux.intel.com> 於 2020年8月27日 週四 下午10:00寫道:
> > >
> > > On Thu, Aug 27, 2020 at 07:18:55PM +0800, cy_huang wrote:
> > > > From: ChiYuan Huang <cy_huang@richtek.com>
> > > >
> > > > Mediatek MT6360 is a multi-functional IC that includes USB Type-C.
> > > > It works with Type-C Port Controller Manager to provide USB PD
> > > > and USB Type-C functionalities.
> > > >
> > > > v1 to v2
> > > > 1. Add fix to Prevent the race condition from interrupt and tcpci port
> > > > unregister during module remove.
> > > >
> > > > v2 to v3
> > > > 1. Change comment style for the head of source code.
> > > > 2. No need to print error for platform_get_irq_byname.
> > > > 3. Fix tcpci_register_port check from IS_ERR_OR_NULL to IS_ERR.
> > > > 4. Rename driver/Kconfig/Makefile form mt6360 to mt636x.
> > > > 5. Rename DT binding documents from mt6360 to mt636x.
> > >
> > > You don't place additional changelog here...
> > >
> > > > Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
> > > > ---
> > >
> > > You put it here, after that '---' marker:
> > > https://www.kernel.org/doc/html/latest/process/submitting-patches.html#the-canonical-patch-format
> > >
> > HI Hekki:
> >      after reading the document, I have a little bit confused how to
> > use diffstat for the changelog.
> >       Is there any example that make me know to write a clear
> > description for the changelog?
>
> Picking the latest patch from linux-usb ml. with version history:
> https://lore.kernel.org/linux-usb/1598083553-31896-11-git-send-email-chunfeng.yun@mediatek.com/
>
> See how the last tag line "Signed-off-by: Chunfeng Yun..." is followed
> by marker "---", which then is followed by the version history (the
> version history is then also ended with the marker "---", a step that
> I don't think is mandatory, but commonly used and often recommended).
>
> That way that patch version history does not contaminate the actual
> commit message.
>
>
> thanks,
>
Ah, I already send the patch v4, I only add the changelog after the
sign-off --- label, but forget to add --- after changelog ended

Please let me resend the patch v4 to add --- label after the change log eded.
> --
> heikki

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 1/3] usb typec: mt6360: Add support for mt6360 Type-C driver
  2020-08-28 10:27       ` ChiYuan Huang
@ 2020-08-28 11:57         ` Heikki Krogerus
  0 siblings, 0 replies; 13+ messages in thread
From: Heikki Krogerus @ 2020-08-28 11:57 UTC (permalink / raw)
  To: ChiYuan Huang
  Cc: Greg KH, robh+dt, matthias.bgg, Guenter Roeck, cy_huang,
	gene_chen, linux-usb, devicetree, linux-arm-kernel,
	linux-mediatek, linux-kernel

On Fri, Aug 28, 2020 at 06:27:26PM +0800, ChiYuan Huang wrote:
> Heikki Krogerus <heikki.krogerus@linux.intel.com> 於 2020年8月28日 週五 下午5:39寫道:
> >
> > On Thu, Aug 27, 2020 at 10:51:43PM +0800, ChiYuan Huang wrote:
> > > Heikki Krogerus <heikki.krogerus@linux.intel.com> 於 2020年8月27日 週四 下午10:00寫道:
> > > >
> > > > On Thu, Aug 27, 2020 at 07:18:55PM +0800, cy_huang wrote:
> > > > > From: ChiYuan Huang <cy_huang@richtek.com>
> > > > >
> > > > > Mediatek MT6360 is a multi-functional IC that includes USB Type-C.
> > > > > It works with Type-C Port Controller Manager to provide USB PD
> > > > > and USB Type-C functionalities.
> > > > >
> > > > > v1 to v2
> > > > > 1. Add fix to Prevent the race condition from interrupt and tcpci port
> > > > > unregister during module remove.
> > > > >
> > > > > v2 to v3
> > > > > 1. Change comment style for the head of source code.
> > > > > 2. No need to print error for platform_get_irq_byname.
> > > > > 3. Fix tcpci_register_port check from IS_ERR_OR_NULL to IS_ERR.
> > > > > 4. Rename driver/Kconfig/Makefile form mt6360 to mt636x.
> > > > > 5. Rename DT binding documents from mt6360 to mt636x.
> > > >
> > > > You don't place additional changelog here...
> > > >
> > > > > Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
> > > > > ---
> > > >
> > > > You put it here, after that '---' marker:
> > > > https://www.kernel.org/doc/html/latest/process/submitting-patches.html#the-canonical-patch-format
> > > >
> > > HI Hekki:
> > >      after reading the document, I have a little bit confused how to
> > > use diffstat for the changelog.
> > >       Is there any example that make me know to write a clear
> > > description for the changelog?
> >
> > Picking the latest patch from linux-usb ml. with version history:
> > https://lore.kernel.org/linux-usb/1598083553-31896-11-git-send-email-chunfeng.yun@mediatek.com/
> >
> > See how the last tag line "Signed-off-by: Chunfeng Yun..." is followed
> > by marker "---", which then is followed by the version history (the
> > version history is then also ended with the marker "---", a step that
> > I don't think is mandatory, but commonly used and often recommended).
> >
> > That way that patch version history does not contaminate the actual
> > commit message.
> >
> >
> > thanks,
> >
> Ah, I already send the patch v4, I only add the changelog after the
> sign-off --- label, but forget to add --- after changelog ended
> 
> Please let me resend the patch v4 to add --- label after the change log eded.

You do not need to do that. Like I said, it is not mandatory.

thanks,

-- 
heikki

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 1/3] usb typec: mt6360: Add support for mt6360 Type-C driver
  2020-08-27 11:18 [PATCH v3 1/3] usb typec: mt6360: Add support for mt6360 Type-C driver cy_huang
                   ` (2 preceding siblings ...)
  2020-08-27 14:00 ` [PATCH v3 1/3] usb typec: mt6360: Add support for mt6360 Type-C driver Heikki Krogerus
@ 2020-08-28 12:10 ` Chunfeng Yun
  2020-08-28 14:33   ` ChiYuan Huang
  3 siblings, 1 reply; 13+ messages in thread
From: Chunfeng Yun @ 2020-08-28 12:10 UTC (permalink / raw)
  To: cy_huang
  Cc: gregkh, robh+dt, matthias.bgg, linux, heikki.krogerus, cy_huang,
	gene_chen, linux-usb, devicetree, linux-arm-kernel,
	linux-mediatek, linux-kernel

On Thu, 2020-08-27 at 19:18 +0800, cy_huang wrote:
> From: ChiYuan Huang <cy_huang@richtek.com>
> 
> Mediatek MT6360 is a multi-functional IC that includes USB Type-C.
> It works with Type-C Port Controller Manager to provide USB PD
> and USB Type-C functionalities.
> 
> v1 to v2
> 1. Add fix to Prevent the race condition from interrupt and tcpci port
> unregister during module remove.
> 
> v2 to v3
> 1. Change comment style for the head of source code.
> 2. No need to print error for platform_get_irq_byname.
> 3. Fix tcpci_register_port check from IS_ERR_OR_NULL to IS_ERR.
> 4. Rename driver/Kconfig/Makefile form mt6360 to mt636x.
> 5. Rename DT binding documents from mt6360 to mt636x.
> 
> Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
> ---
>  drivers/usb/typec/tcpm/Kconfig        |   8 ++
>  drivers/usb/typec/tcpm/Makefile       |   1 +
>  drivers/usb/typec/tcpm/tcpci_mt6360.c | 212 ++++++++++++++++++++++++++++++++++
>  3 files changed, 221 insertions(+)
>  create mode 100644 drivers/usb/typec/tcpm/tcpci_mt6360.c
> 
> diff --git a/drivers/usb/typec/tcpm/Kconfig b/drivers/usb/typec/tcpm/Kconfig
> index fa3f393..58a64e1 100644
> --- a/drivers/usb/typec/tcpm/Kconfig
> +++ b/drivers/usb/typec/tcpm/Kconfig
> @@ -27,6 +27,14 @@ config TYPEC_RT1711H
>  	  Type-C Port Controller Manager to provide USB PD and USB
>  	  Type-C functionalities.
>  
> +config TYPEC_MT6360
> +	tristate "Mediatek MT6360 Type-C driver"
> +	depends on MFD_MT6360
> +	help
> +	  Mediatek MT6360 is a multi-functional IC that includes
> +	  USB Type-C. It works with Type-C Port Controller Manager
> +	  to provide USB PD and USB Type-C functionalities.
> +
>  endif # TYPEC_TCPCI
>  
>  config TYPEC_FUSB302
> diff --git a/drivers/usb/typec/tcpm/Makefile b/drivers/usb/typec/tcpm/Makefile
> index a5ff6c8..7592ccb 100644
> --- a/drivers/usb/typec/tcpm/Makefile
> +++ b/drivers/usb/typec/tcpm/Makefile
> @@ -5,3 +5,4 @@ obj-$(CONFIG_TYPEC_WCOVE)	+= typec_wcove.o
>  typec_wcove-y			:= wcove.o
>  obj-$(CONFIG_TYPEC_TCPCI)	+= tcpci.o
>  obj-$(CONFIG_TYPEC_RT1711H)	+= tcpci_rt1711h.o
> +obj-$(CONFIG_TYPEC_MT6360)	+= tcpci_mt6360.o
> diff --git a/drivers/usb/typec/tcpm/tcpci_mt6360.c b/drivers/usb/typec/tcpm/tcpci_mt6360.c
> new file mode 100644
> index 00000000..f1bd9e0
> --- /dev/null
> +++ b/drivers/usb/typec/tcpm/tcpci_mt6360.c
> @@ -0,0 +1,212 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2020 MediaTek Inc.
> + *
> + * Author: ChiYuan Huang <cy_huang@richtek.com>
> + */
> +
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/usb/tcpm.h>
> +
> +#include "tcpci.h"
> +
> +#define MT6360_REG_VCONNCTRL1	0x8C
> +#define MT6360_REG_MODECTRL2	0x8F
> +#define MT6360_REG_SWRESET	0xA0
> +#define MT6360_REG_DEBCTRL1	0xA1
> +#define MT6360_REG_DRPCTRL1	0xA2
> +#define MT6360_REG_DRPCTRL2	0xA3
> +#define MT6360_REG_I2CTORST	0xBF
> +#define MT6360_REG_RXCTRL2	0xCF
> +#define MT6360_REG_CTDCTRL2	0xEC
> +
> +/* MT6360_REG_VCONNCTRL1 */
> +#define MT6360_VCONNCL_ENABLE	BIT(0)
> +/* MT6360_REG_RXCTRL2 */
> +#define MT6360_OPEN40M_ENABLE	BIT(7)
> +/* MT6360_REG_CTDCTRL2 */
> +#define MT6360_RPONESHOT_ENABLE	BIT(6)
> +
> +struct mt6360_tcpc_info {
> +	struct tcpci_data tdata;
> +	struct tcpci *tcpci;
> +	struct device *dev;
> +	int irq;
> +};
> +
> +static inline int mt6360_tcpc_read16(struct regmap *regmap,
> +				     unsigned int reg, u16 *val)
> +{
> +	return regmap_raw_read(regmap, reg, val, sizeof(u16));
> +}
> +
> +static inline int mt6360_tcpc_write16(struct regmap *regmap,
> +				      unsigned int reg, u16 val)
> +{
> +	return regmap_raw_write(regmap, reg, &val, sizeof(u16));
> +}
> +
> +static int mt6360_tcpc_init(struct tcpci *tcpci, struct tcpci_data *tdata)
> +{
> +	struct regmap *regmap = tdata->regmap;
> +	int ret;
> +
> +	ret = regmap_write(regmap, MT6360_REG_SWRESET, 0x01);
> +	if (ret)
> +		return ret;
> +
> +	/* after reset command, wait 1~2ms to wait IC action */
> +	usleep_range(1000, 2000);
> +
> +	/* write all alert to masked */
> +	ret = mt6360_tcpc_write16(regmap, TCPC_ALERT_MASK, 0);
Wonder why access some register by 2bytes, and others 4bytes?
can we access them all by 4bytes?

 
> +	if (ret)
> +		return ret;
> +
> +	/* config I2C timeout reset enable , and timeout to 200ms */
> +	ret = regmap_write(regmap, MT6360_REG_I2CTORST, 0x8F);
> +	if (ret)
> +		return ret;
> +
> +	/* config CC Detect Debounce : 26.7*val us */
> +	ret = regmap_write(regmap, MT6360_REG_DEBCTRL1, 0x10);
> +	if (ret)
> +		return ret;
> +
> +	/* DRP Toggle Cycle : 51.2 + 6.4*val ms */
> +	ret = regmap_write(regmap, MT6360_REG_DRPCTRL1, 4);
> +	if (ret)
> +		return ret;
> +
> +	/* DRP Duyt Ctrl : dcSRC: /1024 */
> +	ret = mt6360_tcpc_write16(regmap, MT6360_REG_DRPCTRL2, 330);
> +	if (ret)
> +		return ret;
> +
> +	/* Enable VCONN Current Limit function */
> +	ret = regmap_update_bits(regmap, MT6360_REG_VCONNCTRL1, MT6360_VCONNCL_ENABLE,
> +				 MT6360_VCONNCL_ENABLE);
> +	if (ret)
> +		return ret;
> +
> +	/* Enable cc open 40ms when pmic send vsysuv signal */
> +	ret = regmap_update_bits(regmap, MT6360_REG_RXCTRL2, MT6360_OPEN40M_ENABLE,
> +				 MT6360_OPEN40M_ENABLE);
> +	if (ret)
> +		return ret;
> +
> +	/* Enable Rpdet oneshot detection */
> +	ret = regmap_update_bits(regmap, MT6360_REG_CTDCTRL2, MT6360_RPONESHOT_ENABLE,
> +				 MT6360_RPONESHOT_ENABLE);
> +	if (ret)
> +		return ret;
> +
> +	/* Set shipping mode off, AUTOIDLE on */
> +	return regmap_write(regmap, MT6360_REG_MODECTRL2, 0x7A);
> +}
> +
> +static irqreturn_t mt6360_irq(int irq, void *dev_id)
> +{
> +	struct mt6360_tcpc_info *mti = dev_id;
> +
> +	return tcpci_irq(mti->tcpci);
> +}
> +
> +static int mt6360_tcpc_probe(struct platform_device *pdev)
> +{
> +	struct mt6360_tcpc_info *mti;
> +	int ret;
> +
> +	mti = devm_kzalloc(&pdev->dev, sizeof(*mti), GFP_KERNEL);
> +	if (!mti)
> +		return -ENOMEM;
> +
> +	mti->dev = &pdev->dev;
> +
> +	mti->tdata.regmap = dev_get_regmap(pdev->dev.parent, NULL);
> +	if (!mti->tdata.regmap) {
> +		dev_err(&pdev->dev, "Failed to get parent regmap\n");
> +		return -ENODEV;
> +	}
> +
> +	mti->irq = platform_get_irq_byname(pdev, "PD_IRQB");
Use lowercase letters for irq name?

> +	if (mti->irq < 0)
> +		return mti->irq;
> +
> +	mti->tdata.init = mt6360_tcpc_init;
> +	mti->tcpci = tcpci_register_port(&pdev->dev, &mti->tdata);
> +	if (IS_ERR(mti->tcpci)) {
> +		dev_err(&pdev->dev, "Failed to register tcpci port\n");
> +		return PTR_ERR(mti->tcpci);
> +	}
> +
> +	ret = devm_request_threaded_irq(mti->dev, mti->irq, NULL, mt6360_irq, IRQF_ONESHOT,
> +					dev_name(&pdev->dev), mti);
> +	if (ret) {
> +		dev_err(mti->dev, "Failed to register irq\n");
> +		tcpci_unregister_port(mti->tcpci);
> +		return ret;
> +	}
> +
> +	device_init_wakeup(&pdev->dev, true);
> +	platform_set_drvdata(pdev, mti);
> +
> +	return 0;
> +}
> +
> +static int mt6360_tcpc_remove(struct platform_device *pdev)
> +{
> +	struct mt6360_tcpc_info *mti = platform_get_drvdata(pdev);
> +
> +	disable_irq(mti->irq);
> +	tcpci_unregister_port(mti->tcpci);
> +	return 0;
> +}
> +
> +static int __maybe_unused mt6360_tcpc_suspend(struct device *dev)
> +{
> +	struct mt6360_tcpc_info *mti = dev_get_drvdata(dev);
> +
> +	if (device_may_wakeup(dev))
> +		enable_irq_wake(mti->irq);
> +
> +	return 0;
> +}
> +
> +static int __maybe_unused mt6360_tcpc_resume(struct device *dev)
> +{
> +	struct mt6360_tcpc_info *mti = dev_get_drvdata(dev);
> +
> +	if (device_may_wakeup(dev))
> +		disable_irq_wake(mti->irq);
> +
> +	return 0;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(mt6360_tcpc_pm_ops, mt6360_tcpc_suspend, mt6360_tcpc_resume);
> +
> +static const struct of_device_id __maybe_unused mt6360_tcpc_of_id[] = {
> +	{ .compatible = "mediatek,mt6360-tcpc", },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, mt6360_tcpc_of_id);
> +
> +static struct platform_driver mt6360_tcpc_driver = {
> +	.driver = {
> +		.name = "mt6360-tcpc",
> +		.pm = &mt6360_tcpc_pm_ops,
> +		.of_match_table = mt6360_tcpc_of_id,
> +	},
> +	.probe = mt6360_tcpc_probe,
> +	.remove = mt6360_tcpc_remove,
> +};
> +module_platform_driver(mt6360_tcpc_driver);
> +
> +MODULE_AUTHOR("ChiYuan Huang <cy_huang@richtek.com>");
> +MODULE_DESCRIPTION("MT6360 USB Type-C Port Controller Interface Driver");
> +MODULE_LICENSE("GPL v2");


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 1/3] usb typec: mt6360: Add support for mt6360 Type-C driver
  2020-08-28 12:10 ` Chunfeng Yun
@ 2020-08-28 14:33   ` ChiYuan Huang
  2020-08-28 14:48     ` Guenter Roeck
  0 siblings, 1 reply; 13+ messages in thread
From: ChiYuan Huang @ 2020-08-28 14:33 UTC (permalink / raw)
  To: Chunfeng Yun
  Cc: Greg KH, robh+dt, matthias.bgg, Guenter Roeck, Heikki Krogerus,
	cy_huang, gene_chen, linux-usb, devicetree, linux-arm-kernel,
	linux-mediatek, linux-kernel

Chunfeng Yun <chunfeng.yun@mediatek.com> 於 2020年8月28日 週五 下午8:11寫道:
>
> On Thu, 2020-08-27 at 19:18 +0800, cy_huang wrote:
> > From: ChiYuan Huang <cy_huang@richtek.com>
> >
> > Mediatek MT6360 is a multi-functional IC that includes USB Type-C.
> > It works with Type-C Port Controller Manager to provide USB PD
> > and USB Type-C functionalities.
> >
> > v1 to v2
> > 1. Add fix to Prevent the race condition from interrupt and tcpci port
> > unregister during module remove.
> >
> > v2 to v3
> > 1. Change comment style for the head of source code.
> > 2. No need to print error for platform_get_irq_byname.
> > 3. Fix tcpci_register_port check from IS_ERR_OR_NULL to IS_ERR.
> > 4. Rename driver/Kconfig/Makefile form mt6360 to mt636x.
> > 5. Rename DT binding documents from mt6360 to mt636x.
> >
> > Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
> > ---
> >  drivers/usb/typec/tcpm/Kconfig        |   8 ++
> >  drivers/usb/typec/tcpm/Makefile       |   1 +
> >  drivers/usb/typec/tcpm/tcpci_mt6360.c | 212 ++++++++++++++++++++++++++++++++++
> >  3 files changed, 221 insertions(+)
> >  create mode 100644 drivers/usb/typec/tcpm/tcpci_mt6360.c
> >
> > diff --git a/drivers/usb/typec/tcpm/Kconfig b/drivers/usb/typec/tcpm/Kconfig
> > index fa3f393..58a64e1 100644
> > --- a/drivers/usb/typec/tcpm/Kconfig
> > +++ b/drivers/usb/typec/tcpm/Kconfig
> > @@ -27,6 +27,14 @@ config TYPEC_RT1711H
> >         Type-C Port Controller Manager to provide USB PD and USB
> >         Type-C functionalities.
> >
> > +config TYPEC_MT6360
> > +     tristate "Mediatek MT6360 Type-C driver"
> > +     depends on MFD_MT6360
> > +     help
> > +       Mediatek MT6360 is a multi-functional IC that includes
> > +       USB Type-C. It works with Type-C Port Controller Manager
> > +       to provide USB PD and USB Type-C functionalities.
> > +
> >  endif # TYPEC_TCPCI
> >
> >  config TYPEC_FUSB302
> > diff --git a/drivers/usb/typec/tcpm/Makefile b/drivers/usb/typec/tcpm/Makefile
> > index a5ff6c8..7592ccb 100644
> > --- a/drivers/usb/typec/tcpm/Makefile
> > +++ b/drivers/usb/typec/tcpm/Makefile
> > @@ -5,3 +5,4 @@ obj-$(CONFIG_TYPEC_WCOVE)     += typec_wcove.o
> >  typec_wcove-y                        := wcove.o
> >  obj-$(CONFIG_TYPEC_TCPCI)    += tcpci.o
> >  obj-$(CONFIG_TYPEC_RT1711H)  += tcpci_rt1711h.o
> > +obj-$(CONFIG_TYPEC_MT6360)   += tcpci_mt6360.o
> > diff --git a/drivers/usb/typec/tcpm/tcpci_mt6360.c b/drivers/usb/typec/tcpm/tcpci_mt6360.c
> > new file mode 100644
> > index 00000000..f1bd9e0
> > --- /dev/null
> > +++ b/drivers/usb/typec/tcpm/tcpci_mt6360.c
> > @@ -0,0 +1,212 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright (C) 2020 MediaTek Inc.
> > + *
> > + * Author: ChiYuan Huang <cy_huang@richtek.com>
> > + */
> > +
> > +#include <linux/interrupt.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/regmap.h>
> > +#include <linux/usb/tcpm.h>
> > +
> > +#include "tcpci.h"
> > +
> > +#define MT6360_REG_VCONNCTRL1        0x8C
> > +#define MT6360_REG_MODECTRL2 0x8F
> > +#define MT6360_REG_SWRESET   0xA0
> > +#define MT6360_REG_DEBCTRL1  0xA1
> > +#define MT6360_REG_DRPCTRL1  0xA2
> > +#define MT6360_REG_DRPCTRL2  0xA3
> > +#define MT6360_REG_I2CTORST  0xBF
> > +#define MT6360_REG_RXCTRL2   0xCF
> > +#define MT6360_REG_CTDCTRL2  0xEC
> > +
> > +/* MT6360_REG_VCONNCTRL1 */
> > +#define MT6360_VCONNCL_ENABLE        BIT(0)
> > +/* MT6360_REG_RXCTRL2 */
> > +#define MT6360_OPEN40M_ENABLE        BIT(7)
> > +/* MT6360_REG_CTDCTRL2 */
> > +#define MT6360_RPONESHOT_ENABLE      BIT(6)
> > +
> > +struct mt6360_tcpc_info {
> > +     struct tcpci_data tdata;
> > +     struct tcpci *tcpci;
> > +     struct device *dev;
> > +     int irq;
> > +};
> > +
> > +static inline int mt6360_tcpc_read16(struct regmap *regmap,
> > +                                  unsigned int reg, u16 *val)
> > +{
> > +     return regmap_raw_read(regmap, reg, val, sizeof(u16));
> > +}
> > +
> > +static inline int mt6360_tcpc_write16(struct regmap *regmap,
> > +                                   unsigned int reg, u16 val)
> > +{
> > +     return regmap_raw_write(regmap, reg, &val, sizeof(u16));
> > +}
> > +
> > +static int mt6360_tcpc_init(struct tcpci *tcpci, struct tcpci_data *tdata)
> > +{
> > +     struct regmap *regmap = tdata->regmap;
> > +     int ret;
> > +
> > +     ret = regmap_write(regmap, MT6360_REG_SWRESET, 0x01);
> > +     if (ret)
> > +             return ret;
> > +
> > +     /* after reset command, wait 1~2ms to wait IC action */
> > +     usleep_range(1000, 2000);
> > +
> > +     /* write all alert to masked */
> > +     ret = mt6360_tcpc_write16(regmap, TCPC_ALERT_MASK, 0);
> Wonder why access some register by 2bytes, and others 4bytes?
> can we access them all by 4bytes?

Actually, the access bus is still I2C.
Each register is still one byte only.
Register address like as TCPC_ALERT_MASK are continuous two byte used
as IRQ MASK.
That's why it's used wirte16 function to access.

Actually, you also can divided into two bytes write into one byte
access by two times.
For I2C bus efficiency, it seems bad.
>
>
> > +     if (ret)
> > +             return ret;
> > +
> > +     /* config I2C timeout reset enable , and timeout to 200ms */
> > +     ret = regmap_write(regmap, MT6360_REG_I2CTORST, 0x8F);
> > +     if (ret)
> > +             return ret;
> > +
> > +     /* config CC Detect Debounce : 26.7*val us */
> > +     ret = regmap_write(regmap, MT6360_REG_DEBCTRL1, 0x10);
> > +     if (ret)
> > +             return ret;
> > +
> > +     /* DRP Toggle Cycle : 51.2 + 6.4*val ms */
> > +     ret = regmap_write(regmap, MT6360_REG_DRPCTRL1, 4);
> > +     if (ret)
> > +             return ret;
> > +
> > +     /* DRP Duyt Ctrl : dcSRC: /1024 */
> > +     ret = mt6360_tcpc_write16(regmap, MT6360_REG_DRPCTRL2, 330);
> > +     if (ret)
> > +             return ret;
> > +
> > +     /* Enable VCONN Current Limit function */
> > +     ret = regmap_update_bits(regmap, MT6360_REG_VCONNCTRL1, MT6360_VCONNCL_ENABLE,
> > +                              MT6360_VCONNCL_ENABLE);
> > +     if (ret)
> > +             return ret;
> > +
> > +     /* Enable cc open 40ms when pmic send vsysuv signal */
> > +     ret = regmap_update_bits(regmap, MT6360_REG_RXCTRL2, MT6360_OPEN40M_ENABLE,
> > +                              MT6360_OPEN40M_ENABLE);
> > +     if (ret)
> > +             return ret;
> > +
> > +     /* Enable Rpdet oneshot detection */
> > +     ret = regmap_update_bits(regmap, MT6360_REG_CTDCTRL2, MT6360_RPONESHOT_ENABLE,
> > +                              MT6360_RPONESHOT_ENABLE);
> > +     if (ret)
> > +             return ret;
> > +
> > +     /* Set shipping mode off, AUTOIDLE on */
> > +     return regmap_write(regmap, MT6360_REG_MODECTRL2, 0x7A);
> > +}
> > +
> > +static irqreturn_t mt6360_irq(int irq, void *dev_id)
> > +{
> > +     struct mt6360_tcpc_info *mti = dev_id;
> > +
> > +     return tcpci_irq(mti->tcpci);
> > +}
> > +
> > +static int mt6360_tcpc_probe(struct platform_device *pdev)
> > +{
> > +     struct mt6360_tcpc_info *mti;
> > +     int ret;
> > +
> > +     mti = devm_kzalloc(&pdev->dev, sizeof(*mti), GFP_KERNEL);
> > +     if (!mti)
> > +             return -ENOMEM;
> > +
> > +     mti->dev = &pdev->dev;
> > +
> > +     mti->tdata.regmap = dev_get_regmap(pdev->dev.parent, NULL);
> > +     if (!mti->tdata.regmap) {
> > +             dev_err(&pdev->dev, "Failed to get parent regmap\n");
> > +             return -ENODEV;
> > +     }
> > +
> > +     mti->irq = platform_get_irq_byname(pdev, "PD_IRQB");
> Use lowercase letters for irq name?

PD_IRQB is defined in MT6360 pinout. The datasheet is also labeled as PD_IRQB.
My thinking is to keep the pinout name like as the datasheet defined.
B means bar to be identified the pin is active low.
>
> > +     if (mti->irq < 0)
> > +             return mti->irq;
> > +
> > +     mti->tdata.init = mt6360_tcpc_init;
> > +     mti->tcpci = tcpci_register_port(&pdev->dev, &mti->tdata);
> > +     if (IS_ERR(mti->tcpci)) {
> > +             dev_err(&pdev->dev, "Failed to register tcpci port\n");
> > +             return PTR_ERR(mti->tcpci);
> > +     }
> > +
> > +     ret = devm_request_threaded_irq(mti->dev, mti->irq, NULL, mt6360_irq, IRQF_ONESHOT,
> > +                                     dev_name(&pdev->dev), mti);
> > +     if (ret) {
> > +             dev_err(mti->dev, "Failed to register irq\n");
> > +             tcpci_unregister_port(mti->tcpci);
> > +             return ret;
> > +     }
> > +
> > +     device_init_wakeup(&pdev->dev, true);
> > +     platform_set_drvdata(pdev, mti);
> > +
> > +     return 0;
> > +}
> > +
> > +static int mt6360_tcpc_remove(struct platform_device *pdev)
> > +{
> > +     struct mt6360_tcpc_info *mti = platform_get_drvdata(pdev);
> > +
> > +     disable_irq(mti->irq);
> > +     tcpci_unregister_port(mti->tcpci);
> > +     return 0;
> > +}
> > +
> > +static int __maybe_unused mt6360_tcpc_suspend(struct device *dev)
> > +{
> > +     struct mt6360_tcpc_info *mti = dev_get_drvdata(dev);
> > +
> > +     if (device_may_wakeup(dev))
> > +             enable_irq_wake(mti->irq);
> > +
> > +     return 0;
> > +}
> > +
> > +static int __maybe_unused mt6360_tcpc_resume(struct device *dev)
> > +{
> > +     struct mt6360_tcpc_info *mti = dev_get_drvdata(dev);
> > +
> > +     if (device_may_wakeup(dev))
> > +             disable_irq_wake(mti->irq);
> > +
> > +     return 0;
> > +}
> > +
> > +static SIMPLE_DEV_PM_OPS(mt6360_tcpc_pm_ops, mt6360_tcpc_suspend, mt6360_tcpc_resume);
> > +
> > +static const struct of_device_id __maybe_unused mt6360_tcpc_of_id[] = {
> > +     { .compatible = "mediatek,mt6360-tcpc", },
> > +     {},
> > +};
> > +MODULE_DEVICE_TABLE(of, mt6360_tcpc_of_id);
> > +
> > +static struct platform_driver mt6360_tcpc_driver = {
> > +     .driver = {
> > +             .name = "mt6360-tcpc",
> > +             .pm = &mt6360_tcpc_pm_ops,
> > +             .of_match_table = mt6360_tcpc_of_id,
> > +     },
> > +     .probe = mt6360_tcpc_probe,
> > +     .remove = mt6360_tcpc_remove,
> > +};
> > +module_platform_driver(mt6360_tcpc_driver);
> > +
> > +MODULE_AUTHOR("ChiYuan Huang <cy_huang@richtek.com>");
> > +MODULE_DESCRIPTION("MT6360 USB Type-C Port Controller Interface Driver");
> > +MODULE_LICENSE("GPL v2");
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 1/3] usb typec: mt6360: Add support for mt6360 Type-C driver
  2020-08-28 14:33   ` ChiYuan Huang
@ 2020-08-28 14:48     ` Guenter Roeck
  0 siblings, 0 replies; 13+ messages in thread
From: Guenter Roeck @ 2020-08-28 14:48 UTC (permalink / raw)
  To: ChiYuan Huang, Chunfeng Yun
  Cc: Greg KH, robh+dt, matthias.bgg, Heikki Krogerus, cy_huang,
	gene_chen, linux-usb, devicetree, linux-arm-kernel,
	linux-mediatek, linux-kernel

On 8/28/20 7:33 AM, ChiYuan Huang wrote:
> Chunfeng Yun <chunfeng.yun@mediatek.com> 於 2020年8月28日 週五 下午8:11寫道:
>>
>> On Thu, 2020-08-27 at 19:18 +0800, cy_huang wrote:
>>> From: ChiYuan Huang <cy_huang@richtek.com>
>>>
>>> Mediatek MT6360 is a multi-functional IC that includes USB Type-C.
>>> It works with Type-C Port Controller Manager to provide USB PD
>>> and USB Type-C functionalities.
>>>
>>> v1 to v2
>>> 1. Add fix to Prevent the race condition from interrupt and tcpci port
>>> unregister during module remove.
>>>
>>> v2 to v3
>>> 1. Change comment style for the head of source code.
>>> 2. No need to print error for platform_get_irq_byname.
>>> 3. Fix tcpci_register_port check from IS_ERR_OR_NULL to IS_ERR.
>>> 4. Rename driver/Kconfig/Makefile form mt6360 to mt636x.
>>> 5. Rename DT binding documents from mt6360 to mt636x.
>>>
>>> Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
>>> ---
>>>  drivers/usb/typec/tcpm/Kconfig        |   8 ++
>>>  drivers/usb/typec/tcpm/Makefile       |   1 +
>>>  drivers/usb/typec/tcpm/tcpci_mt6360.c | 212 ++++++++++++++++++++++++++++++++++
>>>  3 files changed, 221 insertions(+)
>>>  create mode 100644 drivers/usb/typec/tcpm/tcpci_mt6360.c
>>>
>>> diff --git a/drivers/usb/typec/tcpm/Kconfig b/drivers/usb/typec/tcpm/Kconfig
>>> index fa3f393..58a64e1 100644
>>> --- a/drivers/usb/typec/tcpm/Kconfig
>>> +++ b/drivers/usb/typec/tcpm/Kconfig
>>> @@ -27,6 +27,14 @@ config TYPEC_RT1711H
>>>         Type-C Port Controller Manager to provide USB PD and USB
>>>         Type-C functionalities.
>>>
>>> +config TYPEC_MT6360
>>> +     tristate "Mediatek MT6360 Type-C driver"
>>> +     depends on MFD_MT6360
>>> +     help
>>> +       Mediatek MT6360 is a multi-functional IC that includes
>>> +       USB Type-C. It works with Type-C Port Controller Manager
>>> +       to provide USB PD and USB Type-C functionalities.
>>> +
>>>  endif # TYPEC_TCPCI
>>>
>>>  config TYPEC_FUSB302
>>> diff --git a/drivers/usb/typec/tcpm/Makefile b/drivers/usb/typec/tcpm/Makefile
>>> index a5ff6c8..7592ccb 100644
>>> --- a/drivers/usb/typec/tcpm/Makefile
>>> +++ b/drivers/usb/typec/tcpm/Makefile
>>> @@ -5,3 +5,4 @@ obj-$(CONFIG_TYPEC_WCOVE)     += typec_wcove.o
>>>  typec_wcove-y                        := wcove.o
>>>  obj-$(CONFIG_TYPEC_TCPCI)    += tcpci.o
>>>  obj-$(CONFIG_TYPEC_RT1711H)  += tcpci_rt1711h.o
>>> +obj-$(CONFIG_TYPEC_MT6360)   += tcpci_mt6360.o
>>> diff --git a/drivers/usb/typec/tcpm/tcpci_mt6360.c b/drivers/usb/typec/tcpm/tcpci_mt6360.c
>>> new file mode 100644
>>> index 00000000..f1bd9e0
>>> --- /dev/null
>>> +++ b/drivers/usb/typec/tcpm/tcpci_mt6360.c
>>> @@ -0,0 +1,212 @@
>>> +// SPDX-License-Identifier: GPL-2.0-only
>>> +/*
>>> + * Copyright (C) 2020 MediaTek Inc.
>>> + *
>>> + * Author: ChiYuan Huang <cy_huang@richtek.com>
>>> + */
>>> +
>>> +#include <linux/interrupt.h>
>>> +#include <linux/kernel.h>
>>> +#include <linux/module.h>
>>> +#include <linux/of.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/regmap.h>
>>> +#include <linux/usb/tcpm.h>
>>> +
>>> +#include "tcpci.h"
>>> +
>>> +#define MT6360_REG_VCONNCTRL1        0x8C
>>> +#define MT6360_REG_MODECTRL2 0x8F
>>> +#define MT6360_REG_SWRESET   0xA0
>>> +#define MT6360_REG_DEBCTRL1  0xA1
>>> +#define MT6360_REG_DRPCTRL1  0xA2
>>> +#define MT6360_REG_DRPCTRL2  0xA3
>>> +#define MT6360_REG_I2CTORST  0xBF
>>> +#define MT6360_REG_RXCTRL2   0xCF
>>> +#define MT6360_REG_CTDCTRL2  0xEC
>>> +
>>> +/* MT6360_REG_VCONNCTRL1 */
>>> +#define MT6360_VCONNCL_ENABLE        BIT(0)
>>> +/* MT6360_REG_RXCTRL2 */
>>> +#define MT6360_OPEN40M_ENABLE        BIT(7)
>>> +/* MT6360_REG_CTDCTRL2 */
>>> +#define MT6360_RPONESHOT_ENABLE      BIT(6)
>>> +
>>> +struct mt6360_tcpc_info {
>>> +     struct tcpci_data tdata;
>>> +     struct tcpci *tcpci;
>>> +     struct device *dev;
>>> +     int irq;
>>> +};
>>> +
>>> +static inline int mt6360_tcpc_read16(struct regmap *regmap,
>>> +                                  unsigned int reg, u16 *val)
>>> +{
>>> +     return regmap_raw_read(regmap, reg, val, sizeof(u16));
>>> +}
>>> +
>>> +static inline int mt6360_tcpc_write16(struct regmap *regmap,
>>> +                                   unsigned int reg, u16 val)
>>> +{
>>> +     return regmap_raw_write(regmap, reg, &val, sizeof(u16));
>>> +}
>>> +
>>> +static int mt6360_tcpc_init(struct tcpci *tcpci, struct tcpci_data *tdata)
>>> +{
>>> +     struct regmap *regmap = tdata->regmap;
>>> +     int ret;
>>> +
>>> +     ret = regmap_write(regmap, MT6360_REG_SWRESET, 0x01);
>>> +     if (ret)
>>> +             return ret;
>>> +
>>> +     /* after reset command, wait 1~2ms to wait IC action */
>>> +     usleep_range(1000, 2000);
>>> +
>>> +     /* write all alert to masked */
>>> +     ret = mt6360_tcpc_write16(regmap, TCPC_ALERT_MASK, 0);
>> Wonder why access some register by 2bytes, and others 4bytes?
>> can we access them all by 4bytes?
> 
> Actually, the access bus is still I2C.
> Each register is still one byte only.
> Register address like as TCPC_ALERT_MASK are continuous two byte used
> as IRQ MASK.
> That's why it's used wirte16 function to access.
> 
> Actually, you also can divided into two bytes write into one byte
> access by two times.
> For I2C bus efficiency, it seems bad.

For MT6360_REG_DRPCTRL2, it might also be racy, since the two i2c addresses
are really one register containing a 10-bit value. I would probably not matter
here (afaics the value is changed from the default 327 to 330), but in general
it seems safer to write such a value with a single 16-bit i2c write.

Guenter

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2020-08-28 14:49 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-27 11:18 [PATCH v3 1/3] usb typec: mt6360: Add support for mt6360 Type-C driver cy_huang
2020-08-27 11:18 ` [PATCH v3 2/3] usb typec: mt6360: Rename driver/Kconfig/Makefile from mt6360 to mt636x cy_huang
2020-08-27 16:41   ` Guenter Roeck
2020-08-28  5:54     ` ChiYuan Huang
2020-08-27 11:18 ` [PATCH v3 3/3] usb typec: mt6360: Add MT6360 Type-C DT binding documentation cy_huang
2020-08-27 14:00 ` [PATCH v3 1/3] usb typec: mt6360: Add support for mt6360 Type-C driver Heikki Krogerus
2020-08-27 14:51   ` ChiYuan Huang
2020-08-28  9:38     ` Heikki Krogerus
2020-08-28 10:27       ` ChiYuan Huang
2020-08-28 11:57         ` Heikki Krogerus
2020-08-28 12:10 ` Chunfeng Yun
2020-08-28 14:33   ` ChiYuan Huang
2020-08-28 14:48     ` Guenter Roeck

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).