Hi Anton, I've CC-ed devicetree-discuss. The original patch is at http://patchwork.ozlabs.org/patch/40361/ for reference.
On Sat, 2009-12-05 at 23:51 +0300, Anton Vorontsov wrote: > On Sat, Dec 05, 2009 at 01:32:32PM -0600, Peter Tyser wrote: > [...] > > > > Adding a new "fsl,gpio-mask" device tree property allows a dts file to > > > > accurately describe what GPIO pins are available for use on a given > > > > board. > > > > > > I don't see any real usage for this. If device tree specifies a wrong > > > gpio in the gpios = <> property, then it's a bug in the device tree > > > and should be fixed (or workarounded in the platform code). > > > > > > If a user fiddles with unknown gpios via sysfs interface, then it's > > > user's problem. > > > > Its the sysfs case that I'm concerned about. Primarily because: > > 1. Users scratch their head when they see that the "ngpio" sysfs value > > doesn't match their CPU manual or board vendor's manual, and > > subsequently ask their board vendor's engineers (ie me:) what's up. > > I don't think that adding code and device tree entries just for > documentation purposes is a good idea. Its not just for documentation purposes. Right now, the sysfs "ngpio" value is flat out wrong for some processors, regardless of documentation. Granted its not a critical bug, but I'd still consider it a bug. > > 2. Improperly using GPIO pins could damage hardware for some boards. > > Well, your initial patch tried to solve a different problem: to not > let users to request non-existent GPIOs, which is usually safe. I can update the commit message with this rational if it makes a difference. > [...] > > #2 could be worked around by exporting GPIO pins in platform code so > > that they are not available via sysfs. > > Yes, badly designed hardware deserves ugly hacks in the platform > code. ;-) So for this problem, just request these gpios in the > platform code. I'd wager lots of boards have GPIO pins that a user shouldn't play around with once Linux boots up. Like GPIO pins used to program an FPGA, or control a PLL, etc. 1 device tree property is nicer than hacking up lots of platform code... > > Would it be any more acceptable to instead add > > a "fsl,num-gpio" property so that "ngpio" actually reported an accurate > > value and non-existent GPIO pins couldn't be used/exported? > > I'd think it's actually less acceptable. fsl,gpio-mask is more generic, > since from gpio-mask you can deduce ngpio. But it's still ugly. > > What would be OK to do is to describe in the device tree every > device that is using some GPIO, and then let the userspace request > *only* gpios that are described in the device-tree. That way you > can automatically exclude not-existent gpios. > And if some gpios are just headers on the board, you can still > describe them in the device tree via "gpio-header" nodes. > > Still, a lot of efforts for no real gain... Agreed. Seems like a clean solution, but is a chunk of work. In any case, my high-level thought process is: 1. Currently, the "ngpio" value is wrong for some processors and should be fixed. 2. Adding a new "fsl,gpio-mask" gpio solves #1, and has the benefit of allowing the device tree to easily reserve GPIO pins which should not be used in Linux. I guess I'm not seeing the big downside of a new "fsl,gpio-mask" property. Its the device tree's job to describe the hardware. The change is pretty minimal (~15 lines), and the property can be made optional. Or is there another suggestion on how to resolve #1 above? I consider it a bug and would like to fix it. Best, Peter _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
