| Message ID | 20240222-clk-mv200-v3-2-f30795b50318@outlook.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | clk: hisilicon: add support for Hi3798MV200 | expand |
On 21/02/2024 17:41, Yang Xiwen via B4 Relay wrote: > From: Yang Xiwen <forbidden405@outlook.com> > > Add "syscon" and "simple-mfd" compatibles to CRG node due to recent > binding changes. Why? You claimed you added them in the bindings because DTS has them. In DTS you claim reason is: binding has them. That's confusing. > > Signed-off-by: Yang Xiwen <forbidden405@outlook.com> > --- > arch/arm/boot/dts/hisilicon/hi3519.dtsi | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm/boot/dts/hisilicon/hi3519.dtsi b/arch/arm/boot/dts/hisilicon/hi3519.dtsi > index a42b71cdc5d7..da46e01b8fdc 100644 > --- a/arch/arm/boot/dts/hisilicon/hi3519.dtsi > +++ b/arch/arm/boot/dts/hisilicon/hi3519.dtsi > @@ -35,7 +35,7 @@ clk_3m: clk_3m { > }; > > crg: clock-reset-controller@12010000 { > - compatible = "hisilicon,hi3519-crg"; > + compatible = "hisilicon,hi3519-crg", "syscon", "simple-mfd"; Why? This does not make much sense. No children here, no usage as syscon. Best regards, Krzysztof
On 2/23/2024 2:08 AM, Krzysztof Kozlowski wrote: > On 21/02/2024 17:41, Yang Xiwen via B4 Relay wrote: >> From: Yang Xiwen <forbidden405@outlook.com> >> >> Add "syscon" and "simple-mfd" compatibles to CRG node due to recent >> binding changes. > Why? You claimed you added them in the bindings because DTS has them. In > DTS you claim reason is: binding has them. > > That's confusing. Because the old txt based binding claimed there should not be a "syscon" and "simple-mfd". But it exists in hi3798cv200.dtsi. And i think it does no harm to be there. So should i do it in two commits? > >> Signed-off-by: Yang Xiwen <forbidden405@outlook.com> >> --- >> arch/arm/boot/dts/hisilicon/hi3519.dtsi | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/arch/arm/boot/dts/hisilicon/hi3519.dtsi b/arch/arm/boot/dts/hisilicon/hi3519.dtsi >> index a42b71cdc5d7..da46e01b8fdc 100644 >> --- a/arch/arm/boot/dts/hisilicon/hi3519.dtsi >> +++ b/arch/arm/boot/dts/hisilicon/hi3519.dtsi >> @@ -35,7 +35,7 @@ clk_3m: clk_3m { >> }; >> >> crg: clock-reset-controller@12010000 { >> - compatible = "hisilicon,hi3519-crg"; >> + compatible = "hisilicon,hi3519-crg", "syscon", "simple-mfd"; > Why? This does not make much sense. No children here, no usage as syscon. > > > > Best regards, > Krzysztof >
On 22/02/2024 19:13, Yang Xiwen wrote: > On 2/23/2024 2:08 AM, Krzysztof Kozlowski wrote: >> On 21/02/2024 17:41, Yang Xiwen via B4 Relay wrote: >>> From: Yang Xiwen <forbidden405@outlook.com> >>> >>> Add "syscon" and "simple-mfd" compatibles to CRG node due to recent >>> binding changes. >> Why? You claimed you added them in the bindings because DTS has them. In >> DTS you claim reason is: binding has them. >> >> That's confusing. > > > Because the old txt based binding claimed there should not be a "syscon" > and "simple-mfd". > > > But it exists in hi3798cv200.dtsi. And i think it does no harm to be > there. So should i do it in two commits? hi3798cv200 is not hi3519, is it? You are adding simple-mfd to one SoC because some other has it? I don't see reason to do that. Er, why? Best regards, Krzysztof
On 2/23/2024 2:18 AM, Krzysztof Kozlowski wrote: > On 22/02/2024 19:13, Yang Xiwen wrote: >> On 2/23/2024 2:08 AM, Krzysztof Kozlowski wrote: >>> On 21/02/2024 17:41, Yang Xiwen via B4 Relay wrote: >>>> From: Yang Xiwen <forbidden405@outlook.com> >>>> >>>> Add "syscon" and "simple-mfd" compatibles to CRG node due to recent >>>> binding changes. >>> Why? You claimed you added them in the bindings because DTS has them. In >>> DTS you claim reason is: binding has them. >>> >>> That's confusing. >> >> Because the old txt based binding claimed there should not be a "syscon" >> and "simple-mfd". >> >> >> But it exists in hi3798cv200.dtsi. And i think it does no harm to be >> there. So should i do it in two commits? > hi3798cv200 is not hi3519, is it? You are adding simple-mfd to one SoC > because some other has it? I don't see reason to do that. Er, why? I think it's the careless HiSilicon people who simply forgot to add it. CRG core on these SoCs are very similar. They only provided a bunch of clocks and resets. Some register offsets in them are even the same across SoCs. So I'll say all CRG devices are "syscon" and "simple-mfd". In fact, i do have TRM for Hi3519 so i can prove what i said is true. > > Best regards, > Krzysztof >
diff --git a/arch/arm/boot/dts/hisilicon/hi3519.dtsi b/arch/arm/boot/dts/hisilicon/hi3519.dtsi index a42b71cdc5d7..da46e01b8fdc 100644 --- a/arch/arm/boot/dts/hisilicon/hi3519.dtsi +++ b/arch/arm/boot/dts/hisilicon/hi3519.dtsi @@ -35,7 +35,7 @@ clk_3m: clk_3m { }; crg: clock-reset-controller@12010000 { - compatible = "hisilicon,hi3519-crg"; + compatible = "hisilicon,hi3519-crg", "syscon", "simple-mfd"; #clock-cells = <1>; #reset-cells = <2>; reg = <0x12010000 0x10000>;