Linux-Aspeed Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] hwmon: pmbus: Add Infineon PXE1610 VR driver
From: Guenter Roeck @ 2019-05-30  1:00 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20190529223511.4059120-1-vijaykhemka@fb.com>

On 5/29/19 3:35 PM, Vijay Khemka wrote:
> Added pmbus driver for the new device Infineon pxe1610
> voltage regulator. It also supports similar family device
> PXE1110 and PXM1310.
> 
> Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
> ---
>   drivers/hwmon/pmbus/Kconfig   |   9 +++
>   drivers/hwmon/pmbus/Makefile  |   1 +
>   drivers/hwmon/pmbus/pxe1610.c | 119 ++++++++++++++++++++++++++++++++++
>   3 files changed, 129 insertions(+)
>   create mode 100644 drivers/hwmon/pmbus/pxe1610.c
> 
> diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> index 30751eb9550a..338ef9b5a395 100644
> --- a/drivers/hwmon/pmbus/Kconfig
> +++ b/drivers/hwmon/pmbus/Kconfig
> @@ -154,6 +154,15 @@ config SENSORS_MAX8688
>   	  This driver can also be built as a module. If so, the module will
>   	  be called max8688.
>   
> +config SENSORS_PXE1610
> +	tristate "Infineon PXE1610"
> +	help
> +	  If you say yes here you get hardware monitoring support for Infineon
> +	  PXE1610.
> +
> +	  This driver can also be built as a module. If so, the module will
> +	  be called pxe1610.
> +
>   config SENSORS_TPS40422
>   	tristate "TI TPS40422"
>   	help
> diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
> index 2219b9300316..b0fbd017a91a 100644
> --- a/drivers/hwmon/pmbus/Makefile
> +++ b/drivers/hwmon/pmbus/Makefile
> @@ -18,6 +18,7 @@ obj-$(CONFIG_SENSORS_MAX20751)	+= max20751.o
>   obj-$(CONFIG_SENSORS_MAX31785)	+= max31785.o
>   obj-$(CONFIG_SENSORS_MAX34440)	+= max34440.o
>   obj-$(CONFIG_SENSORS_MAX8688)	+= max8688.o
> +obj-$(CONFIG_SENSORS_PXE1610)	+= pxe1610.o
>   obj-$(CONFIG_SENSORS_TPS40422)	+= tps40422.o
>   obj-$(CONFIG_SENSORS_TPS53679)	+= tps53679.o
>   obj-$(CONFIG_SENSORS_UCD9000)	+= ucd9000.o
> diff --git a/drivers/hwmon/pmbus/pxe1610.c b/drivers/hwmon/pmbus/pxe1610.c
> new file mode 100644
> index 000000000000..01e267944df5
> --- /dev/null
> +++ b/drivers/hwmon/pmbus/pxe1610.c
> @@ -0,0 +1,119 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Hardware monitoring driver for Infineon PXE1610
> + *
> + * Copyright (c) 2019 Facebook Inc
> + *
> + */
> +
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include "pmbus.h"
> +
> +/*
> + * Identify chip parameters.
> + */
> +static int pxe1610_identify(struct i2c_client *client,
> +			  struct pmbus_driver_info *info)

Please align continuation lines with '('.

> +{
> +	if (pmbus_check_byte_register(client, 0, PMBUS_VOUT_MODE)) {
> +		int vout_mode;
> +
> +		vout_mode = pmbus_read_byte_data(client, 0, PMBUS_VOUT_MODE);

pmbus_read_byte_data() can return an error. Calling pmbus_check_byte_register()
doesn't really add any value here, since the second call can still fail,
which needs to be checked.

> +		switch (vout_mode & 0x1f) {
> +		case 1:
> +			info->vrm_version = vr12;
> +		break;

Alignment is off.

> +		case 2:
> +			info->vrm_version = vr13;
> +		break;

Same here.

> +		default:
> +			return -ENODEV;
> +		}
> +	}
> +	return 0;
> +}
> +
> +static int pxe1610_probe(struct i2c_client *client,
> +			 const struct i2c_device_id *id)
> +{
> +	struct pmbus_driver_info *info;
> +	u8 buf[I2C_SMBUS_BLOCK_MAX];
> +	int ret;
> +
> +	if (!i2c_check_functionality(client->adapter,
> +				     I2C_FUNC_SMBUS_READ_BYTE_DATA
> +				| I2C_FUNC_SMBUS_READ_WORD_DATA
> +				| I2C_FUNC_SMBUS_READ_BLOCK_DATA))
> +		return -ENODEV;
> +
> +	/* By default this device doesn't boot to page 0, so set page 0
> +	 * to access all pmbus registers.
> +	 */

Please use standard multi-line comments.

> +	i2c_smbus_write_byte_data(client, 0, 0);
> +

Please use the PMBUS_PAGE command definition.

I wonder if it would make sense to initialize currpage in the core to an unreasonable
number for multi-page chips, but I guess that is a different question.

> +	/* Read Manufacturer id */
> +	ret = i2c_smbus_read_block_data(client, PMBUS_MFR_ID, buf);
> +	if (ret < 0) {
> +		dev_err(&client->dev, "Failed to read PMBUS_MFR_ID\n");
> +		return ret;
> +	}
> +	if (ret != 2 || strncmp(buf, "XP", strlen("XP"))) {

The strlen() is really unnecessary here. Just use 2 (and a define
for it if you like).

> +		dev_err(&client->dev, "MFR_ID unrecognised\n");

unrecognized. Oh well, turns out unrecognised is the British spelling and
just as valid, so feel free to keep it if you like.

> +		return -ENODEV;
> +	}
> +
> +	info = devm_kzalloc(&client->dev, sizeof(struct pmbus_driver_info),
> +			    GFP_KERNEL);
> +	if (!info)
> +		return -ENOMEM;
> +
> +	info->format[PSC_VOLTAGE_IN] = linear;
> +	info->format[PSC_VOLTAGE_OUT] = vid;
> +	info->format[PSC_CURRENT_IN] = linear;
> +	info->format[PSC_CURRENT_OUT] = linear;
> +	info->format[PSC_POWER] = linear;
> +	info->format[PSC_TEMPERATURE] = linear;
> +
> +	info->func[0] = PMBUS_HAVE_VIN
> +		| PMBUS_HAVE_VOUT | PMBUS_HAVE_IIN
> +		| PMBUS_HAVE_IOUT | PMBUS_HAVE_PIN
> +		| PMBUS_HAVE_POUT | PMBUS_HAVE_TEMP
> +		| PMBUS_HAVE_STATUS_VOUT | PMBUS_HAVE_STATUS_IOUT
> +		| PMBUS_HAVE_STATUS_INPUT | PMBUS_HAVE_STATUS_TEMP;
> +	info->func[1] = info->func[0];
> +	info->func[2] = info->func[0];
> +
> +	info->pages = id->driver_data;
> +	info->identify = pxe1610_identify;
> +

It doesn't really add value to initialize all these parameters manually.
I would suggest to use the approach from tps53679.c, ie have a static
structure and use devm_kmemdup() to pass a copy to pmbus_do_probe().

> +	return pmbus_do_probe(client, id, info);
> +}
> +
> +static const struct i2c_device_id pxe1610_id[] = {
> +	{"pxe1610", 3},
> +	{"pxe1110", 3},
> +	{"pxm1310", 3},

Unless there are chips with different page counts in the queue, using
driver_data to pass the number of pages does not add any value. Just
set num_pages to 3.

If you like, feel free to use a define instead of a constant.

> +	{}
> +};
> +
> +MODULE_DEVICE_TABLE(i2c, pxe1610_id);
> +
> +/* This is the driver that will be inserted */

This comment does not add any value.

> +static struct i2c_driver pxe1610_driver = {
> +	.driver = {
> +		   .name = "pxe1610",
> +		   },
> +	.probe = pxe1610_probe,
> +	.remove = pmbus_do_remove,
> +	.id_table = pxe1610_id,
> +};
> +
> +module_i2c_driver(pxe1610_driver);
> +
> +MODULE_AUTHOR("Vijay Khemka <vijaykhemka@fb.com>");
> +MODULE_DESCRIPTION("PMBus driver for Infineon PXE1610, PXE1110 and PXM1310");
> +MODULE_LICENSE("GPL");
> 


^ permalink raw reply

* [PATCH] ARM: dts: aspeed: g4: add video engine support
From: Joel Stanley @ 2019-05-30  0:34 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20190527112753.1681-1-a.filippov@yadro.com>

Hi Alexander,

On Mon, 27 May 2019 at 11:28, Alexander Filippov <a.filippov@yadro.com> wrote:
>
> Add a node to describe the video engine and VGA scratch registers on
> AST2400.

The scratch registers are unrelated to the video engine. As Andrew
pointed out, the bindings are not upstream either.

Can you re-spin this patch wit just the video engine changes?

We also need a platform to enable and test this on. Can you submit the
device tree for your system?

>
> These changes were copied from aspeed-g5.dtsi
>
> Signed-off-by: Alexander Filippov <a.filippov@yadro.com>
> ---
>  arch/arm/boot/dts/aspeed-g4.dtsi | 62 ++++++++++++++++++++++++++++++++
>  1 file changed, 62 insertions(+)
>
> diff --git a/arch/arm/boot/dts/aspeed-g4.dtsi b/arch/arm/boot/dts/aspeed-g4.dtsi
> index 6011692df15a..adc1804918df 100644
> --- a/arch/arm/boot/dts/aspeed-g4.dtsi
> +++ b/arch/arm/boot/dts/aspeed-g4.dtsi
> @@ -168,6 +168,10 @@
>                                         compatible = "aspeed,g4-pinctrl";
>                                 };
>
> +                               vga_scratch: scratch {
> +                                       compatible = "aspeed,bmc-misc";
> +                               };
> +
>                                 p2a: p2a-control {
>                                         compatible = "aspeed,ast2400-p2a-ctrl";
>                                         status = "disabled";
> @@ -195,6 +199,16 @@
>                                 reg = <0x1e720000 0x8000>;      // 32K
>                         };
>
> +                       video: video at 1e700000 {
> +                               compatible = "aspeed,ast2400-video-engine";
> +                               reg = <0x1e700000 0x1000>;
> +                               clocks = <&syscon ASPEED_CLK_GATE_VCLK>,
> +                                        <&syscon ASPEED_CLK_GATE_ECLK>;
> +                               clock-names = "vclk", "eclk";
> +                               interrupts = <7>;
> +                               status = "disabled";
> +                       };
> +
>                         gpio: gpio at 1e780000 {
>                                 #gpio-cells = <2>;
>                                 gpio-controller;
> @@ -1408,6 +1422,54 @@
>         };
>  };
>
> +&vga_scratch {
> +       dac_mux {
> +               offset = <0x2c>;
> +               bit-mask = <0x3>;
> +               bit-shift = <16>;
> +       };
> +       vga0 {
> +               offset = <0x50>;
> +               bit-mask = <0xffffffff>;
> +               bit-shift = <0>;
> +       };
> +       vga1 {
> +               offset = <0x54>;
> +               bit-mask = <0xffffffff>;
> +               bit-shift = <0>;
> +       };
> +       vga2 {
> +               offset = <0x58>;
> +               bit-mask = <0xffffffff>;
> +               bit-shift = <0>;
> +       };
> +       vga3 {
> +               offset = <0x5c>;
> +               bit-mask = <0xffffffff>;
> +               bit-shift = <0>;
> +       };
> +       vga4 {
> +               offset = <0x60>;
> +               bit-mask = <0xffffffff>;
> +               bit-shift = <0>;
> +       };
> +       vga5 {
> +               offset = <0x64>;
> +               bit-mask = <0xffffffff>;
> +               bit-shift = <0>;
> +       };
> +       vga6 {
> +               offset = <0x68>;
> +               bit-mask = <0xffffffff>;
> +               bit-shift = <0>;
> +       };
> +       vga7 {
> +               offset = <0x6c>;
> +               bit-mask = <0xffffffff>;
> +               bit-shift = <0>;
> +       };
> +};
> +
>  &sio_regs {
>         sio_2b {
>                 offset = <0xf0>;
> --
> 2.20.1
>

^ permalink raw reply

* [PATCH 2/2] Docs: hwmon: pmbus: Add PXE1610 driver
From: Vijay Khemka @ 2019-05-29 22:35 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20190529223511.4059120-1-vijaykhemka@fb.com>

Added support for Infenion PXE1610 driver

Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
---
 Documentation/hwmon/pxe1610 | 84 +++++++++++++++++++++++++++++++++++++
 1 file changed, 84 insertions(+)
 create mode 100644 Documentation/hwmon/pxe1610

diff --git a/Documentation/hwmon/pxe1610 b/Documentation/hwmon/pxe1610
new file mode 100644
index 000000000000..b5c83edf027a
--- /dev/null
+++ b/Documentation/hwmon/pxe1610
@@ -0,0 +1,84 @@
+Kernel driver pxe1610
+=====================
+
+Supported chips:
+  * Infinion PXE1610
+    Prefix: 'pxe1610'
+    Addresses scanned: -
+    Datasheet: Datasheet is not publicly available.
+
+  * Infinion PXE1110
+    Prefix: 'pxe1110'
+    Addresses scanned: -
+    Datasheet: Datasheet is not publicly available.
+
+  * Infinion PXM1310
+    Prefix: 'pxm1310'
+    Addresses scanned: -
+    Datasheet: Datasheet is not publicly available.
+
+Author: Vijay Khemka <vijaykhemka@fb.com>
+
+
+Description
+-----------
+
+PXE1610 is a Multi-rail/Multiphase Digital Controllers and
+it is compliant to Intel VR13 DC-DC converter specifications.
+
+
+Usage Notes
+-----------
+
+This driver can be enabled with kernel config CONFIG_SENSORS_PXE1610
+set to 'y' or 'm'(for module).
+
+This driver does not probe for PMBus devices. You will have
+to instantiate devices explicitly.
+
+Example: the following commands will load the driver for an PXE1610
+at address 0x70 on I2C bus #4:
+
+# modprobe pxe1610
+# echo pxe1610 0x70 > /sys/bus/i2c/devices/i2c-4/new_device
+
+It can also be instantiated by declaring in device tree if it is
+built as a kernel not as a module.
+
+
+Sysfs attributes
+----------------
+
+curr1_label		"iin"
+curr1_input		Measured input current
+curr1_alarm		Current high alarm
+
+curr[2-4]_label		"iout[1-3]"
+curr[2-4]_input		Measured output current
+curr[2-4]_crit		Critical maximum current
+curr[2-4]_crit_alarm	Current critical high alarm
+
+in1_label		"vin"
+in1_input		Measured input voltage
+in1_crit		Critical maximum input voltage
+in1_crit_alarm		Input voltage critical high alarm
+
+in[2-4]_label		"vout[1-3]"
+in[2-4]_input		Measured output voltage
+in[2-4]_lcrit		Critical minimum output voltage
+in[2-4]_lcrit_alarm	Output voltage critical low alarm
+in[2-4]_crit		Critical maximum output voltage
+in[2-4]_crit_alarm	Output voltage critical high alarm
+
+power1_label		"pin"
+power1_input		Measured input power
+power1_alarm		Input power high alarm
+
+power[2-4]_label	"pout[1-3]"
+power[2-4]_input	Measured output power
+
+temp[1-3]_input		Measured temperature
+temp[1-3]_crit		Critical high temperature
+temp[1-3]_crit_alarm	Chip temperature critical high alarm
+temp[1-3]_max		Maximum temperature
+temp[1-3]_max_alarm	Chip temperature high alarm
-- 
2.17.1


^ permalink raw reply related

* [PATCH 1/2] hwmon: pmbus: Add Infineon PXE1610 VR driver
From: Vijay Khemka @ 2019-05-29 22:35 UTC (permalink / raw)
  To: linux-aspeed

Added pmbus driver for the new device Infineon pxe1610
voltage regulator. It also supports similar family device
PXE1110 and PXM1310.

Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
---
 drivers/hwmon/pmbus/Kconfig   |   9 +++
 drivers/hwmon/pmbus/Makefile  |   1 +
 drivers/hwmon/pmbus/pxe1610.c | 119 ++++++++++++++++++++++++++++++++++
 3 files changed, 129 insertions(+)
 create mode 100644 drivers/hwmon/pmbus/pxe1610.c

diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
index 30751eb9550a..338ef9b5a395 100644
--- a/drivers/hwmon/pmbus/Kconfig
+++ b/drivers/hwmon/pmbus/Kconfig
@@ -154,6 +154,15 @@ config SENSORS_MAX8688
 	  This driver can also be built as a module. If so, the module will
 	  be called max8688.
 
+config SENSORS_PXE1610
+	tristate "Infineon PXE1610"
+	help
+	  If you say yes here you get hardware monitoring support for Infineon
+	  PXE1610.
+
+	  This driver can also be built as a module. If so, the module will
+	  be called pxe1610.
+
 config SENSORS_TPS40422
 	tristate "TI TPS40422"
 	help
diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
index 2219b9300316..b0fbd017a91a 100644
--- a/drivers/hwmon/pmbus/Makefile
+++ b/drivers/hwmon/pmbus/Makefile
@@ -18,6 +18,7 @@ obj-$(CONFIG_SENSORS_MAX20751)	+= max20751.o
 obj-$(CONFIG_SENSORS_MAX31785)	+= max31785.o
 obj-$(CONFIG_SENSORS_MAX34440)	+= max34440.o
 obj-$(CONFIG_SENSORS_MAX8688)	+= max8688.o
+obj-$(CONFIG_SENSORS_PXE1610)	+= pxe1610.o
 obj-$(CONFIG_SENSORS_TPS40422)	+= tps40422.o
 obj-$(CONFIG_SENSORS_TPS53679)	+= tps53679.o
 obj-$(CONFIG_SENSORS_UCD9000)	+= ucd9000.o
diff --git a/drivers/hwmon/pmbus/pxe1610.c b/drivers/hwmon/pmbus/pxe1610.c
new file mode 100644
index 000000000000..01e267944df5
--- /dev/null
+++ b/drivers/hwmon/pmbus/pxe1610.c
@@ -0,0 +1,119 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Hardware monitoring driver for Infineon PXE1610
+ *
+ * Copyright (c) 2019 Facebook Inc
+ *
+ */
+
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include "pmbus.h"
+
+/*
+ * Identify chip parameters.
+ */
+static int pxe1610_identify(struct i2c_client *client,
+			  struct pmbus_driver_info *info)
+{
+	if (pmbus_check_byte_register(client, 0, PMBUS_VOUT_MODE)) {
+		int vout_mode;
+
+		vout_mode = pmbus_read_byte_data(client, 0, PMBUS_VOUT_MODE);
+		switch (vout_mode & 0x1f) {
+		case 1:
+			info->vrm_version = vr12;
+		break;
+		case 2:
+			info->vrm_version = vr13;
+		break;
+		default:
+			return -ENODEV;
+		}
+	}
+	return 0;
+}
+
+static int pxe1610_probe(struct i2c_client *client,
+			 const struct i2c_device_id *id)
+{
+	struct pmbus_driver_info *info;
+	u8 buf[I2C_SMBUS_BLOCK_MAX];
+	int ret;
+
+	if (!i2c_check_functionality(client->adapter,
+				     I2C_FUNC_SMBUS_READ_BYTE_DATA
+				| I2C_FUNC_SMBUS_READ_WORD_DATA
+				| I2C_FUNC_SMBUS_READ_BLOCK_DATA))
+		return -ENODEV;
+
+	/* By default this device doesn't boot to page 0, so set page 0
+	 * to access all pmbus registers.
+	 */
+	i2c_smbus_write_byte_data(client, 0, 0);
+
+	/* Read Manufacturer id */
+	ret = i2c_smbus_read_block_data(client, PMBUS_MFR_ID, buf);
+	if (ret < 0) {
+		dev_err(&client->dev, "Failed to read PMBUS_MFR_ID\n");
+		return ret;
+	}
+	if (ret != 2 || strncmp(buf, "XP", strlen("XP"))) {
+		dev_err(&client->dev, "MFR_ID unrecognised\n");
+		return -ENODEV;
+	}
+
+	info = devm_kzalloc(&client->dev, sizeof(struct pmbus_driver_info),
+			    GFP_KERNEL);
+	if (!info)
+		return -ENOMEM;
+
+	info->format[PSC_VOLTAGE_IN] = linear;
+	info->format[PSC_VOLTAGE_OUT] = vid;
+	info->format[PSC_CURRENT_IN] = linear;
+	info->format[PSC_CURRENT_OUT] = linear;
+	info->format[PSC_POWER] = linear;
+	info->format[PSC_TEMPERATURE] = linear;
+
+	info->func[0] = PMBUS_HAVE_VIN
+		| PMBUS_HAVE_VOUT | PMBUS_HAVE_IIN
+		| PMBUS_HAVE_IOUT | PMBUS_HAVE_PIN
+		| PMBUS_HAVE_POUT | PMBUS_HAVE_TEMP
+		| PMBUS_HAVE_STATUS_VOUT | PMBUS_HAVE_STATUS_IOUT
+		| PMBUS_HAVE_STATUS_INPUT | PMBUS_HAVE_STATUS_TEMP;
+	info->func[1] = info->func[0];
+	info->func[2] = info->func[0];
+
+	info->pages = id->driver_data;
+	info->identify = pxe1610_identify;
+
+	return pmbus_do_probe(client, id, info);
+}
+
+static const struct i2c_device_id pxe1610_id[] = {
+	{"pxe1610", 3},
+	{"pxe1110", 3},
+	{"pxm1310", 3},
+	{}
+};
+
+MODULE_DEVICE_TABLE(i2c, pxe1610_id);
+
+/* This is the driver that will be inserted */
+static struct i2c_driver pxe1610_driver = {
+	.driver = {
+		   .name = "pxe1610",
+		   },
+	.probe = pxe1610_probe,
+	.remove = pmbus_do_remove,
+	.id_table = pxe1610_id,
+};
+
+module_i2c_driver(pxe1610_driver);
+
+MODULE_AUTHOR("Vijay Khemka <vijaykhemka@fb.com>");
+MODULE_DESCRIPTION("PMBus driver for Infineon PXE1610, PXE1110 and PXM1310");
+MODULE_LICENSE("GPL");
-- 
2.17.1


^ permalink raw reply related

* [PATCH v3 8/8] ARM: dts: aspeed: witherspoon: Enable XDMA Engine
From: Eddie James @ 2019-05-29 18:10 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <1559153408-31190-1-git-send-email-eajames@linux.ibm.com>

Enable the XDMA engine node.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
 arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
index 85b9e40..ad7ccf1 100644
--- a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
@@ -641,4 +641,8 @@
 	status = "okay";
 };
 
+&xdma {
+	status = "okay";
+};
+
 #include "ibm-power9-dual.dtsi"
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH v3 7/8] ARM: dts: aspeed: Add XDMA Engine
From: Eddie James @ 2019-05-29 18:10 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <1559153408-31190-1-git-send-email-eajames@linux.ibm.com>

Add a node for the XDMA engine with all the necessary information.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
 arch/arm/boot/dts/aspeed-g5.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/boot/dts/aspeed-g5.dtsi b/arch/arm/boot/dts/aspeed-g5.dtsi
index 5b1ca26..bfc8328 100644
--- a/arch/arm/boot/dts/aspeed-g5.dtsi
+++ b/arch/arm/boot/dts/aspeed-g5.dtsi
@@ -243,6 +243,14 @@
 				interrupts = <0x19>;
 			};
 
+			xdma: xdma at 1e6e7000 {
+				compatible = "aspeed,ast2500-xdma";
+				reg = <0x1e6e7000 0x100>;
+				resets = <&syscon ASPEED_RESET_XDMA>;
+				interrupts = <6>;
+				status = "disabled";
+			};
+
 			adc: adc at 1e6e9000 {
 				compatible = "aspeed,ast2500-adc";
 				reg = <0x1e6e9000 0xb0>;
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH v3 6/8] drivers/soc: xdma: Add debugfs entries
From: Eddie James @ 2019-05-29 18:10 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <1559153408-31190-1-git-send-email-eajames@linux.ibm.com>

Add debugfs entries for the relevant XDMA engine registers and for
dumping the AST2500 reserved memory space.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
 drivers/soc/aspeed/aspeed-xdma.c | 94 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 94 insertions(+)

diff --git a/drivers/soc/aspeed/aspeed-xdma.c b/drivers/soc/aspeed/aspeed-xdma.c
index ddd5e1e..ea42dbe 100644
--- a/drivers/soc/aspeed/aspeed-xdma.c
+++ b/drivers/soc/aspeed/aspeed-xdma.c
@@ -145,6 +145,12 @@ struct aspeed_xdma {
 
 	char pcidev[4];
 	struct miscdevice misc;
+	struct dentry *debugfs_dir;
+
+#if IS_ENABLED(CONFIG_DEBUG_FS)
+	struct debugfs_regset32 regset;
+	struct debugfs_reg32 regs[XDMA_NUM_DEBUGFS_REGS];
+#endif /* IS_ENABLED(CONFIG_DEBUG_FS) */
 };
 
 struct aspeed_xdma_client {
@@ -603,6 +609,90 @@ static int aspeed_xdma_init_mem(struct aspeed_xdma *ctx, u32 conf)
 	return rc;
 }
 
+static ssize_t aspeed_xdma_debugfs_vga_read(struct file *file,
+					    char __user *buf, size_t len,
+					    loff_t *offset)
+{
+	int rc = -ENOMEM;
+	struct inode *inode = file_inode(file);
+	struct aspeed_xdma *ctx = inode->i_private;
+	loff_t offs = *offset;
+	void *tmp;
+	void __iomem *vga;
+
+	if (len + offs > ctx->vga_size) {
+		if (offs < ctx->vga_size)
+			len = ctx->vga_size - offs;
+		else
+			return 0;
+	}
+
+	vga = ioremap(ctx->vga_phys, ctx->vga_size);
+	if (!vga)
+		return rc;
+
+	tmp = kzalloc(len, GFP_KERNEL);
+	if (!tmp)
+		goto unmap;
+
+	memcpy_fromio(tmp, vga + offs, len);
+
+	rc = copy_to_user(buf, tmp, len);
+	if (rc)
+		goto free;
+
+	*offset = offs + len;
+	rc = len;
+
+free:
+	kfree(tmp);
+
+unmap:
+	iounmap(vga);
+
+	return rc;
+}
+
+static const struct file_operations aspeed_xdma_debugfs_vga_fops = {
+	.owner	= THIS_MODULE,
+	.llseek	= generic_file_llseek,
+	.read	= aspeed_xdma_debugfs_vga_read,
+};
+
+static void aspeed_xdma_init_debugfs(struct aspeed_xdma *ctx)
+{
+	if (!IS_ENABLED(CONFIG_DEBUG_FS))
+		return;
+
+	ctx->debugfs_dir = debugfs_create_dir(DEVICE_NAME, NULL);
+	if (IS_ERR(ctx->debugfs_dir)) {
+		dev_warn(ctx->dev, "Failed to create debugfs directory.\n");
+		return;
+	}
+
+	debugfs_create_file("vga", 0444, ctx->debugfs_dir, ctx,
+			    &aspeed_xdma_debugfs_vga_fops);
+
+	ctx->regs[0].name = "addr";
+	ctx->regs[0].offset = XDMA_BMC_CMD_QUEUE_ADDR;
+	ctx->regs[1].name = "endp";
+	ctx->regs[1].offset = XDMA_BMC_CMD_QUEUE_ENDP;
+	ctx->regs[2].name = "writep";
+	ctx->regs[2].offset = XDMA_BMC_CMD_QUEUE_WRITEP;
+	ctx->regs[3].name = "readp";
+	ctx->regs[3].offset = XDMA_BMC_CMD_QUEUE_READP;
+	ctx->regs[4].name = "control";
+	ctx->regs[4].offset = XDMA_CTRL;
+	ctx->regs[5].name = "status";
+	ctx->regs[5].offset = XDMA_STATUS;
+
+	ctx->regset.regs = ctx->regs;
+	ctx->regset.nregs = XDMA_NUM_DEBUGFS_REGS;
+	ctx->regset.base = ctx->base;
+
+	debugfs_create_regset32("regs", 0444, ctx->debugfs_dir, &ctx->regset);
+}
+
 static int aspeed_xdma_change_pcie_conf(struct aspeed_xdma *ctx, u32 conf)
 {
 	int rc;
@@ -777,6 +867,8 @@ static int aspeed_xdma_probe(struct platform_device *pdev)
 
 	device_create_file(dev, &dev_attr_pcidev);
 
+	aspeed_xdma_init_debugfs(ctx);
+
 	return 0;
 }
 
