Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752902AbdHWAca (ORCPT ); Tue, 22 Aug 2017 20:32:30 -0400 Received: from mail-pf0-f176.google.com ([209.85.192.176]:36731 "EHLO mail-pf0-f176.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752770AbdHWAc2 (ORCPT ); Tue, 22 Aug 2017 20:32:28 -0400 Date: Tue, 22 Aug 2017 17:32:25 -0700 From: Bjorn Andersson To: Chris Lew Cc: andy.gross@linaro.org, david.brown@linaro.org, aneela@codeaurora.org, linux-arm-msm@vger.kernel.org, linux-soc@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 1/3] soc: qcom: smem: Support global partition Message-ID: <20170823003225.GY29306@minitux> References: <1503018948-26629-1-git-send-email-clew@codeaurora.org> <1503018948-26629-2-git-send-email-clew@codeaurora.org> <20170821171733.GU29306@minitux> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.8.3 (2017-05-23) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1520 Lines: 48 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