Date: Sun, 28 Jan 2018 22:39:50 -0800 From: Oleksandr Tymoshenko <gonzo@bluezbox.com> To: Poul-Henning Kamp <phk@phk.freebsd.dk> Cc: Emmanuel Vadot <manu@bidouilliste.com>, Warner Losh <imp@bsdimp.com>, John Baldwin <jhb@freebsd.org>, Ravi Pokala <rpokala@mac.com>, src-committers <src-committers@freebsd.org>, svn-src-all@freebsd.org, svn-src-head@freebsd.org, owner-src-committers@freebsd.org Subject: Re: svn commit: r328257 - in head/sys: arm/broadcom/bcm2835 dts/arm modules Message-ID: <20180129063950.GA59901@bluezbox.com> In-Reply-To: <13025.1517179897@critter.freebsd.dk> References: <201801220710.w0M7AUm9091853@repo.freebsd.org> <CANCZdfpq2QoG4EAj0VW2FF=4VXRv-qQKFfJTJerWH9YOwVoVBA@mail.gmail.com> <90451.1516663240@critter.freebsd.dk> <2987003.eeGRFBb6N8@ralph.baldwin.cx> <CANCZdfrh0NHq7cbkq_genEdzo%2BB3G4TTAcEzpgh11sr%2B82e9aw@mail.gmail.com> <93949.1516733748@critter.freebsd.dk> <20180127210801.37b8001125dd0a2c92372f98@bidouilliste.com> <72042.1517094867@critter.freebsd.dk> <8d8ae9d10058fd72ce3ec467181c9f22@megadrive.org> <13025.1517179897@critter.freebsd.dk>
next in thread | previous in thread | raw e-mail | index | archive | help
Poul-Henning Kamp (phk@phk.freebsd.dk) wrote: > -------- > In message <8d8ae9d10058fd72ce3ec467181c9f22@megadrive.org>, Emmanuel Vadot writes: > > > Sometimes it makes sense to reboot. > > Yes, *sometimes* it does. > > But *always* demanding reboot makes no sense ever. > > > Reference platform doesn't make much sense in the embedded world. > > A reference platform which peple can look at to find out what the > software architecture is supposed to be, in the near total absense of > documentation for said software architecture makes a lot of sense. > > > I'm not upset at you for not using, I'm "upset" at you for not wanting > >to make the effort to implement them. Some are hard, some are easy. > > FreeBSD is a hobby for me these days, that implied a certain amount > of enjoyment and limited time. > > Trying to guess what software architecture you want to be written, > based on the non-existent documentation and with no reference-platform > to look at, and then implementing it on a SOC where the hardware > documentation spans the gamut from from missing over mangled to > misleading, does not qualify as "enjoyment" for me and it certainly > is not something I have time for. > > > What's funny though is that even with a pinctrl and clock management, > >we still don't have what is necessary to implement what you want > >(kldloading a driver and directly use pwm). For that we need overlays at > >runtime, pinmuxing at runtime and probably other things too. > > I'm amazed if those things are not already part of our ambition ? > > > This is where I (and probably) other don't agree, this is backward. > > We must implement first proper pinctrl driver and clock management > > instead of introduce hacks. > > Who exactly are "We" ? > > You indicated that you are not going to do it. > > I can't because I don't know what it is that I am supposed to write. > > Nobody else seems to be inclined to do it either. > > So RPi as a platform is just in limbo forever ? > > And where does this "Spanish Inquisition" road end? > > Why are gpio and spi allowed to exist on the RPi platform? Or is > your next demand going to be that they also be removed pending a > hypothetical pinctrl driver ? So to stear discussion away from heated emotional exchange and to the more practical side. As far as I understand there are three contentious points: pinctl, clock management, ignoring "status" property I2C and SPI drivers predate pinctl and proper FDT support. pin config part there is ugly as hell and should have been re-implemented long ago. Here is the patch that fixes these and PWM drivers: https://reviews.freebsd.org/D14104 I don't know enough about extres/clk framework but I know how to read code and where to look for references so I can document it. RPi3 DTS bindings might be not compatible with our implementation of extres/clk but I can check. Will it help you to fix clkman driver in foreseeable future? Ignoring value of ofw_bus_status_okay(dev) in probe method is wrong for number of reasons and pinctl and clocks were brought up as an example why "status" property is more than just attach/dont-attach flag. So there is strictly technical reason why it should be checked. Once this boilerplate code moves to simplebus bus probe method as I believe Warner proposed drivers will not be able to control this aspect any more. There is blessed way to enable drivers on FDT-based platforms: overlays. There is a bunch of precompiled blobs for RPi provided by vendor[1]. To enable PWM on specific pin you need to add following line to config.txt on boot partition: dtoverlay=pwm,pin=12,func=4 It's documented here[2]. With proposed pinctl patch your driver can work on any configurabe pin. We do not include these overlays in official snapshots and I think this should be fixed. This approach is not optimal but actual solution for dynamic device management is complex and requires multiple steps. There are some drivers in arm/broadcom/bcm2835 that ignore status value, they should be fixed too. Probably on other platforms too. I'll look into this to lead the way so to say. Summary: adding ofw_bus_status_okay check in probe method doesn't require any additional functionality, ignoring it is inconsistent with majority of FDT-based drivers' behavior. There is trivial way to enable PWM device in platform-conformant way. Will you please commit fix for this bug? All these best practices and guidelines are unwriteen, and they're not always implemented on older platforms. And it's the problem from which this situation has risen. They exist as an "oral tradition" among people who extencively work with FDT-based platforms and communicate in reviews or mailing list. There is no entry in MAINTAINERS, no official developer's guide to embedded systems, documentation exists mostly in a form of source code. May be it can be improved. Where should they be codified or documented (except man pages) to be authoritative source? Where would you look for such information before starting new driver with no prior experience in the area? [1] https://github.com/raspberrypi/firmware/blob/master/boot/overlays/README [2] https://github.com/raspberrypi/firmware/blob/master/boot/overlays/README#L1267 -- gonzo
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20180129063950.GA59901>