On 21.06.2021 23:08, Heiner Kallweit wrote:
> Bit SMBHSTCNT_PEC_EN is used only if software calculates the CRC and
> uses register SMBPEC. This is not supported by the driver, it supports
> hw-calculation of CRC only (using bit SMBAUXSTS_CRCE). The chip spec
> states the following, therefore never set bit SMBHSTCNT_PEC_EN.
>
> Chapter SMBus CRC Generation and Checking
> If the AAC bit is set in the Auxiliary Control register, the PCH
> automatically calculates and drives CRC at the end of the transmitted
> packet for write cycles, and will check the CRC for read cycles. It will
> not transmit the contents of the PEC register for CRC. The PEC bit must
> not be set in the Host Control register. If this bit is set, unspecified
> behavior will result.
>
> This patch is based solely on the specification and compile-tested only,
> because I have no PEC-capable devices.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@xxxxxxxxx>
> ---
> This patch may be a candidate for stable (with a little bit of fuzz)
> once somebody with a PEC-capable device has tested it.
> ---
> drivers/i2c/busses/i2c-i801.c | 23 +++++++++--------------
> 1 file changed, 9 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
> index 0153a21f4..161176fcd 100644
> --- a/drivers/i2c/busses/i2c-i801.c
> +++ b/drivers/i2c/busses/i2c-i801.c
> @@ -516,12 +516,9 @@ static int i801_transaction(struct i801_priv *priv, int xact)
>
> static int i801_block_transaction_by_block(struct i801_priv *priv,
> union i2c_smbus_data *data,
> - char read_write, int command,
> - int hwpec)
> + char read_write, int command)
> {
> - int i, len;
> - int status;
> - int xact = hwpec ? SMBHSTCNT_PEC_EN : 0;
> + int i, len, status, xact = 0;
>
> switch (command) {
> case I2C_SMBUS_BLOCK_PROC_CALL:
> @@ -678,8 +675,7 @@ static irqreturn_t i801_isr(int irq, void *dev_id)
> */
> static int i801_block_transaction_byte_by_byte(struct i801_priv *priv,
> union i2c_smbus_data *data,
> - char read_write, int command,
> - int hwpec)
> + char read_write, int command)
> {
> int i, len;
> int smbcmd;
> @@ -777,9 +773,8 @@ static int i801_set_block_buffer_mode(struct i801_priv *priv)
> }
>
> /* Block transaction function */
> -static int i801_block_transaction(struct i801_priv *priv,
> - union i2c_smbus_data *data, char read_write,
> - int command, int hwpec)
> +static int i801_block_transaction(struct i801_priv *priv, union i2c_smbus_data *data,
> + char read_write, int command)
> {
> int result = 0;
> unsigned char hostc;
> @@ -815,11 +810,11 @@ static int i801_block_transaction(struct i801_priv *priv,
> && i801_set_block_buffer_mode(priv) == 0)
> result = i801_block_transaction_by_block(priv, data,
> read_write,
> - command, hwpec);
> + command);
> else
> result = i801_block_transaction_byte_by_byte(priv, data,
> read_write,
> - command, hwpec);
> + command);
>
> if (command == I2C_SMBUS_I2C_BLOCK_DATA
> && read_write == I2C_SMBUS_WRITE) {
> @@ -936,8 +931,7 @@ static s32 i801_access(struct i2c_adapter *adap, u16 addr,
> SMBAUXCTL(priv));
>
> if (block)
> - ret = i801_block_transaction(priv, data, read_write, size,
> - hwpec);
> + ret = i801_block_transaction(priv, data, read_write, size);
> else
> ret = i801_transaction(priv, xact);
>
> @@ -1625,6 +1619,7 @@ static void i801_setup_hstcfg(struct i801_priv *priv)
> unsigned char hstcfg = priv->original_hstcfg;
>
> hstcfg &= ~SMBHSTCFG_I2C_EN; /* SMBus timing */
> + hstcfg &= ~SMBHSTCNT_PEC_EN;
> hstcfg |= SMBHSTCFG_HST_EN;
> pci_write_config_byte(priv->pci_dev, SMBHSTCFG, hstcfg);
>
>
Jean, do you have an opinion on this patch? It's been pending for quite some
time and I have few more patches that I'd like to submit.
