linux-kernel - Re: [PATCH v12 1/2] i2c: designware: add SLAVE mode functions

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]

Date:   Tue, 27 Jun 2017 17:22:57 +0300
From:   Jarkko Nikula <jarkko.nikula@...ux.intel.com>
To:     Luis Oliveira <Luis.Oliveira@...opsys.com>, wsa@...-dreams.de,
        robh+dt@...nel.org, mark.rutland@....com,
        andriy.shevchenko@...ux.intel.com, mika.westerberg@...ux.intel.com,
        linux-i2c@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org
Cc:     Ramiro.Oliveira@...opsys.com, Joao.Pinto@...opsys.com,
        CARLOS.PALMINHA@...opsys.com
Subject: Re: [PATCH v12 1/2] i2c: designware: add SLAVE mode functions

On 06/22/2017 01:17 PM, Luis Oliveira wrote:
> - Changes in Kconfig to enable I2C_DESIGNWARE_SLAVE support
> - Slave functions added to core library file
> - Slave abort sources added to common source file
> - New driver: i2c-designware-slave added
> - Changes in the Makefile to compile the I2C_DESIGNWARE_SLAVE module
>   when supported by the architecture.
>
> All the SLAVE flow is added but it is not enabled via platform
> driver.
>
> Signed-off-by: Luis Oliveira <lolivei@...opsys.com>
> ---
> V11-v12
> - Changed ifdef condition in the Makefile fixed Kbuild error
>
>  drivers/i2c/busses/Kconfig                 |  14 +-
>  drivers/i2c/busses/Makefile                |   3 +
>  drivers/i2c/busses/i2c-designware-common.c |   6 +
>  drivers/i2c/busses/i2c-designware-core.h   |   2 +
>  drivers/i2c/busses/i2c-designware-slave.c  | 395 +++++++++++++++++++++++++++++
>  5 files changed, 419 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/i2c/busses/i2c-designware-slave.c
>
...
> +static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
> +{
> +	u32 raw_stat, stat, enabled;
> +	u8 val, slave_activity;
> +
> +	stat = dw_readl(dev, DW_IC_INTR_STAT);
> +	enabled = dw_readl(dev, DW_IC_ENABLE);
> +	raw_stat = dw_readl(dev, DW_IC_RAW_INTR_STAT);
> +	slave_activity = ((dw_readl(dev, DW_IC_STATUS) &
> +		DW_IC_STATUS_SLAVE_ACTIVITY) >> 6);
> +
> +	if (!enabled || !(raw_stat & ~DW_IC_INTR_ACTIVITY))
> +		return 0;
> +
> +	dev_dbg(dev->dev,
> +		"%#x STAUTS SLAVE_ACTTVITY=%#x : RAW_INTR_STAT=%#x"
> +		" : INTR_STAT=%#x\n",
> +		enabled, slave_activity, raw_stat, stat);
> +

Minor notes:
- Strings that are printed should fit in single line for making easier 
to grep them from sources. Maybe not all of those variables are not 
necessary in this debug print (e.g. are we interested other than bit 0 
in DW_IC_ENABLE and line above already returns if it reads zero).
- Typos "STAUTS SLAVE_ACTTVITY".

You may add my acked by if you want to send another version or I'm fine 
with incremental patch also.

Acked-by: Jarkko Nikula <jarkko.nikula@...ux.intel.com>