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 <kimran@xxxxxxxxxxxxxx> 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 <kimran@xxxxxxxxxxxxxx> >>>>>>> --- >>>>>>> .../ABI/testing/sysfs-driver-qcom_socinfo | 171 +++++++++++++++++++++ >>>>>>> 1 file changed, 171 insertions(+) >>>>>>> create mode 100644 Documentation/ABI/testing/sysfs-driver-qcom_socinfo >>>>>> >>>>>> Sorry to comment late on this (blame LWM), but I think creating this ABI >>>>>> is a mistake. The biggest issue I have is this doesn't scale if every >>>>>> SoC does its own thing. We should have a common interface so for example >>>>>> userspace can retrieve the serial number from any SoC in the same way. >>>>>> Yes, we can have custom attributes, but there should be common base. >>>>>> >>>>> >>>>> 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: linux-arm-msm@xxxxxxxxxxxxxxx >>>>>>> +Description: >>>>>>> + This file shows the id of the accessory chip. >>>>>>> + >>>>>>> +What: /sys/devices/soc0/adsp_image_crm >>>>>>> +What: /sys/devices/soc0/adsp_image_variant >>>>>>> +What: /sys/devices/soc0/adsp_image_version >>>>>>> +Date: January 2017 >>>>>>> +Contact: linux-arm-msm@xxxxxxxxxxxxxxx >>>>>>> +Description: >>>>>>> + These files respectively show the crm version, variant and >>>>>>> + version of the ADSP image. >>>>>> >>>>>> Shouldn't this be part of the ADSP driver? >>>>>> >>>>> 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: linux-arm-msm@xxxxxxxxxxxxxxx >>>>>>> +Description: >>>>>>> + These files respectively show the crm version, variant and >>>>>>> + version of the APPS(Linux kernel, rootfs) image. >>>>>> >>>>>> Assuming that the kernel and rootfs are the same image and updated >>>>>> together? >>>>>> >>>>> >>>>> 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: linux-arm-msm@xxxxxxxxxxxxxxx >>>>>>> +Description: >>>>>>> + These files respectively show the crm version, variant and >>>>>>> + version of the Boot(bootloader) image. >>>>>>> + >>>>>>> +What: /sys/devices/soc0/build_id >>>>>>> +Date: January 2017 >>>>>>> +Contact: linux-arm-msm@xxxxxxxxxxxxxxx >>>>>>> +Description: >>>>>>> + This file shows the unique id of current build being used on >>>>>>> + the system. >>>>>> >>>>>> Build of what? The kernel already has a build version. >>>>>> >>>>> 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: linux-arm-msm@xxxxxxxxxxxxxxx >>>>>>> +Description: >>>>>>> + These files respectively show the crm version, variant and >>>>>>> + version of the CNSS image. >>>>>>> + >>>>>>> +What: /sys/devices/soc0/family >>>>>>> +Date: January 2017 >>>>>>> +Contact: linux-arm-msm@xxxxxxxxxxxxxxx >>>>>>> +Description: >>>>>>> + This file shows the family(e.g Snapdragon) of the SoC. >>>>>> >>>>>> Sounds like a standard attr. >>>>>> >>>>> Yeah. This is standard attribute. Will remove this from Documentation here. >>>>> >>>>>>> + >>>>>>> +What: /sys/devices/soc0/foundry_id >>>>>>> +Date: January 2017 >>>>>>> +Contact: linux-arm-msm@xxxxxxxxxxxxxxx >>>>>>> +Description: >>>>>>> + This file shows the id of the foundry, where soc was >>>>>>> + manufactured. >>>>>> >>>>>> I don't see how userspace should care... >>>>>> >>>>> 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: linux-arm-msm@xxxxxxxxxxxxxxx >>>>>>> +Description: >>>>>>> + This file shows the type of hardware platform >>>>>>> + (e.g MTP, QRD etc) where SoC is being used. >>>>>> >>>>>> What's a platform? >>>>>> >>>>> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
