All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.