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

On 10/15/07, Matt Sealey [off-list ref] wrote:

My nits:

Grant Likely wrote:

quoted

From: Sylvain Munaut <redacted>
+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.

quoted

+     /* 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.

quoted

+int bcom_sram_init(struct device_node *sram_node, char *owner)
+{
+     int rv;
+     const u32 *regaddr_p;
+     u64 regaddr64, size64;
+     unsigned int psize;
+
+     /* Create our state struct */
+     if (bcom_sram) {
+             printk(KERN_ERR "%s: bcom_sram_init: "
+                     "Already initialiwed !\n", owner);
+             return -EBUSY;
+     }

initialised or initialized :)

fixed

quoted

+             printk(KERN_ERR "%s: bcom_sram_init: "
+                     "Couln't request region !\n", owner);
+             rv = -EBUSY;
+             goto error_free;

Couldn't or could not.

fixed; thanks.

Cheers,
g.

--
Matt Sealey [off-list ref]
Genesi, Manager, Developer Relations

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195