Linux-Aspeed Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ARM:dts:aspeed Add Inspur on5263m5 BMC
From: Vijay Khemka @ 2019-02-12 22:07 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <1547708191-8721-1-git-send-email-wangzqbj@inspur.com>



?On 2/12/19, 1:58 PM, "Linux-aspeed on behalf of John Wang" <linux-aspeed-bounces+vijaykhemka=fb.com at lists.ozlabs.org on behalf of wangzqbj@inspur.com> wrote:

    Add initial version of device tree file for on5263m5 ast2500bmc
    
    Signed-off-by: John Wang <wangzqbj@inspur.com>
    ---
     arch/arm/boot/dts/aspeed-bmc-inspur-on5263m5.dts | 146 +++++++++++++++++++++++
     1 file changed, 146 insertions(+)
     create mode 100644 arch/arm/boot/dts/aspeed-bmc-inspur-on5263m5.dts
    
    diff --git a/arch/arm/boot/dts/aspeed-bmc-inspur-on5263m5.dts b/arch/arm/boot/dts/aspeed-bmc-inspur-on5263m5.dts
    new file mode 100644
    index 0000000..064ae30
    --- /dev/null
    +++ b/arch/arm/boot/dts/aspeed-bmc-inspur-on5263m5.dts
    @@ -0,0 +1,146 @@
    +// SPDX-License-Identifier: GPL-2.0
    +// Copyright (c) 2018 Inspur Corporation
    +/dts-v1/;
    +
    +#include "aspeed-g5.dtsi"
    +#include <dt-bindings/gpio/aspeed-gpio.h>
    +
    +/ {
    +	model = "ON5263M5 BMC";
    +	compatible = "inspur,on5263m5-bmc", "aspeed,ast2500";
    +
    +	chosen {
    +		stdout-path = &uart5;
    +		bootargs = "earlyprintk";
    +	};
    +
    +	memory {
    +		reg = <0x80000000 0x20000000>;
    +	};
    +
    +	reserved-memory {
    +		#address-cells = <1>;
    +		#size-cells = <1>;
    +		ranges;
    +
    +		vga_memory: framebuffer at 9f000000 {
    +			no-map;
    +			reg = <0x9f000000 0x01000000>;
    +		};
    +	};
    +	
    +	leds {
    +	    compatible = "gpio-leds";
    +	    
    +	    bmc_alive {
    +	        label = "bmc_alive";
    +		gpios = <&gpio ASPEED_GPIO(I, 1) GPIO_ACTIVE_LOW>;
    +		linux,default-trigger = "timer";
    +	    };
    +	};
    +
    +	iio-hwmon {
    +		compatible = "iio-hwmon";
    +		io-channels = <&adc 0>, <&adc 1>, <&adc 2>, <&adc 3>,
    +			<&adc 4>, <&adc 5>, <&adc 6>, <&adc 7>;
    +	};
    +
    +};
    +
    +&fmc {
    +	status = "okay";
    +	flash at 0 {
    +		status = "okay";
    +		m25p,fast-read;
    +		label = "bmc";
    +#include "openbmc-flash-layout.dtsi"
    +	};
    +};
    +
    +&spi1 {
    +	status = "okay";
    +	pinctrl-names = "default";
    +	pinctrl-0 = <&pinctrl_spi1_default>;
    +
    +	flash at 0 {
    +		status = "okay";
    +		m25p,fast-read;
    +		label = "pnor";
    +	};
    +};
    +
    +&uart5 {
    +	status = "okay";
    +};
    +
    +&mac0 {
    +	status = "okay";
    +
    +	pinctrl-names = "default";
    +	pinctrl-0 = <&pinctrl_rmii1_default>;
    +	use-ncsi;
    +};
    +
    +&mac1 {
    +	status = "okay";
    +
    +	pinctrl-names = "default";
    +	pinctrl-0 = <&pinctrl_rgmii2_default &pinctrl_mdio2_default>;
    +};
    +
    +&i2c6 {
    +	status = "okay";
    +
    +	tmp421 at 4e {
    +	        compatible = "ti,tmp421";
    +		reg = <0x4e>;
    +	};
    +
    +	tmp112 at 48 {
    +		compatible = "ti,tmp112";
    +		reg = <0x48>;
    +	};
    +	
    +	eeprom at 54 {
    +		compatible = "atmel,24c64";
    +    		reg = <0x54>; 
    +		pagesize = <32>;
    +	};
    +};
    +
    +&i2c7 {
    +	status = "okay";
    +
    +	adm1278 at 11 {
    +		  compatible = "adi,adm1278";
    +		  reg = <0x11>;
    +	};
    +};
    +
    +&gfx {
    +	status = "okay";
    +};
    +
    +&pinctrl {
    +	aspeed,external-nodes = <&gfx &lhc>;
    +};
This pinctrl definition is not required as it is already defined in aspeed-g5.dtsi.
    +
    +&pwm_tacho {
    +	status = "okay";
    +	pinctrl-names = "default";
    +	pinctrl-0 = <&pinctrl_pwm0_default &pinctrl_pwm1_default>;
    +
    +	fan at 0 {
    +   		reg = <0x00>;
    +   		aspeed,fan-tach-ch = /bits/ 8 <0x00 0x01>;  
    + 	};
    +
    + 	fan at 1 {
    +   		reg = <0x01>;
    +   		aspeed,fan-tach-ch = /bits/ 8 <0x02 0x03>;
    + 	};
    +};
    +
    +&adc {
    +	status = "okay";
    +};
    -- 
    2.7.4
    
    


^ permalink raw reply

* [PATCH] misc: aspeed-lpc-ctrl: Correct return values
From: Vijay Khemka @ 2019-02-11 20:13 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <1549862567.1175306.1655243744.11AFF30D@webmail.messagingengine.com>



