* [PATCH V7 1/4] i2c: gpio: Add support on ACPI-based system
2022-12-19 12:28 [PATCH V7 0/4] i2c: ls2x: Add support for the Loongson-2K/LS7A I2C controller Binbin Zhou
@ 2022-12-19 12:28 ` Binbin Zhou
2022-12-19 12:28 ` [PATCH V7 2/4] dt-bindings: i2c: add Loongson LS2X I2C controller Binbin Zhou
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: Binbin Zhou @ 2022-12-19 12:28 UTC (permalink / raw)
To: Wolfram Sang, Wolfram Sang, Andy Shevchenko, Mika Westerberg,
linux-i2c
Cc: loongarch, devicetree, Huacai Chen, WANG Xuerui, Andy Shevchenko,
Arnd Bergmann, Rob Herring, Krzysztof Kozlowski, Jianmin Lv,
Binbin Zhou
Add support for the ACPI-based device registration, so that the driver
can be also enabled through ACPI table.
Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
drivers/i2c/busses/i2c-gpio.c | 28 ++++++++++++++++++----------
1 file changed, 18 insertions(+), 10 deletions(-)
diff --git a/drivers/i2c/busses/i2c-gpio.c b/drivers/i2c/busses/i2c-gpio.c
index 0e4385a9bcf7..680936234ef8 100644
--- a/drivers/i2c/busses/i2c-gpio.c
+++ b/drivers/i2c/busses/i2c-gpio.c
@@ -13,9 +13,9 @@
#include <linux/init.h>
#include <linux/interrupt.h>
#include <linux/module.h>
-#include <linux/of.h>
#include <linux/platform_data/i2c-gpio.h>
#include <linux/platform_device.h>
+#include <linux/property.h>
#include <linux/slab.h>
struct i2c_gpio_private_data {
@@ -300,22 +300,23 @@ static inline void i2c_gpio_fault_injector_init(struct platform_device *pdev) {}
static inline void i2c_gpio_fault_injector_exit(struct platform_device *pdev) {}
#endif /* CONFIG_I2C_GPIO_FAULT_INJECTOR*/
-static void of_i2c_gpio_get_props(struct device_node *np,
- struct i2c_gpio_platform_data *pdata)
+/* Get i2c-gpio properties from DT or ACPI table */
+static void i2c_gpio_get_properties(struct device *dev,
+ struct i2c_gpio_platform_data *pdata)
{
u32 reg;
- of_property_read_u32(np, "i2c-gpio,delay-us", &pdata->udelay);
+ device_property_read_u32(dev, "i2c-gpio,delay-us", &pdata->udelay);
- if (!of_property_read_u32(np, "i2c-gpio,timeout-ms", ®))
+ if (!device_property_read_u32(dev, "i2c-gpio,timeout-ms", ®))
pdata->timeout = msecs_to_jiffies(reg);
pdata->sda_is_open_drain =
- of_property_read_bool(np, "i2c-gpio,sda-open-drain");
+ device_property_read_bool(dev, "i2c-gpio,sda-open-drain");
pdata->scl_is_open_drain =
- of_property_read_bool(np, "i2c-gpio,scl-open-drain");
+ device_property_read_bool(dev, "i2c-gpio,scl-open-drain");
pdata->scl_is_output_only =
- of_property_read_bool(np, "i2c-gpio,scl-output-only");
+ device_property_read_bool(dev, "i2c-gpio,scl-output-only");
}
static struct gpio_desc *i2c_gpio_get_desc(struct device *dev,
@@ -373,8 +374,8 @@ static int i2c_gpio_probe(struct platform_device *pdev)
bit_data = &priv->bit_data;
pdata = &priv->pdata;
- if (np) {
- of_i2c_gpio_get_props(np, pdata);
+ if (dev_fwnode(dev)) {
+ i2c_gpio_get_properties(dev, pdata);
} else {
/*
* If all platform data settings are zero it is OK
@@ -489,10 +490,17 @@ static const struct of_device_id i2c_gpio_dt_ids[] = {
MODULE_DEVICE_TABLE(of, i2c_gpio_dt_ids);
+static const struct acpi_device_id i2c_gpio_acpi_match[] = {
+ { "LOON0005" }, /* LoongArch */
+ { }
+};
+MODULE_DEVICE_TABLE(acpi, i2c_gpio_acpi_match);
+
static struct platform_driver i2c_gpio_driver = {
.driver = {
.name = "i2c-gpio",
.of_match_table = i2c_gpio_dt_ids,
+ .acpi_match_table = i2c_gpio_acpi_match,
},
.probe = i2c_gpio_probe,
.remove = i2c_gpio_remove,
--
2.31.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH V7 2/4] dt-bindings: i2c: add Loongson LS2X I2C controller
2022-12-19 12:28 [PATCH V7 0/4] i2c: ls2x: Add support for the Loongson-2K/LS7A I2C controller Binbin Zhou
2022-12-19 12:28 ` [PATCH V7 1/4] i2c: gpio: Add support on ACPI-based system Binbin Zhou
@ 2022-12-19 12:28 ` Binbin Zhou
2022-12-19 12:28 ` [PATCH V7 3/4] i2c: ls2x: Add driver for Loongson-2K/LS7A " Binbin Zhou
2022-12-19 12:29 ` [PATCH V7 4/4] LoongArch: Enable LS2X I2C in loongson3_defconfig Binbin Zhou
3 siblings, 0 replies; 8+ messages in thread
From: Binbin Zhou @ 2022-12-19 12:28 UTC (permalink / raw)
To: Wolfram Sang, Wolfram Sang, Andy Shevchenko, Mika Westerberg,
linux-i2c
Cc: loongarch, devicetree, Huacai Chen, WANG Xuerui, Andy Shevchenko,
Arnd Bergmann, Rob Herring, Krzysztof Kozlowski, Jianmin Lv,
Binbin Zhou, Krzysztof Kozlowski
Add Loongson LS2X I2C controller binding with DT schema format using
json-schema.
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
.../bindings/i2c/loongson,ls2x-i2c.yaml | 51 +++++++++++++++++++
1 file changed, 51 insertions(+)
create mode 100644 Documentation/devicetree/bindings/i2c/loongson,ls2x-i2c.yaml
diff --git a/Documentation/devicetree/bindings/i2c/loongson,ls2x-i2c.yaml b/Documentation/devicetree/bindings/i2c/loongson,ls2x-i2c.yaml
new file mode 100644
index 000000000000..67882ec6e06a
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/loongson,ls2x-i2c.yaml
@@ -0,0 +1,51 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/i2c/loongson,ls2x-i2c.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Loongson LS2X I2C Controller
+
+maintainers:
+ - Binbin Zhou <zhoubinbin@loongson.cn>
+
+allOf:
+ - $ref: /schemas/i2c/i2c-controller.yaml#
+
+properties:
+ compatible:
+ enum:
+ - loongson,ls2k-i2c
+ - loongson,ls7a-i2c
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - interrupts
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ i2c0: i2c@1fe21000 {
+ compatible = "loongson,ls2k-i2c";
+ reg = <0x1fe21000 0x8>;
+ interrupt-parent = <&extioiic>;
+ interrupts = <22 IRQ_TYPE_LEVEL_LOW>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ eeprom@57 {
+ compatible = "atmel,24c16";
+ reg = <0x57>;
+ pagesize = <16>;
+ };
+ };
--
2.31.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH V7 3/4] i2c: ls2x: Add driver for Loongson-2K/LS7A I2C controller
2022-12-19 12:28 [PATCH V7 0/4] i2c: ls2x: Add support for the Loongson-2K/LS7A I2C controller Binbin Zhou
2022-12-19 12:28 ` [PATCH V7 1/4] i2c: gpio: Add support on ACPI-based system Binbin Zhou
2022-12-19 12:28 ` [PATCH V7 2/4] dt-bindings: i2c: add Loongson LS2X I2C controller Binbin Zhou
@ 2022-12-19 12:28 ` Binbin Zhou
2022-12-19 13:04 ` Andy Shevchenko
2022-12-19 12:29 ` [PATCH V7 4/4] LoongArch: Enable LS2X I2C in loongson3_defconfig Binbin Zhou
3 siblings, 1 reply; 8+ messages in thread
From: Binbin Zhou @ 2022-12-19 12:28 UTC (permalink / raw)
To: Wolfram Sang, Wolfram Sang, Andy Shevchenko, Mika Westerberg,
linux-i2c
Cc: loongarch, devicetree, Huacai Chen, WANG Xuerui, Andy Shevchenko,
Arnd Bergmann, Rob Herring, Krzysztof Kozlowski, Jianmin Lv,
Binbin Zhou
This I2C module is integrated into the Loongson-2K SoCs and Loongson
LS7A bridge chip.
Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
drivers/i2c/busses/Kconfig | 11 +
drivers/i2c/busses/Makefile | 1 +
drivers/i2c/busses/i2c-ls2x.c | 369 ++++++++++++++++++++++++++++++++++
3 files changed, 381 insertions(+)
create mode 100644 drivers/i2c/busses/i2c-ls2x.c
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index e50f9603d189..908096a762ca 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -888,6 +888,17 @@ config I2C_OWL
Say Y here if you want to use the I2C bus controller on
the Actions Semiconductor Owl SoC's.
+config I2C_LS2X
+ tristate "Loongson LS2X I2C adapter"
+ depends on MACH_LOONGSON64 || COMPILE_TEST
+ help
+ If you say yes to this option, support will be included for the
+ I2C interface on the Loongson-2K SoCs and Loongson LS7A bridge
+ chip.
+
+ This driver can also be built as a module. If so, the module
+ will be called i2c-ls2x.
+
config I2C_PASEMI
tristate "PA Semi SMBus interface"
depends on PPC_PASEMI && PCI
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index e73cdb1d2b5a..df332ec3c489 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -86,6 +86,7 @@ obj-$(CONFIG_I2C_MV64XXX) += i2c-mv64xxx.o
obj-$(CONFIG_I2C_MXS) += i2c-mxs.o
obj-$(CONFIG_I2C_NOMADIK) += i2c-nomadik.o
obj-$(CONFIG_I2C_NPCM) += i2c-npcm7xx.o
+obj-$(CONFIG_I2C_LS2X) += i2c-ls2x.o
obj-$(CONFIG_I2C_OCORES) += i2c-ocores.o
obj-$(CONFIG_I2C_OMAP) += i2c-omap.o
obj-$(CONFIG_I2C_OWL) += i2c-owl.o
diff --git a/drivers/i2c/busses/i2c-ls2x.c b/drivers/i2c/busses/i2c-ls2x.c
new file mode 100644
index 000000000000..dd0ea95d144d
--- /dev/null
+++ b/drivers/i2c/busses/i2c-ls2x.c
@@ -0,0 +1,369 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Loongson-2K/Loongson LS7A I2C master mode driver
+ *
+ * Copyright (C) 2013 Loongson Technology Corporation Limited.
+ * Copyright (C) 2014-2017 Lemote, Inc.
+ * Copyright (C) 2018-2022 Loongson Technology Corporation Limited.
+ *
+ * Originally written by liushaozong
+ * Rewritten for mainline by Binbin Zhou <zhoubinbin@loongson.cn>
+ */
+
+#include <linux/bits.h>
+#include <linux/completion.h>
+#include <linux/device.h>
+#include <linux/i2c.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/pm_runtime.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/units.h>
+
+/* I2C Registers */
+#define I2C_LS2X_PRER 0x0 /* Freq Division Register(16 bits) */
+#define I2C_LS2X_CTR 0x2 /* Control Register */
+#define I2C_LS2X_TXR 0x3 /* Transport Data Register */
+#define I2C_LS2X_RXR 0x3 /* Receive Data Register */
+#define I2C_LS2X_CR 0x4 /* Command Control Register */
+#define I2C_LS2X_SR 0x4 /* State Register */
+
+/* Command Control Register Bit */
+#define LS2X_CR_START BIT(7) /* Start signal */
+#define LS2X_CR_STOP BIT(6) /* Stop signal */
+#define LS2X_CR_READ BIT(5) /* Read signal */
+#define LS2X_CR_WRITE BIT(4) /* Write signal */
+#define LS2X_CR_ACK BIT(3) /* Response signal */
+#define LS2X_CR_IACK BIT(0) /* Interrupt response signal */
+
+/* State Register Bit */
+#define LS2X_SR_NOACK BIT(7) /* Receive NACK */
+#define LS2X_SR_BUSY BIT(6) /* Bus busy state */
+#define LS2X_SR_AL BIT(5) /* Arbitration lost */
+#define LS2X_SR_TIP BIT(1) /* Transmission state */
+#define LS2X_SR_IF BIT(0) /* Interrupt flag */
+
+/* Control Register Bit */
+#define LS2X_CTR_EN BIT(7) /* 0: I2c frequency setting 1: Normal */
+#define LS2X_CTR_IEN BIT(6) /* Enable i2c interrupt */
+#define LS2X_CTR_MST BIT(5) /* 0: Slave mode 1: Master mode */
+#define CTR_READY_MASK GENMASK(7, 5)
+
+/* The PCLK frequency from LPB */
+#define LS2X_I2C_PCLK_FREQ (50 * HZ_PER_MHZ)
+
+/* The default bus frequency, which is an empirical value */
+#define LS2X_I2C_FREQ_STD (33 * HZ_PER_KHZ)
+
+struct ls2x_i2c_priv {
+ struct i2c_adapter adapter;
+ void __iomem *base;
+ struct i2c_timings i2c_t;
+ struct completion cmd_complete;
+};
+
+/*
+ * Interrupt service routine.
+ * This gets called whenever an I2C interrupt occurs.
+ */
+static irqreturn_t ls2x_i2c_isr(int this_irq, void *dev_id)
+{
+ struct ls2x_i2c_priv *priv = dev_id;
+
+ if (!(readb(priv->base + I2C_LS2X_SR) & LS2X_SR_IF))
+ return IRQ_NONE;
+
+ writeb(LS2X_CR_IACK, priv->base + I2C_LS2X_CR);
+ complete(&priv->cmd_complete);
+ return IRQ_HANDLED;
+}
+
+/*
+ * The LS2X I2C controller supports standard mode and fast mode,
+ * so the maximum bus frequency is '400kHz'.
+ * The bus frequency is set to the empirical value of '33KHz' by default,
+ * but it can also be taken from ACPI or FDT for compatibility with more
+ * devices.
+ */
+static void ls2x_i2c_adjust_bus_speed(struct ls2x_i2c_priv *priv)
+{
+ struct i2c_timings *t = &priv->i2c_t;
+ struct device *dev = priv->adapter.dev.parent;
+ u32 acpi_speed = i2c_acpi_find_bus_speed(dev);
+
+ i2c_parse_fw_timings(dev, t, false);
+
+ if (acpi_speed || t->bus_freq_hz)
+ t->bus_freq_hz = max(t->bus_freq_hz, acpi_speed);
+ else
+ t->bus_freq_hz = LS2X_I2C_FREQ_STD;
+
+ /* Calculate and set LS2X I2C frequency */
+ writew(LS2X_I2C_PCLK_FREQ / (5 * t->bus_freq_hz) - 1,
+ priv->base + I2C_LS2X_PRER);
+}
+
+static void ls2x_i2c_init(struct ls2x_i2c_priv *priv)
+{
+ /* Enable operations about frequency divider register */
+ writeb(readb(priv->base + I2C_LS2X_CR) | LS2X_CR_IACK,
+ priv->base + I2C_LS2X_CR);
+ writeb(readb(priv->base + I2C_LS2X_CTR) & ~LS2X_CTR_EN,
+ priv->base + I2C_LS2X_CTR);
+
+ ls2x_i2c_adjust_bus_speed(priv);
+
+ /* Set the normal I2C operating mode and enable interrupts */
+ writeb(readb(priv->base + I2C_LS2X_CTR) | CTR_READY_MASK,
+ priv->base + I2C_LS2X_CTR);
+}
+
+static int ls2x_i2c_xfer_byte(struct i2c_adapter *adap, u8 txdata, u8 *rxdatap)
+{
+ u8 rxdata;
+ unsigned long time_left;
+ struct ls2x_i2c_priv *priv = i2c_get_adapdata(adap);
+
+ writeb(txdata, priv->base + I2C_LS2X_CR);
+
+ time_left = wait_for_completion_timeout(&priv->cmd_complete,
+ adap->timeout);
+ if (!time_left)
+ return -ETIMEDOUT;
+
+ rxdata = readb(priv->base + I2C_LS2X_SR);
+ if (rxdatap)
+ *rxdatap = rxdata;
+
+ return 0;
+}
+
+static int ls2x_i2c_send_byte(struct i2c_adapter *adap, u8 txdata)
+{
+ int ret;
+ u8 rxdata;
+
+ ret = ls2x_i2c_xfer_byte(adap, txdata, &rxdata);
+ if (ret)
+ return ret;
+
+ if (rxdata & LS2X_SR_AL)
+ return -EAGAIN;
+
+ if (rxdata & LS2X_SR_NOACK)
+ return -ENXIO;
+
+ return 0;
+}
+
+static int ls2x_i2c_stop(struct i2c_adapter *adap)
+{
+ int ret = ls2x_i2c_send_byte(adap, LS2X_CR_STOP);
+
+ return (ret == -ENXIO) ? 0 : ret;
+}
+
+static int ls2x_i2c_start(struct i2c_adapter *adap, struct i2c_msg *msgs)
+{
+ struct ls2x_i2c_priv *priv = i2c_get_adapdata(adap);
+
+ reinit_completion(&priv->cmd_complete);
+
+ writeb(i2c_8bit_addr_from_msg(msgs), priv->base + I2C_LS2X_TXR);
+ return ls2x_i2c_send_byte(adap, LS2X_CR_START | LS2X_CR_WRITE);
+}
+
+static int ls2x_i2c_rx(struct i2c_adapter *adap, u8 *buf, u16 len)
+{
+ unsigned long time_left;
+ struct ls2x_i2c_priv *priv = i2c_get_adapdata(adap);
+
+ while (len--) {
+ writeb(LS2X_CR_READ | (len ? 0 : LS2X_CR_ACK),
+ priv->base + I2C_LS2X_CR);
+
+ time_left = wait_for_completion_timeout(&priv->cmd_complete,
+ adap->timeout);
+ if (!time_left)
+ return -ETIMEDOUT;
+
+ *buf++ = readb(priv->base + I2C_LS2X_RXR);
+ }
+
+ return 0;
+}
+
+static int ls2x_i2c_tx(struct i2c_adapter *adap, u8 *buf, u16 len)
+{
+ int ret;
+ struct ls2x_i2c_priv *priv = i2c_get_adapdata(adap);
+
+ while (len--) {
+ writeb(*buf++, priv->base + I2C_LS2X_TXR);
+
+ ret = ls2x_i2c_send_byte(adap, LS2X_CR_WRITE);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+static int ls2x_i2c_xfer_one(struct i2c_adapter *adap,
+ struct i2c_msg *msg, bool stop)
+{
+ int ret;
+ bool is_read = msg->flags & I2C_M_RD;
+ struct ls2x_i2c_priv *priv = i2c_get_adapdata(adap);
+
+ /* Contains steps to send start condition and address */
+ ret = ls2x_i2c_start(adap, msg);
+ if (!ret) {
+ if (is_read)
+ ret = ls2x_i2c_rx(adap, msg->buf, msg->len);
+ else
+ ret = ls2x_i2c_tx(adap, msg->buf, msg->len);
+
+ if (!ret && stop)
+ ret = ls2x_i2c_stop(adap);
+ }
+
+ if (ret == -ENXIO)
+ ls2x_i2c_stop(adap);
+ else if (ret < 0)
+ ls2x_i2c_init(priv);
+
+ return ret;
+}
+
+static int ls2x_i2c_master_xfer(struct i2c_adapter *adap,
+ struct i2c_msg *msgs, int num)
+{
+ int ret;
+ struct i2c_msg *msg, *emsg = msgs + num;
+
+ for (msg = msgs; msg < emsg; msg++) {
+ ret = ls2x_i2c_xfer_one(adap, msg, msg == (emsg - 1));
+ if (ret)
+ return ret;
+ }
+
+ return num;
+}
+
+static unsigned int ls2x_i2c_func(struct i2c_adapter *adap)
+{
+ return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
+}
+
+static const struct i2c_algorithm ls2x_i2c_algo = {
+ .master_xfer = ls2x_i2c_master_xfer,
+ .functionality = ls2x_i2c_func,
+};
+
+static int ls2x_i2c_probe(struct platform_device *pdev)
+{
+ int r, irq;
+ struct i2c_adapter *adap;
+ struct ls2x_i2c_priv *priv;
+ struct device *dev = &pdev->dev;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ /* Map hardware registers */
+ priv->base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(priv->base))
+ return PTR_ERR(priv->base);
+
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0)
+ return irq;
+
+ /* Add the i2c adapter */
+ adap = &priv->adapter;
+ adap->nr = pdev->id;
+ adap->owner = THIS_MODULE;
+ adap->retries = 5;
+ adap->algo = &ls2x_i2c_algo;
+ adap->dev.parent = dev;
+ device_set_node(&adap->dev, dev_fwnode(dev));
+ i2c_set_adapdata(adap, priv);
+ strscpy(adap->name, pdev->name, sizeof(adap->name));
+ init_completion(&priv->cmd_complete);
+ platform_set_drvdata(pdev, priv);
+
+ ls2x_i2c_init(priv);
+
+ r = devm_request_irq(dev, irq, ls2x_i2c_isr, IRQF_SHARED, "ls2x-i2c",
+ priv);
+ if (r < 0)
+ return dev_err_probe(dev, r, "Unable to request irq %d\n", irq);
+
+ return devm_i2c_add_adapter(dev, adap);
+}
+
+static int ls2x_i2c_suspend(struct device *dev)
+{
+ struct ls2x_i2c_priv *priv = dev_get_drvdata(dev);
+
+ /* Disable I2C interrupt */
+ writeb(readb(priv->base + I2C_LS2X_CTR) & ~LS2X_CTR_IEN,
+ priv->base + I2C_LS2X_CTR);
+
+ return 0;
+}
+
+static int ls2x_i2c_resume(struct device *dev)
+{
+ ls2x_i2c_init(dev_get_drvdata(dev));
+ return 0;
+}
+
+static DEFINE_RUNTIME_DEV_PM_OPS(ls2x_i2c_pm_ops,
+ ls2x_i2c_suspend, ls2x_i2c_resume, NULL);
+
+static const struct of_device_id ls2x_i2c_id_table[] = {
+ { .compatible = "loongson,ls2k-i2c" },
+ { .compatible = "loongson,ls7a-i2c" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, ls2x_i2c_id_table);
+
+static const struct acpi_device_id ls2x_i2c_acpi_match[] = {
+ { "LOON0004" }, /* Loongson LS7A */
+ { }
+};
+MODULE_DEVICE_TABLE(acpi, ls2x_i2c_acpi_match);
+
+static struct platform_driver ls2x_i2c_driver = {
+ .probe = ls2x_i2c_probe,
+ .driver = {
+ .name = "ls2x-i2c",
+ .pm = pm_sleep_ptr(&ls2x_i2c_pm_ops),
+ .of_match_table = ls2x_i2c_id_table,
+ .acpi_match_table = ls2x_i2c_acpi_match,
+ },
+};
+
+/* The DC subsystem depends on it, we should initialize it earlier */
+static int __init ls2x_i2c_init_driver(void)
+{
+ return platform_driver_register(&ls2x_i2c_driver);
+}
+subsys_initcall(ls2x_i2c_init_driver);
+
+static void __exit ls2x_i2c_exit_driver(void)
+{
+ platform_driver_unregister(&ls2x_i2c_driver);
+}
+module_exit(ls2x_i2c_exit_driver);
+
+MODULE_DESCRIPTION("Loongson LS2X I2C Bus driver");
+MODULE_AUTHOR("Loongson Technology Corporation Limited");
+MODULE_LICENSE("GPL");
--
2.31.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH V7 3/4] i2c: ls2x: Add driver for Loongson-2K/LS7A I2C controller
2022-12-19 12:28 ` [PATCH V7 3/4] i2c: ls2x: Add driver for Loongson-2K/LS7A " Binbin Zhou
@ 2022-12-19 13:04 ` Andy Shevchenko
2022-12-20 11:47 ` Binbin Zhou
0 siblings, 1 reply; 8+ messages in thread
From: Andy Shevchenko @ 2022-12-19 13:04 UTC (permalink / raw)
To: Binbin Zhou
Cc: Wolfram Sang, Wolfram Sang, Mika Westerberg, linux-i2c, loongarch,
devicetree, Huacai Chen, WANG Xuerui, Arnd Bergmann, Rob Herring,
Krzysztof Kozlowski, Jianmin Lv
On Mon, Dec 19, 2022 at 08:28:33PM +0800, Binbin Zhou wrote:
> This I2C module is integrated into the Loongson-2K SoCs and Loongson
> LS7A bridge chip.
Almost there, see my comments below (note, you have ~1w of time before this
can be applied anyway, so take you time for carefully addressing comments,
(re-)testing, etc.).
...
> @@ -888,6 +888,17 @@ config I2C_OWL
> Say Y here if you want to use the I2C bus controller on
> the Actions Semiconductor Owl SoC's.
>
> +config I2C_LS2X
I believe in Latin alphabet L goes before O...
> + tristate "Loongson LS2X I2C adapter"
> + depends on MACH_LOONGSON64 || COMPILE_TEST
> + help
> + If you say yes to this option, support will be included for the
> + I2C interface on the Loongson-2K SoCs and Loongson LS7A bridge
> + chip.
> +
> + This driver can also be built as a module. If so, the module
> + will be called i2c-ls2x.
> +
> config I2C_PASEMI
> tristate "PA Semi SMBus interface"
> depends on PPC_PASEMI && PCI
...
> obj-$(CONFIG_I2C_MXS) += i2c-mxs.o
> obj-$(CONFIG_I2C_NOMADIK) += i2c-nomadik.o
> obj-$(CONFIG_I2C_NPCM) += i2c-npcm7xx.o
> +obj-$(CONFIG_I2C_LS2X) += i2c-ls2x.o
...and even before n and m.
> obj-$(CONFIG_I2C_OCORES) += i2c-ocores.o
> obj-$(CONFIG_I2C_OMAP) += i2c-omap.o
> obj-$(CONFIG_I2C_OWL) += i2c-owl.o
...
> +static int ls2x_i2c_xfer_one(struct i2c_adapter *adap,
> + struct i2c_msg *msg, bool stop)
> +{
> + int ret;
> + bool is_read = msg->flags & I2C_M_RD;
> + struct ls2x_i2c_priv *priv = i2c_get_adapdata(adap);
> +
> + /* Contains steps to send start condition and address */
> + ret = ls2x_i2c_start(adap, msg);
> + if (!ret) {
> + if (is_read)
> + ret = ls2x_i2c_rx(adap, msg->buf, msg->len);
> + else
> + ret = ls2x_i2c_tx(adap, msg->buf, msg->len);
> + if (!ret && stop)
> + ret = ls2x_i2c_stop(adap);
So, we will send stop here...
> + }
> + if (ret == -ENXIO)
> + ls2x_i2c_stop(adap);
...and if it fails, we send it again here. Is it okay?
> + else if (ret < 0)
> + ls2x_i2c_init(priv);
> +
> + return ret;
> +}
...
> + ret = ls2x_i2c_xfer_one(adap, msg, msg == (emsg - 1));
Too many parentheses, isn't it?
> + if (ret)
> + return ret;
...
> + r = devm_request_irq(dev, irq, ls2x_i2c_isr, IRQF_SHARED, "ls2x-i2c",
> + priv);
Everywhere else you use 'ret', why is 'r' here?
> + if (r < 0)
> + return dev_err_probe(dev, r, "Unable to request irq %d\n", irq);
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH V7 3/4] i2c: ls2x: Add driver for Loongson-2K/LS7A I2C controller
2022-12-19 13:04 ` Andy Shevchenko
@ 2022-12-20 11:47 ` Binbin Zhou
2022-12-20 13:48 ` Andy Shevchenko
0 siblings, 1 reply; 8+ messages in thread
From: Binbin Zhou @ 2022-12-20 11:47 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Binbin Zhou, Wolfram Sang, Wolfram Sang, Mika Westerberg,
linux-i2c, loongarch, devicetree, Huacai Chen, WANG Xuerui,
Arnd Bergmann, Rob Herring, Krzysztof Kozlowski, Jianmin Lv
Hi Andy:
On Mon, Dec 19, 2022 at 9:04 PM Andy Shevchenko <andy@kernel.org> wrote:
>
> On Mon, Dec 19, 2022 at 08:28:33PM +0800, Binbin Zhou wrote:
> > This I2C module is integrated into the Loongson-2K SoCs and Loongson
> > LS7A bridge chip.
>
> Almost there, see my comments below (note, you have ~1w of time before this
> can be applied anyway, so take you time for carefully addressing comments,
> (re-)testing, etc.).
>
> ...
>
> > @@ -888,6 +888,17 @@ config I2C_OWL
> > Say Y here if you want to use the I2C bus controller on
> > the Actions Semiconductor Owl SoC's.
> >
> > +config I2C_LS2X
>
> I believe in Latin alphabet L goes before O...
Sorry, I will fix it.
>
> > + tristate "Loongson LS2X I2C adapter"
> > + depends on MACH_LOONGSON64 || COMPILE_TEST
> > + help
> > + If you say yes to this option, support will be included for the
> > + I2C interface on the Loongson-2K SoCs and Loongson LS7A bridge
> > + chip.
> > +
> > + This driver can also be built as a module. If so, the module
> > + will be called i2c-ls2x.
> > +
> > config I2C_PASEMI
> > tristate "PA Semi SMBus interface"
> > depends on PPC_PASEMI && PCI
>
> ...
>
> > obj-$(CONFIG_I2C_MXS) += i2c-mxs.o
> > obj-$(CONFIG_I2C_NOMADIK) += i2c-nomadik.o
> > obj-$(CONFIG_I2C_NPCM) += i2c-npcm7xx.o
> > +obj-$(CONFIG_I2C_LS2X) += i2c-ls2x.o
>
> ...and even before n and m.
>
> > obj-$(CONFIG_I2C_OCORES) += i2c-ocores.o
> > obj-$(CONFIG_I2C_OMAP) += i2c-omap.o
> > obj-$(CONFIG_I2C_OWL) += i2c-owl.o
>
> ...
>
> > +static int ls2x_i2c_xfer_one(struct i2c_adapter *adap,
> > + struct i2c_msg *msg, bool stop)
> > +{
> > + int ret;
> > + bool is_read = msg->flags & I2C_M_RD;
> > + struct ls2x_i2c_priv *priv = i2c_get_adapdata(adap);
> > +
> > + /* Contains steps to send start condition and address */
> > + ret = ls2x_i2c_start(adap, msg);
> > + if (!ret) {
> > + if (is_read)
> > + ret = ls2x_i2c_rx(adap, msg->buf, msg->len);
> > + else
> > + ret = ls2x_i2c_tx(adap, msg->buf, msg->len);
>
> > + if (!ret && stop)
> > + ret = ls2x_i2c_stop(adap);
>
> So, we will send stop here...
>
> > + }
>
> > + if (ret == -ENXIO)
> > + ls2x_i2c_stop(adap);
>
> ...and if it fails, we send it again here. Is it okay?
First of all, I think the logic here is fine:
In the first case, stop is called when the last msg is transmitted successfully;
In the second case, stop is called when there is a NOACK during the
transmission;
In the third case, init is called when other errors occur during the
transmission, such as TIMEOUT.
The key point is that the current implementation of the stop function
is probably imperfect, it directly ignores the NOACK. The more
reasonable method would be to judge by LS2X_SR_BUSY(bus busy state).
The basic idea: if the i2c bus is detected as idle within the timeout
range of the stop command issued, the stop command is considered
successful, otherwise -ETIMEDOUT is returned.
As follows:
static int ls2x_i2c_stop(struct i2c_adapter *adap)
{
- int ret = ls2x_i2c_send_byte(adap, LS2X_CR_STOP);
+ u8 value;
+ struct ls2x_i2c_priv *priv = i2c_get_adapdata(adap);
- return (ret == -ENXIO) ? 0 : ret;
+ writeb(LS2X_CR_STOP, priv->base + I2C_LS2X_CR);
+ return readl_poll_timeout(priv->base + I2C_LS2X_SR,
+ value, !(value & LS2X_SR_BUSY),
+ 100, jiffies_to_usecs(adap->timeout));
}
Now, in the case you mentioned above, the last msg transmission is
completed and the stop command is sent. If the stop command fails, it
will only return -ETIMEDOUT and call ls2x_i2c_init().
Of course, I will use tools like i2c-tools to do more tests.
Thanks for the detailed review.
Binbin
>
> > + else if (ret < 0)
> > + ls2x_i2c_init(priv);
> > +
> > + return ret;
> > +}
>
> ...
>
> > + ret = ls2x_i2c_xfer_one(adap, msg, msg == (emsg - 1));
>
> Too many parentheses, isn't it?
>
> > + if (ret)
> > + return ret;
>
> ...
>
> > + r = devm_request_irq(dev, irq, ls2x_i2c_isr, IRQF_SHARED, "ls2x-i2c",
> > + priv);
>
> Everywhere else you use 'ret', why is 'r' here?
>
> > + if (r < 0)
> > + return dev_err_probe(dev, r, "Unable to request irq %d\n", irq);
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH V7 3/4] i2c: ls2x: Add driver for Loongson-2K/LS7A I2C controller
2022-12-20 11:47 ` Binbin Zhou
@ 2022-12-20 13:48 ` Andy Shevchenko
0 siblings, 0 replies; 8+ messages in thread
From: Andy Shevchenko @ 2022-12-20 13:48 UTC (permalink / raw)
To: Binbin Zhou
Cc: Binbin Zhou, Wolfram Sang, Wolfram Sang, Mika Westerberg,
linux-i2c, loongarch, devicetree, Huacai Chen, WANG Xuerui,
Arnd Bergmann, Rob Herring, Krzysztof Kozlowski, Jianmin Lv
On Tue, Dec 20, 2022 at 07:47:25PM +0800, Binbin Zhou wrote:
> On Mon, Dec 19, 2022 at 9:04 PM Andy Shevchenko <andy@kernel.org> wrote:
> > On Mon, Dec 19, 2022 at 08:28:33PM +0800, Binbin Zhou wrote:
...
> > > +static int ls2x_i2c_xfer_one(struct i2c_adapter *adap,
> > > + struct i2c_msg *msg, bool stop)
> > > +{
> > > + int ret;
> > > + bool is_read = msg->flags & I2C_M_RD;
> > > + struct ls2x_i2c_priv *priv = i2c_get_adapdata(adap);
> > > +
> > > + /* Contains steps to send start condition and address */
> > > + ret = ls2x_i2c_start(adap, msg);
> > > + if (!ret) {
> > > + if (is_read)
> > > + ret = ls2x_i2c_rx(adap, msg->buf, msg->len);
> > > + else
> > > + ret = ls2x_i2c_tx(adap, msg->buf, msg->len);
> >
> > > + if (!ret && stop)
> > > + ret = ls2x_i2c_stop(adap);
> >
> > So, we will send stop here...
> >
> > > + }
> >
> > > + if (ret == -ENXIO)
> > > + ls2x_i2c_stop(adap);
> >
> > ...and if it fails, we send it again here. Is it okay?
>
> First of all, I think the logic here is fine:
> In the first case, stop is called when the last msg is transmitted successfully;
> In the second case, stop is called when there is a NOACK during the
> transmission;
> In the third case, init is called when other errors occur during the
> transmission, such as TIMEOUT.
This is not obvious from the code above. Maybe the comment is missing, maybe as
you said the function should be amended and the code is refactored accordingly.
> The key point is that the current implementation of the stop function
> is probably imperfect, it directly ignores the NOACK. The more
> reasonable method would be to judge by LS2X_SR_BUSY(bus busy state).
> The basic idea: if the i2c bus is detected as idle within the timeout
> range of the stop command issued, the stop command is considered
> successful, otherwise -ETIMEDOUT is returned.
>
> As follows:
> static int ls2x_i2c_stop(struct i2c_adapter *adap)
> {
> - int ret = ls2x_i2c_send_byte(adap, LS2X_CR_STOP);
> + u8 value;
> + struct ls2x_i2c_priv *priv = i2c_get_adapdata(adap);
>
> - return (ret == -ENXIO) ? 0 : ret;
> + writeb(LS2X_CR_STOP, priv->base + I2C_LS2X_CR);
> + return readl_poll_timeout(priv->base + I2C_LS2X_SR,
> + value, !(value & LS2X_SR_BUSY),
> + 100, jiffies_to_usecs(adap->timeout));
> }
>
> Now, in the case you mentioned above, the last msg transmission is
> completed and the stop command is sent. If the stop command fails, it
> will only return -ETIMEDOUT and call ls2x_i2c_init().
>
> Of course, I will use tools like i2c-tools to do more tests.
>
> Thanks for the detailed review.
You're welcome!
> > > + else if (ret < 0)
> > > + ls2x_i2c_init(priv);
> > > +
> > > + return ret;
> > > +}
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH V7 4/4] LoongArch: Enable LS2X I2C in loongson3_defconfig
2022-12-19 12:28 [PATCH V7 0/4] i2c: ls2x: Add support for the Loongson-2K/LS7A I2C controller Binbin Zhou
` (2 preceding siblings ...)
2022-12-19 12:28 ` [PATCH V7 3/4] i2c: ls2x: Add driver for Loongson-2K/LS7A " Binbin Zhou
@ 2022-12-19 12:29 ` Binbin Zhou
3 siblings, 0 replies; 8+ messages in thread
From: Binbin Zhou @ 2022-12-19 12:29 UTC (permalink / raw)
To: Wolfram Sang, Wolfram Sang, Andy Shevchenko, Mika Westerberg,
linux-i2c
Cc: loongarch, devicetree, Huacai Chen, WANG Xuerui, Andy Shevchenko,
Arnd Bergmann, Rob Herring, Krzysztof Kozlowski, Jianmin Lv,
Binbin Zhou
This is now supported, enable for Loongson-3 systems.
Other systems are unaffected.
Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
arch/loongarch/configs/loongson3_defconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/loongarch/configs/loongson3_defconfig b/arch/loongarch/configs/loongson3_defconfig
index cb52774c80e8..5677c4f8576e 100644
--- a/arch/loongarch/configs/loongson3_defconfig
+++ b/arch/loongarch/configs/loongson3_defconfig
@@ -602,6 +602,7 @@ CONFIG_HW_RANDOM_VIRTIO=m
CONFIG_I2C_CHARDEV=y
CONFIG_I2C_PIIX4=y
CONFIG_I2C_GPIO=y
+CONFIG_I2C_LS2X=y
CONFIG_SPI=y
CONFIG_GPIO_SYSFS=y
CONFIG_GPIO_LOONGSON=y
--
2.31.1
^ permalink raw reply related [flat|nested] 8+ messages in thread