> From: Sergei Shtylyov
> Sent: Friday, June 24, 2011 8:14 AM
>
> Grosen, Mark wrote:
>
> >> It should work on DA830 as well,
>
> So please make it dependent on ARCH_DAVINCI_DA8XX.
>
> >> but not on real DaVinci, so the name is misleading...
>
> > 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.
>
> > 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
> >>> +/* register for DSP boot address in SYSCFG0 module (section 11.5.6) */
> >>> +#define HOST1CFG 0x44
>
> >> Worth declaring in <mach/da8xx.h> instead...
>
> > 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.
>
> >>> +static inline int davinci_rproc_start(struct rproc *rproc, u64
> >> bootaddr)
> >>> +{
> >>> + 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");
> >>> + return -EINVAL;
> >>> + }
> >>> +
> >>> + dsp_clk = clk_get(dev, pdata->clk_name);
>
> >> We could match using clkdev functionality, but the clock entry
> >> would need to be changed then...
>
> > 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.
>
> > 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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
