public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Albert ARIBAUD <albert.aribaud@free.fr>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] orion5x: edminiv2: add libata support
Date: Thu, 01 Jul 2010 00:35:55 +0200	[thread overview]
Message-ID: <4C2BC6CB.1070802@free.fr> (raw)
In-Reply-To: <20100630213939.1AEBE1524EC@gemini.denx.de>

Hi Wolfgang,

Le 30/06/2010 23:39, Wolfgang Denk a ?crit :
> Dear Albert Aribaud,
>
> In message<1277933418-682-1-git-send-email-albert.aribaud@free.fr>  you wrote:
>>
>> Signed-off-by: Albert Aribaud<albert.aribaud@free.fr>
>> ---
>> This patch:
>> - adds support in libata for the orion5x MVSATAHC controller;
>> - enables orion5x MVSTAHC port 1 on the edmini board;
>> - adds IDE and EXT2 commands to the edminiv2 command set.
>
> What exactly do you mean by "libata"?   Do we have something like this
> in U-Boot?

My mistake: I meant ide. I'd started this work looking into the libata.c 
block driver but realized later that it wasn't what I needed, but the 
term stuck in my mind. I'll fix references to mention IDE instead.

>> +/* ED Mini V2 uses SATA PORT1. Initialize this port and disable
>> + * disable low power on it
>> + */
>> + /* mask for isolating IPM and DET fields in SControl register */
>
> Incorrect multiline comment style.

Will this be ok?

	/*
	 * ED Mini V2 [...]
	 * disable [...]
	 */

	/* mask for [...] */

>> -#if defined(__PPC__) || defined(CONFIG_PXA_PCMCIA) || defined(CONFIG_SH)
>> +#if defined(__PPC__) || defined(CONFIG_PXA_PCMCIA) || defined(CONFIG_SH) \
>> +	|| defined(__ARM__)
>
> Please do not add to this mess of tests, but introduce a single
> feature-specific #define instead.

Will do. Would CONFIG_IDE_USE_INSW_OUTSW do? Basically, that's what the 
test is for: either use outsw()/insw() or use outw()/inw() within a loop.

Note that this change does not relate per se to my patch and requires 
addition of this #define to a lot of config header files. Wouldn't it be 
better if I submitted it as an independent atomic patch?

>> +/* Debugging features */
>> +
>> +/* #define the following if u-boot will boot from RAM */
>> +/* #undef it if u-boot will boot from FLASH */
>> +#undef CONFIG_SKIP_LOWLEVEL_INIT	/* disable board lowlevel_init */
>> +
>
> This seems to be an unrelated change (like someother, smaller ones).
> Please make sure to split your patches into independent parts.

I'll put them in two separate patches, one for fixing typos and one for 
adding the CONFIG_SKIP_LOWLEVEL_INIT comments and line.

> Best regards,
>
> Wolfgang Denk

Thanks for the feedback.

Amicalement,
-- 
Albert.

  reply	other threads:[~2010-06-30 22:35 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-06-30 21:30 [U-Boot] [PATCH] orion5x: edminiv2: add libata support Albert Aribaud
2010-06-30 21:39 ` Wolfgang Denk
2010-06-30 22:35   ` Albert ARIBAUD [this message]
2010-06-30 22:48     ` Wolfgang Denk
2010-07-01  0:35       ` Albert ARIBAUD
2010-07-01  0:37         ` [U-Boot] [PATCH 1/4] ide: add configuration CONFIG_IDE_SWAP_IO Albert Aribaud
2010-07-01  0:38           ` [U-Boot] [PATCH 2/4] orion5x: edminiv2: add CMD_IDE support Albert Aribaud
2010-07-01  0:38             ` [U-Boot] [PATCH 3/4] orion5x: fix typo in comment Albert Aribaud
2010-07-01  0:38               ` [U-Boot] [PATCH 4/4] edminiv2: inttroduce CONFIG_SKIP_LOWLEVEL_INIT Albert Aribaud
2010-06-30 21:42 ` [U-Boot] [PATCH] orion5x: edminiv2: add libata support Albert ARIBAUD
2010-07-01  7:36 ` Tor Krill
2010-07-01  8:41   ` Prafulla Wadaskar
2010-07-01  8:51     ` Tor Krill
2010-07-01 12:00     ` Albert ARIBAUD
2010-07-01  8:51   ` Albert ARIBAUD
2010-07-01  8:52   ` Albert ARIBAUD

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4C2BC6CB.1070802@free.fr \
    --to=albert.aribaud@free.fr \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox