[PATCH v2 4/7] bestcomm: core bestcomm support for Freescale MPC5200

[tnt at blacksnow.net]

> On 10/15/07, Matt Sealey <matt at genesi-usa.com> wrote:
>>
>> My nits:
>>
>> Grant Likely wrote:
>> > From: Sylvain Munaut <tnt at 246tNt.com>
>> > +static int __devinit
>> > +bcom_engine_init(void)
>>
>> Why "bcom" and not "bestcomm"?
>
> I can type 'bcom' twice as fast.  :-)  bcom is a suitable shortening;
> I'm not concerned about it.

I prefer bcom as well. Much shorter and there is no ambiguity.
If you use theses, you are writing a 5200 driver. In that context you
should be able to figure out what 'bcom' stands for ... (and if not, maybe
you shouldn't be using them ;)


>> > +     /* Disable COMM Bus Prefetch, apparently it's not reliable yet
>> */
>> > +     /* FIXME: This should be done on 5200 and not 5200B ... */
>> > +     out_be16(&bcom_eng->regs->PtdCntrl,
>> in_be16(&bcom_eng->regs->PtdCntrl) | 1);
>>
>> This really, really shouldn't even be here, could it be moved to a
>> platform
>> init, or switched on a PVR/SVR here?
>
> I think I'd like to leave it here for getting this series merged; it
> may not be good to have it here; but it's not dangerous either.  I'm
> trying to keep churn on this series down to a minimum.
>
> Please submit a patch to make this change once it's merged.

Mmmm.
I think it _does_ belong here. COMM bus prefetch _is_ a bestcomm engine
option.

But indeed as the comment says, it maybe should be enabled on 5200B.
Not a big concern for me at the moment tough.



   Sylvain