Patchwork [U-Boot] fix IDE_BUS(dev) macro

login
register
mail settings
Submitter Luka Perkov
Date May 14, 2012, 7:29 p.m.
Message ID <20120514192910.GA23116@w500.iskon.local>
Download mbox | patch
Permalink /patch/159129/
State Accepted
Delegated to: Tom Rini
Headers show

Comments

Luka Perkov - May 14, 2012, 7:29 p.m.
Hi Albert,

On Sun, Apr 29, 2012 at 10:19:41PM +0200, Luka Perkov wrote:
> On Thu, Apr 19, 2012 at 08:38:19AM +0200, Albert ARIBAUD wrote:
> > Not my main area of expertise here, but I am not sure how this plays
> > on Marvell non-kirkwood platforms (e.g., orion5x).
> > 
> > ISTR it is not the first time we deal with the whole IDE number of
> > bus / number of ports [ / useable ports ] issue, and we may be
> > running in circles here, fixing one platform and breaking another.
> > 
> > I'll try this on EDMiniV2 in the coming days, and let people know
> > the results in this thread.
> 
> I was just wondering did you have the time to test this patch on your
> board?

Albert, did you maybe have time to test this patch. I have included it
bellow so you dont have to search for it...

Signed-off-by: Simon Baatz <gmbnomis@gmail.com>
Tested-by: Luka Perkov <uboot@lukaperkov.net>
---

Simon discovered this while adding support for new board IB NAS6210.

More info can be found here:

http://lists.denx.de/pipermail/u-boot/2012-April/122525.html

 include/ide.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Albert ARIBAUD - May 15, 2012, 7:42 p.m.
Hi Luka,

Le 14/05/2012 21:29, Luka Perkov a écrit :
> Hi Albert,
>
> On Sun, Apr 29, 2012 at 10:19:41PM +0200, Luka Perkov wrote:
>> On Thu, Apr 19, 2012 at 08:38:19AM +0200, Albert ARIBAUD wrote:
>>> Not my main area of expertise here, but I am not sure how this plays
>>> on Marvell non-kirkwood platforms (e.g., orion5x).
>>>
>>> ISTR it is not the first time we deal with the whole IDE number of
>>> bus / number of ports [ / useable ports ] issue, and we may be
>>> running in circles here, fixing one platform and breaking another.
>>>
>>> I'll try this on EDMiniV2 in the coming days, and let people know
>>> the results in this thread.
>>
>> I was just wondering did you have the time to test this patch on your
>> board?
>
> Albert, did you maybe have time to test this patch. I have included it
> bellow so you dont have to search for it...
>
> Signed-off-by: Simon Baatz<gmbnomis@gmail.com>
> Tested-by: Luka Perkov<uboot@lukaperkov.net>
> ---
>
> Simon discovered this while adding support for new board IB NAS6210.
>
> More info can be found here:
>
> http://lists.denx.de/pipermail/u-boot/2012-April/122525.html
>
>   include/ide.h |    2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/ide.h b/include/ide.h
> index 8ecc9dd..385e909 100644
> --- a/include/ide.h
> +++ b/include/ide.h
> @@ -24,7 +24,7 @@
>   #ifndef	_IDE_H
>   #define _IDE_H
>
> -#define	IDE_BUS(dev)	(dev>>  1)
> +#define IDE_BUS(dev)	(dev / (CONFIG_SYS_IDE_MAXDEVICE / CONFIG_SYS_IDE_MAXBUS))
>
>   #define	ATA_CURR_BASE(dev)	(CONFIG_SYS_ATA_BASE_ADDR+ide_bus_offset[IDE_BUS(dev)])

Sorry, been busier than usual. I'll try and test this tomorrow night; if 
not, that'll be next week I'm afraid.

Amicalement,
Luka Perkov - May 15, 2012, 8:44 p.m.
Hi Albert,

On Tue, May 15, 2012 at 09:42:59PM +0200, Albert ARIBAUD wrote:
> >diff --git a/include/ide.h b/include/ide.h
> >index 8ecc9dd..385e909 100644
> >--- a/include/ide.h
> >+++ b/include/ide.h
> >@@ -24,7 +24,7 @@
> >  #ifndef	_IDE_H
> >  #define _IDE_H
> >
> >-#define	IDE_BUS(dev)	(dev>>  1)
> >+#define IDE_BUS(dev)	(dev / (CONFIG_SYS_IDE_MAXDEVICE / CONFIG_SYS_IDE_MAXBUS))
> >
> >  #define	ATA_CURR_BASE(dev)	(CONFIG_SYS_ATA_BASE_ADDR+ide_bus_offset[IDE_BUS(dev)])
> 
> Sorry, been busier than usual. I'll try and test this tomorrow
> night; if not, that'll be next week I'm afraid.

It's no hurry... I was pinging you so this does not end up forgotten.

If this does not work for your boards we can always use #ifdef's.

Regards,
Luka
Luka Perkov - June 7, 2012, 1:30 p.m.
Hi Albert,

On Tue, May 15, 2012 at 09:42:59PM +0200, Albert ARIBAUD wrote:
> >On Sun, Apr 29, 2012 at 10:19:41PM +0200, Luka Perkov wrote:
> >>On Thu, Apr 19, 2012 at 08:38:19AM +0200, Albert ARIBAUD wrote:
> >>>Not my main area of expertise here, but I am not sure how this plays
> >>>on Marvell non-kirkwood platforms (e.g., orion5x).
> >>>
> >>>ISTR it is not the first time we deal with the whole IDE number of
> >>>bus / number of ports [ / useable ports ] issue, and we may be
> >>>running in circles here, fixing one platform and breaking another.
> >>>
> >>>I'll try this on EDMiniV2 in the coming days, and let people know
> >>>the results in this thread.
> >>
> >>I was just wondering did you have the time to test this patch on your
> >>board?
> >
> >Albert, did you maybe have time to test this patch. I have included it
> >bellow so you dont have to search for it...
> >
> >Signed-off-by: Simon Baatz<gmbnomis@gmail.com>
> >Tested-by: Luka Perkov<uboot@lukaperkov.net>
> >---
> >
> >Simon discovered this while adding support for new board IB NAS6210.
> >
> >More info can be found here:
> >
> >http://lists.denx.de/pipermail/u-boot/2012-April/122525.html
> >
> >  include/ide.h |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >diff --git a/include/ide.h b/include/ide.h
> >index 8ecc9dd..385e909 100644
> >--- a/include/ide.h
> >+++ b/include/ide.h
> >@@ -24,7 +24,7 @@
> >  #ifndef	_IDE_H
> >  #define _IDE_H
> >
> >-#define	IDE_BUS(dev)	(dev>>  1)
> >+#define IDE_BUS(dev)	(dev / (CONFIG_SYS_IDE_MAXDEVICE / CONFIG_SYS_IDE_MAXBUS))
> >
> >  #define	ATA_CURR_BASE(dev)	(CONFIG_SYS_ATA_BASE_ADDR+ide_bus_offset[IDE_BUS(dev)])
> 
> Sorry, been busier than usual. I'll try and test this tomorrow
> night; if not, that'll be next week I'm afraid.

It's me beeing boring again, sorry... Can we get your feedback on this one?

Regards,
Luka
Albert ARIBAUD - June 20, 2012, 8:05 a.m.
Sorry Luka (and all), been tied up pretty heavily recently, had to
adjust to find some free time again.

I will test your patch today and post results tonight.

2012/6/7 Luka Perkov <uboot@lukaperkov.net>
>
> Hi Albert,
>
> On Tue, May 15, 2012 at 09:42:59PM +0200, Albert ARIBAUD wrote:
> > >On Sun, Apr 29, 2012 at 10:19:41PM +0200, Luka Perkov wrote:
> > >>On Thu, Apr 19, 2012 at 08:38:19AM +0200, Albert ARIBAUD wrote:
> > >>>Not my main area of expertise here, but I am not sure how this plays
> > >>>on Marvell non-kirkwood platforms (e.g., orion5x).
> > >>>
> > >>>ISTR it is not the first time we deal with the whole IDE number of
> > >>>bus / number of ports [ / useable ports ] issue, and we may be
> > >>>running in circles here, fixing one platform and breaking another.
> > >>>
> > >>>I'll try this on EDMiniV2 in the coming days, and let people know
> > >>>the results in this thread.
> > >>
> > >>I was just wondering did you have the time to test this patch on your
> > >>board?
> > >
> > >Albert, did you maybe have time to test this patch. I have included it
> > >bellow so you dont have to search for it...
> > >
> > >Signed-off-by: Simon Baatz<gmbnomis@gmail.com>
> > >Tested-by: Luka Perkov<uboot@lukaperkov.net>
> > >---
> > >
> > >Simon discovered this while adding support for new board IB NAS6210.
> > >
> > >More info can be found here:
> > >
> > >http://lists.denx.de/pipermail/u-boot/2012-April/122525.html
> > >
> > >  include/ide.h |    2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > >diff --git a/include/ide.h b/include/ide.h
> > >index 8ecc9dd..385e909 100644
> > >--- a/include/ide.h
> > >+++ b/include/ide.h
> > >@@ -24,7 +24,7 @@
> > >  #ifndef    _IDE_H
> > >  #define _IDE_H
> > >
> > >-#define     IDE_BUS(dev)    (dev>>  1)
> > >+#define IDE_BUS(dev)        (dev / (CONFIG_SYS_IDE_MAXDEVICE / CONFIG_SYS_IDE_MAXBUS))
> > >
> > >  #define    ATA_CURR_BASE(dev)      (CONFIG_SYS_ATA_BASE_ADDR+ide_bus_offset[IDE_BUS(dev)])
> >
> > Sorry, been busier than usual. I'll try and test this tomorrow
> > night; if not, that'll be next week I'm afraid.
>
> It's me beeing boring again, sorry... Can we get your feedback on this one?
>
> Regards,
> Luka
>

Amicalement,
Albert.
Wolfgang Denk - Aug. 9, 2012, 8:36 p.m.
Dear Albert ARIBAUD,

In message <CAJHhwataYrT3GBzhrDcqhphRP5HbX3vd2XyuyYMSLeD8XWE8WA@mail.gmail.com> you wrote:
> Sorry Luka (and all), been tied up pretty heavily recently, had to
> adjust to find some free time again.
> 
> I will test your patch today and post results tonight.

What was the outcome of this?  I cannot find any further messages to
this thread...

Best regards,

Wolfgang Denk
Albert ARIBAUD - Aug. 13, 2012, 12:27 p.m.
Hi Wolfgang,

On Thu, 09 Aug 2012 22:36:33 +0200, Wolfgang Denk <wd@denx.de> wrote:
> Dear Albert ARIBAUD,
> 
> In message
> <CAJHhwataYrT3GBzhrDcqhphRP5HbX3vd2XyuyYMSLeD8XWE8WA@mail.gmail.com>
> you wrote:
> > Sorry Luka (and all), been tied up pretty heavily recently, had to
> > adjust to find some free time again.
> > 
> > I will test your patch today and post results tonight.
> 
> What was the outcome of this?  I cannot find any further messages to
> this thread...

Sorry, I was away from home for a few days with no or little Internet
access.

I wanted to test this on orion5x/edminiv2; I should be able to do this
today in the course of regression-testing preparing an orion5x patch
I want to submit in this window. 

> Best regards,
> 
> Wolfgang Denk

Amicalement,
Albert ARIBAUD - Aug. 14, 2012, 8:30 a.m.
On Mon, 13 Aug 2012 14:27:16 +0200, Albert ARIBAUD
<albert.u.boot@aribaud.net> wrote:
> Hi Wolfgang,
> 
> On Thu, 09 Aug 2012 22:36:33 +0200, Wolfgang Denk <wd@denx.de> wrote:
> > Dear Albert ARIBAUD,
> > 
> > In message
> > <CAJHhwataYrT3GBzhrDcqhphRP5HbX3vd2XyuyYMSLeD8XWE8WA@mail.gmail.com>
> > you wrote:
> > > Sorry Luka (and all), been tied up pretty heavily recently, had to
> > > adjust to find some free time again.
> > > 
> > > I will test your patch today and post results tonight.
> > 
> > What was the outcome of this?  I cannot find any further messages to
> > this thread...
> 
> Sorry, I was away from home for a few days with no or little Internet
> access.
> 
> I wanted to test this on orion5x/edminiv2; I should be able to do this
> today in the course of regression-testing preparing an orion5x patch
> I want to submit in this window. 

... and of course I spent all my allotted time yesterday on catching up
about pull requests, sorry. I should be able to test this today, above
the current tip of u-boot-arm/master.

Amicalement,
DrEagle - Aug. 14, 2012, 12:33 p.m.
Hi,

It works great for my IB-6220 (Dual Sata).

Without the patch, the same disk is seen twice, I have only checked
disks serial numbers :

ib62x0 => version

U-Boot 2012.07-00123-g4d3c95f-dirty (Aug 13 2012 - 11:30:58) RaidSonic
ICY BOX IB-NAS62x0
arm-linux-gnueabi-gcc (Debian 4.4.5-8) 4.4.5
GNU ld (GNU Binutils for Debian) 2.20.1.20100303
ib62x0 => ide reset

Reset IDE: Bus 0: OK Bus 1: OK
  Device 0: Model: WDC WD20EARX-00MMMB0 Firm: 80.00A80 Ser#:
WD-WCAWZ2075392
            Type: Hard Disk
            Supports 48-bit addressing
            Capacity: 1907729.0 MB = 1863.0 GB (-387938128 x 512)
  Device 1: Model: WDC WD20EARX-00MMMB0 Firm: 80.00A80 Ser#:
WD-WCAWZ2075392
            Type: Hard Disk
            Supports 48-bit addressing
            Capacity: 1907729.0 MB = 1863.0 GB (-387938128 x 512)
ib62x0 =>

---
With the patch applied, the two disks are seen :
---
patch -p0 -i ../fix_ide.diff

File to patch: include/ide.h

ib62x0 => version

U-Boot 2012.07-00123-g4d3c95f-dirty (Aug 14 2012 - 14:22:49) RaidSonic
ICY BOX IB-NAS62x0
arm-linux-gnueabi-gcc (Debian 4.4.5-8) 4.4.5
GNU ld (GNU Binutils for Debian) 2.20.1.20100303
ib62x0 => ide reset

Reset IDE: Bus 0: OK Bus 1: OK
  Device 0: Model: WDC WD20EARX-00MMMB0 Firm: 80.00A80 Ser#:
WD-WCAWZ2075392
            Type: Hard Disk
            Supports 48-bit addressing
            Capacity: 1907729.0 MB = 1863.0 GB (-387938128 x 512)
  Device 1: Model: WDC WD20EARX-00MMMB0 Firm: 80.00A80 Ser#:
WD-WCAWZ2083815
            Type: Hard Disk
            Supports 48-bit addressing
            Capacity: 1907729.0 MB = 1863.0 GB (-387938128 x 512)
ib62x0 =>
ib62x0 => ide device 0

IDE device 0: Model: WDC WD20EARX-00MMMB0 Firm: 80.00A80 Ser#:
WD-WCAWZ2075392
            Type: Hard Disk
            Supports 48-bit addressing
            Capacity: 1907729.0 MB = 1863.0 GB (-387938128 x 512)
... is now current device
ib62x0 => ide device 1

IDE device 1: Model: WDC WD20EARX-00MMMB0 Firm: 80.00A80 Ser#:
WD-WCAWZ2083815
            Type: Hard Disk
            Supports 48-bit addressing
            Capacity: 1907729.0 MB = 1863.0 GB (-387938128 x 512)
... is now current device

---
For me this patch has fixed the detection bug of ide disks.

Hope this tests may help.

Le 13/08/2012 14:27, Albert ARIBAUD a écrit :
> Hi Wolfgang,
> 
> On Thu, 09 Aug 2012 22:36:33 +0200, Wolfgang Denk <wd@denx.de> wrote:
>> Dear Albert ARIBAUD,
>>
>> In message
>> <CAJHhwataYrT3GBzhrDcqhphRP5HbX3vd2XyuyYMSLeD8XWE8WA@mail.gmail.com>
>> you wrote:
>>> Sorry Luka (and all), been tied up pretty heavily recently, had to
>>> adjust to find some free time again.
>>>
>>> I will test your patch today and post results tonight.
>>
>> What was the outcome of this?  I cannot find any further messages to
>> this thread...
> 
> Sorry, I was away from home for a few days with no or little Internet
> access.
> 
> I wanted to test this on orion5x/edminiv2; I should be able to do this
> today in the course of regression-testing preparing an orion5x patch
> I want to submit in this window. 
> 
>> Best regards,
>>
>> Wolfgang Denk
> 
> Amicalement,
> 

drEagle
Tom Rini - Oct. 8, 2012, 6:46 p.m.
On Mon, May 14, 2012 at 09:29:10AM -0000, Luka Perkov wrote:

> Hi Albert,
> 
> On Sun, Apr 29, 2012 at 10:19:41PM +0200, Luka Perkov wrote:
> > On Thu, Apr 19, 2012 at 08:38:19AM +0200, Albert ARIBAUD wrote:
> > > Not my main area of expertise here, but I am not sure how this plays
> > > on Marvell non-kirkwood platforms (e.g., orion5x).
> > > 
> > > ISTR it is not the first time we deal with the whole IDE number of
> > > bus / number of ports [ / useable ports ] issue, and we may be
> > > running in circles here, fixing one platform and breaking another.
> > > 
> > > I'll try this on EDMiniV2 in the coming days, and let people know
> > > the results in this thread.
> > 
> > I was just wondering did you have the time to test this patch on your
> > board?
> 
> Albert, did you maybe have time to test this patch. I have included it
> bellow so you dont have to search for it...
> 
> Signed-off-by: Simon Baatz <gmbnomis@gmail.com>
> Tested-by: Luka Perkov <uboot@lukaperkov.net>

With a reworded commit message, applied to u-boot/master, thanks!

Patch

diff --git a/include/ide.h b/include/ide.h
index 8ecc9dd..385e909 100644
--- a/include/ide.h
+++ b/include/ide.h
@@ -24,7 +24,7 @@ 
 #ifndef	_IDE_H
 #define _IDE_H
 
-#define	IDE_BUS(dev)	(dev >> 1)
+#define IDE_BUS(dev)	(dev / (CONFIG_SYS_IDE_MAXDEVICE / CONFIG_SYS_IDE_MAXBUS))
 
 #define	ATA_CURR_BASE(dev)	(CONFIG_SYS_ATA_BASE_ADDR+ide_bus_offset[IDE_BUS(dev)])