omap_device: query on "fck" clk alias created

omap_device: query on "fck" clk alias created

[Vaibhav Hiremath]

Hi,

In OMAP world, we have omap_device layer, which exports api's like
omap_device_build() to create and register platform_device to the
kernel. This layer understands hwmod infrastructure and parses all the
platform specific information from it.
Now with DT migration, the same thing is achieved using a notifier,
which in turn omap_device_build_from_dt(), which in turn does same thing.

One of the thing which is happening in both execution path is, creating
alias for functional clock (hwmod_data->main_clk) for each device
getting created with the con_id = "fck".

Now the problem with this is,

The clk_get() api will not work, unless we pass both the arguments (dev,
con_id) properly. Now expecting driver to always label con_id with "fck"
is undesirable, as the same driver can be reused on multiple platforms,
which can be non-omap and/or non-ti platforms.
Also we have multiple examples where driver simply calls (which is right)

clk = clk_get(&pdev->dev, NULL);

This would fail on OMAP platforms, unless you modify clockxxx_data.c
file with,

CLK("<device>",  NULL,           &xxx_fck,    CK_34XX),


Devices specially with only one clock source always prefer not to
specify con_id.
And it seems wrong thing, as across platforms IP integration can be very
different and you can not expect to change the clock-tree table always.

So the option here we have is,

1. Instead of creating alias with "fck", create an alias only with dev
pointer, that means con_id = NULL.
I have already tested this and it works on AM33xx platform.

2. Add a new flag inside hwmod or omap_device, so that it can be read at
omap_device layer and based on that we can decide whether to add con_id
or not while invoking clkdev_alloc().
This may be required to support legacy drivers which are not migrated to
runtime_pm and still calls clk_get() for both "fck" and "ick", so here
we need "fck" clk alias.


Any comments or opinion on this? Based on the feedback I can create the
changes and submit it to the list.

Thanks,
Vaibhav

omap_device: query on "fck" clk alias created

[Benoit Cousson]

Hi Vaibhav,

On 08/01/2012 02:20 PM, Vaibhav Hiremath wrote:
> Hi,
> 
> In OMAP world, we have omap_device layer, which exports api's like
> omap_device_build() to create and register platform_device to the
> kernel. This layer understands hwmod infrastructure and parses all the
> platform specific information from it.
> Now with DT migration, the same thing is achieved using a notifier,
> which in turn omap_device_build_from_dt(), which in turn does same thing.
> 
> One of the thing which is happening in both execution path is, creating
> alias for functional clock (hwmod_data->main_clk) for each device
> getting created with the con_id = "fck".
> 
> Now the problem with this is,
> 
> The clk_get() api will not work, unless we pass both the arguments (dev,
> con_id) properly. Now expecting driver to always label con_id with "fck"
> is undesirable, as the same driver can be reused on multiple platforms,
> which can be non-omap and/or non-ti platforms.
> Also we have multiple examples where driver simply calls (which is right)
> 
> clk = clk_get(&pdev->dev, NULL);

Mmm, I don't know, but even if this is right, shouldn't we avoid such
usage. It might be better to be explicit than assuming that the IP will
always have an unique clock.
Some IP does have several inputs that might or not be connected
depending or the integration or on the version. So even if only one
clock is useful at some point it will not always be true.
We had similar issue with the IRQ lines in the past.

> This would fail on OMAP platforms, unless you modify clockxxx_data.c
> file with,
> 
> CLK("<device>",  NULL,           &xxx_fck,    CK_34XX),
> 
> 
> Devices specially with only one clock source always prefer not to
> specify con_id.

How many devices do you have with only one clock source? Could you
provide the list of drivers that assume that?

Most IPs in OMAP does have a fck and an ick so that does not seems nice
or even consistent to me to do a clk_get(&pdev->dev, NULL) for "fck" and
clk_get(&pdev->dev, "ick") for the ick.

> And it seems wrong thing, as across platforms IP integration can be very
> different and you can not expect to change the clock-tree table always.

FWIW, that table is done for that exact purpose. And we introduced the
automatic "fck" alias creation mainly to make things easier, but that
method is still very valid, and if the default alias is not good enough
for the driver nothing should prevent you to create a new one.

