Re: [v10, 2/2] Documentation/ABI: Add ABI information for QCOM socinfo driver

[Rob Herring]

On Mon, Feb 20, 2017 at 10:17:15PM +0530, Khan, Imran wrote:
> The socinfo ABI document describes the information provided
> by socinfo driver and the corresponding attributes to access
> that information.
> 
> Signed-off-by: Imran Khan <kimran@xxxxxxxxxxxxxx>
> ---
>  .../ABI/testing/sysfs-driver-qcom_socinfo          | 171 +++++++++++++++++++++
>  1 file changed, 171 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-driver-qcom_socinfo

Sorry to comment late on this (blame LWM), but I think creating this ABI 
is a mistake. The biggest issue I have is this doesn't scale if every 
SoC does its own thing. We should have a common interface so for example 
userspace can retrieve the serial number from any SoC in the same way. 
Yes, we can have custom attributes, but there should be common base.


> diff --git a/Documentation/ABI/testing/sysfs-driver-qcom_socinfo b/Documentation/ABI/testing/sysfs-driver-qcom_socinfo
> new file mode 100644
> index 0000000..cce611f
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-driver-qcom_socinfo
> @@ -0,0 +1,171 @@
> +What:		/sys/devices/soc0/accessory_chip
> +Date:		January 2017
> +Contact:	linux-arm-msm@xxxxxxxxxxxxxxx
> +Description:
> +		This file shows the id of the accessory chip.
> +
> +What:		/sys/devices/soc0/adsp_image_crm
> +What:		/sys/devices/soc0/adsp_image_variant
> +What:		/sys/devices/soc0/adsp_image_version
> +Date:		January 2017
> +Contact:	linux-arm-msm@xxxxxxxxxxxxxxx
> +Description:
> +		These files respectively show the crm version, variant and
> +		version of the ADSP image.

Shouldn't this be part of the ADSP driver?

> +
> +What:		/sys/devices/soc0/apps_image_crm
> +What:		/sys/devices/soc0/apps_image_variant
> +What:		/sys/devices/soc0/apps_image_version
> +Date:		January 2017
> +Contact:	linux-arm-msm@xxxxxxxxxxxxxxx
> +Description:
> +		These files respectively show the crm version, variant and
> +		version of the APPS(Linux kernel, rootfs) image.

Assuming that the kernel and rootfs are the same image and updated 
together?

> +
> +What:		/sys/devices/soc0/boot_image_crm
> +What:		/sys/devices/soc0/boot_image_variant
> +What:		/sys/devices/soc0/boot_image_version
> +Date:		January 2017
> +Contact:	linux-arm-msm@xxxxxxxxxxxxxxx
> +Description:
> +		These files respectively show the crm version, variant and
> +		version of the Boot(bootloader) image.
> +
> +What:		/sys/devices/soc0/build_id
> +Date:		January 2017
> +Contact:	linux-arm-msm@xxxxxxxxxxxxxxx
> +Description:
> +		This file shows the unique id of current build being used on
> +		the system.

Build of what? The kernel already has a build version.

> +
> +What:		/sys/devices/soc0/cnss_image_crm
> +What:		/sys/devices/soc0/cnss_image_variant
> +What:		/sys/devices/soc0/cnss_image_version
> +Date:		January 2017
> +Contact:	linux-arm-msm@xxxxxxxxxxxxxxx
> +Description:
> +		These files respectively show the crm version, variant and
> +		version of the CNSS image.
> +
> +What:		/sys/devices/soc0/family
> +Date:		January 2017
> +Contact:	linux-arm-msm@xxxxxxxxxxxxxxx
> +Description:
> +		This file shows the family(e.g Snapdragon) of the SoC.

Sounds like a standard attr.

> +
> +What:		/sys/devices/soc0/foundry_id
> +Date:		January 2017
> +Contact:	linux-arm-msm@xxxxxxxxxxxxxxx
> +Description:
> +		This file shows the id of the foundry, where soc was
> +		manufactured.

I don't see how userspace should care...

> +
> +What:		/sys/devices/soc0/hw_platform
> +Date:		January 2017
> +Contact:	linux-arm-msm@xxxxxxxxxxxxxxx
> +Description:
> +		This file shows the type of hardware platform
> +		(e.g MTP, QRD etc) where SoC is being used.

What's a platform?

