From mboxrd@z Thu Jan 1 00:00:00 1970 From: Rajendra Nayak Subject: Re: [PATCH v2 1/6] soc: qcom: rpmpd: Add a powerdomain driver to model corners Date: Thu, 31 May 2018 09:50:55 +0530 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 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org To: Ulf Hansson 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 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? Most other powerdomain/clock/regulator drivers I see do it statically, and thats what I followed. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation