On 30.01.2024 01:09, Andi Shyti wrote:
> Hi Heiner,
>
> ...
>
>> +static int i801_i2c_block_transaction(struct i801_priv *priv, union i2c_smbus_data *data,
>> + u8 addr, u8 hstcmd, char read_write, int command)
>> +{
>> + int result;
>> + u8 hostc;
>> +
>> + if (data->block[0] < 1 || data->block[0] > I2C_SMBUS_BLOCK_MAX)
>> + return -EPROTO;
>> + /*
>> + * NB: page 240 of ICH5 datasheet shows that the R/#W bit should be cleared here,
>> + * even when reading. However if SPD Write Disable is set (Lynx Point and later),
>> + * the read will fail if we don't set the R/#W bit.
>> + */
>> + i801_set_hstadd(priv, addr,
>> + priv->original_hstcfg & SMBHSTCFG_SPD_WD ? read_write : I2C_SMBUS_WRITE);
>> +
>> + /* NB: page 240 of ICH5 datasheet shows that DATA1 is the cmd field when reading */
>> + if (read_write == I2C_SMBUS_READ)
>> + outb_p(hstcmd, SMBHSTDAT1(priv));
>> + else
>> outb_p(hstcmd, SMBHSTCMD(priv));
>> - break;
>> +
>> + if (read_write == I2C_SMBUS_WRITE) {
>> + /* set I2C_EN bit in configuration register */
>> + 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)) {
>> + pci_err(priv->pci_dev, "I2C block read is unsupported!\n");
>> + return -EOPNOTSUPP;
>> }
>
> These two if...else blocks can be merged.
>
> But here the case of "command == I2C_SMBUS_BLOCK_DATA" is doing
> something different from the original code. E.g. if command =
> I2C_SMBUS_BLOCK_DATA and read_write = READ, then there is a
> functional change. Or am I getting confused?
>
I2C_SMBUS_BLOCK_DATA is handled by the new function
i801_smbus_block_transaction(). What may contribute to the confusion is that
there's also the command I2C_SMBUS_I2C_BLOCK_DATA, which is handled by
i801_i2c_block_transaction() now.
> Thanks,
> Andi
