On Sun, 2015-11-15 at 12:33 +0100, Christian Fetzer wrote:
> The SB800 chipset supports a multiplexed main SMBus controller with
> four ports. The multiplexed ports share the same SMBus address and
> register set. The port is selected by bits 2:1 of the smb_en register
> (0x2C).
>
> Only one port can be active at any point in time therefore a mutex is
> needed in order to synchronize access.
>
> Additionally, the commit avoids requesting and releasing the SMBus
> base
> address index region on every multiplexed transfer by moving the
> request_region call into piix4_probe.
>
> Tested on HP ProLiant MicroServer G7 N54L (where this patch adds
> support to access sensor data from the w83795adg).
One nitpick below.
>
> +/*
> + * Handles access to multiple SMBus ports on the SB800.
>
> + * The port is selected by bits 2:1 of the smb_en register (0x2C).
(1)
> + * Returns negative errno on error.
> + *
> + * Note: The selected port must be returned to the initial selection
> to avoid
> + * problems on certain systems.
> + */
> +static s32 piix4_access_sb800(struct i2c_adapter *adap, u16 addr,
> + unsigned short flags, char read_write,
> + u8 command, int size, union i2c_smbus_data *data)
> +{
> + struct i2c_piix4_adapdata *adapdata =
> i2c_get_adapdata(adap);
> + u8 smba_en_lo, smb_en = 0x2c;
> + u8 port;
> + int retval;
> +
> + mutex_lock(adapdata->mutex);
> +
> + outb_p(smb_en, SB800_PIIX4_SMB_IDX);
> + smba_en_lo = inb_p(SB800_PIIX4_SMB_IDX + 1);
> +
> + port = adapdata->port;
> + if ((smba_en_lo & 6) != (port << 1))
I think it would be nice to have a definition for magic number along
with (1).
/* The port is selected by bits 2:1 of the smb_en register (0x2C) */
#define SB800_PIIX4_PORT_IDX_MASK 0x06
> + outb_p((smba_en_lo & ~6) | (port << 1),
> + SB800_PIIX4_SMB_IDX + 1);
> +
> + retval = piix4_access(adap, addr, flags, read_write,
> + command, size, data);
> +
> + outb_p(smba_en_lo, SB800_PIIX4_SMB_IDX + 1);
> +
> + mutex_unlock(adapdata->mutex);
> +
> + return retval;
> +}
> +
> static u32 piix4_func(struct i2c_adapter *adapter)
> {
> return I2C_FUNC_SMBUS_QUICK | I2C_FUNC_SMBUS_BYTE |
> @@ -539,6 +581,11 @@ static const struct i2c_algorithm
> smbus_algorithm = {
> .functionality = piix4_func,
> };
>
> +static const struct i2c_algorithm piix4_smbus_algorithm_sb800 = {
> + .smbus_xfer = piix4_access_sb800,
> + .functionality = piix4_func,
> +};
> +
> static const struct pci_device_id piix4_ids[] = {
> { PCI_DEVICE(PCI_VENDOR_ID_INTEL,
> PCI_DEVICE_ID_INTEL_82371AB_3) },
> { PCI_DEVICE(PCI_VENDOR_ID_INTEL,
> PCI_DEVICE_ID_INTEL_82443MX_3) },
> @@ -614,6 +661,53 @@ static int piix4_add_adapter(struct pci_dev
> *dev, unsigned short smba,
> return 0;
> }
>
> +static int piix4_add_adapters_sb800(struct pci_dev *dev, unsigned
> short smba)
> +{
> + struct mutex *mutex;
> + struct i2c_piix4_adapdata *adapdata;
> + int port;
> + int retval;
> +
> + mutex = kzalloc(sizeof(*mutex), GFP_KERNEL);
> + if (mutex == NULL)
> + return -ENOMEM;
> +
> + mutex_init(mutex);
> +
> + for (port = 0; port < PIIX4_MAX_ADAPTERS; port++) {
> + retval = piix4_add_adapter(dev, smba,
> + &piix4_main_adapters[port
> ]);
> + if (retval < 0)
> + goto error;
> +
> + piix4_main_adapters[port]->algo =
> &piix4_smbus_algorithm_sb800;
> +
> + adapdata =
> i2c_get_adapdata(piix4_main_adapters[port]);
> + adapdata->sb800_main = true;
> + adapdata->port = port;
> + adapdata->mutex = mutex;
> + }
> +
> + return retval;
> +
> +error:
> + dev_err(&dev->dev,
> + "Error setting up SB800 adapters.
> Unregistering!\n");
> + while (--port >= 0) {
> + adapdata =
> i2c_get_adapdata(piix4_main_adapters[port]);
> + if (adapdata->smba) {
> + i2c_del_adapter(piix4_main_adapters[port]);
> + kfree(adapdata);
> + kfree(piix4_main_adapters[port]);
> + piix4_main_adapters[port] = NULL;
> + }
> + }
> +
> + kfree(mutex);
> +
> + return retval;
> +}
> +
> static int piix4_probe(struct pci_dev *dev, const struct
> pci_device_id *id)
> {
> int retval;
> @@ -621,20 +715,41 @@ static int piix4_probe(struct pci_dev *dev,
> const struct pci_device_id *id)
> if ((dev->vendor == PCI_VENDOR_ID_ATI &&
> dev->device == PCI_DEVICE_ID_ATI_SBX00_SMBUS &&
> dev->revision >= 0x40) ||
> - dev->vendor == PCI_VENDOR_ID_AMD)
> + dev->vendor == PCI_VENDOR_ID_AMD) {
> + if (!request_region(SB800_PIIX4_SMB_IDX, 2,
> "smba_idx")) {
> + dev_err(&dev->dev,
> +
