On Tue 25 Apr 10:13 PDT 2017, Rob Herring wrote: > On Mon, Apr 24, 2017 at 6:05 PM, Bjorn Andersson > <bjorn.andersson@xxxxxxxxxx> 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: 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... > > 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: 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". > > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
