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 <[email protected]>
>> ---
>> .../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.
>
>> 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: [email protected]
>> +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: [email protected]
>> +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.
>> +
>> +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: [email protected]
>> +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: [email protected]
>> +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: [email protected]
>> +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: [email protected]
>> +Description:
>> + These files respectively show the crm version, variant and
>> + version of the CNSS image.
>> +
>> +What: /sys/devices/soc0/family
>> +Date: January 2017
>> +Contact: [email protected]
>> +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: [email protected]
>> +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.
>> +
>> +What: /sys/devices/soc0/hw_platform
>> +Date: January 2017
>> +Contact: [email protected]
>> +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: [email protected]
>> +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: [email protected]
>> +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: [email protected]
>> +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: [email protected]
>> +Description:
>> + This file show the version of the hardware platform.
>> +
>> +What: /sys/devices/soc0/pmic_die_revision
>> +Date: January 2017
>> +Contact: [email protected]
>> +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: [email protected]
>> +Description:
>> + This file shows name of PMIC model.
>> +
>> +What: /sys/devices/soc0/qcom_odm
>> +Date: January 2017
>> +Contact: [email protected]
>> +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: [email protected]
>> +Description:
>> + This file shows raw version of the SoC.
>> +
>> +What: /sys/devices/soc0/revision
>> +Date: January 2017
>> +Contact: [email protected]
>> +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: [email protected]
>> +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: [email protected]
>> +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: [email protected]
>> +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: [email protected]
>> +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: [email protected]
>> +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: [email protected]
>> +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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a\nmember of the Code Aurora Forum, hosted by The Linux Foundation
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 <[email protected]>
>>> ---
>>> .../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: [email protected]
>>> +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: [email protected]
>>> +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: [email protected]
>>> +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: [email protected]
>>> +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: [email protected]
>>> +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: [email protected]
>>> +Description:
>>> + These files respectively show the crm version, variant and
>>> + version of the CNSS image.
>>> +
>>> +What: /sys/devices/soc0/family
>>> +Date: January 2017
>>> +Contact: [email protected]
>>> +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: [email protected]
>>> +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: [email protected]
>>> +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: [email protected]
>>> +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: [email protected]
>>> +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: [email protected]
>>> +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: [email protected]
>>> +Description:
>>> + This file show the version of the hardware platform.
>>> +
>>> +What: /sys/devices/soc0/pmic_die_revision
>>> +Date: January 2017
>>> +Contact: [email protected]
>>> +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: [email protected]
>>> +Description:
>>> + This file shows name of PMIC model.
>>> +
>>> +What: /sys/devices/soc0/qcom_odm
>>> +Date: January 2017
>>> +Contact: [email protected]
>>> +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: [email protected]
>>> +Description:
>>> + This file shows raw version of the SoC.
>>> +
>>> +What: /sys/devices/soc0/revision
>>> +Date: January 2017
>>> +Contact: [email protected]
>>> +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: [email protected]
>>> +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: [email protected]
>>> +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: [email protected]
>>> +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: [email protected]
>>> +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: [email protected]
>>> +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: [email protected]
>>> +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 [email protected]
>> 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
On 4/18/2017 7:53 PM, Imran Khan wrote:
Hi Rob,
Could you please provide some feedback so that this discussion can move forward
and ABI document can be finalized?
Without the ABI document we are not able to get the corresponding driver
finalized and get merged.
Thanks again for your time,
Imran
> 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 <[email protected]>
>>>> ---
>>>> .../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: [email protected]
>>>> +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: [email protected]
>>>> +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: [email protected]
>>>> +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: [email protected]
>>>> +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: [email protected]
>>>> +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: [email protected]
>>>> +Description:
>>>> + These files respectively show the crm version, variant and
>>>> + version of the CNSS image.
>>>> +
>>>> +What: /sys/devices/soc0/family
>>>> +Date: January 2017
>>>> +Contact: [email protected]
>>>> +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: [email protected]
>>>> +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: [email protected]
>>>> +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: [email protected]
>>>> +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: [email protected]
>>>> +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: [email protected]
>>>> +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: [email protected]
>>>> +Description:
>>>> + This file show the version of the hardware platform.
>>>> +
>>>> +What: /sys/devices/soc0/pmic_die_revision
>>>> +Date: January 2017
>>>> +Contact: [email protected]
>>>> +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: [email protected]
>>>> +Description:
>>>> + This file shows name of PMIC model.
>>>> +
>>>> +What: /sys/devices/soc0/qcom_odm
>>>> +Date: January 2017
>>>> +Contact: [email protected]
>>>> +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: [email protected]
>>>> +Description:
>>>> + This file shows raw version of the SoC.
>>>> +
>>>> +What: /sys/devices/soc0/revision
>>>> +Date: January 2017
>>>> +Contact: [email protected]
>>>> +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: [email protected]
>>>> +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: [email protected]
>>>> +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: [email protected]
>>>> +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: [email protected]
>>>> +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: [email protected]
>>>> +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: [email protected]
>>>> +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 [email protected]
>>> 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
On Mon, Apr 24, 2017 at 11:00 AM, Imran Khan <[email protected]> wrote:
> On 4/18/2017 7:53 PM, Imran Khan wrote:
> Hi Rob,
> Could you please provide some feedback so that this discussion can move forward
> and ABI document can be finalized?
> Without the ABI document we are not able to get the corresponding driver
> finalized and get merged.
>
> Thanks again for your time,
> Imran
>> 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 <[email protected]>
>>>>> ---
>>>>> .../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?
Splitting things between common and private seems like a good direction.
>>>>> 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: [email protected]
>>>>> +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: [email protected]
>>>>> +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?
I don't think parsing things once will save you much.
>>
>>>>> +
>>>>> +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: [email protected]
>>>>> +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.
Maybe for you, but generally those are separate pieces.
>>>
>>>>> +
>>>>> +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: [email protected]
>>>>> +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: [email protected]
>>>>> +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.
That's assuming everything is built together which generally isn't
true. It doesn't seem like that is information the kernel should
provide.
>>>
>>>>> +
>>>>> +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: [email protected]
>>>>> +Description:
>>>>> + These files respectively show the crm version, variant and
>>>>> + version of the CNSS image.
>>>>> +
>>>>> +What: /sys/devices/soc0/family
>>>>> +Date: January 2017
>>>>> +Contact: [email protected]
>>>>> +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: [email protected]
>>>>> +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?
The qcom compatible string format already provides this. I don't think
we need 2 ABIs that are both vendor specific to expose this. Now if
there's other vendors wanting to expose the foundry, then a common
attr would make sense.
compatible = "qcom,<SoC>[-<soc_version>][-<foundry_id>]-<board>[/<subtype>][-<board_version>]"
>>
>>>>> +
>>>>> +What: /sys/devices/soc0/hw_platform
>>>>> +Date: January 2017
>>>>> +Contact: [email protected]
>>>>> +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.
As above, I believe /compatible already provides that information.
Rob
On Mon 24 Apr 09:27 PDT 2017, Rob Herring wrote:
> On Mon, Apr 24, 2017 at 11:00 AM, Imran Khan <[email protected]> wrote:
> > On 4/18/2017 7:53 PM, Imran Khan wrote:
> > Hi Rob,
> > Could you please provide some feedback so that this discussion can move forward
> > and ABI document can be finalized?
> > Without the ABI document we are not able to get the corresponding driver
> > finalized and get merged.
> >
> > Thanks again for your time,
> > Imran
> >> 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 <[email protected]>
> >>>>> ---
> >>>>> .../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?
>
> Splitting things between common and private seems like a good direction.
>
But the pattern of amending the common attributes with vendor or
platform-specific ones is how its done in other drivers, e.g.
integrator.
Why should we for the Qualcomm case make up some other pattern?
Or am I misunderstanding your suggestion?
> >>>>> 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: [email protected]
> >>>>> +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: [email protected]
> >>>>> +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?
>
> I don't think parsing things once will save you much.
>
Defining the struct in some shared header file and implementing the
"parsing" throughout various drivers would probably work out fine.
What I do not fancy is where these "parsers" would be implemented and
where the information would end up in sysfs.
"The ADSP driver" has to refer to the remoteproc driver for the adsp and
it would be reasonable to have version information of the firmware
available for a running piece of remoteproc. The same would work out for
modem and wireless.
The v4l driver is responsible for controlling the venus core, so it
could provide the version attribute - which would show up somewhere in
the /sys/bus/platform hierarchy.
We have a mfd-like driver for communicating with the RPM, so perhaps we
could shoehorn in an attribute there. But the sysfs path will be
completely different, depending on platform and system configuration.
There is no driver representing the boot code.
So, when Android goes belly up and we want to produce a bugreport that
describes all the pieces of the system we will have to all over sysfs to
figure out the versions of the firmware...
> >>
> >>>>> +
> >>>>> +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: [email protected]
> >>>>> +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.
>
> Maybe for you, but generally those are separate pieces.
>
This attribute is special in that it is populated from user space, so
any tool running on apps should be able to fetch it directly anyways
(e.g. from Android properties).
Imran, is there any users of this information outside the apps context?
E.g. does your tools for analysing memory-dumps depend on this
information being available in SMEM?
> >>>
> >>>>> +
> >>>>> +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: [email protected]
> >>>>> +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: [email protected]
> >>>>> +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.
>
> That's assuming everything is built together which generally isn't
> true.
Not necessarily compiled together, but packaged up in something that
gets a "release number" - which I presume is quite common for any type
of embedded products.
E.g. we do give the Linaro releases version numbers such as "16.09".
> It doesn't seem like that is information the kernel should
> provide.
>
I used to say that we could just store this information in a file in
/etc (or in build.prop), then I started doing post-build composition and
realized that you really do want a new system-version if you change e.g.
the version of the boot firmware - without having to regenerate the
rootfs.
So while I agree that this is none of the kernel's business I'm not sure
how you would get this information from SMEM to a user space process for
e.g. bugreport generation purposes.
> >>>
> >>>>> +
> >>>>> +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: [email protected]
> >>>>> +Description:
> >>>>> + These files respectively show the crm version, variant and
> >>>>> + version of the CNSS image.
> >>>>> +
> >>>>> +What: /sys/devices/soc0/family
> >>>>> +Date: January 2017
> >>>>> +Contact: [email protected]
> >>>>> +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: [email protected]
> >>>>> +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?
>
> The qcom compatible string format already provides this. I don't think
> we need 2 ABIs that are both vendor specific to expose this. Now if
> there's other vendors wanting to expose the foundry, then a common
> attr would make sense.
>
> compatible = "qcom,<SoC>[-<soc_version>][-<foundry_id>]-<board>[/<subtype>][-<board_version>]"
>
For userspace to be able to parse out this information from
/proc/device-tree/compatible we need to improve that expression quite a
bit! What's the foundry_id of <"acme,phone-a","qcom,platform-b-c">?
> >>
> >>>>> +
> >>>>> +What: /sys/devices/soc0/hw_platform
> >>>>> +Date: January 2017
> >>>>> +Contact: [email protected]
> >>>>> +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.
>
> As above, I believe /compatible already provides that information.
>
I believe this is what OMAP calls "type", in a custom attribute on the
side of the common ones.
But as far as I can see this information should be trivial to derive
from the compatible of the board. Imran, what is this data used for?
Regards,
Bjorn
On 4/25/2017 4:35 AM, Bjorn Andersson wrote:
> On Mon 24 Apr 09:27 PDT 2017, Rob Herring wrote:
>
>> On Mon, Apr 24, 2017 at 11:00 AM, Imran Khan <[email protected]> wrote:
>>> On 4/18/2017 7:53 PM, Imran Khan wrote:
>>> Hi Rob,
>>> Could you please provide some feedback so that this discussion can move forward
>>> and ABI document can be finalized?
>>> Without the ABI document we are not able to get the corresponding driver
>>> finalized and get merged.
>>>
>>> Thanks again for your time,
>>> Imran
>>>> 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 <[email protected]>
>>>>>>> ---
>>>>>>> .../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?
>>
>> Splitting things between common and private seems like a good direction.
>>
>
> But the pattern of amending the common attributes with vendor or
> platform-specific ones is how its done in other drivers, e.g.
> integrator.
>
> Why should we for the Qualcomm case make up some other pattern?
>
> Or am I misunderstanding your suggestion?
>
>>>>>>> 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: [email protected]
>>>>>>> +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: [email protected]
>>>>>>> +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?
>>
>> I don't think parsing things once will save you much.
>>
>
> Defining the struct in some shared header file and implementing the
> "parsing" throughout various drivers would probably work out fine.
>
> What I do not fancy is where these "parsers" would be implemented and
> where the information would end up in sysfs.
>
> "The ADSP driver" has to refer to the remoteproc driver for the adsp and
> it would be reasonable to have version information of the firmware
> available for a running piece of remoteproc. The same would work out for
> modem and wireless.
>
> The v4l driver is responsible for controlling the venus core, so it
> could provide the version attribute - which would show up somewhere in
> the /sys/bus/platform hierarchy.
>
> We have a mfd-like driver for communicating with the RPM, so perhaps we
> could shoehorn in an attribute there. But the sysfs path will be
> completely different, depending on platform and system configuration.
>
> There is no driver representing the boot code.
>
>
> So, when Android goes belly up and we want to produce a bugreport that
> describes all the pieces of the system we will have to all over sysfs to
> figure out the versions of the firmware...
>
>>>>
>>>>>>> +
>>>>>>> +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: [email protected]
>>>>>>> +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.
>>
>> Maybe for you, but generally those are separate pieces.
>>
>
> This attribute is special in that it is populated from user space, so
> any tool running on apps should be able to fetch it directly anyways
> (e.g. from Android properties).
>
> Imran, is there any users of this information outside the apps context?
> E.g. does your tools for analysing memory-dumps depend on this
> information being available in SMEM?
>
Yes, apart from apps our memory dump analyzers and diag tools make use of this
information
>>>>>
>>>>>>> +
>>>>>>> +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: [email protected]
>>>>>>> +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: [email protected]
>>>>>>> +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.
>>
>> That's assuming everything is built together which generally isn't
>> true.
>
> Not necessarily compiled together, but packaged up in something that
> gets a "release number" - which I presume is quite common for any type
> of embedded products.
>
> E.g. we do give the Linaro releases version numbers such as "16.09".
>
>> It doesn't seem like that is information the kernel should
>> provide.
>>
>
> I used to say that we could just store this information in a file in
> /etc (or in build.prop), then I started doing post-build composition and
> realized that you really do want a new system-version if you change e.g.
> the version of the boot firmware - without having to regenerate the
> rootfs.
>
> So while I agree that this is none of the kernel's business I'm not sure
> how you would get this information from SMEM to a user space process for
> e.g. bugreport generation purposes.
>
>>>>>
>>>>>>> +
>>>>>>> +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: [email protected]
>>>>>>> +Description:
>>>>>>> + These files respectively show the crm version, variant and
>>>>>>> + version of the CNSS image.
>>>>>>> +
>>>>>>> +What: /sys/devices/soc0/family
>>>>>>> +Date: January 2017
>>>>>>> +Contact: [email protected]
>>>>>>> +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: [email protected]
>>>>>>> +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?
>>
>> The qcom compatible string format already provides this. I don't think
>> we need 2 ABIs that are both vendor specific to expose this. Now if
>> there's other vendors wanting to expose the foundry, then a common
>> attr would make sense.
>>
>> compatible = "qcom,<SoC>[-<soc_version>][-<foundry_id>]-<board>[/<subtype>][-<board_version>]"
>>
>
> For userspace to be able to parse out this information from
> /proc/device-tree/compatible we need to improve that expression quite a
> bit! What's the foundry_id of <"acme,phone-a","qcom,platform-b-c">?
>
>>>>
>>>>>>> +
>>>>>>> +What: /sys/devices/soc0/hw_platform
>>>>>>> +Date: January 2017
>>>>>>> +Contact: [email protected]
>>>>>>> +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.
>>
>> As above, I believe /compatible already provides that information.
>>
>
> I believe this is what OMAP calls "type", in a custom attribute on the
> side of the common ones.
>
> But as far as I can see this information should be trivial to derive
> from the compatible of the board. Imran, what is this data used for?
>
This data is used in several cases. For example MTP and QRD boards may use
different thermal calibration, different firmwares, different wcnss settings.
These are the some use cases, I can recall right now, there may be others too.
> Regards,
> Bjorn
>
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a\nmember of the Code Aurora Forum, hosted by The Linux Foundation
On Mon, Apr 24, 2017 at 6:05 PM, Bjorn Andersson
<[email protected]> wrote:
> On Mon 24 Apr 09:27 PDT 2017, Rob Herring wrote:
>
>> On Mon, Apr 24, 2017 at 11:00 AM, Imran Khan <[email protected]> wrote:
>> > On 4/18/2017 7:53 PM, Imran Khan wrote:
>> > Hi Rob,
>> > Could you please provide some feedback so that this discussion can move forward
>> > and ABI document can be finalized?
>> > Without the ABI document we are not able to get the corresponding driver
>> > finalized and get merged.
>> >
>> > Thanks again for your time,
>> > Imran
>> >> 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 <[email protected]>
>> >>>>> ---
>> >>>>> .../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?
>>
>> Splitting things between common and private seems like a good direction.
>>
>
> But the pattern of amending the common attributes with vendor or
> platform-specific ones is how its done in other drivers, e.g.
> integrator.
>
> Why should we for the Qualcomm case make up some other pattern?
>
> Or am I misunderstanding your suggestion?
I'm just trying to ensure that we make things that could be common,
common. The question here was about the implementation.
Right now, drivers/soc is just a free for all dumping ground IMO.
>> >>>>> 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: [email protected]
>> >>>>> +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: [email protected]
>> >>>>> +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?
>>
>> I don't think parsing things once will save you much.
>>
>
> Defining the struct in some shared header file and implementing the
> "parsing" throughout various drivers would probably work out fine.
>
> What I do not fancy is where these "parsers" would be implemented and
> where the information would end up in sysfs.
>
> "The ADSP driver" has to refer to the remoteproc driver for the adsp and
> it would be reasonable to have version information of the firmware
> available for a running piece of remoteproc. The same would work out for
> modem and wireless.
>
> The v4l driver is responsible for controlling the venus core, so it
> could provide the version attribute - which would show up somewhere in
> the /sys/bus/platform hierarchy.
>
> We have a mfd-like driver for communicating with the RPM, so perhaps we
> could shoehorn in an attribute there. But the sysfs path will be
> completely different, depending on platform and system configuration.
>
> There is no driver representing the boot code.
>
>
> So, when Android goes belly up and we want to produce a bugreport that
> describes all the pieces of the system we will have to all over sysfs to
> figure out the versions of the firmware...
Can't you just use the meta build id? That implies the version of
*everything*, right?
>> >>
>> >>>>> +
>> >>>>> +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: [email protected]
>> >>>>> +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.
>>
>> Maybe for you, but generally those are separate pieces.
>>
>
> This attribute is special in that it is populated from user space, so
> any tool running on apps should be able to fetch it directly anyways
> (e.g. from Android properties).
>
> Imran, is there any users of this information outside the apps context?
> E.g. does your tools for analysing memory-dumps depend on this
> information being available in SMEM?
>
>> >>>
>> >>>>> +
>> >>>>> +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: [email protected]
>> >>>>> +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: [email protected]
>> >>>>> +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.
>>
>> That's assuming everything is built together which generally isn't
>> true.
>
> Not necessarily compiled together, but packaged up in something that
> gets a "release number" - which I presume is quite common for any type
> of embedded products.
>
> E.g. we do give the Linaro releases version numbers such as "16.09".
Yes, but I doubt the release number is visible inside the release
unless it is part of some existing version like the kernel's version
string. If this is quite common, then lets make it common.
Why not just make this a file in the filesystem? Why does the kernel
need to provide it?
>> It doesn't seem like that is information the kernel should
>> provide.
>>
>
> I used to say that we could just store this information in a file in
> /etc (or in build.prop), then I started doing post-build composition and
> realized that you really do want a new system-version if you change e.g.
> the version of the boot firmware - without having to regenerate the
> rootfs.
>
> So while I agree that this is none of the kernel's business I'm not sure
> how you would get this information from SMEM to a user space process for
> e.g. bugreport generation purposes.
>
>> >>>
>> >>>>> +
>> >>>>> +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: [email protected]
>> >>>>> +Description:
>> >>>>> + These files respectively show the crm version, variant and
>> >>>>> + version of the CNSS image.
>> >>>>> +
>> >>>>> +What: /sys/devices/soc0/family
>> >>>>> +Date: January 2017
>> >>>>> +Contact: [email protected]
>> >>>>> +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: [email protected]
>> >>>>> +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?
>>
>> The qcom compatible string format already provides this. I don't think
>> we need 2 ABIs that are both vendor specific to expose this. Now if
>> there's other vendors wanting to expose the foundry, then a common
>> attr would make sense.
>>
>> compatible = "qcom,<SoC>[-<soc_version>][-<foundry_id>]-<board>[/<subtype>][-<board_version>]"
>>
>
> For userspace to be able to parse out this information from
> /proc/device-tree/compatible we need to improve that expression quite a
> bit! What's the foundry_id of <"acme,phone-a","qcom,platform-b-c">?
I can't help it if QCom defines an elaborate compatible string format
and then doesn't use it.
>> >>>>> +
>> >>>>> +What: /sys/devices/soc0/hw_platform
>> >>>>> +Date: January 2017
>> >>>>> +Contact: [email protected]
>> >>>>> +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.
>>
>> As above, I believe /compatible already provides that information.
>>
>
> I believe this is what OMAP calls "type", in a custom attribute on the
> side of the common ones.
>
> But as far as I can see this information should be trivial to derive
> from the compatible of the board. Imran, what is this data used for?
>
> Regards,
> Bjorn
On Tue 25 Apr 10:13 PDT 2017, Rob Herring wrote:
> On Mon, Apr 24, 2017 at 6:05 PM, Bjorn Andersson
> <[email protected]> wrote:
> > On Mon 24 Apr 09:27 PDT 2017, Rob Herring wrote:
[..]
> >> Splitting things between common and private seems like a good direction.
> >>
> >
> > But the pattern of amending the common attributes with vendor or
> > platform-specific ones is how its done in other drivers, e.g.
> > integrator.
> >
> > Why should we for the Qualcomm case make up some other pattern?
> >
> > Or am I misunderstanding your suggestion?
>
> I'm just trying to ensure that we make things that could be common,
> common. The question here was about the implementation.
>
> Right now, drivers/soc is just a free for all dumping ground IMO.
>
Unfortunately I fully agree with this, we did spend several revisions on
just trying to match Qualcomm properties to the common attributes - and
I don't know if we got it right.
> >> >>>>> 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: [email protected]
> >> >>>>> +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: [email protected]
> >> >>>>> +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?
> >>
> >> I don't think parsing things once will save you much.
> >>
> >
> > Defining the struct in some shared header file and implementing the
> > "parsing" throughout various drivers would probably work out fine.
> >
> > What I do not fancy is where these "parsers" would be implemented and
> > where the information would end up in sysfs.
> >
> > "The ADSP driver" has to refer to the remoteproc driver for the adsp and
> > it would be reasonable to have version information of the firmware
> > available for a running piece of remoteproc. The same would work out for
> > modem and wireless.
> >
> > The v4l driver is responsible for controlling the venus core, so it
> > could provide the version attribute - which would show up somewhere in
> > the /sys/bus/platform hierarchy.
> >
> > We have a mfd-like driver for communicating with the RPM, so perhaps we
> > could shoehorn in an attribute there. But the sysfs path will be
> > completely different, depending on platform and system configuration.
> >
> > There is no driver representing the boot code.
> >
> >
> > So, when Android goes belly up and we want to produce a bugreport that
> > describes all the pieces of the system we will have to all over sysfs to
> > figure out the versions of the firmware...
>
> Can't you just use the meta build id? That implies the version of
> *everything*, right?
>
For products yes, but when _you_ ask us why WiFi doesn't work on your
Dragonboard it's quite nice to be able to get hold of the boot-loader
and wcnss-firmware versions - and you're likely not on an unmodified
meta-build.
For me this information is more valuable than the meta-build id, but
that's because I'm working with engineering builds.
[..]
> >> >>>>> +What: /sys/devices/soc0/build_id
> >> >>>>> +Date: January 2017
> >> >>>>> +Contact: [email protected]
> >> >>>>> +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.
> >>
> >> That's assuming everything is built together which generally isn't
> >> true.
> >
> > Not necessarily compiled together, but packaged up in something that
> > gets a "release number" - which I presume is quite common for any type
> > of embedded products.
> >
> > E.g. we do give the Linaro releases version numbers such as "16.09".
>
> Yes, but I doubt the release number is visible inside the release
> unless it is part of some existing version like the kernel's version
> string. If this is quite common, then lets make it common.
>
I don't know how common this is - as you suggest below perhaps people
just put it in one of the file systems?
> Why not just make this a file in the filesystem?
Because that forces you to rebuild your file system to update the
version of the system. That said I don't know the details of how
Qualcomm persistently encode this information.
> Why does the kernel need to provide it?
>
Imran, can you elaborate on how this information is travels from the
build system to the SMEM item?
Regards,
Bjorn