From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754388AbdDROXh (ORCPT ); Tue, 18 Apr 2017 10:23:37 -0400 Received: from smtp.codeaurora.org ([198.145.29.96]:49224 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752847AbdDROXd (ORCPT ); Tue, 18 Apr 2017 10:23:33 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org 45EC1607BB Authentication-Results: pdx-caf-mail.web.codeaurora.org; dmarc=none (p=none dis=none) header.from=codeaurora.org Authentication-Results: pdx-caf-mail.web.codeaurora.org; spf=none smtp.mailfrom=kimran@codeaurora.org Subject: Re: [v10, 2/2] Documentation/ABI: Add ABI information for QCOM socinfo driver From: Imran Khan To: Rob Herring Cc: bjorn.andersson@linaro.org, sboyd@codeaurora.org, agross@codeaurora.org, linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org, linux-soc@vger.kernel.org References: <1487609235-19458-3-git-send-email-kimran@codeaurora.org> <20170222140448.qo677wamcd44tgkw@rob-hp-laptop> <0bf24858-33b2-8d08-ddc3-df83a2bd696d@codeaurora.org> Message-ID: <4b85e85b-8c2a-a226-d3b2-67fde0820ada@codeaurora.org> Date: Tue, 18 Apr 2017 19:53:27 +0530 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.0 MIME-Version: 1.0 In-Reply-To: <0bf24858-33b2-8d08-ddc3-df83a2bd696d@codeaurora.org> Content-Type: text/plain; charset=windows-1252 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 >>> --- >>> .../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@vger.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@vger.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@vger.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@vger.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@vger.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@vger.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@vger.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@vger.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@vger.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@vger.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@vger.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@vger.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@vger.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@vger.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@vger.kernel.org >>> +Description: >>> + This file shows name of PMIC model. >>> + >>> +What: /sys/devices/soc0/qcom_odm >>> +Date: January 2017 >>> +Contact: linux-arm-msm@vger.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@vger.kernel.org >>> +Description: >>> + This file shows raw version of the SoC. >>> + >>> +What: /sys/devices/soc0/revision >>> +Date: January 2017 >>> +Contact: linux-arm-msm@vger.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@vger.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@vger.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@vger.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@vger.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@vger.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@vger.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@vger.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