RE: [RFC 5/8] remoteproc: add davinci implementation

From: Nori, Sekhar
Date: Tue Jun 28 2011 - 05:37:54 EST


Hi Mark,

On Mon, Jun 27, 2011 at 23:50:25, Grosen, Mark wrote:

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

The second argument is not a (SoC specific)
clock name, it is the "connection id" which
is specific to the IP (not to the SoC).

For example, the EMAC IP may want to deal
with interface clock, functional clock and
mii clock. So, it passes a connection identifier
to tell the clock framework which particular
clock it is interested in. The names of these
connection identifiers will not change with SoC
as they are IP property. So, it will not be
right to get this through platform data.

If there is no connection identifier in
your case, you can just pass the second
argument as NULL. The clock matching will
happen using device name (which should
remain same across SoCs too).

Of course, this means that any incorrectly defined
clock lookup structures for dsp clock in platform
code needs to be corrected.

Thanks,
Sekhar

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/