linux-kernel - Re: [PATCH v3 11/11] i2c: npcm: Support NPCM845

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:   Thu, 3 Mar 2022 12:43:56 +0200
From:   Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To:     Tyrone Ting <warp5tw@...il.com>
Cc:     avifishman70@...il.com, tmaimon77@...il.com, tali.perry1@...il.com,
        venture@...gle.com, yuenn@...gle.com, benjaminfair@...gle.com,
        robh+dt@...nel.org, krzysztof.kozlowski@...onical.com,
        yangyicong@...ilicon.com, semen.protsenko@...aro.org,
        wsa@...nel.org, jie.deng@...el.com, sven@...npeter.dev,
        bence98@....bme.hu, lukas.bulwahn@...il.com, arnd@...db.de,
        olof@...om.net, tali.perry@...oton.com, Avi.Fishman@...oton.com,
        tomer.maimon@...oton.com, KWLIU@...oton.com, JJLIU0@...oton.com,
        kfting@...oton.com, openbmc@...ts.ozlabs.org,
        linux-i2c@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 11/11] i2c: npcm: Support NPCM845

On Thu, Mar 03, 2022 at 04:31:41PM +0800, Tyrone Ting wrote:
> From: Tyrone Ting <kfting@...oton.com>
> 
> Add NPCM8XX I2C support.
> The NPCM8XX uses a similar i2c module as NPCM7XX.
> The internal HW FIFO is larger in NPCM8XX.
> 
> Signed-off-by: Tyrone Ting <kfting@...oton.com>
> Signed-off-by: Tali Perry <tali.perry1@...il.com>

Wrong SoB chain.

...

> +static const struct npcm_i2c_data npxm7xx_i2c_data = {
> +	.fifo_size = 16,
> +	.segctl_init_val = 0x0333F000,
> +	.txf_sts_tx_bytes = GENMASK(4, 0),
> +	.rxf_sts_rx_bytes = GENMASK(4, 0),
> +	.rxf_ctl_last_pec = BIT(5)

+ Comma.

> +};
> +
> +static const struct npcm_i2c_data npxm8xx_i2c_data = {
> +	.fifo_size = 32,
> +	.segctl_init_val = 0x9333F000,
> +	.txf_sts_tx_bytes = GENMASK(5, 0),
> +	.rxf_sts_rx_bytes = GENMASK(5, 0),
> +	.rxf_ctl_last_pec = BIT(7)

Ditto.

> +};

...

> -	left_in_fifo = FIELD_GET(NPCM_I2CTXF_STS_TX_BYTES,
> -				 ioread8(bus->reg + NPCM_I2CTXF_STS));
> +	left_in_fifo = (bus->data->txf_sts_tx_bytes &
> +			ioread8(bus->reg + NPCM_I2CTXF_STS));

Besides too many parentheses, this is an interesting change. So, in different
versions of IP the field is on different bits? Perhaps it means that you need
something like internal ops structure for all these, where you will have been
using the statically defined masks?

...

> +	match = of_match_device(npcm_i2c_bus_of_table, dev);
> +	if (!match) {
> +		dev_err(dev, "OF data missing\n");
> +		return -EINVAL;
> +	}
> +	bus->data = match->data;

This is NIH of_device_get_match_data().

...

> -static const struct of_device_id npcm_i2c_bus_of_table[] = {
> -	{ .compatible = "nuvoton,npcm750-i2c", },
> -	{}
> -};
> -MODULE_DEVICE_TABLE(of, npcm_i2c_bus_of_table);
> -

Redundant change, leave this as is.

-- 
With Best Regards,
Andy Shevchenko