[lkml]   [2008]   [May]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [RFC][PATCH 2/4] RTC: SWARM I2C board initialization
    Hi Jean,

    > Minor corrections which would ideally belong to a separate patch
    > (there's a whole lot more cleanups that could be done in that driver,
    > BTW...)

    Not suprising, as usually with most pieces of code for the SWARM and the
    SiByte SOC. That can be done gradually, but mixing a driver overhaul with
    functional changes usually only results in confusion later on.

    > I don't think that the minor changes below are enough for you to claim
    > copyright on that driver.

    Well, I decide whether or not to add one based on how important changes
    are from the piece's of code point of view. In this case the change is
    essential for new-style client drivers to work at all, which I think is
    more important than e.g. a lot of cosmetical changes throughout would be.
    But I do not insist on keeping it -- if you think I misjudged on this
    occasion, I see no problem with discarding it.

    > Why do you double the space and the end of comments? Never seen that
    > before, and I can't see the idea.

    This is mostly habitual -- this is what the GNU Coding Standard specifies
    for comments and which is enforced for GNU software which I have dealt a
    lot with. I think the idea is it improves readability and I tend to
    agree. The same goes for using a capital at the beginning and a full stop
    at the end of sentences in comments -- it improves readability and
    (together with a good style of code itself) makes the result look more
    professional. Certainly well-formatted code is easier to comprehend for
    someone looking at it for the first time.

    I do not insist on the extraneous space if you have a strong opinion
    against though.

    > I'm not sure how you intend to push these changes upstream. I would
    > take a patch only touching drivers/i2c/busses/i2c-sibyte.c in my i2c
    > tree, however a patch also touching arch code, must be handled be the
    > maintainer for that architecture or platform.

    Andrew has spoken (thank you, Andrew) and I would only like to add an
    explanation why I have not split this change further. Certainly it is
    functionally consistent. Then adding i2c-swarm.c only breaks things as
    the onchip buses suddenly get the numbers 2 and 3. On the other hand, if
    adding the i2c-sibyte.c change only, it will take a while until it
    propagates back to the MIPS tree and without that as it is there is no
    single way to use the whole set of changes as the clock device will not be

    If you are scared off by the MIPS-specific Makefile (lib vs obj) changes,
    then I think they should be reasonably easy to sort out separately in a
    couple of days as functionally not changing anything. The only other file
    in the affected subdirectory that depends on a config option uses
    CONFIG_KGDB which does not seem to rely on being pulled implicitly by the
    linker. But such a mechanical change by itself would make little sense
    (don't fix what isn't broken), so I have not pushed it without a
    reasonable justification.

    Ralf -- what do you think about the Makefile changes? I can send you a
    separate patch which will reduce the span of this one.


     \ /
      Last update: 2008-05-07 23:19    [W:0.038 / U:9.412 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site