From: Lee Jones <hidden> Date: 2012-07-26 10:29:18
When booting via platform code the AB8500 platform data is now passed
in though the DB8500. However, if pdata_size is not set it will not be
subsequently passed onto subordinate devices. This patch correctly
populates pdata_size.
Signed-off-by: Lee Jones <redacted>
---
drivers/mfd/db8500-prcmu.c | 1 +
1 file changed, 1 insertion(+)
From: Lee Jones <hidden> Date: 2012-07-26 10:29:22
This patch contains a couple of general MSP clean-ups and a bug-fix.
The general clean-ups pertain to layout changes and changing functions
to be void instead of int and not to regardlessly return '0'. The bug
that's fixed is thought to be caused by a merge error. The code
erroneously attempts to platform_device_register a non-existent struct.
It's a simple fix, just reference the correct platform_data structure
in its place.
Signed-off-by: Lee Jones <redacted>
---
arch/arm/mach-ux500/board-mop500-msp.c | 28 +++++++++++++---------------
arch/arm/mach-ux500/board-mop500-msp.h | 14 --------------
arch/arm/mach-ux500/board-mop500.c | 1 -
arch/arm/mach-ux500/board-mop500.h | 3 +++
4 files changed, 16 insertions(+), 30 deletions(-)
delete mode 100644 arch/arm/mach-ux500/board-mop500-msp.h
@@ -1,14 +0,0 @@-/*- * Copyright (C) ST-Ericsson SA 2012- *- * Author: Ola Lilja <ola.o.lilja@stericsson.com>,- * for ST-Ericsson.- *- * License terms:- *- * This program is free software; you can redistribute it and/or modify- * it under the terms of the GNU General Public License version 2 as published- * by the Free Software Foundation.- */--void mop500_msp_init(struct device *parent);
@@ -92,6 +92,9 @@ void __init mop500_stuib_init(void);void__initmop500_pinmaps_init(void);void__initsnowball_pinmaps_init(void);void__inithrefv60_pinmaps_init(void);+voidmop500_msp_init(structdevice*parent);+/* Due for removal once the MSP driver has been fully DT:ed. */+voidmop500_of_msp_init(structdevice*parent);int__initmop500_uib_init(void);voidmop500_uib_i2c_add(intbusnum,structi2c_board_info*info,
From: Lee Jones <hidden> Date: 2012-07-26 10:29:25
If a list of widgets is provided and one of them fails to be added as
a control, the present semantics fail all subsequent widgets. A better
solution would be to only fail that widget, but pursue in attempting
to add the rest of the list.
Signed-off-by: Lee Jones <redacted>
---
sound/soc/soc-dapm.c | 2 --
1 file changed, 2 deletions(-)
@@ -3095,8 +3095,6 @@ int snd_soc_dapm_new_controls(struct snd_soc_dapm_context *dapm,dev_err(dapm->dev,"ASoC: Failed to create DAPM control %s\n",widget->name);-ret=-ENOMEM;-break;}widget++;}
From: Lee Jones <hidden> Date: 2012-07-26 10:29:27
Ensure correct probing and pass though important configuration
options to the AB8500 CODEC driver when DT is enabled
Signed-off-by: Lee Jones <redacted>
---
arch/arm/boot/dts/db8500.dtsi | 6 ++++++
1 file changed, 6 insertions(+)
@@ -370,6 +370,12 @@compatible="stericsson,ab8500-debug";};+codec:ab8500-codec{+compatible="stericsson,ab8500-codec";++stericsson,earpeice-cmv=<950>;/* Units in mV. */+};+ab8500-regulators{compatible="stericsson,ab8500-regulator";
From: Lee Jones <hidden> Date: 2012-07-26 10:29:29
List all four MSP devices which exist on all DB8500 based platforms,
to ensure correct device probing and configuration passing when booting
with DT.
Signed-off-by: Lee Jones <redacted>
---
arch/arm/boot/dts/db8500.dtsi | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
From: Lee Jones <hidden> Date: 2012-07-26 10:29:32
This is the node which links together the platform (PCM), codec (AB8500)
and the CPU side Digital Audio Interface (MCP) with the machine driver.
Signed-off-by: Lee Jones <redacted>
---
arch/arm/boot/dts/db8500.dtsi | 8 ++++++++
1 file changed, 8 insertions(+)
From: Lee Jones <hidden> Date: 2012-07-26 10:29:37
Previous attempts to add platform probing of the Audio related devices
only call from non-DT initialisation functions. This patch extends that
functionality to the Device Tree related ones too.
Signed-off-by: Lee Jones <redacted>
---
arch/arm/mach-ux500/board-mop500.c | 4 ++++
1 file changed, 4 insertions(+)
From: Lee Jones <hidden> Date: 2012-07-26 10:29:39
It isn't currently possible to pass all platform specific configuration
though Device Tree. Thinks like device names used in the clock
infrastructure, call-backs and DMA information have to be passed in via
AUXDATA structures and the MSP is no exception. Here we're passing DMA
settings.
Signed-off-by: Lee Jones <redacted>
---
arch/arm/mach-ux500/board-mop500-msp.c | 8 ++++----
arch/arm/mach-ux500/board-mop500.c | 9 +++++++++
arch/arm/mach-ux500/board-mop500.h | 5 +++++
3 files changed, 18 insertions(+), 4 deletions(-)
@@ -9,6 +9,7 @@/* For NOMADIK_NR_GPIO */#include<mach/irqs.h>+#include<mach/msp.h>#include<linux/amba/mmci.h>/* Snowball specific GPIO assignments, this board has no GPIO expander */
From: Lee Jones <hidden> Date: 2012-07-26 10:29:42
Here we pass platform registration from platform code over to Device
Tree, when DT is enabled.
Signed-off-by: Lee Jones <redacted>
---
arch/arm/mach-ux500/board-mop500-msp.c | 3 ---
sound/soc/ux500/ux500_pcm.c | 6 ++++++
2 files changed, 6 insertions(+), 3 deletions(-)
From: Lee Jones <hidden> Date: 2012-07-26 10:29:44
In the initial submission of the MSP driver msp1 and msp3's associated
pinctrl mechanism was passed back to platform code using a plat_init()
call-back routine, but it has no place in platform code. The MSP driver
should set this up for the appropriate ports. Instead we use a use_pinctrl
identifier which is passed from platform_data/Device Tree which indicates
which ports should use pinctrl.
Signed-off-by: Lee Jones <redacted>
---
arch/arm/mach-ux500/board-mop500-msp.c | 101 ++------------------------------
arch/arm/mach-ux500/include/mach/msp.h | 3 +-
sound/soc/ux500/ux500_msp_i2s.c | 81 ++++++++++++++++++++-----
sound/soc/ux500/ux500_msp_i2s.h | 3 +-
4 files changed, 71 insertions(+), 117 deletions(-)
@@ -219,83 +171,38 @@ struct msp_i2s_platform_data msp3_platform_data = {.id=MSP_I2S_3,.msp_i2s_dma_rx=&msp1_dma_rx,.msp_i2s_dma_tx=NULL,-.msp_i2s_init=msp13_i2s_init,-.msp_i2s_exit=msp13_i2s_exit,+.use_pinctrl=true,};/* Due for removal once the MSP driver has been fully DT:ed. */voidmop500_of_msp_init(structdevice*parent){-structplatform_device*msp1;-pr_info("Initialize MSP I2S-devices.\n");db8500_add_msp_i2s(parent,0,U8500_MSP0_BASE,IRQ_DB8500_MSP0,&msp0_platform_data);-msp1=db8500_add_msp_i2s(parent,1,U8500_MSP1_BASE,IRQ_DB8500_MSP1,+db8500_add_msp_i2s(parent,1,U8500_MSP1_BASE,IRQ_DB8500_MSP1,&msp1_platform_data);db8500_add_msp_i2s(parent,2,U8500_MSP2_BASE,IRQ_DB8500_MSP2,&msp2_platform_data);db8500_add_msp_i2s(parent,3,U8500_MSP3_BASE,IRQ_DB8500_MSP1,&msp3_platform_data);--/* Get the pinctrl handle for MSP1 */-if(msp1){-msp1_p=pinctrl_get(&msp1->dev);-if(IS_ERR(msp1_p))-dev_err(&msp1->dev,"could not get MSP1 pinctrl\n");-else{-msp1_def=pinctrl_lookup_state(msp1_p,-PINCTRL_STATE_DEFAULT);-if(IS_ERR(msp1_def)){-dev_err(&msp1->dev,-"could not get MSP1 defstate\n");-}-msp1_sleep=pinctrl_lookup_state(msp1_p,-PINCTRL_STATE_SLEEP);-if(IS_ERR(msp1_sleep))-dev_err(&msp1->dev,-"could not get MSP1 idlestate\n");-}-}}voidmop500_msp_init(structdevice*parent){-structplatform_device*msp1;-pr_info("%s: Register platform-device 'snd-soc-u8500'.\n",__func__);platform_device_register(&snd_soc_mop500);pr_info("Initialize MSP I2S-devices.\n");db8500_add_msp_i2s(parent,0,U8500_MSP0_BASE,IRQ_DB8500_MSP0,&msp0_platform_data);-msp1=db8500_add_msp_i2s(parent,1,U8500_MSP1_BASE,IRQ_DB8500_MSP1,+db8500_add_msp_i2s(parent,1,U8500_MSP1_BASE,IRQ_DB8500_MSP1,&msp1_platform_data);db8500_add_msp_i2s(parent,2,U8500_MSP2_BASE,IRQ_DB8500_MSP2,&msp2_platform_data);db8500_add_msp_i2s(parent,3,U8500_MSP3_BASE,IRQ_DB8500_MSP1,&msp3_platform_data);-/* Get the pinctrl handle for MSP1 */-if(msp1){-msp1_p=pinctrl_get(&msp1->dev);-if(IS_ERR(msp1_p))-dev_err(&msp1->dev,"could not get MSP1 pinctrl\n");-else{-msp1_def=pinctrl_lookup_state(msp1_p,-PINCTRL_STATE_DEFAULT);-if(IS_ERR(msp1_def)){-dev_err(&msp1->dev,-"could not get MSP1 defstate\n");-}-msp1_sleep=pinctrl_lookup_state(msp1_p,-PINCTRL_STATE_SLEEP);-if(IS_ERR(msp1_sleep))-dev_err(&msp1->dev,-"could not get MSP1 idlestate\n");-}-}-pr_info("%s: Register platform-device 'ux500-pcm'\n",__func__);platform_device_register(&ux500_pcm);}
@@ -352,17 +364,23 @@ static int configure_multichannel(struct ux500_msp *msp,staticintenable_msp(structux500_msp*msp,structux500_msp_config*config){-intstatus=0;+intstatus=0,retval=0;u32reg_val_DMACR,reg_val_GCR;+unsignedlongflags;/* Check msp state whether in RUN or CONFIGURED Mode */-if((msp->msp_state==MSP_STATE_IDLE)&&(msp->plat_init)){-status=msp->plat_init();-if(status){-dev_err(msp->dev,"%s: ERROR: Failed to init MSP (%d)!\n",-__func__,status);-returnstatus;+if(msp->msp_state==MSP_STATE_IDLE&&msp->use_pinctrl){+spin_lock_irqsave(&msp_rxtx_lock,flags);+if(pinctrl_rxtx_ref==0&&+!(IS_ERR(pinctrl_p)||IS_ERR(pinctrl_def))){+retval=pinctrl_select_state(pinctrl_p,+pinctrl_def);+if(retval)+pr_err("could not set MSP defstate\n");}+if(!retval)+pinctrl_rxtx_ref++;+spin_unlock_irqrestore(&msp_rxtx_lock,flags);}/* Configure msp with protocol dependent settings */
@@ -620,7 +638,8 @@ int ux500_msp_i2s_trigger(struct ux500_msp *msp, int cmd, int direction)intux500_msp_i2s_close(structux500_msp*msp,unsignedintdir){-intstatus=0;+intstatus=0,retval=0;+unsignedlongflags;dev_dbg(msp->dev,"%s: Enter (dir = 0x%01x).\n",__func__,dir);
@@ -631,12 +650,19 @@ int ux500_msp_i2s_close(struct ux500_msp *msp, unsigned int dir)writel((readl(msp->registers+MSP_GCR)&(~(FRAME_GEN_ENABLE|SRG_ENABLE))),msp->registers+MSP_GCR);-if(msp->plat_exit)-status=msp->plat_exit();-if(status)-dev_warn(msp->dev,-"%s: WARN: ux500_msp_i2s_exit failed (%d)!\n",-__func__,status);++spin_lock_irqsave(&msp_rxtx_lock,flags);+WARN_ON(!pinctrl_rxtx_ref);+pinctrl_rxtx_ref--;+if(msp->use_pinctrl&&pinctrl_rxtx_ref==0&&+!(IS_ERR(pinctrl_p)||IS_ERR(pinctrl_sleep))){+retval=pinctrl_select_state(pinctrl_p,+pinctrl_sleep);+if(retval)+pr_err("could not set MSP sleepstate\n");+}+spin_unlock_irqrestore(&msp_rxtx_lock,flags);+writel(0,msp->registers+MSP_GCR);writel(0,msp->registers+MSP_TCF);writel(0,msp->registers+MSP_RCF);
@@ -667,6 +693,7 @@ int ux500_msp_i2s_init_msp(struct platform_device *pdev,structresource*res=NULL;structi2s_controller*i2s_cont;structux500_msp*msp;+staticintinitialised=false;dev_dbg(&pdev->dev,"%s: Enter (name: %s, id: %d).\n",__func__,pdev->name,platform_data->id);
@@ -678,8 +705,7 @@ int ux500_msp_i2s_init_msp(struct platform_device *pdev,msp->id=platform_data->id;msp->dev=&pdev->dev;-msp->plat_init=platform_data->msp_i2s_init;-msp->plat_exit=platform_data->msp_i2s_exit;+msp->use_pinctrl=platform_data->use_pinctrl;msp->dma_cfg_rx=platform_data->msp_i2s_dma_rx;msp->dma_cfg_tx=platform_data->msp_i2s_dma_tx;
@@ -717,6 +743,29 @@ int ux500_msp_i2s_init_msp(struct platform_device *pdev,dev_dbg(&pdev->dev,"I2S device-name: '%s'\n",i2s_cont->name);msp->i2s_cont=i2s_cont;+/* MSP1 and MSP3 share pins, so we only need to initialise pinctrl once. */+if(msp->use_pinctrl&&!initialised){+pinctrl_p=pinctrl_get(msp->dev);+if(IS_ERR(pinctrl_p))+dev_err(&pdev->dev,"could not get MSP pinctrl\n");+else{+pinctrl_def=pinctrl_lookup_state(pinctrl_p,+PINCTRL_STATE_DEFAULT);+if(IS_ERR(pinctrl_def)){+dev_err(&pdev->dev,+"could not get MSP defstate (%li)\n",+PTR_ERR(pinctrl_def));+}+pinctrl_sleep=pinctrl_lookup_state(pinctrl_p,+PINCTRL_STATE_SLEEP);+if(IS_ERR(pinctrl_sleep))+dev_err(&pdev->dev,+"could not get MSP idlestate (%li)\n",+PTR_ERR(pinctrl_def));+}+initialised=true;+}+return0;err_i2s_cont:
From: Lee Jones <hidden> Date: 2012-07-26 10:29:49
We continue to allow the AB8500 CODEC to be registered via the AB8500
Multi Functional Device API, only this time we extract its configuration
from the Device Tree binary.
Signed-off-by: Lee Jones <redacted>
---
drivers/mfd/ab8500-core.c | 1 +
include/linux/mfd/abx500/ab8500-codec.h | 6 ++-
sound/soc/codecs/ab8500-codec.c | 79 +++++++++++++++++++++++++++++++
3 files changed, 84 insertions(+), 2 deletions(-)
@@ -2394,9 +2395,62 @@ struct snd_soc_dai_driver ab8500_codec_dai[] = {}};+staticvoidab8500_codec_of_probe(structdevice*dev,structdevice_node*np,+structab8500_codec_platform_data*codec)+{+u32value;++if(of_get_property(np,"stericsson,amic1-type-single-ended",NULL))+codec->amics.mic1_type=AMIC_TYPE_SINGLE_ENDED;+else+codec->amics.mic1_type=AMIC_TYPE_DIFFERENTIAL;++if(of_get_property(np,"stericsson,amic2-type-single-ended",NULL))+codec->amics.mic2_type=AMIC_TYPE_SINGLE_ENDED;+else+codec->amics.mic2_type=AMIC_TYPE_DIFFERENTIAL;++/* Has a non-standard Vamic been requested? */+if(of_get_property(np,"stericsson,amic1a-bias-vamic2",NULL))+codec->amics.mic1a_micbias=AMIC_MICBIAS_VAMIC2;+else+codec->amics.mic1a_micbias=AMIC_MICBIAS_VAMIC1;++if(of_get_property(np,"stericsson,amic1b-bias-vamic2",NULL))+codec->amics.mic1b_micbias=AMIC_MICBIAS_VAMIC2;+else+codec->amics.mic1b_micbias=AMIC_MICBIAS_VAMIC1;++if(of_get_property(np,"stericsson,amic2-bias-vamic1",NULL))+codec->amics.mic2_micbias=AMIC_MICBIAS_VAMIC1;+else+codec->amics.mic2_micbias=AMIC_MICBIAS_VAMIC2;++if(!of_property_read_u32(np,"stericsson,earpeice-cmv",&value)){+switch(value){+case950:+codec->ear_cmv=EAR_CMV_0_95V;+break;+case1100:+codec->ear_cmv=EAR_CMV_1_10V;+break;+case1270:+codec->ear_cmv=EAR_CMV_1_27V;+break;+case1580:+codec->ear_cmv=EAR_CMV_1_58V;+break;+default:+codec->ear_cmv=EAR_CMV_UNKNOWN;+dev_err(dev,"Unsuitable earpiece voltage found in DT\n");+}+}+}+staticintab8500_codec_probe(structsnd_soc_codec*codec){structdevice*dev=codec->dev;+structdevice_node*np=dev->of_node;structab8500_codec_drvdata*drvdata=dev_get_drvdata(dev);structab8500_platform_data*pdata;structfilter_control*fc;
@@ -2406,6 +2460,31 @@ static int ab8500_codec_probe(struct snd_soc_codec *codec)/* Setup AB8500 according to board-settings */pdata=(structab8500_platform_data*)dev_get_platdata(dev->parent);++if(np){+if(!pdata)+pdata=devm_kzalloc(dev,+sizeof(structab8500_platform_data),+GFP_KERNEL);++if(!pdata->codec)+pdata->codec+=devm_kzalloc(dev,+sizeof(structab8500_codec_platform_data),+GFP_KERNEL);++if(!(pdata&&pdata->codec))+return-ENOMEM;++ab8500_codec_of_probe(dev,np,pdata->codec);++}else{+if(!(pdata&&pdata->codec)){+dev_err(dev,"No codec platform data or DT found\n");+return-EINVAL;+}+}+status=ab8500_audio_setup_mics(codec,&pdata->codec->amics);if(status<0){pr_err("%s: Failed to setup mics (%d)!\n",__func__,status);
From: Lee Jones <hidden> Date: 2012-07-26 10:30:00
The 'msp' board file does more than just register MSP devices. It
also registers some other components necessary to get audio working
on ux500 based platforms; such as the PCM and Machine Drivers. For
that reason we're changing the filename to be more encompassing -
'audio'.
Signed-off-by: Lee Jones <redacted>
---
arch/arm/mach-ux500/Makefile | 2 +-
arch/arm/mach-ux500/board-mop500-audio.c | 194 ++++++++++++++++++++++++++++++
arch/arm/mach-ux500/board-mop500-msp.c | 194 ------------------------------
arch/arm/mach-ux500/board-mop500.c | 10 +-
arch/arm/mach-ux500/board-mop500.h | 2 +-
5 files changed, 201 insertions(+), 201 deletions(-)
create mode 100644 arch/arm/mach-ux500/board-mop500-audio.c
delete mode 100644 arch/arm/mach-ux500/board-mop500-msp.c
@@ -0,0 +1,194 @@+/*+*Copyright(C)ST-EricssonSA2010+*+*Licenseterms:GNUGeneralPublicLicense(GPL),version2+*/++#include<linux/platform_device.h>+#include<linux/init.h>+#include<linux/gpio.h>+#include<linux/pinctrl/consumer.h>++#include<plat/gpio-nomadik.h>+#include<plat/pincfg.h>+#include<plat/ste_dma40.h>++#include<mach/devices.h>+#include<mach/hardware.h>+#include<mach/irqs.h>+#include<mach/msp.h>++#include"ste-dma40-db8500.h"+#include"board-mop500.h"+#include"devices-db8500.h"+#include"pins-db8500.h"++staticstructstedma40_chan_cfgmsp0_dma_rx={+.high_priority=true,+.dir=STEDMA40_PERIPH_TO_MEM,++.src_dev_type=DB8500_DMA_DEV31_MSP0_RX_SLIM0_CH0_RX,+.dst_dev_type=STEDMA40_DEV_DST_MEMORY,++.src_info.psize=STEDMA40_PSIZE_LOG_4,+.dst_info.psize=STEDMA40_PSIZE_LOG_4,++/* data_width is set during configuration */+};++staticstructstedma40_chan_cfgmsp0_dma_tx={+.high_priority=true,+.dir=STEDMA40_MEM_TO_PERIPH,++.src_dev_type=STEDMA40_DEV_DST_MEMORY,+.dst_dev_type=DB8500_DMA_DEV31_MSP0_TX_SLIM0_CH0_TX,++.src_info.psize=STEDMA40_PSIZE_LOG_4,+.dst_info.psize=STEDMA40_PSIZE_LOG_4,++/* data_width is set during configuration */+};++structmsp_i2s_platform_datamsp0_platform_data={+.id=MSP_I2S_0,+.msp_i2s_dma_rx=&msp0_dma_rx,+.msp_i2s_dma_tx=&msp0_dma_tx,+};++staticstructstedma40_chan_cfgmsp1_dma_rx={+.high_priority=true,+.dir=STEDMA40_PERIPH_TO_MEM,++.src_dev_type=DB8500_DMA_DEV30_MSP3_RX,+.dst_dev_type=STEDMA40_DEV_DST_MEMORY,++.src_info.psize=STEDMA40_PSIZE_LOG_4,+.dst_info.psize=STEDMA40_PSIZE_LOG_4,++/* data_width is set during configuration */+};++staticstructstedma40_chan_cfgmsp1_dma_tx={+.high_priority=true,+.dir=STEDMA40_MEM_TO_PERIPH,++.src_dev_type=STEDMA40_DEV_DST_MEMORY,+.dst_dev_type=DB8500_DMA_DEV30_MSP1_TX,++.src_info.psize=STEDMA40_PSIZE_LOG_4,+.dst_info.psize=STEDMA40_PSIZE_LOG_4,++/* data_width is set during configuration */+};++structmsp_i2s_platform_datamsp1_platform_data={+.id=MSP_I2S_1,+.msp_i2s_dma_rx=NULL,+.msp_i2s_dma_tx=&msp1_dma_tx,+.use_pinctrl=true,+};++staticstructstedma40_chan_cfgmsp2_dma_rx={+.high_priority=true,+.dir=STEDMA40_PERIPH_TO_MEM,++.src_dev_type=DB8500_DMA_DEV14_MSP2_RX,+.dst_dev_type=STEDMA40_DEV_DST_MEMORY,++/* MSP2 DMA doesn't work with PSIZE == 4 on DB8500v2 */+.src_info.psize=STEDMA40_PSIZE_LOG_1,+.dst_info.psize=STEDMA40_PSIZE_LOG_1,++/* data_width is set during configuration */+};++staticstructstedma40_chan_cfgmsp2_dma_tx={+.high_priority=true,+.dir=STEDMA40_MEM_TO_PERIPH,++.src_dev_type=STEDMA40_DEV_DST_MEMORY,+.dst_dev_type=DB8500_DMA_DEV14_MSP2_TX,++.src_info.psize=STEDMA40_PSIZE_LOG_4,+.dst_info.psize=STEDMA40_PSIZE_LOG_4,++.use_fixed_channel=true,+.phy_channel=1,++/* data_width is set during configuration */+};++staticstructplatform_device*db8500_add_msp_i2s(structdevice*parent,+intid,+resource_size_tbase,intirq,+structmsp_i2s_platform_data*pdata)+{+structplatform_device*pdev;+structresourceres[]={+DEFINE_RES_MEM(base,SZ_4K),+DEFINE_RES_IRQ(irq),+};++pr_info("Register platform-device 'ux500-msp-i2s', id %d, irq %d\n",+id,irq);+pdev=platform_device_register_resndata(parent,"ux500-msp-i2s",id,+res,ARRAY_SIZE(res),+pdata,sizeof(*pdata));+if(!pdev){+pr_err("Failed to register platform-device 'ux500-msp-i2s.%d'!\n",+id);+returnNULL;+}++returnpdev;+}++/* Platform device for ASoC U8500 machine */+staticstructplatform_devicesnd_soc_mop500={+.name="snd-soc-mop500",+.id=0,+.dev={+.platform_data=NULL,+},+};++/* Platform device for Ux500-PCM */+staticstructplatform_deviceux500_pcm={+.name="ux500-pcm",+.id=0,+.dev={+.platform_data=NULL,+},+};++structmsp_i2s_platform_datamsp2_platform_data={+.id=MSP_I2S_2,+.msp_i2s_dma_rx=&msp2_dma_rx,+.msp_i2s_dma_tx=&msp2_dma_tx,+};++structmsp_i2s_platform_datamsp3_platform_data={+.id=MSP_I2S_3,+.msp_i2s_dma_rx=&msp1_dma_rx,+.msp_i2s_dma_tx=NULL,+.use_pinctrl=true,+};++voidmop500_audio_init(structdevice*parent)+{+pr_info("%s: Register platform-device 'snd-soc-u8500'.\n",__func__);+platform_device_register(&snd_soc_mop500);++pr_info("Initialize MSP I2S-devices.\n");+db8500_add_msp_i2s(parent,0,U8500_MSP0_BASE,IRQ_DB8500_MSP0,+&msp0_platform_data);+db8500_add_msp_i2s(parent,1,U8500_MSP1_BASE,IRQ_DB8500_MSP1,+&msp1_platform_data);+db8500_add_msp_i2s(parent,2,U8500_MSP2_BASE,IRQ_DB8500_MSP2,+&msp2_platform_data);+db8500_add_msp_i2s(parent,3,U8500_MSP3_BASE,IRQ_DB8500_MSP1,+&msp3_platform_data);++pr_info("%s: Register platform-device 'ux500-pcm'\n",__func__);+platform_device_register(&ux500_pcm);+}
@@ -641,7 +641,7 @@ static void __init snowball_init_machine(void)mop500_i2c_init(parent);snowball_sdi_init(parent);mop500_spi_init(parent);-mop500_msp_init(parent);+mop500_audio_init(parent);mop500_uart_init(parent);/* This board has full regulator constraints */
From: Lee Jones <hidden> Date: 2012-07-26 10:31:19
Pass registration of both parts of the MSP driver from platform code
to Device Tree so that they are probed when Device Tree is enabled.
Also, as there is platform data involved, we ensure that there is
allocated memory to place the configuration into and that the correct
information is extracted from the DT binary.
Signed-off-by: Lee Jones <redacted>
---
arch/arm/mach-ux500/board-mop500-msp.c | 14 --------------
arch/arm/mach-ux500/board-mop500.c | 2 --
arch/arm/mach-ux500/board-mop500.h | 2 --
sound/soc/ux500/ux500_msp_dai.c | 6 ++++++
sound/soc/ux500/ux500_msp_i2s.c | 33 +++++++++++++++++++++++++++++---
5 files changed, 36 insertions(+), 21 deletions(-)
@@ -174,20 +174,6 @@ struct msp_i2s_platform_data msp3_platform_data = {.use_pinctrl=true,};-/* Due for removal once the MSP driver has been fully DT:ed. */-voidmop500_of_msp_init(structdevice*parent)-{-pr_info("Initialize MSP I2S-devices.\n");-db8500_add_msp_i2s(parent,0,U8500_MSP0_BASE,IRQ_DB8500_MSP0,-&msp0_platform_data);-db8500_add_msp_i2s(parent,1,U8500_MSP1_BASE,IRQ_DB8500_MSP1,-&msp1_platform_data);-db8500_add_msp_i2s(parent,2,U8500_MSP2_BASE,IRQ_DB8500_MSP2,-&msp2_platform_data);-db8500_add_msp_i2s(parent,3,U8500_MSP3_BASE,IRQ_DB8500_MSP1,-&msp3_platform_data);-}-voidmop500_msp_init(structdevice*parent){pr_info("%s: Register platform-device 'snd-soc-u8500'.\n",__func__);
@@ -98,8 +98,6 @@ void __init mop500_pinmaps_init(void);void__initsnowball_pinmaps_init(void);void__inithrefv60_pinmaps_init(void);voidmop500_msp_init(structdevice*parent);-/* Due for removal once the MSP driver has been fully DT:ed. */-voidmop500_of_msp_init(structdevice*parent);int__initmop500_uib_init(void);voidmop500_uib_i2c_add(intbusnum,structi2c_board_info*info,
From: Lee Jones <hidden> Date: 2012-07-26 10:32:06
In this patch we stop registering the MOP500 driver from platform
code and rely solely on Device Tree to do the probing for us. We
also parse the sound node to link together the codec, dma and the
CPU-side Digital Audio Interface.
Signed-off-by: Lee Jones <redacted>
---
arch/arm/mach-ux500/board-mop500-msp.c | 3 ---
sound/soc/ux500/mop500.c | 41 ++++++++++++++++++++++++++++++++
2 files changed, 41 insertions(+), 3 deletions(-)
From: Lee Jones <hidden> Date: 2012-07-26 10:32:29
We've done this before and it worked well last time. Here we're
duplicating a complex registration function to ease the process
of enabling it for Device Tree. As there are quite a few steps
taken during the registration process, it makes sense to break
them up into more manageable chunks. This patch will aid us.
Signed-off-by: Lee Jones <redacted>
---
arch/arm/mach-ux500/board-mop500-msp.c | 42 ++++++++++++++++++++++++++++++++
1 file changed, 42 insertions(+)
@@ -223,6 +223,48 @@ static struct msp_i2s_platform_data msp3_platform_data = {.msp_i2s_exit=msp13_i2s_exit,};+/* Due for removal once the MSP driver has been fully DT:ed. */+voidmop500_of_msp_init(structdevice*parent)+{+structplatform_device*msp1;++pr_info("%s: Register platform-device 'snd-soc-u8500'.\n",__func__);+platform_device_register(&snd_soc_mop500);++pr_info("Initialize MSP I2S-devices.\n");+db8500_add_msp_i2s(parent,0,U8500_MSP0_BASE,IRQ_DB8500_MSP0,+&msp0_platform_data);+msp1=db8500_add_msp_i2s(parent,1,U8500_MSP1_BASE,IRQ_DB8500_MSP1,+&msp1_platform_data);+db8500_add_msp_i2s(parent,2,U8500_MSP2_BASE,IRQ_DB8500_MSP2,+&msp2_platform_data);+db8500_add_msp_i2s(parent,3,U8500_MSP3_BASE,IRQ_DB8500_MSP1,+&msp3_platform_data);++/* Get the pinctrl handle for MSP1 */+if(msp1){+msp1_p=pinctrl_get(&msp1->dev);+if(IS_ERR(msp1_p))+dev_err(&msp1->dev,"could not get MSP1 pinctrl\n");+else{+msp1_def=pinctrl_lookup_state(msp1_p,+PINCTRL_STATE_DEFAULT);+if(IS_ERR(msp1_def)){+dev_err(&msp1->dev,+"could not get MSP1 defstate\n");+}+msp1_sleep=pinctrl_lookup_state(msp1_p,+PINCTRL_STATE_SLEEP);+if(IS_ERR(msp1_sleep))+dev_err(&msp1->dev,+"could not get MSP1 idlestate\n");+}+}++pr_info("%s: Register platform-device 'ux500-pcm'\n",__func__);+platform_device_register(&ux500_pcm);+}+voidmop500_msp_init(structdevice*parent){structplatform_device*msp1;
From: Lee Jones <hidden> Date: 2012-07-26 10:32:45
The current kernel commandline for ux500 based devices includes
hard-coded allocations for things like mali and hwmem, which
actually run over lowmem. Here we enable highmem in order to
avoid memory corruption errors.
Signed-off-by: Lee Jones <redacted>
---
arch/arm/mach-ux500/Kconfig | 1 +
1 file changed, 1 insertion(+)
From: Lee Jones <hidden> Date: 2012-07-26 10:33:14
Nothing special here. We're only providing a compatible string
to ensure the driver is probed using a Device Tree boot.
Signed-off-by: Lee Jones <redacted>
---
arch/arm/boot/dts/db8500.dtsi | 4 ++++
1 file changed, 4 insertions(+)
From: Lee Jones <hidden> Date: 2012-07-26 10:34:27
If a sound codec fails to request a regmap, the 'using_regmap' is
set as true regardless, despite there being no regmap to use. As a
repercussion, when a latter read function checks to see if we are
using regmaps, it assumes we are and attempts to. Only the kernel
oopes, because regmap_* tries to extract information from a NULL
pointer.
Signed-off-by: Lee Jones <redacted>
---
sound/soc/soc-io.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
@@ -52,10 +52,13 @@ static unsigned int hw_read(struct snd_soc_codec *codec, unsigned int reg)if(codec->cache_only)return-1;-ret=regmap_read(codec->control_data,reg,&val);-if(ret==0)-returnval;-else+if(codec->using_regmap){+ret=regmap_read(codec->control_data,reg,&val);+if(ret==0)+returnval;+else+return-1;+}elsereturn-1;}
@@ -141,11 +144,12 @@ int snd_soc_codec_set_cache_io(struct snd_soc_codec *codec,caseSND_SOC_REGMAP:/* Device has made its own regmap arrangements */-codec->using_regmap=true;if(!codec->control_data)codec->control_data=dev_get_regmap(codec->dev,NULL);if(codec->control_data){+codec->using_regmap=true;+ret=regmap_get_val_bytes(codec->control_data);/* Errors are legitimate for non-integer byte*multiples*/
From: Lee Jones <hidden> Date: 2012-07-26 10:34:45
Thought to be another merge error, board-mop500-msp.h has never existed
in the upstream kernel, only msp.h. This patch changes the include files
to match the existing file name.
Signed-off-by: Lee Jones <redacted>
---
sound/soc/ux500/ux500_msp_dai.c | 2 +-
sound/soc/ux500/ux500_msp_i2s.c | 2 +-
sound/soc/ux500/ux500_msp_i2s.h | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
From: Lee Jones <hidden> Date: 2012-07-26 10:35:08
Currently there is no out-of-memory error checking after attempting
to allocate memory for the ux500_msp or ux500_msp_i2s_drvdata data
structures. Instead we go about populating them regardless. This
patch applies the necessary error checking to prevent a panic.
Signed-off-by: Lee Jones <redacted>
---
sound/soc/ux500/ux500_msp_dai.c | 3 +++
sound/soc/ux500/ux500_msp_i2s.c | 2 ++
2 files changed, 5 insertions(+)
From: Lee Jones <hidden> Date: 2012-07-26 10:35:51
This was left over during a recent clean-up which removed Device Tree
helper structs. There is no longer a requirement for it, so we can just
remove it.
Signed-off-by: Lee Jones <redacted>
---
arch/arm/mach-ux500/board-mop500.c | 5 -----
1 file changed, 5 deletions(-)
From: Mark Brown <hidden> Date: 2012-07-26 11:28:52
On Thu, Jul 26, 2012 at 11:28:33AM +0100, Lee Jones wrote:
This patch-set sees some code reinforcement surrounding the recently
accepted MOP500 MSP, PCM, AB8500 CODEC and ux500 Audio Machine Driver.
It contains some extra error checking pertaining to the ux500 specific
drivers themselves, along with bugfixes for issues in core SoC Audio
code happened upon along the way. There are also a couple of minor
clean-ups relating to ux500 platform code, which are hitching a ride
for ease of status tracking.
Please restructure this so that the actual error fixes (which should go
into 3.6) are before the random cleanups (which should go into 3.7)
rather than mixing them together. It'd also be helpful if you could
split out changes to the different subsystems separately when there's no
overlap (which mostly looks to be the case here).
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120726/39709216/attachment.sig>
From: Mark Brown <hidden> Date: 2012-07-26 11:32:09
On Thu, Jul 26, 2012 at 11:28:40AM +0100, Lee Jones wrote:
quoted hunk
@@ -52,10 +52,13 @@ static unsigned int hw_read(struct snd_soc_codec *codec, unsigned int reg) if (codec->cache_only) return -1;- ret = regmap_read(codec->control_data, reg, &val);- if (ret == 0)- return val;- else+ if (codec->using_regmap) {+ ret = regmap_read(codec->control_data, reg, &val);+ if (ret == 0)+ return val;+ else+ return -1;+ } else
No, this makes no sense. There is no non-regmap I/O support in soc-io,
anything using the soc-io hw_read() function must be using regmap.
quoted hunk
case SND_SOC_REGMAP: /* Device has made its own regmap arrangements */- codec->using_regmap = true;
Again, this makes no sense. If we're explicitly being asked to use
regmap then we should be using regmap or just failing to set up I/O
(which is obviously a catastrophic failure).
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120726/3197f7ea/attachment.sig>
From: Lee Jones <hidden> Date: 2012-07-26 11:36:46
On 26/07/12 12:28, Mark Brown wrote:
On Thu, Jul 26, 2012 at 11:28:33AM +0100, Lee Jones wrote:
quoted
This patch-set sees some code reinforcement surrounding the recently
accepted MOP500 MSP, PCM, AB8500 CODEC and ux500 Audio Machine Driver.
It contains some extra error checking pertaining to the ux500 specific
drivers themselves, along with bugfixes for issues in core SoC Audio
code happened upon along the way. There are also a couple of minor
clean-ups relating to ux500 platform code, which are hitching a ride
for ease of status tracking.
Please restructure this so that the actual error fixes (which should go
into 3.6) are before the random cleanups (which should go into 3.7)
rather than mixing them together.
I can.
> It'd also be helpful if you could
split out changes to the different subsystems separately when there's no
overlap (which mostly looks to be the case here).
I can to this too, but you will still have overlap between arch/arm and
sound/soc. The only other subsystem in the patch-set is MFD.
--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
This should be done separately - if it's going to be merged with
something it should be the patch that adds the relevant DT fragments.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120726/080dd61a/attachment.sig>
From: Lee Jones <hidden> Date: 2012-07-26 11:38:22
On 26/07/12 12:32, Mark Brown wrote:
On Thu, Jul 26, 2012 at 11:28:40AM +0100, Lee Jones wrote:
quoted
@@ -52,10 +52,13 @@ static unsigned int hw_read(struct snd_soc_codec *codec, unsigned int reg) if (codec->cache_only) return -1;- ret = regmap_read(codec->control_data, reg, &val);- if (ret == 0)- return val;- else+ if (codec->using_regmap) {+ ret = regmap_read(codec->control_data, reg, &val);+ if (ret == 0)+ return val;+ else+ return -1;+ } else
No, this makes no sense. There is no non-regmap I/O support in soc-io,
anything using the soc-io hw_read() function must be using regmap.
quoted
case SND_SOC_REGMAP: /* Device has made its own regmap arrangements */- codec->using_regmap = true;
Again, this makes no sense. If we're explicitly being asked to use
regmap then we should be using regmap or just failing to set up I/O
(which is obviously a catastrophic failure).
How much work is there involved in regmap:ing a device, so that
dev_get_regmap() doesn't fail?
--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
From: Mark Brown <hidden> Date: 2012-07-26 11:42:19
On Thu, Jul 26, 2012 at 12:38:17PM +0100, Lee Jones wrote:
On 26/07/12 12:32, Mark Brown wrote:
quoted
Again, this makes no sense. If we're explicitly being asked to use
regmap then we should be using regmap or just failing to set up I/O
(which is obviously a catastrophic failure).
How much work is there involved in regmap:ing a device, so that
dev_get_regmap() doesn't fail?
Trivial if it's on a supported bus, otherwise you just need to write the
bus. But why do you care if dev_get_regmap() fails? We only try to use
regmap if the driver asked for regmap I/O (or doesn't have registers at
all in which case it doesn't matter since we never do any I/O). What
you appear to be saying here is that you're using regmap on a device
which doesn't have a regmap set up which is clearly never going to work
terribly well...
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120726/6f924f01/attachment.sig>
Why are we doing this? The MFD cells are a totally Linux specific
thing, there's no reason to represent them in the device tree unless
they're in some way reusable and the "ab8500-codec" name suggests that's
unlikely. Just put the properties on the parent node and instantiate
the MFD cell as normal.
+ /* Has a non-standard Vamic been requested? */
+ if(of_get_property(np, "stericsson,amic1a-bias-vamic2", NULL))
Coding style.
quoted hunk
+ if (!of_property_read_u32(np, "stericsson,earpeice-cmv", &value)) {+ switch (value) {+ case 950 :+ codec->ear_cmv = EAR_CMV_0_95V;+ break;+ case 1100 :+ codec->ear_cmv = EAR_CMV_1_10V;+ break;+ case 1270 :+ codec->ear_cmv = EAR_CMV_1_27V;+ break;+ case 1580 :+ codec->ear_cmv = EAR_CMV_1_58V;+ break;+ default :+ codec->ear_cmv = EAR_CMV_UNKNOWN;+ dev_err(dev, "Unsuitable earpiece voltage found in DT\n");
From: Mark Brown <hidden> Date: 2012-07-26 11:54:54
On Thu, Jul 26, 2012 at 11:28:39AM +0100, Lee Jones wrote:
If a list of widgets is provided and one of them fails to be added as
a control, the present semantics fail all subsequent widgets. A better
solution would be to only fail that widget, but pursue in attempting
to add the rest of the list.
From: Mark Brown <hidden> Date: 2012-07-26 11:57:53
On Thu, Jul 26, 2012 at 11:28:37AM +0100, Lee Jones wrote:
Currently there is no out-of-memory error checking after attempting
to allocate memory for the ux500_msp or ux500_msp_i2s_drvdata data
structures. Instead we go about populating them regardless. This
patch applies the necessary error checking to prevent a panic.
From: Mark Brown <hidden> Date: 2012-07-26 11:59:02
On Thu, Jul 26, 2012 at 11:28:38AM +0100, Lee Jones wrote:
Thought to be another merge error, board-mop500-msp.h has never existed
in the upstream kernel, only msp.h. This patch changes the include files
to match the existing file name.
This should be done separately - if it's going to be merged with
something it should be the patch that adds the relevant DT fragments.
Yes, I can do that.
--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
This has the same issue as your last patch... the way you're doing
things will break audio on all boards using this driver.
It will, why?
--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
Why are we doing this? The MFD cells are a totally Linux specific
thing, there's no reason to represent them in the device tree unless
they're in some way reusable and the "ab8500-codec" name suggests that's
unlikely. Just put the properties on the parent node and instantiate
the MFD cell as normal.
quoted
+ /* Has a non-standard Vamic been requested? */
+ if(of_get_property(np, "stericsson,amic1a-bias-vamic2", NULL))
Coding style.
Missing space? Sorry, typo, I'll change.
quoted
+ if (!of_property_read_u32(np, "stericsson,earpeice-cmv", &value)) {+ switch (value) {+ case 950 :+ codec->ear_cmv = EAR_CMV_0_95V;+ break;+ case 1100 :+ codec->ear_cmv = EAR_CMV_1_10V;+ break;+ case 1270 :+ codec->ear_cmv = EAR_CMV_1_27V;+ break;+ case 1580 :+ codec->ear_cmv = EAR_CMV_1_58V;+ break;+ default :+ codec->ear_cmv = EAR_CMV_UNKNOWN;+ dev_err(dev, "Unsuitable earpiece voltage found in DT\n");
The platform data code picks a default, can't the DT code do the same?
No, I don't think that it does? The original code returns -EINVAL unless
a value is specified.
The original author is keen to have a clear error message in case users
try to specify non-exact values. I'd rather we fail-out than use
incorrect values which would be a great deal harder for a user to debug.
--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
Why are we doing this? The MFD cells are a totally Linux specific
thing, there's no reason to represent them in the device tree unless
they're in some way reusable and the "ab8500-codec" name suggests that's
unlikely. Just put the properties on the parent node and instantiate
the MFD cell as normal.
We have all of the AB8500 devices into the Device Tree to accurately
represent the hardware. We will also be passing configuration
information into the AB8500 Codec from Device Tree. The only reason
we're still registering them using the MFD API is to overcome addressing
issues encountered earlier. Each 'device' still belongs in the 'device'
tree.
If we were to take this Device Tree and use it on something non-Linux,
that OS will still need to know about each of the AB8500 devices and
every associated configuration option. Only in Linux do we continue to
register them though a different API, which doesn't affect any other OS.
--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
This has the same issue as your last patch... the way you're doing
things will break audio on all boards using this driver.
It will, why?
You've just removed registration of the device and not added anything
else to replace that. Even if all boards convert to DT their DTs will
need to be updated which you're not doing.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120726/c696d23d/attachment-0001.sig>
From: Mark Brown <hidden> Date: 2012-07-26 14:28:29
On Thu, Jul 26, 2012 at 03:00:17PM +0100, Lee Jones wrote:
On 26/07/12 12:50, Mark Brown wrote:
quoted
Yet again no binding documentation....
RFC. ;)
I'll write the documentation when/if the properties are accepted.
No, write the documentation. It's way too much effort to reverse
engineer the bindings from the code.
quoted
quoted
+ default :+ codec->ear_cmv = EAR_CMV_UNKNOWN;+ dev_err(dev, "Unsuitable earpiece voltage found in DT\n");
quoted
The platform data code picks a default, can't the DT code do the same?
No, I don't think that it does? The original code returns -EINVAL
unless a value is specified.
The code doesn't specify values for the enumeration so it ought to
default to EAR_CMV_0_95V if nothing is specified.
The original author is keen to have a clear error message in case
users try to specify non-exact values. I'd rather we fail-out than
use incorrect values which would be a great deal harder for a user
to debug.
From: Mark Brown <hidden> Date: 2012-07-26 14:43:40
On Thu, Jul 26, 2012 at 03:15:01PM +0100, Lee Jones wrote:
Sorry missed this:
quoted
Why are we doing this? The MFD cells are a totally Linux specific
thing, there's no reason to represent them in the device tree unless
they're in some way reusable and the "ab8500-codec" name suggests that's
unlikely. Just put the properties on the parent node and instantiate
the MFD cell as normal.
We have all of the AB8500 devices into the Device Tree to accurately
represent the hardware. We will also be passing configuration
information into the AB8500 Codec from Device Tree. The only reason
we're still registering them using the MFD API is to overcome
addressing issues encountered earlier. Each 'device' still belongs
in the 'device' tree.
The device here is the AB8500. The fact that Linux chooses to represent
it as an MFD with a particular set of subdrivers is a Linux specific
decision and may well change over time. For example it's likely that
we'll want to migrate the clocks out of the audio driver and into the
clock API when that becomes useful. Similarly currently a lot of these
devices use ASoC level jack detection but that's going to move over to
extcon over time.
There's no way you're going to be able to reuse this for anything that
isn't an AB8500, there's no abstraction of the SoC integration here. If
you had clearly identifiable, repeatable IPs which you could reasonably
bind to a different bit of silicon then that'd be different but there's
nothing like that here. We already know that the functionality covered
by the driver is going to be present simply by virtue of knowing that
there's an AB8500 and similarly there's no real way this driver could
ever be used without the core driver. All the "device" in the device
tree is doing is serving as a container to place some of the DT
properties into, this needs to be separated out from the instantiation
of the Linux device driver. There's nothing stopping the driver from
looking at the OF node of its parent here.
The goal here isn't just to copy the Linux device model and platform
data into device tree bindings, the device tree bindings need to think
about what the chip actually is so they can be reused by other OSs and
by future versions of Linux.
If we were to take this Device Tree and use it on something
non-Linux, that OS will still need to know about each of the AB8500
devices and every associated configuration option. Only in Linux do
we continue to register them though a different API, which doesn't
affect any other OS.
Another OS might have a different idea about how it's going to split up
the chip which better fits with the models which that OS has for the
functions present on the device. The reason this is a distinct device
in Linux is all to do with how Linux models the hardware.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120726/92995ced/attachment.sig>
From: Lee Jones <hidden> Date: 2012-07-26 14:51:22
On 26/07/12 12:42, Mark Brown wrote:
On Thu, Jul 26, 2012 at 12:38:17PM +0100, Lee Jones wrote:
quoted
On 26/07/12 12:32, Mark Brown wrote:
quoted
quoted
Again, this makes no sense. If we're explicitly being asked to use
regmap then we should be using regmap or just failing to set up I/O
(which is obviously a catastrophic failure).
quoted
How much work is there involved in regmap:ing a device, so that
dev_get_regmap() doesn't fail?
Trivial if it's on a supported bus, otherwise you just need to write the
bus. But why do you care if dev_get_regmap() fails? We only try to use
regmap if the driver asked for regmap I/O (or doesn't have registers at
all in which case it doesn't matter since we never do any I/O). What
you appear to be saying here is that you're using regmap on a device
which doesn't have a regmap set up which is clearly never going to work
terribly well...
I don't think we want to use regmap at all, but we're forced to by
soc-core. How do we over-ride that behavior? By writing some nonsense
into codec->control_data?
--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
From: Lee Jones <hidden> Date: 2012-07-26 14:51:55
On 26/07/12 14:53, Mark Brown wrote:
On Thu, Jul 26, 2012 at 02:51:09PM +0100, Lee Jones wrote:
quoted
On 26/07/12 12:37, Mark Brown wrote:> On Thu, Jul 26, 2012 at 11:28:49AM +0100, Lee Jones wrote:
quoted
quoted
There is no binding documentation here. All bindings should be
documented.
quoted
Yes I know. This is more of an RFC _before_ I waste my time writing
documentation (again).
I can't be the only person who reviews bindings by reading the
documentation for the binding...
I plan to do it, promise. ;)
--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
This has the same issue as your last patch... the way you're doing
things will break audio on all boards using this driver.
quoted
It will, why?
You've just removed registration of the device and not added anything
else to replace that. Even if all boards convert to DT their DTs will
need to be updated which you're not doing.
The initialisation function which calls platform_device_register() is
only executed during a DT boot. The clue is in the title
mop500_of_msp_init(). The DT is populated _before_ this patch, but I
guess you mean if they are separated into subsystem trees and are placed
into -next/Mainline out of order.
I will merge these patches with the DT population instead to overcome
this possibility. It makes more sense to keep the arch/arm stuff
together in any case.
--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
From: Lee Jones <hidden> Date: 2012-07-26 15:01:22
On 26/07/12 15:28, Mark Brown wrote:
On Thu, Jul 26, 2012 at 03:00:17PM +0100, Lee Jones wrote:
quoted
On 26/07/12 12:50, Mark Brown wrote:
quoted
quoted
Yet again no binding documentation....
quoted
RFC. ;)
quoted
I'll write the documentation when/if the properties are accepted.
No, write the documentation. It's way too much effort to reverse
engineer the bindings from the code.
quoted
quoted
quoted
+ default :+ codec->ear_cmv = EAR_CMV_UNKNOWN;+ dev_err(dev, "Unsuitable earpiece voltage found in DT\n");
quoted
quoted
The platform data code picks a default, can't the DT code do the same?
quoted
No, I don't think that it does? The original code returns -EINVAL
unless a value is specified.
The code doesn't specify values for the enumeration so it ought to
default to EAR_CMV_0_95V if nothing is specified.
Ah, I see what you mean. I guess we could compromise and print a warning
_and_ fall back to the 0th original emum.
quoted
The original author is keen to have a clear error message in case
users try to specify non-exact values. I'd rather we fail-out than
use incorrect values which would be a great deal harder for a user
to debug.
By that argument all the properties should be mandatory but it's only
this one IIRC.
This is the only value which the user can pick an obscure value, such as
913, thinking they can pick 913mV. I'm happy to fall-back, as long as
Ola is too.
--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
From: Mark Brown <hidden> Date: 2012-07-26 15:12:25
On Thu, Jul 26, 2012 at 03:51:13PM +0100, Lee Jones wrote:
I don't think we want to use regmap at all, but we're forced to by
soc-core. How do we over-ride that behavior? By writing some
nonsense into codec->control_data?
You should use that for your control data, yes - you're not forced to
use regmap at all. Like I say we've got a bunch of drivers doing so
already.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120726/b69c5c54/attachment.sig>
From: Mark Brown <hidden> Date: 2012-07-26 15:14:09
On Thu, Jul 26, 2012 at 04:01:14PM +0100, Lee Jones wrote:
This is the only value which the user can pick an obscure value,
such as 913, thinking they can pick 913mV. I'm happy to fall-back,
as long as Ola is too.
Erroring out if they pick an invalid value is fine, I'm more concerned
with the case where no property is supplied at all.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120726/0c9093c9/attachment.sig>
From: Lee Jones <hidden> Date: 2012-07-26 15:17:15
On 26/07/12 16:14, Mark Brown wrote:
On Thu, Jul 26, 2012 at 04:01:14PM +0100, Lee Jones wrote:
quoted
This is the only value which the user can pick an obscure value,
such as 913, thinking they can pick 913mV. I'm happy to fall-back,
as long as Ola is too.
Erroring out if they pick an invalid value is fine, I'm more concerned
with the case where no property is supplied at all.
Hmmm... I'll have a think.
--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
From: Lee Jones <hidden> Date: 2012-07-26 15:19:39
On 26/07/12 15:43, Mark Brown wrote:
On Thu, Jul 26, 2012 at 03:15:01PM +0100, Lee Jones wrote:
quoted
Sorry missed this:
quoted
quoted
Why are we doing this? The MFD cells are a totally Linux specific
thing, there's no reason to represent them in the device tree unless
they're in some way reusable and the "ab8500-codec" name suggests that's
unlikely. Just put the properties on the parent node and instantiate
the MFD cell as normal.
quoted
We have all of the AB8500 devices into the Device Tree to accurately
represent the hardware. We will also be passing configuration
information into the AB8500 Codec from Device Tree. The only reason
we're still registering them using the MFD API is to overcome
addressing issues encountered earlier. Each 'device' still belongs
in the 'device' tree.
The device here is the AB8500. The fact that Linux chooses to represent
it as an MFD with a particular set of subdrivers is a Linux specific
decision and may well change over time. For example it's likely that
we'll want to migrate the clocks out of the audio driver and into the
clock API when that becomes useful. Similarly currently a lot of these
devices use ASoC level jack detection but that's going to move over to
extcon over time.
There's no way you're going to be able to reuse this for anything that
isn't an AB8500, there's no abstraction of the SoC integration here. If
you had clearly identifiable, repeatable IPs which you could reasonably
bind to a different bit of silicon then that'd be different but there's
nothing like that here. We already know that the functionality covered
by the driver is going to be present simply by virtue of knowing that
there's an AB8500 and similarly there's no real way this driver could
ever be used without the core driver. All the "device" in the device
tree is doing is serving as a container to place some of the DT
properties into, this needs to be separated out from the instantiation
of the Linux device driver. There's nothing stopping the driver from
looking at the OF node of its parent here.
The goal here isn't just to copy the Linux device model and platform
data into device tree bindings, the device tree bindings need to think
about what the chip actually is so they can be reused by other OSs and
by future versions of Linux.
quoted
If we were to take this Device Tree and use it on something
non-Linux, that OS will still need to know about each of the AB8500
devices and every associated configuration option. Only in Linux do
we continue to register them though a different API, which doesn't
affect any other OS.
Another OS might have a different idea about how it's going to split up
the chip which better fits with the models which that OS has for the
functions present on the device. The reason this is a distinct device
in Linux is all to do with how Linux models the hardware.
Okay, so your suggestion is to strip out all of the sub-devices under
the AB8500. It's doable, but will take some restructuring and thinking
about. This is a job for another day. I think it's okay to continue with
the current semantics for the time-being. The line we're discussing does
need to be split out though. I didn't mean to merge it in with the ASoC
stuff.
--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
From: Lee Jones <hidden> Date: 2012-07-26 15:23:38
On 26/07/12 16:12, Mark Brown wrote:
On Thu, Jul 26, 2012 at 03:51:13PM +0100, Lee Jones wrote:
quoted
I don't think we want to use regmap at all, but we're forced to by
soc-core. How do we over-ride that behavior? By writing some
nonsense into codec->control_data?
You should use that for your control data, yes - you're not forced to
use regmap at all. Like I say we've got a bunch of drivers doing so
already.
What's my 'control data'? It's not used in the original codec patch.
The old way wants to go:
snd_soc_update_bits() -> snd_soc_read() -> ab8500_codec_read_reg()
When then calls back into the abx500.
So what 'control data' should I be storing in the codec struct?
--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
From: Mark Brown <hidden> Date: 2012-07-26 15:24:46
On Thu, Jul 26, 2012 at 04:19:33PM +0100, Lee Jones wrote:
Okay, so your suggestion is to strip out all of the sub-devices
under the AB8500. It's doable, but will take some restructuring and
thinking about. This is a job for another day. I think it's okay to
continue with the current semantics for the time-being. The line
we're discussing does need to be split out though. I didn't mean to
merge it in with the ASoC stuff.
Yes, well any that aren't reusable at any rate. If you could relocate
the device into another SoC integation that'd be different.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120726/f328f81b/attachment.sig>
So what 'control data' should I be storing in the codec struct?
You're supposed to use it for the data you use to call back into the
underlying I/O code.
I don't understand. What 'data'?
Surely if .read and .write are populated in 'struct
snd_soc_codec_driver', then it should just call back into those?
--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog