svn commit: r328257 - in head/sys: arm/broadcom/bcm2835 dts/arm modules

[Oleksandr Tymoshenko]

Poul-Henning Kamp (phk at phk.freebsd.dk) wrote:
> --------
> In message <8d8ae9d10058fd72ce3ec467181c9f22 at 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