[RFC 5/8] remoteproc: add davinci implementation

From: Sergei Shtylyov
Sent: Friday, June 24, 2011 8:14 AM

Grosen, Mark wrote:

quoted

quoted

    It should work on DA830 as well,

    So please make it dependent on ARCH_DAVINCI_DA8XX.

quoted

quoted

but not on real DaVinci, so the name is misleading...

quoted

Yes, we debated calling it da8xx, but felt that with minor changes it could
accomodate the other SoCs in the davinci family.

    I don't think it's a good idea. Using cpu_is_*() is drivers is bad. Using
#ifdef's is not an option either.

quoted

However, it may be better
to start with just the da8xx/omapl13x parts and then rename if we add the
others.

Sergei, I'll respond more to this in a response to Sekhar's ideas. We may be
able to make this work generically for davinci w/o idef's.

Looking into my old PSC manual (can't get the recent documentation from TI's
site right now), the bit is called LRSTz.
    It's worth moving this #define into <mach/psc.h> as well.

Ok, I agree we should try to match the HW names as much as possible

quoted

quoted

quoted

+/* register for DSP boot address in SYSCFG0 module (section 11.5.6) */
+#define HOST1CFG	0x44

quoted

quoted

    Worth declaring in <mach/da8xx.h> instead...

quoted

Possibly - since it is only used for the DSP, I thought it would be better
to keep local to this implementation. I'll adopt whichever approach is the
convention.

    Well, the general approach is to keep the #define's where they are
used, so maybe we should keep this #define here.

Will review as part of the general cleanup.

quoted

quoted

quoted

+static inline int davinci_rproc_start(struct rproc *rproc, u64

bootaddr)

quoted

+{
+	struct device *dev = rproc->dev;
+	struct davinci_rproc_pdata *pdata = dev->platform_data;
+	struct davinci_soc_info *soc_info = &davinci_soc_info;
+	void __iomem *psc_base;
+	struct clk *dsp_clk;
+
+	/* hw requires the start (boot) address be on 1KB boundary */
+	if (bootaddr & 0x3ff) {
+		dev_err(dev, "invalid boot address: must be aligned to

1KB\n");

quoted

+		return -EINVAL;
+	}
+
+	dsp_clk = clk_get(dev, pdata->clk_name);

quoted

quoted

    We could match using clkdev functionality, but the clock entry
would need to be changed then...

quoted

I followed the existing pattern I saw in other drivers.

Probably MUSB? We're trying to move away from passing the clock name to thge
drivers, using match by device instead.

quoted

If there is a new, better way, please point me to an example.

    Look at the da850_clks[] in mach-davinci/da850.c: if the device name is
specified as a first argument to CLK() macro (instead of clock name the second
argument), the matching is done by device, so you don't need to specify the
clock name to clk_get(), passing NULL instead.

Thanks, I'll look at this and ask for Sekhar and Kevin's preferences.

Mark