* [PATCH v2 2/2] iio: adc: Add support for AD4000
From: Marcelo Schmitt @ 2024-04-08 14:31 UTC (permalink / raw)
To: lars, Michael.Hennerich, jic23, robh+dt, krzysztof.kozlowski+dt,
conor+dt, marcelo.schmitt1
Cc: linux-iio, devicetree, linux-kernel
In-Reply-To: <cover.1712585500.git.marcelo.schmitt@analog.com>
Add support for AD4000 family of low noise, low power, high speed,
successive aproximation register (SAR) ADCs.
Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
---
MAINTAINERS | 1 +
drivers/iio/adc/Kconfig | 12 +
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/ad4000.c | 649 +++++++++++++++++++++++++++++++++++++++
4 files changed, 663 insertions(+)
create mode 100644 drivers/iio/adc/ad4000.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 5dfe118a5dd3..86aa96115f5a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1165,6 +1165,7 @@ L: linux-iio@vger.kernel.org
S: Supported
W: https://ez.analog.com/linux-software-drivers
F: Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
+F: drivers/iio/adc/ad4000.c
ANALOG DEVICES INC AD4130 DRIVER
M: Cosmin Tanislav <cosmin.tanislav@analog.com>
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 8db68b80b391..9c9d13d4b74f 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -21,6 +21,18 @@ config AD_SIGMA_DELTA
select IIO_BUFFER
select IIO_TRIGGERED_BUFFER
+config AD4000
+ tristate "Analog Devices AD4000 ADC Driver"
+ depends on SPI
+ select IIO_BUFFER
+ select IIO_TRIGGERED_BUFFER
+ help
+ Say yes here to build support for Analog Devices AD4000 high speed
+ SPI analog to digital converters (ADC).
+
+ To compile this driver as a module, choose M here: the module will be
+ called ad4000.
+
config AD4130
tristate "Analog Device AD4130 ADC Driver"
depends on SPI
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index edb32ce2af02..aa52068d864b 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -6,6 +6,7 @@
# When adding new entries keep the list in alphabetical order
obj-$(CONFIG_AB8500_GPADC) += ab8500-gpadc.o
obj-$(CONFIG_AD_SIGMA_DELTA) += ad_sigma_delta.o
+obj-$(CONFIG_AD4000) += ad4000.o
obj-$(CONFIG_AD4130) += ad4130.o
obj-$(CONFIG_AD7091R) += ad7091r-base.o
obj-$(CONFIG_AD7091R5) += ad7091r5.o
diff --git a/drivers/iio/adc/ad4000.c b/drivers/iio/adc/ad4000.c
new file mode 100644
index 000000000000..7997d9d98743
--- /dev/null
+++ b/drivers/iio/adc/ad4000.c
@@ -0,0 +1,649 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * AD4000 SPI ADC driver
+ *
+ * Copyright 2024 Analog Devices Inc.
+ */
+#include <asm/unaligned.h>
+#include <linux/bits.h>
+#include <linux/bitfield.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/kernel.h>
+#include <linux/math.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/gpio/consumer.h>
+#include <linux/regulator/consumer.h>
+#include <linux/spi/spi.h>
+#include <linux/sysfs.h>
+#include <linux/units.h>
+#include <linux/util_macros.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/iio/trigger_consumer.h>
+
+#define AD400X_READ_COMMAND 0x54
+#define AD400X_WRITE_COMMAND 0x14
+
+/* AD4000 Configuration Register programmable bits */
+#define AD4000_STATUS BIT(4) /* Status bits output */
+#define AD4000_SPAN_COMP BIT(3) /* Input span compression */
+#define AD4000_HIGHZ BIT(2) /* High impedance mode */
+#define AD4000_TURBO BIT(1) /* Turbo mode */
+
+#define AD4000_TQUIET2_NS 60
+
+#define AD4000_18BIT_MSK GENMASK(31, 14)
+#define AD4000_20BIT_MSK GENMASK(31, 12)
+
+#define AD4000_DIFF_CHANNEL(_sign, _real_bits) \
+ { \
+ .type = IIO_VOLTAGE, \
+ .indexed = 1, \
+ .differential = 1, \
+ .channel = 0, \
+ .channel2 = 1, \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
+ BIT(IIO_CHAN_INFO_SCALE), \
+ .info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE),\
+ .scan_type = { \
+ .sign = _sign, \
+ .realbits = _real_bits, \
+ .storagebits = _real_bits > 16 ? 32 : 16, \
+ .shift = _real_bits > 16 ? 32 - _real_bits : 0, \
+ .endianness = IIO_BE, \
+ }, \
+ } \
+
+#define AD4000_PSEUDO_DIFF_CHANNEL(_sign, _real_bits) \
+ { \
+ .type = IIO_VOLTAGE, \
+ .indexed = 1, \
+ .channel = 0, \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
+ BIT(IIO_CHAN_INFO_SCALE) | \
+ BIT(IIO_CHAN_INFO_OFFSET), \
+ .info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE),\
+ .scan_type = { \
+ .sign = _sign, \
+ .realbits = _real_bits, \
+ .storagebits = _real_bits > 16 ? 32 : 16, \
+ .shift = _real_bits > 16 ? 32 - _real_bits : 0, \
+ .endianness = IIO_BE, \
+ }, \
+ } \
+
+enum ad4000_ids {
+ ID_AD4000,
+ ID_AD4001,
+ ID_AD4002,
+ ID_AD4003,
+ ID_AD4004,
+ ID_AD4005,
+ ID_AD4006,
+ ID_AD4007,
+ ID_AD4008,
+ ID_AD4010,
+ ID_AD4011,
+ ID_AD4020,
+ ID_AD4021,
+ ID_AD4022,
+ ID_ADAQ4001,
+ ID_ADAQ4003,
+};
+
+struct ad4000_chip_info {
+ const char *dev_name;
+ struct iio_chan_spec chan_spec;
+};
+
+static const struct ad4000_chip_info ad4000_chips[] = {
+ [ID_AD4000] = {
+ .dev_name = "ad4000",
+ .chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 16),
+ },
+ [ID_AD4001] = {
+ .dev_name = "ad4001",
+ .chan_spec = AD4000_DIFF_CHANNEL('s', 16),
+ },
+ [ID_AD4002] = {
+ .dev_name = "ad4002",
+ .chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 18),
+ },
+ [ID_AD4003] = {
+ .dev_name = "ad4003",
+ .chan_spec = AD4000_DIFF_CHANNEL('s', 18),
+ },
+ [ID_AD4004] = {
+ .dev_name = "ad4004",
+ .chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 16),
+ },
+ [ID_AD4005] = {
+ .dev_name = "ad4005",
+ .chan_spec = AD4000_DIFF_CHANNEL('s', 16),
+ },
+ [ID_AD4006] = {
+ .dev_name = "ad4006",
+ .chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 18),
+ },
+ [ID_AD4007] = {
+ .dev_name = "ad4007",
+ .chan_spec = AD4000_DIFF_CHANNEL('s', 18),
+ },
+ [ID_AD4008] = {
+ .dev_name = "ad4008",
+ .chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 16),
+ },
+ [ID_AD4010] = {
+ .dev_name = "ad4010",
+ .chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 18),
+ },
+ [ID_AD4011] = {
+ .dev_name = "ad4011",
+ .chan_spec = AD4000_DIFF_CHANNEL('s', 18),
+ },
+ [ID_AD4020] = {
+ .dev_name = "ad4020",
+ .chan_spec = AD4000_DIFF_CHANNEL('s', 20),
+ },
+ [ID_AD4021] = {
+ .dev_name = "ad4021",
+ .chan_spec = AD4000_DIFF_CHANNEL('s', 20),
+ },
+ [ID_AD4022] = {
+ .dev_name = "ad4022",
+ .chan_spec = AD4000_DIFF_CHANNEL('s', 20),
+ },
+ [ID_ADAQ4001] = {
+ .dev_name = "adaq4001",
+ .chan_spec = AD4000_DIFF_CHANNEL('s', 16),
+ },
+ [ID_ADAQ4003] = {
+ .dev_name = "adaq4003",
+ .chan_spec = AD4000_DIFF_CHANNEL('s', 18),
+ },
+};
+
+enum ad4000_gains {
+ AD4000_0454_GAIN = 0,
+ AD4000_0909_GAIN = 1,
+ AD4000_1_GAIN = 2,
+ AD4000_1900_GAIN = 3,
+ AD4000_GAIN_LEN
+};
+
+/*
+ * Gains stored and computed as fractions to avoid introducing rounding errors.
+ */
+static const int ad4000_gains_frac[AD4000_GAIN_LEN][2] = {
+ [AD4000_0454_GAIN] = { 227, 500 },
+ [AD4000_0909_GAIN] = { 909, 1000 },
+ [AD4000_1_GAIN] = { 1, 1 },
+ [AD4000_1900_GAIN] = { 19, 10 },
+};
+
+struct ad4000_state {
+ struct spi_device *spi;
+ struct gpio_desc *cnv_gpio;
+ int vref;
+ bool status_bits;
+ bool span_comp;
+ bool turbo_mode;
+ bool high_z_mode;
+
+ enum ad4000_gains pin_gain;
+ int scale_tbl[AD4000_GAIN_LEN][2][2];
+
+ /*
+ * DMA (thus cache coherency maintenance) requires the
+ * transfer buffers to live in their own cache lines.
+ */
+ struct {
+ union {
+ u16 sample_buf16;
+ u32 sample_buf32;
+ } data;
+ s64 timestamp __aligned(8);
+ } scan;
+ __be16 tx_buf __aligned(IIO_DMA_MINALIGN);
+ __be16 rx_buf;
+};
+
+static void ad4000_fill_scale_tbl(struct ad4000_state *st, int scale_bits,
+ const struct ad4000_chip_info *chip)
+{
+ int diff = chip->chan_spec.differential;
+ int val, val2, tmp0, tmp1, i;
+ u64 tmp2;
+
+ val2 = scale_bits;
+ for (i = 0; i < AD4000_GAIN_LEN; i++) {
+ val = st->vref / 1000;
+ /* Multiply by MILLI here to avoid losing precision */
+ val = mult_frac(val, ad4000_gains_frac[i][1] * MILLI,
+ ad4000_gains_frac[i][0]);
+ /* Would multiply by NANO here but we already multiplied by MILLI */
+ tmp2 = shift_right((u64)val * MICRO, val2);
+ tmp0 = (int)div_s64_rem(tmp2, NANO, &tmp1);
+ /* Store scale for when span compression is disabled */
+ st->scale_tbl[i][0][0] = tmp0; /* Integer part */
+ st->scale_tbl[i][0][1] = abs(tmp1); /* Fractional part */
+ /* Store scale for when span compression is enabled */
+ st->scale_tbl[i][1][0] = tmp0;
+ if (diff)
+ st->scale_tbl[i][1][1] = DIV_ROUND_CLOSEST(abs(tmp1) * 4, 5);
+ else
+ st->scale_tbl[i][1][1] = DIV_ROUND_CLOSEST(abs(tmp1) * 9, 10);
+ }
+}
+
+static int ad4000_write_reg(struct ad4000_state *st, uint8_t val)
+{
+ put_unaligned_be16(AD400X_WRITE_COMMAND << BITS_PER_BYTE | val,
+ &st->tx_buf);
+ return spi_write(st->spi, &st->tx_buf, 2);
+}
+
+static int ad4000_read_reg(struct ad4000_state *st, unsigned int *val)
+{
+ struct spi_transfer t[] = {
+ {
+ .tx_buf = &st->tx_buf,
+ .rx_buf = &st->rx_buf,
+ .len = 2,
+ },
+ };
+ int ret;
+
+ put_unaligned_be16(AD400X_READ_COMMAND << BITS_PER_BYTE, &st->tx_buf);
+ ret = spi_sync_transfer(st->spi, t, ARRAY_SIZE(t));
+ if (ret < 0)
+ return ret;
+
+ *val = get_unaligned_be16(&st->rx_buf);
+
+ return ret;
+}
+
+static int ad4000_read_sample(struct ad4000_state *st,
+ const struct iio_chan_spec *chan)
+{
+ struct spi_transfer t[] = {
+ {
+ .rx_buf = &st->scan.data,
+ .len = BITS_TO_BYTES(chan->scan_type.storagebits),
+ .delay = {
+ .value = AD4000_TQUIET2_NS,
+ .unit = SPI_DELAY_UNIT_NSECS,
+ },
+ },
+ };
+ int ret;
+
+ ret = spi_sync_transfer(st->spi, t, ARRAY_SIZE(t));
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
+
+static int ad4000_single_conversion(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan, int *val)
+{
+ struct ad4000_state *st = iio_priv(indio_dev);
+ u32 sample;
+ int ret;
+
+ if (st->cnv_gpio)
+ gpiod_set_value_cansleep(st->cnv_gpio, GPIOD_OUT_HIGH);
+
+ ret = ad4000_read_sample(st, chan);
+ if (ret)
+ return ret;
+
+ if (st->cnv_gpio)
+ gpiod_set_value_cansleep(st->cnv_gpio, GPIOD_OUT_LOW);
+
+ if (chan->scan_type.storagebits > 16)
+ sample = get_unaligned_be32(&st->scan.data);
+ else
+ sample = get_unaligned_be16(&st->scan.data);
+
+ switch (chan->scan_type.realbits) {
+ case 16:
+ break;
+ case 18:
+ sample = FIELD_GET(AD4000_18BIT_MSK, sample);
+ break;
+ case 20:
+ sample = FIELD_GET(AD4000_20BIT_MSK, sample);
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ if (chan->scan_type.sign == 's')
+ *val = sign_extend32(sample, chan->scan_type.realbits - 1);
+
+ return IIO_VAL_INT;
+}
+
+static int ad4000_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan, int *val,
+ int *val2, long info)
+{
+ struct ad4000_state *st = iio_priv(indio_dev);
+
+ switch (info) {
+ case IIO_CHAN_INFO_RAW:
+ iio_device_claim_direct_scoped(return -EBUSY, indio_dev)
+ return ad4000_single_conversion(indio_dev, chan, val);
+ unreachable();
+ case IIO_CHAN_INFO_SCALE:
+ *val = st->scale_tbl[st->pin_gain][st->span_comp][0];
+ *val2 = st->scale_tbl[st->pin_gain][st->span_comp][1];
+ return IIO_VAL_INT_PLUS_NANO;
+ case IIO_CHAN_INFO_OFFSET:
+ *val = 0;
+ if (st->span_comp)
+ *val = mult_frac(st->vref / 1000, 1, 10);
+
+ return IIO_VAL_INT;
+ default:
+ break;
+ }
+
+ return -EINVAL;
+}
+
+static int ad4000_read_avail(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ const int **vals, int *type, int *length,
+ long info)
+{
+ struct ad4000_state *st = iio_priv(indio_dev);
+
+ switch (info) {
+ case IIO_CHAN_INFO_SCALE:
+ *vals = (int *)st->scale_tbl[st->pin_gain];
+ *length = 2 * 2;
+ *type = IIO_VAL_INT_PLUS_NANO;
+ return IIO_AVAIL_LIST;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int ad4000_write_raw_get_fmt(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan, long mask)
+{
+ switch (mask) {
+ case IIO_CHAN_INFO_SCALE:
+ return IIO_VAL_INT_PLUS_NANO;
+ default:
+ return IIO_VAL_INT_PLUS_MICRO;
+ }
+ return -EINVAL;
+}
+
+static int ad4000_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan, int val, int val2,
+ long mask)
+{
+ struct ad4000_state *st = iio_priv(indio_dev);
+ unsigned int reg_val;
+ bool span_comp_en;
+ int ret;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_SCALE:
+ iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
+ ret = ad4000_read_reg(st, ®_val);
+ if (ret < 0)
+ return ret;
+
+ span_comp_en = (val2 == st->scale_tbl[st->pin_gain][1][1]);
+ reg_val &= ~AD4000_SPAN_COMP;
+ reg_val |= FIELD_PREP(AD4000_SPAN_COMP, span_comp_en);
+
+ ret = ad4000_write_reg(st, reg_val);
+ if (ret < 0)
+ return ret;
+
+ st->span_comp = span_comp_en;
+ return 0;
+ }
+ unreachable();
+ default:
+ break;
+ }
+
+ return -EINVAL;
+}
+
+static irqreturn_t ad4000_trigger_handler(int irq, void *p)
+{
+ struct iio_poll_func *pf = p;
+ struct iio_dev *indio_dev = pf->indio_dev;
+ struct ad4000_state *st = iio_priv(indio_dev);
+ int ret;
+
+ if (st->cnv_gpio)
+ gpiod_set_value(st->cnv_gpio, GPIOD_OUT_HIGH);
+
+ ret = ad4000_read_sample(st, &indio_dev->channels[0]);
+ if (ret < 0)
+ goto err_out;
+
+ if (st->cnv_gpio)
+ gpiod_set_value(st->cnv_gpio, GPIOD_OUT_LOW);
+
+ iio_push_to_buffers_with_timestamp(indio_dev, &st->scan,
+ iio_get_time_ns(indio_dev));
+
+err_out:
+ iio_trigger_notify_done(indio_dev->trig);
+ return IRQ_HANDLED;
+}
+
+static const struct iio_info ad4000_info = {
+ .read_raw = &ad4000_read_raw,
+ .read_avail = &ad4000_read_avail,
+ .write_raw = &ad4000_write_raw,
+ .write_raw_get_fmt = &ad4000_write_raw_get_fmt,
+};
+
+static void ad4000_config(struct ad4000_state *st)
+{
+ unsigned int reg_val;
+ int ret;
+
+ reg_val = FIELD_PREP(AD4000_TURBO, 1);
+
+ if (device_property_present(&st->spi->dev, "adi,high-z-input"))
+ reg_val |= FIELD_PREP(AD4000_HIGHZ, 1);
+
+ /*
+ * The ADC SDI pin might be connected to controller CS line in which
+ * case the write might fail. This, however, does not prevent the device
+ * from functioning even though in a configuration other than the
+ * requested one.
+ */
+ ret = ad4000_write_reg(st, reg_val);
+ if (ret < 0)
+ dev_dbg(&st->spi->dev, "Failed to config device\n");
+}
+
+static void ad4000_regulator_disable(void *reg)
+{
+ regulator_disable(reg);
+}
+
+static int ad4000_probe(struct spi_device *spi)
+{
+ const struct ad4000_chip_info *chip;
+ struct regulator *vref_reg;
+ struct iio_dev *indio_dev;
+ struct ad4000_state *st;
+ int ret;
+
+ indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ chip = spi_get_device_match_data(spi);
+ if (!chip)
+ return -EINVAL;
+
+ st = iio_priv(indio_dev);
+ st->spi = spi;
+
+ ret = devm_regulator_get_enable(&spi->dev, "vdd");
+ if (ret)
+ return dev_err_probe(&spi->dev, ret, "Failed to enable VDD supply\n");
+
+ ret = devm_regulator_get_enable(&spi->dev, "vio");
+ if (ret)
+ return dev_err_probe(&spi->dev, ret, "Failed to enable VIO supply\n");
+
+ vref_reg = devm_regulator_get(&spi->dev, "ref");
+ if (IS_ERR(vref_reg))
+ return dev_err_probe(&spi->dev, PTR_ERR(vref_reg),
+ "Failed to get vref regulator\n");
+
+ ret = regulator_enable(vref_reg);
+ if (ret < 0)
+ return dev_err_probe(&spi->dev, ret,
+ "Failed to enable voltage regulator\n");
+
+ ret = devm_add_action_or_reset(&spi->dev, ad4000_regulator_disable, vref_reg);
+ if (ret)
+ return dev_err_probe(&spi->dev, ret,
+ "Failed to add regulator disable action\n");
+
+ st->vref = regulator_get_voltage(vref_reg);
+ if (st->vref < 0)
+ return dev_err_probe(&spi->dev, st->vref, "Failed to get vref\n");
+
+ st->cnv_gpio = devm_gpiod_get_optional(&spi->dev, "cnv", GPIOD_OUT_HIGH);
+ if (IS_ERR(st->cnv_gpio)) {
+ if (PTR_ERR(st->cnv_gpio) == -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+
+ return dev_err_probe(&spi->dev, PTR_ERR(st->cnv_gpio),
+ "Failed to get CNV GPIO");
+ }
+
+ ad4000_config(st);
+
+ indio_dev->name = chip->dev_name;
+ indio_dev->info = &ad4000_info;
+ indio_dev->channels = &chip->chan_spec;
+ indio_dev->num_channels = 1;
+
+ st->pin_gain = AD4000_1_GAIN;
+ if (device_property_present(&spi->dev, "adi,gain-milli")) {
+ u32 val;
+
+ ret = device_property_read_u32(&spi->dev, "adi,gain-milli", &val);
+ if (ret)
+ return ret;
+
+ switch (val) {
+ case 454:
+ st->pin_gain = AD4000_0454_GAIN;
+ break;
+ case 909:
+ st->pin_gain = AD4000_0909_GAIN;
+ break;
+ case 1000:
+ st->pin_gain = AD4000_1_GAIN;
+ break;
+ case 1900:
+ st->pin_gain = AD4000_1900_GAIN;
+ break;
+ default:
+ return dev_err_probe(&spi->dev, -EINVAL,
+ "Invalid firmware provided gain\n");
+ }
+ }
+
+ /*
+ * ADCs that output twos complement code have one less bit to express
+ * voltage magnitude.
+ */
+ if (chip->chan_spec.scan_type.sign == 's')
+ ad4000_fill_scale_tbl(st, chip->chan_spec.scan_type.realbits - 1,
+ chip);
+ else
+ ad4000_fill_scale_tbl(st, chip->chan_spec.scan_type.realbits,
+ chip);
+
+ ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev,
+ &iio_pollfunc_store_time,
+ &ad4000_trigger_handler, NULL);
+ if (ret)
+ return ret;
+
+ return devm_iio_device_register(&spi->dev, indio_dev);
+}
+
+static const struct spi_device_id ad4000_id[] = {
+ { "ad4000", (kernel_ulong_t)&ad4000_chips[ID_AD4000] },
+ { "ad4001", (kernel_ulong_t)&ad4000_chips[ID_AD4001] },
+ { "ad4002", (kernel_ulong_t)&ad4000_chips[ID_AD4002] },
+ { "ad4003", (kernel_ulong_t)&ad4000_chips[ID_AD4003] },
+ { "ad4004", (kernel_ulong_t)&ad4000_chips[ID_AD4004] },
+ { "ad4005", (kernel_ulong_t)&ad4000_chips[ID_AD4005] },
+ { "ad4006", (kernel_ulong_t)&ad4000_chips[ID_AD4006] },
+ { "ad4007", (kernel_ulong_t)&ad4000_chips[ID_AD4007] },
+ { "ad4008", (kernel_ulong_t)&ad4000_chips[ID_AD4008] },
+ { "ad4010", (kernel_ulong_t)&ad4000_chips[ID_AD4010] },
+ { "ad4011", (kernel_ulong_t)&ad4000_chips[ID_AD4011] },
+ { "ad4020", (kernel_ulong_t)&ad4000_chips[ID_AD4020] },
+ { "ad4021", (kernel_ulong_t)&ad4000_chips[ID_AD4021] },
+ { "ad4022", (kernel_ulong_t)&ad4000_chips[ID_AD4022] },
+ { "adaq4001", (kernel_ulong_t)&ad4000_chips[ID_ADAQ4001] },
+ { "adaq4003", (kernel_ulong_t)&ad4000_chips[ID_ADAQ4003] },
+ { }
+};
+MODULE_DEVICE_TABLE(spi, ad4000_id);
+
+static const struct of_device_id ad4000_of_match[] = {
+ { .compatible = "adi,ad4000", .data = &ad4000_chips[ID_AD4000] },
+ { .compatible = "adi,ad4001", .data = &ad4000_chips[ID_AD4001] },
+ { .compatible = "adi,ad4002", .data = &ad4000_chips[ID_AD4002] },
+ { .compatible = "adi,ad4003", .data = &ad4000_chips[ID_AD4003] },
+ { .compatible = "adi,ad4004", .data = &ad4000_chips[ID_AD4004] },
+ { .compatible = "adi,ad4005", .data = &ad4000_chips[ID_AD4005] },
+ { .compatible = "adi,ad4006", .data = &ad4000_chips[ID_AD4006] },
+ { .compatible = "adi,ad4007", .data = &ad4000_chips[ID_AD4007] },
+ { .compatible = "adi,ad4008", .data = &ad4000_chips[ID_AD4008] },
+ { .compatible = "adi,ad4010", .data = &ad4000_chips[ID_AD4010] },
+ { .compatible = "adi,ad4011", .data = &ad4000_chips[ID_AD4011] },
+ { .compatible = "adi,ad4020", .data = &ad4000_chips[ID_AD4020] },
+ { .compatible = "adi,ad4021", .data = &ad4000_chips[ID_AD4021] },
+ { .compatible = "adi,ad4022", .data = &ad4000_chips[ID_AD4022] },
+ { .compatible = "adi,adaq4001", .data = &ad4000_chips[ID_ADAQ4001] },
+ { .compatible = "adi,adaq4003", .data = &ad4000_chips[ID_ADAQ4003] },
+ { }
+};
+MODULE_DEVICE_TABLE(of, ad4000_of_match);
+
+static struct spi_driver ad4000_driver = {
+ .driver = {
+ .name = "ad4000",
+ .of_match_table = ad4000_of_match,
+ },
+ .probe = ad4000_probe,
+ .id_table = ad4000_id,
+};
+module_spi_driver(ad4000_driver);
+
+MODULE_AUTHOR("Mircea Caprioru <mircea.caprioru@analog.com>");
+MODULE_AUTHOR("Marcelo Schmitt <marcelo.schmitt@analog.com>");
+MODULE_DESCRIPTION("Analog Devices AD4000 ADC driver");
+MODULE_LICENSE("GPL");
--
2.43.0
^ permalink raw reply related
* [PATCH v2 1/2] dt-bindings: iio: adc: Add AD4000
From: Marcelo Schmitt @ 2024-04-08 14:31 UTC (permalink / raw)
To: lars, Michael.Hennerich, jic23, robh+dt, krzysztof.kozlowski+dt,
conor+dt, marcelo.schmitt1
Cc: linux-iio, devicetree, linux-kernel
In-Reply-To: <cover.1712585500.git.marcelo.schmitt@analog.com>
Add device tree documentation for AD4000 family of ADC devices.
Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4000-4004-4008.pdf
Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4001-4005.pdf
Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4002-4006-4010.pdf
Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4003-4007-4011.pdf
Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4020-4021-4022.pdf
Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4001.pdf
Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4003.pdf
Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
---
.../bindings/iio/adc/adi,ad4000.yaml | 201 ++++++++++++++++++
MAINTAINERS | 7 +
2 files changed, 208 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
new file mode 100644
index 000000000000..ca06afb5149e
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
@@ -0,0 +1,201 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/adi,ad4000.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices AD4000 and similar Analog to Digital Converters
+
+maintainers:
+ - Marcelo Schmitt <marcelo.schmitt@analog.com>
+
+description: |
+ Analog Devices AD4000 family of Analog to Digital Converters with SPI support.
+ Specifications can be found at:
+ https://www.analog.com/media/en/technical-documentation/data-sheets/ad4000-4004-4008.pdf
+ https://www.analog.com/media/en/technical-documentation/data-sheets/ad4001-4005.pdf
+ https://www.analog.com/media/en/technical-documentation/data-sheets/ad4002-4006-4010.pdf
+ https://www.analog.com/media/en/technical-documentation/data-sheets/ad4003-4007-4011.pdf
+ https://www.analog.com/media/en/technical-documentation/data-sheets/ad4020-4021-4022.pdf
+ https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4001.pdf
+ https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4003.pdf
+
+$ref: /schemas/spi/spi-peripheral-props.yaml#
+
+properties:
+ compatible:
+ enum:
+ - adi,ad4000
+ - adi,ad4001
+ - adi,ad4002
+ - adi,ad4003
+ - adi,ad4004
+ - adi,ad4005
+ - adi,ad4006
+ - adi,ad4007
+ - adi,ad4008
+ - adi,ad4010
+ - adi,ad4011
+ - adi,ad4020
+ - adi,ad4021
+ - adi,ad4022
+ - adi,adaq4001
+ - adi,adaq4003
+
+ reg:
+ maxItems: 1
+
+ spi-max-frequency:
+ maximum: 102040816 # for VIO > 2.7 V, 81300813 for VIO > 1.7 V
+
+ spi-cpha: true
+
+ adi,spi-mode:
+ $ref: /schemas/types.yaml#/definitions/string
+ enum: [ single, chain ]
+ description: |
+ This property indicates the SPI wiring configuration.
+
+ When this property is omitted, it is assumed that the device is using what
+ the datasheet calls "4-wire mode". This is the conventional SPI mode used
+ when there are multiple devices on the same bus. In this mode, the CNV
+ line is used to initiate the conversion and the SDI line is connected to
+ CS on the SPI controller.
+
+ When this property is present, it indicates that the device is using one
+ of the following alternative wiring configurations:
+
+ * single: The datasheet calls this "3-wire mode". (NOTE: The datasheet's
+ definition of 3-wire mode is NOT at all related to the standard
+ spi-3wire property!) This mode is often used when the ADC is the only
+ device on the bus. In this mode, SDI is tied to VIO, and the CNV line
+ can be connected to the CS line of the SPI controller or to a GPIO, in
+ which case the CS line of the controller is unused.
+ * chain: The datasheet calls this "chain mode". This mode is used to save
+ on wiring when multiple ADCs are used. In this mode, the SDI line of
+ one chip is tied to the SDO of the next chip in the chain and the SDI of
+ the last chip in the chain is tied to GND. Only the first chip in the
+ chain is connected to the SPI bus. The CNV line of all chips are tied
+ together. The CS line of the SPI controller can be used as the CNV line
+ only if it is active high.
+
+ '#daisy-chained-devices': true
+
+ vdd-supply:
+ description: A 1.8V supply that powers the chip (VDD).
+
+ vio-supply:
+ description:
+ A 1.8V to 5.5V supply for the digital inputs and outputs (VIO).
+
+ ref-supply:
+ description:
+ A 2.5 to 5V supply for the external reference voltage (REF).
+
+ cnv-gpios:
+ description:
+ The Convert Input (CNV). This input has multiple functions. It initiates
+ the conversions and selects the SPI mode of the device (chain or CS). In
+ 'single' mode, this property is omitted if the CNV pin is connected to the
+ CS line of the SPI controller. If 'single' mode is selected and this GPIO
+ is provided, it must be active low.
+ maxItems: 1
+
+ adi,high-z-input:
+ type: boolean
+ description:
+ High-Z mode allows the amplifier and RC filter in front of the ADC to be
+ chosen based on the signal bandwidth of interest, rather than the settling
+ requirements of the switched capacitor SAR ADC inputs.
+
+ adi,gain-milli:
+ description: |
+ The hardware gain applied to the ADC input (in milli units).
+ The gain provided by the ADC input scaler is defined by the hardware
+ connections between chip pins OUT+, R1K-, R1K1-, R1K+, R1K1+, and OUT-.
+ If not present, default to 1000 (no actual gain applied).
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [454, 909, 1000, 1900]
+ default: 1000
+
+ interrupts:
+ description:
+ The SDO pin can also function as a busy indicator. This node should be
+ connected to an interrupt that is triggered when the SDO line goes low
+ while the SDI line is high and the CNV line is low ('single' mode) or the
+ SDI line is low and the CNV line is high ('multi' mode); or when the SDO
+ line goes high while the SDI and CNV lines are high (chain mode),
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - spi-cpha
+ - vdd-supply
+ - vio-supply
+ - ref-supply
+
+allOf:
+ # in '4-wire' mode, cnv-gpios is required, for other modes it is optional
+ - if:
+ not:
+ required:
+ - adi,spi-mode
+ then:
+ required:
+ - cnv-gpios
+ # chain mode has lower SCLK max rate
+ - if:
+ required:
+ - adi,spi-mode
+ properties:
+ adi,spi-mode:
+ const: chain
+ then:
+ properties:
+ spi-max-frequency:
+ maximum: 50000000 # for VIO > 2.7 V, 40000000 for VIO > 1.7 V
+ required:
+ - '#daisy-chained-devices'
+ else:
+ properties:
+ '#daisy-chained-devices': false
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ /* Example for a AD devices */
+ adc@0 {
+ compatible = "adi,ad4020";
+ reg = <0>;
+ spi-cpha;
+ spi-max-frequency = <71000000>;
+ vdd-supply = <&supply_1_8V>;
+ vio-supply = <&supply_1_8V>;
+ ref-supply = <&supply_5V>;
+ cnv-gpios = <&gpio0 88 GPIO_ACTIVE_HIGH>;
+ };
+ };
+ - |
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ /* Example for a ADAQ devices */
+ adc@0 {
+ compatible = "adi,adaq4003";
+ reg = <0>;
+ spi-cpha;
+ adi,spi-mode = "single";
+ spi-max-frequency = <80000000>;
+ vdd-supply = <&supply_1_8V>;
+ vio-supply = <&supply_1_8V>;
+ ref-supply = <&supply_5V>;
+ adi,high-z-input;
+ adi,gain-milli = <454>;
+ };
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index a7287cf44869..5dfe118a5dd3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1159,6 +1159,13 @@ W: https://ez.analog.com/linux-software-drivers
F: Documentation/devicetree/bindings/iio/dac/adi,ad3552r.yaml
F: drivers/iio/dac/ad3552r.c
+ANALOG DEVICES INC AD4000 DRIVER
+M: Marcelo Schmitt <marcelo.schmitt@analog.com>
+L: linux-iio@vger.kernel.org
+S: Supported
+W: https://ez.analog.com/linux-software-drivers
+F: Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
+
ANALOG DEVICES INC AD4130 DRIVER
M: Cosmin Tanislav <cosmin.tanislav@analog.com>
L: linux-iio@vger.kernel.org
--
2.43.0
^ permalink raw reply related
* [PATCH v2 0/2] Add support for AD4000 series
From: Marcelo Schmitt @ 2024-04-08 14:30 UTC (permalink / raw)
To: lars, Michael.Hennerich, jic23, robh+dt, krzysztof.kozlowski+dt,
conor+dt, marcelo.schmitt1
Cc: linux-iio, devicetree, linux-kernel
This is more like an RFC patch set since configuration read/write is currently
buggy.
Change log v1 -> v2:
- Took device tree provided by David.
- Dropped ABI additions in favor of device tree properties.
- Set differential IIO channel subtype for differential ADCs.
- Set scan_type shift bits to mask out correct real bits from buffer.
- Added __aligned(8) to buffer timestamp.
- Used union to reduce buffer memory usage for 16-bit devices.
- Used SPI transfer functions rather than SPI message.
- Used c99 style structure initialization.
- Used iio_device_claim_direct_scoped().
- Removed unneeded pointer casts.
- Added other power supplies (VDD and VIO).
Link to v1: https://lore.kernel.org/linux-iio/cover.1711131830.git.marcelo.schmitt@analog.com/
Additional topics:
- Why there is no different handling for the different SPI wiring modes?
It looks like there is no need for different handling of "4-wire" and "3-wire"
modes.
If in "4-wire" (dt default mode), SDI is connected to SPI controller CS and
CNV is active high. We can activate the CNV GPIO then let the SPI controller
bring CS (connected to SDI) down when starting the transfer.
If in "3-wire" (dt single mode), if we have a CNV (active low) GPIO we activate
it and then proceed with with the transfer. If controller CS is connected to
CNV it works the same way.
I'm thinking it's better if we can support these devices in similar way
other SPI ADCs are supported. Does that make sense?
To me, the "3-wire" mode with controller CS to ADC CNV is what most resembles
conventional SPI. The only important distinction is that the
controller must be able to keep ADC SDI line high during conversions.
Although, while the spi-engine implementation provided to me can keep SDI up
during conversions, I'm not sure its a thing all SPI controllers can do.
I tried a raspberry pi 4 some time ago and it was leaving the SDI line low if
no tx buffer was provided. Even with a tx full of 1s the controller would
bring SDI down between each 8 bits of transfer.
Anyway, single-shot and buffered reads work with the spi-engine controller
with ADC in "3-wire"/single mode with controller CS line connected to ADC CNV
pin which is how I've been testing it.
- Why did not make vref regulator optional?
Other SAR ADCs I've seen needed a voltage reference otherwise they simply
could not provide any reasonable readings. Isn't it preferable to fail rather
than having a device that can't provide reliable data?
- Why did not split into AD and ADAQ patches?
The main difference between AD and ADAQ is the amplifier in front of the ADC.
If only supporting AD, we could probably avoid the scale table since it would
only have two possible values per ADC. But then the handling of span compression
scale would need refactoring to be in the scale table when adding ADAQ.
I'm not excited to implement something knowing it will need rework in the
following patch. Will do if required.
- Span compression and offset.
For non-differential ADCs, enabling the span compression requires an input offset.
Link: https://www.analog.com/media/en/technical-documentation/data-sheets/AD4000-4004-4008.pdf
page 18
and
Link: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4002-4006-4010.pdf
page 19
I updated the _offset attribute for those ADCs according to span compression
being enabled or not. Is it okay to have an attribute update cause an update to
another one?
Maybe also make the span compression a dt property and have it fixed after probe?
- Configuration register
Despite it doing single-shot and buffered captures, read and writes to the
configuration register are currently buggy. It is as if the register was
"floating". I tried setting up buffers like ad7768-1, adxl355_core, bma220_spi,
bma400_core, and mcp3911.
Thanks,
Marcelo
Marcelo Schmitt (2):
dt-bindings: iio: adc: Add AD4000
iio: adc: Add support for AD4000
.../bindings/iio/adc/adi,ad4000.yaml | 201 ++++++
MAINTAINERS | 8 +
drivers/iio/adc/Kconfig | 12 +
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/ad4000.c | 649 ++++++++++++++++++
5 files changed, 871 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
create mode 100644 drivers/iio/adc/ad4000.c
--
2.43.0
^ permalink raw reply
* Re: [PATCH 3/4] arm64: dts: qcom: sc8280xp-lenovo-thinkpad-x13s: add USB-C orientation GPIOs
From: Konrad Dybcio @ 2024-04-08 14:26 UTC (permalink / raw)
To: Dmitry Baryshkov, Bjorn Andersson, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <20240408-hdk-orientation-gpios-v1-3-8064ba43e52a@linaro.org>
On 4/8/24 04:34, Dmitry Baryshkov wrote:
> Define the USB-C orientation GPIOs so that the USB-C ports orientation
> is known without having to resort to the altmode notifications.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
> arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
> index 15ae94c1602d..2806aa8ec497 100644
> --- a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
> +++ b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
> @@ -100,6 +100,8 @@ pmic-glink {
>
> #address-cells = <1>;
> #size-cells = <0>;
> + orientation-gpios = <&tlmm 166 GPIO_ACTIVE_HIGH>,
> + <&tlmm 49 GPIO_ACTIVE_HIGH>;
These numbers do check out
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Konrad
^ permalink raw reply
* Re: [PATCH v2 08/11] spi: cadence-qspi: add early busywait to cqspi_wait_for_bit()
From: Mark Brown @ 2024-04-08 14:16 UTC (permalink / raw)
To: Théo Lebrun
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Vaishnav Achath,
Thomas Bogendoerfer, Rob Herring, linux-spi, devicetree,
linux-kernel, linux-mips, Vladimir Kondratiev, Gregory CLEMENT,
Thomas Petazzoni, Tawfik Bayouk
In-Reply-To: <20240405-cdns-qspi-mbly-v2-8-956679866d6d@bootlin.com>
[-- Attachment #1: Type: text/plain, Size: 612 bytes --]
On Fri, Apr 05, 2024 at 05:02:18PM +0200, Théo Lebrun wrote:
> If the CQSPI_BUSYWAIT_EARLY quirk flag is on, call
> readl_relaxed_poll_timeout() with no sleep at the start of
> cqspi_wait_for_bit(). If its short timeout expires, a sleeping
> readl_relaxed_poll_timeout() call takes the relay.
>
> Behavior is hidden behind a quirk flag to keep the previous behavior the
> same on all platforms.
>
> The reason is to avoid hrtimer interrupts on the system. All read
> operations take less than 100µs.
Why would this be platform specific, this seems like a very standard
optimisation technique?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v2 02/11] spi: dt-bindings: cdns,qspi-nor: sort compatibles alphabetically
From: Mark Brown @ 2024-04-08 14:14 UTC (permalink / raw)
To: Théo Lebrun
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Vaishnav Achath,
Thomas Bogendoerfer, Rob Herring, linux-spi, devicetree,
linux-kernel, linux-mips, Vladimir Kondratiev, Gregory CLEMENT,
Thomas Petazzoni, Tawfik Bayouk
In-Reply-To: <20240405-cdns-qspi-mbly-v2-2-956679866d6d@bootlin.com>
[-- Attachment #1: Type: text/plain, Size: 1329 bytes --]
On Fri, Apr 05, 2024 at 05:02:12PM +0200, Théo Lebrun wrote:
> Compatibles are ordered by date of addition.
> Switch to (deterministic) alphabetical ordering.
>
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---
> Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml b/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
> index 5509c126b1cf..e53d443c6f93 100644
> --- a/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
> +++ b/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
> @@ -79,13 +79,13 @@ properties:
> - items:
> - enum:
> - amd,pensando-elba-qspi
> - - mobileye,eyeq5-ospi
> - - ti,k2g-qspi
> - - ti,am654-ospi
> - intel,lgm-qspi
> - - xlnx,versal-ospi-1.0
> - intel,socfpga-qspi
> + - mobileye,eyeq5-ospi
> - starfive,jh7110-qspi
> + - ti,am654-ospi
> + - ti,k2g-qspi
> + - xlnx,versal-ospi-1.0
In general it's better to sort trivial cleanup patches like this before
new functionality in order to avoid spurious dependencies.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v2 01/11] spi: dt-bindings: cdns,qspi-nor: add mobileye,eyeq5-ospi compatible
From: Mark Brown @ 2024-04-08 14:13 UTC (permalink / raw)
To: Théo Lebrun
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Vaishnav Achath,
Thomas Bogendoerfer, Rob Herring, linux-spi, devicetree,
linux-kernel, linux-mips, Vladimir Kondratiev, Gregory CLEMENT,
Thomas Petazzoni, Tawfik Bayouk, Krzysztof Kozlowski
In-Reply-To: <20240405-cdns-qspi-mbly-v2-1-956679866d6d@bootlin.com>
[-- Attachment #1: Type: text/plain, Size: 538 bytes --]
On Fri, Apr 05, 2024 at 05:02:11PM +0200, Théo Lebrun wrote:
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: mobileye,eyeq5-ospi
> + then:
> + properties:
> + cdns,fifo-depth: false
> + else:
> + required:
> + - cdns,fifo-depth
My suggestions on the FIFO depth probe patch would mean this would turn
into making cdns,fifo-depth optional for everything. It certainly seems
like many instances of the hardware should support that anyway.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH] dt-bindings: net: rockchip-dwmac: use rgmii-id in example
From: Dragan Simic @ 2024-04-08 14:13 UTC (permalink / raw)
To: Sascha Hauer
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner,
David Wu, netdev, devicetree, linux-arm-kernel, linux-rockchip,
linux-kernel, Andrew Lunn
In-Reply-To: <20240408-rockchip-dwmac-rgmii-id-binding-v1-1-3886d1a8bd54@pengutronix.de>
Hello Sascha,
On 2024-04-08 08:44, Sascha Hauer wrote:
> The dwmac supports specifying the RGMII clock delays, but it is
> recommended to use rgmii-id and to specify the delays in the phy node
> instead [1].
>
> Change the example accordingly to no longer promote this undesired
> setting.
>
> [1]
> https://lore.kernel.org/all/1a0de7b4-f0f7-4080-ae48-f5ffa9e76be3@lunn.ch/
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
I'd suggest that the following link is also added to the patch
description as a reference, because it contains rather useful
information:
https://lore.kernel.org/linux-rockchip/2973a2cb1f478031ae6d478c853c33ae@manjaro.org/
Otherwise,
Reviewed-by: Dragan Simic <dsimic@manjaro.org>
> ---
> Documentation/devicetree/bindings/net/rockchip-dwmac.yaml | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/rockchip-dwmac.yaml
> b/Documentation/devicetree/bindings/net/rockchip-dwmac.yaml
> index 70bbc4220e2ac..6bbe96e352509 100644
> --- a/Documentation/devicetree/bindings/net/rockchip-dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/rockchip-dwmac.yaml
> @@ -137,8 +137,6 @@ examples:
> assigned-clock-parents = <&ext_gmac>;
>
> rockchip,grf = <&grf>;
> - phy-mode = "rgmii";
> + phy-mode = "rgmii-id";
> clock_in_out = "input";
> - tx_delay = <0x30>;
> - rx_delay = <0x10>;
> };
>
> ---
> base-commit: 39cd87c4eb2b893354f3b850f916353f2658ae6f
> change-id: 20240408-rockchip-dwmac-rgmii-id-binding-6166af659845
>
> Best regards,
^ permalink raw reply
* Re: [PATCH v2 05/11] spi: cadence-qspi: add FIFO depth detection quirk
From: Mark Brown @ 2024-04-08 14:10 UTC (permalink / raw)
To: Théo Lebrun
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Vaishnav Achath,
Thomas Bogendoerfer, Rob Herring, linux-spi, devicetree,
linux-kernel, linux-mips, Vladimir Kondratiev, Gregory CLEMENT,
Thomas Petazzoni, Tawfik Bayouk
In-Reply-To: <20240405-cdns-qspi-mbly-v2-5-956679866d6d@bootlin.com>
[-- Attachment #1: Type: text/plain, Size: 2371 bytes --]
On Fri, Apr 05, 2024 at 05:02:15PM +0200, Théo Lebrun wrote:
> Use hardware ability to read the FIFO depth thanks to
> CQSPI_REG_SRAMPARTITION that is partially read-only. Keep current
> behavior identical for existing compatibles.
The behaviour is not identical here - we now unconditionally probe the
FIFO depth on all hardware, the difference with the quirk is that we
will ignore any DT property specifying the depth.
> - if (of_property_read_u32(np, "cdns,fifo-depth", &cqspi->fifo_depth)) {
> + if (!(ddata && ddata->quirks & CQSPI_DETECT_FIFO_DEPTH) &&
> + of_property_read_u32(np, "cdns,fifo-depth", &cqspi->fifo_depth)) {
> dev_err(dev, "couldn't determine fifo-depth\n");
It's not obvious from just the code that we do handle having a FIFO
depth property and detection in the detection code, at least a comment
would be good.
> +static void cqspi_controller_detect_fifo_depth(struct cqspi_st *cqspi)
> +{
> + const struct cqspi_driver_platdata *ddata = cqspi->ddata;
> + struct device *dev = &cqspi->pdev->dev;
> + u32 reg, fifo_depth;
> +
> + /*
> + * Bits N-1:0 are writable while bits 31:N are read as zero, with 2^N
> + * the FIFO depth.
> + */
> + writel(U32_MAX, cqspi->iobase + CQSPI_REG_SRAMPARTITION);
> + reg = readl(cqspi->iobase + CQSPI_REG_SRAMPARTITION);
> + fifo_depth = reg + 1;
> +
> + if (ddata && ddata->quirks & CQSPI_DETECT_FIFO_DEPTH) {
> + cqspi->fifo_depth = fifo_depth;
> + dev_dbg(dev, "using FIFO depth of %u\n", fifo_depth);
> + } else if (fifo_depth != cqspi->fifo_depth) {
> + dev_warn(dev, "detected FIFO depth (%u) different from config (%u)\n",
> + fifo_depth, cqspi->fifo_depth);
> + }
It's not obvious to me that we should ignore an explicitly specified
property if the quirk is present - if anything I'd more expect to see
the new warning in that case, possibly with a higher severity if we're
saying that the quirk means we're more confident that the data reported
by the hardware is reliable. I think what I'd expect is that we always
use an explicitly specified depth (hopefully the user was specifying it
for a reason?).
Pulling all the above together can we just drop the quirk and always do
the detection, or leave the quirk as just controlling the severity with
which we log any difference between detected and explicitly configured
depths?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH] dt-bindings: net: rockchip-dwmac: use rgmii-id in example
From: Andrew Lunn @ 2024-04-08 14:02 UTC (permalink / raw)
To: Sascha Hauer
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner,
David Wu, netdev, devicetree, linux-arm-kernel, linux-rockchip,
linux-kernel
In-Reply-To: <20240408-rockchip-dwmac-rgmii-id-binding-v1-1-3886d1a8bd54@pengutronix.de>
On Mon, Apr 08, 2024 at 08:44:10AM +0200, Sascha Hauer wrote:
> The dwmac supports specifying the RGMII clock delays, but it is
> recommended to use rgmii-id and to specify the delays in the phy node
> instead [1].
>
> Change the example accordingly to no longer promote this undesired
> setting.
>
> [1] https://lore.kernel.org/all/1a0de7b4-f0f7-4080-ae48-f5ffa9e76be3@lunn.ch/
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH 0/5] Add parsing for Zimop ISA extension
From: Andrew Jones @ 2024-04-08 13:56 UTC (permalink / raw)
To: Clément Léger
Cc: Deepak Gupta, Jonathan Corbet, Paul Walmsley, Palmer Dabbelt,
Albert Ou, Conor Dooley, Rob Herring, Krzysztof Kozlowski,
Anup Patel, Shuah Khan, Atish Patra, linux-doc, linux-riscv,
linux-kernel, devicetree, kvm, kvm-riscv, linux-kselftest
In-Reply-To: <89d4a24c-db24-487b-8c5c-bdc1fa2d42b4@rivosinc.com>
On Mon, Apr 08, 2024 at 01:19:39PM +0200, Clément Léger wrote:
>
>
> On 08/04/2024 13:03, Andrew Jones wrote:
> > On Mon, Apr 08, 2024 at 10:01:12AM +0200, Clément Léger wrote:
> >>
> >>
> >> On 05/04/2024 19:33, Deepak Gupta wrote:
> >>> On Fri, Apr 5, 2024 at 8:26 AM Andrew Jones <ajones@ventanamicro.com> wrote:
> >>>>
> >>>> On Thu, Apr 04, 2024 at 12:32:46PM +0200, Clément Léger wrote:
> >>>>> The Zimop ISA extension was ratified recently. This series adds support
> >>>>> for parsing it from riscv,isa, hwprobe export and kvm support for
> >>>>> Guest/VM.
> >>>>
> >>>> I'm not sure we need this. Zimop by itself isn't useful, so I don't know
> >>>> if we need to advertise it at all. When an extension comes along that
> >>>> redefines some MOPs, then we'll advertise that extension, but the fact
> >>>> Zimop is used for that extension is really just an implementation detail.
> >>>
> >>> Only situation I see this can be useful is this:--
> >>>
> >>> An implementer, implemented Zimops in CPU solely for the purpose that they can
> >>> run mainline distro & packages on their hardware and don't want to leverage any
> >>> feature which are built on top of Zimop.
> >>
> >> Yes, the rationale was that some binaries using extensions that overload
> >> MOPs could still be run. With Zimop exposed, the loader could determine
> >> if the binary can be executed without potentially crashing. We could
> >> also let the program run anyway but the execution could potentially
> >> crash unexpectedly, which IMHO is not really good for the user
> >> experience nor for debugging. I already think that the segfaults which
> >> happens when executing binaries that need some missing extension are not
> >> so easy to debug, so better add more guards.
> >
> > OK. It's only one more extension out of dozens, so I won't complain more,
>
> No worries, your point *is* valid since I'm not sure yet that the loader
> will actually do that one day.
>
> BTW, are you aware of any effort to make the elf dynamic loader
> "smarter" and actually check for needed extensions to be present rather
> than blindly running the elf and potentially catching SIGILL ?
Jeff Law told me a bit about FMV (function multiversioning). I don't know
much about this, but, from what he's told me, it sounds like there will be
an ifunc resolver which invokes hwprobe to determine which variants are
possible/best to use, so it should be possible to avoid SIGILL by always
having a basic variant.
Thanks,
drew
>
> Thanks,
>
> Clément
>
> > but I was thinking that binaries that use particular extensions would
> > check for those particular extensions (step 2), rather than Zimop.
> >
> > Thanks,
> > drew
> >
> >>
> >>>
> >>> As an example zicfilp and zicfiss are dependent on zimops. glibc can
> >>> do following
> >>>
> >>> 1) check elf header if binary was compiled with zicfiss and zicfilp,
> >>> if yes goto step 2, else goto step 6.
> >>> 2) check if zicfiss/zicfilp is available in hw via hwprobe, if yes
> >>> goto step 5. else goto step 3
> >>> 3) check if zimop is available via hwprobe, if yes goto step 6, else goto step 4
> >>
> >> I think you meant step 5 rather than step 6.
> >>
> >> Clément
> >>
> >>> 4) This binary won't be able to run successfully on this platform,
> >>> issue exit syscall. <-- termination
> >>> 5) issue prctl to enable shadow stack and landing pad for current task
> >>> <-- enable feature
> >>> 6) let the binary run <-- let the binary run because no harm can be done
^ permalink raw reply
* Re: [PATCH net-next v2 0/9] Add support for OPEN Alliance 10BASE-T1x MACPHY Serial Interface
From: Parthiban.Veerasooran @ 2024-04-08 13:41 UTC (permalink / raw)
To: benjamin
Cc: netdev, devicetree, linux-kernel, linux-doc, Horatiu.Vultur,
Woojung.Huh, Nicolas.Ferre, UNGLinuxDriver, Thorsten.Kummermehr,
davem, edumazet, kuba, pabeni, robh+dt, krzysztof.kozlowski+dt,
conor+dt, corbet, Steen.Hegelund, rdunlap, horms, casper.casan,
andrew
In-Reply-To: <d495577d97a5e7aa809d3e9a90ef6057404782f1.camel@bigler.one>
Hi Benjamin,
On 04/04/24 3:10 am, Benjamin Bigler wrote:
> [Some people who received this message don't often get email from benjamin@bigler.one. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Hi Parthiban,
>
> Sorry for the late answer, I was quite busy the last few days.
No problem.
>
> On Mon, 2024-03-25 at 13:24 +0000, Parthiban.Veerasooran@microchip.com wrote:
>> Hi Benjamin Bigler,
>>
>> Thank you for your testing and feedback. It would be really helpful to
>> bring the driver to a good shape. We really appreciate your efforts on this.
>>
>> On 24/03/24 5:25 pm, Benjamin Bigler wrote:
>>> [Some people who received this message don't often get email from benjamin@bigler.one. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>>>
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> Hi Parthiban
>>>
>>> I hope I send this in the right context as it is not related to just one patch or
>>> some specific code.
>>>
>>> I conducted UDP load testing using three i.MX8MM boards in conjunction with the
>>> LAN8651. The setup involved one board functioning as a server, which is just
>>> echoing back received data, while the remaining two boards acted as clients,
>>> sending UDP packets of different sizes in various bursts to the server.
>>> Due to hardware constraints, the SPI bus speed was limited to 15 MHz, which might
>>> have influenced the results.
>>>
>>> During the tests I experienced some issues:
>>>
>>> - The boards just start receiving after first sending something (ping another board).
>>> Some measurements showed that the irq stays asserted after init. This makes sense
>>> as far as I understand the chapter 7.7 of the specification, the irq is deasserted
>>> on reception of the first data header following CSn being asserted. As a workaround
>>> I trigger the thread at the end of oa_tc6_init.
>> It looks like the IRQ is asserted on RESET completion and expects a data
>> chunk from host to deassert the IRQ. I used to test the driver in RPI 4
>> using iperf3. For some reason I never faced this issue, may be when the
>> network device is being registered there might be some packet
>> transmission which leads to deliver a data chunk so that the IRQ is
>> deasserted. Thanks for the workaround. I think that would be the
>> solution to solve this issue. Adding the below lines in the end of the
>> function oa_tc6_init() will trigger the oa_tc6_spi_thread_handler() to
>> perform an empty data chunk transfer which will deassert the IRQ before
>> starting the actual data transfer.
>
> I have ipv6 disabled and use static ipv4 addresses. That could be the reason why on
> my side no packet is sent.
>
>>
>> /* oa_tc6_sw_reset_macphy() function resets and clears the MAC-PHY reset
>> * complete status. IRQ is also asserted on reset completion and it is
>> * remain asserted until MAC-PHY receives a data chunk. So performing an
>> * empty data chunk transmission will deassert the IRQ. Refer section
>> * 7.7 and 9.2.8.8 in the OPEN Alliance specification for more details.
>> */
>> tc6->int_flag = true;
>> wake_up_interruptible(&tc6->spi_wq);
>
> Perfect, thats the same I added and also works on my side.
>
>>>
>>> - If there is a lot of traffic, the receive buffer overflow error spams the log.
>>>
>>> - If there is a lot of traffic, I got various kernel panics in oa_tc6_update_rx_skb.
>>> Mostly because more data to rx_skb is added than allocated and sometimes because
>>> rx_skb is null in oa_tc6_update_rx_skb or oa_tc6_prcs_rx_frame_end. Some debugging
>>> with a logic analyzer showed that the chip is not behave correctly. There is more
>>> bytes between start_valid and end_valid than there should be. Also there
>>> seems to be 2 end_valid without a start_valid between. What is common is that the incorrect
>>> frame starts in a chunk where end_valid and start_valid is set.
>>> In my opinion its a problem in the chip (maybe related to the errata in the next point)
>>> but the driver should be resilent and just drop the packet and not cause a kernel panic.
>> Usually I run into this issue "receive buffer overflow" when I run RPI 4
>> with default cpu governor setting which is "ondemand". In this case,
>> even though if I set SPI clock speed as 15 MHz the RPI 4 core clock is
>> clocking down when it is idle which leads delivering half of the
>> configured SPI clock speed around 5.9 MHz. So the systems like RPI 4
>> need performance mode enabled to get the proper clock speed for SPI.
>> Refer below link for more details.
>>
>> https://github.com/raspberrypi/linux/issues/3381#issuecomment-1144723750
>>
>> I used to enable performance mode using the below command.
>>
>> echo performance | sudo tee
>> /sys/devices/system/cpu/cpu0/cpufreq/scaling_governor > /dev/null
>>
>> So please ensure the SPI clock speed using a logic analyzer to get the
>> maximum throughput without receive buffer overflow.
>>
>> Of course, I agree that the driver should not crash in case of receive
>> buffer overflow. By referring your investigations, I understand that the
>> buffers in the MAC-PHY is being continuously overwritten again and again
>> as the host is very slow to read the data from the MAC-PHY buffers
>> through SPI which alters the descriptors. There might be two reasons why
>> we run into this situation.
>> 1. The host is busy doing something else and delays to initiate SPI even
>> though SPI clock speed is 15 MHz.
>> 2. The SPI clock speed is less than 15 MHz.
>
> Sorry there is a missunderstanding between us. The receive buffer overflow is not
> causing any harm except filling the log. In my setup I get in one day about 35000
> entries. I am not sure if its appropriate to log these errors.
>
> The SPI Frequency is at 14.8 MHz. If I just have 2 boards connected, I am not able
> to reproduce this. Only with 3 boards when 2 boards sends multiple big ethernet
> frames (1512 byte per Frame) to one, I get these log entries.
> The latency seems to be quite low, from IRQ to start reading first frame it takes
> always less than 500us. Also the boards are just running the udp test.
>
>>
>> I use the below iperf3 setup for my testing and never faced the driver
>> crash issue even though faced "receive buffer overflow" error when I run
>> RPI 4 with "ondemand" default mode.
>>
>> Node 0 - Raspberry Pi 4 with LAN8650 MAC-PHY
>> $ iperf3 -s
>> Node 1 - Raspberry Pi 4 with EVB-LAN8670-USB USB Stick
>> $ iperf3 -c 192.168.5.100 -u -b 10M -i 1 -t 0
>>
>> and vice versa.
>>
>> I never faced "receive buffer overflow" error when I run RPI 4 with
>> "performance" mode enabled and even though all the cores are stressed
>> using the below command,
>>
>> $ yes >/dev/null & yes >/dev/null & yes >/dev/null & yes >/dev/null &
>>
>> Can you share more details about your testing setup and applications you
>> use, so that I will try to reproduce the issue in my setup to debug the
>> driver?
>
> I use a internal tool which does some stress tests using udp. Unfortunately,
> I am not allowed to publish it, but a colleague works on a rust implementation,
> which we can publish, but its not fully ready yet.
> On one board the tool is running in server mode. It just echoes back the received
> data. On the 2 other boards the tool is running in client mode. It sends various
> sized udp-packets in different bursts and then checks if it receives the same
> data in the same order.
>
>
> The crashes only happens when ZARFE is not set (with Rev B0). When the crash
> happens, I see on the logic analyzer that there are more bytes than mtu + headers
> between the frame where start_valid is set and the frame where end_valid is set.
> Then this happens:
Thanks for all the above details. I will include this ZARFE fix in the
next version v4 which I am going to post soon.
>
> [ 437.155673] skbuff: skb_over_panic: text:ffff80007a8c2bd8 len:1600 put:64 head:ffff00000de28080
> data:ffff00000de280c0 tail:0x680 end:0x640 dev:eth1
> [ 437.168987] kernel BUG at net/core/skbuff.c:192!
> [ 437.173612] Internal error: Oops - BUG: 00000000f2000800 [#1] PREEMPT SMP
> [ 437.180407] Modules linked in: ppp_async crc_ccitt ppp_generic slhc lan865x oa_tc6 bec_infoo(O)
> tpm_tis_spi tpm_tis_core spi_imx imx_sdma
> [ 437.196016] CPU: 1 PID: 455 Comm: oa-tc6-spi-thre Tainted: G O 6.6.11-
> gce336e2c2bc3-dirty #1
> [ 437.205853] Hardware name: Toradex Verdin iMX8M Mini on FUMU (DT)
> [ 437.212820] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [ 437.219790] pc : skb_panic+0x58/0x5c
> [ 437.223376] lr : skb_panic+0x58/0x5c
> [ 437.226959] sp : ffff80008362bd90
> [ 437.230278] x29: ffff80008362bda0 x28: 0000000000000000 x27: ffff000001066878
> [ 437.237426] x26: 000000000000001e x25: 00000000000007f8 x24: ffff0000010cea80
> [ 437.244571] x23: 00000000f0f0f0f1 x22: 000000000000001f x21: 0000000000000000
> [ 437.251720] x20: ffff0000010ceaa8 x19: 000000003f20003f x18: ffffffffffffffff
> [ 437.258867] x17: ffff7ffffded9000 x16: ffff800080008000 x15: 073a0764076e0765
> [ 437.266015] x14: 0720073007380736 x13: ffff8000823d1f58 x12: 0000000000000534
> [ 437.273162] x11: 00000000000001bc x10: ffff800082429f58 x9 : ffff8000823d1f58
> [ 437.280310] x8 : 00000000ffffefff x7 : ffff800082429f58 x6 : 0000000000000000
> [ 437.287455] x5 : 000000000000bff4 x4 : 0000000000000000 x3 : 0000000000000000
> [ 437.294606] x2 : 0000000000000000 x1 : ffff000001223b00 x0 : 0000000000000087
> [ 437.301753] Call trace:
> [ 437.304203] skb_panic+0x58/0x5c
> [ 437.307436] skb_find_text+0x0/0xf0
> [ 437.310933] oa_tc6_spi_thread_handler+0x438/0x880 [oa_tc6]
> [ 437.316523] kthread+0x118/0x11c
> [ 437.319758] ret_from_fork+0x10/0x20
> [ 437.323343] Code: f90007e9 b940b908 f90003e8 97ca3c34 (d4210000)
> [ 437.329446] ---[ end trace 0000000000000000 ]---
>
>
> Sometimes there are 2 end_valid after eachother without a start_valid between.
> Then this happens:
>
> [ 469.737297] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000074
> [ 469.746137] Mem abort info:
> [ 469.748950] ESR = 0x0000000096000004
> [ 469.752709] EC = 0x25: DABT (current EL), IL = 32 bits
> [ 469.758036] SET = 0, FnV = 0
> [ 469.761098] EA = 0, S1PTW = 0
> [ 469.764252] FSC = 0x04: level 0 translation fault
> [ 469.769144] Data abort info:
> [ 469.772033] ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
> [ 469.777529] CM = 0, WnR = 0, TnD = 0, TagAccess = 0
> [ 469.782594] GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
> [ 469.787921] user pgtable: 4k pages, 48-bit VAs, pgdp=0000000043c32000
> [ 469.794377] [0000000000000074] pgd=0000000000000000, p4d=0000000000000000
> [ 469.801184] Internal error: Oops: 0000000096000004 [#1] PREEMPT SMP
> [ 469.807459] Modules linked in: ppp_async crc_ccitt ppp_generic slhc lan865x oa_tc6 bec_infoo(O)
> tpm_tis_spi tpm_tis_core spi_imx imx_sdma
> [ 469.823064] CPU: 2 PID: 456 Comm: oa-tc6-spi-thre Tainted: G O 6.6.11-
> g350ed394a6ca-dirty #1
> [ 469.832903] Hardware name: Toradex Verdin iMX8M Mini on FUMU (DT)
> [ 469.839871] pstate: 00000005 (nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [ 469.846841] pc : skb_put+0xc/0x6c
> [ 469.850169] lr : oa_tc6_spi_thread_handler+0x438/0x880 [oa_tc6]
> [ 469.856106] sp : ffff80008376bdb0
> [ 469.859424] x29: ffff80008376bdb0 x28: 0000000000000000 x27: ffff00000194c080
> [ 469.866573] x26: 0000000000000000 x25: 0000000000000000 x24: ffff000001095c80
> [ 469.873720] x23: 00000000f0f0f0f1 x22: 000000000000001f x21: 0000000000000000
> [ 469.880870] x20: ffff000001095ca8 x19: 000000003f20003f x18: 0000000000000000
> [ 469.888023] x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000000
> [ 469.895174] x14: 0000031acf8b86d8 x13: 0000000000000000 x12: 0000000000000000
> [ 469.902321] x11: 0000000000000002 x10: 0000000000000a60 x9 : ffff80008376b970
> [ 469.909467] x8 : ffff00007fb6e580 x7 : 000000000194b080 x6 : 0000000000000000
> [ 469.916616] x5 : 0000000000000000 x4 : 0000000000000000 x3 : 000000000000fc80
> [ 469.923765] x2 : 0000000000000001 x1 : 0000000000000040 x0 : 0000000000000000
> [ 469.930915] Call trace:
> [ 469.933365] skb_put+0xc/0x6c
> [ 469.936342] oa_tc6_spi_thread_handler+0x438/0x880 [oa_tc6]
> [ 469.941929] kthread+0x118/0x11c
> [ 469.945166] ret_from_fork+0x10/0x20
> [ 469.948752] Code: d65f03c0 d503233f a9bf7bfd 910003fd (b9407406)
> [ 469.954854] ---[ end trace 0000000000000000 ]---
>
>
> If interested I can try to get a recording with the logic analyzer and send it to you.
I don't think it is needed.
>
> By the way in the other answer you attached a screenshot of the logic analyzer and you
> have a very nice HLA for oa_tc6. Are they open-source or are there any plans to publish them?
It is already available in the Microchip's github page for public.
Checkout the below link for the same.
https://github.com/MicrochipTech/oa-tc6-saleae-extension
>
>>>
>>> - Sometimes the chip stops working. It always asserts the irq but there is no data (rca=0)
>>> and also exst is not active. I found out that there is an errata (DS80001075) point s3
>>> that explains this. I set the ZARFE bit in CONFIG0. This also fixes the point above.
>>> The driver now works since about 2.5 weeks with various load with just one loss of frame
>>> error where I had to reboot the system after about 4 days.
>> It is good to hear that the driver works fine with the above changes. As
>> mentioned in the errata, this continuous interrupt issue is a known
>> issue with LAN8651 Rev.B0. Switching to LAN8651 Rev.B1 will solve this
>> issue and no need of any workaround. Setting ZARFE bit in the CONFIG0
>> will solve the continuous interrupt issue but don't know how the above
>> "receive buffer overflow" issue also solved. I think it is a good idea
>> to test with LAN8651 Rev.B1 without setting ZARFE bit once. It would be
>> interesting to see the result. I am always using LAN8651 Rev.B1 for my
>> testing.
>
> Unfortunately I just have LAN8651 Rev. B0 Chips. Are you sure that the Rev B1 has the
> issue fixed? The errata here says that B1 is affected too:
> https://ww1.microchip.com/downloads/aemDocuments/documents/AIS/ProductDocuments/Errata/LAN8650-1-Errata-80001075.pdf
As per my knowledge it is fixed in the Rev.B1 but as you said errata
says the issue persists in both revisions. Let me check internally and
get back to you on this. But it is always recommended to use Rev.B1
rather Rev.B0. If possible, I would suggest to use the latest one.
Best regards,
Parthiban V
>
>>
>> I should be able to reproduce the "receive buffer overflow" issue and
>> consequently kernel crash in my setup with LAN8651 Rev.B1 so that I can
>> investigate the issue further. As I am not able to reproduce in my RPI
>> 4, I need your support for the tests and applications you used in your
>> setup.
>>>
>>> Is there a reason why you removed the netdev watchdog which was active in v2?
>> When the timeout occurs, there is no further action except increasing
>> tx_errors. Not seeing this except USB-to-Ethernet which can be removed
>> unexpectedly. But this is SPI interface which will not be removed
>> unexpectedly as it is a platform device. That's why we removed this.
>>
>> Best regards,
>> Parthiban V
>>>
>>> Thanks,
>>> Benjamin Bigler
>>>
>>
>
> Thanks,
> Benjamin Bigler
>
^ permalink raw reply
* [PATCH v20 9/9] usb: dwc3: qcom: Add multiport suspend/resume support for wrapper
From: Krishna Kurapati @ 2024-04-08 13:29 UTC (permalink / raw)
To: Krzysztof Kozlowski, Rob Herring, Bjorn Andersson, Wesley Cheng,
Konrad Dybcio, Greg Kroah-Hartman, Conor Dooley, Thinh Nguyen,
Felipe Balbi, Johan Hovold
Cc: devicetree, linux-arm-msm, linux-usb, linux-kernel, quic_ppratap,
quic_jackp, Krishna Kurapati, Johan Hovold
In-Reply-To: <20240408132925.1880571-1-quic_kriskura@quicinc.com>
Power event IRQ is used for wakeup in cases:
a) where the controller is super speed capable and missing an
ss_phy interrupt.
b) where the GIC is not capable of detecting DP/DM hs phy irq's.
Power event IRQ stat register indicates whether high speed phy
entered and exited L2 successfully during suspend and resume.
Indicate the same for all ports of multiport.
Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>
Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
---
drivers/usb/dwc3/dwc3-qcom.c | 22 +++++++++++++++++-----
1 file changed, 17 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
index 12182e0f8f45..d3e6d5d5e8bf 100644
--- a/drivers/usb/dwc3/dwc3-qcom.c
+++ b/drivers/usb/dwc3/dwc3-qcom.c
@@ -52,6 +52,13 @@
#define APPS_USB_AVG_BW 0
#define APPS_USB_PEAK_BW MBps_to_icc(40)
+static const u32 pwr_evnt_irq_stat_reg[DWC3_MAX_PORTS] = {
+ 0x58,
+ 0x1dc,
+ 0x228,
+ 0x238,
+};
+
struct dwc3_qcom_port {
int qusb2_phy_irq;
int dp_hs_phy_irq;
@@ -421,9 +428,11 @@ static int dwc3_qcom_suspend(struct dwc3_qcom *qcom, bool wakeup)
if (qcom->is_suspended)
return 0;
- val = readl(qcom->qscratch_base + PWR_EVNT_IRQ_STAT_REG);
- if (!(val & PWR_EVNT_LPM_IN_L2_MASK))
- dev_err(qcom->dev, "HS-PHY not in L2\n");
+ for (i = 0; i < qcom->num_ports; i++) {
+ val = readl(qcom->qscratch_base + pwr_evnt_irq_stat_reg[i]);
+ if (!(val & PWR_EVNT_LPM_IN_L2_MASK))
+ dev_err(qcom->dev, "port-%d HS-PHY not in L2\n", i + 1);
+ }
for (i = qcom->num_clocks - 1; i >= 0; i--)
clk_disable_unprepare(qcom->clks[i]);
@@ -472,8 +481,11 @@ static int dwc3_qcom_resume(struct dwc3_qcom *qcom, bool wakeup)
dev_warn(qcom->dev, "failed to enable interconnect: %d\n", ret);
/* Clear existing events from PHY related to L2 in/out */
- dwc3_qcom_setbits(qcom->qscratch_base, PWR_EVNT_IRQ_STAT_REG,
- PWR_EVNT_LPM_IN_L2_MASK | PWR_EVNT_LPM_OUT_L2_MASK);
+ for (i = 0; i < qcom->num_ports; i++) {
+ dwc3_qcom_setbits(qcom->qscratch_base,
+ pwr_evnt_irq_stat_reg[i],
+ PWR_EVNT_LPM_IN_L2_MASK | PWR_EVNT_LPM_OUT_L2_MASK);
+ }
qcom->is_suspended = false;
--
2.34.1
^ permalink raw reply related
* [PATCH v20 8/9] usb: dwc3: qcom: Enable wakeup for applicable ports of multiport
From: Krishna Kurapati @ 2024-04-08 13:29 UTC (permalink / raw)
To: Krzysztof Kozlowski, Rob Herring, Bjorn Andersson, Wesley Cheng,
Konrad Dybcio, Greg Kroah-Hartman, Conor Dooley, Thinh Nguyen,
Felipe Balbi, Johan Hovold
Cc: devicetree, linux-arm-msm, linux-usb, linux-kernel, quic_ppratap,
quic_jackp, Krishna Kurapati, Johan Hovold
In-Reply-To: <20240408132925.1880571-1-quic_kriskura@quicinc.com>
DWC3 Qcom wrapper currently supports only wakeup configuration
for single port controllers. Read speed of each port connected
to the controller and enable wakeup for each of them accordingly.
Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>
Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
---
drivers/usb/dwc3/dwc3-qcom.c | 71 +++++++++++++++++++++---------------
1 file changed, 41 insertions(+), 30 deletions(-)
diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
index 35eb338514bc..12182e0f8f45 100644
--- a/drivers/usb/dwc3/dwc3-qcom.c
+++ b/drivers/usb/dwc3/dwc3-qcom.c
@@ -57,6 +57,7 @@ struct dwc3_qcom_port {
int dp_hs_phy_irq;
int dm_hs_phy_irq;
int ss_phy_irq;
+ enum usb_device_speed usb2_speed;
};
struct dwc3_qcom {
@@ -68,7 +69,6 @@ struct dwc3_qcom {
struct reset_control *resets;
struct dwc3_qcom_port ports[DWC3_MAX_PORTS];
u8 num_ports;
- enum usb_device_speed usb2_speed;
struct extcon_dev *edev;
struct extcon_dev *host_edev;
@@ -307,7 +307,7 @@ static bool dwc3_qcom_is_host(struct dwc3_qcom *qcom)
return dwc->xhci;
}
-static enum usb_device_speed dwc3_qcom_read_usb2_speed(struct dwc3_qcom *qcom)
+static enum usb_device_speed dwc3_qcom_read_usb2_speed(struct dwc3_qcom *qcom, int port_index)
{
struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3);
struct usb_device *udev;
@@ -318,14 +318,8 @@ static enum usb_device_speed dwc3_qcom_read_usb2_speed(struct dwc3_qcom *qcom)
*/
hcd = platform_get_drvdata(dwc->xhci);
- /*
- * It is possible to query the speed of all children of
- * USB2.0 root hub via usb_hub_for_each_child(). DWC3 code
- * currently supports only 1 port per controller. So
- * this is sufficient.
- */
#ifdef CONFIG_USB
- udev = usb_hub_find_child(hcd->self.root_hub, 1);
+ udev = usb_hub_find_child(hcd->self.root_hub, port_index + 1);
#else
udev = NULL;
#endif
@@ -356,26 +350,26 @@ static void dwc3_qcom_disable_wakeup_irq(int irq)
disable_irq_nosync(irq);
}
-static void dwc3_qcom_disable_interrupts(struct dwc3_qcom *qcom)
+static void dwc3_qcom_disable_port_interrupts(struct dwc3_qcom_port *port)
{
- dwc3_qcom_disable_wakeup_irq(qcom->ports[0].qusb2_phy_irq);
+ dwc3_qcom_disable_wakeup_irq(port->qusb2_phy_irq);
- if (qcom->usb2_speed == USB_SPEED_LOW) {
- dwc3_qcom_disable_wakeup_irq(qcom->ports[0].dm_hs_phy_irq);
- } else if ((qcom->usb2_speed == USB_SPEED_HIGH) ||
- (qcom->usb2_speed == USB_SPEED_FULL)) {
- dwc3_qcom_disable_wakeup_irq(qcom->ports[0].dp_hs_phy_irq);
+ if (port->usb2_speed == USB_SPEED_LOW) {
+ dwc3_qcom_disable_wakeup_irq(port->dm_hs_phy_irq);
+ } else if ((port->usb2_speed == USB_SPEED_HIGH) ||
+ (port->usb2_speed == USB_SPEED_FULL)) {
+ dwc3_qcom_disable_wakeup_irq(port->dp_hs_phy_irq);
} else {
- dwc3_qcom_disable_wakeup_irq(qcom->ports[0].dp_hs_phy_irq);
- dwc3_qcom_disable_wakeup_irq(qcom->ports[0].dm_hs_phy_irq);
+ dwc3_qcom_disable_wakeup_irq(port->dp_hs_phy_irq);
+ dwc3_qcom_disable_wakeup_irq(port->dm_hs_phy_irq);
}
- dwc3_qcom_disable_wakeup_irq(qcom->ports[0].ss_phy_irq);
+ dwc3_qcom_disable_wakeup_irq(port->ss_phy_irq);
}
-static void dwc3_qcom_enable_interrupts(struct dwc3_qcom *qcom)
+static void dwc3_qcom_enable_port_interrupts(struct dwc3_qcom_port *port)
{
- dwc3_qcom_enable_wakeup_irq(qcom->ports[0].qusb2_phy_irq, 0);
+ dwc3_qcom_enable_wakeup_irq(port->qusb2_phy_irq, 0);
/*
* Configure DP/DM line interrupts based on the USB2 device attached to
@@ -386,21 +380,37 @@ static void dwc3_qcom_enable_interrupts(struct dwc3_qcom *qcom)
* DP and DM lines as rising edge to detect HS/HS/LS device connect scenario.
*/
- if (qcom->usb2_speed == USB_SPEED_LOW) {
- dwc3_qcom_enable_wakeup_irq(qcom->ports[0].dm_hs_phy_irq,
+ if (port->usb2_speed == USB_SPEED_LOW) {
+ dwc3_qcom_enable_wakeup_irq(port->dm_hs_phy_irq,
IRQ_TYPE_EDGE_FALLING);
- } else if ((qcom->usb2_speed == USB_SPEED_HIGH) ||
- (qcom->usb2_speed == USB_SPEED_FULL)) {
- dwc3_qcom_enable_wakeup_irq(qcom->ports[0].dp_hs_phy_irq,
+ } else if ((port->usb2_speed == USB_SPEED_HIGH) ||
+ (port->usb2_speed == USB_SPEED_FULL)) {
+ dwc3_qcom_enable_wakeup_irq(port->dp_hs_phy_irq,
IRQ_TYPE_EDGE_FALLING);
} else {
- dwc3_qcom_enable_wakeup_irq(qcom->ports[0].dp_hs_phy_irq,
+ dwc3_qcom_enable_wakeup_irq(port->dp_hs_phy_irq,
IRQ_TYPE_EDGE_RISING);
- dwc3_qcom_enable_wakeup_irq(qcom->ports[0].dm_hs_phy_irq,
+ dwc3_qcom_enable_wakeup_irq(port->dm_hs_phy_irq,
IRQ_TYPE_EDGE_RISING);
}
- dwc3_qcom_enable_wakeup_irq(qcom->ports[0].ss_phy_irq, 0);
+ dwc3_qcom_enable_wakeup_irq(port->ss_phy_irq, 0);
+}
+
+static void dwc3_qcom_disable_interrupts(struct dwc3_qcom *qcom)
+{
+ int i;
+
+ for (i = 0; i < qcom->num_ports; i++)
+ dwc3_qcom_disable_port_interrupts(&qcom->ports[i]);
+}
+
+static void dwc3_qcom_enable_interrupts(struct dwc3_qcom *qcom)
+{
+ int i;
+
+ for (i = 0; i < qcom->num_ports; i++)
+ dwc3_qcom_enable_port_interrupts(&qcom->ports[i]);
}
static int dwc3_qcom_suspend(struct dwc3_qcom *qcom, bool wakeup)
@@ -427,7 +437,8 @@ static int dwc3_qcom_suspend(struct dwc3_qcom *qcom, bool wakeup)
* freezable workqueue.
*/
if (dwc3_qcom_is_host(qcom) && wakeup) {
- qcom->usb2_speed = dwc3_qcom_read_usb2_speed(qcom);
+ for (i = 0; i < qcom->num_ports; i++)
+ qcom->ports[i].usb2_speed = dwc3_qcom_read_usb2_speed(qcom, i);
dwc3_qcom_enable_interrupts(qcom);
}
--
2.34.1
^ permalink raw reply related
* [PATCH v20 7/9] usb: dwc3: qcom: Refactor IRQ handling in glue driver
From: Krishna Kurapati @ 2024-04-08 13:29 UTC (permalink / raw)
To: Krzysztof Kozlowski, Rob Herring, Bjorn Andersson, Wesley Cheng,
Konrad Dybcio, Greg Kroah-Hartman, Conor Dooley, Thinh Nguyen,
Felipe Balbi, Johan Hovold
Cc: devicetree, linux-arm-msm, linux-usb, linux-kernel, quic_ppratap,
quic_jackp, Krishna Kurapati, Johan Hovold
In-Reply-To: <20240408132925.1880571-1-quic_kriskura@quicinc.com>
On multiport supported controllers, each port has its own DP/DM
and SS (if super speed capable) interrupts. As per the bindings,
their interrupt names differ from standard ones having "_x" added
as suffix (x indicates port number). Identify from the interrupt
names whether the controller is a multiport controller or not.
Refactor dwc3_qcom_setup_irq() call to parse multiport interrupts
along with non-multiport ones accordingly..
Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>
Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
---
drivers/usb/dwc3/dwc3-qcom.c | 137 ++++++++++++++++++++++++++---------
1 file changed, 103 insertions(+), 34 deletions(-)
diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
index cae5dab8fcfc..35eb338514bc 100644
--- a/drivers/usb/dwc3/dwc3-qcom.c
+++ b/drivers/usb/dwc3/dwc3-qcom.c
@@ -52,6 +52,13 @@
#define APPS_USB_AVG_BW 0
#define APPS_USB_PEAK_BW MBps_to_icc(40)
+struct dwc3_qcom_port {
+ int qusb2_phy_irq;
+ int dp_hs_phy_irq;
+ int dm_hs_phy_irq;
+ int ss_phy_irq;
+};
+
struct dwc3_qcom {
struct device *dev;
void __iomem *qscratch_base;
@@ -59,11 +66,8 @@ struct dwc3_qcom {
struct clk **clks;
int num_clocks;
struct reset_control *resets;
-
- int qusb2_phy_irq;
- int dp_hs_phy_irq;
- int dm_hs_phy_irq;
- int ss_phy_irq;
+ struct dwc3_qcom_port ports[DWC3_MAX_PORTS];
+ u8 num_ports;
enum usb_device_speed usb2_speed;
struct extcon_dev *edev;
@@ -354,24 +358,24 @@ static void dwc3_qcom_disable_wakeup_irq(int irq)
static void dwc3_qcom_disable_interrupts(struct dwc3_qcom *qcom)
{
- dwc3_qcom_disable_wakeup_irq(qcom->qusb2_phy_irq);
+ dwc3_qcom_disable_wakeup_irq(qcom->ports[0].qusb2_phy_irq);
if (qcom->usb2_speed == USB_SPEED_LOW) {
- dwc3_qcom_disable_wakeup_irq(qcom->dm_hs_phy_irq);
+ dwc3_qcom_disable_wakeup_irq(qcom->ports[0].dm_hs_phy_irq);
} else if ((qcom->usb2_speed == USB_SPEED_HIGH) ||
(qcom->usb2_speed == USB_SPEED_FULL)) {
- dwc3_qcom_disable_wakeup_irq(qcom->dp_hs_phy_irq);
+ dwc3_qcom_disable_wakeup_irq(qcom->ports[0].dp_hs_phy_irq);
} else {
- dwc3_qcom_disable_wakeup_irq(qcom->dp_hs_phy_irq);
- dwc3_qcom_disable_wakeup_irq(qcom->dm_hs_phy_irq);
+ dwc3_qcom_disable_wakeup_irq(qcom->ports[0].dp_hs_phy_irq);
+ dwc3_qcom_disable_wakeup_irq(qcom->ports[0].dm_hs_phy_irq);
}
- dwc3_qcom_disable_wakeup_irq(qcom->ss_phy_irq);
+ dwc3_qcom_disable_wakeup_irq(qcom->ports[0].ss_phy_irq);
}
static void dwc3_qcom_enable_interrupts(struct dwc3_qcom *qcom)
{
- dwc3_qcom_enable_wakeup_irq(qcom->qusb2_phy_irq, 0);
+ dwc3_qcom_enable_wakeup_irq(qcom->ports[0].qusb2_phy_irq, 0);
/*
* Configure DP/DM line interrupts based on the USB2 device attached to
@@ -383,20 +387,20 @@ static void dwc3_qcom_enable_interrupts(struct dwc3_qcom *qcom)
*/
if (qcom->usb2_speed == USB_SPEED_LOW) {
- dwc3_qcom_enable_wakeup_irq(qcom->dm_hs_phy_irq,
- IRQ_TYPE_EDGE_FALLING);
+ dwc3_qcom_enable_wakeup_irq(qcom->ports[0].dm_hs_phy_irq,
+ IRQ_TYPE_EDGE_FALLING);
} else if ((qcom->usb2_speed == USB_SPEED_HIGH) ||
(qcom->usb2_speed == USB_SPEED_FULL)) {
- dwc3_qcom_enable_wakeup_irq(qcom->dp_hs_phy_irq,
- IRQ_TYPE_EDGE_FALLING);
+ dwc3_qcom_enable_wakeup_irq(qcom->ports[0].dp_hs_phy_irq,
+ IRQ_TYPE_EDGE_FALLING);
} else {
- dwc3_qcom_enable_wakeup_irq(qcom->dp_hs_phy_irq,
- IRQ_TYPE_EDGE_RISING);
- dwc3_qcom_enable_wakeup_irq(qcom->dm_hs_phy_irq,
- IRQ_TYPE_EDGE_RISING);
+ dwc3_qcom_enable_wakeup_irq(qcom->ports[0].dp_hs_phy_irq,
+ IRQ_TYPE_EDGE_RISING);
+ dwc3_qcom_enable_wakeup_irq(qcom->ports[0].dm_hs_phy_irq,
+ IRQ_TYPE_EDGE_RISING);
}
- dwc3_qcom_enable_wakeup_irq(qcom->ss_phy_irq, 0);
+ dwc3_qcom_enable_wakeup_irq(qcom->ports[0].ss_phy_irq, 0);
}
static int dwc3_qcom_suspend(struct dwc3_qcom *qcom, bool wakeup)
@@ -517,42 +521,107 @@ static int dwc3_qcom_request_irq(struct dwc3_qcom *qcom, int irq,
return ret;
}
-static int dwc3_qcom_setup_irq(struct platform_device *pdev)
+static int dwc3_qcom_setup_port_irq(struct platform_device *pdev, int port_index, bool is_multiport)
{
struct dwc3_qcom *qcom = platform_get_drvdata(pdev);
+ const char *irq_name;
int irq;
int ret;
- irq = platform_get_irq_byname_optional(pdev, "qusb2_phy");
+ if (is_multiport)
+ irq_name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "dp_hs_phy_%d", port_index + 1);
+ else
+ irq_name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "dp_hs_phy_irq");
+ if (!irq_name)
+ return -ENOMEM;
+
+ irq = platform_get_irq_byname_optional(pdev, irq_name);
if (irq > 0) {
- ret = dwc3_qcom_request_irq(qcom, irq, "qusb2_phy");
+ ret = dwc3_qcom_request_irq(qcom, irq, irq_name);
if (ret)
return ret;
- qcom->qusb2_phy_irq = irq;
+ qcom->ports[port_index].dp_hs_phy_irq = irq;
}
- irq = platform_get_irq_byname_optional(pdev, "dp_hs_phy_irq");
+ if (is_multiport)
+ irq_name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "dm_hs_phy_%d", port_index + 1);
+ else
+ irq_name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "dm_hs_phy_irq");
+ if (!irq_name)
+ return -ENOMEM;
+
+ irq = platform_get_irq_byname_optional(pdev, irq_name);
if (irq > 0) {
- ret = dwc3_qcom_request_irq(qcom, irq, "dp_hs_phy_irq");
+ ret = dwc3_qcom_request_irq(qcom, irq, irq_name);
if (ret)
return ret;
- qcom->dp_hs_phy_irq = irq;
+ qcom->ports[port_index].dm_hs_phy_irq = irq;
}
- irq = platform_get_irq_byname_optional(pdev, "dm_hs_phy_irq");
+ if (is_multiport)
+ irq_name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "ss_phy_%d", port_index + 1);
+ else
+ irq_name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "ss_phy_irq");
+ if (!irq_name)
+ return -ENOMEM;
+
+ irq = platform_get_irq_byname_optional(pdev, irq_name);
if (irq > 0) {
- ret = dwc3_qcom_request_irq(qcom, irq, "dm_hs_phy_irq");
+ ret = dwc3_qcom_request_irq(qcom, irq, irq_name);
if (ret)
return ret;
- qcom->dm_hs_phy_irq = irq;
+ qcom->ports[port_index].ss_phy_irq = irq;
}
- irq = platform_get_irq_byname_optional(pdev, "ss_phy_irq");
+ if (is_multiport)
+ return 0;
+
+ irq = platform_get_irq_byname_optional(pdev, "qusb2_phy");
if (irq > 0) {
- ret = dwc3_qcom_request_irq(qcom, irq, "ss_phy_irq");
+ ret = dwc3_qcom_request_irq(qcom, irq, "qusb2_phy");
+ if (ret)
+ return ret;
+ qcom->ports[port_index].qusb2_phy_irq = irq;
+ }
+
+ return 0;
+}
+
+static int dwc3_qcom_find_num_ports(struct platform_device *pdev)
+{
+ char irq_name[14];
+ int port_num;
+ int irq;
+
+ irq = platform_get_irq_byname_optional(pdev, "dp_hs_phy_1");
+ if (irq <= 0)
+ return 1;
+
+ for (port_num = 2; port_num <= DWC3_MAX_PORTS; port_num++) {
+ sprintf(irq_name, "dp_hs_phy_%d", port_num);
+
+ irq = platform_get_irq_byname_optional(pdev, irq_name);
+ if (irq <= 0)
+ return port_num - 1;
+ }
+
+ return DWC3_MAX_PORTS;
+}
+
+static int dwc3_qcom_setup_irq(struct platform_device *pdev)
+{
+ struct dwc3_qcom *qcom = platform_get_drvdata(pdev);
+ bool is_multiport;
+ int ret;
+ int i;
+
+ qcom->num_ports = dwc3_qcom_find_num_ports(pdev);
+ is_multiport = (qcom->num_ports > 1);
+
+ for (i = 0; i < qcom->num_ports; i++) {
+ ret = dwc3_qcom_setup_port_irq(pdev, i, is_multiport);
if (ret)
return ret;
- qcom->ss_phy_irq = irq;
}
return 0;
--
2.34.1
^ permalink raw reply related
* [PATCH v20 5/9] dt-bindings: usb: qcom,dwc3: Add bindings for SC8280 Multiport
From: Krishna Kurapati @ 2024-04-08 13:29 UTC (permalink / raw)
To: Krzysztof Kozlowski, Rob Herring, Bjorn Andersson, Wesley Cheng,
Konrad Dybcio, Greg Kroah-Hartman, Conor Dooley, Thinh Nguyen,
Felipe Balbi, Johan Hovold
Cc: devicetree, linux-arm-msm, linux-usb, linux-kernel, quic_ppratap,
quic_jackp, Krishna Kurapati, Johan Hovold
In-Reply-To: <20240408132925.1880571-1-quic_kriskura@quicinc.com>
Add the compatible string for SC8280 Multiport USB controller from
Qualcomm.
There are 4 power event irq interrupts supported by this controller
(one for each port of multiport). Added all the 4 as non-optional
interrupts for SC8280XP-MP
Also each port of multiport has one DP and oen DM IRQ. Add all DP/DM
IRQ's related to 4 ports of SC8280XP Teritiary controller.
Also added ss phy irq for both SS Ports.
Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>
Reviewed-by: Bjorn Andersson <andersson@kernel.org>
Reviewed-by: Rob Herring <robh@kernel.org>
Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
---
.../devicetree/bindings/usb/qcom,dwc3.yaml | 34 +++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
index 38a3404ec71b..f55f601c0329 100644
--- a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
@@ -30,6 +30,7 @@ properties:
- qcom,sc7180-dwc3
- qcom,sc7280-dwc3
- qcom,sc8280xp-dwc3
+ - qcom,sc8280xp-dwc3-mp
- qcom,sdm660-dwc3
- qcom,sdm670-dwc3
- qcom,sdm845-dwc3
@@ -282,6 +283,7 @@ allOf:
contains:
enum:
- qcom,sc8280xp-dwc3
+ - qcom,sc8280xp-dwc3-mp
- qcom,x1e80100-dwc3
then:
properties:
@@ -470,6 +472,38 @@ allOf:
- const: dm_hs_phy_irq
- const: ss_phy_irq
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - qcom,sc8280xp-dwc3-mp
+ then:
+ properties:
+ interrupts:
+ minItems: 18
+ maxItems: 18
+ interrupt-names:
+ items:
+ - const: pwr_event_1
+ - const: pwr_event_2
+ - const: pwr_event_3
+ - const: pwr_event_4
+ - const: hs_phy_1
+ - const: hs_phy_2
+ - const: hs_phy_3
+ - const: hs_phy_4
+ - const: dp_hs_phy_1
+ - const: dm_hs_phy_1
+ - const: dp_hs_phy_2
+ - const: dm_hs_phy_2
+ - const: dp_hs_phy_3
+ - const: dm_hs_phy_3
+ - const: dp_hs_phy_4
+ - const: dm_hs_phy_4
+ - const: ss_phy_1
+ - const: ss_phy_2
+
additionalProperties: false
examples:
--
2.34.1
^ permalink raw reply related
* [PATCH v20 3/9] usb: dwc3: core: Skip setting event buffers for host only controllers
From: Krishna Kurapati @ 2024-04-08 13:29 UTC (permalink / raw)
To: Krzysztof Kozlowski, Rob Herring, Bjorn Andersson, Wesley Cheng,
Konrad Dybcio, Greg Kroah-Hartman, Conor Dooley, Thinh Nguyen,
Felipe Balbi, Johan Hovold
Cc: devicetree, linux-arm-msm, linux-usb, linux-kernel, quic_ppratap,
quic_jackp, Krishna Kurapati, Johan Hovold
In-Reply-To: <20240408132925.1880571-1-quic_kriskura@quicinc.com>
On some SoC's like SA8295P where the tertiary controller is host-only
capable, GEVTADDRHI/LO, GEVTSIZ, GEVTCOUNT registers are not accessible.
Trying to access them leads to a crash.
For DRD/Peripheral supported controllers, event buffer setup is done
again in gadget_pullup. Skip setup or cleanup of event buffers if
controller is host-only capable.
Suggested-by: Johan Hovold <johan@kernel.org>
Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>
Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
Reviewed-by: Bjorn Andersson <andersson@kernel.org>
---
drivers/usb/dwc3/core.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index ddab30531f8a..1a3d8a9beea8 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -486,6 +486,13 @@ static void dwc3_free_event_buffers(struct dwc3 *dwc)
static int dwc3_alloc_event_buffers(struct dwc3 *dwc, unsigned int length)
{
struct dwc3_event_buffer *evt;
+ unsigned int hw_mode;
+
+ hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0);
+ if (hw_mode == DWC3_GHWPARAMS0_MODE_HOST) {
+ dwc->ev_buf = NULL;
+ return 0;
+ }
evt = dwc3_alloc_one_event_buffer(dwc, length);
if (IS_ERR(evt)) {
@@ -507,6 +514,9 @@ int dwc3_event_buffers_setup(struct dwc3 *dwc)
{
struct dwc3_event_buffer *evt;
+ if (!dwc->ev_buf)
+ return 0;
+
evt = dwc->ev_buf;
evt->lpos = 0;
dwc3_writel(dwc->regs, DWC3_GEVNTADRLO(0),
@@ -524,6 +534,9 @@ void dwc3_event_buffers_cleanup(struct dwc3 *dwc)
{
struct dwc3_event_buffer *evt;
+ if (!dwc->ev_buf)
+ return;
+
evt = dwc->ev_buf;
evt->lpos = 0;
--
2.34.1
^ permalink raw reply related
* [PATCH v20 6/9] usb: dwc3: qcom: Add helper function to request wakeup interrupts
From: Krishna Kurapati @ 2024-04-08 13:29 UTC (permalink / raw)
To: Krzysztof Kozlowski, Rob Herring, Bjorn Andersson, Wesley Cheng,
Konrad Dybcio, Greg Kroah-Hartman, Conor Dooley, Thinh Nguyen,
Felipe Balbi, Johan Hovold
Cc: devicetree, linux-arm-msm, linux-usb, linux-kernel, quic_ppratap,
quic_jackp, Krishna Kurapati, Johan Hovold
In-Reply-To: <20240408132925.1880571-1-quic_kriskura@quicinc.com>
The logic for requesting interrupts is duplicated for each interrupt. In
the upcoming patches that introduces support for multiport, it would be
better to clean up the duplication before reading mulitport related
interrupts.
Refactor interrupt setup call by adding a new helper function for
requesting the wakeup interrupts. To simplify implementation, make
the display name same as the interrupt name expected in DT.
Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>
Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
---
drivers/usb/dwc3/dwc3-qcom.c | 53 ++++++++++++++++--------------------
1 file changed, 24 insertions(+), 29 deletions(-)
diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
index f6b2fab49d5e..cae5dab8fcfc 100644
--- a/drivers/usb/dwc3/dwc3-qcom.c
+++ b/drivers/usb/dwc3/dwc3-qcom.c
@@ -501,6 +501,22 @@ static void dwc3_qcom_select_utmi_clk(struct dwc3_qcom *qcom)
PIPE_UTMI_CLK_DIS);
}
+static int dwc3_qcom_request_irq(struct dwc3_qcom *qcom, int irq,
+ const char *name)
+{
+ int ret;
+
+ /* Keep wakeup interrupts disabled until suspend */
+ ret = devm_request_threaded_irq(qcom->dev, irq, NULL,
+ qcom_dwc3_resume_irq,
+ IRQF_ONESHOT | IRQF_NO_AUTOEN,
+ name, qcom);
+ if (ret)
+ dev_err(qcom->dev, "failed to request irq %s: %d\n", name, ret);
+
+ return ret;
+}
+
static int dwc3_qcom_setup_irq(struct platform_device *pdev)
{
struct dwc3_qcom *qcom = platform_get_drvdata(pdev);
@@ -509,54 +525,33 @@ static int dwc3_qcom_setup_irq(struct platform_device *pdev)
irq = platform_get_irq_byname_optional(pdev, "qusb2_phy");
if (irq > 0) {
- /* Keep wakeup interrupts disabled until suspend */
- ret = devm_request_threaded_irq(qcom->dev, irq, NULL,
- qcom_dwc3_resume_irq,
- IRQF_ONESHOT | IRQF_NO_AUTOEN,
- "qcom_dwc3 QUSB2", qcom);
- if (ret) {
- dev_err(qcom->dev, "qusb2_phy_irq failed: %d\n", ret);
+ ret = dwc3_qcom_request_irq(qcom, irq, "qusb2_phy");
+ if (ret)
return ret;
- }
qcom->qusb2_phy_irq = irq;
}
irq = platform_get_irq_byname_optional(pdev, "dp_hs_phy_irq");
if (irq > 0) {
- ret = devm_request_threaded_irq(qcom->dev, irq, NULL,
- qcom_dwc3_resume_irq,
- IRQF_ONESHOT | IRQF_NO_AUTOEN,
- "qcom_dwc3 DP_HS", qcom);
- if (ret) {
- dev_err(qcom->dev, "dp_hs_phy_irq failed: %d\n", ret);
+ ret = dwc3_qcom_request_irq(qcom, irq, "dp_hs_phy_irq");
+ if (ret)
return ret;
- }
qcom->dp_hs_phy_irq = irq;
}
irq = platform_get_irq_byname_optional(pdev, "dm_hs_phy_irq");
if (irq > 0) {
- ret = devm_request_threaded_irq(qcom->dev, irq, NULL,
- qcom_dwc3_resume_irq,
- IRQF_ONESHOT | IRQF_NO_AUTOEN,
- "qcom_dwc3 DM_HS", qcom);
- if (ret) {
- dev_err(qcom->dev, "dm_hs_phy_irq failed: %d\n", ret);
+ ret = dwc3_qcom_request_irq(qcom, irq, "dm_hs_phy_irq");
+ if (ret)
return ret;
- }
qcom->dm_hs_phy_irq = irq;
}
irq = platform_get_irq_byname_optional(pdev, "ss_phy_irq");
if (irq > 0) {
- ret = devm_request_threaded_irq(qcom->dev, irq, NULL,
- qcom_dwc3_resume_irq,
- IRQF_ONESHOT | IRQF_NO_AUTOEN,
- "qcom_dwc3 SS", qcom);
- if (ret) {
- dev_err(qcom->dev, "ss_phy_irq failed: %d\n", ret);
+ ret = dwc3_qcom_request_irq(qcom, irq, "ss_phy_irq");
+ if (ret)
return ret;
- }
qcom->ss_phy_irq = irq;
}
--
2.34.1
^ permalink raw reply related
* [PATCH v20 4/9] usb: dwc3: core: Refactor PHY logic to support Multiport Controller
From: Krishna Kurapati @ 2024-04-08 13:29 UTC (permalink / raw)
To: Krzysztof Kozlowski, Rob Herring, Bjorn Andersson, Wesley Cheng,
Konrad Dybcio, Greg Kroah-Hartman, Conor Dooley, Thinh Nguyen,
Felipe Balbi, Johan Hovold
Cc: devicetree, linux-arm-msm, linux-usb, linux-kernel, quic_ppratap,
quic_jackp, Krishna Kurapati, Johan Hovold
In-Reply-To: <20240408132925.1880571-1-quic_kriskura@quicinc.com>
Currently the DWC3 driver supports only single port controller
which requires at least one HS PHY and at most one SS PHY.
But the DWC3 USB controller can be connected to multiple ports and
each port can have their own PHYs. Each port of the multiport
controller can either be HS+SS capable or HS only capable
Proper quantification of them is required to modify GUSB2PHYCFG
and GUSB3PIPECTL registers appropriately.
Add support for detecting, obtaining and configuring PHYs supported
by a multiport controller. Limit support to multiport controllers
with up to four ports for now (e.g. as needed for SC8280XP).
Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>
Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
---
drivers/usb/dwc3/core.c | 251 ++++++++++++++++++++++++++++------------
drivers/usb/dwc3/core.h | 14 ++-
drivers/usb/dwc3/drd.c | 15 ++-
3 files changed, 193 insertions(+), 87 deletions(-)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 1a3d8a9beea8..1f4f228c970b 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -124,6 +124,7 @@ static void __dwc3_set_mode(struct work_struct *work)
int ret;
u32 reg;
u32 desired_dr_role;
+ int i;
mutex_lock(&dwc->mutex);
spin_lock_irqsave(&dwc->lock, flags);
@@ -201,8 +202,10 @@ static void __dwc3_set_mode(struct work_struct *work)
} else {
if (dwc->usb2_phy)
otg_set_vbus(dwc->usb2_phy->otg, true);
- phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_HOST);
- phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_HOST);
+ for (i = 0; i < dwc->num_usb2_ports; i++) {
+ phy_set_mode(dwc->usb2_generic_phy[i], PHY_MODE_USB_HOST);
+ phy_set_mode(dwc->usb3_generic_phy[i], PHY_MODE_USB_HOST);
+ }
if (dwc->dis_split_quirk) {
reg = dwc3_readl(dwc->regs, DWC3_GUCTL3);
reg |= DWC3_GUCTL3_SPLITDISABLE;
@@ -217,8 +220,8 @@ static void __dwc3_set_mode(struct work_struct *work)
if (dwc->usb2_phy)
otg_set_vbus(dwc->usb2_phy->otg, false);
- phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_DEVICE);
- phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_DEVICE);
+ phy_set_mode(dwc->usb2_generic_phy[0], PHY_MODE_USB_DEVICE);
+ phy_set_mode(dwc->usb3_generic_phy[0], PHY_MODE_USB_DEVICE);
ret = dwc3_gadget_init(dwc);
if (ret)
@@ -589,22 +592,14 @@ static int dwc3_core_ulpi_init(struct dwc3 *dwc)
return ret;
}
-/**
- * dwc3_phy_setup - Configure USB PHY Interface of DWC3 Core
- * @dwc: Pointer to our controller context structure
- *
- * Returns 0 on success. The USB PHY interfaces are configured but not
- * initialized. The PHY interfaces and the PHYs get initialized together with
- * the core in dwc3_core_init.
- */
-static int dwc3_phy_setup(struct dwc3 *dwc)
+static int dwc3_ss_phy_setup(struct dwc3 *dwc, int index)
{
unsigned int hw_mode;
u32 reg;
hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0);
- reg = dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(0));
+ reg = dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(index));
/*
* Make sure UX_EXIT_PX is cleared as that causes issues with some
@@ -659,9 +654,19 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
if (dwc->dis_del_phy_power_chg_quirk)
reg &= ~DWC3_GUSB3PIPECTL_DEPOCHANGE;
- dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(0), reg);
+ dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(index), reg);
- reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
+ return 0;
+}
+
+static int dwc3_hs_phy_setup(struct dwc3 *dwc, int index)
+{
+ unsigned int hw_mode;
+ u32 reg;
+
+ hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0);
+
+ reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(index));
/* Select the HS PHY interface */
switch (DWC3_GHWPARAMS3_HSPHY_IFC(dwc->hwparams.hwparams3)) {
@@ -673,7 +678,7 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
} else if (dwc->hsphy_interface &&
!strncmp(dwc->hsphy_interface, "ulpi", 4)) {
reg |= DWC3_GUSB2PHYCFG_ULPI_UTMI;
- dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
+ dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(index), reg);
} else {
/* Relying on default value. */
if (!(reg & DWC3_GUSB2PHYCFG_ULPI_UTMI))
@@ -740,7 +745,35 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
if (dwc->ulpi_ext_vbus_drv)
reg |= DWC3_GUSB2PHYCFG_ULPIEXTVBUSDRV;
- dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
+ dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(index), reg);
+
+ return 0;
+}
+
+/**
+ * dwc3_phy_setup - Configure USB PHY Interface of DWC3 Core
+ * @dwc: Pointer to our controller context structure
+ *
+ * Returns 0 on success. The USB PHY interfaces are configured but not
+ * initialized. The PHY interfaces and the PHYs get initialized together with
+ * the core in dwc3_core_init.
+ */
+static int dwc3_phy_setup(struct dwc3 *dwc)
+{
+ int i;
+ int ret;
+
+ for (i = 0; i < dwc->num_usb3_ports; i++) {
+ ret = dwc3_ss_phy_setup(dwc, i);
+ if (ret)
+ return ret;
+ }
+
+ for (i = 0; i < dwc->num_usb2_ports; i++) {
+ ret = dwc3_hs_phy_setup(dwc, i);
+ if (ret)
+ return ret;
+ }
return 0;
}
@@ -748,23 +781,32 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
static int dwc3_phy_init(struct dwc3 *dwc)
{
int ret;
+ int i;
+ int j;
usb_phy_init(dwc->usb2_phy);
usb_phy_init(dwc->usb3_phy);
- ret = phy_init(dwc->usb2_generic_phy);
- if (ret < 0)
- goto err_shutdown_usb3_phy;
+ for (i = 0; i < dwc->num_usb2_ports; i++) {
+ ret = phy_init(dwc->usb2_generic_phy[i]);
+ if (ret < 0)
+ goto err_exit_phy;
- ret = phy_init(dwc->usb3_generic_phy);
- if (ret < 0)
- goto err_exit_usb2_phy;
+ ret = phy_init(dwc->usb3_generic_phy[i]);
+ if (ret < 0) {
+ phy_exit(dwc->usb2_generic_phy[i]);
+ goto err_exit_phy;
+ }
+ }
return 0;
-err_exit_usb2_phy:
- phy_exit(dwc->usb2_generic_phy);
-err_shutdown_usb3_phy:
+err_exit_phy:
+ for (j = i - 1; j >= 0; j--) {
+ phy_exit(dwc->usb3_generic_phy[j]);
+ phy_exit(dwc->usb2_generic_phy[j]);
+ }
+
usb_phy_shutdown(dwc->usb3_phy);
usb_phy_shutdown(dwc->usb2_phy);
@@ -773,8 +815,12 @@ static int dwc3_phy_init(struct dwc3 *dwc)
static void dwc3_phy_exit(struct dwc3 *dwc)
{
- phy_exit(dwc->usb3_generic_phy);
- phy_exit(dwc->usb2_generic_phy);
+ int i;
+
+ for (i = 0; i < dwc->num_usb2_ports; i++) {
+ phy_exit(dwc->usb3_generic_phy[i]);
+ phy_exit(dwc->usb2_generic_phy[i]);
+ }
usb_phy_shutdown(dwc->usb3_phy);
usb_phy_shutdown(dwc->usb2_phy);
@@ -783,23 +829,32 @@ static void dwc3_phy_exit(struct dwc3 *dwc)
static int dwc3_phy_power_on(struct dwc3 *dwc)
{
int ret;
+ int i;
+ int j;
usb_phy_set_suspend(dwc->usb2_phy, 0);
usb_phy_set_suspend(dwc->usb3_phy, 0);
- ret = phy_power_on(dwc->usb2_generic_phy);
- if (ret < 0)
- goto err_suspend_usb3_phy;
+ for (i = 0; i < dwc->num_usb2_ports; i++) {
+ ret = phy_power_on(dwc->usb2_generic_phy[i]);
+ if (ret < 0)
+ goto err_power_off_phy;
- ret = phy_power_on(dwc->usb3_generic_phy);
- if (ret < 0)
- goto err_power_off_usb2_phy;
+ ret = phy_power_on(dwc->usb3_generic_phy[i]);
+ if (ret < 0) {
+ phy_power_off(dwc->usb2_generic_phy[i]);
+ goto err_power_off_phy;
+ }
+ }
return 0;
-err_power_off_usb2_phy:
- phy_power_off(dwc->usb2_generic_phy);
-err_suspend_usb3_phy:
+err_power_off_phy:
+ for (j = i - 1; j >= 0; j--) {
+ phy_power_off(dwc->usb3_generic_phy[j]);
+ phy_power_off(dwc->usb2_generic_phy[j]);
+ }
+
usb_phy_set_suspend(dwc->usb3_phy, 1);
usb_phy_set_suspend(dwc->usb2_phy, 1);
@@ -808,8 +863,12 @@ static int dwc3_phy_power_on(struct dwc3 *dwc)
static void dwc3_phy_power_off(struct dwc3 *dwc)
{
- phy_power_off(dwc->usb3_generic_phy);
- phy_power_off(dwc->usb2_generic_phy);
+ int i;
+
+ for (i = 0; i < dwc->num_usb2_ports; i++) {
+ phy_power_off(dwc->usb3_generic_phy[i]);
+ phy_power_off(dwc->usb2_generic_phy[i]);
+ }
usb_phy_set_suspend(dwc->usb3_phy, 1);
usb_phy_set_suspend(dwc->usb2_phy, 1);
@@ -1201,6 +1260,7 @@ static int dwc3_core_init(struct dwc3 *dwc)
unsigned int hw_mode;
u32 reg;
int ret;
+ int i;
hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0);
@@ -1244,15 +1304,19 @@ static int dwc3_core_init(struct dwc3 *dwc)
if (hw_mode == DWC3_GHWPARAMS0_MODE_DRD &&
!DWC3_VER_IS_WITHIN(DWC3, ANY, 194A)) {
if (!dwc->dis_u3_susphy_quirk) {
- reg = dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(0));
- reg |= DWC3_GUSB3PIPECTL_SUSPHY;
- dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(0), reg);
+ for (i = 0; i < dwc->num_usb3_ports; i++) {
+ reg = dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(i));
+ reg |= DWC3_GUSB3PIPECTL_SUSPHY;
+ dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(i), reg);
+ }
}
if (!dwc->dis_u2_susphy_quirk) {
- reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
- reg |= DWC3_GUSB2PHYCFG_SUSPHY;
- dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
+ for (i = 0; i < dwc->num_usb2_ports; i++) {
+ reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(i));
+ reg |= DWC3_GUSB2PHYCFG_SUSPHY;
+ dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(i), reg);
+ }
}
}
@@ -1372,7 +1436,9 @@ static int dwc3_core_get_phy(struct dwc3 *dwc)
{
struct device *dev = dwc->dev;
struct device_node *node = dev->of_node;
+ char phy_name[9];
int ret;
+ int i;
if (node) {
dwc->usb2_phy = devm_usb_get_phy_by_phandle(dev, "usb-phy", 0);
@@ -1398,22 +1464,36 @@ static int dwc3_core_get_phy(struct dwc3 *dwc)
return dev_err_probe(dev, ret, "no usb3 phy configured\n");
}
- dwc->usb2_generic_phy = devm_phy_get(dev, "usb2-phy");
- if (IS_ERR(dwc->usb2_generic_phy)) {
- ret = PTR_ERR(dwc->usb2_generic_phy);
- if (ret == -ENOSYS || ret == -ENODEV)
- dwc->usb2_generic_phy = NULL;
+ for (i = 0; i < dwc->num_usb2_ports; i++) {
+ if (dwc->num_usb2_ports == 1)
+ sprintf(phy_name, "usb2-phy");
else
- return dev_err_probe(dev, ret, "no usb2 phy configured\n");
- }
+ sprintf(phy_name, "usb2-%d", i);
+
+ dwc->usb2_generic_phy[i] = devm_phy_get(dev, phy_name);
+ if (IS_ERR(dwc->usb2_generic_phy[i])) {
+ ret = PTR_ERR(dwc->usb2_generic_phy[i]);
+ if (ret == -ENOSYS || ret == -ENODEV)
+ dwc->usb2_generic_phy[i] = NULL;
+ else
+ return dev_err_probe(dev, ret, "failed to lookup phy %s\n",
+ phy_name);
+ }
- dwc->usb3_generic_phy = devm_phy_get(dev, "usb3-phy");
- if (IS_ERR(dwc->usb3_generic_phy)) {
- ret = PTR_ERR(dwc->usb3_generic_phy);
- if (ret == -ENOSYS || ret == -ENODEV)
- dwc->usb3_generic_phy = NULL;
+ if (dwc->num_usb2_ports == 1)
+ sprintf(phy_name, "usb3-phy");
else
- return dev_err_probe(dev, ret, "no usb3 phy configured\n");
+ sprintf(phy_name, "usb3-%d", i);
+
+ dwc->usb3_generic_phy[i] = devm_phy_get(dev, phy_name);
+ if (IS_ERR(dwc->usb3_generic_phy[i])) {
+ ret = PTR_ERR(dwc->usb3_generic_phy[i]);
+ if (ret == -ENOSYS || ret == -ENODEV)
+ dwc->usb3_generic_phy[i] = NULL;
+ else
+ return dev_err_probe(dev, ret, "failed to lookup phy %s\n",
+ phy_name);
+ }
}
return 0;
@@ -1423,6 +1503,7 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
{
struct device *dev = dwc->dev;
int ret;
+ int i;
switch (dwc->dr_mode) {
case USB_DR_MODE_PERIPHERAL:
@@ -1430,8 +1511,8 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
if (dwc->usb2_phy)
otg_set_vbus(dwc->usb2_phy->otg, false);
- phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_DEVICE);
- phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_DEVICE);
+ phy_set_mode(dwc->usb2_generic_phy[0], PHY_MODE_USB_DEVICE);
+ phy_set_mode(dwc->usb3_generic_phy[0], PHY_MODE_USB_DEVICE);
ret = dwc3_gadget_init(dwc);
if (ret)
@@ -1442,8 +1523,10 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
if (dwc->usb2_phy)
otg_set_vbus(dwc->usb2_phy->otg, true);
- phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_HOST);
- phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_HOST);
+ for (i = 0; i < dwc->num_usb2_ports; i++) {
+ phy_set_mode(dwc->usb2_generic_phy[i], PHY_MODE_USB_HOST);
+ phy_set_mode(dwc->usb3_generic_phy[i], PHY_MODE_USB_HOST);
+ }
ret = dwc3_host_init(dwc);
if (ret)
@@ -1937,6 +2020,10 @@ static int dwc3_get_num_ports(struct dwc3 *dwc)
iounmap(base);
+ if (dwc->num_usb2_ports > DWC3_MAX_PORTS ||
+ dwc->num_usb3_ports > DWC3_MAX_PORTS)
+ return -ENOMEM;
+
return 0;
}
@@ -2174,6 +2261,7 @@ static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
{
unsigned long flags;
u32 reg;
+ int i;
switch (dwc->current_dr_role) {
case DWC3_GCTL_PRTCAP_DEVICE:
@@ -2192,17 +2280,21 @@ static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
/* Let controller to suspend HSPHY before PHY driver suspends */
if (dwc->dis_u2_susphy_quirk ||
dwc->dis_enblslpm_quirk) {
- reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
- reg |= DWC3_GUSB2PHYCFG_ENBLSLPM |
- DWC3_GUSB2PHYCFG_SUSPHY;
- dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
+ for (i = 0; i < dwc->num_usb2_ports; i++) {
+ reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(i));
+ reg |= DWC3_GUSB2PHYCFG_ENBLSLPM |
+ DWC3_GUSB2PHYCFG_SUSPHY;
+ dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(i), reg);
+ }
/* Give some time for USB2 PHY to suspend */
usleep_range(5000, 6000);
}
- phy_pm_runtime_put_sync(dwc->usb2_generic_phy);
- phy_pm_runtime_put_sync(dwc->usb3_generic_phy);
+ for (i = 0; i < dwc->num_usb2_ports; i++) {
+ phy_pm_runtime_put_sync(dwc->usb2_generic_phy[i]);
+ phy_pm_runtime_put_sync(dwc->usb3_generic_phy[i]);
+ }
break;
case DWC3_GCTL_PRTCAP_OTG:
/* do nothing during runtime_suspend */
@@ -2232,6 +2324,7 @@ static int dwc3_resume_common(struct dwc3 *dwc, pm_message_t msg)
unsigned long flags;
int ret;
u32 reg;
+ int i;
switch (dwc->current_dr_role) {
case DWC3_GCTL_PRTCAP_DEVICE:
@@ -2251,17 +2344,21 @@ static int dwc3_resume_common(struct dwc3 *dwc, pm_message_t msg)
break;
}
/* Restore GUSB2PHYCFG bits that were modified in suspend */
- reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
- if (dwc->dis_u2_susphy_quirk)
- reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
+ for (i = 0; i < dwc->num_usb2_ports; i++) {
+ reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(i));
+ if (dwc->dis_u2_susphy_quirk)
+ reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
- if (dwc->dis_enblslpm_quirk)
- reg &= ~DWC3_GUSB2PHYCFG_ENBLSLPM;
+ if (dwc->dis_enblslpm_quirk)
+ reg &= ~DWC3_GUSB2PHYCFG_ENBLSLPM;
- dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
+ dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(i), reg);
+ }
- phy_pm_runtime_get_sync(dwc->usb2_generic_phy);
- phy_pm_runtime_get_sync(dwc->usb3_generic_phy);
+ for (i = 0; i < dwc->num_usb2_ports; i++) {
+ phy_pm_runtime_get_sync(dwc->usb2_generic_phy[i]);
+ phy_pm_runtime_get_sync(dwc->usb3_generic_phy[i]);
+ }
break;
case DWC3_GCTL_PRTCAP_OTG:
/* nothing to do on runtime_resume */
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 341e4c73cb2e..df2e111aa848 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -33,6 +33,12 @@
#include <linux/power_supply.h>
+/*
+ * Maximum number of ports currently supported for multiport
+ * controllers.
+ */
+#define DWC3_MAX_PORTS 4
+
#define DWC3_MSG_MAX 500
/* Global constants */
@@ -1037,8 +1043,8 @@ struct dwc3_scratchpad_array {
* @usb_psy: pointer to power supply interface.
* @usb2_phy: pointer to USB2 PHY
* @usb3_phy: pointer to USB3 PHY
- * @usb2_generic_phy: pointer to USB2 PHY
- * @usb3_generic_phy: pointer to USB3 PHY
+ * @usb2_generic_phy: pointer to array of USB2 PHYs
+ * @usb3_generic_phy: pointer to array of USB3 PHYs
* @num_usb2_ports: number of USB2 ports
* @num_usb3_ports: number of USB3 ports
* @phys_ready: flag to indicate that PHYs are ready
@@ -1186,8 +1192,8 @@ struct dwc3 {
struct usb_phy *usb2_phy;
struct usb_phy *usb3_phy;
- struct phy *usb2_generic_phy;
- struct phy *usb3_generic_phy;
+ struct phy *usb2_generic_phy[DWC3_MAX_PORTS];
+ struct phy *usb3_generic_phy[DWC3_MAX_PORTS];
u8 num_usb2_ports;
u8 num_usb3_ports;
diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c
index 57ddd2e43022..d76ae676783c 100644
--- a/drivers/usb/dwc3/drd.c
+++ b/drivers/usb/dwc3/drd.c
@@ -331,6 +331,7 @@ void dwc3_otg_update(struct dwc3 *dwc, bool ignore_idstatus)
u32 reg;
int id;
unsigned long flags;
+ int i;
if (dwc->dr_mode != USB_DR_MODE_OTG)
return;
@@ -386,9 +387,12 @@ void dwc3_otg_update(struct dwc3 *dwc, bool ignore_idstatus)
} else {
if (dwc->usb2_phy)
otg_set_vbus(dwc->usb2_phy->otg, true);
- if (dwc->usb2_generic_phy)
- phy_set_mode(dwc->usb2_generic_phy,
- PHY_MODE_USB_HOST);
+ for (i = 0; i < dwc->num_usb2_ports; i++) {
+ if (dwc->usb2_generic_phy[i]) {
+ phy_set_mode(dwc->usb2_generic_phy[i],
+ PHY_MODE_USB_HOST);
+ }
+ }
}
break;
case DWC3_OTG_ROLE_DEVICE:
@@ -400,9 +404,8 @@ void dwc3_otg_update(struct dwc3 *dwc, bool ignore_idstatus)
if (dwc->usb2_phy)
otg_set_vbus(dwc->usb2_phy->otg, false);
- if (dwc->usb2_generic_phy)
- phy_set_mode(dwc->usb2_generic_phy,
- PHY_MODE_USB_DEVICE);
+ if (dwc->usb2_generic_phy[0])
+ phy_set_mode(dwc->usb2_generic_phy[0], PHY_MODE_USB_DEVICE);
ret = dwc3_gadget_init(dwc);
if (ret)
dev_err(dwc->dev, "failed to initialize peripheral\n");
--
2.34.1
^ permalink raw reply related
* Re: [PATCH v3 2/2] media: i2c: Add GC05A2 image sensor driver
From: Kieran Bingham @ 2024-04-08 13:29 UTC (permalink / raw)
To: Zhi Mao 毛智, krzysztof.kozlowski+dt, mchehab,
robh+dt, sakari.ailus
Cc: heiko
In-Reply-To: <5cdf1a2530ffd927fe2e4130ab6666724cf3354d.camel@mediatek.com>
Quoting Zhi Mao (毛智) (2024-04-08 12:50:21)
> Hi Kieran,
>
> Thanks for your review this patch.
>
> It seems that there are some difficult for us(Mediatek) to explain
> these register setting comments.
> As these settings are released by GC sensor vendor, and we have not
> detailed datasheet described them.
> And even if send the letter to ask sensor vendor, I am afraid there may
> be not a clear response.
>
> Can we just focus on the driver code function and control flow part?
>
As I said - You can take my comments with a pinch of salt ... but I
wanted to know your position on it ;-)
--
Kieran
> On Sun, 2024-04-07 at 10:08 +0100, Kieran Bingham wrote:
> >
> > External email : Please do not click links or open attachments until
> > you have verified the sender or the content.
> > Hello,
> >
> > Thanks for helping extending the kernels sensor driver support.
> >
> > My comments below can likely be taken with a pinch of salt, as they
> > are
> > mostly around the tabled register values ... but we have many drivers
> > which are binary blobs of sensor register values and I think it would
> > be
> > far more beneficial to clean these up where possible...
> >
> > So the first question is ... Can we ?
> >
> >
> >
> > Quoting Zhi Mao (2024-04-03 04:38:25)
> > > Add a V4L2 sub-device driver for Galaxycore GC05A2 image sensor.
> > >
> > > Signed-off-by: Zhi Mao <zhi.mao@mediatek.com>
> > > ---
> > > drivers/media/i2c/Kconfig | 10 +
> > > drivers/media/i2c/Makefile | 1 +
> > > drivers/media/i2c/gc05a2.c | 1383
> > ++++++++++++++++++++++++++++++++++++
> > > 3 files changed, 1394 insertions(+)
> > > create mode 100644 drivers/media/i2c/gc05a2.c
> > >
> > > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> > > index 56f276b920ab..97993bf160f9 100644
> > > --- a/drivers/media/i2c/Kconfig
> > > +++ b/drivers/media/i2c/Kconfig
> > > @@ -70,6 +70,16 @@ config VIDEO_GC0308
> > > To compile this driver as a module, choose M here: the
> > > module will be called gc0308.
> > >
> > > +config VIDEO_GC05A2
> > > + tristate "GalaxyCore gc05a2 sensor support"
> > > + select V4L2_CCI_I2C
> > > + help
> > > + This is a Video4Linux2 sensor driver for the GalaxyCore
> > gc05a2
> > > + camera.
> > > +
> > > + To compile this driver as a module, choose M here: the
> > > + module will be called gc05a2.
> > > +
> > > config VIDEO_GC2145
> > > select V4L2_CCI_I2C
> > > tristate "GalaxyCore GC2145 sensor support"
> > > diff --git a/drivers/media/i2c/Makefile
> > b/drivers/media/i2c/Makefile
> > > index dfbe6448b549..8ed6faf0f854 100644
> > > --- a/drivers/media/i2c/Makefile
> > > +++ b/drivers/media/i2c/Makefile
> > > @@ -38,6 +38,7 @@ obj-$(CONFIG_VIDEO_DW9768) += dw9768.o
> > > obj-$(CONFIG_VIDEO_DW9807_VCM) += dw9807-vcm.o
> > > obj-$(CONFIG_VIDEO_ET8EK8) += et8ek8/
> > > obj-$(CONFIG_VIDEO_GC0308) += gc0308.o
> > > +obj-$(CONFIG_VIDEO_GC05A2) += gc05a2.o
> > > obj-$(CONFIG_VIDEO_GC2145) += gc2145.o
> > > obj-$(CONFIG_VIDEO_HI556) += hi556.o
> > > obj-$(CONFIG_VIDEO_HI846) += hi846.o
> > > diff --git a/drivers/media/i2c/gc05a2.c
> > b/drivers/media/i2c/gc05a2.c
> > > new file mode 100644
> > > index 000000000000..461d33055a3b
> > > --- /dev/null
> > > +++ b/drivers/media/i2c/gc05a2.c
> > > @@ -0,0 +1,1383 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Driver for GalaxyCore gc05a2 image sensor
> > > + *
> > > + * Copyright 2024 MediaTek
> > > + *
> > > + * Zhi Mao <zhi.mao@mediatek.com>
> > > + */
> > > +#include <linux/array_size.h>
> > > +#include <linux/bits.h>
> > > +#include <linux/clk.h>
> > > +#include <linux/container_of.h>
> > > +#include <linux/delay.h>
> > > +#include <linux/device.h>
> > > +#include <linux/err.h>
> > > +#include <linux/gpio/consumer.h>
> > > +#include <linux/math64.h>
> > > +#include <linux/mod_devicetable.h>
> > > +#include <linux/pm_runtime.h>
> > > +#include <linux/property.h>
> > > +#include <linux/regulator/consumer.h>
> > > +#include <linux/types.h>
> > > +#include <linux/units.h>
> > > +
> > > +#include <media/v4l2-cci.h>
> > > +#include <media/v4l2-ctrls.h>
> > > +#include <media/v4l2-event.h>
> > > +#include <media/v4l2-fwnode.h>
> > > +#include <media/v4l2-subdev.h>
> > > +
> > > +#define GC05A2_REG_TEST_PATTERN_EN CCI_REG8(0x008c)
> > > +#define GC05A2_REG_TEST_PATTERN_IDX CCI_REG8(0x008d)
> > > +#define GC05A2_TEST_PATTERN_EN 0x01
> > > +
> > > +#define GC05A2_STREAMING_REG CCI_REG8(0x0100)
> > > +
> > > +#define GC05A2_FLIP_REG CCI_REG8(0x0101)
> > > +#define GC05A2_FLIP_H_MASK BIT(0)
> > > +#define GC05A2_FLIP_V_MASK BIT(1)
> > > +
> > > +#define GC05A2_EXP_REG CCI_REG16(0x0202)
> > > +#define GC05A2_EXP_MARGIN 16
> > > +#define GC05A2_EXP_MIN 4
> > > +#define GC05A2_EXP_STEP 1
> > > +
> > > +#define GC05A2_AGAIN_REG CCI_REG16(0x0204)
> > > +#define GC05A2_AGAIN_MIN 1024
> > > +#define GC05A2_AGAIN_MAX (1024 * 16)
> > > +#define GC05A2_AGAIN_STEP 1
> > > +
> > > +#define GC05A2_FRAME_LENGTH_REG CCI_REG16(0x0340)
> > > +#define GC05A2_VTS_MAX 0xffff
> > > +
> > > +#define GC05A2_REG_CHIP_ID CCI_REG16(0x03f0)
> > > +#define GC05A2_CHIP_ID 0x05a2
> > > +
> > > +#define GC05A2_NATIVE_WIDTH 2592
> > > +#define GC05A2_NATIVE_HEIGHT 1944
> > > +
> > > +#define GC05A2_DEFAULT_CLK_FREQ (24 * HZ_PER_MHZ)
> > > +#define GC05A2_MBUS_CODE MEDIA_BUS_FMT_SGRBG10_1X10
> > > +#define GC05A2_DATA_LANES 2
> > > +#define GC05A2_RGB_DEPTH 10
> > > +#define GC05A2_SLEEP_US (2 * USEC_PER_MSEC)
> > > +
> > > +static const char *const gc05a2_test_pattern_menu[] = {
> > > + "No Pattern", "Fade_to_gray_Color Bar", "Color Bar",
> > > + "PN9", "Horizental_gradient", "Checkboard
> > Pattern",
> > > + "Slant", "Resolution", "Solid Black",
> > > + "Solid White",
> > > +};
> > > +
> > > +static const s64 gc05a2_link_freq_menu_items[] = {
> > > + (448 * HZ_PER_MHZ),
> > > + (224 * HZ_PER_MHZ),
> > > +};
> > > +
> > > +static const char *const gc05a2_supply_name[] = {
> > > + "avdd",
> > > + "dvdd",
> > > + "dovdd",
> > > +};
> > > +
> > > +struct gc05a2 {
> > > + struct device *dev;
> > > + struct v4l2_subdev sd;
> > > + struct media_pad pad;
> > > +
> > > + struct clk *xclk;
> > > + struct regulator_bulk_data
> > supplies[ARRAY_SIZE(gc05a2_supply_name)];
> > > + struct gpio_desc *reset_gpio;
> > > +
> > > + struct v4l2_ctrl_handler ctrls;
> > > + struct v4l2_ctrl *pixel_rate;
> > > + struct v4l2_ctrl *link_freq;
> > > + struct v4l2_ctrl *exposure;
> > > + struct v4l2_ctrl *vblank;
> > > + struct v4l2_ctrl *hblank;
> > > + struct v4l2_ctrl *hflip;
> > > + struct v4l2_ctrl *vflip;
> > > +
> > > + struct regmap *regmap;
> > > + unsigned long link_freq_bitmap;
> > > +
> > > + /* True if the device has been identified */
> > > + bool identified;
> > > + const struct gc05a2_mode *cur_mode;
> > > +};
> > > +
> > > +struct gc05a2_reg_list {
> > > + u32 num_of_regs;
> > > + const struct cci_reg_sequence *regs;
> > > +};
> > > +
> > > +static const struct cci_reg_sequence mode_2592x1944[] = {
> > > + /* system */
> > > + { CCI_REG8(0x0135), 0x01 },
> > > +
> > > + /* pre_setting */
> > > + { CCI_REG8(0x0084), 0x21 },
> > > + { CCI_REG8(0x0d05), 0xcc },
> > > + { CCI_REG8(0x0218), 0x00 },
> > > + { CCI_REG8(0x005e), 0x48 },
> > > + { CCI_REG8(0x0d06), 0x01 },
> > > + { CCI_REG8(0x0007), 0x16 },
> > > + { CCI_REG8(0x0101), 0x00 },
> > > +
> > > + /* analog */
> > > + { CCI_REG8(0x0342), 0x07 },
> > > + { CCI_REG8(0x0343), 0x28 },
> > > + { CCI_REG8(0x0220), 0x07 },
> > > + { CCI_REG8(0x0221), 0xd0 },
> > > + { CCI_REG8(0x0202), 0x07 },
> > > + { CCI_REG8(0x0203), 0x32 },
> > > + { CCI_REG8(0x0340), 0x07 },
> > > + { CCI_REG8(0x0341), 0xf0 },
> > > + { CCI_REG8(0x0219), 0x00 },
> > > + { CCI_REG8(0x0346), 0x00 },
> > > + { CCI_REG8(0x0347), 0x04 },
> > > + { CCI_REG8(0x0d14), 0x00 },
> > > + { CCI_REG8(0x0d13), 0x05 },
> > > + { CCI_REG8(0x0d16), 0x05 },
> > > + { CCI_REG8(0x0d15), 0x1d },
> > > + { CCI_REG8(0x00c0), 0x0a },
> > > + { CCI_REG8(0x00c1), 0x30 },
> > > + { CCI_REG8(0x034a), 0x07 },
> > > + { CCI_REG8(0x034b), 0xa8 },
> > > + { CCI_REG8(0x0e0a), 0x00 },
> > > + { CCI_REG8(0x0e0b), 0x00 },
> > > + { CCI_REG8(0x0e0e), 0x03 },
> > > + { CCI_REG8(0x0e0f), 0x00 },
> > > + { CCI_REG8(0x0e06), 0x0a },
> > > + { CCI_REG8(0x0e23), 0x15 },
> > > + { CCI_REG8(0x0e24), 0x15 },
> > > + { CCI_REG8(0x0e2a), 0x10 },
> > > + { CCI_REG8(0x0e2b), 0x10 },
> > > + { CCI_REG8(0x0e17), 0x49 },
> > > + { CCI_REG8(0x0e1b), 0x1c },
> > > + { CCI_REG8(0x0e3a), 0x36 },
> > > + { CCI_REG8(0x0d11), 0x84 },
> > > + { CCI_REG8(0x0e52), 0x14 },
> > > + { CCI_REG8(0x000b), 0x10 },
> > > + { CCI_REG8(0x0008), 0x08 },
> > > + { CCI_REG8(0x0223), 0x17 },
> > > + { CCI_REG8(0x0d27), 0x39 },
> > > + { CCI_REG8(0x0d22), 0x00 },
> > > + { CCI_REG8(0x03f6), 0x0d },
> > > + { CCI_REG8(0x0d04), 0x07 },
> > > + { CCI_REG8(0x03f3), 0x72 },
> > > + { CCI_REG8(0x03f4), 0xb8 },
> > > + { CCI_REG8(0x03f5), 0xbc },
> > > + { CCI_REG8(0x0d02), 0x73 },
> > > +
> > > + /* auto load start */
> > > + { CCI_REG8(0x00cb), 0x00 },
> > > +
> > > + /* OUT 2592*1944 */
> > > + { CCI_REG8(0x0350), 0x01 },
> > > + { CCI_REG8(0x0353), 0x00 },
> > > + { CCI_REG8(0x0354), 0x08 },
> >
> > > + { CCI_REG8(0x034c), 0x0a },
> > > + { CCI_REG8(0x034d), 0x20 },
> >
> > Should/Could this be
> > { CCI_REG16(0x034c), 2592 }, /* Width */
> >
> >
> > > + { CCI_REG8(0x021f), 0x14 },
> > > +
> > > + /* MIPI */
> > > + { CCI_REG8(0x0107), 0x05 },
> > > + { CCI_REG8(0x0117), 0x01 },
> > > + { CCI_REG8(0x0d81), 0x00 },
> > > + { CCI_REG8(0x0d84), 0x0c },
> > > + { CCI_REG8(0x0d85), 0xa8 },
> > > + { CCI_REG8(0x0d86), 0x06 },
> > > + { CCI_REG8(0x0d87), 0x55 },
> > > + { CCI_REG8(0x0db3), 0x06 },
> > > + { CCI_REG8(0x0db4), 0x08 },
> > > + { CCI_REG8(0x0db5), 0x1e },
> > > + { CCI_REG8(0x0db6), 0x02 },
> > > + { CCI_REG8(0x0db8), 0x12 },
> > > + { CCI_REG8(0x0db9), 0x0a },
> > > + { CCI_REG8(0x0d93), 0x06 },
> > > + { CCI_REG8(0x0d94), 0x09 },
> > > + { CCI_REG8(0x0d95), 0x0d },
> > > + { CCI_REG8(0x0d99), 0x0b },
> > > + { CCI_REG8(0x0084), 0x01 },
> > > +
> > > + /* OUT */
> > > + { CCI_REG8(0x0110), 0x01 },
> > > +};
> > > +
> > > +static const struct cci_reg_sequence mode_1280x720[] = {
> > > + /* system */
> > > + { CCI_REG8(0x0135), 0x05 },
> >
> > In 2592x1944 this is 0x01. Do you have a datasheet? Can you explain
> > why
> > they are different? Can you add register definitions that have names
> > to
> > make this more maintainable or extendable in the future?
> >
> > There's discussion in the recent series improving the IMX258 which
> > makes
> > me wonder if we should try harder to have sensor drivers with clearer
> > definitions.
> >
> >
> > > +
> > > + /*pre_setting*/
> >
> > /* pre_setting */ ?
> >
> > > + { CCI_REG8(0x0084), 0x21 },
> > > + { CCI_REG8(0x0d05), 0xcc },
> > > + { CCI_REG8(0x0218), 0x80 },
> > > + { CCI_REG8(0x005e), 0x49 },
> > > + { CCI_REG8(0x0d06), 0x81 },
> > > + { CCI_REG8(0x0007), 0x16 },
> > > + { CCI_REG8(0x0101), 0x00 },
> >
> > In 2592x1944, only register 0x0218 differs. Why? What is that? Can it
> > be
> > broken out to a function that applies the correct configuration at
> > startuup based on a parameter instead of duplicating this table set?
> >
> > > +
> > > + /* analog */
> > > + { CCI_REG8(0x0342), 0x07 },
> > > + { CCI_REG8(0x0343), 0x10 },
> > > + { CCI_REG8(0x0220), 0x07 },
> > > + { CCI_REG8(0x0221), 0xd0 },
> > > + { CCI_REG8(0x0202), 0x03 },
> > > + { CCI_REG8(0x0203), 0x32 },
> > > + { CCI_REG8(0x0340), 0x04 },
> > > + { CCI_REG8(0x0341), 0x08 },
> > > + { CCI_REG8(0x0219), 0x00 },
> > > + { CCI_REG8(0x0346), 0x01 },
> > > + { CCI_REG8(0x0347), 0x00 },
> > > + { CCI_REG8(0x0d14), 0x00 },
> > > + { CCI_REG8(0x0d13), 0x05 },
> > > + { CCI_REG8(0x0d16), 0x05 },
> > > + { CCI_REG8(0x0d15), 0x1d },
> > > + { CCI_REG8(0x00c0), 0x0a },
> > > + { CCI_REG8(0x00c1), 0x30 },
> > > + { CCI_REG8(0x034a), 0x05 },
> > > + { CCI_REG8(0x034b), 0xb0 },
> > > + { CCI_REG8(0x0e0a), 0x00 },
> > > + { CCI_REG8(0x0e0b), 0x00 },
> > > + { CCI_REG8(0x0e0e), 0x03 },
> > > + { CCI_REG8(0x0e0f), 0x00 },
> > > + { CCI_REG8(0x0e06), 0x0a },
> > > + { CCI_REG8(0x0e23), 0x15 },
> > > + { CCI_REG8(0x0e24), 0x15 },
> > > + { CCI_REG8(0x0e2a), 0x10 },
> > > + { CCI_REG8(0x0e2b), 0x10 },
> > > + { CCI_REG8(0x0e17), 0x49 },
> > > + { CCI_REG8(0x0e1b), 0x1c },
> > > + { CCI_REG8(0x0e3a), 0x36 },
> > > + { CCI_REG8(0x0d11), 0x84 },
> > > + { CCI_REG8(0x0e52), 0x14 },
> > > + { CCI_REG8(0x000b), 0x0e },
> > > + { CCI_REG8(0x0008), 0x03 },
> > > + { CCI_REG8(0x0223), 0x16 },
> > > + { CCI_REG8(0x0d27), 0x39 },
> > > + { CCI_REG8(0x0d22), 0x00 },
> > > + { CCI_REG8(0x03f6), 0x0d },
> > > + { CCI_REG8(0x0d04), 0x07 },
> > > + { CCI_REG8(0x03f3), 0x72 },
> > > + { CCI_REG8(0x03f4), 0xb8 },
> > > + { CCI_REG8(0x03f5), 0xbc },
> > > + { CCI_REG8(0x0d02), 0x73 },
> > > +
> >
> > Are any of those able to be broken out to named register to be more
> > clear in their intent?
> >
> > > + /* auto load start */
> > > + { CCI_REG8(0x00cb), 0xfc },
> > > +
> >
> > Why is this auto load start so different to the other modes 'auto
> > load
> > start'? What do the bits refer to ?
> >
> > > + /* OUT 1280x720 */
> > > + { CCI_REG8(0x0350), 0x01 },
> > > + { CCI_REG8(0x0353), 0x00 },
> > > + { CCI_REG8(0x0354), 0x0c },
> >
> > > + { CCI_REG8(0x034c), 0x05 },
> > > + { CCI_REG8(0x034d), 0x00 },
> >
> > Should/Could this be
> > { CCI_REG16(0x034c), 1280 },
> >
> > Are there any other register settings that would make more sense to
> > be
> > in decimal units that match their actual context?
> >
> >
> > > + { CCI_REG8(0x021f), 0x14 },
> >
> > I don't see a setting for 720/0x2d0. Do these registers only set the
> > width?
> >
> > > +
> > > + /* MIPI */
> > > + { CCI_REG8(0x0107), 0x05 },
> > > + { CCI_REG8(0x0117), 0x01 },
> > > + { CCI_REG8(0x0d81), 0x00 },
> > > + { CCI_REG8(0x0d84), 0x06 },
> > > + { CCI_REG8(0x0d85), 0x40 },
> > > + { CCI_REG8(0x0d86), 0x03 },
> > > + { CCI_REG8(0x0d87), 0x21 },
> > > + { CCI_REG8(0x0db3), 0x03 },
> > > + { CCI_REG8(0x0db4), 0x04 },
> > > + { CCI_REG8(0x0db5), 0x0d },
> > > + { CCI_REG8(0x0db6), 0x01 },
> > > + { CCI_REG8(0x0db8), 0x04 },
> > > + { CCI_REG8(0x0db9), 0x06 },
> > > + { CCI_REG8(0x0d93), 0x03 },
> > > + { CCI_REG8(0x0d94), 0x04 },
> > > + { CCI_REG8(0x0d95), 0x05 },
> > > + { CCI_REG8(0x0d99), 0x06 },
> > > + { CCI_REG8(0x0084), 0x01 },
> > > +
> > > + /* OUT */
> >
> > Out where? What is out?
> >
> > > + { CCI_REG8(0x0110), 0x01 },
> > > +};
> > > +
> > > +static const struct cci_reg_sequence mode_table_common[] = {
> > > + { GC05A2_STREAMING_REG, 0x00 },
> > > + /* system */
> > > + { CCI_REG8(0x0315), 0xd4 },
> > > + { CCI_REG8(0x0d06), 0x01 },
> > > + { CCI_REG8(0x0a70), 0x80 },
> > > + { CCI_REG8(0x031a), 0x00 },
> > > + { CCI_REG8(0x0314), 0x00 },
> > > + { CCI_REG8(0x0130), 0x08 },
> > > + { CCI_REG8(0x0132), 0x01 },
> > > + { CCI_REG8(0x0136), 0x38 },
> > > + { CCI_REG8(0x0137), 0x03 },
> > > + { CCI_REG8(0x0134), 0x5b },
> > > + { CCI_REG8(0x031c), 0xe0 },
> > > + { CCI_REG8(0x0d82), 0x14 },
> > > + { CCI_REG8(0x0dd1), 0x56 },
> > > +
> > > + /* gate_mode */
> > > + { CCI_REG8(0x0af4), 0x01 },
> > > + { CCI_REG8(0x0002), 0x10 },
> > > + { CCI_REG8(0x00c3), 0x34 },
> > > +
> > > + /* auto load start */
> >
> > The previous 'auto load start' referenced 0x00cb ?
> >
> > > + { CCI_REG8(0x00c4), 0x00 },
> > > + { CCI_REG8(0x00c5), 0x01 },
> > > + { CCI_REG8(0x0af6), 0x00 },
> > > + { CCI_REG8(0x0ba0), 0x17 },
> > > + { CCI_REG8(0x0ba1), 0x00 },
> > > + { CCI_REG8(0x0ba2), 0x00 },
> > > + { CCI_REG8(0x0ba3), 0x00 },
> > > + { CCI_REG8(0x0ba4), 0x03 },
> > > + { CCI_REG8(0x0ba5), 0x00 },
> > > + { CCI_REG8(0x0ba6), 0x00 },
> > > + { CCI_REG8(0x0ba7), 0x00 },
> > > + { CCI_REG8(0x0ba8), 0x40 },
> > > + { CCI_REG8(0x0ba9), 0x00 },
> > > + { CCI_REG8(0x0baa), 0x00 },
> > > + { CCI_REG8(0x0bab), 0x00 },
> > > + { CCI_REG8(0x0bac), 0x40 },
> > > + { CCI_REG8(0x0bad), 0x00 },
> > > + { CCI_REG8(0x0bae), 0x00 },
> > > + { CCI_REG8(0x0baf), 0x00 },
> > > + { CCI_REG8(0x0bb0), 0x02 },
> > > + { CCI_REG8(0x0bb1), 0x00 },
> > > + { CCI_REG8(0x0bb2), 0x00 },
> > > + { CCI_REG8(0x0bb3), 0x00 },
> > > + { CCI_REG8(0x0bb8), 0x02 },
> > > + { CCI_REG8(0x0bb9), 0x00 },
> > > + { CCI_REG8(0x0bba), 0x00 },
> > > + { CCI_REG8(0x0bbb), 0x00 },
> > > + { CCI_REG8(0x0a70), 0x80 },
> > > + { CCI_REG8(0x0a71), 0x00 },
> > > + { CCI_REG8(0x0a72), 0x00 },
> > > + { CCI_REG8(0x0a66), 0x00 },
> > > + { CCI_REG8(0x0a67), 0x80 },
> > > + { CCI_REG8(0x0a4d), 0x4e },
> > > + { CCI_REG8(0x0a50), 0x00 },
> > > + { CCI_REG8(0x0a4f), 0x0c },
> > > + { CCI_REG8(0x0a66), 0x00 },
> > > + { CCI_REG8(0x00ca), 0x00 },
> > > + { CCI_REG8(0x00cc), 0x00 },
> > > + { CCI_REG8(0x00cd), 0x00 },
> > > + { CCI_REG8(0x0aa1), 0x00 },
> > > + { CCI_REG8(0x0aa2), 0xe0 },
> > > + { CCI_REG8(0x0aa3), 0x00 },
> > > + { CCI_REG8(0x0aa4), 0x40 },
> > > + { CCI_REG8(0x0a90), 0x03 },
> > > + { CCI_REG8(0x0a91), 0x0e },
> > > + { CCI_REG8(0x0a94), 0x80 },
> > > +
> > > + /* standby */
> > > + { CCI_REG8(0x0af6), 0x20 },
> > > + { CCI_REG8(0x0b00), 0x91 },
> > > + { CCI_REG8(0x0b01), 0x17 },
> > > + { CCI_REG8(0x0b02), 0x01 },
> > > + { CCI_REG8(0x0b03), 0x00 },
> > > + { CCI_REG8(0x0b04), 0x01 },
> > > + { CCI_REG8(0x0b05), 0x17 },
> > > + { CCI_REG8(0x0b06), 0x01 },
> > > + { CCI_REG8(0x0b07), 0x00 },
> > > + { CCI_REG8(0x0ae9), 0x01 },
> > > + { CCI_REG8(0x0aea), 0x02 },
> > > + { CCI_REG8(0x0ae8), 0x53 },
> > > + { CCI_REG8(0x0ae8), 0x43 },
> > > +
> > > + /* gain_partition */
> > > + { CCI_REG8(0x0af6), 0x30 },
> > > + { CCI_REG8(0x0b00), 0x08 },
> > > + { CCI_REG8(0x0b01), 0x0f },
> > > + { CCI_REG8(0x0b02), 0x00 },
> > > + { CCI_REG8(0x0b04), 0x1c },
> > > + { CCI_REG8(0x0b05), 0x24 },
> > > + { CCI_REG8(0x0b06), 0x00 },
> > > + { CCI_REG8(0x0b08), 0x30 },
> > > + { CCI_REG8(0x0b09), 0x40 },
> > > + { CCI_REG8(0x0b0a), 0x00 },
> > > + { CCI_REG8(0x0b0c), 0x0e },
> > > + { CCI_REG8(0x0b0d), 0x2a },
> > > + { CCI_REG8(0x0b0e), 0x00 },
> > > + { CCI_REG8(0x0b10), 0x0e },
> > > + { CCI_REG8(0x0b11), 0x2b },
> > > + { CCI_REG8(0x0b12), 0x00 },
> > > + { CCI_REG8(0x0b14), 0x0e },
> > > + { CCI_REG8(0x0b15), 0x23 },
> > > + { CCI_REG8(0x0b16), 0x00 },
> > > + { CCI_REG8(0x0b18), 0x0e },
> > > + { CCI_REG8(0x0b19), 0x24 },
> > > + { CCI_REG8(0x0b1a), 0x00 },
> > > + { CCI_REG8(0x0b1c), 0x0c },
> > > + { CCI_REG8(0x0b1d), 0x0c },
> > > + { CCI_REG8(0x0b1e), 0x00 },
> > > + { CCI_REG8(0x0b20), 0x03 },
> > > + { CCI_REG8(0x0b21), 0x03 },
> > > + { CCI_REG8(0x0b22), 0x00 },
> > > + { CCI_REG8(0x0b24), 0x0e },
> > > + { CCI_REG8(0x0b25), 0x0e },
> > > + { CCI_REG8(0x0b26), 0x00 },
> > > + { CCI_REG8(0x0b28), 0x03 },
> > > + { CCI_REG8(0x0b29), 0x03 },
> > > + { CCI_REG8(0x0b2a), 0x00 },
> > > + { CCI_REG8(0x0b2c), 0x12 },
> > > + { CCI_REG8(0x0b2d), 0x12 },
> > > + { CCI_REG8(0x0b2e), 0x00 },
> > > + { CCI_REG8(0x0b30), 0x08 },
> > > + { CCI_REG8(0x0b31), 0x08 },
> > > + { CCI_REG8(0x0b32), 0x00 },
> > > + { CCI_REG8(0x0b34), 0x14 },
> > > + { CCI_REG8(0x0b35), 0x14 },
> > > + { CCI_REG8(0x0b36), 0x00 },
> > > + { CCI_REG8(0x0b38), 0x10 },
> > > + { CCI_REG8(0x0b39), 0x10 },
> > > + { CCI_REG8(0x0b3a), 0x00 },
> > > + { CCI_REG8(0x0b3c), 0x16 },
> > > + { CCI_REG8(0x0b3d), 0x16 },
> > > + { CCI_REG8(0x0b3e), 0x00 },
> > > + { CCI_REG8(0x0b40), 0x10 },
> > > + { CCI_REG8(0x0b41), 0x10 },
> > > + { CCI_REG8(0x0b42), 0x00 },
> > > + { CCI_REG8(0x0b44), 0x19 },
> > > + { CCI_REG8(0x0b45), 0x19 },
> > > + { CCI_REG8(0x0b46), 0x00 },
> > > + { CCI_REG8(0x0b48), 0x16 },
> > > + { CCI_REG8(0x0b49), 0x16 },
> > > + { CCI_REG8(0x0b4a), 0x00 },
> > > + { CCI_REG8(0x0b4c), 0x19 },
> > > + { CCI_REG8(0x0b4d), 0x19 },
> > > + { CCI_REG8(0x0b4e), 0x00 },
> > > + { CCI_REG8(0x0b50), 0x16 },
> > > + { CCI_REG8(0x0b51), 0x16 },
> > > + { CCI_REG8(0x0b52), 0x00 },
> > > + { CCI_REG8(0x0b80), 0x01 },
> > > + { CCI_REG8(0x0b81), 0x00 },
> > > + { CCI_REG8(0x0b82), 0x00 },
> > > + { CCI_REG8(0x0b84), 0x00 },
> > > + { CCI_REG8(0x0b85), 0x00 },
> > > + { CCI_REG8(0x0b86), 0x00 },
> > > + { CCI_REG8(0x0b88), 0x01 },
> > > + { CCI_REG8(0x0b89), 0x6a },
> > > + { CCI_REG8(0x0b8a), 0x00 },
> > > + { CCI_REG8(0x0b8c), 0x00 },
> > > + { CCI_REG8(0x0b8d), 0x01 },
> > > + { CCI_REG8(0x0b8e), 0x00 },
> > > + { CCI_REG8(0x0b90), 0x01 },
> > > + { CCI_REG8(0x0b91), 0xf6 },
> > > + { CCI_REG8(0x0b92), 0x00 },
> > > + { CCI_REG8(0x0b94), 0x00 },
> > > + { CCI_REG8(0x0b95), 0x02 },
> > > + { CCI_REG8(0x0b96), 0x00 },
> > > + { CCI_REG8(0x0b98), 0x02 },
> > > + { CCI_REG8(0x0b99), 0xc4 },
> > > + { CCI_REG8(0x0b9a), 0x00 },
> > > + { CCI_REG8(0x0b9c), 0x00 },
> > > + { CCI_REG8(0x0b9d), 0x03 },
> > > + { CCI_REG8(0x0b9e), 0x00 },
> > > + { CCI_REG8(0x0ba0), 0x03 },
> > > + { CCI_REG8(0x0ba1), 0xd8 },
> > > + { CCI_REG8(0x0ba2), 0x00 },
> > > + { CCI_REG8(0x0ba4), 0x00 },
> > > + { CCI_REG8(0x0ba5), 0x04 },
> > > + { CCI_REG8(0x0ba6), 0x00 },
> > > + { CCI_REG8(0x0ba8), 0x05 },
> > > + { CCI_REG8(0x0ba9), 0x4d },
> > > + { CCI_REG8(0x0baa), 0x00 },
> > > + { CCI_REG8(0x0bac), 0x00 },
> > > + { CCI_REG8(0x0bad), 0x05 },
> > > + { CCI_REG8(0x0bae), 0x00 },
> > > + { CCI_REG8(0x0bb0), 0x07 },
> > > + { CCI_REG8(0x0bb1), 0x3e },
> > > + { CCI_REG8(0x0bb2), 0x00 },
> > > + { CCI_REG8(0x0bb4), 0x00 },
> > > + { CCI_REG8(0x0bb5), 0x06 },
> > > + { CCI_REG8(0x0bb6), 0x00 },
> > > + { CCI_REG8(0x0bb8), 0x0a },
> > > + { CCI_REG8(0x0bb9), 0x1a },
> > > + { CCI_REG8(0x0bba), 0x00 },
> > > + { CCI_REG8(0x0bbc), 0x09 },
> > > + { CCI_REG8(0x0bbd), 0x36 },
> > > + { CCI_REG8(0x0bbe), 0x00 },
> > > + { CCI_REG8(0x0bc0), 0x0e },
> > > + { CCI_REG8(0x0bc1), 0x66 },
> > > + { CCI_REG8(0x0bc2), 0x00 },
> > > + { CCI_REG8(0x0bc4), 0x10 },
> > > + { CCI_REG8(0x0bc5), 0x06 },
> > > + { CCI_REG8(0x0bc6), 0x00 },
> > > + { CCI_REG8(0x02c1), 0xe0 },
> > > + { CCI_REG8(0x0207), 0x04 },
> > > + { CCI_REG8(0x02c2), 0x10 },
> > > + { CCI_REG8(0x02c3), 0x74 },
> > > + { CCI_REG8(0x02c5), 0x09 },
> > > + { CCI_REG8(0x02c1), 0xe0 },
> > > + { CCI_REG8(0x0207), 0x04 },
> > > + { CCI_REG8(0x02c2), 0x10 },
> > > + { CCI_REG8(0x02c5), 0x09 },
> > > + { CCI_REG8(0x02c1), 0xe0 },
> > > + { CCI_REG8(0x0207), 0x04 },
> > > + { CCI_REG8(0x02c2), 0x10 },
> > > + { CCI_REG8(0x02c5), 0x09 },
> > > +
> > > + /* auto load CH_GAIN */
> > > + { CCI_REG8(0x0aa1), 0x15 },
> > > + { CCI_REG8(0x0aa2), 0x50 },
> > > + { CCI_REG8(0x0aa3), 0x00 },
> > > + { CCI_REG8(0x0aa4), 0x09 },
> > > + { CCI_REG8(0x0a90), 0x25 },
> > > + { CCI_REG8(0x0a91), 0x0e },
> > > + { CCI_REG8(0x0a94), 0x80 },
> > > +
> > > + /* ISP */
> > > + { CCI_REG8(0x0050), 0x00 },
> > > + { CCI_REG8(0x0089), 0x83 },
> > > + { CCI_REG8(0x005a), 0x40 },
> > > + { CCI_REG8(0x00c3), 0x35 },
> > > + { CCI_REG8(0x00c4), 0x80 },
> > > + { CCI_REG8(0x0080), 0x10 },
> > > + { CCI_REG8(0x0040), 0x12 },
> > > + { CCI_REG8(0x0053), 0x0a },
> > > + { CCI_REG8(0x0054), 0x44 },
> > > + { CCI_REG8(0x0055), 0x32 },
> > > + { CCI_REG8(0x0058), 0x89 },
> > > + { CCI_REG8(0x004a), 0x03 },
> > > + { CCI_REG8(0x0048), 0xf0 },
> > > + { CCI_REG8(0x0049), 0x0f },
> > > + { CCI_REG8(0x0041), 0x20 },
> > > + { CCI_REG8(0x0043), 0x0a },
> > > + { CCI_REG8(0x009d), 0x08 },
> > > + { CCI_REG8(0x0236), 0x40 },
> > > +
> > > + /* gain */
> >
> > Is the gain configurable? Is this analogue gain? digital gain? or
> > colour
> > balanace gains ?
> >
> >
> > > + { CCI_REG8(0x0204), 0x04 },
> > > + { CCI_REG8(0x0205), 0x00 },
> > > + { CCI_REG8(0x02b3), 0x00 },
> > > + { CCI_REG8(0x02b4), 0x00 },
> > > + { CCI_REG8(0x009e), 0x01 },
> > > + { CCI_REG8(0x009f), 0x94 },
> > > +
> > > + /* auto load REG */
> > > + { CCI_REG8(0x0aa1), 0x10 },
> > > + { CCI_REG8(0x0aa2), 0xf8 },
> > > + { CCI_REG8(0x0aa3), 0x00 },
> > > + { CCI_REG8(0x0aa4), 0x1f },
> > > + { CCI_REG8(0x0a90), 0x11 },
> > > + { CCI_REG8(0x0a91), 0x0e },
> > > + { CCI_REG8(0x0a94), 0x80 },
> > > + { CCI_REG8(0x03fe), 0x00 },
> > > + { CCI_REG8(0x0a90), 0x00 },
> > > + { CCI_REG8(0x0a70), 0x00 },
> > > + { CCI_REG8(0x0a67), 0x00 },
> > > + { CCI_REG8(0x0af4), 0x29 },
> > > +
> > > + /* DPHY */
> > > + { CCI_REG8(0x0d80), 0x07 },
> > > + { CCI_REG8(0x0dd3), 0x18 },
> > > +
> > > + /* CISCTL_Reset */
> > > + { CCI_REG8(0x031c), 0x80 },
> > > + { CCI_REG8(0x03fe), 0x30 },
> > > + { CCI_REG8(0x0d17), 0x06 },
> > > + { CCI_REG8(0x03fe), 0x00 },
> > > + { CCI_REG8(0x0d17), 0x00 },
> > > + { CCI_REG8(0x031c), 0x93 },
> > > + { CCI_REG8(0x03fe), 0x00 },
> > > + { CCI_REG8(0x031c), 0x80 },
> > > + { CCI_REG8(0x03fe), 0x30 },
> > > + { CCI_REG8(0x0d17), 0x06 },
> > > + { CCI_REG8(0x03fe), 0x00 },
> > > + { CCI_REG8(0x0d17), 0x00 },
> > > + { CCI_REG8(0x031c), 0x93 },
> > > +};
> > > +
> > > +struct gc05a2_mode {
> > > + u32 width;
> > > + u32 height;
> > > + const struct gc05a2_reg_list reg_list;
> > > +
> > > + u32 hts; /* Horizontal timining size */
> > > + u32 vts_def; /* Default vertical timining size */
> > > + u32 vts_min; /* Min vertical timining size */
> > > +};
> > > +
> > > +/* Declare modes in order, from biggest to smallest height. */
> > > +static const struct gc05a2_mode gc05a2_modes[] = {
> > > + {
> > > + /* 2592*1944@30fps */
> > > + .width = GC05A2_NATIVE_WIDTH,
> > > + .height = GC05A2_NATIVE_HEIGHT,
> > > + .reg_list = {
> > > + .num_of_regs = ARRAY_SIZE(mode_2592x1944),
> > > + .regs = mode_2592x1944,
> > > + },
> > > + .hts = 3664,
> > > + .vts_def = 2032,
> > > + .vts_min = 2032,
> > > + },
> > > + {
> > > + /* 1280*720@60fps */
> > > + .width = 1280,
> > > + .height = 720,
> > > + .reg_list = {
> > > + .num_of_regs = ARRAY_SIZE(mode_1280x720),
> > > + .regs = mode_1280x720,
> > > + },
> > > + .hts = 3616,
> > > + .vts_def = 1032,
> > > + .vts_min = 1032,
> > > + },
> > > +};
> > > +
> > > +static inline struct gc05a2 *to_gc05a2(struct v4l2_subdev *sd)
> > > +{
> > > + return container_of(sd, struct gc05a2, sd);
> > > +}
> > > +
> > > +static int gc05a2_power_on(struct device *dev)
> > > +{
> > > + struct v4l2_subdev *sd = dev_get_drvdata(dev);
> > > + struct gc05a2 *gc05a2 = to_gc05a2(sd);
> > > + int ret;
> > > +
> > > + ret = regulator_bulk_enable(ARRAY_SIZE(gc05a2_supply_name),
> > > + gc05a2->supplies);
> > > + if (ret < 0) {
> > > + dev_err(gc05a2->dev, "failed to enable regulators:
> > %d\n", ret);
> > > + return ret;
> > > + }
> > > +
> > > + ret = clk_prepare_enable(gc05a2->xclk);
> > > + if (ret < 0) {
> > >
> > + regulator_bulk_disable(ARRAY_SIZE(gc05a2_supply_name)
> > ,
> > > + gc05a2->supplies);
> > > + dev_err(gc05a2->dev, "clk prepare enable
> > failed\n");
> > > + return ret;
> > > + }
> > > +
> > > + fsleep(GC05A2_SLEEP_US);
> > > +
> > > + gpiod_set_value_cansleep(gc05a2->reset_gpio, 0);
> > > + fsleep(GC05A2_SLEEP_US);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int gc05a2_power_off(struct device *dev)
> > > +{
> > > + struct v4l2_subdev *sd = dev_get_drvdata(dev);
> > > + struct gc05a2 *gc05a2 = to_gc05a2(sd);
> > > +
> > > + clk_disable_unprepare(gc05a2->xclk);
> > > + gpiod_set_value_cansleep(gc05a2->reset_gpio, 1);
> > > + regulator_bulk_disable(ARRAY_SIZE(gc05a2_supply_name),
> > > + gc05a2->supplies);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int gc05a2_enum_mbus_code(struct v4l2_subdev *sd,
> > > + struct v4l2_subdev_state
> > *sd_state,
> > > + struct v4l2_subdev_mbus_code_enum
> > *code)
> > > +{
> > > + if (code->index > 0)
> > > + return -EINVAL;
> > > +
> > > + code->code = GC05A2_MBUS_CODE;
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int gc05a2_enum_frame_size(struct v4l2_subdev *subdev,
> > > + struct v4l2_subdev_state
> > *sd_state,
> > > + struct
> > v4l2_subdev_frame_size_enum *fse)
> > > +{
> > > + if (fse->code != GC05A2_MBUS_CODE)
> > > + return -EINVAL;
> > > +
> > > + if (fse->index >= ARRAY_SIZE(gc05a2_modes))
> > > + return -EINVAL;
> > > +
> > > + fse->min_width = gc05a2_modes[fse->index].width;
> > > + fse->max_width = gc05a2_modes[fse->index].width;
> > > + fse->min_height = gc05a2_modes[fse->index].height;
> > > + fse->max_height = gc05a2_modes[fse->index].height;
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int gc05a2_update_cur_mode_controls(struct gc05a2 *gc05a2,
> > > + const struct gc05a2_mode
> > *mode)
> > > +{
> > > + s64 exposure_max, h_blank;
> > > + int ret;
> > > +
> > > + ret = __v4l2_ctrl_modify_range(gc05a2->vblank,
> > > + mode->vts_min - mode-
> > >height,
> > > + GC05A2_VTS_MAX - mode-
> > >height, 1,
> > > + mode->vts_def - mode-
> > >height);
> > > + if (ret) {
> > > + dev_err(gc05a2->dev, "VB ctrl range update
> > failed\n");
> > > + return ret;
> > > + }
> > > +
> > > + h_blank = mode->hts - mode->width;
> > > + ret = __v4l2_ctrl_modify_range(gc05a2->hblank, h_blank,
> > h_blank, 1,
> > > + h_blank);
> > > + if (ret) {
> > > + dev_err(gc05a2->dev, "HB ctrl range update
> > failed\n");
> > > + return ret;
> > > + }
> > > +
> > > + exposure_max = mode->vts_def - GC05A2_EXP_MARGIN;
> > > + ret = __v4l2_ctrl_modify_range(gc05a2->exposure,
> > GC05A2_EXP_MIN,
> > > + exposure_max,
> > GC05A2_EXP_STEP,
> > > + exposure_max);
> > > + if (ret) {
> > > + dev_err(gc05a2->dev, "exposure ctrl range update
> > failed\n");
> > > + return ret;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static void gc05a2_update_pad_format(struct gc05a2 *gc08a3,
> > > + const struct gc05a2_mode
> > *mode,
> > > + struct v4l2_mbus_framefmt
> > *fmt)
> > > +{
> > > + fmt->width = mode->width;
> > > + fmt->height = mode->height;
> > > + fmt->code = GC05A2_MBUS_CODE;
> > > + fmt->field = V4L2_FIELD_NONE;
> > > + fmt->colorspace = V4L2_COLORSPACE_RAW;
> > > + fmt->ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(fmt-
> > >colorspace);
> > > + fmt->quantization = V4L2_QUANTIZATION_FULL_RANGE;
> > > + fmt->xfer_func = V4L2_XFER_FUNC_NONE;
> > > +}
> > > +
> > > +static int gc05a2_set_format(struct v4l2_subdev *sd,
> > > + struct v4l2_subdev_state *state,
> > > + struct v4l2_subdev_format *fmt)
> > > +{
> > > + struct gc05a2 *gc05a2 = to_gc05a2(sd);
> > > + struct v4l2_mbus_framefmt *mbus_fmt;
> > > + struct v4l2_rect *crop;
> > > + const struct gc05a2_mode *mode;
> > > +
> > > + mode = v4l2_find_nearest_size(gc05a2_modes,
> > ARRAY_SIZE(gc05a2_modes),
> > > + width, height, fmt-
> > >format.width,
> > > + fmt->format.height);
> > > +
> > > + /* update crop info to subdev state */
> > > + crop = v4l2_subdev_state_get_crop(state, 0);
> > > + crop->width = mode->width;
> > > + crop->height = mode->height;
> > > +
> > > + /* update fmt info to subdev state */
> > > + gc05a2_update_pad_format(gc05a2, mode, &fmt->format);
> > > + mbus_fmt = v4l2_subdev_state_get_format(state, 0);
> > > + *mbus_fmt = fmt->format;
> > > +
> > > + if (fmt->which == V4L2_SUBDEV_FORMAT_TRY)
> > > + return 0;
> > > + gc05a2->cur_mode = mode;
> > > + gc05a2_update_cur_mode_controls(gc05a2, mode);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int gc05a2_get_selection(struct v4l2_subdev *sd,
> > > + struct v4l2_subdev_state *state,
> > > + struct v4l2_subdev_selection *sel)
> > > +{
> > > + switch (sel->target) {
> > > + case V4L2_SEL_TGT_CROP_DEFAULT:
> > > + case V4L2_SEL_TGT_CROP:
> > > + sel->r = *v4l2_subdev_state_get_crop(state, 0);
> > > + break;
> > > + case V4L2_SEL_TGT_CROP_BOUNDS:
> > > + sel->r.top = 0;
> > > + sel->r.left = 0;
> > > + sel->r.width = GC05A2_NATIVE_WIDTH;
> > > + sel->r.height = GC05A2_NATIVE_HEIGHT;
> > > + break;
> > > + default:
> > > + return -EINVAL;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int gc05a2_init_state(struct v4l2_subdev *sd,
> > > + struct v4l2_subdev_state *state)
> > > +{
> > > + struct v4l2_subdev_format fmt = {
> > > + .which = V4L2_SUBDEV_FORMAT_TRY,
> > > + .pad = 0,
> > > + .format = {
> > > + .code = GC05A2_MBUS_CODE,
> > > + .width = gc05a2_modes[0].width,
> > > + .height = gc05a2_modes[0].height,
> > > + },
> > > + };
> > > +
> > > + gc05a2_set_format(sd, state, &fmt);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int gc05a2_set_ctrl_hflip(struct gc05a2 *gc05a2, u32
> > ctrl_val)
> > > +{
> > > + int ret;
> > > + u64 val;
> > > +
> > > + ret = cci_read(gc05a2->regmap, GC05A2_FLIP_REG, &val,
> > NULL);
> > > + if (ret) {
> > > + dev_err(gc05a2->dev, "read hflip register failed:
> > %d\n", ret);
> > > + return ret;
> > > + }
> > > +
> > > + return cci_update_bits(gc05a2->regmap, GC05A2_FLIP_REG,
> > > + GC05A2_FLIP_H_MASK,
> > > + ctrl_val ? GC05A2_FLIP_H_MASK : 0,
> > NULL);
> > > +}
> > > +
> > > +static int gc05a2_set_ctrl_vflip(struct gc05a2 *gc05a2, u32
> > ctrl_val)
> > > +{
> > > + int ret;
> > > + u64 val;
> > > +
> > > + ret = cci_read(gc05a2->regmap, GC05A2_FLIP_REG, &val,
> > NULL);
> > > + if (ret) {
> > > + dev_err(gc05a2->dev, "read vflip register failed:
> > %d\n", ret);
> > > + return ret;
> > > + }
> > > +
> > > + return cci_update_bits(gc05a2->regmap, GC05A2_FLIP_REG,
> > > + GC05A2_FLIP_V_MASK,
> > > + ctrl_val ? GC05A2_FLIP_V_MASK : 0,
> > NULL);
> > > +}
> > > +
> > > +static int gc05a2_test_pattern(struct gc05a2 *gc05a2, u32
> > pattern_menu)
> > > +{
> > > + u32 pattern;
> > > + int ret;
> > > +
> > > + if (pattern_menu) {
> > > + switch (pattern_menu) {
> > > + case 1:
> > > + case 2:
> > > + case 3:
> > > + case 4:
> > > + case 5:
> > > + case 6:
> > > + case 7:
> > > + pattern = pattern_menu << 4;
> > > + break;
> > > +
> > > + case 8:
> > > + pattern = 0;
> > > + break;
> > > +
> > > + case 9:
> > > + pattern = 4;
> > > + break;
> > > +
> > > + default:
> > > + pattern = 0x00;
> > > + break;
> > > + }
> >
> > This is fairly terse. Can we add comments, or definitions for the
> > types
> > or such so that the above is easier to interpret?
> >
> > > +
> > > + ret = cci_write(gc05a2->regmap,
> > GC05A2_REG_TEST_PATTERN_IDX,
> > > + pattern, NULL);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + return cci_write(gc05a2->regmap,
> > GC05A2_REG_TEST_PATTERN_EN,
> > > + GC05A2_TEST_PATTERN_EN, NULL);
> > > + } else {
> > > + return cci_write(gc05a2->regmap,
> > GC05A2_REG_TEST_PATTERN_EN,
> > > + 0x00, NULL);
> > > + }
> > > +}
> > > +
> > > +static int gc05a2_set_ctrl(struct v4l2_ctrl *ctrl)
> > > +{
> > > + struct gc05a2 *gc05a2 =
> > > + container_of(ctrl->handler, struct gc05a2, ctrls);
> > > + int ret = 0;
> > > + s64 exposure_max;
> > > + struct v4l2_subdev_state *state;
> > > + const struct v4l2_mbus_framefmt *format;
> > > +
> > > + state = v4l2_subdev_get_locked_active_state(&gc05a2->sd);
> > > + format = v4l2_subdev_state_get_format(state, 0);
> > > +
> > > + if (ctrl->id == V4L2_CID_VBLANK) {
> > > + /* Update max exposure while meeting expected
> > vblanking */
> > > + exposure_max = format->height + ctrl->val -
> > GC05A2_EXP_MARGIN;
> > > + __v4l2_ctrl_modify_range(gc05a2->exposure,
> > > + gc05a2->exposure->minimum,
> > > + exposure_max, gc05a2-
> > >exposure->step,
> > > + exposure_max);
> > > + }
> > > +
> > > + /*
> > > + * Applying V4L2 control value only happens
> > > + * when power is on for streaming.
> > > + */
> > > + if (!pm_runtime_get_if_active(gc05a2->dev))
> > > + return 0;
> > > +
> > > + switch (ctrl->id) {
> > > + case V4L2_CID_EXPOSURE:
> > > + ret = cci_write(gc05a2->regmap, GC05A2_EXP_REG,
> > > + ctrl->val, NULL);
> > > + break;
> > > +
> > > + case V4L2_CID_ANALOGUE_GAIN:
> > > + ret = cci_write(gc05a2->regmap, GC05A2_AGAIN_REG,
> > > + ctrl->val, NULL);
> > > + break;
> > > +
> > > + case V4L2_CID_VBLANK:
> > > + ret = cci_write(gc05a2->regmap,
> > GC05A2_FRAME_LENGTH_REG,
> > > + gc05a2->cur_mode->height + ctrl-
> > >val, NULL);
> > > + break;
> > > +
> > > + case V4L2_CID_HFLIP:
> > > + ret = gc05a2_set_ctrl_hflip(gc05a2, ctrl->val);
> > > + break;
> > > +
> > > + case V4L2_CID_VFLIP:
> > > + ret = gc05a2_set_ctrl_vflip(gc05a2, ctrl->val);
> > > + break;
> > > +
> > > + case V4L2_CID_TEST_PATTERN:
> > > + ret = gc05a2_test_pattern(gc05a2, ctrl->val);
> > > + break;
> > > +
> > > + default:
> > > + break;
> > > + }
> > > +
> > > + pm_runtime_put(gc05a2->dev);
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +static const struct v4l2_ctrl_ops gc05a2_ctrl_ops = {
> > > + .s_ctrl = gc05a2_set_ctrl,
> > > +};
> > > +
> > > +static int gc05a2_identify_module(struct gc05a2 *gc05a2)
> > > +{
> > > + u64 val;
> > > + int ret;
> > > +
> > > + if (gc05a2->identified)
> > > + return 0;
> > > +
> > > + ret = cci_read(gc05a2->regmap, GC05A2_REG_CHIP_ID, &val,
> > NULL);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + if (val != GC05A2_CHIP_ID) {
> > > + dev_err(gc05a2->dev, "chip id mismatch:
> > 0x%x!=0x%llx",
> > > + GC05A2_CHIP_ID, val);
> > > + return -ENXIO;
> > > + }
> > > +
> > > + gc05a2->identified = true;
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int gc05a2_start_streaming(struct gc05a2 *gc05a2)
> > > +{
> > > + const struct gc05a2_mode *mode;
> > > + const struct gc05a2_reg_list *reg_list;
> > > + int ret;
> > > +
> > > + ret = pm_runtime_resume_and_get(gc05a2->dev);
> > > + if (ret < 0)
> > > + return ret;
> > > +
> > > + ret = gc05a2_identify_module(gc05a2);
> > > + if (ret)
> > > + goto err_rpm_put;
> > > +
> > > + ret = cci_multi_reg_write(gc05a2->regmap,
> > > + mode_table_common,
> > > + ARRAY_SIZE(mode_table_common),
> > NULL);
> > > + if (ret)
> > > + goto err_rpm_put;
> > > +
> > > + mode = gc05a2->cur_mode;
> > > + reg_list = &mode->reg_list;
> > > +
> > > + ret = cci_multi_reg_write(gc05a2->regmap,
> > > + reg_list->regs, reg_list-
> > >num_of_regs, NULL);
> > > + if (ret < 0)
> > > + goto err_rpm_put;
> > > +
> > > + ret = __v4l2_ctrl_handler_setup(&gc05a2->ctrls);
> > > + if (ret < 0) {
> > > + dev_err(gc05a2->dev, "could not sync v4l2
> > controls\n");
> > > + goto err_rpm_put;
> > > + }
> > > +
> > > + ret = cci_write(gc05a2->regmap, GC05A2_STREAMING_REG, 1,
> > NULL);
> > > + if (ret < 0) {
> > > + dev_err(gc05a2->dev, "write STREAMING_REG failed:
> > %d\n", ret);
> > > + goto err_rpm_put;
> > > + }
> > > +
> > > + return 0;
> > > +
> > > +err_rpm_put:
> > > + pm_runtime_put(gc05a2->dev);
> > > + return ret;
> > > +}
> > > +
> > > +static int gc05a2_stop_streaming(struct gc05a2 *gc05a2)
> > > +{
> > > + int ret;
> > > +
> > > + ret = cci_write(gc05a2->regmap, GC05A2_STREAMING_REG, 0,
> > NULL);
> > > + if (ret < 0)
> > > + dev_err(gc05a2->dev, "could not sent stop streaming
> > %d\n", ret);
> > > +
> > > + pm_runtime_put(gc05a2->dev);
> > > + return ret;
> > > +}
> > > +
> > > +static int gc05a2_s_stream(struct v4l2_subdev *subdev, int enable)
> > > +{
> > > + struct gc05a2 *gc05a2 = to_gc05a2(subdev);
> > > + struct v4l2_subdev_state *state;
> > > + int ret;
> > > +
> > > + state = v4l2_subdev_lock_and_get_active_state(subdev);
> > > +
> > > + if (enable)
> > > + ret = gc05a2_start_streaming(gc05a2);
> > > + else
> > > + ret = gc05a2_stop_streaming(gc05a2);
> > > +
> > > + v4l2_subdev_unlock_state(state);
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +static const struct v4l2_subdev_video_ops gc05a2_video_ops = {
> > > + .s_stream = gc05a2_s_stream,
> > > +};
> > > +
> > > +static const struct v4l2_subdev_pad_ops gc05a2_subdev_pad_ops = {
> > > + .enum_mbus_code = gc05a2_enum_mbus_code,
> > > + .enum_frame_size = gc05a2_enum_frame_size,
> > > + .get_fmt = v4l2_subdev_get_fmt,
> > > + .set_fmt = gc05a2_set_format,
> > > + .get_selection = gc05a2_get_selection,
> > > +};
> > > +
> > > +static const struct v4l2_subdev_core_ops gc05a2_core_ops = {
> > > + .subscribe_event = v4l2_ctrl_subdev_subscribe_event,
> > > + .unsubscribe_event = v4l2_event_subdev_unsubscribe,
> > > +};
> > > +
> > > +static const struct v4l2_subdev_ops gc05a2_subdev_ops = {
> > > + .core = &gc05a2_core_ops,
> > > + .video = &gc05a2_video_ops,
> > > + .pad = &gc05a2_subdev_pad_ops,
> > > +};
> > > +
> > > +static const struct v4l2_subdev_internal_ops gc05a2_internal_ops =
> > {
> > > + .init_state = gc05a2_init_state,
> > > +};
> > > +
> > > +static int gc05a2_get_regulators(struct device *dev, struct gc05a2
> > *gc05a2)
> > > +{
> > > + unsigned int i;
> > > +
> > > + for (i = 0; i < ARRAY_SIZE(gc05a2_supply_name); i++)
> > > + gc05a2->supplies[i].supply = gc05a2_supply_name[i];
> > > +
> > > + return devm_regulator_bulk_get(dev,
> > ARRAY_SIZE(gc05a2_supply_name),
> > > + gc05a2->supplies);
> > > +}
> > > +
> > > +static int gc05a2_parse_fwnode(struct gc05a2 *gc05a2)
> > > +{
> > > + struct fwnode_handle *endpoint;
> > > + struct v4l2_fwnode_endpoint bus_cfg = {
> > > + .bus_type = V4L2_MBUS_CSI2_DPHY,
> > > + };
> > > + int ret;
> > > + struct device *dev = gc05a2->dev;
> > > +
> > > + endpoint =
> > > + fwnode_graph_get_endpoint_by_id(dev_fwnode(dev), 0,
> > 0,
> > >
> > + FWNODE_GRAPH_ENDPOINT
> > _NEXT);
> > > + if (!endpoint) {
> > > + dev_err(dev, "endpoint node not found\n");
> > > + return -EINVAL;
> > > + }
> > > +
> > > + ret = v4l2_fwnode_endpoint_alloc_parse(endpoint, &bus_cfg);
> > > + if (ret) {
> > > + dev_err(dev, "parsing endpoint node failed\n");
> > > + goto done;
> > > + }
> > > +
> > > + ret = v4l2_link_freq_to_bitmap(dev,
> > bus_cfg.link_frequencies,
> > >
> > + bus_cfg.nr_of_link_frequencies
> > ,
> > > + gc05a2_link_freq_menu_items,
> > >
> > + ARRAY_SIZE(gc05a2_link_freq_me
> > nu_items),
> > > + &gc05a2->link_freq_bitmap);
> > > + if (ret)
> > > + goto done;
> > > +
> > > +done:
> > > + v4l2_fwnode_endpoint_free(&bus_cfg);
> > > + fwnode_handle_put(endpoint);
> > > + return ret;
> > > +}
> > > +
> > > +static u64 gc05a2_to_pixel_rate(u32 f_index)
> > > +{
> > > + u64 pixel_rate =
> > > + gc05a2_link_freq_menu_items[f_index] * 2 *
> > GC05A2_DATA_LANES;
> > > +
> > > + return div_u64(pixel_rate, GC05A2_RGB_DEPTH);
> > > +}
> > > +
> > > +static int gc05a2_init_controls(struct gc05a2 *gc05a2)
> > > +{
> > > + struct i2c_client *client = v4l2_get_subdevdata(&gc05a2-
> > >sd);
> > > + const struct gc05a2_mode *mode = &gc05a2_modes[0];
> > > + const struct v4l2_ctrl_ops *ops = &gc05a2_ctrl_ops;
> > > + struct v4l2_fwnode_device_properties props;
> > > + struct v4l2_ctrl_handler *ctrl_hdlr;
> > > + s64 exposure_max, h_blank;
> > > + int ret;
> > > +
> > > + ctrl_hdlr = &gc05a2->ctrls;
> > > + ret = v4l2_ctrl_handler_init(ctrl_hdlr, 9);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + gc05a2->hflip = v4l2_ctrl_new_std(ctrl_hdlr,
> > &gc05a2_ctrl_ops,
> > > + V4L2_CID_HFLIP, 0, 1, 1,
> > 0);
> > > + gc05a2->vflip = v4l2_ctrl_new_std(ctrl_hdlr,
> > &gc05a2_ctrl_ops,
> > > + V4L2_CID_VFLIP, 0, 1, 1,
> > 0);
> > > + v4l2_ctrl_cluster(2, &gc05a2->hflip);
> > > +
> > > + gc05a2->link_freq =
> > > + v4l2_ctrl_new_int_menu(ctrl_hdlr,
> > > + &gc05a2_ctrl_ops,
> > > + V4L2_CID_LINK_FREQ,
> > >
> > + ARRAY_SIZE(gc05a2_link_freq_menu_items
> > ) - 1,
> > > + 0,
> > > + gc05a2_link_freq_menu_items);
> > > + if (gc05a2->link_freq)
> > > + gc05a2->link_freq->flags |=
> > V4L2_CTRL_FLAG_READ_ONLY;
> > > +
> > > + gc05a2->pixel_rate =
> > > + v4l2_ctrl_new_std(ctrl_hdlr,
> > > + &gc05a2_ctrl_ops,
> > > + V4L2_CID_PIXEL_RATE, 0,
> > > + gc05a2_to_pixel_rate(0),
> > > + 1,
> > > + gc05a2_to_pixel_rate(0));
> > > +
> > > + gc05a2->vblank =
> > > + v4l2_ctrl_new_std(ctrl_hdlr,
> > > + &gc05a2_ctrl_ops,
> > V4L2_CID_VBLANK,
> > > + mode->vts_min - mode->height,
> > > + GC05A2_VTS_MAX - mode->height, 1,
> > > + mode->vts_def - mode->height);
> > > +
> > > + h_blank = mode->hts - mode->width;
> > > + gc05a2->hblank = v4l2_ctrl_new_std(ctrl_hdlr,
> > &gc05a2_ctrl_ops,
> > > + V4L2_CID_HBLANK,
> > h_blank, h_blank, 1,
> > > + h_blank);
> > > + if (gc05a2->hblank)
> > > + gc05a2->hblank->flags |= V4L2_CTRL_FLAG_READ_ONLY;
> > > +
> > > + v4l2_ctrl_new_std(ctrl_hdlr, &gc05a2_ctrl_ops,
> > > + V4L2_CID_ANALOGUE_GAIN, GC05A2_AGAIN_MIN,
> > > + GC05A2_AGAIN_MAX, GC05A2_AGAIN_STEP,
> > > + GC05A2_AGAIN_MIN);
> > > +
> > > + exposure_max = mode->vts_def - GC05A2_EXP_MARGIN;
> > > + gc05a2->exposure = v4l2_ctrl_new_std(ctrl_hdlr,
> > &gc05a2_ctrl_ops,
> > > + V4L2_CID_EXPOSURE,
> > GC05A2_EXP_MIN,
> > > + exposure_max,
> > GC05A2_EXP_STEP,
> > > + exposure_max);
> > > +
> > > + v4l2_ctrl_new_std_menu_items(ctrl_hdlr, &gc05a2_ctrl_ops,
> > > + V4L2_CID_TEST_PATTERN,
> > >
> > + ARRAY_SIZE(gc05a2_test_pattern_m
> > enu) - 1,
> > > + 0, 0,
> > gc05a2_test_pattern_menu);
> > > +
> > > + /* register properties to fwnode (e.g. rotation,
> > orientation) */
> > > + ret = v4l2_fwnode_device_parse(&client->dev, &props);
> > > + if (ret)
> > > + goto error_ctrls;
> > > +
> > > + ret = v4l2_ctrl_new_fwnode_properties(ctrl_hdlr, ops,
> > &props);
> > > + if (ret)
> > > + goto error_ctrls;
> > > +
> > > + if (ctrl_hdlr->error) {
> > > + ret = ctrl_hdlr->error;
> > > + goto error_ctrls;
> > > + }
> > > +
> > > + gc05a2->sd.ctrl_handler = ctrl_hdlr;
> > > +
> > > + return 0;
> > > +
> > > +error_ctrls:
> > > + v4l2_ctrl_handler_free(ctrl_hdlr);
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +static int gc05a2_probe(struct i2c_client *client)
> > > +{
> > > + struct device *dev = &client->dev;
> > > + struct gc05a2 *gc05a2;
> > > + int ret;
> > > +
> > > + gc05a2 = devm_kzalloc(dev, sizeof(*gc05a2), GFP_KERNEL);
> > > + if (!gc05a2)
> > > + return -ENOMEM;
> > > +
> > > + gc05a2->dev = dev;
> > > +
> > > + ret = gc05a2_parse_fwnode(gc05a2);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + gc05a2->regmap = devm_cci_regmap_init_i2c(client, 16);
> > > + if (IS_ERR(gc05a2->regmap))
> > > + return dev_err_probe(dev, PTR_ERR(gc05a2->regmap),
> > > + "failed to init CCI\n");
> > > +
> > > + gc05a2->xclk = devm_clk_get(dev, NULL);
> > > + if (IS_ERR(gc05a2->xclk))
> > > + return dev_err_probe(dev, PTR_ERR(gc05a2->xclk),
> > > + "failed to get xclk\n");
> > > +
> > > + ret = clk_set_rate(gc05a2->xclk, GC05A2_DEFAULT_CLK_FREQ);
> > > + if (ret)
> > > + return dev_err_probe(dev, ret,
> > > + "failed to set xclk
> > frequency\n");
> > > +
> > > + ret = gc05a2_get_regulators(dev, gc05a2);
> > > + if (ret < 0)
> > > + return dev_err_probe(dev, ret,
> > > + "failed to get regulators\n");
> > > +
> > > + gc05a2->reset_gpio = devm_gpiod_get(dev, "reset",
> > GPIOD_OUT_LOW);
> > > + if (IS_ERR(gc05a2->reset_gpio))
> > > + return dev_err_probe(dev, PTR_ERR(gc05a2-
> > >reset_gpio),
> > > + "failed to get gpio\n");
> > > +
> > > + v4l2_i2c_subdev_init(&gc05a2->sd, client,
> > &gc05a2_subdev_ops);
> > > + gc05a2->sd.internal_ops = &gc05a2_internal_ops;
> > > + gc05a2->cur_mode = &gc05a2_modes[0];
> > > +
> > > + ret = gc05a2_init_controls(gc05a2);
> > > + if (ret)
> > > + return dev_err_probe(dev, ret,
> > > + "failed to init controls\n");
> > > +
> > > + gc05a2->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE |
> > > + V4L2_SUBDEV_FL_HAS_EVENTS;
> > > + gc05a2->pad.flags = MEDIA_PAD_FL_SOURCE;
> > > + gc05a2->sd.dev = &client->dev;
> > > + gc05a2->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
> > > +
> > > + ret = media_entity_pads_init(&gc05a2->sd.entity, 1,
> > &gc05a2->pad);
> > > + if (ret < 0) {
> > > + dev_err(dev, "could not register media entity\n");
> > > + goto err_v4l2_ctrl_handler_free;
> > > + }
> > > +
> > > + gc05a2->sd.state_lock = gc05a2->ctrls.lock;
> > > + ret = v4l2_subdev_init_finalize(&gc05a2->sd);
> > > + if (ret < 0) {
> > > + dev_err(dev, "v4l2 subdev init error: %d\n", ret);
> > > + goto err_media_entity_cleanup;
> > > + }
> > > +
> > > + pm_runtime_set_active(gc05a2->dev);
> > > + pm_runtime_enable(gc05a2->dev);
> > > + pm_runtime_set_autosuspend_delay(gc05a2->dev, 1000);
> > > + pm_runtime_use_autosuspend(gc05a2->dev);
> > > + pm_runtime_idle(gc05a2->dev);
> > > +
> > > + ret = v4l2_async_register_subdev_sensor(&gc05a2->sd);
> > > + if (ret < 0) {
> > > + dev_err(dev, "could not register v4l2 device\n");
> > > + goto err_rpm;
> > > + }
> > > +
> > > + return 0;
> > > +
> > > +err_rpm:
> > > + pm_runtime_disable(gc05a2->dev);
> > > + v4l2_subdev_cleanup(&gc05a2->sd);
> > > +
> > > +err_media_entity_cleanup:
> > > + media_entity_cleanup(&gc05a2->sd.entity);
> > > +
> > > +err_v4l2_ctrl_handler_free:
> > > + v4l2_ctrl_handler_free(&gc05a2->ctrls);
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +static void gc05a2_remove(struct i2c_client *client)
> > > +{
> > > + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > > + struct gc05a2 *gc05a2 = to_gc05a2(sd);
> > > +
> > > + v4l2_async_unregister_subdev(&gc05a2->sd);
> > > + v4l2_subdev_cleanup(sd);
> > > + media_entity_cleanup(&gc05a2->sd.entity);
> > > + v4l2_ctrl_handler_free(&gc05a2->ctrls);
> > > +
> > > + pm_runtime_disable(&client->dev);
> > > + if (!pm_runtime_status_suspended(&client->dev))
> > > + gc05a2_power_off(gc05a2->dev);
> > > + pm_runtime_set_suspended(&client->dev);
> > > +}
> > > +
> > > +static const struct of_device_id gc05a2_of_match[] = {
> > > + { .compatible = "galaxycore,gc05a2" },
> > > + {}
> > > +};
> > > +MODULE_DEVICE_TABLE(of, gc05a2_of_match);
> > > +
> > > +static DEFINE_RUNTIME_DEV_PM_OPS(gc05a2_pm_ops,
> > > + gc05a2_power_off,
> > > + gc05a2_power_on,
> > > + NULL);
> > > +
> > > +static struct i2c_driver gc05a2_i2c_driver = {
> > > + .driver = {
> > > + .of_match_table = gc05a2_of_match,
> > > + .pm = pm_ptr(&gc05a2_pm_ops),
> > > + .name = "gc05a2",
> > > + },
> > > + .probe = gc05a2_probe,
> > > + .remove = gc05a2_remove,
> > > +};
> > > +module_i2c_driver(gc05a2_i2c_driver);
> > > +
> > > +MODULE_DESCRIPTION("GalaxyCore gc05a2 Camera driver");
> > > +MODULE_AUTHOR("Zhi Mao <zhi.mao@mediatek.com>");
> > > +MODULE_LICENSE("GPL");
> > > --
> > > 2.25.1
> > >
^ permalink raw reply
* [PATCH v20 2/9] usb: dwc3: core: Access XHCI address space temporarily to read port info
From: Krishna Kurapati @ 2024-04-08 13:29 UTC (permalink / raw)
To: Krzysztof Kozlowski, Rob Herring, Bjorn Andersson, Wesley Cheng,
Konrad Dybcio, Greg Kroah-Hartman, Conor Dooley, Thinh Nguyen,
Felipe Balbi, Johan Hovold
Cc: devicetree, linux-arm-msm, linux-usb, linux-kernel, quic_ppratap,
quic_jackp, Krishna Kurapati, Bjorn Andersson
In-Reply-To: <20240408132925.1880571-1-quic_kriskura@quicinc.com>
All DWC3 Multi Port controllers that exist today only support host mode.
Temporarily map XHCI address space for host-only controllers and parse
XHCI Extended Capabilities registers to read number of usb2 ports and
usb3 ports present on multiport controller. Each USB Port is at least HS
capable.
The port info for usb2 and usb3 phy are identified as num_usb2_ports
and num_usb3_ports. The intention is as follows:
Wherever we need to perform phy operations like:
LOOP_OVER_NUMBER_OF_AVAILABLE_PORTS()
{
phy_set_mode(dwc->usb2_generic_phy[i], PHY_MODE_USB_HOST);
phy_set_mode(dwc->usb3_generic_phy[i], PHY_MODE_USB_HOST);
}
If number of usb2 ports is 3, loop can go from index 0-2 for
usb2_generic_phy. If number of usb3-ports is 2, we don't know for sure,
if the first 2 ports are SS capable or some other ports like (2 and 3)
are SS capable. So instead, num_usb2_ports is used to loop around all
phy's (both hs and ss) for performing phy operations. If any
usb3_generic_phy turns out to be NULL, phy operation just bails out.
num_usb3_ports is used to modify GUSB3PIPECTL registers while setting up
phy's as we need to know how many SS capable ports are there for this.
Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>
Reviewed-by: Bjorn Andersson <quic_bjorande@quicinc.com>
---
drivers/usb/dwc3/core.c | 61 +++++++++++++++++++++++++++++++++++++++++
drivers/usb/dwc3/core.h | 5 ++++
2 files changed, 66 insertions(+)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 31684cdaaae3..ddab30531f8a 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -39,6 +39,7 @@
#include "io.h"
#include "debug.h"
+#include "../host/xhci-ext-caps.h"
#define DWC3_DEFAULT_AUTOSUSPEND_DELAY 5000 /* ms */
@@ -1881,10 +1882,56 @@ static int dwc3_get_clocks(struct dwc3 *dwc)
return 0;
}
+static int dwc3_get_num_ports(struct dwc3 *dwc)
+{
+ void __iomem *base;
+ u8 major_revision;
+ u32 offset;
+ u32 val;
+
+ /*
+ * Remap xHCI address space to access XHCI ext cap regs since it is
+ * needed to get information on number of ports present.
+ */
+ base = ioremap(dwc->xhci_resources[0].start,
+ resource_size(&dwc->xhci_resources[0]));
+ if (!base)
+ return -ENOMEM;
+
+ offset = 0;
+ do {
+ offset = xhci_find_next_ext_cap(base, offset,
+ XHCI_EXT_CAPS_PROTOCOL);
+ if (!offset)
+ break;
+
+ val = readl(base + offset);
+ major_revision = XHCI_EXT_PORT_MAJOR(val);
+
+ val = readl(base + offset + 0x08);
+ if (major_revision == 0x03) {
+ dwc->num_usb3_ports += XHCI_EXT_PORT_COUNT(val);
+ } else if (major_revision <= 0x02) {
+ dwc->num_usb2_ports += XHCI_EXT_PORT_COUNT(val);
+ } else {
+ dev_warn(dwc->dev, "unrecognized port major revision %d\n",
+ major_revision);
+ }
+ } while (1);
+
+ dev_dbg(dwc->dev, "hs-ports: %u ss-ports: %u\n",
+ dwc->num_usb2_ports, dwc->num_usb3_ports);
+
+ iounmap(base);
+
+ return 0;
+}
+
static int dwc3_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
struct resource *res, dwc_res;
+ unsigned int hw_mode;
void __iomem *regs;
struct dwc3 *dwc;
int ret;
@@ -1968,6 +2015,20 @@ static int dwc3_probe(struct platform_device *pdev)
goto err_disable_clks;
}
+ /*
+ * Currently only DWC3 controllers that are host-only capable
+ * can have more than one port.
+ */
+ hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0);
+ if (hw_mode == DWC3_GHWPARAMS0_MODE_HOST) {
+ ret = dwc3_get_num_ports(dwc);
+ if (ret)
+ goto err_disable_clks;
+ } else {
+ dwc->num_usb2_ports = 1;
+ dwc->num_usb3_ports = 1;
+ }
+
spin_lock_init(&dwc->lock);
mutex_init(&dwc->mutex);
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 7e80dd3d466b..341e4c73cb2e 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -1039,6 +1039,8 @@ struct dwc3_scratchpad_array {
* @usb3_phy: pointer to USB3 PHY
* @usb2_generic_phy: pointer to USB2 PHY
* @usb3_generic_phy: pointer to USB3 PHY
+ * @num_usb2_ports: number of USB2 ports
+ * @num_usb3_ports: number of USB3 ports
* @phys_ready: flag to indicate that PHYs are ready
* @ulpi: pointer to ulpi interface
* @ulpi_ready: flag to indicate that ULPI is initialized
@@ -1187,6 +1189,9 @@ struct dwc3 {
struct phy *usb2_generic_phy;
struct phy *usb3_generic_phy;
+ u8 num_usb2_ports;
+ u8 num_usb3_ports;
+
bool phys_ready;
struct ulpi *ulpi;
--
2.34.1
^ permalink raw reply related
* [PATCH v20 1/9] dt-bindings: usb: Add bindings for multiport properties on DWC3 controller
From: Krishna Kurapati @ 2024-04-08 13:29 UTC (permalink / raw)
To: Krzysztof Kozlowski, Rob Herring, Bjorn Andersson, Wesley Cheng,
Konrad Dybcio, Greg Kroah-Hartman, Conor Dooley, Thinh Nguyen,
Felipe Balbi, Johan Hovold
Cc: devicetree, linux-arm-msm, linux-usb, linux-kernel, quic_ppratap,
quic_jackp, Krishna Kurapati, Johan Hovold
In-Reply-To: <20240408132925.1880571-1-quic_kriskura@quicinc.com>
Add bindings to indicate properties required to support multiport
on Synopsys DWC3 controller.
Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>
Reviewed-by: Bjorn Andersson <andersson@kernel.org>
Reviewed-by: Rob Herring <robh@kernel.org>
Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
---
.../devicetree/bindings/usb/snps,dwc3.yaml | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
index 203a1eb66691..bfac0a37d0e4 100644
--- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
@@ -85,15 +85,16 @@ properties:
phys:
minItems: 1
- maxItems: 2
+ maxItems: 8
phy-names:
minItems: 1
- maxItems: 2
- items:
- enum:
- - usb2-phy
- - usb3-phy
+ maxItems: 8
+ oneOf:
+ - items:
+ enum: [ usb2-phy, usb3-phy ]
+ - items:
+ pattern: "^usb[23]-[0-3]$"
power-domains:
description:
--
2.34.1
^ permalink raw reply related
* [PATCH v20 0/9] Add multiport support for DWC3 controllers
From: Krishna Kurapati @ 2024-04-08 13:29 UTC (permalink / raw)
To: Krzysztof Kozlowski, Rob Herring, Bjorn Andersson, Wesley Cheng,
Konrad Dybcio, Greg Kroah-Hartman, Conor Dooley, Thinh Nguyen,
Felipe Balbi, Johan Hovold
Cc: devicetree, linux-arm-msm, linux-usb, linux-kernel, quic_ppratap,
quic_jackp, Krishna Kurapati
Currently the DWC3 driver supports only single port controller which
requires at most two PHYs ie HS and SS PHYs. There are SoCs that has
DWC3 controller with multiple ports that can operate in host mode.
Some of the port supports both SS+HS and other port supports only HS
mode.
This change primarily refactors the Phy logic in core driver to allow
multiport support with Generic Phy's.
Changes have been tested on QCOM SoC SA8295P which has 4 ports (2
are HS+SS capable and 2 are HS only capable).
Changes in v20:
Modified return check in get_num_ports call.
Code re-verified internally and added Bjorn.A RB Tag in patch (2/9)
from internal review.
Changes in v19:
Replaced IS_ERR(ptr) with a NULL check.
Modified name of function reading the port num in core file.
Changes in v18:
Updated variable names in patch-7 for setup_port_irq and
find_num_ports calls.
Changes in v17:
Modified DT handling patch by checking if dp_hs_phy_1 is present
or not and then going for DT parsing.
Changes in v16:
Removing ACPI has simplified the interrupt reading in wrapper. Also
the logic to find number of ports is based on dp_hs_phy interrupt check
in DT. Enabling and disabling interrupts is now done per port. Added
info on power event irq in commit message.
Changes in v15:
Added minItems property in qcom,dwc3 bindings as suggested by Rob.
Retained all RB's/ACK's got in v14.
Changes in v14:
Moved wrapper binding update to 5th patch in the series as it deals
with only wakeup and not enumeration. The first part of the series
deals with enumeration and the next part deals with wakeup.
Updated commit text for wrapper driver patches.
Added error checks in get_port_index and setup_irq call which were
missing in v13.
Added SOB and CDB tags appropriately for the patches.
Rebased code on top of latest usb next.
DT changes have been removed and will be sent as a separate series.
Changes in v13:
This series is a subset of patches in v11 as the first 3 patches in v11
have been mereged into usb-next.
Moved dr_mode property from platform specific files to common sc8280xp DT.
Fixed function call wrapping, added comments and replaced #defines with
enum in dwc3-qcom for identifying IRQ index appropriately.
Fixed nitpicks pointed out in v11 for suspend-resume handling.
Added reported-by tag for phy refactoring patch as a compile error was
found by kernel test bot [1].
Removed reviewed-by tag of maintainer for phy refactoring patch as a minor
change of increasing phy-names array size by 2-bytes was done to fix
compilation issue mentioned in [1].
Changes in v12:
Pushed as a subset of acked but no-yet-merged patches of v11 with intent
of making rebase of other patches easy. Active reviewers from community
suggested that it would be better to push the whole series in one go as it
would give good clarity and context for all the patches in the series.
So pushed v13 for the same addressing comments received in v11.
Changes in v11:
Implemented port_count calculation by reading interrupt-names from DT.
Refactored IRQ handling in dwc3-qcom.
Moving of macros to xhci-ext-caps.h made as a separate patch.
Names of interrupts to be displayed on /proc/interrupts set to the ones
present in DT.
Changes in v10:
Refactored phy init/exit/power-on/off functions in dwc3 core
Refactored dwc3-qcom irq registration and handling
Implemented wakeup for multiport irq's
Moved few macros from xhci.h to xhci-ext-caps.h
Fixed nits pointed out in v9
Fixed Co-developed by and SOB tags in patches 5 and 11
Changes in v9:
Added IRQ support for DP/DM/SS MP Irq's of SC8280
Refactored code to read port count by accessing xhci registers
Changes in v8:
Reorganised code in patch-5
Fixed nitpicks in code according to comments received on v7
Fixed indentation in DT patches
Added drive strength for pinctrl nodes in SA8295 DT
Changes in v7:
Added power event irq's for Multiport controller.
Udpated commit text for patch-9 (adding DT changes for enabling first
port of multiport controller on sa8540-ride).
Fixed check-patch warnings for driver code.
Fixed DT binding errors for changes in snps,dwc3.yaml
Reabsed code on top of usb-next
Changes in v6:
Updated comments in code after.
Updated variables names appropriately as per review comments.
Updated commit text in patch-2 and added additional info as per review
comments.
The patch header in v5 doesn't have "PATHCH v5" notation present. Corrected
it in this version.
Changes in v5:
Added DT support for first port of Teritiary USB controller on SA8540-Ride
Added support for reading port info from XHCI Extended Params registers.
Changes in RFC v4:
Added DT support for SA8295p.
Changes in RFC v3:
Incase any PHY init fails, then clear/exit the PHYs that
are already initialized.
Changes in RFC v2:
Changed dwc3_count_phys to return the number of PHY Phandles in the node.
This will be used now in dwc3_extract_num_phys to increment num_usb2_phy
and num_usb3_phy.
Added new parameter "ss_idx" in dwc3_core_get_phy_ny_node and changed its
structure such that the first half is for HS-PHY and second half is for
SS-PHY.
In dwc3_core_get_phy, for multiport controller, only if SS-PHY phandle is
present, pass proper SS_IDX else pass -1.
Tested enumeration interrupt registration on Tertiary controller of
SA8295 ADP:
/ # lsusb
Bus 001 Device 001: ID 1d6b:0002
Bus 002 Device 001: ID 1d6b:0003
Bus 001 Device 002: ID 046d:c06a
/ #
/ # dmesg | grep ports
[ 0.066250] Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled
[ 0.154668] dwc3 a400000.usb: K: hs-ports: 4 ss-ports: 2
[ 0.223137] xhci-hcd xhci-hcd.0.auto: Host supports USB 3.1 Enhanced SuperSpeed
[ 0.227795] hub 1-0:1.0: 4 ports detected
[ 0.233724] hub 2-0:1.0: 2 ports detected
Tested interrupt registration for all 4 ports of SA8295 ADP:
/ # cat /proc/interrupts |grep phy
162: 0 0 0 0 0 0 0 0 PDC 127 Edge dp_hs_phy_1
163: 0 0 0 0 0 0 0 0 PDC 129 Edge dp_hs_phy_2
164: 0 0 0 0 0 0 0 0 PDC 131 Edge dp_hs_phy_3
165: 0 0 0 0 0 0 0 0 PDC 133 Edge dp_hs_phy_4
166: 0 0 0 0 0 0 0 0 PDC 126 Edge dm_hs_phy_1
167: 0 0 0 0 0 0 0 0 PDC 16 Level ss_phy_1
168: 0 0 0 0 0 0 0 0 PDC 128 Edge dm_hs_phy_2
169: 0 0 0 0 0 0 0 0 PDC 17 Level ss_phy_2
170: 0 0 0 0 0 0 0 0 PDC 130 Edge dm_hs_phy_3
171: 0 0 0 0 0 0 0 0 PDC 132 Edge dm_hs_phy_4
173: 0 0 0 0 0 0 0 0 PDC 14 Edge dp_hs_phy_irq
174: 0 0 0 0 0 0 0 0 PDC 15 Edge dm_hs_phy_irq
175: 0 0 0 0 0 0 0 0 PDC 138 Level ss_phy_irq
Tested working of ADB on SM8550 MTP.
Links to previous versions:
Link to v19: https://lore.kernel.org/all/20240404051229.3082902-1-quic_kriskura@quicinc.com/
Link to v18: https://lore.kernel.org/all/20240326113253.3010447-1-quic_kriskura@quicinc.com/
Link to v17: https://lore.kernel.org/all/20240326102809.2940123-1-quic_kriskura@quicinc.com/
Link to v16: https://lore.kernel.org/all/20240307062052.2319851-1-quic_kriskura@quicinc.com/
Link to v15: https://lore.kernel.org/all/20240216005756.762712-1-quic_kriskura@quicinc.com/
Link to v14: https://lore.kernel.org/all/20240206051825.1038685-1-quic_kriskura@quicinc.com/
Link to v13: https://lore.kernel.org/all/20231007154806.605-1-quic_kriskura@quicinc.com/
Link to v12: https://lore.kernel.org/all/20231004165922.25642-1-quic_kriskura@quicinc.com/
Link to v11: https://lore.kernel.org/all/20230828133033.11988-1-quic_kriskura@quicinc.com/
Link to v10: https://lore.kernel.org/all/20230727223307.8096-1-quic_kriskura@quicinc.com/
Link to v9: https://lore.kernel.org/all/20230621043628.21485-1-quic_kriskura@quicinc.com/
Link to v8: https://lore.kernel.org/all/20230514054917.21318-1-quic_kriskura@quicinc.com/
Link to v7: https://lore.kernel.org/all/20230501143445.3851-1-quic_kriskura@quicinc.com/
Link to v6: https://lore.kernel.org/all/20230405125759.4201-1-quic_kriskura@quicinc.com/
Link to v5: https://lore.kernel.org/all/20230310163420.7582-1-quic_kriskura@quicinc.com/
Link to RFC v4: https://lore.kernel.org/all/20230115114146.12628-1-quic_kriskura@quicinc.com/
Link to RFC v3: https://lore.kernel.org/all/1654709787-23686-1-git-send-email-quic_harshq@quicinc.com/#r
Link to RFC v2: https://lore.kernel.org/all/1653560029-6937-1-git-send-email-quic_harshq@quicinc.com/#r
Krishna Kurapati (9):
dt-bindings: usb: Add bindings for multiport properties on DWC3
controller
usb: dwc3: core: Access XHCI address space temporarily to read port
info
usb: dwc3: core: Skip setting event buffers for host only controllers
usb: dwc3: core: Refactor PHY logic to support Multiport Controller
dt-bindings: usb: qcom,dwc3: Add bindings for SC8280 Multiport
usb: dwc3: qcom: Add helper function to request wakeup interrupts
usb: dwc3: qcom: Refactor IRQ handling in glue driver
usb: dwc3: qcom: Enable wakeup for applicable ports of multiport
usb: dwc3: qcom: Add multiport suspend/resume support for wrapper
.../devicetree/bindings/usb/qcom,dwc3.yaml | 34 ++
.../devicetree/bindings/usb/snps,dwc3.yaml | 13 +-
drivers/usb/dwc3/core.c | 325 +++++++++++++-----
drivers/usb/dwc3/core.h | 19 +-
drivers/usb/dwc3/drd.c | 15 +-
drivers/usb/dwc3/dwc3-qcom.c | 251 +++++++++-----
6 files changed, 482 insertions(+), 175 deletions(-)
--
2.34.1
^ permalink raw reply
* Re: [PATCH v2 1/2] dt-bindings: net: bluetooth: btnxpuart: Add firmware-name property
From: Krzysztof Kozlowski @ 2024-04-08 13:29 UTC (permalink / raw)
To: Neeraj Sanjay Kale, marcel, luiz.dentz, davem, edumazet, kuba,
pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt
Cc: linux-bluetooth, netdev, devicetree, linux-kernel,
amitkumar.karwar, rohit.fule, sherry.sun, ziniu.wang_1,
haibo.chen, LnxRevLi
In-Reply-To: <20240408132241.750792-2-neeraj.sanjaykale@nxp.com>
On 08/04/2024 15:22, Neeraj Sanjay Kale wrote:
> This adds a new optional device tree property called firware-name.
>
> Signed-off-by: Neeraj Sanjay Kale <neeraj.sanjaykale@nxp.com>
BTW, there is no such device as btnxpuart. Bindings are for hardware.
With corrected subject:
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
> v2: Add maxItems, simplify description, remove "nxp/". (Krzysztof)
> ---
> .../devicetree/bindings/net/bluetooth/nxp,88w8987-bt.yaml | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/net/bluetooth/nxp,88w8987-bt.yaml b/Documentation/devicetree/bindings/net/bluetooth/nxp,88w8987-bt.yaml
> index f01a3988538c..6774cc4d6a9e 100644
> --- a/Documentation/devicetree/bindings/net/bluetooth/nxp,88w8987-bt.yaml
> +++ b/Documentation/devicetree/bindings/net/bluetooth/nxp,88w8987-bt.yaml
> @@ -31,6 +31,11 @@ properties:
> This property depends on the module vendor's
> configuration.
>
> + firmware-name:
> + maxItems: 1
> + description:
> + Specify firmware file name.
Drop description, redundant. You did not say anything different than
property is saying already.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 2/4] coresight: Add support for multiple output ports on the funnel
From: Mike Leach @ 2024-04-08 13:28 UTC (permalink / raw)
To: Tao Zhang
Cc: Suzuki K Poulose, Mathieu Poirier, Alexander Shishkin,
Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Jinlong Mao,
Leo Yan, Greg Kroah-Hartman, coresight, linux-arm-kernel,
linux-kernel, devicetree, Tingwei Zhang, Yuanfang Zhang,
Trilok Soni, Song Chai, linux-arm-msm, andersson
In-Reply-To: <ffce4577-b0f9-4af3-a379-0385a02ddae8@quicinc.com>
Hi Tao,
Using a DT label in this way is not how connections should be
described in device tree, The association needs to be between the
input and output ports of the component not and extra link back to the
source.
If your "funnel" has multiple input and output ports, then it is no
longer a funnel. As you describe it - the input ports are statically
associated with an output port, so the device is effectively a set of
individual funnels.
Given that the association between the inputs and outputs is known at
time of compilation - then these can be inferred from the components
compatible string and handled internally during driver probe.
This component should therefore really have its own driver, and not be
described as a standard funnel. In this way you can handle the input
and output associations within the driver and create a path in the
normal way, avoiding the need for large changes to the coresight API.
Having a specific driver will allow creation of sets of associated
input and output connections.
Regards
Mike
On Fri, 29 Mar 2024 at 09:27, Tao Zhang <quic_taozha@quicinc.com> wrote:
>
>
> On 3/22/2024 12:41 AM, Suzuki K Poulose wrote:
> > On 21/03/2024 08:32, Tao Zhang wrote:
> >> Funnel devices are now capable of supporting multiple-inputs and
> >> multiple-outputs configuration with in built hardware filtering
> >> for TPDM devices. Add software support to this function. Output
> >> port is selected according to the source in the trace path.
> >>
> >> The source of the input port on funnels will be marked in the
> >> device tree.
> >> e.g.
> >> tpdm@xxxxxxx {
> >> ... ... ... ...
> >> };
> >>
> >> funnel_XXX: funnel@xxxxxxx {
> >> ... ... ... ...
> >> out-ports {
> >> ... ... ... ...
> >> port@x {
> >> ... ... ... ...
> >> label = "xxxxxxx.tpdm"; <-- To label the source
> >> }; corresponding to the output
> >> ... ... ... ... connection "port@x". And this
> >> }; is a hardware static connections.
> >> ... ... ... ... Here needs to refer to hardware
> >> }; design.
> >>
> >> Then driver will parse the source label marked in the device tree, and
> >> save it to the coresight path. When the function needs to know the
> >> source label, it could obtain it from coresight path parameter. Finally,
> >> the output port knows which source it corresponds to, and it also knows
> >> which input port it corresponds to.
> >
> > Why do we need labels ? We have connection information for all devices
> > (both in and out), so, why do we need this label to find a device ?
>
> Because our funnel's design has multi-output ports, the data stream will not
>
> know which output port should pass in building the data trace path. This
> source
>
> label can make the data stream find the right output port to go.
>
> >
> > And also, I thought TPDM is a source device, why does a funnel output
> > port link to a source ?
>
> No, this label doesn't mean this funnel output port link to a source, it
> just let
>
> the output port know its data source.
>
> >
> > Are these funnels programmable ? Or, are they static ? If they are
> > static, do these need to be described in the DT ? If they are simply
> > acting as a "LINK" (or HWFIFO ?)
>
> These funnels are static, and we will add the "label" to the DT to
> describe the
>
> multi-output ports for these funnels.
>
> "If they are simply acting as a "LINK" (or HWFIFO ?) " I'm not sure
> what's the meaning
>
> of this. Could you describe it in detail?
>
>
> Best,
>
> Tao
>
> >
> > Suzuki
> >
> >>
> >> Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
> >> ---
> >> drivers/hwtracing/coresight/coresight-core.c | 81 ++++++++++++++++---
> >> .../hwtracing/coresight/coresight-platform.c | 5 ++
> >> include/linux/coresight.h | 2 +
> >> 3 files changed, 75 insertions(+), 13 deletions(-)
> >>
> >> diff --git a/drivers/hwtracing/coresight/coresight-core.c
> >> b/drivers/hwtracing/coresight/coresight-core.c
> >> index 5dde597403b3..b1b5e6d9ec7a 100644
> >> --- a/drivers/hwtracing/coresight/coresight-core.c
> >> +++ b/drivers/hwtracing/coresight/coresight-core.c
> >> @@ -113,15 +113,63 @@ struct coresight_device
> >> *coresight_get_percpu_sink(int cpu)
> >> }
> >> EXPORT_SYMBOL_GPL(coresight_get_percpu_sink);
> >> +static struct coresight_device *coresight_get_source(struct
> >> list_head *path)
> >> +{
> >> + struct coresight_device *csdev;
> >> +
> >> + if (!path)
> >> + return NULL;
> >> +
> >> + csdev = list_first_entry(path, struct coresight_node, link)->csdev;
> >> + if (csdev->type != CORESIGHT_DEV_TYPE_SOURCE)
> >> + return NULL;
> >> +
> >> + return csdev;
> >> +}
> >> +
> >> +/**
> >> + * coresight_source_filter - checks whether the connection matches
> >> the source
> >> + * of path if connection is binded to specific source.
> >> + * @path: The list of devices
> >> + * @conn: The connection of one outport
> >> + *
> >> + * Return zero if the connection doesn't have a source binded or
> >> source of the
> >> + * path matches the source binds to connection.
> >> + */
> >> +static int coresight_source_filter(struct list_head *path,
> >> + struct coresight_connection *conn)
> >> +{
> >> + int ret = 0;
> >> + struct coresight_device *source = NULL;
> >> +
> >> + if (conn->source_label == NULL)
> >> + return ret;
> >> +
> >> + source = coresight_get_source(path);
> >> + if (source == NULL)
> >> + return ret;
> >> +
> >> + if (strstr(kobject_get_path(&source->dev.kobj, GFP_KERNEL),
> >> + conn->source_label))
> >> + ret = 0;
> >> + else
> >> + ret = -1;
> >> +
> >> + return ret;
> >> +}
> >> +
> >> static struct coresight_connection *
> >> coresight_find_out_connection(struct coresight_device *src_dev,
> >> - struct coresight_device *dest_dev)
> >> + struct coresight_device *dest_dev,
> >> + struct list_head *path)
> >> {
> >> int i;
> >> struct coresight_connection *conn;
> >> for (i = 0; i < src_dev->pdata->nr_outconns; i++) {
> >> conn = src_dev->pdata->out_conns[i];
> >> + if (coresight_source_filter(path, conn))
> >> + continue;
> >> if (conn->dest_dev == dest_dev)
> >> return conn;
> >> }
> >> @@ -312,7 +360,8 @@ static void coresight_disable_sink(struct
> >> coresight_device *csdev)
> >> static int coresight_enable_link(struct coresight_device *csdev,
> >> struct coresight_device *parent,
> >> - struct coresight_device *child)
> >> + struct coresight_device *child,
> >> + struct list_head *path)
> >> {
> >> int ret = 0;
> >> int link_subtype;
> >> @@ -321,8 +370,8 @@ static int coresight_enable_link(struct
> >> coresight_device *csdev,
> >> if (!parent || !child)
> >> return -EINVAL;
> >> - inconn = coresight_find_out_connection(parent, csdev);
> >> - outconn = coresight_find_out_connection(csdev, child);
> >> + inconn = coresight_find_out_connection(parent, csdev, path);
> >> + outconn = coresight_find_out_connection(csdev, child, path);
> >> link_subtype = csdev->subtype.link_subtype;
> >> if (link_subtype == CORESIGHT_DEV_SUBTYPE_LINK_MERG &&
> >> IS_ERR(inconn))
> >> @@ -341,7 +390,8 @@ static int coresight_enable_link(struct
> >> coresight_device *csdev,
> >> static void coresight_disable_link(struct coresight_device *csdev,
> >> struct coresight_device *parent,
> >> - struct coresight_device *child)
> >> + struct coresight_device *child,
> >> + struct list_head *path)
> >> {
> >> int i;
> >> int link_subtype;
> >> @@ -350,8 +400,8 @@ static void coresight_disable_link(struct
> >> coresight_device *csdev,
> >> if (!parent || !child)
> >> return;
> >> - inconn = coresight_find_out_connection(parent, csdev);
> >> - outconn = coresight_find_out_connection(csdev, child);
> >> + inconn = coresight_find_out_connection(parent, csdev, path);
> >> + outconn = coresight_find_out_connection(csdev, child, path);
> >> link_subtype = csdev->subtype.link_subtype;
> >> if (link_ops(csdev)->disable) {
> >> @@ -507,7 +557,7 @@ static void coresight_disable_path_from(struct
> >> list_head *path,
> >> case CORESIGHT_DEV_TYPE_LINK:
> >> parent = list_prev_entry(nd, link)->csdev;
> >> child = list_next_entry(nd, link)->csdev;
> >> - coresight_disable_link(csdev, parent, child);
> >> + coresight_disable_link(csdev, parent, child, path);
> >> break;
> >> default:
> >> break;
> >> @@ -588,7 +638,7 @@ int coresight_enable_path(struct list_head *path,
> >> enum cs_mode mode,
> >> case CORESIGHT_DEV_TYPE_LINK:
> >> parent = list_prev_entry(nd, link)->csdev;
> >> child = list_next_entry(nd, link)->csdev;
> >> - ret = coresight_enable_link(csdev, parent, child);
> >> + ret = coresight_enable_link(csdev, parent, child, path);
> >> if (ret)
> >> goto err;
> >> break;
> >> @@ -802,7 +852,8 @@ static void coresight_drop_device(struct
> >> coresight_device *csdev)
> >> */
> >> static int _coresight_build_path(struct coresight_device *csdev,
> >> struct coresight_device *sink,
> >> - struct list_head *path)
> >> + struct list_head *path,
> >> + struct coresight_device *source)
> >> {
> >> int i, ret;
> >> bool found = false;
> >> @@ -814,7 +865,7 @@ static int _coresight_build_path(struct
> >> coresight_device *csdev,
> >> if (coresight_is_percpu_source(csdev) &&
> >> coresight_is_percpu_sink(sink) &&
> >> sink == per_cpu(csdev_sink,
> >> source_ops(csdev)->cpu_id(csdev))) {
> >> - if (_coresight_build_path(sink, sink, path) == 0) {
> >> + if (_coresight_build_path(sink, sink, path, source) == 0) {
> >> found = true;
> >> goto out;
> >> }
> >> @@ -825,8 +876,12 @@ static int _coresight_build_path(struct
> >> coresight_device *csdev,
> >> struct coresight_device *child_dev;
> >> child_dev = csdev->pdata->out_conns[i]->dest_dev;
> >> + if (csdev->pdata->out_conns[i]->source_label &&
> >> + !strstr(kobject_get_path(&source->dev.kobj, GFP_KERNEL),
> >> + csdev->pdata->out_conns[i]->source_label))
> >> + continue;
> >> if (child_dev &&
> >> - _coresight_build_path(child_dev, sink, path) == 0) {
> >> + _coresight_build_path(child_dev, sink, path, source) ==
> >> 0) {
> >> found = true;
> >> break;
> >> }
> >> @@ -871,7 +926,7 @@ struct list_head *coresight_build_path(struct
> >> coresight_device *source,
> >> INIT_LIST_HEAD(path);
> >> - rc = _coresight_build_path(source, sink, path);
> >> + rc = _coresight_build_path(source, sink, path, source);
> >> if (rc) {
> >> kfree(path);
> >> return ERR_PTR(rc);
> >> diff --git a/drivers/hwtracing/coresight/coresight-platform.c
> >> b/drivers/hwtracing/coresight/coresight-platform.c
> >> index 9d550f5697fa..f553fb20966d 100644
> >> --- a/drivers/hwtracing/coresight/coresight-platform.c
> >> +++ b/drivers/hwtracing/coresight/coresight-platform.c
> >> @@ -205,6 +205,7 @@ static int of_coresight_parse_endpoint(struct
> >> device *dev,
> >> struct fwnode_handle *rdev_fwnode;
> >> struct coresight_connection conn = {};
> >> struct coresight_connection *new_conn;
> >> + const char *label;
> >> do {
> >> /* Parse the local port details */
> >> @@ -243,6 +244,10 @@ static int of_coresight_parse_endpoint(struct
> >> device *dev,
> >> conn.dest_fwnode = fwnode_handle_get(rdev_fwnode);
> >> conn.dest_port = rendpoint.port;
> >> + conn.source_label = NULL;
> >> + if (!of_property_read_string(ep, "label", &label))
> >> + conn.source_label = label;
> >> +
> >> new_conn = coresight_add_out_conn(dev, pdata, &conn);
> >> if (IS_ERR_VALUE(new_conn)) {
> >> fwnode_handle_put(conn.dest_fwnode);
> >> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
> >> index e8b6e388218c..a9c06ef9bbb2 100644
> >> --- a/include/linux/coresight.h
> >> +++ b/include/linux/coresight.h
> >> @@ -167,6 +167,7 @@ struct coresight_desc {
> >> * struct coresight_connection - representation of a single connection
> >> * @src_port: a connection's output port number.
> >> * @dest_port: destination's input port number @src_port is
> >> connected to.
> >> + * @source_label: source component's label.
> >> * @dest_fwnode: destination component's fwnode handle.
> >> * @dest_dev: a @coresight_device representation of the component
> >> connected to @src_port. NULL until the device is created
> >> @@ -195,6 +196,7 @@ struct coresight_desc {
> >> struct coresight_connection {
> >> int src_port;
> >> int dest_port;
> >> + const char *source_label;
> >> struct fwnode_handle *dest_fwnode;
> >> struct coresight_device *dest_dev;
> >> struct coresight_sysfs_link *link;
> >
--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox