Hi Anton, Thanks for the feedback. > On Fri, Dec 04, 2009 at 01:43:40PM -0600, Peter Tyser wrote: > > This change resolves 2 issues: > > - Different chips have a different number of GPIO pins per controller. > > For example, the MPC8347 has 32, the P2020 16, and the mpc8572 8. > > Previously, the mpc8xxx_gpio driver assumed every chip had 32 GPIO > > pins which resulted in some processors reporting an incorrect 'ngpio' > > field in /sys. Additionally, users could export and "use" 32 GPIO > > pins, although in reality only a subset of the 32 pins had any real > > functionality. > > > > - Some boards don't utilize all available GPIO pins. Previously, > > unused GPIO pins could still be exported and "used", even though the > > pins had no real functionality. This is somewhat confusing to a user > > and also allow a user to do something "bad", like change an unused > > floating output into a floating input. > > There are hundreds of other ways to screw things up. > > Think of /dev/mem, you still able to change the registers. > Before changing any GPIO (whether it is a normal or reserved GPIO), > user has to consult with schematics/docs.
Agreed. This is an attempt to make it just a little bit harder to accidentally screw things up and to make the "ngpio" sysfs value actually contain an accurate value. > > 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. 2. Improperly using GPIO pins could damage hardware for some boards. For example, some of our boards have a voltage regulator controlled via GPIO pins so that a CPU's core voltage can be changed based on its frequency, etc. A user could damage their CPU if they aren't careful with those GPIO pins. For pins like that, it'd be nice to not even let users play with them. #2 could be worked around by exporting GPIO pins in platform code so that they are not available via sysfs. And I agree that if a user is playing with GPIO pins, they had better know what they are doing, so #1 above is my main issue. 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? With the patch as is, if "fsl,gpio-mask" is not given, the driver defaults to enabling all 32 gpio pins. Would it be any better if I respun the patch to only add the "fsl,gpio-mask" property for the mpc8572, p2020, and mpc8379 boards which have less than 32 gpio pins and document the dts property as optional? Thanks, Peter _______________________________________________ Linuxppc-dev mailing list [email protected] https://lists.ozlabs.org/listinfo/linuxppc-dev
