[U-Boot] [PATCH 3/6] sbc8548: use I/O accessors

[Graeme Russ]

On Mon, Sep 21, 2009 at 6:48 PM, Wolfgang Denk <wd at denx.de> wrote:
> Dear Graeme Russ,
>
> In message <d66caabb0909201837g28da0ed5k6069e96775cfb358 at mail.gmail.com> you wrote:
>>
>> > -       ecm->eedr = 0xffffffff;         /* clear ecm errors */
>> > -       ecm->eeer = 0xffffffff;         /* enable ecm errors */
>> > +       out_be32(&ecm->eedr, 0xffffffff);       /* clear ecm errors */
>> > +       out_be32(&ecm->eeer, 0xffffffff);       /* enable ecm errors */
>> >        return 0;
>>
>> Correct me if I'm wrong, but I thought the general rule was the other
>> way (i.e. the way it alreay was). See for example:
>
> You are wrong. We must use I/O accessors.

Thanks for the correction :)

>
>> http://git.denx.de/cgi-bin/gitweb.cgi?p=u-boot.git;a=commit;h=ed7a1b681de1e31d18d5b92e2767ae8df3241687
>
> Ouch. This patch is clearly bloken. Sorry it escaped my attention.

Hence my incorrect assumption ;)

>
> Using a C structure to describe the hardware is a good thing, but we
> still must use I/O accessors to access the registers.
>
> This needs to be fixed in your code.

Will do

>
>> This prevents, for example, accidentally writing words and longs to byte
>> sized registers.
>
> Right. The I/O accessors still allow for strict type checking.

Right, which still makes my patch better than the completely unchecked
previous version of the SC520 MMCR accessors

Regards,

G