Thread (8 messages) 8 messages, 3 authors, 2021-02-18

Re: [PATCH v3] arm64: dts: mediatek: Add mt8192 power domains controller

From: Enric Balletbo Serra <eballetbo@gmail.com>
Date: 2021-02-18 14:03:38
Also in: linux-arm-kernel, linux-mediatek, lkml

Hi Weiyi,

Missatge de Weiyi Lu [off-list ref] del dia dj., 18 de febr.
2021 a les 11:31:
On Mon, 2020-11-30 at 19:16 +0800, Weiyi Lu wrote:
quoted
On Fri, 2020-11-27 at 13:42 +0100, Matthias Brugger wrote:
quoted
On 19/11/2020 15:13, Enric Balletbo Serra wrote:
quoted
Hi Weiyi,

Missatge de Weiyi Lu [off-list ref] del dia dj., 19 de nov.
2020 a les 14:10:
quoted
On Thu, 2020-11-19 at 13:13 +0100, Enric Balletbo Serra wrote:
quoted
Hi Weiyi,

Thank you for the patch

Missatge de Weiyi Lu [off-list ref] del dia dj., 19 de nov.
2020 a les 11:48:
quoted
Add power domains controller node for SoC mt8192

Signed-off-by: Weiyi Lu <redacted>
---
[...]
quoted
quoted
quoted
quoted
+                       /* System Power Manager */
+                       spm: power-controller {
+                               compatible = "mediatek,mt8192-power-controller";
+                               #address-cells = <1>;
+                               #size-cells = <0>;
+                               #power-domain-cells = <1>;
+
+                               /* power domain of the SoC */
+                               audio@MT8192_POWER_DOMAIN_AUDIO {
If you run the dt_bindings_check it should return some errors, as all
these node names should be 'power-domain@'. Which is a bit annoying
because then you will get a bunch of errors like this:

[    1.969110] debugfs: Directory 'power-domain' with parent
'pm_genpd' already present!
[    1.976997] debugfs: Directory 'power-domain' with parent
'pm_genpd' already present!
[    1.984828] debugfs: Directory 'power-domain' with parent
'pm_genpd' already present!
[    1.992657] debugfs: Directory 'power-domain' with parent
'pm_genpd' already present!
[    2.000685] debugfs: Directory 'power-domain' with parent
'pm_genpd' already present!
[    2.008566] debugfs: Directory 'power-domain' with parent
'pm_genpd' already present!
[    2.016395] debugfs: Directory 'power-domain' with parent
'pm_genpd' already present!
[    2.024221] debugfs: Directory 'power-domain' with parent
'pm_genpd' already present!
[    2.032049] debugfs: Directory 'power-domain' with parent
'pm_genpd' already present!
[    2.039874] debugfs: Directory 'power-domain' with parent
'pm_genpd' already present!
[    2.047699] debugfs: Directory 'power-domain' with parent
'pm_genpd' already present!
[    2.055524] debugfs: Directory 'power-domain' with parent
'pm_genpd' already present!
[    2.063352] debugfs: Directory 'power-domain' with parent
'pm_genpd' already present!
[    2.071176] debugfs: Directory 'power-domain' with parent
'pm_genpd' already present!

But that's another problem that should be handled in debugfs system.
Indeed...so I chose to use different name in dts to avoid problems in
debugfs. It does violate the naming rules.
But your binding will not pass (or trigger warnings) the dtb check
then. Rob was clear that names should be generic. Proper fix is fix
debugfs not the binding.
By the way, is anybody working on this debugfs issue?
I think we can solve this problem by adding "name" to the struct
scpsys_domain_data and use this domain_data->name as the genpd.name.
This is very simple. But I want to know if you both like it?
Hi Enric and Matthias,

May I have your opinions on how you might to fix this issue?
I'll try to give another name to each power domain in the
scpsys_domain_data and register power domain with this name like below

struct scpsys_domain_data {
        ...
+       char *name;
 };


-       pd->genpd.name = node->name;
+       pd->genpd.name = pd->data->name ?: node->name;


Does it violate the naming rules to some extent? or it's acceptable?
I think it could be acceptable, I think doesn't violate any rule. My
doubt here is we should name in a generic way in the form
'power-domain@addr' getting the last part of the full node name to
avoid conflicts or if the name should be driver-specific. I think it
makes sense to be driver-specific as is more helpful for debugging
purposes. If we do driver-specific (with data->name) I'd fail if is
not supplied.

Thanks,
  Enric
quoted
quoted
Regards,
Matthias

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help