2019-12-09 18:44:13

by Daniel Mack

[permalink] [raw]
Subject: [PATCH 00/10] mfd: Add support for Analog Devices A2B transceiver

This patch series adds support for Analog Device's AD242x A2B
transceivers.

https://www.analog.com/media/en/technical-documentation/user-guides/AD242x_TRM_Rev1.1.pdf

These transceivers are used to form an audio network by connecting the
parts in a daisy-chain. On top of audio, the devices expose some other
functions such as GPIO, programmable clock outputs and remote-side I2C
bus master. The first node in the chain is called the master node, and
all other devices are called slave nodes. Up to 15 such devices can be
connected this way.

The master device responds on two addresses on the I2C bus. The primary
one is used to access all registers in the master node itself, the
secondary is for accessing remote nodes after prior setup through the
master node. In the driver stack, these details are hidden behind
specific regmap configs.

The driver stack is implemented as MFD core and companion drivers that
can be registered as sub-devices in DT. Drivers for these sub-devices
can be used for both master and slave nodes, as they just interface
with the node's regmap.

The master node is responsible for discovering all the slave nodes at
probe time, and it needs to take the used audio and routing modes in
each of the slave devices into account in order to pre-calculate the
bus-timings correctly. Hence, this bus is not hot-pluggable.

Transceivers can both receive and provide audio, and streams can be
routed from one node to any other, including many others. The tricky
bit is how to expose the audio routing in DT in a sane way.
The way it is implemented here, the slave nodes specify the number of
slots they each consume and generate, and which thereof they forward
from one side to the other. This mimics the internal register
structure and should allow for even exotic setups.

Please let me know what you think and what could be improved.


Thanks,
Daniel


Daniel Mack (10):
dt-bindings: mfd: Add documentation for ad242x
dt-bindings: i2c: Add documentation for ad242x i2c controllers
dt-bindings: gpio: Add documentation for AD242x GPIO controllers
dt-bindings: clock: Add documentation for AD242x clock providers
dt-bindings: sound: Add documentation for AD242x codecs
mfd: Add core driver for AD242x A2B transceivers
i2c: Add driver for AD242x bus controller
gpio: Add driver for AD242x GPIO controllers
clk: Add support for AD242x clock output providers
ASoC: Add codec component for AD242x nodes

.../bindings/clock/adi,ad242x-clk.yaml | 32 +
.../bindings/gpio/adi,ad242x-gpio.yaml | 65 ++
.../bindings/i2c/adi,ad242x-i2c.yaml | 31 +
.../bindings/mfd/adi,ad242x-bus.yaml | 29 +
.../bindings/mfd/adi,ad242x-master.yaml | 235 +++++++
.../bindings/mfd/adi,ad242x-slave.yaml | 108 ++++
.../bindings/sound/adi,ad242x-codec.yaml | 31 +
drivers/clk/Kconfig | 6 +
drivers/clk/Makefile | 1 +
drivers/clk/clk-ad242x.c | 231 +++++++
drivers/gpio/Kconfig | 6 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-ad242x.c | 229 +++++++
drivers/i2c/busses/Kconfig | 10 +
drivers/i2c/busses/Makefile | 1 +
drivers/i2c/busses/i2c-ad242x.c | 178 +++++
drivers/mfd/Kconfig | 11 +
drivers/mfd/Makefile | 1 +
drivers/mfd/ad242x-bus.c | 42 ++
drivers/mfd/ad242x-master.c | 611 ++++++++++++++++++
drivers/mfd/ad242x-node.c | 262 ++++++++
drivers/mfd/ad242x-slave.c | 234 +++++++
include/dt-bindings/clock/adi,ad242x.h | 9 +
include/linux/mfd/ad242x.h | 400 ++++++++++++
sound/soc/codecs/Kconfig | 5 +
sound/soc/codecs/Makefile | 2 +
sound/soc/codecs/ad242x.c | 338 ++++++++++
27 files changed, 3109 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/adi,ad242x-clk.yaml
create mode 100644 Documentation/devicetree/bindings/gpio/adi,ad242x-gpio.yaml
create mode 100644 Documentation/devicetree/bindings/i2c/adi,ad242x-i2c.yaml
create mode 100644 Documentation/devicetree/bindings/mfd/adi,ad242x-bus.yaml
create mode 100644 Documentation/devicetree/bindings/mfd/adi,ad242x-master.yaml
create mode 100644 Documentation/devicetree/bindings/mfd/adi,ad242x-slave.yaml
create mode 100644 Documentation/devicetree/bindings/sound/adi,ad242x-codec.yaml
create mode 100644 drivers/clk/clk-ad242x.c
create mode 100644 drivers/gpio/gpio-ad242x.c
create mode 100644 drivers/i2c/busses/i2c-ad242x.c
create mode 100644 drivers/mfd/ad242x-bus.c
create mode 100644 drivers/mfd/ad242x-master.c
create mode 100644 drivers/mfd/ad242x-node.c
create mode 100644 drivers/mfd/ad242x-slave.c
create mode 100644 include/dt-bindings/clock/adi,ad242x.h
create mode 100644 include/linux/mfd/ad242x.h
create mode 100644 sound/soc/codecs/ad242x.c

--
2.23.0


2019-12-09 18:44:30

by Daniel Mack

[permalink] [raw]
Subject: [PATCH 05/10] dt-bindings: sound: Add documentation for AD242x codecs

This device must be placed as a sub-device of an AD242x MFD node.

Signed-off-by: Daniel Mack <[email protected]>
---
.../bindings/sound/adi,ad242x-codec.yaml | 31 +++++++++++++++++++
1 file changed, 31 insertions(+)
create mode 100644 Documentation/devicetree/bindings/sound/adi,ad242x-codec.yaml

diff --git a/Documentation/devicetree/bindings/sound/adi,ad242x-codec.yaml b/Documentation/devicetree/bindings/sound/adi,ad242x-codec.yaml
new file mode 100644
index 000000000000..2cfb6f9fc548
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/adi,ad242x-codec.yaml
@@ -0,0 +1,31 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/sound/adi,ad242x-codec.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Analog Devices AD242x clock provider
+
+maintainers:
+ - Daniel Mack <[email protected]>
+
+description: |
+ This module is part of the AD242x MFD device. For more details and an example
+ refer to Documentation/devicetree/bindings/mfd/ad242x.yaml.
+
+properties:
+ compatible:
+ enum:
+ - adi,ad2428w-codec
+
+ '#sound-dai-cells':
+ const: 1
+
+ adi,pdm-highpass-filter:
+ type: boolean
+ description: |
+ Enables highpass filtering for data received through the PDM ports
+
+required:
+ - compatible
+ - '#sound-dai-cells'
\ No newline at end of file
--
2.23.0

2019-12-09 18:44:31

by Daniel Mack

[permalink] [raw]
Subject: [PATCH 07/10] i2c: Add driver for AD242x bus controller

This device must be instantiated as a sub-device of the AD242x MFD
device.

