Re: [PATCH v2 4/4] soc: samsung: pm_domains: Provide real name for all supported domains

[Marek Szyprowski <m.szyprowski@samsung.com>]

Hi Krzysztof,

On 2017-01-28 16:16, Krzysztof Kozlowski wrote:
> On Fri, Jan 27, 2017 at 03:48:58PM +0100, Marek Szyprowski wrote:
>> Device tree nodes for each power domain should use generic "power-domain"
>> name, so using it as a domain name doesn't give much benefits. This patch
>> adds human readable names for all supported domains, what makes debugging
>> much easier.
>>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> ---
>> Changelog:
>> v2:
>> - sorted all domains data by domain base address in the arrays
>> ---
>>   drivers/soc/samsung/pm_domains.c | 125 ++++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 123 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/soc/samsung/pm_domains.c b/drivers/soc/samsung/pm_domains.c
>> index 5a0a46bcbe18..43003318b948 100644
>> --- a/drivers/soc/samsung/pm_domains.c
>> +++ b/drivers/soc/samsung/pm_domains.c
>> @@ -30,6 +30,17 @@ struct exynos_pm_domain_config {
>>   	u32 local_pwr_cfg;
>>   };
>>   
>> +struct exynos_pm_domain_data {
>> +	const char *name;
>> +	u32 base;
>> +};
>> +
>> +struct exynos_pm_domain_soc_data {
>> +	const char *compatible;
>> +	unsigned int nr_domains;
>> +	const struct exynos_pm_domain_data *domains;
>> +};
>> +
>>   /*
>>    * Exynos specific wrapper around the generic power domain
>>    */
>> @@ -123,6 +134,91 @@ static int exynos_pd_power_off(struct generic_pm_domain *domain)
>>   	return exynos_pd_power(domain, false);
>>   }
>>   
>> +static const struct exynos_pm_domain_data exynos4210_domains[] __initconst = {
>> +	{ "LCD1",	0x10023CA0 },
>> +};
>> +
>> +static const struct exynos_pm_domain_data exynos4412_domains[] __initconst = {
>> +	{ "CAM",	0x10023C00 },
>> +	{ "TV",		0x10023C20 },
>> +	{ "MFC",	0x10023C40 },
>> +	{ "G3D",	0x10023C60 },
>> +	{ "LCD0",	0x10023C80 },
>> +	{ "ISP",	0x10023CA0 },
>> +	{ "GPS",	0x10023CE0 },
>> +	{ "GPS alive",	0x10023D00 },
> That is a kind of duplication of DT and also spreading the description
> of hardware between DT and driver. I understand the purpose. Messages
> like:
> 	"power-domain has as child subdomain: power-domain."
> are useless. However I do not like putting any of these in the driver.
>
> How about adding a property "label" and parse it? Some other
> drivers/nodes are using labels already to have a meaningful name, either
> for user-space or for just user.
>
> Patches 1-3 look fine.

Okay, I will prepare a new patchset, which rely on 'label' property.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland