[U-Boot] M28: GPIO pin validity check added

This patch adds a validity check for GPIO pins to prevent clobbering reserved bit or even registers, per suggestion of Marek Vasut.

Please note:
1. gpio_request no longer checks validity. Pin validity must be checked explicitly use gpio_invalid prior to request and hence use. Previous validity check in gpio_request was incomplete anyway.
2. MX233 is not supported yet. Until it is, macros defining the number of pins for each bank reside in arch/arm/include/asm/arch-mx28/iomux.h

Signed-off-by: Robert Deliën <robert at delien.nl>

On Wednesday 23 November 2011 10:08:58 Robert Deliën wrote:
> 1. gpio_request no longer checks validity. Pin validity must be checked
> explicitly use gpio_invalid prior to request and hence use.

NAK.  the u-boot gpio API follows the Linux API which means gpio_request() 
does validation.

your patches are still incorrectly formatted.  please read the git manual for 
how this is supposed to be done.
-mike

> On Wednesday 23 November 2011 10:08:58 Robert Deliën wrote:
>> 1. gpio_request no longer checks validity. Pin validity must be checked
>> explicitly use gpio_invalid prior to request and hence use.
> 
> NAK.  the u-boot gpio API follows the Linux API which means gpio_request() 
> does validation.

Can do.

> your patches are still incorrectly formatted.  please read the git manual for 
> how this is supposed to be done.

Can you offer a little more concrete help instead of suggesting to read 100's of pages of manual?

> This patch adds a validity check for GPIO pins to prevent clobbering
> reserved bit or even registers, per suggestion of Marek Vasut.
> 
> Please note:
> 1. gpio_request no longer checks validity. Pin validity must be checked
> explicitly use gpio_invalid prior to request and hence use. Previous
> validity check in gpio_request was incomplete anyway. 2. MX233 is not
> supported yet. Until it is, macros defining the number of pins for each
> bank reside in arch/arm/include/asm/arch-mx28/iomux.h
> 
> Signed-off-by: Robert Deliën <robert at delien.nl>
> 
> diff --git a/arch/arm/include/asm/arch-mx28/gpio.h
> b/arch/arm/include/asm/arch-mx28/gpio.h index be1c944..36f3be8 100644
> --- a/arch/arm/include/asm/arch-mx28/gpio.h
> +++ b/arch/arm/include/asm/arch-mx28/gpio.h
> @@ -25,6 +25,10 @@
> 
> #ifdef	CONFIG_MXS_GPIO
> void mxs_gpio_init(void);
> +
> +extern int gpio_invalid(int gp);
> +#define gpio_invalid(gpio)	gpio_invalid(gpio)
> +
> #else
> inline void mxs_gpio_init(void) {}
> #endif
> diff --git a/arch/arm/include/asm/arch-mx28/iomux.h
> b/arch/arm/include/asm/arch-mx28/iomux.h index 7abdf58..2326fdb 100644
> --- a/arch/arm/include/asm/arch-mx28/iomux.h
> +++ b/arch/arm/include/asm/arch-mx28/iomux.h
> @@ -56,6 +56,16 @@ typedef u32 iomux_cfg_t;
> #define MXS_PAD_PULL_VALID_SHIFT 16
> #define MXS_PAD_PULL_VALID_MASK	((iomux_cfg_t)0x1 <<
> MXS_PAD_PULL_VALID_SHIFT)
> 
> +#define MX28_BANK0_PINS		29
> +#define MX28_BANK1_PINS		32
> +#define MX28_BANK2_PINS		28
> +#define MX28_BANK3_PINS		31
> +#define MX28_BANK4_PINS		21
> +
> +#define MX23_BANK0_PINS		32
> +#define MX23_BANK1_PINS		31
> +#define MX23_BANK2_PINS		32
> +

Do we need this in the header file?

> #define PAD_MUXSEL_0		0
> #define PAD_MUXSEL_1		1
> #define PAD_MUXSEL_2		2
> diff --git a/common/cmd_gpio.c b/common/cmd_gpio.c
> index 9cc790a..f81c7d8 100644
> --- a/common/cmd_gpio.c
> +++ b/common/cmd_gpio.c
> @@ -15,6 +15,10 @@
> #define name_to_gpio(name) simple_strtoul(name, NULL, 10)
> #endif
> 
> +#ifndef gpio_invalid
> +#define gpio_invalid(gpio)	(0)
> +#endif
> +
> enum gpio_cmd {
> 	GPIO_INPUT,
> 	GPIO_SET,
> @@ -56,6 +60,12 @@ static int do_gpio(cmd_tbl_t *cmdtp, int flag, int argc,
> char * const argv[]) if (gpio < 0)
> 		goto show_usage;
> 
> +	/* check bank and pin number for validity */
> +	if (gpio_invalid(gpio)) {

gpio_is_valid() might be a better name?

> +		printf("gpio: invalid pin %u\n", gpio);
> +		return (-1);

return -EINVAL;

> +	}
> +
> 	/* grab the pin before we tweak it */
> 	if (gpio_request(gpio, "cmd_gpio")) {
> 		printf("gpio: requesting pin %u failed\n", gpio);
> diff --git a/drivers/gpio/mxs_gpio.c b/drivers/gpio/mxs_gpio.c
> index b7e9591..d2e9bac 100644
> --- a/drivers/gpio/mxs_gpio.c
> +++ b/drivers/gpio/mxs_gpio.c
> @@ -31,7 +31,6 @@
> #include <asm/arch/imx-regs.h>
> 
> #if	defined(CONFIG_MX23)
> -#define	PINCTRL_BANKS		3
> #define	PINCTRL_DOUT(n)		(0x0500 + ((n) * 0x10))
> #define	PINCTRL_DIN(n)		(0x0600 + ((n) * 0x10))
> #define	PINCTRL_DOE(n)		(0x0700 + ((n) * 0x10))
> @@ -39,7 +38,6 @@
> #define	PINCTRL_IRQEN(n)	(0x0900 + ((n) * 0x10))
> #define	PINCTRL_IRQSTAT(n)	(0x0c00 + ((n) * 0x10))
> #elif	defined(CONFIG_MX28)
> -#define	PINCTRL_BANKS		5
> #define	PINCTRL_DOUT(n)		(0x0700 + ((n) * 0x10))
> #define	PINCTRL_DIN(n)		(0x0900 + ((n) * 0x10))
> #define	PINCTRL_DOE(n)		(0x0b00 + ((n) * 0x10))
> @@ -57,11 +55,25 @@
> #define GPIO_INT_LEV_MASK	(1 << 0)
> #define GPIO_INT_POL_MASK	(1 << 1)
> 
> +static const int mxs_bank_pins[] = {
> +#if	defined(CONFIG_MX23)
> +	MX23_BANK0_PINS,
> +	MX23_BANK1_PINS,
> +	MX23_BANK2_PINS
> +#elif	defined(CONFIG_MX28)
> +	MX28_BANK0_PINS,
> +	MX28_BANK1_PINS,
> +	MX28_BANK2_PINS,
> +	MX28_BANK3_PINS,
> +	MX28_BANK4_PINS
> +#endif
> +};

Why not move it above, since the ifdef CONFIG_MX28 etc. are already there and 
define it twice. It'd be clearer. Also, it's quite obvious those are pin counts, 
so why introduce these defines and not put only plain numbers here. The 
MX28_BANK0_PINS etc are used only here anyway.

> +
> void mxs_gpio_init(void)
> {
> 	int i;
> 
> -	for (i = 0; i < PINCTRL_BANKS; i++) {
> +	for (i = 0; i < ARRAY_SIZE(mxs_bank_pins); i++) {
> 		writel(0, MXS_PINCTRL_BASE + PINCTRL_PIN2IRQ(i));
> 		writel(0, MXS_PINCTRL_BASE + PINCTRL_IRQEN(i));
> 		/* Use SCT address here to clear the IRQSTAT bits */
> @@ -69,6 +81,16 @@ void mxs_gpio_init(void)
> 	}
> }
> 
> +int gpio_invalid(int gp)
> +{
> +	if ( (gp & ~(MXS_PAD_BANK_MASK | MXS_PAD_PIN_MASK)) ||
> +	     (PAD_BANK(gp) >= ARRAY_SIZE(mxs_bank_pins)) ||
> +	     (PAD_PIN(gp) >= mxs_bank_pins[PAD_BANK(gp)]) )
> +		return (-EINVAL);
> +
> +	return (0);
> +}
> +
> int gpio_get_value(int gp)
> {
> 	uint32_t bank = PAD_BANK(gp);
> @@ -120,9 +142,6 @@ int gpio_direction_output(int gp, int value)
> 
> int gpio_request(int gp, const char *label)
> {
> -	if (PAD_BANK(gp) > PINCTRL_BANKS)
> -		return -EINVAL;
> -
> 	return 0;
> }

M

>> +#define MX28_BANK0_PINS		29
>> +#define MX28_BANK1_PINS		32
>> +#define MX28_BANK2_PINS		28
>> +#define MX28_BANK3_PINS		31
>> +#define MX28_BANK4_PINS		21
>> +
>> +#define MX23_BANK0_PINS		32
>> +#define MX23_BANK1_PINS		31
>> +#define MX23_BANK2_PINS		32
>> +
> 
> Do we need this in the header file?

Not really, but I hate magic numbers and macros are free.

>> +	/* check bank and pin number for validity */
>> +	if (gpio_invalid(gpio)) {
> 
> gpio_is_valid() might be a better name?

Yes, it is. In fact, I used that name at first. But it's non-statice, accessible externally, so it should return -EINVAL in case the pin is not valid and 0 in case it is. That would lead to this weird construction when actually using it:
…
if (gpio_is_valid(gp)) {
	printf("Pin is invalid\n");
}
...

>> +		printf("gpio: invalid pin %u\n", gpio);
>> +		return (-1);
> 
> return -EINVAL;

Did that too initially, but U-Boot produced an error about not exiting the shell when returned that value. Since the rest of the code returns -1 on error everywhere, I decided to copy that.

> Why not move it above, since the ifdef CONFIG_MX28 etc. are already there and 
> define it twice. It'd be clearer.

Agreed; I'll probably slip that in later, with another patch. 

> Also, it's quite obvious those are pin counts, 
> so why introduce these defines and not put only plain numbers here. The 
> MX28_BANK0_PINS etc are used only here anyway.

You're right, but I just don't like magic numbers.

By the way: I started testing gpio on my evk board, but I didn't see any pins being toggled. Not even before all of my patches. Is this part finished/tested?

Robert.

On Wednesday 23 November 2011 15:10:29 Robert Deliën wrote:
> > On Wednesday 23 November 2011 10:08:58 Robert Deliën wrote:
> > your patches are still incorrectly formatted.  please read the git manual
> > for how this is supposed to be done.
> 
> Can you offer a little more concrete help instead of suggesting to read
> 100's of pages of manual?

i've already told you multiple times but you're simply ignoring it.  the 
biggest thing is your broken e-mail client not wrapping lines.

the patch itself fails to follow git:
	http://www.denx.de/wiki/U-Boot/Patches

i don't know why you're hand writing these things instead of using proper git 
tools like format-patch and send-email.
-mike

On Wednesday 23 November 2011 10:08:58 Robert Deliën wrote:
> +extern int gpio_invalid(int gp);

if you really want to add this function, the Linux GPIO API uses:
	bool gpio_is_valid(int number);

but that doesn't negate the gpio_request() func from having to validate the 
pin given to it, so i don't see much point in providing this at this time ... 
no code should do something like:
	if (!gpio_is_valid())
		return error;
	if (gpio_request())
		return error;
-mike

sorry if my messages have been a bit harsh with gpio freedback.  the unwrapped 
lines and broken patch formats make me see red.
-mike

> sorry if my messages have been a bit harsh with gpio freedback.  the unwrapped 
> lines and broken patch formats make me see red.

I understand; There are rules to adhere to.
But please also understand what I'm working with here. Making a single patch takes me - I kid you not - one hour. And still it comes out borked.

We are obliged to strictly work under Windows. Laptops with full partition true crypt avoids dual boot and gets the i7 quad-core on it's knees like a heroine hooker. On that base I'm running VM-ware...

At the office we're using Perforce. Not a bad system, but it doesn't deal with patches very well and it's inaccessible from my home office. So I'm using Subversion on my NAS as an intermediate archive to work from, committing my changes to P4 every now and then. You guys are using Git; I'm sure it's superior but I don't have time now to figure that one beyond the basics. Every time I need to make or re-make a patch, I have to clone a clean Git repository, revert my SVN workspace back to the work I'm making a patch off, merge that into the clean Git repository and create a new patch.

And I still have to mail the patch. No attachments allowed, I understand, but copying and pasting between a Linux VM and Windows host trashes the leading space, so I either have to go through the patch and put it back myself. Or I can use Exchange web-based mail, that sometimes decides to send HTML mail anyway.

In my home office I'm using a 12-core MacPro as a workstation. Again Linux is under a VM, because I need a Windows VM too, to have crippled access to the office network. To post patches from my home office, I'm using Mac Mail through my own windows server, but Mail somehow doesn't offer the feature to wrap lines at column 72. Instead it uses the format designator to indicate flow-format, understood by any modern email program, but not anticipating people on the other side receiving email in Vim, or applying patches directly from their mailbox.

So even though I had finally convinced management that it's in their best interest to get our work back into the mainline, I decided to bail out because it's just too much work. Even though not ideal, I think we're better off maintaining our 100 lines worth of work. And even if all technical difficulties could be fixed - I'll express myself mildly, but I cannot leave this unsaid - the social standards here are not really my thing.

Robert.

Dear Robert,

In message <EED98926-42E6-49D6-9F55-0FD2F2ED59B7@delien.nl> you wrote:
> > sorry if my messages have been a bit harsh with gpio freedback.  the unwrapped 
> > lines and broken patch formats make me see red.
> 
> I understand; There are rules to adhere to.
> But please also understand what I'm working with here. Making a single patch takes me - I kid you not - one hour. And still it comes out borked.

Part of this problem might be the result from an attitude of "I don't
have the time to learn doing thisngs right, but I always have time to
repeat them wrongly again and again".

For example, you still haven't figured out how to configure your
mailer so it wraps lines after some 70 characters or so.

...
> At the office we're using Perforce. Not a bad system, but it doesn't deal with patches very well and it's inaccessible from my home office. So I'm using Subversion on my NAS as an intermediate archive to work from, committing my changes to P4 every now 
> and then. You guys are using Git; I'm sure it's superior but I don't have time now to figure that one beyond the basics. Every time I need to make or re-make a patch, I have to clone a clean Git repository, revert my SVN workspace back to the work I'm m
> aking a patch off, merge that into the clean Git repository and create a new patch.

This is something that I really cannot understand.  If I find myself
in such a situation, I would probably start tinking what can be done
to avoid such pain when I do this the second time.  If it's any
significant amount of time I would nover do it a third time, but
rather think of alternatives.

For example, why can't you keep a permanent git tree on your system,
and import your SVN stuff into a branch in that repository.

Yes, this might involve learning git, but each hour invested in that
is _much_ better spent then performing repeated, mechanical work.

> And I still have to mail the patch. No attachments allowed, I understand, but copying and pasting between a Linux VM and Windows host trashes the leading space, so I either have to go through the patch and put it back myself. Or I can use Exchange web-b
> ased mail, that sometimes decides to send HTML mail anyway.

Did you read the available instructions for sending patches, or for
configuring mailers not to mess with your code?

> In my home office I'm using a 12-core MacPro as a workstation. Again Linux is under a VM, because I need a Windows VM too, to have crippled access to the office network. To post patches from my home office, I'm using Mac Mail through my own windows serv
> er, but Mail somehow doesn't offer the feature to wrap lines at column 72. Instead it uses the format designator to indicate flow-format, understood by any modern email program, but not anticipating people on the other side receiving email in Vim, or ap
> plying patches directly from their mailbox.

Again, there are _tons_ of checklists on the web that explain how to
configure your MUA - not to mention that there is wide choice of MUA
for basicly all systems.

> So even though I had finally convinced management that it's in their best interest to get our work back into the mainline, I decided to bail out because it's just too much work. Even though not ideal, I think we're better off maintaining our 100 lines w
> orth of work. And even if all technical difficulties could be fixed - I'll express myself mildly, but I cannot leave this unsaid - the social standards here are not really my thing.

Friction usually requires two sides...

Best regards,

Wolfgang Denk

Hi Wolfgang,

How nice of you to drop a message, I really appreciate it.

> Part of this problem might be the result from an attitude of "I don't
> have the time to learn doing thisngs right, but I always have time to
> repeat them wrongly again and again".

In the end, doing things right always pays, but that's not always obvious
at the beginning. I figured that I could just hand-craft these three
patches in three hours and be done with it. Just for that, switching from
SVN to GIT didn't seem worth the effort.
Now it turns out that whole the mxs gpio subsystem just doesn't work
(consider the irony of bickering about tabs and spaces), so I'll may send
my work to Marek and have him decide whether he will put it to use or
throw it away.

> For example, you still haven't figured out how to configure your
> mailer so it wraps lines after some 70 characters or so.

Exchange can do it, but copying/pasting to exchange damages it.
Exchange Webmail cannot do it, neither can Mac Mail: The option just
isn't there.
This message is hand-wrapped; our of respect.

> This is something that I really cannot understand.  If I find myself
> in such a situation, I would probably start tinking what can be done
> to avoid such pain when I do this the second time.  If it's any
> significant amount of time I would nover do it a third time, but
> rather think of alternatives.

The point is that every time I do it, I think it's the last time I need to, so
there's no point in finding a structural solution. In hindsight however...

> Yes, this might involve learning git, but each hour invested in that
> is _much_ better spent then performing repeated, mechanical work.

It's good to get used to GIT, it's just that timing is a bit inconvenient
right now. In two months from now, when our A-sample is brought
up and all subsystems are working, things will look differently.

> Did you read the available instructions for sending patches, or for
> configuring mailers not to mess with your code?

I've read the instructions on your page on creating and submitting
patches. I have adhered where I could, but unfortunately I couldn't
get GIT to talk to our corporate mail server. I still need to reconfigure
my own Exchange 2010 server to accept mail from GIT, but I lack
some knowledge there (and proud of it).

> Again, there are _tons_ of checklists on the web that explain how to
> configure your MUA - not to mention that there is wide choice of MUA
> for basicly all systems.

I have searched the web, but Mail really doesn't support it. Apple is
using format=flow and assumes everybody handles that properly. 

> Friction usually requires two sides…

That's very Zen-like. And between you and me; Yes, we've had our
history there, if you remember. But I have learned from that and tried
to do things differently, trying to be cooperative, modest and accepting
the Custodian's authorities. But still I didn't feel appreciated or even
welcome. I don't need gratitude, but I do deserve the same social
courtesy as people do in real life. That's what I try to do.
For example, if somebody - who you have never met before - is trying
to help you in real life, would you respond like this if the help turns out
to be a bit clumsy:

> AARGH! Please add the following lines to the commit message:
> 
> Cc: Marek Vasut <marek.vasut@gmail.com>
> Cc: Stefano Babic <sbabic@denx.de>
> 
> !!!!

Even without politenesses like 'sorries' and 'pleases', this looks a lot
friendlier and is educational at the same time:

Next time, include the lines below in the body of your message, and the
mailing list will take care of the rest:
Cc: Marek Vasut <marek.vasut@gmail.com>
Cc: Stefano Babic <sbabic@denx.de>

No caps-lock nor multiple exclamation marks involved.
Or how about this:

> sorry if my messages have been a bit harsh with gpio freedback.  the unwrapped 
> lines and broken patch formats make me see red.

I took it as an informal apology, lightened up with a joke. But if it's not,
please seek help! I can only hope you don't have a gun permit.

It's such a putty: U-Boot is a fantastic Open Source project. Before U-Boot,
it took me a year to develop a proprietary boot loader that didn't do much
more than booting and loading. Nowadays nearly every SoC imaginable
is supported by U-Boot, and U-Boot support has become a mandatory
criterion for SoC selection. Besides regular booting, it caters for very
complicated boot scenarios too. It became an essential tool for development
and board bring up too. Many attempts have been made to copy it, but there
is no equal by far, and all thanks to the people of the community. Unbelievable
what motivated like-minded people can achieve.

Thank you Wolfgang.

Cheers,

         Robert.

Dear Robert,

In message <1EC2DDDF-293C-4C6A-A02B-11F45D06205A@delien.nl> you wrote:
> 
> How nice of you to drop a message, I really appreciate it.

What do you mean?  Which message do you claim I have omitted?  And
where?

> In the end, doing things right always pays, but that's not always obvious
> at the beginning. I figured that I could just hand-craft these three

True.  A lesson I have learned is that one should always watch
himself, and as soon as you find yourself repeating something you
already did in the past, you should sk yourself how likely it might be
that you have to repeat the same work a third time, or even more
often.  And then think how such repetition can be avoided or at least
simplified (say, by writing a script, or at least by creating a
detailled log of your actions, so you have the base for a script the
next time you run into that thing).

> patches in three hours and be done with it. Just for that, switching from
> SVN to GIT didn't seem worth the effort.

3 hours?  Hm... I think it should be possible to read enough git
documents and tutorials in one hour to get at least a basic
understanding of git and import and export from / to a SVN repository.
Add another hour for some practical experiments with a simple test
setup. Add another hour for doing it with your real data...

> Now it turns out that whole the mxs gpio subsystem just doesn't work
> (consider the irony of bickering about tabs and spaces), so I'll may send

Do you understand the benefits and importance of a common coding
style in a big project with many, many contributors?

> It's good to get used to GIT, it's just that timing is a bit inconvenient
> right now. In two months from now, when our A-sample is brought
> up and all subsystems are working, things will look differently.

That's exacty the reasoning I hear always in such situations - and it
always and unavoidably leads into the well-known pattern that we never
have the time to do things right, but we always have the time to do
them twice.

> I have searched the web, but Mail really doesn't support it. Apple is
> using format=flow and assumes everybody handles that properly. 

Then use another MUA.  There is tons of them.

> That's very Zen-like. And between you and me; Yes, we've had our
> history there, if you remember. But I have learned from that and tried
> to do things differently, trying to be cooperative, modest and accepting
> the Custodian's authorities. But still I didn't feel appreciated or even
> welcome. I don't need gratitude, but I do deserve the same social
> courtesy as people do in real life. That's what I try to do.

Working in a big project is never easy, especially not when doing the
first steps into a long-runnign community that has (out of necessity)
highly optimized it's communication.  The removal of communication
overhead may easily be (mis-) taken as unkindliness.

It is important to adjust your own expectations: unless you add a
really cool new feature or fix a really nasty, long-hunted bug, you
will probably never receive any explicit praise when you submit a
poatch - even if it is technically perfect and works like a charme.
On the other hand, you can be pretty sure that eveny minor issues like
CondigStyle violations (say, this TAB versus spaces issue) will be
pointed out without mercy, even if you have submitted the same trivial
patch already a number of times before.

This is not meant personally - it is a necessary optimization of the
process that tries to minimize the efforts spent in later stages.
Just have a look at the git commits, how much work we spent again and
again to clean up bad code that slipped in in the past, when we were
not so consequent yet.  And we have to do this cleanup, to be able to
use other tools that help us to work efficiently - like for example
checkpatch.pl

> For example, if somebody - who you have never met before - is trying
> to help you in real life, would you respond like this if the help turns out
> to be a bit clumsy:
> 
> > AARGH! Please add the following lines to the commit message:
> > 
> > Cc: Marek Vasut <marek.vasut@gmail.com>
> > Cc: Stefano Babic <sbabic@denx.de>
> > 
> > !!!!

What exactly are you complaining about here? Marek even wrote
"Please".  The "AARGH!" can be anything. Maybe Marek meant: "Argh.
What an idion am I noticing this only now.  I should have told you
before, in the first round of review. Please excuse that my negligence
causes you additional, avoidable efforts."

Oh - you thought he was referring to you?  This is one of the problems
of communication by e-mail and other electronic media where you cannot
see nor hear the other side.  Maybe he actually was.  We don't know,
and actually we don't care.

Basic rule: never take such comments personally, always stick to the
raw facts.  Expect that the only purpose of the reviewer is to help to
improve the quality of the code.

> Even without politenesses like 'sorries' and 'pleases', this looks a lot
> friendlier and is educational at the same time:
> 
> Next time, include the lines below in the body of your message, and the
> mailing list will take care of the rest:

But that's nearly twice as many characters to type, and probably
much less memorable :-)

> Thank you Wolfgang.

You are welcome.  I really mean it.  And I am sure that many on this
list welcome all comments, bug reports, contributions, questions. I am
sure that Marek does so, too.  It's just that we have _lots_ of
e-mails to read, and tons of patches to process, so we try to optimize
this process to the extreme.  It's unfortunate that we cannot add
polite embellishments and lengthy explanations with every message we
send.  But there is simply no time for it.  This is a highly specilize
technical channel, and the thing we really care about (and are
willing to spend time for) is the technical progress of the project.
Never take this personally. Even if we are terse and sometimes harsh,
we never intend offence (well, usually not ;-)


Best regards,

Wolfgang Denk

Hi Wolfgang,

> What do you mean?  Which message do you claim I have omitted?  And
> where?

Oh, no, I'm sorry; That's not how I meant it. I didn't mean 'drop' as in 'omit',
but as in 'passing'.

> 3 hours?  Hm... I think it should be possible to read enough git
> documents and tutorials in one hour to get at least a basic
> understanding of git and import and export from / to a SVN repository.
> Add another hour for some practical experiments with a simple test
> setup. Add another hour for doing it with your real data...

True. There's probably a lesson in there somewhere, in hindsight.

> Do you understand the benefits and importance of a common coding
> style in a big project with many, many contributors?

Oh yes! In my own code, I always include the indent command with an
argument set in the comments. I require people to run that before checking
in their code.

> That's exacty the reasoning I hear always in such situations - and it
> always and unavoidably leads into the well-known pattern that we never
> have the time to do things right, but we always have the time to do
> them twice.

With managers it's always easier to claim time at the end of a project, when
there isn't any time anymore. At the beginning they just want to see
something work quickly.

> Then use another MUA.  There is tons of them.

Yet none of them uses the proprietary Exchange protocol to connect to
coorporate mail servers.
And honestly: People's allergy to inappropriately broken lines hardly
seems a problem worth solving. There's always another intollerance.

> Working in a big project is never easy, especially not when doing the
> first steps into a long-runnign community that has (out of necessity)
> highly optimized it's communication.  The removal of communication
> overhead may easily be (mis-) taken as unkindliness.

True, yet in my experience this efficiency is easier put aside to
express dissatisfaction than it is to express kindness.

> It is important to adjust your own expectations: unless you add a
> really cool new feature or fix a really nasty, long-hunted bug, you
> will probably never receive any explicit praise when you submit a
> poatch - even if it is technically perfect and works like a charme.

As said, I don't and didn't expect that. But I don't expect to get
'flamed' either.

I remember Haralt Welte speak at CELF, complaining about
semiconductor manufacturers not supporting Open Source and big
corporations only leaching and not contributing.
Next time I hear him speak, I will be the first at the mic to explain
him that if employees would mail as 'efficiently' as is done on
some mailing lists, we'd have a real nasty atmosphere in the
office. And that it's this atmosphere that's making corporations
think twice, not unwillingness to contribute.

Seriouly; I've show it to my colleagues and they said I was nuts trying
over and over again. And perhaps I am, because this way I don't
enjoy my work.

> On the other hand, you can be pretty sure that eveny minor issues like
> CondigStyle violations (say, this TAB versus spaces issue) will be
> pointed out without mercy, even if you have submitted the same trivial
> patch already a number of times before.

For those there are rules. I tend to fix those along making changes to a
file. You guys seem to prefer to have separate patches for those, but I
doubt if anybody's going to spend time just on cosmetics.

> This is not meant personally - it is a necessary optimization of the
> process that tries to minimize the efforts spent in later stages.

Perhaps. But there're also people trying to lift their own importance just
by commenting. I see this in meetings every day, but there at least I
get to roll my eyes ;-)

> Basic rule: never take such comments personally, always stick to the
> raw facts.  Expect that the only purpose of the reviewer is to help to
> improve the quality of the code.

Same rule applies on the other side: Never make comments personally.

> But that's nearly twice as many characters to type, and probably
> much less memorable :-)

Clubbing somebody takes even less characters and is even more
memorable (if stopped in time :-).
Yes, it takes a couple of extra keystrokes, but that's all it takes and I
think they're worth the effort in the long run. 

> I am
> sure that Marek does so, too.

Marek is a good guy, no argument about it.

> It's unfortunate that we cannot add
> polite embellishments and lengthy explanations with every message we
> send.  But there is simply no time for it.

I don't agree: I think it doesn't take more than a couple of extra
keystrokes to take off the harsh edge, to be constructive or even to show
some encouragement. No more than it takes to be hars or to discourage
people.
Perhaps I expected a bit too much. A bit disappointing after evangelizing
Open Source and convincing management about the importance of
contributing,

> This is a highly specilize
> technical channel, and the thing we really care about (and are
> willing to spend time for) is the technical progress of the project.
> 
> Never take this personally. Even if we are terse and sometimes harsh,
> we never intend offence (well, usually not ;-)

The Time Nuts mailing list is even more specialized (about high accuracy
time keeping), but on that list people tend to _more_ polite than in real
life, just to prevent hat.

Thank you for your time, Wolfgang. I fully understand if you don't take
the time to respond.

With kind regards,

        Robert.