In order to access remote I2C peripherals, the master node is configured
to the slave node number and the remote I2C client address on the remote
side, and then the payload is sent to the BUS client of the master node,
which transparently proxies the traffic through.

Signed-off-by: Daniel Mack <[email protected]>
---
drivers/i2c/busses/Kconfig | 10 ++
drivers/i2c/busses/Makefile | 1 +
drivers/i2c/busses/i2c-ad242x.c | 178 ++++++++++++++++++++++++++++++++
3 files changed, 189 insertions(+)
create mode 100644 drivers/i2c/busses/i2c-ad242x.c

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 6a0aa76859f3..b9cf049bedb0 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -365,6 +365,16 @@ config I2C_POWERMAC

comment "I2C system bus drivers (mostly embedded / system-on-chip)"

+config I2C_AD242X
+ tristate "Analog Devices AD242x"
+ depends on MFD_AD242X
+ help
+ If you say yes to this option, support will be included for the
+ I2C bus controller function of AD242x slave nodes.
+
+ This driver can also be built as a module. If so, the module
+ will be called i2c-ad242x.
+
config I2C_ALTERA
tristate "Altera Soft IP I2C"
depends on (ARCH_SOCFPGA || NIOS2) && OF
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 3ab8aebc39c9..57c31ea8a477 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -32,6 +32,7 @@ obj-$(CONFIG_I2C_HYDRA) += i2c-hydra.o
obj-$(CONFIG_I2C_POWERMAC) += i2c-powermac.o

# Embedded system I2C/SMBus host controller drivers
+obj-$(CONFIG_I2C_AD242X) += i2c-ad242x.o
obj-$(CONFIG_I2C_ALTERA) += i2c-altera.o
obj-$(CONFIG_I2C_AMD_MP2) += i2c-amd-mp2-pci.o i2c-amd-mp2-plat.o
obj-$(CONFIG_I2C_ASPEED) += i2c-aspeed.o
diff --git a/drivers/i2c/busses/i2c-ad242x.c b/drivers/i2c/busses/i2c-ad242x.c
new file mode 100644
index 000000000000..b94056653898
--- /dev/null
+++ b/drivers/i2c/busses/i2c-ad242x.c
@@ -0,0 +1,178 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/i2c.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mfd/ad242x.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+struct ad242x_i2c {
+ struct device *dev;
+ struct ad242x_node *node;
+ struct i2c_adapter adap;
+ u32 node_index;
+};
+
+static int ad242x_set_addr(struct ad242x_node *mnode,
+ struct ad242x_i2c_bus *bus,
+ uint8_t node_id, uint8_t addr)
+{
+ int ret;
+ uint8_t buf[2] = { AD242X_CHIP, addr };
+
+ ret = regmap_update_bits(mnode->regmap, AD242X_NODEADR,
+ AD242X_NODEADR_PERI | AD242X_NODEADR_MASK,
+ node_id);
+ if (ret < 0)
+ return ret;
+
+ /*
+ * We can't use the slave's regmap here as it holds the same
+ * lock we also need to guard this context.
+ */
+ ret = i2c_transfer_buffer_flags(bus->client,
+ buf, sizeof(buf), 0);
+ if (ret < 0)
+ return ret;
+
+ return regmap_update_bits(mnode->regmap, AD242X_NODEADR,
+ AD242X_NODEADR_PERI, AD242X_NODEADR_PERI);
+}
+
+static int ad242x_i2c_xfer(struct i2c_adapter *adap,
+ struct i2c_msg msgs[], int num)
+{
+ struct ad242x_i2c *i2c = adap->algo_data;
+ struct ad242x_i2c_bus *bus = ad242x_master_get_bus(i2c->node->master);
+ struct ad242x_node *mnode = ad242x_master_get_node(i2c->node->master);
+ int ret, i, current_addr = -1;
+
+ mutex_lock(&bus->mutex);
+
+ for (i = 0; i < num; i++) {
+ struct i2c_msg *msg = msgs + i;
+
+ if (msg->addr != current_addr) {
+ ret = ad242x_set_addr(mnode, bus,
+ i2c->node->id, msg->addr);
+ if (ret < 0) {
+ dev_err(i2c->node->dev,
+ "Cannot set address: %d\n", ret);
+ break;
+ }
+
+ current_addr = msg->addr;
+ }
+
+ ret = i2c_transfer_buffer_flags(bus->client,
+ msg->buf, msg->len, msg->flags);
+ if (ret < 0)
+ break;
+ }
+
+ mutex_unlock(&bus->mutex);
+
+ return ret < 0 ? ret : num;
+}
+
+static u32 ad242x_i2c_functionality(struct i2c_adapter *adap)
+{
+ return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
+}
+
+static const struct i2c_algorithm ad242x_i2c_algorithm = {
+ .master_xfer = ad242x_i2c_xfer,
+ .functionality = ad242x_i2c_functionality,
+};
+
+static int ad242x_i2c_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct ad242x_node *node;
+ struct ad242x_i2c *i2c;
+ u32 freq, val = 0;
+ int ret;
+
+ if (!dev->of_node)
+ return -ENODEV;
+
+ node = dev_get_drvdata(dev->parent);
+ if ((node->caps & AD242X_CAPABILITY_I2C) == 0) {
+ dev_err(dev, "Node %d has no I2C capability", node->id);
+ return -ENOTSUPP;
+ }
+
+ if (ad242x_node_is_master(node))
+ return -EINVAL;
+
+ freq = ad242x_master_get_clk_rate(node->master);
+ if (freq == 44100)
+ val |= AD242X_I2CCFG_FRAMERATE;
+
+ if (!of_property_read_u32(dev->of_node, "clock-frequency", &freq)) {
+ if (freq == 400000)
+ val |= AD242X_I2CCFG_DATARATE;
+ else if (freq != 100000)
+ dev_warn(dev, "Unsupported frequency %d\n", freq);
+ }
+
+ ret = regmap_write(node->regmap, AD242X_I2CCFG, val);
+ if (ret < 0)
+ return ret;
+
+ i2c = devm_kzalloc(dev, sizeof(*i2c), GFP_KERNEL);
+ if (!i2c)
+ return -ENOMEM;
+
+ i2c->node = node;
+ i2c->adap.algo = &ad242x_i2c_algorithm;
+ i2c->adap.algo_data = i2c;
+ i2c->adap.dev.parent = dev;
+ i2c->adap.dev.of_node = dev->of_node;
+ i2c_set_adapdata(&i2c->adap, i2c);
+ strlcpy(i2c->adap.name, "ad242x remote I2C bus",
+ sizeof(i2c->adap.name));
+
+ ret = i2c_add_adapter(&i2c->adap);
+ if (ret < 0) {
+ dev_err(dev, "error registering adapter: %d\n", ret);
+ return ret;
+ }
+
+ dev_info(dev, "ad242x i2c driver, node ID %d\n", node->id);
+ platform_set_drvdata(pdev, i2c);
+
+ return 0;
+}
+
+static int ad242x_i2c_remove(struct platform_device *dev)
+{
+ struct ad242x_i2c *i2c = platform_get_drvdata(dev);
+
+ i2c_del_adapter(&i2c->adap);
+
+ return 0;
+}
+
+static const struct of_device_id ad242x_i2c_of_match[] = {
+ { .compatible = "adi,ad2428w-i2c" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, ad242x_i2c_of_match);
+
+static struct platform_driver ad242x_i2c_driver = {
+ .driver = {
+ .name = "ad242x-i2c",
+ .of_match_table = ad242x_i2c_of_match,
+ },
+ .probe = ad242x_i2c_probe,
+ .remove = ad242x_i2c_remove,
+};
+
+module_platform_driver(ad242x_i2c_driver);
+MODULE_LICENSE("GPL");
--
2.23.0

2019-12-09 18:44:41

by Daniel Mack

[permalink] [raw]
Subject: [PATCH 04/10] dt-bindings: clock: Add documentation for AD242x clock providers

This device must be placed as a sub-device of an AD242x MFD node.

Signed-off-by: Daniel Mack <[email protected]>
---
.../bindings/clock/adi,ad242x-clk.yaml | 32 +++++++++++++++++++
include/dt-bindings/clock/adi,ad242x.h | 9 ++++++
2 files changed, 41 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/adi,ad242x-clk.yaml
create mode 100644 include/dt-bindings/clock/adi,ad242x.h

diff --git a/Documentation/devicetree/bindings/clock/adi,ad242x-clk.yaml b/Documentation/devicetree/bindings/clock/adi,ad242x-clk.yaml
new file mode 100644
index 000000000000..f434b3e4928e
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/adi,ad242x-clk.yaml
@@ -0,0 +1,32 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/clock/adi,ad242x-clk.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Analog Devices AD242x clock provider
+
+maintainers:
+ - Daniel Mack <[email protected]>
+
+description: |
+ This module is part of the AD242x MFD device. For more details and an example
+ refer to Documentation/devicetree/bindings/mfd/ad242x.yaml.
+
+properties:
+ compatible:
+ enum:
+ - adi,ad2428w-clk
+
+ '#clock-cells':
+ const: 1
+
+ clock-output-names:
+ minItems: 2
+ maxItems: 2
+ description: |
+ Array of two strings to use as names for the generated output clocks
+
+required:
+ - compatible
+ - '#clock-cells'
\ No newline at end of file
diff --git a/include/dt-bindings/clock/adi,ad242x.h b/include/dt-bindings/clock/adi,ad242x.h
new file mode 100644
index 000000000000..307a6cd1f5a6
--- /dev/null
+++ b/include/dt-bindings/clock/adi,ad242x.h
@@ -0,0 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef __DT_BINDINGS_AD242X_CLK_H
+#define __DT_BINDINGS_AD242X_CLK_H
+
+#define MAX9485_CLKOUT1 0
+#define MAX9485_CLKOUT2 1
+
+#endif /* __DT_BINDINGS_AD242X_CLK_H */
--
2.23.0

2019-12-09 18:44:41

by Daniel Mack

[permalink] [raw]
Subject: [PATCH 08/10] gpio: Add driver for AD242x GPIO controllers

This driver makes the 8 GPIOs on AD242x nodes available to consumers.

Apart from that, it also allows putting the GPIO lines in a 'gpio over
distance' mode. This mirrors the state of several GPIOs in the topology
without further interaction by any driver. For instance, when a GPIO pin
on the master node is put in input mode, and another one on a slave node
is in output mode, they can be linked together through virtual ports.
Then, the pin on the slave node will reflect the logical level on
whatever is applied to the respective pin on the master node.

Signed-off-by: Daniel Mack <[email protected]>
---
drivers/gpio/Kconfig | 6 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-ad242x.c | 229 +++++++++++++++++++++++++++++++++++++
3 files changed, 236 insertions(+)
create mode 100644 drivers/gpio/gpio-ad242x.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 8adffd42f8cb..c8af1159a585 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -845,6 +845,12 @@ endmenu
menu "I2C GPIO expanders"
depends on I2C

+config GPIO_AD242X
+ tristate "AD242x A2B GPIO controller"
+ depends on MFD_AD242X
+ help
+ This option enables support for GPIOs on AD242x A2B nodes.
+
config GPIO_ADP5588
tristate "ADP5588 I2C GPIO expander"
help
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 34eb8b2b12dd..2490ce6e6905 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -22,6 +22,7 @@ obj-$(CONFIG_GPIO_104_IDI_48) += gpio-104-idi-48.o
obj-$(CONFIG_GPIO_104_IDIO_16) += gpio-104-idio-16.o
obj-$(CONFIG_GPIO_74X164) += gpio-74x164.o
obj-$(CONFIG_GPIO_74XX_MMIO) += gpio-74xx-mmio.o
+obj-$(CONFIG_GPIO_AD242X) += gpio-ad242x.o
obj-$(CONFIG_GPIO_ADNP) += gpio-adnp.o
obj-$(CONFIG_GPIO_ADP5520) += gpio-adp5520.o
obj-$(CONFIG_GPIO_ADP5588) += gpio-adp5588.o
diff --git a/drivers/gpio/gpio-ad242x.c b/drivers/gpio/gpio-ad242x.c
new file mode 100644
index 000000000000..8970e434b56a
--- /dev/null
+++ b/drivers/gpio/gpio-ad242x.c
@@ -0,0 +1,229 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/gpio/driver.h>
+#include <linux/init.h>
+#include <linux/mfd/ad242x.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+struct ad242x_gpio {
+ struct gpio_chip chip;
+ struct ad242x_node *node;
+ u32 gpio_od_mask;
+};
+
+static int ad242x_gpio_request(struct gpio_chip *chip, unsigned int gpio)
+{
+ struct ad242x_gpio *ad242x_gpio = gpiochip_get_data(chip);
+
+ if (gpio == 0 && ad242x_node_is_master(ad242x_gpio->node))
+ return -EBUSY;
+
+ if (ad242x_gpio->gpio_od_mask & BIT(gpio))
+ return -EBUSY;
+
+ return 0;
+}
+
+static int ad242x_gpio_get_value(struct gpio_chip *chip, unsigned int gpio)
+{
+ struct ad242x_gpio *ad242x_gpio = gpiochip_get_data(chip);
+ struct regmap *regmap = ad242x_gpio->node->regmap;
+ unsigned int val;
+ int ret;
+
+ ret = regmap_read(regmap, AD242X_GPIODAT_IN, &val);
+ if (ret < 0)
+ return ret;
+
+ return !!(val & BIT(gpio));
+}
+
+static void ad242x_gpio_set_value(struct gpio_chip *chip,
+ unsigned int gpio, int value)
+{
+ struct ad242x_gpio *ad242x_gpio = gpiochip_get_data(chip);
+ struct regmap *regmap = ad242x_gpio->node->regmap;
+ uint8_t bit = BIT(gpio);
+ int ret;
+
+ if (value)
+ ret = regmap_write(regmap, AD242X_GPIODAT_SET, bit);
+ else
+ ret = regmap_write(regmap, AD242X_GPIODAT_CLR, bit);
+
+ if (ret < 0)
+ dev_err(ad242x_gpio->node->dev,
+ "Unable to set GPIO #%d: %d\n", gpio, ret);
+}
+
+static int ad242x_gpio_direction_input(struct gpio_chip *chip,
+ unsigned int gpio)
+{
+ struct ad242x_gpio *ad242x_gpio = gpiochip_get_data(chip);
+ struct regmap *regmap = ad242x_gpio->node->regmap;
+ uint8_t bit = BIT(gpio);
+ int ret;
+
+ ret = regmap_update_bits(regmap, AD242X_GPIOOEN, bit, 0);
+ if (ret < 0)
+ return ret;
+
+ ret = regmap_update_bits(regmap, AD242X_GPIOIEN, bit, bit);
+ if (ret < 0)
+ return ret;
+
+ ret = regmap_update_bits(regmap, AD242X_INTMSK1, bit, bit);
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
+
+static int ad242x_gpio_direction_output(struct gpio_chip *chip,
+ unsigned int gpio, int value)
+{
+ struct ad242x_gpio *ad242x_gpio = gpiochip_get_data(chip);
+ struct regmap *regmap = ad242x_gpio->node->regmap;
+ uint8_t bit = BIT(gpio);
+ int ret;
+
+ ret = regmap_update_bits(regmap, AD242X_GPIOIEN, bit, 0);
+ if (ret < 0)
+ return ret;
+
+ ret = regmap_update_bits(regmap, AD242X_GPIOOEN, bit, bit);
+ if (ret < 0)
+ return ret;
+
+ ret = regmap_update_bits(regmap, AD242X_INTMSK1, bit, 0);
+ if (ret < 0)
+ return ret;
+
+ ad242x_gpio_set_value(chip, gpio, value);
+
+ return 0;
+}
+
+static int ad242x_gpio_over_distance_init(struct device *dev,
+ struct ad242x_gpio *ad242x_gpio)
+{
+ struct regmap *regmap = ad242x_gpio->node->regmap;
+ struct device_node *np, *child_np;
+ int ret = 0;
+
+ np = of_get_child_by_name(dev->of_node, "gpio-over-distance");
+ if (!np)
+ return 0;
+
+ for_each_available_child_of_node(np, child_np) {
+ u32 reg, port_mask, bit;
+ bool output, inv;
+
+ ret = of_property_read_u32(child_np, "reg", &reg);
+ if (ret < 0)
+ continue;
+
+ ret = of_property_read_u32(child_np, "adi,virtual-port-mask",
+ &port_mask);
+ if (ret < 0)
+ continue;
+
+ if (reg > 7) {
+ ret = -EINVAL;
+ break;
+ }
+
+ bit = BIT(reg);
+
+ ret = regmap_update_bits(regmap, AD242X_GPIODEN, bit, bit);
+ if (ret < 0)
+ break;
+
+ ret = regmap_write(regmap, AD242X_GPIOD_MSK(reg), port_mask);
+ if (ret < 0)
+ break;
+
+ output = of_property_read_bool(child_np, "adi,gpio-output");
+ ret = regmap_update_bits(regmap, AD242X_GPIOOEN,
+ bit, output ? bit : 0);
+ if (ret < 0)
+ break;
+
+ inv = of_property_read_bool(child_np, "adi,gpio-inverted");
+ ret = regmap_update_bits(regmap, AD242X_GPIODINV,
+ bit, inv ? bit : 0);
+ if (ret < 0)
+ break;
+
+ ad242x_gpio->gpio_od_mask |= bit;
+ dev_info(dev,
+ "pin %d set up as gpio-over-distance, port mask 0x%02x\n",
+ reg, port_mask);
+ }
+
+ of_node_put(np);
+
+ return ret;
+}
+
+static int ad242x_gpio_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct ad242x_gpio *ad242x_gpio;
+ int ret;
+
+ if (!dev->of_node)
+ return -ENODEV;
+
+ ad242x_gpio = devm_kzalloc(dev, sizeof(*ad242x_gpio), GFP_KERNEL);
+ if (!ad242x_gpio)
+ return -ENOMEM;
+
+ ad242x_gpio->node = dev_get_drvdata(dev->parent);
+
+ ad242x_gpio->chip.request = ad242x_gpio_request;
+ ad242x_gpio->chip.direction_input = ad242x_gpio_direction_input;
+ ad242x_gpio->chip.direction_output = ad242x_gpio_direction_output;
+ ad242x_gpio->chip.get = ad242x_gpio_get_value;
+ ad242x_gpio->chip.set = ad242x_gpio_set_value;
+ ad242x_gpio->chip.can_sleep = 1;
+ ad242x_gpio->chip.base = -1;
+ ad242x_gpio->chip.ngpio = 8;
+ ad242x_gpio->chip.label = "ad242x-gpio";
+ ad242x_gpio->chip.owner = THIS_MODULE;
+ ad242x_gpio->chip.parent = dev;
+
+ dev_info(dev, "A2B node ID %d\n", ad242x_gpio->node->id);
+
+ ret = ad242x_gpio_over_distance_init(dev, ad242x_gpio);
+ if (ret < 0) {
+ dev_err(dev, "GPIO over distance init failed: %d\n", ret);
+ return ret;
+ }
+
+ return devm_gpiochip_add_data(dev, &ad242x_gpio->chip, ad242x_gpio);
+}
+
+static const struct of_device_id ad242x_gpio_of_match[] = {
+ { .compatible = "adi,ad2428w-gpio", },
+ {}
+};
+MODULE_DEVICE_TABLE(of, ad242x_gpio_of_match);
+
+static struct platform_driver ad242x_gpio_driver = {
+ .driver = {
+ .name = "ad242x-gpio",
+ .of_match_table = ad242x_gpio_of_match,
+ },
+ .probe = ad242x_gpio_probe,
+};
+module_platform_driver(ad242x_gpio_driver);
+
+MODULE_DESCRIPTION("AD242x GPIO driver");
+MODULE_AUTHOR("Daniel Mack <[email protected]>");
+MODULE_LICENSE("GPL v2");
--
2.23.0

2019-12-09 18:45:10

by Daniel Mack

[permalink] [raw]
Subject: [PATCH 02/10] dt-bindings: i2c: Add documentation for ad242x i2c controllers

This device must be placed as a sub-device of an AD242x MFD node.

Signed-off-by: Daniel Mack <[email protected]>
---
.../bindings/i2c/adi,ad242x-i2c.yaml | 31 +++++++++++++++++++
1 file changed, 31 insertions(+)
create mode 100644 Documentation/devicetree/bindings/i2c/adi,ad242x-i2c.yaml

diff --git a/Documentation/devicetree/bindings/i2c/adi,ad242x-i2c.yaml b/Documentation/devicetree/bindings/i2c/adi,ad242x-i2c.yaml
new file mode 100644
index 000000000000..ded92f8a791b
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/adi,ad242x-i2c.yaml
@@ -0,0 +1,31 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/i2c/adi,ad242x-i2c.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Analog Devices AD242x I2C controller
+
+maintainers:
+ - Daniel Mack <[email protected]>
+
+allOf:
+ - $ref: /schemas/i2c/i2c-controller.yaml#
+
+description: |
+ This module is part of the AD242x MFD device. For more details and an example
+ refer to Documentation/devicetree/bindings/mfd/ad242x.yaml.
+
+properties:
+ compatible:
+ enum:
+ - adi,ad2428w-i2c
+
+ clock-frequency:
+ $ref: '/schemas/types.yaml#/definitions/uint32'
+ default: 100000
+ enum: [100000, 400000]
+ description: Specifies the I²C clock frequency in Hz.
+
+required:
+ - compatible
--
2.23.0