?On 2/10/19, 9:22 PM, "Andrew Jeffery" <andrew@aj.id.au> wrote:

    On Fri, 25 Jan 2019, at 05:59, Vijay Khemka wrote:
    > 
    > 
    > On 1/24/19, 12:16 AM, "Greg Kroah-Hartman" <gregkh@linuxfoundation.org> wrote:
    > 
    >     On Wed, Jan 23, 2019 at 03:06:34PM -0800, Vijay Khemka wrote:
    >     > Corrected some of return values with appropriate meanings.
    >     > 
    >     > Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
    >     > ---
    >     >  drivers/misc/aspeed-lpc-ctrl.c | 15 +++++++--------
    >     >  1 file changed, 7 insertions(+), 8 deletions(-)
    >     > 
    >     > diff --git a/drivers/misc/aspeed-lpc-ctrl.c b/drivers/misc/aspeed-
    > lpc-ctrl.c
    >     > index 332210e06e98..97ae341109d5 100644
    >     > --- a/drivers/misc/aspeed-lpc-ctrl.c
    >     > +++ b/drivers/misc/aspeed-lpc-ctrl.c
    >     > @@ -68,7 +68,6 @@ static long aspeed_lpc_ctrl_ioctl(struct file 
    > *file, unsigned int cmd,
    >     >  		unsigned long param)
    >     >  {
    >     >  	struct aspeed_lpc_ctrl *lpc_ctrl = file_aspeed_lpc_ctrl(file);
    >     > -	struct device *dev = file->private_data;
    >     >  	void __user *p = (void __user *)param;
    >     >  	struct aspeed_lpc_ctrl_mapping map;
    >     >  	u32 addr;
    >     > @@ -93,8 +92,8 @@ static long aspeed_lpc_ctrl_ioctl(struct file 
    > *file, unsigned int cmd,
    >     >  
    >     >  		/* If memory-region is not described in device tree */
    >     >  		if (!lpc_ctrl->mem_size) {
    >     > -			dev_err(dev, "Didn't find reserved memory\n");
    >     > -			return -EINVAL;
    >     > +			pr_err("aspeed_lpc_ctrl: ioctl: Didn't find reserved memory\n");
    >     
    >     Why did you change from dev_err() to pr_err()?  You just lost a lot of
    >     information that the user previously was getting from dev_err() :(
    > 
    > I did this as per review comment from Andrew Jeffery, as we don't want 
    > to put this error for driver. It has to be handled by userspace. But I 
    > am still reporting some error here.
    
    Sorry? What I was trying to suggest was removing the logging altogether and
    just returning the error code. Not simply changing how the logging is done.
No issue, I can remove logging, will just return error code. Will send new patch with update.
    
    Andrew
    


^ permalink raw reply

* [PATCH v2] i2c: aspeed: Add multi-master use case support
From: Jae Hyun Yoo @ 2019-02-11 18:54 UTC (permalink / raw)
  To: linux-aspeed

In multi-master environment, this driver's master cannot know
exactly when a peer master sends data to this driver's slave so
cases can be happened that this master tries sending data through
the master_xfer function but slave data from a peer master is still
being processed or slave xfer is started by a peer immediately
after it queues a master command. To support multi-master use cases
properly, this H/W provides arbitration in physical level and it
provides priority based command handling too to avoid conflicts in
multi-master environment, means that if a master and a slave events
happen at the same time, H/W will handle a higher priority event
first and a pending event will be handled when bus comes back to
the idle state.

To support this H/W feature properly, this patch adds the 'pending'
state of master and its handling code so that the pending master
xfer can be continued after slave operation properly.

Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
---
Changes since v9:
* Removed cppcheck warnings.

 drivers/i2c/busses/i2c-aspeed.c | 119 +++++++++++++++++++++++++-------
 1 file changed, 93 insertions(+), 26 deletions(-)

diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
index 8dc9161ced38..882c4ab1d18a 100644
--- a/drivers/i2c/busses/i2c-aspeed.c
+++ b/drivers/i2c/busses/i2c-aspeed.c
@@ -117,6 +117,7 @@
 
 enum aspeed_i2c_master_state {
 	ASPEED_I2C_MASTER_INACTIVE,
+	ASPEED_I2C_MASTER_PENDING,
 	ASPEED_I2C_MASTER_START,
 	ASPEED_I2C_MASTER_TX_FIRST,
 	ASPEED_I2C_MASTER_TX,
@@ -126,12 +127,13 @@ enum aspeed_i2c_master_state {
 };
 
 enum aspeed_i2c_slave_state {
-	ASPEED_I2C_SLAVE_STOP,
+	ASPEED_I2C_SLAVE_INACTIVE,
 	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 {
@@ -156,6 +158,8 @@ struct aspeed_i2c_bus {
 	int				cmd_err;
 	/* Protected only by i2c_lock_bus */
 	int				master_xfer_result;
+	/* Multi-master */
+	bool				multi_master;
 #if IS_ENABLED(CONFIG_I2C_SLAVE)
 	struct i2c_client		*slave;
 	enum aspeed_i2c_slave_state	slave_state;
@@ -251,7 +255,7 @@ static u32 aspeed_i2c_slave_irq(struct aspeed_i2c_bus *bus, u32 irq_status)
 	}
 
 	/* Slave is not currently active, irq was for someone else. */
-	if (bus->slave_state == ASPEED_I2C_SLAVE_STOP)
+	if (bus->slave_state == ASPEED_I2C_SLAVE_INACTIVE)
 		return irq_handled;
 
 	dev_dbg(bus->dev, "slave irq status 0x%08x, cmd 0x%08x\n",
@@ -277,16 +281,15 @@ static u32 aspeed_i2c_slave_irq(struct aspeed_i2c_bus *bus, u32 irq_status)
 		irq_handled |= ASPEED_I2CD_INTR_NORMAL_STOP;
 		bus->slave_state = ASPEED_I2C_SLAVE_STOP;
 	}
-	if (irq_status & ASPEED_I2CD_INTR_TX_NAK) {
+	if (irq_status & ASPEED_I2CD_INTR_TX_NAK &&
+	    bus->slave_state == ASPEED_I2C_SLAVE_READ_PROCESSED) {
 		irq_handled |= ASPEED_I2CD_INTR_TX_NAK;
 		bus->slave_state = ASPEED_I2C_SLAVE_STOP;
 	}
-	if (irq_status & ASPEED_I2CD_INTR_TX_ACK)
-		irq_handled |= ASPEED_I2CD_INTR_TX_ACK;
 
 	switch (bus->slave_state) {
 	case ASPEED_I2C_SLAVE_READ_REQUESTED:
-		if (irq_status & ASPEED_I2CD_INTR_TX_ACK)
+		if (unlikely(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);
@@ -294,9 +297,12 @@ static u32 aspeed_i2c_slave_irq(struct aspeed_i2c_bus *bus, u32 irq_status)
 		writel(ASPEED_I2CD_S_TX_CMD, bus->base + ASPEED_I2C_CMD_REG);
 		break;
 	case ASPEED_I2C_SLAVE_READ_PROCESSED:
-		if (!(irq_status & ASPEED_I2CD_INTR_TX_ACK))
+		if (unlikely(!(irq_status & ASPEED_I2CD_INTR_TX_ACK))) {
 			dev_err(bus->dev,
 				"Expected ACK after processed read.\n");
+			break;
+		}
+		irq_handled |= ASPEED_I2CD_INTR_TX_ACK;
 		i2c_slave_event(slave, I2C_SLAVE_READ_PROCESSED, &value);
 		writel(value, bus->base + ASPEED_I2C_BYTE_BUF_REG);
 		writel(ASPEED_I2CD_S_TX_CMD, bus->base + ASPEED_I2C_CMD_REG);
@@ -310,10 +316,15 @@ static u32 aspeed_i2c_slave_irq(struct aspeed_i2c_bus *bus, u32 irq_status)
 		break;
 	case ASPEED_I2C_SLAVE_STOP:
 		i2c_slave_event(slave, I2C_SLAVE_STOP, &value);
+		bus->slave_state = ASPEED_I2C_SLAVE_INACTIVE;
+		break;
+	case ASPEED_I2C_SLAVE_START:
+		/* Slave was just started. Waiting for the next event. */;
 		break;
 	default:
-		dev_err(bus->dev, "unhandled slave_state: %d\n",
+		dev_err(bus->dev, "unknown slave_state: %d\n",
 			bus->slave_state);
+		bus->slave_state = ASPEED_I2C_SLAVE_INACTIVE;
 		break;
 	}
 
@@ -329,6 +340,17 @@ static void aspeed_i2c_do_start(struct aspeed_i2c_bus *bus)
 	u8 slave_addr = i2c_8bit_addr_from_msg(msg);
 
 	bus->master_state = ASPEED_I2C_MASTER_START;
+
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+	/*
+	 * If it's requested in the middle of a slave session, set the master
+	 * state to 'pending' then H/W will continue handling this master
+	 * command when the bus comes back to the idle state.
+	 */
+	if (bus->slave_state != ASPEED_I2C_SLAVE_INACTIVE)
+		bus->master_state = ASPEED_I2C_MASTER_PENDING;
+#endif /* CONFIG_I2C_SLAVE */
+
 	bus->buf_index = 0;
 
 	if (msg->flags & I2C_M_RD) {
@@ -384,10 +406,6 @@ static u32 aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus, u32 irq_status)
 		bus->master_state = ASPEED_I2C_MASTER_INACTIVE;
 		irq_handled |= 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;
 	}
 
 	/*
@@ -399,11 +417,32 @@ static u32 aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus, u32 irq_status)
 	if (ret) {
 		dev_dbg(bus->dev, "received error interrupt: 0x%08x\n",
 			irq_status);
-		bus->cmd_err = ret;
-		bus->master_state = ASPEED_I2C_MASTER_INACTIVE;
 		irq_handled |= (irq_status & ASPEED_I2CD_INTR_MASTER_ERRORS);
-		goto out_complete;
+		if (bus->master_state != ASPEED_I2C_MASTER_INACTIVE) {
+			bus->cmd_err = ret;
+			bus->master_state = ASPEED_I2C_MASTER_INACTIVE;
+			goto out_complete;
+		}
+	}
+
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+	/*
+	 * A pending master command will be started by H/W when the bus comes
+	 * back to idle state after completing a slave operation so change the
+	 * master state from 'pending' to 'start' at here if slave is inactive.
+	 */
+	if (bus->master_state == ASPEED_I2C_MASTER_PENDING) {
+		if (bus->slave_state != ASPEED_I2C_SLAVE_INACTIVE)
+			goto out_no_complete;
+
+		bus->master_state = ASPEED_I2C_MASTER_START;
 	}
+#endif /* CONFIG_I2C_SLAVE */
+
+	/* Master is not currently active, irq was for someone else. */
+	if (bus->master_state == ASPEED_I2C_MASTER_INACTIVE ||
+	    bus->master_state == ASPEED_I2C_MASTER_PENDING)
+		goto out_no_complete;
 
 	/* We are in an invalid state; reset bus to a known state. */
 	if (!bus->msgs) {
@@ -423,6 +462,20 @@ static u32 aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus, u32 irq_status)
 	 * then update the state and handle the new state below.
 	 */
 	if (bus->master_state == ASPEED_I2C_MASTER_START) {
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+		/*
+		 * If a peer master starts a xfer immediately after it queues a
+		 * master command, change its state to 'pending' then H/W will
+		 * continue the queued master xfer just after completing the
+		 * slave mode session.
+		 */
+		if (unlikely(irq_status & ASPEED_I2CD_INTR_SLAVE_MATCH)) {
+			bus->master_state = ASPEED_I2C_MASTER_PENDING;
+			dev_dbg(bus->dev,
+				"master goes pending due to a slave start\n");
+			goto out_no_complete;
+		}
+#endif /* CONFIG_I2C_SLAVE */
 		if (unlikely(!(irq_status & ASPEED_I2CD_INTR_TX_ACK))) {
 			if (unlikely(!(irq_status & ASPEED_I2CD_INTR_TX_NAK))) {
 				bus->cmd_err = -ENXIO;
@@ -566,7 +619,8 @@ static irqreturn_t aspeed_i2c_bus_irq(int irq, void *dev_id)
 	 * interrupt bits. Each case needs to be handled using corresponding
 	 * handlers depending on the current state.
 	 */
-	if (bus->master_state != ASPEED_I2C_MASTER_INACTIVE) {
+	if (bus->master_state != ASPEED_I2C_MASTER_INACTIVE &&
+	    bus->master_state != ASPEED_I2C_MASTER_PENDING) {
 		irq_handled = aspeed_i2c_master_irq(bus, irq_remaining);
 		irq_remaining &= ~irq_handled;
 		if (irq_remaining)
@@ -601,15 +655,16 @@ 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) {
+	/* If bus is busy in a single master environment, attempt recovery. */
+	if (!bus->multi_master &&
+	    (readl(bus->base + ASPEED_I2C_CMD_REG) &
+	     ASPEED_I2CD_BUS_BUSY_STS)) {
+		int ret;
+
 		spin_unlock_irqrestore(&bus->lock, flags);
 		ret = aspeed_i2c_recover_bus(bus);
 		if (ret)
@@ -629,10 +684,20 @@ static int aspeed_i2c_master_xfer(struct i2c_adapter *adap,
 	time_left = wait_for_completion_timeout(&bus->cmd_complete,
 						bus->adap.timeout);
 
-	if (time_left == 0)
+	if (time_left == 0) {
+		/*
+		 * If timed out and bus is still busy in a multi master
+		 * environment, attempt recovery at here.
+		 */
+		if (bus->multi_master &&
+		    (readl(bus->base + ASPEED_I2C_CMD_REG) &
+		     ASPEED_I2CD_BUS_BUSY_STS))
+			aspeed_i2c_recover_bus(bus);
+
 		return -ETIMEDOUT;
-	else
-		return bus->master_xfer_result;
+	}
+
+	return bus->master_xfer_result;
 }
 
 static u32 aspeed_i2c_functionality(struct i2c_adapter *adap)
@@ -672,7 +737,7 @@ static int aspeed_i2c_reg_slave(struct i2c_client *client)
 	__aspeed_i2c_reg_slave(bus, client->addr);
 
 	bus->slave = client;
-	bus->slave_state = ASPEED_I2C_SLAVE_STOP;
+	bus->slave_state = ASPEED_I2C_SLAVE_INACTIVE;
 	spin_unlock_irqrestore(&bus->lock, flags);
 
 	return 0;
@@ -827,7 +892,9 @@ static int aspeed_i2c_init(struct aspeed_i2c_bus *bus,
 	if (ret < 0)
 		return ret;
 
-	if (!of_property_read_bool(pdev->dev.of_node, "multi-master"))
+	if (of_property_read_bool(pdev->dev.of_node, "multi-master"))
+		bus->multi_master = true;
+	else
 		fun_ctrl_reg |= ASPEED_I2CD_MULTI_MASTER_DIS;
 
 	/* Enable Master Mode */
-- 
2.20.1


^ permalink raw reply related

* [PATCH] i2c: aspeed: Add multi-master use case support
From: Jae Hyun Yoo @ 2019-02-11 18:29 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20190211181658.u55ggc2xi4nsxxvy@katana>

On 2/11/2019 10:16 AM, Wolfram Sang wrote:
> 
>> Can you please apply above changes on top of this patch? Or I can submit
>> v2 with it.
> 
> Please send v2.
> 

Okay, I'll send v2.

Thanks,
Jae

^ permalink raw reply

* [PATCH] i2c: aspeed: Add multi-master use case support
From: Wolfram Sang @ 2019-02-11 18:16 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <d26a95ac-41ab-91eb-bbae-e4143235e531@linux.intel.com>


> Can you please apply above changes on top of this patch? Or I can submit
> v2 with it.

Please send v2.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.ozlabs.org/pipermail/linux-aspeed/attachments/20190211/bca941b1/attachment.sig>

^ permalink raw reply

* [PATCH] i2c: aspeed: Add multi-master use case support
From: Jae Hyun Yoo @ 2019-02-11 18:09 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20190208214849.GH12451@kunai>

> Hmmm, the #ifdeffery quite increases with this patch. Maybe it is easier
> to select I2C_SLAVE in Kconfig? I'll let you decide.
> 
> My code checkers found this:
> 
>      CPPCHECK
> drivers/i2c/busses/i2c-aspeed.c:694:10: warning: Variable 'ret' is assigned a value that is never used. [unreadVariable]
> 

Hi Wolfram,

Yes, cppcheck pointed it out correctly. The assignment on the variable
'ret' is not needed at there. Actually, the code should not be wrapped
using I2C_SLAVE because a bus can also be in multi-master environment
even when slave is not enabled. So the warning can be removed by below
fix.

---
diff --git a/drivers/i2c/busses/i2c-aspeed.c 
b/drivers/i2c/busses/i2c-aspeed.c
index 9253ebccb008..882c4ab1d18a 100644
--- a/drivers/i2c/busses/i2c-aspeed.c
+++ b/drivers/i2c/busses/i2c-aspeed.c
@@ -655,7 +655,6 @@ 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;

         spin_lock_irqsave(&bus->lock, flags);
         bus->cmd_err = 0;
@@ -664,6 +663,8 @@ static int aspeed_i2c_master_xfer(struct i2c_adapter 
*adap,
         if (!bus->multi_master &&
             (readl(bus->base + ASPEED_I2C_CMD_REG) &
              ASPEED_I2CD_BUS_BUSY_STS)) {
+               int ret;
+
                 spin_unlock_irqrestore(&bus->lock, flags);
                 ret = aspeed_i2c_recover_bus(bus);
                 if (ret)
@@ -691,7 +692,7 @@ static int aspeed_i2c_master_xfer(struct i2c_adapter 
*adap,
                 if (bus->multi_master &&
                     (readl(bus->base + ASPEED_I2C_CMD_REG) &
                      ASPEED_I2CD_BUS_BUSY_STS))
-                       ret = aspeed_i2c_recover_bus(bus);
+                       aspeed_i2c_recover_bus(bus);

                 return -ETIMEDOUT;
---

The reason of adding changes at line 658 and 667 is to remove below
cppcheck warning when the change at 694 is applied.

[drivers/i2c/busses/i2c-aspeed.c:670]: (style) The scope of the variable 
'ret' can be reduced.

Can you please apply above changes on top of this patch? Or I can submit
v2 with it.

Thanks,
Jae

^ permalink raw reply related

* [PATCH 4.19 089/313] ARM: dts: aspeed: add missing memory unit-address
From: Greg Kroah-Hartman @ 2019-02-11 14:16 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20190211141852.749630980@linuxfoundation.org>

4.19-stable review patch.  If anyone has any objections, please let me know.

------------------

[ Upstream commit 8ef86955fe59f7912a40d57ae4c6d511f0187b4d ]

The base aspeed-g5.dtsi already defines a '/memory at 80000000' node, so
'/memory' in the board files create a duplicate node. We're probably
getting lucky that the bootloader fixes up the memory node that the
kernel ends up using. Add the unit-address so it's merged with the base
node.

Found with DT json-schema checks.

Cc: Joel Stanley <joel@jms.id.au>
Cc: Andrew Jeffery <andrew@aj.id.au>
Cc: devicetree at vger.kernel.org
Cc: linux-arm-kernel at lists.infradead.org
Cc: linux-aspeed at lists.ozlabs.org
Signed-off-by: Rob Herring <robh@kernel.org>
Signed-off-by: Olof Johansson <olof@lixom.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 arch/arm/boot/dts/aspeed-bmc-arm-centriq2400-rep.dts | 2 +-
 arch/arm/boot/dts/aspeed-bmc-intel-s2600wf.dts       | 2 +-
 arch/arm/boot/dts/aspeed-bmc-opp-lanyang.dts         | 3 +--
 arch/arm/boot/dts/aspeed-bmc-portwell-neptune.dts    | 2 +-
 4 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/arch/arm/boot/dts/aspeed-bmc-arm-centriq2400-rep.dts b/arch/arm/boot/dts/aspeed-bmc-arm-centriq2400-rep.dts
index df1227613d48..c2ece0b91885 100644
--- a/arch/arm/boot/dts/aspeed-bmc-arm-centriq2400-rep.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-arm-centriq2400-rep.dts
@@ -13,7 +13,7 @@
 		bootargs = "console=ttyS4,115200 earlyprintk";
 	};
 
-	memory {
+	memory at 80000000 {
 		reg = <0x80000000 0x40000000>;
 	};
 
diff --git a/arch/arm/boot/dts/aspeed-bmc-intel-s2600wf.dts b/arch/arm/boot/dts/aspeed-bmc-intel-s2600wf.dts
index 7a291de02543..22dade6393d0 100644
--- a/arch/arm/boot/dts/aspeed-bmc-intel-s2600wf.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-intel-s2600wf.dts
@@ -13,7 +13,7 @@
 		bootargs = "earlyprintk";
 	};
 
-	memory {
+	memory at 80000000 {
 		reg = <0x80000000 0x20000000>;
 	};
 
diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-lanyang.dts b/arch/arm/boot/dts/aspeed-bmc-opp-lanyang.dts
index d598b6391362..024e52a6cd0f 100644
--- a/arch/arm/boot/dts/aspeed-bmc-opp-lanyang.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-lanyang.dts
@@ -14,7 +14,7 @@
 		bootargs = "console=ttyS4,115200 earlyprintk";
 	};
 
-	memory {
+	memory at 80000000 {
 		reg = <0x80000000 0x40000000>;
 	};
 
@@ -322,4 +322,3 @@
 &adc {
 	status = "okay";
 };
-
diff --git a/arch/arm/boot/dts/aspeed-bmc-portwell-neptune.dts b/arch/arm/boot/dts/aspeed-bmc-portwell-neptune.dts
index 43ed13963d35..33d704541de6 100644
--- a/arch/arm/boot/dts/aspeed-bmc-portwell-neptune.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-portwell-neptune.dts
@@ -17,7 +17,7 @@
 		bootargs = "console=ttyS4,115200 earlyprintk";
 	};
 
-	memory {
+	memory at 80000000 {
 		reg = <0x80000000 0x20000000>;
 	};
 
-- 
2.19.1




^ permalink raw reply related

* [PATCH 4.20 102/352] ARM: dts: aspeed: add missing memory unit-address
From: Greg Kroah-Hartman @ 2019-02-11 14:15 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20190211141846.543045703@linuxfoundation.org>

4.20-stable review patch.  If anyone has any objections, please let me know.

------------------

[ Upstream commit 8ef86955fe59f7912a40d57ae4c6d511f0187b4d ]

The base aspeed-g5.dtsi already defines a '/memory at 80000000' node, so
'/memory' in the board files create a duplicate node. We're probably
getting lucky that the bootloader fixes up the memory node that the
kernel ends up using. Add the unit-address so it's merged with the base
node.

Found with DT json-schema checks.

Cc: Joel Stanley <joel@jms.id.au>
Cc: Andrew Jeffery <andrew@aj.id.au>
Cc: devicetree at vger.kernel.org
Cc: linux-arm-kernel at lists.infradead.org
Cc: linux-aspeed at lists.ozlabs.org
Signed-off-by: Rob Herring <robh@kernel.org>
Signed-off-by: Olof Johansson <olof@lixom.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 arch/arm/boot/dts/aspeed-bmc-arm-centriq2400-rep.dts | 2 +-
 arch/arm/boot/dts/aspeed-bmc-intel-s2600wf.dts       | 2 +-
 arch/arm/boot/dts/aspeed-bmc-opp-lanyang.dts         | 3 +--
 arch/arm/boot/dts/aspeed-bmc-portwell-neptune.dts    | 2 +-
 4 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/arch/arm/boot/dts/aspeed-bmc-arm-centriq2400-rep.dts b/arch/arm/boot/dts/aspeed-bmc-arm-centriq2400-rep.dts
index df1227613d48..c2ece0b91885 100644
--- a/arch/arm/boot/dts/aspeed-bmc-arm-centriq2400-rep.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-arm-centriq2400-rep.dts
@@ -13,7 +13,7 @@
 		bootargs = "console=ttyS4,115200 earlyprintk";
 	};
 
-	memory {
+	memory at 80000000 {
 		reg = <0x80000000 0x40000000>;
 	};
 
diff --git a/arch/arm/boot/dts/aspeed-bmc-intel-s2600wf.dts b/arch/arm/boot/dts/aspeed-bmc-intel-s2600wf.dts
index 7a291de02543..22dade6393d0 100644
--- a/arch/arm/boot/dts/aspeed-bmc-intel-s2600wf.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-intel-s2600wf.dts
@@ -13,7 +13,7 @@
 		bootargs = "earlyprintk";
 	};
 
-	memory {
+	memory at 80000000 {
 		reg = <0x80000000 0x20000000>;
 	};
 
diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-lanyang.dts b/arch/arm/boot/dts/aspeed-bmc-opp-lanyang.dts
index d598b6391362..024e52a6cd0f 100644
--- a/arch/arm/boot/dts/aspeed-bmc-opp-lanyang.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-lanyang.dts
@@ -14,7 +14,7 @@
 		bootargs = "console=ttyS4,115200 earlyprintk";
 	};
 
-	memory {
+	memory at 80000000 {
 		reg = <0x80000000 0x40000000>;
 	};
 
@@ -322,4 +322,3 @@
 &adc {
 	status = "okay";
 };
-
diff --git a/arch/arm/boot/dts/aspeed-bmc-portwell-neptune.dts b/arch/arm/boot/dts/aspeed-bmc-portwell-neptune.dts
index 43ed13963d35..33d704541de6 100644
--- a/arch/arm/boot/dts/aspeed-bmc-portwell-neptune.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-portwell-neptune.dts
@@ -17,7 +17,7 @@
 		bootargs = "console=ttyS4,115200 earlyprintk";
 	};
 
-	memory {
+	memory at 80000000 {
 		reg = <0x80000000 0x20000000>;
 	};
 
-- 
2.19.1




^ permalink raw reply related

* [PATCH] misc: aspeed-lpc-ctrl: Correct return values
From: Andrew Jeffery @ 2019-02-11  5:22 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <13C13AE8-63A4-4002-9E0D-70276C529448@fb.com>

On Fri, 25 Jan 2019, at 05:59, Vijay Khemka wrote:
> 
> 
> ?On 1/24/19, 12:16 AM, "Greg Kroah-Hartman" <gregkh@linuxfoundation.org> wrote:
> 
>     On Wed, Jan 23, 2019 at 03:06:34PM -0800, Vijay Khemka wrote:
>     > Corrected some of return values with appropriate meanings.
>     > 
>     > Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
>     > ---
>     >  drivers/misc/aspeed-lpc-ctrl.c | 15 +++++++--------
>     >  1 file changed, 7 insertions(+), 8 deletions(-)
>     > 
>     > diff --git a/drivers/misc/aspeed-lpc-ctrl.c b/drivers/misc/aspeed-
> lpc-ctrl.c
>     > index 332210e06e98..97ae341109d5 100644
>     > --- a/drivers/misc/aspeed-lpc-ctrl.c
>     > +++ b/drivers/misc/aspeed-lpc-ctrl.c
>     > @@ -68,7 +68,6 @@ static long aspeed_lpc_ctrl_ioctl(struct file 
> *file, unsigned int cmd,
>     >  		unsigned long param)
>     >  {
>     >  	struct aspeed_lpc_ctrl *lpc_ctrl = file_aspeed_lpc_ctrl(file);
>     > -	struct device *dev = file->private_data;
>     >  	void __user *p = (void __user *)param;
>     >  	struct aspeed_lpc_ctrl_mapping map;
>     >  	u32 addr;
>     > @@ -93,8 +92,8 @@ static long aspeed_lpc_ctrl_ioctl(struct file 
> *file, unsigned int cmd,
>     >  
>     >  		/* If memory-region is not described in device tree */
>     >  		if (!lpc_ctrl->mem_size) {
>     > -			dev_err(dev, "Didn't find reserved memory\n");
>     > -			return -EINVAL;
>     > +			pr_err("aspeed_lpc_ctrl: ioctl: Didn't find reserved memory\n");
>     
>     Why did you change from dev_err() to pr_err()?  You just lost a lot of
>     information that the user previously was getting from dev_err() :(
> 
> I did this as per review comment from Andrew Jeffery, as we don't want 
> to put this error for driver. It has to be handled by userspace. But I 
> am still reporting some error here.

Sorry? What I was trying to suggest was removing the logging altogether and
just returning the error code. Not simply changing how the logging is done.

Andrew

^ permalink raw reply

* [PATCH] i2c: aspeed: Add multi-master use case support
From: Wolfram Sang @ 2019-02-08 21:48 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20190116193958.6049-1-jae.hyun.yoo@linux.intel.com>

On Wed, Jan 16, 2019 at 11:39:58AM -0800, Jae Hyun Yoo wrote:
> In multi-master environment, this driver's master cannot know
> exactly when a peer master sends data to this driver's slave so
> cases can be happened that this master tries sending data through
> the master_xfer function but slave data from a peer master is still
> being processed or slave xfer is started by a peer immediately
> after it queues a master command. To support multi-master use cases
> properly, this H/W provides arbitration in physical level and it
> provides priority based command handling too to avoid conflicts in
> multi-master environment, means that if a master and a slave events
> happen at the same time, H/W will handle a higher priority event
> first and a pending event will be handled when bus comes back to
> the idle state.
> 
> To support this H/W feature properly, this patch adds the 'pending'
> state of master and its handling code so that the pending master
> xfer can be continued after slave operation properly.
> 
> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>

Hmmm, the #ifdeffery quite increases with this patch. Maybe it is easier
to select I2C_SLAVE in Kconfig? I'll let you decide.

My code checkers found this:

    CPPCHECK
drivers/i2c/busses/i2c-aspeed.c:694:10: warning: Variable 'ret' is assigned a value that is never used. [unreadVariable]

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.ozlabs.org/pipermail/linux-aspeed/attachments/20190208/a4140498/attachment.sig>

^ permalink raw reply

* [PATCH] i2c: aspeed: Add multi-master use case support
From: Brendan Higgins @ 2019-02-07 19:48 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20190116193958.6049-1-jae.hyun.yoo@linux.intel.com>

On Wed, Jan 16, 2019 at 11:40 AM Jae Hyun Yoo
<jae.hyun.yoo@linux.intel.com> wrote:
>
> In multi-master environment, this driver's master cannot know
> exactly when a peer master sends data to this driver's slave so
> cases can be happened that this master tries sending data through
> the master_xfer function but slave data from a peer master is still
> being processed or slave xfer is started by a peer immediately
> after it queues a master command. To support multi-master use cases
> properly, this H/W provides arbitration in physical level and it
> provides priority based command handling too to avoid conflicts in
> multi-master environment, means that if a master and a slave events
> happen at the same time, H/W will handle a higher priority event
> first and a pending event will be handled when bus comes back to
> the idle state.
>
> To support this H/W feature properly, this patch adds the 'pending'
> state of master and its handling code so that the pending master
> xfer can be continued after slave operation properly.
>
> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
> ---
>  drivers/i2c/busses/i2c-aspeed.c | 118 +++++++++++++++++++++++++-------
>  1 file changed, 92 insertions(+), 26 deletions(-)
>

Reviewed-by: Brendan Higgins <brendanhiggins@google.com>

^ permalink raw reply

* [PATCH v3] dt-bindings: aspeed-lpc: Make parameter optional
From: Lee Jones @ 2019-02-07  7:47 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20190129230538.1044378-1-vijaykhemka@fb.com>

On Tue, 29 Jan 2019, Vijay Khemka wrote:

> Memory-region and flash phandle is not a required parameter, it is
> optional to describe in device tree and needed only use basis.
> 
> Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
> ---
>  Documentation/devicetree/bindings/mfd/aspeed-lpc.txt | 2 ++
>  1 file changed, 2 insertions(+)

Applied, thanks.

-- 
Lee Jones [???]
Linaro Services Technical Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* [PATCH] ARM: dts: aspeed: cmm: enable iio-hwmon-adc
From: Tao Ren @ 2019-02-05 22:46 UTC (permalink / raw)
  To: linux-aspeed

Bind aspeed ADC channels 0-7 to "iio-hwmon" driver so the data of these
voltage sensing channels can be accessed by "lm_sensors". Channels 8-15
are not used on CMM BMC.

Signed-off-by: Tao Ren <taoren@fb.com>
---
 arch/arm/boot/dts/aspeed-bmc-facebook-cmm.dts | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/aspeed-bmc-facebook-cmm.dts b/arch/arm/boot/dts/aspeed-bmc-facebook-cmm.dts
index 9f194b5eeba4..43aba4071a5c 100644
--- a/arch/arm/boot/dts/aspeed-bmc-facebook-cmm.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-facebook-cmm.dts
@@ -56,6 +56,12 @@
 	memory at 80000000 {
 		reg = <0x80000000 0x20000000>;
 	};
+
+	ast-adc-hwmon {
+		compatible = "iio-hwmon";
+		io-channels = <&adc 0>, <&adc 1>, <&adc 2>, <&adc 3>,
+			      <&adc 4>, <&adc 5>, <&adc 6>, <&adc 7>;
+	};
 };
 
 &pinctrl {
-- 
2.17.1


^ permalink raw reply related

* [PATCH] i2c: aspeed: Add multi-master use case support
From: Jae Hyun Yoo @ 2019-01-31 18:20 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20190116193958.6049-1-jae.hyun.yoo@linux.intel.com>

Hi Brendan,

Can you please review this patch?

Thanks,
Jae

On 1/16/2019 11:39 AM, Jae Hyun Yoo wrote:
> In multi-master environment, this driver's master cannot know
> exactly when a peer master sends data to this driver's slave so
> cases can be happened that this master tries sending data through
> the master_xfer function but slave data from a peer master is still
> being processed or slave xfer is started by a peer immediately
> after it queues a master command. To support multi-master use cases
> properly, this H/W provides arbitration in physical level and it
> provides priority based command handling too to avoid conflicts in
> multi-master environment, means that if a master and a slave events
> happen at the same time, H/W will handle a higher priority event
> first and a pending event will be handled when bus comes back to
> the idle state.
> 
> To support this H/W feature properly, this patch adds the 'pending'
> state of master and its handling code so that the pending master
> xfer can be continued after slave operation properly.
> 
> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
> ---
>   drivers/i2c/busses/i2c-aspeed.c | 118 +++++++++++++++++++++++++-------
>   1 file changed, 92 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
> index 8dc9161ced38..9253ebccb008 100644
> --- a/drivers/i2c/busses/i2c-aspeed.c
> +++ b/drivers/i2c/busses/i2c-aspeed.c
> @@ -117,6 +117,7 @@
>   
>   enum aspeed_i2c_master_state {
>   	ASPEED_I2C_MASTER_INACTIVE,
> +	ASPEED_I2C_MASTER_PENDING,
>   	ASPEED_I2C_MASTER_START,
>   	ASPEED_I2C_MASTER_TX_FIRST,
>   	ASPEED_I2C_MASTER_TX,
> @@ -126,12 +127,13 @@ enum aspeed_i2c_master_state {
>   };
>   
>   enum aspeed_i2c_slave_state {
> -	ASPEED_I2C_SLAVE_STOP,
> +	ASPEED_I2C_SLAVE_INACTIVE,
>   	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 {
> @@ -156,6 +158,8 @@ struct aspeed_i2c_bus {
>   	int				cmd_err;
>   	/* Protected only by i2c_lock_bus */
>   	int				master_xfer_result;
> +	/* Multi-master */
> +	bool				multi_master;
>   #if IS_ENABLED(CONFIG_I2C_SLAVE)
>   	struct i2c_client		*slave;
>   	enum aspeed_i2c_slave_state	slave_state;
> @@ -251,7 +255,7 @@ static u32 aspeed_i2c_slave_irq(struct aspeed_i2c_bus *bus, u32 irq_status)
>   	}
>   
>   	/* Slave is not currently active, irq was for someone else. */
> -	if (bus->slave_state == ASPEED_I2C_SLAVE_STOP)
> +	if (bus->slave_state == ASPEED_I2C_SLAVE_INACTIVE)
>   		return irq_handled;
>   
>   	dev_dbg(bus->dev, "slave irq status 0x%08x, cmd 0x%08x\n",
> @@ -277,16 +281,15 @@ static u32 aspeed_i2c_slave_irq(struct aspeed_i2c_bus *bus, u32 irq_status)
>   		irq_handled |= ASPEED_I2CD_INTR_NORMAL_STOP;
>   		bus->slave_state = ASPEED_I2C_SLAVE_STOP;
>   	}
> -	if (irq_status & ASPEED_I2CD_INTR_TX_NAK) {
> +	if (irq_status & ASPEED_I2CD_INTR_TX_NAK &&
> +	    bus->slave_state == ASPEED_I2C_SLAVE_READ_PROCESSED) {
>   		irq_handled |= ASPEED_I2CD_INTR_TX_NAK;
>   		bus->slave_state = ASPEED_I2C_SLAVE_STOP;
>   	}
> -	if (irq_status & ASPEED_I2CD_INTR_TX_ACK)
> -		irq_handled |= ASPEED_I2CD_INTR_TX_ACK;
>   
>   	switch (bus->slave_state) {
>   	case ASPEED_I2C_SLAVE_READ_REQUESTED:
> -		if (irq_status & ASPEED_I2CD_INTR_TX_ACK)
> +		if (unlikely(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);
> @@ -294,9 +297,12 @@ static u32 aspeed_i2c_slave_irq(struct aspeed_i2c_bus *bus, u32 irq_status)
>   		writel(ASPEED_I2CD_S_TX_CMD, bus->base + ASPEED_I2C_CMD_REG);
>   		break;
>   	case ASPEED_I2C_SLAVE_READ_PROCESSED:
> -		if (!(irq_status & ASPEED_I2CD_INTR_TX_ACK))
> +		if (unlikely(!(irq_status & ASPEED_I2CD_INTR_TX_ACK))) {
>   			dev_err(bus->dev,
>   				"Expected ACK after processed read.\n");
> +			break;
> +		}
> +		irq_handled |= ASPEED_I2CD_INTR_TX_ACK;
>   		i2c_slave_event(slave, I2C_SLAVE_READ_PROCESSED, &value);
>   		writel(value, bus->base + ASPEED_I2C_BYTE_BUF_REG);
>   		writel(ASPEED_I2CD_S_TX_CMD, bus->base + ASPEED_I2C_CMD_REG);
> @@ -310,10 +316,15 @@ static u32 aspeed_i2c_slave_irq(struct aspeed_i2c_bus *bus, u32 irq_status)
>   		break;
>   	case ASPEED_I2C_SLAVE_STOP:
>   		i2c_slave_event(slave, I2C_SLAVE_STOP, &value);
> +		bus->slave_state = ASPEED_I2C_SLAVE_INACTIVE;
> +		break;
> +	case ASPEED_I2C_SLAVE_START:
> +		/* Slave was just started. Waiting for the next event. */;
>   		break;
>   	default:
> -		dev_err(bus->dev, "unhandled slave_state: %d\n",
> +		dev_err(bus->dev, "unknown slave_state: %d\n",
>   			bus->slave_state);
> +		bus->slave_state = ASPEED_I2C_SLAVE_INACTIVE;
>   		break;
>   	}
>   
> @@ -329,6 +340,17 @@ static void aspeed_i2c_do_start(struct aspeed_i2c_bus *bus)
>   	u8 slave_addr = i2c_8bit_addr_from_msg(msg);
>   
>   	bus->master_state = ASPEED_I2C_MASTER_START;
> +
> +#if IS_ENABLED(CONFIG_I2C_SLAVE)
> +	/*
> +	 * If it's requested in the middle of a slave session, set the master
> +	 * state to 'pending' then H/W will continue handling this master
> +	 * command when the bus comes back to the idle state.
> +	 */
> +	if (bus->slave_state != ASPEED_I2C_SLAVE_INACTIVE)
> +		bus->master_state = ASPEED_I2C_MASTER_PENDING;
> +#endif /* CONFIG_I2C_SLAVE */
> +
>   	bus->buf_index = 0;
>   
>   	if (msg->flags & I2C_M_RD) {
> @@ -384,10 +406,6 @@ static u32 aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus, u32 irq_status)
>   		bus->master_state = ASPEED_I2C_MASTER_INACTIVE;
>   		irq_handled |= 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;
>   	}
>   
>   	/*
> @@ -399,11 +417,32 @@ static u32 aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus, u32 irq_status)
>   	if (ret) {
>   		dev_dbg(bus->dev, "received error interrupt: 0x%08x\n",
>   			irq_status);
> -		bus->cmd_err = ret;
> -		bus->master_state = ASPEED_I2C_MASTER_INACTIVE;
>   		irq_handled |= (irq_status & ASPEED_I2CD_INTR_MASTER_ERRORS);
> -		goto out_complete;
> +		if (bus->master_state != ASPEED_I2C_MASTER_INACTIVE) {
> +			bus->cmd_err = ret;
> +			bus->master_state = ASPEED_I2C_MASTER_INACTIVE;
> +			goto out_complete;
> +		}
> +	}
> +
> +#if IS_ENABLED(CONFIG_I2C_SLAVE)
> +	/*
> +	 * A pending master command will be started by H/W when the bus comes
> +	 * back to idle state after completing a slave operation so change the
> +	 * master state from 'pending' to 'start' at here if slave is inactive.
> +	 */
> +	if (bus->master_state == ASPEED_I2C_MASTER_PENDING) {
> +		if (bus->slave_state != ASPEED_I2C_SLAVE_INACTIVE)
> +			goto out_no_complete;
> +
> +		bus->master_state = ASPEED_I2C_MASTER_START;
>   	}
> +#endif /* CONFIG_I2C_SLAVE */
> +
> +	/* Master is not currently active, irq was for someone else. */
> +	if (bus->master_state == ASPEED_I2C_MASTER_INACTIVE ||
> +	    bus->master_state == ASPEED_I2C_MASTER_PENDING)
> +		goto out_no_complete;
>   
>   	/* We are in an invalid state; reset bus to a known state. */
>   	if (!bus->msgs) {
> @@ -423,6 +462,20 @@ static u32 aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus, u32 irq_status)
>   	 * then update the state and handle the new state below.
>   	 */
>   	if (bus->master_state == ASPEED_I2C_MASTER_START) {
> +#if IS_ENABLED(CONFIG_I2C_SLAVE)
> +		/*
> +		 * If a peer master starts a xfer immediately after it queues a
> +		 * master command, change its state to 'pending' then H/W will
> +		 * continue the queued master xfer just after completing the
> +		 * slave mode session.
> +		 */
> +		if (unlikely(irq_status & ASPEED_I2CD_INTR_SLAVE_MATCH)) {
> +			bus->master_state = ASPEED_I2C_MASTER_PENDING;
> +			dev_dbg(bus->dev,
> +				"master goes pending due to a slave start\n");
> +			goto out_no_complete;
> +		}
> +#endif /* CONFIG_I2C_SLAVE */
>   		if (unlikely(!(irq_status & ASPEED_I2CD_INTR_TX_ACK))) {
>   			if (unlikely(!(irq_status & ASPEED_I2CD_INTR_TX_NAK))) {
>   				bus->cmd_err = -ENXIO;
> @@ -566,7 +619,8 @@ static irqreturn_t aspeed_i2c_bus_irq(int irq, void *dev_id)
>   	 * interrupt bits. Each case needs to be handled using corresponding
>   	 * handlers depending on the current state.
>   	 */
> -	if (bus->master_state != ASPEED_I2C_MASTER_INACTIVE) {
> +	if (bus->master_state != ASPEED_I2C_MASTER_INACTIVE &&
> +	    bus->master_state != ASPEED_I2C_MASTER_PENDING) {
>   		irq_handled = aspeed_i2c_master_irq(bus, irq_remaining);
>   		irq_remaining &= ~irq_handled;
>   		if (irq_remaining)
> @@ -601,15 +655,15 @@ 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;
> +	int ret;
>   
>   	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) {
> +	/* If bus is busy in a single master environment, attempt recovery. */
> +	if (!bus->multi_master &&
> +	    (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)
> @@ -629,10 +683,20 @@ static int aspeed_i2c_master_xfer(struct i2c_adapter *adap,
>   	time_left = wait_for_completion_timeout(&bus->cmd_complete,
>   						bus->adap.timeout);
>   
> -	if (time_left == 0)
> +	if (time_left == 0) {
> +		/*
> +		 * If timed out and bus is still busy in a multi master
> +		 * environment, attempt recovery at here.
> +		 */
> +		if (bus->multi_master &&
> +		    (readl(bus->base + ASPEED_I2C_CMD_REG) &
> +		     ASPEED_I2CD_BUS_BUSY_STS))
> +			ret = aspeed_i2c_recover_bus(bus);
> +
>   		return -ETIMEDOUT;
> -	else
> -		return bus->master_xfer_result;
> +	}
> +
> +	return bus->master_xfer_result;
>   }
>   
>   static u32 aspeed_i2c_functionality(struct i2c_adapter *adap)
> @@ -672,7 +736,7 @@ static int aspeed_i2c_reg_slave(struct i2c_client *client)
>   	__aspeed_i2c_reg_slave(bus, client->addr);
>   
>   	bus->slave = client;
> -	bus->slave_state = ASPEED_I2C_SLAVE_STOP;
> +	bus->slave_state = ASPEED_I2C_SLAVE_INACTIVE;
>   	spin_unlock_irqrestore(&bus->lock, flags);
>   
>   	return 0;
> @@ -827,7 +891,9 @@ static int aspeed_i2c_init(struct aspeed_i2c_bus *bus,
>   	if (ret < 0)
>   		return ret;
>   
> -	if (!of_property_read_bool(pdev->dev.of_node, "multi-master"))
> +	if (of_property_read_bool(pdev->dev.of_node, "multi-master"))
> +		bus->multi_master = true;
> +	else
>   		fun_ctrl_reg |= ASPEED_I2CD_MULTI_MASTER_DIS;
>   
>   	/* Enable Master Mode */
> 

^ permalink raw reply

* [PATCH 2/2] clk: aspeed: Setup video engine clocking
From: Stephen Boyd @ 2019-01-30 19:28 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <81b100b7-252e-3145-fb34-17a9c0cdd91e@linux.ibm.com>

Quoting Eddie James (2018-12-14 07:47:53)
> 
> 
> On 12/13/2018 07:02 PM, Joel Stanley wrote:
> 
> >
> >> +       { 0x0, 2 },
> >> +       { 0x1, 2 },
> >> +       { 0x2, 3 },
> >> +       { 0x3, 4 },
> >> +       { 0x4, 5 },
> >> +       { 0x5, 6 },
> >> +       { 0x6, 7 },
> >> +       { 0x7, 8 },
> >> +       { 0 }
> >> +};
> >> @@ -317,6 +338,7 @@ struct aspeed_reset {
> >>          [ASPEED_RESET_PECI]     = 10,
> >>          [ASPEED_RESET_I2C]      =  2,
> >>          [ASPEED_RESET_AHB]      =  1,
> >> +       [ASPEED_RESET_VIDEO]    =  6,
> > You've added the reset line to the ASPEED_CLK_GATE_ECLK clock so you
> > do not need to separately expose the reset controller. Instead
> > enabling the clock will deassert the rest line for you.
> >
> > This means you should drop the change from the header too, and it
> > affects the bindings document for the video engine.
> 
> I want that reset available separately for use in the video engine 
> actually. I could do without it, but it's somewhat useful.
> 

Joel, are you happy with these patches? I'm tempted to drop these
changes from my queue because there hasn't been any news in over a
month.


^ permalink raw reply

* [PATCH] ARM: dts: aspeed: tiogapass: Enable VUART
From: Vijay Khemka @ 2019-01-30 18:14 UTC (permalink / raw)
  To: linux-aspeed

Enabling vuart for Facebook tiogapass

Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
---
 arch/arm/boot/dts/aspeed-bmc-facebook-tiogapass.dts | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/aspeed-bmc-facebook-tiogapass.dts b/arch/arm/boot/dts/aspeed-bmc-facebook-tiogapass.dts
index 42e0d7a8e8d0..a058fb2985f7 100644
--- a/arch/arm/boot/dts/aspeed-bmc-facebook-tiogapass.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-facebook-tiogapass.dts
@@ -64,6 +64,11 @@
 	status = "okay";
 };
 
+&vuart {
+	// VUART Host Console
+	status = "okay";
+};
+
 &uart1 {
 	// Host Console
 	status = "okay";
-- 
2.17.1


^ permalink raw reply related

* [PATCH 1/2] ARM: dts: aspeed: Add peci and lpc for Facebook
From: Vijay Khemka @ 2019-01-30 18:01 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <CACPK8Xc91m9TWoRSgDCT7bVJC=C=w7mse8xbdCgw5vMVgMXM=Q@mail.gmail.com>



?On 1/29/19, 7:27 PM, "Joel Stanley" <joel@jms.id.au> wrote:

    On Thu, 24 Jan 2019 at 10:07, Vijay Khemka <vijaykhemka@fb.com> wrote:
    
    > index 73e58a821613..d60dbb019f82 100644
    > --- a/arch/arm/boot/dts/aspeed-bmc-facebook-tiogapass.dts
    > +++ b/arch/arm/boot/dts/aspeed-bmc-facebook-tiogapass.dts
    
    >
    > +&peci0 {
    > +       status = "okay";
    > +};
    
    I don't think we have an upstream tree that contains a node called
    peci0. Therefore this does not build.
    
    I have dropped this hunk from your patch. Please make sure you test
    before sending patches.
Thanks Joel, I will be careful.
    
    Cheers,
    
    Joel
    


^ permalink raw reply

* [PATCH] ARM: dts: aspeed: quanta-q71l: enable uart1
From: Joel Stanley @ 2019-01-30  3:38 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20190129214703.119694-1-venture@google.com>

On Wed, 30 Jan 2019 at 08:47, Patrick Venture <venture@google.com> wrote:
>
> Enable the uart1 node such that the clock will be enabled.
>
> Signed-off-by: Patrick Venture <venture@google.com>

Applied to the aspeed tree for 5.1.

Cheers,

Joel

^ permalink raw reply

* [PATCH] ARM: dts: aspeed: quanta-q71l: enable lpc_ctrl node
From: Joel Stanley @ 2019-01-30  3:37 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20190129214624.119330-1-venture@google.com>

On Wed, 30 Jan 2019 at 08:46, Patrick Venture <venture@google.com> wrote:
>
> Enable the lpc_ctrl node in the quanta-q71l dts such that the LPC_CLK is
> enabled.
>
> Signed-off-by: Patrick Venture <venture@google.com>

Applied to the aspeed tree for 5.1.

Cheers,

Joel

^ permalink raw reply

* [PATCH v3] dt-bindings: aspeed-lpc: Make parameter optional
From: Joel Stanley @ 2019-01-30  3:30 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20190129230538.1044378-1-vijaykhemka@fb.com>

On Wed, 30 Jan 2019 at 10:05, Vijay Khemka <vijaykhemka@fb.com> wrote:
>
> Memory-region and flash phandle is not a required parameter, it is
> optional to describe in device tree and needed only use basis.
>
> Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>

Acked-by: Joel Stanley <joel@jms.id.au>

Thanks for following this up Vijay.

^ permalink raw reply

* [PATCH 2/2] ARM: dts: aspeed: Add uarts for SoL in Facebook
From: Joel Stanley @ 2019-01-30  3:27 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20190123230733.1821501-2-vijaykhemka@fb.com>

On Thu, 24 Jan 2019 at 10:07, Vijay Khemka <vijaykhemka@fb.com> wrote:
>
> Added uart2 and uart3 in Facebook Tiogapass for routing serial input
> from Host to BMC for SoL via LPC.
>
> Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>

Applied for 5.1.

Cheers,

Joel

^ permalink raw reply

* [PATCH 1/2] ARM: dts: aspeed: Add peci and lpc for Facebook
From: Joel Stanley @ 2019-01-30  3:27 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20190123230733.1821501-1-vijaykhemka@fb.com>

On Thu, 24 Jan 2019 at 10:07, Vijay Khemka <vijaykhemka@fb.com> wrote:

> index 73e58a821613..d60dbb019f82 100644
> --- a/arch/arm/boot/dts/aspeed-bmc-facebook-tiogapass.dts
> +++ b/arch/arm/boot/dts/aspeed-bmc-facebook-tiogapass.dts

>
> +&peci0 {
> +       status = "okay";
> +};

I don't think we have an upstream tree that contains a node called
peci0. Therefore this does not build.

I have dropped this hunk from your patch. Please make sure you test
before sending patches.

Cheers,

Joel

^ permalink raw reply

* [PATCH v3] dt-bindings: aspeed-lpc: Make parameter optional
From: Vijay Khemka @ 2019-01-29 23:05 UTC (permalink / raw)
  To: linux-aspeed

Memory-region and flash phandle is not a required parameter, it is
optional to describe in device tree and needed only use basis.

Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
---
 Documentation/devicetree/bindings/mfd/aspeed-lpc.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/aspeed-lpc.txt b/Documentation/devicetree/bindings/mfd/aspeed-lpc.txt
index 34dd89087cff..86446074e206 100644
--- a/Documentation/devicetree/bindings/mfd/aspeed-lpc.txt
+++ b/Documentation/devicetree/bindings/mfd/aspeed-lpc.txt
@@ -135,6 +135,8 @@ Required properties:
 - clocks:	contains a phandle to the syscon node describing the clocks.
 		There should then be one cell representing the clock to use
 
+Optional properties:
+
 - memory-region: A phandle to a reserved_memory region to be used for the LPC
 		to AHB mapping
 
-- 
2.17.1


^ permalink raw reply related

* [PATCH v2] dt-bindings: aspeed-lpc: Make parameter optional
From: Vijay Khemka @ 2019-01-29 23:00 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20190129123008.GC24070@dell>

Thanks Lee,

?On 1/29/19, 4:30 AM, "Lee Jones" <lee.jones@linaro.org> wrote:

    On Fri, 18 Jan 2019, Vijay Khemka wrote:
    
    > Memory-region and flash phandle is not a required parameter, it is
    > optional to describe in device tree and needed only use basis.
    > 
    > Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
    > ---
    >  Documentation/devicetree/bindings/mfd/aspeed-lpc.txt | 3 +++
    >  1 file changed, 3 insertions(+)
    > 
    > diff --git a/Documentation/devicetree/bindings/mfd/aspeed-lpc.txt b/Documentation/devicetree/bindings/mfd/aspeed-lpc.txt
    > index 34dd89087cff..d28c4e117611 100644
    > --- a/Documentation/devicetree/bindings/mfd/aspeed-lpc.txt
    > +++ b/Documentation/devicetree/bindings/mfd/aspeed-lpc.txt
    > @@ -135,6 +135,9 @@ Required properties:
    >  - clocks:	contains a phandle to the syscon node describing the clocks.
    >  		There should then be one cell representing the clock to use
    >  
    > +Optional properties:
    > +These below properties are optional to define.
    
    This line is superfluous.  Please remove it.
I will remove it. I am assuming only 1 line to remove.
    
    > +
    >  - memory-region: A phandle to a reserved_memory region to be used for the LPC
    >  		to AHB mapping
    >  
    
    -- 
    Lee Jones [???]
    Linaro Services Technical Lead
    Linaro.org ? Open source software for ARM SoCs
    Follow Linaro: Facebook | Twitter | Blog
    


^ permalink raw reply

* [PATCH] ARM: dts: aspeed: quanta-q71l: enable uart1
From: Patrick Venture @ 2019-01-29 21:47 UTC (permalink / raw)
  To: linux-aspeed

Enable the uart1 node such that the clock will be enabled.

Signed-off-by: Patrick Venture <venture@google.com>
---
 arch/arm/boot/dts/aspeed-bmc-quanta-q71l.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/aspeed-bmc-quanta-q71l.dts b/arch/arm/boot/dts/aspeed-bmc-quanta-q71l.dts
index 999679b5ef55..0d7c6339da46 100644
--- a/arch/arm/boot/dts/aspeed-bmc-quanta-q71l.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-quanta-q71l.dts
@@ -138,6 +138,10 @@
 	pinctrl-0 = <&pinctrl_rgmii2_default &pinctrl_mdio2_default>;
 };
 
+&uart1 {
+	status = "okay";
+};
+
 &uart5 {
 	status = "okay";
 };
-- 
2.20.1.495.gaa96b0ce6b-goog


^ permalink raw reply related


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