Hi Rob, On Fri, 2021-02-05 at 16:35 -0600, Rob Herring wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you > know the content is safe > > On Wed, Jan 20, 2021 at 09:19:21AM +0100, Steen Hegelund wrote: > > This provides reset driver support for the Microchip Sparx5 PCB134 > > and > > PCB135 reference boards. > > This isn't a compatible change. You need an explanation why that's > okay > if that's intended. > > > > > Signed-off-by: Steen Hegelund <steen.hegelund@xxxxxxxxxxxxx> > > --- > > arch/arm64/boot/dts/microchip/sparx5.dtsi | 14 +++++++++++--- > > 1 file changed, 11 insertions(+), 3 deletions(-) > > > > diff --git a/arch/arm64/boot/dts/microchip/sparx5.dtsi > > b/arch/arm64/boot/dts/microchip/sparx5.dtsi > > index 380281f312d8..4edbb9fcdce0 100644 > > --- a/arch/arm64/boot/dts/microchip/sparx5.dtsi > > +++ b/arch/arm64/boot/dts/microchip/sparx5.dtsi > > @@ -132,9 +132,17 @@ mux: mux-controller { > > }; > > }; > > > > - reset@611010008 { > > - compatible = "microchip,sparx5-chip-reset"; > > - reg = <0x6 0x11010008 0x4>; > > + gcb_ctrl: syscon@611010000 { > > + compatible = "microchip,sparx5-gcb-syscon", > > "syscon"; > > + reg = <0x6 0x11010000 0x10000>; > > + }; > > + > > + reset: reset-controller@0 { > > + compatible = "microchip,sparx5-switch-reset"; > > + reg = <0x6 0x0 0x0>; > > Your register length is 0? Yes, I only use the syscons. > > > + #reset-cells = <1>; > > + cpu-syscon = <&cpu_ctrl>; > > Can't you accomplish the same thing adding these to > "microchip,sparx5-chip-reset"? Or possibly as a child node. > > Define nodes based on h/w blocks, not as containers of things you > happen > to want for some driver. Yes - I think the idea of using syscons came from another reset driver, but it probably makes more sense to just use IO ranges directly. I will try that out. > > > + gcb-syscon = <&gcb_ctrl>; > > }; > > > > uart0: serial@600100000 { > > -- > > 2.29.2 > > Thanks for your comments BR Steen