On 1/9/26 2:36 PM, Mukesh Ojha wrote:
> On Thu, Jan 08, 2026 at 03:21:50PM +0100, Konrad Dybcio wrote:
>> From: Konrad Dybcio <[email protected]>
>>
>> Most modern Qualcomm platforms (>= SM8150) expose information about the
>> DDR memory present on the system via SMEM.
[...]
>> @@ -1236,17 +1241,24 @@ static int qcom_smem_probe(struct platform_device
>> *pdev)
>>
>> __smem = smem;
>>
>> + smem->debugfs_dir = smem_dram_parse(smem->dev);
>
> Is it possible, even after calling qcom_smem_is_available() before calling
> qcom_smem_dram_get_hbb() we are getting __dram as NULL.
>
> is it good to move __smem assignment to the end with barrier so all the
> changes before the assignment are seen when somebody checking
> qcom_smem_is_available()
> with a pair smp store/release pair.
I think just moving the __smem assignment down will be enough, no?
What scenario do you have in mind that would require SMP barriers?
[...]
>> +struct smem_dram {
>> + unsigned long frequencies[MAX_DDR_FREQ_NUM_V5];
>> + u32 num_frequencies;
>
> freq and num_freq_entries ? since you have used freq at various places..
The names in structs come from internal shmem definitions that I didn't
want to stray away from
Making the kernel-side struct fields named the same feels like added
confusion to me
[...]
>> + if (size == sizeof(struct ddr_details_v5)
>> + + 4 * sizeof(struct ddr_region_v5)
>> + + sizeof(struct ddr_xbl2quantum_smem_data)
>> + + sizeof(struct shub_freq_plan_entry))
>> + return INFO_V5;
>
> Why this does not have separate name ?
Because it's the same DDR info structure as "normal v5", with trailing
extras that we don't really care about
[...]
>> +struct dentry *smem_dram_parse(struct device *dev)
>> +{
>> + struct dentry *debugfs_dir;
>> + enum ddr_info_version ver;
>> + struct smem_dram *dram;
>> + size_t actual_size;
>> + void *data = NULL;
>> +
>> + /* No need to check qcom_smem_is_available(), this func is called by
>> the SMEM driver */
>
> This comment seems redundant..
With this one specifically, I don't agree it's obvious..
Konrad
