From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ulf Hansson Subject: Re: [PATCH v2 1/6] soc: qcom: rpmpd: Add a powerdomain driver to model corners Date: Thu, 31 May 2018 13:09:23 +0200 Message-ID: References: <20180525100121.28214-1-rnayak@codeaurora.org> <20180525100121.28214-2-rnayak@codeaurora.org> <0e07d577-9728-e97a-2da0-dd7dd324f058@codeaurora.org> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Return-path: In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org To: Rajendra Nayak Cc: Viresh Kumar , Stephen Boyd , Andy Gross , devicetree@vger.kernel.org, linux-arm-msm , Linux Kernel Mailing List , collinsd@codeaurora.org List-Id: devicetree@vger.kernel.org On 31 May 2018 at 06:20, Rajendra Nayak wrote: > > > On 05/30/2018 06:14 PM, Ulf Hansson wrote: >> [...] >> >>>>> + >>>>> +static DEFINE_MUTEX(rpmpd_lock); >>>>> + >>>>> +/* msm8996 RPM powerdomains */ >>>>> +DEFINE_RPMPD_CORN_SMPA(msm8996, vddcx, vddcx_ao, 1); >>>>> +DEFINE_RPMPD_CORN_SMPA(msm8996, vddmx, vddmx_ao, 2); >>>>> +DEFINE_RPMPD_CORN_LDOA(msm8996, vddsscx, 26); >>>>> + >>>>> +DEFINE_RPMPD_VFC_SMPA(msm8996, vddcx_vfc, 1); >>>>> +DEFINE_RPMPD_VFC_LDOA(msm8996, vddsscx_vfc, 26); >>>>> + >>>>> +static struct rpmpd *msm8996_rpmpds[] = { >>>>> + [0] = &msm8996_vddcx, >>>>> + [1] = &msm8996_vddcx_ao, >>>>> + [2] = &msm8996_vddcx_vfc, >>>>> + [3] = &msm8996_vddmx, >>>>> + [4] = &msm8996_vddmx_ao, >>>>> + [5] = &msm8996_vddsscx, >>>>> + [6] = &msm8996_vddsscx_vfc, >>>>> +}; >>>> >>>> It's not my call, but honestly the above all macros makes the code >>>> less readable. >>> >>> This is all static data per SoC. The macros will keep the new additions >>> needed for every new SoC to a minimal. Currently this supports only >>> msm8996. >> >> Right, that's fine then. >> >>> >>>> >>>> Anyway, I think you should convert to allocate these structs >>>> dynamically from the heap (kzalloc/kcalloc), instead of statically as >>>> above. >> >> However, I assume this is still doable!? > > Perhaps it is, but is there any specific advantage of constructing these structures > dynamically vs statically, given they are static data? Well, I was just thinking that the genpd struct has grown quite big. > Most other powerdomain/clock/regulator drivers I see do it statically, and thats > what I followed. Right, so forget it and keep it as is. Kind regards Uffe