[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4b85e85b-8c2a-a226-d3b2-67fde0820ada@codeaurora.org>
Date: Tue, 18 Apr 2017 19:53:27 +0530
From: Imran Khan <kimran@...eaurora.org>
To: Rob Herring <robh@...nel.org>
Cc: bjorn.andersson@...aro.org, sboyd@...eaurora.org,
agross@...eaurora.org, linux-arm-msm@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-soc@...r.kernel.org
Subject: Re: [v10, 2/2] Documentation/ABI: Add ABI information for QCOM
socinfo driver
Hi Rob,
On 3/6/2017 12:19 PM, Imran Khan wrote:
> On 2/22/2017 7:34 PM, Rob Herring wrote:
>> 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@...eaurora.org>
>>> ---
>>> .../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.
>>
>
> Yeah, I agree about the scalability part. Could you please suggest some way to
> implement a common base for the custom attributes. Like for serial number I think
> we can put it in generic soc_device_attribute but for custom attributes like accessory_chip,
> hw_platform etc., how can we implement a common base. Can we have a private pointer within
> generic soc_device_attribute structure and this private pointer can point to custom attributes.
> Or if you have some other suggestion to implement this common interface, please let me know.
Could you please provide some feedback regarding this?
>>
>>> 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@...r.kernel.org
>>> +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@...r.kernel.org
>>> +Description:
>>> + These files respectively show the crm version, variant and
>>> + version of the ADSP image.
>>
>> Shouldn't this be part of the ADSP driver?
>>
> Yes, It can be but I wanted to keep the image information at a central location,
> rather than pushing it back to each driver. For image information we basically
> read the same item from SMEM but use different offsets within it for different images,
> so the idea was to read this information ( get SMEM handler) just once, rather than
> doing it for each driver.
> But if this idea does not look correct, I can remove it from socinfo driver.
>
Could you please provide some feedback regarding this?
>>> +
>>> +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@...r.kernel.org
>>> +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?
>>
>
> Yes. The kernel and rootfs are same image and they are 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@...r.kernel.org
>>> +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@...r.kernel.org
>>> +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.
>>
> This is not build id of the kernel. This is build ID of complete meta image.
>
>>> +
>>> +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@...r.kernel.org
>>> +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@...r.kernel.org
>>> +Description:
>>> + This file shows the family(e.g Snapdragon) of the SoC.
>>
>> Sounds like a standard attr.
>>
> Yeah. This is standard attribute. Will remove this from Documentation here.
>
>>> +
>>> +What: /sys/devices/soc0/foundry_id
>>> +Date: January 2017
>>> +Contact: linux-arm-msm@...r.kernel.org
>>> +Description:
>>> + This file shows the id of the foundry, where soc was
>>> + manufactured.
>>
>> I don't see how userspace should care...
>>
> Yeah, usually user space would not care for such information. But sometimes we have
> come across h/w issues that were seen only on set of chips from a particular
> foundry. Under such situations we use this information to confirm if a certain h/w
> issue is specific to a batch from a particular foundry or not.
>
Could you please provide some feedback regarding this?
>>> +
>>> +What: /sys/devices/soc0/hw_platform
>>> +Date: January 2017
>>> +Contact: linux-arm-msm@...r.kernel.org
>>> +Description:
>>> + This file shows the type of hardware platform
>>> + (e.g MTP, QRD etc) where SoC is being used.
>>
>> What's a platform?
>>
> We may use same soc on different type of platforms. For example for QCOM we have
> MTP (board with which a debug board can be connected), QRD (no debug connection available).
> Similarly other ODMs may have different kind of platforms based on same soc.
> hw_paltform indicates numeric id for different kind of such platforms.
>
>>> +
>>> +What: /sys/devices/soc0/machine
>>> +Date: January 2017
>>> +Contact: linux-arm-msm@...r.kernel.org
>>> +Description:
>>> + This file shows the machine name as given in the DT.
>>
>> This is already exposed.
>>
> Yeah. Will remove it from this document.
>
>>> +
>>> +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@...r.kernel.org
>>> +Description:
>>> + These files respectively show the crm version, variant and
>>> + version of the MPSS image.
>>
>> Part of the MPSS driver?
>>
> Agree. Kindly see my comment above for the ADSP image(adsp_image_xxx).
>
>>> +
>>> +What: /sys/devices/soc0/platform_subtype
>>> +Date: January 2017
>>> +Contact: linux-arm-msm@...r.kernel.org
>>> +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@...r.kernel.org
>>> +Description:
>>> + This file show the version of the hardware platform.
>>> +
>>> +What: /sys/devices/soc0/pmic_die_revision
>>> +Date: January 2017
>>> +Contact: linux-arm-msm@...r.kernel.org
>>> +Description:
>>> + This file shows revision of PMIC die.
>>
>> Part of the PMIC driver?
>>
> Agree. Kindly see my comment above for the ADSP image(adsp_image_xxx).
>
>>> +
>>> +What: /sys/devices/soc0/pmic_model
>>> +Date: January 2017
>>> +Contact: linux-arm-msm@...r.kernel.org
>>> +Description:
>>> + This file shows name of PMIC model.
>>> +
>>> +What: /sys/devices/soc0/qcom_odm
>>> +Date: January 2017
>>> +Contact: linux-arm-msm@...r.kernel.org
>>> +Description:
>>> + This file shows the ODM using the SoC.
>>
>> The vendor in the top-level compatible should provide this.
>>
> Yeah. Have removed this in the latest version of driver. Will remove it
> from ABI document too.
>
>>> +
>>> +What: /sys/devices/soc0/raw_version
>>> +Date: January 2017
>>> +Contact: linux-arm-msm@...r.kernel.org
>>> +Description:
>>> + This file shows raw version of the SoC.
>>> +
>>> +What: /sys/devices/soc0/revision
>>> +Date: January 2017
>>> +Contact: linux-arm-msm@...r.kernel.org
>>> +Description:
>>> + This file shows revision of the SoC.
>>
>> Why do you need both?
>>
> Revision is version of the soc like 2.0, 3.0, 3.1 etc.
> and raw_version is is raw chip version which is an strictly increasing
> integer counter, increasing for each version of the chip.
> For example:
> Version Raw_version
> 1.0 0
> 1.1 1
> 2.0 2
> 2.1 3
> 2.2 4
> 3.0 5
>
>>> +
>>> +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@...r.kernel.org
>>> +Description:
>>> + These files respectively show the crm version, variant and
>>> + version of the RPM image.
>>
>> RPM driver?
>>
> Agree. Kindly see my comment above for the ADSP image(adsp_image_xxx).
>
>>> +
>>> +What: /sys/devices/soc0/serial_number
>>> +Date: January 2017
>>> +Contact: linux-arm-msm@...r.kernel.org
>>> +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@...r.kernel.org
>>> +Description:
>>> + This file shows the unique numeric id of a Qualcomm SoC.
>>
>> unique per chip or per SoC model?
>>
> This is unique per SoC model. For example 8996 and 8996pro
> would have different soc_id.
>
>>> +
>>> +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@...r.kernel.org
>>> +Description:
>>> + These files respectively show the crm version, variant and
>>> + version of the TZ image.
>>
>> TZ driver?
>>
> Agree. Kindly see my comment above for the ADSP image(adsp_image_xxx).
>
>>> +
>>> +What: /sys/devices/soc0/vendor
>>> +Date: January 2017
>>> +Contact: linux-arm-msm@...r.kernel.org
>>> +Description:
>>> + This file shows the vendor of the SoC.
>>
>> Already in DT.
>>
> Okay. Will remove this field from the driver and from the
> document.
>
>>> +
>>> +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@...r.kernel.org
>>> +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.
>>
> Agree. Kindly see my comment above for the ADSP image(adsp_image_xxx).
>> Rob
>
> Thanks for the review comments.
>
> Thanks and Regards,
> Imran
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-soc" in
>> the body of a message to majordomo@...r.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>
>
Thanks and Regards,
Imran
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a\nmember of the Code Aurora Forum, hosted by The Linux Foundation
Powered by blists - more mailing lists