> +
> +What:		/sys/devices/soc0/machine
> +Date:		January 2017
> +Contact:	linux-arm-msm@xxxxxxxxxxxxxxx
> +Description:
> +		This file shows the machine name as given in the DT.

This is already exposed.

> +
> +What:		/sys/devices/soc0/mpss_image_crm
> +What:		/sys/devices/soc0/mpss_image_variant
> +What:		/sys/devices/soc0/mpss_image_version
> +Date:		January 2017
> +Contact:	linux-arm-msm@xxxxxxxxxxxxxxx
> +Description:
> +		These files respectively show the crm version, variant and
> +		version of the MPSS image.

Part of the MPSS driver?

> +
> +What:		/sys/devices/soc0/platform_subtype
> +Date:		January 2017
> +Contact:	linux-arm-msm@xxxxxxxxxxxxxxx
> +Description:
> +		This file shows the sub-type of hardware platform
> +		(SKUAA, SKUF etc.) where SoC is being used.
> +
> +What:		/sys/devices/soc0/platform_version
> +Date:		January 2017
> +Contact:	linux-arm-msm@xxxxxxxxxxxxxxx
> +Description:
> +		This file show the version of the hardware platform.
> +
> +What:		/sys/devices/soc0/pmic_die_revision
> +Date:		January 2017
> +Contact:	linux-arm-msm@xxxxxxxxxxxxxxx
> +Description:
> +		This file shows revision of PMIC die.

Part of the PMIC driver?

> +
> +What:		/sys/devices/soc0/pmic_model
> +Date:		January 2017
> +Contact:	linux-arm-msm@xxxxxxxxxxxxxxx
> +Description:
> +		This file shows name of PMIC model.
> +
> +What:		/sys/devices/soc0/qcom_odm
> +Date:		January 2017
> +Contact:	linux-arm-msm@xxxxxxxxxxxxxxx
> +Description:
> +		This file shows the ODM using the SoC.

The vendor in the top-level compatible should provide this.

> +
> +What:		/sys/devices/soc0/raw_version
> +Date:		January 2017
> +Contact:	linux-arm-msm@xxxxxxxxxxxxxxx
> +Description:
> +		This file shows raw version of the SoC.
> +
> +What:		/sys/devices/soc0/revision
> +Date:		January 2017
> +Contact:	linux-arm-msm@xxxxxxxxxxxxxxx
> +Description:
> +		This file shows revision of the SoC.

Why do you need both?

> +
> +What:		/sys/devices/soc0/rpm_image_crm
> +What:		/sys/devices/soc0/rpm_image_variant
> +What:		/sys/devices/soc0/rpm_image_version
> +Date:		January 2017
> +Contact:	linux-arm-msm@xxxxxxxxxxxxxxx
> +Description:
> +		These files respectively show the crm version, variant and
> +		version of the RPM image.

RPM driver?

> +
> +What:		/sys/devices/soc0/serial_number
> +Date:		January 2017
> +Contact:	linux-arm-msm@xxxxxxxxxxxxxxx
> +Description:
> +		This file shows serial number of the SoC.

Already have a standard property in DT.

> +
> +What:		/sys/devices/soc0/soc_id
> +Date:		January 2017
> +Contact:	linux-arm-msm@xxxxxxxxxxxxxxx
> +Description:
> +		This file shows the unique numeric id of a Qualcomm SoC.

unique per chip or per SoC model?

> +
> +What:		/sys/devices/soc0/tz_image_crm
> +What:		/sys/devices/soc0/tz_image_variant
> +What:		/sys/devices/soc0/tz_image_version
> +Date:		January 2017
> +Contact:	linux-arm-msm@xxxxxxxxxxxxxxx
> +Description:
> +		These files respectively show the crm version, variant and
> +		version of the TZ image.

TZ driver?

> +
> +What:		/sys/devices/soc0/vendor
> +Date:		January 2017
> +Contact:	linux-arm-msm@xxxxxxxxxxxxxxx
> +Description:
> +		This file shows the vendor of the SoC.

Already in DT.

> +
> +What:		/sys/devices/soc0/video_image_crm
> +What:		/sys/devices/soc0/video_image_variant
> +What:		/sys/devices/soc0/video_image_version
> +Date:		January 2017
> +Contact:	linux-arm-msm@xxxxxxxxxxxxxxx
> +Description:
> +		These files respectively show the crm version, variant and
> +		version of the video image.

Video as in display or video codec? Should be part of the driver.

Rob