On 06.12.2021 22:18, Heiner Kallweit wrote:
> Currently we use the following feature definitions. However, according to
> the respective datasheets, also ICH/ICH0/ICH2/ICH3/ICH4 support I2C block
> read. The implementation we have should work also on these chip versions.
>
Basically the same I found for interrupt support. It's available on all
chip versions. Any specific reason why driver uses interrupts from ICH5
only?
> The commit message of 6342064cad7a ("i2c-i801: Implement I2C block read
> support") states that i2c block read is supported from ICH5 only.
> This doesn't seem to be true. Or is this feature broken on older chip
> versions?
>
> To me it seems we could remove FEATURE_I2C_BLOCK_READ because all chip
> versions support this feature. Below is an experimental patch, for the
> ones with test hw. A test case could be to use decode-dimms that
> uses i2c block read to read the EEPROM content.
>
> * Supports the following Intel I/O Controller Hubs (ICH):
> *
> * I/O Block I2C
> * region SMBus Block proc. block
> * Chip name PCI ID size PEC buffer call read
> * ---------------------------------------------------------------------------
> * 82801AA (ICH) 0x2413 16 no no no no
> * 82801AB (ICH0) 0x2423 16 no no no no
> * 82801BA (ICH2) 0x2443 16 no no no no
> * 82801CA (ICH3) 0x2483 32 soft no no no
> * 82801DB (ICH4) 0x24c3 32 hard yes no no
> * 82801E (ICH5) 0x24d3 32 hard yes yes yes
>
>
> diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
> index 930c6edbe..fd9a19a80 100644
> --- a/drivers/i2c/busses/i2c-i801.c
> +++ b/drivers/i2c/busses/i2c-i801.c
> @@ -294,7 +294,6 @@ struct i801_priv {
> #define FEATURE_SMBUS_PEC BIT(0)
> #define FEATURE_BLOCK_BUFFER BIT(1)
> #define FEATURE_BLOCK_PROC BIT(2)
> -#define FEATURE_I2C_BLOCK_READ BIT(3)
> #define FEATURE_IRQ BIT(4)
> #define FEATURE_HOST_NOTIFY BIT(5)
> /* Not really a feature, but it's convenient to handle it as such */
> @@ -780,10 +779,6 @@ static int i801_block_transaction(struct i801_priv *priv, union i2c_smbus_data *
> pci_read_config_byte(priv->pci_dev, SMBHSTCFG, &hostc);
> pci_write_config_byte(priv->pci_dev, SMBHSTCFG,
> hostc | SMBHSTCFG_I2C_EN);
> - } else if (!(priv->features & FEATURE_I2C_BLOCK_READ)) {
> - dev_err(&priv->pci_dev->dev,
> - "I2C block read is unsupported!\n");
> - return -EOPNOTSUPP;
> }
> }
>
> @@ -956,11 +951,10 @@ static u32 i801_func(struct i2c_adapter *adapter)
> return I2C_FUNC_SMBUS_QUICK | I2C_FUNC_SMBUS_BYTE |
> I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA |
> I2C_FUNC_SMBUS_BLOCK_DATA | I2C_FUNC_SMBUS_WRITE_I2C_BLOCK |
> + I2C_FUNC_SMBUS_READ_I2C_BLOCK |
> ((priv->features & FEATURE_SMBUS_PEC) ? I2C_FUNC_SMBUS_PEC : 0) |
> ((priv->features & FEATURE_BLOCK_PROC) ?
> I2C_FUNC_SMBUS_BLOCK_PROC_CALL : 0) |
> - ((priv->features & FEATURE_I2C_BLOCK_READ) ?
> - I2C_FUNC_SMBUS_READ_I2C_BLOCK : 0) |
> ((priv->features & FEATURE_HOST_NOTIFY) ?
> I2C_FUNC_SMBUS_HOST_NOTIFY : 0);
> }
> @@ -997,7 +991,7 @@ static const struct i2c_algorithm smbus_algorithm = {
> .functionality = i801_func,
> };
>
> -#define FEATURES_ICH5 (FEATURE_BLOCK_PROC | FEATURE_I2C_BLOCK_READ | \
> +#define FEATURES_ICH5 (FEATURE_BLOCK_PROC | \
> FEATURE_IRQ | FEATURE_SMBUS_PEC | \
> FEATURE_BLOCK_BUFFER | FEATURE_HOST_NOTIFY)
> #define FEATURES_ICH4 (FEATURE_SMBUS_PEC | FEATURE_BLOCK_BUFFER | \