2019-12-09 18:45:12

by Daniel Mack

[permalink] [raw]
Subject: [PATCH 10/10] ASoC: Add codec component for AD242x nodes

This driver makes AD242x nodes available as DAIs in ASoC topologies.

The hardware allows multiple TDM channel modes and bitdepths, but
as these modes have influence in the timing calculations at discovery
time, the mode in that the will be used in needs to be configured
statically in the devicetree.

The configuration applied at runtime through hwparams() is then
required to match the pre-configured settings.

Signed-off-by: Daniel Mack <[email protected]>
---
sound/soc/codecs/Kconfig | 5 +
sound/soc/codecs/Makefile | 2 +
sound/soc/codecs/ad242x.c | 338 ++++++++++++++++++++++++++++++++++++++
3 files changed, 345 insertions(+)
create mode 100644 sound/soc/codecs/ad242x.c

diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index 4abf37b5083f..75365abc277f 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -22,6 +22,7 @@ config SND_SOC_ALL_CODECS
select SND_SOC_AD193X_SPI if SPI_MASTER
select SND_SOC_AD193X_I2C if I2C
select SND_SOC_AD1980 if SND_SOC_AC97_BUS
+ select SND_SOC_AD242X if MFD_AD242X
select SND_SOC_AD73311
select SND_SOC_ADAU1373 if I2C
select SND_SOC_ADAU1761_I2C if I2C
@@ -333,6 +334,10 @@ config SND_SOC_AD1980
select REGMAP_AC97
tristate

+config SND_SOC_AD242X
+ tristate "Analog Devices AD242x CODEC"
+ depends on MFD_AD242X
+
config SND_SOC_AD73311
tristate

diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
index ddfd07071925..ec76448fc1da 100644
--- a/sound/soc/codecs/Makefile
+++ b/sound/soc/codecs/Makefile
@@ -7,6 +7,7 @@ snd-soc-ad193x-objs := ad193x.o
snd-soc-ad193x-spi-objs := ad193x-spi.o
snd-soc-ad193x-i2c-objs := ad193x-i2c.o
snd-soc-ad1980-objs := ad1980.o
+snd-soc-ad242x-objs := ad242x.o
snd-soc-ad73311-objs := ad73311.o
snd-soc-adau-utils-objs := adau-utils.o
snd-soc-adau1373-objs := adau1373.o
@@ -294,6 +295,7 @@ obj-$(CONFIG_SND_SOC_AD193X) += snd-soc-ad193x.o
obj-$(CONFIG_SND_SOC_AD193X_SPI) += snd-soc-ad193x-spi.o
obj-$(CONFIG_SND_SOC_AD193X_I2C) += snd-soc-ad193x-i2c.o
obj-$(CONFIG_SND_SOC_AD1980) += snd-soc-ad1980.o
+obj-$(CONFIG_SND_SOC_AD242X) += snd-soc-ad242x.o
obj-$(CONFIG_SND_SOC_AD73311) += snd-soc-ad73311.o
obj-$(CONFIG_SND_SOC_ADAU_UTILS) += snd-soc-adau-utils.o
obj-$(CONFIG_SND_SOC_ADAU1373) += snd-soc-adau1373.o
diff --git a/sound/soc/codecs/ad242x.c b/sound/soc/codecs/ad242x.c
new file mode 100644
index 000000000000..76189a7c3c92
--- /dev/null
+++ b/sound/soc/codecs/ad242x.c
@@ -0,0 +1,338 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/mfd/ad242x.h>
+#include <linux/of_device.h>
+#include <linux/slab.h>
+#include <sound/asoundef.h>
+#include <sound/core.h>
+#include <sound/initval.h>
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+
+struct ad242x_private {
+ struct ad242x_node *node;
+ bool pdm[2];
+ bool pdm_highpass;
+};
+
+static const struct snd_soc_dapm_widget ad242x_dapm_widgets[] = {
+ SND_SOC_DAPM_AIF_IN("RX0", NULL, 0, SND_SOC_NOPM, 0, 0),
+ SND_SOC_DAPM_AIF_IN("RX1", NULL, 0, SND_SOC_NOPM, 0, 0),
+ SND_SOC_DAPM_AIF_OUT("TX0", NULL, 0, SND_SOC_NOPM, 0, 0),
+ SND_SOC_DAPM_AIF_OUT("TX1", NULL, 0, SND_SOC_NOPM, 0, 0),
+};
+
+static const struct snd_soc_dapm_route ad242x_dapm_routes[] = {
+ { "DAI0 Playback", NULL, "RX0" },
+ { "TX0", NULL, "DAI0 Capture" },
+ { "DAI1 Playback", NULL, "RX1" },
+ { "TX1", NULL, "DAI1 Capture" },
+};
+
+static int ad242x_set_dai_fmt(struct snd_soc_dai *codec_dai,
+ unsigned int format,
+ unsigned int index)
+{
+ struct snd_soc_component *component = codec_dai->component;
+ struct ad242x_private *priv = snd_soc_component_get_drvdata(component);
+ int ret, val = 0;
+
+ /* set DAI format */
+ switch (format & SND_SOC_DAIFMT_FORMAT_MASK) {
+ case SND_SOC_DAIFMT_I2S:
+ priv->pdm[index] = false;
+ break;
+ case SND_SOC_DAIFMT_PDM:
+ priv->pdm[index] = true;
+ break;
+ default:
+ dev_err(component->dev, "unsupported dai format\n");
+ return -EINVAL;
+ }
+
+ /*
+ * Setting clock inversion is only supported globally for both DAIs,
+ * so we ignore the settings made for DAI1 here.
+ */
+ if (index == 0) {
+ switch (format & SND_SOC_DAIFMT_INV_MASK) {
+ case SND_SOC_DAIFMT_NB_NF:
+ val = 0;
+ break;
+ case SND_SOC_DAIFMT_IB_NF:
+ val = AD242X_I2SCTL_RXBCLKINV;
+ break;
+ case SND_SOC_DAIFMT_NB_IF:
+ case SND_SOC_DAIFMT_IB_IF:
+ dev_err(component->dev, "unsupported inversion mask\n");
+ return -EINVAL;
+ }
+
+ ret = regmap_update_bits(priv->node->regmap, AD242X_I2SCTL,
+ AD242X_I2SCTL_RXBCLKINV, val);
+ if (ret < 0)
+ return ret;
+ }
+
+ if (ad242x_node_is_master(priv->node) &&
+ ((format & SND_SOC_DAIFMT_MASTER_MASK) != SND_SOC_DAIFMT_CBS_CFS)) {
+ dev_err(component->dev, "master node must be clock slave\n");
+ return -EINVAL;
+ }
+
+ if (!ad242x_node_is_master(priv->node) &&
+ ((format & SND_SOC_DAIFMT_MASTER_MASK) != SND_SOC_DAIFMT_CBM_CFM)) {
+ dev_err(component->dev, "slave node must be clock master\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int ad242x_set_dai_fmt_dai0(struct snd_soc_dai *codec_dai,
+ unsigned int format)
+{
+ return ad242x_set_dai_fmt(codec_dai, format, 0);
+}
+
+static int ad242x_set_dai_fmt_dai1(struct snd_soc_dai *codec_dai,
+ unsigned int format)
+{
+ return ad242x_set_dai_fmt(codec_dai, format, 1);
+}
+
+static int ad242x_hw_params(struct snd_pcm_substream *substream,
+ struct snd_pcm_hw_params *params,
+ struct snd_soc_dai *dai,
+ int index)
+{
+ struct snd_soc_component *component = dai->component;
+ struct ad242x_private *priv = snd_soc_component_get_drvdata(component);
+ unsigned int sff_rate = ad242x_master_get_clk_rate(priv->node->master);
+ unsigned int rate = params_rate(params);
+ unsigned int val, mask;
+ int ret;
+
+ switch (params_format(params)) {
+ case SNDRV_PCM_FORMAT_S16_LE:
+ if (priv->node->tdm_slot_size != 16)
+ return -EINVAL;
+ break;
+ case SNDRV_PCM_FORMAT_S32_LE:
+ if (priv->node->tdm_slot_size != 32)
+ return -EINVAL;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ if (priv->pdm[index]) {
+ if (substream->stream != SNDRV_PCM_STREAM_PLAYBACK)
+ return -EINVAL;
+
+ if (index == 0) {
+ val = AD242X_PDMCTL_PDM0EN;
+ mask = AD242X_PDMCTL_PDM0EN | AD242X_PDMCTL_PDM0SLOTS;
+ } else {
+ val = AD242X_PDMCTL_PDM1EN;
+ mask = AD242X_PDMCTL_PDM1EN | AD242X_PDMCTL_PDM1SLOTS;
+ }
+
+ switch (params_channels(params)) {
+ case 1:
+ break;
+ case 2:
+ val = mask;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ mask |= AD242X_PDMCTL_HPFEN;
+ if (priv->pdm_highpass)
+ val |= AD242X_PDMCTL_HPFEN;
+
+ ret = regmap_update_bits(priv->node->regmap, AD242X_PDMCTL,
+ mask, val);
+ if (ret < 0)
+ return ret;
+ } else {
+ if (params_channels(params) != priv->node->tdm_mode)
+ return -EINVAL;
+
+ if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+ if (index == 0)
+ mask = AD242X_I2SCTL_RX0EN;
+ else
+ mask = AD242X_I2SCTL_RX1EN;
+ } else {
+ if (index == 0)
+ mask = AD242X_I2SCTL_TX0EN;
+ else
+ mask = AD242X_I2SCTL_TX1EN;
+ }
+
+ ret = regmap_update_bits(priv->node->regmap, AD242X_I2SCTL,
+ mask, mask);
+ if (ret < 0)
+ return ret;
+ }
+
+ if (!ad242x_node_is_master(priv->node)) {
+ val = 0;
+
+ if (rate == sff_rate / 2)
+ val = AD242X_I2SRATE_I2SRATE(1);
+ else if (rate == sff_rate / 4)
+ val = AD242X_I2SRATE_I2SRATE(2);
+ else if (rate == sff_rate * 2)
+ val = AD242X_I2SRATE_I2SRATE(5);
+ else if (rate == sff_rate * 4)
+ val = AD242X_I2SRATE_I2SRATE(6);
+ else if (rate != sff_rate)
+ return -EINVAL;
+
+ ret = regmap_write(priv->node->regmap, AD242X_I2SRATE, val);
+ if (ret < 0)
+ return ret;
+ }
+
+ return 0;
+}
+
+static int ad242x_hw_params_dai0(struct snd_pcm_substream *substream,
+ struct snd_pcm_hw_params *params,
+ struct snd_soc_dai *dai)
+{
+ return ad242x_hw_params(substream, params, dai, 0);
+}
+
+static int ad242x_hw_params_dai1(struct snd_pcm_substream *substream,
+ struct snd_pcm_hw_params *params,
+ struct snd_soc_dai *dai)
+{
+ return ad242x_hw_params(substream, params, dai, 1);
+}
+
+static const struct snd_soc_dai_ops ad242x_dai0_ops = {
+ .hw_params = ad242x_hw_params_dai0,
+ .set_fmt = ad242x_set_dai_fmt_dai0,
+};
+
+static const struct snd_soc_dai_ops ad242x_dai1_ops = {
+ .hw_params = ad242x_hw_params_dai1,
+ .set_fmt = ad242x_set_dai_fmt_dai1,
+};
+
+#define AD242X_RATES ( \
+ SNDRV_PCM_RATE_22050 | SNDRV_PCM_RATE_32000 | \
+ SNDRV_PCM_RATE_44100 | SNDRV_PCM_RATE_48000 | \
+ SNDRV_PCM_RATE_88200 | SNDRV_PCM_RATE_96000 | \
+ SNDRV_PCM_RATE_176400 | SNDRV_PCM_RATE_192000)
+#define AD242X_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S32_LE)
+
+static struct snd_soc_dai_driver ad242x_dai[] = {
+ {
+ .name = "ad242x-dai0",
+ .playback = {
+ .stream_name = "DAI0 Playback",
+ .channels_min = 1,
+ .channels_max = 32,
+ .rates = AD242X_RATES,
+ .formats = AD242X_FORMATS,
+ },
+ .capture = {
+ .stream_name = "DAI0 Capture",
+ .channels_min = 1,
+ .channels_max = 32,
+ .rates = AD242X_RATES,
+ .formats = AD242X_FORMATS,
+ },
+ .ops = &ad242x_dai0_ops,
+ },
+ {
+ .name = "ad242x-dai1",
+ .playback = {
+ .stream_name = "DAI1 Playback",
+ .channels_min = 1,
+ .channels_max = 32,
+ .rates = AD242X_RATES,
+ .formats = AD242X_FORMATS,
+ },
+ .capture = {
+ .stream_name = "DAI1 Capture",
+ .channels_min = 1,
+ .channels_max = 32,
+ .rates = AD242X_RATES,
+ .formats = AD242X_FORMATS,
+ },
+ .ops = &ad242x_dai1_ops,
+ },
+};
+
+static int ad242x_soc_probe(struct snd_soc_component *component)
+{
+ struct ad242x_private *priv = snd_soc_component_get_drvdata(component);
+
+ component->regmap = priv->node->regmap;
+
+ return 0;
+}
+
+static const struct snd_soc_component_driver soc_component_device_ad242x = {
+ .probe = ad242x_soc_probe,
+ .dapm_widgets = ad242x_dapm_widgets,
+ .num_dapm_widgets = ARRAY_SIZE(ad242x_dapm_widgets),
+ .dapm_routes = ad242x_dapm_routes,
+ .num_dapm_routes = ARRAY_SIZE(ad242x_dapm_routes),
+ .idle_bias_on = 1,
+ .use_pmdown_time = 1,
+ .endianness = 1,
+ .non_legacy_dai_naming = 1,
+};
+
+static int ad242x_codec_platform_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct ad242x_private *priv;
+
+ if (!dev->of_node)
+ return -ENODEV;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->node = dev_get_drvdata(dev->parent);
+ platform_set_drvdata(pdev, priv);
+
+ priv->pdm_highpass = of_property_read_bool(dev->of_node,
+ "adi,pdm-highpass-filter");
+
+ return devm_snd_soc_register_component(dev,
+ &soc_component_device_ad242x,
+ ad242x_dai,
+ ARRAY_SIZE(ad242x_dai));
+}
+
+static const struct of_device_id ad242x_of_match[] = {
+ { .compatible = "adi,ad2428w-codec", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, ad242x_of_match);
+
+static struct platform_driver ad242x_platform_driver = {
+ .driver = {
+ .name = "ad242x-codec",
+ .of_match_table = ad242x_of_match,
+ },
+ .probe = ad242x_codec_platform_probe,
+};
+
+module_platform_driver(ad242x_platform_driver);
+
+MODULE_AUTHOR("Daniel Mack <[email protected]>");
+MODULE_DESCRIPTION("AD242X ALSA SoC driver");
+MODULE_LICENSE("GPL");
--
2.23.0

2019-12-12 16:12:25

by Luca Ceresoli

[permalink] [raw]
Subject: Re: [PATCH 07/10] i2c: Add driver for AD242x bus controller

Hi Daniel,

On 09/12/19 19:35, Daniel Mack wrote:
> This device must be instantiated as a sub-device of the AD242x MFD
> device.
>
> In order to access remote I2C peripherals, the master node is configured
> to the slave node number and the remote I2C client address on the remote
> side, and then the payload is sent to the BUS client of the master node,
> which transparently proxies the traffic through.

This remote I2C feature in these chips is interesting. It looks somewhat
similar to remote I2C in the video serdes chip by TI and Maxim, but it's
different from both of them. So now we have 3 vendors implementing the
same feature in 3 different ways.

Cool.

> Signed-off-by: Daniel Mack <[email protected]>
> ---
> drivers/i2c/busses/Kconfig | 10 ++
> drivers/i2c/busses/Makefile | 1 +
> drivers/i2c/busses/i2c-ad242x.c | 178 ++++++++++++++++++++++++++++++++
> 3 files changed, 189 insertions(+)
> create mode 100644 drivers/i2c/busses/i2c-ad242x.c
>
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index 6a0aa76859f3..b9cf049bedb0 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -365,6 +365,16 @@ config I2C_POWERMAC
>
> comment "I2C system bus drivers (mostly embedded / system-on-chip)"
>
> +config I2C_AD242X
> + tristate "Analog Devices AD242x"
> + depends on MFD_AD242X
> + help
> + If you say yes to this option, support will be included for the
> + I2C bus controller function of AD242x slave nodes.
> +
> + This driver can also be built as a module. If so, the module
> + will be called i2c-ad242x.
> +
> config I2C_ALTERA
> tristate "Altera Soft IP I2C"
> depends on (ARCH_SOCFPGA || NIOS2) && OF
> diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
> index 3ab8aebc39c9..57c31ea8a477 100644
> --- a/drivers/i2c/busses/Makefile
> +++ b/drivers/i2c/busses/Makefile
> @@ -32,6 +32,7 @@ obj-$(CONFIG_I2C_HYDRA) += i2c-hydra.o
> obj-$(CONFIG_I2C_POWERMAC) += i2c-powermac.o
>
> # Embedded system I2C/SMBus host controller drivers
> +obj-$(CONFIG_I2C_AD242X) += i2c-ad242x.o
> obj-$(CONFIG_I2C_ALTERA) += i2c-altera.o
> obj-$(CONFIG_I2C_AMD_MP2) += i2c-amd-mp2-pci.o i2c-amd-mp2-plat.o
> obj-$(CONFIG_I2C_ASPEED) += i2c-aspeed.o
> diff --git a/drivers/i2c/busses/i2c-ad242x.c b/drivers/i2c/busses/i2c-ad242x.c
> new file mode 100644
> index 000000000000..b94056653898
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-ad242x.c
> @@ -0,0 +1,178 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/i2c.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mfd/ad242x.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +struct ad242x_i2c {
> + struct device *dev;
> + struct ad242x_node *node;
> + struct i2c_adapter adap;
> + u32 node_index;
> +};
> +
> +static int ad242x_set_addr(struct ad242x_node *mnode,
> + struct ad242x_i2c_bus *bus,
> + uint8_t node_id, uint8_t addr)
> +{
> + int ret;
> + uint8_t buf[2] = { AD242X_CHIP, addr };
> +
> + ret = regmap_update_bits(mnode->regmap, AD242X_NODEADR,
> + AD242X_NODEADR_PERI | AD242X_NODEADR_MASK,
> + node_id);
> + if (ret < 0)
> + return ret;
> +
> + /*
> + * We can't use the slave's regmap here as it holds the same
> + * lock we also need to guard this context.
> + */
> + ret = i2c_transfer_buffer_flags(bus->client,
> + buf, sizeof(buf), 0);
> + if (ret < 0)
> + return ret;
> +
> + return regmap_update_bits(mnode->regmap, AD242X_NODEADR,
> + AD242X_NODEADR_PERI, AD242X_NODEADR_PERI);
> +}
> +
> +static int ad242x_i2c_xfer(struct i2c_adapter *adap,
> + struct i2c_msg msgs[], int num)
> +{
> + struct ad242x_i2c *i2c = adap->algo_data;
> + struct ad242x_i2c_bus *bus = ad242x_master_get_bus(i2c->node->master);
> + struct ad242x_node *mnode = ad242x_master_get_node(i2c->node->master);
> + int ret, i, current_addr = -1;
> +
> + mutex_lock(&bus->mutex);
> +
> + for (i = 0; i < num; i++) {
> + struct i2c_msg *msg = msgs + i;
> +
> + if (msg->addr != current_addr) {
> + ret = ad242x_set_addr(mnode, bus,
> + i2c->node->id, msg->addr);
> + if (ret < 0) {
> + dev_err(i2c->node->dev,
> + "Cannot set address: %d\n", ret);
> + break;
> + }
> +
> + current_addr = msg->addr;
> + }
> +
> + ret = i2c_transfer_buffer_flags(bus->client,
> + msg->buf, msg->len, msg->flags);
> + if (ret < 0)
> + break;
> + }
> +
> + mutex_unlock(&bus->mutex);
> +
> + return ret < 0 ? ret : num;
> +}

Your implementation here looks quite clean and simple, and simple is
good, but I think there's a problem in this function. A "normal"
master_xfer function issues a repeated start between one msg and the
next one, at least in the typical case where all msgs have the same
slave address. Your implementation breaks repeated start. At first sight
we might need more complex code here to coalesce all consecutive msgs
with the same address into a single i2c_transfer() call.

--
Luca

2019-12-12 16:35:16

by Wolfram Sang

[permalink] [raw]
Subject: Re: [PATCH 07/10] i2c: Add driver for AD242x bus controller

Hi Luca,

thanks for the review!

> good, but I think there's a problem in this function. A "normal"
> master_xfer function issues a repeated start between one msg and the
> next one, at least in the typical case where all msgs have the same
> slave address. Your implementation breaks repeated start. At first sight
> we might need more complex code here to coalesce all consecutive msgs
> with the same address into a single i2c_transfer() call.

Note that it is by far the standard case that all messages in a transfer
have the same client address (99,999%?). But technically, this is not a
requirement and the repeated start on the bus is totally independent of
the addresses used. It is just a master wanting to send without being
interrupted by another master.

Wolfram


Attachments:
(No filename) (799.00 B)
signature.asc (849.00 B)
Download all attachments

2019-12-15 20:28:17

by Daniel Mack

[permalink] [raw]
Subject: Re: [PATCH 07/10] i2c: Add driver for AD242x bus controller

Hi,

Thanks for the review!

On 12/12/2019 5:33 pm, Wolfram Sang wrote:
> Hi Luca,
>
> thanks for the review!
>
>> good, but I think there's a problem in this function. A "normal"
>> master_xfer function issues a repeated start between one msg and the
>> next one, at least in the typical case where all msgs have the same
>> slave address. Your implementation breaks repeated start. At first sight
>> we might need more complex code here to coalesce all consecutive msgs
>> with the same address into a single i2c_transfer() call.
>
> Note that it is by far the standard case that all messages in a transfer
> have the same client address (99,999%?). But technically, this is not a
> requirement and the repeated start on the bus is totally independent of
> the addresses used. It is just a master wanting to send without being
> interrupted by another master.

I'm not quite sure I understand.

Let's assume the following setup. An i2c client (some driver code) is
sending a list of messages to the a2b xfer function, which in turn is
logically connected to a 'real' i2c bus master that'll put the data on
the wire.

The a2b code has to tell the 'master node' the final destination of the
payload by programming registers on its primary i2c address, and then
forwards the messages to its secondary i2c address. The layout of the
messages don't change, and neither do the flags; i2c messages are being
sent as i2c messages, except their addresses are changed, a bit like NAT
in networking. That procedure is described on page 3-4 of the TRM,
"Remote Peripheral I2C Accesses".

The 'real' i2c master that handles the hardware bus is responsible for
adding start conditions, and as the messages as such are untouched, I
believe it should do the right thing. The code in my xfer functions
merely suppresses reprogramming remote addresses by remembering the last
one that was used, but that is independent of the start conditions on
the wire.

Maybe I'm missing anything. Could you provide an example that explains
in which case this approach would leads to issues?


Thanks,
Daniel

2019-12-24 07:33:21

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH 04/10] dt-bindings: clock: Add documentation for AD242x clock providers

Quoting Daniel Mack (2019-12-09 10:35:05)
> diff --git a/Documentation/devicetree/bindings/clock/adi,ad242x-clk.yaml b/Documentation/devicetree/bindings/clock/adi,ad242x-clk.yaml
> new file mode 100644
> index 000000000000..f434b3e4928e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/adi,ad242x-clk.yaml
> @@ -0,0 +1,32 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/clock/adi,ad242x-clk.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Analog Devices AD242x clock provider
> +
> +maintainers:
> + - Daniel Mack <[email protected]>
> +
> +description: |
> + This module is part of the AD242x MFD device. For more details and an example
> + refer to Documentation/devicetree/bindings/mfd/ad242x.yaml.

I think we usually leave off Documentation/devicetree/ from paths when
they're inside the bindings directory.

> +
> +properties:
> + compatible:
> + enum:
> + - adi,ad2428w-clk
> +
> + '#clock-cells':
> + const: 1
> +
> + clock-output-names:
> + minItems: 2
> + maxItems: 2
> + description: |
> + Array of two strings to use as names for the generated output clocks
> +
> +required:
> + - compatible
> + - '#clock-cells'
> \ No newline at end of file

Why no newline at end of file? Is there an example?

2020-01-08 03:48:03

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH 02/10] dt-bindings: i2c: Add documentation for ad242x i2c controllers

