Re: [PATCH 3/4] of/gpio: Implement GPIOLIB notifier hooks

From: Grant Likely
Date: Fri Mar 05 2010 - 22:55:25 EST


On Fri, Mar 5, 2010 at 5:28 PM, Anton Vorontsov
<avorontsov@xxxxxxxxxxxxx> 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/