> So the option here we have is,
> 
> 1. Instead of creating alias with "fck", create an alias only with dev
> pointer, that means con_id = NULL.
> I have already tested this and it works on AM33xx platform.

Do you mean assuming that the fck is always the clock with a NULL
con_id? And any other clocks for that device will have thus to use a
explicit name?

> 2. Add a new flag inside hwmod or omap_device, so that it can be read at
> omap_device layer and based on that we can decide whether to add con_id
> or not while invoking clkdev_alloc().

hwmod is supposed to contain only HW related information. In that case
it will be Linux driver dependent and not really HW dependent.
That one is thus not really acceptable.

> This may be required to support legacy drivers which are not migrated to
> runtime_pm and still calls clk_get() for both "fck" and "ick", so here
> we need "fck" clk alias.

We can use "fck" even with runtime_pm. Just because sometime you need to
get the clock rate of the main clock.

> Any comments or opinion on this? Based on the feedback I can create the
> changes and submit it to the list.

Well, a #3 suggestion might be to enforce the usage of fck alias in the
drivers instead of assuming that the "NULL" entry is the proper one.

And a #4 will be to create a extra entry in the clkdev like you have done.

For my point of view, the #4 seems to be the one that will minimize the
overall change in the drivers and is probably the best one.

That being said, since the DT clock binding is merged now, the whole
automatic alias creation might disappear as soon as we will have DT boot
on all the platforms... OK, we are not really there still :-)

Regards,
Benoit

omap_device: query on "fck" clk alias created

[Russell King - ARM Linux]

On Wed, Aug 01, 2012 at 05:50:16PM +0530, Vaibhav Hiremath wrote:
> The clk_get() api will not work, unless we pass both the arguments (dev,
> con_id) properly. Now expecting driver to always label con_id with "fck"
> is undesirable, as the same driver can be reused on multiple platforms,
> which can be non-omap and/or non-ti platforms.

Why not?

