Hi Alexandra,
On Wed, 4 Nov 2015 11:09:16 -0800, Alexandra Yates wrote:
> Adding Intel codename Lewisburg platform device IDs for SMBus.
>
> Signed-off-by: Alexandra Yates <alexandra.yates@xxxxxxxxxxxxxxx>
> ---
> Documentation/i2c/busses/i2c-i801 | 1 +
> drivers/i2c/busses/Kconfig | 1 +
> drivers/i2c/busses/i2c-i801.c | 6 ++++++
> 3 files changed, 8 insertions(+)
>
> diff --git a/Documentation/i2c/busses/i2c-i801 b/Documentation/i2c/busses/i2c-i801
> index 6a4b1af..1bba38d 100644
> --- a/Documentation/i2c/busses/i2c-i801
> +++ b/Documentation/i2c/busses/i2c-i801
> @@ -32,6 +32,7 @@ Supported adapters:
> * Intel Sunrise Point-LP (PCH)
> * Intel DNV (SOC)
> * Intel Broxton (SOC)
> + * Intel Lewisburg (PCH)
> Datasheets: Publicly available at the Intel website
>
> On Intel Patsburg and later chipsets, both the normal host SMBus controller
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index e24c2b6..7b0aa82 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -126,6 +126,7 @@ config I2C_I801
> Sunrise Point-LP (PCH)
> DNV (SOC)
> Broxton (SOC)
> + Lewisburg (PCH)
>
> This driver can also be built as a module. If so, the module
> will be called i2c-i801.
> diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
> index c306751..76fcef4 100644
> --- a/drivers/i2c/busses/i2c-i801.c
> +++ b/drivers/i2c/busses/i2c-i801.c
> @@ -62,6 +62,8 @@
> * Sunrise Point-LP (PCH) 0x9d23 32 hard yes yes yes
> * DNV (SOC) 0x19df 32 hard yes yes yes
> * Broxton (SOC) 0x5ad4 32 hard yes yes yes
> + * Lewisburg (PCH) 0xA1A3 32 hard yes yes yes
> + * Lewisburg Supersku (PCH) 0xA223 32 hard yes yes yes
It's a bit unfortunate that you used upper-case letters for the
hexadecimal numbers when all other entries used lower-case letters.
> *
> * Features supported by this driver:
> * Software PEC no
> @@ -181,6 +183,8 @@
> STATUS_ERROR_FLAGS)
>
> /* Older devices have their ID defined in <linux/pci_ids.h> */
> +#define PCI_DEVICE_ID_INTEL_LEWISBURG_SMBUS 0xA1A3
> +#define PCI_DEVICE_ID_INTEL_LEWISBURG_SSKU_SMBUS 0xA223
Same here. Plus, why are you adding the IDs at the top of the list here
while you added them at the bottom of the first list...
> #define PCI_DEVICE_ID_INTEL_BAYTRAIL_SMBUS 0x0f12
> #define PCI_DEVICE_ID_INTEL_BRASWELL_SMBUS 0x2292
> #define PCI_DEVICE_ID_INTEL_COUGARPOINT_SMBUS 0x1c22
> @@ -864,6 +868,8 @@ static const struct pci_device_id i801_ids[] = {
> { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_WILDCATPOINT_SMBUS) },
> { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_WILDCATPOINT_LP_SMBUS) },
> { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_BAYTRAIL_SMBUS) },
> + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_LEWISBURG_SMBUS) },
> + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_LEWISBURG_SSKU_SMBUS) },
> { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_BRASWELL_SMBUS) },
> { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_SUNRISEPOINT_H_SMBUS) },
> { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_SUNRISEPOINT_LP_SMBUS) },
... and now in the middle of the last list? I would appreciate more
consistency.
These are all details of course, overall the changes look good, so you
can add:
Reviewed-by: Jean Delvare <jdelvare@xxxxxxx>
--
Jean Delvare
SUSE L3 Support
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
