On Tue 14 Feb 21:31 PST 2017, Imran Khan wrote: > On 2/15/2017 5:54 AM, Bjorn Andersson wrote: [..] > > > > I thought the conclusion was to drop the "hw_platform", > > "qrd_hw_platform_subtype" and hw_platform_subtype" lists, but to keep > > the cpu_of_id (although named soc_of_id). > > > > The hw_platform ids are re-used by ODMs and might have different > > meaning, but the SoC name is quite useful. So please put the soc_of_id > > back in here. > > > > cpu_of_id was mapping soc-id read from SMEM into SoC name, which in turn > was being used as machine name in soc_device_attribute. But we can also > read machine name from DT. Reading the machine name from DT would make the > solution more flexible as we don't have to make an entry in soc_of_id every > time we get a new SoC. I don't think we need a more flexible solution, as the purpose of this driver is to expose the socinfo information to user space; i.e. there's nothing flexible about that. If we just expose this information as a proprietary magic number, then it will be up to each consumer to keep some mapping table. So you will not get rid of the work of keeping things updated, you will just push the problem to somewhere else. > Also as soc_of_id uses soc-id as index, there is theoretically no limit on how > many elements it may end up having. There's a practical - and reasonable - limit on how much space this data will take, one could however argue that it's unwise to keep the sparse data in a simple array. A better solution would be to maintain an array of id,soc pairs and loop through that on lookup. > Because of the above reasons, I wanted to get rid off soc_of_id (or cpu_of_id) > array and use DT to get machine name as shown in the following snippet: > > + attr->family = "Snapdragon"; > + prop = fdt_get_property(initial_boot_params, 0, "model", NULL); > + if (prop) > + attr->machine = kasprintf(GFP_KERNEL, "%s", prop->data); The data is already in socinfo, we should not manually duplicate it in DT. > > Could you please let me know if this approach looks fine to you? > I would prefer that the socinfo doesn't expose magic numbers without a publicly available lookup table. And I don't see a need for the added flexibility. 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