Re: [PATCH 08/12] PCI/switchtec: Add gen4 support in struct sys_info_regs

[Bjorn Helgaas]

On Mon, Jan 06, 2020 at 12:03:33PM -0700, Logan Gunthorpe wrote:
> Add a union with gen3 and gen4 sys_info structs. Ensure any use of the
> gen3 struct is gaurded by an if statement on stdev->gen.

s/gaurded/guarded/

> Signed-off-by: Logan Gunthorpe <logang@xxxxxxxxxxxx>
> ---
>  drivers/pci/switch/switchtec.c | 41 +++++++++++++++++++++++++++---
>  include/linux/switchtec.h      | 46 +++++++++++++++++++++++++++++++++-
>  2 files changed, 82 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
> index 21d3dd6e74f9..90d9c00984a7 100644
> --- a/drivers/pci/switch/switchtec.c
> +++ b/drivers/pci/switch/switchtec.c
> @@ -317,8 +317,14 @@ static ssize_t field ## _show(struct device *dev, \
>  	struct device_attribute *attr, char *buf) \
>  { \
>  	struct switchtec_dev *stdev = to_stdev(dev); \
> -	return io_string_show(buf, &stdev->mmio_sys_info->gen3.field, \
> -			    sizeof(stdev->mmio_sys_info->gen3.field)); \
> +	struct sys_info_regs __iomem *si = stdev->mmio_sys_info; \
> +	\
> +	if (stdev->gen == SWITCHTEC_GEN4) \
> +		return io_string_show(buf, &si->gen4.field, \
> +				      sizeof(si->gen4.field)); \
> +	else \
> +		return io_string_show(buf, &si->gen3.field, \
> +				      sizeof(si->gen3.field)); \
>  } \
>  \
>  static DEVICE_ATTR_RO(field)
> @@ -326,7 +332,21 @@ static DEVICE_ATTR_RO(field)
>  DEVICE_ATTR_SYS_INFO_STR(vendor_id);
>  DEVICE_ATTR_SYS_INFO_STR(product_id);
>  DEVICE_ATTR_SYS_INFO_STR(product_revision);
> -DEVICE_ATTR_SYS_INFO_STR(component_vendor);
> +
> +static ssize_t component_vendor_show(struct device *dev,
> +	struct device_attribute *attr, char *buf)
> +{
> +	struct switchtec_dev *stdev = to_stdev(dev);
> +	struct sys_info_regs __iomem *si = stdev->mmio_sys_info;
> +
> +	/* component_vendor field not supported after gen4 */
> +	if (stdev->gen != SWITCHTEC_GEN3)

I assume the comment should say "after gen3"?  More occurrences below.

> +		return sprintf(buf, "none\n");
> +
> +	return io_string_show(buf, &si->gen3.component_vendor,
> +			      sizeof(si->gen3.component_vendor));
> +}
> +static DEVICE_ATTR_RO(component_vendor);
>  
>  static ssize_t component_id_show(struct device *dev,
>  	struct device_attribute *attr, char *buf)
> @@ -334,6 +354,10 @@ static ssize_t component_id_show(struct device *dev,
>  	struct switchtec_dev *stdev = to_stdev(dev);
>  	int id = ioread16(&stdev->mmio_sys_info->gen3.component_id);
>  
> +	/* component_id field not supported after gen4 */
> +	if (stdev->gen != SWITCHTEC_GEN3)
> +		return sprintf(buf, "none\n");
> +
>  	return sprintf(buf, "PM%04X\n", id);
>  }
>  static DEVICE_ATTR_RO(component_id);
> @@ -344,6 +368,10 @@ static ssize_t component_revision_show(struct device *dev,
>  	struct switchtec_dev *stdev = to_stdev(dev);
>  	int rev = ioread8(&stdev->mmio_sys_info->gen3.component_revision);
>  
> +	/* component_revision field not supported after gen4 */
> +	if (stdev->gen != SWITCHTEC_GEN3)
> +		return sprintf(buf, "255\n");
> +