On Mon, Dec 09, 2019 at 07:35:02PM +0100, Daniel Mack wrote:
> This device must be placed as a sub-device of an AD242x MFD node.
>
> Signed-off-by: Daniel Mack <[email protected]>
> ---
> .../bindings/i2c/adi,ad242x-i2c.yaml | 31 +++++++++++++++++++
> 1 file changed, 31 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/i2c/adi,ad242x-i2c.yaml
>
> diff --git a/Documentation/devicetree/bindings/i2c/adi,ad242x-i2c.yaml b/Documentation/devicetree/bindings/i2c/adi,ad242x-i2c.yaml
> new file mode 100644
> index 000000000000..ded92f8a791b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/adi,ad242x-i2c.yaml
> @@ -0,0 +1,31 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/i2c/adi,ad242x-i2c.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Analog Devices AD242x I2C controller
> +
> +maintainers:
> + - Daniel Mack <[email protected]>
> +
> +allOf:
> + - $ref: /schemas/i2c/i2c-controller.yaml#
> +
> +description: |
> + This module is part of the AD242x MFD device. For more details and an example
> + refer to Documentation/devicetree/bindings/mfd/ad242x.yaml.
> +
> +properties:
> + compatible:
> + enum:
> + - adi,ad2428w-i2c
> +
> + clock-frequency:
> + $ref: '/schemas/types.yaml#/definitions/uint32'

Can drop as it already has a type.

> + default: 100000
> + enum: [100000, 400000]
> + description: Specifies the I?C clock frequency in Hz.
> +
> +required:
> + - compatible
> --
> 2.23.0
>