Re: [RFC][PATCH 2/4] RTC: SWARM I2C board initialization

[Maciej W. Rozycki]

Hi Jean,

> If you had to add a missing semicolon to a source file to get it to
> build again, it would be an "essential" change (without it nothing
> works) but still, you can't claim you added any intellectual value to

 Agreed about this example because the change is mechanical and can almost
be done by an automaton.

> the source file. So, no copyright. The copyright is about how much
> value you add, not how important the change is in the big picture.

 As I wrote, no big concern about it from my side and it looks I will be 
changing more of the file anyway. ;-)

> I'm not going to tell how bad I think the GNU coding standards are, the
> point here is that we don't follow them at all, so whatever they say is
> totally irrelevant. Read Documentation/CodingStyle, it describes what

 Oh come on -- that's just common sense.  If something is good, there is
no point in discarding it without thinking, just because it is a part of a
bigger entity that we consider bad.  I consider it good not because it is
a part of the GNU standard, but because I have concluded that it is and it
is pure coincidence ;-) I have taken it from the said standard.  But as I
said, this is a minor nit here and I can resist from adding extraneous
spaces in pieces of code you are interested in as long as I am able to
track which ones they actually are.

> we do. Also make sure that you run your patches through
> scripts/checkpatch.pl. The rest is up to you, but in general, when
> modifying existing code, you want to stick to what the surrounding code
> looks like.

 I had no problems with writing code checkpatch.pl would swallow without a
blink even before it existed.  It does not mean I should follow the
surrounding mess if this is what the state of code is.

> I do insist ;) Admittedly, double spaces at end of comments are used in
> some places in the kernel tree (I had never seen that before), but they
> are still outnumbered by single-space ending comments, 50 to 1. Do
> what you want in the drivers your create or maintain, but please don't
> change existing comments, especially not in the middle of functional
> changes.

 Fine with me, no problem.

> That's not a matter of being scared, and I was also _not_ asking you to
> split the patch. That's a matter of synchronizing merges between me and
> the architecture maintainer. If I take a patch in my i2c tree which
> touches architecture-specific files, and I only push it to Linus in 2
> months, then chances are that the architecture-specific files in
> question will change several times meanwhile, resulting in conflicts in
> -next and -mm. I am only trying to prevent this from happening. I
> simply think that it is easier to synchronize patches if all
> architecture-specific patches go through the relevant architecture tree.

 Your concern is valid and this is why I proposed the split.  My point
being, unlike the rest of the MIPS arch tree these days the Broadcom bits
are only touched from time to time by two or three people (myself
included), so it is much easier to coordinate changes if they are limited 
to this subarch.  Which means stripping out changes needed elsewhere from 
the i2c patch itself can only improve things.

> BTW, SWARM seems to be only one of the 4 SiByte platforms we support.

 I think nobody really knows for sure how many Broadcom/SiByte platforms 
we support at the moment. ;-)  I am fairly sure there is some interest in 
the BigSur, SWARM and Sentosa only.

> What about the other ones? Your changes to the i2c-sibyte driver could
> cause the i2c bus registration to fail, as the other platforms do not
> declare I2C devices, the bus numbers 0 and 1 won't be reserved by
> i2c-core. Care to comment on this?

 Well, arch/mips/sibyte/swarm/ is included for all the three above as well 
as a couple of other I may not necessarily be sure what they are even.  So 
this should be of no concern.

 BTW, do you mean i2c_add_numbered_adapter() will fail if no devices have
been declared to exist on the given bus with i2c_register_board_info()?  
That sounds strange...  Note in all cases there are EEPROMs (onboard ones
as well as optionally SPD ones) on both buses on Broadcom/SiByte boards
and they are handled by a legacy client driver.  The Broadcom SOC is
actually capable to bootstrap from one of these EEPROMs (rather than form
the usual system parallel Flash ROM).

  Maciej
--
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/