The connection ID is defined by the driver, and the platform stuff is
expected to provide drivers with what they require.  It's not the other
way around (platforms don't tell drivers what they require.)

In other words, if the device has two clocks, one called ick and one called
fck, then the device _should_ use clk_get() specifying "ick" for one, and
"fck" for the other.

And platforms better provide an "ick" and a "fck" for the device, even if
they have no respresentation for one or other of them (in which case you
supply a dummy clock.)

omap_device: query on "fck" clk alias created

[Hiremath, Vaibhav]

On Wed, Aug 01, 2012 at 19:12:59, Cousson, Benoit wrote:
> Hi Vaibhav,
> 
> On 08/01/2012 02:20 PM, Vaibhav Hiremath wrote:
> > Hi,
> > 
> > In OMAP world, we have omap_device layer, which exports api's like
> > omap_device_build() to create and register platform_device to the
> > kernel. This layer understands hwmod infrastructure and parses all the
> > platform specific information from it.
> > Now with DT migration, the same thing is achieved using a notifier,
> > which in turn omap_device_build_from_dt(), which in turn does same thing.
> > 
> > One of the thing which is happening in both execution path is, creating
> > alias for functional clock (hwmod_data->main_clk) for each device
> > getting created with the con_id = "fck".
> > 
> > Now the problem with this is,
> > 
> > The clk_get() api will not work, unless we pass both the arguments (dev,
> > con_id) properly. Now expecting driver to always label con_id with "fck"
> > is undesirable, as the same driver can be reused on multiple platforms,
> > which can be non-omap and/or non-ti platforms.
> > Also we have multiple examples where driver simply calls (which is right)
> > 
> > clk = clk_get(&pdev->dev, NULL);
> 
> Mmm, I don't know, but even if this is right, shouldn't we avoid such
> usage. It might be better to be explicit than assuming that the IP will
> always have an unique clock.

Isn't this IP specific and driver must know how many clocks he has to 
address? So I believe it will not be assumption, the driver is written
considering clocksources and in some cases IP is designed and meant to
receive only one clock input.

Let me bring a real example which I am dealing with now,

DCAN IP integrated in AM335X device, this IP is not from TI. This is Bosch
IP and doesn't follow any of the TI standards, like HighLander spec. Even it doesn't have SYSCONF register for that matter. The driver is generic, 
obviously used across non-ti devices already and driver is implemented with
Clk_get(dev, NULL);


> Some IP does have several inputs that might or not be connected
> depending or the integration or on the version. So even if only one
> clock is useful at some point it will not always be true.
> We had similar issue with the IRQ lines in the past.
> 
> > This would fail on OMAP platforms, unless you modify clockxxx_data.c
> > file with,
> > 
> > CLK("<device>",  NULL,           &xxx_fck,    CK_34XX),
> > 
> > 
> > Devices specially with only one clock source always prefer not to
> > specify con_id.
> 
> How many devices do you have with only one clock source? Could you
> provide the list of drivers that assume that?
>

I did simple grep on drivers/ directory and here is the output of it -


# grep -r clk_get drivers/* | grep -r ", NULL"

drivers/ata/pata_arasan_cf.c:   acdev->clk = clk_get(&pdev->dev, NULL);
drivers/ata/sata_mv.c:  hpriv->clk = clk_get(&pdev->dev, NULL);
drivers/ata/pata_imx.c: priv->clk = clk_get(&pdev->dev, NULL);
drivers/char/hw_random/nomadik-rng.c:   rng_clk = clk_get(&dev->dev, NULL);
drivers/char/hw_random/picoxcell-rng.c: rng_clk = clk_get(&pdev->dev, NULL);
drivers/char/hw_random/atmel-rng.c:     trng->clk = clk_get(&pdev->dev, NULL);
drivers/crypto/ux500/cryp/cryp_core.c:  device_data->clk = clk_get(&pdev->dev, NULL);
drivers/crypto/ux500/hash/hash_core.c:  device_data->clk = clk_get(dev, NULL);
drivers/crypto/mv_cesa.c:       cp->clk = clk_get(&pdev->dev, NULL);
drivers/dma/ipu/ipu_idmac.c:    ipu_data.ipu_clk = clk_get(&pdev->dev, NULL);
drivers/dma/mv_xor.c:   msp->clk = clk_get(&pdev->dev, NULL);
drivers/dma/mxs-dma.c:  mxs_dma->clk = clk_get(&pdev->dev, NULL);
drivers/dma/ste_dma40.c:        clk = clk_get(&pdev->dev, NULL);
drivers/gpio/gpio-pxa.c:        clk = clk_get(&pdev->dev, NULL);
drivers/gpio/gpio-stp-xway.c:   clk = clk_get(&pdev->dev, NULL);
drivers/i2c/busses/i2c-davinci.c:       dev->clk = clk_get(&pdev->dev, NULL);
drivers/i2c/busses/i2c-sirf.c:  clk = clk_get(&pdev->dev, NULL);
drivers/i2c/busses/i2c-designware-platdrv.c:    dev->clk = clk_get(&pdev->dev, NULL);
drivers/i2c/busses/i2c-nomadik.c:       dev->clk = clk_get(&adev->dev, NULL);
drivers/i2c/busses/i2c-nuc900.c:        i2c->clk = clk_get(&pdev->dev, NULL);
drivers/i2c/busses/i2c-tegra.c: clk = devm_clk_get(&pdev->dev, NULL);
drivers/i2c/busses/i2c-pxa.c:   i2c->clk = clk_get(&dev->dev, NULL);
drivers/i2c/busses/i2c-sh_mobile.c:     pd->clk = clk_get(&dev->dev, NULL);
drivers/i2c/busses/i2c-imx.c:   i2c_imx->clk = devm_clk_get(&pdev->dev, NULL);
drivers/i2c/busses/i2c-pnx.c:   alg_data->clk = clk_get(&pdev->dev, NULL);
drivers/ide/palm_bk3710.c:      clk = clk_get(&pdev->dev, NULL);
drivers/input/keyboard/nomadik-ske-keypad.c:    keypad->clk = clk_get(&pdev->dev, NULL);
drivers/input/keyboard/tnetv107x-keypad.c:      kp->clk = clk_get(dev, NULL);
drivers/input/keyboard/pxa27x_keypad.c: keypad->clk = clk_get(&pdev->dev, NULL);
drivers/input/keyboard/tegra-kbc.c:     kbc->clk = clk_get(&pdev->dev, NULL);
drivers/input/keyboard/spear-keyboard.c:        kbd->clk = clk_get(&pdev->dev, NULL);
drivers/input/keyboard/ep93xx_keypad.c: keypad->clk = clk_get(&pdev->dev, NULL);
drivers/input/keyboard/w90p910_keypad.c:        keypad->clk = clk_get(&pdev->dev, NULL);
drivers/input/touchscreen/lpc32xx_ts.c: tsc->clk = clk_get(&pdev->dev, NULL);
drivers/input/touchscreen/w90p910_ts.c: w90p910_ts->clk = clk_get(&pdev->dev, NULL);
drivers/input/touchscreen/tnetv107x-ts.c:       ts->clk = clk_get(dev, NULL);
drivers/leds/leds-renesas-tpu.c:        p->clk = clk_get(&pdev->dev, NULL);
drivers/media/video/mx3_camera.c:       mx3_cam->clk = clk_get(&pdev->dev, NULL);
drivers/media/video/pxa_camera.c:       pcdev->clk = clk_get(&pdev->dev, NULL);
drivers/media/video/mx2_camera.c:       pcdev->clk_csi = clk_get(&pdev->dev, NULL);
drivers/media/video/mx2_emmaprp.c:      pcdev->clk_emma = clk_get(&pdev->dev, NULL);
drivers/mfd/davinci_voicecodec.c:       davinci_vc->clk = clk_get(&pdev->dev, NULL);
drivers/mfd/ti-ssp.c:   ssp->clk = clk_get(dev, NULL);
drivers/misc/spear13xx_pcie_gadget.c:           clk = clk_get_sys("pcie1", NULL);
drivers/misc/spear13xx_pcie_gadget.c:           clk = clk_get_sys("pcie2", NULL);
drivers/mmc/host/mxs-mmc.c:     host->clk = clk_get(&pdev->dev, NULL);
drivers/mmc/host/mvsdio.c:      host->clk = clk_get(&pdev->dev, NULL);
drivers/mmc/host/pxamci.c:      host->clk = clk_get(&pdev->dev, NULL);
drivers/mmc/host/sdhci-spear.c: sdhci->clk = clk_get(&pdev->dev, NULL);
drivers/mmc/host/sdhci-tegra.c: clk = clk_get(mmc_dev(host->mmc), NULL);
drivers/mmc/host/mmci.c:        host->clk = clk_get(&dev->dev, NULL);
drivers/mtd/devices/spear_smi.c:        dev->clk = clk_get(&pdev->dev, NULL);
drivers/mtd/nand/gpmi-nand/gpmi-nand.c: res->clock = clk_get(&this->pdev->dev, NULL);
drivers/mtd/nand/fsmc_nand.c:   host->clk = clk_get(&pdev->dev, NULL);
drivers/mtd/nand/nuc900_nand.c: nuc900_nand->clk = clk_get(&pdev->dev, NULL);
drivers/mtd/nand/orion_nand.c:  clk = clk_get(&pdev->dev, NULL);
drivers/mtd/nand/orion_nand.c:  clk = clk_get(&pdev->dev, NULL);
drivers/mtd/nand/pxa3xx_nand.c: info->clk = clk_get(&pdev->dev, NULL);
drivers/net/can/c_can/c_can_platform.c: clk = clk_get(&pdev->dev, NULL);
drivers/net/can/flexcan.c:              clk = clk_get(&pdev->dev, NULL);



 
> Most IPs in OMAP does have a fck and an ick so that does not seems nice
> or even consistent to me to do a clk_get(&pdev->dev, NULL) for "fck" and
> clk_get(&pdev->dev, "ick") for the ick.
> 

I completely agree from OMAP device perspective, but it may not be 
applicable for other architectures or devices, right?


> > And it seems wrong thing, as across platforms IP integration can be very
> > different and you can not expect to change the clock-tree table always.
> 
> FWIW, that table is done for that exact purpose. And we introduced the
> automatic "fck" alias creation mainly to make things easier, but that
> method is still very valid, and if the default alias is not good enough
> for the driver nothing should prevent you to create a new one.
> 
> > So the option here we have is,
> > 
> > 1. Instead of creating alias with "fck", create an alias only with dev
> > pointer, that means con_id = NULL.
> > I have already tested this and it works on AM33xx platform.
> 
> Do you mean assuming that the fck is always the clock with a NULL
> con_id? And any other clocks for that device will have thus to use a
> explicit name?
> 

Yes.

> > 2. Add a new flag inside hwmod or omap_device, so that it can be read at
> > omap_device layer and based on that we can decide whether to add con_id
> > or not while invoking clkdev_alloc().
> 
> hwmod is supposed to contain only HW related information. In that case
> it will be Linux driver dependent and not really HW dependent.
> That one is thus not really acceptable.
> 

Agreed, that was one of the options. What about omap_device???

> > This may be required to support legacy drivers which are not migrated to
> > runtime_pm and still calls clk_get() for both "fck" and "ick", so here
> > we need "fck" clk alias.
> 
> We can use "fck" even with runtime_pm. Just because sometime you need to
> get the clock rate of the main clock.
> 
> > Any comments or opinion on this? Based on the feedback I can create the
> > changes and submit it to the list.
> 
> Well, a #3 suggestion might be to enforce the usage of fck alias in the
> drivers instead of assuming that the "NULL" entry is the proper one.
> 

Can we??? Not sure whether we can really force this.

> And a #4 will be to create a extra entry in the clkdev like you have done.
> 

If nothing works out from above options, then this is the only option we 
have.
But the issue here is, how can we fix dev_id? With DT support I have to 
Really make use of "of_dev_auxdata" to fix dev id.


> For my point of view, the #4 seems to be the one that will minimize the
> overall change in the drivers and is probably the best one.
> 
> That being said, since the DT clock binding is merged now, the whole
> automatic alias creation might disappear as soon as we will have DT boot
> on all the platforms... OK, we are not really there still :-)
> 

