Re: [PATCH 1/3] soc: qcom: smem: Support global partition

[Bjorn Andersson <bjorn.andersson@linaro.org>]

On Tue 22 Aug 17:28 PDT 2017, Chris Lew wrote:
> On 8/21/2017 10:17 AM, Bjorn Andersson wrote:
[..]
> > > +static int qcom_smem_set_global_partition(struct qcom_smem *smem,
> > > +				struct smem_ptable_entry *entry)
> > > +{
[..]
> > > +	if (header->size != entry->size) {
> > 
> > This happens to work, because they are both in the same endian. But
> > please sprinkle some le32_to_cpu() here as well.
> > 
> 
> 
> These checks mimic the sanity checks being done in enumerate_partitions.
> Should I create a patch to increase le32_to_cpu usage in
> qcom_smem_enumerate_partitions?
> 

Oops, yeah please do, just for completeness sake.

[..]
> > > @@ -647,6 +714,12 @@ static int qcom_smem_enumerate_partitions(struct qcom_smem *smem,
> > >   		host0 = le16_to_cpu(entry->host0);
> > >   		host1 = le16_to_cpu(entry->host1);
> > > +		if (host0 == SMEM_GLOBAL_HOST && host0 == host1) {
> > > +			if (qcom_smem_set_global_partition(smem, entry))
> > > +				return -EINVAL;
> > > +			continue;
> > > +		}
> > > +
> > 
> > As you're not able to leverage any of the checks from this loop I think
> > it's cleaner to duplicate the traversal of the partition table in both
> > functions and call the "search for global partition" directly from
> > probe - if the version indicates there should be one.
> > 
> 
> 
> Ok, will set the global partition in the version case statement and error
> out of the probe if finding the global partition fails since it is not
> optional in the new version.
> 

Sounds good.

Regards,
Bjorn