* [PATCH 13/14] arm: dts: OpenPower Romulus system can use coprocessor for FSI
From: Joel Stanley @ 2018-06-28 4:12 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180626232605.13420-14-benh@kernel.crashing.org>
On 27 June 2018 at 08:56, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
I will take this through the ASPEED SoC tree once we've got acks on
the bindings.
Cheers,
Joel
> ---
> arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts | 15 +++++++++++++--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts b/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
> index 347938673c83..070b8f8f1d62 100644
> --- a/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
> @@ -37,6 +37,11 @@
> compatible = "shared-dma-pool";
> reusable;
> };
> +
> + coldfire_memory: codefire_memory at 9ef00000 {
> + reg = <0x9ef00000 0x00100000>;
> + no-map;
> + };
> };
>
> leds {
> @@ -56,10 +61,16 @@
> };
>
> fsi: gpio-fsi {
> - compatible = "fsi-master-gpio", "fsi-master";
> + compatible = "ibm,fsi-master-ast2500-cf", "fsi-master";
> #address-cells = <2>;
> #size-cells = <0>;
> - no-gpio-delays;
> +
> + memory-region = <&coldfire_memory>;
> + sram = <&sram>;
> + cvic = <&cvic>;
> +
> + fw-name = "romulus";
> + fw-platform-sig = <0x526d>;
>
> clock-gpios = <&gpio ASPEED_GPIO(AA, 0) GPIO_ACTIVE_HIGH>;
> data-gpios = <&gpio ASPEED_GPIO(AA, 2) GPIO_ACTIVE_HIGH>;
> --
> 2.17.1
>
^ permalink raw reply
* [PATCH 11/14] dt-bindings: fsi: Document binding for the fsi-master-ast-cf "device"
From: Joel Stanley @ 2018-06-28 4:12 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180626232605.13420-12-benh@kernel.crashing.org>
On 27 June 2018 at 08:56, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> This isn't per-se a real device, it's a pseudo-device that
> represents the use of the Aspeed built-in ColdFire to
> implement the FSI protocol by bitbanging the GPIOs instead
> of doing it from the ARM core.
>
> Thus it's a drop-in replacement for the existing
> fsi-master-gpio pseudo-device for use on systems for which
> a corresponding firmware file exists. It has most of the
> same properties, plus some more needed to operate the
> coprocessor.
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Acked-by: Joel Stanley <joel@jms.id.au>
^ permalink raw reply
* [PATCH 10/14] fsi: Move various master definitions to a common header
From: Joel Stanley @ 2018-06-28 4:12 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180626232605.13420-11-benh@kernel.crashing.org>
On 27 June 2018 at 08:56, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> This moves the definitions for various protocol details
> (message & response codes, delays etc...) out of
> fsi-master-gpio.c to fsi-master.h in order to share them
> with other master implementations.
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Reviewed-by: Joel Stanley <joel@jms.id.au>
^ permalink raw reply
* [PATCH 09/14] fsi: master-gpio: Add missing release function
From: Joel Stanley @ 2018-06-28 4:12 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180626232605.13420-10-benh@kernel.crashing.org>
On 27 June 2018 at 08:56, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> The embedded struct device needs a release function to be
> able to successfully remove the driver.
>
> We remove the devm_gpiod_put() as they are unnecessary
> (the resources will be released automatically) and because
> fsi_master_unregister() will cause the master structure to
> be freed.
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Reviewed-by: Joel Stanley <joel@jms.id.au>
^ permalink raw reply
* [PATCH 08/14] fsi: master-gpio: Remove "GPIO" prefix on some definitions
From: Joel Stanley @ 2018-06-28 4:11 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180626232605.13420-9-benh@kernel.crashing.org>
On 27 June 2018 at 08:55, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> Some definitions are generic to the FSI protocol or any
> give master implementation. Rename them to remove the
> "GPIO" prefix in preparation for moving them to a common
> header.
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Reviewed-by: Joel Stanley <joel@jms.id.au>
^ permalink raw reply
* [PATCH 07/14] fsi: master-gpio: Remove unused definitions
From: Joel Stanley @ 2018-06-28 4:11 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180626232605.13420-8-benh@kernel.crashing.org>
On 27 June 2018 at 08:55, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Reviewed-by: Joel Stanley <joel@jms.id.au>
^ permalink raw reply
* [PATCH 06/14] fsi: master-gpio: Add more tracepoints
From: Joel Stanley @ 2018-06-28 4:11 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180626232605.13420-7-benh@kernel.crashing.org>
On 27 June 2018 at 08:55, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> This adds a few more tracepoints that have proven useful when
> debugging issues with the FSI bus.
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Reviewed-by: Joel Stanley <joel@jms.id.au>
> ---
> drivers/fsi/fsi-master-gpio.c | 16 ++++---
> include/trace/events/fsi_master_gpio.h | 59 ++++++++++++++++++++++++++
> 2 files changed, 69 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c
> index 48e0e65b2982..a00a85aa6d56 100644
> --- a/drivers/fsi/fsi-master-gpio.c
> +++ b/drivers/fsi/fsi-master-gpio.c
> @@ -130,10 +130,17 @@ static void set_sda_output(struct fsi_master_gpio *master, int value)
>
> static void clock_zeros(struct fsi_master_gpio *master, int count)
> {
> + trace_fsi_master_gpio_clock_zeros(master, count);
> set_sda_output(master, 1);
> clock_toggle(master, count);
> }
>
> +static void echo_delay(struct fsi_master_gpio *master)
> +{
> + clock_zeros(master, master->t_echo_delay);
> +}
This doesn't look like it belongs in this patch.
You've just moved it up. Not worth a re-spin.
> +
> +
> static void serial_in(struct fsi_master_gpio *master, struct fsi_gpio_msg *msg,
> uint8_t num_bits)
> {
> @@ -279,16 +286,19 @@ static void build_ar_command(struct fsi_master_gpio *master,
> addr_bits = 2;
> opcode_bits = 2;
> opcode = FSI_GPIO_CMD_SAME_AR;
> + trace_fsi_master_gpio_cmd_same_addr(master);
>
> } else if (check_relative_address(master, id, addr, &rel_addr)) {
> /* 8 bits plus sign */
> addr_bits = 9;
> addr = rel_addr;
> opcode = FSI_GPIO_CMD_REL_AR;
> + trace_fsi_master_gpio_cmd_rel_addr(master, rel_addr);
>
> } else {
> addr_bits = 21;
> opcode = FSI_GPIO_CMD_ABS_AR;
> + trace_fsi_master_gpio_cmd_abs_addr(master, addr);
> }
>
> /*
> @@ -337,12 +347,6 @@ static void build_epoll_command(struct fsi_gpio_msg *cmd, uint8_t slave_id)
> msg_push_crc(cmd);
> }
>
> -static void echo_delay(struct fsi_master_gpio *master)
> -{
> - set_sda_output(master, 1);
> - clock_toggle(master, master->t_echo_delay);
> -}
> -
> static void build_term_command(struct fsi_gpio_msg *cmd, uint8_t slave_id)
> {
> cmd->bits = 0;
^ permalink raw reply
* [PATCH 05/14] fsi: master-gpio: Add support for link_config
From: Joel Stanley @ 2018-06-28 4:11 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180626232605.13420-6-benh@kernel.crashing.org>
On 27 June 2018 at 08:55, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> To configure the send and echo delays
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Reviewed-by: Joel Stanley <joel@jms.id.au>
^ permalink raw reply
* [PATCH 04/14] fsi: master-gpio: Rename and adjust send delay
From: Joel Stanley @ 2018-06-28 4:10 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180626232321.12372-5-benh@kernel.crashing.org>
On 27 June 2018 at 08:53, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> What the driver called "FSI_GPIO_PRIME_SLAVE_CLOCKS" is what
> the FSI spec calls tSendDelay and should be 16 clocks by
> default.
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Reviewed-by: Joel Stanley <joel@jms.id.au>
^ permalink raw reply
* [PATCH 03/14] fsi: Add mechanism to set the tSendDelay and tEchoDelay values
From: Joel Stanley @ 2018-06-28 4:10 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180626232605.13420-4-benh@kernel.crashing.org>
On 27 June 2018 at 08:55, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> Those values control the amount of "dummy" clocks between commands and
> between a command and its response.
>
> This adds a way to configure them from sysfs (to be later extended to
> defaults in the device-tree). The default remains 16 (the HW default).
We should add these to Documentation/ABI/testing/sysfs-bus-fsi.
> This is only supported if the backend supports the new link_config()
> callback to configure the generation of those delays.
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Reviewed-by: Joel Stanley <joel@jms.id.au>
> ---
> ---
> drivers/fsi/fsi-core.c | 109 ++++++++++++++++++++++++++++++++-------
> drivers/fsi/fsi-master.h | 2 +
> 2 files changed, 93 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
> index 2f6f9b8c75e4..1ae5be31b4bf 100644
> --- a/drivers/fsi/fsi-core.c
> +++ b/drivers/fsi/fsi-core.c
> @@ -81,6 +81,8 @@ struct fsi_slave {
> int id;
> int link;
> uint32_t size; /* size of slave address space */
> + u8 t_send_delay;
> + u8 t_echo_delay;
> };
>
> #define to_fsi_master(d) container_of(d, struct fsi_master, dev)
> @@ -239,15 +241,15 @@ static inline uint32_t fsi_smode_sid(int x)
> return (x & FSI_SMODE_SID_MASK) << FSI_SMODE_SID_SHIFT;
> }
>
> -static uint32_t fsi_slave_smode(int id)
> +static uint32_t fsi_slave_smode(int id, u8 t_senddly, u8 t_echodly)
Can I buy you a vowel? :)
> {
> return FSI_SMODE_WSC | FSI_SMODE_ECRC
> | fsi_smode_sid(id)
> - | fsi_smode_echodly(0xf) | fsi_smode_senddly(0xf)
> + | fsi_smode_echodly(t_echodly - 1) | fsi_smode_senddly(t_senddly - 1)
> | fsi_smode_lbcrr(0x8);
> }
>
^ permalink raw reply
* [PATCH 02/14] fsi: Move code around to avoid forward declaration
From: Joel Stanley @ 2018-06-28 4:10 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180626232321.12372-3-benh@kernel.crashing.org>
On 27 June 2018 at 08:53, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> Move fsi_slave_set_smode() and its helpers to before it's
> first user and remove the corresponding forward declaration.
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Reviewed-by: Joel Stanley <joel@jms.id.au>
^ permalink raw reply
* [PATCH 01/14] devres: Add devm_of_iomap()
From: Joel Stanley @ 2018-06-28 4:10 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180626232109.10944-2-benh@kernel.crashing.org>
On 27 June 2018 at 08:50, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> There are still quite a few cases where a device might want
> to get to a different node of the device-tree, obtain the
> resources and map them.
>
> We have of_iomap() and of_io_request_and_map() but they both
> have shortcomings, such as not returning the size of the
> resource found (which can be useful) and not being "managed".
>
> This adds a devm_of_iomap() that provides all of these and
> should probably replace uses of the above in most drivers.
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Reviewed-by: Joel Stanley <joel@jms.id.au>
^ permalink raw reply
* [PATCH] i2c: aspeed: Improve driver to support multi-master use cases stably
From: Jae Hyun Yoo @ 2018-06-27 18:01 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <60d0dad1-b735-0650-47b4-40c57b1a5209@linux.intel.com>
Hi Jarkko,
Thanks for the review. Please see my answer below.
On 6/27/2018 12:48 AM, Jarkko Nikula wrote:
> Hi
>
> On 06/26/2018 07:58 PM, Jae Hyun Yoo wrote:
>> BMC firmware should support some multi-master use cases such as
>> multi-node,
>> IPMB, BMC-ME link and so on but the current ASPEED I2C driver is a bit
>> unstable for the multi-master use case. So this patch improves ASPEED I2C
>> driver to support the multi-master use case stably.
>>
>> Changes:
>> * Added XFER_MODE status register checking logic into
>> aspeed_i2c_master_xfer to improve the current bus busy checking logic.
>> * Changed the order of enum aspeed_i2c_master_state and
>> enum aspeed_i2c_slave_state defines to make their initial values
>> set to
>> ASPEED_I2C_MASTER_INACTIVE and ASPEED_I2C_SLAVE_STOP respectively.
>> In case of multi-master use with previous code, if a slave data comes
>> ahead of the first master xfer, master_state starts from an invalid
>> state. This change fixed the issue.
>> * Adjusted spin_lock scope to make it wrap the whole irq handler using
>> a single lock and unlock pair covers both master and slave handlers.
>> * Added irq_status variable as a member of the struct aspeed_i2c_bus to
>> collect handled interrupt bits throughout the master and the slave irq
>> handlers.
>> * Added control logic to put an order on calling the master and the slave
>> irq handlers based on their current states.
>>
> This does many unrelated looking changes in one patch making it more
> vulnerable for potential multiple regressions. For instance busy
> checking goes from single read to loop with 250 ms timeout in this patch
> while changing also spin lock logic and interrupt handling.
>
> Now if there is some regression it might be difficult to find what
> change in this patch is causing it and more over things goes more
> complicated if some other kind of regressions are found pointing into
> the same commit.
>
> I suggest splitting this into multiple smaller patches. For instance
> having first simple conversions patches that are unlikely to cause a
> regression like one patch adding '\n' to error print, another moving
> irq_status variable into struct aspeed_i2c_bus and so on followed by
> patches that change logic like busy checking, spin lock change and then
> patch or more for multi-master support.
>
Yes, that makes sense and I agree with you. I'll split out this patch
into multiple smaller patches as you suggested.
Thanks,
Jae
^ permalink raw reply
* [PATCH] i2c: aspeed: Improve driver to support multi-master use cases stably
From: Jae Hyun Yoo @ 2018-06-27 17:55 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <CAFd5g45z--ucXf-So517tgrayZT6x2TGvaUvaDF7zeRNn7CPXw@mail.gmail.com>
Thanks Brendan for the review. Please see my answers inline.
On 6/27/2018 12:36 AM, Brendan Higgins wrote:
> On Tue, Jun 26, 2018 at 9:58 AM Jae Hyun Yoo
> <jae.hyun.yoo@linux.intel.com> wrote:
>>
>> BMC firmware should support some multi-master use cases such as multi-node,
>> IPMB, BMC-ME link and so on but the current ASPEED I2C driver is a bit
>> unstable for the multi-master use case. So this patch improves ASPEED I2C
>> driver to support the multi-master use case stably.
>>
>> Changes:
>> * Added XFER_MODE status register checking logic into
>> aspeed_i2c_master_xfer to improve the current bus busy checking logic.
>> * Changed the order of enum aspeed_i2c_master_state and
>> enum aspeed_i2c_slave_state defines to make their initial values set to
>> ASPEED_I2C_MASTER_INACTIVE and ASPEED_I2C_SLAVE_STOP respectively.
>> In case of multi-master use with previous code, if a slave data comes
>> ahead of the first master xfer, master_state starts from an invalid
>> state. This change fixed the issue.
>> * Adjusted spin_lock scope to make it wrap the whole irq handler using
>> a single lock and unlock pair covers both master and slave handlers.
>> * Added irq_status variable as a member of the struct aspeed_i2c_bus to
>> collect handled interrupt bits throughout the master and the slave irq
>> handlers.
>> * Added control logic to put an order on calling the master and the slave
>> irq handlers based on their current states.
>>
>> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
>> ---
>> drivers/i2c/busses/i2c-aspeed.c | 200 +++++++++++++++++++-------------
>> 1 file changed, 118 insertions(+), 82 deletions(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
>> index 60e4d0e939a3..ac3e17d9a365 100644
>> --- a/drivers/i2c/busses/i2c-aspeed.c
>> +++ b/drivers/i2c/busses/i2c-aspeed.c
>> @@ -4,6 +4,7 @@
>> * Copyright (C) 2012-2017 ASPEED Technology Inc.
>> * Copyright 2017 IBM Corporation
>> * Copyright 2017 Google, Inc.
>> + * Copyright (c) 2018 Intel Corporation
>> *
>> * This program is free software; you can redistribute it and/or modify
>> * it under the terms of the GNU General Public License version 2 as
>> @@ -12,6 +13,7 @@
>>
>> #include <linux/clk.h>
>> #include <linux/completion.h>
>> +#include <linux/delay.h>
>> #include <linux/err.h>
>> #include <linux/errno.h>
>> #include <linux/i2c.h>
>> @@ -82,6 +84,11 @@
>> #define ASPEED_I2CD_INTR_RX_DONE BIT(2)
>> #define ASPEED_I2CD_INTR_TX_NAK BIT(1)
>> #define ASPEED_I2CD_INTR_TX_ACK BIT(0)
>> +#define ASPEED_I2CD_INTR_ERRORS \
>> + (ASPEED_I2CD_INTR_SDA_DL_TIMEOUT | \
>> + ASPEED_I2CD_INTR_SCL_TIMEOUT | \
>> + ASPEED_I2CD_INTR_ABNORMAL | \
>> + ASPEED_I2CD_INTR_ARBIT_LOSS)
>> #define ASPEED_I2CD_INTR_ALL \
>> (ASPEED_I2CD_INTR_SDA_DL_TIMEOUT | \
>> ASPEED_I2CD_INTR_BUS_RECOVER_DONE | \
>> @@ -94,6 +101,7 @@
>> ASPEED_I2CD_INTR_TX_ACK)
>>
>> /* 0x14 : I2CD Command/Status Register */
>> +#define ASPEED_I2CD_XFER_MODE_STS_MASK GENMASK(22, 19)
>> #define ASPEED_I2CD_SCL_LINE_STS BIT(18)
>> #define ASPEED_I2CD_SDA_LINE_STS BIT(17)
>> #define ASPEED_I2CD_BUS_BUSY_STS BIT(16)
>> @@ -110,23 +118,27 @@
>> /* 0x18 : I2CD Slave Device Address Register */
>> #define ASPEED_I2CD_DEV_ADDR_MASK GENMASK(6, 0)
>>
>> +/* Timeout for bus busy checking */
>> +#define BUS_BUSY_CHECK_TIMEOUT 250000 /* 250ms */
>> +#define BUS_BUSY_CHECK_INTERVAL 10000 /* 10ms */
>
> Could you add a comment on where you got these values from?
>
These are coming from ASPEED SDK code. Actually, they use 100ms for
timeout and 10ms for interval but I increased the timeout value to
250ms so that it covers a various range of bus speed. I think, it
should be computed at run time based on the current bus speed, or
we could add these as device tree settings. How do you think about it?
>
> Also, please use the same naming pattern as above.
>
Sure. Will change it using the same naming pattern as above.
>> +
>> enum aspeed_i2c_master_state {
>> + ASPEED_I2C_MASTER_INACTIVE,
>> ASPEED_I2C_MASTER_START,
>> ASPEED_I2C_MASTER_TX_FIRST,
>> ASPEED_I2C_MASTER_TX,
>> ASPEED_I2C_MASTER_RX_FIRST,
>> ASPEED_I2C_MASTER_RX,
>> ASPEED_I2C_MASTER_STOP,
>> - ASPEED_I2C_MASTER_INACTIVE,
>> };
>>
>> enum aspeed_i2c_slave_state {
>> + ASPEED_I2C_SLAVE_STOP,
>> ASPEED_I2C_SLAVE_START,
>> ASPEED_I2C_SLAVE_READ_REQUESTED,
>> ASPEED_I2C_SLAVE_READ_PROCESSED,
>> ASPEED_I2C_SLAVE_WRITE_REQUESTED,
>> ASPEED_I2C_SLAVE_WRITE_RECEIVED,
>> - ASPEED_I2C_SLAVE_STOP,
>> };
>>
>> struct aspeed_i2c_bus {
>> @@ -150,6 +162,7 @@ struct aspeed_i2c_bus {
>> int cmd_err;
>> /* Protected only by i2c_lock_bus */
>> int master_xfer_result;
>> + u32 irq_status;
>> #if IS_ENABLED(CONFIG_I2C_SLAVE)
>> struct i2c_client *slave;
>> enum aspeed_i2c_slave_state slave_state;
>> @@ -229,37 +242,30 @@ static int aspeed_i2c_recover_bus(struct aspeed_i2c_bus *bus)
>> #if IS_ENABLED(CONFIG_I2C_SLAVE)
>> static bool aspeed_i2c_slave_irq(struct aspeed_i2c_bus *bus)
>> {
>> - u32 command, irq_status, status_ack = 0;
>> + u32 command, status_ack = 0;
>> struct i2c_client *slave = bus->slave;
>> - bool irq_handled = true;
>> u8 value;
>>
>> - spin_lock(&bus->lock);
>> - if (!slave) {
>> - irq_handled = false;
>> - goto out;
>> - }
>> + if (!slave)
>> + return false;
>>
>> command = readl(bus->base + ASPEED_I2C_CMD_REG);
>> - irq_status = readl(bus->base + ASPEED_I2C_INTR_STS_REG);
>>
>> /* Slave was requested, restart state machine. */
>> - if (irq_status & ASPEED_I2CD_INTR_SLAVE_MATCH) {
>> + if (bus->irq_status & ASPEED_I2CD_INTR_SLAVE_MATCH) {
>> status_ack |= ASPEED_I2CD_INTR_SLAVE_MATCH;
>> bus->slave_state = ASPEED_I2C_SLAVE_START;
>> }
>>
>> /* Slave is not currently active, irq was for someone else. */
>> - if (bus->slave_state == ASPEED_I2C_SLAVE_STOP) {
>> - irq_handled = false;
>> - goto out;
>> - }
>> + if (bus->slave_state == ASPEED_I2C_SLAVE_STOP)
>> + return false;
>>
>> dev_dbg(bus->dev, "slave irq status 0x%08x, cmd 0x%08x\n",
>> - irq_status, command);
>> + bus->irq_status, command);
>>
>> /* Slave was sent something. */
>> - if (irq_status & ASPEED_I2CD_INTR_RX_DONE) {
>> + if (bus->irq_status & ASPEED_I2CD_INTR_RX_DONE) {
>> value = readl(bus->base + ASPEED_I2C_BYTE_BUF_REG) >> 8;
>> /* Handle address frame. */
>> if (bus->slave_state == ASPEED_I2C_SLAVE_START) {
>> @@ -274,28 +280,29 @@ static bool aspeed_i2c_slave_irq(struct aspeed_i2c_bus *bus)
>> }
>>
>> /* Slave was asked to stop. */
>> - if (irq_status & ASPEED_I2CD_INTR_NORMAL_STOP) {
>> + if (bus->irq_status & ASPEED_I2CD_INTR_NORMAL_STOP) {
>> status_ack |= ASPEED_I2CD_INTR_NORMAL_STOP;
>> bus->slave_state = ASPEED_I2C_SLAVE_STOP;
>> }
>> - if (irq_status & ASPEED_I2CD_INTR_TX_NAK) {
>> + if (bus->irq_status & ASPEED_I2CD_INTR_TX_NAK) {
>> status_ack |= ASPEED_I2CD_INTR_TX_NAK;
>> bus->slave_state = ASPEED_I2C_SLAVE_STOP;
>> }
>> + if (bus->irq_status & ASPEED_I2CD_INTR_TX_ACK) {
>> + status_ack |= ASPEED_I2CD_INTR_TX_ACK;
>> + }
>>
>> switch (bus->slave_state) {
>> case ASPEED_I2C_SLAVE_READ_REQUESTED:
>> - if (irq_status & ASPEED_I2CD_INTR_TX_ACK)
>> + if (bus->irq_status & ASPEED_I2CD_INTR_TX_ACK)
>> dev_err(bus->dev, "Unexpected ACK on read request.\n");
>> bus->slave_state = ASPEED_I2C_SLAVE_READ_PROCESSED;
>> -
>> i2c_slave_event(slave, I2C_SLAVE_READ_REQUESTED, &value);
>> writel(value, bus->base + ASPEED_I2C_BYTE_BUF_REG);
>> writel(ASPEED_I2CD_S_TX_CMD, bus->base + ASPEED_I2C_CMD_REG);
>> break;
>> case ASPEED_I2C_SLAVE_READ_PROCESSED:
>> - status_ack |= ASPEED_I2CD_INTR_TX_ACK;
>> - if (!(irq_status & ASPEED_I2CD_INTR_TX_ACK))
>> + if (!(bus->irq_status & ASPEED_I2CD_INTR_TX_ACK))
>> dev_err(bus->dev,
>> "Expected ACK after processed read.\n");
>> i2c_slave_event(slave, I2C_SLAVE_READ_PROCESSED, &value);
>> @@ -318,15 +325,8 @@ static bool aspeed_i2c_slave_irq(struct aspeed_i2c_bus *bus)
>> break;
>> }
>>
>> - if (status_ack != irq_status)
>> - dev_err(bus->dev,
>> - "irq handled != irq. expected %x, but was %x\n",
>> - irq_status, status_ack);
>> - writel(status_ack, bus->base + ASPEED_I2C_INTR_STS_REG);
>> -
>> -out:
>> - spin_unlock(&bus->lock);
>> - return irq_handled;
>> + bus->irq_status ^= status_ack;
>> + return !bus->irq_status;
>> }
>> #endif /* CONFIG_I2C_SLAVE */
>>
>> @@ -384,20 +384,19 @@ static int aspeed_i2c_is_irq_error(u32 irq_status)
>>
>> static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
>> {
>> - u32 irq_status, status_ack = 0, command = 0;
>> + u32 status_ack = 0, command = 0;
>> struct i2c_msg *msg;
>> u8 recv_byte;
>> int ret;
>>
>> - spin_lock(&bus->lock);
>> - irq_status = readl(bus->base + ASPEED_I2C_INTR_STS_REG);
>> - /* Ack all interrupt bits. */
>> - writel(irq_status, bus->base + ASPEED_I2C_INTR_STS_REG);
>> -
>> - if (irq_status & ASPEED_I2CD_INTR_BUS_RECOVER_DONE) {
>> + if (bus->irq_status & ASPEED_I2CD_INTR_BUS_RECOVER_DONE) {
>> bus->master_state = ASPEED_I2C_MASTER_INACTIVE;
>> status_ack |= ASPEED_I2CD_INTR_BUS_RECOVER_DONE;
>> goto out_complete;
>> + } else {
>> + /* Master is not currently active, irq was for someone else. */
>> + if (bus->master_state == ASPEED_I2C_MASTER_INACTIVE)
>> + goto out_no_complete;
>> }
>>
>> /*
>> @@ -405,20 +404,23 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
>> * should clear the command queue effectively taking us back to the
>> * INACTIVE state.
>> */
>> - ret = aspeed_i2c_is_irq_error(irq_status);
>> - if (ret < 0) {
>> - dev_dbg(bus->dev, "received error interrupt: 0x%08x",
>> - irq_status);
>> + ret = aspeed_i2c_is_irq_error(bus->irq_status);
>> + if (ret) {
>> + dev_dbg(bus->dev, "received error interrupt: 0x%08x\n",
>> + bus->irq_status);
>> bus->cmd_err = ret;
>> bus->master_state = ASPEED_I2C_MASTER_INACTIVE;
>> + status_ack |= (bus->irq_status & ASPEED_I2CD_INTR_ERRORS);
>> goto out_complete;
>> }
>>
>> /* We are in an invalid state; reset bus to a known state. */
>> if (!bus->msgs) {
>> - dev_err(bus->dev, "bus in unknown state");
>> + dev_err(bus->dev, "bus in unknown state irq_status: 0x%x\n",
>> + bus->irq_status);
>> bus->cmd_err = -EIO;
>> - if (bus->master_state != ASPEED_I2C_MASTER_STOP)
>> + if (bus->master_state != ASPEED_I2C_MASTER_STOP &&
>> + bus->master_state != ASPEED_I2C_MASTER_INACTIVE)
>> aspeed_i2c_do_stop(bus);
>> goto out_no_complete;
>> }
>> @@ -430,7 +432,13 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
>> * then update the state and handle the new state below.
>> */
>> if (bus->master_state == ASPEED_I2C_MASTER_START) {
>> - if (unlikely(!(irq_status & ASPEED_I2CD_INTR_TX_ACK))) {
>> + if (unlikely(!(bus->irq_status & ASPEED_I2CD_INTR_TX_ACK))) {
>> + if (unlikely(!(bus->irq_status &
>> + ASPEED_I2CD_INTR_TX_NAK))) {
>> + bus->cmd_err = -ENXIO;
>> + bus->master_state = ASPEED_I2C_MASTER_INACTIVE;
>> + goto out_complete;
>> + }
>> pr_devel("no slave present at %02x", msg->addr);
>> status_ack |= ASPEED_I2CD_INTR_TX_NAK;
>> bus->cmd_err = -ENXIO;
>> @@ -450,12 +458,13 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
>>
>> switch (bus->master_state) {
>> case ASPEED_I2C_MASTER_TX:
>> - if (unlikely(irq_status & ASPEED_I2CD_INTR_TX_NAK)) {
>> - dev_dbg(bus->dev, "slave NACKed TX");
>> + if (unlikely(bus->irq_status & ASPEED_I2CD_INTR_TX_NAK)) {
>> + dev_dbg(bus->dev, "slave NACKed TX\n");
>> status_ack |= ASPEED_I2CD_INTR_TX_NAK;
>> goto error_and_stop;
>> - } else if (unlikely(!(irq_status & ASPEED_I2CD_INTR_TX_ACK))) {
>> - dev_err(bus->dev, "slave failed to ACK TX");
>> + } else if (unlikely(!(bus->irq_status &
>> + ASPEED_I2CD_INTR_TX_ACK))) {
>> + dev_err(bus->dev, "slave failed to ACK TX\n");
>> goto error_and_stop;
>> }
>> status_ack |= ASPEED_I2CD_INTR_TX_ACK;
>> @@ -473,12 +482,12 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
>> goto out_no_complete;
>> case ASPEED_I2C_MASTER_RX_FIRST:
>> /* RX may not have completed yet (only address cycle) */
>> - if (!(irq_status & ASPEED_I2CD_INTR_RX_DONE))
>> + if (!(bus->irq_status & ASPEED_I2CD_INTR_RX_DONE))
>> goto out_no_complete;
>> /* fallthrough intended */
>> case ASPEED_I2C_MASTER_RX:
>> - if (unlikely(!(irq_status & ASPEED_I2CD_INTR_RX_DONE))) {
>> - dev_err(bus->dev, "master failed to RX");
>> + if (unlikely(!(bus->irq_status & ASPEED_I2CD_INTR_RX_DONE))) {
>> + dev_err(bus->dev, "master failed to RX\n");
>> goto error_and_stop;
>> }
>> status_ack |= ASPEED_I2CD_INTR_RX_DONE;
>> @@ -508,8 +517,11 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
>> }
>> goto out_no_complete;
>> case ASPEED_I2C_MASTER_STOP:
>> - if (unlikely(!(irq_status & ASPEED_I2CD_INTR_NORMAL_STOP))) {
>> - dev_err(bus->dev, "master failed to STOP");
>> + if (unlikely(!(bus->irq_status &
>> + ASPEED_I2CD_INTR_NORMAL_STOP))) {
>> + dev_err(bus->dev,
>> + "master failed to STOP irq_status:0x%x\n",
>> + bus->irq_status);
>> bus->cmd_err = -EIO;
>> /* Do not STOP as we have already tried. */
>> } else {
>> @@ -520,8 +532,8 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
>> goto out_complete;
>> case ASPEED_I2C_MASTER_INACTIVE:
>> dev_err(bus->dev,
>> - "master received interrupt 0x%08x, but is inactive",
>> - irq_status);
>> + "master received interrupt 0x%08x, but is inactive\n",
>> + bus->irq_status);
>> bus->cmd_err = -EIO;
>> /* Do not STOP as we should be inactive. */
>> goto out_complete;
>> @@ -543,26 +555,61 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
>> bus->master_xfer_result = bus->msgs_index + 1;
>> complete(&bus->cmd_complete);
>> out_no_complete:
>> - if (irq_status != status_ack)
>> - dev_err(bus->dev,
>> - "irq handled != irq. expected 0x%08x, but was 0x%08x\n",
>> - irq_status, status_ack);
>> - spin_unlock(&bus->lock);
>> - return !!irq_status;
>> + bus->irq_status ^= status_ack;
>> + return !bus->irq_status;
>> }
>>
>> static irqreturn_t aspeed_i2c_bus_irq(int irq, void *dev_id)
>> {
>> struct aspeed_i2c_bus *bus = dev_id;
>> + u32 irq_received;
>> +
>> + spin_lock(&bus->lock);
>> + irq_received = readl(bus->base + ASPEED_I2C_INTR_STS_REG);
>> + bus->irq_status = irq_received;
>>
>> #if IS_ENABLED(CONFIG_I2C_SLAVE)
>> - if (aspeed_i2c_slave_irq(bus)) {
>> - dev_dbg(bus->dev, "irq handled by slave.\n");
>> - return IRQ_HANDLED;
>> + if (bus->master_state != ASPEED_I2C_MASTER_INACTIVE) {
>> + if (!aspeed_i2c_master_irq(bus))
>
> Why do you check the slave if master fails (or vice versa)? I
> understand that there are some status bits that have not been handled,
> but it doesn't seem reasonable to assume that there is state that the
> other should do something with; the only way this would happen is if
> the state that you think you are in does not match the status bits you
> have been given, but if this is the case, you are already hosed; I
> don't think trying the other handler is likely to make things better,
> unless there is something that I am missing.
>
In most of cases, interrupt bits are set one by one but there are also a
lot of other cases that ASPEED I2C H/W sends multiple interrupt bits
with combining master and slave events using a single interrupt call. It
happens much in multi-master environment than single-master. For
example, when master is waiting for a NORMAL_STOP interrupt in its
MASTER_STOP state, SLAVE_MATCH and RX_DONE interrupts could come along
with the NORMAL_STOP in case of an another master immediately sends data
just after acquiring the bus - it happens a lot in BMC-ME connection
practically. In this case, the NORMAL_STOP interrupt should be handled
by master_irq and the SLAVE_MATCH and RX_DONE interrupts should be
handled by slave_irq so it's the reason why this code is added.
>> + aspeed_i2c_slave_irq(bus);
>> + } else {
>> + if (!aspeed_i2c_slave_irq(bus))
>> + aspeed_i2c_master_irq(bus);
>> }
>> +#else
>> + aspeed_i2c_master_irq(bus);
>> #endif /* CONFIG_I2C_SLAVE */
>>
>> - return aspeed_i2c_master_irq(bus) ? IRQ_HANDLED : IRQ_NONE;
>> + if (bus->irq_status)
>> + dev_err(bus->dev,
>> + "irq handled != irq. expected 0x%08x, but was 0x%08x\n",
>> + irq_received, irq_received ^ bus->irq_status);
>> +
>> + /* Ack all interrupt bits. */
>> + writel(irq_received, bus->base + ASPEED_I2C_INTR_STS_REG);
>> + spin_unlock(&bus->lock);
>> + return bus->irq_status ? IRQ_NONE : IRQ_HANDLED;
>> +}
>> +
>> +static int aspeed_i2c_check_bus_busy_timeout(struct aspeed_i2c_bus *bus)
>> +{
>> + ktime_t timeout = ktime_add_us(ktime_get(), BUS_BUSY_CHECK_TIMEOUT);
>> +
>> + might_sleep();
>> +
>> + for (;;) {
>> + if (!(readl(bus->base + ASPEED_I2C_CMD_REG) &
>> + (ASPEED_I2CD_BUS_BUSY_STS |
>> + ASPEED_I2CD_XFER_MODE_STS_MASK)))
>
> Is using the Transfer Mode State Machine bits necessary? The
> documentation marks it as "for debugging purpose only," so relying on
> it makes me nervous.
>
As you said, the documentation marks it as "for debugging purpose only."
but ASPEED also uses this way in their SDK code because it's the best
way for checking bus busy status which can cover both single and
multi-master use cases.
>> + return 0;
>> + if (ktime_compare(ktime_get(), timeout) > 0)
>> + break;
>> + usleep_range((BUS_BUSY_CHECK_INTERVAL >> 2) + 1,
>
> Where did you get this minimum value?
>
No source for the minimum value. ASPEED uses mdelay(10) in their SDK
but I changed that code using usleep_range and the range value was set
with considering time stretching of usleep_range.
regmap_read_poll_timeout was a reference for this code.
Thanks,
Jae
>> + BUS_BUSY_CHECK_INTERVAL);
>> + }
>> +
>> + dev_err(bus->dev, "timeout waiting for idle. attempting recovery\n");
>> + return aspeed_i2c_recover_bus(bus);
>> }
>>
>> static int aspeed_i2c_master_xfer(struct i2c_adapter *adap,
>> @@ -570,22 +617,11 @@ static int aspeed_i2c_master_xfer(struct i2c_adapter *adap,
>> {
>> struct aspeed_i2c_bus *bus = i2c_get_adapdata(adap);
>> unsigned long time_left, flags;
>> - int ret = 0;
>> -
>> - spin_lock_irqsave(&bus->lock, flags);
>> - bus->cmd_err = 0;
>>
>> - /* If bus is busy, attempt recovery. We assume a single master
>> - * environment.
>> - */
>> - if (readl(bus->base + ASPEED_I2C_CMD_REG) & ASPEED_I2CD_BUS_BUSY_STS) {
>> - spin_unlock_irqrestore(&bus->lock, flags);
>> - ret = aspeed_i2c_recover_bus(bus);
>> - if (ret)
>> - return ret;
>> - spin_lock_irqsave(&bus->lock, flags);
>> - }
>> + if (aspeed_i2c_check_bus_busy_timeout(bus))
>> + return -EAGAIN;
>>
>> + spin_lock_irqsave(&bus->lock, flags);
>> bus->cmd_err = 0;
>> bus->msgs = msgs;
>> bus->msgs_index = 0;
>> @@ -851,7 +887,7 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
>> bus->rst = devm_reset_control_get_shared(&pdev->dev, NULL);
>> if (IS_ERR(bus->rst)) {
>> dev_err(&pdev->dev,
>> - "missing or invalid reset controller device tree entry");
>> + "missing or invalid reset controller device tree entry\n");
>> return PTR_ERR(bus->rst);
>> }
>> reset_control_deassert(bus->rst);
>> --
>> 2.17.1
>>
^ permalink raw reply
* [PATCH] i2c: aspeed: Improve driver to support multi-master use cases stably
From: Jarkko Nikula @ 2018-06-27 7:48 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180626165812.4141-1-jae.hyun.yoo@linux.intel.com>
Hi
On 06/26/2018 07:58 PM, Jae Hyun Yoo wrote:
> BMC firmware should support some multi-master use cases such as multi-node,
> IPMB, BMC-ME link and so on but the current ASPEED I2C driver is a bit
> unstable for the multi-master use case. So this patch improves ASPEED I2C
> driver to support the multi-master use case stably.
>
> Changes:
> * Added XFER_MODE status register checking logic into
> aspeed_i2c_master_xfer to improve the current bus busy checking logic.
> * Changed the order of enum aspeed_i2c_master_state and
> enum aspeed_i2c_slave_state defines to make their initial values set to
> ASPEED_I2C_MASTER_INACTIVE and ASPEED_I2C_SLAVE_STOP respectively.
> In case of multi-master use with previous code, if a slave data comes
> ahead of the first master xfer, master_state starts from an invalid
> state. This change fixed the issue.
> * Adjusted spin_lock scope to make it wrap the whole irq handler using
> a single lock and unlock pair covers both master and slave handlers.
> * Added irq_status variable as a member of the struct aspeed_i2c_bus to
> collect handled interrupt bits throughout the master and the slave irq
> handlers.
> * Added control logic to put an order on calling the master and the slave
> irq handlers based on their current states.
>
This does many unrelated looking changes in one patch making it more
vulnerable for potential multiple regressions. For instance busy
checking goes from single read to loop with 250 ms timeout in this patch
while changing also spin lock logic and interrupt handling.
Now if there is some regression it might be difficult to find what
change in this patch is causing it and more over things goes more
complicated if some other kind of regressions are found pointing into
the same commit.
I suggest splitting this into multiple smaller patches. For instance
having first simple conversions patches that are unlikely to cause a
regression like one patch adding '\n' to error print, another moving
irq_status variable into struct aspeed_i2c_bus and so on followed by
patches that change logic like busy checking, spin lock change and then
patch or more for multi-master support.
--
Jarkko
^ permalink raw reply
* [PATCH] i2c: aspeed: Improve driver to support multi-master use cases stably
From: Brendan Higgins @ 2018-06-27 7:36 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180626165812.4141-1-jae.hyun.yoo@linux.intel.com>
On Tue, Jun 26, 2018 at 9:58 AM Jae Hyun Yoo
<jae.hyun.yoo@linux.intel.com> wrote:
>
> BMC firmware should support some multi-master use cases such as multi-node,
> IPMB, BMC-ME link and so on but the current ASPEED I2C driver is a bit
> unstable for the multi-master use case. So this patch improves ASPEED I2C
> driver to support the multi-master use case stably.
>
> Changes:
> * Added XFER_MODE status register checking logic into
> aspeed_i2c_master_xfer to improve the current bus busy checking logic.
> * Changed the order of enum aspeed_i2c_master_state and
> enum aspeed_i2c_slave_state defines to make their initial values set to
> ASPEED_I2C_MASTER_INACTIVE and ASPEED_I2C_SLAVE_STOP respectively.
> In case of multi-master use with previous code, if a slave data comes
> ahead of the first master xfer, master_state starts from an invalid
> state. This change fixed the issue.
> * Adjusted spin_lock scope to make it wrap the whole irq handler using
> a single lock and unlock pair covers both master and slave handlers.
> * Added irq_status variable as a member of the struct aspeed_i2c_bus to
> collect handled interrupt bits throughout the master and the slave irq
> handlers.
> * Added control logic to put an order on calling the master and the slave
> irq handlers based on their current states.
>
> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
> ---
> drivers/i2c/busses/i2c-aspeed.c | 200 +++++++++++++++++++-------------
> 1 file changed, 118 insertions(+), 82 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
> index 60e4d0e939a3..ac3e17d9a365 100644
> --- a/drivers/i2c/busses/i2c-aspeed.c
> +++ b/drivers/i2c/busses/i2c-aspeed.c
> @@ -4,6 +4,7 @@
> * Copyright (C) 2012-2017 ASPEED Technology Inc.
> * Copyright 2017 IBM Corporation
> * Copyright 2017 Google, Inc.
> + * Copyright (c) 2018 Intel Corporation
> *
> * This program is free software; you can redistribute it and/or modify
> * it under the terms of the GNU General Public License version 2 as
> @@ -12,6 +13,7 @@
>
> #include <linux/clk.h>
> #include <linux/completion.h>
> +#include <linux/delay.h>
> #include <linux/err.h>
> #include <linux/errno.h>
> #include <linux/i2c.h>
> @@ -82,6 +84,11 @@
> #define ASPEED_I2CD_INTR_RX_DONE BIT(2)
> #define ASPEED_I2CD_INTR_TX_NAK BIT(1)
> #define ASPEED_I2CD_INTR_TX_ACK BIT(0)
> +#define ASPEED_I2CD_INTR_ERRORS \
> + (ASPEED_I2CD_INTR_SDA_DL_TIMEOUT | \
> + ASPEED_I2CD_INTR_SCL_TIMEOUT | \
> + ASPEED_I2CD_INTR_ABNORMAL | \
> + ASPEED_I2CD_INTR_ARBIT_LOSS)
> #define ASPEED_I2CD_INTR_ALL \
> (ASPEED_I2CD_INTR_SDA_DL_TIMEOUT | \
> ASPEED_I2CD_INTR_BUS_RECOVER_DONE | \
> @@ -94,6 +101,7 @@
> ASPEED_I2CD_INTR_TX_ACK)
>
> /* 0x14 : I2CD Command/Status Register */
> +#define ASPEED_I2CD_XFER_MODE_STS_MASK GENMASK(22, 19)
> #define ASPEED_I2CD_SCL_LINE_STS BIT(18)
> #define ASPEED_I2CD_SDA_LINE_STS BIT(17)
> #define ASPEED_I2CD_BUS_BUSY_STS BIT(16)
> @@ -110,23 +118,27 @@
> /* 0x18 : I2CD Slave Device Address Register */
> #define ASPEED_I2CD_DEV_ADDR_MASK GENMASK(6, 0)
>
> +/* Timeout for bus busy checking */
> +#define BUS_BUSY_CHECK_TIMEOUT 250000 /* 250ms */
> +#define BUS_BUSY_CHECK_INTERVAL 10000 /* 10ms */
Could you add a comment on where you got these values from?
Also, please use the same naming pattern as above.
> +
> enum aspeed_i2c_master_state {
> + ASPEED_I2C_MASTER_INACTIVE,
> ASPEED_I2C_MASTER_START,
> ASPEED_I2C_MASTER_TX_FIRST,
> ASPEED_I2C_MASTER_TX,
> ASPEED_I2C_MASTER_RX_FIRST,
> ASPEED_I2C_MASTER_RX,
> ASPEED_I2C_MASTER_STOP,
> - ASPEED_I2C_MASTER_INACTIVE,
> };
>
> enum aspeed_i2c_slave_state {
> + ASPEED_I2C_SLAVE_STOP,
> ASPEED_I2C_SLAVE_START,
> ASPEED_I2C_SLAVE_READ_REQUESTED,
> ASPEED_I2C_SLAVE_READ_PROCESSED,
> ASPEED_I2C_SLAVE_WRITE_REQUESTED,
> ASPEED_I2C_SLAVE_WRITE_RECEIVED,
> - ASPEED_I2C_SLAVE_STOP,
> };
>
> struct aspeed_i2c_bus {
> @@ -150,6 +162,7 @@ struct aspeed_i2c_bus {
> int cmd_err;
> /* Protected only by i2c_lock_bus */
> int master_xfer_result;
> + u32 irq_status;
> #if IS_ENABLED(CONFIG_I2C_SLAVE)
> struct i2c_client *slave;
> enum aspeed_i2c_slave_state slave_state;
> @@ -229,37 +242,30 @@ static int aspeed_i2c_recover_bus(struct aspeed_i2c_bus *bus)
> #if IS_ENABLED(CONFIG_I2C_SLAVE)
> static bool aspeed_i2c_slave_irq(struct aspeed_i2c_bus *bus)
> {
> - u32 command, irq_status, status_ack = 0;
> + u32 command, status_ack = 0;
> struct i2c_client *slave = bus->slave;
> - bool irq_handled = true;
> u8 value;
>
> - spin_lock(&bus->lock);
> - if (!slave) {
> - irq_handled = false;
> - goto out;
> - }
> + if (!slave)
> + return false;
>
> command = readl(bus->base + ASPEED_I2C_CMD_REG);
> - irq_status = readl(bus->base + ASPEED_I2C_INTR_STS_REG);
>
> /* Slave was requested, restart state machine. */
> - if (irq_status & ASPEED_I2CD_INTR_SLAVE_MATCH) {
> + if (bus->irq_status & ASPEED_I2CD_INTR_SLAVE_MATCH) {
> status_ack |= ASPEED_I2CD_INTR_SLAVE_MATCH;
> bus->slave_state = ASPEED_I2C_SLAVE_START;
> }
>
> /* Slave is not currently active, irq was for someone else. */
> - if (bus->slave_state == ASPEED_I2C_SLAVE_STOP) {
> - irq_handled = false;
> - goto out;
> - }
> + if (bus->slave_state == ASPEED_I2C_SLAVE_STOP)
> + return false;
>
> dev_dbg(bus->dev, "slave irq status 0x%08x, cmd 0x%08x\n",
> - irq_status, command);
> + bus->irq_status, command);
>
> /* Slave was sent something. */
> - if (irq_status & ASPEED_I2CD_INTR_RX_DONE) {
> + if (bus->irq_status & ASPEED_I2CD_INTR_RX_DONE) {
> value = readl(bus->base + ASPEED_I2C_BYTE_BUF_REG) >> 8;
> /* Handle address frame. */
> if (bus->slave_state == ASPEED_I2C_SLAVE_START) {
> @@ -274,28 +280,29 @@ static bool aspeed_i2c_slave_irq(struct aspeed_i2c_bus *bus)
> }
>
> /* Slave was asked to stop. */
> - if (irq_status & ASPEED_I2CD_INTR_NORMAL_STOP) {
> + if (bus->irq_status & ASPEED_I2CD_INTR_NORMAL_STOP) {
> status_ack |= ASPEED_I2CD_INTR_NORMAL_STOP;
> bus->slave_state = ASPEED_I2C_SLAVE_STOP;
> }
> - if (irq_status & ASPEED_I2CD_INTR_TX_NAK) {
> + if (bus->irq_status & ASPEED_I2CD_INTR_TX_NAK) {
> status_ack |= ASPEED_I2CD_INTR_TX_NAK;
> bus->slave_state = ASPEED_I2C_SLAVE_STOP;
> }
> + if (bus->irq_status & ASPEED_I2CD_INTR_TX_ACK) {
> + status_ack |= ASPEED_I2CD_INTR_TX_ACK;
> + }
>
> switch (bus->slave_state) {
> case ASPEED_I2C_SLAVE_READ_REQUESTED:
> - if (irq_status & ASPEED_I2CD_INTR_TX_ACK)
> + if (bus->irq_status & ASPEED_I2CD_INTR_TX_ACK)
> dev_err(bus->dev, "Unexpected ACK on read request.\n");
> bus->slave_state = ASPEED_I2C_SLAVE_READ_PROCESSED;
> -
> i2c_slave_event(slave, I2C_SLAVE_READ_REQUESTED, &value);
> writel(value, bus->base + ASPEED_I2C_BYTE_BUF_REG);
> writel(ASPEED_I2CD_S_TX_CMD, bus->base + ASPEED_I2C_CMD_REG);
> break;
> case ASPEED_I2C_SLAVE_READ_PROCESSED:
> - status_ack |= ASPEED_I2CD_INTR_TX_ACK;
> - if (!(irq_status & ASPEED_I2CD_INTR_TX_ACK))
> + if (!(bus->irq_status & ASPEED_I2CD_INTR_TX_ACK))
> dev_err(bus->dev,
> "Expected ACK after processed read.\n");
> i2c_slave_event(slave, I2C_SLAVE_READ_PROCESSED, &value);
> @@ -318,15 +325,8 @@ static bool aspeed_i2c_slave_irq(struct aspeed_i2c_bus *bus)
> break;
> }
>
> - if (status_ack != irq_status)
> - dev_err(bus->dev,
> - "irq handled != irq. expected %x, but was %x\n",
> - irq_status, status_ack);
> - writel(status_ack, bus->base + ASPEED_I2C_INTR_STS_REG);
> -
> -out:
> - spin_unlock(&bus->lock);
> - return irq_handled;
> + bus->irq_status ^= status_ack;
> + return !bus->irq_status;
> }
> #endif /* CONFIG_I2C_SLAVE */
>
> @@ -384,20 +384,19 @@ static int aspeed_i2c_is_irq_error(u32 irq_status)
>
> static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
> {
> - u32 irq_status, status_ack = 0, command = 0;
> + u32 status_ack = 0, command = 0;
> struct i2c_msg *msg;
> u8 recv_byte;
> int ret;
>
> - spin_lock(&bus->lock);
> - irq_status = readl(bus->base + ASPEED_I2C_INTR_STS_REG);
> - /* Ack all interrupt bits. */
> - writel(irq_status, bus->base + ASPEED_I2C_INTR_STS_REG);
> -
> - if (irq_status & ASPEED_I2CD_INTR_BUS_RECOVER_DONE) {
> + if (bus->irq_status & ASPEED_I2CD_INTR_BUS_RECOVER_DONE) {
> bus->master_state = ASPEED_I2C_MASTER_INACTIVE;
> status_ack |= ASPEED_I2CD_INTR_BUS_RECOVER_DONE;
> goto out_complete;
> + } else {
> + /* Master is not currently active, irq was for someone else. */
> + if (bus->master_state == ASPEED_I2C_MASTER_INACTIVE)
> + goto out_no_complete;
> }
>
> /*
> @@ -405,20 +404,23 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
> * should clear the command queue effectively taking us back to the
> * INACTIVE state.
> */
> - ret = aspeed_i2c_is_irq_error(irq_status);
> - if (ret < 0) {
> - dev_dbg(bus->dev, "received error interrupt: 0x%08x",
> - irq_status);
> + ret = aspeed_i2c_is_irq_error(bus->irq_status);
> + if (ret) {
> + dev_dbg(bus->dev, "received error interrupt: 0x%08x\n",
> + bus->irq_status);
> bus->cmd_err = ret;
> bus->master_state = ASPEED_I2C_MASTER_INACTIVE;
> + status_ack |= (bus->irq_status & ASPEED_I2CD_INTR_ERRORS);
> goto out_complete;
> }
>
> /* We are in an invalid state; reset bus to a known state. */
> if (!bus->msgs) {
> - dev_err(bus->dev, "bus in unknown state");
> + dev_err(bus->dev, "bus in unknown state irq_status: 0x%x\n",
> + bus->irq_status);
> bus->cmd_err = -EIO;
> - if (bus->master_state != ASPEED_I2C_MASTER_STOP)
> + if (bus->master_state != ASPEED_I2C_MASTER_STOP &&
> + bus->master_state != ASPEED_I2C_MASTER_INACTIVE)
> aspeed_i2c_do_stop(bus);
> goto out_no_complete;
> }
> @@ -430,7 +432,13 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
> * then update the state and handle the new state below.
> */
> if (bus->master_state == ASPEED_I2C_MASTER_START) {
> - if (unlikely(!(irq_status & ASPEED_I2CD_INTR_TX_ACK))) {
> + if (unlikely(!(bus->irq_status & ASPEED_I2CD_INTR_TX_ACK))) {
> + if (unlikely(!(bus->irq_status &
> + ASPEED_I2CD_INTR_TX_NAK))) {
> + bus->cmd_err = -ENXIO;
> + bus->master_state = ASPEED_I2C_MASTER_INACTIVE;
> + goto out_complete;
> + }
> pr_devel("no slave present at %02x", msg->addr);
> status_ack |= ASPEED_I2CD_INTR_TX_NAK;
> bus->cmd_err = -ENXIO;
> @@ -450,12 +458,13 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
>
> switch (bus->master_state) {
> case ASPEED_I2C_MASTER_TX:
> - if (unlikely(irq_status & ASPEED_I2CD_INTR_TX_NAK)) {
> - dev_dbg(bus->dev, "slave NACKed TX");
> + if (unlikely(bus->irq_status & ASPEED_I2CD_INTR_TX_NAK)) {
> + dev_dbg(bus->dev, "slave NACKed TX\n");
> status_ack |= ASPEED_I2CD_INTR_TX_NAK;
> goto error_and_stop;
> - } else if (unlikely(!(irq_status & ASPEED_I2CD_INTR_TX_ACK))) {
> - dev_err(bus->dev, "slave failed to ACK TX");
> + } else if (unlikely(!(bus->irq_status &
> + ASPEED_I2CD_INTR_TX_ACK))) {
> + dev_err(bus->dev, "slave failed to ACK TX\n");
> goto error_and_stop;
> }
> status_ack |= ASPEED_I2CD_INTR_TX_ACK;
> @@ -473,12 +482,12 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
> goto out_no_complete;
> case ASPEED_I2C_MASTER_RX_FIRST:
> /* RX may not have completed yet (only address cycle) */
> - if (!(irq_status & ASPEED_I2CD_INTR_RX_DONE))
> + if (!(bus->irq_status & ASPEED_I2CD_INTR_RX_DONE))
> goto out_no_complete;
> /* fallthrough intended */
> case ASPEED_I2C_MASTER_RX:
> - if (unlikely(!(irq_status & ASPEED_I2CD_INTR_RX_DONE))) {
> - dev_err(bus->dev, "master failed to RX");
> + if (unlikely(!(bus->irq_status & ASPEED_I2CD_INTR_RX_DONE))) {
> + dev_err(bus->dev, "master failed to RX\n");
> goto error_and_stop;
> }
> status_ack |= ASPEED_I2CD_INTR_RX_DONE;
> @@ -508,8 +517,11 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
> }
> goto out_no_complete;
> case ASPEED_I2C_MASTER_STOP:
> - if (unlikely(!(irq_status & ASPEED_I2CD_INTR_NORMAL_STOP))) {
> - dev_err(bus->dev, "master failed to STOP");
> + if (unlikely(!(bus->irq_status &
> + ASPEED_I2CD_INTR_NORMAL_STOP))) {
> + dev_err(bus->dev,
> + "master failed to STOP irq_status:0x%x\n",
> + bus->irq_status);
> bus->cmd_err = -EIO;
> /* Do not STOP as we have already tried. */
> } else {
> @@ -520,8 +532,8 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
> goto out_complete;
> case ASPEED_I2C_MASTER_INACTIVE:
> dev_err(bus->dev,
> - "master received interrupt 0x%08x, but is inactive",
> - irq_status);
> + "master received interrupt 0x%08x, but is inactive\n",
> + bus->irq_status);
> bus->cmd_err = -EIO;
> /* Do not STOP as we should be inactive. */
> goto out_complete;
> @@ -543,26 +555,61 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
> bus->master_xfer_result = bus->msgs_index + 1;
> complete(&bus->cmd_complete);
> out_no_complete:
> - if (irq_status != status_ack)
> - dev_err(bus->dev,
> - "irq handled != irq. expected 0x%08x, but was 0x%08x\n",
> - irq_status, status_ack);
> - spin_unlock(&bus->lock);
> - return !!irq_status;
> + bus->irq_status ^= status_ack;
> + return !bus->irq_status;
> }
>
> static irqreturn_t aspeed_i2c_bus_irq(int irq, void *dev_id)
> {
> struct aspeed_i2c_bus *bus = dev_id;
> + u32 irq_received;
> +
> + spin_lock(&bus->lock);
> + irq_received = readl(bus->base + ASPEED_I2C_INTR_STS_REG);
> + bus->irq_status = irq_received;
>
> #if IS_ENABLED(CONFIG_I2C_SLAVE)
> - if (aspeed_i2c_slave_irq(bus)) {
> - dev_dbg(bus->dev, "irq handled by slave.\n");
> - return IRQ_HANDLED;
> + if (bus->master_state != ASPEED_I2C_MASTER_INACTIVE) {
> + if (!aspeed_i2c_master_irq(bus))
Why do you check the slave if master fails (or vice versa)? I
understand that there are some status bits that have not been handled,
but it doesn't seem reasonable to assume that there is state that the
other should do something with; the only way this would happen is if
the state that you think you are in does not match the status bits you
have been given, but if this is the case, you are already hosed; I
don't think trying the other handler is likely to make things better,
unless there is something that I am missing.
> + aspeed_i2c_slave_irq(bus);
> + } else {
> + if (!aspeed_i2c_slave_irq(bus))
> + aspeed_i2c_master_irq(bus);
> }
> +#else
> + aspeed_i2c_master_irq(bus);
> #endif /* CONFIG_I2C_SLAVE */
>
> - return aspeed_i2c_master_irq(bus) ? IRQ_HANDLED : IRQ_NONE;
> + if (bus->irq_status)
> + dev_err(bus->dev,
> + "irq handled != irq. expected 0x%08x, but was 0x%08x\n",
> + irq_received, irq_received ^ bus->irq_status);
> +
> + /* Ack all interrupt bits. */
> + writel(irq_received, bus->base + ASPEED_I2C_INTR_STS_REG);
> + spin_unlock(&bus->lock);
> + return bus->irq_status ? IRQ_NONE : IRQ_HANDLED;
> +}
> +
> +static int aspeed_i2c_check_bus_busy_timeout(struct aspeed_i2c_bus *bus)
> +{
> + ktime_t timeout = ktime_add_us(ktime_get(), BUS_BUSY_CHECK_TIMEOUT);
> +
> + might_sleep();
> +
> + for (;;) {
> + if (!(readl(bus->base + ASPEED_I2C_CMD_REG) &
> + (ASPEED_I2CD_BUS_BUSY_STS |
> + ASPEED_I2CD_XFER_MODE_STS_MASK)))
Is using the Transfer Mode State Machine bits necessary? The
documentation marks it as "for debugging purpose only," so relying on
it makes me nervous.
> + return 0;
> + if (ktime_compare(ktime_get(), timeout) > 0)
> + break;
> + usleep_range((BUS_BUSY_CHECK_INTERVAL >> 2) + 1,
Where did you get this minimum value?
> + BUS_BUSY_CHECK_INTERVAL);
> + }
> +
> + dev_err(bus->dev, "timeout waiting for idle. attempting recovery\n");
> + return aspeed_i2c_recover_bus(bus);
> }
>
> static int aspeed_i2c_master_xfer(struct i2c_adapter *adap,
> @@ -570,22 +617,11 @@ static int aspeed_i2c_master_xfer(struct i2c_adapter *adap,
> {
> struct aspeed_i2c_bus *bus = i2c_get_adapdata(adap);
> unsigned long time_left, flags;
> - int ret = 0;
> -
> - spin_lock_irqsave(&bus->lock, flags);
> - bus->cmd_err = 0;
>
> - /* If bus is busy, attempt recovery. We assume a single master
> - * environment.
> - */
> - if (readl(bus->base + ASPEED_I2C_CMD_REG) & ASPEED_I2CD_BUS_BUSY_STS) {
> - spin_unlock_irqrestore(&bus->lock, flags);
> - ret = aspeed_i2c_recover_bus(bus);
> - if (ret)
> - return ret;
> - spin_lock_irqsave(&bus->lock, flags);
> - }
> + if (aspeed_i2c_check_bus_busy_timeout(bus))
> + return -EAGAIN;
>
> + spin_lock_irqsave(&bus->lock, flags);
> bus->cmd_err = 0;
> bus->msgs = msgs;
> bus->msgs_index = 0;
> @@ -851,7 +887,7 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
> bus->rst = devm_reset_control_get_shared(&pdev->dev, NULL);
> if (IS_ERR(bus->rst)) {
> dev_err(&pdev->dev,
> - "missing or invalid reset controller device tree entry");
> + "missing or invalid reset controller device tree entry\n");
> return PTR_ERR(bus->rst);
> }
> reset_control_deassert(bus->rst);
> --
> 2.17.1
>
^ permalink raw reply
* [PATCH 14/14] arm: dts: OpenPower Palmetto system can use coprocessor for FSI
From: Benjamin Herrenschmidt @ 2018-06-26 23:26 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180626232605.13420-1-benh@kernel.crashing.org>
This switches away from userspace bitbanging to kernel FSI
using the coprocessor.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts | 31 ++++++++++++++-----
1 file changed, 24 insertions(+), 7 deletions(-)
diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts b/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts
index e6095f51ecf5..ba49b812e0bd 100644
--- a/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts
@@ -31,6 +31,11 @@
no-map;
reg = <0x98000000 0x01000000>; /* 16MB */
};
+
+ coldfire_memory: codefire_memory at 5ee00000 {
+ reg = <0x5ee00000 0x00200000>;
+ no-map;
+ };
};
leds {
@@ -49,6 +54,25 @@
};
};
+ fsi: gpio-fsi {
+ compatible = "ibm,fsi-master-ast2400-cf", "fsi-master";
+ #address-cells = <2>;
+ #size-cells = <0>;
+
+ memory-region = <&coldfire_memory>;
+ sram = <&sram>;
+ cvic = <&cvic>;
+
+ fw-name = "palmetto";
+ fw-platform-sig = <0x5061>;
+
+ clock-gpios = <&gpio ASPEED_GPIO(A, 4) GPIO_ACTIVE_HIGH>;
+ data-gpios = <&gpio ASPEED_GPIO(A, 5) GPIO_ACTIVE_HIGH>;
+ mux-gpios = <&gpio ASPEED_GPIO(A, 6) GPIO_ACTIVE_HIGH>;
+ enable-gpios = <&gpio ASPEED_GPIO(D, 0) GPIO_ACTIVE_HIGH>;
+ trans-gpios = <&gpio ASPEED_GPIO(H, 6) GPIO_ACTIVE_HIGH>;
+ };
+
gpio-keys {
compatible = "gpio-keys";
@@ -323,13 +347,6 @@
line-name = "SYS_PWROK_BMC";
};
- pin_gpio_h6 {
- gpio-hog;
- gpios = <ASPEED_GPIO(H, 6) GPIO_ACTIVE_HIGH>;
- output-high;
- line-name = "SCM1_FSI0_DATA_EN";
- };
-
pin_gpio_h7 {
gpio-hog;
gpios = <ASPEED_GPIO(H, 7) GPIO_ACTIVE_HIGH>;
--
2.17.1
^ permalink raw reply related
* [PATCH 13/14] arm: dts: OpenPower Romulus system can use coprocessor for FSI
From: Benjamin Herrenschmidt @ 2018-06-26 23:26 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180626232605.13420-1-benh@kernel.crashing.org>
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts b/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
index 347938673c83..070b8f8f1d62 100644
--- a/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
@@ -37,6 +37,11 @@
compatible = "shared-dma-pool";
reusable;
};
+
+ coldfire_memory: codefire_memory at 9ef00000 {
+ reg = <0x9ef00000 0x00100000>;
+ no-map;
+ };
};
leds {
@@ -56,10 +61,16 @@
};
fsi: gpio-fsi {
- compatible = "fsi-master-gpio", "fsi-master";
+ compatible = "ibm,fsi-master-ast2500-cf", "fsi-master";
#address-cells = <2>;
#size-cells = <0>;
- no-gpio-delays;
+
+ memory-region = <&coldfire_memory>;
+ sram = <&sram>;
+ cvic = <&cvic>;
+
+ fw-name = "romulus";
+ fw-platform-sig = <0x526d>;
clock-gpios = <&gpio ASPEED_GPIO(AA, 0) GPIO_ACTIVE_HIGH>;
data-gpios = <&gpio ASPEED_GPIO(AA, 2) GPIO_ACTIVE_HIGH>;
--
2.17.1
^ permalink raw reply related
* [PATCH 12/14] fsi: master-ast-cf: Add new FSI master using Aspeed ColdFire
From: Benjamin Herrenschmidt @ 2018-06-26 23:26 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180626232605.13420-1-benh@kernel.crashing.org>
The Aspeed AST2x00 can contain a ColdFire v1 coprocessor which
is currently unused on OpenPower systems.
This adds an alternative to the fsi-master-gpio driver that
uses that coprocessor instead of bit banging from the ARM
core itself. The end result is about 4 times faster.
The firmware for the coprocessor and its source code can be
found at https://github.com/ozbenh/cf-fsi and is system specific.
Currently tested on Romulus and Palmetto systems.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
drivers/fsi/Kconfig | 9 +
drivers/fsi/Makefile | 1 +
drivers/fsi/cf-fsi-fw.h | 131 ++
drivers/fsi/fsi-master-ast-cf.c | 1376 ++++++++++++++++++++++
include/trace/events/fsi_master_ast_cf.h | 150 +++
5 files changed, 1667 insertions(+)
create mode 100644 drivers/fsi/cf-fsi-fw.h
create mode 100644 drivers/fsi/fsi-master-ast-cf.c
create mode 100644 include/trace/events/fsi_master_ast_cf.h
diff --git a/drivers/fsi/Kconfig b/drivers/fsi/Kconfig
index 322cec393cf2..e0220d1e1357 100644
--- a/drivers/fsi/Kconfig
+++ b/drivers/fsi/Kconfig
@@ -27,6 +27,15 @@ config FSI_MASTER_HUB
allow chaining of FSI links to an arbitrary depth. This allows for
a high target device fanout.
+config FSI_MASTER_AST_CF
+ tristate "FSI master based on Aspeed ColdFire coprocessor"
+ depends on GPIOLIB
+ depends on GPIO_ASPEED
+ ---help---
+ This option enables a FSI master using the AST2400 and AST2500 GPIO
+ lines driven by the internal ColdFire coprocessor. This requires
+ the corresponding machine specific ColdFire firmware to be available.
+
config FSI_SCOM
tristate "SCOM FSI client device driver"
---help---
diff --git a/drivers/fsi/Makefile b/drivers/fsi/Makefile
index 75fdc6d8cfc4..62687ec86d2e 100644
--- a/drivers/fsi/Makefile
+++ b/drivers/fsi/Makefile
@@ -2,6 +2,7 @@
obj-$(CONFIG_FSI) += fsi-core.o
obj-$(CONFIG_FSI_MASTER_HUB) += fsi-master-hub.o
obj-$(CONFIG_FSI_MASTER_GPIO) += fsi-master-gpio.o
+obj-$(CONFIG_FSI_MASTER_AST_CF) += fsi-master-ast-cf.o
obj-$(CONFIG_FSI_SCOM) += fsi-scom.o
obj-$(CONFIG_FSI_SBEFIFO) += fsi-sbefifo.o
obj-$(CONFIG_FSI_OCC) += fsi-occ.o
diff --git a/drivers/fsi/cf-fsi-fw.h b/drivers/fsi/cf-fsi-fw.h
new file mode 100644
index 000000000000..5f6cffd30861
--- /dev/null
+++ b/drivers/fsi/cf-fsi-fw.h
@@ -0,0 +1,131 @@
+#ifndef __CF_FSI_FW_H
+#define __CF_FSI_FW_H
+
+/*
+ * uCode file layout
+ *
+ * 0000...03ff : m68k exception vectors
+ * 0400...04ff : Header info & boot config block
+ * 0500....... : Code & stack
+ */
+
+/*
+ * Header info & boot config area
+ *
+ * The Header info is built into the ucode and provide version and
+ * platform information.
+ *
+ * the Boot config needs to be adjusted by the ARM prior to starting
+ * the ucode if the Command/Status area isn't at 0x320000 in CF space
+ * (ie. beginning of SRAM).
+ */
+
+#define HDR_OFFSET 0x400
+
+/* Info: Signature & version */
+#define HDR_SYS_SIG 0x00 /* 2 bytes system signature */
+#define SYS_SIG_ROMULUS 0x526d /* 'Rm' */
+#define SYS_SIG_WITHERSPOON 0x5773 /* 'Ws' */
+#define HDR_FW_VERS 0x02 /* 2 bytes Major.Minor */
+#define HDR_API_VERS 0x04 /* 2 bytes Major.Minor */
+#define API_VERSION_MAJ 1 /* Current version */
+#define API_VERSION_MIN 1
+#define HDR_FW_OPTIONS 0x08 /* 4 bytes option flags */
+#define FW_OPTION_TRACE_EN 0x00000001 /* FW tracing enabled */
+
+/* Boot Config: Address of Command/Status area */
+#define HDR_CMD_STAT_AREA 0x80 /* 4 bytes CF address */
+
+/*
+ * Command/Status area layout: Main part
+ */
+
+/* Command/Status register:
+ *
+ * +---------------------------+
+ * | STAT | RLEN | CLEN | CMD |
+ * | 8 | 8 | 8 | 8 |
+ * +---------------------------+
+ * | | | |
+ * status | | |
+ * Response len | |
+ * (in bits) | |
+ * | |
+ * Command len |
+ * (in bits) |
+ * |
+ * Command code
+ *
+ * Due to the big endian layout, that means that a byte read will
+ * return the status byte
+ */
+#define CMD_STAT_REG 0x00
+#define CMD_REG_CMD_MASK 0x000000ff
+#define CMD_REG_CMD_SHIFT 0
+#define CMD_NONE 0x00
+#define CMD_COMMAND 0x01
+#define CMD_BREAK 0x02
+#define CMD_IDLE_CLOCKS 0x03 /* clen = #clocks */
+#define CMD_INVALID 0xff
+#define CMD_REG_CLEN_MASK 0x0000ff00
+#define CMD_REG_CLEN_SHIFT 8
+#define CMD_REG_RLEN_MASK 0x00ff0000
+#define CMD_REG_RLEN_SHIFT 16
+#define CMD_REG_STAT_MASK 0xff000000
+#define CMD_REG_STAT_SHIFT 24
+#define STAT_WORKING 0x00
+#define STAT_COMPLETE 0x01
+#define STAT_ERR_INVAL_CMD 0x80
+#define STAT_ERR_INVAL_IRQ 0x81
+#define STAT_ERR_MTOE 0x82
+
+/* Response tag & CRC */
+#define STAT_RTAG 0x04
+
+/* Response CRC */
+#define STAT_RCRC 0x05
+
+/* Echo and Send delay */
+#define ECHO_DLY_REG 0x08
+#define SEND_DLY_REG 0x09
+
+/* Command data area
+ *
+ * Last byte of message must be left aligned
+ */
+#define CMD_DATA 0x10 /* 64 bit of data */
+
+/* Response data area, right aligned, unused top bits are 1 */
+#define RSP_DATA 0x20 /* 32 bit of data */
+
+/* Misc */
+#define INT_CNT 0x30 /* 32-bit interrupt count */
+#define BAD_INT_VEC 0x34
+#define CF_STARTED 0x38 /* byte, set to -1 when copro started */
+
+/*
+ * SRAM layout: GPIO arbitration part
+ */
+#define ARB_REG 0x40
+#define ARB_ARM_REQ 0x01
+#define ARB_ARM_ACK 0x02
+
+/*
+ * SRAM layout: Trace buffer (debug builds only)
+ */
+#define TRACEBUF 0x100
+#define TR_CLKOBIT0 0xc0
+#define TR_CLKOBIT1 0xc1
+#define TR_CLKOSTART 0x82
+#define TR_OLEN 0x83 /* + len */
+#define TR_CLKZ 0x84 /* + count */
+#define TR_CLKWSTART 0x85
+#define TR_CLKTAG 0x86 /* + tag */
+#define TR_CLKDATA 0x87 /* + len */
+#define TR_CLKCRC 0x88 /* + raw crc */
+#define TR_CLKIBIT0 0x90
+#define TR_CLKIBIT1 0x91
+#define TR_END 0xff
+
+#endif /* __CF_FSI_FW_H */
+
diff --git a/drivers/fsi/fsi-master-ast-cf.c b/drivers/fsi/fsi-master-ast-cf.c
new file mode 100644
index 000000000000..6b17f27c27f6
--- /dev/null
+++ b/drivers/fsi/fsi-master-ast-cf.c
@@ -0,0 +1,1376 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * A FSI master controller, using a simple GPIO bit-banging interface
+ */
+
+#include <linux/crc4.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/fsi.h>
+#include <linux/gpio/consumer.h>
+#include <linux/io.h>
+#include <linux/irqflags.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/regmap.h>
+#include <linux/firmware.h>
+#include <linux/gpio/aspeed.h>
+#include <linux/mfd/syscon.h>
+#include <linux/of_address.h>
+#include <linux/genalloc.h>
+
+#include "fsi-master.h"
+#include "cf-fsi-fw.h"
+
+/* Common SCU based coprocessor control registers */
+#define SCU_COPRO_CTRL 0x100
+#define SCU_COPRO_RESET 0x00000002
+#define SCU_COPRO_CLK_EN 0x00000001
+
+/* AST2500 specific ones */
+#define SCU_2500_COPRO_SEG0 0x104
+#define SCU_2500_COPRO_SEG1 0x108
+#define SCU_2500_COPRO_SEG2 0x10c
+#define SCU_2500_COPRO_SEG3 0x110
+#define SCU_2500_COPRO_SEG4 0x114
+#define SCU_2500_COPRO_SEG5 0x118
+#define SCU_2500_COPRO_SEG6 0x11c
+#define SCU_2500_COPRO_SEG7 0x120
+#define SCU_2500_COPRO_SEG8 0x124
+#define SCU_2500_COPRO_SEG_SWAP 0x00000001
+#define SCU_2500_COPRO_CACHE_CTL 0x128
+#define SCU_2500_COPRO_CACHE_EN 0x00000001
+#define SCU_2500_COPRO_SEG0_CACHE_EN 0x00000002
+#define SCU_2500_COPRO_SEG1_CACHE_EN 0x00000004
+#define SCU_2500_COPRO_SEG2_CACHE_EN 0x00000008
+#define SCU_2500_COPRO_SEG3_CACHE_EN 0x00000010
+#define SCU_2500_COPRO_SEG4_CACHE_EN 0x00000020
+#define SCU_2500_COPRO_SEG5_CACHE_EN 0x00000040
+#define SCU_2500_COPRO_SEG6_CACHE_EN 0x00000080
+#define SCU_2500_COPRO_SEG7_CACHE_EN 0x00000100
+#define SCU_2500_COPRO_SEG8_CACHE_EN 0x00000200
+
+#define SCU_2400_COPRO_SEG0 0x104
+#define SCU_2400_COPRO_SEG2 0x108
+#define SCU_2400_COPRO_SEG4 0x10c
+#define SCU_2400_COPRO_SEG6 0x110
+#define SCU_2400_COPRO_SEG8 0x114
+#define SCU_2400_COPRO_SEG_SWAP 0x80000000
+#define SCU_2400_COPRO_CACHE_CTL 0x118
+#define SCU_2400_COPRO_CACHE_EN 0x00000001
+#define SCU_2400_COPRO_SEG0_CACHE_EN 0x00000002
+#define SCU_2400_COPRO_SEG2_CACHE_EN 0x00000004
+#define SCU_2400_COPRO_SEG4_CACHE_EN 0x00000008
+#define SCU_2400_COPRO_SEG6_CACHE_EN 0x00000010
+#define SCU_2400_COPRO_SEG8_CACHE_EN 0x00000020
+
+/* CVIC registers */
+#define CVIC_EN_REG 0x10
+#define CVIC_TRIG_REG 0x18
+
+/*
+ * System register base address (needed for configuring the
+ * coldfire maps)
+ */
+#define SYSREG_BASE 0x1e600000
+
+/* Amount of SRAM required */
+#define SRAM_SIZE 0x1000
+
+#define LAST_ADDR_INVALID 0x1
+
+struct fsi_master_acf {
+ struct fsi_master master;
+ struct device *dev;
+ struct regmap *scu;
+ struct mutex lock; /* mutex for command ordering */
+ struct gpio_desc *gpio_clk;
+ struct gpio_desc *gpio_data;
+ struct gpio_desc *gpio_trans; /* Voltage translator */
+ struct gpio_desc *gpio_enable; /* FSI enable */
+ struct gpio_desc *gpio_mux; /* Mux control */
+ uint32_t cf_mem_addr;
+ size_t cf_mem_size;
+ void __iomem *cf_mem;
+ void __iomem *cvic;
+ struct gen_pool *sram_pool;
+ void __iomem *sram;
+ bool is_ast2500;
+ bool external_mode;
+ bool trace_enabled;
+ uint32_t last_addr;
+ uint8_t t_send_delay;
+ uint8_t t_echo_delay;
+ uint32_t cvic_sw_irq;
+};
+#define to_fsi_master_acf(m) container_of(m, struct fsi_master_acf, master)
+
+struct fsi_msg {
+ uint64_t msg;
+ uint8_t bits;
+};
+
+#define CREATE_TRACE_POINTS
+#include <trace/events/fsi_master_ast_cf.h>
+
+static void msg_push_bits(struct fsi_msg *msg, uint64_t data, int bits)
+{
+ msg->msg <<= bits;
+ msg->msg |= data & ((1ull << bits) - 1);
+ msg->bits += bits;
+}
+
+static void msg_push_crc(struct fsi_msg *msg)
+{
+ uint8_t crc;
+ int top;
+
+ top = msg->bits & 0x3;
+
+ /* start bit, and any non-aligned top bits */
+ crc = crc4(0, 1 << top | msg->msg >> (msg->bits - top), top + 1);
+
+ /* aligned bits */
+ crc = crc4(crc, msg->msg, msg->bits - top);
+
+ msg_push_bits(msg, crc, 4);
+}
+
+static void msg_finish_cmd(struct fsi_msg *cmd)
+{
+ /* Left align message */
+ cmd->msg <<= (64 - cmd->bits);
+}
+
+static bool check_same_address(struct fsi_master_acf *master, int id,
+ uint32_t addr)
+{
+ /* this will also handle LAST_ADDR_INVALID */
+ return master->last_addr == (((id & 0x3) << 21) | (addr & ~0x3));
+}
+
+static bool check_relative_address(struct fsi_master_acf *master, int id,
+ uint32_t addr, uint32_t *rel_addrp)
+{
+ uint32_t last_addr = master->last_addr;
+ int32_t rel_addr;
+
+ if (last_addr == LAST_ADDR_INVALID)
+ return false;
+
+ /* We may be in 23-bit addressing mode, which uses the id as the
+ * top two address bits. So, if we're referencing a different ID,
+ * use absolute addresses.
+ */
+ if (((last_addr >> 21) & 0x3) != id)
+ return false;
+
+ /* remove the top two bits from any 23-bit addressing */
+ last_addr &= (1 << 21) - 1;
+
+ /* We know that the addresses are limited to 21 bits, so this won't
+ * overflow the signed rel_addr */
+ rel_addr = addr - last_addr;
+ if (rel_addr > 255 || rel_addr < -256)
+ return false;
+
+ *rel_addrp = (uint32_t)rel_addr;
+
+ return true;
+}
+
+static void last_address_update(struct fsi_master_acf *master,
+ int id, bool valid, uint32_t addr)
+{
+ if (!valid)
+ master->last_addr = LAST_ADDR_INVALID;
+ else
+ master->last_addr = ((id & 0x3) << 21) | (addr & ~0x3);
+}
+
+/*
+ * Encode an Absolute/Relative/Same Address command
+ */
+static void build_ar_command(struct fsi_master_acf *master,
+ struct fsi_msg *cmd, uint8_t id,
+ uint32_t addr, size_t size,
+ const void *data)
+{
+ int i, addr_bits, opcode_bits;
+ bool write = !!data;
+ uint8_t ds, opcode;
+ uint32_t rel_addr;
+
+ cmd->bits = 0;
+ cmd->msg = 0;
+
+ /* we have 21 bits of address max */
+ addr &= ((1 << 21) - 1);
+
+ /* cmd opcodes are variable length - SAME_AR is only two bits */
+ opcode_bits = 3;
+
+ if (check_same_address(master, id, addr)) {
+ /* we still address the byte offset within the word */
+ addr_bits = 2;
+ opcode_bits = 2;
+ opcode = FSI_CMD_SAME_AR;
+ trace_fsi_master_acf_cmd_same_addr(master);
+
+ } else if (check_relative_address(master, id, addr, &rel_addr)) {
+ /* 8 bits plus sign */
+ addr_bits = 9;
+ addr = rel_addr;
+ opcode = FSI_CMD_REL_AR;
+ trace_fsi_master_acf_cmd_rel_addr(master, rel_addr);
+
+ } else {
+ addr_bits = 21;
+ opcode = FSI_CMD_ABS_AR;
+ trace_fsi_master_acf_cmd_abs_addr(master, addr);
+ }
+
+ /*
+ * The read/write size is encoded in the lower bits of the address
+ * (as it must be naturally-aligned), and the following ds bit.
+ *
+ * size addr:1 addr:0 ds
+ * 1 x x 0
+ * 2 x 0 1
+ * 4 0 1 1
+ *
+ */
+ ds = size > 1 ? 1 : 0;
+ addr &= ~(size - 1);
+ if (size == 4)
+ addr |= 1;
+
+ msg_push_bits(cmd, id, 2);
+ msg_push_bits(cmd, opcode, opcode_bits);
+ msg_push_bits(cmd, write ? 0 : 1, 1);
+ msg_push_bits(cmd, addr, addr_bits);
+ msg_push_bits(cmd, ds, 1);
+ for (i = 0; write && i < size; i++)
+ msg_push_bits(cmd, ((uint8_t *)data)[i], 8);
+
+ msg_push_crc(cmd);
+ msg_finish_cmd(cmd);
+}
+
+static void build_dpoll_command(struct fsi_msg *cmd, uint8_t slave_id)
+{
+ cmd->bits = 0;
+ cmd->msg = 0;
+
+ msg_push_bits(cmd, slave_id, 2);
+ msg_push_bits(cmd, FSI_CMD_DPOLL, 3);
+ msg_push_crc(cmd);
+ msg_finish_cmd(cmd);
+}
+
+static void build_epoll_command(struct fsi_msg *cmd, uint8_t slave_id)
+{
+ cmd->bits = 0;
+ cmd->msg = 0;
+
+ msg_push_bits(cmd, slave_id, 2);
+ msg_push_bits(cmd, FSI_CMD_EPOLL, 3);
+ msg_push_crc(cmd);
+ msg_finish_cmd(cmd);
+}
+
+static void build_term_command(struct fsi_msg *cmd, uint8_t slave_id)
+{
+ cmd->bits = 0;
+ cmd->msg = 0;
+
+ msg_push_bits(cmd, slave_id, 2);
+ msg_push_bits(cmd, FSI_CMD_TERM, 6);
+ msg_push_crc(cmd);
+ msg_finish_cmd(cmd);
+}
+
+static int do_copro_command(struct fsi_master_acf *master, uint32_t op)
+{
+ uint32_t timeout = 10000000;
+ uint8_t stat;
+
+ trace_fsi_master_acf_copro_command(master, op);
+
+ /* Send command */
+ writel(cpu_to_be32(op), master->sram + CMD_STAT_REG);
+
+ /* Read back to avoid ordering issue */
+ (void)readl(master->sram + CMD_STAT_REG);
+
+ /* Ring doorbell if any */
+ if (master->cvic)
+ writel(0x2, master->cvic + CVIC_TRIG_REG);
+
+ /* Wait for status to indicate completion (or error) */
+ do {
+ if (timeout-- == 0) {
+ dev_warn(master->dev,
+ "Timeout waiting for coprocessor completion\n");
+ return -ETIMEDOUT;
+ }
+ stat = readb(master->sram + CMD_STAT_REG);
+ } while(stat < STAT_COMPLETE || stat == 0xff);
+
+ if (stat == STAT_COMPLETE)
+ return 0;
+ switch(stat) {
+ case STAT_ERR_INVAL_CMD:
+ return -EINVAL;
+ case STAT_ERR_INVAL_IRQ:
+ return -EIO;
+ case STAT_ERR_MTOE:
+ return -ESHUTDOWN;
+ }
+ return -ENXIO;
+}
+
+static int clock_zeros(struct fsi_master_acf *master, int count)
+{
+ while (count) {
+ int rc, lcnt = min(count, 255);
+
+ rc = do_copro_command(master,
+ CMD_IDLE_CLOCKS | (lcnt << CMD_REG_CLEN_SHIFT));
+ if (rc)
+ return rc;
+ count -= lcnt;
+ }
+ return 0;
+}
+
+static int send_request(struct fsi_master_acf *master, struct fsi_msg *cmd,
+ bool is_write)
+{
+ u32 op;
+
+ trace_fsi_master_acf_send_request(master, cmd, is_write);
+
+ /* Store message into SRAM */
+ writel(cpu_to_be32(cmd->msg >> 32), master->sram + CMD_DATA);
+ writel(cpu_to_be32(cmd->msg & 0xffffffff), master->sram + CMD_DATA + 4);
+
+ op = CMD_COMMAND;
+ op |= cmd->bits << CMD_REG_CLEN_SHIFT;
+ if (!is_write)
+ op |= 32 << CMD_REG_RLEN_SHIFT;
+
+ return do_copro_command(master, op);
+}
+
+static int read_copro_response(struct fsi_master_acf *master, uint8_t size,
+ __be32 *response, u8 *tag)
+{
+ u8 rtag = readb(master->sram + STAT_RTAG);
+ u8 rcrc = readb(master->sram + STAT_RCRC);
+ __be32 rdata = 0;
+ u32 crc;
+ u8 ack;
+
+ *tag = ack = rtag & 3;
+
+ /* we have a whole message now; check CRC */
+ crc = crc4(0, 1, 1);
+ crc = crc4(crc, rtag, 4);
+ if (ack == FSI_RESP_ACK && size) {
+ rdata = readl(master->sram + RSP_DATA);
+ crc = crc4(crc, be32_to_cpu(rdata), 32);
+ if (response)
+ *response = rdata;
+ }
+ crc = crc4(crc, rcrc, 4);
+
+ trace_fsi_master_acf_copro_response(master, rtag, rcrc, rdata, crc == 0);
+
+ if (crc) {
+ /*
+ * Check if it's all 1's or all 0's, that probably means
+ * the host is off
+ */
+ if ((rtag == 0xf && rcrc == 0xf) || (rtag == 0 && rcrc == 0))
+ return -ENODEV;
+ dev_dbg(master->dev, "Bad response CRC !\n");
+ return -EAGAIN;
+ }
+ return 0;
+}
+
+static int send_term(struct fsi_master_acf *master, uint8_t slave)
+{
+ struct fsi_msg cmd;
+ uint8_t tag;
+ int rc;
+
+ build_term_command(&cmd, slave);
+
+ rc = send_request(master, &cmd, true);
+ if (rc) {
+ dev_warn(master->dev, "Error %d sending term\n", rc);
+ return rc;
+ }
+
+ rc = read_copro_response(master, 0, NULL, &tag);
+ if (rc < 0) {
+ dev_err(master->dev,
+ "TERM failed; lost communication with slave\n");
+ return -EIO;
+ } else if (tag != FSI_RESP_ACK) {
+ dev_err(master->dev, "TERM failed; response %d\n", tag);
+ return -EIO;
+ }
+ return 0;
+}
+
+static void dump_trace(struct fsi_master_acf *master)
+{
+ char trbuf[52];
+ char *p;
+ int i;
+
+ dev_dbg(master->dev,
+ "CMDSTAT:%08x RTAG=%02x RCRC=%02x RDATA=%02x #INT=%08x\n",
+ be32_to_cpu(readl(master->sram + CMD_STAT_REG)),
+ readb(master->sram + STAT_RTAG),
+ readb(master->sram + STAT_RCRC),
+ be32_to_cpu(readl(master->sram + RSP_DATA)),
+ be32_to_cpu(readl(master->sram + INT_CNT)));
+
+ for (i = 0; i < 512; i++) {
+ uint8_t v;
+ if ((i % 16) == 0)
+ p = trbuf;
+ v = readb(master->sram + TRACEBUF + i);
+ p += sprintf(p, "%02x ", v);
+ if (((i % 16) == 15) || v == TR_END)
+ dev_dbg(master->dev, "%s\n", trbuf);
+ if (v == TR_END)
+ break;
+ }
+}
+
+static int handle_response(struct fsi_master_acf *master,
+ uint8_t slave, uint8_t size, void *data)
+{
+ int busy_count = 0, rc;
+ int crc_err_retries = 0;
+ struct fsi_msg cmd;
+ __be32 response;
+ uint8_t tag;
+retry:
+ rc = read_copro_response(master, size, &response, &tag);
+
+ /* Handle retries on CRC errors */
+ if (rc == -EAGAIN) {
+ /* Too many retries ? */
+ if (crc_err_retries++ > FSI_CRC_ERR_RETRIES) {
+ /*
+ * Pass it up as a -EIO otherwise upper level will retry
+ * the whole command which isn't what we want here.
+ */
+ rc = -EIO;
+ goto bail;
+ }
+ trace_fsi_master_acf_crc_rsp_error(master, crc_err_retries);
+ if (master->trace_enabled)
+ dump_trace(master);
+ rc = clock_zeros(master, FSI_MASTER_EPOLL_CLOCKS);
+ if (rc) {
+ dev_warn(master->dev,
+ "Error %d clocking zeros for E_POLL\n", rc);
+ return rc;
+ }
+ build_epoll_command(&cmd, slave);
+ rc = send_request(master, &cmd, size == 0);
+ if (rc) {
+ dev_warn(master->dev, "Error %d sending E_POLL\n", rc);
+ return -EIO;
+ }
+ goto retry;
+ }
+ if (rc)
+ return rc;
+
+ switch (tag) {
+ case FSI_RESP_ACK:
+ if (size && data) {
+ if (size == 4)
+ *(__be32 *)data = response;
+ else if (size == 2)
+ *(__be16 *)data = response;
+ else
+ *(u8 *)data = response;
+ }
+ break;
+ case FSI_RESP_BUSY:
+ /*
+ * Its necessary to clock slave before issuing
+ * d-poll, not indicated in the hardware protocol
+ * spec. < 20 clocks causes slave to hang, 21 ok.
+ */
+ dev_dbg(master->dev, "Busy, retrying...\n");
+ if (master->trace_enabled)
+ dump_trace(master);
+ rc = clock_zeros(master, FSI_MASTER_DPOLL_CLOCKS);
+ if (rc) {
+ dev_warn(master->dev,
+ "Error %d clocking zeros for D_POLL\n", rc);
+ break;
+ }
+ if (busy_count++ < FSI_MASTER_MAX_BUSY) {
+ build_dpoll_command(&cmd, slave);
+ rc = send_request(master, &cmd, size == 0);
+ if (rc) {
+ dev_warn(master->dev, "Error %d sending D_POLL\n", rc);
+ break;
+ }
+ goto retry;
+ }
+ dev_dbg(master->dev,
+ "ERR slave is stuck in busy state, issuing TERM\n");
+ send_term(master, slave);
+ rc = -EIO;
+ break;
+
+ case FSI_RESP_ERRA:
+ dev_dbg(master->dev, "ERRA received\n");
+ if (master->trace_enabled)
+ dump_trace(master);
+ rc = -EIO;
+ break;
+ case FSI_RESP_ERRC:
+ dev_dbg(master->dev, "ERRC received\n");
+ if (master->trace_enabled)
+ dump_trace(master);
+ rc = -EAGAIN;
+ break;
+ }
+ bail:
+ if (busy_count > 0) {
+ trace_fsi_master_acf_poll_response_busy(master, busy_count);
+ }
+
+ return rc;
+}
+
+static int fsi_master_acf_xfer(struct fsi_master_acf *master, uint8_t slave,
+ struct fsi_msg *cmd, size_t resp_len, void *resp)
+{
+ int rc = -EAGAIN, retries = 0;
+
+ while ((retries++) < FSI_CRC_ERR_RETRIES) {
+ rc = send_request(master, cmd, resp_len == 0);
+ if (rc) {
+ if (rc != -ESHUTDOWN)
+ dev_warn(master->dev, "Error %d sending command\n", rc);
+ break;
+ }
+ rc = handle_response(master, slave, resp_len, resp);
+ if (rc != -EAGAIN)
+ break;
+ rc = -EIO;
+ dev_dbg(master->dev, "ECRC retry %d\n", retries);
+
+ /* Pace it a bit before retry */
+ msleep(1);
+ }
+
+ return rc;
+}
+
+static int fsi_master_acf_read(struct fsi_master *_master, int link,
+ uint8_t id, uint32_t addr, void *val,
+ size_t size)
+{
+ struct fsi_master_acf *master = to_fsi_master_acf(_master);
+ struct fsi_msg cmd;
+ int rc;
+
+ if (link != 0)
+ return -ENODEV;
+
+ mutex_lock(&master->lock);
+ dev_dbg(master->dev, "read id %d addr %x size %d\n", id, addr, size);
+ build_ar_command(master, &cmd, id, addr, size, NULL);
+ rc = fsi_master_acf_xfer(master, id, &cmd, size, val);
+ last_address_update(master, id, rc == 0, addr);
+ if (rc)
+ dev_dbg(master->dev, "read id %d addr 0x%08x err: %d\n",
+ id, addr, rc);
+ mutex_unlock(&master->lock);
+
+ return rc;
+}
+
+static int fsi_master_acf_write(struct fsi_master *_master, int link,
+ uint8_t id, uint32_t addr, const void *val,
+ size_t size)
+{
+ struct fsi_master_acf *master = to_fsi_master_acf(_master);
+ struct fsi_msg cmd;
+ int rc;
+
+ if (link != 0)
+ return -ENODEV;
+
+ mutex_lock(&master->lock);
+ build_ar_command(master, &cmd, id, addr, size, val);
+ dev_dbg(master->dev, "write id %d addr %x size %d raw_data: %08x\n",
+ id, addr, size, *(u32 *)val);
+ rc = fsi_master_acf_xfer(master, id, &cmd, 0, NULL);
+ last_address_update(master, id, rc == 0, addr);
+ if (rc)
+ dev_dbg(master->dev, "write id %d addr 0x%08x err: %d\n",
+ id, addr, rc);
+ mutex_unlock(&master->lock);
+
+ return rc;
+}
+
+static int fsi_master_acf_term(struct fsi_master *_master,
+ int link, uint8_t id)
+{
+ struct fsi_master_acf *master = to_fsi_master_acf(_master);
+ struct fsi_msg cmd;
+ int rc;
+
+ if (link != 0)
+ return -ENODEV;
+
+ mutex_lock(&master->lock);
+ build_term_command(&cmd, id);
+ dev_dbg(master->dev, "term id %d\n", id);
+ rc = fsi_master_acf_xfer(master, id, &cmd, 0, NULL);
+ last_address_update(master, id, false, 0);
+ mutex_unlock(&master->lock);
+
+ return rc;
+}
+
+static int fsi_master_acf_break(struct fsi_master *_master, int link)
+{
+ struct fsi_master_acf *master = to_fsi_master_acf(_master);
+ int rc;
+
+ if (link != 0)
+ return -ENODEV;
+
+ mutex_lock(&master->lock);
+ if (master->external_mode) {
+ mutex_unlock(&master->lock);
+ return -EBUSY;
+ }
+ dev_dbg(master->dev, "sending BREAK\n");
+ rc = do_copro_command(master, CMD_BREAK);
+ last_address_update(master, 0, false, 0);
+ mutex_unlock(&master->lock);
+
+ /* Wait for logic reset to take effect */
+ udelay(200);
+
+ return rc;
+}
+
+static void reset_cf(struct fsi_master_acf *master)
+{
+ regmap_write(master->scu, SCU_COPRO_CTRL, SCU_COPRO_RESET);
+ usleep_range(20,20);
+ regmap_write(master->scu, SCU_COPRO_CTRL, 0);
+ usleep_range(20,20);
+}
+
+static void start_cf(struct fsi_master_acf *master)
+{
+ regmap_write(master->scu, SCU_COPRO_CTRL, SCU_COPRO_CLK_EN);
+}
+
+static void setup_ast2500_cf_maps(struct fsi_master_acf *master)
+{
+ /*
+ * Note about byteswap setting: the bus is wired backwards,
+ * so setting the byteswap bit actually makes the ColdFire
+ * work "normally" for a BE processor, ie, put the MSB in
+ * the lowest address byte.
+ *
+ * We thus need to set the bit for our main memory which
+ * contains our program code. We create two mappings for
+ * the register, one with each setting.
+ *
+ * Segments 2 and 3 has a "swapped" mapping (BE)
+ * and 6 and 7 have a non-swapped mapping (LE) which allows
+ * us to avoid byteswapping register accesses since the
+ * registers are all LE.
+ */
+
+ /* Setup segment 0 to our memory region */
+ regmap_write(master->scu, SCU_2500_COPRO_SEG0, master->cf_mem_addr |
+ SCU_2500_COPRO_SEG_SWAP);
+
+ /* Segments 2 and 3 to sysregs with byteswap (for SRAM) */
+ regmap_write(master->scu, SCU_2500_COPRO_SEG2, SYSREG_BASE |
+ SCU_2500_COPRO_SEG_SWAP);
+ regmap_write(master->scu, SCU_2500_COPRO_SEG3, SYSREG_BASE | 0x100000 |
+ SCU_2500_COPRO_SEG_SWAP);
+
+ /* And segment 6 and 7 to sysregs no byteswap */
+ regmap_write(master->scu, SCU_2500_COPRO_SEG6, SYSREG_BASE);
+ regmap_write(master->scu, SCU_2500_COPRO_SEG7, SYSREG_BASE | 0x100000);
+
+ /* Memory cachable, regs and SRAM not cachable */
+ regmap_write(master->scu, SCU_2500_COPRO_CACHE_CTL,
+ SCU_2500_COPRO_SEG0_CACHE_EN | SCU_2500_COPRO_CACHE_EN);
+}
+
+static void setup_ast2400_cf_maps(struct fsi_master_acf *master)
+{
+ /* Setup segment 0 to our memory region */
+ regmap_write(master->scu, SCU_2400_COPRO_SEG0, master->cf_mem_addr |
+ SCU_2400_COPRO_SEG_SWAP);
+
+ /* Segments 2 to sysregs with byteswap (for SRAM) */
+ regmap_write(master->scu, SCU_2400_COPRO_SEG2, SYSREG_BASE |
+ SCU_2400_COPRO_SEG_SWAP);
+
+ /* And segment 6 to sysregs no byteswap */
+ regmap_write(master->scu, SCU_2400_COPRO_SEG6, SYSREG_BASE);
+
+ /* Memory cachable, regs and SRAM not cachable */
+ regmap_write(master->scu, SCU_2400_COPRO_CACHE_CTL,
+ SCU_2400_COPRO_SEG0_CACHE_EN | SCU_2400_COPRO_CACHE_EN);
+}
+
+static int setup_gpios_for_copro(struct fsi_master_acf *master)
+{
+
+ int rc;
+
+ /* This aren't under ColdFire control, just set them up appropriately */
+ gpiod_direction_output(master->gpio_mux, 1);
+ gpiod_direction_output(master->gpio_enable, 1);
+
+ /* Those are under ColdFire control, let it configure them */
+ rc = aspeed_gpio_copro_grab_gpio(master->gpio_clk);
+ if (rc) {
+ dev_err(master->dev, "failed to assign clock gpio to coprocessor\n");
+ return rc;
+ }
+ rc = aspeed_gpio_copro_grab_gpio(master->gpio_data);
+ if (rc) {
+ dev_err(master->dev, "failed to assign data gpio to coprocessor\n");
+ aspeed_gpio_copro_release_gpio(master->gpio_clk);
+ return rc;
+ }
+ rc = aspeed_gpio_copro_grab_gpio(master->gpio_trans);
+ if (rc) {
+ dev_err(master->dev, "failed to assign trans gpio to coprocessor\n");
+ aspeed_gpio_copro_release_gpio(master->gpio_clk);
+ aspeed_gpio_copro_release_gpio(master->gpio_data);
+ return rc;
+ }
+ return 0;
+}
+
+static void release_copro_gpios(struct fsi_master_acf *master)
+{
+ aspeed_gpio_copro_release_gpio(master->gpio_clk);
+ aspeed_gpio_copro_release_gpio(master->gpio_data);
+ aspeed_gpio_copro_release_gpio(master->gpio_trans);
+}
+
+static int load_copro_firmware(struct fsi_master_acf *master)
+{
+#define MAX_FW_NAME 32
+ char name[MAX_FW_NAME + 1] = {0};
+ const struct firmware *fw;
+ const char *pl_name;
+ int rc;
+
+ pl_name = of_get_property(dev_of_node(master->dev), "fw-name", NULL);
+ if (!pl_name) {
+ dev_err(master->dev, "Missing 'fw-name' property !\n");
+ return -ENODEV;
+ }
+ snprintf(name, MAX_FW_NAME, "cf-fsi-%s.bin", pl_name);
+ rc = request_firmware(&fw, name, master->dev);
+ if (rc) {
+ dev_err(master->dev, "Error %d to load firwmare '%s' !\n", rc, name);
+ return rc;
+ }
+ if (fw->size > master->cf_mem_size) {
+ dev_err(master->dev, "FW size (%zd) bigger than memory reserve (%zd)\n",
+ fw->size, master->cf_mem_size);
+ rc = -ENOMEM;
+ } else {
+ memcpy_toio(master->cf_mem, fw->data, fw->size);
+ }
+ release_firmware(fw);
+
+ return rc;
+}
+
+static int check_firmware_image(struct fsi_master_acf *master)
+{
+ u32 platform_sig, fw_sig, fw_vers, fw_api, fw_options;
+ int rc;
+
+ rc = of_property_read_u32(dev_of_node(master->dev), "fw-platform-sig", &platform_sig);
+ if (rc) {
+ dev_err(master->dev, "Missing 'fw-platform-sig' property\n");
+ return -ENODEV;
+ }
+
+ fw_sig = be16_to_cpu(readw(master->cf_mem + HDR_OFFSET + HDR_SYS_SIG));
+ fw_vers = be16_to_cpu(readw(master->cf_mem + HDR_OFFSET + HDR_FW_VERS));
+ fw_api = be16_to_cpu(readw(master->cf_mem + HDR_OFFSET + HDR_API_VERS));
+ fw_options = be32_to_cpu(readl(master->cf_mem + HDR_OFFSET + HDR_FW_OPTIONS));
+ master->trace_enabled = !!(fw_options & FW_OPTION_TRACE_EN);
+
+ /* Check version and signature */
+ dev_info(master->dev, "ColdFire initialized, firmware v%d API v%d.%d (trace %s)\n",
+ fw_vers, fw_api >> 8, fw_api & 0xff,
+ master->trace_enabled ? "enabled" : "disabled");
+
+ if ((fw_api >> 8) != API_VERSION_MAJ) {
+ dev_err(master->dev, "Unsupported coprocessor API version !\n");
+ return -ENODEV;
+ }
+
+ if (platform_sig != fw_sig) {
+ dev_err(master->dev, "Platform signature mismatch ! Want %04x got %04x\n",
+ platform_sig, fw_sig);
+ return -ENODEV;
+ }
+ return 0;
+}
+
+static int copro_enable_sw_irq(struct fsi_master_acf *master)
+{
+ int timeout;
+ u32 val;
+ /*
+ * Enable coprocessor interrupt input. I've had problems getting the
+ * value to stick, so try in a loop
+ */
+ for (timeout = 0; timeout < 10; timeout++) {
+ writel(0x2, master->cvic + CVIC_EN_REG);
+ val = readl(master->cvic + CVIC_EN_REG);
+ if (val & 2)
+ break;
+ msleep(1);
+ }
+ if (!(val & 2)) {
+ dev_err(master->dev, "Failed to enable coprocessor interrupt !\n");
+ return -ENODEV;
+ }
+ return 0;
+}
+
+static int fsi_master_acf_setup(struct fsi_master_acf *master)
+{
+ int timeout, rc;
+ u32 val;
+
+ /* Make sure the ColdFire is stopped */
+ reset_cf(master);
+
+ /*
+ * Clear SRAM. This needs to happen before we setup the GPIOs
+ * as we might start trying to arbitrate as soon as that happens.
+ */
+ memset_io(master->sram, 0, SRAM_SIZE);
+
+ /* Configure GPIOs */
+ rc = setup_gpios_for_copro(master);
+ if (rc)
+ return rc;
+
+ /* Load the firmware into the reserved memory */
+ rc = load_copro_firmware(master);
+ if (rc)
+ return rc;
+
+ /* Read signature and check versions */
+ rc = check_firmware_image(master);
+ if (rc)
+ return rc;
+
+ /* Setup coldfire memory map */
+ if (master->is_ast2500)
+ setup_ast2500_cf_maps(master);
+ else
+ setup_ast2400_cf_maps(master);
+
+ /* Start the ColdFire */
+ start_cf(master);
+
+ /* Wait for status register to indicate command completion
+ * which signals the initialization is complete
+ */
+ for (timeout = 0; timeout < 10; timeout++) {
+ val = readb(master->sram + CF_STARTED);
+ if (val)
+ break;
+ msleep(1);
+ };
+ if (!val) {
+ dev_err(master->dev, "Coprocessor startup timeout !\n");
+ rc = -ENODEV;
+ goto err;
+ }
+
+ /* Configure echo & send delay */
+ writeb(master->t_send_delay, master->sram + SEND_DLY_REG);
+ writeb(master->t_echo_delay, master->sram + ECHO_DLY_REG);
+
+ /* Enable SW interrupt to copro if any */
+ if (master->cvic) {
+ rc = copro_enable_sw_irq(master);
+ if (rc)
+ goto err;
+ }
+ return 0;
+ err:
+ /* An error occurred, don't leave the coprocessor running */
+ reset_cf(master);
+
+ /* Release the GPIOs */
+ release_copro_gpios(master);
+
+ return rc;
+}
+
+
+static void fsi_master_acf_terminate(struct fsi_master_acf *master)
+{
+ unsigned long flags;
+
+ /*
+ * A GPIO arbitration requestion could come in while this is
+ * happening. To avoid problems, we disable interrupts so it
+ * cannot preempt us on this CPU
+ */
+
+ local_irq_save(flags);
+
+ /* Stop the coprocessor */
+ reset_cf(master);
+
+ /* We mark the copro not-started */
+ writel(0, master->sram + CF_STARTED);
+
+ /* We mark the ARB register as having given up arbitration to
+ * deal with a potential race with the arbitration request
+ */
+ writeb(ARB_ARM_ACK, master->sram + ARB_REG);
+
+ local_irq_restore(flags);
+
+ /* Return the GPIOs to the ARM */
+ release_copro_gpios(master);
+}
+
+static void fsi_master_acf_setup_external(struct fsi_master_acf *master)
+{
+ /* Setup GPIOs for external FSI master (FSP box) */
+ gpiod_direction_output(master->gpio_mux, 0);
+ gpiod_direction_output(master->gpio_trans, 0);
+ gpiod_direction_output(master->gpio_enable, 1);
+ gpiod_direction_input(master->gpio_clk);
+ gpiod_direction_input(master->gpio_data);
+}
+
+static int fsi_master_acf_link_enable(struct fsi_master *_master, int link)
+{
+ struct fsi_master_acf *master = to_fsi_master_acf(_master);
+ int rc = -EBUSY;
+
+ if (link != 0)
+ return -ENODEV;
+
+ mutex_lock(&master->lock);
+ if (!master->external_mode) {
+ gpiod_set_value(master->gpio_enable, 1);
+ rc = 0;
+ }
+ mutex_unlock(&master->lock);
+
+ return rc;
+}
+
+static int fsi_master_acf_link_config(struct fsi_master *_master, int link,
+ u8 t_send_delay, u8 t_echo_delay)
+{
+ struct fsi_master_acf *master = to_fsi_master_acf(_master);
+
+ if (link != 0)
+ return -ENODEV;
+
+ mutex_lock(&master->lock);
+ master->t_send_delay = t_send_delay;
+ master->t_echo_delay = t_echo_delay;
+ dev_dbg(master->dev, "Changing delays: send=%d echo=%d\n",
+ t_send_delay, t_echo_delay);
+ writeb(master->t_send_delay, master->sram + SEND_DLY_REG);
+ writeb(master->t_echo_delay, master->sram + ECHO_DLY_REG);
+ mutex_unlock(&master->lock);
+
+ return 0;
+}
+
+static ssize_t external_mode_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct fsi_master_acf *master = dev_get_drvdata(dev);
+
+ return snprintf(buf, PAGE_SIZE - 1, "%u\n",
+ master->external_mode ? 1 : 0);
+}
+
+static ssize_t external_mode_store(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ struct fsi_master_acf *master = dev_get_drvdata(dev);
+ unsigned long val;
+ bool external_mode;
+ int err;
+
+ err = kstrtoul(buf, 0, &val);
+ if (err)
+ return err;
+
+ external_mode = !!val;
+
+ mutex_lock(&master->lock);
+
+ if (external_mode == master->external_mode) {
+ mutex_unlock(&master->lock);
+ return count;
+ }
+
+ master->external_mode = external_mode;
+ if (master->external_mode) {
+ fsi_master_acf_terminate(master);
+ fsi_master_acf_setup_external(master);
+ } else
+ fsi_master_acf_setup(master);
+
+ mutex_unlock(&master->lock);
+
+ fsi_master_rescan(&master->master);
+
+ return count;
+}
+
+static DEVICE_ATTR(external_mode, 0664,
+ external_mode_show, external_mode_store);
+
+static int fsi_master_acf_gpio_request(void *data)
+{
+ struct fsi_master_acf *master = data;
+ int timeout;
+ u8 val;
+
+ /* Note: This doesn't require holding out mutex */
+
+ /* Write reqest */
+ writeb(ARB_ARM_REQ, master->sram + ARB_REG);
+
+ /* Read back to avoid ordering issue */
+ (void)readb(master->sram + ARB_REG);
+
+ /*
+ * There is a race (which does happen at boot time) when we get an
+ * arbitration request as we are either about to or just starting
+ * the coprocessor.
+ *
+ * To handle it, we first check if we are running. If not yet we
+ * check whether the copro is started in the SCU.
+ *
+ * If it's not started, we can basically just assume we have arbitration
+ * and return. Otherwise, we wait normally expecting for the arbitration
+ * to eventually complete.
+ */
+ if (readl(master->sram + CF_STARTED) == 0) {
+ unsigned int reg = 0;
+
+ regmap_read(master->scu, SCU_COPRO_CTRL, ®);
+ if (!reg & SCU_COPRO_CLK_EN)
+ return 0;
+ }
+
+ /* Ring doorbell if any */
+ if (master->cvic)
+ writel(0x2, master->cvic + CVIC_TRIG_REG);
+
+ for (timeout = 0; timeout < 10000; timeout++) {
+ val = readb(master->sram + ARB_REG);
+ if (val != ARB_ARM_REQ)
+ break;
+ udelay(1);
+ }
+
+ /* If it failed, override anyway */
+ if (val != ARB_ARM_ACK)
+ dev_warn(master->dev, "GPIO request arbitration timeout\n");
+
+ return 0;
+}
+
+static int fsi_master_acf_gpio_release(void *data)
+{
+ struct fsi_master_acf *master = data;
+
+ /* Write release */
+ writeb(0, master->sram + ARB_REG);
+
+ /* Ring doorbell if any */
+ if (master->cvic)
+ writel(0x2, master->cvic + CVIC_TRIG_REG);
+
+ return 0;
+}
+
+static void fsi_master_acf_release(struct device *dev)
+{
+ struct fsi_master_acf *master = to_fsi_master_acf(dev_to_fsi_master(dev));
+
+ /* Cleanup, stop coprocessor */
+ mutex_lock(&master->lock);
+ fsi_master_acf_terminate(master);
+ aspeed_gpio_copro_set_ops(NULL, NULL);
+ mutex_unlock(&master->lock);
+
+ /* Free resources */
+ gen_pool_free(master->sram_pool, (unsigned long)master->sram, SRAM_SIZE);
+ of_node_put(dev_of_node(master->dev));
+
+ kfree(dev);
+}
+
+static const struct aspeed_gpio_copro_ops fsi_master_acf_gpio_ops = {
+ .request_access = fsi_master_acf_gpio_request,
+ .release_access = fsi_master_acf_gpio_release,
+};
+
+static int fsi_master_acf_probe(struct platform_device *pdev)
+{
+ struct device_node *np, *mnode = dev_of_node(&pdev->dev);
+ struct genpool_data_fixed gpdf;
+ struct fsi_master_acf *master;
+ struct gpio_desc *gpio;
+ struct resource res;
+ u32 cf_mem_align;
+ int rc;
+
+ master = devm_kzalloc(&pdev->dev, sizeof(*master), GFP_KERNEL);
+ if (!master)
+ return -ENOMEM;
+
+ master->dev = &pdev->dev;
+ master->master.dev.parent = master->dev;
+ master->last_addr = LAST_ADDR_INVALID;
+
+ /* AST2400 vs. AST2500 */
+ master->is_ast2500 = of_device_is_compatible(mnode, "ibm,fsi-master-ast2500-cf");
+
+ /* Grab the SCU, we'll need to access it to configure the coprocessor */
+ if (master->is_ast2500)
+ master->scu = syscon_regmap_lookup_by_compatible("aspeed,ast2500-scu");
+ else
+ master->scu = syscon_regmap_lookup_by_compatible("aspeed,ast2400-scu");
+ if (IS_ERR(master->scu)) {
+ dev_err(&pdev->dev, "failed to find SCU regmap\n");
+ return PTR_ERR(master->scu);
+ }
+
+ /* Grab all the GPIOs we need */
+ gpio = devm_gpiod_get(&pdev->dev, "clock", 0);
+ if (IS_ERR(gpio)) {
+ dev_err(&pdev->dev, "failed to get clock gpio\n");
+ return PTR_ERR(gpio);
+ }
+ master->gpio_clk = gpio;
+
+ gpio = devm_gpiod_get(&pdev->dev, "data", 0);
+ if (IS_ERR(gpio)) {
+ dev_err(&pdev->dev, "failed to get data gpio\n");
+ return PTR_ERR(gpio);
+ }
+ master->gpio_data = gpio;
+
+ /* Optional GPIOs */
+ gpio = devm_gpiod_get_optional(&pdev->dev, "trans", 0);
+ if (IS_ERR(gpio)) {
+ dev_err(&pdev->dev, "failed to get trans gpio\n");
+ return PTR_ERR(gpio);
+ }
+ master->gpio_trans = gpio;
+
+ gpio = devm_gpiod_get_optional(&pdev->dev, "enable", 0);
+ if (IS_ERR(gpio)) {
+ dev_err(&pdev->dev, "failed to get enable gpio\n");
+ return PTR_ERR(gpio);
+ }
+ master->gpio_enable = gpio;
+
+ gpio = devm_gpiod_get_optional(&pdev->dev, "mux", 0);
+ if (IS_ERR(gpio)) {
+ dev_err(&pdev->dev, "failed to get mux gpio\n");
+ return PTR_ERR(gpio);
+ }
+ master->gpio_mux = gpio;
+
+ /* Grab the reserved memory region (use DMA API instead ?) */
+ np = of_parse_phandle(mnode, "memory-region", 0);
+ if (!np) {
+ dev_err(&pdev->dev, "Didn't find reserved memory\n");
+ return -EINVAL;
+ }
+ rc = of_address_to_resource(np, 0, &res);
+ of_node_put(np);
+ if (rc) {
+ dev_err(&pdev->dev, "Couldn't address to resource for reserved memory\n");
+ return -ENOMEM;
+ }
+ master->cf_mem_size = resource_size(&res);
+ master->cf_mem_addr = (u32)res.start;
+ cf_mem_align = master->is_ast2500 ? 0x00100000 : 0x00200000;
+ if (master->cf_mem_addr & (cf_mem_align - 1)) {
+ dev_err(&pdev->dev, "Reserved memory has insufficient alignment\n");
+ return -ENOMEM;
+ }
+ master->cf_mem = devm_ioremap_resource(&pdev->dev, &res);
+ if (IS_ERR(master->cf_mem)) {
+ rc = PTR_ERR(master->cf_mem);
+ dev_err(&pdev->dev, "Error %d mapping coldfire memory\n", rc);
+ return rc;
+ }
+ dev_dbg(&pdev->dev, "DRAM allocation @%x\n", master->cf_mem_addr);
+
+ /* AST2500 has a SW interrupt to the coprocessor */
+ if (master->is_ast2500) {
+ /* Grab the CVIC (ColdFire interrupts controller) */
+ np = of_parse_phandle(mnode, "cvic", 0);
+ if (!np) {
+ dev_err(&pdev->dev, "Didn't find CVIC\n");
+ return -EINVAL;
+ }
+ master->cvic = devm_of_iomap(&pdev->dev, np, 0, NULL);
+ if (IS_ERR(master->cvic)) {
+ rc = PTR_ERR(master->cvic);
+ dev_err(&pdev->dev, "Error %d mapping CVIC\n", rc);
+ return rc;
+ }
+ rc = of_property_read_u32(np, "copro-sw-interrupts",
+ &master->cvic_sw_irq);
+ if (rc) {
+ dev_err(&pdev->dev, "Can't find coprocessor SW interrupt\n");
+ return rc;
+ }
+ }
+
+ /* Grab the SRAM */
+ master->sram_pool = of_gen_pool_get(dev_of_node(&pdev->dev), "sram", 0);
+ if (!master->sram_pool) {
+ rc = -ENODEV;
+ dev_err(&pdev->dev, "Can't find sram pool\n");
+ return rc;
+ }
+
+ /* Current microcode only deals with fixed location in SRAM */
+ gpdf.offset = 0;
+ master->sram = (void __iomem *)gen_pool_alloc_algo(master->sram_pool, SRAM_SIZE,
+ gen_pool_fixed_alloc, &gpdf);
+ if (!master->sram) {
+ rc = -ENOMEM;
+ dev_err(&pdev->dev, "Failed to allocate sram from pool\n");
+ return rc;
+ }
+ dev_dbg(&pdev->dev, "SRAM allocation @%lx\n",
+ (unsigned long)gen_pool_virt_to_phys(master->sram_pool,
+ (unsigned long)master->sram));
+
+ /*
+ * Hookup with the GPIO driver for arbitration of GPIO banks
+ * ownership.
+ */
+ aspeed_gpio_copro_set_ops(&fsi_master_acf_gpio_ops, master);
+
+ /* Default FSI command delays */
+ master->t_send_delay = FSI_SEND_DELAY_CLOCKS;
+ master->t_echo_delay = FSI_ECHO_DELAY_CLOCKS;
+
+ master->master.n_links = 1;
+ master->master.flags = FSI_MASTER_FLAG_SWCLOCK;
+ master->master.read = fsi_master_acf_read;
+ master->master.write = fsi_master_acf_write;
+ master->master.term = fsi_master_acf_term;
+ master->master.send_break = fsi_master_acf_break;
+ master->master.link_enable = fsi_master_acf_link_enable;
+ master->master.link_config = fsi_master_acf_link_config;
+ master->master.dev.of_node = of_node_get(dev_of_node(master->dev));
+ master->master.dev.release = fsi_master_acf_release;
+ platform_set_drvdata(pdev, master);
+ mutex_init(&master->lock);
+
+ mutex_lock(&master->lock);
+ rc = fsi_master_acf_setup(master);
+ mutex_unlock(&master->lock);
+ if (rc)
+ goto release_dev;
+
+ rc = device_create_file(&pdev->dev, &dev_attr_external_mode);
+ if (rc)
+ goto stop_copro;
+
+ rc = fsi_master_register(&master->master);
+ if (!rc)
+ return 0;
+
+ device_remove_file(master->dev, &dev_attr_external_mode);
+
+ stop_copro:
+ fsi_master_acf_terminate(master);
+ release_dev:
+ aspeed_gpio_copro_set_ops(NULL, NULL);
+ gen_pool_free(master->sram_pool, (unsigned long)master->sram, SRAM_SIZE);
+ of_node_put(dev_of_node(master->dev));
+
+ return rc;
+}
+
+
+static int fsi_master_acf_remove(struct platform_device *pdev)
+{
+ struct fsi_master_acf *master = platform_get_drvdata(pdev);
+
+ device_remove_file(master->dev, &dev_attr_external_mode);
+
+ fsi_master_unregister(&master->master);
+
+ return 0;
+}
+
+static const struct of_device_id fsi_master_acf_match[] = {
+ { .compatible = "ibm,fsi-master-ast2400-cf" },
+ { .compatible = "ibm,fsi-master-ast2500-cf" },
+ { },
+};
+
+static struct platform_driver fsi_master_acf = {
+ .driver = {
+ .name = "fsi-master-acf",
+ .of_match_table = fsi_master_acf_match,
+ },
+ .probe = fsi_master_acf_probe,
+ .remove = fsi_master_acf_remove,
+};
+
+module_platform_driver(fsi_master_acf);
+MODULE_LICENSE("GPL");
diff --git a/include/trace/events/fsi_master_ast_cf.h b/include/trace/events/fsi_master_ast_cf.h
new file mode 100644
index 000000000000..424da0172cdd
--- /dev/null
+++ b/include/trace/events/fsi_master_ast_cf.h
@@ -0,0 +1,150 @@
+
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM fsi_master_ast_cf
+
+#if !defined(_TRACE_FSI_MASTER_ACF_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_FSI_MASTER_ACF_H
+
+#include <linux/tracepoint.h>
+
+TRACE_EVENT(fsi_master_acf_copro_command,
+ TP_PROTO(const struct fsi_master_acf *master, uint32_t op),
+ TP_ARGS(master, op),
+ TP_STRUCT__entry(
+ __field(int, master_idx)
+ __field(uint32_t, op)
+ ),
+ TP_fast_assign(
+ __entry->master_idx = master->master.idx;
+ __entry->op = op;
+ ),
+ TP_printk("fsi-acf%d command %08x",
+ __entry->master_idx, __entry->op
+ )
+);
+
+TRACE_EVENT(fsi_master_acf_send_request,
+ TP_PROTO(const struct fsi_master_acf *master, const struct fsi_msg *cmd, bool is_write),
+ TP_ARGS(master, cmd, is_write),
+ TP_STRUCT__entry(
+ __field(int, master_idx)
+ __field(uint64_t, msg)
+ __field(u8, bits)
+ __field(bool, is_write)
+ ),
+ TP_fast_assign(
+ __entry->master_idx = master->master.idx;
+ __entry->msg = cmd->msg;
+ __entry->bits = cmd->bits;
+ __entry->is_write = is_write;
+ ),
+ TP_printk("fsi-acf%d cmd: %016llx/%d (%c)",
+ __entry->master_idx, (unsigned long long)__entry->msg,
+ __entry->bits, __entry->is_write ? 'W' : 'R'
+ )
+);
+
+TRACE_EVENT(fsi_master_acf_copro_response,
+ TP_PROTO(const struct fsi_master_acf *master, u8 rtag, u8 rcrc, __be32 rdata, bool crc_ok),
+ TP_ARGS(master, rtag, rcrc, rdata, crc_ok),
+ TP_STRUCT__entry(
+ __field(int, master_idx)
+ __field(u8, rtag)
+ __field(u8, rcrc)
+ __field(u32, rdata)
+ __field(bool, crc_ok)
+ ),
+ TP_fast_assign(
+ __entry->master_idx = master->master.idx;
+ __entry->rtag = rtag;
+ __entry->rcrc = rcrc;
+ __entry->rdata = be32_to_cpu(rdata);
+ __entry->crc_ok = crc_ok;
+ ),
+ TP_printk("fsi-acf%d rsp: tag=%04x crc=%04x data=%08x %c\n",
+ __entry->master_idx, __entry->rtag, __entry->rcrc,
+ __entry->rdata, __entry->crc_ok ? ' ' : '!'
+ )
+);
+
+TRACE_EVENT(fsi_master_acf_crc_rsp_error,
+ TP_PROTO(const struct fsi_master_acf *master, int retries),
+ TP_ARGS(master, retries),
+ TP_STRUCT__entry(
+ __field(int, master_idx)
+ __field(int, retries)
+ ),
+ TP_fast_assign(
+ __entry->master_idx = master->master.idx;
+ __entry->retries = retries;
+ ),
+ TP_printk("fsi-acf%d CRC error in response retry %d",
+ __entry->master_idx, __entry->retries
+ )
+);
+
+TRACE_EVENT(fsi_master_acf_poll_response_busy,
+ TP_PROTO(const struct fsi_master_acf *master, int busy_count),
+ TP_ARGS(master, busy_count),
+ TP_STRUCT__entry(
+ __field(int, master_idx)
+ __field(int, busy_count)
+ ),
+ TP_fast_assign(
+ __entry->master_idx = master->master.idx;
+ __entry->busy_count = busy_count;
+ ),
+ TP_printk("fsi-acf%d: device reported busy %d times",
+ __entry->master_idx, __entry->busy_count
+ )
+);
+
+TRACE_EVENT(fsi_master_acf_cmd_abs_addr,
+ TP_PROTO(const struct fsi_master_acf *master, u32 addr),
+ TP_ARGS(master, addr),
+ TP_STRUCT__entry(
+ __field(int, master_idx)
+ __field(u32, addr)
+ ),
+ TP_fast_assign(
+ __entry->master_idx = master->master.idx;
+ __entry->addr = addr;
+ ),
+ TP_printk("fsi-acf%d: Sending ABS_ADR %06x",
+ __entry->master_idx, __entry->addr
+ )
+);
+
+TRACE_EVENT(fsi_master_acf_cmd_rel_addr,
+ TP_PROTO(const struct fsi_master_acf *master, u32 rel_addr),
+ TP_ARGS(master, rel_addr),
+ TP_STRUCT__entry(
+ __field(int, master_idx)
+ __field(u32, rel_addr)
+ ),
+ TP_fast_assign(
+ __entry->master_idx = master->master.idx;
+ __entry->rel_addr = rel_addr;
+ ),
+ TP_printk("fsi-acf%d: Sending REL_ADR %03x",
+ __entry->master_idx, __entry->rel_addr
+ )
+);
+
+TRACE_EVENT(fsi_master_acf_cmd_same_addr,
+ TP_PROTO(const struct fsi_master_acf *master),
+ TP_ARGS(master),
+ TP_STRUCT__entry(
+ __field(int, master_idx)
+ ),
+ TP_fast_assign(
+ __entry->master_idx = master->master.idx;
+ ),
+ TP_printk("fsi-acf%d: Sending SAME_ADR",
+ __entry->master_idx
+ )
+);
+
+#endif /* _TRACE_FSI_MASTER_ACF_H */
+
+#include <trace/define_trace.h>
--
2.17.1
^ permalink raw reply related
* [PATCH 11/14] dt-bindings: fsi: Document binding for the fsi-master-ast-cf "device"
From: Benjamin Herrenschmidt @ 2018-06-26 23:26 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180626232605.13420-1-benh@kernel.crashing.org>
This isn't per-se a real device, it's a pseudo-device that
represents the use of the Aspeed built-in ColdFire to
implement the FSI protocol by bitbanging the GPIOs instead
of doing it from the ARM core.
Thus it's a drop-in replacement for the existing
fsi-master-gpio pseudo-device for use on systems for which
a corresponding firmware file exists. It has most of the
same properties, plus some more needed to operate the
coprocessor.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
.../bindings/fsi/fsi-master-ast-cf.txt | 36 +++++++++++++++++++
1 file changed, 36 insertions(+)
create mode 100644 Documentation/devicetree/bindings/fsi/fsi-master-ast-cf.txt
diff --git a/Documentation/devicetree/bindings/fsi/fsi-master-ast-cf.txt b/Documentation/devicetree/bindings/fsi/fsi-master-ast-cf.txt
new file mode 100644
index 000000000000..50913ae685cc
--- /dev/null
+++ b/Documentation/devicetree/bindings/fsi/fsi-master-ast-cf.txt
@@ -0,0 +1,36 @@
+Device-tree bindings for ColdFire offloaded gpio-based FSI master driver
+------------------------------------------------------------------------
+
+Required properties:
+ - compatible =
+ "fsi-master-ast-2400-cf" for an AST2400 based system
+ or
+ "fsi-master-ast-2500-cf" for an AST2500 based system
+
+ - clock-gpios = <gpio-descriptor>; : GPIO for FSI clock
+ - data-gpios = <gpio-descriptor>; : GPIO for FSI data signal
+ - enable-gpios = <gpio-descriptor>; : GPIO for enable signal
+ - trans-gpios = <gpio-descriptor>; : GPIO for voltage translator enable
+ - mux-gpios = <gpio-descriptor>; : GPIO for pin multiplexing with other
+ functions (eg, external FSI masters)
+ - memory-region = <phandle>; : Reference to the reserved memory for
+ the ColdFire. Must be 2M aligned on
+ AST2400 and 1M aligned on AST2500
+ - sram = <phandle>; : Reference to the SRAM node.
+ - cvic = <phandle>; : Reference to the CVIC node.
+ - fw-name = <string>; : The name used to construct the firmware
+ file name (cf-fsi-<name>.bin)
+ - fw-platform-sig = <value>; : A signature value that must match the one
+ contained in the firmware image. Known
+ values are listed in the firmware interface
+ file cf-fsi-fw.h
+Examples:
+
+ fsi-master {
+ compatible = "fsi-master-gpio", "fsi-master";
+ clock-gpios = <&gpio 0>;
+ data-gpios = <&gpio 1>;
+ enable-gpios = <&gpio 2>;
+ trans-gpios = <&gpio 3>;
+ mux-gpios = <&gpio 4>;
+ }
--
2.17.1
^ permalink raw reply related
* [PATCH 10/14] fsi: Move various master definitions to a common header
From: Benjamin Herrenschmidt @ 2018-06-26 23:26 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180626232605.13420-1-benh@kernel.crashing.org>
This moves the definitions for various protocol details
(message & response codes, delays etc...) out of
fsi-master-gpio.c to fsi-master.h in order to share them
with other master implementations.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
drivers/fsi/fsi-master-gpio.c | 29 -----------------------------
drivers/fsi/fsi-master.h | 33 +++++++++++++++++++++++++++++++++
2 files changed, 33 insertions(+), 29 deletions(-)
diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c
index bad7951a2677..edc257e40102 100644
--- a/drivers/fsi/fsi-master-gpio.c
+++ b/drivers/fsi/fsi-master-gpio.c
@@ -17,35 +17,6 @@
#include "fsi-master.h"
#define FSI_GPIO_STD_DLY 1 /* Standard pin delay in nS */
-#define FSI_ECHO_DELAY_CLOCKS 16 /* Number clocks for echo delay */
-#define FSI_SEND_DELAY_CLOCKS 16 /* Number clocks for send delay */
-#define FSI_PRE_BREAK_CLOCKS 50 /* Number clocks to prep for break */
-#define FSI_BREAK_CLOCKS 256 /* Number of clocks to issue break */
-#define FSI_POST_BREAK_CLOCKS 16000 /* Number clocks to set up cfam */
-#define FSI_INIT_CLOCKS 5000 /* Clock out any old data */
-#define FSI_MASTER_DPOLL_CLOCKS 50 /* < 21 will cause slave to hang */
-#define FSI_MASTER_EPOLL_CLOCKS 50 /* Number of clocks for E_POLL retry */
-
-#define FSI_CRC_ERR_RETRIES 10
-
-#define FSI_CMD_DPOLL 0x2
-#define FSI_CMD_EPOLL 0x3
-#define FSI_CMD_TERM 0x3f
-#define FSI_CMD_ABS_AR 0x4
-#define FSI_CMD_REL_AR 0x5
-#define FSI_CMD_SAME_AR 0x3 /* but only a 2-bit opcode... */
-
-/* Slave responses */
-#define FSI_RESP_ACK 0 /* Success */
-#define FSI_RESP_BUSY 1 /* Slave busy */
-#define FSI_RESP_ERRA 2 /* Any (misc) Error */
-#define FSI_RESP_ERRC 3 /* Slave reports master CRC error */
-
-#define FSI_MASTER_MAX_BUSY 200
-
-#define FSI_MASTER_MTOE_COUNT 1000
-#define FSI_CRC_SIZE 4
-
#define LAST_ADDR_INVALID 0x1
struct fsi_master_gpio {
diff --git a/drivers/fsi/fsi-master.h b/drivers/fsi/fsi-master.h
index 7d619c68ab9b..f653f75da7be 100644
--- a/drivers/fsi/fsi-master.h
+++ b/drivers/fsi/fsi-master.h
@@ -19,6 +19,39 @@
#include <linux/device.h>
+/* Various protocol delays */
+#define FSI_ECHO_DELAY_CLOCKS 16 /* Number clocks for echo delay */
+#define FSI_SEND_DELAY_CLOCKS 16 /* Number clocks for send delay */
+#define FSI_PRE_BREAK_CLOCKS 50 /* Number clocks to prep for break */
+#define FSI_BREAK_CLOCKS 256 /* Number of clocks to issue break */
+#define FSI_POST_BREAK_CLOCKS 16000 /* Number clocks to set up cfam */
+#define FSI_INIT_CLOCKS 5000 /* Clock out any old data */
+#define FSI_MASTER_DPOLL_CLOCKS 50 /* < 21 will cause slave to hang */
+#define FSI_MASTER_EPOLL_CLOCKS 50 /* Number of clocks for E_POLL retry */
+
+/* Various retry maximums */
+#define FSI_CRC_ERR_RETRIES 10
+#define FSI_MASTER_MAX_BUSY 200
+#define FSI_MASTER_MTOE_COUNT 1000
+
+/* Command encodings */
+#define FSI_CMD_DPOLL 0x2
+#define FSI_CMD_EPOLL 0x3
+#define FSI_CMD_TERM 0x3f
+#define FSI_CMD_ABS_AR 0x4
+#define FSI_CMD_REL_AR 0x5
+#define FSI_CMD_SAME_AR 0x3 /* but only a 2-bit opcode... */
+
+/* Slave responses */
+#define FSI_RESP_ACK 0 /* Success */
+#define FSI_RESP_BUSY 1 /* Slave busy */
+#define FSI_RESP_ERRA 2 /* Any (misc) Error */
+#define FSI_RESP_ERRC 3 /* Slave reports master CRC error */
+
+/* Misc */
+#define FSI_CRC_SIZE 4
+
+/* fsi-master definition and flags */
#define FSI_MASTER_FLAG_SWCLOCK 0x1
struct fsi_master {
--
2.17.1
^ permalink raw reply related
* [PATCH 09/14] fsi: master-gpio: Add missing release function
From: Benjamin Herrenschmidt @ 2018-06-26 23:26 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180626232605.13420-1-benh@kernel.crashing.org>
The embedded struct device needs a release function to be
able to successfully remove the driver.
We remove the devm_gpiod_put() as they are unnecessary
(the resources will be released automatically) and because
fsi_master_unregister() will cause the master structure to
be freed.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
drivers/fsi/fsi-master-gpio.c | 28 +++++++++++++++++-----------
1 file changed, 17 insertions(+), 11 deletions(-)
diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c
index 91d89597784a..bad7951a2677 100644
--- a/drivers/fsi/fsi-master-gpio.c
+++ b/drivers/fsi/fsi-master-gpio.c
@@ -792,6 +792,15 @@ static ssize_t external_mode_store(struct device *dev,
static DEVICE_ATTR(external_mode, 0664,
external_mode_show, external_mode_store);
+static void fsi_master_gpio_release(struct device *dev)
+{
+ struct fsi_master_gpio *master = to_fsi_master_gpio(dev_to_fsi_master(dev));
+
+ of_node_put(dev_of_node(master->dev));
+
+ kfree(dev);
+}
+
static int fsi_master_gpio_probe(struct platform_device *pdev)
{
struct fsi_master_gpio *master;
@@ -805,6 +814,7 @@ static int fsi_master_gpio_probe(struct platform_device *pdev)
master->dev = &pdev->dev;
master->master.dev.parent = master->dev;
master->master.dev.of_node = of_node_get(dev_of_node(master->dev));
+ master->master.dev.release = fsi_master_gpio_release;
master->last_addr = LAST_ADDR_INVALID;
gpio = devm_gpiod_get(&pdev->dev, "clock", 0);
@@ -871,25 +881,21 @@ static int fsi_master_gpio_probe(struct platform_device *pdev)
if (rc)
return rc;
- return fsi_master_register(&master->master);
+ rc = fsi_master_register(&master->master);
+ if (rc)
+ device_remove_file(&pdev->dev, &dev_attr_external_mode);
+ return rc;
}
+
static int fsi_master_gpio_remove(struct platform_device *pdev)
{
struct fsi_master_gpio *master = platform_get_drvdata(pdev);
- devm_gpiod_put(&pdev->dev, master->gpio_clk);
- devm_gpiod_put(&pdev->dev, master->gpio_data);
- if (master->gpio_trans)
- devm_gpiod_put(&pdev->dev, master->gpio_trans);
- if (master->gpio_enable)
- devm_gpiod_put(&pdev->dev, master->gpio_enable);
- if (master->gpio_mux)
- devm_gpiod_put(&pdev->dev, master->gpio_mux);
- fsi_master_unregister(&master->master);
+ device_remove_file(&pdev->dev, &dev_attr_external_mode);
- of_node_put(master->master.dev.of_node);
+ fsi_master_unregister(&master->master);
return 0;
}
--
2.17.1
^ permalink raw reply related
* [PATCH 08/14] fsi: master-gpio: Remove "GPIO" prefix on some definitions
From: Benjamin Herrenschmidt @ 2018-06-26 23:25 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180626232605.13420-1-benh@kernel.crashing.org>
Some definitions are generic to the FSI protocol or any
give master implementation. Rename them to remove the
"GPIO" prefix in preparation for moving them to a common
header.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
drivers/fsi/fsi-master-gpio.c | 70 ++++++++++++++++++-----------------
1 file changed, 36 insertions(+), 34 deletions(-)
diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c
index 2bc85514bb0c..91d89597784a 100644
--- a/drivers/fsi/fsi-master-gpio.c
+++ b/drivers/fsi/fsi-master-gpio.c
@@ -23,26 +23,28 @@
#define FSI_BREAK_CLOCKS 256 /* Number of clocks to issue break */
#define FSI_POST_BREAK_CLOCKS 16000 /* Number clocks to set up cfam */
#define FSI_INIT_CLOCKS 5000 /* Clock out any old data */
-#define FSI_GPIO_DPOLL_CLOCKS 50 /* < 21 will cause slave to hang */
-#define FSI_GPIO_EPOLL_CLOCKS 50 /* Number of clocks for E_POLL retry */
+#define FSI_MASTER_DPOLL_CLOCKS 50 /* < 21 will cause slave to hang */
+#define FSI_MASTER_EPOLL_CLOCKS 50 /* Number of clocks for E_POLL retry */
+
#define FSI_CRC_ERR_RETRIES 10
-#define FSI_GPIO_CMD_DPOLL 0x2
-#define FSI_GPIO_CMD_EPOLL 0x3
-#define FSI_GPIO_CMD_TERM 0x3f
-#define FSI_GPIO_CMD_ABS_AR 0x4
-#define FSI_GPIO_CMD_REL_AR 0x5
-#define FSI_GPIO_CMD_SAME_AR 0x3 /* but only a 2-bit opcode... */
+#define FSI_CMD_DPOLL 0x2
+#define FSI_CMD_EPOLL 0x3
+#define FSI_CMD_TERM 0x3f
+#define FSI_CMD_ABS_AR 0x4
+#define FSI_CMD_REL_AR 0x5
+#define FSI_CMD_SAME_AR 0x3 /* but only a 2-bit opcode... */
/* Slave responses */
-#define FSI_GPIO_RESP_ACK 0 /* Success */
-#define FSI_GPIO_RESP_BUSY 1 /* Slave busy */
-#define FSI_GPIO_RESP_ERRA 2 /* Any (misc) Error */
-#define FSI_GPIO_RESP_ERRC 3 /* Slave reports master CRC error */
+#define FSI_RESP_ACK 0 /* Success */
+#define FSI_RESP_BUSY 1 /* Slave busy */
+#define FSI_RESP_ERRA 2 /* Any (misc) Error */
+#define FSI_RESP_ERRC 3 /* Slave reports master CRC error */
+
+#define FSI_MASTER_MAX_BUSY 200
-#define FSI_GPIO_MAX_BUSY 200
-#define FSI_GPIO_MTOE_COUNT 1000
-#define FSI_GPIO_CRC_SIZE 4
+#define FSI_MASTER_MTOE_COUNT 1000
+#define FSI_CRC_SIZE 4
#define LAST_ADDR_INVALID 0x1
@@ -279,19 +281,19 @@ static void build_ar_command(struct fsi_master_gpio *master,
/* we still address the byte offset within the word */
addr_bits = 2;
opcode_bits = 2;
- opcode = FSI_GPIO_CMD_SAME_AR;
+ opcode = FSI_CMD_SAME_AR;
trace_fsi_master_gpio_cmd_same_addr(master);
} else if (check_relative_address(master, id, addr, &rel_addr)) {
/* 8 bits plus sign */
addr_bits = 9;
addr = rel_addr;
- opcode = FSI_GPIO_CMD_REL_AR;
+ opcode = FSI_CMD_REL_AR;
trace_fsi_master_gpio_cmd_rel_addr(master, rel_addr);
} else {
addr_bits = 21;
- opcode = FSI_GPIO_CMD_ABS_AR;
+ opcode = FSI_CMD_ABS_AR;
trace_fsi_master_gpio_cmd_abs_addr(master, addr);
}
@@ -327,7 +329,7 @@ static void build_dpoll_command(struct fsi_gpio_msg *cmd, uint8_t slave_id)
cmd->msg = 0;
msg_push_bits(cmd, slave_id, 2);
- msg_push_bits(cmd, FSI_GPIO_CMD_DPOLL, 3);
+ msg_push_bits(cmd, FSI_CMD_DPOLL, 3);
msg_push_crc(cmd);
}
@@ -337,7 +339,7 @@ static void build_epoll_command(struct fsi_gpio_msg *cmd, uint8_t slave_id)
cmd->msg = 0;
msg_push_bits(cmd, slave_id, 2);
- msg_push_bits(cmd, FSI_GPIO_CMD_EPOLL, 3);
+ msg_push_bits(cmd, FSI_CMD_EPOLL, 3);
msg_push_crc(cmd);
}
@@ -347,7 +349,7 @@ static void build_term_command(struct fsi_gpio_msg *cmd, uint8_t slave_id)
cmd->msg = 0;
msg_push_bits(cmd, slave_id, 2);
- msg_push_bits(cmd, FSI_GPIO_CMD_TERM, 6);
+ msg_push_bits(cmd, FSI_CMD_TERM, 6);
msg_push_crc(cmd);
}
@@ -369,14 +371,14 @@ static int read_one_response(struct fsi_master_gpio *master,
local_irq_save(flags);
/* wait for the start bit */
- for (i = 0; i < FSI_GPIO_MTOE_COUNT; i++) {
+ for (i = 0; i < FSI_MASTER_MTOE_COUNT; i++) {
msg.bits = 0;
msg.msg = 0;
serial_in(master, &msg, 1);
if (msg.msg)
break;
}
- if (i == FSI_GPIO_MTOE_COUNT) {
+ if (i == FSI_MASTER_MTOE_COUNT) {
dev_warn(master->dev,
"Master time out waiting for response\n");
local_irq_restore(flags);
@@ -392,11 +394,11 @@ static int read_one_response(struct fsi_master_gpio *master,
tag = msg.msg & 0x3;
/* If we have an ACK and we're expecting data, clock the data in too */
- if (tag == FSI_GPIO_RESP_ACK && data_size)
+ if (tag == FSI_RESP_ACK && data_size)
serial_in(master, &msg, data_size * 8);
/* read CRC */
- serial_in(master, &msg, FSI_GPIO_CRC_SIZE);
+ serial_in(master, &msg, FSI_CRC_SIZE);
local_irq_restore(flags);
@@ -439,7 +441,7 @@ static int issue_term(struct fsi_master_gpio *master, uint8_t slave)
dev_err(master->dev,
"TERM failed; lost communication with slave\n");
return -EIO;
- } else if (tag != FSI_GPIO_RESP_ACK) {
+ } else if (tag != FSI_RESP_ACK) {
dev_err(master->dev, "TERM failed; response %d\n", tag);
return -EIO;
}
@@ -475,7 +477,7 @@ static int poll_for_response(struct fsi_master_gpio *master,
trace_fsi_master_gpio_crc_rsp_error(master);
build_epoll_command(&cmd, slave);
local_irq_save(flags);
- clock_zeros(master, FSI_GPIO_EPOLL_CLOCKS);
+ clock_zeros(master, FSI_MASTER_EPOLL_CLOCKS);
serial_out(master, &cmd);
echo_delay(master);
local_irq_restore(flags);
@@ -484,7 +486,7 @@ static int poll_for_response(struct fsi_master_gpio *master,
goto fail;
switch (tag) {
- case FSI_GPIO_RESP_ACK:
+ case FSI_RESP_ACK:
if (size && data) {
uint64_t val = response.msg;
/* clear crc & mask */
@@ -497,16 +499,16 @@ static int poll_for_response(struct fsi_master_gpio *master,
}
}
break;
- case FSI_GPIO_RESP_BUSY:
+ case FSI_RESP_BUSY:
/*
* Its necessary to clock slave before issuing
* d-poll, not indicated in the hardware protocol
* spec. < 20 clocks causes slave to hang, 21 ok.
*/
- if (busy_count++ < FSI_GPIO_MAX_BUSY) {
+ if (busy_count++ < FSI_MASTER_MAX_BUSY) {
build_dpoll_command(&cmd, slave);
local_irq_save(flags);
- clock_zeros(master, FSI_GPIO_DPOLL_CLOCKS);
+ clock_zeros(master, FSI_MASTER_DPOLL_CLOCKS);
serial_out(master, &cmd);
echo_delay(master);
local_irq_restore(flags);
@@ -515,17 +517,17 @@ static int poll_for_response(struct fsi_master_gpio *master,
dev_warn(master->dev,
"ERR slave is stuck in busy state, issuing TERM\n");
local_irq_save(flags);
- clock_zeros(master, FSI_GPIO_DPOLL_CLOCKS);
+ clock_zeros(master, FSI_MASTER_DPOLL_CLOCKS);
local_irq_restore(flags);
issue_term(master, slave);
rc = -EIO;
break;
- case FSI_GPIO_RESP_ERRA:
+ case FSI_RESP_ERRA:
dev_warn(master->dev, "ERRA received: 0x%x\n", (int)response.msg);
rc = -EIO;
break;
- case FSI_GPIO_RESP_ERRC:
+ case FSI_RESP_ERRC:
dev_warn(master->dev, "ERRC received: 0x%x\n", (int)response.msg);
trace_fsi_master_gpio_crc_cmd_error(master);
rc = -EAGAIN;
--
2.17.1
^ permalink raw reply related
* [PATCH 07/14] fsi: master-gpio: Remove unused definitions
From: Benjamin Herrenschmidt @ 2018-06-26 23:25 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180626232605.13420-1-benh@kernel.crashing.org>
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
drivers/fsi/fsi-master-gpio.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c
index a00a85aa6d56..2bc85514bb0c 100644
--- a/drivers/fsi/fsi-master-gpio.c
+++ b/drivers/fsi/fsi-master-gpio.c
@@ -25,9 +25,6 @@
#define FSI_INIT_CLOCKS 5000 /* Clock out any old data */
#define FSI_GPIO_DPOLL_CLOCKS 50 /* < 21 will cause slave to hang */
#define FSI_GPIO_EPOLL_CLOCKS 50 /* Number of clocks for E_POLL retry */
-#define FSI_GPIO_STD_DELAY 10 /* Standard GPIO delay in nS */
- /* todo: adjust down as low as */
- /* possible or eliminate */
#define FSI_CRC_ERR_RETRIES 10
#define FSI_GPIO_CMD_DPOLL 0x2
@@ -45,10 +42,7 @@
#define FSI_GPIO_MAX_BUSY 200
#define FSI_GPIO_MTOE_COUNT 1000
-#define FSI_GPIO_DRAIN_BITS 20
#define FSI_GPIO_CRC_SIZE 4
-#define FSI_GPIO_MSG_ID_SIZE 2
-#define FSI_GPIO_MSG_RESPID_SIZE 2
#define LAST_ADDR_INVALID 0x1
--
2.17.1
^ permalink raw reply related
* [PATCH 06/14] fsi: master-gpio: Add more tracepoints
From: Benjamin Herrenschmidt @ 2018-06-26 23:25 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180626232605.13420-1-benh@kernel.crashing.org>
This adds a few more tracepoints that have proven useful when
debugging issues with the FSI bus.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
drivers/fsi/fsi-master-gpio.c | 16 ++++---
include/trace/events/fsi_master_gpio.h | 59 ++++++++++++++++++++++++++
2 files changed, 69 insertions(+), 6 deletions(-)
diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c
index 48e0e65b2982..a00a85aa6d56 100644
--- a/drivers/fsi/fsi-master-gpio.c
+++ b/drivers/fsi/fsi-master-gpio.c
@@ -130,10 +130,17 @@ static void set_sda_output(struct fsi_master_gpio *master, int value)
static void clock_zeros(struct fsi_master_gpio *master, int count)
{
+ trace_fsi_master_gpio_clock_zeros(master, count);
set_sda_output(master, 1);
clock_toggle(master, count);
}
+static void echo_delay(struct fsi_master_gpio *master)
+{
+ clock_zeros(master, master->t_echo_delay);
+}
+
+
static void serial_in(struct fsi_master_gpio *master, struct fsi_gpio_msg *msg,
uint8_t num_bits)
{
@@ -279,16 +286,19 @@ static void build_ar_command(struct fsi_master_gpio *master,
addr_bits = 2;
opcode_bits = 2;
opcode = FSI_GPIO_CMD_SAME_AR;
+ trace_fsi_master_gpio_cmd_same_addr(master);
} else if (check_relative_address(master, id, addr, &rel_addr)) {
/* 8 bits plus sign */
addr_bits = 9;
addr = rel_addr;
opcode = FSI_GPIO_CMD_REL_AR;
+ trace_fsi_master_gpio_cmd_rel_addr(master, rel_addr);
} else {
addr_bits = 21;
opcode = FSI_GPIO_CMD_ABS_AR;
+ trace_fsi_master_gpio_cmd_abs_addr(master, addr);
}
/*
@@ -337,12 +347,6 @@ static void build_epoll_command(struct fsi_gpio_msg *cmd, uint8_t slave_id)
msg_push_crc(cmd);
}
-static void echo_delay(struct fsi_master_gpio *master)
-{
- set_sda_output(master, 1);
- clock_toggle(master, master->t_echo_delay);
-}
-
static void build_term_command(struct fsi_gpio_msg *cmd, uint8_t slave_id)
{
cmd->bits = 0;
diff --git a/include/trace/events/fsi_master_gpio.h b/include/trace/events/fsi_master_gpio.h
index 389082132433..70ef66e63e84 100644
--- a/include/trace/events/fsi_master_gpio.h
+++ b/include/trace/events/fsi_master_gpio.h
@@ -50,6 +50,22 @@ TRACE_EVENT(fsi_master_gpio_out,
)
);
+TRACE_EVENT(fsi_master_gpio_clock_zeros,
+ TP_PROTO(const struct fsi_master_gpio *master, int clocks),
+ TP_ARGS(master, clocks),
+ TP_STRUCT__entry(
+ __field(int, master_idx)
+ __field(int, clocks)
+ ),
+ TP_fast_assign(
+ __entry->master_idx = master->master.idx;
+ __entry->clocks = clocks;
+ ),
+ TP_printk("fsi-gpio%d clock %d zeros",
+ __entry->master_idx, __entry->clocks
+ )
+);
+
TRACE_EVENT(fsi_master_gpio_break,
TP_PROTO(const struct fsi_master_gpio *master),
TP_ARGS(master),
@@ -107,6 +123,49 @@ TRACE_EVENT(fsi_master_gpio_poll_response_busy,
__entry->master_idx, __entry->busy)
);
+TRACE_EVENT(fsi_master_gpio_cmd_abs_addr,
+ TP_PROTO(const struct fsi_master_gpio *master, u32 addr),
+ TP_ARGS(master, addr),
+ TP_STRUCT__entry(
+ __field(int, master_idx)
+ __field(u32, addr)
+ ),
+ TP_fast_assign(
+ __entry->master_idx = master->master.idx;
+ __entry->addr = addr;
+ ),
+ TP_printk("fsi-gpio%d: Sending ABS_ADR %06x",
+ __entry->master_idx, __entry->addr)
+);
+
+TRACE_EVENT(fsi_master_gpio_cmd_rel_addr,
+ TP_PROTO(const struct fsi_master_gpio *master, u32 rel_addr),
+ TP_ARGS(master, rel_addr),
+ TP_STRUCT__entry(
+ __field(int, master_idx)
+ __field(u32, rel_addr)
+ ),
+ TP_fast_assign(
+ __entry->master_idx = master->master.idx;
+ __entry->rel_addr = rel_addr;
+ ),
+ TP_printk("fsi-gpio%d: Sending REL_ADR %03x",
+ __entry->master_idx, __entry->rel_addr)
+);
+
+TRACE_EVENT(fsi_master_gpio_cmd_same_addr,
+ TP_PROTO(const struct fsi_master_gpio *master),
+ TP_ARGS(master),
+ TP_STRUCT__entry(
+ __field(int, master_idx)
+ ),
+ TP_fast_assign(
+ __entry->master_idx = master->master.idx;
+ ),
+ TP_printk("fsi-gpio%d: Sending SAME_ADR",
+ __entry->master_idx)
+);
+
#endif /* _TRACE_FSI_MASTER_GPIO_H */
#include <trace/define_trace.h>
--
2.17.1
^ permalink raw reply related
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