lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1b77df67-63f4-253a-8333-4f5ca2f41a14@nokia.com>
Date:   Mon, 26 Aug 2019 11:59:08 +0000
From:   "Sverdlin, Alexander (Nokia - DE/Ulm)" <alexander.sverdlin@...ia.com>
To:     "Adamski, Krzysztof (Nokia - PL/Wroclaw)" 
        <krzysztof.adamski@...ia.com>, Wolfram Sang <wsa@...-dreams.de>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-i2c@...r.kernel.org" <linux-i2c@...r.kernel.org>
Subject: Re: [PATCH v3] i2c: axxia: support slave mode

Hi!

On 19/08/2019 11:07, Adamski, Krzysztof (Nokia - PL/Wroclaw) wrote:
> This device contains both master and slave controllers which can be
> enabled simultaneously. Both controllers share the same SDA/SCL lines
> and interrupt source but has separate control and status registers.
> Controllers also works in loopback mode - slave device can communicate
> with its own master controller internally. The controller can handle up
> to two addresses, both of which may be 10 bit. Most of the logic
> (sending (N)ACK, handling repeated start or switching between
> write/read) is handled automatically which makes working with this
> controller quite easy.
> 
> For simplicity, this patch adds basic support, limiting to only one
> slave address. Support for the 2nd device may be added in the future.
> 
> Note that synchronize_irq() is used to ensure any running slave interrupt
> is finished to make sure slave i2c_client structure can be safely used
> by i2c_slave_event.

Reviewed-by: Alexander Sverdlin <alexander.sverdlin@...ia.com>

> Signed-off-by: Krzysztof Adamski <krzysztof.adamski@...ia.com>
> ---
> Changes in v3:
> - Fixed braces for one-line if bodies
> 
> Changes in v2:
> - Reduced the number of dev_dbg messages.
> 
>  drivers/i2c/busses/Kconfig     |   1 +
>  drivers/i2c/busses/i2c-axxia.c | 152 +++++++++++++++++++++++++++++++--
>  2 files changed, 145 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index 69f19312a574..b8262301c86d 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -429,6 +429,7 @@ config I2C_AXXIA
>  	tristate "Axxia I2C controller"
>  	depends on ARCH_AXXIA || COMPILE_TEST
>  	default ARCH_AXXIA
> +	select I2C_SLAVE
>  	help
>  	  Say yes if you want to support the I2C bus on Axxia platforms.
>  
> diff --git a/drivers/i2c/busses/i2c-axxia.c b/drivers/i2c/busses/i2c-axxia.c
> index ff3142b15cab..0214daa913ff 100644
> --- a/drivers/i2c/busses/i2c-axxia.c
> +++ b/drivers/i2c/busses/i2c-axxia.c
> @@ -77,6 +77,40 @@
>  				 MST_STATUS_IP)
>  #define MST_TX_BYTES_XFRD	0x50
>  #define MST_RX_BYTES_XFRD	0x54
> +#define SLV_ADDR_DEC_CTL	0x58
> +#define   SLV_ADDR_DEC_GCE	BIT(0)  /* ACK to General Call Address from own master (loopback) */
> +#define   SLV_ADDR_DEC_OGCE	BIT(1)  /* ACK to General Call Address from external masters */
> +#define   SLV_ADDR_DEC_SA1E	BIT(2)  /* ACK to addr_1 enabled */
> +#define   SLV_ADDR_DEC_SA1M	BIT(3)  /* 10-bit addressing for addr_1 enabled */
> +#define   SLV_ADDR_DEC_SA2E	BIT(4)  /* ACK to addr_2 enabled */
> +#define   SLV_ADDR_DEC_SA2M	BIT(5)  /* 10-bit addressing for addr_2 enabled */
> +#define SLV_ADDR_1		0x5c
> +#define SLV_ADDR_2		0x60
> +#define SLV_RX_CTL		0x64
> +#define   SLV_RX_ACSA1		BIT(0)  /* Generate ACK for writes to addr_1 */
> +#define   SLV_RX_ACSA2		BIT(1)  /* Generate ACK for writes to addr_2 */
> +#define   SLV_RX_ACGCA		BIT(2)  /* ACK data phase transfers to General Call Address */
> +#define SLV_DATA		0x68
> +#define SLV_RX_FIFO		0x6c
> +#define   SLV_FIFO_DV1		BIT(0)  /* Data Valid for addr_1 */
> +#define   SLV_FIFO_DV2		BIT(1)  /* Data Valid for addr_2 */
> +#define   SLV_FIFO_AS		BIT(2)  /* (N)ACK Sent */
> +#define   SLV_FIFO_TNAK		BIT(3)  /* Timeout NACK */
> +#define   SLV_FIFO_STRC		BIT(4)  /* First byte after start condition received */
> +#define   SLV_FIFO_RSC		BIT(5)  /* Repeated Start Condition */
> +#define   SLV_FIFO_STPC		BIT(6)  /* Stop Condition */
> +#define   SLV_FIFO_DV		(SLV_FIFO_DV1 | SLV_FIFO_DV2)
> +#define SLV_INT_ENABLE		0x70
> +#define SLV_INT_STATUS		0x74
> +#define   SLV_STATUS_RFH	BIT(0)  /* FIFO service */
> +#define   SLV_STATUS_WTC	BIT(1)  /* Write transfer complete */
> +#define   SLV_STATUS_SRS1	BIT(2)  /* Slave read from addr 1 */
> +#define   SLV_STATUS_SRRS1	BIT(3)  /* Repeated start from addr 1 */
> +#define   SLV_STATUS_SRND1	BIT(4)  /* Read request not following start condition */
> +#define   SLV_STATUS_SRC1	BIT(5)  /* Read canceled */
> +#define   SLV_STATUS_SRAT1	BIT(6)  /* Slave Read timed out */
> +#define   SLV_STATUS_SRDRE1	BIT(7)  /* Data written after timed out */
> +#define SLV_READ_DUMMY		0x78
>  #define SCL_HIGH_PERIOD		0x80
>  #define SCL_LOW_PERIOD		0x84
>  #define SPIKE_FLTR_LEN		0x88
> @@ -111,6 +145,8 @@ struct axxia_i2c_dev {
>  	struct clk *i2c_clk;
>  	u32 bus_clk_rate;
>  	bool last;
> +	struct i2c_client *slave;
> +	int irq;
>  };
>  
>  static void i2c_int_disable(struct axxia_i2c_dev *idev, u32 mask)
> @@ -276,13 +312,65 @@ static int axxia_i2c_fill_tx_fifo(struct axxia_i2c_dev *idev)
>  	return ret;
>  }
>  
> +static void axxia_i2c_slv_fifo_event(struct axxia_i2c_dev *idev)
> +{
> +	u32 fifo_status = readl(idev->base + SLV_RX_FIFO);
> +	u8 val;
> +
> +	dev_dbg(idev->dev, "slave irq fifo_status=0x%x\n", fifo_status);
> +
> +	if (fifo_status & SLV_FIFO_DV1) {
> +		if (fifo_status & SLV_FIFO_STRC)
> +			i2c_slave_event(idev->slave,
> +					I2C_SLAVE_WRITE_REQUESTED, &val);
> +
> +		val = readl(idev->base + SLV_DATA);
> +		i2c_slave_event(idev->slave, I2C_SLAVE_WRITE_RECEIVED, &val);
> +	}
> +	if (fifo_status & SLV_FIFO_STPC) {
> +		readl(idev->base + SLV_DATA); /* dummy read */
> +		i2c_slave_event(idev->slave, I2C_SLAVE_STOP, &val);
> +	}
> +	if (fifo_status & SLV_FIFO_RSC)
> +		readl(idev->base + SLV_DATA); /* dummy read */
> +}
> +
> +static irqreturn_t axxia_i2c_slv_isr(struct axxia_i2c_dev *idev)
> +{
> +	u32 status = readl(idev->base + SLV_INT_STATUS);
> +	u8 val;
> +
> +	dev_dbg(idev->dev, "slave irq status=0x%x\n", status);
> +
> +	if (status & SLV_STATUS_RFH)
> +		axxia_i2c_slv_fifo_event(idev);
> +	if (status & SLV_STATUS_SRS1) {
> +		i2c_slave_event(idev->slave, I2C_SLAVE_READ_REQUESTED, &val);
> +		writel(val, idev->base + SLV_DATA);
> +	}
> +	if (status & SLV_STATUS_SRND1) {
> +		i2c_slave_event(idev->slave, I2C_SLAVE_READ_PROCESSED, &val);
> +		writel(val, idev->base + SLV_DATA);
> +	}
> +	if (status & SLV_STATUS_SRC1)
> +		i2c_slave_event(idev->slave, I2C_SLAVE_STOP, &val);
> +
> +	writel(INT_SLV, idev->base + INTERRUPT_STATUS);
> +	return IRQ_HANDLED;
> +}
> +
>  static irqreturn_t axxia_i2c_isr(int irq, void *_dev)
>  {
>  	struct axxia_i2c_dev *idev = _dev;
> +	irqreturn_t ret = IRQ_NONE;
>  	u32 status;
>  
> -	if (!(readl(idev->base + INTERRUPT_STATUS) & INT_MST))
> -		return IRQ_NONE;
> +	status = readl(idev->base + INTERRUPT_STATUS);
> +
> +	if (status & INT_SLV)
> +		ret = axxia_i2c_slv_isr(idev);
> +	if (!(status & INT_MST))
> +		return ret;
>  
>  	/* Read interrupt status bits */
>  	status = readl(idev->base + MST_INT_STATUS);
> @@ -583,9 +671,58 @@ static u32 axxia_i2c_func(struct i2c_adapter *adap)
>  	return caps;
>  }
>  
> +static int axxia_i2c_reg_slave(struct i2c_client *slave)
> +{
> +	struct axxia_i2c_dev *idev = i2c_get_adapdata(slave->adapter);
> +	u32 slv_int_mask = SLV_STATUS_RFH;
> +	u32 dec_ctl;
> +
> +	if (idev->slave)
> +		return -EBUSY;
> +
> +	idev->slave = slave;
> +
> +	/* Enable slave mode as well */
> +	writel(GLOBAL_MST_EN | GLOBAL_SLV_EN, idev->base + GLOBAL_CONTROL);
> +	writel(INT_MST | INT_SLV, idev->base + INTERRUPT_ENABLE);
> +
> +	/* Set slave address */
> +	dec_ctl = SLV_ADDR_DEC_SA1E;
> +	if (slave->flags & I2C_CLIENT_TEN)
> +		dec_ctl |= SLV_ADDR_DEC_SA1M;
> +
> +	writel(SLV_RX_ACSA1, idev->base + SLV_RX_CTL);
> +	writel(dec_ctl, idev->base + SLV_ADDR_DEC_CTL);
> +	writel(slave->addr, idev->base + SLV_ADDR_1);
> +
> +	/* Enable interrupts */
> +	slv_int_mask |= SLV_STATUS_SRS1 | SLV_STATUS_SRRS1 | SLV_STATUS_SRND1;
> +	slv_int_mask |= SLV_STATUS_SRC1;
> +	writel(slv_int_mask, idev->base + SLV_INT_ENABLE);
> +
> +	return 0;
> +}
> +
> +static int axxia_i2c_unreg_slave(struct i2c_client *slave)
> +{
> +	struct axxia_i2c_dev *idev = i2c_get_adapdata(slave->adapter);
> +
> +	/* Disable slave mode */
> +	writel(GLOBAL_MST_EN, idev->base + GLOBAL_CONTROL);
> +	writel(INT_MST, idev->base + INTERRUPT_ENABLE);
> +
> +	synchronize_irq(idev->irq);
> +
> +	idev->slave = NULL;
> +
> +	return 0;
> +}
> +
>  static const struct i2c_algorithm axxia_i2c_algo = {
>  	.master_xfer = axxia_i2c_xfer,
>  	.functionality = axxia_i2c_func,
> +	.reg_slave = axxia_i2c_reg_slave,
> +	.unreg_slave = axxia_i2c_unreg_slave,
>  };
>  
>  static const struct i2c_adapter_quirks axxia_i2c_quirks = {
> @@ -599,7 +736,6 @@ static int axxia_i2c_probe(struct platform_device *pdev)
>  	struct axxia_i2c_dev *idev = NULL;
>  	struct resource *res;
>  	void __iomem *base;
> -	int irq;
>  	int ret = 0;
>  
>  	idev = devm_kzalloc(&pdev->dev, sizeof(*idev), GFP_KERNEL);
> @@ -611,10 +747,10 @@ static int axxia_i2c_probe(struct platform_device *pdev)
>  	if (IS_ERR(base))
>  		return PTR_ERR(base);
>  
> -	irq = platform_get_irq(pdev, 0);
> -	if (irq < 0) {
> +	idev->irq = platform_get_irq(pdev, 0);
> +	if (idev->irq < 0) {
>  		dev_err(&pdev->dev, "missing interrupt resource\n");
> -		return irq;
> +		return idev->irq;
>  	}
>  
>  	idev->i2c_clk = devm_clk_get(&pdev->dev, "i2c");
> @@ -643,10 +779,10 @@ static int axxia_i2c_probe(struct platform_device *pdev)
>  		goto error_disable_clk;
>  	}
>  
> -	ret = devm_request_irq(&pdev->dev, irq, axxia_i2c_isr, 0,
> +	ret = devm_request_irq(&pdev->dev, idev->irq, axxia_i2c_isr, 0,
>  			       pdev->name, idev);
>  	if (ret) {
> -		dev_err(&pdev->dev, "failed to claim IRQ%d\n", irq);
> +		dev_err(&pdev->dev, "failed to claim IRQ%d\n", idev->irq);
>  		goto error_disable_clk;
>  	}

-- 
Best regards,
Alexander Sverdlin.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