* [PATCH v2 2/6] ARM: dts: mmp2: fix the SPI nodes
From: Lubomir Rintel @ 2019-08-02 10:33 UTC (permalink / raw)
To: Olof Johansson
Cc: Rob Herring, Mark Rutland, linux-arm-kernel, devicetree,
linux-kernel, Pavel Machek, Lubomir Rintel
In-Reply-To: <20190802103326.531250-1-lkundrak@v3.sk>
The SPI bus has a single address cell and not size cells.
Also, dtc thinks the SPI nodes are preferrably called "spi" and it is
right to think so.
Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
arch/arm/boot/dts/mmp2.dtsi | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/arch/arm/boot/dts/mmp2.dtsi b/arch/arm/boot/dts/mmp2.dtsi
index 50b6c38b39cc3..e64639ce57a91 100644
--- a/arch/arm/boot/dts/mmp2.dtsi
+++ b/arch/arm/boot/dts/mmp2.dtsi
@@ -346,35 +346,43 @@
status = "disabled";
};
- ssp1: ssp@d4035000 {
+ ssp1: spi@d4035000 {
compatible = "marvell,mmp2-ssp";
reg = <0xd4035000 0x1000>;
clocks = <&soc_clocks MMP2_CLK_SSP0>;
interrupts = <0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
status = "disabled";
};
- ssp2: ssp@d4036000 {
+ ssp2: spi@d4036000 {
compatible = "marvell,mmp2-ssp";
reg = <0xd4036000 0x1000>;
clocks = <&soc_clocks MMP2_CLK_SSP1>;
interrupts = <1>;
+ #address-cells = <1>;
+ #size-cells = <0>;
status = "disabled";
};
- ssp3: ssp@d4037000 {
+ ssp3: spi@d4037000 {
compatible = "marvell,mmp2-ssp";
reg = <0xd4037000 0x1000>;
clocks = <&soc_clocks MMP2_CLK_SSP2>;
interrupts = <20>;
+ #address-cells = <1>;
+ #size-cells = <0>;
status = "disabled";
};
- ssp4: ssp@d4039000 {
+ ssp4: spi@d4039000 {
compatible = "marvell,mmp2-ssp";
reg = <0xd4039000 0x1000>;
clocks = <&soc_clocks MMP2_CLK_SSP3>;
interrupts = <21>;
+ #address-cells = <1>;
+ #size-cells = <0>;
status = "disabled";
};
};
--
2.21.0
^ permalink raw reply related
* [PATCH v2 1/6] ARM: dts: mmp2: trivial whitespace fix
From: Lubomir Rintel @ 2019-08-02 10:33 UTC (permalink / raw)
To: Olof Johansson
Cc: Mark Rutland, devicetree, linux-kernel, Lubomir Rintel,
Rob Herring, Pavel Machek, linux-arm-kernel
In-Reply-To: <20190802103326.531250-1-lkundrak@v3.sk>
A missing space before a curly brace.
Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
arch/arm/boot/dts/mmp2.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/mmp2.dtsi b/arch/arm/boot/dts/mmp2.dtsi
index b6f40743e07b0..50b6c38b39cc3 100644
--- a/arch/arm/boot/dts/mmp2.dtsi
+++ b/arch/arm/boot/dts/mmp2.dtsi
@@ -379,7 +379,7 @@
};
};
- soc_clocks: clocks{
+ soc_clocks: clocks {
compatible = "marvell,mmp2-clock";
reg = <0xd4050000 0x1000>,
<0xd4282800 0x400>,
--
2.21.0
^ permalink raw reply related
* [PATCH v2 0/6] ARM: dts: mmp2: devicetree updates
From: Lubomir Rintel @ 2019-08-02 10:33 UTC (permalink / raw)
To: Olof Johansson
Cc: Mark Rutland, devicetree, linux-kernel, Rob Herring, Pavel Machek,
linux-arm-kernel
Hi,
Here's a couple of updates for the MMP2 SoC devicetree files.
The only change from the last submission is the addition of the
OLPC XO 1.75 dts file. Apart from that one, the patches are
independent of each other, can be applied in any order.
Hopefully I'm sending the patch set in the correct direction.
Lubo
^ permalink raw reply
* [PATCH 4/4] dt-bindings: iio: adc: Add AD7606B ADC documentation
From: Beniamin Bia @ 2019-08-02 10:03 UTC (permalink / raw)
To: jic23
Cc: lars, Michael.Hennerich, knaack.h, pmeerw, gregkh, linux-iio,
devel, linux-kernel, mark.rutland, robh+dt, devicetree, paulmck,
mchehab+samsung, linus.walleij, nicolas.ferre, biabeniamin,
Beniamin Bia
In-Reply-To: <20190802100304.15899-1-beniamin.bia@analog.com>
Documentation for AD7606B Analog to Digital Converter and software
mode was added.
Signed-off-by: Beniamin Bia <beniamin.bia@analog.com>
---
Documentation/devicetree/bindings/iio/adc/adi,ad7606.txt | 8 ++++++++
Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml | 4 +++-
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.txt b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.txt
index d8652460198e..9cc7ea19eca6 100644
--- a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.txt
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.txt
@@ -7,6 +7,7 @@ Required properties for the AD7606:
* "adi,ad7606-8"
* "adi,ad7606-6"
* "adi,ad7606-4"
+ * "adi,ad7606b"
* "adi,ad7616"
- reg: SPI chip select number for the device
- spi-max-frequency: Max SPI frequency to use
@@ -42,6 +43,12 @@ Optional properties:
- adi,oversampling-ratio-gpios: must be the device tree identifier of the over-sampling
mode pins. As the line is active high, it should be marked
GPIO_ACTIVE_HIGH.
+- adi,sw-mode: Boolean, software mode of operation, so far available only for ad7606b.
+ Software mode is enabled when all three oversampling mode pins are connected to
+ high level. The AD7606B is configured by the corresponding registers. If the
+ adi,oversampling-ratio-gpios property is defined, then the driver will set the
+ oversampling gpios to high. Otherwise, it is assumed that the pins are hardwired
+ to VDD.
Example:
@@ -63,4 +70,5 @@ Example:
&gpio 23 GPIO_ACTIVE_HIGH
&gpio 26 GPIO_ACTIVE_HIGH>;
standby-gpios = <&gpio 24 GPIO_ACTIVE_LOW>;
+ adi,sw-mode;
};
diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
index 509dbe9c84d2..2afe31747a70 100644
--- a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
@@ -13,6 +13,7 @@ maintainers:
description: |
Analog Devices AD7606 Simultaneous Sampling ADC
https://www.analog.com/media/en/technical-documentation/data-sheets/ad7606_7606-6_7606-4.pdf
+ https://www.analog.com/media/en/technical-documentation/data-sheets/AD7606B.pdf
https://www.analog.com/media/en/technical-documentation/data-sheets/AD7616.pdf
properties:
@@ -22,6 +23,7 @@ properties:
- adi,ad7606-8
- adi,ad7606-6
- adi,ad7606-4
+ - adi,ad7606b
- adi,ad7616
reg:
@@ -87,7 +89,7 @@ properties:
adi,sw-mode:
description:
- Software mode of operation, so far available only for ad7616.
+ Software mode of operation, so far available only for ad7616 and ad7606B.
It is enabled when all three oversampling mode pins are connected to
high level. The device is configured by the corresponding registers. If the
adi,oversampling-ratio-gpios property is defined, then the driver will set the
--
2.17.1
^ permalink raw reply related
* [PATCH 3/4] dt-bindings: iio: adc: Migrate AD7606 documentation to yaml
From: Beniamin Bia @ 2019-08-02 10:03 UTC (permalink / raw)
To: jic23
Cc: lars, Michael.Hennerich, knaack.h, pmeerw, gregkh, linux-iio,
devel, linux-kernel, mark.rutland, robh+dt, devicetree, paulmck,
mchehab+samsung, linus.walleij, nicolas.ferre, biabeniamin,
Beniamin Bia
In-Reply-To: <20190802100304.15899-1-beniamin.bia@analog.com>
The documentation for ad7606 was migrated to yaml, the new Linux Kernel
standard.
Signed-off-by: Beniamin Bia <beniamin.bia@analog.com>
---
.../bindings/iio/adc/adi,ad7606.yaml | 134 ++++++++++++++++++
MAINTAINERS | 2 +-
2 files changed, 135 insertions(+), 1 deletion(-)
create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
new file mode 100644
index 000000000000..509dbe9c84d2
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
@@ -0,0 +1,134 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/adi,ad7606.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices AD7606 Simultaneous Sampling ADC
+
+maintainers:
+ - Beniamin Bia <beniamin.bia@analog.com>
+ - Stefan Popa <stefan.popa@analog.com>
+
+description: |
+ Analog Devices AD7606 Simultaneous Sampling ADC
+ https://www.analog.com/media/en/technical-documentation/data-sheets/ad7606_7606-6_7606-4.pdf
+ https://www.analog.com/media/en/technical-documentation/data-sheets/AD7616.pdf
+
+properties:
+ compatible:
+ enum:
+ - adi,ad7605-4
+ - adi,ad7606-8
+ - adi,ad7606-6
+ - adi,ad7606-4
+ - adi,ad7616
+
+ reg:
+ maxItems: 1
+
+ spi-cpha: true
+
+ avcc-supply:
+ description:
+ Phandle to the Avcc power supply
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ adi,conversion-start-gpios:
+ description:
+ Must be the device tree identifier of the CONVST pin.
+ This logic input is used to initiate conversions on the analog
+ input channels. As the line is active high, it should be marked
+ GPIO_ACTIVE_HIGH.
+ maxItems: 1
+
+ reset-gpios:
+ description:
+ Must be the device tree identifier of the RESET pin. If specified,
+ it will be asserted during driver probe. As the line is active high,
+ it should be marked GPIO_ACTIVE_HIGH.
+ maxItems: 1
+
+ standby-gpios:
+ description:
+ Must be the device tree identifier of the STBY pin. This pin is used
+ to place the AD7606 into one of two power-down modes, Standby mode or
+ Shutdown mode. As the line is active low, it should be marked
+ GPIO_ACTIVE_LOW.
+ maxItems: 1
+
+ adi,first-data-gpios:
+ description:
+ Must be the device tree identifier of the FRSTDATA pin.
+ The FRSTDATA output indicates when the first channel, V1, is
+ being read back on either the parallel, byte or serial interface.
+ As the line is active high, it should be marked GPIO_ACTIVE_HIGH.
+ maxItems: 1
+
+ adi,range-gpios:
+ description:
+ Must be the device tree identifier of the RANGE pin. The polarity on
+ this pin determines the input range of the analog input channels. If
+ this pin is tied to a logic high, the analog input range is ±10V for
+ all channels. If this pin is tied to a logic low, the analog input range
+ is ±5V for all channels. As the line is active high, it should be marked
+ GPIO_ACTIVE_HIGH.
+ maxItems: 1
+
+ adi,oversampling-ratio-gpios:
+ description:
+ Must be the device tree identifier of the over-sampling
+ mode pins. As the line is active high, it should be marked
+ GPIO_ACTIVE_HIGH.
+ maxItems: 1
+
+ adi,sw-mode:
+ description:
+ Software mode of operation, so far available only for ad7616.
+ It is enabled when all three oversampling mode pins are connected to
+ high level. The device is configured by the corresponding registers. If the
+ adi,oversampling-ratio-gpios property is defined, then the driver will set the
+ oversampling gpios to high. Otherwise, it is assumed that the pins are hardwired
+ to VDD.
+ maxItems: 1
+ type: boolean
+
+required:
+ - compatible
+ - reg
+ - spi-cpha
+ - avcc-supply
+ - interrupts
+ - adi,conversion-start-gpios
+
+examples:
+ - |
+ spi0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ adc@0 {
+ compatible = "adi,ad7606-8";
+ reg = <0>;
+ spi-max-frequency = <1000000>;
+ spi-cpol;
+
+ avcc-supply = <&adc_vref>;
+
+ interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
+ interrupt-parent = <&gpio>;
+
+ adi,conversion-start-gpios = <&gpio 17 GPIO_ACTIVE_HIGH>;
+ reset-gpios = <&gpio 27 GPIO_ACTIVE_HIGH>;
+ adi,first-data-gpios = <&gpio 22 GPIO_ACTIVE_HIGH>;
+ adi,oversampling-ratio-gpios = <&gpio 18 GPIO_ACTIVE_HIGH
+ &gpio 23 GPIO_ACTIVE_HIGH
+ &gpio 26 GPIO_ACTIVE_HIGH>;
+ standby-gpios = <&gpio 24 GPIO_ACTIVE_LOW>;
+ adi,sw-mode;
+ };
+ };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 052d7a8591fb..d2e465772071 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -900,7 +900,7 @@ L: linux-iio@vger.kernel.org
W: http://ez.analog.com/community/linux-device-drivers
S: Supported
F: drivers/iio/adc/ad7606.c
-F: Documentation/devicetree/bindings/iio/adc/adi,ad7606.txt
+F: Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
ANALOG DEVICES INC AD7768-1 DRIVER
M: Stefan Popa <stefan.popa@analog.com>
--
2.17.1
^ permalink raw reply related
* [PATCH 2/4] MAINTAINERS: Add Beniamin Bia for AD7606 driver
From: Beniamin Bia @ 2019-08-02 10:03 UTC (permalink / raw)
To: jic23
Cc: lars, Michael.Hennerich, knaack.h, pmeerw, gregkh, linux-iio,
devel, linux-kernel, mark.rutland, robh+dt, devicetree, paulmck,
mchehab+samsung, linus.walleij, nicolas.ferre, biabeniamin,
Beniamin Bia
In-Reply-To: <20190802100304.15899-1-beniamin.bia@analog.com>
Add Beniamin Bia as maintainer for AD7606 driver.
Signed-off-by: Beniamin Bia <beniamin.bia@analog.com>
---
MAINTAINERS | 1 +
1 file changed, 1 insertion(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index ad498428b38c..052d7a8591fb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -895,6 +895,7 @@ F: Documentation/devicetree/bindings/iio/adc/adi,ad7124.txt
ANALOG DEVICES INC AD7606 DRIVER
M: Stefan Popa <stefan.popa@analog.com>
+M: Beniamin Bia <beniamin.bia@analog.com>
L: linux-iio@vger.kernel.org
W: http://ez.analog.com/community/linux-device-drivers
S: Supported
--
2.17.1
^ permalink raw reply related
* [PATCH 1/4] iio: adc: ad7606: Add support for AD7606B ADC
From: Beniamin Bia @ 2019-08-02 10:03 UTC (permalink / raw)
To: jic23
Cc: lars, Michael.Hennerich, knaack.h, pmeerw, gregkh, linux-iio,
devel, linux-kernel, mark.rutland, robh+dt, devicetree, paulmck,
mchehab+samsung, linus.walleij, nicolas.ferre, biabeniamin,
Stefan Popa, Beniamin Bia
From: Stefan Popa <stefan.popa@analog.com>
The AD7606B is a 16-bit ADC that supports simultaneous sampling of 8
channels. It is pin compatible to AD7606, but adds extra modes by
writing to the register map.
The AD7606B can be configured to work in software mode by setting all
oversampling pins to high. This mode is selected by default.
The oversampling ratio is configured from the OS_MODE register (address
0x08) with the addition of OS=128 and OS=256 that were not available in
hardware mode.
The device is configured to output data on a single spi channel, but this
configuration must be done right after restart. That is why the delay was
removed for devices which doesn't require it.
Moreover, in software mode, the range gpio has no longer its function.
Instead, the scale can be configured individually for each channel from
the RANGE_CH registers (address 0x03 to 0x06). Besides the already
supported ±10 V and ±5 V ranges, software mode can also accommodate the
±2.5 V range.
Signed-off-by: Stefan Popa <stefan.popa@analog.com>
Co-developed-by: Beniamin Bia <beniamin.bia@analog.com>
Signed-off-by: Beniamin Bia <beniamin.bia@analog.com>
---
drivers/iio/adc/ad7606.c | 13 ++++-
drivers/iio/adc/ad7606.h | 4 ++
drivers/iio/adc/ad7606_spi.c | 107 +++++++++++++++++++++++++++++++++++
3 files changed, 122 insertions(+), 2 deletions(-)
diff --git a/drivers/iio/adc/ad7606.c b/drivers/iio/adc/ad7606.c
index ed2d08437e5d..f5ba94c03a8d 100644
--- a/drivers/iio/adc/ad7606.c
+++ b/drivers/iio/adc/ad7606.c
@@ -410,12 +410,19 @@ static const struct ad7606_chip_info ad7606_chip_info_tbl[] = {
.oversampling_avail = ad7606_oversampling_avail,
.oversampling_num = ARRAY_SIZE(ad7606_oversampling_avail),
},
+ [ID_AD7606B] = {
+ .channels = ad7606_channels,
+ .num_channels = 9,
+ .oversampling_avail = ad7606_oversampling_avail,
+ .oversampling_num = ARRAY_SIZE(ad7606_oversampling_avail),
+ },
[ID_AD7616] = {
.channels = ad7616_channels,
.num_channels = 17,
.oversampling_avail = ad7616_oversampling_avail,
.oversampling_num = ARRAY_SIZE(ad7616_oversampling_avail),
.os_req_reset = true,
+ .init_delay_ms = 15,
},
};
@@ -631,8 +638,10 @@ int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
dev_warn(st->dev, "failed to RESET: no RESET GPIO specified\n");
/* AD7616 requires al least 15ms to reconfigure after a reset */
- if (msleep_interruptible(15))
- return -ERESTARTSYS;
+ if (st->chip_info->init_delay_ms) {
+ if (msleep_interruptible(st->chip_info->init_delay_ms))
+ return -ERESTARTSYS;
+ }
st->write_scale = ad7606_write_scale_hw;
st->write_os = ad7606_write_os_hw;
diff --git a/drivers/iio/adc/ad7606.h b/drivers/iio/adc/ad7606.h
index eeaaa8b905db..9350ef1f63b5 100644
--- a/drivers/iio/adc/ad7606.h
+++ b/drivers/iio/adc/ad7606.h
@@ -46,6 +46,8 @@
* oversampling ratios.
* @oversampling_num number of elements stored in oversampling_avail array
* @os_req_reset some devices require a reset to update oversampling
+ * @init_delay_ms required delay in miliseconds for initialization
+ * after a restart
*/
struct ad7606_chip_info {
const struct iio_chan_spec *channels;
@@ -53,6 +55,7 @@ struct ad7606_chip_info {
const unsigned int *oversampling_avail;
unsigned int oversampling_num;
bool os_req_reset;
+ unsigned long init_delay_ms;
};
/**
@@ -155,6 +158,7 @@ enum ad7606_supported_device_ids {
ID_AD7606_8,
ID_AD7606_6,
ID_AD7606_4,
+ ID_AD7606B,
ID_AD7616,
};
diff --git a/drivers/iio/adc/ad7606_spi.c b/drivers/iio/adc/ad7606_spi.c
index 98ed52b74507..070ee7e31e2c 100644
--- a/drivers/iio/adc/ad7606_spi.c
+++ b/drivers/iio/adc/ad7606_spi.c
@@ -31,6 +31,20 @@
/* The range of the channel is stored on 2 bits*/
#define AD7616_RANGE_CH_MSK(ch) (0b11 << (((ch) & 0b11) * 2))
#define AD7616_RANGE_CH_MODE(ch, mode) ((mode) << ((((ch) & 0b11)) * 2))
+
+#define AD7606_CONFIGURATION_REGISTER 0x02
+#define AD7606_SINGLE_DOUT 0x0
+
+/*
+ * Range for AD7606B channels are stored in registers starting with address 0x3.
+ * Each register stores range for 2 channels(4 bits per channel).
+ */
+#define AD7606_RANGE_CH_MSK(ch) (GENMASK(3, 0) << (4 * ((ch) & 0x1)))
+#define AD7606_RANGE_CH_MODE(ch, mode) \
+ ((GENMASK(3, 0) & mode) << (4 * ((ch) & 0x1)))
+#define AD7606_RANGE_CH_ADDR(ch) (0x03 + ((ch) >> 1))
+#define AD7606_OS_MODE 0x08
+
static const struct iio_chan_spec ad7616_sw_channels[] = {
IIO_CHAN_SOFT_TIMESTAMP(16),
AD7616_CHANNEL(0),
@@ -51,6 +65,22 @@ static const struct iio_chan_spec ad7616_sw_channels[] = {
AD7616_CHANNEL(15),
};
+static const struct iio_chan_spec ad7606B_sw_channels[] = {
+ IIO_CHAN_SOFT_TIMESTAMP(8),
+ AD7616_CHANNEL(0),
+ AD7616_CHANNEL(1),
+ AD7616_CHANNEL(2),
+ AD7616_CHANNEL(3),
+ AD7616_CHANNEL(4),
+ AD7616_CHANNEL(5),
+ AD7616_CHANNEL(6),
+ AD7616_CHANNEL(7),
+};
+
+static const unsigned int ad7606B_oversampling_avail[9] = {
+ 1, 2, 4, 8, 16, 32, 64, 128, 256
+};
+
static u16 ad7616_spi_rd_wr_cmd(int addr, char isWriteOp)
{
/*
@@ -60,6 +90,16 @@ static u16 ad7616_spi_rd_wr_cmd(int addr, char isWriteOp)
return ((addr & 0x7F) << 1) | ((isWriteOp & 0x1) << 7);
}
+static u16 ad7606B_spi_rd_wr_cmd(int addr, char isWriteOp)
+{
+ /*
+ * The address of register consists of one bit which
+ * specifies a read command placed bit 6, followed by
+ * 6 bits of address.
+ */
+ return (addr & 0x3F) | (((~isWriteOp) & 0x1) << 6);
+}
+
static int ad7606_spi_read_block(struct device *dev,
int count, void *buf)
{
@@ -169,6 +209,23 @@ static int ad7616_write_os_sw(struct iio_dev *indio_dev, int val)
AD7616_OS_MASK, val << 2);
}
+static int ad7606_write_scale_sw(struct iio_dev *indio_dev, int ch, int val)
+{
+ struct ad7606_state *st = iio_priv(indio_dev);
+
+ return ad7606_spi_write_mask(st,
+ AD7606_RANGE_CH_ADDR(ch),
+ AD7606_RANGE_CH_MSK(ch),
+ AD7606_RANGE_CH_MODE(ch, val));
+}
+
+static int ad7606_write_os_sw(struct iio_dev *indio_dev, int val)
+{
+ struct ad7606_state *st = iio_priv(indio_dev);
+
+ return ad7606_spi_reg_write(st, AD7606_OS_MODE, val);
+}
+
static int ad7616_sw_mode_config(struct iio_dev *indio_dev)
{
struct ad7606_state *st = iio_priv(indio_dev);
@@ -189,6 +246,42 @@ static int ad7616_sw_mode_config(struct iio_dev *indio_dev)
AD7616_BURST_MODE | AD7616_SEQEN_MODE);
}
+static int ad7606B_sw_mode_config(struct iio_dev *indio_dev)
+{
+ struct ad7606_state *st = iio_priv(indio_dev);
+ unsigned long os[3] = {1};
+
+ /*
+ * Software mode is enabled when all three oversampling
+ * pins are set to high. If oversampling gpios are defined
+ * in the device tree, then they need to be set to high,
+ * otherwise, they must be hardwired to VDD
+ */
+ if (st->gpio_os) {
+ gpiod_set_array_value(ARRAY_SIZE(os),
+ st->gpio_os->desc, st->gpio_os->info, os);
+ }
+ /* OS of 128 and 256 are available only in software mode */
+ st->oversampling_avail = ad7606B_oversampling_avail;
+ st->num_os_ratios = ARRAY_SIZE(ad7606B_oversampling_avail);
+
+ st->write_scale = ad7606_write_scale_sw;
+ st->write_os = &ad7606_write_os_sw;
+
+ /* Configure device spi to output on a single channel */
+ st->bops->reg_write(st,
+ AD7606_CONFIGURATION_REGISTER,
+ AD7606_SINGLE_DOUT);
+
+ /*
+ * Scale can be configured individually for each channel
+ * in software mode.
+ */
+ indio_dev->channels = ad7606B_sw_channels;
+
+ return 0;
+}
+
static const struct ad7606_bus_ops ad7606_spi_bops = {
.read_block = ad7606_spi_read_block,
};
@@ -202,6 +295,15 @@ static const struct ad7606_bus_ops ad7616_spi_bops = {
.sw_mode_config = ad7616_sw_mode_config,
};
+static const struct ad7606_bus_ops ad7606B_spi_bops = {
+ .read_block = ad7606_spi_read_block,
+ .reg_read = ad7606_spi_reg_read,
+ .reg_write = ad7606_spi_reg_write,
+ .write_mask = ad7606_spi_write_mask,
+ .rd_wr_cmd = ad7606B_spi_rd_wr_cmd,
+ .sw_mode_config = ad7606B_sw_mode_config,
+};
+
static int ad7606_spi_probe(struct spi_device *spi)
{
const struct spi_device_id *id = spi_get_device_id(spi);
@@ -211,6 +313,9 @@ static int ad7606_spi_probe(struct spi_device *spi)
case ID_AD7616:
bops = &ad7616_spi_bops;
break;
+ case ID_AD7606B:
+ bops = &ad7606B_spi_bops;
+ break;
default:
bops = &ad7606_spi_bops;
break;
@@ -226,6 +331,7 @@ static const struct spi_device_id ad7606_id_table[] = {
{ "ad7606-4", ID_AD7606_4 },
{ "ad7606-6", ID_AD7606_6 },
{ "ad7606-8", ID_AD7606_8 },
+ { "ad7606b", ID_AD7606B },
{ "ad7616", ID_AD7616 },
{}
};
@@ -236,6 +342,7 @@ static const struct of_device_id ad7606_of_match[] = {
{ .compatible = "adi,ad7606-4" },
{ .compatible = "adi,ad7606-6" },
{ .compatible = "adi,ad7606-8" },
+ { .compatible = "adi,ad7606b" },
{ .compatible = "adi,ad7616" },
{ },
};
--
2.17.1
^ permalink raw reply related
* Re: [PATCH 0/4] dt-bindings: i2c: renesas: Rename bindings documentation files
From: Wolfram Sang @ 2019-08-02 10:02 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Simon Horman, Wolfram Sang, Chris Brandt, Rob Herring,
Mark Rutland, Magnus Damm, Linux I2C,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux-Renesas
In-Reply-To: <CAMuHMdVBVFbcTDcXbBT18NfTnAxW-Gz6XnBgKO5REdHtw9zeaQ@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1160 bytes --]
On Fri, Aug 02, 2019 at 11:27:19AM +0200, Geert Uytterhoeven wrote:
> Hi Wolfram,
>
> On Thu, Aug 1, 2019 at 3:16 PM Wolfram Sang <wsa@the-dreams.de> wrote:
> > On Wed, Jul 24, 2019 at 02:15:55PM +0200, Simon Horman wrote:
> > > Rename the bindings documentation file for Renesas I2C controllers.
> > >
> > > This is part of an ongoing effort to name bindings documentation files for
> > > Renesas IP blocks consistently, in line with the compat strings they
> > > document.
> > >
> > > Based on v5.3-rc1
> > >
> > > Simon Horman (4):
> > > dt-bindings: i2c: sh_mobile: Rename bindings documentation file
> > > dt-bindings: i2c: rcar: Rename bindings documentation file
> > > dt-bindings: i2c: riic: Rename bindings documentation file
> > > dt-bindings: i2c: riic: Rename bindings documentation file
> >
> > Applied to for-next, thanks!
>
> Without the wrong names fixed?
> And with an R-b I didn't give, probably due to the two last patches having
> the same oneline-summary?
Geez, seems I was somehow distracted yesterday. Did quite some mistakes.
Simon, please resend this series, with comments from Geert addressed.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 2/4] counter: new TI eQEP driver
From: William Breathitt Gray @ 2019-08-02 9:27 UTC (permalink / raw)
To: David Lechner
Cc: linux-iio, linux-omap, devicetree, Rob Herring, Mark Rutland,
Benoît Cousson, Tony Lindgren, Thierry Reding, linux-kernel,
linux-pwm
In-Reply-To: <20190722154538.5314-3-david@lechnology.com>
On Mon, Jul 22, 2019 at 10:45:36AM -0500, David Lechner wrote:
> This adds a new counter driver for the Texas Instruments Enhanced
> Quadrature Encoder Pulse (eQEP) module.
>
> Only very basic functionality is currently implemented - only enough to
> be able to read the position. The actual device has many more features
> which can be added to the driver on an as-needed basis.
>
> Signed-off-by: David Lechner <david@lechnology.com>
Some changes suggested below, but most of the important stuff is there
and correct so good job.
William Breathitt Gray
> ---
> MAINTAINERS | 6 +
> drivers/counter/Kconfig | 12 ++
> drivers/counter/Makefile | 1 +
> drivers/counter/ti-eqep.c | 381 ++++++++++++++++++++++++++++++++++++++
> drivers/pwm/Kconfig | 2 +-
> 5 files changed, 401 insertions(+), 1 deletion(-)
> create mode 100644 drivers/counter/ti-eqep.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 783569e3c4b4..f3b5e275732b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -16014,6 +16014,12 @@ S: Maintained
> F: drivers/media/platform/davinci/
> F: include/media/davinci/
>
> +TI ENHANCED QUADRATURE ENCODER PULSE (eQEP) DRIVER
> +R: David Lechner <david@lechnology.com>
> +L: linux-iio@vger.kernel.org
> +F: Documentation/devicetree/bindings/counter/ti-eqep.txt
> +F: drivers/counter/ti-eqep.c
> +
> TI ETHERNET SWITCH DRIVER (CPSW)
> R: Grygorii Strashko <grygorii.strashko@ti.com>
> L: linux-omap@vger.kernel.org
> diff --git a/drivers/counter/Kconfig b/drivers/counter/Kconfig
> index 2967d0a9ff91..7eeb310f0cda 100644
> --- a/drivers/counter/Kconfig
> +++ b/drivers/counter/Kconfig
> @@ -49,6 +49,18 @@ config STM32_LPTIMER_CNT
> To compile this driver as a module, choose M here: the
> module will be called stm32-lptimer-cnt.
>
> +config TI_EQEP
> + tristate "TI eQEP counter driver"
> + depends on (SOC_AM33XX || COMPILE_TEST)
> + select PWM
> + select REGMAP_MMIO
> + help
> + Select this option to enable the Texas Instruments Enhanced Quadrature
> + Encoder Pulse (eQEP) counter driver.
> +
> + To compile this driver as a module, choose M here: the module will be
> + called ti-eqep.
> +
> config FTM_QUADDEC
> tristate "Flex Timer Module Quadrature decoder driver"
> depends on HAS_IOMEM && OF
> diff --git a/drivers/counter/Makefile b/drivers/counter/Makefile
> index 40d35522937d..55142d1f4c43 100644
> --- a/drivers/counter/Makefile
> +++ b/drivers/counter/Makefile
> @@ -8,4 +8,5 @@ obj-$(CONFIG_COUNTER) += counter.o
> obj-$(CONFIG_104_QUAD_8) += 104-quad-8.o
> obj-$(CONFIG_STM32_TIMER_CNT) += stm32-timer-cnt.o
> obj-$(CONFIG_STM32_LPTIMER_CNT) += stm32-lptimer-cnt.o
> +obj-$(CONFIG_TI_EQEP) += ti-eqep.o
> obj-$(CONFIG_FTM_QUADDEC) += ftm-quaddec.o
> diff --git a/drivers/counter/ti-eqep.c b/drivers/counter/ti-eqep.c
> new file mode 100644
> index 000000000000..7aaa4abbc9c5
> --- /dev/null
> +++ b/drivers/counter/ti-eqep.c
> @@ -0,0 +1,381 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2019 David Lechner <david@lechnology.com>
> + *
> + * Counter driver for Texas Instruments Enhanced Quadrature Encoder Pulse (eQEP)
> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/counter.h>
> +#include <linux/kernel.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> +
> +/* 32-bit registers */
> +#define QPOSCNT 0x0
> +#define QPOSINIT 0x4
> +#define QPOSMAX 0x8
> +#define QPOSCMP 0xc
> +#define QPOSILAT 0x10
> +#define QPOSSLAT 0x14
> +#define QPOSLAT 0x18
> +#define QUTMR 0x1c
> +#define QUPRD 0x20
> +
> +/* 16-bit registers */
> +#define QWDTMR 0x0 /* 0x24 */
> +#define QWDPRD 0x2 /* 0x26 */
> +#define QDECCTL 0x4 /* 0x28 */
> +#define QEPCTL 0x6 /* 0x2a */
> +#define QCAPCTL 0x8 /* 0x2c */
> +#define QPOSCTL 0xa /* 0x2e */
> +#define QEINT 0xc /* 0x30 */
> +#define QFLG 0xe /* 0x32 */
> +#define QCLR 0x10 /* 0x34 */
> +#define QFRC 0x12 /* 0x36 */
> +#define QEPSTS 0x14 /* 0x38 */
> +#define QCTMR 0x16 /* 0x3a */
> +#define QCPRD 0x18 /* 0x3c */
> +#define QCTMRLAT 0x1a /* 0x3e */
> +#define QCPRDLAT 0x1c /* 0x40 */
> +
> +#define QDECCTL_QSRC_SHIFT 14
> +#define QDECCTL_QSRC GENMASK(15, 14)
> +#define QDECCTL_SOEN BIT(13)
> +#define QDECCTL_SPSEL BIT(12)
> +#define QDECCTL_XCR BIT(11)
> +#define QDECCTL_SWAP BIT(10)
> +#define QDECCTL_IGATE BIT(9)
> +#define QDECCTL_QAP BIT(8)
> +#define QDECCTL_QBP BIT(7)
> +#define QDECCTL_QIP BIT(6)
> +#define QDECCTL_QSP BIT(5)
> +
> +#define QEPCTL_FREE_SOFT GENMASK(15, 14)
> +#define QEPCTL_PCRM GENMASK(13, 12)
> +#define QEPCTL_SEI GENMASK(11, 10)
> +#define QEPCTL_IEI GENMASK(9, 8)
> +#define QEPCTL_SWI BIT(7)
> +#define QEPCTL_SEL BIT(6)
> +#define QEPCTL_IEL GENMASK(5, 4)
> +#define QEPCTL_PHEN BIT(3)
> +#define QEPCTL_QCLM BIT(2)
> +#define QEPCTL_UTE BIT(1)
> +#define QEPCTL_WDE BIT(0)
> +
> +/* EQEP Inputs */
> +enum {
> + TI_EQEP_SIGNAL_QEPA, /* QEPA/XCLK */
> + TI_EQEP_SIGNAL_QEPB, /* QEPB/XDIR */
> + TI_EQEP_SIGNAL_QEPI, /* Index */
> + TI_EQEP_SIGNAL_QEPS, /* Strobe */
> +};
> +
> +/* Position Counter Input Modes */
> +enum {
> + TI_EQEP_COUNT_FUNC_QUAD_COUNT,
> + TI_EQEP_COUNT_FUNC_DIR_COUNT,
> + TI_EQEP_COUNT_FUNC_UP_COUNT,
> + TI_EQEP_COUNT_FUNC_DOWN_COUNT,
> +};
> +
> +struct ti_eqep_cnt {
> + struct counter_device counter;
> + struct regmap *regmap32;
> + struct regmap *regmap16;
> +};
> +
> +static int ti_eqep_count_read(struct counter_device *counter,
> + struct counter_count *count,
> + struct counter_count_read_value *val)
> +{
> + struct ti_eqep_cnt *priv = counter->priv;
> + u32 cnt;
> +
> + regmap_read(priv->regmap32, QPOSCNT, &cnt);
> + counter_count_read_value_set(val, COUNTER_COUNT_POSITION, &cnt);
> +
> + return 0;
> +}
> +
> +static int ti_eqep_count_write(struct counter_device *counter,
> + struct counter_count *count,
> + struct counter_count_write_value *val)
> +{
> + struct ti_eqep_cnt *priv = counter->priv;
> + u32 cnt, max;
> + int err;
> +
> + err = counter_count_write_value_get(&cnt, COUNTER_COUNT_POSITION, val);
> + if (err)
> + return err;
> +
> + regmap_read(priv->regmap32, QPOSMAX, &max);
> + if (cnt > max)
> + return -EINVAL;
> +
> + return regmap_write(priv->regmap32, QPOSCNT, cnt);
> +}
> +
> +static int ti_eqep_function_get(struct counter_device *counter,
> + struct counter_count *count, size_t *function)
> +{
> + struct ti_eqep_cnt *priv = counter->priv;
> + u32 qdecctl;
> +
> + regmap_read(priv->regmap16, QDECCTL, &qdecctl);
> + *function = (qdecctl & QDECCTL_QSRC) >> QDECCTL_QSRC_SHIFT;
> +
> + return 0;
> +}
> +
> +static int ti_eqep_function_set(struct counter_device *counter,
> + struct counter_count *count, size_t function)
> +{
> + struct ti_eqep_cnt *priv = counter->priv;
> +
> + return regmap_write_bits(priv->regmap16, QDECCTL, QDECCTL_QSRC,
> + function << QDECCTL_QSRC_SHIFT);
> +}
> +
> +static ssize_t ti_eqep_position_ceiling_read(struct counter_device *counter,
> + struct counter_count *count,
> + void *ext_priv, char *buf)
> +{
> + struct ti_eqep_cnt *priv = counter->priv;
> + u32 qposmax;
> +
> + regmap_read(priv->regmap32, QPOSMAX, &qposmax);
> +
> + return sprintf(buf, "%u\n", qposmax);
> +}
> +
> +static ssize_t ti_eqep_position_ceiling_write(struct counter_device *counter,
> + struct counter_count *count,
> + void *ext_priv, const char *buf,
> + size_t len)
> +{
> + struct ti_eqep_cnt *priv = counter->priv;
> + int err;
> + u32 res;
> +
> + err = kstrtouint(buf, 10, &res);
In Documentation/ABI/testing/sysfs-bus-counter, a base is not specified
for the ceiling attribute so the expectation is for the base of the
string to be automatically detected with the conventional semantics.
That means you should pass 0 here to kstrtouint instead of 10.
> + if (err < 0)
> + return err;
> +
> + regmap_write(priv->regmap32, QPOSMAX, res);
> +
> + return len;
> +}
> +
> +static ssize_t ti_eqep_position_floor_read(struct counter_device *counter,
> + struct counter_count *count,
> + void *ext_priv, char *buf)
> +{
> + struct ti_eqep_cnt *priv = counter->priv;
> + u32 qposinit;
> +
> + regmap_read(priv->regmap32, QPOSINIT, &qposinit);
> +
> + return sprintf(buf, "%u\n", qposinit);
> +}
> +
> +static ssize_t ti_eqep_position_floor_write(struct counter_device *counter,
> + struct counter_count *count,
> + void *ext_priv, const char *buf,
> + size_t len)
> +{
> + struct ti_eqep_cnt *priv = counter->priv;
> + int err;
> + u32 res;
> +
> + err = kstrtouint(buf, 10, &res);
For the same reason as the ceiling attribute, you should pass 0 here to
kstrtouint instead of 10.
> + if (err < 0)
> + return err;
> +
> + regmap_write(priv->regmap32, QPOSINIT, res);
> +
> + return len;
> +}
> +
> +static ssize_t ti_eqep_position_enable_read(struct counter_device *counter,
> + struct counter_count *count,
> + void *ext_priv, char *buf)
> +{
> + struct ti_eqep_cnt *priv = counter->priv;
> + u32 qepctl;
> +
> + regmap_read(priv->regmap16, QEPCTL, &qepctl);
> +
> + return sprintf(buf, "%u\n", !!(qepctl & QEPCTL_PHEN));
> +}
> +
> +static ssize_t ti_eqep_position_enable_write(struct counter_device *counter,
> + struct counter_count *count,
> + void *ext_priv, const char *buf,
> + size_t len)
> +{
> + struct ti_eqep_cnt *priv = counter->priv;
> + int err;
> + bool res;
> +
> + err = kstrtobool(buf, &res);
> + if (err < 0)
> + return err;
> +
> + regmap_write_bits(priv->regmap16, QEPCTL, QEPCTL_PHEN, res ? -1 : 0);
> +
> + return len;
> +}
> +
> +static const struct regmap_config ti_eqep_regmap32_config = {
> + .name = "32-bit",
> + .reg_bits = 32,
> + .val_bits = 32,
> + .reg_stride = 4,
> + .max_register = 0x24,
> +};
> +
> +static const struct regmap_config ti_eqep_regmap16_config = {
> + .name = "16-bit",
> + .reg_bits = 16,
> + .val_bits = 16,
> + .reg_stride = 2,
> + .max_register = 0x1e,
> +};
> +
> +static const struct counter_ops ti_eqep_counter_ops = {
> + .count_read = ti_eqep_count_read,
> + .count_write = ti_eqep_count_write,
> + .function_get = ti_eqep_function_get,
> + .function_set = ti_eqep_function_set,
> +};
Are you able to provide a signal_read function, or are the Signals not
exposed to the user by this device? Sometimes quadrature encoder devices
provide an instanteous read of the signal lines to tell whether they are
high or low, so I figured I'd ask.
You should define an action_get function as well along with Synapses
corresponding to each Signal. This will allow users to know whether the
Synapse fires on a rising edge, falling edge, no edge, or both edges.
For example, consider the drivers/counter/104-quad-8.c file. Each count
register has three associated signal lines: Quadrature A, Quadrature B,
and Index.
Quadrature A and B are your typical quadrature encoder lines and
depending on the function mode selected (quadrature x4, pulse-direction,
etc.) could have a Synapse action mode of none, rising edge, falling
edge, or both edges; see the quad8_synapse_actions_list array.
In contrast, the Index signal line only has two Synapse action modes:
rising edge (in the case preset functionality is enabled) or none.
For the TI eQEP driver, there will be four Synapses corresponding to the
four Signals: QEPA, QEPB, QEPI, and QEPS. See if you are able to
implement the Synapses and action_get function by using the 104-quad-8.c
file as a reference. That file does have a lot of extra functionality
tossed in compared to yours, so if you have trouble groking it, just let
me know and I'll try to help.
> +
> +static const enum counter_count_function ti_eqep_position_functions[] = {
> + [TI_EQEP_COUNT_FUNC_QUAD_COUNT] = COUNTER_COUNT_FUNCTION_QUADRATURE_X4,
> + [TI_EQEP_COUNT_FUNC_DIR_COUNT] = COUNTER_COUNT_FUNCTION_PULSE_DIRECTION,
> + [TI_EQEP_COUNT_FUNC_UP_COUNT] = COUNTER_COUNT_FUNCTION_INCREASE,
> + [TI_EQEP_COUNT_FUNC_DOWN_COUNT] = COUNTER_COUNT_FUNCTION_DECREASE,
> +};
> +
> +static struct counter_signal ti_eqep_signals[] = {
> + [TI_EQEP_SIGNAL_QEPA] = {
> + .id = TI_EQEP_SIGNAL_QEPA,
> + .name = "QEPA"
> + },
> + [TI_EQEP_SIGNAL_QEPB] = {
> + .id = TI_EQEP_SIGNAL_QEPB,
> + .name = "QEPB"
> + },
> + [TI_EQEP_SIGNAL_QEPI] = {
> + .id = TI_EQEP_SIGNAL_QEPI,
> + .name = "QEPI"
> + },
> + [TI_EQEP_SIGNAL_QEPS] = {
> + .id = TI_EQEP_SIGNAL_QEPS,
> + .name = "QEPS"
> + },
> +};
> +
> +static struct counter_count_ext ti_eqep_position_ext[] = {
> + {
> + .name = "ceiling",
> + .read = ti_eqep_position_ceiling_read,
> + .write = ti_eqep_position_ceiling_write,
> + },
> + {
> + .name = "floor",
> + .read = ti_eqep_position_floor_read,
> + .write = ti_eqep_position_floor_write,
> + },
> + {
> + .name = "enable",
> + .read = ti_eqep_position_enable_read,
> + .write = ti_eqep_position_enable_write,
> + },
> +};
> +
> +static struct counter_count ti_eqep_counts[] = {
> + {
> + .id = 0,
> + .name = "QPOSCNT",
> + .functions_list = ti_eqep_position_functions,
> + .num_functions = ARRAY_SIZE(ti_eqep_position_functions),
> + .ext = ti_eqep_position_ext,
> + .num_ext = ARRAY_SIZE(ti_eqep_position_ext),
When you have your Synapses defined, pass them here to your Count via
the synapses and num_synapses members.
> + },
> +};
> +
> +static int ti_eqep_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct ti_eqep_cnt *priv;
> + struct resource *res;
> + void __iomem *base;
> +
> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + base = devm_ioremap_resource(dev, res);
> + if (IS_ERR(base))
> + return PTR_ERR(base);
> +
> + priv->regmap32 = devm_regmap_init_mmio(dev, base,
> + &ti_eqep_regmap32_config);
> + if (IS_ERR(priv->regmap32))
> + return PTR_ERR(priv->regmap32);
> +
> + priv->regmap16 = devm_regmap_init_mmio(dev, base + 0x24,
> + &ti_eqep_regmap16_config);
> + if (IS_ERR(priv->regmap16))
> + return PTR_ERR(priv->regmap16);
> +
> + priv->counter.name = dev_name(dev);
> + priv->counter.parent = dev;
> + priv->counter.ops = &ti_eqep_counter_ops;
> + priv->counter.counts = ti_eqep_counts;
> + priv->counter.num_counts = ARRAY_SIZE(ti_eqep_counts);
> + priv->counter.signals = ti_eqep_signals;
> + priv->counter.num_signals = ARRAY_SIZE(ti_eqep_signals);
> + priv->counter.priv = priv;
> +
> + pm_runtime_enable(dev);
> + pm_runtime_get(dev);
> +
> + return devm_counter_register(dev, &priv->counter);
> +}
> +
> +static int ti_eqep_remove(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> +
> + pm_runtime_put(dev),
> + pm_runtime_disable(dev);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id ti_eqep_of_match[] = {
> + { .compatible = "ti,am3352-eqep", },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, ti_eqep_of_match);
> +
> +static struct platform_driver ti_eqep_driver = {
> + .probe = ti_eqep_probe,
> + .remove = ti_eqep_remove,
> + .driver = {
> + .name = "ti-eqep-cnt",
> + .of_match_table = ti_eqep_of_match,
> + },
> +};
> +module_platform_driver(ti_eqep_driver);
> +
> +MODULE_AUTHOR("David Lechner <david@lechnology.com>");
> +MODULE_DESCRIPTION("TI eQEP counter driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> index a7e57516959e..ddcbb8573894 100644
> --- a/drivers/pwm/Kconfig
> +++ b/drivers/pwm/Kconfig
> @@ -499,7 +499,7 @@ config PWM_TIEHRPWM
>
> config PWM_TIPWMSS
> bool
> - default y if (ARCH_OMAP2PLUS) && (PWM_TIECAP || PWM_TIEHRPWM)
> + default y if (ARCH_OMAP2PLUS) && (PWM_TIECAP || PWM_TIEHRPWM || TI_EQEP)
> help
> PWM Subsystem driver support for AM33xx SOC.
I was surprised to see this pwm Kconfig change in this patch. Is
PWM_TIPWMSS required for TI_EQEP to work? If not required, then this
could be a separate patch; otherwise, put in a mention about why in the
commit message so that the purpose of this change is clearer.
>
> --
> 2.17.1
>
^ permalink raw reply
* Re: [PATCH 0/4] dt-bindings: i2c: renesas: Rename bindings documentation files
From: Geert Uytterhoeven @ 2019-08-02 9:27 UTC (permalink / raw)
To: Wolfram Sang
Cc: Simon Horman, Wolfram Sang, Chris Brandt, Rob Herring,
Mark Rutland, Magnus Damm, Linux I2C,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux-Renesas
In-Reply-To: <20190801125750.GM1659@ninjato>
Hi Wolfram,
On Thu, Aug 1, 2019 at 3:16 PM Wolfram Sang <wsa@the-dreams.de> wrote:
> On Wed, Jul 24, 2019 at 02:15:55PM +0200, Simon Horman wrote:
> > Rename the bindings documentation file for Renesas I2C controllers.
> >
> > This is part of an ongoing effort to name bindings documentation files for
> > Renesas IP blocks consistently, in line with the compat strings they
> > document.
> >
> > Based on v5.3-rc1
> >
> > Simon Horman (4):
> > dt-bindings: i2c: sh_mobile: Rename bindings documentation file
> > dt-bindings: i2c: rcar: Rename bindings documentation file
> > dt-bindings: i2c: riic: Rename bindings documentation file
> > dt-bindings: i2c: riic: Rename bindings documentation file
>
> Applied to for-next, thanks!
Without the wrong names fixed?
And with an R-b I didn't give, probably due to the two last patches having
the same oneline-summary?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH/RFC 12/12] arm64: dts: renesas: Add EK874 board with idk-2121wr display support
From: Geert Uytterhoeven @ 2019-08-02 9:11 UTC (permalink / raw)
To: Fabrizio Castro
Cc: Laurent Pinchart, Kieran Bingham, Jacopo Mondi, Rob Herring,
Mark Rutland, Simon Horman, Magnus Damm, Linux-Renesas,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Geert Uytterhoeven, Chris Paterson, Biju Das, ebiharaml
In-Reply-To: <1564731249-22671-13-git-send-email-fabrizio.castro@bp.renesas.com>
Hi Fabrizio,
On Fri, Aug 2, 2019 at 9:35 AM Fabrizio Castro
<fabrizio.castro@bp.renesas.com> wrote:
> The EK874 is advertised as compatible with panel IDK-2121WR from
> Advantech, however the panel isn't sold alongside the board.
> A new dts, adding everything that's required to get the panel to
> to work with the EK874, is the most convenient way to support the
> EK874 when it's connected to the IDK-2121WR.
>
> Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
Thanks for your patch!
> --- /dev/null
> +++ b/arch/arm64/boot/dts/renesas/r8a774c0-ek874-idk-2121wr.dts
> @@ -0,0 +1,112 @@
[...]
> + panel-lvds {
> + compatible = "advantech,idk-2121wr", "panel-lvds";
> +
> + width-mm = <476>;
> + height-mm = <268>;
> +
> + data-mapping = "vesa-24";
> +
> + panel-timing {
> + clock-frequency = <148500000>;
> + hactive = <1920>;
> + vactive = <1080>;
> + hsync-len = <44>;
> + hfront-porch = <88>;
> + hback-porch = <148>;
> + vfront-porch = <4>;
> + vback-porch = <36>;
> + vsync-len = <5>;
> + };
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> + lvds0_panel_in: endpoint {
> + remote-endpoint = <&lvds0_out>;
> + };
> + };
> +
> + port@1 {
> + reg = <1>;
> + lvds1_panel_in: endpoint {
> + remote-endpoint = <&lvds1_out>;
> + };
> + };
> + };
> + };
> +};
[...]
> +&lvds0 {
> + renesas,swap-data;
> +
> + ports {
> + port@1 {
> + lvds0_out: endpoint {
> + remote-endpoint = <&lvds0_panel_in>;
> + };
> + };
> + };
> +};
> +
> +&lvds1 {
> + status = "okay";
> +
> + clocks = <&cpg CPG_MOD 727>, <&x13_clk>, <&extal_clk>;
> + clock-names = "fck", "dclkin.0", "extal";
> +
> + ports {
> + port@1 {
> + lvds1_out: endpoint {
> + remote-endpoint = <&lvds1_panel_in>;
> + };
> + };
> + };
> +};
Shouldn't the actual panel definition, and the lvds remote-endpoint setup,
be extracted into a separate .dtsi, to be included here?
Cfr. arch/arm/boot/dts/r8a77xx-aa104xd12-panel.dtsi and
arch/arm/boot/dts/r8a77xx-aa121td01-panel.dtsi.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH/RFC 10/12] arm64: dts: renesas: r8a774c0: Point LVDS0 to its companion LVDS1
From: Laurent Pinchart @ 2019-08-02 9:10 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Fabrizio Castro, Kieran Bingham, Jacopo Mondi, Rob Herring,
Mark Rutland, Simon Horman, Magnus Damm, Linux-Renesas,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Geert Uytterhoeven, Chris Paterson, Biju Das
In-Reply-To: <CAMuHMdUFHddHJW=FsF8Ha0PZUiAyKrWGy6yg-0PtevB7HiHoHg@mail.gmail.com>
Hi Geert,
On Fri, Aug 02, 2019 at 11:03:54AM +0200, Geert Uytterhoeven wrote:
> On Fri, Aug 2, 2019 at 10:27 AM Laurent Pinchart wrote:
> > On Fri, Aug 02, 2019 at 08:34:07AM +0100, Fabrizio Castro wrote:
> > > Add the new renesas,companion property to the LVDS0 node to point to the
> > > companion LVDS encoder LVDS1.
> > > Based on similar work from Laurent Pinchart for the r8a7799[05].
> > >
> > > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> >
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >
> > and taken in my tree.
>
> Shouldn't this go through renesas-devel and arm-soc?
I'm collecting multimedia-related DT patches for v5.4, but if you or
Simon want to take this patch, it will save me from sending a pull
request, so please go ahead :-)
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH/RFC 10/12] arm64: dts: renesas: r8a774c0: Point LVDS0 to its companion LVDS1
From: Geert Uytterhoeven @ 2019-08-02 9:03 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Fabrizio Castro, Kieran Bingham, Jacopo Mondi, Rob Herring,
Mark Rutland, Simon Horman, Magnus Damm, Linux-Renesas,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Geert Uytterhoeven, Chris Paterson, Biju Das
In-Reply-To: <20190802082754.GK5008@pendragon.ideasonboard.com>
Hi Laurent,
On Fri, Aug 2, 2019 at 10:27 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Fri, Aug 02, 2019 at 08:34:07AM +0100, Fabrizio Castro wrote:
> > Add the new renesas,companion property to the LVDS0 node to point to the
> > companion LVDS encoder LVDS1.
> > Based on similar work from Laurent Pinchart for the r8a7799[05].
> >
> > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> and taken in my tree.
Shouldn't this go through renesas-devel and arm-soc?
> > ---
> > arch/arm64/boot/dts/renesas/r8a774c0.dtsi | 2 ++
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH v2 5/5] ARM: dts: stm32: remove phy-dsi-supply property on stm32mp157c-dk2 board
From: Yannick FERTRE @ 2019-08-02 9:02 UTC (permalink / raw)
To: Philippe CORNU, Benjamin GAIGNARD, Vincent ABRIOU, David Airlie,
Daniel Vetter, Rob Herring, Mark Rutland, Maxime Coquelin,
Alexandre TORGUE, dri-devel@lists.freedesktop.org,
devicetree@vger.kernel.org,
linux-stm32@st-md-mailman.stormreply.com,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
In-Reply-To: <1557498023-10766-6-git-send-email-yannick.fertre@st.com>
Hi Alexandre,
this patch can be abandoned.
BR
--
Yannick Fertré | TINA: 166 7152 | Tel: +33 244027152 | Mobile: +33 620600270
Microcontrollers and Digital ICs Group | Microcontrolleurs Division
On 5/10/19 4:20 PM, Yannick Fertré wrote:
> This property is already defined into stm32mp157c.dtsi file.
>
> Signed-off-by: Yannick Fertré <yannick.fertre@st.com>
> ---
> arch/arm/boot/dts/stm32mp157c-dk2.dts | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/stm32mp157c-dk2.dts b/arch/arm/boot/dts/stm32mp157c-dk2.dts
> index 020ea0f..09f6e7b 100644
> --- a/arch/arm/boot/dts/stm32mp157c-dk2.dts
> +++ b/arch/arm/boot/dts/stm32mp157c-dk2.dts
> @@ -17,7 +17,6 @@
> #address-cells = <1>;
> #size-cells = <0>;
> status = "okay";
> - phy-dsi-supply = <®18>;
>
> ports {
> #address-cells = <1>;
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH v2 4/5] ARM: dts: stm32: move fixe regulators reg11 & reg18
From: Yannick FERTRE @ 2019-08-02 9:02 UTC (permalink / raw)
To: Philippe CORNU, Benjamin GAIGNARD, Vincent ABRIOU, David Airlie,
Daniel Vetter, Rob Herring, Mark Rutland, Maxime Coquelin,
Alexandre TORGUE, dri-devel@lists.freedesktop.org,
devicetree@vger.kernel.org,
linux-stm32@st-md-mailman.stormreply.com,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
In-Reply-To: <1557498023-10766-5-git-send-email-yannick.fertre@st.com>
Hi Alexandre,
this patch can be abandoned.
BR
--
Yannick Fertré | TINA: 166 7152 | Tel: +33 244027152 | Mobile: +33 620600270
Microcontrollers and Digital ICs Group | Microcontrolleurs Division
On 5/10/19 4:20 PM, Yannick Fertré wrote:
> Move regulators reg11 & reg18 from device-tree files stm32mp157c-ed1.dts
> & stm32mp157c-dk2.dts to file stm32mp157c.dtsi.
>
> Signed-off-by: Yannick Fertré <yannick.fertre@st.com>
> ---
> arch/arm/boot/dts/stm32mp157c-dk2.dts | 8 --------
> arch/arm/boot/dts/stm32mp157c-ed1.dts | 16 ----------------
> arch/arm/boot/dts/stm32mp157c.dtsi | 16 ++++++++++++++++
> 3 files changed, 16 insertions(+), 24 deletions(-)
>
> diff --git a/arch/arm/boot/dts/stm32mp157c-dk2.dts b/arch/arm/boot/dts/stm32mp157c-dk2.dts
> index 20ea601..020ea0f 100644
> --- a/arch/arm/boot/dts/stm32mp157c-dk2.dts
> +++ b/arch/arm/boot/dts/stm32mp157c-dk2.dts
> @@ -11,14 +11,6 @@
> / {
> model = "STMicroelectronics STM32MP157C-DK2 Discovery Board";
> compatible = "st,stm32mp157c-dk2", "st,stm32mp157";
> -
> - reg18: reg18 {
> - compatible = "regulator-fixed";
> - regulator-name = "reg18";
> - regulator-min-microvolt = <1800000>;
> - regulator-max-microvolt = <1800000>;
> - regulator-always-on;
> - };
> };
>
> &dsi {
> diff --git a/arch/arm/boot/dts/stm32mp157c-ed1.dts b/arch/arm/boot/dts/stm32mp157c-ed1.dts
> index 62a8c78..f41189c 100644
> --- a/arch/arm/boot/dts/stm32mp157c-ed1.dts
> +++ b/arch/arm/boot/dts/stm32mp157c-ed1.dts
> @@ -27,22 +27,6 @@
> serial0 = &uart4;
> };
>
> - reg11: reg11 {
> - compatible = "regulator-fixed";
> - regulator-name = "reg11";
> - regulator-min-microvolt = <1100000>;
> - regulator-max-microvolt = <1100000>;
> - regulator-always-on;
> - };
> -
> - reg18: reg18 {
> - compatible = "regulator-fixed";
> - regulator-name = "reg18";
> - regulator-min-microvolt = <1800000>;
> - regulator-max-microvolt = <1800000>;
> - regulator-always-on;
> - };
> -
> sd_switch: regulator-sd_switch {
> compatible = "regulator-gpio";
> regulator-name = "sd_switch";
> diff --git a/arch/arm/boot/dts/stm32mp157c.dtsi b/arch/arm/boot/dts/stm32mp157c.dtsi
> index 6b14f1e..aaac51cd 100644
> --- a/arch/arm/boot/dts/stm32mp157c.dtsi
> +++ b/arch/arm/boot/dts/stm32mp157c.dtsi
> @@ -11,6 +11,22 @@
> #address-cells = <1>;
> #size-cells = <1>;
>
> + reg11: reg11 {
> + compatible = "regulator-fixed";
> + regulator-name = "reg11";
> + regulator-min-microvolt = <1100000>;
> + regulator-max-microvolt = <1100000>;
> + regulator-always-on;
> + };
> +
> + reg18: reg18 {
> + compatible = "regulator-fixed";
> + regulator-name = "reg18";
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + regulator-always-on;
> + };
> +
> cpus {
> #address-cells = <1>;
> #size-cells = <0>;
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH v2 3/5] ARM: dts: stm32: add phy-dsi-supply property on stm32mp157c
From: Yannick FERTRE @ 2019-08-02 9:01 UTC (permalink / raw)
To: Philippe CORNU, Benjamin GAIGNARD, Vincent ABRIOU, David Airlie,
Daniel Vetter, Rob Herring, Mark Rutland, Maxime Coquelin,
Alexandre TORGUE, dri-devel@lists.freedesktop.org,
devicetree@vger.kernel.org,
linux-stm32@st-md-mailman.stormreply.com,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
In-Reply-To: <1557498023-10766-4-git-send-email-yannick.fertre@st.com>
Hi Alexandre,
this patch can be abandoned.
BR
--
Yannick Fertré | TINA: 166 7152 | Tel: +33 244027152 | Mobile: +33 620600270
Microcontrollers and Digital ICs Group | Microcontrolleurs Division
On 5/10/19 4:20 PM, Yannick Fertré wrote:
> The dsi physical layer is powered by the 1v8 power controller supply.
>
> Signed-off-by: Yannick Fertré <yannick.fertre@st.com>
> ---
> arch/arm/boot/dts/stm32mp157c.dtsi | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/boot/dts/stm32mp157c.dtsi b/arch/arm/boot/dts/stm32mp157c.dtsi
> index 2afeee6..6b14f1e 100644
> --- a/arch/arm/boot/dts/stm32mp157c.dtsi
> +++ b/arch/arm/boot/dts/stm32mp157c.dtsi
> @@ -1156,6 +1156,7 @@
> clock-names = "pclk", "ref", "px_clk";
> resets = <&rcc DSI_R>;
> reset-names = "apb";
> + phy-dsi-supply = <®18>;
> status = "disabled";
> };
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH v2 07/20] ARM: dts: imx7-colibri: fix 1.8V/UHS support
From: Stefan Agner @ 2019-08-02 8:51 UTC (permalink / raw)
To: Philippe Schenker
Cc: festevam, s.hauer, Marcel Ziswiler, kernel, Max Krummenacher,
mark.rutland, devicetree, michal.vokac, shawnguo, Stefan Agner,
linux-kernel, robh+dt, linux-imx, linux-arm-kernel
In-Reply-To: <723f191c5893984c8fbe711163524dc7ebf09a5b.camel@toradex.com>
On 2019-07-31 16:52, Philippe Schenker wrote:
> On Wed, 2019-07-31 at 09:56 -0300, Fabio Estevam wrote:
>> On Wed, Jul 31, 2019 at 9:38 AM Philippe Schenker
>> <philippe.schenker@toradex.com> wrote:
>> > From: Stefan Agner <stefan.agner@toradex.com>
>> >
>> > Add pinmuxing and do not specify voltage restrictions in the
>> > module level device tree.
>>
>> It would be nice to explain the reason for doing this.
>
> This commit is in preparation of another patch that didn't made into this
> patchset (downstream stuff in there). But I will do another patch on top that
> will use this patch here. That should anyway be in mainline.
I guess what Fabio meant here is explain this patch.
The commit message really could be improved, e.g.:
Add pinmuxing and do not specify voltage restrictions for the usdhc
instance
available on the modules edge connector. This allows to use SD-cards
with
higher transfer modes if supported by the carrier board.
--
Stefan
>
> Philippe
>
>>
>> > Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
>> > Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com>
>> > ---
>> >
>> > Changes in v2: None
>> >
>> > arch/arm/boot/dts/imx7-colibri.dtsi | 23 ++++++++++++++++++++++-
>> > 1 file changed, 22 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/arch/arm/boot/dts/imx7-colibri.dtsi b/arch/arm/boot/dts/imx7-
>> > colibri.dtsi
>> > index 16d1a1ed1aff..67f5e0c87fdc 100644
>> > --- a/arch/arm/boot/dts/imx7-colibri.dtsi
>> > +++ b/arch/arm/boot/dts/imx7-colibri.dtsi
>> > @@ -326,7 +326,6 @@
>> > &usdhc1 {
>> > pinctrl-names = "default";
>> > pinctrl-0 = <&pinctrl_usdhc1 &pinctrl_cd_usdhc1>;
>> > - no-1-8-v;
>> > cd-gpios = <&gpio1 0 GPIO_ACTIVE_LOW>;
>> > disable-wp;
>> > vqmmc-supply = <®_LDO2>;
>> > @@ -671,6 +670,28 @@
>> > >;
>> > };
>> >
>> > + pinctrl_usdhc1_100mhz: usdhc1grp_100mhz {
>> > + fsl,pins = <
>> > + MX7D_PAD_SD1_CMD__SD1_CMD 0x5a
>> > + MX7D_PAD_SD1_CLK__SD1_CLK 0x1a
>> > + MX7D_PAD_SD1_DATA0__SD1_DATA0 0x5a
>> > + MX7D_PAD_SD1_DATA1__SD1_DATA1 0x5a
>> > + MX7D_PAD_SD1_DATA2__SD1_DATA2 0x5a
>> > + MX7D_PAD_SD1_DATA3__SD1_DATA3 0x5a
>> > + >;
>> > + };
>> > +
>> > + pinctrl_usdhc1_200mhz: usdhc1grp_200mhz {
>> > + fsl,pins = <
>> > + MX7D_PAD_SD1_CMD__SD1_CMD 0x5b
>> > + MX7D_PAD_SD1_CLK__SD1_CLK 0x1b
>> > + MX7D_PAD_SD1_DATA0__SD1_DATA0 0x5b
>> > + MX7D_PAD_SD1_DATA1__SD1_DATA1 0x5b
>> > + MX7D_PAD_SD1_DATA2__SD1_DATA2 0x5b
>> > + MX7D_PAD_SD1_DATA3__SD1_DATA3 0x5b
>> > + >;
>> > + };
>>
>> You add the entries for 100MHz and 200MHz, but I don't see them being
>> referenced anywhere.
^ permalink raw reply
* Re: [PATCH] ARM: dts: stm32: add DFSDM pins to stm32mp157c
From: Olivier MOYSAN @ 2019-08-02 8:49 UTC (permalink / raw)
To: Alexandre TORGUE, linux-stm32@st-md-mailman.stormreply.com,
robh@kernel.org, mark.rutland@arm.com, devicetree@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
In-Reply-To: <a95e5d74-c8e3-42f9-cabf-f42623aee255@st.com>
Hi ALex,
On 8/2/19 10:09 AM, Alexandre Torgue wrote:
> Hi Olivier
>
> On 8/1/19 9:46 AM, Olivier Moysan wrote:
>> Add DFSDM pins to stm32mp157c.
>>
>> Signed-off-by: Olivier Moysan <olivier.moysan@st.com>
>> ---
>> arch/arm/boot/dts/stm32mp157-pinctrl.dtsi | 39 +++++++++++++++++++++++++++++++
>> 1 file changed, 39 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/stm32mp157-pinctrl.dtsi b/arch/arm/boot/dts/stm32mp157-pinctrl.dtsi
>> index 9eaec9bf8cb8..f96a928cbc49 100644
>> --- a/arch/arm/boot/dts/stm32mp157-pinctrl.dtsi
>> +++ b/arch/arm/boot/dts/stm32mp157-pinctrl.dtsi
>> @@ -230,6 +230,45 @@
>> };
>> };
>>
>
> I use to only take pinconfig which are used in board. So please resend
> with the "board patch".
>
The DFSDM is one of the interface used in the STM32MP15 soundcard.
This soundcard also uses the Wolfson wm8994 audio codec.
The wm8994 codec driver requires adaptations, and the upstream of
these changes is not planned today.
So, the related board patches cannot be sent.
BRs
Olivier
> regards
> Alex
>
>
>> + dfsdm_clkout_pins_a: dfsdm-clkout-pins-0 {
>> + pins {
>> + pinmux = <STM32_PINMUX('B', 13, AF3)>; /* DFSDM_CKOUT */
>> + bias-disable;
>> + drive-push-pull;
>> + slew-rate = <0>;
>> + };
>> + };
>> +
>> + dfsdm_clkout_sleep_pins_a: dfsdm-clkout-sleep-pins-0 {
>> + pins {
>> + pinmux = <STM32_PINMUX('B', 13, ANALOG)>; /* DFSDM_CKOUT */
>> + };
>> + };
>> +
>> + dfsdm_data1_pins_a: dfsdm-data1-pins-0 {
>> + pins {
>> + pinmux = <STM32_PINMUX('C', 3, AF3)>; /* DFSDM_DATA1 */
>> + };
>> + };
>> +
>> + dfsdm_data1_sleep_pins_a: dfsdm-data1-sleep-pins-0 {
>> + pins {
>> + pinmux = <STM32_PINMUX('C', 3, ANALOG)>; /* DFSDM_DATA1 */
>> + };
>> + };
>> +
>> + dfsdm_data3_pins_a: dfsdm-data3-pins-0 {
>> + pins {
>> + pinmux = <STM32_PINMUX('F', 13, AF6)>; /* DFSDM_DATA3 */
>> + };
>> + };
>> +
>> + dfsdm_data3_sleep_pins_a: dfsdm-data3-sleep-pins-0 {
>> + pins {
>> + pinmux = <STM32_PINMUX('F', 13, ANALOG)>; /* DFSDM_DATA3 */
>> + };
>> + };
>> +
>> ethernet0_rgmii_pins_a: rgmii-0 {
>> pins1 {
>> pinmux = <STM32_PINMUX('G', 5, AF11)>, /* ETH_RGMII_CLK125 */
>>
^ permalink raw reply
* Re: [PATCH] scripts/dtc: dtx_diff - add color output support
From: Geert Uytterhoeven @ 2019-08-02 8:44 UTC (permalink / raw)
To: Frank Rowand
Cc: Geert Uytterhoeven, Rob Herring,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
In-Reply-To: <88417bc8-3cd8-bb54-e487-8fa6b0b1f346@gmail.com>
Hi Frank,
On Thu, Aug 1, 2019 at 9:55 PM Frank Rowand <frowand.list@gmail.com> wrote:
> On 8/1/19 5:13 AM, Geert Uytterhoeven wrote:
> > On Wed, Jul 31, 2019 at 10:30 PM Frank Rowand <frowand.list@gmail.com> wrote:
> >> On 7/31/19 5:37 AM, Geert Uytterhoeven wrote:
> >>> Add new -c/--color options, to enhance the diff output with color, and
> >>> improve the user's experience.
> >>>
> >>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >>> ---
> >>> scripts/dtc/dtx_diff | 10 +++++++++-
> >>> 1 file changed, 9 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/scripts/dtc/dtx_diff b/scripts/dtc/dtx_diff
> >>> index e9ad7834a22d9459..4e2c8617f69a333e 100755
> >>> --- a/scripts/dtc/dtx_diff
> >>> +++ b/scripts/dtc/dtx_diff
> >>> @@ -20,6 +20,8 @@ Usage:
> >>>
> >>>
> >>> --annotate synonym for -T
> >>> + --color synonym for -c
> >>> + -c enable colored output
> >>> -f print full dts in diff (--unified=99999)
> >>> -h synonym for --help
> >>> -help synonym for --help
> >
> >> I like the idea, but...
> >>
> >> I have various linux distro releases across my many systems, but only one is
> >> new enough to have the diff command that supports --color.
> >
> > Seems to have been added in diffutils release 3.4 (2016-08-08).
> > I almost can't believe it was that recent, but then I remembered using a
> > wrapper before (colordiff; other wrappers may exist).
> >
> >> Can you enhance this patch to test whether --color is supported? Maybe
> >> something like (untested):
> >>
> >> -c | --color )
> >> if `diff --color <(echo a) <(echo a) 2>/dev/null` ; then
> >> diff_color="--color=always"
> >> fi
> >> shift
> >> ;;
> >>
> >> Then add some text to the usage for -c and --color saying that they will
> >> be silently ignored if diff does not support --color.
> >>
> >> I first wrote up a suggested version that printed an error message and
> >> exited, but I think silently ignoring is more robust, even though it
> >> may be more confusing to someone who is wondering why --color does not
> >> work.
> >
> > Given this is an optional feature, to be enabled explicitly by the user,
> > I'm not so fond of going through hoops to auto-detect the availability.
> >
> > So what about just documenting this in the help text instead?
> >
> > - -c enable colored output
> > + -c enable colored output (requires diff with --color support)
>
> ----- thought 1 -----
>
> My first thought was:
>
> If the hoops were complex and ugly, I might agree with you. But since it is
> a simple one line "if" (two lines including "fi") I prefer the check.
>
> The help text update looks good to me, along with the check.
OK.
> ----- thought 2 -----
>
> Then I reconsidered, and thought "well, Geert has a good idea". So I
> decided to see how useful the diff error message would be. The message is:
>
> $ scripts/dtc/dtx_diff -c a.dts b.dts
> diff: unrecognized option '--color=always'
> diff: Try 'diff --help' for more information.
> $
> Possible hints to resolve the above error:
> (hints might not fix the problem)
>
> No hints available.
>
> It is interesting that the shell prompt arrives before the full set of
> messages from the script, but that is not my issue. My issue is that
That is due to the output coming from the two "<(compile_to_dts ...)"
sub-processes, not from the diff sub-process.
> when the diff fails, the script tries to find suggestions to solve
> the problem. (The suggestions exist to catch some likely problems
> with the shell variable "ARCH".)
Interesting. I didn't know about the hints (never saw them), and had to
try hard to trigger them (I usually do DTB comparisons only).
But I succeeded ;-)
With a small tweak as my diff does support --color:
$ scripts/dtc/dtx_diff -c
arch/arm64/boot/dts/renesas/r8a7799*{ebisu,draak}.dts
diff: unrecognized option '--olor=always'
diff: Try 'diff --help' for more information.
$
Possible hints to resolve the above error:
(hints might not fix the problem)
shell variable $ARCH not set
architecture arm64 is in file path,
but does not match shell variable $ARCH
>>$ARCH<< is: >><<
Possible hints to resolve the above error:
(hints might not fix the problem)
shell variable $ARCH not set
architecture arm64 is in file path,
but does not match shell variable $ARCH
>>$ARCH<< is: >><<
> Unfortunately in the case of the "--color" problem, the useful warning
> from diff becomes less visible because of the early prompt and the
> not so helpful messages about hints.
Yeah, they are not so helpful.
In fact $ARCH is indeed not set, but that's not an issue at all.
> If the hints related messages were not present, then I was ready to
> accept that the diff warning was sufficient. But since the hints
> messages are present, and hiding them would be more complex than
> the original check that I suggested for whether diff supports
> the --color option, I am back to my first thought: I prefer the
> check whether diff supports "--color" is done when dtx_diff detects
> the "--color" option.
OK, you managed to convince me. Will fix.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH v6 11/22] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI
From: Michael Nazzareno Trimarchi @ 2019-08-02 8:38 UTC (permalink / raw)
To: Maxime Ripard
Cc: Jagan Teki, David Airlie, Daniel Vetter, Chen-Yu Tsai,
Michael Turquette, Rob Herring, Mark Rutland, linux-arm-kernel,
linux-kernel, linux-clk, dri-devel, devicetree, linux-amarula,
linux-sunxi
In-Reply-To: <CAOf5uwkvCs62zHcUoFuJwau_ZZFdnVf8ua6JY_wzUb9m8rLTTw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hi Maxime
On Mon, Jul 29, 2019 at 8:59 AM Michael Nazzareno Trimarchi
<michael-dyjBcgdgk7Pe9wHmmfpqLFaTQe2KTcn/@public.gmane.org> wrote:
>
> Hi
>
> On Wed, Jul 24, 2019 at 11:05 AM Maxime Ripard
> <maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> wrote:
> >
> > On Mon, Jul 22, 2019 at 03:51:04PM +0530, Jagan Teki wrote:
> > > Hi Maxime,
> > >
> > > On Sat, Jul 20, 2019 at 3:02 PM Maxime Ripard <maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> wrote:
> > > >
> > > > On Sat, Jul 20, 2019 at 12:46:27PM +0530, Jagan Teki wrote:
> > > > > On Sat, Jul 20, 2019 at 12:28 PM Maxime Ripard
> > > > > <maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> wrote:
> > > > > >
> > > > > > On Thu, Jul 11, 2019 at 07:43:16PM +0200, Michael Nazzareno Trimarchi wrote:
> > > > > > > > > tcon-pixel clock is the rate that you want to achive on display side
> > > > > > > > > and if you have 4 lanes 32bit or lanes and different bit number that
> > > > > > > > > you need to have a clock that is able to put outside bits and speed
> > > > > > > > > equal to pixel-clock * bits / lanes. so If you want a pixel-clock of
> > > > > > > > > 40 mhz and you have 32bits and 4 lanes you need to have a clock of
> > > > > > > > > 40 * 32 / 4 in no-burst mode. I think that this is done but most of
> > > > > > > > > the display.
> > > > > > > >
> > > > > > > > So this is what the issue is then?
> > > > > > > >
> > > > > > > > This one does make sense, and you should just change the rate in the
> > > > > > > > call to clk_set_rate in sun4i_tcon0_mode_set_cpu.
> > > > > > > >
> > > > > > > > I'm still wondering why that hasn't been brought up in either the
> > > > > > > > discussion or the commit log before though.
> > > > > > > >
> > > > > > > Something like this?
> > > > > > >
> > > > > > > drivers/gpu/drm/sun4i/sun4i_tcon.c | 20 +++++++++++---------
> > > > > > > drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h | 2 --
> > > > > > > 2 files changed, 11 insertions(+), 11 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > > b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > > index 64c43ee6bd92..42560d5c327c 100644
> > > > > > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > > @@ -263,10 +263,11 @@ static int sun4i_tcon_get_clk_delay(const struct
> > > > > > > drm_display_mode *mode,
> > > > > > > }
> > > > > > >
> > > > > > > static void sun4i_tcon0_mode_set_common(struct sun4i_tcon *tcon,
> > > > > > > - const struct drm_display_mode *mode)
> > > > > > > + const struct drm_display_mode *mode,
> > > > > > > + u32 tcon_mul)
> > > > > > > {
> > > > > > > /* Configure the dot clock */
> > > > > > > - clk_set_rate(tcon->dclk, mode->crtc_clock * 1000);
> > > > > > > + clk_set_rate(tcon->dclk, mode->crtc_clock * tcon_mul * 1000);
> > > > > > >
> > > > > > > /* Set the resolution */
> > > > > > > regmap_write(tcon->regs, SUN4I_TCON0_BASIC0_REG,
> > > > > > > @@ -335,12 +336,13 @@ static void sun4i_tcon0_mode_set_cpu(struct
> > > > > > > sun4i_tcon *tcon,
> > > > > > > u8 bpp = mipi_dsi_pixel_format_to_bpp(device->format);
> > > > > > > u8 lanes = device->lanes;
> > > > > > > u32 block_space, start_delay;
> > > > > > > - u32 tcon_div;
> > > > > > > + u32 tcon_div, tcon_mul;
> > > > > > >
> > > > > > > - tcon->dclk_min_div = SUN6I_DSI_TCON_DIV;
> > > > > > > - tcon->dclk_max_div = SUN6I_DSI_TCON_DIV;
> > > > > > > + tcon->dclk_min_div = 4;
> > > > > > > + tcon->dclk_max_div = 127;
> > > > > > >
> > > > > > > - sun4i_tcon0_mode_set_common(tcon, mode);
> > > > > > > + tcon_mul = bpp / lanes;
> > > > > > > + sun4i_tcon0_mode_set_common(tcon, mode, tcon_mul);
> > > > > > >
> > > > > > > /* Set dithering if needed */
> > > > > > > sun4i_tcon0_mode_set_dithering(tcon, sun4i_tcon_get_connector(encoder));
> > > > > > > @@ -366,7 +368,7 @@ static void sun4i_tcon0_mode_set_cpu(struct
> > > > > > > sun4i_tcon *tcon,
> > > > > > > */
> > > > > > > regmap_read(tcon->regs, SUN4I_TCON0_DCLK_REG, &tcon_div);
> > > > > > > tcon_div &= GENMASK(6, 0);
> > > > > > > - block_space = mode->htotal * bpp / (tcon_div * lanes);
> > > > > > > + block_space = mode->htotal * tcon_div * tcon_mul;
> > > > > > > block_space -= mode->hdisplay + 40;
> > > > > > >
> > > > > > > regmap_write(tcon->regs, SUN4I_TCON0_CPU_TRI0_REG,
> > > > > > > @@ -408,7 +410,7 @@ static void sun4i_tcon0_mode_set_lvds(struct
> > > > > > > sun4i_tcon *tcon,
> > > > > > >
> > > > > > > tcon->dclk_min_div = 7;
> > > > > > > tcon->dclk_max_div = 7;
> > > > > > > - sun4i_tcon0_mode_set_common(tcon, mode);
> > > > > > > + sun4i_tcon0_mode_set_common(tcon, mode, 1);
> > > > > > >
> > > > > > > /* Set dithering if needed */
> > > > > > > sun4i_tcon0_mode_set_dithering(tcon, sun4i_tcon_get_connector(encoder));
> > > > > > > @@ -487,7 +489,7 @@ static void sun4i_tcon0_mode_set_rgb(struct
> > > > > > > sun4i_tcon *tcon,
> > > > > > >
> > > > > > > tcon->dclk_min_div = 6;
> > > > > > > tcon->dclk_max_div = 127;
> > > > > > > - sun4i_tcon0_mode_set_common(tcon, mode);
> > > > > > > + sun4i_tcon0_mode_set_common(tcon, mode, 1);
> > > > > > >
> > > > > > > /* Set dithering if needed */
> > > > > > > sun4i_tcon0_mode_set_dithering(tcon, connector);
> > > > > > > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> > > > > > > b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> > > > > > > index 5c3ad5be0690..a07090579f84 100644
> > > > > > > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> > > > > > > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> > > > > > > @@ -13,8 +13,6 @@
> > > > > > > #include <drm/drm_encoder.h>
> > > > > > > #include <drm/drm_mipi_dsi.h>
> > > > > > >
> > > > > > > -#define SUN6I_DSI_TCON_DIV 4
> > > > > > > -
> > > > > > > struct sun6i_dsi {
> > > > > > > struct drm_connector connector;
> > > > > > > struct drm_encoder encoder;
> > > > > >
> > > > > > I had more something like this in mind:
> > > > > > http://code.bulix.org/nlp5a4-803511
> > > > >
> > > > > Worth to look at it. was it working on your panel? meanwhile I will check it.
> > > >
> > > > I haven't tested it.
> > > >
> > > > > We have updated with below change [1], seems working on but is
> > > > > actually checking the each divider as before start with 4... till 127.
> > > > >
> > > > > This new approach, is start looking the best divider from 4.. based on
> > > > > the idea vs rounded it will ended up best divider like [2]
> > > >
> > > > But why?
> > > >
> > > > I mean, it's not like it's the first time I'm asking this...
> > > >
> > > > If the issue is what Micheal described, then the divider has nothing
> > > > to do with it. We've had that discussion over and over again.
> > >
> > > This is what Michael is mentioned in above mail "tcon-pixel clock is
> > > the rate that you want to achive on display side and if you have 4
> > > lanes 32bit or lanes and different bit number that you need to have
> > > a clock that is able to put outside bits and speed equal to
> > > pixel-clock * bits / lanes. so If you want a pixel-clock of 40 mhz
> > > and you have 32bits and 4 lanes you need to have a clock of 40 * 32
> > > / 4 in no-burst mode. "
> >
> > Yeah, so we need to change the clock rate.
> >
> > > He is trying to manage the bpp/lanes into dclk_mul (in last mail)
> > > and it can multiply with pixel clock which is rate argument in
> > > sun4i_dclk_round_rate.
> > >
> > > The solution I have mentioned in dclk_min, max is bpp/lanes also
> > > multiple rate in dotclock sun4i_dclk_round_rate.
> > >
> > > In both cases the overall pll_rate depends on dividers, the one that I
> > > have on this patch is based on BSP and the Michael one is more generic
> > > way so-that it can not to touch other functionalities and looping
> > > dividers to find the best one.
> > >
> > > If dclk_min/max is bpp/lanes then dotclock directly using divider 6
> > > (assuming 24-bit and 4 lanes) and return the pll_rate and divider 6
> > > associated.
> > >
> > > if dclk_mul is bpp/lanes, on Michael new change, the dividers start
> > > with 4 and end with 127 but the constant ideal rate which rate *
> > > bpp/lanes but the loop from sun4i_dclk_round_rate computed the divider
> > > as 6 only, ie what I'm mentioned on the above mail.
> >
> > We've been over this a couple of times already.
> >
> > The clock is generated like this:
> >
> > PLL -> TCON Module Clock -> TCON DCLK
> >
> > You want the TCON DCLK to be at the pixel clock rate * bpp /
> > lanes. Fine, that makes sense.
> >
> > Except that the patch you've sent, instead of changing the rate
> > itself, changes the ratio between the module clock and DCLK.
> >
> > And this is where the issue lies. First, from a logical viewpoint, it
> > doesn't make sense. If you want to change the clock rate, then just do
> > it. Don't hack around the multipliers trying to fall back to something
> > that works for you.
> >
> > Then, the ratio itself needs to be set to 4. This is the part that
> > we've discussed way too many times already, but in the Allwinner BSP,
> > that ratio is hardcoded to 4, and we've had panels that need it at
> > that value.
> >
> > So, what you want to do is to have:
> >
> > TCON DCLK = pixel clock * bpp / lanes
> > TCON Module Clock = DCLK * 4
> > PLL = Module Clock * Module Clock Divider (which I believe is 1 in most cases)
>
> pll-mipi 1 1 1 178200000
> 0 0 50000
> tcon0 2 2 1 178200000
> 0 0 50000
> tcon-pixel-clock 1 1 1 29700000
> 0 0 50000
>
> This is an english problem from my side:
> tcon-pixel-clock is DCLK
> tcon0 must be tcon-pixel-clock * bpp / lanes because the logic need to
> put a bit every cycle.
>
> One solution can be:
> - set_rate_exclusive to tcon0 and calculate as display pixel clock *
> bpp / lanes
> - calculate the tcon-pixel-clock using all divider
>
> Problem is that the function that calculate tcon-pixel-clock does not
> have any constrain on the ideal value. What you are
> suggesting is not correct in my opinion or I'm not following your
> suggesstion. What I know is that if we have a pixel-clock
> of dvi display of 50Mhz and we have 4 lanes 32bit we need a clock in
> the logic of 400Mhz (this is the ideal throughtput).
> So tcon-pixel-clock is 50mhz and tcon0 is 400Mhz.
>
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index 94f24c5e2dc5..ffb7906054e5 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -338,8 +338,9 @@ static void sun4i_tcon0_mode_set_cpu(struct
sun4i_tcon *tcon,
u32 block_space, start_delay;
u32 tcon_div;
- tcon->dclk_min_div = bpp/lanes;
- tcon->dclk_max_div = bpp/lanes;
+ tcon->dclk_min_div = 4;
+ tcon->dclk_max_div = 127;
+ clk_set_rate_exclusive(tcon->sclk0, mode->crtc_clock * 1000 * bpp / lanes);
sun4i_tcon0_mode_set_common(tcon, mode);
Something like this on top of jagan proposal
Michael
> Michael
>
>
> >
> > So you want to increase the PLL. Fortunately for use, this is exactly
> > what a call to clk_set_rate will end up doing.
> >
> > Maxime
> >
> > --
> > Maxime Ripard, Bootlin
> > Embedded Linux and Kernel engineering
> > https://bootlin.com
>
>
>
> --
> | Michael Nazzareno Trimarchi Amarula Solutions BV |
> | COO - Founder Cruquiuskade 47 |
> | +31(0)851119172 Amsterdam 1018 AM NL |
> | [`as] http://www.amarulasolutions.com |
--
| Michael Nazzareno Trimarchi Amarula Solutions BV |
| COO - Founder Cruquiuskade 47 |
| +31(0)851119172 Amsterdam 1018 AM NL |
| [`as] http://www.amarulasolutions.com |
^ permalink raw reply related
* [PATCH v4 4/4] ARM: dts: aspeed: Add Mihawk BMC platform
From: Ben Pai @ 2019-08-02 8:37 UTC (permalink / raw)
To: robh+dt, mark.rutland, joel, andrew, devicetree, linux-arm-kernel,
linux-aspeed, linux-kernel
Cc: wangat, Ben Pai
The Mihawk BMC is an ASPEED ast2500 based BMC that is part of an
OpenPower Power9 server.
Signed-off-by: Ben Pai <Ben_Pai@wistron.com>
---
arch/arm/boot/dts/Makefile | 1 +
arch/arm/boot/dts/aspeed-bmc-opp-mihawk.dts | 918 ++++++++++++++++++++
2 files changed, 919 insertions(+)
create mode 100755 arch/arm/boot/dts/aspeed-bmc-opp-mihawk.dts
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index eb6de52c1936..cdfe0f43ffd3 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -1275,6 +1275,7 @@ dtb-$(CONFIG_ARCH_ASPEED) += \
aspeed-bmc-lenovo-hr630.dtb \
aspeed-bmc-microsoft-olympus.dtb \
aspeed-bmc-opp-lanyang.dtb \
+ aspeed-bmc-opp-mihawk.dtb \
aspeed-bmc-opp-palmetto.dtb \
aspeed-bmc-opp-romulus.dtb \
aspeed-bmc-opp-swift.dtb \
diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-mihawk.dts b/arch/arm/boot/dts/aspeed-bmc-opp-mihawk.dts
new file mode 100755
index 000000000000..bbf4a4671421
--- /dev/null
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-mihawk.dts
@@ -0,0 +1,918 @@
+/dts-v1/;
+
+#include "aspeed-g5.dtsi"
+#include <dt-bindings/gpio/aspeed-gpio.h>
+#include <dt-bindings/leds/leds-pca955x.h>
+
+/ {
+ model = "Mihawk BMC";
+ compatible = "ibm,mihawk-bmc", "aspeed,ast2500";
+
+
+ chosen {
+ stdout-path = &uart5;
+ bootargs = "console=ttyS4,115200 earlyprintk";
+ };
+
+ memory@80000000 {
+ reg = <0x80000000 0x20000000>; /* address and size of RAM(512MB) */
+ };
+
+ reserved-memory {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+
+ flash_memory: region@98000000 {
+ no-map;
+ reg = <0x98000000 0x04000000>; /* 64M */
+ };
+
+ gfx_memory: framebuffer {
+ size = <0x01000000>;
+ alignment = <0x01000000>;
+ compatible = "shared-dma-pool";
+ reusable;
+ };
+
+ video_engine_memory: jpegbuffer {
+ size = <0x02000000>; /* 32MM */
+ alignment = <0x01000000>;
+ compatible = "shared-dma-pool";
+ reusable;
+ };
+ };
+
+ gpio-keys {
+ compatible = "gpio-keys";
+
+ air-water {
+ label = "air-water";
+ gpios = <&gpio ASPEED_GPIO(F, 6) GPIO_ACTIVE_LOW>;
+ linux,code = <ASPEED_GPIO(F, 6)>;
+ };
+
+ checkstop {
+ label = "checkstop";
+ gpios = <&gpio ASPEED_GPIO(J, 2) GPIO_ACTIVE_LOW>;
+ linux,code = <ASPEED_GPIO(J, 2)>;
+ };
+
+ ps0-presence {
+ label = "ps0-presence";
+ gpios = <&gpio ASPEED_GPIO(Z, 2) GPIO_ACTIVE_LOW>;
+ linux,code = <ASPEED_GPIO(Z, 2)>;
+ };
+
+ ps1-presence {
+ label = "ps1-presence";
+ gpios = <&gpio ASPEED_GPIO(Z, 0) GPIO_ACTIVE_LOW>;
+ linux,code = <ASPEED_GPIO(Z, 0)>;
+ };
+ id-button {
+ label = "id-button";
+ gpios = <&gpio ASPEED_GPIO(F, 1) GPIO_ACTIVE_LOW>;
+ linux,code = <ASPEED_GPIO(F, 1)>;
+ };
+ };
+
+ gpio-keys-polled {
+ compatible = "gpio-keys-polled";
+ poll-interval = <1000>;
+
+ fan0-presence {
+ label = "fan0-presence";
+ gpios = <&pca9552 9 GPIO_ACTIVE_LOW>;
+ linux,code = <9>;
+ };
+
+ fan1-presence {
+ label = "fan1-presence";
+ gpios = <&pca9552 10 GPIO_ACTIVE_LOW>;
+ linux,code = <10>;
+ };
+
+ fan2-presence {
+ label = "fan2-presence";
+ gpios = <&pca9552 11 GPIO_ACTIVE_LOW>;
+ linux,code = <11>;
+ };
+
+ fan3-presence {
+ label = "fan3-presence";
+ gpios = <&pca9552 12 GPIO_ACTIVE_LOW>;
+ linux,code = <12>;
+ };
+
+ fan4-presence {
+ label = "fan4-presence";
+ gpios = <&pca9552 13 GPIO_ACTIVE_LOW>;
+ linux,code = <13>;
+ };
+
+ fan5-presence {
+ label = "fan5-presence";
+ gpios = <&pca9552 14 GPIO_ACTIVE_LOW>;
+ linux,code = <14>;
+ };
+ };
+
+ leds {
+ compatible = "gpio-leds";
+
+ fault {
+ retain-state-shutdown;
+ default-state = "keep";
+ gpios = <&gpio ASPEED_GPIO(AA, 0) GPIO_ACTIVE_LOW>;
+ };
+
+ power {
+ retain-state-shutdown;
+ default-state = "keep";
+ gpios = <&gpio ASPEED_GPIO(AA, 1) GPIO_ACTIVE_LOW>;
+ };
+
+ rear-id {
+ retain-state-shutdown;
+ default-state = "keep";
+ gpios = <&gpio ASPEED_GPIO(AA, 2) GPIO_ACTIVE_LOW>;
+ };
+
+ rear-g {
+ retain-state-shutdown;
+ default-state = "keep";
+ gpios = <&gpio ASPEED_GPIO(AA, 4) GPIO_ACTIVE_LOW>;
+ };
+
+ rear-ok {
+ retain-state-shutdown;
+ default-state = "keep";
+ gpios = <&gpio ASPEED_GPIO(Y, 0) GPIO_ACTIVE_LOW>;
+ };
+
+ fan0 {
+ retain-state-shutdown;
+ default-state = "keep";
+ gpios = <&pca9552 0 GPIO_ACTIVE_LOW>;
+ };
+
+ fan1 {
+ retain-state-shutdown;
+ default-state = "keep";
+ gpios = <&pca9552 1 GPIO_ACTIVE_LOW>;
+ };
+
+ fan2 {
+ retain-state-shutdown;
+ default-state = "keep";
+ gpios = <&pca9552 2 GPIO_ACTIVE_LOW>;
+ };
+
+ fan3 {
+ retain-state-shutdown;
+ default-state = "keep";
+ gpios = <&pca9552 3 GPIO_ACTIVE_LOW>;
+ };
+
+ fan4 {
+ retain-state-shutdown;
+ default-state = "keep";
+ gpios = <&pca9552 4 GPIO_ACTIVE_LOW>;
+ };
+
+ fan5 {
+ retain-state-shutdown;
+ default-state = "keep";
+ gpios = <&pca9552 5 GPIO_ACTIVE_LOW>;
+ };
+ };
+
+ fsi: gpio-fsi {
+ compatible = "fsi-master-gpio", "fsi-master";
+ #address-cells = <2>;
+ #size-cells = <0>;
+ no-gpio-delays;
+
+ clock-gpios = <&gpio ASPEED_GPIO(E, 6) GPIO_ACTIVE_HIGH>;
+ data-gpios = <&gpio ASPEED_GPIO(E, 7) GPIO_ACTIVE_HIGH>;
+ mux-gpios = <&gpio ASPEED_GPIO(E, 5) GPIO_ACTIVE_HIGH>;
+ enable-gpios = <&gpio ASPEED_GPIO(D, 0) GPIO_ACTIVE_HIGH>;
+ trans-gpios = <&gpio ASPEED_GPIO(R, 2) GPIO_ACTIVE_HIGH>;
+ };
+ iio-hwmon-12v {
+ compatible = "iio-hwmon";
+ io-channels = <&adc 0>;
+ };
+
+ iio-hwmon-5v {
+ compatible = "iio-hwmon";
+ io-channels = <&adc 1>;
+ };
+
+ iio-hwmon-3v {
+ compatible = "iio-hwmon";
+ io-channels = <&adc 2>;
+ };
+
+ iio-hwmon-vdd0 {
+ compatible = "iio-hwmon";
+ io-channels = <&adc 3>;
+ };
+
+ iio-hwmon-vdd1 {
+ compatible = "iio-hwmon";
+ io-channels = <&adc 4>;
+ };
+
+ iio-hwmon-vcs0 {
+ compatible = "iio-hwmon";
+ io-channels = <&adc 5>;
+ };
+
+ iio-hwmon-vcs1 {
+ compatible = "iio-hwmon";
+ io-channels = <&adc 6>;
+ };
+
+ iio-hwmon-vdn0 {
+ compatible = "iio-hwmon";
+ io-channels = <&adc 7>;
+ };
+
+ iio-hwmon-vdn1 {
+ compatible = "iio-hwmon";
+ io-channels = <&adc 8>;
+ };
+
+ iio-hwmon-vio0 {
+ compatible = "iio-hwmon";
+ io-channels = <&adc 9>;
+ };
+
+ iio-hwmon-vio1 {
+ compatible = "iio-hwmon";
+ io-channels = <&adc 10>;
+ };
+
+ iio-hwmon-vddra {
+ compatible = "iio-hwmon";
+ io-channels = <&adc 11>;
+ };
+
+ iio-hwmon-battery {
+ compatible = "iio-hwmon";
+ io-channels = <&adc 12>;
+ };
+
+ iio-hwmon-vddrb {
+ compatible = "iio-hwmon";
+ io-channels = <&adc 13>;
+ };
+
+ iio-hwmon-vddrc {
+ compatible = "iio-hwmon";
+ io-channels = <&adc 14>;
+ };
+
+ iio-hwmon-vddrd {
+ compatible = "iio-hwmon";
+ io-channels = <&adc 15>;
+ };
+};
+
+&pwm_tacho {
+ status = "okay";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_pwm0_default &pinctrl_pwm1_default
+ &pinctrl_pwm2_default &pinctrl_pwm3_default
+ &pinctrl_pwm4_default &pinctrl_pwm5_default>;
+
+ fan@0 {
+ reg = <0x00>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x00>;
+ };
+
+ fan@1 {
+ reg = <0x01>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x01>;
+ };
+
+ fan@2 {
+ reg = <0x02>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x02>;
+ };
+
+ fan@3 {
+ reg = <0x03>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x03>;
+ };
+
+ fan@4 {
+ reg = <0x04>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x04>;
+ };
+
+ fan@5 {
+ reg = <0x05>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x05>;
+ };
+
+ fan@6 {
+ reg = <0x00>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x06>;
+ };
+
+ fan@7 {
+ reg = <0x01>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x07>;
+ };
+
+ fan@8 {
+ reg = <0x02>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x08>;
+ };
+
+ fan@9 {
+ reg = <0x03>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x09>;
+ };
+
+ fan@10 {
+ reg = <0x04>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x0a>;
+ };
+
+ fan@11 {
+ reg = <0x05>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x0b>;
+ };
+};
+
+&fmc {
+ status = "okay";
+ flash@0 {
+ status = "okay";
+ label = "bmc";
+ m25p,fast-read;
+ spi-max-frequency = <50000000>;
+ partitions {
+ #address-cells = < 1 >;
+ #size-cells = < 1 >;
+ compatible = "fixed-partitions";
+ u-boot@0 {
+ reg = < 0 0x60000 >;
+ label = "u-boot";
+ };
+ u-boot-env@60000 {
+ reg = < 0x60000 0x20000 >;
+ label = "u-boot-env";
+ };
+ obmc-ubi@80000 {
+ reg = < 0x80000 0x1F80000 >;
+ label = "obmc-ubi";
+ };
+ };
+ };
+ flash@1 {
+ status = "okay";
+ label = "alt-bmc";
+ m25p,fast-read;
+ spi-max-frequency = <50000000>;
+ partitions {
+ #address-cells = < 1 >;
+ #size-cells = < 1 >;
+ compatible = "fixed-partitions";
+ u-boot@0 {
+ reg = < 0 0x60000 >;
+ label = "alt-u-boot";
+ };
+ u-boot-env@60000 {
+ reg = < 0x60000 0x20000 >;
+ label = "alt-u-boot-env";
+ };
+ obmc-ubi@80000 {
+ reg = < 0x80000 0x1F80000 >;
+ label = "alt-obmc-ubi";
+ };
+ };
+ };
+};
+
+&spi1 {
+ status = "okay";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_spi1_default>;
+
+ flash@0 {
+ status = "okay";
+ label = "pnor";
+ m25p,fast-read;
+ spi-max-frequency = <100000000>;
+ };
+};
+
+&lpc_ctrl {
+ status = "okay";
+ memory-region = <&flash_memory>;
+ flash = <&spi1>;
+};
+
+&uart1 {
+ /* Rear RS-232 connector */
+ status = "okay";
+
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_txd1_default
+ &pinctrl_rxd1_default
+ &pinctrl_nrts1_default
+ &pinctrl_ndtr1_default
+ &pinctrl_ndsr1_default
+ &pinctrl_ncts1_default
+ &pinctrl_ndcd1_default
+ &pinctrl_nri1_default>;
+};
+
+&uart2 {
+ /* APSS */
+ status = "okay";
+
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_txd2_default &pinctrl_rxd2_default>;
+};
+
+&uart5 {
+ status = "okay";
+};
+
+&mac0 {
+ status = "okay";
+
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_rmii1_default>;
+ use-ncsi;
+};
+
+&mac1 {
+ status = "okay";
+
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_rgmii2_default &pinctrl_mdio2_default>;
+};
+
+&i2c0 {
+ status = "disabled";
+};
+
+&i2c1 {
+ status = "disabled";
+};
+
+&i2c2 {
+ status = "okay";
+
+ /* SAMTEC P0 */
+ /* SAMTEC P1 */
+
+};
+
+&i2c3 {
+ status = "okay";
+
+ /* APSS */
+ /* CPLD */
+
+ /* PCA9516 (repeater) ->
+ * CLK Buffer 9FGS9092
+ * CLK Buffer 9DBL0651BKILFT
+ * CLK Buffer 9DBL0651BKILFT
+ * Power Supply 0
+ * Power Supply 1
+ * PCA 9552 LED
+ */
+
+ power-supply@58 {
+ compatible = "ibm,cffps1";
+ reg = <0x58>;
+ };
+
+ power-supply@5b {
+ compatible = "ibm,cffps1";
+ reg = <0x5b>;
+ };
+
+ pca9552: pca9552@60 {
+ compatible = "nxp,pca9552";
+ reg = <0x60>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ gpio-controller;
+ #gpio-cells = <2>;
+
+ gpio@0 {
+ reg = <0>;
+ type = <PCA955X_TYPE_GPIO>;
+ };
+ gpio@1 {
+ reg = <1>;
+ type = <PCA955X_TYPE_GPIO>;
+ };
+ gpio@2 {
+ reg = <2>;
+ type = <PCA955X_TYPE_GPIO>;
+ };
+ gpio@3 {
+ reg = <3>;
+ type = <PCA955X_TYPE_GPIO>;
+ };
+ gpio@4 {
+ reg = <4>;
+ type = <PCA955X_TYPE_GPIO>;
+ };
+ gpio@5 {
+ reg = <5>;
+ type = <PCA955X_TYPE_GPIO>;
+ };
+ gpio@6 {
+ reg = <6>;
+ type = <PCA955X_TYPE_GPIO>;
+ };
+ gpio@7 {
+ reg = <7>;
+ type = <PCA955X_TYPE_GPIO>;
+ };
+ gpio@8 {
+ reg = <8>;
+ type = <PCA955X_TYPE_GPIO>;
+ };
+ gpio@9 {
+ reg = <9>;
+ type = <PCA955X_TYPE_GPIO>;
+ };
+ gpio@10 {
+ reg = <10>;
+ type = <PCA955X_TYPE_GPIO>;
+ };
+ gpio@11 {
+ reg = <11>;
+ type = <PCA955X_TYPE_GPIO>;
+ };
+ gpio@12 {
+ reg = <12>;
+ type = <PCA955X_TYPE_GPIO>;
+ };
+ gpio@13 {
+ reg = <13>;
+ type = <PCA955X_TYPE_GPIO>;
+ };
+ gpio@14 {
+ reg = <14>;
+ type = <PCA955X_TYPE_GPIO>;
+ };
+ gpio@15 {
+ reg = <15>;
+ type = <PCA955X_TYPE_GPIO>;
+ };
+
+ };
+
+};
+
+&i2c4 {
+ status = "okay";
+
+ /* CP0 VDD & VCS : IR35221 */
+ /* CP0 VDN : IR35221 */
+ /* CP0 VIO : IR38064 */
+ /* CP0 VDDR : PXM1330 */
+
+ ir35221@70 {
+ compatible = "infineon,ir35221";
+ reg = <0x70>;
+ };
+
+ ir35221@72 {
+ compatible = "infineon,ir35221";
+ reg = <0x72>;
+ };
+
+};
+
+&i2c5 {
+ status = "okay";
+
+ /* CP0 VDD & VCS : IR35221 */
+ /* CP0 VDN : IR35221 */
+ /* CP0 VIO : IR38064 */
+ /* CP0 VDDR : PXM1330 */
+
+ ir35221@70 {
+ compatible = "infineon,ir35221";
+ reg = <0x70>;
+ };
+
+ ir35221@72 {
+ compatible = "infineon,ir35221";
+ reg = <0x72>;
+ };
+
+};
+
+&i2c6 {
+ status = "okay";
+
+ /* pca9548 -> NVMe1 to 8 */
+
+ pca9548@70 {
+ compatible = "nxp,pca9548";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x70>;
+ };
+
+};
+
+&i2c7 {
+ status = "okay";
+
+ /* pca9548 -> NVMe9 to 16 */
+
+ pca9548@70 {
+ compatible = "nxp,pca9548";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x70>;
+ };
+
+};
+
+&i2c8 {
+ status = "okay";
+
+ eeprom@50 {
+ compatible = "atmel,24c64";
+ reg = <0x50>;
+ };
+};
+
+&i2c9 {
+ status = "okay";
+
+ /* pca9545 Riser ->
+ * PCIe x8 Slot3
+ * PCIe x16 slot4
+ * PCIe x8 slot5
+ * I2C BMC RISER PCA9554
+ * BMC SCL/SDA PCA9554
+ * PCA9554
+ */
+
+ /* pca9545 ->
+ * PCIe x16 Slot1
+ * PCIe x8 slot2
+ * PEX8748
+ */
+
+ pca9545riser@70 {
+ compatible = "nxp,pca9545";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x70>;
+
+ i2c-mux-idle-disconnect;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ };
+
+ pca9545@71 {
+ compatible = "nxp,pca9545";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x71>;
+
+ i2c-mux-idle-disconnect;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ };
+};
+
+&i2c10 {
+ status = "okay";
+
+ /* pca9545 Riser ->
+ * PCIe x8 Slot8
+ * PCIe x16 slot9
+ * PCIe x8 slot10
+ * I2C BMC RISER PCA9554
+ * BMC SCL/SDA PCA9554
+ * PCA9554
+ */
+
+ /* pca9545 ->
+ * PCIe x16 Slot1
+ * PCIe x8 slot2
+ * PEX8748
+ */
+
+ pca9545riser@70 {
+ compatible = "nxp,pca9545";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x70>;
+
+ i2c-mux-idle-disconnect;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ };
+
+ pca9545@71 {
+ compatible = "nxp,pca9545";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x71>;
+
+ i2c-mux-idle-disconnect;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ };
+};
+
+&i2c11 {
+ status = "okay";
+
+ /* TPM */
+ /* RTC RX8900CE */
+ /* FPGA for power sequence */
+ /* TMP275A */
+ /* TMP275A */
+ /* EMC1462 */
+
+ tpm@57 {
+ compatible = "infineon,slb9645tt";
+ reg = <0x57>;
+ };
+
+ rtc@32 {
+ compatible = "epson,rx8900";
+ reg = <0x32>;
+ };
+
+ tmp275@48 {
+ compatible = "ti,tmp275";
+ reg = <0x48>;
+ };
+
+ tmp275@49 {
+ compatible = "ti,tmp275";
+ reg = <0x49>;
+ };
+
+ /* chip emc1462 use emc1403 driver */
+ emc1403@4c {
+ compatible = "smsc,emc1403";
+ reg = <0x4c>;
+ };
+
+};
+
+&i2c12 {
+ status = "okay";
+
+ /* pca9545 ->
+ * SAS BP1
+ * SAS BP2
+ * NVMe BP
+ * M.2 riser
+ */
+
+ pca9545@70 {
+ compatible = "nxp,pca9545";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x70>;
+
+ interrupt-controller;
+ #interrupt-cells = <2>;
+
+ i2c@0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0>;
+
+ eeprom@50 {
+ compatible = "atmel,24c64";
+ reg = <0x50>;
+ };
+ };
+
+ i2c@1 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <1>;
+
+ eeprom@50 {
+ compatible = "atmel,24c64";
+ reg = <0x50>;
+ };
+ };
+
+ i2c@2 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <2>;
+
+ eeprom@50 {
+ compatible = "atmel,24c64";
+ reg = <0x50>;
+ };
+ };
+
+ i2c@3 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <3>;
+
+ tmp275@48 {
+ compatible = "ti,tmp275";
+ reg = <0x48>;
+ };
+ };
+
+ };
+
+};
+
+&i2c13 {
+ status = "okay";
+
+ /* pca9548 ->
+ * NVMe BP
+ * NVMe HDD17 to 24
+ */
+
+ pca9548@70 {
+ compatible = "nxp,pca9548";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x70>;
+ };
+};
+
+&vuart {
+ status = "okay";
+};
+
+&gfx {
+ status = "okay";
+ memory-region = <&gfx_memory>;
+};
+
+&adc {
+ status = "okay";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_adc0_default
+ &pinctrl_adc1_default
+ &pinctrl_adc2_default
+ &pinctrl_adc3_default
+ &pinctrl_adc4_default
+ &pinctrl_adc5_default
+ &pinctrl_adc6_default
+ &pinctrl_adc7_default
+ &pinctrl_adc8_default
+ &pinctrl_adc9_default
+ &pinctrl_adc10_default
+ &pinctrl_adc11_default
+ &pinctrl_adc12_default
+ &pinctrl_adc13_default
+ &pinctrl_adc14_default
+ &pinctrl_adc15_default>;
+};
+
+&wdt1 {
+ aspeed,reset-type = "none";
+ aspeed,external-signal;
+ aspeed,ext-push-pull;
+ aspeed,ext-active-high;
+
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_wdtrst1_default>;
+};
+
+&wdt2 {
+ aspeed,alt-boot;
+};
+
+&ibt {
+ status = "okay";
+};
+
+&vhub {
+ status = "okay";
+};
+
+&video {
+ status = "okay";
+ memory-region = <&video_engine_memory>;
+};
+
+#include "ibm-power9-dual.dtsi"
+
--
2.17.1
^ permalink raw reply related
* Re: [PATCH/RFC 12/12] arm64: dts: renesas: Add EK874 board with idk-2121wr display support
From: Laurent Pinchart @ 2019-08-02 8:34 UTC (permalink / raw)
To: Fabrizio Castro
Cc: Kieran Bingham, Jacopo Mondi, Rob Herring, Mark Rutland,
Simon Horman, Magnus Damm, linux-renesas-soc, devicetree,
Geert Uytterhoeven, Chris Paterson, Biju Das, ebiharaml
In-Reply-To: <1564731249-22671-13-git-send-email-fabrizio.castro@bp.renesas.com>
Hi Fabrizio,
Thank you for the patch.
On Fri, Aug 02, 2019 at 08:34:09AM +0100, Fabrizio Castro wrote:
> The EK874 is advertised as compatible with panel IDK-2121WR from
> Advantech, however the panel isn't sold alongside the board.
> A new dts, adding everything that's required to get the panel to
> to work with the EK874, is the most convenient way to support the
> EK874 when it's connected to the IDK-2121WR.
>
> Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> ---
> arch/arm64/boot/dts/renesas/Makefile | 3 +-
> .../boot/dts/renesas/r8a774c0-ek874-idk-2121wr.dts | 112 +++++++++++++++++++++
> 2 files changed, 114 insertions(+), 1 deletion(-)
> create mode 100644 arch/arm64/boot/dts/renesas/r8a774c0-ek874-idk-2121wr.dts
>
> diff --git a/arch/arm64/boot/dts/renesas/Makefile b/arch/arm64/boot/dts/renesas/Makefile
> index 42b74c2..ce48478 100644
> --- a/arch/arm64/boot/dts/renesas/Makefile
> +++ b/arch/arm64/boot/dts/renesas/Makefile
> @@ -1,7 +1,8 @@
> # SPDX-License-Identifier: GPL-2.0
> dtb-$(CONFIG_ARCH_R8A774A1) += r8a774a1-hihope-rzg2m.dtb
> dtb-$(CONFIG_ARCH_R8A774A1) += r8a774a1-hihope-rzg2m-ex.dtb
> -dtb-$(CONFIG_ARCH_R8A774C0) += r8a774c0-cat874.dtb r8a774c0-ek874.dtb
> +dtb-$(CONFIG_ARCH_R8A774C0) += r8a774c0-cat874.dtb r8a774c0-ek874.dtb \
> + r8a774c0-ek874-idk-2121wr.dtb
> dtb-$(CONFIG_ARCH_R8A7795) += r8a7795-salvator-x.dtb r8a7795-h3ulcb.dtb
> dtb-$(CONFIG_ARCH_R8A7795) += r8a7795-h3ulcb-kf.dtb
> dtb-$(CONFIG_ARCH_R8A7795) += r8a7795-salvator-xs.dtb
> diff --git a/arch/arm64/boot/dts/renesas/r8a774c0-ek874-idk-2121wr.dts b/arch/arm64/boot/dts/renesas/r8a774c0-ek874-idk-2121wr.dts
> new file mode 100644
> index 0000000..d989998
> --- /dev/null
> +++ b/arch/arm64/boot/dts/renesas/r8a774c0-ek874-idk-2121wr.dts
> @@ -0,0 +1,112 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Device Tree Source for the Silicon Linux RZ/G2E evaluation kit (EK874),
> + * connected to an Advantech IDK-2121WR 21.5" LVDS panel
> + *
> + * Copyright (C) 2019 Renesas Electronics Corp.
> + */
> +
> +#include "r8a774c0-ek874.dts"
> +
> +/ {
> + backlight: backlight {
> + compatible = "pwm-backlight";
> + pwms = <&pwm5 0 50000>;
> +
> + brightness-levels = <0 4 8 16 32 64 128 255>;
> + default-brightness-level = <6>;
> +
> + power-supply = <®_12p0v>;
> + enable-gpios = <&gpio6 12 GPIO_ACTIVE_HIGH>;
> + };
> +
> + panel-lvds {
> + compatible = "advantech,idk-2121wr", "panel-lvds";
> +
> + width-mm = <476>;
> + height-mm = <268>;
> +
> + data-mapping = "vesa-24";
> +
> + panel-timing {
> + clock-frequency = <148500000>;
> + hactive = <1920>;
> + vactive = <1080>;
> + hsync-len = <44>;
> + hfront-porch = <88>;
> + hback-porch = <148>;
> + vfront-porch = <4>;
> + vback-porch = <36>;
> + vsync-len = <5>;
> + };
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> + lvds0_panel_in: endpoint {
> + remote-endpoint = <&lvds0_out>;
> + };
> + };
> +
> + port@1 {
> + reg = <1>;
> + lvds1_panel_in: endpoint {
> + remote-endpoint = <&lvds1_out>;
> + };
> + };
> + };
> + };
> +};
> +
> +&gpio0 {
> + lvds-connector-en-gpio{
> + gpio-hog;
> + gpios = <17 GPIO_ACTIVE_HIGH>;
> + output-low;
> + line-name = "lvds-connector-en-gpio";
> + };
Any chance to specify this as the panel's enable signal in the panel DT
node ?
> +};
> +
> +&lvds0 {
> + renesas,swap-data;
Let's discuss this property in reply to the DT bindings patch.
> +
> + ports {
> + port@1 {
> + lvds0_out: endpoint {
> + remote-endpoint = <&lvds0_panel_in>;
> + };
> + };
> + };
> +};
> +
> +&lvds1 {
> + status = "okay";
> +
> + clocks = <&cpg CPG_MOD 727>, <&x13_clk>, <&extal_clk>;
> + clock-names = "fck", "dclkin.0", "extal";
> +
> + ports {
> + port@1 {
> + lvds1_out: endpoint {
> + remote-endpoint = <&lvds1_panel_in>;
> + };
> + };
> + };
> +};
> +
> +&pfc {
> + pwm5_pins: pwm5 {
> + groups = "pwm5_a";
> + function = "pwm5";
> + };
> +};
> +
> +&pwm5 {
> + pinctrl-0 = <&pwm5_pins>;
> + pinctrl-names = "default";
> +
> + status = "okay";
> +};
I haven't reviewed pinouts in detail, but the patch otherwise looks sane
to me. Another candidate for DT overlays though ;-)
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [alsa-devel] [PATCH v2 3/3] ASoC: TDA7802: Add turn-on diagnostic routine
From: Thomas Preston @ 2019-08-02 8:32 UTC (permalink / raw)
To: Mark Brown
Cc: Mark Rutland, devicetree, alsa-devel, Charles Keepax,
Kuninori Morimoto, Kirill Marinushkin, Liam Girdwood,
Marco Felsch, Annaliese McDermond, Takashi Iwai, Paul Cercueil,
Vinod Koul, Rob Herring, Srinivas Kandagatla, Jerome Brunet,
linux-kernel, Cheng-Yi Chiang
In-Reply-To: <20190801234241.GG5488@sirena.org.uk>
On 02/08/2019 00:42, Mark Brown wrote:
> On Tue, Jul 30, 2019 at 05:28:11PM +0100, Thomas Preston wrote:
>> On 30/07/2019 16:50, Mark Brown wrote:
>
>>> Like I say it's not just debugfs though, there's the standard driver
>>> interface too.
>
>> Ah right, I understand. So if we run the turn-on diagnostics routine, there's
>> nothing stopping anyone from interacting with the device in other ways.
>
>> I guess there's no way to share that mutex with ALSA? In that case, it doesn't
>> matter if this mutex is there or not - this feature is incompatible. How
>> compatible do debugfs interfaces have to be? I was under the impression anything
>> goes. I would argue that the debugfs is better off for having the mutex so
>> that no one re-reads "diagnostic" within the 5s poll timeout.
>
> It's not really something that's supported; like Charles says the DAPM
> mutex is exposed but if the regular controls would still be able to do
> stuff. It is kind of a "you broke it, you fix it" thing but on the
> other hand it's better to make things safer if we can since it might not
> be obvious later on why things are broken.
>
>> Alternatively, this diagnostic feature could be handled with an external-handler
>> kcontrol SOC_SINGLE_EXT? I'm not sure if this is an atomic interface either.
>>
>> What would be acceptable?
>
> Yes, that's definitely doable - we've got some other drivers with
> similar things like calibration triggers exposed that way.
>
One problem with using a kcontrol as a trigger for the turn-on diagnostic
is that the diagnostic routine has a "return value".
It goes like this:
- Bring device to low-quiescent state
- Initiate diagnostics
- Poll for diagnostics-complete bit
- Read the four channel status registers
The final read clears the status registers, so this isn't something I
can just do with regmap.
One idea I had was to initiate the turn-on diagnostics using a kcontrol,
whose handler saves the four channel status registers and an epoch in
tda7802_priv. Then this can be read from debugfs. But it seems strange
to have to turn on this control over here, then go over there and read
this value.
Hm, maybe a better idea is to have the turn on diagnostic only run on
device probe (as its name suggests!), and print something to dmesg:
modprobe tda7802 turn_on_diagnostic=1
tda7802-codec i2c-TDA7802:00: Turn on diagnostic 04 04 04 04
Kirill Marinushkin mentioned this in the first review [0], it just didn't
really sink in until now!
[0] https://lkml.org/lkml/2019/6/14/1344
^ permalink raw reply
* Re: [PATCH/RFC 11/12] arm64: dts: renesas: cat874: Add definition for 12V regulator
From: Laurent Pinchart @ 2019-08-02 8:29 UTC (permalink / raw)
To: Fabrizio Castro
Cc: Geert Uytterhoeven, Kieran Bingham, Jacopo Mondi, Rob Herring,
Mark Rutland, Simon Horman, Magnus Damm, linux-renesas-soc,
devicetree, Chris Paterson, Biju Das, ebiharaml
In-Reply-To: <1564731249-22671-12-git-send-email-fabrizio.castro@bp.renesas.com>
Hi Fabrizio,
Thank you for the patch.
On Fri, Aug 02, 2019 at 08:34:08AM +0100, Fabrizio Castro wrote:
> Power rail "D12.0V" comes straight from the power barrel connector,
> and it's used in both main board and sub board.
>
> Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
I don't plan to take this in my tree without patch 12/12, so if you
think the rest of the series won't be ready in time for v5.4, feel free
to get this patch merged through Simon or Geert already.
> ---
> arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts b/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
> index 46a77ee..651383c 100644
> --- a/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
> +++ b/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
> @@ -65,6 +65,15 @@
> reg = <0x0 0x48000000 0x0 0x78000000>;
> };
>
> + reg_12p0v: regulator-12p0v {
> + compatible = "regulator-fixed";
> + regulator-name = "D12.0V";
> + regulator-min-microvolt = <12000000>;
> + regulator-max-microvolt = <12000000>;
> + regulator-boot-on;
> + regulator-always-on;
> + };
> +
> sound: sound {
> compatible = "simple-audio-card";
>
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [RFC PATCH V2 4/4] platform: mtk-isp: Add Mediatek FD driver
From: Tomasz Figa @ 2019-08-02 8:28 UTC (permalink / raw)
To: Jerry-ch Chen
Cc: devicetree, Sean.Cheng, laurent.pinchart+renesas, Rynn.Wu,
christie.yu, srv_heupstream, po-yang.huang, suleiman, shik,
jungo.lin, sj.huang, yuzhao, hans.verkuil, zwisler, frederic.chen,
matthias.bgg, linux-mediatek, mchehab, linux-arm-kernel,
linux-media
In-Reply-To: <1562661672-22439-5-git-send-email-Jerry-Ch.chen@mediatek.com>
Hi Jerry,
On Tue, Jul 09, 2019 at 04:41:12PM +0800, Jerry-ch Chen wrote:
> From: Jerry-ch Chen <jerry-ch.chen@mediatek.com>
>
> This patch adds the driver of Face Detection (FD) unit in
> Mediatek camera system, providing face detection function.
>
> The mtk-isp directory will contain drivers for multiple IP
> blocks found in Mediatek ISP system. It will include ISP Pass 1
> driver (CAM), sensor interface driver, DIP driver and face
> detection driver.
>
> Signed-off-by: Jerry-ch Chen <jerry-ch.chen@mediatek.com>
> ---
> drivers/media/platform/Makefile | 2 +
> drivers/media/platform/mtk-isp/fd/Makefile | 5 +
> drivers/media/platform/mtk-isp/fd/mtk_fd.h | 157 +++
> drivers/media/platform/mtk-isp/fd/mtk_fd_40.c | 1259 +++++++++++++++++++++++++
> include/uapi/linux/v4l2-controls.h | 4 +
> 5 files changed, 1427 insertions(+)
> create mode 100644 drivers/media/platform/mtk-isp/fd/Makefile
> create mode 100644 drivers/media/platform/mtk-isp/fd/mtk_fd.h
> create mode 100644 drivers/media/platform/mtk-isp/fd/mtk_fd_40.c
>
Thanks for the patch! I finally got a chance to fully review the code. Sorry
for the delay. Please check my comments inline.
> diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
> index e6deb25..8b817cc 100644
> --- a/drivers/media/platform/Makefile
> +++ b/drivers/media/platform/Makefile
> @@ -94,6 +94,8 @@ obj-$(CONFIG_VIDEO_MEDIATEK_MDP) += mtk-mdp/
>
> obj-$(CONFIG_VIDEO_MEDIATEK_JPEG) += mtk-jpeg/
>
> +obj-$(CONFIG_VIDEO_MEDIATEK_FD) += mtk-isp/fd/
> +
> obj-$(CONFIG_VIDEO_QCOM_CAMSS) += qcom/camss/
>
> obj-$(CONFIG_VIDEO_QCOM_VENUS) += qcom/venus/
> diff --git a/drivers/media/platform/mtk-isp/fd/Makefile b/drivers/media/platform/mtk-isp/fd/Makefile
> new file mode 100644
> index 0000000..9b1c501
> --- /dev/null
> +++ b/drivers/media/platform/mtk-isp/fd/Makefile
> @@ -0,0 +1,5 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +mtk-fd-objs += mtk_fd_40.o
> +
> +obj-$(CONFIG_VIDEO_MEDIATEK_FD) += mtk-fd.o
> \ No newline at end of file
> diff --git a/drivers/media/platform/mtk-isp/fd/mtk_fd.h b/drivers/media/platform/mtk-isp/fd/mtk_fd.h
> new file mode 100644
> index 0000000..289999b
> --- /dev/null
> +++ b/drivers/media/platform/mtk-isp/fd/mtk_fd.h
> @@ -0,0 +1,157 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +//
> +// Copyright (c) 2018 MediaTek Inc.
> +
> +#ifndef __MTK_FD_HW_H__
> +#define __MTK_FD_HW_H__
> +
> +#include <linux/io.h>
> +#include <linux/types.h>
> +#include <linux/platform_device.h>
> +#include <media/v4l2-ctrls.h>
> +#include <media/v4l2-device.h>
> +#include <media/videobuf2-v4l2.h>
> +
> +#define MTK_FD_OUTPUT_MIN_WIDTH 26U
> +#define MTK_FD_OUTPUT_MIN_HEIGHT 26U
> +#define MTK_FD_OUTPUT_MAX_WIDTH 640U
> +#define MTK_FD_OUTPUT_MAX_HEIGHT 480U
> +
> +/* Control the user defined image widths and heights
> + * to be scaled and performed face detection in FD HW.
> + * MTK FD support up to 14 user defined image sizes to perform face detection.
> + */
> +#define V4L2_CID_MTK_FD_SCALE_IMG_WIDTH (V4L2_CID_USER_MTK_FD_BASE + 1)
> +#define V4L2_CID_MTK_FD_SCALE_IMG_HEIGHT (V4L2_CID_USER_MTK_FD_BASE + 2)
> +
> +/* Control the numbers of user defined image sizes.
> + * The default value is 0 which means user is not going
> + * to define the specific image sizes.
> + */
> +#define V4L2_CID_MTK_FD_SCALE_IMG_NUM (V4L2_CID_USER_MTK_FD_BASE + 3)
> +
> +/* Control the Face Pose to be detected.
> + * Here describe the value as following:
> + * {0, detect the front face with rotation from 0 to 270 degrees},
> + * {1, detect the front face with rotation from 0 to 240 and 300 degrees},
> + * {2, detect the front face with rotation from 0 to 240 and 330 degrees},
> + * {3, detect the front face with rotation from 0 to 240 and left side face}.
> + */
> +#define V4L2_CID_MTK_FD_DETECT_POSE (V4L2_CID_USER_MTK_FD_BASE + 4)
> +#define V4L2_CID_MTK_FD_DETECT_SPEEDUP (V4L2_CID_USER_MTK_FD_BASE + 5)
> +#define V4L2_CID_MTK_FD_EXTRA_MODEL (V4L2_CID_USER_MTK_FD_BASE + 6)
> +
> +/* We reserve 16 controls for this driver. */
> +#define V4L2_CID_MTK_FD_MAX 16
> +
> +#define ENABLE_FD 0x111
> +#define FD_HW_ENABLE 0x4
> +#define FD_INT_EN 0x15c
> +#define FD_INT 0x168
> +#define FD_RESULT 0x178
> +#define FD_IRQ_MASK 0x001
> +
> +#define RS_MAX_BUF_SIZE 2288788
> +#define FD_MAX_SPEEDUP 7
> +#define FD_MAX_POSE_VAL 0xfffffffffffffff
> +#define FD_DEF_POSE_VAL 0x3ff
> +#define MAX_FD_SEL_NUM 1026
> +
> +/* The max. number of face sizes could be detected, for feature scaling */
> +#define FACE_SIZE_NUM_MAX 14
> +/* FACE_SIZE_NUM_MAX + 1, first scale for input image W/H */
> +#define FD_SCALE_NUM 15
> +
> +enum fd_state {
> + FD_ENQ,
> + FD_CBD,
> +};
> +
> +enum fd_img_format {
> + FMT_VYUY = 2,
> + FMT_UYVY,
> + FMT_YVYU,
> + FMT_YUYV,
> + FMT_UNKNOWN
> +};
Please use #define macros for hardware/firmware interface definitions.
> +
> +struct fd_buffer {
> + __u32 scp_addr; /* used by SCP */
> + __u32 dma_addr; /* used by DMA HW */
> +} __packed;
> +
> +enum fd_scp_cmd {
> + FD_CMD_INIT,
> + FD_CMD_ENQUEUE,
> +};
Ditto.
> +
> +struct fd_user_output {
> + __u64 results[MAX_FD_SEL_NUM];
> + __u16 number;
> +};
> +
> +struct user_param {
> + u8 fd_pose;
> + u8 fd_speedup;
> + u8 fd_extra_model;
> + u8 scale_img_num;
> + u8 src_img_fmt;
> + __u16 scale_img_width[FD_SCALE_NUM];
> + __u16 scale_img_height[FD_SCALE_NUM];
> +} __packed;
> +
> +struct fd_hw_param {
> + struct fd_buffer src_img;
> + struct fd_buffer user_result;
> + struct user_param user_param;
> +} __packed;
> +
> +struct cmd_init_info {
> + struct fd_buffer fd_manager;
> + __u32 rs_dma_addr;
> +} __packed;
> +
> +struct ipi_message {
> + u8 cmd_id;
> + union {
> + struct cmd_init_info init_param;
> + struct fd_hw_param hw_param;
> + };
> +} __packed;
> +
> +struct mtk_fd_hw {
> + struct clk *fd_clk;
> + struct rproc *rproc_handle;
> + struct platform_device *scp_pdev;
> + struct fd_buffer scp_mem;
> + wait_queue_head_t wq;
> + void __iomem *fd_base;
> + atomic_t fd_user_cnt;
> + enum fd_state state;
> + u32 fd_irq_result;
> + /* Ensure only one job in hw */
> + struct mutex fd_hw_lock;
> +};
> +
> +struct mtk_fd_dev {
> + struct platform_device *pdev;
> + struct v4l2_device v4l2_dev;
> + struct v4l2_m2m_dev *m2m_dev;
> + struct media_device mdev;
> + struct video_device vfd;
> + struct mtk_fd_hw fd_hw;
Could we just put the contents of that struct directly here? That should
simplify dereference chains in the code.
> + /* Lock for V4L2 operations */
> + struct mutex vfd_lock;
> +};
> +
> +struct mtk_fd_ctx {
> + struct mtk_fd_dev *fd_dev;
> + struct device *dev;
> + struct v4l2_fh fh;
> + struct v4l2_ctrl_handler hdl;
> + struct v4l2_pix_format_mplane src_fmt;
> + struct v4l2_meta_format dst_fmt;
> + struct user_param user_param;
> +};
> +
> +#endif/*__MTK_FD_HW_H__*/
> diff --git a/drivers/media/platform/mtk-isp/fd/mtk_fd_40.c b/drivers/media/platform/mtk-isp/fd/mtk_fd_40.c
> new file mode 100644
> index 0000000..246d3aa
> --- /dev/null
> +++ b/drivers/media/platform/mtk-isp/fd/mtk_fd_40.c
> @@ -0,0 +1,1259 @@
> +// SPDX-License-Identifier: GPL-2.0
> +//
> +// Copyright (c) 2018 MediaTek Inc.
> +
> +#include <linux/clk.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/interrupt.h>
> +#include <linux/jiffies.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_data/mtk_scp.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/remoteproc.h>
> +#include <media/videobuf2-dma-contig.h>
> +#include <media/videobuf2-v4l2.h>
> +#include <linux/wait.h>
> +#include <media/v4l2-ioctl.h>
> +#include <media/v4l2-event.h>
> +#include <media/v4l2-ctrls.h>
> +#include <media/v4l2-mem2mem.h>
> +#include <media/videobuf2-core.h>
> +
> +#include "mtk_fd.h"
> +
> +static struct v4l2_meta_format fw_param_fmts[] = {
const?
Also, isn't this the buffer with the detected faces rather than params?
> + {
> + .dataformat = V4L2_META_FMT_MTISP_PARAMS,
This should use its own format.
> + .buffersize = 1024 * 30,
Please define a C struct for this buffer and use sizeof() here.
> + },
> +};
Actually, is there a reason to have this array at all if there is only 1
format supported? I think we could just put the right constants directly in
the code.
> +
> +static const struct v4l2_pix_format_mplane in_img_fmts[] = {
> + {
> + .width = MTK_FD_OUTPUT_MAX_WIDTH,
> + .height = MTK_FD_OUTPUT_MAX_HEIGHT,
> + .pixelformat = V4L2_PIX_FMT_VYUY,
> + .colorspace = V4L2_COLORSPACE_BT2020,
> + .field = V4L2_FIELD_NONE,
> + .num_planes = 1,
> + },
> + {
> + .width = MTK_FD_OUTPUT_MAX_WIDTH,
> + .height = MTK_FD_OUTPUT_MAX_HEIGHT,
> + .pixelformat = V4L2_PIX_FMT_YUYV,
> + .colorspace = V4L2_COLORSPACE_BT2020,
> + .field = V4L2_FIELD_NONE,
> + .num_planes = 1,
> + },
> + {
> + .width = MTK_FD_OUTPUT_MAX_WIDTH,
> + .height = MTK_FD_OUTPUT_MAX_HEIGHT,
> + .pixelformat = V4L2_PIX_FMT_YVYU,
> + .colorspace = V4L2_COLORSPACE_BT2020,
> + .field = V4L2_FIELD_NONE,
> + .num_planes = 1,
> + },
> + {
> + .width = MTK_FD_OUTPUT_MAX_WIDTH,
> + .height = MTK_FD_OUTPUT_MAX_HEIGHT,
> + .pixelformat = V4L2_PIX_FMT_UYVY,
> + .colorspace = V4L2_COLORSPACE_BT2020,
> + .field = V4L2_FIELD_NONE,
> + .num_planes = 1,
If all the formats have the same values for a field, there is no reason to
have the field initialized here. Just make initialize them to the constant
values inside TRY_FMT.
Hmm, are the interleaved YUV formats the only formats supported by this
hardware? That would be quite unfortunate given the formats supported by the
other IP blocks on this SoC use the more typical planar formats.
> + },
> +};
> +
> +#define NUM_FORMATS ARRAY_SIZE(in_img_fmts)
> +
> +static inline struct mtk_fd_dev *mtk_fd_hw_to_dev(struct mtk_fd_hw *fd_hw)
> +{
> + return container_of(fd_hw, struct mtk_fd_dev, fd_hw);
> +}
> +
> +static inline struct mtk_fd_ctx *fh_to_ctx(struct v4l2_fh *fh)
> +{
> + return container_of(fh, struct mtk_fd_ctx, fh);
> +}
> +
> +static int mtk_fd_load_scp(struct mtk_fd_hw *fd_hw)
> +{
> + struct mtk_fd_dev *fd_dev = mtk_fd_hw_to_dev(fd_hw);
> + struct device *dev = &fd_dev->pdev->dev;
> + phandle rproc_phandle;
> + int ret;
> +
> + /* init scp */
> + fd_hw->scp_pdev = scp_get_pdev(fd_dev->pdev);
> + if (!fd_hw->scp_pdev) {
> + dev_err(dev, "Failed to get scp device\n");
> + return -ENODEV;
> + }
> +
> + if (of_property_read_u32(fd_dev->pdev->dev.of_node, "mediatek,scp",
> + &rproc_phandle)) {
> + dev_err(dev, "Could not get scp device\n");
> + return -EINVAL;
> + }
> +
> + fd_hw->rproc_handle = rproc_get_by_phandle(rproc_phandle);
> + if (!fd_hw->rproc_handle) {
> + dev_err(dev, "Could not get FD's rproc_handle\n");
> + return -EINVAL;
> + }
> +
> + ret = rproc_boot(fd_hw->rproc_handle);
> + if (ret < 0) {
> + /**
> + * Return 0 if downloading firmware successfully,
> + * otherwise it is failed
> + */
> + dev_err(dev, "rproc_boot failed\n");
> + return -ENODEV;
> + }
> +
> + return ret;
> +}
> +
> +static dma_addr_t mtk_fd_hw_alloc_rs_dma_addr(struct mtk_fd_hw *fd_hw)
> +{
> + struct mtk_fd_dev *fd_dev = mtk_fd_hw_to_dev(fd_hw);
> + struct device *dev = &fd_dev->pdev->dev;
> + void *va;
> + dma_addr_t dma_handle;
> +
> + va = dma_alloc_coherent(dev, RS_MAX_BUF_SIZE, &dma_handle, GFP_KERNEL);
I see this allocated here, but I don't see this freed anywhere.
> + if (!va) {
> + dev_err(dev, "dma_alloc null va\n");
> + return -ENOMEM;
> + }
> + memset(va, 0, RS_MAX_BUF_SIZE);
> +
> + return dma_handle;
> +}
> +
> +static int mtk_fd_send_ipi_init(struct mtk_fd_hw *fd_hw)
> +{
> + struct ipi_message fd_init_msg;
> + dma_addr_t rs_dma_addr;
> +
> + fd_init_msg.cmd_id = FD_CMD_INIT;
> +
> + fd_init_msg.init_param.fd_manager.scp_addr = fd_hw->scp_mem.scp_addr;
> + fd_init_msg.init_param.fd_manager.dma_addr = fd_hw->scp_mem.dma_addr;
> +
> + rs_dma_addr = mtk_fd_hw_alloc_rs_dma_addr(fd_hw);
> + if (!rs_dma_addr)
> + return -ENOMEM;
> +
> + fd_init_msg.init_param.rs_dma_addr = rs_dma_addr;
> +
> + return scp_ipi_send(fd_hw->scp_pdev, SCP_IPI_FD_CMD, &fd_init_msg,
> + sizeof(fd_init_msg), 0);
> +}
> +
> +static int mtk_fd_hw_enable(struct mtk_fd_hw *fd_hw)
> +{
> + int ret;
> +
> + ret = mtk_fd_load_scp(fd_hw);
> + if (ret)
> + return ret;
> +
> + ret = mtk_fd_send_ipi_init(fd_hw);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> +static int mtk_fd_hw_connect(struct mtk_fd_hw *fd_hw)
> +{
> + struct mtk_fd_dev *fd_dev = mtk_fd_hw_to_dev(fd_hw);
> + s32 usercount;
> +
> + mutex_lock(&fd_hw->fd_hw_lock);
First of all, we don't need a mutex here, because all the V4L2 ioctls are
already running with the video device mutex.
> + usercount = atomic_inc_return(&fd_hw->fd_user_cnt);
If we already use a mutex, there is no point in using atomic types.
> + if (usercount == 1) {
> + pm_runtime_get_sync(&fd_dev->pdev->dev);
> + if (mtk_fd_hw_enable(fd_hw)) {
> + pm_runtime_put_sync(&fd_dev->pdev->dev);
> + atomic_dec_return(&fd_hw->fd_user_cnt);
> + mutex_unlock(&fd_hw->fd_hw_lock);
> + return -EINVAL;
> + }
> + }
This is a simple mem-to-mem device, so there is no reason to keep it active
all the time it's streaming. Please just get the runtime PM counter when
queuing a job to the hardware and release it when the job finishes.
I guess we might still want to do the costly operations like rproc_boot()
when we start streaming, though.
> + mutex_unlock(&fd_hw->fd_hw_lock);
> +
> + return 0;
> +}
> +
> +static int mtk_fd_wait_irq(struct mtk_fd_hw *fd_hw)
> +{
> + int timeout;
> + struct mtk_fd_dev *fd_dev = mtk_fd_hw_to_dev(fd_hw);
> + struct device *dev = &fd_dev->pdev->dev;
> +
> + timeout = wait_event_interruptible_timeout
> + (fd_hw->wq, (fd_hw->fd_irq_result & FD_IRQ_MASK),
> + usecs_to_jiffies(1000000));
> + if (!timeout) {
> + dev_err(dev, "%s timeout, %d\n", __func__,
> + fd_hw->fd_irq_result);
> + return -EAGAIN;
> + } else if (!(fd_hw->fd_irq_result & FD_IRQ_MASK)) {
> + dev_err(dev, "%s No IRQ mask:0x%8x\n",
> + __func__, fd_hw->fd_irq_result);
> + return -EINVAL;
> + }
> + fd_hw->fd_irq_result = 0;
> +
> + return 0;
> +}
> +
> +static void mtk_fd_hw_disconnect(struct mtk_fd_hw *fd_hw)
> +{
> + struct mtk_fd_dev *fd_dev = mtk_fd_hw_to_dev(fd_hw);
> + s32 usercount;
> +
> + mutex_lock(&fd_hw->fd_hw_lock);
> + atomic_dec_return(&fd_hw->fd_user_cnt);
> + usercount = atomic_read(&fd_hw->fd_user_cnt);
> + if (usercount == 0) {
> + if (fd_hw->state == FD_ENQ)
> + mtk_fd_wait_irq(fd_hw);
This shouldn't be possible to happen as the queues should be already stopped
at this point.
> +
> + pm_runtime_put_sync(&fd_dev->pdev->dev);
Any reason to use pm_runtime_put_sync() over pm_runtime_put()?
> + rproc_shutdown(fd_hw->rproc_handle);
> + rproc_put(fd_hw->rproc_handle);
> + }
> + mutex_unlock(&fd_hw->fd_hw_lock);
> +}
> +
> +static void mtk_fd_hw_job_finish(struct mtk_fd_hw *fd_hw,
> + struct fd_hw_param *fd_param,
> + enum vb2_buffer_state vb_state)
> +{
> + struct mtk_fd_dev *fd_dev = mtk_fd_hw_to_dev(fd_hw);
> + struct mtk_fd_ctx *ctx;
> + struct device *dev = &fd_dev->pdev->dev;
> + struct vb2_buffer *src_vb, *dst_vb;
> + struct vb2_v4l2_buffer *src_vbuf = NULL, *dst_vbuf = NULL;
> +
> + ctx = v4l2_m2m_get_curr_priv(fd_dev->m2m_dev);
> + if (!ctx) {
> + dev_err(dev, "Instance released before end of transaction\n");
> + return;
> + }
This shouldn't be possible. I suspect you're seeing this because
.stop_streaming is not implemented correctly. See my comment there.
> +
> + src_vb = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
> + if (WARN_ON(!src_vb))
> + return;
This shouldn't be possible.
> + src_vbuf = to_vb2_v4l2_buffer(src_vb);
> +
> + dst_vb = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
> + if (WARN_ON(!dst_vb))
> + return;
Ditto.
> + dst_vbuf = to_vb2_v4l2_buffer(dst_vb);
> +
> + dst_vbuf->vb2_buf.timestamp = src_vbuf->vb2_buf.timestamp;
> + dst_vbuf->timecode = src_vbuf->timecode;
> + dst_vbuf->flags &= ~V4L2_BUF_FLAG_TSTAMP_SRC_MASK;
> + dst_vbuf->flags |= src_vbuf->flags & V4L2_BUF_FLAG_TSTAMP_SRC_MASK;
We should be able to just use v4l2_m2m_buf_copy_metadata().
> +
> + v4l2_m2m_buf_done(src_vbuf, vb_state);
> + v4l2_m2m_buf_done(dst_vbuf, vb_state);
> + v4l2_m2m_job_finish(fd_dev->m2m_dev, ctx->fh.m2m_ctx);
> +}
> +
> +static int mtk_fd_hw_job_exec(struct mtk_fd_hw *fd_hw,
> + struct fd_hw_param *fd_param,
> + void *output_vaddr)
> +{
> + struct fd_user_output *fd_output;
> + struct ipi_message fd_ipi_msg;
> + int ret;
> + u32 num;
> +
> + if (fd_param->user_param.src_img_fmt == FMT_UNKNOWN)
> + goto param_err;
Is this possible?
> +
> + mutex_lock(&fd_hw->fd_hw_lock);
> + fd_hw->state = FD_ENQ;
What is this state for?
> + fd_ipi_msg.cmd_id = FD_CMD_ENQUEUE;
> + memcpy(&fd_ipi_msg.hw_param, fd_param, sizeof(fd_ipi_msg.hw_param));
> + ret = scp_ipi_send(fd_hw->scp_pdev, SCP_IPI_FD_CMD, &fd_ipi_msg,
> + sizeof(fd_ipi_msg), 0);
> + if (ret)
> + goto buf_err;
> +
> + ret = mtk_fd_wait_irq(fd_hw);
> + if (ret)
> + goto buf_err;
This function is called from device_run and it shouldn't be synchronous. It
should only queue the job to the hardware to be handled asynchronously when
it finishes.
> +
> + num = readl(fd_hw->fd_base + FD_RESULT);
> + /* Disable FD ISR */
> + writel(0x0, fd_hw->fd_base + FD_INT_EN);
> +
> + fd_output = (struct fd_user_output *)output_vaddr;
> + fd_output->number = num;
> + fd_hw->state = FD_CBD;
> + mutex_unlock(&fd_hw->fd_hw_lock);
> +
> + mtk_fd_hw_job_finish(fd_hw, fd_param, VB2_BUF_STATE_DONE);
> + return 0;
> +
> +buf_err:
> + mutex_unlock(&fd_hw->fd_hw_lock);
> +param_err:
> + mtk_fd_hw_job_finish(fd_hw, fd_param, VB2_BUF_STATE_ERROR);
> + return ret;
> +}
> +
> +static int mtk_fd_vb2_buf_out_validate(struct vb2_buffer *vb)
> +{
> + struct vb2_v4l2_buffer *v4l2_buf = to_vb2_v4l2_buffer(vb);
> +
> + v4l2_buf->field = V4L2_FIELD_NONE;
We need to fail with -EINVAL if v4l2_buf->field was different than
V4L2_FIELD_ANY or _NONE.
> +
> + return 0;
> +}
> +
> +static int mtk_fd_vb2_buf_prepare(struct vb2_buffer *vb)
> +{
> + struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> + struct vb2_queue *vq = vb->vb2_queue;
> + struct mtk_fd_ctx *ctx = vb2_get_drv_priv(vq);
> + struct device *dev = ctx->dev;
> + struct v4l2_pix_format_mplane *pixfmt;
> +
> + switch (vq->type) {
> + case V4L2_BUF_TYPE_META_CAPTURE:
> + if (vb2_plane_size(vb, 0) < ctx->dst_fmt.buffersize) {
> + dev_err(dev, "meta size %d is too small\n");
Please make this dev_dbg(), because userspace misbehavior must not trigger
kernel error logs.
> + return -EINVAL;
> + }
> + break;
> + case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
> + pixfmt = &ctx->src_fmt;
> +
> + if (vbuf->field == V4L2_FIELD_ANY)
> + vbuf->field = V4L2_FIELD_NONE;
> +
> + if (vb->num_planes != 1 || vbuf->field != V4L2_FIELD_NONE) {
> + dev_err(dev, "plane or field %d not supported\n",
> + vb->num_planes, vbuf->field);
Ditto.
> + return -EINVAL;
> + }
> + if (vb2_plane_size(vb, 0) < pixfmt->plane_fmt[0].sizeimage) {
> + dev_err(dev, "plane %d is too small\n");
Ditto.
> + return -EINVAL;
> + }
> + break;
> + default:
> + dev_err(dev, "invalid queue type: %d\n", vq->type);
> + return -EINVAL;
We shouldn't need to handle this.
> + }
> +
> + return 0;
> +}
> +
> +static void mtk_fd_vb2_buf_queue(struct vb2_buffer *vb)
> +{
> + struct mtk_fd_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
> + struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> +
> + v4l2_m2m_buf_queue(ctx->fh.m2m_ctx, vbuf);
> +}
> +
> +static int mtk_fd_vb2_queue_setup(struct vb2_queue *vq,
> + unsigned int *num_buffers,
> + unsigned int *num_planes,
> + unsigned int sizes[],
> + struct device *alloc_devs[])
> +{
> + struct mtk_fd_ctx *ctx = vb2_get_drv_priv(vq);
> + struct device *dev = ctx->dev;
> + unsigned int size;
> +
> + switch (vq->type) {
> + case V4L2_BUF_TYPE_META_CAPTURE:
> + size = ctx->dst_fmt.buffersize;
> + break;
> + case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
> + size = ctx->src_fmt.plane_fmt[0].sizeimage;
> + break;
> + default:
> + dev_err(dev, "invalid queue type: %d\n", vq->type);
We should need to handle this.
> + return -EINVAL;
> + }
> +
> + if (!*num_planes) {
> + *num_planes = 1;
> + sizes[0] = size;
> + }
We need to handle the case when *num_planes is non-zero. We then need to
check if it's 1 and *sizes >= size and fail with -EINVAL if not.
> +
> + return 0;
> +}
> +
> +static int mtk_fd_vb2_start_streaming(struct vb2_queue *vq, unsigned int count)
> +{
> + struct mtk_fd_ctx *ctx = vb2_get_drv_priv(vq);
> +
> + return mtk_fd_hw_connect(&ctx->fd_dev->fd_hw);
This would be called twice for every context, once for the VIDEO_OUTPUT
queue and once for the META_CAPTURE queue. Perhaps it would make sense to
just do this for the VIDEO_OUTPUT queue?
> +}
> +
> +static void mtk_fd_vb2_stop_streaming(struct vb2_queue *vq)
> +{
> + struct mtk_fd_ctx *ctx = vb2_get_drv_priv(vq);
> + struct vb2_buffer *vb;
How do we guarantee here that the hardware isn't still accessing the buffers
removed below?
> +
> + if (V4L2_TYPE_IS_OUTPUT(vq->type))
> + vb = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
> + else
> + vb = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
> +
> + while (vb) {
> + v4l2_m2m_buf_done(to_vb2_v4l2_buffer(vb), VB2_BUF_STATE_ERROR);
> + if (V4L2_TYPE_IS_OUTPUT(vq->type))
> + vb = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
> + else
> + vb = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
> + }
We can use v4l2_m2m_buf_remove(). Also we can move the call into the loop
condition:
while ((vb == v4l2_m2m_buf_remove(...)))
v4l2_m2m_buf_done(...);
> +
> + mtk_fd_hw_disconnect(&ctx->fd_dev->fd_hw);
This should also probably be done only for 1 queue. Since the VIDEO_OUTPUT
queue supports requestes, I'd consider it the master one.
> +}
> +
> +static void mtk_fd_vb2_request_complete(struct vb2_buffer *vb)
> +{
> + struct mtk_fd_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
> +
> + v4l2_ctrl_request_complete(vb->req_obj.req, &ctx->hdl);
> +}
> +
> +static void mtk_fd_fill_pixfmt_mp(struct v4l2_pix_format_mplane *dfmt,
> + const struct v4l2_pix_format_mplane *sfmt)
> +{
> + dfmt->width = sfmt->width;
> + dfmt->height = sfmt->height;
> + dfmt->pixelformat = sfmt->pixelformat;
> + dfmt->field = sfmt->field;
> + dfmt->colorspace = sfmt->colorspace;
> + dfmt->num_planes = sfmt->num_planes;
> +
> + /* Use default */
> + dfmt->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
> + dfmt->quantization = V4L2_QUANTIZATION_DEFAULT;
> + dfmt->xfer_func =
> + V4L2_MAP_XFER_FUNC_DEFAULT(dfmt->colorspace);
> + dfmt->plane_fmt[0].bytesperline = dfmt->width * 2;
> + dfmt->plane_fmt[0].sizeimage =
> + dfmt->height * dfmt->plane_fmt[0].bytesperline;
> + memset(dfmt->reserved, 0, sizeof(dfmt->reserved));
> +}
Could we unify this function with mtk_fd_m2m_try_fmt_out_mp()? That function
should be almost directly reusable for the default format initialization +/-
the arguments passed to it.
> +
> +static const struct v4l2_pix_format_mplane *mtk_fd_find_fmt(u32 format)
> +{
> + unsigned int i;
> + const struct v4l2_pix_format_mplane *dev_fmt;
> +
> + for (i = 0; i < NUM_FORMATS; i++) {
> + dev_fmt = &in_img_fmts[i];
> + if (dev_fmt->pixelformat == format)
> + return dev_fmt;
> + }
> +
> + return NULL;
> +}
> +
> +static int mtk_fd_m2m_querycap(struct file *file, void *fh,
> + struct v4l2_capability *cap)
> +{
> + struct mtk_fd_dev *fd_dev = video_drvdata(file);
> + struct device *dev = &fd_dev->pdev->dev;
> +
> + strscpy(cap->driver, dev->driver->name, sizeof(cap->driver));
Please use dev_driver_string().
> + strscpy(cap->card, fd_dev->vfd.name, sizeof(cap->card));
I think we can also make this dev_driver_string().
> + snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:%s",
> + dev_name(&fd_dev->pdev->dev));
> +
> + return 0;
> +}
> +
> +static int mtk_fd_m2m_enum_fmt_out_mp(struct file *file, void *fh,
> + struct v4l2_fmtdesc *f)
> +{
> + int i;
> +
> + for (i = 0; i < NUM_FORMATS; ++i) {
> + if (i == f->index) {
> + f->pixelformat = in_img_fmts[i].pixelformat;
> + return 0;
> + }
> + }
Why don't we just check if f->index is within the [0, ARRAY_SIZE()-1] bounds
and then just use it to index the array directly?
> +
> + return -EINVAL;
> +}
> +
> +static int mtk_fd_m2m_try_fmt_out_mp(struct file *file,
> + void *fh,
> + struct v4l2_format *f)
I think we could just shorten the function prefixes to "mtk_fd_".
> +{
> + struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp;
> + const struct v4l2_pix_format_mplane *fmt;
> +
> + fmt = mtk_fd_find_fmt(pix_mp->pixelformat);
> + if (!fmt) {
> + /* Get default img fmt */
> + fmt = &in_img_fmts[0];
> + f->fmt.pix.pixelformat = fmt->pixelformat;
> + }
> +
> + /* Use default */
> + pix_mp->field = fmt->field;
> + pix_mp->colorspace = fmt->colorspace;
> + pix_mp->num_planes = fmt->num_planes;
> + pix_mp->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
> + pix_mp->quantization = V4L2_QUANTIZATION_DEFAULT;
> + pix_mp->xfer_func =
> + V4L2_MAP_XFER_FUNC_DEFAULT(pix_mp->colorspace);
> +
> + /* Keep user setting as possible */
> + pix_mp->width = clamp(pix_mp->width,
> + MTK_FD_OUTPUT_MIN_WIDTH,
> + MTK_FD_OUTPUT_MAX_WIDTH);
> + pix_mp->height = clamp(pix_mp->height,
> + MTK_FD_OUTPUT_MIN_HEIGHT,
> + MTK_FD_OUTPUT_MAX_HEIGHT);
> +
> + pix_mp->plane_fmt[0].bytesperline = pix_mp->width * 2;
Please add a comment explaining why this is always 2.
> + pix_mp->plane_fmt[0].sizeimage =
> + pix_mp->plane_fmt[0].bytesperline * pix_mp->height;
> + memset(pix_mp->plane_fmt[0].reserved, 0,
> + sizeof(pix_mp->plane_fmt[0].reserved));
No need to zero this here. The core will handle it.
> +
> + return 0;
> +}
> +
> +static int mtk_fd_m2m_g_fmt_out_mp(struct file *file, void *fh,
> + struct v4l2_format *f)
> +{
> + struct mtk_fd_ctx *ctx = fh_to_ctx(fh);
> +
> + f->fmt.pix_mp = ctx->src_fmt;
> +
> + return 0;
> +}
> +
> +static int mtk_fd_m2m_s_fmt_out_mp(struct file *file, void *fh,
> + struct v4l2_format *f)
> +{
> + struct mtk_fd_ctx *ctx = fh_to_ctx(fh);
> + struct vb2_queue *vq;
> +
> + /* Change not allowed if queue is streaming. */
> + vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, f->type);
> + if (vb2_is_streaming(vq) || vb2_is_busy(vq)) {
vb2_is_busy() is a superset of vb2_is_streaming(), so it's enough to just
check the former.
> + dev_dbg(ctx->dev, "vb2_is_streaming or vb2_is_busy");
> + return -EBUSY;
> + }
> +
> + mtk_fd_m2m_try_fmt_out_mp(file, fh, f);
> + ctx->src_fmt = f->fmt.pix_mp;
> +
> + return 0;
> +}
> +
> +static int mtk_fd_m2m_enum_fmt_meta_cap(struct file *file, void *fh,
> + struct v4l2_fmtdesc *f)
> +{
> + if (f->index)
> + return -EINVAL;
> +
> + strscpy(f->description, "Face detection result",
> + sizeof(f->description));
> + f->pixelformat = fw_param_fmts[0].dataformat;
> + f->flags = 0;
> +
> + return 0;
> +}
> +
> +static int mtk_fd_m2m_g_fmt_meta_cap(struct file *file, void *fh,
> + struct v4l2_format *f)
> +{
> + f->fmt.meta.dataformat = fw_param_fmts[0].dataformat;
> + f->fmt.meta.buffersize = fw_param_fmts[0].buffersize;
> +
> + return 0;
> +}
> +
> +static const struct vb2_ops mtk_fd_vb2_ops = {
> + .queue_setup = mtk_fd_vb2_queue_setup,
> + .buf_out_validate = mtk_fd_vb2_buf_out_validate,
> + .buf_prepare = mtk_fd_vb2_buf_prepare,
> + .buf_queue = mtk_fd_vb2_buf_queue,
> + .start_streaming = mtk_fd_vb2_start_streaming,
> + .stop_streaming = mtk_fd_vb2_stop_streaming,
> + .wait_prepare = vb2_ops_wait_prepare,
> + .wait_finish = vb2_ops_wait_finish,
> + .buf_request_complete = mtk_fd_vb2_request_complete,
> +};
> +
> +static const struct v4l2_ioctl_ops mtk_fd_v4l2_video_out_ioctl_ops = {
> + .vidioc_querycap = mtk_fd_m2m_querycap,
> + .vidioc_enum_fmt_vid_out_mplane = mtk_fd_m2m_enum_fmt_out_mp,
> + .vidioc_g_fmt_vid_out_mplane = mtk_fd_m2m_g_fmt_out_mp,
> + .vidioc_s_fmt_vid_out_mplane = mtk_fd_m2m_s_fmt_out_mp,
> + .vidioc_try_fmt_vid_out_mplane = mtk_fd_m2m_try_fmt_out_mp,
> + .vidioc_enum_fmt_meta_cap = mtk_fd_m2m_enum_fmt_meta_cap,
> + .vidioc_g_fmt_meta_cap = mtk_fd_m2m_g_fmt_meta_cap,
> + .vidioc_s_fmt_meta_cap = mtk_fd_m2m_g_fmt_meta_cap,
> + .vidioc_try_fmt_meta_cap = mtk_fd_m2m_g_fmt_meta_cap,
> + .vidioc_reqbufs = v4l2_m2m_ioctl_reqbufs,
> + .vidioc_create_bufs = v4l2_m2m_ioctl_create_bufs,
> + .vidioc_expbuf = v4l2_m2m_ioctl_expbuf,
> + .vidioc_prepare_buf = v4l2_m2m_ioctl_prepare_buf,
> + .vidioc_querybuf = v4l2_m2m_ioctl_querybuf,
> + .vidioc_qbuf = v4l2_m2m_ioctl_qbuf,
> + .vidioc_dqbuf = v4l2_m2m_ioctl_dqbuf,
> + .vidioc_streamon = v4l2_m2m_ioctl_streamon,
> + .vidioc_streamoff = v4l2_m2m_ioctl_streamoff,
> + .vidioc_subscribe_event = v4l2_ctrl_subscribe_event,
> + .vidioc_unsubscribe_event = v4l2_event_unsubscribe,
> +};
> +
> +static int
> +mtk_fd_queue_init(void *priv, struct vb2_queue *src_vq,
> + struct vb2_queue *dst_vq)
> +{
> + struct mtk_fd_ctx *ctx = priv;
> + int ret;
> +
> + src_vq->type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
> + src_vq->io_modes = VB2_MMAP | VB2_DMABUF;
> + src_vq->supports_requests = true;
> + src_vq->drv_priv = ctx;
> + src_vq->ops = &mtk_fd_vb2_ops;
> + src_vq->mem_ops = &vb2_dma_contig_memops;
> + src_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
> + src_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> + src_vq->lock = &ctx->fd_dev->vfd_lock;
> + src_vq->dev = ctx->fd_dev->v4l2_dev.dev;
> +
> + ret = vb2_queue_init(src_vq);
> + if (ret)
> + return ret;
> +
> + dst_vq->type = V4L2_BUF_TYPE_META_CAPTURE;
> + dst_vq->io_modes = VB2_MMAP | VB2_DMABUF;
> + dst_vq->drv_priv = ctx;
> + dst_vq->ops = &mtk_fd_vb2_ops;
> + dst_vq->mem_ops = &vb2_dma_contig_memops;
> + dst_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
> + dst_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> + dst_vq->lock = &ctx->fd_dev->vfd_lock;
> + dst_vq->dev = ctx->fd_dev->v4l2_dev.dev;
> +
> + return vb2_queue_init(dst_vq);
> +}
> +
> +static int mtk_fd_dev_g_ctrl(struct v4l2_ctrl *ctrl)
> +{
> + struct mtk_fd_ctx *ctx = ctrl->priv;
> + int i;
> +
> + switch (ctrl->id) {
> + case V4L2_CID_MTK_FD_SCALE_IMG_WIDTH:
> + for (i = 0; i < ctrl->elems; i++)
> + ctrl->p_new.p_u16[i] =
> + ctx->user_param.scale_img_width[i];
> + break;
> + case V4L2_CID_MTK_FD_SCALE_IMG_HEIGHT:
> + for (i = 0; i < ctrl->elems; i++)
> + ctrl->p_new.p_u16[i] =
> + ctx->user_param.scale_img_height[i];
> + break;
> + case V4L2_CID_MTK_FD_DETECT_POSE:
> + ctrl->val = ctx->user_param.fd_pose;
> + break;
> + case V4L2_CID_MTK_FD_DETECT_SPEEDUP:
> + ctrl->val = ctx->user_param.fd_speedup;
> + break;
> + case V4L2_CID_MTK_FD_SCALE_IMG_NUM:
> + ctrl->val = ctx->user_param.scale_img_num;
> + break;
> + case V4L2_CID_MTK_FD_EXTRA_MODEL:
> + ctrl->val = ctx->user_param.fd_extra_model;
> + break;
> + default:
> + dev_dbg(ctx->dev, "%s: unexpected control: 0x%x:%d",
> + __func__, ctrl->id, ctrl->val);
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int mtk_fd_dev_s_ctrl(struct v4l2_ctrl *ctrl)
> +{
> + struct mtk_fd_ctx *ctx = ctrl->priv;
> + int i;
> +
> + switch (ctrl->id) {
> + case V4L2_CID_MTK_FD_SCALE_IMG_WIDTH:
> + for (i = 0; i < ctrl->elems; i++)
> + ctx->user_param.scale_img_width[i] =
> + ctrl->p_new.p_u16[i];
> + break;
> + case V4L2_CID_MTK_FD_SCALE_IMG_HEIGHT:
> + for (i = 0; i < ctrl->elems; i++)
> + ctx->user_param.scale_img_height[i] =
> + ctrl->p_new.p_u16[i];
> + break;
> + case V4L2_CID_MTK_FD_DETECT_POSE:
> + ctx->user_param.fd_pose = ctrl->val;
> + break;
> + case V4L2_CID_MTK_FD_DETECT_SPEEDUP:
> + ctx->user_param.fd_speedup = ctrl->val;
> + break;
> + case V4L2_CID_MTK_FD_SCALE_IMG_NUM:
> + ctx->user_param.scale_img_num = ctrl->val;
> + break;
> + case V4L2_CID_MTK_FD_EXTRA_MODEL:
> + ctx->user_param.fd_extra_model = ctrl->val;
> + break;
> + default:
> + dev_dbg(ctx->dev, "%s: unexpected control: 0x%x:%d",
> + __func__, ctrl->id, ctrl->val);
> + return -EINVAL;
No need to handle this, as the framework will only pass the controls we
registered earlier to this function.
> + }
> +
> + return 0;
> +}
> +
> +static const struct v4l2_ctrl_ops mtk_fd_dev_ctrl_ops = {
> + .g_volatile_ctrl = mtk_fd_dev_g_ctrl,
I don't see any volatile controls below. No need to implement this callback.
> + .s_ctrl = mtk_fd_dev_s_ctrl,
I think you might not even need to implement this function (or just provide
a dummy one that returns 0 if its required) if you just use the controls
directly when preparing the job for the hardware. Check v4l2_ctrl_find().
> +};
> +
> +struct v4l2_ctrl_config mtk_fd_controls[] = {
> + {
> + .ops = &mtk_fd_dev_ctrl_ops,
> + .id = V4L2_CID_MTK_FD_SCALE_IMG_WIDTH,
> + .name = "FD scale image widths",
> + .type = V4L2_CTRL_TYPE_U16,
> + .min = MTK_FD_OUTPUT_MIN_WIDTH,
> + .max = MTK_FD_OUTPUT_MAX_WIDTH,
> + .step = 1,
> + .def = MTK_FD_OUTPUT_MAX_WIDTH,
> + .dims = { FD_SCALE_NUM },
Something wrong with indentation here.
> + },
> + {
> + .ops = &mtk_fd_dev_ctrl_ops,
> + .id = V4L2_CID_MTK_FD_SCALE_IMG_HEIGHT,
> + .name = "FD scale image heights",
> + .type = V4L2_CTRL_TYPE_U16,
> + .min = MTK_FD_OUTPUT_MIN_HEIGHT,
> + .max = MTK_FD_OUTPUT_MAX_HEIGHT,
> + .step = 1,
> + .def = MTK_FD_OUTPUT_MAX_HEIGHT,
> + .dims = { FD_SCALE_NUM },
> + },
> + {
> + .ops = &mtk_fd_dev_ctrl_ops,
> + .id = V4L2_CID_MTK_FD_SCALE_IMG_NUM,
> + .name = "FD scale size counts",
> + .type = V4L2_CTRL_TYPE_INTEGER,
> + .min = 0,
> + .max = FACE_SIZE_NUM_MAX,
> + .step = 1,
> + .def = 0,
> + },
> + {
> + .ops = &mtk_fd_dev_ctrl_ops,
> + .id = V4L2_CID_MTK_FD_DETECT_POSE,
> + .name = "FD detect face pose",
> + .type = V4L2_CTRL_TYPE_INTEGER,
> + .min = 0,
> + .max = 3,
> + .step = 1,
> + .def = 0,
> + },
> + {
> + .ops = &mtk_fd_dev_ctrl_ops,
> + .id = V4L2_CID_MTK_FD_DETECT_SPEEDUP,
> + .name = "FD detection speedup",
> + .type = V4L2_CTRL_TYPE_INTEGER,
> + .min = 0,
> + .max = FD_MAX_SPEEDUP,
> + .step = 1,
> + .def = 0,
> + },
> + {
> + .ops = &mtk_fd_dev_ctrl_ops,
> + .id = V4L2_CID_MTK_FD_EXTRA_MODEL,
> + .name = "FD use extra model",
> + .type = V4L2_CTRL_TYPE_BOOLEAN,
> + .min = 0,
> + .max = 1,
> + .step = 1,
> + .def = 0,
> + },
> +};
> +
> +static int mtk_fd_ctrls_setup(struct mtk_fd_ctx *ctx)
> +{
> + struct v4l2_ctrl_handler *hdl = &ctx->hdl;
> + struct v4l2_ctrl *ctl;
> + int i;
> +
> + v4l2_ctrl_handler_init(hdl, V4L2_CID_MTK_FD_MAX);
> + if (hdl->error)
> + return hdl->error;
> +
> + for (i = 0; i < ARRAY_SIZE(mtk_fd_controls); i++) {
> + ctl = v4l2_ctrl_new_custom(hdl, &mtk_fd_controls[i], ctx);
> + if (hdl->error) {
> + v4l2_ctrl_handler_free(hdl);
> + dev_err(ctx->dev, "Failed to register controls:%d", i);
> + return hdl->error;
> + }
> + }
> +
> + ctx->fh.ctrl_handler = &ctx->hdl;
> + v4l2_ctrl_handler_setup(hdl);
> +
> + return 0;
> +}
> +
> +static unsigned int get_fd_img_fmt(unsigned int fourcc)
> +{
> + switch (fourcc) {
> + case V4L2_PIX_FMT_VYUY:
> + return FMT_VYUY;
> + case V4L2_PIX_FMT_YUYV:
> + return FMT_YUYV;
> + case V4L2_PIX_FMT_YVYU:
> + return FMT_YVYU;
> + case V4L2_PIX_FMT_UYVY:
> + return FMT_UYVY;
> + default:
> + return FMT_UNKNOWN;
> + }
> +}
> +
> +static void init_ctx_fmt(struct mtk_fd_ctx *ctx)
> +{
> + const struct v4l2_pix_format_mplane *fmt;
> +
> + /* Initialize M2M source fmt */
> + fmt = &in_img_fmts[0];
> + mtk_fd_fill_pixfmt_mp(&ctx->src_fmt, fmt);
> +
> + /* Initialize M2M destination fmt */
> + ctx->dst_fmt.buffersize = fw_param_fmts[0].buffersize;
> + ctx->dst_fmt.dataformat = fw_param_fmts[0].dataformat;
Why not just make dst_fmt a pointer and assign &fw_params_fmt[0] there?
> +}
> +
> +/*
> + * V4L2 file operations.
> + */
> +static int mtk_vfd_open(struct file *filp)
> +{
> + struct mtk_fd_dev *fd_dev = video_drvdata(filp);
> + struct video_device *vdev = video_devdata(filp);
> + struct mtk_fd_ctx *ctx;
> + int ret;
> +
> + ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
> + if (!ctx)
> + return -ENOMEM;
> +
> + ctx->fd_dev = fd_dev;
> + ctx->dev = &fd_dev->pdev->dev;
> +
> + v4l2_fh_init(&ctx->fh, vdev);
> + filp->private_data = &ctx->fh;
> +
> + init_ctx_fmt(ctx);
> +
> + ret = mtk_fd_ctrls_setup(ctx);
> + if (ret) {
> + dev_err(ctx->dev, "Failed to set up controls:%d\n", ret);
> + goto err_fh_ctrl;
> + }
> +
> + ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(fd_dev->m2m_dev, ctx,
> + &mtk_fd_queue_init);
> + if (IS_ERR(ctx->fh.m2m_ctx)) {
> + ret = PTR_ERR(ctx->fh.m2m_ctx);
> + goto err_init_ctx;
> + }
> +
> + v4l2_fh_add(&ctx->fh);
> +
> + return 0;
> +
> +err_init_ctx:
> + v4l2_ctrl_handler_free(&ctx->hdl);
> +err_fh_ctrl:
> + v4l2_fh_exit(&ctx->fh);
> + kfree(ctx);
> +
> + return ret;
> +}
> +
> +static int mtk_vfd_release(struct file *filp)
> +{
> + struct mtk_fd_ctx *ctx = container_of(filp->private_data,
> + struct mtk_fd_ctx, fh);
> +
> + v4l2_m2m_ctx_release(ctx->fh.m2m_ctx);
> +
> + v4l2_ctrl_handler_free(&ctx->hdl);
> + v4l2_fh_del(&ctx->fh);
> + v4l2_fh_exit(&ctx->fh);
> +
> + kfree(ctx);
> +
> + return 0;
> +}
> +
> +static const struct v4l2_file_operations fd_video_fops = {
> + .owner = THIS_MODULE,
> + .open = mtk_vfd_open,
> + .release = mtk_vfd_release,
> + .poll = v4l2_m2m_fop_poll,
> + .unlocked_ioctl = video_ioctl2,
> + .mmap = v4l2_m2m_fop_mmap,
We also need the compat_ioctl here for 32-bit userspace on 64-bit kernels.
> +};
> +
> +static void mtk_fd_device_run(void *priv)
> +{
> + struct mtk_fd_ctx *ctx = priv;
> + struct vb2_v4l2_buffer *src_buf, *dst_buf;
> + struct fd_hw_param fd_param;
> + void *fd_result_vaddr;
> +
> + src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
> + dst_buf = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
> +
> + memset(&fd_param, 0, sizeof(fd_param));
> +
> + fd_param.src_img.dma_addr =
> + vb2_dma_contig_plane_dma_addr(&src_buf->vb2_buf, 0);
> + fd_param.user_result.dma_addr =
> + vb2_dma_contig_plane_dma_addr(&dst_buf->vb2_buf, 0);
> + fd_result_vaddr = vb2_plane_vaddr(&dst_buf->vb2_buf, 0);
> +
> + ctx->user_param.src_img_fmt = get_fd_img_fmt(ctx->src_fmt.pixelformat);
> + memcpy(&fd_param.user_param, &ctx->user_param, sizeof(ctx->user_param));
> +
> + /* Complete request controls if any */
> + v4l2_ctrl_request_complete(src_buf->vb2_buf.req_obj.req, &ctx->hdl);
> +
> + mtk_fd_hw_job_exec(&ctx->fd_dev->fd_hw, &fd_param, fd_result_vaddr);
> +}
> +
> +static struct v4l2_m2m_ops fd_m2m_ops = {
> + .device_run = mtk_fd_device_run,
> +};
> +
> +static int mtk_fd_request_validate(struct media_request *req)
> +{
> + unsigned int count;
> +
> + count = vb2_request_buffer_cnt(req);
> + if (!count)
> + return -ENOENT;
Why -ENOENT?
> + else if (count > 1)
> + return -EINVAL;
> +
> + return vb2_request_validate(req);
> +}
> +
> +static const struct media_device_ops fd_m2m_media_ops = {
> + .req_validate = mtk_fd_request_validate,
> + .req_queue = v4l2_m2m_request_queue,
> +};
> +
> +static int mtk_fd_video_device_register(struct mtk_fd_dev *fd_dev)
> +{
> + struct video_device *vfd = &fd_dev->vfd;
> + struct v4l2_m2m_dev *m2m_dev = fd_dev->m2m_dev;
> + struct device *dev = &fd_dev->pdev->dev;
> + int function, ret;
> +
> + vfd->fops = &fd_video_fops;
> + vfd->release = video_device_release;
> + vfd->lock = &fd_dev->vfd_lock;
> + vfd->v4l2_dev = &fd_dev->v4l2_dev;
> + vfd->vfl_dir = VFL_DIR_M2M;
> + vfd->device_caps = V4L2_CAP_STREAMING | V4L2_CAP_VIDEO_OUTPUT_MPLANE |
> + V4L2_CAP_META_CAPTURE;
> + vfd->ioctl_ops = &mtk_fd_v4l2_video_out_ioctl_ops;
> +
> + strscpy(vfd->name, "MTK-FD-V4L2", sizeof(vfd->name));
Please use dev_driver_string().
> + video_set_drvdata(vfd, fd_dev);
> +
> + ret = video_register_device(vfd, VFL_TYPE_GRABBER, 0);
> + if (ret) {
> + dev_err(dev, "Failed to register video device\n");
> + goto err_free_dev;
> + }
> +
> + function = MEDIA_ENT_F_PROC_VIDEO_DECODER;
MEDIA_ENT_F_PROC_VIDEO_STATISTICS fits here much more.
Also nit: Any reason to have this assigned to this intermediate variable
rather than just directly passed to the function?
> + ret = v4l2_m2m_register_media_controller(m2m_dev, vfd, function);
> + if (ret) {
> + dev_err(dev, "Failed to init mem2mem media controller\n");
> + goto err_unreg_video;
> + }
> + return 0;
> +
> +err_unreg_video:
> + video_unregister_device(vfd);
> +err_free_dev:
> + video_device_release(vfd);
> + return ret;
> +}
> +
> +static int mtk_fd_dev_v4l2_init(struct mtk_fd_dev *fd_dev)
> +{
> + struct media_device *mdev = &fd_dev->mdev;
> + struct device *dev = &fd_dev->pdev->dev;
> + int ret;
> +
> + ret = v4l2_device_register(dev, &fd_dev->v4l2_dev);
> + if (ret) {
> + dev_err(dev, "Failed to register v4l2 device\n");
> + return ret;
> + }
> +
> + fd_dev->m2m_dev = v4l2_m2m_init(&fd_m2m_ops);
> + if (IS_ERR(fd_dev->m2m_dev)) {
> + dev_err(dev, "Failed to init mem2mem device\n");
> + ret = PTR_ERR(fd_dev->m2m_dev);
> + goto fail_m2m_dev;
Please name the labels after the next clean-up step to be done, not the
failure.
> + }
> +
> + mdev->dev = dev;
> + strscpy(mdev->model, "MTK-FD-V4L2", sizeof(mdev->model));
Could we just use dev_driver_string() here instead?
> + snprintf(mdev->bus_info, sizeof(mdev->bus_info),
> + "platform:%s", dev_name(dev));
> + media_device_init(mdev);
> + mdev->ops = &fd_m2m_media_ops;
> + fd_dev->v4l2_dev.mdev = mdev;
> +
> + ret = mtk_fd_video_device_register(fd_dev);
> + if (ret) {
> + dev_err(dev, "Failed to register video device\n");
> + goto err_vdev;
> + }
> +
> + ret = media_device_register(mdev);
> + if (ret) {
> + dev_err(dev, "Failed to register mem2mem media device\n");
> + goto fail_mdev;
> + }
> +
> + return 0;
> +
> +fail_mdev:
> + v4l2_m2m_unregister_media_controller(fd_dev->m2m_dev);
> + video_unregister_device(&fd_dev->vfd);
> + video_device_release(&fd_dev->vfd);
> +err_vdev:
> + media_device_cleanup(mdev);
> + v4l2_m2m_release(fd_dev->m2m_dev);
> +fail_m2m_dev:
> + v4l2_device_unregister(&fd_dev->v4l2_dev);
> + return ret;
> +}
> +
> +static void mtk_fd_dev_v4l2_release(struct mtk_fd_dev *fd_dev)
> +{
> + v4l2_m2m_unregister_media_controller(fd_dev->m2m_dev);
> + video_unregister_device(&fd_dev->vfd);
> + video_device_release(&fd_dev->vfd);
> + media_device_cleanup(&fd_dev->mdev);
> + v4l2_m2m_release(fd_dev->m2m_dev);
> + v4l2_device_unregister(&fd_dev->v4l2_dev);
> +}
> +
> +static irqreturn_t mtk_fd_irq(int irq, void *data)
> +{
> + struct mtk_fd_hw *fd_hw = (struct mtk_fd_hw *)data;
> +
> + fd_hw->fd_irq_result = readl(fd_hw->fd_base + FD_INT);
We should be able to just handle the job completion directly from here.
> + wake_up_interruptible(&fd_hw->wq);
> + return IRQ_HANDLED;
> +}
> +
> +static int mtk_fd_hw_get_scp_mem(struct mtk_fd_hw *fd_hw,
> + struct fd_buffer *scp_mem)
> +{
> + struct mtk_fd_dev *fd_dev = mtk_fd_hw_to_dev(fd_hw);
> + struct device *dev = &fd_dev->pdev->dev;
> + dma_addr_t addr;
> + u32 size;
> +
> + scp_mem->scp_addr = scp_get_reserve_mem_phys(SCP_FD_MEM_ID);
> + size = scp_get_reserve_mem_size(SCP_FD_MEM_ID);
> + if (!scp_mem->scp_addr || !size)
> + return -EPROBE_DEFER;
Why -EPROBE_DEFER? Should we have some way to distinguish between the SCP
driver not being initialized yet and some other error?
> +
> + /* get dma addr address */
> + addr = dma_map_page_attrs(dev, phys_to_page(scp_mem->scp_addr), 0,
> + size, DMA_BIDIRECTIONAL,
Should this be really bidirectional?
> + DMA_ATTR_SKIP_CPU_SYNC);
> + if (dma_mapping_error(dev, addr)) {
> + scp_mem->scp_addr = 0;
> + dev_err(dev, "Failed to map scp addr\n");
> + return -ENOMEM;
> + }
> + scp_mem->dma_addr = addr;
> +
> + return 0;
> +}
> +
> +static int mtk_fd_probe(struct platform_device *pdev)
> +{
> + struct mtk_fd_dev *fd_dev;
nit: Perhaps just fd, to have shorter code?
> + struct device *dev = &pdev->dev;
> + struct mtk_fd_hw *fd_hw;
> + struct resource *res;
> + int irq;
> + int ret;
> +
> + fd_dev = devm_kzalloc(&pdev->dev, sizeof(*fd_dev), GFP_KERNEL);
> + if (!fd_dev)
> + return -ENOMEM;
> +
> + dev_set_drvdata(dev, fd_dev);
> + fd_hw = &fd_dev->fd_hw;
> + fd_dev->pdev = pdev;
Is pdev useful for anything? Perhaps you want to store &pdev->dev instead?
> +
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0) {
> + dev_err(dev, "no IRQ:%d resource info\n", irq);
> + return irq;
> + }
> + ret = devm_request_irq(dev, irq, mtk_fd_irq, IRQF_SHARED,
> + dev_driver_string(dev),
> + fd_hw);
Why not just fd_dev?
> + if (ret) {
> + dev_err(dev, "req_irq fail:%d\n", irq);
> + return ret;
> + }
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + fd_hw->fd_base = devm_ioremap_resource(dev, res);
> + if (!fd_hw->fd_base) {
> + dev_err(dev, "unable to get fd reg base\n");
> + return PTR_ERR(fd_hw->fd_base);
> + }
> +
> + fd_hw->fd_clk = devm_clk_get(dev, "fd");
> + if (IS_ERR(fd_hw->fd_clk)) {
> + dev_err(dev, "cannot get fd_clk_img_fd clock\n");
> + return PTR_ERR(fd_hw->fd_clk);
> + }
> +
> + ret = mtk_fd_hw_get_scp_mem(fd_hw, &fd_hw->scp_mem);
> + if (ret) {
> + dev_err(dev, "scp memory init failed: %d\n", ret);
nit: Could we make the error messages a bit more consistent? For example
"Failed to get IRQ resource (%d)", "Failed to request IRQ (%d)", "Failed to
... (%d)", etc.
> + return ret;
> + }
> +
> + atomic_set(&fd_hw->fd_user_cnt, 0);
> + init_waitqueue_head(&fd_hw->wq);
> + mutex_init(&fd_dev->vfd_lock);
> + mutex_init(&fd_hw->fd_hw_lock);
> + pm_runtime_enable(dev);
> +
> + ret = mtk_fd_dev_v4l2_init(fd_dev);
> + if (ret) {
> + mutex_destroy(&fd_dev->fd_hw.fd_hw_lock);
> + mutex_destroy(&fd_dev->vfd_lock);
> + pm_runtime_disable(&pdev->dev);
Please move the clean-up to an error path on the bottom of the function.
> + dev_err(dev, "v4l2 init failed: %d\n", ret);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int mtk_fd_remove(struct platform_device *pdev)
> +{
> + struct mtk_fd_dev *fd_dev = dev_get_drvdata(&pdev->dev);
> +
> + mtk_fd_dev_v4l2_release(fd_dev);
> + mutex_destroy(&fd_dev->fd_hw.fd_hw_lock);
> + mutex_destroy(&fd_dev->vfd_lock);
> + pm_runtime_disable(&pdev->dev);
The order of calls in remove should normally be the opposite to probe.
> +
> + return 0;
> +}
> +
> +static int mtk_fd_suspend(struct device *dev)
> +{
> + struct mtk_fd_dev *fd_dev = dev_get_drvdata(dev);
> +
> + if (pm_runtime_suspended(dev))
> + return 0;
> +
> + /* suspend FD HW */
> + writel(0x0, fd_dev->fd_hw.fd_base + FD_HW_ENABLE);
> + writel(0x0, fd_dev->fd_hw.fd_base + FD_INT_EN);
> + clk_disable_unprepare(fd_dev->fd_hw.fd_clk);
> + return 0;
> +}
> +
> +static int mtk_fd_resume(struct device *dev)
> +{
> + struct mtk_fd_dev *fd_dev = dev_get_drvdata(dev);
> + int ret;
> +
> + if (pm_runtime_suspended(dev))
> + return 0;
> +
> + ret = clk_prepare_enable(fd_dev->fd_hw.fd_clk);
> + if (ret < 0) {
> + dev_dbg(dev, "open fd clk failed\n");
> + clk_disable_unprepare(fd_dev->fd_hw.fd_clk);
> + }
> +
> + /* resume FD HW */
> + writel(ENABLE_FD, fd_dev->fd_hw.fd_base + FD_HW_ENABLE);
> + if (fd_dev->fd_hw.state == FD_ENQ)
> + writel(0x1, fd_dev->fd_hw.fd_base + FD_INT_EN);
> + return 0;
> +}
> +
> +static const struct dev_pm_ops mtk_fd_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(mtk_fd_suspend, mtk_fd_resume)
> + SET_RUNTIME_PM_OPS(mtk_fd_suspend, mtk_fd_resume, NULL)
We need separate runtime and system PM ops. Their behavior is expected to be
different.
For runtime PM ops, you the functions should just unconditionally power on
or power off the device.
For system PM ops, suspend needs to make sure that no further job is queued
to the hardware and that any job being already processed by the hardware is
completed. Resume needs to resume the processing if there are any further
jobs to be queued to the hardware.
> +};
> +
> +static const struct of_device_id mtk_fd_of_ids[] = {
> + { .compatible = "mediatek,mt8183-fd", },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, mtk_fd_of_ids);
> +
> +static struct platform_driver mtk_fd_driver = {
> + .probe = mtk_fd_probe,
> + .remove = mtk_fd_remove,
> + .driver = {
> + .name = "mtk-fd-4.0",
> + .of_match_table = of_match_ptr(mtk_fd_of_ids),
> + .pm = &mtk_fd_pm_ops,
> + }
> +};
> +module_platform_driver(mtk_fd_driver);
> +
> +MODULE_DESCRIPTION("Mediatek FD driver");
> +MODULE_LICENSE("GPL");
GPL v2
Best regards,
Tomasz
^ 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