Yeah, I know and understand. This is another issue we have. So we should not consider automatic aliases here.


Thanks,
Vaibhav

omap_device: query on "fck" clk alias created

[Hiremath, Vaibhav]

On Wed, Aug 01, 2012 at 19:44:14, Russell King - ARM Linux wrote:
> On Wed, Aug 01, 2012 at 05:50:16PM +0530, Vaibhav Hiremath wrote:
> > The clk_get() api will not work, unless we pass both the arguments (dev,
> > con_id) properly. Now expecting driver to always label con_id with "fck"
> > is undesirable, as the same driver can be reused on multiple platforms,
> > which can be non-omap and/or non-ti platforms.
> 
> Why not?
> 
> The connection ID is defined by the driver, and the platform stuff is
> expected to provide drivers with what they require.  It's not the other
> way around (platforms don't tell drivers what they require.)
> 
> In other words, if the device has two clocks, one called ick and one called
> fck, then the device _should_ use clk_get() specifying "ick" for one, and
> "fck" for the other.
> 
> And platforms better provide an "ick" and a "fck" for the device, even if
> they have no respresentation for one or other of them (in which case you
> supply a dummy clock.)
> 

Russell, I completely agree with you, infact I am on the same page.
Let me give you a real example, which I am dealing with now,

On AM335x device we have integrated DCAN, this IP is not from TI.
IP is from Bosch and doesn't follow any of the TI IP standards, like 
HighLander spec. The IP is just integrated with interconnect bus and given a 
required clocksource (in this case its one clock). 

The driver for this IP is generic, (drivers/net/can/c_can/c_can_platform.c) 
obviously used across non-ti devices already and driver is implemented with,
    clk_get(dev, NULL);


So in this context,

  1. Either I have to change the driver to specify con_id. Modifying a 
     generic driver.
OR
  2. Change the clock tree table to add an clock entry for this clock with 
     con_id = NULL, but I have to fix dev_id as well. I believe this I can 
     achieve using " of_dev_auxdata".
OR
  3. Create an alias at run time (omap is doing currently) with using 
     dynamic dev_id and con_id = NULL.


Thanks,
Vaibhav

omap_device: query on "fck" clk alias created

[Russell King - ARM Linux]

On Thu, Aug 02, 2012 at 12:55:42PM +0000, Hiremath, Vaibhav wrote:
> On Wed, Aug 01, 2012 at 19:12:59, Cousson, Benoit wrote:
> > Mmm, I don't know, but even if this is right, shouldn't we avoid such
> > usage. It might be better to be explicit than assuming that the IP will
> > always have an unique clock.
> 
> Isn't this IP specific and driver must know how many clocks he has to 
> address? So I believe it will not be assumption, the driver is written
> considering clocksources and in some cases IP is designed and meant to
> receive only one clock input.

Devices themselves define what clocks they need to operate.  Drivers
designed to be used with the clk API are supposed to obtain all clocks
that the device requires.  The device dictates what clocks are required.

This does not change the way drivers deal with clocks depending on how
they're integrated into a SoC: devices that need five clocks should
_always_ call clk_get() five times to get each of those five clocks.

Where a clock is not individually distinguishable or available in a SoC,
the SoC level must arrange to return a dummy clock to the device driver.

If a device only takes one single clock, then using a connection ID of
NULL is acceptable.  Otherwise, an explicit name must be used, which is
chosen either from the device IPs documentation (eg, the name of the
_clock_ _input_) or with a lack of that information, a name chosen by
the driver writer according to the function of the clock.

You must stop thinking about the picture you get when something gets
integrated into a SoC... and making driver code conditional on the view
presented by the SoC.

omap_device: query on "fck" clk alias created

[Russell King - ARM Linux]

On Thu, Aug 02, 2012 at 01:04:37PM +0000, Hiremath, Vaibhav wrote:
> On Wed, Aug 01, 2012 at 19:44:14, Russell King - ARM Linux wrote:
> > On Wed, Aug 01, 2012 at 05:50:16PM +0530, Vaibhav Hiremath wrote:
> > > The clk_get() api will not work, unless we pass both the arguments (dev,
> > > con_id) properly. Now expecting driver to always label con_id with "fck"
> > > is undesirable, as the same driver can be reused on multiple platforms,
> > > which can be non-omap and/or non-ti platforms.
> > 
> > Why not?
> > 
> > The connection ID is defined by the driver, and the platform stuff is
> > expected to provide drivers with what they require.  It's not the other
> > way around (platforms don't tell drivers what they require.)
> > 
> > In other words, if the device has two clocks, one called ick and one called
> > fck, then the device _should_ use clk_get() specifying "ick" for one, and
> > "fck" for the other.
> > 
> > And platforms better provide an "ick" and a "fck" for the device, even if
> > they have no respresentation for one or other of them (in which case you
> > supply a dummy clock.)
> > 
> 
> Russell, I completely agree with you, infact I am on the same page.
> Let me give you a real example, which I am dealing with now,
> 
> On AM335x device we have integrated DCAN, this IP is not from TI.
> IP is from Bosch and doesn't follow any of the TI IP standards, like 
> HighLander spec. The IP is just integrated with interconnect bus and given a 
> required clocksource (in this case its one clock). 
> 
> The driver for this IP is generic, (drivers/net/can/c_can/c_can_platform.c) 
> obviously used across non-ti devices already and driver is implemented with,
>     clk_get(dev, NULL);

So, elsewhere it only has one clock.  Are there any other clocks it needs
on OMAP?

If the answer is no, then the driver is doing the right thing, and OMAP's
clk stuff needs to be adjusted to suit the drivers requirements.

I've said many times over the years that clock connection IDs are defined
by the device or driver and *nothing* else - they're certainly *not* source
names.

That's why, when I modified OMAP drivers to fix them, and they take an
interface and function clock, they end up asking for "ick" and "fck" not
"blah_ick" and "blah_fck".

omap_device: query on "fck" clk alias created

[Hiremath, Vaibhav]

On Thu, Aug 02, 2012 at 18:42:50, Russell King - ARM Linux wrote:
> On Thu, Aug 02, 2012 at 01:04:37PM +0000, Hiremath, Vaibhav wrote:
> > On Wed, Aug 01, 2012 at 19:44:14, Russell King - ARM Linux wrote:
> > > On Wed, Aug 01, 2012 at 05:50:16PM +0530, Vaibhav Hiremath wrote:
> > > > The clk_get() api will not work, unless we pass both the arguments (dev,
> > > > con_id) properly. Now expecting driver to always label con_id with "fck"
> > > > is undesirable, as the same driver can be reused on multiple platforms,
> > > > which can be non-omap and/or non-ti platforms.
> > > 
> > > Why not?
> > > 
> > > The connection ID is defined by the driver, and the platform stuff is
> > > expected to provide drivers with what they require.  It's not the other
> > > way around (platforms don't tell drivers what they require.)
> > > 
> > > In other words, if the device has two clocks, one called ick and one called
> > > fck, then the device _should_ use clk_get() specifying "ick" for one, and
> > > "fck" for the other.
> > > 
> > > And platforms better provide an "ick" and a "fck" for the device, even if
> > > they have no respresentation for one or other of them (in which case you
> > > supply a dummy clock.)
> > > 
> > 
> > Russell, I completely agree with you, infact I am on the same page.
> > Let me give you a real example, which I am dealing with now,
> > 
> > On AM335x device we have integrated DCAN, this IP is not from TI.
> > IP is from Bosch and doesn't follow any of the TI IP standards, like 
> > HighLander spec. The IP is just integrated with interconnect bus and given a 
> > required clocksource (in this case its one clock). 
> > 
> > The driver for this IP is generic, (drivers/net/can/c_can/c_can_platform.c) 
> > obviously used across non-ti devices already and driver is implemented with,
> >     clk_get(dev, NULL);
> 
> So, elsewhere it only has one clock.  Are there any other clocks it needs
> on OMAP?
> 
> If the answer is no, then the driver is doing the right thing, and OMAP's
> clk stuff needs to be adjusted to suit the drivers requirements.
> 
> I've said many times over the years that clock connection IDs are defined
> by the device or driver and *nothing* else - they're certainly *not* source
> names.
> 
> That's why, when I modified OMAP drivers to fix them, and they take an
> interface and function clock, they end up asking for "ick" and "fck" not
> "blah_ick" and "blah_fck".
> 

I am in agreement with you and I believe I came to the solution for this 
issue, I used "of_dev_auxdata" to manipulate dev_id and updated clock-tree 
table with required clock entry and it works, I tried it on BeagleBone 
platform.

Can you please review it?




diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c
index 6f93a20..893cc0c 100644
--- a/arch/arm/mach-omap2/board-generic.c
+++ b/arch/arm/mach-omap2/board-generic.c
@@ -37,11 +37,24 @@ static struct of_device_id omap_dt_match_table[] __initdata = {
        { }
 };

+/*
+ * Lookup table for attaching a specific name and platform_data        pointer to
+ * devices as they get created by of_platform_populate().  Ideally this table
+ * would not exist, but the current clock implementation depends on some devices
+ * having a specific name.
+ */
+static const struct of_dev_auxdata omap_auxdata_lookup[] __initconst = {
+       OF_DEV_AUXDATA("bosch,d_can", 0x481cc000, "d_can.0", NULL),
+       OF_DEV_AUXDATA("bosch,d_can", 0x481d0000, "d_can.1", NULL),
+       { },
+};
+
+
 static void __init omap_generic_init(void)
 {
        omap_sdrc_init(NULL, NULL);

-       of_platform_populate(NULL, omap_dt_match_table, NULL, NULL);
+       of_platform_populate(NULL, omap_dt_match_table, omap_auxdata_lookup, NULL);
}

 #ifdef CONFIG_SOC_OMAP2420
diff --git a/arch/arm/mach-omap2/clock33xx_data.c b/arch/arm/mach-omap2/clock33xx_data.c
index ae27de8..f061834 100644
--- a/arch/arm/mach-omap2/clock33xx_data.c
+++ b/arch/arm/mach-omap2/clock33xx_data.c
@@ -1028,6 +1028,8 @@ static struct omap_clk am33xx_clks[] = {
        CLK(NULL,       "clkdiv32k_ick",        &clkdiv32k_ick, CK_AM33XX),
        CLK(NULL,       "dcan0_fck",            &dcan0_fck,     CK_AM33XX),
        CLK(NULL,       "dcan1_fck",            &dcan1_fck,     CK_AM33XX),
+       CLK("d_can.0",  NULL,           &dcan0_fck,     CK_AM33XX),
+       CLK("d_can.1",  NULL,           &dcan1_fck,     CK_AM33XX),
        CLK(NULL,       "debugss_ick",          &debugss_ick,   CK_AM33XX),
        CLK(NULL,       "pruss_ocp_gclk",       &pruss_ocp_gclk,        CK_AM33XX),
        CLK("davinci-mcasp.0",  NULL,           &mcasp0_fck,    CK_AM33XX),