Re: [PATCH v3] regulator: pfuze100: add pfuze100 regulator driver
On Fri, Jul 19, 2013 at 06:14:51PM +0800, Robin Gong wrote: Add pfuze100 regulator driver. Looks good, there's some minor issues below but they should be small. --- a/drivers/regulator/Kconfig +++ b/drivers/regulator/Kconfig @@ -158,6 +158,13 @@ config REGULATOR_MC13892 Say y here to support the regulators found on the Freescale MC13892 PMIC. +config REGULATOR_PFUZE100 + tristate Support regulators on Freescale PFUZE100 PMIC + depends on I2C + help + Say y here to support the regulators found on the Freescale PFUZE100 + PMIC. + Please keep this and the Makefile sorted - if there's issues merging then please use my topic/kconfig as a base. + if (val 0x40) + ramp_delay = 5 / (4 * ramp_delay); + else + ramp_delay = 25000 / (2 * ramp_delay); + if (id = PFUZE100_SW1C) + ramp_delay = 25000 / (2 * ramp_delay); This is a bit confusing as there's a cumalaitve modification applied to ramp_delay in the case that id is less than SW1C but it's not obvious that this is intentional given the calculations. Please change this to make it clearer what the intended logic is (assuming it's not buggy) - something like if ... else if ... else for example. + ret = regmap_read(pfuze_chip-regmap, PFUZE100_REVID, value); + if (ret) + return ret; + dev_info(pfuze_chip-dev, + Full lay: %x ,Metal lay: %x\n, + (value 0xf0) 4, value 0x0f); The space should be after not before the comma in the displayed string. + dev_info(pfuze_chip-dev, FAB: %x , FIN: %x\n, + (value 0xc) 2, value 0x3); Only a space after the , for normal formatting. ie both should look like %x, bar. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] ASoC: atmel: add wm8904 based audio machine driver
On Fri, Jul 19, 2013 at 05:42:57PM +0800, Bo Shen wrote:
Add wm8904 based audio machine driver for Atmel EK board
Applied, thanks though...
+ mclk = clk_get(NULL, pck0);
+ if (IS_ERR(mclk)) {
+ dev_err(pdev-dev, failed to get pck0\n);
+ ret = PTR_ERR(mclk);
+ goto err_set_audio;
+ }
+
+ clk_src = clk_get(NULL, clk32k);
+ if (IS_ERR(clk_src)) {
+ dev_err(pdev-dev, failed to get clk32k\n);
+ ret = PTR_ERR(clk_src);
+ goto err_set_audio;
+ }
These should really be devm_clk_get()s and use a device. This can be
revisited after the common clock framework conversion though.
signature.asc
Description: Digital signature
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v1 05/24] clk: wrap I/O access for improved portability
On Thu, Jul 18, 2013 at 10:06:57AM +0200, Sascha Hauer wrote: I think regmap has the potential to solve a number of issues like the hardcoded readl/writel in the common clock blocks, issues with i2c clocks and your endianess issue. The biggest question probably is how to get there without putting too much of a burden on you. It's probably not an option to convert all users to regmap, so it seems additional functions like clk_register_gate_regmap are better to handle. That's basically what regulator does. There's versions of the ops that all the drivers can use. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v2 01/24] spi: mpc512x: cleanup clock API use
On Thu, Jul 18, 2013 at 07:00:32PM +0200, Gerhard Sittig wrote: + psc_num = master-bus_num; + snprintf(clk_name, sizeof(clk_name), psc%d_mclk, psc_num); + mps-clk_mclk = clk_get(dev, clk_name); + if (IS_ERR(mps-clk_mclk)) + goto free_irq; Should be using devm_clk_get(). signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V2 0/2] regulator: palmas-pmic: doc: update device tree bindings
On Tue, Jul 16, 2013 at 11:41:08AM -0500, Nishanth Menon wrote: We seem to have a few missing updates to device tree bindings with the latest set of changes getting merged in. Applied both, thanks. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v1 01/24] spi: mpc512x: prepare clocks before enabling them
On Wed, Jul 17, 2013 at 01:22:29PM +0200, Gerhard Sittig wrote: On Mon, Jul 15, 2013 at 21:17 +0100, Mark Brown wrote: On Mon, Jul 15, 2013 at 08:47:30PM +0200, Gerhard Sittig wrote: sprintf(name, psc%d_mclk, master-bus_num); spiclk = clk_get(master-dev, name); - clk_enable(spiclk); + clk_prepare_enable(spiclk); mps-mclk = clk_get_rate(spiclk); clk_put(spiclk); This code is *clearly* buggy and should be fixed rather than papered over. Not only is there no matching put the driver is also dropping the reference to the clock rather than holding on to it while it's in use. This code refers to the driver's original condition, right? I agree that the driver has been suffering from incomplete clock handling since it was born three years ago. And that this issue shall get addressed. The question is just whether it needs to be part of this series which has a different focus. This is a pretty long e-mail. It'd probably have taken less time to fix the issues than to reply to the e-mail... but anyway. A big part of the issue with the state of the driver is that there's obvious clock API abuse going on that isn't corrected here - the main one is that the sprintf() for the clock name is a fairly clear warning sign, the driver should be using a fixed value there. This raises a warning flag for me about the quality of the common clock API implementation that's being sent and/or the potential for bugs to be noticed with the common clock framework. I'd not expect this code to actually work, and looking at the rest of the series I don't see how it does since I can't see what forces the name. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [patch] spi/xilinx: signedness issue checking platform_get_irq()
On Wed, Jul 17, 2013 at 03:26:38PM +0300, Dan Carpenter wrote: xspi-irq is unsigned int so it's never less than zero. I have added a cast. Why is this better than changing irq to a signed type? signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v1 01/24] spi: mpc512x: prepare clocks before enabling them
On Wed, Jul 17, 2013 at 04:26:28PM +0200, Gerhard Sittig wrote: On Wed, Jul 17, 2013 at 13:07 +0100, Mark Brown wrote: This is a pretty long e-mail. It'd probably have taken less time to fix the issues than to reply to the e-mail... but anyway. Not quite. Please consider that careful submission is as expensive as thorough review is, and that a lot of work is done before v1 gets submitted, which you may not always notice from looking at a concentrated series that may no longer suggest how much it took to get there (especially when prepared carefully). This is rather undermined the more time and effort gets spent pushing back against doing trival fixes, of course... besides, the issues here are all really simple to fix and test. It's not a major or risky rewrite of the driver or anything like that - most of this can be tested by just probing the driver. As mentioned before I did not question the need to fix issues as they get identified. I just asked whether reworking the serial OK, so send patches then. The initial COMMON_CLK implementation in part 09/24 registers clkdev items for compatibility during migration. The string isn't greppable since it's constructed by the preprocessor in the MCLK data setup, see the MCLK_SETUP_DATA_PSC() macro. Ugh, right - it didn't show up in searches due to being hidden by the macro. Now let me see how I can improve the SPI driver with regard to overall clock handling and beyond mere operation by coincidence in the absence of errors. But please understand that I don't want to stall the common clock support for a whole platform just because some of the drivers it needs to touch to keep them operational have other issues that weren't addressed yet, while these issues aren't introduced with the series but preceed it. Again, you're not being asked to implement substantial new functionality here. From my point of view I can't test this at all so I'm looking at code that's obviously not good for the standard clock API and wondering if it even works or how robust it's going to be going forward as the common clock code changes which makes me relucatant to say it'll be OK. The fact that you're switching over to use generic code is itself a reason to make sure that the API usage is sane, dodgy API usage against open coded clock implementations is less risky than against the common code. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [patch v2] spi/xilinx: signedness issue checking platform_get_irq()
On Wed, Jul 17, 2013 at 06:34:48PM +0300, Dan Carpenter wrote: In xilinx_spi_probe() we use xspi-irq to store negative error codes so it has to be signed. We weren't going to use the upper bit any way so this is fine. Applied, thanks. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH 1/2] ASoC: tlv320aic3x: Add compatible strings for specific devices
From: Mark Brown broo...@linaro.org
The driver supports a range of devices but currently doesn't allow those
device names to be used for enumeration on DT. Add the currently listed
I2C IDs as compatible strings.
Signed-off-by: Mark Brown broo...@linaro.org
---
Documentation/devicetree/bindings/sound/tlv320aic3x.txt | 8 +++-
sound/soc/codecs/tlv320aic3x.c | 2 ++
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/sound/tlv320aic3x.txt
b/Documentation/devicetree/bindings/sound/tlv320aic3x.txt
index f47c3f5..26f65f9 100644
--- a/Documentation/devicetree/bindings/sound/tlv320aic3x.txt
+++ b/Documentation/devicetree/bindings/sound/tlv320aic3x.txt
@@ -3,7 +3,13 @@ Texas Instruments - tlv320aic3x Codec module
The tlv320aic3x serial control bus communicates through I2C protocols
Required properties:
-- compatible - string - ti,tlv320aic3x
+
+- compatible - string - One of:
+ti,tlv320aic3x - Generic TLV320AIC3x device
+ti,tlv320aic33 - TLV320AIC33
+ti,tlv320aic3007 - TLV320AIC3007
+
+
- reg - int - I2C slave address
diff --git a/sound/soc/codecs/tlv320aic3x.c b/sound/soc/codecs/tlv320aic3x.c
index e5b9268..c9bb760 100644
--- a/sound/soc/codecs/tlv320aic3x.c
+++ b/sound/soc/codecs/tlv320aic3x.c
@@ -1582,6 +1582,8 @@ static int aic3x_i2c_remove(struct i2c_client *client)
#if defined(CONFIG_OF)
static const struct of_device_id tlv320aic3x_of_match[] = {
{ .compatible = ti,tlv320aic3x, },
+ { .compatible = ti,tlv320aic33 },
+ { .compatible = ti,tlv320aic3007 },
{},
};
MODULE_DEVICE_TABLE(of, tlv320aic3x_of_match);
--
1.8.3.2
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH 2/2] ASoC: tlv320aic3x: List tlv320aic3106 as a supported device
From: Mark Brown broo...@linaro.org
Currently there is no specific handling for it but the tlv320aic3106 is
supported using this driver.
Signed-off-by: Mark Brown broo...@linaro.org
---
Documentation/devicetree/bindings/sound/tlv320aic3x.txt | 1 +
sound/soc/codecs/tlv320aic3x.c | 2 ++
2 files changed, 3 insertions(+)
diff --git a/Documentation/devicetree/bindings/sound/tlv320aic3x.txt
b/Documentation/devicetree/bindings/sound/tlv320aic3x.txt
index 26f65f9..705a6b1 100644
--- a/Documentation/devicetree/bindings/sound/tlv320aic3x.txt
+++ b/Documentation/devicetree/bindings/sound/tlv320aic3x.txt
@@ -8,6 +8,7 @@ Required properties:
ti,tlv320aic3x - Generic TLV320AIC3x device
ti,tlv320aic33 - TLV320AIC33
ti,tlv320aic3007 - TLV320AIC3007
+ti,tlv320aic3106 - TLV320AIC3106
- reg - int - I2C slave address
diff --git a/sound/soc/codecs/tlv320aic3x.c b/sound/soc/codecs/tlv320aic3x.c
index c9bb760..cad4fb1 100644
--- a/sound/soc/codecs/tlv320aic3x.c
+++ b/sound/soc/codecs/tlv320aic3x.c
@@ -1492,6 +1492,7 @@ static const struct i2c_device_id aic3x_i2c_id[] = {
{ tlv320aic3x, AIC3X_MODEL_3X },
{ tlv320aic33, AIC3X_MODEL_33 },
{ tlv320aic3007, AIC3X_MODEL_3007 },
+ { tlv320aic3106, AIC3X_MODEL_3X },
{ }
};
MODULE_DEVICE_TABLE(i2c, aic3x_i2c_id);
@@ -1584,6 +1585,7 @@ static const struct of_device_id tlv320aic3x_of_match[] =
{
{ .compatible = ti,tlv320aic3x, },
{ .compatible = ti,tlv320aic33 },
{ .compatible = ti,tlv320aic3007 },
+ { .compatible = ti,tlv320aic3106 },
{},
};
MODULE_DEVICE_TABLE(of, tlv320aic3x_of_match);
--
1.8.3.2
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] ASoC: imx-sgtl5000: fix error return code in imx_sgtl5000_probe()
On Tue, Jul 16, 2013 at 08:05:07PM +0800, Wei Yongjun wrote: From: Wei Yongjun yongjun_...@trendmicro.com.cn Fix to return a negative error code from the error handling case instead of 0, as done elsewhere in this function. Applied, thanks. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 0/2] regulator: palmas-pmic: doc: update device tree bindings
On Tue, Jul 16, 2013 at 09:27:10AM -0500, Nishanth Menon wrote: On 09:23-20130716, Nishanth Menon wrote: We seem to have a few missing updates to device tree bindings with the latest set of changes getting merged in. Oops.. seems like I have an old mailID for Mark :( I'll need the actual patches as well... signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v2] regulator: pfuze100: add pfuze100 regulator driver
On Sat, Jul 13, 2013 at 12:15:12AM +0800, Robin Gong wrote:
Please fix your mail program to word wrap between paragraphs.
On Fri, Jul 12, 2013 at 03:40:37PM +0100, Mark Brown wrote:
+static const int pfuze100_swbst[] = {
+ 500, 505, 510, 515,
+};
This looks like a linear map, the steps are all 50mV?
Yes, but the swbst regulator share the same define type with the vsnvs
regulator, and the later voltage table is not linear, so I use
volt_table in swbst regulator . I don't want to add another regulator
type for this.
You should do so; it's not hard.
You should just register all the regulators rather than only registering
those that the user explicitly selects. This allows users to inpect the
current configuration and simplifies the code - for example you don't
need to count the DT nodes and you can just have a simple array in the
platform data (see how wm831x does this for an example).
Yes, it will simplifies the code, but sometimes we will not use all
the regulators on boards, in this case, Is it better that only
register the available regulators?
It's better to have everything, that way the framework can do things
like power down unused regualtors that got left enabled.
You should really be doing this on a copy of the regulators table rather
than on the table itself.
everyone of the four regulators(SW2~SW4) has two different linear
voltage table which decided by the specific bit(one regulator ,one
different bit) . So will modify the voltage table dynamically before
regulator register. I think this way is more simple , although looks
little weird and uncomfortable.
You're missing the point here. You shouldn't be modifying global data
(which should be marked as const) at all, you should be working on a
copy of it if it needs modifying.
signature.asc
Description: Digital signature
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v5 1/7] sound: codec: wm8731: add rates constraints
On Mon, Jul 15, 2013 at 04:53:46PM +0200, Richard Genoud wrote: 2013/7/12 Mark Brown broo...@kernel.org: This isn't going to work with systems which have a variable clock as the input to the CODEC. If it's imposing constraints the driver needs to allow setting the clock to zero as a way of removing constraints (and any existing drivers should be updated to do this if needed). Maybe I'm wrong, but I didn't find any system using variable clock with this codec. The driver should be written with that possibility in mind even if there were no users; it only takes a couple of lines of code. The sam9g20ek (soc/atmel/sam9g20_wm8731.c) is not using a crystal, but it's using a fixed clock anyway. But there's soc/pxa/corgi.c and soc/pxa/poodle.c that puzzle me. They seems to use a crystal, but they are setting a different sysclk depending on the rate. That seems wrong, but as I'm a newbie in ASoC... Note that the CPU is clock master for those - it's going to be outputting a clock based on the sample rate selected automatically. These boards would be broken by your change as it stands. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [Ksummit-2013-discuss] [ATTEND] Handling of devicetree bindings
On Mon, Jul 15, 2013 at 09:56:20AM -0700, Greg KH wrote: How about a hint for subsystem maintainers as to what exactly we should be looking for with these bindings? I for one have no idea what is right vs. wrong with them, so a document explaining this would be good to have. Or if we already have it, a pointer to it perhaps? At the minute it's about at the level of saying that if you're not sure or don't know you should get the devicetree-discuss mailing list to review it. Ideally someone would write that document, though I wouldn't hold my breath and there is a bunch of convention involved. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [Ksummit-2013-discuss] [ATTEND] Handling of devicetree bindings
On Mon, Jul 15, 2013 at 11:41:57AM -0700, Greg KH wrote: On Mon, Jul 15, 2013 at 06:06:00PM +0100, Mark Brown wrote: At the minute it's about at the level of saying that if you're not sure or don't know you should get the devicetree-discuss mailing list to review it. Ideally someone would write that document, though I wouldn't hold my breath and there is a bunch of convention involved. So, what should I, as a subsystem maintainer, do if I get a patch with a binding in it? Wait some random amount of time before I can just apply it because no one on the mailing list said anything about it? Trust the developer who wrote it? Or just reject them all? What do you advise? Right, this is the problem. Given that this is supposed to be about defining an ABI probably sit on it until someone with more DT experience turns up to help though obviously that isn't always constructive. Trying to figure things out enough to decide if it's sane is another option, as is helping the submitter make their binding easier to review by doing things like split out more complex elements into incremental additions or making sure they aren't deluging the DT list with lots of subsystem stuff other than bindings. Ideally a lot of this stuff will be fairly standard for the subsystem so hopefully it'll very quickly become apparent what's unusual which will help a lot. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v1 01/24] spi: mpc512x: prepare clocks before enabling them
On Mon, Jul 15, 2013 at 08:47:30PM +0200, Gerhard Sittig wrote: clocks need to get prepared before they can get enabled, fix the MPC512x PSC SPI master's initialization Signed-off-by: Gerhard Sittig g...@denx.de --- drivers/spi/spi-mpc512x-psc.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/spi/spi-mpc512x-psc.c b/drivers/spi/spi-mpc512x-psc.c index 29fce6a..76b20ea 100644 --- a/drivers/spi/spi-mpc512x-psc.c +++ b/drivers/spi/spi-mpc512x-psc.c @@ -395,7 +395,7 @@ static int mpc512x_psc_spi_port_config(struct spi_master *master, sprintf(name, psc%d_mclk, master-bus_num); spiclk = clk_get(master-dev, name); - clk_enable(spiclk); + clk_prepare_enable(spiclk); mps-mclk = clk_get_rate(spiclk); clk_put(spiclk); This code is *clearly* buggy and should be fixed rather than papered over. Not only is there no matching put the driver is also dropping the reference to the clock rather than holding on to it while it's in use. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v5 1/7] sound: codec: wm8731: add rates constraints
On Thu, Jul 11, 2013 at 06:15:53PM +0200, Richard Genoud wrote:
Please always try to use commit logs that look like normal commit logs
for the subsystem.
switch (freq) {
- case 11289600:
case 1200:
+ wm8731-constraints = wm8731_constraints_1200;
+ break;
case 12288000:
- case 16934400:
case 18432000:
- wm8731-sysclk = freq;
+ wm8731-constraints = wm8731_constraints_12288000_18432000;
+ break;
+ case 16934400:
+ case 11289600:
+ wm8731-constraints = wm8731_constraints_11289600_16934400;
break;
default:
return -EINVAL;
}
This isn't going to work with systems which have a variable clock as the
input to the CODEC. If it's imposing constraints the driver needs to
allow setting the clock to zero as a way of removing constraints (and
any existing drivers should be updated to do this if needed).
signature.asc
Description: Digital signature
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v5 2/7] sound: sam9x5_wm8731: machine driver for at91sam9x5 wm8731 boards
On Thu, Jul 11, 2013 at 06:15:54PM +0200, Richard Genoud wrote:
From: Nicolas Ferre nicolas.fe...@atmel.com
Description of the Asoc machine driver for an at91sam9x5 based board
ASoC.
+sam9x5 pins:
+ * LOUT
+ * ROUT
+ * LHPOUT
+ * RHPOUT
+ * LLINEIN
+ * RLINEIN
+ * MICIN
These aren't pins on the CPU, they're pins on the CODEC, and you should
be adding this to the binding document for the CODEC and referring to
that rather than having them in each individual binding document. This
also helps if any new variants are added (not that this is likely for
the WM8731).
+static struct sam9x5_drvdata sam9x5_priv;
Why is this a global static?
+ ret = snd_soc_register_card(snd_soc_sam9x5);
+ if (ret) {
+ dev_err(pdev-dev,
+ ASoC: Platform device allocation failed\n);
+ goto out_put_audio;
+ }
+ platform_set_drvdata(pdev, snd_soc_sam9x5);
It should be being dynamically allocated and retrieved as driver data
when needed.
signature.asc
Description: Digital signature
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v2] regulator: pfuze100: add pfuze100 regulator driver
On Fri, Jul 12, 2013 at 12:54:15PM +0800, Robin Gong wrote:
Add pfuze100 regulator driver.
This looks mostly good. A few small issues below but nothing major.
+enum pfuze_id {
+ PFUZE_ID_PFUZE100,
+ PFUZE_ID_INVALID,
+};
+struct pfuze_chip {
Missing blank line here - there are a few other small coding style
things, checkpatch should help.
+static struct regulator_ops pfuze100_ldo_regulator_ops;
+static struct regulator_ops pfuze100_fixed_regulator_ops;
+static struct regulator_ops pfuze100_sw_regulator_ops;
+static struct regulator_ops pfuze100_swb_regulator_ops;
Better to just reorder things so that no forward declaration is needed.
+static const int pfuze100_swbst[] = {
+ 500, 505, 510, 515,
+};
This looks like a linear map, the steps are all 50mV?
+ num_regulators = pfuze_get_num_regulators_dt(client-dev);
+ if (num_regulators = 0 pdata)
+ num_regulators = pdata-num_regulators;
+ if (num_regulators = 0) {
+ dev_err(client-dev, no platform data,please add it!\n);
+ ret = -EINVAL;
+ return ret;
You should just register all the regulators rather than only registering
those that the user explicitly selects. This allows users to inpect the
current configuration and simplifies the code - for example you don't
need to count the DT nodes and you can just have a simple array in the
platform data (see how wm831x does this for an example).
+ /* SW2~SW4 high bit check and modify the voltage value table */
+ if (i PFUZE100_SW1C i PFUZE100_SWBST) {
+ regmap_read(pfuze_chip-regmap, PFUZE100_SW2VOL +
+ (i - PFUZE100_SW2) * 7, val);
+ if (val 0x40) {
+ pfuze100_regulators[id].desc.min_uV = 80;
+ pfuze100_regulators[id].desc.uV_step = 5;
+ }
+ }
You should really be doing this on a copy of the regulators table rather
than on the table itself.
+ for (i = 0; i pfuze_chip-num_regulators; i++)
+ regulator_unregister(pfuze_chip-regulators[i]);
+ kfree(pfuze_chip);
Use devm_kzalloc().
signature.asc
Description: Digital signature
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC PATCH V2 1/8] regulator: Introduce OMAP regulator to control PMIC over VC/VP
On Tue, Jul 09, 2013 at 11:04:23AM -0500, Nishanth Menon wrote: On 07/09/2013 10:29 AM, Mark Brown wrote: This seems like something we should be able to cope with by for example adding a bus for the custom PMIC interface or otherwise finding a way to I had considered introducing a custom bus for custom PMIC interface, but as you stated below, standard regulators will probably need some custom monkeying around. We should just be able to use the platform bus here. get to the data at runtime based on the compatible string. This would need some custom code in the regulators but would have the advantage of keeping the data the same at least. Hrm. Ofcourse,this will be to add custom set/get_voltage implementation using this custom API which we discussed was'nt that good an idea. No, if the regulator isn't being interacted with directly then it doesn't need to export any operations - just data. The operations would come from the magic SoC hardware that controls the regulator. The code in the drivers should be very small, if it isn't there's no point in doing this. between PMICs? yep, twl4030 does 4mV/uSec, 6030 can do 6mV/uSec, TPS62361 can do 32mV/uSec, TWL6035/37 does 0.220mV/uSec Those are ramp rates, they're not I2C I/O limits. Ramp rates we already know about. I think what you're saying here is that this latency value is actually about worst case ramp times? Arrgh.. my bad. I confused ramp time with I2C transfer timeout parameter. I know that I2C bus can be held[1] by PMIC as long as it is busy. Some custom ASIC can do some weird stuff I suppose. I dont seem to have clear data points in the sketchy TRMs for 6030/2 , 6035, 5030, for these other than to state it is i2c specification compliant (/me grumbles). So, I just have emperical value which is a bit conservative and seem to work on the devices. OK, no problem - like we said further up the thread I think adding something to get the data signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v4 1/7] sound: sam9x5_wm8731: machine driver for at91sam9x5 wm8731 boards
On Wed, Jul 10, 2013 at 11:25:37AM +0200, Richard Genoud wrote: 2013/7/9 Mark Brown broo...@kernel.org: Shouldn't the SSC driver be enforcing this constraint if it comes from the SSC hardware? If the clock is reprogrammable the usual convention for drivers is to not constrain if the clock is set to zero so a machine driver could remove the constraint. Actually, my comment is buggy here (or at least, unrelated to the authorized rates). The MCLK_RATE is the master clock of the wm8731 codec (a 12.288MHz crystal). According to the datasheet of wm8731, when a 12.288 crystal is used, the authorized rates are 8, 32, 48 and 96kHz (I have to remove 16 and 64kHz). So, is this the right place for the rates ? No, the CODEC driver should be enforcing the restrictions if that's where they come from. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v4 3/7] Documentation: DT: update atmel SSC with DMA binding
On Wed, Jul 10, 2013 at 11:48:27AM +0200, Richard Genoud wrote: 2013/7/9 Mark Brown broo...@kernel.org: ...this first example is now invalid and should probably just be being extended with the new required properties. Well, I have to rewrite that to make it clearer. The thing is: with atmel,at91rm9200-ssc the SSC doesn't work with DMA. with atmel,at91sam9g45-ssc, the SSC work ONLY with DMA. So the dmas/dma-names properties are only required for g45-ssc, and useless for rm9200-ssc Maybe the best will be to write a paragraph for g45-ssc and another for rm9200-ssc, even if there's some identical lines between them. OK, or just write a section Required for devices with compatible X. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] mfd: DT bindings for the palmas family MFD
On Mon, Jun 03, 2013 at 03:18:51PM +0100, Lee Jones wrote: On Mon, 03 Jun 2013, J Keerthy wrote: + optional chip specific regulator fields :- + ti,warm-reset - maintain voltage during warm reset(boolean) Pushing the boat out a bit here, but is it possible to reuse 'regulator-always-on' for this? This sounds more like don't reset over reboot than never change the enable state. + ti,roof-floor - control voltage selection by pin(boolean) Is this the same as a GPIO regulator? If so, you might not need to add superfluous vendor specific properties. Lots of regulators have the ability to do things like switch between programmable voltages based on GPIOs (enabling a fast change to a known voltage) - the roof-floor naming sounds like this. Usually there's also register based element for selecting the voltage. See: Documentation/devicetree/bindings/regulator/gpio-regulator.txt + ti,sleep-mode - mode to adopt in pmic sleep 0 - off, 1 - auto, + 2 - eco, 3 - forced pwm I've seen lots of sleep-mode properties, can't we define a generic one? We should make some of this more standard (at least things like voltages) but the whole concept of what sleep mode is is at best fuzzy. You typically need different selections for suspend to RAM and suspend to disk, plus often the suspend configuration is dynamic depending on what the system is doing since suspend is just CPU suspend not system suspend and there's also some changes that might happen depending on which wake sources are currently available. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v1 4/4] spi/xilinx: Use of_property_read_u32 for reading value from node
On Tue, Jul 09, 2013 at 07:26:27AM +0200, Michal Simek wrote: On 07/08/2013 04:51 PM, Mark Brown wrote: Applied, thanks. have you applied this patch? Yes, network is spotty here so pushes aren't working so well. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v2 1/5] sound: sam9x5_wm8731: machine driver for at91sam9x5 wm8731 boards
On Tue, Jul 09, 2013 at 10:19:45AM +0200, Richard Genoud wrote: 2013/7/8 Mark Brown broo...@kernel.org: The obvious question here is of course if we can use the same driver for both of them. I haven't got a g20 to test that, but that's the goal. I think I do somewhere. For now, g20 is still non-DT, so I think it's best to have a DT-only driver like this one for the 9x5 family (9g15, 9g25, 9x25, 9g35, 9x25). When the g20 will move to DT completely, we can drop sam9g20_wm8731.c and adjust sam9x5_wm8731.c (mainly master clock and widgets it seems) By the way, maybe g45 could use that also (and SAMA5 ?) If this is the goal then this driver needs a more generic name. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v2 2/5] ARM: AT91: DTS: sam9x5: add SSC DMA parameters
On Tue, Jul 09, 2013 at 10:27:53AM +0200, Richard Genoud wrote: The dma binding is documented here Documentation/devicetree/bindings/dma/atmel-dma.txt But you're right, I shoud update Documentation/devicetree/bindings/misc/atmel-ssc.txt. No, you need to write a binding for this machine driver - the DMA and SSC need to be documented as well but so too does the audio driver that pulls them together with the CODEC. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v4 0/7] Sound support for at91sam9x5-wm8731 based boards
On Tue, Jul 09, 2013 at 04:25:26PM +0200, Richard Genoud wrote: [I've just seen that I forgot to cc Uwe to the cover letter, sorry for that.] This is the second or third copy of this patch set I've been sent *today*. Worse, everything seems to be being sent in reply to the same thread increasing the nesting substantially. Please slow down, it's flooding my mailbox and discouraging me from looking since it seems likely that there will just be another resend. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v4 1/7] sound: sam9x5_wm8731: machine driver for at91sam9x5 wm8731 boards
On Tue, Jul 09, 2013 at 04:25:27PM +0200, Richard Genoud wrote:
+/*
+ * Authorized rates are:
+ * Rate = MCLK_RATE / (n * 2)
+ * Where n is in [1..4095]
+ * (cf register SSC_CMR)
+ */
+static unsigned int rates[] = {
+ 8000,
+ 16000,
+ 32000,
+ 48000,
+ 64000,
+ 96000,
+};
Shouldn't the SSC driver be enforcing this constraint if it comes from
the SSC hardware? If the clock is reprogrammable the usual convention
for drivers is to not constrain if the clock is set to zero so a machine
driver could remove the constraint.
+ ret = atmel_ssc_set_audio(0);
+ if (ret != 0) {
+ dev_err(pdev-dev,
+ ASoC: Failed to set SSC 0 for audio: %d\n, ret);
+ return ret;
+ }
Shouldn't this be a parameter in the DT too?
+ cpu_np = of_parse_phandle(np, atmel,ssc-controller, 0);
+ if (!cpu_np) {
+ dev_err(pdev-dev, ssc controller node missing\n);
+ ret = -EINVAL;
+ goto out;
+ }
+ at91sam9x5ek_dai.cpu_of_node = cpu_np;
+ at91sam9x5ek_dai.platform_of_node = cpu_np;
After all we're looking things up in the DT...
+ at91sam9x5ek_dai.dai_fmt = snd_soc_of_parse_daifmt(np, atmel,);
Is this really something that machines would want to reconfigure? If so
why?
signature.asc
Description: Digital signature
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v4 2/7] Documentation: DT: add sam9x5ek-wm8731 machine driver
On Tue, Jul 09, 2013 at 04:25:28PM +0200, Richard Genoud wrote: This add the sound DT binding for sam9x5ek-wm8731 machine driver Signed-off-by: Richard Genoud richard.gen...@gmail.com --- .../bindings/sound/atmel-sam9x5-wm8731-audio.txt | 30 Put new binding documents in the same patch that reads them, this makes review easier. + - atmel,audio-routing: A list of the connections between audio components. This needs to be more specific and list the available board specific nodes for routing. For the CODEC you can just refer to the CODEC binding documentation. + - atmel,format: DAI format. Must be i2s So why not just omit this then? + - atmel,bitclock-master: DAI clock master + - atmel,frame-master: DAI frame master The driver isn't handling these and there's no information on how to set them. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v4 3/7] Documentation: DT: update atmel SSC with DMA binding
On Tue, Jul 09, 2013 at 04:25:29PM +0200, Richard Genoud wrote:
- reg: Should contain SSC registers location and length
- interrupts: Should contain SSC interrupt
+For dma transfer:
+- dmas: DMA specifier, consisting of a phandle to DMA controller node,
+ the memory interface and SSC DMA channel ID (for tx and rx).
+ See Documentation/devicetree/bindings/dma/atmel-dma.txt for details.
+- dma-names: Must be tx, rx.
This is added as a required property so...
-Example:
+Examples:
ssc0: ssc@fffbc000 {
compatible = atmel,at91rm9200-ssc;
reg = 0xfffbc000 0x4000;
interrupts = 14 4 5;
};
...this first example is now invalid and should probably just be being
extended with the new required properties.
signature.asc
Description: Digital signature
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC PATCH V2 1/8] regulator: Introduce OMAP regulator to control PMIC over VC/VP
On Mon, Jul 08, 2013 at 12:22:36PM -0500, Nishanth Menon wrote: case #1 - TPS62361 has a single SMPS and a single generic i2c bus, one can talk on. In this case, you'd associate the regulator device in one place - i2cX or on custom SoC hardware interface. When used with custom SoC hardware interface, generic tps62361 regulator which talks regular i2c wont even probe for omap_pmic to get the regulator data from tps62361 regulator driver. Even if we were to define the generic TPS62361 in dts nodes, it will fail probe as it cant access any of it's registers using generic i2c. This seems like something we should be able to cope with by for example adding a bus for the custom PMIC interface or otherwise finding a way to get to the data at runtime based on the compatible string. This would need some custom code in the regulators but would have the advantage of keeping the data the same at least. Hrm. Another option is for the drivers to provide the data and use the helpers for their linear ranges as part of a more complex implementation. Having looked at a few regulator driver implementations, there seems to be the following combinations: a) drivers which have n ranges of linear voltages for incremental selector b) drivers with 1 range of linear voltages only for all ranges of selectors c) drivers with 1 range of linear voltages and nonlinear voltage values for other vsels. Everything else is just a special case of option a - either there's just a single range or there's a bunch of ranges each with a single value. I suspect that ranges will be the most useful thing for any hardware which can practically be used by these regulators anyway. OK, that's a bit more fun but I think the kernel wants that information in general anyway since a software cpufreq driver or something might want to make the same latency decisions. This is what set_voltage_time() is for in part. But to a first approximation is there really much variation in the numbers? between PMICs? yep, twl4030 does 4mV/uSec, 6030 can do 6mV/uSec, TPS62361 can do 32mV/uSec, TWL6035/37 does 0.220mV/uSec Those are ramp rates, they're not I2C I/O limits. Ramp rates we already know about. I think what you're saying here is that this latency value is actually about worst case ramp times? signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH] of/platform: Staticize of_platform_device_create_pdata()
From: Mark Brown broo...@linaro.org It is not used outside of this file so doesn't need to be in the global namespace. Signed-off-by: Mark Brown broo...@linaro.org --- drivers/of/platform.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/of/platform.c b/drivers/of/platform.c index e0a6514..b0d1ff8 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -196,7 +196,7 @@ EXPORT_SYMBOL(of_device_alloc); * Returns pointer to created platform device, or NULL if a device was not * registered. Unavailable devices will not get registered. */ -struct platform_device *of_platform_device_create_pdata( +static struct platform_device *of_platform_device_create_pdata( struct device_node *np, const char *bus_id, void *platform_data, -- 1.8.3.2 ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v9 08/10] ASoC: Add phycore-ac97-dt driver
On Sun, Jul 07, 2013 at 09:19:49AM +0200, Markus Pargmann wrote:
On Wed, Jul 03, 2013 at 05:17:27PM +0100, Mark Brown wrote:
+extern void pca100_ac97_cold_reset(struct snd_ac97 *ac97);
+extern void pca100_ac97_warm_reset(struct snd_ac97 *ac97);
+#else
+static void pca100_ac97_cold_reset(struct snd_ac97 *ac97) { }
+static void pca100_ac97_warm_reset(struct snd_ac97 *ac97) { }
These functions have no reason to be anywhere except in the driver and
really you should just be specifying which pins to use there - ideally
via pinctrl but I don't think i.MX has adopted that yet.
There are no pinctrl driver for imx27(pca100) or imx35(pcm043). The
iomux/pad configure functions are defined inside mach-imx/ including
their headers. Both reset functions have to configure iomux/pad before
and after using gpio. So I think it's not possible to make the reset
based on the gpio pins without the necessary iomux/pad configuration.
That all sounds fixable - either make pinctrl available or make the
headers usefully visible. You can see the arch headers in the build
of the entire kernel...
signature.asc
Description: Digital signature
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v9 04/10] ASoC: fsl-ssi: Add support for imx-pcm-fiq
On Sat, Jul 06, 2013 at 07:13:23PM +0200, Markus Pargmann wrote: On Wed, Jul 03, 2013 at 05:06:37PM +0100, Mark Brown wrote: On Thu, Jun 20, 2013 at 03:20:23PM +0200, Markus Pargmann wrote: + ssi_private-use_dma = !of_property_read_bool(np, fsl,imx-fiq); This binding should be documented. I'm not sure it really needs to be a binding, though - is it not possible for the driver to just figure out that DMA won't work automatically (for example by looking at the CODEC in use)? I'm not sure this is a good name either, it should be saying why the FIQ is needed rather than saying that we should use the FIQ. I think fsl_ssi_startup is the first function in which we know which codec is connected to fsl-ssi. There we have access to the pcm runtime, which stores the codec used. But that is too late for the ssi setup. I could use of_find_compatible_node to search for the wm9712 codec, but that would assume that there is only one codec attached to the system. If the SSI is doing anything it will be connected to a CODEC. If it's not connected to a CODEC then it doesn't need to be configured at all so it doesn't really matter. Perhaps fsl,fiq-filter-codec-stream is a better name for the binding? Possibly. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [alsa-devel] [PATCH v2 3/5] ARM: AT91: DTS: sam9x5ek: add WM8731 codec
On Mon, Jul 08, 2013 at 03:29:51PM +0200, Richard Genoud wrote: The WM8731 codec on sam9x5ek board is on i2c, address 1A Signed-off-by: Richard Genoud richard.gen...@gmail.com Acked-by: Mark Brown broo...@linaro.org This should go via arm-soc. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC PATCH V2 1/8] regulator: Introduce OMAP regulator to control PMIC over VC/VP
On Fri, Jul 05, 2013 at 08:55:07AM -0500, Nishanth Menon wrote: Please write in paragraphs, an enormous wall of unbroken text isn't helpful for legibility. On 16:41-20130704, Mark Brown wrote: So, this still has the thing where all the data about the PMIC is replicated (but now in this driver). It should be possible to pull all the above information except possibly the I2C timeout and perhaps the I2C address (if there's a separate control interface) from the standard regulator core data structures for the PMIC. This would allow the driver to Just Work with any PMIC without needing to add it in two places. option 1) we just bypass get_voltage/set_voltage to point through to this function. result will be something similar to what we got here[1] I don't really know what you mean when you say bypass get_voltage/set_voltage so it's kind of hard to comment... the link you posted appears to be a link to the code I'm reviewing so it's not terribly enlightening. Again, based on this discussion, it is wrong - and we already did implement the *wrong* way in OMAP and the new code is supposed to throw away the old code once all relevant platforms are converted to DT. - now, we could improve this by passing rdev and slurp out required data from regulator structures, but obviously, as you pointed out, it wont be sufficient. - In this solution, we will have existing regulator drivers supporting additional data path. *but* that also means that existing regulator drivers will need to be modified to handle multiple data path and Just Work wont happen - so not really good other than screw up existing regulator drivers with handling OMAP specific APIs in them. What makes you think that the existing regulator drivers need to be modified? They already have all the data exported to the core (or should do)... the only thing that's missing is the timeouts and it's not at all obvious why those need to be tuned per device. option 2) make omap_pmic regulator use twl_regulator - regulator chaining. Again I don't know what this is. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC PATCH V2 1/8] regulator: Introduce OMAP regulator to control PMIC over VC/VP
On Fri, Jul 05, 2013 at 12:33:10PM -0500, Nishanth Menon wrote: Taking an example of twl-regulator and omap_pmic, are you suggesting omap_pmic to be a user twl-regulator using include/linux/regulator/consumer.h? or are you suggesting that omap_pmic should not be a regulator at all? No, I'm suggesting that omap_pmic find the TWL driver data at runtime (eg, using the device tree to locate the relevant regulator) and get the information out of the regulator driver that way. It can then tell the hardware about the data that way without having to explictly add every single regulator both standalone and to the OMAP driver. There's no information about how to use this register in your bindings... but anyway, can't be too hard to add this if it's actually used. Yes it is, and also happens to be how OMAPs achieve maximum power savings - when low power modes are achieved in OMAP(automatic hardware assisted commands are send to the specific command registers in PMIC and viceversa on wakeup) - but this also happens to be very specific to OMAP way of handling things. I can refer to the Reference Manual as to how it actually works, but that'd be an overkill, I will try to expand on the bindings a little more, I guess. OK, so this is a register defined by the OMAP architecture? I think it's reasonable to add something to allow this to be obtained to the core, using a DT property seems yucky since every board usingt this is going to have to cut'n'paste the value. Some sort of custom parameter readback thing perhaps, it doesn't have to be too generic. Anything that implements a custom set_voltage() won't work with your data structure either... It would not work with OMAP either ;). But that said, drivers do Yes, that's kind of my point - as with the code Paul was implementing it doesn't matter if you can't support every single regualtor since the hardware design constrains what the regulator can do. The regualtor framework already has helpers which factor out the code for anything which has the limiations the OMAP hardware has (or where it doesn't we could add them) so there shouldn't be any need for a driver to provide custom callbacks. freely implement custom set_voltage/get_voltage primarily because there are ranges in supported voltages that are non-linear and try to be generic to work on non-OMAP platforms as well. However, within the supported range, only the linear ranges are used with OMAP. OK, that's a bit more interesting but I expect such regulators will actually work with the linear ranges helpers I added the other day (Marvell had a PMIC using them and I realised that the same pattern can be applied to a bunch of other devices). Do you think that'd cover the cases you're aware of? Another option is for the drivers to provide the data and use the helpers for their linear ranges as part of a more complex implementation. OMAP VC hardware has no idea about how long to wait before giving up on an ongoing i2c transaction. This may depend on PMIC and what it does before acking on i2c. So pick a high number (it's only for error cases...)? from hardware perspective yeah, if it does not lockup (based on erratas on specific devices ;) ). it also controls in part, the latency of response to Voltage processor from Voltage controller also needed for computing SmartReflex latencies (as it should consider worst case conditions) OK, that's a bit more fun but I think the kernel wants that information in general anyway since a software cpufreq driver or something might want to make the same latency decisions. This is what set_voltage_time() is for in part. But to a first approximation is there really much variation in the numbers? signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC PATCH V2 1/8] regulator: Introduce OMAP regulator to control PMIC over VC/VP
On Fri, Jun 21, 2013 at 04:25:42PM -0500, Nishanth Menon wrote:
+static const struct omap_pmic_info omap_twl4030_vdd1 = {
+ .slave_addr = 0x12,
+ .voltage_reg_addr = 0x00,
+ .cmd_reg_addr = 0x00,
+ .i2c_timeout_us = 200,
+ .slew_rate_uV = 4000,
+ .step_size_uV = 12500,
+ .min_uV = 60,
+ .max_uV = 145,
+ .voltage_selector_offset = 0,
+ .voltage_selector_mask = 0x7F,
+ .voltage_selector_setbits = 0x0,
+ .voltage_selector_zero = false,
+};
So, this still has the thing where all the data about the PMIC is
replicated (but now in this driver). It should be possible to pull all
the above information except possibly the I2C timeout and perhaps the
I2C address (if there's a separate control interface) from the standard
regulator core data structures for the PMIC. This would allow the
driver to Just Work with any PMIC without needing to add it in two
places.
Other than that this looks good, the only issue I see is where the
driver is getting the data from.
signature.asc
Description: Digital signature
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: Appended DTB files for multi-machine kernels
On Thu, Jul 04, 2013 at 06:56:24PM +0200, Daniel Mack wrote: Unless I missed some recent discussion, this case is not easy to handle. Yes, I know that these kind of things should be handled by a next-generation bootloader, but in our case, we want to avoid a loader update of already shipped hardware by all means. There was some discussion about appending multiple DTBs recently. I can't actually recall anything about it though so that's not an entirely helpful thing... As a solution, I'm thinking of a small framework that could for example work as follows. a) A small mechanism allows storing multiple DTB binary files inside the kernel binary at compile time, and a simple function can extract them again by name at runtime (something like what the firmware framework does, but I don't know if that one can be used at such an early stage in the boot process). Another way of skinning this would be for either the kernel to contain a set of machine ID to compatible string mappings or for the device trees for the boards to have an additional properties giving the machine IDs that the boards match. The kernel could then look for multiple DTBs appended to the image and try to pick one based on ATAGs. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 1/1] of/documentation: Update s5m8767-regulator bindings document
On Mon, Jun 24, 2013 at 03:06:57PM +0530, Sachin Kamat wrote: s5m8767 regulator is used on Exynos platforms which use pin controller to configure GPIOs. Update the example accordingly. Applied, thanks. Please use subject lines that match the subsystem and try to make your changelogs clearer. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v9 02/10] ASoC: imx-pcm-fiq: Introduce pcm-fiq-params
On Thu, Jun 20, 2013 at 03:20:21PM +0200, Markus Pargmann wrote: Cleaner parameter passing for imx-pcm-fiq. Create a seperated fiq-params struct to pass all arguments. Applied, thanks. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v9 04/10] ASoC: fsl-ssi: Add support for imx-pcm-fiq
On Thu, Jun 20, 2013 at 03:20:23PM +0200, Markus Pargmann wrote: + ssi_private-use_dma = !of_property_read_bool(np, fsl,imx-fiq); + This binding should be documented. I'm not sure it really needs to be a binding, though - is it not possible for the driver to just figure out that DMA won't work automatically (for example by looking at the CODEC in use)? I'm not sure this is a good name either, it should be saying why the FIQ is needed rather than saying that we should use the FIQ. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v9 03/10] ASoC: fsl: Move soc_ac97_ops from imx-ssi to fsl_ssi
On Thu, Jun 20, 2013 at 03:20:22PM +0200, Markus Pargmann wrote: fsl_ssi and imx-ssi can be both enabled at the same time. To be able to add AC97 support to fsl_ssi, soc_ac97_ops have to be available to both drivers. This needs to be redone on top of the changes that went into v3.11 to support multiplatform AC'97 - it won't apply any more. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v9 09/10] ASoC: fsl: Move fsl-ssi binding doc to sound/
On Thu, Jun 20, 2013 at 03:20:28PM +0200, Markus Pargmann wrote: fsl-ssi was located in powerpc/fsl/ssi.txt. This is no powerpc specific device, so it should be moved to sound/ as it connects to differen audio codecs. Applied, thanks. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v9 01/10] ASoC: imx-pcm-dma: DT support
On Thu, Jun 20, 2013 at 03:20:20PM +0200, Markus Pargmann wrote: This patch removes the NO_DT flag. The pdev pointer may have a proper of_node with the dmas property, so we can use it to request DMA channels. Applied, thanks. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v9 08/10] ASoC: Add phycore-ac97-dt driver
On Thu, Jun 20, 2013 at 03:20:27PM +0200, Markus Pargmann wrote:
Notes:
Changes in v9:
- Fix blank line at end of file.
Please don't include enormous changelogs like this, they're just noise.
+config SND_SOC_PHYCORE_AC97_DT
+ bool SoC Audio support for Phytec phyCORE (and phyCARD) boards
(devicetree only)
+ depends on MACH_PCA100 || MACH_PCM043
Is there an actual dependency on the machine type? This seems wrong for
a DT driver.
+static struct snd_soc_ops imx_phycore_hifi_ops = {
+};
+
You shouldn't have this if there's nothing in it.
+static struct platform_device *imx_phycore_snd_device;
This shouldn't be a global, it should be in driver data.
+/*
+ * Pointer to AC97 reset functions for specific boards
+ */
+#if IS_ENABLED(CONFIG_MACH_PCA100)
+extern void pca100_ac97_cold_reset(struct snd_ac97 *ac97);
+extern void pca100_ac97_warm_reset(struct snd_ac97 *ac97);
+#else
+static void pca100_ac97_cold_reset(struct snd_ac97 *ac97) { }
+static void pca100_ac97_warm_reset(struct snd_ac97 *ac97) { }
+#endif
+ if (of_machine_is_compatible(phytec,imx27-pca100)) {
+ phycore_ac97_reset = pca100_ac97_cold_reset;
+ phycore_ac97_warm_reset = pca100_ac97_warm_reset;
+ } else if (of_machine_is_compatible(phytec,imx35-pcm043)) {
+ phycore_ac97_reset = pcm043_ac97_cold_reset;
+ phycore_ac97_warm_reset = pcm043_ac97_warm_reset;
+ } else {
+ dev_err(pdev-dev, Failed to set AC97 reset functions,
unknown board.\n);
+ return -EINVAL;
+ }
These functions have no reason to be anywhere except in the driver and
really you should just be specifying which pins to use there - ideally
via pinctrl but I don't think i.MX has adopted that yet.
signature.asc
Description: Digital signature
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v9 10/10] ASoC: fsl: Update fsl-ssi binding doc
On Thu, Jun 20, 2013 at 03:20:29PM +0200, Markus Pargmann wrote: Update the fsl-ssi bindings. DMA is no required property anymore and uses the generic DMA bindings. imx-fiq is a new alternative to DMA These updates should be done as part of the patches making the code changes - see the review comment I had earlier for an example. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC PATCH v4 2/2] drivers: mfd: vexpress: add Serial Power Controller (SPC) support
On Tue, Jun 18, 2013 at 11:22:13AM +0100, Lorenzo Pieralisi wrote: On Mon, Jun 17, 2013 at 06:44:51PM +0100, Olof Johansson wrote: That'll trim down the driver to a point where I think you'll find it much easier to get merged. :-) To start with I have to understand in which directory this code should live. Moving the frequency settings in clk/CPUfreq drivers should be feasible with extra DT complexity for their bindings. You shoudn't really need to change the DT bindings at all - MFD is a purely Linux construct, it doesn't affect how the hardware is constructed. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH -next] spi: tegra114: remove redundant dev_err call in tegra_spi_probe()
On Tue, Jul 02, 2013 at 08:37:44AM +0800, Wei Yongjun wrote: From: Wei Yongjun yongjun_...@trendmicro.com.cn There is a error message within devm_ioremap_resource already, so remove the dev_err call to avoid redundant error message. Applied, thanks. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] ASoC: mop500: add .owner to struct snd_soc_card
On Tue, Jul 02, 2013 at 05:25:37PM +0800, Wei Yongjun wrote: From: Wei Yongjun yongjun_...@trendmicro.com.cn Add missing .owner of struct snd_soc_card. This prevents the module from being removed from underneath its users. Applied, thanks. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH] of/platform: Staticize of_platform_device_create_pdata()
From: Mark Brown broo...@linaro.org It is not used outside of this file so doesn't need to be in the global namespace. Signed-off-by: Mark Brown broo...@linaro.org --- drivers/of/platform.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/of/platform.c b/drivers/of/platform.c index e0a6514..b0d1ff8 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -196,7 +196,7 @@ EXPORT_SYMBOL(of_device_alloc); * Returns pointer to created platform device, or NULL if a device was not * registered. Unavailable devices will not get registered. */ -struct platform_device *of_platform_device_create_pdata( +static struct platform_device *of_platform_device_create_pdata( struct device_node *np, const char *bus_id, void *platform_data, -- 1.8.3.1 ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 1/1] of/documentation: Update s5m8767-regulator bindings document
On Tue, Jun 25, 2013 at 11:56:12AM +0530, Sachin Kamat wrote: There is no change in the bindings, but just a correction in the documentation to reflect the implementation. Earlier when Samsung platforms did not have pinctrl driver, legacy GPIO driver was used which took those 5 parameters. Now since we are using pinctrl, we need only 3 parameters. The document was somehow not updated to reflect this change. So there was a previous change to the code that mistakenly didn't update the binding document? signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] ASoC: Add PCM1681 codec driver.
On Wed, Jun 26, 2013 at 03:16:56PM +0200, Daniel Mack wrote: Hi Marek, On 26.06.2013 15:05, Marek Belisko wrote: PCM1681 can be controlled via I2C, SPI or in bootstrap mode. This code add support only for I2C mode. Guys, please delete unneeded context from replies so it's possible to find the content - otherwise people are wading through screen after screen to find what you added. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] ASoC: Add PCM1681 codec driver.
On Wed, Jun 26, 2013 at 03:05:28PM +0200, Marek Belisko wrote:
+#define PCM1681_ATT_CONTROL(X) (X = 6 ? X : X + 9) /* Attenuation
level */
Write a function for this.
+static bool pcm1681_writeable_reg(struct device *dev, unsigned register reg)
+{
+ return pcm1681_accessible_reg(dev, reg)
+ (reg != PCM1681_ZERO_DETECT_STATUS);
+}
+static int pcm1681_digital_mute(struct snd_soc_dai *dai, int mute)
+{
+ struct snd_soc_codec *codec = dai-codec;
+ struct pcm1681_private *priv = snd_soc_codec_get_drvdata(codec);
+ int ret, val = 0;
+
+ if (mute)
+ val = PCM1681_SOFT_MUTE_ALL;
+
This would be clearer if written as an if .. else - otherwise it looks
like an uninitalised value might be used.
+static int pcm1681_hw_params(struct snd_pcm_substream *substream,
+ struct snd_pcm_hw_params *params,
+ struct snd_soc_dai *dai)
+{
+ struct snd_soc_codec *codec = dai-codec;
+ struct pcm1681_private *priv = snd_soc_codec_get_drvdata(codec);
+ int val = 0;
+ int pcm_format = params_format(params);
+
+ priv-rate = params_rate(params);
+
Shouldn't there be a call to set the deemphasis here?
+static int pcm1681_probe(struct snd_soc_codec *codec)
+{
+ return 0;
+}
+
+static int pcm1681_remove(struct snd_soc_codec *codec)
+{
+ return 0;
+}
Remove empty functions.
signature.asc
Description: Digital signature
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v3 03/10] pinctrl:st: Add pinctrl and pinconf support.
On Thu, Jun 20, 2013 at 03:05:38PM +0100, Srinivas KANDAGATLA wrote: This patch add pinctrl support to ST SoCs. About hardware: ST Set-Top-Box parts have two blocks called PIO and PIO-mux which handle pin configurations. Applied, thanks. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v3 03/10] pinctrl:st: Add pinctrl and pinconf support.
On Mon, Jun 24, 2013 at 01:57:56PM +0200, Linus Walleij wrote: This seems fairly complete, but I cannot have such a basic dependency onto the regmap tree this late in the merge window, i.e. I'm not ready to pull all of regmap into the pinctrl tree. I'd consider this for merging for the next kernel cycle so it's more orthogonal. I've got the patch on a topic branch in regmap to allow for this sort of stuff - I can readily provide a signed tag for you to get just that patch if that'd help? Alternatively I'm happy to carry the pinctrl patch in regmap? signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCHv12] spi: omap2-mcspi: add generic DMA request support to the DT binding
On Mon, Jun 24, 2013 at 05:31:59PM +0530, Sourav Poddar wrote: The binding definition is based on the generic DMA request binding Applied, thanks. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 1/1] of/documentation: Update s5m8767-regulator bindings document
On Mon, Jun 24, 2013 at 03:06:57PM +0530, Sachin Kamat wrote: s5m8767 regulator is used on Exynos platforms which use pin controller to configure GPIOs. Update the example accordingly. This smells bad, why does a driver using GPIOs through the GPIO API see a change in the binding? signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v12 09/11] spi: omap2-mcspi: convert to dma_request_slave_channel_compat()
On Fri, Jun 21, 2013 at 04:07:51PM +0530, Sekhar Nori wrote: We can resend the patch if you don't have it from the mailing list. I'll probably have it assuming it's been sent to some mailing list I read (the CC list here looks absurldy large...) but if you don't send me the patch and/or ignore bounces then it's going to take longer. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 1/2] regulator: of: Added a property to indicate bypass mode support
On Thu, Jun 20, 2013 at 02:07:37PM +0530, Kishon Vijay Abraham I wrote: Added a property to indicate if the regulator supports bypass mode. Also modified of_get_regulation_constraints() to check for that property and set appropriate constraints. Applied, thanks. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v3 4/4] regulator: Palmas: Add TPS659038 support
On Wed, Jun 19, 2013 at 11:27:50AM +0530, Keerthy wrote: From: J Keerthy j-keer...@ti.com Add TPS659038 support. Signed-off-by: J Keerthy j-keer...@ti.com This doesn't apply against my current tree as the PMIC bindings document isn't in mainline yet. Acked-by: Mark Brown broo...@linaro.org assuming there's a tree where that does exist. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v3 1/4] MFD: Palmas: Check if irq is valid
On Wed, Jun 19, 2013 at 11:27:47AM +0530, Keerthy wrote: From: J Keerthy j-keer...@ti.com Check if irq value obtained is valid. If it is not valid then skip the irq request step and go ahead with the probe. Signed-off-by: J Keerthy j-keer...@ti.com Reviewed-by: Mark Brown broo...@linaro.org signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] MFD: Change TWL6025 references to TWL6032
On Wed, Jun 19, 2013 at 03:24:02PM +0300, Oleksandr Kozaruk wrote: There are non-mainline branches that use twl6032 by its name (for example git://git.omapzoom.org/kernel/omap.git). There is intention to add support of twl6032 device in mainline, but we'd like to know if we can use twl6032 instead of twl6025 in our new patches, that we are going to provide. Related discussion: https://patchwork.kernel.org/patch/2686331/ It's always been OK to use the new name, the only question was if it was better to keep the old name supported as well. Given that the chips are essentially nonexistant now your current approach seems sensible so Reviwed-by: Mark Brown broo...@linaro.org signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v4 02/10] pinctrl: mvebu: dove pinctrl driver
On Tue, Jun 18, 2013 at 04:11:16PM +0100, Russell King - ARM Linux wrote: On Tue, Jun 18, 2013 at 05:02:49PM +0200, Linus Walleij wrote: Nowadays I would do the above with regmap_update_bits(). Mutual exclusion for read-modify-write of individual bits in a register is one of those cases where doing a regmap over a memory-mapped register range makes a lot of sense. (drivers/mfd/syscon.c being a nice example) So, for that solution we need to have some kind of global regmap per register or somesuch. Then you run into regmap needing a struct device - well, with a shared register, which struct device do you use, or do you have to invent one? That sounds more heavy-weight than is really necessary. Yes, regmap is far too heavyweight for a lot of these things - it shouldn't really go in performance critical at the CPU level paths, it's designed around things where I/O costs are considerable and worth avoiding. I do think that this is a very good idea - with the I2C/SPI drivers I was reviewing prior to regmap there were always just far too many simple bugs with read/modify/write cycles so factoring out the code really helped with review. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v3 1/2] spi: omap2-mcspi: Move bytes per word calculation to the function
On Fri, Jun 14, 2013 at 07:12:07PM +0300, Illia Smyrnov wrote: Introduce mcspi_bytes_per_word function as replacement for the next code fragment: Applied, thanks. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v3 2/2] spi: omap2-mcspi: Add FIFO buffer support
On Fri, Jun 14, 2013 at 07:12:08PM +0300, Illia Smyrnov wrote: The MCSPI controller has a built-in FIFO buffer to unload the DMA or interrupt handler and improve data throughput. This patch adds FIFO buffer support for SPI transfers in DMA mode. This looks good but doesn't apply against my topic/omap branch - can you please check and resend? Probably something that applies against -next will be fine. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v4] spi: omap2-mcspi: Add FIFO buffer support
On Mon, Jun 17, 2013 at 04:31:06PM +0300, Illia Smyrnov wrote: The MCSPI controller has a built-in FIFO buffer to unload the DMA or interrupt handler and improve data throughput. This patch adds FIFO buffer support for SPI transfers in DMA mode. Applied, thanks. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC PATCH 1/4] regulator: Introduce OMAP regulator to control PMIC over VC/VP
On Thu, Jun 13, 2013 at 08:39:50AM -0500, Nishanth Menon wrote: I am having a bit of a difficulty trying to understand your concern here. Your device tree for this stuff appears to mostly consist of repeating the description of the PMIC that we already have - this really doesn't seem like a great result. Problem statement: OMAP has this weird custom h/w where one programs the voltage and that voltage is send over i2c - this is not same as Tegra's lookup table array which automatically sends out entries, in OMAP, software has to trigger the voltage transition The basic idea that's important here is that you need to figure out how to tell the hardware what to write - how those writes get triggered is a separate problem. If your concern was describing PMIC parameters in dts, I can easily move them inside the omap_pmic driver and provide required compatible flags. If, on the other hand, the entire approach followed is flawed, I'd like to understand the rationale for the same. That's the biggest problem I saw so far but to be honest I've not drilled down too much into the specifics. From my point of view the main thing is how this fits into the frameworks and so on, having the register information in the DT was an alarm flag that suggested the overall approach was a concern. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC PATCH 1/4] regulator: Introduce OMAP regulator to control PMIC over VC/VP
On Thu, Jun 13, 2013 at 09:58:03AM -0500, Nishanth Menon wrote: I am proposing moving the following into OF match data. ti,i2c-slave-address ti,i2c-voltage-register ti,i2c-command-register ti,slew-rate-microvolt ti,step-size-micro-volts ti,voltage-selector-set-bits ti,voltage-selector-mask ti,voltage-selector-offset ti,non-zero-voltage-selector The only thing I propose to retain is board specific variations - e.g. gpios, boot voltage and standard regulator min,max overrides if any. I can also do voltage selector based operations while at it. OK, this sounds like a step in the right direction. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v2 2/2] spi: omap2-mcspi: Add FIFO buffer support
On Tue, Jun 11, 2013 at 08:09:15PM +0300, Illia Smyrnov wrote: The MCSPI controller has a built-in FIFO buffer to unload the DMA or interrupt handler and improve data throughput. This patch adds FIFO buffer support for SPI transfers in DMA mode. The FIFO could be enabled for SPI module by setting up the ti,spi-fifo-enabled configuration parameter in DT. If FIFO enabled, the largest possible FIFO buffer size will be calculated and setup for each SPI transfer. Even if the FIFO is enabled in DT, it won't be used for the SPI transfers when: calculated FIFO buffer size is less then 2 bytes or the FIFO buffer size isn't multiple of the SPI word length. Why is the default to disable the FIFO? signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] ASoC: rt5640: add device tree support
On Tue, Jun 11, 2013 at 02:40:40PM -0600, Stephen Warren wrote:
+- realtek,ldo1-en-gpios : The GPIO that controls the CODEC's LDO1_EN pin.
Why gpios and not gpio?
- if (rt5640-pdata.ldo1_en) {
+ if (gpio_is_valid(rt5640-pdata.ldo1_en)) {
ret = devm_gpio_request_one(i2c-dev, rt5640-pdata.ldo1_en,
GPIOF_OUT_INIT_HIGH,
Unfortunately gpio_is_valid() is unhelpful for platform data since often
zero is a valid GPIO but it's also the default do nothing platform
data. It's therefore better to either include a check for non-zero as
well or have code that takes a zero in the platform data and sets it to
a negative value instead.
signature.asc
Description: Digital signature
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] ASoC: rt5640: add device tree support
On Wed, Jun 12, 2013 at 10:56:46AM -0600, Stephen Warren wrote: On 06/12/2013 10:46 AM, Mark Brown wrote: Unfortunately gpio_is_valid() is unhelpful for platform data since often zero is a valid GPIO but it's also the default do nothing platform data. It's therefore better to either include a check for non-zero as well or have code that takes a zero in the platform data and sets it to a negative value instead. I think people filling in platform data should simply be required to put a valid/correct value in that field. There aren't any users of this, so it's not like adding a new field where backwards-compatibility might be a concern (and even then, updating all users of this platform data type wouldn't be hard), and it should be obvious if you get it wrong. It's not idiomatic to do it this way, it'll catch people out - sadly the effects are often non-obvious, depending on what actually happens with the GPIO. Were it not for renumbering pain it'd probably be most sensible to just make 0 never a valid GPIO :/ signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V2] ASoC: rt5640: add device tree support
On Wed, Jun 12, 2013 at 11:34:30AM -0600, Stephen Warren wrote: From: Stephen Warren swar...@nvidia.com Modify the RT5640 driver to parse platform data from device tree. Write a DT binding document to describe those properties. Applied, thanks. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] ASoC: tegra: add tegra+RT5640 machine driver
On Wed, Jun 12, 2013 at 11:35:34AM -0600, Stephen Warren wrote: + RT5640 pins: + + * DMIC1 + * DMIC2 + * MICBIAS1 + * IN1P + * IN1R + * IN2P + * IN2R + * HPOL + * HPOR + * LOUTL + * LOUTR + * MONOP + * MONON + * SPOLP + * SPOLN + * SPORP + * SPORN These should really go in the binding document for the CODEC and be referenced there in case new packaging variants turn up or something. I know we've not done that for some older drivers but would still be a good idea to start, if nothing else it saves typing for anyone making any other machine drivers with the same CODEC. Anyway I'll apply, this can be changed later. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [patch -next] spi: spi-xilinx: cleanup a check in xilinx_spi_txrx_bufs()
On Sun, Jun 09, 2013 at 04:07:28PM +0300, Dan Carpenter wrote: '!' has higher precedence than comparisons so the original condition is equivalent to if (xspi-remaining_bytes == 0). This makes the static checkers complain. Applied, thanks. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC PATCH 1/4] regulator: Introduce OMAP regulator to control PMIC over VC/VP
On Wed, May 22, 2013 at 01:18:34PM -0500, Nishanth Menon wrote: So, the biggest problem here has been patch 4 (having to have a hack to deploy this stuff is a bit worrying) plus the general not having a real driver thing. +- ti,i2c-slave-address - I2C slave address of the PMIC +- ti,i2c-voltage-register - I2C register address where voltage commands are + to be set. +- ti,i2c-command-register - I2C register address where commands are to be set + when OMAP enters low power state. This may be the same as + ti,i2c-voltage-register depending on the PMIC. +- ti,slew-rate-microvolt - worst case slew rate of rise / fall for voltage + transition in microvolts per microseconds (uV/uS) +- step-size-micro-volts - Step size in micovolts as to what one step in voltage + selector increment translates to. See example. +- regulator-min-microvolt - Minimum voltage in microvolts which is supported by + the PMIC in ti,step-size-microvolt increments. See example. +- regulator-max-microvolt - Maximum voltage in microvolts which is supported + by the PMIC in ti,step-size-microvolt increments. See example. The other thing is this whole business of encoding the properties of the PMIC in the DT like this. Paul Walmsley has started doing some work for some similiar hardware where instead of doing this the regulator is in the DT as normal and then the driver for the offloaded voltage scaling gets the information about the register layout from the regulator driver. This is a bit neater overall and would cope with determining which method to use at runtime. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC PATCH 1/4] regulator: Introduce OMAP regulator to control PMIC over VC/VP
On Mon, Jun 10, 2013 at 11:16:59AM -0500, Nishanth Menon wrote: On Mon, Jun 10, 2013 at 5:31 AM, Mark Brown broo...@kernel.org wrote: On Wed, May 22, 2013 at 01:18:34PM -0500, Nishanth Menon wrote: So, the biggest problem here has been patch 4 (having to have a hack to deploy this stuff is a bit worrying) plus the general not having a real driver thing. Patch #4 in this series was a hack as it was not properly split up and organized as a proper DTS series -it was meant as a proof of concept - not entirely meant to indicate the remaining 1-3 patches were hacks :). The way it reads is that you're building up to a hack - if what you've done isn't enabling a sensible solution there might be a problem with the earlier steps. I think you mean http://marc.info/?t=13705924913r=1w=2 series. I will dig into it. if it is possible for Tegra and OMAP to use the same framework and strategy to deal with these kind of h/w blocks, all the more better. Not just better, if each system doing this sort of thing needs to reinvent the wheel something is going wrong. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V5] ASoC: fsl: add imx-wm8962 machine driver
On Fri, Jun 07, 2013 at 08:26:14PM +0800, Nicolin Chen wrote:
+
+struct imx_priv {
+ int hp_gpio;
+ int hp_active_low;
+ int hp_status;
+ int amic_gpio;
+ int amic_active_low;
+ int amic_status;
These are just recorded in the struct but never actually seem to be
used?
+ struct platform_device *pdev;
+ struct snd_pcm_substream *first_stream;
+ struct snd_pcm_substream *second_stream;
These aren't used any more.
signature.asc
Description: Digital signature
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V3] ASoC: WM8962: Add device tree binding
On Fri, Jun 07, 2013 at 11:23:27AM +0800, Nicolin Chen wrote: Document the device tree binding for the WM8962 codec, and modify the driver to extract platform data from the device tree, if present. Applied, thanks. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V3] ASoC: fsl: add imx-wm8962 machine driver
On Fri, Jun 07, 2013 at 01:59:47PM +0800, Nicolin Chen wrote: +Optional properties: +- hp-det-gpios : The gpio port of Headphone jack detection. +- mic-det-gpios: The gpio port of Micphone jack detection. These still need better documentation to say what they are electrically. Like I say I really don't expect that your hp-det-gpio is actually detecting headphones at all, I think it's detecting if something is present in the jack. Otherwise this looks good. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 1/2] mmc: dw_mmc: Handle late vmmc regulator with EPROBE_DEFER
On Fri, Jun 07, 2013 at 12:19:58PM +0200, Tomasz Figa wrote: On Thursday 06 of June 2013 21:46:45 Doug Anderson wrote: dw_mmc is probed. This regulator is optional, though a warning will be printed if it's missing. The fact that the regulator is optional means that (at the moment) it's not possible to use a regulator that probes _after_ dw_mmc. Fix this limitation by adding the ability to make vmmc required. If a vmmc-supply is specified in the device tree we'll assume that vmmc is required. This interesting case makes me think that regulator core should differentiate between regulator lookup failure due to no lookup specified and due to the regulator specified in lookup being unavailable, returning appropriate (different) error codes. It does exactly that in so far as it can - you get -ENODEV if there's definitely no supply and -EPROBE_DEFER otherwise. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V4] ASoC: fsl: add imx-wm8962 machine driver
On Fri, Jun 07, 2013 at 06:14:13PM +0800, Nicolin Chen wrote: +Optional properties: +- hp-det-gpios : The gpio pin to detect plug in/out event that happens to + Headphone jack. + +- mic-det-gpios: The gpio pin to detect plug in/out event that happens to + Micphone jack. So this is for physically distinct headphone and microphone jacks rather than a headset? signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 1/2] ASoC: WM8962: Create default platform data structure
On Thu, Jun 06, 2013 at 11:21:26AM +0800, Nicolin Chen wrote:
Hmm..sorry I don't fully get it. Does that mean I should do something like:
struct wm8962_priv {
struct wm8962_pdata pdata;
instead of pointer? so no devm_kzalloc() for it any more?
Yes.
signature.asc
Description: Digital signature
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [net-next PATCH v4 1/5] net: cpsw: enhance pinctrl support
On Thu, Jun 06, 2013 at 11:29:39AM +0530, Mugunthan V N wrote: On 6/6/2013 12:53 AM, Mark Brown wrote: Linus Walleij posted some patches which factor the state setting code out into generic functions earlier on today - it probably makes sense to pick those up rather than open coding But this can go in as Linus Walleij's patch is not accepted yet. Once that is accepted and present in net git repo I will submit a separate patch to use those APIs from pin ctrl core. Linus' change has pretty much gone in already but in any case what would be sensible here would be to write this in turns of Linus' changes and then send the patch to him to add to his series so it can go in via the same route. One of the major reasons for the patch was that lots of people were querying the amount of noise caused by this sort of change. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V2] ASoC: fsl: add imx-wm8962 machine driver
On Thu, Jun 06, 2013 at 12:39:20PM +0800, Nicolin Chen wrote: On Wed, Jun 05, 2013 at 12:55:44PM +0100, Mark Brown wrote: +- hp-det-gpios : The gpio port of Headphone detection. +- mic-det-gpios: The gpio port of Micphone detection. I'd say a bit more about these - obviously the CODEC has its own accessory detection here, and I rather suspect that in fact the HP detection you have there is really jack detection. hasn't been included in this patch though. So since hardware has pins here, I think it's better to put in the binding doc as well. But I guess it'd be better to put it into OPTIONAL area? Yes, they need to be optional and you need to explain what the function of these pins is so people can tell what they are. As I said I really do expect that the pin you have labelled headphone detect is really a jack detect pin. The sample rate checking can be done with the symmmetric_rates feature in the core. The sample_bits check can't be but it's something that a lot of systems probably ought to have so it ought to be factored out into the core too rather than open coded in the driver. [Nic:] fsl_ssi.c uses symmmetric_rates, but last time I traced soc-pcm.c and found the core only cares about symmmetric_rates during pcm_open(), which means the startup() in dai/machine driver, while what I'm considering is something like arecord -Dhw:0 -r 44100 | aplay -Dhw:0 -r 48000: The two startup() would run almost at the same time and each of them would be earlier than hw_param() -- the exact point we can get the sample rate from application, but before this point no sample-rate actually be set to make hw_constraint(). Then the two hw_param() would continue their code and successively call set_fll() twice with different sample rates. But I agree that it's kinda ugly to put code here. So I think maybe I need to drop this part of code and to think about another patch for the core? Yes, you certainly need to drop this patch. There is no way of fixing this with the current ALSA APIs, they are fundamentally racy and you will always have the possibility of failures during simultaneous startup for hardware with these limits. You're enabling and disabling the CODEC only while there's an audio stream active. This means that it's not possible to support analogue bypass paths (which the device can do) - you should probably also support enabling the FLL via set_bias_level() and use set_bias_level() to turn it off (which works in all cases). [Nic] I've seen and tested set_bias_level() way as samsung/tobermory.c does. But I find a flaw to the method that if I play two and more wav files like 'aplay -Dhw:0 audio44Khz.wav audio48Khz.wav audio22Khz.wav', which needs to set_fll() before each playback and to keep SYSCLK valid by changing its source to MCLK after the playback, but I only found that hw_param() and hw_free() are symmetrically called before/after each playback in this case. Two things here. One is that of course there's no reference counting on FLL enables, they just happen, and the other is that as you'd expect the bias level callbacks just won't happen for the second stream as the device is powered up. You only need to set the clocking up for the first stream as the clocking can't be changed when the device is active. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [Arm-netbook] getting allwinner SoC support upstream (was Re: Uploading linux (3.9.4-1))
On Thu, Jun 06, 2013 at 02:01:14AM +0200, Tomasz Figa wrote: I don't see any other solution here than moving all the Allwinner code to DT (as it has been suggested in this thread several times already), as this is the only hardware description method supported by ARM Linux. Well, the server guys are working on ACPI - people could contribute to that effort instead if they preferred (though I can see that they might not!). signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 1/2] spi: spi-omap2-mcspi.c: Add dts for slave device configuration.
On Thu, Jun 06, 2013 at 01:08:46PM +0300, Illia Smyrnov wrote: On 06/05/2013 02:57 PM, Mark Brown wrote: Turbo mode gives the expected results not for all cases. There are some limitations: - works only if a single channel is enabled (no effect when several channels are enable); - improves the throughput on RX direction only; - effective only when a transfer exceeds two words. For single SPI word transfers OMAP TRM [1] recommends deactivate turbo mode. So it is useful to have the property in DT, that allow us to switch turbo mode off/on for certain slave. Why? These sound like conditions that can readily be detected at runtime. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 2/2] spi: omap2-mcspi: Add FIFO buffer support
On Thu, Jun 06, 2013 at 01:08:56PM +0300, Illia Smyrnov wrote: On 06/05/2013 03:03 PM, Mark Brown wrote: Why is this defined for slaves? Surely the size of the FIFO in the controller is a property of the controller not the slave? According to OMAP TRM [1] the FIFO buffer can be used by only one channel at a time. If several channels are selected and several FIFO enable bit fields are set to 1, the controller forces the buffer not to be used. The controller ought to be able to figure this out for itself. As a first pass just grabbing the FIFO on a first come first served basis will probably work well most of the time, the device would have to be very active for it to constantly be doing transfers on all channels. If there's more contention than that we probably ought to be looking at how we handle this in general, it seems like we'd have more problems than just the FIFO to worry about. If there are several slaves on the controller we must select which of slaves will use the FIFO for SPI transfers. Also, optimal FIFO A single controller is only going to be able to talk to one slave at once, everything on the bus except chip select is shared. size is heavily dependent of the SPI transfers length specific for certain slave. The transfer length doesn't seem like something that we want to be encoding in DT, particularly not indirectly - it is obviously readily available at runtime, variable during runtime (eg, firmware download may do large transfers on a device that only does small transfers most of the time) and is something that updates to the drivers could change. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V2 1/2] ASoC: WM8962: Create default platform data structure
On Thu, Jun 06, 2013 at 07:38:45PM +0800, Nicolin Chen wrote: Embed a copy of struct wm8962_pdata in stuct wm8962_priv so that there's no need to check validity of pdata any more. Applied, thanks. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V2 2/2] ASoC: WM8962: Add device tree binding
On Thu, Jun 06, 2013 at 07:38:46PM +0800, Nicolin Chen wrote: +Optional properties: + - spk-mono: Default register value for SPK_MONO of R51 (Class D Control 2). This doesn't correspond to the code. This is a boolean property, if it's present then that bit gets set indicating that the speaker is in mono mode. + - mic-cfg : Default register value for R48 (Additional Control 4). +If absent, the default is 0. Again if it's absent the default should be the register default. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V2] ASoC: fsl: add imx-wm8962 machine driver
On Thu, Jun 06, 2013 at 08:49:53PM +0800, Nicolin Chen wrote:
On Wed, Jun 05, 2013 at 12:55:44PM +0100, Mark Brown wrote:
Since it's easy to define a fixed rate clock (there's a generic driver
for that) I'd just require the user to provide a clock API clock and fix
the rate using that. This is going to be less error prone and makes the
code simpler.
I tried to use fixed rate clock as below:
data-codec_clk = devm_clk_get(codec_dev-dev, NULL);
if (IS_ERR(data-codec_clk)) {
of_fixed_clk_setup(codec_np);
data-codec_clk = clk_get(NULL, codec_np-name);
No, this is silly. Have the board define the clock in the DT.
signature.asc
Description: Digital signature
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [alsa-devel] [PATCH V2] ASoC: fsl: add imx-wm8962 machine driver
On Thu, Jun 06, 2013 at 11:02:43AM -0300, Fabio Estevam wrote: You could try something like (pass the real clock in the device tree): http://permalink.gmane.org/gmane.linux.alsa.devel/107614 (I will re-post this patch later) That's exactly what I'm suggesting. signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V2] ASoC: fsl: add imx-wm8962 machine driver
On Wed, Jun 05, 2013 at 04:21:41PM +0800, Nicolin Chen wrote:
Looks pretty good, a few comments but they're all fairly minor.
+ WM8962 pins:
+ * HPOUTL
+ * HPOUTR
+ * SPKOUTL
+ * SPKOUTR
+ * MICBIAS
+ * IN3R
+ * DMIC
+ * DMICDAT
No need to list all these, just reference the CODEC (and add them to the
CODEC binding documentation if they're not there which they probably
aren't). This is generally good practice if there's a family of devices
sharing a driver and means that you don't have to worry about missing
all the other input pins as you've done here. :)
+- hp-det-gpios : The gpio port of Headphone detection.
+- mic-det-gpios: The gpio port of Micphone detection.
I'd say a bit more about these - obviously the CODEC has its own
accessory detection here, and I rather suspect that in fact the HP
detection you have there is really jack detection.
+static int check_hw_params(struct snd_pcm_substream *substream,
+ snd_pcm_format_t sample_format, unsigned int sample_rate)
+{
+ struct imx_priv *priv = card_priv;
+ struct device *dev = substream-pcm-card-dev;
+
+ substream-runtime-sample_bits =
+ snd_pcm_format_physical_width(sample_format);
+ substream-runtime-rate = sample_rate;
+ substream-runtime-format = sample_format;
+
+ if (!priv-first_stream) {
+ priv-first_stream = substream;
+ } else {
+ priv-second_stream = substream;
+
+ if (priv-first_stream-runtime-rate !=
+ priv-second_stream-runtime-rate) {
+ dev_err(dev, KEEP THE SAME SAMPLE RATE: %d!\n,
+ priv-first_stream-runtime-rate);
+ return -EINVAL;
+ }
+ if (priv-first_stream-runtime-sample_bits !=
+ priv-second_stream-runtime-sample_bits) {
+ snd_pcm_format_t first_format =
+ priv-first_stream-runtime-format;
+
+ dev_err(dev, KEEP THE SAME FORMAT: %s!\n,
+ snd_pcm_format_name(first_format));
+ return -EINVAL;
+ }
+ }
The sample rate checking can be done with the symmmetric_rates feature
in the core. The sample_bits check can't be but it's something that a
lot of systems probably ought to have so it ought to be factored out
into the core too rather than open coded in the driver.
+ /*
+ * SYSCLK of wm8962's not disabled yet. If we wanna close FLL,
No '.
+ ret = snd_soc_dai_set_sysclk(codec_dai, WM8962_SYSCLK_MCLK,
+ data-clk_frequency, SND_SOC_CLOCK_IN);
+ if (ret 0) {
+ dev_err(dev, Failed to set SYSCLK: %d\n, ret);
+ return ret;
+ }
+
+ /* Disable FLL and let codec do pm_runtime_put() */
+ ret = snd_soc_dai_set_pll(codec_dai, WM8962_FLL, 0, 0, 0);
+ if (ret 0) {
+ dev_err(dev, Failed to disable FLL: %d\n, ret);
+ return ret;
You're enabling and disabling the CODEC only while there's an audio
stream active. This means that it's not possible to support analogue
bypass paths (which the device can do) - you should probably also
support enabling the FLL via set_bias_level() and use set_bias_level()
to turn it off (which works in all cases).
+ data-codec_clk = clk_get(codec_dev-dev, NULL);
+ if (IS_ERR(data-codec_clk)) {
devm_clk_get().
+ /* assuming clock enabled by default */
+ data-codec_clk = NULL;
+ ret = of_property_read_u32(codec_np, clock-frequency,
+ data-clk_frequency);
+ if (ret) {
+ dev_err(codec_dev-dev,
+ clock-frequency missing or invalid\n);
+ goto fail;
+ }
Since it's easy to define a fixed rate clock (there's a generic driver
for that) I'd just require the user to provide a clock API clock and fix
the rate using that. This is going to be less error prone and makes the
code simpler.
+ } else {
+ data-clk_frequency = clk_get_rate(data-codec_clk);
+ clk_prepare_enable(data-codec_clk);
Not needed immediately but if there is actually a clock we have control
of we might want to vary the frequency...
signature.asc
Description: Digital signature
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 1/2] spi: spi-omap2-mcspi.c: Add dts for slave device configuration.
On Wed, Jun 05, 2013 at 02:39:57PM +0300, Illia Smyrnov wrote: +SPI Controller specific data in SPI slave nodes: +- The spi slave nodes can provide the following information which is used + by the spi controller: + - ti,spi-turbo-mode: Set turbo mode for this device. + What is turb mode and why would we not want to just enable it all the time? Based on this documentation it's not really possible to tell... signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 2/2] spi: omap2-mcspi: Add FIFO buffer support
On Wed, Jun 05, 2013 at 02:39:58PM +0300, Illia Smyrnov wrote:
- The spi slave nodes can provide the following information which is used
by the spi controller:
- ti,spi-turbo-mode: Set turbo mode for this device.
+ - ti,spi-fifo-depth: Enable FIFO and set up buffer depth.
Why is this defined for slaves? Surely the size of the FIFO in the
controller is a property of the controller not the slave?
+ bytes_per_word = (cs-word_len = 8) ? 1 :
+ (cs-word_len = 16) ? 2 :
+ /* cs-word_len = 32 */ 4;
This isn't legible. Use a switch statement, or better yet just divide.
+ level = (t-len mcspi-fifo_depth ? t-len :
+ mcspi-fifo_depth) - 1;
+
+ mcspi_write_reg(master, OMAP2_MCSPI_XFERLEVEL,
+ ((wcnt 16) | (level (is_read ? 8 : 0;
+
+ chconf |= is_read ? OMAP2_MCSPI_CHCONF_FFER :
+ OMAP2_MCSPI_CHCONF_FFET;
Please avoid such extensive use of the ternery operator, it's not good
for legibility.
+ } else {
+ mcspi-fifo_depth = 0;
+ chconf = ~(is_read ? OMAP2_MCSPI_CHCONF_FFER :
+ OMAP2_MCSPI_CHCONF_FFET);
+ }
+
+ mcspi_write_chconf0(spi, chconf);
We have a bunch of return statements further up the function in cases
where the FIFO can't be used which means that if we're in FIFO mode then
hit one of those we'll not disable FIFO mode as far as I can tell?
signature.asc
Description: Digital signature
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 1/2] ASoC: WM8962: Create default platform data structure
On Wed, Jun 05, 2013 at 08:12:55PM +0800, Nicolin Chen wrote:
struct wm8962_priv {
+ struct wm8962_pdata *pdata;
More idiomatic style for this is to just embed a copy of the platform
data struct in the private data then copy any driver model platform data
on top of it. This simplifies usage as now the driver can assume that
there is platform data available at all times.
signature.asc
Description: Digital signature
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 2/2] ASoC: WM8962: Add device tree binding
On Wed, Jun 05, 2013 at 08:12:56PM +0800, Nicolin Chen wrote: + - gpio-cfg : A list of GPIO configuration register values. The list must +be 6 entries long. If absent, no configuration of these registers is +performed. And note that the max value for each entry is 0x, don't +fill a large one. It is nicer for users if out of band values are ignored, providing them with a method for specifying the default register value without having to actually encode it. This is both more legible (they explicitly say use the default, I don't care) and less work (as the default doesn't need to be checked). signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [net-next PATCH v4 1/5] net: cpsw: enhance pinctrl support
On Wed, Jun 05, 2013 at 10:38:15PM +0530, Mugunthan V N wrote: From: Hebbar Gururaja gururaja.heb...@ti.com Amend cpsw controller to optionally take a pin control handle and set the state of the pins to: - default on boot, resume - sleep on suspend() Linus Walleij posted some patches which factor the state setting code out into generic functions earlier on today - it probably makes sense to pick those up rather than open coding signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
