On Wed, Feb 17, 2021 at 11:49:27AM +0100, Mark Kettenis wrote:
> > Date: Wed, 17 Feb 2021 20:59:06 +1100
> > From: Jonathan Gray <j...@jsg.id.au>
> > 
> > On Wed, Feb 17, 2021 at 10:33:18AM +0100, Patrick Wildt wrote:
> > > Am Wed, Feb 17, 2021 at 11:56:16AM +1100 schrieb Jonathan Gray:
> > > > Add a driver for "simple-pm-bus" a way to enable a clock and/or
> > > > power domain for a group of devices.
> > > > 
> > > > https://www.kernel.org/doc/Documentation/devicetree/bindings/bus/simple-pm-bus.txt
> > > > 
> > > > This is needed to use am335x/omap4 dtbs from linux 5.11.
> > > 
> > > "A Simple Power-Managed Bus is a transparent bus that doesn't need a real
> > > driver, as it's typically initialized by the boot loader."
> > > 
> > > That's a bit funny though. :-)  But I think they meant "it doesn't need
> > > a real driver, apart from the generic simple-pm-bus driver".
> > > 
> > > 
> > > > Index: sys/dev/fdt/files.fdt
> > > > ===================================================================
> > > > RCS file: /cvs/src/sys/dev/fdt/files.fdt,v
> > > > retrieving revision 1.146
> > > > diff -u -p -r1.146 files.fdt
> > > > --- sys/dev/fdt/files.fdt       5 Feb 2021 00:05:20 -0000       1.146
> > > > +++ sys/dev/fdt/files.fdt       17 Feb 2021 00:49:02 -0000
> > > > @@ -23,6 +23,10 @@ device       simplepanel
> > > >  attach simplepanel at fdt
> > > >  file   dev/fdt/simplepanel.c           simplepanel
> > > >  
> > > > +device simplepmbus: fdt
> > > > +attach simplepmbus at fdt
> > > > +file   dev/fdt/simplepmbus.c           simplepmbus
> > > > +
> > > >  device sxiccmu
> > > >  attach sxiccmu at fdt
> > > >  file   dev/fdt/sxiccmu.c               sxiccmu
> > > > Index: share/man/man4/Makefile
> > > > ===================================================================
> > > > RCS file: /cvs/src/share/man/man4/Makefile,v
> > > > retrieving revision 1.792
> > > > diff -u -p -r1.792 Makefile
> > > > --- share/man/man4/Makefile     4 Feb 2021 16:25:38 -0000       1.792
> > > > +++ share/man/man4/Makefile     17 Feb 2021 00:49:02 -0000
> > > > @@ -72,7 +72,8 @@ MAN=  aac.4 abcrtc.4 abl.4 ac97.4 acphy.4
> > > >         rl.4 rlphy.4 route.4 rsu.4 rtsx.4 rum.4 run.4 rtw.4 rtwn.4 \
> > > >         safe.4 safte.4 sbus.4 schsio.4 scsi.4 sd.4 \
> > > >         sdmmc.4 sdhc.4 se.4 ses.4 sf.4 sili.4 \
> > > > -       simpleamp.4 simpleaudio.4 simplefb.4 simplepanel.4 siop.4 sis.4 
> > > > sk.4 \
> > > > +       simpleamp.4 simpleaudio.4 simplefb.4 simplepanel.4 
> > > > simplepmbus.4 \
> > > > +       siop.4 sis.4 sk.4 \
> > > >         sm.4 smsc.4 softraid.4 spdmem.4 sdtemp.4 speaker.4 sppp.4 
> > > > sqphy.4 \
> > > >         ssdfb.4 st.4 ste.4 stge.4 sti.4 stp.4 sv.4 switch.4 sxiccmu.4 \
> > > >         sxidog.4 sximmc.4 sxipio.4 sxipwm.4 sxirsb.4 sxirtc.4 sxisid.4 \
> > > > Index: sys/arch/armv7/conf/GENERIC
> > > > ===================================================================
> > > > RCS file: /cvs/src/sys/arch/armv7/conf/GENERIC,v
> > > > retrieving revision 1.134
> > > > diff -u -p -r1.134 GENERIC
> > > > --- sys/arch/armv7/conf/GENERIC 4 Feb 2021 16:25:39 -0000       1.134
> > > > +++ sys/arch/armv7/conf/GENERIC 17 Feb 2021 00:49:02 -0000
> > > > @@ -31,6 +31,7 @@ config                bsd     swap generic
> > > >  # The main bus device
> > > >  mainbus0       at root
> > > >  simplebus*     at fdt?
> > > > +simplepmbus*   at fdt?
> > > >  cpu0           at mainbus?
> > > >  
> > > >  # Cortex-A9
> > > > Index: sys/arch/armv7/conf/RAMDISK
> > > > ===================================================================
> > > > RCS file: /cvs/src/sys/arch/armv7/conf/RAMDISK,v
> > > > retrieving revision 1.119
> > > > diff -u -p -r1.119 RAMDISK
> > > > --- sys/arch/armv7/conf/RAMDISK 18 Jun 2020 08:48:04 -0000      1.119
> > > > +++ sys/arch/armv7/conf/RAMDISK 17 Feb 2021 00:49:02 -0000
> > > > @@ -30,6 +30,7 @@ config                bsd root on rd0a swap on rd0b
> > > >  mainbus0       at root
> > > >  softraid0      at root
> > > >  simplebus*     at fdt?
> > > > +simplepmbus*   at fdt?
> > > >  cpu0           at mainbus?
> > > >  
> > > >  # Cortex-A9
> > > > --- /dev/null   Wed Feb 17 11:49:05 2021
> > > > +++ sys/dev/fdt/simplepmbus.c   Tue Feb 16 17:24:55 2021
> > > > @@ -0,0 +1,62 @@
> > > > +/*     $OpenBSD$       */
> > > > +/*
> > > > + * Copyright (c) 2021 Jonathan Gray <j...@openbsd.org>
> > > > + *
> > > > + * Permission to use, copy, modify, and distribute this software for 
> > > > any
> > > > + * purpose with or without fee is hereby granted, provided that the 
> > > > above
> > > > + * copyright notice and this permission notice appear in all copies.
> > > > + *
> > > > + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL 
> > > > WARRANTIES
> > > > + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> > > > + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE 
> > > > FOR
> > > > + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY 
> > > > DAMAGES
> > > > + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN 
> > > > AN
> > > > + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING 
> > > > OUT OF
> > > > + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> > > > + */
> > > > +
> > > > +#include <sys/param.h>
> > > > +#include <sys/device.h>
> > > > +
> > > > +#include <machine/fdt.h>
> > > > +
> > > > +#include <dev/ofw/openfirm.h>
> > > > +#include <dev/ofw/ofw_clock.h>
> > > > +#include <dev/ofw/ofw_power.h>
> > > > +
> > > > +#include <arm/simplebus/simplebusvar.h>
> > > 
> > > Maybe like this, was we have in rkpinctrl?
> > > 
> > > #ifdef __armv7__
> > > #include <arm/simplebus/simplebusvar.h>
> > > #else
> > > #include <arm64/dev/simplebusvar.h>
> > > #endif
> > 
> > The known arm64 usage is quite small. The only use in
> > 5.11 linux/arch/arm64/boot/dts/ is qcom/msm8996.dtsi
> > 
> > Though it would be nice if a machine include could be used without the
> > ifdef for cases like this in general.
> > 
> > > 
> > > Generally I was wondering if we shouldn't just add this to simplebus.c
> > > itself?  You'd need to add a compatible to the match function, and in
> > > the attach function you'd need to check for the compatible and then
> > > do
> > > 
> > >   power_domain_enable(faa->fa_node);
> > >   clock_enable_all(faa->fa_node);
> > > 
> > > in the if-block.
> > > 
> > > Haven't made my mind up yet what I like better, maybe kettenis@ has an
> > > opinion.
> > 
> > The binding text has
> > 
> > 'While "simple-pm-bus" follows the "simple-bus" set of properties,
> > as specified in the Devicetree Specification, it is not an extension
> > of "simple-bus".'
> 
> Ceci n'est pas pas une "simple-bus"!
> 
> Not sure what they're smoking, but for all practical purpose it is an
> extensions of "simple-bus".  Maybe there are some subtle differences
> related to power-management.  But we don't do any of that on OpenBSD
> yet.  But yes I think putting the functionality in simplebus(4) would
> make sense.

