Re: [PATCH v4 3/8] dt-bindings: mfd: renesas,rzn1-sysctrl: document RZ/N1 sysctrl node
Hi Michel,
On Tue, Apr 17, 2018 at 9:56 AM, Michel Pollet
wrote:
> On 13 April 2018 19:06, Rob Herring:
>> On Tue, Apr 10, 2018 at 09:30:03AM +0100, Michel Pollet wrote:
>> > The Renesas RZ/N1 Family (Part #R9A06G0xx) has a multi-function system
>> > controller. This documents the node used to encapsulate it's sub
>> > drivers.
>> >
>> > Signed-off-by: Michel Pollet
>> > ---
>> > .../bindings/mfd/renesas,rzn1-sysctrl.txt | 23
>> ++
>> > 1 file changed, 23 insertions(+)
>> > create mode 100644
>> > Documentation/devicetree/bindings/mfd/renesas,rzn1-sysctrl.txt
>> >
>> > diff --git
>> > a/Documentation/devicetree/bindings/mfd/renesas,rzn1-sysctrl.txt
>> > b/Documentation/devicetree/bindings/mfd/renesas,rzn1-sysctrl.txt
>> > new file mode 100644
>> > index 000..9897f8f
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/mfd/renesas,rzn1-sysctrl.txt
>> > @@ -0,0 +1,23 @@
>> > +DT bindings for the Renesas RZ/N1 System Controller
>> > +
>> > +== System Controller Node ==
>> > +
>> > +The system controller node currently only hosts a single sub-node to
>> > +handle the rebooting of the CPU. Eventually it will host the clock
>> > +driver, SMP start handler, watchdog etc.
>>
>> Please submit a complete binding for the h/w block.
>>
>> Again, if the only reason you have sub nodes is to define compatible strings
>> and in turn enumerate drivers, then you don't need the nodes in DT. DT is
>> not the only way to instantiate drivers.
>
> I can't document it before I have the code. There is 0.000% chance of my clock
> driver for example to be upstreamed the way I would imagine making it -- in
> fact pretty much any other driver will have to be reworked to fit, so
> documenting
> bindings first is impossible.
>
> So, if I understand correctly, you are telling me to make a 'sysctrl' driver
> and use
> platform_device to instantiate my sub-drivers? Isn't that what machine files
> used
> to do? And they are now banned?
>
> Geert, any guidance here?
It depends on how many and which subnodes you want to add to the sysctrl
node. Without a complete binding for the block, we cannot know.
If the major part will be the clock driver, I would make that the main
driver for the sysctrl node. The clock driver can easily register
e.g. a simple reset handler.
Cfr. the renesas-cpg-mssr driver, which also handles (module) resets.
There are plenty of other examples of drivers providing multiple
functionalities (e.g. pinctrl drivers also registering GPIO controllers).
If a monolithic driver becomes too large, it can still be split using the
MFD framework.
E.g. the BD9571 PMIC driver is split in an MFD core driver, and 2 drivers
for different functionalities:
drivers/gpio/gpio-bd9571mwv.c
drivers/mfd/bd9571mwv.c
drivers/regulator/bd9571mwv-regulator.c
include/linux/mfd/bd9571mwv.h
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
RE: [PATCH v4 3/8] dt-bindings: mfd: renesas,rzn1-sysctrl: document RZ/N1 sysctrl node
Hi Rob, On 13 April 2018 19:06, Rob Herring: > On Tue, Apr 10, 2018 at 09:30:03AM +0100, Michel Pollet wrote: > > The Renesas RZ/N1 Family (Part #R9A06G0xx) has a multi-function system > > controller. This documents the node used to encapsulate it's sub > > drivers. > > > > Signed-off-by: Michel Pollet > > --- > > .../bindings/mfd/renesas,rzn1-sysctrl.txt | 23 > ++ > > 1 file changed, 23 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/mfd/renesas,rzn1-sysctrl.txt > > > > diff --git > > a/Documentation/devicetree/bindings/mfd/renesas,rzn1-sysctrl.txt > > b/Documentation/devicetree/bindings/mfd/renesas,rzn1-sysctrl.txt > > new file mode 100644 > > index 000..9897f8f > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/mfd/renesas,rzn1-sysctrl.txt > > @@ -0,0 +1,23 @@ > > +DT bindings for the Renesas RZ/N1 System Controller > > + > > +== System Controller Node == > > + > > +The system controller node currently only hosts a single sub-node to > > +handle the rebooting of the CPU. Eventually it will host the clock > > +driver, SMP start handler, watchdog etc. > > Please submit a complete binding for the h/w block. > > Again, if the only reason you have sub nodes is to define compatible strings > and in turn enumerate drivers, then you don't need the nodes in DT. DT is > not the only way to instantiate drivers. I can't document it before I have the code. There is 0.000% chance of my clock driver for example to be upstreamed the way I would imagine making it -- in fact pretty much any other driver will have to be reworked to fit, so documenting bindings first is impossible. So, if I understand correctly, you are telling me to make a 'sysctrl' driver and use platform_device to instantiate my sub-drivers? Isn't that what machine files used to do? And they are now banned? Geert, any guidance here? > > Rob Thanks! Michel Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709.
Re: [PATCH v4 3/8] dt-bindings: mfd: renesas,rzn1-sysctrl: document RZ/N1 sysctrl node
On Tue, Apr 10, 2018 at 09:30:03AM +0100, Michel Pollet wrote: > The Renesas RZ/N1 Family (Part #R9A06G0xx) has a multi-function > system controller. This documents the node used to encapsulate > it's sub drivers. > > Signed-off-by: Michel Pollet > --- > .../bindings/mfd/renesas,rzn1-sysctrl.txt | 23 > ++ > 1 file changed, 23 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/mfd/renesas,rzn1-sysctrl.txt > > diff --git a/Documentation/devicetree/bindings/mfd/renesas,rzn1-sysctrl.txt > b/Documentation/devicetree/bindings/mfd/renesas,rzn1-sysctrl.txt > new file mode 100644 > index 000..9897f8f > --- /dev/null > +++ b/Documentation/devicetree/bindings/mfd/renesas,rzn1-sysctrl.txt > @@ -0,0 +1,23 @@ > +DT bindings for the Renesas RZ/N1 System Controller > + > +== System Controller Node == > + > +The system controller node currently only hosts a single sub-node to handle > +the rebooting of the CPU. Eventually it will host the clock driver, SMP > +start handler, watchdog etc. Please submit a complete binding for the h/w block. Again, if the only reason you have sub nodes is to define compatible strings and in turn enumerate drivers, then you don't need the nodes in DT. DT is not the only way to instantiate drivers. Rob
