| Message ID | 20100205203236.GC1475@oksana.dev.rtsoft.ru (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Grant Likely |
| Headers | show |
On Fri, Feb 5, 2010 at 1:32 PM, Anton Vorontsov <avorontsov@ru.mvista.com> wrote: > This patch implements GPIOLIB notifier hooks, and thus makes device-enabled > GPIO chips (i.e. the ones that have gpio_chip->dev specified) automatically > attached to the OpenFirmware subsystem. Which means that now we can handle > I2C and SPI GPIO chips almost* transparently. > > * "Almost" because some chips still require platform data, and for these > chips OF-glue is still needed, though with this support the glue will > be much smaller. > > Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com> > --- > drivers/of/gpio.c | 100 +++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 files changed, 100 insertions(+), 0 deletions(-) > > diff --git a/drivers/of/gpio.c b/drivers/of/gpio.c > index 12c4af0..9d8df77 100644 > --- a/drivers/of/gpio.c > +++ b/drivers/of/gpio.c > @@ -13,6 +13,7 @@ > > #include <linux/kernel.h> > #include <linux/errno.h> > +#include <linux/notifier.h> > #include <linux/io.h> > #include <linux/of.h> > #include <linux/of_gpio.h> > @@ -236,3 +237,102 @@ err0: > return ret; > } > EXPORT_SYMBOL(of_mm_gpiochip_add); > + > +/** > + * of_gpiochip_register_simple - Register a chip with the OF GPIO subsystem > + * @chip pointer to a GPIO chip > + * @np: device node to register the GPIO chip with > + * > + * This function registers a GPIO chip with the OF infrastructure. It is > + * assumed that the chip was previsously allocated and added to a generic > + * GPIOLIB framework (using gpiochip_add() function). > + * > + * The `simple' name means that the chip is using simple two-cells scheme for > + * the gpio-specifier. > + */ > +static int of_gpiochip_register_simple(struct gpio_chip *chip, > + struct device_node *np) > +{ > + struct of_gpio_chip *of_gc; > + > + if (np->data) { > + WARN_ON(1); > + return -EBUSY; > + } > + > + of_gc = kzalloc(sizeof(*of_gc), GFP_KERNEL); > + if (!of_gc) > + return -ENOMEM; > + > + of_gc->gpio_cells = 2; > + of_gc->xlate = of_gpio_simple_xlate; > + of_gc->chip = chip; One concern. How does an OF-aware GPIO driver override these settings? What is to be done when a GPIO chip requires a different xlate hook? Or a different number of gpio_cells? g.
On Fri, Feb 5, 2010 at 1:32 PM, Anton Vorontsov <avorontsov@ru.mvista.com> wrote: > This patch implements GPIOLIB notifier hooks, and thus makes device-enabled > GPIO chips (i.e. the ones that have gpio_chip->dev specified) automatically > attached to the OpenFirmware subsystem. Which means that now we can handle > I2C and SPI GPIO chips almost* transparently. > > * "Almost" because some chips still require platform data, and for these > chips OF-glue is still needed, though with this support the glue will > be much smaller. > > Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com> > --- > +static struct notifier_block of_gpio_nb = { > + .notifier_call = of_gpio_notify, > +}; > + > +static int __init of_gpio_notifier_init(void) > +{ > + return blocking_notifier_chain_register(&gpio_notifier, &of_gpio_nb); > +} > +arch_initcall(of_gpio_notifier_init); Another concern; if any gpio chips get registered before this arch_initcall (not sure if it is possible or not), then those chips won't get registered with the of gpio infrastructure. g.
On Tue, Feb 09, 2010 at 10:08:00AM -0700, Grant Likely wrote: > On Fri, Feb 5, 2010 at 1:32 PM, Anton Vorontsov > <avorontsov@ru.mvista.com> wrote: > > This patch implements GPIOLIB notifier hooks, and thus makes device-enabled > > GPIO chips (i.e. the ones that have gpio_chip->dev specified) automatically > > attached to the OpenFirmware subsystem. Which means that now we can handle > > I2C and SPI GPIO chips almost* transparently. [...] > One concern. > > How does an OF-aware GPIO driver override these settings? What is to > be done when a GPIO chip requires a different xlate hook? Or a > different number of gpio_cells? A lot of options... 1. They can hook up onto a notifier chain, ensure that their callback will be called after OF GPIO subsystem (using notifiers' priority mechanism), and fixup needed stuff. 2. They can write their own full fledged OF bindings, i.e. they will need to allocate of_gc struct and save it into np->data. of_gpiochip_register_simple() won't touch np->data if it's already used. If/when needed, we can write some helper function, i.e. of_gpiochip_register(gpiochip, node, xlate_callback). 3. Or of/gpio.c code will handle this by itself, iff the xlate and gpio-cells scheme seems generic enough. 4. May be more... Thanks,
On Tue, Feb 09, 2010 at 10:13:11AM -0700, Grant Likely wrote: [...] > > +static int __init of_gpio_notifier_init(void) > > +{ > > + return blocking_notifier_chain_register(&gpio_notifier, &of_gpio_nb); > > +} > > +arch_initcall(of_gpio_notifier_init); > > Another concern; if any gpio chips get registered before this > arch_initcall (not sure if it is possible or not), then those chips > won't get registered with the of gpio infrastructure. Technically, it is possible, but registering usual GPIO controllers in arch_initcall feels not quite right approach in the first place (and, btw, it won't work most of the time, because even early drivers do not register itself earlier than subsys_initcall). And arch gpio controllers (like QE GPIO) are usually device-less, and they use of_mm_gpiochip_add(), so we fully control them. Plus I don't see any reason why we couldn't move of_gpio_notifier_init() into, say, postcore_initcall, if we ever need it. Thanks,
On Fri, Mar 5, 2010 at 1:35 PM, Andrew Morton <akpm@linux-foundation.org> wrote: > On Fri, 5 Mar 2010 13:28:32 -0700 > Grant Likely <grant.likely@secretlab.ca> wrote: > >> On Fri, Mar 5, 2010 at 1:00 PM, Andrew Morton <akpm@linux-foundation.org> wrote: >> > On Tue, 9 Feb 2010 22:16:20 +0300 >> > Anton Vorontsov <avorontsov@ru.mvista.com> wrote: >> > >> >> On Tue, Feb 09, 2010 at 10:13:11AM -0700, Grant Likely wrote: >> >> [...] >> >> > > +static int __init of_gpio_notifier_init(void) >> >> > > +{ >> >> > > + __ __ __ return blocking_notifier_chain_register(&gpio_notifier, &of_gpio_nb); >> >> > > +} >> >> > > +arch_initcall(of_gpio_notifier_init); >> >> > >> >> > Another concern; __if any gpio chips get registered before this >> >> > arch_initcall (not sure if it is possible or not), then those chips >> >> > won't get registered with the of gpio infrastructure. >> >> >> >> Technically, it is possible, but registering usual GPIO controllers >> >> in arch_initcall feels not quite right approach in the first place >> >> (and, btw, it won't work most of the time, because even early drivers >> >> do not register itself earlier than subsys_initcall). >> >> >> >> And arch gpio controllers (like QE GPIO) are usually device-less, >> >> and they use of_mm_gpiochip_add(), so we fully control them. >> >> >> >> Plus I don't see any reason why we couldn't move >> >> of_gpio_notifier_init() into, say, postcore_initcall, if we ever >> >> need it. >> >> >> > >> > I'll assume that you're OK with that response. >> >> No, not really, > > You left me dangling :( Sorry, got caught up with other things. >> I'm not really very comfortable with the whole >> approach being taken. And, while I acked the first patch in the >> series, that patch isn't needed by anything except patches 2, 3 & 4. >> >> Also, the OF stuff is a moving target at the moment with all the >> rework is being undertaken. I'd rather let this series sit out for >> another merge cycle so that the underlying OF stuff can settle down. > > OK, please take it up on-list? Okay. I'm making this reply on list. Anton, as I've stated before, I'm not thrilled with the approach. Combine that with the changes being made to drivers/of right now and the addition device tree to ARM and other architectures, my preference is to let this patch series lie fallow for one more merge cycle so that things can settle out in the OF infrastructure code. g.
On Fri, Mar 05, 2010 at 04:47:06PM -0700, Grant Likely wrote: [...] > >> I'm not really very comfortable with the whole > >> approach being taken. And, while I acked the first patch in the > >> series, that patch isn't needed by anything except patches 2, 3 & 4. But you didn't answer my replies, ie were sitting silent like for a month? So you didn't give my any chance to make them comfortable to you. Is there any punishment ready for that? ;-) I see one: apply these patches, and rework this stuff as you like when you have some time? Or tell me your idea, and I'll do the rework for you, in 2.6.35. But in the meantime, these patches can be nicely used to support I2C/SPI GPIO controllers. > >> Also, the OF stuff is a moving target at the moment with all the > >> rework is being undertaken. I'd rather let this series sit out for > >> another merge cycle so that the underlying OF stuff can settle down. > > > > OK, please take it up on-list? > > Okay. I'm making this reply on list. > > Anton, as I've stated before, I'm not thrilled with the approach. Again, great timing for telling that, I must say. Yes, you said it once with some minor arguments (doubts and questions), to which I replied long ago. Then nothing. > Combine that with the changes being made to drivers/of right now and > the addition device tree to ARM and other architectures, my preference > is to let this patch series lie fallow for one more merge cycle so > that things can settle out in the OF infrastructure code. How exactly OF rework affects these patches? And why some rework should be used as an excuse for not adding a hardware support?
On Fri, Mar 5, 2010 at 5:28 PM, Anton Vorontsov <avorontsov@ru.mvista.com> wrote: > On Fri, Mar 05, 2010 at 04:47:06PM -0700, Grant Likely wrote: > [...] >> >> I'm not really very comfortable with the whole >> >> approach being taken. And, while I acked the first patch in the >> >> series, that patch isn't needed by anything except patches 2, 3 & 4. > > But you didn't answer my replies, ie were sitting silent like for > a month? So you didn't give my any chance to make them comfortable > to you. The last version of the patches were posted on Feb 8. -rc8 was released on Feb 12. For changes to common code, that is a little late for getting queued up for the merge window. If it was a subsystem that I maintain, say SPI, then I doubt I would have picked it up for 2.6.34. But I am not the GPIO maintainer. I've stated my case, I'm not fond of the approach, and I'd rather have another merge cycle before committing to the method of making OF gpio bindings more generic. I missed your request to merge this via the powerpc tree and I had higher priority concerns, so I really didn't think much of it. I assumed that David would look at the arguments and make his own decision. For the record, my main concerns are: - Now that I see the implementation, I think that it is too complex. The bus notifiers really aren't needed and it can be done with much lower impact on the core gpiolib code. - Using notifiers adds an unnecessary race condition, however unlikely. > Is there any punishment ready for that? ;-) I see one: apply > these patches, and rework this stuff as you like when you have some > time? Changes to common code don't work that way. Sometimes things just don't get enough attention and they wait another cycle, get reworked, or get dropped entirely. > Or tell me your idea, and I'll do the rework for you, in > 2.6.35. > > But in the meantime, these patches can be nicely used to support > I2C/SPI GPIO controllers. ...and anyone who need it immediately is welcome to pull your changes into their private tree. Skipping a cycle is not the end of the world. >> Combine that with the changes being made to drivers/of right now and >> the addition device tree to ARM and other architectures, my preference >> is to let this patch series lie fallow for one more merge cycle so >> that things can settle out in the OF infrastructure code. > > How exactly OF rework affects these patches? For one, the device node pointer is moving out of archdata into 'struct device' proper and I've got patches adding OF hooks into the core of the platform bus. If those patches look good to GregKH, then I'll be pursing the same pattern for the other bus types (i2c, spi, etc), and it will be further argument for putting the OF hooks directly into gpiolib instead of using a notifier. I'll be posting the patches as soon as the merge window closes. > And why some rework > should be used as an excuse for not adding a hardware support? If this was a standalone device driver then I'd agree. However, this is an infrastructure change. Infrastructure changes get more scrutiny and are always harder to merge. Especially just before the merge window opens with very little linux-next exposure. g.
On Fri, Mar 05, 2010 at 08:54:56PM -0700, Grant Likely wrote: [...] > The last version of the patches were posted on Feb 8. -rc8 was > released on Feb 12. For changes to common code, that is a little late > for getting queued up for the merge window. If it was a subsystem > that I maintain, say SPI, then I doubt I would have picked it up for > 2.6.34. And of course the part of the OF rework, which was first posted for *review* on Feb 03, is a completely different story? 48 files changed, 317 insertions(+), 575 deletions(-) It's in Linus' tree now. And the other part of the OF rework that was posted for review on Feb 13 is another story too? It's in Linus' tree as well. Your patches touch 3 architectures, and a lot of the code that is used by all the OF drivers, still 03 and 13 Feb was OK for them. > But I am not the GPIO maintainer. David is. And I heard only positive feedback on the patches last time. > For the record, my main concerns are: > - Now that I see the implementation, I think that it is too complex. > The bus notifiers really aren't needed and it can be done with much > lower impact on the core gpiolib code. That's a non-argument, what is "lower impact"? Do I touch any hot paths? And if nothing has changed, David (again, the gpiolib maintainer) is happy with the notifiers approach, why would you care? Anyhow, changing the notifier to a direct call is a matter of a trivial patch that we can queue anytime in 2.6.35. > - Using notifiers adds an unnecessary race condition, however unlikely. Where? Is it a real one? I see a lot of race conditions, but none of them because of the notifiers approach. > > Is there any punishment ready for that? ;-) I see one: apply > > these patches, and rework this stuff as you like when you have some > > time? > > Changes to common code don't work that way. Sometimes things just > don't get enough attention and they wait another cycle, get reworked, > or get dropped entirely. See above wrt OF rework patches. > > Or tell me your idea, and I'll do the rework for you, in > > 2.6.35. > > > > But in the meantime, these patches can be nicely used to support > > I2C/SPI GPIO controllers. > > ...and anyone who need it immediately is welcome to pull your changes > into their private tree. Skipping a cycle is not the end of the > world. Using notifiers is not the end of the world either. > >> Combine that with the changes being made to drivers/of right now and > >> the addition device tree to ARM and other architectures, my preference > >> is to let this patch series lie fallow for one more merge cycle so > >> that things can settle out in the OF infrastructure code. > > > > How exactly OF rework affects these patches? > > For one, the device node pointer is moving out of archdata into > 'struct device' proper and I've got patches adding OF hooks into the > core of the platform bus. If those patches look good to GregKH, then > I'll be pursing the same pattern for the other bus types (i2c, spi, > etc), and it will be further argument for putting the OF hooks > directly into gpiolib instead of using a notifier. I'll be posting > the patches as soon as the merge window closes. I don't get it. Why is it a problem to change your patches that ought to be queued for 2.6.*35*? > > And why some rework > > should be used as an excuse for not adding a hardware support? > > If this was a standalone device driver then I'd agree. However, this > is an infrastructure change. Infrastructure changes get more scrutiny > and are always harder to merge. Especially just before the merge > window opens with very little linux-next exposure. See above wrt OF rework patches.
On Fri, Mar 5, 2010 at 10:05 PM, Anton Vorontsov <avorontsov@ru.mvista.com> wrote: > On Fri, Mar 05, 2010 at 08:54:56PM -0700, Grant Likely wrote: > [...] >> The last version of the patches were posted on Feb 8. -rc8 was >> released on Feb 12. For changes to common code, that is a little late >> for getting queued up for the merge window. If it was a subsystem >> that I maintain, say SPI, then I doubt I would have picked it up for >> 2.6.34. > > And of course the part of the OF rework, which was first posted > for *review* on Feb 03, is a completely different story? > > 48 files changed, 317 insertions(+), 575 deletions(-) Completely uncontroversial changes with zero functional behaviour change. There was no uncertainty about these ones and they were posted almost a week earlier. > It's in Linus' tree now. > > And the other part of the OF rework that was posted for review > on Feb 13 is another story too? It's in Linus' tree as well. All cleanups and bugfixes except for "Don't assume HAVE_LMB" which Jeremy had already posted earlier for review. > Your patches touch 3 architectures, and a lot of the code that > is used by all the OF drivers, still 03 and 13 Feb was OK for > them. > >> But I am not the GPIO maintainer. > > David is. And I heard only positive feedback on the patches > last time. > >> For the record, my main concerns are: >> - Now that I see the implementation, I think that it is too complex. >> The bus notifiers really aren't needed and it can be done with much >> lower impact on the core gpiolib code. > > That's a non-argument, what is "lower impact"? Do I touch any > hot paths? And if nothing has changed, David (again, the gpiolib > maintainer) is happy with the notifiers approach, why would you > care? Adding unneeded notifier infrastructure is churn I don't want to see. >> Changes to common code don't work that way. Sometimes things just >> don't get enough attention and they wait another cycle, get reworked, >> or get dropped entirely. > > See above wrt OF rework patches. which all got attention, were uncontroversial, and did not introduce functional changes. >> For one, the device node pointer is moving out of archdata into >> 'struct device' proper and I've got patches adding OF hooks into the >> core of the platform bus. If those patches look good to GregKH, then >> I'll be pursing the same pattern for the other bus types (i2c, spi, >> etc), and it will be further argument for putting the OF hooks >> directly into gpiolib instead of using a notifier. I'll be posting >> the patches as soon as the merge window closes. > > I don't get it. Why is it a problem to change your patches that > ought to be queued for 2.6.*35*? It's not, and they are going to be queued for 2.6.35. In fact, I didn't posted them this week to avoid adding confusion to the merge window. The issues isn't changing my patches. It is that I don't like the notifier approach, and I intend to prove that it can be done in a better way. g.
On Sat, Mar 06, 2010 at 09:43:20AM -0700, Grant Likely wrote: > > On Fri, Mar 05, 2010 at 08:54:56PM -0700, Grant Likely wrote: > > And of course the part of the OF rework, which was first posted > > for *review* on Feb 03, is a completely different story? > > > > 48 files changed, 317 insertions(+), 575 deletions(-) > > Completely uncontroversial changes with zero functional behaviour > change. There was no uncertainty about these ones and they were > posted almost a week earlier. The patches simply touch too many things, so I'd say that the possible breakage impact is on par with the OF GPIO stuff. [...] > > That's a non-argument, what is "lower impact"? Do I touch any > > hot paths? And if nothing has changed, David (again, the gpiolib > > maintainer) is happy with the notifiers approach, why would you > > care? > > Adding unneeded notifier infrastructure is churn I don't want to see. You could reply to my answers earlier and I would change and repost the patches in a jiffy, since I am interested in these patches. But you're obviously not interested in this support since you didn't answer my replies. I'll explain. If you were interested in some support you could give some chance to make patches comfortable to you, and then you could even test them, and maybe defend their inclusion. Look at what an interested person does: http://www.mail-archive.com/linux-mmc@vger.kernel.org/msg00895.html Note that that was v2 with my comments fixed, and that was just seconds before 2.6.33 merge window closed. See? Someone with a direct interest! I gave my comments, they were fixed, and I felt grateful and responsible for pushing the support upstream. But you're not interested in the support, so I don't see why you block it without any good technical reason. And note that not only I'm interested in this support, the I2C/SPI GPIO controllers issue was brought on ml several times by many people. [...] > > I don't get it. Why is it a problem to change your patches that > > ought to be queued for 2.6.*35*? > > It's not, and they are going to be queued for 2.6.35. In fact, I > didn't posted them this week to avoid adding confusion to the merge > window. The issues isn't changing my patches. Then why you mentioned OF rework as some reason to block these patches? > It is that I don't > like the notifier approach, and I intend to prove that it can be done > in a better way. No doubt that you have some better ideas (not to mention that notifiers was your idea as well :-). Here are some technical arguments: 1. You can implement your new ideas on top of the current solution. Or I can happily do that for you. 2. The patches don't change any API, instead they just build a bridge between GPIOLIB and OF GPIO infrastructure. So it's just a matter *taste* how to build that bridge. It's an internal issue of how GPIOLIB and OF GPIO interact.
On Sat, Mar 6, 2010 at 6:47 PM, Anton Vorontsov <avorontsov@ru.mvista.com> wrote: > You could reply to my answers earlier and I would change and > repost the patches in a jiffy, since I am interested in these > patches. > > But you're obviously not interested in this support since you > didn't answer my replies. I'll explain. If you were interested > in some support you could give some chance to make patches > comfortable to you, and then you could even test them, and > maybe defend their inclusion. I'm sorry you feel that way, and I am sorry that I wasn't able to reply earlier. I am interested in generic gpio of support, but you also need to understand that the run up to the merge window is a busy time as I need to collect all the patches that I'm responsible for and get them into linux-next for testing well before the merge window opens. I do not maintain gpiolib, and it was therefore quite low on my priority list at that time. After the merge window closes, I'll have time to be more responsive again. Regardless, this series was not going to be merged through any of my trees because I'm not maintaining gpiolib. Andrew asked me opinion on them, and I gave him my answer. Andrew can make his own decision about whether or not to merge them, but my opinion still stands. I'm sorry that I upset you. It was not my intention. Please keep in mind that it is not unusual for patches to take more than one cycle to get merged. For example, I've got patches out on the ARM list that were posted well before the merge window that have neither received comments, nor will be merged. I'll pursue them again after this merge window closes and other maintainers have more bandwidth to make a good decision about them. g.
On Fri, 5 Feb 2010 23:32:36 +0300 Anton Vorontsov <avorontsov@ru.mvista.com> wrote: > This patch implements GPIOLIB notifier hooks, and thus makes device-enabled > GPIO chips (i.e. the ones that have gpio_chip->dev specified) automatically > attached to the OpenFirmware subsystem. Which means that now we can handle > I2C and SPI GPIO chips almost* transparently. > > ... > > +static int of_gpiochip_register_simple(struct gpio_chip *chip, > + struct device_node *np) Why is this called "register_simple" but the unregistration function isn't called "unregister_simple"? > +{ > + struct of_gpio_chip *of_gc; > + > + if (np->data) { > + WARN_ON(1); > + return -EBUSY; > + } > + > + of_gc = kzalloc(sizeof(*of_gc), GFP_KERNEL); > + if (!of_gc) > + return -ENOMEM; > + > + of_gc->gpio_cells = 2; > + of_gc->xlate = of_gpio_simple_xlate; > + of_gc->chip = chip; > + np->data = of_gc; > + of_node_get(np); > + > + return 0; > +} > +EXPORT_SYMBOL(of_gpiochip_register_simple); Makes no sense to export a static symbol and to provide no declaration of it in a .h file. I assume the export was unintended. My plot is somewhat lost. Grant, could you please summarise in easy-for-akpm-to-understand terms what your issues are with this patchset and how you think we should proceed? Thanks.
On Fri, Mar 12, 2010 at 2:07 PM, Andrew Morton <akpm@linux-foundation.org> wrote: > On Fri, 5 Feb 2010 23:32:36 +0300 > Anton Vorontsov <avorontsov@ru.mvista.com> wrote: > >> This patch implements GPIOLIB notifier hooks, and thus makes device-enabled >> GPIO chips (i.e. the ones that have gpio_chip->dev specified) automatically >> attached to the OpenFirmware subsystem. Which means that now we can handle >> I2C and SPI GPIO chips almost* transparently. >> >> ... >> >> +static int of_gpiochip_register_simple(struct gpio_chip *chip, >> + struct device_node *np) > > Why is this called "register_simple" but the unregistration function > isn't called "unregister_simple"? > >> +{ >> + struct of_gpio_chip *of_gc; >> + >> + if (np->data) { >> + WARN_ON(1); >> + return -EBUSY; >> + } >> + >> + of_gc = kzalloc(sizeof(*of_gc), GFP_KERNEL); >> + if (!of_gc) >> + return -ENOMEM; >> + >> + of_gc->gpio_cells = 2; >> + of_gc->xlate = of_gpio_simple_xlate; >> + of_gc->chip = chip; >> + np->data = of_gc; >> + of_node_get(np); >> + >> + return 0; >> +} >> +EXPORT_SYMBOL(of_gpiochip_register_simple); > > Makes no sense to export a static symbol and to provide no declaration > of it in a .h file. I assume the export was unintended. > > > My plot is somewhat lost. Grant, could you please summarise in > easy-for-akpm-to-understand terms what your issues are with this > patchset and how you think we should proceed? Sure. I suggested the notifier approach in the first place, but now that I see what is required to implement it I don't like it. I also don't like the potential race condition between registering GPIO devices and registering the OF gpio notifier. It is simpler and less code to put the of_gpio hook directly into the GPIO registration path. If CONFIG_OF is not set, then the OF hooks can resolve to empty static inline functions. How to proceed: I'd like to leave this series out for the 2.6.34 cycle and I'll pick it into my OF tree before the 2.6.35 merge window, but I'll probably modify it to call the OF hooks directly and leave out the unnecessary notifier infrastructure. g.
On Fri, Mar 12, 2010 at 02:38:02PM -0700, Grant Likely wrote: [...] > How to proceed: I'd like to leave this series out for the 2.6.34 > cycle and I'll pick it into my OF tree before the 2.6.35 merge window, > but I'll probably modify it to call the OF hooks directly and leave > out the unnecessary notifier infrastructure. Ping?
diff --git a/drivers/of/gpio.c b/drivers/of/gpio.c index 12c4af0..9d8df77 100644 --- a/drivers/of/gpio.c +++ b/drivers/of/gpio.c @@ -13,6 +13,7 @@ #include <linux/kernel.h> #include <linux/errno.h> +#include <linux/notifier.h> #include <linux/io.h> #include <linux/of.h> #include <linux/of_gpio.h> @@ -236,3 +237,102 @@ err0: return ret; } EXPORT_SYMBOL(of_mm_gpiochip_add); + +/** + * of_gpiochip_register_simple - Register a chip with the OF GPIO subsystem + * @chip pointer to a GPIO chip + * @np: device node to register the GPIO chip with + * + * This function registers a GPIO chip with the OF infrastructure. It is + * assumed that the chip was previsously allocated and added to a generic + * GPIOLIB framework (using gpiochip_add() function). + * + * The `simple' name means that the chip is using simple two-cells scheme for + * the gpio-specifier. + */ +static int of_gpiochip_register_simple(struct gpio_chip *chip, + struct device_node *np) +{ + struct of_gpio_chip *of_gc; + + if (np->data) { + WARN_ON(1); + return -EBUSY; + } + + of_gc = kzalloc(sizeof(*of_gc), GFP_KERNEL); + if (!of_gc) + return -ENOMEM; + + of_gc->gpio_cells = 2; + of_gc->xlate = of_gpio_simple_xlate; + of_gc->chip = chip; + np->data = of_gc; + of_node_get(np); + + return 0; +} +EXPORT_SYMBOL(of_gpiochip_register_simple); + +/** + * of_gpiochip_unregister - Unregister a GPIO chip + * @chip pointer to a GPIO chip + * @np: device node for which the GPIO chip was previously registered + * + * This function unregisters a GPIO chip that was previsously registered + * with of_gpiochip_register*(). + */ +static int of_gpiochip_unregister(struct gpio_chip *chip, + struct device_node *np) +{ + struct of_gpio_chip *of_gc = np->data; + + if (!of_gc || of_gc->chip != chip) { + WARN_ON(1); + return -EINVAL; + } + + np->data = NULL; + kfree(of_gc); + of_node_put(np); + + return 0; +} + +static int of_gpio_notify(struct notifier_block *nb, unsigned long msg, + void *chip) +{ + struct gpio_chip *gc = chip; + struct device_node *np; + int ret = 0; + + if (!gc->dev) + return NOTIFY_DONE; + + np = dev_archdata_get_node(&gc->dev->archdata); + if (!np) + return NOTIFY_DONE; + + switch (msg) { + case GPIO_NOTIFY_CHIP_ADDED: + ret = of_gpiochip_register_simple(gc, np); + break; + case GPIO_NOTIFY_CHIP_REMOVE: + ret = of_gpiochip_unregister(gc, np); + break; + default: + break; + } + + return ret ? notifier_from_errno(ret) : NOTIFY_OK; +} + +static struct notifier_block of_gpio_nb = { + .notifier_call = of_gpio_notify, +}; + +static int __init of_gpio_notifier_init(void) +{ + return blocking_notifier_chain_register(&gpio_notifier, &of_gpio_nb); +} +arch_initcall(of_gpio_notifier_init);
This patch implements GPIOLIB notifier hooks, and thus makes device-enabled GPIO chips (i.e. the ones that have gpio_chip->dev specified) automatically attached to the OpenFirmware subsystem. Which means that now we can handle I2C and SPI GPIO chips almost* transparently. * "Almost" because some chips still require platform data, and for these chips OF-glue is still needed, though with this support the glue will be much smaller. Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com> --- drivers/of/gpio.c | 100 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 100 insertions(+), 0 deletions(-)