diff mbox series

[RFC,v1,13/14] riscv: dts: Introduce power domain node with simple-bus compatible

Message ID 20241203134137.2114847-14-m.wilczynski@samsung.com (mailing list archive)
State New
Headers show
Series [RFC,v1,01/14] clk: thead: Refactor TH1520 clock driver to share common code | expand

Checks

Context Check Description
conchuod/vmtest-fixes-PR fail merge-conflict

Commit Message

Michal Wilczynski Dec. 3, 2024, 1:41 p.m. UTC
The DRM Imagination GPU requires a power-domain driver, but the driver
for "thead,th1520-aon" is not yet available. To ensure that the 'aon'
node and its child 'pd' node are properly recognized and probed by the
kernel, add "simple-bus" to the compatible property of the 'aon' node.

This change allows the kernel to treat the 'aon' node as a simple bus,
enabling the child nodes to be probed and initialized independently. It
ensures that the power domain can be managed appropriately until the
specific AON driver is developed.

This commit introduces some errors while running dtbs_check, as the aon
doesn't have the dt-bindings yet.

Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
---
 arch/riscv/boot/dts/thead/th1520.dtsi | 11 +++++++++++
 1 file changed, 11 insertions(+)

Comments

Krzysztof Kozlowski Dec. 3, 2024, 3:52 p.m. UTC | #1
On 03/12/2024 14:41, Michal Wilczynski wrote:
> The DRM Imagination GPU requires a power-domain driver, but the driver
> for "thead,th1520-aon" is not yet available. To ensure that the 'aon'
> node and its child 'pd' node are properly recognized and probed by the
> kernel, add "simple-bus" to the compatible property of the 'aon' node.
> 
> This change allows the kernel to treat the 'aon' node as a simple bus,
> enabling the child nodes to be probed and initialized independently. It
> ensures that the power domain can be managed appropriately until the
> specific AON driver is developed.
> 
> This commit introduces some errors while running dtbs_check, as the aon
> doesn't have the dt-bindings yet.
> 
> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
> ---
>  arch/riscv/boot/dts/thead/th1520.dtsi | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi b/arch/riscv/boot/dts/thead/th1520.dtsi
> index 39d39059160d..58f93ad3eb6e 100644
> --- a/arch/riscv/boot/dts/thead/th1520.dtsi
> +++ b/arch/riscv/boot/dts/thead/th1520.dtsi
> @@ -6,6 +6,7 @@
>  
>  #include <dt-bindings/interrupt-controller/irq.h>
>  #include <dt-bindings/clock/thead,th1520-clk.h>
> +#include <dt-bindings/power/thead,th1520-power.h>
>  
>  / {
>  	compatible = "thead,th1520";
> @@ -229,6 +230,16 @@ stmmac_axi_config: stmmac-axi-config {
>  		snps,blen = <0 0 64 32 0 0 0>;
>  	};
>  
> +	aon {
> +		compatible = "thead,th1520-aon", "simple-bus";

1. No, that's not a bus.
2. Please run scripts/checkpatch.pl and fix reported warnings. Then
please run `scripts/checkpatch.pl --strict` and (probably) fix more
warnings. Some warnings can be ignored, especially from --strict run,
but the code here looks like it needs a fix. Feel free to get in touch
if the warning is not clear.

Sorry, this patchset is not ready, unless by RFC you meant - do not
review, because it is not ready. Then it is fine. But then *clearly
express* this in cover letter, so we know what you expect from us (and I
would not waste my time to go through all this).

Best regards,
Krzysztof
Michal Wilczynski Dec. 4, 2024, 10:34 a.m. UTC | #2
On 12/3/24 16:52, Krzysztof Kozlowski wrote:
> On 03/12/2024 14:41, Michal Wilczynski wrote:
>> The DRM Imagination GPU requires a power-domain driver, but the driver
>> for "thead,th1520-aon" is not yet available. To ensure that the 'aon'
>> node and its child 'pd' node are properly recognized and probed by the
>> kernel, add "simple-bus" to the compatible property of the 'aon' node.
>>
>> This change allows the kernel to treat the 'aon' node as a simple bus,
>> enabling the child nodes to be probed and initialized independently. It
>> ensures that the power domain can be managed appropriately until the
>> specific AON driver is developed.
>>
>> This commit introduces some errors while running dtbs_check, as the aon
>> doesn't have the dt-bindings yet.
>>
>> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
>> ---
>>  arch/riscv/boot/dts/thead/th1520.dtsi | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi b/arch/riscv/boot/dts/thead/th1520.dtsi
>> index 39d39059160d..58f93ad3eb6e 100644
>> --- a/arch/riscv/boot/dts/thead/th1520.dtsi
>> +++ b/arch/riscv/boot/dts/thead/th1520.dtsi
>> @@ -6,6 +6,7 @@
>>  
>>  #include <dt-bindings/interrupt-controller/irq.h>
>>  #include <dt-bindings/clock/thead,th1520-clk.h>
>> +#include <dt-bindings/power/thead,th1520-power.h>
>>  
>>  / {
>>  	compatible = "thead,th1520";
>> @@ -229,6 +230,16 @@ stmmac_axi_config: stmmac-axi-config {
>>  		snps,blen = <0 0 64 32 0 0 0>;
>>  	};
>>  
>> +	aon {
>> +		compatible = "thead,th1520-aon", "simple-bus";
> 
> 1. No, that's not a bus.

I understand that using "simple-bus" for the 'aon' node was not
appropriate. Since the 'aon' node isn't needed for testing this
patchset, I will remove it and move the power-domain device tree node to
the SoC. Future changes to the 'aon' node will be handled in a separate
patchset.

> 2. Please run scripts/checkpatch.pl and fix reported warnings. Then
> please run `scripts/checkpatch.pl --strict` and (probably) fix more
> warnings. Some warnings can be ignored, especially from --strict run,
> but the code here looks like it needs a fix. Feel free to get in touch
> if the warning is not clear.
> 
> Sorry, this patchset is not ready, unless by RFC you meant - do not
> review, because it is not ready. Then it is fine. But then *clearly
> express* this in cover letter, so we know what you expect from us (and I
> would not waste my time to go through all this).

My intention with this patchset was to gather feedback on the overall
direction of the changes. I understand that clearer communication in the
cover letter would have been beneficial. Moving forward, I will ensure
that the patch's readiness and expectations are explicitly stated.

Thanks a lot for your review.
Michał

> 
> Best regards,
> Krzysztof
>
diff mbox series

Patch

diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi b/arch/riscv/boot/dts/thead/th1520.dtsi
index 39d39059160d..58f93ad3eb6e 100644
--- a/arch/riscv/boot/dts/thead/th1520.dtsi
+++ b/arch/riscv/boot/dts/thead/th1520.dtsi
@@ -6,6 +6,7 @@ 
 
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/clock/thead,th1520-clk.h>
+#include <dt-bindings/power/thead,th1520-power.h>
 
 / {
 	compatible = "thead,th1520";
@@ -229,6 +230,16 @@  stmmac_axi_config: stmmac-axi-config {
 		snps,blen = <0 0 64 32 0 0 0>;
 	};
 
+	aon {
+		compatible = "thead,th1520-aon", "simple-bus";
+
+		pd: power-domain {
+			compatible = "thead,th1520-pd";
+			thead,vosys-regmap = <&vosys_reg>;
+			#power-domain-cells = <1>;
+		};
+	};
+
 	soc {
 		compatible = "simple-bus";
 		interrupt-parent = <&plic>;