| Message ID | 87fxd0ff4q.fsf@deeprootsystems.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
Kevin Hilman wrote: > Well, it's not actually this patch that started the problem, it was > the platform_data updates where clock names were added as part of the > new platform_data struct shared between platform code and ASoC. > > My original proposal was to pass a clock pointer, not a clock name, > but that was not done, so... > > I've pushed an update my 'temp/asoc' branch with the patch below which > converts to pasing clock pointers instead of clock names. > > I'll rework this into two parts. One for DaVinci git and one for > Mark's for-2.6.32 branch and submit. > > Kevin > Can you explain the philosophy of why this is better than the previous? They look about the same to me. Both call clk_get with a name string, just in a different place.
Troy Kisky <troy.kisky@boundarydevices.com> writes: > Kevin Hilman wrote: >> Well, it's not actually this patch that started the problem, it was >> the platform_data updates where clock names were added as part of the >> new platform_data struct shared between platform code and ASoC. >> >> My original proposal was to pass a clock pointer, not a clock name, >> but that was not done, so... >> >> I've pushed an update my 'temp/asoc' branch with the patch below which >> converts to pasing clock pointers instead of clock names. >> >> I'll rework this into two parts. One for DaVinci git and one for >> Mark's for-2.6.32 branch and submit. >> >> Kevin >> > > Can you explain the philosophy of why this is better than the > previous? They look about the same to me. Both call clk_get with a > name string, just in a different place. Correct. But in the updatd version, the code that uses the name is platform specific code that has to know which physical clock to hook up. The point is that we should not be passing around platform-dependent physical clock name strings to platform-indepenent code. Kevin
Kevin Hilman wrote: > The point is that we should not be passing around platform-dependent > physical clock name strings to platform-indepenent code. > > Kevin > I don't see the light, but that's ok. I'm not afraid of the dark. Troy
Russell King - ARM Linux <linux@arm.linux.org.uk> writes: > On Mon, Jul 13, 2009 at 02:27:01PM -0700, Kevin Hilman wrote: >> Well, it's not actually this patch that started the problem, it was >> the platform_data updates where clock names were added as part of the >> new platform_data struct shared between platform code and ASoC. >> >> My original proposal was to pass a clock pointer, not a clock name, >> but that was not done, so... >> >> I've pushed an update my 'temp/asoc' branch with the patch below which >> converts to pasing clock pointers instead of clock names. > > Also wrong. Why do you want to pass a clock name _or_ a clock pointer? > Why not follow the clk API and derive it from the struct device. This is how it is currently is in davinci mainline. There was some objection to this on the ASoC side when support for a new DaVinci SoC was added, so things were changed to pass clock name strings. I need to better understand from ASoC folks why the original model wasn't working so we try to keep the current mainline code, or possibly use clk_add_alias() if needed. Kevin
diff --git a/arch/arm/mach-davinci/board-dm355-evm.c b/arch/arm/mach-davinci/board-dm355-evm.c index 78a9604..cd87f89 100644 --- a/arch/arm/mach-davinci/board-dm355-evm.c +++ b/arch/arm/mach-davinci/board-dm355-evm.c @@ -118,9 +118,7 @@ static struct davinci_i2c_platform_data i2c_pdata = { .bus_delay = 0 /* usec */, }; -static struct snd_platform_data dm355_evm_snd_data = { - .clk_name = "asp1", -}; +static struct snd_platform_data dm355_evm_snd_data; static int dm355evm_mmc_gpios = -EINVAL; @@ -286,7 +284,10 @@ static __init void dm355_evm_init(void) ARRAY_SIZE(dm355_evm_spi_info)); /* DM335 EVM uses ASP1; line-out is a stereo mini-jack */ - dm355_init_asp1(ASP1_TX_EVT_EN | ASP1_RX_EVT_EN, &dm355_evm_snd_data); + dm355_evm_snd_data.clk = clk_get(NULL, "asp1"); + if (!WARN_ON(!dm355_evm_snd_data.clk)) + dm355_init_asp1(ASP1_TX_EVT_EN | ASP1_RX_EVT_EN, + &dm355_evm_snd_data); } static __init void dm355_evm_irq_init(void) diff --git a/arch/arm/mach-davinci/board-dm644x-evm.c b/arch/arm/mach-davinci/board-dm644x-evm.c index 30b2fd4..611583b 100644 --- a/arch/arm/mach-davinci/board-dm644x-evm.c +++ b/arch/arm/mach-davinci/board-dm644x-evm.c @@ -226,9 +226,7 @@ static struct platform_device ide_dev = { }, }; -static struct snd_platform_data dm644x_evm_snd_data = { - .clk_name = "asp0", -}; +static struct snd_platform_data dm644x_evm_snd_data; /*----------------------------------------------------------------------*/ @@ -671,7 +669,10 @@ static __init void davinci_evm_init(void) davinci_setup_mmc(0, &dm6446evm_mmc_config); davinci_serial_init(&uart_config); - dm644x_init_asp(&dm644x_evm_snd_data); + + dm644x_evm_snd_data.clk = clk_get(NULL, "asp0"); + if (!WARN_ON(!dm644x_evm_snd_data.clk)) + dm644x_init_asp(&dm644x_evm_snd_data); soc_info->emac_pdata->phy_mask = DM644X_EVM_PHY_MASK; soc_info->emac_pdata->mdio_max_freq = DM644X_EVM_MDIO_FREQUENCY; diff --git a/arch/arm/mach-davinci/board-dm646x-evm.c b/arch/arm/mach-davinci/board-dm646x-evm.c index 575c6ca..b8fe6f4 100644 --- a/arch/arm/mach-davinci/board-dm646x-evm.c +++ b/arch/arm/mach-davinci/board-dm646x-evm.c @@ -33,6 +33,7 @@ #include <linux/i2c/at24.h> #include <linux/i2c/pcf857x.h> #include <linux/etherdevice.h> +#include <linux/clk.h> #include <asm/setup.h> #include <asm/mach-types.h> @@ -217,7 +218,6 @@ static u8 dm646x_dit_serializer_direction[] = { static struct snd_platform_data dm646x_evm_snd_data[] = { { - .clk_name = "mcasp0", .tx_dma_offset = 0x400, .rx_dma_offset = 0x400, .op_mode = DAVINCI_MCASP_IIS_MODE, @@ -227,7 +227,6 @@ static struct snd_platform_data dm646x_evm_snd_data[] = { .eventq_no = EVENTQ_0, }, { - .clk_name = "mcasp1", .tx_dma_offset = 0x400, .rx_dma_offset = 0, .op_mode = DAVINCI_MCASP_DIT_MODE, @@ -271,8 +270,13 @@ static __init void evm_init(void) evm_init_i2c(); davinci_serial_init(&uart_config); - dm646x_init_mcasp0(&dm646x_evm_snd_data[0]); - dm646x_init_mcasp1(&dm646x_evm_snd_data[1]); + + dm646x_evm_snd_data[0].clk = clk_get(NULL, "mcasp0"); + if (!WARN_ON(!dm646x_evm_snd_data[0].clk)) + dm646x_init_mcasp0(&dm646x_evm_snd_data[0]); + dm646x_evm_snd_data[1].clk = clk_get(NULL, "mcasp1"); + if (!WARN_ON(!dm646x_evm_snd_data[1].clk)) + dm646x_init_mcasp1(&dm646x_evm_snd_data[1]); soc_info->emac_pdata->phy_mask = DM646X_EVM_PHY_MASK; soc_info->emac_pdata->mdio_max_freq = DM646X_EVM_MDIO_FREQUENCY; diff --git a/arch/arm/mach-davinci/include/mach/asp.h b/arch/arm/mach-davinci/include/mach/asp.h index 038ecb7..2df4ecb 100644 --- a/arch/arm/mach-davinci/include/mach/asp.h +++ b/arch/arm/mach-davinci/include/mach/asp.h @@ -33,7 +33,7 @@ #define DAVINCI_ASP1_TX_INT IRQ_MBXINT struct snd_platform_data { - char *clk_name; + struct clk *clk; u32 tx_dma_offset; u32 rx_dma_offset; enum dma_event_q eventq_no; /* event queue number */ diff --git a/sound/soc/davinci/davinci-i2s.c b/sound/soc/davinci/davinci-i2s.c index f7330e1..ddc047e 100644 --- a/sound/soc/davinci/davinci-i2s.c +++ b/sound/soc/davinci/davinci-i2s.c @@ -528,7 +528,7 @@ static int davinci_i2s_probe(struct platform_device *pdev) goto err_release_region; } - dev->clk = clk_get(&pdev->dev, pdata->clk_name); + dev->clk = pdata->clk; if (IS_ERR(dev->clk)) { ret = -ENODEV; goto err_free_mem; diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c index b27aab6..3b095f9 100644 --- a/sound/soc/davinci/davinci-mcasp.c +++ b/sound/soc/davinci/davinci-mcasp.c @@ -764,7 +764,7 @@ static int davinci_mcasp_probe(struct platform_device *pdev) } pdata = pdev->dev.platform_data; - dev->clk = clk_get(&pdev->dev, pdata->clk_name); + dev->clk = pdata->clk; if (IS_ERR(dev->clk)) { ret = -ENODEV; goto err_release_region;