here is a diff for that which still works for me

Index: simplebus.c
===================================================================
RCS file: /cvs/src/sys/arch/arm/simplebus/simplebus.c,v
retrieving revision 1.16
diff -u -p -r1.16 simplebus.c
--- simplebus.c 29 Apr 2020 15:25:07 -0000      1.16
+++ simplebus.c 17 Feb 2021 11:07:55 -0000
@@ -23,6 +23,8 @@
 
 #include <dev/ofw/openfirm.h>
 #include <dev/ofw/fdt.h>
+#include <dev/ofw/ofw_clock.h>
+#include <dev/ofw/ofw_power.h>
 
 #include <arm/fdt.h>
 #include <arm/simplebus/simplebusvar.h>
@@ -54,10 +56,8 @@ simplebus_match(struct device *parent, v
        if (fa->fa_node == 0)
                return (0);
 
-       if (!OF_is_compatible(fa->fa_node, "simple-bus"))
-               return (0);
-
-       return (1);
+       return (OF_is_compatible(fa->fa_node, "simple-bus") ||
+           OF_is_compatible(fa->fa_node, "simple-pm-bus"));
 }
 
 void
@@ -108,6 +108,11 @@ simplebus_attach(struct device *parent, 
                    M_TEMP, M_WAITOK);
                OF_getpropintarray(sc->sc_node, "dma-ranges",
                    sc->sc_dmaranges, sc->sc_dmarangeslen);
+       }
+
+       if (OF_is_compatible(sc->sc_node, "simple-pm-bus")) {
+               power_domain_enable(sc->sc_node);
+               clock_enable_all(sc->sc_node);
        }
 
        /* Scan the whole tree. */

Reply via email to