All the device nodes for the Exynos5420 power-domains have a quite
generic "power-domain" name. So in case of an error, the Exynos PD
driver shows the following (not very useful) message:
"Power domain power-domain disable failed"
Use descriptive names to know on which PD enable or disable failed.
Signed-off-by: Javier Martinez Canillas <[email protected]>
---
arch/arm/boot/dts/exynos5420.dtsi | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
index b63b569ca4b4..4642cec50c0d 100644
--- a/arch/arm/boot/dts/exynos5420.dtsi
+++ b/arch/arm/boot/dts/exynos5420.dtsi
@@ -247,19 +247,19 @@
};
};
- gsc_pd: power-domain@10044000 {
+ gsc_pd: gsc-power-domain@10044000 {
compatible = "samsung,exynos4210-pd";
reg = <0x10044000 0x20>;
#power-domain-cells = <0>;
};
- isp_pd: power-domain@10044020 {
+ isp_pd: isp-power-domain@10044020 {
compatible = "samsung,exynos4210-pd";
reg = <0x10044020 0x20>;
#power-domain-cells = <0>;
};
- mfc_pd: power-domain@10044060 {
+ mfc_pd: mfc-power-domain@10044060 {
compatible = "samsung,exynos4210-pd";
reg = <0x10044060 0x20>;
clocks = <&clock CLK_FIN_PLL>, <&clock CLK_MOUT_SW_ACLK333>,
@@ -268,13 +268,13 @@
#power-domain-cells = <0>;
};
- msc_pd: power-domain@10044120 {
+ msc_pd: msc-power-domain@10044120 {
compatible = "samsung,exynos4210-pd";
reg = <0x10044120 0x20>;
#power-domain-cells = <0>;
};
- disp_pd: power-domain@100440C0 {
+ disp_pd: disp-power-domain@100440C0 {
compatible = "samsung,exynos4210-pd";
reg = <0x100440C0 0x20>;
#power-domain-cells = <0>;
--
2.1.3
Hello.
On 02/06/2015 08:37 PM, Javier Martinez Canillas wrote:
> All the device nodes for the Exynos5420 power-domains have a quite
> generic "power-domain" name.
And this is in conformance to the ePAPR standard.
> So in case of an error, the Exynos PD
> driver shows the following (not very useful) message:
> "Power domain power-domain disable failed"
Why not fix the message instead to use the full device name?
> Use descriptive names to know on which PD enable or disable failed.
> Signed-off-by: Javier Martinez Canillas <[email protected]>
WBR, Sergei
Hello Sergei,
Thanks a lot for your feedback.
On 02/06/2015 08:09 PM, Sergei Shtylyov wrote:
> Hello.
>
> On 02/06/2015 08:37 PM, Javier Martinez Canillas wrote:
>
>> All the device nodes for the Exynos5420 power-domains have a quite
>> generic "power-domain" name.
>
> And this is in conformance to the ePAPR standard.
>
True, I forgot that the ePAPR recommends that the node names should be
somewhat generic but OTOH this is the only Exynos DTSI file that follows
the standard for the power domain device nodes. All other Exynos DTSI
use a prefix to differentiate between each power domain.
>> So in case of an error, the Exynos PD
>> driver shows the following (not very useful) message:
>
>> "Power domain power-domain disable failed"
>
> Why not fix the message instead to use the full device name?
>
Well, the full node name is also not very useful IMHO since you have
to check the DTSI or SoC manual to map the device node unit-address to
the corresponding power domain.
I used $subject when debugging an HDMI issue and instead of dropping
it, I just posted it in case someone considered useful. I don't really
mind if the patch is nacked / not picked.
Best regards,
Javier
2015-02-06 21:50 GMT+01:00 Javier Martinez Canillas
<[email protected]>:
> Hello Sergei,
>
> Thanks a lot for your feedback.
>
> On 02/06/2015 08:09 PM, Sergei Shtylyov wrote:
>> Hello.
>>
>> On 02/06/2015 08:37 PM, Javier Martinez Canillas wrote:
>>
>>> All the device nodes for the Exynos5420 power-domains have a quite
>>> generic "power-domain" name.
>>
>> And this is in conformance to the ePAPR standard.
>>
>
> True, I forgot that the ePAPR recommends that the node names should be
> somewhat generic but OTOH this is the only Exynos DTSI file that follows
> the standard for the power domain device nodes. All other Exynos DTSI
> use a prefix to differentiate between each power domain.
>
>>> So in case of an error, the Exynos PD
>>> driver shows the following (not very useful) message:
>>
>>> "Power domain power-domain disable failed"
>>
>> Why not fix the message instead to use the full device name?
>>
>
> Well, the full node name is also not very useful IMHO since you have
> to check the DTSI or SoC manual to map the device node unit-address to
> the corresponding power domain.
>
> I used $subject when debugging an HDMI issue and instead of dropping
> it, I just posted it in case someone considered useful. I don't really
> mind if the patch is nacked / not picked.
Additionally (on Arndale Octa):
$ cat /sys/kernel/debug/pm_genpd/pm_genpd_summary
domain status slaves
/device runtime status
----------------------------------------------------------------------
power-domain on
/devices/platform/amba/3880000.adma suspended
power-domain off
power-domain off
power-domain off
power-domain off
power-domain off
This really is not helpful. From the power domain debugfs code it is
complicated to extract of_node of power domain. It is easier to print
the name of power domain. But wait... all names are the same! :) So
why do we have the name in the first place?
Hello.
On 2/10/2015 2:46 PM, Krzysztof Kozlowski wrote:
>>>> All the device nodes for the Exynos5420 power-domains have a quite
>>>> generic "power-domain" name.
>>> And this is in conformance to the ePAPR standard.
>> True, I forgot that the ePAPR recommends that the node names should be
>> somewhat generic but OTOH this is the only Exynos DTSI file that follows
>> the standard for the power domain device nodes. All other Exynos DTSI
>> use a prefix to differentiate between each power domain.
>>>> So in case of an error, the Exynos PD
>>>> driver shows the following (not very useful) message:
>>>> "Power domain power-domain disable failed"
>>> Why not fix the message instead to use the full device name?
>> Well, the full node name is also not very useful IMHO since you have
>> to check the DTSI or SoC manual to map the device node unit-address to
>> the corresponding power domain.
>> I used $subject when debugging an HDMI issue and instead of dropping
>> it, I just posted it in case someone considered useful. I don't really
>> mind if the patch is nacked / not picked.
> Additionally (on Arndale Octa):
> $ cat /sys/kernel/debug/pm_genpd/pm_genpd_summary
> domain status slaves
> /device runtime status
> ----------------------------------------------------------------------
> power-domain on
> /devices/platform/amba/3880000.adma suspended
> power-domain off
> power-domain off
> power-domain off
> power-domain off
> power-domain off
> This really is not helpful. From the power domain debugfs code it is
> complicated to extract of_node of power domain.
You shouldn't need it.
> It is easier to print
> the name of power domain. But wait... all names are the same! :) So
> why do we have the name in the first place?
I'm not sure why the full platform device names aren't printed -- they
should all be different.
WBR, Sergei
On wto, 2015-02-10 at 14:55 +0300, Sergei Shtylyov wrote:
> Hello.
>
> On 2/10/2015 2:46 PM, Krzysztof Kozlowski wrote:
> > Additionally (on Arndale Octa):
>
> > $ cat /sys/kernel/debug/pm_genpd/pm_genpd_summary
> > domain status slaves
> > /device runtime status
> > ----------------------------------------------------------------------
> > power-domain on
> > /devices/platform/amba/3880000.adma suspended
> > power-domain off
> > power-domain off
> > power-domain off
> > power-domain off
> > power-domain off
>
> > This really is not helpful. From the power domain debugfs code it is
> > complicated to extract of_node of power domain.
>
> You shouldn't need it.
>
> > It is easier to print
> > the name of power domain. But wait... all names are the same! :) So
> > why do we have the name in the first place?
>
> I'm not sure why the full platform device names aren't printed -- they
> should all be different.
This debugfs code iterates over list of generic_pm_domains (gpd_list). I
cannot find function for translating from genpd to its platform device
so only genpd->name can be printed.
Best regards,
Krzysztof
On 2/10/2015 3:17 PM, Krzysztof Kozlowski wrote:
>>> Additionally (on Arndale Octa):
>>> $ cat /sys/kernel/debug/pm_genpd/pm_genpd_summary
>>> domain status slaves
>>> /device runtime status
>>> ----------------------------------------------------------------------
>>> power-domain on
>>> /devices/platform/amba/3880000.adma suspended
>>> power-domain off
>>> power-domain off
>>> power-domain off
>>> power-domain off
>>> power-domain off
>>> This really is not helpful. From the power domain debugfs code it is
>>> complicated to extract of_node of power domain.
>> You shouldn't need it.
>>> It is easier to print
>>> the name of power domain. But wait... all names are the same! :) So
>>> why do we have the name in the first place?
>> I'm not sure why the full platform device names aren't printed -- they
>> should all be different.
> This debugfs code iterates over list of generic_pm_domains (gpd_list). I
> cannot find function for translating from genpd to its platform device
> so only genpd->name can be printed.
Then why power domains aren't just named with the platform device names?
> Best regards,
> Krzysztof
WBR, Sergei
On wto, 2015-02-10 at 15:21 +0300, Sergei Shtylyov wrote:
> On 2/10/2015 3:17 PM, Krzysztof Kozlowski wrote:
>
> >>> Additionally (on Arndale Octa):
>
> >>> $ cat /sys/kernel/debug/pm_genpd/pm_genpd_summary
> >>> domain status slaves
> >>> /device runtime status
> >>> ----------------------------------------------------------------------
> >>> power-domain on
> >>> /devices/platform/amba/3880000.adma suspended
> >>> power-domain off
> >>> power-domain off
> >>> power-domain off
> >>> power-domain off
> >>> power-domain off
>
> >>> This really is not helpful. From the power domain debugfs code it is
> >>> complicated to extract of_node of power domain.
>
> >> You shouldn't need it.
>
> >>> It is easier to print
> >>> the name of power domain. But wait... all names are the same! :) So
> >>> why do we have the name in the first place?
>
> >> I'm not sure why the full platform device names aren't printed -- they
> >> should all be different.
>
> > This debugfs code iterates over list of generic_pm_domains (gpd_list). I
> > cannot find function for translating from genpd to its platform device
> > so only genpd->name can be printed.
>
> Then why power domains aren't just named with the platform device names?
Right, the mach-exynos/pm_domains.c set the name equal to OF node name.
I'll send a patch extending the name.
Best regards,
Krzysztof
Hello,
On 02/10/2015 01:30 PM, Krzysztof Kozlowski wrote:
> On wto, 2015-02-10 at 15:21 +0300, Sergei Shtylyov wrote:
>> On 2/10/2015 3:17 PM, Krzysztof Kozlowski wrote:
>>
>> >>> Additionally (on Arndale Octa):
>>
>> >>> $ cat /sys/kernel/debug/pm_genpd/pm_genpd_summary
>> >>> domain status slaves
>> >>> /device runtime status
>> >>> ----------------------------------------------------------------------
>> >>> power-domain on
>> >>> /devices/platform/amba/3880000.adma suspended
>> >>> power-domain off
>> >>> power-domain off
>> >>> power-domain off
>> >>> power-domain off
>> >>> power-domain off
>>
>> >>> This really is not helpful. From the power domain debugfs code it is
>> >>> complicated to extract of_node of power domain.
>>
Not very useful indeed.
>> >> You shouldn't need it.
>>
>> >>> It is easier to print
>> >>> the name of power domain. But wait... all names are the same! :) So
>> >>> why do we have the name in the first place?
>>
>> >> I'm not sure why the full platform device names aren't printed -- they
>> >> should all be different.
>>
Yes, but like I said in a previous email the fact that are different doesn't
necessarily mean that they will be helpful for debugging purposes.
>> > This debugfs code iterates over list of generic_pm_domains (gpd_list). I
>> > cannot find function for translating from genpd to its platform device
>> > so only genpd->name can be printed.
>>
>> Then why power domains aren't just named with the platform device names?
>
> Right, the mach-exynos/pm_domains.c set the name equal to OF node name.
> I'll send a patch extending the name.
>
IIRC the OF core uses the device node unit address and node name to create
the platform device names so you will have something like 10044000.power-domain.
Same if using the node full_name since it will /power-domain@10044000. In both
cases the DTS should have to be checked to know which power domain really is
unless someone knows by heart the power domains addresses.
But if using generic names for the power domains as suggested by ePAPR is so
important then we should change all the other Exynos DTS files which don't do.
> Best regards,
> Krzysztof
>
>
Best regards,
Javier
On 10/02/15 13:46, Javier Martinez Canillas wrote:
>>>> This debugfs code iterates over list of generic_pm_domains (gpd_list). I
>>>> >> > cannot find function for translating from genpd to its platform device
>>>> >> > so only genpd->name can be printed.
>>> >>
>>> >> Then why power domains aren't just named with the platform device names?
>> >
>> > Right, the mach-exynos/pm_domains.c set the name equal to OF node name.
>> > I'll send a patch extending the name.
>> >
> IIRC the OF core uses the device node unit address and node name to create
> the platform device names so you will have something like 10044000.power-domain.
>
> Same if using the node full_name since it will /power-domain@10044000. In both
> cases the DTS should have to be checked to know which power domain really is
> unless someone knows by heart the power domains addresses.
>
> But if using generic names for the power domains as suggested by ePAPR is so
> important then we should change all the other Exynos DTS files which don't do.
Perhaps we could assign OF aliases to the power domain device nodes in DT
and then in the power domains driver map those aliases to more descriptive
names when creating the power domains?
--
Regards,
Sylwester
On wto, 2015-02-10 at 14:00 +0100, Sylwester Nawrocki wrote:
> On 10/02/15 13:46, Javier Martinez Canillas wrote:
> >>>> This debugfs code iterates over list of generic_pm_domains (gpd_list). I
> >>>> >> > cannot find function for translating from genpd to its platform device
> >>>> >> > so only genpd->name can be printed.
> >>> >>
> >>> >> Then why power domains aren't just named with the platform device names?
> >> >
> >> > Right, the mach-exynos/pm_domains.c set the name equal to OF node name.
> >> > I'll send a patch extending the name.
> >> >
> > IIRC the OF core uses the device node unit address and node name to create
> > the platform device names so you will have something like 10044000.power-domain.
> >
> > Same if using the node full_name since it will /power-domain@10044000. In both
> > cases the DTS should have to be checked to know which power domain really is
> > unless someone knows by heart the power domains addresses.
For the kernel developer that would be descriptive enough to find the
real domain but... as you said each time one would have to grep through
manual or DTS which is slower. However for end-user that still won't be
descriptive enough.
> >
> > But if using generic names for the power domains as suggested by ePAPR is so
> > important then we should change all the other Exynos DTS files which don't do.
>
> Perhaps we could assign OF aliases to the power domain device nodes in DT
> and then in the power domains driver map those aliases to more descriptive
> names when creating the power domains?
That would required additional alias in DT but it could be the most
descriptive for a user.
Best regards,
Krzysztof
On 10/02/15 14:14, Krzysztof Kozlowski wrote:
> On wto, 2015-02-10 at 14:00 +0100, Sylwester Nawrocki wrote:
>> > On 10/02/15 13:46, Javier Martinez Canillas wrote:
>>>>>> > >>>> This debugfs code iterates over list of generic_pm_domains (gpd_list). I
>>>>>>>>> > >>>> >> > cannot find function for translating from genpd to its platform device
>>>>>>>>> > >>>> >> > so only genpd->name can be printed.
>>>>>>> > >>> >>
>>>>>>> > >>> >> Then why power domains aren't just named with the platform device names?
>>>>> > >> >
>>>>> > >> > Right, the mach-exynos/pm_domains.c set the name equal to OF node name.
>>>>> > >> > I'll send a patch extending the name.
>>>>> > >> >
>>> > > IIRC the OF core uses the device node unit address and node name to create
>>> > > the platform device names so you will have something like 10044000.power-domain.
>>> > >
>>> > > Same if using the node full_name since it will /power-domain@10044000. In both
>>> > > cases the DTS should have to be checked to know which power domain really is
>>> > > unless someone knows by heart the power domains addresses.
> For the kernel developer that would be descriptive enough to find the
> real domain but... as you said each time one would have to grep through
> manual or DTS which is slower. However for end-user that still won't be
> descriptive enough.
>
>>> > >
>>> > > But if using generic names for the power domains as suggested by ePAPR is so
>>> > > important then we should change all the other Exynos DTS files which don't do.
>> >
>> > Perhaps we could assign OF aliases to the power domain device nodes in DT
>> > and then in the power domains driver map those aliases to more descriptive
>> > names when creating the power domains?
>
> That would required additional alias in DT but it could be the most
> descriptive for a user.
Yes, we could fall back to of_node->full_name if alias is not present in DT.
--
Regards,
Sylwester