@@ -784,6 +876,8 @@ static int aspeed_xdma_remove(struct platform_device *pdev)
 {
 	struct aspeed_xdma *ctx = platform_get_drvdata(pdev);
 
+	debugfs_remove_recursive(ctx->debugfs_dir);
+
 	device_remove_file(ctx->dev, &dev_attr_pcidev);
 
 	misc_deregister(&ctx->misc);
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH v3 5/8] drivers/soc: xdma: Add PCI device configuration sysfs
From: Eddie James @ 2019-05-29 18:10 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <1559153408-31190-1-git-send-email-eajames@linux.ibm.com>

The AST2500 has two PCI devices embedded. The XDMA engine can use either
device to perform DMA transfers. Users need the capability to choose
which device to use. This commit therefore adds two sysfs files that
toggle the AST2500 and XDMA engine between the two PCI devices.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
 drivers/soc/aspeed/aspeed-xdma.c | 103 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 100 insertions(+), 3 deletions(-)

diff --git a/drivers/soc/aspeed/aspeed-xdma.c b/drivers/soc/aspeed/aspeed-xdma.c
index 39f6545..ddd5e1e 100644
--- a/drivers/soc/aspeed/aspeed-xdma.c
+++ b/drivers/soc/aspeed/aspeed-xdma.c
@@ -143,6 +143,7 @@ struct aspeed_xdma {
 	void *cmdq_vga_virt;
 	struct gen_pool *vga_pool;
 
+	char pcidev[4];
 	struct miscdevice misc;
 };
 
@@ -165,6 +166,10 @@ struct aspeed_xdma_client {
 	SCU_PCIE_CONF_VGA_EN_IRQ | SCU_PCIE_CONF_VGA_EN_DMA |
 	SCU_PCIE_CONF_RSVD;
 
+static char *_pcidev = "vga";
+module_param_named(pcidev, _pcidev, charp, 0600);
+MODULE_PARM_DESC(pcidev, "Default PCI device used by XDMA engine for DMA ops");
+
 static void aspeed_scu_pcie_write(struct aspeed_xdma *ctx, u32 conf)
 {
 	u32 v = 0;
@@ -512,7 +517,7 @@ static int aspeed_xdma_release(struct inode *inode, struct file *file)
 	.release		= aspeed_xdma_release,
 };
 
-static int aspeed_xdma_init_mem(struct aspeed_xdma *ctx)
+static int aspeed_xdma_init_mem(struct aspeed_xdma *ctx, u32 conf)
 {
 	int rc;
 	u32 scu_conf = 0;
@@ -522,7 +527,7 @@ static int aspeed_xdma_init_mem(struct aspeed_xdma *ctx)
 	const u32 vga_sizes[4] = { 0x800000, 0x1000000, 0x2000000, 0x4000000 };
 	void __iomem *sdmc_base = ioremap(0x1e6e0000, 0x100);
 
-	aspeed_scu_pcie_write(ctx, aspeed_xdma_vga_pcie_conf);
+	aspeed_scu_pcie_write(ctx, conf);
 
 	regmap_read(ctx->scu, SCU_STRAP, &scu_conf);
 	ctx->vga_size = vga_sizes[FIELD_GET(SCU_STRAP_VGA_MEM, scu_conf)];
@@ -598,10 +603,91 @@ static int aspeed_xdma_init_mem(struct aspeed_xdma *ctx)
 	return rc;
 }
 
+static int aspeed_xdma_change_pcie_conf(struct aspeed_xdma *ctx, u32 conf)
+{
+	int rc;
+
+	mutex_lock(&ctx->start_lock);
+	rc = wait_event_interruptible_timeout(ctx->wait,
+					      !test_bit(XDMA_IN_PRG,
+							&ctx->flags),
+					      msecs_to_jiffies(1000));
+	if (rc < 0) {
+		mutex_unlock(&ctx->start_lock);
+		return -EINTR;
+	}
+
+	/* previous op didn't complete, wake up waiters anyway */
+	if (!rc)
+		wake_up_interruptible_all(&ctx->wait);
+
+	reset_control_assert(ctx->reset);
+	msleep(10);
+
+	aspeed_scu_pcie_write(ctx, conf);
+	msleep(10);
+
+	reset_control_deassert(ctx->reset);
+	msleep(10);
+
+	aspeed_xdma_init_eng(ctx);
+
+	mutex_unlock(&ctx->start_lock);
+
+	return 0;
+}
+
+static int aspeed_xdma_pcidev_to_conf(struct aspeed_xdma *ctx,
+				      const char *pcidev, u32 *conf)
+{
+	if (!strcasecmp(pcidev, "vga")) {
+		*conf = aspeed_xdma_vga_pcie_conf;
+		return 0;
+	}
+
+	if (!strcasecmp(pcidev, "bmc")) {
+		*conf = aspeed_xdma_bmc_pcie_conf;
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
+static ssize_t aspeed_xdma_show_pcidev(struct device *dev,
+				       struct device_attribute *attr,
+				       char *buf)
+{
+	struct aspeed_xdma *ctx = dev_get_drvdata(dev);
+
+	return snprintf(buf, PAGE_SIZE - 1, "%s", ctx->pcidev);
+}
+
+static ssize_t aspeed_xdma_store_pcidev(struct device *dev,
+					struct device_attribute *attr,
+					const char *buf, size_t count)
+{
+	u32 conf;
+	struct aspeed_xdma *ctx = dev_get_drvdata(dev);
+	int rc = aspeed_xdma_pcidev_to_conf(ctx, buf, &conf);
+
+	if (rc)
+		return rc;
+
+	rc = aspeed_xdma_change_pcie_conf(ctx, conf);
+	if (rc)
+		return rc;
+
+	strcpy(ctx->pcidev, buf);
+	return count;
+}
+static DEVICE_ATTR(pcidev, 0644, aspeed_xdma_show_pcidev,
+		   aspeed_xdma_store_pcidev);
+
 static int aspeed_xdma_probe(struct platform_device *pdev)
 {
 	int irq;
 	int rc;
+	u32 conf;
 	struct resource *res;
 	struct device *dev = &pdev->dev;
 	struct aspeed_xdma *ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
@@ -657,7 +743,14 @@ static int aspeed_xdma_probe(struct platform_device *pdev)
 
 	msleep(10);
 
-	rc = aspeed_xdma_init_mem(ctx);
+	if (aspeed_xdma_pcidev_to_conf(ctx, _pcidev, &conf)) {
+		conf = aspeed_xdma_vga_pcie_conf;
+		strcpy(ctx->pcidev, "vga");
+	} else {
+		strcpy(ctx->pcidev, _pcidev);
+	}
+
+	rc = aspeed_xdma_init_mem(ctx, conf);
 	if (rc) {
 		reset_control_assert(ctx->reset);
 		return rc;
@@ -682,6 +775,8 @@ static int aspeed_xdma_probe(struct platform_device *pdev)
 		return rc;
 	}
 
+	device_create_file(dev, &dev_attr_pcidev);
+
 	return 0;
 }
 
@@ -689,6 +784,8 @@ static int aspeed_xdma_remove(struct platform_device *pdev)
 {
 	struct aspeed_xdma *ctx = platform_get_drvdata(pdev);
 
+	device_remove_file(ctx->dev, &dev_attr_pcidev);
+
 	misc_deregister(&ctx->misc);
 	gen_pool_free(ctx->vga_pool, (unsigned long)ctx->cmdq_vga_virt,
 		      XDMA_CMDQ_SIZE);
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH v3 4/8] Documentation: ABI: Add aspeed-xdma sysfs documentation
From: Eddie James @ 2019-05-29 18:10 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <1559153408-31190-1-git-send-email-eajames@linux.ibm.com>

Document the pcidev sysfs attribute used by the aspeed-xdma driver.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
 Documentation/ABI/testing/sysfs-devices-platform-aspeed-xdma | 11 +++++++++++
 1 file changed, 11 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-devices-platform-aspeed-xdma

diff --git a/Documentation/ABI/testing/sysfs-devices-platform-aspeed-xdma b/Documentation/ABI/testing/sysfs-devices-platform-aspeed-xdma
new file mode 100644
index 0000000..27189bbd
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-devices-platform-aspeed-xdma
@@ -0,0 +1,11 @@
+What:		/sys/bus/platform/devices/1e6e7000.xdma/pcidev
+Date:		May 2019
+KernelVersion:	5.2
+Contact:	Eddie James <eajames@linux.ibm.com>
+Description:	When written, this file sets the PCI device that will be used
+		for DMA operations by the XDMA engine. When read, it provides
+		the current PCI device being used by the XDMA engine.
+		Valid values: vga or bmc.
+		Default vga, can be set by aspeed-xdma.pcidev= module
+		parameter.
+Users:		aspeed-xdma driver
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH v3 3/8] drivers/soc: xdma: Add user interface
From: Eddie James @ 2019-05-29 18:10 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <1559153408-31190-1-git-send-email-eajames@linux.ibm.com>

This commits adds a miscdevice to provide a user interface to the XDMA
engine. The interface provides the write operation to start DMA
operations. The DMA parameters are passed as the data to the write call.
The actual data to transfer is NOT passed through write. Note that both
directions of DMA operation are accomplished through the write command;
BMC to host and host to BMC.

The XDMA engine is restricted to only accessing the reserved memory
space on the AST2500, typically used by the VGA. For this reason, the
VGA memory space is pooled and allocated with genalloc. Users calling
mmap allocate pages from this pool for their usage. The space allocated
by a client will be the space used in the DMA operation. For an
"upstream" (BMC to host) operation, the data in the client's area will
be transferred to the host. For a "downstream" (host to BMC) operation,
the host data will be placed in the client's memory area.

Poll is also provided in order to determine when the DMA operation is
complete for non-blocking IO.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
 drivers/soc/aspeed/aspeed-xdma.c | 201 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 201 insertions(+)

diff --git a/drivers/soc/aspeed/aspeed-xdma.c b/drivers/soc/aspeed/aspeed-xdma.c
index 3dc0ce4..39f6545 100644
--- a/drivers/soc/aspeed/aspeed-xdma.c
+++ b/drivers/soc/aspeed/aspeed-xdma.c
@@ -129,6 +129,8 @@ struct aspeed_xdma {
 
 	unsigned long flags;
 	unsigned int cmd_idx;
+	struct mutex list_lock;
+	struct mutex start_lock;
 	wait_queue_head_t wait;
 	struct aspeed_xdma_client *current_client;
 
@@ -140,6 +142,8 @@ struct aspeed_xdma {
 	dma_addr_t cmdq_vga_phys;
 	void *cmdq_vga_virt;
 	struct gen_pool *vga_pool;
+
+	struct miscdevice misc;
 };
 
 struct aspeed_xdma_client {
@@ -331,6 +335,183 @@ static irqreturn_t aspeed_xdma_irq(int irq, void *arg)
 	return IRQ_HANDLED;
 }
 
+static ssize_t aspeed_xdma_write(struct file *file, const char __user *buf,
+				 size_t len, loff_t *offset)
+{
+	int rc;
+	struct aspeed_xdma_op op;
+	struct aspeed_xdma_client *client = file->private_data;
+	struct aspeed_xdma *ctx = client->ctx;
+	u32 offs = client->phys ? (client->phys - ctx->vga_phys) :
+		XDMA_CMDQ_SIZE;
+
+	if (len != sizeof(struct aspeed_xdma_op))
+		return -EINVAL;
+
+	rc = copy_from_user(&op, buf, len);
+	if (rc)
+		return rc;
+
+	if (op.len > (ctx->vga_size - offs) || op.len < XDMA_BYTE_ALIGN)
+		return -EINVAL;
+
+	if (file->f_flags & O_NONBLOCK) {
+		if (!mutex_trylock(&ctx->start_lock))
+			return -EAGAIN;
+
+		if (test_bit(XDMA_IN_PRG, &ctx->flags)) {
+			mutex_unlock(&ctx->start_lock);
+			return -EAGAIN;
+		}
+	} else {
+		mutex_lock(&ctx->start_lock);
+
+		rc = wait_event_interruptible(ctx->wait,
+					      !test_bit(XDMA_IN_PRG,
+							&ctx->flags));
+		if (rc) {
+			mutex_unlock(&ctx->start_lock);
+			return -EINTR;
+		}
+	}
+
+	ctx->current_client = client;
+	set_bit(XDMA_IN_PRG, &client->flags);
+
+	aspeed_xdma_start(ctx, &op, ctx->vga_phys + offs);
+
+	mutex_unlock(&ctx->start_lock);
+
+	if (!(file->f_flags & O_NONBLOCK)) {
+		rc = wait_event_interruptible(ctx->wait,
+					      !test_bit(XDMA_IN_PRG,
+							&ctx->flags));
+		if (rc)
+			return -EINTR;
+	}
+
+	return len;
+}
+
+static __poll_t aspeed_xdma_poll(struct file *file,
+				 struct poll_table_struct *wait)
+{
+	__poll_t mask = 0;
+	__poll_t req = poll_requested_events(wait);
+	struct aspeed_xdma_client *client = file->private_data;
+	struct aspeed_xdma *ctx = client->ctx;
+
+	if (req & (EPOLLIN | EPOLLRDNORM)) {
+		if (test_bit(XDMA_IN_PRG, &client->flags))
+			poll_wait(file, &ctx->wait, wait);
+
+		if (!test_bit(XDMA_IN_PRG, &client->flags))
+			mask |= EPOLLIN | EPOLLRDNORM;
+	}
+
+	if (req & (EPOLLOUT | EPOLLWRNORM)) {
+		if (test_bit(XDMA_IN_PRG, &ctx->flags))
+			poll_wait(file, &ctx->wait, wait);
+
+		if (!test_bit(XDMA_IN_PRG, &ctx->flags))
+			mask |= EPOLLOUT | EPOLLWRNORM;
+	}
+
+	return mask;
+}
+
+static void aspeed_xdma_vma_close(struct vm_area_struct *vma)
+{
+	struct aspeed_xdma_client *client = vma->vm_private_data;
+
+	gen_pool_free(client->ctx->vga_pool, (unsigned long)client->virt,
+		      client->size);
+
+	client->virt = NULL;
+	client->phys = 0;
+	client->size = 0;
+}
+
+static const struct vm_operations_struct aspeed_xdma_vm_ops = {
+	.close =	aspeed_xdma_vma_close,
+};
+
+static int aspeed_xdma_mmap(struct file *file, struct vm_area_struct *vma)
+{
+	int rc;
+	struct aspeed_xdma_client *client = file->private_data;
+	struct aspeed_xdma *ctx = client->ctx;
+
+	/* restrict file to one mapping */
+	if (client->size)
+		return -ENOMEM;
+
+	client->size = vma->vm_end - vma->vm_start;
+	client->virt = gen_pool_dma_alloc(ctx->vga_pool, client->size,
+					  &client->phys);
+	if (!client->virt) {
+		client->phys = 0;
+		client->size = 0;
+		return -ENOMEM;
+	}
+
+	vma->vm_pgoff = (client->phys - ctx->vga_phys) >> PAGE_SHIFT;
+	vma->vm_ops = &aspeed_xdma_vm_ops;
+	vma->vm_private_data = client;
+
+	rc = dma_mmap_coherent(ctx->dev, vma, ctx->vga_virt, ctx->vga_dma,
+			       ctx->vga_size);
+	if (rc) {
+		gen_pool_free(ctx->vga_pool, (unsigned long)client->virt,
+			      client->size);
+
+		client->virt = NULL;
+		client->phys = 0;
+		client->size = 0;
+		return rc;
+	}
+
+	dev_dbg(ctx->dev, "mmap: v[%08lx] to p[%08x], s[%08x]\n",
+		vma->vm_start, (u32)client->phys, client->size);
+
+	return 0;
+}
+
+static int aspeed_xdma_open(struct inode *inode, struct file *file)
+{
+	struct miscdevice *misc = file->private_data;
+	struct aspeed_xdma *ctx = container_of(misc, struct aspeed_xdma, misc);
+	struct aspeed_xdma_client *client = kzalloc(sizeof(*client),
+						    GFP_KERNEL);
+
+	if (!client)
+		return -ENOMEM;
+
+	client->ctx = ctx;
+	file->private_data = client;
+	return 0;
+}
+
+static int aspeed_xdma_release(struct inode *inode, struct file *file)
+{
+	struct aspeed_xdma_client *client = file->private_data;
+
+	if (client->ctx->current_client == client)
+		client->ctx->current_client = NULL;
+
+	kfree(client);
+	return 0;
+}
+
+static const struct file_operations aspeed_xdma_fops = {
+	.owner			= THIS_MODULE,
+	.write			= aspeed_xdma_write,
+	.poll			= aspeed_xdma_poll,
+	.mmap			= aspeed_xdma_mmap,
+	.open			= aspeed_xdma_open,
+	.release		= aspeed_xdma_release,
+};
+
 static int aspeed_xdma_init_mem(struct aspeed_xdma *ctx)
 {
 	int rc;
@@ -431,6 +612,8 @@ static int aspeed_xdma_probe(struct platform_device *pdev)
 	ctx->dev = dev;
 	platform_set_drvdata(pdev, ctx);
 	init_waitqueue_head(&ctx->wait);
+	mutex_init(&ctx->list_lock);
+	mutex_init(&ctx->start_lock);
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	ctx->base = devm_ioremap_resource(dev, res);
@@ -482,6 +665,23 @@ static int aspeed_xdma_probe(struct platform_device *pdev)
 
 	aspeed_xdma_init_eng(ctx);
 
+	ctx->misc.minor = MISC_DYNAMIC_MINOR;
+	ctx->misc.fops = &aspeed_xdma_fops;
+	ctx->misc.name = "xdma";
+	ctx->misc.parent = dev;
+	rc = misc_register(&ctx->misc);
+	if (rc) {
+		dev_err(dev, "Unable to register xdma miscdevice.\n");
+
+		gen_pool_free(ctx->vga_pool, (unsigned long)ctx->cmdq_vga_virt,
+			      XDMA_CMDQ_SIZE);
+		dma_free_coherent(dev, ctx->vga_size, ctx->vga_virt,
+				  ctx->vga_dma);
+		dma_release_declared_memory(dev);
+		reset_control_assert(ctx->reset);
+		return rc;
+	}
+
 	return 0;
 }
 
@@ -489,6 +689,7 @@ static int aspeed_xdma_remove(struct platform_device *pdev)
 {
 	struct aspeed_xdma *ctx = platform_get_drvdata(pdev);
 
+	misc_deregister(&ctx->misc);
 	gen_pool_free(ctx->vga_pool, (unsigned long)ctx->cmdq_vga_virt,
 		      XDMA_CMDQ_SIZE);
 	dma_free_coherent(ctx->dev, ctx->vga_size, ctx->vga_virt,
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH v3 2/8] drivers/soc: Add Aspeed XDMA Engine Driver
From: Eddie James @ 2019-05-29 18:10 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <1559153408-31190-1-git-send-email-eajames@linux.ibm.com>

The XDMA engine embedded in the AST2500 SOC performs PCI DMA operations
between the SOC (acting as a BMC) and a host processor in a server.

This commit adds a driver to control the XDMA engine and adds functions
to initialize the hardware and memory and start DMA operations.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
 MAINTAINERS                      |  10 +
 drivers/soc/aspeed/Kconfig       |   8 +
 drivers/soc/aspeed/Makefile      |   1 +
 drivers/soc/aspeed/aspeed-xdma.c | 520 +++++++++++++++++++++++++++++++++++++++
 include/uapi/linux/aspeed-xdma.h |  26 ++
 5 files changed, 565 insertions(+)
 create mode 100644 drivers/soc/aspeed/aspeed-xdma.c
 create mode 100644 include/uapi/linux/aspeed-xdma.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 7e09dda..84e2b62 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2584,6 +2584,16 @@ S:	Maintained
 F:	drivers/media/platform/aspeed-video.c
 F:	Documentation/devicetree/bindings/media/aspeed-video.txt
 
+ASPEED XDMA ENGINE DRIVER
+M:	Eddie James <eajames@linux.ibm.com>
+L:	linux-aspeed at lists.ozlabs.org (moderated for non-subscribers)
+L:	linux-kernel at vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/misc/aspeed,xdma.txt
+F:	Documentation/ABI/testing/sysfs-devices-platform-aspeed-xdma
+F:	drivers/soc/aspeed/aspeed-xdma.c
+F:	include/uapi/linux/aspeed-xdma.h
+
 ASUS NOTEBOOKS AND EEEPC ACPI/WMI EXTRAS DRIVERS
 M:	Corentin Chary <corentin.chary@gmail.com>
 L:	acpi4asus-user at lists.sourceforge.net
diff --git a/drivers/soc/aspeed/Kconfig b/drivers/soc/aspeed/Kconfig
index 323e177..8b08310 100644
--- a/drivers/soc/aspeed/Kconfig
+++ b/drivers/soc/aspeed/Kconfig
@@ -29,4 +29,12 @@ config ASPEED_P2A_CTRL
 	  ioctl()s, the driver also provides an interface for userspace mappings to
 	  a pre-defined region.
 
+config ASPEED_XDMA
+	tristate "Aspeed XDMA Engine Driver"
+	depends on SOC_ASPEED && REGMAP && MFD_SYSCON && HAS_DMA
+	help
+	  Enable support for the Aspeed XDMA Engine found on the Aspeed AST2500
+	  SOC. The XDMA engine can perform automatic PCI DMA operations between
+	  the AST2500 (acting as a BMC) and a host processor.
+
 endmenu
diff --git a/drivers/soc/aspeed/Makefile b/drivers/soc/aspeed/Makefile
index b64be47..977b046 100644
--- a/drivers/soc/aspeed/Makefile
+++ b/drivers/soc/aspeed/Makefile
@@ -2,3 +2,4 @@
 obj-$(CONFIG_ASPEED_LPC_CTRL)	+= aspeed-lpc-ctrl.o
 obj-$(CONFIG_ASPEED_LPC_SNOOP)	+= aspeed-lpc-snoop.o
 obj-$(CONFIG_ASPEED_P2A_CTRL)	+= aspeed-p2a-ctrl.o
+obj-$(CONFIG_ASPEED_XDMA)	+= aspeed-xdma.o
diff --git a/drivers/soc/aspeed/aspeed-xdma.c b/drivers/soc/aspeed/aspeed-xdma.c
new file mode 100644
index 0000000..3dc0ce4
--- /dev/null
+++ b/drivers/soc/aspeed/aspeed-xdma.c
@@ -0,0 +1,520 @@
+// SPDX-License-Identifier: GPL-2.0+
+// Copyright IBM Corp 2019
+
+#include <linux/aspeed-xdma.h>
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/debugfs.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/dma-mapping.h>
+#include <linux/fs.h>
+#include <linux/genalloc.h>
+#include <linux/interrupt.h>
+#include <linux/jiffies.h>
+#include <linux/list.h>
+#include <linux/mfd/syscon.h>
+#include <linux/miscdevice.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
+#include <linux/of_reserved_mem.h>
+#include <linux/platform_device.h>
+#include <linux/poll.h>
+#include <linux/regmap.h>
+#include <linux/reset.h>
+#include <linux/string.h>
+#include <linux/uaccess.h>
+#include <linux/wait.h>
+
+#define DEVICE_NAME			"aspeed-xdma"
+
+#define SCU_STRAP			0x070
+#define  SCU_STRAP_VGA_MEM		GENMASK(3, 2)
+
+#define SCU_PCIE_CONF			0x180
+#define  SCU_PCIE_CONF_VGA_EN		BIT(0)
+#define  SCU_PCIE_CONF_VGA_EN_MMIO	BIT(1)
+#define  SCU_PCIE_CONF_VGA_EN_LPC	BIT(2)
+#define  SCU_PCIE_CONF_VGA_EN_MSI	BIT(3)
+#define  SCU_PCIE_CONF_VGA_EN_MCTP	BIT(4)
+#define  SCU_PCIE_CONF_VGA_EN_IRQ	BIT(5)
+#define  SCU_PCIE_CONF_VGA_EN_DMA	BIT(6)
+#define  SCU_PCIE_CONF_BMC_EN		BIT(8)
+#define  SCU_PCIE_CONF_BMC_EN_MMIO	BIT(9)
+#define  SCU_PCIE_CONF_BMC_EN_MSI	BIT(11)
+#define  SCU_PCIE_CONF_BMC_EN_MCTP	BIT(12)
+#define  SCU_PCIE_CONF_BMC_EN_IRQ	BIT(13)
+#define  SCU_PCIE_CONF_BMC_EN_DMA	BIT(14)
+#define  SCU_PCIE_CONF_RSVD		GENMASK(19, 18)
+
+#define SDMC_CONF			0x004
+#define  SDMC_CONF_MEM			GENMASK(1, 0)
+#define SDMC_REMAP			0x008
+#define  SDMC_REMAP_MAGIC		GENMASK(17, 16)
+
+#define XDMA_CMD_SIZE			4
+#define XDMA_CMDQ_SIZE			PAGE_SIZE
+#define XDMA_BYTE_ALIGN			16
+#define XDMA_MAX_LINE_SIZE		BIT(10)
+#define XDMA_NUM_CMDS			\
+	(XDMA_CMDQ_SIZE / sizeof(struct aspeed_xdma_cmd))
+#define XDMA_NUM_DEBUGFS_REGS		6
+
+#define XDMA_CMD_BMC_CHECK		BIT(0)
+#define XDMA_CMD_BMC_ADDR		GENMASK(29, 4)
+#define XDMA_CMD_BMC_DIR_US		BIT(31)
+
+#define XDMA_CMD_COMM1_HI_HOST_PITCH	GENMASK(14, 3)
+#define XDMA_CMD_COMM1_HI_BMC_PITCH	GENMASK(30, 19)
+
+#define XDMA_CMD_CONF_CHECK		BIT(1)
+#define XDMA_CMD_CONF_LINE_SIZE		GENMASK(14, 4)
+#define XDMA_CMD_CONF_IRQ_BMC		BIT(15)
+#define XDMA_CMD_CONF_NUM_LINES		GENMASK(27, 16)
+#define XDMA_CMD_CONF_IRQ		BIT(31)
+
+#define XDMA_CMD_ID_UPDIR		GENMASK(17, 16)
+#define  XDMA_CMD_ID_UPDIR_BMC		0
+#define  XDMA_CMD_ID_UPDIR_HOST		1
+#define  XDMA_CMD_ID_UPDIR_VGA		2
+
+#define XDMA_DS_PCIE_REQ_SIZE_128	0
+#define XDMA_DS_PCIE_REQ_SIZE_256	1
+#define XDMA_DS_PCIE_REQ_SIZE_512	2
+#define XDMA_DS_PCIE_REQ_SIZE_1K	3
+#define XDMA_DS_PCIE_REQ_SIZE_2K	4
+#define XDMA_DS_PCIE_REQ_SIZE_4K	5
+
+#define XDMA_BMC_CMD_QUEUE_ADDR		0x10
+#define XDMA_BMC_CMD_QUEUE_ENDP		0x14
+#define XDMA_BMC_CMD_QUEUE_WRITEP	0x18
+#define XDMA_BMC_CMD_QUEUE_READP	0x1c
+#define  XDMA_BMC_CMD_QUEUE_READP_MAGIC	0xee882266
+#define XDMA_CTRL			0x20
+#define  XDMA_CTRL_US_COMP		BIT(4)
+#define  XDMA_CTRL_DS_COMP		BIT(5)
+#define  XDMA_CTRL_DS_DIRTY		BIT(6)
+#define  XDMA_CTRL_DS_PCIE_REQ_SIZE	GENMASK(19, 17)
+#define  XDMA_CTRL_DS_DATA_TIMEOUT	BIT(28)
+#define  XDMA_CTRL_DS_CHECK_ID		BIT(29)
+#define XDMA_STATUS			0x24
+#define  XDMA_STATUS_US_COMP		BIT(4)
+#define  XDMA_STATUS_DS_COMP		BIT(5)
+
+enum {
+	XDMA_IN_PRG,
+	XDMA_UPSTREAM,
+};
+
+struct aspeed_xdma_cmd {
+	u32 host_addr_lo;
+	u32 host_addr_hi;
+	u32 bmc_addr;
+	u32 comm1_hi;
+	u32 conf;
+	u32 id;
+	u32 resv0;
+	u32 resv1;
+};
+
+struct aspeed_xdma_client;
+
+struct aspeed_xdma {
+	struct device *dev;
+	void __iomem *base;
+	struct regmap *scu;
+	struct reset_control *reset;
+
+	unsigned long flags;
+	unsigned int cmd_idx;
+	wait_queue_head_t wait;
+	struct aspeed_xdma_client *current_client;
+
+	u32 vga_phys;
+	u32 vga_size;
+	dma_addr_t vga_dma;
+	void *cmdq;
+	void *vga_virt;
+	dma_addr_t cmdq_vga_phys;
+	void *cmdq_vga_virt;
+	struct gen_pool *vga_pool;
+};
+
+struct aspeed_xdma_client {
+	struct aspeed_xdma *ctx;
+
+	unsigned long flags;
+	void *virt;
+	dma_addr_t phys;
+	u32 size;
+};
+
+static const u32 aspeed_xdma_bmc_pcie_conf = SCU_PCIE_CONF_BMC_EN |
+	SCU_PCIE_CONF_BMC_EN_MSI | SCU_PCIE_CONF_BMC_EN_MCTP |
+	SCU_PCIE_CONF_BMC_EN_IRQ | SCU_PCIE_CONF_BMC_EN_DMA |
+	SCU_PCIE_CONF_RSVD;
+
+static const u32 aspeed_xdma_vga_pcie_conf = SCU_PCIE_CONF_VGA_EN |
+	SCU_PCIE_CONF_VGA_EN_MSI | SCU_PCIE_CONF_VGA_EN_MCTP |
+	SCU_PCIE_CONF_VGA_EN_IRQ | SCU_PCIE_CONF_VGA_EN_DMA |
+	SCU_PCIE_CONF_RSVD;
+
+static void aspeed_scu_pcie_write(struct aspeed_xdma *ctx, u32 conf)
+{
+	u32 v = 0;
+
+	regmap_write(ctx->scu, SCU_PCIE_CONF, conf);
+	regmap_read(ctx->scu, SCU_PCIE_CONF, &v);
+
+	dev_dbg(ctx->dev, "write scu pcie_conf[%08x]\n", v);
+}
+
+static u32 aspeed_xdma_reg_read(struct aspeed_xdma *ctx, u32 reg)
+{
+	u32 v = readl(ctx->base + reg);
+
+	dev_dbg(ctx->dev, "read %02x[%08x]\n", reg, v);
+	return v;
+}
+
+static void aspeed_xdma_reg_write(struct aspeed_xdma *ctx, u32 reg, u32 val)
+{
+	writel(val, ctx->base + reg);
+	dev_dbg(ctx->dev, "write %02x[%08x]\n", reg, readl(ctx->base + reg));
+}
+
+static void aspeed_xdma_init_eng(struct aspeed_xdma *ctx)
+{
+	const u32 ctrl = XDMA_CTRL_US_COMP | XDMA_CTRL_DS_COMP |
+		XDMA_CTRL_DS_DIRTY | FIELD_PREP(XDMA_CTRL_DS_PCIE_REQ_SIZE,
+						XDMA_DS_PCIE_REQ_SIZE_256) |
+		XDMA_CTRL_DS_DATA_TIMEOUT | XDMA_CTRL_DS_CHECK_ID;
+
+	aspeed_xdma_reg_write(ctx, XDMA_BMC_CMD_QUEUE_ENDP,
+			      XDMA_CMD_SIZE * XDMA_NUM_CMDS);
+	aspeed_xdma_reg_write(ctx, XDMA_BMC_CMD_QUEUE_READP,
+			      XDMA_BMC_CMD_QUEUE_READP_MAGIC);
+	aspeed_xdma_reg_write(ctx, XDMA_BMC_CMD_QUEUE_WRITEP, 0);
+	aspeed_xdma_reg_write(ctx, XDMA_CTRL, ctrl);
+
+	aspeed_xdma_reg_write(ctx, XDMA_BMC_CMD_QUEUE_ADDR,
+			      ctx->cmdq_vga_phys);
+
+	ctx->cmd_idx = 0;
+	ctx->flags = 0;
+}
+
+static void aspeed_xdma_reset(struct aspeed_xdma *ctx)
+{
+	reset_control_assert(ctx->reset);
+
+	msleep(10);
+
+	reset_control_deassert(ctx->reset);
+
+	msleep(10);
+
+	aspeed_xdma_init_eng(ctx);
+}
+
+static void aspeed_xdma_start(struct aspeed_xdma *ctx,
+			      struct aspeed_xdma_op *op, u32 bmc_addr)
+{
+	u32 conf = XDMA_CMD_CONF_CHECK | XDMA_CMD_CONF_IRQ_BMC |
+		XDMA_CMD_CONF_IRQ;
+	unsigned int line_size = op->len / XDMA_BYTE_ALIGN;
+	unsigned int num_lines = 1;
+	unsigned int nidx = (ctx->cmd_idx + 1) % XDMA_NUM_CMDS;
+	unsigned int pitch = 1;
+	struct aspeed_xdma_cmd *cmd =
+		&(((struct aspeed_xdma_cmd *)ctx->cmdq)[ctx->cmd_idx]);
+
+	if (line_size > XDMA_MAX_LINE_SIZE) {
+		unsigned int rem;
+		unsigned int total;
+
+		num_lines = line_size / XDMA_MAX_LINE_SIZE;
+		total = XDMA_MAX_LINE_SIZE * num_lines;
+		rem = line_size - total;
+		line_size = XDMA_MAX_LINE_SIZE;
+		pitch = line_size;
+
+		if (rem) {
+			unsigned int offs = total * XDMA_BYTE_ALIGN;
+			u32 r_bmc_addr = bmc_addr + offs;
+			u64 r_host_addr = op->host_addr + (u64)offs;
+			struct aspeed_xdma_cmd *r_cmd =
+				&(((struct aspeed_xdma_cmd *)ctx->cmdq)[nidx]);
+
+			r_cmd->host_addr_lo =
+				(u32)(r_host_addr & 0xFFFFFFFFULL);
+			r_cmd->host_addr_hi = (u32)(r_host_addr >> 32ULL);
+			r_cmd->bmc_addr = (r_bmc_addr & XDMA_CMD_BMC_ADDR) |
+				XDMA_CMD_BMC_CHECK |
+				(op->upstream ? XDMA_CMD_BMC_DIR_US : 0);
+			r_cmd->conf = conf |
+				FIELD_PREP(XDMA_CMD_CONF_LINE_SIZE, rem) |
+				FIELD_PREP(XDMA_CMD_CONF_NUM_LINES, 1);
+			r_cmd->comm1_hi =
+				FIELD_PREP(XDMA_CMD_COMM1_HI_HOST_PITCH, 1) |
+				FIELD_PREP(XDMA_CMD_COMM1_HI_BMC_PITCH, 1);
+
+			/* do not trigger IRQ for first command */
+			conf = XDMA_CMD_CONF_CHECK;
+
+			nidx = (nidx + 1) % XDMA_NUM_CMDS;
+		}
+
+		/* undocumented formula to get required number of lines */
+		num_lines = (num_lines * 2) - 1;
+	}
+
+	/* ctrl == 0 indicates engine hasn't started properly; restart it */
+	if (!aspeed_xdma_reg_read(ctx, XDMA_CTRL))
+		aspeed_xdma_reset(ctx);
+
+	cmd->host_addr_lo = (u32)(op->host_addr & 0xFFFFFFFFULL);
+	cmd->host_addr_hi = (u32)(op->host_addr >> 32ULL);
+	cmd->bmc_addr = (bmc_addr & XDMA_CMD_BMC_ADDR) | XDMA_CMD_BMC_CHECK |
+		(op->upstream ? XDMA_CMD_BMC_DIR_US : 0);
+	cmd->conf = conf |
+		FIELD_PREP(XDMA_CMD_CONF_LINE_SIZE, line_size) |
+		FIELD_PREP(XDMA_CMD_CONF_NUM_LINES, num_lines);
+	cmd->comm1_hi = FIELD_PREP(XDMA_CMD_COMM1_HI_HOST_PITCH, pitch) |
+			FIELD_PREP(XDMA_CMD_COMM1_HI_BMC_PITCH, pitch);
+
+	memcpy(ctx->cmdq_vga_virt, ctx->cmdq, XDMA_CMDQ_SIZE);
+
+	if (op->upstream)
+		set_bit(XDMA_UPSTREAM, &ctx->flags);
+	else
+		clear_bit(XDMA_UPSTREAM, &ctx->flags);
+
+	set_bit(XDMA_IN_PRG, &ctx->flags);
+
+	aspeed_xdma_reg_write(ctx, XDMA_BMC_CMD_QUEUE_WRITEP,
+			      nidx * XDMA_CMD_SIZE);
+	ctx->cmd_idx = nidx;
+}
+
+static void aspeed_xdma_done(struct aspeed_xdma *ctx)
+{
+	if (ctx->current_client) {
+		clear_bit(XDMA_IN_PRG, &ctx->current_client->flags);
+
+		ctx->current_client = NULL;
+	}
+
+	clear_bit(XDMA_IN_PRG, &ctx->flags);
+	wake_up_interruptible_all(&ctx->wait);
+}
+
+static irqreturn_t aspeed_xdma_irq(int irq, void *arg)
+{
+	struct aspeed_xdma *ctx = arg;
+	u32 status = aspeed_xdma_reg_read(ctx, XDMA_STATUS);
+
+	if (status & XDMA_STATUS_US_COMP) {
+		if (test_bit(XDMA_UPSTREAM, &ctx->flags))
+			aspeed_xdma_done(ctx);
+	}
+
+	if (status & XDMA_STATUS_DS_COMP) {
+		if (!test_bit(XDMA_UPSTREAM, &ctx->flags))
+			aspeed_xdma_done(ctx);
+	}
+
+	aspeed_xdma_reg_write(ctx, XDMA_STATUS, status);
+
+	return IRQ_HANDLED;
+}
+
+static int aspeed_xdma_init_mem(struct aspeed_xdma *ctx)
+{
+	int rc;
+	u32 scu_conf = 0;
+	u32 mem_size = 0x20000000;
+	const u32 mem_sizes[4] = { 0x8000000, 0x10000000, 0x20000000,
+				   0x40000000 };
+	const u32 vga_sizes[4] = { 0x800000, 0x1000000, 0x2000000, 0x4000000 };
+	void __iomem *sdmc_base = ioremap(0x1e6e0000, 0x100);
+
+	aspeed_scu_pcie_write(ctx, aspeed_xdma_vga_pcie_conf);
+
+	regmap_read(ctx->scu, SCU_STRAP, &scu_conf);
+	ctx->vga_size = vga_sizes[FIELD_GET(SCU_STRAP_VGA_MEM, scu_conf)];
+
+	if (sdmc_base) {
+		u32 sdmc = readl(sdmc_base + SDMC_CONF);
+		u32 remap = readl(sdmc_base + SDMC_REMAP);
+
+		remap |= SDMC_REMAP_MAGIC;
+		writel(remap, sdmc_base + SDMC_REMAP);
+		remap = readl(sdmc_base + SDMC_REMAP);
+
+		mem_size = mem_sizes[sdmc & SDMC_CONF_MEM];
+		iounmap(sdmc_base);
+	}
+
+	ctx->vga_phys = (mem_size - ctx->vga_size) + 0x80000000;
+
+	ctx->cmdq = devm_kzalloc(ctx->dev, XDMA_CMDQ_SIZE, GFP_KERNEL);
+	if (!ctx->cmdq) {
+		dev_err(ctx->dev, "Failed to allocate command queue.\n");
+		return -ENOMEM;
+	}
+
+	rc = dma_set_mask_and_coherent(ctx->dev, DMA_BIT_MASK(32));
+	if (rc) {
+		dev_err(ctx->dev, "Failed to set DMA mask: %d.\n", rc);
+		return rc;
+	}
+
+	rc = dma_declare_coherent_memory(ctx->dev, ctx->vga_phys,
+					 ctx->vga_phys, ctx->vga_size);
+	if (rc) {
+		dev_err(ctx->dev, "Failed to declare coherent memory: %d.\n",
+			rc);
+		return rc;
+	}
+
+	ctx->vga_virt = dma_alloc_coherent(ctx->dev, ctx->vga_size,
+					   &ctx->vga_dma, GFP_KERNEL);
+	if (!ctx->vga_virt) {
+		dev_err(ctx->dev, "Failed to allocate DMA.\n");
+		rc = -ENOMEM;
+		goto err_dma;
+	}
+
+	rc = gen_pool_add_virt(ctx->vga_pool, (unsigned long)ctx->vga_virt,
+			       ctx->vga_phys, ctx->vga_size, -1);
+	if (rc) {
+		dev_err(ctx->dev, "Failed to add memory to genalloc pool.\n");
+		goto err_genalloc;
+	}
+
+	ctx->cmdq_vga_virt = gen_pool_dma_alloc(ctx->vga_pool, XDMA_CMDQ_SIZE,
+						&ctx->cmdq_vga_phys);
+	if (!ctx->cmdq_vga_virt) {
+		dev_err(ctx->dev, "Failed to genalloc cmdq.\n");
+		rc = -ENOMEM;
+		goto err_genalloc;
+	}
+
+	dev_dbg(ctx->dev, "VGA mapped at phys[%08x], size[%08x].\n",
+		ctx->vga_phys, ctx->vga_size);
+
+	return 0;
+
+err_dma:
+	dma_release_declared_memory(ctx->dev);
+
+err_genalloc:
+	dma_free_coherent(ctx->dev, ctx->vga_size, ctx->vga_virt,
+			  ctx->vga_dma);
+	return rc;
+}
+
+static int aspeed_xdma_probe(struct platform_device *pdev)
+{
+	int irq;
+	int rc;
+	struct resource *res;
+	struct device *dev = &pdev->dev;
+	struct aspeed_xdma *ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
+
+	if (!ctx)
+		return -ENOMEM;
+
+	ctx->dev = dev;
+	platform_set_drvdata(pdev, ctx);
+	init_waitqueue_head(&ctx->wait);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	ctx->base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(ctx->base)) {
+		dev_err(dev, "Unable to ioremap registers.\n");
+		return PTR_ERR(ctx->base);
+	}
+
+	irq = irq_of_parse_and_map(dev->of_node, 0);
+	if (!irq) {
+		dev_err(dev, "Unable to find IRQ.\n");
+		return -ENODEV;
+	}
+
+	rc = devm_request_irq(dev, irq, aspeed_xdma_irq, IRQF_SHARED,
+			      DEVICE_NAME, ctx);
+	if (rc < 0) {
+		dev_err(dev, "Unable to request IRQ %d.\n", irq);
+		return rc;
+	}
+
+	ctx->scu = syscon_regmap_lookup_by_compatible("aspeed,ast2500-scu");
+	if (IS_ERR(ctx->scu)) {
+		dev_err(ctx->dev, "Unable to grab SCU regs.\n");
+		return PTR_ERR(ctx->scu);
+	}
+
+	ctx->reset = devm_reset_control_get_exclusive(dev, NULL);
+	if (IS_ERR(ctx->reset)) {
+		dev_err(dev, "Unable to request reset control.\n");
+		return PTR_ERR(ctx->reset);
+	}
+
+	ctx->vga_pool = devm_gen_pool_create(dev, ilog2(PAGE_SIZE), -1, NULL);
+	if (!ctx->vga_pool) {
+		dev_err(dev, "Unable to setup genalloc pool.\n");
+		return -ENOMEM;
+	}
+
+	reset_control_deassert(ctx->reset);
+
+	msleep(10);
+
+	rc = aspeed_xdma_init_mem(ctx);
+	if (rc) {
+		reset_control_assert(ctx->reset);
+		return rc;
+	}
+
+	aspeed_xdma_init_eng(ctx);
+
+	return 0;
+}
+
+static int aspeed_xdma_remove(struct platform_device *pdev)
+{
+	struct aspeed_xdma *ctx = platform_get_drvdata(pdev);
+
+	gen_pool_free(ctx->vga_pool, (unsigned long)ctx->cmdq_vga_virt,
+		      XDMA_CMDQ_SIZE);
+	dma_free_coherent(ctx->dev, ctx->vga_size, ctx->vga_virt,
+			  ctx->vga_dma);
+	dma_release_declared_memory(ctx->dev);
+	reset_control_assert(ctx->reset);
+
+	return 0;
+}
+
+static const struct of_device_id aspeed_xdma_match[] = {
+	{ .compatible = "aspeed,ast2500-xdma" },
+	{ },
+};
+
+static struct platform_driver aspeed_xdma_driver = {
+	.probe = aspeed_xdma_probe,
+	.remove = aspeed_xdma_remove,
+	.driver = {
+		.name = DEVICE_NAME,
+		.of_match_table = aspeed_xdma_match,
+	},
+};
+
+module_platform_driver(aspeed_xdma_driver);
+
+MODULE_AUTHOR("Eddie James");
+MODULE_DESCRIPTION("Aspeed XDMA Engine Driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/uapi/linux/aspeed-xdma.h b/include/uapi/linux/aspeed-xdma.h
new file mode 100644
index 0000000..998459e
--- /dev/null
+++ b/include/uapi/linux/aspeed-xdma.h
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/* Copyright IBM Corp 2019 */
+
+#ifndef _UAPI_LINUX_ASPEED_XDMA_H_
+#define _UAPI_LINUX_ASPEED_XDMA_H_
+
+#include <linux/types.h>
+
+/*
+ * aspeed_xdma_op
+ *
+ * host_addr: the DMA address on the host side, typically configured by PCI
+ *            subsystem
+ *
+ * len: the size of the transfer in bytes; it should be a multiple of 16 bytes
+ *
+ * upstream: boolean indicating the direction of the DMA operation; upstream
+ *           means a transfer from the BMC to the host
+ */
+struct aspeed_xdma_op {
+	__u64 host_addr;
+	__u32 len;
+	__u32 upstream;
+};
+
+#endif /* _UAPI_LINUX_ASPEED_XDMA_H_ */
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH v3 1/8] dt-bindings: soc: Add Aspeed XDMA engine binding documentation
From: Eddie James @ 2019-05-29 18:10 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <1559153408-31190-1-git-send-email-eajames@linux.ibm.com>

Document the bindings.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
 .../devicetree/bindings/soc/aspeed/xdma.txt        | 23 ++++++++++++++++++++++
 1 file changed, 23 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/aspeed/xdma.txt

diff --git a/Documentation/devicetree/bindings/soc/aspeed/xdma.txt b/Documentation/devicetree/bindings/soc/aspeed/xdma.txt
new file mode 100644
index 0000000..85e82ea
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/aspeed/xdma.txt
@@ -0,0 +1,23 @@
+* Device tree bindings for the Aspeed XDMA Engine
+
+The XDMA Engine embedded in the AST2500 SOC can perform automatic DMA
+operations over PCI between the AST2500 (acting as a BMC) and a host processor.
+
+Required properties:
+
+ - compatible		"aspeed,ast2500-xdma"
+ - reg			contains the offset and length of the memory region
+			assigned to the XDMA registers
+ - resets		reset specifier for the syscon reset associated with
+			the XDMA engine
+ - interrupts		the interrupt associated with the XDMA engine on this
+			platform
+
+Example:
+
+    xdma at 1e6e7000 {
+        compatible = "aspeed,ast2500-xdma";
+        reg = <0x1e6e7000 0x100>;
+        resets = <&syscon ASPEED_RESET_XDMA>;
+        interrupts = <6>;
+    };
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH v3 0/8] drivers/soc: Add Aspeed XDMA Engine Driver
From: Eddie James @ 2019-05-29 18:10 UTC (permalink / raw)
  To: linux-aspeed

The XDMA engine embedded in the AST2500 SOC performs PCI DMA operations
between the SOC (acting as a BMC) and a host processor in a server.

This series adds a driver to control the XDMA engine in order to easily
perform DMA operations to and from the host processor.

Changes since v2:
 - Switch to one pci device config sysfs file
 - Add documentation for pci device config sysfs file
 - Add module parameter for pci device config
 - Switch to genalloc instead of custom allocator
 - Fix alignment of user structure
 - Cleaned up debugfs functions

I previously sent this series v1 to drivers/misc, but I'm now fairly certain
it belongs in drivers/soc, especially since the other Aspeed drivers have been
moved to soc.

Changes since v1:
 - Correct the XDMA command pitch
 - Don't use packed for the aspeed_xdma_op structure
 - Correct the SCU PCI config change

Eddie James (8):
  dt-bindings: soc: Add Aspeed XDMA engine binding documentation
  drivers/soc: Add Aspeed XDMA Engine Driver
  drivers/soc: xdma: Add user interface
  Documentation: ABI: Add aspeed-xdma sysfs documentation
  drivers/soc: xdma: Add PCI device configuration sysfs
  drivers/soc: xdma: Add debugfs entries
  ARM: dts: aspeed: Add XDMA Engine
  ARM: dts: aspeed: witherspoon: Enable XDMA Engine

 .../ABI/testing/sysfs-devices-platform-aspeed-xdma |  11 +
 .../devicetree/bindings/soc/aspeed/xdma.txt        |  23 +
 MAINTAINERS                                        |  10 +
 arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts   |   4 +
 arch/arm/boot/dts/aspeed-g5.dtsi                   |   8 +
 drivers/soc/aspeed/Kconfig                         |   8 +
 drivers/soc/aspeed/Makefile                        |   1 +
 drivers/soc/aspeed/aspeed-xdma.c                   | 912 +++++++++++++++++++++
 include/uapi/linux/aspeed-xdma.h                   |  26 +
 9 files changed, 1003 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-devices-platform-aspeed-xdma
 create mode 100644 Documentation/devicetree/bindings/soc/aspeed/xdma.txt
 create mode 100644 drivers/soc/aspeed/aspeed-xdma.c
 create mode 100644 include/uapi/linux/aspeed-xdma.h

-- 
1.8.3.1


^ permalink raw reply

* [PATCH v2 11/11] media: aspeed: add a workaround to fix a silicon bug
From: Jae Hyun Yoo @ 2019-05-29 17:29 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <03a3cf74-3fd3-982e-ec37-014ed4a13b47@linux.vnet.ibm.com>

On 5/29/2019 7:07 AM, Eddie James wrote:
> 
> On 5/24/19 6:17 PM, Jae Hyun Yoo wrote:
>> AST2500 silicon revision A1 and A2 have a silicon bug which causes
>> extremly long capturing time on specific resolutions (1680 width).
>> To fix the bug, this commit adjusts the capturing window register
>> setting to 1728 if detected width is 1680. The compression window
>> register setting will be kept as the original width so output
>> result will be the same.
> 
> 
> This is a bit curious, why 1728 in particular? And what is the behavior 
> of the VE when the capture window is larger than the actual source 
> resolution?

For an example, if resolution is 1680x1050, capturing operation takes
very long time because VE has the silicon bug. So this patch adjusts
the 'Capture Window' register slightly larger than 1680 to avoid the
issue. As a result, source buffer will copy 1728x1050 frames from the
original screen buffer but the image is still has valid information.
As the next step in compression phase, it will set the 'Compression
Window' register as '1680x1050' so it will compress using the original
image resolution which is a cropped image from the '1728x1050' source
buffer.

You can compare results using these shell commands in Ubuntu GUI
desktop.

$ xrandr --newmode "1680x1050_60.00"  146.25  1680 1784 1960 1240  1050 
1053 1059 1089 -hsync +vsync
$ xrandr --addmode VGA-1 1680x1050_60.00
$ xrandr --output VGA-1 --mode 1680x1050_60.00

I'm also curious about why that is 1728. Actually, this workaround was
provided from the chip vendor, Aspeed, and they use this in their SDK
code too. Let's check it to Ryan.


Hi Ryan,

Can you please explain why that is 1728 in particular.

Thanks,
Jae

> 
> Thanks,
> 
> Eddie
> 
> 
>>
>> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
>> ---
>> v1 -> v2:
>> ? New.
>>
>> ? drivers/media/platform/aspeed-video.c | 26 +++++++++++++++++++-------
>> ? 1 file changed, 19 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/media/platform/aspeed-video.c 
>> b/drivers/media/platform/aspeed-video.c
>> index b05b073b63bc..f93989f532d6 100644
>> --- a/drivers/media/platform/aspeed-video.c
>> +++ b/drivers/media/platform/aspeed-video.c
>> @@ -824,8 +824,27 @@ static void aspeed_video_set_resolution(struct 
>> aspeed_video *video)
>> ????? struct v4l2_bt_timings *act = &video->active_timings;
>> ????? unsigned int size = act->width * act->height;
>> +??? /* Set capture/compression frame sizes */
>> ????? aspeed_video_calc_compressed_size(video, size);
>> +??? if (video->active_timings.width == 1680) {
>> +??????? /*
>> +???????? * This is a workaround to fix a silicon bug on A1 and A2
>> +???????? * revisions. Since it doesn't break capturing operation on A0
>> +???????? * revision, use it for all revisions without checking the
>> +???????? * revision ID.
>> +???????? */
>> +??????? aspeed_video_write(video, VE_CAP_WINDOW,
>> +?????????????????? 1728 << 16 | act->height);
>> +??????? size += (1728 - 1680) * video->active_timings.height;
>> +??? } else {
>> +??????? aspeed_video_write(video, VE_CAP_WINDOW,
>> +?????????????????? act->width << 16 | act->height);
>> +??? }
>> +??? aspeed_video_write(video, VE_COMP_WINDOW,
>> +?????????????? act->width << 16 | act->height);
>> +??? aspeed_video_write(video, VE_SRC_SCANLINE_OFFSET, act->width * 4);
>> +
>> ????? /* Don't use direct mode below 1024 x 768 (irqs don't fire) */
>> ????? if (size < DIRECT_FETCH_THRESHOLD) {
>> ????????? aspeed_video_write(video, VE_TGS_0,
>> @@ -842,13 +861,6 @@ static void aspeed_video_set_resolution(struct 
>> aspeed_video *video)
>> ????????? aspeed_video_update(video, VE_CTRL, 0, VE_CTRL_DIRECT_FETCH);
>> ????? }
>> -??? /* Set capture/compression frame sizes */
>> -??? aspeed_video_write(video, VE_CAP_WINDOW,
>> -?????????????? act->width << 16 | act->height);
>> -??? aspeed_video_write(video, VE_COMP_WINDOW,
>> -?????????????? act->width << 16 | act->height);
>> -??? aspeed_video_write(video, VE_SRC_SCANLINE_OFFSET, act->width * 4);
>> -
>> ????? size *= 4;
>> ????? if (size == video->srcs[0].size / 2) {
> 
> 

^ permalink raw reply

* [PATCH] misc: aspeed-lpc-ctrl: make parameter optional
From: Vijay Khemka @ 2019-05-29 17:22 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20190524155923.GA7516@kroah.com>



?On 5/24/19, 8:59 AM, "Greg Kroah-Hartman" <gregkh@linuxfoundation.org> wrote:

    On Wed, May 01, 2019 at 03:34:11PM -0700, Vijay Khemka wrote:
    > Makiing memory-region and flash as optional parameter in device
    > tree if user needs to use these parameter through ioctl then
    > need to define in devicetree.
    > 
    > Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
    > ---
    >  drivers/misc/aspeed-lpc-ctrl.c | 58 +++++++++++++++++++++-------------
    >  1 file changed, 36 insertions(+), 22 deletions(-)
    
    File is no longer at this location :(

I have rebased to new location and combined both patches and submitted again. __

Regards
-Vijay
    


^ permalink raw reply

* [PATCH] soc: aspeed: lpc-ctrl: make parameter optional
From: Vijay Khemka @ 2019-05-29 17:21 UTC (permalink / raw)
  To: linux-aspeed

Makiing memory-region and flash as optional parameter in device
tree if user needs to use these parameter through ioctl then
need to define in devicetree.

Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
---
 drivers/soc/aspeed/aspeed-lpc-ctrl.c | 58 +++++++++++++++++-----------
 1 file changed, 36 insertions(+), 22 deletions(-)

diff --git a/drivers/soc/aspeed/aspeed-lpc-ctrl.c b/drivers/soc/aspeed/aspeed-lpc-ctrl.c
index a024f8042259..aca13779764a 100644
--- a/drivers/soc/aspeed/aspeed-lpc-ctrl.c
+++ b/drivers/soc/aspeed/aspeed-lpc-ctrl.c
@@ -68,6 +68,7 @@ static long aspeed_lpc_ctrl_ioctl(struct file *file, unsigned int cmd,
 		unsigned long param)
 {
 	struct aspeed_lpc_ctrl *lpc_ctrl = file_aspeed_lpc_ctrl(file);
+	struct device *dev = file->private_data;
 	void __user *p = (void __user *)param;
 	struct aspeed_lpc_ctrl_mapping map;
 	u32 addr;
@@ -90,6 +91,12 @@ static long aspeed_lpc_ctrl_ioctl(struct file *file, unsigned int cmd,
 		if (map.window_id != 0)
 			return -EINVAL;
 
+		/* If memory-region is not described in device tree */
+		if (!lpc_ctrl->mem_size) {
+			dev_dbg(dev, "Didn't find reserved memory\n");
+			return -ENXIO;
+		}
+
 		map.size = lpc_ctrl->mem_size;
 
 		return copy_to_user(p, &map, sizeof(map)) ? -EFAULT : 0;
@@ -126,9 +133,18 @@ static long aspeed_lpc_ctrl_ioctl(struct file *file, unsigned int cmd,
 			return -EINVAL;
 
 		if (map.window_type == ASPEED_LPC_CTRL_WINDOW_FLASH) {
+			if (!lpc_ctrl->pnor_size) {
+				dev_dbg(dev, "Didn't find host pnor flash\n");
+				return -ENXIO;
+			}
 			addr = lpc_ctrl->pnor_base;
 			size = lpc_ctrl->pnor_size;
 		} else if (map.window_type == ASPEED_LPC_CTRL_WINDOW_MEMORY) {
+			/* If memory-region is not described in device tree */
+			if (!lpc_ctrl->mem_size) {
+				dev_dbg(dev, "Didn't find reserved memory\n");
+				return -ENXIO;
+			}
 			addr = lpc_ctrl->mem_base;
 			size = lpc_ctrl->mem_size;
 		} else {
@@ -196,17 +212,17 @@ static int aspeed_lpc_ctrl_probe(struct platform_device *pdev)
 	if (!lpc_ctrl)
 		return -ENOMEM;
 
+	/* If flash is described in device tree then store */
 	node = of_parse_phandle(dev->of_node, "flash", 0);
 	if (!node) {
-		dev_err(dev, "Didn't find host pnor flash node\n");
-		return -ENODEV;
-	}
-
-	rc = of_address_to_resource(node, 1, &resm);
-	of_node_put(node);
-	if (rc) {
-		dev_err(dev, "Couldn't address to resource for flash\n");
-		return rc;
+		dev_dbg(dev, "Didn't find host pnor flash node\n");
+	} else {
+		rc = of_address_to_resource(node, 1, &resm);
+		of_node_put(node);
+		if (rc) {
+			dev_err(dev, "Couldn't address to resource for flash\n");
+			return rc;
+		}
 	}
 
 	lpc_ctrl->pnor_size = resource_size(&resm);
@@ -214,22 +230,22 @@ static int aspeed_lpc_ctrl_probe(struct platform_device *pdev)
 
 	dev_set_drvdata(&pdev->dev, lpc_ctrl);
 
+	/* If memory-region is described in device tree then store */
 	node = of_parse_phandle(dev->of_node, "memory-region", 0);
 	if (!node) {
-		dev_err(dev, "Didn't find reserved memory\n");
-		return -EINVAL;
-	}
+		dev_dbg(dev, "Didn't find reserved memory\n");
+	} else {
+		rc = of_address_to_resource(node, 0, &resm);
+		of_node_put(node);
+		if (rc) {
+			dev_err(dev, "Couldn't address to resource for reserved memory\n");
+			return -ENXIO;
+		}
 
-	rc = of_address_to_resource(node, 0, &resm);
-	of_node_put(node);
-	if (rc) {
-		dev_err(dev, "Couldn't address to resource for reserved memory\n");
-		return -ENOMEM;
+		lpc_ctrl->mem_size = resource_size(&resm);
+		lpc_ctrl->mem_base = resm.start;
 	}
 
-	lpc_ctrl->mem_size = resource_size(&resm);
-	lpc_ctrl->mem_base = resm.start;
-
 	lpc_ctrl->regmap = syscon_node_to_regmap(
 			pdev->dev.parent->of_node);
 	if (IS_ERR(lpc_ctrl->regmap)) {
@@ -258,8 +274,6 @@ static int aspeed_lpc_ctrl_probe(struct platform_device *pdev)
 		goto err;
 	}
 
-	dev_info(dev, "Loaded at %pr\n", &resm);
-
 	return 0;
 
 err:
-- 
2.17.1


^ permalink raw reply related

* [PATCH v2 10/11] media: aspeed: fix an incorrect timeout checking in mode detection
From: Jae Hyun Yoo @ 2019-05-29 17:04 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <50abcb27-30af-9dfc-3a53-4261832f272d@linux.vnet.ibm.com>


On 5/29/2019 7:03 AM, Eddie James wrote:
> 
> On 5/24/19 6:17 PM, Jae Hyun Yoo wrote:
>> There is an incorrect timeout checking in mode detection logic so
>> it misses resolution detecting chances. This commit fixes the bug.
>>
>> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
>> ---
>> v1 -> v2:
>> ? New.
>>
>> ? drivers/media/platform/aspeed-video.c | 2 +-
>> ? 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/platform/aspeed-video.c 
>> b/drivers/media/platform/aspeed-video.c
>> index 67f476bf0a03..b05b073b63bc 100644
>> --- a/drivers/media/platform/aspeed-video.c
>> +++ b/drivers/media/platform/aspeed-video.c
>> @@ -735,7 +735,7 @@ static void aspeed_video_get_resolution(struct 
>> aspeed_video *video)
>> ????? do {
>> ????????? if (tries) {
>> ????????????? set_current_state(TASK_INTERRUPTIBLE);
>> -??????????? if (schedule_timeout(INVALID_RESOLUTION_DELAY))
>> +??????????? if (!schedule_timeout(INVALID_RESOLUTION_DELAY))
>> ????????????????? return;
> 
> 
> schedule_timeout returns 0 when the timer has expired otherwise the 
> remaining time in? jiffies will be returned. So if it was interrupted 
> (timer did not expire and it returns non-zero) then we should return, 
> rather than keep trying. So I think it was correct before. Thanks, Eddie

I thought that there is an explicit waking up case of this waiting
before the delay expires but I checked code again that there isn't. So
yes, you are right. Will drop this change from this series.

Thanks for the review!

Regards,
Jae

^ permalink raw reply

* [PATCH v2 08/11] media: aspeed: remove source buffer allocation before mode detection
From: Eddie James @ 2019-05-29 16:07 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20190524231725.12320-9-jae.hyun.yoo@linux.intel.com>


On 5/24/19 6:17 PM, Jae Hyun Yoo wrote:
> Mode detection doesn't require source buffer allocation so this
> commit removes that.


Reviewed-by: Eddie James <eajames@linux.ibm.com>


>
> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
> ---
> v1 -> v2:
>   New.
>
>   drivers/media/platform/aspeed-video.c | 21 ---------------------
>   1 file changed, 21 deletions(-)
>
> diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
> index c0b889141b8f..4647ed2e9e63 100644
> --- a/drivers/media/platform/aspeed-video.c
> +++ b/drivers/media/platform/aspeed-video.c
> @@ -731,27 +731,6 @@ static void aspeed_video_get_resolution(struct aspeed_video *video)
>   	det->height = MIN_HEIGHT;
>   	video->v4l2_input_status = V4L2_IN_ST_NO_SIGNAL;
>   
> -	/*
> -	 * Since we need max buffer size for detection, free the second source
> -	 * buffer first.
> -	 */
> -	if (video->srcs[1].size)
> -		aspeed_video_free_buf(video, &video->srcs[1]);
> -
> -	if (video->srcs[0].size < VE_MAX_SRC_BUFFER_SIZE) {
> -		if (video->srcs[0].size)
> -			aspeed_video_free_buf(video, &video->srcs[0]);
> -
> -		if (!aspeed_video_alloc_buf(video, &video->srcs[0],
> -					    VE_MAX_SRC_BUFFER_SIZE)) {
> -			dev_err(video->dev,
> -				"Failed to allocate source buffers\n");
> -			return;
> -		}
> -	}
> -
> -	aspeed_video_write(video, VE_SRC0_ADDR, video->srcs[0].dma);
> -
>   	do {
>   		if (tries) {
>   			set_current_state(TASK_INTERRUPTIBLE);


^ permalink raw reply

* [PATCH v2 09/11] media: aspeed: use different delays for triggering VE H/W reset
From: Eddie James @ 2019-05-29 16:07 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20190524231725.12320-10-jae.hyun.yoo@linux.intel.com>


On 5/24/19 6:17 PM, Jae Hyun Yoo wrote:
> In case of watchdog timeout detected while doing mode detection,
> it's better triggering video engine hardware reset immediately so
> this commit fixes code for the case. Other than the case, it will
> trigger video engine hardware reset after RESOLUTION_CHANGE_DELAY.


Reviewed-by: Eddie James <eajames@linux.ibm.com>


>
> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
> ---
> v1 -> v2:
>   New.
>
>   drivers/media/platform/aspeed-video.c | 9 +++++----
>   1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
> index 4647ed2e9e63..67f476bf0a03 100644
> --- a/drivers/media/platform/aspeed-video.c
> +++ b/drivers/media/platform/aspeed-video.c
> @@ -522,7 +522,7 @@ static void aspeed_video_bufs_done(struct aspeed_video *video,
>   	spin_unlock_irqrestore(&video->lock, flags);
>   }
>   
> -static void aspeed_video_irq_res_change(struct aspeed_video *video)
> +static void aspeed_video_irq_res_change(struct aspeed_video *video, ulong delay)
>   {
>   	dev_dbg(video->dev, "Resolution changed; resetting\n");
>   
> @@ -532,7 +532,7 @@ static void aspeed_video_irq_res_change(struct aspeed_video *video)
>   	aspeed_video_off(video);
>   	aspeed_video_bufs_done(video, VB2_BUF_STATE_ERROR);
>   
> -	schedule_delayed_work(&video->res_work, RESOLUTION_CHANGE_DELAY);
> +	schedule_delayed_work(&video->res_work, delay);
>   }
>   
>   static irqreturn_t aspeed_video_irq(int irq, void *arg)
> @@ -545,7 +545,7 @@ static irqreturn_t aspeed_video_irq(int irq, void *arg)
>   	 * re-initialize
>   	 */
>   	if (sts & VE_INTERRUPT_MODE_DETECT_WD) {
> -		aspeed_video_irq_res_change(video);
> +		aspeed_video_irq_res_change(video, 0);
>   		return IRQ_HANDLED;
>   	}
>   
> @@ -563,7 +563,8 @@ static irqreturn_t aspeed_video_irq(int irq, void *arg)
>   			 * Signal acquired while NOT doing resolution
>   			 * detection; reset the engine and re-initialize
>   			 */
> -			aspeed_video_irq_res_change(video);
> +			aspeed_video_irq_res_change(video,
> +						    RESOLUTION_CHANGE_DELAY);
>   			return IRQ_HANDLED;
>   		}
>   	}


^ permalink raw reply

* [PATCH v2 11/11] media: aspeed: add a workaround to fix a silicon bug
From: Eddie James @ 2019-05-29 14:07 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20190524231725.12320-12-jae.hyun.yoo@linux.intel.com>


On 5/24/19 6:17 PM, Jae Hyun Yoo wrote:
> AST2500 silicon revision A1 and A2 have a silicon bug which causes
> extremly long capturing time on specific resolutions (1680 width).
> To fix the bug, this commit adjusts the capturing window register
> setting to 1728 if detected width is 1680. The compression window
> register setting will be kept as the original width so output
> result will be the same.


This is a bit curious, why 1728 in particular? And what is the behavior 
of the VE when the capture window is larger than the actual source 
resolution?

Thanks,

Eddie


>
> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
> ---
> v1 -> v2:
>   New.
>
>   drivers/media/platform/aspeed-video.c | 26 +++++++++++++++++++-------
>   1 file changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
> index b05b073b63bc..f93989f532d6 100644
> --- a/drivers/media/platform/aspeed-video.c
> +++ b/drivers/media/platform/aspeed-video.c
> @@ -824,8 +824,27 @@ static void aspeed_video_set_resolution(struct aspeed_video *video)
>   	struct v4l2_bt_timings *act = &video->active_timings;
>   	unsigned int size = act->width * act->height;
>   
> +	/* Set capture/compression frame sizes */
>   	aspeed_video_calc_compressed_size(video, size);
>   
> +	if (video->active_timings.width == 1680) {
> +		/*
> +		 * This is a workaround to fix a silicon bug on A1 and A2
> +		 * revisions. Since it doesn't break capturing operation on A0
> +		 * revision, use it for all revisions without checking the
> +		 * revision ID.
> +		 */
> +		aspeed_video_write(video, VE_CAP_WINDOW,
> +				   1728 << 16 | act->height);
> +		size += (1728 - 1680) * video->active_timings.height;
> +	} else {
> +		aspeed_video_write(video, VE_CAP_WINDOW,
> +				   act->width << 16 | act->height);
> +	}
> +	aspeed_video_write(video, VE_COMP_WINDOW,
> +			   act->width << 16 | act->height);
> +	aspeed_video_write(video, VE_SRC_SCANLINE_OFFSET, act->width * 4);
> +
>   	/* Don't use direct mode below 1024 x 768 (irqs don't fire) */
>   	if (size < DIRECT_FETCH_THRESHOLD) {
>   		aspeed_video_write(video, VE_TGS_0,
> @@ -842,13 +861,6 @@ static void aspeed_video_set_resolution(struct aspeed_video *video)
>   		aspeed_video_update(video, VE_CTRL, 0, VE_CTRL_DIRECT_FETCH);
>   	}
>   
> -	/* Set capture/compression frame sizes */
> -	aspeed_video_write(video, VE_CAP_WINDOW,
> -			   act->width << 16 | act->height);
> -	aspeed_video_write(video, VE_COMP_WINDOW,
> -			   act->width << 16 | act->height);
> -	aspeed_video_write(video, VE_SRC_SCANLINE_OFFSET, act->width * 4);
> -
>   	size *= 4;
>   
>   	if (size == video->srcs[0].size / 2) {


^ permalink raw reply

* [PATCH v2 10/11] media: aspeed: fix an incorrect timeout checking in mode detection
From: Eddie James @ 2019-05-29 14:03 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20190524231725.12320-11-jae.hyun.yoo@linux.intel.com>


On 5/24/19 6:17 PM, Jae Hyun Yoo wrote:
> There is an incorrect timeout checking in mode detection logic so
> it misses resolution detecting chances. This commit fixes the bug.
>
> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
> ---
> v1 -> v2:
>   New.
>
>   drivers/media/platform/aspeed-video.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
> index 67f476bf0a03..b05b073b63bc 100644
> --- a/drivers/media/platform/aspeed-video.c
> +++ b/drivers/media/platform/aspeed-video.c
> @@ -735,7 +735,7 @@ static void aspeed_video_get_resolution(struct aspeed_video *video)
>   	do {
>   		if (tries) {
>   			set_current_state(TASK_INTERRUPTIBLE);
> -			if (schedule_timeout(INVALID_RESOLUTION_DELAY))
> +			if (!schedule_timeout(INVALID_RESOLUTION_DELAY))
>   				return;


schedule_timeout returns 0 when the timer has expired otherwise the 
remaining time in  jiffies will be returned. So if it was interrupted (timer did not expire 
and it returns non-zero) then we should return, rather than keep trying. 
So I think it was correct before. Thanks, Eddie

>   		}
>   


^ permalink raw reply

* [PATCH v2 00/11] Improve stability and add bug fixes of Aspeed video engine driver
From: Hans Verkuil @ 2019-05-29  9:55 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20190524231725.12320-1-jae.hyun.yoo@linux.intel.com>

Eddie,

Can you review the last 4 new patches? If all is OK, then I can make a pull
request for this series.

Regards,

	Hans

On 5/25/19 1:17 AM, Jae Hyun Yoo wrote:
> This patch series improves stability of Aspeed video engine driver by fixing
> clock control and irq handling logic in the driver. Also, it adds a couple of
> bug fixes and a workaroud for a silicon bug.
> 
> Changes since v1:
> - Removed spinlock handling code from 0001 patch.
> - Added 4 more patches.
> 
> Jae Hyun Yoo (11):
>   media: aspeed: fix a kernel warning on clk control
>   media: aspeed: refine clock control logic
>   media: aspeed: change irq to threaded irq
>   media: aspeed: remove IRQF_SHARED flag
>   media: aspeed: reduce noisy log printing outs
>   media: aspeed: remove checking of VE_INTERRUPT_CAPTURE_COMPLETE
>   media: aspeed: refine interrupt handling logic
>   media: aspeed: remove source buffer allocation before mode detection
>   media: aspeed: use different delays for triggering VE H/W reset
>   media: aspeed: fix an incorrect timeout checking in mode detection
>   media: aspeed: add a workaround to fix a silicon bug
> 
>  drivers/media/platform/aspeed-video.c | 140 +++++++++++++++-----------
>  1 file changed, 80 insertions(+), 60 deletions(-)
> 


^ permalink raw reply

* [PATCH -next] EDAC: aspeed: Remove set but not used variable 'np'
From: Stefan Schaeckeler @ 2019-05-29  3:10 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <04f103fb-54b1-4911-8164-44b20bfd1e72@www.fastmail.com>

On  Tuesday, May 28, 2019 at 6:27 PM, Andrew Jeffery wrote:
> On Sun, 26 May 2019, at 00:12, YueHaibing wrote:
> > Fixes gcc '-Wunused-but-set-variable' warning:
> >
> > drivers/edac/aspeed_edac.c: In function aspeed_probe:
> > drivers/edac/aspeed_edac.c:284:22: warning: variable np set but not
> > used [-Wunused-but-set-variable]
> >
> > It is never used and can be removed.
> >
> > Signed-off-by: YueHaibing <yuehaibing@huawei.com>
>
> Reviewed-by: Andrew Jeffery <andrew@aj.id.au>

Reviewed-by: Stefan Schaeckeler <sschaeck@cisco.com>

> > ---
> >  drivers/edac/aspeed_edac.c | 4 ----
> >  1 file changed, 4 deletions(-)
> >
> > diff --git a/drivers/edac/aspeed_edac.c b/drivers/edac/aspeed_edac.c
> > index 11833c0a5d07..5634437bb39d 100644
> > --- a/drivers/edac/aspeed_edac.c
> > +++ b/drivers/edac/aspeed_edac.c
> > @@ -281,15 +281,11 @@ static int aspeed_probe(struct platform_device *pdev)
> >  	struct device *dev = &pdev->dev;
> >  	struct edac_mc_layer layers[2];
> >  	struct mem_ctl_info *mci;
> > -	struct device_node *np;
> >  	struct resource *res;
> >  	void __iomem *regs;
> >  	u32 reg04;
> > 	int rc;
> >
> > -	/* setup regmap */
> > -	np = dev->of_node;
> > -
> >  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >  	if (!res)
> >  		return -ENOENT;
> > --
> > 2.17.1


^ permalink raw reply

* [PATCH v2 2/7] drivers/soc: Add Aspeed XDMA Engine Driver
From: Andrew Jeffery @ 2019-05-29  1:59 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <687e4a77-0df1-4982-1edd-9d0559c489fe@linux.vnet.ibm.com>



On Sat, 25 May 2019, at 01:39, Eddie James wrote:
> 
> On 5/21/19 7:02 AM, Arnd Bergmann wrote:
> > On Mon, May 20, 2019 at 10:19 PM Eddie James <eajames@linux.ibm.com> wrote:
> >> diff --git a/include/uapi/linux/aspeed-xdma.h b/include/uapi/linux/aspeed-xdma.h
> >> new file mode 100644
> >> index 0000000..2a4bd13
> >> --- /dev/null
> >> +++ b/include/uapi/linux/aspeed-xdma.h
> >> @@ -0,0 +1,26 @@
> >> +/* SPDX-License-Identifier: GPL-2.0+ */
> >> +/* Copyright IBM Corp 2019 */
> >> +
> >> +#ifndef _UAPI_LINUX_ASPEED_XDMA_H_
> >> +#define _UAPI_LINUX_ASPEED_XDMA_H_
> >> +
> >> +#include <linux/types.h>
> >> +
> >> +/*
> >> + * aspeed_xdma_op
> >> + *
> >> + * upstream: boolean indicating the direction of the DMA operation; upstream
> >> + *           means a transfer from the BMC to the host
> >> + *
> >> + * host_addr: the DMA address on the host side, typically configured by PCI
> >> + *            subsystem
> >> + *
> >> + * len: the size of the transfer in bytes; it should be a multiple of 16 bytes
> >> + */
> >> +struct aspeed_xdma_op {
> >> +       __u32 upstream;
> >> +       __u64 host_addr;
> >> +       __u32 len;
> >> +};
> >> +
> >> +#endif /* _UAPI_LINUX_ASPEED_XDMA_H_ */
> > If this is a user space interface, please remove the holes in the
> > data structure.
> 
> 
> Surely it's 4-byte aligned and there won't be holes??

__u64 is 8-byte aligned, so you have a hole after upstream.

Easiest just to put upstream after len?

Andrew

^ permalink raw reply

* [PATCH -next] EDAC: aspeed: Remove set but not used variable 'np'
From: Andrew Jeffery @ 2019-05-29  1:27 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20190525144153.2028-1-yuehaibing@huawei.com>



On Sun, 26 May 2019, at 00:12, YueHaibing wrote:
> Fixes gcc '-Wunused-but-set-variable' warning:
> 
> drivers/edac/aspeed_edac.c: In function aspeed_probe:
> drivers/edac/aspeed_edac.c:284:22: warning: variable np set but not 
> used [-Wunused-but-set-variable]
> 
> It is never used and can be removed.
> 
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>

Reviewed-by: Andrew Jeffery <andrew@aj.id.au>

> ---
>  drivers/edac/aspeed_edac.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/drivers/edac/aspeed_edac.c b/drivers/edac/aspeed_edac.c
> index 11833c0a5d07..5634437bb39d 100644
> --- a/drivers/edac/aspeed_edac.c
> +++ b/drivers/edac/aspeed_edac.c
> @@ -281,15 +281,11 @@ static int aspeed_probe(struct platform_device *pdev)
>  	struct device *dev = &pdev->dev;
>  	struct edac_mc_layer layers[2];
>  	struct mem_ctl_info *mci;
> -	struct device_node *np;
>  	struct resource *res;
>  	void __iomem *regs;
>  	u32 reg04;
>  	int rc;
>  
> -	/* setup regmap */
> -	np = dev->of_node;
> -
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	if (!res)
>  		return -ENOENT;
> -- 
> 2.17.1
> 
> 
>

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox