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

[Michal Wilczynski <m.wilczynski@samsung.com>]

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
>